refactor(billing): retire the parity-phase getUsage source toggle#6764
refactor(billing): retire the parity-phase getUsage source toggle#6764pfreixes wants to merge 3 commits into
Conversation
Post-cutover, the ClickHouse path is authoritative for /getUsage on
every account. The parity-phase machinery that let a dev-only flag
route individual requests to Orb instead is dead code:
- `billingUsageSource` field on GET /api/v1/meta (was hardcoded to
'clickhouse' anyway)
- `source=clickhouse|orb` query param on GET /api/v1/plans/billing-usage
- `source` field on `GetBillingUsageOpts`
- `FLAG_ALLOW_OVERRIDE_GETUSAGE_SERVICE` env var
- Orb-override branch in `UsageTracker.getBillingUsage` (with its Orb
cache-key sanitisation and cumulative-usage post-processing — all
unreachable now)
- `params.append('source', ...)` in the webapp's `useApiGetBillingUsage`
and `useApiGetBillingUsageDetail`
Webapp `useBreakdownEnabled` now returns `true` unconditionally. The
downstream `if (breakdownEnabled)` conditionals are still there but
always take the true branch; that inlining is left as follow-up cleanup
so this diff stays focused on the removal.
Companion cleanup needed in nango-environments: remove
`FLAG_ALLOW_OVERRIDE_GETUSAGE_SERVICE` from `apps/development/nango-values.yaml`.
Nothing to do for staging/prod (never set there).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 5/5
- In
packages/webapp/src/pages/Team/Billing/useBreakdownEnabled.ts, the unconditional return makes the Dev Tools “Usage breakdown” toggle ineffective, so users can changeusageBreakdownwithout any visible dashboard behavior change; this is mainly a confusing UX mismatch rather than a functional break — remove/disable the toggle (or restore conditional behavior) before merging to avoid misleading controls.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/webapp/src/pages/Team/Billing/useBreakdownEnabled.ts">
<violation number="1" location="packages/webapp/src/pages/Team/Billing/useBreakdownEnabled.ts:7">
P3: The Dev Tools “Usage breakdown” switch is now a no-op: this unconditional return means toggling its persisted `usageBreakdown` value never changes the dashboard. Removing or disabling that control alongside the now-unused feature-flag state would avoid presenting a setting that appears to work but has no effect.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const { data: meta } = useMeta(); | ||
| const devFlag = useFeatureFlagsStore((s) => s.usageBreakdown); | ||
| return meta?.data.billingUsageSource === 'clickhouse' || devFlag; | ||
| return true; |
There was a problem hiding this comment.
P3: The Dev Tools “Usage breakdown” switch is now a no-op: this unconditional return means toggling its persisted usageBreakdown value never changes the dashboard. Removing or disabling that control alongside the now-unused feature-flag state would avoid presenting a setting that appears to work but has no effect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/webapp/src/pages/Team/Billing/useBreakdownEnabled.ts, line 7:
<comment>The Dev Tools “Usage breakdown” switch is now a no-op: this unconditional return means toggling its persisted `usageBreakdown` value never changes the dashboard. Removing or disabling that control alongside the now-unused feature-flag state would avoid presenting a setting that appears to work but has no effect.</comment>
<file context>
@@ -1,17 +1,8 @@
- const { data: meta } = useMeta();
- const devFlag = useFeatureFlagsStore((s) => s.usageBreakdown);
- return meta?.data.billingUsageSource === 'clickhouse' || devFlag;
+ return true;
}
</file context>
Addresses cubic's P3 on nango#6764: with useBreakdownEnabled returning true unconditionally, the Dev Tools "Usage breakdown" switch still wrote to localStorage but nothing read it — a persisted setting that looked like it worked but had no effect. Drops: - the switch itself + its "Feature Flags" section header in DevToolPanel (last item there, section is now empty) - the entire useFeatureFlagsStore (usageBreakdown was its only field) - the LocalStorageKeys.FeatureFlags entry + its KEY_CATEGORY row If any user still has the `nango_feature_flags` key persisted in localStorage from before, it just becomes orphaned data — no runtime effect, cleared naturally on next full logout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
macko911
left a comment
There was a problem hiding this comment.
nitpick (non-blocking): UsageTracker.billingClient is now write-only — assigned in the constructor but no longer read anywhere, since the removed orbOverride branch was its only reader (the UsageBillingClient import now survives only for its type). Worth dropping, or folding into the same follow-up as the other no-op cleanups.
Otherwise LGTM — clean removal, verified no dangling references and that stray ?source= params are silently dropped (non-strict schema) rather than 400ing.
Second follow-up to cubic's review on nango#6764. With the orbOverride branch gone from UsageTracker.getBillingUsage, nothing calls UsageBillingClient — its only method (`getUsage`) is the Orb-side read path we retired at cutover. Metering's HTTP-event push path is a separate surface (`@nangohq/billing`'s OrbClient.ingest via `billing.add`) and stays untouched. Removes: - packages/usage/lib/billing.ts (the class) - packages/usage/lib/billing.unit.test.ts (its tests) - The `billingClient` field + constructor init on UsageTracker - Stale reference in clickhouse.ts:547's docstring Drops per-service-process allocation of a Redis rate limiter that no downstream consumer touches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| * Whether the usage breakdown view is shown. Always true post-cutover — kept | ||
| * as a hook so call sites don't have to change while the always-true | ||
| * conditionals downstream get inlined in follow-up cleanup. | ||
| */ | ||
| export function useBreakdownEnabled(): boolean { | ||
| const { data: meta } = useMeta(); | ||
| return meta?.data.billingUsageSource === 'clickhouse'; | ||
| return true; |
There was a problem hiding this comment.
I think this could be cleaned up?
Summary
/getUsageon every account. Removes the parity-phase machinery that let a dev-only flag route individual requests to Orb instead — dead code now./api/v1/metaresponse, one query param + its handling from/api/v1/plans/billing-usage, one env var, the Orb-fallback branch inUsageTracker.getBillingUsage(plus the cumulative-usage helper only that branch used), and the corresponding webapp plumbing.Removed
billingUsageSourceonGET /api/v1/meta(was hardcoded to'clickhouse'anyway).source=clickhouse|orbquery param onGET /api/v1/plans/billing-usage.source?field onGetBillingUsageOpts.FLAG_ALLOW_OVERRIDE_GETUSAGE_SERVICEenv var (packages/utils/lib/environment/parse.ts).orbOverridebranch inUsageTracker.getBillingUsage, including the Orb cache-key sanitisation and cumulative-usage post-processing that only that branch reached.params.append('source', ...)in the webapp'suseApiGetBillingUsageanduseApiGetBillingUsageDetail.Left as follow-up
useBreakdownEnablednow returnstrueunconditionally. Downstreamif (breakdownEnabled)conditionals still evaluate correctly (always take the true branch) but could be inlined. Kept out of scope to keep this diff focused on the removal.The webapp
usageBreakdowndev toggle inDevToolPanel+ its slot in the feature-flags store are no-ops now — same story, follow-up cleanup.Companion nango-environments change
Remove
FLAG_ALLOW_OVERRIDE_GETUSAGE_SERVICEfromapps/development/nango-values.yaml. Nothing to do for staging/prod (never set there).Test plan
npm run ts-buildclean.npm run lintclean.npm run format:checkclean.packages/server/lib/controllers/v1/meta/getMeta.integration.test.tsstill passes.GET /api/v1/plans/billing-usage?source=orbno longer routes to Orb (should silently ignore or 400 depending on strict-schema config).🤖 Generated with Claude Code