mirror of
https://github.com/pydantic/pydantic-ai-harness.git
synced 2026-07-21 02:45:34 +00:00
fix(overflow): read_tool_result returns on a missing handle instead of raising (#293)
* fix(overflow): read_tool_result returns on a missing handle instead of raising A wrong handle (e.g. a model passing a tool-call id) or a result no longer in the store made `read_tool_result` raise `ModelRetry`, which consumes a tool retry and, once the budget is spent, escalates to a fatal `UnexpectedModelBehavior` that kills the run. This took down a pydanty reproducer run (the model passed `functions.read_tool_result_44.1` as a handle; max retries was 1). `_read_slice` now returns a guiding message on a missing handle -- pointing the model at the exact handle string from the spill marker, or to re-run the original tool -- so a bad handle can never crash a run. The `offset`/`limit` input validations stay `ModelRetry` (deterministic caller errors the model corrects). 100% branch coverage retained. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(overflow): don't echo the store error in the missing-handle message A FileNotFoundError (and custom-store errors) can carry the resolved filesystem path or other backend detail; the model only needs the handle and recovery guidance. Drop ": {exc}" and assert the store path is not in the returned message. Addresses review feedback on #293. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
e6dfd4c303
commit
1b709d8e30
@@ -529,8 +529,17 @@ async def _read_slice(
|
||||
|
||||
try:
|
||||
data = await store.read(handle)
|
||||
except OSError as exc:
|
||||
raise ModelRetry(f'No stored tool result for handle {handle!r}: {exc}.') from exc
|
||||
except OSError:
|
||||
# Return, not raise: a wrong handle (e.g. the model passing a tool-call id) or a
|
||||
# result that is no longer stored must not consume a tool retry and escalate to a
|
||||
# fatal `UnexpectedModelBehavior`. Guide the model to a valid handle instead. The
|
||||
# exception is intentionally not echoed -- a store's error can carry the resolved
|
||||
# filesystem path or other backend detail the model has no need for.
|
||||
return (
|
||||
f'[No stored tool result for handle {handle!r}. Use the exact handle string from a '
|
||||
'"[Tool output too large ... stored to handle ...]" marker; if the result is no longer '
|
||||
'available, re-run the original tool.]'
|
||||
)
|
||||
|
||||
lines = data.decode('utf-8', errors='replace').splitlines()
|
||||
if pattern is not None:
|
||||
|
||||
@@ -697,9 +697,14 @@ class TestReadBack:
|
||||
assert len(out) < 60_000
|
||||
|
||||
async def test_read_slice_missing_handle(self, tmp_path: Path):
|
||||
# A missing/wrong handle returns a guiding message (it does NOT raise): a bad
|
||||
# handle must not consume a retry and escalate to a fatal UnexpectedModelBehavior.
|
||||
store = LocalFileStore(base_dir=tmp_path)
|
||||
with pytest.raises(ModelRetry, match='No stored tool result'):
|
||||
await _read_slice(store, 'missing/1.0', offset=0, limit=10, from_end=False, pattern=None)
|
||||
out = await _read_slice(store, 'missing/1.0', offset=0, limit=10, from_end=False, pattern=None)
|
||||
assert 'No stored tool result' in out
|
||||
assert 're-run the original tool' in out
|
||||
# The store's error (which can carry the resolved filesystem path) is not leaked.
|
||||
assert str(tmp_path) not in out
|
||||
|
||||
async def test_get_toolset_registers_read_tool(self, tmp_path: Path):
|
||||
store = LocalFileStore(base_dir=tmp_path)
|
||||
|
||||
Reference in New Issue
Block a user