diff --git a/CLAUDE.md b/CLAUDE.md index 77ad1100..82d0e8ed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,12 +62,13 @@ python3 plugins/ndf/scripts/instructions-check.py --root . `/ndf:cross-refactoring` は参加者に構造改善を提案させ、同じ参加者から輪番で選んだ 1 者が適用する。新しい提案が出なくなるまで繰り返す。参加者の既定は **codex / kiro とホスト（ホストが codex / kiro なら 2 者）** で、`--exclude` / `--include` で名指しで変える（agy は `--include agy` で戻す）。レビューは最終ゲートの `cross-review` が担う。 ```bash -/ndf:cross-refactoring 130 --scope src/services --baseline-test "pytest -q" +/ndf:cross-refactoring 130 --scope src/services tests/services --round-test "pytest tests/services -q" --baseline-test "pytest -q" /ndf:cross-refactoring 130 --scope src --model codex=gpt-5.5 --model claude=claude-opus-5 /ndf:cross-refactoring 130 --scope src --include agy --exclude kiro ``` - `--scope` は必須。提案が発散して PR が肥大するのを防ぐ。**検証にも効く**ので、現状固定テストの置き場所も含める +- v10.17.4 の次の版で、群ごとの検証は `--round-test`（範囲のテスト）で走らせ、`--baseline-test`（全体のテスト）は着手前と最終ゲートの 2 回だけ走らせる。`--round-test` を省くと全体のテストが群ごとに走る - ホストと同じランタイムが適用担当になる場合も、サブエージェントではなく **CLI プロセス**として起動する - モデルを比べるなら `--model <ランタイム>=` を参加者の全員に指定する。実際に動いたモデルを取得できるのは claude だけで、残りは指定値で代用する。指定が無いラウンドは集計から分離される - 適用担当は参加者の数のラウンドで 1 周する。輪番は適用ラウンドごとに進むため、`--max-outer-rounds`（既定 3）が切る提案の回数とは対応しない diff --git a/docs/specifications/README.md b/docs/specifications/README.md index 5a468ed9..8ba5d673 100644 --- a/docs/specifications/README.md +++ b/docs/specifications/README.md @@ -21,6 +21,7 @@ | [cross-review-writes-to-conductor.md](cross-review-writes-to-conductor.md) | GitHub と git への書き込み（レビューの投稿・返信・決着・修正のまとめ・修正の送信）をレビューを回す側だけが行うこと、担当が書く 2 つのファイルと改名の順序、二度書かない照合、差分の外を指す指摘の退避、途中で止まったときの立て直し、起動し直しを初回と同じ経路へ通すこと。手順は `cross-review` の `SKILL.md` と `docs/` が正 | | [cross-refactoring-apply-intake.md](cross-refactoring-apply-intake.md) | 担当が結果を残さない起動を 3 つの取り込みが同じ手順で受けること（範囲の確定・未検証のコミットの取り消し・結末の記録）、適用ラウンドの開き直しの判定と試行の上限 2 回、項目の無い適用ラウンドを作らないこと、帰属の段落の後ろから必須の記名を読むこと。手順は `cross-refactoring` の `SKILL.md` と `docs/` が正 | | [cross-refactoring-participants.md](cross-refactoring-participants.md) | cross-refactoring の参加者（codex / kiro とホストを既定に足す者／外す者で変える・確認を通らない者を外して続ける）、提案と適用を同じ参加者で回す輪番、再開で渡した引数の反映、呼び手の無くなった共通層の旧関数の削除。手順は `cross-refactoring` の `SKILL.md` と `docs/` が正 | +| [cross-refactoring-round-tests-and-assess.md](cross-refactoring-round-tests-and-assess.md) | cross-refactoring の群と修正コミットを範囲のテスト（`--round-test`）で検証し全体のテストを着手前と最終ゲートの 2 回に限ること、打ち切りが子の終了で戻ること、構造改善を飛ばしてよいかの判定（`refactor.py assess`）と飛ばした記録、テスト整備と適用で `.md` の文言固定テストを採らないこと、リポジトリに文言固定テストを置かない分類の規則。手順は `cross-refactoring` の `SKILL.md` と `development-workflow` の `references/` が正 | | [ndf-cleanup-and-bundle-closing.md](ndf-cleanup-and-bundle-closing.md) | 後片付けが止まる条件（git の拒否だけ）、実行前確認の要否を決める 3 つの問い、まとまりの課題を終わりの工程で閉じる条件と結果の 4 値、配布の記録の形と読み方。手順は `merged` / `progress-tracking` / `release` の SKILL.md が正 | | [ndf-agent-layers-unattended-run.md](ndf-agent-layers-unattended-run.md) | `/goal` の工程を conductor / supervisor / worker の 3 層で通す運転。持ち場 5 つ、報告の 2 段、続けさせる回数、上限（429）で中断した層の再開。手順は `development-workflow` の `references/agent-layers.md` が正 | | [ndf-context-window-metrics.md](ndf-context-window-metrics.md) | 会話の記録から context window を 3 層で測る部品（`transcript_agents.py`）の値の取り方と、`skill-stats --agents` の 4 つの表と印。値の取り方はこの文書が正 | diff --git a/docs/specifications/cross-refactoring-round-tests-and-assess.md b/docs/specifications/cross-refactoring-round-tests-and-assess.md new file mode 100644 index 00000000..ca2d06bb --- /dev/null +++ b/docs/specifications/cross-refactoring-round-tests-and-assess.md @@ -0,0 +1,358 @@ +# cross-refactoring: 群ごとに全体のテストが走り、打ち切りが猶予を待ち切り、飛ばしてよい条件が無く、文書の文言を固定するテストが採られていた → 群は範囲のテストで検証し、打ち切りは子の終了で戻り、飛ばすかを差分から判定し、文言固定テストを採らない + +## 目的 + +**群（適用ラウンド）と修正のコミットの検証は、範囲のテスト（`--round-test`）で行う。** 全体の +テスト（`--baseline-test`）は着手前と最終ゲートの 2 回だけ走る。範囲の外への波及は最終ゲートの +全体のテストが見る（#880）。 + +**テストの打ち切りは、子が終わった時点で戻る。** SIGTERM を無視する子には今のとおり猶予の後に +SIGKILL が届く（#883）。 + +**構造改善を飛ばしてよいかは、差分から機械で判定する（`refactor.py assess`）。** 飛ばしたことは +工程の記録と Pull Request の本文の 1 行に残り、通し忘れ（記録なし）と区別できる（#494）。 + +**`.md` の文言・見出し・表の並びを固定するテストは、テスト整備の提案でも適用の検証でも採らない** +（#723）。**リポジトリにもそのテストを置かない。** 文書が指す参照の実在は検査スクリプトが失敗 +として出す（#885）。 + +例: PR #844 の検査の持ち場（`--scope` は 6 ファイル）をこの仕組みでやり直すと、次のようになる。 + +| 項目 | 変更の前 | 今 | +| --- | --- | --- | +| 群 1 つの検証で走るテスト | 全体のテスト（1 群あたり約 8 分） | `--round-test` に渡した範囲のテスト（数秒） | +| 全体のテストを走らせる回数 | 着手前 1 回 ＋ 群ごと ＋ 修正コミットごと | 着手前 1 回 ＋ 最終ゲート 1 回 | +| テストが打ち切られたとき | 子がすぐ終わっても猶予の 5 秒を待ち切る | 子が終わった時点で戻る | +| テスト整備ラウンドが `.md` を読むテストを足したとき | そのまま採られ、後の文書の整理が「期待する振る舞いの変更」で取り消される | 提案の時点で見送り、適用の検証でも群を取り消す | + +文書だけを変える Pull Request では `assess` が「飛ばしてよい」（終了コード 3）を返し、検査の +持ち場は `cross-refactoring` を起動しない。 + +**手順と引数の表は +[`cross-refactoring` の SKILL.md](../../plugins/ndf/skills/cross-refactoring/SKILL.md)・ +[`docs/`](../../plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md)と、 +`development-workflow` の +[`workflow-modes.md`](../../plugins/ndf/skills/development-workflow/references/workflow-modes.md) +「構造改善の退避先」・ +[`stage-notes.md`](../../plugins/ndf/skills/development-workflow/references/stage-notes.md) +が正である。テストを書く規約は +[`tdd-cycle` の `test-quality.md`](../../plugins/ndf/skills/tdd-cycle/references/test-quality.md) +が正である。** ここに書き写さない。この文書が扱うのは、そこに書かない決定の理由と、進行の内部の契約である。 + +## 用語 + +本文は左の業務用語で書く。識別子は表・コードブロック・業務用語の初出の括弧書きにだけ置く。 +提案ラウンド・適用ラウンド・修正ラウンド・最終ゲートは +[結果なしの取り込みの用語](cross-refactoring-apply-intake.md#用語)と同じ意味で使う。 + +| 業務用語 | 識別子 | 何を指すか | +| --- | --- | --- | +| 範囲のテスト | `--round-test` / `round_test` | `--scope` のテストの置き場所を走らせるテストのコマンド。着手前・群の検証・修正のコミットごとに走る | +| 全体のテスト | `--baseline-test` / `baseline_test` | リポジトリのテストの全体。着手前と最終ゲートで走る | +| 群 | `rounds[].apply_rounds[]` | 適用ラウンド。書き換えるファイルが重ならない項目の集まりで、1 コミットにまとめて検証する | +| 実行集合の起点 | `round_test_roots` / `baseline_search_roots` | テストのコマンドの引数のうち、走らせる範囲を限る語。空なら全体を走らせるとみなす | +| 範囲のテストの解決 | `round_test_command(state)` | 群と修正の検証に使うコマンドを状態から返す。`round_test` が無ければ `baseline_test` | +| 本番コード | `production_code_changes` | コードの拡張子を持ち、テストの置き場所でないファイル | +| 飛ばす / 退避する | — | 飛ばす: 構造改善の工程で何も起動しない。退避する: `cross-refactoring` の代わりに `refactoring` 単独で進める | +| 文言固定テスト | `doc_wording_tests` | リポジトリで追跡している `.md` を読み、その文字列・見出し・表の並びを照合するテスト | +| 追跡している `.md` | `tracked_markdown` | `git ls-files -z '*.md'` の一覧 | + +## 対象範囲 + +| 扱う | 扱わない | +| --- | --- | +| 群と修正コミットの検証のコマンド、全体のテストを走らせる時点、`init` の関門と案内、検証の所要の記録 | 適用を並列にすること（#755 #754）、駆動を `scripts/` へ寄せること（#560 #870） | +| テストの打ち切り（`_kill_process_group`） | テストの一般的な高速化（#390 #884） | +| 構造改善を飛ばしてよいかの判定（`assess`）と、飛ばしたときの記録 | `projects-sync.sh` の引数の形（飛ばした記録に値を足さない） | +| テスト整備の提案・取り込み・適用の検証での文言固定テストの除外 | `--scope` をファイルより細かく絞ること（#622） | +| リポジトリに文言固定テストを置かない方針と、残すテストの分類の規則 | cross-review の母集合・待ち方（#892 #901） | + +## 背景 + +**群ごとに全体のテストが走っていた。** 範囲が狭い変更でも、1 項目を適用するたびに +`--baseline-test` を丸ごと回していた。PR #844 では群 1 つの検証に約 8 分かかり、現状固定テスト +10 項目に約 90 分を使って構造改善のラウンドへ入れず打ち切った。 + +**打ち切りが猶予を待ち切っていた。** 打ち切った親シェルがゾンビとして残り、グループの存否の +判定（`killpg(pgid, 0)`）がそれを「生きている」と数えた。子がすぐ終わっても毎回 5 秒待った。 + +**飛ばしてよい条件がどの Skill にも無かった。** 構造改善の所要の中央値は 1 本あたり 75.9 分で +（2026-09-01〜09-15）、数行の変更にも同じ費用がかかった。飛ばしたかどうかも記録に残らなかった。 + +**テスト整備ラウンドが `.md` の文言を固定するテストを足していた。** 後の提案ラウンドの文書の +整理が「テストの期待する振る舞いが変わっています」で取り消された。リポジトリ全体にも同じ形の +テストが数百関数あった。文言を照合するテストは、プロンプトの振る舞いを保証しない（同じ文面から +同じ結果が返るわけではない）。 + +## 決定と理由 + +| 決定 | 理由 | +| --- | --- | +| 群と修正コミットの検証は範囲のテストで行い、全体のテストは `init` と最終ゲートの 2 回に限る | 範囲外を触ったコミットは範囲の検査で落ちるため、群ごとに全体を走らせても範囲のテストより多くは分からない。範囲の外への波及は最終ゲートの 1 回で見る | +| 範囲のテストは明示の引数にし、`--scope` から組み立てない | テストのコマンドの形はリポジトリごとに違う。範囲を決めた起動する側が範囲のテストも決める | +| `--round-test` を省けば変更の前と同じ検証をし、範囲より広い全体のテストには案内だけを出す | 互換を保つ。範囲より広いことは誤りではなく時間がかかるだけなので止めない | +| 単独の起動でも、2 つのテストが違えば最終ゲートで全体のテストを 1 回走らせる | 群を範囲のテストで検証すると、単独の起動では範囲の外への波及を見る機会がほかに無い | +| 最終ゲートの修正のコミットはテストを走らせず、直後の最終ゲートが判定する | `--ci-check` の実行で手元のテストを 1 度も走らせない排他（#436）と衝突しない。最終ゲートの修正は全体のテストの失敗を直すもので、範囲のテストでは直ったか分からない | +| 飛ばす条件は「本番コードの差分が無い、または変更が 10 行以下」とし、差分から機械で判定する | 「改善の対象になる構造を持たない」は機械で決まらないため行数で近似する。#489（本番コード 1 行）を飛ばせる値として置いた | +| テストの置き場所の判定は既存の `is_test_path` を使い、テストだけの差分も飛ばす | 同じ印の定義を 2 か所に持たない。構造改善はテストを守りに本番コードを整える工程で、テストそのものは対象にしない | +| `assess` は cross-refactoring の `refactor.py` に置く | 判定に使う知識（テストの置き場所・範囲の扱い）を cross-refactoring が持つ | +| 飛ばしたときも工程「構造改善」を記録し、区別は Pull Request の本文の 1 行が持つ | 工程の値を増やすと盤面の単一選択に響き、記録の入口の引数を変えると hook の観測の形に触る。通し忘れは「記録なし」のまま残るため見え方が分かれる | +| 打ち切りは親シェルを回収してからグループの存否を見る。判定はグループの存否のまま | 親の終了だけで打ち切ると、SIGTERM を無視する子がグループに残って作業ディレクトリを書き換え続ける | +| 文言固定テストは提案の取り込みと適用の検証の 2 か所で弾き、誤検知は群の取り消しの側に倒す | 提案の基準だけでは、担当が書いたテストに混じったときに止まらない。判定を追跡している `.md` との一致で行えば、一時ファイルの `.md` を入力に渡す検査スクリプトのテストは弾かない | +| リポジトリの文言固定テストを削り、参照の実在は検査スクリプトへ寄せる | 参照の実在は構造を見ており残す価値があるが、pytest で `.md` を読む形のままでは文言固定と区別が付かない | +| 残すかどうかは「その `.md` をスクリプトが実行時に読むか、テストがその中のコードを実行するか」の 1 問で決める | どちらでもなければテストが見ているのは文言である。文書の値とコードの定数の一致（スクリプトが読まない文書）も削る。食い違いに気付けない場面が増えることは受け入れた費用で、文書の正しさはレビューが見る | + +## 仕様 + +### 常に成り立つ条件 + +| 条件 | 破れたときの扱い | +| --- | --- | +| 群の検証の記録（`rounds[].verifications[]`）の `command` は、範囲のテストの解決が返すコマンドである | 群の検証と修正のコミットの検証は `round_test_command(state)` だけから取る | +| 範囲のテストを全体のテストと別に渡した実行では、全体のテストが走るのは `init` の 1 回と最終ゲートの判定（修正ラウンドの後の判定し直しを含む）だけである | 全体のテストを呼ぶのは `init` の `_run_baseline_test` と最終ゲートの `_local_gate` の 2 か所に限る | +| `round_test` を持たない状態ファイル（この変更の前の実行）の再開は、変更の前と同じ検証をする | 範囲のテストの解決が `baseline_test.command` を返す | +| `--scope` のテストの置き場所は、群の検証が走らせるテストの実行集合に入る | `init` が終了コード 4 で止まる。状態ファイルは作らない | +| 打ち切りの後に、テストのプロセスグループの子が残らない | 猶予を過ぎたらグループへ SIGKILL を送る | +| `assess` が判定できないとき（終了コード 2）を、飛ばしてよいと読まない | 呼ぶ側は値を直して打ち直す | +| テスト整備ラウンドが採る項目の `target` は `.md` でない | 取り込みが見送り（`deferred_items`）へ理由付きで入れる | +| 適用ラウンドのコミットが、追跡している `.md` を指すテストの行を足さない | 群を失敗として取り消す | + +### 2 つのテストと走る時点 + +| 引数 | 意味 | 既定 | 走る時点 | +| --- | --- | --- | --- | +| `--baseline-test CMD` | 全体のテスト（必須） | — | `init` の 1 回、最終ゲートの判定ごとに 1 回 | +| `--round-test CMD` | 範囲のテスト（任意） | `--baseline-test` と同じ | `init` の 1 回、群の検証（`verify-round`）ごと、修正ラウンドの修正のコミットごと | + +最終ゲートの修正のコミット（`merge-final-fix`）はテストを走らせない。範囲と申告だけを見て、 +テストの合否は直後の最終ゲートが採った側（手元の全体のテスト / 継続的統合）で 1 度だけ見る。 + +担当の CLI へ渡すテストのコマンドも同じ解決に揃える。起動の部品（`launch-cli.sh`）は、適用と +修正のプロンプトへ `.round_test.command // .baseline_test.command` を `$RF_ROUND_TEST` として渡す。 +最終ゲートの修正のプロンプトは全体のテストのままである。 + +### `init` の関門と案内 + +| 状況 | 動き | 終了コード | +| --- | --- | ---: | +| `--round-test` を渡した | `--scope` のテストの置き場所が、範囲のテストの実行集合の起点のどれかと同じか、その下にあることを求める。全体のテストには関門を当てない | 外れれば 4 | +| `--round-test` を省いた | 変更の前のとおり、全体のテストの実行集合に置き場所が入ることを求める | 外れれば 4 | +| `--round-test` を省き、全体のテストの起点が無いか、置き場所より広い | 案内を 1 行出して続ける（下の文面） | 0 | +| 範囲のテストが成功しない（テストが 1 件も集まらない終了コード 5 を含む）・時間内に終わらない | 「範囲のテストが成功しません」または打ち切りの 1 行で止まる。全体のテストの結果とは別に出す | 4 | +| `--round-test` を省いたか、全体のテストと同じ文字列 | 全体のテストを 1 回だけ実行し、その結果を `round_test` にも写す | — | + +案内の文面: + +```text +ℹ --baseline-test は --scope より広い範囲を走らせます。群ごとの検証を短くするには --round-test に範囲のテストを渡します（例: <プログラム> <テストの置き場所>） +``` + +**実行集合の起点（範囲のテスト）** はコマンドを語に分けて次の順で決める。 + +1. 先頭の語（プログラム名）・`-` で始まる語・絶対パスは見ない +2. 値を取ると分かっているオプション（`VALUE_OPTIONS`。`uv run` の `--project` `--with` ほか、 + pytest の `--rootdir` `-k` `-n` ほか）の直後の語は数えない。**値を取るか分からないオプションの + 直後の語は数える。** 値とみなして消すと `pytest --verbose tests/unit` の唯一の対象が消え、全体を + 覆うとみなして関門を通してしまうため、止める側に倒す +3. pytest のノード ID（`<ファイル>::<名前>`）は `::` より前のファイルの部分で読む +4. 作業ディレクトリの根そのもの（正規化して `.` になる語）は数えない +5. 残った語のうち、実在するディレクトリと、テストの置き場所に当たる実在するファイルを起点にする。 + テストの置き場所でないファイル（`bash scripts/run-scope-tests.sh` のようなラッパー）は数えない。 + ラッパーの中身は解析しない + +起点が 1 つも残らないコマンドは全体を走らせるとみなし、関門を通す。範囲の外を走らせても、最終 +ゲートの全体のテストが見る。 + +**再開で渡した `--round-test` は反映しない。** 状態と違えば「反映しない」の 1 行を出す（再開の +表の扱いは [参加者の確定仕様](cross-refactoring-participants.md#再開で渡した引数の扱い)）。 + +### 最終ゲート + +| 起動のされ方 | 2 つのテスト | 動き | +| --- | --- | --- | +| 単独 | 同じ（`--round-test` を省いた） | テストを走らせず、Step 7 を `cross-review` へ渡す（変更の前のまま） | +| 単独 | 違う | 全体のテスト（`--ci-check` があれば継続的統合）を 1 回通す。通れば `cross-review` へ渡し、落ちれば工程として起動したときと同じ修正ラウンドへ入る | +| 工程として（`--workflow-step`） | どちらでも | 変更の前のとおり、全体のテストか継続的統合で判定する | + +最終ゲートの記録（`final_gate.checks[]`）は各件に `command` と `seconds` を持つ。 + +### テストの打ち切り + +```text +SIGTERM をグループへ送る +猶予（既定 5 秒）が切れるまで 0.2 秒ごとに: + proc.poll() ← 親シェルを回収する（ゾンビを残さない） + グループに生きたプロセスが無い → 戻る +猶予を過ぎたら SIGKILL をグループへ送る +``` + +| 入力 | 戻るまで | +| --- | --- | +| `exec sleep 30`、上限 1 秒 | 約 1.2 秒（SIGTERM で子が終わった次の点検で戻る） | +| `trap '' TERM; (sleep 3 && touch x) & sleep 30`、上限 1 秒・猶予 1 秒 | 約 2 秒（子がグループに残るため猶予を待ってから SIGKILL） | + +### 構造改善を飛ばしてよいかの判定（`assess`） + +```bash +python3 "$RF/refactor.py" assess --base origin/develop; rc=$?; echo "exit=$rc" +``` + +| 引数 | 意味 | 既定 | +| --- | --- | --- | +| `--base REF` | 起点。`...HEAD` の差分を見る（必須） | — | +| `--max-lines N` | 本番コードの変更行（追加 ＋ 削除）がこれ以下なら飛ばしてよい | 10 | + +| 出力の行 | 例 | +| --- | --- | +| 判定 | `判定: 通す` / `判定: 飛ばしてよい` | +| 理由 | `理由: 本番コードの差分がありません` / `理由: 本番コードの変更が 4 行で、上限 10 行以下です` / `理由: 本番コードの変更が 40 行です` | +| 数えた値 | `本番コード: 1 ファイル・40 行（<パス>）` | + +| 終了コード | 意味 | 呼ぶ側の動き | +| ---: | --- | --- | +| 0 | 通す | 退避の 3 条件を見たうえで `cross-refactoring` を起動する | +| 3 | 飛ばしてよい | 何も起動しない。工程「構造改善」を記録し、Pull Request の本文に 1 行残す | +| 2 | `` を解けない・引数の誤り | 値を直して打ち直す。**飛ばさない** | + +**本番コードの数え方:** `git diff --numstat -z --no-renames ...HEAD` の各行を見る。rename は +旧パスの削除と新パスの追加に分かれ、両方のパスで判定する。拡張子が `CODE_EXTENSIONS`（`.py .sh +.bash .js .mjs .cjs .ts .tsx .jsx .php .rb .go .rs .java .kt .swift .c .h .cc .cpp .cs`）のどれかで、 +`is_test_path` が偽のファイルを数える。行数は追加と削除の和で、削除だけのファイルも入る。バイナリ +（`numstat` が `-`）は 0 行と数える。`.md`・`.json`・`.yml`・`.toml`・画像・テストの置き場所は数えない。 + +**`assess` は判定の材料を出すだけで、退避の 3 条件（テストが無い・CLI が使えない・範囲を絞れない）は +見ない。** それらは `init` が止めて知らせる。状態ファイルも読まない（`init` より前に呼ぶため）。 + +### 飛ばしたときの記録 + +| 記録先 | 何を残すか | +| --- | --- | +| 進行の記録 | 工程「構造改善」（`projects-sync.sh <課題番号> stage "構造改善"`。値を増やさない） | +| Pull Request の本文 | `構造改善: 飛ばした（）` の 1 行 | +| 持ち場の報告 | 同じ 1 行を理由に含める | + +### 文言固定テストを採らない（テスト整備と適用） + +| 段 | 何をするか | 理由の文面 | +| --- | --- | --- | +| 提案の基準（`prompts/propose-tests.md`） | `.md` の文言・見出し・表の並びを固定するテストを提案しないと定める。変更が文書中心か、受け入れ条件のテストが既にあるかを条件にしない | — | +| 提案の取り込み（`merge_test_proposals`） | `target` の `#` より前が `.md` で終わる提案を見送り（`deferred_items`）へ入れる | `文書の文言を固定するテストは足さない` | +| 適用の検証（`verify_apply_round`） | テストの期待値の変更の検査の直後に `doc_wording_tests` を呼び、当たりがあれば群を失敗にする。テスト整備と提案の両方の群に効く | `文書の文言を固定するテストは足さない（<ファイル>: <文字列>）` | + +**当たりの判定:** テストのファイルの追加行（置き換えた行を含む）から文字列リテラルを取り出し、 +`.md` で終わり、追跡している `.md` のパスと一致するか、`/` の区切りで揃えたパスの末尾と一致すれば +当たりとする。`tmp_path / "a.md"` のように追跡していない名前は当たらない。`"README.md"` のように +追跡している名前と同じ一時ファイルを使うテストは当たる（群が取り消されるだけで、Pull Request に +文言固定テストが残る側には倒れない）。 + +次の名前を経由する場合も当たりとする。 + +1. テストのファイルの変更の後の内容を AST で読み、モジュールの直下の代入のうち、右辺の文字列が追跡している `.md` を指す名前 +2. 同じディレクトリの補助モジュールから `from <補助> import <名前>` した名前（補助モジュールを群で触っていなければ git から読み、1 と同じ判定をする） +3. 追加行が 1・2 の名前を識別子として使っていれば当たり + +動的に組み立てたパス（`glob` の結果など）は追わない。提案の基準とレビューが見る。 + +### リポジトリに文言固定テストを置かない（#885） + +**判定の問いは 1 つである。その `.md` をスクリプトが実行時に読むか、テストがその中のコードを +実行するか。** どちらでもなければテストは文言を見ており、置かない。 + +| 形 | 扱い | +| --- | --- | +| 文書の言い回し・見出し・表の並び・語の有無を照合する | 置かない | +| 2 つの文書の表の並びが一致する | 置かない | +| 文書の中のコマンドの形を照合する（`gh` に `--repo` が付く など） | 置かない | +| 消した名前のファイルが無いこと | 置かない（改名の後始末の確認で、役目を終える） | +| スクリプトが読まない文書の値とコードの定数の一致 | 置かない。文書の正しさはレビューが見る | +| 文書が指すファイルの実在 | pytest に置かず、検査スクリプトが失敗として出す（リンクは `scripts/check-markdown-links.py`） | +| 文書に埋め込んだ bash を取り出して実行する | 残す（文書の中にしか無いロジックの振る舞いを実行で確かめる） | +| スクリプトが実行時に読むプロンプトのプレースホルダと解析する語 | 残す（スクリプトとプロンプトの契約）。説明の文言は照合しない | +| スクリプトが実行時に生成した `*-prompt.md` | 残す（リポジトリの文書ではなく、スクリプトの出力） | +| スクリプトが実行時に読む表とコードの一致 | 残す（機械が読む契約。例 `vocabulary.py` が読む `vocabulary.md`） | +| frontmatter・hook の配線（YAML・JSON） | 残す（ランタイムが読む構造） | +| Markdown の構文（バッククォートの対応など） | 残す（文言ではなく構文） | +| 一時ディレクトリの `.md` を入力に渡す検査スクリプトのテスト | 残す（入力） | + +この方針はテストを書く規約の 3 か所が持つ。`tdd-cycle` の `references/test-quality.md` の +「8. `.md` の文言を照合する」と「削除してよいテスト」、`quality-gates` の SKILL.md の「受け入れ +条件との対応」、`AGENTS.md` の「DON'T」。受け入れ条件のうち文書で満たすものは、文書の該当の節を +読んで確かめる。 + +## データ・設定 + +### 状態ファイルに増える項目 + +```json +{ + "round_test": {"command": "uv run --with pytest pytest plugins/ndf/skills/cross-refactoring/tests -q", "status": "green", "checked_at": "…"}, + "rounds": [{"verifications": [{"apply_round": 1, "command": "…", "status": "pass", "seconds": 4.1}]}], + "final_gate": {"checks": [{"mode": "test", "command": "…", "status": "pass", "detail": "…", "seconds": 58.3}]} +} +``` + +| 項目 | 新規の状態ファイル | この変更の前の状態ファイル | +| --- | --- | --- | +| `round_test` | `command`・`status`・`checked_at`。`--round-test` を省けば全体のテストと同じ値 | 無い。範囲のテストの解決が `baseline_test` を返す | +| `rounds[].verifications[].seconds` | 群の検証の所要の秒数（小数 1 桁） | 無い | +| `final_gate.checks[].command` / `seconds` | 最終ゲートの検査のコマンド（継続的統合なら検査の名前）と所要の秒数 | 無い | + +**全体のテストの実行の回数は、状態ファイルから数えられる。** `init` の 1 回は `baseline_test.status`、 +最終ゲートの 1 回は `final_gate.checks[]`（`mode` が `test` の件）にある。 + +## 運用 + +**構造改善の工程に入ったら、最初に `assess` を実行する。** 通すときは `--round-test` に `--scope` の +テストの置き場所だけを走らせるコマンド、`--baseline-test` に全体のテストを渡す（`stage-notes.md`）。 +全体のテストは `quality-gates` の全体のテストと重ねて回さない。 + +**構造改善の工程を通常どおり通せることは、次の 4 つで確かめた。** #880 #883 #494 #723 を実装した +Pull Request #917 自身の検査の持ち場で `cross-refactoring --workflow-step` を通し、その実行が 4 つとも +満たした（[記録](https://github.com/devbasex/ai-plugins/pull/917#issuecomment-5797876310)）。 +同じ変更の後に構造改善の工程が詰まったら、同じ 4 つで見直す。 + +| 条件 | 何で見るか | +| --- | --- | +| `--round-test` と `--baseline-test` を分けて渡し、利用者の指示による中止なしに最終ゲートまで完走する | 実行の結末 | +| 全体のテストの実行が `init` と最終ゲートの 2 回だけで、群の検証の `seconds` の中央値が 60 秒以下 | 状態ファイル（`.cross_refactoring/cross-refactoring-rf<番号>-state.json`） | +| テスト整備ラウンドが採った項目に、追跡している `.md` を読んで照合するテストが 0 件 | Pull Request の差分と見送りの一覧 | +| 提案ラウンドの群が「テストの期待する振る舞いが変わっています」で取り消された件数のうち、文言固定テストが原因のものが 0 件 | 状態ファイルの失敗の理由 | + +**行数の上限 10 は初期値である。** 構造改善を通した Pull Request の本番コードの変更行と、採用された +項目の数の関係は測っていない。飛ばした Pull Request の件数と合わせて振り返りで見直す。群の検証の +中央値 60 秒は範囲のテストが数秒で終わる前提の上限で、範囲にテストが多い Skill では超えうる。 +超えたら値ではなく範囲の決め方を見直す。 + +## テスト観点 + +| 観点 | 確かめ方 | +| --- | --- | +| `init --round-test X` が `round_test.command` に X を保存し、省くと全体のテストと同じになり、そのとき `init` のテストの実行が 1 回であること | `plugins/ndf/skills/cross-refactoring/tests/test_init.py` | +| 範囲のテストが失敗・終了コード 5 のとき `init` が終了コード 4 で止まること | 同上 | +| 範囲の置き場所の外を走らせる範囲のテスト（ファイルの名指し・`uv run --project .` を前置した形・ノード ID を含む）で止まり、ラッパーのスクリプトだけのコマンドでは止まらないこと。値を取るか分からないオプションの直後の対象を起点に数えること | `cross-refactoring/tests/test_scope_gate.py` | +| 案内が、起点の無いコマンドと範囲より広い起点でだけ出ること | 同上 | +| 群の検証と修正の取り込みが範囲のテストだけを実行し、検証の記録が 0 以上の `seconds` を持つこと | `cross-refactoring/tests/test_verify_round.py` / `test_abandon_items.py` | +| `round_test` を持たない状態ファイルで群の検証が全体のテストを実行すること | `cross-refactoring/tests/test_verify_round.py` | +| 単独の起動と工程としての起動の両方で、最終ゲートの全体のテストの呼び出しが 1 回であること。単独で 2 つのテストが同じならテストを走らせず、`--ci-check` があれば継続的統合で代え、落ちれば修正ラウンドへ入ること。記録が `command` と `seconds` を持つこと | `cross-refactoring/tests/test_final_gate.py` | +| 適用と修正の担当のプロンプトに範囲のテストのコマンドが入ること | `cross-refactoring/tests/test_launch_round_test.py` | +| `exec sleep 30` を上限 1 秒で打ち切ると 2 秒以内に戻ること。SIGTERM を無視する子に SIGKILL が届くこと。打ち切りの 3 件の所要の合計が 10 秒未満であること | `cross-refactoring/tests/test_git_facts.py`（`pytest --durations`） | +| `.md` だけ・テストだけ・設定の値だけの差分、本番コード 10 行で終了コード 3、11 行と変更を伴う rename で 0、`--max-lines` で上限が変わり、起点を解けないと 2、バイナリは 0 行と数えること | `cross-refactoring/tests/test_assess.py` | +| `target` が `SKILL.md#節` の提案が見送りへ理由付きで入ること | `cross-refactoring/tests/test_test_rounds.py` | +| 追跡している `.md` のリテラル・末尾が一致するリテラル・既存の定数・補助モジュールから import した定数が当たり、追跡していない名前と変更していない行は当たらないこと。適用の検証がその理由で失敗すること | `cross-refactoring/tests/test_doc_wording_tests.py` | +| 手順書・規約の該当の節（`SKILL.md` の引数の表、`workflow-modes.md`「構造改善の退避先」、`stage-notes.md` の構造改善、`propose-tests.md`、テストを書く規約の 3 か所）が上の契約を書くこと | 文書を読んで確かめる（文言を固定するテストは書かない） | +| 文書が指す参照を 1 つ消すと検査スクリプトが失敗を返すこと | `python3 scripts/check-markdown-links.py --root .`（`scripts/tests/test_check_markdown_links.py`） | +| 全体のテストが通ること | `uv run --project plugins/playwright-kit/skills/playwright-kit-ops --with pytest pytest . -q -n 4` | + +## 関連リンク + +- [issue #880](https://github.com/devbasex/ai-plugins/issues/880) — 1 項目ごとに全体のテストを走らせる +- [issue #883](https://github.com/devbasex/ai-plugins/issues/883) — 打ち切りが猶予を待ち切る +- [issue #494](https://github.com/devbasex/ai-plugins/issues/494) — 構造改善を飛ばしてよい条件が無い +- [issue #723](https://github.com/devbasex/ai-plugins/issues/723) — テスト整備ラウンドが文言固定テストを足す +- [issue #885](https://github.com/devbasex/ai-plugins/issues/885) — リポジトリの文言固定テスト +- [PR #911](https://github.com/devbasex/ai-plugins/pull/911)（設計） / [PR #918](https://github.com/devbasex/ai-plugins/pull/918)（#885 の実装） / [PR #917](https://github.com/devbasex/ai-plugins/pull/917)（#880 #883 #494 #723 の実装） +- [結果なしの取り込み](cross-refactoring-apply-intake.md) — 取り込みと適用ラウンドの開き直し +- [cross-refactoring の参加者](cross-refactoring-participants.md) — 再開で渡した引数の扱い +- [説明文書の検査](doc-consistency-checks.md) — 参照の実在を見るリンクの検査 +- [`cross-refactoring` の手順](../../plugins/ndf/skills/cross-refactoring/SKILL.md) +- [テストの品質の規約](../../plugins/ndf/skills/tdd-cycle/references/test-quality.md) diff --git a/docs/specifications/doc-consistency-checks.md b/docs/specifications/doc-consistency-checks.md index bcf0f44e..228e1a26 100644 --- a/docs/specifications/doc-consistency-checks.md +++ b/docs/specifications/doc-consistency-checks.md @@ -16,6 +16,8 @@ - **`issues/`（`issues/old/` を含む）はリンクの検査の走査に入る。** 設計文書と計画は分割や 移設のたびに参照が壊れうるため、`docs/` へ移る前の期間も検査する - **インラインコードの中の記法はリンクとして読まない。** 記法を説明する例で誤って落ちない +- **文書が指す参照の実在は、pytest ではなくこの検査が失敗として出す。** `.md` を読む pytest は + 文言の照合と区別が付かないため置かない（[#885 の分類の規則](cross-refactoring-round-tests-and-assess.md#リポジトリに文言固定テストを置かない885)） - **検査 J は正本（`docs/versioning-and-distribution.md`）の章を読む。** 章に並ぶ版数の基底を 現行版と比べる規則に加え、版の形の表と「次を開発するなら」の例を**例どうしで**比べる @@ -198,5 +200,6 @@ graph TD - [issue #543](https://github.com/devbasex/ai-plugins/issues/543) / [PR #596](https://github.com/devbasex/ai-plugins/pull/596)（設計） / [PR #600](https://github.com/devbasex/ai-plugins/pull/600)（実装） — `issues/` の走査とインラインコード - [issue #499](https://github.com/devbasex/ai-plugins/issues/499) / [PR #594](https://github.com/devbasex/ai-plugins/pull/594)（実装） — 検査 J の読む先を正本へ移す - [issue #566](https://github.com/devbasex/ai-plugins/issues/566) / [PR #604](https://github.com/devbasex/ai-plugins/pull/604)（設計） / [PR #609](https://github.com/devbasex/ai-plugins/pull/609)（実装） — 版の形の表と次の開発の例 +- [issue #885](https://github.com/devbasex/ai-plugins/issues/885) / [PR #918](https://github.com/devbasex/ai-plugins/pull/918)（実装） — 参照の実在を見ていた pytest をこの検査へ寄せる - [版と配布の正本](../versioning-and-distribution.md) - [NDF 知識構造・Kiro CLI 仕様](ndf-knowledge-and-kiro.md) — `AGENTS.md` と正本の役割の分け方 diff --git a/issues/old/README.md b/issues/old/README.md index f850695a..6d4df5a0 100644 --- a/issues/old/README.md +++ b/issues/old/README.md @@ -48,6 +48,7 @@ | [#828](https://github.com/devbasex/ai-plugins/issues/828) / [#680](https://github.com/devbasex/ai-plugins/issues/680) | サブエージェントに Skill 本文を丸ごと読ませず、仕事を分ける器を比べて選ぶ（マイルストーン 26「17 トークン消費の削減」）。確定仕様は [ndf-worker-agent-and-skill-excerpts.md](../../docs/specifications/ndf-worker-agent-and-skill-excerpts.md) | [milestone-26-worker-agent/](milestone-26-worker-agent/issue-828-680-requirements.md)（要求・設計・決定・計画の 4 本） | | [#892](https://github.com/devbasex/ai-plugins/issues/892) / [#901](https://github.com/devbasex/ai-plugins/issues/901) | cross-review の母集合にホストを入れ、supervisor が worker の途中の通知で止まらないようにする（マイルストーン 26「17 トークン消費の削減」）。確定仕様は [cross-review-participants-and-seats.md](../../docs/specifications/cross-review-participants-and-seats.md) / [ndf-token-waits-and-context-cut.md](../../docs/specifications/ndf-token-waits-and-context-cut.md) | [milestone-26-review-pool-and-interim-wait/](milestone-26-review-pool-and-interim-wait/issue-892-901-requirements.md)（要求・設計・決定・計画の 4 本） | | [#895](https://github.com/devbasex/ai-plugins/issues/895) | 区間の切れ目の再起動と次のコマンドの入力を前景の中継で自動にする（マイルストーン 26「17 トークン消費の削減」）。確定仕様は [ndf-relay-segment-restart.md](../../docs/specifications/ndf-relay-segment-restart.md) / [ndf-token-waits-and-context-cut.md](../../docs/specifications/ndf-token-waits-and-context-cut.md) | [milestone-26-relay-restart/](milestone-26-relay-restart/issue-895-requirements.md)（要求・設計・決定・計画の 4 本） | +| [#880](https://github.com/devbasex/ai-plugins/issues/880) / [#883](https://github.com/devbasex/ai-plugins/issues/883) / [#494](https://github.com/devbasex/ai-plugins/issues/494) / [#723](https://github.com/devbasex/ai-plugins/issues/723) / [#885](https://github.com/devbasex/ai-plugins/issues/885) | cross-refactoring の群を範囲のテストで検証し、打ち切りを子の終了で戻し、構造改善を飛ばしてよいかを差分から判定し、`.md` の文言固定テストを採らず置かない（マイルストーン 26「17 トークン消費の削減」のまとまり 2）。確定仕様は [cross-refactoring-round-tests-and-assess.md](../../docs/specifications/cross-refactoring-round-tests-and-assess.md) | [milestone-26-cross-refactoring-fixes/](milestone-26-cross-refactoring-fixes/issue-880-885-requirements.md)（要求・設計・決定・計画の 5 本） | ## 計画と調査資料 diff --git a/issues/old/milestone-26-cross-refactoring-fixes/issue-880-883-494-723-plan.md b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-883-494-723-plan.md new file mode 100644 index 00000000..b10dd02b --- /dev/null +++ b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-883-494-723-plan.md @@ -0,0 +1,88 @@ +# #880 / #883 / #494 / #723: cross-refactoring の是正 — 実装計画（PR 2） + +## 関連リンク + +- 要求: [issue-880-885-requirements.md](issue-880-885-requirements.md) +- 設計: [issue-880-885-design.md](issue-880-885-design.md)（「実装の分け方と順序」の PR 2 の行） +- 決定: [issue-880-885-design-decisions.md](issue-880-885-design-decisions.md) +- 設計 PR #911（関門 1 承認 2026-09-23、develop `ff9754e7`） + +## モード + +standard（起動指示のとおり。設計は承認済みで、この PR は実装だけを載せる）。 + +## 目的と非目的 + +達成したい状態: +- 群の検証が範囲のテストだけを走らせ、全体テストは `init` と最終ゲートの 2 回に寄る（#880） +- 打ち切りが、子が終わった時点で戻る（#883） +- 構造改善を飛ばしてよいかを差分から判定できる（#494） +- `.md` の文言を固定するテストが提案・適用ラウンドで採られない（#723） + +やらないこと: +- `.md` の文言固定テストの削除・検査スクリプト・`tdd-cycle`・`quality-gates`・`AGENTS.md`（PR 1、#885 の担当） +- AC19a〜AC19c（PR 1）と AC20〜AC23（次の検査の持ち場で PR 1 の取り込み後に測る） +- `waiting.md`・`agent-layers.md`（#892 #901 が触る）、`CLAUDE.md` の cross-review の節 + +## 受け入れ条件 + +要求文書の AC1〜AC18（AC19 系は PR 1）と AC24。検証手段は設計の「テスト設計」の表に従う。 +AC8・AC15・AC16 は文書を読んで確かめる（文言を固定するテストは書かない）。 + +## 修正対象 + +- `plugins/ndf/skills/cross-refactoring/scripts/refactor.py` +- `plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/{setup,converge,gate}.py` +- `plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/assess.py`（新設） +- `plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/{gitfacts,scope,proposals,verify}.py` +- `plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md` +- `plugins/ndf/skills/cross-refactoring/SKILL.md` +- `CLAUDE.md` の cross-refactoring の節 +- `plugins/ndf/skills/development-workflow/references/{workflow-modes,stage-notes}.md` +- `plugins/ndf/skills/cross-refactoring/tests/`（テストの追加・待ちの短縮） + +## タスク分解 + +### Task 1: 打ち切りが子の終了で戻る（#883） +- **対象:** `gitfacts.py` の `_kill_process_group`、`tests/test_git_facts.py` +- **変更:** SIGTERM 後の点検で `proc.poll()` を先に呼ぶ。既存 3 件の確認の待ちを 1.5 秒へ縮め、`exec sleep 30` のテストを足す +- **満たす AC:** AC9・AC10・AC11 +- **進め方:** `exec sleep 30` が 2 秒以内に戻るテストを先に書いて落とす → 1 行の修正 + +### Task 2: `assess` の副命令（#494） +- **対象:** `gitfacts.py` の `production_code_changes`、`commands/assess.py`、`refactor.py`、テスト +- **変更:** `git diff --numstat --no-renames ...HEAD` から本番コードを数え、終了コード 0 / 3 / 2 と 3 行を出す +- **満たす AC:** AC12・AC13・AC14 +- **進め方:** 一時リポジトリで各差分を作るテストを先に書く → 実装 + +### Task 3: `--round-test` と検証の秒数（#880） +- **対象:** `refactor.py` の `init` 引数、`commands/setup.py`・`converge.py`・`gate.py`、`scope.py` の `round_test_hint`・`round_test_roots`、テスト +- **変更:** 状態の `round_test` の保存・`init` での実行・`_round_test_command(state)`・`verify-round` と修正コミットの検証の差し替え・`seconds` の記録・単独起動の最終ゲートの全体テスト 1 回・案内の 1 行 +- **満たす AC:** AC1〜AC7 +- **進め方:** AC ごとに失敗するテスト → 実装 + +### Task 4: `.md` の文言固定テストを採らない（#723） +- **対象:** `proposals.py` の `merge_test_proposals`、`verify.py` の `doc_wording_tests` と `verify_apply_round`、`prompts/propose-tests.md`、テスト +- **満たす AC:** AC16・AC17・AC18 +- **進め方:** 見送り・失敗のテストを先に書く → 実装。`propose-tests.md` は文書の追記 + +### Task 5: 文書（#880 #494） +- **対象:** `SKILL.md`・`CLAUDE.md`・`workflow-modes.md`・`stage-notes.md` +- **満たす AC:** AC8・AC15 +- **進め方:** テスト駆動を適用しない（文書。文言を固定するテストは書かない） + +## リスクと対処 + +| リスク | 対処 | +| --- | --- | +| `gitfacts.py`（1182 行）・`apply.py`（1156 行）が大きい | 実装の後の構造改善で足りる（触るのは関数単位で狭く、テストが厚い）。タスクごとに cross-refactoring のテストを通す | +| 既存の `.md` 文言固定テストが文書の変更で落ちる | 削除は PR 1 の担当。この PR の変更で壊れたものだけを最小に直す | + +## 切り戻し手順 + +コードと文書だけの変更で、状態ファイルは `round_test` が無ければ変更前の振る舞いに戻る（AC7）。PR の revert で戻せる。 + +## 完了の定義 + +- [ ] AC1〜AC18（AC19 系を除く）を満たし、条件ごとにテストか文書の節が対応する +- [ ] AC24: `uv run --project plugins/playwright-kit/skills/playwright-kit-ops --with pytest pytest . -q -n 4` が通る diff --git a/issues/issue-880-885-design-decisions.md b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-design-decisions.md similarity index 91% rename from issues/issue-880-885-design-decisions.md rename to issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-design-decisions.md index 81f3a455..5479acbe 100644 --- a/issues/issue-880-885-design-decisions.md +++ b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-design-decisions.md @@ -121,3 +121,14 @@ cross-refactoring のコードとテストに閉じ、範囲のテストがあ P3' を残す形は採らない。文書の値の照合であることは言い回しの照合と変わらず、残すと「どこまでが一致の 検査か」を毎回判断することになる。削った結果、文書とコードが食い違っても気付けない場面は増える。 これは #885 が受け入れた費用である。 + +### 決定 14: 最終ゲートの修正コミットはテストを走らせず、直後の最終ゲートが判定する + +AC2 は当初、最終ゲートの修正のコミットでも範囲のテストを走らせると定めていた。実装で、これが #436 の +決定 11（`--ci-check` を渡した実行では手元のテストを 1 度も走らせない排他）と衝突すると分かった。 +修正コミットごとに範囲のテストを走らせると、`--ci-check` の実行でこの経路だけが手元のテストを走らせる。 + +今の振る舞い（修正コミットは範囲と申告だけを見て、テストの合否は直後の `final-gate` が採った側 — +手元の全体テスト / 継続的統合 — で 1 度だけ見る）を採る。最終ゲートの修正は全体テストの失敗を +直すものであり、範囲のテストを通しても直ったかは分からない。見るべきものは直後の最終ゲートが見る。 +AC2 の文言をこれに合わせて直した。 diff --git a/issues/issue-880-885-design.md b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-design.md similarity index 99% rename from issues/issue-880-885-design.md rename to issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-design.md index 5a1d0013..6c246866 100644 --- a/issues/issue-880-885-design.md +++ b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-design.md @@ -127,7 +127,7 @@ graph LR | 引数 | 意味 | 既定 | 実行する時点 | | --- | --- | --- | --- | | `--baseline-test CMD` | 全体のテスト（必須） | — | `init` の 1 回、最終ゲートの 1 回 | -| `--round-test CMD` | 範囲のテスト（任意） | `--baseline-test` と同じ | `init` の 1 回、群の検証（`verify-round`）ごと、修正コミットごと、最終ゲートの修正コミットごと | +| `--round-test CMD` | 範囲のテスト（任意） | `--baseline-test` と同じ | `init` の 1 回、群の検証（`verify-round`）ごと、修正コミットごと（最終ゲートの修正コミットは走らせない。決定 14） | 状態ファイルに足す欄: diff --git a/issues/issue-880-885-requirements.md b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-requirements.md similarity index 98% rename from issues/issue-880-885-requirements.md rename to issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-requirements.md index 06a3f785..703d343d 100644 --- a/issues/issue-880-885-requirements.md +++ b/issues/old/milestone-26-cross-refactoring-fixes/issue-880-885-requirements.md @@ -62,7 +62,7 @@ PR #844（#829 #830）の `--scope` は 6 ファイルだった。`--baseline-te ### 群ごとの検証（#880） - [ ] AC1: `refactor.py init` が `--round-test CMD` を受け付け、状態ファイルの `round_test.command` に保存する。渡さなければ `round_test` は `baseline_test` と同じコマンドになる（今の振る舞い）。このとき `init` のテストの実行は 1 回 -- [ ] AC2: `verify-round`（群の検証）と、修正ラウンド・最終ゲートの修正のコミットの検証が、`round_test` のコマンドを実行する。`baseline_test` のコマンドを実行しない +- [ ] AC2: `verify-round`（群の検証）と、修正ラウンドの修正のコミットの検証が、`round_test` のコマンドを実行する。`baseline_test` のコマンドを実行しない。最終ゲートの修正のコミットはテストを実行せず、直後の最終ゲートが 1 度だけ判定する（決定 14） - [ ] AC3: `--baseline-test` の実行は `init` の 1 回と最終ゲートの 1 回に限られる。最終ゲートは `--workflow-step` の有無に依らず、`round_test` が `baseline_test` と違えば手元で 1 回実行する（`--ci-check` があれば今のとおり継続的統合で代える） - [ ] AC4: `init` は `--round-test` を 1 回実行し、成功しなければ止まる（テストが 1 件も集まらない終了コード 5 も失敗）。`--scope` のテストの置き場所が `--round-test` の実行集合に入らなければ止まる（今の `scope_problem` を `round_test` へ当てる）。実行集合の起点は `--round-test` の引数のうち、オプションの値と作業ディレクトリの根を除いた、実在するディレクトリとテストの置き場所に当たる実在するファイルである（ラッパーのスクリプトは数えない。規則は設計の `round_test_roots`）。置き場所それぞれについて起点のどれかが同じか祖先でなければならない。起点が 1 つも無いコマンドは全体を覆うとみなす - [ ] AC5: `--round-test` を渡さず、`--baseline-test` が `--scope` のテストの置き場所より広いとき（探索の起点が無いときを含む）、`init` が 1 行の案内を出す。案内は `--round-test` に範囲のテストを渡せることを伝える。止まらない diff --git a/issues/issue-885-plan.md b/issues/old/milestone-26-cross-refactoring-fixes/issue-885-plan.md similarity index 100% rename from issues/issue-885-plan.md rename to issues/old/milestone-26-cross-refactoring-fixes/issue-885-plan.md diff --git a/plugins/ndf/skills/cross-refactoring/SKILL.md b/plugins/ndf/skills/cross-refactoring/SKILL.md index 6bf574fc..cac0c587 100644 --- a/plugins/ndf/skills/cross-refactoring/SKILL.md +++ b/plugins/ndf/skills/cross-refactoring/SKILL.md @@ -1,7 +1,7 @@ --- name: cross-refactoring description: "Let several CLIs propose, apply, and review refactorings on a PR until no new proposal appears. Use when structural improvement should converge across runtimes（クロスリファクタリング・多AIリファクタリング・収束リファクタリング）." -argument-hint: "[PR番号] --scope PATH... [--host claude|codex|agy|kiro] [--exclude NAMES] [--include NAMES] [--require-all] [--model RT=MODEL] [--baseline-test CMD] [--max-test-rounds N] [--max-outer-rounds N] [--max-fix-rounds N] [--max-items-per-round N] [--ci-check NAME] [--workflow-step]" +argument-hint: "[PR番号] --scope PATH... [--host claude|codex|agy|kiro] [--exclude NAMES] [--include NAMES] [--require-all] [--model RT=MODEL] [--baseline-test CMD] [--round-test CMD] [--max-test-rounds N] [--max-outer-rounds N] [--max-fix-rounds N] [--max-items-per-round N] [--ci-check NAME] [--workflow-step]" allowed-tools: - Bash - Read @@ -63,7 +63,7 @@ allowed-tools: | --- | --- | | 参加者 | **全員 CLI プロセス。** ホストのサブエージェント機能は使わない。ホストと同じランタイムが実装担当のラウンドでも別プロセスで起動する | | 参加者 | **提案と適用を同じ参加者で回す。** 既定は codex / kiro とホストで、`--exclude` / `--include` で名指しで変える。確認を通らない者は外して続ける | -| 検証の単位 | **適用ラウンド（群）に対して 1 回。** 判定は `--baseline-test` の合否で決まり、レビュー CLI は起動しない | +| 検証の単位 | **適用ラウンド（群）に対して 1 回。** 判定は `--round-test`（範囲のテスト）の合否で決まり、レビュー CLI は起動しない。全体のテスト（`--baseline-test`）は着手前と最終ゲートの 2 回だけ走る | | 収束しない項目 | **捨てる。** リファクタリングは任意の作業なので、揉める提案を Pull Request に残さない | | コミットの単位 | **1 適用ラウンド = 1 コミット。** テストも適用ラウンドの単位で 1 回だけ求める | | 改修計画 | **Pull Request のコメント 1 件へ残す。** 理由と手順は提案の時点でしか残らない。ラウンドが進むたびに同じコメントを編集する。URL は永続で、マージの後も開ける。`--plan-file` を明示したときだけファイルにする | @@ -91,7 +91,8 @@ allowed-tools: | `--include NAMES` | 参加者に足す者（例: `--include agy`）。再開で `none` を渡すと空へ戻す | なし | | `--require-all` | 確認を通らない者が 1 者でもいれば中断する（終了コード 4）。付けなければ外して続ける | 外して続ける | | `--model RT=MODEL` | ランタイムごとのモデル。繰り返し指定できる | CLI の既定 | -| `--baseline-test CMD` | 着手前と各コミットで実行するテスト。**振る舞い不変を示す手段が無い書き換えは構造改善ではないため必須** | 必須 | +| `--baseline-test CMD` | 着手前と最終ゲートで実行する全体のテスト。**振る舞い不変を示す手段が無い書き換えは構造改善ではないため必須** | 必須 | +| `--round-test CMD` | 着手前・群の検証・修正のコミットごとに実行する範囲のテスト。`--scope` のテストの置き場所を覆わなければ `init` が止まる。省くと `--baseline-test` が群ごとに走る | `--baseline-test` と同じ | | `--max-test-rounds N` | **テスト整備ラウンド**の上限。到達したら採用が残っていても提案ラウンドへ進む | `2` | | `--max-outer-rounds N` | **提案ラウンド**の上限。切るのは提案の回数であって、適用できる件数ではない | `3` | | `--max-fix-rounds N` | **1 つの適用ラウンドあたり**の修正ラウンドの上限 | `3` | @@ -104,7 +105,7 @@ allowed-tools: | `--plan-file PATH` | 改修計画を**ファイル**へ書き出す先（**対象リポジトリからの相対パス**）。空文字を渡すと記録しない | Pull Request のコメント 1 件 | ```text -/ndf:cross-refactoring 130 --scope src/services tests/services --baseline-test "pytest -q" +/ndf:cross-refactoring 130 --scope src/services tests/services --round-test "pytest tests/services -q" --baseline-test "pytest -q" /ndf:cross-refactoring 130 --scope src --baseline-test "pytest -q" --sync-command "make generate" /ndf:cross-refactoring 130 --scope src --model codex=gpt-5.5 --model claude=claude-opus-5 /ndf:cross-refactoring 130 --scope src --host codex --max-outer-rounds 1 @@ -175,6 +176,17 @@ allowed-tools: - 対象の Pull Request が Draft で開いている（未作成なら `/ndf:pr` で先に作る） +- 本番コードの差分がある。起動の前に `assess` で飛ばしてよいかを見る。**終了コード 3 なら + 起動しない**（本番コードの差分が無いか、変更が `--max-lines`（既定 10）行以下）。2 は + 判定できなかったことを示し、飛ばしてよいとは読まない。飛ばしたときの記録の残し方は + `development-workflow` の `references/workflow-modes.md`「構造改善の退避先」にある。 + +```bash +# 「実行」節の決め方で解決したこの Skill の scripts ディレクトリ +SCRIPTS="<この Skill のディレクトリ>/scripts" +python3 "$SCRIPTS/refactor.py" assess --base origin/develop; echo "exit=$?" +``` + ## 全体フロー **ラウンドは 4 層である。** テスト整備ラウンドと提案ラウンドは、集める提案の中身が @@ -293,7 +305,8 @@ rf_eval() { } rf_eval init "$PR" --scope $SCOPE \ - --baseline-test "$BASELINE" ${HOST:+--host "$HOST"} \ + --baseline-test "$BASELINE" ${ROUND_TEST:+--round-test "$ROUND_TEST"} \ + ${HOST:+--host "$HOST"} \ ${EXCLUDE:+--exclude "$EXCLUDE"} ${INCLUDE:+--include "$INCLUDE"} \ ${REQUIRE_ALL:+--require-all} \ --max-test-rounds "$MAX_TEST" --max-outer-rounds "$MAX_OUTER" \ diff --git a/plugins/ndf/skills/cross-refactoring/docs/01-state-and-propose.md b/plugins/ndf/skills/cross-refactoring/docs/01-state-and-propose.md index 68434085..6b61f636 100644 --- a/plugins/ndf/skills/cross-refactoring/docs/01-state-and-propose.md +++ b/plugins/ndf/skills/cross-refactoring/docs/01-state-and-propose.md @@ -58,10 +58,10 @@ export CROSS_REFACTORING_TMP_DIR="$TMP_DIR" 実行は初期化時ではなく、**push の直前**に進行側が行う。同期を実装担当の責務に すると範囲外の変更になり、範囲の検査で全件失敗する（実測 0/5） 9. **`--scope` の関門** — テストの置き場所が範囲に含まれること**と**、その置き場所が - `--baseline-test` の実行集合に入ることを確かめる。どちらかを満たさなければ + `--round-test`（省けば `--baseline-test`）の実行集合に入ることを確かめる。どちらかを満たさなければ **中断する**（終了コード 4）。**案内だけでは同じ失敗を繰り返す**（実測では 4 ラウンド続けて同じ理由で項目が落ちた）。止めれば利用者は 1 度だけ範囲を直せばよい -10. **着手前のテスト** — `--baseline-test` を実行する。**失敗していたら開始しない**。 +10. **着手前のテスト** — `--baseline-test` を実行し、`--round-test` を渡したときはそれも 1 回実行する。**どちらかが失敗していたら開始しない**。 壊れた状態から始めると、壊したのか元から壊れていたのか区別できない。 この引数は**必須**である。振る舞いが変わっていないことを示す手段が無い書き換えは、 `refactoring` Skill の定義からして構造改善ではない @@ -71,7 +71,7 @@ export CROSS_REFACTORING_TMP_DIR="$TMP_DIR" | 見るもの | 見ない場合に起きること | | --- | --- | | `--scope` にテストの置き場所が含まれているか | テスト整備ラウンドが足すテストが範囲外になり、その項目は必ず失敗する | -| その置き場所が `--baseline-test` の実行集合に入るか | 足したテストが一度も実行されず、検証（Step 5）の判定に効かない | +| その置き場所が `--round-test`（省けば `--baseline-test`）の実行集合に入るか | 足したテストが一度も実行されず、検証（Step 5）の判定に効かない | **2 つを 1 つの関門で見る。** 直す先はどちらも利用者が与える引数であり、別々に 止めると 2 度直すことになる。 diff --git a/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md b/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md index 81185715..77f07b88 100644 --- a/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md +++ b/plugins/ndf/skills/cross-refactoring/docs/02-apply-and-review.md @@ -284,7 +284,7 @@ fi 整備ラウンドで「テストを足せ」と「範囲外を触るな」が両立しなくなり、その項目は 必ず失敗する。**`init` がこれを関門として見て、含まれていなければ止める** （#436 決定 5。案内だけでは同じ失敗を繰り返し、実測では 4 ラウンド続けて同じ理由で -項目が落ちた）。**あわせて、その置き場所が `--baseline-test` の実行集合に入ることも +項目が落ちた）。**あわせて、その置き場所が `--round-test`（省けば `--baseline-test`）の実行集合に入ることも 同じ関門で見る**（入らないと、足したテストが検証で一度も実行されない）。 ```bash @@ -454,11 +454,11 @@ Impl-Model: gpt-5.5 ``` **判定はテストの結果で決まる**（#436 決定 3）。**2 CLI のレビューは起動しない。** -`--baseline-test` が指すコマンドを作業ディレクトリの HEAD で実行し、その合否で決める。 +`--round-test`（範囲のテスト。省けば `--baseline-test`）が指すコマンドを作業ディレクトリの HEAD で実行し、その合否で決める。 | 決めること | 結論 | | --- | --- | -| 何を実行するか | `--baseline-test` が指すコマンド。着手前と同じものを使う | +| 何を実行するか | `--round-test` が指すコマンド。省いたときは `--baseline-test`。全体のテストは着手前と最終ゲートの 2 回だけ走る | | 判定の単位 | **適用ラウンド（群）。** 失敗を項目までは特定しない | | 失敗したら | 修正ラウンドを回す。上限に達したら**その群だけ**取り消す | | 継続的統合で代替できるか | **代替しない**（後述） | diff --git a/plugins/ndf/skills/cross-refactoring/prompts/apply.md b/plugins/ndf/skills/cross-refactoring/prompts/apply.md index c3091132..dcba071c 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/apply.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/apply.md @@ -13,7 +13,7 @@ $RF_ROUND_NOTE - リポジトリ: $RF_REPO / Pull Request #$RF_PR - 作業ディレクトリ: `$RF_WORKDIR`（**ここでだけ作業する**） - ブランチ: `$RF_HEAD_BRANCH`（base は `$RF_BASE_BRANCH`） -- テストコマンド: `$RF_BASELINE_TEST` +- テストコマンド: `$RF_ROUND_TEST` ## 手順書 @@ -29,7 +29,7 @@ $RF_ITEMS 1. 配列の順に、各項目の `plan` の手順を適用する。`test_gap` が真の項目では、 その項目の現状固定テストも同じ作業の中で足す -2. **全項目を終えてから** `$RF_BASELINE_TEST` を **1 回**実行する。落ちたら原因の手を戻す +2. **全項目を終えてから** `$RF_ROUND_TEST` を **1 回**実行する。落ちたら原因の手を戻す 3. 通ったら、**この適用ラウンドの変更をまとめて 1 コミットにする** **テストもコミットも適用ラウンドの単位で 1 回です。** 手ごとや項目ごとに回すと、 diff --git a/plugins/ndf/skills/cross-refactoring/prompts/fix.md b/plugins/ndf/skills/cross-refactoring/prompts/fix.md index d0fdf562..167ee9b7 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/fix.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/fix.md @@ -9,7 +9,7 @@ - リポジトリ: $RF_REPO / Pull Request #$RF_PR - 作業ディレクトリ: `$RF_WORKDIR`（**ここでだけ作業する**） - ブランチ: `$RF_HEAD_BRANCH` -- テストコマンド: `$RF_BASELINE_TEST` +- テストコマンド: `$RF_ROUND_TEST` ## 手順書 @@ -25,11 +25,11 @@ $RF_ITEMS ## やること -1. `$RF_BASELINE_TEST` を実行し、**どのテストがなぜ落ちているか**を読む +1. `$RF_ROUND_TEST` を実行し、**どのテストがなぜ落ちているか**を読む 2. 落ちている原因が、上の項目の適用によるものかを確かめる 3. **振る舞いを元へ戻す方向で**直す。テストの側を書き換えて通すのではありません （**テストを緩めた時点で、振る舞い不変を示す手段が消えます**） -4. 直してから `$RF_BASELINE_TEST` をもう 1 回実行し、**項目ごとに 1 コミット**へまとめる +4. 直してから `$RF_ROUND_TEST` をもう 1 回実行し、**項目ごとに 1 コミット**へまとめる **振る舞いを変えないと直せない**と判断したときは、直さずに終えてください。 修正ラウンドの上限に達すると、**その適用ラウンドごと取り消されます**。無理に diff --git a/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md b/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md index 1a264189..aa8fb3cc 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md @@ -92,4 +92,7 @@ $RF_VOCAB_LEVELS - **構造を変える提案はしない。** それは次のラウンドで扱います - **実装詳細に結合したテストを提案しない**（内部メソッドの呼び出し回数、private な 関数の直接呼び出し、表示文字列の完全一致） +- **`.md` の文言・見出し・表の並びを固定するテストを提案しない。** `target` に `.md` の + ファイルを書いた提案は採られません。文書の振る舞いは文言そのものであり、固定すると + 後の文書の整理が振る舞いの変更として取り消されます - 既に固定されている経路を挙げない diff --git a/plugins/ndf/skills/cross-refactoring/scripts/launch-cli.sh b/plugins/ndf/skills/cross-refactoring/scripts/launch-cli.sh index cd357584..61efef94 100755 --- a/plugins/ndf/skills/cross-refactoring/scripts/launch-cli.sh +++ b/plugins/ndf/skills/cross-refactoring/scripts/launch-cli.sh @@ -41,6 +41,10 @@ load_common_state() { SCOPE=$(jq -r '.target_scope | join(" ")' "$STATE") MODEL=$(jq -r --arg rt "$RUNTIME" '.models[$rt] // ""' "$STATE") BASELINE_TEST=$(jq -r '.baseline_test.command // ""' "$STATE") + # 適用と修正の担当には、進行側が群と修正コミットを検証するコマンドを渡す（#880）。 + # `round_test_command` と同じく、`round_test` が無ければ `baseline_test` を採る。 + # 最終ゲートの修正（final-fix）は全体のテストで判定するため `BASELINE_TEST` を使う。 + ROUND_TEST=$(jq -r '.round_test.command // .baseline_test.command // ""' "$STATE") MAX_ITEMS=$(jq -r '.max_items_per_round' "$STATE") # ラウンド番号は表示と項目の絞り込みに使う。未指定なら開いている最新ラウンドを採る。 @@ -228,7 +232,7 @@ export_prompt_env() { export RF_REPO=$REPO RF_PR=$PR RF_ROUND=${ROUND:-} RF_RUNTIME=$RUNTIME export RF_MODEL=${MODEL:-default} RF_WORKDIR=$WORKDIR RF_STEM=$STEM export RF_SCOPE=$SCOPE RF_HEAD_BRANCH=$HEAD_BRANCH RF_BASE_BRANCH=$BASE_BRANCH -export RF_BASELINE_TEST=$BASELINE_TEST RF_MAX_ITEMS=$MAX_ITEMS +export RF_BASELINE_TEST=$BASELINE_TEST RF_ROUND_TEST=$ROUND_TEST RF_MAX_ITEMS=$MAX_ITEMS export RF_SKILL_BLOCK=$SKILL_BLOCK RF_EXCLUDED=$EXCLUDED RF_SKILL_BASE=$SKILL_BASE export RF_ITEMS=$ITEMS_JSON RF_TMP_DIR=$TMP_DIR export RF_APPLY_ROUND=$APPLY_ROUND diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor.py index 01df6cf5..b15d0091 100755 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor.py @@ -49,6 +49,7 @@ cmd_merge_proposals, cmd_next_apply_round, ) +from refactor_lib.commands.assess import DEFAULT_MAX_LINES, cmd_assess # noqa: E402 from refactor_lib.commands.converge import ( # noqa: E402 cmd_abandon_items, cmd_merge_fix, @@ -94,12 +95,8 @@ def _write_run_summary(path: pathlib.Path, state: dict) -> None: # ---------------- main ---------------- -def main() -> None: - p = argparse.ArgumentParser( - description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter - ) - sub = p.add_subparsers(dest="cmd", required=True) - +def add_init_parser(sub: argparse._SubParsersAction) -> None: + """`init` を登録する。""" init = sub.add_parser( "init", help="Step 0 — ホスト確定 / 参加者の確定 / 作業ディレクトリ root / 状態初期化・再開") @@ -163,8 +160,14 @@ def main() -> None: "ラウンドが進むたびに同じコメントを編集する。" "空文字を渡すと記録しない") init.add_argument("--baseline-test", required=True, - help="着手前と各コミットで実行するテストコマンド。" + help="着手前と最終ゲートで実行する全体のテスト。" "振る舞い不変を示す手段が無い書き換えは構造改善ではないため必須") + # **群と修正コミットの検証は範囲のテストで行う**（#880）。全体テストを群ごとに + # 走らせると、群と修正コミットの数だけ費用が積み上がる。 + init.add_argument("--round-test", default=None, metavar="CMD", + help="群の検証と修正コミットごとに実行する範囲のテスト。" + "--scope のテストの置き場所を走らせること。" + "省くと --baseline-test と同じ") # **起動のされ方は引数で受け取る**（#436 決定 7）。環境変数や控えの読み取りは、 # 起動元が違っても同じ値になりうる。呼ぶ側が明示すれば判定が 1 か所で済む。 init.add_argument("--workflow-step", action="store_true", default=None, @@ -174,6 +177,9 @@ def main() -> None: init.add_argument("--worktree-root", default=None) init.set_defaults(func=cmd_init) + +def add_id_commands(sub: argparse._SubParsersAction) -> None: + """提案ラウンドの番号 `id` だけを受け取る副コマンドを登録する。""" for name, func, help_ in ( ("start-round", cmd_start_round, "Step 2 — 提案ラウンドを開く。実装担当を返す"), @@ -191,6 +197,9 @@ def main() -> None: sp.add_argument("id", type=int) sp.set_defaults(func=func) + +def add_round_commands(sub: argparse._SubParsersAction) -> None: + """`id` と適用ラウンドの `round` を受け取る副コマンドを登録する。""" for name, func, help_ in ( ("next-apply-round", cmd_next_apply_round, "Step 4 — 次の適用ラウンド（群）を開く。実装担当と対象の項目を返す"), @@ -207,6 +216,9 @@ def main() -> None: sp.add_argument("round", type=int) sp.set_defaults(func=func) + +def add_dry_run_commands(sub: argparse._SubParsersAction) -> None: + """`id` / `round` に加えて `--dry-run` を受け取る副コマンドを登録する。""" # コミットを取り消しうる 2 つは、実行前に何が消えるかを確かめられるようにする。 for name, func, help_ in ( ("merge-apply", cmd_merge_apply, @@ -221,6 +233,23 @@ def main() -> None: help="取り消すコミットを表示するだけで実行しない") sp.set_defaults(func=func) + +def add_assess_parser(sub: argparse._SubParsersAction) -> None: + """`assess` を登録する。""" + ap = sub.add_parser( + "assess", + help="構造改善を飛ばしてよいかを差分から判定する。" + "終了コード 0 = 通す / 3 = 飛ばしてよい / 2 = 判定できない") + ap.add_argument("--base", required=True, + help="起点の ref。`...HEAD` の差分を見る") + ap.add_argument("--max-lines", type=int, default=DEFAULT_MAX_LINES, + help="本番コードの変更行（追加 + 削除）がこれ以下なら飛ばしてよい " + f"(default: {DEFAULT_MAX_LINES})") + ap.set_defaults(func=cmd_assess) + + +def add_report_parser(sub: argparse._SubParsersAction) -> None: + """`report` を登録する。""" rp = sub.add_parser( "report", help="Step 8 — ラウンド表・項目表・見送り・指標") rp.add_argument("id", type=int) @@ -228,6 +257,19 @@ def main() -> None: help="ランタイムとモデルの組で指標を集計する") rp.set_defaults(func=cmd_report) + +def main() -> None: + p = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + sub = p.add_subparsers(dest="cmd", required=True) + add_init_parser(sub) + add_id_commands(sub) + add_round_commands(sub) + add_dry_run_commands(sub) + add_assess_parser(sub) + add_report_parser(sub) + args = p.parse_args() args.func(args) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py index f18a57c4..a2e58232 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py @@ -30,6 +30,7 @@ safe_int, collect_commit_facts, commits_in_range, + tracked_markdown, ) from ..intake import ( IntakeScope, @@ -41,10 +42,10 @@ from ..proposals import assign_apply_rounds, merge_proposals, merge_test_proposals from ..rounds import ( TEST, + append_deferred_abandoned_items, apply_groups, attempt_of, current_group, - deferred_record, entry_kind, group_reopening, impl_for_seq, @@ -197,6 +198,14 @@ def _update_state_from_merged_proposals( # 提案は読むだけなので、この時点の HEAD が着手前の状態である。 entry["apply_base_sha"] = git_out(state["worktrees"]["work"], ["rev-parse", "HEAD"]) + _next_phase_after_merge(state, entry, adopted) + statefile.save(path, state) + + +def _next_phase_after_merge( + state: dict[str, Any], entry: dict[str, Any], adopted: list[dict[str, Any]] +) -> None: + """統合の結果から次の局面を決める。収束したときは終了理由も確定させる。""" if adopted: state["phase"] = "apply" elif entry_kind(entry) == TEST: @@ -209,7 +218,66 @@ def _update_state_from_merged_proposals( # 終了理由をここで確定させないと、報告が「未終了」のままになる。 state["final"] = "no_more_proposals" state["ended_at"] = statefile.now() - statefile.save(path, state) + + +def _replay_merged_proposals( + state: dict[str, Any], entry: dict[str, Any], kind: str +) -> bool: + """統合済みラウンドの結果を再表示し、処理済みかを返す。""" + if entry.get("proposal_keys") is None: + return False + info( + f"↻ ラウンド {entry['round']} は統合済みです" + f"（採用 {entry.get('adopted', 0)} 件 / 見送り {entry.get('deferred', 0)} 件）" + ) + for item_id in entry.get("items", []): + item = find_item(state, item_id, required=False) + if item is not None: + info(f" {item_id} {item_label(item)}") + if not entry.get("adopted") and kind != TEST: + sys.exit(2) + return True + + +def _merge_proposals_for_kind( + state: dict[str, Any], entry: dict[str, Any], kind: str, + proposals: dict[str, list[dict[str, Any]]], excluded: set[tuple[str, ...]], +) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]: + """ラウンドの種類に対応する方法で提案を統合する。""" + if kind == TEST: + return merge_test_proposals( + proposals, + max_items=state["max_items_per_round"], + excluded_keys=excluded, + ) + return merge_proposals( + proposals, + threshold=state["severity_threshold"], + max_items=state["max_items_per_round"], + excluded_keys=excluded, + ) + + +def _report_merged_proposals( + state: dict[str, Any], entry: dict[str, Any], kind: str, + adopted: list[dict[str, Any]], +) -> None: + """統合結果を表示し、採用 0 件なら種類に応じて終了を制御する。""" + info( + f"提案 {sum(entry['proposed'].values())} 件 → 統合 {entry['merged']} 件 → " + f"採用 {entry['adopted']} 件 / 見送り {entry['deferred']} 件" + ) + for item_id in entry["items"]: + info(f" {item_id} {_item_summary(find_item(state, item_id))}") + if adopted: + return + if kind == TEST: + # **終了ではない。** 足すべきテストが出なくなっただけで、この後に + # 構造改善の提案ラウンドが続く。切り替えは `advance` が行う。 + info("テスト整備の採用 0 件のため、構造改善の提案ラウンドへ進みます") + return + info("採用 0 件のため、提案ラウンドの繰り返しを終えます") + sys.exit(2) def cmd_merge_proposals(args: argparse.Namespace) -> None: @@ -224,51 +292,17 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: entry = current_round(state) kind = entry_kind(entry) - if entry.get("proposal_keys") is not None: - info( - f"↻ ラウンド {entry['round']} は統合済みです" - f"（採用 {entry.get('adopted', 0)} 件 / 見送り {entry.get('deferred', 0)} 件）" - ) - for item_id in entry.get("items", []): - item = find_item(state, item_id, required=False) - if item is not None: - info(f" {item_id} {item_label(item)}") - if not entry.get("adopted") and kind != TEST: - sys.exit(2) + if _replay_merged_proposals(state, entry, kind): return proposals = _load_runtime_proposals(state, entry) excluded = {item_key(d) for d in state["deferred_items"]} - if kind == TEST: - adopted, deferred = merge_test_proposals( - proposals, - max_items=state["max_items_per_round"], - excluded_keys=excluded, - ) - else: - adopted, deferred = merge_proposals( - proposals, - threshold=state["severity_threshold"], - max_items=state["max_items_per_round"], - excluded_keys=excluded, - ) + adopted, deferred = _merge_proposals_for_kind( + state, entry, kind, proposals, excluded) _update_state_from_merged_proposals(path, state, entry, adopted, deferred) - info( - f"提案 {sum(entry['proposed'].values())} 件 → 統合 {entry['merged']} 件 → " - f"採用 {entry['adopted']} 件 / 見送り {entry['deferred']} 件" - ) - for item_id in entry["items"]: - info(f" {item_id} {_item_summary(find_item(state, item_id))}") - if not adopted: - if kind == TEST: - # **終了ではない。** 足すべきテストが出なくなっただけで、この後に - # 構造改善の提案ラウンドが続く。切り替えは `advance` が行う。 - info("テスト整備の採用 0 件のため、構造改善の提案ラウンドへ進みます") - return - info("採用 0 件のため、提案ラウンドの繰り返しを終えます") - sys.exit(2) + _report_merged_proposals(state, entry, kind, adopted) def _item_summary(item: dict[str, Any]) -> str: @@ -884,6 +918,7 @@ def _collect_apply_group_facts( def _determine_apply_problem( ctx: _ApplyExecutionContext, + work: pathlib.Path, items: list[dict[str, Any]], missing: list[str], facts: list[dict[str, Any]], @@ -895,7 +930,11 @@ def _determine_apply_problem( "（群の全項目を 1 つのコミットへまとめ、各項目へ同じ SHA を申告します）" ) scope = ctx.state.get("target_scope") or [] - return verify_apply_round(items, facts, scope) + # **追跡している `.md` の一覧は群ごとに 1 回だけ読む**（#723）。 + return verify_apply_round( + items, facts, scope, + work=str(work), tracked_md=tracked_markdown(str(work)), + ) def _record_apply_group_outcome( @@ -942,7 +981,7 @@ def _verify_apply_group( """ items = [find_item(ctx.state, i) for i in ctx.group["items"]] missing, shas, facts = _collect_apply_group_facts(ctx, commit_range, reported) - problem = _determine_apply_problem(ctx, items, missing, facts) + problem = _determine_apply_problem(ctx, commit_range.work, items, missing, facts) _record_apply_group_outcome(ctx, items, shas, facts, problem) if problem: return [], list(ctx.group["items"]) @@ -962,39 +1001,26 @@ def _defer_abandoned_items(state: dict[str, Any], group: dict[str, Any]) -> None 除外の鍵は種類で変わる（改善項目は `path` + `symbol` + `smell`、テスト項目は `target` + `case`）。記録の形は `rounds.deferred_record` が持つ。 """ - already = {d.get("item_id") for d in state["deferred_items"]} + items = [] for item_id in group["items"]: item = find_item(state, item_id, required=False) - if item is None or item.get("status") != "abandoned" or item_id in already: - continue - state["deferred_items"].append(deferred_record( - item, item_id, - item.get("failure_reason") or "適用結果の検証を通らなかった", - )) + if item is not None and item.get("status") == "abandoned": + items.append(item) + append_deferred_abandoned_items( + state, items, "適用結果の検証を通らなかった") -def _apply_drop( +def _drop_and_settle_adoption( path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], group: dict[str, Any], failed: list[str], ) -> list[str]: - """検証に失敗した適用ラウンドを取り消し、採用として残る項目 ID を返す。 - - **中断しても再開できる形で記録する。** 失敗の位置で必要な再開が変わるため、 - 印は次の順で切り替える。 - - | 中断した位置 | 残る印 | 次の実行がすること | - | --- | --- | --- | - | 取り消しの途中 | `pending_drop` あり / `merged_at` なし | 取り消しをやり直す | - | 取り消し後・push 前 | `pending_drop` なし / `merged_at` あり / `pending_push` あり | **push の再送だけ** | + """取り消しを実行し、item・entry・group の採用状態を反映して採用 ID を返す。 - 取り消しより先に `merged_at` を立てると、取り消しに失敗したときに次の実行が - 処理済みガードで素通りし、**再試行できない**。逆に push まで終えるまで - `merged_at` を立てないと、push だけ失敗したときに次の実行が適用の検証をやり直し、 - 取り消しと積み直しのコミットを「未割当」と判定して群ごと巻き込む。 + `run_drop` が積み直せなかった（`mode=round`）ときは、この群の全件を捨てる。 + **他の群には及ばない。** """ - work = state["worktrees"]["work"] result = run_drop(path, state, entry, failed) applied = list(entry["apply"].get("applied") or []) if result["mode"] == "round": @@ -1009,6 +1035,14 @@ def _apply_drop( applied = [] entry["apply"]["applied"] = [] entry["apply"]["failed"] = list(group["items"]) + return applied + + +def _settle_drop_state( + state: dict[str, Any], entry: dict[str, Any], group: dict[str, Any], + work: pathlib.Path, applied: list[str], +) -> None: + """取り消し後の起点・群の状態・次 phase を確定する。""" if not applied: # 取り消し後の状態を新しい起点にする（叩き直しでの二重取り消しを防ぐ）。 entry["apply_base_sha"] = git_out(work, ["rev-parse", "HEAD"]) @@ -1018,18 +1052,53 @@ def _apply_drop( group["status"] = "applied" state["phase"] = phase_after_group(entry) + +def _persist_drop( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + group: dict[str, Any], +) -> None: + """deferred 記録・pending_drop 解除・merged_at 設定・push を永続化する。 + + **取り消しが済んだことを push より先に、印の解除と同じ保存で永続化する。** + 保存せずに push して失敗すると、次の実行が適用の検証をやり直し、取り消しと + 積み直しのコミットを「未割当」と判定して群ごと巻き込んでしまう。 + `pending_push` は残るので、次の実行は push の再送だけを行う。 + """ # 取り消した項目は「対象外」として残す。次のラウンドで同じ提案が採用され、 # 同じ理由で失敗するのを防ぐ。 _defer_abandoned_items(state, group) - # **取り消しが済んだことを push より先に、印の解除と同じ保存で永続化する。** - # 保存せずに push して失敗すると、次の実行が適用の検証をやり直し、取り消しと - # 積み直しのコミットを「未割当」と判定して群ごと巻き込んでしまう。 - # `pending_push` は残るので、次の実行は push の再送だけを行う。 entry["pending_drop"] = [] entry["apply"]["merged_at"] = statefile.now() # 印は `run_drop` が立ててある。ここは保存・push・印の解除を行う # （`push_with_retry_marker` が立て直しても値は変わらない）。 push_with_retry_marker(path, state, entry) + + +def _apply_drop( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + group: dict[str, Any], failed: list[str], +) -> list[str]: + """検証に失敗した適用ラウンドを取り消し、採用として残る項目 ID を返す。 + + **中断しても再開できる形で記録する。** 失敗の位置で必要な再開が変わるため、 + 印は次の順で切り替える。 + + | 中断した位置 | 残る印 | 次の実行がすること | + | --- | --- | --- | + | 取り消しの途中 | `pending_drop` あり / `merged_at` なし | 取り消しをやり直す | + | 取り消し後・push 前 | `pending_drop` なし / `merged_at` あり / `pending_push` あり | **push の再送だけ** | + + 取り消しより先に `merged_at` を立てると、取り消しに失敗したときに次の実行が + 処理済みガードで素通りし、**再試行できない**。逆に push まで終えるまで + `merged_at` を立てないと、push だけ失敗したときに次の実行が適用の検証をやり直し、 + 取り消しと積み直しのコミットを「未割当」と判定して群ごと巻き込む。 + + 取り消し、状態の反映、永続化の 3 段に分ける。 + """ + work = state["worktrees"]["work"] + applied = _drop_and_settle_adoption(path, state, entry, group, failed) + _settle_drop_state(state, entry, group, work, applied) + _persist_drop(path, state, entry, group) return applied @@ -1153,4 +1222,3 @@ def cmd_merge_test_judgements(args: argparse.Namespace) -> None: path, state, entry, current_group(entry), outcome["problem"]) _apply_group_judgements(path, state, entry, group_of_round, verdicts) - diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/assess.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/assess.py new file mode 100644 index 00000000..fa307116 --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/assess.py @@ -0,0 +1,48 @@ +"""構造改善を飛ばしてよいかを、差分から判定する（#494）。 + +`assess` を持つ。状態ファイルは読まない（`init` より前に呼ぶため）。 +""" +from __future__ import annotations + +import argparse +import os +import sys + +from ..gitfacts import production_code_changes + +DEFAULT_MAX_LINES = 10 + +PASS = 0 +SKIP = 3 +UNDECIDABLE = 2 + + +def cmd_assess(args: argparse.Namespace) -> None: + """`...HEAD` の本番コードの差分から、構造改善を通すか飛ばしてよいかを出す。 + + 終了コード: 0 = 通す / 3 = 飛ばしてよい / 2 = `` を解けない。 + **2 を飛ばしてよいと読まない。** 判定できないことは飛ばす理由にならない。 + + 退避の 3 条件（テストが無い・CLI が使えない・範囲を絞れない）は見ない。それらは + `init` が止めて知らせる。 + """ + changes = production_code_changes(os.getcwd(), args.base) + if changes is None: + print(f"ERROR: 起点 {args.base} から HEAD までの差分を取れません", + file=sys.stderr) + sys.exit(UNDECIDABLE) + total = sum(n for _, n in changes) + if not changes: + verdict, reason = SKIP, "本番コードの差分がありません" + elif total <= args.max_lines: + verdict = SKIP + reason = f"本番コードの変更が {total} 行で、上限 {args.max_lines} 行以下です" + else: + verdict, reason = PASS, f"本番コードの変更が {total} 行です" + counted = f"本番コード: {len(changes)} ファイル・{total} 行" + if changes: + counted += f"（{'、'.join(p for p, _ in changes)}）" + print("判定: " + ("通す" if verdict == PASS else "飛ばしてよい")) + print("理由: " + reason) + print(counted) + sys.exit(verdict) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py index ecbeff79..87e3b0f3 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/converge.py @@ -11,6 +11,8 @@ import hashlib import pathlib import sys +import time +from dataclasses import dataclass from typing import Any, Optional import statefile @@ -41,11 +43,12 @@ from ..outbound import dropped_line, item_lines, plan_line from ..paths import git_out, load_state, result_path, stem_for from ..rounds import ( + append_deferred_abandoned_items, current_group, - deferred_record, phase_after_group, prepare_fix_phase, ) +from ..scope import round_test_command from ..verify import ( unassigned_fix_commits, verify_commit_granularity, @@ -61,11 +64,15 @@ def _verification_record( code: int, timed_out: bool, passed: bool, + seconds: float, ) -> dict[str, Any]: """検証 1 回分の記録を作る。**状態は変えない。** 判定を作る段と、判定を状態へ反映する段を分ける。合否そのものは呼び出し側が 決めており、ここは何を記録に残すかだけを持つ。 + + **所要の秒数を残す**（#880）。範囲のテストへ寄せた効果は、状態ファイルから + 測れなければ確かめられない。 """ return { "apply_round": group["apply_round"], @@ -75,6 +82,7 @@ def _verification_record( "status": "pass" if passed else "fail", "exit_code": code, "timed_out": timed_out, + "seconds": seconds, } @@ -128,8 +136,9 @@ def cmd_verify_round(args: argparse.Namespace) -> None: 終了コード: 0 = テストが通った / 2 = 落ちた（修正ラウンドへ）。 - **2 CLI のレビューは起動しない**（決定 3）。`--baseline-test` が指す - コマンドを作業ディレクトリの HEAD で実行し、その合否で決める。 + **2 CLI のレビューは起動しない**（決定 3）。`--round-test` が指す範囲の + テストを作業ディレクトリの HEAD で実行し、その合否で決める（#880）。全体の + テストは着手前と最終ゲートにしか走らせない。 **失敗をどの項目に紐づけるかは決めない。** 適用ラウンドの中は 1 コミットで あり、分離しても取り消せない。判定の単位と取り消しの単位を一致させる。 @@ -148,14 +157,16 @@ def cmd_verify_round(args: argparse.Namespace) -> None: code=2, ) - command = (state.get("baseline_test") or {}).get("command") or "" + command = round_test_command(state) work = str(state["worktrees"]["work"]) timeout = safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT) + started = time.monotonic() code, timed_out = run_with_timeout(command, work, timeout) + seconds = round(time.monotonic() - started, 1) passed = (not timed_out) and code == 0 entry.setdefault("verifications", []).append( - _verification_record(group, entry, command, code, timed_out, passed) + _verification_record(group, entry, command, code, timed_out, passed, seconds) ) if passed: @@ -187,16 +198,68 @@ def _record_deferred_abandoned_items( state: dict[str, Any], targets: list[str] ) -> None: """取り消し対象項目の status を abandoned に更新し、未登録なら deferred_items に追記する。""" - already = {d.get("item_id") for d in state["deferred_items"]} + items = [] for item_id in targets: item = find_item(state, item_id) item["status"] = "abandoned" item.setdefault( "failure_reason", "修正ラウンドの上限に達してもテストが通らなかった") - if item_id in already: - continue - state["deferred_items"].append( - deferred_record(item, item_id, item["failure_reason"])) + items.append(item) + append_deferred_abandoned_items( + state, items, "修正ラウンドの上限に達してもテストが通らなかった") + + +def _resume_abandon_leftovers( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any] +) -> None: + """やり残した取り消しを push の再送より先に片づける。 + + 先に push すると、取り消しが途中の HEAD をそのまま Pull Request へ反映して + しまう。取り消しが残っていれば再実行し、無ければ保留の push だけ流す。 + """ + if entry.get("pending_drop"): + info("↻ 前回終わらなかった取り消しを再実行します") + run_drop(path, state, entry, list(entry["pending_drop"])) + else: + flush_pending_push(path, state, entry) + + +def _abandon_no_targets( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + group: dict[str, Any], dry_run: bool, +) -> None: + """取り消す項目が無い群を処理済みとして記録する。""" + info("取り消す項目はありません") + if not dry_run: + group["abandoned"] = [] + entry["abandoned"] = [] + statefile.save(path, state) + + +def _complete_abandon( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + group: dict[str, Any], targets: list[str], +) -> None: + """取り消しを実行し、見送りの記録・状態の更新・push を行う。 + + 見送りの記録と印の解除を**同じ保存で**行う。保存してから push するので、 + push が失敗しても記録とローカルの git が食い違わない。 + """ + run_drop(path, state, entry, targets) + + _record_deferred_abandoned_items(state, targets) + + # **内訳は書かない。件数だけ述べ、内訳は改修計画へ譲る**（#436 決定 6-b）。 + info(f"↩ 適用ラウンド {group['apply_round']}: {dropped_line(state, len(targets))}") + group["abandoned"] = targets + group["status"] = "dropped" + entry["abandoned"] = targets + entry["pending_drop"] = [] + entry["apply_base_sha"] = git_out( + state["worktrees"]["work"], ["rev-parse", "HEAD"]) + group["base_sha"] = entry["apply_base_sha"] + state["phase"] = phase_after_group(entry) + push_with_retry_marker(path, state, entry) def cmd_abandon_items(args: argparse.Namespace) -> None: @@ -210,13 +273,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: entry = round_of(state, args.round) group = current_group(entry) if not args.dry_run: - # **やり残した取り消しを push の再送より先に片づける。** 先に push すると、 - # 取り消しが途中の HEAD をそのまま Pull Request へ反映してしまう。 - if entry.get("pending_drop"): - info("↻ 前回終わらなかった取り消しを再実行します") - run_drop(path, state, entry, list(entry["pending_drop"])) - else: - flush_pending_push(path, state, entry) + _resume_abandon_leftovers(path, state, entry) # 取り消し自体は `reverted` で冪等だが、見送りの記録は重複しうる。 if group.get("abandoned") is not None: @@ -226,11 +283,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: targets = list((entry.get("apply") or {}).get("applied") or []) if not targets: - info("取り消す項目はありません") - if not args.dry_run: - group["abandoned"] = [] - entry["abandoned"] = [] - statefile.save(path, state) + _abandon_no_targets(path, state, entry, group, args.dry_run) return if args.dry_run: @@ -238,23 +291,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: info("（dry-run）状態ファイルは更新していません") return - run_drop(path, state, entry, targets) - - _record_deferred_abandoned_items(state, targets) - - # 見送りの記録と印の解除を**同じ保存で**行う。保存してから push するので、 - # push が失敗しても記録とローカルの git が食い違わない。 - # **内訳は書かない。件数だけ述べ、内訳は改修計画へ譲る**（#436 決定 6-b）。 - info(f"↩ 適用ラウンド {group['apply_round']}: {dropped_line(state, len(targets))}") - group["abandoned"] = targets - group["status"] = "dropped" - entry["abandoned"] = targets - entry["pending_drop"] = [] - entry["apply_base_sha"] = git_out( - state["worktrees"]["work"], ["rev-parse", "HEAD"]) - group["base_sha"] = entry["apply_base_sha"] - state["phase"] = phase_after_group(entry) - push_with_retry_marker(path, state, entry) + _complete_abandon(path, state, entry, group, targets) def _fix_merge_key(entry: dict[str, Any], result: pathlib.Path) -> str: @@ -396,26 +433,32 @@ def _revert_invalid_fix_round( return set() -def _resolve_fix_range( - path: pathlib.Path, - state: dict[str, Any], - entry: dict[str, Any], - work: str, - head_now: str, -) -> list[str]: +@dataclass +class FixExecutionContext: + """修正の取り込み 1 回分に属する値の組。""" + + path: pathlib.Path + state: dict[str, Any] + entry: dict[str, Any] + scope: IntakeScope + work: str + + +def _resolve_fix_range(ctx: FixExecutionContext, head_now: str) -> list[str]: """修正の範囲を**オーケストレータが記録した起点**から確定して返す。 起点は `verify-round` がテストの失敗を返したときの HEAD である。確定できない ときは修正ラウンドを 1 つ進めて保存したうえで `die` する。 """ - ordered_range = commits_in_range(work, entry.get("fix_base_sha"), head_now) + entry = ctx.entry + ordered_range = commits_in_range(ctx.work, entry.get("fix_base_sha"), head_now) if ordered_range is None: # **修正ラウンドは進める。** 進めないと `should-abandon` が見送りへ移る # 条件（`fix_rounds` が上限に達する）を永久に満たさず、修正フェーズと # 再レビューを無限に往復する。この修正は採らないので、範囲外の記録は # 何も足さない。 entry["fix_rounds"] += 1 - statefile.save(path, state) + statefile.save(ctx.path, ctx.state) die( "修正の範囲を確定できませんでした" f"（起点 {entry.get('fix_base_sha')} / HEAD {head_now}）。" @@ -429,20 +472,21 @@ def _inspect_fix_commits( state: dict[str, Any], work: str, payload: dict[str, Any], - baseline: dict[str, Any], ordered_range: list[str], ) -> tuple[list[str], list[str], list[tuple[str, str]]]: """修正コミットを **git と実際のテスト実行から**検証する。 結果ファイルの申告で済ませると、手順を満たさない変更が収束済みになれてしまう。 未割当コミットの一覧・問題点の一覧・受理した (item_id, sha) を返す。 + + **コミットごとに走らせるのは範囲のテストである**（#880）。 """ claimed_shas = reported_shas(payload) unassigned = unassigned_fix_commits(work, claimed_shas, ordered_range) facts = collect_commit_facts( work, claimed_shas, set(ordered_range), - baseline.get("command") or "true", state["head_branch"], + round_test_command(state) or "true", state["head_branch"], safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), ) @@ -457,10 +501,7 @@ def _inspect_fix_commits( def _settle_fix_round( - path: pathlib.Path, - state: dict[str, Any], - entry: dict[str, Any], - scope: IntakeScope, + ctx: FixExecutionContext, ordered_range: list[str], resolved: set[str], unassigned: list[str], @@ -469,11 +510,13 @@ def _settle_fix_round( ) -> None: """検証結果に応じて修正ラウンドを取り消すか受理し、解決の印を付ける。""" if unassigned or problems: - resolved = _revert_invalid_fix_round(path, state, scope, ordered_range) + resolved = _revert_invalid_fix_round( + ctx.path, ctx.state, ctx.scope, ordered_range + ) else: - _record_accepted_fix_commits(state, accepted) + _record_accepted_fix_commits(ctx.state, accepted) - _mark_resolved_fix_findings(entry, resolved) + _mark_resolved_fix_findings(ctx.entry, resolved) def _fix_scope(entry: dict[str, Any], impl: str) -> IntakeScope: @@ -526,17 +569,17 @@ def _close_failed_fix( def _fetch_fix_result( - path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], - scope: IntakeScope, impl: str, round_no: int, + ctx: FixExecutionContext, impl: str, round_no: int, ) -> tuple[Optional[dict[str, Any]], Optional[str]]: """修正結果を取得し、`(payload, merge_key)` を返す。 結果を残さなかった試行は `_close_failed_fix` が終了させる。取り込み済みの 結果なら `(None, None)` を返し、呼び出し側が何もせず戻れるようにする。 """ + state, entry = ctx.state, ctx.entry outcome = read_result(state, impl, "fix", round_no) if outcome.payload is None: - _close_failed_fix(path, state, entry, scope, outcome) + _close_failed_fix(ctx.path, state, entry, ctx.scope, outcome) payload = outcome.payload result = result_path(state, impl, stem_for(impl, "fix", state["id"], round_no)) @@ -547,31 +590,30 @@ def _fetch_fix_result( def _confirm_and_settle_fix( - path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], - scope: IntakeScope, work: str, head_now: str, payload: dict[str, Any], + ctx: FixExecutionContext, head_now: str, payload: dict[str, Any], ) -> set[str]: """Git 範囲を確定し、修正コミットを検証して取り消すか受理する。 採用した解決スレッドの集合を返す（取り込みの通知に使う）。 """ + state = ctx.state resolved = _resolved_fix_thread_ids(payload, state["repo"], state["current_pr"]) - baseline = state.get("baseline_test") or {} - ordered_range = _resolve_fix_range(path, state, entry, work, head_now) + ordered_range = _resolve_fix_range(ctx, head_now) unassigned, problems, accepted = _inspect_fix_commits( - state, work, payload, baseline, ordered_range + state, ctx.work, payload, ordered_range ) _settle_fix_round( - path, state, entry, scope, ordered_range, resolved, unassigned, problems, - accepted, + ctx, ordered_range, resolved, unassigned, problems, accepted, ) return resolved def _record_and_publish_fix( - path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + ctx: FixExecutionContext, merge_key: str, payload: dict[str, Any], resolved: set[str], ) -> None: """取り込み済みの鍵・修正回数・所要時間を記録し、保存して公開する。""" + path, state, entry = ctx.path, ctx.state, ctx.entry entry["fix_merged_keys"].append(merge_key) entry["fix_rounds"] += 1 entry.setdefault("durations", {})["fix"] = ( @@ -607,15 +649,13 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: info("↻ この修正の試行は結果なしとして記録済みです") sys.exit(2) - payload, merge_key = _fetch_fix_result( - path, state, entry, scope, impl, args.round + ctx = FixExecutionContext( + path, state, entry, scope, state["worktrees"]["work"] ) + payload, merge_key = _fetch_fix_result(ctx, impl, args.round) if payload is None: return - work = state["worktrees"]["work"] - head_now = git_out(work, ["rev-parse", "HEAD"]) or "" - resolved = _confirm_and_settle_fix( - path, state, entry, scope, work, head_now, payload - ) - _record_and_publish_fix(path, state, entry, merge_key, payload, resolved) + head_now = git_out(ctx.work, ["rev-parse", "HEAD"]) or "" + resolved = _confirm_and_settle_fix(ctx, head_now, payload) + _record_and_publish_fix(ctx, merge_key, payload, resolved) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py index 85f2dd20..af410917 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/gate.py @@ -17,6 +17,7 @@ import argparse import pathlib import sys +import time from typing import Any, Optional import statefile @@ -42,6 +43,7 @@ ) from ..paths import git_out, load_state from ..rounds import impl_for_seq +from ..scope import round_test_command from ..verify import verify_final_fix_commit from ..vocabulary import DEFAULT_TEST_TIMEOUT from ..verify import unassigned_fix_commits @@ -55,50 +57,109 @@ def cmd_final_gate(args: argparse.Namespace) -> None: **Step 7 は push 済みの地点である。** 上限に達しても取り消さない。取り消しの 判断は Pull Request の読み手が持つため、失敗として報告に書く。 + + **単独起動でも、群を範囲のテストで検証してきたなら全体のテストを 1 回通す** + （#880）。範囲の外への波及を見る機会が、ほかに無いためである。通れば今のとおり + `cross-review` へ渡し、落ちれば工程として起動したときと同じ修正ラウンドへ入る。 """ path, state = load_state(args.id) gate = state.setdefault("final_gate", {"fix_rounds": 0, "checks": []}) + standalone = not state.get("workflow_step") - if not state.get("workflow_step"): - gate["mode"] = "cross-review" - statefile.save(path, state) - info("単独起動のため、Step 7 は /ndf:cross-review を実行します") - statefile.emit(FINAL_GATE="cross-review") + if standalone and not _round_test_differs(state): + _emit_cross_review( + path, state, gate, "単独起動のため、Step 7 は /ndf:cross-review を実行します" + ) + return + + passed, detail = _run_and_record_gate_check(state, gate) + + if passed and standalone: + _emit_cross_review( + path, state, gate, + f"✅ 全体のテストが通りました（{detail}）。Step 7 は /ndf:cross-review を実行します", + ) + return + if passed: + _gate_passed(path, state, gate, detail) + return + + limit = safe_int(state.get("max_fix_rounds"), 3) + if safe_int(gate.get("fix_rounds")) >= limit: + _gate_limit_reached(path, state, gate, detail, limit) return + _gate_failing(path, state, gate, detail, limit) + +def _run_and_record_gate_check( + state: dict[str, Any], gate: dict[str, Any] +) -> tuple[bool, str]: + """最終ゲートの検査を 1 回走らせ、`checks` へ記録して結果を返す。""" # **排他である。** `--ci-check` があれば手元のテストを実行せず継続的統合の成功 # だけで判定し、無ければ手元のテストだけで判定する。「どちらか一方が通れば通過」 # とはしない（OR で採ると、手元のテストの失敗を継続的統合の成功が覆す）。 ci_check = str(state.get("ci_check") or "").strip() gate["mode"] = "ci" if ci_check else "test" + started = time.monotonic() passed, detail = ( _ci_gate(state, ci_check) if ci_check else _local_gate(state) ) + _record_gate_check( + gate, ci_check or _baseline_command(state), passed, detail, + round(time.monotonic() - started, 1), + ) + return passed, detail + + +def _emit_cross_review( + path: pathlib.Path, state: dict[str, Any], gate: dict[str, Any], message: str +) -> None: + """Step 7 を `cross-review` へ委譲する結末。""" + gate["mode"] = "cross-review" + statefile.save(path, state) + info(message) + statefile.emit(FINAL_GATE="cross-review") + + +def _record_gate_check( + gate: dict[str, Any], command: str, passed: bool, detail: str, seconds: float +) -> None: + """最終ゲートの検査 1 件を `checks` へ追記する。""" gate.setdefault("checks", []).append({ "at": statefile.now(), "mode": gate["mode"], + "command": command, "status": "pass" if passed else "fail", "detail": detail, + "seconds": seconds, }) - if passed: - gate["status"] = "passed" - statefile.save(path, state) - info(f"✅ 最終ゲートを通過しました（{detail}）") - statefile.emit(FINAL_GATE="passed") - return - limit = safe_int(state.get("max_fix_rounds"), 3) - if safe_int(gate.get("fix_rounds")) >= limit: - gate["status"] = "failed" - statefile.save(path, state) - info( - f"❌ 最終ゲートが通らないまま修正の上限 {limit} に達しました（{detail}）。" - "**既に push してあるため取り消しません。** 失敗として報告します" - ) - statefile.emit(FINAL_GATE="failed") - sys.exit(1) +def _gate_passed( + path: pathlib.Path, state: dict[str, Any], gate: dict[str, Any], detail: str +) -> None: + gate["status"] = "passed" + statefile.save(path, state) + info(f"✅ 最終ゲートを通過しました（{detail}）") + statefile.emit(FINAL_GATE="passed") + +def _gate_limit_reached( + path: pathlib.Path, state: dict[str, Any], gate: dict[str, Any], detail: str, limit: int +) -> None: + gate["status"] = "failed" + statefile.save(path, state) + info( + f"❌ 最終ゲートが通らないまま修正の上限 {limit} に達しました（{detail}）。" + "**既に push してあるため取り消しません。** 失敗として報告します" + ) + statefile.emit(FINAL_GATE="failed") + sys.exit(1) + + +def _gate_failing( + path: pathlib.Path, state: dict[str, Any], gate: dict[str, Any], detail: str, limit: int +) -> None: gate["fix_rounds"] = safe_int(gate.get("fix_rounds")) + 1 gate["status"] = "failing" # **修正の起点と担当をここで記録する。** 記録しないと `merge-final-fix` が範囲を @@ -119,6 +180,15 @@ def cmd_final_gate(args: argparse.Namespace) -> None: sys.exit(2) +def _baseline_command(state: dict[str, Any]) -> str: + return str((state.get("baseline_test") or {}).get("command") or "") + + +def _round_test_differs(state: dict[str, Any]) -> bool: + """群の検証が全体のテストと違うコマンドで行われたか（#880）。""" + return round_test_command(state) != _baseline_command(state) + + def _final_fix_impl(state: dict[str, Any], gate: dict[str, Any]) -> str: """最終ゲートの修正担当を決める。**最初に落ちたときだけ輪番を 1 つ進める。** @@ -326,8 +396,8 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: def _local_gate(state: dict[str, Any]) -> tuple[bool, str]: - """全体のテストを手元で実行する。""" - command = (state.get("baseline_test") or {}).get("command") or "" + """全体のテストを手元で実行する。**全体のテストを呼ぶのは `init` とここだけである。**""" + command = _baseline_command(state) work = str(state["worktrees"]["work"]) timeout = safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT) code, timed_out = run_with_timeout(command, work, timeout) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py index c6dd584c..de4c6528 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py @@ -20,7 +20,7 @@ from ..outbound import plan_reference from ..paths import load_state from ..proposals import duplicate_rate -from ..rounds import finish_outer_rounds, STRUCTURE, TEST, entry_kind, item_kind, item_label +from ..rounds import finish_outer_rounds, STRUCTURE, TEST, entry_kind, item_kind, item_label, rounds_of_kind from ..vocabulary import DEFAULT_MAX_TEST_ROUNDS, DUPLICATE_RATE_THRESHOLD @@ -47,13 +47,13 @@ def cmd_advance(args: argparse.Namespace) -> None: if entry_kind(last) == TEST: _advance_test_rounds(path, state, last) return - if len(_of_kind(rounds, STRUCTURE)) >= state["max_outer_rounds"]: + if len(rounds_of_kind(rounds, STRUCTURE)) >= state["max_outer_rounds"]: finish_outer_rounds(path, state, "max_outer_rounds") sys.exit(1) if last.get("adopted") == 0: finish_outer_rounds(path, state, "no_more_proposals") sys.exit(1) - previous = _of_kind(rounds[:-1], STRUCTURE) + previous = rounds_of_kind(rounds[:-1], STRUCTURE) if previous: # **同じ種類どうしで測る。** 鍵の形が種類で違うため、テスト整備ラウンドを # 相手にすると重なりが常に 0 になり、収束の判定が働かない。 @@ -67,11 +67,6 @@ def cmd_advance(args: argparse.Namespace) -> None: sys.exit(1) -def _of_kind(rounds: list[dict[str, Any]], kind: str) -> list[dict[str, Any]]: - """その種類のラウンドだけを取り出す。上限はそれぞれ別に数える。""" - return [r for r in rounds if entry_kind(r) == kind] - - def _advance_test_rounds( path: pathlib.Path, state: dict[str, Any], last: dict[str, Any] ) -> None: @@ -81,7 +76,7 @@ def _advance_test_rounds( 残っていても移る。**どちらで移ったかを記録する**（収束して終わったのか、 歯止めで止まったのかを報告で読み分けるため）。 """ - done = len(_of_kind(state["rounds"], TEST)) + done = len(rounds_of_kind(state["rounds"], TEST)) limit = safe_int(state.get("max_test_rounds"), DEFAULT_MAX_TEST_ROUNDS) if last.get("adopted") == 0: reason = "no_more_test_proposals" diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py index 6857f63f..7e3da9f4 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py @@ -38,8 +38,9 @@ finish_outer_rounds, impl_for_seq, round_kind, + rounds_of_kind, ) -from ..scope import require_scope_covers_tests +from ..scope import require_scope_covers_tests, round_test_hint from ..vocabulary import ( DEFAULT_MAX_TEST_ROUNDS, DEFAULT_SEVERITY_THRESHOLD, @@ -80,6 +81,7 @@ statefile.ResumeField("scope", "target_scope", "notify"), statefile.ResumeField("model", "models", "notify"), statefile.ResumeField("baseline_test", "baseline_test", "notify"), + statefile.ResumeField("round_test", "round_test", "notify"), statefile.ResumeField("ci_check", "ci_check", "notify"), statefile.ResumeField("severity_threshold", "severity_threshold", "notify"), statefile.ResumeField("sync_command", "sync_command", "notify"), @@ -278,6 +280,7 @@ class InitialContext: participants: dict[str, Any] model_spec: dict[str, Optional[str]] baseline: dict[str, Any] + round_test: dict[str, Any] def _build_initial_state( @@ -333,6 +336,8 @@ def _build_initial_state( "round_kind": TEST, "severity_threshold": args.severity_threshold, "baseline_test": ctx.baseline, + # **群と修正コミットの検証が実行するテスト**（#880）。省けば全体テストと同じ。 + "round_test": ctx.round_test, # 生成物の同期は**進行側の責務**。push の直前に実行する。 "sync_command": args.sync_command, # **改修計画の既定は Pull Request のコメント 1 件である**（#436 決定 6）。 @@ -358,19 +363,73 @@ def cmd_init(args: argparse.Namespace) -> None: codex / kiro とホストを既定とし、足す者・外す者で変える。確認を通らない者は外して 続ける。前回の状態が残っていれば再開し、渡した引数を反映の表に従って扱う。 """ + inputs = _resolve_init_inputs(args) + if inputs is None: + return + prep = _prepare_init(args) + if _resume_if_pending(args, inputs, prep): + return + + for key, value in NEW_RUN_DEFAULTS.items(): + if getattr(args, key, None) is None: + setattr(args, key, value) + + participants, baseline, round_record = _verify_init(args, inputs, prep) + state = _save_initial_state(args, inputs, prep, participants, baseline, round_record) + # **出力は入口から直接呼ぶ。** 手順書の変数の出所の検査 + # （`scripts/check-skill-shell-vars.py`）は `cmd_*` からヘルパーを 1 段だけたどる。 + _emit_init(state) + + +@dataclass +class _InitInputs: + """`init` の引数から解決したホスト・モデル・足す者・外す者。""" + + host: str + detection: str + model_spec: dict[str, Optional[str]] + include: Optional[list[str]] + exclude: Optional[list[str]] + + +@dataclass +class _InitPreparation: + """Pull Request の文脈と、用意した作業ディレクトリ。""" + + repo: str + base_branch: str + head_branch: str + is_own_pr: bool + root: pathlib.Path + work: pathlib.Path + tmp_dir: pathlib.Path + state_file: pathlib.Path + round_test: Optional[str] + + +def _resolve_init_inputs(args: argparse.Namespace) -> Optional[_InitInputs]: + """ホスト・モデル・足す者・外す者を解決する。解決できなければ止めて `None` を返す。""" try: host, detection = assignment.detect_host(args.host) except assignment.AssignmentError as e: die(str(e)) - return + return None try: model_spec = models_lib.parse_model_args(args.model) except models_lib.ModelSpecError as e: die(str(e)) - return - include = _names_arg(args, "include") - exclude = _names_arg(args, "exclude") + return None + return _InitInputs( + host=host, + detection=detection, + model_spec=model_spec, + include=_names_arg(args, "include"), + exclude=_names_arg(args, "exclude"), + ) + +def _prepare_init(args: argparse.Namespace) -> _InitPreparation: + """Pull Request の文脈を取り、作業ディレクトリを用意して `--scope` の関門を通す。""" # リポジトリ名は git の設定から求め、Pull Request の応答で確かめる（#271）。 repo, base_branch, head_branch, is_own_pr, author = _fetch_pr_context(args.pr) if is_own_pr: @@ -388,53 +447,96 @@ def cmd_init(args: argparse.Namespace) -> None: # ラウンドが足したテストが検証に効かない。案内だけでは同じ失敗を繰り返す # ため、**止める**。作業ディレクトリが要るのは、探索範囲の語がディレクトリか # どうかを実物で確かめるためである。 - require_scope_covers_tests(args.scope, args.baseline_test, str(work)) + # **足したテストが入るべき実行集合は `--round-test` である**（#880）。群の検証が + # 走らせるのはこちらで、全体テストは着手前と最終ゲートにしか走らない。 + round_test = getattr(args, "round_test", None) + if round_test: + require_scope_covers_tests(args.scope, round_test, str(work), round_test=True) + else: + require_scope_covers_tests(args.scope, args.baseline_test, str(work)) tmp_dir = tmp_dir_for(work) tmp_dir.mkdir(parents=True, exist_ok=True) - state_file = state_path(tmp_dir, args.pr) - - if state_file.exists(): - state = statefile.load(state_file) - if state.get("final") is None: - _resume(state_file, state, args, model_spec, include, exclude, is_own_pr) - return + return _InitPreparation( + repo=repo, + base_branch=base_branch, + head_branch=head_branch, + is_own_pr=is_own_pr, + root=root, + work=work, + tmp_dir=tmp_dir, + state_file=state_path(tmp_dir, args.pr), + round_test=round_test, + ) - for key, value in NEW_RUN_DEFAULTS.items(): - if getattr(args, key, None) is None: - setattr(args, key, value) +def _resume_if_pending( + args: argparse.Namespace, inputs: _InitInputs, prep: _InitPreparation +) -> bool: + """終わっていない前回の状態があれば再開し、`True` を返す。""" + if not prep.state_file.exists(): + return False + state = statefile.load(prep.state_file) + if state.get("final") is not None: + return False + _resume(prep.state_file, state, args, inputs.model_spec, + inputs.include, inputs.exclude, prep.is_own_pr) + return True + + +def _verify_init( + args: argparse.Namespace, inputs: _InitInputs, prep: _InitPreparation +) -> tuple[dict[str, Any], dict[str, Any], dict[str, Any]]: + """参加者を確定し、着手前のテストと範囲のテストを実行する。""" # **確認は着手前のテストより先に行う。** 使える者がいなければ、テストに時間を # 使わずに止める。 participants = resolve_participants( - host, include or [], exclude or [], bool(getattr(args, "require_all", None))) - _warn_unmeasurable_models(model_spec, participants["available"]) + inputs.host, inputs.include or [], inputs.exclude or [], + bool(getattr(args, "require_all", None))) + _warn_unmeasurable_models(inputs.model_spec, participants["available"]) + + hint = round_test_hint(prep.round_test, args.baseline_test, args.scope, str(prep.work)) + if hint: + info(hint) - baseline = _run_baseline_test(args.baseline_test, work, args.test_timeout) + baseline = _run_baseline_test(args.baseline_test, prep.work, args.test_timeout) + round_record = _run_round_test(prep.round_test, baseline, prep.work, args.test_timeout) + return participants, baseline, round_record + +def _save_initial_state( + args: argparse.Namespace, + inputs: _InitInputs, + prep: _InitPreparation, + participants: dict[str, Any], + baseline: dict[str, Any], + round_record: dict[str, Any], +) -> dict[str, Any]: + """初期の状態を組み立てて保存し、保存した状態を返す。""" context = InitialContext( - repo=repo, - base_branch=base_branch, - head_branch=head_branch, - root=root, - work=work, - tmp_dir=tmp_dir, - host=host, - detection=detection, + repo=prep.repo, + base_branch=prep.base_branch, + head_branch=prep.head_branch, + root=prep.root, + work=prep.work, + tmp_dir=prep.tmp_dir, + host=inputs.host, + detection=inputs.detection, participants=participants, - model_spec=model_spec, + model_spec=inputs.model_spec, baseline=baseline, + round_test=round_record, ) state = _build_initial_state(args, context) # GitHub は自分の Pull Request への `APPROVE` と `REQUEST_CHANGES` を # `HTTP 422` で拒む。判定はそのまま結果ファイルへ残し、**投稿の event だけ** # を倒す。収束判定は結果ファイルの判定を見るので、倒しても進行は変わらない。 - _apply_post_event(state, is_own_pr) - statefile.save(state_file, state) - info(f"✅ 状態を初期化しました: {state_file}") - info(f" ホスト: {host}（{detection}）") + _apply_post_event(state, prep.is_own_pr) + statefile.save(prep.state_file, state) + info(f"✅ 状態を初期化しました: {prep.state_file}") + info(f" ホスト: {inputs.host}（{inputs.detection}）") info(f" 参加者（提案と適用）: {' / '.join(state['runtimes'])}") - _emit_init(state) + return state def _resume( @@ -496,6 +598,7 @@ def _notify_view( """ view = dict(state) view["baseline_test"] = (state.get("baseline_test") or {}).get("command") + view["round_test"] = (state.get("round_test") or {}).get("command") given = argparse.Namespace(**{f.arg: getattr(args, f.arg, None) for f in RESUME_NOTIFY_FIELDS}) if given.model is not None: given.model = model_spec @@ -625,9 +728,65 @@ def _run_baseline_test( return {"command": command, "status": status, "checked_at": statefile.now()} -def rounds_of_kind(state: dict[str, Any], kind: str) -> list[dict[str, Any]]: - """その種類のラウンドだけを取り出す。上限はそれぞれ別に数える。""" - return [r for r in state.get("rounds") or [] if entry_kind(r) == kind] +def _run_round_test( + command: Optional[str], baseline: dict[str, Any], work: pathlib.Path, + timeout: int = DEFAULT_TEST_TIMEOUT, +) -> dict[str, Any]: + """範囲のテストを着手前に 1 回実行して記録する（#880）。 + + **省いたとき、または全体テストと同じ文字列のときは実行しない。** 同じコマンドを + 2 度走らせても判定は変わらず、時間だけが掛かる。全体テストの結果を写す。 + + **失敗は全体テストと別に止める。** 全体テストが通っても範囲のテストが通らない + （テストが 1 件も集まらない終了コード 5 を含む）なら、群の検証が初回から落ちる。 + """ + if not command or command == baseline["command"]: + return {"command": baseline["command"], "status": baseline["status"], + "checked_at": baseline["checked_at"]} + code, timed_out = run_with_timeout(command, str(work), timeout) + if timed_out: + die(f"範囲のテストが {timeout} 秒で終わりませんでした（{command}）。打ち切りました") + raise SystemExit(ABORT) + if code != 0: + die( + f"範囲のテストが成功しません（{command} / 終了コード {code}）。" + "--round-test が --scope のテストの置き場所を走らせるかを確かめてください" + ) + raise SystemExit(ABORT) + info(f"✅ 着手前の範囲のテスト成功: {command}") + return {"command": command, "status": "green", "checked_at": statefile.now()} + + +def _new_round_entry( + state: dict[str, Any], round_no: int, kind: str +) -> dict[str, Any]: + """新しいラウンドの記録を組み立てる。""" + impl, requested = impl_for_seq(state, round_no) + return { + "round": round_no, + # **種類はラウンドごとに残す。** 上限を別々に数えるためと、提案の + # 重複率を同じ種類どうしで測るためである。 + "kind": kind, + "started_at": statefile.now(), + "impl": impl, + "impl_model": {"requested": requested, "observed": None}, + "proposed": {}, + "merged": 0, "adopted": 0, "deferred": 0, + "items": [], + "apply": {"applied": [], "failed": [], "base_sha": None, "head_sha": None}, + "fix_rounds": 0, + "durations": {}, + "reviews": [], + } + + +def _round_label_and_limit( + state: dict[str, Any], kind: str +) -> tuple[str, Optional[int]]: + """ラウンドの種類に対応する表示名と上限を返す。""" + if kind == TEST: + return "テスト整備ラウンド", state.get("max_test_rounds") + return "提案ラウンド", state["max_outer_rounds"] def cmd_start_round(args: argparse.Namespace) -> None: @@ -652,43 +811,22 @@ def cmd_start_round(args: argparse.Namespace) -> None: rounds = state["rounds"] kind = round_kind(state) - if kind == STRUCTURE and len(rounds_of_kind(state, STRUCTURE)) >= state["max_outer_rounds"]: + if kind == STRUCTURE and len(rounds_of_kind(state.get("rounds") or [], STRUCTURE)) >= state["max_outer_rounds"]: finish_outer_rounds(path, state, "max_outer_rounds") sys.exit(1) round_no = len(rounds) + 1 existing = next((r for r in rounds if r["round"] == round_no), None) if existing is None: - impl, requested = impl_for_seq(state, round_no) - existing = { - "round": round_no, - # **種類はラウンドごとに残す。** 上限を別々に数えるためと、提案の - # 重複率を同じ種類どうしで測るためである。 - "kind": kind, - "started_at": statefile.now(), - "impl": impl, - "impl_model": {"requested": requested, "observed": None}, - "proposed": {}, - "merged": 0, "adopted": 0, "deferred": 0, - "items": [], - "apply": {"applied": [], "failed": [], "base_sha": None, "head_sha": None}, - "fix_rounds": 0, - "durations": {}, - "reviews": [], - } + existing = _new_round_entry(state, round_no, kind) rounds.append(existing) state["outer_round"] = round_no state["phase"] = "propose" statefile.save(path, state) kind = entry_kind(existing) - if kind == TEST: - label = "テスト整備ラウンド" - limit = state.get("max_test_rounds") - else: - label = "提案ラウンド" - limit = state["max_outer_rounds"] - seq = len(rounds_of_kind(state, kind)) + label, limit = _round_label_and_limit(state, kind) + seq = len(rounds_of_kind(state.get("rounds") or [], kind)) info( f"=== {label} {seq} / {limit} " f"（実装 {existing['impl']}）===" diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py index f4b19107..67958aac 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py @@ -12,6 +12,7 @@ import signal import subprocess import time +from dataclasses import dataclass from typing import Any, Optional import models as models_lib @@ -36,6 +37,12 @@ TEST_PATH_MARKERS = ("/test/", "/tests/", "/spec/", "/specs/", "__tests__/") TEST_NAME_MARKERS = (".test.", ".spec.", "_test.", "_spec.", "test_", "spec_") +# 本番コードの拡張子。構造改善を飛ばしてよいかの判定（`assess`）に使う（#494）。 +CODE_EXTENSIONS = frozenset({ + ".py", ".sh", ".bash", ".js", ".mjs", ".cjs", ".ts", ".tsx", ".jsx", ".php", + ".rb", ".go", ".rs", ".java", ".kt", ".swift", ".c", ".h", ".cc", ".cpp", ".cs", +}) + def safe_int(value: Any, fallback: int = 0) -> int: """LLM が返した値を int にする。数値として読めなければ `fallback`。 @@ -173,7 +180,7 @@ def commit_test_changes(work: str, sha: str) -> dict[str, tuple[list[str], list[ out = git_out(work, ["show", "--name-only", "--format=", sha]) changes: dict[str, tuple[list[str], list[str]]] = {} for path in (out or "").splitlines(): - if not path.strip() or not _is_test_path(path): + if not path.strip() or not is_test_path(path): continue before = git_out(work, ["show", f"{sha}^:{path}"]) or "" after = git_out(work, ["show", f"{sha}:{path}"]) or "" @@ -182,7 +189,17 @@ def commit_test_changes(work: str, sha: str) -> dict[str, tuple[list[str], list[ return changes -def _is_test_path(path: str) -> bool: +def tracked_markdown(work: str) -> list[str]: + """追跡している `.md` のリポジトリ相対パス（#723）。 + + `-z` で読む。既定の出力は ASCII 以外を含むパスを引用符と 8 進数で書き換える。 + パターン `*.md` は `/` をまたいで一致し、下の階層の `.md` も拾う。 + """ + out = git_out(work, ["ls-files", "-z", "*.md"], strip=False) + return [p for p in (out or "").split("\0") if p] + + +def is_test_path(path: str) -> bool: """テストの置き場所か。判定は `commit_touches_tests` と同じ印で行う。""" lowered = f"/{path.lower()}" name = lowered.rsplit("/", 1)[-1] @@ -190,9 +207,34 @@ def _is_test_path(path: str) -> bool: or any(m in name for m in TEST_NAME_MARKERS)) +def production_code_changes(work: str, base: str) -> Optional[list[tuple[str, int]]]: + """`...HEAD` の差分のうち、本番コードのファイルと変更行（追加 + 削除）を返す。 + + `` を解けないときは `None`。**`--no-renames` を付ける。** 付けないと rename が + `dir/{old.py => new.py}` の形になり、拡張子で判定できない。付ければ旧パスの削除と + 新パスの追加に分かれ、両方のパスで判定できる。`-z` は、ASCII 以外を含むパスが + 引用符付きで出て拡張子が読めなくなるのを防ぐ。 + """ + out = git_out(work, ["diff", "--numstat", "-z", "--no-renames", f"{base}...HEAD"]) + if out is None: + return None + changes: list[tuple[str, int]] = [] + for line in out.split("\0"): + parts = line.split("\t") + if len(parts) < 3: + continue + path = parts[2] + if (pathlib.PurePosixPath(path).suffix.lower() not in CODE_EXTENSIONS + or is_test_path(path)): + continue + # バイナリは `-` になるので数えない + changes.append((path, sum(int(n) for n in parts[:2] if n.isdigit()))) + return changes + + def commit_touches_tests(work: str, sha: str) -> bool: """コミットがテストの置き場所を触っているか。""" - return any(_is_test_path(p) for p in commit_files(work, sha)) + return any(is_test_path(p) for p in commit_files(work, sha)) def run_with_timeout( @@ -262,6 +304,9 @@ def _kill_process_group( deadline = time.monotonic() + grace while time.monotonic() < deadline: + # 親シェルを先に回収する。回収しないとゾンビがグループに残り、 + # 子がすべて終わっていても猶予を最後まで待つ（#883） + proc.poll() if not _process_group_alive(pgid): return time.sleep(0.2) @@ -604,10 +649,33 @@ def _pending_drop_item_ids(state: dict[str, Any], drop_ids: list[str]) -> list[s ] +@dataclass +class _DropPlan: + """取り消しと積み直しの計画。`drop_items` が組み立て、実行と記録の段へ渡す。 + + `ordered` は取り消す範囲（新しい順）、`replay` は積み直す SHA（古い順）、 + `owner` は `コミット → 改善項目 ID` の対応である。 + """ + + pending: list[str] + ordered: list[str] + owner: dict[str, str] + keep_ids: list[str] + replay: list[str] + + +def _drop_summary( + mode: str, dropped: list[str], reverted: int, replayed: int +) -> dict[str, Any]: + """取り消しの結果の形。戻り値と `entry.drops` の記録が同じ鍵を持つ。""" + return {"mode": mode, "dropped": dropped, + "reverted": reverted, "replayed": replayed} + + def _drop_replay_plan( state: dict[str, Any], entry: dict[str, Any], pending: list[str], ordered: list[str], -) -> tuple[dict[str, str], list[str], list[str]]: +) -> _DropPlan: """残す項目 (`keep_ids`) と積み直す SHA (`replay`) を求める。 `ordered` は新しい順なので、積み直しは反転して古い順にする。 @@ -622,33 +690,30 @@ def _drop_replay_plan( and not (find_item(state, i, required=False) or {}).get("reverted") ] replay = [s for s in reversed(ordered) if owner.get(s) in keep_ids] - return owner, keep_ids, replay + return _DropPlan(pending, ordered, owner, keep_ids, replay) -def _dry_run_drop_plan( - pending: list[str], ordered: list[str], replay: list[str] -) -> dict[str, Any]: +def _dry_run_drop_plan(plan: _DropPlan) -> dict[str, Any]: """dry-run 時の出力と戻り値を作る。実際の revert/cherry-pick は行わない。""" - for sha in ordered: + for sha in plan.ordered: info(f"（dry-run）git revert --no-edit {sha}") - for sha in replay: + for sha in plan.replay: info(f"（dry-run）git cherry-pick {sha}") - return {"mode": "item", "dropped": pending, - "reverted": len(ordered), "replayed": len(replay)} + return _drop_summary("item", plan.pending, len(plan.ordered), len(plan.replay)) def _execute_drop_replay( - work: str, ordered: list[str], head: Optional[str], replay: list[str], + work: str, plan: _DropPlan, head: Optional[str], ) -> tuple[dict[str, str], str]: """範囲を取り消して残す項目を積み直す。積み直しに失敗したら round モードへ退避する。 戻り値は `(mapping, mode)`。`mapping` は積み直し後の SHA 対応 （`round` モードでは空）。 """ - _revert_range(work, ordered, head) + _revert_range(work, plan.ordered, head) # 取り消しが済んだ地点。積み直しに失敗したらここへ戻せばよい。 reverted_head = git_out(work, ["rev-parse", "HEAD"]) - mapping = _replay_commits(work, replay) + mapping = _replay_commits(work, plan.replay) if mapping is None: info("⚠ 残す項目を積み直せませんでした。このラウンドは全件取り消します") # **着手前まで戻して取り消しをやり直さない。** 同じ範囲に対する取り消しが @@ -661,38 +726,33 @@ def _execute_drop_replay( def _record_drop_result( state: dict[str, Any], entry: dict[str, Any], - pending: list[str], - keep_ids: list[str], - ordered: list[str], - replay: list[str], - owner: dict[str, str], + plan: _DropPlan, mapping: dict[str, str], mode: str, ) -> dict[str, Any]: """item の reverted/commits と entry.drops を更新し、結果を返す。""" scoped = scoped_item_ids(entry) - dropped = list(scoped) if mode == "round" else pending + dropped = list(scoped) if mode == "round" else plan.pending for item_id in scoped: item = find_item(state, item_id, required=False) if item is None: continue - if mode == "round" or item_id not in keep_ids: + if mode == "round" or item_id not in plan.keep_ids: item["reverted"] = True continue # **積み直しで SHA が変わる。** 記録を更新しないと、次の取り消しが # 履歴に無い SHA を指してしまう。 - item["commits"] = [mapping[s] for s in replay if owner.get(s) == item_id] + item["commits"] = [mapping[s] for s in plan.replay if plan.owner.get(s) == item_id] entry.setdefault("drops", []).append({ - "at": statefile.now(), "mode": mode, "dropped": dropped, - "reverted": len(ordered), "replayed": len(mapping), + "at": statefile.now(), + **_drop_summary(mode, dropped, len(plan.ordered), len(mapping)), }) info( - f"↩ 取り消し {len(ordered)} コミット / 積み直し {len(mapping)} コミット" + f"↩ 取り消し {len(plan.ordered)} コミット / 積み直し {len(mapping)} コミット" f"（{'ラウンド全件へ退避' if mode == 'round' else '項目単位'}）" ) - return {"mode": mode, "dropped": dropped, - "reverted": len(ordered), "replayed": len(mapping)} + return _drop_summary(mode, dropped, len(plan.ordered), len(mapping)) def _drop_legacy_by_item( @@ -707,8 +767,7 @@ def _drop_legacy_by_item( reverted = 0 for item_id in pending: reverted += revert_item_commits(state, find_item(state, item_id), dry_run) - return {"mode": "item", "dropped": pending, - "reverted": reverted, "replayed": 0} + return _drop_summary("item", pending, reverted, 0) def drop_items( @@ -737,7 +796,7 @@ def drop_items( pending = _pending_drop_item_ids(state, drop_ids) if not pending: info("↩ 取り消し対象は取り消し済みです") - return {"mode": "skip", "dropped": [], "reverted": 0, "replayed": 0} + return _drop_summary("skip", [], 0, 0) head = git_out(work, ["rev-parse", "HEAD"]) ordered = commits_in_range(work, entry.get("apply_base_sha"), head or "HEAD") @@ -745,16 +804,14 @@ def drop_items( # 起点を記録していない状態ファイル（旧版）では積み直せない。 return _drop_legacy_by_item(state, pending, dry_run) - owner, keep_ids, replay = _drop_replay_plan(state, entry, pending, ordered) + plan = _drop_replay_plan(state, entry, pending, ordered) if dry_run: - return _dry_run_drop_plan(pending, ordered, replay) + return _dry_run_drop_plan(plan) - mapping, mode = _execute_drop_replay(work, ordered, head, replay) + mapping, mode = _execute_drop_replay(work, plan, head) - return _record_drop_result( - state, entry, pending, keep_ids, ordered, replay, owner, mapping, mode, - ) + return _record_drop_result(state, entry, plan, mapping, mode) def _order_newest_first(work: str, shas: list[str]) -> list[str]: diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py index 6752482a..6c8cf95e 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py @@ -306,6 +306,10 @@ def reject(item: dict[str, Any]) -> Optional[str]: "語彙外の値を含むため対象外" "（`case` と `level` は列挙した識別子のいずれかで書く）" ) + # **文書の文言を固定するテストは採らない**（#723）。文書の振る舞いは文言 + # そのものであり、固定すると後の文書の整理が振る舞いの変更として取り消される。 + if item["target"].split("#", 1)[0].endswith(".md"): + return "文書の文言を固定するテストは足さない" return None return _select( diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py index 0b4c825c..5004afb6 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py @@ -36,6 +36,11 @@ def entry_kind(entry: dict[str, Any]) -> str: return TEST if entry.get("kind") == TEST else STRUCTURE +def rounds_of_kind(rounds: list[dict[str, Any]], kind: str) -> list[dict[str, Any]]: + """その種類のラウンドだけを取り出す。上限はそれぞれ別に数える。""" + return [r for r in rounds if entry_kind(r) == kind] + + def item_kind(item: dict[str, Any]) -> str: """項目 1 件の種類。改善項目とテスト項目は同じ一覧に並ぶ。""" return TEST if item.get("kind") == TEST else STRUCTURE @@ -87,6 +92,21 @@ def deferred_record( record.update({"symbol": item.get("symbol"), "smell": item.get("smell")}) return record + +def append_deferred_abandoned_items( + state: dict[str, Any], items: list[dict[str, Any]], default_reason: str +) -> None: + """abandoned の項目を未登録時だけ見送り記録へ追記する。""" + already = {d.get("item_id") for d in state["deferred_items"]} + for item in items: + item_id = item["item_id"] + if item_id in already: + continue + reason = item.get("failure_reason") or default_reason + state["deferred_items"].append(deferred_record(item, item_id, reason)) + already.add(item_id) + + def finish_outer_rounds(path: pathlib.Path, state: dict[str, Any], reason: str) -> None: state["final"] = reason state["ended_at"] = statefile.now() diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/scope.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/scope.py index c007044c..b5edf2a1 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/scope.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/scope.py @@ -17,9 +17,10 @@ import os import pathlib import shlex -from typing import Iterable, Optional +from typing import Any, Callable, Iterable, Optional from . import die +from .gitfacts import is_test_path # テストの置き場所とみなすディレクトリの名前。**言語をまたいで使われるものだけ**を # 並べる。増やすほど「テストの置き場所がある」と誤って判定して関門が素通りする。 @@ -33,6 +34,25 @@ ) +# `--round-test` のうち、次の語を値として取ると分かっているオプション（#880）。 +# `--opt=value` の形は 1 語に値を含むため並べない。**知らないオプションの直後の語は +# 起点に数える**（`round_test_roots`）。ここに無い値付きオプションは関門を止める側に +# 倒れるだけで、範囲の外を素通りさせない。 +VALUE_OPTIONS: frozenset[str] = frozenset({ + # uv run + "--project", "--directory", "--with", "--with-editable", "--with-requirements", + "--python", "-p", "--package", "--extra", "--group", "--only-group", + "--env-file", "--index", "--default-index", "--index-url", "--extra-index-url", + "--find-links", "-f", "--cache-dir", "--config-file", + # pytest + "--rootdir", "-c", "--confcutdir", "--basetemp", "-k", "-m", "-n", + "--numprocesses", "--dist", "-o", "--override-ini", "--tb", "--ignore", + "--ignore-glob", "--deselect", "--junit-xml", "--junitxml", "--log-file", + "--cov", "--cov-report", "--cov-config", "--maxfail", "--durations", + "-W", "--pythonwarnings", "--import-mode", "--capture", "-r", +}) + + def _matches_by_name(path: str) -> bool: """名前だけで置き場所と読めるか。**実在は見ない。** @@ -104,36 +124,138 @@ def baseline_search_roots(command: str, work: str) -> list[str]: 語として読めないコマンド（引用符が閉じていないなど）は限定なしとして扱う。 ここは範囲の宣言を読むための補助であり、コマンドの妥当性を判定する場所ではない。 """ + return _scope_roots( + command, work, + lambda word, previous, normalized: (pathlib.Path(work) / word).is_dir(), + ) + + +def covered_by_roots(location: str, roots: list[str]) -> bool: + """テストの置き場所が探索範囲の中にあるか。**限定が無ければ全て入る。**""" + if not roots: + return True + normalized = os.path.normpath(str(location)) + return any( + normalized == root or normalized.startswith(root + "/") for root in roots + ) + + +def round_test_roots(command: str, work: str) -> list[str]: + """`--round-test` の実行集合の起点を返す（#880）。空なら全体を走らせるとみなす。 + + `baseline_search_roots` と違い、**テストの置き場所に当たる実在するファイルも起点に + 数える。** 範囲のテストは 1 ファイルを名指しすることがあり、それを限定なしと読むと + 範囲の置き場所を走らせないコマンドが関門を通る。 + + 数えない語は 3 つある。先頭の語（プログラム名）と `-` で始まる語、値を取ると + 分かっているオプションの直後の語（`--project .` の `.`）、作業ディレクトリの根 + （`.`）である。テストの置き場所でないファイル（`bash scripts/run-scope-tests.sh` の + ラッパー）も数えない。**ラッパーの中身は解析しない。** 範囲の外を走らせても、 + 最終ゲートの全体テストが見る。 + + **値を取るかが分からないオプションの直後の語は起点に数える。** 値とみなして + 消すと、`pytest --verbose tests/unit` の唯一の対象が消えて起点が空になり、全体を + 覆うとみなして範囲の外だけを走らせるコマンドが関門を通る。数えすぎたときは + 関門が止まる側に倒れ、利用者が 1 度直せば済む。 + """ + def accept(word: str, previous: str, normalized: str) -> bool: + if previous in VALUE_OPTIONS: + return False + if normalized == ".": + return False + target = pathlib.Path(work) / word + return target.is_dir() or (target.is_file() and is_test_path(normalized)) + + return _scope_roots(command, work, accept) + + +def _scope_roots( + command: str, work: str, accept: Callable[[str, str, str], bool] +) -> list[str]: + """コマンドの語のうち `accept(語, 直前の語, 正規化した語)` が真のものを返す。 + + **先頭の語**（プログラム名）と `-` で始まる語、絶対パスは見ない。ノード ID は + `::` より前のファイルの部分で読む。正規化した + 語を重複なく、現れた順に集める。語として読めないコマンドは空を返す。 + """ try: words = shlex.split(str(command or "")) except ValueError: return [] roots: list[str] = [] + previous = words[0] if words else "" for word in words[1:]: + before, previous = previous, word if word.startswith("-") or os.path.isabs(word): continue - if not (pathlib.Path(work) / word).is_dir(): - continue + # pytest のノード ID（`<ファイル>::<名前>`）はファイルの部分で読む。 + word = word.split("::", 1)[0] normalized = os.path.normpath(word) - if normalized not in roots: + if accept(word, before, normalized) and normalized not in roots: roots.append(normalized) return roots -def covered_by_roots(location: str, roots: list[str]) -> bool: - """テストの置き場所が探索範囲の中にあるか。**限定が無ければ全て入る。**""" - if not roots: - return True - normalized = os.path.normpath(str(location)) - return any( - normalized == root or normalized.startswith(root + "/") for root in roots +def round_test_command(state: dict[str, Any]) -> str: + """群と修正コミットの検証に使うコマンド（#880）。 + + `round_test` を持たない状態ファイル（変更の前の実行）は `baseline_test` を返す。 + 再開した実行の検証を、変更の前と同じにするためである。 + """ + command = (state.get("round_test") or {}).get("command") + if command: + return str(command) + return str((state.get("baseline_test") or {}).get("command") or "") + + +def _example_program(baseline_test: str, roots: list[str]) -> str: + """案内の例に使うプログラムの部分。起点があれば、最初の起点より前の語である。""" + try: + words = shlex.split(str(baseline_test or "")) + except ValueError: + return str(baseline_test or "") + for index, word in enumerate(words): + if roots and os.path.normpath(word) in roots: + return " ".join(words[:index]) + while len(words) > 1 and words[-1].startswith("-"): + words.pop() + return " ".join(words) + + +def round_test_hint( + round_test: Optional[str], baseline_test: str, + scope: Iterable[str], work: str, +) -> Optional[str]: + """`--round-test` を渡せば群ごとの検証が短くなるときに、案内の 1 行を返す（#880）。 + + 案内するのは、`--round-test` が無く、`--baseline-test` の探索の起点が無いか + `--scope` のテストの置き場所より広いときである。**止めない。** 全体を走らせても + 検証として誤りではなく、時間が掛かるだけである。 + """ + if round_test: + return None + locations = test_locations(list(scope), work) + if not locations: + return None + roots = baseline_search_roots(baseline_test, work) + if roots and all(covered_by_roots(root, locations) for root in roots): + return None + example = f"{_example_program(baseline_test, roots)} {' '.join(locations)}".strip() + return ( + "ℹ --baseline-test は --scope より広い範囲を走らせます。" + "群ごとの検証を短くするには --round-test に範囲のテストを渡します" + f"（例: {example}）" ) def scope_problem( - scope: Iterable[str], baseline_test: str, work: str + scope: Iterable[str], command: str, work: str, round_test: bool = False ) -> Optional[str]: - """関門に引っかかる理由を返す。問題が無ければ `None`。""" + """関門に引っかかる理由を返す。問題が無ければ `None`。 + + `round_test` が真なら、`command` を `--round-test` として読む（#880）。群の検証が + 走らせるのは範囲のテストであり、足したテストが入るべき実行集合はこちらである。 + """ listed = list(scope) locations = test_locations(listed, work) if not locations: @@ -144,23 +266,24 @@ def scope_problem( "含めないとその項目は必ず失敗します。" "例: --scope src/services tests/services" ) - roots = baseline_search_roots(baseline_test, work) + option = "--round-test" if round_test else "--baseline-test" + roots = (round_test_roots if round_test else baseline_search_roots)(command, work) outside = [loc for loc in locations if not covered_by_roots(loc, roots)] if outside: return ( f"--scope のテストの置き場所（{', '.join(outside)}）が " - f"--baseline-test の実行集合に入りません" + f"{option} の実行集合に入りません" f"（探索の起点: {', '.join(roots)}）。" "足したテストが一度も実行されず、検証の判定に効きません。" - "--baseline-test の対象へ含めるか、--scope の置き場所を合わせてください" + f"{option} の対象へ含めるか、--scope の置き場所を合わせてください" ) return None def require_scope_covers_tests( - scope: Iterable[str], baseline_test: str, work: str + scope: Iterable[str], command: str, work: str, round_test: bool = False ) -> None: """関門を通す。通らなければ**中断する**（終了コード 4）。""" - problem = scope_problem(scope, baseline_test, work) + problem = scope_problem(scope, command, work, round_test=round_test) if problem: die(problem) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py index db5b9b10..0cfd403e 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py @@ -5,9 +5,13 @@ """ from __future__ import annotations +import ast +import difflib +import posixpath import re from collections import Counter +from dataclasses import dataclass from typing import Any, Iterable, Optional @@ -242,6 +246,8 @@ def _verify_apply_commit_count(facts: list[dict[str, Any]]) -> Optional[str]: def verify_apply_round( items: list[dict[str, Any]], facts: list[dict[str, Any]], scope: Optional[Iterable[str]] = None, + work: Optional[str] = None, + tracked_md: Iterable[str] = (), ) -> Optional[str]: """適用ラウンド 1 つ分の適用結果を検証する。問題があれば失敗理由を返す。 @@ -253,42 +259,87 @@ def verify_apply_round( `facts` は `collect_commit_facts()` が git から作る。振る舞い不変そのものは ここでは確かめない（テストは `verify-round` が実行する）が、**手順が守られたかは 結果から確かめられる**。 + + `tracked_md` は追跡している `.md` の一覧（`tracked_markdown()`）、`work` は + 補助モジュールを git から読む作業ディレクトリである（`doc_wording_tests`）。 """ - if not facts: + context = _ApplyRoundContext(items, facts, scope, work, tracked_md) + for stage in _APPLY_ROUND_STAGES: + problem = stage(context) + if problem: + return problem + return None + + +@dataclass +class _ApplyRoundContext: + """`verify_apply_round` の各段が受け取る入力。""" + + items: list[dict[str, Any]] + facts: list[dict[str, Any]] + scope: Optional[Iterable[str]] + work: Optional[str] + tracked_md: Iterable[str] + + +def _apply_round_basics(context: _ApplyRoundContext) -> Optional[str]: + """コミットの実在と、各コミットの基礎検査。""" + if not context.facts: return ( "コミットが 1 件もありません" "（適用ラウンド = 1 コミットの前提を満たしていません）" ) - for commit in facts: + for commit in context.facts: problem = _verify_commit_basics( commit, - scope, + context.scope, f"コミット {commit.get('sha', '?')} が base..head の範囲にありません" "（申告だけで実体がありません）", check_test=False, ) if problem: return problem + return None + - problem = _verify_test_gap_present(items, facts) +def _apply_round_test_protection(context: _ApplyRoundContext) -> Optional[str]: + """現状固定テストの有無と、テストの期待値の変更。""" + problem = _verify_test_gap_present(context.items, context.facts) if problem: return problem # **テストの期待値が変わっていないか**（#443）。段 1（機械）で決まるものだけを # ここで落とす。決まらないものは `pending_test_judgements` が集め、進行側が # 段 2（AI エージェント）へ渡す。 - changes = collect_test_changes(facts) - problem = verify_test_changes(changes) - if problem: - return problem + changes = collect_test_changes(context.facts) + return verify_test_changes(changes) - problem = _verify_diff_budget(items, facts) + +def _apply_round_diff_constraints(context: _ApplyRoundContext) -> Optional[str]: + """文言固定テスト・差分予算・コミット粒度。""" + # **文書の文言を固定するテストを足していないか**（#723）。 + hits = doc_wording_tests(context.facts, context.tracked_md, context.work) + if hits: + return ( + "文書の文言を固定するテストは足さない" + f"（{'、'.join(f'{path}: {literal}' for path, literal in hits)}）" + ) + + problem = _verify_diff_budget(context.items, context.facts) if problem: return problem # 粒度は最後に見る。トレーラーや範囲の問題を粒度の失敗で覆い隠さない。 - return _verify_apply_commit_count(facts) + return _verify_apply_commit_count(context.facts) + + +# 検査の順序そのものが規則である。先の段の問題を後の段の失敗で覆い隠さない。 +_APPLY_ROUND_STAGES = ( + _apply_round_basics, + _apply_round_test_protection, + _apply_round_diff_constraints, +) def commit_limit_for(item: dict[str, Any]) -> int: @@ -527,3 +578,139 @@ def apply_judgements_to_group( ) record_pending_judgements(entry, group, remaining) return remaining + + +# ---------- 文書の文言を固定するテスト（#723） ---------- +# +# **判定は追跡している `.md` のパスとの一致で行う。** `.md` で終わる文字列をすべて弾くと、 +# 一時ファイルの `.md` を入力に渡す検査スクリプトのテストまで弾く。追跡している `.md` と +# 同じ名前の一時ファイル（`README.md` など）を使うテストは当たるが、そのときも群が +# 取り消されるだけで、Pull Request に文言固定テストが残る側には倒れない（決定 9）。 +# +# **動的に組み立てたパス（`glob` の結果など）は追わない。** 提案の基準とレビューが見る。 + +_STRING = re.compile(r"""[rRbBuUfF]{0,2}(["'])((?:\\.|(?!\1)[^\\])*)\1""") + + +def _names_markdown(literal: str, tracked: Iterable[str]) -> bool: + """文字列が追跡している `.md` のパスか、`/` の区切りで揃えたその末尾に一致するか。""" + if not literal.endswith(".md"): + return False + return any(p == literal or p.endswith("/" + literal) for p in tracked) + + +def _markdown_literals(line: str, tracked: list[str]) -> list[str]: + return [m.group(2) for m in _STRING.finditer(line) + if _names_markdown(m.group(2), tracked)] + + +def _added_lines(before: list[str], after: list[str]) -> list[str]: + """変更の後にだけある行。置き換えた行も追加として数える。""" + matcher = difflib.SequenceMatcher(None, before, after, autojunk=False) + return [line for tag, _, _, j1, j2 in matcher.get_opcodes() + if tag in {"insert", "replace"} for line in after[j1:j2]] + + +def _parse(source: str) -> Optional[ast.Module]: + try: + return ast.parse(source) + except (SyntaxError, ValueError): + return None + + +def _markdown_constants(tree: ast.Module, tracked: list[str]) -> dict[str, str]: + """モジュールの直下の代入のうち、右辺が追跡している `.md` を指す名前と、その文字列。""" + found: dict[str, str] = {} + for node in tree.body: + if isinstance(node, ast.Assign): + targets, value = node.targets, node.value + elif isinstance(node, ast.AnnAssign) and node.value is not None: + targets, value = [node.target], node.value + else: + continue + literal = next(( + c.value for c in ast.walk(value) + if isinstance(c, ast.Constant) and isinstance(c.value, str) + and _names_markdown(c.value, tracked) + ), None) + if literal is None: + continue + for target in targets: + if isinstance(target, ast.Name): + found[target.id] = literal + return found + + +def _helper_source( + helper: str, changes: dict[str, tuple[list[str], list[str]]], + work: Optional[str], sha: Optional[str], +) -> Optional[str]: + """補助モジュールの変更の後の内容。群で触っていなければ git から読む。""" + if helper in changes: + return "".join(changes[helper][1]) + if not work or not sha: + return None + return git_out(work, ["show", f"{sha}:{helper}"], strip=False) + + +def _imported_constants( + tree: ast.Module, test_path: str, tracked: list[str], + changes: dict[str, tuple[list[str], list[str]]], + work: Optional[str], sha: Optional[str], +) -> dict[str, str]: + """同じディレクトリの補助モジュールから import した、`.md` を指す定数。""" + found: dict[str, str] = {} + folder = posixpath.dirname(test_path) + for node in tree.body: + if (not isinstance(node, ast.ImportFrom) or not node.module + or "." in node.module or node.level > 1): + continue + source = _helper_source( + posixpath.join(folder, f"{node.module}.py"), changes, work, sha) + helper_tree = _parse(source) if source else None + if helper_tree is None: + continue + constants = _markdown_constants(helper_tree, tracked) + for alias in node.names: + if alias.name in constants: + found[alias.asname or alias.name] = constants[alias.name] + return found + + +def _last_sha(facts: Iterable[dict[str, Any]]) -> Optional[str]: + shas = [c.get("sha") for c in facts if c.get("exists", True) and c.get("sha")] + return shas[-1] if shas else None + + +def doc_wording_tests( + facts: Iterable[dict[str, Any]], tracked_md: Iterable[str], + work: Optional[str] = None, +) -> list[tuple[str, str]]: + """追加したテストの行が、追跡している `.md` を指していれば `(ファイル, 文字列)` を返す。 + + 当たりは、追加行の文字列リテラルと、テストのファイルの直下の定数・同じディレクトリの + 補助モジュールから import した定数のうち `.md` を指すものを、追加行が識別子として + 使う場合である。補助モジュールを群で触っていなければ、`work` の git から読む。 + """ + facts = list(facts) + tracked = list(tracked_md) + if not tracked: + return [] + changes = collect_test_changes(facts) + sha = _last_sha(facts) + hits: list[tuple[str, str]] = [] + for path, (before, after) in sorted(changes.items()): + added = _added_lines(before, after) + found = {lit for line in added for lit in _markdown_literals(line, tracked)} + tree = _parse("".join(after)) if path.endswith(".py") else None + if tree is not None: + names = { + **_markdown_constants(tree, tracked), + **_imported_constants(tree, path, tracked, changes, work, sha), + } + found.update( + literal for name, literal in names.items() + if any(re.search(rf"\b{re.escape(name)}\b", line) for line in added) + ) + hits.extend((path, literal) for literal in sorted(found)) + return hits diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py b/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py index 4fe84471..f017764f 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_abandon_items.py @@ -1131,3 +1131,189 @@ def test_a_missing_fix_result_reverts_the_commits_in_range( entry = read_state(state_path)["rounds"][0] assert entry["fix_base_sha"] == "AFTER_REVERT" assert entry["apply_rounds"][0]["failed_attempts"][0]["reverted"] == 2 + + +# ---------- 修正コミットの検証は範囲のテスト（#880 の AC2） ---------- + +def test_fix_commits_are_verified_with_the_round_test(patch_lib, refactor, gitfacts, cmd_converge, tmp_path, env_tmp_dir, monkeypatch): + """AC2 — 修正コミットごとに走るのは `round_test` だけで、全体テストは走らない。""" + real_collect = gitfacts.collect_commit_facts + state_path = _prepare_fix(patch_lib, refactor, tmp_path, env_tmp_dir, monkeypatch, ["PRRT_a"]) + state = read_state(state_path) + state["round_test"] = {"command": "pytest tests/services -q", "status": "green"} + state_path.write_text(__import__("json").dumps(state), encoding="utf-8") + + ran: list[str] = [] + patch_lib("collect_commit_facts", real_collect) + patch_lib("run_with_timeout", + lambda command, cwd, timeout, grace=5.0: ran.append(command) or (0, False)) + patch_lib("commit_trailers", lambda work, sha: _fix_commit()["trailers"]) + patch_lib("commit_diff_lines", lambda work, sha: 10) + patch_lib("commit_files", lambda work, sha: ["src/foo.py"]) + patch_lib("commit_touches_tests", lambda work, sha: False) + patch_lib("commit_test_changes", lambda work, sha: {}) + patch_lib("resolved_threads_on_github", lambda repo, pr: {"PRRT_a"}) + monkeypatch.setattr(gitfacts.subprocess, "run", + lambda cmd, **kw: subprocess.CompletedProcess(cmd, 0, "", "")) + + cmd_converge.cmd_merge_fix(type("A", (), {"id": 130, "round": 1})()) + + assert ran == ["pytest tests/services -q"] + assert "fix111" in read_state(state_path)["items"][0]["commits"] + + +# ---------- run_drop が mode=round を返したときの取り消し（R2-002） ---------- +# +# 現状固定テスト。適用の検証に落ちた群を、run_drop が「積み直せなかった」 +# （mode=round）で返したときに cmd_merge_apply（_apply_drop）が行う状態の +# 書き換えは、どのテストも通していなかった（apply.py 1008-1017 が未到達）。 +# gitfacts.drop_items が round 退避を返すこと自体は test_drop_items_git.py が +# 固定しているので、ここではそれを受けた cmd_merge_apply 側だけを、run_drop を +# 差し替えて固定する。mode=item の側を対照として並べ、分岐の両側を固定する。 + + +def _fact_for(sha, **over): + """`collect_commit_facts()` が git から作る事実。""" + base = { + "sha": sha, "exists": True, "test_status": "pass", + "touches_tests": False, "diff_lines": 30, + "trailers": {"Item-Id": "R1-001", "Round": "1", + "Impl-Runtime": "codex", "Impl-Model": "gpt-5.5"}, + } + base.update(over) + return base + + +def _two_item_group_state(tmp_path): + """群に 2 項目を持ち、検証に落ちる（片方の申告が欠ける）適用結果の状態。""" + from crossref_helpers import write_result + items = [_item("R1-001", []), _item("R1-002", [])] + for it in items: + it["status"] = "pending" + state_path = make_state( + tmp_path, + items=items, + rounds=[{ + "round": 1, "impl": "codex", "reviewers": REVIEWERS, + "impl_model": {"requested": "gpt-5.5", "observed": None}, + "reviewer_models": {r: {"requested": None, "observed": None} + for r in REVIEWERS}, + "proposed": {}, "merged": 2, "adopted": 2, "deferred": 0, + "items": ["R1-001", "R1-002"], + "apply_rounds": [{ + "apply_round": 1, "impl": "codex", + "impl_model": {"requested": "gpt-5.5", "observed": None}, + "items": ["R1-001", "R1-002"], "status": "pending", + "base_sha": None, "head_sha": None, "fix_rounds": 0, + }], + "apply_round": 1, + "apply": {"applied": [], "failed": []}, "fix_rounds": 0, + "durations": {}, "reviews": [], + }], + ) + write_result(state_path, "codex-apply-r1", { + "base_sha": "aaa", "elapsed_seconds": 100, + "items": [ + # 群は 1 コミットを共有する。所有権の検査は通るが、コミット自体が + # 検証（トレーラー欠け）に落ちて群の全件が失敗する + {"item_id": "R1-001", "commits": [{"sha": "ok111"}]}, + {"item_id": "R1-002", "commits": [{"sha": "ok111"}]}, + ], + }) + return state_path + + +def _stub_apply_facts(patch_lib): + """適用の検証が git から取る事実を差し替える。ok111 は範囲にあるが検証に落ちる。""" + # `Impl-Model` を欠いたトレーラーにして、群の検証を失敗させる。 + bad_trailers = {"Item-Id": "R1-001", "Round": "1", "Impl-Runtime": "codex"} + mapping = {"ok111": _fact_for("ok111", trailers=bad_trailers)} + patch_lib("commits_in_range", lambda work, base, head: ["ok111"]) + patch_lib("collect_commit_facts", + lambda work, shas, rng, cmd, branch, timeout=None: [ + mapping.get(s, {"sha": s, "exists": False}) for s in shas]) + + +def test_merge_apply_drops_the_whole_group_when_run_drop_returns_round( + patch_lib, refactor, tmp_path, env_tmp_dir, monkeypatch +): + """R2-002 — run_drop が mode=round のとき、群の全件を abandoned にする（apply.py 1008-1017）。""" + state_path = _two_item_group_state(tmp_path) + env_tmp_dir(state_path) + _stub_apply_facts(patch_lib) + # `rev-parse HEAD` は固定の SHA。他の git 呼び出しは SHA をそのまま返す。 + patch_lib("git_out", + lambda work, args, **_kw: "DROPPED_HEAD" if args == ["rev-parse", "HEAD"] + else args[-1].replace("^{commit}", "")) + dropped: list = [] + patch_lib("run_drop", + lambda path, state, entry, failed: dropped.append(list(failed)) + or {"mode": "round", "dropped": [], "reverted": 2, "replayed": 0}) + _apply_mod = sys.modules["refactor_lib.commands.apply"] + patch_lib("push_with_retry_marker", + lambda path, state, entry: _apply_mod.statefile.save(path, state)) + monkeypatch.setattr(sys.modules["refactor_lib.commands.apply"].statefile, "now", + lambda: "2026-09-23T00:00:00") + + with pytest.raises(SystemExit) as e: + refactor.cmd_merge_apply(_args()) + assert e.value.code == 2 + + state = read_state(state_path) + entry = state["rounds"][0] + by_id = {i["item_id"]: i for i in state["items"]} + # 群の 2 項目がともに abandoned で failure_reason を持つ + assert by_id["R1-001"]["status"] == "abandoned" + assert by_id["R1-002"]["status"] == "abandoned" + assert by_id["R1-001"]["failure_reason"] and by_id["R1-002"]["failure_reason"] + # apply.applied は空、apply.failed は群の全項目 + assert entry["apply"]["applied"] == [] + assert entry["apply"]["failed"] == ["R1-001", "R1-002"] + # apply_base_sha は取り直した固定の SHA + assert entry["apply_base_sha"] == "DROPPED_HEAD" + # deferred_items に 2 項目が載る + assert {d["item_id"] for d in state["deferred_items"]} == {"R1-001", "R1-002"} + + +def test_merge_apply_keeps_run_drops_item_result_when_mode_is_item( + patch_lib, refactor, tmp_path, env_tmp_dir, monkeypatch +): + """R2-002（対照）— run_drop が mode=item のとき、_apply_drop は群を一括で abandoned にしない。 + + 項目単位の abandoned は run_drop（drop_items）側の責務なので、ここでは + fake run_drop がそれを模す。_apply_drop は apply.failed を群全体へ書き換えない。 + """ + state_path = _two_item_group_state(tmp_path) + env_tmp_dir(state_path) + _stub_apply_facts(patch_lib) + patch_lib("git_out", + lambda work, args, **_kw: "DROPPED_HEAD" if args == ["rev-parse", "HEAD"] + else args[-1].replace("^{commit}", "")) + + def fake_item_drop(path, state, entry, failed): + # 実際の drop_items（mode=item）は失敗した項目だけを abandoned にする + for item_id in failed: + find = next(i for i in state["items"] if i["item_id"] == item_id) + find["status"] = "abandoned" + find.setdefault("failure_reason", "項目単位で取り消した") + return {"mode": "item", "dropped": list(failed), "reverted": 1, "replayed": 1} + + patch_lib("run_drop", fake_item_drop) + _apply_mod = sys.modules["refactor_lib.commands.apply"] + patch_lib("push_with_retry_marker", + lambda path, state, entry: _apply_mod.statefile.save(path, state)) + monkeypatch.setattr(sys.modules["refactor_lib.commands.apply"].statefile, "now", + lambda: "2026-09-23T00:00:00") + + with pytest.raises(SystemExit) as e: + refactor.cmd_merge_apply(_args()) + assert e.value.code == 2 + + state = read_state(state_path) + entry = state["rounds"][0] + # mode=item では _apply_drop が apply.failed を群全体へ置き換えない + # （検証の記録のまま。群の全件が failed のまま残る）。 + assert entry["apply"]["failed"] == ["R1-001", "R1-002"] + # applied は検証で空になっており、_apply_drop はここで群を明示的に + # 上書きしない（round の分岐だけが上書きする）。 + assert entry["apply"]["applied"] == [] diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py b/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py index 2daa0f85..9b12b0ff 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_apply_rounds.py @@ -9,12 +9,19 @@ """ from __future__ import annotations +import os +import pathlib +import re +import subprocess import sys import pytest from crossref_helpers import make_state, read_state, write_result +LAUNCH = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "launch-cli.sh" + + def prop(path="src/a.py", symbol="f", **over): base = { "path": path, "symbol": symbol, "smell": "long_method", @@ -318,6 +325,58 @@ def test_phase_after_group_returns_to_propose_when_every_group_is_verified(round assert rounds.phase_after_group(entry) == "propose" +def _prompt_item_ids(tmp_path, apply_round, phase): + rounds = [ + {"round": 1, "apply_round": apply_round}, + {"round": 2}, + ] + items = [ + {"item_id": "R1-001", "round": 1, "apply_round": 1}, + {"item_id": "R1-002", "round": 1, "apply_round": 2}, + {"item_id": "R1-003", "round": 1}, + {"item_id": "R2-001", "round": 2}, + ] + state_path = make_state(tmp_path, rounds=rounds, items=items) + (tmp_path / "work").mkdir(exist_ok=True) + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + stub = bin_dir / "codex" + stub.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + stub.chmod(0o755) + + subprocess.run( + [str(LAUNCH), "codex", phase, "130", "1"], + env={ + **os.environ, + "CROSS_REFACTORING_TMP_DIR": str(state_path.parent), + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + }, + check=True, + capture_output=True, + text=True, + ) + prompt = state_path.parent / f"codex-{phase}-r1-prompt.md" + text = prompt.read_text(encoding="utf-8") + items_block = re.search( + r"## (?:適用する項目|この適用ラウンドの項目).*?```json\n(.*?)\n```", + text, + re.DOTALL, + ) + assert items_block is not None + return set(re.findall(r"R\d+-\d{3}", items_block.group(1))) + + +@pytest.mark.parametrize("phase", ["apply", "fix"]) +def test_launch_prompt_contains_only_the_current_apply_round(tmp_path, phase): + assert _prompt_item_ids(tmp_path, 2, phase) == {"R1-002"} + + +def test_launch_prompt_contains_the_whole_round_before_a_group_is_selected(tmp_path): + assert _prompt_item_ids(tmp_path, 0, "apply") == { + "R1-001", "R1-002", "R1-003" + } + + def test_phase_after_group_returns_to_propose_when_there_is_no_group(rounds): """現状固定: 群が空なら適用するものが無く、提案へ戻る。""" entry = _round_with_groups([], items=()) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_assess.py b/plugins/ndf/skills/cross-refactoring/tests/test_assess.py new file mode 100644 index 00000000..6aff3f20 --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/tests/test_assess.py @@ -0,0 +1,136 @@ +"""`refactor.py assess` を、**実際の git リポジトリ**と入口の CLI で確かめる（#494 の AC12〜AC14）。 + +起点のコミットに対して差分を 1 つ積み、終了コード（0 = 通す / 3 = 飛ばしてよい / +2 = 判定できない）と、出力の 3 行（`判定:` / `理由:` / `本番コード:`）を見る。 +""" +from __future__ import annotations + +import pathlib +import subprocess +import sys + +import pytest + +_SCRIPT = pathlib.Path(__file__).resolve().parents[1] / "scripts" / "refactor.py" + + +def _git(*args, cwd): + return subprocess.run(["git", *args], cwd=cwd, capture_output=True, + text=True, check=True) + + +def _lines(n: int, prefix: str = "x") -> str: + return "".join(f"{prefix}{i} = {i}\n" for i in range(n)) + + +@pytest.fixture +def repo(tmp_path): + """起点のコミット（ブランチ `base`）だけがある作業ディレクトリ。""" + work = tmp_path / "repo" + work.mkdir() + _git("init", "-q", "-b", "main", cwd=work) + _git("config", "user.email", "t@e.st", cwd=work) + _git("config", "user.name", "test", cwd=work) + (work / "src").mkdir() + (work / "src" / "a.py").write_text(_lines(12, "a")) + (work / "README.md").write_text("# readme\n") + _git("add", "-A", cwd=work) + _git("commit", "-qm", "init", cwd=work) + _git("branch", "base", cwd=work) + return work + + +def _commit(work: pathlib.Path, files: dict[str, str]) -> None: + for rel, body in files.items(): + path = work / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body) + _git("add", "-A", cwd=work) + _git("commit", "-qm", "change", cwd=work) + + +def _assess(work: pathlib.Path, *extra: str) -> tuple[int, list[str]]: + r = subprocess.run([sys.executable, str(_SCRIPT), "assess", *extra], + cwd=work, capture_output=True, text=True) + return r.returncode, r.stdout.splitlines() + + +@pytest.mark.parametrize("files", [ + {"docs/guide.md": "# guide\n" + _lines(30)}, + {"tests/test_a.py": _lines(30, "t")}, + {"config/settings.json": "{\n" + "".join(f'"k{i}": {i},\n' for i in range(30)) + "}\n"}, +], ids=["md-only", "tests-only", "json-only"]) +def test_no_production_code_may_be_skipped(repo, files): + _commit(repo, files) + rc, out = _assess(repo, "--base", "base") + assert rc == 3 + assert out == [ + "判定: 飛ばしてよい", + "理由: 本番コードの差分がありません", + "本番コード: 0 ファイル・0 行", + ] + + +def test_ten_lines_of_production_code_may_be_skipped(repo): + _commit(repo, {"src/b.py": _lines(10, "b")}) + rc, out = _assess(repo, "--base", "base") + assert rc == 3 + assert out == [ + "判定: 飛ばしてよい", + "理由: 本番コードの変更が 10 行で、上限 10 行以下です", + "本番コード: 1 ファイル・10 行（src/b.py）", + ] + + +def test_eleven_lines_of_production_code_pass(repo): + _commit(repo, {"src/b.py": _lines(11, "b"), "README.md": _lines(50)}) + rc, out = _assess(repo, "--base", "base") + assert rc == 0 + assert out == [ + "判定: 通す", + "理由: 本番コードの変更が 11 行です", + "本番コード: 1 ファイル・11 行（src/b.py）", + ] + + +def test_rename_counts_both_paths(repo): + """rename は旧パスの削除と新パスの追加として、両方のパスで判定する。""" + _git("mv", "src/a.py", "src/b.py", cwd=repo) + _commit(repo, {"src/b.py": _lines(12, "a") + "extra = 1\n"}) + rc, out = _assess(repo, "--base", "base") + assert rc == 0 + assert out == [ + "判定: 通す", + "理由: 本番コードの変更が 25 行です", + "本番コード: 2 ファイル・25 行（src/a.py、src/b.py）", + ] + + +def test_max_lines_changes_the_limit(repo): + _commit(repo, {"src/b.py": _lines(11, "b")}) + rc, out = _assess(repo, "--base", "base", "--max-lines", "20") + assert rc == 3 + assert out[1] == "理由: 本番コードの変更が 11 行で、上限 20 行以下です" + + +def test_unresolvable_base_exits_2(repo): + rc, out = _assess(repo, "--base", "no-such-ref") + assert rc == 2 + assert out == [] + + +def test_binary_production_file_counts_as_zero_lines(repo): + """numstat が `-` を返すコード拡張子のバイナリも、現状どおり 0 行と数える。""" + path = repo / "src" / "binary.py" + path.write_bytes(b"before\0after") + _git("add", "-A", cwd=repo) + _git("commit", "-qm", "change", cwd=repo) + + rc, out = _assess(repo, "--base", "base") + + assert rc == 3 + assert out == [ + "判定: 飛ばしてよい", + "理由: 本番コードの変更が 0 行で、上限 10 行以下です", + "本番コード: 1 ファイル・0 行（src/binary.py）", + ] diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_doc_wording_tests.py b/plugins/ndf/skills/cross-refactoring/tests/test_doc_wording_tests.py new file mode 100644 index 00000000..53f447e6 --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/tests/test_doc_wording_tests.py @@ -0,0 +1,147 @@ +"""適用ラウンドの検証が、`.md` の文言を固定するテストの追加を弾くことのテスト（#723）。 + +**提案の基準だけでは、実装担当が書いたテストに混じったときに止まらない。** 取り込みで +`target` が `.md` の提案を見送るのに加えて、追加したテストの行が**追跡している `.md`** +を指していれば群を取り消す。`.md` で終わる文字列をすべて弾く形は採らない +（一時ファイルの `.md` を入力に渡すテストは残す）。 +""" +from __future__ import annotations + +import subprocess + +import pytest + +TRACKED = ["plugins/ndf/skills/x/SKILL.md", "README.md"] +REASON = "文書の文言を固定するテストは足さない" + + +def _change(before: list[str], added: list[str]) -> tuple[list[str], list[str]]: + """`test_changes` の 1 件（変更前後の行）。`added` を末尾へ足した形にする。""" + return before, before + added + + +def _fact(changes: dict, sha: str = "abc1234") -> dict: + return { + "sha": sha, "exists": True, "test_status": "pass", "touches_tests": True, + "diff_lines": 3, "files": sorted(changes), + "trailers": {"Item-Id": "T1-001", "Round": "1", + "Impl-Runtime": "codex", "Impl-Model": "gpt-5.5"}, + "test_changes": changes, + } + + +def _hit_files(verify, facts, work=None) -> list[str]: + return sorted({path for path, _ in verify.doc_wording_tests(facts, TRACKED, work)}) + + +# ---------- 追加行のリテラル ---------- + +def test_only_literals_naming_a_tracked_markdown_hit(verify): + """追跡している `.md` のパス・その末尾が一致するものだけが当たる。""" + facts = [_fact({ + "tests/test_full.py": _change([], [' p = ROOT / "plugins/ndf/skills/x/SKILL.md"\n']), + "tests/test_tail.py": _change([], [' p = ROOT / "x/SKILL.md"\n']), + "tests/test_tmp.py": _change([], [' p = tmp_path / "a.md"\n']), + })] + assert _hit_files(verify, facts) == ["tests/test_full.py", "tests/test_tail.py"] + + +def test_a_literal_only_in_unchanged_lines_does_not_hit(verify): + """見るのは追加行だけ。既存の行の文字列は、この群が足したものではない。""" + facts = [_fact({ + "tests/test_old.py": _change(['P = "README.md"\n'], ["def test_x():\n", " pass\n"]), + })] + assert _hit_files(verify, facts) == [] + + +# ---------- 定数と import を経由する ---------- + +def test_an_added_line_using_an_existing_constant_hits(verify): + """`SKILL = ROOT / "SKILL.md"` の形の定数を、追加行が使っていれば当たる。""" + before = ['SKILL = ROOT / "SKILL.md"\n', "\n"] + facts = [_fact({ + "tests/test_skill.py": _change( + before, ["def test_x():\n", ' assert "手順" in SKILL.read_text()\n']), + })] + assert verify.doc_wording_tests(facts, TRACKED) == [("tests/test_skill.py", "SKILL.md")] + + +def _git(*args, cwd): + return subprocess.run(["git", *args], cwd=cwd, capture_output=True, + text=True, check=True) + + +@pytest.fixture +def repo(tmp_path): + """補助モジュールだけを先に置き、テストのファイルを次のコミットで足す作業ディレクトリ。""" + work = tmp_path / "repo" + (work / "tests").mkdir(parents=True) + _git("init", "-q", "-b", "main", cwd=work) + _git("config", "user.email", "t@e.st", cwd=work) + _git("config", "user.name", "test", cwd=work) + (work / "README.md").write_text("# x\n") + (work / "tests" / "doc_helpers.py").write_text( + "import pathlib\nROOT = pathlib.Path(__file__).parents[1]\n" + 'DOC = ROOT / "README.md"\nTMP = "a.md"\n') + _git("add", "-A", cwd=work) + _git("commit", "-qm", "init", cwd=work) + return work + + +def _commit_test(work, body: str) -> str: + (work / "tests" / "test_doc.py").write_text(body) + _git("add", "-A", cwd=work) + _git("commit", "-qm", "add test", cwd=work) + return _git("rev-parse", "HEAD", cwd=work).stdout.strip() + + +def test_an_added_line_using_a_constant_imported_from_a_helper_hits(verify, gitfacts, repo): + """同じディレクトリの補助モジュールから import した定数も追う。 + + 補助モジュールはこの群で触っていないため、git から読む。 + """ + sha = _commit_test(repo, ( + "from doc_helpers import DOC, TMP\n\n" + "def test_doc():\n" + ' assert "x" in DOC.read_text()\n')) + facts = [{"sha": sha, "exists": True, + "test_changes": gitfacts.commit_test_changes(str(repo), sha)}] + hits = verify.doc_wording_tests(facts, ["README.md"], str(repo)) + assert hits == [("tests/test_doc.py", "README.md")] + + +def test_an_imported_constant_naming_an_untracked_file_does_not_hit(verify, gitfacts, repo): + sha = _commit_test(repo, ( + "from doc_helpers import TMP\n\n" + "def test_tmp(tmp_path):\n" + " (tmp_path / TMP).write_text('x')\n")) + facts = [{"sha": sha, "exists": True, + "test_changes": gitfacts.commit_test_changes(str(repo), sha)}] + assert verify.doc_wording_tests(facts, ["README.md"], str(repo)) == [] + + +def test_the_tracked_markdown_list_comes_from_git(gitfacts, repo): + assert gitfacts.tracked_markdown(str(repo)) == ["README.md"] + + +# ---------- 適用ラウンドの検証への配線 ---------- + +def test_the_apply_round_fails_with_the_reason(verify): + items = [{"item_id": "T1-001", "technique": "", "estimated_diff_lines": 100, + "path": "tests/test_full.py"}] + facts = [_fact({ + "tests/test_full.py": _change([], [' p = ROOT / "README.md"\n']), + })] + problem = verify.verify_apply_round(items, facts, tracked_md=TRACKED) + assert problem is not None + assert problem.startswith(REASON) + assert "tests/test_full.py: README.md" in problem + + +def test_the_apply_round_passes_a_temporary_markdown(verify): + items = [{"item_id": "T1-001", "technique": "", "estimated_diff_lines": 100, + "path": "tests/test_tmp.py"}] + facts = [_fact({ + "tests/test_tmp.py": _change([], [' p = tmp_path / "a.md"\n']), + })] + assert verify.verify_apply_round(items, facts, tracked_md=TRACKED) is None diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_drop_items_git.py b/plugins/ndf/skills/cross-refactoring/tests/test_drop_items_git.py index a2e9e12b..67695b23 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_drop_items_git.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_drop_items_git.py @@ -226,3 +226,32 @@ def built_commits_still_reachable(built) -> bool: """着手前のコミットが履歴から消えていないこと。""" log = _git("rev-list", "HEAD", cwd=built["repo"]).stdout.split() return built["c1"] in log and built["base"] in log + + +# ---------- 現状固定: dry-run と旧版の状態ファイル ---------- + +def test_dry_run_reports_the_plan_without_touching_history(gitfacts, distant_repo): + """dry-run は取り消しと積み直しの件数だけを返し、履歴も状態も変えないこと。""" + state, entry = _state(distant_repo) + head = _git("rev-parse", "HEAD", cwd=distant_repo["repo"]).stdout.strip() + + result = gitfacts.drop_items(state, entry, ["R1-001"], dry_run=True) + + assert result == {"mode": "item", "dropped": ["R1-001"], + "reverted": 2, "replayed": 1} + assert _git("rev-parse", "HEAD", cwd=distant_repo["repo"]).stdout.strip() == head + assert "drops" not in entry + assert not any(i.get("reverted") for i in state["items"]) + + +def test_legacy_state_reverts_only_the_item_commits(gitfacts, distant_repo): + """起点（`apply_base_sha`）の無い旧版では、項目のコミットだけを戻すこと。""" + state, entry = _state(distant_repo) + del entry["apply_base_sha"] + + result = gitfacts.drop_items(state, entry, ["R1-002"]) + + assert result == {"mode": "item", "dropped": ["R1-002"], + "reverted": 1, "replayed": 0} + assert "line3-by-R1-001" in _content(distant_repo) + assert "line31-by-R1-002" not in _content(distant_repo) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_final_gate.py b/plugins/ndf/skills/cross-refactoring/tests/test_final_gate.py index ee57bf85..9f859bba 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_final_gate.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_final_gate.py @@ -281,3 +281,131 @@ def test_the_apply_round_verification_never_uses_the_ci( assert spy["tests"] == ["pytest -q"], "Step 5 は手元のテストを実行する" assert spy["gh"] == [], "Step 5 で継続的統合は読まない" + + +# ---------- 全体テストは最終ゲートで 1 回（#880 の AC3） ---------- + +ROUND_TEST = {"command": "pytest tests/services -q", "status": "green", + "checked_at": "2026-08-15T00:00:00"} + + +def test_a_standalone_run_with_a_round_test_runs_the_baseline_test_once( + refactor, cmd_gate, tmp_path, env_tmp_dir, spy, capsys +): + """AC3 — 単独起動でも、範囲のテストで検証してきたなら全体テストを 1 回通す。""" + state_path = _state(tmp_path, round_test=ROUND_TEST) + env_tmp_dir(state_path) + + cmd_gate.cmd_final_gate(_args()) + + assert spy["tests"] == ["pytest -q"] + assert "FINAL_GATE=cross-review" in capsys.readouterr().out + gate = read_state(state_path)["final_gate"] + assert gate["mode"] == "cross-review" + assert gate["checks"][-1]["status"] == "pass" + + +def test_a_workflow_step_run_with_a_round_test_runs_the_baseline_test_once( + refactor, cmd_gate, tmp_path, env_tmp_dir, spy, capsys +): + """AC3 — 工程として起動したときも、全体テストの呼び出しは 1 回。""" + state_path = _state(tmp_path, workflow_step=True, round_test=ROUND_TEST) + env_tmp_dir(state_path) + + cmd_gate.cmd_final_gate(_args()) + + assert spy["tests"] == ["pytest -q"] + assert "FINAL_GATE=passed" in capsys.readouterr().out + + +def test_a_standalone_run_whose_baseline_test_fails_enters_the_fix_round( + refactor, cmd_gate, tmp_path, env_tmp_dir, spy, capsys +): + """落ちれば `--workflow-step` と同じ修正ラウンドへ入る。cross-review へは渡さない。""" + state_path = _state(tmp_path, round_test=ROUND_TEST) + env_tmp_dir(state_path) + spy["test_code"] = 1 + + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_final_gate(_args()) + + assert e.value.code == 2 + out = capsys.readouterr().out + assert "FINAL_GATE=failing" in out and "FINAL_GATE=cross-review" not in out + assert read_state(state_path)["final_gate"]["fix_rounds"] == 1 + + +def test_a_standalone_run_with_the_same_round_test_runs_no_test( + refactor, cmd_gate, tmp_path, env_tmp_dir, spy +): + """範囲のテストが全体テストと同じなら、群の検証が全体を見ている。""" + state_path = _state(tmp_path, round_test={"command": "pytest -q", "status": "green"}) + env_tmp_dir(state_path) + + cmd_gate.cmd_final_gate(_args()) + + assert spy["tests"] == [] + + +def test_a_standalone_run_with_a_ci_check_reads_the_ci_instead( + refactor, cmd_gate, tmp_path, env_tmp_dir, spy, capsys +): + """`--ci-check` があれば、手元の全体テストの代わりに継続的統合を見る。""" + state_path = _state(tmp_path, round_test=ROUND_TEST, ci_check="tests") + env_tmp_dir(state_path) + spy["gh_out"] = _check_runs(_run("tests")) + + cmd_gate.cmd_final_gate(_args()) + + assert spy["tests"] == [] + assert "FINAL_GATE=cross-review" in capsys.readouterr().out + + +def test_the_gate_check_records_the_command_and_seconds( + refactor, cmd_gate, tmp_path, env_tmp_dir, spy +): + """最終ゲートの記録は、実行したコマンドと所要の秒数を持つ。""" + state_path = _state(tmp_path, workflow_step=True, round_test=ROUND_TEST) + env_tmp_dir(state_path) + + cmd_gate.cmd_final_gate(_args()) + + check = read_state(state_path)["final_gate"]["checks"][-1] + assert check["mode"] == "test" + assert check["command"] == "pytest -q" + assert isinstance(check["seconds"], (int, float)) and check["seconds"] >= 0 + + +# ---------- 全体テストの打ち切り（run_with_timeout の timed_out=True）（R2-003） ---------- +# +# 現状固定テスト。最終ゲートの成功と非ゼロ終了は固定されているが、全体テストが +# 打ち切り（timed_out=True）で止まったときに、失敗として記録し修正ラウンドへ進む +# 経路は固定されていなかった（gate.py の `_local_gate` の timed_out 分岐）。 + + +def test_final_gate_records_a_timed_out_whole_test_and_enters_a_fix_round( + patch_lib, refactor, cmd_gate, tmp_path, env_tmp_dir, spy, capsys +): + """R2-003 — 全体テストが打ち切りなら失敗として記録し、修正ラウンドへ進む。""" + state_path = _state(tmp_path, workflow_step=True, test_timeout=60) + env_tmp_dir(state_path) + # 全体テストの実行を打ち切りへ差し替える（spy の差し替えを上書きする）。 + patch_lib("run_with_timeout", + lambda command, cwd, timeout, grace=5.0: (None, True)) + + with pytest.raises(SystemExit) as e: + cmd_gate.cmd_final_gate(_args()) + # 現状固定: 失敗の終了コード（修正ラウンドへ）。 + assert e.value.code == 2 + out = capsys.readouterr().out + assert "FINAL_GATE=failing" in out and "FINAL_GATE=cross-review" not in out + + gate = read_state(state_path)["final_gate"] + # 修正ラウンドへ進む。 + assert gate["fix_rounds"] == 1 + assert gate["status"] == "failing" + # 最終ゲートの記録は「打ち切り」相当の詳細を持つ（文言の完全一致は取らず、 + # 打ち切った秒数が含まれることだけを見る）。 + check = gate["checks"][-1] + assert check["status"] == "fail" + assert "60" in check["detail"] diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py b/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py index 6b5e65b1..eacd271d 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_git_facts.py @@ -231,6 +231,22 @@ def test_hanging_test_is_cut_off(gitfacts, work): assert _git("rev-parse", "--abbrev-ref", "HEAD", cwd=work).stdout.strip() == "main" +def test_cutting_off_returns_once_the_group_is_gone(gitfacts, work): + """SIGTERM で子が終われば、猶予を待たずに戻る（#883）。 + + 親シェルを回収しないとゾンビがグループに残り、猶予の 5 秒を毎回待つ。 + """ + import time + + started = time.monotonic() + code, timed_out = gitfacts.run_with_timeout("exec sleep 30", str(work), 1) + elapsed = time.monotonic() - started + + assert timed_out is True + assert code is None + assert elapsed < 2, f"打ち切りの後に猶予を待っている: {elapsed:.1f}s" + + def test_cutting_off_a_test_kills_its_children(gitfacts, work): """打ち切るときは**子プロセスまで**止めること。 @@ -248,7 +264,7 @@ def test_cutting_off_a_test_kills_its_children(gitfacts, work): status = gitfacts.run_test_at(str(work), sha, command, "main", timeout=1) assert status == "fail" - time.sleep(3) + time.sleep(1.5) assert not marker.exists(), "子プロセスが生き残って書き込んでいる" @@ -273,7 +289,7 @@ def test_cutting_off_kills_children_that_ignore_sigterm(gitfacts, work): assert status == "fail" assert elapsed < 20, f"打ち切りに時間がかかりすぎている: {elapsed:.1f}s" - time.sleep(4) + time.sleep(1.5) assert not marker.exists(), "SIGTERM を無視する子が生き残っている" diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_init.py b/plugins/ndf/skills/cross-refactoring/tests/test_init.py index aac848e0..561eaf1a 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_init.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_init.py @@ -747,3 +747,150 @@ def test_a_failed_rebuild_leaves_the_state_untouched(run_init, tmp_path): probe={"claude": "x", "codex": "y"}) assert e.value.code == refactor_abort() assert path.read_text(encoding="utf-8") == before + + +# ---------- 範囲のテスト `--round-test`（#880 の AC1・AC4・AC5） ---------- + +@pytest.fixture +def test_calls(patch_lib): + """`init` が実行したテストのコマンドを記録する。終了コードはコマンドごとに決める。""" + seen: list[str] = [] + codes: dict[str, int] = {} + + def fake_run(command, cwd, timeout, grace=5.0): + seen.append(command) + return codes.get(command, 0), False + + patch_lib("run_with_timeout", fake_run) + seen_codes = codes + return type("Calls", (), {"seen": seen, "codes": seen_codes})() + + +def test_the_round_test_is_parsed_and_unset_by_default(patch_lib, refactor, monkeypatch): + assert _parsed_init_args(patch_lib, refactor, monkeypatch)["round_test"] is None + captured = _parsed_init_args( + patch_lib, refactor, monkeypatch, "--round-test", "pytest tests -q") + assert captured["round_test"] == "pytest tests -q" + + +def test_init_records_the_round_test(run_init, tmp_path, test_calls): + """AC1 — `--round-test` は状態の `round_test.command` に残る。""" + run_init(_args(tmp_path, round_test="pytest -q -k scope", baseline_test="true")) + _, state = _state_of(tmp_path) + assert state["round_test"]["command"] == "pytest -q -k scope" + assert state["round_test"]["status"] == "green" + assert state["baseline_test"]["command"] == "true" + assert test_calls.seen == ["true", "pytest -q -k scope"], "全体テストの後に範囲のテストを 1 回" + + +def test_an_omitted_round_test_is_the_baseline_test_and_runs_once(run_init, tmp_path, test_calls): + """AC1 — 省けば `baseline_test` と同じコマンドで、テストの実行は 1 回。""" + run_init(_args(tmp_path, baseline_test="true")) + _, state = _state_of(tmp_path) + assert state["round_test"]["command"] == state["baseline_test"]["command"] == "true" + assert state["round_test"]["status"] == "green" + assert test_calls.seen == ["true"] + + +def test_a_round_test_equal_to_the_baseline_test_runs_once(run_init, tmp_path, test_calls): + run_init(_args(tmp_path, round_test="true", baseline_test="true")) + _, state = _state_of(tmp_path) + assert state["round_test"]["command"] == "true" + assert test_calls.seen == ["true"] + + +@pytest.mark.parametrize("command", ["false", "exit 5"]) +def test_init_stops_when_the_round_test_fails(run_init, tmp_path, command): + """AC4 — 範囲のテストが成功しなければ止める。集まらない終了コード 5 も失敗。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, round_test=command, baseline_test="true")) + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() + + +def test_init_stops_when_the_round_test_runs_outside_the_scope_tests(run_init, tmp_path, test_calls): + """AC4 — `--scope` のテストの置き場所が `--round-test` の実行集合の外なら止める。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, scope=["src", "tests"], + round_test="pytest src", baseline_test="true")) + assert e.value.code == refactor_abort() + assert test_calls.seen == [], "関門はテストの実行より先" + + +def test_init_hints_the_round_test_when_the_baseline_test_is_broad(run_init, tmp_path, capsys): + """AC5 — `--round-test` が無く全体を走らせる `--baseline-test` なら、案内して続ける。""" + run_init(_args(tmp_path, baseline_test="true")) + assert "--round-test" in capsys.readouterr().err + assert _state_path(tmp_path).exists() + + +def test_init_does_not_hint_when_the_round_test_is_given(run_init, tmp_path, capsys): + run_init(_args(tmp_path, round_test="true", baseline_test="true")) + assert "--round-test" not in capsys.readouterr().err + + +def test_resume_notifies_a_changed_round_test(run_init, tmp_path, capsys): + """再開では `--round-test` を反映せず、違えば知らせる。""" + run_init(_args(tmp_path, round_test="true")) + _, before = _state_of(tmp_path) + capsys.readouterr() + + run_init(_args(tmp_path, round_test="pytest -q")) + _, after = _state_of(tmp_path) + assert "ℹ --round-test は再開では反映しません" in capsys.readouterr().err + assert after["round_test"] == before["round_test"] + + +# ---------- 打ち切り（run_with_timeout の timed_out=True）（R2-001） ---------- +# +# 現状固定テスト。着手前のテストと範囲のテストが「打ち切り」で止まる 2 経路 +# （setup.py の `_run_baseline_test` / `_run_round_test` の timed_out=True）は +# どのテストも通していなかった。失敗（終了コード非 0）とは別の分岐なので、現状の +# 終了コードと、状態ファイルが書かれないことをそのまま記録する。 + + +@pytest.fixture +def timeout_calls(patch_lib): + """`init` のテスト実行を差し替え、コマンドごとに打ち切り（timed_out=True）へ倒せる。 + + `timed_out` に載せたコマンドだけ `(None, True)` を返す。それ以外は `(0, False)`。 + """ + seen: list[str] = [] + timed_out: set[str] = set() + + def fake_run(command, cwd, timeout, grace=5.0): + seen.append(command) + if command in timed_out: + return None, True + return 0, False + + patch_lib("run_with_timeout", fake_run) + return type("Calls", (), {"seen": seen, "timed_out": timed_out})() + + +def test_init_aborts_when_the_baseline_test_times_out(run_init, tmp_path, timeout_calls, capsys): + """R2-001 — 着手前のテストが打ち切りで止まる経路（setup.py 630-634）。""" + timeout_calls.timed_out.add("true") + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, baseline_test="true", test_timeout=60)) + # 現状固定: `die` の既定の終了コード（打ち切り）。 + assert e.value.code == refactor_abort() + # 状態ファイルは打ち切りの後の保存に届かないため書かれない。 + assert not _state_path(tmp_path).exists() + # 出力は文言の完全一致を取らず、打ち切った秒数（60）が含まれることだけを見る。 + assert "60" in capsys.readouterr().err + + +def test_init_aborts_when_the_round_test_times_out(run_init, tmp_path, timeout_calls, capsys): + """R2-001 — 範囲のテストが打ち切りで止まる経路（setup.py 662-663）。""" + # 着手前のテストは通し、範囲のテストだけ打ち切る。 + timeout_calls.timed_out.add("pytest -q -k scope") + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, round_test="pytest -q -k scope", baseline_test="true", + test_timeout=60)) + # 現状固定: 範囲のテストの打ち切りは ABORT。 + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() + # 着手前のテストを通したあと、範囲のテストで止まる順序。 + assert timeout_calls.seen == ["true", "pytest -q -k scope"] + assert "60" in capsys.readouterr().err diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_launch_agy_phases.py b/plugins/ndf/skills/cross-refactoring/tests/test_launch_agy_phases.py index 6ca8462f..d2352e50 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_launch_agy_phases.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_launch_agy_phases.py @@ -125,3 +125,55 @@ def test_the_workspace_and_stem_for_the_remaining_phases(tmp_path, phase: str) - added = [args[i + 1] for i, a in enumerate(args) if a == "--add-dir"] assert added == [str(tmp_path / workdir_name), str(state_path.parent)] assert (state_path.parent / f"{stem}-prompt.md").is_file() + + +def _run_codex(tmp_path, rounds, *args): + state_path = make_state(tmp_path, rounds=rounds) + (tmp_path / "work").mkdir(exist_ok=True) + (tmp_path / "codex").mkdir(exist_ok=True) + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + args_file = tmp_path / "args.txt" + stub = bin_dir / "codex" + stub.write_text(STUB, encoding="utf-8") + stub.chmod(0o755) + result = subprocess.run( + [str(LAUNCH), *args], + env={ + **os.environ, + "CROSS_REFACTORING_TMP_DIR": str(state_path.parent), + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "NDF_TEST_ARGS_FILE": str(args_file), + }, + capture_output=True, + text=True, + ) + return result, state_path, args_file + + +def test_apply_without_a_round_stops_when_no_round_exists(tmp_path): + result, state_path, args_file = _run_codex(tmp_path, [], "codex", "apply", "130") + assert result.returncode != 0 + assert not args_file.exists() + assert not (state_path.parent / "codex-apply-r0-prompt.md").exists() + + +def test_apply_without_a_round_uses_the_latest_round(tmp_path): + result, state_path, args_file = _run_codex( + tmp_path, [{"round": 1}, {"round": 2}], "codex", "apply", "130" + ) + assert result.returncode == 0 + for _ in range(200): + if args_file.is_file(): + break + time.sleep(0.05) + assert args_file.exists() + assert (state_path.parent / "codex-apply-r2-prompt.md").is_file() + + +def test_unknown_runtime_stops_before_writing_a_prompt(tmp_path): + result, state_path, _ = _run_codex( + tmp_path, [{"round": 1}], "unknown", "propose", "130", "1" + ) + assert result.returncode != 0 + assert not (state_path.parent / "unknown-propose-rf130-r1-prompt.md").exists() diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_launch_round_test.py b/plugins/ndf/skills/cross-refactoring/tests/test_launch_round_test.py new file mode 100644 index 00000000..0f72535e --- /dev/null +++ b/plugins/ndf/skills/cross-refactoring/tests/test_launch_round_test.py @@ -0,0 +1,69 @@ +"""担当へ渡すテストコマンドが、進行側の検証と同じであること（#880）。 + +進行側は群と修正コミットを `round_test_command(state)`（`round_test`、無ければ +`baseline_test`）で検証する。適用と修正の担当が別のコマンドを走らせると、担当が +通したつもりの変更を進行側が落とす（またはその逆）。最終ゲートの修正は全体の +テストで判定するため、`baseline_test` のままである。 +""" +from __future__ import annotations + +import os +import pathlib +import subprocess + +import pytest + +from crossref_helpers import make_state + +LAUNCH = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "launch-cli.sh" +BASELINE = "pytest -q whole-suite" +ROUND_TEST = "pytest -q tests/unit/test_scope_only.py" + + +def _prompt(tmp_path, phase, round_test): + over = {"baseline_test": {"command": BASELINE, "status": "green"}, + "rounds": [{"round": 1, "apply_round": 1}], + "items": [{"item_id": "R1-001", "round": 1, "apply_round": 1}]} + if round_test is not None: + over["round_test"] = {"command": round_test} + state_path = make_state(tmp_path, **over) + (tmp_path / "work").mkdir(exist_ok=True) + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + stub = bin_dir / "codex" + stub.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + stub.chmod(0o755) + args = [str(LAUNCH), "codex", phase, "130"] + if phase != "final-fix": + args.append("1") + subprocess.run( + args, + env={**os.environ, + "CROSS_REFACTORING_TMP_DIR": str(state_path.parent), + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}"}, + check=True, capture_output=True, text=True, + ) + name = "codex-final-fix" if phase == "final-fix" else f"codex-{phase}-r1" + return (state_path.parent / f"{name}-prompt.md").read_text(encoding="utf-8") + + +@pytest.mark.parametrize("phase", ["apply", "fix"]) +def test_apply_and_fix_run_the_round_test(tmp_path, phase): + text = _prompt(tmp_path, phase, ROUND_TEST) + assert ROUND_TEST in text + assert BASELINE not in text + assert "$RF_" not in text + + +@pytest.mark.parametrize("phase", ["apply", "fix"]) +def test_apply_and_fix_fall_back_to_the_baseline(tmp_path, phase): + """`round_test` を持たない状態ファイルは、進行側の検証と同じく baseline になる。""" + text = _prompt(tmp_path, phase, None) + assert BASELINE in text + assert "$RF_" not in text + + +def test_final_fix_runs_the_whole_suite(tmp_path): + text = _prompt(tmp_path, "final-fix", ROUND_TEST) + assert BASELINE in text + assert ROUND_TEST not in text diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_merge_proposals.py b/plugins/ndf/skills/cross-refactoring/tests/test_merge_proposals.py index 008ab1ed..7d77e5ae 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_merge_proposals.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_merge_proposals.py @@ -245,6 +245,30 @@ def test_non_object_proposal_result_is_treated_as_empty( assert len(state["items"]) == 1 +def test_invalid_json_proposal_result_is_treated_as_empty( + refactor, cmd_apply, tmp_path, env_tmp_dir, no_git +): + """壊れた JSON の 1 者を提案なしとして、残りの提案を統合し続ける。""" + state_path = make_state(tmp_path, rounds=[{ + "round": 1, "impl": "codex", "reviewers": ["agy", "kiro"], + "impl_model": {"requested": None, "observed": None}, + "reviewer_models": {}, "proposed": {}, "items": [], + "apply": {"applied": [], "failed": []}, "fix_rounds": 0, + "durations": {}, "reviews": [], + }]) + env_tmp_dir(state_path) + broken = state_path.parent / "codex-propose-rf130-r1-result.json" + broken.write_text("{not valid json", encoding="utf-8") + write_result(state_path, "agy-propose-rf130-r1", {"items": [proposal()]}) + write_result(state_path, "kiro-propose-rf130-r1", {"items": [proposal()]}) + + cmd_apply.cmd_merge_proposals(type("A", (), {"id": 130})()) + + state = read_state(state_path) + assert state["rounds"][0]["proposed"].get("codex", 0) == 0 + assert len(state["items"]) == 1 + + def test_merge_proposals_is_idempotent(cmd_apply, tmp_path, env_tmp_dir, no_git): """同じラウンドで叩き直しても項目を二重に作らないこと。 diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_models_and_metrics.py b/plugins/ndf/skills/cross-refactoring/tests/test_models_and_metrics.py index 4a20f613..8477f49f 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_models_and_metrics.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_models_and_metrics.py @@ -347,3 +347,34 @@ def test_models_are_fixed_across_rounds(cmd_setup, tmp_path, env_tmp_dir): # 次のラウンドを開けるように、いま開いたラウンドを閉じる entry["adopted"] = 1 state_path.write_text(json.dumps(state, ensure_ascii=False), encoding="utf-8") + + +# ---------- 実測モデルを状態へ反映する正常経路（R2-005） ---------- +# +# 現状固定テスト。モデル文字列の解析（observed_model）と不一致判定 +# （mismatch_warning）は個別に固定されているが、CLI の stdout ログから得た +# 実測モデルを、適用ラウンドの状態へ反映する正常経路（gitfacts.record_observed_model） +# はどのテストも通していなかった。 + + +def test_record_observed_model_saves_the_observed_value( + gitfacts, tmp_path +): + """R2-005 — stdout ログの実測モデルを impl_model.observed へ保存する。""" + tmp_dir = tmp_path / "tmp" + tmp_dir.mkdir() + # 適用フェーズ・ラウンド 1 の骨格は `claude-apply-r1`。 + (tmp_dir / "claude-apply-r1-stdout.log").write_text( + json.dumps({ + "type": "result", "is_error": False, + "modelUsage": {"claude-opus-5": {"inputTokens": 100}}, + }), + encoding="utf-8", + ) + state = {"id": 130, "tmp_dir": str(tmp_dir)} + entry = {"impl_model": {"requested": "claude-opus-5", "observed": None}} + + gitfacts.record_observed_model(entry, "claude", state, "apply", 1) + + # 現状固定: stdout ログから拾った実測値が保存される。 + assert entry["impl_model"]["observed"] == "claude-opus-5" diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_paths.py b/plugins/ndf/skills/cross-refactoring/tests/test_paths.py index 3d2bc91a..8dec273e 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_paths.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_paths.py @@ -11,6 +11,8 @@ import pathlib import tempfile +import pytest + STATE_ID = 130 @@ -85,6 +87,24 @@ def test_it_uses_the_cwd_when_the_env_var_is_unset(paths, tmp_path, monkeypatch) assert state["phase"] == "from-cwd" +def test_load_state_exits_4_when_the_target_is_missing( + paths, tmp_path, monkeypatch, capsys +): + """現状固定: 指定先と現在地のどちらにも対象 ID が無ければ停止する。""" + env_dir = tmp_path / "env" + env_dir.mkdir() + cwd = tmp_path / "cwd" + cwd.mkdir() + monkeypatch.setenv("CROSS_REFACTORING_TMP_DIR", str(env_dir)) + monkeypatch.chdir(cwd) + + with pytest.raises(SystemExit) as exc: + paths.load_state(STATE_ID) + + assert exc.value.code == 4 + assert str(STATE_ID) in capsys.readouterr().err + + def test_the_explicit_worktree_base_is_resolved(paths, tmp_path, monkeypatch): """現状固定: 明示した作業ディレクトリの親を絶対パスへ解決する。""" monkeypatch.chdir(tmp_path) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py b/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py index d6adf114..c570b91b 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py @@ -238,3 +238,132 @@ def test_report_says_no_record_for_an_older_state(cmd_report, tmp_path, env_tmp_ env_tmp_dir(state_path) cmd_report.cmd_report(_report_args()) assert "- 使える者: 記録なし" in capsys.readouterr().out + + +# ---------- 報告の未到達分岐の固定（R2-004） ---------- +# +# 現状固定テスト。完了報告の既存テストが通っていなかった分岐を固定する。 +# 行全体の完全一致は避け、分岐の判定に関わる値の部分だけを比較する。 + + +def _participants(**over): + """参加者の記録を組み立てるヘルパ。""" + base = { + "pool": ["claude", "codex", "kiro"], "included": [], + "excluded": [], "available": ["claude", "codex", "kiro"], + "unavailable": {}, "probe_skipped": False, "require_all": False, + } + base.update(over) + return base + + +@pytest.mark.parametrize("probe_skipped, expected_text", [ + (True, "確認を飛ばした（NDF_SKIP_AUTH_CHECK）"), + (False, "確認を通らなかった者: なし"), +]) +def test_report_probe_skipped_vs_no_unavailable( + cmd_report, tmp_path, env_tmp_dir, capsys, probe_skipped, expected_text +): + """R2-004(a)(b) — unavailable が空のとき probe_skipped で出力が変わる。""" + state_path = make_state( + tmp_path, + participants=_participants(probe_skipped=probe_skipped), + resume_changes=[], + ) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + assert expected_text in capsys.readouterr().out + + +def test_report_empty_resume_changes_says_none( + cmd_report, tmp_path, env_tmp_dir, capsys +): + """R2-004(c) — 再開で変えた値が空なら「なし」と出る。""" + state_path = make_state( + tmp_path, + participants=_participants(), + resume_changes=[], + ) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + assert "再開で変えた値: なし" in capsys.readouterr().out + + +def test_report_resume_change_with_participants_shows_available( + cmd_report, tmp_path, env_tmp_dir, capsys +): + """R2-004(d) — 再開で変えた値に参加者（available を持つ dict）があれば、使える者だけを出す。""" + state_path = make_state( + tmp_path, + participants=_participants(), + resume_changes=[{ + "at": "2026-09-22T00:00:00", "field": "participants", + "from": {"available": ["claude", "codex"]}, + "to": {"available": ["claude", "codex", "kiro"]}, + }], + ) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + out = capsys.readouterr().out + assert "claude / codex → claude / codex / kiro" in out + + +def test_report_shows_test_rounds_final_when_present( + cmd_report, tmp_path, env_tmp_dir, capsys +): + """R2-004(e) — test_rounds_final があれば「テスト整備の終わり方」を出す。""" + state_path = make_state( + tmp_path, + test_rounds_final="max_test_rounds", + participants=_participants(), + ) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + out = capsys.readouterr().out + assert "テスト整備の終わり方" in out + assert "max_test_rounds" in out + + +def test_report_item_table_shows_case_and_level_for_test_items( + cmd_report, tmp_path, env_tmp_dir, capsys +): + """R2-004(f) — テスト整備の項目（kind=test）は case と level を項目表に出す。""" + test_item = { + "item_id": "R2-001", "round": 1, "kind": "test", + "path": "tests/test_init.py", + "target": "scripts/refactor_lib/commands/setup.py#cmd_init", + "case": "error", "level": "unit", + "rationale": "", "plan": "", "test_gap": False, + "estimated_diff_lines": 0, "proposed_by": ["claude"], + "status": "applied", "commits": ["abc"], + } + structure_item = { + "item_id": "R1-001", "round": 1, + "path": "src/foo.py", "symbol": "Foo.handle", + "smell": "long_method", "technique": "extract_method", "severity": "major", + "rationale": "", "plan": "", "test_gap": False, + "estimated_diff_lines": 10, "proposed_by": ["codex"], + "status": "done", "commits": ["def"], + } + state_path = make_state( + tmp_path, + items=[structure_item, test_item], + rounds=[{ + "round": 1, "kind": "structure", "impl": "codex", + "impl_model": {"requested": None, "observed": None}, + "adopted": 2, + "apply": {"applied": ["R1-001", "R2-001"], "failed": []}, + "fix_rounds": 0, "reviews": [], + }], + ) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + lines = capsys.readouterr().out.splitlines() + # テスト項目の行で case と level が出ること + test_row = next(line for line in lines if "R2-001" in line) + assert "error" in test_row + assert "unit" in test_row + # 構造改善項目の行は smell と technique が出ること（既存動作の確認） + struct_row = next(line for line in lines if "R1-001" in line) + assert "long_method" in struct_row + assert "extract_method" in struct_row diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_scope_gate.py b/plugins/ndf/skills/cross-refactoring/tests/test_scope_gate.py index d0e0fb17..d0d0a8be 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_scope_gate.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_scope_gate.py @@ -179,3 +179,143 @@ def test_the_matched_location_is_checked_against_the_search_roots(scope, tmp_pat ["skills/development-workflow"], "pytest other", str(tmp_path)) assert problem is not None assert "skills/development-workflow/tests" in problem + + +# ---------- `--round-test` の実行集合（#880 の AC4） ---------- + +def _services(tmp_path): + (tmp_path / "tests" / "services").mkdir(parents=True) + (tmp_path / "tests" / "services" / "test_one.py").write_text("", encoding="utf-8") + (tmp_path / "scripts").mkdir() + (tmp_path / "scripts" / "run-scope-tests.sh").write_text("", encoding="utf-8") + (tmp_path / "scripts" / "run_tests.py").write_text("", encoding="utf-8") + + +def test_a_test_file_is_a_round_test_root(scope, tmp_path): + _services(tmp_path) + assert scope.round_test_roots( + "pytest tests/services/test_one.py", str(tmp_path)) == [ + "tests/services/test_one.py"] + + +@pytest.mark.parametrize("command", [ + "pytest tests/services/test_one.py::test_a", + "pytest tests/services/test_one.py::TestA::test_b -q", +]) +def test_a_node_id_counts_its_file_as_a_round_test_root(scope, tmp_path, command): + """ノード ID はファイルの部分を起点に数える。数えないと起点が空になり、全体を覆うとみなす。""" + _services(tmp_path) + assert scope.round_test_roots(command, str(tmp_path)) == [ + "tests/services/test_one.py"] + + +def test_a_node_id_narrower_than_the_scope_stops(refactor_lib, scope, tmp_path): + """範囲のテストの置き場所の一部しか走らせないノード ID は関門で止める。""" + _services(tmp_path) + problem = scope.scope_problem( + ["src", "tests/services"], "pytest tests/services/test_one.py::test_a", + str(tmp_path), round_test=True) + assert problem is not None and "--round-test" in problem + + +def test_an_option_value_and_the_work_root_are_not_round_test_roots(scope, tmp_path): + """`--project .` の `.` はオプションの値で、作業ディレクトリの根でもある。""" + _services(tmp_path) + assert scope.round_test_roots( + "uv run --project . pytest tests/services/test_one.py", str(tmp_path)) == [ + "tests/services/test_one.py"] + assert scope.round_test_roots("pytest . -q", str(tmp_path)) == [] + + +@pytest.mark.parametrize("command", [ + "bash scripts/run-scope-tests.sh", "python scripts/run_tests.py", +]) +def test_a_wrapper_script_is_not_a_round_test_root(scope, tmp_path, command): + _services(tmp_path) + assert scope.round_test_roots(command, str(tmp_path)) == [] + + +@pytest.mark.parametrize("command, expected", [ + ("pytest --verbose tests/services", ["tests/services"]), + ("pytest -q --tb short tests/services/test_one.py", ["tests/services/test_one.py"]), + ("uv run --project scripts --with pytest pytest --lf tests/services", ["tests/services"]), + ("pytest --rootdir scripts -p no:cacheprovider tests/services", ["tests/services"]), +]) +def test_a_flag_without_a_value_keeps_the_next_round_test_root(scope, tmp_path, command, expected): + """値を取らないオプションの直後の対象を、オプションの値として消さない。 + + 消すと起点が空になり、全体を覆うとみなして範囲の外だけを走らせるコマンドが関門を通る。 + 値を取ると分かっているオプション（`--project` / `--rootdir` / `-p`）の直後だけを除く。 + """ + _services(tmp_path) + assert scope.round_test_roots(command, str(tmp_path)) == expected + + +def test_a_flag_before_an_outside_target_stops(refactor_lib, scope, tmp_path): + """真偽のオプションの直後の範囲外の対象だけを走らせるコマンドは止める。""" + _services(tmp_path) + problem = scope.scope_problem( + ["src", "tests/services"], "pytest --verbose scripts", str(tmp_path), + round_test=True) + assert problem is not None and "--round-test" in problem + + +def test_a_directory_is_a_round_test_root(scope, tmp_path): + _services(tmp_path) + assert scope.round_test_roots( + "uv run --with pytest pytest tests/services -q", str(tmp_path)) == [ + "tests/services"] + + +@pytest.mark.parametrize("command", [ + "pytest tests/services/test_one.py", + "uv run --project . pytest tests/services/test_one.py", + "pytest scripts", +]) +def test_a_round_test_narrower_than_the_scope_tests_stops(refactor_lib, scope, tmp_path, command): + """AC4 — 置き場所それぞれについて、起点のどれかが同じか祖先でなければ止める。""" + _services(tmp_path) + problem = scope.scope_problem( + ["src", "tests/services"], command, str(tmp_path), round_test=True) + assert problem is not None and "--round-test" in problem + with pytest.raises(SystemExit) as e: + scope.require_scope_covers_tests( + ["src", "tests/services"], command, str(tmp_path), round_test=True) + assert e.value.code == refactor_lib.ABORT + + +@pytest.mark.parametrize("command", [ + "bash scripts/run-scope-tests.sh", + "pytest -q", + "pytest tests/services", + "pytest tests", +]) +def test_a_round_test_covering_the_scope_tests_passes(scope, tmp_path, command): + """起点の無いコマンドは全体を覆うとみなす。""" + _services(tmp_path) + assert scope.scope_problem( + ["src", "tests/services"], command, str(tmp_path), round_test=True) is None + + +# ---------- `--round-test` の案内（#880 の AC5） ---------- + +@pytest.mark.parametrize("baseline, expected", [ + ("pytest -q", True), + ("pytest tests", True), + ("pytest tests/services", False), +]) +def test_the_round_test_hint(scope, tmp_path, baseline, expected): + """起点が無いか範囲より広いときだけ、1 行の案内を返す。""" + _services(tmp_path) + hint = scope.round_test_hint( + None, baseline, ["src", "tests/services"], str(tmp_path)) + assert (hint is not None) is expected + if expected: + assert "--round-test" in hint and "\n" not in hint + + +def test_no_round_test_hint_when_the_round_test_is_given(scope, tmp_path): + _services(tmp_path) + assert scope.round_test_hint( + "pytest tests/services", "pytest -q", ["src", "tests/services"], + str(tmp_path)) is None diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_test_rounds.py b/plugins/ndf/skills/cross-refactoring/tests/test_test_rounds.py index 7498ec9a..64962dfd 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_test_rounds.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_test_rounds.py @@ -93,6 +93,27 @@ def test_a_proposal_without_a_target_is_dropped(refactor): assert adopted == [] and deferred == [] +# ---------- 文書を対象にした提案は採らない（#723） ---------- + +def test_a_proposal_targeting_a_markdown_section_is_deferred(refactor): + """`target` の `#` より前が `.md` の提案を、理由付きで見送る。 + + 文書の振る舞いは文言そのものであり、固定すると後の整理が取り消される。 + """ + adopted, deferred = _merge(refactor, { + "codex": [tprop(target="plugins/ndf/skills/x/SKILL.md#手順")], + }) + assert adopted == [] + assert deferred[0]["target"] == "plugins/ndf/skills/x/SKILL.md#手順" + assert deferred[0]["defer_reason"] == "文書の文言を固定するテストは足さない" + + +def test_a_proposal_whose_symbol_mentions_markdown_is_still_adopted(refactor): + """見るのは `#` より前のファイルだけ。シンボル側の `.md` は見ない。""" + adopted, _ = _merge(refactor, {"codex": [tprop(target="src/doc.py#render.md")]}) + assert len(adopted) == 1 + + def test_the_vocabulary_comes_from_the_existing_references(vocabulary): """`case` は現状固定テストの表、`level` はテストの階層から採る（決定 9）。""" assert list(vocabulary.TEST_CASES) == ["normal", "branch", "boundary", "error"] diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_verify_round.py b/plugins/ndf/skills/cross-refactoring/tests/test_verify_round.py index 23b9ee29..b1580f18 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_verify_round.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_verify_round.py @@ -240,3 +240,50 @@ def test_a_failing_verification_still_names_the_plan(patch_lib, refactor, cmd_co cmd_converge.cmd_verify_round(_args()) assert url in capsys.readouterr().err + + +# ---------- 範囲のテスト `--round-test`（#880 の AC2・AC6・AC7） ---------- + +ROUND_TEST = {"command": "pytest tests/services -q", "status": "green", + "checked_at": "2026-08-15T00:00:00"} + + +def test_the_round_is_verified_with_the_round_test(patch_lib, refactor, cmd_converge, tmp_path, env_tmp_dir, monkeypatch): + """AC2 — 群の検証は `round_test` だけを実行する。全体テストは実行しない。""" + state_path = _state(tmp_path, round_test=ROUND_TEST) + env_tmp_dir(state_path) + seen = _test_run(patch_lib, refactor, monkeypatch, code=0) + + cmd_converge.cmd_verify_round(_args()) + + assert [c for c, _, _ in seen] == ["pytest tests/services -q"] + record = read_state(state_path)["rounds"][0]["verifications"][-1] + assert record["command"] == "pytest tests/services -q" + + +@pytest.mark.parametrize("code", [0, 1]) +def test_the_verification_records_its_seconds(patch_lib, refactor, cmd_converge, tmp_path, env_tmp_dir, monkeypatch, code): + """AC6 — 検証の記録は所要の秒数を持つ。""" + state_path = _state(tmp_path, round_test=ROUND_TEST) + env_tmp_dir(state_path) + _test_run(patch_lib, refactor, monkeypatch, code=code) + + try: + cmd_converge.cmd_verify_round(_args()) + except SystemExit: + pass + + seconds = read_state(state_path)["rounds"][0]["verifications"][-1]["seconds"] + assert isinstance(seconds, (int, float)) and seconds >= 0 + + +def test_a_state_without_the_round_test_verifies_with_the_baseline_test(patch_lib, refactor, cmd_converge, tmp_path, env_tmp_dir, monkeypatch): + """AC7 — 変更の前の状態ファイルを再開したときは、`baseline_test` で検証する。""" + state_path = _state(tmp_path) + assert "round_test" not in read_state(state_path) + env_tmp_dir(state_path) + seen = _test_run(patch_lib, refactor, monkeypatch, code=0) + + cmd_converge.cmd_verify_round(_args()) + + assert [c for c, _, _ in seen] == ["pytest -q"] diff --git a/plugins/ndf/skills/development-workflow/references/stage-notes.md b/plugins/ndf/skills/development-workflow/references/stage-notes.md index eba4d957..fe123459 100644 --- a/plugins/ndf/skills/development-workflow/references/stage-notes.md +++ b/plugins/ndf/skills/development-workflow/references/stage-notes.md @@ -110,6 +110,15 @@ 退避先は [workflow-modes.md](workflow-modes.md)「構造改善の退避先」にある。 +**工程に入ったら、次の順で進める。** 全体テストを群ごとに回すと、1 回の実行が長くなり +構造改善を通す費用が見合わなくなる。 + +| 順 | 手 | +| ---: | --- | +| 1 | `refactor.py assess --base <起点のブランチ>` を実行する。終了コード 3 なら飛ばす（記録の残し方は「構造改善の退避先」） | +| 2 | `--round-test` に **`--scope` のテストの置き場所だけを走らせるコマンド**を渡す（例: `uv run --with pytest pytest <範囲のテストの置き場所> -q`）。群の検証と修正のコミットの検証はこれで走る | +| 3 | `--baseline-test` に**全体テスト**を渡す。全体テストは `init` と最終ゲートで 1 回ずつ走る。`quality-gates` の全体テストと重ねて回さない | + **この工程で `cross-refactoring` を通したら、次の「実装レビュー」を重ねて呼ばない。** その Skill は 最後に Pull Request 全体を `cross-review` にかけるため、実装レビューの工程が求めるものをそこで 満たす。構造改善の後に実装の差分が増えた場合だけ、改めて呼ぶ。 diff --git a/plugins/ndf/skills/development-workflow/references/workflow-modes.md b/plugins/ndf/skills/development-workflow/references/workflow-modes.md index d00ffcef..61f88895 100644 --- a/plugins/ndf/skills/development-workflow/references/workflow-modes.md +++ b/plugins/ndf/skills/development-workflow/references/workflow-modes.md @@ -194,8 +194,35 @@ ## 構造改善の退避先 工程表は `standard` と `legacy-refactor` の構造改善に `cross-refactoring` を置く。 -`light` と `operation` は工程そのものが対象外である。 -**次のいずれかに当たるときは `refactoring` 単独で進める。** +`light` と `operation` は工程そのものが対象外である。**工程に入ったら、最初に +`refactor.py assess` で飛ばしてよいかを見る。** 飛ばす条件と、`refactoring` 単独へ退避する +条件は別である。 + +```bash +python3 "$RF/refactor.py" assess --base origin/develop; rc=$?; echo "exit=$rc" +``` + +`$RF` は `cross-refactoring` の `scripts/` の絶対パスである。 + +**飛ばす。** 次に当たるときは何も起動しない。 + +| 条件 | 理由 | +| --- | --- | +| `assess` が終了コード 3（本番コードの差分が無い、または変更が 10 行以下） | 整える対象が無いか、提案の費用が見合わない | + +`assess` が終了コード 2（引数の誤り・起点を解けない）を返したときは、値を直して打ち直す。 +**判定できないことを、飛ばしてよいと読まない。** + +飛ばしたときは次の 3 つを残す。**工程の記録は通した場合と同じ値にする。** 区別は Pull Request の +本文の 1 行が持つ。記録を打たずに進めると「記録なし」のまま残り、飛ばした場合と見分けられる。 + +| 記録先 | 何を残すか | +| --- | --- | +| 進行の記録 | 工程「構造改善」（`projects-sync.sh <課題番号> stage "構造改善"`） | +| Pull Request の本文 | `構造改善: 飛ばした（）` の 1 行 | +| 持ち場の報告 | 同じ 1 行を `理由` に含める | + +**`refactoring` 単独へ退避する。** `assess` が終了コード 0 で、次のいずれかに当たるとき。 | 条件 | 理由 | | --- | --- |