run_project KREVDE verdict_input og kjorte capture_verdict ubetinget; CLI-en
defaultet det til {"approved", "reviewed by expert"} og hosting listet det som
PAAKREVD. Netto: hver flaggloes kjoering myntet en ekspertgodkjenning ingen ga,
den gikk inn i den delte storen, og run_portfolio bar den inn i neste prosjekts
hypotese-prompt som en prior expert verdict -- paa flaten som ble overlevert
14.08. Non-goal 3, brutt i en soem.
RunResult.verdict er naa Verdict | None, og None er hva stillhet produserer:
ingenting myntes, ingenting lagres, ingenting varsles. Prinsippet sto allerede i
repoet -- RunFailure sin docstring: aa fylle et felt med en dummy legger
FABRIKKERT proveniens inn i aggregatet.
Traceability koster ingenting: RunResult.verdict_key (property, derivert fra
kandidaten) er verdicts.verdict_key sitt alt dokumenterte formaal -- identisk
med verdict.id naar en dom BLE gitt, og fortsatt meningsfull naar ingen ble det.
Det er den outboxen og den hostede responsen stempler.
Halv dom NEKTES paa begge doerer (FeedbackContract er eneste sted formen
valideres; CLI-en nekter ved navn FOER enhver mode-dispatch). Validering, aldri
reparasjon. De to mode-partisjonene fikk --decision/--rationale inn: kommentarene
sa ordrett at en aerlig nekt var uimplementerbar fordi de non-None
argparse-defaultene gjorde en eksplisitt verdi uskillbar fra defaulten -- med
defaultene borte er den implementerbar.
Hosting er WIDENING, ikke bryting: verdict_input flyttet fra _REQUIRED_FIELDS
til _OPTIONAL_FIELDS. Ingen ekstern kaller brekker.
AERLIGHETS-GRENSE: referanse-fixturens SYNTETISKE verdict_input-rader staar
uroert -- de er merket SYNTETISK paa fire steder og er reviewens F5 (maaling av
misjonspaastanden), ikke F2. Project.verdict_input er naa valgfri.
Load-bearing MAALT (tests/test_ungiven_verdict_loadbearing.py, 15 armer), aatte
mutasjoner alle roede mot HELE suiten + gronn kontroll 1080/5 og golden
demo-transcript.stdout BYTE-UENDRET (ea8c534773acdbe41ae68f2c55724d69aaf8be4f).
En mutasjon falsifiserte testen foerst (vakuoes-gate-klassen, ellevte gang):
--report-armen brukte et bart --report, som nekter rc 1 uansett fordi --ledger
mangler.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
420 lines
18 KiB
Python
420 lines
18 KiB
Python
"""F2 (misjonsreview ``docs/2026-08-25-fable-misjonsreview.md``, non-goal 3): an expert verdict
|
|
must not be able to ARISE without an expert having given one, and a verdict nobody gave must not
|
|
PROPAGATE into the next project's hypothesis prompt as if it were one.
|
|
|
|
Before this seam, ``run_project`` REQUIRED ``verdict_input`` and unconditionally ran
|
|
``capture_verdict(features, verdict_input["decision"], ...)``. The CLI defaulted that to
|
|
``{"decision": "approved", "rationale": "reviewed by expert"}``, so every flagless run minted an
|
|
approval nobody spoke; the hosted surface listed the field as REQUIRED, so an external caller had
|
|
to invent one to get a run at all; and in ``run_portfolio`` the minted verdict entered the shared
|
|
store and reached the next project's ExpeL few-shot.
|
|
|
|
The tests here fasten the GOAL, not the mechanism: absence of a verdict must be REPRESENTABLE and
|
|
must be what silence produces. Every negative assert is paired with a control that proves the
|
|
event it denies can actually happen — a test that can only be green proves nothing (this repo's
|
|
recurring vacuous-gate class).
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import shutil
|
|
from collections.abc import Callable
|
|
from importlib.resources import files
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
import pytest
|
|
from agent_framework import BaseChatClient
|
|
from pydantic import ValidationError
|
|
from conftest import SyntheticUsageChatClient
|
|
|
|
from portfolio_optimiser import hosting, run
|
|
from portfolio_optimiser.ledger import LedgerEntry, SavingsLedger
|
|
from portfolio_optimiser.reference_domain import Project
|
|
from portfolio_optimiser.run import run_portfolio, run_project
|
|
|
|
BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
|
|
_PID = "BYGG-KONTOR-NORD"
|
|
_MINI_BUNDLE = str(files("portfolio_optimiser").joinpath("data/bundles/bygg-energi-mikro-a"))
|
|
|
|
_VALID_PROPOSER_REPLY = (
|
|
'{"measure":"LED-retrofit av kontorbelysning","affected_items":'
|
|
'[{"code":"ENERGI-TOTAL-EL","quantity":300000,"unit_cost":1.0}],"claimed_saving_nok":30000}'
|
|
)
|
|
_CHECKER_APPROVE = "The retrofit is supported by the cited documents. VERDICT: APPROVE"
|
|
_GIVEN = {"decision": "approved", "rationale": "expert reviewed (sim)"}
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _isolate_model_env(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
monkeypatch.delenv("PORTFOLIO_MODEL_MAP", raising=False)
|
|
monkeypatch.delenv("PORTFOLIO_FOUNDRY_PROJECT_ENDPOINT", raising=False)
|
|
|
|
|
|
def _role_factory(proposer_reply: str, checker_reply: str) -> Callable[[str], BaseChatClient]:
|
|
def factory(role: str) -> BaseChatClient:
|
|
return SyntheticUsageChatClient(
|
|
default_reply=checker_reply if role == "checker" else proposer_reply
|
|
)
|
|
|
|
return factory
|
|
|
|
|
|
@pytest.fixture()
|
|
def bundle(tmp_path: Path) -> Path:
|
|
"""A throwaway COPY — the shared fixture is commons-owned and is never mutated by a test."""
|
|
dst = tmp_path / "bundle"
|
|
shutil.copytree(BUNDLE_DIR, dst)
|
|
return dst
|
|
|
|
|
|
@pytest.fixture()
|
|
def replies_file(tmp_path: Path) -> Path:
|
|
path = tmp_path / "replies.json"
|
|
path.write_text(
|
|
json.dumps({"proposer": _VALID_PROPOSER_REPLY, "checker": _CHECKER_APPROVE}),
|
|
encoding="utf-8",
|
|
)
|
|
return path
|
|
|
|
|
|
def _argv(bundle: Path, replies_file: Path) -> list[str]:
|
|
return [
|
|
_PID,
|
|
"--docs-dir",
|
|
str(bundle),
|
|
"--bundle-dir",
|
|
str(bundle),
|
|
"--scripted-replies",
|
|
str(replies_file),
|
|
]
|
|
|
|
|
|
# --- T1: a verdict cannot ARISE from silence (library seam) ---------------------------------
|
|
|
|
|
|
async def test_run_without_verdict_input_captures_no_verdict(tmp_path) -> None:
|
|
"""T1 GOAL: a run nobody reviewed produces NO verdict — the field is ``None`` and the store
|
|
stays empty. RED before the seam: ``verdict_input`` was a REQUIRED keyword and step 8 minted
|
|
an approval unconditionally."""
|
|
result = await run_project(
|
|
_PID,
|
|
"local",
|
|
docs_dir=str(BUNDLE_DIR),
|
|
bundle_dir=str(BUNDLE_DIR),
|
|
client_factory=_role_factory(_VALID_PROPOSER_REPLY, _CHECKER_APPROVE),
|
|
max_rounds=2,
|
|
)
|
|
assert result.verdict is None, "a verdict nobody gave was minted anyway"
|
|
assert result.store.verdicts == [], "an ungiven verdict entered the learning store"
|
|
|
|
|
|
async def test_run_with_verdict_input_still_captures_it(tmp_path) -> None:
|
|
"""T1 CONTROL: when an expert DOES speak, the verdict is captured and stored exactly as
|
|
before. Without this arm T1 would pass against an implementation that never captures at all."""
|
|
result = await run_project(
|
|
_PID,
|
|
"local",
|
|
docs_dir=str(BUNDLE_DIR),
|
|
bundle_dir=str(BUNDLE_DIR),
|
|
verdict_input=_GIVEN,
|
|
client_factory=_role_factory(_VALID_PROPOSER_REPLY, _CHECKER_APPROVE),
|
|
max_rounds=2,
|
|
)
|
|
assert result.verdict is not None
|
|
assert result.verdict.decision == "approved"
|
|
assert result.verdict.rationale == "expert reviewed (sim)"
|
|
assert [v.id for v in result.store.verdicts] == [result.verdict.id]
|
|
|
|
|
|
async def test_verdict_key_is_available_even_with_no_verdict(tmp_path) -> None:
|
|
"""T1b: the KEY an expert verdict on this candidate will arrive under is always available —
|
|
it is derived from the candidate, not from a decision. This is what keeps the outbox artefact
|
|
and the hosted response judgeable on a run nobody has reviewed yet (``verdicts.verdict_key``'s
|
|
own documented purpose), so 'no verdict' costs no traceability."""
|
|
result = await run_project(
|
|
_PID,
|
|
"local",
|
|
docs_dir=str(BUNDLE_DIR),
|
|
bundle_dir=str(BUNDLE_DIR),
|
|
client_factory=_role_factory(_VALID_PROPOSER_REPLY, _CHECKER_APPROVE),
|
|
max_rounds=2,
|
|
)
|
|
assert result.verdict is None
|
|
assert result.verdict_key, "the run must still name the key a verdict on it would arrive under"
|
|
|
|
given = await run_project(
|
|
_PID,
|
|
"local",
|
|
docs_dir=str(BUNDLE_DIR),
|
|
bundle_dir=str(BUNDLE_DIR),
|
|
verdict_input=_GIVEN,
|
|
client_factory=_role_factory(_VALID_PROPOSER_REPLY, _CHECKER_APPROVE),
|
|
max_rounds=2,
|
|
)
|
|
# Same candidate -> same key, and a captured verdict keys under exactly it.
|
|
assert given.verdict is not None
|
|
assert given.verdict.id == given.verdict_key == result.verdict_key
|
|
|
|
|
|
# --- T2: the CLI's silence is silence, not an approval --------------------------------------
|
|
|
|
|
|
def test_cli_without_decision_flags_reports_no_verdict(bundle, replies_file, capsys) -> None:
|
|
"""T2 GOAL: a flagless CLI run says a verdict was NOT given. RED before the seam: argparse
|
|
defaulted to ``approved``/``reviewed by expert`` and the line read ``verdict id=…,
|
|
decision=approved`` for a run nobody reviewed."""
|
|
rc = run.main(_argv(bundle, replies_file))
|
|
out = capsys.readouterr().out
|
|
assert rc == 0, out
|
|
assert "verdict id=" not in out, out
|
|
assert "no expert verdict" in out, out
|
|
|
|
|
|
def test_cli_with_decision_flags_reports_the_verdict(bundle, replies_file, capsys) -> None:
|
|
"""T2 CONTROL: an operator who DOES record a verdict gets the unchanged line. Proves T2's
|
|
negative is caused by the absent flags, not by the reporting having been removed."""
|
|
rc = run.main(
|
|
[*_argv(bundle, replies_file), "--decision", "approved", "--rationale", "I reviewed it"]
|
|
)
|
|
out = capsys.readouterr().out
|
|
assert rc == 0, out
|
|
assert "verdict id=" in out, out
|
|
assert "decision=approved" in out, out
|
|
|
|
|
|
def test_cli_outbox_artefact_still_carries_a_verdict_key(
|
|
bundle, replies_file, tmp_path, capsys
|
|
) -> None:
|
|
"""T2b: 'no verdict' must not cost the artefact its identity — the outcome file still carries
|
|
the key a later expert verdict on this candidate will arrive under, which is how the honest
|
|
inbox channel (Step 7) joins back to this run."""
|
|
outbox = tmp_path / "outbox"
|
|
rc = run.main([*_argv(bundle, replies_file), "--outbox-dir", str(outbox), "--run-id", "r1"])
|
|
assert rc == 0, capsys.readouterr().out
|
|
payload = json.loads((outbox / "r1-outcome.json").read_text(encoding="utf-8"))
|
|
assert payload["verdict_id"], "the outbox lost the candidate's verdict key"
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("flags", "missing"),
|
|
[
|
|
(["--decision", "approved"], "--rationale"),
|
|
(["--rationale", "I reviewed it"], "--decision"),
|
|
],
|
|
)
|
|
def test_cli_half_a_verdict_is_refused_by_name(
|
|
bundle, replies_file, capsys, flags: list[str], missing: str
|
|
) -> None:
|
|
"""T3: a verdict is a decision AND its reasoning. Half of one is refused BY NAME before any
|
|
model call — validation, never repair (the alternative is filling the other half in on the
|
|
expert's behalf, which is the very defect F2 closes). RED before the seam: the missing half
|
|
silently took its argparse default."""
|
|
rc = run.main([*_argv(bundle, replies_file), *flags])
|
|
captured = capsys.readouterr()
|
|
assert rc == 1, captured.out
|
|
assert missing in captured.err, captured.err
|
|
assert "verdict id=" not in captured.out, captured.out
|
|
|
|
|
|
# --- T4/T5: an ungiven verdict cannot PROPAGATE to the next project --------------------------
|
|
|
|
_SENTINEL = "SENTINEL-F2-3d71ac realiseringskorreksjon fra prosjekt k"
|
|
_ALIGNED_REPLY = (
|
|
'{"measure":"LED-retrofit av kontorbelysning","affected_items":'
|
|
'[{"code":"ENERGI-TOTAL-EL","quantity":180000,"unit_cost":1.0}],"claimed_saving_nok":18000}'
|
|
)
|
|
|
|
|
|
def _generation_prompts(sink: list[str]) -> list[str]:
|
|
return [p for p in sink if "SavingsProposal" in p]
|
|
|
|
|
|
def _make_docs(tmp_path: Path, name: str) -> str:
|
|
d = tmp_path / name
|
|
d.mkdir()
|
|
(d / "cost.txt").write_text(
|
|
"Asphalt Ab11 unit rate renegotiation reduced the paving cost on the school stretch.",
|
|
encoding="utf-8",
|
|
)
|
|
return str(d)
|
|
|
|
|
|
def _road_k(tmp_path: Path, *, verdict_input: dict[str, str] | None) -> Project:
|
|
return Project(
|
|
id="ROAD-K",
|
|
name="Road k",
|
|
description="road-backed project k",
|
|
currency="NOK",
|
|
cost_items=(),
|
|
docs_dir=_make_docs(tmp_path, "k-docs"),
|
|
verdict_input=verdict_input,
|
|
bundle_dir=None,
|
|
verdict_dir=None,
|
|
)
|
|
|
|
|
|
def _bundle_kplus1(tmp_path: Path) -> Project:
|
|
return Project(
|
|
id="BYGG-ENERGI-MIKRO-A",
|
|
name="Bundle k+1",
|
|
description="bundle-backed project k+1",
|
|
currency="NOK",
|
|
cost_items=(),
|
|
docs_dir=_make_docs(tmp_path, "kplus1-docs"),
|
|
verdict_input={"decision": "approved", "rationale": "k+1 reviewed (sim)"},
|
|
bundle_dir=_MINI_BUNDLE,
|
|
verdict_dir=None,
|
|
)
|
|
|
|
|
|
async def test_a_given_verdict_on_k_does_reach_kplus1(
|
|
tmp_path, monkeypatch, make_recording_client_factory
|
|
) -> None:
|
|
"""T4 CONTROL (mirrors ``test_portfolio_learning_loadbearing``): when an expert DID review
|
|
project *k*, that verdict reaches *k+1*'s hypothesis prompt. This is the event T5 denies —
|
|
proving first that it can happen is what makes T5's negative assert mean anything."""
|
|
k = _road_k(tmp_path, verdict_input={"decision": "approved", "rationale": _SENTINEL})
|
|
kplus1 = _bundle_kplus1(tmp_path)
|
|
monkeypatch.setattr("portfolio_optimiser.run.load_reference_projects", lambda: (k, kplus1))
|
|
factory, recorded = make_recording_client_factory(_ALIGNED_REPLY)
|
|
|
|
result = await run_portfolio(profile="local", client_factory=factory)
|
|
|
|
assert result.runs[0].verdict is not None
|
|
prompts = _generation_prompts(recorded)
|
|
assert any(_SENTINEL in p for p in prompts)
|
|
assert any(result.runs[0].verdict.id in p for p in prompts)
|
|
|
|
|
|
async def test_an_ungiven_verdict_on_k_never_reaches_kplus1(
|
|
tmp_path, monkeypatch, make_recording_client_factory
|
|
) -> None:
|
|
"""T5 GOAL: project *k* that nobody reviewed contributes NOTHING to *k+1*'s hypothesis
|
|
prompt — no verdict is minted, none enters the shared store, and *k*'s candidate key never
|
|
shows up as a prior judgement. RED before the seam: *k* minted an ``approved`` verdict from a
|
|
``verdict_input`` no expert supplied, and the threaded store carried it forward."""
|
|
k = _road_k(tmp_path, verdict_input=None)
|
|
kplus1 = _bundle_kplus1(tmp_path)
|
|
monkeypatch.setattr("portfolio_optimiser.run.load_reference_projects", lambda: (k, kplus1))
|
|
factory, recorded = make_recording_client_factory(_ALIGNED_REPLY)
|
|
|
|
result = await run_portfolio(profile="local", client_factory=factory)
|
|
|
|
assert result.runs[0].verdict is None, "k minted a verdict nobody gave"
|
|
k_key = result.runs[0].verdict_key
|
|
prompts = _generation_prompts(recorded)
|
|
assert prompts, "the generation calls must have happened"
|
|
assert not any(k_key in p for p in prompts), (
|
|
"k's ungiven verdict propagated into k+1's hypothesis prompt as a prior judgement"
|
|
)
|
|
assert [v.id for v in result.store.verdicts] == [result.runs[1].verdict.id], (
|
|
"the shared store holds a verdict for a project nobody reviewed"
|
|
)
|
|
|
|
|
|
# --- T6: the hosted surface no longer FORCES a caller to invent a verdict ---------------------
|
|
|
|
|
|
async def test_hosted_invocation_without_verdict_input_is_accepted(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""T6 GOAL: ``verdict_input`` is no longer a REQUIRED field, so an external caller who has no
|
|
expert verdict can run at all. A RELAXATION — a caller that still sends the field is
|
|
unaffected (the control below). RED before the seam: 400, naming ``verdict_input``."""
|
|
calls: list[dict[str, Any]] = []
|
|
|
|
async def _runner(*args: Any, **kwargs: Any):
|
|
calls.append(kwargs)
|
|
raise AssertionError("stop after the whitelist") # pragma: no cover
|
|
|
|
monkeypatch.setattr(hosting, "run_project", _runner)
|
|
payload = {"project_id": "P1", "docs_dir": "docs"}
|
|
with pytest.raises(AssertionError, match="stop after the whitelist"):
|
|
await hosting.invoke(payload)
|
|
assert calls, "the whitelist refused a payload that omitted verdict_input"
|
|
assert calls[0].get("verdict_input") is None
|
|
|
|
|
|
async def test_hosted_invocation_with_verdict_input_still_forwards_it(
|
|
monkeypatch: pytest.MonkeyPatch,
|
|
) -> None:
|
|
"""T6 CONTROL: the outward-facing contract is only WIDENED — a caller that sends the field
|
|
still has it forwarded verbatim, so no call that exists out there breaks."""
|
|
calls: list[dict[str, Any]] = []
|
|
|
|
async def _runner(*args: Any, **kwargs: Any):
|
|
calls.append(kwargs)
|
|
raise AssertionError("stop after the whitelist") # pragma: no cover
|
|
|
|
monkeypatch.setattr(hosting, "run_project", _runner)
|
|
payload = {"project_id": "P1", "docs_dir": "docs", "verdict_input": _GIVEN}
|
|
with pytest.raises(AssertionError, match="stop after the whitelist"):
|
|
await hosting.invoke(payload)
|
|
assert calls[0]["verdict_input"] == _GIVEN
|
|
|
|
|
|
# --- T7: the library refuses half a verdict too, and the partitions no longer drop a real one ---
|
|
|
|
|
|
async def test_library_half_a_verdict_is_refused_by_field_name(tmp_path) -> None:
|
|
"""T7: the CLI is not the only door. A caller that hands ``run_project`` half a verdict is
|
|
refused at step 1 by ``FeedbackContract`` — the ONE place the shape is validated — rather than
|
|
having the missing half filled in for the expert. RED if that contract is made tolerant."""
|
|
with pytest.raises(ValidationError):
|
|
await run_project(
|
|
_PID,
|
|
"local",
|
|
docs_dir=str(BUNDLE_DIR),
|
|
bundle_dir=str(BUNDLE_DIR),
|
|
verdict_input={"decision": "approved"},
|
|
client_factory=_role_factory(_VALID_PROPOSER_REPLY, _CHECKER_APPROVE),
|
|
max_rounds=2,
|
|
)
|
|
|
|
|
|
def test_portfolio_mode_refuses_a_recorded_verdict_by_name(bundle, replies_file, capsys) -> None:
|
|
"""T8a: before F2 the argparse defaults made ``--decision`` indistinguishable from its default,
|
|
so the partition could not refuse it and an operator's real expert verdict was silently
|
|
dropped in portfolio mode (a pass takes each project's verdict from its own row). It is
|
|
distinguishable now, so it is refused BY NAME — 'refused, never ignored' is the partition's own
|
|
rule. RED when the rows are dropped from ``single_only``."""
|
|
rc = run.main(["--portfolio", "--decision", "approved", "--rationale", "I reviewed it"])
|
|
captured = capsys.readouterr()
|
|
assert rc == 1, captured.out
|
|
assert "--decision" in captured.err, captured.err
|
|
assert "--rationale" in captured.err, captured.err
|
|
|
|
|
|
def test_report_mode_refuses_a_recorded_verdict(tmp_path, capsys) -> None:
|
|
"""T8b: same rule on the OTHER partition. Report mode's refusal is generic by construction (it
|
|
names the allowlist, not the offender), so the discriminator has to be the OUTCOME — and the
|
|
argv must be one that report mode would otherwise ACCEPT. Measured while mutating: with a bare
|
|
``--report`` this test was VACUOUS (green with the rows dropped), because ``--report`` without
|
|
``--ledger`` refuses with rc 1 for a completely different reason. A valid ``--ledger`` makes
|
|
rc 0 the mutant's outcome, so rc 1 here means the partition refused."""
|
|
led = SavingsLedger()
|
|
led.add_realized(
|
|
LedgerEntry(
|
|
project_id="P1",
|
|
dimension="energi",
|
|
candidate_identity="c-a",
|
|
amount_ore=100000,
|
|
verdict_id="v1",
|
|
provenance="p1",
|
|
)
|
|
)
|
|
ledger_path = tmp_path / "ledger.json"
|
|
led.save(str(ledger_path))
|
|
|
|
# CONTROL: the same argv WITHOUT the verdict flags is accepted and prints a report.
|
|
assert run.main(["--report", "--ledger", str(ledger_path)]) == 0
|
|
capsys.readouterr()
|
|
|
|
rc = run.main(
|
|
["--report", "--ledger", str(ledger_path), "--decision", "approved", "--rationale", "x"]
|
|
)
|
|
captured = capsys.readouterr()
|
|
assert rc == 1, captured.out
|
|
assert "--report" in captured.err, captured.err
|