diff --git a/.console/log.md b/.console/log.md index a6fed6845..f46317fd5 100644 --- a/.console/log.md +++ b/.console/log.md @@ -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 diff --git a/config/operations_center.example.yaml b/config/operations_center.example.yaml index 32d951011..c03c0b706 100644 --- a/config/operations_center.example.yaml +++ b/config/operations_center.example.yaml @@ -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 # --------------------------------------------------------------------------- diff --git a/docs/design/COUNCIL_VERDICT.md b/docs/design/COUNCIL_VERDICT.md index 93fc34c94..fb1f5b9b3 100644 --- a/docs/design/COUNCIL_VERDICT.md +++ b/docs/design/COUNCIL_VERDICT.md @@ -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 @@ -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 diff --git a/src/operations_center/config/settings.py b/src/operations_center/config/settings.py index 545be93fa..a7a5f2a6c 100644 --- a/src/operations_center/config/settings.py +++ b/src/operations_center/config/settings.py @@ -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) @@ -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 diff --git a/src/operations_center/entrypoints/maintenance/drift_monitor_task.py b/src/operations_center/entrypoints/maintenance/drift_monitor_task.py index 86c9ca0b7..af0d193f3 100644 --- a/src/operations_center/entrypoints/maintenance/drift_monitor_task.py +++ b/src/operations_center/entrypoints/maintenance/drift_monitor_task.py @@ -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 ( @@ -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: @@ -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 @@ -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: @@ -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 @@ -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) diff --git a/src/operations_center/entrypoints/pr_review_watcher/main.py b/src/operations_center/entrypoints/pr_review_watcher/main.py index 541075dd3..f4ba2252c 100644 --- a/src/operations_center/entrypoints/pr_review_watcher/main.py +++ b/src/operations_center/entrypoints/pr_review_watcher/main.py @@ -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, @@ -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, diff --git a/src/operations_center/entrypoints/pr_review_watcher/member_runner.py b/src/operations_center/entrypoints/pr_review_watcher/member_runner.py new file mode 100644 index 000000000..01c1b0906 --- /dev/null +++ b/src/operations_center/entrypoints/pr_review_watcher/member_runner.py @@ -0,0 +1,49 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""Shared CLI-argv builder for one review-panel member (C1/C3). + +Extracted from ``pr_review_watcher/main.py`` (a pure move, no logic change) so +that code outside the reviewer's merge-critical module — namely the EVAL +cross-family panel invoker (``eval/panel_invoker.py``, C3) — can build the +exact same backend/model CLI invocation the live council uses, without +importing ``main.py`` itself (which pulls in the full reviewer state +machine). ``main.py`` keeps a thin alias so its own callers/tests are +unaffected. +""" + +from __future__ import annotations + + +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 + + +__all__ = ["build_member_argv"] diff --git a/src/operations_center/entrypoints/spec_hygiene/main.py b/src/operations_center/entrypoints/spec_hygiene/main.py index e5b110613..52e1187a0 100644 --- a/src/operations_center/entrypoints/spec_hygiene/main.py +++ b/src/operations_center/entrypoints/spec_hygiene/main.py @@ -45,6 +45,7 @@ ) from operations_center.entrypoints.maintenance.parked_unpark_task import ParkedUnparkTask from operations_center.entrypoints.maintenance.queue_healing_task import QueueHealingTask +from operations_center.eval.panel_invoker import build_panel_extractors, resolve_available_families from operations_center.maintenance.ledger_maintain import LedgerMaintainTask from operations_center.spec_author.campaign_builder import CampaignBuilder from operations_center.spec_author.models import ( @@ -695,7 +696,45 @@ def register_maintenance_tasks( # blocking) when the model's verdict drifts from the signed answer — the # semantic reviewer miss the deterministic gate is blind to. Skipped until # OC_EVAL_DRIFT_MONITOR=1 + a model extractor is wired (no model, no false drift). - registry.register(DriftMonitorTask(settings, plane_client=client)) + # + # C3 (COUNCIL_VERDICT.md C3): when settings.eval_panel.panel is non-empty AND + # eval_panel.enabled, build a cross-family extractor per configured family and + # pass BOTH the full configured panel (panel_families) and only the subset + # this host can actually run (family_extractors, gated on + # resolve_available_families — a CLI probe). DriftMonitorTask compares the + # two and skips loudly on any gap rather than silently grading a smaller + # (possibly same-family) panel. eval_panel.panel defaults EMPTY, so by + # default this is a no-op and the task keeps its existing inert/skipped + # behavior — no CLI is ever spawned unless an operator opts in. Read + # defensively (getattr) so a settings stand-in without eval_panel (older + # config / test fakes) also defaults to OFF rather than raising. + _eval_panel = getattr(settings, "eval_panel", None) + _eval_panel_families = list(getattr(_eval_panel, "panel", None) or []) + _eval_panel_extractors: dict[str, Any] = {} + if _eval_panel_families and getattr(_eval_panel, "enabled", False): + try: + _eval_panel_extractors = build_panel_extractors( + resolve_available_families(_eval_panel_families) + ) + except Exception: # noqa: BLE001 — a misconfigured family (e.g. an + # unrecognized tag) must not crash maintenance-task registration; + # leaving extractors empty makes DriftMonitorTask see every + # configured family as "missing" and skip loudly (never a + # same-family collapse), the same fail-safe as an unavailable CLI. + logger.warning( + "spec_hygiene: failed to build EVAL panel extractors for %s — " + "drift monitor will skip (degraded panel)", + _eval_panel_families, + ) + registry.register( + DriftMonitorTask( + settings, + plane_client=client, + panel_families=_eval_panel_families, + family_extractors=_eval_panel_extractors, + votes=getattr(_eval_panel, "votes", 3), + ) + ) # Deterministic blocked-queue healer (inventory #4). Recycles retry-safe # Blocked tasks back to Ready-for-AI/Backlog and escalates budget-exhausted # lineages — non-destructively (never deletes). FAIL-SAFE: registered but diff --git a/src/operations_center/eval/panel_critic.py b/src/operations_center/eval/panel_critic.py new file mode 100644 index 000000000..e6e07bba9 --- /dev/null +++ b/src/operations_center/eval/panel_critic.py @@ -0,0 +1,132 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""C3 — cross-family EVAL panel aggregation (COUNCIL_VERDICT.md C3, D-EVAL-5). + +``critic.run_drift_monitor`` grades a case with a SINGLE injected extractor — +today that extractor is (by construction, never enforced) some non-implementer +family. The guide-gap audit's same-family-generator/evaluator finding applies +here just as much as it does to the C1 merge council: a lone extractor, or a +naive pooled vote across several extractors, lets a dominant family's answer +outvote a different family's disagreement — exactly the collusion-by-shared- +blindspot the cross-family requirement exists to prevent. + +This module is the grading-specific analogue of ``verdict.aggregate_council`` +— but council's unanimous-LGTM/merge shape is the wrong fit here (grading is +never a merge decision, and it must never gate — see ``critic`` module intro). +Instead: each configured family votes ``votes`` times **on its own**; each +family's OWN majority is compared against the signed answer; the case is +flagged drifted if **any single family's** majority disagrees — regardless of +how many other families (or how many total votes) agree. A 2-seat family can +never out-vote a 1-seat family's dissent because votes are never pooled across +families for the drift decision — only within a family, to compute that +family's own majority. + +Pure and injectable (families passed in as ``CheckExtractor``s), same shape as +``critic.run_drift_monitor``, so tests can inject deterministic fakes instead +of live models.""" + +from __future__ import annotations + +from collections import Counter +from dataclasses import dataclass, field +from typing import Mapping + +from operations_center.entrypoints.pr_review_watcher.verdict import compute_verdict +from operations_center.eval.corpus import Case +from operations_center.eval.critic import CheckExtractor, DriftResult + + +@dataclass(frozen=True) +class PanelDriftResult(DriftResult): + """``DriftResult`` extended with the per-family breakdown that makes the + cross-family grading control auditable — which family (if any) drifted, + not just whether the panel as a whole did. + + The inherited ``majority``/``agree_votes``/``total_votes`` fields keep + ``DriftResult``'s original meaning (the panel's votes pooled as one big + N-of-M tally, for continuity with the single-extractor report shape). + ``drifted`` does NOT come from that pooled tally, though — see + ``run_panel_drift_monitor``. ``per_family`` is the authoritative signal: + ``{family: {"majority": {...}, "agree_votes": int, "total_votes": int}}``. + """ + + per_family: dict[str, dict[str, object]] = field(default_factory=dict) + + +def run_panel_drift_monitor( + cases: list[Case], + family_extractors: Mapping[str, CheckExtractor], + *, + votes: int = 3, +) -> list[PanelDriftResult]: + """Replay each case through every family's extractor independently; + flag drift when ANY family's own majority disagrees with the signed answer. + + ``family_extractors`` maps a family tag (e.g. ``"claude_code"``, + ``"codex_cli"``) to a ``CheckExtractor`` for that family. Each family gets + its own independent ``votes``-vote majority (never pooled with another + family's votes for the drift decision) — that per-family isolation is what + stops a dominant/larger family from masking its own drift by outvoting a + smaller one. + + Raises ``ValueError`` on ``votes < 1`` or an empty ``family_extractors`` + (an empty panel has no cross-family control to speak of — the caller + is expected to treat "no panel configured" as feature-OFF *before* + calling this, not by handing it zero families).""" + if votes < 1: + raise ValueError("votes must be >= 1") + if not family_extractors: + raise ValueError("family_extractors must be non-empty") + + out: list[PanelDriftResult] = [] + for case in cases: + gt = case.ground_truth + expected = {"result": gt.get("result"), "failing": sorted(gt.get("failing", []) or [])} + + per_family: dict[str, dict[str, object]] = {} + pooled_tally: Counter[str] = Counter() + pooled_rendered: dict[str, dict[str, object]] = {} + drifted_families: list[str] = [] + + for family, extractor in family_extractors.items(): + tally: Counter[str] = Counter() + rendered: dict[str, dict[str, object]] = {} + for v in range(votes): + result, failing = compute_verdict(extractor(case, vote=v)) + key = f"{result}|{','.join(sorted(failing))}" + tally[key] += 1 + rendered[key] = {"result": result, "failing": sorted(failing)} + pooled_tally[key] += 1 + pooled_rendered[key] = rendered[key] + + top_key, agree = tally.most_common(1)[0] + majority = rendered[top_key] + per_family[family] = { + "majority": majority, + "agree_votes": agree, + "total_votes": votes, + } + if majority != expected: + drifted_families.append(f"{family} majority {majority} != answer {expected}") + + pooled_top_key, pooled_agree = pooled_tally.most_common(1)[0] + pooled_majority = pooled_rendered[pooled_top_key] + drifted = bool(drifted_families) + detail = "; ".join(drifted_families) + + out.append( + PanelDriftResult( + case_id=case.case_id, + expected=expected, + majority=pooled_majority, + agree_votes=pooled_agree, + total_votes=votes * len(family_extractors), + drifted=drifted, + detail=detail, + per_family=per_family, + ) + ) + return out + + +__all__ = ["PanelDriftResult", "run_panel_drift_monitor"] diff --git a/src/operations_center/eval/panel_invoker.py b/src/operations_center/eval/panel_invoker.py new file mode 100644 index 000000000..e0ede82ce --- /dev/null +++ b/src/operations_center/eval/panel_invoker.py @@ -0,0 +1,165 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""C3 — live cross-family invoker for the EVAL panel (COUNCIL_VERDICT.md C3). + +``panel_critic.run_panel_drift_monitor`` is pure: it takes a ``CheckExtractor`` +per family and never touches a subprocess. This module is the impure seam that +wires a real family to a real CLI — one per configured family tag (e.g. +``"claude_code"``, ``"codex_cli"``) — reusing the exact argv shape the C1 +council already runs in production (``member_runner.build_member_argv``) +instead of inventing a bespoke invocation for the grading lane. + +Each call runs the family's CLI once, bounded, in a fresh empty tmpdir (mirrors +``pr_review_watcher._run_member_review``): reads ``verdict.json`` if the +backend wrote it, else falls back to the last balanced JSON object on stdout +(``verdict.last_json_object`` — codex answers on stdout instead of the file +contract). Any failure — missing binary, timeout, crash, unparseable output — +returns ``[]`` (no checks), which ``compute_verdict`` turns into CONCERNS: an +invoker hiccup must read as drift, never as a silent pass. + +Kept deliberately thin: the only logic here is "run one CLI, get the checks +array out of it." Genuinely untestable subprocess glue is marked +``# pragma: no cover``; nothing else in this module should be.""" + +from __future__ import annotations + +import json +import shutil +import subprocess +import tempfile +from pathlib import Path + +from operations_center.entrypoints.pr_review_watcher.member_runner import build_member_argv +from operations_center.entrypoints.pr_review_watcher.verdict import last_json_object +from operations_center.eval.check_extractors import build_extraction_prompt +from operations_center.eval.corpus import Case +from operations_center.eval.critic import CheckExtractor + +DEFAULT_TIMEOUT_SECONDS = 300 + +# The default (backend, model) for each family tag — mirrors the family-level +# seat the C1 council already runs (verdict._COUNCIL_PANEL): cheapest model +# that still represents that family's judgment, not a specific council lens. +DEFAULT_MODEL_BY_FAMILY: dict[str, str] = { + "claude_code": "sonnet", + "codex_cli": "codex", +} + +# The CLI binary that backs each family tag — used only to PROBE availability +# (shutil.which) at wiring time, mirroring worker_backend_probe's own binary +# names. Not a full PATH-fallback resolver (that's the probe module's job); +# this is a cheap, deterministic "is it even on PATH" gate so a missing binary +# is caught BEFORE the panel runs, as a degraded-panel skip, rather than +# surfacing as an empty-checks "drift" from a subprocess that never started. +_FAMILY_BINARY: dict[str, str] = { + "claude_code": "claude", + "codex_cli": "codex", +} + + +def resolve_available_families(families: list[str]) -> list[str]: + """Return the subset of ``families`` whose CLI binary is resolvable on PATH. + + Called at wiring time (spec_hygiene/main.py) to compute the runnable + subset of the *configured* panel BEFORE building any extractors. The + caller passes the full configured panel to ``DriftMonitorTask`` as + ``panel_families`` and only the resolvable subset's extractors as + ``family_extractors`` — the task compares the two and skips loudly on any + gap instead of silently grading with whatever's left (COUNCIL_VERDICT.md + C3's "never collapse to a smaller/same-family panel" rule). + """ + return [f for f in families if shutil.which(_FAMILY_BINARY.get(f, f)) is not None] + + +class LiveFamilyExtractor: + """A :class:`critic.CheckExtractor` for one family, backed by a live CLI call. + + Not unit-tested beyond construction — exercising it means actually + spawning ``claude``/``codex``, which is exactly the "genuinely untestable" + glue the pure ``panel_critic`` module was split out to avoid needing.""" + + def __init__( + self, backend: str, model: str, *, timeout_seconds: int = DEFAULT_TIMEOUT_SECONDS + ) -> None: + self._backend = backend + self._model = model + self._timeout = timeout_seconds + + def __call__(self, case: Case, *, vote: int) -> object: # pragma: no cover — live subprocess glue + prompt = build_extraction_prompt(case) + argv = build_member_argv(self._backend, self._model, prompt) + if argv is None: + return [] + try: + with tempfile.TemporaryDirectory(prefix="oc-eval-panel-") as tmpdir: + tmp = Path(tmpdir) + proc = subprocess.run( + argv, + cwd=str(tmp), + capture_output=True, + text=True, + timeout=self._timeout, + ) + verdict_path = tmp / "verdict.json" + raw: dict | None = None + if verdict_path.exists(): + try: + raw = json.loads(verdict_path.read_text(encoding="utf-8")) + except Exception: + raw = None + if raw is None: + # Fallback for a backend that answers on stdout instead of + # writing the file (observed with codex). + raw = last_json_object(proc.stdout) + except Exception: + # Missing binary, timeout, crash — an invoker failure reads as + # drift (empty checks -> CONCERNS), never a silent pass. + return [] + checks = raw.get("checks") if isinstance(raw, dict) else None + return checks if isinstance(checks, list) else [] + + +def build_family_extractor( + family: str, + *, + model: str | None = None, + timeout_seconds: int = DEFAULT_TIMEOUT_SECONDS, +) -> CheckExtractor: + """Build a live :class:`critic.CheckExtractor` for one family tag. + + Raises ``ValueError`` for a family with no known default model and none + given explicitly — the caller (``DriftMonitorTask``) treats a build + failure the same as a degraded/unavailable family: skip the whole panel + run with a loud reason, never silently drop to a smaller (possibly + same-family) panel. + """ + resolved_model = model or DEFAULT_MODEL_BY_FAMILY.get(family) + if resolved_model is None: + raise ValueError(f"no default model for family {family!r}; pass model explicitly") + return LiveFamilyExtractor(family, resolved_model, timeout_seconds=timeout_seconds) + + +def build_panel_extractors( + families: list[str], *, timeout_seconds: int = DEFAULT_TIMEOUT_SECONDS +) -> dict[str, CheckExtractor]: + """Build one live ``CheckExtractor`` per configured family tag. + + Raises (propagates ``build_family_extractor``'s ``ValueError``) on an + unrecognized family rather than silently omitting it — an omitted family + would shrink the panel without the caller noticing, which is exactly the + same-family-collapse failure mode C3 exists to prevent. + """ + return { + family: build_family_extractor(family, timeout_seconds=timeout_seconds) + for family in families + } + + +__all__ = [ + "DEFAULT_MODEL_BY_FAMILY", + "DEFAULT_TIMEOUT_SECONDS", + "LiveFamilyExtractor", + "build_family_extractor", + "build_panel_extractors", + "resolve_available_families", +] diff --git a/tests/unit/entrypoints/maintenance/test_board_unblock_task.py b/tests/unit/entrypoints/maintenance/test_board_unblock_task.py index 15db59408..8009cafed 100644 --- a/tests/unit/entrypoints/maintenance/test_board_unblock_task.py +++ b/tests/unit/entrypoints/maintenance/test_board_unblock_task.py @@ -316,3 +316,110 @@ def test_dead_subsystem_tasks_registered_in_live_loop(self): # And they are disabled-by-default (fail-safe). assert by_name["queue_healing"].enabled is False assert by_name["parked_unpark"].enabled is False + + def test_eval_panel_wired_from_settings_eval_panel(self): + """C3 (COUNCIL_VERDICT.md C3): a configured+enabled eval_panel is + translated into panel_families/family_extractors on the REAL + DriftMonitorTask — pins the spec_hygiene wiring shape end to end.""" + from operations_center.entrypoints.spec_hygiene import main as sh + + registry = mock.Mock() + registered_objs: list[object] = [] + registry.register.side_effect = lambda t: registered_objs.append(t) + + settings = SimpleNamespace( + **{ + **vars(_settings()), + "eval_panel": SimpleNamespace( + panel=["claude_code", "codex_cli"], enabled=True, votes=5 + ), + } + ) + fake_extractors = {"claude_code": object(), "codex_cli": object()} + + with ( + mock.patch.object( + sh, "SpecHygieneTask", lambda *a, **k: SimpleNamespace(name="spec_hygiene") + ), + mock.patch.object( + sh, "LedgerMaintainTask", lambda *a, **k: SimpleNamespace(name="ledger_maintain") + ), + mock.patch.object( + sh, "BoardUnblockTask", lambda *a, **k: SimpleNamespace(name="board_unblock") + ), + mock.patch.object( + sh, "EgressProbeTask", lambda *a, **k: SimpleNamespace(name="egress_probe") + ), + mock.patch.object( + sh, "HeartbeatStallTask", lambda *a, **k: SimpleNamespace(name="heartbeat_stall") + ), + mock.patch.object( + sh, "OutcomeFlaggerTask", lambda *a, **k: SimpleNamespace(name="outcome_flagger") + ), + mock.patch.object( + sh, "resolve_available_families", lambda families: list(families) + ), + mock.patch.object( + sh, "build_panel_extractors", lambda families: dict(fake_extractors) + ), + ): + sh.register_maintenance_tasks(registry, settings, mock.Mock()) + + by_name = {t.name: t for t in registered_objs} + drift_task = by_name["drift_monitor"] + assert drift_task._panel_families == ["claude_code", "codex_cli"] + assert drift_task._family_extractors == fake_extractors + assert drift_task._panel_enabled is True + assert drift_task._votes == 5 + + def test_eval_panel_build_failure_degrades_to_empty_never_crashes(self): + """A misconfigured family (build_panel_extractors raising) must not + crash maintenance-task registration — it degrades to an empty + family_extractors map, which makes DriftMonitorTask see every + configured family as missing and skip loudly (never a same-family + collapse, and never a hard registration failure either).""" + from operations_center.entrypoints.spec_hygiene import main as sh + + registry = mock.Mock() + registered_objs: list[object] = [] + registry.register.side_effect = lambda t: registered_objs.append(t) + + settings = SimpleNamespace( + **{ + **vars(_settings()), + "eval_panel": SimpleNamespace(panel=["unknown_family"], enabled=True, votes=3), + } + ) + + def _boom(families): + raise ValueError("no default model for family 'unknown_family'") + + with ( + mock.patch.object( + sh, "SpecHygieneTask", lambda *a, **k: SimpleNamespace(name="spec_hygiene") + ), + mock.patch.object( + sh, "LedgerMaintainTask", lambda *a, **k: SimpleNamespace(name="ledger_maintain") + ), + mock.patch.object( + sh, "BoardUnblockTask", lambda *a, **k: SimpleNamespace(name="board_unblock") + ), + mock.patch.object( + sh, "EgressProbeTask", lambda *a, **k: SimpleNamespace(name="egress_probe") + ), + mock.patch.object( + sh, "HeartbeatStallTask", lambda *a, **k: SimpleNamespace(name="heartbeat_stall") + ), + mock.patch.object( + sh, "OutcomeFlaggerTask", lambda *a, **k: SimpleNamespace(name="outcome_flagger") + ), + mock.patch.object(sh, "resolve_available_families", lambda families: list(families)), + mock.patch.object(sh, "build_panel_extractors", _boom), + ): + # Must not raise. + sh.register_maintenance_tasks(registry, settings, mock.Mock()) + + by_name = {t.name: t for t in registered_objs} + drift_task = by_name["drift_monitor"] + assert drift_task._panel_families == ["unknown_family"] + assert drift_task._family_extractors == {} diff --git a/tests/unit/entrypoints/maintenance/test_drift_monitor_task.py b/tests/unit/entrypoints/maintenance/test_drift_monitor_task.py index 5bf998c03..e79f914f8 100644 --- a/tests/unit/entrypoints/maintenance/test_drift_monitor_task.py +++ b/tests/unit/entrypoints/maintenance/test_drift_monitor_task.py @@ -117,3 +117,111 @@ def test_drift_ticket_dedup(monkeypatch, tmp_path): ) task.run_once(_ctx(plane)) assert plane.created == [] + + +# ── C3 — cross-family panel wiring (COUNCIL_VERDICT.md C3) ──────────────────── + +_RIGHT = [ + {"check_id": "code_quality", "status": "fail"}, + {"check_id": "no_tooling_artifacts", "status": "pass"}, +] # computes to CONCERNS — matches the seeded x-concern answer +_WRONG = [ + {"check_id": "code_quality", "status": "pass"}, + {"check_id": "no_tooling_artifacts", "status": "pass"}, +] # computes to LGTM — disagrees + + +def test_panel_empty_by_default_falls_back_to_legacy_skip(monkeypatch, tmp_path): + """No panel_families/panel_enabled given and no single extractor either — + empty-panel-config means the feature is OFF, matching the existing inert + fail-safe (not a new code path).""" + monkeypatch.setenv("OC_EVAL_DRIFT_MONITOR", "1") + task = DriftMonitorTask(settings=None, corpus_path=_extraction_corpus(tmp_path), votes=1) + result = task.run_once(_ctx()) + assert result.status == "skipped" + + +def test_panel_disabled_ignores_configured_family_extractors(monkeypatch, tmp_path): + """panel_enabled=False must not run the panel even if family_extractors are + injected and non-empty — enabled is a real gate, not just a presence check.""" + monkeypatch.setenv("OC_EVAL_DRIFT_MONITOR", "1") + task = DriftMonitorTask( + settings=None, corpus_path=_extraction_corpus(tmp_path), votes=1, + panel_families=["claude_code", "codex_cli"], + family_extractors={"claude_code": _extractor(_RIGHT), "codex_cli": _extractor(_RIGHT)}, + panel_enabled=False, + ) + result = task.run_once(_ctx()) + assert result.status == "skipped" + + +def test_panel_full_quorum_runs_and_catches_family_drift(monkeypatch, tmp_path): + """Full panel (every configured family has a runnable extractor): runs + run_panel_drift_monitor and still catches a single family's drift.""" + monkeypatch.setenv("OC_EVAL_DRIFT_MONITOR", "1") + plane = _FakePlane() + task = DriftMonitorTask( + settings=None, corpus_path=_extraction_corpus(tmp_path), votes=1, + panel_families=["claude_code", "codex_cli"], + family_extractors={"claude_code": _extractor(_WRONG), "codex_cli": _extractor(_RIGHT)}, + panel_enabled=True, + ) + result = task.run_once(_ctx(plane)) + assert result.status == "ok" + assert result.details["drifted"] == 1 + assert result.details["panel"] == ["claude_code", "codex_cli"] + assert len(plane.created) == 1 + + +def test_panel_no_drift_when_every_family_agrees(monkeypatch, tmp_path): + monkeypatch.setenv("OC_EVAL_DRIFT_MONITOR", "1") + plane = _FakePlane() + task = DriftMonitorTask( + settings=None, corpus_path=_extraction_corpus(tmp_path), votes=1, + panel_families=["claude_code", "codex_cli"], + family_extractors={"claude_code": _extractor(_RIGHT), "codex_cli": _extractor(_RIGHT)}, + panel_enabled=True, + ) + result = task.run_once(_ctx(plane)) + assert result.status == "ok" + assert result.details["drifted"] == 0 + assert plane.created == [] + + +def test_panel_degraded_missing_family_skips_never_collapses(monkeypatch, tmp_path): + """The essential degraded-panel rule: a configured family (codex_cli) has + NO runnable extractor here (e.g. its CLI wasn't resolvable). The task must + skip loudly — never silently grade with just claude_code (a same-family + grade is exactly the HIGH finding C3 exists to close).""" + monkeypatch.setenv("OC_EVAL_DRIFT_MONITOR", "1") + plane = _FakePlane() + task = DriftMonitorTask( + settings=None, corpus_path=_extraction_corpus(tmp_path), votes=1, + panel_families=["claude_code", "codex_cli"], + family_extractors={"claude_code": _extractor(_WRONG)}, # codex_cli missing + panel_enabled=True, + ) + result = task.run_once(_ctx(plane)) + assert result.status == "skipped" + assert "degraded" in result.details["reason"] + assert result.details["missing"] == ["codex_cli"] + assert plane.created == [] # never ran, never ticketed + + +def test_panel_enabled_via_settings_object(monkeypatch, tmp_path): + """panel_families/panel_enabled derive from settings.eval_panel when not + passed explicitly — the spec_hygiene wiring shape (a real Settings).""" + from types import SimpleNamespace + + monkeypatch.setenv("OC_EVAL_DRIFT_MONITOR", "1") + settings = SimpleNamespace( + eval_panel=SimpleNamespace(panel=["claude_code", "codex_cli"], enabled=True, votes=1) + ) + plane = _FakePlane() + task = DriftMonitorTask( + settings=settings, corpus_path=_extraction_corpus(tmp_path), + family_extractors={"claude_code": _extractor(_WRONG), "codex_cli": _extractor(_RIGHT)}, + ) + result = task.run_once(_ctx(plane)) + assert result.status == "ok" + assert result.details["drifted"] == 1 diff --git a/tests/unit/eval/test_panel_critic.py b/tests/unit/eval/test_panel_critic.py new file mode 100644 index 000000000..2b1229a61 --- /dev/null +++ b/tests/unit/eval/test_panel_critic.py @@ -0,0 +1,105 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""C3 — cross-family EVAL panel aggregation (per-family drift, not pooled).""" + +from __future__ import annotations + +import pytest + +from operations_center.eval.corpus import Case +from operations_center.eval.panel_critic import PanelDriftResult, run_panel_drift_monitor + + +def _case(cid="c") -> Case: + return Case( + case_id=cid, + kind="extraction", + input={"diff": "..."}, + ground_truth={"result": "CONCERNS", "failing": ["code_quality"]}, + ) + + +def _fixed(checks): + return lambda case, *, vote: checks + + +_WRONG = [ + {"check_id": "code_quality", "status": "pass"}, + {"check_id": "no_tooling_artifacts", "status": "pass"}, +] # computes to LGTM — disagrees with the CONCERNS answer +_RIGHT = [ + {"check_id": "code_quality", "status": "fail"}, + {"check_id": "no_tooling_artifacts", "status": "pass"}, +] # computes to CONCERNS — matches the answer + + +def test_claude_family_drift_caught_even_when_codex_agrees(): + """The essential C3 property: a dominant/agreeing family can't mask a + DIFFERENT family's drift. claude_code alone disagrees with the signed + answer; codex_cli fully agrees. The panel must still flag drift, and + must attribute it to claude_code specifically.""" + panel = {"claude_code": _fixed(_WRONG), "codex_cli": _fixed(_RIGHT)} + results = run_panel_drift_monitor([_case()], panel, votes=3) + r = results[0] + assert isinstance(r, PanelDriftResult) + assert r.drifted is True + assert r.per_family["claude_code"]["majority"] != r.expected + assert r.per_family["codex_cli"]["majority"] == r.expected + + +def test_codex_family_drift_caught_even_when_claude_agrees(): + """Symmetric case: codex_cli disagrees, claude_code agrees — still flagged, + and still attributed to the actual offending family (codex_cli).""" + panel = {"claude_code": _fixed(_RIGHT), "codex_cli": _fixed(_WRONG)} + results = run_panel_drift_monitor([_case()], panel, votes=3) + r = results[0] + assert r.drifted is True + assert r.per_family["codex_cli"]["majority"] != r.expected + assert r.per_family["claude_code"]["majority"] == r.expected + + +def test_no_drift_when_every_family_agrees(): + panel = {"claude_code": _fixed(_RIGHT), "codex_cli": _fixed(_RIGHT)} + results = run_panel_drift_monitor([_case()], panel, votes=3) + r = results[0] + assert r.drifted is False + assert all(fam["majority"] == r.expected for fam in r.per_family.values()) + + +def test_per_family_majority_vote_is_independent_per_family(): + """Each family's majority is computed from ONLY its own votes, never + pooled with another family's — a flaky claude vote can't be smoothed out + by codex's votes (or vice versa).""" + + def flaky_claude(case, *, vote): + # 2 of 3 votes wrong, 1 right — majority is still wrong for this family. + return _RIGHT if vote == 1 else _WRONG + + panel = {"claude_code": flaky_claude, "codex_cli": _fixed(_RIGHT)} + results = run_panel_drift_monitor([_case()], panel, votes=3) + r = results[0] + assert r.per_family["claude_code"]["agree_votes"] == 2 + assert r.per_family["claude_code"]["total_votes"] == 3 + assert r.per_family["claude_code"]["majority"] != r.expected + assert r.drifted is True + + +def test_rejects_zero_votes(): + with pytest.raises(ValueError): + run_panel_drift_monitor([_case()], {"claude_code": _fixed(_RIGHT)}, votes=0) + + +def test_rejects_empty_family_extractors(): + """Empty panel has no cross-family control — callers must treat 'no + panel configured' as feature-OFF before reaching here, not by handing an + empty map through (see DriftMonitorTask, which does exactly that).""" + with pytest.raises(ValueError): + run_panel_drift_monitor([_case()], {}, votes=3) + + +def test_multiple_cases_each_get_their_own_result(): + panel = {"claude_code": _fixed(_RIGHT), "codex_cli": _fixed(_WRONG)} + cases = [_case("a"), _case("b")] + results = run_panel_drift_monitor(cases, panel, votes=1) + assert [r.case_id for r in results] == ["a", "b"] + assert all(r.drifted for r in results) diff --git a/tests/unit/eval/test_panel_invoker.py b/tests/unit/eval/test_panel_invoker.py new file mode 100644 index 000000000..491eb192b --- /dev/null +++ b/tests/unit/eval/test_panel_invoker.py @@ -0,0 +1,137 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 ProtocolWarden +"""C3 — live cross-family invoker: family resolution, availability probing, +and the codex-stdout-instead-of-file fallback (verdict.last_json_object).""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from operations_center.eval import panel_invoker +from operations_center.eval.corpus import Case +from operations_center.eval.panel_invoker import ( + DEFAULT_MODEL_BY_FAMILY, + LiveFamilyExtractor, + build_family_extractor, + build_panel_extractors, + resolve_available_families, +) + + +def _case() -> Case: + return Case( + case_id="c", + kind="extraction", + input={"diff": "def f():\n- return 1\n+ return None"}, + ground_truth={"result": "CONCERNS", "failing": ["code_quality"]}, + ) + + +class _FakeProc: + def __init__(self, stdout: str = "", returncode: int = 0) -> None: + self.stdout = stdout + self.returncode = returncode + + +def test_resolve_available_families_filters_missing_binaries(monkeypatch): + monkeypatch.setattr( + panel_invoker.shutil, "which", lambda name: "/usr/bin/claude" if name == "claude" else None + ) + assert resolve_available_families(["claude_code", "codex_cli"]) == ["claude_code"] + + +def test_resolve_available_families_all_present(monkeypatch): + monkeypatch.setattr(panel_invoker.shutil, "which", lambda name: f"/usr/bin/{name}") + assert resolve_available_families(["claude_code", "codex_cli"]) == ["claude_code", "codex_cli"] + + +def test_resolve_available_families_none_present(monkeypatch): + monkeypatch.setattr(panel_invoker.shutil, "which", lambda name: None) + assert resolve_available_families(["claude_code", "codex_cli"]) == [] + + +def test_build_family_extractor_known_families_use_defaults(): + for family, model in DEFAULT_MODEL_BY_FAMILY.items(): + ext = build_family_extractor(family) + assert isinstance(ext, LiveFamilyExtractor) + assert ext._model == model + assert ext._backend == family + + +def test_build_family_extractor_unknown_family_raises(): + with pytest.raises(ValueError): + build_family_extractor("some_unknown_family") + + +def test_build_family_extractor_explicit_model_overrides_default(): + ext = build_family_extractor("claude_code", model="haiku") + assert ext._model == "haiku" + + +def test_build_panel_extractors_builds_one_per_family(): + extractors = build_panel_extractors(["claude_code", "codex_cli"]) + assert set(extractors) == {"claude_code", "codex_cli"} + assert all(isinstance(e, LiveFamilyExtractor) for e in extractors.values()) + + +def test_live_extractor_prefers_verdict_json_file(monkeypatch): + """The claude-style contract: the backend writes verdict.json to its cwd.""" + written = {"checks": [{"check_id": "code_quality", "status": "fail"}]} + + def _fake_run(argv, *, cwd, capture_output, text, timeout): + (Path(cwd) / "verdict.json").write_text(json.dumps(written), encoding="utf-8") + return _FakeProc() + + monkeypatch.setattr(panel_invoker.subprocess, "run", _fake_run) + ext = LiveFamilyExtractor("claude_code", "sonnet") + assert ext(_case(), vote=0) == written["checks"] + + +def test_live_extractor_falls_back_to_stdout_codex_style(monkeypatch): + """Codex-stdout fallback: no verdict.json written, answer only on stdout, + prose-wrapped — reuses verdict.last_json_object (same fallback the C1 + council's _run_member_review uses).""" + + def _fake_run(argv, *, cwd, capture_output, text, timeout): + return _FakeProc( + stdout=( + "Sure, here is my review:\n" + '{"checks": [{"check_id": "code_quality", "status": "pass"}]}\n' + "Hope that helps!" + ) + ) + + monkeypatch.setattr(panel_invoker.subprocess, "run", _fake_run) + ext = LiveFamilyExtractor("codex_cli", "codex") + checks = ext(_case(), vote=0) + assert checks == [{"check_id": "code_quality", "status": "pass"}] + + +def test_live_extractor_returns_empty_on_unparseable_stdout(monkeypatch): + monkeypatch.setattr( + panel_invoker.subprocess, "run", lambda *a, **k: _FakeProc(stdout="not json at all") + ) + ext = LiveFamilyExtractor("codex_cli", "codex") + assert ext(_case(), vote=0) == [] + + +def test_live_extractor_returns_empty_on_subprocess_failure(monkeypatch): + def _boom(*a, **k): + raise OSError("no such binary") + + monkeypatch.setattr(panel_invoker.subprocess, "run", _boom) + ext = LiveFamilyExtractor("codex_cli", "codex") + # A crash reads as drift (empty checks -> CONCERNS), never a silent pass. + assert ext(_case(), vote=0) == [] + + +def test_live_extractor_unsupported_backend_returns_empty_without_spawning(monkeypatch): + def _boom(*a, **k): + raise AssertionError("must not spawn a subprocess for an unsupported backend") + + monkeypatch.setattr(panel_invoker.subprocess, "run", _boom) + ext = LiveFamilyExtractor("unknown_backend", "model") + assert ext(_case(), vote=0) == []