fix(session): guard malformed cost tiers

[macurandb]

Issue for this PR

Closes#43254

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

#43248 guarded the operands that go into decimal.js, but not the tier selection two lines above them, which still reads item.tier.type and item.tier.size unguarded. cost() in provider/provider.ts copies tier: item.tier straight from the models.dev / plugin payload with no guard of its own — unlike input/output/cache — so a malformed tiers still throws out of getUsage and takes the turn with it.

Three commits, each closing one way a malformed cost.tiers misbehaves:

1. Crashes. A tiers that is not an array, a null entry, or an entry missing tier each threw.
2. A tier that matches everything.size: 0 or a negative number passed the finite check and, through contextTokens > size, matched every context — a corrupt tier won the selection and billed at its own rates (108.9 instead of the base 4.5 in the test). Also fixes a regression the first commit introduced: the old comparison coerced contextTokens > "5000", so requiring a number silently disabled tiers whose size is a numeric string. size goes through Number() again; only unparseable or non-positive values are dropped.
3. A tier that zeroes the cost. An entry with a valid tier but no finite input/output won the selection and, via ?? 0, produced cost = 0 instead of falling back to the base rates.

Throughout, a malformed value is dropped, never coerced to 0 — coercing is what makes a bad tier match every context instead of none.

How did you verify your code works?

Each case was reproduced first and confirmed failing on dev before writing the fix:

cost.tiers	on dev
[{ input, output, cache }] (no tier)	TypeError: undefined is not an object
[null]	TypeError: null is not an object
{} (not an array)	TypeError: ...tiers?.filter is not a function
[{ …, tier: { type: "context", size: -1 } }]	billed 108.9 instead of 4.5
[{ tier: { type: "context", size: 5000 } }] (no rates)	billed 0 instead of 4.5

Five tests in the existing describe("SessionNs.getUsage") block cover them, plus one asserting a numeric-string size still applies its tier. To confirm: revert session.ts and re-run that file.

bun test test/session/compaction.test.ts → 61 pass, 1 skip, 0 fail. test/session/llm.test.ts + test/server/negative-tokens-regression.test.ts → 29 pass. tsgo --noEmit clean, prettier and oxlint clean.

Screenshots / recordings

Not a UI change.

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[Enough1122]

AI code review — automated review for reference; please use your judgment.

1. packages/opencode/src/session/session.ts:384-388 — the guard validates item.tier shape but not the tier's pricing payload: an entry like { tier: { type: "context", size: 5000 } } (missing input/output/cache) passes the filter and can be selected as costInfo. Why it matters: downstream arithmetic over undefined rates yields NaN cost silently — trading one malformed-data symptom (throw) for a subtler one (NaN billing numbers). Suggestion: also require finite numeric rates on the surviving entries, e.g. Number.isFinite(item.input) && Number.isFinite(item.output).

2. packages/opencode/src/session/session.ts:380-389 — malformed tiers are now silently dropped at usage-computation time. Why it matters: a provider/config typo (e.g. tiers: {}) disables tiered pricing with zero signal, so users get wrong-looking costs that are hard to trace back to the malformed source data. Suggestion: keep the defensive read, but emit a one-time log/warning when entries are discarded so bad model definitions surface during development instead of hiding forever. Longer term, validating cost.tiers at the config/provider schema boundary would reject this class of data at load time.

3. packages/opencode/src/session/session.ts:386 — Number.isFinite(item.tier.size) admits negative and zero sizes; combined with contextTokens > item.tier.size, a stray size: -1 tier matches every context and, being largest after sort... actually smallest — still wins whenever no bigger valid tier exists. Why it matters: negative sizes are almost certainly corrupt data, yet they're treated as legitimate ultra-low thresholds. Suggestion: require item.tier.size >= 0 in the filter.

4. packages/opencode/test/session/compaction.test.ts:1960-2001 — solid coverage of null entries, missing tier, non-numeric size, and non-array tiers; but there's no test where a surviving tier has malformed pricing fields (the NaN path from point 1), nor one asserting tier selection order when several valid tiers match. Suggestion: add both cases to lock in the intended semantics before this logic gets copied elsewhere.

— reviewer-h (automated AI reviewer)

[macurandb]

Thanks — went through all four against the code. Two landed, two didn't. Pushed the fixes.

3 (negative size) — confirmed, fixed.tiers: [{ input: 99, output: 99, tier: { type: "context", size: -1 } }] billed 108.9 instead of the base 4.5: the tier matched every context and won. Now dropped. Worth noting >= 0 isn't enough — size: 0 still matches every non-zero context, so the guard is size > 0.

Also fixed, which nobody flagged: my own guard regressed numeric strings. The comparison it replaced coerced contextTokens > "5000", so Number.isFinite("5000") silently disabled a tier that used to apply. size goes through Number() again; only unparseable or non-positive values are dropped.

1 (tier without pricing) — the mechanism doesn't hold, but there is a real issue underneath. No NaN: finite(costInfo?.input ?? 0) from #43248 turns the missing rates into 0, so that case returns cost = 0, isNaN = false. What does happen is that such a tier wins the selection and the cost silently drops to 0 instead of the base 4.5. That's preexistent — the old filter selected it too — and fixing it means deciding a policy (drop the tier vs. fall back to base rates), so I left it out of a PR that's about the crash. Happy to add Number.isFinite(item.input) && Number.isFinite(item.output) here if you'd rather it land in one go, or file it separately.

2 (silent drop) — agreed in principle, not here.getUsage is per-turn and logs nothing today, so a warn would be recurring noise. The right place is the boundary: cost() in provider/provider.ts copies tier: item.tier from the models.dev payload with no guard at all, unlike input/output/cache. Happy to open an issue for that.

4 (b) — already covered."uses matching context cost tier before over-200k fallback" (compaction.test.ts) uses context 650_000 where both the 200k and 500k tiers match, and asserts 2.75 + 0.6 + 0.05, which only holds if the 500k one wins. That is the ordering test. (a) only makes sense together with the point-1 guard — as things stand it would lock in cost = 0 as expected behaviour.

New tests cover the numeric string and both non-positive sizes.

[macurandb]

Correction to my previous comment: point 1 did land here after all, in 04aa839 — a tier with a valid tier but no finite input/output is now dropped so the cost falls back to the base rates instead of 0. I wrote that comment before that commit; the diff is the source of truth, not the comment.

So of the four: 1 and 3 fixed, 2 still belongs at the cost() boundary rather than here, 4(b) was already covered by the existing ordering test.

Current numbers, all reproducible against this branch: base rates 4.5 for the test usage; size: -1 and size: 0 billed 108.9 before, 4.5 now; a tier with no rates billed 0 before, 4.5 now; a numeric-string size still applies its tier (108.9). bun test test/session/compaction.test.ts → 61 pass, 1 skip, 0 fail. The PR description has been updated to match the three commits.