fix(submit): exclude unpriced usage without aborting - #1053
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![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: 9deb76344c
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
does #1052 is better? |
|
Merged. This was a direct call against #1052, which fixes the same abort from the opposite direction, so recording the reasoning in both places. @pengtianabc — two points on why this one: 1. Excluding vs. zeroing. #1052 submits unpriced token-bearing usage at 2. Blast radius and verification. This is 3 files, +199/−53, and it generalizes machinery already in the tree ( To be clear about what this does not do: it is a stopgap for the v4.9.0 abort regression, not a fix for #1044. Excluded rows still shrink that day's tokens and cost, and the server still overwrites both — so a user with several thousand Thanks @Soju06. |
…1051) parse_gen_metadata read the model from chatModel.#19 and fell back to the literal string "unknown", which inferred_provider_from_model cannot place, producing unpriceable antigravity/unknown rows. Antigravity CLI drops a whole group of chatModel fields (1, 2, 8, 10, 16, 19) on some continuation and tool turns, but #21 — the model display label — survives on every one, and sibling rows carry the machine id next to the identical label. Turns are now attributed by resolving that label against labels seen elsewhere in the same database. Rejected: fall back to the conversation's sole model whenever a row lacks a label | a conversation containing a model switch would bill the wrong model instead of staying unknown (cubic P1, fixed in 73d5937 by withholding the fallback when any label is unresolved) Confidence: high Scope-risk: narrow Directive: post-#1053 these rows no longer abort a submission — they are excluded silently, so a regression here is invisible rather than loud Not-tested: a database where the same display label maps to two different machine ids
#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. The existing hard-fail path never fired because generate_submission_graph always passes Some(..) — get_or_init degrades each failed source to an empty map rather than erroring — so the guarding test asserted an unreachable None. Two #1053 core tests and one CLI test used an empty PricingService to mean "this model is unpriced", the same conflation; all 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
…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
|
Shipped in v4.11.0 — https://github.com/junhoyeo/tokscale/releases/tag/v4.11.0 @Soju06 thanks for this. It's the headline fix of the release and it unblocked several people at once (#1013, #1021, #1035). The way you scoped it — generalising the existing gemini-default exclusion rather than inventing a new mechanism, and keeping the whole thing out of the reporting path — is why it went in cleanly. Two follow-ups landed on top of it, both from reviewing the shape you established:
@pengtianabc thanks for the discussion on the alternative approach; the sequencing argument from that thread fed directly into #1055. If tokscale has been useful to you, a ⭐ on the repo helps other people find it. |
custom-pricing.json is the documented escape hatch for models no upstream dataset publishes, but it required at least one rate to be POSITIVE. A free tier was therefore unexpressable: the user knows the price is $0, the file refuses to record it, and the usage stays unpriced and excluded from submission with no way to correct it (#1021). Same free-vs-absent confusion as the Cursor cache-creation gap, at the user-facing layer. 0.0 is an assertion ("this is free"); None is an absence. Only absence is rejected now — reading a rate-less row as free would invent a $0 total for unknown pricing. Negative rates still rejected. All four READMEs said a rate must be present and positive, which would have kept users away from the hatch this adds; updated in the same PR. The reporter's other two asks need no code: #1053 made unpriced rows exclude-with-warning by default (their --prune-unpriced request), and zai/glm-4.5-flash resolves today. Confidence: high Scope-risk: narrow Directive: 0.0 and None are not interchangeable in pricing rows anywhere in this crate — 0.0 prices, None excludes
3 participants
Summary
Why
Submission validation currently aborts the whole batch when any message has an incomplete price. This still affects valid model rows whose input, output, and cache-read rates resolve but whose cache-write rate is absent, as well as isolated local test-model usage. A single such row prevents all otherwise priced history from being submitted.
This change excludes the entire affected message rather than pricing the missing bucket at zero or submitting only part of its tokens. That keeps submitted token totals and costs consistent. The existing CLI exclusion warning makes the loss explicit, and a batch containing only excluded usage still reports that there is no usage to submit.
Follow-up to #1013.
Tests
cargo fmt --all --checkcargo test -p tokscale-core(1,479 passed, 1 ignored; all integration suites passed)cargo test -p tokscale-cli -- --skip headless_capture(1,022 unit tests and 148 CLI integration tests passed; 1 unit test ignored)target/debug/tokscale --no-spinner submit --dry-runagainst real local history containing 10 affected provider/model groups (exit 0, warnings emitted, priced remainder produced, no submission performed)The three skipped
headless_capture_*tests are existing wall-clock threshold tests. They also exceeded their timing bounds when rerun alone on this host; no headless code is changed here.