From c8a04a110f6006e04f3ddabf678799a75423d35c Mon Sep 17 00:00:00 2001 From: javis-bot Date: Fri, 12 Jun 2026 21:59:26 +0900 Subject: [PATCH] fix(brain): recover colon-JSON and single tool_call object forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qwen2.5:3b emits tool calls in text shapes the parser dropped, breaking two reviewer-reported behaviours: - `getWeather: {"location": "Seoul"}` (a JSON object after the colon) was dumped wholesale into {"query": "{...}"}, so `location` never reached the tool. getWeather then ran with empty args, returned the auto-detected location's weather, the model noticed the mismatch and retried — looping up to 8 times before giving up with an English error. Now the JSON object after the colon is parsed directly as the argument dict. - `call_stop: {"id":..., "function": {"name": "setBroadcast", "arguments": "{\"action\": \"stop\"}"}}` — a single tool_call object without the `tool_calls: [...]` array wrapper, behind a `call_xxx:` label — matched no form, so the raw JSON leaked to the user AND setBroadcast never ran ("방송 꺼줘" did nothing). Now name + arguments are pulled from the embedded `function` object when the name is in the allow-list. Field-captured from the live qwen2.5:3b brain (2026-06-12). Tests cover both shapes, non-ASCII args, dict/string arguments, and unknown-tool rejection. --- src/jarvis/reply/engine.py | 51 +++++++++++++++++++++ tests/test_text_tool_call_parser.py | 69 +++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) diff --git a/src/jarvis/reply/engine.py b/src/jarvis/reply/engine.py index 2984cbc..65e6b62 100644 --- a/src/jarvis/reply/engine.py +++ b/src/jarvis/reply/engine.py @@ -431,6 +431,21 @@ def _extract_text_tool_call(content_field: str, known_names: set): if m and m.group(1) in known_names: name = m.group(1) rest = m.group(2).strip() + # If the value after the colon is itself a JSON object, it already IS + # the argument dict — parse it directly. Small models routinely emit + # `toolName: {"location": "Seoul"}`. Without this fast-path the whole + # object is dumped into {"query": "{...}"} below, so the real named + # arguments (e.g. location) never reach the tool. The tool then runs + # with empty args (e.g. weather falls back to auto-detected location), + # the model notices the answer doesn't match and retries, looping + # until the turn cap. + if rest.startswith("{"): + try: + obj = json.loads(rest) + if isinstance(obj, dict): + return name, obj, f"call_{uuid.uuid4().hex[:8]}" + except Exception: + pass args: dict = {} for pair in re.split(r"[\n,]", rest): pair = pair.strip() @@ -460,6 +475,42 @@ def _extract_text_tool_call(content_field: str, known_names: set): parsed_args = {"query": inside.strip().strip('"').strip("'")} return name, parsed_args, f"call_{uuid.uuid4().hex[:8]}" + # Form: a single tool_call OBJECT emitted without the `tool_calls: [...]` + # array wrapper, optionally behind a `call_xxx:` label. Captured from + # qwen2.5:3b (2026-06-12) on "방송 꺼줘": + # call_stop: {"id": "call_stop", "type": "function", + # "function": {"name": "setBroadcast", + # "arguments": "{\"action\": \"stop\"}"}} + # The colon/array forms above don't match (the label isn't a tool name and + # there's no array), so without this the raw JSON leaked to the user AND the + # chosen tool never ran. Pull name + arguments straight out of the embedded + # `"function": {...}` object. + func_match = re.search( + r'"function"\s*:\s*\{\s*"name"\s*:\s*"([^"]+)"' + r'(?:\s*,\s*"arguments"\s*:\s*(\{.*?\}|"(?:[^"\\]|\\.)*"))?', + content_field, + re.DOTALL, + ) + if func_match and func_match.group(1).strip() in known_names: + fname = func_match.group(1).strip() + raw_args = func_match.group(2) + parsed_args = {} + if raw_args: + try: + val = json.loads(raw_args) + if isinstance(val, dict): + parsed_args = val + elif isinstance(val, str): + # arguments was a JSON string (double-encoded) — unwrap once. + try: + inner = json.loads(val) + parsed_args = inner if isinstance(inner, dict) else {"query": val} + except Exception: + parsed_args = {"query": val} + except Exception: + parsed_args = {} + return fname, parsed_args, f"call_{uuid.uuid4().hex[:8]}" + return None, None, None diff --git a/tests/test_text_tool_call_parser.py b/tests/test_text_tool_call_parser.py index 7885f52..0a302b9 100644 --- a/tests/test_text_tool_call_parser.py +++ b/tests/test_text_tool_call_parser.py @@ -84,6 +84,75 @@ class TestSimplifiedColonForm: assert name is None +class TestColonFormWithJsonObjectValue: + """Form 2b: `toolName: {json object}`. + + Field-captured from qwen2.5:3b (2026-06-12): the model emits the weather + call as ``getWeather: {"location": "Seoul"}``. The whole JSON object must + become the argument dict. Before the fix it was dumped into + ``{"query": "{...}"}``, so ``location`` never reached the tool, the tool + fell back to the auto-detected location, and the model looped retrying + different cities until the turn cap (observed: 8 getWeather calls, then an + English error fallback). + """ + + def test_json_object_after_colon_becomes_args(self): + content = 'getWeather: {"location": "Seoul"}' + name, args, _ = _extract(content, tool_name="getWeather") + assert name == "getWeather" + assert args.get("location") == "Seoul" + assert "query" not in args + + def test_empty_json_object_after_colon(self): + content = "getWeather: {}" + name, args, _ = _extract(content, tool_name="getWeather") + assert name == "getWeather" + assert args == {} + + def test_non_ascii_location_after_colon(self): + content = 'getWeather: {"location": "서울"}' + name, args, _ = _extract(content, tool_name="getWeather") + assert name == "getWeather" + assert args.get("location") == "서울" + + +class TestSingleToolCallObjectForm: + """Form 2c: a single tool_call object without the `tool_calls: [...]` array. + + Field-captured from qwen2.5:3b (2026-06-12) on "방송 꺼줘": the model picked + the right tool but emitted it behind a `call_xxx:` label as a bare object. + The name + arguments must be pulled from the embedded ``function`` object; + before the fix this leaked the raw JSON to the user and the tool never ran. + """ + + def test_single_object_with_string_arguments(self): + content = ( + 'call_stop: {"id": "call_stop", "type": "function", ' + '"function": {"name": "setBroadcast", ' + '"arguments": "{\\"action\\": \\"stop\\"}"}}' + ) + name, args, _ = _extract(content, tool_name="setBroadcast") + assert name == "setBroadcast" + assert args.get("action") == "stop" + + def test_single_object_with_dict_arguments(self): + content = ( + '{"id": "c1", "type": "function", ' + '"function": {"name": "getWeather", "arguments": {"location": "Seoul"}}}' + ) + name, args, _ = _extract(content, tool_name="getWeather") + assert name == "getWeather" + assert args.get("location") == "Seoul" + + def test_single_object_rejects_unknown_tool(self): + content = ( + '{"function": {"name": "fileSystem_write", ' + '"arguments": "{\\"path\\": \\"/tmp/x\\"}"}}' + ) + name, _args, _ = _extract(content, tool_name="setBroadcast") + assert name is None + + class TestFunctionCallForm: """Form 3: `toolName(...)`."""