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

fix(run): the portfolio CLI stops swallowing its offline door and its failures

Browse source 
Walking --portfolio end-to-end as a downloader would -- which no session had
done -- surfaced two defects of a class this repo already legislates against.

(A) --scripted-replies was silently DROPPED in portfolio mode. main()'s
portfolio dispatch returned before the block that builds the scripted client
factory, and the flag was absent from the single_only refusal set: neither
honoured nor refused. Measured against the shipped reference portfolio: no
banner, four real model calls attempted, four APIConnectionError. The previous
session joined this flag to the --report allowlist and missed the portfolio
partition. Resolved by WIRING rather than refusing -- run_portfolio already
exposes the same client_factory seam, and refusing would have left portfolio
mode with no offline door at all for an adopter without a model budget. The
scripted block is hoisted above the dispatch; the single-project required-arg
and semantic-retrieval refusals are hoisted with it so an incomplete argv is
still refused BEFORE the honesty banner could claim a scripted run happened,
and the refusal order within single-project mode is unchanged.

(B) A portfolio pass reported one of its four outcome channels. failures
(S3.3 collect-and-continue) and budget_stop (S3.4 global cap) never reached the
operator and rc was unconditionally 0, so the four-failure pass above printed
NOTHING and exited 0 -- silence read as success. BudgetStop is a separate field
precisely so exhaustion can be told from success; the CLI showed neither.
Failures now print to stderr with project id, error type and message; the
budget stop prints its four numbers; rc is 1 iff something raised. A budget
stop alone stays rc 0: exhaustion is a structured stop the operator asked for
by setting a cap, not a crash. Completed runs still print, so the non-zero rc
does not undo collect-and-continue.

Load-bearing MEASURED against the whole 662-test suite, seven mutations all
red, including both controls: detach the client_factory wiring - make the
banner a single-project courtesy again - detach the failure print - revert rc
to 0 - detach the budget-stop print - print the failure line unconditionally
(control) - print the budget-stop line unconditionally (control).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0118noV9rCfrdREH26XqZB5z
This commit is contained in:
Kjell Tore Guttormsen 2026-08-05 10:48:13 +02:00
commit 415ebbb7f2
2 changed files with 344 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

159
src/portfolio_optimiser/run.py
View file

@ -1230,6 +1230,81 @@ def main(argv: list[str] | None = None) -> int:
)
return 1
# Single-project mode requires PROJECT_ID + --docs-dir (compensating for the relaxed argparse
# required/positional so the legacy contract keeps failing loudly via the refusal surface).
# HOISTED above the scripted door (below) so an incomplete argv is refused BEFORE the honesty
# banner could claim a scripted run happened; the refusal ORDER within single-project mode
# (required args -> semantic-retrieval -> scripted) is unchanged.
if not args.portfolio and (args.project_id is None or args.docs_dir is None):
print(
"run refused: single-project mode requires PROJECT_ID and --docs-dir "
"(use --portfolio for portfolio mode)",
file=sys.stderr,
)
return 1
# --semantic-retrieval is refused, never silently ignored (the repo's flag contract). In
# single-project mode it can only do observable work with BOTH of these: the Step-1 fold is
# gated on ``bundle_dir``, and ``--verdict-dir`` is the only route by which ``main()`` can hand
# ``run_project`` a non-empty store (``main()`` never passes ``store=``, and ``run_project``
# never seeds one). Without them the flag would rank nothing that reaches a prompt, and
# ``RunResult.retrieved`` never leaves the process — ``main()`` prints one outcome line only.
#
# DELIBERATELY STATIC. There is no runtime "refuse if the store ends up empty" check: a
# missing, empty or partially-skipped inbox is the Steg-7 tolerant-load contract, so refusing
# there would fire on a legitimate first run. The refusal is therefore necessary, not
# sufficient — it catches the configuration that CANNOT work, not every run that finds nothing.
#
# main() only. As a library API, ``run_project(semantic_retrieval=True, store=…)`` with a
# caller-supplied store stays legitimate — that is the path the tests drive. Portfolio mode is
# unaffected: ``run_portfolio`` always resolves a store and populates it by cross-project capture.
if not args.portfolio and args.semantic_retrieval:
required = {"--bundle-dir": args.bundle_dir, "--verdict-dir": args.verdict_dir}
missing = [name for name, value in required.items() if not value]
if missing:
print(
f"run refused: --semantic-retrieval requires {' and '.join(missing)} in "
"single-project mode (the Step-1 fold is bundle-path-only, and --verdict-dir is "
"the only route to a non-empty store)",
file=sys.stderr,
)
return 1
# The scripted door (offline WHOLE-loop run over the caller's own data). Resolved BEFORE the
# dry-run branch so the two offline modes cannot both be honoured — and BEFORE the portfolio
# dispatch, because the door serves BOTH modes. It originally sat below that dispatch, which
# made ``--portfolio --scripted-replies`` silently drop the flag: no banner, and four real
# model calls attempted (measured). That is the failure mode the "refused, never ignored"
# partition exists to prevent, and here the honest resolution is to WIRE it — ``run_portfolio``
# already exposes the same ``client_factory`` seam ``run_project`` does, so refusing would have
# left portfolio mode with no offline door at all for an adopter without a model budget.
scripted_client_factory: Callable[[str], BaseChatClient] | None = None
if args.scripted_replies is not None:
if args.live_dry_run:
# Both are offline, and they contradict: --live-dry-run stops before the first model
# call while --scripted-replies answers every one of them. Refuse rather than let one
# silently win (S5.3's "refused, never ignored" partition). Unreachable in portfolio
# mode, where --live-dry-run is already refused by the single_only partition above.
print(
"run refused: --scripted-replies and --live-dry-run are both offline modes and "
"contradict each other (dry-run stops before the first model call; scripted "
"answers all of them) — pick one",
file=sys.stderr,
)
return 1
try:
replies = _load_scripted_replies(args.scripted_replies)
except (OSError, ValueError) as exc:
print(f"run refused: {exc}", file=sys.stderr)
return 1
# Imported HERE rather than at module scope: ``simulation`` imports ``run``, so a top-level
# import would be circular. The scripted client already exists as MAF-side scaffolding —
# this flag is a DOOR onto that one seam, never a second implementation of it.
from portfolio_optimiser.simulation import scripted_factory
scripted_client_factory = scripted_factory(replies, [])
print(_SCRIPTED_BANNER)
if args.portfolio:
# Portfolio mode (Step 3): dispatch to the EXISTING run_portfolio via the fail-fast loaders
# (run_portfolio itself is unchanged). Loader/ValueError failures surface through the same
@ -1253,6 +1328,7 @@ def main(argv: list[str] | None = None) -> int:
ledger=ledger,
goals=goals,
semantic_retrieval=args.semantic_retrieval,
client_factory=scripted_client_factory,
)
)
except (ValueError, FileNotFoundError, ValidationError) as exc:
@ -1267,72 +1343,29 @@ def main(argv: list[str] | None = None) -> int:
f"observed_ore={sr.observed_ore} limit_ore={sr.limit_ore} "
f"stopped_early={portfolio_result.stopped_early}"
)
return 0
# Single-project mode requires PROJECT_ID + --docs-dir (compensating for the relaxed argparse
# required/positional so the legacy contract keeps failing loudly via the refusal surface).
if args.project_id is None or args.docs_dir is None:
print(
"run refused: single-project mode requires PROJECT_ID and --docs-dir "
"(use --portfolio for portfolio mode)",
file=sys.stderr,
)
return 1
# --semantic-retrieval is refused, never silently ignored (the repo's flag contract). In
# single-project mode it can only do observable work with BOTH of these: the Step-1 fold is
# gated on ``bundle_dir``, and ``--verdict-dir`` is the only route by which ``main()`` can hand
# ``run_project`` a non-empty store (``main()`` never passes ``store=``, and ``run_project``
# never seeds one). Without them the flag would rank nothing that reaches a prompt, and
# ``RunResult.retrieved`` never leaves the process — ``main()`` prints one outcome line only.
#
# DELIBERATELY STATIC. There is no runtime "refuse if the store ends up empty" check: a
# missing, empty or partially-skipped inbox is the Steg-7 tolerant-load contract, so refusing
# there would fire on a legitimate first run. The refusal is therefore necessary, not
# sufficient — it catches the configuration that CANNOT work, not every run that finds nothing.
#
# main() only. As a library API, ``run_project(semantic_retrieval=True, store=…)`` with a
# caller-supplied store stays legitimate — that is the path the tests drive. Portfolio mode is
# unaffected: ``run_portfolio`` always resolves a store and populates it by cross-project capture.
if args.semantic_retrieval:
required = {"--bundle-dir": args.bundle_dir, "--verdict-dir": args.verdict_dir}
missing = [name for name, value in required.items() if not value]
if missing:
# A ``PortfolioResult`` has FOUR outcome channels and this branch reported one of them:
# ``failures`` and ``budget_stop`` never reached the operator, and rc was unconditionally 0
# — so a pass in which every project died printed nothing and exited 0 (measured: four
# projects, four APIConnectionError, silent success). ``BudgetStop`` is kept apart from
# ``stop_reason`` precisely so a caller can tell exhaustion from success; showing neither
# collapsed the distinction the dataclass was split to preserve.
if portfolio_result.budget_stop is not None:
bs = portfolio_result.budget_stop
print(
f"run refused: --semantic-retrieval requires {' and '.join(missing)} in "
"single-project mode (the Step-1 fold is bundle-path-only, and --verdict-dir is "
"the only route to a non-empty store)",
f"budget stop: limit_tokens={bs.limit_tokens} spent_tokens={bs.spent_tokens} "
f"remaining_tokens={bs.remaining_tokens} required_tokens={bs.required_tokens} "
f"stopped_early={portfolio_result.stopped_early}"
)
for failure in portfolio_result.failures:
print(
f"project failed: {failure.project_id} [{failure.error_type}] {failure.error}",
file=sys.stderr,
)
return 1
# The scripted door (offline WHOLE-loop run over the caller's own bundle). Resolved BEFORE the
# dry-run branch so the two offline modes cannot both be honoured.
scripted_client_factory: Callable[[str], BaseChatClient] | None = None
if args.scripted_replies is not None:
if args.live_dry_run:
# Both are offline, and they contradict: --live-dry-run stops before the first model
# call while --scripted-replies answers every one of them. Refuse rather than let one
# silently win (S5.3's "refused, never ignored" partition).
print(
"run refused: --scripted-replies and --live-dry-run are both offline modes and "
"contradict each other (dry-run stops before the first model call; scripted "
"answers all of them) — pick one",
file=sys.stderr,
)
return 1
try:
replies = _load_scripted_replies(args.scripted_replies)
except (OSError, ValueError) as exc:
print(f"run refused: {exc}", file=sys.stderr)
return 1
# Imported HERE rather than at module scope: ``simulation`` imports ``run``, so a top-level
# import would be circular. The scripted client already exists as MAF-side scaffolding —
# this flag is a DOOR onto that one seam, never a second implementation of it.
from portfolio_optimiser.simulation import scripted_factory
scripted_client_factory = scripted_factory(replies, [])
print(_SCRIPTED_BANNER)
# rc 1 iff something RAISED. Collect-and-continue (S3.3) exists so a partial pass does not
# LOSE the work that completed — every finished run still printed above — not so a pass with
# dead projects can report success to a scripted caller. A ``budget_stop`` alone stays rc 0:
# exhaustion is a structured stop the operator asked for by setting a cap, not a crash.
return 1 if portfolio_result.failures else 0
if args.live_dry_run:
# S4.2 drill (comparison protocol §4 pkt 2/3): walk the offline path, STOP before the first

267
tests/test_portfolio_cli_offline_loadbearing.py Normal file
View file

@ -0,0 +1,267 @@
"""The portfolio CLI's two silent surfaces, measured from a fresh clone (adoption walk 2/2).
Walking ``--portfolio`` end-to-end as a downloader would — which no session had done; the
2026-08-03 measurement found ZERO reachings of ``run_portfolio`` in the simulation — turned up
two defects of the SAME class the repo already legislates against elsewhere, and neither was
reachable by a test that never invoked the CLI's portfolio branch:
**(A) ``--scripted-replies`` is silently DROPPED in portfolio mode.** ``main()``'s portfolio
dispatch returns (``run.py``, the ``if args.portfolio:`` block) BEFORE the scripted-replies block
that builds the client factory, and the flag is absent from the ``single_only`` refusal set — so
the flag neither takes effect nor is refused, the honesty banner never prints, and the pass
attempts REAL model calls. MEASURED against the shipped reference portfolio: four projects, four
``APIConnectionError`` failures. This is precisely the "refused, never ignored" partition
(S5.3) the ``--report`` allowlist enforces for this very flag — the previous session joined the
flag to one partition and missed the other. The fix WIRES it rather than refusing it:
``run_portfolio`` already exposes ``client_factory``, the same seam ``--scripted-replies`` was
built to expose on the single-project path, and refusing would leave portfolio mode with no
offline door at all for an adopter without a model budget.
**(B) A portfolio pass reports only ONE of its four outcome channels.** ``main()`` printed
``runs`` and ``stop_reason`` and nothing else: ``failures`` (S3.3 collect-and-continue) and
``budget_stop`` (S3.4 global cap) never reached the operator, and rc was unconditionally 0.
MEASURED: the four-failure pass above printed NOTHING AT ALL and exited 0 — silence read as
success. ``BudgetStop``'s own docstring argues that folding exhaustion into ``stop_reason``
"would let a caller read 'we stopped' without being able to tell which happened"; the CLI showed
neither. The rc rule is ``failures`` non-empty -> 1: collect-and-continue exists so a partial pass
does not LOSE the completed work (which still prints on stdout), not so a pass with dead projects
can call itself a success.
Load-bearing (each blade detaches exactly one thing):
1. the scripted factory reaches ``run_portfolio`` -> a REAL offline portfolio pass (RED when the
``client_factory=`` wiring is dropped: the pass falls back to the production factory);
2. that pass is genuinely model-free (RED if a real factory is built);
3. the banner prints in portfolio mode too (RED when the banner is moved back below the dispatch);
4. control: no banner without the flag, so blade 3 cannot pass on a constant;
5. failures are visible AND carry their own values (RED when the failure print is detached);
6. rc reflects them (RED when rc stays 0);
7. control: a clean pass prints no failure line and exits 0 — so 5/6 cannot pass on a constant;
8. a completed run still prints alongside a failure (collect-and-continue is not undone by 6);
9. the budget stop is visible with its own numbers (RED when that print is detached);
10. control: no budget-stop line when the pass was not budget-stopped.
"""
from __future__ import annotations
import json
from pathlib import Path
import pytest
from portfolio_optimiser import run
from portfolio_optimiser.ledger import LedgerEntry, SavingsLedger
from portfolio_optimiser.run import BudgetStop, PortfolioResult, RunFailure
from portfolio_optimiser.verdicts import VerdictStore
# The same caller-supplied answers the single-project door documents. On the shipped ROAD
# portfolio these are rejected by S4.0's baseline anchoring (the cost code belongs to the bygg
# bundle, not to a road project's estimate) — which is the correct outcome and is beside the
# point here: the blades below assert that the pass RUNS offline, not that it validates.
_VALID_PROPOSAL = (
'{"measure":"LED-retrofit av kontorbelysning","affected_items":'
'[{"code":"ENERGI-TOTAL-EL","quantity":300000,"unit_cost":1.0}],"claimed_saving_nok":30000}'
)
_CHECKER_APPROVE = "Tallene er innenfor feasibelt område og resonnementet holder. VERDICT: APPROVE"
@pytest.fixture(autouse=True)
def _isolate_model_env(monkeypatch: pytest.MonkeyPatch) -> None:
"""Hermetic env (mirrors ``test_scripted_cli_door_loadbearing.py``)."""
monkeypatch.delenv("PORTFOLIO_MODEL_MAP", raising=False)
monkeypatch.delenv("PORTFOLIO_FOUNDRY_PROJECT_ENDPOINT", raising=False)
@pytest.fixture()
def replies_file(tmp_path: Path) -> Path:
path = tmp_path / "replies.json"
path.write_text(
json.dumps({"proposer": _VALID_PROPOSAL, "checker": _CHECKER_APPROVE}),
encoding="utf-8",
)
return path
# --------------------------------------------------------------------------------------------
# (A) the offline door reaches portfolio mode
# --------------------------------------------------------------------------------------------
def test_portfolio_runs_offline_with_scripted_replies(replies_file, capsys) -> None:
"""Blade 1 — ``--portfolio --scripted-replies`` completes a real pass over the shipped
reference portfolio with zero model calls. RED before the wiring: every project raises
``APIConnectionError`` (measured: 4 runs, 4 failures) so no outcome line is printed."""
rc = run.main(["--portfolio", "--scripted-replies", str(replies_file)])
out = capsys.readouterr().out
assert rc == 0, out
# One outcome line per project in the shipped portfolio (4), each carrying a minted verdict id.
assert out.count("verdict id=") == 4, out
def test_portfolio_scripted_pass_makes_no_real_client(replies_file, monkeypatch, capsys) -> None:
"""Blade 2 — genuinely model-free. Detonates if the portfolio dispatch falls back to the
production factory (which is exactly what the un-wired flag did)."""
def _boom(_profile: str): # pragma: no cover - the point is that it never runs
raise AssertionError("a real client factory was built on the scripted portfolio path")
monkeypatch.setattr(run, "_default_factory", _boom)
rc = run.main(["--portfolio", "--scripted-replies", str(replies_file)])
assert rc == 0, capsys.readouterr().out
def test_portfolio_scripted_pass_says_so_unmistakably(replies_file, capsys) -> None:
"""Blade 3 — the honesty banner is not a single-project-mode courtesy. A scripted portfolio
pass that reads like a model pass is the same failure mode (målbilde §1)."""
run.main(["--portfolio", "--scripted-replies", str(replies_file)])
out = capsys.readouterr().out.upper()
assert "SCRIPTED" in out
assert "NO MODEL" in out or "INGEN MODELLKALL" in out
def test_no_banner_in_portfolio_mode_without_the_flag(tmp_path, capsys) -> None:
"""Blade 4 (control) — the banner must be CAUSED by the flag. Uses a met portfolio goal so the
control stops offline at the goal check, before any client is built."""
goals = tmp_path / "goals.json"
goals.write_text('{"portfolio": {"absolute_ore": 1, "mode": "hard"}}', encoding="utf-8")
led = SavingsLedger()
led.add_realized(
LedgerEntry(
project_id="FV42-GSV-E1",
dimension="energi",
candidate_identity="c1",
amount_ore=1,
verdict_id="v1",
provenance="x",
)
)
ledger = tmp_path / "ledger.json"
led.save(str(ledger))
rc = run.main(["--portfolio", "--goals", str(goals), "--ledger", str(ledger)])
assert rc == 0
assert "SCRIPTED" not in capsys.readouterr().out.upper()
# --------------------------------------------------------------------------------------------
# (B) the silent outcome channels
# --------------------------------------------------------------------------------------------
def _result(
*,
runs: tuple = (),
failures: tuple[RunFailure, ...] = (),
budget_stop: BudgetStop | None = None,
) -> PortfolioResult:
"""A crafted ``PortfolioResult`` — the unit under test is ``main()``'s reporting block, and
driving a genuine mid-pass failure through the CLI is impossible offline (the CLI builds its
own client factory from the replies file, so no failing client can be injected)."""
return PortfolioResult(
runs=runs,
store=VerdictStore([]),
validated_count=0,
rejected_count=0,
sum_claimed_saving_nok=0.0,
sum_token_usage=0,
stopped_early=budget_stop is not None,
failures=failures,
budget_stop=budget_stop,
)
@pytest.fixture()
def stub_portfolio(monkeypatch):
"""Install a ``run_portfolio`` stub returning a chosen ``PortfolioResult``."""
def _install(result: PortfolioResult) -> None:
async def _fake(*_args, **_kwargs) -> PortfolioResult:
return result
monkeypatch.setattr(run, "run_portfolio", _fake)
return _install
def test_failures_are_visible_with_their_own_values(stub_portfolio, capsys) -> None:
"""Blade 5 — a project that RAISED must reach the operator. TWO failures with distinct ids and
distinct messages: a canned string cannot produce both, so this cannot pass on a constant.
RED before the fix: the pass printed nothing at all."""
stub_portfolio(
_result(
failures=(
RunFailure("FV42-GSV-E1", "Connection error.", "APIConnectionError"),
RunFailure("RV13-RAS-TP", "budget blew up", "BudgetExceeded"),
)
)
)
rc = run.main(["--portfolio"])
err = capsys.readouterr().err
assert rc == 1
assert "FV42-GSV-E1" in err and "Connection error." in err
assert "RV13-RAS-TP" in err and "budget blew up" in err
assert "APIConnectionError" in err and "BudgetExceeded" in err
def test_a_pass_with_failures_does_not_exit_zero(stub_portfolio, capsys) -> None:
"""Blade 6 — silence-as-success was the worse half of the defect: a scripted caller checking
only rc learned nothing. MEASURED before the fix: four dead projects, rc 0."""
stub_portfolio(_result(failures=(RunFailure("FV42-GSV-E1", "boom", "RuntimeError"),)))
assert run.main(["--portfolio"]) == 1
capsys.readouterr()
def test_a_clean_pass_stays_silent_and_exits_zero(stub_portfolio, capsys) -> None:
"""Blade 7 (control) — no failures means no failure line and rc 0, so blades 5/6 cannot pass on
an unconditional print or an unconditional rc."""
stub_portfolio(_result())
rc = run.main(["--portfolio"])
captured = capsys.readouterr()
assert rc == 0
assert "failed" not in captured.err.lower()
assert captured.err.strip() == ""
def test_completed_runs_still_print_alongside_a_failure(replies_file, monkeypatch, capsys) -> None:
"""Blade 8 — the non-zero rc must not undo collect-and-continue: the projects that COMPLETED
still report their outcome on stdout. Driven through the real scripted pass, then one crafted
failure appended, so the ``runs`` side is genuine and not a stub artefact."""
real_run_portfolio = run.run_portfolio
async def _fake(*args, **kwargs) -> PortfolioResult:
genuine = await real_run_portfolio(*args, **kwargs)
return _result(
runs=genuine.runs,
failures=(RunFailure("RV13-RAS-TP", "boom", "RuntimeError"),),
)
monkeypatch.setattr(run, "run_portfolio", _fake)
rc = run.main(["--portfolio", "--scripted-replies", str(replies_file)])
captured = capsys.readouterr()
assert rc == 1
assert captured.out.count("verdict id=") == 4, captured.out
assert "RV13-RAS-TP" in captured.err
def test_budget_stop_is_visible_with_its_own_numbers(stub_portfolio, capsys) -> None:
"""Blade 9 — the S3.4 global-cap stop is a distinct outcome from a goal stop (``BudgetStop``
is a separate field for exactly that reason) and was equally invisible. The four numbers are
asserted individually: their DIFFERENCE is the operator's next decision."""
stub_portfolio(
_result(
budget_stop=BudgetStop(
limit_tokens=500, spent_tokens=470, remaining_tokens=30, required_tokens=120
)
)
)
rc = run.main(["--portfolio"])
out = capsys.readouterr().out
assert rc == 0 # exhaustion is a structured stop, not a failure — it does not fail the pass
assert "budget" in out.lower()
for number in ("500", "470", "30", "120"):
assert number in out, out
def test_no_budget_stop_line_when_the_pass_was_not_stopped(stub_portfolio, capsys) -> None:
"""Blade 10 (control) — blade 9 cannot pass on an unconditional line."""
stub_portfolio(_result())
run.main(["--portfolio"])
assert "budget" not in capsys.readouterr().out.lower()