fix(router): correct NVIDIA routed endpoints - #3614
Conversation
📝 WalkthroughWalkthroughThis PR fixes three configuration errors in the Model Router that prevented inference requests from reaching NVIDIA APIs. The router pool configuration is corrected with the proper upstream endpoint and valid model identifiers, and regression tests are added to validate these fixes and prevent recurrence of issue ChangesModel Router configuration fixes and validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/validate-blueprint.test.ts`:
- Around line 189-202: The test builds modelsByName from (pool.models ?? [])
which can silently drop duplicate model.name entries and hide missing fields;
modify the test so it first validates pool.models is present and then asserts
uniqueness of model.name (e.g., collect names and fail if any duplicate) and
asserts each model has the required litellm_model field before constructing
modelsByName; after those checks proceed with the existing expectations on
modelsByName and litellmModel to ensure duplicate/missing entries produce
deterministic test failures (referencing modelsByName, pool.models, model.name,
and litellm_model).
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b2560d4f-6da8-4e90-83ad-a9e113807bd3
📒 Files selected for processing (2)
nemoclaw-blueprint/router/pool-config.yamltest/validate-blueprint.test.ts
| const modelsByName = new Map( | ||
| (pool.models ?? []).map((model) => [model.name, model.litellm_model]), | ||
| ); | ||
| expect(modelsByName.get("nemotron-3-nano-reasoning")).toBe( | ||
| "openai/nvidia/nemotron-3-nano-30b-a3b", | ||
| ); | ||
| expect(modelsByName.get("nemotron-3-super")).toBe( | ||
| "openai/nvidia/nemotron-3-super-120b-a12b", | ||
| ); | ||
| for (const litellmModel of modelsByName.values()) { | ||
| expect(litellmModel).not.toMatch(/nvidia\/nvidia\//); | ||
| expect(litellmModel).not.toContain("Nemotron-3-Nano-30B-A3B"); | ||
| expect(litellmModel).not.toContain("nemotron-3-super-v3"); | ||
| } |
There was a problem hiding this comment.
Harden regression checks against duplicate/missing model entries.
Building modelsByName via Map can mask duplicate name rows, and optional fields make failures less explicit. Add a uniqueness/assertion step before the value checks so this test catches malformed pool entries deterministically.
Suggested patch
it("regression `#3255`: uses valid LiteLLM NVIDIA model identifiers", () => {
- const modelsByName = new Map(
- (pool.models ?? []).map((model) => [model.name, model.litellm_model]),
- );
+ const models = pool.models ?? [];
+ const names = models.map((model) => model.name);
+ expect(names.every((name) => typeof name === "string" && name.length > 0)).toBe(true);
+ expect(new Set(names).size).toBe(names.length);
+
+ const modelsByName = new Map(models.map((model) => [model.name, model.litellm_model]));
@@
- for (const litellmModel of modelsByName.values()) {
+ for (const litellmModel of modelsByName.values()) {
+ expect(typeof litellmModel).toBe("string");
expect(litellmModel).not.toMatch(/nvidia\/nvidia\//);
expect(litellmModel).not.toContain("Nemotron-3-Nano-30B-A3B");
expect(litellmModel).not.toContain("nemotron-3-super-v3");
}
});🤖 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 `@test/validate-blueprint.test.ts` around lines 189 - 202, The test builds
modelsByName from (pool.models ?? []) which can silently drop duplicate
model.name entries and hide missing fields; modify the test so it first
validates pool.models is present and then asserts uniqueness of model.name
(e.g., collect names and fail if any duplicate) and asserts each model has the
required litellm_model field before constructing modelsByName; after those
checks proceed with the existing expectations on modelsByName and litellmModel
to ensure duplicate/missing entries produce deterministic test failures
(referencing modelsByName, pool.models, model.name, and litellm_model).
Selective E2E Results — ✅ All requested jobs passedRun: 25932461604
|
wscurran
added
NV QA
3 participants
Summary
Fixes #3255 by correcting the Model Router pool config used by the Provider Routed path:
nvapi-*keys tohttps://integrate.api.nvidia.com/v1Validation
npm test -- --run test/validate-blueprint.test.tsnpm test -- --run test/onboard.test.ts -t "Model Router"npm run build:cliE2E guard
Failing-test-first guard: #3594
RED evidence on main-equivalent code: https://github.com/NVIDIA/NemoClaw/actions/runs/25922557128
After this PR is open, dispatch:
Summary by CodeRabbit
Chores
Tests