feat(gateway): honor per-model parser overrides from model cards

[pallasathena92]

Description

Problem

Parser selection on the gRPC serving path is process-global: one --tool-call-parser / --reasoning-parser name per router process. The tokenizer side is already per-model (TokenizerRegistry), so a gateway serving two model families selects the right tokenizer for each but not the right parsers — one family's tool calls come back unparsed in content. Meanwhile ModelCard.tool_parser / ModelCard.reasoning_parser exist in crates/protocols with builders and zero read sites.

Solution

Wire the card fields end to end via the existing label pipeline: workers (or explicit WorkerSpec cards) declare per-model parser names; the gRPC path resolves per request with precedence card override → configured global → existing name-based auto-detection (unchanged). Unknown names fail worker registration loudly instead of shipping silently unparsed output.

Changes

• build_model_card maps tool_parser / reasoning_parser labels into the card; explicit user-provided cards keep their own values (same precedence tokenizer_path uses).
• CreateLocalWorkerStep validates override names against the parser registries at registration (same fail-fast the global CLI names get in AppContext).
• A ParserResolver replaces the two Option<String>s threaded through ResponseProcessor, StreamingProcessor, chat/messages preparation stages, and SharedComponents; resolution borrows from worker metadata (no card clones on the hot path); conflicting same-model overrides resolve to the lexicographically smallest name (deterministic) with a loud registration-time warning, and resolved names are threaded through all downstream helpers so one request never mixes parsers even if the registry changes mid-stream.
• Parser-free endpoints (completion/embeddings/classify) keep a disabled() resolver — prior behavior byte-for-byte. Debug /parse/* endpoints keep their explicit per-request override.

Test Plan

• build_model_card: label mapping, empty-label = unset, explicit-card precedence.
• validate_parser_overrides: known/unknown names, absent factories skip.
• ParserResolver: card override wins over configured, configured fallback without a card, none→none, disabled resolver, deterministic conflict resolution under both registration orders.
• cargo test -p smg --lib: 1456 passed. cargo +nightly fmt clean; cargo clippy -p smg --all-targets -- -D warnings clean locally (--all-features via CI).
• No behavior change without overrides: globals then auto-detection, as today.

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets --all-features -- -D warnings passes
• (Optional) Documentation updated
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Parser selection now adapts per model, allowing model-specific tool and reasoning parser overrides to take precedence over configured defaults.
  • Consistent parser behavior is supported across chat, Messages API, tool calls, reasoning, and streaming responses.
  • Local worker registration now carries parser settings from model metadata.

• Bug Fixes

  • Invalid parser configurations are detected during worker registration with clear failure reporting.
  • Parser lookup and availability checks now use the effective settings for each model.

Walkthrough

The change adds ParserResolver for per-model tool and reasoning parser selection. Worker creation imports parser overrides from labels and validates parser names. gRPC regular and streaming paths use resolved names for constraints, availability checks, tag detection, and parser creation.

Changes

Parser metadata and resolution

Layer / File(s)	Summary
Worker parser metadata and validation model_gateway/src/workflow/steps/local/create_worker.rs	Worker creation populates missing model-card parser fields from labels and validates configured parser names when parser factories are available. Tests cover precedence, empty values, known parsers, unknown parsers, and absent factories.
ParserResolver contract model_gateway/src/routers/grpc/utils/parsers.rs, model_gateway/src/routers/grpc/utils/mod.rs	ParserResolver checks model-card overrides before configured parser names and provides a disabled mode. Tests cover override, fallback, missing-source, and disabled behavior.

gRPC integration

Layer / File(s)	Summary
Shared resolver wiring model_gateway/src/routers/grpc/context.rs, model_gateway/src/routers/grpc/router.rs, model_gateway/src/routers/grpc/pipeline.rs, model_gateway/src/routers/grpc/regular/processor.rs, model_gateway/src/routers/grpc/regular/streaming.rs	SharedComponents, ResponseProcessor, and StreamingProcessor now store ParserResolver. Configured pipelines share a resolver, while default pipelines use disabled resolvers.
Non-streaming parser selection model_gateway/src/routers/grpc/regular/processor.rs, model_gateway/src/routers/grpc/regular/stages/chat/preparation.rs, model_gateway/src/routers/grpc/regular/stages/messages/preparation.rs	Chat and Messages API paths resolve parser names from the request model before generating constraints, checking availability, detecting structural tags, and creating parsers.
Streaming parser selection model_gateway/src/routers/grpc/regular/streaming.rs	Streaming chat and Messages API paths use model-resolved parser names for availability checks, structural-tag detection, special-token handling, and parser creation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Request
  participant ParserResolver
  participant ResponseProcessor
  participant ParserFactory

  Request->>ResponseProcessor: process model request
  ResponseProcessor->>ParserResolver: resolve tool and reasoning parsers
  ParserResolver-->>ResponseProcessor: return effective parser names
  ResponseProcessor->>ParserFactory: check availability and create parsers
  ParserFactory-->>ResponseProcessor: return parser instances
  ResponseProcessor-->>Request: return parsed response

Loading

Possibly related PRs

• smg-project/smg#1031: Modifies the same response-processing and parser utility paths for per-request parser selection.
• smg-project/smg#1090: Modifies gRPC preparation paths and structural-tag constraint handling.
• smg-project/smg#1642: Modifies reasoning-parser handling and parser instance creation.

Suggested labels: tool-parser, reasoning-parser

Suggested reviewers: catherinesue, slin1237

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description clearly explains the per-model parser override changes, resolution precedence, validation, integration scope, and test coverage.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: honoring per-model parser overrides from model cards.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/per-model-parser-override

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Clean PR. The ParserResolver abstraction is well-designed — precedence chain is correct (card override → configured global → auto-detect), alias handling is safe via get_by_model, validation rejects unknown parser names at registration time, and tests cover all branches. No issues found.

0 🔴 Important | 0 🟡 Nit | 0 🟣 Pre-existing

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

model_gateway/src/routers/grpc/regular/streaming.rs (1)

320-370: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🔴 Important Keep the resolved parser names stable for one request.

The availability checks use a name resolved at request start. Later helpers query the live WorkerRegistry again. A worker registration or removal can change the selected override during processing. Chat streaming can then reach the expect at process_reasoning_stream after availability was checked for a different parser.

• model_gateway/src/routers/grpc/regular/streaming.rs#L320-L370: pass the resolved Chat parser names into process_reasoning_stream and process_tool_calls_stream.
• model_gateway/src/routers/grpc/regular/streaming.rs#L1889-L1944: pass the resolved Messages reasoning parser name into process_messages_reasoning.
• model_gateway/src/routers/grpc/regular/processor.rs#L246-L271: pass the resolved Chat parser names into process_single_choice and parse_tool_calls.
• model_gateway/src/routers/grpc/regular/processor.rs#L510-L576: keep the resolved Messages parser names through parser creation and tool parsing.

Add tests that change registry membership after availability checks and verify that the request continues with its initial parser selection.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model_gateway/src/routers/grpc/regular/streaming.rs` around lines 320 - 370,
Keep parser selection stable for each request by threading the names resolved
during initial availability checks through all downstream parser operations. In
model_gateway/src/routers/grpc/regular/streaming.rs:320-370, pass the resolved
Chat names to process_reasoning_stream and process_tool_calls_stream; in
streaming.rs:1889-1944, pass the resolved Messages reasoning name to
process_messages_reasoning; in
model_gateway/src/routers/grpc/regular/processor.rs:246-271, pass the resolved
Chat names to process_single_choice and parse_tool_calls; and in
processor.rs:510-576, retain the resolved Messages names through parser creation
and tool parsing instead of rereading WorkerRegistry. Add tests that modify
registry membership after availability checks and verify the request continues
using its initial parser selection.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@model_gateway/src/routers/grpc/utils/parsers.rs`:
- Around line 78-87: Update parser resolution around the registry lookup and
worker-registration flow so same-model workers cannot silently select the first
parser override by iteration order. Reject conflicting parser names for workers
registered under one model, or resolve the parser using the selected output
worker in both regular and PD dual-dispatch paths; add coverage with same-model
workers declaring different parsers and verify the conflict is rejected or the
output worker’s parser is used.

---

Outside diff comments:
In `@model_gateway/src/routers/grpc/regular/streaming.rs`:
- Around line 320-370: Keep parser selection stable for each request by
threading the names resolved during initial availability checks through all
downstream parser operations. In
model_gateway/src/routers/grpc/regular/streaming.rs:320-370, pass the resolved
Chat names to process_reasoning_stream and process_tool_calls_stream; in
streaming.rs:1889-1944, pass the resolved Messages reasoning name to
process_messages_reasoning; in
model_gateway/src/routers/grpc/regular/processor.rs:246-271, pass the resolved
Chat names to process_single_choice and parse_tool_calls; and in
processor.rs:510-576, retain the resolved Messages names through parser creation
and tool parsing instead of rereading WorkerRegistry. Add tests that modify
registry membership after availability checks and verify the request continues
using its initial parser selection.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f3b02df-027b-4c7a-bc99-a79240417d8e

📥 Commits

Reviewing files that changed from the base of the PR and between 6d06af5 and e7744cd.

📒 Files selected for processing (10)

• model_gateway/src/routers/grpc/context.rs
• model_gateway/src/routers/grpc/pipeline.rs
• model_gateway/src/routers/grpc/regular/processor.rs
• model_gateway/src/routers/grpc/regular/stages/chat/preparation.rs
• model_gateway/src/routers/grpc/regular/stages/messages/preparation.rs
• model_gateway/src/routers/grpc/regular/streaming.rs
• model_gateway/src/routers/grpc/router.rs
• model_gateway/src/routers/grpc/utils/mod.rs
• model_gateway/src/routers/grpc/utils/parsers.rs
• model_gateway/src/workflow/steps/local/create_worker.rs

[pallasathena92]

Addressed both findings in the amended commit:

• Conflicting same-model overrides (coderabbitai 🔴): resolution is no longer iteration-order dependent — ParserResolver scans all workers' cards for the model and picks the lexicographically smallest name on conflict (covered by conflicting_overrides_resolve_deterministically, which asserts the same winner under both registration orders). Registration now also warns loudly when a new worker's override disagrees with an already-registered same-model worker, naming both workers and both parsers. Chose warn-not-reject deliberately: rejecting would block rolling upgrades that intentionally change a model's parser, and mixed labels are exactly the transient state those produce. Note the preparation stages (tool-constraint generation) run before worker selection, so resolution is inherently model-level, not output-worker-level — per-output-worker resolution would still leave prefill-vs-decode divergence unaddressed at constraint time.
• Per-request parser-name stability (coderabbitai 🔴 outside-diff, claude 🟡): the resolved names are now threaded from the per-request resolution points into every downstream helper — process_reasoning_stream, process_tool_calls_stream, process_messages_reasoning, process_single_choice, parse_tool_calls — instead of re-consulting the registry. The availability check and the lazy parser creation now see the same value by construction, restoring the pre-PR invariant behind the expects.