Repository navigation
fix(switchyard_model): match vllm_model for reasoning models; pin nemo-switchyard 0.3.0 - #3664
Conversation
yaoyu-33
added
area:model
github-actions
Bot
added
the
sla:triage-overdue
github-actions
Bot
added
sla:review-overdue
github-actions
Bot
removed
the
sla:review-overdue
1a74fe6 to
f835d1a
Compare
github-actions
Bot
added
the
sla:review-overdue
|
/ok to test 24c3cee |
yaoyu-33
added
the
ready-to-merge
|
Hi @ko3n1g, @linj-glitch has updated the docstring per your comment and the required CI is green. Could you take another look? |
|
I'll watch this ready-to-merge PR and report substantive progress. Running on |
|
💬 I can’t verify the PR’s latest checks or label state: both workstation launch attempts were denied by the shared memory quota. Can an operator free workstation quota and confirm when access is restored? The PR watch remains armed. |
|
💬 Access is restored. GitHub reports that the PR is still open at |
|
💬 I couldn’t retrieve the failing job logs: both read attempts returned HTTP 503 because the broker’s native Git authority is unavailable. I haven’t retried CI or changed the PR. Can an operator restore native Git authority and confirm when it’s ready? The PR watch remains active. |
Three gaps kept switchyard_model from standing in for vllm_model when a reasoning model sits behind the proxy. Measured on SWE-bench Verified with Nemotron 3 Nano (500 tasks, k=3): 30.9% resolved through vllm_model, 17.5% through this server with the same weights, tools and sampling. Switchyard forwards the body byte-for-byte; the gap was in what this server handed it. Reasoning on the way back: the server returned the upstream's separate reasoning field verbatim. Agents replay the assistant turn on the next call and Gym's chat request schema forbids reasoning_content there, so every multi-turn rollout died on its second call. Fold it into <think> tags as vllm_model does, behind the same uses_reasoning_parser flag. Reasoning on the way out: vllm_model moves a replayed turn's <think> block back into reasoning_content/reasoning before forwarding; this server sent the tags inline. The chat template renders the two forms differently and the model treats the inline form as a stop cue: replaying six real failing requests four times each, 6/24 turns ended empty after the think block with the inline form and 0/24 with the rewrite. Mirror the rewrite under uses_interleaved_reasoning, default on. Sampling: no way to state it, so a proxied arm ran on checkpoint defaults while a vllm_model arm pinned its own. Add sampling_overrides with vllm_model's semantics; the route id is set last and is not pinnable. Pin nemo-switchyard 0.3.0, the current release; its hosted Server API and request forwarding were verified identical to 0.2.0 for this use. With the change the proxied arm scores 33.5% against 30.9% direct, within one standard error, with matching turn counts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
Retain the reviewer fixes carried by the earlier merge commit, use the renamed ChatCompletionsConverter public API, and lock nemo-switchyard to the configured 0.3.0 version. Signed-off-by: Lin Jia <linj@nvidia.com>
|
/resolve-conflict |
2fa810c to
655d4c9
Compare
|
💬 Head |
|
💬 GitHub now reports approval for |
|
/ok to test 655d4c9 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/42162668264 |
|
✅ CI is green on |
|
The PR is no longer an open ready-to-merge PR targeting main. |
|
The PR is no longer an open ready-to-merge PR targeting main. |
When a reasoning model is routed through
switchyard_model, this change preserves vLLM-style reasoning replay and configured sampling pins: caller parameters cannot override the pins, and historical reasoning is restored or omitted according touses_interleaved_reasoning.With reasoning parsing or non-empty sampling overrides enabled, supported Responses inputs use Gym's shared Chat converter and Responses-only inputs return HTTP 422. With both options disabled, native Responses forwarding stays transparent. The dependency and managed lockfile pin
nemo-switchyard==0.3.0; the dependency comment distinguishes that exact pin from the 0.2.0 minimum needed for Responses usage fields.Rebased onto main
4f2e9678a906d5f9eaeda3537fffc66828fdbfb7, preserving the reviewer fixes and native integration tests from the earlier merge commit. Uses the current public package imports andChatCompletionsConverter. This is a focused follow-up to the Switchyard model integration in #2141; no separate issue is needed. README and configuration documentation cover the opt-in behavior.Validation with Python 3.13.14, uv 0.13.7, native Switchyard 0.3.0, and imports verified against this checkout:
uv lock --check --project responses_api_models/switchyard_modelanduv sync --locked --project responses_api_models/switchyard_modelpassed.gym env test +entrypoint=responses_api_models/switchyard_model +should_validate_data=truepassed: 64 tests.python -m pytest -q -o addopts= responses_api_models/switchyard_model/tests tests/unit_tests/models/test_chat_completions_converter.py tests/unit_tests/test_public_api_facades.pypassed on the final rebased head: 1,075 tests.pre-commit run --all-files, PR-diff whitespace checks, and DCO sign-off verification for all four commits passed.The previously recorded real Qwen3-1.7B/vLLM 0.29.0 transport smoke returned 4 then 12 across two turns with both interleaving settings, preserved or omitted reasoning as configured, and sent all four requests with pinned
max_tokens=192,top_k=5, and temperature 0 despite callermax_output_tokens=384. It was not an agent benchmark. No new model rollout was run for this rebase: the reviewer-fixed behavior is preserved, with native proxy integration tests covering the migration and lockfile update. Current-head CI and GitHub review gates remain separate from the local validation.