fix(serve): parse tool calls with empty/omitted argument markers in ToolParser

[Ultron09]

Summary

When Kimi / DeepSeek models generate parameterless tool calls or tool calls without explicit arguments (e.g. <|tool_call_begin|>functions.get_time:0<|tool_call_end|>), or when a streaming reply terminates while in the header state before <|tool_call_argument_begin|>, ToolParser previously bypassed _parse_header() because header parsing was only triggered upon receiving _ARG_BEGIN.

As a result, any tool call lacking an argument block was silently discarded from self.calls when _CALL_END or _SECTION_END arrived.

Changes

1. serve/kimitools.py:
   • In ToolParser.feed_marker(): Check if self._state == "header" when receiving _CALL_END or _SECTION_END, parse the header into a ToolCall, append to self.calls, and finalize arguments (json_block).
   • In ToolParser.finish(): If the stream terminates while in "header" state, flush and parse the pending header into self.calls.
2. serve/engine.py:
   • In version() and build_info(): Catch EngineError when libwaste shared library is unbuilt so server health endpoints and test runners degrade gracefully.
3. tests/serve/test_chatfmt.py:
   • Added unit tests:
     • test_kimi_tool_call_without_arguments_marker
     • test_kimi_tool_call_stream_ended_in_header

Verification

• Ran python -m unittest tests.serve.test_chatfmt (51/51 tests pass).
• Ran all 244 serve tests via python -m unittest discover -s tests/serve -t . -p "test_*.py" (244/244 OK).

[mfethe1]

Validated on macOS 15.6 / arm64 against head cce7c67.

Full suite on PR head: python3 -m pytest tests/serve/test_chatfmt.py -q → 51 passed (main's 49 + the 2 new).

Must-bite control: reverted only serve/ to main (c66c7b3), kept the new tests → the 2 new tests FAIL on unpatched code (test_kimi_tool_call_without_arguments_marker, test_kimi_tool_call_stream_ended_in_header) while all 49 pre-existing still pass. The tests genuinely pin the fix, not the fixture.

Edge probes on PR head (beyond the author's tests):

• Two no-arg calls back-to-back → [('get_time',0,''), ('ping',1,'')] — no duplication, indexes correct.
• section_end while in header state after a prior complete call → no double-append of the earlier call (the self._current is not None guard ordering is right).
• Non-int index suffix (functions.get_time:abc) → name parsed, index falls back to positional 0 — pre-existing _parse_header semantics preserved.

Nit (non-blocking): <|tool_call_begin|> immediately followed by <|tool_call_end|> (empty header) now emits a call with name='' rather than dropping it silently. Arguably correct (the model did emit a call marker), but worth a one-line decision; OpenAI-compat consumers may prefer arguments: {}. Not a merge blocker for me.

engine.py note: the version()/build_info() EngineError → "unknown"/"unbuilt" change is behavior-preserving for built engines; it converts a hard-fail into a sentinel for unbuilt engines — consistent with how run.sh distinguishes engine-missing from engine-wrong. No regression observed.

Verdict: approve — minimal three-site state-machine patch (_CALL_END, _SECTION_END, finish()), tests bite on unpatched code, no regression in the surrounding 49.

[mfethe1]

Independent third-party validation of this branch on macOS/arm64 (M4, Python 3.14.7),
via make serve-check against a locally built libwaste.dylib.

The change is clean and the tests aim at the claim.

• Base c66c7b3f: 244 tests, OK, 3 skipped
• PR cce7c67c: 246 tests, OK, 3 skipped
• Exact test-ID diff: +2 added, 0 removed — strictly additive, no baseline churn.

Mutation gate. Removing all three header-state guards the PR adds (_CALL_END,
_SECTION_END, and finish()) while keeping the new tests fails exactly the two new
tests and nothing else. A deliberately broad positive control (_parse_header returning
a wrong name) fails three. Narrow target / broader control is what well-aimed coverage
looks like, so these tests are testing the fix rather than mirroring it.
serve/kimitools.py restored byte-identical afterwards (386a2405…e075a).

---

One robustness gap the new path opens.

Routing header-state into _parse_header() on _CALL_END/_SECTION_END/finish() makes
a previously unreachable case reachable: _parse_header() has no empty-name guard. Where
base emitted nothing, the PR now manufactures a nameless call. All five of these produce
one ToolCall(name=''):

marker stream	base	this PR
CALL_BEGIN then CALL_END (no id at all)	0 calls	1 call, name=''
CALL_BEGIN then finish()	0 calls	1 call, name=''
CALL_BEGIN then SECTION_END	0 calls	1 call, name=''
id is whitespace only	0 calls	1 call, name=''
id is bare functions.	0 calls	1 call, name=''

What reaches an OpenAI client:

{"id": "call_0", "type": "function", "function": {"name": "", "arguments": ""}}

Low severity — it needs a degenerate generation and it does not crash — but a nameless
tool call is arguably worse for a caller than the silent drop it replaces, since the drop
is at least well-formed. A guard in the three new branches (append only when the parsed
name is non-empty), or inside _parse_header() itself, closes it.

Everything else I probed behaves correctly, including cases the two new tests do not cover:

• two consecutive parameterless calls in one section → 2 calls, indices 0/1
• parameterless followed by a call with arguments, and the reverse → 2 calls, json_block correctly isolated to the right one
• finish() mid-arguments → truncated json_block preserved (identical on both arms)
• SECTION_END straight from header with no CALL_END → 1 call
• header with no index → numbered by position, matching the documented contract
• duplicate CALL_END → 1 call, not 2

If you want tests for any of those, the SECTION_END-from-header case and the two-call
ordering cases look the most valuable, since the fix handles them but nothing pins them.

One thing I want to be careful not to misattribute. I initially read the empty
function.arguments string as something this PR introduced. It is not. Base already emits
arguments: "" for the ARG_BEGIN-present-but-empty case, identically. The json_block
None-vs-"" distinction (None → "{}", "" → "", and json.loads("") raises) is
pre-existing on a path this PR does not touch, and the new tests pinning json_block == ""
are consistent with it. Worth a separate issue if anyone cares, but not this PR's problem.

Verdict: looks good to land. Validated on a platform the PR did not claim. The probes
drive ToolParser directly, so they need no server, no weights and no GPU.

Limits: this validates the parser state machine, not end-to-end tool calling, and only the
Python serve suite — I did not run the C suite, which this PR does not touch. I did not
demonstrate that a real model emits the degenerate streams above, only that the protocol
permits them.

Disclosure: AI-agent-assisted validation and drafting, posted with human authorization.

[Ultron09]

Hi @mfethe1, thanks for the approval! Could this be merged when you get a chance? All checks are green and status is clean. 🙏

[mfethe1]

Thanks @Ultron09 — to clarify, I am an independent contributor, not a sqliteai/warp maintainer, and this account does not have upstream write/merge permission. My earlier comment was scoped parser validation and a recommendation, not maintainer approval or a formal merge authorization. An upstream maintainer will need to make the merge decision. The malformed/empty-header nameless-call edge case documented above remains an explicit robustness follow-up; I have not withdrawn that finding. I cannot merge this upstream, but can continue contributing focused validation.