[Jasen2201]

Problem

On ~1M-token prompts Kimi-K3 sometimes skips its think channel. The prompt opens <|open|>think<|sep|>, and the model writes its reply straight into that channel. It closes the reply with the answer's own closer, <|close|>response<|sep|><|close|>message<|sep|>, and never emits <|close|>think<|sep|> or <|open|>response<|sep|>.

Both delivery modes returned the whole reply as reasoning_content, with content empty. The Kimi Vendor Verifier's BEAM (1M) benchmark scores content only, so about one reply in five scored 0. Upstream vLLM shows the same thing on the same requests, so this is the model's behaviour, not an ATOM numerics problem:

BEAM 700, same requests	empty content	of those, ending in <|close|>response<|sep|>
ATOM	141	(reasoning not kept in that run)
upstream vLLM	144	141; the 556 answered replies have none

SGLang reads this shape too (sgl-project/sglang#37743): non-streaming returns it as content, while streaming re-sends what it already streamed as reasoning, so its two modes return different reasoning_content.

Change

A skipped channel is only known when the channel ends, after a streamed reply has already gone out as reasoning_content. This PR sends the reply again as content, and does it the same way on every endpoint and in both delivery modes.

One reader. The non-streaming split is ReasoningFilter run over the whole output (ReasoningChannel.split → ReasoningFilter.read_whole_events). The per-dialect split functions are gone (_split_channel, _split_inline_tag, _inline_tag_split, ReasoningDialect.split, SplitResult, strip_framing, separate_reasoning). The filter is chunk-invariant, so chat, /v1/messages and /v1/responses return the same reasoning and content, streamed or not.

When the channel was the reply. ReasoningDialect.answer_end_markers declares the answer's closer; for Kimi-K3 that is k3.RESPONSE_END. Inside the think channel, the filter drops the closer and remembers it. The channel's text is replayed as content when all of these hold:

• the closer was the channel's last words, ignoring whitespace and framing;
• the channel ends at the end of the output, or at a channel other than the answer's own (tools). An exit through <|close|>think<|sep|> or <|open|>response<|sep|> means the model answered in its own channel, so nothing is replayed;
• at the end of the output, the model stopped itself. Every streaming flush and every non-streaming read passes stopped=protocol.ended_by_stop(finish_reason) (eos, stop_sequence, stop_<id>). It is a required keyword, so a trace cut at max_tokens right after a quoted closer stays reasoning.

A closer the channel goes on past (a quoted one) changes nothing. The replay goes out in 4 KiB content events. The channel text is kept as one io.StringIO, only for a dialect that declares an answer closer, and dropped when the channel ends.

Thinking off. At offset 0, only the think exit means "declined to think". A <|open|>tools<|sep|> is a channel switch: it passes through as text, so the call keeps its opener.

MarkerScanner reads a whole output in linear time. This scanner is shared with the tool-call parser. It keeps one buffer and a cursor, and finds the next marker with one compiled alternation, longest first. That is the same leftmost-then-longest answer as a find per marker. A hit leaves the text after it in the scanner: feed("") reads on, and take_rest() hands it to a caller that routes it elsewhere. Scan no longer has rest. Handing the remainder back on every hit made one large feed quadratic in its marker count. On the merge base, /v1/messages took 295 ms on 460K characters with 2000 framing markers and 10.5 s on 819K with 32,768. It now takes 5.8 ms and 86 ms. On ordinary outputs both readers are 2-3x faster than before, and per-token streaming cost is unchanged.

Blocks. read_whole_blocks joins adjacent reasoning segments. A chain of thought that the filter splits at framing inside it is then one thinking block or Responses reasoning item, as streaming sends it.

The alternative to replaying was holding the channel back. It would stream no K3 reasoning until the channel closed, on every request, to avoid a copy on a shape seen at ~1M tokens.

A reply ended by EOS with no closer still reads as reasoning. It cannot be told apart from a trace cut off at max_tokens.

With thinking off, a K3 tools opener also moves the filter to the answer state, wherever it falls. It is forwarded unchanged, so a think marker inside the call stays in the argument. Markers inside arguments for the inline <think> dialects, and framing inside arguments, predate this PR and are tracked in #2974.

Tests

• test_reasoning.py, TestTheChannelEndsAtWhicheverCloserComesFirst, which pins split's (reasoning, content) for the K3 shapes:
  • the closer at the end;
  • the closer and framing;
  • the closer and whitespace;
  • the closer cut mid-marker;
  • the closer then the think exit;
  • a quoted closer;
  • a self-opened channel;
  • a call after the reply;
  • tools at offset 0 with thinking off;
  • a length cut versus a stop;
  • a replay in pieces;
  • ended_by_stop (in protocol.py, one stop set shared with openai_stop_reason) on the engine's raw reasons;
  • reasoning markers inside a K3 tool argument with thinking off;
  • the chat builder reading the finish reason.
• test_api_compatibility.py, test_the_reasoning_reader_is_told_whether_the_model_stopped: chat, /v1/messages and /v1/responses, streamed and not, through the real routes.
• test_stream_marker_properties.py, TestTheReasoningReadIsChunkInvariant: the split against 1/3/11-character streaming over every dialect's corpus. The corpus is generated from answer_end_markers and includes outputs over 4096 characters with a marker across 4095/4096.
• test_marker_scanner.py, TestOneLargeFeed: a whole feed reads what one-character feeds read, and neither copies the remainder nor searches the text twice. The old hand-back fails it.
• tests/entrypoints + test_required_channel.py: 2744 passed, 93 skipped, 3 xfailed. black 26.10.1 and ruff are clean.

Validation

Replay. The 700 raw upstream-vLLM BEAM outputs, with their channel tokens kept, run through this PR (stream and split agree at 1/5/64 characters on every output):

• The 141 replies that end in the answer's closer come back as content. Each ends in exactly <|close|>response<|sep|><|close|>message<|sep|>, by EOS.
• The 3 without it stay empty: one cut off by max_tokens, two EOS with no marker.
• The 556 answered replies are byte-identical to vLLM's content.

The GPU result below was measured on the first version of this change. Every later revision gives the same content and reasoning_content on all 700 replayed outputs. KVV's BEAM request is non-streaming and scores content only, so the result carries over.

On GPU. Kimi-K3 on 8x MI355X with the #4311 serving config (DSpark 4, FP8 KV, ptpc_fp8). The client sends KVV's own BEAM request: thinking_effort=max, temperature 1.0, top_p 0.95, non-streaming. Kimi-K3 is the judge, since KVV names none.

BEAM (1M), 700 questions	empty content	score
ATOM main	162	0.246
ATOM main + this change	10	0.320
upstream vLLM	144	0.263
KVV leaderboard (Moonshot / vLLM ref. / SGLang ref.)	-	0.31 / 0.3188 / 0.3107

The 10 left are:

• 6 EOS inside the think channel with no closer;
• 2 cut off at max_tokens;
• 2 where the request declared no tools but the model opened the tools channel after its reply.

Paired question by question against vLLM's answers, the replies themselves score the same (+0.003 ± 0.022 on the questions both engines answered). So the BEAM gap was this parsing alone.

Tool calls. The KVV tool_call_json_schema pre-flight passes all 408 cases with this change, the same as without it.

🤖 Generated with Claude Code

[github-actions]

🏷️ CI Guide

Runs automatically on every eligible PR before approval:

• ✅ Pre Checkin: Black, Ruff, catalog schema validation, non-GPU unit tests

Heavy model tests:

• ✅ Run after the PR is approved and Pre Checkin passes
• ✅ Run immediately when an approval review is submitted
• ✅ Can be requested before approval with labels

Label	Tests
ci:full	Run all heavy PR model tests: native ATOM, vLLM, and SGLang
ci:atom	Run native ATOM model accuracy tests
ci:vllm	Run ATOM vLLM OOT model accuracy tests
ci:sglang	Run ATOM SGLang model accuracy tests

Heavy jobs are skipped when the PR is not approved and no matching ci:* label is present.
Add labels via the sidebar or gh pr edit 2910 --add-label <label>

[Copilot]

🟢 Approval recommended

No active review issues were identified for the Kimi-K3 reply-parsing changes.

No open issues 🎉

What changed in this PR

This PR fixes Kimi-K3 reply parsing when the model writes an answer directly into its think channel, ensuring the reply reaches content in both delivery modes.

Changes:

• Declares answer-end markers in reasoning dialects and validates that they are also content framing.
• Updates non-streaming and streaming parsing to recognize skipped think channels, while preserving already-streamed reasoning.
• Adds regression coverage for chunk boundaries, tool calls, normal reasoning, and truncated output.

File	Description
tests/​entrypoints/​test_k3_skipped_think_channel.py	Tests skipped-channel recovery and preservation of existing behavior.
atom/​entrypoints/​openai/​reasoning.py	Replays streamed text as content when an answer closer identifies a skipped channel.
atom/​entrypoints/​openai/​reasoning_dialects.py	Defines and validates answer-end markers and updates non-streaming splitting.

_{Review effort: Balanced}

[valar-cop]

a few small things..

ci green.

[valar-cop]

this holds one str per token for the whole trace, even when no skip happens. coalesce every N chunks?

[Jasen2201]

Fair point. The replay needs the text itself either way; what the list adds is one str object per chunk. Measured on 30K chunks of ~14 chars, the list holds 1.90 MB against 0.42 MB of text (an io.StringIO would be 0.44 MB). So it is about 1.5 MB extra per long K3 trace, held only during state 1 and released as soon as the channel closes the ordinary way. It only applies to dialects that declare answer_end_markers, which today means K3 alone. I'd rather keep this PR as is and switch to io.StringIO as a follow-up, unless you'd like it here.

[valar-cop]

this is a no-op since answer_end_markers are already in framing, drop it or leave a comment?

[Jasen2201]

Right, it's redundant: watched starts from _framing, and __post_init__ requires answer_end_markers to be a subset of content_framing. I kept it on purpose so that this line spells out everything state 1 watches. If the framing check is ever relaxed, the skipped-channel branch still sees its marker instead of going silent.

[Jasen2201]

@valar-cop review the pr

[valar-cop]

@Jasen2201 already reviewed df236b1: #2910 (review) say @valar-cop re-review if you want a fresh one.

[valar-cop]

ci triage for b4e72a3:

ATOM SGLang Test passed. https://github.com/ROCm/ATOM/actions/runs/41662381924

ATOM vLLM Test passed. https://github.com/ROCm/ATOM/actions/runs/41662382047

ATOM Test passed. https://github.com/ROCm/ATOM/actions/runs/41662381892

[Jasen2201]

ci triage for df236b1:

ATOM vLLM Test passed. https://github.com/ROCm/ATOM/actions/runs/41560287699

ATOM SGLang Test passed. https://github.com/ROCm/ATOM/actions/runs/41560287825

ATOM Test / Offline inference smoke test failed — not sure yet (low): The job log is completely empty and the failing step is not reported, so there is no evidence of what actually failed. The same job passes consistently on main (8/8 recent runs), and the PR touches the OpenAI reasoning entrypoint plus a new test, so a PR-caused break is plausible but unproven.

Full job log: (empty — no log output was captured for this job)
Failed step(s): ? (not reported by the workflow)
Infra signatures matched by regex: none
next: Rerun the 'Offline inference smoke test' job to obtain an actual log; if it fails again, capture the failing step and server/test output. If the rerun is clean, treat as transient runner/log-upload issue and ping the CI owner about runner linux-atom-mi35x-1-hstps-runner-sqd42 not producing logs. https://github.com/ROCm/ATOM/actions/runs/41560287785/job/125675282810

done

[valarLip]

Thanks for tracking this to the model's own behaviour; the BEAM replay makes the case clearly. I reviewed df236b1 by reading the code and running the affected tests (1265 passed, 29 skipped). I also ran a CPU probe through the real split and filter.

The blocker is the divergence the description accepts. Streaming and non-streaming must return the same reasoning and content for the same output, at every chunking, with no exceptions. That is the contract ReasoningChannel exists to keep, and the docs promise it. Please rework this so that no case diverges.

Must change

1. The divergence is not limited to streamed reasoning_content. Today the three endpoints disagree with each other.

The probe used the PR's own input: The answer is 42.<|close|>response<|sep|><|close|>message<|sep|>.

• Non-streaming /v1/chat/completions (the split): (None, "The answer is 42.").
• Non-streaming /v1/messages and /v1/responses: these read the whole output through the streaming filter (read_whole_blocks, api_server.py:2490 / :2829). They return [thinking("The answer is 42."), text("The answer is 42.")], so the reply is delivered twice. /v1/responses always carries the reasoning item, so a Codex multi-turn conversation feeds the reply back as reasoning and as message.
• Streaming: reasoning_content "The answer is 42." in pieces, then content "The answer is 42." again.

About one BEAM reply in five takes this path.

The way out is a single definition of the result. Today there are two: the split (_split_channel) and the filter (ReasoningFilter.process re-sending state 1 as content, reasoning.py:296). As long as both exist, a rule added to one has to be re-derived in the other, and they drift; this PR is that drift. Pick one rule that the stream can actually deliver, then make every path produce exactly that. There are two choices, and each has a cost:

• Hold back. The filter keeps think-channel bytes until the channel is decided, so nothing is ever re-sent. The cost is that the chain of thought is no longer streamed live for this dialect. If you go this way, please measure what it does to time-to-first-reasoning-token.
• The streamed shape is the answer. The reply appears as reasoning and then as content in every path, including the non-streaming chat split. It is duplicated, but identically everywhere, and content is still filled for BEAM.

Whichever you pick, the non-streaming split should come from the same code as the stream, so that the two cannot differ by construction.

2. The new shapes have to go through the existing consistency gate, with no exemption. TestTheReasoningSplitAgreesWithItself builds its corpus from each dialect's declared markers and compares split and stream byte for byte. It does not read answer_end_markers, so it never sees this shape, and adding the shape would fail test_the_chain_of_thought_is_the_same_too today. Please do the following:

• make reasoning_shapes(dialect) generate the answer-end shapes from dialect.answer_end_markers;
• delete tests/entrypoints/test_k3_skipped_think_channel.py. Its streamed() duplicates split_reasoning_streaming (test_stream_marker_properties.py:753), and its K3 fixture duplicates the one in test_reasoning.py:269;
• move the remaining cases into test_reasoning.py (TestTheChannelEndsAtWhicheverCloserComesFirst) on the existing fixture.

3. A quoted closer turns the whole chain of thought into content (reasoning_dialects.py:210). If the chain of thought contains the literal text <|close|>response<|sep|>, the split treats the channel as skipped. The probe input closes with <RE> so<|close|>think<|sep|><|open|>response<|sep|>ok<RE> gives (None, "closes with so<|close|>think<|sep|>ok"), so the chain of thought and a raw think marker reach the client as content. Before this PR it gave ("closes with so", "ok"). test_a_quoted_token_does_not_open_a_channel_that_was_closed guards the same class of problem for openers; this needs the same treatment, decided on token ids, not text.

Worth addressing

• Memory (reasoning.py:277). Every K3 request keeps its entire chain of thought, chunk by chunk, in _released_in_channel until think closes. That is O(CoT) resident memory per request, times concurrency and n, for a rare event. It is held to the end on a max_tokens-truncated trace. With the hold-back rule there is nothing to keep. With the other rule, nothing needs to be kept past the current call.
• Docs. docs/serving_benchmarking_guide.md:149 ("What is held" and the dialect paragraph) needs the answer-end rule and its cost, whichever you pick.
• Comments. About 15 lines of code carry about 45 lines of comments. The BEAM / KVV numbers and the SGLang PR reference appear four times: the field comment (reasoning_dialects.py:103), the _split_channel docstring, the 14-line comment in process() and the test module docstring. They belong in the description. The code needs one constraint line. Also, _split_channel's docstring opens with "One rule" and is immediately followed by "Unless".
• __post_init__ check (reasoning_dialects.py:116). This validates one module constant at import time, plus a dedicated fake-dialect test. The registry invariants are asserted in TestTheDialectsAsRegistered (test_reasoning.py:463), which takes one line here. Better still, if the split drops the answer-end marker itself, as the filter does, the "must also be framing" coupling goes away.
• reasoning.py:330 clears _released_in_channel on the 0→1 transition too, where it is always empty. The two self._answer_end_markers checks state what the list's emptiness already says.

[Jasen2201]

@valarLip Thanks for the careful review. Reworked in cbc0766.

How SGLang handles this (sgl-project/sglang#37743, KimiK3Detector):

• A <|close|>response<|sep|> that appears before every think exit marks the channel as skipped. The check is on text and is not anchored to the end.
• Non-streaming returns the text as content, with empty reasoning.
• Streaming re-sends what it had already streamed as reasoning as content. The streamed reasoning keeps its copy; their test only asserts it is a prefix of the reply.
• With stream_reasoning off, it holds the channel back instead.

So SGLang has both problems you raised: its streaming and non-streaming modes disagree, and a quoted closer flips the channel. The previous head copied that design, divergence included.

I think your rule is the more reasonable one, so this head follows it. It takes your second option, the streamed shape is the answer, with every path on one reader:

1. One definition.
   • separate_reasoning is now ReasoningFilter run over the whole output in one chunk.
   • Deleted: _split_channel, _split_inline_tag, _inline_tag_split, ReasoningDialect.split, SplitResult and strip_framing.
   • The filter is chunk-invariant: 0 differences over 120,000 random marker strings, at 1/2/3/5-character and whole-output chunks, for every dialect, seeded and not. So chat, /v1/messages and /v1/responses, streamed or not, agree by construction.
   • Your probe now returns reasoning The answer is 42. and content The answer is 42. on all three endpoints, in both modes.
   • One side effect: on main the two readers already disagreed for K3 with thinking off when the output opens with <|open|>tools<|sep|>, or with <|close|>think<|sep|> after framing. Non-streaming now matches what streaming did. The K3 tool parser still finds the call by its <|open|>call tool=" prefix.
   • I didn't take hold-back because the skip is only known at the channel's end. Holding back would mean no K3 reasoning streams until the channel closes, on every request, for a shape seen at ~1M tokens.
2. Consistency gate.
   • reasoning_shapes generates the answer-end shapes from dialect.answer_end_markers: the closer at the end, followed by each framing marker, in a self-opened channel, and a closer the channel goes on past. The parity sweep runs on them with no exemption.
   • test_k3_skipped_think_channel.py is deleted; its cases are in TestTheChannelEndsAtWhicheverCloserComesFirst on the existing fixture.
3. Quoted closer.
   • The closer counts only when it is the channel's last word, framing aside: followed by the end of the output or the next channel. Your probe reads as ("closes with so", "ok") again.
   • On token ids: agreed that text can't tell a quoted <|close|>response<|sep|> from a structural one. <|open|>/<|close|>/<|sep|> are non-special added tokens, and the same string can be spelled with ordinary tokens. But ids answer a different question: a structural closer that the channel goes on past still did not end the reply. Position answers the question that matters here, and it doesn't need ids in a layer that is text-only today.
   • On the 700 upstream-vLLM BEAM outputs, all 141 skipped replies end in exactly <|close|>response<|sep|><|close|>message<|sep|>, and none of the 556 answered replies has the closer inside its reasoning. So anchoring loses nothing.

Worth addressing:

• Memory. The channel text is one io.StringIO, kept only for a dialect with an answer closer, and set to None when the channel ends (it never reopens). That is about 1x the channel text: 0.42 MB for 30K chunks of ~14 chars, against 1.90 MB as a list. It can't be zero with this rule, because the copy has to come from somewhere when the channel ends; hold-back would hold the same bytes.
• Docs. The holding section of serving_benchmarking_guide.md now states the rule and its cost, and that the split is the filter.
• Comments. The BEAM/KVV numbers and the SGLang reference live only in the description now.
• __post_init__ and its fake-dialect test are gone. With one reader dropping the marker, there is no coupling left to check.
• The 0→1 clear and the redundant _answer_end_markers checks are gone.

BEAM is unaffected. On the 700 raw outputs, content is byte-identical to df236b1, so the 0.320 carries over. The only change is that non-streaming reasoning_content now also carries the reply on those 141. tests/entrypoints: 2610 passed; black 26.10.1 and ruff are clean.

[Copilot]

🟡 Changes recommended

Processing the entire response causes quadratic parsing for marker-rich answers and blocks the API event loop, so bounded chunks and a single flush are needed.

1 open issue

• Whole-response processing causes quadratic parsing on marker-rich answers · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The full-reply parser exhibits quadratic behavior on repeated literal end tags, synchronously blocking the event loop and delaying other requests.

1 open issue

• Whole-response processing causes quadratic parsing on marker-rich answers

_{Review effort: Balanced}

[valarLip]

Re-read at 57d4f62. I ran the affected tests (1364 passed, 29 skipped) and a CPU A/B of ReasoningChannel.split against the base, about 60k marker combinations at chunk sizes 1, 7 and whole output. Thanks for taking the one-reader route. For <think> and <mm:think> the new reader is byte-identical to the base, the quoted-closer case reads ("closes with so", "ok") again, and a 400 KB non-streaming split got faster (K3 0.436 → 0.399 ms, <think> 0.216 → 0.115 ms). I'm not re-raising the channel-text memory, which you've answered.

Three K3 behaviours still differ from what they should be. Two come from the "last words" anchor I pushed for last round, so they're partly on me. After those come the cleanups the single reader makes possible. Each item ends with the change I'm asking for.

A. Correctness

1. A chain of thought that ends with the answer closer, followed by a proper think exit, is replayed into content (reasoning.py:300-313). _close_channel runs on any end marker, including the ordinary think exit <|close|>think<|sep|> and <|open|>response<|sep|>. Take this K3 output (starts_open=True):
CoT ...<|close|>response<|sep|><|close|>think<|sep|><|open|>response<|sep|>Final answer.<|close|>response<|sep|><|close|>message<|sep|>
It gives content CoT ... Final answer. at chunk sizes 1, 7 and whole output; the base gave ("CoT ...", "Final answer."). Concretely, a model that drafts its reply in the think channel, exits normally and writes it again gets the reply twice in content. The quoted-closer test doesn't cover this because it has text between the closer and the exit.
→ Change: replay only when the channel ends at the end of the output, or at a channel that isn't the think exit (e.g. tools). An exit through think_end_marker / <|open|>response<|sep|> means the model did answer in the response channel, so nothing is replayed. Add this shape, with its expected (reasoning, content), to TestTheChannelEndsAtWhicheverCloserComesFirst.

2. One whitespace byte after the closer defeats the anchor (reasoning.py:262-266). Any released byte in state 1 clears _answer_closed. So Reply.<|close|>response<|sep|>\n<|close|>message<|sep|>, or the same with a trailing \n, still gives ("Reply.\n", "") at every chunk size. The 141 BEAM samples happen to have no whitespace there, so the tests can't see it.
→ Change: treat whitespace-only releases like framing when deciding whether the closer was the channel's last word (keep the flag set and keep the whitespace in the channel text), and add both shapes to the tests.

3. With thinking off, a leading tool call loses its opener (reasoning.py:285-292). The offset-0 "declined to think" rule matches every end marker, including the extra <|open|>tools<|sep|>. Since non-streaming now goes through the filter, a thinking-off K3 output that opens with <|open|>tools<|sep|><|open|>call tool="get_weather"...<|close|>tools<|sep|> has its tools opener consumed, leaving <|close|>tools<|sep|> unbalanced. You noted the K3 parser still finds the call by its prefix (confirmed). But where no K3 tool parser is resolved, the client gets raw call tokens without their opener, and the base kept them intact. It accounts for the tools-at-offset-0 and framing-then-tools differences in my A/B (~2.1k cases).
→ Change: let the offset-0 rule recognise think_end_marker only. An opener such as tools is a channel switch, not "declined to think", so it should be released like any other text in state 0.

B. Cleanups the single reader makes possible

4. The "whole output through the filter" loop exists three times (reasoning.py:353-358). It appears in ReasoningChannel.split's literal-answer branch, in separate_reasoning (whose only production caller is split), and in serving_anthropic.py:231 read_whole_blocks. Copilot's quadratic finding on feeding the whole output as one chunk applies to all three.
→ Change: one helper, ReasoningFilter.read_whole(text) (or ReasoningChannel.events(text)), that feeds bounded chunks and flushes once. Have split and read_whole_blocks use it. Delete separate_reasoning, or reduce it to a one-line delegate.

5. The parity test now compares the filter with itself (tests/entrypoints/test_stream_marker_properties.py:855). With one reader, which is what I asked for, TestTheReasoningSplitAgreesWithItself checks chunk invariance only. Its whole-text chunk (10 000) runs exactly the computation split does on every corpus string, and its docstring still says "both readers". test_the_messages_and_responses_bodies_read_it_the_same is filter against filter too. Item 1 passes all of it at every chunk size. The 120k-string sweep in the description isn't committed, so it doesn't gate anything.
→ Change: rename the class to a chunk-invariance test and drop the self-comparing whole-text chunk and the tautological endpoint test. Add pinned expected (reasoning, content) for the K3 answer-end shapes: closer at end, closer + framing, closer + think exit (item 1), closer + whitespace (item 2), and tools at offset 0 (item 3).

6. Redundant state (reasoning.py:265, :311, flush). The channel never reopens (state 2 doesn't return to 1), so self._answer_closed = False in _close_channel is dead. The "released text clears the flag" rule is written in process() and again in flush() (if rest: self._answer_closed = False). The _channel_text is not None checks restate what an empty answer_end_markers already says.
→ Change: decide "did text follow the closer" in one place (at the scanner's release point, which also gives item 2 its one home), and delete the dead reset and the second copy in flush().

7. The answer closer has two sources (reasoning_dialects.py:150). answer_end_markers=(CHANNEL_RESPONSE_END,) uses this module's local string, while the line above uses k3.TOOLS_START. That the closer counts as framing now depends on k3.CHANNEL_FRAMING happening to contain the same string.
→ Change: use k3.RESPONSE_END. While there, delete the local CHANNEL_TOOLS_START / CHANNEL_MESSAGE_END / CHANNEL_END_OF_MSG / CHANNEL_CALL_PREFIX, which have no references.

8. Docs and comments describe the deleted two-reader structure, or a change log (reasoning_dialects.py:6, docs/serving_benchmarking_guide.md:359):

• the module docstring says it "iterates DIALECTS to detect/split";
• the extra_end_markers comment says "while the non-streaming split read it correctly";
• ReasoningFilter's docstring talks about "keeping the two paths on one answer";
• separate_reasoning's docstring and the new guide paragraph ("Both are now one reader ... the per-dialect split functions ... are gone") are history.

→ Change: describe what the code does now (one reader; the split is the filter run over the whole output) and move the history to the commit message.

[valar-cop]

a few small things..

ci green.

[valar-cop]

held suffix is a marker prefix, not model text.. so content ends up empty here? keep it out of the replay and only clear _answer_closed when it's not a prefix

[Jasen2201]

Right: the held suffix is a marker's first bytes, not model text, and it cleared the flag, so Reply.<|close|>response<|sep|><|close|>mes came back with empty content. Fixed in b4e72a3. flush() no longer touches the flag, and the held text never enters the replay. Whether text followed the closer is now decided only in _keep, at the scanner's release point, and whitespace doesn't count either. That case now reads ("Reply.<|close|>mes", "Reply."), pinned as cut-mid-marker.

[valar-cop]

/v1/messages drops reasoning unless thinking is requested, so content only there.. scope this sentence?

[Jasen2201]

Scoped in b4e72a3: on /v1/messages the reply is in content alone unless thinking was requested, since that endpoint drops reasoning otherwise.

[Jasen2201]

@valarLip Thanks for the re-review. All eight points hold up; I reproduced each with a CPU probe before changing anything. Fixed in b4e72a3.

A. Correctness

1. Closer followed by the think exit. The closer marks a skipped channel only when the channel ends at the end of the output, or at a channel other than the answer's own (tools). An exit through <|close|>think<|sep|> or <|open|>response<|sep|> replays nothing. In the filter, those are the end markers that are either the think exit or answer framing (_exits_to_answer). Your shape now reads as ("CoT ...", "Final answer.") at every chunking. It is pinned for both exits in test_a_closer_then_the_exit_into_the_answer_is_not_the_reply.
2. Whitespace after the closer. Whitespace-only releases keep the flag set and stay in the channel text. Both of your shapes now give ("Reply.\n", "Reply.\n"). valar-cop's held-suffix case is the same rule: a marker cut off at the end of the output is not words, so it neither clears the flag nor enters the replay. All three are pinned.
3. Tools opener at offset 0 with thinking off. In state 0 the filter now watches only think_end_marker, so <|open|>tools<|sep|> is released as text and the call keeps its opener. One note: on main this was already the streaming behaviour (chunk 1 and 7 dropped the opener), and only the non-streaming split kept it. So this also fixes the streamed side.

B. Cleanups

4. ReasoningFilter.read_whole(text) feeds 4 KiB pieces and flushes once. ReasoningChannel.split and read_whole_blocks use it, and separate_reasoning is a one-line delegate (tests still call it). The quadratic case: 819K characters with 32,768 markers took 31.7 s at 57d4f62 and now takes 174 ms. A normal 800K output takes 0.5 ms. Smaller pieces only get the worst case down to about 115 ms, because the per-marker filter cost is the floor there. The streamed path pays the same per-marker cost.
5. The class is now TestTheReasoningReadIsChunkInvariant, at 1/3/11 against the split. The whole-text chunk and the self-comparing endpoint test are gone. reasoning_shapes also generates closer + immediate exit and closer + whitespace. Pinned (reasoning, content) cases cover closer at the end, closer + framing, closer + whitespace (two shapes), closer cut mid-marker, closer + think exit (two exits), tools at offset 0, the self-opened channel and a call after the reply. Each is checked through the split and through the stream at 1 and 7 characters.
6. "Did text follow the closer" is decided once, in _keep at the release point. The dead reset in _close_channel and the duplicate in flush() are gone. _close_channel no longer re-checks _channel_text is not None.
7. answer_end_markers=(k3.RESPONSE_END,). The unused CHANNEL_RESPONSE_END, CHANNEL_TOOLS_START, CHANNEL_MESSAGE_END, CHANNEL_END_OF_MSG and CHANNEL_CALL_PREFIX are deleted.
8. The module docstrings, the extra_end_markers comment, the ReasoningFilter docstring and the guide now describe the single reader as it is. The history moved to the commit messages. The guide also states the new replay condition and the /v1/messages scope.

BEAM is unchanged. I replayed the 700 raw upstream-vLLM BEAM outputs through 57d4f62 and b4e72a3. Both content and reasoning_content are identical on all 700: 141 recovered, 3 still empty, 556 answered unchanged. Stream and split agree at 1/5/64 characters. None of the 700 has the A1 or A2 shape: every closer is followed by exactly <|close|>message<|sep|>, and no answered reply has a closer in its think channel. So the GPU result (0.320) carries over.

tests/entrypoints: 2651 passed, 93 skipped, 3 xfailed. black 26.10.1 and ruff are clean.

[Copilot]

🟡 Changes recommended

Bounded reads split long reasoning traces into separate blocks and Responses items sharing the same ID, creating client ambiguity that needs coalescing and regression coverage beyond 4 KiB.

1 open issue

• Coalesce adjacent reasoning fragments in non-streaming output · New

1 resolved since last review

• Whole-response processing causes quadratic parsing on marker-rich answers

_{Review effort: Balanced}

[valar-cop]

some changes needed..

ci green.

[valar-cop]

4 KiB pieces split one reasoning run into many blocks/items with the same id, streaming keeps one. coalesce adjacent reasoning events in read_whole_blocks and add a >4 KiB test shape?

[Jasen2201]

Fixed in fda4bcf, in two ways. The 4 KiB pieces are gone: MarkerScanner now reads a whole output in one feed in linear time. And read_whole_blocks joins adjacent reasoning segments into one event, which matters independently of the pieces. On main, framing inside a chain of thought (aa<|end_of_msg|>bb) already produced two thinking blocks / reasoning items. A 9,000-character trace and that framing case now each give one, as streaming does. Both shapes are in the tests, including outputs over 4096 characters with a marker across 4095/4096.

[valar-cop]

this comment is stale now, read_whole feeds bounded pieces. update it so it matches how this path reads the output?

[Jasen2201]

Updated in fda4bcf. It is now read_whole_events, and it reads the whole output in one feed again: the quadratic cost was fixed in MarkerScanner (one buffer and a cursor, no hand-back), so the bounded pieces are gone.

[valarLip]

Re-read at b4e72a3, by reading the code, running tests/entrypoints (2750 passed; the one failure is test_structured_output needing llguidance, unrelated), and two probes against main, alternated (split cost and per-token streaming cost). Running the streaming filter over the whole output and deleting the per-dialect splits is the right direction, so keep it. The quoted-closer, whitespace and tools-at-offset-0 fixes read right.

Two of the items below come from my own last two rounds (the bounded pieces, and treating whitespace as framing), so they're partly on me. Each item ends with the change I'm asking for.

A. Correctness

1. A trace cut off right after a quoted closer is replayed as content (reasoning.py:298). With starts_open, the input I will end with <|close|>response<|sep|>\n ending at EOF (e.g. max_tokens) returns ('I will end with \n', 'I will end with \n'), while main returns content ''. Since whitespace now counts as framing, which I asked for, a closer followed only by whitespace at EOF looks like a finished reply. So a chain of thought truncated at that point is sent again as content, which contradicts the description's "a max_tokens-truncated trace stays reasoning".
→ Change: at end of output, replay only if the sequence ended normally (stop / EOS), not on a length cut. Pass the finish reason into flush (e.g. flush(stopped: bool)), and add both shapes, stopped and length-cut, to the K3 cases.

B. Performance

2. Fix the quadratic scan in MarkerScanner, not by chunking in one reader (reasoning.py:38, _WHOLE_OUTPUT_PIECE = 4096). I asked for bounded pieces last round, but that was the wrong layer. MarkerScanner.feed runs find for every marker over the buffer and copies rest on each hit, so one large feed is quadratic in the marker count. The 4096 piece works around that in the reasoning reader only, while tool_parser.stream.read_whole still feeds the whole output at once. It also costs on marker-dense outputs: split over 460k chars with 2000 framing markers takes 3.98 ms here against 0.68 ms on main (0.09 vs 0.24 ms with no markers). The pieces also cause the multiple same-id reasoning blocks / items that Copilot and valar-cop flagged.
→ Change: make feed scan by offset without copying the remainder (keep one buffer and a cursor; search only from the cursor). Then delete _WHOLE_OUTPUT_PIECE and feed the whole output in one call, so the block / item split goes away and both read_whole paths benefit.

3. Streaming replay sends the whole channel as one content event (reasoning.py:316). For a 100k-character reply the probe sees one content event of length 100 000. That's one pass through the tool parser on the event loop and one oversized SSE frame.
→ Change: emit the replayed text in bounded pieces (this is where a size bound belongs), yielding to the loop between them.

4. Per-token streaming cost for every K3 request (reasoning.py:206). The think channel is written into a StringIO and every released piece gets an isspace check, for a shape seen near 1M tokens. A quick alternating A/B on the K3 think channel (one run each, noisy): main 727 / 751 ns per token, this head 819 / 809 (about +10%), plus a full copy of the chain of thought held per concurrent request.
→ Change: decide without keeping the text. Track only where the last closer ended and whether anything non-framing followed it. At replay, rebuild the channel text from the output the caller already holds (streaming already has it in the request's accumulated text), and drop the StringIO.

C. Cleanups

5. separate_reasoning has no production caller left (reasoning.py:138). Only tests call it, and test_stream_marker_properties' SEEDED list maintains rules for it.
→ Change: delete it and point those tests at ReasoningChannel.split.

6. Two read_whole with different contracts (reasoning.py:327 vs tool_parser/stream.read_whole). One returns an event list, the other (content, calls), and serving_anthropic uses both.
→ Change: rename the reasoning one read_whole_events, matching tool_parser.read_whole_events.

7. Two sources for the K3 tokens (reasoning_dialects.py:141-143). One line mixes local CHANNEL_RESPONSE_START with k3.TOOLS_START, and the next uses k3.RESPONSE_END. Three local CHANNEL_* constants still duplicate kimi_k3_tokens.
→ Change: use k3.* throughout and delete the remaining local CHANNEL_*.

8. Comments that point at removed text or describe the old read.

• reasoning_dialects.py:92 points to separate_reasoning for a rule whose explanation was removed with SplitResult.
• api_server.py:2604 still says "Both stages over one chunk", though serving_anthropic.py's copy of that sentence was updated.

→ Change: point the first at ReasoningChannel.split, and make the second match how the output is now read (after item 2, "the whole output, once").

9. Tests: a duplicated sweep, and no chunk-boundary case.

• tests/entrypoints/test_reasoning.py:321 _every_way hand-writes chunked streaming against split again; test_stream_marker_properties already sweeps chunk invariance, and the answer-end shapes are in its corpus.
• test_stream_marker_properties.py:877 dropped the 10 000 chunk, and every corpus string is far shorter than 4096, so nothing covers a marker straddling a piece boundary.

→ Change: keep test_reasoning.py to asserting split's results. Add one long case (over 4096 chars, with a marker across 4095/4096) to the sweep, which still matters for the streaming feed after item 2.

10. The guide restates the code comments (docs/serving_benchmarking_guide.md:155). The two new paragraphs re-describe _close_channel, _keep and the dialect field almost line by line, including "~1M tokens" and the memory cost, so the rule now lives in three places.
→ Change: keep one sentence: a dialect that declares answer_end_markers replays a skipped channel as content. Leave the details in the code.

[Jasen2201]

@valarLip Thanks. Taken in fda4bcf. Each point is below, with what I measured. CPU numbers compare the merge base 825b63b ("main") with this head, as medians.

A. Correctness

1. Length-cut after a quoted closer. flush(stopped=...) and split(text, stopped=...) now take the finish reason. At the end of the output, a skipped channel is replayed only if the model stopped itself (ended_by_stop: eos, stop_<id> or stop_sequence). Every streaming flush (chat, n>1 chat, /v1/messages, /v1/responses, the atomesh standalone service) passes the engine's reason, and so does every non-streaming read (chat split, both read_whole_blocks callers). Your input now reads ('I will end with \n', '') when cut and ('I will end with \n', 'I will end with \n') when stopped. Both shapes, with and without the trailing whitespace, are pinned in test_a_closer_at_a_length_cut_is_not_the_reply.

B. Performance

2. Fixed in MarkerScanner. The pieces are gone. The scanner keeps one buffer and a cursor. After a hit, the text past it stays in the scanner: feed("") reads on, and take_rest() hands it over when a tool region opens or the reasoning state changes. One note on "search only from the cursor": with a find per marker, a marker that is absent still scans to the end of the buffer on every hit, so that alone stays quadratic. The scanner instead uses one compiled alternation of the markers, longest first. That is the same leftmost-then-longest answer as a find per marker, and each search stops at the next marker. Scan loses rest and is a NamedTuple. Both whole-output readers now read in one feed.

   output	chat non-stream (split + parse_tool_calls), main → now	/v1/messages (read_whole_blocks), main → now
   460K chars, 2000 framing markers	0.81 → 3.1 ms	295 ms → 5.8 ms
   819K chars, 32,768 framing markers	4.6 → 45 ms	10.5 s → 86 ms
   460K content, no markers	0.47 → 0.21 ms	0.71 → 0.21 ms
   460K CoT + short answer	0.60 → 0.21 ms	0.56 → 0.20 ms

   On main the whole-output tool-parser read was already quadratic, which is the /v1/messages column. Main's chat column looks cheap on marker-dense content because main's K3 split did not drop framing in content. One gap is left: about 1 µs per marker hit. That is the Python call per step: the scanner loop alone is 2.4 ms for those 2000 hits, against 0.6 ms for one re.sub over the same text. Going below it would take a separate whole-output path with its own semantics, which is the two-reader shape this PR removes. So I kept the one reader, which is linear everywhere and faster than main on ordinary outputs. test_the_text_is_neither_copied_nor_searched_twice fails on the old hand-back.

3. Replay in bounded pieces. The replay goes out as 4 KiB content events, so a 100K reply is 25 events, each through the tool parser and its own SSE frame. The filter is synchronous, so the pieces are bounded but it does not yield between them itself. The endpoints' generators yield one frame per event.

4. Per-token cost. I kept the text, because there is nothing to rebuild it from. The streaming endpoints hold no accumulated output text. The only per-request state is the detokenizer's token ids (IncrementalStreamDetokenizer.tokens), and each loop sees one delta. Rebuilding would mean decoding the channel's token span again in the dispatch layer. What I changed is the per-token work: isspace runs only after a closer has been seen, so the common case is one StringIO.write. Through the K3 think channel this measures 1280 ns per token, against 1311 and 1353 ns on main in two runs. Filter plus tool parser streaming is 2.54 µs per token against 2.86 µs on main. The kept text is one string of the chain of thought, and only for a dialect that declares an answer closer.

C. Cleanups

5. separate_reasoning is deleted. Its tests use ReasoningChannel.split, and the seeding scan now checks ReasoningChannel/ReasoningFilter, including ReasoningChannel's positional seed.
6. Renamed to ReasoningFilter.read_whole_events.
7. The K3 dialect reads every token from kimi_k3_tokens. The remaining local CHANNEL_* constants are deleted.
8. The reasoning_dialects.py pointer now goes to ReasoningChannel.split, and api_server.py's comment says "over the whole output, once".
9. The K3 cases in test_reasoning.py assert split only; _every_way is gone. reasoning_shapes adds outputs over 4096 characters with the marker across 4095/4096: the think exit, a self-opened channel and the answer closer, for every dialect.
10. The guide keeps one sentence and points to _close_channel.

Copilot's and valar-cop's block split is fixed too. Removing the pieces fixes the 4 KiB case, and read_whole_blocks also joins adjacent reasoning segments. That matters because main already split a chain of thought into two thinking blocks / Responses items wherever framing sits inside it (aa<|end_of_msg|>bb gave ['aa', 'bb']).

BEAM is unchanged. The 700 raw upstream-vLLM outputs, replayed through b4e72a3 and fda4bcf, give identical content and reasoning_content: 141 recovered, 3 empty, 556 unchanged. All of them end by EOS, so stopped holds. The 0.320 GPU result carries over.

tests/entrypoints + test_required_channel.py: 2712 passed, 93 skipped, 3 xfailed. black 26.10.1 and ruff are clean.

[Copilot]

🔵 Needs a closer look

Human assessment is needed because reasoning markers inside tool-call string arguments can silently alter those arguments even when thinking is disabled.

No open issues 🎉

1 resolved since last review

• Coalesce adjacent reasoning fragments in non-streaming output

_{Review effort: Balanced}

[Jasen2201]

@valarLip I ran ATOM's own review-pr skill (main's .claude/skills/review-pr) over this PR at fda4bcf, plus an independent second pass. That covered the affected unit tests run the way CI runs them (25 files, against the merge base 825b63b) and CPU probes on head and base. It found no correctness blocker. What it did find is fixed in f64e874:

1. The finish-reason wiring was untested, and the default allowed it to be skipped. With ended_by_stop forced to True, all 1668 reasoning and endpoint tests passed. Now:

   • stopped is keyword-only with no default on flush, split, read_whole_events and read_whole_blocks;
   • test_the_reasoning_reader_is_told_whether_the_model_stopped checks chat, /v1/messages and /v1/responses, streamed and not, for eos / stop_<id> / max_tokens, through the real routes;
   • ended_by_stop is pinned on the engine's raw reasons.

   The same mutant now fails 12 tests.

2. The PR description had fallen behind the diff. It is rewritten to cover the MarkerScanner rewrite, the replay conditions including stopped, the thinking-off rule, reasoning coalescing and the bounded replay.

3. Copilot's note on reasoning markers inside tool-call arguments. Confirmed by probe. It predates this PR, except for one streamed thinking-off shape that follows from the tools-at-offset-0 fix. Filed as Reasoning markers inside a tool-call argument are read as channel markers #2974, since the fix needs the reasoning and tool stages to share region state.

4. Wording fixes: the split docstring, two test docstrings, _pattern_for, and one over-long comment line.

Checked and cleared:

• The new MarkerScanner matches the old one over 40,000 random cases, including prefix pairs. A shortest-first control shows 9,831 differences, so the probe does see a tie-break change.
• No caller still relies on Scan.rest.
• Every call site passes the raw engine reason.
• The one test the script flagged (test_multiprocess_metrics[api_server]) is a 30 s import timeout on a loaded host, and the merge base fails it too.

BEAM is unchanged: the 700 replayed outputs give identical content and reasoning_content. tests/entrypoints + test_required_channel.py: 2741 passed, 93 skipped, 3 xfailed. black and ruff are clean.

[Copilot]

🟡 Changes recommended

A critical filter-state bug keeps reasoning detection active inside tool arguments when thinking is off, causing literal reasoning markers to divert argument content into reasoning and leave the tool argument empty.

1 open issue

• K3 tools opener leaves reasoning detection active in tool arguments · New

_{Review effort: Balanced}

[valar-cop]

a few small things..

ci green.

[valar-cop]

why re-spell the stop set here? let's derive it from one list in protocol.py, next to openai_stop_reason

[Jasen2201]

Agreed, done in a3190e5. ended_by_stop now lives in protocol.py, next to openai_stop_reason, and the stop set is spelled once there: MODEL_STOP_REASONS = ("eos", "stop", "stop_sequence") plus the stop_<token_id> prefix. openai_stop_reason derives its stop branch from the same predicate, and its mapping is unchanged (max_tokens → length, everything else → stop). The callers import it from protocol, and reasoning.py only takes the bool.

[Copilot]

🟢 Approval recommended

No active review issues block approval of the unified response parsing and Kimi-K3 reply handling changes.

No open issues 🎉

1 resolved since last review

• K3 tools opener leaves reasoning detection active in tool arguments

_{Review effort: Balanced}