Skip to content

Savings are under-counted ~1000x outside en-US: the footer is written with toLocaleString() and read back with a comma-only regex #338

Open

Description

@Ercaner1988

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 1990

And 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"     -> 1990

So 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_bucket on the session_summary telemetry 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:

  1. Format the footer's number with a fixed locale. src/cli-epilogue.ts:50 already does exactly this (nodes.toLocaleString("en-US")), so there is precedent in the tree, and it keeps the reader untouched.
  2. Or emit the footer's number ungrouped and widen the regex to (\d+). Least pretty, most robust — nothing to strip, nothing to get wrong.
  3. Or keep the display locale-aware and strip every separator Intl might have used on the way in. This one is a trap: 1.990 is 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

  1. added 2 commits that reference this issue on Sep 10, 2026
    ba7e3b4
    dcaefcc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions