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
24 changes: 24 additions & 0 deletions .console/log.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
## 2026-07-15 — feat(eval): C3 cross-family EVAL panel — close same-family generator↔evaluator (COUNCIL_VERDICT.md)

Council spec Phase 3 (C3), the last council phase. The guide-gap audit's HIGH
finding was same-family generator↔evaluator: the EVAL drift monitor is meant to
grade the claude reviewer with a DIFFERENT family, but that was only a code
comment (`critic.py`/`check_extractors.py`) and the task was wired
`extractor=None` (inert). C3 makes cross-family a CONTROL. New
`eval/panel_critic.run_panel_drift_monitor` grades each configured family
INDEPENDENTLY (per-family majority vote, never pooled for the drift decision)
and flags `drifted = any family's own majority != signed answer` — so a
dominant/larger family can't mask its own drift by outvoting a smaller one.
`eval/panel_invoker.LiveFamilyExtractor` runs each family via the shared
`build_member_argv` (extracted verbatim from pr_review_watcher/main.py into a
new `member_runner.py` — a pure move so the EVAL invoker never imports the
merge-critical reviewer module; C1's 166 reviewer tests stay green) + codex
stdout fallback. New `EvalPanelSettings` (panel=[] / enabled=False ⇒ OFF by
default, mirroring C1). DriftMonitorTask refuses to run a degraded panel —
missing family ⇒ `skipped` with a loud reason, NEVER a same-family collapse
(that would re-open the finding). Still inert in prod until an extraction-kind
corpus exists (seed corpus is verdict-kind) — wired + fully unit-tested with
injected fakes. tests/unit 86.03% (gate 85%); reviewer suite 166 green.
ty: narrowed `self._extractor` at the single-extractor call with `cast` (the
elif-guard already proves it non-None; ruff bans `assert`) — CI type-check green.

## 2026-07-15 — Stage 4: Refactor existing code to use the new shared helper (objective DONE)

Stage 2 already performed the actual migration (15 call sites across 9
Expand Down
21 changes: 21 additions & 0 deletions config/operations_center.example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,27 @@ repos:
# # all families; 2 allows a degraded quorum (unanimity among the two available).
# min_council_members: 3

# ---------------------------------------------------------------------------
# C3 — cross-family EVAL panel (drift monitor grading; COUNCIL_VERDICT.md C3).
# The non-blocking drift monitor (entrypoints/maintenance/drift_monitor_task.py)
# replays extraction-kind corpus cases through a model to check whether it still
# reproduces the signed answer. Grading with the SAME family as the implementer
# is the same same-family-generator/evaluator gap C1 closes for merging — a
# dominant family can mask its own drift. eval_panel.panel wires a cross-family
# panel (family tags, e.g. claude_code + codex_cli) and the monitor aggregates
# PER-FAMILY (a family's own majority drifting is enough to flag), so no single
# family can outvote its own regression. Defaults EMPTY ⇒ feature OFF — the
# drift monitor stays on its existing inert/single-extractor path. Populating
# this is a deliberate follow-up once a non-implementer backend is wired AND an
# extraction-kind corpus exists (the seed corpus today is verdict-kind only).
# eval_panel:
# panel: []
# # - claude_code
# # - codex_cli
# enabled: false
# # N-of-M votes per family per case (majority-voted per family).
# votes: 3

# ---------------------------------------------------------------------------
# Scheduled tasks — periodic Plane work-item seeders
# ---------------------------------------------------------------------------
Expand Down
55 changes: 48 additions & 7 deletions docs/design/COUNCIL_VERDICT.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,50 @@ Unsigned mode gains a keyless analogue of restore-by-consumption. At

Signed reference present ⇒ Track C behavior wins unchanged.

### C3 — EVAL cross-family panel (separate work item)

The guide-gap audit flagged same-family generator↔evaluator as HIGH. Grading
panels get the same cross-family treatment as C1 (claude-generated work is
judged with codex on the panel and vice versa). Specced separately once C1's
panel plumbing exists to reuse.
### C3 — EVAL cross-family panel (SHIPPED)

The guide-gap audit flagged same-family generator↔evaluator as HIGH. C1 closes
that for the MERGE decision (a guardrail PR); C3 closes the matching gap for
the EVAL drift monitor's GRADING decision — the non-blocking lane that checks
whether a model still reproduces the corpus's signed check-extraction answers
(`entrypoints/maintenance/drift_monitor_task.py`, `eval/critic.py`). Before C3
that lane's "different-family critic" requirement was only a code comment; the
extractor was wired `None` (inert) at every call site. C3 makes it a control:

1. **Panel, not a single extractor**: `settings.eval_panel.panel` names family
tags (e.g. `claude_code`, `codex_cli`); `eval.panel_invoker` builds one live
`CheckExtractor` per family, reusing C1's own argv builder
(`entrypoints/pr_review_watcher/member_runner.build_member_argv`, extracted
from `main.py` for exactly this reuse) so grading spawns the same CLI shape
the council already runs in production.
2. **Aggregation is PER-FAMILY, never pooled**: `eval.panel_critic.
run_panel_drift_monitor` votes each family independently (`votes` per
family) and takes THAT family's own majority. A case is `drifted` if **any
single family's** majority disagrees with the signed answer — a dominant or
larger family can never mask a different family's disagreement by
outvoting it, because votes are never pooled across families for the drift
decision (only within one family, to get that family's own majority).
`verdict.aggregate_council`'s unanimous-LGTM/merge shape is the wrong fit
here (grading is never a merge decision and must never gate) — C3 uses its
own aggregator instead.
3. **Degraded panel fails LOUD, never SMALL**: the drift monitor is handed both
the full *configured* panel and only the *runnable* subset for this host
(`eval.panel_invoker.resolve_available_families`, a PATH probe at wiring
time). Any gap between the two — one family's CLI unavailable — skips the
whole run with a loud reason. It never silently grades with the remaining
families, because a remaining-families grade can degenerate to a
single-family (same-family) grade, which is the exact HIGH finding this
spec closes.
4. **Off by default**: `eval_panel.panel` defaults empty and `eval_panel.
enabled` defaults `false` — populating the panel is a deliberate follow-up,
same rollout shape as C1's `guardrail_paths`. Even with a panel configured,
nothing runs without the existing `OC_EVAL_DRIFT_MONITOR=1` opt-in.
5. **Still inert pending a corpus**: the drift monitor only grades
`extraction`-kind corpus cases (real diff/context for a model to review);
the seed corpus today is `verdict`-kind (pre-filled checks) only. C3 is
fully wired and unit-tested end to end with injected fake extractors, but
has nothing to grade live until an extraction-kind corpus exists — same
caveat `check_extractors.BackendCheckExtractor` already carried before C3.

## Rollout

Expand All @@ -128,7 +166,10 @@ panel plumbing exists to reuse.
2. **Phase 2 (OC)**: C1 council mode behind `council.guardrail_paths`
(empty default), then a follow-up PR populating the path set — that PR is
the council's first live case.
3. **Phase 3 (OC)**: C3 EVAL panel.
3. **Phase 3 (OC)**: C3 EVAL panel — shipped behind `eval_panel.panel`/
`eval_panel.enabled` (both default off); populating the panel + wiring a
live non-implementer backend is a deliberate follow-up, same shape as
Phase 2's guardrail-path population.

Each phase ships through the normal PR flow. Phase 2's reviewer changes are
themselves guardrail paths, so after the path set is populated, changes to
Expand Down
31 changes: 31 additions & 0 deletions src/operations_center/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,34 @@ class CouncilSettings(BaseModel):
min_council_members: int = 3


class EvalPanelSettings(BaseModel):
"""C3 — cross-family EVAL panel for the drift monitor (COUNCIL_VERDICT.md C3).

Modeled directly on ``CouncilSettings`` (C1): the guide-gap audit's
same-family-generator/evaluator HIGH finding applies to *grading* too — a
drift monitor graded entirely by the implementer's own family (N copies of
one family is N=1, shared blindspots) can't see its own regressions. This
makes cross-family a CONTROL for the grading lane: the drift monitor is
driven by a panel of family tags (e.g. ``claude_code``, ``codex_cli``) and
aggregated PER-FAMILY (see ``eval.panel_critic.run_panel_drift_monitor``)
so a dominant family can't mask its own drift by outvoting the other.

``panel`` defaults EMPTY ⇒ the feature is OFF (the drift monitor stays on
its existing single-extractor/inert path — see ``DriftMonitorTask``).
Populating it is a deliberate follow-up once a non-implementer backend is
wired and an extraction corpus exists; this rollout must not itself force
a live model call.
"""

# Family tags (worker-backend names, e.g. "claude_code"/"codex_cli") that
# make up the panel; empty ⇒ feature OFF (no cross-family grading).
panel: list[str] = Field(default_factory=list)
enabled: bool = False
# N-of-M votes per family per case (majority-voted per family; see
# panel_critic.run_panel_drift_monitor).
votes: int = 3


class ReviewerSettings(BaseModel):
# GitHub logins whose comments are always ignored (bots, CI accounts)
bot_logins: list[str] = Field(default_factory=list)
Expand Down Expand Up @@ -636,6 +664,9 @@ class Settings(BaseModel):
resource_gate: ResourceGateSettings = Field(default_factory=ResourceGateSettings)
repos: dict[str, RepoSettings] = Field(default_factory=dict)
reviewer: ReviewerSettings = Field(default_factory=ReviewerSettings)
# C3 — cross-family EVAL panel for the drift monitor. Empty ``panel`` (the
# default) ⇒ OFF; see EvalPanelSettings.
eval_panel: EvalPanelSettings = Field(default_factory=EvalPanelSettings)
report_root: Path = Path("tools/report/runs")
# The repo key that identifies this OperationsCenter installation itself.
# Tasks targeting this repo require a "self-modify: approved" label before
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,24 @@
invoker is a deliberate seam: with none configured the task ``skipped`` (no model,
no false drift). Opt-in via ``OC_EVAL_DRIFT_MONITOR=1`` once an extractor is wired.
* **Non-blocking** — drift becomes a deduplicated operator ticket, never a build
failure (voting smooths onset-of-regression variance; it must not gate)."""
failure (voting smooths onset-of-regression variance; it must not gate).

C3 (COUNCIL_VERDICT.md C3) turns "different family" from a comment into a CONTROL:
when a cross-family ``panel_families`` list is configured (``settings.eval_panel``)
and enabled, cases are graded with ``panel_critic.run_panel_drift_monitor`` instead
of the single-extractor path — every configured family votes and is aggregated
PER-FAMILY, so no one family can outvote another family's dissent. A gap between
the configured panel and the families this process could actually build/probe as
runnable (``family_extractors``) is a DEGRADED panel: this task skips loudly rather
than silently falling back to whatever subset is left (that fallback is exactly the
same-family collapse the control exists to prevent)."""

from __future__ import annotations

import os
import time
from pathlib import Path
from typing import TYPE_CHECKING, Any, Literal
from typing import TYPE_CHECKING, Any, Literal, Mapping, cast

from operations_center.eval.corpus import load_ledger
from operations_center.eval.critic import (
Expand All @@ -33,6 +43,7 @@
DriftResult,
run_drift_monitor,
)
from operations_center.eval.panel_critic import run_panel_drift_monitor
from operations_center.maintenance.contracts import MaintenanceResult

if TYPE_CHECKING:
Expand Down Expand Up @@ -62,6 +73,9 @@ def __init__(
extractor: CheckExtractor | None = None,
votes: int = 3,
plane_client: PlaneClient | None = None,
panel_families: list[str] | None = None,
family_extractors: Mapping[str, CheckExtractor] | None = None,
panel_enabled: bool | None = None,
) -> None:
self._settings = settings
self.interval_seconds = interval_seconds
Expand All @@ -70,6 +84,26 @@ def __init__(
self._extractor = extractor
self._votes = votes
self._plane_client = plane_client
# C3 — cross-family panel (COUNCIL_VERDICT.md C3). ``panel_families`` is
# the FULL configured panel (from settings.eval_panel.panel when not
# given explicitly); ``family_extractors`` is whichever of those
# families this process actually has a runnable extractor for. A gap
# between the two is a degraded panel — see run_once. Both default to
# settings-derived values so a bare ``DriftMonitorTask(settings)`` (the
# spec_hygiene wiring) picks up config with no extra plumbing, while
# tests can inject either directly (``settings=None`` is fine).
eval_panel = getattr(settings, "eval_panel", None)
self._panel_families: list[str] = (
list(panel_families) if panel_families is not None
else list(getattr(eval_panel, "panel", []) or [])
)
self._family_extractors: dict[str, CheckExtractor] = (
dict(family_extractors) if family_extractors is not None else {}
)
self._panel_enabled: bool = (
panel_enabled if panel_enabled is not None
else bool(getattr(eval_panel, "enabled", False))
)

def _make_plane_client(self) -> PlaneClient:
if self._plane_client is not None:
Expand All @@ -86,12 +120,40 @@ def _make_plane_client(self) -> PlaneClient:

def run_once(self, ctx: MaintenanceContext) -> MaintenanceResult:
started = time.monotonic()
# Opt-in + injected-extractor required. No extractor (no wired model) →
# skipped: no model means no false drift (§0.1 fail-safe).
if self._extractor is None or os.environ.get(_ENABLE_ENV) != "1":
# Opt-in required either way — no model means no false drift (§0.1
# fail-safe), whether that's the legacy single-extractor path or the
# C3 cross-family panel.
if os.environ.get(_ENABLE_ENV) != "1":
return self._result(
"skipped", started, {"reason": "drift monitor not enabled / no extractor"}
)

use_panel = bool(self._panel_families) and self._panel_enabled
if use_panel:
missing = sorted(f for f in self._panel_families if f not in self._family_extractors)
if missing:
# Degraded panel (a configured family has no runnable extractor
# here — e.g. its CLI wasn't resolvable at wiring time). NEVER
# silently grade with the smaller/remaining subset — that is
# exactly the same-family collapse C3 exists to prevent.
return self._result(
"skipped",
started,
{
"reason": (
f"degraded eval panel: family extractor(s) {missing} "
"unavailable — refusing to collapse to a smaller panel"
),
"panel": sorted(self._panel_families),
"missing": missing,
},
)
elif self._extractor is None:
# No panel configured/enabled and no single extractor wired either.
return self._result(
"skipped", started, {"reason": "drift monitor not enabled / no extractor"}
)

try:
cases = [c for c in load_ledger(self._corpus_path).cases() if c.kind == EXTRACTION_KIND]
except Exception as exc: # noqa: BLE001 — a corpus read error must not halt the loop
Expand All @@ -100,12 +162,23 @@ def run_once(self, ctx: MaintenanceContext) -> MaintenanceResult:
return self._result("skipped", started, {"reason": "no extraction-kind cases"})

try:
results = run_drift_monitor(cases, self._extractor, votes=self._votes)
if use_panel:
panel = {f: self._family_extractors[f] for f in self._panel_families}
results = run_panel_drift_monitor(cases, panel, votes=self._votes)
else:
# Not use_panel ⇒ the `elif self._extractor is None: return` guard
# above already handled the None case, so the single extractor is
# present here (cast narrows it for the type checker).
results = run_drift_monitor(
cases, cast("CheckExtractor", self._extractor), votes=self._votes
)
except Exception as exc: # noqa: BLE001 — a flaky backend must not halt the loop
return self._result("failed", started, {}, error=f"drift_run_failed: {exc}")

drifted = [r for r in results if r.drifted]
details: dict[str, object] = {"cases": len(cases), "drifted": len(drifted)}
if use_panel:
details["panel"] = sorted(self._panel_families)
if drifted:
details["tickets"] = self._emit_tickets(ctx, drifted)
return self._result("ok", started, details)
Expand Down
35 changes: 3 additions & 32 deletions src/operations_center/entrypoints/pr_review_watcher/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@
make_nonce,
sanitize_for_comment,
)
from operations_center.entrypoints.pr_review_watcher.member_runner import (
build_member_argv as _build_member_argv,
)
from operations_center.entrypoints.pr_review_watcher.verdict import (
_COUNCIL_PANEL,
CONCERNS,
Expand Down Expand Up @@ -561,38 +564,6 @@ def _select_review_backend(settings, *, usage_store=None, now=None):
return None


def _build_member_argv(backend: str, model: str, prompt: str) -> list[str] | None:
"""Build the CLI argv for one review-panel member.

Mirrors :func:`worker_backend_probe._probe_command` — the same binary/flag
shape the controller and the cooldown-probe already use — so the reviewer's
own invocation matches the rest of the fleet instead of a bespoke one-off.
Returns ``None`` for an unsupported ``(backend, model)`` pair.
"""
if backend == "claude_code":
# Preserve the live single-review invocation exactly (only the model
# varies per council seat): `--effort low` keeps reviews cheap+fast, and
# NOT passing --dangerously-skip-permissions matches the path that has
# run in production — a reviewer in an empty tmpdir needs neither.
return [
"claude",
"--model",
model,
"-p",
"--effort",
"low",
prompt,
]
if backend == "codex_cli":
return [
"codex",
"exec",
"--dangerously-bypass-approvals-and-sandbox",
prompt,
]
return None


def _run_member_review(
oc_root: Path,
goal_text: str,
Expand Down
Loading
Loading