From ecdc52fce91d850dbc24946e9f1dfc04c70aed7d Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 21 Jun 2026 20:37:40 +0900 Subject: [PATCH] ticket: request changes on inflight snapshot --- .yoi/tickets/00001KVMT2J25/item.md | 2 +- .yoi/tickets/00001KVMT2J25/thread.md | 71 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/.yoi/tickets/00001KVMT2J25/item.md b/.yoi/tickets/00001KVMT2J25/item.md index 462e4319..b3773982 100644 --- a/.yoi/tickets/00001KVMT2J25/item.md +++ b/.yoi/tickets/00001KVMT2J25/item.md @@ -2,7 +2,7 @@ title: 'Pod protocol: in-flight LLM response reconnect snapshot should include unfinished blocks' state: 'inprogress' created_at: '2026-06-21T10:02:01Z' -updated_at: '2026-06-21T11:31:55Z' +updated_at: '2026-06-21T11:37:35Z' assignee: null readiness: 'implementation_ready' risk_flags: ['protocol', 'session-history', 'persistence', 'tui-reconnect', 'stream-state'] diff --git a/.yoi/tickets/00001KVMT2J25/thread.md b/.yoi/tickets/00001KVMT2J25/thread.md index 9f3f30bf..855b26f1 100644 --- a/.yoi/tickets/00001KVMT2J25/thread.md +++ b/.yoi/tickets/00001KVMT2J25/thread.md @@ -308,3 +308,74 @@ Reviewer focus: Orchestrator will wait for reviewer verdict before integration。 --- + + + +## Review: request changes + +Verdict: `request_changes` + +確認範囲: +- Ticket authority: + - `.yoi/tickets/00001KVMT2J25/item.md` + - `.yoi/tickets/00001KVMT2J25/thread.md` +- Diff `155e039e..74aca6f6`, including: + - `crates/protocol/src/lib.rs` + - `crates/pod/src/in_flight.rs` + - `crates/pod/src/controller.rs` + - `crates/pod/src/ipc/alerter.rs` + - `crates/pod/src/ipc/server.rs` + - `crates/pod/src/pod.rs` + - `crates/pod/src/segment_log_sink.rs` + - `crates/tui/src/app.rs` + - constructor/test updates in pod/TUI/discovery/spawn/dashboard-related test files + +Blocking issue: + +1. Snapshot/live boundary still has a gap for finalized assistant items committed between the session-log snapshot and the in-flight snapshot。 + +The implementation makes in-flight streaming deltas mostly gap-free by holding the in-flight mutex across subscribe/snapshot, but the full connection snapshot is not atomic across the committed session-log mirror and the new in-flight accumulator。 + +Relevant path: +- `crates/pod/src/ipc/server.rs:112` takes `handle.sink.subscribe_with_snapshot()` first。 +- `crates/pod/src/ipc/server.rs:118-120` then subscribes/snapshots alerts + in-flight state。 +- `crates/pod/src/pod.rs:182-191` persists an entry, clears matching in-flight state for `LogEntry::AssistantItem`, then publishes to the session-log sink。 +- `crates/pod/src/segment_log_sink.rs:90-103` documents that `AssistantItem` is mirror-only and not live-broadcast。 +- `crates/pod/src/segment_log_sink.rs:120-127` confirms only `SegmentStart`, `UserInput`, `SystemItem`, and `Invoke` are live-relevant。 + +Race: +1. New client connects。 +2. Server snapshots session-log entries at N。 +3. LLM response finalizes and appends `LogEntry::AssistantItem`。 +4. `append_entry` clears the matching in-flight text/thinking block。 +5. `SegmentLogSink::publish` mirrors the `AssistantItem`, but does not broadcast it live。 +6. Server snapshots in-flight state, now empty/cleared。 +7. Client receives `Event::Snapshot` with no finalized assistant item in `entries`, no unfinished/finalized in-flight block, and no live committed assistant event forthcoming。 + +This violates the Ticket invariant that Snapshot and post-Snapshot live events must be gap-free and duplicate-free。 + +Required fix: +- Make the connection snapshot atomic enough across session-log mirror and in-flight state, or otherwise ensure committed assistant items cannot disappear in this boundary。 +- Add an integrated regression test around session-log snapshot + in-flight snapshot + mirror-only assistant commit boundary。 +- Ensure the fix still avoids persisting unfinished output as final history and still does not duplicate content when final AssistantItem is included in committed snapshot。 + +Non-blocking concerns: +- The suggested focused test name `subscribe_with_alerts_and_in_flight_snapshot_splits_snapshot_and_live_deltas` selected 0 tests; no matching test exists。Add/rename an integrated focused test so validation can target the boundary reliably。 +- Minor protocol doc issue: `Greeting` doc comment appears displaced so “Pod self-description...” now sits immediately before `InFlightSnapshot` in `crates/protocol/src/lib.rs`。Not blocking but should be cleaned up if easy。 + +Reviewer validation: +- `git diff --check 155e039e..HEAD`: passed +- `cargo fmt --all --check`: passed +- `cargo test -p protocol`: passed, 40 tests +- `cargo test -p pod in_flight`: passed selected tests +- `cargo test -p pod subscribe_with_alerts_and_in_flight_snapshot_splits_snapshot_and_live_deltas`: selected 0 tests / no matching test +- `cargo test -p tui snapshot_in_flight_blocks_continue_with_live_deltas`: passed +- `cargo test -p tui`: passed, 367 tests +- `cargo check -p protocol -p pod -p tui`: passed +- `target/debug/yoi ticket doctor`: passed (`doctor: ok`) +- Full `cargo test -p pod`: failed only on the two known unrelated prompt-resource assertion tests。 + +Worktree status: +- Clean on `impl/00001KVMT2J25-inflight-snapshot`。 + +---