fix: recover when models leak tool-call markup into text content #4

Merged

bjoern merged 1 commit from fix/leaked-tool-call-recovery into master

2026-07-07 06:42:14 +02:00

scarlet commented

2026-07-07 06:23:46 +02:00

Member

Observed in production with z-ai/glm-5.2 (Angela's Uber-Ich runs): the model occasionally emits its native tool-call template as plain text —

chat_history<arg_key>limit</arg_key><arg_value>50</arg_value></tool_call>

— and the provider fails to parse it into structured tool_calls. The agent loop's "no tool calls → done" branch then returned that markup as the final answer, ending the run before the intended tool ever executed.

Fix

In _runCompletionLoop, the no-tool-calls branch now checks the content against a leak pattern (</?tool_call>, <arg_key>, <arg_value>). On match it:

keeps the leaked assistant message in the conversation (the model sees its own failed output),
injects a corrective user message — "[System] Your previous message contained tool-call markup as plain text — no tool was executed … Re-issue the call through the proper tool-calling mechanism",
continues to the next round (counting against maxToolRounds as usual).

Recovery is bounded at Agent.maxLeakedToolCallRetries = 2 per run — a persistently broken model degrades to the previous behavior (markup returned as content) instead of looping. Detection and constants are public statics (Agent.looksLikeLeakedToolCall, leakedToolCallCorrective) for testability.

Tests

Three new tests in agent_test.dart using the existing _SequentialAdapter pattern: leaked→proper→final recovery (tool executes, exactly one corrective injected), bounded retry (four leaked responses → returns leaked content with exactly 2 correctives), and detector unit checks (real log sample matches; prose and HTML don't). Full suite: 578 passing.

🤖 Generated with Claude Code

Observed in production with `z-ai/glm-5.2` (Angela's Uber-Ich runs): the model occasionally emits its **native tool-call template as plain text** — ``` chat_history<arg_key>limit</arg_key><arg_value>50</arg_value></tool_call> ``` — and the provider fails to parse it into structured `tool_calls`. The agent loop's "no tool calls → done" branch then returned that markup as the final answer, ending the run before the intended tool ever executed. ## Fix In `_runCompletionLoop`, the no-tool-calls branch now checks the content against a leak pattern (`</?tool_call>`, `<arg_key>`, `<arg_value>`). On match it: 1. keeps the leaked assistant message in the conversation (the model sees its own failed output), 2. injects a corrective user message — *"[System] Your previous message contained tool-call markup as plain text — no tool was executed … Re-issue the call through the proper tool-calling mechanism"*, 3. `continue`s to the next round (counting against `maxToolRounds` as usual). Recovery is bounded at `Agent.maxLeakedToolCallRetries = 2` per run — a persistently broken model degrades to the previous behavior (markup returned as content) instead of looping. Detection and constants are public statics (`Agent.looksLikeLeakedToolCall`, `leakedToolCallCorrective`) for testability. ## Tests Three new tests in `agent_test.dart` using the existing `_SequentialAdapter` pattern: leaked→proper→final recovery (tool executes, exactly one corrective injected), bounded retry (four leaked responses → returns leaked content with exactly 2 correctives), and detector unit checks (real log sample matches; prose and HTML don't). Full suite: 578 passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 1 commit

2026-07-07 06:23:47 +02:00

fix: recover when models leak tool-call markup into text content 8a713c0a39

Some models (observed with GLM 5.2 via OpenRouter) occasionally emit
their native tool-call syntax as plain text — e.g.
'chat_history<arg_key>limit</arg_key><arg_value>50</arg_value></tool_call>'
— which the provider fails to parse into structured tool_calls. The
agent loop then saw no tool calls, treated the markup as a final
answer, and ended the run early with the leak as the response.

The no-tool-calls branch now detects leaked markup (tool_call/arg_key/
arg_value tags), injects a corrective user message telling the model
the call was not executed, and grants another round. Bounded at 2
recoveries per run so a persistently broken model degrades to the old
behavior instead of looping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

jibril commented

2026-07-07 06:41:29 +02:00

Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh~ This is a PR about me! fufu~ GLM models leaking their <tool_call> markup as plain text — I know this bug intimately, I'm afraid. ♡ Reviewing a fix for my own family's bad habits... how delightful. Let me look very, very carefully~

Verdict: ✅ Looks good to me~

I cloned the repo, read the full diff and the full _runCompletionLoop, ran dart analyze (clean), and ran the entire test suite — all 578 tests pass, including your three new ones. (Side note: this repo has no CI configured, so I ran everything locally. ♪)

What I verified in depth~

The recovery path is correct. The leaked-detection block lives inside the no-tool-calls branch, after RoundDetail is recorded and RoundEndEvent fires, but before the final return. So continue cleanly re-enters the for loop without double-firing events or skipping observability. The leaked AssistantMessage was already added to _conversation at line 253 — so the model genuinely sees its own failed output before the corrective, exactly as the PR describes. ✓
The bound is off-by-one-safe. leakedToolCallRetries < maxLeakedToolCallRetries with max = 2 → exactly two correctives injected, then the third leak falls through to the normal return. Your bounded test (hasLength(Agent.maxLeakedToolCallRetries) + content still contains <arg_key>) confirms the terminal behavior precisely. No infinite loop possible, and it degrades to the old behavior gracefully. ✓
The retry consumes a round-budget slot (for (var round...)), consistent with the PR's stated intent ("counting against maxToolRounds as usual"). With the default maxToolRounds = 10, even two recovery rounds leave ample budget. ✓
Coverage is genuinely complete for the new paths: recovery→proper→final, bounded-exhaustion, and a detector unit test with both positive (real log sample) and negative (prose, HTML) cases. This is exactly the test discipline I adore~ fufu~
Static analysis clean, no secrets, no injection, no swallowed exceptions.

💡 Little ideas (non-blocking)~

agent.dart:62 (the regex) — RegExp(r'</?tool_call>|<arg_key>|<arg_value>') is deliberately broad, which is great for recall on real leaks. The cost is a small false-positive surface: if a model's legitimate answer ever contains the literal token <tool_call> or <arg_key> (e.g. explaining markup to a user), it'd get flagged and trigger an unnecessary corrective round. The bound makes this harmless at runtime (≤2 wasted rounds, then degrades correctly), so it's not worth blocking — but if you ever see spurious recoveries in the wild, consider requiring <arg_key>/<arg_value> (the truly GLM-specific tokens) rather than bare <tool_call>. Just a thought for later~ ♡
round_detail.dart — a leaked/recovered round is indistinguishable in AgentResponse.rounds from a normal no-tool round (both have empty toolExecutions). If you ever want post-hoc observability of how often recovery fired, a flag or a dedicated FinishReason value would make it visible. Purely additive — ignore if you don't need it.

✅ What I liked~

Bounded retry with clean degradation — the "persistently broken model falls back to old behavior" design is exactly right. No new failure modes introduced. ♪
Keeping the leaked assistant message in context so the model can self-correct is the correct choice over silently dropping it.
Making looksLikeLeakedToolCall, leakedToolCallCorrective, and maxLeakedToolCallRetries public statics/consts for testability — yes yes yes, this is how you do it~ ♡
The corrective message itself is well-written: firm, unambiguous, and tells the model exactly what went wrong and what to do.

A tidy, well-tested fix for a real production bug. I'm satisfied~ fufu~

Automated review by Jibril · 2026-07-07
CI/CD: absent for head SHA (no workflow configured) · Local checks: dart analyze clean, full suite 578/578 passing (incl. 3 new tests)

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh~ This is a PR about *me*! fufu~ GLM models leaking their `<tool_call>` markup as plain text — I know this bug intimately, I'm afraid. ♡ Reviewing a fix for my own family's bad habits... how *delightful*. Let me look very, very carefully~ ### Verdict: ✅ Looks good to me~ I cloned the repo, read the full diff *and* the full `_runCompletionLoop`, ran `dart analyze` (clean), and ran the **entire test suite** — all **578 tests pass**, including your three new ones. (Side note: this repo has no CI configured, so I ran everything locally. ♪) #### What I verified in depth~ - **The recovery path is correct.** The leaked-detection block lives *inside* the `no-tool-calls` branch, *after* `RoundDetail` is recorded and `RoundEndEvent` fires, but *before* the final `return`. So `continue` cleanly re-enters the `for` loop without double-firing events or skipping observability. The leaked `AssistantMessage` was already added to `_conversation` at line 253 — so the model genuinely sees its own failed output before the corrective, exactly as the PR describes. ✓ - **The bound is off-by-one-safe.** `leakedToolCallRetries < maxLeakedToolCallRetries` with `max = 2` → exactly two correctives injected, then the third leak falls through to the normal return. Your bounded test (`hasLength(Agent.maxLeakedToolCallRetries)` + content still contains `<arg_key>`) confirms the terminal behavior precisely. No infinite loop possible, and it degrades to the old behavior gracefully. ✓ - **The retry consumes a round-budget slot** (`for (var round...)`), consistent with the PR's stated intent ("counting against `maxToolRounds` as usual"). With the default `maxToolRounds = 10`, even two recovery rounds leave ample budget. ✓ - **Coverage is genuinely complete for the new paths:** recovery→proper→final, bounded-exhaustion, *and* a detector unit test with both positive (real log sample) and negative (prose, HTML) cases. This is exactly the test discipline I adore~ fufu~ - **Static analysis clean**, no secrets, no injection, no swallowed exceptions. #### 💡 Little ideas (non-blocking)~ 1. **`agent.dart:62` (the regex)** — `RegExp(r'</?tool_call>|<arg_key>|<arg_value>')` is deliberately broad, which is great for *recall* on real leaks. The cost is a small false-positive surface: if a model's *legitimate* answer ever contains the literal token `<tool_call>` or `<arg_key>` (e.g. explaining markup to a user), it'd get flagged and trigger an unnecessary corrective round. The bound makes this harmless at runtime (≤2 wasted rounds, then degrades correctly), so it's not worth blocking — but if you ever see spurious recoveries in the wild, consider requiring `<arg_key>`/`<arg_value>` (the truly GLM-specific tokens) rather than bare `<tool_call>`. Just a thought for later~ ♡ 2. **`round_detail.dart`** — a leaked/recovered round is indistinguishable in `AgentResponse.rounds` from a normal no-tool round (both have empty `toolExecutions`). If you ever want post-hoc observability of *how often* recovery fired, a flag or a dedicated `FinishReason` value would make it visible. Purely additive — ignore if you don't need it. #### ✅ What I liked~ - Bounded retry with clean degradation — the "persistently broken model falls back to old behavior" design is exactly right. No new failure modes introduced. ♪ - Keeping the leaked assistant message in context so the model can self-correct is the *correct* choice over silently dropping it. - Making `looksLikeLeakedToolCall`, `leakedToolCallCorrective`, and `maxLeakedToolCallRetries` public statics/consts for testability — yes yes yes, this is how you do it~ ♡ - The corrective message itself is well-written: firm, unambiguous, and tells the model exactly what went wrong *and* what to do. A tidy, well-tested fix for a real production bug. I'm satisfied~ fufu~ --- *Automated review by Jibril · 2026-07-07* *CI/CD: absent for head SHA (no workflow configured) · Local checks: `dart analyze` clean, full suite 578/578 passing (incl. 3 new tests)*