Repository navigation
fix(api): scope DevPass aggregates by org.kind - #2762
Conversation
The DevPass KPI, timeseries, and usage aggregate queries in admin.ts inferred DevPass membership from `devPlan != 'none'` or transaction history (dev_plan_* / legacy subscription_* rows). That heuristic missed churned orgs (devPlan reset to 'none') and risked misattributing org Pro `subscription_*` revenue as DevPass. Scope every aggregate by the authoritative `organization.kind = 'devpass'` column, which is stable across the subscription lifecycle, so churned subscribers stay counted and non-DevPass orgs are excluded. This also drops the correlated EXISTS subqueries in favor of a plain indexed filter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAll DevPass admin reporting queries in ChangesDevPass Query Scoping
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7664280f57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| )!; | ||
| // Filter: only DevPass orgs (kind = 'devpass'). Stable across the | ||
| // subscription lifecycle, so churned orgs (devPlan = 'none') stay included. | ||
| const devpassOrgFilter = eq(tables.organization.kind, "devpass"); |
There was a problem hiding this comment.
When a code-app signup or /dev-plans/personal-org call runs, getOrCreatePersonalOrg() creates a kind='devpass' org before any subscription while devPlan is still none; the previous predicate also required an active plan or a start transaction. With this kind-only filter, /devpass/usage (and the same kind-only cost filter above) can include rollups from never-subscribed personal orgs if they have project usage from migrated/personal credits, inflating DevPass usage/cost. Keep the kind check but retain a current-or-history DevPass predicate.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/admin.ts`:
- Around line 10040-10045: The WHERE clause in the query is filtering for only
"dev_plan_start" transaction types, which excludes legacy "subscription_start"
rows from the churned KPI count. Modify the condition that checks
`tables.transaction.type` to include both the new "dev_plan_start" type and the
legacy "subscription_start" type so that pre-rename transaction rows are counted
in the churned KPI calculation. Replace the single equality check with a
condition that accepts multiple transaction type values.
- Around line 10152-10157: The WHERE clause in the query for totalMrrCycle and
totalRealCostCycle calculations is missing a filter to exclude expired
subscriptions. Currently it only filters for organization.kind equal to
"devpass" and devPlan not equal to "none", but this includes expired
organizations unlike the activeRows and utilRow calculations. Add an additional
condition to the and() clause that filters out expired subscriptions using the
same expiration logic used in the activeRows and utilRow queries to ensure all
KPI metrics use consistent filtering criteria.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b2c49622-01fa-433b-b8aa-8570ec91dbb7
📒 Files selected for processing (1)
apps/api/src/routes/admin.ts
| .where( | ||
| and( | ||
| eq(tables.organization.kind, "devpass"), | ||
| eq(tables.transaction.type, "dev_plan_start"), | ||
| eq(tables.organization.devPlan, "none"), | ||
| ), |
There was a problem hiding this comment.
Include legacy starts in the churned KPI.
Now that this query is gated by organization.kind = "devpass", pre-rename subscription_start rows are safe to count here too. As written, legacy churned DevPass orgs can appear in the subscriber list via subscribedSinceSub, but are omitted from kpis.churned.
Proposed fix
eq(tables.organization.kind, "devpass"),
- eq(tables.transaction.type, "dev_plan_start"),
+ inArray(tables.transaction.type, [
+ "dev_plan_start",
+ "subscription_start",
+ ]),
eq(tables.organization.devPlan, "none"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .where( | |
| and( | |
| eq(tables.organization.kind, "devpass"), | |
| eq(tables.transaction.type, "dev_plan_start"), | |
| eq(tables.organization.devPlan, "none"), | |
| ), | |
| .where( | |
| and( | |
| eq(tables.organization.kind, "devpass"), | |
| inArray(tables.transaction.type, [ | |
| "dev_plan_start", | |
| "subscription_start", | |
| ]), | |
| eq(tables.organization.devPlan, "none"), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/routes/admin.ts` around lines 10040 - 10045, The WHERE clause in
the query is filtering for only "dev_plan_start" transaction types, which
excludes legacy "subscription_start" rows from the churned KPI count. Modify the
condition that checks `tables.transaction.type` to include both the new
"dev_plan_start" type and the legacy "subscription_start" type so that
pre-rename transaction rows are counted in the churned KPI calculation. Replace
the single equality check with a condition that accepts multiple transaction
type values.
universeRow (totalMrrCycle/totalRealCostCycle) is documented as the active subscriber universe but lacked the expiration filter that activeRows and utilRow apply, so expired-but-not-yet-churned orgs inflated the KPI cycle totals. Add the same `expiresAt IS NULL OR expiresAt > NOW()` guard for consistency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
The broader DevPass aggregate queries in
apps/api/src/routes/admin.ts— the KPI strip, the/devpass/timeseriesendpoint, and the/devpass/usageendpoint — identified DevPass orgs by inferring membership fromdevPlan != 'none'or from transaction history (dev_plan_*/ legacysubscription_*rows). This was flagged during thekindrename PR (#2743) as a pre-existing analytics-scoping concern.That heuristic had two problems:
devPlanreset to'none'(unless they still had matching transaction history).subscription_*transaction types are still written for non-personal org Pro subscriptions.This PR scopes every DevPass aggregate by the authoritative
organization.kind = 'devpass'column, which is set at org creation and stays stable across the subscription lifecycle (the cancel path only clearsdevPlan, neverkind). The subscriber list and detail handlers already usedkind; this brings the aggregates in line.Changes
All in
apps/api/src/routes/admin.ts:activeRows,churnedRow,startsRow,endsRow,refundsRow,utilRow,universeRow): addeq(kind, 'devpass')(joiningorganizationwhere the query didn't already).oldestrange anchor,revenuePerDay,refundsPerDay,costPerDay): replace theor(dev_plan types, and(legacy, kind))/EXISTS(...)patterns with a plainkind = 'devpass'filter.devpassOrgFilter): replace thedevPlan != 'none' OR EXISTS(...)heuristic withkind = 'devpass', dropping the correlated subquery in favor of a plain indexed filter.Testing
pnpm formatpnpm turbo run build --filter=api✅🤖 Generated with Claude Code
Summary by CodeRabbit