From 4ec778c8556ae9972b09c644925f756938ce75cd Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Tue, 30 Jun 2026 07:24:30 +0200 Subject: [PATCH] feat(fase3): make the maker-checker checker actually gate the reasoning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes gap #3 (maalbilde §5): the GroupChat checker critiqued into the void — output_from=[proposer] surfaced only the proposer, so an explicit checker rejection was ignored and the deterministic validator was the sole gate. Two falsifiers now act on the same candidate: the validator gates the NUMBERS (blocking, unchanged), the checker gates the REASONING (maalbilde §2/§6). - workflow.py: output_from=agents surfaces both participants; the checker instruction ends with a VERDICT: APPROVE / VERDICT: REJECT - line. - run.py: _authored_texts() reads author_name through out.messages (MAF 1.9.0 puts it there, not on the AgentResponse); _debate_text() now selects the PROPOSER-authored output (fixes a latent texts[-1] regression that would feed the checker's verdict to generation at even round counts); _checker_verdict() parses the gate decision. An explicit REJECT overrides an otherwise-validated outcome to a checker-sourced Rejection. Opt-in-reject (fail-open on a missing marker). RunResult gains checker_verdict; provenance.validator_decision is stamped from the validator outcome BEFORE the override, so it never conflates the two falsifiers (provenance honesty). Load-bearing (maalbilde §7): tests/test_checker_gate_loadbearing.py is a PAIR — an explicit checker REJECT on a VALIDATOR-VALID proposal yields a Rejection whose reason carries the checker's reason while validator_decision stays "validated"; the causality control (checker APPROVE, same proposer) validates normally. Proven RED on BOTH detach points (revert output_from, or drop the override). Suite 134->136 passed, 4 skipped; mypy + ruff check clean. Pre-existing ruff-format drift (backends/budget/verdicts/test_contracts) left untouched for a surgical diff. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01MHR8iKxJRxDiDfNw8HZmWE --- CLAUDE.md | 9 +++ README.md | 3 +- src/portfolio_optimiser/run.py | 89 +++++++++++++++------ src/portfolio_optimiser/workflow.py | 23 ++++-- tests/test_checker_gate_loadbearing.py | 102 +++++++++++++++++++++++++ 5 files changed, 194 insertions(+), 32 deletions(-) create mode 100644 tests/test_checker_gate_loadbearing.py diff --git a/CLAUDE.md b/CLAUDE.md index efb110f..a81523b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,6 +37,15 @@ Python ≥3.10. MAF (`agent-framework-core` 1.9.0). Pakkehåndtering: `uv`. To b `test_step1_expel_loadbearing.py` (realiseringssignalet lekker aldri inn via kontekst). - **Stoppkriterier + budsjett-tak påkrevd ved oppstart** (fail-fast, aldri ubegrenset loop). - **Group Chat maker-checker** som debatt-default (IKKE Magentic, som er eksperimentell). +- **To falsifiserere, samme kandidat (Steg 3/4, målbilde §2/§6):** den deterministiske validatoren + gater *tallene* (blokkerende), checkeren gater *resonnementet*. Checkeren avslutter turen med en + `VERDICT: APPROVE` / `VERDICT: REJECT — `-linje; et eksplisitt avslag blokkerer et ellers + validert forslag (`run_project` overflater begge debatt-deltakere via `output_from=agents` og + overstyrer utfallet til en checker-kilde-`Rejection`). Gaten er opt-in-reject (fail-open ved + manglende markør), og `provenance.validator_decision` forblir ærlig — den speiler KUN validatoren, + aldri checkeren (de to falsifisererne blandes aldri). Load-bearing: + `tests/test_checker_gate_loadbearing.py` blir rød ved BEGGE detach-punkt (revert `output_from`, + eller fjern override). Checkeren «må faktisk gate, ELLER vi slutter å kalle det maker-checker». - **Kostnadsdisiplin:** utvikle primært på lokal profil (gratis); Foundry/Azure (privat tenant finnes) kun til målrettet, minimal verifisering; billigste modeller + små syntetiske data + harde token-tak. Ingen tunge test-kjøringer. - **STATE.md er local-only** (gitignored). Voyage session-state er efemert; STATE.md er kanonisk kontinuitet. - Prosess: Voyage-plugin (`/trekbrief → /trekplan → /trekexecute → /trekreview`) per større fase. diff --git a/README.md b/README.md index 6d9c8df..3e40c70 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,8 @@ The mandatory deterministic backbone (validator + budget meter + provenance) is - **Context by navigation, not stuffing.** The agent read-context is built by **navigating** the project's [OKF bundle](shared/examples/bygg-energi-mikro/) (`index.md` + frontmatter + cross-links, progressive disclosure) — never keyword chunk-stuffing (target picture §2/§4). - **Verdict layer gated out of context.** The `type: verdict` layer is excluded from that context; the candidate's prior expert verdicts reach the hypothesis prompt *only* through the gated ExpeL fold (folded in *before* generation), so a prior verdict provably — and exclusively — influences the next hypothesis. - Two load-bearing tests fail when the seam is detached: the realization signal must reach the prompt via the fold (`tests/test_step1_expel_loadbearing.py`), and must never leak via context (`tests/test_okf.py::test_bundle_context_excludes_verdict_layer`). -- **Steps 3–8** (checker gating, informed refinement, async file feedback, gated wiki promotion) — not yet wired. +- **Steps 3/4 — checker gates the reasoning (wired).** Two falsifiers now act on the same candidate: the deterministic validator gates the **numbers** (blocking, as before), and the maker-checker's checker gates the **reasoning** (target picture §2/§6). The checker ends its turn with a `VERDICT: APPROVE` / `VERDICT: REJECT — ` line; an explicit reject blocks an otherwise-validated proposal (`run_project` surfaces both debate participants via `output_from=agents` and overrides the outcome to a checker-sourced `Rejection`). The gate is opt-in-reject (fail-open on a missing marker), and `provenance.validator_decision` stays honest — it reflects the validator only, never the checker. Load-bearing: `tests/test_checker_gate_loadbearing.py` goes red on *either* detach (revert `output_from`, or drop the override). +- **Steps 5–8** (informed refinement, async file feedback, gated wiki promotion) — not yet wired. ## Docs diff --git a/src/portfolio_optimiser/run.py b/src/portfolio_optimiser/run.py index d8ee9d3..34b5fac 100644 --- a/src/portfolio_optimiser/run.py +++ b/src/portfolio_optimiser/run.py @@ -60,8 +60,11 @@ from portfolio_optimiser.workflow import fresh_workflow @dataclass(frozen=True) class RunResult: """The outcome of one project run: the validated/rejected proposal, its first-class - provenance, the captured (Layer-2) verdict, the ExpeL hits surfaced for it, the store, and - the debate's converged output that the candidate was generated from (F1 traceability).""" + provenance, the captured (Layer-2) verdict, the ExpeL hits surfaced for it, the store, the + debate's converged output that the candidate was generated from (F1 traceability), and the + checker's gate decision (Step 3/4: ``"approve" | "reject" | "absent"``). ``checker_verdict`` + records the checker's decision distinctly from ``provenance.validator_decision`` so the two + falsifiers (reasoning vs numbers) are never conflated.""" outcome: ValidatedProposal | Rejection provenance: ProvenanceStamp @@ -69,6 +72,7 @@ class RunResult: retrieved: list[Verdict] store: VerdictStore debate_output: str + checker_verdict: str = "absent" @dataclass(frozen=True) @@ -90,24 +94,47 @@ class PortfolioResult: sum_token_usage: int -# The GroupChat orchestrator emits this terminal notice as a participant output; it is NOT the -# debate's substantive result, so the F1 extractor filters it out (verified against MAF 1.9.0). -_ORCH_NOTICE = "reached the maximum number of rounds" +def _authored_texts(result: Any, name: str) -> list[str]: + """The texts of ``get_outputs()`` entries authored by participant ``name`` (proposer/checker), + in surfaced order. MAF surfaces ``author_name`` on each output's ``messages`` — NOT on the + ``AgentResponse`` itself (verified against 1.9.0) — so we match through ``messages``. This + separates the proposer's converged output (fed to generation, F1) from the checker's gate + verdict (Step 3/4); the orchestrator's termination notice is authored by neither, so it is + excluded automatically.""" + texts: list[str] = [] + for out in result.get_outputs(): + if not any(getattr(m, "author_name", None) == name for m in getattr(out, "messages", [])): + continue + text = out if isinstance(out, str) else getattr(out, "text", None) + if text: + texts.append(text) + return texts def _debate_text(result: Any) -> str: - """Extract the proposer's converged output from a ``WorkflowRunResult`` (surfaced by - ``output_from=[proposer]``), excluding the orchestrator's termination notice. Returns ``""`` - when no substantive participant text is present.""" - texts: list[str] = [] - for out in result.get_outputs(): - text = out if isinstance(out, str) else getattr(out, "text", None) - if not text: - continue - if getattr(out, "author_name", None) == "group_chat_orchestrator" or _ORCH_NOTICE in text: - continue - texts.append(text) - return texts[-1] if texts else "" + """The PROPOSER's converged output (fed into generation, F1). With ``output_from=agents`` both + participants surface, so we select proposer-authored outputs specifically — taking the last of + ALL surfaced outputs would feed the checker's verdict to generation at even round counts. + Returns ``""`` when the proposer produced no surfaced text.""" + proposer_texts = _authored_texts(result, "proposer") + return proposer_texts[-1] if proposer_texts else "" + + +def _checker_verdict(result: Any) -> tuple[str, str]: + """Parse the checker's gate verdict from its surfaced debate output (Step 3/4, målbilde §2/§6). + Returns ``(decision, reason)``: ``"reject"`` ONLY on an explicit ``VERDICT: REJECT`` (with the + trailing reason), ``"approve"`` on an explicit ``VERDICT: APPROVE``, else ``"absent"``. The gate + is opt-in-reject (fail-open): a missing/unparseable marker never blocks, so the deterministic + validator stays the sole gate on those runs.""" + checker_texts = _authored_texts(result, "checker") + text = checker_texts[-1] if checker_texts else "" + upper = text.upper() + if "VERDICT: REJECT" in upper: + reason = text[upper.index("VERDICT: REJECT") + len("VERDICT: REJECT") :] + return "reject", reason.lstrip(" -:—").strip() + if "VERDICT: APPROVE" in upper: + return "approve", "" + return "absent", "" def _project_by_id(project_id: str) -> Project: @@ -232,8 +259,10 @@ async def run_project( ) result = await debate.run(f"Find a cost-saving measure for {project.id}.\nContext:\n{context}") # F1: the candidate must derive from the DEBATE. Feed the proposer's converged output into - # generation (retrieval context is the last-resort fallback only). + # generation (retrieval context is the last-resort fallback only). The checker's verdict + # (Step 3/4) is parsed from the SAME debate result and gates the outcome below. debate_output = _debate_text(result) + checker_decision, checker_reason = _checker_verdict(result) gen_context = debate_output or context # Step-1 ExpeL wiring (Fase 2a, målbilde §5/§7): fold the candidate's prior verdicts INTO the @@ -247,15 +276,17 @@ async def run_project( fewshot = ExpeLContextProvider(store, expel_query, k=top_k).format_fewshot() gen_context = f"{fewshot}\n\n{gen_context}" - # 5. Structured candidate -> blocking validation; token bound = the meter in this loop. + # 5. Structured candidate -> blocking validation on the NUMBERS; token bound = the meter. proposer_client = factory("proposer") - outcome = await generate_via_llm(proposer_client, project, gen_context, meter) - proposal = outcome.proposal + validator_outcome = await generate_via_llm(proposer_client, project, gen_context, meter) + proposal = validator_outcome.proposal # 6. First-class provenance stamp (authoritative; independent of MAF Annotation). # F1: an injected client_factory stamps the injected client's REAL model ("unknown" is the # neutral fallback for a client that doesn't surface one -- never a fabricated name); the - # default path keeps the deterministic resolve_model. + # default path keeps the deterministic resolve_model. validator_decision reflects the VALIDATOR + # (the numbers) ONLY -- stamped from validator_outcome BEFORE the checker override below, so a + # checker-gated proposal whose numbers passed is never mislabelled as validator-rejected. model = ( (getattr(proposer_client, "model", None) or "unknown") if client_factory is not None @@ -265,10 +296,21 @@ async def run_project( citations=citations, model=model, role="proposer", - validator_decision="validated" if isinstance(outcome, ValidatedProposal) else "rejected", + validator_decision=( + "validated" if isinstance(validator_outcome, ValidatedProposal) else "rejected" + ), token_usage=meter.tokens, ) + # 6b. Step 3/4 checker gate (målbilde §2/§6): the validator falsifies the numbers, the checker + # falsifies the reasoning. An explicit checker REJECT blocks an otherwise-validated proposal; a + # validator rejection (the numbers) already stands. Fail-open: APPROVE/absent never blocks. + outcome: ValidatedProposal | Rejection + if isinstance(validator_outcome, ValidatedProposal) and checker_decision == "reject": + outcome = Rejection(proposal=proposal, reason=f"checker rejected: {checker_reason}") + else: + outcome = validator_outcome + # 7. ExpeL (regression guard + traceability): exercises the two-arg extend_instructions # injection on a REAL SessionContext (the Critical Fase-1 GA-signature guard), and surfaces # the proposal-keyed retrieval for RunResult.retrieved. On the bundle path the load-bearing @@ -294,6 +336,7 @@ async def run_project( retrieved=retrieved, store=store, debate_output=debate_output, + checker_verdict=checker_decision, ) diff --git a/src/portfolio_optimiser/workflow.py b/src/portfolio_optimiser/workflow.py index f07dc62..65ec78b 100644 --- a/src/portfolio_optimiser/workflow.py +++ b/src/portfolio_optimiser/workflow.py @@ -26,7 +26,12 @@ from agent_framework.orchestrations import GroupChatBuilder _MAKER_CHECKER_ROLES = ("proposer", "checker") _INSTRUCTIONS = { "proposer": "You propose one concrete cost-saving measure for the project.", - "checker": "You critique the proposal and flag any constraint violation.", + "checker": ( + "You critique the proposal's reasoning and flag any constraint violation. " + "End your reply with exactly one verdict line: 'VERDICT: APPROVE' if the reasoning " + "holds, or 'VERDICT: REJECT - ' if it does not. The validator gates the " + "numbers; your verdict gates the reasoning (Step 3/4 — målbilde §2)." + ), } @@ -79,10 +84,11 @@ def fresh_workflow( is called once per role, so each run owns its own clients — no state survives between runs. ``tools`` + ``middleware`` are attached to each agent (F2/F7; constructed by the - orchestrator). ``output_from=[proposer]`` makes ``WorkflowRunResult.get_outputs()`` surface - the proposer's converged output — without it, ``get_outputs()`` yields only the - orchestrator's "reached max rounds" notice, so the F1 debate->generation dataflow could not - read the debate result (verified against installed 1.9.0). + orchestrator). ``output_from=agents`` makes ``WorkflowRunResult.get_outputs()`` surface BOTH + participants' converged outputs — the proposer's (fed to generation, F1) AND the checker's + gate verdict (Step 3/4); ``run.py`` separates them by per-message ``author_name``. Without it, + ``get_outputs()`` yields only the orchestrator's "reached max rounds" notice (verified against + installed 1.9.0). """ agents = maker_checker_agents(client_factory, tools=tools, middleware=middleware) # Agents are built from _MAKER_CHECKER_ROLES in order with name=role, so the role tuple @@ -100,9 +106,10 @@ def fresh_workflow( selection_func=select, # Safety net well above the hard cap; with_max_rounds is the binding bound (B4). termination_condition=make_termination(max_rounds * len(names) + 1), - # Surface the PROPOSER's converged output so get_outputs() carries the debate - # result (F1); the default surfaces only the orchestrator's termination notice. - output_from=[agents[0]], + # Surface BOTH participants so get_outputs() carries the proposer's converged output + # (fed to generation, F1) AND the checker's gate verdict (Step 3/4); run.py separates + # them by author_name. The default surfaces only the orchestrator's termination notice. + output_from=agents, ).with_max_rounds(max_rounds) if enable_layer1_hitl: # Layer-1: in-run synchronous review on the checker (no checkpoint — research 01). diff --git a/tests/test_checker_gate_loadbearing.py b/tests/test_checker_gate_loadbearing.py new file mode 100644 index 0000000..a3968ea --- /dev/null +++ b/tests/test_checker_gate_loadbearing.py @@ -0,0 +1,102 @@ +"""Step 3/4 load-bearing seam (målbilde §2/§6/§7): an explicit checker REJECT MUST gate. + +The gap (fot-i-bakken, verified in code): the GroupChat checker critiqued into the void — +``output_from=[proposer]`` surfaced only the proposer, so an explicit checker rejection was +ignored and the ONLY blocking gate was the deterministic validator (on the *numbers*). Fase 3 +surfaces the checker verdict and makes an explicit ``VERDICT: REJECT`` block an otherwise- +validated proposal — qualitative falsification on the *reasoning*, distinct from the validator +on the *numbers* (målbilde §2 step 4). + +These two tests are a load-bearing PAIR, not a smoke test (per the Fase-2 green-but-dead trap): +- the positive test uses a VALIDATOR-VALID proposer, so the ONLY possible rejecter is the + checker; it asserts the checker's reason propagates into ``outcome.reason`` AND that provenance + stays honest (``validator_decision == "validated"``). It goes RED on EITHER detach: revert + ``output_from=agents`` to proposer-only, OR drop the run.py checker override. +- the approve test is the causality control: the SAME valid proposer with a checker APPROVE + validates normally — proving the rejection above is CAUSED by the checker verdict, not the + fixture. +""" + +from __future__ import annotations + +from collections.abc import Callable +from pathlib import Path + +from agent_framework import BaseChatClient +from conftest import SyntheticUsageChatClient + +from portfolio_optimiser.run import run_project +from portfolio_optimiser.validator import Rejection, ValidatedProposal + +BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro" + +# A VALIDATOR-VALID BYGG-KONTOR-NORD proposal (same shape as the Step-1 test): affected total +# 300000 x 1.0; no assumptions -> degenerate Monte Carlo P90 = 0.30 x 300000 = 90000 >= claimed +# 30000 -> validates on the first attempt. So in the reject test the ONLY possible rejecter is +# the checker, never the validator (closes the green-but-dead trap). +_VALID_PROPOSER_REPLY = ( + '{"measure":"LED-retrofit av kontorbelysning","affected_items":' + '[{"code":"ENERGI-TOTAL-EL","quantity":300000,"unit_cost":1.0}],"claimed_saving_nok":30000}' +) +_VERDICT_INPUT = {"decision": "approved", "rationale": "expert reviewed (sim)"} +# A unique reason substring: it can only reach outcome.reason if the checker's output actually +# reached the gate (the "third assertion" that makes the pair load-bearing, not green-but-dead). +_REJECT_REASON = "payback exceeds horizon (sim-unique a1b2c3)" + + +def _role_factory(proposer_reply: str, checker_reply: str) -> Callable[[str], BaseChatClient]: + """A role-aware synthetic factory: the checker speaks its verdict, the proposer its proposal. + The existing make_*_client_factory fixtures are role-agnostic, so the gate needs this.""" + + def factory(role: str) -> BaseChatClient: + return SyntheticUsageChatClient( + default_reply=checker_reply if role == "checker" else proposer_reply + ) + + return factory + + +async def test_explicit_checker_reject_gates_a_validated_proposal() -> None: + """LOAD-BEARING: an explicit checker REJECT blocks an otherwise-validated proposal. Goes RED + if ``output_from`` drops the checker OR the run.py checker override is removed.""" + factory = _role_factory(_VALID_PROPOSER_REPLY, f"VERDICT: REJECT - {_REJECT_REASON}") + + result = await run_project( + "BYGG-KONTOR-NORD", + "local", + docs_dir=str(BUNDLE_DIR), + bundle_dir=str(BUNDLE_DIR), + verdict_input=_VERDICT_INPUT, + client_factory=factory, + ) + + assert isinstance(result.outcome, Rejection), ( + "checker said REJECT but the proposal was not gated — the checker verdict is ignored" + ) + assert _REJECT_REASON in result.outcome.reason, ( + "the checker's reason did not propagate into the outcome — its output never reached the gate" + ) + # Provenance honesty: the VALIDATOR passed (the numbers are feasible); only the checker + # rejected. validator_decision must reflect the numbers, never conflate the two falsifiers. + assert result.provenance.validator_decision == "validated", ( + "provenance conflates the two falsifiers — validator_decision must reflect the numbers only" + ) + assert result.checker_verdict == "reject" + + +async def test_checker_approve_lets_a_validated_proposal_through() -> None: + """CAUSALITY CONTROL: the SAME valid proposer with a checker APPROVE validates normally — + proving the rejection above is caused by the checker verdict, not the fixture's mere shape.""" + factory = _role_factory(_VALID_PROPOSER_REPLY, "VERDICT: APPROVE") + + result = await run_project( + "BYGG-KONTOR-NORD", + "local", + docs_dir=str(BUNDLE_DIR), + bundle_dir=str(BUNDLE_DIR), + verdict_input=_VERDICT_INPUT, + client_factory=factory, + ) + + assert isinstance(result.outcome, ValidatedProposal) + assert result.checker_verdict == "approve"