Skip to content

fix(processors): treat Fireworks model ids as reasoning-hint incompatible #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
himorishige wants to merge 1 commit into
base: main
Choose a base branch
from
+13 −5
Open

fix(processors): treat Fireworks model ids as reasoning-hint incompatible #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions switchyard/lib/processors/reasoning_hint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,21 @@

from __future__ import annotations

# Anthropic-served (Claude) ids reject the vLLM ``chat_template_kwargs`` field;
# vLLM-served reasoning models that need the hint never carry these tokens.
_NO_REASONING_HINT_TAGS = ("anthropic", "bedrock", "claude")
# Anthropic-served (Claude) ids reject the vLLM ``chat_template_kwargs``
# field, and Fireworks AI's OpenAI-compatible API validates request bodies
# strictly and 400s on it as well ("Extra inputs are not permitted");
# Fireworks ids always carry the provider namespace
# (``accounts/fireworks/models/...``). vLLM-served reasoning models that
# need the hint never carry these tokens.
_NO_REASONING_HINT_TAGS = ("anthropic", "bedrock", "claude", "fireworks")

@elyasmnvidian elyasmnvidian Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This adds fireworks to the deny list so we stop sending chat_template_kwargs to Fireworks. Recap for anyone reading: chat_template_kwargs is a vLLM-specific request field — it passes keyword arguments into the model's chat template and isn't part of the OpenAI API. model_accepts_reasoning_hint decides whether we send it, answering "no" only when the model name contains one of these tags.

Two things I looked into that make me wonder if a different structure would hold up better:

1. This list has to grow one provider at a time, and it already misses one. azure/deepseek-ai/deepseek-v4-pro runs on the same gateway, its name matches none of these tags, so we'd still send it the field — and Azure also validates the body strictly. Every new strict provider needs another tag added here by hand.

2. Each provider turns reasoning off with a different knob, so a yes/no "does it accept the hint?" can only ever suppress the field — it can't send the right one. From the docs and source:

Provider Accepts chat_template_kwargs? How you actually turn reasoning off
vLLM yes (unknown fields are ignored, not rejected) chat_template_kwargs.enable_thinking=false for Qwen3 — DeepSeek-V3.1 uses the key thinking with the opposite default
Fireworks no — 400 top-level reasoning_effort: "none"
Azure AI (DeepSeek) no — 400 no request parameter; the model decides
OpenAI no reasoning_effort
Anthropic no thinking: {"type": "disabled"}
DeepSeek API no choose the model: deepseek-chat (off) vs deepseek-reasoner (on)

Sources: vLLM defines the field and allows (ignores) unknown ones rather than rejecting them — protocol.py, engine/protocol.py, reasoning outputs; Fireworks reasoning guide; OpenAI reasoning; Anthropic extended thinking; DeepSeek thinking mode; Azure chat reasoning.

Concrete suggestion: instead of a deny-list, a map keyed by provider / served-model family → a small dict of per-provider request settings. reasoning_off is just one entry in that dict, so the next provider-specific quirk we hit (a required header, another body field, a format tweak) becomes another key alongside it — not a whole new parallel map to keep in sync:

# provider / model-family -> per-provider request settings
# reasoning_off is one key here; future per-provider quirks live alongside it
PROVIDER_SETTINGS = {
    "vllm-qwen":     {"reasoning_off": {"chat_template_kwargs": {"enable_thinking": False}}},
    "vllm-deepseek": {"reasoning_off": {}},                        # DeepSeek-V3.1 default is off (key is `thinking`)
    "fireworks":     {"reasoning_off": {"reasoning_effort": "none"}},
    "openai":        {"reasoning_off": {"reasoning_effort": "minimal"}},
    "anthropic":     {"reasoning_off": {"thinking": {"type": "disabled"}}},
    # azure deepseek / deepseek-api: no per-request reasoning switch
}
# lookup: PROVIDER_SETTINGS.get(provider, {}).get("reasoning_off")

One honest caveat so we don't over-build it: the hard part is the key, not the map — vLLM's own knob differs by model family (Qwen3 enable_thinking vs DeepSeek-V3.1 thinking), so keying on the model-id string still needs care. If we'd rather not maintain provider detection here at all, the other option is to let each target/endpoint declare its reasoning-off body in config (I left that note on #122).



def model_accepts_reasoning_hint(model: str) -> bool:
"""Whether ``model`` tolerates the vLLM ``enable_thinking=False`` hint.

``False`` for Anthropic/Bedrock/Claude ids (they 400 on it), else ``True``
— usable directly as a classifier ``disable_reasoning`` default.
``False`` for Anthropic/Bedrock/Claude and Fireworks-served ids (they
400 on it), else ``True`` — usable directly as a classifier
``disable_reasoning`` default.

@elyasmnvidian elyasmnvidian Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some context for this line: the LLM classifier is a small model that reads each incoming request and returns a JSON decision about which tier (weak or strong) should handle it. disable_reasoning controls whether we send enable_thinking=false. That hint matters because DeepSeek V4 is a reasoning model: without it, the model can put its JSON decision in the reasoning_content field and leave the normal content field empty. The classifier only reads content, so an empty content means it can't parse a decision and falls back to the default tier.

This change makes model_accepts_reasoning_hint return False for Fireworks, so the classifier stops sending the hint to a Fireworks classifier model. My question: does a Fireworks-served DeepSeek V4 still put its answer in content when we don't send the hint? If it behaves like the vLLM version and leaves content empty, we've replaced a clear error (the 400) with a silent one — the classifier gets nothing back and falls back on every request, with no error to notice.

Concrete suggestion: worth one real call to a Fireworks DeepSeek V4 with a classifier-style prompt and no hint, then check whether content is filled in. If it comes back empty, note that Fireworks disables reasoning with the top-level reasoning_effort: "none" field, not chat_template_kwargs (docs) — so the fix would be to send that for Fireworks rather than nothing, which is what the map on the line above would let us do. The two places that read disable_reasoning are llm_classifier/request_processor.py and stage_router/classifier.py.

"""
lowered = model.lower()
return not any(tag in lowered for tag in _NO_REASONING_HINT_TAGS)
Expand Down
3 changes: 3 additions & 0 deletions tests/test_reasoning_hint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
"azure/anthropic/claude-opus-4-6",
"anthropic/claude-3-5-sonnet",
"Bedrock-Claude-Whatever", # case-insensitive
"accounts/fireworks/models/deepseek-v4-flash",
"accounts/fireworks/models/deepseek-v4-pro",
"accounts/fireworks/models/glm-5p2",
],
)
def test_claude_family_rejects_hint(model: str) -> None:
Expand Down