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

fix(hitl): planen et menneske signerer er TEKST, aldri en objekt-repr (BLOCKER-1, ORDRE 20260902T151931Z)

Browse source 
Fire steder gjorde `str(review.plan)` paa en MAF `Message` som ikke har noen
`__str__` (maalt: `type(Message).__str__ is object.__str__`), saa BEGGE
HITL-doerene viste og lagret `<agent_framework._types.Message object at 0x…>`:
terminalen F4 spoer ved (`:1395`), de to opptakene i `plan_reviews` (`:1404`
approve, `:1418` revise) og den PARKERTE spoersmaalsfila U12 (`:1478`) som er
det ENESTE som krysser prosessgrensen. En ekspert som svarte `approve` signerte
blindt. Fiksen er den eksisterende `_plan_text` (`:966`) paa alle fire; ingen ny
hjelper.

Fire asserts var gronne mot defekten fordi de var TRUTHINESS eller
selv-sammenligning: `reviews[1]["plan"] != ""`, `waiting[0].plan`, og
`reviews[0]["plan"][:40] in out` — den siste sammenlignet den samme repr-en med
seg selv, saa de to flatene var enige mens begge var uleselige. Diskriminatoren
er innhold som KUN kan komme av `Message.text`: MAF komponerer plan-meldingen
rundt managerens svar ordrett (maalt), saa en sentinel i det skriptede svaret er
til stede naar teksten ble tatt og fravaerende naar repr-en ble det. Sentinelen
bor i et `reason`-felt fordi ingenting leser dem — aa nokle den til en ANSWER
ville endret kjoringen den maaler. I tillegg en NEGATIV assert (` object at 0x`
finnes ingen steder i stdout, artefaktet eller spoersmaalsfila), som fanger hele
defektklassen og ikke bare denne ene.

MAALT: fire mutasjoner, EN PER LINJE (ordrens ene samlede revert underteste —
`:1418` er revise-grenens egen kopi og hadde ellers ikke noe roedt vitne):
:1395 -> 1 roed (T2 stdout) · :1404 -> 2 roede · :1418 -> 1 roed (T1 alene) ·
:1478 -> 1 roed (async T9). Suite 2 failed (F15-diffen, KJENT) / 1078 passed /
5 skipped — uendret fra baseline. Golden `demo-transcript.stdout` BYTE-UENDRET
(ea8c534773acdbe41ae68f2c55724d69aaf8be4f).

Aerlighetsgrense, uttalt: `str(review.current_progress)` (`:1396`/`:1479`) er
IKKE roert. Den er en `MagenticProgressLedger | None`, ikke en `Message`, og
maalt renderer pydantic den lesbart — men i disse kjoringene er den `None`, saa
terminalen skriver «progress so far: None». Det er en annen defekt og en annen
ordre.

Co-Authored-By: Claude <claude-opus-5>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-02 17:38:25 +02:00
commit 4075b46a00
3 changed files with 84 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

8
src/portfolio_optimiser/explore.py
View file

@ -1392,7 +1392,7 @@ async def _drive(
decision = plan_reviewer(
PlanReviewRequest(
index=len(plan_reviews),
plan=str(review.plan),
plan=_plan_text(review.plan),
current_progress=str(review.current_progress),
is_stalled=_truthy(review.is_stalled),
)
@ -1401,7 +1401,7 @@ async def _drive(
plan_reviews.append(
PlanReview(
index=len(plan_reviews),
plan=str(review.plan),
plan=_plan_text(review.plan),
is_stalled=_truthy(review.is_stalled),
decision="approve",
)
@ -1415,7 +1415,7 @@ async def _drive(
plan_reviews.append(
PlanReview(
index=len(plan_reviews),
plan=str(review.plan),
plan=_plan_text(review.plan),
is_stalled=_truthy(review.is_stalled),
decision="revise",
feedback=decision.feedback,
@ -1475,7 +1475,7 @@ async def _park(
request_id=str(request.request_id),
checkpoint_id=str(latest.checkpoint_id),
index=len(trace.plan_reviews),
plan=str(review.plan),
plan=_plan_text(review.plan),
current_progress=str(review.current_progress),
is_stalled=_truthy(review.is_stalled),
bundle_dirs=tuple(bundle_dirs),

32
tests/test_async_plan_review_loadbearing.py
View file

@ -43,6 +43,18 @@ _BUNDLE_DIR = _REPO / "shared" / "examples" / "bygg-energi-mikro"
_PID = "BYGG-KONTOR-NORD"
_RUN_ID = "async-review"
#: See ``tests/test_plan_review_cli_door_loadbearing.py`` for the measurement behind this: MAF
#: composes the task-ledger plan ``Message`` around the manager's answer verbatim, and ``Message``
#: has no ``__str__``, so ``str(review.plan)`` renders ``<…Message object at 0x…>``. The sentinel
#: can therefore appear only where the plan's TEXT was taken. Restated here rather than imported,
#: because this file already carries its own copy of the whole scripted-reply block: the two
#: scenarios park and resume differently and are meant to be readable apart.
_PLAN_SENTINEL = "PLAN-TEXT-SENTINEL-4b1e"
#: The defect asserted NEGATIVELY — any default object repr on a surface an expert reads, not only
#: this one.
_REPR_LEAK = " object at 0x"
_PROPOSER_REPLY = json.dumps(
{
"measure": "LED-retrofit",
@ -52,7 +64,7 @@ _PROPOSER_REPLY = json.dumps(
)
_MANAGER_REPLY = json.dumps(
{
"is_request_satisfied": {"reason": "r", "answer": True},
"is_request_satisfied": {"reason": _PLAN_SENTINEL, "answer": True},
"is_in_loop": {"reason": "r", "answer": False},
"is_progress_being_made": {"reason": "r", "answer": True},
"next_speaker": {"reason": "r", "answer": "hypothesiser"},
@ -405,7 +417,23 @@ def test_a_parked_review_is_pending_until_its_own_answer_lands(tmp_path, capsys)
waiting = hitl.pending_plan_reviews(outbox, inbox)
assert [p.run_id for p in waiting] == [_RUN_ID]
assert waiting[0].plan, "an expert cannot answer a review that does not show them the plan"
# BLOCKER-1, on the ASYNCHRONOUS door. ``assert waiting[0].plan`` was truthiness, and a repr is
# truthy: the register said an expert could answer while what it showed them was
# ``<…Message object at 0x…>``. Both surfaces the expert actually reads are asserted — the
# question file ``explore.py:1478`` writes, and the register that joins it to the inbox.
question = _question(tmp_path)
assert _PLAN_SENTINEL in question["plan"], (
"the parked question is the ONLY thing crossing the process boundary; an expert answering "
"days later can read nothing else, so it must carry the plan as text"
)
assert _REPR_LEAK not in question["plan"], (
f"no default object repr may stand in for the plan: {question['plan'][:80]!r}"
)
assert _PLAN_SENTINEL in waiting[0].plan, (
"an expert cannot answer a review that does not show them the plan — and a repr shows "
"them nothing they can judge"
)
_answer(tmp_path, decision="approve")
inbox_file = tmp_path / "review-inbox" / f"{_RUN_ID}-plan-review-answer.json"

55
tests/test_plan_review_cli_door_loadbearing.py
View file

@ -46,9 +46,22 @@ _PROPOSER_REPLY = json.dumps(
"claimed_saving_nok": 30000,
}
)
#: A string that exists ONLY inside the manager's scripted reply, so it reaches a surface exactly
#: when that surface rendered the plan's TEXT. MAF composes the task-ledger plan ``Message`` around
#: the manager's answer verbatim (measured), and ``Message`` has no ``__str__``
#: (``type(Message).__str__ is object.__str__``, measured) — so ``str(review.plan)`` yields
#: ``<agent_framework._types.Message object at 0x…>``, in which the sentinel cannot appear. It sits
#: in a ``reason`` field because nothing reads those: the ledger's ANSWERS drive the loop and are
#: asserted elsewhere, so keying the sentinel to them would change the run it is measuring.
_PLAN_SENTINEL = "PLAN-TEXT-SENTINEL-4b1e"
#: The shape of the defect itself, asserted NEGATIVELY. Stronger than the sentinel's absence: it
#: catches ANY object rendered by its default repr onto a surface a human reads, not only this one.
_REPR_LEAK = " object at 0x"
_MANAGER_REPLY = json.dumps(
{
"is_request_satisfied": {"reason": "r", "answer": True},
"is_request_satisfied": {"reason": _PLAN_SENTINEL, "answer": True},
"is_in_loop": {"reason": "r", "answer": False},
"is_progress_being_made": {"reason": "r", "answer": True},
"next_speaker": {"reason": "r", "answer": "hypothesiser"},
@ -158,28 +171,60 @@ def test_an_operator_answer_typed_at_the_cli_reaches_the_manager_and_is_asked_ag
assert reviews[0]["feedback"] == _FEEDBACK, (
"what a human told the loop is worth nothing paraphrased"
)
assert reviews[1]["plan"] != "", "the second review must show the replanned plan"
# BLOCKER-1: the two recorded plans come from two DIFFERENT lines — the revise branch writes
# its own copy and the approve branch writes another. ``!= ""`` was green against a repr, so
# reverting either line alone stayed green; the sentinel is what separates them.
assert _PLAN_SENTINEL in reviews[0]["plan"], (
"the REVISE record must carry the plan as text — an expert reading the artefact to see "
"what they revised cannot use an object repr"
)
assert _PLAN_SENTINEL in reviews[1]["plan"], (
"the second review must show the replanned plan as TEXT, not as a repr of the message "
"that carried it"
)
def test_the_operator_is_shown_the_plan_and_the_answer_vocabulary(
tmp_path, capsys, monkeypatch
) -> None:
"""T2: a review nobody can read is a review nobody can answer. The prompt carries the plan,
the progress and the two words that answer it.
"""T2: a review nobody can read is a review nobody can answer. The prompt carries the plan
AS TEXT, the progress and the two words that answer it.
Detach point: print only "plan review?" → RED. The control for T1: T1 proves the answer is
used, this proves the question was askable.
**Sharpened for BLOCKER-1.** The old form asserted ``reviews[0]["plan"][:40] in out``, which
an object repr satisfies just as well as the plan does — the same string was shown and stored,
so the two agreed while both were unreadable. What discriminates is content that can only come
from ``Message.text``: the sentinel, plus the negative that no default repr reached either
surface. Mutation: revert ``explore.py:1395`` → the operator is shown a repr (stdout RED);
revert ``:1404`` → the artefact stores one (artefact RED).
"""
_stdin(monkeypatch, "approve\napprove\napprove\n")
run.main(_argv(tmp_path))
out = capsys.readouterr().out
reviews = _artefact(tmp_path)["plan_reviews"]
artefact = _artefact(tmp_path)
reviews = artefact["plan_reviews"]
assert reviews, "the run must actually have reached a plan review"
assert reviews[0]["plan"][:40] in out, (
"the operator must be shown the plan they are signing off"
)
assert _PLAN_SENTINEL in out, (
"an operator asked to sign a plan must be shown the PLAN; a repr of the message that "
"carried it is a signature given blind"
)
assert _PLAN_SENTINEL in reviews[0]["plan"], (
"the artefact is the record of what was approved, so it must hold the plan as text too"
)
assert _REPR_LEAK not in out, (
f"no default object repr may reach a surface a human reads; found in stdout: "
f"{out[max(0, out.find(_REPR_LEAK) - 60) : out.find(_REPR_LEAK) + 20]!r}"
)
assert _REPR_LEAK not in json.dumps(artefact), (
"nor may one be written into the artefact that outlives the run"
)
assert "approve" in out and "revise" in out, "the prompt must name the vocabulary it accepts"