fix(accounting): remove heuristic chat token usage - #975
Conversation
Use provider-reported counts or exact native raw-text tokenizers for declared models. Keep unreconstructible chat usage nullable, route conservatively, and fail enabled budgets closed when measurement is unavailable. Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
b2a2607
into
fix/provider-embedding-current-main
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| candidate_prices_available = ( | ||
| self.budget_max_cost_usd is None | ||
| or all(agent.model in self.price_per_million for agent in self.agents) | ||
| ) | ||
| measurement_available = ( | ||
| not self._budget_unavailable_run_ids and candidate_prices_available |
There was a problem hiding this comment.
🔴 Unrelated agents disable chat budgets
With a cost budget, budget_status requires prices for every enabled agent, including agents that cannot serve chat. One unpriced embedding-only agent blocks all chat requests despite fully priced eligible agents.
Prompt for agents
Restrict cost-budget price-availability checks to models that can actually participate in the pending chat execution. The current global check in TaskOrchestrator.budget_status includes every enabled capability agent, so unrelated embedding, image, audio, or other non-chat agents can disable chat. Account for exact requested-model restrictions and valid failover/model-group candidates where request context is available, while preserving fail-closed behavior for any model that can actually be selected.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if measurement_status == "unavailable": | ||
| provider_response["usage"] = None | ||
| provider_response["usage_measurement_status"] = measurement_status |
There was a problem hiding this comment.
🟡 Valid winner usage is discarded
When an endpoint race has one loser without usage, complete replaces valid winner usage with null. Clients lose authoritative counts even though only aggregate duplicate cost is unknown.
Prompt for agents
Separate client-facing winner usage from aggregate race cost status in CostRoutingCoordinator.complete's provider_request branch. Preserve and validate the winning provider_response usage independently, as the ordinary completion branch does with client_usage_records. Keep aggregate cost unavailable when any completed race call lacks usage, and retain unavailable ledger evidence for that loser.
Was this helpful? React with 👍 or 👎 to provide feedback.
| self.token_counter = token_counter or ( | ||
| build_token_counter(postgres_dsn) if postgres_dsn else HeuristicTokenCounter() | ||
| ) | ||
| self.token_counter = token_counter or build_token_counter(postgres_dsn) |
There was a problem hiding this comment.
🟡 Configured counters miss spend accounting
Passing an exact counter or PostgreSQL DSN to CostRoutingCoordinator does not configure the orchestrator’s new counter. Spend analytics remain unavailable, and enabled budgets can block after the first request.
Prompt for agents
Unify the authoritative raw-output counter used by CostRoutingCoordinator and TaskOrchestrator. A token_counter or postgres_dsn supplied to the coordinator must also drive TaskOrchestrator._trace_budget_spend, _run_budget_output_by_model, and spend_analytics, without overriding an explicitly configured orchestrator counter unexpectedly. Add coverage through build_server for injected and PostgreSQL-backed counters.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const price = row.price_per_million_usd == null ? "—" : escapeHtml(row.price_per_million_usd); | ||
| const cost = row.estimated_cost_usd == null ? `<span class="chip" title="${escapeHtml(t("spend_no_price_action"))}">${escapeHtml(t("spend_no_price"))}</span>` : ("$" + escapeHtml(row.estimated_cost_usd)); | ||
| return `<tr><td>${escapeHtml(row.model)}</td><td>${escapeHtml(row.estimated_output_tokens)}</td><td>${escapeHtml(row.step_count)}</td><td>${price}</td><td>${cost}</td></tr>`; | ||
| const cost = row.cost_usd == null ? `<span class="chip" title="${escapeHtml(t("spend_no_price_action"))}">${escapeHtml(t("spend_no_price"))}</span>` : ("$" + escapeHtml(row.cost_usd)); |
There was a problem hiding this comment.
🟡 Unavailable usage looks unpriced
When token counts are unavailable for a priced model, renderSpend labels its null cost as “No price set.” Operators receive the wrong remediation and can overwrite valid pricing.
Prompt for agents
Render null per-model cost according to both pricing and usage availability. Use the existing price_per_million_usd and usage_source fields to distinguish an unpriced model from unavailable token evidence, and add localized text for the unavailable-cost state in both supported locales.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if ( | ||
| type(prompt_tokens) is int | ||
| and prompt_tokens >= 0 | ||
| and type(completion_tokens) is int | ||
| and completion_tokens >= 0 | ||
| ): | ||
| usage = {**reported_usage, "usage_source": "reported"} | ||
| measurement_status = "measured" |
There was a problem hiding this comment.
🟡 Malformed totals become measured usage
With valid component counts, _chat_response_sse_chunks copies malformed total_tokens unchanged and marks usage measured. Streaming clients can receive negative, nonnumeric, or inconsistent totals.
| if ( | |
| type(prompt_tokens) is int | |
| and prompt_tokens >= 0 | |
| and type(completion_tokens) is int | |
| and completion_tokens >= 0 | |
| ): | |
| usage = {**reported_usage, "usage_source": "reported"} | |
| measurement_status = "measured" | |
| if ( | |
| type(prompt_tokens) is int | |
| and prompt_tokens >= 0 | |
| and type(completion_tokens) is int | |
| and completion_tokens >= 0 | |
| ): | |
| usage = { | |
| "prompt_tokens": prompt_tokens, | |
| "completion_tokens": completion_tokens, | |
| "total_tokens": prompt_tokens + completion_tokens, | |
| "usage_source": "reported", | |
| } | |
| measurement_status = "measured" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| ## References | ||
|
|
||
| OpenAI. (n.d.). *Tiktoken model mappings*. | ||
| https://github.com/openai/tiktoken/blob/main/tiktoken/model.py | ||
|
|
||
| OpenAI. (n.d.). *Chat Completions API reference*. | ||
| https://platform.openai.com/docs/api-reference/chat/create | ||
|
|
||
| PyO3 Project. (n.d.). *Python modules*. | ||
| https://pyo3.rs/main/module | ||
|
|
||
| ContextualWisdomLab. (2026). *Cost-aware sync-versus-batch routing* | ||
| (ADR 0003). | ||
| https://github.com/ContextualWisdomLab/contextual-orchestrator/blob/main/docs/adr/0003-cost-aware-sync-batch-routing.md |
There was a problem hiding this comment.
🔍 Research artifacts need follow-up
This substantive accounting change adds vendor references but no paper artifact or redistribution note. The repository’s research-grounding rule requires reviewer follow-up.
Was this helpful? React with 👍 or 👎 to provide feedback.
| measurement_status = ( | ||
| "unavailable" | ||
| if not output_available or not prompt_available | ||
| else "measured" | ||
| if all(row["usage_source"] == "reported" for row in rows) | ||
| else "exact_tokenizer" |
There was a problem hiding this comment.
📝 Info: Partial output evidence stays distinguishable
Exact output counts remain visible when prompt usage is unavailable. Aggregate status and cost still fail closed, so the partial count is not presented as complete usage.
Was this helpful? React with 👍 or 👎 to provide feedback.
| prompt_tokens=item.prompt_tokens, | ||
| completion_tokens=item.completion_tokens, |
There was a problem hiding this comment.
📝 Info: Zero counts remain measured
Batch retrieval now preserves literal zero counts instead of converting them to missing values. Empty measured results therefore remain distinct from unavailable usage.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Stack
This PR is stacked on #970 and must merge only after that base lands. Prepared against exact #970 head
028bebc394b3ebf7c66b626c0efbe46190a0afe5.Validation
uv run --locked --extra api --extra db --extra queue --group dev python -m pytest -q— 2907 passed, 2 skippedcargo test --manifest-path rust/token_counter/Cargo.toml— 10 passedcargo fmt --manifest-path rust/token_counter/Cargo.toml -- --checkcl100k,o200k, andpack_cl100k— 1 passedgit diff --checkNo provider credentials or real request data are included.