Skip to content

Shared chat parsing and constrained decoding for the OpenAI endpoints - #790

Merged
zaneni6 merged 24 commits into
mainfrom
openai-refactor
Oct 19, 2026
Merged

zaneni6 merged 24 commits into
mainfrom
openai-refactor

Conversation

@zaneni6

@zaneni6 zaneni6 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Every model carried its own reasoning and tool-call parser, and tool_choice was a single masked token. This replaces both with one shared chat library (src/common/chat) and adds constrained decoding to /v1/chat/completions.

Parsing

  • The chat format is detected once per model from its chat template and vocabulary: hermes_json, qwen_xml, gemma4, lfm2_pythonic, harmony or content_only. The server log prints the result at load (Chat format: ...).
  • Streamed and non-streamed replies go through the same parser, so the streamed deltas (reasoning_content, content, tool_calls) add up to the non-streamed message. Tool markup only counts as a call when the request has tools.
  • The per-model parse_stream_content / parse_nstream_content overrides, the parser state on AutoModel and openai_policy.hpp are removed (about 4,300 lines).
  • Gemma4 now stops at <|tool_response> and GPT-OSS at <|call|>.

Constrained decoding (new dependency: xgrammar v0.2.8, Apache-2.0, as a submodule)

  • tool_choice (auto, none, required, a named function), parallel_tool_calls and response_format (json_object, json_schema) become a per-token constraint. Malformed values, unknown functions, required without tools and unbuildable schemas return 400.
  • Every model samples through AutoModel::_sample. Models that force think-prefix tokens in generate() report them with _force_token / _skip_insert_sample.
  • When a model's thinking is off, the grammar does not offer a reasoning block either. Before this, Gemma4 could escape into a thought block under response_format and answer with empty content.
  • Qwen3.8-MTP turns speculative decoding off under a constraint.

Behaviour change: streaming usage

  • Following OpenAI, a streamed response carries usage only when the request sets stream_options.include_usage. Every chunk then has usage: null except a final chunk with no choices. Clients that read usage from streams without asking for it will no longer get it.

Since the last sync with main

  • MiniCPM-V 4.7 (added on main) moves onto the shared library. Its template is detected as qwen_xml with reasoning.
  • The test harnesses that compile automodel.cpp now get the chat sources and xgrammar. The xgrammar build moves into common/chat/chat_sources.cmake, shared by flm, add_npu_test and the rai frontend tests.

The raw text models write to the stream is unchanged, so the CLI, /v1/completions and the Ollama endpoints behave as before. docs/docs/instructions/server/openapi.md documents the new request fields.

Testing

  • flm builds in Release (MSVC, XRT).
  • MiniCPM-V 4.7 on flm serve, 18 end-to-end checks on /v1/chat/completions, with thinking on and off: plain chat (stream and non-stream, no raw tags in output), tools with tool_choice auto, none, required and a named function, response_format json_schema, and image input. All pass.
  • The MiniCPM-V harness runs all four turns, including a prompt-cache follow-up and an image.
  • test_gemma4_frontend (rai) builds and passes.

Not in this PR

🤖 Generated with Claude Code

zaneni6 and others added 13 commits September 23, 2026 12:29
xgrammar (mlc-ai, Apache-2.0) compiles JSON schemas and structural tags into
grammars and computes, per sampling step, which tokens keep the output
valid. It will back tool_choice and response_format.

The submodule is pinned at v0.2.8. Its sources are built as the static
library flm_xgrammar instead of through its own CMakeLists, which would add
install rules and -Werror; cpptrace stays off. Clones need
`git submodule update --init --recursive` for its dlpack headers.

Co-Authored-By: Claude <[email protected]>
Every model carried its own reasoning and tool-call parser. This adds one
library, src/common/chat, that all of them can share:

- chat_format: detects a model's format once, from its chat template and
  vocabulary (hermes_json, qwen_xml, gemma4, lfm2_pythonic, harmony,
  content_only), and derives the prefill the template already wrote.
- chat_parser / chat_args: split output into reasoning, content and tool
  calls; convert Gemma4's relaxed syntax and LFM2's Python calls to JSON,
  including the irregular forms models produce.
- chat_stream / chat_msg: reparse on every flush and emit only what changed.
  The parser holds back anything uncertain (partial markers, unfinished
  names, bare values), so every field only grows and the deltas add up to
  the non-streamed result.
- chat_constraint / chat_grammar / decode_constraint: turn tool_choice,
  parallel_tool_calls and response_format into a per-token decoding
  constraint. tool_choice none bans the tool-call opener; everything else
  is an xgrammar structural tag built from the same format spec the parser
  uses (xgrammar's qwen_3, qwen_3_5 and harmony styles; hand-written EBNF
  for Gemma4 and LFM2 arguments).

Nothing uses the library yet.

Co-Authored-By: Claude <[email protected]>
Sampler::sample and sample_greedy take an optional bitmask (bit i of word
i/32 set when token i may be sampled). Tokens outside it are set to -inf in
the sampler's own fp32 copy of the logits; the engine's buffer is only read.
Without a mask the sampling path is unchanged.

Co-Authored-By: Claude <[email protected]>
Wire src/common/chat into AutoModel, the models and /v1/chat/completions,
and delete the per-model parsers it replaces (about 4,300 lines).

Parsing:
- AutoModel detects the chat format when it sets up the tokenizer (also for
  Phi-4 and Qwen3.5-Omni) and keeps the last 32 prompt tokens so the parser
  knows what the template already opened. Gemma4 now stops at
  <|tool_response> and GPT-OSS at <|call|>.
- Streaming goes through chat_stream: OpenAI deltas for reasoning_content,
  content and tool_calls, with ids made once per call. Non-streamed replies
  use the same parser. Tool markup only counts as a call when the request
  has tools; content is null for a reply that is only tool calls.
- Removed: parse_stream_content / parse_nstream_content in every model,
  parser state on AutoModel, openai_policy.hpp, the duplicated Gemma4
  argument parsers and GPT-OSS's unused tool state machine.

Constrained decoding:
- tool_choice (auto, none, required, a named function), parallel_tool_calls
  and response_format (json_object, json_schema) become a decode_constraint
  on chat_meta_info_t. Malformed values, unknown functions, required without
  tools and unbuildable schemas return 400.
- Every model samples through AutoModel::_sample, which applies the
  constraint's mask. _begin_turn feeds the prefill, _force_token reports the
  think tokens some generate()s force, and _skip_insert_sample drops the
  sample insert() made when generate() does not use it.
- Qwen3.8-MTP turns speculative decoding off under a constraint.
- The per-model tool start token ids are gone; tool_choice none bans the
  format's opener.

The raw text models write to the stream is unchanged, so the CLI,
/v1/completions and the Ollama endpoints behave as before.

Co-Authored-By: Claude <[email protected]>
…r it

Following OpenAI, a streamed chat or text completion no longer carries
usage unless the request sets stream_options.include_usage. When it
does, every chunk has a null usage and one final chunk with no choices
carries the usage of the whole request before [DONE]. The server log
still prints the usage either way.

The usage contract test and the sweep script now request it, and the
contract test checks the chunk layout.

Co-Authored-By: Claude <[email protected]>
xgrammar's sources hold UTF-8 literals, among them the SentencePiece "▁"
that DetectMetadataFromHF compares tokenizer.json against. MSVC reads
source in the system code page unless told otherwise, so those bytes only
survived on machines whose code page happens to pass them through.

Co-Authored-By: Claude <[email protected]>
The constraint only dropped the optional reasoning block when the request
said think=false or reasoning_effort=none. Gemma4 thinks only when asked,
so with response_format set it was kept from its first choice of token
and escaped into a <|channel>thought block, often spending the whole
budget there and answering with empty content.

AutoModel::thinking_enabled() reports the switch the chat template is
rendered with; the six models that have one return it, and make_constraint
turns allow_reasoning off when it is false.

Co-Authored-By: Claude <[email protected]>
MiniCPM-V landed on main with its own reasoning and tool-call parsers and
the single-token tool_choice mask. Drop both: its template is detected as
qwen_xml with reasoning, and decode() samples through _sample() so
tool_choice and response_format constraints hold. The think prefix that
insert() holds back is reported to the constraint like Qwen3_5VL does.

Co-Authored-By: Claude <[email protected]>
Every harness that compiles automodel.cpp stopped linking once it started
parsing and constraining through the chat library. The xgrammar build moves
into common/chat/chat_sources.cmake, which flm, add_npu_test and the rai
frontend tests now share; the Linux Makefiles add the chat sources and link
the main build's xgrammar. The rai Sampler stubs take the new mask argument.

Co-Authored-By: Claude <[email protected]>

@tawei-amd tawei-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review notes: 4 findings, posted inline. The highest-priority one is the constraint turning itself off in chat_grammar.cpp.

The new parser, the streamed-delta logic and the argument converters held up: tracing partial and complete parses for every format found no case where a later parse rewrites text that was already streamed. The sampler mask, the per-model forced/skip-insert calls and the gpt-oss / Gemma4 stop-token handling also look consistent.

🤖 Generated with Claude Code

Comment thread src/common/chat/chat_grammar.cpp Outdated
text = text.substr(skip_.size());
}
prefill_ = text;
if (!text.empty() && !matcher_.AcceptString(text)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constraint turns off for the whole request when the prompt already opens a reasoning block the grammar doesn't allow.

If AcceptString(prefill) is rejected here, give_up() drops the constraint for the rest of the request. It doesn't switch to with_reasoning_, the grammar make() compiles for exactly this case and which accept_forced() already uses.

Example: Qwen3.8-MTP with thinking on puts <think>\n in the prompt, so the prefill is "<think>". A request with reasoning_effort: "none" sets allow_reasoning=false without changing enable_think. The prefill is rejected, and response_format / tool_choice are then not enforced; the only trace is a WARNING line in the server log. The same happens for any model whose template opens <think> without supporting think:false.

Suggestion: fall back to with_reasoning_ when the prefill is a reasoning opener (as accept_forced() does), or return a 400 when reasoning can't be turned off for the model.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, fixed in 444a1e4. start() now falls back to with_reasoning_ when the prefill is rejected. The replay logic moved into use_reasoning_grammar(), which start() and accept_forced() now share.

One correction to the example: on Qwen3.8-MTP, reasoning_effort: "none" does reach the model through configure_chat_engine_parameters(), so it sets enable_think=false and the template closes the block itself. That request didn't trigger this. The case that does is the one you mentioned at the end: a model with no thinking switch whose template always opens <think>, sent think: false.

@@ -590,6 +588,8 @@ std::string Qwen3_5_Omni::generate(chat_meta_info_t& meta_info, int length_limit
// manually decoding <think> \n <\think> \n

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qwen3.5-Omni doesn't override thinking_enabled(), so it inherits the default true.

generate() always writes an empty <think>\n\n</think>\n\n block into the context, so this model effectively always runs with thinking off. If its template mentions <think>, the grammar still offers an optional reasoning block before the JSON or tool call. Under response_format, the model can then open <think> and the answer lands in reasoning_content with empty content. This is the same escape this PR fixes for Gemma4.

Suggestion: override thinking_enabled() to return false here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, fixed in f1d2a11. Qwen3_5_Omni::thinking_enabled() now returns false, which matches the empty think block generate() always feeds. That's the same as Qwen3.5-VL's enable_think=false path.

Comment thread src/common/chat/chat_constraint.cpp Outdated
chat_constraint_request& out, std::string& error) {
out = chat_constraint_request();
out.tools = &tools;
out.parallel_tool_calls = request.value("parallel_tool_calls", true);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit null values in request fields now cause errors.

  • request.value("parallel_tool_calls", true) (here) and request.value("reasoning_effort", "") (L19) throw json::type_error when the key is present with null. The outer catch turns that into a 500.
  • "tool_choice": null (L23) becomes a 400. Before this PR it fell back to auto with a warning.

Some OpenAI clients send explicit null for unset options. Suggest treating null the same as a missing key, e.g. check contains(k) && !request[k].is_null() before reading.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 84dffcf. chat_constraint_request_from_openai() now reads null as missing for parallel_tool_calls, reasoning_effort, tool_choice, and the nested response_format.type / tool_choice.function.name. A non-boolean parallel_tool_calls now gets a 400 instead of a 500.

Fixing only this file wasn't enough. configure_chat_engine_parameters() already returned a 500 on main for any null sampling option, think or reasoning_effort, and the OpenAI chat handler did the same for max_tokens, stream and tools. Both now go through has_value() / value_or() helpers. test_errors.py has a new null_options_are_unset case.

meta_info.insert_sample_pending = false;
}

std::shared_ptr<decode_constraint> AutoModel::make_constraint(const chat_constraint_request& request) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tool_choice: "required" or a named function now forces a tool call even on models that never see the tools.

On models with enable_tool=false (Qwen3 up to 1.7B, Qwen3.5-VL 0.8B, small Gemma4e), apply_chat_template leaves the tools out of the prompt, but make_constraint still builds a grammar that forces a call. The response follows the schema, but the model filled in the arguments without being told what the tool does.

The old get_tool_start_token_id() was gated on enable_tool, so "required" fell back to auto on these models. Suggest keeping that gate here, or returning a 400 for required/named tool choice when the model has tools disabled.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on the problem, fixed in 3b52b39. A small correction on the history: on main, "required" and the named-function form fell back to auto on every model. get_tool_start_token_id() was only used to ban the opener for "none".

The fix adds a tools_enabled() hook beside thinking_enabled(), overridden in the classes whose apply_chat_template checks enable_tool. When it's false, make_constraint() returns a 400 for required or a named function, and drops the tool grammar under auto.

Small Gemma4e isn't affected: its apply_chat_template passes tools whatever enable_tool says, both on main and here, so I left it alone. In practice this applies to qwen3:0.6b, qwen3:1.7b and qwen3.5:0.8b. The API tests skip tool cases on the new error, and the docs list these models.

@tawei-amd

Copy link
Copy Markdown
Collaborator

License check on the new xgrammar dependency

There's no license conflict. One attribution item is missing, though the repo already lacks it for its other dependencies.

This PR compiles xgrammar (submodule pinned to 97787376) into the static library flm_xgrammar and links it into flm, so its code ships in the release binaries:

Component License Compiled in
xgrammar cpp/, include/ Apache-2.0, with a NOTICE file yes
picojson (bundled with xgrammar) BSD-2-Clause yes, headers only
dlpack (bundled with xgrammar) Apache-2.0 yes, headers only
cpptrace (bundled with xgrammar) MIT no (XGRAMMAR_ENABLE_CPPTRACE=0)
cpp/tvm_ffi/ — no, excluded by the PR

All of these are permissive licenses and compatible with the MIT runtime. I found no GPL code and no code taken from other projects in xgrammar's sources: every file has only an "XGrammar Contributors" copyright header.

Missing: when the binaries ship (Windows installer, .deb), Apache-2.0 §4(a) and §4(d) require a copy of the license and the text of xgrammar's NOTICE file. BSD-2 requires picojson's copyright notice and disclaimer in the documentation shipped with the binaries. The PR adds neither, and nothing in src/inno/terms.txt or debian/ covers them. tokenizers-cpp (also Apache-2.0) and minja have the same gap; today they are only credited with a line in the README.

Suggestions:

  • In this PR: add xgrammar to the README credits next to tokenizers-cpp and minja.
  • Follow-up: add a THIRD_PARTY_NOTICES.txt covering xgrammar, picojson, dlpack, tokenizers-cpp and minja, and include it in the installer and the .deb.

This isn't legal advice. If there's an internal open-source review process for new dependencies, this one should go through it.

🤖 Generated with Claude Code

zaneni6 and others added 9 commits October 16, 2026 15:14
Add guides for reasoning and tool calls, structured output, and the
chat output internals, and update the OpenAPI and tool calling pages.

Co-Authored-By: Claude <[email protected]>
Cover response shape, tool calls, response_format, reasoning, and
error handling against a running FLM server.

Co-Authored-By: Claude <[email protected]>
When a request turns thinking off but the template still opens <think>
(a model without a thinking switch), the prefill was rejected and the
constraint gave up for the whole request, leaving response_format and
tool_choice unenforced. start() now falls back to the precompiled
grammar with reasoning, as accept_forced() already did; both share
use_reasoning_grammar().

Co-Authored-By: Claude <[email protected]>
generate() always feeds an empty think block, but the model inherited
thinking_enabled() == true, so a constrained request still offered an
optional reasoning block before the JSON or tool call and the answer
could escape into reasoning_content.

Co-Authored-By: Claude <[email protected]>
Models with enable_tool off (Qwen3 up to 1.7B, Qwen3.5 0.8B) leave the
tools out of the prompt, yet tool_choice "required" or a named function
built a grammar that forced a call, with arguments made up for a tool
the model never saw. A new tools_enabled() hook lets make_constraint()
answer those with a 400 and drop the tool grammar under "auto". The API
tests skip tool cases on the new error.

Co-Authored-By: Claude <[email protected]>
Some OpenAI clients send null for every option they leave unset.
parallel_tool_calls: null became a 500 and tool_choice: null a 400;
null sampling options, think, reasoning_effort, max_tokens, stream and
tools were already 500s. The constraint parser and the OpenAI chat
handler, including configure_chat_engine_parameters(), now read null as
missing.

Co-Authored-By: Claude <[email protected]>
xgrammar is now statically linked into flm, with picojson and dlpack
compiled in through it. Their notices go into THIRD_PARTY_NOTICES.txt,
which the installers and the .deb already ship: xgrammar's Apache-2.0
attribution and NOTICE text (Apache-2.0 4(a), 4(d)), picojson's BSD-2
copyright and disclaimer, and dlpack's Apache-2.0 attribution. The
README credits xgrammar next to tokenizers-cpp and minja.

Co-Authored-By: Claude <[email protected]>
@zaneni6

zaneni6 commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

Thanks for the license check. Fixed in 5072086.

  • THIRD_PARTY_NOTICES.txt now covers XGrammar (style change on Readme #29: Apache-2.0 attribution plus the text of its NOTICE file), PicoJSON (removed remote machine file upload #30: full BSD-2 copyright and disclaimer, from picojson.h) and DLPack (0.1.5 doc update #31: Apache-2.0 attribution). The Apache-2.0 text is already at the end of the file. cpptrace isn't listed, since it isn't compiled in (XGRAMMAR_ENABLE_CPPTRACE=0).
  • The README credits xgrammar next to tokenizers-cpp and minja.

Main has picked up most of the follow-up you suggested since your comment (7d0b0957, merged into this branch). THIRD_PARTY_NOTICES.txt already lists tokenizers-cpp and minja, and it ships with the Inno installer (src/inno/flm.iss), the WiX MSI (src/wix/flm.wxs), and the .deb through cmake --install (src/CMakeLists.txt). So the only gap left was the three components this PR adds.

I agree this should still go through the internal open-source review for new dependencies.

Edit: renumbered to #29–31 after merging main (b413aef), which dropped FFmpeg and added the image/audio decoders as #23–28.

zaneni6 and others added 2 commits October 19, 2026 10:50
Conflict in THIRD_PARTY_NOTICES.txt: main dropped FFmpeg and added the
image and audio decoders (now 28 entries). Took main's file and re-added
XGrammar, PicoJSON and DLPack as entries 29-31.

Co-Authored-By: Claude <[email protected]>
Resolve THIRD_PARTY_NOTICES.txt: main added HRX, flatcc and pkgconf as
entries 29-31; renumber this branch's XGrammar, PicoJSON and DLPack to 32-34.

Co-Authored-By: Claude <[email protected]>
@zaneni6
zaneni6 merged commit 4fc1758 into main Oct 19, 2026
10 of 13 checks passed
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.

2 participants