From 36540c47a989705d2ed87b8bb6066ac452ab4eda Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sun, 16 Aug 2026 20:27:57 +0000 Subject: [PATCH] =?UTF-8?q?Docs:=20cross-refactoring=20=E4=BF=AE=E6=AD=A3?= =?UTF-8?q?=E5=BE=8C=E3=81=AE=E5=86=8D=E6=A4=9C=E8=A8=BC=E3=83=AC=E3=83=9D?= =?UTF-8?q?=E3=83=BC=E3=83=88=E3=82=92=E6=AE=8B=E3=81=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 不具合 9 件の修正（#119）が実機で成立することを確かめ、レビュー・判定・輪番・ 集計まで到達した。あわせて新しい不具合を 2 件見つけた（#121 で修正済み）。 構造改善の成果 4 項目は取り込まない。#121 が同じ refactor.py を大きく変えており、 手でコンフリクトを解消すると「2 者のレビューを通った内容」という性質が失われる。 同じ提案は再々検証でまた出る見込みなので、レポートだけを残す。 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GSwBvT9CH8mKfgyFn2JWfS --- issues/issue-113-cross-refactoring-retrial.md | 204 ++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 issues/issue-113-cross-refactoring-retrial.md diff --git a/issues/issue-113-cross-refactoring-retrial.md b/issues/issue-113-cross-refactoring-retrial.md new file mode 100644 index 00000000..a11eb495 --- /dev/null +++ b/issues/issue-113-cross-refactoring-retrial.md @@ -0,0 +1,204 @@ +# cross-refactoring 修正後の再検証 + +不具合 9 件の修正（[issue-113-cross-refactoring-defect-fixes.md](issue-113-cross-refactoring-defect-fixes.md)、 +NDF v8.2.0）を実機で確かめた記録である。対象は PR #120（Draft のまま閉じた）。 + +前回（PR #118）は適用結果の検証で失敗した項目を取り消す経路が破綻し、 +レビューフェーズより先へ一度も進めなかった。今回の目的は**その先を通すこと**にある。 + +## 実行条件 + +| 項目 | 値 | +| --- | --- | +| ホスト | Claude Code（提案・レビューには不参加） | +| 提案・レビュー | codex / gemini / kiro | +| 適用の母集合 | claude / codex / kiro | +| 使用する版 | リポジトリ内の `plugins/ndf-claude`（v8.2.0）。プラグインキャッシュ（8.1.0）は使わない | +| ラウンド上限 | 3 | + +```bash +PLUGIN_ROOT=/work/ai-plugins/plugins/ndf-claude + +/ndf:cross-refactoring 120 \ + --scope plugins/ndf-shared/skills/cross-refactoring/scripts \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/scripts/lib \ + --baseline-test "uv run --with pytest python -m pytest \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/tests -q" +``` + +`--scope` に**テストの置き場所を含める**。範囲は適用結果の検証にも効くようになったため、 +含めないと `test_gap` が真の項目で「テストを先に足せ」と「範囲外を触るな」が両立しない。 + +## 確かめること + +修正した 9 件が実機で成立するか。 + +| # | 直したこと | 何が観測できれば通ったと言えるか | +| --- | --- | --- | +| 1 | 取り消しの巻き戻しと積み直し | 検証に失敗した項目だけが消え、合意済みの項目が残る。分離できない位置関係なら `rounds[].drops[].mode = round` が記録される | +| 2 | 中断と全件失敗の区別 | 取り消しに失敗したら終了コード 4 で進行が止まる（次の提案ラウンドへ進まない） | +| 3 | 判定の逐次記録 | `rounds[].apply_progress` に項目ごとの判定が残る | +| 4 | 再送信の印 | 取り消しが Pull Request へ反映される。`pending_push` が残らない | +| 5 | 範囲の検査 | 配布物 3 系統が実装担当の差分に現れない | +| 6 | 提案結果のラウンド別保存 | `<ランタイム>-propose-rf-r<ラウンド>-result.json` が巡ごとに残る | +| 7 | gemini の読み取り | gemini の提案が語彙内で返る（`read_file` が拒否されない） | +| 8 | 語彙の列挙 | 3 ランタイムとも `smell` / `technique` が英字の識別子で返る | +| 9 | 認証の確認 | 初期化時に 4 CLI の認証状態が出力される | + +## 前回まったく実行できていない範囲 + +ここを通すことが今回の主目的である。 + +- レビュー担当 2 者の並列実行と指摘の投稿、承認判定 +- 指摘の修正と再レビューの繰り返し、上限到達時の項目単位の見送り +- 実装担当の輪番 +- 提案の重複率による収束判定 +- 集計値（`report --metrics`）の出力 + +## 結果 + +**修正した 9 件はすべて実機で成立した。** 前回は適用結果の検証で失敗した項目を取り消す +経路が破綻して停止したが、今回はレビュー・判定・輪番・集計まで到達した。 +一方で**新しい不具合を 2 件見つけ**、うち 1 件は収束ループを実質的に不能にする。 + +## 到達点 + +```mermaid +flowchart TD + Init([初期化・作業ディレクトリ・手順書配置]):::ok --> P1 + P1["R1 提案（3 CLI 並列）11 件"]:::ok --> M1 + M1["統合 7 件 → 採用 5 件"]:::ok --> A1 + A1["R1 適用（codex）11 コミット"]:::ok --> V1 + V1["検証: 採用 4 / 失敗 1"]:::ok --> D1 + D1["取り消し 11 → 積み直し 8
（項目単位）"]:::ok --> RV1 + RV1["R1 レビュー（gemini + kiro）
両者 APPROVE"]:::ok --> P2 + P2["R2 提案 → 採用 5 件"]:::ok --> A2 + A2["R2 適用（kiro）12 コミット"]:::ok --> V2 + V2["検証: 全 5 件が範囲外で失敗"]:::ng --> D2 + D2["取り消し 12 → 積み直し 0
ラウンドごと失敗"]:::ok --> Stop + Stop["不具合 10 のため中止"]:::ng + + classDef ok fill:#dfd,stroke:#383 + classDef ng fill:#fdd,stroke:#933 +``` + +前回の到達点は `V1` の直前までだった。 + +## 修正 9 件の実機確認 + +| # | 直したこと | 実機での観測 | +| --- | --- | --- | +| 1 | 取り消しの巻き戻しと積み直し | `取り消し 11 コミット / 積み直し 8 コミット（項目単位）`。失敗した 1 項目だけを落とし、**同じ `refactor.py` を触る 3 項目 6 コミットを積み直した** | +| 2 | 中断と全件失敗の区別 | push 失敗で終了コード 4。次の提案ラウンドへ進まず停止した | +| 3 | 判定の逐次記録 | `rounds[].apply_progress` に全項目の判定が残った | +| 4 | 再送信の印 | 中断を跨いで `pending_push` が残り、次の実行が**処理済み判定より先に** push を再送した | +| 5 | 範囲の検査 | 範囲外コミットを検出して項目を失敗にした（R1-004 / R2 全件） | +| 6 | 提案結果のラウンド別保存 | `*-propose-rf120-r1-result.json` と `-r2-` が**両方残った** | +| 7 | gemini の手順書読み取り | `.gemini/settings.json` が配置され、差分にも出なかった | +| 8 | 語彙の列挙 | 3 ランタイムとも `smell` / `technique` を英字の識別子で返した | +| 9 | 認証の確認 | 初期化時に 4 CLI の認証状態を出力した | + +不具合 1 は、**前回破綻したのと同じ条件**（採用項目の多くが同一ファイルを触る）で成立した。 + +## 前回まったく実行できていなかった範囲 + +| 対象 | 結果 | +| --- | --- | +| レビュー担当 2 者の並列実行と承認判定 | ✅ gemini / kiro を並列起動し、両者 `APPROVE` で `judge-review` が終了コード 0 | +| 実装担当の輪番 | ✅ R1 codex → R2 kiro。レビュー担当も gemini+kiro → codex+gemini へ入れ替わった | +| 提案ラウンドの収束判定 | ✅ `advance` が継続を判定 | +| 集計値の出力 | ✅ `report --metrics` が実装担当・レビュー担当の表と、`auto` モデルの分離注記まで出した | +| 指摘の修正と再レビューの繰り返し | ❌ **未到達**。R1 は指摘 0 件、R2 は適用が全件失敗したため | +| 上限到達時の項目単位の見送り | ❌ **未到達**（同上） | + +## 新しく見つかった不具合 + +### 10. pre-push の同期検査と範囲ルールが両立しない + +このリポジトリは `.githooks/pre-push` で生成物の同期を検査する。一方この Skill は +「実装担当は編集元だけを触る / 生成物の同期は**進行側が収束後に**まとめて行う」と決めた。 +この 2 つは両立しない。 + +```console +$ git push origin refactor/cross-refactoring-retrial +Generated directory is out of date: plugins/ndf-claude/skills +exited 1 +``` + +**実装担当の push も、取り消しを反映する進行側の push も落ちる。** 結果、検証を通って +いない R1-004 の変更が Pull Request に残った（取り消しはローカルのみ）。 +**不具合 4 が防ごうとした状態そのもの**である。 + +さらに悪いことに、**この検査は実装担当を範囲ルール違反へ誘導する。** ラウンド 2 で +kiro は push を通すために配布物 3 系統を同期し、その結果**採用 5 件が全件範囲外で失敗** +して 1 ラウンドを丸ごと失った。 + +| ラウンド | 実装担当 | 配布物を同期したか | 適用成功 | +| --- | --- | --- | ---: | +| 1 | codex | しなかった（push は失敗のまま放置） | 4 / 5 | +| 2 | kiro | した（4 コミットすべてで） | 0 / 5 | + +「収束後にまとめて同期する」という決定が、**ループの途中で push が起きること**を +見落としていた。 + +**修正の方向**: 実装担当に push させず、**検証を通った後で進行側だけが push する**。 +同期はその直前に行う（`--sync-command` を新設し、状態ファイルへ保持する）。 +これで「未検証の変更が公開される」経路自体が無くなり、不具合 4 は緩和ではなく根絶できる。 + +本再検証では、進行側が生成物を同期するコミットを積んで回避した。 + +### 11. 適用で失敗した項目が「対象外」に入らない + +手順書は「見送った項目は理由付きで記録し、次ラウンドの提案時に**対象外として渡す**。 +そうしないと同じ提案が毎ラウンド出続けて収束しない」と定めている。しかし +`merge-apply` の失敗経路だけがこの記録を行わない（`deferred_items` に入らない）。 + +実測では、ラウンド 1 で失敗した `monitor.py#monitor_agent`（R1-004）が +**3 ランタイム全員から再提案され、合意 3 で最優先に採用された**（R2-001）。 +同じ理由で必ず失敗するため、ラウンドを 1 つ丸ごと消費する。 + +**修正の方向**: `merge-apply` が失敗させた項目も `deferred_items` へ理由付きで記録する。 + +## 運用上の注意 + +`--scope` には、範囲内の各ソースに対応する**テストの置き場所をすべて**含める。 +今回は `cross-review/scripts/lib` を範囲に入れながら `cross-review/tests` を入れ忘れた +ため、`monitor.py` の現状固定テストが範囲外となり R1-004 が失敗した。 +**検査は正しく働いている**（指定の誤り）。 + +## 構造改善の成果をどう扱ったか + +ラウンド 1 では 4 項目が採用・検証・レビュー承認まで通った。 + +| ID | 対象 | 手法 | +| --- | --- | --- | +| R1-001 | `refactor.py#cmd_merge_apply` | `extract_method` | +| R1-002 | `metrics.py#aggregate` | `extract_method` | +| R1-003 | `refactor.py#cmd_merge_fix` | `extract_method` | +| R1-005 | `refactor.py#cmd_merge_apply` | `consolidate_duplication` | + +ラウンド 2 の 12 コミットは全件取り消した。 + +**この 4 項目は取り込まなかった。** 不具合 10・11 の修正が同じ `refactor.py` を +大きく変えたため、取り込むには手でコンフリクトを解消する必要があり、そうすると +**「2 者のレビューを通った内容」という性質が失われる**（解消した結果は誰も見ていない）。 +同じ提案は再々検証でまた出る見込みなので、このレポートだけを残した。 + +再検証そのものの目的は「収束ループが実機で成立するか」を確かめることであり、 +その目的は達している。 + +## 次にすること + +不具合 10 と 11 は修正済み（PR #121）。 + +- 実装担当は push しない。公開は進行側が検証を通した後に行う +- 生成物の同期は `--sync-command` として push の直前に進行側が実行する +- 適用で失敗した項目も「対象外」として記録する + +これを前提に、**指摘の修正と再レビューの繰り返し**と**上限到達時の項目単位の見送り**を +通す再々検証を行う。実行時は次の 2 点に注意する。 + +- `--scope` に、範囲内の各ソースに対応するテストの置き場所をすべて含める +- 生成物を持つリポジトリでは `--sync-command` を必ず指定する