Skip to content

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

Merged
leehack merged 5 commits into
mainfrom
fix/1561-1566-parse-parity
Oct 29, 2026
Merged

leehack merged 5 commits into
mainfrom
fix/1561-1566-parse-parity

Conversation

@leehack

@leehack leehack commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Seed-OSS: a reply to a prompt that opens no thought (the stock template) has one only when it starts with <seed:think> after ASCII whitespace only (C isspace, as upstream; NBSP/U+3000 do not count), kept verbatim up to the first </seed:think>, as llama.cpp 620d2225 parses it; later thinking tags stay content. Fixes #1561.
  • Cohere2 MoE (#1566):
    1. With a thought a handler closed for thinking off, Plan<|START_TEXT|>Sunny. now gives reasoning Plan / content Sunny.: a reply that reaches a marker parses (and streams) as after the forced-open prompt llama.cpp renders.
    2. Thinking tags inside a text or action body are text (e.g. a literal <|END_THINKING|> in a text block).
    3. Response-schema renders (another handler renders the Cohere2 MoE template) close the thought with thinking off, as the Cohere2 MoE handler does.
  • No public API change. The stream parser parses its renders through the internal parseRenderedOutput, which reads the render's internal closed-thought mark, so the engine's stream and final message match llama.cpp for marked and unmarked renders. The public ChatTemplateEngine.parse keeps its signature and cannot see the mark, so for Seed-OSS it still ends a thought at a </seed:think> in a reply that does not start with <seed:think>, as base did and as llama.cpp does after a template that opens the thought (e.g. genkit_llamadart's parse of a custom open-thought template). Trade-off: a public parse of a stock-template reply with such a stray end tag keeps base's result, not upstream's.
  • #1565 items: tests kill Hold a closed Cohere2 MoE, Command R7B or Seed-OSS thought; stop holding Ministral and Solar Open #1560 audit survivors S5, S10, S11; changelog "hold it the same way" rewording.
  • Audit follow-ups at 1bb9148: tests kill N20 (Cohere2 MoE end tag before a body, upstream-accepted on templates that open the thought), A13 (empty reply on a marked render), A14 (bare <seed:think> at EOF), A26 (response-schema close only for Cohere2 MoE) and A28 (its multimodal render path); changelog . ( typo fixed.

Root cause: Seed-OSS parsed any <seed:think>…</seed:think> anywhere as reasoning, while llama.cpp's autoparser only takes a leading thought when the prompt opens none. Cohere2 MoE extracted thinking over the whole reply instead of stopping at the body, and the marked (thought-closed) result parsed as unforced, so text before <|START_TEXT|> was content; the response-schema render path skipped the handler that closes the thought.

Production-readiness scope

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 unsupported paths)
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant. (chat-template-and-parsing.md, template-engine-internals.md; both changelogs. README/support matrix/examples/skills unaffected.)
  • 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: parse-parity fix, no new capability)
  • 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 above; regression tests are upstream-oracle fixture cases in test/fixtures/closed_thought_upstream.json (new no_thought_templates group plus Cohere2 MoE open-thought cases) driven by test/integration/core/engine/closed_thought_upstream_test.dart, and stream-parser unit tests. Affected: all platforms.
  • Feature PR: N/A. 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
  • dart test -p chrome --exclude-tags local-only — ran the template/engine subset (test/unit/core/template test/unit/core/engine); the change is pure Dart parsing.
  • Other: auditor's oracle harness (main, X and Y corpora) at the head; dart run tool/testing/check_platform_boundaries.dart; dart test -p vm test/unit/tooling/; ./tool/testing/run_template_parity_suites.sh (exit 0).

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
static-format-analyze format, analyzer, boundaries macOS, Flutter 3.47.1 PASS 0 changed, no issues, boundaries OK
root-vm VM unit/integration macOS arm64 PASS (+9639 ~126) -j 1 --exclude-tags local-only
root-chrome browser template/engine subset Chrome PASS (+2135 ~1) test/unit/core/template test/unit/core/engine
coverage-lib lib coverage CI N/A CI lane
template-parity vendored llama.cpp detection/render/parse parity, GBNF macOS, llama.cpp b10549 sources PASS all five suites pass, exit 0
structured-output-adversarial partial/final streaming and rollback, tools on/off, thinking on/off, pinned upstream parse parity llama.cpp 620d2225 oracle PASS Oracle: 0 OK→BAD, 0 new stream≠parse, 0 public-parse OK→BAD over 229,656 rows (below)
high-risk-exact-head-independent-qa independent blocking review of exact head fresh auditor session, exact head 70332a4 PASS CLEAR: CLEAR delta re-audit at 70332a4, 0 P1/P2; P3s on #1565; evidence /opt/UnitySrc/personal/llama/llamadart-evidence/pr-1569/evidence-1569-70332a4.json
gguf-chat-features-smoke / llama-cpp-chat-template-smoke real GGUF — N/A no Seed-OSS or Cohere2 MoE weights available; evidence is primary upstream parser emissions (llama.cpp 620d2225) plus durable fixtures

Upstream oracle (auditor's harness: llama.cpp 620d2225 vs llamadart engine-derived parse, public ChatTemplateEngine.parse, and whole/bytewise/every-2-split streams; base f3706ff vs head):

Corpus Rows OK→BAD New stream≠parse Public parse OK→BAD BAD→OK stream≠parse fixed
Main 185,160 0 0 0 2,236 (Seed-OSS 2,144, Cohere2 MoE 92) 2,160
X corpus 37,056 0 0 0 823 637
Auditor Y corpus 7,440 0 0 0 408 178

The 237 X and 45 Y X-Seed-open public-parse rows the audit flagged match upstream again. Visible-byte rows differing only by a trailing < on Seed-OSS with tools (8 main, 6 X) are unchanged from the audited head.

Mutation: first pass 34 mutants, 32 killed (S14 equivalent; N20 now killed). Audit pass at 1bb9148: N20, A13, A14, A26, A28 survived on correct code; all five are now killed, plus the new code's mutants (public Seed-OSS end-tag rule removed, ignoring a leading start tag, applied to every format, applied to the internal parse; Unicode trimLeft in the handler and the stream; narrowed ASCII whitespace set): 13 of 13 killed.

Review Notes

  • Independent review status: CLEAR at exact head 70332a4 by a fresh auditor session (0 P1, 0 P2).
  • CI status / head SHA: 70332a4
  • Latency: unmarked Seed-OSS content streams earlier; unmarked Cohere2 MoE holds a trailing opening prefix (first content of a bare <|START_ACTION|> without tools moves from byte 9 to 16); marked Cohere2 MoE text with tools streams at EOF, as forced-open Cohere2 MoE with tools already does.
  • Pre-existing, unchanged: forced-open Cohere2 MoE action whose arguments contain <|START_TEXT|>, incomplete actions, <|END_OF_TURN_TOKEN|> after a text, and a trailing < partial differ from upstream; forced Seed-OSS (custom template opening the thought) is still not verbatim; unmarked Cohere2 MoE with an end tag before any start tag streams its first characters as content when tokens split before the end tag; Seed-OSS content keeps Dart's Unicode trim() at its ends.

High-risk regression review

  • Classification: high-risk (surfaces: backendRuntime, structuredOutput)
  • Implementation task: llamadart 0.17.0 lane (#922) for #1561 and #1566
  • Independent blocking QA task: fresh auditor session, CLEAR, 0 known PR-caused P1, 0 unresolved threads
  • Exact head / current base: 70332a4 / dc8d179 (merge-tree clean)
  • Production-branch deletion, bypass, or miswire proof: mutation checks above (remaining survivor S14 is equivalent); failing-first: new fixture cases fail at base (18 failures)
  • Affected-family real-model/artifact evidence: unavailable (Seed-OSS, Cohere2 MoE weights); primary upstream emissions via llama.cpp 620d2225 parser plus durable fixtures
  • Explicit unavailable-family or other N/A evidence: Seed-OSS, Cohere2 MoE GGUF weights unavailable
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

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.
ChatTemplateEngine.parse no longer takes thoughtClosedForThinkingOff: the
mark it depends on is internal. The stream parser derives thinkingForcedOpen
with the internal parsesThinkingForcedOpen instead.
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.
@leehack
leehack marked this pull request as ready for review October 29, 2026 00:13
@leehack
leehack merged commit 04821e0 into main Oct 29, 2026
63 of 65 checks passed
@leehack
leehack deleted the fix/1561-1566-parse-parity branch October 29, 2026 00:13
@github-actions

Copy link
Copy Markdown
Contributor

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

leehack added a commit that referenced this pull request Oct 29, 2026
…eed-OSS ASCII trim (#1572)

* Pin the ASCII-only trim of the public Seed-OSS thought rule

A Seed-OSS reply that starts with a no-break space before <seed:think>
and ends a thought parses as after a template that opens the thought,
as llama.cpp does; a Unicode trim there (mutant B12 of #1569) now fails.

* Reuse a cached native bundle only while it matches its archive

The build hook writes extracted.json beside each llama.cpp bundle's
extracted/ directory, recording the archive's SHA-256, size and
modification time and every extracted file's size. A later build reuses
extracted/ only while the record matches the cached archive and the
files; otherwise it extracts the archive again or downloads it. A cache
without a record is extracted again once from its cached archive,
offline: verified for the pinned release or a cached SHA256SUMS, as is
for an override cached before the hook kept SHA256SUMS.

Hook tests that seed fake extracted/ dirs now write a matching archive
and marker; without them the hook would download the real bundles.
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.

Seed-OSS parse takes a leading thought as reasoning when the prompt never opened one

1 participant