open/portfolio-optimiser
1
0
Fork 0
Code Issues Releases 3 Activity

feat(major2): --proposal-review answers the review at the terminal; four refusals by name, EOF stops the run [skip-docs]

Browse source 
Ordre 20260904T173146Z-8102814273-from-portfolio-optimiser, steg 6 av 10.

(a) argparse --proposal-review. (b) rader i BEGGE partisjonene (report_forbidden og
single_only) - report-modus og portefoelje returnerer over dispatchen, saa en utelatelse
er et STILLE DROPP, ikke en nekt (F4-gapet). (c) TRE navngitte nekter i EN topp-nivaa-blokk
if args.proposal_review: - plasseringen er MAALT, ikke plassert paa oeyemaal: naboen
--scripted-replies/--live-dry-run er nostet under if args.scripted_replies, og
--explore/--live-dry-run under if args.explore, saa under noen av dem ville et bart
--live-dry-run --proposal-review falt rett gjennom til dry-run-dispatchen og droppet flagget.
(d) reviewer bygget paa KALLSTEDET + except ProposalReviewInputError -> "run stopped:" rc 1,
en DISTINKT kanal fra "run refused:". (e) proposal_review_notice printes fra kjoeringens EGEN
post. (f) _load_scripted_replies' aerlighetsgrense navngir review-stien.

--resume KOMPONERER (A3 verifisert av en arm, ikke utsatt): resume-blokka gir mandatet og
faller gjennom til SAMME full-run-dispatch.

RODT foer impl: 9 armer. T13 og T16 kjoerer i et BARN (P4). Nekt-armene kjoerer in-process
med _default_factory som REISER - ved exit-koden ser en nekt etter forbruket identisk ut med
en foer (oekt 57).

TO ARMER BLE FALSIFISERT AV MAALINGEN FOER de kunne gate noe:
(1) T18s rc-0-kontroll avslorte at F4-testens ledger-fixtur ({"entries": []}) faar rc 1 av
SavingsLedger.load ("must be a JSON array"), ikke av partisjonsraden - armen ville vaert
groenn mot en fjernet rad. Fixturen er naa en JSON-array, og kontrollen beviser at argv-en
ellers ville blitt AKSEPTERT.
(2) notice-null-armen ga BudgetExceeded i stedet for en avvist kjoering: et to-stegs
proposer-manus mot max_attempts=3 faller til default-svaret, som aldri parser, og rundeboka
fyrer - noeyaktig aerlighetsgrensen _load_scripted_replies uttaler, reprodusert ved uhell.
Manuset har naa like mange steg som forsoek.

Planens T19 er foldet inn i T13 og uttalt: "et bart, uskriptet flaggparse" ville kalt en
levende modell, saa argparse-vitnet er barnets egen unrecognized-arguments-assert.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-05 07:06:22 +02:00
commit fedf9897c4
2 changed files with 437 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

356
tests/test_proposal_review_loop_loadbearing.py
View file

@ -26,6 +26,7 @@ from __future__ import annotations
import io
import json
import subprocess
import sys
from collections.abc import Sequence
from pathlib import Path
@ -41,6 +42,7 @@ from portfolio_optimiser.generate import ParseFailure, _build_messages, generate
from portfolio_optimiser.ir import AffectedItem, SavingsProposal
from portfolio_optimiser.mandate import OWN_PROPOSAL_ID, Approach, Mandate
from portfolio_optimiser.reference_domain import load_reference_projects
from portfolio_optimiser import run
from portfolio_optimiser.run import RunResult, run_project
from portfolio_optimiser.simulation import ScriptedChatClient
from portfolio_optimiser.validator import Rejection, ValidatedProposal, proposal_for
@ -810,8 +812,8 @@ async def test_t5run_a_revise_on_one_approach_leaves_the_next_untouched(tmp_path
sink=control_sink,
)
assert isinstance(treated, RunResult) and isinstance(control, RunResult)
for run in (treated, control):
assert [row.status for row in run.coverage].count("not_evaluated") == 0
for finished in (treated, control):
assert [row.status for row in finished.coverage].count("not_evaluated") == 0
def _gen_prompts(sink: list[str], label: str) -> list[str]:
return [b for b in sink if _GENERATION_MARK in b and label in b]
@ -1043,3 +1045,353 @@ def test_t20_the_streams_are_resolved_at_call_time(monkeypatch: pytest.MonkeyPat
monkeypatch.setattr(sys, "stdout", out)
assert door(_request()) == pr.ProposalReviewDecision.revise("later")
assert "PROPOSAL REVIEW" in out.getvalue()
# ---------------------------------------------------------------------------------------------
# Group C (Step 6) — the CLI door: the flag, the four refusals, the ``run stopped:`` channel and
# the notice. The two door arms run in a CHILD (P4: stdin/stdout/rc are answered in a subprocess,
# never with ``capsys``); the refusal arms run in-process with the model factory RAISING, because
# at the exit code a refusal after the spend looks exactly like a refusal before it.
# ---------------------------------------------------------------------------------------------
_CLI_FEEDBACK = f"Bruk 40000, ikke 30000. {_FEEDBACK_SENTINEL}"
_CLI_MEASURE = "Behovsstyrt belysning i fellesarealer"
def _cli_replies_file(
tmp_path: Path, *, claims: tuple[int, ...] = (30_000, 40_000), name: str = "replies.json"
) -> str:
"""A TWO-ENTRY proposer step list. A single constant string would make attempt 2 byte-identical
to attempt 1 — the door would go green while proving nothing about the answer being used."""
path = tmp_path / name
path.write_text(
json.dumps(
{
"proposer": [_run_reply(_CLI_MEASURE, claim) for claim in claims],
"checker": "VERDICT: APPROVE",
}
),
encoding="utf-8",
)
return str(path)
def _cli_argv(tmp_path: Path, *, outbox: str, replies: str | None = None) -> list[str]:
return [
_RUN_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--scripted-replies",
replies or _cli_replies_file(tmp_path),
"--proposal-review",
"--outbox-dir",
str(tmp_path / outbox),
"--run-id",
_RUN_ID,
]
def _child(argv: list[str], *, stdin: str) -> subprocess.CompletedProcess[str]:
return subprocess.run(
[sys.executable, "-m", "portfolio_optimiser.run", *argv],
input=stdin,
capture_output=True,
text=True,
cwd=str(_REPO),
)
def _refuse_model(monkeypatch: pytest.MonkeyPatch) -> None:
"""The CLI's ONE injection point (``test_run_cli_loadbearing``'s seam). A refusal that fires
after the spend is indistinguishable from one that fires before it at the exit code — økt 57's
rule — so every refusal arm asserts on ZERO model clients, not only on rc 1."""
def _raise(_profile: Any) -> Any:
raise AssertionError("a model client was built on a path that must make no model calls")
monkeypatch.setattr(run, "_default_factory", _raise)
def test_t13_the_flag_answers_the_review_from_a_real_argv_and_the_answer_is_used(
tmp_path: Path,
) -> None:
"""T13 — the CLI door, end to end in a CHILD. Detach points: the argparse flag (M20), the
reviewer wiring at the dispatch, the sink, the artefact.
The child settles three things no in-process arm can: that the flag exists on the parsed
command line, that a real pipe reaches the reviewer, and that stdout renders the candidate as
TEXT. The proposer's second step carries a DIFFERENT amount, so "the answer was used" is the
outcome, not the presence of a record.
(The plan's separate T19 — "a bare, unscripted flag parse" — is folded in here: without a
script that argv would call a live model, so the argparse witness is this child's own
``unrecognized arguments`` assertion.)"""
proc = _child(_cli_argv(tmp_path, outbox="outbox"), stdin=f"revise {_CLI_FEEDBACK}\napprove\n")
assert "unrecognized arguments" not in proc.stderr, proc.stderr
assert "Traceback" not in proc.stderr, proc.stderr
assert proc.returncode == 0, proc.stderr
payload = _reviews_on_disk(tmp_path / "outbox")
assert [(r["decision"], r["honoured"]) for r in payload["reviews"]] == [
("revise", True),
("approve", True),
]
assert payload["reviews"][0]["feedback"] == _CLI_FEEDBACK # VERBATIM
assert [r["attempt"] for r in payload["reviews"]] == [0, 1] # exactly two generation attempts
outcome = json.loads(
(tmp_path / "outbox" / f"{_RUN_ID}-proposal.json").read_text(encoding="utf-8")
)
assert outcome["proposal"]["claimed_saving_nok"] == 40_000 # entry 2 — the answer was USED
assert _CLI_MEASURE in proc.stdout # the candidate was rendered as TEXT
assert _REPR_LEAK not in proc.stdout
assert "proposal review: 2 answer(s)" in proc.stdout
def test_t16_input_that_ends_without_an_answer_stops_the_run_on_its_own_channel(
tmp_path: Path,
) -> None:
"""T16 — EOF at the CLI. Detach points: reading EOF as approve (M15), not catching
``ProposalReviewInputError`` by name (M16).
``run stopped:`` is a DISTINCT channel from ``run refused:``: the argv was fine and the run
had already spent tokens, so "refused" would mislabel it — and a ``ValueError``-shaped error
would sit one frame from ``_fetch_parsed``'s catch-all and be captured as a *parse failure*
instead. Both are asserted: no traceback, and no ``{run_id}-parse-failures.json``.
T13's run (same fixture, same flags, a SEPARATE outbox) is the positive control that
``-outcome.json`` CAN appear when the door is answered."""
control = _child(
_cli_argv(tmp_path, outbox="answered"), stdin=f"revise {_CLI_FEEDBACK}\napprove\n"
)
assert control.returncode == 0, control.stderr
assert (tmp_path / "answered" / f"{_RUN_ID}-outcome.json").exists()
proc = _child(_cli_argv(tmp_path, outbox="silent"), stdin="")
assert proc.returncode == 1
assert "Traceback" not in proc.stderr, proc.stderr
assert "run refused:" not in proc.stderr
assert "run stopped:" in proc.stderr
assert "--proposal-review" in proc.stderr
assert "end of input" in proc.stderr
silent = tmp_path / "silent"
assert not (silent / f"{_RUN_ID}-outcome.json").exists()
assert not (silent / f"{_RUN_ID}-parse-failures.json").exists()
# The ``finally`` still wrote the record, and it is empty of decisions.
assert _reviews_on_disk(silent) == {"reviews": [], "run_id": _RUN_ID}
def test_t17_the_door_belongs_to_single_project_mode(
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""T17 — the documented partition. Detach point: dropping the ``single_only`` row (M18).
Concurrent portfolio waves would share ONE terminal and interleave prompts from several
projects. The assert names ``--portfolio``, never the shared ``--proposal-review``: a dropped
row falls through to a neighbouring refusal that names the flag too, and asserting on the
shared token would stay green against no partition entry at all (økt 57's own mutation)."""
_refuse_model(monkeypatch)
assert run.main(["--portfolio", "--proposal-review"]) == 1
assert "--portfolio" in capsys.readouterr().err
def test_t18_the_door_is_refused_in_report_mode(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""T18 — report mode returns ABOVE every dispatch, so a flag missing from
``report_forbidden`` is a silent DROP, not a refusal (the F4 gap). Detach point: dropping the
row (M19).
Run against an argv report mode would otherwise ACCEPT, with the rc-0 control proving it —
otherwise rc 1 could come from the missing ``--ledger`` rather than from this row."""
_refuse_model(monkeypatch)
ledger = tmp_path / "ledger.json"
# A JSON ARRAY: ``SavingsLedger.load`` refuses a dict, and an rc-1 from THAT would make the
# arm green for the wrong reason — which is precisely what the rc-0 control below catches.
ledger.write_text(json.dumps([]), encoding="utf-8")
assert run.main(["--report", "--ledger", str(ledger)]) == 0
capsys.readouterr()
assert run.main(["--report", "--ledger", str(ledger), "--proposal-review"]) == 1
assert "mode-exclusive" in capsys.readouterr().err
def test_the_door_and_a_dry_run_contradict(
monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""Detach point: dropping the ``--live-dry-run`` refusal (M30).
The dry-run cut returns ABOVE generation, so no candidate ever reaches a reviewer — the flag
would be accepted and then silently inert. Asserts on the PARTNER token."""
_refuse_model(monkeypatch)
rc = run.main(
[
_RUN_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--live-dry-run",
"--proposal-review",
]
)
assert rc == 1
assert "--live-dry-run" in capsys.readouterr().err
def test_the_door_has_nothing_to_answer_under_proposals_from_mandate(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""Detach point: dropping the ``--proposals-from-mandate`` refusal (M31).
That mode is SYNC by construction — it settles the commission against the derived baseline and
returns at the terminal dispatch, above any generation — so the flag would exit 0 having asked
nobody anything. The rc-0 control (same argv without the flag) proves the refusal is this row
and not some other precondition."""
_refuse_model(monkeypatch)
mandate_path = tmp_path / "mandate.json"
mandate_path.write_text(
Mandate(
objective="Finn kostnadsbesparelser i K2",
approaches=(
Approach(
id="a1",
label="Redusert sprengningsvolum i sone A",
affected_codes=("21.1",),
claimed_saving_nok=200_000.0,
),
),
allow_own_proposals=False,
).model_dump_json(),
encoding="utf-8",
)
priced = str(_REPO / "tests" / "fixtures" / "k2-prisskjema-SYNTETISK")
argv = [
"K2",
"--docs-dir",
priced,
"--bundle-dir",
priced,
"--derive-cost-baseline",
"--proposals-from-mandate",
"--mandate",
str(mandate_path),
]
assert run.main(argv) == 0
capsys.readouterr()
assert run.main([*argv, "--proposal-review"]) == 1
assert "--proposals-from-mandate" in capsys.readouterr().err
def test_the_door_and_a_parked_exploration_contradict(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""The FOURTH return above generation. Detach point: dropping the ``--checkpoint-dir``
refusal (M39).
A parked exploration returns on the park leg before any candidate exists, so the flag would be
accepted and never used. The refusal names ``--resume``, which is where the door DOES compose
— a refusal that only forbids leaves the operator without the door that works."""
_refuse_model(monkeypatch)
rc = run.main(
[
_RUN_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--explore",
"Find the cheapest saving.",
"--checkpoint-dir",
str(tmp_path / "checkpoints"),
"--outbox-dir",
str(tmp_path / "outbox"),
"--run-id",
_RUN_ID,
"--proposal-review",
]
)
assert rc == 1
err = capsys.readouterr().err
assert "--checkpoint-dir" in err
assert "--resume" in err
def test_the_door_composes_with_resume(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""A3, VERIFIED rather than deferred: the resume block yields the parked exploration's mandate
and falls through to the SAME full-run dispatch, so the reviewer built at the CLI is reached.
Detach point: refusing ``--resume`` together with the door, or forgetting to pass the reviewer
on that path.
``run_project`` is replaced by a recorder, so the arm measures the WIRING rather than a whole
resumed exploration."""
calls: list[dict[str, Any]] = []
async def _recorder(*args: Any, **kwargs: Any) -> Any:
calls.append(kwargs)
raise ValueError("recorded")
monkeypatch.setattr(run, "run_project", _recorder)
_refuse_model(monkeypatch)
run.main(
[
_RUN_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--scripted-replies",
_cli_replies_file(tmp_path),
"--outbox-dir",
str(tmp_path / "outbox"),
"--run-id",
_RUN_ID,
"--proposal-review",
]
)
assert len(calls) == 1
assert calls[0]["proposal_reviewer"] is not None
def test_a_reviewer_nobody_could_consult_says_so_on_stdout(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
) -> None:
"""The notice's zero case, at the CLI. Detach points: the renderer returning ``None`` on zero
with a reviewer present (M33), and the print itself (M37).
An operator who passed ``--proposal-review`` and sees nothing cannot tell "no candidate was
ever validated, so nobody was asked" from "the door hung"."""
monkeypatch.setattr(sys, "stdin", io.StringIO(""))
rc = run.main(
_cli_argv(
tmp_path,
outbox="outbox",
# BOTH attempts claim above the P90 cap (90 000), so the validator rejects each one
# and the reviewer is never reached — offered, never consulted.
# As many entries as ``max_attempts`` (3), so every attempt PARSES and is rejected
# by the deterministic gate. A shorter list would fall through to the selector's
# default reply, which never parses, and the round ledger would fire instead — the
# honesty limit ``_load_scripted_replies`` states, reproduced here by accident once.
replies=_cli_replies_file(
tmp_path, claims=(200_000, 200_000, 200_000), name="rejected.json"
),
)
)
assert rc == 0
assert "proposal review offered, never consulted" in capsys.readouterr().out
def test_a_run_with_no_reviewer_prints_no_review_line(
tmp_path: Path, capsys: pytest.CaptureFixture[str]
) -> None:
"""CONTROL for the notice (M32): the renderer must return ``None`` when no reviewer was
offered, so a run that answered nobody says nothing."""
argv = [a for a in _cli_argv(tmp_path, outbox="outbox") if a != "--proposal-review"]
assert run.main(argv) == 0
assert "proposal review" not in capsys.readouterr().out