diff --git a/issues/issue-113-cross-refactoring-4th-trial-report.md b/issues/issue-113-cross-refactoring-4th-trial-report.md new file mode 100644 index 00000000..cfeb055f --- /dev/null +++ b/issues/issue-113-cross-refactoring-4th-trial-report.md @@ -0,0 +1,158 @@ +# cross-refactoring 4 回目の実機試行の結果 + +Pull Request #127(NDF v8.5.0)で直した 5 件を実機で確かめた記録である。 +対象は Pull Request #128(Draft のまま)。 + +経緯は次の 3 つにある。 + +- [issue-113-cross-refactoring-trial-report.md](issue-113-cross-refactoring-trial-report.md) — 1 回目 +- [issue-113-cross-refactoring-retrial.md](issue-113-cross-refactoring-retrial.md) — 2 回目 +- [issue-113-cross-refactoring-re-retrial.md](issue-113-cross-refactoring-re-retrial.md) — 3 回目 + +## 結果 + +**3 回目に見つけた 4 件のうち、実機で踏める 3 件は直っていた。** +一方でラウンド 2 の再レビューで**進行が終わらない経路**に入り、手で止めた。 +原因は 3 回目より前から存在した欠落で、レビュー結果が 2 回続けて欠けたときにだけ現れる。 + +## 実行条件 + +| 項目 | 値 | +| --- | --- | +| ホスト | Claude Code(提案・レビューには不参加) | +| 提案・レビュー | codex / gemini / kiro | +| 適用の母集合 | claude / codex / kiro | +| 使用する版 | プラグインキャッシュの v8.5.0 | +| ラウンド上限 | 3(実際は 2 ラウンド目で停止) | +| 着手前のテスト | 463 passed | + +```bash +/ndf:cross-refactoring 128 \ + --scope plugins/ndf-shared/skills/cross-refactoring/scripts \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/scripts/lib \ + plugins/ndf-shared/skills/cross-review/tests \ + --sync-command "bash scripts/build-runtime-plugins.sh" \ + --baseline-test "uv run --with pytest python -m pytest \ + plugins/ndf-shared/skills/cross-refactoring/tests \ + plugins/ndf-shared/skills/cross-review/tests -q" \ + --max-outer-rounds 3 +``` + +## 到達点 + +```mermaid +flowchart TD + Init([初期化・作業ディレクトリ・Skill 配置]):::ok --> P1 + P1["R1 提案(3 CLI 並列)11 件"]:::ok --> M1 + M1["統合 5 件 → 採用 5 件"]:::ok --> A1 + A1["R1 適用(codex)13 コミット / 901 秒"]:::ok --> V1 + V1["検証: 採用 2 / 失敗 3"]:::ok --> D1 + D1["積み直し競合 → ラウンド全件へ退避
取り消し 13 コミット"]:::ok --> P2 + P2["R2 提案(同期後)8 件 → 採用 5 件"]:::ok --> A2 + A2["R2 適用(kiro)5 コミット"]:::ok --> V2 + V2["検証: 採用 5 / 失敗 0"]:::ok --> S2 + S2["生成物を同期(9 ファイル)→ 進行側が push"]:::ok --> RV2 + RV2["R2 レビュー: gemini 承認 / codex 結果なし"]:::ng --> LOOP + LOOP["差し戻し上限 → 変更要求として扱う
修正フェーズ → 起点なしで弾かれる
修正ラウンドが進まず同じ経路を反復"]:::ng --> Stop + Stop([手で停止]):::stop + + classDef ok fill:#dfd,stroke:#383 + classDef ng fill:#fdd,stroke:#933 + classDef stop fill:#eef,stroke:#557 +``` + +## v8.5.0 の修正の確認 + +| # | 直したこと | 観測 | 判定 | +| --- | --- | --- | --- | +| 12 | 生成物の同期コミット | `🔧 生成物を同期しました(bash scripts/build-runtime-plugins.sh / 9 ファイル)` が出て、`Chore: 生成物を同期する(cross-refactoring 進行側)` が push まで届いた | 成立 | +| 13 | 同期の後段の失敗 | 実機では後段が落ちなかったため未到達。旧版と新版を並べた再現で確認した(下記) | 別手段で確認 | +| 14 | 実装担当のコミット | 適用フェーズで codex / kiro とも手順書どおりコミットを作れた。作業ツリーに残骸は出なかった | 成立 | +| 15 | 見送り後の読み取り同期 | ラウンド 1 の全件取り消しで HEAD が `4642657` → `a66ed1e` へ動いた後、提案の直前に読み取り用 3 つとも `a66ed1e` へ同期された。ラウンド 2 の提案は消えた関数を 1 件も指していない | 成立 | + +修正 13 は実機で踏めなかったため、`pre-commit` で `git commit` を必ず落とす作業ツリーを +作り、旧版と新版で `_sync_generated` を呼び分けて比べた。 + +| 版 | 中断後の作業ツリー | +| --- | --- | +| 8.3.0 | `' M generated.txt'` が残る | +| 8.5.0 | 空(`reset --hard` と `clean -fd` で戻る) | + +修正 12 も同じ方法で、旧版が `_worktree_changes()` から `{'enerated.txt': 'M '}` を返して +`git add` が `pathspec ... did not match any files` で落ちること、新版が +`{'generated.txt': ' M'}` を返して通ることを確認した。 + +## 見つけた不具合 + +### 16. レビュー結果が欠け続けると進行が終わらない + +**進行が止まらない。** レビュー結果が 2 回続けて欠けた後、修正フェーズと再レビューを +無限に往復する。実測ではラウンド 2 で 3 巡し、手で停止するまで終わらなかった。 + +`cmd_judge_review` には変更要求を返す出口が 2 つある。 + +| 出口 | `fix_base_sha` の記録 | +| --- | --- | +| 通常の変更要求 | する | +| 差し戻しの上限に達したため変更要求として扱う | **しない** | + +`cmd_merge_fix` は `commits_in_range(work, entry.get("fix_base_sha"), head)` で修正の +範囲を求め、起点が空なら `None` が返る。範囲を確定できないので終了コード 2 で弾かれるが、 +このとき `fix_rounds` は増えない。`cmd_should_abandon` は `fix_rounds` が上限に達したかで +見送りへ移るため、**上限に永久に到達しない**。 + +``` +修正ラウンド 0 / 3 — まだ修正します +❌ 修正の範囲を確定できませんでした(起点 None / HEAD 0b45dc9...)。検証できない修正は採りません +===== レビュー ===== +(以降くり返し) +``` + +この欠落は v8.5.0 で入ったものではない。v8.3.0 でも同じ位置にあり、レビュー結果が +2 回続けて欠ける条件を過去 3 回の試行が満たさなかったため現れていない。 + +**直し方**: 差し戻し上限の出口でも `fix_base_sha` を記録する。あわせて、 +`merge-fix` が範囲を確定できずに終わったときも `fix_rounds` を進め、見送りへ到達させる。 + +### 17. 実装担当に直せない指摘が渡る + +レビュー結果の欠落は、`findings` へ次の 1 件として記録される。 + +```json +{"reviewer": "cross-refactoring", "item_id": null, + "summary": "レビュー結果の形式が 2 回続けて不正だった: codex のレビュー結果がありません", + "resolved": false} +``` + +対象の改善項目が無く、コードのどこを直せば解決するのかも書かれていない。実装担当は +これを受け取っても何もできず、修正フェーズが空回りする。不具合 16 を直しても、 +承認済みの項目が「レビュー担当が動かなかった」という理由だけで見送りへ進む。 + +**直し方**: レビュー担当が結果を残さなかったことは、実装担当への変更要求ではなく +**進行側の問題**として扱う。片方の結果が欠けたまま上限に達したら、残る 1 者の判定で +決めるか、進行を中断して利用者に判断を渡す。 + +## 運用上の観測 + +| 事象 | 内容 | +| --- | --- | +| codex がレビュー結果を残さない | ラウンド 2 で 3 回とも、手順書を読み進めた後に何も投稿せず終了した。`sentinel` は書かれるため監視は「完了」と見なし、`result.json` の不在だけが手掛かりになる | +| 差分予算による失敗 | ラウンド 1 の 2 件が予算超過で落ちた(265 行 / 240 行、183 行 / 180 行)。`long_method` の抽出は見積より膨らみやすい | +| 積み直しの競合 | ラウンド 1 は同じファイルの隣接行を触る項目が重なり、3 回目と同じくラウンド全件へ退避した | + +## 集計 + +| 実装担当 | 担当R | 適用 | 見送り | 予算超過率 | 所要秒 | +| --- | ---: | ---: | ---: | ---: | ---: | +| codex / default | 1 | 0 | 5 | 0.40 | 1800 | +| kiro / default | 1 | 0 | 0 | — | 320 | + +| レビュー担当 | レビュー回数 | 指摘 | 所要秒 | +| --- | ---: | ---: | ---: | +| codex / default | 0 | 0 | 0 | +| gemini / default | 3 | 0 | 480 | +| kiro / default | 0 | 0 | 0 | + +kiro は既定モデル(auto)で動いたため、両ラウンドとも比較用の集計から分離された。 +1 回の実行内の値なので、ランタイムの優劣を読む材料にはならない。 diff --git a/issues/issue-113-cross-refactoring-4th-trial.md b/issues/issue-113-cross-refactoring-4th-trial.md new file mode 100644 index 00000000..254d3400 --- /dev/null +++ b/issues/issue-113-cross-refactoring-4th-trial.md @@ -0,0 +1,51 @@ +# cross-refactoring 4 回目の実機試行 + +Pull Request #127(NDF v8.5.0)で直した 5 件が実機で成立するかを確かめる。 + +経緯は次の 3 つにある。 + +- [issue-113-cross-refactoring-trial-report.md](issue-113-cross-refactoring-trial-report.md) — 1 回目 +- [issue-113-cross-refactoring-retrial.md](issue-113-cross-refactoring-retrial.md) — 2 回目 +- [issue-113-cross-refactoring-re-retrial.md](issue-113-cross-refactoring-re-retrial.md) — 3 回目 + +## 目的 + +3 回目で見つけた 4 件と、投稿の成否を突き合わせない課題を直した。生成物を持つ +リポジトリで進行が止まる不具合 12 は、`--sync-command` を渡す構成でしか踏まないため、 +同じ構成でもう一度通す。 + +## 確かめること + +| # | 直したこと | 何が観測できれば通ったと言えるか | +| --- | --- | --- | +| 12 | 生成物の同期コミット | `Chore: 生成物を同期する(cross-refactoring 進行側)` が積まれ、`git add` が落ちない | +| 13 | 同期の後段の失敗 | 失敗しても作業ツリーが綺麗に戻り、次の実行が清浄性の検査で止まらない | +| 14 | 実装担当のコミット | 手順書の迂回手段でコミットが作れる。作れなくても取り込みが 0 件として続行する | +| 15 | 見送り後の読み取り同期 | 次ラウンドの提案が、取り消しで消えた対象を指さない | +| — | 投稿の成否の突き合わせ | cross-review が申告と GitHub 側の実数を照合する | + +## 対象範囲 + +| パス | 内容 | +| --- | --- | +| `plugins/ndf-shared/skills/cross-refactoring/scripts/` | `refactor.py` / `launch-cli.sh` / `prepare-worktrees.sh` | +| `plugins/ndf-shared/skills/cross-refactoring/tests/` | 現状固定テスト | +| `plugins/ndf-shared/skills/cross-review/scripts/lib/` | 収束ループの共通層 | +| `plugins/ndf-shared/skills/cross-review/tests/` | 現状固定テスト | + +## 実行条件 + +| 項目 | 値 | +| --- | --- | +| ホスト | Claude Code(提案・レビューには不参加) | +| 提案・レビュー | codex / gemini / kiro | +| 適用の母集合 | claude / codex / kiro | +| 使用する版 | リポジトリ内の `plugins/ndf-claude`(v8.5.0) | +| ラウンド上限 | 3 | +| 着手前のテスト | 463 passed | + +## 注意 + +`plugins/ndf-shared/` は編集元であり、`scripts/build-runtime-plugins.sh` で +`ndf-claude` / `ndf-codex` / `ndf-kiro` へ同期する。同期は進行側の責務のため +`--sync-command` で渡す。 diff --git a/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py index 2aa3c0d2..186b1274 100755 --- a/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-claude/skills/cross-refactoring/scripts/refactor.py @@ -976,6 +976,28 @@ def cmd_start_round(args: argparse.Namespace) -> None: ) +def _load_proposals( + state: dict[str, Any], entry: dict[str, Any] +) -> dict[str, list[dict[str, Any]]]: + """各ランタイムの提案結果を読み込み、ランタイム→項目リストの辞書で返す。""" + proposals: dict[str, list[dict[str, Any]]] = {} + for runtime in state["runtimes"]: + result = _result_path( + state, runtime, + stem_for(runtime, "propose", state["id"], entry["round"]), + ) + payload = _read_result_lenient(result, runtime) + if payload is None: + proposals[runtime] = [] + entry["proposed"][runtime] = 0 + continue + items = payload.get("items") + proposals[runtime] = [i for i in items if isinstance(i, dict)] \ + if isinstance(items, list) else [] + entry["proposed"][runtime] = len(proposals[runtime]) + return proposals + + def cmd_merge_proposals(args: argparse.Namespace) -> None: """Step 3 — 提案をマージして改善項目を作る。 @@ -1000,34 +1022,7 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) return - proposals: dict[str, list[dict[str, Any]]] = {} - for runtime in state["runtimes"]: - result = _result_path( - state, runtime, - stem_for(runtime, "propose", state["id"], entry["round"]), - ) - if not result.exists(): - info(f"⚠ {runtime} の提案結果がありません: {result}") - continue - try: - payload = json.loads(result.read_text(encoding="utf-8")) - except json.JSONDecodeError as e: - info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") - continue - if not isinstance(payload, dict): - # 配列や数値のまま `payload.get(...)` を呼ぶと落ちる。 - # 提案は無かったものとして続ける(1 者の不調で全体を止めない)。 - info( - f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" - f"({type(payload).__name__})。提案なしとして扱います" - ) - proposals[runtime] = [] - entry["proposed"][runtime] = 0 - continue - items = payload.get("items") - proposals[runtime] = [i for i in items if isinstance(i, dict)] \ - if isinstance(items, list) else [] - entry["proposed"][runtime] = len(proposals[runtime]) + proposals = _load_proposals(state, entry) excluded = { (d["path"], d["symbol"], d["smell"]) for d in state["deferred_items"] @@ -1259,9 +1254,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 残さないと同じ提案が次のラウンドで再び採用される。 _defer_abandoned_items(state, entry) statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) sys.exit(2) applied: list[str] = [] @@ -1340,11 +1333,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # **全項目が通ったときも進行側が公開する。** 実装担当は push しないため、 # ここで公開しないとレビュー担当が Pull Request 上の差分へ指摘を書けない。 entry["apply"]["merged_at"] = statefile.now() - entry["pending_push"] = True - statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) if not applied: info("全項目が失敗したため、このラウンドのレビューは行いません") @@ -1444,9 +1433,7 @@ def _apply_drop( entry["pending_drop"] = [] entry["apply"]["merged_at"] = statefile.now() statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) return applied @@ -1667,9 +1654,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: entry["pending_drop"] = [] state["phase"] = "propose" statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) def cmd_merge_fix(args: argparse.Namespace) -> None: @@ -2760,6 +2745,29 @@ def _find_item( return None +def _read_result_lenient(path: pathlib.Path, runtime: str) -> Optional[dict[str, Any]]: + """結果ファイルを読む。不在・壊れ・非オブジェクトの場合は None を返す。 + + 1 者の不調で全体を止めたくない箇所(提案のマージなど)向け。致命的な箇所には + `_read_result` を使う。 + """ + if not path.exists(): + info(f"⚠ {runtime} の提案結果がありません: {path}") + return None + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") + return None + if not isinstance(payload, dict): + info( + f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" + f"({type(payload).__name__})。提案なしとして扱います" + ) + return None + return payload + + def _read_result(path: pathlib.Path, runtime: str) -> dict[str, Any]: """結果ファイルを読む。**JSON オブジェクトでなければ失敗させる。** 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..9388a282 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,111 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _aggregate_impl_round( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分の実装担当集計を *impl* bucket へ加算する。""" + round_no = entry.get("round") + impl_runtime = entry.get("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) + + +def _record_first_review( + entry: dict[str, Any], + impl: dict[str, dict[str, Any]], +) -> None: + """初回レビュー結果を実装担当 bucket へ反映する。""" + impl_runtime = entry.get("impl") + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + 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 + + +def _aggregate_reviewer_round( + entry: dict[str, Any], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分のレビュー担当集計を *reviewer* bucket へ加算する。""" + round_no = entry.get("round") + reviewer_models = entry.get("reviewer_models") or {} + reviews = _round_reviews(entry) + + 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 + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +157,12 @@ 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) - - 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 - ) + _aggregate_impl_round(entry, items_by_id, impl, unmeasured) + _record_first_review(entry, impl) + _aggregate_reviewer_round(entry, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-claude/skills/cross-review/scripts/lib/monitor.py b/plugins/ndf-claude/skills/cross-review/scripts/lib/monitor.py index 401a760f..b7df5831 100755 --- a/plugins/ndf-claude/skills/cross-review/scripts/lib/monitor.py +++ b/plugins/ndf-claude/skills/cross-review/scripts/lib/monitor.py @@ -473,6 +473,44 @@ def _pid_cmdline_matches(pid: int, expected: str) -> Optional[bool]: return None +def _read_log_tail_text(path: pathlib.Path) -> Optional[str]: + """err.log の末尾 200KB を読み、ANSI を除去したテキストを返す。 + + ファイルが存在しない・読めない場合は None。 + """ + if not path.exists(): + return None + try: + sz = path.stat().st_size + with path.open("rb") as f: + if sz > 200 * 1024: + f.seek(sz - 200 * 1024) + data = f.read().decode("utf-8", errors="replace") + except OSError: + return None + return _strip_ansi(data) + + +def _matched_line(data: str, m: re.Match[str]) -> tuple[str, int]: + """マッチ位置から行全体を切り出し、(行テキスト, 行の開始オフセット) を返す。""" + line_start = data.rfind("\n", 0, m.start()) + 1 + line_end = data.find("\n", m.end()) + line_end = line_end if line_end != -1 else len(data) + return data[line_start:line_end], line_start + + +def _is_benign_match( + line: str, match_start_in_line: int, match_end_in_line: int, + benign_patterns: list[re.Pattern[str]], +) -> bool: + """マッチが benign(誤検知)かどうかを判定する。""" + if any(b.search(line) for b in benign_patterns): + return True + if _match_is_quoted(line, match_start_in_line, match_end_in_line): + return True + return False + + def _scan_patterns( path: pathlib.Path, patterns: list[re.Pattern[str]], @@ -492,32 +530,23 @@ def _scan_patterns( 扱いしてしまった (例: `Error in: mcpServers.serena\\n...\\nTraceback ...` で Traceback が誤抑制された)。 """ - if not path.exists(): + data = _read_log_tail_text(path) + if data is None: return None - try: - sz = path.stat().st_size - with path.open("rb") as f: - if sz > 200 * 1024: - f.seek(sz - 200 * 1024) - data = f.read().decode("utf-8", errors="replace") - except OSError: - return None - data = _strip_ansi(data) benign_patterns = EARLY_ERROR_BENIGN if benign is None else benign for pat in patterns: for m in pat.finditer(data): - line_start = data.rfind("\n", 0, m.start()) + 1 - line_end = data.find("\n", m.end()) - line_end = line_end if line_end != -1 else len(data) - line = data[line_start:line_end] + line, line_start = _matched_line(data, m) # benign パターンはマッチ行そのものに当てる。markdown 引用や # `Error in: mcpServers.X` のような行単位パターンは「その行」だけを # 評価すれば判定可能で、文脈窓を広げると誤判定の原因になる。 - if any(b.search(line) for b in benign_patterns): - continue - # マッチ部位が backtick / 日本語「」 で引用されている場合も benign。 - if _match_is_quoted(line, m.start() - line_start, m.end() - line_start): + if _is_benign_match( + line, + m.start() - line_start, + m.end() - line_start, + benign_patterns, + ): continue return line.strip() return None diff --git a/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py index 2aa3c0d2..186b1274 100755 --- a/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-codex/skills/cross-refactoring/scripts/refactor.py @@ -976,6 +976,28 @@ def cmd_start_round(args: argparse.Namespace) -> None: ) +def _load_proposals( + state: dict[str, Any], entry: dict[str, Any] +) -> dict[str, list[dict[str, Any]]]: + """各ランタイムの提案結果を読み込み、ランタイム→項目リストの辞書で返す。""" + proposals: dict[str, list[dict[str, Any]]] = {} + for runtime in state["runtimes"]: + result = _result_path( + state, runtime, + stem_for(runtime, "propose", state["id"], entry["round"]), + ) + payload = _read_result_lenient(result, runtime) + if payload is None: + proposals[runtime] = [] + entry["proposed"][runtime] = 0 + continue + items = payload.get("items") + proposals[runtime] = [i for i in items if isinstance(i, dict)] \ + if isinstance(items, list) else [] + entry["proposed"][runtime] = len(proposals[runtime]) + return proposals + + def cmd_merge_proposals(args: argparse.Namespace) -> None: """Step 3 — 提案をマージして改善項目を作る。 @@ -1000,34 +1022,7 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) return - proposals: dict[str, list[dict[str, Any]]] = {} - for runtime in state["runtimes"]: - result = _result_path( - state, runtime, - stem_for(runtime, "propose", state["id"], entry["round"]), - ) - if not result.exists(): - info(f"⚠ {runtime} の提案結果がありません: {result}") - continue - try: - payload = json.loads(result.read_text(encoding="utf-8")) - except json.JSONDecodeError as e: - info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") - continue - if not isinstance(payload, dict): - # 配列や数値のまま `payload.get(...)` を呼ぶと落ちる。 - # 提案は無かったものとして続ける(1 者の不調で全体を止めない)。 - info( - f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" - f"({type(payload).__name__})。提案なしとして扱います" - ) - proposals[runtime] = [] - entry["proposed"][runtime] = 0 - continue - items = payload.get("items") - proposals[runtime] = [i for i in items if isinstance(i, dict)] \ - if isinstance(items, list) else [] - entry["proposed"][runtime] = len(proposals[runtime]) + proposals = _load_proposals(state, entry) excluded = { (d["path"], d["symbol"], d["smell"]) for d in state["deferred_items"] @@ -1259,9 +1254,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 残さないと同じ提案が次のラウンドで再び採用される。 _defer_abandoned_items(state, entry) statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) sys.exit(2) applied: list[str] = [] @@ -1340,11 +1333,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # **全項目が通ったときも進行側が公開する。** 実装担当は push しないため、 # ここで公開しないとレビュー担当が Pull Request 上の差分へ指摘を書けない。 entry["apply"]["merged_at"] = statefile.now() - entry["pending_push"] = True - statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) if not applied: info("全項目が失敗したため、このラウンドのレビューは行いません") @@ -1444,9 +1433,7 @@ def _apply_drop( entry["pending_drop"] = [] entry["apply"]["merged_at"] = statefile.now() statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) return applied @@ -1667,9 +1654,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: entry["pending_drop"] = [] state["phase"] = "propose" statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) def cmd_merge_fix(args: argparse.Namespace) -> None: @@ -2760,6 +2745,29 @@ def _find_item( return None +def _read_result_lenient(path: pathlib.Path, runtime: str) -> Optional[dict[str, Any]]: + """結果ファイルを読む。不在・壊れ・非オブジェクトの場合は None を返す。 + + 1 者の不調で全体を止めたくない箇所(提案のマージなど)向け。致命的な箇所には + `_read_result` を使う。 + """ + if not path.exists(): + info(f"⚠ {runtime} の提案結果がありません: {path}") + return None + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") + return None + if not isinstance(payload, dict): + info( + f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" + f"({type(payload).__name__})。提案なしとして扱います" + ) + return None + return payload + + def _read_result(path: pathlib.Path, runtime: str) -> dict[str, Any]: """結果ファイルを読む。**JSON オブジェクトでなければ失敗させる。** 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..9388a282 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,111 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _aggregate_impl_round( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分の実装担当集計を *impl* bucket へ加算する。""" + round_no = entry.get("round") + impl_runtime = entry.get("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) + + +def _record_first_review( + entry: dict[str, Any], + impl: dict[str, dict[str, Any]], +) -> None: + """初回レビュー結果を実装担当 bucket へ反映する。""" + impl_runtime = entry.get("impl") + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + 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 + + +def _aggregate_reviewer_round( + entry: dict[str, Any], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分のレビュー担当集計を *reviewer* bucket へ加算する。""" + round_no = entry.get("round") + reviewer_models = entry.get("reviewer_models") or {} + reviews = _round_reviews(entry) + + 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 + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +157,12 @@ 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) - - 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 - ) + _aggregate_impl_round(entry, items_by_id, impl, unmeasured) + _record_first_review(entry, impl) + _aggregate_reviewer_round(entry, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-codex/skills/cross-review/scripts/lib/monitor.py b/plugins/ndf-codex/skills/cross-review/scripts/lib/monitor.py index 401a760f..b7df5831 100755 --- a/plugins/ndf-codex/skills/cross-review/scripts/lib/monitor.py +++ b/plugins/ndf-codex/skills/cross-review/scripts/lib/monitor.py @@ -473,6 +473,44 @@ def _pid_cmdline_matches(pid: int, expected: str) -> Optional[bool]: return None +def _read_log_tail_text(path: pathlib.Path) -> Optional[str]: + """err.log の末尾 200KB を読み、ANSI を除去したテキストを返す。 + + ファイルが存在しない・読めない場合は None。 + """ + if not path.exists(): + return None + try: + sz = path.stat().st_size + with path.open("rb") as f: + if sz > 200 * 1024: + f.seek(sz - 200 * 1024) + data = f.read().decode("utf-8", errors="replace") + except OSError: + return None + return _strip_ansi(data) + + +def _matched_line(data: str, m: re.Match[str]) -> tuple[str, int]: + """マッチ位置から行全体を切り出し、(行テキスト, 行の開始オフセット) を返す。""" + line_start = data.rfind("\n", 0, m.start()) + 1 + line_end = data.find("\n", m.end()) + line_end = line_end if line_end != -1 else len(data) + return data[line_start:line_end], line_start + + +def _is_benign_match( + line: str, match_start_in_line: int, match_end_in_line: int, + benign_patterns: list[re.Pattern[str]], +) -> bool: + """マッチが benign(誤検知)かどうかを判定する。""" + if any(b.search(line) for b in benign_patterns): + return True + if _match_is_quoted(line, match_start_in_line, match_end_in_line): + return True + return False + + def _scan_patterns( path: pathlib.Path, patterns: list[re.Pattern[str]], @@ -492,32 +530,23 @@ def _scan_patterns( 扱いしてしまった (例: `Error in: mcpServers.serena\\n...\\nTraceback ...` で Traceback が誤抑制された)。 """ - if not path.exists(): + data = _read_log_tail_text(path) + if data is None: return None - try: - sz = path.stat().st_size - with path.open("rb") as f: - if sz > 200 * 1024: - f.seek(sz - 200 * 1024) - data = f.read().decode("utf-8", errors="replace") - except OSError: - return None - data = _strip_ansi(data) benign_patterns = EARLY_ERROR_BENIGN if benign is None else benign for pat in patterns: for m in pat.finditer(data): - line_start = data.rfind("\n", 0, m.start()) + 1 - line_end = data.find("\n", m.end()) - line_end = line_end if line_end != -1 else len(data) - line = data[line_start:line_end] + line, line_start = _matched_line(data, m) # benign パターンはマッチ行そのものに当てる。markdown 引用や # `Error in: mcpServers.X` のような行単位パターンは「その行」だけを # 評価すれば判定可能で、文脈窓を広げると誤判定の原因になる。 - if any(b.search(line) for b in benign_patterns): - continue - # マッチ部位が backtick / 日本語「」 で引用されている場合も benign。 - if _match_is_quoted(line, m.start() - line_start, m.end() - line_start): + if _is_benign_match( + line, + m.start() - line_start, + m.end() - line_start, + benign_patterns, + ): continue return line.strip() return None diff --git a/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py index 2aa3c0d2..186b1274 100755 --- a/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-kiro/skills/cross-refactoring/scripts/refactor.py @@ -976,6 +976,28 @@ def cmd_start_round(args: argparse.Namespace) -> None: ) +def _load_proposals( + state: dict[str, Any], entry: dict[str, Any] +) -> dict[str, list[dict[str, Any]]]: + """各ランタイムの提案結果を読み込み、ランタイム→項目リストの辞書で返す。""" + proposals: dict[str, list[dict[str, Any]]] = {} + for runtime in state["runtimes"]: + result = _result_path( + state, runtime, + stem_for(runtime, "propose", state["id"], entry["round"]), + ) + payload = _read_result_lenient(result, runtime) + if payload is None: + proposals[runtime] = [] + entry["proposed"][runtime] = 0 + continue + items = payload.get("items") + proposals[runtime] = [i for i in items if isinstance(i, dict)] \ + if isinstance(items, list) else [] + entry["proposed"][runtime] = len(proposals[runtime]) + return proposals + + def cmd_merge_proposals(args: argparse.Namespace) -> None: """Step 3 — 提案をマージして改善項目を作る。 @@ -1000,34 +1022,7 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) return - proposals: dict[str, list[dict[str, Any]]] = {} - for runtime in state["runtimes"]: - result = _result_path( - state, runtime, - stem_for(runtime, "propose", state["id"], entry["round"]), - ) - if not result.exists(): - info(f"⚠ {runtime} の提案結果がありません: {result}") - continue - try: - payload = json.loads(result.read_text(encoding="utf-8")) - except json.JSONDecodeError as e: - info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") - continue - if not isinstance(payload, dict): - # 配列や数値のまま `payload.get(...)` を呼ぶと落ちる。 - # 提案は無かったものとして続ける(1 者の不調で全体を止めない)。 - info( - f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" - f"({type(payload).__name__})。提案なしとして扱います" - ) - proposals[runtime] = [] - entry["proposed"][runtime] = 0 - continue - items = payload.get("items") - proposals[runtime] = [i for i in items if isinstance(i, dict)] \ - if isinstance(items, list) else [] - entry["proposed"][runtime] = len(proposals[runtime]) + proposals = _load_proposals(state, entry) excluded = { (d["path"], d["symbol"], d["smell"]) for d in state["deferred_items"] @@ -1259,9 +1254,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 残さないと同じ提案が次のラウンドで再び採用される。 _defer_abandoned_items(state, entry) statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) sys.exit(2) applied: list[str] = [] @@ -1340,11 +1333,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # **全項目が通ったときも進行側が公開する。** 実装担当は push しないため、 # ここで公開しないとレビュー担当が Pull Request 上の差分へ指摘を書けない。 entry["apply"]["merged_at"] = statefile.now() - entry["pending_push"] = True - statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) if not applied: info("全項目が失敗したため、このラウンドのレビューは行いません") @@ -1444,9 +1433,7 @@ def _apply_drop( entry["pending_drop"] = [] entry["apply"]["merged_at"] = statefile.now() statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) return applied @@ -1667,9 +1654,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: entry["pending_drop"] = [] state["phase"] = "propose" statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) def cmd_merge_fix(args: argparse.Namespace) -> None: @@ -2760,6 +2745,29 @@ def _find_item( return None +def _read_result_lenient(path: pathlib.Path, runtime: str) -> Optional[dict[str, Any]]: + """結果ファイルを読む。不在・壊れ・非オブジェクトの場合は None を返す。 + + 1 者の不調で全体を止めたくない箇所(提案のマージなど)向け。致命的な箇所には + `_read_result` を使う。 + """ + if not path.exists(): + info(f"⚠ {runtime} の提案結果がありません: {path}") + return None + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") + return None + if not isinstance(payload, dict): + info( + f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" + f"({type(payload).__name__})。提案なしとして扱います" + ) + return None + return payload + + def _read_result(path: pathlib.Path, runtime: str) -> dict[str, Any]: """結果ファイルを読む。**JSON オブジェクトでなければ失敗させる。** 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..9388a282 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,111 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _aggregate_impl_round( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分の実装担当集計を *impl* bucket へ加算する。""" + round_no = entry.get("round") + impl_runtime = entry.get("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) + + +def _record_first_review( + entry: dict[str, Any], + impl: dict[str, dict[str, Any]], +) -> None: + """初回レビュー結果を実装担当 bucket へ反映する。""" + impl_runtime = entry.get("impl") + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + 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 + + +def _aggregate_reviewer_round( + entry: dict[str, Any], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分のレビュー担当集計を *reviewer* bucket へ加算する。""" + round_no = entry.get("round") + reviewer_models = entry.get("reviewer_models") or {} + reviews = _round_reviews(entry) + + 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 + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +157,12 @@ 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) - - 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 - ) + _aggregate_impl_round(entry, items_by_id, impl, unmeasured) + _record_first_review(entry, impl) + _aggregate_reviewer_round(entry, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-kiro/skills/cross-review/scripts/lib/monitor.py b/plugins/ndf-kiro/skills/cross-review/scripts/lib/monitor.py index 401a760f..b7df5831 100755 --- a/plugins/ndf-kiro/skills/cross-review/scripts/lib/monitor.py +++ b/plugins/ndf-kiro/skills/cross-review/scripts/lib/monitor.py @@ -473,6 +473,44 @@ def _pid_cmdline_matches(pid: int, expected: str) -> Optional[bool]: return None +def _read_log_tail_text(path: pathlib.Path) -> Optional[str]: + """err.log の末尾 200KB を読み、ANSI を除去したテキストを返す。 + + ファイルが存在しない・読めない場合は None。 + """ + if not path.exists(): + return None + try: + sz = path.stat().st_size + with path.open("rb") as f: + if sz > 200 * 1024: + f.seek(sz - 200 * 1024) + data = f.read().decode("utf-8", errors="replace") + except OSError: + return None + return _strip_ansi(data) + + +def _matched_line(data: str, m: re.Match[str]) -> tuple[str, int]: + """マッチ位置から行全体を切り出し、(行テキスト, 行の開始オフセット) を返す。""" + line_start = data.rfind("\n", 0, m.start()) + 1 + line_end = data.find("\n", m.end()) + line_end = line_end if line_end != -1 else len(data) + return data[line_start:line_end], line_start + + +def _is_benign_match( + line: str, match_start_in_line: int, match_end_in_line: int, + benign_patterns: list[re.Pattern[str]], +) -> bool: + """マッチが benign(誤検知)かどうかを判定する。""" + if any(b.search(line) for b in benign_patterns): + return True + if _match_is_quoted(line, match_start_in_line, match_end_in_line): + return True + return False + + def _scan_patterns( path: pathlib.Path, patterns: list[re.Pattern[str]], @@ -492,32 +530,23 @@ def _scan_patterns( 扱いしてしまった (例: `Error in: mcpServers.serena\\n...\\nTraceback ...` で Traceback が誤抑制された)。 """ - if not path.exists(): + data = _read_log_tail_text(path) + if data is None: return None - try: - sz = path.stat().st_size - with path.open("rb") as f: - if sz > 200 * 1024: - f.seek(sz - 200 * 1024) - data = f.read().decode("utf-8", errors="replace") - except OSError: - return None - data = _strip_ansi(data) benign_patterns = EARLY_ERROR_BENIGN if benign is None else benign for pat in patterns: for m in pat.finditer(data): - line_start = data.rfind("\n", 0, m.start()) + 1 - line_end = data.find("\n", m.end()) - line_end = line_end if line_end != -1 else len(data) - line = data[line_start:line_end] + line, line_start = _matched_line(data, m) # benign パターンはマッチ行そのものに当てる。markdown 引用や # `Error in: mcpServers.X` のような行単位パターンは「その行」だけを # 評価すれば判定可能で、文脈窓を広げると誤判定の原因になる。 - if any(b.search(line) for b in benign_patterns): - continue - # マッチ部位が backtick / 日本語「」 で引用されている場合も benign。 - if _match_is_quoted(line, m.start() - line_start, m.end() - line_start): + if _is_benign_match( + line, + m.start() - line_start, + m.end() - line_start, + benign_patterns, + ): continue return line.strip() return None diff --git a/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py b/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py index 2aa3c0d2..186b1274 100755 --- a/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py +++ b/plugins/ndf-shared/skills/cross-refactoring/scripts/refactor.py @@ -976,6 +976,28 @@ def cmd_start_round(args: argparse.Namespace) -> None: ) +def _load_proposals( + state: dict[str, Any], entry: dict[str, Any] +) -> dict[str, list[dict[str, Any]]]: + """各ランタイムの提案結果を読み込み、ランタイム→項目リストの辞書で返す。""" + proposals: dict[str, list[dict[str, Any]]] = {} + for runtime in state["runtimes"]: + result = _result_path( + state, runtime, + stem_for(runtime, "propose", state["id"], entry["round"]), + ) + payload = _read_result_lenient(result, runtime) + if payload is None: + proposals[runtime] = [] + entry["proposed"][runtime] = 0 + continue + items = payload.get("items") + proposals[runtime] = [i for i in items if isinstance(i, dict)] \ + if isinstance(items, list) else [] + entry["proposed"][runtime] = len(proposals[runtime]) + return proposals + + def cmd_merge_proposals(args: argparse.Namespace) -> None: """Step 3 — 提案をマージして改善項目を作る。 @@ -1000,34 +1022,7 @@ def cmd_merge_proposals(args: argparse.Namespace) -> None: sys.exit(2) return - proposals: dict[str, list[dict[str, Any]]] = {} - for runtime in state["runtimes"]: - result = _result_path( - state, runtime, - stem_for(runtime, "propose", state["id"], entry["round"]), - ) - if not result.exists(): - info(f"⚠ {runtime} の提案結果がありません: {result}") - continue - try: - payload = json.loads(result.read_text(encoding="utf-8")) - except json.JSONDecodeError as e: - info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") - continue - if not isinstance(payload, dict): - # 配列や数値のまま `payload.get(...)` を呼ぶと落ちる。 - # 提案は無かったものとして続ける(1 者の不調で全体を止めない)。 - info( - f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" - f"({type(payload).__name__})。提案なしとして扱います" - ) - proposals[runtime] = [] - entry["proposed"][runtime] = 0 - continue - items = payload.get("items") - proposals[runtime] = [i for i in items if isinstance(i, dict)] \ - if isinstance(items, list) else [] - entry["proposed"][runtime] = len(proposals[runtime]) + proposals = _load_proposals(state, entry) excluded = { (d["path"], d["symbol"], d["smell"]) for d in state["deferred_items"] @@ -1259,9 +1254,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # 残さないと同じ提案が次のラウンドで再び採用される。 _defer_abandoned_items(state, entry) statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) sys.exit(2) applied: list[str] = [] @@ -1340,11 +1333,7 @@ def cmd_merge_apply(args: argparse.Namespace) -> None: # **全項目が通ったときも進行側が公開する。** 実装担当は push しないため、 # ここで公開しないとレビュー担当が Pull Request 上の差分へ指摘を書けない。 entry["apply"]["merged_at"] = statefile.now() - entry["pending_push"] = True - statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) if not applied: info("全項目が失敗したため、このラウンドのレビューは行いません") @@ -1444,9 +1433,7 @@ def _apply_drop( entry["pending_drop"] = [] entry["apply"]["merged_at"] = statefile.now() statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) return applied @@ -1667,9 +1654,7 @@ def cmd_abandon_items(args: argparse.Namespace) -> None: entry["pending_drop"] = [] state["phase"] = "propose" statefile.save(path, state) - _push_head(state) - entry["pending_push"] = False - statefile.save(path, state) + _push_with_retry_marker(path, state, entry) def cmd_merge_fix(args: argparse.Namespace) -> None: @@ -2760,6 +2745,29 @@ def _find_item( return None +def _read_result_lenient(path: pathlib.Path, runtime: str) -> Optional[dict[str, Any]]: + """結果ファイルを読む。不在・壊れ・非オブジェクトの場合は None を返す。 + + 1 者の不調で全体を止めたくない箇所(提案のマージなど)向け。致命的な箇所には + `_read_result` を使う。 + """ + if not path.exists(): + info(f"⚠ {runtime} の提案結果がありません: {path}") + return None + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except json.JSONDecodeError as e: + info(f"⚠ {runtime} の提案結果が JSON として読めません: {e}") + return None + if not isinstance(payload, dict): + info( + f"⚠ {runtime} の提案結果が JSON オブジェクトではありません" + f"({type(payload).__name__})。提案なしとして扱います" + ) + return None + return payload + + def _read_result(path: pathlib.Path, runtime: str) -> dict[str, Any]: """結果ファイルを読む。**JSON オブジェクトでなければ失敗させる。** 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..9388a282 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,111 @@ def _verdict(review: dict[str, Any], reviewer: str) -> Optional[str]: return value if isinstance(value, str) else None +def _aggregate_impl_round( + entry: dict[str, Any], + items_by_id: dict[str, dict[str, Any]], + impl: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分の実装担当集計を *impl* bucket へ加算する。""" + round_no = entry.get("round") + impl_runtime = entry.get("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) + + +def _record_first_review( + entry: dict[str, Any], + impl: dict[str, dict[str, Any]], +) -> None: + """初回レビュー結果を実装担当 bucket へ反映する。""" + impl_runtime = entry.get("impl") + impl_model = entry.get("impl_model") or {} + requested = impl_model.get("requested") + + bucket = impl.setdefault(_key(impl_runtime, requested), _new_impl_bucket()) + 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 + + +def _aggregate_reviewer_round( + entry: dict[str, Any], + reviewer: dict[str, dict[str, Any]], + unmeasured: list[str], +) -> None: + """1 ラウンド分のレビュー担当集計を *reviewer* bucket へ加算する。""" + round_no = entry.get("round") + reviewer_models = entry.get("reviewer_models") or {} + reviews = _round_reviews(entry) + + 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 + ) + + def aggregate(state: dict[str, Any]) -> dict[str, Any]: """状態ファイルから実装担当・レビュー担当それぞれの指標を出す。 @@ -52,83 +157,12 @@ 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) - - 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 - ) + _aggregate_impl_round(entry, items_by_id, impl, unmeasured) + _record_first_review(entry, impl) + _aggregate_reviewer_round(entry, reviewer, unmeasured) return { "impl": {k: _finish_impl(v) for k, v in sorted(impl.items())}, diff --git a/plugins/ndf-shared/skills/cross-review/scripts/lib/monitor.py b/plugins/ndf-shared/skills/cross-review/scripts/lib/monitor.py index 401a760f..b7df5831 100755 --- a/plugins/ndf-shared/skills/cross-review/scripts/lib/monitor.py +++ b/plugins/ndf-shared/skills/cross-review/scripts/lib/monitor.py @@ -473,6 +473,44 @@ def _pid_cmdline_matches(pid: int, expected: str) -> Optional[bool]: return None +def _read_log_tail_text(path: pathlib.Path) -> Optional[str]: + """err.log の末尾 200KB を読み、ANSI を除去したテキストを返す。 + + ファイルが存在しない・読めない場合は None。 + """ + if not path.exists(): + return None + try: + sz = path.stat().st_size + with path.open("rb") as f: + if sz > 200 * 1024: + f.seek(sz - 200 * 1024) + data = f.read().decode("utf-8", errors="replace") + except OSError: + return None + return _strip_ansi(data) + + +def _matched_line(data: str, m: re.Match[str]) -> tuple[str, int]: + """マッチ位置から行全体を切り出し、(行テキスト, 行の開始オフセット) を返す。""" + line_start = data.rfind("\n", 0, m.start()) + 1 + line_end = data.find("\n", m.end()) + line_end = line_end if line_end != -1 else len(data) + return data[line_start:line_end], line_start + + +def _is_benign_match( + line: str, match_start_in_line: int, match_end_in_line: int, + benign_patterns: list[re.Pattern[str]], +) -> bool: + """マッチが benign(誤検知)かどうかを判定する。""" + if any(b.search(line) for b in benign_patterns): + return True + if _match_is_quoted(line, match_start_in_line, match_end_in_line): + return True + return False + + def _scan_patterns( path: pathlib.Path, patterns: list[re.Pattern[str]], @@ -492,32 +530,23 @@ def _scan_patterns( 扱いしてしまった (例: `Error in: mcpServers.serena\\n...\\nTraceback ...` で Traceback が誤抑制された)。 """ - if not path.exists(): + data = _read_log_tail_text(path) + if data is None: return None - try: - sz = path.stat().st_size - with path.open("rb") as f: - if sz > 200 * 1024: - f.seek(sz - 200 * 1024) - data = f.read().decode("utf-8", errors="replace") - except OSError: - return None - data = _strip_ansi(data) benign_patterns = EARLY_ERROR_BENIGN if benign is None else benign for pat in patterns: for m in pat.finditer(data): - line_start = data.rfind("\n", 0, m.start()) + 1 - line_end = data.find("\n", m.end()) - line_end = line_end if line_end != -1 else len(data) - line = data[line_start:line_end] + line, line_start = _matched_line(data, m) # benign パターンはマッチ行そのものに当てる。markdown 引用や # `Error in: mcpServers.X` のような行単位パターンは「その行」だけを # 評価すれば判定可能で、文脈窓を広げると誤判定の原因になる。 - if any(b.search(line) for b in benign_patterns): - continue - # マッチ部位が backtick / 日本語「」 で引用されている場合も benign。 - if _match_is_quoted(line, m.start() - line_start, m.end() - line_start): + if _is_benign_match( + line, + m.start() - line_start, + m.end() - line_start, + benign_patterns, + ): continue return line.strip() return None