Skip to content

Hold a closed Cohere2 MoE, Command R7B or Seed-OSS thought; stop holding Ministral and Solar Open - #1560

Merged
leehack merged 2 commits into
mainfrom
fix/1553-closed-thought-handlers
Oct 28, 2026
Merged

leehack merged 2 commits into
mainfrom
fix/1553-closed-thought-handlers

Conversation

@leehack

@leehack leehack commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • With thinking off, Cohere2 MoE replies no longer stream a thought the model writes before <|END_THINKING|> as content. llamadart renders the template with reasoning: false, which closes the thought, while llama.cpp 620d2225 sets no reasoning, keeps it open and parses the leading thought as reasoning. The Cohere2 MoE handler now marks that result (withThoughtClosedForThinkingOff), as do the Command R7B, Cohere2 MoE and Seed-OSS handlers where they append the end tag to a thought the template opened. Marks apply only where llamadart closed the thought: a caller's own reasoning: false or reasoning_effort: none closes it in llama.cpp too, and llama.cpp then parses no leading thought, so those stay unmarked.
  • A Cohere2 MoE hold also releases at <|START_TEXT|> or <|START_ACTION|>, where llama.cpp ends the open thought (cohere2moe.cpp: until_one_of({THINK_END, TEXT_START, ACTION_START})). Without this, every tagless thinking-off Cohere2 MoE reply (the normal case, since the prompt closes the thought) would arrive only at the end.
  • Ministral and Solar Open no longer mark: their PEG parse takes a thought only after its start tag (Ministral keeps Plan[/THINK]Sunny. as content; Solar Open rejects it), so holding gained nothing. The registry test now parses with the render's own parser, which is how it missed this.
  • Tests pin that template-closed results (DeepSeek V3.1, Qwen3, QwQ, Qwen3.5) and a Hermes render without an open thought stream unheld (audit mutants X4 and X10).
  • Closes #1553, closes #1557. No public API change.

Production-readiness scope

  • User-facing scope: with enableThinking: false on Cohere2 MoE (North) templates, streamed chunk.text matches the final message when the model still writes a thought: content waits for a thinking tag, <|START_TEXT|> or <|START_ACTION|>, then streams live. Custom Command R7B or Seed-OSS templates that open the thought with thinking off behave like the #1532 templates. Ministral and Solar Open templates that open [THINK]/<|think|> with thinking off stream as they arrive again (no on-disk template reaches this).
  • Supported platforms/paths: all; Dart-side render and stream, shared by native and Web.
  • Unsupported or intentionally unavailable paths: N/A. Thinking-on (forced-open) streams and thoughts the caller's or template's own setting closes are unchanged.
  • Out of scope / follow-ups:
    • #1557 item 3 (held text on a backend error) is left as is: on a source error the stream drops every withheld buffer (partial tags, tool envelopes, undecided prefixes) and runs no final parse, and the hold is one more such buffer. Flushing it would emit text the parse may call reasoning as content, which the hold exists to prevent. Decision deferred to the maintainer.
    • Pre-existing, unchanged: Seed-OSS on-disk templates never open a thought, yet llamadart's parse takes Plan</seed:think>Sunny. as reasoning where llama.cpp keeps it as content (the stream already matches llama.cpp there). Cohere2 MoE Plan <|START_ACTION|>...<|END_ACTION|><|END_THINKING|>Done. and multi-end-tag replies stay stream != parse as on base.

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 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: #1555 marked handler-closed thoughts, but the Command R7B/Cohere2 MoE and Seed-OSS handlers were excluded (under change in #1550), and the issue's on-disk repro is not a post-render close at all: the handler's reasoning: false default makes the Cohere2 MoE template close the thought. Ministral and Solar Open marked unconditionally although their parse never takes an untagged leading thought; the registry test parsed without the render's parser. Tests:
    • Cohere2 MoE upstream fixture cases (thought, end tag only, thought then call, text, call), captured from llama.cpp 620d2225 with and without the tool, streamed whole, bytewise, in 3-char chunks and at every 2-chunk split, thinking off and on; the mark survives required-tool grammar and token counting.
    • Registry check parses with each render's parser: each handler marks iff it closed an open thought and that parse ends a leading thought at the end tag; never for a render without an open thought, and never with thinking on.
    • Cohere2 MoE marks only where llamadart closes the thought (reasoning: false / reasoning_effort: none kwargs unmarked, reasoning: true marked); the on-disk Command R7B template, which closes the thought itself as in llama.cpp, unmarked.
    • Synthetic marked Cohere2 MoE, Command R7B and Seed-OSS results at every split (stream == parse), plus latency checks: Cohere2 MoE text after <|START_TEXT|> streams as it arrives; Command R7B waits for a thinking tag; other formats do not release at Cohere openings.
    • Template-closed fixtures stream a tagless reply as it arrives, thinking off and on (X4/X10).
    • 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
  • dart test -p vm -j 1 --exclude-tags local-only (isolated TMPDIR): +9664 ~5 at merged head c0449714a, all passed (+9522 ~123 before the merge)
  • 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 +1135, all passed
  • Other targeted/local-only validation: check_platform_boundaries.dart OK; ./tool/testing/run_template_parity_suites.sh (detection +74, diagnostics +93, unit template +1615, upstream chat +10, GBNF +58, all passed); oracle and mutation runs below.

Oracle: the #1555 audit harness (llama.cpp 620d2225 common_chat_session, 128,572 rows over 94 templates; every row streamed whole, paced bytewise and at every 2-split). Base 49c90e45c, head lib 3151c7a94 (the merge of main adds only #1551, no template or stream code).

Check Base Head
Failing-first: new/strengthened tests 8 fail (Cohere2 MoE fixture render, mark, 3 stream cases; registry; Cohere kwarg marks; Cohere text latency) all pass
Oracle OK (parse and all stream modes equal upstream) 52,934 52,954; 0 OK->BAD; 20 BAD->OK
Stream != parse (any mode) 40,612 40,312; 0 new; 300 fixed
Rows whose parse, prompt or closed-thought mark changed 0
Rows whose stream changed 320, all thinking-off Cohere2 MoE (held); 0 unheld rows changed
Held rows with thinking on or forced open 0

Latency: content arrival moves only on held Cohere2 MoE rows (944); every move is a reply with no thinking tag or Cohere opening arriving at EOF, the #1555 trade-off. No row arrives later than upstream where upstream emits content.

Mutants (29, VM template/engine suites): R7B append does not mark; Cohere default close never marks / ignores the user reasoning kwarg / ignores reasoning_effort / ignores the thinking flag / ignores the prompt end; Cohere2 MoE does not open by default; R7B opens by default; Seed-OSS does not mark; no Cohere release openings; Cohere openings for every format; scan misses a split marker; release yields only the new token; hold drops text at EOF; releases only at the end tag / only at the start tag; Ministral and Solar Open handler files reverted to base (mark again); audit X3, X4, X10, A-S1, A-S2; six handlers do not mark. 29 of 29 killed (X4 and X10 by the new template-closed and registry pins).

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 template detection/render/parse parity and GBNF acceptance macOS 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 and mutation runs above
high-risk-exact-head-independent-qa independent blocking review of exact head fresh auditor session, exact head c044971 PASS CLEAR: CLEAR at c044971, 0 P1/P2; P3s on #1565; evidence /opt/UnitySrc/personal/llama/llamadart-evidence/pr-1560/evidence-1560-c044971.json
release-metadata-verification release metadata N/A no release or version change
litert-lm-json-schema LiteRT-LM JSON Schema N/A LiteRT-LM untouched

Review Notes

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

High-risk regression review

…ing Ministral and Solar Open

With thinking off, Cohere2 MoE's `reasoning: false` default closes a thought
llama.cpp leaves open, and the Command R7B, Cohere2 MoE and Seed-OSS handlers
close one a template opens, but their parse still takes text before the end
tag as reasoning. They now mark the result, and a Cohere2 MoE hold also ends
at `<|START_TEXT|>` or `<|START_ACTION|>`, where llama.cpp ends the thought.
Ministral and Solar Open no longer mark: their parse takes a thought only
after its start tag. Tests pin that template-closed and non-forced Hermes
results stream unheld.

Closes #1553. Closes #1557.
@leehack
leehack marked this pull request as ready for review October 28, 2026 21:25
@leehack
leehack merged commit 5fba80f into main Oct 28, 2026
38 checks passed
@leehack
leehack deleted the fix/1553-closed-thought-handlers branch October 28, 2026 21:25
@github-actions

Copy link
Copy Markdown
Contributor

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

leehack added a commit that referenced this pull request Oct 29, 2026
…pen (#1569)

* Match llama.cpp's Seed-OSS and Cohere2 MoE parse when no thought is open

A Seed-OSS reply to a prompt that opens no thought now has one only when it
starts with `<seed:think>`, kept as generated up to the first end tag; later
thinking tags are content. A Cohere2 MoE reply whose thought a handler closed
for thinking off parses as after the forced-open prompt llama.cpp renders, so
`Plan<|START_TEXT|>Sunny.` gives reasoning Plan, and thinking tags inside a
text or action are text. Response-schema Cohere2 MoE renders close the
thought with thinking off, as the Cohere2 MoE handler does.
`ChatTemplateEngine.parse` takes `thoughtClosedForThinkingOff` so a final
parse matches the stream. Tests pin the stream hold release from #1560.

Closes #1561. Closes #1566.

* Keep the closed-thought parse flag internal

ChatTemplateEngine.parse no longer takes thoughtClosedForThinkingOff: the
mark it depends on is internal. The stream parser derives thinkingForcedOpen
with the internal parsesThinkingForcedOpen instead.

* Keep the public Seed-OSS parse of an opened thought; pin audit gaps

ChatTemplateEngine.parse cannot see whether a template opened the thought,
so a Seed-OSS reply that ends one it did not open parses as before, as
llama.cpp parses it after such a template. The stream parses its renders
through the internal parseRenderedOutput. Only ASCII whitespace may precede
a leading <seed:think>, as in llama.cpp. Tests cover a stray end tag before
a Cohere2 MoE body, an empty held reply, a bare start tag, and the response
schema close on other formats and the multimodal path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant