Recover truncated tool calls, and never pass a leaked one off as an answer#219
Open
hacksics wants to merge 1 commit into
Open
Recover truncated tool calls, and never pass a leaked one off as an answer#219hacksics wants to merge 1 commit into
hacksics wants to merge 1 commit into
Conversation
…nswer Small local models drift off the tool-call format, especially with a large tool schema in play. Two failures followed from that, both of which ended the turn looking like the model had simply stopped mid-sentence. Salvage handled well-formed Qwen/Hermes XML but gave up entirely on a call cut off partway through. It now takes the function name plus every parameter that actually closed. A trailing unterminated `<parameter=…>` is dropped rather than guessed, so a half-written path or file body can never reach a tool; if that leaves a required argument missing, the call fails validation and the model gets a corrective tool error, which is the agent loop working. Worse, a call that never parsed at all was reported as `status: completed` — indistinguishable from the model deciding it was done, leaving the user with narration trailing off into stray closing tags. It now ends on the error path, so the GUI offers Retry. That is the right affordance here: the drift is probabilistic, not deterministic, so the same model usually succeeds on a second attempt. Detection ignores fenced and inline code, so a model *explaining* tool-call syntax is still a real answer, and requires that tools were offered at all. Reported against qwen3.5-9b on LM Studio, 2026-07-26. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two failures that both end a turn looking like the model simply stopped mid-sentence. Reported against
qwen3.5-9bon LM Studio, but the cause is model-agnostic: small local models drift off the tool-call format, especially with a large tool schema in play.Independent of #206 — this is a bug fix in the salvage path and the turn loop, and touches no provider descriptor. Either can land first.
1. Truncated calls are now recovered
_salvage_tool_calls_from_textalready handled well-formed Qwen/Hermes XML, but gave up entirely on a call cut off partway through — which is exactly what a small model produces when it runs out of room mid-call.It now takes the function name plus every parameter that actually closed. A trailing unterminated
<parameter=…>is dropped rather than guessed, so a half-written path or file body can never reach a tool:If dropping it leaves a required argument missing, the call fails validation and the model gets a corrective tool error — the agent loop working, and strictly better than the turn dying on the fragment. Unknown tool names are still filtered, so there are no new false positives.
2. A leaked call is no longer reported as a completed answer
This is the worse half. When salvage found nothing, the turn ended
status: completed— indistinguishable from the model deciding it was done. The user was left with narration trailing off into stray closing tags:It now ends on the error path, so the GUI offers Retry. That is the right affordance: the drift is probabilistic rather than deterministic, so retrying the same model usually succeeds. The message names the likely cause and suggests a larger model.
Detection strips fenced and inline code first — a model explaining tool-call syntax is giving a real answer and must not be turned into an error — and requires that tools were offered at all. Both directions are pinned by tests.
Testing
894 passed(+5: three intest_provider_router.pyfor the salvage and detector edges, two intest_engine.pyfor the turn-end behaviour in both directions).Also verified end-to-end against a live LM Studio: a real Developer-persona turn ran reasoning →
read_file→ answer →completed, with the new detector correctly silent on a legitimate answer.Worth being straight about what I could not do: I couldn't force the leak on demand. Capping
max_tokenstruncates during narration, before the XML starts, and LM Studio's parser handles that boundary cleanly — which is itself evidence that this is genuine drift rather than a reproducible boundary condition. So the leak path is covered by unit tests against captured output, not by a live reproduction.Unrelated observation
tests/test_automation.py::test_scheduler_runs_due_task_and_advancesis a pre-existing timing flake — it asserts on a 200 ms wall-clock window with a 50 ms tick, and misses occasionally under full-suite load. Passes in isolation every time. Not touched here.🤖 Generated with Claude Code