portfolio-optimiser/tests/test_hitl_loadbearing.py
Kjell Tore Guttormsen 7e86896bc8 fix(s51): close 2 review MINORs — non-UTF-8 skip + affected_codes parity
Both live in the tolerant inbox-read path and both contradicted the module's
own "skipped, never raised" / loader-parity docstrings (S5.1 review: ALLOW,
2 MINOR, non-gating):

- UnicodeDecodeError (a *.json hand-saved in Latin-1 with Norwegian æ/ø/å is
  invalid UTF-8) now SKIPPED in hitl._load_json_dict AND
  verdicts.load_verdicts_from_dir — was: crashed pending/route with a raw
  traceback (a ValueError subclass, caught by neither OSError nor
  JSONDecodeError). Fix symmetric across both readers.
- hitl._inbox_verdict_ids now skips a non-iterable affected_codes exactly as
  load_verdicts_from_dir does (frozenset() raises TypeError) — was: marked the
  proposal judged on key-presence alone → a silent false-negative in the
  operator pending queue. hitl-only: verdicts is the correct reference.

TDD: 3 RED-then-green (both UnicodeDecodeError twins + the parity gap) + 1
ground-truth pin locking the loader side so the parity cannot rot.

Gate: pytest → 389 passed, 4 skipped (was 385); ruff check + format clean; mypy clean.
2026-07-15 20:59:01 +02:00

309 lines
13 KiB
Python

"""S5.1 HITL — load-bearing detach seams (målbilde §3 / §7). Each test goes RED when the guarded
mechanism is removed; a bare happy-path pass would not catch the regression.
Seams pinned here:
- the outbox↔inbox **id-join** (a landed verdict must clear the queue);
- **causality control** (a pre-judged proposal is never listed);
- **inbox-predicate parity** with ``load_verdicts_from_dir`` (wrong decision + malformed features
are skipped, so ``pending`` never counts as judged a file the real loader would drop);
- (Step 3) **route classification** detach;
- (Step 5) **transitive import-graph** MAF-freedom probe.
"""
from __future__ import annotations
import ast
import json
from pathlib import Path
from portfolio_optimiser.ir import AffectedItem, SavingsProposal
from portfolio_optimiser.outbox import write_outbox
from portfolio_optimiser.provenance import Citation, ProvenanceStamp
from portfolio_optimiser.retrieval import TextSpan
from portfolio_optimiser.validator import ValidatedProposal
from portfolio_optimiser.verdicts import ProposalFeatures, Verdict, write_verdict
from portfolio_optimiser import hitl
_PROVENANCE = ProvenanceStamp(
citations=[Citation(file="f.md", locator=TextSpan(start_index=0, end_index=5), snippet="hi")],
model="synthetic",
role="proposer",
validator_decision="validated",
token_usage=8,
)
def _write_proposal(
outbox: Path, run_id: str, *, verdict_id: str, codes: list[str] | None = None
) -> None:
items = [AffectedItem(code=c, quantity=1000.0, unit_cost=1.0) for c in (codes or ["05.2"])]
proposal = SavingsProposal(
project_id="P1", measure="LED-retrofit", affected_items=items, claimed_saving_nok=200.0
)
write_outbox(
str(outbox),
run_id,
outcome=ValidatedProposal(
proposal=proposal, p10=100.0, p50=150.0, p90=200.0, nominal_feasible=180.0
),
provenance=_PROVENANCE,
checker_verdict="approve",
verdict_id=verdict_id,
)
def _write_raw_verdict(inbox: Path, verdict_id: str, payload: dict) -> None:
"""Hand-write an inbox ``{id}.json`` — used to construct decisions/shapes ``write_verdict``
cannot emit (e.g. ``approved_with_adjustment``, or malformed features)."""
inbox.mkdir(parents=True, exist_ok=True)
(inbox / f"{verdict_id}.json").write_text(json.dumps(payload), encoding="utf-8")
# --- id-join (the flagship seam) ------------------------------------------------------------------
def test_verdict_landing_removes_from_pending(tmp_path: Path) -> None:
"""LOAD-BEARING: a proposal is pending until a verdict with the EXACT SAME id lands in the inbox.
The id is the shared literal passed to ``write_outbox(verdict_id=X)`` and ``Verdict(id=X)`` — not
a re-hash of features. Detach the ``verdict_id ∉ inbox id-set`` predicate (return all outbox
proposals) → the landed verdict never clears the queue → RED."""
outbox = tmp_path / "outbox"
inbox = tmp_path / "inbox"
_write_proposal(outbox, "run-1", verdict_id="SHARED-VID")
assert [p.run_id for p in hitl.pending(str(outbox), str(inbox))] == ["run-1"]
write_verdict(
str(inbox),
Verdict(
id="SHARED-VID",
proposal_features=ProposalFeatures(
affected_codes=frozenset({"05.2"}),
measure_type="scope_reduction",
claimed_saving_nok=200.0,
),
decision="approved",
rationale="expert reviewed (test)",
),
)
assert hitl.pending(str(outbox), str(inbox)) == []
def test_pending_ignores_prejudged_proposal(tmp_path: Path) -> None:
"""CAUSALITY CONTROL: a proposal whose verdict is ALREADY in the inbox at first read is never
listed — proving the removal above is caused by the id-join, not incidental."""
outbox = tmp_path / "outbox"
inbox = tmp_path / "inbox"
_write_proposal(outbox, "run-1", verdict_id="SHARED-VID")
write_verdict(
str(inbox),
Verdict(
id="SHARED-VID",
proposal_features=ProposalFeatures(
affected_codes=frozenset({"05.2"}),
measure_type="scope_reduction",
claimed_saving_nok=200.0,
),
decision="approved",
rationale="expert reviewed (test)",
),
)
assert hitl.pending(str(outbox), str(inbox)) == []
# --- inbox-predicate parity with load_verdicts_from_dir -------------------------------------------
def test_inbox_idset_skips_wrong_decision(tmp_path: Path) -> None:
"""An otherwise-fully-valid inbox verdict (all required keys, well-formed proposal_features) whose
ONLY defect is ``decision="approved_with_adjustment"`` (outside the binary run-path vocabulary)
does NOT clear pending — parity with ``load_verdicts_from_dir`` skipping it. Being otherwise
valid, dropping the decision filter is the ONLY reason it would skip → genuinely load-bearing."""
outbox = tmp_path / "outbox"
inbox = tmp_path / "inbox"
_write_proposal(outbox, "run-1", verdict_id="SHARED-VID")
_write_raw_verdict(
inbox,
"SHARED-VID",
{
"id": "SHARED-VID",
"decision": "approved_with_adjustment",
"rationale": "adjusted",
"proposal_features": {
"affected_codes": ["05.2"],
"measure_type": "scope_reduction",
"claimed_saving_nok": 200.0,
"description": "",
},
},
)
assert [p.run_id for p in hitl.pending(str(outbox), str(inbox))] == ["run-1"]
# --- route classification (Step 3) ----------------------------------------------------------------
def test_route_classification_is_load_bearing(tmp_path: Path) -> None:
"""LOAD-BEARING: ``_matches`` genuinely classifies. Detach it (route everything to ``entries[0]``)
→ both assertions flip RED: an unmatched proposal would stop being UNROUTABLE, and a two-entry
match would tie-break to ``entries[0]`` instead of the sorted-first id. ``entries[0]`` is
deliberately NOT the sorted-first id, so the tie-break assertion is sensitive to the detach."""
outbox = tmp_path / "outbox"
inbox = tmp_path / "inbox"
config = hitl.RoutingConfig(
entries=[
hitl.RoutingEntry(id="z-vei", allowed_code_prefixes=frozenset({"07"}), expert="Zeta"),
hitl.RoutingEntry(
id="a-energi", allowed_code_prefixes=frozenset({"05"}), expert="Alpha"
),
]
)
_write_proposal(outbox, "run-A", verdict_id="vA", codes=["99.9"]) # matches nothing
_write_proposal(outbox, "run-C", verdict_id="vC", codes=["05.1", "07.1"]) # matches BOTH
routed = {r.pending.run_id: r for r in hitl.route(str(outbox), str(inbox), config)}
assert routed["run-A"].expert is None # detach → routed to Zeta → RED
assert routed["run-C"].ambiguous is True
assert routed["run-C"].dimension_id == "a-energi" # sorted-first, NOT entries[0] z-vei
assert routed["run-C"].expert == "Alpha"
def test_inbox_idset_skips_malformed_features(tmp_path: Path) -> None:
"""An inbox verdict with all four top-level keys but ``proposal_features`` MISSING ``measure_type``
(which ``verdict_from_dict`` reads → would raise) is skipped, so it does NOT clear pending —
parity with the real loader dropping it."""
outbox = tmp_path / "outbox"
inbox = tmp_path / "inbox"
_write_proposal(outbox, "run-1", verdict_id="SHARED-VID")
_write_raw_verdict(
inbox,
"SHARED-VID",
{
"id": "SHARED-VID",
"decision": "approved",
"rationale": "ok",
"proposal_features": {"affected_codes": ["05.2"], "claimed_saving_nok": 200.0},
},
)
assert [p.run_id for p in hitl.pending(str(outbox), str(inbox))] == ["run-1"]
def test_inbox_idset_skips_non_iterable_affected_codes(tmp_path: Path) -> None:
"""PARITY (wrong-type): an inbox verdict with every required key present but ``affected_codes`` a
NON-ITERABLE (``null``) is skipped — the real loader's ``frozenset(pf['affected_codes'])``
(verdicts.py) raises ``TypeError`` and ``load_verdicts_from_dir`` drops it. So ``pending`` must NOT
count it as judged; otherwise ``hitl`` marks the proposal judged while the learning pipeline still
treats it pending — a silent false-negative in the operator queue, the exact failure the parity
docstring says it prevents. Detach the ``affected_codes`` iterability check (accept on key-presence
alone) → the file clears pending → RED. Ground-truth twin:
``test_verdicts.test_load_skips_non_iterable_affected_codes``."""
outbox = tmp_path / "outbox"
inbox = tmp_path / "inbox"
_write_proposal(outbox, "run-1", verdict_id="SHARED-VID")
_write_raw_verdict(
inbox,
"SHARED-VID",
{
"id": "SHARED-VID",
"decision": "approved",
"rationale": "ok",
"proposal_features": {
"affected_codes": None, # non-iterable → real loader's frozenset() raises → skips
"measure_type": "scope_reduction",
"claimed_saving_nok": 200.0,
},
},
)
assert [p.run_id for p in hitl.pending(str(outbox), str(inbox))] == ["run-1"]
# --- transitive import-graph MAF-freedom probe (Step 5) -------------------------------------------
_SRC_DIR = Path(__file__).resolve().parents[1] / "src" / "portfolio_optimiser"
_MAF_ROOTS = {"agent_framework", "mcp"}
def _first_party_submodule_edges(tree: ast.Module) -> set[str]:
"""Collect ``portfolio_optimiser.<name>`` submodule import edges from a parsed module —
``from portfolio_optimiser.<name> import ...`` and ``import portfolio_optimiser.<name>``.
DISCOUNTED by design (Assumption 3): the package top ``from portfolio_optimiser import X`` (X is
a re-exported symbol from the eager ``__init__`` ``__all__`` set, e.g. ``run_project``), and
``__init__`` itself — never traversed, else every walk would reach ``run`` → ``agent_framework``
and the probe would be unsatisfiable for a clean module. Known limitation: a hypothetical
``from portfolio_optimiser import verdicts`` (submodule via the package) is indistinguishable from
a symbol import here and would be discounted — optional future hardening; the specced detach
(``from portfolio_optimiser.verdicts import ...``) is a ``len>=2`` edge and IS caught."""
edges: set[str] = set()
for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom):
parts = (node.module or "").split(".")
if len(parts) >= 2 and parts[0] == "portfolio_optimiser" and parts[1] != "__init__":
edges.add(parts[1])
elif isinstance(node, ast.Import):
for alias in node.names:
parts = alias.name.split(".")
if len(parts) >= 2 and parts[0] == "portfolio_optimiser" and parts[1] != "__init__":
edges.add(parts[1])
return edges
def _imports_maf(tree: ast.Module) -> bool:
for node in ast.walk(tree):
if isinstance(node, ast.Import):
if any(a.name.split(".")[0] in _MAF_ROOTS for a in node.names):
return True
elif isinstance(node, ast.ImportFrom):
if (node.module or "").split(".")[0] in _MAF_ROOTS:
return True
return False
def test_hitl_transitive_import_graph_is_maf_free() -> None:
"""LOAD-BEARING: no module reachable from ``hitl.py``'s OWN first-party import edges imports
``agent_framework``/``mcp``. A static AST BFS (never traversing ``__init__.py`` — see the discount
note) — NOT a ``sys.modules`` probe, which is unsatisfiable because ``__init__.py:11`` eagerly
loads ``run`` → MAF. hitl re-implements the outbox shape + admits-logic inline, so its steady
state is an EMPTY first-party edge set. Detach point (documented): add
``from portfolio_optimiser.verdicts import load_verdicts_from_dir`` to ``hitl.py`` → the walk
reaches ``verdicts.py`` (``agent_framework`` at line 29) → RED. A clean hitl → GREEN."""
seen: set[str] = set()
queue = list(_first_party_submodule_edges(ast.parse((_SRC_DIR / "hitl.py").read_text("utf-8"))))
maf_bearing: list[str] = []
while queue:
mod = queue.pop()
if mod in seen:
continue
seen.add(mod)
mod_file = _SRC_DIR / f"{mod}.py"
if not mod_file.is_file():
continue
tree = ast.parse(mod_file.read_text("utf-8"))
if _imports_maf(tree):
maf_bearing.append(mod)
queue.extend(_first_party_submodule_edges(tree) - seen)
assert maf_bearing == [], (
f"hitl's transitive first-party import graph reaches MAF-bearing module(s): {sorted(maf_bearing)}"
)
def test_hitl_source_has_no_network_import() -> None:
"""hitl.py imports no network library (``socket``/``urllib``/``http``/``requests``/``httpx``) —
the inspection tool reads local folders only, never egresses (målbilde §1 no-silent-egress)."""
forbidden = {"socket", "urllib", "http", "requests", "httpx"}
tree = ast.parse((_SRC_DIR / "hitl.py").read_text("utf-8"))
imported: set[str] = set()
for node in ast.walk(tree):
if isinstance(node, ast.Import):
imported |= {a.name.split(".")[0] for a in node.names}
elif isinstance(node, ast.ImportFrom):
imported.add((node.module or "").split(".")[0])
assert imported & forbidden == set(), (
f"hitl must not import network libs: {imported & forbidden}"
)