Skip to content

fix(backend): revert argMaxIf to argMax in readDevicesCF query#1625

Merged
riderx merged 1 commit into
mainfrom
fix/readdevicescf-argmaxif
Feb 11, 2026
Merged

fix(backend): revert argMaxIf to argMax in readDevicesCF query#1625
riderx merged 1 commit into
mainfrom
fix/readdevicescf-argmaxif

Conversation

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

@WcaleNieWolny WcaleNieWolny commented Feb 11, 2026

Summary (AI generated)

  • Reverted argMaxIf(blob5, timestamp, blob5 != '') back to argMax(blob5, timestamp) in the readDevicesCF query (cloudflare.ts:602)

Motivation (AI generated)

Commit 5a4828b97 ("Gate device custom_id from /stats", #1615) introduced argMaxIf in the device list query to preserve the last non-empty custom_id when newer /stats pings write '' to blob5. However, argMaxIf is not a supported function in Cloudflare Analytics Engine SQL API. The query fails with "unknown function call: ARGMAXIF", the error is silently caught, and an empty [] is returned to the client.

This broke POST /private/devices for all apps, all users — the console device list has been returning empty since #1615 was merged.

Beyond the missing function, the argMaxIf approach was also logically flawed:

  • It made it impossible to unset a device's custom_id — empty values were always ignored in favor of older non-empty ones, regardless of the allow_device_custom_id setting.
  • It contradicted the write side, which intentionally strips custom_id to '' when allow_device_custom_id is false.

argMax(blob5, timestamp) is correct: last write wins. If the write side clears custom_id, the read side should reflect that.

Business Impact (AI generated)

Critical / P0. The device list in the Capgo console has been completely broken for all customers since #1615 was merged. No user can see their devices. This is a core feature of the dashboard and directly impacts customer experience and trust.

Test Plan (AI generated)

  • Verified argMaxIf fails in CF Analytics Engine SQL Studio: "Error: Input was invalid: unknown function call: ARGMAXIF"
  • Verified argMax query returns correct results in CF Analytics Engine SQL Studio (11 rows for com.shelf.app)
  • Verified device_info dataset has 338M+ records with active writes (latest: 2026-02-11 14:32:15)
  • Deployed to preprod (bun run deploy:cloudflare:api:preprod) and tested POST https://api.preprod.capgo.app/private/devices — returns non-empty results
  • Deploy to production and verify console device list works

Generated with AI

Summary by CodeRabbit

  • Bug Fixes
    • Device custom ID now consistently reflects the most recent value, including when the ID has been cleared, so dashboards and exports match the current device state.
    • Reduces cases where an outdated non-empty custom ID persisted after removal, improving data accuracy in device views and reports.
    • No UI changes; users may notice corrected custom ID values appearing as blank when recently cleared.

argMaxIf is not supported by Cloudflare Analytics Engine SQL API,
causing the entire device list query to fail silently and return [].
This broke /private/devices for all apps since 5a4828b.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 11, 2026

📝 Walkthrough

Walkthrough

A SQL query modification in the Cloudflare utilities file changes the custom_id extraction logic from a conditional aggregate (argMaxIf with a non-empty guard) to an unconditional aggregate (argMax), allowing the most recent value to be selected regardless of whether it is empty.

Changes

Cohort / File(s) Summary
SQL Query Modification
supabase/functions/_backend/utils/cloudflare.ts
Changed custom_id extraction from argMaxIf(blob5, timestamp, blob5 != '') to argMax(blob5, timestamp), removing the guard that filtered out empty values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A guard once stood at the gate so tall,
Filtering empty values, letting none through at all,
But now it's gone, and the newest shall reign,
Whether full or empty, it matters the same!
The rabbit hops on, with queries so swift,
One line gained, three lost—a logical shift! 🌙

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: reverting argMaxIf to argMax in the readDevicesCF query, which directly addresses the critical bug.
Description check ✅ Passed The PR description includes a comprehensive summary and motivation, but is missing the required 'Test Plan' and 'Checklist' sections from the template, though test details are embedded in the description body.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/readdevicescf-argmaxif

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@riderx riderx merged commit af3d748 into main Feb 11, 2026
12 checks passed
@riderx riderx deleted the fix/readdevicescf-argmaxif branch February 11, 2026 14:55
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants