3523dcf93e
plans/ held 58 documents and exactly one said whether it was open. The rest
mixed finished work, reviews of shipped work, parked specs and genuinely
pending ones, with nothing distinguishing them, so "how many plans are in
the queue" had no answer short of reading all 58.
Now `grep -H '^Status:' plans/*.md` is the answer:
50 done 3 in progress 2 planned 2 reference 1 parked
Statuses were derived rather than guessed: CLAUDE.md's own built list and
"What's NOT built yet" section, plus checking the subject exists in the
code. A review of work that shipped counts as done -- it records what was
found, it is not a request for anything. `reference` separates the two docs
that are conventions rather than work items (admin-design-standards,
admin-work-framework), which otherwise read as permanently-open plans.
The vocabulary is deliberately five words. A larger one invites "mostly
done" and "blocked-ish", which is how the directory became unreadable.
test_plans_declare_status.py keeps it from rotting: a new plan without a
marker fails, as does an unknown status, one buried below the eighth line,
or an open status with no reason -- "planned" alone is the state that rots,
since nobody can tell later whether it waits on a decision, a dependency,
or just nobody's turn.
Also updates the sweep plan with what landed and what did not, including
that #9 was not a defect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U
129 lines
6.3 KiB
Markdown
129 lines
6.3 KiB
Markdown
# Text integrity: every place the router touches bytes
|
|
|
|
Status: in progress -- mutation points fixed; contract checks and canary probes still open
|
|
|
|
An audit of the mutation points, prompted by `8518114` -- a double-encode in
|
|
the SSE proxy that corrupted every streamed non-ASCII character for months
|
|
behind a green test suite.
|
|
|
|
The invariant this establishes, in one line:
|
|
|
|
> **Never re-serialize what you are only forwarding, and never cut text
|
|
> anywhere but a character boundary.**
|
|
|
|
## The sites
|
|
|
|
Every `encode`/`decode`/slice on content, classified by what it can do.
|
|
|
|
### Forwarded to the client -- must be byte-exact
|
|
|
|
| site | what it does | verdict |
|
|
|---|---|---|
|
|
| `dispatcher.py` SSE proxy | forwards upstream bytes, decodes a private copy to sniff telemetry | **fixed in 8518114**; was decode-latin1 + encode-utf8 |
|
|
| `dispatcher.py` router-generated SSE (6 sites) | `json.dumps(...).encode()` | safe -- `ensure_ascii=True` makes the payload pure ASCII before encoding, so the encode is lossless |
|
|
|
|
The second row is worth stating rather than assuming: those six lines look
|
|
like the bug that was just fixed, and they are not, for a reason that is one
|
|
keyword deep. If anyone ever passes `ensure_ascii=False` there to save
|
|
bytes, they reintroduce a charset decision on an output path.
|
|
|
|
### Sent to a provider or a local model -- corrupts the model's input
|
|
|
|
| site | what it does | verdict |
|
|
|---|---|---|
|
|
| `context_prune` head/tail elision (3 sites) | shortens old tool results | **fixed**; now cuts on cluster boundaries |
|
|
| `clamp_for_classifier` | head+tail clamp to `max_input_chars` | **fixed** |
|
|
| `verification.excerpt` | head+tail elision for the local checker | **fixed** |
|
|
| request bodies | `requests(json=...)` | safe -- requests serializes with `json.dumps`, ASCII on the wire |
|
|
|
|
These slice `str`, so none could ever produce mojibake -- Python indexes
|
|
codepoints. What they could do is cut a **grapheme cluster**: the string
|
|
`"cafe" + U+0301` renders as four characters but stores five codepoints, and
|
|
slicing it at 4 drops the accent while leaving a bare combining mark at the
|
|
head of the tail. `src/textcut.py` moves the cut to the nearest boundary.
|
|
|
|
Severity is genuinely lower than the SSE bug -- a stray combining mark, not
|
|
a mangled document -- but the failure *shape* is the one this project keeps
|
|
paying for: the router corrupts the model's input, the model faithfully
|
|
reproduces the corruption, and the output reads as the model's fault.
|
|
|
|
### Diagnostic only -- never reaches the client
|
|
|
|
| site | what it does | verdict |
|
|
|---|---|---|
|
|
| `dispatcher.py:4442` | decodes upstream bytes with `errors="replace"` to inspect | safe by design -- the forwarded bytes are untouched; only the telemetry sniff and the logged completion see a replacement char |
|
|
| `admin.py:1822` | subprocess output, `errors="replace"` | acceptable; a maintenance job emitting non-UTF-8 shows replacement chars in the portal |
|
|
| `dispatcher.py:2691` | `sha256(content[:4000].encode())` session fingerprint | safe; deterministic, never rendered |
|
|
| `context_prune` savings estimate | `len(text) - head_len - tail_len` | left alone deliberately -- it is a planning estimate, not a cut, and a 1-2 char difference changes nothing |
|
|
|
|
## Why the suite did not catch the SSE bug
|
|
|
|
This is the more useful half of the audit, because the code review that
|
|
would have caught the decode never happened -- the tests were supposed to.
|
|
|
|
There is a test named
|
|
`test_a_stream_is_proxied_verbatim_including_the_telemetry_comments`. It
|
|
passed, for months, against a proxy that was not proxying verbatim. Two
|
|
independent reasons, both now fixed:
|
|
|
|
1. **The assertions were ASCII.** It checked three ASCII substrings. A test
|
|
that only ever sees ASCII cannot observe a charset bug, by construction.
|
|
2. **The fakes modelled the boundary instead of exercising it.** All five
|
|
`FakeResponse.iter_lines` implementations yielded `self._lines` unchanged
|
|
in both modes. A test holding `str` lines got those `str` back even under
|
|
`decode_unicode=True` -- i.e. the fake modelled a stream that had already
|
|
been decoded *correctly*. A fake that hands back the right answer cannot
|
|
reproduce a decode bug.
|
|
|
|
The fakes now treat the wire as UTF-8 bytes whatever the test wrote, derive
|
|
`encoding` from `Content-Type` via requests' own
|
|
`get_encoding_from_headers`, and **default to the charset-less
|
|
`text/event-stream` that OpenRouter really sends**. The hostile case is the
|
|
default. The shared `STREAM_LINES` sample carries a raw UTF-8 em dash, so
|
|
the whole streaming surface exercises it.
|
|
|
|
Measured by reverting `8518114` and running the suite:
|
|
|
|
| | tests that caught it |
|
|
|---|---|
|
|
| before this work | 1 (the one written for the bug) |
|
|
| after | 2, including the verbatim test that had been lying |
|
|
|
|
And for the truncation sites, reverting `textcut` fails 3 of the 5 new
|
|
call-site tests.
|
|
|
|
## What this does and does not buy
|
|
|
|
Prevention covers faults **the router introduces**. It is the right first
|
|
move and it is now largely done: the mutation points are enumerated, the two
|
|
real classes are fixed, and the tests exercise the boundary rather than a
|
|
model of it.
|
|
|
|
It covers none of:
|
|
|
|
- a provider changing quantization, or silently serving a different model;
|
|
- corruption upstream of us, in a provider's own edge;
|
|
- a model that starts looping, refusing, or truncating.
|
|
|
|
It also has no memory. Once the audit is done there is no ongoing evidence
|
|
the invariant still holds -- against a provider added next month, or one
|
|
that changes its headers next quarter. OpenRouter was re-added under #45 and
|
|
the corruption arrived with it.
|
|
|
|
So the natural follow-ons, cheapest first:
|
|
|
|
1. **Provider contract checks at poll time.** Assert what we assume and
|
|
never verify: SSE declares a charset, the stream shape parses, a model id
|
|
resolves to a chat endpoint. No traffic, no spend. This bug was 100%
|
|
inside it.
|
|
2. **Canary probes.** A fixed string with an em dash, combining marks, CJK
|
|
and a ZWJ sequence, echoed by each model and diffed exactly. The only
|
|
thing that tests the composed system on the real wire rather than against
|
|
a fake. `seed_energy.py` already establishes the pattern and
|
|
`llm-router-seed.timer` already provides the schedule.
|
|
|
|
`plans/mangled-output-detection.md` covers what to do once something is
|
|
actually detected, and argues for keeping the first response loud rather
|
|
than corrective -- auto-mitigation would have hidden this bug rather than
|
|
surfacing it.
|