feat(prepass,explore): padding dies at the prompt; a refusal the model can act on is a return value [skip-docs]
Order 20260908T195801Z. Findings 4 and 5 from the S7 acid test, then the two things finding 99 measured and deliberately did not fix (D3, D2). No user-facing surface changes: no new flag, no new command, no changed output contract. Both seams are internal (the pre-pass rendering, and the shape a tool answers a model with), so [skip-docs] rather than a README edit that would describe nothing an operator can do differently. FINDING 4 -- MEASURED, NOTHING BUILT. K2's price schedule IS readable without guessing (8 column spans, 71 of 91 non-blank rows give >= 2 cells, the split stable for K = 2..64). But 0 of 92 rows name all three of code/quantity/unit_cost -- also under a looser substring match -- and 0 of 91 data rows carry code + quantity + amount. The triple is not formatted away; it is not in the document. It is a price SUMMARY plus nine rate cards whose unit-price columns are empty (pre-award). The order's binding decision rule therefore falls against building: --derive-cost-baseline keeps refusing, and MAJOR-4's own honesty limit holds. FINDING 5 -- BUILT. Measured on the actual rendering path (concept_text, not the raw file): the delivered excerpt is 104 lines / 67 245 chars, carrying 208 interior whitespace runs, 117 of them >= 100 and the longest 887 -- 56 806 of 67 245 characters = 84.5 %, over 72 of 104 lines. collapse_padding, called from _data_blocks (the one renderer both arms share, and therefore AFTER verify_against_bundle -- collapsing in concept_text would break every payload's own digest), gives -72.4 %: line count invariant, non-whitespace byte-identical, leading indentation untouched, no number changed. F99-D3 -- read_file / read_dir / read_bundle now RETURN their refusal. MAF turns a tool raise into "Error: Function failed." (_tools.py:1410-1432, :1427) and counts it against DEFAULT_MAX_CONSECUTIVE_ERRORS_PER_REQUEST = 3, so everything the refusing arm knows is destroyed on the way out. The gates are unchanged; the property they exist for -- the reason travels, the bytes never do -- is now asserted explicitly on the returned value. The arm is keyed on named classes, never bare Exception, because ExplorationError is itself a RuntimeError subclass. F99-D2 -- the invariant row, plus one for finding 5 (a stated deviation from "one row only": finding 5 is a separately built seam and the ledger's standing rule requires its own row). 19 existing arms rewritten, never deleted and never weakened: where the class carried a distinction, the refusal KIND carries it now. 13 mutations, all red against the whole suite (W1-W5, M1-M8), each restored from scratchpad with shasum -c. Control 1543 passed / 5 skipped (from 1529/5, a strict superset, 0 removed). Golden demo-transcript.stdout unchanged (shasum -a 1 of the CONTENT = ea8c534773acdbe41ae68f2c55724d69aaf8be4f). Measurement: docs/2026-09-08-funn-4-5-og-read-nekt.md Co-Authored-By: Claude <Opus 5>
This commit is contained in:
parent
078a099898
commit
eb4137415c
13 changed files with 975 additions and 69 deletions
184
tests/test_explore_read_tools_refusal.py
Normal file
184
tests/test_explore_read_tools_refusal.py
Normal file
|
|
@ -0,0 +1,184 @@
|
|||
"""``read_file`` / ``read_dir`` / ``read_bundle`` RETURN their refusal; they no longer raise.
|
||||
|
||||
**F99-D3.** Finding 99 closed exactly one half of this and said so
|
||||
(``docs/2026-09-08-funn-99-chatclient-refusal.md`` lines 132-135): ``quick_validate`` was made to
|
||||
return ``{"decision": "refused", ...}``, while "``read_file`` / ``read_dir`` / ``read_bundle``
|
||||
raiser fortsatt, og deres raises telles på nøyaktig samme måte ... **MÅLT, RAPPORTERT, IKKE
|
||||
FIKSET.**" The trio measured immediately before the live 400 was three ``read_file`` refusals on
|
||||
``del-ii-bilag-7-prisskjema*`` -- the same document findings 4 and 5 of this order are about.
|
||||
|
||||
**Why a raise is worse than useless here, MEASURED in the framework's own source.** MAF turns a
|
||||
tool's exception into the opaque string ``"Error: Function failed."``
|
||||
(``agent_framework/_tools.py:1410-1432``), suppressing the detail unless ``include_detailed_errors``
|
||||
is set (``:1427``), and counts it against ``DEFAULT_MAX_CONSECUTIVE_ERRORS_PER_REQUEST = 3``
|
||||
(``:96``, ``:2718-2731``). So everything the refusing arm KNOWS -- which base ids exist, which
|
||||
document the path really names, which rung reads it -- is destroyed on the way out, and three of
|
||||
them end the request. The model is left guessing, which is what finding 99 recorded it doing in its
|
||||
own words.
|
||||
|
||||
**The gates themselves are UNCHANGED.** This is the shape of the answer, never its content: the
|
||||
verdict layer and the §4.1a dimension scope refuse exactly what they refused before, and the
|
||||
property they exist for -- **the bytes never reach the model** -- is asserted explicitly on the
|
||||
returned value, not assumed from the fact that an exception used to be raised.
|
||||
|
||||
**Form per tool, and the difference is forced by the return type.** ``read_bundle`` and ``read_dir``
|
||||
return ``dict``, so the refusal is a mapping with no key a successful listing has. ``read_file``
|
||||
returns ``str``: making it a mapping would change the shape of every SUCCESSFUL read and therefore
|
||||
every prompt byte S2c measured, so the refusal is a string with a fixed leading sentinel. A
|
||||
``str`` refusal a caller cannot tell from a document would be worthless, and the honesty limit --
|
||||
a document whose first characters happen to be that sentinel -- is stated in the docstring rather
|
||||
than pretended away.
|
||||
|
||||
**Keyed on the class, never on bare ``Exception``.** The known-negative below raises a plain
|
||||
``RuntimeError`` from inside a tool body and requires it to propagate: we are not in the business of
|
||||
hiding failures we did not name, and ``ExplorationError`` being itself a ``RuntimeError`` subclass
|
||||
is exactly why the arm has to be a list of names rather than a base class.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from portfolio_optimiser import okf
|
||||
from portfolio_optimiser.explore import navigator_tools
|
||||
|
||||
BUNDLE_DIR = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
|
||||
_VERDICT_FILE = "verdict-led-fro.md"
|
||||
_CONCEPT_FILE = "metode-ipmvp-a.md"
|
||||
_SENTINEL = "READ-TOOL-LEAK-SENTINEL-z9y8x7"
|
||||
|
||||
|
||||
def _tools(bundle_dir: str | Path, dimension: str | None = None) -> dict[str, Any]:
|
||||
return {t.name: t for t in navigator_tools([str(bundle_dir)], dimension=dimension)}
|
||||
|
||||
|
||||
def _marked_copy(tmp_path: Path, target: str) -> Path:
|
||||
"""A copy of the fixture base with ``_SENTINEL`` appended to one document's body, so a leak of
|
||||
that document's CONTENT into a refusal is detectable by a string that exists nowhere else."""
|
||||
copy = tmp_path / "bundle"
|
||||
shutil.copytree(BUNDLE_DIR, copy)
|
||||
doc = copy / target
|
||||
doc.write_text(doc.read_text(encoding="utf-8") + f"\n{_SENTINEL}\n", encoding="utf-8")
|
||||
return copy
|
||||
|
||||
|
||||
# --- (1) the unknown base id: the trio measured in front of the live 400 ------------------------
|
||||
|
||||
|
||||
def test_an_unknown_base_is_returned_as_a_refusal_by_all_three_tools() -> None:
|
||||
"""(1) The measured case. All three name the configured ids, so the model can correct itself
|
||||
instead of receiving ``"Error: Function failed."`` three times and ending the request."""
|
||||
tools = _tools(BUNDLE_DIR)
|
||||
guessed = "renholdstekniske_funksjonskrav"
|
||||
|
||||
listing = tools["read_bundle"].func(bundle_id=guessed)
|
||||
assert isinstance(listing, dict) and "refused" in listing
|
||||
assert BUNDLE_DIR.name in listing["refused"]
|
||||
|
||||
directory = tools["read_dir"].func(bundle_id=guessed, path="x")
|
||||
assert isinstance(directory, dict) and "refused" in directory
|
||||
assert BUNDLE_DIR.name in directory["refused"]
|
||||
|
||||
document = tools["read_file"].func(bundle_id=guessed, path="index.md")
|
||||
assert isinstance(document, str) and document.startswith("REFUSED")
|
||||
assert BUNDLE_DIR.name in document
|
||||
|
||||
|
||||
# --- (2) the verdict layer: the reason travels, the bytes never do ------------------------------
|
||||
|
||||
|
||||
def test_the_verdict_layer_refusal_carries_the_reason_and_never_the_content(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""(2) The gate is unchanged; only the shape of the answer is. The assertion the order requires
|
||||
is the second one: a returned refusal must not become a channel for the bytes the gate exists
|
||||
to withhold."""
|
||||
base = _marked_copy(tmp_path, _VERDICT_FILE)
|
||||
answer = _tools(base)["read_file"].func(bundle_id=base.name, path=_VERDICT_FILE)
|
||||
assert isinstance(answer, str) and answer.startswith("REFUSED")
|
||||
assert "verdict" in answer.lower()
|
||||
assert _SENTINEL not in answer, "the refusal leaked the document it refused"
|
||||
|
||||
|
||||
def test_an_ordinary_document_in_the_same_base_still_reads_whole(tmp_path: Path) -> None:
|
||||
"""(2) CONTROL. Without this, arm (2) is satisfied by a tool that refuses everything."""
|
||||
base = _marked_copy(tmp_path, _CONCEPT_FILE)
|
||||
body = _tools(base)["read_file"].func(bundle_id=base.name, path=_CONCEPT_FILE)
|
||||
assert _SENTINEL in body
|
||||
assert not body.startswith("REFUSED")
|
||||
|
||||
|
||||
# --- (3) the §4.1a dimension scope --------------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_dimension_refusal_carries_the_reason_and_never_the_content(tmp_path: Path) -> None:
|
||||
"""(3) The other gate this change touches. Same two halves: the reason travels, the body does
|
||||
not."""
|
||||
base = _marked_copy(tmp_path, _CONCEPT_FILE)
|
||||
foreign = base / "fremmed-dimensjon.md"
|
||||
foreign.write_text(
|
||||
f"---\ntype: methodology\ntitle: Asfalt\ndimension: asfalt\n---\n\n{_SENTINEL}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
index = base / "index.md"
|
||||
index.write_text(
|
||||
index.read_text(encoding="utf-8") + "\n- [Asfalt](fremmed-dimensjon.md)\n", encoding="utf-8"
|
||||
)
|
||||
answer = _tools(base, dimension="tunnel")["read_file"].func(
|
||||
bundle_id=base.name, path="fremmed-dimensjon.md"
|
||||
)
|
||||
assert isinstance(answer, str) and answer.startswith("REFUSED")
|
||||
assert "tunnel" in answer
|
||||
assert _SENTINEL not in answer, "the refusal leaked the out-of-scope document"
|
||||
|
||||
|
||||
# --- (4) the wrong rung, both directions --------------------------------------------------------
|
||||
|
||||
|
||||
def test_the_wrong_rung_is_returned_by_both_tools() -> None:
|
||||
"""(4) F3's asymmetry, preserved through the change: each refusal still names the rung that
|
||||
WOULD read the thing, and the two branches still share no wording."""
|
||||
tools = _tools(BUNDLE_DIR)
|
||||
as_document = tools["read_dir"].func(bundle_id=BUNDLE_DIR.name, path=_CONCEPT_FILE)
|
||||
assert "refused" in as_document and "read_file" in as_document["refused"]
|
||||
|
||||
unknown = tools["read_dir"].func(bundle_id=BUNDLE_DIR.name, path="kategori-99")
|
||||
assert "refused" in unknown and "read_file" not in unknown["refused"]
|
||||
assert unknown["refusal"] != as_document["refusal"], (
|
||||
"a wrong rung and a path that does not exist are different facts; collapsing them into one "
|
||||
"kind leaves a caller unable to tell 'use the other tool' from 'this does not exist'"
|
||||
)
|
||||
|
||||
|
||||
# --- (5) KNOWN-NEGATIVE: an unnamed failure still propagates ------------------------------------
|
||||
|
||||
|
||||
def test_a_plain_runtime_error_still_propagates(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""(5) KNOWN-NEGATIVE, and the reason the arm is a list of NAMES. ``ExplorationError`` is itself
|
||||
a ``RuntimeError`` subclass, so an arm written as ``except RuntimeError`` would swallow this
|
||||
one too -- turning an unknown failure into a confident-looking answer, which is worse than the
|
||||
opaque string this change removes."""
|
||||
tools = _tools(BUNDLE_DIR)
|
||||
|
||||
def boom(*args: Any, **kwargs: Any) -> Any:
|
||||
raise RuntimeError("something we never named")
|
||||
|
||||
monkeypatch.setattr(okf, "navigate_bundle", boom)
|
||||
with pytest.raises(RuntimeError, match="something we never named"):
|
||||
tools["read_bundle"].func(bundle_id=BUNDLE_DIR.name)
|
||||
with pytest.raises(RuntimeError, match="something we never named"):
|
||||
tools["read_dir"].func(bundle_id=BUNDLE_DIR.name, path="x")
|
||||
|
||||
|
||||
# --- (6) a successful answer can never be mistaken for a refusal --------------------------------
|
||||
|
||||
|
||||
def test_a_successful_listing_carries_no_refusal_key() -> None:
|
||||
"""(6) The other half of "a refusal a caller cannot tell from an answer is worthless"."""
|
||||
listing = _tools(BUNDLE_DIR)["read_bundle"].func(bundle_id=BUNDLE_DIR.name)
|
||||
assert "refused" not in listing and "refusal" not in listing
|
||||
assert listing["documents"]
|
||||
Loading…
Add table
Add a link
Reference in a new issue