Skip to content

[Bugfix][Frontend] GLM-4.7: guided-decode forced tool calls when strict tool-calling is disabled - #47175

Closed
ifaddict1 wants to merge 1 commit into
vllm-project:mainfrom
ifaddict1:glm47-honor-strict-tool-calling
Closed

ifaddict1 wants to merge 1 commit into
vllm-project:mainfrom
ifaddict1:glm47-honor-strict-tool-calling

Conversation

@ifaddict1

@ifaddict1 ifaddict1 commented Jun 30, 2026 •

Copy link
Copy Markdown

Purpose

On GLM-4.7 / GLM-5.2, forced tool calls (tool_choice="required" or a named function) return unconstrained native tool syntax (or an empty tool_calls list) when VLLM_ENFORCE_STRICT_TOOL_CALLING=0, which then fails downstream argument-schema validation — most visibly for tool schemas that use nested $defs/$ref.

There are two halves to the bug, and fixing only one is not enough:

  1. Parse side. Glm47MoeModelToolParser hardcodes supports_required_and_named = False, overriding ToolParser.__init_subclass__. So even at strict=0, GLM never takes the JSON-based parse path for forced calls (every other structural-tag parser flips this to True at strict=0). This is the change in Allow GLM-4.7 required tool parsing when strict mode is disabled #47146.
  2. Encode side. At strict=0 the structural-tag path is skipped (get_structural_tag(...) is None) and nothing else installs a grammar for forced tool calls. The request therefore goes to the engine with no structured-outputs grammar, the model free-generates native tool syntax, and the now-JSON-based parse path cannot validate it. Flipping the flag alone (Allow GLM-4.7 required tool parsing when strict mode is disabled #47146 on its own) helps flat schemas but still leaves nested-schema forced calls unconstrained.

This PR fixes both:

  • glm47_moe_tool_parser: drop the hardcoded flag and let __init_subclass__ govern it via VLLM_ENFORCE_STRICT_TOOL_CALLING, like every other structural-tag parser.
  • DelegatingParser: when no structural tag applies (strict=0) but tool_choice is required/named, build StructuredOutputsParams(json=...) from the request's own tool schema (mirroring the response_format json path). For required, the parameters are nested under {name, parameters} to match the list[FunctionDefinition] shape the parse path validates, and any $defs are hoisted to the top level so #/$defs/... refs stay resolvable after nesting.

The grammar is built entirely from the caller's existing tool definitions (no transpilation/heuristics), so a forced call is constrained to exactly the schema the caller already declared. The change only takes effect when supports_required_and_named is True (i.e. strict=0) and structured_outputs is not already set, so the structural-tag (strict=1) and response_format paths are untouched.

Builds on #47146 (parse-side flag) by adding the missing encode-side grammar.

Test Plan

Unit tests (tests/tool_parsers/test_structural_tag_registry.py):

pytest tests/tool_parsers/test_structural_tag_registry.py -k "build_tool_choice or glm47_does_not_pin or honors_strict_env_toggle"

End-to-end on a GLM-4.7-family model served with --enable-auto-tool-choice --tool-call-parser glm47 and VLLM_ENFORCE_STRICT_TOOL_CALLING=0, sending a tool whose parameters contain a nested object referenced via $defs/$ref, with tool_choice="required", a named tool_choice, and a response_format json_schema control.

Test Result

Unit tests: 4 passed (flag toggle is honored; GLM-4.7 no longer pins the flag; _build_tool_choice_json_schema returns the function params for a named choice and an array-of-{name, parameters} with hoisted $defs for required).

E2E (GLM-5.2, strict=0):

  • Before: tool_choice="required" → empty tool_calls; named tool_choice → arguments contained raw native tool syntax; both failed schema validation on nested fields.
  • After: both required and named return schema-valid JSON with the nested object populated (e.g. {"location": {"file": "app.py", "line": 14}, ...}). response_format json output is unchanged.

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR.
  • The test plan.
  • The test results.
  • (Optional) Documentation update — n/a (behavior fix, no new surface).

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

GLM-4.7's tool parser hardcoded `supports_required_and_named = False`, so
even with `VLLM_ENFORCE_STRICT_TOOL_CALLING=0` forced (required/named) tool
calls never took the JSON-based parse path. Combined with the structural-tag
encode path being skipped at strict=0, forced calls were emitted with no
grammar at all and came back as unconstrained native syntax that failed
schema validation -- especially for nested `$defs`/`$ref` argument schemas.

- glm47_moe_tool_parser: drop the hardcoded flag and let `__init_subclass__`
  govern it via `VLLM_ENFORCE_STRICT_TOOL_CALLING`, like every other
  structural-tag parser (strict=0 -> True).
- DelegatingParser: when no structural tag applies (strict=0) but tool_choice
  is required/named, build `StructuredOutputsParams(json=...)` from the
  request's own tool schema so forced calls guided-decode to schema-valid JSON
  that the JSON-based parse path validates. For "required" the parameters are
  nested under {name, parameters} and `$defs` are hoisted to the top level so
  `#/$defs/...` refs stay resolvable.

Adds unit tests for the flag toggle and the schema builder (named + required
with $defs hoisting).

Signed-off-by: Selim <selim@aikido.dev>
@ifaddict1
ifaddict1 force-pushed the glm47-honor-strict-tool-calling branch from 493d021 to 73aab0b Compare June 30, 2026 15:16
@ifaddict1 ifaddict1 changed the title [tool] GLM-4.7: guided-decode forced tool calls when strict mode is disabled [Bugfix][Frontend] GLM-4.7: guided-decode forced tool calls when strict tool-calling is disabled Jun 30, 2026
@mergify mergify Bot added the bug Something isn't working label Jun 30, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@bbartels

bbartels commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

We are running into this issue as well, thanks for raising!

alexeldeib added a commit to alexeldeib/vllm that referenced this pull request Jul 13, 2026
GLM required-tool structural decoding can emit <tool_call> and
</tool_call> as ordinary subtokens rather than their dedicated special-token
IDs. Once any token IDs have appeared, the streaming parser intentionally
demotes text-matched token-ID terminals to content. The GLM tool parser then
leaks raw tool markup into message.content and emits no tool_calls.

Preserve strict token-ID matching by default. Add an explicit lexical fallback
and enable it only for GLM TOOL_START and TOOL_END while processing
tool_choice="required". Auto and plain GLM generations retain the existing
literal-marker protection, and other parser-engine models are unchanged.

Configure the fallback through GLM's request-aware tool-parsing hook so both
direct ParserEngine streams and ParserEngineToolAdapter streams use it. Add
coverage for the default strict behavior, the explicit fallback, and the GLM
tool-adapter path that exposed the bug.

Duplicate-work check:

- vllm-project#48095 reports the same v0.24 user-visible failure.
- vllm-project#47175 and vllm-project#47146 address non-strict JSON-guided generation, not streamed
  marker classification.
- vllm-project#48103 was closed and changed request grammar selection rather than parser
  routing.
- No open PR was found for GLM lexical marker routing.

Validation:

- pre-commit passed for all changed files, including ruff and mypy.
- git diff --check passed.
- A vLLM 0.24 image carrying the same runtime changes passed all four targeted
  GLM-5.2 tool-contract tests and produced a full Maru result of 40 passed,
  1 unrelated structured-output multipleOf failure, and 4 skipped.
- Targeted pytest could not collect in the current lightweight local environment because numpy is not installed; no dependencies were added to the shared environment.

AI assistance was used to inspect parser routing, prepare the patch and tests,
and run the listed validation. The human submitter must review and defend the
change end to end before proposing it upstream.

Assisted-by: OpenAI Codex
Signed-off-by: Ace Eldeib <aeldeib@coreweave.com>
yoloyash added a commit to yoloyash/overtchat that referenced this pull request Jul 26, 2026
Stacks the work that accumulated on `dev` since v0.10.0 into the v0.11.0 web release.

## What's changed

- Reworked web search around automatic tool choice. Tool-capable models now decide when to search instead of requiring a per-chat toggle, with a one-message **Search** action in the composer for forcing it and a persistent hard-disable under **Settings → Tools**.
- Added a per-model **Tool calling** switch (migration 0004) so models without tool support can be excluded from tool-bearing requests. The connection tester now verifies an actual tool call when the switch is on.
- Added provider prompt caching: Anthropic cache breakpoints on the system prefix and the latest user turn, and a stable per-chat `promptCacheKey` for OpenAI derived from a hash of the chat ID. The prompt is append-only so prior prefixes stay byte-for-byte reusable.
- Surfaced cache accounting in message stats — uncached input, cache read, and cache write tokens.
- Replaced the per-request runtime-context block with a date-only `Current date:` line resolved against a validated client time zone, keeping the system prefix cache-stable across turns.
- Rewrote the citation instructions and anchor scheme so anchors address the search call and result index (`turnNsearchI`), fixing attribution when a response performs more than one search.
- Upgraded the Vercel AI SDK from v6 to v7 across web, mobile, and shared, moving chat execution onto `ToolLoopAgent` with `createUIMessageStreamResponse`.
- Expanded model brand icon coverage and model-ID matching to more top providers.
- Added a `?q=` shortcut for starting a new chat from a URL, normalizing repeated and blank values and waiting for a valid selected model before submitting.
- Restructured the chat title prompt as a single labelled task with the user's message fenced at the end, which stops markdown formatting and conversational replies from leaking into titles.
- Updated the default Compose app image to 0.11.0.

## Upgrade note

Migration 0004 runs on startup and adds `tool_calling_enabled`, defaulting to on for existing model configurations.

**Web search is now automatic and on by default**, where it used to be opt-in per chat. Turn it off under **Settings → Tools**, or switch off **Tool calling** for a model that can't use tools — otherwise its connection test will fail, since the test now requires a real tool call. Older Android clients keep working and will start searching once the server is upgraded.

One caveat if you self-host inference: the Search action and the connection test both send `tool_choice: "required"`, and vLLM's XML-format tool parsers (`poolside_v1`, GLM-4) silently downgrade that to `auto` instead of erroring — so on a model like Laguna S2.1, automatic search works but forcing it doesn't, and the connection test can fail on a model whose tool calls are otherwise fine. Nothing to fix on this side; see vllm-project/vllm#47175.

## Validation

`npm run test` (166), `lint`, `typecheck`, `build`, `deps:check`, `docker compose config`, `docker build`. Playwright smoke against the standalone server with live Gemini streaming. Upgraded a v0.10.0-era database to confirm 0004 applies and existing model configurations survive.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@mergify mergify Bot added the glm label Aug 20, 2026
josevh pushed a commit to josevh/overtchat that referenced this pull request Sep 2, 2026
…ash#107)

Stacks the work that accumulated on `dev` since v0.10.0 into the v0.11.0 web release.

## What's changed

- Reworked web search around automatic tool choice. Tool-capable models now decide when to search instead of requiring a per-chat toggle, with a one-message **Search** action in the composer for forcing it and a persistent hard-disable under **Settings → Tools**.
- Added a per-model **Tool calling** switch (migration 0004) so models without tool support can be excluded from tool-bearing requests. The connection tester now verifies an actual tool call when the switch is on.
- Added provider prompt caching: Anthropic cache breakpoints on the system prefix and the latest user turn, and a stable per-chat `promptCacheKey` for OpenAI derived from a hash of the chat ID. The prompt is append-only so prior prefixes stay byte-for-byte reusable.
- Surfaced cache accounting in message stats — uncached input, cache read, and cache write tokens.
- Replaced the per-request runtime-context block with a date-only `Current date:` line resolved against a validated client time zone, keeping the system prefix cache-stable across turns.
- Rewrote the citation instructions and anchor scheme so anchors address the search call and result index (`turnNsearchI`), fixing attribution when a response performs more than one search.
- Upgraded the Vercel AI SDK from v6 to v7 across web, mobile, and shared, moving chat execution onto `ToolLoopAgent` with `createUIMessageStreamResponse`.
- Expanded model brand icon coverage and model-ID matching to more top providers.
- Added a `?q=` shortcut for starting a new chat from a URL, normalizing repeated and blank values and waiting for a valid selected model before submitting.
- Restructured the chat title prompt as a single labelled task with the user's message fenced at the end, which stops markdown formatting and conversational replies from leaking into titles.
- Updated the default Compose app image to 0.11.0.

## Upgrade note

Migration 0004 runs on startup and adds `tool_calling_enabled`, defaulting to on for existing model configurations.

**Web search is now automatic and on by default**, where it used to be opt-in per chat. Turn it off under **Settings → Tools**, or switch off **Tool calling** for a model that can't use tools — otherwise its connection test will fail, since the test now requires a real tool call. Older Android clients keep working and will start searching once the server is upgraded.

One caveat if you self-host inference: the Search action and the connection test both send `tool_choice: "required"`, and vLLM's XML-format tool parsers (`poolside_v1`, GLM-4) silently downgrade that to `auto` instead of erroring — so on a model like Laguna S2.1, automatic search works but forcing it doesn't, and the connection test can fail on a model whose tool calls are otherwise fine. Nothing to fix on this side; see vllm-project/vllm#47175.

## Validation

`npm run test` (166), `lint`, `typecheck`, `build`, `deps:check`, `docker compose config`, `docker build`. Playwright smoke against the standalone server with live Gemini streaming. Upgraded a v0.10.0-era database to confirm 0004 applies and existing model configurations survive.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@mergify

mergify Bot commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @ifaddict1.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 27, 2026

@sfeng33 sfeng33 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the work! Closing in favor of #47512. On current main, ToolParser.adjust_request already builds the JSON grammar for required/named calls, including moving $defs to the top level, so the encode-side change isn't needed. The actual bug was that grammar being applied to parsers that read their native format. We're fixing that by skipping the JSON constraint for those parsers rather than switching GLM-4.7 to JSON parsing. For enforced forced calls on GLM, keep VLLM_ENFORCE_STRICT_TOOL_CALLING=1 (the default), which uses the structural tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants