feat(major2): terminal proposal reviewer - text never repr, closed vocabulary, EOF is never a sign-off [skip-docs]

Ordre 20260904T173146Z-8102814273-from-portfolio-optimiser, steg 5 av 10.

terminal_proposal_reviewer er et SOESKEN av explore.terminal_plan_reviewer ved FORM -
kopiert, ikke delt: en felles "terminal reviewer"-abstraksjon over to doerer er den
enkeltbruks-generaliseringen repoet nekter til en tredje doer finnes.

Kandidaten rendres som TEKST fra den typede IR-en (BLOCKER-1): maal, kostlinjer, krevd
besparelse, validatorens persentiler, checkerens dom (D3) og attempts remaining. Begge
halvdeler er gatet - en POSITIV sentinel bare tekst-stien kan sende, og den NEGATIVE
formen paa selve defekten (" object at 0x"), fordi den positive alene ville vaert
tilfreds med en renderer som printer ingenting.

Stroemmene resolveres ved KALL-tid, ikke i fabrikken.

Fail-closed paa ekspertens EGEN input: skrivefeil, blank linje og bar "revise" spoerres
paa nytt; D1(a) nekter en revise som ikke kan kjoepes AT THE DOOR med et faktum, aldri
med et botemiddel CLI-en ikke kan utfoere (det finnes ingen --max-attempts, og D1 legger
ingen til) - en tredje doer-TILSTAND, ikke et tredje ord. EOF reiser
ProposalReviewInputError: aa lese stillhet som godkjenning ville latt en kjoering baere
en kandidat ingen signerte, usynlig.

RODT foer impl: fem armer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-05 06:59:54 +02:00
commit c391fb5d67
2 changed files with 193 additions and 1 deletions

View file

@ -29,9 +29,10 @@ what the validator then rules is the validator's (D6).
from __future__ import annotations
import sys
from collections.abc import Callable, Sequence
from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, Final, Literal
from typing import TYPE_CHECKING, Any, Final, Literal, TextIO
if TYPE_CHECKING: # pragma: no cover - typing only, keeps the runtime import surface minimal
from portfolio_optimiser.ir import SavingsProposal
@ -215,3 +216,90 @@ def proposal_review_notice(reviews: Sequence[ProposalReview], *, offered: bool)
f" proposal review: {answers} answer(s) across {candidates} candidate(s) — "
f"{approvals} approve, {revisions} revise{tail}"
)
def _proposal_text(request: ProposalReviewRequest) -> list[str]:
"""Render the candidate as TEXT for a human to read (BLOCKER-1: never a repr).
Every number an expert needs in order to answer is here the measure, the cost lines the
claim rests on, the claimed saving, and the validator's percentiles — and each is rendered
from the typed IR rather than from ``str()`` of an object that has no ``__str__``. BLOCKER-1
measured what the alternative looks like at a signing door: ``<...Message object at 0x>``.
"""
validated = request.proposal
proposal = validated.proposal
lines = [f"measure: {proposal.measure}", "affected cost lines:"]
lines += [
f" {item.code}: quantity {item.quantity} x unit cost {item.unit_cost} = {item.total}"
for item in proposal.affected_items
]
lines += [
f"claimed saving (NOK): {proposal.claimed_saving_nok}",
(
f"validator: p10 {validated.p10}, p50 {validated.p50}, p90 {validated.p90}, "
f"nominal feasible {validated.nominal_feasible}"
),
f"checker: {request.checker_verdict}",
f"attempts remaining: {request.attempts_remaining}",
]
return lines
def terminal_proposal_reviewer(
*, stream_in: TextIO | None = None, stream_out: TextIO | None = None
) -> ProposalReviewer:
"""A ``ProposalReviewer`` that shows the candidate at a terminal and reads the typed answer.
The operator door onto MAJOR-2: ``run.py``'s ``--proposal-review`` builds one of these and
hands it to ``run_project``. Blocking is the contract, not an oversight the attempt loop
calls the reviewer synchronously, so generation waits on the human exactly as the
``ProposalReviewer`` alias says. It is also why the hosted surface refuses this door: blocking
there would block the request AND the event loop that answers ``/readiness``.
**The streams are resolved at CALL time** (the ``shared_root()`` idiom, F4's measurement): a
closure that captured ``sys.stdin`` at construction could not be driven by a caller or a
test that replaces the stream afterwards, and the only way left to exercise the door would
be a subprocess.
**Fail-closed on the expert's own input.** ``approve`` takes the candidate as it stands;
``revise <what to change>`` buys one more attempt and sends the words into its prompt.
Anything else a blank line, a typo, a bare ``revise`` is asked AGAIN, never read as a
decision. **D1(a):** when no attempt remains, a ``revise`` is refused AT THE DOOR and re-asked
with a statement of FACT rather than a remedy this CLI cannot perform (the brief measured no
``--max-attempts`` flag, and D1 adds none) a third door STATE, not a third word. The expert
still sees the candidate, so silence is never read as consent. End of input raises
``ProposalReviewInputError``.
"""
def review(request: ProposalReviewRequest) -> ProposalReviewDecision:
source = sys.stdin if stream_in is None else stream_in
sink = sys.stdout if stream_out is None else stream_out
print(f"\nPROPOSAL REVIEW #{request.attempt + 1}{request.approach_label}", file=sink)
print("--- the candidate the deterministic validator accepted ---", file=sink)
for line in _proposal_text(request):
print(line, file=sink)
while True:
print(
f'Answer "{_APPROVE_ANSWER}" to take it as is, '
f'or "{_REVISE_ANSWER} <what to change>": ',
file=sink,
)
sink.flush()
line = source.readline()
if line == "":
raise ProposalReviewInputError(
"the proposal review reached end of input without an answer. Silence is not "
"a sign-off: --proposal-review will not take a proposal nobody answered for"
)
answer = line.strip()
if answer == _APPROVE_ANSWER:
return ProposalReviewDecision.approve()
verb, _, feedback = answer.partition(" ")
if verb == _REVISE_ANSWER and feedback.strip():
if request.attempts_remaining <= 0:
print("no attempts remain for this candidate; answer approve", file=sink)
continue
return ProposalReviewDecision.revise(feedback.strip())
print(f"Not an answer: {answer!r}.", file=sink)
return review

View file

@ -24,7 +24,9 @@ to today, golden transcript included.
from __future__ import annotations
import io
import json
import sys
from collections.abc import Sequence
from pathlib import Path
from typing import Any
@ -939,3 +941,105 @@ async def test_the_review_artefact_is_invisible_to_the_hitl_registry(tmp_path: P
with_artefact = hitl.pending(str(outbox), str(inbox))
_reviews_artefact(outbox).unlink()
assert len(hitl.pending(str(outbox), str(inbox))) == len(with_artefact) == 1
# ---------------------------------------------------------------------------------------------
# Group C (Step 5) — the terminal door, driven directly with in-memory streams.
# ---------------------------------------------------------------------------------------------
def _request(
*, attempts_remaining: int = 2, label: str = "Night setback"
) -> pr.ProposalReviewRequest:
return pr.ProposalReviewRequest(
project_id=_PID,
approach_id="a2",
approach_label=label,
attempt=0,
attempts_remaining=attempts_remaining,
proposal=_validated(p50=17_500.0),
checker_verdict="approve",
)
def _door(text: str) -> tuple[pr.ProposalReviewer, io.StringIO]:
out = io.StringIO()
return pr.terminal_proposal_reviewer(stream_in=io.StringIO(text), stream_out=out), out
def test_t14_the_candidate_is_shown_as_text_and_never_as_a_repr() -> None:
"""T14 — BLOCKER-1's lesson, applied at construction. Detach point: rendering the request or
the proposal with ``str()`` (M25).
An expert who answers ``approve`` on ``<...ValidatedProposal object at 0x10d...>`` has signed
blindly. Both halves are asserted: a POSITIVE sentinel only the text path can emit (the
measure, the claimed saving, the percentile), and the NEGATIVE shape of the defect itself
the positive alone would be satisfied by a renderer that prints nothing."""
door, out = _door("approve\n")
door(_request())
rendered = out.getvalue()
assert "LED-retrofit" in rendered # the measure, from the typed IR
assert "30000" in rendered # the claimed saving
assert "17500" in rendered # the validator's p50
assert "Night setback" in rendered # WHICH candidate is being asked about
assert "attempts remaining: 2" in rendered
assert "checker: approve" in rendered
assert "approve" in rendered and "revise" in rendered
assert _REPR_LEAK not in rendered
def test_t15_anything_outside_the_closed_vocabulary_is_asked_again() -> None:
"""T15 — fail-closed on the expert's OWN input. Detach point: treating any non-revise line as
an approval (M17).
Three things are re-asked and none of them is a decision: a typo, a blank line, and a bare
``revise`` (which says nothing to revise). Validation, never repair."""
door, out = _door("yes please\n\nrevise\nrevise F\napprove\n")
first = door(_request())
second = door(_request())
assert first == pr.ProposalReviewDecision.revise("F")
assert second == pr.ProposalReviewDecision.approve()
rendered = out.getvalue()
assert rendered.count("PROPOSAL REVIEW") == 2
assert rendered.count("Not an answer:") == 3
def test_the_door_refuses_a_revise_it_cannot_buy_and_says_why() -> None:
"""D1(a) — the THIRD door state: a re-ask, not a third word. Detach point: letting the door
return a revise with nothing left to buy.
The line states a FACT ("no attempts remain for this candidate; answer approve") rather than
offering a remedy this CLI cannot perform there is no ``--max-attempts`` flag, and D1 adds
none. The expert still SEES the candidate, so silence is never read as consent."""
door, out = _door("revise F\napprove\n")
decision = door(_request(attempts_remaining=0))
assert decision == pr.ProposalReviewDecision.approve()
assert "no attempts remain for this candidate; answer approve" in out.getvalue()
def test_t16_end_of_input_is_never_a_sign_off() -> None:
"""T16 (unit) — EOF raises. Detach point: reading end of input as an approval (M15).
Reading silence as approval would let a run carry a candidate nobody signed for, and do it
invisibly. The message names the flag and the condition so the CLI's ``run stopped:`` line can
print it verbatim."""
door, _ = _door("")
with pytest.raises(pr.ProposalReviewInputError) as excinfo:
door(_request())
assert "end of input" in str(excinfo.value)
assert "--proposal-review" in str(excinfo.value)
def test_t20_the_streams_are_resolved_at_call_time(monkeypatch: pytest.MonkeyPatch) -> None:
"""T20 — the ``shared_root()`` idiom, F4's measurement. Detach point: capturing
``sys.stdin``/``sys.stdout`` at construction (M21).
A closure that bound the streams when the factory ran could not be driven by a caller or a
test that replaces them afterwards, and the only way left to exercise the door would be a
subprocess. The reviewer is built FIRST, the streams swapped AFTER, and only then called."""
door = pr.terminal_proposal_reviewer()
out = io.StringIO()
monkeypatch.setattr(sys, "stdin", io.StringIO("revise later\n"))
monkeypatch.setattr(sys, "stdout", out)
assert door(_request()) == pr.ProposalReviewDecision.revise("later")
assert "PROPOSAL REVIEW" in out.getvalue()