Issue #7: mid-stream robustness fix discovered via 2026-05-22 crash. Long mimir TUI conversation (turn 93, 1077 events consumed) crashed on event 1078 with JSONDecodeError("Expecting value: line 1 column 1 (char 0)") from json.loads('') on an empty-data SSE frame. _iter_events unconditionally called json.loads on every dispatched event; when httpx_sse surfaces a frame with id: present but data: empty/missing (a known library-vs-spec divergence), parsing fails and propagates. Two-rule fix in _iter_events: - Empty sse.data (exact `== ''`): SKIP silently per SSE spec (keepalive semantics). Don't yield, don't advance last_sse_id, don't set terminal_seen. ORDERING: skip fires BEFORE _parse_sse_id, so a keepalive with a malformed id is still a keepalive (intentional). - Non-empty sse.data that fails json.loads: raise new MalformedSseData (sibling to MalformedSseId, mirrors raw[:200] truncation pattern). Wire-level protocol error; presenters route to [malformed_sse_data] + exit 22 in cli, transcript label + state→idle in tui (INV-008). Volva paraphrase round: 4 ambiguities, all amended: 1. INV-001 prose tightened — exact `sse.data == ''` rule made prominent; "keepalive" framing demoted to intent-not-rule; whitespace-only data explicitly listed as malformed (not skipped); specific state names (last_sse_id, terminal_seen) instead of vague "any counter". 2. STEPS pseudocode spells out the ordering — empty-skip happens BEFORE _parse_sse_id; empty-data with bad id is silently swallowed. 3. empty_data_skipped test description fixed (had off-by-one count + wrong wording around last_sse_id intermediate state). 4. (paired with #1 above). Volva code-review post-implementation: 3 findings, all addressed: F1 (test-gap): empty_data_skipped proves yielded events but not internal last_sse_id non-advancement. New empty_data_skip_preserves_last_seen_sse_id test probes via SseConnectionDropped.last_seen_sse_id after a drop following the skipped frame — if the skip had transiently advanced last_sse_id, the exception payload would carry the wrong value. F2 (precision, contract amend): MalformedSseData ERROR_ROUTING said "log truncated raw" but stream_turn doesn't log — sse_client is a library, presenters own observability. Amended to "propagate to caller (no logging at sse_client layer); presenters log exc.raw." F3 (test-gap): cli malformed_sse_data test asserted label but not `raw='X'` shape and not truncation. Tightened existing test + added malformed_sse_data_truncation with 5000-char payload — verifies MalformedSseData.raw truncation carries through the presenter's repr() rendering. **Calibration milestone**: issue #7 is the first issue with ZERO drift findings from Volva code-review. TDD caught all runtime behavior cleanly. The 3 findings were assertion-precision and architectural-correctness-of-wording, not behavioral. Hypothesis: tighter contract spec + smaller code surface shifts Volva's role from "catch behavioral drift" to "tighten observability + wording". Cumulative calibration table: #1 (4 findings, 3 drift + 1 test-gap), #2 (3, 1+1+1 precision), #3 (5, 3+1+1), #4 (8, 5+2+1), #7 (3, 0 drift + 2 test-gap + 1 precision). Contracts touched (all drift-check clean): - docs/contracts/issues/7.contract.md (new): the coordinating record. - docs/contracts/issues/1.contract.md: _iter_events STEP 3.0 empty-skip + ordering note; STEP 3.c JSONDecodeError → MalformedSseData; new MalformedSseData ERROR_ROUTING (propagate-to-caller wording per F2); 4 new TESTS entries including F1's last-seen probe. - docs/contracts/issues/3.contract.md: _run_turn ERROR_ROUTING + malformed_sse_data tests (incl. F3 truncation). - docs/contracts/issues/4.contract.md: INV-008 mentions MalformedSseData; _stream_turn_worker ERROR_ROUTING + new TEST.
16 KiB
contract_version, target_module, scope, depends_on, used_by, language, complexity, estimated_loc, confidence, assumptions, open_questions, prd, dependencies
| contract_version | target_module | scope | depends_on | used_by | language | complexity | estimated_loc | confidence | assumptions | open_questions | prd | dependencies | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2.1 | ratatoskr.sse_client | Stop crashing mid-stream when an SSE event with empty `data:` is dispatched. Two-rule fix in `_iter_events`: (a) when `sse.data` is empty, SKIP the event silently (treat as keepalive per SSE spec — don't yield, don't update `last_sse_id`, don't crash); (b) when `sse.data` is non-empty but fails `json.loads`, raise a new `MalformedSseData(raw=sse.data[:200])` exception (mirrors the existing `MalformedSseId` shape — wire-level protocol error, surfaceable to presenters). Presenters (`cli._run_turn`, `tui._stream_turn_worker`) gain a `MalformedSseData` case in their ERROR_ROUTING. No new modules; in-place amendments to issues #1 (sse_client), #3 (cli), #4 (tui). Discovered by mid-stream crash 2026-05-22: turn 93 emitted 1077 events successfully, then event 1078 arrived with `id: 93:1078` but empty `data:`; `json.loads('')` raised `JSONDecodeError`, propagated through Textual's worker, crashed the app. |
|
python | low | 25 | 0.9 |
|
|
|
|
Mid-stream robustness — empty SSE data is skipped, malformed is raised cleanly
Context
During a TUI conversation with mimir on 2026-05-22, ratatoskr consumed
1077 SSE events successfully then crashed on event 1078:
JSONDecodeError: Expecting value: line 1 column 1 (char 0)
at src/ratatoskr/sse_client.py:301
in event = _envelope_for_type(json.loads(sse.data), sse_id=sse_id)
sse = ServerSentEvent(event='message', id='93:1078')
data = '' # ← EMPTY but id present
sse_id = SseId(turn_id=93, seq=1078)
_iter_events unconditionally calls json.loads(sse.data) on every
dispatched event. When httpx_sse surfaces an event with empty data,
the JSON parser fails, the exception propagates through stream_turn →
_stream_turn_worker → Textual's Worker → app abort → traceback dumps
to the operator's terminal. The turn is lost.
The SSE spec allows empty-data events (commonly keepalives). The Worldtree
spec says every event carries JSON data:, so an empty-data frame from
Worldtree is either a server bug, an httpx_sse permissive-parsing quirk,
or mid-stream truncation. The cause is upstream; ratatoskr's responsibility
is to not crash the operator's conversation over a single anomalous frame.
This issue adds two robustness rules in _iter_events: empty data is
silently skipped (keepalive semantics); non-empty malformed JSON raises
a new named exception (MalformedSseData) that presenters surface as a
clean error label, not a raw traceback.
Data flow
Input change:
_iter_eventscontinues to consumeServerSentEventobjects fromhttpx_sse.EventSource. Now distinguishes three cases onsse.data:- Empty (
sse.data == ''): skip — don't yield, don't updatelast_sse_id, don't increment any counter. Continue to next. - Non-empty + parses as JSON: yield as today (typed Event per
_envelope_for_type). - Non-empty + JSONDecodeError: raise
MalformedSseData(raw=sse.data[:200]).
- Empty (
Output change:
- A stream containing K empty-data events interleaved with N regular events yields N events (the K empty are invisible to the caller).
- A stream containing a malformed-data event raises
MalformedSseDatathe same wayMalformedSseIdis raised today (consumer catches in ERROR_ROUTING).
No side effects: purely an iterator-internal behavior change. No new disk I/O, no new HTTP calls, no new state.
Invariants
- INV-001 [hard]:
_iter_eventsMUST silently skip events wheresse.data == ''(exact empty string equality — NOT truthy-check, NOT a stripped-whitespace check, NOT a "looks like a keepalive" pattern). Whitespace-only data (' ','\n','\t') is NOT empty and MUST go through the malformed-raise path (INV-002). The "keepalive" framing is intent-of-the-skip, not a generalized rule — implementers MUST apply the literalsse.data == ''check. Skip means: don't yield, don't updatelast_sse_id, don't setterminal_seento True, don't call_parse_sse_idonsse.id, don't raise. The iteration continues to the next event fromevent_source.aiter_sse(). (No other counters or accumulators exist in_iter_events—last_sse_idandterminal_seenare the only state to preserve. Future implementations must not silently grow the state set without re-evaluating this invariant.) - INV-002 [hard]:
_iter_eventsMUST raiseMalformedSseData(NOTJSONDecodeError, NOTSseConnectionDropped) whensse.datais non-empty but failsjson.loads(...). Truncation:raw=sse.data[:200]in the exception. The raw field is the only payload-derived data on the exception — keep it bounded. - INV-003 [hard]: Skip does NOT advance
last_sse_id. Rationale:last_sse_idis what callers use forreconnect_turnonSseConnectionDropped. A skipped empty-data event was not user-visible content; the caller should resume from the last real event, not from the skipped one. (Iflast_sse_idadvanced through the skip and a drop happened immediately after, the server would replay nothing of interest — the operator loses no data, but the precision is wrong.) - INV-004 [hard]:
MalformedSseDatais a sibling ofMalformedSseIdin the public exception surface. Both indicate wire-level protocol violations from the server; both abort the stream (not recoverable in process); both belong to exit-code bucket 22 (protocol failures) in cli's table.MalformedSseData.rawmirrorsMalformedSseId.raw— both stored as truncatedstrattributes on the exception. - INV-005 [hard]: No
core.*/worldtree.*imports (existing boundary; unchanged).
Out of scope
- Diagnosing upstream root cause. Whether Worldtree is emitting empty-data events legitimately (keepalive) or as a server bug is a worldtree-dev question worth filing separately if frequency is non-trivial. Ratatoskr's robustness fix is independent.
- Telemetry on skipped events. No stderr-label or counter for
[sse_keepalive]-style. v1 silent skip; revisit if frequency turns out to matter for observability. - Auto-reconnect on
MalformedSseData.reconnect_turnexists for clean drops; whether to invoke it after a malformed frame is policy and currently the consumer's call. Presenters today don't auto- reconnect on any error class; that pattern stays unchanged here. - Whitespace-only data handling beyond malformed-raise. Treating
' 'or'\\n'as malformed (per the raise rule) is the v1 stance; if a real-world server emits whitespace keepalives, file a follow-up. SseConnectionDroppedreclassification. Existing wire-level exceptions (ReadError,RemoteProtocolError,ReadTimeout, clean-EOF-before-terminal) continue to map toSseConnectionDropped.MalformedSseDatais a distinct class: payload-level malformation, not transport-level failure.
Constraints
- [compatibility] Spec pin unchanged. The empty-data handling is a client-side robustness change; nothing about the wire contract moves.
- [performance] A skipped event costs one extra branch + continue in the loop. Negligible.
- [security]
MalformedSseData.rawtruncates to 200 chars to bound the exception body — same hygiene asMalformedSseId.raw[:64]andSseConnectFailed.body[:1024]. - [style] Exception class follows existing pattern (frozen-ish:
__init__storesself.raw,super().__init__(message)). Ruff line-length=100.
In-place amendments (the work)
This issue's contract is small because the real work is amendments to issues #1, #3, #4. The amendments are pinned here so reviewers see the whole change in one place; the actual contract files are amended in-place as part of this issue's commit.
Issue #1 (ratatoskr.sse_client) amendments
New public exception:
class MalformedSseData(Exception):
"""Raised when an SSE event's `data:` payload is non-empty but fails JSON parsing."""
def __init__(self, raw: str) -> None:
super().__init__(f"malformed SSE data: {raw[:200]!r}")
self.raw = raw[:200]
_iter_events STEPS amended:
FOR EACH sse in event_source.aiter_sse():
IF sse.data == "":
# INV-001: empty-data event is treated as a keepalive; skip silently.
# ORDERING NOTE: this branch fires BEFORE _parse_sse_id(sse.id). An
# event with empty data AND a malformed id is therefore silently
# skipped (MalformedSseId is NOT raised). This is intentional — a
# keepalive with a bad id is still a keepalive, and the user-visible
# behavior is "the frame had no content; ignore it". Don't reorder
# the id-parse before the empty-skip check.
# Do NOT yield, do NOT update last_sse_id, do NOT raise.
CONTINUE
CALL _parse_sse_id(sse.id) → SseId # existing; only runs for non-empty-data events
... existing turn_id checks ...
TRY:
body = json.loads(sse.data)
CATCH json.JSONDecodeError as exc:
# INV-002: malformed data is a protocol error; surface as named exception.
RAISE MalformedSseData(raw=sse.data)
event = _envelope_for_type(body, sse_id=sse_id)
YIELD event
... existing terminal-event handling ...
stream_turn ERROR_ROUTING amended:
MalformedSseData:
local_handling: log truncated raw `data:` payload; re-raise to caller
flow_control: abort
state_recovery: none (payload corruption; can't decode; consumer's call to reconnect)
(Mirrors the existing MalformedSseId entry shape.)
New TESTS entries on _iter_events / stream_turn:
empty_data_skipped [trace]: stream yields four frames in order —text(42:1),empty-data(42:2 — id present, data empty),text(42:3),done(42:4). Consumer yields exactly three Events:Text(42:1),Text(42:3),Done(42:4)— the empty-data frame at42:2is invisible to the caller. Per INV-003, intermediate state after consuming the secondText(i.e., immediately after yielding the event withsse_id=(42, 3)) islast_sse_id == (42, 3)— proves the skip did NOT advance through42:2. Final state after consumingDoneislast_sse_id == (42, 4)(the terminal event advances it as normal).malformed_data_raises [error]: stream yields text(42:1) then event withdata: not-json→ consumer yields one Text then raisesMalformedSseData(raw="not-json"). Iteration aborts.whitespace_data_raises [adversarial]: stream yields one event withdata:(single space) → consumer raisesMalformedSseData(whitespace is non-empty and JSON-parse-fails; the skip rule is empty-string only).malformed_data_truncation [security]: stream yields one event with 5000-char malformed data →MalformedSseData.rawis exactly the first 200 chars; exception message includes only the truncated form.
assumptions block adds: empty httpx_sse.ServerSentEvent.data
behavior across the library's parsing modes.
Issue #3 (ratatoskr.cli) amendments
_run_turn ERROR_ROUTING gains MalformedSseData:
MalformedSseData:
local_handling: write `[malformed_sse_data] raw={exc.raw!r}` to stderr
flow_control: abort
state_recovery: none (server-side wire bug)
exit_code: 22
New TEST:
malformed_sse_data [error]: mock yields one Text then an event withdata: not-json→_run_turnreturns 22; stderr[malformed_sse_data]is present;raw=...truncated.
Issue #4 (ratatoskr.tui) amendments
_stream_turn_worker ERROR_ROUTING gains MalformedSseData:
MalformedSseData:
local_handling: log.write(f"[malformed_sse_data] raw={exc.raw!r}") to transcript
flow_control: abort (the iteration aborts; finally-block restores state)
state_recovery: state → idle; footer hint reset (INV-008: mid-session errors do NOT exit the app)
New TEST:
malformed_sse_data_returns_to_idle [error]: mock yields text + event with malformed data → transcript has[malformed_sse_data]; state → idle; app does NOT exit.
Acceptance
- All three amended contracts (#1, #3, #4) drift-check clean.
- All existing tests + new
MalformedSseDatacoverage GREEN underuv run pytest tests/. uv run ruff check src/ tests/clean.- Boundary smoke
tests/test_no_worldtree_imports.pystill passes. - Regression smoke: re-run the long-conversation TUI smoke that
previously crashed (mimic the 2026-05-22 crash conditions — a turn
long enough that mimir emits ≥1000 events). Confirm no crash; the
conversation completes cleanly to
[done]. - The pre-existing happy-path smokes (mimir short responses) still work — empty-data skip is invisible when no empty-data events occur.