diff --git a/docs/bugfix-batch-plan.md b/docs/bugfix-batch-plan.md index 49dd0b8..afef698 100644 --- a/docs/bugfix-batch-plan.md +++ b/docs/bugfix-batch-plan.md @@ -13,7 +13,9 @@ | # | PR | 対象順位 | 状態 | |---|---|---|---| | A | fix(merge-pipeline): feedback ループの誤 bail・誤ブロック解消 | 444 + 328 + 347 | **完了。** 444 は [PR #417](https://github.com/aloekun/claude-code-hook-test/pull/417) でマージ済み。328 は順位 398 の guard 変更で既に解消済みと判明し、再発防止テストのみ追加。347 は実装済み | -| B | fix(merge-pipeline): 分析ソース選定を陽性照合ベースに統一 | 336 + 288(a) + 446 | 未着手 | +| B-1 | fix(merge-pipeline): transcript の連結順序を時系列にする | 446 (再定義) | 実装済み | +| B-2 | fix(merge-pipeline): 分析ソース選定を陽性照合ベースに統一 | 336 + 288(a) | 未着手 | +| B-3 | fix(merge-pipeline): transcript 抽出を workspace 横断にする | 469 (446 から分離) | 未着手 | | C | fix(hooks): smoke suite の ETXTBSY 解消 | 396 | 未着手 | | D | fix(check-ci-coderabbit): rate-limit 第 3 format + 実レビュー有無分離 | 318 + 320 | 未着手 | | E | fix(ci): 監視系 workflow の誤動作修正 | 319 + 431 | 未着手 | @@ -79,16 +81,19 @@ ## PR B: fix(merge-pipeline): 分析ソース選定を陽性照合ベースに統一 (順位 336 + 288(a) + 446) -**束ねる理由**: 3 件とも `src/cli-merge-pipeline/src/feedback/` の context.rs / transcript.rs が「時刻範囲だけで分析ソースを選ぶ」同一欠陥。336 の commit/bookmark 照合と 288(a) の全 run 集約は同じ関数 (`find_latest_prepush_reports_dir`) の変更で、446 の transcript 探索にも同じ陽性一致原則を適用する。 +**束ねる理由 (起案時)**: 3 件とも `src/cli-merge-pipeline/src/feedback/` の context.rs / transcript.rs が「時刻範囲だけで分析ソースを選ぶ」同一欠陥、と見立てていた。**446 は切り分けの結果この見立てから外れた** (下記)。 -**着手順: 446 の切り分けを最初に行う** (実装方針に影響するため)。 +**着手順: 446 の切り分けを最初に行う** (実装方針に影響するため) — 実施済み。 -### 順位 446: transcript 抽出が並列 jj workspace のセッションを取りこぼす (まず切り分け) +> **PR B は 3 本に分割した (2026-08-18、ユーザー判断)。** 446 の切り分けで、実障害の原因と当初仮説が別物だと判明したため。**B-1 = 実障害の修正** (連結順序、下記)、**B-2 = 分析ソース選定の修正** (順位 336 + 288(a))、**B-3 = 将来リスクの予防** (workspace 横断の可視性 = 新規順位 469)。実障害の修正と未発現リスクへの予防を同じ diff に入れると切り戻し単位が粗くなる。 -- **不具合疑い**: transcript 抽出は cwd 由来の**単一 project-id フォルダ**しか見ないが、ADR-045 の並列 workspace 運用で `~/.claude/projects/` には複数 project-id が実在する。PR #395 の feedback で `session_data_unavailable` が実発生。 -- **切り分け**: project-id 群と jj workspace 一覧を突合し、PR #395 の実装セッションの所在を特定。取りこぼしが確認できなければ**別原因として再定義するか、negative result を永続化して閉じる** (dev-conventions の見送り convention)。 -- **対処の制約**: 探索候補は「広げる」のではなく「束縛する」— repo root / workspace path / PR 番号 / bookmark / session metadata の**陽性一致を必須条件**に課す。全 project-id 走査や時刻範囲だけの検索は無関係セッションを引き込み、誤った知見が台帳に入る (transcript が無いより悪い)。 -- **完了基準**: fixture テストだけでは完了としない — 実際に複数 project-id を用意し **exe 実行の実経路**で対象 transcript が選ばれ、無関係な project-id が選ばれないことを確認する。 +### 順位 446 (切り分けで再定義 — B-1 で実装): transcript の連結順序が時系列でない + +- **当初仮説は誤りだった (negative result)**: 「並列 workspace のセッションが不可視」が原因と想定していたが、PR #395 のブランチ名を両 project-id フォルダで grep したところ**メイン workspace 側にのみ出現**し、抽出対象フォルダの選択は正しかった。 +- **真因**: `collect_jsonl_paths_in_deterministic_order` はファイルを `(mtime, path)` 順に読み、その順のまま連結する。**決定論的ではあるが時系列ではない。** #395 の範囲を再現した実測では、1189 行中 **11 箇所で時刻が逆行**し最大 **560 分**巻き戻っていた。先頭行は `15:18` だが真の最古は `15:02`。 +- **なぜ気づきにくいか**: 抽出そのものは正しく、**行数は合っている** (session-analysis の報告 1189 行 = 実測 1189 行)。誤るのは範囲だけで、facet はこの非単調な列から「2.5 分しか無い」と判断し `session_data_unavailable` を報告した。報告された 2.5 分は 27 ファイル中 1 本の span と正確に一致していた。 +- **対処**: 出力を timestamp 昇順にする。同一 timestamp は `(file_index, line_index)` で tie-break するため決定論は失わない。 +- **完了基準**: 達成済み。実データ (#395 の範囲) で逆行 0 回・先頭が真の最古 `15:02:41Z` になることを確認。単体テストで「ファイル順に引きずられない」「同時刻は決定論的」「同一ファイル内は元の行順」を seal。 ### 順位 336: pre-push run / transcript の選定を対象 PR の commit/bookmark 照合に変更 diff --git a/docs/todo-summary2.md b/docs/todo-summary2.md index 69d9a1a..0a88381 100644 --- a/docs/todo-summary2.md +++ b/docs/todo-summary2.md @@ -178,7 +178,6 @@ | 442 | 🔧 Tier 2 | **security facet に「新規 fail-closed 検査の抜けを敵対的に探す」観点を追加** | todo22.md | S | なし (2026-08-12 起票。ADR-056 確定判定の二重 miss 分析で最も再現性の高い失敗パターン = PR #313 Critical 3 件) | | 443 | 💎 Tier 3 | **fix 検証縮小 × re-gate 全 group 再実行の flaky 当たり面の縮小検討** | todo22.md | S-M | なし (2026-08-12 起票。ADR-058 確定判定で唯一の changed_block が flaky 誤 block と判明。negative result の永続化も正規の出口) | | 445 | 🔧 Tier 2 | **todo preamble と facet routing 記述の整合を lint で機械検証** | todo22.md | S | なし (2026-08-13 起票。PR #395 feedback 採用。dev-conventions の暫定 convention を置換する) | -| 446 | 🚀 Tier 1 | **post-merge-feedback の transcript 抽出が並列 jj workspace のセッションを取りこぼす** | todo22.md | S | なし (2026-08-13 起票。PR #395 feedback 採用。ADR-030 の分析入力が無言欠落、まず切り分け) | | 447 | 🚀 Tier 1 | **台帳の `✅無人可` と判断留保キーワードの矛盾を決定論層で検出 (PR #400 T1-2)** | todo23.md | S | なし (2026-08-14 採用。#400 の正準タグ規約は instruction 層のみで機械強制が無い。実装先は custom lint rule か ledger.rs の fail-closed 検査かを着手時に決める) | | 448 | 🔧 Tier 2 | **判断留保キーワード検査の回帰テスト (canonical / tagged / untagged の 3 分類) (PR #400 T2-1)** | todo23.md | S | 447 (検証対象が 447 の成果物。走査の実体が現状 Rust に無いため単独着手は不可) | | 450 | 🔧 Tier 2 | **push-runner の bookmark 不在を早期検出し fallback のノイズを除去 (PR #400 T2-3)** | todo23.md | S | なし (2026-08-14 実測。削除済み bookmark への fallback がパースエラーを出してから中断し、対処法が読み取りにくい) | @@ -198,6 +197,7 @@ | 466 | 💎 Tier 3 | **出力先と検証設計の convention を明文化する (#409-#414 feedback 系統 C+E を統合)** | todo24.md | S | なし (2026-08-17 採用。docs のみ。出力の visible paths / fixture と実データの対 / step outcome の組み合わせ の 3 点) | | 467 | 🔧 Tier 2 | **夜間ループとレポート出力の小さな穴を塞ぐ (#409-#414 feedback 系統 D + dispatch 実走 F-2)** | todo24.md | S | なし (2026-08-17 採用。ブランチ削除の事前存在確認 / parse エラー診断強化 / GIT_DIR 警告抑止。D-1 の効果確認は実走が要る) | | 468 | 🔧 Tier 2 | **post-merge-feedback の takt run が起動直後に死ぬ経路 — 終了理由が記録されない** | todo24.md | S | なし (2026-08-18 起票。PR #417 の調査で判明。142 run 中 2 件が analyze 起動 34 秒以内に成果物ゼロで死亡。順位 444 は回復層の修正で死因には触れていない。まず終了コード / シグナルの観測を足す) | +| 469 | 🔧 Tier 2 | **transcript 抽出が別 jj workspace のセッションを見られない** | todo24.md | S-M | なし (2026-08-18 起票。順位 446 の切り分けで当初仮説が #395 の原因**ではない**と判明し分離。未発現の構造リスク。実装コスト調査済 = 3〜4 ファイル。`cwd_to_project_id` の case 取り扱いが Linux で壊れる既知欠陥も同時に直す) | **戦略**: Tier 1 を 2〜3 セッションで片付け → Tier 2 で計測基盤 (gate telemetry / weekly-review 保存) + rate-limit + convergence cost 削減を進める → Tier 3 でドキュメント整備。Tier 4-5 は cleanup / 外部展開で daily efficiency への直接効果は小さい。(2026-08-12 更新: 旧記述の ADR-032 は ADR-057 置換で欠番) diff --git a/docs/todo22.md b/docs/todo22.md index 94e728a..7f0ae5f 100644 --- a/docs/todo22.md +++ b/docs/todo22.md @@ -670,32 +670,3 @@ - 検出が fixture テストで固定され、`cargo test --workspace` が green であること。 --- - -### 順位 446: post-merge-feedback の transcript 抽出が並列 jj workspace のセッションを取りこぼす - -> **動機**: `cli-merge-pipeline` の transcript 抽出は `cwd` から導出した**単一 project-id フォルダ** (`~/.claude/projects//`) しか見ない。ところが本リポジトリは [ADR-045](adr/adr-045-jj-workspace-parallel-sessions.md) の並列 workspace 運用をしており、`~/.claude/projects/` には `c--Users-owner-work-claude-code-hook-test` と `C--Users-owner-work-claude-code-hook-test-improve` という**別 project-id のフォルダが実在する**。secondary workspace で実装したセッションの transcript は、現在のロジックから構造的に不可視になる。 -> -> 2026-08-13 の PR #395 の post-merge feedback で `session-analysis` が `session_data_unavailable` で失敗し、期待範囲 14 時間に対し無関係な 2.5 分の記録しか得られなかった。**[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md) の前提であるセッション知見の抽出が systematic に無力化されうる**。 -> -> なお「PR #395 が実際に secondary workspace で実装されたか」は未確認で、まず**原因の切り分け**が要る (project-id フォルダが複数実在することは事実確認済み)。同じ「ハーネスが自分の壊れに気づけない」層の問題である (orphan reaper の恒久 block は PR #417 で解消済み)。 -> -> **参照**: [transcript.rs](../src/cli-merge-pipeline/src/feedback/transcript.rs) (`project_transcript_dir` / `cwd_to_project_id`)、[ADR-045](adr/adr-045-jj-workspace-parallel-sessions.md)、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md)。 -> -> **実行優先度**: 🚀 Tier 1 — Severity **High** (feedback の分析入力が無言で欠落する) / Frequency Medium / Effort S (切り分けは短時間) / Adoption Risk None。 - -#### 設計決定 - -まず切り分け、結果に応じて対処を決める (先に実装を決めない)。 - -**対処を決める際の制約: 候補の絞り込みは「広げる」ではなく「束縛する」方向で行う。** (a) の全 project-id 走査や (c) の時刻範囲だけの検索は、**無関係なセッションを分析入力に引き込む**。実際 `~/.claude/projects/` には本リポジトリ以外の project-id も同居しうるし、同じ時間帯に別作業のセッションが走っていることもある。誤った transcript で feedback を生成するのは、transcript が無いより悪い (もっともらしい誤った知見が台帳へ入る)。したがって候補には **repo root / workspace path / PR 番号 / bookmark / session metadata のいずれかの陽性一致**を必須条件として課す ([ADR-064](adr/adr-064-monitor-success-positive-evidence.md) の陽性証拠要求と同じ姿勢)。canonical 化 (b) だけでは secondary workspace 配下に保存された transcript を発見できない場合がある点にも注意する。 - -- [ ] `~/.claude/projects/` の project-id 群と jj workspace 一覧を突合し、どの workspace がどの project-id に対応するかを確定する -- [ ] PR #395 の実装セッションがどの project-id 配下にあったかを特定し、取りこぼしが実際に起きたのかを確認する (起きていなければ別原因として本タスクを再定義する) -- [ ] 対処案を上記の束縛制約のもとで決める (探索範囲を広げる案は、必ず陽性一致条件とセットにする) -- [ ] 回帰テスト: 複数 project-id フォルダを持つ fixture で対象セッションが選ばれること。**bad ケースとして「無関係な project-id に同一時刻範囲のセッションがある」fixture を必ず含める** (時刻だけで拾わないことの担保) - -#### 完了基準 - -- 並列 workspace で実装したセッションの transcript が post-merge-feedback から参照できること。**fixture テストだけでは完了としない** — 実際に並列 workspace + 複数 project-id を用意し、`cli-merge-pipeline` の実経路 (exe 実行) で対象 transcript が選ばれることを確認する ([dev-conventions.md](dev-conventions.md) § LLM を含む自動化経路は実走でしか検証できない / [ADR-067](adr/adr-067-phase-b-unattended-fix-push.md))。 -- 無関係な project-id のセッションが選ばれないことも同じ実経路で確認すること。 -- 取りこぼしが再現しない場合は、**実行手順と観測結果を negative result として永続化**して閉じる ([dev-conventions.md](dev-conventions.md) § spike / 実験タスクの見送り (negative result) 永続化 convention)。 diff --git a/docs/todo24.md b/docs/todo24.md index 206eb70..14157de 100644 --- a/docs/todo24.md +++ b/docs/todo24.md @@ -222,3 +222,40 @@ lane モデルへの移行 ([ADR-072](adr/adr-072-nightly-todo-loop.md) 決定 1 - 起動直後に死んだ run について、**なぜ止まったか**が run ディレクトリの記録だけで判別できること。 - 分類の結果として対処が不要と判断した場合は、その根拠を negative result として永続化して閉じること ([dev-conventions.md](dev-conventions.md) § spike / 実験タスクの見送り (negative result) 永続化 convention)。 + +--- + +### transcript 抽出が別 jj workspace のセッションを見られない (順位 446 の切り分けで分離) + +> **動機**: `cli-merge-pipeline` の transcript 抽出は `cwd` から導出した**単一 project-id フォルダ**しか見ない。本リポジトリは [ADR-045](adr/adr-045-jj-workspace-parallel-sessions.md) の並列 workspace 運用をしており、`~/.claude/projects/` には 2 つの project-id フォルダが実在する (2026-08-18 実測: メイン 1788 セッション / `improve` 318 セッション)。**実装と `pnpm merge-pr` を別 workspace で行うと、実装セッションの transcript が構造的に不可視**になる。 +> +> **これは順位 446 の当初仮説だったが、#395 の失敗原因ではなかった (negative result)**: PR #395 のブランチ `claude/weekly-review-promotion-flow` を両フォルダで grep したところ、メイン workspace 側にのみ出現し `improve` 側は 0 件だった。実装はメイン workspace で行われており、抽出対象フォルダの選択は正しかった。#395 の真因は連結順序が時系列でなかったことで、そちらは 2026-08-18 に修正済み (`filter_transcripts` の doc 参照)。**したがって本エントリは実障害ではなく、未発現の構造リスクである。** +> +> **調査済みの実装コスト (2026-08-18)**: 抜本的な改修は不要で 3〜4 ファイルに収まる。 +> +> - workspace の絶対パスは `jj workspace list -T 'self.name() ++ "\t" ++ self.root() ++ "\n"'` で 1 コマンドで取れる (`.root()` が絶対パスを返す) +> - transcript の各エントリは `cwd` / `sessionId` / `gitBranch` を持ち、`cwd` は workspace root の絶対パスそのもの。**陽性一致条件をそのまま満たせる** +> - 呼び出し経路は `pipeline.rs:283` → `FeedbackInput` → `prepare_transcript` → `filter_transcripts` の一直線で分岐なし +> +> **参照**: [transcript.rs](../src/cli-merge-pipeline/src/feedback/transcript.rs) (`cwd_to_project_id` / `project_transcript_dir`)、[ADR-045](adr/adr-045-jj-workspace-parallel-sessions.md)、[ADR-030](adr/adr-030-deterministic-post-merge-feedback.md)、[ADR-024](adr/adr-024-shared-jj-helpers-library.md) (workspace 列挙の置き場)。 +> +> **実行優先度**: 🔧 Tier 2 — Severity Medium (発現すれば feedback の分析入力が無言で欠落する) / Frequency Low (cross-workspace merge の実績は未観測) / Effort S-M / Adoption Risk Low。 + +#### 設計決定 + +**探索範囲を広げるだけにしない。** 候補フォルダを増やす変更は、無関係なセッションを分析入力へ引き込む危険と表裏である。`cwd` が**このリポジトリのいずれかの workspace root 配下にある**ことを必須条件として課す ([ADR-064](adr/adr-064-monitor-success-positive-evidence.md) の陽性証拠要求)。完全一致ではなく前方一致にするのは、リポジトリのサブディレクトリで起動したセッションを落とさないため。 + +**同時に直すべき既知の欠陥**: `cwd_to_project_id` は path を `to_lowercase()` するが、実フォルダ名は大文字小文字が保存されている (`c--Users-owner-…` と `C--Users-owner-…-improve` が併存)。Windows は case-insensitive なので現状は偶然動いているだけで、**Linux では一致しない**。[ADR-063](adr/adr-063-linux-portability-release-binaries.md) のクラウドセッションと [ADR-065](adr/adr-065-ci-matrix-cross-os-regression.md) の Linux CI matrix を持つ以上、これは今日すでに潜在するバグである。 + +#### 作業計画 + +- [ ] `lib-jj-helpers` に workspace root 列挙を追加する (ADR-024 の共有ヘルパー) +- [ ] `cwd_to_project_id` の case 取り扱いを修正し、Linux で一致することをテストで固定する +- [ ] `filter_transcripts` を複数ディレクトリ対応にし、`cwd` の前方一致を必須条件にする +- [ ] 回帰テスト: 複数 project-id フォルダの fixture で対象セッションが選ばれること。**bad ケースとして「無関係な project-id に同一時刻範囲のセッションがある」fixture を必ず含める** (時刻だけで拾わないことの担保) + +#### 完了基準 + +- 別 workspace で実装したセッションの transcript が post-merge-feedback から参照できること。**fixture テストだけでは完了としない** — 実際に並列 workspace + 複数 project-id を用意し、`cli-merge-pipeline` の実経路 (exe 実行) で確認する ([dev-conventions.md](dev-conventions.md) § LLM を含む自動化経路は実走でしか検証できない)。 +- 無関係な project-id のセッションが選ばれないことを同じ実経路で確認すること。 +- `cwd_to_project_id` が case-sensitive filesystem でも正しく解決すること。 diff --git a/src/cli-merge-pipeline/src/feedback/transcript.rs b/src/cli-merge-pipeline/src/feedback/transcript.rs index d430a4a..56b6733 100644 --- a/src/cli-merge-pipeline/src/feedback/transcript.rs +++ b/src/cli-merge-pipeline/src/feedback/transcript.rs @@ -38,6 +38,21 @@ pub fn project_transcript_dir(cwd: &Path) -> Option { /// 入力: `source_dir` 配下の `*.jsonl` /// 出力: `out_path` に [first_commit_time, merged_at] かつ type が user/assistant の行のみ /// 戻り値: 書き込んだ行数 +/// +/// # 出力は timestamp 昇順であることを保証する (2026-08-18) +/// +/// 旧実装はファイルを `(mtime, path)` 順に読み、その順のまま連結していた。これは +/// **決定論的ではあるが時系列ではない** — セッションが並行していれば、あるファイルが +/// 14 時間を覆う一方で別ファイルが数分を覆い、連結列の時刻が飛び飛びに前後する。 +/// +/// 実測 (PR #395 の範囲を再現): 1189 行中 **11 箇所で時刻が逆行**し、最大 **560 分** +/// 巻き戻っていた。先頭行は 15:18 だが実際の最古エントリは 15:02 と、先頭・末尾を見て +/// 範囲を推定すると誤る。実際 `session-analysis` facet はこの列を読み、14 時間分 +/// (1189 行) が揃っているのに「2.5 分しか無い」と判断して `session_data_unavailable` +/// を報告した。**行数は合っていたのに範囲だけが誤る**ため、欠落として気づきにくい。 +/// +/// 決定論は失っていない。同一 timestamp は `(file_index, line_index)` で tie-break する +/// ため、同じ入力からは常に同じ出力になる ([`MatchedEntry::sort_key`])。 pub fn filter_transcripts( source_dir: &Path, range: &PrTimeRange, @@ -52,28 +67,60 @@ pub fn filter_transcripts( .map(std::io::BufWriter::new) .map_err(|e| format!("出力ファイル作成失敗 {}: {}", out_path.display(), e))?; - let mut written = 0usize; let jsonl_paths = collect_jsonl_paths_in_deterministic_order(source_dir)?; + let mut entries = collect_matching_entries(&jsonl_paths, range); + entries.sort_by(|a, b| a.sort_key().cmp(&b.sort_key())); + + let written = entries.len(); + for entry in entries { + writeln!(writer, "{}", entry.line).map_err(|e| format!("出力書込失敗: {}", e))?; + } + + writer.flush().map_err(|e| format!("flush 失敗: {}", e))?; + Ok(written) +} + +/// range に入る 1 行と、並べ替えに必要な位置情報。 +struct MatchedEntry { + /// 精度を揃えた timestamp (→ [`normalize_timestamp_for_comparison`])。 + timestamp: String, + /// [`collect_jsonl_paths_in_deterministic_order`] が決めたファイル順の index。 + file_index: usize, + /// ファイル内での出現順。 + line_index: usize, + line: String, +} + +impl MatchedEntry { + fn sort_key(&self) -> (&str, usize, usize) { + (&self.timestamp, self.file_index, self.line_index) + } +} - for path in jsonl_paths { - let file = match fs::File::open(&path) { - Ok(f) => f, - Err(_) => continue, +/// 各ファイルを走査し、range に入る user/assistant 行を集める。 +fn collect_matching_entries(jsonl_paths: &[PathBuf], range: &PrTimeRange) -> Vec { + let mut entries = Vec::new(); + for (file_index, path) in jsonl_paths.iter().enumerate() { + let Ok(file) = fs::File::open(path) else { + continue; }; let reader = BufReader::new(file); - for line in reader.lines().map_while(Result::ok) { + for (line_index, line) in reader.lines().map_while(Result::ok).enumerate() { if line.trim().is_empty() { continue; } - if entry_matches_filter(&line, range) { - writeln!(writer, "{}", line).map_err(|e| format!("出力書込失敗: {}", e))?; - written += 1; - } + let Some(timestamp) = matched_timestamp(&line, range) else { + continue; + }; + entries.push(MatchedEntry { + timestamp, + file_index, + line_index, + line, + }); } } - - writer.flush().map_err(|e| format!("flush 失敗: {}", e))?; - Ok(written) + entries } /// `source_dir` 内の `*.jsonl` を決定論的な順序で収集する。 @@ -123,27 +170,21 @@ fn normalize_timestamp_for_comparison(ts: &str) -> String { } } -/// transcript の 1 行が時刻 range + type filter に該当するかを判定する。 -fn entry_matches_filter(line: &str, range: &PrTimeRange) -> bool { - let value: serde_json::Value = match serde_json::from_str(line) { - Ok(v) => v, - Err(_) => return false, - }; +/// transcript の 1 行が時刻 range + type filter に該当すれば、正規化した timestamp を返す。 +/// +/// 並べ替えにも timestamp が要るため、判定と同時に取り出す (判定後に再パースしない)。 +fn matched_timestamp(line: &str, range: &PrTimeRange) -> Option { + let value: serde_json::Value = serde_json::from_str(line).ok()?; let entry_type = value.get("type").and_then(|v| v.as_str()).unwrap_or(""); if !matches!(entry_type, "user" | "assistant") { - return false; + return None; } - let timestamp = match value.get("timestamp").and_then(|v| v.as_str()) { - Some(t) => t, - None => return false, - }; - - let ts = normalize_timestamp_for_comparison(timestamp); + let ts = normalize_timestamp_for_comparison(value.get("timestamp").and_then(|v| v.as_str())?); let lower = normalize_timestamp_for_comparison(range.first_commit_time.as_str()); let upper = normalize_timestamp_for_comparison(range.merged_at.as_str()); - ts >= lower && ts <= upper + (ts >= lower && ts <= upper).then_some(ts) } #[cfg(test)] @@ -170,6 +211,30 @@ mod tests { path } + /// 1 行が range + type filter に該当するか (判定だけを見るテスト用の薄い包み)。 + fn entry_matches_filter(line: &str, range: &PrTimeRange) -> bool { + matched_timestamp(line, range).is_some() + } + + /// `read_first` を `read_second` より古い mtime にする。 + /// + /// **`thread::sleep` で差をつけない。** mtime 分解能の粗い filesystem では同値になり得て、 + /// 同値だと旧実装 (ファイル順のまま連結) も path 順で `aaa` を先に出すため、 + /// [`filter_transcripts_orders_entries_chronologically_across_files`] が**旧実装でも + /// 通ってしまう**。回帰テストの識別力を filesystem の分解能に委ねない。 + fn set_mtimes_so_the_later_timestamp_is_read_first(read_first: &Path, read_second: &Path) { + filetime::set_file_mtime( + read_first, + filetime::FileTime::from_unix_time(1_745_571_600, 0), + ) + .unwrap(); + filetime::set_file_mtime( + read_second, + filetime::FileTime::from_unix_time(1_745_571_601, 0), + ) + .unwrap(); + } + fn range_covering_0900_to_0930() -> PrTimeRange { PrTimeRange { first_commit_time: "2026-04-25T08:00:00.000Z".into(), @@ -314,59 +379,58 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + /// **順位 446 の核心**: 出力は timestamp 昇順で、ファイルの読み順に引きずられない。 + /// + /// fixture は「後に書かれた (= mtime が新しい) ファイルの方が古い timestamp を持つ」 + /// 交差ケース。旧実装はファイル順のまま連結したため出力の時刻が逆行し、先頭・末尾から + /// 範囲を推定する消費側が誤った (PR #395 実測: 11 箇所逆行 / 最大 560 分巻き戻り)。 #[test] - fn filter_transcripts_orders_by_mtime_not_filename() { + fn filter_transcripts_orders_entries_chronologically_across_files() { let dir = unique_temp_dir("order"); - write_transcript_line( + let zzz_path = write_transcript_line( &dir, "zzz-session.jsonl", - "2026-04-25T09:00:00.000Z", - "first-written", + "2026-04-25T09:05:00.000Z", + "later-timestamp", ); - std::thread::sleep(std::time::Duration::from_millis(20)); - write_transcript_line( + let aaa_path = write_transcript_line( &dir, "aaa-session.jsonl", - "2026-04-25T09:05:00.000Z", - "second-written", + "2026-04-25T09:00:00.000Z", + "earlier-timestamp", ); + set_mtimes_so_the_later_timestamp_is_read_first(&zzz_path, &aaa_path); let out_path = dir.join("filtered.jsonl"); let written = filter_transcripts(&dir, &range_covering_0900_to_0930(), &out_path).unwrap(); assert_eq!(written, 2); let out = fs::read_to_string(&out_path).unwrap(); - let first_pos = out - .find("first-written") - .expect("first-written 行が存在する"); - let second_pos = out - .find("second-written") - .expect("second-written 行が存在する"); + let earlier_pos = out + .find("earlier-timestamp") + .expect("earlier-timestamp 行が存在する"); + let later_pos = out + .find("later-timestamp") + .expect("later-timestamp 行が存在する"); assert!( - first_pos < second_pos, - "mtime が古いファイルが filename の alphabetical 順に関わらず先に処理されるべき: {out}" + earlier_pos < later_pos, + "mtime / filename に関わらず timestamp の昇順で並ぶべき: {out}" ); let _ = fs::remove_dir_all(&dir); } + /// timestamp が同値のときだけ、ファイル順 (mtime → path) が順序を決める。 + /// + /// 時系列順にしても決定性を失わないことの担保。 #[test] - fn filter_transcripts_breaks_mtime_ties_by_path_deterministically() { + fn filter_transcripts_breaks_timestamp_ties_by_file_order_deterministically() { let dir = unique_temp_dir("tie"); - let zzz_path = write_transcript_line( - &dir, - "zzz-session.jsonl", - "2026-04-25T09:00:00.000Z", - "zzz-line", - ); - let aaa_path = write_transcript_line( - &dir, - "aaa-session.jsonl", - "2026-04-25T09:05:00.000Z", - "aaa-line", - ); + let same_timestamp = "2026-04-25T09:00:00.000Z"; + let zzz_path = write_transcript_line(&dir, "zzz-session.jsonl", same_timestamp, "zzz-line"); + let aaa_path = write_transcript_line(&dir, "aaa-session.jsonl", same_timestamp, "aaa-line"); let shared_mtime = filetime::FileTime::from_unix_time(1_745_571_600, 0); filetime::set_file_mtime(&zzz_path, shared_mtime).unwrap(); @@ -381,7 +445,33 @@ mod tests { let zzz_pos = out.find("zzz-line").expect("zzz-line 行が存在する"); assert!( aaa_pos < zzz_pos, - "mtime 同値のとき二次キー PathBuf の昇順 (aaa < zzz) で決定論的に処理されるべき: {out}" + "timestamp 同値なら mtime 同値の二次キー PathBuf 昇順 (aaa < zzz) で決まるべき: {out}" + ); + + let _ = fs::remove_dir_all(&dir); + } + + /// 同一ファイル内で timestamp が同値の行は、元の出現順を保つ。 + #[test] + fn filter_transcripts_keeps_original_line_order_within_a_file() { + let dir = unique_temp_dir("within-file"); + + let same_timestamp = "2026-04-25T09:00:00.000Z"; + let path = dir.join("session.jsonl"); + let body = format!( + "{{\"type\":\"user\",\"timestamp\":\"{same_timestamp}\",\"id\":\"line-1\"}}\n\ + {{\"type\":\"user\",\"timestamp\":\"{same_timestamp}\",\"id\":\"line-2\"}}\n" + ); + fs::write(&path, body).unwrap(); + + let out_path = dir.join("filtered.jsonl"); + let written = filter_transcripts(&dir, &range_covering_0900_to_0930(), &out_path).unwrap(); + assert_eq!(written, 2); + + let out = fs::read_to_string(&out_path).unwrap(); + assert!( + out.find("line-1").unwrap() < out.find("line-2").unwrap(), + "同一ファイル・同一 timestamp は元の行順を保つべき: {out}" ); let _ = fs::remove_dir_all(&dir);