Repository navigation
Savings are under-counted ~1000x outside en-US: the footer is written with toLocaleString() and read back with a comma-only regex #338
Description
graft writes its savings footer with the machine's locale and reads it back with a regex that only knows ,. On any locale that groups thousands with something else, the number it recovers is the first group — 1.990 comes back as 1.
src/context/savings.ts, fifteen lines apart:
// writer, line 121
`[graft] tokens saved ≈ ${delta.toLocaleString()} (${pct}%) — this output ≈ ` +
// reader, line 137
for (const m of text.matchAll(/\[graft\] tokens saved ≈ ([\d,]+)/g)) {
total += Number(m[1].replace(/,/g, '')) || 0;The comment above them says they live next to each other "so the two can't drift". They have not drifted from each other — they have drifted from Intl.
Repro
A round trip through the pair, on a tr-TR machine, no arguments involved:
$ node --import tsx probe.ts
default locale: tr-TR
footer written: [graft] tokens saved ≈ 1.990 (100%) — this out
read back : 1 (should be 1990)// probe.ts
import { savingsLine, sumSavingsFooters } from "./src/context/savings.js";
const line = savingsLine("x".repeat(40), { files: 2, baselineChars: 8000 })!;
console.log(sumSavingsFooters(line)); // 1, not 1990And the reader against footers rendered in a few locales — this part is locale-independent, so it reproduces anywhere:
en-US "1,990" -> 1990
tr-TR "1.990" -> 1
de-DE "1.990" -> 1
fr-FR "1 990" -> 1 # U+202F narrow no-break space
ru-RU "1 990" -> 1
hi-IN "1,990" -> 1990So it is not a Turkish problem: de, es, it, nl, pt-BR, id, da, el group with ., and fr, ru, sv, pl, cs, fi group with a space. Any of those under-counts by roughly a factor of a thousand — and the smaller the true number, the closer the wrong one looks to plausible.
What it costs
Everything downstream of savedTokens reads that recovered number, not the real one:
- the statusline's
~N tok saved(src/claude/format.ts:42) - its dollar value — the 0.17.0 headline (feat(claude): publish the dollar value of what graft saved #282). A session that saved 40k tokens prices 40.
graft stats(src/claude/session-metrics.ts:202)saved_tokens_bucketon thesession_summarytelemetry event (src/telemetry/sessions.ts:67), so every affected user reports a near-zero bucket and the aggregate under-states graft's payoff by however large that share of your users is.
The parser is the only one of its kind (sumSavingsFooters is the single [\d,] regex in src/), so the blast radius is exactly the savings accounting — nothing about the graph itself is affected.
How it surfaces
npm test on such a machine fails 8 tests, on main, with no local changes:
| test | file |
|---|---|
tool-savings counts a REAL savings line (with the turn nudge) exactly once |
test/claude-hooks.test.ts |
formatRetrieval appends a tokens-saved line when ask reports a baseline |
test/claude-format.test.ts |
renderStatusline carries the dollar value once the session has been billed |
test/claude-format.test.ts |
renderStatusline shows tokens alone until a turn has been billed |
test/claude-format.test.ts |
savingsLine: reports saved tokens and percent when the output is smaller |
test/savings.test.ts |
formatSessionStats renders the mix, savings and last query |
test/session-metrics.test.ts |
formatSessionStats prices the saving once the session has been billed |
test/session-metrics.test.ts |
formatSessionStats omits the dollar line rather than claiming zero |
test/session-metrics.test.ts |
Seven of those are assertions that assume , — a rendering difference, not a defect. The eighth is the bug itself, and it is worth reading on its own:
✖ tool-savings counts a REAL savings line (with the turn nudge) exactly once
AssertionError: Expected values to be strictly equal:
1 !== 1990
at test/claude-hooks.test.ts:464
That test feeds a real savingsLine() through the tool-savings hook and asserts the session recorded 1990. It is already the round-trip test for this; it just has no way to fail on a , locale.
CI cannot see any of it: GitHub runners are en-US, so the comma is always there.
To reproduce on Linux or macOS, LC_ALL=de_DE.UTF-8 npm test should do it — Node takes its default locale from the environment there. I have only verified it on Windows, where the locale comes from the OS regional setting instead and those variables have no effect.
Fix
Your call which end to pin, but they have to agree:
- Format the footer's number with a fixed locale.
src/cli-epilogue.ts:50already does exactly this (nodes.toLocaleString("en-US")), so there is precedent in the tree, and it keeps the reader untouched. - Or emit the footer's number ungrouped and widen the regex to
(\d+). Least pretty, most robust — nothing to strip, nothing to get wrong. - Or keep the display locale-aware and strip every separator
Intlmight have used on the way in. This one is a trap:1.990is a valid en-US decimal, so the reader would be guessing, and it has to know about U+202F and U+00A0 too.
Whichever you pick, the seven rendering assertions want a fixed locale in the test rather than the machine's, or they will keep failing for contributors outside en-US.
Happy to send a patch for whichever you prefer.
Environment
graft main (0.17.0, 3a85a2f), Node v24.16.0 (ICU 78.3, full ICU), Windows 11 x64, OS locale tr-TR. Found while running the suite for #337; unrelated to that PR, which is why it is here and not in it.
Activity
fix(savings): pin rendered numbers to one locale so graft can read it…
test(savings): cover the `graft ask` footer, the third writer of the …