From ab24b9604f9bfd959b401c78916e35f476664007 Mon Sep 17 00:00:00 2001 From: ojassug Date: Sun, 9 Aug 2026 21:08:42 +0530 Subject: [PATCH 1/3] fix(drift): a witness that existed before does not count if none of it survived MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the measurement half of audit C1. `findUnwitnessedItems` built its probe bundle from the *before* item, so it asked "did evidence exist?" rather than "did any of it survive?", and an item whose witnesses were all destroyed was exempt on the grounds that they had once been there. Measured on this repository's own files, on both the file and stdin routes: `CODE_OF_CONDUCT.md` went 3,542 -> 72 bytes and `SECURITY.md` 1,154 -> 72, at `fallbackUsed: false`, `validation.passed: true`, both gates reporting `pass`. On the CLI that is unrecoverable — no `TokenHasher` is supplied, so the removed bytes exist nowhere. Neither gate could fire, and the arithmetic is closed-form rather than a tuning miss. Prose yields no symbols, so `R_AST = 1.0` as an empty-set default and contributes a free 0.60. `filepath:` is derived from `item.path` and no content transform can destroy it, so `R_struct = 1/(N+1)` for N headings. Therefore `S_k = 0.4·N/(N+1)`, which approaches 0.40 from below and never reaches it, for any N, against a retention gate firing on `> 0.40`. The two stdin rows landed on exactly 0.400 and were admitted by the strict comparison — the supremum of the expression waved through by the operator, not a near miss. An item that changed is now refused when it yields no symbols and no content-derived markers survive in the *after* item. Two properties keep it safe: it is scoped to symbol-free items, so whole-item elision of code still refuses as SEMANTIC_DRIFT_EXCEEDED — the accurate reason, since `R_AST` measured that loss exactly — rather than being relabelled unmeasurable; and it only ever adds refusals, since refusing on the surviving set is strictly stronger than refusing on the before set, so every §33 refusal still refuses. Measured cost over a frozen 293-file corpus, 586 rows across both routes: 4 rows changed, and all four are this defect. Everything else is byte-identical to baseline — TypeScript 14.00%, Python 14.98%, every uncovered-language bucket 0.00%, all unchanged. The prose bucket goes 0.67% -> 0.00%, which was the loss. The new tests are verified to fail against the unfixed tracker (2 red), and the three that pin behaviour C1a must *not* change pass in both arms. Deferred: `filepath:` is still counted in `R_struct` (audit C1b). That is the deeper half — it is why `R_struct` is pinned at 1.0 for code and contributes a free 0.40, and therefore why a code file can lose 66.7% of its symbols and pass. It moves every published figure in the project and wants its own measurement pass. Also updates `tools/corpus-harness/recipe.json`: typescript 56 -> 57, prose 25 -> 28. Those buckets count this repository's own files, which grow; the harness refusing to run on the mismatch is it working as designed. See DECISIONS §37. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 30 +++++ DECISIONS.md | 73 ++++++++++++ src/core/ledger/drift-tracker.ts | 37 ++++++- test/unit/drift-unwitnessed-elision.test.ts | 116 ++++++++++++++++++++ tools/corpus-harness/recipe.json | 92 ++++++++++------ 5 files changed, 310 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea74886..6a5790e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,36 @@ Commits on `main` beyond the `v1.1.0` tag (`807f6f0`). Not yet tagged or release `git log v1.1.0..HEAD` to confirm current scope before relying on this list. ### Fixed +- **A document whose witnesses were all destroyed is no longer certified — audit C1a**: + `DriftTracker.findUnwitnessedItems` asked *did evidence exist before?* (it built its probe + from the **before** item), so an item whose witnesses were entirely destroyed was exempt on + the grounds that they had once been there. Measured on this repository's own files, on both + routes: `CODE_OF_CONDUCT.md` **3,542 → 72 bytes** and `SECURITY.md` **1,154 → 72 bytes**, at + `fallbackUsed: false`, `validation.passed: true`, both gates reporting `pass` — and + unrecoverable on the CLI, which supplies no `TokenHasher`. + + Neither gate could fire, and the arithmetic is closed-form: prose yields no symbols so + `R_AST = 1.0` as an empty-set default (a free 0.60), and `filepath:` is derived from + `item.path` and survives any content transform, so `R_struct = 1/(N+1)` for N headings. Hence + `S_k = 0.4·N/(N+1)`, which approaches 0.40 from below and never reaches it, against a gate + firing on `> 0.40`. The two stdin rows landed on **exactly 0.400** and were admitted by the + strict comparison. + + An item that changed is now refused when it yields no symbols **and** no content-derived + markers survive in the *after* item. Scoped to symbol-free items, so whole-item elision of + code still refuses as `SEMANTIC_DRIFT_EXCEEDED` (the accurate reason) rather than being + relabelled; and strictly additive, so every §33 refusal still refuses. + + Cost, over a frozen 293-file corpus (586 rows, both routes): **4 rows changed, all four this + defect.** Everything else byte-identical — TypeScript 14.00%, Python 14.98%, uncovered + buckets 0.00%, all unchanged. Prose goes 0.67% → 0.00%, which was the loss. Guarded by + `test/unit/drift-unwitnessed-elision.test.ts`, verified to fail against the unfixed tracker. + See DECISIONS §37. + + **Deferred:** `filepath:` is still counted in `R_struct` (audit C1b). That is why `R_struct` + is pinned at 1.0 for code, and why a code file can lose 66.7% of its symbols and pass. It + moves every published figure and wants its own measurement pass. + - **The regression baseline now measures the fixture set the product ships — audit H3**: `test/integration/bench.test.ts` Tests 1–5 loaded `loadBenchmarkFixtures('humaneval')`, and Test 2 did not use the shipped fixtures at all — it built a private two-fixture set inline diff --git a/DECISIONS.md b/DECISIONS.md index dd10393..6f897e8 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2172,3 +2172,76 @@ The MIT → MPL migration itself has no entry in this file. It was made in the R LICENSE and nowhere else, so nothing prompted a sweep of the other places the license is asserted. The lesson is narrow and worth keeping: a license is asserted in four files, and changing it in one is a change to none of the others. + +--- + +## 37. A Witness That Existed Before Does Not Count If None of It Survived + +**Date:** 2026-08-09 · **Status:** Accepted · **Closes:** max_audit.md C1 (measurement half) + +§33 widened the measurement gate from validator-covered items to every item, and was right to. +What it did not change was the **tense** of the question. `findUnwitnessedItems` asked *did +evidence exist before?* — it built its probe bundle from the *before* item — so an item whose +witnesses were all destroyed was exempt, on the grounds that they had once been there. + +A structured document therefore walked between the two gates that were split apart to catch +exactly this. Measured on this repository's own files, on both the file and stdin routes: + +| file | before | after | `fallbackUsed` | `S_k` | +|---|---|---|---|---| +| `CODE_OF_CONDUCT.md` | 3,542 B | **72 B** | `false` | 0.369 / **0.400** | +| `SECURITY.md` | 1,154 B | **72 B** | `false` | 0.333 / **0.400** | + +`validation.passed: true`, both gates `pass`, the content gone and — on the CLI, which supplies +no `TokenHasher` — unrecoverable. + +### Why neither gate fired + +The arithmetic is closed-form, which is what makes this a design defect rather than a tuning +miss. Prose yields no symbols, so `R_AST = 1.0` as an empty-set default and contributes a free +0.60. `collectMarkers` adds a `filepath:` marker derived from `item.path`, which no content +transform can destroy, so `R_struct = 1/(N+1)` for N headings. Therefore: + +``` +S_k = 0.6·0 + 0.4·(N/(N+1)) = 0.4·N/(N+1) +``` + +which approaches 0.40 from below and never reaches it, for any N — against a retention gate +that fires on `driftScore > 0.40`. **The retention gate cannot fire for markdown at all.** The +two stdin rows above landed on *exactly* 0.400 and were admitted by the strict `>`; that is the +supremum of the expression being waved through by the comparison, not a near miss. + +And the measurement gate exempted them because their headings had existed. + +### Decision + +An item that changed is refused when it yields **no symbols** and **no content-derived markers +survive in the after item**. Two properties make this safe: + +- **It is scoped to symbol-free items.** An item carrying symbols is left to the retention gate, + because `R_AST` is measuring it for real. Whole-item elision of code still refuses as + `SEMANTIC_DRIFT_EXCEEDED`, which is the accurate reason — reporting "unmeasurable" for an + item whose loss was measured exactly would restore the conflation the split undid. +- **It only ever adds refusals.** Refusing on the surviving set is strictly stronger than + refusing on the before set, so every §33 refusal still refuses. Nothing that was caught is + now let through. + +### Measured cost + +A frozen 293-file corpus, 586 rows across both routes: **4 rows changed, and all four are this +defect.** Every other row is byte-identical to baseline. TypeScript stays at 14.00%, Python at +14.98%, and every uncovered-language bucket stays at 0.00%. The prose bucket goes 0.67% → 0.00%, +which was the data loss. + +### Not done here + +`filepath:` is still counted in `R_struct` (audit C1b, §3.2). That is the deeper half: it is why +`R_struct` is pinned at 1.0 for code and contributes a free 0.40, which in turn is why a code +file can lose **66.7%** of its symbols and pass. Fixing it moves every published reduction figure +in the project and wants its own measurement pass, so it is deliberately deferred rather than +folded in here. C1a closes the data loss; C1b closes the arithmetic. + +`extractContentMarkers` remains the right primitive for both — its own doc comment has said since +§28 that metadata-derived markers "cannot serve as *evidence* that content was retained, because +they are preserved whether it was or not". The principle was already written down. This applies +it where the decision is made. diff --git a/src/core/ledger/drift-tracker.ts b/src/core/ledger/drift-tracker.ts index 1ec9dac..c48df31 100644 --- a/src/core/ledger/drift-tracker.ts +++ b/src/core/ledger/drift-tracker.ts @@ -314,10 +314,41 @@ export class DriftTracker { continue; // untouched: retention needs no evidence } - const single: ContextBundle = { ...beforeBundle, items: Object.freeze([item]) }; - if (this.extractSymbols(single).size > 0 || this.extractContentMarkers(single).size > 0) { - continue; // measurable: the ratios above already scored it + const beforeSingle: ContextBundle = { ...beforeBundle, items: Object.freeze([item]) }; + const afterSingle: ContextBundle = { ...beforeBundle, items: Object.freeze([after]) }; + + // Symbols present *before* means `R_AST` is measuring this item for real rather than + // reporting its empty-set default, so retention is evidenced and the retention gate owns + // the verdict. Whole-item elision of code lands here: `R_AST = 0`, `S_k` pins at 0.60, + // refused as EXCEEDED — which is the accurate reason. Claiming "unmeasurable" for an + // item whose loss was measured exactly would be the same conflation the two-gate split + // exists to undo. + if (this.extractSymbols(beforeSingle).size > 0) { + continue; } + + // No symbols, so 60% of `S_k` is a free 0.60 from an empty-set default that looked at + // nothing. Content markers are the only real evidence left — and they must **survive**. + // + // This asked `extractContentMarkers(beforeSingle)` until 2026-08-09, i.e. *did evidence + // exist?* rather than *did any of it survive?*. Measured, that let a markdown document be + // deleted in its entirety with every gate green: a 233-byte runbook became a 72-byte + // marker at `fallbackUsed: false`, `S_k = 0.3000`, both gates passing. The arithmetic is + // closed-form and worth stating, because it shows the old rule could never have caught + // it: with no symbols `R_AST = 1.0`, and `R_struct = 1/(N+1)` for N headings because + // `filepath:` is derived from `item.path` and no content transform can destroy it. So + // `S_k = 0.4·N/(N+1)`, which approaches 0.40 from below and never reaches it, for any N. + // The retention gate compares with strict `>`. It cannot fire for markdown at all. + // + // §33 widened this rule from validator-covered items to every item and was right to; + // what it did not change was the tense of the question. Refusing on the surviving set is + // strictly more refusing than refusing on the before set, so every §33 refusal (the + // symbol-free Perl file elided whole) still refuses — nothing that was caught is now let + // through. See DECISIONS §37 and max_audit.md C1. + if (this.extractContentMarkers(afterSingle).size > 0) { + continue; + } + unwitnessed.push(item.id); } return unwitnessed; diff --git a/test/unit/drift-unwitnessed-elision.test.ts b/test/unit/drift-unwitnessed-elision.test.ts index 0373dfd..ca9496b 100644 --- a/test/unit/drift-unwitnessed-elision.test.ts +++ b/test/unit/drift-unwitnessed-elision.test.ts @@ -292,4 +292,120 @@ describe('drift refuses to certify an elision it has no evidence for', () => { expect(result.validation.driftCoverage?.symbolBearingItems).toBe(1); }); }); + /** + * The surviving-witness rule — C1a, 2026-08-09. + * + * Until this test existed the rule above asked *did evidence exist before?* rather than + * *did any of it survive?*, and a structured document walked between the two gates. Measured + * on this repository's own files: `CODE_OF_CONDUCT.md` went **3,542 bytes to 72** and + * `SECURITY.md` **1,154 to 72**, on both the file and stdin routes, with + * `fallbackUsed: false`, `validation.passed: true`, and both gates reporting `pass`. + * + * The arithmetic shows the old rule could never have caught it. Prose yields no symbols, so + * `R_AST = 1.0` as an empty-set default and contributes a free 0.60. `collectMarkers` adds a + * `filepath:` marker derived from `item.path`, which no content transform can destroy, so + * `R_struct = 1/(N+1)` for N headings. Therefore `S_k = 0.4 * N/(N+1)` — strictly below 0.40 + * for every N, against a gate that fires on `driftScore > 0.40`. + * + * The stdin rows landed on **exactly 0.400** and passed on the strict `>`. That is not a + * near-miss: it is the supremum of the expression above being admitted by the comparison. + * Both are asserted below, because a later change to either the weights or the comparison + * operator should have to confront this case explicitly. + * + * Cost of the rule, measured over a frozen 293-file corpus (586 rows, both routes): **4 rows + * changed, all four of them this defect**. Every code bucket is byte-identical to baseline — + * TypeScript 14.00%, Python 14.98% unchanged. See DECISIONS §37 and max_audit.md C1. + */ + describe('a witness that existed before does not count if none of it survived', () => { + const DOC = [ + '# Runbook', + '', + 'Steps when the queue backs up:', + '', + '- Check the lag dashboard', + '- Scale the consumer group to 12 workers', + '', + '## Escalation', + '', + 'Page the platform on-call if lag exceeds 30 minutes.', + '', + ].join('\n'); + + const docItem = (content: string) => + createContextItem({ + id: 'doc', + kind: 'file', + content, + path: 'docs/runbook.md', + language: 'markdown', + contentType: 'markdown', + }); + + it('refuses a markdown document elided whole, though its headings witnessed it before', () => { + const before = bundleOf(docItem(DOC)); + const after = bundleOf(docItem('[TokenDamper: 11 markdown lines elided, 233 bytes, sha256:60429ebe370b]')); + + const report = tracker.calculateDrift(before, after); + + expect(report.unwitnessedItemIds).toEqual(['doc']); + expect(report.measurementGate).toBe('refuse'); + expect(report.shouldFallback).toBe(true); + + // The evidence existed before and is what made the old rule exempt this item. + expect(report.contentMarkersBeforeCount).toBeGreaterThan(0); + expect(report.astMeasured).toBe(false); + }); + + it('pins the arithmetic: S_k stays under 0.40 for any heading count, so retention alone cannot fire', () => { + for (const headings of [1, 3, 10, 50]) { + const body = Array.from({ length: headings }, (_, i) => `## Section ${i}\n\nBody text ${i}.\n`).join('\n'); + const before = bundleOf(docItem(body)); + const after = bundleOf(docItem('[TokenDamper: elided, sha256:deadbeefcafe]')); + + const report = tracker.calculateDrift(before, after); + + // The supremum is 0.40 and it is never exceeded — `>` cannot fire, for any N. + expect(report.driftScore).toBeLessThanOrEqual(0.4 + 1e-9); + expect(report.retentionGate).toBe('pass'); + + // Which is exactly why the measurement gate has to be the one that refuses. + expect(report.measurementGate).toBe('refuse'); + expect(report.shouldFallback).toBe(true); + } + }); + + it('leaves an item whose witnesses survived alone — the rule adds refusals, it does not invert', () => { + const before = bundleOf(docItem(DOC)); + const after = bundleOf(docItem('# Runbook\n\n## Escalation\n\n[TokenDamper: 6 markdown lines elided]')); + + const report = tracker.calculateDrift(before, after); + + expect(report.unwitnessedItemIds).toEqual([]); + expect(report.measurementGate).toBe('pass'); + }); + + it('leaves symbol-bearing code to the retention gate, so its refusal reason stays accurate', () => { + const codeItem = (content: string) => + createContextItem({ + id: 'code', + kind: 'file', + content, + path: 'src/thing.ts', + language: 'typescript', + contentType: 'code', + }); + + const before = bundleOf(codeItem('export function alpha() {\n return 1;\n}\n')); + const after = bundleOf(codeItem('[TokenDamper: 3 code lines elided, sha256:0123456789ab]')); + + const report = tracker.calculateDrift(before, after); + + // Symbols existed, so `R_AST` measured the loss exactly rather than defaulting. Calling + // that "unmeasurable" would reintroduce the conflation the two-gate split undid. + expect(report.astMeasured).toBe(true); + expect(report.measurementGate).toBe('pass'); + expect(report.retentionGate).toBe('refuse'); + expect(report.shouldFallback).toBe(true); + }); + }); }); diff --git a/tools/corpus-harness/recipe.json b/tools/corpus-harness/recipe.json index 8618458..191a086 100644 --- a/tools/corpus-harness/recipe.json +++ b/tools/corpus-harness/recipe.json @@ -1,65 +1,76 @@ { - "$comment": [ - "Declarative corpus spec. `collect.js` reads this, walks each root recursively, and", - "selects deterministically (sort by relative path, take the first `limit`).", - "", - "Roots are machine-specific by nature — this corpus is third-party source found on a", - "developer machine, not vendored bytes. That is deliberate: vendoring GPL/BSD source into", - "this repo to measure a classifier would be a licensing problem for a measurement.", - "The recipe is the reproducible artifact; `manifest.json` records what a given machine", - "actually produced, so two runs on the same machine are comparable and a run on a", - "different machine is visibly a different corpus rather than silently one.", - "", - "`expect` is asserted after selection. It exists because the 4b.3 A/B loop globbed one", - "directory level and measured 132 of 144 files without noticing (see", - "docs/phase-4b-lever-disposition.md finding 3). A count that is not asserted is a count", - "that will be wrong eventually." - ], + "$comment": "Declarative corpus spec. `collect.js` reads this, walks each root recursively, and,selects deterministically (sort by relative path, take the first `limit`).,,Roots are machine-specific by nature — this corpus is third-party source found on a,developer machine, not vendored bytes. That is deliberate: vendoring GPL/BSD source into,this repo to measure a classifier would be a licensing problem for a measurement.,The recipe is the reproducible artifact; `manifest.json` records what a given machine,actually produced, so two runs on the same machine are comparable and a run on a,different machine is visibly a different corpus rather than silently one.,,`expect` is asserted after selection. It exists because the 4b.3 A/B loop globbed one,directory level and measured 132 of 144 files without noticing (see,docs/phase-4b-lever-disposition.md finding 3). A count that is not asserted is a count,that will be wrong eventually. | 2026-08-09: typescript 56->57 and prose 25->28. These count this repository's own files, which grow; the harness refusing on the mismatch is it working. prose gained max_audit.md, tools/corpus-harness/README.md and tokendamper-benchmark/CLAUDE.md.", "buckets": [ { "name": "shell", - "roots": ["C:/msys64", "C:/Program Files/Git"], - "extensions": ["sh"], + "roots": [ + "C:/msys64", + "C:/Program Files/Git" + ], + "extensions": [ + "sh" + ], "limit": 40, "expect": 40, "why": "extension IS in isCodeExtension -> `code` on the file route, `markdown` over stdin" }, { "name": "perl", - "roots": ["C:/msys64"], - "extensions": ["pl"], + "roots": [ + "C:/msys64" + ], + "extensions": [ + "pl" + ], "limit": 40, "expect": 40, "why": "extension NOT in isCodeExtension -> `markdown` on BOTH routes" }, { "name": "tcl", - "roots": ["C:/msys64"], - "extensions": ["tcl"], + "roots": [ + "C:/msys64" + ], + "extensions": [ + "tcl" + ], "limit": 40, "expect": 40, "why": "extension NOT in isCodeExtension -> `markdown` on BOTH routes" }, { "name": "c", - "roots": ["C:/msys64/ucrt64/include"], - "extensions": ["c", "h"], + "roots": [ + "C:/msys64/ucrt64/include" + ], + "extensions": [ + "c", + "h" + ], "limit": 30, "expect": 30, "why": "isCodeExtension -> `code` -> handed to the TypeScript validator" }, { "name": "rust", - "roots": ["C:/msys64/usr/share/git"], - "extensions": ["rs"], + "roots": [ + "C:/msys64/usr/share/git" + ], + "extensions": [ + "rs" + ], "limit": 40, "expect": 3, "why": "isCodeExtension; only three real .rs files exist on this machine" }, { "name": "css", - "roots": ["C:/msys64"], - "extensions": ["css"], + "roots": [ + "C:/msys64" + ], + "extensions": [ + "css" + ], "limit": 15, "expect": 10, "why": "isCodeExtension; brace-bearing but symbol-free under extractSymbols" @@ -69,25 +80,36 @@ "roots": [ "C:/Users/ojass/AppData/Local/Programs/Python/Python312/Lib/site-packages/pip/_internal" ], - "extensions": ["py"], + "extensions": [ + "py" + ], "limit": 45, "expect": 45, "why": "AST-lite COVERS this - the positive control" }, { "name": "typescript", - "roots": ["src"], - "extensions": ["ts"], + "roots": [ + "src" + ], + "extensions": [ + "ts" + ], "limit": 64, - "expect": 56, + "expect": 57, "why": "AST-lite COVERS this - the positive control, and the repo's own corpus. NOTE: 56, not the 64 quoted in docs/phase-4b-*.md - 8 of the 64 sources are under minBytes and are excluded here. Aggregates are not directly comparable to those documents." }, { "name": "prose", - "roots": [".", "docs"], - "extensions": ["md"], + "roots": [ + ".", + "docs" + ], + "extensions": [ + "md" + ], "limit": 40, - "expect": 25, + "expect": 28, "why": "real markdown - the negative set any looksLikeMarkdown change must not break. `.agents` is excluded below: it holds prior agent scratchpads, and because sort-then-take is alphabetical they filled all 40 slots on the first run and pushed out every hand-written document. Deterministic is not the same as representative." } ], From 99672af558827e3cd28e630e26bdd4146896f886 Mon Sep 17 00:00:00 2001 From: ojassug Date: Sun, 9 Aug 2026 21:20:00 +0530 Subject: [PATCH 2/3] fix(gateway): read the request body as bytes, not as string fragments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes audit C2 and L3. `GatewayServer.onRequest` accumulated its body with `body += chunk`, which calls `Buffer.prototype.toString('utf8')` on each chunk *independently*. A multi-byte UTF-8 sequence straddling a chunk boundary is therefore decoded as two truncated fragments and becomes U+FFFD on both sides — at the socket, before the pipeline exists, on the bytes that are then forwarded to the provider. Node reads in ~64 KB chunks, so this fires by chance on any body large enough to be chunked and deterministically for a body split at the wrong offset. Measured against the unfixed server, each body written in two `req.write()` calls split on a UTF-8 continuation byte: héllo — ünïcode ✓ 日本語 😀 94B sent, 98B forwarded as `h??llo …` こんにちは世界 76B sent, 82B forwarded ┌─┐│ build ok │└─┘ 89B sent, 95B forwarded A corrupted body is always longer than it was sent, because U+FFFD re-encodes to three bytes. Nothing was elided on any of these turns. This is DECISIONS §35 at a different seam. Phase B's reasoning — "rawInput is a decoded string, so the evidence is gone by the time a request exists" — is correct and generalizes; it was applied to the adapter that reads from disk and not to the one that reads from a socket, where it is worse, because the bytes reach a provider rather than a terminal. MCP was never affected, instructively: `setEncoding('utf8')` installs a `StringDecoder`, which holds partial sequences across chunk boundaries. Manual concatenation is exactly what bypasses that. The fix collects `Buffer[]`, concatenates on `end`, and decodes once. Then it applies the CLI's own round-trip test and, when that fails, forwards the caller's bytes untouched — concatenating correctly does not make a body that was never valid UTF-8 representable, so that is a separate question with a separate answer. Optimizing such a body is not an option, because every stage, validator and token estimate operates on the decoded string and would be reasoning about content the caller never sent. Rejecting it is not an option either: a body the provider might well accept is not a transparent proxy's to refuse. `ProxyRequestResult` gains an optional `bodyBytes`, preferred over `body` by both the upstream `fetch` and the locally-returned branch of `writeProxyResult`. Also removes the O(n²) body-size check, which recomputed `Buffer.byteLength(body, 'utf8')` over the whole accumulated string on every chunk (L3). A running total falls out of collecting buffers anyway. The new tests choose their split offsets programmatically to land on a continuation byte, so they test the actual hazard rather than an offset that might be character-aligned, and are verified to fail 4/4 against the unfixed server. Untouched and independent: the `exec` token handoff (C3), the 0-bytes-saved measurement (H1), structured content flattened to a string (C4), and the two environment branches in the request path (M8). This is a correctness fix to the pass-through, not a claim that the mode is finished. See DECISIONS §38. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 25 ++++ DECISIONS.md | 66 +++++++++ src/gateway/proxy.ts | 58 +++++++- src/gateway/server.ts | 44 +++++- src/gateway/types.ts | 22 +++ .../integration/gateway-byte-fidelity.test.ts | 130 ++++++++++++++++++ 6 files changed, 336 insertions(+), 9 deletions(-) create mode 100644 test/integration/gateway-byte-fidelity.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a5790e..a74be2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,31 @@ Commits on `main` beyond the `v1.1.0` tag (`807f6f0`). Not yet tagged or release `git log v1.1.0..HEAD` to confirm current scope before relying on this list. ### Fixed +- **The Gateway no longer corrupts non-ASCII request bodies — audit C2, L3**: + `GatewayServer.onRequest` accumulated the body with `body += chunk`, decoding each chunk + independently, so a multi-byte UTF-8 sequence straddling a chunk boundary became U+FFFD on + both sides — at the socket, before the pipeline exists, on the bytes forwarded upstream. + Measured with two-write splits on a continuation byte: `héllo — ünïcode ✓ 日本語 😀` went out + 94 B and forwarded as **98 B** (`h��llo …`); CJK 76 → **82 B**; box-drawing 89 → **95 B**. A + corrupted body is always *longer*, because U+FFFD re-encodes to three bytes. + + Now collects `Buffer[]`, concatenates on `end`, and decodes once. Bodies that fail a UTF-8 + round trip — which correct concatenation does not fix, since the decode is still lossy — are + forwarded verbatim rather than optimized or rejected: optimizing would have every stage + reasoning about content the caller never sent, and rejecting is not a transparent proxy's + call. `ProxyRequestResult` gains `bodyBytes` for this, preferred over `body` by both the + upstream `fetch` and `writeProxyResult`. + + This is DECISIONS §35 at a different seam. Phase B applied that reasoning to the adapter that + reads from disk and not to the one that reads from a socket, where it is worse — the bytes + reach a provider, not a terminal. MCP was never affected: `setEncoding('utf8')` installs a + `StringDecoder`, which holds partial sequences across chunks; manual concatenation is exactly + what bypasses it. + + Also removes the O(n²) body-size check, which re-measured the whole accumulated string on + every chunk (L3). Guarded by `test/integration/gateway-byte-fidelity.test.ts`, verified to + fail 4/4 against the unfixed server. See DECISIONS §38. + - **A document whose witnesses were all destroyed is no longer certified — audit C1a**: `DriftTracker.findUnwitnessedItems` asked *did evidence exist before?* (it built its probe from the **before** item), so an item whose witnesses were entirely destroyed was exempt on diff --git a/DECISIONS.md b/DECISIONS.md index 6f897e8..77049cd 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2245,3 +2245,69 @@ folded in here. C1a closes the data loss; C1b closes the arithmetic. §28 that metadata-derived markers "cannot serve as *evidence* that content was retained, because they are preserved whether it was or not". The principle was already written down. This applies it where the decision is made. + +--- + +## 38. The Gateway Reads Bytes, Not String Fragments + +**Date:** 2026-08-09 · **Status:** Accepted · **Closes:** max_audit.md C2, L3 + +`GatewayServer.onRequest` accumulated its request body with `body += chunk`. That invokes +`Buffer.prototype.toString('utf8')` on **each chunk independently**, so a multi-byte UTF-8 +sequence straddling a chunk boundary is decoded as two truncated fragments and becomes U+FFFD on +both sides. Node reads in ~64 KB chunks, so this fires by chance on any body large enough to be +chunked, and deterministically for a body split at the wrong offset. + +Measured against the unfixed server, with each body written in two `req.write()` calls split on +a UTF-8 continuation byte: + +| body | sent | forwarded | +|---|---|---| +| `héllo — ünïcode ✓ 日本語 😀` | 94 B | **98 B**, `h��llo …` | +| `こんにちは世界` | 76 B | **82 B**, `���んにちは世界` | +| `┌─┐│ build ok │└─┘` | 89 B | **95 B**, `���─┐│ build ok │└─┘` | + +A corrupted body is always *longer* than it was sent, because U+FFFD re-encodes to three bytes. +Nothing was elided on any of these turns — the corruption happens at the socket, before the +pipeline exists, and the corrupted string is what goes upstream. + +### This is DECISIONS §35 at a different seam + +Phase B's reasoning — *"`rawInput` is a decoded string, so the evidence is gone by the time a +request exists"* — is correct and generalizes. It was applied to the one adapter that reads from +disk and not to the one that reads from a socket, where it is worse: the bytes reach a provider +rather than a terminal. The MCP transport is unaffected, and instructively so: `setEncoding('utf8')` +installs a `StringDecoder`, which holds partial sequences across chunk boundaries. Manual +concatenation is exactly what bypasses that machinery. + +### Decision + +Collect `Buffer[]`, `Buffer.concat` on `end`, decode **once**. Then apply the CLI's own round-trip +test (`Buffer.from(str, 'utf8').equals(buf)`) and, when it fails, pass the caller's bytes through +untouched. + +Concatenating correctly fixes the chunk-boundary defect. It does not make a body that was never +valid UTF-8 representable — the decode is still lossy — so the round trip is a separate question +and gets a separate answer. Optimizing such a body is not an option, because every stage, +validator and token estimate operates on the decoded string and would be reasoning about content +the caller never sent; a saving measured against corrupted input is worse than none. Rejecting it +is not an option either: TokenDamper is a transparent proxy, and a body the provider might well +accept is not TokenDamper's to refuse. So it is forwarded verbatim, which is invariant 3 on the +Gateway. + +`ProxyRequestResult` gains an optional `bodyBytes`; `body` is still populated with the lossy +decode so existing readers keep working, but anything that puts bytes on the wire prefers +`bodyBytes`. Both the upstream `fetch` and the locally-returned branch in `writeProxyResult` do. + +### Also fixed + +The body-size cap recomputed `Buffer.byteLength(body, 'utf8')` over the entire accumulated string +on every chunk — O(n²) in the length of the request (audit L3). It is now a running total, which +falls out of collecting buffers anyway. + +### Not done here + +The remaining Gateway findings are untouched and independent: the `exec` token handoff (C3), the +0-bytes-saved measurement (H1), structured message content flattened to a string (C4), and the +two environment branches in the request path (M8). C2 is a correctness fix to the pass-through, +not an argument that the mode is finished. diff --git a/src/gateway/proxy.ts b/src/gateway/proxy.ts index 9a4af94..6e668bb 100644 --- a/src/gateway/proxy.ts +++ b/src/gateway/proxy.ts @@ -21,7 +21,7 @@ import { estimateBundleTokens } from '../core/hashing/tokenizer'; import { ConfidenceLedger } from '../core/ledger/confidence-ledger'; import { TOKENDAMPER_VERSION } from '../version'; import { GatewaySessionStore } from './session-store'; -import type { AnthropicMessagesPayload, OpenAiChatPayload, ProxyHandlerOptions, ProxyRequestResult, SessionContentEntry } from './types'; +import type { AnthropicMessagesPayload, GatewaySession, OpenAiChatPayload, ProxyHandlerOptions, ProxyRequestResult, SessionContentEntry } from './types'; /** * Handles incoming API requests, normalizing payloads, running cross-turn deduplication, @@ -39,6 +39,16 @@ export async function handleProxyRequest( const sessionId = getSessionIdFromHeaders(headers, rawBody); const session = options.sessionStore.getOrCreateSession(sessionId); + // Can the string model represent what the caller actually sent? + // + // A round trip, not a charset sniff — the only question that matters is whether these exact + // bytes survive the representation everything downstream is built on. Identical reasoning to + // the CLI's `inputSurvivesDecoding` (DECISIONS §35); the difference is that here an + // unfaithful decode is not merely printed, it is forwarded to a provider as if the caller had + // sent it. + const bodyBytes = options.rawBodyBytes; + const bodyIsLossless = bodyBytes === undefined || Buffer.from(rawBody, 'utf8').equals(bodyBytes); + const cleanHeaders: Record = {}; for (const [key, val] of Object.entries(headers)) { if (val !== undefined && key.toLowerCase() !== 'host' && key.toLowerCase() !== 'content-length') { @@ -57,7 +67,9 @@ export async function handleProxyRequest( // Handle OpenAI API endpoint if (routePath === '/v1/chat/completions') { - const optimized = processOpenAiRequest(rawBody, cleanHeaders, session, options); + const optimized = bodyIsLossless + ? processOpenAiRequest(rawBody, cleanHeaders, session, options) + : passThroughUnrepresentable(bodyBytes as Buffer, rawBody, cleanHeaders, session); if (optimized.statusCode !== 200 || shouldUseMockUpstream()) { return optimized; } @@ -77,6 +89,7 @@ export async function handleProxyRequest( provider: 'openai', requestUrl, body: optimized.body, + ...(optimized.bodyBytes ? { bodyBytes: optimized.bodyBytes } : {}), incomingHeaders: cleanHeaders, streamRequested: isStreamRequested(optimized.body), session: optimized.session, @@ -86,7 +99,9 @@ export async function handleProxyRequest( // Handle Anthropic API endpoint if (routePath === '/v1/messages') { - const optimized = processAnthropicRequest(rawBody, cleanHeaders, session, options); + const optimized = bodyIsLossless + ? processAnthropicRequest(rawBody, cleanHeaders, session, options) + : passThroughUnrepresentable(bodyBytes as Buffer, rawBody, cleanHeaders, session); if (optimized.statusCode !== 200 || shouldUseMockUpstream()) { return optimized; } @@ -106,6 +121,7 @@ export async function handleProxyRequest( provider: 'anthropic', requestUrl, body: optimized.body, + ...(optimized.bodyBytes ? { bodyBytes: optimized.bodyBytes } : {}), incomingHeaders: cleanHeaders, streamRequested: isStreamRequested(optimized.body), session: optimized.session, @@ -128,6 +144,8 @@ interface ForwardUpstreamOptions { readonly provider: UpstreamProvider; readonly requestUrl: URL; readonly body: string; + /** Preferred over `body` when set — see `ProxyRequestResult.bodyBytes`. */ + readonly bodyBytes?: Buffer | undefined; readonly incomingHeaders: Record; readonly streamRequested: boolean; readonly session: ReturnType; @@ -151,7 +169,12 @@ async function forwardUpstreamRequest(params: ForwardUpstreamOptions): Promise

, + session: GatewaySession, +): ProxyRequestResult { + return { + statusCode: 200, + headers, + body: lossyBody, + bodyBytes: bytes, + session, + }; +} + interface GatewayOptimizationOutcome { readonly finalBundle: ContextBundle; readonly fallbackUsed: boolean; diff --git a/src/gateway/server.ts b/src/gateway/server.ts index e72d70c..4679058 100644 --- a/src/gateway/server.ts +++ b/src/gateway/server.ts @@ -99,21 +99,47 @@ export class GatewayServer { } } - let body = ''; + // Collect bytes, not string fragments. + // + // This was `body += chunk`, which invokes `Buffer.prototype.toString('utf8')` on each chunk + // *independently*. A multi-byte UTF-8 sequence straddling a chunk boundary is therefore + // decoded as two truncated fragments and becomes U+FFFD on both sides — silently, before + // the pipeline exists, on the bytes that then get forwarded to the provider. Node reads in + // ~64 KB chunks, so this fires by chance on any body large enough to be chunked, and + // deterministically for a body split at the wrong offset. Reproduced with an 89-byte body + // written in two `req.write()` calls split inside `é`. + // + // This is the same defect Phase B fixed in the CLI (DECISIONS §35) — "`rawInput` is a + // *decoded string*, so the evidence is gone by the time a request exists" — arriving at the + // socket instead of at `readFileSync`. It is worse here, because the corrupted bytes are + // sent upstream rather than printed to a terminal. Note the MCP transport is *not* affected: + // `setEncoding('utf8')` installs a `StringDecoder`, which holds partial sequences across + // chunk boundaries. Manual concatenation is precisely what bypasses that. + const chunks: Buffer[] = []; + let receivedBytes = 0; const MAX_BODY_BYTES = 10 * 1024 * 1024; - req.on('data', (chunk) => { - body += chunk; - if (Buffer.byteLength(body, 'utf8') > MAX_BODY_BYTES) { + req.on('data', (chunk: Buffer | string) => { + const buf = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk, 'utf8'); + // Running total rather than re-measuring the accumulated body on every chunk, which was + // O(n²) over the length of the request (audit L3). + receivedBytes += buf.length; + if (receivedBytes > MAX_BODY_BYTES) { res.writeHead(413, { 'content-type': 'application/json' }); res.end(JSON.stringify({ error: 'Payload Too Large' })); req.destroy(); + return; } + chunks.push(buf); }); req.on('end', async () => { if (req.destroyed) return; + // Concatenate first, decode exactly once. + const rawBuffer = Buffer.concat(chunks); + const body = rawBuffer.toString('utf8'); + const abortController = new AbortController(); res.on('close', () => { if (!res.writableEnded) { @@ -127,6 +153,11 @@ export class GatewayServer { upstreamOpenAiUrl: this.config.upstreamOpenAiUrl, upstreamAnthropicUrl: this.config.upstreamAnthropicUrl, abortSignal: abortController.signal, + // The bytes as received, so the proxy can tell whether the string above is a faithful + // representation of them. Concatenating correctly removes the chunk-boundary defect; + // it does not make a body that was never valid UTF-8 representable. The CLI applies + // the same round-trip test for the same reason (`main.ts`, `inputSurvivesDecoding`). + rawBodyBytes: rawBuffer, }); await this.writeProxyResult(res, result); @@ -142,7 +173,10 @@ export class GatewayServer { res.writeHead(result.statusCode, result.headers); if (!result.upstreamBody) { - res.end(result.body); + // Prefer the bytes when the result carries them. This is the locally-returned branch — + // mock upstream, and the `NODE_ENV === 'test'` no-credentials path — where writing + // `result.body` would re-encode the lossy decode and undo the pass-through. + res.end(result.bodyBytes ?? result.body); return; } diff --git a/src/gateway/types.ts b/src/gateway/types.ts index e865e56..0d195e4 100644 --- a/src/gateway/types.ts +++ b/src/gateway/types.ts @@ -61,6 +61,20 @@ export interface ProxyHandlerOptions { readonly upstreamOpenAiUrl?: string | undefined; readonly upstreamAnthropicUrl?: string | undefined; readonly abortSignal?: AbortSignal | undefined; + /** + * The request body exactly as it arrived on the socket. + * + * The pipeline is string-based, so `rawBody` is a decode of these bytes. When the decode is + * not faithful — the body was not valid UTF-8 — every stage, validator and token estimate + * downstream would be reasoning about content the caller never sent, and the re-encoded + * result is what gets forwarded to the provider. Supplying the bytes lets the proxy detect + * that and pass the original through untouched. + * + * Optional because in-process callers (tests, and `processOpenAiRequest` used directly) + * legitimately have only a string; absent, the body is trusted as-is, which is what the + * behaviour was before. + */ + readonly rawBodyBytes?: Buffer | undefined; } export interface GatewaySessionStoreInterface { @@ -82,6 +96,14 @@ export interface ProxyRequestResult { readonly statusCode: number; readonly headers: Record; readonly body: string; + /** + * Bytes to forward upstream in place of `body`, when the two are not interchangeable. + * + * Set only on the pass-through path for a body that does not survive a UTF-8 round trip. + * `body` is still populated (lossily) so existing readers keep working; anything that + * actually puts bytes on the wire must prefer this when present. + */ + readonly bodyBytes?: Buffer | undefined; readonly upstreamBody?: ReadableStream | null | undefined; readonly session: GatewaySession; readonly optimizationResult?: OptimizationResult | undefined; diff --git a/test/integration/gateway-byte-fidelity.test.ts b/test/integration/gateway-byte-fidelity.test.ts new file mode 100644 index 0000000..57345b2 --- /dev/null +++ b/test/integration/gateway-byte-fidelity.test.ts @@ -0,0 +1,130 @@ +import { request as httpRequest } from 'node:http'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { GatewayServer } from '../../src/gateway/server'; + +/** + * Byte fidelity on the Gateway — audit C2, invariant 3 on the only path that carries live + * provider traffic. + * + * The server accumulated its body with `body += chunk`, which calls `toString('utf8')` on each + * chunk *independently*. A multi-byte UTF-8 sequence straddling a chunk boundary is decoded as + * two truncated fragments and becomes U+FFFD on both sides — before the pipeline exists, on the + * bytes that are then forwarded upstream. Node reads in ~64 KB chunks, so this fires by chance + * on any body large enough to be chunked and deterministically for a body split at the wrong + * offset. Measured against the unfixed server, every case below came back longer than it went + * out: 94 -> 98, 76 -> 82, 89 -> 95 bytes. + * + * This is the defect Phase B closed in the CLI (DECISIONS §35) arriving at the socket instead of + * at `readFileSync`, and it is worse here because the corrupted bytes reach a provider rather + * than a terminal. Note the MCP transport is *not* affected: `setEncoding('utf8')` installs a + * `StringDecoder`, which holds partial sequences across chunk boundaries. Manual concatenation + * is precisely what bypasses that. + * + * The splits below are chosen programmatically to land on a UTF-8 continuation byte, so this + * tests the actual hazard rather than an arbitrary offset that might be character-aligned. + */ +describe('the Gateway forwards the caller bytes it was given', () => { + let server: GatewayServer; + let port: number; + let priorMock: string | undefined; + + beforeAll(async () => { + // Mock upstream echoes the outgoing request body back as the response, which is what makes + // "what would have been forwarded" observable without a real provider. + priorMock = process.env.TOKENDAMPER_MOCK_UPSTREAM; + process.env.TOKENDAMPER_MOCK_UPSTREAM = 'true'; + server = new GatewayServer({ port: 0 }); + await server.start(); + const boundPort = server.port; + expect(boundPort).toBeTypeOf('number'); + port = boundPort as number; + }); + + afterAll(async () => { + await server.stop(); + if (priorMock === undefined) delete process.env.TOKENDAMPER_MOCK_UPSTREAM; + else process.env.TOKENDAMPER_MOCK_UPSTREAM = priorMock; + }); + + /** POSTs `body` as two separate TCP writes split at `splitAt`. */ + const postSplit = (body: Buffer, splitAt: number): Promise<{ status: number; body: Buffer }> => + new Promise((resolve, reject) => { + const req = httpRequest( + { + host: '127.0.0.1', + port, + path: '/v1/messages', + method: 'POST', + headers: { + 'content-type': 'application/json', + 'x-api-key': 'sk-test', + 'content-length': body.length, + }, + }, + (res) => { + const chunks: Buffer[] = []; + res.on('data', (c: Buffer) => chunks.push(c)); + res.on('end', () => resolve({ status: res.statusCode ?? 0, body: Buffer.concat(chunks) })); + }, + ); + req.on('error', reject); + req.write(body.subarray(0, splitAt)); + setTimeout(() => req.end(body.subarray(splitAt)), 20); + }); + + /** First offset that is a UTF-8 continuation byte (10xxxxxx), i.e. inside a character. */ + const firstContinuationByte = (buf: Buffer): number => { + for (let i = 1; i < buf.length; i++) { + const byte = buf[i]; + if (byte !== undefined && (byte & 0xc0) === 0x80) return i; + } + return -1; + }; + + const cases: ReadonlyArray = [ + ['accented, em-dash, CJK and emoji', 'héllo — ünïcode ✓ 日本語 😀'], + ['CJK only', 'こんにちは世界'], + ['box drawing, as in captured terminal output', '┌─┐│ build ok │└─┘'], + ]; + + for (const [label, text] of cases) { + it(`preserves ${label} across a chunk boundary inside a character`, async () => { + const body = Buffer.from( + JSON.stringify({ model: 'm', messages: [{ role: 'user', content: text }] }), + 'utf8', + ); + const splitAt = firstContinuationByte(body); + + // Invariant 10: if the split were not inside a multi-byte character this test would pass + // against the unfixed server too, and assert nothing. + expect(splitAt).toBeGreaterThan(0); + expect(body[splitAt]! & 0xc0).toBe(0x80); + + const { status, body: got } = await postSplit(body, splitAt); + + expect(status).toBe(200); + expect(got.equals(body)).toBe(true); + // Stated separately because it is the symptom that shows up in production: U+FFFD + // re-encodes to three bytes, so a corrupted body is always *longer* than it was sent. + expect(got.length).toBe(body.length); + expect(got.toString('utf8')).not.toContain('�'); + }); + } + + it('forwards a body that is not valid UTF-8 at all, rather than re-encoding it', async () => { + // Latin-1 bytes inside a JSON string: 0xE9 is `é` in Latin-1 and an illegal lone lead byte + // in UTF-8. Concatenating chunks correctly does not make this representable — the decode is + // still lossy — so the round-trip check has to catch it and pass the original through. + const body = Buffer.concat([ + Buffer.from('{"model":"m","messages":[{"role":"user","content":"caf', 'utf8'), + Buffer.from([0xe9]), + Buffer.from('"}]}', 'utf8'), + ]); + expect(Buffer.from(body.toString('utf8'), 'utf8').equals(body)).toBe(false); + + const { status, body: got } = await postSplit(body, 20); + + expect(status).toBe(200); + expect(got.equals(body)).toBe(true); + }); +}); From b59722a914e322cbf3b799b4cf16626cb94ad81c Mon Sep 17 00:00:00 2001 From: ojassug Date: Sun, 9 Aug 2026 21:27:46 +0530 Subject: [PATCH 3/3] fix(trace): carry what the stages computed, and stop claiming items fit a budget they exceed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes audit M6. `buildTrace` projected every `StageResult` down to `{ stageId, status, durationMs: 0, changed }`. The stage's `metrics` and `notes` were discarded and the duration was a literal constant. So the trace could say that `compression:token-hashing` ran and changed something, and nothing about what it removed, how much, whether any elision was reversible, or how long it took — for a product whose stated differentiator is auditability. `--diff` and `--diff-html` partially compensate on the CLI; the MCP `get_optimization_trace` tool and the Gateway had nothing else. `StageTrace` now carries `metrics` and an optional `notes` verbatim. A CLI trace shows `regionsHashed: 4`, `bytesSaved: 14509`, `irreversibleElisions: 1` and the note explaining that no token hasher was supplied so the removed content is retained nowhere. `durationMs` is measured by the engine rather than by the stage: a stage that read a clock would stop being a pure function of its input (invariant 1), whereas timing an opaque call from the outside is an observation about the stage and cannot change what it returns. `performance.now()` rather than `Date.now()`, because most stages finish inside a millisecond and integer resolution would report the same uninformative 0 the constant already did. The trace was already non-deterministic — it carries a UUID `requestId` — so this changes nothing about invariant 1, which is a statement about emitted bytes. The pruner's note was not vague, it was false. `pruning:topology-pruner` returned "All items fit within token budget; no pruning required." unconditionally whenever `itemsPruned === 0`. Measured, a 5,405-token file at `maxInputTokens: 10` reported that all items fit. The mechanism is also H5: `applyCacheAwarePrefixLocking` pins every item inside the first 1,024 tokens, `solve01Knapsack` places pinned items outside the candidate set and always selects them, and `createContextBundle` produces a one-item bundle for CLI, MCP and bench — so item 0 is always pinned and `itemsPruned` is always 0. The note announced that pruning was *unnecessary* for the case where it was *impossible*. It now distinguishes the three cases and names the mechanism, and the metrics carry `bundleTokens` and `maxTokens` so the claim is checkable rather than asserted. This does not fix H5 — the knapsack remains unreachable on every shipping path. It stops the trace concealing it behind a reassuring sentence, which is the necessary first step: the defect is now visible where a user would look. New tests verified to fail 4/4 against the unfixed trace. See DECISIONS §39. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 21 +++++ DECISIONS.md | 54 +++++++++++++ src/core/engine/index.ts | 13 +++ src/core/model/types.ts | 18 +++++ src/core/trace/index.ts | 19 ++++- src/stages/pruning/topology-pruner.ts | 27 ++++++- test/unit/trace-explains.test.ts | 111 ++++++++++++++++++++++++++ 7 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 test/unit/trace-explains.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a74be2d..da034fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,27 @@ Commits on `main` beyond the `v1.1.0` tag (`807f6f0`). Not yet tagged or release `git log v1.1.0..HEAD` to confirm current scope before relying on this list. ### Fixed +- **The explainability trace now explains — audit M6**: `buildTrace` projected every + `StageResult` down to `{ stageId, status, durationMs: 0, changed }`, discarding each stage's + `metrics` and `notes` and hardcoding the duration. `StageTrace` now carries `metrics` and + `notes` verbatim, and `durationMs` is measured by the engine with `performance.now()` — by + the engine because a stage that read a clock would stop being a pure function of its input + (invariant 1), and `performance.now()` because most stages finish inside a millisecond and + integer resolution would report the same uninformative `0` the constant already did. + + A CLI trace now shows, for example, `regionsHashed: 4`, `bytesSaved: 14509` and + `irreversibleElisions: 1` with the note explaining that no token hasher was supplied so the + removed content is retained nowhere — none of which was previously knowable from the trace. + + **`pruning:topology-pruner`'s note was not vague, it was false.** It returned "All items fit + within token budget; no pruning required." unconditionally whenever `itemsPruned === 0`; + measured, a 5,405-token file at `maxInputTokens: 10` reported that all items fit. The note now + distinguishes the three cases and names the mechanism (everything pinned by cache-prefix + locking → pinned items bypass the knapsack → the budget could not be enforced), and the + metrics carry `bundleTokens` and `maxTokens` so the claim is checkable. This does not fix H5, + but it stops the trace concealing it. Guarded by `test/unit/trace-explains.test.ts`, verified + to fail 4/4 against the unfixed trace. See DECISIONS §39. + - **The Gateway no longer corrupts non-ASCII request bodies — audit C2, L3**: `GatewayServer.onRequest` accumulated the body with `body += chunk`, decoding each chunk independently, so a multi-byte UTF-8 sequence straddling a chunk boundary became U+FFFD on diff --git a/DECISIONS.md b/DECISIONS.md index 77049cd..ac02904 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -2311,3 +2311,57 @@ The remaining Gateway findings are untouched and independent: the `exec` token h 0-bytes-saved measurement (H1), structured message content flattened to a string (C4), and the two environment branches in the request path (M8). C2 is a correctness fix to the pass-through, not an argument that the mode is finished. + +--- + +## 39. The Trace Carries What the Stages Computed + +**Date:** 2026-08-09 · **Status:** Accepted · **Closes:** max_audit.md M6 + +`buildTrace` projected every `StageResult` down to `{ stageId, status, durationMs: 0, changed }`. +The stage's `metrics` and `notes` were discarded and the duration was a literal constant. + +So the trace could say that `compression:token-hashing` ran and changed something, and nothing +about what it removed, how much, whether any elision was reversible, or how long it took. The +stages compute that telemetry carefully — `itemsHashed`, `regionsHashed`, `bytesSaved`, +`irreversibleElisions`, `skippedPostConditionRejected` — and all of it was thrown away one +function call after being calculated. `--diff` and `--diff-html` partially compensate on the CLI; +the MCP `get_optimization_trace` tool and the Gateway had nothing else at all. + +For a product whose stated differentiator is auditability, the audit surface was the least +informative one in the system. + +### Decision + +`StageTrace` gains `metrics` and an optional `notes`, carried through verbatim. `durationMs` is +measured by the **engine**, not by the stage: a stage that read a clock would stop being a pure +function of its input (invariant 1), whereas timing an opaque call from outside is an observation +*about* the stage and cannot change what it returns. `performance.now()` rather than `Date.now()`, +because most stages finish inside a millisecond and integer resolution would report the same +uninformative `0` the hardcoded constant already did. + +The trace was already non-deterministic — it carries a UUID `requestId` — so this changes nothing +about invariant 1, which is a statement about emitted **bytes**. + +### The pruner's note was not vague, it was false + +`pruning:topology-pruner` returned `notes: 'All items fit within token budget; no pruning +required.'` unconditionally whenever `itemsPruned === 0`. Measured, a 5,405-token file at +`maxInputTokens: 10` reported that all items fit. They do not. + +The mechanism is worth stating because it is also H5: `applyCacheAwarePrefixLocking` pins every +item inside the first 1,024 tokens, `solve01Knapsack` places pinned items outside the candidate +set and always selects them, and `createContextBundle` produces a **one-item** bundle for CLI, +MCP and bench. Item 0 is therefore always pinned and `itemsPruned` is always 0. The note reported +that pruning was *unnecessary* for the case where it was *impossible*. + +The note now distinguishes the three cases and names the mechanism, and the metrics carry +`bundleTokens` and `maxTokens` so the claim is checkable rather than asserted: + +> Nothing prunable: all 1 item(s) are pinned by cache-prefix locking, but the bundle is 5405 +> tokens against a budget of 10. Pinned items bypass the knapsack (invariant 7), so the budget +> could not be enforced. + +This does not fix H5 — the knapsack is still unreachable on every shipping path. It stops the +trace from concealing that behind a reassuring sentence, which is the necessary first step: +the defect is now visible in the one place a user would look. diff --git a/src/core/engine/index.ts b/src/core/engine/index.ts index 81c1b9f..cf62adc 100644 --- a/src/core/engine/index.ts +++ b/src/core/engine/index.ts @@ -62,6 +62,14 @@ export function optimize( const stageCatalog = getBuiltInStageCatalog(); const selectedPlan = plan(request.bundle, request.budget, request.config, stageCatalog); const stageResults: StageResult[] = []; + // Wall time per stage, positionally aligned with `stageResults`. + // + // Measured here rather than inside the stages: a stage that read a clock would stop being a + // pure function of its input (invariant 1). Timing an opaque call from the outside is an + // observation about the stage, not an input to it. `performance.now()` rather than + // `Date.now()` because most stages finish inside a millisecond, and integer-millisecond + // resolution would report the same uninformative 0 the hardcoded value already did. + const stageDurationsMs: number[] = []; let currentBundle = request.bundle; let stageFailed = false; let failureReason: string | undefined; @@ -73,6 +81,7 @@ export function optimize( }; for (const stageId of selectedPlan.stageIds) { + const startedAt = performance.now(); try { const result = executeBuiltInStage( stageId, @@ -81,6 +90,7 @@ export function optimize( options?.sessionContext, compressionContext, ); + stageDurationsMs.push(performance.now() - startedAt); stageResults.push(result); if (result.status === 'ok' && result.changed) { currentBundle = result.bundle; @@ -91,6 +101,8 @@ export function optimize( } } catch (err) { const msg = err instanceof Error ? err.message : String(err); + // A stage that threw still consumed time, and a reader diagnosing a failure wants it. + stageDurationsMs.push(performance.now() - startedAt); stageResults.push( createStageResult({ stageId, @@ -282,6 +294,7 @@ export function optimize( const emittedOutput = fallback.output; const trace = buildTrace(request, selectedPlan, stageResults, validation, fallback, emittedOutput, { debtScore: debtBreakdown.debtScore, + stageDurationsMs, ...(validation.driftReport?.driftScore !== undefined ? { driftScore: validation.driftReport.driftScore } : {}), diff --git a/src/core/model/types.ts b/src/core/model/types.ts index 62c52f3..10e8f01 100644 --- a/src/core/model/types.ts +++ b/src/core/model/types.ts @@ -245,8 +245,26 @@ export interface ValidationReport { export interface StageTrace { readonly stageId: string; readonly status: StageStatus; + /** + * Wall time for this stage, measured by the engine. + * + * Measured by the engine and not by the stage: a stage that read a clock would no longer be + * a pure function of its input (invariant 1). Timing an opaque call from outside is an + * observation *about* the stage, not an input to it, and cannot change what it returns. + */ readonly durationMs: number; readonly changed: boolean; + /** + * The stage's own counters — `itemsHashed`, `bytesSaved`, `regionsHashed`, + * `irreversibleElisions`, `skippedPostConditionRejected`, and so on. + * + * Discarded entirely until 2026-08-09, along with `notes`. The stages compute this telemetry + * carefully and the trace threw all of it away, so a reader could see *that* a stage ran and + * changed something but not what it did, how much it removed, or whether the elisions were + * reversible — on a product whose thesis is auditability. (audit M6) + */ + readonly metrics: Readonly>; + readonly notes?: string | undefined; } /** diff --git a/src/core/trace/index.ts b/src/core/trace/index.ts index b58d884..6f3ecfc 100644 --- a/src/core/trace/index.ts +++ b/src/core/trace/index.ts @@ -19,13 +19,26 @@ export function buildTrace( validation: ValidationReport, fallback: FallbackOutcome, finalOutput: string, - metrics?: { readonly debtScore?: number; readonly driftScore?: number }, + metrics?: { + readonly debtScore?: number; + readonly driftScore?: number; + /** Per-stage wall time, positionally aligned with `stageResults`. See `StageTrace`. */ + readonly stageDurationsMs?: ReadonlyArray; + }, ): OptimizationTrace { - const stageTraces = stageResults.map((stage) => ({ + // Carry the stage's own telemetry through instead of discarding it. + // + // This used to project away `metrics` and `notes` and hardcode `durationMs: 0`, so the trace + // reported that a stage ran and nothing about what it did. `--diff` and `--diff-html` + // partially compensated on the CLI; the MCP `get_optimization_trace` tool and the Gateway had + // nothing else at all. (audit M6) + const stageTraces = stageResults.map((stage, index) => ({ stageId: stage.stageId, status: stage.status, - durationMs: 0, + durationMs: metrics?.stageDurationsMs?.[index] ?? 0, changed: stage.changed, + metrics: stage.metrics, + ...(stage.notes !== undefined ? { notes: stage.notes } : {}), })); return createOptimizationTrace({ diff --git a/src/stages/pruning/topology-pruner.ts b/src/stages/pruning/topology-pruner.ts index 924e882..203786e 100644 --- a/src/stages/pruning/topology-pruner.ts +++ b/src/stages/pruning/topology-pruner.ts @@ -61,6 +61,27 @@ export function runTopologyPrunerStage( const itemsPruned = bundle.items.length - selectedItems.length; if (itemsPruned === 0) { + const pinnedCount = knapsackItems.filter((i) => i.isPinned).length; + const bundleTokens = knapsackItems.reduce((sum, i) => sum + i.weight, 0); + + // Say which of the two very different reasons produced "nothing pruned". + // + // This unconditionally reported "All items fit within token budget; no pruning required." + // — which is a factual claim, and it was false in the case that matters. `solve01Knapsack` + // places pinned items outside the candidate set and always selects them, and + // `applyCacheAwarePrefixLocking` pins everything inside the first 1,024 tokens; on the + // one-item bundle `createContextBundle` builds for CLI, MCP and bench, item 0 is therefore + // always pinned. Measured: a 4,600-token file at `maxInputTokens: 10` reported "all items + // fit" — 4,600 tokens do not fit in 10. It announced that pruning was unnecessary for a + // case where pruning was impossible. (audit M6, H5) + const everythingPinned = pinnedCount === knapsackItems.length && knapsackItems.length > 0; + const overBudget = bundleTokens > maxTokens; + const notes = everythingPinned && overBudget + ? `Nothing prunable: all ${pinnedCount} item(s) are pinned by cache-prefix locking, but the bundle is ${bundleTokens} tokens against a budget of ${maxTokens}. Pinned items bypass the knapsack (invariant 7), so the budget could not be enforced.` + : overBudget + ? `No items pruned, but the bundle is ${bundleTokens} tokens against a budget of ${maxTokens}; the knapsack selected every candidate.` + : `All items fit within token budget (${bundleTokens} of ${maxTokens} tokens); no pruning required.`; + return createStageResult({ stageId, status: 'ok', @@ -69,10 +90,12 @@ export function runTopologyPrunerStage( metrics: { itemsPruned: 0, tokensSaved: 0, - pinnedCount: knapsackItems.filter((i) => i.isPinned).length, + pinnedCount, selectedCount: bundle.items.length, + bundleTokens, + maxTokens, }, - notes: 'All items fit within token budget; no pruning required.', + notes, }); } diff --git a/test/unit/trace-explains.test.ts b/test/unit/trace-explains.test.ts new file mode 100644 index 0000000..af0ae3a --- /dev/null +++ b/test/unit/trace-explains.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; +import { parse } from '../../src/adapters/cli'; +import { loadConfig } from '../../src/config/load'; +import { optimize } from '../../src/core/engine'; + +/** + * The explainability trace has to explain — audit M6. + * + * `buildTrace` projected each `StageResult` down to `{ stageId, status, durationMs: 0, changed }`, + * discarding every stage's `metrics` and `notes` and hardcoding the duration. The stages compute + * that telemetry carefully — `itemsHashed`, `bytesSaved`, `regionsHashed`, `irreversibleElisions`, + * `skippedPostConditionRejected` — and the trace threw all of it away. A reader could see *that* + * `compression:token-hashing` ran and changed something, but not what it removed, how much, or + * whether the elision was reversible. `--diff` and `--diff-html` partially compensated on the + * CLI; the MCP `get_optimization_trace` tool and the Gateway had nothing else. + * + * That is a problem for a product whose stated thesis is auditability, and it is the same shape + * as invariant 10 — a field that reports `0` whether or not anything was measured. + */ +describe('the trace carries what the stages actually computed', () => { + const runOn = (content: string, path: string, budget: Record) => { + const config = loadConfig(); + const request = parse(content, config, { sourceKind: 'file', sourcePath: path }); + return optimize({ ...request, budget: { ...request.budget, ...budget } }); + }; + + const TS_SOURCE = [ + 'export function alpha(a: number, b: number): number {', + ' let total = 0;', + ' for (let i = 0; i < a; i++) {', + ' total += i * b;', + ' }', + ' return total;', + '}', + '', + 'export function beta(values: string[]): string {', + ' const parts: string[] = [];', + ' for (const value of values) {', + ' parts.push(value.trim());', + ' }', + ' return parts.join(", ");', + '}', + '', + ].join('\n'); + + it('carries per-stage metrics and notes rather than projecting them away', () => { + const result = runOn(TS_SOURCE, 'src/sample.ts', { targetReductionRatio: 0.5 }); + + // Invariant 10: none of the below means anything if no stage ran. + expect(result.trace.stageCount).toBeGreaterThan(0); + + for (const stage of result.trace.stageTraces) { + expect(stage.metrics).toBeDefined(); + expect(typeof stage.metrics).toBe('object'); + } + + // At least one stage must have produced non-empty telemetry, or this test would pass + // against a trace that carried `{}` for everything. + const withMetrics = result.trace.stageTraces.filter((s) => Object.keys(s.metrics).length > 0); + expect(withMetrics.length).toBeGreaterThan(0); + + const withNotes = result.trace.stageTraces.filter((s) => typeof s.notes === 'string' && s.notes.length > 0); + expect(withNotes.length).toBeGreaterThan(0); + }); + + it('reports whether an elision was reversible, which was previously unknowable from the trace', () => { + // No `TokenHasher` is supplied here, which is the CLI's situation: the removed bytes are + // retained nowhere. The stage has always known this; the trace could not say it. + const result = runOn(TS_SOURCE, 'src/sample.ts', { targetReductionRatio: 0.5 }); + + const hashing = result.trace.stageTraces.find((s) => s.stageId === 'compression:token-hashing'); + expect(hashing).toBeDefined(); + expect(hashing?.changed).toBe(true); + expect(hashing?.metrics.irreversibleElisions).toBeGreaterThan(0); + expect(hashing?.notes).toContain('irreversible'); + }); + + it('measures stage duration instead of reporting a hardcoded zero', () => { + const result = runOn(TS_SOURCE, 'src/sample.ts', { targetReductionRatio: 0.5 }); + + for (const stage of result.trace.stageTraces) { + expect(stage.durationMs).toBeGreaterThanOrEqual(0); + expect(Number.isFinite(stage.durationMs)).toBe(true); + } + + // Sub-millisecond resolution is the point: with `Date.now()` every one of these would be a + // flat 0 and the field would be exactly as uninformative as the constant it replaced. + const anyPositive = result.trace.stageTraces.some((s) => s.durationMs > 0); + expect(anyPositive).toBe(true); + }); + + it('does not claim items fit a budget they exceed', () => { + // The audit's reproduction. `applyCacheAwarePrefixLocking` pins everything inside the first + // 1,024 tokens, `solve01Knapsack` always selects pinned items, and `createContextBundle` + // builds a one-item bundle — so item 0 is always pinned and `itemsPruned` is always 0. The + // stage reported "All items fit within token budget; no pruning required." for a bundle + // hundreds of times over budget: not a vague note but a false factual claim, and one that + // concealed the fact that pruning was impossible rather than unnecessary. + const big = Array.from({ length: 400 }, (_, i) => `export const value${i} = ${i};`).join('\n'); + const result = runOn(big, 'src/big.ts', { maxInputTokens: 10 }); + + const pruner = result.trace.stageTraces.find((s) => s.stageId === 'pruning:topology-pruner'); + expect(pruner).toBeDefined(); + expect(pruner?.metrics.itemsPruned).toBe(0); + + expect(pruner?.metrics.bundleTokens).toBeGreaterThan(10); + expect(pruner?.metrics.maxTokens).toBe(10); + expect(pruner?.notes).not.toContain('All items fit'); + expect(pruner?.notes).toContain('pinned'); + }); +});