From 2b25e58a8f9d27ff55bd510766f4a83d5f9a4e63 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 11:26:08 +0000 Subject: [PATCH 01/21] =?UTF-8?q?Fix:=20=E8=AA=A4=E3=82=8A=E3=82=92?= =?UTF-8?q?=E7=A4=BA=E3=81=95=E3=82=8C=E3=81=A6=E3=81=84=E3=81=AA=E3=81=84?= =?UTF-8?q?=E9=87=8D=E5=A4=A7=E3=81=AA=E6=8C=87=E6=91=98=E3=82=92=E6=9C=AA?= =?UTF-8?q?=E5=8F=8D=E8=A8=BC=E3=81=A8=E3=81=97=E3=81=A6=E6=95=B0=E3=81=88?= =?UTF-8?q?=E3=80=81=E5=8F=8D=E8=A8=BC=E3=81=8C=E6=8F=83=E3=82=8F=E3=81=AA?= =?UTF-8?q?=E3=81=84=E3=83=A9=E3=82=A6=E3=83=B3=E3=83=89=E3=81=AE=E5=8D=B0?= =?UTF-8?q?=E3=82=92=E5=A4=96=E3=81=99=EF=BC=88#732=20#624=20#706=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 収束の判定が数えないのは、誤りだと示された棄却と軽微な指摘だけにする。 人の判断待ちの条件から根拠の 2 項目を外し、重大な指摘の残余を新しい区分 「未反証」（unrefuted）として数え、理由（no_critique / not_supported）を 状態ファイルに残す。測定スクリプトの数える集合も同じ 3 区分に揃え、一致を テストで固定する。反証が揃わない取り込みでは先に付いていた印を外す。 収束の判定の本体・終了コード・出力の変数は変えない。 Co-Authored-By: Claude Fable 5.1 --- .../skills/cross-review/scripts/measure.py | 7 +- .../ndf/skills/cross-review/scripts/state.py | 66 ++++-- .../tests/test_classify_findings.py | 196 +++++++++++++++++- .../cross-review/tests/test_critiques.py | 45 ++++ .../skills/cross-review/tests/test_measure.py | 21 +- 5 files changed, 303 insertions(+), 32 deletions(-) diff --git a/plugins/ndf/skills/cross-review/scripts/measure.py b/plugins/ndf/skills/cross-review/scripts/measure.py index 9fa53bf8..ba521c43 100755 --- a/plugins/ndf/skills/cross-review/scripts/measure.py +++ b/plugins/ndf/skills/cross-review/scripts/measure.py @@ -416,9 +416,10 @@ def _majority(representatives: list[dict[str, Any]], return _method_output(finding_ids, oracle_ids) -# 3 本目の区分のうち、この変更の方式が採る 2 つ（`state.py` の -# `COUNTED_CLASSIFICATIONS` と同じ）。**残る 3 つは採らない。** -COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment") +# 3 本目の区分（#732 で 6 つ）のうち、この変更の方式が採る 3 つ（`state.py` の +# `COUNTED_CLASSIFICATIONS` と同じ。一致は `test_measure.py` が固定する）。 +# **残る 3 つは採らない。** +COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment", "unrefuted") def _evidence_rounds(st: dict[str, Any]) -> set[int]: diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index f0a02e60..9ee5c2ac 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -3384,11 +3384,16 @@ def _handle_incomplete_critiques( ) -> None: """有効な反証が揃わなかったラウンドの扱い（#549 レビュー対応）。 - **印は付けない。** 印の無いラウンドは従来どおり全件を数えるため、未検証の `major` - が区分の絞り込みで落ちて収束することがない。**取り直しは同じラウンドで 1 度だけ + **印は付けず、先に付いていた印は外す**（#732）。印の無いラウンドは従来どおり全件を + 数えるため、反証が届いていない `major` が区分の絞り込みで落ちて収束することがない。 + 取り直しの後もそのラウンドに印が残ると、「印を付けないため、このラウンドは全件を + 数えます」の出力と実際の数え方が食い違う。**取り直しは同じラウンドで 1 度だけ である**（`judge` の結果なしと同じ作法。2 度続けて揃わないのは対象ではなく実行 環境の側の事象であり、そのときも印を付けないまま工程を進める）。 """ + st["evidence_rounds"] = [ + r for r in st.get("evidence_rounds") or [] + if not _same_round_no(r, round_no)] entry = next( (r for r in st.get("rounds") or [] if r.get("round") == round_no), None) relaunched = list((entry or {}).get("critique_relaunched") or []) @@ -3408,6 +3413,15 @@ def _handle_incomplete_critiques( sys.exit(7) +def _same_round_no(value: Any, round_no: int) -> bool: + """印の番号がそのラウンドを指すか。**番号の読み方は `_evidence_completed` と同じ** + （`int` へ換算して比べ、旧い状態ファイルの文字列の番号も同じラウンドとして読む）。""" + try: + return int(value) == int(round_no) + except (TypeError, ValueError): + return False + + # 実行の結果の強さ。**組から選び直すときの順である。** _VERIFY_RANK = {"reproduced": 2, "not_reproduced": 1, "not_run": 0} @@ -3426,9 +3440,10 @@ def _declared_duplicate_targets(finding: dict[str, Any]) -> set: return targets -# 収束の判定が数える区分（#156）。**残る 3 つは数えない。** 棄却した指摘を数えると、 -# そのぶんラウンドが増える（#69 で同じ論点が 5 ラウンド続いた事象）。 -COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment") +# 収束の判定が数える区分（#156、#732）。**残る 3 つは数えない。** 数えないのは、誤りだと +# 示された棄却と、承認を妨げない軽微な指摘だけである。棄却した指摘を数えると、そのぶん +# ラウンドが増える（#69 で同じ論点が 5 ラウンド続いた事象）。 +COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment", "unrefuted") def _verdicts(finding: dict[str, Any], verdict: str) -> list[str]: @@ -3441,11 +3456,16 @@ def _verdicts(finding: dict[str, Any], verdict: str) -> list[str]: def _classify_finding(finding: dict[str, Any]) -> str: - """1 件の指摘を 5 つの区分のいずれかへ分ける（#156）。 + """1 件の指摘を 6 つの区分のいずれかへ分ける（#156、#732）。 **上から順に見て、最初に当たった区分を採る。** 実行で再現した指摘を先に採ることで、 「実行の結果を担当の支持より先に見る」を順序そのもので表す。順 3 を先に置くと、 機械が再現した事実を担当の再評価が覆す。 + + **数えない側へ落とすのは、棄却（順 3）と `minor` 以下（順 6）だけである。** 誰にも + 誤りを示されていない `major` は、反証の有無・担当の数・根拠の 2 項目の有無によらず + `unrefuted`（順 5）として数える。「立証できない」「範囲外」は誤りだという主張では + ない（#706）。担当 1 者で反証する相手がいない指摘も同じである（#624）。 """ result = _verify_result(finding) major = _SEVERITY_RANK.get(str(finding.get("severity")), -1) >= _SEVERITY_RANK["major"] @@ -3456,18 +3476,32 @@ def _classify_finding(finding: dict[str, Any]) -> str: if result == "not_reproduced" or _verdicts(finding, "refute"): return "rejected" # **`minor` 以下は数えない。** 支持が 1 件付いただけでラウンドが増えるのを避ける。 - if finding.get("has_evidence") and major and ( - _verdicts(finding, "support") - or len(finding.get("origin_runtimes") or []) >= 2 - ): + if not major: + return "insufficient_evidence" + # **根拠の 2 項目は見ない。** 別の担当が支持した、または 2 者が独立に出した時点で + # 「確かめる」目的は果たされている（#706 で支持つきの 2 件が根拠の欠けで落ちた）。 + if _verdicts(finding, "support") or len(finding.get("origin_runtimes") or []) >= 2: return "needs_human_judgment" - return "insufficient_evidence" + return "unrefuted" + + +def _unrefuted_reason(finding: dict[str, Any]) -> str: + """なぜ独立に確かめられていないか。**反証の記録は提案者以外の値だけを持つ。** + + 空は「反証を返した担当が 0 者」を表す（`no_critique`）。1 件以上あれば、反証は + あるが支持も否定も無い（`not_supported`）。 + """ + return "not_supported" if finding.get("critiques") else "no_critique" def _apply_classification(finding: dict[str, Any]) -> str: - """区分を決めて要素へ書く。**棄却したものには理由を残す。**""" + """区分を決めて要素へ書く。**棄却と未反証には理由を残し、他の区分では消す。**""" classification = _classify_finding(finding) finding["classification"] = classification + if classification == "unrefuted": + finding["unrefuted_reason"] = _unrefuted_reason(finding) + else: + finding.pop("unrefuted_reason", None) if classification != "rejected": finding.pop("rejection_reason", None) return classification @@ -3492,9 +3526,9 @@ def _apply_classification(finding: dict[str, Any]) -> str: def _counted_finding_ids(st: dict[str, Any], round_no: int) -> list[str]: """新規性が数える指摘の `finding_id`（#156）。 - **数えるのは `verified_blocking` と `needs_human_judgment` だけである。** - 棄却した指摘を数えると、そのぶんラウンドが増える（#69 で同じ論点が 5 ラウンド - 続いた事象）。どちらも `major` 以上で、修正の工程へ渡る。 + **数えるのは `verified_blocking` と `needs_human_judgment` と `unrefuted` の 3 つ + である。** 棄却した指摘を数えると、そのぶんラウンドが増える（#69 で同じ論点が + 5 ラウンド続いた事象）。いずれも `major` 以上で、修正の工程へ渡る。 """ ids: list[str] = [] for finding in st.get("review_findings") or []: @@ -3742,7 +3776,7 @@ def _new_finding_count(st: dict[str, Any], pr: int) -> tuple[int, bool]: # 「測れなかった」と扱うと、元の REQUEST_CHANGES のまま終わらない。 if not curr: return 0, False - # **証拠集約を通ったラウンドだけを、数える 2 つへ絞る**（#156）。通っていない + # **証拠集約を通ったラウンドだけを、数える 3 つへ絞る**（#156、#732）。通っていない # ラウンドは従来どおり全件を数える（旧い状態ファイルと、3 本目より前に開いた # ラウンドがこれに当たる）。**`review_findings` の有無では判定しない** # （旧版でも取り込みの時点で積まれるため、区分も検証結果も持たない旧いラウンドが diff --git a/plugins/ndf/skills/cross-review/tests/test_classify_findings.py b/plugins/ndf/skills/cross-review/tests/test_classify_findings.py index 394b1f08..8207c248 100644 --- a/plugins/ndf/skills/cross-review/tests/test_classify_findings.py +++ b/plugins/ndf/skills/cross-review/tests/test_classify_findings.py @@ -100,16 +100,42 @@ def test_a_supported_minor_is_not_judged(state_mod): )) == "insufficient_evidence" -def test_support_without_evidence_is_insufficient(state_mod): +def test_support_without_evidence_needs_judgment(state_mod): + """**支持が付いた `major` は、根拠の 2 項目を欠いても人の判断待ちである**（#706。実測 F）。 + + 別の担当が支持を返した時点で「確かめる」目的は果たされている。根拠の欠けで落とすと、 + 2 人が同じことを言っている情報が判定に効かない。 + """ assert classify(state_mod, _finding( has_evidence=False, critiques=[_critique("kiro", "support")], - )) == "insufficient_evidence" + )) == "needs_human_judgment" + + +def test_two_proposers_are_enough_without_evidence(state_mod): + """2 者が独立に出した `major` は、根拠の 2 項目を欠いても人の判断待ちである（実測 H）。""" + assert classify(state_mod, _finding( + has_evidence=False, origin_runtimes=["codex", "kiro"], + )) == "needs_human_judgment" + + +# ---------- 順 5: 未反証（#732 #624 #706） ---------- +def test_a_major_nothing_matched_is_unrefuted(state_mod): + """誰にも誤りを示されていない `major` は数える側へ入る（実測 B）。""" + assert classify(state_mod, _finding()) == "unrefuted" -# ---------- 順 5 ---------- -def test_nothing_matched_is_insufficient(state_mod): - assert classify(state_mod, _finding()) == "insufficient_evidence" +def test_a_lone_major_with_evidence_is_unrefuted(state_mod): + """担当 1 者で反証する相手がいない `major` は未反証である（#624。実測 A）。""" + assert classify(state_mod, _finding(has_evidence=True)) == "unrefuted" + + +@pytest.mark.parametrize("verdict", ["insufficient_evidence", "out_of_scope"]) +def test_a_major_the_other_could_not_verify_is_unrefuted(state_mod, verdict): + """**「立証できない」「範囲外」は誤りだという主張ではない**（#706。実測 D・E）。""" + assert classify(state_mod, _finding( + has_evidence=True, critiques=[_critique("kiro", verdict)], + )) == "unrefuted" def test_not_run_is_not_the_same_as_not_reproduced(state_mod): @@ -123,7 +149,56 @@ def test_not_run_is_not_the_same_as_not_reproduced(state_mod): def test_a_finding_without_verification_is_readable(state_mod): f = _finding() del f["verification"] - assert classify(state_mod, f) == "insufficient_evidence" + assert classify(state_mod, f) == "unrefuted" + + +# ---------- 順 6: 立証不足（軽微な指摘の残余） ---------- + +def test_a_lone_minor_with_evidence_is_insufficient(state_mod): + """**`minor` 以下は反証の有無によらず数えない**（実測 C）。""" + assert classify(state_mod, _finding( + severity="minor", has_evidence=True)) == "insufficient_evidence" + + +# ---------- 未反証の理由 ---------- + +def test_an_unrefuted_finding_without_critiques_says_no_critique(state_mod): + f = _finding(has_evidence=True) + state_mod._apply_classification(f) + assert f["classification"] == "unrefuted" + assert f["unrefuted_reason"] == "no_critique" + + +def test_an_unrefuted_finding_with_critiques_says_not_supported(state_mod): + f = _finding(has_evidence=True, critiques=[_critique("kiro", "insufficient_evidence")]) + state_mod._apply_classification(f) + assert f["classification"] == "unrefuted" + assert f["unrefuted_reason"] == "not_supported" + + +def test_the_unrefuted_reason_is_dropped_when_the_classification_changes(state_mod): + """**理由は区分が未反証のときだけ存在する**（棄却の理由と同じ扱い）。""" + f = _finding(has_evidence=True) + state_mod._apply_classification(f) + assert f["unrefuted_reason"] == "no_critique" + + f["critiques"] = [_critique("kiro", "refute")] + state_mod._apply_classification(f) + + assert f["classification"] == "rejected" + assert "unrefuted_reason" not in f + assert "rejection_reason" in f + + +def test_other_classifications_carry_no_unrefuted_reason(state_mod): + for f in ( + _finding(verification=_verified("reproduced")), + _finding(has_evidence=True, critiques=[_critique("kiro", "support")]), + _finding(severity="minor"), + ): + state_mod._apply_classification(f) + assert f["classification"] != "unrefuted" + assert "unrefuted_reason" not in f # ---------- 棄却の理由 ---------- @@ -164,8 +239,11 @@ def counted(state_mod, findings, round_no=1): return state_mod._counted_finding_ids(st, round_no) -def test_only_two_classifications_are_counted(state_mod): - """**`rejected` と `insufficient_evidence` は数えない。**""" +def test_only_three_classifications_are_counted(state_mod): + """**数えないのは棄却（`rejected`）と軽微な指摘だけである**（#732）。 + + 誰にも誤りを示されていない `major`（`d`。未反証）は数える。 + """ out = counted(state_mod, [ _finding(finding_id="a", verification=_verified("reproduced")), _finding(finding_id="b", has_evidence=True, @@ -174,8 +252,9 @@ def test_only_two_classifications_are_counted(state_mod): _finding(finding_id="d"), _finding(finding_id="e", severity="minor", verification=_verified("reproduced")), + _finding(finding_id="f", severity="minor"), ]) - assert sorted(out) == ["a", "b"] + assert sorted(out) == ["a", "b", "d"] def test_a_merged_side_is_not_counted(state_mod): @@ -321,3 +400,102 @@ def test_measurability_is_decided_before_narrowing(state_mod, tmp_path, monkeypa assert measurable is True # 読めている assert count == 0 # 数える区分が 0 件 + + +# ---------- 担当 1 者・起動し直した担当の指摘を数える（#732 #624 #706） ---------- + +def _judge_rc(state_mod, pr): + import argparse + with pytest.raises(SystemExit) as e: + state_mod.cmd_judge(argparse.Namespace(pr=pr)) + return e.value.code + + +def _single_reviewer_state(tmp_path, finding, intent="REQUEST_CHANGES"): + """担当 1 者（`only: "codex"`）で印の付いたラウンドが 1 つある状態ファイル。 + + 担当 1 者は `only` で表す（`_round_reviewers` が最初に読む値）。判定が読めるよう、 + 状態ファイルと担当の payload を `CROSS_REVIEW_TMP_DIR` へ書く。 + """ + import json + st = { + "current_pr": 1, "repo": "o/r", "max_rounds": 12, "rotate_after": 8, + "only": "codex", "host": "claude", + "rounds": [{"round": 1, "pr": 1, "started_at": "2026-09-19T00:00:00+00:00", + "codex": {"intent": intent, + "by_severity": {finding["severity"]: 1}}}], + "evidence_rounds": [1], + "review_findings": [finding], + "deferred_nits": [], "carried_over": None, "final": None, + } + (tmp_path / "cross-review-pr1-state.json").write_text(json.dumps(st)) + _payload(tmp_path, "codex", 1, 1, [ + {"path": finding["path"], "line": finding["line"], "body": finding["body"], + "severity": finding["severity"]}]) + return st + + +def test_a_single_reviewer_unrefuted_major_is_counted(state_mod, tmp_path, monkeypatch): + """AC1: 反証する相手のいない `major` を数え、収束させない（#624 の再現）。 + + 変更前は `(0, True)` で新規 0 件となり、`REQUEST_CHANGES` のまま終了コード 0 + （承認）で終わっていた。 + """ + monkeypatch.setenv("CROSS_REVIEW_TMP_DIR", str(tmp_path)) + st = _single_reviewer_state(tmp_path, _finding(has_evidence=True)) + + assert state_mod._new_finding_count(st, 1) == (1, True) + assert st["review_findings"][0]["classification"] == "unrefuted" + assert _judge_rc(state_mod, 1) == 2 + + +def test_a_single_reviewer_major_without_evidence_is_still_counted( + state_mod, tmp_path, monkeypatch): + """AC2: 根拠の 2 項目を欠いても数える。`has_evidence` の値は残る。""" + monkeypatch.setenv("CROSS_REVIEW_TMP_DIR", str(tmp_path)) + st = _single_reviewer_state(tmp_path, _finding(has_evidence=False)) + + assert state_mod._new_finding_count(st, 1) == (1, True) + assert st["review_findings"][0]["classification"] == "unrefuted" + assert st["review_findings"][0]["has_evidence"] is False + assert _judge_rc(state_mod, 1) == 2 + + +def test_a_single_reviewer_minor_is_not_counted(state_mod, tmp_path, monkeypatch): + """AC3: `minor` は担当 1 者でも数えない。""" + monkeypatch.setenv("CROSS_REVIEW_TMP_DIR", str(tmp_path)) + st = _single_reviewer_state( + tmp_path, _finding(severity="minor", has_evidence=True)) + + assert state_mod._new_finding_count(st, 1) == (0, True) + assert st["review_findings"][0]["classification"] == "insufficient_evidence" + + +def test_a_relaunched_reviewers_major_without_critiques_is_counted( + state_mod, tmp_path, monkeypatch): + """AC4: 反証を取り込んだ後に入った担当の `major` を数える（#583 の収束の部分）。 + + `agy` + `kiro` のラウンドで、`kiro` の指摘には反証（`refute`）が付いて棄却され、 + `agy` の根拠を持つ `major` は反証 0 件のまま入っている。 + """ + monkeypatch.setenv("CROSS_REVIEW_TMP_DIR", str(tmp_path)) + agy = _finding(finding_id="agy-r1-0", agent="agy", origin_runtimes=["agy"], + path="a.py", line=1, body="x", has_evidence=True) + kiro = _finding(finding_id="kiro-r1-0", agent="kiro", origin_runtimes=["kiro"], + path="b.py", line=2, body="y", has_evidence=True, + critiques=[_critique("agy", "refute")]) + st = { + "current_pr": 1, "repo": "o/r", + "rounds": [{"round": 1, "pr": 1, "reviewers": ["agy", "kiro"]}], + "evidence_rounds": [1], + "review_findings": [agy, kiro], + } + _payload(tmp_path, "agy", 1, 1, [ + {"path": "a.py", "line": 1, "body": "x", "severity": "major"}]) + _payload(tmp_path, "kiro", 1, 1, [ + {"path": "b.py", "line": 2, "body": "y", "severity": "major"}]) + + assert state_mod._new_finding_count(st, 1) == (1, True) + assert agy["classification"] == "unrefuted" + assert agy["unrefuted_reason"] == "no_critique" + assert kiro["classification"] == "rejected" diff --git a/plugins/ndf/skills/cross-review/tests/test_critiques.py b/plugins/ndf/skills/cross-review/tests/test_critiques.py index 80b198f2..56e0d006 100644 --- a/plugins/ndf/skills/cross-review/tests/test_critiques.py +++ b/plugins/ndf/skills/cross-review/tests/test_critiques.py @@ -351,6 +351,51 @@ def test_the_retry_happens_once_per_round(tmp_dir, state_mod): assert st.get("evidence_rounds", []) == [] +def test_an_incomplete_collection_removes_an_existing_marker(tmp_dir, state_mod): + """**反証が揃わない取り込みは、先に付いていた印を外す**（#732 の AC13）。 + + 印が残ると「印を付けないため、このラウンドは全件を数えます」の出力と実際の数え方が + 食い違い、反証が届いていない `major` が区分の絞り込みへ掛かる。 + """ + _write(tmp_dir, _state([_finding("agy-r1-0", "agy")], evidence_rounds=[1])) + (tmp_dir / f"agy-review-pr{PR}-round1-payload.json").write_text(json.dumps( + {"comments": [{"path": "a.py", "line": 1, "body": "x", + "severity": "major"}]})) + + collect(state_mod, expect_rc=7) + + st = _read(tmp_dir) + assert st["evidence_rounds"] == [] + assert state_mod._evidence_completed(st, 1) is False + count, measurable = state_mod._new_finding_count(st, PR) + assert (count, measurable) == (1, True) # payload の全件を数える + + +def test_a_marker_stays_off_after_the_second_incomplete_collection(tmp_dir, state_mod): + """取り直した後も揃わないとき（2 度目）も印は付かない。""" + _write(tmp_dir, _state([_finding("codex-r1-0", "codex")], evidence_rounds=[1])) + + collect(state_mod, expect_rc=7) + collect(state_mod, expect_rc=0) + + st = _read(tmp_dir) + assert st["evidence_rounds"] == [] + assert sorted(st["rounds"][0]["critique_relaunched"]) == ["agy", "kiro"] + + +def test_an_incomplete_collection_keeps_other_rounds_markers(tmp_dir, state_mod): + """外すのはそのラウンドの番号だけである。前のラウンドの印は残る。""" + _write(tmp_dir, _state( + [_finding("codex-r2-0", "codex", round=2)], + rounds=[{"round": 1, "pr": PR}, {"round": 2, "pr": PR}], + evidence_rounds=[1, 2], + )) + + collect(state_mod, expect_rc=7) + + assert _read(tmp_dir)["evidence_rounds"] == [1] + + def test_a_proposer_only_round_is_marked_without_any_file(tmp_dir, state_mod): """**反証の対象が無い担当は不足に数えない。** 全員が提案者なら印が付く。""" _write(tmp_dir, _state([ diff --git a/plugins/ndf/skills/cross-review/tests/test_measure.py b/plugins/ndf/skills/cross-review/tests/test_measure.py index 3d9b9737..28967b5b 100644 --- a/plugins/ndf/skills/cross-review/tests/test_measure.py +++ b/plugins/ndf/skills/cross-review/tests/test_measure.py @@ -573,10 +573,10 @@ def test_proposed_reports_all_rounds_when_every_round_is_marked(measure_mod): "oracle_scope": "all_rounds", "oracle_base": 2} -def test_proposed_takes_only_the_two_counted_classifications(measure_mod): - """採るのは `verified_blocking` と `needs_human_judgment` の 2 つだけである。 +def test_proposed_takes_only_the_three_counted_classifications(measure_mod): + """採るのは `verified_blocking` / `needs_human_judgment` / `unrefuted` の 3 つである（#732）。 - 棄却した指摘と立証できなかった指摘は採らない。 + 棄却した指摘と軽微な指摘（立証不足）は採らない。 """ st = _state( evidence_rounds=[1], @@ -588,10 +588,23 @@ def test_proposed_takes_only_the_two_counted_classifications(measure_mod): classification="insufficient_evidence"), _finding("agy-r1-0", 1, "d.py", 40, agent="agy", classification="needs_human_judgment"), + _finding("agy-r1-1", 1, "e.py", 50, agent="agy", + classification="unrefuted", unrefuted_reason="no_critique"), ], ) - assert measure_mod.measure(st)["methods"]["proposed"]["found"] == 2 + assert measure_mod.measure(st)["methods"]["proposed"]["found"] == 3 + + +def test_the_counted_classifications_match_the_state_script(measure_mod, state_mod): + """**収束の判定と測定は同じ指摘を数える**（#732 の AC11）。 + + 片方だけに `unrefuted` を足すと、判定が数えた指摘を測定が採らず、この方式の再現率が + 実際より低く出る。 + """ + assert measure_mod.COUNTED_CLASSIFICATIONS == state_mod.COUNTED_CLASSIFICATIONS + assert set(measure_mod.COUNTED_CLASSIFICATIONS) == { + "verified_blocking", "needs_human_judgment", "unrefuted"} def test_proposed_ignores_findings_from_unmarked_rounds(measure_mod): From c70ef287beb48526d52f75b0a52cca40d4cbe0e7 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 11:26:08 +0000 Subject: [PATCH 02/21] =?UTF-8?q?Docs:=20=E5=8F=8D=E8=A8=BC=E3=81=AE?= =?UTF-8?q?=E3=83=97=E3=83=AD=E3=83=B3=E3=83=97=E3=83=88=E3=81=A8=E8=A6=8F?= =?UTF-8?q?=E7=B4=84=E3=83=BB=E7=A2=BA=E5=AE=9A=E4=BB=95=E6=A7=98=E3=82=92?= =?UTF-8?q?=206=20=E5=8C=BA=E5=88=86=E3=81=AB=E6=8F=83=E3=81=88=E3=80=81?= =?UTF-8?q?=E5=AE=9F=E8=A3=85=E8=A8=88=E7=94=BB=E3=82=92=E7=BD=AE=E3=81=8F?= =?UTF-8?q?=EF=BC=88#732=20#624=20#706=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 反証のプロンプトに「立証できないと返しても指摘は数から落ちない。誤りを 示せるなら否定を返す」を書く。規約 3 文書と確定仕様の区分の表を 6 行にし、 数えるのは 3 つと書き、揃わないときに印を外すことを足す。実装計画を issues/ に置き、設計文書の「未確認のまま残ること」を実装で決めた結果で更新する。 Co-Authored-By: Claude Fable 5.1 --- .../cross-review-evidence-based.md | 53 +++--- issues/issue-732-624-706-design.md | 6 +- issues/issue-732-624-706-plan.md | 162 ++++++++++++++++++ .../skills/cross-review/docs/04-contracts.md | 26 ++- .../docs/05-pool-and-convergence.md | 8 + .../skills/cross-review/docs/06-evidence.md | 59 ++++--- .../skills/cross-review/scripts/critique.sh | 4 + .../cross-review/tests/test_skill_layout.py | 30 ++++ 8 files changed, 292 insertions(+), 56 deletions(-) create mode 100644 issues/issue-732-624-706-plan.md diff --git a/docs/specifications/cross-review-evidence-based.md b/docs/specifications/cross-review-evidence-based.md index 6cde1da2..0e2983d9 100644 --- a/docs/specifications/cross-review-evidence-based.md +++ b/docs/specifications/cross-review-evidence-based.md @@ -17,8 +17,9 @@ **進行側は、担当の再評価より先に検証手順を実行する。** 機械が再現した事実は、担当の支持 より確かである。 -**指摘は 5 つの区分へ分かれ、収束の判定は担当の判定（`event`）ではなく区分を見る。** -数えるのは `verified_blocking` と `needs_human_judgment` の 2 つだけである。 +**指摘は 6 つの区分へ分かれ、収束の判定は担当の判定（`event`）ではなく区分を見る。** +数えるのは `verified_blocking` と `needs_human_judgment` と `unrefuted` の 3 つである。数えない +のは、誤りだと示された棄却と、承認を妨げない `minor` 以下の指摘だけである。 **却下した指摘は、位置・重要度・理由とともにラウンドをまたいで残る。** 次のラウンドの レビュープロンプトへ渡り、同じ論点が戻ることを止める。 @@ -37,7 +38,7 @@ | 反証条件 | 何が成り立てば棄却できるか（`falsification`） | | 検証手順 | 実行できる形で書いた確かめ方（`suggested_check`） | | 反証 | 提案者以外の担当が、各指摘へ返す 1 つの値 | -| 区分 | 1 件の指摘を分ける 5 つの分類（`classification`） | +| 区分 | 1 件の指摘を分ける 6 つの分類（`classification`） | | 方式 | 効果の測定で指摘を採る規則。`single` / `majority` / `proposed` / `oracle` の 4 つ | | 代表 | 統合した組で、判定が読む 1 件。束ねられた側は `merged_into` を持つ | | 印 | そのラウンドが統合・実行検証・反証を通ったこと（`evidence_rounds`） | @@ -82,9 +83,9 @@ | 反証は新しいラウンドを足さず、同じラウンドの中で回す | ラウンド数が 2 倍になり、収束の上限（12）の意味が変わる | | 申告による統合（2 段目）は次のラウンドへ回さない | 回すと、同じ主張を 2 者が別の本文で出した組が、統合される前に `insufficient_evidence` へ落ちて収束する | | 反証の値は担当ごとに置き換え、積み増さない | 取り直したときに古い値が残る。`refute` を `support` へ訂正しても両方が並び、区分の順で `refute` が先に当たって指摘が `rejected` のままになる | -| 収束の判定が数えるのは 2 区分だけである | 棄却した指摘と `minor` の指摘を数えると、そのぶんラウンドが増える（#69 の 5 ラウンド） | +| 数えないのは棄却した指摘と `minor` の指摘だけである（誤りを示されていない `major` は `unrefuted` として数える） | 棄却した指摘と `minor` の指摘を数えると、そのぶんラウンドが増える（#69 の 5 ラウンド）。誤りを示されていない `major` を数えないと、未解決の `major` を残して承認で収束する（#624 #706） | | 測れたかどうかは、区分で絞る**前**に決める | 絞った後の集合へ「空なら測れない」を適用すると、全件を棄却したラウンドが「測れなかった」ことになり、元の判定のままループが終わらない | -| 印（`evidence_rounds`）で母集合を決め、`review_findings` の有無では判定しない | 取り込みはこの変更より前から要素を積む。存在で判定すると、区分も検証結果も持たない旧いラウンドが絞り込みに掛かり、修正必須の `major` が落ちて収束する | +| 印（`evidence_rounds`）で母集合を決め、`review_findings` の有無では判定しない | 印の役割は、取り込みだけを済ませた旧いラウンドと反証が届いていないラウンドを、棄却と `minor` も含めて全件を数える側に置くことである。存在で判定すると、区分も検証結果も持たない旧いラウンドが絞り込みに掛かる。反証が揃わないときは印を付けず、先に付いていた印も外す | | 印を付けるのは経路の最後で、対象ごとに有効な反証が揃ったときだけである | 途中で付けると反証を結ぶ前の値で数える。結果ファイルの欠落でも付けると、未検証のまま収束する | | `needs_human_judgment` を人へのエスカレーションにしない | 収束のループはこの工程の中で回っており、止めて人を待つと自動で進まなくなる。決めるのは修正の担当で、その判断は却下の記録へ残る | | 振動の検知（一致の判定・閾値 0.5）は変えない | 母集合は指摘の構造化で既に広がっている。判定式まで同時に変えると、ラウンド数が動いたときにどちらが原因かを切り分けられない | @@ -106,7 +107,7 @@ - **束ねられた側は消さない。** `merged_into` を書いて残す。消すと、反証の結果がその指摘を 指したときに結び先を失う。判定・区分・測定はいずれも代表だけを数える - **実行できなかったこと（`not_run`）と、再現しなかったこと（`not_reproduced`）を同じに - しない。** 前者は区分の順 3 の前半に当たらず、区分は反証と根拠で決まる + しない。** 前者は区分の順 3 の前半に当たらず、区分は重大度と反証で決まる - **`ran_at` は実行した記録にだけ入る。** 実行しなかった記録では `exit_code` とともに `null` である。時刻が残ると、実行済みと見分けられない - **1 つの（ラウンド, 指摘, 担当）が持つ反証は 1 つである** @@ -189,7 +190,7 @@ ### 実行検証 **実行してよいのは、起動した側が渡したコマンドだけである。** 渡されなければ実行検証を -行わず、区分は根拠と反証で決まる。**新しい実行系は導入しない。** +行わず、区分は重大度と反証で決まる。**新しい実行系は導入しない。** | 守ること | なぜ | | --- | --- | @@ -231,24 +232,29 @@ **上から順に見て、最初に当たった区分を採る。** -| 順 | 区分 | 条件 | -| --- | --- | --- | -| 1 | `verified_blocking` | 再現した、かつ `major` 以上 | -| 2 | `verified_non_blocking` | 再現した、かつ `minor` 以下 | -| 3 | `rejected` | 再現しなかった、または `refute` が 1 件以上 | -| 4 | `needs_human_judgment` | 根拠を持ち、`major` 以上で、`support` が 1 件以上**または** `origin_runtimes` が 2 者以上 | -| 5 | `insufficient_evidence` | 上のいずれにも当たらない | +| 順 | 区分 | 条件 | 数える | 理由の項目 | +| --- | --- | --- | --- | --- | +| 1 | `verified_blocking` | 再現した、かつ `major` 以上 | はい | — | +| 2 | `verified_non_blocking` | 再現した、かつ `minor` 以下 | いいえ | — | +| 3 | `rejected` | 再現しなかった、または `refute` が 1 件以上 | いいえ | `rejection_reason` | +| 4 | `needs_human_judgment` | `major` 以上で、`support` が 1 件以上**または** `origin_runtimes` が 2 者以上 | はい | — | +| 5 | `unrefuted` | `major` 以上（上のいずれにも当たらない） | はい | `unrefuted_reason`（`no_critique` / `not_supported`） | +| 6 | `insufficient_evidence` | 上のいずれにも当たらない（`minor` 以下） | いいえ | — | **順 3 を順 1・2 より先に置かない。** 置くと、機械が再現した事実を担当の再評価が覆す。 **独立に到達した担当の数を、支持と並べて数える。** 担当は 2 者であるため、2 者が同じ指摘を 独立に出すと提案者以外が 1 人も残らず、支持は必ず 0 件になる。支持の数だけを見ると、最も -強い一致である全員一致が `insufficient_evidence` へ落ちて収束する。 +強い一致である全員一致が、誰も確かめていない指摘と同じ `unrefuted` になり、独立に到達した +事実が記録に残らない。 **順 4 の分岐は、ラウンドの担当が 2 者であることに支えられている。** 担当を 3 者以上へ 広げると、単一の `refute` が順 3 で全員一致を覆す。広げるときに順 3 と順 4 の順序を決め直す。 -**棄却には理由を残す**（実行の結果か、`refute` の理由）。 +**棄却と未反証には理由を残す。** 棄却は実行の結果か `refute` の理由、未反証はなぜ独立に +確かめられていないか（反証を返した担当が 0 者なら `no_critique`、反証はあるが支持も否定も +無ければ `not_supported`）である。「立証できない」「範囲外」は誤りだという主張ではないため、 +その反証を受けた `major` は数から落とさない。 ### 区分ごとの行き先 @@ -256,6 +262,7 @@ | --- | --- | --- | | `verified_blocking` | 渡す | 直す。機械が再現しているため判断の余地は無い | | `needs_human_judgment` | 渡す | **修正の担当が読んで決める。** 直す・却下の理由を返す・範囲外として起票する | +| `unrefuted` | 渡す | 同上。理由（`unrefuted_reason`）から、誰も見ていないのか相手が確かめられなかったのかを読める | | `verified_non_blocking` | 渡さない | 最終スイープ。再現した事実は記録に残る | | `insufficient_evidence` | 渡さない | 同上 | | `rejected` | 渡さない | 棄却の理由が残り、次のラウンドのレビュープロンプトへ渡る | @@ -270,8 +277,8 @@ | 状態 | 返る値 | | --- | --- | | 指摘の記録を読めない | `(0, 測れない)`。従来の判定（全員が pass か）へ落ちる | -| 記録はあるが、数える 2 区分が 0 件 | **`(0, 測れた)`。収束させる** | -| 記録があり、数える 2 区分に一致しない指摘がある | `(件数, 測れた)` | +| 記録はあるが、数える 3 区分が 0 件 | **`(0, 測れた)`。収束させる** | +| 記録があり、数える 3 区分に一致しない指摘がある | `(件数, 測れた)` | **記録が読めることと、数える対象があることは別である。** 全件を棄却したラウンドは「新しい 修正必須の指摘が 0 件」であって、測れなかったラウンドではない。 @@ -288,7 +295,7 @@ | --- | --- | | `single` | 1 者だけの結果。担当ごとに 1 通り出す | | `majority` | `origin_runtimes` が 2 者以上の指摘 | -| `proposed` | 区分が `verified_blocking` または `needs_human_judgment` の指摘 | +| `proposed` | 区分が `verified_blocking` / `needs_human_judgment` / `unrefuted` の指摘。状態の管理スクリプトが数える集合と同じで、一致をテストで固定する | | `oracle` | いずれかの担当が出した指摘のうち、**修正された**もの | **`origin_runtimes` を持たない指摘は、取り込み時の担当 1 者として読む。** この値は統合の @@ -347,7 +354,7 @@ | `origin_runtimes` / `merged_from` / `merged_into` / `evidence_from` / `duplicate_candidates` | 統合 | | `verification` | 実行検証。`command` / `exit_code` / `result` / `finding_id` / `ran_at` | | `critiques` | 反証。要素は `agent` / `verdict` / `reason`（`duplicate` のときは `duplicate_of`） | -| `classification` / `rejection_reason` | 区分 | +| `classification` / `rejection_reason` / `unrefuted_reason` | 区分。理由の 2 つはその区分のときだけ持つ | **識別子は取り込みの時点で採番する。** 形は `<担当>-r<ラウンド>-<索引>` で、索引はその担当の `payload.json` の並びである。同じ組の取り込みは入れ替えであるため、再実行しても同じ指摘へ @@ -413,10 +420,10 @@ measure.py <状態ファイルのパス> [--output <パス>] | 提案者以外だけが賛否を返し、5 つの値が記録されること | 同 `tests/test_critiques.py` | | 結び先の無い反証が残ること | 同上 | | 反証が揃わないラウンドに印が付かず、1 度だけ取り直すこと | 同上 | -| 5 つの区分へ分かれ、棄却に理由が残ること | 同 `tests/test_classify_findings.py` | +| 6 つの区分へ分かれ、棄却と未反証に理由が残ること | 同 `tests/test_classify_findings.py` | | 再現した指摘が反証があっても棄却されず、再現しない指摘が支持が多くても棄却されること | 同上 | | 全員一致の指摘が収束しないこと | 同上 | -| 新規性が 2 区分だけを数え、全件を棄却したラウンドが収束すること | 同上 | +| 新規性が 3 区分を数え、全件を棄却したラウンドが収束すること | 同上 | | 走らせる順序が手順と実装で揃っていること | 同 `tests/test_findings_pipeline_wiring.py` | | 4 つの方式が同じ記録から出て、統合された指摘を 2 回数えないこと | 同 `tests/test_measure.py` | | `origin_runtimes` を持たない記録でも担当別の件数が残ること | 同上 | @@ -431,7 +438,7 @@ measure.py <状態ファイルのパス> [--output <パス>] ## 運用 **実行検証を使うかは、起動する側が決める。** 引数を渡さないリポジトリでは実行検証が走らず、 -区分は根拠と反証で決まる。既定の一覧は持たない（リポジトリによってテストの起動が違う）。 +区分は重大度と反証で決まる。既定の一覧は持たない（リポジトリによってテストの起動が違う）。 **印を持たない記録では `proposed` を計算できず、位置の記録を持たない記録では `oracle` と 再現率を計算できない。** この変更より前に回した Pull Request が該当する。変更の前後を diff --git a/issues/issue-732-624-706-design.md b/issues/issue-732-624-706-design.md index ffd65d26..f0718089 100644 --- a/issues/issue-732-624-706-design.md +++ b/issues/issue-732-624-706-design.md @@ -331,7 +331,7 @@ graph TD ## 未確認のまま残ること -6 件である。実装で決めるもの 2 件（テストの置き場所、プロンプトの文言）と、配布後の運用か別の課題で決まるもの 4 件に分かれる。 +6 件である。実装で決めたもの 2 件（テストの置き場所、プロンプトの文言）と、配布後の運用か別の課題で決まるもの 4 件に分かれる。 | 項目 | 内容 | いつ決まるか | | --- | --- | --- | @@ -339,8 +339,8 @@ graph TD | 未反証が多いときの修正の担当の負荷 | 誰も確かめていない重大な指摘が修正の工程へ渡る件数が増える。却下の理由を書く回数が増える | 同上 | | 担当を 3 者以上へ広げたときの順 3 と順 4 | 確定仕様が「広げるときに決め直す」としている。この変更は 2 者のまま | #478 の後 | | 立証不足の区分の改名 | 決定 6 で残す。意味が狭まった名前をいつ付け替えるかは未決 | 要求が出たとき | -| テストの置き場所 | テスト設計の表の置き場所は既存ファイルに合わせた目安である | **実装で決める** | -| プロンプトの文言 | 決定 9 の段落は、含める 2 つの内容だけを決めた | **実装で決める** | +| テストの置き場所 | テスト設計の表のとおりに置いた。区分の単体と AC1〜AC4・AC10 は `test_classify_findings.py`、AC11・AC12 は `test_measure.py`、AC13 は `test_critiques.py`、AC18〜AC20 は `test_skill_layout.py` | 実装で決めた（実装 Pull Request） | +| プロンプトの文言 | 「返す値」の表の下に 3 文を置いた。「立証できない」を返しても指摘は数から落ちず未反証として修正の工程へ渡ること、誤りを示せるなら何がそう言えるかを理由へ書いて否定を返すこと、指摘を数から落とす手段は否定だけであること | 実装で決めた（実装 Pull Request） | ## 申し送り（並行する設計との境界） diff --git a/issues/issue-732-624-706-plan.md b/issues/issue-732-624-706-plan.md new file mode 100644 index 00000000..393859fe --- /dev/null +++ b/issues/issue-732-624-706-plan.md @@ -0,0 +1,162 @@ +# cross-review: 誤りを示されていない重大な指摘が数えられずに承認で終わる → 数えない指摘を棄却と軽微な指摘に限る（#732 #624 #706 の実装計画） + +## 関連リンク + +| 文書 | 何を持つか | +| --- | --- | +| [issue-732-624-706-requirements.md](issue-732-624-706-requirements.md) | 何を満たすか（受け入れ条件 AC1〜AC22） | +| [issue-732-624-706-design.md](issue-732-624-706-design.md) | どう作るか（決定 1〜11、区分の条件、入出力の契約、テスト設計）。**業務用語と識別子の対応表はこの文書の冒頭にある** | +| 親 #732、子 #624 #706 | 課題。設計 Pull Request は #783 | + +## モード + +`standard`。収束の判定が数える指摘の範囲（本番の振る舞い）を変えるため。 + +## 目的と非目的 + +達成したい状態: + +- 収束の判定が数えないのは、誤りだと示された指摘（棄却）と、承認を妨げない軽微な指摘だけになる +- 誰にも誤りを示されていない重大な指摘は、新しい区分「未反証」として数えられ、修正の工程へ渡る。「なぜ独立に確かめられていないか」の理由が状態ファイルに残る +- 効果の測定の方式「この変更の方式」が、収束の判定と同じ 3 区分を数える +- 反証が揃わなかったラウンドは、先に付いていた印が外れ、全件を数える側へ戻る +- 規約 3 文書と確定仕様が、実装と同じ差分で新しい区分を書く + +やらないこと: + +- 収束の判定の本体・終了コード・標準出力の変数の変更（#729 の束が持つ境界） +- 担当を 1 者へ絞る指定の意味づけ（#727 の束） +- 投稿の重なりの扱い（#730 の束） +- 軽微な指摘を数えること +- 立証不足の区分の改名（設計文書の決定 6） +- 既存の設計文書（`issue-624-478-648-design.md`）の本体の書き換え（設計文書の決定 1） + +## 前提 + +- 前提 1: 反証の記録は提案者以外の担当の値だけを持つ。そのため、記録が空であることは「反証を返した担当が 0 者」と同じである（状態の管理スクリプトの取り込みが提案者自身の値を落とす。既存のテスト `test_a_critique_on_own_finding_is_dropped` が固定する） +- 前提 2: 区分は保存された値を読まず、判定のたびに計算し直す。そのため、この変更より前の状態ファイルに移行の処理は要らない +- 前提 3: 収束の判定の本体を触らずに数え方を変えられる。数える指摘の抽出が数える集合を参照しており、集合の値を変えれば判定の本体は変わらない（`develop` 2b140606 の `_new_finding_count` → `_counted_finding_keys` → `COUNTED_CLASSIFICATIONS` の呼び出しで確認） + +## 受け入れ条件 + +要求文書の AC1〜AC22 をそのまま使う。条件ごとの検証手段は設計文書の「テスト設計」の表にある。この計画では、タスクごとに満たす条件の番号を書く。 + +## ドメイン用語 + +設計文書の「用語の対応表」を正本とし、ここには写さない。本文は業務用語で書き、識別子はコードブロック・表・ファイルの指し示しにだけ使う。 + +## 不変条件 + +- 実行で再現した指摘は、担当の反証によらず「再現した」の区分に入る（順 1・2 が最初に当たる） +- 誤りだと示された指摘（再現しなかった、または否定が 1 件以上）は、重大度によらず棄却になる（順 3 が順 4・5 より先に当たる） +- 軽微な指摘は、反証の有無・支持の有無によらず数えない +- 未反証の理由は、区分が未反証のときだけ存在する。区分が変われば消える（棄却の理由と同じ扱い） +- 数える区分の集合は、状態の管理スクリプトと測定スクリプトで同じ値を持つ + +## 互換性 + +| 対象 | 変更 | 互換性の扱い | +| --- | --- | --- | +| 状態の管理スクリプトの引数・終了コード・標準出力の変数 | 変えない | 変えない | +| 状態ファイルの `review_findings[].classification` | 値の集合が 5 つから 6 つになる（`unrefuted` が増える） | 追加のみ。読む側は測定スクリプトだけで、知らない値は「採らない」に落ちる | +| 状態ファイルの `review_findings[].unrefuted_reason` | 新設 | 追加のみ。`version` は上げない。旧い状態ファイルは次の判定で区分が付け直される | +| 反証のプロンプトの返す値の語彙 | 変えない | 説明の段落を足すだけ | + +## 修正対象 + +```text +plugins/ndf/skills/cross-review/ +├── docs/04-contracts.md +├── docs/05-pool-and-convergence.md +├── docs/06-evidence.md +├── scripts/critique.sh +├── scripts/measure.py +├── scripts/state.py +└── tests/ + ├── test_classify_findings.py + ├── test_critiques.py + ├── test_measure.py + └── test_skill_layout.py +docs/specifications/cross-review-evidence-based.md +issues/issue-732-624-706-design.md（「未確認のまま残ること」の 2 行だけ） +``` + +配布物の生成（`bash scripts/build-runtime-plugins.sh`）で変わるファイルがあれば同じ Pull Request に含める。 + +## タスク分解 + +機能単位で分ける。各タスクは、失敗するテスト → 通す最小実装 → 整理の順で進める。 + +### Task 1: 誤りを示されていない重大な指摘を「未反証」として数え、理由を残す + +- **対象ファイル:** `scripts/state.py`（`_classify_finding` / `_apply_classification` / `COUNTED_CLASSIFICATIONS` とその注記）、`tests/test_classify_findings.py` +- **変更内容:** 区分の判定を 6 区分にする。順 4（人の判断待ち）の条件から根拠の有無を外し、順 5 に未反証（重大な指摘の残余）を置き、順 6 を立証不足（軽微な指摘の残余）にする。区分の書き込みは、未反証に理由（反証の記録が空なら `no_critique`、あれば `not_supported`）を書き、他の区分では理由を消す。数える集合に `unrefuted` を足す。関数の docstring と注記の「5 つ」「2 つだけ」を新しい数に合わせる +- **テスト:** 設計文書の実測 A〜K を単体で固定する（A・B・D・E・F・H が変わる 6 件、C・G・I・J・K が変わらない 5 件）。AC1〜AC4 は状態ファイルを組み、新しい指摘の数え上げと収束の判定の終了コードで確かめる（既存の `test_the_new_count_uses_the_classification` の形）。AC10 は理由の値と、区分が変わったときに消えることを見る。期待値を変える既存のテストは AC16 の 4 件だけで、名前を新しい振る舞いに合わせて変える +- **満たす受け入れ条件:** AC1〜AC10、AC14、AC16 +- **進め方:** 失敗するテスト → 最小実装 → 整理 + +### Task 2: 効果の測定の方式「この変更の方式」を、数える 3 区分に揃える + +- **対象ファイル:** `scripts/measure.py`（`COUNTED_CLASSIFICATIONS` とその注記）、`tests/test_measure.py` +- **変更内容:** 測定スクリプトの数える集合に `unrefuted` を足す。注記の「2 つ」「残る 3 つ」を「3 つ」「残る 3 つ」に直す +- **テスト:** 2 つのスクリプトの集合が等しく、3 つの値を持つこと（AC11）。印のあるラウンドの未反証を「この変更の方式」が数えること（AC12。既存の `test_proposed_takes_only_the_two_counted_classifications` を 3 区分へ改める） +- **満たす受け入れ条件:** AC11、AC12 +- **進め方:** 失敗するテスト → 最小実装 → 整理 + +### Task 3: 反証が揃わない取り込みでは、先に付いていた印を外す + +- **対象ファイル:** `scripts/state.py`（`_handle_incomplete_critiques`）、`tests/test_critiques.py` +- **変更内容:** 反証の不足の扱いの先頭で、そのラウンドの番号を印の一覧から除く。取り直す担当を返す動きと終了コード 7 は変えない +- **テスト:** 印の付いた状態で反証の取り込みを 2 回呼び（1 回目は取り直し、2 回目も揃わない）、印が消えていることと、新しい指摘の数え上げがレビュー結果の本体の全件を数えることを見る（AC13） +- **満たす受け入れ条件:** AC13、AC15 +- **進め方:** 失敗するテスト → 最小実装 → 整理 + +### Task 4: 反証のプロンプトに「立証できないと返しても指摘は数から落ちない」を書く + +- **対象ファイル:** `scripts/critique.sh`、`tests/test_skill_layout.py` +- **変更内容:** 「返す値」の表の下に 1 段落を足す。書くのは 2 つ。「立証できない」を返しても指摘は数から落ちず修正の工程へ渡ること、誤りを示せるなら理由を添えて否定を返すこと。返す値の語彙は変えない +- **テスト:** プロンプトの文字列に「数から落ち」が含まれること（AC19） +- **満たす受け入れ条件:** AC19 +- **進め方:** 失敗するテスト → 最小実装 + +### Task 5: 規約 3 文書と確定仕様を、6 区分・数える 3 つへ揃える + +- **対象ファイル:** `docs/04-contracts.md`、`docs/05-pool-and-convergence.md`、`docs/06-evidence.md`、`docs/specifications/cross-review-evidence-based.md`、`tests/test_skill_layout.py` +- **変更内容:** 規約 06 の区分の表を 6 行にし、数えるのは 3 つと書き、反証が揃わないときに印を外すことを書く。規約 04 の区分の項を 6 区分・数える 3 つにし、`unrefuted_reason` の項を足す。規約 05 の終了基準に、担当 1 者と起動し直した担当の指摘の数え方を足す。確定仕様の概要・決定の表・区分の表・行き先の表・収束の判定の表・測定の表・テスト観点の行を揃える（経緯の節は仕様化の工程が足す）。測定の方式の表（規約 06・確定仕様）も 3 区分にする +- **テスト:** AC18 の 4 つの grep と AC20 の grep を配置テストで固定する +- **満たす受け入れ条件:** AC18、AC20 +- **進め方:** 失敗するテスト → 文書の更新。文書は `markdown-writing` の規約で書く（説明文の主語・目的語に識別子を置かない） + +### Task 6: 検証と配布物の同期 + +- **対象ファイル:** 生成物（`bash scripts/build-runtime-plugins.sh` が変えるもの）、`issues/issue-732-624-706-design.md`（「未確認のまま残ること」の「テストの置き場所」「プロンプトの文言」の 2 行を決めた結果で更新） +- **変更内容:** 全体テスト、フロントマターの検査、文書の鮮度・リンク・行数の検査を通す。配布物を同期する +- **満たす受け入れ条件:** AC17（収束の判定の本体の行が差分に無いことを `git diff` で見る）、AC21、AC22 +- **進め方:** コマンドの実行と結果の記録（テスト駆動は当たらない。検証の工程である） + +## 影響範囲 + +- 印のあるラウンドで、反証を受けていない・支持されていない・根拠の項目を欠く重大な指摘が数えられる。2 者で相手が「立証できない」「範囲外」を返した重大な指摘も数える。収束までのラウンド数が増えることがある（指摘 1 件につき最大 1 回） +- 修正の担当が読む指摘に未反証が加わる。扱いは人の判断待ちと同じ（直す・却下の理由を返す・範囲外として起票する） +- 効果の測定の「この変更の方式」の再現率が、収束の判定と同じ母集合で出る + +## リスクと対処 + +| リスク | 対処 | +| --- | --- | +| 状態の管理スクリプトは 3700 行を超える 1 ファイルで、G3（#729）が同じファイルの別の関数を同時に触る | タスクごとにテストを通す。触る関数を区分の判定・書き込み・数える集合・反証の不足の扱いの 4 つに限り、収束の判定の本体の行を書き換えない。競合は後からマージする側が解く | +| 数える集合が 2 か所にあり、片方だけ変わる | Task 2 の一致のテストが固定する | +| 文書の「2 つだけ」「5 つの区分」の記述が残る | Task 5 で `grep -rn "2 つだけ\|2 区分\|5 つの区分" plugins/ndf/skills/cross-review docs/specifications/cross-review-evidence-based.md` を実行し、0 件を確かめる | +| 実装の後の構造改善で足りるか | 触る範囲が狭く（関数 4 つ・定数 2 つ）、区分のテストが 30 件以上ある。構造改善は後の工程（`cross-refactoring`）で足りる | + +## 切り戻し手順 + +- Pull Request の revert で戻せる。状態ファイルの `version` を上げないため、データの移行は無い。戻した後の状態ファイルに残る `unrefuted` / `unrefuted_reason` は、次の判定で区分が付け直されるときに上書き・削除される（区分は毎回計算し直す） + +## 完了の定義 + +- [ ] AC1〜AC22 をすべて満たし、条件ごとに検証手段と結果が対応している +- [ ] `uv run --with pytest pytest scripts/tests plugins/ndf -q` が通る +- [ ] AC22 の 4 つの検査が終了コード 0 で終わる +- [ ] 収束の判定の本体（`cmd_judge`）の行が差分に含まれない +- [ ] 配布物が同期され、pre-commit の検査が通る diff --git a/plugins/ndf/skills/cross-review/docs/04-contracts.md b/plugins/ndf/skills/cross-review/docs/04-contracts.md index b73c2bf3..d630f829 100644 --- a/plugins/ndf/skills/cross-review/docs/04-contracts.md +++ b/plugins/ndf/skills/cross-review/docs/04-contracts.md @@ -106,17 +106,27 @@ **1 つの `(ラウンド, finding_id, 担当)` が持つ値は 1 つである。** 取り直した反証は 古い値へ積まず置き換える（積むと、`refute` を `support` へ訂正しても両方が並び、 区分の順で `refute` が先に当たって指摘が `rejected` のままになる） -- `review_findings[].classification` — 5 つの区分（#156）。**収束の判定が数えるのは - `verified_blocking` と `needs_human_judgment` の 2 つだけである** +- `review_findings[].classification` — 6 つの区分（#156、#732）。値は `verified_blocking` / + `verified_non_blocking` / `rejected` / `needs_human_judgment` / `unrefuted` / + `insufficient_evidence`。**収束の判定が数えるのは `verified_blocking` と + `needs_human_judgment` と `unrefuted` の 3 つである。** 数えないのは、誤りだと示された + 棄却と、承認を妨げない `minor` 以下だけである +- `review_findings[].unrefuted_reason` — 未反証の理由（#732）。**`classification` が + `unrefuted` のときだけ持つ。** 値は `no_critique`（反証を返した担当が 0 者）/ + `not_supported`（反証はあるが支持も否定も無い）。区分が変わると消える（`rejection_reason` + と同じ扱い） - `unmatched_critiques` — 結び先の無い反証（#156）。**捨てない**（反証 0 件のラウンドと、 結び先を誤ったラウンドを区別するため） - `evidence_rounds` — 証拠集約（統合・実行検証・反証）を通ったラウンドの番号（#156）。 - **収束の判定はこの印で母集合を決める。** 印を持つラウンドだけを区分の 2 つへ絞り、 - 持たないラウンドは従来どおり全件を数える。**`review_findings` の有無では判定しない** - （取り込みはこの変更より前から要素を積むため、区分も `verification` も持たない旧い - ラウンドが絞り込みに掛かり、修正必須の `major` が `insufficient_evidence` へ落ちて - 新規 0 件で収束する）。印を書くのは経路の最後（`collect-critiques`）で、**対象ごとに - 有効な反証が揃ったときだけである** + **収束の判定はこの印で母集合を決める。** 印を持つラウンドだけを数える 3 区分へ絞り、 + 持たないラウンドは従来どおり全件を数える。印の役割は、取り込みだけを済ませた旧いラウンドと、 + 反証が届いていないラウンドを、棄却と `minor` 以下も含めて全件を数える側に置くことである + （`major` は誤りを示されていなければ `unrefuted` として数えられるが、否定が届いていない + かもしれないラウンドでは全件を数える側が安全である）。**`review_findings` の有無では判定 + しない**（取り込みは印より前から要素を積むため、区分も `verification` も持たない旧い + ラウンドが絞り込みに掛かる）。印を書くのは経路の最後（`collect-critiques`）で、**対象 + ごとに有効な反証が揃ったときだけである**。揃わないときは付けないだけでなく、**先に付いて + いたそのラウンドの印を外す**（取り直しの後も印が残ると、出力と実際の数え方が食い違う） - `rounds[].critique_relaunched` — 反証を取り直した担当（#549 レビュー対応）。 **同じラウンドで 1 度だけ取り直す**ための控えである - `rejected_findings` — 却下した指摘を **per-item** で蓄積する。`rounds[].fix.rejected` は diff --git a/plugins/ndf/skills/cross-review/docs/05-pool-and-convergence.md b/plugins/ndf/skills/cross-review/docs/05-pool-and-convergence.md index 48ef20bd..0800e1ad 100644 --- a/plugins/ndf/skills/cross-review/docs/05-pool-and-convergence.md +++ b/plugins/ndf/skills/cross-review/docs/05-pool-and-convergence.md @@ -51,6 +51,14 @@ Step 1（ラウンドの開始）と Step 3（判定）が読む基準を持つ しまう。測れないときは従来の判定（全員が pass か）に従い、出力の `NEW_FINDINGS` は `-` に なる。 +**担当が 1 者のラウンドと、起動し直した担当の指摘は、未反証（`unrefuted`）として新規性の層が +数える。** 担当が 1 者なら反証を返す相手がいない。起動し直した担当の指摘は、反証を取り込んだ +後に取り込まれるため反証を持たない。いずれも誰も誤りを示していない重大な指摘であり、数えない +と未解決のまま承認で収束する。1 者で回したラウンドの収束は、この新規性と、全員が通したかの +2 つで決まる。担当が承認か、重大な指摘の無いコメントを返せば、全員が通したとして収束する。 +修正要求なら、未反証の重大な指摘が新規に数えられ、修正の工程へ進む。修正の後のラウンドで同じ +指摘が戻れば、前のラウンドと一致して新規 0 件になる。戻らなければ承認で収束する。 + **全員 `APPROVE` は、最も止まらない参加者に律速される。** #156 が観測した 6 ラウンドでは、 一方が round 4 以降 3 ラウンド連続で承認を返す間、もう一方が round 6 まで指摘を出し続け、 round 5 の指摘は修正済みの箇所を指していた。再提出された指摘は Pull Request に残り、 diff --git a/plugins/ndf/skills/cross-review/docs/06-evidence.md b/plugins/ndf/skills/cross-review/docs/06-evidence.md index b68bdf75..d828fd68 100644 --- a/plugins/ndf/skills/cross-review/docs/06-evidence.md +++ b/plugins/ndf/skills/cross-review/docs/06-evidence.md @@ -97,12 +97,15 @@ 母集合を決める。`review_findings` の有無では、取り込みだけを済ませた旧いラウンドと 区別できない（[04-contracts.md](04-contracts.md)）。 -**通り切ったかどうかは、対象ごとに有効な反証が揃ったかで見る。** 結果ファイルの欠落や -不正でも印を付けると、実行検証を持たない単独の `major` が `insufficient_evidence` へ -落ち、新規 0 件のまま**未検証で収束する**。`collect-critiques` は揃わないときに印を -付けず、終了コード 7 と `CRITIQUE_RETRY_AGENTS` を返して再取得へ戻す。**取り直しは -同じラウンドで 1 度だけである**（`judge` の結果なしと同じ作法）。2 度目も揃わなければ -印を付けないまま進み、そのラウンドは全件を数える。 +**通り切ったかどうかは、対象ごとに有効な反証が揃ったかで見る。** 印の役割は、取り込み +だけを済ませた旧いラウンドと、反証が届いていないラウンドを、全件を数える側に置くことである。 +印の無いラウンドは棄却と軽微な指摘も数えるため、否定が届いていないかもしれないラウンドでは +全件を数える側が安全である。`collect-critiques` は揃わないときに印を付けず、**先に付いて +いたそのラウンドの印を外す**。外さずに取り直しへ進むと、「印を付けないため、このラウンドは +全件を数えます」の出力と実際の数え方が食い違う。そのうえで終了コード 7 と +`CRITIQUE_RETRY_AGENTS` を返して再取得へ戻す。**取り直しは同じラウンドで 1 度だけである** +（`judge` の結果なしと同じ作法）。2 度目も揃わなければ印を付けないまま進み、そのラウンドは +全件を数える。揃えば印を付ける処理が印を戻す。 **監視へ渡すのは、実際に起動した担当だけである。** `critique.sh` は反証の対象が無い 担当（自分の指摘だけ、または指摘 0 件）で `launch-cli.sh` を呼ばずに終わるため @@ -112,8 +115,9 @@ 冒頭で捨てる（`` はラウンドを名前に持たないため、残骸を起動済みと読む）。 **申告による統合（2 段目）は次のラウンドへ回さない。** 回すと、同じ主張を 2 者が別の -本文で出した組が、どちらも `origin_runtimes` 1 者・`support` 0 件のまま -`insufficient_evidence` へ落ち、統合される前に収束する。 +本文で出した組が、どちらも `origin_runtimes` 1 者・`support` 0 件のまま別々の `unrefuted` +として 2 件に数えられ、「2 者が独立に到達した」という一致が `needs_human_judgment` として +記録に残らない。軽微な指摘なら `insufficient_evidence` へ落ち、統合される前に収束する。 ## 反証 @@ -163,8 +167,8 @@ ことは別である。 **束ねた組の全員の `suggested_check` を対象にする。** 代表の値だけを読むと、代表が手順を -書いていない組は、束ねられた側が実行できる手順を書いていても `not_run` のまま -`insufficient_evidence` へ落ちる。**どちらが先に取り込まれたかで採否が変わる。** 代表が +書いていない組は、束ねられた側が実行できる手順を書いていても `not_run` のままで、機械が +再現した事実が区分に効かない。**どちらが先に取り込まれたかで区分が変わる。** 代表が 持つのは組の集約で、`reproduced` > `not_reproduced` > `not_run` の順で最初に当たった 1 件を採り、出所を `verification.finding_id` へ残す。**同じコマンドは 1 度しか実行しない。** @@ -172,19 +176,30 @@ **上から順に見て、最初に当たった区分を採る。** -| 順 | 区分 | 条件 | -| --- | --- | --- | -| 1 | `verified_blocking` | 再現した、かつ `major` 以上 | -| 2 | `verified_non_blocking` | 再現した、かつ `minor` 以下 | -| 3 | `rejected` | 再現しなかった、または `refute` が 1 件以上 | -| 4 | `needs_human_judgment` | 根拠を持ち、`major` 以上で、`support` が 1 件以上または `origin_runtimes` が 2 者以上 | -| 5 | `insufficient_evidence` | 上のいずれにも当たらない | +| 順 | 区分 | 条件 | 数える | 理由の項目 | +| --- | --- | --- | --- | --- | +| 1 | `verified_blocking` | 再現した、かつ `major` 以上 | はい | — | +| 2 | `verified_non_blocking` | 再現した、かつ `minor` 以下 | いいえ | — | +| 3 | `rejected` | 再現しなかった、または `refute` が 1 件以上 | いいえ | `rejection_reason` | +| 4 | `needs_human_judgment` | `major` 以上で、`support` が 1 件以上または `origin_runtimes` が 2 者以上 | はい | — | +| 5 | `unrefuted` | `major` 以上（上のいずれにも当たらない） | はい | `unrefuted_reason`（`no_critique` / `not_supported`） | +| 6 | `insufficient_evidence` | 上のいずれにも当たらない（`minor` 以下） | いいえ | — | **実行で再現した指摘を先に採るのは、順序そのもので「実行の結果を担当の支持より先に 見る」を表すためである。** 順 3 を先に置くと、機械が再現した事実を担当の再評価が覆す。 -**収束の判定が数えるのは、`verified_blocking` と `needs_human_judgment` の 2 つだけ** -である。棄却した指摘を数えると、そのぶんラウンドが増える。 +**収束の判定が数えるのは、`verified_blocking` と `needs_human_judgment` と `unrefuted` の +3 つ**である。数えないのは、誤りだと示された棄却と、承認を妨げない軽微な指摘だけである。 +棄却した指摘を数えると、そのぶんラウンドが増える。 + +**未反証（`unrefuted`）は、誰も誤りを示しておらず、独立に確かめた担当もいない重大な指摘で +ある。** 反証の「立証できない」「範囲外」は誤りだという主張ではないため、その指摘を数から +落とさない。数えないと、未解決の重大な指摘を残したまま承認で収束する。なぜ確かめられていない +かは理由（`unrefuted_reason`）として残す。反証を返した担当が 0 者なら `no_critique`、反証は +あるが支持も否定も無ければ `not_supported` である。修正の担当の扱いは `needs_human_judgment` +と同じで、直す・却下の理由を返す・範囲外として起票する、のいずれかを決める。根拠の 2 項目 +（`evidence` / `falsification`）の有無は区分を変えない。別の担当が支持した、または 2 者が +独立に出した時点で、確かめる目的は果たされている。 **`needs_human_judgment` を人へのエスカレーションにしない。** 決めるのは修正の担当で、 その判断は却下の記録（`rejected_findings`）へ残る。 @@ -221,7 +236,7 @@ $SCRIPTS/measure.py <状態ファイルのパス> [--output <パス>] | --- | --- | | `single` | 1 者だけの結果。**担当ごとに 1 通り出す**（誰を選ぶかで結果が変わるため） | | `majority` | `origin_runtimes` が 2 者以上の指摘 | -| `proposed` | 区分が `verified_blocking` または `needs_human_judgment` の指摘 | +| `proposed` | 区分が `verified_blocking` / `needs_human_judgment` / `unrefuted` の指摘 | | `oracle` | いずれかの担当が出した指摘のうち、**修正された**もの | **母集合は代表だけである。** 4 つとも `merged_into` を持つ要素を数えない。統合された側を @@ -232,8 +247,8 @@ $SCRIPTS/measure.py <状態ファイルのパス> [--output <パス>] 過去の記録の `single` が全件 0 になる。補った値は 1 者であるため `majority` には入らない。 **`proposed` が読むのは印（`evidence_rounds`）のあるラウンドだけである。** 印の無いラウンドを -母集合へ入れると、区分の付かない指摘が `insufficient_evidence` として落ち、再現率が実際より -低く出る。**分母も印のあるラウンドに限る**（分子だけを絞ると、印の混ざった記録で過小に出る）。 +母集合へ入れると、区分の付かない指摘が数える 3 区分のいずれにも当たらずに落ち、再現率が +実際より低く出る。**分母も印のあるラウンドに限る**（分子だけを絞ると、印の混ざった記録で過小に出る）。 分母が他の 3 つと違うことは `oracle_scope` と `oracle_base` の 2 つのキーで常に出す。 ### `oracle` の結び方 diff --git a/plugins/ndf/skills/cross-review/scripts/critique.sh b/plugins/ndf/skills/cross-review/scripts/critique.sh index a46d62a2..4078b30e 100755 --- a/plugins/ndf/skills/cross-review/scripts/critique.sh +++ b/plugins/ndf/skills/cross-review/scripts/critique.sh @@ -96,6 +96,10 @@ cat > "$PROMPT" < None: def test_the_skill_points_at_the_contract_document() -> None: assert "docs/04-contracts.md" in SKILL.read_text(encoding="utf-8") + + +# ---- 区分の 6 つ目「未反証」（#732） ---- +# +# 誤りを示されていない `major` を `unrefuted` として数える。規約 3 文書・反証のプロンプト・ +# 確定仕様が同じ語で書いていることを固定する（AC18〜AC20）。 + +CRITIQUE_SH = HERE / "scripts/critique.sh" +EVIDENCE = DOCS / "06-evidence.md" +POOL = DOCS / "05-pool-and-convergence.md" +SPEC = HERE.parents[3] / "docs/specifications/cross-review-evidence-based.md" + + +def test_the_critique_prompt_says_insufficient_evidence_does_not_drop_the_finding() -> None: + """「立証できない」を返しても指摘は数から落ちないことを、プロンプトが担当へ言う（AC19）。""" + assert "数から落ち" in CRITIQUE_SH.read_text(encoding="utf-8") + + +@pytest.mark.parametrize("doc", (EVIDENCE, CONTRACTS, POOL), ids=lambda p: p.name) +def test_the_review_docs_name_the_unrefuted_classification(doc: pathlib.Path) -> None: + assert "unrefuted" in doc.read_text(encoding="utf-8"), doc.name + + +def test_the_evidence_doc_says_the_mark_is_removed_when_critiques_are_incomplete() -> None: + assert "印を外す" in EVIDENCE.read_text(encoding="utf-8") + + +def test_the_specification_holds_the_same_six_classifications() -> None: + """区分の表と行き先の表の両方が `unrefuted` を持つ（AC20）。""" + assert SPEC.read_text(encoding="utf-8").count("unrefuted") >= 2 From 37dec905a79146bc866135240b6fbdd3a5aed88d Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 19:56:01 +0000 Subject: [PATCH 03/21] =?UTF-8?q?Test:=20cross-review=20=E3=81=AE=E7=8F=BE?= =?UTF-8?q?=E7=8A=B6=E5=9B=BA=E5=AE=9A=E3=83=86=E3=82=B9=E3=83=88=E3=82=92?= =?UTF-8?q?=E8=BF=BD=E5=8A=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit measure の境界値、反証再取得ループ、PR prepare の公開入口を固定する。 Item-Id: R1-001 Round: 1 Impl-Runtime: codex Impl-Model: default --- .../cross-review/tests/test_critiques.py | 45 +++++++++++ .../skills/cross-review/tests/test_measure.py | 32 ++++++++ .../tests/test_rotate_pr_queue.py | 74 +++++++++++++++++++ 3 files changed, 151 insertions(+) diff --git a/plugins/ndf/skills/cross-review/tests/test_critiques.py b/plugins/ndf/skills/cross-review/tests/test_critiques.py index 56e0d006..6303258e 100644 --- a/plugins/ndf/skills/cross-review/tests/test_critiques.py +++ b/plugins/ndf/skills/cross-review/tests/test_critiques.py @@ -8,6 +8,7 @@ import argparse import json import pathlib +import shutil import pytest @@ -573,6 +574,50 @@ def test_a_stale_pidfile_is_not_read_as_a_launch(tmp_dir, tmp_path): assert elapsed < ROUND_TIME_LIMIT, f"{elapsed:.1f} 秒かかった（監視が待っている）" +def test_a_retry_launches_only_the_agents_requested_by_collect(tmp_path): + """現状固定: 終了コード 7 の再取得では不足した担当だけを起動し直す。""" + script_dir = tmp_path / "scripts" + script_dir.mkdir() + shutil.copy2(SCRIPTS / "critique-round.sh", script_dir / "critique-round.sh") + calls = tmp_path / "critique-calls.txt" + collects = tmp_path / "collect-count.txt" + + (script_dir / "_tmpdir.sh").write_text( + 'tmpdir() { printf "%s\\n" "$CROSS_REVIEW_TMP_DIR"; }\n', encoding="utf-8") + (script_dir / "critique.sh").write_text( + "#!/usr/bin/env bash\n" + 'printf "%s\\n" "$1" >> "$CRITIQUE_CALLS"\n' + 'touch "$CROSS_REVIEW_TMP_DIR/$1-critique-pr$2.pid"\n', encoding="utf-8") + (script_dir / "monitor.py").write_text( + "#!/usr/bin/env bash\nexit 0\n", encoding="utf-8") + (script_dir / "state.py").write_text( + "#!/usr/bin/env bash\n" + 'count=0; [ ! -f "$COLLECT_COUNT" ] || count=$(cat "$COLLECT_COUNT")\n' + 'count=$((count + 1)); printf "%s\\n" "$count" > "$COLLECT_COUNT"\n' + 'if [ "$count" -eq 1 ]; then\n' + " printf \"CRITIQUE_RETRY_AGENTS='kiro'\\n\"\n" + " exit 7\n" + "fi\n" + "exit 0\n", encoding="utf-8") + for name in ("critique.sh", "monitor.py", "state.py"): + (script_dir / name).chmod(0o755) + + env = dict( + os.environ, + CROSS_REVIEW_TMP_DIR=str(tmp_path), + CRITIQUE_CALLS=str(calls), + COLLECT_COUNT=str(collects), + ) + result = subprocess.run( + ["bash", str(script_dir / "critique-round.sh"), str(PR), "1", "agy", "kiro"], + capture_output=True, text=True, env=env, check=False, + ) + + assert result.returncode == 0, result.stderr + assert calls.read_text(encoding="utf-8").splitlines() == ["agy", "kiro", "kiro"] + assert collects.read_text(encoding="utf-8").strip() == "2" + + def test_the_stale_pidfile_is_removed_even_without_targets(tmp_dir, tmp_path): """捨てるのは、対象が無くて起動しない経路より前である。""" work = tmp_path / "work" diff --git a/plugins/ndf/skills/cross-review/tests/test_measure.py b/plugins/ndf/skills/cross-review/tests/test_measure.py index 28967b5b..b20aed8d 100644 --- a/plugins/ndf/skills/cross-review/tests/test_measure.py +++ b/plugins/ndf/skills/cross-review/tests/test_measure.py @@ -142,6 +142,17 @@ def test_empty_state_does_not_crash(measure_mod): assert "methods" in result +@pytest.mark.parametrize("state", [None, [], "not-a-state"]) +def test_non_mapping_state_falls_back_to_an_empty_state(measure_mod, state): + """現状固定: 辞書以外の入力も空の状態として指標の全キーを返す。""" + result = measure_mod.measure(state) + + assert set(result) == {"pr", "prs", "rounds", "methods", "cost", "convergence"} + assert result["pr"] is None + assert result["prs"] == [] + assert result["rounds"] == 0 + + def test_wall_clock_is_null_while_the_run_has_not_ended(measure_mod): """終わっていない実行では実時間を出さない。**0 で埋めない。**""" st = _state(rounds=[_round(1)]) @@ -573,6 +584,27 @@ def test_proposed_reports_all_rounds_when_every_round_is_marked(measure_mod): "oracle_scope": "all_rounds", "oracle_base": 2} +def test_proposed_normalizes_duplicate_and_invalid_evidence_rounds(measure_mod): + """現状固定: 有効な番号は型をそろえて一つの印にし、不正値は無視する。""" + st = _state( + evidence_rounds=["1", 1, "invalid", None], + rounds=[ + _round(1, fix=_fix(_position("T1", "a.py", 10))), + _round(2, fix=_fix(_position("T2", "b.py", 20))), + ], + review_findings=[ + _finding("codex-r1-0", 1, "a.py", 10, + classification="verified_blocking"), + _finding("codex-r2-0", 2, "b.py", 20, + classification="verified_blocking"), + ], + ) + + assert measure_mod.measure(st)["methods"]["proposed"] == { + "found": 1, "matched": 1, "of_oracle": 1.0, + "oracle_scope": "evidence_rounds", "oracle_base": 1} + + def test_proposed_takes_only_the_three_counted_classifications(measure_mod): """採るのは `verified_blocking` / `needs_human_judgment` / `unrefuted` の 3 つである（#732）。 diff --git a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py index 238c3742..3430718c 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py +++ b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py @@ -297,3 +297,77 @@ def test_an_unknown_flag_is_rejected() -> None: assert out.returncode == 2 assert "unknown arg: --unknown-flag" in out.stderr + + +def test_prepare_connects_state_pr_metadata_and_git_summary(tmp_path) -> None: + """現状固定: 公開 CLI が prepare.json と eval 用の代入を組み立てる。""" + state_pr = 41 + current_pr = 43 + tmp_dir = tmp_path / "tmp" + worktree = tmp_path / "worktree" + bin_dir = tmp_path / "bin" + tmp_dir.mkdir() + worktree.mkdir() + bin_dir.mkdir() + (tmp_dir / f"cross-review-pr{state_pr}-state.json").write_text(json.dumps({ + "worktree_path": str(worktree), + "current_pr": current_pr, + "repo": "o/r", + "viewer_login": "tester", + "rounds": [ + {"round": 1, "pr": 40}, + {"round": 2, "pr": current_pr}, + {"round": 3, "pr": current_pr}, + ], + }), encoding="utf-8") + (bin_dir / "gh").write_text( + "#!/usr/bin/env bash\n" + "printf '%s\\n' '{\"number\":43,\"url\":\"https://github.com/o/r/pull/43\"," + "\"title\":\"Current title\",\"body\":\"Current body\"," + "\"headRefName\":\"feature/prepare\",\"baseRefName\":\"develop\"," + "\"isDraft\":true}'\n", encoding="utf-8") + (bin_dir / "git").write_text( + "#!/usr/bin/env bash\n" + "case \"$1\" in\n" + " fetch|rev-parse) exit 0 ;;\n" + " log) printf 'abc123 First commit\\ndef456 Second commit\\n' ;;\n" + " diff) printf ' a.py | 2 ++\\n 1 file changed, 2 insertions(+)\\n' ;;\n" + " *) exit 3 ;;\n" + "esac\n", encoding="utf-8") + for command in ("gh", "git"): + (bin_dir / command).chmod(0o755) + + env = { + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "CROSS_REVIEW_TMP_DIR": str(tmp_dir), + } + out = subprocess.run( + ["bash", str(ROTATE), "prepare", str(state_pr)], + capture_output=True, text=True, env=env, check=False, + ) + + assert out.returncode == 0, out.stderr + evaluated = subprocess.run( + ["bash", "-c", + 'eval "$1"; printf "%s\\n" "$PREPARE_JSON" "$OLD_PR" "$HEAD_BRANCH" ' + '"$BASE_BRANCH" "$IS_DRAFT"', "bash", out.stdout], + capture_output=True, text=True, check=True, + ).stdout.splitlines() + prepare_path = tmp_dir / f"rotate-pr{state_pr}-prepare.json" + assert evaluated == [ + str(prepare_path), str(current_pr), "feature/prepare", "develop", "true"] + assert json.loads(prepare_path.read_text(encoding="utf-8")) == { + "state_pr": state_pr, + "old_pr": current_pr, + "old_pr_url": "https://github.com/o/r/pull/43", + "worktree_path": str(worktree), + "head_branch": "feature/prepare", + "base_branch": "develop", + "is_draft": True, + "round_in_pr": 2, + "old_title": "Current title", + "old_body": "Current body", + "git_log": "abc123 First commit\ndef456 Second commit", + "git_diff_stat": " a.py | 2 ++\n 1 file changed, 2 insertions(+)", + } From e772af39cd852a95179eb47b849f0b3e3ab8055e Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 19:56:44 +0000 Subject: [PATCH 04/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 84 ++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 issues/refactoring-plan-rf790.md diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md new file mode 100644 index 00000000..73f013c4 --- /dev/null +++ b/issues/refactoring-plan-rf790.md @@ -0,0 +1,84 @@ +# 改修計画 — devbasex/ai-plugins #790 + +`/ndf:cross-refactoring` が提案し、適用した改善項目の記録である。 +理由と手順は提案の時点でしか残らないため、公開の直前に書き出している。 + +- 対象範囲: plugins/ndf/skills/cross-review/scripts, plugins/ndf/skills/cross-review/tests +- 着手前のテスト: uv run --with pytest pytest scripts/tests plugins/ndf -q + +## ラウンド 1（実装 codex / レビュー agy / kiro） + +### R1-001 — `plugins/ndf/skills/cross-review/scripts/measure.py#measure` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| boundary | unit | — | codex / agy | 検証中 | 1 | + +**なぜ**: measure 関数に None や辞書以外の型が渡されたとき、空辞書にフォールバックして例外なく指標辞書（pr, prs, rounds, methods, cost, convergence）を返す境界値の振る舞いが固定されていない + +**手順**: 1. evidence_rounds に文字列の有効ラウンド番号、重複する整数、不正値を含み、印付き・印なし両方の findings を持つ state を作る +2. measure を実行する +3. 有効番号が一つの印として扱われ、不正値が無視され、proposed の found・oracle_scope・oracle_base が現在の値になることを比較する + +### R1-002 — `plugins/ndf/skills/cross-review/scripts/critique-round.sh#critique-round` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| branch | integration | — | kiro | 検証中 | 1 | + +**なぜ**: run_round を通す既存テストは 2 本とも指摘 0 件で、collect-critiques が 1 回目に 0 を返して 1 回で抜ける経路しか固定していない。exit 7 と CRITIQUE_RETRY_AGENTS を受けて 2 回目の起動を回すループ本体（for _attempt in 1 2）はどのテストも通っていない。 + +**手順**: 1. critique-round.sh を temp ディレクトリへ複製し、隣に stub の critique.sh・monitor.py・state.py・_tmpdir.sh を置く（test_wait_review.py と同じ、兄弟スクリプトを差し替える方式） +2. stub の state.py collect-critiques を、呼び出し回数を記録したうえで 1 回目は stdout に CRITIQUE_RETRY_AGENTS='kiro' を出して終了コード 7、2 回目は終了コード 0 を返すようにする +3. stub の critique.sh は渡された担当名を追記で記録し、対応する pid ファイルを作る +4. critique-round.sh に PR・ROUND・agy kiro を渡して実行し、終了コード 0 を確かめる +5. critique.sh の記録が 2 回目は kiro だけへ絞られている（agy は再起動されない）ことと、collect-critiques が 2 回呼ばれたことを比較する + +### R1-003 — `plugins/ndf/skills/cross-review/scripts/critique-round.sh#critique-round` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| error | integration | — | kiro | 未着手 | 0 | + +**なぜ**: collect-critiques が 7 以外を返したときに exit "$COLLECT_RC" でその終了コードを素通しする分岐が固定されていない。既存テストは 0 で抜ける経路だけを見ており、失敗の終了コードがラウンドの外へ伝わるかを誰も確かめていない。 + +**手順**: 1. critique-round.sh を temp ディレクトリへ複製し、隣に stub の critique.sh・monitor.py・state.py・_tmpdir.sh を置く +2. stub の state.py collect-critiques を、呼び出し回数を記録して終了コード 5 で終わるようにする +3. critique-round.sh に PR・ROUND・担当を渡して実行する +4. 終了コードが 5（collect-critiques が返した値）と一致することを確かめる +5. collect-critiques が 1 回だけ呼ばれ、2 回目の起動へ進んでいないことを記録から確かめる + +### R1-004 — `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#cmd_prepare` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| normal | integration | — | codex | 検証中 | 1 | + +**なぜ**: 公開入口 prepare は state、GitHub の PR メタデータ、git log/diff をつないで prepare.json と eval 用の出力を作るが、既存テストは生成済み prepare.json を与えるだけで、この経路を実行していない + +**手順**: 1. 一時 worktree と state を作り、gh pr view・git fetch/log/diff を現状の出力を返す代替コマンドへ差し替える +2. rotate-pr.sh prepare を公開 CLI から実行する +3. 終了コード 0、stdout の shell 代入を評価して得る値、prepare.json の PR・branch・draft・round・git 要約を比較する + +### R1-005 — `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#execute_squash` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| error | integration | — | codex | 未着手 | 0 | + +**なぜ**: 新 PR 作成失敗時に旧 PR を reopen する経路は light モードだけ固定され、同じ ERR trap を使う squash モードでは未固定である + +**手順**: 1. squash の close までは成功し、gh pr create だけが失敗する代替 git・gh と一時 state を用意する +2. rotate-pr.sh execute --mode squash を公開 CLI から実行する +3. 非ゼロ終了、新 PR が存在しないこと、旧 PR の最終状態が open に戻ること、成功用の NEW_PR が出ないことを比較する + +## 見送った項目 + +| ラウンド | 対象 | 兆候・経路 | 理由 | +| --- | --- | --- | --- | +| 1 | `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#execute_squash` | normal | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_check_oscillation` | branch | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_check_oscillation` | normal | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_collect_critiques` | boundary | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_collect_critiques` | error | 1 ラウンドの採用上限 5 件を超えた | +| 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_verify_findings` | error | 1 ラウンドの採用上限 5 件を超えた | From 47384be070f17f42dc7515eef0243a020c80aa44 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:06:09 +0000 Subject: [PATCH 05/21] =?UTF-8?q?Test:=20=E7=8F=BE=E7=8A=B6=E5=9B=BA?= =?UTF-8?q?=E5=AE=9A=E3=83=86=E3=82=B9=E3=83=88=E8=BF=BD=E5=8A=A0=20?= =?UTF-8?q?=E2=80=94=20critique-round=20/=20rotate-pr=20(squash)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - critique-round.sh: collect-critiques 失敗時の終了コード素通し分岐を固定 (R1-003) - rotate-pr.sh: squash モードでの新 PR 作成失敗時の旧 PR 復旧経路を固定 (R1-005) Item-Id: R1-003 Round: 1 Impl-Runtime: agy Impl-Model: default --- .../cross-review/tests/test_critiques.py | 40 +++++++++++++++++++ .../tests/test_rotate_pr_queue.py | 34 +++++++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/plugins/ndf/skills/cross-review/tests/test_critiques.py b/plugins/ndf/skills/cross-review/tests/test_critiques.py index 6303258e..7c262dd0 100644 --- a/plugins/ndf/skills/cross-review/tests/test_critiques.py +++ b/plugins/ndf/skills/cross-review/tests/test_critiques.py @@ -618,6 +618,46 @@ def test_a_retry_launches_only_the_agents_requested_by_collect(tmp_path): assert collects.read_text(encoding="utf-8").strip() == "2" +def test_collect_failure_propagates_exit_code_without_retry(tmp_path): + """現状固定: collect-critiques が 7 以外（例: 5）を返したとき、終了コードを素通しして直ちに終了する。""" + script_dir = tmp_path / "scripts" + script_dir.mkdir() + shutil.copy2(SCRIPTS / "critique-round.sh", script_dir / "critique-round.sh") + calls = tmp_path / "critique-calls.txt" + collects = tmp_path / "collect-count.txt" + + (script_dir / "_tmpdir.sh").write_text( + 'tmpdir() { printf "%s\\n" "$CROSS_REVIEW_TMP_DIR"; }\n', encoding="utf-8") + (script_dir / "critique.sh").write_text( + "#!/usr/bin/env bash\n" + 'printf "%s\\n" "$1" >> "$CRITIQUE_CALLS"\n' + 'touch "$CROSS_REVIEW_TMP_DIR/$1-critique-pr$2.pid"\n', encoding="utf-8") + (script_dir / "monitor.py").write_text( + "#!/usr/bin/env bash\nexit 0\n", encoding="utf-8") + (script_dir / "state.py").write_text( + "#!/usr/bin/env bash\n" + 'count=0; [ ! -f "$COLLECT_COUNT" ] || count=$(cat "$COLLECT_COUNT")\n' + 'count=$((count + 1)); printf "%s\\n" "$count" > "$COLLECT_COUNT"\n' + "exit 5\n", encoding="utf-8") + for name in ("critique.sh", "monitor.py", "state.py"): + (script_dir / name).chmod(0o755) + + env = dict( + os.environ, + CROSS_REVIEW_TMP_DIR=str(tmp_path), + CRITIQUE_CALLS=str(calls), + COLLECT_COUNT=str(collects), + ) + result = subprocess.run( + ["bash", str(script_dir / "critique-round.sh"), str(PR), "1", "agy", "kiro"], + capture_output=True, text=True, env=env, check=False, + ) + + assert result.returncode == 5 + assert collects.read_text(encoding="utf-8").strip() == "1" + assert calls.read_text(encoding="utf-8").splitlines() == ["agy", "kiro"] + + def test_the_stale_pidfile_is_removed_even_without_targets(tmp_dir, tmp_path): """捨てるのは、対象が無くて起動しない経路より前である。""" work = tmp_path / "work" diff --git a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py index 3430718c..206e6225 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py +++ b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py @@ -221,7 +221,7 @@ def __init__(self, tmp_path: pathlib.Path) -> None: encoding="utf-8", ) - def run(self, create_ok: bool) -> subprocess.CompletedProcess[str]: + def run(self, create_ok: bool, mode: str = "light") -> subprocess.CompletedProcess[str]: env = { **os.environ, "PATH": f"{self.bin}{os.pathsep}{os.environ['PATH']}", @@ -231,7 +231,7 @@ def run(self, create_ok: bool) -> subprocess.CompletedProcess[str]: "GH_CREATE": "ok" if create_ok else "fail", } return subprocess.run( - ["bash", str(ROTATE), "execute", str(_STATE_PR), "--mode", "light"], + ["bash", str(ROTATE), "execute", str(_STATE_PR), "--mode", mode], capture_output=True, text=True, timeout=180, env=env, ) @@ -273,6 +273,36 @@ def test_a_create_success_closes_the_old_pr_and_opens_the_new_pr(rotation: _Rota assert not any(c.startswith("pr reopen") for c in rotation.gh_calls()) +def test_a_create_failure_in_squash_mode_reopens_the_old_pr_and_emits_no_new_pr( + rotation: _Rotation, +) -> None: + """現状固定: squash モードでも新 PR 作成が失敗すると非ゼロ終了で旧 PR が open へ戻り、NEW_PR は出ない。""" + out = rotation.run(create_ok=False, mode="squash") + + assert out.returncode != 0, out.stderr + states = rotation.pr_states() + assert states[str(_OLD_PR)] == "open" # reopen で戻る + assert str(_NEW_PR) not in states # 新 PR は作られていない + assert "NEW_PR=" not in out.stdout # 成功結果を出力していない + joined = rotation.gh_calls() + assert any(c.startswith(f"pr close {_OLD_PR}") for c in joined) + assert any(c.startswith(f"pr reopen {_OLD_PR}") for c in joined) + + +def test_a_create_success_in_squash_mode_closes_the_old_pr_and_opens_the_new_pr( + rotation: _Rotation, +) -> None: + """比較用: squash モードで作成が成功すると旧 PR は closed、新 PR は open、NEW_PR が作成結果を指す。""" + out = rotation.run(create_ok=True, mode="squash") + + assert out.returncode == 0, out.stderr + states = rotation.pr_states() + assert states[str(_OLD_PR)] == "closed" + assert states[str(_NEW_PR)] == "open" + assert f"NEW_PR={_NEW_PR}" in out.stdout + assert not any(c.startswith("pr reopen") for c in rotation.gh_calls()) + + # ---- execute の引数検証（R2-004、現状固定） ---- # # `--mode` の値検証と未知フラグの検出は gh/git を一切呼ばない純粋な引数解析であり、 From c76ae1913eb15549c87240b76ad352cbc7f92ce6 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:06:57 +0000 Subject: [PATCH 06/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index 73f013c4..e72b66b3 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -12,7 +12,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| boundary | unit | — | codex / agy | 検証中 | 1 | +| boundary | unit | — | codex / agy | 採用 | 1 | **なぜ**: measure 関数に None や辞書以外の型が渡されたとき、空辞書にフォールバックして例外なく指標辞書（pr, prs, rounds, methods, cost, convergence）を返す境界値の振る舞いが固定されていない @@ -24,7 +24,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| branch | integration | — | kiro | 検証中 | 1 | +| branch | integration | — | kiro | 採用 | 1 | **なぜ**: run_round を通す既存テストは 2 本とも指摘 0 件で、collect-critiques が 1 回目に 0 を返して 1 回で抜ける経路しか固定していない。exit 7 と CRITIQUE_RETRY_AGENTS を受けて 2 回目の起動を回すループ本体（for _attempt in 1 2）はどのテストも通っていない。 @@ -38,7 +38,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| error | integration | — | kiro | 未着手 | 0 | +| error | integration | — | kiro | 検証中 | 1 | **なぜ**: collect-critiques が 7 以外を返したときに exit "$COLLECT_RC" でその終了コードを素通しする分岐が固定されていない。既存テストは 0 で抜ける経路だけを見ており、失敗の終了コードがラウンドの外へ伝わるかを誰も確かめていない。 @@ -52,7 +52,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| normal | integration | — | codex | 検証中 | 1 | +| normal | integration | — | codex | 採用 | 1 | **なぜ**: 公開入口 prepare は state、GitHub の PR メタデータ、git log/diff をつないで prepare.json と eval 用の出力を作るが、既存テストは生成済み prepare.json を与えるだけで、この経路を実行していない @@ -64,7 +64,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| error | integration | — | codex | 未着手 | 0 | +| error | integration | — | codex | 検証中 | 1 | **なぜ**: 新 PR 作成失敗時に旧 PR を reopen する経路は light モードだけ固定され、同じ ERR trap を使う squash モードでは未固定である From b02b30dc8a7a1d7f92f7e0d6b5c2d8924e56be8e Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:25:05 +0000 Subject: [PATCH 07/21] =?UTF-8?q?Test:=20=E7=8F=BE=E7=8A=B6=E5=9B=BA?= =?UTF-8?q?=E5=AE=9A=E3=83=86=E3=82=B9=E3=83=88=E3=82=92=E8=BF=BD=E5=8A=A0?= =?UTF-8?q?=20=E2=80=94=20cross-review=20=E3=81=AE=E6=9C=AA=E5=9B=BA?= =?UTF-8?q?=E5=AE=9A=E5=88=86=E5=B2=90=203=20=E4=BB=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R2-001: measure の oracle 算出で resolved_thread_positions に非辞書要素 （文字列・null）が混じっても unmatched に数え、例外を出さず測定を続ける経路。 R2-002: rotate-pr.sh execute の --mode 値なし境界（${2:?...} で落ちる）と、 引数 0 個の entrypoint usage（exit 2）。いずれも gh/git を呼ぶ前で止まる。 R2-005: state.py cmd_set_current_pr で pr_history に過去 PR と現在 PR が並ぶとき、 過去 PR を変えず直前の現在 PR だけ閉じて新 PR を末尾へ足す分岐。 対象コードは変更していない。現状固定テストのみを追加。 Item-Id: R2-001 Round: 2 Impl-Runtime: kiro Impl-Model: default --- .../skills/cross-review/tests/test_measure.py | 44 +++++++++++++++++ .../tests/test_rotate_pr_queue.py | 30 ++++++++++++ .../tests/test_state_rotation_head_branch.py | 49 +++++++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/plugins/ndf/skills/cross-review/tests/test_measure.py b/plugins/ndf/skills/cross-review/tests/test_measure.py index b20aed8d..a6844a56 100644 --- a/plugins/ndf/skills/cross-review/tests/test_measure.py +++ b/plugins/ndf/skills/cross-review/tests/test_measure.py @@ -324,6 +324,50 @@ def test_oracle_counts_a_thread_without_a_position_as_unmatched(measure_mod): "found": 0, "unmatched": 1, "ambiguous": 0} +def test_oracle_counts_a_non_dict_position_as_unmatched(measure_mod): + """現状固定（R2-001）。位置の一覧に非辞書要素（文字列・null）が混じっても、 + + 落とさずに `unmatched` へ数え、例外を出さずに測定結果を返す。 + `_resolved_position` は辞書でない要素へ `(None, None)` を返し、 + `_add_oracle_match` がそれを `unmatched + 1` として扱う経路を固定する。 + """ + fix = { + "commit": "abc1234", "fixed": 1, "resolved_threads": 1, + "resolved_thread_ids": ["T1"], + "resolved_thread_positions": ["not-a-dict"], + } + st = _state( + rounds=[_round(1, fix=fix)], + review_findings=[_finding("codex-r1-0", 1, "a.py", 10)], + ) + + result = measure_mod.measure(st) + + assert result["methods"]["oracle"] == { + "found": 0, "unmatched": 1, "ambiguous": 0} + + +def test_oracle_counts_a_null_position_as_unmatched(measure_mod): + """現状固定（R2-001）。位置の一覧に `null` が混じっても `unmatched` に数える。 + + 非辞書要素の代表として `None`（JSON の null）でも同じ経路を通ることを固定する。 + """ + fix = { + "commit": "abc1234", "fixed": 1, "resolved_threads": 1, + "resolved_thread_ids": ["T1"], + "resolved_thread_positions": [None], + } + st = _state( + rounds=[_round(1, fix=fix)], + review_findings=[_finding("codex-r1-0", 1, "a.py", 10)], + ) + + result = measure_mod.measure(st) + + assert result["methods"]["oracle"] == { + "found": 0, "unmatched": 1, "ambiguous": 0} + + def test_oracle_does_not_count_a_finding_without_an_id(measure_mod): """`finding_id` を持たない指摘は結ばない（#558 レビュー）。 diff --git a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py index 206e6225..9202705a 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py +++ b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py @@ -329,6 +329,36 @@ def test_an_unknown_flag_is_rejected() -> None: assert "unknown arg: --unknown-flag" in out.stderr +def test_mode_without_a_value_is_rejected() -> None: + """現状固定（R2-002）。`--mode` の直後に値が無いと `${2:?...}` で落ちる。 + + state.json も newtext.json も用意せず、gh/git を呼ぶ前の引数解析だけで止まる。 + `${2:?...}` は set -u と相まって execute のループに入る前に落ちるため、 + load_state（state.json 読み込み）にも到達しない。 + """ + out = subprocess.run( + ["bash", str(ROTATE), "execute", "123", "--mode"], + capture_output=True, text=True, timeout=60, + ) + + assert out.returncode != 0 + assert "--mode requires light|squash" in out.stderr + + +def test_no_arguments_prints_usage() -> None: + """現状固定（R2-002）。引数が 0 個のとき entrypoint は usage を出して exit 2。 + + state.json を用意せず、引数解析だけで止まることを確かめる。 + """ + out = subprocess.run( + ["bash", str(ROTATE)], + capture_output=True, text=True, timeout=60, + ) + + assert out.returncode == 2 + assert "Usage:" in out.stderr + + def test_prepare_connects_state_pr_metadata_and_git_summary(tmp_path) -> None: """現状固定: 公開 CLI が prepare.json と eval 用の代入を組み立てる。""" state_pr = 41 diff --git a/plugins/ndf/skills/cross-review/tests/test_state_rotation_head_branch.py b/plugins/ndf/skills/cross-review/tests/test_state_rotation_head_branch.py index b5310c75..f14bd341 100644 --- a/plugins/ndf/skills/cross-review/tests/test_state_rotation_head_branch.py +++ b/plugins/ndf/skills/cross-review/tests/test_state_rotation_head_branch.py @@ -105,6 +105,55 @@ def test_an_empty_lookup_keeps_the_previous_branch(tmp_dir, state_mod, monkeypat assert _state(tmp_dir)["head_branch"] == OLD_BRANCH +def test_only_the_current_pr_entry_is_closed_when_history_has_past_prs( + tmp_dir, state_mod, monkeypatch +): + """現状固定（R2-005）。過去に閉じた PR を含む履歴で、直前の現在 PR だけを閉じる。 + + `pr_history` に閉じた過去 PR（`closed_at` 設定済み）と現在の PR（`closed_at` + が None）を順に持たせて `cmd_set_current_pr` を実行する。過去 PR は変わらず、 + 直前の現在 PR に `closed_at` と `rounds` が入り、新 PR エントリが + `closed_at: None` / `rounds: 0` で末尾へ足される分岐を固定する。 + """ + past_pr = 4200 + state = { + "current_pr": PR, + "repo": "o/r", + "head_branch": OLD_BRANCH, + "rounds": [ + {"round": 1, "pr": past_pr}, + {"round": 2, "pr": PR}, + {"round": 3, "pr": PR}, + ], + "pr_history": [ + {"pr": past_pr, "opened_at": "t0", "closed_at": "t1", "rounds": 1}, + {"pr": PR, "opened_at": "t2", "closed_at": None, "rounds": 0}, + ], + "final": None, + } + (tmp_dir / f"cross-review-pr{PR}-state.json").write_text(json.dumps(state)) + # 引数で枝名を渡し、GitHub を呼ばない経路で確かめる。 + monkeypatch.setattr( + state_mod, "_sh", lambda cmd, check=True: pytest.fail("GitHub を呼んでいる") + ) + + state_mod.cmd_set_current_pr(_args(head_branch=NEW_BRANCH)) + + history = _state(tmp_dir)["pr_history"] + # 過去 PR は変わらない。 + assert history[0] == { + "pr": past_pr, "opened_at": "t0", "closed_at": "t1", "rounds": 1} + # 直前の現在 PR に closed_at と rounds（その PR のラウンド数 2）が入る。 + assert history[1]["pr"] == PR + assert history[1]["closed_at"] is not None + assert history[1]["rounds"] == 2 + # 新 PR エントリが末尾に closed_at: None / rounds: 0 で足される。 + assert history[2]["pr"] == NEW_PR + assert history[2]["closed_at"] is None + assert history[2]["rounds"] == 0 + assert len(history) == 3 + + def test_the_skeleton_passes_the_new_branch(state_mod) -> None: """手順書と参照の骨組みが `--head-branch` を渡していることを固定する。""" here = pathlib.Path(__file__).resolve().parent.parent From 678f61f09d678108288aa58320a2830ba8d7ef38 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:25:38 +0000 Subject: [PATCH 08/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 74 +++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index e72b66b3..45b29e8a 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -38,7 +38,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| error | integration | — | kiro | 検証中 | 1 | +| error | integration | — | kiro | 採用 | 1 | **なぜ**: collect-critiques が 7 以外を返したときに exit "$COLLECT_RC" でその終了コードを素通しする分岐が固定されていない。既存テストは 0 で抜ける経路だけを見ており、失敗の終了コードがラウンドの外へ伝わるかを誰も確かめていない。 @@ -64,7 +64,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| error | integration | — | codex | 検証中 | 1 | +| error | integration | — | codex | 採用 | 1 | **なぜ**: 新 PR 作成失敗時に旧 PR を reopen する経路は light モードだけ固定され、同じ ERR trap を使う squash モードでは未固定である @@ -72,6 +72,75 @@ 2. rotate-pr.sh execute --mode squash を公開 CLI から実行する 3. 非ゼロ終了、新 PR が存在しないこと、旧 PR の最終状態が open に戻ること、成功用の NEW_PR が出ないことを比較する +## ラウンド 2（実装 agy / レビュー codex / kiro） + +### R2-001 — `plugins/ndf/skills/cross-review/scripts/measure.py#measure` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| branch | unit | — | codex / agy | 検証中 | 1 | + +**なぜ**: measure の oracle 算出において、resolved_thread_positions の要素が辞書形式である経路は固定されているが、リスト内に非辞書要素（文字列や null など）が混在した場合にそれを unmatched として数えて測定を継続する分岐が未固定である + +**手順**: 1. resolved_thread_positions に文字列や null などの非辞書要素を含む状態ファイルを用意する +2. measure を実行する +3. oracle の found が 0、unmatched が 1、ambiguous が 0 と計算され、例外を出さずに全体の測定結果が返ることを確かめる + +### R2-002 — `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#cmd_execute` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| boundary | integration | — | agy / kiro | 検証中 | 1 | + +**なぜ**: cmd_execute の引数解析は不正な --mode 値と未知フラグ（exit 2）は固定済みだが、--mode の直後に値が無いとき ${2:?--mode requires light|squash} で落ちる境界と、そもそも引数が 0 個のときの entrypoint の usage（exit 2）は固定されていない。 + +**手順**: 1. rotate-pr.sh execute --mode を値なしで実行し、終了コードが 0 以外で stderr に --mode requires light|squash が出ることを確かめる +2. rotate-pr.sh を引数なしで実行し、終了コードが 2 で usage が stderr に出ることを確かめる +3. いずれも state.json を用意せず、gh/git を呼ぶ前の引数解析だけで止まることを確かめる + +### R2-003 — `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#execute_light` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| branch | integration | — | agy / kiro | 未着手 | 0 | + +**なぜ**: execute_light は prepare.json / newtext.json の有無と title/body の null を 4 本の分岐で弾くが、既存テストは prepare.json と newtext.json が両方揃った成功・失敗経路（_Rotation）しか通していない。前提ファイルが欠ける分岐と、newtext.json の title が空・body が null になる分岐はどのテストも到達していない。 + +**手順**: 1. test_rotate_pr_queue.py の _Rotation と同じ組み立て（state.json・bin の gh/git 代替）を使い、gh/git は呼ばれる前に止まることを見込む +2. prepare.json を書かずに execute --mode light を実行し、終了コードが 1 で stderr に prepare.json not found が出ることを確かめる +3. prepare.json は置き newtext.json を書かずに実行し、終了コード 1 と stderr の newtext.json not found を確かめる +4. newtext.json に {"title": "", "body": "x"} を書いて実行し、終了コード 1 を確かめる +5. newtext.json に {"title": "x", "body": null} を書いて実行し、終了コード 1 を確かめる +6. いずれの分岐でも gh の呼び出し記録（GH_CALLS）が空で、旧 PR を close していないことを確かめる + +### R2-004 — `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#load_state` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| error | integration | — | agy / kiro | 未着手 | 0 | + +**なぜ**: load_state は state.json が不在または空のときに終了コード 1 と state.json not found を返して中断するが、rotate-pr.sh の公開入口を経由してこのエラー経路を通すテストが無い。launch-reviewer.sh 等では固定されているが rotate-pr.sh では未固定である + +**手順**: 1. CROSS_REVIEW_TMP_DIR を空の temp ディレクトリに向け、state.json を置かない +2. rotate-pr.sh execute を実行する +3. 終了コードが 1 であることを確かめる +4. stderr に state.json not found が含まれることを確かめる +5. gh/git の代替を PATH に置き、呼び出し記録が空（load_state の手前で止まる）であることを確かめる + +### R2-005 — `plugins/ndf/skills/cross-review/scripts/state.py#cmd_set_current_pr` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| branch | unit | — | codex / agy | 検証中 | 1 | + +**なぜ**: cmd_set_current_pr において、pr_history に既に複数の履歴（過去に閉じた PR と現在開いている PR）が存在する場合に、過去 PR のエントリを変更せず直前の現在 PR のみ closed_at と rounds を更新して新 PR エントリを追加する分岐が未固定である + +**手順**: 1. 閉じた過去 PR（closed_at 設定済み）と現在の PR（closed_at が None）を順に含む pr_history を持つ状態ファイルを用意する +2. cmd_set_current_pr(pr, new_pr, head_branch) を実行する +3. 過去 PR の closed_at や rounds が変更されず保持されることを確かめる +4. 直前の現在 PR に closed_at が記録され、rounds がその PR のラウンド数と一致することを確かめる +5. 新 PR エントリが closed_at: None、rounds: 0 で末尾に追加されることを確かめる + ## 見送った項目 | ラウンド | 対象 | 兆候・経路 | 理由 | @@ -82,3 +151,4 @@ | 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_collect_critiques` | boundary | 1 ラウンドの採用上限 5 件を超えた | | 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_collect_critiques` | error | 1 ラウンドの採用上限 5 件を超えた | | 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_verify_findings` | error | 1 ラウンドの採用上限 5 件を超えた | +| 2 | `plugins/ndf/skills/cross-review/scripts/launch-reviewer.sh#launch_reviewer` | branch | 1 ラウンドの採用上限 5 件を超えた | From 4cd469bc388e45e7c6e77f0793dc45cbad66c08c Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:32:54 +0000 Subject: [PATCH 09/21] =?UTF-8?q?Test:=20=E7=8F=BE=E7=8A=B6=E5=9B=BA?= =?UTF-8?q?=E5=AE=9A=E3=83=86=E3=82=B9=E3=83=88=E3=82=92=E8=BF=BD=E5=8A=A0?= =?UTF-8?q?=20=E2=80=94=20rotate-pr.sh#execute=5Flight=20=E3=81=AE?= =?UTF-8?q?=E5=89=8D=E6=8F=90=E3=83=95=E3=82=A1=E3=82=A4=E3=83=AB=E6=A4=9C?= =?UTF-8?q?=E8=A8=BC=204=20=E5=88=86=E5=B2=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit execute --mode light が prepare.json の欠落・newtext.json の欠落・title の 空文字列・body の null をそれぞれ終了コード 1 で弾き、gh を呼ばず旧 PR を open のまま残すことを固定する。対象のコードは変更しない。 Item-Id: R2-003 Round: 2 Impl-Runtime: claude Impl-Model: default Co-Authored-By: Claude Fable 5.1 --- .../tests/test_rotate_pr_queue.py | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py index 9202705a..b5dfc9aa 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py +++ b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py @@ -303,6 +303,73 @@ def test_a_create_success_in_squash_mode_closes_the_old_pr_and_opens_the_new_pr( assert not any(c.startswith("pr reopen") for c in rotation.gh_calls()) +# ---- execute --mode light の前提ファイル検証（R2-003、現状固定） ---- +# +# execute_light は prepare.json / newtext.json の有無と title / body の欠落を 4 本の分岐で +# 弾く。いずれも `git push` と旧 PR の close より前で止まるため、gh は 1 度も呼ばれず +# 旧 PR は open のまま残る。既存の _Rotation は両ファイルを揃えて書くので、ここでは +# 欠けさせる・書き換える操作を足してから実行する。正しさを主張しない現状固定テスト。 + + +def _prepare_path(rotation: _Rotation) -> pathlib.Path: + return rotation.tmp / f"rotate-pr{_STATE_PR}-prepare.json" + + +def _newtext_path(rotation: _Rotation) -> pathlib.Path: + return rotation.tmp / f"rotate-pr{_STATE_PR}-newtext.json" + + +def _assert_stopped_before_touching_the_old_pr(rotation: _Rotation) -> None: + assert rotation.gh_calls() == [] + assert rotation.pr_states() == {str(_OLD_PR): "open"} + + +def test_light_mode_stops_when_prepare_json_is_missing(rotation: _Rotation) -> None: + """prepare.json が無いと終了コード 1 で止まり、gh は呼ばれない。""" + _prepare_path(rotation).unlink() + + out = rotation.run(create_ok=True) + + assert out.returncode == 1 + assert "prepare.json not found" in out.stderr + _assert_stopped_before_touching_the_old_pr(rotation) + + +def test_light_mode_stops_when_newtext_json_is_missing(rotation: _Rotation) -> None: + """prepare.json はあっても newtext.json が無いと終了コード 1 で止まり、gh は呼ばれない。""" + _newtext_path(rotation).unlink() + + out = rotation.run(create_ok=True) + + assert out.returncode == 1 + assert "newtext.json not found" in out.stderr + _assert_stopped_before_touching_the_old_pr(rotation) + + +def test_light_mode_stops_when_the_title_is_empty(rotation: _Rotation) -> None: + """newtext.json の title が空文字列だと終了コード 1 で止まり、gh は呼ばれない。""" + _newtext_path(rotation).write_text( + json.dumps({"title": "", "body": "x"}), encoding="utf-8") + + out = rotation.run(create_ok=True) + + assert out.returncode == 1 + assert ".title がない" in out.stderr + _assert_stopped_before_touching_the_old_pr(rotation) + + +def test_light_mode_stops_when_the_body_is_null(rotation: _Rotation) -> None: + """newtext.json の body が null だと終了コード 1 で止まり、gh は呼ばれない。""" + _newtext_path(rotation).write_text( + json.dumps({"title": "x", "body": None}), encoding="utf-8") + + out = rotation.run(create_ok=True) + + assert out.returncode == 1 + assert ".body がない" in out.stderr + _assert_stopped_before_touching_the_old_pr(rotation) + + # ---- execute の引数検証（R2-004、現状固定） ---- # # `--mode` の値検証と未知フラグの検出は gh/git を一切呼ばない純粋な引数解析であり、 From b9026de935a0aed8030de27550c3d00d313801c5 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:33:19 +0000 Subject: [PATCH 10/21] =?UTF-8?q?Revert=20"Test:=20=E7=8F=BE=E7=8A=B6?= =?UTF-8?q?=E5=9B=BA=E5=AE=9A=E3=83=86=E3=82=B9=E3=83=88=E3=82=92=E8=BF=BD?= =?UTF-8?q?=E5=8A=A0=20=E2=80=94=20rotate-pr.sh#execute=5Flight=20?= =?UTF-8?q?=E3=81=AE=E5=89=8D=E6=8F=90=E3=83=95=E3=82=A1=E3=82=A4=E3=83=AB?= =?UTF-8?q?=E6=A4=9C=E8=A8=BC=204=20=E5=88=86=E5=B2=90"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 4cd469bc388e45e7c6e77f0793dc45cbad66c08c. --- .../tests/test_rotate_pr_queue.py | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py index b5dfc9aa..9202705a 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py +++ b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py @@ -303,73 +303,6 @@ def test_a_create_success_in_squash_mode_closes_the_old_pr_and_opens_the_new_pr( assert not any(c.startswith("pr reopen") for c in rotation.gh_calls()) -# ---- execute --mode light の前提ファイル検証（R2-003、現状固定） ---- -# -# execute_light は prepare.json / newtext.json の有無と title / body の欠落を 4 本の分岐で -# 弾く。いずれも `git push` と旧 PR の close より前で止まるため、gh は 1 度も呼ばれず -# 旧 PR は open のまま残る。既存の _Rotation は両ファイルを揃えて書くので、ここでは -# 欠けさせる・書き換える操作を足してから実行する。正しさを主張しない現状固定テスト。 - - -def _prepare_path(rotation: _Rotation) -> pathlib.Path: - return rotation.tmp / f"rotate-pr{_STATE_PR}-prepare.json" - - -def _newtext_path(rotation: _Rotation) -> pathlib.Path: - return rotation.tmp / f"rotate-pr{_STATE_PR}-newtext.json" - - -def _assert_stopped_before_touching_the_old_pr(rotation: _Rotation) -> None: - assert rotation.gh_calls() == [] - assert rotation.pr_states() == {str(_OLD_PR): "open"} - - -def test_light_mode_stops_when_prepare_json_is_missing(rotation: _Rotation) -> None: - """prepare.json が無いと終了コード 1 で止まり、gh は呼ばれない。""" - _prepare_path(rotation).unlink() - - out = rotation.run(create_ok=True) - - assert out.returncode == 1 - assert "prepare.json not found" in out.stderr - _assert_stopped_before_touching_the_old_pr(rotation) - - -def test_light_mode_stops_when_newtext_json_is_missing(rotation: _Rotation) -> None: - """prepare.json はあっても newtext.json が無いと終了コード 1 で止まり、gh は呼ばれない。""" - _newtext_path(rotation).unlink() - - out = rotation.run(create_ok=True) - - assert out.returncode == 1 - assert "newtext.json not found" in out.stderr - _assert_stopped_before_touching_the_old_pr(rotation) - - -def test_light_mode_stops_when_the_title_is_empty(rotation: _Rotation) -> None: - """newtext.json の title が空文字列だと終了コード 1 で止まり、gh は呼ばれない。""" - _newtext_path(rotation).write_text( - json.dumps({"title": "", "body": "x"}), encoding="utf-8") - - out = rotation.run(create_ok=True) - - assert out.returncode == 1 - assert ".title がない" in out.stderr - _assert_stopped_before_touching_the_old_pr(rotation) - - -def test_light_mode_stops_when_the_body_is_null(rotation: _Rotation) -> None: - """newtext.json の body が null だと終了コード 1 で止まり、gh は呼ばれない。""" - _newtext_path(rotation).write_text( - json.dumps({"title": "x", "body": None}), encoding="utf-8") - - out = rotation.run(create_ok=True) - - assert out.returncode == 1 - assert ".body がない" in out.stderr - _assert_stopped_before_touching_the_old_pr(rotation) - - # ---- execute の引数検証（R2-004、現状固定） ---- # # `--mode` の値検証と未知フラグの検出は gh/git を一切呼ばない純粋な引数解析であり、 From 532d9356cfddb69b9585ccf08589e36ae173de40 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:33:19 +0000 Subject: [PATCH 11/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index 45b29e8a..0ceba6c8 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -78,7 +78,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| branch | unit | — | codex / agy | 検証中 | 1 | +| branch | unit | — | codex / agy | 採用 | 1 | **なぜ**: measure の oracle 算出において、resolved_thread_positions の要素が辞書形式である経路は固定されているが、リスト内に非辞書要素（文字列や null など）が混在した場合にそれを unmatched として数えて測定を継続する分岐が未固定である @@ -90,7 +90,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| boundary | integration | — | agy / kiro | 検証中 | 1 | +| boundary | integration | — | agy / kiro | 採用 | 1 | **なぜ**: cmd_execute の引数解析は不正な --mode 値と未知フラグ（exit 2）は固定済みだが、--mode の直後に値が無いとき ${2:?--mode requires light|squash} で落ちる境界と、そもそも引数が 0 個のときの entrypoint の usage（exit 2）は固定されていない。 @@ -102,7 +102,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| branch | integration | — | agy / kiro | 未着手 | 0 | +| branch | integration | — | agy / kiro | 取り消し | 1 | **なぜ**: execute_light は prepare.json / newtext.json の有無と title/body の null を 4 本の分岐で弾くが、既存テストは prepare.json と newtext.json が両方揃った成功・失敗経路（_Rotation）しか通していない。前提ファイルが欠ける分岐と、newtext.json の title が空・body が null になる分岐はどのテストも到達していない。 @@ -131,7 +131,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| branch | unit | — | codex / agy | 検証中 | 1 | +| branch | unit | — | codex / agy | 採用 | 1 | **なぜ**: cmd_set_current_pr において、pr_history に既に複数の履歴（過去に閉じた PR と現在開いている PR）が存在する場合に、過去 PR のエントリを変更せず直前の現在 PR のみ closed_at と rounds を更新して新 PR エントリを追加する分岐が未固定である @@ -152,3 +152,4 @@ | 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_collect_critiques` | error | 1 ラウンドの採用上限 5 件を超えた | | 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_verify_findings` | error | 1 ラウンドの採用上限 5 件を超えた | | 2 | `plugins/ndf/skills/cross-review/scripts/launch-reviewer.sh#launch_reviewer` | branch | 1 ラウンドの採用上限 5 件を超えた | +| 2 | `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#execute_light` | branch | コミット 4cd469bc388e45e7c6e77f0793dc45cbad66c08c にトレーラーが欠けています: Item-Id, Round, Impl-Runtime, Impl-Model | From 39fb6e09154fc91d2c80aa844f9e655b5aa8cc92 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:37:05 +0000 Subject: [PATCH 12/21] =?UTF-8?q?Test:=20rotate-pr=20=E3=81=AE=20state=20?= =?UTF-8?q?=E4=B8=8D=E5=9C=A8=E7=B5=8C=E8=B7=AF=E3=82=92=E5=9B=BA=E5=AE=9A?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 公開入口 execute が state.json 不在時に終了コード 1 とエラーを返し、gh/git を呼ばない現状を固定する。 Item-Id: R2-004 Round: 2 Impl-Runtime: codex Impl-Model: default --- .../tests/test_rotate_pr_queue.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py index 9202705a..d1865dc2 100644 --- a/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py +++ b/plugins/ndf/skills/cross-review/tests/test_rotate_pr_queue.py @@ -345,6 +345,37 @@ def test_mode_without_a_value_is_rejected() -> None: assert "--mode requires light|squash" in out.stderr +def test_execute_stops_when_state_json_is_missing(tmp_path) -> None: + """現状固定（R2-004）。state 不在なら外部コマンドを呼ばずに終了する。""" + tmp_dir = tmp_path / "tmp" + bin_dir = tmp_path / "bin" + calls = tmp_path / "calls.log" + tmp_dir.mkdir() + bin_dir.mkdir() + calls.write_text("", encoding="utf-8") + + fake_command = "#!/usr/bin/env bash\nprintf '%s\\n' \"$0 $*\" >> \"$CALLS\"\n" + for command in ("gh", "git"): + executable = bin_dir / command + executable.write_text(fake_command, encoding="utf-8") + executable.chmod(0o755) + + env = { + **os.environ, + "PATH": f"{bin_dir}{os.pathsep}{os.environ['PATH']}", + "CROSS_REVIEW_TMP_DIR": str(tmp_dir), + "CALLS": str(calls), + } + out = subprocess.run( + ["bash", str(ROTATE), "execute", str(_STATE_PR)], + capture_output=True, text=True, timeout=60, env=env, + ) + + assert out.returncode == 1 + assert "state.json not found" in out.stderr + assert calls.read_text(encoding="utf-8") == "" + + def test_no_arguments_prints_usage() -> None: """現状固定（R2-002）。引数が 0 個のとき entrypoint は usage を出して exit 2。 From bab284edbcab40710348b9924b4c78f70b69708c Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 20:37:39 +0000 Subject: [PATCH 13/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index 0ceba6c8..4972cdbc 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -117,7 +117,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| error | integration | — | agy / kiro | 未着手 | 0 | +| error | integration | — | agy / kiro | 検証中 | 1 | **なぜ**: load_state は state.json が不在または空のときに終了コード 1 と state.json not found を返して中断するが、rotate-pr.sh の公開入口を経由してこのエラー経路を通すテストが無い。launch-reviewer.sh 等では固定されているが rotate-pr.sh では未固定である From 5cfb48f41dfd0bf6f39e29cf4641ad212ddc7c43 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:01:30 +0000 Subject: [PATCH 14/21] =?UTF-8?q?Refactor:=20extract=5Fmethod=20=E2=80=94?= =?UTF-8?q?=20plugins/ndf/skills/cross-review/scripts/state.py#=5Fverify?= =?UTF-8?q?=5Ffindings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _verify_findings から以下の helper を抽出: - 1 finding の verification record を生成しキャッシュと結果分類を行う _verify_one_finding - merged_into の関係をたどり代表へ最良の verification を選ぶ _select_best_verification _verify_findings を対象抽出、各 finding の検証、代表結果の集約の 3 段階に整理。 Item-Id: R3-001 Round: 3 Impl-Runtime: agy Impl-Model: default --- .../ndf/skills/cross-review/scripts/state.py | 82 ++++++++++++------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index 9ee5c2ac..ad35f086 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -3065,6 +3065,55 @@ def _merged_root( return current +def _verify_one_finding( + finding: dict[str, Any], + allowed: list[str], + work: str, + codes: set[int], + run: Any, + ran: dict[tuple[str, ...], Optional[int]], +) -> dict[str, Any]: + """1 つの指摘の suggested_check を検証し、verification レコードを生成する。""" + check = str(finding.get("suggested_check") or "") + record: dict[str, Any] = { + "command": check, "finding_id": finding.get("finding_id"), + "exit_code": None, "result": "not_run", "ran_at": None, + } + argv = _verify_argv(check, allowed, work) if allowed else None + if argv is not None: + key = tuple(argv) + if key in ran: + code = ran[key] + else: + code = run(argv, work) + ran[key] = code + record["exit_code"] = code + record["ran_at"] = _now() + if code in codes: + record["result"] = "reproduced" + elif code == 0: + record["result"] = "not_reproduced" + return record + + +def _select_best_verification( + rep: dict[str, Any], + targets: list[dict[str, Any]], + by_id: dict[Any, dict[str, Any]], +) -> dict[str, Any]: + """merged_into の関係をたどり、代表へ最良の verification を選ぶ。""" + best = rep["verification"] + for member in targets: + if member is rep or not member.get("merged_into"): + continue + if _merged_root(member, by_id) is not rep: + continue + if _VERIFY_RANK.get(_verify_result(member), -1) > \ + _VERIFY_RANK.get(str(best.get("result") or "not_run"), -1): + best = member["verification"] + return best + + def _verify_findings( st: dict[str, Any], round_no: int, @@ -3103,40 +3152,15 @@ def _verify_findings( ran: dict[tuple[str, ...], Optional[int]] = {} for finding in targets: - check = str(finding.get("suggested_check") or "") - record: dict[str, Any] = { - "command": check, "finding_id": finding.get("finding_id"), - "exit_code": None, "result": "not_run", "ran_at": None, - } - argv = _verify_argv(check, allowed, work) if allowed else None - if argv is not None: - key = tuple(argv) - if key in ran: - code = ran[key] - else: - code = run(argv, work) - ran[key] = code - record["exit_code"] = code - record["ran_at"] = _now() - if code in codes: - record["result"] = "reproduced" - elif code == 0: - record["result"] = "not_reproduced" - finding["verification"] = record + finding["verification"] = _verify_one_finding( + finding, allowed, work, codes, run, ran, + ) # 代表は組から選び直す。**実行し直さない**（記録済みの結果を選ぶだけである）。 for rep in targets: if rep.get("merged_into"): continue - best = rep["verification"] - for member in targets: - if member is rep or not member.get("merged_into"): - continue - if _merged_root(member, by_id) is not rep: - continue - if _VERIFY_RANK.get(_verify_result(member), -1) > \ - _VERIFY_RANK.get(str(best.get("result") or "not_run"), -1): - best = member["verification"] + best = _select_best_verification(rep, targets, by_id) if best is not rep["verification"]: rep["verification"] = dict(best) From 9e73ace957a4a99f8cf6caa0c18722487127bfc3 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:13:25 +0000 Subject: [PATCH 15/21] =?UTF-8?q?Refactor:=20centralize=5Fconfiguration=20?= =?UTF-8?q?=E2=80=94=20plugins/ndf/skills/cross-review/scripts/state.py#CO?= =?UTF-8?q?UNTED=5FCLASSIFICATIONS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cross-review の scripts / tests の構造改善（振る舞い不変）を 1 コミットにまとめる。 - R4-001: COUNTED_CLASSIFICATIONS を scripts/classifications.py へ集約し、 state.py と measure.py が共有定義を import する（両者の重複を解消）。 - R4-004: conftest.py の autouse fixture を分割。gh 実行ガードは state_mod に 依存させず、state.py の既定差し替えは state_mod を要求するテストだけへ適用する。 - R4-005: measure.py の _proposed から oracle のラウンド別分母絞りを _scoped_oracle_ids として抽出する。 Item-Id: R4-001 Round: 4 Impl-Runtime: kiro Impl-Model: default --- .../cross-review/scripts/classifications.py | 12 ++++ .../skills/cross-review/scripts/measure.py | 56 ++++++++++++------- .../ndf/skills/cross-review/scripts/state.py | 11 ++-- .../ndf/skills/cross-review/tests/conftest.py | 24 +++++++- 4 files changed, 74 insertions(+), 29 deletions(-) create mode 100644 plugins/ndf/skills/cross-review/scripts/classifications.py diff --git a/plugins/ndf/skills/cross-review/scripts/classifications.py b/plugins/ndf/skills/cross-review/scripts/classifications.py new file mode 100644 index 00000000..d25983e0 --- /dev/null +++ b/plugins/ndf/skills/cross-review/scripts/classifications.py @@ -0,0 +1,12 @@ +"""cross-review の区分に関する共有定義（#156、#732）。 + +収束の判定（`state.py`）と効果の測定（`measure.py`）が同じ区分を数えるための +唯一の定義を置く。片方だけに区分を足すと、判定が数えた指摘を測定が採らず、 +その方式の再現率が実際より低く出る（`test_measure.py` が両者の一致を固定する）。 +""" +from __future__ import annotations + +# 収束の判定が数える区分（#156、#732）。**残る 3 つは数えない。** 数えないのは、誤りだと +# 示された棄却と、承認を妨げない軽微な指摘だけである。棄却した指摘を数えると、そのぶん +# ラウンドが増える（#69 で同じ論点が 5 ラウンド続いた事象）。 +COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment", "unrefuted") diff --git a/plugins/ndf/skills/cross-review/scripts/measure.py b/plugins/ndf/skills/cross-review/scripts/measure.py index ba521c43..cf21ac73 100755 --- a/plugins/ndf/skills/cross-review/scripts/measure.py +++ b/plugins/ndf/skills/cross-review/scripts/measure.py @@ -29,6 +29,11 @@ import sys from typing import Any, NamedTuple +# 区分の定義は scripts 配下の共有モジュールに 1 か所だけ置く（#156、#732）。 +# `state.py` も同じ定義を読み、両者の一致は `test_measure.py` が固定する。 +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +from classifications import COUNTED_CLASSIFICATIONS # noqa: E402 + # **担当の名前は 4 つである。** `reviewers` を持たない古い記録で、結果を残した # 担当を数えるために使う（`state.py` の `LEGACY_AGENTS` は 2 者で、母集合を @@ -416,12 +421,6 @@ def _majority(representatives: list[dict[str, Any]], return _method_output(finding_ids, oracle_ids) -# 3 本目の区分（#732 で 6 つ）のうち、この変更の方式が採る 3 つ（`state.py` の -# `COUNTED_CLASSIFICATIONS` と同じ。一致は `test_measure.py` が固定する）。 -# **残る 3 つは採らない。** -COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment", "unrefuted") - - def _evidence_rounds(st: dict[str, Any]) -> set[int]: """証拠集約（統合・実行検証・反証）を通ったラウンドの印。 @@ -446,6 +445,31 @@ def _all_rounds_marked(st: dict[str, Any], marked: set[int]) -> bool: return all(_round_no(rounds, i) in marked for i in range(len(rounds))) +def _scoped_oracle_ids( + representatives: list[dict[str, Any]], + oracle_ids: set[str] | None, + marked: set[int], +) -> set[str] | None: + """上限の方式の集合を、印のあるラウンドの指摘だけへ絞る（#156）。 + + 分母を全ラウンドのままにすると、印の混ざった記録で再現率が過小に出る。分子と + 同じ母集合（印のあるラウンド）へ絞るため、`finding_id` から `round` を引いて + `marked` に含まれるものだけを残す。 + + Returns: + - `oracle_ids` が `None`（上限を計算できない）なら `None` を返す。 + - `marked` が全ラウンドを覆うなら、絞り込みの結果は `oracle_ids` と同じになる。 + - 一部のラウンドだけが印を持つなら、そのラウンドの指摘だけが残る。 + """ + if oracle_ids is None: + return None + rounds_by_id = { + str(finding.get("finding_id")): _as_int(finding.get("round")) + for finding in representatives + } + return {fid for fid in oracle_ids if rounds_by_id.get(fid) in marked} + + def _proposed(st: dict[str, Any], representatives: list[dict[str, Any]], oracle_ids: set[str] | None) -> dict[str, Any]: """この変更の方式。**読むのは証拠集約を通ったラウンドだけである。** @@ -453,10 +477,11 @@ def _proposed(st: dict[str, Any], representatives: list[dict[str, Any]], 印の無いラウンドを母集合へ入れると、区分の付かない指摘が `insufficient_evidence` として落ち、方式の再現率が実際より低く出る。 - **分母も印のあるラウンドに限る。** 分子だけを絞ると、印の混ざった記録で - 再現率が過小に出る。印の無い round 1 と印のある round 2 に修正された指摘が - 1 件ずつあるとき、採れるのは round 2 の 1 件だけであり、全ラウンドの上限 - （2 件）で割ると**拾えるものを全部拾っても 0.5 にしかならない**。 + **分母も印のあるラウンドに限る**（絞り込みは `_scoped_oracle_ids` が持つ）。 + 分子だけを絞ると、印の混ざった記録で再現率が過小に出る。印の無い round 1 と + 印のある round 2 に修正された指摘が 1 件ずつあるとき、採れるのは round 2 の + 1 件だけであり、全ラウンドの上限（2 件）で割ると**拾えるものを全部拾っても + 0.5 にしかならない**。 **分母が全ラウンドと違うことは出力へ出す。** 添えないと、読む側がこの方式の 再現率を他の 3 つと同じ分母の値として読む。 @@ -474,16 +499,7 @@ def _proposed(st: dict[str, Any], representatives: list[dict[str, Any]], if _as_int(finding.get("round")) in marked and finding.get("classification") in COUNTED_CLASSIFICATIONS } - if oracle_ids is None: - base_ids = None - else: - rounds_by_id = { - str(finding.get("finding_id")): _as_int(finding.get("round")) - for finding in representatives - } - base_ids = { - fid for fid in oracle_ids if rounds_by_id.get(fid) in marked - } + base_ids = _scoped_oracle_ids(representatives, oracle_ids, marked) result = _method_output(finding_ids, base_ids) result["oracle_scope"] = ( "all_rounds" if _all_rounds_marked(st, marked) else "evidence_rounds" diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index ad35f086..80dc773c 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -36,6 +36,11 @@ import post_queue # noqa: E402 import run_metrics # noqa: E402 実行の要約（#662） +# 区分の定義は scripts 配下の共有モジュールに 1 か所だけ置く（#156、#732）。 +# `measure.py` も同じ定義を読み、両者の一致は `test_measure.py` が固定する。 +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +from classifications import COUNTED_CLASSIFICATIONS # noqa: E402 + # ---------------- helpers ---------------- @@ -3464,12 +3469,6 @@ def _declared_duplicate_targets(finding: dict[str, Any]) -> set: return targets -# 収束の判定が数える区分（#156、#732）。**残る 3 つは数えない。** 数えないのは、誤りだと -# 示された棄却と、承認を妨げない軽微な指摘だけである。棄却した指摘を数えると、そのぶん -# ラウンドが増える（#69 で同じ論点が 5 ラウンド続いた事象）。 -COUNTED_CLASSIFICATIONS = ("verified_blocking", "needs_human_judgment", "unrefuted") - - def _verdicts(finding: dict[str, Any], verdict: str) -> list[str]: """その値を返した担当の一覧。""" return [ diff --git a/plugins/ndf/skills/cross-review/tests/conftest.py b/plugins/ndf/skills/cross-review/tests/conftest.py index e9bcbc23..67cd6aec 100644 --- a/plugins/ndf/skills/cross-review/tests/conftest.py +++ b/plugins/ndf/skills/cross-review/tests/conftest.py @@ -89,7 +89,7 @@ def _default_host(monkeypatch) -> None: @pytest.fixture(autouse=True) -def _no_github(monkeypatch, state_mod) -> None: +def _no_github(monkeypatch) -> None: """テストから GitHub を呼ばない。 収束の判定は継続的統合を照会するようになった（#327）。差し替えを忘れると、 @@ -97,6 +97,10 @@ def _no_github(monkeypatch, state_mod) -> None: **差し替えていない `gh` の実行はその場で落とす。** `subprocess.run` そのものを差し替えるテストは、この見張りを上書きして先へ進む。 + + **state.py の内部関数の差し替えは持たない。** それは `state_mod` を利用する + テストだけが必要とする（`_no_github_state`）。ここに混ぜると、monitor.py や + measure.py だけを検査するテストまで state.py を読み込む。 """ real = subprocess.run @@ -109,8 +113,22 @@ def _guard(cmd, *args, **kwargs): return real(cmd, *args, **kwargs) monkeypatch.setattr(subprocess, "run", _guard) - # 照会は既定で「確かめられなかった」に倒す。判定は収束を止めない側へ倒すため、 - # 検査ジョブを見ない既存のテストは期待値を変えずに通る。 + + +@pytest.fixture(autouse=True) +def _no_github_state(request, monkeypatch) -> None: + """state.py の GitHub 照会を既定で「確かめられなかった」に倒す。 + + **`state_mod` を要求するテストだけへ適用する。** monitor.py や measure.py だけを + 検査するテストは `state_mod` を要求しないため、この差し替えを通らず state.py を + 読み込まない。要求するテストでは従来どおり実 GitHub 呼び出しを防ぐ。 + + 判定は収束を止めない側へ倒すため、検査ジョブを見ない既存のテストは期待値を + 変えずに通る。 + """ + if "state_mod" not in request.fixturenames: + return + state_mod = request.getfixturevalue("state_mod") monkeypatch.setattr(state_mod, "_fetch_check_runs", lambda repo, sha: None) monkeypatch.setattr(state_mod, "_fetch_pr_metadata", lambda pr, repo=None: None) From 8f8d7d1706570ee4b4a0c8037d784d99aaa7fe2b Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:14:04 +0000 Subject: [PATCH 16/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 132 ++++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 1 deletion(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index 4972cdbc..51a02bee 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -117,7 +117,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| error | integration | — | agy / kiro | 検証中 | 1 | +| error | integration | — | agy / kiro | 採用 | 1 | **なぜ**: load_state は state.json が不在または空のときに終了コード 1 と state.json not found を返して中断するが、rotate-pr.sh の公開入口を経由してこのエラー経路を通すテストが無い。launch-reviewer.sh 等では固定されているが rotate-pr.sh では未固定である @@ -141,6 +141,133 @@ 4. 直前の現在 PR に closed_at が記録され、rounds がその PR のラウンド数と一致することを確かめる 5. 新 PR エントリが closed_at: None、rounds: 0 で末尾に追加されることを確かめる +## ラウンド 3（実装 kiro / レビュー codex / agy） + +### R3-001 — `plugins/ndf/skills/cross-review/scripts/state.py#_verify_findings` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 未着手 | 0 | + +**なぜ**: 検証コマンドの正規化・重複実行の抑止・実行結果の分類・各 finding への記録・統合グループ代表の最良結果選択という独立した段階が 1 関数に連続し、実行キャッシュと統合関係の走査を同時に追う必要がある。 + +**手順**: 1. 1 finding の verification record を生成し、コマンド実行キャッシュを利用して結果を分類する helper を抽出する +2. merged_into の関係をたどって代表へ最良の verification を選ぶ処理を別 helper へ抽出する +3. _verify_findings は対象抽出、各 finding の検証、代表結果の集約という 3 段階だけを並べる +4. test_verify_findings.py と findings pipeline の既存テストで、同一コマンドの実行回数、結果優先順位、finding_id、ran_at が不変であることを確認する + +### R3-002 — `plugins/ndf/skills/cross-review/scripts/state.py#_resume_from_state` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 未着手 | 0 | + +**なぜ**: 再開 state の探索・互換フィールドの補完・未解決指摘の引き継ぎ・待ち行列の flush・worktree 同期・結果出力という複数段階が 1 関数に同居し、書き戻しと flush の順序制約まで同じ本体で管理している。 + +**手順**: 1. state の互換フィールド補完と review_instructions 再構成を、state と変更有無を返す helper へ抽出する +2. 書き戻し後の auto-flush と worktree 同期を、順序を保持した再開準備 helper へ抽出する +3. _resume_from_state は state の有無・完了判定、各 helper の呼び出し、既存の _print_init_result だけを順に行う構成へ縮める +4. 既存の再開・carried-over・worktree 同期・run metrics のテストで出力と副作用順が不変であることを確認する + +### R3-003 — `plugins/ndf/skills/cross-review/scripts/state.py#_thread_ids` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| duplication | consolidate_duplication | minor | agy | 未着手 | 0 | + +**なぜ**: _thread_ids における入力データ（リスト、単一辞書、数値等）の辞書要素抽出・正規化ロジックが、同モジュール内の共通関数 _normalize_dict_items と同じ関心をインラインで再実装しており重複している。_thread_positions と同様に _normalize_dict_items を呼び出す形に統一することで、入力値の正規化処理を一元化し一貫性と保守性を高められる。 + +**手順**: 1. _thread_ids 内の辞書要素抽出処理を _normalize_dict_items(value) の呼び出しに置き換える +2. 既存の test_state_thread_ids.py を実行し、各種入力に対する戻り値が変わらないことを確認する + +### R3-004 — `plugins/ndf/skills/cross-review/scripts/state.py#_is_generated_path` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| magic_value | introduce_named_constant | minor | agy | 未着手 | 0 | + +**なぜ**: パス分類判定関数群（_is_dependency_path, _is_config_ci_path, _is_infra_path 等）がモジュール定数（DEPENDENCY_FILENAMES, CONFIG_CI_FILENAMES, INFRA_FILENAMES 等）を参照しているのに対し、_is_generated_path 内にのみロックファイル名の一覧 set リテラルがハードコードされている。名前付きモジュール定数 GENERATED_LOCK_FILENAMES を定義して参照させることで、定数管理の一貫性と保守性を向上できる。 + +**手順**: 1. モジュール定数 GENERATED_LOCK_FILENAMES を定義する +2. _is_generated_path 内の set リテラルを GENERATED_LOCK_FILENAMES の参照に置き換える +3. 既存テストでパス分類の判定動作が不変であることを確認する + +### R3-005 — `plugins/ndf/skills/cross-review/scripts/state.py#_absorb` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| duplication | consolidate_duplication | minor | kiro | 未着手 | 0 | + +**なぜ**: 「2 つの指摘のうち検証結果 (reproduced > not_reproduced > not_run) が高い方を採る」という同じ業務ルールが _absorb (3645-3646 行) と _verify_findings の代表選び直しループ (3137-3138 行) の 2 箇所に _VERIFY_RANK.get(...) > _VERIFY_RANK.get(...) の比較として書かれている。_VERIFY_RANK の順位定義を変えるときや、片方だけ result の取り出し方 (_verify_result vs best.get('result')) を直したときに、もう片方だけ取り残される。両者の docstring がどちらも同じ順位を根拠に挙げており、同じ理由で一緒に変わる重複である。 + +**手順**: 1. _VERIFY_RANK 定義の直後に、2 つの verification dict を受け取り順位の高い方を返すヘルパー _higher_ranked_verification(current, candidate) を追加する（rank は _verify_result で正規化して比較する） +2. _verify_findings の代表選び直しループ (3135-3140) を、best と member['verification'] をヘルパーへ渡して best を更新する形へ置き換える +3. _absorb の verification 継承部 (3644-3648) を、同じヘルパーで rep['verification'] を更新する形へ置き換える +4. test_verify_findings.py / test_merge_duplicates.py / test_state_merge_fix.py を実行し、reproduced/not_reproduced/not_run の組で代表が採る値が変わらないことを確認する + +## ラウンド 4（実装 claude / レビュー codex / kiro） + +### R4-001 — `plugins/ndf/skills/cross-review/scripts/state.py#COUNTED_CLASSIFICATIONS` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| scattered_config | centralize_configuration | major | codex | 検証中 | 1 | + +**なぜ**: 収束判定が数える区分の組が state.py と measure.py に重複し、両者の一致をテストで監視している。区分追加時に片方だけ変わると、実行時の収束判定と事後測定が異なる集合を数える。 + +**手順**: 1. scripts 配下の小さな共有モジュールへ COUNTED_CLASSIFICATIONS を移す +2. state.py と measure.py は共有定義を import して各判定に使う +3. 値そのものと両経路の既存出力を既存テストで固定する + +### R4-002 — `plugins/ndf/skills/cross-review/scripts/state.py#_finding_keys` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | split_into_pipeline | major | codex | 未着手 | 0 | + +**なぜ**: レビュワーごとのファイル解決、JSON 読み込み、payload と comments の境界検証、path・line・本文の正規化が1つの二重ループに入り、入力境界の失敗とキー変換の責務が分離されていない。 + +**手順**: 1. payload ファイルの読み込みと dict 検証を第1段へ抽出する +2. comments 要素の検証と3要素キーへの変換を第2段へ抽出する +3. _finding_keys はレビュワー列挙から各段をつなぐ処理だけにする +4. 不正 payload・不正 comment・欠損位置・正常な振動照合の既存テストを各段階で実行する + +### R4-003 — `plugins/ndf/skills/cross-review/scripts/state.py#_init_new_state` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | major | codex | 未着手 | 0 | + +**なぜ**: 新規初期化の1関数内に PR 所有権解決、レビュー条件作成、worktree と既存コメントの準備、担当認証、初期 state 構築、保存と表示がネスト関数として同居し、各段階を単独で参照・テストできない。 + +**手順**: 1. ネストされた各段階を同じ入出力のモジュールレベル関数へ順に移す +2. _init_new_state はコンテキストを段階間で受け渡すオーケストレーションだけにする +3. init の再開・新規作成・既存 worktree・コメント取得失敗の既存テストを各抽出後に実行する + +### R4-004 — `plugins/ndf/skills/cross-review/tests/conftest.py#_no_github` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| test_bypasses_module_boundary | move_responsibility | minor | codex | 検証中 | 1 | + +**なぜ**: autouse fixture が state_mod を引数に取るため、monitor.py や measure.py だけを検査するテストまで state.py を共通入口から読み込み、GitHub 照会の内部関数を一律に差し替えている。 + +**手順**: 1. subprocess の gh 実行ガードと state.py の既定差し替えを別 fixture に分ける +2. state.py の差し替えは state_mod を利用するテスト経路だけが要求する形へ移す +3. monitor・measure のテストが state.py を読み込まず、state 系テストでは従来どおり実 GitHub 呼び出しを防ぐことを確認する + +### R4-005 — `plugins/ndf/skills/cross-review/scripts/measure.py#_proposed` + +| 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | +| --- | --- | --- | --- | --- | ---: | +| long_method | extract_method | minor | codex | 検証中 | 1 | + +**なぜ**: 証拠ラウンドの検査、採用 finding 集合の作成、oracle のラウンド別分母への絞り込み、出力メタデータ付与を1関数が連続して担い、分母規則だけを独立に検証しにくい。 + +**手順**: 1. finding_id から round を引き分母を絞る処理を _scoped_oracle_ids として抽出する +2. oracle が未計算の場合と evidence_rounds が一部だけの場合の戻り値を明示する +3. _proposed は採用集合の作成と出力組み立てだけに残し、既存の measure テストを実行する + ## 見送った項目 | ラウンド | 対象 | 兆候・経路 | 理由 | @@ -153,3 +280,6 @@ | 1 | `plugins/ndf/skills/cross-review/scripts/state.py#cmd_verify_findings` | error | 1 ラウンドの採用上限 5 件を超えた | | 2 | `plugins/ndf/skills/cross-review/scripts/launch-reviewer.sh#launch_reviewer` | branch | 1 ラウンドの採用上限 5 件を超えた | | 2 | `plugins/ndf/skills/cross-review/scripts/rotate-pr.sh#execute_light` | branch | コミット 4cd469bc388e45e7c6e77f0793dc45cbad66c08c にトレーラーが欠けています: Item-Id, Round, Impl-Runtime, Impl-Model | +| 3 | `plugins/ndf/skills/cross-review/scripts/state.py#_apply_classification` | long_method | 1 ラウンドの採用上限 5 件を超えた | +| 4 | `plugins/ndf/skills/cross-review/scripts/state.py#_finding_keys` | duplication | 1 ラウンドの採用上限 5 件を超えた | +| 4 | `plugins/ndf/skills/cross-review/scripts/state.py#_print_init_result` | long_parameter_list | 1 ラウンドの採用上限 5 件を超えた | From 0f6fd03a4c5fd476b92173bf91e3466b7f195b39 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:24:51 +0000 Subject: [PATCH 17/21] =?UTF-8?q?Refactor:=20split=5Finto=5Fpipeline=20?= =?UTF-8?q?=E2=80=94=20plugins/ndf/skills/cross-review/scripts/state.py#?= =?UTF-8?q?=5Ffinding=5Fkeys?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit レビュワーごとのファイル解決・JSON 読み込み・payload と comments の境界検証・ path / line / 本文の正規化が 1 つの二重ループに入っていた `_finding_keys` を 3 段の連鎖へ分ける。 - 第 1 段 `_read_finding_payload`: payload ファイルの読み込みと dict 検証 （無い・読めないときは None、dict でなければ die(code=3)） - 第 2 段 `_comment_keys`: comments 要素の dict 検証と 3 つ組への変換 - `_finding_keys`: レビュワー列挙から各段をつなぐだけにする 振る舞いは変えない。die のメッセージと終了コード、位置欠損・行の型不正の 読み飛ばしは元のまま。 Item-Id: R4-002 Round: 4 Impl-Runtime: claude Impl-Model: default Co-Authored-By: Claude Fable 5.1 --- .../ndf/skills/cross-review/scripts/state.py | 82 ++++++++++++------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index 80dc773c..fde27994 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -3693,38 +3693,64 @@ def _finding_keys( keys: list[tuple[str, int, str]] = [] for agent in _round_reviewers(st, round_no): p = _payload_path(agent, pr, round_no) - if not p.exists(): + payload = _read_finding_payload(agent, p) + if payload is None: continue - try: - payload = json.loads(p.read_text(encoding="utf-8")) - except json.JSONDecodeError: - continue - # gemini round 4 指摘: payload は本来 dict (comments: [...]) だが、 - # launcher のバグで list / str が入り込むと `payload.get(...)` で - # AttributeError になる。不正な review payload はバグなので - # 即時 die(code=3) で停止させる。 - if not isinstance(payload, dict): + keys.extend(_comment_keys(agent, p, payload)) + return keys + + +def _read_finding_payload(agent: str, p: pathlib.Path) -> dict[str, Any] | None: + """判定の直前に読む payload.json を dict として返す（第 1 段: 入力境界）。 + + 無い・JSON として読めないときは None を返して読み飛ばす。dict でないときは + launcher のバグとして `die(code=3)` で止める（`_load_payload` と違い、ここで + 止めても失われる記録が無い）。 + """ + if not p.exists(): + return None + try: + payload = json.loads(p.read_text(encoding="utf-8")) + except json.JSONDecodeError: + return None + # gemini round 4 指摘: payload は本来 dict (comments: [...]) だが、 + # launcher のバグで list / str が入り込むと `payload.get(...)` で + # AttributeError になる。不正な review payload はバグなので + # 即時 die(code=3) で停止させる。 + if not isinstance(payload, dict): + die( + f"{agent}: payload.json が dict ではない " + f"({p}, type={type(payload).__name__})。" + " review launcher の出力形式不正。", + code=3, + ) + return payload + + +def _comment_keys( + agent: str, p: pathlib.Path, payload: dict[str, Any] +) -> list[tuple[str, int, str]]: + """`comments[]` を (ファイル, 行, 正規化した本文) の 3 つ組へ変換する（第 2 段）。 + + 要素が dict でなければ `die(code=3)`。位置（path / line）が欠ける要素と、行が + 整数に読めない要素は読み飛ばす。 + """ + keys: list[tuple[str, int, str]] = [] + for c in payload.get("comments", []): + if not isinstance(c, dict): + # comments エントリが dict でない場合も同様に致命扱い die( - f"{agent}: payload.json が dict ではない " - f"({p}, type={type(payload).__name__})。" - " review launcher の出力形式不正。", + f"{agent}: payload.comments のエントリが dict ではない " + f"({p}, type={type(c).__name__})。", code=3, ) - for c in payload.get("comments", []): - if not isinstance(c, dict): - # comments エントリが dict でない場合も同様に致命扱い - die( - f"{agent}: payload.comments のエントリが dict ではない " - f"({p}, type={type(c).__name__})。", - code=3, - ) - path = c.get("path") - line = c.get("line") or c.get("start_line") - if path and line is not None: - try: - keys.append((str(path), int(line), _normalized_body(c.get("body")))) - except (TypeError, ValueError): - continue + path = c.get("path") + line = c.get("line") or c.get("start_line") + if path and line is not None: + try: + keys.append((str(path), int(line), _normalized_body(c.get("body")))) + except (TypeError, ValueError): + continue return keys From 42141142c683688dfbc7ec21682f0ab56b8f295a Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:25:45 +0000 Subject: [PATCH 18/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index 51a02bee..d80c1bb5 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -211,7 +211,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| scattered_config | centralize_configuration | major | codex | 検証中 | 1 | +| scattered_config | centralize_configuration | major | codex | 採用 | 1 | **なぜ**: 収束判定が数える区分の組が state.py と measure.py に重複し、両者の一致をテストで監視している。区分追加時に片方だけ変わると、実行時の収束判定と事後測定が異なる集合を数える。 @@ -223,7 +223,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| long_method | split_into_pipeline | major | codex | 未着手 | 0 | +| long_method | split_into_pipeline | major | codex | 検証中 | 1 | **なぜ**: レビュワーごとのファイル解決、JSON 読み込み、payload と comments の境界検証、path・line・本文の正規化が1つの二重ループに入り、入力境界の失敗とキー変換の責務が分離されていない。 @@ -248,7 +248,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| test_bypasses_module_boundary | move_responsibility | minor | codex | 検証中 | 1 | +| test_bypasses_module_boundary | move_responsibility | minor | codex | 採用 | 1 | **なぜ**: autouse fixture が state_mod を引数に取るため、monitor.py や measure.py だけを検査するテストまで state.py を共通入口から読み込み、GitHub 照会の内部関数を一律に差し替えている。 @@ -260,7 +260,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| long_method | extract_method | minor | codex | 検証中 | 1 | +| long_method | extract_method | minor | codex | 採用 | 1 | **なぜ**: 証拠ラウンドの検査、採用 finding 集合の作成、oracle のラウンド別分母への絞り込み、出力メタデータ付与を1関数が連続して担い、分母規則だけを独立に検証しにくい。 From 77d9383c32e6d9b7a90e6af5467a6c5cbdbb1932 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:37:21 +0000 Subject: [PATCH 19/21] =?UTF-8?q?Refactor:=20extract=5Fmethod=20=E2=80=94?= =?UTF-8?q?=20plugins/ndf/skills/cross-review/scripts/state.py#=5Finit=5Fn?= =?UTF-8?q?ew=5Fstate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 新規初期化の各段階をモジュールレベル関数へ抽出し、初期化関数をオーケストレーションだけに整理する。構造依存テストも抽出後の段階呼び出しを検査する形へ更新する。 Item-Id: R4-003 Round: 4 Impl-Runtime: codex Impl-Model: default --- .../ndf/skills/cross-review/scripts/state.py | 354 +++++++++--------- .../tests/test_init_body_not_duplicated.py | 16 +- 2 files changed, 185 insertions(+), 185 deletions(-) diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index fde27994..543070e4 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -1688,6 +1688,178 @@ class _InitialStateContext(NamedTuple): manual_extra_review: str +def _resolve_pr_and_ownership( + pr: object, repo: str, worktree: str, args_worktree: str | None +) -> _InitPRContext | None: + """PR のメタデータとレビュー実行者との所有関係を解決する。""" + # **作成者・head・base は REST の 1 回でまとめて取る。** 項目ごとに `gh pr view` を + # 投げていた分（GraphQL 3 点）と、リポジトリ名の解決（同 1 点）が 0 点になる。 + meta = _fetch_pr_metadata(pr, repo) + if meta is None: + die(f"PR #{pr} のメタデータを取得できません（リポジトリ名: {repo}）") + return None + if meta.repo != repo: + repo = meta.repo + if not args_worktree: + worktree = str(_default_worktree_base() / _repo_slug(repo) / f"pr{pr}") + if meta.rate_remaining is not None: + info(f"ℹ GitHub REST の残量: {meta.rate_remaining}") + + me = _sh(["gh", "api", "user", "--jq", ".login"]) + author = meta.author + is_own = (me == author) + event_downgrade = is_own + if is_own: + info(f"⚠ 自分の PR (author={me}) — REQUEST_CHANGES → COMMENT 強制ダウングレード") + + return _InitPRContext( + repo=repo, + worktree=worktree, + meta=meta, + me=me, + author=author, + is_own=is_own, + event_downgrade=event_downgrade, + ) + + +def _prepare_review_instructions( + pr: object, repo: str, manual_extra_review: str +) -> _InitReviewContext: + """変更ファイルから自動・手動のレビュー条件を組み立てる。""" + changed_files = _fetch_changed_files(pr, repo) + auto_review_categories = _classify_changed_files(changed_files) + auto_review = _auto_review_instructions(auto_review_categories) + review_instructions = _combined_review_instructions(auto_review, manual_extra_review) + return _InitReviewContext( + changed_files=changed_files, + auto_review_categories=auto_review_categories, + auto_review=auto_review, + review_instructions=review_instructions, + ) + + +def _prepare_worktree_and_comments( + worktree: str, pr: object, head_branch: str, repo: str +) -> _InitWorkspaceContext: + """worktree と既存コメントのスナップショットを準備する。""" + # worktree 分離 — _tmp_dir() より先に worktree を作成/確認する + if not pathlib.Path(worktree).exists(): + _create_worktree(worktree, pr, head_branch) + elif _is_registered_worktree(worktree): + info(f"↻ 既存 worktree 流用: {worktree}") + _sync_worktree(worktree, pr, head_branch) + else: + # パスは存在するが現リポジトリの worktree ではない (別リポジトリの残骸等)。 + # 流用すると git 操作が壊れるため退避して作り直す。 + stale = f"{worktree}.stale-{time.strftime('%Y%m%d%H%M%S')}" + pathlib.Path(worktree).rename(stale) + info(f"⚠ 現リポジトリの worktree でないため退避: {stale}") + _create_worktree(worktree, pr, head_branch) + + # worktree 作成/確認後に _tmp_dir() を呼ぶ (ここで .cross_review/ が作られる) + tmp_dir = _tmp_dir(worktree) + state_file = tmp_dir / f"cross-review-pr{pr}-state.json" + + # 既存コメントスナップショット（重複指摘防止）。 + # 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を + # fix skill の共有スクリプトで一括取得する。 + fetch_script = pathlib.Path(__file__).resolve().parent.parent.parent / "fix" / "scripts" / "fetch-pr-comments.sh" + r = subprocess.run( + [str(fetch_script), repo, str(pr)], + capture_output=True, text=True, + ) + existing_path = tmp_dir / f"cross-review-pr{pr}-existing-comments.txt" + if r.returncode == 0: + existing_path.write_text(r.stdout, encoding="utf-8") + else: + die(f"既存コメント取得失敗 (重複検出無効のため中断): {r.stderr.strip()[:200]}") + + return _InitWorkspaceContext(tmp_dir=tmp_dir, state_file=state_file) + + +def _prepare_initial_assignment(args: argparse.Namespace) -> _InitialAssignment: + """担当ホストを確定し、起動対象の認証を検査する。""" + # **ホストを先に確定する。** 誤ると母集合が狂い、ホストが自分自身をレビューする。 + # 推定できないときに既定を置かない（間違ったまま一周してしまう）。 + try: + host, host_source = assignment.detect_host(getattr(args, "host", None)) + except assignment.AssignmentError as e: + die(str(e)) + raise + reviewers = assignment.review_pool(host) + info(f"ホスト: {host}（{host_source}） / レビュワーの母集合: {' / '.join(reviewers)}") + _validate_only(args.only, host) + # 未認証の CLI は起動から短時間で終わり、結果を残さないまま担当から欠ける。 + # **確かめるのは実際に起動する担当だけである。** + auth.check_auth(_auth_targets(args.only, host), info=info, die=lambda m: die(m)) + return _InitialAssignment(host=host, host_source=host_source) + + +def _build_initial_review_state( + args: argparse.Namespace, + ctx: _InitialStateContext, +) -> dict[str, Any]: + """確定済みの材料から、副作用なしに初期状態を組み立てる。""" + host, host_source = ctx.assignment + return { + "started_at": _now(), + "host": host, + "host_source": host_source, + "max_rounds": args.max_rounds, + "rotate_after": args.rotate_after, + "only": args.only, + "current_pr": ctx.pr, + "worktree_path": ctx.pr_ctx.worktree, + "tmp_dir": str(ctx.ws_ctx.tmp_dir), + "repo": ctx.pr_ctx.repo, + "head_branch": ctx.pr_ctx.meta.head_branch, + "base_branch": ctx.pr_ctx.meta.base_branch, + "pr_author": ctx.pr_ctx.author, + "viewer_login": ctx.pr_ctx.me, + "is_own_pr": ctx.pr_ctx.is_own, + "event_downgrade": ctx.pr_ctx.event_downgrade, + "changed_files": ctx.review_ctx.changed_files, + "auto_review_categories": ctx.review_ctx.auto_review_categories, + "auto_review_instructions": ctx.review_ctx.auto_review, + "manual_extra_review_instructions": ctx.manual_extra_review, + "extra_review_instructions": ctx.manual_extra_review, + "review_instructions": ctx.review_ctx.review_instructions, + "pr_history": [{"pr": ctx.pr, "opened_at": _now(), "closed_at": None, "rounds": 0}], + "rounds": [], + "deferred_nits": [], + "rejected_findings": [], + "review_findings": [], + "evidence_rounds": [], + "verify_commands": list(getattr(args, "verify_command", None) or []), + "verify_exit_codes": list(getattr(args, "verify_exit_code", None) or []), + "carried_over": None, + "final": None, + } + + +def _save_and_print_initial_state( + args: argparse.Namespace, ctx: _InitialStateContext +) -> None: + """初期状態を保存し、init の結果を表示する。""" + state = _build_initial_review_state(args, ctx) + _write_state(ctx.ws_ctx.state_file, state) + info(f"✅ state 初期化: {ctx.ws_ctx.state_file}") + _print_init_result( + ctx.pr, + ctx.pr_ctx.worktree, + ctx.ws_ctx.tmp_dir, + ctx.pr_ctx.repo, + ctx.pr_ctx.meta.head_branch, + ctx.pr_ctx.meta.base_branch, + ctx.pr_ctx.is_own, + ctx.pr_ctx.event_downgrade, + bool(ctx.review_ctx.review_instructions), + 0, + False, + ) + + def _init_new_state( args: argparse.Namespace, pr: object, @@ -1696,182 +1868,6 @@ def _init_new_state( manual_extra_review: str, ) -> None: """新規 init 経路: プリチェック → worktree 作成 → state 構築 → 出力。""" - - def _resolve_pr_and_ownership( - pr: object, repo: str, worktree: str, args_worktree: str | None - ) -> _InitPRContext | None: - # 新規 init: プリチェック。 - # **作成者・head・base は REST の 1 回でまとめて取る。** 項目ごとに `gh pr view` を - # 投げていた分（GraphQL 3 点）と、リポジトリ名の解決（同 1 点）が 0 点になる。 - meta = _fetch_pr_metadata(pr, repo) - if meta is None: - die(f"PR #{pr} のメタデータを取得できません（リポジトリ名: {repo}）") - return None - if meta.repo != repo: - repo = meta.repo - if not args_worktree: - worktree = str(_default_worktree_base() / _repo_slug(repo) / f"pr{pr}") - if meta.rate_remaining is not None: - info(f"ℹ GitHub REST の残量: {meta.rate_remaining}") - - me = _sh(["gh", "api", "user", "--jq", ".login"]) - author = meta.author - is_own = (me == author) - event_downgrade = is_own - if is_own: - info(f"⚠ 自分の PR (author={me}) — REQUEST_CHANGES → COMMENT 強制ダウングレード") - - return _InitPRContext( - repo=repo, - worktree=worktree, - meta=meta, - me=me, - author=author, - is_own=is_own, - event_downgrade=event_downgrade, - ) - - def _prepare_review_instructions( - pr: object, repo: str, manual_extra_review: str - ) -> _InitReviewContext: - changed_files = _fetch_changed_files(pr, repo) - auto_review_categories = _classify_changed_files(changed_files) - auto_review = _auto_review_instructions(auto_review_categories) - review_instructions = _combined_review_instructions(auto_review, manual_extra_review) - return _InitReviewContext( - changed_files=changed_files, - auto_review_categories=auto_review_categories, - auto_review=auto_review, - review_instructions=review_instructions, - ) - - def _prepare_worktree_and_comments( - worktree: str, pr: object, head_branch: str, repo: str - ) -> _InitWorkspaceContext: - # worktree 分離 — _tmp_dir() より先に worktree を作成/確認する - if not pathlib.Path(worktree).exists(): - _create_worktree(worktree, pr, head_branch) - elif _is_registered_worktree(worktree): - info(f"↻ 既存 worktree 流用: {worktree}") - _sync_worktree(worktree, pr, head_branch) - else: - # パスは存在するが現リポジトリの worktree ではない (別リポジトリの残骸等)。 - # 流用すると git 操作が壊れるため退避して作り直す。 - stale = f"{worktree}.stale-{time.strftime('%Y%m%d%H%M%S')}" - pathlib.Path(worktree).rename(stale) - info(f"⚠ 現リポジトリの worktree でないため退避: {stale}") - _create_worktree(worktree, pr, head_branch) - - # worktree 作成/確認後に _tmp_dir() を呼ぶ (ここで .cross_review/ が作られる) - tmp_dir = _tmp_dir(worktree) - state_file = tmp_dir / f"cross-review-pr{pr}-state.json" - - # 既存コメントスナップショット（重複指摘防止）。 - # 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を - # fix skill の共有スクリプトで一括取得する。 - fetch_script = pathlib.Path(__file__).resolve().parent.parent.parent / "fix" / "scripts" / "fetch-pr-comments.sh" - r = subprocess.run( - [str(fetch_script), repo, str(pr)], - capture_output=True, text=True, - ) - existing_path = tmp_dir / f"cross-review-pr{pr}-existing-comments.txt" - if r.returncode == 0: - existing_path.write_text(r.stdout, encoding="utf-8") - else: - die(f"既存コメント取得失敗 (重複検出無効のため中断): {r.stderr.strip()[:200]}") - - return _InitWorkspaceContext( - tmp_dir=tmp_dir, - state_file=state_file, - ) - - def _prepare_initial_assignment(args: argparse.Namespace) -> _InitialAssignment: - """担当ホストを確定し、起動対象の認証を検査する。""" - # **ホストを先に確定する。** 誤ると母集合が狂い、ホストが自分自身をレビューする。 - # 推定できないときに既定を置かない（間違ったまま一周してしまう）。 - try: - host, host_source = assignment.detect_host(getattr(args, "host", None)) - except assignment.AssignmentError as e: - die(str(e)) - raise - reviewers = assignment.review_pool(host) - info(f"ホスト: {host}（{host_source}） / レビュワーの母集合: {' / '.join(reviewers)}") - _validate_only(args.only, host) - # 未認証の CLI は起動から短時間で終わり、結果を残さないまま担当から欠ける。 - # **確かめるのは実際に起動する担当だけである。** - auth.check_auth(_auth_targets(args.only, host), info=info, die=lambda m: die(m)) - return _InitialAssignment(host=host, host_source=host_source) - - def _build_initial_review_state( - args: argparse.Namespace, - ctx: _InitialStateContext, - ) -> dict[str, Any]: - """確定済みの材料から、副作用なしに初期状態を組み立てる。""" - host, host_source = ctx.assignment - return { - "started_at": _now(), - "host": host, - "host_source": host_source, - "max_rounds": args.max_rounds, - "rotate_after": args.rotate_after, - "only": args.only, - "current_pr": ctx.pr, - "worktree_path": ctx.pr_ctx.worktree, - "tmp_dir": str(ctx.ws_ctx.tmp_dir), - "repo": ctx.pr_ctx.repo, - "head_branch": ctx.pr_ctx.meta.head_branch, - "base_branch": ctx.pr_ctx.meta.base_branch, - "pr_author": ctx.pr_ctx.author, - "viewer_login": ctx.pr_ctx.me, - "is_own_pr": ctx.pr_ctx.is_own, - "event_downgrade": ctx.pr_ctx.event_downgrade, - "changed_files": ctx.review_ctx.changed_files, - "auto_review_categories": ctx.review_ctx.auto_review_categories, - "auto_review_instructions": ctx.review_ctx.auto_review, - "manual_extra_review_instructions": ctx.manual_extra_review, - "extra_review_instructions": ctx.manual_extra_review, - "review_instructions": ctx.review_ctx.review_instructions, - "pr_history": [{"pr": ctx.pr, "opened_at": _now(), "closed_at": None, "rounds": 0}], - "rounds": [], - "deferred_nits": [], - "rejected_findings": [], - "review_findings": [], - "evidence_rounds": [], - "verify_commands": list(getattr(args, "verify_command", None) or []), - "verify_exit_codes": list(getattr(args, "verify_exit_code", None) or []), - "carried_over": None, - "final": None, - } - - def _finalize_initial_state( - args: argparse.Namespace, - pr: object, - pr_ctx: _InitPRContext, - review_ctx: _InitReviewContext, - ws_ctx: _InitWorkspaceContext, - manual_extra_review: str, - ) -> None: - initial_assignment = _prepare_initial_assignment(args) - context = _InitialStateContext( - pr, pr_ctx, review_ctx, ws_ctx, initial_assignment, manual_extra_review - ) - state = _build_initial_review_state(args, context) - _write_state(ws_ctx.state_file, state) - info(f"✅ state 初期化: {ws_ctx.state_file}") - _print_init_result( - pr, - pr_ctx.worktree, - ws_ctx.tmp_dir, - pr_ctx.repo, - pr_ctx.meta.head_branch, - pr_ctx.meta.base_branch, - pr_ctx.is_own, - pr_ctx.event_downgrade, - bool(review_ctx.review_instructions), - 0, - False, - ) - pr_ctx = _resolve_pr_and_ownership(pr, repo, worktree, args.worktree) if pr_ctx is None: return @@ -1880,9 +1876,11 @@ def _finalize_initial_state( ws_ctx = _prepare_worktree_and_comments( pr_ctx.worktree, pr, pr_ctx.meta.head_branch, pr_ctx.repo ) - _finalize_initial_state( - args, pr, pr_ctx, review_ctx, ws_ctx, manual_extra_review + initial_assignment = _prepare_initial_assignment(args) + context = _InitialStateContext( + pr, pr_ctx, review_ctx, ws_ctx, initial_assignment, manual_extra_review ) + _save_and_print_initial_state(args, context) # **母集合を広げる前からある 2 者。** `host` を持たない状態ファイル（このリポジトリの diff --git a/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py b/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py index b8f91814..cef0efbf 100644 --- a/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py +++ b/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py @@ -6,8 +6,8 @@ `auth.check_auth` が二重に走り、機械可読ブロック（`PR=…RESUMED=0`）が標準出力へ 2 回出ていた。 -**経路そのものは `gh` を要するため実行では確かめない。** 関数の構造（同じ文が 2 回 -現れない・出力が 1 回だけ）を構文木で見る。 +**経路そのものは `gh` を要するため実行では確かめない。** オーケストレーションの構造 +（同じ文が 2 回現れない・抽出した各段階が 1 回だけ）を構文木で見る。 """ from __future__ import annotations @@ -19,12 +19,14 @@ STATE_PY = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "state.py" -# 1 回しか呼んではいけないもの。**副作用を持つ**か、標準出力の機械可読ブロックを書く。 +# `_init_new_state` が順に 1 回ずつ呼ぶ段階。各段階の内部にある副作用の重複は、 +# オーケストレーターから同じ段階を二重に呼ばないことで防ぐ。 SINGLE_CALL = ( - "_print_init_result", - "_fetch_pr_metadata", - "_fetch_changed_files", - "_tmp_dir", + "_resolve_pr_and_ownership", + "_prepare_review_instructions", + "_prepare_worktree_and_comments", + "_prepare_initial_assignment", + "_save_and_print_initial_state", ) From f682913d1312b1ca77e03701c5407b726725390e Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:37:56 +0000 Subject: [PATCH 20/21] =?UTF-8?q?Revert=20"Refactor:=20extract=5Fmethod=20?= =?UTF-8?q?=E2=80=94=20plugins/ndf/skills/cross-review/scripts/state.py#?= =?UTF-8?q?=5Finit=5Fnew=5Fstate"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 77d9383c32e6d9b7a90e6af5467a6c5cbdbb1932. --- .../ndf/skills/cross-review/scripts/state.py | 354 +++++++++--------- .../tests/test_init_body_not_duplicated.py | 16 +- 2 files changed, 185 insertions(+), 185 deletions(-) diff --git a/plugins/ndf/skills/cross-review/scripts/state.py b/plugins/ndf/skills/cross-review/scripts/state.py index 543070e4..fde27994 100755 --- a/plugins/ndf/skills/cross-review/scripts/state.py +++ b/plugins/ndf/skills/cross-review/scripts/state.py @@ -1688,178 +1688,6 @@ class _InitialStateContext(NamedTuple): manual_extra_review: str -def _resolve_pr_and_ownership( - pr: object, repo: str, worktree: str, args_worktree: str | None -) -> _InitPRContext | None: - """PR のメタデータとレビュー実行者との所有関係を解決する。""" - # **作成者・head・base は REST の 1 回でまとめて取る。** 項目ごとに `gh pr view` を - # 投げていた分（GraphQL 3 点）と、リポジトリ名の解決（同 1 点）が 0 点になる。 - meta = _fetch_pr_metadata(pr, repo) - if meta is None: - die(f"PR #{pr} のメタデータを取得できません（リポジトリ名: {repo}）") - return None - if meta.repo != repo: - repo = meta.repo - if not args_worktree: - worktree = str(_default_worktree_base() / _repo_slug(repo) / f"pr{pr}") - if meta.rate_remaining is not None: - info(f"ℹ GitHub REST の残量: {meta.rate_remaining}") - - me = _sh(["gh", "api", "user", "--jq", ".login"]) - author = meta.author - is_own = (me == author) - event_downgrade = is_own - if is_own: - info(f"⚠ 自分の PR (author={me}) — REQUEST_CHANGES → COMMENT 強制ダウングレード") - - return _InitPRContext( - repo=repo, - worktree=worktree, - meta=meta, - me=me, - author=author, - is_own=is_own, - event_downgrade=event_downgrade, - ) - - -def _prepare_review_instructions( - pr: object, repo: str, manual_extra_review: str -) -> _InitReviewContext: - """変更ファイルから自動・手動のレビュー条件を組み立てる。""" - changed_files = _fetch_changed_files(pr, repo) - auto_review_categories = _classify_changed_files(changed_files) - auto_review = _auto_review_instructions(auto_review_categories) - review_instructions = _combined_review_instructions(auto_review, manual_extra_review) - return _InitReviewContext( - changed_files=changed_files, - auto_review_categories=auto_review_categories, - auto_review=auto_review, - review_instructions=review_instructions, - ) - - -def _prepare_worktree_and_comments( - worktree: str, pr: object, head_branch: str, repo: str -) -> _InitWorkspaceContext: - """worktree と既存コメントのスナップショットを準備する。""" - # worktree 分離 — _tmp_dir() より先に worktree を作成/確認する - if not pathlib.Path(worktree).exists(): - _create_worktree(worktree, pr, head_branch) - elif _is_registered_worktree(worktree): - info(f"↻ 既存 worktree 流用: {worktree}") - _sync_worktree(worktree, pr, head_branch) - else: - # パスは存在するが現リポジトリの worktree ではない (別リポジトリの残骸等)。 - # 流用すると git 操作が壊れるため退避して作り直す。 - stale = f"{worktree}.stale-{time.strftime('%Y%m%d%H%M%S')}" - pathlib.Path(worktree).rename(stale) - info(f"⚠ 現リポジトリの worktree でないため退避: {stale}") - _create_worktree(worktree, pr, head_branch) - - # worktree 作成/確認後に _tmp_dir() を呼ぶ (ここで .cross_review/ が作られる) - tmp_dir = _tmp_dir(worktree) - state_file = tmp_dir / f"cross-review-pr{pr}-state.json" - - # 既存コメントスナップショット（重複指摘防止）。 - # 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を - # fix skill の共有スクリプトで一括取得する。 - fetch_script = pathlib.Path(__file__).resolve().parent.parent.parent / "fix" / "scripts" / "fetch-pr-comments.sh" - r = subprocess.run( - [str(fetch_script), repo, str(pr)], - capture_output=True, text=True, - ) - existing_path = tmp_dir / f"cross-review-pr{pr}-existing-comments.txt" - if r.returncode == 0: - existing_path.write_text(r.stdout, encoding="utf-8") - else: - die(f"既存コメント取得失敗 (重複検出無効のため中断): {r.stderr.strip()[:200]}") - - return _InitWorkspaceContext(tmp_dir=tmp_dir, state_file=state_file) - - -def _prepare_initial_assignment(args: argparse.Namespace) -> _InitialAssignment: - """担当ホストを確定し、起動対象の認証を検査する。""" - # **ホストを先に確定する。** 誤ると母集合が狂い、ホストが自分自身をレビューする。 - # 推定できないときに既定を置かない（間違ったまま一周してしまう）。 - try: - host, host_source = assignment.detect_host(getattr(args, "host", None)) - except assignment.AssignmentError as e: - die(str(e)) - raise - reviewers = assignment.review_pool(host) - info(f"ホスト: {host}（{host_source}） / レビュワーの母集合: {' / '.join(reviewers)}") - _validate_only(args.only, host) - # 未認証の CLI は起動から短時間で終わり、結果を残さないまま担当から欠ける。 - # **確かめるのは実際に起動する担当だけである。** - auth.check_auth(_auth_targets(args.only, host), info=info, die=lambda m: die(m)) - return _InitialAssignment(host=host, host_source=host_source) - - -def _build_initial_review_state( - args: argparse.Namespace, - ctx: _InitialStateContext, -) -> dict[str, Any]: - """確定済みの材料から、副作用なしに初期状態を組み立てる。""" - host, host_source = ctx.assignment - return { - "started_at": _now(), - "host": host, - "host_source": host_source, - "max_rounds": args.max_rounds, - "rotate_after": args.rotate_after, - "only": args.only, - "current_pr": ctx.pr, - "worktree_path": ctx.pr_ctx.worktree, - "tmp_dir": str(ctx.ws_ctx.tmp_dir), - "repo": ctx.pr_ctx.repo, - "head_branch": ctx.pr_ctx.meta.head_branch, - "base_branch": ctx.pr_ctx.meta.base_branch, - "pr_author": ctx.pr_ctx.author, - "viewer_login": ctx.pr_ctx.me, - "is_own_pr": ctx.pr_ctx.is_own, - "event_downgrade": ctx.pr_ctx.event_downgrade, - "changed_files": ctx.review_ctx.changed_files, - "auto_review_categories": ctx.review_ctx.auto_review_categories, - "auto_review_instructions": ctx.review_ctx.auto_review, - "manual_extra_review_instructions": ctx.manual_extra_review, - "extra_review_instructions": ctx.manual_extra_review, - "review_instructions": ctx.review_ctx.review_instructions, - "pr_history": [{"pr": ctx.pr, "opened_at": _now(), "closed_at": None, "rounds": 0}], - "rounds": [], - "deferred_nits": [], - "rejected_findings": [], - "review_findings": [], - "evidence_rounds": [], - "verify_commands": list(getattr(args, "verify_command", None) or []), - "verify_exit_codes": list(getattr(args, "verify_exit_code", None) or []), - "carried_over": None, - "final": None, - } - - -def _save_and_print_initial_state( - args: argparse.Namespace, ctx: _InitialStateContext -) -> None: - """初期状態を保存し、init の結果を表示する。""" - state = _build_initial_review_state(args, ctx) - _write_state(ctx.ws_ctx.state_file, state) - info(f"✅ state 初期化: {ctx.ws_ctx.state_file}") - _print_init_result( - ctx.pr, - ctx.pr_ctx.worktree, - ctx.ws_ctx.tmp_dir, - ctx.pr_ctx.repo, - ctx.pr_ctx.meta.head_branch, - ctx.pr_ctx.meta.base_branch, - ctx.pr_ctx.is_own, - ctx.pr_ctx.event_downgrade, - bool(ctx.review_ctx.review_instructions), - 0, - False, - ) - - def _init_new_state( args: argparse.Namespace, pr: object, @@ -1868,6 +1696,182 @@ def _init_new_state( manual_extra_review: str, ) -> None: """新規 init 経路: プリチェック → worktree 作成 → state 構築 → 出力。""" + + def _resolve_pr_and_ownership( + pr: object, repo: str, worktree: str, args_worktree: str | None + ) -> _InitPRContext | None: + # 新規 init: プリチェック。 + # **作成者・head・base は REST の 1 回でまとめて取る。** 項目ごとに `gh pr view` を + # 投げていた分（GraphQL 3 点）と、リポジトリ名の解決（同 1 点）が 0 点になる。 + meta = _fetch_pr_metadata(pr, repo) + if meta is None: + die(f"PR #{pr} のメタデータを取得できません（リポジトリ名: {repo}）") + return None + if meta.repo != repo: + repo = meta.repo + if not args_worktree: + worktree = str(_default_worktree_base() / _repo_slug(repo) / f"pr{pr}") + if meta.rate_remaining is not None: + info(f"ℹ GitHub REST の残量: {meta.rate_remaining}") + + me = _sh(["gh", "api", "user", "--jq", ".login"]) + author = meta.author + is_own = (me == author) + event_downgrade = is_own + if is_own: + info(f"⚠ 自分の PR (author={me}) — REQUEST_CHANGES → COMMENT 強制ダウングレード") + + return _InitPRContext( + repo=repo, + worktree=worktree, + meta=meta, + me=me, + author=author, + is_own=is_own, + event_downgrade=event_downgrade, + ) + + def _prepare_review_instructions( + pr: object, repo: str, manual_extra_review: str + ) -> _InitReviewContext: + changed_files = _fetch_changed_files(pr, repo) + auto_review_categories = _classify_changed_files(changed_files) + auto_review = _auto_review_instructions(auto_review_categories) + review_instructions = _combined_review_instructions(auto_review, manual_extra_review) + return _InitReviewContext( + changed_files=changed_files, + auto_review_categories=auto_review_categories, + auto_review=auto_review, + review_instructions=review_instructions, + ) + + def _prepare_worktree_and_comments( + worktree: str, pr: object, head_branch: str, repo: str + ) -> _InitWorkspaceContext: + # worktree 分離 — _tmp_dir() より先に worktree を作成/確認する + if not pathlib.Path(worktree).exists(): + _create_worktree(worktree, pr, head_branch) + elif _is_registered_worktree(worktree): + info(f"↻ 既存 worktree 流用: {worktree}") + _sync_worktree(worktree, pr, head_branch) + else: + # パスは存在するが現リポジトリの worktree ではない (別リポジトリの残骸等)。 + # 流用すると git 操作が壊れるため退避して作り直す。 + stale = f"{worktree}.stale-{time.strftime('%Y%m%d%H%M%S')}" + pathlib.Path(worktree).rename(stale) + info(f"⚠ 現リポジトリの worktree でないため退避: {stale}") + _create_worktree(worktree, pr, head_branch) + + # worktree 作成/確認後に _tmp_dir() を呼ぶ (ここで .cross_review/ が作られる) + tmp_dir = _tmp_dir(worktree) + state_file = tmp_dir / f"cross-review-pr{pr}-state.json" + + # 既存コメントスナップショット（重複指摘防止）。 + # 3 ソース (インラインコメント / レビュー body / PR レベルコメント) を + # fix skill の共有スクリプトで一括取得する。 + fetch_script = pathlib.Path(__file__).resolve().parent.parent.parent / "fix" / "scripts" / "fetch-pr-comments.sh" + r = subprocess.run( + [str(fetch_script), repo, str(pr)], + capture_output=True, text=True, + ) + existing_path = tmp_dir / f"cross-review-pr{pr}-existing-comments.txt" + if r.returncode == 0: + existing_path.write_text(r.stdout, encoding="utf-8") + else: + die(f"既存コメント取得失敗 (重複検出無効のため中断): {r.stderr.strip()[:200]}") + + return _InitWorkspaceContext( + tmp_dir=tmp_dir, + state_file=state_file, + ) + + def _prepare_initial_assignment(args: argparse.Namespace) -> _InitialAssignment: + """担当ホストを確定し、起動対象の認証を検査する。""" + # **ホストを先に確定する。** 誤ると母集合が狂い、ホストが自分自身をレビューする。 + # 推定できないときに既定を置かない（間違ったまま一周してしまう）。 + try: + host, host_source = assignment.detect_host(getattr(args, "host", None)) + except assignment.AssignmentError as e: + die(str(e)) + raise + reviewers = assignment.review_pool(host) + info(f"ホスト: {host}（{host_source}） / レビュワーの母集合: {' / '.join(reviewers)}") + _validate_only(args.only, host) + # 未認証の CLI は起動から短時間で終わり、結果を残さないまま担当から欠ける。 + # **確かめるのは実際に起動する担当だけである。** + auth.check_auth(_auth_targets(args.only, host), info=info, die=lambda m: die(m)) + return _InitialAssignment(host=host, host_source=host_source) + + def _build_initial_review_state( + args: argparse.Namespace, + ctx: _InitialStateContext, + ) -> dict[str, Any]: + """確定済みの材料から、副作用なしに初期状態を組み立てる。""" + host, host_source = ctx.assignment + return { + "started_at": _now(), + "host": host, + "host_source": host_source, + "max_rounds": args.max_rounds, + "rotate_after": args.rotate_after, + "only": args.only, + "current_pr": ctx.pr, + "worktree_path": ctx.pr_ctx.worktree, + "tmp_dir": str(ctx.ws_ctx.tmp_dir), + "repo": ctx.pr_ctx.repo, + "head_branch": ctx.pr_ctx.meta.head_branch, + "base_branch": ctx.pr_ctx.meta.base_branch, + "pr_author": ctx.pr_ctx.author, + "viewer_login": ctx.pr_ctx.me, + "is_own_pr": ctx.pr_ctx.is_own, + "event_downgrade": ctx.pr_ctx.event_downgrade, + "changed_files": ctx.review_ctx.changed_files, + "auto_review_categories": ctx.review_ctx.auto_review_categories, + "auto_review_instructions": ctx.review_ctx.auto_review, + "manual_extra_review_instructions": ctx.manual_extra_review, + "extra_review_instructions": ctx.manual_extra_review, + "review_instructions": ctx.review_ctx.review_instructions, + "pr_history": [{"pr": ctx.pr, "opened_at": _now(), "closed_at": None, "rounds": 0}], + "rounds": [], + "deferred_nits": [], + "rejected_findings": [], + "review_findings": [], + "evidence_rounds": [], + "verify_commands": list(getattr(args, "verify_command", None) or []), + "verify_exit_codes": list(getattr(args, "verify_exit_code", None) or []), + "carried_over": None, + "final": None, + } + + def _finalize_initial_state( + args: argparse.Namespace, + pr: object, + pr_ctx: _InitPRContext, + review_ctx: _InitReviewContext, + ws_ctx: _InitWorkspaceContext, + manual_extra_review: str, + ) -> None: + initial_assignment = _prepare_initial_assignment(args) + context = _InitialStateContext( + pr, pr_ctx, review_ctx, ws_ctx, initial_assignment, manual_extra_review + ) + state = _build_initial_review_state(args, context) + _write_state(ws_ctx.state_file, state) + info(f"✅ state 初期化: {ws_ctx.state_file}") + _print_init_result( + pr, + pr_ctx.worktree, + ws_ctx.tmp_dir, + pr_ctx.repo, + pr_ctx.meta.head_branch, + pr_ctx.meta.base_branch, + pr_ctx.is_own, + pr_ctx.event_downgrade, + bool(review_ctx.review_instructions), + 0, + False, + ) + pr_ctx = _resolve_pr_and_ownership(pr, repo, worktree, args.worktree) if pr_ctx is None: return @@ -1876,11 +1880,9 @@ def _init_new_state( ws_ctx = _prepare_worktree_and_comments( pr_ctx.worktree, pr, pr_ctx.meta.head_branch, pr_ctx.repo ) - initial_assignment = _prepare_initial_assignment(args) - context = _InitialStateContext( - pr, pr_ctx, review_ctx, ws_ctx, initial_assignment, manual_extra_review + _finalize_initial_state( + args, pr, pr_ctx, review_ctx, ws_ctx, manual_extra_review ) - _save_and_print_initial_state(args, context) # **母集合を広げる前からある 2 者。** `host` を持たない状態ファイル（このリポジトリの diff --git a/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py b/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py index cef0efbf..b8f91814 100644 --- a/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py +++ b/plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py @@ -6,8 +6,8 @@ `auth.check_auth` が二重に走り、機械可読ブロック（`PR=…RESUMED=0`）が標準出力へ 2 回出ていた。 -**経路そのものは `gh` を要するため実行では確かめない。** オーケストレーションの構造 -（同じ文が 2 回現れない・抽出した各段階が 1 回だけ）を構文木で見る。 +**経路そのものは `gh` を要するため実行では確かめない。** 関数の構造（同じ文が 2 回 +現れない・出力が 1 回だけ）を構文木で見る。 """ from __future__ import annotations @@ -19,14 +19,12 @@ STATE_PY = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "state.py" -# `_init_new_state` が順に 1 回ずつ呼ぶ段階。各段階の内部にある副作用の重複は、 -# オーケストレーターから同じ段階を二重に呼ばないことで防ぐ。 +# 1 回しか呼んではいけないもの。**副作用を持つ**か、標準出力の機械可読ブロックを書く。 SINGLE_CALL = ( - "_resolve_pr_and_ownership", - "_prepare_review_instructions", - "_prepare_worktree_and_comments", - "_prepare_initial_assignment", - "_save_and_print_initial_state", + "_print_init_result", + "_fetch_pr_metadata", + "_fetch_changed_files", + "_tmp_dir", ) From cef8f3655713df49a90547bfc48793f0b54761d5 Mon Sep 17 00:00:00 2001 From: "takemi.ohama" Date: Sat, 19 Sep 2026 21:37:56 +0000 Subject: [PATCH 21/21] =?UTF-8?q?Docs:=20=E6=94=B9=E4=BF=AE=E8=A8=88?= =?UTF-8?q?=E7=94=BB=E3=82=92=E8=A8=98=E9=8C=B2=E3=81=99=E3=82=8B=EF=BC=88?= =?UTF-8?q?cross-refactoring=20=E9=80=B2=E8=A1=8C=E5=81=B4=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit なぜ直すのか（理由）とどう直すのか（手順）は提案の時点でしか残らない。 状態ファイルは差分から除外されるため、Pull Request から読める場所へ置く。 --- issues/refactoring-plan-rf790.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/issues/refactoring-plan-rf790.md b/issues/refactoring-plan-rf790.md index d80c1bb5..3ebd316d 100644 --- a/issues/refactoring-plan-rf790.md +++ b/issues/refactoring-plan-rf790.md @@ -223,7 +223,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| long_method | split_into_pipeline | major | codex | 検証中 | 1 | +| long_method | split_into_pipeline | major | codex | 採用 | 1 | **なぜ**: レビュワーごとのファイル解決、JSON 読み込み、payload と comments の境界検証、path・line・本文の正規化が1つの二重ループに入り、入力境界の失敗とキー変換の責務が分離されていない。 @@ -236,7 +236,7 @@ | 兆候・経路 | 手法・階層 | 重要度 | 提案元 | 状態 | コミット | | --- | --- | --- | --- | --- | ---: | -| long_method | extract_method | major | codex | 未着手 | 0 | +| long_method | extract_method | major | codex | 取り消し | 1 | **なぜ**: 新規初期化の1関数内に PR 所有権解決、レビュー条件作成、worktree と既存コメントの準備、担当認証、初期 state 構築、保存と表示がネスト関数として同居し、各段階を単独で参照・テストできない。 @@ -283,3 +283,4 @@ | 3 | `plugins/ndf/skills/cross-review/scripts/state.py#_apply_classification` | long_method | 1 ラウンドの採用上限 5 件を超えた | | 4 | `plugins/ndf/skills/cross-review/scripts/state.py#_finding_keys` | duplication | 1 ラウンドの採用上限 5 件を超えた | | 4 | `plugins/ndf/skills/cross-review/scripts/state.py#_print_init_result` | long_parameter_list | 1 ラウンドの採用上限 5 件を超えた | +| 4 | `plugins/ndf/skills/cross-review/scripts/state.py#_init_new_state` | long_method | テストの期待する振る舞いが変わっています（plugins/ndf/skills/cross-review/tests/test_init_body_not_duplicated.py）。構造改善では期待出力を変えません。振る舞いの変更は別の変更に分けてください |