feat(outbox): every evaluated approach becomes something an expert can judge
A run commissioned to evaluate three approaches wrote ONE proposal artefact, so
only the approach it selected could ever receive a verdict. The other two were
evaluated, reported in the settlement, and then taught the learning loop nothing.
The defect class is a key collapse, and it had two halves — fixing either alone
leaves it intact:
* the WRITER wrote one pair per run, so the non-selected approaches never existed
on disk;
* the READER (hitl._read_outbox_proposals) joins proposal to outcome on the
run_id FIELD read from file CONTENT, never the filename. Three files sharing
one run_id collapse onto one dict key, last write wins — so widening only the
filename would have produced three artefacts and still one pending row. This is
the S3.2 collision class: two rows under one key silently become one.
Artefacts are now keyed {run_id}-{approach_id}-*.json AND carry approach_id in the
payload; the join key is (run_id, approach_id). Two properties make them genuinely
judgeable rather than merely present:
* verdict_id is minted per approach (verdicts.verdict_key, the S3.2 content hash)
— reusing the run's single id would let one delivered verdict clear all three
from the queue;
* provenance.validator_decision follows ITS OWN approach — the run's stamp would
report a rejected candidate as validated, and nothing downstream could correct it.
verdicts.verdict_key is public so a run can stamp the key a verdict WILL arrive
under without capturing a decision nobody has made; it delegates to _mint_id
rather than restating the hash (the (p) rule: one keying rule, one copy).
The per-approach set REPLACES the run-level pair rather than joining it — the
selected approach is already among them, and writing both would count it twice in
hitl pending. The selected one carries the run's final outcome, so the outbox can
never disagree with the RunResult; the others carry the validator's verdict, the
only falsifier that ran on them.
mandate.py is deliberately untouched: hanging a ValidatedProposal off a coverage
row would drag validator — and pulp — into a module kept to pydantic+stdlib for
D7 portability, so _evaluate_mandate returns the evaluated outcomes alongside.
Ran it, not just tested it: a real CLI run wrote six artefacts and hitl pending
listed three rows. It also showed the honest edge — three approaches that produce
an identical candidate share one content-hash key, so one verdict settles all
three. That is correct (they were one candidate), and it is now documented.
Load-bearing MEASURED (tests/test_a5_per_approach_artifacts_loadbearing.py) against
the whole 750-test suite, five mutations all red: detach the per-approach writer ·
drop approach_id from the join key · reuse the run's verdict id · reuse the run's
provenance stamp · widen the filename but not the payload. Control: on a full
detach exactly the 5 new tests fail and 745 pre-existing ones stay green — the
no-mandate path is inert, and writes neither the filename segment nor the field.
Docs: bestille-en-kjoring.md (what the commissioner gets) + ekspert-svar.md (what
the expert's queue looks like, and that "rejected" is the validator's verdict on
the numbers, never a professional judgement of the idea).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VtRd8y1PDPGwkrRXFhubqr
This commit is contained in:
parent
9668e17f2f
commit
455d93d33e
7 changed files with 443 additions and 33 deletions
|
|
@ -120,6 +120,26 @@ alternativer — ikke besparelser som legges oppå hverandre. Derfor står det *
|
|||
og *hvilken kjøringen bærer videre*, aldri en sum. (Dette ble oppdaget ved å faktisk kjøre den:
|
||||
tre tilnærminger mot samme linje ga «totalt 90 000», som ingen av dem kunne innfri.)
|
||||
|
||||
## Hver tilnærming kan dømmes for seg
|
||||
|
||||
Kjører du med `--outbox-dir` (og `--run-id`), legger kjøringen igjen **én artefakt per vurdert
|
||||
tilnærming** — `run-001-led-retrofit-proposal.json`, `run-001-driftsavtale-proposal.json`, og så
|
||||
videre — ikke bare for den kjøringen bar videre.
|
||||
|
||||
Det er ikke en bokføringsdetalj. Uten det kunne bare den valgte tilnærmingen få en fagdom, og de
|
||||
andre du bestilte ville aldri nådd læringssløyfa: de ble vurdert, rapportert i oppgjøret, og deretter
|
||||
glemt. Nå står de hver for seg i køen din:
|
||||
|
||||
```
|
||||
uv run python -m portfolio_optimiser.hitl pending --outbox-dir utboks --verdict-dir innboks
|
||||
run-001 c91cb2fe1aa139a9 validated [led-retrofit]
|
||||
run-001 4f0a1d77b2e5c318 rejected [driftsavtale]
|
||||
```
|
||||
|
||||
Hvordan du svarer på dem står i [ekspert-svar.md](ekspert-svar.md). En tilnærming med status
|
||||
NOT EVALUATED har ingen artefakt — kjøringen produserte aldri et forslag for den, så det er ingenting
|
||||
å dømme.
|
||||
|
||||
## Hva bestillingen ikke gjør
|
||||
|
||||
- **Den overstyrer ikke validatoren.** Se ærlighetsmerkingen øverst.
|
||||
|
|
|
|||
|
|
@ -67,6 +67,27 @@ din, så den må gjengis **ordrett**. Resten av feltene ligger i `utboks/run-001
|
|||
`measure` → `measure_type`, hver `affected_items[].code` → `affected_codes`, og
|
||||
`claimed_saving_nok` uendret.
|
||||
|
||||
### Har du bestilt flere tilnærminger?
|
||||
|
||||
Da får **hver vurderte tilnærming sin egen artefakt** — ellers kunne du bare dømt den ene kjøringen
|
||||
valgte, og de andre du bestilte ville ikke lært systemet noe. Filene heter
|
||||
`utboks/run-001-<tilnærming>-proposal.json`, og `pending` lister dem hver for seg med
|
||||
tilnærmingen i klammer:
|
||||
|
||||
```
|
||||
run-001 c91cb2fe1aa139a9 validated [behovsstyrt-lys]
|
||||
run-001 4f0a1d77b2e5c318 rejected [aggregat-bytte]
|
||||
run-001 9b31e0c4a7d6f025 validated [own-proposal]
|
||||
```
|
||||
|
||||
`own-proposal` er systemets eget forslag, ikke et av dine. Hver linje dømmes for seg, med sin egen
|
||||
`id` — akkurat som over. To tilnærminger som endte på nøyaktig samme forslag deler `id` (den er en
|
||||
innholds-hash), og da gjør én dom opp for begge; det er ikke en feil, det er at de var samme forslag.
|
||||
|
||||
**Merk hva artefaktene *ikke* sier:** en tilnærming som står som `rejected`, er avvist av den
|
||||
deterministiske validatoren — altså på tallene. Det er ikke en fagdom over ideen. Det er nettopp
|
||||
derfor den fortsatt havner i `pending` og venter på deg.
|
||||
|
||||
**Utboks og innboks skal være to forskjellige mapper.** De har motsatt eierskap: systemet skriver
|
||||
utboksen, du skriver innboksen. (Målt: å peke dem på samme mappe ødelegger ingenting i dag —
|
||||
utboksens filer heter `run-001-*.json` og mangler dom-feltene, så innboks-lasteren hopper over dem.
|
||||
|
|
|
|||
|
|
@ -50,43 +50,59 @@ _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."""
|
||||
keys (Step 3); ``verdict_id`` is the id-join key against the inbox. ``approach_id`` names the
|
||||
commissioned approach the artefact belongs to (A5), and is ``""`` for an artefact written
|
||||
without a mandate — a run nobody commissioned has no approach to name."""
|
||||
|
||||
run_id: str
|
||||
verdict_id: str
|
||||
outcome_type: str
|
||||
measure: str
|
||||
codes: frozenset[str]
|
||||
approach_id: str = ""
|
||||
|
||||
|
||||
def _join_key(data: dict[str, Any]) -> tuple[str, str]:
|
||||
"""The key one artefact is filed under: ``(run_id, approach_id)``, read from file CONTENT.
|
||||
|
||||
``run_id`` alone was the key until A5 gave a run several judgeable approaches. Once it does, a
|
||||
``run_id``-only key collapses every approach of one run onto a single dict entry (last write
|
||||
wins) and the expert's queue silently reports one candidate where three were evaluated — the
|
||||
S3.2 key-collision class. An artefact with no ``approach_id`` keys on ``""``, which is exactly
|
||||
the pre-A5 behaviour for pre-A5 files."""
|
||||
approach_id = data.get("approach_id")
|
||||
return str(data["run_id"]), str(approach_id) if isinstance(approach_id, str) else ""
|
||||
|
||||
|
||||
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."""
|
||||
"""Read the outbox, joining ``{run_id}[-{approach_id}]-proposal.json`` and its ``-outcome.json``
|
||||
on the ``run_id``/``approach_id`` FIELDS 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]] = {}
|
||||
proposals: dict[tuple[str, 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
|
||||
proposals[_join_key(data)] = data
|
||||
|
||||
outcomes: dict[str, dict[str, Any]] = {}
|
||||
outcomes: dict[tuple[str, 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
|
||||
outcomes[_join_key(data)] = 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]
|
||||
for key in proposals.keys() & outcomes.keys(): # inner join — orphans on either side dropped
|
||||
run_id, approach_id = key
|
||||
proposal = proposals[key]["proposal"]
|
||||
outcome = outcomes[key]
|
||||
verdict_id = outcome.get("verdict_id")
|
||||
outcome_type = outcome.get("outcome_type")
|
||||
if not isinstance(verdict_id, str) or not isinstance(outcome_type, str):
|
||||
|
|
@ -102,6 +118,7 @@ def _read_outbox_proposals(outbox_dir: str) -> list[PendingProposal]:
|
|||
outcome_type=outcome_type,
|
||||
measure=str(proposal.get("measure", "")),
|
||||
codes=codes,
|
||||
approach_id=approach_id,
|
||||
)
|
||||
)
|
||||
return result
|
||||
|
|
@ -141,10 +158,11 @@ def _inbox_verdict_ids(verdict_dir: str) -> set[str]:
|
|||
|
||||
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)``."""
|
||||
sorted deterministically by ``(run_id, approach_id, verdict_id)`` — one row per evaluated
|
||||
approach, since each is judged on its own key."""
|
||||
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))
|
||||
return sorted(unjudged, key=lambda p: (p.run_id, p.approach_id, p.verdict_id))
|
||||
|
||||
|
||||
# --- Routing config: self-contained dimension→expert table (fail-fast) ----------------------------
|
||||
|
|
@ -286,7 +304,11 @@ def main(argv: list[str] | None = None) -> int:
|
|||
|
||||
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}")
|
||||
# The approach is appended only when there is one: an artefact written without a
|
||||
# mandate has no approach, and printing an empty column would suggest a missing value
|
||||
# rather than a run nobody commissioned by approach.
|
||||
approach = f" [{proposal.approach_id}]" if proposal.approach_id else ""
|
||||
print(f"{proposal.run_id} {proposal.verdict_id} {proposal.outcome_type}{approach}")
|
||||
return 0
|
||||
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -53,19 +53,34 @@ def write_outbox(
|
|||
provenance: ProvenanceStamp,
|
||||
checker_verdict: str | None,
|
||||
verdict_id: str,
|
||||
approach_id: str | None = None,
|
||||
) -> tuple[Path, Path]:
|
||||
"""Write ``{run_id}-proposal.json`` + ``{run_id}-outcome.json`` into ``outbox_dir`` (created if
|
||||
needed) and return their paths. The proposal file carries the candidate IR + provenance; the
|
||||
outcome file branches on the outcome type — a ``ValidatedProposal`` writes its percentiles, a
|
||||
``Rejection`` writes its reason (and NO percentiles, mirroring the type distinction)."""
|
||||
``Rejection`` writes its reason (and NO percentiles, mirroring the type distinction).
|
||||
|
||||
``approach_id`` (A5) names WHICH commissioned approach an artefact belongs to, so a run that
|
||||
evaluated several can have each of them judged rather than only the one it selected. It widens
|
||||
the file key to ``{run_id}-{approach_id}-*.json`` AND is written into the payload, because the
|
||||
reader (``hitl._read_outbox_proposals``) joins proposal to outcome on file CONTENT, never on the
|
||||
filename — a widened filename alone would still collapse three approaches onto one ``run_id``
|
||||
key. When it is ``None`` the key and the payload are exactly as before: the field is OMITTED
|
||||
rather than written as null, since these artefacts are byte-deterministic by contract and a run
|
||||
nobody commissioned has no approach to name."""
|
||||
directory = Path(outbox_dir)
|
||||
directory.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
proposal_path = directory / f"{run_id}-proposal.json"
|
||||
stem = run_id if approach_id is None else f"{run_id}-{approach_id}"
|
||||
keys: dict[str, Any] = {"run_id": run_id}
|
||||
if approach_id is not None:
|
||||
keys["approach_id"] = approach_id
|
||||
|
||||
proposal_path = directory / f"{stem}-proposal.json"
|
||||
proposal_path.write_text(
|
||||
_dump(
|
||||
{
|
||||
"run_id": run_id,
|
||||
**keys,
|
||||
"proposal": outcome.proposal.model_dump(),
|
||||
"provenance": provenance.model_dump(),
|
||||
}
|
||||
|
|
@ -75,7 +90,7 @@ def write_outbox(
|
|||
|
||||
if isinstance(outcome, ValidatedProposal):
|
||||
outcome_payload: dict[str, Any] = {
|
||||
"run_id": run_id,
|
||||
**keys,
|
||||
"outcome_type": "validated",
|
||||
"p10": outcome.p10,
|
||||
"p50": outcome.p50,
|
||||
|
|
@ -86,13 +101,13 @@ def write_outbox(
|
|||
}
|
||||
else:
|
||||
outcome_payload = {
|
||||
"run_id": run_id,
|
||||
**keys,
|
||||
"outcome_type": "rejected",
|
||||
"reason": outcome.reason,
|
||||
"checker_verdict": checker_verdict,
|
||||
"verdict_id": verdict_id,
|
||||
}
|
||||
outcome_path = directory / f"{run_id}-outcome.json"
|
||||
outcome_path = directory / f"{stem}-outcome.json"
|
||||
outcome_path.write_text(_dump(outcome_payload), encoding="utf-8")
|
||||
|
||||
return proposal_path, outcome_path
|
||||
|
|
|
|||
|
|
@ -92,6 +92,7 @@ from portfolio_optimiser.verdicts import (
|
|||
capture_verdict,
|
||||
load_verdicts_from_dir,
|
||||
similarity,
|
||||
verdict_key,
|
||||
)
|
||||
from portfolio_optimiser.value_report import (
|
||||
build_value_report,
|
||||
|
|
@ -263,10 +264,23 @@ def _select_outcome(
|
|||
async def _evaluate_mandate(
|
||||
mandate: Mandate,
|
||||
evaluate: Callable[[Approach | None], Awaitable[ValidatedProposal | Rejection]],
|
||||
) -> tuple[ValidatedProposal | Rejection, tuple[ApproachOutcome, ...]]:
|
||||
) -> tuple[
|
||||
ValidatedProposal | Rejection,
|
||||
tuple[ApproachOutcome, ...],
|
||||
tuple[tuple[str, ValidatedProposal | Rejection], ...],
|
||||
]:
|
||||
"""Evaluate every commissioned approach, then the run's own proposal when allowed, and report
|
||||
what became of each (Trekk A3/A4).
|
||||
|
||||
Returns the selected outcome, the coverage report, and — third — every EVALUATED approach's own
|
||||
outcome paired with its id (A5), which is what lets each of them be written as a judgeable
|
||||
outbox artefact. It is returned alongside rather than folded into ``ApproachOutcome`` on
|
||||
purpose: ``mandate.py`` imports only ``pydantic`` + stdlib to stay D7-portable (guarded by
|
||||
``test_okf_is_maf_free``), and hanging a ``ValidatedProposal`` off a coverage row would drag
|
||||
``validator`` — and with it ``pulp`` — into that deliberately thin module. Rows the run never
|
||||
reached are absent here by construction: a ``not_evaluated`` approach has no proposal, so there
|
||||
is nothing to write and nothing to judge.
|
||||
|
||||
Budget exhaustion mid-list is REPORTED, not swallowed: the approaches that were never reached
|
||||
become ``not_evaluated`` rows. But if the very first approach exhausts the budget there is
|
||||
nothing honest to return, so ``BudgetExceeded`` propagates exactly as it did before — a run
|
||||
|
|
@ -278,6 +292,7 @@ async def _evaluate_mandate(
|
|||
|
||||
rows: list[ApproachOutcome] = []
|
||||
produced: list[tuple[int, ValidatedProposal | Rejection]] = []
|
||||
evaluated: list[tuple[str, ValidatedProposal | Rejection]] = []
|
||||
for index, (row_id, label, approach) in enumerate(plan):
|
||||
try:
|
||||
outcome = await evaluate(approach)
|
||||
|
|
@ -295,9 +310,10 @@ async def _evaluate_mandate(
|
|||
)
|
||||
break
|
||||
produced.append((index, outcome))
|
||||
evaluated.append((row_id, outcome))
|
||||
rows.append(_coverage_row(row_id, label, outcome))
|
||||
|
||||
return _select_outcome(produced), tuple(rows)
|
||||
return _select_outcome(produced), tuple(rows), tuple(evaluated)
|
||||
|
||||
|
||||
def _authored_texts(result: Any, name: str) -> list[str]:
|
||||
|
|
@ -619,10 +635,11 @@ async def run_project(
|
|||
)
|
||||
|
||||
coverage: tuple[ApproachOutcome, ...] = ()
|
||||
evaluated: tuple[tuple[str, ValidatedProposal | Rejection], ...] = ()
|
||||
if mandate is None:
|
||||
validator_outcome = await _evaluate(None)
|
||||
else:
|
||||
validator_outcome, coverage = await _evaluate_mandate(mandate, _evaluate)
|
||||
validator_outcome, coverage, evaluated = await _evaluate_mandate(mandate, _evaluate)
|
||||
proposal = validator_outcome.proposal
|
||||
|
||||
# 6. First-class provenance stamp (authoritative; independent of MAF Annotation).
|
||||
|
|
@ -693,14 +710,50 @@ async def run_project(
|
|||
# is guaranteed non-None by the fail-fast guard at the top.
|
||||
if outbox_dir is not None:
|
||||
assert run_id is not None # narrowed by the step-0 guard; keeps the type checker honest
|
||||
outbox.write_outbox(
|
||||
outbox_dir,
|
||||
run_id,
|
||||
outcome=outcome,
|
||||
provenance=stamp,
|
||||
checker_verdict=checker_decision,
|
||||
verdict_id=verdict.id,
|
||||
)
|
||||
if not evaluated:
|
||||
outbox.write_outbox(
|
||||
outbox_dir,
|
||||
run_id,
|
||||
outcome=outcome,
|
||||
provenance=stamp,
|
||||
checker_verdict=checker_decision,
|
||||
verdict_id=verdict.id,
|
||||
)
|
||||
else:
|
||||
# A5: one judgeable artefact PER evaluated approach. Without this the expert can only
|
||||
# judge the approach the run happened to select, so every other approach they
|
||||
# commissioned teaches the learning loop nothing. The per-approach set REPLACES the
|
||||
# single run-level pair rather than joining it — the selected approach is already among
|
||||
# these, and writing both would make ``hitl pending`` count it twice.
|
||||
for approach_id, approach_outcome in evaluated:
|
||||
# The SELECTED approach carries the run's final outcome, so the outbox can never
|
||||
# disagree with the ``RunResult``: the checker/dimension overrides above apply to
|
||||
# that one. The others carry the validator's verdict, which is the only falsifier
|
||||
# that ran on them.
|
||||
final = outcome if approach_outcome is validator_outcome else approach_outcome
|
||||
outbox.write_outbox(
|
||||
outbox_dir,
|
||||
run_id,
|
||||
outcome=final,
|
||||
# ``validator_decision`` must follow ITS OWN approach — stamping every artefact
|
||||
# with the selected approach's decision would report a rejected candidate as
|
||||
# validated. Everything else (model, citations, token usage) is the run's.
|
||||
provenance=stamp.model_copy(
|
||||
update={
|
||||
"validator_decision": (
|
||||
"validated"
|
||||
if isinstance(approach_outcome, ValidatedProposal)
|
||||
else "rejected"
|
||||
)
|
||||
}
|
||||
),
|
||||
checker_verdict=checker_decision,
|
||||
# The key an expert verdict on THIS candidate will arrive under (S3.2 content
|
||||
# hash). Reusing the run's single verdict id would let one delivered verdict
|
||||
# clear every approach from the pending queue.
|
||||
verdict_id=verdict_key(_features_of(approach_outcome.proposal)),
|
||||
approach_id=approach_id,
|
||||
)
|
||||
|
||||
return RunResult(
|
||||
outcome=outcome,
|
||||
|
|
|
|||
|
|
@ -98,6 +98,18 @@ def _mint_id(features: ProposalFeatures) -> str:
|
|||
return hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:16]
|
||||
|
||||
|
||||
def verdict_key(features: ProposalFeatures) -> str:
|
||||
"""The id a verdict on a proposal with these features keys under — the learning-loop key.
|
||||
|
||||
Public because a run must be able to stamp an artefact with the key an expert verdict on THAT
|
||||
candidate will arrive under, WITHOUT capturing a decision nobody has made yet (A5: every
|
||||
evaluated approach gets its own judgeable artefact, but only one of them is the run's outcome).
|
||||
One key, one minting rule: this delegates to ``_mint_id`` rather than restating the hash, so a
|
||||
caller can never drift from the id ``capture_verdict`` actually assigns (the ``(p)`` precedent
|
||||
— a second private copy of a keying rule is the defect, not the convenience)."""
|
||||
return _mint_id(features)
|
||||
|
||||
|
||||
def capture_verdict(features: ProposalFeatures, decision: str, rationale: str) -> Verdict:
|
||||
"""Layer-2 out-of-band verdict constructor: mint a stable content-hash id (the
|
||||
learning-loop key) and build the ``Verdict`` to persist in the store."""
|
||||
|
|
|
|||
267
tests/test_a5_per_approach_artifacts_loadbearing.py
Normal file
267
tests/test_a5_per_approach_artifacts_loadbearing.py
Normal file
|
|
@ -0,0 +1,267 @@
|
|||
"""Load-bearing: EVERY evaluated approach gets its own judgeable artefacts (Trekk A5).
|
||||
|
||||
Trekk A3/A4 made a run *evaluate* every commissioned approach and *report* what became of each.
|
||||
That closes the reporting half of krav 1 but not the learning half: ``write_outbox`` still wrote
|
||||
ONE proposal per run, so of three evaluated approaches only the selected one could ever receive an
|
||||
expert verdict. The other two taught the system nothing — the coverage report said they happened,
|
||||
and the learning loop never saw them.
|
||||
|
||||
The defect class is a KEY COLLAPSE, and it has two independent halves, each pinned here:
|
||||
|
||||
* the WRITER — one artefact pair per run means the non-selected approaches are never written;
|
||||
* the READER — ``hitl._read_outbox_proposals`` joins proposal to outcome on the ``run_id`` FIELD
|
||||
read from file content. Per-approach files that all carry the same ``run_id`` collapse onto one
|
||||
dict key (last write wins), so writing three pairs while joining on ``run_id`` alone still yields
|
||||
one pending row. This is the S3.2 key-collision class: two rows sharing one key silently become
|
||||
one.
|
||||
|
||||
A third property is what makes the artefacts genuinely judgeable rather than merely present: each
|
||||
one must carry the verdict id THAT approach's proposal would be judged under
|
||||
(``verdicts.verdict_key``, the S3.2 content hash). Sharing one run-level verdict id would mean a
|
||||
single expert verdict marked all three approaches judged — the collapse again, one layer down.
|
||||
|
||||
Detach points, each RED on its own:
|
||||
|
||||
* write one artefact pair per run instead of one per evaluated approach;
|
||||
* key the outbox join on ``run_id`` alone -> ``hitl pending`` reports one row for three approaches;
|
||||
* stamp every per-approach artefact with the run's single verdict id -> judging one clears all.
|
||||
|
||||
The control (``test_without_a_mandate_the_outbox_is_byte_unchanged``) proves the addition is inert
|
||||
on the no-mandate path: the same two filenames as before, carrying no ``approach_id`` key at all.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
from portfolio_optimiser import hitl
|
||||
from portfolio_optimiser.mandate import OWN_PROPOSAL_ID, Approach, Mandate
|
||||
from portfolio_optimiser.run import run_project
|
||||
from portfolio_optimiser.simulation import ScriptedChatClient
|
||||
from portfolio_optimiser.verdicts import (
|
||||
ProposalFeatures,
|
||||
VerdictStore,
|
||||
capture_verdict,
|
||||
write_verdict,
|
||||
)
|
||||
|
||||
BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
|
||||
_VERDICT_INPUT = {"decision": "approved", "rationale": "expert reviewed (sim)"}
|
||||
_RUN_ID = "run-a5"
|
||||
|
||||
# BYGG-KONTOR-NORD: affected total = 300000 x 1.0 -> degenerate Monte Carlo P90 = 0.30 x 300000
|
||||
# = 90000. A claim <= 90000 validates; a claim above it is REJECTED by the deterministic validator.
|
||||
|
||||
|
||||
def _reply(measure: str, claimed: int) -> str:
|
||||
return (
|
||||
f'{{"measure":"{measure}","affected_items":'
|
||||
f'[{{"code":"ENERGI-TOTAL-EL","quantity":300000,"unit_cost":1.0}}],'
|
||||
f'"claimed_saving_nok":{claimed}}}'
|
||||
)
|
||||
|
||||
|
||||
# Labels ABSENT from the bundle's own prose (the "LED-retrofit" trap: it appears in 6 bundle files,
|
||||
# so a client keyed on it would match every prompt through the context and prove nothing).
|
||||
_LED = Approach(id="led-retrofit", label="Behovsstyrt belysning i fellesarealer")
|
||||
_HVAC = Approach(id="hvac-swap", label="Utskifting av ventilasjonsaggregat")
|
||||
|
||||
_LED_MEASURE = "Behovsstyrt belysning i fellesarealer"
|
||||
_HVAC_MEASURE = "Utskifting av ventilasjonsaggregat"
|
||||
_OWN_MEASURE = "Systemets eget forslag"
|
||||
|
||||
#: LED validates (30k <= cap); HVAC is above the cap -> the validator rejects it. The three claims
|
||||
#: are DISTINCT, so each approach mints a distinct verdict key — a test where two approaches shared
|
||||
#: a claim could not tell per-approach keying from one shared key.
|
||||
#:
|
||||
#: Written as FLOATS where a verdict key is minted from them: ``SavingsProposal.claimed_saving_nok``
|
||||
#: is typed ``float``, so pydantic coerces the JSON ``30000`` to ``30000.0`` — and ``_mint_id``
|
||||
#: hashes the raw value, where ``30000`` and ``30000.0`` are different keys (the S3.2 magnitude
|
||||
#: rule). An expert judging the artefact reads the same coerced value back out of it.
|
||||
_LED_CLAIM, _HVAC_CLAIM, _OWN_CLAIM = 30_000, 200_000, 20_000
|
||||
|
||||
_REPLY_BY_LABEL = {
|
||||
_LED.label: _reply(_LED_MEASURE, _LED_CLAIM),
|
||||
_HVAC.label: _reply(_HVAC_MEASURE, _HVAC_CLAIM),
|
||||
}
|
||||
_DEFAULT_REPLY = _reply(_OWN_MEASURE, _OWN_CLAIM)
|
||||
|
||||
|
||||
def _select_reply(blob: str, _role: str) -> str:
|
||||
"""Reply according to WHICH approach the prompt carries (canonical ``reply_selector`` seam —
|
||||
never a copied ``_inner_get_response`` body, S2.5 consolidation guard)."""
|
||||
return next((r for label, r in _REPLY_BY_LABEL.items() if label in blob), _DEFAULT_REPLY)
|
||||
|
||||
|
||||
def _factory(sink: list[str]) -> Callable[[str], ScriptedChatClient]:
|
||||
def factory(role: str) -> ScriptedChatClient:
|
||||
return ScriptedChatClient(
|
||||
sink=sink, role=role, reply_selector=_select_reply, default_reply=_DEFAULT_REPLY
|
||||
)
|
||||
|
||||
return factory
|
||||
|
||||
|
||||
_MANDATE = Mandate(
|
||||
objective="Cut energy cost without rebuilding.",
|
||||
approaches=(_LED, _HVAC),
|
||||
allow_own_proposals=True,
|
||||
)
|
||||
|
||||
|
||||
async def _run(mandate: Mandate | None, outbox_dir: Path):
|
||||
sink: list[str] = []
|
||||
return await run_project(
|
||||
"BYGG-KONTOR-NORD",
|
||||
"local",
|
||||
docs_dir=str(BUNDLE_DIR),
|
||||
bundle_dir=str(BUNDLE_DIR),
|
||||
verdict_input=_VERDICT_INPUT,
|
||||
store=VerdictStore(verdicts=[]),
|
||||
client_factory=_factory(sink),
|
||||
mandate=mandate,
|
||||
outbox_dir=str(outbox_dir),
|
||||
run_id=_RUN_ID,
|
||||
)
|
||||
|
||||
|
||||
def _payload(path: Path) -> dict:
|
||||
return json.loads(path.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def _verdict_key_for(measure: str, claimed: float) -> str:
|
||||
"""The id an expert verdict on THAT proposal would key under — minted through the SAME public
|
||||
primitive the run uses, so the test cannot pass against a private copy of the hash."""
|
||||
return capture_verdict(
|
||||
ProposalFeatures(
|
||||
affected_codes=frozenset({"ENERGI-TOTAL-EL"}),
|
||||
measure_type=measure,
|
||||
claimed_saving_nok=claimed,
|
||||
description=measure,
|
||||
),
|
||||
"approved",
|
||||
"irrelevant to the id",
|
||||
).id
|
||||
|
||||
|
||||
async def test_each_evaluated_approach_gets_its_own_artefacts(tmp_path: Path) -> None:
|
||||
"""Three evaluated approaches -> three proposal artefacts, each carrying ITS OWN candidate.
|
||||
|
||||
RED when the writer stays one-pair-per-run: only the selected approach's file exists, and the
|
||||
two the expert also commissioned cannot be judged.
|
||||
"""
|
||||
outbox = tmp_path / "outbox"
|
||||
await _run(_MANDATE, outbox)
|
||||
|
||||
proposals = sorted(p.name for p in outbox.glob("*-proposal.json"))
|
||||
assert proposals == [
|
||||
f"{_RUN_ID}-hvac-swap-proposal.json",
|
||||
f"{_RUN_ID}-led-retrofit-proposal.json",
|
||||
f"{_RUN_ID}-{OWN_PROPOSAL_ID}-proposal.json",
|
||||
]
|
||||
|
||||
by_approach = {
|
||||
_payload(p)["approach_id"]: _payload(p)["proposal"] for p in outbox.glob("*-proposal.json")
|
||||
}
|
||||
assert by_approach["led-retrofit"]["measure"] == _LED_MEASURE
|
||||
assert by_approach["hvac-swap"]["measure"] == _HVAC_MEASURE
|
||||
assert by_approach[OWN_PROPOSAL_ID]["measure"] == _OWN_MEASURE
|
||||
|
||||
|
||||
async def test_each_approach_outcome_is_written_with_its_own_status(tmp_path: Path) -> None:
|
||||
"""The outcome half must follow its own approach too: the rejected approach's artefact carries
|
||||
the rejection, not the selected approach's success."""
|
||||
outbox = tmp_path / "outbox"
|
||||
await _run(_MANDATE, outbox)
|
||||
|
||||
by_approach = {_payload(p)["approach_id"]: _payload(p) for p in outbox.glob("*-outcome.json")}
|
||||
assert by_approach["led-retrofit"]["outcome_type"] == "validated"
|
||||
assert by_approach["hvac-swap"]["outcome_type"] == "rejected"
|
||||
assert by_approach["hvac-swap"]["reason"], "a rejected approach must carry the reason"
|
||||
assert by_approach[OWN_PROPOSAL_ID]["outcome_type"] == "validated"
|
||||
|
||||
|
||||
async def test_each_artefact_stamps_its_own_validator_decision(tmp_path: Path) -> None:
|
||||
"""Provenance follows the artefact it stamps.
|
||||
|
||||
RED when every per-approach artefact reuses the run's stamp: the rejected approach's file would
|
||||
then carry ``validator_decision="validated"`` — the artefact would claim the deterministic gate
|
||||
admitted a candidate it actually refused, which is the one field in the stamp nothing else can
|
||||
correct.
|
||||
"""
|
||||
outbox = tmp_path / "outbox"
|
||||
await _run(_MANDATE, outbox)
|
||||
|
||||
decisions = {
|
||||
_payload(p)["approach_id"]: _payload(p)["provenance"]["validator_decision"]
|
||||
for p in outbox.glob("*-proposal.json")
|
||||
}
|
||||
assert decisions == {
|
||||
"led-retrofit": "validated",
|
||||
"hvac-swap": "rejected",
|
||||
OWN_PROPOSAL_ID: "validated",
|
||||
}
|
||||
|
||||
|
||||
async def test_hitl_pending_lists_each_approach_separately(tmp_path: Path) -> None:
|
||||
"""The READER half. RED while the outbox join keys on ``run_id`` alone: three files sharing one
|
||||
``run_id`` collapse to a single pending row, and two commissioned approaches disappear from the
|
||||
expert's queue even though their artefacts are on disk."""
|
||||
outbox, inbox = tmp_path / "outbox", tmp_path / "inbox"
|
||||
inbox.mkdir()
|
||||
await _run(_MANDATE, outbox)
|
||||
|
||||
rows = hitl.pending(str(outbox), str(inbox))
|
||||
assert {r.approach_id for r in rows} == {"led-retrofit", "hvac-swap", OWN_PROPOSAL_ID}
|
||||
assert len({r.verdict_id for r in rows}) == 3, "each approach must be judgeable on its own key"
|
||||
|
||||
|
||||
async def test_judging_one_approach_leaves_the_others_pending(tmp_path: Path) -> None:
|
||||
"""The keys must be the approaches' OWN verdict keys, not one run-level id.
|
||||
|
||||
RED when every per-approach artefact is stamped with the run's single verdict id: one delivered
|
||||
verdict would then clear all three from the queue, and the two unjudged approaches would be
|
||||
reported as judged.
|
||||
"""
|
||||
outbox, inbox = tmp_path / "outbox", tmp_path / "inbox"
|
||||
await _run(_MANDATE, outbox)
|
||||
|
||||
judged = _verdict_key_for(_LED_MEASURE, float(_LED_CLAIM))
|
||||
write_verdict(
|
||||
str(inbox),
|
||||
capture_verdict(
|
||||
ProposalFeatures(
|
||||
affected_codes=frozenset({"ENERGI-TOTAL-EL"}),
|
||||
measure_type=_LED_MEASURE,
|
||||
claimed_saving_nok=float(_LED_CLAIM),
|
||||
description=_LED_MEASURE,
|
||||
),
|
||||
"approved",
|
||||
"the expert judged this approach and only this one",
|
||||
),
|
||||
)
|
||||
|
||||
rows = hitl.pending(str(outbox), str(inbox))
|
||||
assert judged not in {r.verdict_id for r in rows}
|
||||
assert {r.approach_id for r in rows} == {"hvac-swap", OWN_PROPOSAL_ID}
|
||||
|
||||
|
||||
async def test_without_a_mandate_the_outbox_is_byte_unchanged(tmp_path: Path) -> None:
|
||||
"""Control: the no-mandate path keeps today's two filenames and carries NO ``approach_id`` key.
|
||||
|
||||
A run nobody commissioned has no approaches to key on, and adding a null field would change the
|
||||
bytes of every existing artefact (the writers are byte-deterministic by contract).
|
||||
"""
|
||||
outbox = tmp_path / "outbox"
|
||||
await _run(None, outbox)
|
||||
|
||||
written = sorted(p.name for p in outbox.glob("*.json"))
|
||||
assert written == [
|
||||
f"{_RUN_ID}-outcome.json",
|
||||
f"{_RUN_ID}-proposal.json",
|
||||
f"{_RUN_ID}-runconfig.json",
|
||||
]
|
||||
assert "approach_id" not in _payload(outbox / f"{_RUN_ID}-proposal.json")
|
||||
assert "approach_id" not in _payload(outbox / f"{_RUN_ID}-outcome.json")
|
||||
Loading…
Add table
Add a link
Reference in a new issue