You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(router): pass plugin selectors to the transcription router - #18116
A worker that serves a transcription model cannot register with a frontend that uses round-robin, random or least-loaded routing. Round-robin is the frontend default. The frontend logs RoundRobin routing needs instance selectors; pass PushRouterOptions::selectors and retries. The model never appears in /v1/models, and /v1/audio/transcriptions returns 404.
Since #17713, these routing modes are built-in router plugins, and the router for each worker needs the plugins' instance selectors when it is created. #17713 changed every worker router in the frontend's model discovery to pass them. The transcription router came from #16208, which merged about a day earlier, so #17713 missed it. The transcription router still passes no selectors, and its creation fails for these modes. The defect is in Dynamo.
Reproduction
Without the fix
With the fix
Note
Exact failure on the reported setup
Not run
—
Needs a GPU and a vLLM transcription worker; the coding run had no GPU and no engine.
Exact failure on a similar setup
❌ Exact failure
✅ Passed
Frontend with default round-robin and two minimal Python transcription workers on CPU. Without the fix the model never registered and requests got 404; with it, 4 of 4 requests succeeded across both workers.
Exact error in a test
❌ Exact error
✅ Passed
The new watcher unit test, on CPU: RoundRobin routing needs instance selectors without the fix, passes for all three modes with it.
Key Changes
The frontend now creates the transcription router the same way as the other worker routers, with the selectors for the configured routing mode. Fault detection stays on, and no worker monitor is attached, as before.
Case
Before
After
Transcription worker, round-robin, random or least-loaded routing
Registration fails; requests get 404
Model registers and serves requests
Transcription worker, other routing modes
Registers
Unchanged
Other model types
Register
Unchanged
Core change: about 6 lines (+4/−2). The logic is in 1 file:
about 6 lines in watcher.rs: Creates the transcription router with the routing-mode selectors, as the neighboring routers do.
The rest of the +89/−2 diff is a unit test.
Validation
This PR adds one unit test, text_transcriptions_register_under_selector_router_modes, to lib/llm/src/discovery/watcher.rs. It runs on CPU with a process-local runtime and needs no workers or GPU.
Behavior checked
Checked by
Without the fix
With the fix
A transcription model registers under round-robin, random and least-loaded routing
The transcription PushRouter in the discovery watcher was still built with
`from_client_with_monitor(.., None)`, which leaves `PushRouterOptions::selectors`
unset. Since ai-dynamo#17713 made selectors required for RoundRobin, Random and
LeastLoaded, registering a `ModelType::Transcriptions` worker under those
modes (round-robin is the frontend default) failed with
"RoundRobin routing needs instance selectors; pass PushRouterOptions::selectors".
Build it with `from_client_with_options(.., push_router_options())` like the
other text routers. The old call passed no worker monitor, and
`push_router_options()` also leaves `worker_monitor` unset with fault
detection on, so only the selectors change.
Add a watcher test that registers a Text + Transcriptions card under each
selector mode.
Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
👋 Hi glamr-agent! Thank you for contributing to ai-dynamo/dynamo.
Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.
Checks the changed Rust crates with cargo check and Clippy.
Passed (exit 0)
Workspace cargo check --locked --all-targets and Clippy with -D warnings; the new watcher test failed without the fix (RoundRobin routing needs instance selectors) and passed with it; all 37 watcher tests passed.
Starts the Dynamo frontend with a Mocker worker on CPU and sends the reported request.
Passed (exit 0)
The Mocker cannot advertise a transcription model, so two minimal Python transcription workers stood in for it. Default round-robin: without the fix the model never registered and 4 of 4 requests got 404; with it, 4 of 4 requests returned 200 across both workers.
The reported setup, a vLLM transcription worker on a GPU, was not run; the run had no GPU and no engine.
Detailed evidence — validation complete
Left out of the table because CI repeats them: Builds the changed Dynamo source and confirms that Python can import its compiled extension (passed, exit 0).
Evidence tally: [3/3 validated]
Advisory AI assessment: sound
Validation result: pass — the new test fails without the fix with RoundRobin routing needs instance selectors and passes with it; a live frontend with round-robin and two transcription workers failed to register the model without the fix and served 4 of 4 requests with it; watcher tests, runtime selector test, fmt, workspace check, workspace Clippy and pre-commit are clean.
Evidence audit: complete [3/3 validated] — the command report comes from recorded runs.
Commands and results [3/3 validated]
Generated from the commands recorded during this run.
Check 1
Builds the changed Dynamo source and confirms that Python can import its compiled extension.
Result: Passed (exit 0)
Command:
Not shown because the exact command contained private run data.
Check 2
Checks the changed Rust crates with cargo check and Clippy.
Result: Passed (exit 0)
Command:
Not shown because the exact command contained private run data.
Check 3
Starts the Dynamo frontend with a Mocker worker on CPU and sends the reported request.
Result: Passed (exit 0)
Command:
Not shown because the exact command contained private run data.
No actionable comments were generated in the recent review.
Walkthrough
The transcription PushRouter now uses shared plugin router options. A test checks transcription engine registration for text-input transcription cards under RoundRobin, Random, and LeastLoaded modes.
Changes
Text-input transcription routing
Layer / File(s)
Summary
Router setup and validation lib/llm/src/discovery/watcher.rs
The transcription PushRouter now uses shared push-router options. The test prepares and commits a text-input transcription model under RoundRobin, Random, and LeastLoaded, then checks that each model has a transcription engine.
The change supplies transcription routing selectors; no merge-blocking issue was identified.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title names the change: the transcription router now receives plugin selectors. The change set matches this. The title is short, specific, and uses the conventional prefix. It does not cover the u…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description follows the repository template closely. It has Overview, Key Changes, Validation, and Issues and Ongoing Work sections, plus the checklist and a Closes DYN-5370 line. It explains the …
Full details: Title check
Explanation
The title names the change: the transcription router now receives plugin selectors. The change set matches this. The title is short, specific, and uses the conventional prefix. It does not cover the unit test, which is expected.
Full details: Description check
Explanation
The description follows the repository template closely. It has Overview, Key Changes, Validation, and Issues and Ongoing Work sections, plus the checklist and a Closes DYN-5370 line. It explains the failure mode, the root cause (the transcription router created without plugin selectors after #17713), and the change itself. It gives reproduction evidence, both a CPU frontend reproduction and a unit test. The GPU/vLLM reproduction was not run, and the author says so. The template's 'Closes #XXXX' line is replaced with a Linear ticket reference, which the template's advisory issue-link check may not recognize. Several sections are supplemented with extra tables and a 'Failure behavior' heading that the template does not include. Overall the description is mostly complete and specific.
Full details: Docstring Coverage
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between d61b28b and d6ca5bc.
Files selected for processing: 1
lib/llm/src/discovery/watcher.rs
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
nursery: I changed nothing in watcher.rs for the documentation warning in CodeRabbit’s summary. prepare_worker_set already has a doc comment. The other function is the private test text_transcriptions_register_under_selector_router_modes; its name says what it checks. The repository’s Rust review guide asks us to avoid comments that repeat function names. I also added the requested PR headings and checklist, preserved the first description line, and used the supported Closes DYN-5370 reference.
Fifth upstream merge for the split v2 PR: the four commits past
20c5f11, no conflicts, no router-owned path.
- ai-dynamo#18116 pass plugin selectors to the transcription router:
lib/llm/src/discovery/watcher.rs (auto-merged).
- ai-dynamo#17691 DynamoDB registration store for the Global Discovery Service:
lib/global-discovery-service, root Cargo.toml workspace deps, Cargo.lock.
- ai-dynamo#16288 deployment diagnostics tests, ai-dynamo#16723 helm KAI queue wait: outside
the router.
Signed-off-by: Ishan Dhanani <ishandhanani@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub Actions
Inactive
github-actions
Bot
added
external-contribution
Failure behavior
Overview
A worker that serves a transcription model cannot register with a frontend that uses round-robin, random or least-loaded routing. Round-robin is the frontend default. The frontend logs
RoundRobin routing needs instance selectors; pass PushRouterOptions::selectorsand retries. The model never appears in/v1/models, and/v1/audio/transcriptionsreturns 404.Since #17713, these routing modes are built-in router plugins, and the router for each worker needs the plugins' instance selectors when it is created. #17713 changed every worker router in the frontend's model discovery to pass them. The transcription router came from #16208, which merged about a day earlier, so #17713 missed it. The transcription router still passes no selectors, and its creation fails for these modes. The defect is in Dynamo.
RoundRobin routing needs instance selectorswithout the fix, passes for all three modes with it.Key Changes
The frontend now creates the transcription router the same way as the other worker routers, with the selectors for the configured routing mode. Fault detection stays on, and no worker monitor is attached, as before.
Validation
This PR adds one unit test,
text_transcriptions_register_under_selector_router_modes, tolib/llm/src/discovery/watcher.rs. It runs on CPU with a process-local runtime and needs no workers or GPU.Issues and Ongoing Work
Closes DYN-5370.
Pull Request Checklist
/pr-checklistskill.Summary by CodeRabbit