Skip to content

Parse Seed-OSS tool calls closer to llama.cpp (#1898) - #1908

Closed
leehack wants to merge 3 commits into
fix/1898-qwen3-coder-xml-parityfrom
fix/1898-seed-oss-parity
Closed

leehack wants to merge 3 commits into
fix/1898-qwen3-coder-xml-parityfrom
fix/1898-seed-oss-parity

Conversation

@leehack

@leehack leehack commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

Stacked on #1906. Its own change is the top commit.

Parse Seed-OSS tool calls closer to llama.cpp cd959bae9:

  • Unclosed last scope in a finished reply. If the call is complete through </function>\n, it is kept. A scope that ends before its call does (Hello <seed:tool_call>\n, a cut-off name) is dropped from content.
  • String arguments. An argument the tool declares as a string keeps its value as generated, so \n42\n and {"a": 1} stay strings. The handler now implements ToolSchemaAwareChatTemplateHandler.
  • Required grammar. With ToolChoice.required and no forced-open thought, the grammar allows a leading <seed:think>…</seed:think> and then text without <seed:tool_call>\n before the calls.
  • Grammar helper fix. ToolCallGrammarUtils.excludingRules now escapes control characters. Without this, a delimiter containing a line break wrote a raw newline into its rule.

Part of #1898 (plan #1899).

Production-readiness scope

  • User-facing scope: Seed-OSS replies keep string values as generated, recover a complete call whose scope was not closed, and required-call grammars accept a thought and text first.
  • Supported platforms/paths: all (pure Dart template engine).
  • Unsupported or intentionally unavailable paths: these stay lenient where llama.cpp fails the reply:
  • Out of scope / follow-ups:
    • Seed-OSS partial (streaming) parse alignment with llama.cpp's partial parse; the stream still equals the final parse.
    • Cut-off calls that llama.cpp returns with unterminated JSON.

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.
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant (template-engine internals, both changelogs).
  • 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 (Remaining llama.cpp parity divergences after 0.19.4 #1898).

PR type guidance

  • Bugfix PR:
    • Root cause: the Seed parse did not handle an unclosed last scope the way llama.cpp's autoparser does, it decoded every value as JSON regardless of the declared type, and its required grammar had to start at the scope.
    • Regression coverage: seed_oss_tool_call_upstream.json has 156 llama.cpp-recorded cases, and 136 of them fail on the base. seed_oss_handler_test.dart holds verdicts recorded from llama.cpp's grammar.

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: ran test/unit/core, test/integration/core and test/unit/tooling locally; the full suite runs in CI. windows_registry_qualification_test also fails locally on main (host ProcessException).
  • dart test -p chrome --exclude-tags local-only: ran test/unit/core/template and chat_completion_stream_parser_test.dart.
  • coverage-lib matrix row: 93.68% line coverage (33717/35993) at Seed head b3605ed, which contains this stack; the full dart test -p vm --exclude-tags local-only run there had 13444 passes, and its only failures were the 6 windows_registry_qualification_test host failures that main also has
  • Other targeted/local-only validation:
    • specialized_tool_grammar_validation_e2e_test.dart with LLAMA_CPP_GBNF_VALIDATOR
    • ./tool/testing/run_template_parity_suites.sh (PASS)

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
structured-output-adversarial parse, stream (whole, bytewise, 3-char, every 2-split), required grammar, auto/required, thinking on/off, parallel on/off macOS VM; llama.cpp cd959bae9 probe; vendored ByteDance-Seed-OSS.jinja PASS Oracle final mismatches 148 → 48, the rest being llama.cpp errors or BADJSON. Required grammar vs llama.cpp's: 0 mismatches over 48 replies × 4 requests (base 20–30 per request). All-template sweep vs the Qwen head: 0 regressions. 13 mutants killed.
high-risk-exact-head-independent-qa independent blocking-only review — PENDING Requested after CI.
release-metadata-verification — — N/A Not a release PR.
litert-lm-* — — N/A No LiteRT-LM change.

Review Notes

  • A forced-open thought keeps the previous required grammar: the thought end, then the calls. llama.cpp's real Seed-OSS prompt never forces the thought open, so there is no upstream grammar to match there.
  • Audit follow-up (head 6e241a3): when the scope end _withLastScopeEnded adds still yields no call (two consecutive unclosed scopes, where llama.cpp fails the reply), the reply is parsed as generated so the added </seed:tool_call> no longer leaks into content. Failing-first test in seed_oss_handler_test.dart; mutants L1–L3 and the earlier S1–S13 all killed. Merged the new Parse Qwen3-Coder XML call bodies as llama.cpp does (#1898) #1906 head (61a3140) so this branch fast-forwards.
  • Independent review status: passed at b3605ed; delta re-audit pending.
  • CI status / head SHA: ci.yml workflow_dispatch run 42886419606: success (26 jobs, 6 skipped) at 6e241a3.

High-risk regression review

  • Classification: high-risk (surfaces: structuredOutput)
  • Implementation task: lane 4 of plan 0.19.5 release plan: audit follow-ups, parity, Ling, LiteRT-LM structured output #1899
  • Independent blocking QA task: pending
  • Exact head / current base: 6e241a3 / 61a3140 (stacked on fix/1898-qwen3-coder-xml-parity; main 33e00f5)
  • Production-branch deletion, bypass, or miswire proof: all 13 mutants were killed. They cover string-argument handling, scope ending (removed, kept, newline check, rest check, non-call text), grammar composition (no text, no thought, unrestricted text, forced-open) and control-character escaping.
  • Affected-family real-model/artifact evidence: no weights; primary upstream emissions from llama.cpp cd959bae9 on the vendored template, kept as durable fixtures.
  • Explicit unavailable-family or other N/A evidence: Seed-OSS weights not downloaded; parser/grammar parity evidence only.
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

A finished reply whose last scope is never closed keeps a call complete
through `</function>` and drops a scope cut off before its call ends. An
argument the tool declares as a string keeps its value as generated. With
`ToolChoice.required`, the grammar lets a thought and text without a scope
start precede the calls, as llama.cpp cd959bae9's does. The excluding-text
grammar rules escape control characters so a delimiter's line break stays
on its rule's line.
…1898)

When the added scope end yields no call, parse the reply as generated so
the added tag does not show in content.
@leehack

leehack commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

Landed via batch integration PR #1912 (rebase merge, main 2cfb936). The integration audit passed at a9c7772, and this PR's audited head is recorded in #1912's constituent table.

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