feat(sse_client,cli,tui): implement issue #7 — empty-data skip + MalformedSseData

Bundles initial TDD impl + Volva-code-review F1/F3 amendments.

sse_client.py:
- New MalformedSseData(raw) exception; truncates raw to 200 chars at
  __init__ (mirrors MalformedSseId.raw[:64] precedent).
- _iter_events gains `if sse.data == '': continue` BEFORE
  _parse_sse_id. Empty-data frames are silently skipped per issue #7
  INV-001 (keepalive semantics). Empty-data + bad-id is still a
  keepalive; intentional ordering, don't reorder.
- _iter_events json.loads(sse.data) now wrapped — JSONDecodeError →
  MalformedSseData(raw=sse.data).

cli.py:
- Imports MalformedSseData; _run_turn ERROR_ROUTING gains the case →
  stderr `[malformed_sse_data] raw={exc.raw!r}` + exit 22 (protocol-
  failure bucket, same as MalformedSseId/TurnIdFlip).

tui.py:
- Imports MalformedSseData; _stream_turn_worker ERROR_ROUTING gains
  the case → transcript label; finally block restores state→idle
  per INV-008 (mid-session errors don't exit the app).

Tests (6 new):
- test_sse_client.py: empty_data_skipped (tracer — 4 frames in, 3
  events out), malformed_data_raises, whitespace_data_raises,
  malformed_data_truncation, AND empty_data_skip_preserves_last_seen_sse_id
  (F1 from Volva code-review — drop-after-empty probes internal
  last_sse_id non-advancement via SseConnectionDropped.last_seen_sse_id).
- test_cli.py: malformed_sse_data (tightened to assert exact
  `[malformed_sse_data] raw='not-json'` shape per F3),
  malformed_sse_data_truncation (5000-char payload — verifies
  truncation carries through presenter rendering, F3).
- test_tui.py: malformed_sse_data_returns_to_idle (state→idle per
  INV-008; app does NOT exit).

Smoke validation (2026-05-22): the original crashing prompt
("what about system 1 and system 2 framing?") now completes cleanly
end-to-end. mimir streamed 3193 tokens (50 seconds, 374980-token
context), `[done] turn_id=96 duration_ms=50436`. Empty-data frames
somewhere in the stream silently skipped; no crash.

172/172 tests GREEN; ruff clean; all 5 issue contracts (#1, #3, #4,
#5, #7) drift-check clean.

Persistent-memory updated per the commit-along rule: status reflects
v0+#7 milestone; new dated decisions for #5/#6/#7 filing + #7
implementation; foot-gun entry for unguarded json.loads(sse.data).
This commit is contained in:
vh
2026-05-22 16:41:38 -07:00
parent 7028c5bc11
commit c713208585
7 changed files with 279 additions and 29 deletions
+40 -28
View File
@@ -1,6 +1,6 @@
# Persistent memory — ratatoskr
_Last updated: 2026-05-21_
_Last updated: 2026-05-22_
This file captures durable intent and supporting evidence (goals, decisions,
foot-gun warnings, in-flight state) across context resets. Read it at session
@@ -34,39 +34,48 @@ separate dev team rather than an in-tree Worldtree tool.
_As of 2026-05-21:_
**Status: v0 milestone landed.** Four issues complete end-to-end against
their contracts: `ratatoskr.sse_client` (#1), `ratatoskr.sessions` (#2),
`ratatoskr.cli --send` non-interactive presenter (#3), `ratatoskr.tui` shell
(#4). 164/164 tests GREEN (42 sse_client + 19 sessions + 58 cli + 43 tui +
2 boundary/metadata); ruff clean; all four issue contracts drift-check
clean. `ratatoskr` console-script entry point dispatches `--send` → cli
(stdout) and bare invocation → TUI (lazy textual import preserves issue #3
INV-001). All work pushed to `origin/main`.
**Status: v0 milestone + issue #7 (mid-stream robustness) landed.** Four
core issues complete end-to-end (`sse_client` #1, `sessions` #2, `cli`
#3, `tui` #4) + one robustness fix on top: `MalformedSseData` exception
+ empty-data skip in `_iter_events` (issue #7, fixes mid-stream
JSONDecodeError crash discovered during long TUI conversations).
172/172 tests GREEN (46 sse_client + 19 sessions + 60 cli + 44 tui +
2 boundary/metadata + 1 from #7 last-seen probe); ruff clean; all five
issue contracts (#1, #3, #4, #5, #7) drift-check clean.
`--send` validated end-to-end against personal Worldtree
(`http://10.250.50.152:8081`, mimir agent on qwen3.6-35-a3b). Smoke key
lives in gitignored `env.sh` (delivered by infra-ops 2026-05-21; see
[[personal-worldtree-smoke-target]] in auto-memory).
(`http://10.250.50.152:8081`, mimir on qwen3.6-35-a3b). Smoke key in
gitignored `env.sh` (delivered by infra-ops 2026-05-21; see
[[personal-worldtree-smoke-target]] in auto-memory). Long-conversation
smoke 2026-05-22 (3193-token completion, 374980-token context, 50s
streaming) confirmed empty-data frames are silently skipped — the
original 2026-05-22 crash unreproducible after fix.
**Nothing in flight.** Branch: `main` (clean). Remote: `origin →
git@gitea.phasefinal.com:vh/ratatoskr.git`.
**In-flight: issue #5 (`--end-user-id` flag for per-user agents like
lofn).** Contract drafted + parked (untracked at
`docs/contracts/issues/5.contract.md`); Volva paraphrase + TDD + smoke
pending. Issue #6 (TUI startup error visibility) also filed but
unscaffolded. Branch: `main` (clean apart from #5 contract). Remote:
`origin → git@gitea.phasefinal.com:vh/ratatoskr.git`.
**Next natural moves:**
1. **TUI smoke (operator-side)** — `source env.sh && uv run ratatoskr --new
--agent mimir` from an interactive terminal. Validates the Textual app
lifecycle + post-Done markdown re-render end-to-end. Needs a TTY which
CC sessions don't have.
2. **Side-pane issues** — design-brief §5 lists 5 side panes (Persona,
Tools, AdminEvents, BifrostState, ServerLog). Each gets its own issue
+ contract + TDD pass on top of the shell. Persona is the natural first
(file-tail of `persona.log` — cheap; no new Worldtree wire).
3. **Recorded SSE snapshot fixtures** from a running Worldtree. `--send
--new` redirected to a fixture file IS the recording probe — capture
outputs to `tests/snapshots/` for replay-based regression coverage.
The smoke proved the recording probe works.
4. **Startup session picker** — design-brief §4 `DataTable` of `GET
/sessions`. Modest scope; pairs naturally with the TUI shell.
1. **Resume issue #5 cadence** — `--end-user-id` flag for per-user agents
(lofn). Contract drafted + parked; next steps: Volva paraphrase, TDD,
smoke against lofn. Unblocks the Persona/Vili agent family.
2. **Issue #6 (TUI startup error visibility)** — filed, unscaffolded.
Restructure: move session-create out of `on_mount` into `run_tui`
pre-App.run() so errors print to real stderr (not the alt-screen that
tears down before user can read them). Independent of #5.
3. **TUI smoke (operator-side)** — `source env.sh && uv run ratatoskr
--new --agent mimir` from an interactive terminal. Validates Textual
app lifecycle + post-Done markdown re-render end-to-end. Needs a TTY
which CC sessions don't have.
4. **Side-pane issues** — design-brief §5 lists 5 side panes (Persona,
Tools, AdminEvents, BifrostState, ServerLog). Persona is the natural
first (file-tail of `persona.log`).
5. **Recorded SSE snapshot fixtures** from a running Worldtree.
`--send --new > fixture` IS the recording probe.
## Recent decisions
@@ -89,6 +98,8 @@ decision. Captures rationale that won't be obvious from code alone.
- `[2026-05-21]` **Default issue-tracker labels seeded** (17 total). Sleipnir gating, triage, type, resolution, Ratatoskr-specific area labels (sse-client, tui, cli, observability).
- `[2026-05-21]` **Volva paraphrase + code-review across all 4 issues — calibration consistent.** Paraphrase rounds flag 3-5 contract ambiguities per issue; code-review rounds flag 3-8 code-vs-contract drifts after TDD-passing implementation. Hit rates: #1 paraphrase 3-of-5 amended / code-review 4 findings; #2 3-of-5 / 3 findings; #3 5-of-5 / 5 findings; #4 5-of-5 / 8 findings. The post-TDD code-review consistently catches three classes of gap the test-author's hypotheses don't cover: PRE-assertion boundary drift, exception-payload truncation / never-rendered-to-user observability misses, and "tested the state but not whether the user can see it" gaps (issue #4's primary finding: TUI footer state stored but never rendered to a visible widget — same-model TDD would systematically miss this).
- `[2026-05-21]` **Manual smoke is load-bearing — found a real defect tests couldn't.** First wire-level smoke against personal Worldtree (post-TDD, post-Volva-code-review on #4) revealed httpx's default 5s read timeout killed the SSE connection mid-stream during mimir's thinking phase (~30s LLM latency >> 5s read timeout). The unit/contract test infrastructure (respx-mocked SSE wire) doesn't model real LLM latency, so the gap was invisible at the test layer. Fix: caller-owned `httpx.AsyncClient` constructed with `timeout=httpx.Timeout(connect=10.0, read=None, write=10.0, pool=10.0)`; defense in depth: `sse_client.stream_turn` ERROR_ROUTING catches `httpx.ReadTimeout` → `SseConnectionDropped`. Three contracts amended in-place to document the timeout policy. **Lesson: keep manual-smoke step in the per-issue cadence; mock-only validation is insufficient for streaming-against-real-server code.** Re-smoke succeeded: `[done] turn_id=88 model=qwen3.6-35-a3b duration_ms=2351`. Wire-compat envelope (personal v0.16.2 vs ratatoskr's v0.19.0 pin) confirmed end-to-end.
- `[2026-05-22]` **Issues #5/#6/#7 filed: per-user-agent support + TUI-startup-visibility + mid-stream-robustness.** Discovered during 2026-05-22 mimir TUI conversation: long completion (turn 93, 1077 events consumed) crashed with `JSONDecodeError("Expecting value: line 1 column 1 (char 0)")` from `json.loads('')` on an empty-`data:` SSE frame. Diagnosis surfaced #7 (the crash). Earlier same day, `ratatoskr --new --agent lofn` failed with 422 `end_user_id_required` — surfacing #5 (`--end-user-id` flag needed for per-user agents). #6 (TUI alt-screen masks the diagnostic before user can read it) was a corollary observation. All three filed; user reordered to #7 first (highest-impact for daily TUI use).
- `[2026-05-22]` **Issue #7 (`MalformedSseData` + empty-skip) implemented via TDD + Volva-code-reviewed + smoked.** Contract → Volva paraphrase (4 ambiguities, all amended; INV-001 wording tightened around exact `sse.data == ''` rule, ordering-before-id-parse made explicit, test-description bug fixed) → TDD (6 tests, full vertical-slice ordering) → Volva code-review (3 findings — F1 test-gap probing internal `last_sse_id` non-advancement via post-skip drop, F2 contract precision around log-vs-propagate responsibility, F3 cli test tightening for `raw='X'` shape + truncation coverage; all amended) → smoke (3193-token completion against personal Worldtree confirmed clean termination; original crash unreproducible). **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: the tighter the contract spec + the smaller the code surface, the more Volva's role shifts from "catch behavioral drift" to "tighten observability + wording". Calibration table now: #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).
_For per-issue TDD implementation notes, Volva findings, and contract amendments, see the git log (commits `9703eb2..61c3941` carry the full per-issue trail with structured commit messages)._
@@ -104,3 +115,4 @@ defense against re-attempting the same cul-de-sac.
- `[2026-05-21]` **RichLog widget with `markup=True`.** Default impulse, but Rich interprets `[xxx]` spans as style markup and silently strips them. Every labeled stderr-style line — `[cancel_failed]`, `[done]`, `[error]`, `[busy]`, `[worker_phase]` — would render as just the content after the bracketed label, breaking the user-visible observability surface. Fix: `markup=False`. The post-Done Markdown rendering still works because `rich.markdown.Markdown` is a Renderable that ignores widget-level markup setting. Don't flip back to `markup=True` without first renaming every labeled-line format away from `[bracket]` notation.
- `[2026-05-21]` **Querying `self.query_one("#transcript", RichLog)` from inside a Textual `run_worker` coroutine.** Failed initially with `NoMatches` because the worker fires before the test's `pilot.pause()` allows the Input.Submitted handler to fully dispatch (and thus the widget tree to settle). Initial reactive fix: widen worker signature to take `log` as a parameter (passed from the handler). Volva code-review flagged this as contract drift (signature didn't match spec). Reverted to single-param signature. The real fix was test-side: add `await pilot.pause()` between `inp.action_submit()` and the polling loop in `_submit_and_wait` so the handler finishes dispatching before the worker reads the widget tree. Don't widen worker signatures to dodge test timing.
- `[2026-05-21]` **TUI session-identity rendering via `self.sub_title` + `self.hint` plain attributes.** Stored state but never rendered to a visible widget. The contract's "session-identity-always-visible" invariant was satisfied at the state-attribute level but not the user-visible-widget level. Tests asserted the attributes (which passed); Volva code-review flagged the gap. Fix: dedicated `Static(id="identity")` + `Static(id="hint")` widgets in compose; `_set_hint()` helper mirrors state → widget. Calibration evidence for the "TDD catches state, code-review catches whether the user can see it" pattern.
- `[2026-05-22]` **`json.loads(sse.data)` unguarded against empty data.** `_iter_events` unconditionally called `json.loads` on every dispatched `ServerSentEvent`. When `httpx_sse` surfaced a frame with `id:` present but `data:` empty (a known library-vs-spec divergence — RFC says don't dispatch; httpx_sse is permissive), `json.loads('')` raised `JSONDecodeError` → propagated through Textual's worker → app crash. Crashed mimir conversation at turn 93/seq 1078 after 1077 successful events. Fix: `if sse.data == '': continue` BEFORE `_parse_sse_id` (empty-data event with a malformed id is still a keepalive — don't reorder). Non-empty malformed data raises new `MalformedSseData(raw[:200])`. Don't reintroduce unconditional `json.loads(sse.data)`; always pre-check for the empty case.