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

fix(s7b): BudgetExceeded refuses instead of tracebacking, approaches land in exploration.json

Browse source 
Fiks-ordre 20260904T070930Z-8335015575-from-.claude. Two scoped defects the
S7b syretest observed and left unfixed (docs/2026-09-04-syretest-s7b-k2.md
§ 3.6):

1. `BudgetExceeded` (a RuntimeError, econ 56) could leave `--explore` as a
   raw Python traceback from TWO raise sites: the exploration loop's own
   round/token cap (`explore()`/`resume_exploration()`, uncaught in
   `main()`'s exploration `try`), and `generate_via_llm`'s retry loop when a
   mandate's own-proposal evaluation hits an unparseable reply (the full-run
   dispatch's `except` tuple only knew `ValueError`/`FileNotFoundError`/
   `ValidationError`). Both now end as `run refused: {exc}` on stderr, rc 1,
   same shape as every other loader refusal in run.py.

2. `{run_id}-exploration.json` carried rounds/tool_calls/plan_reviews/
   quick_validations but not the approaches the loop actually shaped — those
   stood only in the stdout mandate announcement. `explore.trace_payload`
   now takes a required `mandate` keyword and renders `mandate.approaches`
   under an "approaches" key, so an operator reading the artefact days later
   (the whole point of the async U12 door) can recover what the run decided
   to evaluate without the terminal.

Fifteen mutations across three detach points, all red against the full
suite: the exploration-loop except clause (2 red), the full-run except
tuple (1 red), and the approaches rendering (1 red) — plus a control per
fix proving the happy path is unaffected. Green control 1295 passed / 5
skipped (supersett of S7b's 1290/5, 0 removed), golden
demo-transcript.stdout byte-unchanged (shasum -a 1 = ea8c534…), ruff +
mypy clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-04 16:33:35 +02:00
commit 5d8844fef5
7 changed files with 479 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

172
tests/test_exploration_artefact_approaches_loadbearing.py Normal file
View file

@ -0,0 +1,172 @@
"""Fiks-ordre 20260904T070930Z, punkt 2 — ``{run_id}-exploration.json`` must carry the approaches
the loop actually SHAPED, not just the mandate announcement on stdout.
**Measured** on K2's syretest (``docs/2026-09-04-syretest-s7b-k2.md § 3.6``): the artefact had
``rounds``/``tool_calls``/``plan_reviews``/``quick_validations``, while which approaches the loop
minted stood only in ``mandate.announce``'s stdout block — printed by ``run.py``, never written.
An operator reading the artefact days later (the whole point of the async U12 door) could not
recover what the run had decided to evaluate without also having kept the terminal's stdout.
The fix widens ``explore.trace_payload`` with a required ``mandate`` keyword: it renders
``mandate.approaches`` under an ``"approaches"`` key, using the SAME fields ``mandate.announce``
reads off ``Approach`` (id, label). This file proves the artefact and the stdout announcement
agree — not just that a key exists.
"""
from __future__ import annotations
import json
from collections.abc import Callable
from pathlib import Path
from typing import Any
from agent_framework import BaseChatClient
from portfolio_optimiser import explore, run
from portfolio_optimiser.explore import ExplorationTrace
from portfolio_optimiser.simulation import ScriptedChatClient
_BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
_PID = "BYGG-KONTOR-NORD"
_LABEL = "SENTINEL-APPROACHES-9f2a"
_RATIONALE = "because the fittings are old (sentinel rationale)"
_ENERGY_REPLY = (
'{"measure":"LED-retrofit av kontorbelysning","affected_items":'
'[{"code":"ENERGI-TOTAL-EL","quantity":300000,"unit_cost":1.0}],"claimed_saving_nok":30000}'
)
_CONTRACT_JSON: dict[str, Any] = {
"max_rounds": 4,
"max_tokens": 100_000,
"max_stall_count": 2,
"max_reset_count": 1,
"max_plan_revisions": 0,
"enable_plan_review": False,
}
def _ledger_json(*, satisfied: bool, speaker: str = "hypothesiser") -> str:
return json.dumps(
{
"is_request_satisfied": {"reason": "r", "answer": satisfied},
"is_in_loop": {"reason": "r", "answer": False},
"is_progress_being_made": {"reason": "r", "answer": True},
"next_speaker": {"reason": "r", "answer": speaker},
"instruction_or_question": {"reason": "r", "answer": "Shape one hypothesis."},
}
)
def _manager_script(ledgers: list[str]) -> Callable[[str, str], str]:
def _select(blob: str, _role: str) -> str:
if "provide the final answer" in blob:
return "FINAL: exploration done."
if "pure JSON format" in blob:
return ledgers.pop(0) if ledgers else _ledger_json(satisfied=True)
if "went wrong on this last run" in blob:
return "PLAN-UPDATE: revised plan."
if "rewrite the following fact sheet" in blob:
return "FACTS-UPDATE: revised facts."
if "bullet-point plan" in blob:
return "PLAN: - ask the hypothesiser"
if "pre-survey" in blob:
return "FACTS: the bundle is anchored."
return "{}"
return _select
def _hypothesis_line(label: str, rationale: str) -> str:
return f"{explore.HYPOTHESIS_MARKER} " + json.dumps({"label": label, "rationale": rationale})
def _factory(*, ledgers: list[str], hypothesiser: list[str]) -> Callable[[str], BaseChatClient]:
def factory(role: str) -> BaseChatClient:
if role == explore.MANAGER_ROLE:
return ScriptedChatClient(reply_selector=_manager_script(ledgers), role=role)
if role == explore.HYPOTHESISER_ROLE:
replies = list(hypothesiser)
def _hyp(_blob: str, _role: str) -> str:
return replies.pop(0) if replies else "nothing further."
return ScriptedChatClient(reply_selector=_hyp, role=role)
if role == explore.NAVIGATOR_ROLE:
return ScriptedChatClient("NAVIGATOR: index read.", role=role)
return ScriptedChatClient(_ENERGY_REPLY, role=role)
return factory
def _config_file(tmp_path: Path) -> str:
path = tmp_path / "exploration.json"
path.write_text(json.dumps(_CONTRACT_JSON), encoding="utf-8")
return str(path)
def _base_argv(tmp_path: Path) -> list[str]:
return [
_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--explore",
"Find the cheapest saving.",
"--explore-config",
_config_file(tmp_path),
]
def test_the_artefact_names_the_same_approach_the_announcement_did(
tmp_path, capsys, monkeypatch
) -> None:
"""Goal test: the label ``mandate.announce`` printed on stdout is the SAME label the artefact
carries, under the SAME minted id — read back from disk, not scraped off stdout.
Detach point: drop the ``"approaches"`` key from ``trace_payload`` → ``KeyError`` here.
Detach point: render an empty list regardless of ``mandate`` → the id/label assertions go RED.
"""
factory = _factory(
ledgers=[_ledger_json(satisfied=False), _ledger_json(satisfied=True)],
hypothesiser=[_hypothesis_line(_LABEL, _RATIONALE)],
)
monkeypatch.setattr("portfolio_optimiser.run._default_factory", lambda profile: factory)
outbox = tmp_path / "outbox"
rc = run.main(_base_argv(tmp_path) + ["--outbox-dir", str(outbox), "--run-id", "r3"])
assert rc == 0, capsys.readouterr().err
out = capsys.readouterr().out
assert f"1. hypothesis-1 — {_LABEL}" in out, "the stdout announcement is the fixture's oracle"
payload = json.loads((outbox / "r3-exploration.json").read_text(encoding="utf-8"))
assert payload["approaches"] == [
{
"id": "hypothesis-1",
"label": _LABEL,
"description": _RATIONALE,
"affected_codes": [],
"claimed_saving_nok": None,
"bundle_id": "bygg-energi-mikro",
}
], (
"the artefact must name the same approach the terminal announced, or a reader without the terminal learns nothing"
)
def test_a_mandate_with_no_commissioned_approaches_renders_an_empty_but_present_list() -> None:
"""Control: ``mandate.approaches == ()`` (pure own-proposals) is a legitimate state and must
render as ``[]``, not be confused with "no mandate was ever formed" — that absence is what
``completed=False`` already says (``trace_payload``'s own docstring)."""
trace = ExplorationTrace()
from portfolio_optimiser.mandate import Mandate
empty_mandate = Mandate(objective="find something", approaches=())
payload = explore.trace_payload(trace, stop=None, completed=True, mandate=empty_mandate)
assert payload["approaches"] == []
payload_no_mandate = explore.trace_payload(trace, stop=None, completed=False, mandate=None)
assert payload_no_mandate["approaches"] == []

218
tests/test_explore_budget_refusal_loadbearing.py Normal file
View file

@ -0,0 +1,218 @@
"""Fiks-ordre 20260904T070930Z, punkt 1 — ``BudgetExceeded`` out of ``--explore`` must never
traceback.
**Measured by accident** on K2's syretest (``docs/2026-09-04-syretest-s7b-k2.md § 3.6``): "an
unparseable proposer response burned the round cap" — ``generate_via_llm``'s own retry loop
(``generate.py:_fetch_parsed``) raises ``BudgetExceeded`` from ``meter.tick_round()`` when a
proposer's reply never parses, and ``_evaluate_mandate`` (``run.py``) only swallows that mid-list:
the FIRST approach the pipeline evaluates hitting the cap re-raises by design ("nothing honest to
return"). That is ``main()``'s full single-project run dispatch (``run_project(...)``), whose
``except`` clause caught ``ValueError``/``FileNotFoundError``/``ValidationError`` but not
``BudgetExceeded`` — a ``RuntimeError`` (econ 56) — so the exception reached the interpreter as a
raw traceback.
**A second, distinct raise site reaches the same defect the same way**: the exploration loop
itself (``explore()``/``resume_exploration()``) raises the identical exception type on its OWN
round/token cap, from the ``try`` around the exploration call one block above the full-run
dispatch. Both are fixed, at their own raise sites, because a handler at only one would leave the
other tracebacking on a differently-shaped scripted run.
**Why a subprocess, not just the in-process arms this order also fixes
(``test_explore_callsites_loadbearing.py::test_the_artefact_is_written_even_when_the_exploration_was_cut_short``).**
An in-process call can only assert that ``main()`` RETURNS 1 instead of raising — it cannot see
whether a Python traceback was ever printed, because an exception that unwinds past ``capsys`` was
never captured as text in the first place (the P4 precedent: stdout/stderr is answered in a
subprocess, never in-process — ``CLAUDE.md``'s ``capsys`` note). This file is the one witness that
can tell "rc 1, one refusal line" apart from "the process crashed with an exit code that happens to
be 1 and a traceback nobody looked for".
"""
from __future__ import annotations
import json
import subprocess
import sys
from pathlib import Path
from typing import Any
_REPO = Path(__file__).resolve().parents[1]
_BUNDLE_DIR = _REPO / "shared" / "examples" / "bygg-energi-mikro"
_PID = "BYGG-KONTOR-NORD"
_VALID_PROPOSER_REPLY = json.dumps(
{
"measure": "LED-retrofit av kontorbelysning",
"affected_items": [{"code": "ENERGI-TOTAL-EL", "quantity": 300000, "unit_cost": 1.0}],
"claimed_saving_nok": 30000,
}
)
_MANAGER_SATISFIED = json.dumps(
{
"is_request_satisfied": {"reason": "r", "answer": True},
"is_in_loop": {"reason": "r", "answer": False},
"is_progress_being_made": {"reason": "r", "answer": True},
"next_speaker": {"reason": "r", "answer": "hypothesiser"},
"instruction_or_question": {"reason": "r", "answer": "go"},
}
)
_MANAGER_UNSATISFIED = json.dumps(
{
"is_request_satisfied": {"reason": "r", "answer": False},
"is_in_loop": {"reason": "r", "answer": False},
"is_progress_being_made": {"reason": "r", "answer": True},
"next_speaker": {"reason": "r", "answer": "hypothesiser"},
"instruction_or_question": {"reason": "r", "answer": "keep looking"},
}
)
#: The happy-path reply set — a run against this must reach rc 0 with empty stderr. Every other
#: reply set in this file is a copy of this one with exactly ONE role swapped for an unparseable
#: or never-satisfied reply, so each test isolates the ONE cap it means to trip.
_REPLIES: dict[str, str] = {
"proposer": _VALID_PROPOSER_REPLY,
"checker": "VERDICT: APPROVE",
"manager": _MANAGER_SATISFIED,
"navigator": "NAVIGATOR: read the index.",
"hypothesiser": "still thinking.",
}
def _config_file(tmp_path: Path, **overrides: Any) -> str:
path = tmp_path / "exploration.json"
path.write_text(
json.dumps(
{
"max_rounds": 2,
"max_tokens": 200_000,
"max_stall_count": 2,
"max_reset_count": 1,
"max_plan_revisions": 0,
"enable_plan_review": False,
**overrides,
}
),
encoding="utf-8",
)
return str(path)
def _replies_file(tmp_path: Path, replies: dict[str, str] | None = None) -> str:
path = tmp_path / "replies.json"
path.write_text(json.dumps(replies if replies is not None else _REPLIES), encoding="utf-8")
return str(path)
def _argv(tmp_path: Path, *, replies: dict[str, str] | None = None) -> list[str]:
return [
_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--explore",
"Find the cheapest saving.",
"--explore-config",
_config_file(tmp_path),
"--scripted-replies",
_replies_file(tmp_path, replies),
]
def _run_in_a_fresh_process(argv: list[str]) -> subprocess.CompletedProcess[str]:
return subprocess.run(
[sys.executable, "-m", "portfolio_optimiser.run", *argv],
capture_output=True,
text=True,
cwd=str(_REPO),
)
def _last_line(stderr: str) -> str:
lines = [line for line in stderr.splitlines() if line.strip()]
assert lines, "the refusal must say something"
return lines[-1]
# ---------------------------------------------------------------------------------------------
# 1. The exploration loop's OWN round cap (explore()/resume_exploration())
# ---------------------------------------------------------------------------------------------
def test_the_exploration_round_cap_refuses_instead_of_tracebacking(tmp_path: Path) -> None:
"""Never-satisfied manager replies burn the exploration's ``max_rounds=2``.
Stderr also carries MAF's own ``"Magentic Orchestrator: Max round count reached"`` log line —
unrelated noise ``run.main()`` never installs a filter for (that filter is
``simulation.main()``'s own — CLAUDE.md's "installeres i main(), ALDRI ved import" invariant),
so the assertion reads the LAST line rather than the whole stream.
Detach point: remove ``except BudgetExceeded`` around the ``explore()``/``resume_exploration()``
call in ``run.py`` → the process exits nonzero with a traceback, and the ``"Traceback"``
assertion goes RED (rc stays 1 either way — Python's own unhandled-exception exit code — so rc
alone cannot gate this; the traceback text is the discriminator).
"""
replies = dict(_REPLIES)
replies["manager"] = _MANAGER_UNSATISFIED
replies["hypothesiser"] = "still thinking."
completed = _run_in_a_fresh_process(_argv(tmp_path, replies=replies))
assert completed.returncode == 1, completed.stderr
assert "Traceback (most recent call last)" not in completed.stderr, completed.stderr
assert _last_line(completed.stderr).startswith(
"run refused: budget exceeded: exploration_rounds"
), completed.stderr
# ---------------------------------------------------------------------------------------------
# 2. The full-run mandate evaluation's round cap (generate_via_llm's retry loop) — the exact
# scenario measured on K2: an unparseable proposer reply burns run_project's OWN cap.
# ---------------------------------------------------------------------------------------------
def test_an_unparseable_proposer_reply_refuses_instead_of_tracebacking(tmp_path: Path) -> None:
"""The exploration concludes cleanly (manager satisfied on round 1, no hypothesis marked, so
the mandate carries zero commissioned approaches); the pipeline then evaluates its OWN
proposal — the only row in the plan — against an unparseable proposer reply, which burns
``run_project``'s default ``max_rounds=3`` inside ``generate_via_llm``. This is the shape
``docs/2026-09-04-syretest-s7b-k2.md`` describes verbatim.
Detach point: revert ``BudgetExceeded`` out of the full-run dispatch's ``except`` tuple in
``run.py`` → traceback, RED here.
"""
replies = dict(_REPLIES)
replies["proposer"] = "{}"
completed = _run_in_a_fresh_process(_argv(tmp_path, replies=replies))
assert completed.returncode == 1, completed.stderr
assert "Traceback (most recent call last)" not in completed.stderr, completed.stderr
assert _last_line(completed.stderr).startswith("run refused: budget exceeded: rounds"), (
completed.stderr
)
# ---------------------------------------------------------------------------------------------
# 3. Control — the happy path must still reach rc 0 with nothing on stderr
# ---------------------------------------------------------------------------------------------
def test_a_run_that_never_capped_is_unaffected(tmp_path: Path) -> None:
"""Neither ``except`` clause may swallow a run that actually finishes.
Detach point: an over-widened handler (e.g. ``except Exception`` in either spot) would also
make this arm pass, which is why the two goal tests' traceback assertions carry the real
weight — this control only proves the door still opens for a happy path.
Stderr is not asserted empty: the debate's own ``GroupChatOrchestrator`` logs
``"reached max_rounds=…; forcing completion"`` on its OWN cap (unrelated MAF noise
``simulation.main()``'s filter would quiet, but ``run.main()`` never installs it) even on a
run that otherwise succeeds — asserted absent are only the two things a refusal or a crash
would add.
"""
completed = _run_in_a_fresh_process(_argv(tmp_path))
assert completed.returncode == 0, completed.stderr
assert "Traceback (most recent call last)" not in completed.stderr, completed.stderr
assert "run refused" not in completed.stderr, completed.stderr

59
tests/test_explore_callsites_loadbearing.py
View file

@ -472,7 +472,7 @@ def test_the_exploration_artefact_carries_the_rounds_and_the_advisory_verdicts(
def test_the_artefact_is_written_even_when_the_exploration_was_cut_short(
tmp_path, monkeypatch
tmp_path, monkeypatch, capsys
) -> None:
"""T13: a capped exploration is the run whose evidence matters MOST, and it is the one that
returns nothing — so the write lives in a ``finally`` (the ``write_parse_failures`` precedent).
@ -481,7 +481,18 @@ def test_the_artefact_is_written_even_when_the_exploration_was_cut_short(
``stop: null`` that meant BOTH "concluded normally" and "we never found out" would be the kind
of silence this repo writes required fields to close.
Detach point: move the write out of the ``finally`` → RED.
**``main()`` no longer lets ``BudgetExceeded`` escape as a traceback** (fiks-ordre
20260904T070930Z, observed by accident on K2's syretest): this test used to wrap the call in
``pytest.raises(BudgetExceeded)``, which is the in-process shape of the very defect the order
fixes — the exception reaching all the way out of ``main()`` IS the traceback a subprocess
would print. It now asserts the typed refusal instead: rc 1, one line on stderr, same shape as
every other loader refusal in this function. The subprocess proof that no Python traceback
text reaches stderr lives in ``tests/test_explore_budget_refusal_loadbearing.py`` (P4's own
"stdout/stderr is answered in a subprocess" precedent — this in-process arm cannot see printed
text that was never printed because the exception unwound past ``capsys`` instead).
Detach point: move the write out of the ``finally`` → RED. Detach point for the refusal:
remove the ``except BudgetExceeded`` clause → this arm errors instead of asserting rc 1.
"""
factory = _factory(
ledgers=[_ledger_json(satisfied=False), _ledger_json(satisfied=False)],
@ -491,29 +502,33 @@ def test_the_artefact_is_written_even_when_the_exploration_was_cut_short(
monkeypatch.setattr("portfolio_optimiser.run._default_factory", lambda profile: factory)
outbox = tmp_path / "outbox"
with pytest.raises(BudgetExceeded):
run.main(
[
_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--explore",
"go",
"--explore-config",
_config_file(tmp_path, max_rounds=2),
"--outbox-dir",
str(outbox),
"--run-id",
"r2",
]
)
rc = run.main(
[
_PID,
"--docs-dir",
str(_BUNDLE_DIR),
"--bundle-dir",
str(_BUNDLE_DIR),
"--explore",
"go",
"--explore-config",
_config_file(tmp_path, max_rounds=2),
"--outbox-dir",
str(outbox),
"--run-id",
"r2",
]
)
assert rc == 1
err = capsys.readouterr().err
assert err.startswith("run refused: budget exceeded: exploration_rounds"), err
payload = json.loads((outbox / "r2-exploration.json").read_text(encoding="utf-8"))
assert payload["completed"] is False
assert payload["stop"] is None
assert len(payload["rounds"]) == 2
assert payload["approaches"] == [], "no mandate ever formed, so there is nothing to list"
def test_the_artefact_payload_is_byte_deterministic() -> None:
@ -537,8 +552,8 @@ def test_the_artefact_payload_is_byte_deterministic() -> None:
bundle_id="b", proposal_json="{}", verdict={"decision": "unparseable"}
)
)
first = explore.trace_payload(trace, stop=None, completed=True)
second = explore.trace_payload(trace, stop=None, completed=True)
first = explore.trace_payload(trace, stop=None, completed=True, mandate=None)
second = explore.trace_payload(trace, stop=None, completed=True, mandate=None)
assert json.dumps(first, sort_keys=True) == json.dumps(second, sort_keys=True)

2
tests/test_scripted_explore_door_loadbearing.py
View file

@ -275,7 +275,7 @@ def test_the_record_leaves_the_run_in_the_artefact_beside_the_advisory_verdicts(
trace = ex.ExplorationTrace()
trace.tool_calls.append(ex.ToolCall(name="read_bundle", bundle_id="bygg-energi-mikro", path=""))
payload = ex.trace_payload(trace, stop=None, completed=True)
payload = ex.trace_payload(trace, stop=None, completed=True, mandate=None)
# ``path`` joined the record in S7a-3 pkt. 3 and is ``""`` here because ``read_bundle`` takes
# none; its own gate is ``tests/test_tool_call_path_loadbearing.py``.

2
tests/test_tool_call_path_loadbearing.py
View file

@ -112,7 +112,7 @@ def test_the_artefact_carries_the_path_beside_the_name_and_the_base() -> None:
trace = ex.ExplorationTrace()
trace.tool_calls.append(ex.ToolCall(name="read_file", bundle_id="k2", path="prisskjema.md"))
payload = ex.trace_payload(trace, stop=None, completed=True)
payload = ex.trace_payload(trace, stop=None, completed=True, mandate=None)
assert payload["tool_calls"] == [
{"name": "read_file", "bundle_id": "k2", "path": "prisskjema.md"}