From 772ec41594a2099a9bbbb3d1a9b8bbbfb76096c9 Mon Sep 17 00:00:00 2001 From: Alfred Pennyworth Date: Sun, 5 Apr 2026 13:00:42 -0700 Subject: [PATCH] fix(tool_call_parsers): recover from truncated/unbalanced JSON in hermes and longcat parsers Local-model tool-call emissions (qwen3.5-122b, hermes-family, longcat) occasionally produce unbalanced JSON inside ... tags: the model stops after closing the inner `arguments` object but before closing the outer function-call object, leaving one or more trailing `}` missing. Similarly, argument values containing file contents carry literal newlines inside JSON strings (strict JSON forbids this). Both cases caused `json.loads` to raise, and the bare `except Exception: return text, None` in hermes_parser and longcat_parser silently dropped the tool call, returning the raw text to the caller. In Phase 2 training loops this poisons reward signals; in deployment it surfaces to users as the model "replying with a tool-call as prose". Changes: - Add `robust_json_loads()` helper in `environments/tool_call_parsers/__init__.py` that uses `json.JSONDecoder(strict=False).raw_decode()` (tolerates literal control chars in strings) and appends 1-3 trailing `}` when the initial decode fails (recovers truncated objects). - Switch `hermes_parser` and `longcat_parser` to call the helper, drop the bare `except Exception`, and emit debug log lines instead of silently swallowing errors. - Add 16 regression tests covering: well-formed JSON, missing 1/2 close braces, literal newlines in strings, combined newline+truncation, empty/None inputs, unrecoverable garbage, non-dict top-level results, multi-line content arguments, and the unclosed-tag + unbalanced-JSON combined case. Qwen 2.5 parser inherits from hermes and benefits automatically. Other parsers (deepseek, kimi, glm, mistral) use similar patterns and may want the same treatment in follow-up PRs. --- environments/tool_call_parsers/__init__.py | 49 +++++++ .../tool_call_parsers/hermes_parser.py | 26 +++- .../tool_call_parsers/longcat_parser.py | 21 ++- tests/tools/test_tool_call_parsers.py | 134 ++++++++++++++++++ 4 files changed, 223 insertions(+), 7 deletions(-) diff --git a/environments/tool_call_parsers/__init__.py b/environments/tool_call_parsers/__init__.py index 8bff3f9d1f06..c2af55092f1c 100644 --- a/environments/tool_call_parsers/__init__.py +++ b/environments/tool_call_parsers/__init__.py @@ -18,6 +18,7 @@ # tool_calls = list of ChatCompletionMessageToolCall objects, or None """ +import json import logging from abc import ABC, abstractmethod from typing import Dict, List, Optional, Tuple, Type @@ -105,6 +106,54 @@ def list_parsers() -> List[str]: return sorted(PARSER_REGISTRY.keys()) +def robust_json_loads(raw: str) -> Optional[dict]: + """ + Parse a JSON object from `raw`, tolerating two common model-output quirks: + + 1. Unescaped control characters inside strings (local models frequently + emit literal newlines/tabs inside argument strings containing file + contents). Handled via `strict=False`. + 2. Truncated generation where the model stops mid-object, missing one to + three trailing close-braces. Handled by appending `}` up to 3 times + and retrying. This addresses the common case where a model emits + `{"name": "x", "arguments": {...}` and stops after the inner `}`, + leaving the outer object unclosed. + + Returns the parsed dict on success, or None on unrecoverable failure + (logs a debug message in that case). Callers should fall back to plain + text when this returns None. + """ + if not raw or not raw.strip(): + return None + decoder = json.JSONDecoder(strict=False) + try: + obj, _ = decoder.raw_decode(raw) + except (json.JSONDecodeError, ValueError): + obj = None + stripped = raw.rstrip() + for extra in ("}", "}}", "}}}"): + try: + candidate, _ = decoder.raw_decode(stripped + extra) + if isinstance(candidate, dict): + obj = candidate + logger.debug( + "robust_json_loads: recovered by appending %d close-brace(s)", + len(extra), + ) + break + except (json.JSONDecodeError, ValueError): + continue + if obj is None: + logger.debug("robust_json_loads: unrecoverable raw=%r", raw[:120]) + return None + if not isinstance(obj, dict): + logger.debug( + "robust_json_loads: non-dict result type=%s", type(obj).__name__ + ) + return None + return obj + + # Import all parser modules to trigger registration via @register_parser decorators # Each module registers itself when imported from environments.tool_call_parsers.hermes_parser import HermesToolCallParser # noqa: E402, F401 diff --git a/environments/tool_call_parsers/hermes_parser.py b/environments/tool_call_parsers/hermes_parser.py index c1902fd623c3..c65f1cf49346 100644 --- a/environments/tool_call_parsers/hermes_parser.py +++ b/environments/tool_call_parsers/hermes_parser.py @@ -6,6 +6,7 @@ """ import json +import logging import re import uuid from typing import List, Optional, Tuple @@ -15,7 +16,14 @@ Function, ) -from environments.tool_call_parsers import ParseResult, ToolCallParser, register_parser +from environments.tool_call_parsers import ( + ParseResult, + ToolCallParser, + register_parser, + robust_json_loads, +) + +logger = logging.getLogger(__name__) @register_parser("hermes") @@ -24,7 +32,10 @@ class HermesToolCallParser(ToolCallParser): Parser for Hermes-format tool calls. Matches ... tags containing JSON with "name" and "arguments". - Also handles unclosed at end-of-string (truncated generation). + Also handles unclosed at end-of-string (truncated generation), + and unbalanced JSON inside the tags (model stopped mid-object before emitting + all closing braces — common with local models where finish_reason=stop fires + after the inner `arguments` object closes). """ # Matches both closed and unclosed tool_call tags @@ -48,7 +59,13 @@ def parse(self, text: str) -> ParseResult: if not raw_json.strip(): continue - tc_data = json.loads(raw_json) + tc_data = robust_json_loads(raw_json) + if tc_data is None or "name" not in tc_data: + logger.debug( + "hermes_parser: dropping unparseable tool_call raw=%r", + raw_json[:120], + ) + continue tool_calls.append( ChatCompletionMessageToolCall( id=f"call_{uuid.uuid4().hex[:8]}", @@ -69,5 +86,6 @@ def parse(self, text: str) -> ParseResult: content = text[: text.find("")].strip() return content if content else None, tool_calls - except Exception: + except Exception as e: + logger.debug("hermes_parser: unexpected parse error: %s", e) return text, None diff --git a/environments/tool_call_parsers/longcat_parser.py b/environments/tool_call_parsers/longcat_parser.py index afecdb862926..29a7bbb935a1 100644 --- a/environments/tool_call_parsers/longcat_parser.py +++ b/environments/tool_call_parsers/longcat_parser.py @@ -6,6 +6,7 @@ """ import json +import logging import re import uuid from typing import List, Optional @@ -15,7 +16,14 @@ Function, ) -from environments.tool_call_parsers import ParseResult, ToolCallParser, register_parser +from environments.tool_call_parsers import ( + ParseResult, + ToolCallParser, + register_parser, + robust_json_loads, +) + +logger = logging.getLogger(__name__) @register_parser("longcat") @@ -45,7 +53,13 @@ def parse(self, text: str) -> ParseResult: if not raw_json.strip(): continue - tc_data = json.loads(raw_json) + tc_data = robust_json_loads(raw_json) + if tc_data is None or "name" not in tc_data: + logger.debug( + "longcat_parser: dropping unparseable tool_call raw=%r", + raw_json[:120], + ) + continue tool_calls.append( ChatCompletionMessageToolCall( id=f"call_{uuid.uuid4().hex[:8]}", @@ -65,5 +79,6 @@ def parse(self, text: str) -> ParseResult: content = text[: text.find("")].strip() return content if content else None, tool_calls - except Exception: + except Exception as e: + logger.debug("longcat_parser: unexpected parse error: %s", e) return text, None diff --git a/tests/tools/test_tool_call_parsers.py b/tests/tools/test_tool_call_parsers.py index bdea75698a89..4040f3b40c73 100644 --- a/tests/tools/test_tool_call_parsers.py +++ b/tests/tools/test_tool_call_parsers.py @@ -272,3 +272,137 @@ def test_malformed_json_fallback(self, parser): text = "[TOOL_CALLS] not valid json" content, tool_calls = parser.parse(text) assert tool_calls is None + + +# ─── robust_json_loads tests ──────────────────────────────────────────── + +class TestRobustJsonLoads: + """Tests for the shared JSON recovery helper.""" + + def test_well_formed(self): + from environments.tool_call_parsers import robust_json_loads + obj = robust_json_loads('{"name": "x", "arguments": {"a": 1}}') + assert obj == {"name": "x", "arguments": {"a": 1}} + + def test_missing_one_close_brace(self): + """Model stopped after closing `arguments` but before outer `}`.""" + from environments.tool_call_parsers import robust_json_loads + obj = robust_json_loads('{"name": "x", "arguments": {"a": 1}') + assert obj is not None + assert obj["name"] == "x" + assert obj["arguments"] == {"a": 1} + + def test_missing_two_close_braces(self): + from environments.tool_call_parsers import robust_json_loads + obj = robust_json_loads('{"name": "x", "arguments": {"a": {"b": 1}') + assert obj is not None + assert obj["name"] == "x" + + def test_literal_newline_in_string(self): + """Local models frequently emit literal \\n inside argument strings.""" + from environments.tool_call_parsers import robust_json_loads + raw = '{"name": "write_file", "arguments": {"content": "line1\nline2\nline3"}}' + obj = robust_json_loads(raw) + assert obj is not None + assert obj["arguments"]["content"] == "line1\nline2\nline3" + + def test_literal_newline_plus_missing_brace(self): + """Both recovery modes combined.""" + from environments.tool_call_parsers import robust_json_loads + raw = '{"name": "write_file", "arguments": {"content": "a\nb\nc"}' + obj = robust_json_loads(raw) + assert obj is not None + assert obj["name"] == "write_file" + assert obj["arguments"]["content"] == "a\nb\nc" + + def test_empty_string(self): + from environments.tool_call_parsers import robust_json_loads + assert robust_json_loads("") is None + assert robust_json_loads(" ") is None + + def test_none_input(self): + from environments.tool_call_parsers import robust_json_loads + assert robust_json_loads(None) is None + + def test_unrecoverable_garbage(self): + from environments.tool_call_parsers import robust_json_loads + assert robust_json_loads("not json at all") is None + + def test_non_dict_result(self): + """A top-level JSON array should not be accepted as a tool call.""" + from environments.tool_call_parsers import robust_json_loads + assert robust_json_loads('[1, 2, 3]') is None + + +# ─── Hermes parser robustness tests ──────────────────────────────────── + +class TestHermesParserRobustness: + @pytest.fixture + def parser(self): + return get_parser("hermes") + + def test_unbalanced_inner_json_recovered(self, parser): + """Model emitted the inner `arguments` `}` but stopped before the outer `}`.""" + text = '{"name": "terminal", "arguments": {"command": "ls -la"}' + content, tool_calls = parser.parse(text) + assert tool_calls is not None, "parser should recover unbalanced JSON" + assert len(tool_calls) == 1 + assert tool_calls[0].function.name == "terminal" + args = json.loads(tool_calls[0].function.arguments) + assert args["command"] == "ls -la" + + def test_tool_call_with_multiline_content_argument(self, parser): + """Argument strings with embedded newlines (common with write_file / skill_manage).""" + text = ( + '{"name": "write_file", ' + '"arguments": {"path": "out.md", "content": "line1\nline2\nline3"}}' + '' + ) + content, tool_calls = parser.parse(text) + assert tool_calls is not None + assert len(tool_calls) == 1 + assert tool_calls[0].function.name == "write_file" + args = json.loads(tool_calls[0].function.arguments) + assert args["content"] == "line1\nline2\nline3" + + def test_unclosed_tag_plus_unbalanced_json(self, parser): + """Model truncated both the JSON and the closing tag.""" + text = '{"name": "search", "arguments": {"q": "hello"}' + content, tool_calls = parser.parse(text) + assert tool_calls is not None + assert tool_calls[0].function.name == "search" + + def test_missing_name_field_skipped(self, parser): + """Object without 'name' is not a tool call.""" + text = '{"arguments": {"a": 1}}' + content, tool_calls = parser.parse(text) + assert tool_calls is None + + def test_unrecoverable_garbage_returns_text(self, parser): + text = 'this is not json at all' + content, tool_calls = parser.parse(text) + assert tool_calls is None + + +# ─── Longcat parser robustness tests ─────────────────────────────────── + +class TestLongcatParserRobustness: + @pytest.fixture + def parser(self): + return get_parser("longcat") + + def test_unbalanced_inner_json_recovered(self, parser): + text = '{"name": "terminal", "arguments": {"command": "pwd"}' + content, tool_calls = parser.parse(text) + assert tool_calls is not None + assert tool_calls[0].function.name == "terminal" + + def test_multiline_content_argument(self, parser): + text = ( + '{"name": "write_file", ' + '"arguments": {"content": "a\nb"}}' + ) + content, tool_calls = parser.parse(text) + assert tool_calls is not None + args = json.loads(tool_calls[0].function.arguments) + assert args["content"] == "a\nb"