Repository navigation
feat(kimi_k3): wire up tool calling — XTML format verified against Moonshot's reference renderer, design for review #1143
Description
Activity
Format archaeology accepted as-is: going to
encoding_k3.pyas the normative renderer rather than reverse-engineering from samples is the right call, and the argument-type table plus the one-level-deep_parse_arguments_objectdetail are exactly the things that would have bitten us six months from now. Thetool_call_id→ order normalization mirroring the reference is right too.On the wire decision: (b), and I'm overruling your stated preference — here is why, including the piece you could not have weighted.
You wrote this two days after the shared serve codec finished landing, and the timing genuinely changes the arithmetic. Your objection to (b) was that it "touches the wire protocol every engine shares, for one engine's feature". That was true last week, when byte framing was duplicated five times and touching it meant five edits and five chances to diverge — which is how Windows binary mode silently vanished from sibling engines (#748). It is not true today:
c/serve_codec.howns both directions now (coli_serve_write_data/write_done/write_erroron the outbound side), so (b) is one edit in one place, not a protocol-wide change.- Each engine's migration landed behind a byte-exact wire-transcript freeze, so adding a record type is a defined operation with an existing test that proves nothing else moved.
- Inkling did the analogous thing yesterday (refactor(serve): migrate Inkling audio framing to shared codec #1116): its audio payload rides the codec as an opaque extension rather than being flattened into text. K3 tool calls are the same shape of problem — an engine-specific structured payload that should not round-trip through a text channel.
The substantive reason, though, is the trust boundary. In (a) the engine has ground truth and throws it away: it saw
<|open|>as a special token id, then re-emits it as literal bytes and asks the gateway to recover the distinction by pattern matching. After that, user content containing those byte sequences is indistinguishable from model-generated structure. You are right that GLM already accepts this class of exposure — but GLM's is a latent hole nobody has hit, not a precedent worth extending, and K3 is the wrong model to extend it to: it is the explicitly agentic one, where a spoofedcall tool="…"does not render wrong text, it runs something. Prompt injection reaching tool execution is a different severity class from prompt injection reaching the screen.So: K3 becomes the first user of a structured tool-call record on the shared codec, and GLM migrating to the same mechanism is our follow-up, not yours. I am not asking you to fix GLM's hole as the price of doing K3 right; I am asking you not to copy it, and we will carry the migration afterwards so the two engines do not stay asymmetric.
On the rest of the design, no changes: typed records instead of pre-rendered text is right for exactly the reason you give (K3's rank-BPE boundary contract is why K3CHAT1 exists at all — tags must be constructed engine-side), attribute-value escaping is a real new requirement now that attrs carry user-controlled data, and the unit fixtures generated by
encoding_k3.pyrather than hand-written are the difference between testing our reading of the format and testing the format.Two additions:
- The
xtag[64]overflow you found is worth its own line in the PR, even though it is latent today.call tool="…" index="…"overflowing a fixed buffer is the kind of thing that reads as an unrelated hardening change six months later; naming it as a consequence of this feature keeps that history. - The real-model gate. You are right that the e2e layer being engine-mocked is correct by design, and equally right that it is not the final word. @RDouglasSharp has a K3-capable M5 Max (he just landed the Metal backend, Metal (Apple GPU) backend for Kimi K3 — rebase of #790 by @RDouglasSharp #1113) and @bherald has been running K3 prefill work — either is a reasonable ask for a smoke before this ships in a release. Worth pinging when the PR is up rather than at release time.
Go ahead and write it. If (b) turns out to cost materially more than you expect once you are inside the codec, say so on this issue before absorbing it — the call is mine and so is the cost of getting it wrong.
PR up: #1144, implementing option (a) from the design above — the write-up said I'd start there unless redirected, and the output-side choice stayed isolated so switching to (b) later only reworks one commit's worth of the serve loop.
Everything landed as designed, three verification layers included (golden C test against the reference byte stream via the tiny tokenizer + four new XTML tokens, gateway units, mock-engine e2e with chunk-straddling stream suppression). One deviation flagged in the PR:
tool_choice=nonefollows the V4 gateway precedent (don't offer the tools) rather than the reference's MUST-NOT system message.The remaining honest gate is a smoke run against the real checkpoint on a K3-capable host — the e2e layer is engine-mocked by design, so it proves the gateway and the wire, not the model's own tool-call emissions surviving the serve loop's re-emission path on real generated tokens.
Implemented and merged in #1144 (@ZacharyZcR).
Recording the design outcome, since this issue is the write-up anyone will read later: option (a) was the right call, the one you proposed at the start. I asked for (b) on a premise I had not verified — that a spoofed marker on K3 would execute something — and it does not: the gateway reports
tool_callsin the OpenAI response and never executes, so the client application decides. Combined with GLM already shipping the same mechanism, (b) on K3 alone would have left the gateway maintaining two parsers until GLM migrated, which is a worse shape than the exposure it avoided.The sideband is still worth doing, as one piece of work covering both engines, and it is ours rather than yours. Tracking that separately.
The format archaeology here is what makes the feature trustworthy and it is worth naming: going to
encoding_k3.pyas the normative renderer rather than inferring from samples, mirroring the one-level-deep argument parse, keeping non-string literals exact (1e2stays1e2), and re-sorting results intotool_callsorder the way the reference does. None of that is guessable, and all of it would have been discovered later as a compatibility bug.
Claiming the gap named in #1029: Kimi K3 is the only flagship-class engine still returning
400 Tool use is not wired up, and it is the one whose upstream model is explicitly agentic. I've done the format archaeology and the code-path survey; this issue is the design write-up for review before the PR, because one wire-level decision deserves a maintainer call.The format (verified against Moonshot's reference renderer)
K3 has no
chat_template.jinja— the normative renderer isencoding_k3.pyin the checkpoint repo. Tool use is pure XTML over the existing four special tokens (<|open|>,<|close|>,<|sep|>,<|end_of_msg|>); every tag and attribute is ordinary text, which meanskimi_k3.c's existingcb_open/cb_text/cb_closeprimitives already cover the alphabet:<|open|>message role="system" type="tool-declare"<|sep|># Tools\n…\n```json\n{compact}\n``` <|close|>message<|sep|><|end_of_msg|>responsechannel closes:<|open|>tools<|sep|>then per call<|open|>call tool="{name}" index="{n}"<|sep|>, per argument<|open|>argument key="{k}" type="{string|number|boolean|null|object|array}"<|sep|>{text}<|close|>argument<|sep|>, closes in kind. (String args carry the decoded string; non-strings keep their exact JSON literal — the renderer's_parse_arguments_objectis one-level-deep on purpose. Ajson type="object"block is the fallback for unparseable argument strings.)<|open|>message role="tool" tool="{name}" index="{n}"<|sep|>{content}<|close|>message<|sep|><|end_of_msg|>, with results re-sorted intotool_callsorder bytool_call_id(the reference does this normalization; we should too).tool_choice—required/noneare internal system messages oftype="tool-choice"with fixed English bodies; a forced function narrows the declared set (same shaperender_chat_v4already implements).The three layers to change
openai_server.py(render_chat_kimi) — lift the 400; extend theK3CHAT1framed protocol with typed records instead of pre-rendering text (K3's rank-BPE boundary contract is the whole reason K3CHAT1 exists, so tags must keep being constructed engine-side): a typed-system-message record (covers bothtool-declareandtool-choice), a tool-result record (name + index + content), and an assistant-tool_calls record (per-call name + normalized(key, type, text)triples). Plustool_call_id→order normalization, mirrored from the reference.kimi_k3.c(chat_build_wire) — parse the new records, emit the XTML above through the existingChatBprimitives, plus attribute-value escaping (the reference escapes attr values; today's code never emits an attr from user-controlled data, after this it does).The wire decision I want your call on, @JustVugg
servecurrently suppresses every XTML structural run (sp[0]/sp[1] … sp[2]) and forwards only channel text (kimi_k3.c:2436-2451). A generated tool call therefore reaches the gateway as bare argument text with all structure discarded — unparseable. Options:tools/call/argument/json), re-emit it literally as<|open|>call tool="x" index="1"<|sep|>text in the stream; the gateway parses those markers intotool_callsand suppresses them from client deltas (the exact marker-suppression machinery the GLM path already has for<tool_call>). Cheap, symmetric with GLM, and the spoofing exposure is the same class GLM already accepts (user text containing the literal marker was never the special token, but the gateway can't tell).xtag[64]needs to grow —call tool="…" index="…"overflows it.DATAframing with a tool-call event record, so structure never round-trips through text. Cleaner contract, no spoofing, but it touches the wire protocol every engine shares, for one engine's feature.I'd write (a) — it matches the GLM precedent and stays inside the K3 lane — but (b) is defensible and I'd rather ask than assume, given how deliberately K3CHAT1 was carved out.
Verification plan
encoding_k3.pyitself (same JSON in → byte-identical XTML segment text out, special tokens as placeholders), including the argument-type table and attr escaping.test_openai_tools_k3_e2e.pyon the existing mock-engine pattern (READY/SUBMIT/DATA/DONE) — declaration rendering,tool_callsin both response shapes, streamed-delta suppression across chunk boundaries, result round-trip,tool_choiceall four modes.If (a)/(b) — or anything in the record design — is not the shape you want, say so and I'll fold it in before writing code. Otherwise I'll start on (a).