Skip to content

Commit ba7e3b4

Browse files
Ercaner1988claude
andcommitted
fix(savings): pin rendered numbers to one locale so graft can read its own footer
savingsLine() wrote the `[graft] tokens saved ≈ N` footer with the machine's locale; sumSavingsFooters() read it back with `([\d,]+)`. The two are fifteen lines apart, under a comment saying they are kept together so they cannot drift — and they had not drifted from each other, only from Intl. On a machine that groups thousands with anything but a comma, graft parsed its own footer as the first group: de-DE and tr-TR write `1.990` and get 1, fr-FR and ru-RU write `1 990` (narrow no-break space) and get 1. Everything downstream of savedTokens carried that number, not the real one — the statusline's tokens saved and its dollar value, `graft stats`, and the `saved_tokens_bucket` on session_summary telemetry, all short by ~1000x for every user outside a comma locale. One pinned formatter, used by all five sites that render one of these numbers: the three that write the parsed footer (context/savings.ts, ask/ask.ts, claude/format.ts) and the two displays beside it. The displays are never parsed, but a statusline reading `1.990` above a footer reading `1,990` would be its own small bug, and cli-epilogue.ts already pins the same way. The reader is untouched: widening it would mean guessing, since `1.990` is a valid en-US decimal. The test moves the *default* locale rather than the ambient one — which a process cannot set for itself — so it fails without this fix on an en-US runner too. That is the property CI was missing: its runners always had the comma, so the round trip could never break there. Fixes #338 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent de8456e commit ba7e3b4

6 files changed

Lines changed: 89 additions & 15 deletions

File tree

‎src/ask/ask.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { existsSync, readdirSync, readFileSync } from "node:fs";
1717
import { join, resolve } from "node:path";
1818
import matter from "gray-matter";
1919
import { contextDirFor } from "../context/node-file.js";
20-
import { withSavings, savingsFor, savingsTurnNudge, type Savings } from "../context/savings.js";
20+
import { withSavings, savingsFor, savingsTurnNudge, groupDigits, type Savings } from "../context/savings.js";
2121
import { loadGraphCached, loadAskIndexCached } from "../graph/load.js";
2222
import {
2323
assertPrefixIndexed,
@@ -1561,9 +1561,9 @@ function askSavingsLine(r: AskResult, body: string): string {
15611561
const saved = base - pack;
15621562
const pct = Math.round((saved / base) * 100);
15631563
return (
1564-
`[graft] tokens saved ≈ ${saved.toLocaleString()} (${pct}%) — this pack ≈ ` +
1565-
`${pack.toLocaleString()} tok vs reading the ${r.saved.files} source file(s) whole ≈ ` +
1566-
`${base.toLocaleString()} tok. Estimate (baseline = those files read in full).` +
1564+
`[graft] tokens saved ≈ ${groupDigits(saved)} (${pct}%) — this pack ≈ ` +
1565+
`${groupDigits(pack)} tok vs reading the ${r.saved.files} source file(s) whole ≈ ` +
1566+
`${groupDigits(base)} tok. Estimate (baseline = those files read in full).` +
15671567
savingsTurnNudge(saved)
15681568
);
15691569
}

‎src/claude/format.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { GraphV1, EdgeV1 } from '../graph/types.js';
66
// question in both places, and one set of calibrated numbers beats two.
77
import { HIGH_FLOOR, STRONG_FLOOR } from '../ask/fuse.js';
88
import { dollarsSaved, formatDollars } from '../context/price.js';
9+
import { groupDigits } from '../context/savings.js';
910

1011
const C = {
1112
indigo: (s: string) => `\x1b[38;2;84;111;255m${s}\x1b[0m`,
@@ -39,7 +40,7 @@ export function renderStatusline(
3940
// stands alone rather than carrying a rate nobody measured.
4041
const usd = dollarsSaved(saved, session?.inputCostMicros, session?.inputTokensBilled);
4142
const money = usd === null ? '' : ` · ~${formatDollars(usd)}`;
42-
top.push(C.indigo(`~${saved.toLocaleString()} tok saved${money}`));
43+
top.push(C.indigo(`~${groupDigits(saved)} tok saved${money}`));
4344
}
4445

4546
const bottom: string[] = [];
@@ -133,9 +134,9 @@ export function formatRetrieval(ask: AskJson, cap = 5): string | null {
133134
const base = tokensOf(ask.saved!.baselineChars);
134135
const pct = Math.round((saved / base) * 100);
135136
return (
136-
`${body}\n[graft] tokens saved ≈ ${saved.toLocaleString()} (${pct}%); this pack ≈ ` +
137-
`${tokensOf(body.length).toLocaleString()} tok vs reading the ${ask.saved!.files} file(s) whole ≈ ` +
138-
`${base.toLocaleString()} tok (estimate).`
137+
`${body}\n[graft] tokens saved ≈ ${groupDigits(saved)} (${pct}%); this pack ≈ ` +
138+
`${groupDigits(tokensOf(body.length))} tok vs reading the ${ask.saved!.files} file(s) whole ≈ ` +
139+
`${groupDigits(base)} tok (estimate).`
139140
);
140141
}
141142

‎src/claude/session-metrics.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
*/
3232
import { statSync } from 'node:fs';
3333
import { join } from 'node:path';
34-
import { sumSavingsFooters } from '../context/savings.js';
34+
import { groupDigits, sumSavingsFooters } from '../context/savings.js';
3535
import { dollarsSaved, formatDollars } from '../context/price.js';
3636
import { readSession, writeSession, sessionDir, listSessionIds, type SessionState } from './state.js';
3737
import type { AgentHost } from '../telemetry/contract.js';
@@ -199,7 +199,7 @@ export function formatSessionStats(s: SessionSummary | null): string {
199199
` graft reads: ${graft}`,
200200
` source reads: ${source} (Read / Grep / Glob)`,
201201
` mix: ${mix}`,
202-
` tokens saved: ~${saved.toLocaleString()}`,
202+
` tokens saved: ~${groupDigits(saved)}`,
203203
];
204204
// Omitted, not zeroed, when no turn has been billed yet: a host whose hooks
205205
// name no transcript can't know what a token costs here, and a made-up rate

‎src/context/savings.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,25 @@ export function savingsTurnNudge(savedTokens: number): string {
105105
);
106106
}
107107

108+
/**
109+
* Digits grouped for display, pinned to en-US.
110+
*
111+
* Not a style choice: the `[graft] tokens saved ≈ N` line below is read back by
112+
* {@link sumSavingsFooters}, whose regex knows `,` and nothing else. Rendered
113+
* with the machine's locale instead, a de-DE or tr-TR box wrote `1.990` and
114+
* graft parsed its own footer as `1` — every savings total downstream, and the
115+
* dollar value beside it, off by a factor of a thousand (#338). fr-FR and ru-RU
116+
* group with a narrow no-break space and lost it the same way.
117+
*
118+
* Every number graft renders goes through here, not only the parsed one: a
119+
* statusline that grouped differently from the footer right above it would be
120+
* its own small bug. `src/cli-epilogue.ts` pins the same way, for the same
121+
* reason.
122+
*/
123+
export function groupDigits(n: number): string {
124+
return n.toLocaleString('en-US');
125+
}
126+
108127
/** The one-line savings estimate for a command's text output, so the agent gets
109128
* the number for free — no extra tool call. `body` is the exact rendered output
110129
* the agent reads (the pack). Returns "" when there's nothing honest to claim:
@@ -118,9 +137,9 @@ export function savingsLine(body: string, saved: Savings | undefined): string {
118137
const delta = base - pack;
119138
const pct = Math.round((delta / base) * 100);
120139
return (
121-
`[graft] tokens saved ≈ ${delta.toLocaleString()} (${pct}%) — this output ≈ ` +
122-
`${pack.toLocaleString()} tok vs reading the ${saved.files} file(s) it covers whole ≈ ` +
123-
`${base.toLocaleString()} tok (estimate).` +
140+
`[graft] tokens saved ≈ ${groupDigits(delta)} (${pct}%) — this output ≈ ` +
141+
`${groupDigits(pack)} tok vs reading the ${saved.files} file(s) it covers whole ≈ ` +
142+
`${groupDigits(base)} tok (estimate).` +
124143
savingsTurnNudge(delta)
125144
);
126145
}

‎test/savings-locale.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* #338: graft rendered its savings footer with the machine's locale and read it
3+
* back with a regex that knows only `,`, so a `de-DE`/`tr-TR` box wrote `1.990`
4+
* and parsed it as `1`.
5+
*
6+
* A process cannot change its own ambient locale — Node takes it from the
7+
* environment on posix and from the OS on Windows — so these tests move the
8+
* *default* instead: `toLocaleString()` with no argument answers the way a
9+
* `.`-grouping locale would, while calls that name a locale are left alone.
10+
* That is exactly the difference the fix turns on, and it behaves identically on
11+
* an en-US CI runner, which is why CI never caught the original.
12+
*/
13+
import { test } from 'node:test';
14+
import assert from 'node:assert/strict';
15+
import { savingsLine, sumSavingsFooters } from '../src/context/savings.js';
16+
import { formatSessionStats } from '../src/claude/session-metrics.js';
17+
18+
/** Run `fn` on a machine whose default locale groups thousands with `locale`'s
19+
* separator. Restores the real method even if `fn` throws. */
20+
function withDefaultLocale<T>(locale: string, fn: () => T): T {
21+
const real = Number.prototype.toLocaleString;
22+
Number.prototype.toLocaleString = function (
23+
this: number,
24+
locales?: string | string[],
25+
options?: Intl.NumberFormatOptions,
26+
): string {
27+
return real.call(this, locales ?? locale, options);
28+
};
29+
try {
30+
return fn();
31+
} finally {
32+
Number.prototype.toLocaleString = real;
33+
}
34+
}
35+
36+
test('#338: the savings footer round-trips on a machine that is not en-US', () => {
37+
// Sanity: the stand-in really does change what an unqualified call renders,
38+
// or the rest of this file would pass for the wrong reason.
39+
assert.equal(withDefaultLocale('de-DE', () => (1990).toLocaleString()), '1.990');
40+
41+
const line = withDefaultLocale('de-DE', () => savingsLine('x'.repeat(40), { files: 2, baselineChars: 8000 }));
42+
assert.match(line, /tokens saved ≈ 1,990/, 'grouped for the reader, not for the machine');
43+
assert.equal(sumSavingsFooters(line), 1990, 'and read back whole, not as its first group');
44+
});
45+
46+
test('#338: what graft displays groups the same way as what it parses', () => {
47+
const out = withDefaultLocale('de-DE', () =>
48+
formatSessionStats({ id: 'abc', perAgentQuery: {}, graftReads: 8, sourceReads: 2, savedTokens: 12345 }),
49+
);
50+
assert.match(out, /tokens saved:\s+~12,345/);
51+
});

‎test/savings.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55
import { test } from 'node:test';
66
import assert from 'node:assert/strict';
7-
import { savingsFor, savingsLine, withSavings, toTokens, setInputRate } from '../src/context/savings.js';
7+
import { savingsFor, savingsLine, withSavings, toTokens, setInputRate, groupDigits } from '../src/context/savings.js';
88
import { hasSavingsTally } from '../src/claude/tally.js';
99
import type { GraphV1, NodeV1 } from '../src/graph/types.js';
1010

@@ -49,7 +49,10 @@ test('savingsLine: reports saved tokens and percent when the output is smaller',
4949
assert.match(footer, /tokens saved ≈ [\d,]+ \(\d+%\)/);
5050
assert.match(footer, /2 file\(s\)/);
5151
const base = toTokens(8000);
52-
assert.ok(footer.includes((base - toTokens(body.length)).toLocaleString()));
52+
// Through the same pinned formatter the footer uses: rendered with the test
53+
// machine's locale instead, this asserted `1.990` against a footer that says
54+
// `1,990` — the mirror image of #338, and just as invisible on an en-US runner.
55+
assert.ok(footer.includes(groupDigits(base - toTokens(body.length))));
5356
// The nudge rides along so the agent reports the turn total without SKILL.md.
5457
assert.match(footer, /end of your reply/i);
5558
assert.match(footer, /graft saved ~N tokens this turn/);

0 commit comments

Comments
 (0)