Skip to content

Keep the response-format grammar applied through a budgeted forced-open thought - #1956

Merged
leehack merged 3 commits into
mainfrom
lane-1953-grammar-pause
Nov 12, 2026
Merged

leehack merged 3 commits into
mainfrom
lane-1953-grammar-pause

Conversation

@leehack

@leehack leehack commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Fixes constrained JSON (responseFormat) and required tool calls ignoring the grammar after a ThinkingBudget closes a template-opened thought on llama.cpp (Qwen3.5 and other forced-open thinking templates).
  • Root cause: LlamaCppService set pauseGrammarDuringReasoning for every grammar, but the grammar built for a forced-open thought (root ::= think-body "</think>" ...) consumes the thought itself. Paused, it never saw the forced \n</think> and resumed inside the thought.
  • Fix: new ToolCallGrammarUtils.takesForcedOpenThought; pause only lazy grammars and grammars that do not take the thought. A caller GenerationParams.grammar still waits for the thought to close.
  • Rejected: upstream's "pause only lazy grammars" breaks caller grammars (empty grammar stack at budget 0, answer in reasoning at budget 16); mutant B below.

Fixes #1953

Production-readiness scope

  • User-facing scope: responseFormat / required tool calls return schema-valid output at every thinking budget on forced-open thinking templates.
  • Supported platforms/paths: native llama.cpp (flag change in llama_cpp_service.dart; no new native call, so nothing new outside LlamaCppObjectCalls / ExitTeardownApi).
  • Unsupported or intentionally unavailable paths: unchanged.
  • Out of scope / follow-ups: packages/genkit_llamadart (genkit_llamadart: resolve the thinking budget through core (#1929) #1952 is open there).

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 change)
  • Public API docs, README/website docs, examples, support matrices, and changelog entries are updated where relevant.
  • Regression coverage covers the original issue plus key negative/version-skew paths where relevant.
  • Public capability changes update the validation coverage registry. (N/A: no capability change; validation_coverage_test.dart passes)
  • Security/privacy review completed: no secrets or paths touched.
  • Follow-up work tracked: None.

PR type guidance

  • Bugfix PR: root cause above; regression tests below; affected platforms: native llama.cpp on every OS.

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 (CI, and locally via coverage-lib)
  • dart test -p chrome --exclude-tags local-only (CI)
  • coverage-lib (see Matrix Evidence)
  • check_platform_boundaries, validation_coverage_test.dart, package_skills_test.dart, tool/testing/run_template_parity_suites.sh

Matrix Evidence

Matrix row Scope covered Platform / model / backend Result Evidence / notes
structured-output-adversarial grammar + thinking prefix with budgets 0/1/4/16/64 macOS CPU, Qwen3.5-0.8B Q4_K_M (affected family, local-only test) + stories15M (default CI) PASS main fails, fix passes; mutants A/B/C each caught
template-parity template/grammar parity macOS PASS run_template_parity_suites.sh
coverage-lib lib/ line coverage macOS 93.87% line coverage (local; threshold 70%)
high-risk-exact-head-independent-qa independent audit 8267bff PASS deep audit PASS at 7f1adf9f, wording delta PASS at 8267bffe. Evidence llamadart-evidence/pr-1956/ (evaluator exit 2)

Failing-first on origin/main (4b96fde): thinking_budget_grammar_test returns ' </think>th. He was a very special boy...' instead of ' </think>ok'; the unit test does not compile (no takesForcedOpenThought); the Qwen3.5 test fails at budget 0.

Mutant Result
A: pause every grammar (= main) stories15M and Qwen3.5 tests fail
B: pause only lazy grammars (upstream rule) caller-grammar control fails
C: predicate matches any think-body rule unit test fails

Primary upstream emission: llama-server b12110 (same llama.cpp commit cd959bae9) returns {"city": "Paris"} at budgets 16 and 64, with the answer in content and the thought in reasoning (llamadart-evidence/issue-1953/upstream-llama-server-b12110.log).

Review Notes

  • Independent review status: deep audit PASS at 7f1adf9f; delta re-audit PASS at 8267bffe
  • CI status / head SHA: all required checks pass at head 7f1adf9 (Linux, Windows, macOS, Chrome)

High-risk regression review

  • Classification: high-risk (backendRuntime, structuredOutput)
  • Implementation task: lane-1953-grammar-pause worker
  • Independent blocking QA task: fresh auditor sessions (not the planner or the author), with zero PR-caused P1s and zero unresolved threads. A cross-format probe over every vendored template passed, and real Qwen3.5-0.8B passes at budgets 0, 1, 4, 16 and 64. P3s are on 0.19.8 audit follow-ups #1950.
  • Exact head / current base: 8267bff / merge-base 4b96fde. main is now b096ebf (genkit and CI changes only; merge-tree is clean).
  • Production-branch deletion, bypass, or miswire proof: mutants A/B above revert the production branch and are caught by the new tests
  • Affected-family real-model/artifact evidence: Qwen3.5-0.8B-Q4_K_M, budgets 0/1/4/16/64, JSON schema returned each time (local-only test)
  • Explicit unavailable-family or other N/A evidence: Nemotron 3 Nano / StepFun 3.5 Flash weights not run; same Qwen3-Coder XML template path, covered by the Qwen3.5 template fixture
  • Known PR-caused P1 regressions: 0
  • Unresolved review threads: 0

@leehack
leehack marked this pull request as ready for review November 12, 2026 15:49
@leehack
leehack merged commit f816d29 into main Nov 12, 2026
71 of 73 checks passed
@leehack
leehack deleted the lane-1953-grammar-pause branch November 12, 2026 15:49
@github-actions

Copy link
Copy Markdown
Contributor

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

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.

Qwen3.5: constrained JSON with a thinking budget ignores the grammar after the forced </think>

1 participant