fix(tools): read_file paa en KATALOG nektes ved navn og peker paa read_dir
Funn (c) fra oekt 94s levende K2-maaling (docs/2026-09-06-major2-levende-k2.md
§ 3/§ 4): en levende modell gikk stigen list_bundles -> read_bundle -> read_dir
-> read_file, naadde et nivaa med underkataloger (30-1, 30-7, 521-001 ...) og
kalte read_file paa en av dem. Tre slike kall i EN kjoering.
SJEKKET FOERST, som ordren ber om: ordren aapner for to aarsaker. Det er den
ANDRE. En listing SKILLER allerede de to slagene strukturelt - hver
directory_listing-payload svarer med "directories" (oppfoeringer noeklet "path",
med subtre-telling) og "documents" (noeklet "name", med type/title/chars) som TO
distinkte noekler, og en katalog opptrer aldri blant dokumentene. Arm (4) pinner
det, fordi det er en egenskap denne fila naa HVILER paa. Derfor ingen
trailing-/-markoer: formen sier allerede hva som er hva, og aa endre payloaden
ville flyttet en listing tre eldre gates maaler byte for byte.
Det som MANGLET var den andre halvdelen. MAALT foer arbeidet, paa den shippede
nestede eksempelbasen:
read_file(id, "a") -> IsADirectoryError: [Errno 21] Is a directory: <abs sti>
En OSError, altsaa krasj-kanalen i stedet for CLI-ens nekt-tuppel og hostings
400-arm, og den navnga verken hva stien VAR eller hvilken sprosse kalleren
skulle brukt. DirectoryPathRefused (ValueError, BundlePathNotFound- og
DimensionScopeRefused-presedensen) navngir begge. Regelen bor i VERKTOEYET, saa
den gjelder begge kallere (utforskningen, og siden S2c debatten) - samme
plassering som verdict-gaten, og FOER den: declares_verdict_type leser stiens
frontmatter, saa paa en katalog ville den reist noeyaktig den OSError-en denne
grenen finnes for aa erstatte.
MAALT, RAPPORTERT, IKKE FIKSET (utenfor ordren): det levende kallet var
read_file(".../30-7.md") - modellen la .md paa et katalogNAVN, som resolverer
til en sti som ikke finnes i det hele tatt, ikke til katalogen. Maalt paa samme
base gir den FileNotFoundError, altsaa samme form (OSError paa krasj-kanalen der
en navngitt nekt hoerer hjemme). Denne ordren fikser katalog-tilfellet; arm (5)
pinner maalingen av naboen, saa gapet er et faktum i suiten og ikke en setning i
en rapport.
RoedT foerst: import-feil paa DirectoryPathRefused (klassen fantes ikke).
Ingen endring i prompter.
Suite 1381 passed / 5 skipped (fra 1368/5; +13, 0 fjernet - strengt supersett).
ruff + mypy rene. Golden demo-transcript.stdout BYTE-UENDRET,
shasum -a 1 av INNHOLDET = ea8c534773acdbe41ae68f2c55724d69aaf8be4f.
Ordre 20260906T212735Z-2448754-from-.claude, funn (c).
Co-Authored-By: Claude <claude-opus-5>
This commit is contained in:
parent
c6886ed605
commit
ab747bc7e8
2 changed files with 165 additions and 0 deletions
|
|
@ -242,6 +242,22 @@ class DimensionScopeRefused(ValueError):
|
|||
"""
|
||||
|
||||
|
||||
class DirectoryPathRefused(ValueError):
|
||||
"""``read_file`` was asked for a DIRECTORY — the wrong rung of the navigation ladder.
|
||||
|
||||
Measured against a live model on K2 (``docs/2026-09-06-major2-levende-k2.md`` § 3/§ 4, finding
|
||||
(c)): a listing hands back subdirectories and concept documents as two distinct keys, and the
|
||||
model asked ``read_file`` for one of the directories anyway, three times in one run. Before
|
||||
this the answer was ``IsADirectoryError`` — an ``OSError``, so it left by the crash channel,
|
||||
and it named neither what the path was nor which tool reads it.
|
||||
|
||||
A ``ValueError``, the ``BundlePathNotFound``/``DimensionScopeRefused`` precedent: the caller is
|
||||
a model choosing a path, so a refusal belongs on the CLI's refusal tuple and hosting's 400 arm.
|
||||
The message names ``read_dir`` because a refusal that only says "no" leaves the caller with the
|
||||
same next move it just made.
|
||||
"""
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class LedgerEntry:
|
||||
"""One progress-ledger round, reduced to the five fields the manager steers on (C.2).
|
||||
|
|
@ -1030,6 +1046,15 @@ def navigator_tools(
|
|||
# model-chosen path is untrusted input by definition, so it goes through the same gate the
|
||||
# navigation walk uses rather than a second, laxer check.
|
||||
resolved = Path(safe_resolve(bundle_dir, path))
|
||||
# The wrong RUNG, answered as such (finding (c) of the live K2 run). This is BEFORE the
|
||||
# verdict gate for a mechanical reason as well as a readable one: ``declares_verdict_type``
|
||||
# reads the path's frontmatter, so on a directory it would raise ``IsADirectoryError``
|
||||
# first and the caller would get the OSError this branch exists to replace.
|
||||
if resolved.is_dir():
|
||||
raise DirectoryPathRefused(
|
||||
f"{path!r} in knowledge base {bundle_id!r} is a directory, not a document; "
|
||||
"use read_dir to list what it holds, then read_file on one of the names it gives"
|
||||
)
|
||||
# The verdict layer, refused HOWEVER the path was found (order 20260904T172353Z). No
|
||||
# listing names it — ``context_files`` drops it at every level, so ``read_bundle`` and
|
||||
# ``read_dir`` never mention one — but a GUESSED path reached it, and reaching it that way
|
||||
|
|
|
|||
140
tests/test_read_file_directory_refusal_loadbearing.py
Normal file
140
tests/test_read_file_directory_refusal_loadbearing.py
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
"""``read_file`` on a DIRECTORY is refused by name and pointed at ``read_dir``.
|
||||
|
||||
Finding (c) of the live K2 session (``docs/2026-09-06-major2-levende-k2.md`` § 3/§ 4): a live model
|
||||
walking the ladder ``list_bundles -> read_bundle -> read_dir -> read_file`` reached a level holding
|
||||
subdirectories (``30-1``, ``30-7``, ``521-001`` ...) and then called ``read_file`` on one of them.
|
||||
Three such calls in one run.
|
||||
|
||||
**What was CHECKED FIRST, and what it found.** The order allows for two causes and asks which one
|
||||
holds. It is the second. A listing DOES already separate the two kinds structurally: every
|
||||
``directory_listing`` payload answers with ``directories`` (entries keyed ``path``, carrying a
|
||||
subtree ``documents`` count) and ``documents`` (entries keyed ``name``, carrying ``type``, ``title``
|
||||
and ``chars``) as two distinct keys, and no directory ever appears among the documents. Arm (4)
|
||||
pins that, because it is a property this file now depends on rather than one it introduces. So no
|
||||
trailing-``/`` marker is added: the shape already says which is which, and changing the payload
|
||||
would move a listing three older gates measure byte for byte.
|
||||
|
||||
What was missing is the OTHER half. MEASURED before this work, on the shipped nested example base:
|
||||
|
||||
read_file(id, "a") -> builtins.IsADirectoryError: [Errno 21] Is a directory: '<abs path>'
|
||||
|
||||
An ``OSError``, so it lands on the crash channel rather than the CLI's refusal tuple and hosting's
|
||||
400 arm, and it says nothing about which rung the caller should have used. The refusal now names
|
||||
both -- that the path is a directory, and ``read_dir`` -- following ``VerdictLayerRefused`` and
|
||||
``DimensionScopeRefused``: the rule lives in the TOOL, so it holds for both callers (the
|
||||
exploration and, since S2c, the debate), and it is a ``ValueError`` because the caller is a model
|
||||
choosing a path, not a program that is broken.
|
||||
|
||||
**MEASURED, REPORTED, NOT FIXED (outside this order).** The live call was
|
||||
``read_file(".../30-7.md")`` -- the model appended ``.md`` to a directory NAME, which resolves to a
|
||||
path that does not exist at all, not to the directory. Measured on the same base:
|
||||
|
||||
read_file(id, "a.md") -> builtins.FileNotFoundError: [Errno 2] No such file or directory
|
||||
|
||||
That is a second, adjacent defect with the same shape (an OSError on the crash channel where a
|
||||
named refusal belongs), and the fix ordered here does not cover it. Arm (5) pins the measurement so
|
||||
the gap is a fact in the suite rather than a sentence in a report.
|
||||
|
||||
Detach point: remove the directory branch from ``read_file`` -> RED on (1), (2) and (3).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from portfolio_optimiser.explore import DirectoryPathRefused, navigator_tools
|
||||
|
||||
_EXAMPLES = Path(__file__).resolve().parents[1] / "shared" / "examples"
|
||||
#: The only NESTED base shipped -- the one that HAS a directory to ask for.
|
||||
_NESTED = _EXAMPLES / "nav-golden-hierarchy" / "bundle"
|
||||
|
||||
|
||||
def _tools(bundle_dir: Path) -> dict[str, Any]:
|
||||
return {t.name: t for t in navigator_tools((str(bundle_dir),))}
|
||||
|
||||
|
||||
async def _invoke(tool: Any, **arguments: Any) -> str:
|
||||
"""A tool's answer as text (S2c's helper: ``invoke`` returns ``[Content]``, and a test that
|
||||
stringified the list would compare object reprs and pass against anything)."""
|
||||
return "".join(getattr(c, "text", "") or "" for c in await tool.invoke(arguments=arguments))
|
||||
|
||||
|
||||
def _a_directory(bundle_dir: Path) -> str:
|
||||
"""A directory path taken out of the listing itself -- the same way a model gets one."""
|
||||
listing = _tools(bundle_dir)["read_bundle"].func(bundle_id=bundle_dir.name)
|
||||
assert listing["directories"], "fixture has no subdirectory to ask for"
|
||||
return str(listing["directories"][0]["path"])
|
||||
|
||||
|
||||
def test_a_directory_path_is_refused_and_the_other_rung_is_named() -> None:
|
||||
"""(1) the refusal says WHAT the path is and WHICH tool reads it."""
|
||||
path = _a_directory(_NESTED)
|
||||
with pytest.raises(DirectoryPathRefused) as excinfo:
|
||||
_tools(_NESTED)["read_file"].func(bundle_id=_NESTED.name, path=path)
|
||||
message = str(excinfo.value)
|
||||
assert path in message
|
||||
assert "directory" in message
|
||||
assert "read_dir" in message, "a refusal that does not name the other rung strands the caller"
|
||||
|
||||
|
||||
def test_the_refusal_is_a_value_error_not_an_os_error() -> None:
|
||||
"""(2) the channel, which is the half a message alone cannot carry.
|
||||
|
||||
``ValueError`` is the ``BundlePathNotFound``/``DimensionScopeRefused`` precedent -- the CLI's
|
||||
refusal tuple and hosting's 400 arm. The ``OSError`` assert is what tells a REPLACEMENT from a
|
||||
wrapper: re-raising ``IsADirectoryError`` with a better message would pass arm (1).
|
||||
"""
|
||||
path = _a_directory(_NESTED)
|
||||
with pytest.raises(ValueError) as excinfo:
|
||||
_tools(_NESTED)["read_file"].func(bundle_id=_NESTED.name, path=path)
|
||||
assert not isinstance(excinfo.value, OSError)
|
||||
|
||||
|
||||
async def test_the_refusal_holds_through_the_real_tool_surface() -> None:
|
||||
"""(3) reached through ``invoke``, not only through ``.func``.
|
||||
|
||||
The seam a model actually calls is the tool, and a check that only ran on the plain function
|
||||
would be absent from the one caller it was written for.
|
||||
"""
|
||||
path = _a_directory(_NESTED)
|
||||
with pytest.raises(DirectoryPathRefused):
|
||||
await _invoke(_tools(_NESTED)["read_file"], bundle_id=_NESTED.name, path=path)
|
||||
|
||||
|
||||
def test_a_listing_already_separates_directories_from_documents() -> None:
|
||||
"""(4) the "check first" arm: the payload's shape is what distinguishes the two kinds.
|
||||
|
||||
Also the anti-vacuity control for this file -- it proves the fixture really does hold both
|
||||
kinds, so arm (1) is asking for a directory rather than for nothing.
|
||||
"""
|
||||
listing = _tools(_NESTED)["read_bundle"].func(bundle_id=_NESTED.name)
|
||||
directories = {d["path"] for d in listing["directories"]}
|
||||
documents = {d["name"] for d in listing["documents"]}
|
||||
assert directories and documents, "fixture must hold both kinds for this to mean anything"
|
||||
assert directories.isdisjoint(documents)
|
||||
assert all(set(d) == {"path", "documents"} for d in listing["directories"])
|
||||
assert all(set(d) == {"name", "type", "title", "chars"} for d in listing["documents"])
|
||||
|
||||
|
||||
def test_a_document_is_still_returned_whole() -> None:
|
||||
"""CONTROL: the branch is a gate, not a wall -- the rung it guards still works."""
|
||||
listing = _tools(_NESTED)["read_bundle"].func(bundle_id=_NESTED.name)
|
||||
name = listing["documents"][0]["name"]
|
||||
body = _tools(_NESTED)["read_file"].func(bundle_id=_NESTED.name, path=name)
|
||||
assert body.strip(), "reading a real document must be untouched by the directory branch"
|
||||
|
||||
|
||||
def test_a_nonexistent_sibling_is_still_an_os_error() -> None:
|
||||
"""(5) MEASURED, REPORTED, NOT FIXED: the live call's ACTUAL shape.
|
||||
|
||||
The model appended ``.md`` to a directory name, which is not a directory -- it is a path that
|
||||
does not exist. This order fixes the directory case; this arm records that the adjacent case is
|
||||
unchanged, so the gap cannot be mistaken for covered. Written as an assertion on the CURRENT
|
||||
behaviour: when it goes red, someone has closed it, and that is a decision to be recorded.
|
||||
"""
|
||||
path = _a_directory(_NESTED) + ".md"
|
||||
with pytest.raises(FileNotFoundError):
|
||||
_tools(_NESTED)["read_file"].func(bundle_id=_NESTED.name, path=path)
|
||||
Loading…
Add table
Add a link
Reference in a new issue