portfolio-optimiser/src/portfolio_optimiser/hitl.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

310 lines
15 KiB
Python

"""S5.1 — HITL pending registry + dimension→expert routing (roadmap E, målbilde §3).
An operator inspection tool (mirrors ``preflight.py`` / ``costsim.py``) that makes the async
verdict queue visible. After every run the OUTBOX accumulates ``{run_id}-proposal.json`` /
``{run_id}-outcome.json`` (``outbox.write_outbox``); each outcome carries a ``verdict_id`` (the
``_mint_id`` content-hash). A matching expert verdict lands in the INBOX as ``{id}.json``
(``verdicts.write_verdict``). This module derives, from those two folders:
- a file-derived **pending registry** — outbox ``verdict_id`` MINUS inbox ``id`` (an id-join), i.e.
the proposals still awaiting an expert dom; and
- a declarative **``dimension → expert`` routing** — which expert should supply each pending dom,
classified by **cost-code prefix** (see routing notes in Step 3 + the plan's Risk #1).
Exposed via ``python -m portfolio_optimiser.hitl pending|route``. It is an inspection tool, NOT wired
into ``run_project`` — the system READS these folders, the expert/persona WRITES them (målbilde §3
role split).
**MAF-free source** (D7-portable): pure stdlib + pydantic. The inbox id-set predicate mirrors
``verdicts.load_verdicts_from_dir`` INLINE rather than importing it — ``verdicts.py`` imports
``agent_framework`` at module top, so importing its reader would pull MAF into this D7-portable
logic. Registered in ``tests/test_okf.py``'s ``_MAF_FREE_MODULES`` (direct AST guard) and pinned by a
transitive import-graph probe (``test_hitl_loadbearing.py``).
**Honesty limitation (CLI needs MAF at runtime):** this module's SOURCE is MAF-clean, but invoking
it via the package (``python -m portfolio_optimiser.hitl``) first runs ``__init__.py`` → ``run`` →
``agent_framework`` before hitl's body. The "no agent runtime" framing is therefore source-level
(the logic stays portable); a lazy ``__init__`` that would make the CLI itself MAF-free is out of
S5.1 scope. The static import-graph probe guards hitl's OWN logic against a MAF-bearing edge.
"""
from __future__ import annotations
import json
import sys
from dataclasses import dataclass
from pathlib import Path
from typing import Any
from pydantic import BaseModel, Field, ValidationError, model_validator
# Mirrored INLINE from verdicts.py (NOT imported — verdicts.py:29 pulls agent_framework). The inbox
# predicate must match load_verdicts_from_dir EXACTLY, else pending would count as judged a file the
# real loader drops. Kept in lockstep with verdicts._REQUIRED_VERDICT_KEYS / _INBOX_DECISION_VOCABULARY
# / the inner keys verdict_from_dict reads; pinned by the parity tests in test_hitl_loadbearing.py.
_REQUIRED_VERDICT_KEYS = {"id", "decision", "rationale", "proposal_features"}
_INBOX_DECISION_VOCABULARY = frozenset({"approved", "rejected"})
_REQUIRED_FEATURE_KEYS = {"affected_codes", "measure_type", "claimed_saving_nok"}
@dataclass(frozen=True)
class PendingProposal:
"""One outbox proposal still awaiting an expert verdict. ``codes``/``measure`` carry the routing
keys (Step 3); ``verdict_id`` is the id-join key against the inbox."""
run_id: str
verdict_id: str
outcome_type: str
measure: str
codes: frozenset[str]
def _read_outbox_proposals(outbox_dir: str) -> list[PendingProposal]:
"""Read the outbox, joining ``{run_id}-proposal.json`` and ``{run_id}-outcome.json`` on the
``run_id`` FIELD read from file content (never the filename). TOLERANT (RAW layer, contrast
``okf.load_ir_projection``'s fail-fast): a missing dir yields ``[]``; unparseable files, and
orphans (a proposal without its outcome or vice-versa), are SKIPPED, never raised — a live run
writes the pair non-atomically, so half-written state is realistic."""
directory = Path(outbox_dir)
if not directory.is_dir():
return []
proposals: dict[str, dict[str, Any]] = {}
for file in sorted(directory.glob("*-proposal.json")):
data = _load_json_dict(file)
if data is None or "run_id" not in data or not isinstance(data.get("proposal"), dict):
continue
proposals[str(data["run_id"])] = data
outcomes: dict[str, dict[str, Any]] = {}
for file in sorted(directory.glob("*-outcome.json")):
data = _load_json_dict(file)
if data is None or "run_id" not in data:
continue
outcomes[str(data["run_id"])] = data
result: list[PendingProposal] = []
for run_id in proposals.keys() & outcomes.keys(): # inner join — orphans on either side dropped
proposal = proposals[run_id]["proposal"]
outcome = outcomes[run_id]
verdict_id = outcome.get("verdict_id")
outcome_type = outcome.get("outcome_type")
if not isinstance(verdict_id, str) or not isinstance(outcome_type, str):
continue
try:
codes = frozenset(item["code"] for item in proposal.get("affected_items", []))
except (KeyError, TypeError):
continue
result.append(
PendingProposal(
run_id=run_id,
verdict_id=verdict_id,
outcome_type=outcome_type,
measure=str(proposal.get("measure", "")),
codes=codes,
)
)
return result
def _inbox_verdict_ids(verdict_dir: str) -> set[str]:
"""Collect the ``id`` of every inbox verdict file that ``load_verdicts_from_dir`` WOULD accept —
the same tolerant predicate mirrored inline: a dict carrying all ``_REQUIRED_VERDICT_KEYS``, a
``decision`` in the binary vocabulary ``{approved, rejected}``, and a ``proposal_features`` dict
carrying the inner keys ``verdict_from_dict`` reads AND an ``affected_codes`` that
``frozenset(...)`` accepts (a non-iterable — ``null``/int/float/bool — makes the real loader's
``frozenset(pf['affected_codes'])`` raise ``TypeError``, so it is skipped here too). A file that
would be skipped or raise in the real loader is skipped here too, so ``pending`` never treats it as
a delivered dom."""
directory = Path(verdict_dir)
if not directory.is_dir():
return set()
ids: set[str] = set()
for file in sorted(directory.glob("*.json")):
data = _load_json_dict(file)
if data is None or not _REQUIRED_VERDICT_KEYS <= data.keys():
continue
if data.get("decision") not in _INBOX_DECISION_VOCABULARY:
continue
features = data.get("proposal_features")
if not isinstance(features, dict) or not _REQUIRED_FEATURE_KEYS <= features.keys():
continue
# Mirror verdict_from_dict EXACTLY: frozenset(affected_codes) raises TypeError on a
# non-iterable (null/int/float/bool), which the real loader skips — so skip it here too.
try:
frozenset(features["affected_codes"])
except TypeError:
continue
ids.add(data["id"])
return ids
def pending(outbox_dir: str, verdict_dir: str) -> list[PendingProposal]:
"""The pending registry: outbox proposals whose ``verdict_id`` is NOT yet in the inbox id-set,
sorted deterministically by ``(run_id, verdict_id)``."""
judged = _inbox_verdict_ids(verdict_dir)
unjudged = [p for p in _read_outbox_proposals(outbox_dir) if p.verdict_id not in judged]
return sorted(unjudged, key=lambda p: (p.run_id, p.verdict_id))
# --- Routing config: self-contained dimension→expert table (fail-fast) ----------------------------
# A minimal MVP stand-in for the S3.5 dimension catalog (kept DISTINCT — see the plan's Non-Goals).
# Field names mirror ``dimension.Dimension`` so the two reconcile cleanly when S3.5 lands. No ``label``
# field: ``_matches`` never builds a ``Dimension``, so a label would be dead single-use surface.
class RoutingEntry(BaseModel):
"""One ``dimension → expert`` routing rule. ``allowed_measure_types`` EMPTY means "any measure"
(route by code prefix alone — see ``_matches`` + the plan's Risk #1); a non-empty set restores a
measure gate for deployments that want one."""
id: str = Field(min_length=1)
allowed_measure_types: frozenset[str] = frozenset()
allowed_code_prefixes: frozenset[str] = frozenset()
expert: str = Field(min_length=1)
class RoutingConfig(BaseModel):
"""The routing table. Entry ``id``s must be unique — a duplicate would make the sorted-first
tie-break ambiguous."""
entries: list[RoutingEntry]
@model_validator(mode="after")
def _unique_entry_ids(self) -> RoutingConfig:
ids = [e.id for e in self.entries]
if len(ids) != len(set(ids)):
raise ValueError("routing config has duplicate entry ids")
return self
def load_routing_config(path: str | Path) -> RoutingConfig:
"""Load + validate the routing config, fail-fast (mirrors ``costsim.load_pricing``): a missing
file raises ``FileNotFoundError``; malformed data raises ``pydantic.ValidationError``; a duplicate
entry id raises ``ValueError`` (via the after-validator). Top-level ``_``-prefixed keys are
ignored (doc/comment convention)."""
p = Path(path)
if not p.is_file():
raise FileNotFoundError(f"routing config not found: {str(p)!r}")
raw = json.loads(p.read_text(encoding="utf-8"))
data = {k: v for k, v in raw.items() if not k.startswith("_")}
return RoutingConfig(**data)
# --- Routing: classify each pending proposal to an expert (Step 3) --------------------------------
def _matches(entry: RoutingEntry, *, measure: str, codes: frozenset[str]) -> bool:
"""Mirror ``dimension.admits`` BUT with an OPTIONAL measure gate: an empty
``allowed_measure_types`` means "any measure" (route by code prefix alone). ``measure`` is
open-vocabulary prose (verified non-discriminating — plan Risk #1), so code prefixes are the
reliable domain key; the measure gate is a strict opt-in filter for deployments that set one."""
if entry.allowed_measure_types and measure not in entry.allowed_measure_types:
return False
if not entry.allowed_code_prefixes:
return True
return any(code.startswith(prefix) for code in codes for prefix in entry.allowed_code_prefixes)
@dataclass(frozen=True)
class RoutedProposal:
"""A pending proposal classified to an expert. ``expert``/``dimension_id`` are ``None`` when no
entry admits it (unroutable, still emitted); ``ambiguous`` flags a >1-entry match resolved by the
sorted-first ``entry.id`` tie-break."""
pending: PendingProposal
expert: str | None
dimension_id: str | None
ambiguous: bool
def route(outbox_dir: str, verdict_dir: str, config: RoutingConfig) -> list[RoutedProposal]:
"""Classify each pending proposal to the expert who owns its dimension. 0 matching entries →
unroutable (emitted with ``expert=None``); 1 → that entry; >1 → the sorted-first ``entry.id``
(deterministic) flagged ``ambiguous``. Order follows ``pending`` (sorted, deterministic)."""
routed: list[RoutedProposal] = []
for proposal in pending(outbox_dir, verdict_dir):
matches = [
entry
for entry in config.entries
if _matches(entry, measure=proposal.measure, codes=proposal.codes)
]
if not matches:
routed.append(RoutedProposal(proposal, expert=None, dimension_id=None, ambiguous=False))
continue
winner = min(matches, key=lambda entry: entry.id)
routed.append(
RoutedProposal(
proposal, expert=winner.expert, dimension_id=winner.id, ambiguous=len(matches) > 1
)
)
return routed
def _load_json_dict(file: Path) -> dict[str, Any] | None:
"""Tolerant read: parse ``file`` as JSON and return it only if it is a dict, else ``None`` (an
unreadable / non-JSON / non-object file is skipped by every reader here)."""
try:
data = json.loads(file.read_text(encoding="utf-8"))
except (OSError, UnicodeDecodeError, json.JSONDecodeError):
# UnicodeDecodeError: a *.json file hand-saved in Latin-1 (Norwegian æ/ø/å) is invalid UTF-8 —
# skip it, not raise (a ValueError subclass, so neither OSError nor JSONDecodeError catches it).
return None
return data if isinstance(data, dict) else None
# --- CLI: python -m portfolio_optimiser.hitl pending|route (Step 4) --------------------------------
def main(argv: list[str] | None = None) -> int:
"""CLI entry: ``python -m portfolio_optimiser.hitl pending|route`` — the operator inspection tool.
``pending`` prints one ``run_id verdict_id outcome_type`` line per un-judged proposal; ``route``
prints ``run_id verdict_id → <expert|UNROUTABLE> [dim:<id>][ AMBIGUOUS]``. Output is sorted /
deterministic. A config-load error → structured ``hitl: <reason>`` on stderr + rc 1 (never a
traceback). rc 0 on success."""
import argparse
parser = argparse.ArgumentParser(
prog="portfolio_optimiser.hitl",
description="HITL-inspeksjon (S5.1): vis ventende forslag (outbox uten inbox-dom) og rut dem "
"til fagekspert etter kostnadskode-prefiks. Leser mapper; ingen modellkall, ingen skriving.",
)
sub = parser.add_subparsers(dest="command", required=True)
p_pending = sub.add_parser("pending", help="list proposals still awaiting an expert verdict")
p_pending.add_argument(
"--outbox-dir", required=True, help="run outbox (proposal/outcome files)"
)
p_pending.add_argument("--verdict-dir", required=True, help="expert verdict inbox")
p_route = sub.add_parser("route", help="route pending proposals to experts by cost-code prefix")
p_route.add_argument("--outbox-dir", required=True, help="run outbox (proposal/outcome files)")
p_route.add_argument("--verdict-dir", required=True, help="expert verdict inbox")
p_route.add_argument("--routing-config", required=True, help="dimension→expert routing config")
args = parser.parse_args(argv)
if args.command == "pending":
for proposal in pending(args.outbox_dir, args.verdict_dir):
print(f"{proposal.run_id} {proposal.verdict_id} {proposal.outcome_type}")
return 0
try:
config = load_routing_config(args.routing_config)
except (FileNotFoundError, ValidationError, ValueError) as exc:
print(f"hitl: {exc}", file=sys.stderr)
return 1
for routed in route(args.outbox_dir, args.verdict_dir, config):
p = routed.pending
if routed.expert is None:
print(f"{p.run_id} {p.verdict_id} → UNROUTABLE")
else:
suffix = " AMBIGUOUS" if routed.ambiguous else ""
print(
f"{p.run_id} {p.verdict_id}{routed.expert} [dim:{routed.dimension_id}]{suffix}"
)
return 0
if __name__ == "__main__": # pragma: no cover - console entry
raise SystemExit(main())