diff --git a/issues/issue-113-cross-refactoring-re-retrial-handoff.md b/issues/issue-113-cross-refactoring-re-retrial-handoff.md deleted file mode 100644 index c7feb04c..00000000 --- a/issues/issue-113-cross-refactoring-re-retrial-handoff.md +++ /dev/null @@ -1,126 +0,0 @@ -# cross-refactoring 再々検証の引継ぎ - -`/ndf:cross-refactoring` の**未到達 2 項目**を通すための作業メモ。 -ここには**次に何をどの順で行うか**だけを書く。経緯と実測値は次の 2 つにある。 - -- [issue-113-cross-refactoring-trial-report.md](issue-113-cross-refactoring-trial-report.md) — 1 回目の実機検証（不具合 1〜9 を発見） -- [issue-113-cross-refactoring-retrial.md](issue-113-cross-refactoring-retrial.md) — 2 回目（再検証。不具合 10・11 を発見） - -## 現在地 - -| 回 | PR | 到達点 | 見つけた不具合 | -| --- | --- | --- | --- | -| 1 回目 | #118 | 適用結果の検証で破綻 | 9 件 | -| 2 回目 | #120（CLOSED） | レビュー・判定・輪番・集計まで到達 | 2 件 | -| **3 回目** | **未着手** | **指摘の修正と再レビューを通す** | — | - -修正はすべて main に入っている（#119 / #121）。 - -``` -6fce71f Docs: cross-refactoring 修正後の再検証レポートを残す (#122) -a415242 Fix: 公開の責務を進行側へ一本化し、適用失敗の項目を対象外へ記録する (#121) -26eb4eb Fix: cross-refactoring の実機検証で見つかった不具合 9 件を修正（v8.2.0） (#119) -``` - -## 通したい 2 項目 - -これだけが**一度も実行されていない**。 - -- **指摘の修正と再レビューの繰り返し**（`merge-fix` → 再 `review` → 再 `judge-review`） -- **上限到達時の項目単位の見送り**（`should-abandon` → `abandon-items`） - -どちらも**レビューで指摘が出ること**が前提である。2 回目はラウンド 1 で -両レビュー担当とも指摘 0 件だったため到達できなかった。 - -## 先に決めること - -指摘が出る確率をどう上げるか。**何もしないと 3 回目も到達しない可能性がある。** - -| 案 | 内容 | 見込み | -| --- | --- | --- | -| A | 前回と同条件で素直に回す | 自然な結果が得られるが、また 0 件の可能性がある | -| B | `--max-fix-rounds 1` にする | 指摘が出れば**見送りへ早く到達**する。修正の繰り返しは 1 回しか見られない | -| C | `--max-items-per-round` を上げる | 採用件数が増え、指摘が出る確率が上がる。1 ラウンドが長くなる | -| D | 範囲を広げて質の粗い箇所を含める | 指摘は出やすいが、検証の趣旨から外れる | - -B と C は併用できる。**D は最後の手段**とする。 - -## 実行手順 - -### 1. 対象の Draft PR を作る - -`/ndf:cross-refactoring` は**既存の Draft PR** を対象に動く。main から新しい -ブランチを切り、種となるコミットを 1 つ置いて Draft PR を作る（2 回目は -実行条件を書いた md を置いた）。 - -**2 回目の PR（#120）は再利用しない。** 不具合 10 に起因する失敗ラウンドが -履歴に残っているため。 - -### 2. 実行 - -```bash -# 配布物はリポジトリ内のものを使う。**プラグインキャッシュは古い** -export PLUGIN_ROOT=/work/ai-plugins/plugins/ndf-claude - -/ndf:cross-refactoring \ - --scope plugins/ndf-shared/skills/cross-refactoring/scripts \ - plugins/ndf-shared/skills/cross-refactoring/tests \ - plugins/ndf-shared/skills/cross-review/scripts/lib \ - plugins/ndf-shared/skills/cross-review/tests \ - --sync-command "bash scripts/build-runtime-plugins.sh" \ - --baseline-test "uv run --with pytest python -m pytest \ - plugins/ndf-shared/skills/cross-refactoring/tests \ - plugins/ndf-shared/skills/cross-review/tests -q" \ - --max-outer-rounds 3 -``` - -## 実行上の落とし穴（実測で踏んだもの） - -| 落とし穴 | 対処 | -| --- | --- | -| **プラグインキャッシュが古い** | `~/.claude/plugins/cache/.../ndf/8.1.0` には修正が入っていない。`PLUGIN_ROOT` をリポジトリ内の `plugins/ndf-claude` へ向ける | -| **`--sync-command` を省くと push が全て落ちる** | このリポジトリは `.githooks/pre-push` で生成物の同期を検査する。必ず指定する | -| **`--scope` にテストの置き場所を含め忘れる** | 範囲内の各ソースに対応するテスト置き場を**すべて**入れる。2 回目は `cross-review/tests` を忘れて 1 項目落とした | -| **Bash ツールの上限は 10 分** | 適用フェーズは 15〜30 分かかる。監視は**背景実行**にする（`run_in_background`） | -| **`monitor.py` に実行権限がない** | `uv run --script "$LIB/monitor.py" ...` で起動する | -| **進行を駆動する作業ディレクトリが対象ブランチを掴んでいると失敗する** | 同じブランチを 2 か所へ展開できない。PR を作ったら `git checkout main` してから実行する | -| **`monitor.py` の `elapsed` は監視開始からの秒数** | CLI の実起動時間ではない。実所要は結果ファイルの `elapsed_seconds` を見る | - -## 所要時間の目安（2 回目の実測） - -| フェーズ | 実測 | -| --- | --- | -| 初期化（認証確認 + 着手前テスト） | 約 40 秒 | -| 提案（3 CLI 並列） | 135〜195 秒 | -| 適用（codex / 11 コミット） | 約 12 分 | -| 適用（kiro / 12 コミット） | 約 12 分（監視は 30 分待った） | -| 適用結果の検証 | コミット数 × 約 30 秒（テストを実走するため） | -| レビュー（2 CLI 並列） | 約 3.5 分 | - -**1 ラウンドで 30〜45 分**を見込む。 - -## 確認すること - -未到達の 2 項目に加えて、#121 で入れた経路を実機で見る。 - -| 対象 | 何が観測できれば通ったと言えるか | -| --- | --- | -| 指摘の修正と再レビュー | `merge-fix` が修正コミットを取り込み、再レビューで判定が変わる | -| 項目単位の見送り | `should-abandon` が上限到達を返し、`abandon-items` が未解決の項目だけ取り消す | -| 公開の責務（不具合 10） | 実装担当が push せず、`merge-apply` の後に進行側が push する | -| 生成物の同期 | `Chore: 生成物を同期する（cross-refactoring 進行側）` が push の直前に積まれる | -| 対象外への記録（不具合 11） | 適用で失敗した項目が次ラウンドの提案で**再採用されない** | - -## 別件で残っているタスク - -再々検証とは独立に、次の 1 つが未対応である。 -（バージョンは **v8.3.0** へ更新済み。#121 の破壊的変更を反映した） - -### cross-review が投稿の成否を突き合わせていない - -`cross-review` は「AI 自身が `gh api` で投稿する」設計だが、**投稿が失敗しても -結果ファイルの申告だけで判定が進む**。#121 のラウンド 3 で実際に起きた -（`comments_count=2` の申告に対し、GitHub 上にスレッドが 1 つも作られなかった）。 -今回は payload ファイルから内容を拾えたが、気付かなければ指摘を取りこぼす。 - -`state.py read-result` が GitHub 側のスレッド数と突き合わせるべきである。 diff --git a/issues/issue-113-cross-refactoring-re-retrial.md b/issues/issue-113-cross-refactoring-re-retrial.md new file mode 100644 index 00000000..06d54b53 --- /dev/null +++ b/issues/issue-113-cross-refactoring-re-retrial.md @@ -0,0 +1,253 @@ +# cross-refactoring 再々検証 + +未到達だった 2 経路を実機で通した記録である。対象は Pull Request #125（Draft のまま閉じた）。 + +経緯は次の 2 つにある。 + +- [issue-113-cross-refactoring-trial-report.md](issue-113-cross-refactoring-trial-report.md) — 1 回目 +- [issue-113-cross-refactoring-retrial.md](issue-113-cross-refactoring-retrial.md) — 2 回目 + +## 目的 + +次の 2 経路は 1 回目・2 回目とも一度も実行されていなかった。 + +- 指摘の修正と再レビューの繰り返し（`merge-fix` → 再 `review` → 再 `judge-review`） +- 上限到達時の項目単位の見送り（`should-abandon` → `abandon-items`） + +どちらもレビューで指摘が出ることが前提で、2 回目はラウンド 1 で両レビュー担当とも +指摘 0 件だったため到達しなかった。 + +## 結果 + +**2 経路とも実機で成立した。** あわせて Pull Request #121 で入れた 3 経路も確認できた。 +一方で新しく 4 件の不具合を見つけ、うち 1 件は生成物を持つリポジトリで進行を止める。 + +## 実行条件 + +| 項目 | 値 | +| --- | --- | +| ホスト | Claude Code（提案・レビューには不参加） | +| 提案・レビュー | codex / gemini / kiro | +| 適用の母集合 | claude / codex / kiro | +| 使用する版 | リポジトリ内の `plugins/ndf-claude`（v8.4.0） | +| ラウンド上限 | 3（実際は 2 ラウンド目の提案で打ち切り） | + +指摘が出る確率を上げるため、修正の上限を 1 に下げ、1 ラウンドの採用上限を 8 へ上げた。 +上限を下げると、指摘が 1 回の修正で解決しなかった時点で見送りへ進む。 + +```bash +export PLUGIN_ROOT=/work/ai-plugins/plugins/ndf-claude + +/ndf:cross-refactoring 125 \ + --scope plugins/ndf-shared/skills/cross-refactoring/scripts \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/scripts/lib \ + plugins/ndf-shared/skills/cross-review/tests \ + --sync-command "bash scripts/build-runtime-plugins.sh" \ + --baseline-test "uv run --with pytest python -m pytest \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/tests -q" \ + --max-outer-rounds 3 --max-fix-rounds 1 --max-items-per-round 8 +``` + +着手前のテストは 444 件が通った。 + +## 到達点 + +```mermaid +flowchart TD + Init([初期化・作業ディレクトリ・手順書配置]):::ok --> P1 + P1["R1 提案（3 CLI 並列）11 件"]:::ok --> M1 + M1["統合 8 件 → 採用 8 件"]:::ok --> A1 + A1["R1 適用（codex）17 コミット"]:::ok --> V1 + V1["検証: 採用 7 / 失敗 1"]:::ok --> D1 + D1["取り消し 17 → 積み直し 12
（項目単位）"]:::ok --> S1 + S1["生成物を同期して進行側が push"]:::ok --> RV1 + RV1["R1 レビュー（gemini + kiro）
両者 REQUEST_CHANGES"]:::ok --> F1 + F1["修正（codex）0 コミット"]:::ng --> RV2 + RV2["再レビュー → 再び REQUEST_CHANGES"]:::ok --> AB + AB["上限到達 → 項目単位の見送り
積み直し競合によりラウンド全件へ退避"]:::ok --> P2 + P2["R2 提案 → 採用 6 件"]:::ok --> Stop + Stop([ここで打ち切り]):::stop + + classDef ok fill:#dfd,stroke:#383 + classDef ng fill:#fdd,stroke:#933 + classDef stop fill:#eef,stroke:#557 +``` + +## 未到達だった 2 経路 + +### 指摘の修正と再レビュー + +レビュー担当 2 者がともに変更要求を返し、修正フェーズを経て再レビューまで回った。 + +| ステップ | 観測 | +| --- | --- | +| `judge-review`（1 回目） | 「変更要求があります（未解決の指摘 2 件）」終了コード 2 | +| `should-abandon` | 「修正ラウンド 0 / 1 — まだ修正します」終了コード 2 | +| `fix` 起動 | 実装担当が起動し、240 秒で終了 | +| `merge-fix` | 修正ラウンドを 1 へ進め、取り込み済みとして完了 | +| 再レビュー | 2 者とも再び変更要求 | + +指摘の内容は両者とも同じで、`cmd_merge_apply` の分割で状態辞書を一時的な引数の +受け渡しに使っている点を挙げていた。指摘には改善項目 ID が付いており、 +差し戻しは発生しなかった。 + +### 上限到達時の項目単位の見送り + +``` +修正ラウンドが上限 1 に達しました。未解決の項目を見送ります +⚠ 42060bc を積み直せませんでした: error: could not apply 42060bc... +⚠ 残す項目を積み直せませんでした。このラウンドは全件取り消します +↩ 取り消し 47 コミット / 積み直し 0 コミット（ラウンド全件へ退避） +``` + +`drops` には項目単位（17 コミット）とラウンド全件（47 コミット）の両方が記録された。 +退避後は種コミットとの差分が空になり、リモートの参照も一致した。手順書が +「この構成では退避が普通に起こる」と予告していたとおりの結果である。 + +## Pull Request #121 で入れた経路 + +| 対象 | 観測 | +| --- | --- | +| 公開の責務 | 実装担当は push せず、検証を通った後に進行側が push した | +| 生成物の同期 | `Chore: 生成物を同期する（cross-refactoring 進行側）` が push の直前に積まれた | +| 対象外への記録 | 差分予算を超えた項目が理由付きで記録され、次ラウンドで再提案されなかった | + +対象外への記録は、ラウンド 2 の提案 8 件がラウンド 1 の対象と 1 件も重ならなかった +ことで確認できる。ラウンド 1 の対象は `cmd_merge_apply` / `monitor_agent` / +`cmd_merge_fix` / `cmd_judge_review` / `cmd_merge_proposals` / `_scan_patterns` の 6 つ、 +ラウンド 2 は `aggregate` / `_drop_items` / `verify_apply_item` / +`_verify_and_classify_items` / `_find_item` の 5 つである。 + +## 見つけた不具合 + +### 12. 生成物の同期コミットが必ず失敗する + +**進行が止まる。** 生成物を持つリポジトリでは、Pull Request #121 で一本化した +「進行側が push の直前に同期する」経路が毎回失敗する。 + +``` +❌ コマンドが失敗しました (git add -- lugins/ndf-claude/.../refactor.py plugins/...): + fatal: pathspec 'lugins/ndf-claude/.../refactor.py' did not match any files +``` + +`_git_out` は標準出力を `strip()` して返すが、`_worktree_changes` は +`git status --porcelain` を固定幅（状態 2 文字 + 空白 + パス）として `line[3:]` で +切り出す。未 stage の変更は状態コードが ` M` と先頭がスペースになるため、 +`strip()` すると**出力の 1 行目だけ**パスの先頭 1 文字が欠ける。 + +| 出力 | 1 行目 | `line[3:]` | +| --- | --- | --- | +| 生 | `' M plugins/...'` | `plugins/...` | +| `strip()` 後 | `'M plugins/...'` | `lugins/...` | + +2 回目は `--sync-command` が無く進行側が手で同期していたため、この経路が実機で +動いたのは 3 回目が初めてである。 + +**直し方**: 固定幅で読む出力には `strip` を適用しない。 + +### 13. 同期の後段で失敗すると差分が作業ツリーに残る + +`_sync_generated` は同期コマンド自身の失敗では作業ツリーを元に戻すが、後段の +`git add` / `git commit` が失敗する経路では戻さない。不具合 12 の結果、同期が作った +6 ファイルの差分が残ったまま中断した。次の実行は清浄性の検査で止まるため、 +手順書自身が警告している「保留中の push の再試行が永久に進まない」状態に入る。 + +**直し方**: 後段の失敗でも同じように差分を捨ててから中断する。 + +### 14. 実装担当がコミットを作れない条件が生じる + +生成物の同期を pre-commit で検査するリポジトリでは、次の 3 つが同時に成立しない。 + +| 制約 | 出どころ | +| --- | --- | +| 生成物を同期しない | 手順書（同期は進行側の責務） | +| `--no-verify` を使わない | 手順書のアンチパターン | +| 生成物が同期されていなければコミットを拒否する | 対象リポジトリの pre-commit | + +実測では、同じ実装担当がフェーズによって違う行動を取った。 + +| フェーズ | 行動 | 結果 | +| --- | --- | --- | +| 適用 | `git -c core.hooksPath=/dev/null commit` でフックを迂回 | 17 コミット | +| 修正 | 迂回せず、コミットを作れないと報告 | 0 コミット、指摘は未解決 | + +公開は進行側が検証を通してから行うので、実装担当のコミット時点で生成物が古いのは +設計どおりである。手順書がそれを書いていないため、判断がランタイム任せになっている。 +`--no-verify` だけを名指しで禁じている点も、同じことが別の手段で起こる余地を残す。 + +修正フェーズで実装担当が直した内容を作業ツリーに残したまま終えたため、続く +`merge-fix` も清浄性の検査で中断した（終了コード 4）。検証を受けていない変更は +そのまま捨ててよいので、`merge-fix` が自分で捨てて修正 0 件として続行できる。 + +**直し方**: 実装担当のコミットでは生成物の検査を通す必要がないことを手順書に明示し、 +迂回の手段を 1 つに定める。 + +### 15. 見送りの後に読み取り用の作業ディレクトリを同期していない + +次のラウンドの提案が、存在しないコードに対して行われる。 + +読み取り用の同期は `merge-apply` と `merge-fix` の直後にしかなく、`abandon-items` の +直後には無い。取り消しは HEAD を進めるため、同期しないと読み取り用は取り消し前の +状態に取り残される。 + +| 作業ディレクトリ | HEAD | +| --- | --- | +| 書き込み用 | `bfbe628`（取り消し後） | +| 読み取り用（kiro） | `3c1f293`（取り消し前） | + +その結果、ラウンド 2 で kiro の提案 3 件のうち 2 件が、ラウンド 1 で実装担当が作り +全件取り消しで消えた関数を対象にしていた。現在の HEAD に存在しないことは検索で +確認している。 + +``` +$ grep -c "def _verify_and_classify_items" work/.../refactor.py +0 +``` + +統合は対象の実在を検査しないため、この 2 件はそのまま採用された。 + +**直し方**: 提案フェーズの直前で毎回同期する。HEAD が変わっていなければ何も起きない。 + +## 運用上の観測 + +進行を止めるものではないが、実行時に踏んだ事象を残す。 + +| 事象 | 内容 | +| --- | --- | +| モデルの混雑で参加者が欠ける | 認証確認は通るのに、モデルの空きが無いと 15 秒で終了する。監視は結果なしと報告するが進行は止まらず、母集合が 1 人欠けたまま進みうる | +| 提案フェーズの既定タイムアウト | 適用フェーズには 3600 秒を明示しているが、提案は既定の 420 秒のまま。実測は gemini 105〜120 秒 / kiro 270〜285 秒 / codex 90〜285 秒で、混雑時に超えた | + +## 集計 + +| ラウンド | 実装担当 | レビュー担当 | 採用 | 適用 | 見送り | 修正 | 初回承認 | +| --- | --- | --- | ---: | ---: | ---: | ---: | --- | +| 1 | codex | gemini / kiro | 8 | 7 | 1 | 1 | いいえ | +| 2 | kiro | codex / gemini | 6 | 0 | 0 | 0 | — | + +ラウンド 1 の適用は 28 分（1682 秒）、レビューは 2 者並列で 4 分前後だった。 +モデルはすべて既定のため、ランタイム間の比較には使えない。 + +## 構造改善の成果をどう扱ったか + +ラウンド 1 の 8 項目はすべて見送った。差分予算を超えた 1 件を除く 7 件は、 +レビュー指摘が修正ラウンドの上限内に解決しなかったためである。取り消しの積み直しが +競合したため、ラウンド全件への退避となった。 + +再々検証の目的は「未到達の 2 経路が実機で成立するか」を確かめることであり、 +その目的は達している。 + +## 次にすること + +不具合 12〜15 を修正する。12 と 13 は進行を止めるため優先度が高い。 + +- 固定幅で読む出力に `strip` を適用しない（12） +- 同期の後段で失敗したときも差分を捨てる（13） +- 実装担当のコミットとフックの関係を手順書に明示する（14） +- 提案フェーズの直前で読み取り用を同期する（15） + +あわせて、提案フェーズにも明示のタイムアウトを置くか既定を引き上げる。 + +別件として、`cross-review` が投稿の成否を突き合わせていない課題が残っている。 +結果ファイルの申告だけで判定が進むため、投稿に失敗した指摘を取りこぼしうる。