Skip to content

fix(viewer): harden conversation record inspection#8

Merged
siriusctrl merged 1 commit into
agent/conversation-jsonfrom
review/conversation-json
Jul 22, 2026
Merged

fix(viewer): harden conversation record inspection#8
siriusctrl merged 1 commit into
agent/conversation-jsonfrom
review/conversation-json

Conversation

@siriusctrl

@siriusctrl siriusctrl commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

This is an independent review/fix layer on top of the conversation JSON implementation in #7.

  • require explicit media context before collapsing sibling payloads, and validate standard versus URL-safe base64 alphabets separately
  • make nested typed tool results suppress duplicate envelope links while preserving explicit canonical fallbacks
  • retain exact lazy JSONL records in an immutable raw spool so source rewrites cannot change an open snapshot
  • force a full terminal redraw when switching between structured and raw content spaces
  • bound invalid UTF-8 boundary adjustment to at most three bytes

Root cause and impact

The original display paths mixed a few independent assumptions: payload characters could imply encoding, a role-tool envelope could compete with its typed child, lazy raw mode reread mutable source bytes, and terminal scroll deltas crossed content-space boundaries. Together those could produce false media summaries, incorrect tool navigation, non-repeatable raw records, or stale terminal rows after r toggles.

The fix keeps redirected output and tool arguments token-preserving, keeps media scanning streaming, and leaves all terminal lifecycle/rendering ownership outside fmtview-core.

Validation

  • focused adversarial suites: media transform 27, tool links 24, lazy/raw 4, headless viewer 8
  • cargo test --workspace --all-targets --no-fail-fast
  • cargo clippy --workspace --all-targets -- -D warnings
  • RUSTDOCFLAGS=-Dwarnings cargo doc --workspace --no-deps
  • cargo publish -p fmtview-core --dry-run --locked
  • 16 MiB media load smoke: 48.023 ms median, formatted spool held to 58 bytes
  • fresh Kitty/Xvfb flow plus a long-record r -> j x48 -> r recording; raw moved from 20% to 53% and the structured screen restored cleanly
  • final PTY capture: alternate screen and all mouse modes balanced 1:1, synchronized updates balanced 5:5, exact raw/structured screen markers passed

Local durable evidence is under target/fmtview-emulator-recordings/raw-scroll-review-d80e725-3/: Kitty video/contact sheet and text snapshots, plus pty.log, pty-check.json, pty-raw-screen.txt, and pty-structured-screen.txt.

Documented streaming limitations

  • sibling metadata must precede data for streaming recognition
  • escaped data-URI headers remain uncollapsed

Raw search also intentionally does not match across the artificial 32 KiB chunk seam.

Blockers addressed:
- require explicit media typing and validate standard versus URL-safe alphabets
- suppress duplicate tool-result envelopes while preserving canonical fallbacks
- snapshot lazy raw records so source rewrites cannot change an open record
- force a full redraw when switching structured and raw content spaces

Invariants preserved:
- redirected output and tool arguments remain token-preserving
- media payloads stay streaming and decoded-size-only in the display spool
- raw record reads remain bounded to 32 KiB plus at most 3 UTF-8 boundary bytes
- terminal lifecycle stays outside fmtview-core
@siriusctrl
siriusctrl marked this pull request as ready for review July 22, 2026 09:10
@siriusctrl
siriusctrl merged commit feccae4 into agent/conversation-json Jul 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant