feat(admin): add churn chart unit toggle#2101
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds plan-level conversion-rate columns and previous-period MRR to global_stats (migration, types, trigger, backfill), exposes them via getAdminGlobalStatsTrend (correlated subqueries), and updates the admin revenue page with a churn $ / % toggle plus a Plan Conversion Rate chart. ChangesChurn & Plan Conversion Rates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@supabase/functions/_backend/utils/pg.ts`:
- Line 1197: The current LAG(...) OVER (ORDER BY date_id ASC) in the calculation
of previous_mrr pulls the previous available row (not necessarily the previous
calendar day); to fix, join or generate a contiguous calendar of dates (e.g.,
via generate_series or an existing calendar table) and left-join global_stats
onto that calendar, compute mrr (COALESCE(mrr,0)) on the calendar rows, then use
LAG(COALESCE(mrr,0)) OVER (ORDER BY calendar_date) to produce previous_day MRR;
update the expression that defines previous_mrr (and any references to date_id)
to use the calendar_date-backed ordering so missing days become zeros rather
than skipping and distorting churn calculations.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2225621-6728-4157-8bfc-b86295917ce0
📒 Files selected for processing (2)
src/pages/admin/dashboard/revenue.vuesupabase/functions/_backend/utils/pg.ts
Merging this PR will not alter performance
Comparing 
Footnotes
|
SpeedyArt
left a comment
SpeedyArt
left a comment
There was a problem hiding this comment.
I think the new plan-rate series are using the wrong denominator.
The backend now returns only previous_mrr, and the frontend computes each plan line as churn_revenue_<plan> / previous_mrr. But the numerator is plan-specific lost MRR while previous_mrr is total prior MRR, so the labeled Solo Churn (%), Maker Churn (%), etc. are not actually plan churn rates. For example, if Solo lost $10 against $100 of prior Solo MRR while total prior MRR was $1,000, the chart would show 1% instead of the plan’s 10% churn rate.
global_stats already has separate plan revenue columns (revenue_solo, revenue_maker, revenue_team, revenue_enterprise; those are ARR, so the per-plan MRR denominator would be prior-day revenue / 12). Could we return previous per-plan MRR denominators and use those for the plan series, or else relabel these lines as each plan’s lost MRR share of total prior MRR if that is the intended metric? A small regression with two plans that have very different prior MRR would catch the distinction.
|
Thanks, the latest commit looks like it addresses the denominator issue I raised: the backend now returns per-plan |
riderx
force-pushed
the
codex/admin-churn-chart-unit-toggle
branch
from
104b63f to
79372a0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/admin-stripe-backfill-scripts.unit.test.ts (1)
14-89: ⚡ Quick winAdd one plan-only delta case to lock in new change-detection behavior.
Line 161 in the script now flags rows when plan rates change even if
org_conversion_ratedoes not. This test currently covers “both changed” and “none changed,” but not “only plan changed.”Suggested additional assertion case
+ it.concurrent('marks row as changed when only plan conversion rate differs', () => { + const rows = buildOrgConversionRateBackfillRows([ + { + date_id: '2026-04-10', + paying: 40, + org_conversion_rate: 20, + plan_enterprise: 0, + plan_enterprise_conversion_rate: 0, + plan_maker: 10, + plan_maker_conversion_rate: 4.5, // intentionally stale + plan_solo: 30, + plan_solo_conversion_rate: 15, + plan_team: 0, + plan_team_conversion_rate: 0, + }, + ] as any, Array.from({ length: 200 }, () => ({ created_at: '2026-04-01T12:00:00.000Z' }))) + + expect(rows[0]?.current_rate).toBe(20) + expect(rows[0]?.next_rate).toBe(20) + expect(rows[0]?.changed).toBe(true) + })🤖 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 `@tests/admin-stripe-backfill-scripts.unit.test.ts` around lines 14 - 89, The test for buildOrgConversionRateBackfillRows needs an extra case that covers "only plan changed" (plan rate deltas without org_conversion_rate change) so the new change-detection logic is exercised; add another input row in the it.concurrent('marks only changed org conversion rows') scenario (or a new it case) where date_id differs (e.g., '2026-04-03') with org_conversion_rate equal to the prior value but one or more plan_*_conversion_rate values changed, then assert the resulting row has changed: true while current_rate and next_rate (org_conversion_rate) remain equal — locate buildOrgConversionRateBackfillRows and the test's expected array to insert the new input and expected output.supabase/functions/_backend/utils/pg.ts (1)
1209-1233: ⚡ Quick winUse one previous-day join instead of five correlated subqueries.
Line 1209-1233 repeats the same previous-day lookup for each metric. A single
LEFT JOINtoprev(once per row) will be easier to maintain and reduce redundant lookups.🤖 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 `@supabase/functions/_backend/utils/pg.ts` around lines 1209 - 1233, The query repeats five correlated subqueries on global_stats to fetch previous-day metrics; replace them with a single LEFT JOIN to a prev alias (join condition: prev.date_id = (gs.date_id::date - 1)::text) and then reference prev.mrr, prev.revenue_solo, prev.revenue_maker, prev.revenue_team, prev.revenue_enterprise in the SELECT, applying the same COALESCE(...,0)::float and /12 casts where needed (e.g., previous_mrr := COALESCE(prev.mrr,0)::float; previous_mrr_solo := (COALESCE(prev.revenue_solo,0)::float/12) etc.), and remove the five correlated subqueries that reference prev in the SELECT.
🤖 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.
Nitpick comments:
In `@supabase/functions/_backend/utils/pg.ts`:
- Around line 1209-1233: The query repeats five correlated subqueries on
global_stats to fetch previous-day metrics; replace them with a single LEFT JOIN
to a prev alias (join condition: prev.date_id = (gs.date_id::date - 1)::text)
and then reference prev.mrr, prev.revenue_solo, prev.revenue_maker,
prev.revenue_team, prev.revenue_enterprise in the SELECT, applying the same
COALESCE(...,0)::float and /12 casts where needed (e.g., previous_mrr :=
COALESCE(prev.mrr,0)::float; previous_mrr_solo :=
(COALESCE(prev.revenue_solo,0)::float/12) etc.), and remove the five correlated
subqueries that reference prev in the SELECT.
In `@tests/admin-stripe-backfill-scripts.unit.test.ts`:
- Around line 14-89: The test for buildOrgConversionRateBackfillRows needs an
extra case that covers "only plan changed" (plan rate deltas without
org_conversion_rate change) so the new change-detection logic is exercised; add
another input row in the it.concurrent('marks only changed org conversion rows')
scenario (or a new it case) where date_id differs (e.g., '2026-04-03') with
org_conversion_rate equal to the prior value but one or more
plan_*_conversion_rate values changed, then assert the resulting row has
changed: true while current_rate and next_rate (org_conversion_rate) remain
equal — locate buildOrgConversionRateBackfillRows and the test's expected array
to insert the new input and expected output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0fd55489-8116-4897-8e52-35ce2aed6a98
📒 Files selected for processing (7)
scripts/backfill_org_conversion_rate_trend.tssrc/pages/admin/dashboard/revenue.vuesupabase/functions/_backend/triggers/logsnag_insights.tssupabase/functions/_backend/utils/pg.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260510214806_add_plan_conversion_rates_to_global_stats.sqltests/admin-stripe-backfill-scripts.unit.test.ts
✅ Files skipped from review due to trivial changes (2)
- supabase/migrations/20260510214806_add_plan_conversion_rates_to_global_stats.sql
- supabase/functions/_backend/utils/supabase.types.ts
|
2 participants
Summary (AI generated)
$/%toggle to the admin revenue churn chart.Motivation (AI generated)
Business Impact (AI generated)
Test Plan (AI generated)
bunx eslint --no-ignore src/pages/admin/dashboard/revenue.vue supabase/functions/_backend/utils/pg.tsbun lintbun typecheckgit diff --check origin/main...HEADbun run buildSummary by CodeRabbit