Issues / #3075

#3075 serve: a tool call is parsed and returned even when the request declares no tools (content state, frontend.py:1285)

open · @riverhh76 · 2 commentaires · Sur GitHub

BenchmarksServer & APIAMD / HIPModels & quants

Description

## Environment

- Strata `main` @ `477ac13c` (fetched 2026-10-19), `--engine strata`, OpenAI-compatible serve
- Models: `swift-1.5-iq3_xxs` and the `qwen3.8-flash-next` family — **model-independent** (see
  "Another observation, not part of the fix")
- Sampling: `{"temperature": 0, "top_p": 1.0, "presence_penalty": 0}` — greedy
- Reproduces on **every** model tested; also present in the released `0.1.45` container image

---

## Summary

`OutputParser` scans the answer for `<​tool_call>` in two states. The scan in the
**reasoning** state is gated on whether the request declared tools; the scan in the
**content** state is not. So a request with **no `tools` field at all** still yields
`message.tool_calls` and `finish_reason: "tool_calls"` whenever the model writes that
XML — and the function name is never checked against the declared set (there is no
declared set).

A client that does the normal thing — `if (msg.tool_calls): run(msg.tool_calls)` — then
executes calls for tools it never offered. When the client instead *ignores* them because
it offered none, it gets `content: ""` back, i.e. **an empty answer**.

---

## Minimal reproduction

No GPU, no model, no server — the parser alone.

```python
import sys; sys.path.insert(0, "serve")
import frontend as F

def call(name, **params):
    body = F.FUNC_START + name + ">\n"
    for k, v in params.items():
        body += F.PARAM_START + k + ">\n" + str(v) + "\n" + F.PARAM_END + "\n"
    return F.CALL_START + body + "</function>" + F.CALL_END

# exactly what the model wrote in the incident below
out = call("get_indicators", curr_date="2026-10-15", indicator="close_10_ema", symbol=600076) \
    + call("get_indicators", curr_date="2026-10-15", indicator="close_50_sma", symbol=600076)

p = F.OutputParser(thinking=False, tools=None, stream_tools=False)
events = p.feed(out)
print([e.call.name for e in events if e.kind == "tool_call"])
print("".join(e.text for e in events if e.kind == "content"))
```

Actual output — `tools=None` was passed:

```
['get_indicators', 'get_indicators']
''            # content is empty
```

`tools=[]` behaves the same. The streaming path reproduces it too: over HTTP with
`stream: true` and `stream_tools` always on (`server.py:4216`), all four subtests of the
regression test below fail on the unpatched tree.

---

## Observed behaviour matrix

Same parser, same model output, varying only what the request declared:

| Request declared | `tool_calls` emitted | `content` |
|---|---|---|
| nothing (`tools` absent) | **2** | **empty** ← wrong |
| `tools: []` | **2** | **empty** ← wrong |
| `tools: [get_indicators]` | 2 | empty (correct — they are real) |
| `tools: [some_other_tool]` | **2** ← names `get_indicators` | **empty** ← wrong |

The last row is a second, independent gap: a request that *does* declare tools still
gets a call to a function that is not in the set. The guard proposed below does not cover
it; it is written up separately under "A second, related gap" because the obvious patch
for it does not work.

---

## Root cause

One line, sitting next to the guarded twin of it:

```python
# :1246  reasoning state — gated
tool = self.buf.find(CALL_START) if self.schemas else -1

# :1285  content state — NOT gated   ← this line
i = self.buf.find(CALL_START)
```

With no tools, `self.schemas == {}` (built at `:887`), yet the content state still
transitions into `"call"` and runs `_finish_call`, which builds the `ToolCall` at `:872`
without ever comparing `name` with the declared names (`:1448-1451` does
`self.schemas.get(name)`, which is `None` and is accepted).

**Why this reads as an oversight rather than a decision.** Every neighbouring path in the
same parser checks something the content path does not:

| path | line | gated on declared tools? |
|---|---|---|
| `rcall` state (a call quoted in the reasoning) | `:1203` | yes — `if name in self.schemas` |
| reasoning-state opener scan | `:1246`, `:1258` | yes — gated on `self.schemas` |
| `json_tool_call` (the `recover` path) | `:829-830` | yes — `obj["name"] not in schemas → None` |
| **content-state opener scan** | **`:1285`** | **no** |

The JSON sibling of the same feature validates the name; the XML path does not.

This also runs against the principle the parser has already been fixed to hold elsewhere:
a `<​tool_call>` that is *quoted* is text, not a call — #1058, #2039, and #700 ("serve: a
`<​tool_call>` named in prose before the real call is content, not a malformed call"). The
rule those fixes established is that text which looks like a tool call is text. Here the
inverse holds: a tool call naming a function that was never declared is still a tool call.

---

## Real-world impact

Production agent (`TradingAgents`) on 600076.SH, 2026-10-15. The technical analyst ran
four tool rounds, then sent a convergence turn that deliberately **omits `tools`** and
appends *"tool call rounds are exhausted, you must not call any more tools"*. The
response was:

```json
{"content": "", "finish_reason": "tool_calls",
 "tool_calls": [{"name": "get_indicators", "arguments": {"curr_date": "2026-10-15",
                    "indicator": "close_10_ema", "look_back_days": 30, "symbol": 600076}},
                {"name": "get_indicators", "arguments": {..., "indicator": "close_50_sma", ...}}],
 "usage": {"prompt_tokens": 7328, "completion_tokens": 172}}
```

The agent writes `content` only when `tool_calls` is empty, so the analyst's report became
the empty string, the data-quality gate graded it **F**, and the final research document
shipped a section reading *(empty)*. A production research pipeline produced a report with
a silent hole in it.

This is not rare, and it predates the current release: our own bench logs from **2026-10-04**
(Strata 0.1.38/0.1.39) already recorded `finish=tool_calls` on requests that carried no
`tools`, twice, noted then as *"occasional tool_calls hallucination"* and attributed to the
model. It is the parser.

Anyone whose convergence/final-turn pattern is "stop calling tools" is exposed — that is a
common and reasonable agent design.

---

## Test gap

`serve/test_server.py:4313 test_auto_absent_and_none_leave_the_tools_to_the_model` is the
test for this case, and it only checks the **prompt** side:

```python
offered = "search the web" in self.tok.decode(self.engine.prompts[0])
self.assertEqual(offered, extra.get("tool_choice") != "none")
```

Its fake engine never emits tool-call XML, so the answer side is never exercised. There is
no test anywhere asserting that a request which declared no tools cannot come back with
`tool_calls`.

---

## Suggested direction

Mirror the guard that already exists at `:1246`:

```python
# :1285
i = self.buf.find(CALL_START) if self.schemas else -1
```

That one line is the whole fix for this report. Measured with the repo's own
`serve/test_server.py` inside the 0.1.45 image (jinja2 3.1.6):

| | result |
|---|---|
| baseline `477ac13c`, with the new regression test | **4 subtests fail** (`nothing declared` and `tool_choice none`, streaming and not) |
| with the guard | **4 subtests pass** |
| full suite, baseline | 337 passed, 482 subtests passed |
| full suite, with the guard **and** the new test | **338 passed, 486 subtests passed** |

The new test is `ForcedToolChoice.test_a_call_the_request_never_offered_stays_text`: a model
that, after `</think>`, writes a complete well-formed call naming a tool the request never
offered. Today that comes back as `tool_calls` with an empty content; with the guard the
whole answer comes back as text, call and all.

The test also pins the case where the request declared *other* tools and `tool_choice: none`
— both must produce text, and both do today only by accident of the first guard being absent.

---

## A second, related gap: an undeclared name, with tools declared

When a request *does* declare tools but the model names a function that is not among them,
the call is emitted anyway:

| request declared | model writes | result today |
|---|---|---|
| `[some_other_tool]` | a call to `get_indicators` | `tool_calls: [get_indicators]`, content empty |

This one is **not** fixed by the guard above, and I want to be precise about why, because the
obvious patch does not work.

`Service.run` always constructs the parser with streaming on (`server.py:4216`,
`stream_tools=True`). With streaming on, the call is consumed by `_scan()` **before**
`_finish_call()` runs (`frontend.py:1395-1408`):

- `_scan()` hits `<function=NAME>` at `frontend.py:1078-1087`, builds
  `ToolCall(name=name, arguments={})` with **no membership check** — `self.schemas.get(name)`
  at `:1085` is used only to read declared parameter *types* — and immediately emits
  `tool_start`, so the client is already told about the call.
- `finish()` (`:1428-1431`) emits the final `tool_call` for that already-announced call.

Adding a name check inside `_finish_call` therefore changes nothing observable over the wire:
by the time it runs, the `tool_start`/`tool_args` chunks are already on the wire and the
client has assembled the call. I confirmed this by measurement — with such a check added, the
response carried **both** the text and a `tool_calls` entry.

The right place is `_scan()` at the `ss == "name"` step (`:1083`): the name is complete there
and nothing has been emitted yet, so an undeclared name can still be refused and the raw text
returned as content. That is a real change to the streaming state machine, so I would rather
raise it than guess at it. Flagging it as a separate item rather than folding it into the
one-line fix above — happy to work on it if you want it in the same PR.

---

## Another observation, not part of the fix

`serve/chat_template.jinja:130-157` renders an assistant message's `tool_calls`
**unconditionally** — that loop sits *outside* the `{%- if tools ... %}` block at `:72`. So a
request that declares no tools, but whose history contains tool-calling turns, puts verbatim
tool-call demonstrations into the prompt:

```
<|im_start|>assistant
<think>

</think>

<​tool_call>
<function=get_indicators>
<parameter=indicator>
rsi
</parameter>
...
</function>
</​tool_call><|im_end|>
```

Measured on `swift-1.5-iq3_xxs` with the repo's own tokenizer
(`tools/strata_tokenizer.py` over `/data/packs/swift-iq3_xxs/tokenizer`):

| conversation | `tools=None` | `tools=[one tool]` |
|---|---|---|
| 5-message demo (1 historical call) | 396 tokens | 641 tokens (the tools block adds 245) |
| the real 13-message incident payload | **7364 tokens, 4 verbatim call demonstrations** | — |

The prompt then ends with an **already-opened assistant turn** (`<|im_start|>assistant\n<think>\n\n`),
after a final user turn that reads *"Tool call rounds are exhausted. You must not call any
more tools. Write the final report now."*

Greedy decoding on that context continues the demonstrated pattern. That is expected, and it
is not the bug — a model writing `<​tool_call>` after being shown four examples is behaving
reasonably. The bug is that **the parser treats the result as a real call**.

Worth noting that the reasoning-state equivalent is already handled: a call quoted inside the
thinking stays reasoning text (`rcall` at `:1194-1212`, guard at `:1203`/`:1246`). The answer
of a non-thinking turn, or the part of a thinking turn after `</think>`, was not. I confirmed
that split by measurement: the same XML before `</think>` stays text today; the same XML after
it becomes a call.

I am not proposing a change here — the history rendering cannot simply be dropped without
corrupting the conversation — but the trigger is prompt hygiene, not the weights, so
swapping models does not remove it.

---

## What I did not test

- Only the `Content` → `call` path was exercised directly. The `rcall` (reasoning) path
  already gates correctly and was left alone.
- No end-to-end run against a live engine for the fix; the regression data above is the
  parser plus the full serve suite, not a served request.
- Vision and MCP paths u

Sur le site

Liens install, modèles, releases.