fix(scorecard): bind costs to provider routes#4351
Conversation
Accept optional provider provenance in offline scorecard records and fail legacy, custom, local, and unknown routes closed instead of guessing price from a model id. Resolve dollar costs from provider-scoped catalog offerings, retain exact-route DeepSeek USD/CNY pricing, expose per-currency completeness, and skip cost regressions against legacy incomplete baselines. Fixes Hmbown#4335. Builds on the original Hmbown#3388 scorecard by @findshan. Co-authored-by: Wenshan Deng <224246733+findshan@users.noreply.github.com> Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
|
@Hmbown Could you please request a Copilot code review for this PR? GitHub rejects the fork author's request with RequestReviewsByLogin permission. I'll address and resolve every actionable AI review thread before finalizing. |
![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: 8b76652e06
ℹ️ 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".
| .iter() | ||
| .map(|r| TurnInput { | ||
| turn_id: r.turn_id.clone(), | ||
| provider: r.provider.as_deref(), |
There was a problem hiding this comment.
When the scorecard input is generated from the current turn_end hook payloads, this field is always None: turn_end_payload still emits model and usage but no provider/effective_provider (crates/tui/src/hooks.rs:1288-1305). Because Scorecard::from_turns now treats missing provider as unpriced and skips USD cost regressions unless cost_complete, existing release-gate recordings silently lose all cost totals/comparisons; wire the effective provider into the hook/export payload or derive it before building TurnInput.
Useful? React with 👍 / 👎.
| pub turn_id: String, | ||
| #[serde(default, alias = "effective_provider")] | ||
| pub provider: Option<String>, | ||
| pub model: String, |
There was a problem hiding this comment.
When users feed persisted runtime TurnRecord exports such as the turns returned by the runtime thread detail API, those rows carry effective_provider and effective_model rather than provider and model (crates/tui/src/runtime_threads.rs:250-256). This deserializer aliases only effective_provider and still requires model, so those records fail to parse even though they contain the provider provenance this change is meant to use; add an effective_model alias before requiring provider-aware pricing.
Useful? React with 👍 / 👎.
| let Some(pricing) = OfferingPricing::from_catalog_offering(&offering) else { | ||
| return AvailableCost::default(); | ||
| }; | ||
| let Some(amount) = pricing.estimate_cost(token_usage) else { |
There was a problem hiding this comment.
For OpenAI API runs on gpt-5.5-pro (or any priced catalog model with no cache-read discount), a turn with prompt_cache_hit_tokens > 0 reaches estimate_cost, which returns None because cache_read_per_million is absent; the whole turn becomes unpriced even though the route has authoritative input/output prices and the existing pricing table treated gpt-5.5-pro cache hits at the input rate. This makes USD totals and cost regressions disappear for otherwise priced API routes whenever cache telemetry is present.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| let Some(pricing) = OfferingPricing::from_catalog_offering(&offering) else { |
There was a problem hiding this comment.
For Anthropic claude-sonnet-5 records before September 1, 2026, this path uses the bundled catalog's standard 3.00/15.00 rates instead of the existing claude_sonnet_5_pricing intro window, which pricing.rs explicitly keeps at 2.00/10.00 through August 31, 2026. That overstates scorecard cost and can create false cost regressions for this model during the active intro period; keep the existing time-aware pricing override after verifying the provider/model route.
Useful? React with 👍 / 👎.
| let Some(offering) = catalog_offering_for_model(provider, model) else { | ||
| return AvailableCost::default(); |
There was a problem hiding this comment.
For direct DeepSeek turns recorded with the still-supported compatibility aliases deepseek-chat or deepseek-reasoner (the config marks them valid until July 24, 2026), this exact catalog lookup returns None because the catalog only has the v4 ids, so the scorecard marks those priced DeepSeek calls as unavailable before the pricing layer can map them to the flash rate. Normalize these direct DeepSeek aliases before the catalog check, or otherwise preserve pricing for them until their configured retirement.
Useful? React with 👍 / 👎.
1 participant
Summary
provider/effective_providerprovenance in offline scorecard records while keeping legacy model-only JSON readable(provider, wire_model_id)catalog offerings so Codex OAuth, local/self-hosted, custom, unknown, and unpriced gateway routes fail closed instead of borrowing a model-only priceRoot cause
The offline scorecard accepted only
model + usageand called a model-only pricing helper. A model id such asgpt-5.5could therefore inherit OpenAI API pricing even when the recorded route was ChatGPT/Codex OAuth, Ollama, or another provider where that dollar price is unavailable.User impact
Token/cache metrics remain available for every record. Dollar and CNY values are now reported only when the recorded provider/model route has authoritative pricing; a real zero-cost turn is distinct from unavailable pricing in both the human summary and JSON output.
Validation
cargo fmt --all -- --check— passedgit diff --check upstream/main...HEAD— passedcargo test -p codewhale-tui --bin codewhale-tui --locked scorecard— 13 passedcargo test -p codewhale-config --locked— 360 passed--jsonsmoke with OpenAI API, Codex OAuth, and legacy records — passed; API subtotal was$0.0200, while OAuth/legacy and unavailable CNY were explicitly marked unavailablepython scripts/check-coauthor-trailers.py --range upstream/main..HEAD --check-authors— passedThe full Windows TUI suite completed with 6,329 passed, 2 ignored, and 5 failures. Two failures passed in isolation. The remaining three setup-matrix failures reproduce unchanged in a detached clean
upstream/main@3e97b278worktree, so they are not caused by this diff:doctor_setup_state_tests::doctor_setup_provider_model_json_covers_cn_codex_and_local_matrixtui::provider_picker::tests::setup_provider_key_entry_matrix_keeps_hosted_codex_and_local_hints_distincttui::setup::tests::provider_model_detail_lines_keep_codex_oauth_url_freeLocal Rust 1.95
clippy --all-targets -D warningsis also blocked by five pre-existing new lints in unchangedcompaction.rs,tui/ui.rs,tui/widgets/footer.rs, and twocore/engine/tests.rssites. No clippy warning pointed to this PR's files.Fixes #4335.