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

fix(explore): lukk KeyError:'navigator' i --explore --scripted-replies

Browse source 
_SCRIPTED_ROLES = ("proposer", "checker") var det ENESTE rollesettet
_load_scripted_replies validerte OG returnerte. explore()s tre ekstra
roller (manager/navigator/hypothesiser) ble filtrert bort selv når de
fantes i --scripted-replies-JSON-en, og manglet en av dem krasjet CLI-en
med en rå KeyError midt i eksplorasjonssløyfa i stedet for en ren
"run refused:"-linje (MAJOR-2, docs/2026-08-25-syretest-vei-ab.md).

_EXPLORATION_SCRIPTED_ROLES legges nå til kravet KUN når --explore er
satt, slik at en manglende rolle nektes VED NAVN ved døren, før
explore() kalles. En rein debattkjøring skal ikke måtte svare for
roller den aldri bruker.

Målt (ikke bare antatt): med alle fem roller scriptet fullfører CLI-en
uten krasj, men sløyfa er fortsatt delvis vakuøs (1 runde, 0 approaches)
som syretesten forutså — en konstant streng per rolle kan ikke svare
korrekt på magentic-managerens stadiespesifikke former.

1028 passed / 5 skipped (+3 nye tester, RØD→GRØNN). Golden
demo-transcript byte-uendret.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YSHNrYvKxDR5uct1QoVZng
This commit is contained in:
Kjell Tore Guttormsen 2026-08-25 15:42:15 +02:00
commit 444fea7e94
2 changed files with 172 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

32
src/portfolio_optimiser/run.py
View file

@ -55,6 +55,9 @@ from portfolio_optimiser.datasource import (
)
from portfolio_optimiser.dimension import Dimension, admits, load_dimension
from portfolio_optimiser.explore import (
HYPOTHESISER_ROLE,
MANAGER_ROLE,
NAVIGATOR_ROLE,
ExplorationContract,
ExplorationResult,
ExplorationTrace,
@ -1530,6 +1533,12 @@ async def run_mandate_across_bundles(
# caught at the door instead of mid-run.
_SCRIPTED_ROLES = ("proposer", "checker")
# The THREE more roles ``explore()`` asks the same factory for (``explore.py:576``) — added to
# ``_SCRIPTED_ROLES`` at the door only when ``--explore`` is in play (MAJOR-2,
# docs/2026-08-25-syretest-vei-ab.md): a plain debate-only run must not be made to answer for
# roles it never uses.
_EXPLORATION_SCRIPTED_ROLES = (MANAGER_ROLE, NAVIGATOR_ROLE, HYPOTHESISER_ROLE)
# The honesty banner for the scripted door. It is a REQUIREMENT, not decoration (målbilde §1):
# a scripted run that reads like a model run is worse than having no offline mode at all, so this
# prints on every scripted invocation and mirrors ``simulation.main``'s banner.
@ -1544,10 +1553,13 @@ _SCRIPTED_BANNER = (
)
def _load_scripted_replies(path: str) -> dict[str, str]:
"""Load the caller's scripted answers, fail-fast. Every role the debate can ask for must be
def _load_scripted_replies(
path: str, required_roles: Sequence[str] = _SCRIPTED_ROLES
) -> dict[str, str]:
"""Load the caller's scripted answers, fail-fast. Every role ``required_roles`` names must be
present AND a string: a missing role would otherwise surface as a ``KeyError`` deep inside
``scripted_factory``'s lookup, mid-run, long after the run appeared to start cleanly."""
``scripted_factory``'s lookup, mid-run, long after the run appeared to start cleanly (MAJOR-2:
measured for the three ``explore()`` adds on top of the debate's own two)."""
try:
raw = json.loads(Path(path).read_text(encoding="utf-8"))
except FileNotFoundError as exc:
@ -1556,13 +1568,13 @@ def _load_scripted_replies(path: str) -> dict[str, str]:
raise ValueError(f"--scripted-replies is not valid JSON ({path}): {exc}") from exc
if not isinstance(raw, dict):
raise ValueError(f"--scripted-replies must be a JSON object of role -> reply ({path})")
missing = [r for r in _SCRIPTED_ROLES if not isinstance(raw.get(r), str)]
missing = [r for r in required_roles if not isinstance(raw.get(r), str)]
if missing:
raise ValueError(
f"--scripted-replies needs a string reply for each of {', '.join(_SCRIPTED_ROLES)}; "
f"--scripted-replies needs a string reply for each of {', '.join(required_roles)}; "
f"missing or non-string: {', '.join(missing)} ({path})"
)
return {role: raw[role] for role in _SCRIPTED_ROLES}
return {role: raw[role] for role in required_roles}
def main(argv: list[str] | None = None) -> int:
@ -2032,8 +2044,14 @@ def main(argv: list[str] | None = None) -> int:
file=sys.stderr,
)
return 1
# ``--explore`` asks the same factory for three roles the debate never uses (MAJOR-2) — the
# door must know about them BEFORE loading the file, or a missing one crashes deep inside
# ``explore()`` instead of being refused here, at the door, by name.
required_scripted_roles: Sequence[str] = _SCRIPTED_ROLES
if args.explore is not None:
required_scripted_roles = _SCRIPTED_ROLES + _EXPLORATION_SCRIPTED_ROLES
try:
replies = _load_scripted_replies(args.scripted_replies)
replies = _load_scripted_replies(args.scripted_replies, required_scripted_roles)
except (OSError, ValueError) as exc:
print(f"run refused: {exc}", file=sys.stderr)
return 1

147
tests/test_scripted_explore_door_loadbearing.py Normal file
View file

@ -0,0 +1,147 @@
"""MAJOR-2 (docs/2026-08-25-syretest-vei-ab.md) — ``--explore --scripted-replies`` must not
crash with a raw ``KeyError: 'navigator'``.
``_SCRIPTED_ROLES = ("proposer", "checker")`` (``run.py:1531``) is the debate's two roles.
``explore()`` asks the SAME ``client_factory`` for three more: ``manager``, ``navigator``,
``hypothesiser`` (``explore.py:576``). ``_load_scripted_replies`` was fail-fast for the two roles
it knew about — measured (docs/2026-08-25-syretest-vei-ab.md § MAJOR-2) to let the three it did not
know about surface exactly the ``KeyError`` deep inside ``scripted_factory``'s lookup its own
docstring warns against, mid-run, after the banner had already printed.
The fix widens the required-role set to include the exploration's three roles WHEN ``--explore``
is in play, so a missing role is refused BY NAME before any model/agent work starts — the same
door, never a second one.
"""
from __future__ import annotations
import json
from pathlib import Path
from typing import Any
from portfolio_optimiser import run
_BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
_PID = "BYGG-KONTOR-NORD"
_PROPOSER_REPLY = json.dumps(
{
"measure": "LED-retrofit",
"affected_items": [{"code": "ENERGI-TOTAL-EL", "quantity": 300000, "unit_cost": 1.0}],
"claimed_saving_nok": 30000,
}
)
def _config_file(tmp_path: Path, **overrides: Any) -> str:
path = tmp_path / "exploration.json"
path.write_text(
json.dumps(
{
"max_rounds": 4,
"max_tokens": 100_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, roles: dict[str, str]) -> str:
path = tmp_path / "replies.json"
path.write_text(json.dumps(roles), encoding="utf-8")
return str(path)
def _base_argv(tmp_path: Path, replies_path: str) -> 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_path,
]
def test_explore_with_debate_only_scripted_replies_is_refused_by_name(tmp_path, capsys) -> None:
"""A ``--scripted-replies`` file that only answers the debate (proposer/checker) — exactly the
file MAJOR-2 was measured against — is refused BY NAME, never left to crash mid-run.
Detach point: revert ``_SCRIPTED_ROLES`` to the fixed debate-only tuple used for ``--explore``
too → this raises an unhandled ``KeyError`` instead of returning 1 (RED, reproduces MAJOR-2).
"""
replies = _replies_file(tmp_path, {"proposer": _PROPOSER_REPLY, "checker": "VERDICT: APPROVE"})
rc = run.main(_base_argv(tmp_path, replies))
assert rc == 1, "a role the exploration needs is missing — this must be a clean refusal"
err = capsys.readouterr().err
assert "run refused" in err
for role in ("manager", "navigator", "hypothesiser"):
assert role in err, f"the refusal must name the missing role {role!r}"
def test_explore_with_debate_only_scripted_replies_never_reaches_a_model_call(
tmp_path, capsys, monkeypatch
) -> None:
"""The refusal fires at the DOOR — before ``explore()`` is even entered. A spy on ``explore``
proves zero exploration work happened, the same way econ 57's outbox/run-id hoist was proved.
Detach point: let the flag through and catch the ``KeyError`` further in → this spy would still
record a call (RED).
"""
calls: list[object] = []
monkeypatch.setattr(
"portfolio_optimiser.run.explore",
lambda *a, **kw: (
calls.append((a, kw)) or (_ for _ in ()).throw(AssertionError("unreached"))
),
)
replies = _replies_file(tmp_path, {"proposer": _PROPOSER_REPLY, "checker": "VERDICT: APPROVE"})
rc = run.main(_base_argv(tmp_path, replies))
assert rc == 1
assert calls == [], "explore() must never be entered when a required role is missing"
def test_explore_with_the_full_five_role_scripted_replies_does_not_crash(tmp_path, capsys) -> None:
"""With every role ``explore()`` can ask for supplied as a constant string, the CLI door must
run the loop to completion (or a typed budget/refusal outcome) — never a raw traceback.
A constant per-role reply cannot answer every stage-specific shape the magentic manager can be
prompted with (facts / plan / progress-ledger JSON / final answer all differ) — so this does
not assert the exploration finds anything, only that the documented crash is gone.
"""
replies = _replies_file(
tmp_path,
{
"proposer": _PROPOSER_REPLY,
"checker": "VERDICT: APPROVE",
"manager": '{"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"}}',
"navigator": "NAVIGATOR: read the index.",
"hypothesiser": "HYPOTHESIS: " + json.dumps({"label": "x", "rationale": "y"}),
},
)
rc = run.main(_base_argv(tmp_path, replies))
err = capsys.readouterr().err
assert "KeyError" not in err
assert "Traceback" not in err
assert rc in (0, 1), f"expected a clean exit, got rc={rc} stderr={err!r}"