diff --git a/issues/issue-113-cross-refactoring-retrial.md b/issues/issue-113-cross-refactoring-retrial.md new file mode 100644 index 00000000..e1afe2fa --- /dev/null +++ b/issues/issue-113-cross-refactoring-retrial.md @@ -0,0 +1,187 @@ +# cross-refactoring 修正後の再検証 + +不具合 9 件の修正（[issue-113-cross-refactoring-defect-fixes.md](issue-113-cross-refactoring-defect-fixes.md)、 +NDF v8.2.0）を実機で確かめる。**この Pull Request がその対象**である。 + +前回（PR #118）は適用結果の検証で失敗した項目を取り消す経路が破綻し、 +レビューフェーズより先へ一度も進めなかった。今回の目的は**その先を通すこと**にある。 + +## 実行条件 + +| 項目 | 値 | +| --- | --- | +| ホスト | Claude Code（提案・レビューには不参加） | +| 提案・レビュー | codex / gemini / kiro | +| 適用の母集合 | claude / codex / kiro | +| 使用する版 | リポジトリ内の `plugins/ndf-claude`（v8.2.0）。プラグインキャッシュ（8.1.0）は使わない | +| ラウンド上限 | 3 | + +```bash +PLUGIN_ROOT=/work/ai-plugins/plugins/ndf-claude + +/ndf:cross-refactoring <この PR 番号> \ + --scope plugins/ndf-shared/skills/cross-refactoring/scripts \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/scripts/lib \ + --baseline-test "uv run --with pytest python -m pytest \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/tests -q" +``` + +`--scope` に**テストの置き場所を含める**。範囲は適用結果の検証にも効くようになったため、 +含めないと `test_gap` が真の項目で「テストを先に足せ」と「範囲外を触るな」が両立しない。 + +## 確かめること + +修正した 9 件が実機で成立するか。 + +| # | 直したこと | 何が観測できれば通ったと言えるか | +| --- | --- | --- | +| 1 | 取り消しの巻き戻しと積み直し | 検証に失敗した項目だけが消え、合意済みの項目が残る。分離できない位置関係なら `rounds[].drops[].mode = round` が記録される | +| 2 | 中断と全件失敗の区別 | 取り消しに失敗したら終了コード 4 で進行が止まる（次の提案ラウンドへ進まない） | +| 3 | 判定の逐次記録 | `rounds[].apply_progress` に項目ごとの判定が残る | +| 4 | 再送信の印 | 取り消しが Pull Request へ反映される。`pending_push` が残らない | +| 5 | 範囲の検査 | 配布物 3 系統が実装担当の差分に現れない | +| 6 | 提案結果のラウンド別保存 | `<ランタイム>-propose-rf-r<ラウンド>-result.json` が巡ごとに残る | +| 7 | gemini の読み取り | gemini の提案が語彙内で返る（`read_file` が拒否されない） | +| 8 | 語彙の列挙 | 3 ランタイムとも `smell` / `technique` が英字の識別子で返る | +| 9 | 認証の確認 | 初期化時に 4 CLI の認証状態が出力される | + +## 前回まったく実行できていない範囲 + +ここを通すことが今回の主目的である。 + +- レビュー担当 2 者の並列実行と指摘の投稿、承認判定 +- 指摘の修正と再レビューの繰り返し、上限到達時の項目単位の見送り +- 実装担当の輪番 +- 提案の重複率による収束判定 +- 集計値（`report --metrics`）の出力 + +## 結果 + +**修正した 9 件はすべて実機で成立した。** 前回は適用結果の検証で失敗した項目を取り消す +経路が破綻して停止したが、今回はレビュー・判定・輪番・集計まで到達した。 +一方で**新しい不具合を 2 件見つけ**、うち 1 件は収束ループを実質的に不能にする。 + +## 到達点 + +```mermaid +flowchart TD + Init([初期化・作業ディレクトリ・手順書配置]):::ok --> P1 + P1["R1 提案（3 CLI 並列）11 件"]:::ok --> M1 + M1["統合 7 件 → 採用 5 件"]:::ok --> A1 + A1["R1 適用（codex）11 コミット"]:::ok --> V1 + V1["検証: 採用 4 / 失敗 1"]:::ok --> D1 + D1["取り消し 11 → 積み直し 8
（項目単位）"]:::ok --> RV1 + RV1["R1 レビュー（gemini + kiro）
両者 APPROVE"]:::ok --> P2 + P2["R2 提案 → 採用 5 件"]:::ok --> A2 + A2["R2 適用（kiro）12 コミット"]:::ok --> V2 + V2["検証: 全 5 件が範囲外で失敗"]:::ng --> D2 + D2["取り消し 12 → 積み直し 0
ラウンドごと失敗"]:::ok --> Stop + Stop["不具合 10 のため中止"]:::ng + + classDef ok fill:#dfd,stroke:#383 + classDef ng fill:#fdd,stroke:#933 +``` + +前回の到達点は `V1` の直前までだった。 + +## 修正 9 件の実機確認 + +| # | 直したこと | 実機での観測 | +| --- | --- | --- | +| 1 | 取り消しの巻き戻しと積み直し | `取り消し 11 コミット / 積み直し 8 コミット（項目単位）`。失敗した 1 項目だけを落とし、**同じ `refactor.py` を触る 3 項目 6 コミットを積み直した** | +| 2 | 中断と全件失敗の区別 | push 失敗で終了コード 4。次の提案ラウンドへ進まず停止した | +| 3 | 判定の逐次記録 | `rounds[].apply_progress` に全項目の判定が残った | +| 4 | 再送信の印 | 中断を跨いで `pending_push` が残り、次の実行が**処理済み判定より先に** push を再送した | +| 5 | 範囲の検査 | 範囲外コミットを検出して項目を失敗にした（R1-004 / R2 全件） | +| 6 | 提案結果のラウンド別保存 | `*-propose-rf120-r1-result.json` と `-r2-` が**両方残った** | +| 7 | gemini の手順書読み取り | `.gemini/settings.json` が配置され、差分にも出なかった | +| 8 | 語彙の列挙 | 3 ランタイムとも `smell` / `technique` を英字の識別子で返した | +| 9 | 認証の確認 | 初期化時に 4 CLI の認証状態を出力した | + +不具合 1 は、**前回破綻したのと同じ条件**（採用項目の多くが同一ファイルを触る）で成立した。 + +## 前回まったく実行できていなかった範囲 + +| 対象 | 結果 | +| --- | --- | +| レビュー担当 2 者の並列実行と承認判定 | ✅ gemini / kiro を並列起動し、両者 `APPROVE` で `judge-review` が終了コード 0 | +| 実装担当の輪番 | ✅ R1 codex → R2 kiro。レビュー担当も gemini+kiro → codex+gemini へ入れ替わった | +| 提案ラウンドの収束判定 | ✅ `advance` が継続を判定 | +| 集計値の出力 | ✅ `report --metrics` が実装担当・レビュー担当の表と、`auto` モデルの分離注記まで出した | +| 指摘の修正と再レビューの繰り返し | ❌ **未到達**。R1 は指摘 0 件、R2 は適用が全件失敗したため | +| 上限到達時の項目単位の見送り | ❌ **未到達**（同上） | + +## 新しく見つかった不具合 + +### 10. pre-push の同期検査と範囲ルールが両立しない + +このリポジトリは `.githooks/pre-push` で生成物の同期を検査する。一方この Skill は +「実装担当は編集元だけを触る / 生成物の同期は**進行側が収束後に**まとめて行う」と決めた。 +この 2 つは両立しない。 + +```console +$ git push origin refactor/cross-refactoring-retrial +Generated directory is out of date: plugins/ndf-claude/skills +exited 1 +``` + +**実装担当の push も、取り消しを反映する進行側の push も落ちる。** 結果、検証を通って +いない R1-004 の変更が Pull Request に残った（取り消しはローカルのみ）。 +**不具合 4 が防ごうとした状態そのもの**である。 + +さらに悪いことに、**この検査は実装担当を範囲ルール違反へ誘導する。** ラウンド 2 で +kiro は push を通すために配布物 3 系統を同期し、その結果**採用 5 件が全件範囲外で失敗** +して 1 ラウンドを丸ごと失った。 + +| ラウンド | 実装担当 | 配布物を同期したか | 適用成功 | +| --- | --- | --- | ---: | +| 1 | codex | しなかった（push は失敗のまま放置） | 4 / 5 | +| 2 | kiro | した（4 コミットすべてで） | 0 / 5 | + +「収束後にまとめて同期する」という決定が、**ループの途中で push が起きること**を +見落としていた。 + +**修正の方向**: 実装担当に push させず、**検証を通った後で進行側だけが push する**。 +同期はその直前に行う（`--sync-command` を新設し、状態ファイルへ保持する）。 +これで「未検証の変更が公開される」経路自体が無くなり、不具合 4 は緩和ではなく根絶できる。 + +本再検証では、進行側が生成物を同期するコミットを積んで回避した。 + +### 11. 適用で失敗した項目が「対象外」に入らない + +手順書は「見送った項目は理由付きで記録し、次ラウンドの提案時に**対象外として渡す**。 +そうしないと同じ提案が毎ラウンド出続けて収束しない」と定めている。しかし +`merge-apply` の失敗経路だけがこの記録を行わない（`deferred_items` に入らない）。 + +実測では、ラウンド 1 で失敗した `monitor.py#monitor_agent`（R1-004）が +**3 ランタイム全員から再提案され、合意 3 で最優先に採用された**（R2-001）。 +同じ理由で必ず失敗するため、ラウンドを 1 つ丸ごと消費する。 + +**修正の方向**: `merge-apply` が失敗させた項目も `deferred_items` へ理由付きで記録する。 + +## 運用上の注意 + +`--scope` には、範囲内の各ソースに対応する**テストの置き場所をすべて**含める。 +今回は `cross-review/scripts/lib` を範囲に入れながら `cross-review/tests` を入れ忘れた +ため、`monitor.py` の現状固定テストが範囲外となり R1-004 が失敗した。 +**検査は正しく働いている**（指定の誤り）。 + +## この Pull Request に残ったもの + +ラウンド 1 で採用・検証・レビュー承認まで通った 4 項目が残る。 + +| ID | 対象 | 手法 | +| --- | --- | --- | +| R1-001 | `refactor.py#cmd_merge_apply` | `extract_method` | +| R1-002 | `metrics.py#aggregate` | `extract_method` | +| R1-003 | `refactor.py#cmd_merge_fix` | `extract_method` | +| R1-005 | `refactor.py#cmd_merge_apply` | `consolidate_duplication` | + +ラウンド 2 の 12 コミットは全件取り消し済み。テストは 430 件成功し、生成物も同期済みである。 + +## 次にすること + +不具合 10 と 11 を直してから、**指摘の修正と再レビューの繰り返し**を通す再々検証を行う。 +不具合 10 を直さない限り、生成物を持つリポジトリでは収束ループが成立しない。 diff --git a/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py index 3ad3134b..f4093f23 100755 --- a/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py @@ -1078,6 +1078,131 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) +def _resolve_reported_to_full(work: str, reported_shas: list[str]) -> set[str]: + """申告 SHA のうち実在するコミットを完全 SHA に正規化する。""" + return { + full for full in ( + _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) + for sha in reported_shas + ) if full + } + + +def _revert_range_with_recovery( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + ordered_range: list[str], + base_sha_key: str, + dry_run: bool = False, +) -> None: + """範囲を取り消し、中断復帰用の push 印と新しい起点を保存する。""" + if base_sha_key == "apply_base_sha": + item_id = f"R{entry['round']}-range" + else: + item_id = f"R{entry['round']}-fix{entry['fix_rounds'] + 1}" + if not dry_run: + entry["pending_push"] = True + statefile.save(path, state) + _revert_item_commits( + state, {"item_id": item_id, "commits": list(ordered_range)}, dry_run=dry_run, + ) + if not dry_run: + entry[base_sha_key] = _git_out(work, ["rev-parse", "HEAD"]) + statefile.save(path, state) + + +def _validate_commit_ownership( + work: str, + ordered_range: list[str], + payload: dict[str, Any], + round_items: set[str], +) -> tuple[ + dict[str, dict[str, Any]], dict[str, str], list[str], list[str], list[str] +]: + """申告されたコミットがラウンド内で一意に所有されているか調べる。""" + reported: dict[str, dict[str, Any]] = {} + unknown_ids: list[str] = [] + raw_items = payload.get("items") + if not isinstance(raw_items, list): + info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") + raw_items = [] + for reported_item in raw_items: + if not isinstance(reported_item, dict): + continue + item_id = reported_item.get("item_id") + if item_id in round_items: + reported[item_id] = reported_item + elif item_id is not None: + unknown_ids.append(str(item_id)) + + owner_of: dict[str, str] = {} + duplicated: list[str] = [] + for item_id, reported_item in reported.items(): + for full in _resolve_reported_to_full(work, _reported_shas(reported_item)): + if full in owner_of and owner_of[full] != item_id: + duplicated.append(full) + owner_of.setdefault(full, item_id) + + unassigned = sorted(set(ordered_range) - set(owner_of)) + return reported, owner_of, unassigned, unknown_ids, duplicated + + +def _verify_and_record_items( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + reported: dict[str, dict[str, Any]], + in_range: set[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, + dry_run: bool, +) -> tuple[list[str], list[str]]: + """適用項目を検証し、項目と進捗に判定を記録する。""" + applied: list[str] = [] + failed: list[str] = [] + progress: list[dict[str, Any]] = [] + entry["apply_progress"] = progress + for item_id in entry["items"]: + item = _find_item(state, item_id) + got = reported.get(item_id) + if got is None: + problem = "適用結果に項目がありません" + facts: list[dict[str, Any]] = [] + else: + facts = collect_commit_facts( + work, _reported_shas(got), in_range, test_command, head_branch, timeout, + ) + problem = verify_apply_item(item, facts, scope) + if problem: + item["status"] = "abandoned" + item["failure_reason"] = problem + item["test_failed"] = bool(got and "テストが成功していません" in problem) + item["budget_exceeded"] = bool(got and "差分予算" in problem) + item["out_of_scope"] = bool(got and "対象範囲の外" in problem) + item["commits"] = _reported_shas(got) + failed.append(item_id) + info(f"❌ {item_id}: {problem}") + else: + item["status"] = "reviewing" + item["commits"] = _reported_shas(got) + item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) + applied.append(item_id) + info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") + progress.append({ + "item_id": item_id, "at": statefile.now(), + "result": "failed" if problem else "ok", + "reason": problem, "commits": list(item.get("commits") or []), + }) + if not dry_run: + statefile.save(path, state) + return applied, failed + + def cmd_merge_apply(args: argparse.Namespace) -> None: """Step 4 — 適用結果を検証して取り込む。 @@ -1152,21 +1277,6 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # コミットを数に入れると、割り当て済みに見えるのに項目別の検証にも入らず、 # そのまま Pull Request に残せてしまう。 round_items = set(entry["items"]) - reported: dict[str, dict[str, Any]] = {} - unknown_ids: list[str] = [] - raw_items = payload.get("items") - if not isinstance(raw_items, list): - info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") - raw_items = [] - for r in raw_items: - if not isinstance(r, dict): - continue - item_id = r.get("item_id") - if item_id in round_items: - reported[item_id] = r - elif item_id is not None: - unknown_ids.append(str(item_id)) - # **範囲のコミットは全て、いずれかの改善項目に割り当てられていること。** # 申告から漏れたコミットはテストもトレーラーも差分予算も検査されず、そのまま # Pull Request に残る。都合の悪い変更を申告しないだけで検査を回避できてしまう。 @@ -1176,18 +1286,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # # 判定は**完全な SHA へ正規化してから**行う。申告の文字列をそのまま鍵にすると、 # 一方が完全 SHA、他方が短縮 SHA で同じコミットを指したときに重複を見逃す。 - owner_of: dict[str, str] = {} - duplicated: list[str] = [] - for item_id, r in reported.items(): - for sha in _reported_shas(r): - full = _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) - if full is None: - continue # 実在しない申告は項目ごとの検証で落ちる - if full in owner_of and owner_of[full] != item_id: - duplicated.append(full) - owner_of.setdefault(full, item_id) - - unassigned = sorted(in_range - set(owner_of)) + reported, owner_of, unassigned, unknown_ids, duplicated = ( + _validate_commit_ownership(work, ordered_range, payload, round_items) + ) if unassigned or unknown_ids or duplicated: causes = [] if unassigned: @@ -1218,20 +1319,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 範囲全体を取り消す。どのコミットが安全かを決められない以上、 # 起点まで戻すのが最も確実である。順序は `_revert_item_commits` が # git の履歴から決め直す。 - whole_round = { - "item_id": f"R{entry['round']}-range", - "commits": list(ordered_range), - } - if not args.dry_run: - # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push - # できずに終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits(state, whole_round, args.dry_run) - if not args.dry_run: - # 取り消し後の状態を新しい起点にする。叩き直しても範囲が空になり、 - # 取り消しコミット自体を「未割当」として再び戻すことがない。 - entry["apply_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "apply_base_sha", args.dry_run, + ) entry["apply"] = { "applied": [], "failed": list(entry["items"]), "base_sha": entry.get("apply_base_sha"), "head_sha": head_sha, @@ -1250,51 +1340,15 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: statefile.save(path, state) sys.exit(2) - applied: list[str] = [] - failed: list[str] = [] scope = state.get("target_scope") or [] # **判定はその都度残す。** まとめて最後に保存すると、取り消しの途中で中断した # ときに適用の記録が一切残らず、どのコミットが検証を通ったのかを状態から # 復元できなくなる。再開可能性は収束ループの前提なので、ここが崩れると # 中断からの復帰手段が無くなる。 - progress: list[dict[str, Any]] = [] - entry["apply_progress"] = progress - for item_id in entry["items"]: - item = _find_item(state, item_id) - got = reported.get(item_id) - if got is None: - problem = "適用結果に項目がありません" - facts: list[dict[str, Any]] = [] - else: - facts = collect_commit_facts( - work, _reported_shas(got), in_range, test_command, head_branch, - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - problem = verify_apply_item(item, facts, scope) - if problem: - item["status"] = "abandoned" - item["failure_reason"] = problem - item["test_failed"] = bool(got and "テストが成功していません" in problem) - item["budget_exceeded"] = bool(got and "差分予算" in problem) - item["out_of_scope"] = bool(got and "対象範囲の外" in problem) - # 取り消しは全項目の判定が出そろってから**まとめて**行う。項目ごとに - # その場で戻すと、まだ判定していない項目のコミットと競合する。 - item["commits"] = _reported_shas(got) - failed.append(item_id) - info(f"❌ {item_id}: {problem}") - else: - item["status"] = "reviewing" - item["commits"] = _reported_shas(got) - item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) - applied.append(item_id) - info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") - progress.append({ - "item_id": item_id, "at": statefile.now(), - "result": "failed" if problem else "ok", - "reason": problem, "commits": list(item.get("commits") or []), - }) - if not args.dry_run: - statefile.save(path, state) + applied, failed = _verify_and_record_items( + path, state, entry, work, reported, in_range, scope, test_command, head_branch, + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), args.dry_run, + ) entry["apply"] = { "applied": applied, @@ -1626,6 +1680,34 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: statefile.save(path, state) +def _validate_fix_commits( + work: str, + ordered_range: list[str], + reported_shas: list[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, +) -> tuple[list[tuple[str, str]], list[str], list[str]]: + """修正コミットの申告漏れと手順適合を検証する。""" + reported_full = _resolve_reported_to_full(work, reported_shas) + unassigned = sorted(set(ordered_range) - reported_full) + facts = collect_commit_facts( + work, reported_shas, set(ordered_range), test_command, head_branch, timeout, + ) + accepted: list[tuple[str, str]] = [] + problems: list[str] = [] + for commit in facts: + item_id = (commit.get("trailers") or {}).get("Item-Id") + problem = verify_fix_commit(commit, scope) + if problem: + problems.append(problem) + info(f"❌ 修正コミットが手順を満たしていません: {problem}") + continue + accepted.append((item_id, commit["sha"])) + return accepted, unassigned, problems + + def cmd_merge_fix(args: argparse.Namespace) -> None: """Step 6 — 修正結果を取り込み、修正ラウンドを 1 つ進める。""" path, state = _load(args.id) @@ -1705,35 +1787,16 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: # 適用と同じく、**範囲のコミットは全て申告されていること**を求める。 # 申告から漏れた修正コミットは検証を受けないまま Pull Request に残る。 - reported_full = { - full for full in ( - _git_out(work, ["rev-parse", "--verify", f"{s}^{{commit}}"]) - for s in reported_shas - ) if full - } - unassigned = sorted(set(ordered_range) - reported_full) - - facts = collect_commit_facts( - work, reported_shas, set(ordered_range), - baseline.get("command") or "true", state["head_branch"], - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - # **不正なコミットが 1 件でもあれば、修正ラウンドの範囲ごと取り消す。** # 状態を記録しないだけでは、未検証の変更が Pull Request に残り続ける # （見送りの対象にもならない）。どのコミットが安全かは決められないので、 # 適用フェーズの未割当コミットと同じ扱いにする。 - problems: list[str] = [] - accepted: list[tuple[str, str]] = [] # (item_id, sha) needs_push = False - for commit in facts: - item_id = (commit.get("trailers") or {}).get("Item-Id") - problem = verify_fix_commit(commit, state.get("target_scope") or []) - if problem: - problems.append(problem) - info(f"❌ 修正コミットが手順を満たしていません: {problem}") - continue - accepted.append((item_id, commit["sha"])) + accepted, unassigned, problems = _validate_fix_commits( + work, ordered_range, reported_shas, state.get("target_scope") or [], + baseline.get("command") or "true", state["head_branch"], + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), + ) if unassigned: info( @@ -1747,19 +1810,9 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: info("検証を通らない変更を残さないため、この修正ラウンドの範囲を取り消します") # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push できずに # 終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits( - state, - {"item_id": f"R{entry['round']}-fix{entry['fix_rounds'] + 1}", - "commits": list(ordered_range)}, - dry_run=False, + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "fix_base_sha", ) - # 取り消し後の状態を新しい起点にし、**その場で保存する**。ここで保存せずに - # 落ちると、次の実行は古い起点から範囲を取り直して取り消しコミット自体を - # 「未申告」と判定し、**取り消しを取り消して**しまう。 - entry["fix_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) - statefile.save(path, state) # **push は保存のあと。** ここで push して失敗すると、取り消しコミットは # ローカルに残るのに起点の更新が保存されず、叩き直しで二重に取り消してしまう。 needs_push = True diff --git a/plugins/ndf-claude/skills/cross-review/scripts/lib/metrics.py b/plugins/ndf-claude/skills/cross-review/scripts/lib/metrics.py index c2b576b4..4cc4e661 100644 --- a/plugins/ndf-claude/skills/cross-review/scripts/lib/metrics.py +++ b/plugins/ndf-claude/skills/cross-review/scripts/lib/metrics.py @@ -39,6 +39,93 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _update_impl_metrics( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドの実装担当指標とモデル警告を更新する。""" + round_no = entry.get("round") + impl_runtime = entry["impl"] + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + observed = impl_model.get("observed") + + warning = _models.mismatch_warning(impl_runtime, requested, observed) + if warning: + unmeasured.append(f"round {round_no}: {warning}") + if not _models.is_measurable(impl_runtime, requested): + unmeasured.append( + f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" + "実装担当の集計から分離する" + ) + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + bucket["rounds"] += 1 + bucket["seconds"] += _duration(entry, ("apply", "fix")) + round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] + bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") + bucket["abandoned"] += sum( + 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} + ) + bucket["budget_exceeded"] += sum(1 for i in round_items if i.get("budget_exceeded")) + bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) + bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + + reviews = _round_reviews(entry) + if reviews: + approved_first = all( + _verdict(reviews[0], reviewer) == "APPROVE" + for reviewer in entry.get("reviewers", []) + ) + bucket["first_review_total"] += 1 + bucket["first_review_approved"] += 1 if approved_first else 0 + + +def _update_reviewer_metrics( + entry: dict[str, Any], + name: str, + reviews: list[dict[str, Any]], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドのレビュー担当 1 名分の指標と警告を更新する。""" + spec = (entry.get("reviewer_models") or {}).get(name) or {} + requested = spec.get("requested") + observed = spec.get("observed") + warning = _models.mismatch_warning(name, requested, observed) + if warning: + unmeasured.append(f"round {entry.get('round')}: {warning}") + if not _models.is_measurable(name, requested): + unmeasured.append( + f"round {entry.get('round')}: {name} が既定モデル（auto）で動いたため、" + "レビュー担当の集計から分離する" + ) + bucket = reviewer.setdefault(_key(name, requested), _new_reviewer_bucket()) + bucket["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) + for review in reviews: + if _verdict(review, name) is None: + continue + bucket["reviews"] += 1 + findings = [ + finding for finding in review.get("findings", []) + if isinstance(finding, dict) and finding.get("reviewer") == name + ] + bucket["findings"] += len(findings) + bucket["findings_resolved"] += sum( + 1 for finding in findings if finding.get("resolved") + ) + for other in (candidate for candidate in entry.get("reviewers", []) if candidate != name): + other_verdict = _verdict(review, other) + if other_verdict is None: + continue + bucket["verdict_pairs"] += 1 + bucket["verdict_agreements"] += int( + other_verdict == _verdict(review, name) + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +139,14 @@ def aggregate(state: dict[str, Any]) -> dict[str, Any]: unmeasured: list[str] = [] for entry in state.get("rounds", []): - round_no = entry.get("round") impl_runtime = entry.get("impl") if not impl_runtime: continue - impl_model = (entry.get("impl_model") or {}) - requested = impl_model.get("requested") - observed = impl_model.get("observed") - - warning = _models.mismatch_warning(impl_runtime, requested, observed) - if warning: - unmeasured.append(f"round {round_no}: {warning}") - if not _models.is_measurable(impl_runtime, requested): - unmeasured.append( - f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" - "実装担当の集計から分離する" - ) - - bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) - bucket["rounds"] += 1 - bucket["seconds"] += _duration(entry, ("apply", "fix")) - - round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] - bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") - bucket["abandoned"] += sum( - 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} - ) - bucket["budget_exceeded"] += sum( - 1 for i in round_items if i.get("budget_exceeded") - ) - bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) - bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + _update_impl_metrics(entry, items_by_id, impl, unmeasured) reviews = _round_reviews(entry) - if reviews: - first = reviews[0] - approved_first = all( - _verdict(first, r) == "APPROVE" for r in entry.get("reviewers", []) - ) - bucket["first_review_total"] += 1 - bucket["first_review_approved"] += 1 if approved_first else 0 - - reviewer_models = entry.get("reviewer_models") or {} for name in entry.get("reviewers", []): - spec = reviewer_models.get(name) or {} - r_requested = spec.get("requested") - r_observed = spec.get("observed") - r_warning = _models.mismatch_warning(name, r_requested, r_observed) - if r_warning: - unmeasured.append(f"round {round_no}: {r_warning}") - if not _models.is_measurable(name, r_requested): - unmeasured.append( - f"round {round_no}: {name} が既定モデル（auto）で動いたため、" - "レビュー担当の集計から分離する" - ) - rb = reviewer.setdefault(_key(name, r_requested), _new_reviewer_bucket()) - # 担当ごとの所要時間があればそれを使う。無ければ 0 のままにする。 - # ラウンドの合計を配ると 2 者分を両方に数えてしまい、比較が成り立たない。 - rb["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) - for review in reviews: - if _verdict(review, name) is None: - continue - rb["reviews"] += 1 - findings = [ - f for f in review.get("findings", []) - if isinstance(f, dict) and f.get("reviewer") == name - ] - rb["findings"] += len(findings) - rb["findings_resolved"] += sum(1 for f in findings if f.get("resolved")) - others = [o for o in entry.get("reviewers", []) if o != name] - for other in others: - other_verdict = _verdict(review, other) - if other_verdict is None: - continue - rb["verdict_pairs"] += 1 - rb["verdict_agreements"] += ( - 1 if other_verdict == _verdict(review, name) else 0 - ) + _update_reviewer_metrics(entry, name, reviews, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py index 3ad3134b..f4093f23 100755 --- a/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py @@ -1078,6 +1078,131 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) +def _resolve_reported_to_full(work: str, reported_shas: list[str]) -> set[str]: + """申告 SHA のうち実在するコミットを完全 SHA に正規化する。""" + return { + full for full in ( + _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) + for sha in reported_shas + ) if full + } + + +def _revert_range_with_recovery( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + ordered_range: list[str], + base_sha_key: str, + dry_run: bool = False, +) -> None: + """範囲を取り消し、中断復帰用の push 印と新しい起点を保存する。""" + if base_sha_key == "apply_base_sha": + item_id = f"R{entry['round']}-range" + else: + item_id = f"R{entry['round']}-fix{entry['fix_rounds'] + 1}" + if not dry_run: + entry["pending_push"] = True + statefile.save(path, state) + _revert_item_commits( + state, {"item_id": item_id, "commits": list(ordered_range)}, dry_run=dry_run, + ) + if not dry_run: + entry[base_sha_key] = _git_out(work, ["rev-parse", "HEAD"]) + statefile.save(path, state) + + +def _validate_commit_ownership( + work: str, + ordered_range: list[str], + payload: dict[str, Any], + round_items: set[str], +) -> tuple[ + dict[str, dict[str, Any]], dict[str, str], list[str], list[str], list[str] +]: + """申告されたコミットがラウンド内で一意に所有されているか調べる。""" + reported: dict[str, dict[str, Any]] = {} + unknown_ids: list[str] = [] + raw_items = payload.get("items") + if not isinstance(raw_items, list): + info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") + raw_items = [] + for reported_item in raw_items: + if not isinstance(reported_item, dict): + continue + item_id = reported_item.get("item_id") + if item_id in round_items: + reported[item_id] = reported_item + elif item_id is not None: + unknown_ids.append(str(item_id)) + + owner_of: dict[str, str] = {} + duplicated: list[str] = [] + for item_id, reported_item in reported.items(): + for full in _resolve_reported_to_full(work, _reported_shas(reported_item)): + if full in owner_of and owner_of[full] != item_id: + duplicated.append(full) + owner_of.setdefault(full, item_id) + + unassigned = sorted(set(ordered_range) - set(owner_of)) + return reported, owner_of, unassigned, unknown_ids, duplicated + + +def _verify_and_record_items( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + reported: dict[str, dict[str, Any]], + in_range: set[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, + dry_run: bool, +) -> tuple[list[str], list[str]]: + """適用項目を検証し、項目と進捗に判定を記録する。""" + applied: list[str] = [] + failed: list[str] = [] + progress: list[dict[str, Any]] = [] + entry["apply_progress"] = progress + for item_id in entry["items"]: + item = _find_item(state, item_id) + got = reported.get(item_id) + if got is None: + problem = "適用結果に項目がありません" + facts: list[dict[str, Any]] = [] + else: + facts = collect_commit_facts( + work, _reported_shas(got), in_range, test_command, head_branch, timeout, + ) + problem = verify_apply_item(item, facts, scope) + if problem: + item["status"] = "abandoned" + item["failure_reason"] = problem + item["test_failed"] = bool(got and "テストが成功していません" in problem) + item["budget_exceeded"] = bool(got and "差分予算" in problem) + item["out_of_scope"] = bool(got and "対象範囲の外" in problem) + item["commits"] = _reported_shas(got) + failed.append(item_id) + info(f"❌ {item_id}: {problem}") + else: + item["status"] = "reviewing" + item["commits"] = _reported_shas(got) + item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) + applied.append(item_id) + info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") + progress.append({ + "item_id": item_id, "at": statefile.now(), + "result": "failed" if problem else "ok", + "reason": problem, "commits": list(item.get("commits") or []), + }) + if not dry_run: + statefile.save(path, state) + return applied, failed + + def cmd_merge_apply(args: argparse.Namespace) -> None: """Step 4 — 適用結果を検証して取り込む。 @@ -1152,21 +1277,6 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # コミットを数に入れると、割り当て済みに見えるのに項目別の検証にも入らず、 # そのまま Pull Request に残せてしまう。 round_items = set(entry["items"]) - reported: dict[str, dict[str, Any]] = {} - unknown_ids: list[str] = [] - raw_items = payload.get("items") - if not isinstance(raw_items, list): - info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") - raw_items = [] - for r in raw_items: - if not isinstance(r, dict): - continue - item_id = r.get("item_id") - if item_id in round_items: - reported[item_id] = r - elif item_id is not None: - unknown_ids.append(str(item_id)) - # **範囲のコミットは全て、いずれかの改善項目に割り当てられていること。** # 申告から漏れたコミットはテストもトレーラーも差分予算も検査されず、そのまま # Pull Request に残る。都合の悪い変更を申告しないだけで検査を回避できてしまう。 @@ -1176,18 +1286,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # # 判定は**完全な SHA へ正規化してから**行う。申告の文字列をそのまま鍵にすると、 # 一方が完全 SHA、他方が短縮 SHA で同じコミットを指したときに重複を見逃す。 - owner_of: dict[str, str] = {} - duplicated: list[str] = [] - for item_id, r in reported.items(): - for sha in _reported_shas(r): - full = _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) - if full is None: - continue # 実在しない申告は項目ごとの検証で落ちる - if full in owner_of and owner_of[full] != item_id: - duplicated.append(full) - owner_of.setdefault(full, item_id) - - unassigned = sorted(in_range - set(owner_of)) + reported, owner_of, unassigned, unknown_ids, duplicated = ( + _validate_commit_ownership(work, ordered_range, payload, round_items) + ) if unassigned or unknown_ids or duplicated: causes = [] if unassigned: @@ -1218,20 +1319,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 範囲全体を取り消す。どのコミットが安全かを決められない以上、 # 起点まで戻すのが最も確実である。順序は `_revert_item_commits` が # git の履歴から決め直す。 - whole_round = { - "item_id": f"R{entry['round']}-range", - "commits": list(ordered_range), - } - if not args.dry_run: - # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push - # できずに終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits(state, whole_round, args.dry_run) - if not args.dry_run: - # 取り消し後の状態を新しい起点にする。叩き直しても範囲が空になり、 - # 取り消しコミット自体を「未割当」として再び戻すことがない。 - entry["apply_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "apply_base_sha", args.dry_run, + ) entry["apply"] = { "applied": [], "failed": list(entry["items"]), "base_sha": entry.get("apply_base_sha"), "head_sha": head_sha, @@ -1250,51 +1340,15 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: statefile.save(path, state) sys.exit(2) - applied: list[str] = [] - failed: list[str] = [] scope = state.get("target_scope") or [] # **判定はその都度残す。** まとめて最後に保存すると、取り消しの途中で中断した # ときに適用の記録が一切残らず、どのコミットが検証を通ったのかを状態から # 復元できなくなる。再開可能性は収束ループの前提なので、ここが崩れると # 中断からの復帰手段が無くなる。 - progress: list[dict[str, Any]] = [] - entry["apply_progress"] = progress - for item_id in entry["items"]: - item = _find_item(state, item_id) - got = reported.get(item_id) - if got is None: - problem = "適用結果に項目がありません" - facts: list[dict[str, Any]] = [] - else: - facts = collect_commit_facts( - work, _reported_shas(got), in_range, test_command, head_branch, - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - problem = verify_apply_item(item, facts, scope) - if problem: - item["status"] = "abandoned" - item["failure_reason"] = problem - item["test_failed"] = bool(got and "テストが成功していません" in problem) - item["budget_exceeded"] = bool(got and "差分予算" in problem) - item["out_of_scope"] = bool(got and "対象範囲の外" in problem) - # 取り消しは全項目の判定が出そろってから**まとめて**行う。項目ごとに - # その場で戻すと、まだ判定していない項目のコミットと競合する。 - item["commits"] = _reported_shas(got) - failed.append(item_id) - info(f"❌ {item_id}: {problem}") - else: - item["status"] = "reviewing" - item["commits"] = _reported_shas(got) - item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) - applied.append(item_id) - info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") - progress.append({ - "item_id": item_id, "at": statefile.now(), - "result": "failed" if problem else "ok", - "reason": problem, "commits": list(item.get("commits") or []), - }) - if not args.dry_run: - statefile.save(path, state) + applied, failed = _verify_and_record_items( + path, state, entry, work, reported, in_range, scope, test_command, head_branch, + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), args.dry_run, + ) entry["apply"] = { "applied": applied, @@ -1626,6 +1680,34 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: statefile.save(path, state) +def _validate_fix_commits( + work: str, + ordered_range: list[str], + reported_shas: list[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, +) -> tuple[list[tuple[str, str]], list[str], list[str]]: + """修正コミットの申告漏れと手順適合を検証する。""" + reported_full = _resolve_reported_to_full(work, reported_shas) + unassigned = sorted(set(ordered_range) - reported_full) + facts = collect_commit_facts( + work, reported_shas, set(ordered_range), test_command, head_branch, timeout, + ) + accepted: list[tuple[str, str]] = [] + problems: list[str] = [] + for commit in facts: + item_id = (commit.get("trailers") or {}).get("Item-Id") + problem = verify_fix_commit(commit, scope) + if problem: + problems.append(problem) + info(f"❌ 修正コミットが手順を満たしていません: {problem}") + continue + accepted.append((item_id, commit["sha"])) + return accepted, unassigned, problems + + def cmd_merge_fix(args: argparse.Namespace) -> None: """Step 6 — 修正結果を取り込み、修正ラウンドを 1 つ進める。""" path, state = _load(args.id) @@ -1705,35 +1787,16 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: # 適用と同じく、**範囲のコミットは全て申告されていること**を求める。 # 申告から漏れた修正コミットは検証を受けないまま Pull Request に残る。 - reported_full = { - full for full in ( - _git_out(work, ["rev-parse", "--verify", f"{s}^{{commit}}"]) - for s in reported_shas - ) if full - } - unassigned = sorted(set(ordered_range) - reported_full) - - facts = collect_commit_facts( - work, reported_shas, set(ordered_range), - baseline.get("command") or "true", state["head_branch"], - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - # **不正なコミットが 1 件でもあれば、修正ラウンドの範囲ごと取り消す。** # 状態を記録しないだけでは、未検証の変更が Pull Request に残り続ける # （見送りの対象にもならない）。どのコミットが安全かは決められないので、 # 適用フェーズの未割当コミットと同じ扱いにする。 - problems: list[str] = [] - accepted: list[tuple[str, str]] = [] # (item_id, sha) needs_push = False - for commit in facts: - item_id = (commit.get("trailers") or {}).get("Item-Id") - problem = verify_fix_commit(commit, state.get("target_scope") or []) - if problem: - problems.append(problem) - info(f"❌ 修正コミットが手順を満たしていません: {problem}") - continue - accepted.append((item_id, commit["sha"])) + accepted, unassigned, problems = _validate_fix_commits( + work, ordered_range, reported_shas, state.get("target_scope") or [], + baseline.get("command") or "true", state["head_branch"], + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), + ) if unassigned: info( @@ -1747,19 +1810,9 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: info("検証を通らない変更を残さないため、この修正ラウンドの範囲を取り消します") # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push できずに # 終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits( - state, - {"item_id": f"R{entry['round']}-fix{entry['fix_rounds'] + 1}", - "commits": list(ordered_range)}, - dry_run=False, + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "fix_base_sha", ) - # 取り消し後の状態を新しい起点にし、**その場で保存する**。ここで保存せずに - # 落ちると、次の実行は古い起点から範囲を取り直して取り消しコミット自体を - # 「未申告」と判定し、**取り消しを取り消して**しまう。 - entry["fix_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) - statefile.save(path, state) # **push は保存のあと。** ここで push して失敗すると、取り消しコミットは # ローカルに残るのに起点の更新が保存されず、叩き直しで二重に取り消してしまう。 needs_push = True diff --git a/plugins/ndf-codex/skills/cross-review/scripts/lib/metrics.py b/plugins/ndf-codex/skills/cross-review/scripts/lib/metrics.py index c2b576b4..4cc4e661 100644 --- a/plugins/ndf-codex/skills/cross-review/scripts/lib/metrics.py +++ b/plugins/ndf-codex/skills/cross-review/scripts/lib/metrics.py @@ -39,6 +39,93 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _update_impl_metrics( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドの実装担当指標とモデル警告を更新する。""" + round_no = entry.get("round") + impl_runtime = entry["impl"] + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + observed = impl_model.get("observed") + + warning = _models.mismatch_warning(impl_runtime, requested, observed) + if warning: + unmeasured.append(f"round {round_no}: {warning}") + if not _models.is_measurable(impl_runtime, requested): + unmeasured.append( + f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" + "実装担当の集計から分離する" + ) + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + bucket["rounds"] += 1 + bucket["seconds"] += _duration(entry, ("apply", "fix")) + round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] + bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") + bucket["abandoned"] += sum( + 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} + ) + bucket["budget_exceeded"] += sum(1 for i in round_items if i.get("budget_exceeded")) + bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) + bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + + reviews = _round_reviews(entry) + if reviews: + approved_first = all( + _verdict(reviews[0], reviewer) == "APPROVE" + for reviewer in entry.get("reviewers", []) + ) + bucket["first_review_total"] += 1 + bucket["first_review_approved"] += 1 if approved_first else 0 + + +def _update_reviewer_metrics( + entry: dict[str, Any], + name: str, + reviews: list[dict[str, Any]], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドのレビュー担当 1 名分の指標と警告を更新する。""" + spec = (entry.get("reviewer_models") or {}).get(name) or {} + requested = spec.get("requested") + observed = spec.get("observed") + warning = _models.mismatch_warning(name, requested, observed) + if warning: + unmeasured.append(f"round {entry.get('round')}: {warning}") + if not _models.is_measurable(name, requested): + unmeasured.append( + f"round {entry.get('round')}: {name} が既定モデル（auto）で動いたため、" + "レビュー担当の集計から分離する" + ) + bucket = reviewer.setdefault(_key(name, requested), _new_reviewer_bucket()) + bucket["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) + for review in reviews: + if _verdict(review, name) is None: + continue + bucket["reviews"] += 1 + findings = [ + finding for finding in review.get("findings", []) + if isinstance(finding, dict) and finding.get("reviewer") == name + ] + bucket["findings"] += len(findings) + bucket["findings_resolved"] += sum( + 1 for finding in findings if finding.get("resolved") + ) + for other in (candidate for candidate in entry.get("reviewers", []) if candidate != name): + other_verdict = _verdict(review, other) + if other_verdict is None: + continue + bucket["verdict_pairs"] += 1 + bucket["verdict_agreements"] += int( + other_verdict == _verdict(review, name) + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +139,14 @@ def aggregate(state: dict[str, Any]) -> dict[str, Any]: unmeasured: list[str] = [] for entry in state.get("rounds", []): - round_no = entry.get("round") impl_runtime = entry.get("impl") if not impl_runtime: continue - impl_model = (entry.get("impl_model") or {}) - requested = impl_model.get("requested") - observed = impl_model.get("observed") - - warning = _models.mismatch_warning(impl_runtime, requested, observed) - if warning: - unmeasured.append(f"round {round_no}: {warning}") - if not _models.is_measurable(impl_runtime, requested): - unmeasured.append( - f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" - "実装担当の集計から分離する" - ) - - bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) - bucket["rounds"] += 1 - bucket["seconds"] += _duration(entry, ("apply", "fix")) - - round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] - bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") - bucket["abandoned"] += sum( - 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} - ) - bucket["budget_exceeded"] += sum( - 1 for i in round_items if i.get("budget_exceeded") - ) - bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) - bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + _update_impl_metrics(entry, items_by_id, impl, unmeasured) reviews = _round_reviews(entry) - if reviews: - first = reviews[0] - approved_first = all( - _verdict(first, r) == "APPROVE" for r in entry.get("reviewers", []) - ) - bucket["first_review_total"] += 1 - bucket["first_review_approved"] += 1 if approved_first else 0 - - reviewer_models = entry.get("reviewer_models") or {} for name in entry.get("reviewers", []): - spec = reviewer_models.get(name) or {} - r_requested = spec.get("requested") - r_observed = spec.get("observed") - r_warning = _models.mismatch_warning(name, r_requested, r_observed) - if r_warning: - unmeasured.append(f"round {round_no}: {r_warning}") - if not _models.is_measurable(name, r_requested): - unmeasured.append( - f"round {round_no}: {name} が既定モデル（auto）で動いたため、" - "レビュー担当の集計から分離する" - ) - rb = reviewer.setdefault(_key(name, r_requested), _new_reviewer_bucket()) - # 担当ごとの所要時間があればそれを使う。無ければ 0 のままにする。 - # ラウンドの合計を配ると 2 者分を両方に数えてしまい、比較が成り立たない。 - rb["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) - for review in reviews: - if _verdict(review, name) is None: - continue - rb["reviews"] += 1 - findings = [ - f for f in review.get("findings", []) - if isinstance(f, dict) and f.get("reviewer") == name - ] - rb["findings"] += len(findings) - rb["findings_resolved"] += sum(1 for f in findings if f.get("resolved")) - others = [o for o in entry.get("reviewers", []) if o != name] - for other in others: - other_verdict = _verdict(review, other) - if other_verdict is None: - continue - rb["verdict_pairs"] += 1 - rb["verdict_agreements"] += ( - 1 if other_verdict == _verdict(review, name) else 0 - ) + _update_reviewer_metrics(entry, name, reviews, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py index 3ad3134b..f4093f23 100755 --- a/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py @@ -1078,6 +1078,131 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) +def _resolve_reported_to_full(work: str, reported_shas: list[str]) -> set[str]: + """申告 SHA のうち実在するコミットを完全 SHA に正規化する。""" + return { + full for full in ( + _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) + for sha in reported_shas + ) if full + } + + +def _revert_range_with_recovery( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + ordered_range: list[str], + base_sha_key: str, + dry_run: bool = False, +) -> None: + """範囲を取り消し、中断復帰用の push 印と新しい起点を保存する。""" + if base_sha_key == "apply_base_sha": + item_id = f"R{entry['round']}-range" + else: + item_id = f"R{entry['round']}-fix{entry['fix_rounds'] + 1}" + if not dry_run: + entry["pending_push"] = True + statefile.save(path, state) + _revert_item_commits( + state, {"item_id": item_id, "commits": list(ordered_range)}, dry_run=dry_run, + ) + if not dry_run: + entry[base_sha_key] = _git_out(work, ["rev-parse", "HEAD"]) + statefile.save(path, state) + + +def _validate_commit_ownership( + work: str, + ordered_range: list[str], + payload: dict[str, Any], + round_items: set[str], +) -> tuple[ + dict[str, dict[str, Any]], dict[str, str], list[str], list[str], list[str] +]: + """申告されたコミットがラウンド内で一意に所有されているか調べる。""" + reported: dict[str, dict[str, Any]] = {} + unknown_ids: list[str] = [] + raw_items = payload.get("items") + if not isinstance(raw_items, list): + info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") + raw_items = [] + for reported_item in raw_items: + if not isinstance(reported_item, dict): + continue + item_id = reported_item.get("item_id") + if item_id in round_items: + reported[item_id] = reported_item + elif item_id is not None: + unknown_ids.append(str(item_id)) + + owner_of: dict[str, str] = {} + duplicated: list[str] = [] + for item_id, reported_item in reported.items(): + for full in _resolve_reported_to_full(work, _reported_shas(reported_item)): + if full in owner_of and owner_of[full] != item_id: + duplicated.append(full) + owner_of.setdefault(full, item_id) + + unassigned = sorted(set(ordered_range) - set(owner_of)) + return reported, owner_of, unassigned, unknown_ids, duplicated + + +def _verify_and_record_items( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + reported: dict[str, dict[str, Any]], + in_range: set[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, + dry_run: bool, +) -> tuple[list[str], list[str]]: + """適用項目を検証し、項目と進捗に判定を記録する。""" + applied: list[str] = [] + failed: list[str] = [] + progress: list[dict[str, Any]] = [] + entry["apply_progress"] = progress + for item_id in entry["items"]: + item = _find_item(state, item_id) + got = reported.get(item_id) + if got is None: + problem = "適用結果に項目がありません" + facts: list[dict[str, Any]] = [] + else: + facts = collect_commit_facts( + work, _reported_shas(got), in_range, test_command, head_branch, timeout, + ) + problem = verify_apply_item(item, facts, scope) + if problem: + item["status"] = "abandoned" + item["failure_reason"] = problem + item["test_failed"] = bool(got and "テストが成功していません" in problem) + item["budget_exceeded"] = bool(got and "差分予算" in problem) + item["out_of_scope"] = bool(got and "対象範囲の外" in problem) + item["commits"] = _reported_shas(got) + failed.append(item_id) + info(f"❌ {item_id}: {problem}") + else: + item["status"] = "reviewing" + item["commits"] = _reported_shas(got) + item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) + applied.append(item_id) + info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") + progress.append({ + "item_id": item_id, "at": statefile.now(), + "result": "failed" if problem else "ok", + "reason": problem, "commits": list(item.get("commits") or []), + }) + if not dry_run: + statefile.save(path, state) + return applied, failed + + def cmd_merge_apply(args: argparse.Namespace) -> None: """Step 4 — 適用結果を検証して取り込む。 @@ -1152,21 +1277,6 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # コミットを数に入れると、割り当て済みに見えるのに項目別の検証にも入らず、 # そのまま Pull Request に残せてしまう。 round_items = set(entry["items"]) - reported: dict[str, dict[str, Any]] = {} - unknown_ids: list[str] = [] - raw_items = payload.get("items") - if not isinstance(raw_items, list): - info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") - raw_items = [] - for r in raw_items: - if not isinstance(r, dict): - continue - item_id = r.get("item_id") - if item_id in round_items: - reported[item_id] = r - elif item_id is not None: - unknown_ids.append(str(item_id)) - # **範囲のコミットは全て、いずれかの改善項目に割り当てられていること。** # 申告から漏れたコミットはテストもトレーラーも差分予算も検査されず、そのまま # Pull Request に残る。都合の悪い変更を申告しないだけで検査を回避できてしまう。 @@ -1176,18 +1286,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # # 判定は**完全な SHA へ正規化してから**行う。申告の文字列をそのまま鍵にすると、 # 一方が完全 SHA、他方が短縮 SHA で同じコミットを指したときに重複を見逃す。 - owner_of: dict[str, str] = {} - duplicated: list[str] = [] - for item_id, r in reported.items(): - for sha in _reported_shas(r): - full = _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) - if full is None: - continue # 実在しない申告は項目ごとの検証で落ちる - if full in owner_of and owner_of[full] != item_id: - duplicated.append(full) - owner_of.setdefault(full, item_id) - - unassigned = sorted(in_range - set(owner_of)) + reported, owner_of, unassigned, unknown_ids, duplicated = ( + _validate_commit_ownership(work, ordered_range, payload, round_items) + ) if unassigned or unknown_ids or duplicated: causes = [] if unassigned: @@ -1218,20 +1319,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 範囲全体を取り消す。どのコミットが安全かを決められない以上、 # 起点まで戻すのが最も確実である。順序は `_revert_item_commits` が # git の履歴から決め直す。 - whole_round = { - "item_id": f"R{entry['round']}-range", - "commits": list(ordered_range), - } - if not args.dry_run: - # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push - # できずに終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits(state, whole_round, args.dry_run) - if not args.dry_run: - # 取り消し後の状態を新しい起点にする。叩き直しても範囲が空になり、 - # 取り消しコミット自体を「未割当」として再び戻すことがない。 - entry["apply_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "apply_base_sha", args.dry_run, + ) entry["apply"] = { "applied": [], "failed": list(entry["items"]), "base_sha": entry.get("apply_base_sha"), "head_sha": head_sha, @@ -1250,51 +1340,15 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: statefile.save(path, state) sys.exit(2) - applied: list[str] = [] - failed: list[str] = [] scope = state.get("target_scope") or [] # **判定はその都度残す。** まとめて最後に保存すると、取り消しの途中で中断した # ときに適用の記録が一切残らず、どのコミットが検証を通ったのかを状態から # 復元できなくなる。再開可能性は収束ループの前提なので、ここが崩れると # 中断からの復帰手段が無くなる。 - progress: list[dict[str, Any]] = [] - entry["apply_progress"] = progress - for item_id in entry["items"]: - item = _find_item(state, item_id) - got = reported.get(item_id) - if got is None: - problem = "適用結果に項目がありません" - facts: list[dict[str, Any]] = [] - else: - facts = collect_commit_facts( - work, _reported_shas(got), in_range, test_command, head_branch, - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - problem = verify_apply_item(item, facts, scope) - if problem: - item["status"] = "abandoned" - item["failure_reason"] = problem - item["test_failed"] = bool(got and "テストが成功していません" in problem) - item["budget_exceeded"] = bool(got and "差分予算" in problem) - item["out_of_scope"] = bool(got and "対象範囲の外" in problem) - # 取り消しは全項目の判定が出そろってから**まとめて**行う。項目ごとに - # その場で戻すと、まだ判定していない項目のコミットと競合する。 - item["commits"] = _reported_shas(got) - failed.append(item_id) - info(f"❌ {item_id}: {problem}") - else: - item["status"] = "reviewing" - item["commits"] = _reported_shas(got) - item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) - applied.append(item_id) - info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") - progress.append({ - "item_id": item_id, "at": statefile.now(), - "result": "failed" if problem else "ok", - "reason": problem, "commits": list(item.get("commits") or []), - }) - if not args.dry_run: - statefile.save(path, state) + applied, failed = _verify_and_record_items( + path, state, entry, work, reported, in_range, scope, test_command, head_branch, + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), args.dry_run, + ) entry["apply"] = { "applied": applied, @@ -1626,6 +1680,34 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: statefile.save(path, state) +def _validate_fix_commits( + work: str, + ordered_range: list[str], + reported_shas: list[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, +) -> tuple[list[tuple[str, str]], list[str], list[str]]: + """修正コミットの申告漏れと手順適合を検証する。""" + reported_full = _resolve_reported_to_full(work, reported_shas) + unassigned = sorted(set(ordered_range) - reported_full) + facts = collect_commit_facts( + work, reported_shas, set(ordered_range), test_command, head_branch, timeout, + ) + accepted: list[tuple[str, str]] = [] + problems: list[str] = [] + for commit in facts: + item_id = (commit.get("trailers") or {}).get("Item-Id") + problem = verify_fix_commit(commit, scope) + if problem: + problems.append(problem) + info(f"❌ 修正コミットが手順を満たしていません: {problem}") + continue + accepted.append((item_id, commit["sha"])) + return accepted, unassigned, problems + + def cmd_merge_fix(args: argparse.Namespace) -> None: """Step 6 — 修正結果を取り込み、修正ラウンドを 1 つ進める。""" path, state = _load(args.id) @@ -1705,35 +1787,16 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: # 適用と同じく、**範囲のコミットは全て申告されていること**を求める。 # 申告から漏れた修正コミットは検証を受けないまま Pull Request に残る。 - reported_full = { - full for full in ( - _git_out(work, ["rev-parse", "--verify", f"{s}^{{commit}}"]) - for s in reported_shas - ) if full - } - unassigned = sorted(set(ordered_range) - reported_full) - - facts = collect_commit_facts( - work, reported_shas, set(ordered_range), - baseline.get("command") or "true", state["head_branch"], - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - # **不正なコミットが 1 件でもあれば、修正ラウンドの範囲ごと取り消す。** # 状態を記録しないだけでは、未検証の変更が Pull Request に残り続ける # （見送りの対象にもならない）。どのコミットが安全かは決められないので、 # 適用フェーズの未割当コミットと同じ扱いにする。 - problems: list[str] = [] - accepted: list[tuple[str, str]] = [] # (item_id, sha) needs_push = False - for commit in facts: - item_id = (commit.get("trailers") or {}).get("Item-Id") - problem = verify_fix_commit(commit, state.get("target_scope") or []) - if problem: - problems.append(problem) - info(f"❌ 修正コミットが手順を満たしていません: {problem}") - continue - accepted.append((item_id, commit["sha"])) + accepted, unassigned, problems = _validate_fix_commits( + work, ordered_range, reported_shas, state.get("target_scope") or [], + baseline.get("command") or "true", state["head_branch"], + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), + ) if unassigned: info( @@ -1747,19 +1810,9 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: info("検証を通らない変更を残さないため、この修正ラウンドの範囲を取り消します") # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push できずに # 終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits( - state, - {"item_id": f"R{entry['round']}-fix{entry['fix_rounds'] + 1}", - "commits": list(ordered_range)}, - dry_run=False, + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "fix_base_sha", ) - # 取り消し後の状態を新しい起点にし、**その場で保存する**。ここで保存せずに - # 落ちると、次の実行は古い起点から範囲を取り直して取り消しコミット自体を - # 「未申告」と判定し、**取り消しを取り消して**しまう。 - entry["fix_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) - statefile.save(path, state) # **push は保存のあと。** ここで push して失敗すると、取り消しコミットは # ローカルに残るのに起点の更新が保存されず、叩き直しで二重に取り消してしまう。 needs_push = True diff --git a/plugins/ndf-kiro/skills/cross-review/scripts/lib/metrics.py b/plugins/ndf-kiro/skills/cross-review/scripts/lib/metrics.py index c2b576b4..4cc4e661 100644 --- a/plugins/ndf-kiro/skills/cross-review/scripts/lib/metrics.py +++ b/plugins/ndf-kiro/skills/cross-review/scripts/lib/metrics.py @@ -39,6 +39,93 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _update_impl_metrics( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドの実装担当指標とモデル警告を更新する。""" + round_no = entry.get("round") + impl_runtime = entry["impl"] + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + observed = impl_model.get("observed") + + warning = _models.mismatch_warning(impl_runtime, requested, observed) + if warning: + unmeasured.append(f"round {round_no}: {warning}") + if not _models.is_measurable(impl_runtime, requested): + unmeasured.append( + f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" + "実装担当の集計から分離する" + ) + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + bucket["rounds"] += 1 + bucket["seconds"] += _duration(entry, ("apply", "fix")) + round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] + bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") + bucket["abandoned"] += sum( + 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} + ) + bucket["budget_exceeded"] += sum(1 for i in round_items if i.get("budget_exceeded")) + bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) + bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + + reviews = _round_reviews(entry) + if reviews: + approved_first = all( + _verdict(reviews[0], reviewer) == "APPROVE" + for reviewer in entry.get("reviewers", []) + ) + bucket["first_review_total"] += 1 + bucket["first_review_approved"] += 1 if approved_first else 0 + + +def _update_reviewer_metrics( + entry: dict[str, Any], + name: str, + reviews: list[dict[str, Any]], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドのレビュー担当 1 名分の指標と警告を更新する。""" + spec = (entry.get("reviewer_models") or {}).get(name) or {} + requested = spec.get("requested") + observed = spec.get("observed") + warning = _models.mismatch_warning(name, requested, observed) + if warning: + unmeasured.append(f"round {entry.get('round')}: {warning}") + if not _models.is_measurable(name, requested): + unmeasured.append( + f"round {entry.get('round')}: {name} が既定モデル（auto）で動いたため、" + "レビュー担当の集計から分離する" + ) + bucket = reviewer.setdefault(_key(name, requested), _new_reviewer_bucket()) + bucket["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) + for review in reviews: + if _verdict(review, name) is None: + continue + bucket["reviews"] += 1 + findings = [ + finding for finding in review.get("findings", []) + if isinstance(finding, dict) and finding.get("reviewer") == name + ] + bucket["findings"] += len(findings) + bucket["findings_resolved"] += sum( + 1 for finding in findings if finding.get("resolved") + ) + for other in (candidate for candidate in entry.get("reviewers", []) if candidate != name): + other_verdict = _verdict(review, other) + if other_verdict is None: + continue + bucket["verdict_pairs"] += 1 + bucket["verdict_agreements"] += int( + other_verdict == _verdict(review, name) + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +139,14 @@ def aggregate(state: dict[str, Any]) -> dict[str, Any]: unmeasured: list[str] = [] for entry in state.get("rounds", []): - round_no = entry.get("round") impl_runtime = entry.get("impl") if not impl_runtime: continue - impl_model = (entry.get("impl_model") or {}) - requested = impl_model.get("requested") - observed = impl_model.get("observed") - - warning = _models.mismatch_warning(impl_runtime, requested, observed) - if warning: - unmeasured.append(f"round {round_no}: {warning}") - if not _models.is_measurable(impl_runtime, requested): - unmeasured.append( - f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" - "実装担当の集計から分離する" - ) - - bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) - bucket["rounds"] += 1 - bucket["seconds"] += _duration(entry, ("apply", "fix")) - - round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] - bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") - bucket["abandoned"] += sum( - 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} - ) - bucket["budget_exceeded"] += sum( - 1 for i in round_items if i.get("budget_exceeded") - ) - bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) - bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + _update_impl_metrics(entry, items_by_id, impl, unmeasured) reviews = _round_reviews(entry) - if reviews: - first = reviews[0] - approved_first = all( - _verdict(first, r) == "APPROVE" for r in entry.get("reviewers", []) - ) - bucket["first_review_total"] += 1 - bucket["first_review_approved"] += 1 if approved_first else 0 - - reviewer_models = entry.get("reviewer_models") or {} for name in entry.get("reviewers", []): - spec = reviewer_models.get(name) or {} - r_requested = spec.get("requested") - r_observed = spec.get("observed") - r_warning = _models.mismatch_warning(name, r_requested, r_observed) - if r_warning: - unmeasured.append(f"round {round_no}: {r_warning}") - if not _models.is_measurable(name, r_requested): - unmeasured.append( - f"round {round_no}: {name} が既定モデル（auto）で動いたため、" - "レビュー担当の集計から分離する" - ) - rb = reviewer.setdefault(_key(name, r_requested), _new_reviewer_bucket()) - # 担当ごとの所要時間があればそれを使う。無ければ 0 のままにする。 - # ラウンドの合計を配ると 2 者分を両方に数えてしまい、比較が成り立たない。 - rb["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) - for review in reviews: - if _verdict(review, name) is None: - continue - rb["reviews"] += 1 - findings = [ - f for f in review.get("findings", []) - if isinstance(f, dict) and f.get("reviewer") == name - ] - rb["findings"] += len(findings) - rb["findings_resolved"] += sum(1 for f in findings if f.get("resolved")) - others = [o for o in entry.get("reviewers", []) if o != name] - for other in others: - other_verdict = _verdict(review, other) - if other_verdict is None: - continue - rb["verdict_pairs"] += 1 - rb["verdict_agreements"] += ( - 1 if other_verdict == _verdict(review, name) else 0 - ) + _update_reviewer_metrics(entry, name, reviews, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py index 3ad3134b..f4093f23 100755 --- a/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py @@ -1078,6 +1078,131 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) +def _resolve_reported_to_full(work: str, reported_shas: list[str]) -> set[str]: + """申告 SHA のうち実在するコミットを完全 SHA に正規化する。""" + return { + full for full in ( + _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) + for sha in reported_shas + ) if full + } + + +def _revert_range_with_recovery( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + ordered_range: list[str], + base_sha_key: str, + dry_run: bool = False, +) -> None: + """範囲を取り消し、中断復帰用の push 印と新しい起点を保存する。""" + if base_sha_key == "apply_base_sha": + item_id = f"R{entry['round']}-range" + else: + item_id = f"R{entry['round']}-fix{entry['fix_rounds'] + 1}" + if not dry_run: + entry["pending_push"] = True + statefile.save(path, state) + _revert_item_commits( + state, {"item_id": item_id, "commits": list(ordered_range)}, dry_run=dry_run, + ) + if not dry_run: + entry[base_sha_key] = _git_out(work, ["rev-parse", "HEAD"]) + statefile.save(path, state) + + +def _validate_commit_ownership( + work: str, + ordered_range: list[str], + payload: dict[str, Any], + round_items: set[str], +) -> tuple[ + dict[str, dict[str, Any]], dict[str, str], list[str], list[str], list[str] +]: + """申告されたコミットがラウンド内で一意に所有されているか調べる。""" + reported: dict[str, dict[str, Any]] = {} + unknown_ids: list[str] = [] + raw_items = payload.get("items") + if not isinstance(raw_items, list): + info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") + raw_items = [] + for reported_item in raw_items: + if not isinstance(reported_item, dict): + continue + item_id = reported_item.get("item_id") + if item_id in round_items: + reported[item_id] = reported_item + elif item_id is not None: + unknown_ids.append(str(item_id)) + + owner_of: dict[str, str] = {} + duplicated: list[str] = [] + for item_id, reported_item in reported.items(): + for full in _resolve_reported_to_full(work, _reported_shas(reported_item)): + if full in owner_of and owner_of[full] != item_id: + duplicated.append(full) + owner_of.setdefault(full, item_id) + + unassigned = sorted(set(ordered_range) - set(owner_of)) + return reported, owner_of, unassigned, unknown_ids, duplicated + + +def _verify_and_record_items( + path: pathlib.Path, + state: dict[str, Any], + entry: dict[str, Any], + work: str, + reported: dict[str, dict[str, Any]], + in_range: set[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, + dry_run: bool, +) -> tuple[list[str], list[str]]: + """適用項目を検証し、項目と進捗に判定を記録する。""" + applied: list[str] = [] + failed: list[str] = [] + progress: list[dict[str, Any]] = [] + entry["apply_progress"] = progress + for item_id in entry["items"]: + item = _find_item(state, item_id) + got = reported.get(item_id) + if got is None: + problem = "適用結果に項目がありません" + facts: list[dict[str, Any]] = [] + else: + facts = collect_commit_facts( + work, _reported_shas(got), in_range, test_command, head_branch, timeout, + ) + problem = verify_apply_item(item, facts, scope) + if problem: + item["status"] = "abandoned" + item["failure_reason"] = problem + item["test_failed"] = bool(got and "テストが成功していません" in problem) + item["budget_exceeded"] = bool(got and "差分予算" in problem) + item["out_of_scope"] = bool(got and "対象範囲の外" in problem) + item["commits"] = _reported_shas(got) + failed.append(item_id) + info(f"❌ {item_id}: {problem}") + else: + item["status"] = "reviewing" + item["commits"] = _reported_shas(got) + item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) + applied.append(item_id) + info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") + progress.append({ + "item_id": item_id, "at": statefile.now(), + "result": "failed" if problem else "ok", + "reason": problem, "commits": list(item.get("commits") or []), + }) + if not dry_run: + statefile.save(path, state) + return applied, failed + + def cmd_merge_apply(args: argparse.Namespace) -> None: """Step 4 — 適用結果を検証して取り込む。 @@ -1152,21 +1277,6 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # コミットを数に入れると、割り当て済みに見えるのに項目別の検証にも入らず、 # そのまま Pull Request に残せてしまう。 round_items = set(entry["items"]) - reported: dict[str, dict[str, Any]] = {} - unknown_ids: list[str] = [] - raw_items = payload.get("items") - if not isinstance(raw_items, list): - info(f"⚠ 適用結果の items が配列ではありません（{type(raw_items).__name__}）") - raw_items = [] - for r in raw_items: - if not isinstance(r, dict): - continue - item_id = r.get("item_id") - if item_id in round_items: - reported[item_id] = r - elif item_id is not None: - unknown_ids.append(str(item_id)) - # **範囲のコミットは全て、いずれかの改善項目に割り当てられていること。** # 申告から漏れたコミットはテストもトレーラーも差分予算も検査されず、そのまま # Pull Request に残る。都合の悪い変更を申告しないだけで検査を回避できてしまう。 @@ -1176,18 +1286,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # # 判定は**完全な SHA へ正規化してから**行う。申告の文字列をそのまま鍵にすると、 # 一方が完全 SHA、他方が短縮 SHA で同じコミットを指したときに重複を見逃す。 - owner_of: dict[str, str] = {} - duplicated: list[str] = [] - for item_id, r in reported.items(): - for sha in _reported_shas(r): - full = _git_out(work, ["rev-parse", "--verify", f"{sha}^{{commit}}"]) - if full is None: - continue # 実在しない申告は項目ごとの検証で落ちる - if full in owner_of and owner_of[full] != item_id: - duplicated.append(full) - owner_of.setdefault(full, item_id) - - unassigned = sorted(in_range - set(owner_of)) + reported, owner_of, unassigned, unknown_ids, duplicated = ( + _validate_commit_ownership(work, ordered_range, payload, round_items) + ) if unassigned or unknown_ids or duplicated: causes = [] if unassigned: @@ -1218,20 +1319,9 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 範囲全体を取り消す。どのコミットが安全かを決められない以上、 # 起点まで戻すのが最も確実である。順序は `_revert_item_commits` が # git の履歴から決め直す。 - whole_round = { - "item_id": f"R{entry['round']}-range", - "commits": list(ordered_range), - } - if not args.dry_run: - # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push - # できずに終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits(state, whole_round, args.dry_run) - if not args.dry_run: - # 取り消し後の状態を新しい起点にする。叩き直しても範囲が空になり、 - # 取り消しコミット自体を「未割当」として再び戻すことがない。 - entry["apply_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "apply_base_sha", args.dry_run, + ) entry["apply"] = { "applied": [], "failed": list(entry["items"]), "base_sha": entry.get("apply_base_sha"), "head_sha": head_sha, @@ -1250,51 +1340,15 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: statefile.save(path, state) sys.exit(2) - applied: list[str] = [] - failed: list[str] = [] scope = state.get("target_scope") or [] # **判定はその都度残す。** まとめて最後に保存すると、取り消しの途中で中断した # ときに適用の記録が一切残らず、どのコミットが検証を通ったのかを状態から # 復元できなくなる。再開可能性は収束ループの前提なので、ここが崩れると # 中断からの復帰手段が無くなる。 - progress: list[dict[str, Any]] = [] - entry["apply_progress"] = progress - for item_id in entry["items"]: - item = _find_item(state, item_id) - got = reported.get(item_id) - if got is None: - problem = "適用結果に項目がありません" - facts: list[dict[str, Any]] = [] - else: - facts = collect_commit_facts( - work, _reported_shas(got), in_range, test_command, head_branch, - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - problem = verify_apply_item(item, facts, scope) - if problem: - item["status"] = "abandoned" - item["failure_reason"] = problem - item["test_failed"] = bool(got and "テストが成功していません" in problem) - item["budget_exceeded"] = bool(got and "差分予算" in problem) - item["out_of_scope"] = bool(got and "対象範囲の外" in problem) - # 取り消しは全項目の判定が出そろってから**まとめて**行う。項目ごとに - # その場で戻すと、まだ判定していない項目のコミットと競合する。 - item["commits"] = _reported_shas(got) - failed.append(item_id) - info(f"❌ {item_id}: {problem}") - else: - item["status"] = "reviewing" - item["commits"] = _reported_shas(got) - item["diff_lines"] = sum(_safe_int(c.get("diff_lines")) for c in facts) - applied.append(item_id) - info(f"✅ {item_id}: {len(item['commits'])} コミット / {item['diff_lines']} 行") - progress.append({ - "item_id": item_id, "at": statefile.now(), - "result": "failed" if problem else "ok", - "reason": problem, "commits": list(item.get("commits") or []), - }) - if not args.dry_run: - statefile.save(path, state) + applied, failed = _verify_and_record_items( + path, state, entry, work, reported, in_range, scope, test_command, head_branch, + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), args.dry_run, + ) entry["apply"] = { "applied": applied, @@ -1626,6 +1680,34 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: statefile.save(path, state) +def _validate_fix_commits( + work: str, + ordered_range: list[str], + reported_shas: list[str], + scope: list[str], + test_command: str, + head_branch: str, + timeout: int, +) -> tuple[list[tuple[str, str]], list[str], list[str]]: + """修正コミットの申告漏れと手順適合を検証する。""" + reported_full = _resolve_reported_to_full(work, reported_shas) + unassigned = sorted(set(ordered_range) - reported_full) + facts = collect_commit_facts( + work, reported_shas, set(ordered_range), test_command, head_branch, timeout, + ) + accepted: list[tuple[str, str]] = [] + problems: list[str] = [] + for commit in facts: + item_id = (commit.get("trailers") or {}).get("Item-Id") + problem = verify_fix_commit(commit, scope) + if problem: + problems.append(problem) + info(f"❌ 修正コミットが手順を満たしていません: {problem}") + continue + accepted.append((item_id, commit["sha"])) + return accepted, unassigned, problems + + def cmd_merge_fix(args: argparse.Namespace) -> None: """Step 6 — 修正結果を取り込み、修正ラウンドを 1 つ進める。""" path, state = _load(args.id) @@ -1705,35 +1787,16 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: # 適用と同じく、**範囲のコミットは全て申告されていること**を求める。 # 申告から漏れた修正コミットは検証を受けないまま Pull Request に残る。 - reported_full = { - full for full in ( - _git_out(work, ["rev-parse", "--verify", f"{s}^{{commit}}"]) - for s in reported_shas - ) if full - } - unassigned = sorted(set(ordered_range) - reported_full) - - facts = collect_commit_facts( - work, reported_shas, set(ordered_range), - baseline.get("command") or "true", state["head_branch"], - _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), - ) - # **不正なコミットが 1 件でもあれば、修正ラウンドの範囲ごと取り消す。** # 状態を記録しないだけでは、未検証の変更が Pull Request に残り続ける # （見送りの対象にもならない）。どのコミットが安全かは決められないので、 # 適用フェーズの未割当コミットと同じ扱いにする。 - problems: list[str] = [] - accepted: list[tuple[str, str]] = [] # (item_id, sha) needs_push = False - for commit in facts: - item_id = (commit.get("trailers") or {}).get("Item-Id") - problem = verify_fix_commit(commit, state.get("target_scope") or []) - if problem: - problems.append(problem) - info(f"❌ 修正コミットが手順を満たしていません: {problem}") - continue - accepted.append((item_id, commit["sha"])) + accepted, unassigned, problems = _validate_fix_commits( + work, ordered_range, reported_shas, state.get("target_scope") or [], + baseline.get("command") or "true", state["head_branch"], + _safe_int(state.get("test_timeout"), DEFAULT_TEST_TIMEOUT), + ) if unassigned: info( @@ -1747,19 +1810,9 @@ def cmd_merge_fix(args: argparse.Namespace) -> None: info("検証を通らない変更を残さないため、この修正ラウンドの範囲を取り消します") # **取り消しへ着手する前に印を立てる。** 取り消しは済んだのに push できずに # 終わると、未検証の変更が Pull Request に残ったままになる。 - entry["pending_push"] = True - statefile.save(path, state) - _revert_item_commits( - state, - {"item_id": f"R{entry['round']}-fix{entry['fix_rounds'] + 1}", - "commits": list(ordered_range)}, - dry_run=False, + _revert_range_with_recovery( + path, state, entry, work, ordered_range, "fix_base_sha", ) - # 取り消し後の状態を新しい起点にし、**その場で保存する**。ここで保存せずに - # 落ちると、次の実行は古い起点から範囲を取り直して取り消しコミット自体を - # 「未申告」と判定し、**取り消しを取り消して**しまう。 - entry["fix_base_sha"] = _git_out(work, ["rev-parse", "HEAD"]) - statefile.save(path, state) # **push は保存のあと。** ここで push して失敗すると、取り消しコミットは # ローカルに残るのに起点の更新が保存されず、叩き直しで二重に取り消してしまう。 needs_push = True diff --git a/plugins/ndf-shared/skills/cross-review/scripts/lib/metrics.py b/plugins/ndf-shared/skills/cross-review/scripts/lib/metrics.py index c2b576b4..4cc4e661 100644 --- a/plugins/ndf-shared/skills/cross-review/scripts/lib/metrics.py +++ b/plugins/ndf-shared/skills/cross-review/scripts/lib/metrics.py @@ -39,6 +39,93 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _update_impl_metrics( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドの実装担当指標とモデル警告を更新する。""" + round_no = entry.get("round") + impl_runtime = entry["impl"] + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + observed = impl_model.get("observed") + + warning = _models.mismatch_warning(impl_runtime, requested, observed) + if warning: + unmeasured.append(f"round {round_no}: {warning}") + if not _models.is_measurable(impl_runtime, requested): + unmeasured.append( + f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" + "実装担当の集計から分離する" + ) + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + bucket["rounds"] += 1 + bucket["seconds"] += _duration(entry, ("apply", "fix")) + round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] + bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") + bucket["abandoned"] += sum( + 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} + ) + bucket["budget_exceeded"] += sum(1 for i in round_items if i.get("budget_exceeded")) + bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) + bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + + reviews = _round_reviews(entry) + if reviews: + approved_first = all( + _verdict(reviews[0], reviewer) == "APPROVE" + for reviewer in entry.get("reviewers", []) + ) + bucket["first_review_total"] += 1 + bucket["first_review_approved"] += 1 if approved_first else 0 + + +def _update_reviewer_metrics( + entry: dict[str, Any], + name: str, + reviews: list[dict[str, Any]], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """ラウンドのレビュー担当 1 名分の指標と警告を更新する。""" + spec = (entry.get("reviewer_models") or {}).get(name) or {} + requested = spec.get("requested") + observed = spec.get("observed") + warning = _models.mismatch_warning(name, requested, observed) + if warning: + unmeasured.append(f"round {entry.get('round')}: {warning}") + if not _models.is_measurable(name, requested): + unmeasured.append( + f"round {entry.get('round')}: {name} が既定モデル（auto）で動いたため、" + "レビュー担当の集計から分離する" + ) + bucket = reviewer.setdefault(_key(name, requested), _new_reviewer_bucket()) + bucket["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) + for review in reviews: + if _verdict(review, name) is None: + continue + bucket["reviews"] += 1 + findings = [ + finding for finding in review.get("findings", []) + if isinstance(finding, dict) and finding.get("reviewer") == name + ] + bucket["findings"] += len(findings) + bucket["findings_resolved"] += sum( + 1 for finding in findings if finding.get("resolved") + ) + for other in (candidate for candidate in entry.get("reviewers", []) if candidate != name): + other_verdict = _verdict(review, other) + if other_verdict is None: + continue + bucket["verdict_pairs"] += 1 + bucket["verdict_agreements"] += int( + other_verdict == _verdict(review, name) + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +139,14 @@ def aggregate(state: dict[str, Any]) -> dict[str, Any]: unmeasured: list[str] = [] for entry in state.get("rounds", []): - round_no = entry.get("round") impl_runtime = entry.get("impl") if not impl_runtime: continue - impl_model = (entry.get("impl_model") or {}) - requested = impl_model.get("requested") - observed = impl_model.get("observed") - - warning = _models.mismatch_warning(impl_runtime, requested, observed) - if warning: - unmeasured.append(f"round {round_no}: {warning}") - if not _models.is_measurable(impl_runtime, requested): - unmeasured.append( - f"round {round_no}: {impl_runtime} が既定モデル（auto）で動いたため、" - "実装担当の集計から分離する" - ) - - bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) - bucket["rounds"] += 1 - bucket["seconds"] += _duration(entry, ("apply", "fix")) - - round_items = [items_by_id[i] for i in entry.get("items", []) if i in items_by_id] - bucket["applied"] += sum(1 for i in round_items if i.get("status") == "done") - bucket["abandoned"] += sum( - 1 for i in round_items if i.get("status") in {"abandoned", "blocked"} - ) - bucket["budget_exceeded"] += sum( - 1 for i in round_items if i.get("budget_exceeded") - ) - bucket["test_failed"] += sum(1 for i in round_items if i.get("test_failed")) - bucket["fix_rounds"] += int(entry.get("fix_rounds") or 0) + _update_impl_metrics(entry, items_by_id, impl, unmeasured) reviews = _round_reviews(entry) - if reviews: - first = reviews[0] - approved_first = all( - _verdict(first, r) == "APPROVE" for r in entry.get("reviewers", []) - ) - bucket["first_review_total"] += 1 - bucket["first_review_approved"] += 1 if approved_first else 0 - - reviewer_models = entry.get("reviewer_models") or {} for name in entry.get("reviewers", []): - spec = reviewer_models.get(name) or {} - r_requested = spec.get("requested") - r_observed = spec.get("observed") - r_warning = _models.mismatch_warning(name, r_requested, r_observed) - if r_warning: - unmeasured.append(f"round {round_no}: {r_warning}") - if not _models.is_measurable(name, r_requested): - unmeasured.append( - f"round {round_no}: {name} が既定モデル（auto）で動いたため、" - "レビュー担当の集計から分離する" - ) - rb = reviewer.setdefault(_key(name, r_requested), _new_reviewer_bucket()) - # 担当ごとの所要時間があればそれを使う。無ければ 0 のままにする。 - # ラウンドの合計を配ると 2 者分を両方に数えてしまい、比較が成り立たない。 - rb["seconds"] += float((entry.get("reviewer_seconds") or {}).get(name, 0)) - for review in reviews: - if _verdict(review, name) is None: - continue - rb["reviews"] += 1 - findings = [ - f for f in review.get("findings", []) - if isinstance(f, dict) and f.get("reviewer") == name - ] - rb["findings"] += len(findings) - rb["findings_resolved"] += sum(1 for f in findings if f.get("resolved")) - others = [o for o in entry.get("reviewers", []) if o != name] - for other in others: - other_verdict = _verdict(review, other) - if other_verdict is None: - continue - rb["verdict_pairs"] += 1 - rb["verdict_agreements"] += ( - 1 if other_verdict == _verdict(review, name) else 0 - ) + _update_reviewer_metrics(entry, name, reviews, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())},