Skip to content

Hold a thought a handler closed for thinking off until a thinking tag arrives (#1532) - #1555

Merged
leehack merged 2 commits into
mainfrom
fix/1532-stream-open-thought
Oct 28, 2026
Merged

leehack merged 2 commits into
mainfrom
fix/1532-stream-open-thought

Conversation

@leehack

@leehack leehack commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • With thinking off, many handlers close a thought their template opened (StepFun3.5-Flash's <think>\n becomes <think></think>\n), but the parse still ends a reply's leading thought at the first end tag, as llama.cpp 620d2225 parses the thought it keeps open. A stream in small chunks emitted that thought (Plan\n</think>\n\n) as content before the end tag arrived, so streamed content and the final message disagreed.
  • Handlers now mark such a result (internal withThoughtClosedForThinkingOff / thoughtClosedForThinkingOffOf in thinking_utils.dart, an Expando like the closed-thought mark from #1525). Template-result copies keep both marks through withThoughtMarksOf. The stream holds a marked reply until it contains a start or end thinking tag, or ends, then runs the held text through the existing parser state machine. Text after the tag streams as before.
  • Only handlers whose parse ends a leading thought at the end tag set the mark. Gemma 4, MuseGlimmer and MiniMax M1 don't, so their thinking-off content streams as before. A forced-open thought still streams its reasoning as it arrives, as upstream's partial parse does.
  • Closes #1532. No public API change.

Production-readiness scope

  • User-facing scope: with enableThinking: false (or a thinking-off kwarg), replies on templates that open the thought anyway (StepFun3.5-Flash, DeepSeek R1 distills with the current upstream template, MiniMax M2, Nemotron Nano v2, and any template the marking handlers render that way) never stream a thought as chunk.text. Content waits for </think>, <think> or the end of the reply. Once the model writes </think>, content after it streams live. If the model never writes a thinking tag, the whole reply arrives at the end. That is the cost of matching the final parse at every chunking. With thinking off, reasoning deltas are not emitted, so streaming the held text as reasoning would look the same to the caller.
  • Supported platforms/paths: all. The change is in the Dart-side render and stream, shared by native and Web.
  • Unsupported or intentionally unavailable paths: N/A. Thinking-on (forced-open) streams and prompts the template itself closes are unchanged.
  • Out of scope / follow-ups: Command R7B, Cohere2 MoE and Seed-OSS handlers don't mark yet because #1550 is changing those files: #1553. Some stream != parse cases predate this PR and stay as they are: a second end tag after a released thought (Plan.</think>A</think>B<think>X</think>C, 10 rows per affected template), and non-forced prompts where llamadart's parse takes a leading thought that llama.cpp keeps as content. In those cases the bytewise stream already matches upstream, so changing it would regress upstream parity.

Completeness checklist

  • Declared scope is fully implemented, or explicitly reduced above.
  • Unsupported platform/option combinations fail loudly with actionable diagnostics or are clearly documented as unavailable. (N/A: no new option.)
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant. (template-engine-internals.md, chat-template-and-parsing.md, skills/llamadart-chat-streaming/SKILL.md, both changelogs. README/examples/support matrix: N/A, no capability change.)
  • Regression coverage covers the original issue plus key negative/version-skew paths where relevant.
  • Public capability changes update the validation coverage registry with a pack or a reasoned, issue-linked exemption; inventory checks pass (or N/A with scope). N/A: stream/parse parity fix, no public capability change.
  • Security/privacy review completed: no secrets, bearer tokens, signed URLs, or raw secret-bearing paths leak through logs, cache keys, metadata, errors, or snapshots.
  • Follow-up work that is useful but not required for this PR is tracked in GitHub Issues, or explicitly marked "None" above.

PR type guidance

  • Bugfix PR: root cause: when a handler closed a thought that the template opened, thinkingForcedOpen was false, so the stream started outside a thought and released text as content right away. The parse's pre-opened rule (extractThinking: an end tag before any start tag makes the text before it reasoning) only applied once the whole text was available, so a one-chunk stream matched and smaller chunks did not. Tests:
    • StepFun3.5-Flash upstream fixture cases, with and without tools, streamed whole, bytewise, in 3-char chunks and at every 2-chunk split, with thinking off and on. Content, reasoning and calls must equal llama.cpp. The old test only checked one-chunk content.
    • Synthetic marked results for Qwen3-Coder XML, Hermes, DeepSeek R1, MiniMax M2, Nemotron v2 and GLM 4.5 at every split, checking stream == parse.
    • Latency checks: content after </think> or <think> streams as it arrives, an unmarked result is not held, and a forced-open thought streams reasoning.
    • A registry-wide check that each handler marks iff it closed an open thought and its parse ends a leading thought at the end tag.
    • The mark survives required-tool and response-schema grammar and token counting.
    • Forced-open (thinking-on) DeepSeek V3.1, QwQ and Qwen3.5 fixture outputs at every split, with and without tools, checking stream == parse. These already passed on base and guard against regression.
    • Platforms: all (Dart).
  • Feature PR / Docs-only PR: N/A.

Test Plan

  • dart run tool/prepare_workspace.dart
  • dart format --output=none --set-exit-if-changed .
  • dart analyze lib test tool
  • dart test -p vm -j 1 --exclude-tags local-only (isolated TMPDIR): +9482 ~123, all passed
  • dart test -p chrome --exclude-tags local-only (CI). Locally: dart test -p chrome test/unit/core/template test/unit/core/engine/chat_completion_stream_parser_test.dart (+1127, all passed)
  • Other targeted/local-only validation: dart run tool/testing/check_platform_boundaries.dart (OK); ./tool/testing/run_template_parity_suites.sh (detection +74, diagnostics +93, unit template +1611, upstream chat suites +10, GBNF +58; all passed, llama.cpp b10549); oracle, mutation and real-model runs below.

Evidence. The oracle is the #1525 auditor harness built from llama.cpp 620d2225 (common_chat_session whole and bytewise): 127,332 cases over 94 templates, thinking on/off, kwarg on/off, notools/none/auto/required/auto-parallel. The stream is fed whole, bytewise, in 3-char chunks and split in two at every point. The 2-split runs on every row where the parse has reasoning or any stream mode differs from the parse.

Check Base (ae5c9abe2) Head
Failing-first: new/strengthened tests 15 fail (StepFun stream content at small chunks, mark tests, new unit stream group) all pass
Oracle vs upstream (all stream modes and parse) OK 52,435 OK 53,689; 0 OK->BAD; 1,254 BAD->OK
Stream != parse (any mode) 41,516 39,966; 0 new; 1,550 fixed
Rows whose parse changed 0
Rows whose stream changed 1,600, all thinking-off rows of StepFun3.5-Flash, DeepSeek-R1-Distill-Llama-8B, llama-cpp-deepseek-r1, MiniMax-M2, Nemotron-Nano-v2 (320 each)
Every 2-split on all 7,440 rows of those templates stream != parse 2,368 818; 0 new

Mutants (37): stream ignores the mark; ignores forced-open precedence; releases only at the end tag; only at the start tag; drops held text at EOF; misses a tag split across chunks; holds until EOF; copies drop the new mark; copies drop the closed-thought mark; the mark ignores closed; DirectJinja ignores parsesThoughtBeforeEndTag; MuseGlimmer or MiniMax M1 claim it; each of 18 handlers does not mark; the renderer and each of 5 engine copies drop marks. 34 of 37 killed. The 3 survivors are the Ministral tool-choice-none copy, the generic/content-only grammar copy and the content-only _withFormat copy. No marked result reaches them: Ministral with none has no grammar, and a response schema re-renders Hermes-style templates through the generic handler, which never closes a thought. The same copies were equivalent for the #1525 mark.

Real-model check (affected family): DeepSeek-R1-Distill-Llama-8B Q4_K_M, native Metal, temp 0, enableThinking: false, current upstream template (<think>\n, closed by llamadart). In 4 of 4 prompts the model still wrote a thought ending in </think>. Replaying the recorded emissions: on base, streamed content != final content in 8 of 8 replays (native chunks and bytewise), and </think> leaks bytewise. On head, 8 of 8 match with no leak. engine.create on head streams **Solution:** ... with no thought. StepFun3.5-Flash, MiniMax M2 and Nemotron Nano v2 weights are unavailable locally: the evidence for them is the upstream emissions in the oracle plus the durable StepFun fixture.

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
static-format-analyze format, analyzer, web/native import boundaries macOS, Flutter 3.47.1 PASS local; CI pending
root-chrome browser-compatible unit/integration coverage Chrome PASS local template + stream parser subset; full lane in CI
template-parity vendored llama.cpp chat-template detection/render/parse parity and GBNF acceptance macOS, llama.cpp b10549 PASS run_template_parity_suites.sh
structured-output-adversarial partial/final streaming with thinking prefixes, tool choices, upstream parity 94 templates, llama.cpp 620d2225 oracle PASS oracle, every-split and mutation runs above
high-risk-exact-head-independent-qa independent blocking review of exact head fresh auditor session, exact head 3df1355 PASS CLEAR: CLEAR re-audit at 3df1355, 0 P1/P2, 3 P3 noted; evidence /opt/UnitySrc/personal/llama/llamadart-evidence/pr-1555/audit/evidence-1555-3df1355.json
gguf-chat-features-smoke real GGUF chat streaming DeepSeek-R1-Distill-Llama-8B, Metal PASS (ad hoc) real-model replay above; scenario not run

Review Notes

  • Independent review status: CLEAR at exact head 3df1355 by a fresh auditor session (0 P1, 0 P2).
  • CI status / head SHA: a32e1eca1, CI pending.

High-risk regression review

  • Classification: high-risk (surfaces: backendRuntime, structuredOutput)
  • Implementation task: Streaming: small chunks emit an open thought as content before </think> arrives #1532 lane (0.17 follow-ups, #922)
  • Independent blocking QA task: fresh auditor session, CLEAR, 0 known PR-caused P1, 0 unresolved threads
  • Exact head / current base: 3df1355 / c55f566 (merge-tree clean)
  • Production-branch deletion, bypass, or miswire proof: mutants above (stream gate, release conditions, each handler's mark, copy propagation)
  • Affected-family real-model/artifact evidence: DeepSeek-R1-Distill-Llama-8B Q4_K_M (Metal) replay above
  • Explicit unavailable-family or other N/A evidence: StepFun3.5-Flash, MiniMax M2, Nemotron Nano v2: weights unavailable; llama.cpp 620d2225 emissions (oracle) plus the durable StepFun fixture
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

With thinking off, handlers close a thought the template opened, but the
parse still ends a leading thought at the first end tag, as llama.cpp
parses the thought it keeps open. Small-chunk streams emitted that thought
as content before the end tag arrived. Handlers now mark such results and
the stream holds the reply until a thinking tag or the end of the output.

Closes #1532.
@leehack

leehack commented Oct 28, 2026

Copy link
Copy Markdown
Owner Author

Design decision (maintainer, 2026-10-28): accept the hold. For results where the handler closed a template-opened thought and the parse takes a leading thought, the stream holds content until <think>/</think> or the end of the reply, so streamed text never shows the thought as reply text. Cost accepted: with thinking off on these templates, a reply with no thinking tag arrives at the end instead of streaming. Independent audit of a32e1eca1 started.

@leehack
leehack marked this pull request as ready for review October 28, 2026 19:18
@leehack
leehack merged commit 49c90e4 into main Oct 28, 2026
38 checks passed
@leehack
leehack deleted the fix/1532-stream-open-thought branch October 28, 2026 19:18
@github-actions

Copy link
Copy Markdown
Contributor

Chat app preview removed for leehack/llamadart-chat-pr-1555.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Streaming: small chunks emit an open thought as content before </think> arrives

1 participant