Skip to content

Keep a stray </think> as content after a prompt that opened no thought, as llama.cpp does - #1866

Closed
leehack wants to merge 4 commits into
fix/1531-reasoning-parityfrom
fix/1858-bare-think-stream
Closed

leehack wants to merge 4 commits into
fix/1531-reasoning-parityfrom
fix/1858-bare-think-stream

Conversation

@leehack

@leehack leehack commented Nov 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • After a prompt that opens no thought (for example Qwen3 with thinking on, Hermes 2/3, Qwen2.5, Qwen3-Coder), a Hermes, Qwen3-Coder XML or DeepSeek V3 reply with a </think> before any <think> now keeps that end tag, and every thinking tag after it, as content. llama.cpp does the same at 620d2225c and cd959bae9. Plan.</think>Answer. used to parse as reasoning Plan. and stream differently by chunking: one char at a time gave content Plan.Answer. plus reasoning Plan.. Every chunking now equals the final parse.
  • Parse: parseRenderedOutput, which parseRendered and the stream use, takes the tags as text (thoughtTagsAreText) when the render opened no thought and endsUnopenedThought holds. The public ChatTemplateEngine.parse cannot see the render, so it keeps ending a thought there, as it already does for Seed-OSS.
  • Stream: for those formats, an end tag that comes before any start tag starts content in which thinking tags are text (the existing bodyStarted path).
  • Oracle-recorded no_thought_templates cases (finished, without tools, partial) for vendored Hermes 2 Pro, Qwen3-Coder and Qwen3 4B (thinking on).

Fixes #1858
Refs #1845

Stacked on #1859 (fix/1531-reasoning-parity at 93621a38). Review and merge #1859 first; this PR's base is that branch.

Production-readiness scope

  • User-facing scope: Replies with a stray </think> after a prompt that opened no thought stream the same text for every chunking, with no duplication, and match llama-server.
  • Supported platforms/paths: All. Pure Dart template parsing and streaming used by native, Web and LiteRT-LM chat.
  • Unsupported or intentionally unavailable paths: None.
  • Out of scope / follow-ups: Thinking tags that llama.cpp keeps as text still parse as a thought in other formats and mid-reply #1865: the same bare-end-tag shape in other formats (generic/content-only, Llama 3, GLM 4.5, LFM2, Kimi K2, DeepSeek R1/V3.2/V4, Granite, MiniCPM5, Mistral Nemo, Laguna), plus mid-reply thoughts (Answer <think>X</think> more.) and a stray end tag after a leading thought (<think>A</think>B</think>C) in Hermes/Qwen3-Coder. These are older divergences, and this PR does not change them (sweep below). The pre-existing partial-parse difference for a trailing < (Use 3 <, where llama.cpp drops the < from a partial parse) is also unchanged and not recorded as upstream_partial.

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 combination.
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant. Both changelogs, chat-template-and-parsing.md, template-engine-internals.md and the parseRendered dartdoc are updated. No public API or skills 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: 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: the handlers' extractThinking treats an end tag before any start tag as the end of a thought the prompt opened. The parse then reclassifies text the stream has already sent as content, so the final reconciliation adds it again as reasoning and skips the content delta (prefix mismatch). llama.cpp takes a thought only where the prompt opened one or at the start of the reply. The fix gives the rendered parse and the stream the same rule. It applies only when the render opened no thought: forced-open, closed (template's empty thought) and closed-for-thinking-off renders are unchanged.
  • Feature PR / Docs-only PR: N/A.

Test Plan

  • dart run tool/prepare_workspace.dart (clean worktree)
  • dart format --output=none --set-exit-if-changed . (0 changed)
  • dart analyze (no issues); check_platform_boundaries.dart OK
  • dart test -p vm -j 1 --exclude-tags local-only: full VM suite ran under coverage (11860 passed). The only failures were windows_registry_qualification_test.dart (6, ProcessException: No such file or directory, which fails the same way at base 93621a38 on this Mac) and qwen_content_contract_test.dart thinking=true (3). That test fed a synthetic Check values.</think> with no start tag. It now feeds <think>Check values.</think>, which is what the native LiteRT channel assembler emits (it always writes the start tag). Affected suites rerun at head: 3957 passed; markdown-selected suites: 729 passed.
  • dart test -p chrome --exclude-tags local-only: the changed Chrome-capable tests (stream parser, engine, LiteRT Qwen contract, thinking utils): 557 passed. CI runs the full shards.
  • coverage-lib matrix row: LCOV line coverage 93.64% (threshold 70).
  • Other targeted/local-only validation: ./tool/testing/run_template_parity_suites.sh (74/103/1865/10/79 passed), plus the oracle sweeps below.

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
structured-output-adversarial final and partial parse, stream over whole, 1-, 2- and 3-char chunks and every 2-split, tools none/auto/required/parallel, thinking on/off and kwargs macOS arm64 VM; all 76 llama.cpp models/templates plus 18 vendored test/fixtures/templates Pass Oracle: unchanged llama.cpp cd959bae9 (common_chat_session feed/finish, and common_chat_peg_parse with is_partial). Base 93621a38 vs head, 0 match-to-mismatch transitions in every corpus (oracle parity and stream-equals-parse). See the sweep table below.
template-parity detection, diagnostics, unit templates, upstream chat e2e, grammar macOS arm64 VM Pass 74 / 103 / 1865 / 10 / 79
coverage-lib lib line coverage macOS arm64 VM Pass 93.64%
high-risk-exact-head-independent-qa exact-head adversarial review n/a Pending Needs an independent auditor before ready
Affected-family real model n/a n/a N/A The bug needs a reply with a stray </think> after a prompt that opened no thought, which a real Qwen3/Hermes run does not produce on demand. Evidence is primary upstream emissions (oracle at the pin) plus durable fixtures.

Sweeps (base 93621a38 → head; "oracle" = parse and every chunking equal llama.cpp; "self" = every chunking equals the final parse):

Corpus Cases Oracle match Self-consistent Changed formats Regressions
#1525 thinking corpus, 76 upstream templates (finished) 103212 46620 → 51758 78168 → 83688 hermes, qwen3CoderXml (non-forced only) 0
Bare-end-tag corpus, 76 upstream templates (finished) 52080 18375 → 22921 32106 → 36706 same 0
Bare-end-tag corpus, partial parse 52080 19005 → 23263 n/a same 0
#1525 corpus, vendored templates 24120 12190 → 13042 16196 → 17096 Qwen3-4B, Qwen3-VL-2B 0
Bare corpus, vendored (finished / partial) 12000 / 12000 3426 → 4176 / 3642 → 4356 5538 → 6288 same 0
#1539/#1542 corpus (Seed, Qwen3-Coder, StepFun, Nemotron, Qwen3.5) 192 89 → 95 186 → 192 Qwen3-Coder 0

Hermes and Qwen3-Coder non-forced renders have no stream-vs-parse mismatch left in these corpora. Remaining oracle differences there are the #1865 shapes and cross-format tool envelopes. Evidence: llamadart-evidence/issue-1858/ (harness, per-case JSONL for base and head, summaries, mutants).

Mutations (targeted VM suites, production restored after each):

Mutant Result
P1 rendered parse never applies the rule (base parse) Killed
P2 rule also after a thought closed for thinking off Killed
P3 rule also after a forced-open prompt Killed
P4 promptOpensNoThought && endsUnopenedThought → || Killed
U1 endsUnopenedThought ignores start-tag order / U2 any start tag disables it Killed / Killed
S1 stream never keeps the unopened end tag (base stream) Killed
S2 stream rule also after a thought closed for thinking off Killed
S3 stream rule for every format, not only the three the parse covers Killed
S4 stream rule also after a start tag was seen Killed
S5 stream ignores a start tag earlier in the same buffer Killed
S6 stream does not switch the rest of the reply to content Killed

A !_usesPegParser guard in the stream condition was removed after review: with a PEG parser these formats never reach the split (parsed mode with tools, the leading-thought path without), and the head sweeps are byte-identical with and without it.

Review Notes

  • Independent review status: pending (high-risk; needs an exact-head auditor).
  • CI status / head SHA: 4b4934a4bcef. CI runs only on PRs into main, so it was dispatched on this branch: run 42657556794, success (26 passed, 6 skipped by selection, 0 failed).

High-risk regression review

  • Classification: high-risk (backendRuntime, structuredOutput)
  • Implementation task: lane 10 of 0.19.2 release plan: audit follow-ups and correctness fixes #1845
  • Independent blocking QA task: pending, coordinator-assigned auditor
  • Exact head / current base: 4b4934a4bcef / 93621a3813ab (Keep a trailing partial tag in an unended thought, as llama.cpp v0.9.0 does #1859 head; that PR's base is 87aaec616)
  • Production-branch deletion, bypass, or miswire proof: P1 and S1 (each half of the fix removed) and S3 (stream miswired to other formats) are killed.
  • Affected-family real-model/artifact evidence: primary upstream emissions at the pin (oracle), recorded as fixtures for Hermes 2 Pro, Qwen3-Coder and Qwen3 4B.
  • Explicit unavailable-family or other N/A evidence: see the matrix row: the shape cannot be produced on demand by a real model.
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

leehack added a commit that referenced this pull request Nov 9, 2026
…t, as llama.cpp does (#1866)

Constituent of the 0.19.2 batch integration (#1845). Stacked on #1859: this commit is the delta from #1859's pinned head 93621a3 to #1866's pinned head 4b4934a (git diff 93621a3..4b4934a).
@leehack

leehack commented Nov 9, 2026

Copy link
Copy Markdown
Owner Author

Landed through the 0.19.2 batch integration PR #1870 (rebase merge, one commit per constituent; this PR's pinned head was audited/CI-gated there).

@leehack leehack closed this Nov 9, 2026
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.

1 participant