Add NVIDIA NIM API provider support#1012
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds NVIDIA NIM models, configuration, provider registration, API-key setup, legacy model resolution, OpenAI-compatible session routing, and unit coverage for classification, provider resolution, endpoint selection, and authorization. ChangesNVIDIA NIM provider support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Session
participant Background
participant ProviderRegistry
participant NVIDIA_NIM_API
Session->>Background: Select NVIDIA NIM model
Background->>ProviderRegistry: Resolve nvidia-nim provider
ProviderRegistry-->>Background: Return endpoint and API key
Background->>NVIDIA_NIM_API: Send chat completions request
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
PR Summary by QodoAdd NVIDIA NIM as built-in OpenAI-compatible provider with model presets
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of PR #1012 (Add NVIDIA NIM API provider support) over NotesThe incremental change registers NVIDIA NIM as a built-in OpenAI-compatible provider and is fully additive:
All prior active review comments on this PR target files outside the current incremental diff ( Files Reviewed (incremental, 8 files)
Previous Review Summaries (12 snapshots, latest commit 54f8a3a)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 54f8a3a)Status: No Issues Found | Recommendation: Merge Incremental review of PR #1012 (Add NVIDIA NIM API provider support) over NotesThe incremental change is a small, focused fix to
All prior active review comments on the changed files are either confirmed fixed in earlier commits or are optional maintainability suggestions explicitly excluded by the author. No new blocking or warning-level issue remains. Files Reviewed (incremental, 2 files)
Previous review (commit 366c25f)Status: No Issues Found | Recommendation: Merge Incremental review of PR #1012 (Add NVIDIA NIM API provider support) over NotesThe incremental changes are focused provider-registration and test-coverage work, consistent with the previously reviewed fixes:
All prior active review comments on the changed files are either confirmed fixed in earlier commits (provider-ID canonicalization, legacy-ID retention, disabled-provider reference handling, renamed-ID URL disambiguation) or are optional maintainability suggestions explicitly excluded by the author. No new blocking or warning-level issue remains. Files Reviewed (incremental, 17 files)
Previous review (commit eab051b)Status: No Issues Found | Recommendation: Merge Incremental review of PR #1012 (Add NVIDIA NIM API provider support) over NotesThe incremental changes are focused, consistent bug fixes with accompanying test coverage:
Existing platform-bot and maintainer comments were reviewed and left in place; none correspond to a new defect in the incremental diff. Files Reviewed (incremental, 8 files)
Previous review (commit 1248673)Status: No Issues Found | Recommendation: Merge Files Reviewed (16 files)
NotesFull review of PR #1012 (Add NVIDIA NIM API provider support) over Verified previously-raised items against current HEAD
Outstanding items previously raised by platform bots remain (not re-raised here to avoid duplicates): qodo #716 (disabled Previous review (commit 421fc0d)Status: No Issues Found | Recommendation: Merge Files Reviewed (incremental over 9d24d5e..421fc0d)Incremental scope: the only changes since the prior review commit are two fixes the author already verified as fixed in 421fc0d:
NotesIncremental review of HEAD Previous review (commit 9d24d5e)Status: No Issues Found | Recommendation: Merge Files Reviewed (16 files, incremental over 3fb39ff)
NotesIncremental review of HEAD Previously-flagged platform findings are confirmed addressed in the current code:
No new high-confidence defects (security, runtime, logic, typos) were found in the changed lines. The secret-migration flow was traced against the new test suite and the builtin-key sync at the top of Previous review (commit 3fb39ff)Status: No Issues Found | Recommendation: Merge Files Reviewed (40 files, incremental since d02d04a)
NotesIncremental review of HEAD The expanded PR now also introduces provider-resolution hardening and a new transport-error reporting layer beyond the original NVIDIA NIM provider work:
No new high-confidence defects found in the changed lines. The 5 active inline comments from other review bots on Previous review (commit d02d04a)Status: No Issues Found | Recommendation: Merge Files Reviewed (11 files)
NotesIncremental review of commit The expanded migration logic in
No new high-confidence defects found in the changed lines. The four active inline comments from other review bots (chatgpt-codex-connector, coderabbit, qodo) are not duplicated here. The prior recommendation to activate an NVIDIA NIM preset by default is now satisfied by Previous review (commit d48fe61)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
NotesThe PR was expanded beyond the prior incremental review into a full provider integration. The prior finding about NVIDIA NIM not being selectable by default (line 135 of Previous review (commit 27c6770)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
NotesThe PR was expanded beyond the prior incremental review into a full provider integration. The prior finding about NVIDIA NIM not being selectable by default (line 135 of Previous review (commit 589b988)Status: No Issues Found | Recommendation: Merge The incremental changes add four new NVIDIA NIM model presets ( Files Reviewed (2 files)
Previous review (commit 7cb9c9e)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Reviewed by hy3:free · Input: 73.4K · Output: 6K · Cached: 606K |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cb9c9e027
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Review by Qodo
1.
|
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Pull request overview
Adds NVIDIA NIM as a first-class OpenAI-compatible API provider in the extension’s provider registry/config, including model presets and unit coverage to ensure provider resolution and routing work end-to-end with the existing OpenAI-compatible infrastructure.
Changes:
- Register
nvidia-nimas a built-in OpenAI-compatible provider (base URL + chat completions path) and add legacy preset/provider resolution. - Add NVIDIA NIM model group + presets (GLM-5.2, Nemotron-3-Ultra, DeepSeek V4 Pro) and wire the API-key field into config/mappings.
- Extend unit tests to cover provider-id resolution, request resolution, and thin adapter routing for NVIDIA NIM.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/services/apis/thin-adapters.test.mjs | Adds thin-adapter coverage ensuring NVIDIA NIM requests use the expected base URL and provider secret. |
| tests/unit/services/apis/provider-registry.test.mjs | Adds tests for legacy preset resolution and OpenAI-compatible request resolution for NVIDIA NIM. |
| tests/unit/config/config-predicates.test.mjs | Adds predicate coverage for detecting NVIDIA NIM API model usage. |
| src/services/apis/provider-registry.mjs | Registers NVIDIA NIM as a built-in provider and supports resolving it from legacy model names. |
| src/popup/sections/GeneralPart.jsx | Adds the NVIDIA API key setup link for the nvidia-nim provider. |
| src/config/openai-provider-mappings.mjs | Maps NVIDIA NIM API mode group to provider id and wires its legacy API-key field mapping. |
| src/config/index.mjs | Adds NVIDIA NIM model group/presets and the nvidiaNimApiKey config field + predicate. |
| src/background/index.mjs | Treats NVIDIA NIM sessions as OpenAI-compatible API sessions for routing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7cb9c9e to
589b988
Compare
|
Code review by qodo was updated up to the latest commit 589b988 |
589b988 to
27c6770
Compare
|
Code review by qodo was updated up to the latest commit 27c6770 |
27c6770 to
d48fe61
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/config/index.mjs`:
- Around line 1222-1227: Update the session remap flow around
sessionProviderIdRenameLookup so it is derived from the final provider IDs after
all promoteCustomModeApiKeyToProvider operations complete, rather than from the
intermediate providerIdRenameLookup state. Ensure every subsequent session
persistence path, including the related logic at the other referenced locations,
uses the refreshed mapping when promotion reassigns a mode.
- Around line 1857-1863: Update the session migration flow around
migrateUserConfig and remapSessionProviderIds to run the sessions read,
provider-ID remapping, and storage.local.set write through the existing session
persistence lock/queue. Keep the entire read-modify-write sequence serialized,
including the related migration path near the second session update, so
concurrent session saves are not overwritten.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 136a53c2-9bef-4714-b2dd-f4e3bf0d8a7b
📒 Files selected for processing (11)
src/background/index.mjssrc/config/index.mjssrc/config/openai-provider-mappings.mjssrc/popup/sections/GeneralPart.jsxsrc/popup/sections/import-data-cleanup.mjssrc/services/apis/provider-registry.mjstests/unit/config/config-predicates.test.mjstests/unit/config/migrate-nvidia-nim-provider.test.mjstests/unit/popup/import-data-cleanup.test.mjstests/unit/services/apis/provider-registry.test.mjstests/unit/services/apis/thin-adapters.test.mjs
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/unit/config/config-predicates.test.mjs
- tests/unit/services/apis/thin-adapters.test.mjs
- tests/unit/services/apis/provider-registry.test.mjs
- src/config/openai-provider-mappings.mjs
- src/popup/sections/GeneralPart.jsx
- src/services/apis/provider-registry.mjs
- src/background/index.mjs
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit d48fe61 |
d48fe61 to
d02d04a
Compare
9d24d5e to
421fc0d
Compare
|
Reviewed the outside-diff suggestion about disabled-provider URL recovery. No code change is needed: |
|
Code review by qodo was updated up to the latest commit 421fc0d |
421fc0d to
1248673
Compare
|
Code review by qodo was updated up to the latest commit 1248673 |
1248673 to
eab051b
Compare
|
Code review by qodo was updated up to the latest commit eab051b |
eab051b to
366c25f
Compare
|
Code review by qodo was updated up to the latest commit 366c25f |
366c25f to
54f8a3a
Compare
|
Code review by qodo was updated up to the latest commit 54f8a3a |
Register NVIDIA NIM as a built-in OpenAI-compatible chat provider and expose selected featured model presets. Wire shared API-key storage, request routing, setup links, and focused coverage for the NVIDIA hosted endpoint.
54f8a3a to
9aa53a6
Compare
✅ Action performedReview finished.
|
|
Code review by qodo was updated up to the latest commit 9aa53a6 |
Summary
Validation
npm test(795 tests passed)npm run lintnpm run buildManual browser smoke testing was not run because a browser extension runtime is not available in this environment.