From 5f2eeca1c8604390143b007ec128d23774c6fc97 Mon Sep 17 00:00:00 2001 From: Rohit C Prasad Date: Sat, 15 Aug 2026 06:55:10 -0700 Subject: [PATCH] Diagnose truncated tool calls instead of executing their mangled args Unparseable (_raw) args now get a truthful error: cut-off-by-output-limit says 'smaller pieces', bad JSON says 're-send with declared parameters'. Raw junk is shrunk before entering history so replays can't teach the model the _raw shape. Anthropic default max_tokens 16k -> 32k so typical report files fit outright. --- coworker/engine.py | 67 ++++++++++++++ coworker/providers/anthropic_provider.py | 8 +- tests/test_mangled_tool_calls.py | 110 +++++++++++++++++++++++ 3 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 tests/test_mangled_tool_calls.py diff --git a/coworker/engine.py b/coworker/engine.py index 874efe04..613b94a6 100644 --- a/coworker/engine.py +++ b/coworker/engine.py @@ -137,6 +137,9 @@ class TurnEngine: ): self.messages.insert(0, {"role": "system", "content": instructions}) self._cancel = asyncio.Event() + # Whether the latest assistant turn hit the output-token limit — decides which + # diagnosis a mangled (unparseable-args) tool call gets answered with. + self._turn_truncated = False # Each pending steering message: (text, optional MessageSource sidecar dict). self._steering: list[tuple[str, Optional[dict[str, Any]]]] = [] # tool_call.id → the standing rule that auto-allowed it ("tool → target"), so the @@ -419,6 +422,8 @@ class TurnEngine: # window on this round-trip (estimate fallback when never reported). self._last_context_tokens = turn.usage.context_tokens + self._turn_truncated = turn.finish_reason == "length" + _sanitize_mangled_calls(turn) self.messages.append(_assistant_message(turn, model=self.model)) payload: dict[str, Any] = { "text": turn.text, @@ -617,6 +622,13 @@ class TurnEngine: {"name": tool_call.name, "arguments": tool_call.arguments}, ) self._audit(tool_call, stage="proposed") + if _is_mangled(tool_call): + # The arguments never parsed as JSON (a `{"_raw": …}` fallback from the + # provider). Executing would produce a bare parameter error the model + # misreads — seen in the field as an endless "wrong parameter" retry + # loop. Answer with the ACTUAL diagnosis instead. + yield self._mangled_tool(tool_call) + continue # `request_directory` and `propose_plan` are interactive: the user decides # out-of-band and that decision IS the consent, so they skip the # permission/registry path. @@ -671,6 +683,37 @@ class TurnEngine: result, status = await asyncio.to_thread(self._execute_sync, tool_call) yield self._record_result(tool_call, result, status) + def _mangled_tool(self, tool_call: ToolCall) -> Event: + """Answer a tool call whose arguments never parsed, with the real diagnosis. + + Two causes, two different cures — and the model can only pick the right one if + the error says which happened. Truncation (`finish_reason == "length"`) means + "same content, smaller pieces"; plain bad JSON means "re-send with the declared + parameters". Either way the raw text is NOT replayed into history: a stored + `{"_raw": …}` call reads as a worked example and teaches the model to emit + `_raw` on purpose (observed 2026-08-15), on top of re-sending the junk tokens + every turn.""" + if self._turn_truncated: + reason = ( + "your tool-call arguments were cut off by the output-token limit before " + "they finished streaming — the tool never received them. Produce the same " + "content in smaller pieces: several calls that each write or append a " + "section, keeping each call's content well under the limit. Do not retry " + "the identical oversized call." + ) + else: + reason = ( + "your tool-call arguments did not parse as a JSON object, so the tool " + "received nothing. `_raw` is not a parameter — it is the unparsed text of " + "the failed call. Re-issue the call using the tool's declared parameters." + ) + self.messages.append(_tool_error_message(tool_call, reason)) + self._audit(tool_call, stage="finished", status="error", reason=reason) + return Event( + EventType.TOOL_FINISHED, + {"name": tool_call.name, "status": "error", "reason": reason}, + ) + def _interrupted_tool(self, tool_call: ToolCall) -> Event: """The stop-path answer for a call that will not run: a tool-error result in the history (hosted chat templates reject orphaned tool_calls, and durable-resume @@ -1282,6 +1325,30 @@ def _assistant_message(turn: AssistantTurn, model: Optional[str] = None) -> dict return message +_MANGLED_PREVIEW_CHARS = 200 + + +def _is_mangled(tool_call: ToolCall) -> bool: + """Provider arg-parsers fall back to `{"_raw": }` when a tool call's + arguments aren't a JSON object (typically a stream truncated mid-arguments).""" + return set(tool_call.arguments or {}) == {"_raw"} + + +def _sanitize_mangled_calls(turn: AssistantTurn) -> None: + """Shrink each mangled call's stored raw text to a short preview BEFORE the turn + enters history. The full text is junk (half a JSON document): replaying it costs + thousands of tokens per turn and, worse, teaches the model that `_raw` is a real + parameter shape it should imitate.""" + for tc in turn.tool_calls: + if _is_mangled(tc): + raw = str(tc.arguments.get("_raw") or "") + if len(raw) > _MANGLED_PREVIEW_CHARS: + tc.arguments = { + "_raw": raw[:_MANGLED_PREVIEW_CHARS] + + f"… [unparsed tool-call text, {len(raw)} chars, truncated in history]" + } + + def _tool_result_message(tool_call: ToolCall, result: Any) -> dict[str, Any]: content = result if isinstance(result, str) else json.dumps(result, default=str) return { diff --git a/coworker/providers/anthropic_provider.py b/coworker/providers/anthropic_provider.py index 05bc8f3a..4f059e75 100644 --- a/coworker/providers/anthropic_provider.py +++ b/coworker/providers/anthropic_provider.py @@ -45,8 +45,12 @@ def _usage_from(usage: Any) -> Optional[TokenUsage]: cache_write=int(getattr(usage, "cache_creation_input_tokens", 0) or 0), ) -# Required by the Messages API; a ceiling, not a spend target. -DEFAULT_MAX_TOKENS = 16000 +# Required by the Messages API; a ceiling, not a spend target. Sized for file +# generation, not just chat: a coworker writing a self-contained HTML report ships the +# whole file inside one tool call's arguments, and 16k proved too small in the field +# (the call truncates mid-arguments and the write fails). Current Claude models all +# accept ≥32k output. +DEFAULT_MAX_TOKENS = 32000 # Extended thinking is ON by default (owner call 2026-07-23: no user-facing setting — # most users wouldn't know what a budget is; a per-turn composer control is future work). diff --git a/tests/test_mangled_tool_calls.py b/tests/test_mangled_tool_calls.py new file mode 100644 index 00000000..9f1d29e8 --- /dev/null +++ b/tests/test_mangled_tool_calls.py @@ -0,0 +1,110 @@ +"""Mangled tool calls (`{"_raw": …}` fallback args) get a real diagnosis, not execution. + +The field failure (2026-08-15, report-page generation): a large write_file call blew the +output-token limit, the truncated JSON became `{"_raw": …}`, and the tool's bare +parameter error sent the model into a "wrong parameter" retry loop. Worse, each stored +`_raw` call read as a worked example — after a few, the model began emitting `_raw` as +if it were a real parameter, and every replay re-sent thousands of junk tokens. +""" + +from __future__ import annotations + +import json + +import pytest + +from coworker.engine import EventType, TurnEngine, _MANGLED_PREVIEW_CHARS +from coworker.permissions import Mode, PermissionEngine +from coworker.providers import AssistantTurn, ModelCapabilities, ProviderClient, ToolCall +from coworker.tools import ToolRegistry + + +class MangledProvider(ProviderClient): + """One turn with unparseable write_file args, then a plain reply.""" + + def __init__(self, finish_reason: str, raw: str = "x" * 5000): + self.calls = 0 + self.finish_reason = finish_reason + self.raw = raw + + def complete(self, *, model, messages, tools=None, **settings): + self.calls += 1 + if self.calls == 1: + return AssistantTurn( + text="", + tool_calls=[ + ToolCall(id="t1", name="write_file", arguments={"_raw": self.raw}) + ], + finish_reason=self.finish_reason, + ) + return AssistantTurn(text="done", tool_calls=[]) + + def capabilities(self, model): + return ModelCapabilities(tools=True) + + +def _engine(tmp_path, provider) -> TurnEngine: + return TurnEngine( + provider=provider, + registry=ToolRegistry(), + permissions=PermissionEngine(workspace_root=tmp_path, mode=Mode.INTERACTIVE), + model="m", + ) + + +async def _run(engine) -> list: + return [e async for e in engine.run("build the report")] + + +def _last_tool_message(engine) -> str: + return str([m for m in engine.messages if m.get("role") == "tool"][-1]["content"]) + + +@pytest.mark.asyncio +async def test_truncated_call_is_answered_with_the_truncation_diagnosis(tmp_path): + """finish_reason "length" + unparseable args = the call was cut off. The model's cure + is smaller pieces — the error must say so instead of a bare parameter complaint.""" + engine = _engine(tmp_path, MangledProvider("length")) + events = await _run(engine) + + finished = [e for e in events if e.type is EventType.TOOL_FINISHED] + assert finished[0].data["status"] == "error" + body = _last_tool_message(engine) + assert "output-token limit" in body + assert "smaller pieces" in body + # The turn continues — no retry loop, the model answers after the diagnosis. + assert [e for e in events if e.type is EventType.TURN_END] + + +@pytest.mark.asyncio +async def test_plain_bad_json_is_answered_with_the_parameter_diagnosis(tmp_path): + """No truncation → the model wrote bad JSON (or imitated `_raw`). Tell it `_raw` is + not a parameter and to re-issue with the declared ones.""" + engine = _engine(tmp_path, MangledProvider("stop")) + await _run(engine) + body = _last_tool_message(engine) + assert "_raw" in body and "not a parameter" in body + assert "declared parameters" in body + + +@pytest.mark.asyncio +async def test_raw_junk_is_shrunk_before_it_enters_history(tmp_path): + """The unparsed text must not be replayed at full size: it costs tokens every turn + and teaches the model that `_raw` is a shape to imitate.""" + engine = _engine(tmp_path, MangledProvider("length", raw="y" * 5000)) + await _run(engine) + + assistant = [m for m in engine.messages if m.get("role") == "assistant" and m.get("tool_calls")][0] + stored = json.loads(assistant["tool_calls"][0]["function"]["arguments"]) + assert len(stored["_raw"]) < _MANGLED_PREVIEW_CHARS + 100 + assert "truncated in history" in stored["_raw"] + + +@pytest.mark.asyncio +async def test_short_raw_args_are_kept_verbatim(tmp_path): + """Below the preview cap there is nothing to shrink — storage stays faithful.""" + engine = _engine(tmp_path, MangledProvider("stop", raw='{"path": "repo')) + await _run(engine) + assistant = [m for m in engine.messages if m.get("role") == "assistant" and m.get("tool_calls")][0] + stored = json.loads(assistant["tool_calls"][0]["function"]["arguments"]) + assert stored == {"_raw": '{"path": "repo'}