From bee79b43568dd2f871dc27dadcb77989d7348767 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 09:41:56 +0000 Subject: [PATCH 1/9] =?UTF-8?q?Update:=20cross-refactoring=20=E3=81=AE?= =?UTF-8?q?=E5=8F=82=E5=8A=A0=E8=80=85=E3=82=92=E5=85=B1=E9=80=9A=E5=B1=A4?= =?UTF-8?q?=E3=81=A7=E6=B1=BA=E3=82=81=E3=80=81=E9=81=A9=E7=94=A8=E3=81=AE?= =?UTF-8?q?=E8=BC=AA=E7=95=AA=E3=82=92=E5=8F=82=E5=8A=A0=E8=80=85=E3=81=AE?= =?UTF-8?q?=E4=B8=AD=E3=81=A7=E5=9B=9E=E3=81=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 初期化は母集合の既定（codex / kiro とホスト）と使える者の解決を止めない確認で呼び、 確認を通らない者を外して続ける。足す者・外す者・全員を要する指定の引数を足す - 適用専用の母集合（状態の項目と初期化の出力）とラウンドのレビュー担当を消し、 適用の輪番を参加者の一覧から決める - 再開で上限を反映し、状態に載る他の引数は違えば知らせる。担当に関わる引数を 渡した再開でだけ参加者を作り直す - 報告と改修計画の表示を 1 つの母集合に揃え、参加者の節を足す - 呼び手の無くなった旧関数 4 つを共通層から消し、残らないことをテストで固定する Refs #664 #727 Co-Authored-By: Claude Opus 5 (1M context) --- plugins/ndf/scripts/lib/assignment.py | 92 +--- plugins/ndf/scripts/lib/auth.py | 34 +- plugins/ndf/scripts/tests/test_auth_probe.py | 49 --- .../ndf/scripts/tests/test_lib_assignment.py | 89 +--- .../cross-refactoring/scripts/refactor.py | 50 ++- .../scripts/refactor_lib/commands/apply.py | 4 +- .../scripts/refactor_lib/commands/report.py | 75 +++- .../scripts/refactor_lib/commands/setup.py | 259 ++++++++--- .../scripts/refactor_lib/gitfacts.py | 13 +- .../scripts/refactor_lib/plan.py | 4 +- .../scripts/refactor_lib/rounds.py | 9 +- .../tests/crossref_helpers.py | 1 - .../tests/test_apply_attempts.py | 10 +- .../tests/test_assignment.py | 149 ++----- .../cross-refactoring/tests/test_init.py | 415 ++++++++++-------- .../tests/test_models_and_metrics.py | 3 +- .../tests/test_plan_comment.py | 8 + .../cross-refactoring/tests/test_rounds.py | 79 +++- .../tests/test_start_round_emits_runtimes.py | 32 +- .../tests/test_rejected_findings.py | 3 +- .../tests/test_state_review_pool.py | 5 +- scripts/tests/test_shared_lib_layout.py | 21 + 22 files changed, 742 insertions(+), 662 deletions(-) diff --git a/plugins/ndf/scripts/lib/assignment.py b/plugins/ndf/scripts/lib/assignment.py index a01928c44..42c7b29c1 100644 --- a/plugins/ndf/scripts/lib/assignment.py +++ b/plugins/ndf/scripts/lib/assignment.py @@ -1,29 +1,17 @@ """ホスト判定と担当の決定（収束ループ共通層）。 -**役割ごとに母集合が違う**ことがこの層の要点である。 +**母集合の既定は Skill ごとに違う**ことがこの層の要点である。 -| 母集合 | 定義 | 中身 | +| Skill | 母集合の既定 | 中身 | | --- | --- | --- | -| 提案・レビュー | 全ランタイム − ホスト | 常に 3 者 | -| 適用 | 全ランタイム | 常に 4 者 | - -**担当の選び方は、適用の役があるかどうかで分かれる。** `assign()` は実装担当を先に決めて -から残りを絞り、`review_assign()` は母集合から直接 2 者を選ぶ。どちらも返すレビュー担当は -2 者である。 - -参加する 4 者はいずれも NDF の配布先であるため、**適用から外す者はいない**。 -ホストは提案・レビューから外れるが適用には入るため、2 つの母集合は重なるが -一致しない。輪番の式はホストによらず同じ形になる。 - -## 使える者の解決と席の埋め方（#727） +| cross-review | `review_pool(host)` | 全ランタイム − ホスト | +| cross-refactoring | `refactor_pool(host)` | `DEFAULT_REFACTOR_RUNTIMES`（codex / kiro）とホスト | 参加者は「母集合の既定 ∪ 足す者 − 外す者」で決め（`resolve_participants`）、確認を -通った者だけを使える者（`available`）として記録する。cross-refactoring の母集合の -既定は `refactor_pool(host)`（`DEFAULT_REFACTOR_RUNTIMES` とホスト）、cross-review は -`review_pool(host)` のまま。担当の単位は席の名前（`SEAT_PATTERN`。`claude-2` のように -同じランタイムの 2 つ目を表す）で、cross-review の 2 席は `review_seats` が、 -cross-refactoring の適用担当は `impl_assign` が決める。上の表と `impl_pool` / -`review_assign` / `assign` は、母集合が 1 つになる次の Pull Request（P7）まで残す。 +通った者だけを使える者（`available`）として記録する。担当の単位は席の名前 +（`SEAT_PATTERN`。`claude-2` のように同じランタイムの 2 つ目を表す）で、cross-review の +2 席は `review_seats` が、cross-refactoring の適用担当は `impl_assign` が決める（#727）。 +cross-refactoring は提案と適用を同じ参加者で回し、レビュー担当を持たない。 """ from __future__ import annotations @@ -73,7 +61,7 @@ def detect_host( """ホストを確定し、`(ホスト名, 判定根拠)` を返す。 判定根拠は `explicit`（`--host` の明示指定）か `env`（環境変数からの推定）。 - 誤検出すると**提案・レビューの母集合が狂う**（ホストが提案側に混ざる、 + 誤検出すると**母集合の既定が狂う**（ホストが cross-review の担当に混ざる、 参加すべき者が外れる）ため、呼び出し側は結果を必ず出力と状態ファイルへ残す。 推定できないときは例外を上げる。既定値を勝手に置くと、間違ったまま一周して @@ -97,67 +85,12 @@ def detect_host( def review_pool(host: str) -> list[str]: - """提案・レビューの母集合（全ランタイム − ホスト）。常に 3 者になる。""" + """cross-review の母集合の既定（全ランタイム − ホスト）。常に 3 者になる。""" if host not in HOST_RUNTIMES: raise AssignmentError(f"ホストになれないランタイムです: {host}") return [r for r in ALL_RUNTIMES if r != host] -def impl_pool() -> list[str]: - """適用の母集合（全ランタイム）。ホストによらず常に同じ。 - - **関数として残す。** 呼び出し側が提案・レビューの母集合と適用の母集合を - 別々に確定する構造を保つためである。両者は依然として一致しない - （適用はホストを含み、提案・レビューは含まない）。 - """ - return list(ALL_RUNTIMES) - - -def review_assign(round_no: int, host: str) -> list[str]: - """ラウンド番号から**レビュー担当 2 者**を決める。適用の役を持たない工程が使う。 - - 母集合は `review_pool(host)` の 3 者で、外す 1 者をラウンドごとに回す。 - - レビュー担当 = 母集合 − 母集合[(ラウンド番号 - 1) % 3] - - `assign()` と分けているのは、**適用の役があるかどうかで選び方が変わる**ためである。 - `assign()` は先に実装担当を決めてから残りを絞るが、この工程には適用が無く、母集合から - 直接 2 者を選ぶ。3 者すべてを毎ラウンド起動しないのは、起動回数が 1.5 倍になるためで、 - ラウンドを重ねれば 3 者とも差分を見る。 - """ - if round_no < 1: - raise AssignmentError(f"ラウンド番号は 1 以上です: {round_no}") - pool = review_pool(host) - dropped = (round_no - 1) % len(pool) - return [r for i, r in enumerate(pool) if i != dropped] - - -def assign(round_no: int, host: str) -> tuple[str, list[str]]: - """ラウンド番号から `(実装担当, レビュー担当 2 者)` を決める。 - - 輪番の単位は**ラウンド**である。1 ラウンドの適用を 1 者へ集約することで、 - レビュー担当を「実装担当以外」から機械的に決められる。 - - 実装担当 = 適用候補[ラウンド番号 % 4] - 候補 = 提案・レビュー − 実装担当 - レビュー担当 = 候補が 2 者ならそのまま - 3 者なら 候補[(ラウンド番号 // 4) % 3] を除いた 2 者 - - 実装担当がホストと同じランタイムのとき、その者は提案・レビューの母集合に - 含まれないため候補が 3 者残る。**レビュー担当は常に 2 者**とし（起動回数を - 抑える方針と揃える）、余る 1 者はラウンドを跨いで順に外して負荷を均す。 - """ - if round_no < 1: - raise AssignmentError(f"ラウンド番号は 1 以上です: {round_no}") - pool = impl_pool() - impl = pool[round_no % len(pool)] - candidates = [r for r in review_pool(host) if r != impl] - if len(candidates) > 2: - dropped = (round_no // len(pool)) % len(candidates) - candidates = [r for i, r in enumerate(candidates) if i != dropped] - return impl, candidates - - def _in_fixed_order(names: Iterable[str]) -> list[str]: """`ALL_RUNTIMES` の順に並べ直す（重複は 1 つにする）。""" wanted = set(names) @@ -323,7 +256,7 @@ def review_seats(round_no: int, available: list[str], fallback: list[str]) -> li | 0 | `fallback[0]` と `-2`。`fallback` が空なら `AssignmentError` | `available` の並びは `ALL_RUNTIMES` の順（`resolve_participants` が保つ）。n = 3 の値は - 変更前の `review_assign` と一致する。埋め合わせの候補は使える者に含まれない者だけを + この関数より前の輪番（外す 1 者を `(round_no - 1) % 3` で回す式）と一致する。埋め合わせの候補は使える者に含まれない者だけを 使い、含まれる者は飛ばす（同じ席の名前を 2 つ返さないため）。`only` の処理は呼び出し側が 先に行う（1 者指定は埋め合わせをしない）。 """ @@ -347,7 +280,8 @@ def review_seats(round_no: int, available: list[str], fallback: list[str]) -> li def impl_assign(round_no: int, participants: list[str]) -> str: """cross-refactoring の適用担当 1 者を決める: `participants[round_no % len]`。 - 式は変更前の `assign()` と同じで、除数だけを参加者の数にする（設計の決定 7）。 + 式はこの関数より前の輪番（4 者の固定の順を `round_no % 4` で引く式）と同じで、 + 除数だけを参加者の数にする（設計の決定 7）。 ラウンド 1 が `participants[1]` から始まるため、ホスト claude の既定 （claude / codex / kiro）でもホストが最初に適用する形にならない。 """ diff --git a/plugins/ndf/scripts/lib/auth.py b/plugins/ndf/scripts/lib/auth.py index 810f3db88..c981f2824 100644 --- a/plugins/ndf/scripts/lib/auth.py +++ b/plugins/ndf/scripts/lib/auth.py @@ -75,38 +75,6 @@ def _probe_all(runtimes: Iterable[str], info: Callable[[str], None]) -> ProbeRes return results -def check_auth( - runtimes: Iterable[str], - *, - info: Callable[[str], None], - die: Callable[[str], None], - env: Optional[dict[str, str]] = None, -) -> ProbeResult: - """参加する CLI の認証状態を確かめる。1 つでも欠けたら呼び出し側を中断させる。 - - **出力と中断の手段は呼び出し側から受け取る。** 工程ごとに終了コードの意味が違う - （`cross-refactoring` の中断は 4、`cross-review` は 1）ため、この層で決めない。 - - 確認コマンドは CLI の版で変わりうるので、`NDF_SKIP_AUTH_CHECK` で飛ばせるように - しておく。飛ばしたことは必ず出力へ残す（黙って劣化させない）。 - - P7 で消す。止めない確認は `probe_auth`、止めるかの判断は - `assignment.resolve_participants` の `require_all` が持つ。 - """ - if _skipped(env, info): - return {} - - results = _probe_all(runtimes, info) - failed = [f"{name}（{r['detail']}）" for name, r in results.items() if not r["ok"]] - if failed: - die( - "認証されていない CLI があります: " + " / ".join(failed) + "。" - "参加者が欠けたまま進むと、その者のレビューが無いまま収束します。" - "各 CLI でログインしてから再実行してください" - ) - return results - - def probe_auth( runtimes: Iterable[str], *, @@ -119,7 +87,7 @@ def probe_auth( **例外を上げず、呼び出し側も中断させない。** 通らなかった者を外して続けるか、 全員を要して止めるかは、使える者の解決（`assignment.resolve_participants`）が 決める。確認コマンド・未認証の文言・時間切れの秒数・飛ばす環境変数は - `check_auth` と同じものを使う。 + この層の定数（`AUTH_PROBES` ほか）が持つ。 `NDF_SKIP_AUTH_CHECK` が立てば確認コマンドを 1 回も呼ばず `({}, True)` を返す。 飛ばしたことは出力へ残す（黙って劣化させない）。 diff --git a/plugins/ndf/scripts/tests/test_auth_probe.py b/plugins/ndf/scripts/tests/test_auth_probe.py index fd93932a8..f8bca89a5 100644 --- a/plugins/ndf/scripts/tests/test_auth_probe.py +++ b/plugins/ndf/scripts/tests/test_auth_probe.py @@ -2,7 +2,6 @@ 主題は止めない確認 `probe_auth`（#727）である。失敗しても例外を上げず、`ok` と理由を 返し、`NDF_SKIP_AUTH_CHECK` が立てば確認コマンドを 1 回も呼ばない（AC5 / AC6）。 -従来の `check_auth` のテストは、その関数を消す Pull Request（P7）まで末尾に残す。 """ from __future__ import annotations @@ -166,51 +165,3 @@ def run(cmd, **kw): assert results["codex"]["ok"] is False assert results["agy"]["ok"] is True - - -# ---------- check_auth（従来の確認。P7 で消す） ---------- - -def test_unknown_runtime_is_ignored(): - auth = _load_auth() - messages: list[str] = [] - failures: list[str] = [] - - results = auth.check_auth(["unknown"], info=messages.append, die=failures.append, env={}) - - assert "unknown" not in results - assert failures == [] - - -def test_probe_timeout_is_reported(monkeypatch): - auth = _load_auth() - messages: list[str] = [] - failures: list[str] = [] - - def time_out(*args, **kwargs): - raise subprocess.TimeoutExpired(args[0], kwargs["timeout"]) - - monkeypatch.setattr(auth.subprocess, "run", time_out) - results = auth.check_auth(["codex"], info=messages.append, die=failures.append, env={}) - - assert results["codex"]["ok"] is False - assert str(auth.AUTH_PROBE_TIMEOUT) in results["codex"]["detail"] - assert len(failures) == 1 - - -def test_unauthenticated_marker_fails_even_when_probe_exits_zero(monkeypatch): - auth = _load_auth() - messages: list[str] = [] - failures: list[str] = [] - - monkeypatch.setattr( - auth.subprocess, - "run", - lambda *args, **kwargs: SimpleNamespace( - returncode=0, stdout="Not logged in", stderr="" - ), - ) - - results = auth.check_auth(["codex"], info=messages.append, die=failures.append, env={}) - - assert results["codex"]["ok"] is False - assert len(failures) == 1 diff --git a/plugins/ndf/scripts/tests/test_lib_assignment.py b/plugins/ndf/scripts/tests/test_lib_assignment.py index 96d7bc637..73400c8cb 100644 --- a/plugins/ndf/scripts/tests/test_lib_assignment.py +++ b/plugins/ndf/scripts/tests/test_lib_assignment.py @@ -9,50 +9,6 @@ ASSIGNMENT = Path(__file__).resolve().parents[1] / "lib" / "assignment.py" -EXPECTED = { - "claude": [ - ("codex", ["agy", "kiro"]), - ("agy", ["codex", "kiro"]), - ("kiro", ["codex", "agy"]), - ("claude", ["codex", "kiro"]), - ("codex", ["agy", "kiro"]), - ("agy", ["codex", "kiro"]), - ("kiro", ["codex", "agy"]), - ("claude", ["codex", "agy"]), - ], - "codex": [ - ("codex", ["agy", "kiro"]), - ("agy", ["claude", "kiro"]), - ("kiro", ["claude", "agy"]), - ("claude", ["agy", "kiro"]), - ("codex", ["claude", "kiro"]), - ("agy", ["claude", "kiro"]), - ("kiro", ["claude", "agy"]), - ("claude", ["agy", "kiro"]), - ], - "agy": [ - ("codex", ["claude", "kiro"]), - ("agy", ["codex", "kiro"]), - ("kiro", ["claude", "codex"]), - ("claude", ["codex", "kiro"]), - ("codex", ["claude", "kiro"]), - ("agy", ["claude", "kiro"]), - ("kiro", ["claude", "codex"]), - ("claude", ["codex", "kiro"]), - ], - "kiro": [ - ("codex", ["claude", "agy"]), - ("agy", ["claude", "codex"]), - ("kiro", ["codex", "agy"]), - ("claude", ["codex", "agy"]), - ("codex", ["claude", "agy"]), - ("agy", ["claude", "codex"]), - ("kiro", ["claude", "agy"]), - ("claude", ["codex", "agy"]), - ], -} - - @pytest.fixture(scope="module") def assignment(): spec = importlib.util.spec_from_file_location("ndf_lib_assignment", ASSIGNMENT) @@ -106,49 +62,12 @@ def test_detect_host_rejects_an_environment_without_hints(assignment): assignment.detect_host(None, {}) -@pytest.mark.parametrize("host", EXPECTED) -def test_assign_keeps_the_eight_round_rotation(assignment, host): - actual = [assignment.assign(round_no, host) for round_no in range(1, 9)] - - assert actual == EXPECTED[host] - assert any(impl == host for impl, _ in actual) - assert any(impl != host for impl, _ in actual) - assert all(len(reviewers) == 2 for _, reviewers in actual) - assert all(impl not in reviewers for impl, reviewers in actual) - - -@pytest.mark.parametrize("host", ("claude", "codex", "agy", "kiro")) -def test_assign_rejects_a_bad_round(assignment, host): - """round_no < 1 の場合に AssignmentError が送出される（R2-001）。""" - for round_no in (0, -1): - with pytest.raises( - assignment.AssignmentError, - match=r"^ラウンド番号は 1 以上です:", - ) as excinfo: - assignment.assign(round_no, host) - assert "ラウンド番号は 1 以上です" in str(excinfo.value) - - -def test_review_assign_rejects_a_bad_round(assignment): - """round_no < 1 の下限境界で AssignmentError が送出される（R2-003）。 - - 同モジュールの `assign` / `review_seats` は下限境界を固定しているが、 - `review_assign` だけ抜けていたため現状の振る舞いを固定する。 - """ - for round_no in (0, -1): - with pytest.raises( - assignment.AssignmentError, - match=r"^ラウンド番号は 1 以上です:", - ) as excinfo: - assignment.review_assign(round_no, "claude") - assert "ラウンド番号は 1 以上です" in str(excinfo.value) - - @pytest.mark.parametrize("host", ("gemini", "unknown")) -def test_review_assign_rejects_a_host_outside_host_runtimes(assignment, host): - """HOST_RUNTIMES に含まれないホストを拒否する現状を固定する（R2-005）。""" +@pytest.mark.parametrize("pool", ("review_pool", "refactor_pool")) +def test_the_default_pools_reject_a_host_outside_host_runtimes(assignment, pool, host): + """HOST_RUNTIMES に含まれないホストを、どちらの母集合の既定も拒否する。""" with pytest.raises(assignment.AssignmentError) as excinfo: - assignment.review_assign(1, host) + getattr(assignment, pool)(host) assert "ホストになれないランタイムです" in str(excinfo.value) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor.py index 79203a407..01df6cf50 100755 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor.py @@ -61,7 +61,11 @@ cmd_report, cmd_status, ) -from refactor_lib.commands.setup import cmd_init, cmd_start_round # noqa: E402 +from refactor_lib.commands.setup import ( # noqa: E402 + cmd_init, + cmd_start_round, + runtime_list, +) from refactor_lib.measure import summary_extra # noqa: E402 import run_metrics # noqa: E402 @@ -83,6 +87,10 @@ def _write_run_summary(path: pathlib.Path, state: dict) -> None: SEVERITY_ORDER, ) +# **状態ファイルに載る引数の既定は `None` にする**（#727 の決定 13）。既定値を引数に +# 持たせると、再開で「渡さなかった」と「既定値を渡した」を区別できない。新規の +# 初期化が `commands/setup.py` の `NEW_RUN_DEFAULTS` で置き換える。 + # ---------------- main ---------------- @@ -94,39 +102,51 @@ def main() -> None: init = sub.add_parser( "init", - help="Step 0 — ホスト確定 / 母集合の確定 / 作業ディレクトリ root / 状態初期化") + help="Step 0 — ホスト確定 / 参加者の確定 / 作業ディレクトリ root / 状態初期化・再開") init.add_argument("pr", type=int) init.add_argument("--scope", nargs="+", required=True, help="対象範囲。提案が無制限に広がらないよう必須にしている") init.add_argument("--host", choices=list(assignment.HOST_RUNTIMES), default=None, help="ホストの明示指定。未指定時は環境変数から推定する") + # **参加者は既定に足し引きして決める**（#727 の決定 4）。既定は codex / kiro と + # ホストで、使う側を並べる形にしないのは、ホストが変わるたびに書き直さずに済むため。 + init.add_argument("--exclude", action="append", type=runtime_list, default=None, + help="参加者から外す者（ホストも外せる）。カンマ区切り・繰り返し可。" + "再開で none を渡すと空へ戻す") + init.add_argument("--include", action="append", type=runtime_list, default=None, + help="参加者に足す者（例: agy）。カンマ区切り・繰り返し可。" + "再開で none を渡すと空へ戻す") + init.add_argument("--require-all", dest="require_all", + action=argparse.BooleanOptionalAction, default=None, + help="確認を通らない者が 1 者でもいれば中断する。" + "既定は外して続ける") # **切るのは提案の回数であって、適用できる件数ではない**（#436 決定 8）。 # 適用ラウンドを分けたことで、1 回の提案で通せる件数は上限に縛られなくなった。 # 取り消した項目は除外されるため、同じ提案が積み上がって回数を食うこともない。 # **輪番の 1 周を根拠にしない。** 適用の担当は適用ラウンドごとに進むので、 # 1 つの提案ラウンドが複数の群を持てば輪番は 1 周しうる。 - init.add_argument("--max-outer-rounds", type=int, default=3, - help="構造改善の提案ラウンドの上限") + init.add_argument("--max-outer-rounds", type=int, default=None, + help="構造改善の提案ラウンドの上限 (default: 3)") # テスト整備は母集合が増えない（対象のコードを変えないため、テストが薄い経路の # 集合は最初から確定している）。2 回目に出るのは 1 回目の挙げ漏らしだけである。 - init.add_argument("--max-test-rounds", type=int, - default=DEFAULT_MAX_TEST_ROUNDS, + init.add_argument("--max-test-rounds", type=int, default=None, help="テスト整備ラウンドの上限。到達したら採用が残っていても " "構造改善の提案ラウンドへ進む " f"(default: {DEFAULT_MAX_TEST_ROUNDS})") - init.add_argument("--max-fix-rounds", type=int, default=3, - help="1 つの適用ラウンドあたりの修正ラウンドの上限") - init.add_argument("--max-items-per-round", type=int, default=5, - help="1 つの提案ラウンド／テスト整備ラウンドの採用上限") + init.add_argument("--max-fix-rounds", type=int, default=None, + help="1 つの適用ラウンドあたりの修正ラウンドの上限 (default: 3)") + init.add_argument("--max-items-per-round", type=int, default=None, + help="1 つの提案ラウンド／テスト整備ラウンドの採用上限 (default: 5)") init.add_argument("--ci-check", default=None, metavar="NAME", help="最終ゲートで手元のテストの代わりに見る検査の名前。" "**指定すると手元のテストは実行しない**（排他）。" "指定が無ければ手元のテストで判定する") - init.add_argument("--severity-threshold", default=DEFAULT_SEVERITY_THRESHOLD, - choices=[s for s in SEVERITY_ORDER if s != "unknown"]) + init.add_argument("--severity-threshold", default=None, + choices=[s for s in SEVERITY_ORDER if s != "unknown"], + help=f"この重要度未満は採用しない (default: {DEFAULT_SEVERITY_THRESHOLD})") init.add_argument("--model", action="append", metavar="RUNTIME=MODEL", help="ランタイムごとのモデル指定。繰り返し指定できる") - init.add_argument("--test-timeout", type=int, default=DEFAULT_TEST_TIMEOUT, + init.add_argument("--test-timeout", type=int, default=None, help="テスト 1 回あたりの上限秒数。超えたら失敗として扱う " f"(default: {DEFAULT_TEST_TIMEOUT})") init.add_argument("--sync-command", default=None, @@ -147,7 +167,7 @@ def main() -> None: "振る舞い不変を示す手段が無い書き換えは構造改善ではないため必須") # **起動のされ方は引数で受け取る**（#436 決定 7）。環境変数や控えの読み取りは、 # 起動元が違っても同じ値になりうる。呼ぶ側が明示すれば判定が 1 か所で済む。 - init.add_argument("--workflow-step", action="store_true", + init.add_argument("--workflow-step", action="store_true", default=None, help="`development-workflow` の 1 工程として起動したことを" "伝える。Step 7 の `cross-review` を省き、" "全体のテストで判定する") @@ -156,7 +176,7 @@ def main() -> None: for name, func, help_ in ( ("start-round", cmd_start_round, - "Step 2 — 提案ラウンドを開く。実装担当とレビュー担当を返す"), + "Step 2 — 提案ラウンドを開く。実装担当を返す"), ("merge-proposals", cmd_merge_proposals, "Step 3 — 提案の語彙検証・重複排除・優先度付け・採否"), ("advance", cmd_advance, "ラウンドの収束判定と、ラウンドの種類の切り替え"), 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 e89feec90..c3bd6ceaf 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 @@ -568,7 +568,7 @@ def _switch_apply_impl(state: dict[str, Any], group: dict[str, Any]) -> bool: } tried.add(group.get("impl")) seq = safe_int(state.get("apply_seq")) - for _ in range(len(state.get("impl_capable") or []) or 4): + for _ in range(len(state.get("runtimes") or [])): seq += 1 impl, requested = impl_for_seq(state, seq) if impl in tried: @@ -649,7 +649,7 @@ def _load_apply_context( ctx: _ApplyExecutionContext, payload: dict[str, Any], ) -> tuple[dict[str, Any], _ApplyCommitRange]: impl = ctx.group.get("impl") or ctx.entry["impl"] - record_observed_model(ctx.entry, "impl", impl, ctx.state, "apply", ctx.args.round) + record_observed_model(ctx.entry, impl, ctx.state, "apply", ctx.args.round) # 検証の材料は git から取る。結果ファイルから使うのは # 「どのコミットがこの群のものか」という対応付けだけ。 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 955bfea46..c6dd584c9 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 @@ -104,8 +104,8 @@ def cmd_status(args: argparse.Namespace) -> None: _, state = load_state(args.id) print(f"# cross-refactoring rf{state['id']}（{state['repo']} #{state['current_pr']}）") print(f"ホスト: {state['host']}（{state['host_detection']}）") - print(f"提案・レビュー: {' / '.join(state['runtimes'])}") - print(f"適用の母集合: {' / '.join(state['impl_capable'])}") + # **母集合は 1 つである**（#727 の決定 5）。提案と適用は同じ参加者で回す。 + print(f"参加者（提案と適用）: {' / '.join(state['runtimes'])}") print(f"局面: {state['phase']} / 提案ラウンド {state['outer_round']} " f"/ {state['max_outer_rounds']}") print(f"終了理由: {state.get('final') or '（未終了）'}") @@ -125,6 +125,8 @@ def cmd_report(args: argparse.Namespace) -> None: print("## 改善項目") print() print(_item_table(state)) + print() + _print_participants(state) # **取り消した項目の内訳は書かない**（#436 決定 6-b）。件数だけ述べ、内訳は # 改修計画へ譲る。同じ一覧を 2 か所に置くと、片方だけが古くなる。 print() @@ -158,6 +160,54 @@ def _print_header(state: dict[str, Any]) -> None: f" / 修正 {gate.get('fix_rounds', 0)} 回）") +def _print_participants(state: dict[str, Any]) -> None: + """「参加した者」の節を出す（#727 の F6）。 + + 途中から誰を外したか・誰が確認を通らなかったかを、完了報告だけで読めるように + する。参加者の記録を持たない状態ファイル（この変更の前に始めた実行）では + 「記録なし」と出す。 + """ + print("## 参加した者") + print() + p = state.get("participants") + if not p: + print("- 使える者: 記録なし") + print() + return + + def _names(values: Any) -> str: + return " / ".join(values) if values else "なし" + + unavailable = p.get("unavailable") or {} + if unavailable: + failed = " / ".join(f"{n}（{d}）" for n, d in unavailable.items()) + elif p.get("probe_skipped"): + failed = "確認を飛ばした（NDF_SKIP_AUTH_CHECK）" + else: + failed = "なし" + print(f"- 母集合: {_names(p.get('pool'))}") + print(f"- 使える者: {_names(p.get('available'))}") + print(f"- --exclude で外した者: {_names(p.get('excluded'))}") + print(f"- --include で足した者: {_names(p.get('included'))}") + print(f"- 確認を通らなかった者: {failed}") + changes = state.get("resume_changes") or [] + if not changes: + print("- 再開で変えた値: なし") + else: + print("- 再開で変えた値:") + for c in changes: + print(f" - {c.get('at')} {c.get('field')}: " + f"{_change_value(c.get('from'))} → {_change_value(c.get('to'))}") + print() + + +def _change_value(value: Any) -> str: + """再開で変えた値の 1 つを 1 行へ収める。参加者の記録は使える者だけを出す。""" + if isinstance(value, dict) and "available" in value: + return " / ".join(value.get("available") or []) or "なし" + return str(value) + + def _print_deferred(state: dict[str, Any]) -> None: """見送り節（件数と改修計画への参照）を出す。""" print("## 見送った提案") @@ -180,29 +230,22 @@ def _print_run_metrics(path: pathlib.Path, state: dict[str, Any]) -> None: def _round_table(state: dict[str, Any]) -> str: + """ラウンド表。**レビュー担当の列を持たない**（#727 の決定 6）。 + + レビュー工程は #436 で消えた。古い状態ファイルがレビュー担当を持っていても出さない。 + """ lines = [ - "| R | 種類 | 実装担当 | モデル | レビュー担当 | モデル | 採用 | 適用 | 見送り | 修正 | 初回承認 |", - "| --- | --- | --- | --- | --- | --- | ---: | ---: | ---: | ---: | --- |", + "| R | 種類 | 実装担当 | モデル | 採用 | 適用 | 見送り | 修正 |", + "| --- | --- | --- | --- | ---: | ---: | ---: | ---: |", ] for entry in state["rounds"]: - reviewers = entry.get("reviewers", []) - reviewer_models = entry.get("reviewer_models") or {} - reviews = entry.get("reviews") or [] - first_approved = "—" - if reviews: - first_approved = ( - "はい" if all(reviews[0].get(r) == "APPROVE" for r in reviewers) else "いいえ" - ) lines.append( f"| {entry['round']} | " f"{'テスト整備' if entry_kind(entry) == TEST else '構造改善'} | " f"{entry.get('impl', '—')} | " f"{models_lib.label((entry.get('impl_model') or {}).get('requested'))} | " - f"{' / '.join(reviewers) or '—'} | " - f"{' / '.join(models_lib.label((reviewer_models.get(r) or {}).get('requested')) for r in reviewers) or '—'} | " f"{entry.get('adopted', 0)} | {len(entry.get('apply', {}).get('applied', []))} | " - f"{len(entry.get('apply', {}).get('failed', []))} | {entry.get('fix_rounds', 0)} | " - f"{first_approved} |" + f"{len(entry.get('apply', {}).get('failed', []))} | {entry.get('fix_rounds', 0)} |" ) return "\n".join(lines) if state["rounds"] else "（ラウンドなし）" 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 011ec37a5..6857f63f3 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 @@ -1,7 +1,7 @@ """ラウンドの入口。`init` と `start-round` を持つ。 -対象の Pull Request の文脈・参加する CLI の認証・作業ツリーの用意・状態ファイルの -初期化と、提案ラウンドの開始を扱う。 +対象の Pull Request の文脈・参加者の決定・作業ツリーの用意・状態ファイルの +初期化と再開と、提案ラウンドの開始を扱う。 """ from __future__ import annotations @@ -31,9 +31,18 @@ tmp_dir_for, ) from ..plan import PLAN_COMMENT, PLAN_FILE, PLAN_NONE, normalize_plan_file -from ..rounds import finish_outer_rounds, STRUCTURE, TEST, entry_kind, round_kind +from ..rounds import ( + STRUCTURE, + TEST, + entry_kind, + finish_outer_rounds, + impl_for_seq, + round_kind, +) from ..scope import require_scope_covers_tests from ..vocabulary import ( + DEFAULT_MAX_TEST_ROUNDS, + DEFAULT_SEVERITY_THRESHOLD, DEFAULT_TEST_TIMEOUT, IMPL_STALL_MARGIN, REQUIRED_SKILLS, @@ -42,14 +51,103 @@ ) -def check_auth(runtimes: Iterable[str]) -> dict[str, dict[str, Any]]: - """参加する CLI の認証状態を確かめる。1 つでも欠けたら初期化を中断する。 +# 再開で指定を外す予約語（#727 の決定 15）。足す者・外す者に渡すと一覧を空へ戻す。 +NONE_WORD = "none" + +# 新規の初期化で、未指定の引数を置き換える現行の既定。**引数の既定は `None` にする。** +# 既定値を引数に持たせると、再開で「渡さなかった」と「既定値を渡した」を区別できない +# （#727 の決定 13）。 +NEW_RUN_DEFAULTS: dict[str, Any] = { + "max_outer_rounds": 3, + "max_test_rounds": DEFAULT_MAX_TEST_ROUNDS, + "max_fix_rounds": 3, + "max_items_per_round": 5, + "test_timeout": DEFAULT_TEST_TIMEOUT, + "severity_threshold": DEFAULT_SEVERITY_THRESHOLD, + "workflow_step": False, +} + +# 再開で渡した引数の反映の表（#727 の決定 13）。**状態ファイルに載る引数は、この 2 つの +# 表のどちらかに必ず載る。** `replace` は状態へ書いて記録へ積み、`notify` は状態と違う +# ときだけ「反映しない」と知らせる。 +RESUME_REPLACE_FIELDS = tuple( + statefile.ResumeField(key, key, "replace") + for key in ("max_outer_rounds", "max_test_rounds", "max_fix_rounds", + "max_items_per_round", "test_timeout") +) +RESUME_NOTIFY_FIELDS = ( + statefile.ResumeField("host", "host", "notify"), + statefile.ResumeField("scope", "target_scope", "notify"), + statefile.ResumeField("model", "models", "notify"), + statefile.ResumeField("baseline_test", "baseline_test", "notify"), + statefile.ResumeField("ci_check", "ci_check", "notify"), + statefile.ResumeField("severity_threshold", "severity_threshold", "notify"), + statefile.ResumeField("sync_command", "sync_command", "notify"), + statefile.ResumeField("plan_file", "plan_file", "notify"), + statefile.ResumeField("workflow_step", "workflow_step", "notify"), + statefile.ResumeField("worktree_root", "worktree_root", "notify"), +) + + +def runtime_list(value: str) -> list[str]: + """`--exclude` / `--include` の型。カンマ区切りの 4 つの名前、または `none`。""" + names = [n.strip() for n in value.split(",") if n.strip()] + if not names: + raise argparse.ArgumentTypeError("名前を 1 つ以上指定してください") + for name in names: + if name != NONE_WORD and name not in assignment.ALL_RUNTIMES: + raise argparse.ArgumentTypeError( + f"{'/'.join(assignment.ALL_RUNTIMES)} か {NONE_WORD} を指定してください: {name}") + return names - 実装は共通層（`lib/auth.py`）にある。**この工程の中断は終了コード 4 である**ため、 - 出力と中断の手段をここから渡す。 + +def _names_arg(args: argparse.Namespace, option: str) -> Optional[list[str]]: + """`--include` / `--exclude` を平らな一覧へ直す。未指定は `None`、`none` は空。 + + `action="append"` の入れ子を平らにし、`--exclude agy --exclude kiro` と + `--exclude agy,kiro` を同じにする。`none` と名前の混在は中断する。 """ - return auth.check_auth(runtimes, info=info, die=die) + raw = getattr(args, option, None) + if raw is None: + return None + names: list[str] = [] + for group in raw: + names.extend(group if isinstance(group, list) else [group]) + if NONE_WORD in names: + if len(names) > 1: + die(f"--{option} に {NONE_WORD} と名前を同時に指定できません: {', '.join(names)}") + return [] + return names + + +def resolve_participants( + host: str, include: list[str], exclude: list[str], require_all: bool, +) -> dict[str, Any]: + """参加者を決め、状態ファイルの `participants` を返す（#727 の決定 2〜5）。 + 母集合の既定は `refactor_pool(host)`（codex / kiro とホスト）。確認は止めない確認 + （`auth.probe_auth`）で、通らない者は外して続ける。名前の矛盾・全員を要する指定で + 欠け・使える者が 0 者は、この工程の中断（終了コード 4）へ写す。状態ファイルは + この関数の後に書かれるため、失敗したときは作られも書き換えられもしない。 + """ + try: + pool = assignment.refactor_pool(host) + resolved = assignment.resolve_participants( + pool, host=host, include=include, exclude=exclude, + probe=lambda names: auth.probe_auth(names, info=info), + require_all=require_all, + ) + except assignment.AssignmentError as e: + die(str(e)) + raise + info(f"ホスト: {host} / 母集合: {' / '.join(pool)}" + f" / 使える者: {' / '.join(resolved.available) or 'なし'}") + for name, reason in resolved.unavailable.items(): + info(f"⚠ {name} を担当から外しました（{reason}）") + if not resolved.available: + die(f"使える者がいません: 参加者の全員が確認を通りませんでした" + f"（{' / '.join(f'{n}: {d}' for n, d in resolved.unavailable.items())}）") + return resolved.to_state() def _apply_post_event(state: dict[str, Any], is_own_pr: bool) -> None: @@ -177,10 +275,8 @@ class InitialContext: tmp_dir: pathlib.Path host: str detection: str - runtimes: list[str] - impl_capable: list[str] + participants: dict[str, Any] model_spec: dict[str, Optional[str]] - auth: dict[str, dict[str, Any]] baseline: dict[str, Any] @@ -194,6 +290,7 @@ def _build_initial_state( （`cmd_init`）が済ませたうえで値として渡す。この関数が持つのは、状態ファイルに 何という鍵で何を残すかだけである。 """ + runtimes = list(ctx.participants["available"]) return { "id": args.pr, "started_at": statefile.now(), @@ -204,16 +301,17 @@ def _build_initial_state( "worktree_root": str(ctx.root), "worktrees": { "work": str(ctx.work), - **{r: str(ctx.root / r) for r in ctx.runtimes}, + **{r: str(ctx.root / r) for r in runtimes}, }, "tmp_dir": str(ctx.tmp_dir), "target_scope": list(args.scope), "host": ctx.host, "host_detection": ctx.detection, - "runtimes": ctx.runtimes, - "impl_capable": ctx.impl_capable, + # **提案の対象と適用の輪番が同じ一覧を読む**（#727 の決定 5）。使える者と同じ値。 + "runtimes": runtimes, + "participants": ctx.participants, + "resume_changes": [], "models": ctx.model_spec, - "auth": ctx.auth, # 提案プロンプトへ許容値をそのまま列挙するために持たせる。 # 定義は検証側（この CLI）にあり、状態ファイル経由で起動側へ渡す。 "vocabulary": vocabulary(), @@ -254,10 +352,11 @@ def _build_initial_state( def cmd_init(args: argparse.Namespace) -> None: - """Step 0 — ホストと母集合を確定し、作業ディレクトリ root と状態を用意する。 + """Step 0 — ホストと参加者を確定し、作業ディレクトリ root と状態を用意する。 - **提案・レビューの母集合（全 − ホスト）と適用の母集合（全 − agy）を - 別々に確定する。** 両者は重なるが一致しない。 + **母集合は 1 つである**（#727 の決定 5）。提案と適用は同じ参加者で回す。参加者は + codex / kiro とホストを既定とし、足す者・外す者で変える。確認を通らない者は外して + 続ける。前回の状態が残っていれば再開し、渡した引数を反映の表に従って扱う。 """ try: host, detection = assignment.detect_host(args.host) @@ -269,16 +368,8 @@ def cmd_init(args: argparse.Namespace) -> None: except models_lib.ModelSpecError as e: die(str(e)) return - - runtimes = assignment.review_pool(host) - impl_capable = assignment.impl_pool() - if host in runtimes: - die(f"提案・レビューの母集合にホスト {host} が含まれています（判定の誤り）") - _warn_unmeasurable_models(model_spec, set(runtimes) | set(impl_capable)) - - # **認証は作業ディレクトリを作る前に確かめる。** 未認証のまま進むと、 - # 参加者が欠けた構成のまま最後まで走り切ってしまう。 - auth = check_auth(sorted(set(runtimes) | set(impl_capable))) + include = _names_arg(args, "include") + exclude = _names_arg(args, "exclude") # リポジトリ名は git の設定から求め、Pull Request の応答で確かめる（#271）。 repo, base_branch, head_branch, is_own_pr, author = _fetch_pr_context(args.pr) @@ -306,12 +397,19 @@ def cmd_init(args: argparse.Namespace) -> None: if state_file.exists(): state = statefile.load(state_file) if state.get("final") is None: - info(f"↻ 前回中断した状態から再開します（提案ラウンド {state.get('outer_round', 0)}）") - _apply_post_event(state, is_own_pr) - statefile.save(state_file, state) - _emit_init(state) + _resume(state_file, state, args, model_spec, include, exclude, is_own_pr) return + for key, value in NEW_RUN_DEFAULTS.items(): + if getattr(args, key, None) is None: + setattr(args, key, value) + + # **確認は着手前のテストより先に行う。** 使える者がいなければ、テストに時間を + # 使わずに止める。 + participants = resolve_participants( + host, include or [], exclude or [], bool(getattr(args, "require_all", None))) + _warn_unmeasurable_models(model_spec, participants["available"]) + baseline = _run_baseline_test(args.baseline_test, work, args.test_timeout) context = InitialContext( @@ -323,10 +421,8 @@ def cmd_init(args: argparse.Namespace) -> None: tmp_dir=tmp_dir, host=host, detection=detection, - runtimes=runtimes, - impl_capable=impl_capable, + participants=participants, model_spec=model_spec, - auth=auth, baseline=baseline, ) state = _build_initial_state(args, context) @@ -337,11 +433,81 @@ def cmd_init(args: argparse.Namespace) -> None: statefile.save(state_file, state) info(f"✅ 状態を初期化しました: {state_file}") info(f" ホスト: {host}（{detection}）") - info(f" 提案・レビュー: {' / '.join(runtimes)}") - info(f" 適用の母集合: {' / '.join(impl_capable)}") + info(f" 参加者（提案と適用）: {' / '.join(state['runtimes'])}") + _emit_init(state) + + +def _resume( + state_file: pathlib.Path, + state: dict[str, Any], + args: argparse.Namespace, + model_spec: dict[str, Optional[str]], + include: Optional[list[str]], + exclude: Optional[list[str]], + is_own_pr: bool, +) -> None: + """前回中断した状態から再開する（#727 / #648 の決定 13〜16）。 + + 上限は渡せば反映し、状態に載る他の引数は状態と違えば知らせる。足す者・外す者・ + 全員を要する指定のどれかを渡したときだけ確かめ直し、**渡さなかった値は記録から + 補う**。作り直しは `resume_changes` に 1 件として積む。作り直しが失敗したときは + 書き込みの前に中断するため、状態ファイルは変わらない。 + """ + info(f"↻ 前回中断した状態から再開します（提案ラウンド {state.get('outer_round', 0)}）") + for line in statefile.apply_resume_args(state, args, RESUME_REPLACE_FIELDS): + info(line) + view, given = _notify_view(state, args, model_spec) + for line in statefile.apply_resume_args(view, given, RESUME_NOTIFY_FIELDS): + info(line) + + require_all = getattr(args, "require_all", None) + if include is not None or exclude is not None or require_all is not None: + recorded = state.get("participants") or {} + participants = resolve_participants( + str(state["host"]), + include if include is not None else list(recorded.get("included") or []), + exclude if exclude is not None else list(recorded.get("excluded") or []), + bool(require_all) if require_all is not None else bool(recorded.get("require_all")), + ) + state.setdefault("resume_changes", []).append({ + "at": statefile.now(), "field": "participants", + "from": state.get("participants"), "to": participants, + }) + state["participants"] = participants + state["runtimes"] = list(participants["available"]) + worktrees = state.setdefault("worktrees", {}) + for runtime in state["runtimes"]: + worktrees.setdefault(runtime, str(pathlib.Path(state["worktree_root"]) / runtime)) + + _apply_post_event(state, is_own_pr) + statefile.save(state_file, state) _emit_init(state) +def _notify_view( + state: dict[str, Any], args: argparse.Namespace, + model_spec: dict[str, Optional[str]], +) -> tuple[dict[str, Any], argparse.Namespace]: + """「知らせる」の比較を、状態と引数の形を揃えて行うための写しを返す。 + + 状態は着手前のテストを `{command, status, checked_at}` で、モデルを全ランタイムの + 辞書で、作業ディレクトリ root を解決済みのパスで持つ。引数の形のまま比べると、 + 同じ値でも「違う」と知らせてしまう。 + """ + view = dict(state) + view["baseline_test"] = (state.get("baseline_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 + if given.worktree_root is not None: + given.worktree_root = str(pathlib.Path(given.worktree_root).resolve()) + if given.plan_file is not None: + given.plan_file = normalize_plan_file(given.plan_file) + if given.scope is not None: + given.scope = list(given.scope) + return view, given + + def _emit_init(state: dict[str, Any]) -> None: statefile.emit( ID=state["id"], @@ -349,7 +515,6 @@ def _emit_init(state: dict[str, Any]) -> None: HOST=state["host"], RUNTIMES=" ".join(state["runtimes"]), RUNTIMES_CSV=",".join(state["runtimes"]), - IMPL_POOL=" ".join(state["impl_capable"]), WORKTREE_ROOT=state["worktree_root"], WORK=state["worktrees"]["work"], TMP_DIR=state["tmp_dir"], @@ -466,7 +631,10 @@ def rounds_of_kind(state: dict[str, Any], kind: str) -> list[dict[str, Any]]: def cmd_start_round(args: argparse.Namespace) -> None: - """Step 2 — ラウンドを開き、実装担当とレビュー担当を返す。 + """Step 2 — ラウンドを開き、実装担当を返す。 + + **レビュー担当は返さない**（#727 の決定 6）。レビュー工程は #436 で消え、Step 7 の + cross-review が担う。 終了コード: 0 = ラウンドを開いた / 1 = 繰り返しが終了済み。 @@ -491,8 +659,7 @@ def cmd_start_round(args: argparse.Namespace) -> None: round_no = len(rounds) + 1 existing = next((r for r in rounds if r["round"] == round_no), None) if existing is None: - impl, reviewers = assignment.assign(round_no, state["host"]) - models = state["models"] + impl, requested = impl_for_seq(state, round_no) existing = { "round": round_no, # **種類はラウンドごとに残す。** 上限を別々に数えるためと、提案の @@ -500,11 +667,7 @@ def cmd_start_round(args: argparse.Namespace) -> None: "kind": kind, "started_at": statefile.now(), "impl": impl, - "impl_model": {"requested": models.get(impl), "observed": None}, - "reviewers": reviewers, - "reviewer_models": { - r: {"requested": models.get(r), "observed": None} for r in reviewers - }, + "impl_model": {"requested": requested, "observed": None}, "proposed": {}, "merged": 0, "adopted": 0, "deferred": 0, "items": [], @@ -528,7 +691,7 @@ def cmd_start_round(args: argparse.Namespace) -> None: seq = len(rounds_of_kind(state, kind)) info( f"=== {label} {seq} / {limit} " - f"（実装 {existing['impl']} / レビュー {' + '.join(existing['reviewers'])}）===" + f"（実装 {existing['impl']}）===" ) statefile.emit( ROUND=round_no, @@ -543,7 +706,5 @@ def cmd_start_round(args: argparse.Namespace) -> None: PROPOSE_PHASE="propose-tests" if kind == TEST else "propose", IMPL=existing["impl"], IMPL_MODEL=existing["impl_model"]["requested"], - REVIEWERS=" ".join(existing["reviewers"]), - REVIEWERS_CSV=",".join(existing["reviewers"]), MAX_FIX_ROUNDS=state["max_fix_rounds"], ) 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 740b49f02..f4b19107d 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py @@ -1131,10 +1131,10 @@ def read_result( def record_observed_model( - entry: dict[str, Any], role: str, runtime: str, + entry: dict[str, Any], runtime: str, state: dict[str, Any], phase: str, round_no: Optional[int], ) -> None: - """CLI の出力から実際に使われたモデル名を拾って記録する。 + """実装担当の CLI の出力から、実際に使われたモデル名を拾って記録する。 取れるのは claude だけである。取れないランタイムは `None` のままにし、 報告では既定モデルのラウンドとして集計から区別する。 @@ -1148,13 +1148,8 @@ def record_observed_model( ) if not observed: return - if role == "impl": - entry["impl_model"]["observed"] = observed - requested = entry["impl_model"]["requested"] - else: - entry["reviewer_models"].setdefault(runtime, {"requested": None, "observed": None}) - entry["reviewer_models"][runtime]["observed"] = observed - requested = entry["reviewer_models"][runtime]["requested"] + entry["impl_model"]["observed"] = observed + requested = entry["impl_model"]["requested"] warning = models_lib.mismatch_warning(runtime, requested, observed) if warning: info(warning) diff --git a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/plan.py b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/plan.py index 8b68979b7..3fbaf69d0 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/plan.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/plan.py @@ -199,10 +199,8 @@ def _plan_deferred_section(state: dict[str, Any]) -> list[str]: def _plan_round_section(state: dict[str, Any], entry: dict[str, Any]) -> list[str]: """1 ラウンド分の見出しと、そのラウンドの改善項目を並べる。""" - reviewers = " / ".join(entry.get("reviewers") or []) or "—" lines = [ - f"## ラウンド {entry['round']}" - f"（実装 {entry.get('impl', '—')} / レビュー {reviewers}）", + f"## ラウンド {entry['round']}（実装 {entry.get('impl', '—')}）", "", ] items = [i for i in state.get("items") or [] if i.get("round") == entry["round"]] 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 fa17d6efa..0b4c825c2 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py @@ -173,11 +173,12 @@ def group_reopening(group: dict[str, Any]) -> str: def impl_for_seq(state: dict[str, Any], seq: int) -> tuple[str, Optional[str]]: """輪番の通し番号から、作業を任せる担当と要求するモデルを引く。 - **輪番を引く呼び出しはここだけにする**（#728 の決定 9）。参加者の決め方が - 変わったとき（#727）に、変える場所がこの中だけで済む。読むのは 3 か所 - （群の割り当て・結果なしの試行の交代先・最終ゲートの修正担当）である。 + **輪番を引く呼び出しはここだけにする**（#728 の決定 9）。読むのは 4 か所 + （ラウンドの開始・群の割り当て・結果なしの試行の交代先・最終ゲートの修正担当） + である。輪番は参加者の一覧（`runtimes`）の中で回す（#727 の決定 5・7）。この + 変更の前に始めた実行の状態ファイルも、適用専用の母集合を読まずに同じ一覧で決める。 """ - impl, _reviewers = assignment.assign(seq, state["host"]) + impl = assignment.impl_assign(seq, list(state["runtimes"])) return impl, (state.get("models") or {}).get(impl) def phase_after_group(entry: dict[str, Any]) -> str: diff --git a/plugins/ndf/skills/cross-refactoring/tests/crossref_helpers.py b/plugins/ndf/skills/cross-refactoring/tests/crossref_helpers.py index 9914d55a8..b6d249946 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/crossref_helpers.py +++ b/plugins/ndf/skills/cross-refactoring/tests/crossref_helpers.py @@ -36,7 +36,6 @@ def make_state(tmp_path: pathlib.Path, **overrides: Any) -> pathlib.Path: "host": host, "host_detection": "explicit", "runtimes": runtimes, - "impl_capable": ["claude", "codex", "kiro"], "models": {"claude": None, "codex": None, "agy": None, "kiro": None}, "skills": {"required": ["refactoring", "tdd-cycle", "quality-gates"]}, "max_outer_rounds": 3, diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py b/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py index 0afd3505c..c1da76994 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_apply_attempts.py @@ -344,12 +344,14 @@ def test_the_group_assignment_goes_through_the_single_rotation_function( rounds, monkeypatch ): """AC49: 輪番から担当を引く関数を差し替えると、群の担当がそれに従う。""" - state = {"host": "claude", "models": {"kiro": "auto"}} + state = {"host": "claude", "runtimes": ["claude", "codex", "kiro"], + "models": {"kiro": "auto"}} - impl, requested = rounds.impl_for_seq(state, 3) + impl, requested = rounds.impl_for_seq(state, 2) - assert impl in {"claude", "codex", "agy", "kiro"} - assert requested == state["models"].get(impl) + # 輪番は参加者の一覧の中で回る（#727 の決定 7: `runtimes[seq % n]`） + assert impl == "kiro" + assert requested == "auto" # ---------- 修正結果が無く範囲も確定できないとき（converge.cmd_merge_fix / R1-003） ---------- diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_assignment.py b/plugins/ndf/skills/cross-refactoring/tests/test_assignment.py index 134de695f..12f6475fd 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_assignment.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_assignment.py @@ -1,8 +1,7 @@ -"""担当の決定（ホスト判定 / 母集合 / 輪番）のテスト。 +"""担当の決定（ホスト判定 / 母集合の既定 / 輪番 / 席）のテスト。 -**`runtimes` と `impl_capable` を同一視しない**ことがここの主題である。 -前者はホストを除いた 3 者（提案・レビュー）、後者は参加する 4 者すべて（適用）で、 -重なるが一致しない。 +cross-refactoring は提案と適用を 1 つの参加者の一覧で回し（#727 の決定 5）、 +cross-review はホストを除く母集合から 2 席を決める。母集合の既定は Skill ごとに違う。 """ from __future__ import annotations @@ -61,10 +60,15 @@ def test_review_pool_is_all_minus_host(assignment, host): assert set(pool) == set(assignment.ALL_RUNTIMES) - {host} -@pytest.mark.parametrize("host", HOSTS) -def test_impl_pool_is_host_independent(assignment, host): - """適用の母集合はホストによらず参加する 4 者すべてになる。""" - assert assignment.impl_pool() == ["claude", "codex", "agy", "kiro"] +@pytest.mark.parametrize("host, expected", [ + ("claude", ["claude", "codex", "kiro"]), + ("codex", ["codex", "kiro"]), + ("agy", ["codex", "agy", "kiro"]), + ("kiro", ["codex", "kiro"]), +]) +def test_refactor_pool_is_codex_kiro_and_the_host(assignment, host, expected): + """AC31 / AC32 — cross-refactoring の既定はホストを含む。並びは固定の順。""" + assert assignment.refactor_pool(host) == expected def test_no_runtime_is_excluded_from_applying(assignment): @@ -75,88 +79,42 @@ def test_no_runtime_is_excluded_from_applying(assignment): assert not hasattr(assignment, "IMPL_EXCLUDED") -# ---------- 輪番 ---------- +# ---------- 適用の輪番（cross-refactoring） ---------- @pytest.mark.parametrize("host", HOSTS) -def test_impl_and_reviewers_never_overlap(assignment, host): - for round_no in range(1, 17): - impl, reviewers = assignment.assign(round_no, host) - assert impl not in reviewers, f"round {round_no} で実装担当がレビューにも入っている" - assert len(reviewers) == 2, f"round {round_no} のレビュー担当が 2 者でない" - assert host not in reviewers, f"round {round_no} でホストがレビューに入っている" +def test_every_participant_implements_within_one_cycle(assignment, host): + """参加者の数のラウンドで、参加者が 1 度ずつ適用担当になる（#727 の決定 7）。""" + participants = assignment.refactor_pool(host) + impls = [assignment.impl_assign(r, participants) for r in range(1, len(participants) + 1)] + assert sorted(impls) == sorted(participants) @pytest.mark.parametrize("host", HOSTS) -def test_every_participant_implements_within_four_rounds(assignment, host): - """4 ラウンドで 4 者が 1 度ずつ適用担当になる（`--max-outer-rounds` の既定と揃う）。""" - impls = [assignment.assign(r, host)[0] for r in range(1, 5)] - assert set(impls) == set(assignment.ALL_RUNTIMES) - assert len(set(impls)) == len(impls), f"同じ担当が 2 度入っている: {impls}" - - -@pytest.mark.parametrize("host", HOSTS) -def test_host_takes_impl_turn_at_least_once(assignment, host): - """ホストは適用にだけ参加する。4 ラウンド回れば必ず 1 度は担当する。""" - impls = {assignment.assign(r, host)[0] for r in range(1, 5)} - assert host in impls - - -def test_reviewers_narrow_to_two_when_impl_is_host(rounds, assignment): - """実装担当がホストと同じラウンドでも、レビュー担当は 3 者にならず 2 者になる。""" - host = "claude" - rounds = [r for r in range(1, 13) if assignment.assign(r, host)[0] == host] - assert rounds, "ホストが実装担当になるラウンドが無い" - for round_no in rounds: - _, reviewers = assignment.assign(round_no, host) - assert len(reviewers) == 2 - - -def test_excluded_reviewer_rotates_across_rounds(assignment): - """余る 1 者はラウンドを跨いで順に外れ、負荷が偏らないこと。""" - host = "claude" - pool = set(assignment.review_pool(host)) - excluded = [] - for round_no in range(1, 13): - impl, reviewers = assignment.assign(round_no, host) - if impl != host: - continue - excluded.append((pool - {impl} - set(reviewers)).pop()) - assert len(set(excluded)) > 1, f"常に同じ 1 者だけが外れている: {excluded}" +def test_the_host_does_not_implement_first(assignment, host): + """ラウンド 1 は参加者の 2 番目から始まる。ホストが最初に適用する形にならない。""" + participants = assignment.refactor_pool(host) + if participants[0] == host: + assert assignment.impl_assign(1, participants) != host @pytest.mark.parametrize("host", HOSTS) def test_assignment_is_deterministic(assignment, host): """再開しても担当が変わらないこと（同じ入力なら同じ結果）。""" + participants = assignment.refactor_pool(host) for round_no in range(1, 13): - assert assignment.assign(round_no, host) == assignment.assign(round_no, host) + assert assignment.impl_assign(round_no, participants) == \ + assignment.impl_assign(round_no, participants) def test_round_number_must_be_positive(assignment): with pytest.raises(assignment.AssignmentError): - assignment.assign(0, "claude") + assignment.impl_assign(0, ["claude", "codex", "kiro"]) -# ---------- 割り当てを直に固定する（#214 / #216） ---------- +# ---------- 固定の順（#214） ---------- # `gemini` があった位置へ `agy` を入れた（#214）。並べ替えると同じラウンド番号でも # 担当が変わり、これまでの記録と突き合わせられなくなる。 -# 適用の母集合を 4 者にしたため（#216）、ラウンド 2 以降の担当が 1 つずつずれる。 -# 適用担当は 4 ラウンドで 1 周し、レビュー担当は適用担当がホストと重なるラウンドで -# 1 者を落とすため 12 ラウンドで 1 周する。読み替えた結果を直に置く。 -EXPECTED_FOR_CLAUDE = { - 1: ("codex", ["agy", "kiro"]), - 2: ("agy", ["codex", "kiro"]), - 3: ("kiro", ["codex", "agy"]), - 4: ("claude", ["codex", "kiro"]), - 5: ("codex", ["agy", "kiro"]), - 6: ("agy", ["codex", "kiro"]), - 7: ("kiro", ["codex", "agy"]), - 8: ("claude", ["codex", "agy"]), - 9: ("codex", ["agy", "kiro"]), - 10: ("agy", ["codex", "kiro"]), - 11: ("kiro", ["codex", "agy"]), - 12: ("claude", ["agy", "kiro"]), -} def test_the_participant_list_keeps_the_replaced_position(assignment): @@ -168,57 +126,24 @@ def test_host_runtimes_covers_every_participant(assignment): assert assignment.HOST_RUNTIMES == assignment.ALL_RUNTIMES -@pytest.mark.parametrize("round_no", sorted(EXPECTED_FOR_CLAUDE)) -def test_the_rotation_matches_the_renamed_result(assignment, round_no): - assert assignment.assign(round_no, "claude") == EXPECTED_FOR_CLAUDE[round_no] - - -# ---------- レビューだけの輪番（cross-review が使う） ---------- - -def test_review_assign_excludes_the_host(assignment): - """母集合はホストを除く 3 者で、返るのは常に 2 者である。""" - for host in assignment.HOST_RUNTIMES: - for round_no in range(1, 10): - picked = assignment.review_assign(round_no, host) - assert len(picked) == 2 - assert host not in picked - assert set(picked) <= set(assignment.review_pool(host)) - - -def test_review_assign_rotates_the_excluded_one(assignment): - """外す 1 者はラウンドごとに回り、3 ラウンドで 1 周する。""" - host = "claude" - pool = assignment.review_pool(host) - dropped = [ - set(pool) - set(assignment.review_assign(r, host)) for r in (1, 2, 3) - ] - assert [next(iter(d)) for d in dropped] == pool - # 4 ラウンド目は 1 ラウンド目と同じ担当へ戻る - assert assignment.review_assign(4, host) == assignment.review_assign(1, host) - - -def test_review_assign_rejects_a_bad_round(assignment): - with pytest.raises(assignment.AssignmentError): - assignment.review_assign(0, "claude") - +# ---------- 席の埋め方と席の名前（#727。cross-review が使う） ---------- -def test_review_assign_rejects_an_unknown_host(assignment): - with pytest.raises(assignment.AssignmentError): - assignment.review_assign(1, "gemini") +def _previous_review_rotation(round_no: int, pool: list[str]) -> list[str]: + """席の埋め方より前の輪番。3 者の母集合から `(round_no - 1) % 3` の者を外した 2 者。 + 関数は消えたため、式を期待値として持つ（AC8 の主張を保つ）。 + """ + dropped = (round_no - 1) % len(pool) + return [r for i, r in enumerate(pool) if i != dropped] -# ---------- 席の埋め方と席の名前（#727。cross-review が使う） ---------- -# -# 変更前の席の割り当て（`review_assign`）を期待値に使えるよう、同じファイルに置く。 -# `review_assign` のテストは P7 で消す。 -def test_review_seats_match_review_assign_for_three_available(assignment): +def test_review_seats_match_the_previous_rotation_for_three_available(assignment): """AC8: 使える者が 3 者のとき、変更前の輪番と同じ値になる（4 ホスト × ラウンド 1〜12）。""" for host in assignment.HOST_RUNTIMES: pool = assignment.review_pool(host) for round_no in range(1, 13): assert assignment.review_seats(round_no, pool, []) == \ - assignment.review_assign(round_no, host), f"host={host} round={round_no}" + _previous_review_rotation(round_no, pool), f"host={host} round={round_no}" def test_review_seats_with_four_available_give_each_two_turns(assignment): diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_init.py b/plugins/ndf/skills/cross-refactoring/tests/test_init.py index adc7e054d..a63d844e1 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_init.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_init.py @@ -77,8 +77,10 @@ def run_init(refactor_lib, paths, patch_lib, refactor, origin_repo, monkeypatch) 常に `author` なので、両者を一致させると自分の Pull Request になる。 """ refactor_lib = sys.modules["refactor_lib"] + probed: list[list[str]] = [] - def _run(args, viewer="someone-else"): + def _run(args, viewer="someone-else", probe=None): + """`probe` を渡すと確認を差し替える。`{ランタイム: 理由}` の者だけが通らない。""" real_sh = paths.sh def fake_sh(cmd, cwd=None, check=True): @@ -108,10 +110,24 @@ def fake_sh(cmd, cwd=None, check=True): patch_lib("sh", fake_sh) monkeypatch.chdir(origin_repo) monkeypatch.delenv("CROSS_REFACTORING_TMP_DIR", raising=False) - # 認証確認は実際の CLI を起動する。ここでは対象外なので飛ばす - # （確認そのものは `test_init_checks_cli_authentication` で見る）。 - monkeypatch.setenv("NDF_SKIP_AUTH_CHECK", "1") + # 認証確認は実際の CLI を起動する。既定では飛ばし、`probe` を渡したときだけ + # 止めない確認（`probe_auth`）を差し替えて結果を決める。 + if probe is None: + monkeypatch.setenv("NDF_SKIP_AUTH_CHECK", "1") + else: + monkeypatch.delenv("NDF_SKIP_AUTH_CHECK", raising=False) + cmd_setup = sys.modules["refactor_lib.commands.setup"] + probed.clear() + + def fake_probe(runtimes, *, info, env=None): + names = list(runtimes) + probed.append(names) + return {n: {"command": n, "ok": n not in probe, "detail": probe.get(n, "")} + for n in names}, False + + monkeypatch.setattr(cmd_setup.auth, "probe_auth", fake_probe) refactor.cmd_init(args) + _run.probed = probed return _run @@ -120,9 +136,13 @@ def refactor_abort(): return 4 -def _state_of(tmp_path): - path = (tmp_path / "rf130" / "work" / ".cross_refactoring" +def _state_path(tmp_path): + return (tmp_path / "rf130" / "work" / ".cross_refactoring" / "cross-refactoring-rf130-state.json") + + +def _state_of(tmp_path): + path = _state_path(tmp_path) return path, json.loads(path.read_text(encoding="utf-8")) @@ -136,15 +156,82 @@ def test_init_creates_the_writable_worktree_from_origin(run_init, tmp_path): assert head.stdout.strip() == HEAD_BRANCH -def test_init_records_cohorts_separately(run_init, tmp_path): - """提案・レビューと適用の母集合は別物である。""" - run_init(_args(tmp_path)) +def test_init_uses_codex_kiro_and_the_host_as_the_participants(run_init, tmp_path, capsys): + """AC31 — 既定の参加者は codex / kiro とホスト。agy は確かめず、母集合は 1 つだけ。""" + run_init(_args(tmp_path), probe={}) _, state = _state_of(tmp_path) - assert state["runtimes"] == ["codex", "agy", "kiro"] - assert state["impl_capable"] == ["claude", "codex", "agy", "kiro"] + assert state["runtimes"] == ["claude", "codex", "kiro"] + assert run_init.probed == [["claude", "codex", "kiro"]], "agy を確かめている" + assert "impl_capable" not in state + assert "IMPL_POOL=" not in capsys.readouterr().out + assert state["participants"]["available"] == ["claude", "codex", "kiro"] + assert state["participants"]["pool"] == ["claude", "codex", "kiro"] + assert state["resume_changes"] == [] assert state["host"] == "claude" assert state["host_detection"] == "explicit" - assert state["host"] not in state["runtimes"] + + +@pytest.mark.parametrize("host, expected", [ + ("codex", ["codex", "kiro"]), + ("agy", ["codex", "agy", "kiro"]), + ("kiro", ["codex", "kiro"]), +]) +def test_the_participants_follow_the_host(run_init, tmp_path, host, expected): + """AC32 — ホストが既定の参加者の表にいれば 2 者、いなければ 3 者になる。""" + run_init(_args(tmp_path, host=host), probe={}) + assert _state_of(tmp_path)[1]["runtimes"] == expected + + +@pytest.mark.parametrize("over, expected", [ + ({"include": [["agy"]]}, ["claude", "codex", "agy", "kiro"]), + ({"exclude": [["kiro"]]}, ["claude", "codex"]), + ({"exclude": [["claude"]]}, ["codex", "kiro"]), +]) +def test_include_and_exclude_change_the_participants(run_init, tmp_path, over, expected): + """AC33 — 足す者・外す者で名指しで変えられる。ホストも母集合にいるので外せる。""" + run_init(_args(tmp_path, **over), probe={}) + _, state = _state_of(tmp_path) + assert state["runtimes"] == expected + assert state["participants"]["included"] == over.get("include", [[]])[0] + assert state["participants"]["excluded"] == over.get("exclude", [[]])[0] + + +def test_a_failed_probe_drops_the_runtime_and_keeps_going(run_init, tmp_path, capsys): + """AC35 — 1 者の確認が通らなくても止めず、理由を残して使える者で始める。""" + run_init(_args(tmp_path), probe={"kiro": "Not logged in"}) + _, state = _state_of(tmp_path) + assert state["runtimes"] == ["claude", "codex"] + assert state["participants"]["unavailable"] == {"kiro": "Not logged in"} + assert "kiro を担当から外しました（Not logged in）" in capsys.readouterr().err + + +def test_require_all_stops_without_writing_the_state(run_init, tmp_path): + """AC35 — 全員を要する指定では従来の関門で止め、状態ファイルを作らない。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, require_all=True), probe={"kiro": "Not logged in"}) + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() + + +def test_no_available_runtime_stops_without_writing_the_state(run_init, tmp_path, capsys): + """AC36 — 使える者が 0 者なら終了コード 4 で止め、状態ファイルを作らない。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path), probe={"claude": "x", "codex": "y", "kiro": "z"}) + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() + assert "使える者がいません" in capsys.readouterr().err + + +@pytest.mark.parametrize("over", [ + {"exclude": [["agy"]]}, # 母集合に無い者は外せない + {"include": [["agy"]], "exclude": [["agy"]]}, # 足す者と外す者の重なり +]) +def test_contradicting_names_stop_the_init(run_init, tmp_path, over): + """名前の矛盾は共通層が弾き、この工程の中断（終了コード 4）へ写す。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, **over), probe={}) + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() def test_init_records_models(run_init, tmp_path): @@ -175,18 +262,26 @@ def test_init_warns_when_kiro_is_given_auto_explicitly(run_init, tmp_path, capsy def test_init_warns_when_codex_or_agy_has_no_model(run_init, tmp_path, capsys): - """実測できないランタイムで指定が無いラウンドも、kiro の auto と同じく分離される。""" + """実測できないランタイムで指定が無いラウンドも、kiro の auto と同じく分離される。 + + 警告の対象は参加者だけである。既定で外れる agy は、足したときだけ警告する。 + """ run_init(_args(tmp_path, model=["kiro=claude-opus-5"])) warning = capsys.readouterr().err assert "codex のモデルが default です" in warning - assert "agy のモデルが default です" in warning + assert "agy のモデルが" not in warning + + +def test_init_warns_about_agy_when_it_is_included(run_init, tmp_path, capsys): + run_init(_args(tmp_path, model=["kiro=claude-opus-5"], include=[["agy"]])) + assert "agy のモデルが default です" in capsys.readouterr().err def test_init_does_not_warn_when_every_model_can_be_measured(run_init, tmp_path, capsys): """claude だけは指定が無くても実測できるため、警告の対象にならない。""" run_init(_args(tmp_path, model=[ "codex=gpt-5.5", "agy=gemini-3.8", "kiro=claude-opus-5", - ])) + ], include=[["agy"]])) assert "集計から分離されます" not in capsys.readouterr().err @@ -200,8 +295,7 @@ def test_init_accepts_agy_as_host(run_init, tmp_path): run_init(_args(tmp_path, host="agy")) _, state = _state_of(tmp_path) assert state["host"] == "agy" - assert state["runtimes"] == ["claude", "codex", "kiro"] - assert state["impl_capable"] == ["claude", "codex", "agy", "kiro"] + assert state["runtimes"] == ["codex", "agy", "kiro"] def test_init_runs_the_baseline_test(run_init, tmp_path): @@ -256,19 +350,44 @@ def _parsed_init_args(patch_lib, refactor, monkeypatch, *extra): return captured -def test_the_round_caps_have_their_own_defaults(patch_lib, refactor, monkeypatch): +def test_the_round_caps_are_unset_in_the_arguments(patch_lib, refactor, monkeypatch): + """引数の既定は未指定で、再開で「渡さなかった」と読める（#727 の決定 13）。""" + captured = _parsed_init_args(patch_lib, refactor, monkeypatch) + for key in ("max_test_rounds", "max_outer_rounds", "max_fix_rounds", + "max_items_per_round", "test_timeout", "severity_threshold", + "workflow_step", "include", "exclude", "require_all"): + assert captured[key] is None, key + + +def test_a_new_run_fills_the_round_caps_with_their_defaults(run_init, tmp_path): """E1 — 4 つの上限は別々の単位に掛かる（#436 決定 8）。 - `--max-outer-rounds` が 3 でよいのは、適用ラウンドを分けたことで**1 回の提案で - 通せる件数が上限に縛られなくなった**ためである。輪番の 1 周を根拠にしない - （適用の担当は適用ラウンドごとに進むので、1 つの提案ラウンドでも輪番は 1 周 - しうる）。 + 新規の初期化が現行の既定（提案 3 / テスト整備 2 / 修正 3 / 採用 5）へ置き換える。 """ - captured = _parsed_init_args(patch_lib, refactor, monkeypatch) - assert captured["max_test_rounds"] == 2 - assert captured["max_outer_rounds"] == 3 - assert captured["max_fix_rounds"] == 3 - assert captured["max_items_per_round"] == 5 + run_init(_args(tmp_path, max_outer_rounds=None, max_test_rounds=None, + max_fix_rounds=None, max_items_per_round=None, + test_timeout=None, severity_threshold=None, workflow_step=None)) + _, state = _state_of(tmp_path) + assert (state["max_outer_rounds"], state["max_test_rounds"], + state["max_fix_rounds"], state["max_items_per_round"]) == (3, 2, 3, 5) + assert state["test_timeout"] == 900 + assert state["severity_threshold"] == "minor" + assert state["workflow_step"] is False + + +def test_include_and_exclude_parse_names_and_none(patch_lib, refactor, monkeypatch): + """カンマ区切りと繰り返しの両方を受ける。綴りの誤りは argparse が弾く。""" + captured = _parsed_init_args(patch_lib, refactor, monkeypatch, + "--exclude", "kiro", "--include", "agy,claude", + "--require-all") + assert captured["exclude"] == [["kiro"]] + assert captured["include"] == [["agy", "claude"]] + assert captured["require_all"] is True + assert _parsed_init_args(patch_lib, refactor, monkeypatch, + "--exclude", "none")["exclude"] == [["none"]] + with pytest.raises(SystemExit) as e: + _parsed_init_args(patch_lib, refactor, monkeypatch, "--exclude", "gemini") + assert e.value.code == 2 def test_the_ci_check_is_not_set_by_default(patch_lib, refactor, monkeypatch): @@ -319,10 +438,10 @@ def test_init_is_idempotent(run_init, tmp_path, capsys): def test_init_emits_shell_assignments(run_init, tmp_path, capsys): run_init(_args(tmp_path)) out = capsys.readouterr().out - assert "RUNTIMES_CSV=codex,agy,kiro" in out + assert "RUNTIMES_CSV=claude,codex,kiro" in out # 空白を含む値は必ず引用する。引用しないと呼び出し側の eval で語が割れる。 - assert "RUNTIMES='codex agy kiro'" in out - assert "IMPL_POOL='claude codex agy kiro'" in out + assert "RUNTIMES='claude codex kiro'" in out + assert "IMPL_POOL=" not in out assert "TMP_DIR=" in out and "WORK=" in out @@ -421,161 +540,95 @@ def test_init_records_the_vocabulary_for_the_prompt(run_init, tmp_path, vocabula assert state["vocabulary"]["smells"] == vocabulary.SMELLS -def _probe_result(cmd_setup, refactor, monkeypatch, outcomes): - """認証確認コマンドの結果を差し替える。`{ランタイム: (rc, 出力)}`。""" - def fake_run(cmd, **kwargs): - for runtime, probe in cmd_setup.auth.AUTH_PROBES.items(): - if list(cmd) == list(probe): - rc, out = outcomes.get(runtime, (0, "ok")) - return subprocess.CompletedProcess(cmd, rc, out, "") - raise AssertionError(f"想定外の呼び出し: {cmd}") - monkeypatch.setattr(cmd_setup.auth.subprocess, "run", fake_run) - - -def test_check_auth_passes_when_every_cli_is_logged_in(refactor, cmd_setup, monkeypatch): - monkeypatch.delenv("NDF_SKIP_AUTH_CHECK", raising=False) - _probe_result(cmd_setup, refactor, monkeypatch, {}) - results = cmd_setup.check_auth(["claude", "codex", "agy", "kiro"]) - assert all(r["ok"] for r in results.values()) - - -def test_check_auth_fails_on_a_non_zero_exit(refactor_lib, cmd_setup, refactor, monkeypatch): - monkeypatch.delenv("NDF_SKIP_AUTH_CHECK", raising=False) - _probe_result(cmd_setup, refactor, monkeypatch, {"kiro": (1, "")}) - with pytest.raises(SystemExit) as e: - cmd_setup.check_auth(["claude", "codex", "agy", "kiro"]) - assert e.value.code == refactor_abort() - - -def test_check_auth_fails_when_the_output_says_not_logged_in(refactor, cmd_setup, monkeypatch): - """終了コード 0 でも未認証を示すことがある（kiro は成否を終了コードで表さない）。""" - monkeypatch.delenv("NDF_SKIP_AUTH_CHECK", raising=False) - _probe_result(cmd_setup, refactor, monkeypatch, {"kiro": (0, "Not logged in")}) - with pytest.raises(SystemExit): - cmd_setup.check_auth(["claude", "codex", "agy", "kiro"]) - - -def test_check_auth_fails_when_the_cli_is_missing(refactor, cmd_setup, monkeypatch): - cmd_setup = sys.modules["refactor_lib.commands.setup"] - monkeypatch.delenv("NDF_SKIP_AUTH_CHECK", raising=False) - - def missing(cmd, **kwargs): - raise FileNotFoundError(cmd[0]) - - monkeypatch.setattr(cmd_setup.auth.subprocess, "run", missing) - with pytest.raises(SystemExit): - cmd_setup.check_auth(["codex"]) - - -def test_check_auth_can_be_skipped_explicitly(refactor, cmd_setup, monkeypatch): - """確認コマンドは CLI の版で変わる。飛ばせる逃げ道を残す。""" - monkeypatch.setenv("NDF_SKIP_AUTH_CHECK", "1") - - def never(cmd, **kwargs): - raise AssertionError("認証確認を実行してはいけない") - - monkeypatch.setattr(cmd_setup.auth.subprocess, "run", never) - assert cmd_setup.check_auth(["codex", "agy"]) == {} - - -def test_init_checks_cli_authentication(patch_lib, refactor, cmd_setup, origin_repo, monkeypatch, tmp_path): - """未認証の CLI があれば初期化ごと中断すること。 - - 参加者が 1 人欠けた構成のまま進むと、その者の提案とレビューが無いまま収束する。 - """ - monkeypatch.delenv("NDF_SKIP_AUTH_CHECK", raising=False) - monkeypatch.chdir(origin_repo) - monkeypatch.delenv("CROSS_REFACTORING_TMP_DIR", raising=False) - _probe_result(cmd_setup, refactor, monkeypatch, {"agy": (1, "Authentication failed")}) - patch_lib("sh", - lambda cmd, **k: pytest.fail("認証確認より前に gh を呼んでいる"), - ) - with pytest.raises(SystemExit) as e: - cmd_setup.cmd_init(_args(tmp_path)) - assert e.value.code == refactor_abort() - - -def test_init_downgrades_the_posting_event_on_own_pull_request(run_init, tmp_path): - """自分の Pull Request では投稿の event を `COMMENT` へ倒すこと。 - - GitHub は自分の Pull Request への `APPROVE` と `REQUEST_CHANGES` を - `HTTP 422` で拒む。倒さないとレビュー担当が投稿に失敗する。 - """ - run_init(_args(tmp_path), viewer="me") - _, state = _state_of(tmp_path) - assert state["is_own_pr"] is True - assert state["event_downgrade"] is True - - -def test_init_keeps_the_posting_event_on_someone_elses_pull_request(run_init, tmp_path): - """他者の Pull Request では判定をそのまま投稿すること。""" - run_init(_args(tmp_path), viewer="someone-else") - _, state = _state_of(tmp_path) - assert state["is_own_pr"] is False - assert state["event_downgrade"] is False - - -def test_init_continues_when_the_viewer_cannot_be_read(run_init, tmp_path): - """ログイン名を読めない環境でも `init` を続けること。 - - bot トークン（Actions の `GITHUB_TOKEN` など）は `/user` を読めず - `HTTP 403` を返す。この値は自分の Pull Request かどうかの判定にしか - 使わないので、読めなければ他者の Pull Request として扱う。 - """ - run_init(_args(tmp_path), viewer=None) - _, state = _state_of(tmp_path) - assert state["is_own_pr"] is False - assert state["event_downgrade"] is False - - -def test_init_fills_the_posting_event_when_resuming_an_old_state(run_init, tmp_path): - """この指示が入る前の状態ファイルから再開しても投稿の event を倒すこと。 - - 再開の分岐は状態ファイルをそのまま使って戻る。項目が無い状態ファイルを - そのまま渡すと、起動側は空の指示を読み、自分の Pull Request で - `HTTP 422` を踏み続ける。 - """ - run_init(_args(tmp_path), viewer="me") - path, state = _state_of(tmp_path) - # 旧版が書いた状態ファイル（2 項目が無い）を再現する - for key in ("is_own_pr", "event_downgrade"): - state.pop(key) - state["outer_round"] = 2 - path.write_text(json.dumps(state, ensure_ascii=False), encoding="utf-8") - - run_init(_args(tmp_path), viewer="me") - - _, resumed = _state_of(tmp_path) - assert resumed["outer_round"] == 2, "再開であって初期化ではないこと" - assert resumed["is_own_pr"] is True - assert resumed["event_downgrade"] is True +# ---------- 再開（#727 / #648 の決定 13〜16） ---------- -# ---------- 改修計画の書き出し先 ---------- - -def test_init_defaults_the_plan_to_a_pull_request_comment(run_init, tmp_path): - """D4 — 既定は Pull Request のコメント 1 件（#436 決定 6）。 - - **改修計画は実行の記録であって、リポジトリの知識ではない。** ファイルに - すると差分に混ざり、URL がブランチの後片付けで切れる。 - """ +@pytest.mark.parametrize("arg, value", [ + ("max_outer_rounds", 5), ("max_test_rounds", 4), + ("max_fix_rounds", 6), ("max_items_per_round", 8), +]) +def test_resume_reflects_a_changed_cap(run_init, tmp_path, capsys, arg, value): + """AC38 — 上限は再開で渡せば反映し、`旧 → 新` を 1 行出し、記録に 1 件積む。""" run_init(_args(tmp_path)) + _, before = _state_of(tmp_path) + capsys.readouterr() + + run_init(_args(tmp_path, **{arg: value})) + _, after = _state_of(tmp_path) + assert after[arg] == value + assert f"{arg}: {before[arg]} → {value}" in capsys.readouterr().err + assert [c["field"] for c in after["resume_changes"]] == [arg] + + +@pytest.mark.parametrize("over, option", [ + ({"model": ["codex=x"]}, "--model"), + ({"host": "codex"}, "--host"), + ({"scope": ["other", "tests"]}, "--scope"), + ({"baseline_test": "pytest -q"}, "--baseline-test"), + ({"severity_threshold": "major"}, "--severity-threshold"), +]) +def test_resume_notifies_arguments_it_does_not_reflect( + run_init, tmp_path, capsys, origin_repo, over, option): + """AC39 — 反映しない引数は状態を変えず、引数ごとに 1 行知らせる。""" + (origin_repo / "other").mkdir(exist_ok=True) + run_init(_args(tmp_path)) + path, before = _state_of(tmp_path) + capsys.readouterr() + + run_init(_args(tmp_path, **over)) + _, after = _state_of(tmp_path) + err = capsys.readouterr().err + assert f"ℹ {option} は再開では反映しません" in err + for key in ("models", "host", "target_scope", "baseline_test", "severity_threshold"): + assert after[key] == before[key], key + assert after["resume_changes"] == [] + + +def test_resume_without_arguments_changes_nothing(run_init, tmp_path, capsys): + """AC39 — 何も渡さない再開では、上限・モデル・参加者が変わらず、確認もしない。""" + run_init(_args(tmp_path), probe={}) + _, before = _state_of(tmp_path) + + run_init(_args(tmp_path), probe={"kiro": "Not logged in"}) + _, after = _state_of(tmp_path) + assert run_init.probed == [], "担当に関わる引数を渡していないのに確かめ直している" + for key in ("max_outer_rounds", "max_test_rounds", "max_fix_rounds", + "max_items_per_round", "models", "runtimes", "participants"): + assert after[key] == before[key], key + assert "再開では反映しません" not in capsys.readouterr().err + + +def test_resume_with_exclude_rebuilds_the_participants(run_init, tmp_path): + """AC40 — 外す者を渡した再開では確かめ直し、渡さなかった足す者は記録から補う。""" + run_init(_args(tmp_path, include=[["agy"]]), probe={}) + run_init(_args(tmp_path, exclude=[["kiro"]]), probe={}) _, state = _state_of(tmp_path) - assert state["plan_mode"] == "comment" - assert state["plan_file"] == "" - - -def test_init_keeps_an_explicit_plan_file(run_init, tmp_path): - """**`--plan-file` は残す。** 明示したときだけファイルにする。""" - run_init(_args(tmp_path, plan_file="docs/plan.md")) + assert run_init.probed == [["claude", "codex", "agy"]] + assert state["runtimes"] == ["claude", "codex", "agy"] + assert state["participants"]["included"] == ["agy"] + assert state["participants"]["excluded"] == ["kiro"] + changes = state["resume_changes"] + assert [c["field"] for c in changes] == ["participants"], "作り直しは 1 件として積む" + assert changes[0]["from"]["available"] == ["claude", "codex", "agy", "kiro"] + + +def test_resume_with_none_clears_the_recorded_names(run_init, tmp_path): + """予約語 `none` は記録の一覧を空へ戻す（決定 15）。""" + run_init(_args(tmp_path, exclude=[["kiro"]]), probe={}) + run_init(_args(tmp_path, exclude=[["none"]]), probe={}) _, state = _state_of(tmp_path) - assert state["plan_mode"] == "file" - assert state["plan_file"] == "docs/plan.md" + assert state["participants"]["excluded"] == [] + assert state["runtimes"] == ["claude", "codex", "kiro"] -def test_init_accepts_an_empty_plan_file_as_off(run_init, tmp_path): - """計画を残したくないリポジトリのために、空文字で無効にできる。""" - run_init(_args(tmp_path, plan_file="")) - _, state = _state_of(tmp_path) - assert state["plan_mode"] == "none" - assert state["plan_file"] == "" +def test_a_failed_rebuild_leaves_the_state_untouched(run_init, tmp_path): + """作り直しが 0 者なら終了コード 4 で止め、状態ファイルを書き換えない。""" + run_init(_args(tmp_path), probe={}) + path, _ = _state_of(tmp_path) + before = path.read_text(encoding="utf-8") + + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, exclude=[["kiro"]], max_outer_rounds=9), + probe={"claude": "x", "codex": "y"}) + assert e.value.code == refactor_abort() + assert path.read_text(encoding="utf-8") == before 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 49cc39ba6..4a20f6135 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 @@ -343,8 +343,7 @@ def test_models_are_fixed_across_rounds(cmd_setup, tmp_path, env_tmp_dir): state = read_state(state_path) entry = state["rounds"][-1] assert entry["impl_model"]["requested"] == state["models"][entry["impl"]] - for r in entry["reviewers"]: - assert entry["reviewer_models"][r]["requested"] == state["models"][r] + assert "reviewer_models" not in entry # 次のラウンドを開けるように、いま開いたラウンドを閉じる entry["adopted"] = 1 state_path.write_text(json.dumps(state, ensure_ascii=False), encoding="utf-8") diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_plan_comment.py b/plugins/ndf/skills/cross-refactoring/tests/test_plan_comment.py index a37e8522f..077f3894c 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_plan_comment.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_plan_comment.py @@ -134,6 +134,14 @@ def test_the_body_carries_the_marker_and_the_plan(plan, tmp_path): assert "R1-001" in body and "src/foo.py" in body +def test_the_round_heading_has_no_reviewers(plan, tmp_path): + """AC37 — 改修計画のラウンドの見出しにレビュー担当を出さない。古い記録にあっても。""" + _, state = _state(tmp_path) + body = plan.plan_comment_body(state) + assert "## ラウンド 1（実装 codex）" in body + assert "レビュー" not in body.split("## ラウンド 1", 1)[1].splitlines()[0] + + def test_a_failed_post_does_not_stop_the_run(plan, tmp_path, gh): """記録が残らないことと、変更が検証を通っていないことは別である。""" _, state = _state(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 f7221bb63..7017e2338 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py @@ -28,16 +28,16 @@ def round_of(round_no, **over): # ---------- start-round ---------- def test_start_round_opens_and_records_assignment(refactor, tmp_path, env_tmp_dir): - state_path = make_state(tmp_path) + state_path = make_state(tmp_path, runtimes=["claude", "codex", "kiro"]) env_tmp_dir(state_path) refactor.cmd_start_round(_args()) state = read_state(state_path) assert len(state["rounds"]) == 1 entry = state["rounds"][0] + # ラウンド 1 は参加者の 2 番目から始まり、ホストが最初に適用しない（#727 の決定 7） assert entry["impl"] == "codex" - assert entry["reviewers"] == ["agy", "kiro"] - assert entry["impl"] not in entry["reviewers"] + assert "reviewers" not in entry assert state["phase"] == "propose" @@ -156,10 +156,75 @@ def test_report_renders_tables(cmd_report, tmp_path, env_tmp_dir, capsys): assert "R1-001" in out -def test_status_reports_cohorts(cmd_report, tmp_path, env_tmp_dir, capsys): - state_path = make_state(tmp_path) +def test_status_reports_one_cohort(cmd_report, tmp_path, env_tmp_dir, capsys): + """AC37 — 母集合は 1 行で出す。提案と適用で分けない（#727 の決定 5）。""" + state_path = make_state(tmp_path, runtimes=["claude", "codex", "kiro"]) + env_tmp_dir(state_path) + cmd_report.cmd_status(_args()) + out = capsys.readouterr().out + assert "参加者（提案と適用）: claude / codex / kiro" in out + assert "提案・レビュー" not in out + assert "適用の母集合" not in out + + +def test_status_reads_an_older_state_with_the_implementation_cohort( + cmd_report, tmp_path, env_tmp_dir, capsys): + """AC41 — 適用専用の母集合を持つ古い状態ファイルも読める。表示は参加者の一覧だけ。""" + state_path = make_state(tmp_path, impl_capable=["claude", "codex", "agy", "kiro"]) env_tmp_dir(state_path) cmd_report.cmd_status(_args()) + assert "参加者（提案と適用）: codex / agy / kiro" in capsys.readouterr().out + + +def _report_args(): + return type("A", (), {"id": 130, "metrics": False})() + + +def test_report_has_no_reviewer_column(cmd_report, tmp_path, env_tmp_dir, capsys): + """AC37 — ラウンド表にレビュー担当の列が無い。古い記録にあっても出さない。""" + state_path = make_state(tmp_path, rounds=[{ + "round": 1, "kind": "structure", "impl": "codex", + "impl_model": {"requested": None, "observed": None}, + "reviewers": ["agy", "kiro"], "reviewer_models": {}, "adopted": 1, + "apply": {"applied": [], "failed": []}, "fix_rounds": 0, "reviews": [], + }]) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + out = capsys.readouterr().out + header = next(line for line in out.splitlines() if line.startswith("| R |")) + assert "レビュー担当" not in header + assert "| 1 | 構造改善 | codex |" in out + assert "agy / kiro" not in out + + +def test_report_prints_the_participants(cmd_report, tmp_path, env_tmp_dir, capsys): + """F6 — 参加者・外した者・足した者・確認を通らなかった者・再開で変えた値を出す。""" + state_path = make_state( + tmp_path, runtimes=["claude", "codex"], + participants={ + "pool": ["claude", "codex", "kiro"], "included": ["agy"], + "excluded": ["kiro"], "available": ["claude", "codex"], + "unavailable": {"agy": "Not logged in"}, + "probe_skipped": False, "require_all": False, + }, + resume_changes=[{"at": "2026-09-22T00:00:00", "field": "max_outer_rounds", + "from": 3, "to": 5}], + ) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) out = capsys.readouterr().out - assert "提案・レビュー: codex / agy / kiro" in out - assert "適用の母集合: claude / codex / kiro" in out + assert "## 参加した者" in out + assert "- 母集合: claude / codex / kiro" in out + assert "- 使える者: claude / codex" in out + assert "- --exclude で外した者: kiro" in out + assert "- --include で足した者: agy" in out + assert "- 確認を通らなかった者: agy（Not logged in）" in out + assert "max_outer_rounds: 3 → 5" in out + + +def test_report_says_no_record_for_an_older_state(cmd_report, tmp_path, env_tmp_dir, capsys): + """AC41 — 参加者の記録を持たない古い状態ファイルでは「記録なし」と出す。""" + state_path = make_state(tmp_path, impl_capable=["claude", "codex", "agy", "kiro"]) + env_tmp_dir(state_path) + cmd_report.cmd_report(_report_args()) + assert "- 使える者: 記録なし" in capsys.readouterr().out diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_start_round_emits_runtimes.py b/plugins/ndf/skills/cross-refactoring/tests/test_start_round_emits_runtimes.py index e783be161..3ff6a3a77 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_start_round_emits_runtimes.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_start_round_emits_runtimes.py @@ -1,10 +1,11 @@ -"""`start-round` が提案・レビューの母集合を返すこと（#518-1）。 +"""`start-round` が参加者の一覧と実装担当を返すこと（#518-1 / #727）。 **繰り返しの中で使う値は、繰り返しの中で得られる。** 母集合を `init` だけが返すと、 状態ファイルから再開する経路と、骨組みを抜粋して写す経路の両方で未定義になる。 """ from __future__ import annotations +import json import shlex import pytest @@ -56,18 +57,33 @@ def test_existing_values_are_untouched(refactor, tmp_path, env_tmp_dir, capsys): emitted = _emitted(capsys) for key in ( - "ROUND", "ROUND_KIND", "PROPOSE_PHASE", "IMPL", "IMPL_MODEL", - "REVIEWERS", "REVIEWERS_CSV", "MAX_FIX_ROUNDS", + "ROUND", "ROUND_KIND", "PROPOSE_PHASE", "IMPL", "IMPL_MODEL", "MAX_FIX_ROUNDS", ): assert key in emitted, f"{key} が出力から消えている" -def test_the_pool_is_wider_than_the_reviewers(refactor, tmp_path, env_tmp_dir, capsys): - """`REVIEWERS` で代用すると提案する者が 1 人減る。""" - state_path = make_state(tmp_path, runtimes=["codex", "agy", "kiro"]) +def test_start_round_has_no_reviewers(refactor, tmp_path, env_tmp_dir, capsys): + """AC34 — レビュー工程は #436 で消えた。存在しない役を出力にも記録にも残さない。""" + state_path = make_state(tmp_path, runtimes=["claude", "codex", "kiro"]) env_tmp_dir(state_path) refactor.cmd_start_round(_args()) emitted = _emitted(capsys) - assert len(emitted["RUNTIMES"].split()) == 3 - assert len(emitted["REVIEWERS"].split()) == 2 + assert "REVIEWERS" not in emitted + assert "REVIEWERS_CSV" not in emitted + entry = json.loads(state_path.read_text(encoding="utf-8"))["rounds"][0] + assert "reviewers" not in entry + assert "reviewer_models" not in entry + + +def test_the_implementer_rotates_within_the_participants( + refactor, tmp_path, env_tmp_dir, capsys): + """AC34 / AC41 — 実装担当は参加者の一覧から決まる。適用専用の母集合は読まない。""" + state_path = make_state(tmp_path, runtimes=["claude", "codex", "kiro"], + impl_capable=["claude", "codex", "agy", "kiro"]) + env_tmp_dir(state_path) + impls = [] + for _ in range(3): + refactor.cmd_start_round(_args()) + impls.append(_emitted(capsys)["IMPL"]) + assert impls == ["codex", "kiro", "claude"] diff --git a/plugins/ndf/skills/cross-review/tests/test_rejected_findings.py b/plugins/ndf/skills/cross-review/tests/test_rejected_findings.py index 1041e1923..822a81972 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rejected_findings.py +++ b/plugins/ndf/skills/cross-review/tests/test_rejected_findings.py @@ -92,7 +92,8 @@ def test_init_stores_an_empty_rejected_findings_list(tmp_dir, state_mod, monkeyp monkeypatch.setattr(state_mod, "_sync_worktree", lambda *args: None) monkeypatch.setattr(state_mod.subprocess, "run", lambda *args, **kwargs: subprocess.CompletedProcess(args[0], 0, stdout="", stderr="")) - monkeypatch.setattr(state_mod.auth, "check_auth", lambda *args, **kwargs: None) + monkeypatch.setattr(state_mod.auth, "probe_auth", lambda runtimes, **kwargs: ( + {r: {"command": r, "ok": True, "detail": ""} for r in runtimes}, False)) state_mod.cmd_init(argparse.Namespace( pr=PR, max_rounds=12, rotate_after=8, only=None, worktree=str(worktree), diff --git a/plugins/ndf/skills/cross-review/tests/test_state_review_pool.py b/plugins/ndf/skills/cross-review/tests/test_state_review_pool.py index df13827ff..4fe3d06bc 100644 --- a/plugins/ndf/skills/cross-review/tests/test_state_review_pool.py +++ b/plugins/ndf/skills/cross-review/tests/test_state_review_pool.py @@ -549,9 +549,10 @@ def test_a_state_without_participants_keeps_the_old_rotation(state_mod, tmp_path """AC22: `participants` が無くても、`host` があれば変更前の輪番と同じ値を返す。""" path = _state(tmp_path, host="codex") st = json.loads(path.read_text(encoding="utf-8")) + # 変更前の輪番: 母集合 claude / agy / kiro から `(round_no - 1) % 3` の者を外した 2 者 + previous = [["agy", "kiro"], ["claude", "kiro"], ["claude", "agy"]] for round_no in range(1, 7): - assert state_mod._round_reviewers(st, round_no) == \ - state_mod.assignment.review_assign(round_no, "codex") + assert state_mod._round_reviewers(st, round_no) == previous[(round_no - 1) % 3] del st["host"] assert state_mod._round_reviewers(st, 1) == ["codex", "agy"] diff --git a/scripts/tests/test_shared_lib_layout.py b/scripts/tests/test_shared_lib_layout.py index 735fe6069..569e0c510 100644 --- a/scripts/tests/test_shared_lib_layout.py +++ b/scripts/tests/test_shared_lib_layout.py @@ -322,3 +322,24 @@ def test_the_identifier_still_has_to_be_an_integer() -> None: out = _monitor("10.2.0", "--agents", "deploy") assert out.returncode != 0 assert "invalid int value" in out.stderr + + +# ---------- 呼び手の無くなった旧関数（#727 の AC7） ---------- + +# 使える者の解決と席・適用の割り当てが共通層の新しい関数へ移り、呼び手が無くなった 4 つ。 +# **片方の Skill にだけ古い形が残らない**（親 #727 の完了条件）ことを、名前が残らない +# ことで固定する。部品（`scripts/`）だけを見る。テストと文書は古い値を期待値や経緯として +# 持ちうる。 +RETIRED_FUNCTIONS = ("check_auth", "impl_pool", "review_assign", "assign") + + +def test_the_retired_assignment_functions_are_gone() -> None: + result = subprocess.run( + ["git", "grep", "-n", "-w", + *[arg for name in RETIRED_FUNCTIONS for arg in ("-e", name)], + "--", "plugins/ndf/scripts/lib/", + "plugins/ndf/skills/cross-review/scripts/", + "plugins/ndf/skills/cross-refactoring/scripts/"], + cwd=ROOT, capture_output=True, text=True, + ) + assert result.returncode == 1, f"旧関数の名前が残っている:\n{result.stdout}" From 5bfb08e259f593fc21e0d6f85e8126fec0775e7d Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 09:48:47 +0000 Subject: [PATCH 2/9] =?UTF-8?q?Docs:=20cross-refactoring=20=E3=81=AE?= =?UTF-8?q?=E6=89=8B=E9=A0=86=E6=9B=B8=E3=81=A8=E6=8C=87=E7=A4=BA=E6=9B=B8?= =?UTF-8?q?=E3=82=92=201=20=E3=81=A4=E3=81=AE=E5=8F=82=E5=8A=A0=E8=80=85?= =?UTF-8?q?=E3=81=A8=E8=BC=AA=E7=95=AA=E3=81=AB=E5=90=88=E3=82=8F=E3=81=9B?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 手順書の担当の決め方を 1 つの表にし、足す者・外す者・全員を要する指定を 引数の表と argument-hint に足す。前提の「すべてログイン済み」を外す - init が返す変数の表から適用専用の母集合を消す - CLAUDE.md の cross-refactoring の節を codex / kiro とホストの既定と、参加者の数で 1 周する輪番に直し、--max-outer-rounds の既定を 3 と書く（#736）。 cross-review の節を席の規則に直す - 実装計画を足し、設計文書の「未確認のまま残ること」に P7 で決めた 6 件を足す Refs #664 #736 #727 Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 13 +- ...issue-664-p7-refactor-participants-plan.md | 176 ++++++++++++++++++ issues/issue-727-687-478-664-648-design.md | 11 ++ plugins/ndf/skills/cross-refactoring/SKILL.md | 71 ++++--- .../docs/01-state-and-propose.md | 42 ++--- .../prompts/propose-tests.md | 4 +- .../cross-refactoring/prompts/propose.md | 2 +- .../tests/test_skill_terms.py | 64 ++++++- 8 files changed, 320 insertions(+), 63 deletions(-) create mode 100644 issues/issue-664-p7-refactor-participants-plan.md diff --git a/CLAUDE.md b/CLAUDE.md index e1cf21f00..b6cbb18f7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,27 +59,28 @@ python3 plugins/ndf/scripts/instructions-check.py --root . ## cross-refactoring -`/ndf:cross-refactoring` は codex / agy / kiro / claude のうち **ホストを除く 3 者**に構造改善を提案させ、**参加する 4 者**から輪番で選んだ 1 者が適用し、残り 2 者がレビューする。新しい提案が出なくなるまで繰り返す。 +`/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 --model codex=gpt-5.5 --model claude=claude-opus-5 +/ndf:cross-refactoring 130 --scope src --include agy --exclude kiro ``` - `--scope` は必須。提案が発散して PR が肥大するのを防ぐ。**検証にも効く**ので、現状固定テストの置き場所も含める - ホストと同じランタイムが適用担当になる場合も、サブエージェントではなく **CLI プロセス**として起動する -- モデルを比べるなら `--model <ランタイム>=` を 4 つとも指定する。実際に動いたモデルを取得できるのは claude だけで、残り 3 つは指定値で代用する。指定が無いラウンドは集計から分離される -- 適用担当は 4 ラウンドで 1 周する。`--max-outer-rounds` の既定が 4 なのは、上限 3 では 4 者目の順番へ届かないため +- モデルを比べるなら `--model <ランタイム>=` を参加者の全員に指定する。実際に動いたモデルを取得できるのは claude だけで、残りは指定値で代用する。指定が無いラウンドは集計から分離される +- 適用担当は参加者の数のラウンドで 1 周する。輪番は適用ラウンドごとに進むため、`--max-outer-rounds`（既定 3）が切る提案の回数とは対応しない - 収束しない改善項目は **項目単位で取り消す**。合意済みの項目は PR に残る。ただし同一ファイルの隣接行を触る項目どうしは git だけでは分離できないため、そのラウンドは全件取り消しへ退避する - 生成物・配布物の同期は **進行側の責務**。実装担当にはさせない（範囲外の変更になる）。同期の手順は `--sync-command "bash scripts/build-runtime-plugins.sh"` のように渡す - 公開するのは **進行側だけ**。実装担当は push しない。進行側が検証を通した後に push するので、未検証の変更が公開されない - 履歴に残るのは **1 改善項目 = 1 コミット**。現状固定テストが要る項目だけ 2 コミット。テストも項目の単位で 1 回だけ求める - 改修計画は `--plan-file`（既定 `issues/refactoring-plan-rf.md`）へ書き出され、生成物の同期と同じコミットで公開される -- `init` が参加 CLI の認証状態を確認する。誤検知するときは `NDF_SKIP_AUTH_CHECK=1` +- `init` が参加者の認証状態を確認し、通らない者を外して続ける。全員が揃わないなら止めたいときは `--require-all`。誤検知するときは `NDF_SKIP_AUTH_CHECK=1` ## cross-review -`/ndf:cross-review` は codex / agy の両方に PR レビューを委譲し、両者が `APPROVE` するまで修正ループを回す。agy の progress log を heartbeat に表示するため、無言に見える時間でも `scan` / `analyze` / `post` / `done` などの作業段階を確認できる。 +`/ndf:cross-review` はホストを除く 3 つのランタイムのうち使える者から毎ラウンド 2 席を選んで PR レビューを委譲し、両席が `APPROVE` するまで修正ループを回す。使える者が 2 者に満たなければ、ホスト、次に同じランタイムの 2 つ目が席を埋める。agy の progress log を heartbeat に表示するため、無言に見える時間でも `scan` / `analyze` / `post` / `done` などの作業段階を確認できる。 追加レビュー観点は以下のどちらかで渡す: @@ -88,4 +89,4 @@ python3 plugins/ndf/scripts/instructions-check.py --root . /ndf:cross-review 123 --extra-instructions-file /tmp/review-focus.md ``` -PR の変更ファイルから docs only / code / DB migration / test / dependency / CI設定 / API契約 / 認証認可 / frontend / performance / deletion / generated / i18n / infra を自動分類し、該当するレビュー観点テンプレートも codex / agy 両方に渡す。 +PR の変更ファイルから docs only / code / DB migration / test / dependency / CI設定 / API契約 / 認証認可 / frontend / performance / deletion / generated / i18n / infra を自動分類し、該当するレビュー観点テンプレートも両席に渡す。 diff --git a/issues/issue-664-p7-refactor-participants-plan.md b/issues/issue-664-p7-refactor-participants-plan.md new file mode 100644 index 000000000..02523452f --- /dev/null +++ b/issues/issue-664-p7-refactor-participants-plan.md @@ -0,0 +1,176 @@ +# cross-refactoring: 担当を外す引数が無く、使える者が 2 者だとレビュー担当が 1 者になる → 使える者だけで始まり、参加者は codex / kiro とホストを既定に足し引きでき、適用の輪番はその参加者の中で回る（実装計画 P7: cross-refactoring と旧関数の削除 / #664 #736） + +## 関連リンク + +- 親 issue #727、子 issue #664。あわせて #736（リポジトリの根の `CLAUDE.md` の上限の既定の記述） +- 設計: [issue-727-687-478-664-648-design.md](issue-727-687-478-664-648-design.md)（決定 20 件。識別子は冒頭の用語の対応表で引く） +- 要求: [issue-727-687-478-664-648-requirements.md](issue-727-687-478-664-648-requirements.md)（受け入れ条件 AC1〜AC50） +- 契約: [issue-727-687-478-664-648-contracts.md](issue-727-687-478-664-648-contracts.md)（状態ファイル・引数・関数の形） +- 確定仕様: [cross-review-participants-and-seats.md](../docs/specifications/cross-review-participants-and-seats.md)（共通層と cross-review 側） +- 1 本目の実装: [issue-727-p6-participants-plan.md](issue-727-p6-participants-plan.md)（PR #793、マージ済み） + +## モード + +`standard`。収束ループの初期化と担当の決め方を変え、共通層と cross-refactoring と文書にまたがる。 + +## 目的と非目的 + +達成したい状態: + +- 参加する CLI のどれか 1 者が導入・認証されていなくても、cross-refactoring の初期化が使える者で始まる。使えない者と理由が状態ファイルに残る +- 既定の参加者が codex / kiro とホストになり、足す者・外す者の指定で名指しで変えられる。agy は足す者の指定で戻せる +- 適用の輪番が参加者の中で回る。存在しない役（レビュー担当）の記録と出力が消える +- 中断したループを引数を変えて再開すると、上限は反映され、反映しない引数は知らされる +- 使える者の決め方が共通層の 1 か所だけになり、両 Skill に古い形が残らない（親 #727 の完了条件） + +やらないこと: + +- cross-review 側の変更（1 本目で済んだ）。ただし旧関数に依存する cross-review のテスト 3 件は、関数を消すのに合わせて期待値を書き直す +- 起動した後に分かる使えなさで担当を自動的に外す仕組み（設計の決定 18） +- 引数の型の検査（カンマ区切りの名前と予約語）の共通層への移動。cross-review の状態の部品は並行する束（G5）が触っており、この Pull Request では cross-refactoring の側に同じ規則の型を置く +- 1 者指定を cross-refactoring に足すこと（契約の引数の表に無い） +- 指示書の cross-refactoring の節のうち、参加者と輪番と上限以外の古い行（コミットの単位・改修計画の置き場所・取り消しの単位）。#799 として起票した + +## 前提 + +- 前提 1: 設計文書の決定 20 件は変えない。実装で決めたことは「実装で決めたこと」に書き、設計文書の「未確認のまま残ること」に P7 の節を同じ Pull Request で足す +- 前提 2: 並行する束（G5、#730）は cross-review の投稿の部分と共通層の投稿の待ち行列を触る。旧関数を使う箇所が G5 のブランチに無いことを、消す前に `git grep` で確かめる（着手時点で `origin` に G5 の実装ブランチは無く、`develop` の呼び手はこの Pull Request が置き換える箇所だけだった） +- 前提 3: 新しい参加者（ホストが提案に入ること）の所要は測らない。設計の「未確認のまま残ること」のまま運用へ渡す + +## 受け入れ条件 + +要求文書の AC7、AC31〜AC43、AC47、AC49〜AC50 をそのまま使う。検証手段は設計文書の「テスト設計」の表にある。AC47（#664 の再現手順）は手元で実行し、結果を Pull Request 本文に残す。 + +## ドメイン用語 + +識別子は設計文書の用語の対応表と同じ。この計画で新しく使う語は次の 1 つだけである。 + +| 用語 | 意味 | +| --- | --- | +| 反映の表（cross-refactoring） | 再開で渡した引数ごとに「反映する」か「知らせる」かを決める表。cross-review の表と同じ形で、cross-refactoring の部品が持つ | + +## 不変条件 + +- 状態ファイルの参加者の一覧（`runtimes`）は、参加者の記録の使える者と同じ値である +- 新規の状態ファイルに適用専用の母集合の項目とラウンドのレビュー担当の項目が無い +- この変更の前に始めた実行の状態ファイルを、書き換えずに読める（適用の輪番は参加者の一覧から決まる） +- 使える者が 0 者、または全員を要する指定で欠けがあるとき、状態ファイルを作らない・書き換えない + +## 互換性 + +| 対象 | 変更 | 互換性の扱い | +| --- | --- | --- | +| 初期化の引数 | 足す者・外す者・全員を要する指定を足す。上限と重要度の既定を未指定にする | 追加のみ。未指定のときの既定値は変えない | +| 初期化の出力 | 適用専用の母集合の変数を出さない | 読み手は手順書の表だけ（骨組みは読まない）。手順書から消す | +| ラウンドの開始の出力 | レビュー担当の 2 変数を出さない | 読み手はテストの期待値だけ（設計文書の「実測」） | +| 状態ファイル | 参加者の記録と再開で変えた値の記録を足し、適用専用の母集合とレビュー担当を書かない | 古い状態ファイルは項目が無いまま読む。報告は「記録なし」と出す | +| 共通層の関数 | 従来の確認・適用専用の母集合・従来の席と適用の割り当ての 4 つを消す | 呼び手が 0 件になったことを `git grep` のテストで固定する | + +## 修正対象 + +```text +plugins/ndf/scripts/lib/assignment.py 旧関数 3 つと説明を消す +plugins/ndf/scripts/lib/auth.py 従来の確認を消す +plugins/ndf/scripts/tests/test_lib_assignment.py 旧関数のテストを消す +plugins/ndf/scripts/tests/test_auth_probe.py 従来の確認のテストを消す +plugins/ndf/scripts/tests/test_shared_lib_layout.py 旧関数が残らないことの検査を足す（AC7） +plugins/ndf/skills/cross-refactoring/scripts/refactor.py 引数の追加と既定の変更 +plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/setup.py 初期化・再開・ラウンドの開始 +plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/rounds.py 適用の輪番の包み +plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/apply.py 担当の交代の上限 +plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/commands/report.py 母集合の 1 行・参加者の節・列の削除 +plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/plan.py 改修計画の見出しからレビュー担当を消す +plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/gitfacts.py 観測したモデルの記録からレビュー側の枝を消す +plugins/ndf/skills/cross-refactoring/tests/ test_init.py / test_start_round_emits_runtimes.py / test_assignment.py ほか +plugins/ndf/skills/cross-review/tests/ 旧関数に依存する 3 件の期待値 +plugins/ndf/skills/cross-refactoring/SKILL.md / docs/01-state-and-propose.md +CLAUDE.md cross-refactoring の節と cross-review の節 +issues/issue-727-687-478-664-648-design.md 「未確認のまま残ること」に P7 の節 +``` + +## タスク分解 + +各タスクは失敗するテストを先に書き、通す最小の実装を足し、整える。旧関数の削除（Task 6）は呼び手をすべて置き換えた後に行う。 + +### Task 1: 新規の初期化を共通層の使える者の解決へ載せ替える + +- **対象ファイル:** `refactor.py`、`commands/setup.py`、`tests/test_init.py` +- **変更内容:** 母集合の既定（`refactor_pool`）と使える者の解決（`resolve_participants`）を止めない確認（`probe_auth`）で呼ぶ。状態ファイルへ参加者の一覧と参加者の記録と空の再開の記録を書き、適用専用の母集合を書かない・出さない。割り当ての失敗と 0 者を終了コード 4 へ写す。足す者・外す者・全員を要する指定の引数を足す +- **満たす受け入れ条件:** AC31、AC32、AC33、AC35、AC36、AC47 +- **進め方:** 確認を差し替えた初期化のテストを先に書く + +### Task 2: 適用の輪番を参加者の中で回し、レビュー担当を消す + +- **対象ファイル:** `rounds.py`、`commands/setup.py`、`commands/apply.py`、`gitfacts.py`、`tests/test_start_round_emits_runtimes.py`、`tests/test_apply_attempts.py` +- **変更内容:** 輪番の包み（`impl_for_seq`）の中身を適用の輪番（`impl_assign(seq, state["runtimes"])`）へ替え、ラウンドの開始もこれを使う。ラウンドの記録にレビュー担当を書かず、2 変数を出さない。担当の交代を試す回数を参加者の数にする +- **満たす受け入れ条件:** AC34、AC41 +- **進め方:** 出力と記録にレビュー担当が無いテストを先に書く（既存の期待値を反転する） + +### Task 3: 再開で上限を反映し、他の引数を知らせ、担当に関わる引数で参加者を作り直す + +- **対象ファイル:** `refactor.py`、`commands/setup.py`、`tests/test_init.py` +- **変更内容:** 状態に載る引数の既定を未指定にし、新規の経路で現行の既定へ置き換える。反映の表を置き、再開で共通層の再開の反映（`apply_resume_args`）を呼ぶ。足す者・外す者・全員を要する指定のどれかを渡した再開では、渡さなかった値を記録から補って作り直し、1 件として積む。失敗したら書き換えずに終了コード 4 +- **満たす受け入れ条件:** AC38、AC39、AC40 +- **進め方:** 状態ファイルを置いた作業ツリーで再開するテストを先に書く + +### Task 4: 報告と改修計画の表示を 1 つの母集合に揃える + +- **対象ファイル:** `commands/report.py`、`plan.py`、関連するテスト +- **変更内容:** 状態の表示の母集合を 1 行にし、ラウンド表からレビュー担当と初回承認の列を消す。完了報告に参加者の節を足す（参加者の記録が無ければ「記録なし」）。改修計画の見出しからレビュー担当を消す +- **満たす受け入れ条件:** AC37、AC41 +- **進め方:** 報告と改修計画の出力のテストを先に書く + +### Task 5: 手順書とリポジトリの根の指示書を実装に合わせる + +- **対象ファイル:** `SKILL.md`、`docs/01-state-and-propose.md`、`CLAUDE.md` +- **変更内容:** 担当の決め方を 1 つの表にし、引数の表と `argument-hint` に 3 つの引数を足し、前提の「すべてログイン済み」とホストごとの CLI の表を直す。`init` の変数の表から適用専用の母集合を消す。指示書の cross-refactoring の節を新しい母集合と輪番と上限の既定 3 に直し（#736）、cross-review の節を席の規則に直す +- **満たす受け入れ条件:** AC42、AC43 +- **進め方:** 文書の語の検査（既存のテスト）に期待を足してから直す + +### Task 6: 呼び手の無くなった旧関数を消す + +- **対象ファイル:** `assignment.py`、`auth.py`、共通層のテスト、cross-review と cross-refactoring の旧関数のテスト +- **変更内容:** 従来の確認・適用専用の母集合・従来の席と適用の割り当てを消す。消した関数を期待値に使っていたテストは、変更前の値を定数で持つ形へ直す。4 つの名前が共通層と両 Skill の部品に残らないことをテストで固定する +- **満たす受け入れ条件:** AC7 +- **進め方:** 残っていないことの検査を先に足す（失敗する）→ 消す + +### Task 7: 設計文書を更新し、配布物と検査を通す + +- **対象ファイル:** 設計文書、配布物 +- **変更内容:** 「未確認のまま残ること」に P7 で決めたことを足す。配布物を同期し、全体のテストと 6 つの検査を通す +- **満たす受け入れ条件:** AC49、AC50 +- **進め方:** テスト駆動の対象外（検査の実行） + +## 実装で決めたこと + +| 項目 | 決めたこと | +| --- | --- | +| 状態ファイルの確認の結果の項目（`auth`） | 新規の状態に書かない。読み手が無く、確認を通らなかった者と理由は参加者の記録（`participants.unavailable`）が持つ | +| 確認を行う位置 | 新規の経路で、作業ディレクトリの用意と範囲の関門の後、着手前のテストの前。状態ファイルの有無（新規か再開か）を見てから確かめるためで、再開では担当に関わる引数を渡したときだけ確かめる | +| 反映の表の中身 | 「反映する」は上限 4 つとテストの制限時間。「知らせる」はホスト・範囲・モデル・着手前のテスト・継続的統合の検査の名前・重要度の閾値・同期のコマンド・改修計画のファイル・起動のされ方・作業ディレクトリ root の 10 個。着手前のテストはコマンドで、モデルは全ランタイムの辞書で、作業ディレクトリ root は解決したパスで比べる | +| 引数の型の置き場所 | `--exclude` / `--include` の型（カンマ区切りの名前と予約語 `none`）は cross-refactoring の初期化の部品に置く。共通層へ移すと cross-review の状態の部品も触ることになり、並行する束（G5）と重なる | +| 完了報告の参加者の節 | cross-review と同じ行の形にし、席の埋め合わせの行は持たない（cross-refactoring に席は無い）。ラウンド表からはレビュー担当とモデルの列に加え、レビュー担当の判定から作っていた初回承認の列も消す | +| モデルの警告の対象 | 参加者だけ。既定で外れる agy は、足したときだけ警告する | + +## 影響範囲 + +- cross-refactoring の起動する CLI の集合が変わる（既定で agy が外れ、ホストが提案に入る） +- 担当名の読み手（監視・起動）は参加者の一覧をそのまま使うため変わらない +- 指標の集計（共通層の `metrics.py`）は、古い状態ファイルのレビュー担当を読む経路を残す + +## リスクと対処 + +| リスク | 対処 | +| --- | --- | +| 初期化の関数（`cmd_init`）は新規と再開の 2 経路と確認を 1 関数に持ち、再開の反映を足すと長くなる | タスクごとにテストを通す。再開の経路は別の関数に出す | +| 旧関数を消す時点で、並行する束が新しい呼び手を足している | 消す直前に `develop` と並行する束のブランチを `git grep` で確かめる。消した後の検査テストが継続的統合で拾う | +| 期待値に旧関数を使うテストの意味が変わる | 変更前の値を定数として持ち、テストの主張（3 者のときの席は変更前と一致する）を保つ | + +## 切り戻し手順 + +- この Pull Request を revert すれば元へ戻る。データ移行は無い。新しい形で作った状態ファイルは、戻した後の版では適用専用の母集合が無いため、実行の途中で戻すなら状態ファイルを消して最初から始める + +## 完了の定義 + +- [ ] 上の受け入れ条件をすべて満たし、条件ごとに検証手段と結果が対応している +- [ ] 全体のテスト、配布物の同期の検査、6 つの検査が終了コード 0 で終わる diff --git a/issues/issue-727-687-478-664-648-design.md b/issues/issue-727-687-478-664-648-design.md index 16521bb92..80cada95c 100644 --- a/issues/issue-727-687-478-664-648-design.md +++ b/issues/issue-727-687-478-664-648-design.md @@ -448,6 +448,17 @@ P6（共通層と cross-review）→ P7（cross-refactoring と旧関数の削 | 出力の文言 | 反映した行は `↻ <項目>: <旧> → <新>`、知らせる行は `ℹ --<引数> は再開では反映しません（状態: <値> / 指定: <値>）`、通らなかった者は `⚠ <名前> を担当から外しました（<理由>）`、埋め合わせは `⚠ 使える者が <数> 者のため、席を<相手>で埋めます（観点が減ります）`。既存の初期化の出力の印（`↻` / `ℹ` / `⚠`）に揃える | | テストの置き場所 | テスト設計の表のとおり。席の埋め方は cross-refactoring の割り当てのテスト（変更前の席の割り当ての期待値が同じファイルにある）、起動と監視と計測の席の名前は cross-review の `tests/test_seat_names.py`（新設） | +### P7 の実装で決めた 6 件 + +| 項目 | 決めたこと | +| --- | --- | +| 状態ファイルの確認の結果の項目（`auth`） | 新規の状態に書かない。読み手が無く、確認を通らなかった者と理由は参加者の記録（`participants.unavailable`）が持つ | +| 確認を行う位置 | 新規の経路で、作業ディレクトリの用意と範囲の関門の後、着手前のテストの前。状態ファイルの有無（新規か再開か）を見てから確かめるためで、再開では担当に関わる引数を渡したときだけ確かめる | +| 反映の表の中身 | 「反映する」は上限 4 つとテストの制限時間。「知らせる」はホスト・範囲・モデル・着手前のテスト・継続的統合の検査の名前・重要度の閾値・同期のコマンド・改修計画のファイル・起動のされ方・作業ディレクトリ root の 10 個。着手前のテストはコマンドで、モデルは全ランタイムの辞書で、作業ディレクトリ root は解決したパスで比べる | +| 引数の型の置き場所 | `--exclude` / `--include` の型（カンマ区切りの名前と予約語 `none`）は cross-refactoring の初期化の部品に置く。共通層へ移すと cross-review の状態の部品も触ることになり、並行する束（G5）と重なる | +| 完了報告の参加者の節 | cross-review と同じ行の形にし、席の埋め合わせの行は持たない（cross-refactoring に席は無い）。ラウンド表からはレビュー担当とモデルの列に加え、レビュー担当の判定から作っていた初回承認の列も消す | +| モデルの警告の対象 | 参加者だけ。既定で外れる agy は、足したときだけ警告する | + ## 申し送り（並行する設計との境界） 並行する 4 つの設計と 2 つの issue との境界を、決めた契約と分担で書く。 diff --git a/plugins/ndf/skills/cross-refactoring/SKILL.md b/plugins/ndf/skills/cross-refactoring/SKILL.md index 4a5acd0fb..6bf574fc3 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] [--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] [--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 @@ -44,8 +44,8 @@ allowed-tools: | 語 | 何の単位か | 上限を決めるもの | | --- | --- | --- | -| テスト整備ラウンド | **足すべきテストを集める。** 3 者が提案し、採否を決める | `--max-test-rounds`（既定 2） | -| 提案ラウンド | **構造改善の提案を集める。** 3 者が提案し、採否を決める | `--max-outer-rounds`（既定 3） | +| テスト整備ラウンド | **足すべきテストを集める。** 参加者が提案し、採否を決める | `--max-test-rounds`（既定 2） | +| 提案ラウンド | **構造改善の提案を集める。** 参加者が提案し、採否を決める | `--max-outer-rounds`（既定 3） | | 適用ラウンド | **同時に適用して検証する。** 書き換えるファイルが重ならない項目だけを含む。**上の 2 つのラウンドが共有する** | 同じ群を開き直すのは 2 回まで（引数を持たない固定値）。件数は `--max-items-per-round` が実質の上限 | | 修正ラウンド | **検証の失敗を直す。上の 2 つのラウンドが共有する** | `--max-fix-rounds`（既定 3） | | 改善項目 | 構造改善の提案の 1 件。`<ファイル>#<シンボル>` と兆候で識別する | — | @@ -62,7 +62,7 @@ allowed-tools: | 観点 | 方針 | | --- | --- | | 参加者 | **全員 CLI プロセス。** ホストのサブエージェント機能は使わない。ホストと同じランタイムが実装担当のラウンドでも別プロセスで起動する | -| 役割の分離 | 提案は**ホストを除く 3 者**、適用は**参加する 4 者すべて**。両者は重なるが一致しない | +| 参加者 | **提案と適用を同じ参加者で回す。** 既定は codex / kiro とホストで、`--exclude` / `--include` で名指しで変える。確認を通らない者は外して続ける | | 検証の単位 | **適用ラウンド（群）に対して 1 回。** 判定は `--baseline-test` の合否で決まり、レビュー CLI は起動しない | | 収束しない項目 | **捨てる。** リファクタリングは任意の作業なので、揉める提案を Pull Request に残さない | | コミットの単位 | **1 適用ラウンド = 1 コミット。** テストも適用ラウンドの単位で 1 回だけ求める | @@ -87,6 +87,9 @@ allowed-tools: | `[PR番号]` | 対象の Pull Request | 必須 | | `--scope PATH...` | 対象範囲。**提案が無制限に広がらないよう必須。** 検証にも効くので、現状固定テストの置き場所も含める | 必須 | | `--host claude\|codex\|agy\|kiro` | ホストの明示指定。未指定時は環境変数から推定（agy は推定できないため明示する） | 推定 | +| `--exclude NAMES` | 参加者から外す者（カンマ区切り・繰り返し可）。ホストも外せる。再開で `none` を渡すと空へ戻す | なし | +| `--include NAMES` | 参加者に足す者（例: `--include agy`）。再開で `none` を渡すと空へ戻す | なし | +| `--require-all` | 確認を通らない者が 1 者でもいれば中断する（終了コード 4）。付けなければ外して続ける | 外して続ける | | `--model RT=MODEL` | ランタイムごとのモデル。繰り返し指定できる | CLI の既定 | | `--baseline-test CMD` | 着手前と各コミットで実行するテスト。**振る舞い不変を示す手段が無い書き換えは構造改善ではないため必須** | 必須 | | `--max-test-rounds N` | **テスト整備ラウンド**の上限。到達したら採用が残っていても提案ラウンドへ進む | `2` | @@ -105,40 +108,47 @@ allowed-tools: /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 +/ndf:cross-refactoring 130 --scope src tests --baseline-test "pytest -q" --include agy --exclude kiro ``` -**モデルを比べたいなら `--model <ランタイム>=` を 4 つとも指定する。** -実際に動いたモデルを取得できるのは claude だけで、残る 3 者は指定値で代用する。 +**モデルを比べたいなら `--model <ランタイム>=` を参加者の全員に指定する。** +実際に動いたモデルを取得できるのは claude だけで、残る者は指定値で代用する。 指定が無いラウンドは何が動いたか分からないため、集計から分離される （kiro の既定 `auto` も同じ扱いになる）。 ## 担当の決め方 -ホストセッションは**進行の制御に徹し、提案とレビューには参加しない**。 -ただし**適用だけはホストと同じランタイムも担当しうる**。その場合も CLI プロセスとして -起動するため、ホストセッションの作業文脈からは切り離されている。 +ホストセッションは**進行の制御に徹する**。提案と適用はどちらも CLI プロセスとして +起動するため、ホストと同じランタイムが担当するときもホストセッションの作業文脈からは +切り離されている。 -| 母集合 | 定義 | 中身 | +| 参加者（`runtimes`） | 決め方 | ホストごとの既定 | | --- | --- | --- | -| 提案（`runtimes`） | 全ランタイム − ホスト | 常に 3 者 | -| 適用（`impl_capable`） | 全ランタイム | 常に claude / codex / agy / kiro | - -- **適用から外す者はいない。** 4 者はいずれも NDF の配布先で、適用で読ませる - `refactoring` / `tdd-cycle` / `quality-gates` を配っている -- **ホストは適用にだけ参加する。** 提案から外れているので、 - 「実装した者と提案した者が同一モデルにならない」構造は保たれる -- **適用担当は適用ラウンドごとに輪番を進める。** 1 つの提案ラウンドが複数の群を - 持てば、その分だけ輪番も進む。**`--max-outer-rounds` が切るのは提案の回数だけ**で、 - 輪番の 1 周とは対応しない +| 提案と適用の両方 | 既定（codex / kiro とホスト）＋ `--include` − `--exclude`。確認を通らない者は外す | claude: claude / codex / kiro、codex: codex / kiro、agy: codex / agy / kiro、kiro: codex / kiro | + +- **agy は既定に入らない。** 起動の失敗が参加者の中で最も多く、提案の所要も最も長かった（#664）。 + 戻すときは `--include agy` を渡す +- **確認を通らない者は外して続ける。** 外した者と理由は状態ファイルと完了報告に残る。 + 全員が揃わないなら始めたくないときは `--require-all` を付ける。使える者が 0 者なら + 中断する（終了コード 4） +- **適用担当は適用ラウンドごとに輪番を進め、参加者の数のラウンドで 1 周する。** + ラウンド 1 は参加者の 2 番目から始まるため、ホストが最初に適用する形にならない。 + 提案者と適用者が同じランタイムになることは避けない（適用の結果はテストと Step 7 が見る） +- **`--max-outer-rounds` が切るのは提案の回数だけ**で、輪番の 1 周とは対応しない。 + 1 つの提案ラウンドが複数の群を持てば、その分だけ輪番も進む +- **レビュー担当はいない。** レビューは Step 7 の `cross-review` が担う 割り当ては `refactor.py start-round` が返し、状態ファイルへ記録する。**再開しても変わらない。** +再開で `--exclude` / `--include` / `--require-all` を渡したときだけ確認をやり直し、 +次に開くラウンドから反映する。上限（`--max-*-rounds` と `--test-timeout`）は再開で渡せば +反映し、状態に載る他の引数は状態と違えば「反映しない」と知らせる。 ## 前提 - `gh` CLI が認証済みで、`jq` と `uv`（または Python 3.10 以上）が使える -- 参加する CLI が**すべてログイン済み**である。`init` が認証状態を確認し、1 つでも - 未認証なら中断する（未認証の CLI は起動から 15 秒で終わり、結果を残さないまま - 担当から脱落するため、確認しないと参加者が欠けた構成のまま進行する） +- 参加者の CLI がログイン済みである。`init` が認証状態を確認し、通らない者を担当から + 外して続ける（未認証の CLI は起動から 15 秒で終わり、結果を残さないまま担当から + 脱落するため、確認しないと参加者が欠けた構成のまま進行する） | ランタイム | 確認コマンド | | --- | --- | @@ -150,16 +160,17 @@ allowed-tools: 確認コマンドは CLI の版で変わりうる。誤検知するときは `NDF_SKIP_AUTH_CHECK=1` で 飛ばせる（飛ばしたことは出力に残る） -- ホストごとに次の CLI が使える（不足していると初期化時に失敗する） +- ホストごとに次の CLI が使える（使えない者は担当から外れる） | ホスト | 必要な CLI | | --- | --- | - | Claude Code | `codex` / `agy` / `kiro-cli` | - | Codex | `claude` / `agy` / `kiro-cli` | - | agy | `claude` / `codex` / `kiro-cli` | - | Kiro CLI | `claude` / `codex` / `agy` | + | Claude Code | `codex` / `kiro-cli` | + | Codex | `kiro-cli` | + | agy | `codex` / `kiro-cli` | + | Kiro CLI | `codex` | - 適用にはホスト自身も参加するため、ホストのコマンドも起動できる必要がある。 + ホスト自身も参加者に入るため、ホストのコマンドも起動できる必要がある。 + `--include` で足した者の CLI も要る。 **agy がホストのときは `--host agy` を明示する**（環境変数からは推定しない） - 対象の Pull Request が Draft で開いている（未作成なら `/ndf:pr` で先に作る） @@ -283,6 +294,8 @@ rf_eval() { rf_eval init "$PR" --scope $SCOPE \ --baseline-test "$BASELINE" ${HOST:+--host "$HOST"} \ + ${EXCLUDE:+--exclude "$EXCLUDE"} ${INCLUDE:+--include "$INCLUDE"} \ + ${REQUIRE_ALL:+--require-all} \ --max-test-rounds "$MAX_TEST" --max-outer-rounds "$MAX_OUTER" \ --max-fix-rounds "$MAX_FIX" --max-items-per-round "$MAX_ITEMS" \ ${CI_CHECK:+--ci-check "$CI_CHECK"} ${WORKFLOW_STEP:+--workflow-step} \ 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 ed457e148..684340854 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 @@ -17,8 +17,7 @@ export CROSS_REFACTORING_TMP_DIR="$TMP_DIR" | 変数 | 内容 | | --- | --- | | `ID` | 状態ファイルの鍵（最初に初期化した Pull Request 番号） | -| `RUNTIMES` / `RUNTIMES_CSV` | 提案の母集合（ホストを除く 3 者） | -| `IMPL_POOL` | 適用の母集合（参加する 4 者すべて） | +| `RUNTIMES` / `RUNTIMES_CSV` | 参加者（提案と適用の両方。既定は codex / kiro とホスト） | | `WORKTREE_ROOT` / `WORK` / `TMP_DIR` | 作業ディレクトリと一時ディレクトリ | | `REPO` / `HEAD_BRANCH` / `BASE_BRANCH` / `SCOPE` | 対象の情報 | @@ -26,12 +25,11 @@ export CROSS_REFACTORING_TMP_DIR="$TMP_DIR" 1. **ホストの確定** — `--host` の明示指定を第一とし、未指定時のみ環境変数 （`CLAUDE_PLUGIN_ROOT` / `CODEX_HOME` / `KIRO_AGENT` など）から推定する。 - 推定できなければ**既定値を置かずに失敗する**。誤検出すると提案の母集合が - 狂う（ホストが提案側に混ざる、参加すべき者が外れる）ため、確定結果は出力と状態 - ファイルの両方へ残す -2. **母集合の確定** — 提案（全 − ホスト）と適用（全）を**別々に**確定する。 - 提案の母集合にホストが含まれていたら初期化ごと失敗させる。 - 適用から外す者はいないが、**関数は分けたまま**にする（ホストを含むか否かが違う） + 推定できなければ**既定値を置かずに失敗する**。誤検出すると参加者が + 狂う（参加すべき者が外れる）ため、確定結果は出力と状態ファイルの両方へ残す +2. **参加者の確定** — 既定（codex / kiro とホスト）に `--include` を足し、`--exclude` を + 除く。**提案と適用は同じ参加者で回す。** 母集合に無い者を外す指定と、足す者と外す者の + 重なりは中断する（終了コード 4） 3. **モデルの確定** — `--model <ランタイム>=<モデル>` を繰り返し受け取る。指定値は **全ラウンドで固定**する。途中で変えると比較が成立しないため、再開時も変えない 4. **書き込み用の作業ディレクトリ作成** — `/work/` に head ブランチを checkout する。 @@ -40,12 +38,14 @@ export CROSS_REFACTORING_TMP_DIR="$TMP_DIR" 使うと古い HEAD に対して提案・適用してしまう。早送りできない（履歴が分かれた） ときは中断する。`git fetch` に失敗したときも中断する（古い `origin/` へ 早送りして「同期したつもり」になるのを防ぐ） -5. **認証状態の確認** — 参加する CLI を 1 つずつ確認し、未認証なら**初期化ごと中断する** - （終了コード 4）。存在確認だけでは足りない。未認証の CLI は起動から 15 秒で終わり、 - 結果ファイルを残さないまま担当から脱落するが、それでも初期化は成功として扱われるため、 - **参加者が 1 人欠けた構成のまま最後まで進んでしまう**（実測）。作業ディレクトリを - 作る前に確認する。確認コマンドは CLI の版で変わりうるので `NDF_SKIP_AUTH_CHECK=1` - で飛ばせるが、飛ばしたことは必ず出力へ残す +5. **認証状態の確認** — 参加者の CLI を 1 つずつ確認し、通らない者を**担当から外して + 続ける**。外した者と理由は状態ファイルの `participants` に残る。存在確認だけでは + 足りない。未認証の CLI は起動から 15 秒で終わり、結果ファイルを残さないまま担当から + 脱落するが、それでも初期化は成功として扱われるため、**確認しないと参加者が 1 人欠けた + 構成のまま最後まで進んでしまう**（実測）。着手前のテストより先に確認する。 + `--require-all` を付けると 1 者でも通らなければ中断し、使える者が 0 者のときも + 中断する（どちらも終了コード 4 で、状態ファイルを作らない）。確認コマンドは CLI の版で + 変わりうるので `NDF_SKIP_AUTH_CHECK=1` で飛ばせるが、飛ばしたことは必ず出力へ残す 6. **語彙の受け渡し** — 検証側が持つ兆候・手法・重要度の集合を状態ファイルの `vocabulary` へ書く。提案プロンプトはここから**許容値をそのまま列挙する**。 定義を 1 箇所に保ったまま、読ませ方の不確実性を減らすためである @@ -201,7 +201,7 @@ claude 17 本 / codex 1 メソッド / kiro 0 本と揃わなかった。最後 | --- | --- | | いつ | 初期化の後、**最初の提案ラウンドの前** | | 何回 | 新しいテストの提案が出なくなるまで。上限は `--max-test-rounds`（既定 2） | -| 誰が | 提案は 3 者（ホストを除く）。適用は輪番。**提案ラウンドと同じ母集合** | +| 誰が | 提案は参加者の全員。適用は輪番。**提案ラウンドと同じ参加者** | | 何を | 対象範囲のうちテストの薄い経路へ現状固定テストを足す。**構造は変えない** | | 何を足さないか | **このラウンドの後、構造改善の提案で `test_gap` を真にできない。** テストは既に足してある | | 収束の条件 | **採用 0 件**（提案ラウンドと同じ形） | @@ -242,7 +242,7 @@ done `smell`）と同じ役目を持つ。**`level` は鍵に入れない**（同じ経路を別の階層で 2 度 固定させないため）。同じ経路に複数の階層が挙がったときは**低い方**を採る。 -**経路を自由文だけで挙げさせない。** 同じ経路が 3 者から別の言い回しで出て、重複 +**経路を自由文だけで挙げさせない。** 同じ経路が複数の参加者から別の言い回しで出て、重複 排除が効かなくなる。語彙外の値を含む提案は、構造改善の提案と同じく降格して対象外へ 落とす。 @@ -257,9 +257,9 @@ done --stem-template "{agent}-propose-rf{id}-r$ROUND" --phase propose ``` -3 CLI を並列で起動し、同一のプロンプトで提案させる。**提案フェーズにホストは現れない** -（母集合にいないため）。ただし `launch-cli.sh` はホストと同じランタイムを起動しうる -（適用担当のとき）ので、「ホストなら起動しない」といった分岐を入れてはならない。 +参加者の CLI を並列で起動し、同一のプロンプトで提案させる。**ホストと同じランタイムも +参加者として起動する**（CLI プロセスなので、ホストセッションの作業文脈からは切り離されて +いる）。「ホストなら起動しない」といった分岐を入れてはならない。 提出形式は [prompts/propose.md](../prompts/propose.md) にある。 @@ -304,7 +304,7 @@ CLI の起動時に同名の結果ファイルを消すため、**提案の結 `init` が状態ファイルの `vocabulary` へ書き、`launch-cli.sh` が読んで差し込む。 **同じ一覧を 2 か所に書かない。** -**採用上限も同じ経路で渡す**（`--max-items-per-round`）。「3 者の合計で N 件までが +**採用上限も同じ経路で渡す**（`--max-items-per-round`）。「参加者の合計で N 件までが 採用される」と書き、**多く出すより採れる提案を出す**ことを求める。テスト整備ラウンドの プロンプトも同じ形で伝える。 @@ -346,7 +346,7 @@ CLI の起動時に同名の結果ファイルを消すため、**提案の結 鍵は種類で変わる。改善項目は `path` + `symbol` + `smell`、**テスト項目は `target` + `case`** である。 -**採用上限は提案の時点で伝える**（受け入れ条件 C2）。上限を知らせないと、3 者が +**採用上限は提案の時点で伝える**（受け入れ条件 C2）。上限を知らせないと、参加者が 上限を超える件数を出し、超えた分は見送りとして記録されて以後は「対象外」になる。 提案の労力がそのまま無駄になる。 diff --git a/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md b/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md index 1e3c30f5e..1a2641898 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/propose-tests.md @@ -13,7 +13,7 @@ - 作業ディレクトリ: `$RF_WORKDIR`（**ここから外は読まない・書かない**） - 対象範囲: `$RF_SCOPE`（**この範囲の外は提案しない**） - 着手前のテスト: `$RF_BASELINE_TEST` -- 採用上限: 3 者の提案を統合したうえで、**合計 $RF_MAX_ITEMS 件までが採用**されます +- 採用上限: 参加者全員の提案を統合したうえで、**合計 $RF_MAX_ITEMS 件までが採用**されます ## 手順書 @@ -80,7 +80,7 @@ $RF_VOCAB_LEVELS - `path` は**テストを足す先**のファイル（リポジトリ相対） - `target` は**固定する入口**で、`<ファイル>#<シンボル>` の形で書く -- **`target` + `case` が同じ提案は 1 件へ統合されます。** 3 者が同じ経路を挙げる +- **`target` + `case` が同じ提案は 1 件へ統合されます。** 複数の参加者が同じ経路を挙げる ため、この 2 つが重複排除の鍵になります。**他のランタイムと合意した提案ほど 優先される**ので、独自性を狙わず素直に挙げてください - `level` は鍵に入りません。同じ経路に複数の階層が挙がったときは**低い方**が採られます diff --git a/plugins/ndf/skills/cross-refactoring/prompts/propose.md b/plugins/ndf/skills/cross-refactoring/prompts/propose.md index 621cd7744..2deeff18d 100644 --- a/plugins/ndf/skills/cross-refactoring/prompts/propose.md +++ b/plugins/ndf/skills/cross-refactoring/prompts/propose.md @@ -9,7 +9,7 @@ - 作業ディレクトリ: `$RF_WORKDIR`（**ここから外は読まない・書かない**） - 対象範囲: `$RF_SCOPE`（**この範囲の外は提案しない**） - 着手前のテスト: `$RF_BASELINE_TEST` -- 採用上限: 3 者の提案を統合したうえで、**合計 $RF_MAX_ITEMS 件までが採用**されます +- 採用上限: 参加者全員の提案を統合したうえで、**合計 $RF_MAX_ITEMS 件までが採用**されます ## 手順書 diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py b/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py index ee7c4afd2..8277a2a3b 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_skill_terms.py @@ -58,16 +58,72 @@ def test_every_cap_appears_in_the_argument_table(skill): assert f"`{cap} N`" in skill, f"引数の表に {cap} が無い" -def test_the_defaults_match_the_implementation(refactor, vocabulary, skill): - """既定値は 1 か所（`refactor.py`）が持ち、表はそれを写す。""" +def test_the_defaults_match_the_implementation(cmd_setup, vocabulary, skill): + """既定値は 1 か所（新規の初期化が置き換える表）が持ち、手順書の表はそれを写す。""" assert "| `--max-test-rounds N` | " in skill - for cap, default in (("--max-test-rounds", 2), ("--max-outer-rounds", 3), - ("--max-fix-rounds", 3), ("--max-items-per-round", 5)): + for cap in CAPS: + default = cmd_setup.NEW_RUN_DEFAULTS[cap[2:].replace("-", "_")] row = next(l for l in skill.splitlines() if l.startswith(f"| `{cap} N`")) assert f"`{default}`" in row, f"{cap} の既定が表と実装で食い違う" assert vocabulary.DEFAULT_MAX_TEST_ROUNDS == 2 +# ---------- 参加者と担当の決め方（#727 の AC42 / AC43） ---------- + +PARTICIPANT_ARGS = ("--exclude", "--include", "--require-all") +DOC01 = SKILL.parent / "docs" / "01-state-and-propose.md" +CLAUDE_MD = SKILL.parents[4] / "CLAUDE.md" + + +def test_the_participant_arguments_are_documented(skill): + """AC43 — 引数の表と `argument-hint` に 3 つの引数がある。""" + hint = next(l for l in skill.splitlines() if l.startswith("argument-hint:")) + rows = [l for l in skill.splitlines() if l.startswith("| `--")] + for arg in PARTICIPANT_ARGS: + assert arg in hint, f"argument-hint に {arg} が無い" + assert any(r.startswith(f"| `{arg}") for r in rows), f"引数の表に {arg} が無い" + + +def test_the_assignment_section_has_one_cohort(skill): + """AC43 — 担当の決め方は母集合を 1 つの表で書き、適用専用の母集合を持たない。""" + lines = skill.splitlines() + start = lines.index("## 担当の決め方") + end = next(i for i, l in enumerate(lines[start + 1:], start + 1) if l.startswith("## ")) + section = "\n".join(lines[start:end]) + assert "impl_capable" not in section + assert sum(1 for l in lines[start:end] if l.startswith("| ---")) == 1 + assert "codex / kiro" in section and "--include agy" in section + + +def test_the_prerequisites_no_longer_demand_every_cli(skill): + """AC43 — 前提から「すべてログイン済み」が消え、要る CLI は codex / kiro-cli になる。""" + lines = skill.splitlines() + start = lines.index("## 前提") + end = next(i for i, l in enumerate(lines[start + 1:], start + 1) if l.startswith("## ")) + section = "\n".join(lines[start:end]) + assert "すべてログイン済み" not in section + assert "| Claude Code | `codex` / `kiro-cli` |" in section + + +def test_init_variables_do_not_list_the_implementation_cohort(): + """AC43 — `init` が返す変数の表に適用専用の母集合が無い。""" + assert "IMPL_POOL" not in DOC01.read_text(encoding="utf-8") + + +def test_claude_md_describes_the_participants_and_the_rotation(): + """AC42 — 指示書の cross-refactoring の節が新しい母集合と輪番を書く（#736 を含む）。""" + text = CLAUDE_MD.read_text(encoding="utf-8") + lines = text.splitlines() + start = lines.index("## cross-refactoring") + end = next(i for i, l in enumerate(lines[start + 1:], start + 1) if l.startswith("## ")) + section = "\n".join(lines[start:end]) + assert "codex / kiro とホスト（ホストが codex / kiro なら 2 者）" in section + assert "適用担当は参加者の数のラウンドで 1 周する" in section + for stale in ("ホストを除く 3 者", "参加する 4 者", "codex / agy の両方", + "既定が 4"): + assert stale not in text, f"CLAUDE.md に {stale} が残っている" + + # ---------- 実行のコマンド列（A1 / B6） ---------- def _run_block(text: str) -> str: From 5900ef07f250ec10137c0400a25bde07359798bb Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 10:06:01 +0000 Subject: [PATCH 3/9] =?UTF-8?q?Test:=20=E5=A2=83=E7=95=8C=E5=88=86?= =?UTF-8?q?=E5=B2=90=E3=81=AE=E7=8F=BE=E7=8A=B6=E5=9B=BA=E5=AE=9A=E3=83=86?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=82=92=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 再開時の参加者追加、少人数での適用担当輪番、空ラウンドの進行判定を固定する。 Item-Id: R1-001 Round: 1 Impl-Runtime: codex Impl-Model: default --- plugins/ndf/scripts/tests/test_lib_assignment.py | 10 ++++++++++ .../skills/cross-refactoring/tests/test_init.py | 14 ++++++++++++++ .../skills/cross-refactoring/tests/test_rounds.py | 10 ++++++++++ 3 files changed, 34 insertions(+) diff --git a/plugins/ndf/scripts/tests/test_lib_assignment.py b/plugins/ndf/scripts/tests/test_lib_assignment.py index 73400c8cb..23f2b67cb 100644 --- a/plugins/ndf/scripts/tests/test_lib_assignment.py +++ b/plugins/ndf/scripts/tests/test_lib_assignment.py @@ -81,6 +81,16 @@ def test_impl_assign_rotates_over_the_participants_starting_after_the_host(assig assert actual == ["codex", "kiro", "claude", "codex", "kiro", "claude"] +def test_impl_assign_with_one_participant_always_returns_that_participant(assignment): + participants = ["codex"] + assert [assignment.impl_assign(r, participants) for r in (1, 2)] == ["codex", "codex"] + + +def test_impl_assign_with_two_participants_rotates_between_them(assignment): + participants = ["codex", "kiro"] + assert [assignment.impl_assign(r, participants) for r in (1, 2)] == ["kiro", "codex"] + + def test_impl_assign_rejects_a_bad_round(assignment): with pytest.raises(assignment.AssignmentError): assignment.impl_assign(0, ["claude", "codex"]) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_init.py b/plugins/ndf/skills/cross-refactoring/tests/test_init.py index a63d844e1..dedcfc1d7 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_init.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_init.py @@ -612,6 +612,20 @@ def test_resume_with_exclude_rebuilds_the_participants(run_init, tmp_path): assert changes[0]["from"]["available"] == ["claude", "codex", "agy", "kiro"] +def test_resume_with_include_adds_the_participant_worktree(run_init, tmp_path): + """足す者を渡した再開では参加者と作業ツリーの対応をともに補う。""" + run_init(_args(tmp_path), probe={}) + + run_init(_args(tmp_path, include=[["agy"]]), probe={}) + + _, state = _state_of(tmp_path) + assert state["runtimes"] == ["claude", "codex", "agy", "kiro"] + assert state["worktrees"]["agy"] == str(tmp_path / "rf130" / "agy") + changes = state["resume_changes"] + assert [change["field"] for change in changes] == ["participants"] + assert changes[0]["to"]["available"] == state["runtimes"] + + def test_resume_with_none_clears_the_recorded_names(run_init, tmp_path): """予約語 `none` は記録の一覧を空へ戻す（決定 15）。""" run_init(_args(tmp_path, exclude=[["kiro"]]), probe={}) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py b/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py index 7017e2338..d6adf1145 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_rounds.py @@ -79,6 +79,16 @@ def test_start_round_stops_when_already_final(refactor, tmp_path, env_tmp_dir): # ---------- advance（収束判定） ---------- +def test_advance_with_no_rounds_leaves_the_state_unchanged(cmd_report, tmp_path, env_tmp_dir): + state_path = make_state(tmp_path, rounds=[], final=None) + env_tmp_dir(state_path) + before = read_state(state_path) + + cmd_report.cmd_advance(_args()) + + assert read_state(state_path) == before + + def test_advance_continues_when_progress_is_made(cmd_report, tmp_path, env_tmp_dir): state_path = make_state(tmp_path, rounds=[round_of(1)]) env_tmp_dir(state_path) From da54b7665818bcdd0db0f52421afa5859cbe89d4 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 10:12:42 +0000 Subject: [PATCH 4/9] =?UTF-8?q?Test:=20=E5=BC=95=E6=95=B0=E8=A7=A3?= =?UTF-8?q?=E6=9E=90=E3=81=A7=20none=20=E3=81=A8=E5=90=8D=E5=89=8D?= =?UTF-8?q?=E3=81=AE=E6=B7=B7=E5=9C=A8=E3=82=92=E6=8B=92=E3=82=80=E5=88=86?= =?UTF-8?q?=E5=B2=90=E3=81=AE=E7=8F=BE=E7=8A=B6=E5=9B=BA=E5=AE=9A=E3=83=86?= =?UTF-8?q?=E3=82=B9=E3=83=88=E3=82=92=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd_init において --exclude や --include に none と通常のランタイム名が同時に指定された場合に終了コード 4 で処理が中断され、状態ファイルが作成されない振る舞いを固定する。 Item-Id: R1-002 Round: 1 Impl-Runtime: agy Impl-Model: default --- .../ndf/skills/cross-refactoring/tests/test_init.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_init.py b/plugins/ndf/skills/cross-refactoring/tests/test_init.py index dedcfc1d7..f5cb0ba6f 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_init.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_init.py @@ -234,6 +234,18 @@ def test_contradicting_names_stop_the_init(run_init, tmp_path, over): assert not _state_path(tmp_path).exists() +@pytest.mark.parametrize("over", [ + {"exclude": [["none", "kiro"]]}, + {"include": [["none", "agy"]]}, +]) +def test_none_mixed_with_runtime_names_stops_the_init(run_init, tmp_path, over): + """none とランタイム名の混在は中断（終了コード 4）し、状態ファイルを作らない。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, **over), probe={}) + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() + + def test_init_records_models(run_init, tmp_path): run_init(_args(tmp_path, model=["codex=gpt-5.5", "kiro=claude-opus-5"])) _, state = _state_of(tmp_path) From 1267db4a60c8ae6606dd8311a65664e5d7e9fd33 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 10:21:36 +0000 Subject: [PATCH 5/9] =?UTF-8?q?Test:=20=E7=8F=BE=E7=8A=B6=E5=9B=BA?= =?UTF-8?q?=E5=AE=9A=20=E2=80=94=20refactor.py=20init=20=E3=81=AE=20--incl?= =?UTF-8?q?ude/--exclude=20=E3=81=AE=E6=8B=92=E5=90=A6=E7=B5=8C=E8=B7=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 空の --include が argparse の型で終了コード 2 になり初期化へ進まないこと、 none と実行者名を混在させた --exclude が終了コード 4 で中断し状態ファイルを 作らないことを現状固定テストとして追加する。対象コードは変更しない。 Item-Id: R1-004 Round: 1 Impl-Runtime: kiro Impl-Model: default --- .../cross-refactoring/tests/test_init.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/plugins/ndf/skills/cross-refactoring/tests/test_init.py b/plugins/ndf/skills/cross-refactoring/tests/test_init.py index f5cb0ba6f..e5d8cdaa7 100644 --- a/plugins/ndf/skills/cross-refactoring/tests/test_init.py +++ b/plugins/ndf/skills/cross-refactoring/tests/test_init.py @@ -402,6 +402,36 @@ def test_include_and_exclude_parse_names_and_none(patch_lib, refactor, monkeypat assert e.value.code == 2 +@pytest.mark.parametrize("empty", ["", " ", ","]) +def test_empty_include_is_rejected_before_init_runs( + patch_lib, refactor, monkeypatch, empty): + """R1-004 — 空の `--include` は argparse の型が弾き、初期化へ進まない。 + + `runtime_list` が空の値で `ArgumentTypeError` を上げ、argparse が終了コード 2 で + 止める。`cmd_init` は差し替えた入口を通らないため、捕えた引数は空のままになる。 + """ + captured = {} + monkeypatch.setattr(refactor, "cmd_init", + lambda args: captured.update(vars(args))) + monkeypatch.setattr( + refactor.sys, "argv", + ["refactor.py", "init", "130", "--scope", "src", "--host", "claude", + "--baseline-test", "true", "--include", empty], + ) + with pytest.raises(SystemExit) as e: + refactor.main() + assert e.value.code == 2 + assert captured == {}, "初期化処理へ進んでいる" + + +def test_none_mixed_with_a_runtime_name_in_exclude_stops_the_init(run_init, tmp_path): + """R1-004 — none と実行者名を混在させた `--exclude` は中断し、状態を作らない。""" + with pytest.raises(SystemExit) as e: + run_init(_args(tmp_path, exclude=[["none", "kiro"]]), probe={}) + assert e.value.code == refactor_abort() + assert not _state_path(tmp_path).exists() + + def test_the_ci_check_is_not_set_by_default(patch_lib, refactor, monkeypatch): """指定が無ければ代替しない。**手元のテストで判定する**（決定 7 の排他）。""" assert _parsed_init_args(patch_lib, refactor, monkeypatch)["ci_check"] is None From 101761abe497b0720e65f60613bbf221db646644 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 10:35:58 +0000 Subject: [PATCH 6/9] =?UTF-8?q?Refactor:=20extract=5Fmethod=20=E2=80=94=20?= =?UTF-8?q?plugins/ndf/skills/cross-refactoring/scripts/refactor=5Flib/com?= =?UTF-8?q?mands/apply.py#cmd=5Fnext=5Fapply=5Fround?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 適用ラウンド 1 の 3 項目を、振る舞いを変えずに関数の抽出で分割する。 - R2-001: cmd_next_apply_round から、開く群の選択（_select_next_apply_group）と 開いた群の状態に応じた適用の状態の組み立て（_prepare_apply_entry）を抽出 - R2-003: verify_apply_round から、現状固定テストの有無（_verify_test_gap_present）・ 差分予算（_verify_diff_budget）・粒度（_verify_apply_commit_count）の検査を抽出 - R2-005: cmd_merge_final_fix から、結果と範囲の確定（_collect_final_fix_range）・ 申告コミットの検証（_verify_final_fix_commits）・採否の反映（_apply_final_fix_verdict）を抽出 Item-Id: R2-001 Round: 2 Impl-Runtime: claude Impl-Model: claude-opus-5 Co-Authored-By: Claude Opus 5 (1M context) --- .../scripts/refactor_lib/commands/apply.py | 74 ++++++----- .../scripts/refactor_lib/commands/gate.py | 121 ++++++++++++------ .../scripts/refactor_lib/verify.py | 87 ++++++++----- 3 files changed, 184 insertions(+), 98 deletions(-) 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 c3bd6ceaf..e949fa0e8 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 @@ -283,36 +283,25 @@ def _item_summary(item: dict[str, Any]) -> str: -def cmd_next_apply_round(args: argparse.Namespace) -> None: - """Step 4 — 次の適用ラウンドを開き、実装担当と対象の項目を返す。 - - 終了コード: 0 = 群を開いた / 1 = 残りの群が無い（提案ラウンドへ戻る）。 - - **群の起点はここで確定させる。** 後続の群は先行の群を適用した後の作業ツリーを - 読むため、起点はその時点の HEAD になる。取り消しの範囲もこの起点で決まる。 - - **修正ラウンドの数え直しも群ごとである。** `--max-fix-rounds` は 1 つの適用 - ラウンドあたりの上限だからである。 +def _select_next_apply_group( + groups: list[dict[str, Any]], +) -> tuple[Optional[dict[str, Any]], str]: + """次に開く群と、その群の開き直しの判定を返す。無ければ群は None。 + + **`applied` の群も開き直す。** 適用は取り込んだが検証まで進めずに落ちた場合、 + 飛ばすとその群の項目が採用でも取り消しでもないまま残る。再開できることは + 収束ループの前提である。 + + **未着手の群は、開き直しの判定へ掛ける**（#647）。無条件に開き直すと、結果を + 残さない担当に当たり続けて上限なく起動する。項目が無い群と上限に達した群は、 + ここで取り消し済みにして次を探す。 """ - path, state = load_state(args.id) - entry = round_of(state, args.round) - groups = apply_groups(entry) - - # **`applied` の群も開き直す。** 適用は取り込んだが検証まで進めずに落ちた場合、 - # 飛ばすとその群の項目が採用でも取り消しでもないまま残る。再開できることは - # 収束ループの前提である。 - # - # **未着手の群は、開き直しの判定へ掛ける**（#647）。無条件に開き直すと、結果を - # 残さない担当に当たり続けて上限なく起動する。項目が無い群と上限に達した群は、 - # ここで取り消し済みにして次を探す。 - opened: Optional[dict[str, Any]] = None reopening = "" for group in groups: if group.get("status") not in {"pending", "applied"}: continue if group.get("status") == "applied": - opened = group - break + return group, reopening reopening = group_reopening(group) if reopening in {"empty", "exhausted"}: group["status"] = "dropped" @@ -324,14 +313,15 @@ def cmd_next_apply_round(args: argparse.Namespace) -> None: f"（{'項目なし' if reopening == 'empty' else '試行の上限'}）" ) continue - opened = group - break + return group, reopening + return None, reopening - if opened is None: - statefile.save(path, state) - info(f"提案ラウンド {args.round} の適用ラウンドは残っていません") - sys.exit(1) +def _prepare_apply_entry( + state: dict[str, Any], entry: dict[str, Any], + opened: dict[str, Any], reopening: str, +) -> None: + """開いた群の状態に応じて、提案ラウンドの適用の状態を組み立てる。""" entry["apply_round"] = opened["apply_round"] if opened.get("status") == "pending" and reopening == "open": # 起点は**オーケストレータ側で**確定させる。実装担当の申告に委ねると、 @@ -354,6 +344,30 @@ def cmd_next_apply_round(args: argparse.Namespace) -> None: # 取り込み済みの群を開き直した。**起点も修正の回数も動かさない。** info(f"↻ 適用ラウンド {opened['apply_round']} は取り込み済みです（検証から再開）") entry["apply_base_sha"] = opened.get("base_sha") + + +def cmd_next_apply_round(args: argparse.Namespace) -> None: + """Step 4 — 次の適用ラウンドを開き、実装担当と対象の項目を返す。 + + 終了コード: 0 = 群を開いた / 1 = 残りの群が無い（提案ラウンドへ戻る）。 + + **群の起点はここで確定させる。** 後続の群は先行の群を適用した後の作業ツリーを + 読むため、起点はその時点の HEAD になる。取り消しの範囲もこの起点で決まる。 + + **修正ラウンドの数え直しも群ごとである。** `--max-fix-rounds` は 1 つの適用 + ラウンドあたりの上限だからである。 + """ + path, state = load_state(args.id) + entry = round_of(state, args.round) + groups = apply_groups(entry) + + opened, reopening = _select_next_apply_group(groups) + if opened is None: + statefile.save(path, state) + info(f"提案ラウンド {args.round} の適用ラウンドは残っていません") + sys.exit(1) + + _prepare_apply_entry(state, entry, opened, reopening) state["phase"] = "apply" statefile.save(path, state) 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 08596df4f..85f2dd203 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 @@ -182,46 +182,18 @@ def _close_failed_final_fix( sys.exit(2) -def cmd_merge_final_fix(args: argparse.Namespace) -> None: - """Step 7 — 最終ゲートの修正結果を取り込む。 - - **`merge-fix` では代用できない。** あちらは適用ラウンド（群）の控えを読み、 - 範囲の起点・担当・改善項目の 3 つをそこから取る。最終ゲートにはそのどれも無い。 - 実際に流用すると次の 3 つが起きる。 - - | 流用したときに起きること | なぜ | - | --- | --- | - | 「起点 None」で止まり修正を取り込めない | 最後の群が検証を通っていれば `fix_base_sha` が無い | - | 正常なコミットまで取り消される | 古い起点が残っていると、そこから HEAD までが範囲になる | - | トレーラーが揃わず全件が不正になる | `Item-Id` を要求するが、最終ゲートの修正は項目に属さない | - - 終了コード: 0 = 取り込んだ / 2 = 取り込めなかった（範囲を確定できない、または - 担当が結果を残さなかった）。合否そのものは判定せず、**次の `final-gate` が - 採った側で 1 度だけ見る**。 +def _collect_final_fix_range( + path: pathlib.Path, + state: dict[str, Any], + gate: dict[str, Any], + scope: IntakeScope, + impl: str, + work: str, +) -> tuple[dict[str, Any], str, list[str]]: + """修正担当の結果と、取り込む範囲（HEAD と起点からのコミット）を確定する。 - **結果を残さなかったときも、作られたコミットは取り消す。** 取り消さずに抜けると、 - 次の最終ゲートがそのコミットを含む先端でテストし、落ちれば起点をそこへ置き直す。 - 未検証の差分が Pull Request に残る（#674）。 + 結果が無いとき・範囲を確定できないときは、ここで終了する。 """ - path, state = load_state(args.id) - gate = state.setdefault("final_gate", {"fix_rounds": 0, "checks": []}) - impl = str(gate.get("impl") or "") - if not impl: - die( - "最終ゲートの修正担当が記録されていません。" - "先に `final-gate` を実行してください", - code=4, - ) - - work = str(state["worktrees"]["work"]) - discard_impl_leftovers(state, work) - flush_pending_push(path, state, gate) - - scope = _final_fix_scope(gate, impl) - if already_closed(scope): - info("↻ この最終ゲートの修正の試行は結果なしとして記録済みです") - sys.exit(2) - outcome = read_result(state, impl, "final-fix") if outcome.payload is None: _close_failed_final_fix(path, state, gate, scope, outcome) @@ -236,7 +208,16 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: "検証できない修正は採りません", code=2, ) + return payload, head_now, ordered_range + +def _verify_final_fix_commits( + state: dict[str, Any], + work: str, + payload: dict[str, Any], + ordered_range: list[str], +) -> tuple[list[str], list[str]]: + """申告されたコミットを検証し、未申告のコミットと問題の一覧を返す。""" claimed_shas = reported_shas(payload) unassigned = unassigned_fix_commits(work, claimed_shas, ordered_range) # **テストコマンドは渡さない。** 合否は `final-gate` が採った側で 1 度だけ見る @@ -250,7 +231,20 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: for c in facts ) if p ] + return unassigned, problems + +def _apply_final_fix_verdict( + path: pathlib.Path, + state: dict[str, Any], + gate: dict[str, Any], + scope: IntakeScope, + head_now: str, + ordered_range: list[str], + unassigned: list[str], + problems: list[str], +) -> None: + """検証の結果に応じて、修正を取り消すか最終ゲートの記録へ取り込む。""" if unassigned: info( f"❌ どの申告にも含まれていない修正コミットが {len(unassigned)} 件あります" @@ -269,6 +263,57 @@ def cmd_merge_final_fix(args: argparse.Namespace) -> None: gate.setdefault("fix_commits", []).extend(ordered_range) info(f"修正を取り込みました（{len(ordered_range)} コミット）") + +def cmd_merge_final_fix(args: argparse.Namespace) -> None: + """Step 7 — 最終ゲートの修正結果を取り込む。 + + **`merge-fix` では代用できない。** あちらは適用ラウンド（群）の控えを読み、 + 範囲の起点・担当・改善項目の 3 つをそこから取る。最終ゲートにはそのどれも無い。 + 実際に流用すると次の 3 つが起きる。 + + | 流用したときに起きること | なぜ | + | --- | --- | + | 「起点 None」で止まり修正を取り込めない | 最後の群が検証を通っていれば `fix_base_sha` が無い | + | 正常なコミットまで取り消される | 古い起点が残っていると、そこから HEAD までが範囲になる | + | トレーラーが揃わず全件が不正になる | `Item-Id` を要求するが、最終ゲートの修正は項目に属さない | + + 終了コード: 0 = 取り込んだ / 2 = 取り込めなかった（範囲を確定できない、または + 担当が結果を残さなかった）。合否そのものは判定せず、**次の `final-gate` が + 採った側で 1 度だけ見る**。 + + **結果を残さなかったときも、作られたコミットは取り消す。** 取り消さずに抜けると、 + 次の最終ゲートがそのコミットを含む先端でテストし、落ちれば起点をそこへ置き直す。 + 未検証の差分が Pull Request に残る（#674）。 + """ + path, state = load_state(args.id) + gate = state.setdefault("final_gate", {"fix_rounds": 0, "checks": []}) + impl = str(gate.get("impl") or "") + if not impl: + die( + "最終ゲートの修正担当が記録されていません。" + "先に `final-gate` を実行してください", + code=4, + ) + + work = str(state["worktrees"]["work"]) + discard_impl_leftovers(state, work) + flush_pending_push(path, state, gate) + + scope = _final_fix_scope(gate, impl) + if already_closed(scope): + info("↻ この最終ゲートの修正の試行は結果なしとして記録済みです") + sys.exit(2) + + payload, head_now, ordered_range = _collect_final_fix_range( + path, state, gate, scope, impl, work, + ) + unassigned, problems = _verify_final_fix_commits( + state, work, payload, ordered_range, + ) + _apply_final_fix_verdict( + path, state, gate, scope, head_now, ordered_range, unassigned, problems, + ) + gate.setdefault("durations", {})["fix"] = ( gate.get("durations", {}).get("fix", 0) + safe_int(payload.get("elapsed_seconds")) 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 c6bd02ffe..db5b9b10f 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/verify.py @@ -189,6 +189,56 @@ def diff_budget_factor(technique: Optional[str]) -> int: return DIFF_BUDGET_FACTOR +def _verify_test_gap_present( + items: list[dict[str, Any]], facts: list[dict[str, Any]], +) -> Optional[str]: + """テストが乏しい項目を含む群で、現状固定テストの追加が先行しているか。""" + if any(i.get("test_gap") for i in items): + # テストが乏しいと申告された項目は、現状固定テストの追加が先行していること。 + # 「テストを足した」かどうかは、そのコミットがテストの置き場所を触ったかで見る。 + if not facts[0].get("touches_tests"): + return ( + "テストが乏しい項目を含むのに、現状固定テストの追加が伴っていません" + f"（先頭コミット {facts[0].get('sha', '?')} がテストを触っていません）" + ) + return None + + +def _verify_diff_budget( + items: list[dict[str, Any]], facts: list[dict[str, Any]], +) -> Optional[str]: + """実差分が、見積の行数から決まる差分予算に収まっているか。""" + estimated = sum(safe_int(i.get("estimated_diff_lines")) for i in items) + factor = max( + (diff_budget_factor(i.get("technique")) for i in items), + default=DIFF_BUDGET_FACTOR, + ) + budget = estimated * factor + actual = sum(int(c.get("diff_lines") or 0) for c in facts) + if budget and actual > budget: + return ( + f"実差分 {actual} 行が差分予算 {budget} 行" + f"（見積 {estimated} 行 × {factor}）を超えました（範囲の逸脱）" + ) + return None + + +def _verify_apply_commit_count(facts: list[dict[str, Any]]) -> Optional[str]: + """適用ラウンドのコミットが 1 件に収まっているか。 + + 数えるのは**実在するコミットの数**である。同じコミットを群の全項目が + 申告するのは正しい形なので、重ねた申告では落とさない。 + """ + count = len({c.get("sha") for c in facts}) + if count > 1: + return ( + f"適用ラウンドのコミットが {count} 件あります" + "（残すのは適用ラウンド = 1 コミット。" + "群の中の項目はまとめて 1 つのコミットにします）" + ) + return None + + def verify_apply_round( items: list[dict[str, Any]], facts: list[dict[str, Any]], scope: Optional[Iterable[str]] = None, @@ -221,14 +271,9 @@ def verify_apply_round( if problem: return problem - if any(i.get("test_gap") for i in items): - # テストが乏しいと申告された項目は、現状固定テストの追加が先行していること。 - # 「テストを足した」かどうかは、そのコミットがテストの置き場所を触ったかで見る。 - if not facts[0].get("touches_tests"): - return ( - "テストが乏しい項目を含むのに、現状固定テストの追加が伴っていません" - f"（先頭コミット {facts[0].get('sha', '?')} がテストを触っていません）" - ) + problem = _verify_test_gap_present(items, facts) + if problem: + return problem # **テストの期待値が変わっていないか**（#443）。段 1（機械）で決まるものだけを # ここで落とす。決まらないものは `pending_test_judgements` が集め、進行側が @@ -238,30 +283,12 @@ def verify_apply_round( if problem: return problem - estimated = sum(safe_int(i.get("estimated_diff_lines")) for i in items) - factor = max( - (diff_budget_factor(i.get("technique")) for i in items), - default=DIFF_BUDGET_FACTOR, - ) - budget = estimated * factor - actual = sum(int(c.get("diff_lines") or 0) for c in facts) - if budget and actual > budget: - return ( - f"実差分 {actual} 行が差分予算 {budget} 行" - f"（見積 {estimated} 行 × {factor}）を超えました（範囲の逸脱）" - ) + problem = _verify_diff_budget(items, facts) + if problem: + return problem # 粒度は最後に見る。トレーラーや範囲の問題を粒度の失敗で覆い隠さない。 - # 数えるのは**実在するコミットの数**である。同じコミットを群の全項目が - # 申告するのは正しい形なので、重ねた申告では落とさない。 - count = len({c.get("sha") for c in facts}) - if count > 1: - return ( - f"適用ラウンドのコミットが {count} 件あります" - "（残すのは適用ラウンド = 1 コミット。" - "群の中の項目はまとめて 1 つのコミットにします）" - ) - return None + return _verify_apply_commit_count(facts) def commit_limit_for(item: dict[str, Any]) -> int: From 5cb07ed4b343ccc35dc3b51885b8ed90ca950f62 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 10:46:07 +0000 Subject: [PATCH 7/9] =?UTF-8?q?Refactor:=20flatten=5Fconditional=20?= =?UTF-8?q?=E2=80=94=20apply.py#=5Fload=5Fruntime=5Fproposals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ランタイムごとの提案読み取りを抽出し、提案統合のループを早期 continue で平坦化する。 Item-Id: R2-002 Round: 2 Impl-Runtime: codex Impl-Model: default --- .../scripts/refactor_lib/commands/apply.py | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) 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 e949fa0e8..394026544 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 @@ -80,6 +80,30 @@ class _ApplyCommitRange: in_range: set[str] +def _read_runtime_proposal( + result: pathlib.Path, +) -> Optional[list[dict[str, Any]]]: + runtime = result.name.split("-", 1)[0] + if not result.exists(): + info(f"⚠ {runtime} の提案結果がありません: {result}") + return None + try: + payload = json.loads(result.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") + return None + if not isinstance(payload, dict): + info( + f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" + f"（{type(payload).__name__}）。提案なしとして扱います" + ) + return [] + items = payload.get("items") + if not isinstance(items, list): + return [] + return [item for item in items if isinstance(item, dict)] + + def _load_runtime_proposals( state: dict[str, Any], entry: dict[str, Any] ) -> dict[str, list[dict[str, Any]]]: @@ -94,28 +118,11 @@ def _load_runtime_proposals( state, runtime, stem_for(runtime, "propose", state["id"], entry["round"]), ) - if not result.exists(): - info(f"⚠ {runtime} の提案結果がありません: {result}") - continue - try: - payload = json.loads(result.read_text(encoding="utf-8")) - except json.JSONDecodeError as e: - info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") - continue - if not isinstance(payload, dict): - # 配列や数値のまま `payload.get(...)` を呼ぶと落ちる。 - # 提案は無かったものとして続ける（1 者の不調で全体を止めない）。 - info( - f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" - f"（{type(payload).__name__}）。提案なしとして扱います" - ) - proposals[runtime] = [] - entry["proposed"][runtime] = 0 + runtime_proposals = _read_runtime_proposal(result) + if runtime_proposals is None: continue - items = payload.get("items") - proposals[runtime] = [i for i in items if isinstance(i, dict)] \ - if isinstance(items, list) else [] - entry["proposed"][runtime] = len(proposals[runtime]) + proposals[runtime] = runtime_proposals + entry["proposed"][runtime] = len(runtime_proposals) return proposals From 2d510a9b8d90274d7c83cedb8aae1dc8ad7da819 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 10:57:23 +0000 Subject: [PATCH 8/9] =?UTF-8?q?Refactor:=20extract=5Fmethod=20=E2=80=94=20?= =?UTF-8?q?plugins/ndf/skills/cross-refactoring/scripts/refactor=5Flib/com?= =?UTF-8?q?mands/apply.py#cmd=5Fmerge=5Ftest=5Fjudgements?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 保留の特定・担当の結果読み込み・changed時の項目放棄と群の取り消し・正常時の 群への判定適用と通知を、それぞれ独立した関数へ抽出した。入力取得と破棄を伴う 状態遷移が混ざっていた本体を、境界の見える 4 段に分けた。振る舞いは不変。 Item-Id: R2-004 Round: 2 Impl-Runtime: kiro Impl-Model: default --- .../scripts/refactor_lib/commands/apply.py | 127 +++++++++++------- 1 file changed, 82 insertions(+), 45 deletions(-) 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 394026544..f18a57c4c 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 @@ -1049,6 +1049,82 @@ def _resume_incomplete_apply( flush_pending_push(path, state, entry) +def _pending_judgements_for_round( + entry: dict[str, Any], group_no: int +) -> list[str]: + """この群の保留を取り出す。**全ての群をまとめて解かない。**""" + records = entry.get("pending_test_judgements") + if isinstance(records, dict): + return list(records.get(str(group_no), [])) + return [] + + +def _read_group_judge_verdicts( + state: dict[str, Any], impl: Optional[str], round_no: int, group_no: int, +) -> list[dict[str, Any]]: + """この群を判定した担当の結果ファイルを読み、dict の verdict だけを返す。 + + **読むのは、この群を判定した担当の結果だけである。** 全ランタイムを読むと、 + 前の群で別の担当が返した古い答えが混ざり、今回の `changed` を打ち消す。 + """ + if not impl: + return [] + result = result_path( + state, impl, + f"{impl}-judge-test-changes-r{round_no}-g{group_no}") + if not result.exists(): + return [] + try: + payload = json.loads(result.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + payload = {} + found = payload.get("verdicts") + if isinstance(found, list): + return [v for v in found if isinstance(v, dict)] + return [] + + +def _drop_round_on_changed_judgement( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + group: dict[str, Any], problem: str, +) -> None: + """`changed` があった群を取り消し、項目へ印を残す。**必ず終了する。**""" + failed = list(group.get("items") or []) + # **`entry["items"]` は項目 ID の並びである。** 実体は `state["items"]` にある。 + for item_id in failed: + item = find_item(state, item_id, required=False) + if item: + item["status"] = "abandoned" + item["failure_reason"] = problem + _apply_drop(path, state, entry, group, failed) + # **取り消した群の保留だけを消す。** 先行する群でレビューへ引き継ぐと決めた + # 分まで捨てない。 + record_pending_judgements(entry, group.get("apply_round") or 1, []) + statefile.save(path, state) + info(f"❌ 適用ラウンド {group.get('apply_round')}: {problem}") + # **終了コードは 2 にする。** 進行側は「取り消した」と読んで次の群へ進む。 + sys.exit(2) + + +def _apply_group_judgements( + path: pathlib.Path, state: dict[str, Any], entry: dict[str, Any], + group_no: int, verdicts: list[dict[str, Any]], +) -> None: + """当該群だけへ判定を適用し、残りをレビューへ引き継ぐと知らせる。 + + **解くのは、判定が実際に見た群の保留だけである。** 段 2 へ渡すのはその群の + 差分であるため、別の群で同じファイルが残っていてもそちらは解かない。 + """ + remaining = apply_judgements_to_group(entry, group_no, verdicts) + if remaining: + info( + f"{len(remaining)} 件はレビューへ引き継ぎます: " + ", ".join(remaining) + ) + else: + info("この適用群のテストの差分は、期待する振る舞いを変えていません") + statefile.save(path, state) + + def cmd_merge_test_judgements(args: argparse.Namespace) -> None: """段 2（AI エージェント）の答えを取り込む（#443）。 @@ -1061,59 +1137,20 @@ def cmd_merge_test_judgements(args: argparse.Namespace) -> None: path, state = load_state(args.id) entry = round_of(state, args.round) # **判定の対象はこの群の保留である。** 全ての群をまとめて解かない。 - records = entry.get("pending_test_judgements") group_of_round = (current_group(entry) or {}).get("apply_round") or 1 - pending = list((records or {}).get(str(group_of_round), [])) \ - if isinstance(records, dict) else [] + pending = _pending_judgements_for_round(entry, group_of_round) if not pending: info("判定を待っているテストはありません") return - # **読むのは、この群を判定した担当の結果だけである。** 全ランタイムを読むと、 - # 前の群で別の担当が返した古い答えが混ざり、今回の `changed` を打ち消す。 impl = (current_group(entry) or {}).get("impl") or entry.get("impl") - verdicts: list[dict[str, Any]] = [] - if impl: - result = result_path( - state, impl, - f"{impl}-judge-test-changes-r{args.round}-g{group_of_round}") - if result.exists(): - try: - payload = json.loads(result.read_text(encoding="utf-8")) - except (OSError, json.JSONDecodeError): - payload = {} - found = payload.get("verdicts") - if isinstance(found, list): - verdicts = [v for v in found if isinstance(v, dict)] + verdicts = _read_group_judge_verdicts( + state, impl, args.round, group_of_round) outcome = merge_test_judgements(pending, verdicts) if outcome["problem"]: - group = current_group(entry) - failed = list(group.get("items") or []) - # **`entry["items"]` は項目 ID の並びである。** 実体は `state["items"]` にある。 - for item_id in failed: - item = find_item(state, item_id, required=False) - if item: - item["status"] = "abandoned" - item["failure_reason"] = outcome["problem"] - _apply_drop(path, state, entry, group, failed) - # **取り消した群の保留だけを消す。** 先行する群でレビューへ引き継ぐと決めた - # 分まで捨てない。 - record_pending_judgements(entry, group.get("apply_round") or 1, []) - statefile.save(path, state) - info(f"❌ 適用ラウンド {group.get('apply_round')}: {outcome['problem']}") - # **終了コードは 2 にする。** 進行側は「取り消した」と読んで次の群へ進む。 - sys.exit(2) + _drop_round_on_changed_judgement( + path, state, entry, current_group(entry), outcome["problem"]) - # **解くのは、判定が実際に見た群の保留だけである。** 段 2 へ渡すのはその群の - # 差分であるため、別の群で同じファイルが残っていてもそちらは解かない。 - group_no = (current_group(entry) or {}).get("apply_round") or 1 - remaining = apply_judgements_to_group(entry, group_no, verdicts) - if remaining: - info( - f"{len(remaining)} 件はレビューへ引き継ぎます: " + ", ".join(remaining) - ) - else: - info("この適用群のテストの差分は、期待する振る舞いを変えていません") - statefile.save(path, state) + _apply_group_judgements(path, state, entry, group_of_round, verdicts) From 7f0fe7642758df77323fab4c6c0099a6ef0876a8 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Tue, 22 Sep 2026 11:10:21 +0000 Subject: [PATCH 9/9] =?UTF-8?q?Refactor:=20consolidate=5Fduplication=20?= =?UTF-8?q?=E2=80=94=20plugins/ndf/skills/cross-refactoring/scripts/refact?= =?UTF-8?q?or=5Flib/proposals.py#merge=5Fproposals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - R3-001: merge_proposals と merge_test_proposals が共有していた提案の正規化・重複排除・統合の二重ループを _build_merged へ集約した - R3-002: format_report の実装担当・レビュー担当の行組み立てを _impl_rows / _reviewer_rows へ抽出した Item-Id: R3-001 Round: 3 Impl-Runtime: claude Impl-Model: default Co-Authored-By: Claude Opus 5 (1M context) --- plugins/ndf/scripts/lib/metrics.py | 36 ++++++++------ .../scripts/refactor_lib/proposals.py | 48 ++++++++++--------- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/plugins/ndf/scripts/lib/metrics.py b/plugins/ndf/scripts/lib/metrics.py index d900fc888..af2d625c9 100644 --- a/plugins/ndf/scripts/lib/metrics.py +++ b/plugins/ndf/scripts/lib/metrics.py @@ -276,10 +276,9 @@ def _emit_table( lines += [*headers, *rows] -def format_report(metrics: dict[str, Any]) -> str: - """人が読む形へ整形する。比較の限界を必ず添える。""" - lines: list[str] = [] - impl_rows = [ +def _impl_rows(metrics: dict[str, Any]) -> list[str]: + """実装担当の表の行を組む。""" + return [ ( f"| {key} | {m['rounds']} | {m['applied']} | {m['abandoned']} | " f"{_fmt(m['first_review_approval_rate'])} | {_fmt(m['avg_fix_rounds'])} | " @@ -288,6 +287,23 @@ def format_report(metrics: dict[str, Any]) -> str: ) for key, m in metrics["impl"].items() ] + + +def _reviewer_rows(metrics: dict[str, Any]) -> list[str]: + """レビュー担当の表の行を組む。""" + return [ + ( + f"| {key} | {m['reviews']} | {m['findings']} | " + f"{_fmt(m['resolution_rate'])} | {_fmt(m['agreement_rate'])} | " + f"{m['seconds']:.0f} |" + ) + for key, m in metrics["reviewer"].items() + ] + + +def format_report(metrics: dict[str, Any]) -> str: + """人が読む形へ整形する。比較の限界を必ず添える。""" + lines: list[str] = [] _emit_table( lines, "実装担当", @@ -295,17 +311,9 @@ def format_report(metrics: dict[str, Any]) -> str: "| ランタイム / モデル | 担当R | 適用 | 見送り | 初回承認率 | 平均修正R | 予算超過率 | テスト失敗率 | 所要秒 |", "| --- | ---: | ---: | ---: | ---: | ---: | ---: | ---: | ---: |", ), - impl_rows, + _impl_rows(metrics), ) - reviewer_rows = [ - ( - f"| {key} | {m['reviews']} | {m['findings']} | " - f"{_fmt(m['resolution_rate'])} | {_fmt(m['agreement_rate'])} | " - f"{m['seconds']:.0f} |" - ) - for key, m in metrics["reviewer"].items() - ] lines.append("") _emit_table( lines, @@ -314,7 +322,7 @@ def format_report(metrics: dict[str, Any]) -> str: "| ランタイム / モデル | レビュー回数 | 指摘 | 修正に至った率 | 判定一致率 | 所要秒 |", "| --- | ---: | ---: | ---: | ---: | ---: |", ), - reviewer_rows, + _reviewer_rows(metrics), ) if metrics["unmeasured"]: 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 69e3c5e8e..6752482aa 100644 --- a/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py +++ b/plugins/ndf/skills/cross-refactoring/scripts/refactor_lib/proposals.py @@ -135,17 +135,7 @@ def merge_proposals( `excluded_keys` には過去に見送った項目の鍵を渡す。見送った項目を毎ラウンド 再提案されると収束しないため、対象外として落とす。 """ - merged: dict[tuple[str, ...], dict[str, Any]] = {} - for source, items in proposals.items(): - for raw in items: - norm = _normalize_proposal(raw, source) - if norm is None: - continue - key = _dedupe_key(norm) - if key in merged: - _merge_one(merged[key], norm) - else: - merged[key] = norm + merged = _build_merged(proposals, _normalize_proposal, _merge_one) min_severity = SEVERITY_ORDER.get( threshold, SEVERITY_ORDER[DEFAULT_SEVERITY_THRESHOLD]) @@ -170,6 +160,30 @@ def reject(item: dict[str, Any]) -> Optional[str]: ) +def _build_merged( + proposals: dict[str, list[dict[str, Any]]], + normalize: Callable[[dict[str, Any], str], Optional[dict[str, Any]]], + merge_one: Callable[[dict[str, Any], dict[str, Any]], None], +) -> dict[tuple[str, ...], dict[str, Any]]: + """提案を正規化し、重複排除の鍵ごとに統合した辞書を返す。**種類で分けない。** + + 正規化と統合の関数だけが種類ごとに違う。重複排除の基準（`_dedupe_key`）が + 片方だけ直されて食い違わないよう 1 箇所に置く。 + """ + merged: dict[tuple[str, ...], dict[str, Any]] = {} + for source, items in proposals.items(): + for raw in items: + norm = normalize(raw, source) + if norm is None: + continue + key = _dedupe_key(norm) + if key in merged: + merge_one(merged[key], norm) + else: + merged[key] = norm + return merged + + def _select( merged: dict[tuple[str, ...], dict[str, Any]], *, @@ -284,17 +298,7 @@ def merge_test_proposals( 採否の詰めは `merge_proposals` と同じ `_select` が行う。 """ - merged: dict[tuple[str, ...], dict[str, Any]] = {} - for source, items in proposals.items(): - for raw in items: - norm = _normalize_test_proposal(raw, source) - if norm is None: - continue - key = _dedupe_key(norm) - if key in merged: - _merge_test_one(merged[key], norm) - else: - merged[key] = norm + merged = _build_merged(proposals, _normalize_test_proposal, _merge_test_one) def reject(item: dict[str, Any]) -> Optional[str]: if item["case"] == "unknown" or item["level"] == "unknown":