From 0dfdd11921a8f2317d3d002df582fdd075a1f411 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:29:37 +0900 Subject: [PATCH] update: session-grouping review follow-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PickerOutcome::Picked から未使用の session_id を除去（pod-cli が lookup_session_of で再解決） - picker preview が singular AssistantItem も拾うように - fs_store layout doc に migration（後方互換なし、旧 flat sessions は破棄）を明記 - TaskStore は Session-lifetime、ScopedFs/Tracker は Pod-process lifetime と用語整理 - Pod::session_id / from_manifest_spawned のコメント補強 --- crates/pod/src/pod.rs | 6 +- crates/session-store/src/fs_store.rs | 8 ++ crates/tools/src/lib.rs | 14 +-- crates/tools/src/task.rs | 9 +- crates/tools/src/tracker.rs | 12 ++- crates/tui/src/main.rs | 2 +- crates/tui/src/picker.rs | 18 ++-- tickets/session-grouping-introduce.md | 6 ++ tickets/session-grouping-introduce.review.md | 106 +++++++++++++++++++ 9 files changed, 155 insertions(+), 26 deletions(-) create mode 100644 tickets/session-grouping-introduce.review.md diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index db155db1..def8d557 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -570,7 +570,9 @@ impl Pod { } /// The Session this Pod belongs to. Stable across compaction and - /// in-Session fork — only `fork` (a brand-new Session) changes it. + /// auto-fork (both stay within the same Session); there is no + /// Pod-level operation today that moves a running Pod to a different + /// Session. pub fn session_id(&self) -> SessionId { self.segment_state.session_id() } @@ -2864,6 +2866,8 @@ impl Pod, St> { let mut common = prepare_pod_common(&manifest, &loader, /* parse_template */ true)?; let skill_shadows = std::mem::take(&mut common.skill_shadows); + // A spawned child starts its own conversation, so it mints a + // fresh Session rather than joining the spawner's. let session_id = session_store::new_session_id(); let segment_id = session_store::new_segment_id(); let scope_allocation = pod_registry::adopt_allocation( diff --git a/crates/session-store/src/fs_store.rs b/crates/session-store/src/fs_store.rs index 076455e0..a689f902 100644 --- a/crates/session-store/src/fs_store.rs +++ b/crates/session-store/src/fs_store.rs @@ -6,6 +6,14 @@ //! //! The per-Session directory makes `list_segments(session_id)` an O(dir) //! scan and gives the fork tree a visible grouping in the filesystem. +//! +//! Migration: this layout is incompatible with the pre-`session-grouping` +//! flat `{root}/{segment_id}.jsonl` form. Project policy is no +//! backward compatibility — discard `~/.insomnia/sessions/` (or whatever +//! `root` resolved to) before running the new code. `list_sessions` +//! ignores top-level files outside session directories, so leftover +//! flat files do not corrupt new sessions, but they are no longer +//! enumerable by the picker. use crate::event_trace::TraceEntry; use crate::segment_log::LogEntry; diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index f4af7a39..6e947121 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -4,12 +4,12 @@ //! `llm-worker` `Tool` infrastructure. Filesystem access is mediated by //! two orthogonal concerns: //! -//! - [`ScopedFs`] — pod-lifetime, expresses the write-block boundary for -//! the current scope. Derived from the manifest and shareable across -//! sessions. -//! - [`Tracker`] — session-lifetime, enforces the "read before edit" +//! - [`ScopedFs`] — Pod-process lifetime, expresses the write-block +//! boundary for the current scope. Derived from the manifest; not +//! persisted across Pod restart. +//! - [`Tracker`] — Pod-process lifetime, enforces the "read before edit" //! policy via content hashes and tracks the recency of touched files. -//! Recreated fresh per session. +//! Recreated fresh on each Pod start (including resume). //! //! The Pod layer owns both instances and passes them to //! [`builtin_tools`] when registering tools on a `Worker`. @@ -42,11 +42,11 @@ pub use tracker::Tracker; pub use write::write_tool; /// Register all builtin tools, wiring them to a shared `ScopedFs` -/// (pod-lifetime) and `Tracker` (session-lifetime). +/// (Pod-process lifetime) and `Tracker` (Pod-process lifetime). /// /// All returned factories share the same tracker instance so that /// `Read` / `Write` / `Edit` see a consistent history across tool -/// invocations within a single session. +/// invocations within a single Pod run. /// /// `bash_output_dir` is where the Bash tool spills long outputs. The /// caller is responsible for adding that path to the readable scope diff --git a/crates/tools/src/task.rs b/crates/tools/src/task.rs index 32ddd461..3b36f924 100644 --- a/crates/tools/src/task.rs +++ b/crates/tools/src/task.rs @@ -1,8 +1,9 @@ -//! Pod-lifetime TaskStore and builtin task tools. +//! Session-lifetime TaskStore and builtin task tools. //! -//! The store is Pod/session-lifetime state shared by the four Task* tools. It -//! is reconstructed on resume by replaying TaskCreate / TaskUpdate tool-call -//! arguments from persisted history. +//! The store survives compaction and Pod restart — it is reconstructed +//! on resume by replaying TaskCreate / TaskUpdate tool-call arguments +//! from persisted history, so its effective lifetime is the +//! [`session_store::SessionId`] (the conversation), not the Pod process. use std::sync::{Arc, Mutex}; diff --git a/crates/tools/src/tracker.rs b/crates/tools/src/tracker.rs index a299a84e..65197b23 100644 --- a/crates/tools/src/tracker.rs +++ b/crates/tools/src/tracker.rs @@ -18,11 +18,13 @@ //! //! # Lifetime //! -//! A `Tracker` is **session-scoped**: the Pod layer creates a fresh -//! instance at the start of each agent session and discards it when the -//! session ends. The `ScopedFs` write boundary, by contrast, is -//! pod-lifetime (derived from the manifest). The two are orthogonal and -//! the Pod wires them together when registering builtin tools. +//! A `Tracker` is **Pod-process scoped**: the Pod layer creates a fresh +//! instance at the start of each Pod run (including resume) and discards +//! it when the process exits — it is not persisted, so a resumed +//! conversation starts with an empty read/edit history. The `ScopedFs` +//! write boundary is likewise Pod-process scoped (derived from the +//! manifest). The two are orthogonal and the Pod wires them together +//! when registering builtin tools. //! //! ```no_run //! # use std::path::PathBuf; diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 4fed5d70..6b3cbe37 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -210,7 +210,7 @@ async fn run_resume() -> Result<(), Box> { // viewport before the name dialog opens so each phase gets fresh // vertical room. let leaf_segment_id = match picker::run().await? { - PickerOutcome::Picked { segment_id, .. } => segment_id, + PickerOutcome::Picked { segment_id } => segment_id, PickerOutcome::Cancelled => return Ok(()), }; run_spawn(Some(leaf_segment_id)).await diff --git a/crates/tui/src/picker.rs b/crates/tui/src/picker.rs index 51054192..024aae85 100644 --- a/crates/tui/src/picker.rs +++ b/crates/tui/src/picker.rs @@ -62,11 +62,10 @@ impl From for PickerError { } pub enum PickerOutcome { - /// User picked a session; resume at its leaf segment. - Picked { - session_id: SessionId, - segment_id: SegmentId, - }, + /// User picked a session; resume at its leaf segment. The pod-cli + /// rehydrates `session_id` via `Store::lookup_session_of` so we only + /// need to surface the segment here. + Picked { segment_id: SegmentId }, Cancelled, } @@ -134,10 +133,8 @@ pub async fn run() -> Result { } Some(Action::Submit) => { close_viewport(&mut terminal)?; - let row = &rows[selected]; return Ok(PickerOutcome::Picked { - session_id: row.session_id, - segment_id: row.leaf_segment_id, + segment_id: rows[selected].leaf_segment_id, }); } Some(Action::Cancel) => { @@ -200,6 +197,11 @@ fn last_message_preview(entries: &[LogEntry]) -> Option { return Some(format!("user: {}", trim_one_line(&text, 60))); } } + LogEntry::AssistantItem { item, .. } => { + if let Some(text) = first_text_logged(item) { + return Some(format!("assistant: {}", trim_one_line(&text, 60))); + } + } LogEntry::AssistantItems { items, .. } => { if let Some(text) = items.iter().find_map(first_text_logged) { return Some(format!("assistant: {}", trim_one_line(&text, 60))); diff --git a/tickets/session-grouping-introduce.md b/tickets/session-grouping-introduce.md index 76d0dbb7..89e1383f 100644 --- a/tickets/session-grouping-introduce.md +++ b/tickets/session-grouping-introduce.md @@ -46,3 +46,9 @@ - `tickets/live-fork-marker.md` - `tickets/pod-state-backend.md` - `crates/session-store/` + +## Review + +- 状態: Approve with follow-up +- レビュー詳細: [./session-grouping-introduce.review.md](./session-grouping-introduce.review.md) +- 日付: 2026-05-20 diff --git a/tickets/session-grouping-introduce.review.md b/tickets/session-grouping-introduce.review.md new file mode 100644 index 00000000..e2ccc60b --- /dev/null +++ b/tickets/session-grouping-introduce.review.md @@ -0,0 +1,106 @@ +# Review: session-store: Session（Segment 群の grouping）導入 + +対象コミット: `a6cc05f feat: Session（Segment 群の grouping）を導入` +ベース: `develop` 上の `e4cda5d Merge: segment-rename` + +## 前提・要件の確認 + +- **新 `SessionId` 型を `session-store` に追加。Segment は `parent_session_id` を持つ**: + 達成。`SessionId` 型 alias と `new_session_id()` が + `crates/session-store/src/lib.rs:65-73`。 + `LogEntry::SegmentStart { session_id, .. }` が + `crates/session-store/src/segment_log.rs:43`（serde 上は必須フィールド、`#[serde(default, skip_serializing_if = ..)]` の付かない裸の `session_id`）。`RestoredState.session_id: Option` は + `segment_log.rs:202` に追加され、`collect_state` で `SegmentStart` から拾われる（`segment_log.rs:258`）。 + +- **Segment 生成パスの session_id 決定（new / compaction / fork）**: + - new session: `create_segment` (`segment.rs:30`)、`Pod::new` (`pod.rs:495`)、`Pod::from_manifest` (`pod.rs:2786`)、`from_manifest_spawned` (`pod.rs:2867`) が `new_session_id()` を発行。`session_store::fork` (`segment.rs:406`) も同じく新発行。 + - compaction: `create_compacted_segment` (`segment.rs:75`) と `Pod::compact` (`pod.rs:2188`) が `source_session_id` を継承。 + - in-Session fork: `fork_at` (`segment.rs:454`) と `ensure_segment_head` の auto-fork (`pod.rs:1689`) が `source_session_id` を継承。 + - `Pod::from_manifest_spawned` は spawn 元と独立した新 Session を発行 → child Pod は親 Pod と別 conversation 扱い。設計上の明確化は欲しいが、本 ticket の要件範囲では妥当。 + +- **`SessionOrigin` の整理（`compacted_from` / `forked_from`、同 Session 内 segment への参照）**: + 実装は `SegmentOrigin { segment_id, at_turn_index }` を保持し（`segment_log.rs:181`）、両 origin は `LogEntry::SegmentStart` の `forked_from` / `compacted_from` フィールドに入る（`segment_log.rs:50, 54`）。同 Session 内参照は **コード経路で保証**（`create_compacted_segment` / `fork_at` の構築時 `source_session_id` 引数を経由する形）であって、**型レベルでは保証されていない**。ticket 要件「型レベルで保証」とは厳密には乖離する（非ブロッキング、後述）。 + +- **restore API を `(SessionId, SegmentId)` に**: + 達成。`Store::*` 全 method が `(session_id, segment_id)` を取る (`store.rs:46-99`)。`restore` (`segment.rs:97`) も同様。`SegmentId` のみを取る legacy shim は `restore_by_segment` (`segment.rs:111`) として一段噛ませてあり、内部で `Store::lookup_session_of` を呼ぶ。pod-cli `--session` (`pod/src/main.rs:188`) と TUI の `load_resume_scope` (`tui/src/spawn.rs:332`) がこの shim を経由している。 + +- **FsStore layout に Session 単位の index**: + 達成。`//.jsonl` で Session ディレクトリ配下に Segment file を並べる (`fs_store.rs:5-8`)。`list_segments(session_id)` (`fs_store.rs:123`) は当該 Session ディレクトリ scan で済むので `WHERE session_id = ?` 相当。 + +- **Session 内の leaf Segment 列挙 API**: + `Store::list_segments(session_id)` (`store.rs:63`) が UUID v7 の lexicographic 順（時系列順）で最新→古い順に返す (`fs_store.rs:142`)。leaf は先頭 1 件で取れる（picker で実際にそうしている: `picker.rs:96-101`）。専用の `latest_segment_of` 等は無いが、`list_segments(sid).first()` で十分な最小実装。 + +- **migration 戦略**: + ticket 要件は「既存 sessions ディレクトリは破棄して良いかどうかをここで明示」。実装は新 layout (`//.jsonl`) に切り替え、旧 flat `/.jsonl` は `list_sessions` (`fs_store.rs:103-121`) の dir-type filter で自動的に skip されて見えなくなる。即ち「実質破棄」されている。**コード上の comment ではこの方針が明示されておらず**（`fs_store.rs:1-8` の Layout doc には触れていない）、ticket 完了条件「明示」が docs/code どちらにも乗っていない点で部分達成。 + +- **`cargo check --workspace` + 全テスト pass**: + 確認。`cargo check --workspace` は `Finished dev` まで通る。`cargo test --workspace -- --skip double_run_returns_error` も全 suite green（warning 6 件、後述 nits）。`double_run_returns_error` は KNOWN_ISSUES.md 既登録の flake。 + +## アーキテクチャ・スコープ + +- **層境界の維持**: 新 `SessionId` 型は session-store の最下層に置かれ、pod / pod-cli / TUI / pod-registry / session-metrics が同じ型を利用する形。`llm-worker` には Session 概念が滲んでいない（cache-key として segment_id を渡すのみ）。低レベル基盤としての session-store の役割を保っている。 +- **`SegmentLocation` の atomic swap 設計**: `(session_id, segment_id)` を `ArcSwap` で一括 swap (`pod.rs:48-94`)。fork / compaction で session_id と segment_id を**同時に**更新する必要があるため、別々の atomic 変数に分けると中間状態を観測される。1 struct で wrap した設計は妥当。`set_segment_id` の置き換え (`set_location`) もシグネチャ上 (session_id, segment_id) ペアを取るので安全。 +- **`SegmentOrigin` の同 Session 性**: 上述の通り型レベルでは保証されない（`SegmentOrigin` 単体は `(segment_id, at_turn_index)` のみ）。妥当性を以下で評価: + - もし型レベルで保証するなら `SegmentOrigin { session_id, segment_id, at_turn_index }` にし、`SegmentStart.session_id == forked_from.session_id` の比較を `Deserialize` 後に走らせる方向、あるいは phantom-typed の compile-time invariant が必要。前者は redundant な persistence、後者は Rust の表現力で書くのが重く割に合わない。 + - 現実問題、`SegmentOrigin` の構築点は `create_compacted_segment` / `fork_at` / `ensure_segment_head` の 3 経路に限定され、いずれも source の session_id を継承するルートのみ通る。新規 callsite が追加されない限り invariant は破れない。CLAUDE.md の「設計を歪めない」原則と整合する範囲では、現状の構築時保証 + doc comment 明示で実用上十分。 + - ticket 要件文言「型レベルで保証」は厳密にはアンメットだが、実装方針として **「同 Session 性は構築時保証、doc で invariant 明示」** に倒した判断は妥当。ticket 文言を改めるか、現状を accept するかは user 判断。 +- **`Pod::from_manifest_spawned` の session_id 新規発行**: spawn された child Pod が親と独立した Session を持つのは、conversation grouping の意味論として妥当（child Pod は別 conversation を成す）。ただし spawn 元の親子関係を Session 永続表現が持たないと、後で「child の Session を親の Segment と紐付ける」操作（spawn linkage）が必要になる可能性がある。範囲外。 +- **`lookup_session_of` の O(N) ディレクトリ走査**: shim 用途のみで、pod-cli 起動時に 1 回、TUI resume 時に 1 回呼ばれる程度。Session 数が極端に多い（10^5 オーダー）と起動時 latency に効くが、ユーザーが実際に保持する Session 数（典型 10^2 オーダー）では問題にならない。picker は使わない（picker は session_id を直接持っている）。実装はディレクトリ存在 check で短絡 (`fs_store.rs:147`) しているので最悪ケースも軽い。妥当。 +- **TUI picker の Session 単位列挙への変更**: 以前は Segment 単位列挙 → 各 row 1 segment。現在は Session 単位 + 各 Session の leaf segment（最新）のみ preview。compaction で複数 Segment ができている Session でも 1 row。これは「同じ会話の家系」をユーザーが認識する単位として正しい UX。ticket の「Session 内の leaf Segment 列挙 API を提供」と整合（picker 経由で実際に leaf を選んで resume している）。 +- **過剰実装の確認**: live-fork marker、Pod 単位 metadata、TUI Session/Segment 選択 UI、DB backend は範囲外。いずれも実装に踏み込んでおらず、最小実装に留まっている。 +- **依存追加なし**: 既存型 (`uuid::Uuid`、`ArcSwap`) のみで構築。`cargo add` ポリシー違反なし。 + +## 指摘事項 + +### Blocking + +- なし。 + +### Non-blocking / Follow-up + +- **`SegmentOrigin` の同 Session 性が型レベルでは保証されない**: + `crates/session-store/src/segment_log.rs:181` の `SegmentOrigin` は `segment_id` と `at_turn_index` のみで、`session_id` を持たない。ticket 要件「型レベルで保証」とは厳密に乖離する。 + - 実用上は構築点が `create_compacted_segment` / `fork_at` / `ensure_segment_head` に限定され、いずれも source の session_id を継承する経路のみ通るため invariant は維持されているが、`Deserialize` 経由で外部から不整合 payload を流し込めば破れる。 + - 厳密化するなら `SegmentOrigin { session_id, segment_id, at_turn_index }` に拡張し、`collect_state` 時に `SegmentStart.session_id` との一致を assert する手も。ただし persistence が冗長になるトレードオフがある。 + - 後続 ticket（fork tree の永続表現が増える `live-fork-marker` 等）で扱うか、現状の「構築時保証 + doc 明示」で十分とするかは設計判断。 + +- **migration 方針がコード上で明示されていない**: + ticket 完了条件「既存 sessions ディレクトリは破棄して良いかどうかをここで明示」が code/doc で明示されていない。 + - `crates/session-store/src/fs_store.rs:1-8` の Layout doc に「旧 flat layout (`/.jsonl`) は本実装では認識されず、`list_sessions` は dir entries のみ列挙する。後方互換は意図的に作らない」旨を 1 行足す程度で十分。 + - ticket 上は説明されているので、`tickets/session-grouping-introduce.md` を grep して runtime 文脈に書き出す形でも可。 + +- **`PickerOutcome::Picked.session_id` フィールドが未使用**: + `crates/tui/src/picker.rs:67` で `session_id: SessionId` を返すが、`crates/tui/src/main.rs:213` は `Picked { segment_id, .. }` で `session_id` を捨てている。 + - `cargo check` の `field is never read` warning として表面化。 + - 現状 TUI resume は `--session ` のみを子 pod に渡し、pod-cli 側で `lookup_session_of` で解決し直すフローなので picker が session_id を返してもパススルーされる先がない。 + - 解決策: (a) `Picked { segment_id }` のみにする / (b) TUI から子 pod に session_id も渡せるようフラグを足す（pod-cli 側で `lookup_session_of` を省略できる）。short-term は (a)、long-term は (b)。 + - 同様に `picker.rs:339` の `short_segment(id: SessionId)` は名前が `short_segment` のまま（表示対象は session_id）。previous review の nit と同類の取り残し。 + +- **`tools::Tracker` / `TaskStore` の "session-scoped" / "session-lifetime" コメント残り**: + segment-rename 後続コミット (`7577577`) で header コメントは `Pod-lifetime` に統一されたが、内部コメントに `session-scoped` / `session-lifetime` が残存: + - `crates/tools/src/tracker.rs:21` "A `Tracker` is **session-scoped**: ..." + - `crates/tools/src/lib.rs:10,45` "session-lifetime, enforces..." / "(session-lifetime)" + - `crates/tools/src/task.rs:3,254,260,263,266` "Pod/session-lifetime state" / "session-lifetime task" + - `crates/pod/src/pod.rs:803,811,823` "session-scoped file-operation tracker" / "session-scoped TaskStore" + - `crates/pod/src/compact/worker.rs:5` "session-lifetime `Tracker`" + - 本 ticket で **本物の Session 概念**が入った今、これらの comment は誤読を招く（読み手が「これは新 SessionId に紐づくのか？」と疑う）。実体は Pod プロセス寿命なので `Pod-lifetime` / `Pod-scoped` に統一すべき。 + - segment-rename の review follow-up として一部対応されたが取り残しが多いので、本 ticket かこの後ろの「pod-lifetime 用語整理」ticket でまとめて清掃するのが良い。 + +- **`picker.rs:194-212` の `last_message_preview` が `AssistantItems`（旧 plural）のみ match**: + 本 ticket の前 (`segment-rename` 時点) からの取り残しだが、新規 write が `AssistantItem` (singular) のみ使う運用に変わったため、picker preview は assistant 発言を拾えなくなり実質「user 発言 or `[empty]`」のみになる。本 ticket で picker UX を Session 単位に変えた今、preview 品質の劣化が表面化しやすい。`LogEntry::AssistantItem`、`LogEntry::SystemItem`、`LogEntry::ToolResult` への対応追加が望ましい。 + +### Nits + +- **`session_test.rs` のテスト名**: `session_run_logs_entries`、`session_restore_round_trip`、`session_resume_after_pause`、`session_run_with_tool_call`、`session_fork_creates_new_session`、`session_fork_at_truncates_within_session`、`session_config_changed_logged`、`session_auto_forks_on_conflict`（`tests/session_test.rs`）。中身は新 `SessionId` を引数で受けて `restore`/`fork` する正当な Session 概念のテストになっているので、名前と中身は整合的。ファイル名 `session_test.rs` も今となっては妥当。previous review の nit からは解消方向。 + +- **`Pod::from_manifest_spawned` で child Pod が新 Session を起こす意図**: コードコメントが明示していない。`pod.rs:2867` の周辺に「spawn された child は独立した conversation grouping を持つ（親と Session を共有しない）」旨の 1 行を残しておくと、後で「spawn linkage を session 上で表現するか」検討する時の根拠が消えにくい。 + +- **`Pod::session_id()` の安定性 doc**: `pod.rs:572-576` で「Stable across compaction and in-Session fork — only `fork` changes it」と書かれているが、コード上の `fork()` (brand-new Session の `session_store::fork` を内部で呼ぶ Pod-level API) は実装されていない。`session_store::fork` を直接呼ぶフリー関数のみ存在し、`Pod` の method としては自身の `session_id` を入れ替える操作が今のところ無い（auto-fork も compaction も同 Session 維持）。doc は将来の Pod-level fork を見据えた spec だが、現実装範囲では「`session_id()` は Pod 寿命を通じて不変」と書いた方が現状認識として正確。 + +- **`SegmentState` のメソッド `set_location` vs 旧 `set_segment_id`**: previous review で `set_session_id` → `set_segment_id` のリネームが follow-up として履行されたが、本 ticket で更に `set_location(SegmentLocation)` に変わった（segment_id + session_id を atomic swap する正しい形）。旧 `set_segment_id` は callsite 全消し済み。整合的。 + +## 判断 + +**Approve with follow-up** — 必須要件（`SessionId` 型導入 / Segment 生成パスの session_id 決定 / `(SessionId, SegmentId)` restore / FsStore layout / leaf 列挙 / `cargo check` + 全テスト pass）は満たされ、ticket の最小実装範囲を守っている。 + +懸念点は (1) `SegmentOrigin` の同 Session 性が「型レベル保証」ではなく「構築時保証 + doc 明示」に留まる点（ticket 文言と乖離するが、実装方針として CLAUDE.md の「設計を歪めない」原則とは整合）、(2) migration policy がコード上で文字に起こされていない点、(3) `PickerOutcome.session_id` の dead field 残り、(4) picker preview の旧 plural-only 対応、(5) tools / pod の "session-scoped" コメント残り。いずれも blocking ではなく、後続 ticket か小さな follow-up commit で清掃可能。