fix(submit): reject exclusions no pricing dataset backs - #1055
Conversation
#1053 made submission exclude any token-bearing message the pricing service cannot cover. A service with no dataset covers nothing, so a cold cache with no network excluded the whole batch, left total_tokens at 0, and let the CLI print "No usage data found to submit" and exit 0 — indistinguishable from an empty history, and reported as success to autosubmit. Verified against 3ab58f5: the same fixture exits 1 there. Exclusions are now rejected when no dataset loaded. Both conditions are required: a batch whose costs are all provider-reported never consults pricing and still submits, and a populated dataset that merely lacks a price for some model stays non-fatal, which is the case #1053 exists to handle. has_pricing_data() deliberately ignores the compiled-in cursor/sakana tables, which are present on every run and would mask a total outage. The guard runs after exclusion because the exclusion list is the signal; validate_priced_messages sees only survivors, and when everything is excluded that slice is empty and validates trivially. That is why the existing hard-fail path never fired: generate_submission_graph always passes Some(..) since get_or_init degrades each failed source to an empty map rather than erroring, so submission_without_any_pricing_data_still_fails was guarding an unreachable None. It now covers both shapes. Two #1053 tests used an empty PricingService to mean "this model is unpriced" — the same conflation — and prime_pricing_cache writes an empty `data` object, so the CLI exclusion test ran with no pricing at all and passed only because OpenCode marks its positive costs provider-reported. Both now use a populated, non-covering dataset. Constraint: server upserts daily_breakdown.cost unconditionally (#1044), so a submission must never be quietly emptied by degraded pricing Rejected: fail whenever everything was excluded | breaks autosubmit for users whose usage is legitimately unpriceable, the trap #1044 documents Rejected: guard before exclusion | fires when every cost is authoritative and pricing is irrelevant Confidence: high Scope-risk: narrow Directive: cursor/sakana must stay out of has_pricing_data — counting them reports healthy pricing during a total upstream outage Not-tested: partial degradation (one upstream cached, two empty), which still under-reports a day and remains #1044's to fix
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f772082968
ℹ️ 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".
| /// question and is otherwise indistinguishable from healthy pricing that | ||
| /// happens not to cover the model in hand. | ||
| pub fn has_pricing_data(&self) -> bool { | ||
| !self.custom.is_empty() || self.lookup.has_upstream_dataset() |
There was a problem hiding this comment.
Don't treat a custom override as a complete pricing dataset
When all three upstream sources and caches are unavailable but the user has any unrelated entry in custom-pricing.json, this returns true and require_trustworthy_exclusions accepts every exclusion. Custom pricing is documented and implemented as an exact-match override that falls through to upstream sources, not a complete catalog, so an offline submission containing other models can again exclude the entire batch and exit successfully as if there were no usage—the failure this change is intended to prevent. Only a successfully loaded upstream dataset should establish that an unmatched model is genuinely unpriced; custom entries can still price the models they actually cover.
Useful? React with 👍 / 👎.
…ing it The first pass replaced one inaccurate claim with three more, all found in adversarial review: - "excluded with a warning and the rest is submitted" ignored #1055: with no pricing dataset loaded at all, require_trustworthy_exclusions fails the submission rather than excluding anything. - "carrying a char-based estimate" no longer describes production. #1037 set allow_char_estimate: false for Claude transcripts, so the extractor returns None and a text-only tool result is dropped for zero usage. Only explicit tool-result token metadata can reach the model-inheritance branch. - "invisible in normal use" overstated it. The CLI prints a named exclusion warning, and a batch left with nothing prints "No usage data found to submit" and performs no HTTP submission. Also fixes the opposite stale claim left in the same file, where a test still said submission "rejects" the escaped usage. Confidence: high Scope-risk: narrow Directive: this comment describes behavior owned by lib.rs, not this file — re-check it against the submission path when either changes
…e mode (#1057) The guard added in #1045 was documented as preventing an abort with "pricing is unavailable for submitted token usage". #1053 replaced that abort with per-model exclusion, so the comment was stale. The first correction overstated in the other direction. Adversarial review found three inaccuracies in it, all fixed here: - exclusion is not unconditional. With no pricing dataset loaded at all, #1055's require_trustworthy_exclusions fails the submission. - "char-based estimate" no longer describes production. #1037 set allow_char_estimate: false for Claude transcripts, so a text-only tool result yields no usage and is dropped; only explicit tool-result token metadata reaches the model-inheritance branch. - "invisible" was wrong. The CLI prints a named exclusion warning, and a batch left empty prints "No usage data found to submit" without submitting. Also removes the opposite stale claim in the same file, where a test still described submission as rejecting the escaped usage. Comment-only; 1,490 core tests pass, fmt and clippy --all-targets clean. Confidence: high Scope-risk: narrow Directive: this comment describes behavior owned by lib.rs, not this file — re-check it against the submission path when either changes
Summary
Follow-up to #1053. That PR made submission exclude any token-bearing message the pricing service cannot cover. A service with no dataset covers nothing, so a cold cache with no network excluded the whole batch, left
total_tokensat 0, and let the CLI printNo usage data found to submitand exit 0 — indistinguishable from an empty history, and reported as success to autosubmit.Verified against
3ab58f58(pre-#1053): the same fixture exits 1 there withError: pricing is unavailable for submitted token usage.Exclusions are now rejected when no dataset loaded. Both conditions are required:
has_pricing_data()deliberately ignores the compiled-incursor/sakanatables. They are present on every run, so counting them would report healthy pricing during a total upstream outage — exactly the condition being detected.Why the existing guard never fired
The check runs after exclusion, because the exclusion list is the signal. It cannot live in
validate_priced_messages, which sees only survivors — and when everything is excluded that slice is empty and validates trivially.That is also why the existing hard-fail path was dead:
generate_submission_graphalways passesSome(..), sincePricingService::get_or_initdegrades each failed source to an empty map rather than erroring (combine_fetched_sourcesreturnsOkon every path). Sosubmission_without_any_pricing_data_still_failswas guarding an unreachableNone. It now covers both shapes.Test fixtures that hid this
PricingServiceto mean "this model is unpriced" — the same conflation the product had.prime_pricing_cachewrites"data":{}, sotest_submit_excludes_unpriced_usage_and_keeps_the_restran with no pricing data at all. It passed only because OpenCode marks its positivecostvalues provider-reported, so the "priced" remainder survived on authoritative costs rather than pricing coverage.All three now use a populated, non-covering dataset via a new
prime_pricing_cache_with_a_priced_modelhelper.Added
test_submit_offline_without_pricing_cache_fails(end-to-end, restoring the intent of the test #1053 repurposed) and a core test for the authoritative-costs-without-pricing case.Tests
cargo test -p tokscale-core -p tokscale-cli— 1,479 core lib + 1,023 CLI unit + 153 CLI integration + all integration suites, 0 failurescargo fmt --all --checkcleancargo clippy --all-targets -- -D warningsclean (note--all-targetslints test code, which CI omits)Not addressed
#1044 is untouched. Partial degradation — one upstream cached, two empty — passes the predicate, excludes a subset, and still under-reports that day's tokens and cost against the unconditional
cost = EXCLUDED.costupsert. That remains thecostIsCompleteprotocol change.Summary by cubic
Reject exclusions when no pricing dataset is loaded to prevent empty submissions from being reported as success. The CLI now fails with "pricing data is unavailable for submission" instead of printing "No usage data found to submit" in this scenario.
tokscale-core: if any messages were excluded and pricing has no upstream/custom dataset, error with "pricing data is unavailable for submission".PricingService::has_pricing_data()andPricingLookup::has_upstream_dataset()that ignore compiled-incursor/sakanatables to detect real outages.tokscale-clitests withprime_pricing_cache_with_a_priced_modeland added an offline regression test that asserts the failure path.Written for commit f772082. Summary will update on new commits.