diff --git a/.console/log.md b/.console/log.md index 3b84e4fcb..783691d60 100644 --- a/.console/log.md +++ b/.console/log.md @@ -7001,3 +7001,19 @@ detect_regression/generate_alerts/save_snapshot/save_alert from d12_baseline — D12 gate confirms 0. (calculate_trend_slope/volatility/get_historical_data and categorize_alert/get_routes_for_alert remain genuinely unwired public API — stay baselined.) Observer suite 1389 green; ruff+ty+audit(B2)+doctor clean. + +## 2026-06-18 — COMPLETE merge-decision metrics: surface export_metrics_json + +Observer-plane #313 remediation. Investigation corrected the premise: the audit +flagged MergeDecisionInstrumenter/DecisionMetricsCollector as "never +instantiated", but they're ALREADY wired — pr_review_watcher calls the module- +level record_decision_outcome at 5 decision points → get_instrumenter() → +MergeDecisionInstrumenter records every merge decision. The genuine gap was +narrow: export_metrics_json / get_metrics_summary had NO caller, so the +collected metrics went nowhere. Wired it: _export_decision_metrics(status_dir) +writes get_instrumenter().export_metrics_json() to status_dir/ +merge_decision_metrics.json each poll cycle (alongside the heartbeat), +best-effort. Pruned export_metrics_json from d12_baseline (D12 gate confirms 0). +1 test; reviewer suite 113 green (tests/ root + the #322 dedicated CI job); +audit B2-env + doctor + D12 clean. Another false-positive corrected — the +instrumenter wasn't unwired, only its export surface was. diff --git a/.custodian/config.yaml b/.custodian/config.yaml index 629a5f522..4b76ba5a4 100644 --- a/.custodian/config.yaml +++ b/.custodian/config.yaml @@ -52,7 +52,6 @@ audit: - evaluate_alerts_dry_run - evaluate_per_collector_thresholds - export_all - - export_metrics_json - failure_reason_summary - filter_by_category - filter_by_extraction_status diff --git a/src/operations_center/entrypoints/pr_review_watcher/main.py b/src/operations_center/entrypoints/pr_review_watcher/main.py index cc0a3a869..1cb48dfd0 100644 --- a/src/operations_center/entrypoints/pr_review_watcher/main.py +++ b/src/operations_center/entrypoints/pr_review_watcher/main.py @@ -57,6 +57,7 @@ close_without_receipt_allowed, ) from operations_center.reviewer.instrumentation import ( + get_instrumenter, record_decision_outcome, record_ci_gate_defer, record_escalation, @@ -2512,6 +2513,22 @@ def _write_heartbeat(status_dir: Path) -> None: pass +def _export_decision_metrics(status_dir: Path) -> None: + """Surface the merge-decision metrics the instrumenter collects. + + pr_review_watcher records every merge decision via record_decision_outcome + (→ the global MergeDecisionInstrumenter), but the collected metrics had no + reader: export_metrics_json was never called. Write the instrumenter's + summary to the status dir each cycle so the metrics are actually observable. + Best-effort — metrics export must never break the review loop.""" + try: + status_dir.mkdir(parents=True, exist_ok=True) + metrics_path = status_dir / "merge_decision_metrics.json" + metrics_path.write_text(get_instrumenter().export_metrics_json(), encoding="utf-8") + except Exception: + pass + + # ── Poll cycle ──────────────────────────────────────────────────────────────── @@ -2643,6 +2660,7 @@ def main() -> int: logger.error("pr_review_watcher: error — %s", exc, exc_info=True) return 1 _write_heartbeat(status_dir) + _export_decision_metrics(status_dir) return 0 logger.info("pr_review_watcher: starting — poll_interval=%ds", args.poll_interval) @@ -2653,6 +2671,7 @@ def main() -> int: except Exception as exc: logger.error("pr_review_watcher: unhandled error — %s", exc, exc_info=True) _write_heartbeat(status_dir) + _export_decision_metrics(status_dir) time.sleep(args.poll_interval) diff --git a/tests/test_pr_review_watcher.py b/tests/test_pr_review_watcher.py index 08ca44326..9218436be 100644 --- a/tests/test_pr_review_watcher.py +++ b/tests/test_pr_review_watcher.py @@ -1284,6 +1284,20 @@ def test_write_heartbeat_creates_file(tmp_path: Path) -> None: assert data["status"] == "active" +def test_export_decision_metrics_writes_instrumenter_summary(tmp_path: Path) -> None: + # The reviewer records every merge decision via record_decision_outcome (→ the + # global MergeDecisionInstrumenter); _export_decision_metrics surfaces those + # collected metrics to the status dir (previously export_metrics_json had no + # caller — the metrics went nowhere). + watcher.record_decision_outcome(pr_number=7, repo_key="r", outcome="merge", reason="lgtm") + watcher._export_decision_metrics(tmp_path) + metrics_file = tmp_path / "merge_decision_metrics.json" + assert metrics_file.exists() + data = json.loads(metrics_file.read_text()) + assert "total_decisions" in data and "outcomes" in data + assert data["total_decisions"] >= 1 + + def test_write_heartbeat_idempotent(tmp_path: Path) -> None: watcher._write_heartbeat(tmp_path) watcher._write_heartbeat(tmp_path)