Skip to content

fix(inspect): bound batches across writer races#5

Merged
siriusctrl merged 2 commits into
agent/inspect-transcriptfrom
review/inspect-transcript-fresh
Jul 22, 2026
Merged

fix(inspect): bound batches across writer races#5
siriusctrl merged 2 commits into
agent/inspect-transcriptfrom
review/inspect-transcript-fresh

Conversation

@siriusctrl

@siriusctrl siriusctrl commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • preflight the next checkpoint before a non-empty forward batch crosses its record or byte budget
  • keep whole checkpoints atomic without eagerly decoding or formatting the following oversized group
  • retry confirmed truncate/replace races and publish refresh state only after one coherent file observation
  • upgrade the published crates.io fmtview facade to 0.6.2 so tail-first follow stays attached on its first pretty-JSON frame
  • add regressions for oversized groups, malformed starts, buffered-reader position, and concurrent pending-tail replacement

Root causes

The forward reader checked its budget only after decoding a checkpoint. A small checkpoint followed by a large group therefore decoded and returned both, crossing the requested bound and doing large eager work.

The refresh path also mutated its suffix decoder while reading a length captured before a concurrent writer truncate/replace. A short read could terminate follow mode and leave partial scan state behind.

Finally, fmtview 0.6.1 sticky-layout convergence let the one-draw preserved position override the exact active-follow tail when a JSON breadcrumb changed first-frame height. Upstream fmtview #15 fixed that issue and released it as 0.6.2.

Dependency boundary

Cargo.toml depends only on the published crates.io fmtview = "0.6.2" facade. There is no path dependency, Git dependency, or direct fmtview-core dependency; the locked core crate is registry-transitive only.

Validation

  • cargo fmt --check
  • cargo check --all-targets
  • cargo clippy --all-targets -- -D warnings
  • cargo test --all-targets — 300 passed, 0 failed, 1 ignored
  • git diff --check
  • million-record acceptance from the same review branch before the dependency-only bump: passed in 15.54s test time / 18.06s wall, 90,184 KiB maximum RSS; bounded tail-open instrumentation remained below 512 KiB

Fresh post-0.6.2 PTY checks pass for:

  • first frame is tail-first with follow:on
  • incomplete checkpoints remain hidden, then appear atomically and auto-follow after commit
  • Up detaches without append jumps; G reattaches
  • f pause/resume preserves the viewport and later catches up
  • a new RunDirStore invokes prepare_append, replaces an uncommitted tail, and exposes the replacement exactly once without discarded records or duplicate refs
  • nested tool call/result t navigation round-trips
  • r shows the exact {"cmd":"cargo test --all-targets"} source argument
  • q exits zero and restores alternate screen, mouse capture, cursor visibility, and byte-for-byte PTY termios state

Fresh Kitty/Xvfb/ffmpeg evidence also covers those visible states. The 25.07-second real-emulator recording has twelve key screenshots, ten sampled nonblank frames, a contact sheet, and a video timeline sheet. All recorder, emulator, display, and tmux processes were cleaned up.

Local ignored evidence bundle:

/root/.codex/worktrees/fiasco-inspect-fresh-review/target/fiasco-inspect-follow-review-v2/

Preflight the next checkpoint before a non-empty forward batch crosses its record or byte budget. This keeps large committed groups atomic without decoding or formatting them until a caller has room, while still validating malformed checkpoint starts.

Treat refresh reads as a coherent transaction over one file observation. Retry confirmed truncate/replace races, discard partial suffix decoder state, and publish offsets and prefix samples only after the observed file remains stable.

Cover oversized forward groups, malformed preflight data, buffered-reader position restoration, and deterministic concurrent tail replacement. Document the checkpoint-budget and refresh snapshot semantics.
Upgrade the published crates.io fmtview dependency to 0.6.2. Upstream PR #15 fixed sticky-layout convergence where the one-draw preserved position overrode the exact active-follow tail after a JSON breadcrumb changed the first frame height, immediately producing follow:detached.

Keep fiasco behind the public fmtview facade; fmtview-core remains registry-transitive only, with no path or Git dependency.
@siriusctrl
siriusctrl marked this pull request as ready for review July 22, 2026 13:37
@siriusctrl
siriusctrl merged commit 6bb2406 into agent/inspect-transcript Jul 22, 2026
2 checks 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