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

fix(cli): a provider failure leaves the CLI as one line, and a guessed base id is correctable

Browse source 
Funn 99, measured offline against the artefacts the paid Q5=B run left behind — no paid
run here.

ROOT, verbatim from the records: the three failing quick_validate calls all sent
bundle_id="renholdstekniske_funksjonskrav" — a CONCEPT name guessed out of the seeded cut,
while the base's id is k2-trinn1-20260903. Both arguments parsed against the signature, so
it was _resolve_bundle's raise MAF counted, proven by quick_validations being EMPTY while
all three stand in tool_calls. Denominator: 12 tool calls, and those three came BEFORE
list_bundles.

The order's causal chain is FELLED: the quick_validate triple is records 4-6 and the run
continued for 13 more model calls; the triple immediately before the 400 is the navigator's
three read_file refusals on del-ii-bilag-7-prisskjema*. The limit fired TWICE.

(A) ChatClientException is caught on BOTH seams — the exploration dispatch and the full-run
dispatch — because the debate's own model calls go through the same provider. The line is
"run stopped:", not "run refused:" (a stated divergence from the order): the argv was fine
and tokens were already spent, which is the MAJOR-2 arm's own reason, verbatim. Caught
INSIDE the try/finally so the exploration artefact still lands.

(B) quick_validate answers an unknown base id with {"decision": "refused", ...} naming the
configured ids, and records it in the sink. MAF turns a tool raise into the opaque
"Error: Function failed." (_tools.py:1426), so the one thing the refusal knew and the model
did not never reached it — the replies show it guessing at the JSON format instead.
read_file/read_dir/read_bundle still raise: measured, reported, out of scope.

Seven mutations all red against the whole suite, green control 1529/5, golden ea8c534
unchanged. One existing gate REWRITTEN, not deleted; its second half is what keeps (B)
scoped. The test double raises from the reply_selector seam rather than a new
_inner_get_response body, so the S2.5 consolidation guard stays untouched.

Co-Authored-By: Claude <claude-opus-5>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-08 21:03:18 +02:00
commit 078a099898
5 changed files with 543 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

15
tests/test_explore_loadbearing.py
View file

@ -752,10 +752,23 @@ def test_an_unknown_knowledge_base_is_refused_by_name() -> None:
Model-chosen arguments are untrusted input. Answering an unknown id with an empty result would
let the manager conclude the base is empty rather than absent — the fourth face of the
verification law, arrived at through a tool rather than a query.
REWRITTEN, not weakened (funn 99): ``quick_validate`` now answers with a ``refused`` VERDICT
rather than a raise, because MAF converts a tool raise into the opaque ``"Error: Function
failed."`` and counts three of them as grounds to stop function calling for the whole request —
so the one thing this refusal knows and the model does not (which ids exist) never reached it.
The property T20 was written for is untouched and asserted here in both halves: the answer is
still a refusal, and it still names what IS configured. The ``navigator_tools`` half is what
keeps the change SCOPED — those three tools still raise.
"""
validate = explore.quick_validate_tool(("/tmp/base-a",))
verdict = validate.func(bundle_id="base-b", proposal_json="{}")
assert verdict["decision"] == "refused"
assert "base-b" in verdict["reason"] and "base-a" in verdict["reason"]
read_file = next(t for t in explore.navigator_tools(("/tmp/base-a",)) if t.name == "read_file")
with pytest.raises(explore.ExplorationError) as excinfo:
validate.func(bundle_id="base-b", proposal_json="{}")
read_file.func(bundle_id="base-b", path="x.md")
assert "base-a" in str(excinfo.value)

268
tests/test_run_cli_chatclient_refusal.py Normal file
View file

@ -0,0 +1,268 @@
"""Funn 99 — a provider failure must LEAVE the CLI as one line, not as a traceback.
MEASURED FIRST, on the artefacts the paid Q5=B run left behind (``scratchpad/q5b/live/``,
never re-run here): the exploration died on
agent_framework.exceptions.ChatClientException:
<class 'FoundryChatClient'> service failed to complete the prompt: Error code: 400 -
{'error': {'message': 'No tool call found for function call output with call_id
call_AyeuYJmqvmmej9utu361Phtt.', ...}}
and that class is in NONE of ``main()``'s refusal tuples (``run.py`` catches
``FileNotFoundError, ValidationError, ValueError`` plus ``BudgetExceeded``/``PlanReviewParked``),
so the process tracebacked — the same defect class ``BudgetExceeded`` was added to that block for.
TWO seams are closed here and each one has its OWN arm, because each can regress alone:
(A) the EXPLORATION dispatch (``run.py``'s ``explore()``/``resume_exploration()`` block), which
is where the measured failure happened; and
(A2) the FULL-RUN dispatch (``run_project``), which the debate's own model calls go through —
a fix on only one of the two leaves the other tracebacking.
The KNOWN-NEGATIVE is the point of the arm, not decoration: a ``RuntimeError`` from the same seam
must still propagate. We are closing ONE named provider channel, never hiding unknown failures.
Arm (B) is the tool side, and it is authorised by the measurement rather than by symmetry: the
three failing ``quick_validate`` calls in ``Bseed-records.json`` were NOT verdicts. All three sent
``bundle_id="renholdstekniske_funksjonskrav"`` — a concept name, guessed out of the seeded cut,
never a base id — with a well-formed ``proposal_json``, so the arguments parsed against the
signature and it was ``_resolve_bundle``'s raise that MAF counted. Proof it was the raise and not
a verdict: ``q5b-Bseed-exploration.json`` records all three in ``tool_calls`` while
``quick_validations`` is EMPTY, and the sink is appended on every verdict branch.
"""
from __future__ import annotations
import json
from collections.abc import Callable
from pathlib import Path
from typing import Any
import pytest
from agent_framework import BaseChatClient
from agent_framework.exceptions import ChatClientException
from portfolio_optimiser import explore, run
from portfolio_optimiser.simulation import ScriptedChatClient
_BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
_PID = "BYGG-KONTOR-NORD"
#: The provider text VERBATIM from the measured run — truncated only where the records truncate it.
_PROVIDER_TEXT = (
"<class 'agent_framework_foundry._chat_client.FoundryChatClient'> service failed to complete "
"the prompt: Error code: 400 - {'error': {'message': 'No tool call found for function call "
"output with call_id call_AyeuYJmqvmmej9utu361Phtt.', 'type': 'invalid_request_error'}}"
)
_CONTRACT_JSON: dict[str, Any] = {
"max_rounds": 2,
"max_tokens": 50_000,
"max_stall_count": 2,
"max_reset_count": 1,
"max_plan_revisions": 0,
"enable_plan_review": False,
}
@pytest.fixture(autouse=True)
def _isolate_model_env(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.delenv("PORTFOLIO_MODEL_MAP", raising=False)
monkeypatch.delenv("PORTFOLIO_OTEL", raising=False)
def _raising_factory(exc: BaseException) -> Callable[[str], Callable[[str], BaseChatClient]]:
"""A factory whose clients raise ``exc`` on the FIRST model call, before any reply exists.
The raise lives in the ``reply_selector`` seam rather than in an ``_inner_get_response``
override, and that is deliberate: the canonical body calls the selector SYNCHRONOUSLY
(``simulation.py:443``) before it builds any coroutine, so a raise there leaves the client at
exactly the point a provider's would — and the S2.5 consolidation guard keeps its property
that there is no second copy of the scripted body anywhere in the tree.
"""
def _raise(_prompt: str, _role: str) -> str:
raise exc
def outer(_profile: Any) -> Callable[[str], BaseChatClient]:
def factory(role: str) -> BaseChatClient:
return ScriptedChatClient(reply_selector=_raise, role=role)
return factory
return outer
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 _explore_argv(tmp_path: Path, *extra: 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),
*extra,
]
# ---------------------------------------------------------------------------------------------
# (A) the exploration dispatch
# ---------------------------------------------------------------------------------------------
def test_a_provider_failure_in_the_exploration_leaves_the_cli_as_one_line(
tmp_path, monkeypatch, capsys
) -> None:
"""T1 — POSITIVE. ``--explore`` against a client that raises ``ChatClientException`` must give
rc 1 and ONE stderr line, never a traceback.
Detach point: remove the ``except ChatClientException`` arm from the exploration block → the
exception escapes ``main()`` and pytest reports the raise instead of an rc → RED.
"""
monkeypatch.setattr(
"portfolio_optimiser.run._default_factory",
_raising_factory(ChatClientException(_PROVIDER_TEXT)),
)
rc = run.main(_explore_argv(tmp_path))
err = capsys.readouterr().err
assert rc == 1
assert "Traceback" not in err
assert len([line for line in err.splitlines() if line.strip()]) == 1
assert err.startswith("run stopped:")
assert "No tool call found for function call output" in err
def test_an_unknown_failure_from_the_same_seam_still_propagates(tmp_path, monkeypatch) -> None:
"""T2 — KNOWN-NEGATIVE. The arm is ONE named provider channel, not a blanket ``except``.
Without this, a fix written as ``except Exception`` would pass T1 and hide every programming
error the exploration can make. Detach point: widen the arm to ``Exception`` → RED.
"""
monkeypatch.setattr(
"portfolio_optimiser.run._default_factory",
_raising_factory(RuntimeError("a defect, not a provider")),
)
with pytest.raises(RuntimeError, match="a defect, not a provider"):
run.main(_explore_argv(tmp_path))
def test_the_exploration_artefact_survives_the_refusal(tmp_path, monkeypatch, capsys) -> None:
"""T3 — the evidence written BEFORE the failure must not disappear with it.
``run.py``'s ``finally`` writes ``{run_id}-exploration.json`` whatever ended the block; an arm
placed so the ``finally`` is skipped (or one that returns before it) would take the one record
of what the run had already spent with it. Detach point: catch the exception OUTSIDE the
try/finally → RED.
"""
outbox = tmp_path / "outbox"
monkeypatch.setattr(
"portfolio_optimiser.run._default_factory",
_raising_factory(ChatClientException(_PROVIDER_TEXT)),
)
rc = run.main(_explore_argv(tmp_path, "--outbox-dir", str(outbox), "--run-id", "funn99"))
assert rc == 1
assert capsys.readouterr().err.startswith("run stopped:")
artefact = outbox / "funn99-exploration.json"
assert artefact.exists()
payload = json.loads(artefact.read_text(encoding="utf-8"))
assert payload["completed"] is False
# ---------------------------------------------------------------------------------------------
# (A2) the full-run dispatch — the debate's own model calls
# ---------------------------------------------------------------------------------------------
def test_a_provider_failure_in_the_full_run_leaves_the_cli_as_one_line(monkeypatch, capsys) -> None:
"""T4 — the SECOND seam. The debate calls the same provider, and a fix on the exploration
block alone leaves an ordinary ``run.main([...])`` tracebacking.
Detach point: remove the ``except ChatClientException`` arm from the full-run dispatch → RED,
and T1 stays green — which is exactly why this is its own arm.
"""
monkeypatch.setattr(
"portfolio_optimiser.run._default_factory",
_raising_factory(ChatClientException(_PROVIDER_TEXT)),
)
rc = run.main([_PID, "--docs-dir", str(_BUNDLE_DIR), "--bundle-dir", str(_BUNDLE_DIR)])
err = capsys.readouterr().err
assert rc == 1
assert "Traceback" not in err
assert len([line for line in err.splitlines() if line.strip()]) == 1
assert err.startswith("run stopped:")
def test_an_unknown_failure_in_the_full_run_still_propagates(monkeypatch) -> None:
"""T5 — KNOWN-NEGATIVE for the second seam, for T2's reason."""
monkeypatch.setattr(
"portfolio_optimiser.run._default_factory",
_raising_factory(RuntimeError("a defect, not a provider")),
)
with pytest.raises(RuntimeError, match="a defect, not a provider"):
run.main([_PID, "--docs-dir", str(_BUNDLE_DIR), "--bundle-dir", str(_BUNDLE_DIR)])
# ---------------------------------------------------------------------------------------------
# (B) the tool — a base id the model guessed wrong is a thing it can CORRECT, not a run-ender
# ---------------------------------------------------------------------------------------------
def test_an_unknown_base_id_comes_back_as_a_refused_verdict() -> None:
"""T6 — POSITIVE. ``quick_validate`` against an id no configured base carries must RETURN a
``refused`` verdict naming the configured ids, not raise.
MEASURED root, verbatim from ``Bseed-records.json``: three calls with
``bundle_id="renholdstekniske_funksjonskrav"``. Each raise became MAF's opaque
``"Error: Function failed."`` (``_tools.py:1426``; details are suppressed unless
``include_detailed_errors``), so the reason po had written — *which* ids exist — never reached
the model, and the replies show it guessing at the JSON format instead. Three in a row hit
``DEFAULT_MAX_CONSECUTIVE_ERRORS_PER_REQUEST`` (``_tools.py:96``, value 3).
Detach point: restore the raise → RED.
"""
sink: list[explore.QuickValidation] = []
tool = explore.quick_validate_tool((str(_BUNDLE_DIR),), sink=sink)
verdict = tool.func(bundle_id="renholdstekniske_funksjonskrav", proposal_json="{}")
assert verdict["decision"] == "refused"
assert "renholdstekniske_funksjonskrav" in verdict["reason"]
assert _BUNDLE_DIR.name in verdict["reason"] or "bygg" in verdict["reason"]
assert verdict["anchored"] is False
def test_a_known_base_id_still_reaches_the_validator() -> None:
"""T7 — CONTROL. Without it, an implementation that answered ``refused`` to EVERY call would
pass T6 while destroying the tool. Detach point: return ``refused`` unconditionally → RED."""
sink: list[explore.QuickValidation] = []
tool = explore.quick_validate_tool((str(_BUNDLE_DIR),), sink=sink)
verdict = tool.func(bundle_id=_bundle_id(), proposal_json="{}")
assert verdict["decision"] == "unparseable"
def test_the_refusal_is_recorded_in_the_sink() -> None:
"""T8 — a refused call is a thing the hypothesiser ASKED for, and the artefact is where an
operator reads that it happened.
While it raised, the call left ``quick_validations`` empty and was visible only in
``tool_calls`` — which is precisely why this session had to read two artefacts to find the
root. Detach point: skip the sink append on the refused branch → RED.
"""
sink: list[explore.QuickValidation] = []
tool = explore.quick_validate_tool((str(_BUNDLE_DIR),), sink=sink)
tool.func(bundle_id="renholdstekniske_funksjonskrav", proposal_json="{}")
assert [entry.verdict["decision"] for entry in sink] == ["refused"]
assert sink[0].bundle_id == "renholdstekniske_funksjonskrav"
def _bundle_id() -> str:
from portfolio_optimiser import okf
return okf.reconcile_bundle_id(str(_BUNDLE_DIR)).id