Skip to content

billing: stop printing the allowance and the wallet as one number - #208

Open
jwfing wants to merge 1 commit into
mainfrom
feat/credits-pay-subscription
Open

billing: stop printing the allowance and the wallet as one number#208
jwfing wants to merge 1 commit into
mainfrom
feat/credits-pay-subscription

Conversation

@jwfing

@jwfing jwfing commented Sep 11, 2026

Copy link
Copy Markdown
Member

insta billing read totals.creditsUsd, which the platform computes as the remaining plan allowance plus the wallet balance. That sum is not a quantity anyone can spend: the allowance reduces billable usage, the wallet reduces the amount payable, and the two never pay for the same thing. A free org with $5 of allowance left and $3 of gift credit printed credits: $8.00 — a figure it could not spend on anything.

Reads creditBalanceUsd instead (already on the wire, no backend change needed) so credits: means the wallet alone, and renames included: to included usage: to match the name the console and pricing page now use. Labels re-padded to fit.

This is one half of the naming split the PRD asks for — "never mix included usage and credits". The console half is in insta-frontend, and neither depends on the other.

Testing

npm run typecheck clean, 1,206 tests passing. The regression is pinned directly: a free org with $5 allowance remaining and a $3 wallet must not render $8.00 anywhere.

Note

The legacy creditsUsd field stays on the JSON wire for now — this only stops the CLI reading it. insta-skills documents it as not-to-be-read in a companion PR.

🤖 Generated with Claude Code


Summary by cubic

insta billing printed credits: as the remaining plan allowance plus the wallet balance, which was a sum nobody could spend. This change reads creditBalanceUsd instead so credits: means the wallet alone, and renames included: to included usage: to match the console. This is the CLI half of the PRD's "never mix included usage and credits" split.

Notes

  • A free org with $5 allowance remaining and a $3 wallet now renders credits: $3.00, never $8.00; the regression is pinned in a test.
  • The legacy creditsUsd field stays on the JSON wire; only the CLI stops reading it.
  • The console half lives in insta-frontend, and neither change depends on the other.

Written for commit 45f81c5. Summary will update on new commits.

Review in cubic

`insta billing` read `totals.creditsUsd`, which the platform computes as the
remaining plan allowance plus the wallet balance. That sum is not a quantity:
the allowance feeds the usage calculation and the wallet feeds the amount
payable, and the two are never spent against the same thing. A free org with
$5 of allowance left and $3 of gift credit printed "credits: $8.00" — a figure
it could not spend on anything.

Read `creditBalanceUsd` instead (already on the wire) so `credits:` means the
wallet alone, and rename `included:` to `included usage:` to match the name the
console and pricing page use. Labels re-padded to fit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary
The change correctly separates wallet credits from included usage by rendering creditBalanceUsd, while preserving the raw API response for --json. No correctness, security, or performance blockers were found. The regression test is useful, though its fixture could model the legacy aggregate more directly.

Requirements context
Intent was derived primarily from the PR title and description: credits: must represent the wallet alone, included usage: must remain separate, and the legacy aggregate may remain on the JSON wire. No matching local product requirements were found; the repository docs only require command/flag changes to update the external CLI reference, which does not apply to this output-label change. The linked Notion PRD was not accessible, so its quoted “never mix included usage and credits” requirement could not be independently verified.

Findings

Critical
(none)

Suggestion

  • test/billing.test.ts:37: Model the legacy aggregate explicitly in the regression fixture - The regression fixture contains creditBalanceUsd: 3 and includedUsd: 5, but no creditsUsd: 8. Adding the legacy wire field to a structurally compatible fixture would directly prove that rendering selects the wallet field even when the misleading aggregate is present; currently the $8.00 negative assertion only guards against locally adding the two fixture values.

Information

  • src/commands/billing.ts:13: Wallet-only rendering implements the requested split - The text formatter now reads creditBalanceUsd, labels the allowance as included usage, and never combines the values. The --json path still prints the unmodified runtime response, so retaining the legacy wire field remains compatible with the stated rollout plan.
  • Testing: Changed behavior has focused unit coverage - Tests check the revised labels, alignment, wallet-only $3.00 output, absence of $8.00, and unchanged subscription/breakdown behavior. The implementation follows the existing pure formatter and Vitest conventions.
  • Security: No security-sensitive behavior changed - The patch adds no dependencies, input handling, authorization changes, requests, logging of secrets, or new data exposure; it only selects and formats an existing billing response field.
  • Performance: No performance regression identified - The change performs the same constant amount of synchronous string formatting and adds no requests, loops, blocking I/O, or material allocations.

Tests
git diff --check main...HEAD passed. Attempted npm run typecheck && npm test, but typecheck could not start because tsc is not installed; a separate focused npm test -- test/billing.test.ts attempt likewise could not start because vitest is unavailable. Dependencies were not installed because the review environment is read-only.

Verdict
approved: There are no Critical findings, so the strict verdict rule requires approval despite the non-blocking test suggestion.

@Fermionic-Lyu Fermionic-Lyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Approved.

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