fix(agent): close unclosed JSON tool-call args in LIFO order - #77395
fix(agent): close unclosed JSON tool-call args in LIFO order#77395swissly wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness bug in Hermes’ tool-call argument sanitization pipeline: when repairing truncated JSON, the previous “count openers then append closers” approach could generate invalid JSON for nested structures, causing _repair_tool_call_arguments to fall back to {} and silently discard the model’s intended tool arguments.
Changes:
- Update
_repair_tool_call_argumentsto close unclosed{/[structures using a delimiter stack and append missing closers in LIFO order. - Add regression tests covering single-delimiter truncation, nested truncation (the LIFO bug), and balanced JSON passthrough.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
agent/message_sanitization.py |
Fixes JSON truncation repair by closing missing delimiters in LIFO order to preserve nested validity. |
tests/run_agent/test_repair_tool_call_arguments.py |
Adds regression tests to prevent reintroducing invalid closer ordering and to validate nested repair behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The LIFO stack still treats |
|
Thanks for the sharp catch — confirmed and fixed. The string-blind stack treated Fix (commit 4a9d49f883): the delimiter scan now tracks quoted-string state and backslash escapes — Regression tests added (4):
Verified both core cases fail on the string-blind code and pass with the fix; full file 12/12 green. |
_repair_tool_call_arguments closed unclosed structures by counting
openers: it appended all '}' then all ']'. For nested input like
'{"a": [1,2' this produced '{"a": [1,2}]' — invalid JSON, which then
fell through to the '{}' last resort, silently discarding the model's
arguments.
Track the opening stack and append closers in LIFO order (last opened,
first closed) so nested structures repair to valid JSON:
'{"a": [1,2' -> '{"a": [1,2]}'.
Found by behavioral testing on the tool-call repair pipeline
(2026-08-03). Applies the same class of fix to all nesting depths;
balanced JSON is untouched.
The LIFO stack treated {/[ inside quoted string values as structural
delimiters. '{"a":"[","b":[1,2' pushed ']' for the literal '[',
leaving the string unterminated and the repair falling back to '{}' —
silently discarding the model's arguments.
The scan now tracks quoted-string state and backslash escapes: delimiters
inside strings are literal characters, not structure. Adds 4 regression
tests (bracket-in-string, brace-in-string, escaped-quote, delimiters
after closed string); the two core cases fail on the string-blind code.
4a9d49f to
1e61db1
Compare
…62640) Ports the tool-call repair observability layer onto current main as a fresh, scoped PR. Supersedes NousResearch#62640 (5434 commits stale, never merged). - agent/tool_repair_stats.py: thread-safe ring-buffer singleton, per-model and per-pattern counts, 21 tests. Final review version (no dead set_current_model). - Instrumentation in the 3 repair paths: message_sanitization (_stat), agent_runtime_helpers (truncated_args), model_tools (bare-string/object wrap). Lazy-imported + defensive no-op so the module can be absent. - Operator output surface: new 'hermes repair-stats' CLI command wires summary() to a real call site (fixes Teknium finding NousResearch#4 — summary was dead code in the original PR). - 21 new tests + existing sanitize/coerce regression pass. Steps: Step 0 overlap check done — NousResearch#77395 (LIFO close) already merged upstream (functional part), NousResearch#34132/NousResearch#68612 are the repair logic itself not observability. This is the only stats/observability PR.
|
Ping @teknium1 — this PR is mergeable/clean and ready for review (all review threads addressed, CI green). Open since 2026-08-03. |
Summary
_repair_tool_call_argumentsinagent/message_sanitization.pyclosed unclosed JSON structures by counting openers: it appended all}then all]. For nested input like{"a": [1,2this produced{"a": [1,2}]— invalid JSON — which then fell through to the{}last resort, silently discarding the model's arguments.Root cause
Counting is order-insensitive. With
{"a": [1,2it appends}first, producing{"a": [1,2}], thenjson.loadsfails → UNREPAIRABLE →{}.Fix
Track the opening stack and append closers in LIFO order (last opened, first closed), so nested structures repair to valid JSON:
{"a": [1,2→{"a": [1,2]}(was{}){"a": [1, {"b": 2→{"a": [1, {"b": 2}]}Tests
Added 5 regression tests to
tests/run_agent/test_repair_tool_call_arguments.py(single brace, single bracket, nested LIFO, nested object-in-array, balanced passthrough). Verified the LIFO regression test fails on the buggy code and passes with the fix.Validation
pytest tests/run_agent/test_repair_tool_call_arguments.py→ 8 passed{}last resort).