fix: repair misnested tool-call closers by inserting the missing one before the misplaced one
Balanced-but-misnested argument JSON ({"a": [{"b": 1}, {"c": 2}}]}, the reporter's
deepseek-v4-flash shape where the "]" of an array of objects is dropped and the
neighbouring "}" closes in its place) has equal delimiter counts, so the closer
arithmetic had nothing to append and the call degraded to "{}" — silently losing
the tool call. The string-aware scan from #115096 now rewrites the text: a closer
that matches a deeper opener gets the missing inner closers inserted in front of
it, and the remaining stack is appended in order. Anything ending inside a string
still returns "{}" (unrecoverable content is never guessed).
Tests trimmed to invariants: string-aware balance and stack-order close (from
#115096), the five reporter shapes, and the streaming assembler seam
(_StreamingCall._assemble_tool_calls) repairing instead of flagging truncation,
with the mid-string-cut control still flagged.
Part of #115061 (JSON-repair half; the plugin button-callback half is a
separate design change the reporter agreed to file standalone).
This commit is contained in:
@@ -159,12 +159,19 @@ def _loads_ok(text: str) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _scan_json_stack(raw: str) -> list[str] | None:
|
||||
"""Open brace/bracket stack of a JSON prefix, ignoring delimiters inside string
|
||||
values (``{"code": "}"}`` keeps one open brace, not a balanced document). ``None``
|
||||
when the text ends inside an unterminated string — the caller must close the open
|
||||
quote before closing any brackets.
|
||||
_JSON_CLOSERS = {"{": "}", "[": "]"}
|
||||
|
||||
|
||||
def _rebalance_json_closers(raw: str) -> str | None:
|
||||
"""Close a JSON prefix's open braces/brackets in stack order, ignoring delimiters
|
||||
inside string values (``{"code": "}"}`` keeps one open brace, not a balanced
|
||||
document). A closer that does not match the stack top but does match a deeper opener
|
||||
gets the missing inner closers inserted BEFORE it: ``{"a": [{"b": 1}}`` → the model
|
||||
dropped the ``]`` and let the neighbouring ``}`` close in its place, so the counts
|
||||
balance and nothing can be appended. ``None`` when the text ends inside an
|
||||
unterminated string — that content is unrecoverable and must not be guessed.
|
||||
"""
|
||||
out: list[str] = []
|
||||
stack: list[str] = []
|
||||
in_string = False
|
||||
i, n = 0, len(raw)
|
||||
@@ -172,20 +179,24 @@ def _scan_json_stack(raw: str) -> list[str] | None:
|
||||
ch = raw[i]
|
||||
if in_string:
|
||||
if ch == "\\":
|
||||
out.append(raw[i:i + 2])
|
||||
i += 2
|
||||
continue
|
||||
if ch == '"':
|
||||
in_string = False
|
||||
elif ch == '"':
|
||||
in_string = True
|
||||
elif ch in "{[":
|
||||
elif ch in _JSON_CLOSERS:
|
||||
stack.append(ch)
|
||||
elif ch in "}]":
|
||||
expected = "{" if ch == "}" else "["
|
||||
if stack and stack[-1] == expected:
|
||||
stack.pop()
|
||||
elif ch in "}]" and ch in (_JSON_CLOSERS[o] for o in stack):
|
||||
while _JSON_CLOSERS[stack[-1]] != ch:
|
||||
out.append(_JSON_CLOSERS[stack.pop()])
|
||||
stack.pop()
|
||||
out.append(ch)
|
||||
i += 1
|
||||
return None if in_string else stack
|
||||
if in_string:
|
||||
return None
|
||||
return "".join(out) + "".join(_JSON_CLOSERS[ch] for ch in reversed(stack))
|
||||
|
||||
|
||||
def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str:
|
||||
@@ -213,12 +224,11 @@ def _repair_tool_call_arguments(raw_args: str, tool_name: str = "?") -> str:
|
||||
|
||||
# Passes 2-4: strip trailing commas, close unclosed structures, trim excess closers
|
||||
# (bounded). Bracket counting is string-aware: delimiters inside string values
|
||||
# ({"code": "}"}) are not structure, and the closers are appended in stack order —
|
||||
# {"items": [{"n": 1}, {"n": 2 needs "}]}", not "}}".
|
||||
# ({"code": "}"}) are not structure, and the closers land in stack order — a truncated
|
||||
# {"items": [{"n": 1}, {"n": 2 needs "}]}" appended, and a misnested
|
||||
# {"a": [{"b": 1}, {"c": 2}} needs "]" inserted before the misplaced "}".
|
||||
fixed = re.sub(r",\s*([}\]])", r"\1", raw_stripped)
|
||||
stack = _scan_json_stack(fixed)
|
||||
if stack:
|
||||
fixed += "".join("}" if ch == "{" else "]" for ch in reversed(stack))
|
||||
fixed = _rebalance_json_closers(fixed) or fixed
|
||||
for _ in range(50):
|
||||
if _loads_ok(fixed) or not (
|
||||
(fixed.endswith('}') and fixed.count('}') > fixed.count('{'))
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.message_sanitization import _repair_tool_call_arguments
|
||||
class TestRepairToolCallArguments:
|
||||
"""Verify each repair stage in the pipeline."""
|
||||
@@ -51,20 +53,28 @@ class TestRepairToolCallArguments:
|
||||
result = _repair_tool_call_arguments('{"code": "}", "x": 1', "t")
|
||||
assert json.loads(result) == {"code": "}", "x": 1}
|
||||
|
||||
def test_braces_inside_values_offset_the_count_the_other_way(self):
|
||||
# An unclosed "{" inside a value makes naive counting append one "}" too many.
|
||||
result = _repair_tool_call_arguments('{"code": "if (x) {", "y": 2', "t")
|
||||
assert json.loads(result) == {"code": "if (x) {", "y": 2}
|
||||
|
||||
def test_truncated_nested_array_closes_in_stack_order(self):
|
||||
# {"items": [{"n": 1}, {"n": 2 needs "}]} appended (stack order), not "}}" —
|
||||
# count-based appending grouped all braces before all brackets and never parsed.
|
||||
result = _repair_tool_call_arguments('{"items": [{"n": 1}, {"n": 2', "t")
|
||||
assert json.loads(result) == {"items": [{"n": 1}, {"n": 2}]}
|
||||
|
||||
def test_truncated_flat_array_closes_correctly(self):
|
||||
result = _repair_tool_call_arguments('{"a": [1, 2', "t")
|
||||
assert json.loads(result) == {"a": [1, 2]}
|
||||
# -- Balanced but misnested: the "]" of an array of objects dropped, a "}" closing in
|
||||
# its place (#115061, deepseek-v4-flash via a portal). Counts balance, so nothing can be
|
||||
# appended; the missing closer has to be inserted BEFORE the misplaced one. --
|
||||
|
||||
@pytest.mark.parametrize("raw, expected", [
|
||||
('{"a": [{"b": 1}, {"c": 2}}]}', {"a": [{"b": 1}, {"c": 2}]}),
|
||||
('{"edits": [{"path": "a.py", "mode": "w"}, {"path": "b.py", "mode": "w"}}',
|
||||
{"edits": [{"path": "a.py", "mode": "w"}, {"path": "b.py", "mode": "w"}]}),
|
||||
('{"tool": "edit", "args": {"items": [{"k": 1}, {"k": 2}}}}',
|
||||
{"tool": "edit", "args": {"items": [{"k": 1}, {"k": 2}]}}),
|
||||
('{"calls": [{"name": "a", "arguments": {"x": 1}}, {"name": "b", "arguments": {"y": 2}}}',
|
||||
{"calls": [{"name": "a", "arguments": {"x": 1}}, {"name": "b", "arguments": {"y": 2}}]}),
|
||||
('{"a": [1, 2}', {"a": [1, 2]}),
|
||||
])
|
||||
def test_misnested_closer_is_inserted_before_the_misplaced_one(self, raw, expected):
|
||||
assert json.loads(_repair_tool_call_arguments(raw, "t")) == expected
|
||||
|
||||
# -- Valid JSON passthrough (this path is via except, but still works) --
|
||||
|
||||
|
||||
@@ -13,7 +13,34 @@ unclosed brackets, Python None) don't kill the session.
|
||||
|
||||
import json
|
||||
|
||||
from agent.chat_completion_helpers import _StreamingCall
|
||||
from agent.message_sanitization import _repair_tool_call_arguments
|
||||
|
||||
|
||||
def _accumulated(arguments: str) -> dict:
|
||||
return {0: {"id": "call_1", "type": "function", "function": {"name": "edit", "arguments": arguments}}}
|
||||
|
||||
|
||||
def test_streamed_misnested_arguments_are_repaired_not_flagged_truncated():
|
||||
# deepseek-v4-flash via a portal closes an array of objects with "}}" where "}]}" is
|
||||
# required (#115061); the assembler must hand the tool a repaired call, not drop it
|
||||
# as an unrepairable "{}" flagged as truncated args.
|
||||
raw = '{"edits": [{"path": "a.py", "mode": "w"}, {"path": "b.py", "mode": "w"}}'
|
||||
calls, truncated = _StreamingCall._assemble_tool_calls(_accumulated(raw), "tool_calls")
|
||||
assert truncated is False
|
||||
assert json.loads(calls[0].function.arguments) == {
|
||||
"edits": [{"path": "a.py", "mode": "w"}, {"path": "b.py", "mode": "w"}],
|
||||
}
|
||||
|
||||
|
||||
def test_streamed_mid_string_cut_stays_unrepairable():
|
||||
# Control: content cut inside a string is unrecoverable and must still be flagged,
|
||||
# never guessed into a wrong tool call.
|
||||
calls, truncated = _StreamingCall._assemble_tool_calls(_accumulated('{"q": "unterminated string'), None)
|
||||
assert truncated is True
|
||||
assert calls[0].function.arguments == '{"q": "unterminated string'
|
||||
|
||||
|
||||
class TestStreamingAssemblyRepair:
|
||||
"""Verify that _repair_tool_call_arguments is applied to streaming tool
|
||||
call arguments before they're assembled into mock_tool_calls.
|
||||
|
||||
Reference in New Issue
Block a user