Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaced three SQL conditions checking the isAnonymous flag with COALESCE-based checks so NULL/missing isAnonymous values are treated as non-anonymous. Changes are in loadUsersByCountry, loadDailyActiveUsers, and loadRecentlyActiveUsers within the metrics route; no exported/public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant M as Metrics Route
participant DB as Database
C->>M: GET /internal/metrics?include_anonymous={flag}
alt include_anonymous = true
M->>DB: Run queries without isAnonymous exclusion
note right of DB: Return all rows
else include_anonymous = false
M->>DB: Run queries with filter:<br/>COALESCE(data->>'isAnonymous','false') != 'true'
note right of DB: Exclude rows where isAnonymous = 'true',<br/>treat NULL as 'false'
end
DB-->>M: Aggregated results (country / daily / recent)
M-->>C: JSON metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
✨ Finishing Touches
🧪 Generate unit tests
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. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR fixes a critical data handling bug in the metrics API where NULL values in the isAnonymous field were causing incorrect user filtering. The changes update three SQL queries in /apps/backend/src/app/api/latest/internal/metrics/route.tsx to use COALESCE("data"->>'isAnonymous', 'false') != 'true' instead of the previous direct comparison NOT "data"->>'isAnonymous' = 'true'.
The root issue stems from PostgreSQL's NULL handling behavior - when isAnonymous is NULL, the original condition NOT "data"->>'isAnonymous' = 'true' would evaluate to NULL (not true), causing those records to be incorrectly excluded from non-anonymous user queries. The fix ensures that NULL values are treated as 'false' (non-anonymous), which aligns with the default behavior established in the database schema migration 20250320223454_anonymous_users where the isAnonymous column defaults to false.
The changes are applied consistently across three key analytics functions:
loadUsersByCountry: Filters users by country while respecting anonymous user preferencesloadDailyActiveUsers: Counts distinct active users per dayloadRecentlyActiveUsers: Retrieves recently active user data
This fix ensures accurate metrics reporting and prevents undercounting of users in the analytics dashboard, maintaining data integrity across the user tracking system.
Confidence score: 5/5
- This PR is safe to merge with minimal risk as it fixes a clear data handling bug with a well-understood solution
- Score reflects the straightforward nature of the fix using standard SQL NULL handling patterns and consistent application across affected queries
- No files require special attention as the changes are isolated to a single metrics file with clear, targeted fixes
1 file reviewed, no comments
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/app/api/latest/internal/metrics/route.tsx (1)
41-45: Narrow types before Object.fromEntries to satisfy Record<string, number> and avoid nullable keys.Filtering after map doesn’t narrow the tuple’s key type; this can trip strict TS. Filter first with a type guard.
Apply:
- const rec = Object.fromEntries( - a.map(({ userCount, countryCode }) => [countryCode, Number(userCount)]) - .filter(([countryCode, userCount]) => countryCode) - ); + const rec = Object.fromEntries( + a + .filter( + (row): row is { countryCode: string; userCount: bigint } => + row.countryCode !== null + ) + .map(({ countryCode, userCount }) => [countryCode, Number(userCount)]) + );Optional: if counts can exceed 2^53-1, return stringified counts instead of Number to avoid precision loss.
🧹 Nitpick comments (3)
apps/backend/src/app/api/latest/internal/metrics/route.tsx (3)
30-31: COALESCE tweak is correct; consider casting to boolean to avoid string compares.Avoid text comparison on JSON. Casting yields simpler, safer predicates.
Apply:
-AND (${includeAnonymous} OR COALESCE("data"->>'isAnonymous', 'false') != 'true') +AND (${includeAnonymous} OR COALESCE(("data"->>'isAnonymous')::boolean, false) = false)
90-96: Simplify COUNT with FILTER and normalize userId to text.FILTER is clearer and casting avoids JSON identity pitfalls (e.g., 1 vs "1").
Apply:
- COUNT(DISTINCT CASE WHEN (${includeAnonymous} OR COALESCE("data"->>'isAnonymous', 'false') != 'true') THEN "data"->'userId' ELSE NULL END) AS "dau" + COUNT(DISTINCT ("data"->>'userId')) + FILTER (WHERE ${includeAnonymous} OR COALESCE(("data"->>'isAnonymous')::boolean, false) = false) AS "dau"
149-151: Mirror the boolean cast pattern here for consistency.Keep all isAnonymous checks using boolean casting.
Apply:
-AND (${includeAnonymous} OR COALESCE("data"->>'isAnonymous', 'false') != 'true') +AND (${includeAnonymous} OR COALESCE(("data"->>'isAnonymous')::boolean, false) = false)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/backend/src/app/api/latest/internal/metrics/route.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/backend/src/app/api/latest/**
📄 CodeRabbit inference engine (AGENTS.md)
apps/backend/src/app/api/latest/**: Organize backend API routes by resource under /api/latest (e.g., auth at /api/latest/auth/, users at /api/latest/users/, teams at /api/latest/teams/, oauth providers at /api/latest/oauth-providers/)
Use the custom route handler system in the backend to ensure consistent API responses
Files:
apps/backend/src/app/api/latest/internal/metrics/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
apps/backend/src/app/api/latest/internal/metrics/route.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: lint_and_build (latest)
- GitHub Check: build (22.x)
- GitHub Check: all-good
- GitHub Check: restart-dev-and-test
- GitHub Check: docker
- GitHub Check: docker
- GitHub Check: build (22.x)
- GitHub Check: setup-tests
- GitHub Check: Security Check
🔇 Additional comments (2)
apps/backend/src/app/api/latest/internal/metrics/route.tsx (2)
182-209: Request schema alignment check for API contract.users_by_country returns an object map. If any client expects a Map, confirm serialization. Given JSON, object is correct; just verifying downstream expectations.
68-69: No update required for isAnonymous filters.
The ProjectUser.isAnonymous field is defined as non-nullable Boolean with a default of false in schema.prisma and the SQL migration, so it never contains NULL and the existingpu."isAnonymous" = false(and{ isAnonymous: false }) filters are correct.
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md --> <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Fixes handling of `isAnonymous` field in metrics queries by using `COALESCE` to default null values to 'false' in `route.tsx`. > > - **Behavior**: > - Fixes handling of `isAnonymous` field in metrics queries by using `COALESCE` to default null values to 'false'. > - Affects `loadUsersByCountry`, `loadDailyActiveUsers`, and `loadRecentlyActiveUsers` functions in `route.tsx`. > - **Functions**: > - `loadUsersByCountry`: Updates query condition to use `COALESCE` for `isAnonymous`. > - `loadDailyActiveUsers`: Updates query condition to use `COALESCE` for `isAnonymous`. > - `loadRecentlyActiveUsers`: Updates query condition to use `COALESCE` for `isAnonymous`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=stack-auth%2Fstack-auth&utm_source=github&utm_medium=referral)<sup> for b3ad219. You can [customize](https://app.ellipsis.dev/stack-auth/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN --> <!-- RECURSEML_SUMMARY:START --> ## Review by RecurseML _🔍 Review performed on [9318e2b..b3ad219](stack-auth/stack-auth@9318e2b...b3ad2194304928c64612045e00dea9f3e518c3f0)_ ✨ No bugs found, your code is sparkling clean <details> <summary>✅ Files analyzed, no issues (1)</summary> • `apps/backend/src/app/api/latest/internal/metrics/route.tsx` </details> [](https://discord.gg/n3SsVDAW6U) <!-- RECURSEML_SUMMARY:END --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected handling of anonymous users in analytics: records missing an isAnonymous flag are now treated as non-anonymous when anonymous users are excluded. * Impacts country breakdown, daily active users, and recently active metrics; overall total users unchanged. * Expect slightly higher non-anonymous counts and more consistent reporting across these metrics. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
2 participants
Important
Fixes handling of
isAnonymousfield in metrics queries by usingCOALESCEto default null values to 'false' inroute.tsx.isAnonymousfield in metrics queries by usingCOALESCEto default null values to 'false'.loadUsersByCountry,loadDailyActiveUsers, andloadRecentlyActiveUsersfunctions inroute.tsx.loadUsersByCountry: Updates query condition to useCOALESCEforisAnonymous.loadDailyActiveUsers: Updates query condition to useCOALESCEforisAnonymous.loadRecentlyActiveUsers: Updates query condition to useCOALESCEforisAnonymous.This description was created by
for b3ad219. You can customize this summary. It will automatically update as commits are pushed.
Review by RecurseML
🔍 Review performed on 9318e2b..b3ad219
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (1)
•
apps/backend/src/app/api/latest/internal/metrics/route.tsxSummary by CodeRabbit