ticket: segment-rename 完了
This commit is contained in:
parent
c2b55a498b
commit
a084324830
1
TODO.md
1
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)
|
||||
- SessionId → SegmentId リネーム → [tickets/segment-rename.md](tickets/segment-rename.md)
|
||||
- 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 単位永続化
|
||||
|
|
|
|||
|
|
@ -1,41 +0,0 @@
|
|||
# session-store: SessionId → SegmentId へのリネーム
|
||||
|
||||
## 背景
|
||||
|
||||
永続化層の現 `SessionId` は、append-only log の物理的 / 復元上の単位を指している（compaction や fork で新 ID が発行される）。一方ユーザー視点では「同じ会話の継続」が compaction / fork を跨いで成立しており、ここに**ユーザー視点の会話の家系 = `Session`** と **物理 append-only 単位 = `Segment`** の 2 階層を導入したい。
|
||||
|
||||
本チケットは 2 階層導入のうち、**先に物理単位側のリネームだけ**を機械的に済ませる。新 `SessionId`（grouping 概念）は次チケットで導入する。
|
||||
|
||||
## 要件
|
||||
|
||||
- `crates/session-store/` 内の `SessionId` 型・関連関数 (`SessionLogWriter` / `Store::append` / `fork` / `fork_at` 等) を `SegmentId` に統一。
|
||||
- 同様に `pod` / `pod-registry` / `pod-cli` / `llm-worker` 関連の呼び出し元の参照を `SegmentId` に追従。
|
||||
- `SessionLogWriter` / `SessionStart` / `SessionOrigin` 等の `Session` プレフィックス型のうち、**segment レベルを指しているもの**は同時に `Segment` プレフィックスへ変える。
|
||||
- 線引きの基準: そのデータが「fork / compaction で新規生成される log 1 本」に紐づくなら `Segment`。会話の家系全体に紐づく概念は何も無いはずなので（次チケットで初めて出てくる）、本チケット内では全て Segment 系に倒す。
|
||||
- crate 名 (`session-store`) の rename 要否は本チケットでは扱わず保留。中身が Segment 中心になった事実のみで判断材料を残す。
|
||||
- `--session <UUID>` 系 CLI 引数は内部実装としては `SegmentId` を受けるが、外部表記は変更しない（ユーザー向け ID 体系の整理は `session-grouping-introduce` で扱う）。
|
||||
|
||||
## 完了条件
|
||||
|
||||
- `SessionId` 型がコードベースに残らない（後続の Session grouping で導入する新 `SessionId` は無関係）。
|
||||
- `cargo check --workspace` および全テストが通る。
|
||||
- 既存 session の JSONL を Segment 中心の API で読み書きできる。
|
||||
|
||||
## 範囲外
|
||||
|
||||
- 新 `SessionId` (Segment 群の grouping) の導入（次チケット `session-grouping-introduce`）。
|
||||
- session-store crate 名の rename。
|
||||
- 外部公開 CLI 引数の rename。
|
||||
|
||||
## 関連
|
||||
|
||||
- `tickets/entry-hash-abolish.md` (前提)
|
||||
- `tickets/session-grouping-introduce.md` (後続)
|
||||
- `crates/session-store/`
|
||||
- `crates/pod/src/pod.rs`
|
||||
|
||||
## Review
|
||||
|
||||
- 状態: Approve with follow-up
|
||||
- レビュー詳細: [./segment-rename.review.md](./segment-rename.review.md)
|
||||
- 日付: 2026-05-20
|
||||
|
|
@ -1,69 +0,0 @@
|
|||
# Review: session-store: SessionId → SegmentId へのリネーム
|
||||
|
||||
対象コミット:
|
||||
- `0d7c37f update: SessionId / SessionStart / SessionOrigin 等を Segment 系名称へ`
|
||||
- `7577577 update: Session-lifetime/scoped を Pod-lifetime に修正`
|
||||
|
||||
ベース: `develop` (`d5c7330 Merge: entry-hash-abolish`)
|
||||
|
||||
## 前提・要件の確認
|
||||
|
||||
- **`SessionId` 型がコードベースに残らない**: 達成。`grep -rn 'SessionId' crates --include='*.rs'` でヒットゼロ。`SegmentId` 型 alias は `crates/session-store/src/lib.rs:56` に定義され、`new_segment_id()` (同 lib.rs:59) も改名済み。
|
||||
- **`SessionStart` / `SessionOrigin` / `SessionState` / `SessionLogSink` / `SessionLockInfo` 等の segment レベル `Session*` 型を Segment に統一**: 達成。同 grep でこれらの PascalCase 識別子も全て不在。`SegmentStart{,State}` (`session-store/src/segment.rs:18`)、`SegmentOrigin` (`segment_log.rs:175`)、`SegmentState` (`pod/src/pod.rs:60`)、`SegmentLogSink` (`pod/src/segment_log_sink.rs:42`)、`SegmentLockInfo` (pod-registry) に置き換わっている。
|
||||
- **`Store::{list_segments, create_segment}` / `create_compacted_segment` / `pod_registry::{update_segment, lookup_segment, find_by_segment}` への改名**: 達成。`session-store/src/store.rs:48,51` で trait 名前が更新され、`pod-registry/src/{lifecycle,mutate,table}.rs` の呼び出し元と本体も全て追従。
|
||||
- **`protocol::Event::SegmentRotated` / `CompactDone.new_segment_id`**: 達成。`crates/protocol/src/lib.rs:383, 411`。`SessionRotated` の grep もヒットゼロ。
|
||||
- **module / file rename**: `crates/session-store/src/{session.rs → segment.rs}`、`{session_log.rs → segment_log.rs}`、`crates/pod/src/{session_log_sink.rs → segment_log_sink.rs}` が `git mv` で実行され、`pub mod` / `use` 宣言も追従。
|
||||
- **crate 名 `session-store` の保留**: 維持。`Cargo.toml` の `name = "session-store"` はそのまま。
|
||||
- **CLI flag `--session` / TUI 内部 `ResumeWithSession` / `InvalidSession` / `~/.insomnia/sessions/` ディレクトリ**: 維持。`crates/tui/src/main.rs:54-99`、`crates/pod/src/main.rs:188`、`manifest::paths::sessions_dir()` 経路は触られていない。defer 方針通り。
|
||||
- **`cargo check --workspace` と全テスト pass**: 確認。`target/debug` で `Finished dev` まで通り、`cargo test --workspace` も `test result: ok` のみで完走（`double_run_returns_error` の flakiness はチケット冒頭で報告済みの KNOWN_ISSUES 既知別件）。
|
||||
- **既存 JSONL を Segment 中心 API で読み書きできる**: `LogEntry::SegmentStart` で serde 表記が新しく `kind = "segment_start"` 系に倒れる方向 (Rust の rename 規約に基づく)。旧 JSONL を読みたい場合は migration が要るが、CLAUDE.md 方針「不必要な後方互換は作らない」と整合。新コードで作って読む限りは round-trip する (`session-store/tests/fs_store_test.rs`, `session_test.rs` の全 pass で確認)。
|
||||
|
||||
## アーキテクチャ・スコープ
|
||||
|
||||
- 機械的 rename のみで、層境界・API 構造に変化なし。llm-worker / pod / pod-registry / protocol / tui の全層で同じ語彙が一斉に移っているので、コードベースの語彙整合性は素直に上がっている。
|
||||
- 第 2 コミット (`7577577`) の `Session-lifetime` → `Pod-lifetime` 是正は、ticket の rename 趣旨を越えるが、関連語彙の整理として妥当。`tools::TaskStore` / `tools::Tracker` は compaction を跨いで `Pod` プロセス寿命まで生きるという事実関係に doc が即している。むしろ「旧 SessionId=Segment 時代でも `Session-scoped` 表現は不正確だった」点を解消できているので、本チケットの中で片付けた判断は妥当。
|
||||
- file rename を要求していない要件に対して、`session.rs → segment.rs` 等の module rename まで踏み込んだ判断は、対称性確保として妥当。`pub mod` / `pub use` の更新コストは小さく、ファイル名と中身の概念が乖離する状態を避ける方が長期保守に有利。過剰ではない。
|
||||
- 次チケット `session-grouping-introduce` で新たに `SessionId` 型 (Segment 群の grouping) が入る予定なので、現状の rename は新型導入と語彙衝突しない土台を提供できている。
|
||||
|
||||
## 指摘事項
|
||||
|
||||
### Blocking
|
||||
- なし。
|
||||
|
||||
### Non-blocking / Follow-up
|
||||
|
||||
- **公開エラー variant が `Session*` のまま残っている**: 以下は segment レベルの条件で発火するため、ticket の rename 方針 (segment レベルを指す Session* 型は Segment に倒す) に従って後続で追従するのが望ましい。次チケットで新 `SessionId` が入ると、これらの variant 名は誤読を招く。
|
||||
- `PodError::SessionEmpty { segment_id: SegmentId }` (`crates/pod/src/pod.rs:3238`) → `SegmentEmpty`
|
||||
- `PodError::SessionScopeMissing { segment_id: SegmentId }` (`crates/pod/src/pod.rs:3243`) → `SegmentScopeMissing`
|
||||
- `ScopeLockError::SessionConflict { segment_id: SegmentId, ... }` (`crates/pod-registry/src/error.rs:33`) → `SegmentConflict`
|
||||
- 対応する `#[error("session {segment_id} ...")]` メッセージ文字列、`tui/src/spawn.rs:60` の重複文言、テスト (`pod/tests/restore_test.rs:78,79,104,105`, `pod-registry/src/{lifecycle.rs:323-331, mutate.rs:591,603,611}`) も連動。
|
||||
- **`SegmentState` の setter 名が `set_session_id`**: `crates/pod/src/pod.rs:72`。型 (`SegmentState`) と getter (`segment_id()`) は改名済みだが setter だけ旧名のまま。`set_segment_id` に揃えるのが自然 (呼び出しは `pod.rs:1659, 2163` の 2 箇所のみ)。
|
||||
- **`Pod` 構造体のフィールド `session_state: Arc<SegmentState>`**: `crates/pod/src/pod.rs:167` および計 12 箇所 (340, 385, 481, 548, 607, 630, 632, 1621, 1659, 1660, 2148, 2163, 2438, 2768, 2838, 2977)。型は `SegmentState` なのにフィールド名が旧概念に固定されている。`segment_state` に揃えるのが整合的。
|
||||
- **`pod_cli.rs:78-79` の取り残し**: 第 2 コミットで line 57 を `Segment:` に直したが、末尾の `println!("\nSession ID: {}", pod.segment_id())` は据え置き。ユーザー向けサンプルなので意図的なら問題ないが、第 2 コミットの趣旨 (`Session:` → `Segment:`) と非対称。
|
||||
- **doc comment の旧語彙残り (narrative)**:
|
||||
- `crates/session-store/src/lib.rs:5` "Sessions are recorded as a sequence of `LogEntry`..." (本文は Segment が主語のはず)
|
||||
- `crates/session-store/src/segment_log.rs:154-159` "session-store は payload を不透明扱い... 「session 寿命に縛りたいが session-store の型を汚したくない」"
|
||||
- `crates/pod/src/segment_log_sink.rs:1,36,47,51,107,142,158,170,181` "session-log mirror" / "session log mirror mutex poisoned" 等
|
||||
- `crates/pod/src/pod.rs:157,164,166` "persists session state via `session-store`" / "Shared session pointer"
|
||||
- これらは旧チケットの慣性で残っているもの。即時の修正は必須でないが、`session-grouping-introduce` で本物の Session 概念が入ると曖昧度が増す。
|
||||
|
||||
### Nits
|
||||
|
||||
- **テスト関数名と test ファイル名の取り残し**:
|
||||
- `crates/session-store/tests/session_test.rs` (ファイル名)。中身は segment 操作のテスト群 (`session_run_*`, `session_restore_round_trip` 等)。次の grouping 導入で本物の session レベルのテストを足す場合に名前が衝突する。
|
||||
- `crates/session-store/tests/fs_store_test.rs:56,82,135` (`create_session_writes_all_entries`, `list_sessions_returns_newest_first`, `not_found_error_for_missing_session`)。
|
||||
- `crates/pod-registry/src/lifecycle.rs:258,279,299,320` (`lookup_session_returns_live_writer_info`, `update_session_rewrites_allocation_session_id`, `update_session_rejects_when_target_already_held`)。
|
||||
- `crates/pod-registry/src/mutate.rs:576` (`register_pod_rejects_session_id_collision`)。
|
||||
- `crates/pod-registry/src/table.rs:224` (`find_by_session_skips_none_placeholders`)。
|
||||
- `crates/pod/tests/restore_test.rs:35,58,85` (`restore_from_manifest_rejects_unknown_session`, `_rejects_empty_session_log`, `_rejects_session_without_scope_snapshot`)。
|
||||
- `crates/pod/tests/compact_events_test.rs:182,217,572` (`system_texts_in_sink_session_start`, `compact_emits_session_start_carrying_summary_and_task_snapshot`, `detached_extract_does_not_fork_session_log`)。
|
||||
- `crates/pod/tests/system_prompt_template_test.rs:178` (`session_start_state_captures_rendered_prompt`)。
|
||||
- `crates/pod/src/segment_log_sink.rs:203` (test helper fn `session_start()`)。
|
||||
- **ローカル変数名の取り残し**: `source_session_id` (`pod.rs:2148, 2438`, `session-store/src/segment.rs:63,74`)、`prev_session_id` (`pod.rs:1621`)、`old_session_id` (`pod.rs:2148`)、`session_id_for_usage` (`controller.rs:433`)、`usage_session_id` (`memory/src/tool/read.rs:42`)、`sessions` (`session-store/src/fs_store.rs:83-98`)。`ensure_session_head` 関数名 (`pod.rs:1619`) も同様。
|
||||
- **`tui/src/picker.rs:316`, `tui/src/spawn.rs:476`**: `short_session(id: SegmentId)` 関数。短縮表示の対象は SegmentId だが関数名は旧名のまま。CLI flag `--session` 側のユーザー文脈に寄せているなら据え置きでも可。
|
||||
|
||||
## 判断
|
||||
|
||||
**Approve with follow-up** — 要件 (型 rename 網羅 / build & test pass / defer 対象は据え置き) は全て満たされ、`SessionId` 識別子の根絶も完遂。第 2 コミットの `Pod-lifetime` 是正も無関係な改善ではなく、ticket 趣旨の周辺整理として妥当。
|
||||
|
||||
ただし `PodError::SessionEmpty` / `ScopeLockError::SessionConflict` 等の **public な error variant 名** と、`Pod::session_state` フィールド・`SegmentState::set_session_id` メソッドの取り残しは、次チケット `session-grouping-introduce` で新 `SessionId` 型が導入される前に片付けておくのが理想 (放置すると新型と命名衝突して読みにくくなる)。
|
||||
Loading…
Reference in New Issue
Block a user