Repository navigation
fix(core): refuse tool calls with unparseable JSON arguments instead of executing them [K-02] - #5
Merged
Conversation
…of executing them [K-02]
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.
Summary
A tool call whose argument JSON cannot be parsed is now refused instead of executed with empty
arguments, and the parse error is fed back to the model so it can re-issue the call. Repeating
broken JSON ends the turn instead of looping.
Why
core.py:1099-1112built each call's argument dict like this:_repair_tool_call_argumentsreturns its failure sentinel"{}"(utils/json_utils.py:96) when itcannot repair a payload, so the inner
json.loadssucceeds and the tool runs on{}— withevery parameter missing. A truncated payload is the worst case: repair closes the braces it was cut
before, so half-streamed arguments become valid-looking JSON.
Measured with a scratch probe against the real
_run_conversation_loop(mechanism emulated so theold path is callable, see Verification):
The user-visible symptom is a "tool bug" — a tool acting on nothing — while the real cause (broken
JSON from the model) stays invisible. There was also no truncation refusal: args cut by the output
limit or a dropped stream were indistinguishable from a parameterless call.
What changed
utils/json_utils.py— newparse_tool_call_arguments(raw_args, tool_name)returning(arguments, error):errorisNoneonly when the payload parsed (after the existing repairpasses). A payload that does not close its JSON object/array is reported as truncated before
repair runs (
tool_call_arguments_look_truncated); non-object payloads are rejected too. Emptypayloads stay a legal parameterless call.
core.py— the loop calls the new parser. If any call in a round has unparseable arguments,nothing in that round executes: the invalid call gets a tool result carrying the reason, and
its siblings get an explicit "Skipped" result, so role/tool-result pairing stays intact. The model
then gets another round to re-issue the call, bounded at 3 consecutive rounds; past that the turn
ends with a visible notice instead of looping to
MAX_ROUNDS.core.py— removed the now-unusedjson as _jsonlocal import and the two staleutils.json_utilsimports, replacing them with the one symbol actually used.
Provenance
Ported (mechanism and edge cases, re-implemented in koza's style) from
hermes:agent/turn_tool_validation.py:148-223— invalid/truncated argument validation, thetruncation refusal, and recovery results used to re-prompt the model.
Verification
No test that was red on
mainbecame red here; the 10 new tests intests/test_tool_call_arguments.py(force-added —tests/is gitignored) cover: empty/repairable/truncated/non-object payloads, a truncated call never reaching the tool, a valid sibling being
skipped in a broken round, the bounded stop after repeated broken JSON, and repair still executing
normally. Four of them drive the real
_run_conversation_loopwith a stubbed streaming provider.Live probe (
_run_conversation_loop, truncated payload{"command": "rm -rf /tmp):Risk / rollback
Contained to the tool-call argument path in
core.pyand two new pure functions inutils/json_utils.py; no dependency, config or prompt change. Behavioural change is intentional:a broken-JSON call no longer executes — it is reported to the model and retried, then the turn stops.
Rollback: revert this commit;
_repair_tool_call_argumentsis untouched and still exported.Roadmap
Roadmap: K-02