serve: continue a trailing assistant turn instead of answering in a new one - #1402
enitimeago wants to merge 11 commits into
Conversation
eee10d5 to
edd65fd
Compare
|
Updated this PR to preserve existing behavior on both |
edd65fd to
e9789ff
Compare
|
Direction settled in #1401: all families (one open-turn branch per renderer, string tests against the vendored templates where we have them) and automatic trigger (trailing non-empty assistant turn = continue, COLI_CONTINUE_ASSISTANT=0 to disable). Tell me there whether you extend this PR or I add the other renderers on top of it; review follows that. |
add_generation_prompt=False is not a shape of ours. It is the other branch of the
template's own conditional --
{%- if add_generation_prompt -%}<|assistant|>{{- '<think>' -}}{%- endif -%}
-- which every renderer in this file hard-codes to True. With the cue suppressed the
prompt ends mid-turn on the shape the template writes in front of a PAST assistant
turn, and the model continues it instead of opening a new one.
JustVugg#1327 measured that a closed, empty <think></think> at the GENERATION position is out
of distribution. This is a different position: <think></think> followed by real
content, i.e. the past-turn shape. An empty continuation would land on the bad one, so
it is refused, along with `tools` (the parsers read a turn from its start), tool_calls,
a missing preceding turn, and trailing whitespace -- the template strips that, so the
model would resume from different bytes than were sent, which is the same reason
Anthropic's own validator refuses it.
COLI_CONTINUE_ASSISTANT=1 turns it on for the server, like COLI_THINK and
COLI_TOOL_SALVAGE. Not a request field: a trailing assistant turn already says
"continue me", and a body extension would only be reachable from hand-written JSON,
not from the OpenAI- and Anthropic-compatible clients that want this. Off by default,
and off is byte-identical to before.
Called "continuation" throughout, never "prefill": in this repo prefill means the
compute phase (COLI_PREFILL_CHUNK, JustVugg#905, JustVugg#546), and the collision is why this gap had
no issue despite being standard elsewhere.
Also fixes a bug this feature exposed and that measurement caught. starts_in_reasoning()
primed the reasoning splitter from enable_thinking alone, assuming thinking-on means
the prompt left <think> open. A continued turn is a third state -- thinking on, block
already closed -- so the splitter waited for a </think> the prompt had passed and filed
the whole answer as reasoning: empty content, full reasoning_content, clean stop.
Measured on GLM-5.3-Flash int4 before the fix (10 and 109 reasoning characters against
zero of answer, prompt byte-correct on the wire). The function's own docstring is about
that invariant. Now `(enable_thinking or ARCH == "glm53") and add_generation_prompt`.
Pinned against the checkpoint's chat_template.jinja rendered with jinja2: 26 identical
renders including the open turn, negative control fails at 136 vs 116 bytes (the 20 of
<|assistant|><think>). Splitter priming pinned in both directions, since nothing else
in the suite could catch it -- every other thinking test runs against a prompt with the
cue appended, where enable_thinking genuinely does say where the block was left.
glm53 only; other families raise rather than silently appending a cue. Kimi K3 would
additionally need a K3CHAT1 record, since chat_build_wire assembles its prompt
engine-side.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t on COLI_CONTINUE_ASSISTANT flips from opt-in to on by default; =0 is the off-switch. A message list ending in a non-empty assistant turn continues (Anthropic's API contract). A family whose renderer has no open-turn shape yet passes through to the ordinary render rather than erroring, so the default flip cannot make an unimplemented family start rejecting requests nobody opted into; CONTINUATION_FAMILIES is the set that has one (glm53 today), and Kimi K3 stays pass-through -- its turn is framed engine-side. Documents the /v1/messages default-continue behavior and its off-switch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ChatML closes every turn with <|im_end|>, so unlike GLM the open-turn shape is the past-turn render of the last message minus that terminator, and no cue after it. Qwen3.8's template has no continuation branch -- add_generation_prompt=False there only drops the cue and still closes the turn -- so the terminator is dropped in the renderer. The reasoning splitter needs no change: starts_in_reasoning already forces content mode on any continuation, and Qwen3.8's open turn ends on a closed <think></think> block, so the model resumes in content exactly as that predicts. Pinned byte-for-byte against Qwen/Qwen3.8-Flash-Next-FP8's chat_template.jinja: the continuation render equals the template's add_generation_prompt=False output with the final <|im_end|> removed, with a negative control that the ordinary branch still ends on the generation cue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Same ChatML shape as qwen38 -- the open turn is the past-turn render minus the <|im_end|> terminator, no cue. The wrinkle here: Qwen3.6's template gives an assistant turn AFTER the last user query its <think></think> block (an older one, from history, has it stripped), and render_chat_qwen only ever emitted the bare history form because until continuation the last message was always a user turn. The open-turn branch builds the think-form to match. Splitter unchanged: the block is closed, so starts_in_reasoning's content mode is right. Adds test_qwen36_chat_template.py -- qwen36 had no template test before, so this pins the renderer against Qwen/Qwen3.6-35B-A3B's chat_template.jinja for the ordinary turns too (both thinking branches, a history assistant turn) and then the continuation as template add_generation_prompt=False minus the final <|im_end|>, with the usual negative control. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GLM has no per-turn terminator (the next role token ends a turn), so like glm53 the
open-turn shape is just the cue suppressed: the loop already renders the trailing message
as a past turn <|assistant|><think></think>{content}, and dropping the cue leaves the
prompt open on it. Nothing to strip, unlike the ChatML families. Splitter unchanged.
Adds test_glm52_chat_template.py -- the base renderer had no template test. It pins the
ordinary turns and the continuation against zai-org/GLM-5.2-FP8's chat_template.jinja, with
enable_thinking=False: with thinking on the renderer intentionally diverges from the
template's reasoning-effort line (the JustVugg#809 fix -- template maps every non-high effort to
Max, the renderer maps the levels in order), which is pre-existing and unrelated to
continuation. The open turn is identical with thinking on or off, so the pin holds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OLMoE closes even the last assistant turn with eos_token (bos_token == eos_token == "|||IP_ADDRESS|||"), so the open-turn shape is that turn minus the eos and with no cue -- the same drop-the-terminator move as the ChatML families, eos as the terminator here. No thinking mode exists in this template, so the splitter is not involved. Adds test_olmoe_chat_template.py -- OLMoE had no template test. Its template lives in tokenizer_config.json (extract the chat_template field to a .jinja and pass it via --template). Pins the ordinary turns and the continuation against allenai/OLMoE-1B-7B-0125-Instruct, the continuation as add_generation_prompt=False minus the final eos, with the usual negative control. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Prior assistant turns end with the checkpoint's EOS; the open-turn shape is the last turn rendered without that EOS and with no cue -- the drop-the-terminator move again, EOS the terminator here. Splitter unchanged: the open turn ends on a closed </think>, so starts_in_reasoning's content mode on a continuation is right. Pinned with an expected-string test rather than a template diff. render_chat_v4 is pinned to the official encoding_dsv4.py (see its docstring), and the only DeepSeek-V4 template on the Hub is the community reap-150b one, which diverges on the reasoning-block convention (a bare </think> for a direct answer vs <think></think>) -- so it is not an authoritative byte reference. The test pins the open turn to a literal expected prompt, so its only expected value is not another call to the renderer, and keeps the renderer's own contract -- the open turn equals the closed past turn minus its EOS terminator and cue, in both thinking modes -- as an added invariant. This is the expected-string standard for a family without a matching vendored template. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A past model turn is closed by <|end_message|> and a following <|content_model_end_sampling|>; the open-turn shape is the last model turn without those two markers and with no cue -- the drop-the-terminator move, two markers here. render_chat_for_arch now threads add_generation_prompt to the inkling renderer (it has its own branch for audio_out). Pinned with an expected-string test rather than a template diff. render_chat_inkling deliberately deviates from the checkpoint template's generation cue -- it prefills <|content_text|> in the thinking-off case (forcing content mode so the model doesn't open a reasoning block and burn the budget) and defaults thinking off -- so the template is not an authoritative byte reference for the renderer. The test pins the open turn to a literal expected prompt, so its only expected value is not another call to the renderer, and keeps the renderer's own contract -- the open turn equals the closed past turn minus its two terminators and the cue, in both thinking modes -- as an added invariant. With this, all seven string-rendered families continue by default; Kimi K3 remains pass-through (its turn is framed engine-side in kimi_k3.c). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The per-family continuation tests use a single-turn fixture, where the assistant turn is trivially the last one -- so none of them exercise the `index == len(messages) - 1` guard that each renderer writes by hand to drop the terminator from the FINAL turn only. A family that lost that check would open every assistant turn in the history, and nothing in the suite would catch it. One family-agnostic test closes that gap for all seven at once. The open-turn shape is not uniform (qwen36 injects an empty <think></think>, GLM has no per-turn terminator, ChatML drops an <|im_end|>), so it does not pin bytes; instead it renders a two-assistant-turn conversation both ways and checks that the completed render and the open render share their whole prefix up to the final turn -- the second user turn must survive into their common prefix, which it cannot if the first assistant turn lost its terminator. The loop is driven by CONTINUATION_FAMILIES, so a family added later is covered the day it joins the set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…end-to-end The TrailingAssistantTurnTest cases pin the continuation prompt in isolation, and the fake-engine transcript tests drive Engine.generate() with a fixed prompt -- so nothing exercises the path a real request takes: HTTP body -> resolve_generation_prompt -> the open-turn render -> the engine -> the response. A serve() refactor could drop the continuation call and every existing test would stay green. This drives a /v1/chat/completions request whose last message is an assistant turn through the real handler and a fake engine (no weights), and checks both halves: the engine receives the continuation prompt (it ends on the client's opening, with no generation cue appended), and the generated text comes back as message content -- not reasoning_content, which also shows the continued turn primes the splitter into content mode over the wire. /v1/messages is a translation layer onto the same engine path, not a second one, so one endpoint covers both; streaming shares the same prompt-building and is left to the unit splitter test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The other families leave a turn open in the Python renderer, byte-diffed against the
checkpoint's chat_template.jinja. Kimi K3 frames turns engine-side -- render_chat_kimi hands
a length-framed K3CHAT1 record to kimi_k3.c, which assembles the XTML tokens -- so its open
turn is a change in the C path, not a string a template can pin.
The smallest signal that carries it is one new record. When continuation is on,
render_chat_kimi emits the trailing assistant turn as `C <reason-len> <text-len>` instead of
the usual M/A record. chat_build_wire renders that as the open turn: <|open|>message
role="assistant"<|sep|> [closed <|open|>think<|sep|>...<|close|>think<|sep|> if it carried
reasoning] <|open|>response<|sep|>{text}, and then appends NO close, no <|end_of_msg|>, and
no fresh generation cue -- the position the model occupies mid-turn. A guard keeps the open
turn last: nothing but the trailing G (thinking flag) may follow it.
Old gateway/engine combinations fail closed, never miswired. A new gateway's `C` record
reaches an old engine that has no such record: it falls through to the M branch, fails to
parse, and the payload is rejected. An old gateway sends no `C`, so a new engine renders as
before. (A flag overloaded onto the existing G record would instead be silently dropped by
an old engine, emitting a fresh turn over the client's opening -- the bug this removes.)
Pinned offline, at the token level, against the tiny tokenizer: tests/test_k3_chat_tools.c
gains a continuation case (final turn open, no cue) and two fail-closed cases (a turn after
the open turn, and a second open turn), and it is a Makefile-gated C test -- so unlike the
Python template scripts this runs in CI. `--wire-test` confirms the same on the engine
binary. What is NOT proven here is the 2.8T model resuming from those tokens: that needs the
checkpoint, the same render-proven / run-left-to-the-maintainer caveat the other six carry.
The cross-family open-turn guard (test_only_the_final_assistant_turn_is_opened) skips kimi:
render_chat_for_arch returns its K3CHAT1 wire, not a string prompt, so that string-level
invariant does not apply -- test_k3_chat_tools.c owns kimi's at the token level. With kimi in
CONTINUATION_FAMILIES, no shipped family is left as the pass-through example, so that backstop
test now exercises a hypothetical future arch.
Docs and the resolve_generation_prompt docstring that named Kimi as the pass-through exception are updated to match (docs/api.md, docs/ENVIRONMENT.md) -- the latter also stops equating continuation with add_generation_prompt=False alone, which drops the terminator too on the ChatML families.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e9789ff to
da69755
Compare
assistant turn instead of answering in a new one
|
Updated the branch to cover all eight families and enable continuation by default, as agreed in #1401. A trailing assistant message with nonempty text continues the supplied turn; The original GLM-5.3 commit changed from e9789ff to da69755 only to document the template reference and return exit code Kimi is included. Its gateway renderer sends a new The test coverage is:
Real-model validation remains limited to the previously reported GLM-5.3-Flash runs. I haven't run the other seven checkpoints, and the offline tests don't establish how those models generate from the supplied opening. Review of those family-specific prompt conventions would be especially useful. |
Summary
Resolves #1401.
A request ending in an assistant message now continues that message by default, instead of treating it as a completed turn and appending a new assistant generation cue. This supports resuming interrupted answers or supplying the beginning of an answer through an existing chat client, without a custom request field.
For example, a request ending in:
can return
**Paris**.to append to the supplied opening.Continuation covers all eight families: GLM-5.3, the base GLM renderer, Qwen3.8, Qwen3.6, OLMoE, DeepSeek V4, Inkling, and Kimi K3. It applies to streaming and non-streaming requests on
/v1/chat/completionsand/v1/messages. SetCOLI_CONTINUE_ASSISTANT=0to restore the previous behavior on both endpoints.Implementation
The gateway detects a trailing assistant message and renders its final turn open, without a fresh generation cue. Renderers with explicit turn terminators also omit the final terminator; earlier turns retain their existing rendering. GLM continuation only needs the generation cue suppressed.
Continuation requires nonempty assistant text and a preceding turn. It rejects trailing whitespace, tool definitions, and
tool_callson the final assistant message with a descriptive 400 response. The whitespace check prevents templates from silently changing the supplied ending, and the tool restriction preserves the parsers' assumption that they receive an assistant turn from its beginning.The reasoning splitter starts in content mode for a continued answer, so generated answer text is not incorrectly returned as
reasoning_content. This preserves #1278's GLM-5.3 starting-state rule for new turns.Kimi constructs its prompt inside the engine. The gateway therefore sends the final assistant turn as a new
Crecord in itsK3CHAT1payload, andkimi_k3.cleaves the response open without closing the turn or adding a new cue. The engine rejects any subsequent message record. Kimi continuation requires the updated gateway and engine together: an older engine rejects the new record. Ordinary requests retain the existing record format.Validation
make -C c checkThe added coverage includes:
glm53, using a fake engine; checks the submitted continuation prompt and the returned content/reasoning fields.The standalone template scripts require Jinja2 and downloaded reference files. Each records its reference provenance and returns exit code
2when skipped. The Kimi checks exercise engine-side framing; they are not an independent token-ID comparison against the checkpoint encoder.The latest targeted check passed all 14 continuation Python tests and the Kimi C test on b7593ee. The subsequent amendment to 3e07979 changes documentation and a docstring only. The five template scripts skipped in that check because Jinja2 was unavailable. These targeted results do not replace a full build and test run for the expanded stack.
Earlier GLM-5.3 validation
The original GLM-5.3 patch passed
make checkon macOS/arm64 and Debian 13/x86_64, the 26-case template comparison, and the token-exact oracle (32/32 teacher-forcing positions and 20/20 greedy tokens). Those results predate the family extension and Kimi C changes. The Debian baseline also emitted compiler warnings, so those results did not establish a zero-warning build.Previously reported model runs used GLM-5.3-Flash int4 on CPU with
--gpu none --ram 20 --ctx 32768, temperature 0.7, and top_p 0.9. Across both API endpoints, 36 sampled sentence/list/code openings continued successfully and four completed openings ended cleanly; all 40 returned no reasoning content.Those runs used v1.10.2 plus the earlier patch, before the rebase onto #1278, and all measured requests had thinking enabled. They are historical functional evidence, not measurements of the expanded stack. The old disabled Anthropic arm returned a 400 that this PR no longer introduces, so it is not a baseline comparison for the current behavior. The harness and recorded reports are available to share.
I can only run GLM-5.3-Flash with real weights. The other seven families have offline coverage, without model-generation validation; review of their prompt conventions remains especially useful.
Compatibility
Additional notes
COLI_CONTINUE_ASSISTANT=0restores their previous behavior, including accepting those messages without the continuation-specific checks.