Uh oh!
There was an error while loading. Please reload this page.
fix(session): guard cost calc against non-numeric provider usage/cost fields - #43096
Closed
macurandb wants to merge 5 commits into
Closed
fix(session): guard cost calc against non-numeric provider usage/cost fields#43096macurandb wants to merge 5 commits into
macurandb wants to merge 5 commits into
Conversation
… fields `getUsage` fed `model.cost` values straight into `decimal.js`. Those fields are declared as `Schema.Finite`, but the raw models.dev / provider payload is never decoded against that schema, so a non-numeric field (e.g. an object) reached `new Decimal(...)`/`.mul(...)` and threw `DecimalError: Invalid argument: [object Object]`, aborting the turn. Coerce every operand fed to decimal.js to a finite number, defaulting to 0, so cost degrades to a correct partial value instead of throwing. Well-formed providers are unaffected: numbers in, same cost out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
The tier selection right above the cost block reads `item.tier.type` and `item.tier.size` unguarded, and `cost()` in provider/provider.ts copies `tier: item.tier` straight from the models.dev / plugin payload with no `?? 0` of its own — unlike `input`/`output`/`cache`. A tiers entry without a `tier` key therefore throws `TypeError: undefined is not an object (evaluating 'item.tier.type')` from the same unvalidated source, with the same symptom the rest of this PR fixes. Drop malformed tiers rather than trusting them. A non-numeric `size` must not be coerced to 0 either: that would make the tier match every context instead of none. Also drop the redundant `num()` around `totalNanoAiu` — that branch is already narrowed to a finite non-negative number. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tier field
The previous commit guarded `item.tier`, but the surrounding shape was still
trusted: `cost.tiers` that is not an array throws on `.filter`, and a null entry
throws on `item.tier` — both before the guard runs, both from the same
unvalidated payload. Verified by probe: `tiers: {}` and `tiers: [null]` each
still aborted the turn.
Treat a non-array `tiers` as empty and skip null entries, so the claim this
commit series makes actually holds for the whole tiers payload.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Every `tokens.*` value already goes through `safe()` a few lines above, which returns 0 for anything non-finite — so wrapping them again said "this could be non-numeric" about values that provably cannot be. Two independent reviewers read it as noise. The guard now sits exactly where the unvalidated data enters: the `costInfo` operands. No behaviour change: same inputs, same cost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first guard only accepted `typeof value === "number"`, which silently turned
a numeric-string cost field into 0. That is a real behaviour regression, and in
exactly the scenario this PR exists for: the payload is not schema-decoded, so a
hand-written provider config or a plugin can hand us `"3"` — and `decimal.js`
priced that correctly before the guard existed.
Verified against decimal.js 10.5.0: for every string it could parse ("3", "1e3",
"0x10") the computed cost is identical with and without this change; the only
inputs that behave differently are the ones that used to throw ("abc", " 3.5 ")
or that already collapsed to 0 via `safe()` ("NaN", "Infinity").
Trading a crash for a silently under-reported cost would have been a poor deal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>macurandb
commented
Aug 18, 2026
Author
The core of this landed in #43248 — closing in its favour. One case it didn't cover: the tier selection two lines above the cost operands still reads |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes#43098
Type of change
What does this PR do?
getUsagefeedsmodel.coststraight into decimal.js. That payload is never decoded against its schema and?? 0only covers null/undefined, so a non-numeric cost field throws[DecimalError] Invalid argument: [object Object]and takes the turn down with it — cosmetic on the CLI, where the answer is already printed, but fatal underopencode acp, where a tool-using turn dies mid-flight.The tier selection just above it breaks the same way from the same payload: a
tiersentry missing itstier, a null entry, or a non-arraytierseach throw a TypeError.So: non-numeric cost fields now price at 0 and malformed tiers are dropped, and the cost degrades to a partial value instead of aborting. Numeric strings still price as before — decimal.js parsed those fine, and turning them into 0 would trade a crash for silent under-billing.
How did you verify your code works?
Six unit tests in the existing
getUsagedescribe block. I wrote each one first and confirmed it fails ondevwith the error above before writing the fix.To confirm it: revert
session.tsand re-run that file — the four malformed-payload tests fail, the two well-formed ones stay green.bun test test/session/compaction.test.ts test/server/negative-tokens-regression.test.ts→ 61 pass,test/session/llm.test.ts→ 28 pass,tsgo --noEmitclean, oxlint no new errors.Screenshots / recordings
Not a UI change.
Checklist