Uh oh!
There was an error while loading. Please reload this page.
A cut stream with no content emits no assistant row on any wire shape - #592
Conversation
…#591) The Anthropic and Chat Completions stream reconstructions both mark a stream that ended without its terminal event `stop_reason = 'error'` and return whatever arrived. When nothing arrived, that was still a row: `content: []` with a live `message_index` and no native `message_id`, so it stayed eligible for settlement's ordinal/time fallback and could acquire a `message_id` belonging to another turn. Every content-free assistant row also hashes to one canonical match key (5d008246) regardless of which decoder built it, so empty rows could collide across wire shapes too. The floor now sits at the one place a row is assembled, the projectedMessages loop in `project()`, so both shapes (and any added later) inherit it. The condition is "cut AND empty", keyed on the synthetic cut marker now named `CUT_STREAM_STOP_REASON`: a response that reached its terminal event and genuinely produced nothing keeps its row, and Chat Completions' delta stitching is untouched, so a cut stream that did carry partial text still records it. Co-Authored-By: Claude <noreply@anthropic.com>
…#591) `isEmptyCutRow` decided cut-ness by comparing the row's `stop_reason` to the literal `'error'`, on the premise that neither API sends `error` as a real stop reason. That premise does not cover the traffic this projector actually reads. An unrecognized upstream is parsed as the Anthropic wire by design, and an OpenAI-compatible endpoint reached through a config upstream can send `finish_reason: "error"`; a terminal response that said `error` and produced nothing is a real answer, and the row was the only record it happened. Reproduced on all three shapes: a terminal Chat Completions chunk with `finish_reason: 'error'`, a non-streamed body with the same, and an Anthropic `message_delta` + `message_stop` carrying `stop_reason: 'error'` all lost their assistant row. The same premise covered request history: an Anthropic history turn that replays `stop_reason: 'error'` on an empty assistant message was dropped too, which the role test alone does not prevent. Cut-ness is now carried out of band, in a module-level `WeakSet` the two reconstructions add to on the branch that stamps the marker. A wire body is parsed JSON, so it can forge any field value but never membership in that set, which makes "cut AND empty" true by construction rather than by an assumption about upstream stop-reason vocabularies. The emitted `stop_reason` is unchanged, and every cut-and-empty case the fix already dropped still drops. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 4, 2026
Review: findings (data-loss bug in the fix itself, fixed and pushed as |
| case | result |
|---|---|
terminal Anthropic message_stop, empty content blocks | records, stop_reason=end_turn |
terminal Anthropic message_stop, no message_delta at all | records, no stop_reason |
terminal Chat Completions finish_reason: content_filter, no content | records |
non-streamed body content: null + wire finish_reason | records |
cut Chat Completions with partial text par | records, text intact |
cut Anthropic with partial text par | records, text intact |
cut Chat Completions whose only content is a half-stitched tool_call | records, tool name preserved |
role !== 'assistant' carrying stop_reason: 'error' | records |
Revert tests
| test | verdict |
|---|---|
no assistant row for a cut OpenAI stream... | fails on revert, load-bearing |
no assistant row for a cut Anthropic stream... | fails on revert, load-bearing |
still records a terminal Anthropic stream... | passes either way |
still records a terminal OpenAI stream... | passes either way |
still records a non-streamed OpenAI response... | passes either way |
The last three are not vacuous: mutation-testing the guard down to "drop on emptiness alone" fails all three while the first two still pass, so they pin the boundary against the most plausible wrong implementation. Honest characterisation: 2 regression tests, 3 pinning tests. Two further tests added with cdfeb07 (terminal wire error on all three shapes; request-history replay), both revert-tested against 4c73ac2.
Composition with PR #586
Merges cleanly (git merge pr-586-head, ort, no conflicts; 194/194 openclaw tests pass on the merged tree). The case of interest, a terminal response.completed whose output really is [], is preserved and not by luck: openaiResponsesAssistant derives stop_reason from incomplete_details.reason ?? (status !== 'completed' ? status : undefined), so completed-with-empty-output carries no stop_reason, and response.failed / response.incomplete carry failed / content_filter. None is 'error'.
R1 response.completed, output [] -> records, no stop_reason
R2 response.failed, output [] -> records, stop=failed
R3 response.incomplete/content_filter -> records, stop=content_filter
R4 cut stream, zero done items -> no row (#586's own guard)
R6 cut stream, one done item with text -> records, text intact
One merge obligation this creates.#586's responsesAssistantFromStream writes partial.stop_reason = 'error' as a bare literal. Under the original string-comparison floor the Responses path inherited the drop automatically; under identity-based cut-ness it does not. A cut Responses stream whose one finished item is of a type openaiResponsesAssistant does not map (e.g. image_generation_call) emits a content-free row on the merged tree. #586 already guards its main case, so this is narrow, and the fix is one line at merge time: replace partial.stop_reason = 'error' with markCutStream(partial). That is why markCutStream exists as a function rather than two statements, and its doc says so.
New finding
[low] A cut Anthropic stream that opened a text block still emits a content-free row.isEmptyCutRow (projector.js:419) tests content.length === 0. A stream cut between content_block_start {type:'text',text:''} and the first content_block_delta yields content: [{"type":"text","text":""}], which is not length-0:
A4 cut anthropic, message_start + content_block_start(text), no deltas
-> assistant content:[{"type":"text","text":""}] stop=error mk=b8501a26
That row carries a live message_index, zero information, and a match key as canonical across turns as 5d008246, so it sits in the ordinal fallback's candidate list exactly like the row this PR removes. Left unfixed deliberately: the window between those two events is milliseconds (unlike message_start to content_block_start, where prefill latency lives), so reachability is genuinely low, and widening the emptiness predicate risks its own over-reach (a thinking block with empty text but a real signature, or a tool_use block with {} input, are both real evidence). Flagged so it is a decision rather than an oversight. The isEmptyCutRow docstring's "no content at all" overstates what the predicate covers.
[info] Pre-existing, out of scope:wireMatchKey('assistant', undefined) and wireMatchKey('assistant', []) produce the same key, so an assistant body with no content field projects a row with content absent and the canonical empty key. Present on master.
Conventions
Clean. No em dashes anywhere in the diff, no semicolons, JSDoc types only, no @typedef, no inline import('...'). @ref LLP 0161#match-keys resolves to a real heading (llp/0161-openclaw-full-capture.design.md:435) and the gloss is true and non-obvious: that section's fallback-matcher paragraph is what makes a live message_index on a content-free row an identity hazard, which is not visible from the code or filename.
Verification
npm test: 3345 tests, 3344 pass, 1 skipped, 0 fail (atcdfeb07)npm run typecheck: clean- Merged with
pr-586-head: 194/194 openclaw tests pass - Pushed:
cdfeb07
Round-2 review follow-up. The floor's safety rests on an unenforced convention: `markCutStream` is the only thing allowed to write `CUT_STREAM_STOP_REASON`, because the recorded value and the WeakSet membership that actually decides the drop are halves of one fact. The two wire shapes that exist are covered by behavior tests (verified by mutation: introducing a copy between mark and check, or stamping the literal directly, fails them). A wire shape added later is not, and PR #586's Responses reconstruction is exactly that case waiting to happen: it writes `partial.stop_reason = 'error'` as a bare literal and has to become `markCutStream(partial)` at merge time. Adds a source lint, the same shape as `house-style-em-dash.test.js`, that fails when the marker is stamped anywhere but inside `markCutStream`, and points `markCutStream`'s docstring at it. No behavior change. Co-Authored-By: Claude <noreply@anthropic.com>
The comment said request-history messages are "excluded twice over, by the set and by the assistant-role test". Only the set excludes them. A replayed history turn can carry role: 'assistant' with an empty content array, which is precisely the claim round 1 of this PR made and round 2 falsified by execution, so leaving the sentence standing invites the same mistake back. Comment only. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 4, 2026
Round 3: the WeakSet holds. Two durability fixes pushed.Independent check of the fix to the fix, run by a third agent. Everything below comes from fixtures built fresh against Verdict: the identity mechanism is sound and round 2's diagnosis is correct. The data loss it claimed reproduces, plus one case beyond its list. Two gaps found and fixed: an unenforceable convention the whole mechanism rests on, and a docstring repeating the exact overclaim round 1 was falsified on. Fixtures at |
| # | fixture | at cdfeb07 | at 4c73ac2 |
|---|---|---|---|
| A1 | cut chat-completions, role-only delta | drops | drops |
| A2 | cut chat-completions, content: '' delta | drops | drops |
| A3 | cut anthropic, message_start only | drops | drops |
| A4 | cut anthropic, content_block_start(text), no deltas | records [{"type":"text","text":""}] | same (the residual) |
| B1 | terminal chat-completions finish_reason: error, no content | records | dropped |
| B2 | non-streamed body content: null + finish_reason: error | records | dropped |
| B3 | terminal anthropic stop_reason: error + message_stop | records | dropped |
| B4 | anthropic stop_reason: error, nomessage_stop | records | dropped |
| B5 | non-streamed anthropic stop_reason: error, content: [] | records | dropped |
| C1 | anthropic history replays {role:'assistant',content:[],stop_reason:'error'} | records | dropped |
| D1-D7 | every keep-recording case from round 1 (terminal empties, partial text, half-stitched tool_call) | records | records |
| E1-E2 | non-assistant history rows | records | records |
| F1 | wire body carrying every field a cut row has | records (cannot forge membership) | dropped |
Round 2's finding is real, and six losing cases reproduce at 4c73ac2, one more than it reported: B4, an Anthropic stream carrying a wire stop_reason: 'error' in a message_delta but cut before message_stop. cdfeb07 records it correctly, because markCutStream is gated on message.stop_reason == null.
Identity mechanism: every path from mark to check
Two markCutStream call sites and one isEmptyCutRow call site, repo-wide.
- Anthropic streamed:
reconstructAssistantMessagebuilds a fresh literal, marks it (:549), returns it;anthropicMessagespushes by reference;project()'s loop checks it (:190). - OpenAI streamed: same shape via
reconstructOpenaiAssistantMessage(:857).
Nothing on either path copies, spreads, clones, serializes or re-parses. project() builds its projected object after the check, so the check sees the original. Two rebuilds sit immediately adjacent in the very same functions, which is why the copy hazard is worth pinning: anthropicMessages maps request history through .map((m) => ({ ...m })), and openaiMessages rebuilds history through openaiWireMessage. Neither touches the reconstructed assistant, but widening either by one line would break the floor silently.
Wrongly gaining membership: not possible. Both call sites are guarded (!sawMessageStop && message.stop_reason == null; else if (!sawFinish)), so a stream that reached its terminal event or carried a wire stop reason is never marked (B1/B3/B4/D3). The set is module-private, never exported, and F1 confirms a wire body cannot forge it.
Cross-exchange leakage: none. Both marked objects are allocated per project() call. Three identical cut exchanges on one projector each drop independently; a terminal empty stream immediately after records. Feeding the same input object to two projector instances re-parses into new objects and both drop correctly. WeakSet entries are collectable, so no growth.
The copy risk is already covered, proven by mutation
| mutation | result |
|---|---|
for (const message of messages.map((m) => ({ ...m }))) in project() | tests 11 and 12 fail |
messages.push({ ...assistant }) in anthropicMessages | test 12 fails |
anthropic site stamps the literal instead of calling markCutStream | test 11 fails |
Round 1's two regression tests already are the copy gate, on both shapes.
But the sibling risk was completely uncovered, and it is live
The mechanism's safety rests on a convention the docstring stated as a "should": every reconstruction added here should mark its cut branch through this function. Nothing enforced it. Appending a plausible new reconstruction that writes partial.stop_reason = 'error' as a bare literal:
# tests 19 / # pass 19 / # fail 0
Zero tests fail, because no behavior test can see a wire shape nobody has written yet. This is not hypothetical: it is exactly the #586 merge obligation already recorded on that thread (partial.stop_reason = 'error' must become markCutStream(partial)), which nothing in the tree would notice being forgotten.
Fixed in cd6c8b3: a source lint, same shape and rationale as the repo's existing test/core/house-style-em-dash.test.js ("a rule that is only written down is a rule that drifts back"). It asserts the only .stop_reason = CUT_STREAM_STOP_REASON | 'error' assignment in the file sits inside markCutStream's body. Re-running the same mutation:
not ok 18 - the cut-stream marker is only ever stamped through markCutStream
# tests 20 / # pass 19 / # fail 1
Only the lint catches it. No behavior change.
Revert tests
Both cdfeb07 tests are load-bearing (projector.js at 4c73ac2, tests at cdfeb07):
not ok 16 - still records a terminal response whose wire stop reason is literally error
not ok 17 - never drops a request-history assistant turn that replays stop_reason error
# tests 19 / # pass 17 / # fail 2
Test 16 is a 3-in-1: B1/B2/B3 confirm all three shapes fail independently. Test 17 falsifies round 1's stated claim that request history could never be eligible. Honest tally for the PR: 4 regression tests (2 from round 1, 2 from round 2), 3 pinning tests, 1 source lint.
The unfixed residual (A4)
Reproduced. This is the same hazard class, not a lesser one: ordinalFallbackMatch keys on message_index + role + a time window and never consults content, so any assistant row at that index is a candidate; the match key only decides whether the content pass matches first.
Round 2's two counterexamples do not actually block a precise fix. A predicate restricted to { type: 'text', text: '' } blocks touches neither: a tool_use block from content_block_start carries a real id and name, and a thinking block's signature is unreachable here anyway, since reconstructAssistantMessage handles text_delta/thinking_delta/input_json_delta and has no signature_delta case at all.
Not pushed, and the outcome is still right: the reachability argument holds (the window between content_block_start and the first text_delta really is milliseconds, unlike the prefill latency before content_block_start), no failing test demands it, and the shape of the emptiness predicate deserves to be a recorded decision rather than a reviewer's aside on a third-round thread. Severity low. Filed separately so it stays a decision.
New findings
| sev | finding | location | status |
|---|---|---|---|
| low | Nothing enforced that CUT_STREAM_STOP_REASON is stamped only via markCutStream; a new wire shape (concretely #586's Responses reconstruction) could break the floor with zero test failures | projector.js:112, :857 | fixed, cd6c8b3 |
| nit | isEmptyCutRow's docstring claimed history is "excluded twice over, by the set and by the assistant-role test". Only the set excludes it: a replayed history turn can carry role: 'assistant' with content: [], which is the claim round 1 made and round 2 falsified by execution | projector.js:425 | fixed, a8d20fe (comment only) |
| info | B4 is a sixth case round 1 silently dropped, beyond the five round 2 listed. Correct at cdfeb07 | - | no action |
Conventions
Clean. No em dashes, no semicolons, JSDoc types only, no @typedef, no inline import('...'). @ref LLP 0161#match-keys [constrained-by] resolves to a real heading and its gloss is true and non-obvious: that section's Fallback matcher paragraph is the ordinal-plus-time-window rule the gloss cites.
Verification
npm test: 3346 tests, 3345 pass, 1 skipped, 0 fail (ata8d20fe)npm run typecheck: clean- Fixture suite re-run against a freshly fetched tree: identical results
- Pushed:
cd6c8b3,a8d20fe
philcunliffe
commented
Aug 4, 2026
Triage: shipThe review-round cap was reached with one finding unresolved (residual A4: a cut Anthropic stream that opened a text block still emits a content-free row). Classified non-blocking, and the deciding fact is checkable rather than a judgment call:
Deferred to #595, which records the precise-fix analysis round 3 did (why a Disclosure: this triage was performed by the reconciler rather than dispatched to a separate judgment-tier agent, because round 3 had already classified the residual with evidence and the deciding fact above is a git check, not an assessment. Recorded so a reader can weigh it. Three review rounds, four regression tests, three pinning tests and one source lint. This PR is mergeable, green, and reviewed. Next tick it flips out of draft and holds for a human. |
#586 landed its Responses reconstruction stamping `partial.stop_reason = 'error'` as a bare literal, which is the merge obligation recorded on that PR's thread. Merging master into this branch reproduced it: the lint added in cd6c8b3 fails with not ok 18 - the cut-stream marker is only ever stamped through markCutStream GitHub reported the merge CLEAN, because the conflict is semantic rather than textual: no rung sees it and nothing but the lint would have caught it before it reached master. Routing the Responses path through markCutStream also closes the residual round 3 identified: a cut Responses stream whose finished items produce no mapped content emitted a content-free row with a live message_index, which #586's own doneItems.length === 0 guard does not cover. Fixes#591
philcunliffe
commented
Aug 4, 2026
Re-triage at |
Uh oh!
There was an error while loading. Please reload this page.
Root cause
Both stream reconstructions in
hypaware-core/plugins-workspace/openclaw/src/projector.jsmark a stream that ended without its terminal event (message_stopon the Anthropic wire, afinish_reasonon Chat Completions) withstop_reason = 'error'and return whatever arrived. Neither checked whether anything actually arrived.When nothing did, the result was still a row:
content: [], a livemessage_indexminted by the gateway, and no nativemessage_id(this projector always emits fallback identity). That leaves it eligible for settlement's ordinal/time fallback (LLP 0161 Section 5), so it can acquire a nativemessage_idbelonging to some other turn. And becausewireMatchKey('assistant', [])is one canonical value, every empty assistant row hashes to the same key (5d008246) whichever decoder built it, so empty rows can collide across wire shapes as well.PR #586 closes exactly this for the OpenAI Responses decoder it introduces. That guard is not on
master, so nothing here touches it; this PR puts the equivalent floor at the row-assembly point, which the Responses path will also pass through once #586 lands.What the fix does
CUT_STREAM_STOP_REASONand has both reconstructions set it, instead of two bare'error'literals. Neither API sendserroras a real stop reason, so it unambiguously means "this projector cut the answer short".isEmptyCutRow(role, content, stopReason)and applies it in theprojectedMessagesloop ofproject(). That is the single place a row is assembled, so both wire shapes, and any shape added later, inherit the floor rather than each re-deriving it.deltastitching is untouched. Its condition is "no content at all after stitching", soproject() marks a truncated OpenAI stream stop_reason=error(partial textpar, nofinish_reason) still records its row with the partial text intact.Cases deliberately left recording
The condition is cut AND empty, not empty. A response that reached its terminal event and genuinely produced nothing is a real answer, and its row is the only record that the turn happened at all:
message_stop, or amessage_deltacarrying a wirestop_reason) whose content blocks are empty.finish_reason: 'content_filter'and no content.choices[0].message.content: nulland a wirefinish_reason.This matches what #586's guard deliberately preserves on the Responses side (a
response.completedwhoseoutputreally is[]still records). Each of the three is covered by a new test that passed before this change and still passes after, so the floor is pinned from both directions.Request-history rows are also never eligible:
isEmptyCutRowrequiresrole === 'assistant', and a caller-supplied message never carries this projector's synthetic marker.Regression test
test/plugins/openclaw-projector-shape.test.jsgains two failing-first tests, one per decoder, each asserting both that no assistant row is emitted and that no emitted row carries the canonical empty-assistant match key.Before the fix (
node --test test/plugins/openclaw-projector-shape.test.js, atf17b091):The match key those rows carried is
5d008246..., byte-identical to the key in the issue.After the fix:
# tests 17 / # pass 17 / # fail 0.Checks
npm test: 3343 tests, 3342 pass, 1 skipped, 0 fail.npm run typecheck: clean.Fixes#591