fix(scorecard): bind costs to provider routes#4351
Conversation
|
@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. |
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".
Carry turn identity, timestamp, and effective provider/model provenance through cancellation and compaction before emitting turn_end records. Use recorded-time pricing, explicit cache-rate fallbacks, and runtime field aliases while failing closed when route pricing is unavailable. Treat USD/CNY pricing completeness loss as a release-gate regression and exclude shell-only hook records from model scorecards. This addresses the cloud AI review feedback on PR Hmbown#4351. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25fc4c05a8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8411254d
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ab0717f99
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9264067441
ℹ️ 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".
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Thanks @nightt5879 — the provider-route binding work here looks thorough and the review trail is appreciated. Heads-up: v0.8.68 (#4361) and follow-ups just landed on Generated by Claude Code |
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>
Carry turn identity, timestamp, and effective provider/model provenance through cancellation and compaction before emitting turn_end records. Use recorded-time pricing, explicit cache-rate fallbacks, and runtime field aliases while failing closed when route pricing is unavailable. Treat USD/CNY pricing completeness loss as a release-gate regression and exclude shell-only hook records from model scorecards. This addresses the cloud AI review feedback on PR Hmbown#4351. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Require exact provider/model catalog provenance before recorded-time live cost estimates, while retaining DeepSeek alias retirement and Sonnet 5 time-window semantics. Integrate the upstream cache-write tiers into provider-time and scorecard paths, and clear underwater completion state before cancel-race suppression. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Canonicalize supported direct DeepSeek and Arcee aliases before the exact catalog lookup. Keep OpenRouter's Arcee namespace separate from the direct provider. When a catalog row omits cost, reuse legacy rates only for the authoritative direct Arcee Trinity Mini and MiniMax M2.7 routes; aggregator and foreign-provider rows remain unpriced. Validated with 21 scorecard tests, 29 pricing tests, cargo fmt, and a strict warning-free TUI check. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Classify the actual StepFun endpoint as PAYG or Step Plan without persisting raw URLs, then carry that non-secret provenance through turn-end hooks, runtime records, usage aggregation, and offline scorecards. Price only the exact StepFun PAYG fallback route with the official cache rate. Route-blind background and foreign-provider calls now fail closed so subscription quota cannot be reported as token spend. Validation: cargo fmt --all -- --check; RUSTFLAGS=-D warnings cargo check -p codewhale-tui --bin codewhale-tui --locked; targeted scorecard/pricing/runtime/hook tests; full TUI suite (6408 passed, 2 ignored before the final narrow gate, then gate-specific regression tests passed). Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Address the Rust 1.95 diagnostics surfaced by the PR lint job: remove the scorecard needless borrow, simplify equivalent input/footer control flow, and document narrow allowances where boxing Clap args or moving a mutable borrow into a match guard would make the code worse. Validation: CI-equivalent workspace/all-features clippy; 56 compaction tests; footer compact-work regression; 22 scorecard tests. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Always run turn_end observers for TurnComplete events that have no matching TurnStarted, including manual compaction, purge, and shell-only lifecycle completions. Give those records a unique lifecycle id, observed timestamp, and no route so they remain visible without inheriting stale model provenance or cost. Validation: CI-equivalent workspace/all-features clippy; completion-only and TurnStarted metadata regressions; turn_end payload regression. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Price runtime aggregates at each turn's recorded timestamp and route all scorecard estimates through the same provider-aware path. Fail closed for costless catalog offerings, retain only explicit provider-owned hand-price fallbacks such as OpenAI API gpt-5-codex, and derive cache savings from the exact provider route. Add regressions for historical Sonnet pricing, absent catalog hand rows, foreign costless routes, and scorecard parity. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Regenerate the embedded TUI changelog slice after documenting the final provider-aware pricing fixes. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Keep MiniMax M3 usage tiers and M2.7 cache-write pricing on provider-scoped paths, including the new Anthropic-compatible route. Reapply live OAuth and token-plan billing presentation before recorded-time cost accrual, and remove superseded helpers exposed by the conflict resolution. Validated with cargo fmt, 36 pricing tests, 23 scorecard tests, and targeted runtime, hook, route-capture, and subagent-cost tests. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Use Option::is_none directly so the latest stable clippy nonminimal_bool lint accepts the underwater action availability guard without changing behavior. Validated with the CI-equivalent all-features workspace clippy command. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
bdd66df to
38da51a
Compare
|
Rebased onto current Conflict resolution kept both sides' pricing semantics: the provider/recorded-route provenance from this PR, plus v0.8.68's MiniMax-M3 512K tier, MiniMax-M2.7 cache-write rate, MiniMax Anthropic route, and OAuth/token-plan billing presentation. The release changelogs remain synchronized, and every rewritten commit now carries a DCO trailer. Local validation completed:
The PR is now reported |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38da51aa96
ℹ️ 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".
Prefer the captured active turn route when recording provider health failures after TurnStarted clears the pending route. Keep the pending and selected routes as pre-start fallbacks, and cover the routed-turn case. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Classify StepFun PAYG and Step Plan routes before presenting usage, retain unknown billing for child routes without endpoint provenance, and allow the documented GPT-5.5 cache-write fallback without filling unrelated catalog gaps. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Deserialize runtime turns before usage is available and filter them from model-backed scoring. Compare complete CNY totals alongside USD totals so cost regressions cannot bypass the threshold gate. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
|
Addressed all five Codex review findings in
Local verification:
All five review threads have been replied to and resolved. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Take the shared test environment lock while comparing the generated missing-auth message with the active auth-file path. This prevents parallel environment guards from changing the path between the two reads on Windows CI. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
|
Windows CI on the previous head ran 6,666 TUI tests and found one unrelated OAuth test race ( Fixed in |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@Hmbown I resolved the conflic |
Summary
turn_end, runtime exports, usage aggregation, and scorecard ingestion(stepfun, step-3.7-flash, stepfun-payg)usage at the published cache-hit/cache-miss/output ratesgpt-5-codexremain availablemodel_backed: falselifecycle records, and preserve @findshan credit for v0.8.66 EPIC: Token, cache, and context discipline release gate #3388Root cause
The scorecard accepted only
model + usageand called model-only pricing. Identical model text can represent different providers and billing systems. StepFun adds a second ambiguity: its standard API and Step Plan share provider/model text but one is token-priced and the other is subscription quota. The recorder did not retain enough authoritative route provenance to distinguish them safely.Review and integration follow-up
Every submitted cloud review thread through review
4683378407has been investigated, answered, and resolved.fbdd9a3eaddressed the initial five findings around turn provenance, aliases, cache fallbacks, and recorded-time pricing.713568bdsynchronized upstreammainat02f05b03.31890408,25fc4c05, and3f841125preserved upstream cache-write behavior, fixed Windows fixture isolation, and constrained provider-owned fallback aliases.e4e01127added endpoint-derived StepFun PAYG/Plan provenance and exact PAYG pricing while all ambiguous routes fail closed.0ab0717ffixed stable-Clippy findings;92640674retained completion-only lifecycle observer events.6faa2b3daddresses all three findings from review4683378407: runtime aggregates now useturn.created_at, costless catalog routes cannot fall through to model-only prices, and explicit provider-owned hand rows work without a catalog offering.bdd66df9synchronizes the generated TUI changelog slice required by the Version drift check.Final Codex review completed on
bdd66df9with no major issues; no actionable threads were added.Validation
cargo fmt --all -- --check— passedgit diff --check— passedcargo clippy --workspace --all-features --lockedwith repository lint flags — passed29240683764onbdd66df9— passed, including Lint, Version drift, Ubuntu, macOS, Windows, Mobile, npm, DCO, Claude review, and GitGuardian; conditional jobs skipped as expectedFixes #4335.