From 7542605ec9942e9c804fb56fe1686d5e35af7bb2 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 20 May 2026 06:29:43 +0900 Subject: [PATCH] =?UTF-8?q?ticket:=20session-grouping-introduce=20?= =?UTF-8?q?=E5=AE=8C=E4=BA=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- TODO.md | 1 - tickets/session-grouping-introduce.md | 54 ---------- tickets/session-grouping-introduce.review.md | 106 ------------------- 3 files changed, 161 deletions(-) delete mode 100644 tickets/session-grouping-introduce.md delete mode 100644 tickets/session-grouping-introduce.review.md diff --git a/TODO.md b/TODO.md index ce20a86a..4bf43af2 100644 --- a/TODO.md +++ b/TODO.md @@ -8,7 +8,6 @@ - Pod: 任意ターンからの Fork（複数ターン巻き戻しを汎用化） → [tickets/pod-session-fork.md](tickets/pod-session-fork.md) - Pod: Inbound PodEvent ハンドリングの重複を統合 → [tickets/pod-inbound-pod-event-dedup.md](tickets/pod-inbound-pod-event-dedup.md) - 永続化層整理 (Storage) - - Session (Segment 群の grouping) 導入 → [tickets/session-grouping-introduce.md](tickets/session-grouping-introduce.md) - live auto-fork の marker 形式確定 → [tickets/live-fork-marker.md](tickets/live-fork-marker.md) - Pod 単位永続化 - Pod state backend と FsStore 実装 → [tickets/pod-state-backend.md](tickets/pod-state-backend.md) diff --git a/tickets/session-grouping-introduce.md b/tickets/session-grouping-introduce.md deleted file mode 100644 index 89e1383f..00000000 --- a/tickets/session-grouping-introduce.md +++ /dev/null @@ -1,54 +0,0 @@ -# session-store: Session（Segment 群の grouping）導入 - -## 背景 - -`segment-rename` で物理 append-only 単位を `Segment` に揃えた。続けて、ユーザー視点の「同じ会話の家系」を表す **`Session`** を導入する。 - -`Session` は fork tree 全体を 1 つにまとめる grouping で、compaction / fork は同 Session 内に新 Segment を生やす操作になる。これにより: - -- Session 数が fork で爆発せず、`WHERE session_id = ?` だけで fork tree 全体が取れる。 -- resume の指定は `(SessionId, SegmentId)` の組で行える。 -- pod-persistent-state 側で Pod ↔ Session の対応関係を扱いやすくなる。 - -## 要件 - -- 新 `SessionId` 型を `session-store` に追加。Segment は `parent_session_id` を持つ。 -- Segment 生成パスでの session_id 決定: - - new session: 新 `SessionId` を発行 - - compaction: 元 Segment と同 `SessionId` を継承 - - fork (live auto / 過去 fork いずれも): 元 Segment と同 `SessionId` を継承 -- `SessionOrigin` を以下のいずれかに整理: - - `compacted_from { segment_id, at_turn_index }` - - `forked_from { segment_id, at_turn_index }` - - どちらも同 Session 内 segment への参照であることを型レベルで保証。 -- restore API を `(SessionId, SegmentId)` を取る形に。`SegmentId` のみを取る既存経路は内部で `SessionId` を解決する shim を一段噛ませる。 -- FsStore layout に Session 単位の index を追加（Session → Segment 列挙が `WHERE session_id = ?` 相当で引けること）。形式は `/sessions//.jsonl` または別ファイル index、実装時に決定。 -- Session 内の leaf Segment 列挙 API を提供（pod-name resume 等から使う）。 - -## 完了条件 - -- `SessionId` 型と Session 単位 metadata の永続表現が決まり、FsStore で読み書きできる。 -- compaction / fork が同 Session 内 Segment 増分として記録される。 -- `(SessionId, SegmentId)` での restore が動作し、leaf 以外を指定した read-only restore も実装上は可能。 -- 既存 session を Session 単位に grouping する migration 戦略が決まっている（プロジェクト方針として後方互換は作らないため、既存 sessions ディレクトリは破棄して良いかどうかをここで明示）。 -- `cargo check --workspace` および全テストが通る。 - -## 範囲外 - -- live auto-fork の marker 形式（別チケット `live-fork-marker`）。 -- Pod 単位 metadata（Phase 2 一連のチケット）。 -- TUI からの Session/Segment 選択 UI。 -- DB backend 実装。 - -## 関連 - -- `tickets/segment-rename.md` (前提) -- `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 deleted file mode 100644 index e2ccc60b..00000000 --- a/tickets/session-grouping-introduce.review.md +++ /dev/null @@ -1,106 +0,0 @@ -# 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 で清掃可能。