Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .console/log.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 0 additions & 1 deletion .custodian/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions src/operations_center/entrypoints/pr_review_watcher/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 ────────────────────────────────────────────────────────────────


Expand Down Expand Up @@ -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)
Expand All @@ -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)


Expand Down
14 changes: 14 additions & 0 deletions tests/test_pr_review_watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading