Rename the reference-project helpers and test names (_reference_k, test_reference_path_*) and the cost-baseline helper (_kontor_it_baseline, which already returned KONTOR-IT-E1). Names only; no assertion changes. Co-Authored-By: Claude Opus 5.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(
|
|
"Licence unit price renegotiation reduced the office-suite cost for the head office.",
|
|
encoding="utf-8",
|
|
)
|
|
return str(d)
|
|
|
|
|
|
def _reference_k(tmp_path: Path, *, verdict_input: dict[str, str] | None) -> Project:
|
|
return Project(
|
|
id="REF-K",
|
|
name="Reference k",
|
|
description="reference-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 = _reference_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 = _reference_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
|