Harden billing data integrity and audit capacity - #13
Conversation
An HTTP 200 with a failing business code or a missing result structure was treated as a valid empty/zero response: fetch_request_usage never checked the business code (and a missing total silently stopped pagination after the first page), and fetch_credits could not tell a missing Accounts structure from a legitimately empty balance. Both now raise on a non-zero business code or missing data.data/total/Accounts structure, while a present-but-empty result remains a valid zero. Verified by new failure-semantics tests; existing paging and HTTP tests unchanged and passing.
fetch_credits read only the first page of 100 packages and fetch_request_usage stopped silently at the 6000-row cap, so balances and usage could be understated while looking complete. Both now page until a short page (with a hard cap) and return partial=True when the cap is hit; the flag is stored in the credit ledger, surfaced by aggregate_credits, and exposed on the billing totals so an incomplete snapshot is visible instead of pretending to be exact. Verified by new pagination and page-cap tests (full suite passed).
Usage sync rebuilt the aggregate from scratch each round and published it whenever at least one account succeeded, so an account whose fetch failed saw its previously synced usage vanish while fetched_at claimed a fresh sync. Each account now keeps its own snapshot in usage_daily_accounts; a successful sync replaces only that account's data, and a failed one keeps its history while the published view is marked partial with stale_accounts listed. Snapshots of removed or disabled credentials are excluded from the aggregate. Verified by a new sync regression test covering failure, recovery, and credential removal (test_credits.py passes).
The usage endpoint applied a single weighted-average price to every day and model, so with mixed domestic/international usage the per-day and per-model amounts were wrong even when the grand total happened to be right. daily_costs is now computed from each site group's own by_day at that site's price and then merged; the balance identity with total_usage still holds because both derive from the same per-site totals. The response also surfaces partial and stale_accounts when the usage data is incomplete. Verified by a two-site different-days arithmetic test and the existing billing identity tests.
…igurable Up to three extra generations after malformed tool calls consumed real credits, but only the final usage was recorded. Each discarded generation now lands in the request's attempts with its sequence number and total_tokens, and the budget moves from a hardcoded constant to --tool-call-max-retry / CODEBUDDY2API_TOOL_CALL_MAX_RETRY (default 3, 0 disables retries). Verified by a new endpoint test covering retry-with-usage recording and the zero budget (affected test files pass).
The ingest dedup table had no time dimension and intentionally outlived all detail eviction, so it grew monotonically with unique requests and the detail byte budget could not bound the database file. Schema v2 adds created_at to ingest (migrated from v1 with timestamps backfilled from the matching detail row), expires dedup rows at the same retention cutoff with bounded batches, and reports them in pending_cleanup. Within the retention window dedup is unchanged; beyond it an ancient id may record again instead of being kept forever. Aggregates are still never deleted by budget or retention. Verified by a v1-migration regression test and the updated retention contract tests (test_audit_store.py passes).
Sync the advanced guides: partial/stale_accounts semantics on billing endpoints, per-site daily pricing, the per-account snapshot behavior on sync failures, and the new --tool-call-max-retry option with its credit cost note.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 958fa7a39d
ℹ️ 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".
| if page > CREDITS_MAX_PAGES: | ||
| partial = True | ||
| break | ||
| rows = _fetch_accounts_page(client, url, headers, page, retry_empty=(page == 1)) |
There was a problem hiding this comment.
When an account has more than one page and the upstream’s known transient empty Accounts response occurs on page 2 or later, retry_empty is false, so the loop accepts that empty page as the end and returns an understated balance with partial: false. Retry empty results on subsequent pages as well, or use authoritative pagination metadata to distinguish a real terminal page.
Useful? React with 👍 / 👎.
| if cred_id in stale: | ||
| partial = True | ||
| stale_out.append(Path(cred_id).name) |
There was a problem hiding this comment.
On an initial sync where one enabled account succeeds and another fails, the failed account is in stale but absent from accounts, so this condition is never evaluated for it. The successful account is then published with partial: false and no stale_accounts, making an incomplete aggregate appear exact; derive missing failures from stale ∩ enabled, not only from snapshots already present.
Useful? React with 👍 / 👎.
| if t.get("partial"): | ||
| out["partial"] = True |
There was a problem hiding this comment.
Only the usage response forwards this marker. If credit pagination reaches its cap, _billing_totals() has partial=True, but /v1/dashboard/billing/subscription still returns truncated remaining-credit and hard-limit values without any indication that they are incomplete, despite the documented balance-integrity contract. Forward the partial state, and relevant stale-account information, from the subscription response too.
Useful? React with 👍 / 👎.
| observe_attempt("tool_args_retry", attempt=tool_attempt, max_attempts=budget, | ||
| total_tokens=discarded.get("total_tokens")) |
There was a problem hiding this comment.
In production, observe_attempt immediately passes this metadata through app.audit_store.safe_attempt, whose numeric allowlist excludes both total_tokens and max_attempts; the persisted attempt therefore contains only stage and attempt. The new test mocks observe_attempt, bypassing this sanitizer, so it does not catch that discarded-generation usage still disappears from the audit record.
Useful? React with 👍 / 👎.
| budget = CONFIG.get("tool_call_max_retry", _TOOL_CALL_MAX_RETRY) | ||
| if detector.detected or not body.get("tools") or tool_attempt >= budget: | ||
| raise UpstreamResponseError(502, b"Invalid upstream tool_calls after retries") |
There was a problem hiding this comment.
When the budget is zero or every permitted retry also returns malformed tool calls, this branch raises before observe_attempt runs. The last consumed—and potentially billed—generation is therefore absent from the request’s attempts even after the sanitizer is fixed, making failed/exhausted requests systematically omit one generation’s usage.
Useful? React with 👍 / 👎.
1 participant
What changes
Seven commits making balances and usage honest about incompleteness:
Billing API responses
fetch_request_usagenow validates the business code and requires thedata.data/totalstructure — an HTTP 200 with a failing code or missing structure is an error, not a silent zero (a missingtotalalso used to stop pagination after page 1)fetch_creditsdistinguishes a legitimately empty balance (Accounts key present) from a missing structure (error), so a malformed success can no longer overwrite history with zeroPagination
fetch_creditspages until a short page (was: first 100 packages only);fetch_request_usagekeeps its 30-page cap but both now returnpartial: truewhen capped, the flag is stored in the ledger, surfaced byaggregate_credits, and exposed on billing responsesSync resilience
usage_daily_accounts): a failed account keeps its last good data instead of vanishing from the aggregate, and the published view carriespartial+stale_accounts. Removed/disabled credentials drop out of the aggregatedaily_costsare priced per site per day at that site's price instead of one blended average; the balance identity withtotal_usagestill holdsTool-call retries
total_tokens; the retry budget is now--tool-call-max-retry/CODEBUDDY2API_TOOL_CALL_MAX_RETRY(default 3, 0 disables)Audit capacity
ingestdedup rows gaincreated_at(backfilled on migration) and expire at the same retention cutoff as their details, so the dedup table can no longer grow without bound; aggregates are still never deleted by budget/retentionDocs: new "Billing data integrity" section and updated retry-row in
docs/advanced.md/docs/advanced.zh-CN.md.Verification
Full suite on the branch: 649 passed, 2346 subtests, including new regression tests for failure semantics, pagination caps, per-account snapshot retention, per-site pricing arithmetic, retry-attempt recording, and the v1→v2 audit migration.