fix(providers): activate vision sidecar for NVIDIA NIM text-only models - #964
fix(providers): activate vision sidecar for NVIDIA NIM text-only models#964Yuxin-Qiao wants to merge 1 commit into
Conversation
Add noVisionModels to the nvidia registry entry (issue lidge-jun#956). The NIM /v1/models catalog carries no input-modality metadata, so the registry is the only source of truth: without the list, the vision sidecar never runs and text-only NIM models either 400 on raw image parts upstream or are advertised text-only so the app blocks attachments client-side. Covers the text-only chat/code families from the live catalog (2026-08-04) plus the documented kimi family. Vision-capable NIM models (llama-3.2-*- vision-instruct, phi-3-vision, neva-22b, vila, nemotron-*-vl, ...) and non-chat endpoints stay out. Regression tests: registry list contents/exclusions, bare-config router inheritance, planVisionSidecar activation, and catalog inputModalities advertisement.
📝 WalkthroughWalkthroughThe NVIDIA NIM registry now classifies text-only models through ChangesNVIDIA NIM vision routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Request
participant NVIDIA_NIM_Registry
participant Vision_Sidecar_Planner
participant Catalog
Request->>Vision_Sidecar_Planner: Submit parsed request
NVIDIA_NIM_Registry->>Vision_Sidecar_Planner: Provide noVisionModels
Vision_Sidecar_Planner->>Vision_Sidecar_Planner: Plan sidecar when images target text-only models
NVIDIA_NIM_Registry->>Catalog: Provide model capability metadata
Catalog-->>Request: Advertise image input for text-only routed models
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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 `@src/providers/registry.ts`:
- Line 555: Remove minimaxai/minimax-m3, moonshotai/kimi-k2.6, and
thinkingmachines/inkling from NVIDIA_NIM_NO_VISION_MODELS in the provider
registry. Update the corresponding nvidia-nim-hardening tests so all three model
IDs are asserted as excluded from the no-vision classification and therefore
retain native image handling.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4f6170b5-d339-4ff4-abf8-e683f8a62dac
📒 Files selected for processing (2)
src/providers/registry.tstests/nvidia-nim-hardening.test.ts
| "meta/llama-3.3-70b-instruct", | ||
| "meta/llama2-70b", | ||
| "microsoft/phi-3.5-moe-instruct", | ||
| "minimaxai/minimax-m3", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove native vision models from NVIDIA_NIM_NO_VISION_MODELS.
minimaxai/minimax-m3, moonshotai/kimi-k2.6, and thinkingmachines/inkling accept image input. Their current classification activates planVisionSidecar and routes image requests through the sidecar instead of the model’s native vision path. It also contradicts the stated exclusion rule for vision-capable models. NVIDIA documents image input for all three models. (build.nvidia.com)
Remove these IDs. Update tests/nvidia-nim-hardening.test.ts so MiniMax M3, Kimi K2.6, and Inkling are asserted as exclusions.
Proposed fix
- "minimaxai/minimax-m3",
...
- "moonshotai/kimi-k2.6",
...
- "thinkingmachines/inkling",Also applies to: 563-563, 588-588
🤖 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 `@src/providers/registry.ts` at line 555, Remove minimaxai/minimax-m3,
moonshotai/kimi-k2.6, and thinkingmachines/inkling from
NVIDIA_NIM_NO_VISION_MODELS in the provider registry. Update the corresponding
nvidia-nim-hardening tests so all three model IDs are asserted as excluded from
the no-vision classification and therefore retain native image handling.
![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: 220b4f728a
ℹ️ 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".
| "openai/gpt-oss-20b", | ||
| "poolside/laguna-xs-2.1", | ||
| "stepfun-ai/step-3.7-flash", | ||
| "thinkingmachines/inkling", |
There was a problem hiding this comment.
When an NVIDIA user sends an image to thinkingmachines/inkling, this entry makes planVisionSidecar replace the native image with a lossy description—or strip it entirely when no sidecar is available—even though the same exact model ID is already recorded as text,image in BASE10_MODEL_INPUT_MODALITIES at src/providers/registry.ts:638-643. Remove image-capable entries such as Inkling from this list and add a regression that verifies their images remain untouched.
Useful? React with 👍 / 👎.
| // Free pricing, but an API key is still required (free key from build.nvidia.com). | ||
| freeTier: true, | ||
| parallelToolCalls: false, | ||
| noVisionModels: NVIDIA_NIM_NO_VISION_MODELS, |
There was a problem hiding this comment.
This registry flag changes visible NVIDIA behavior: affected models now advertise image input and may consume a configured OpenAI or Anthropic vision sidecar before every image-bearing request, but the NVIDIA provider documentation remains only a base-URL table entry. Add an NVIDIA-specific note identifying the classified model families and the sidecar/auth implications so users are not surprised by the extra upstream call.
AGENTS.md reference: src/AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
| "nvidia/nemotron-4-340b-instruct", | ||
| "nvidia/nemotron-mini-4b-instruct", | ||
| "nvidia/nemotron-nano-3-30b-a3b", | ||
| "nvidia/nvidia-nemotron-nano-9b-v2", |
There was a problem hiding this comment.
For the live text-only model nvidia/nemotron-nano-9b-v2, this entry never matches because it contains an extra nvidia- segment. modelInList performs exact matching for slash-namespaced IDs, so image requests to this model continue bypassing the sidecar and reach the text-only upstream unchanged; correct the ID and add it to the registry coverage test.
Useful? React with 👍 / 👎.
|
Please put your Pull-Request on Ready for Review, once you are finished. |
|
Confirmed as a real defect and left open for its own review rather than folded into the current review stack (#951–#973). The reasoning: this is a provider-registry change — Worth flagging for whoever reviews it: the registry change is small, but the blast radius is every NIM text-only model listed in the PR, so the useful evidence is a live NIM request with an image part before and after. Fixes #956. |
|
Superseded by #980 (stack 7/7), commit Your finding is correct and it is now fixed. The What I changed, and why I owe you the detail. Six ids in the submitted list are natively image-capable per NVIDIA's own documentation:
Listing a vision-capable model there is a silent defect: the model could read the image, but the proxy substitutes another model's text description instead. No error, worse answers, extra cost, and no test fails. This is not a lapse on your part. Issue #956's own body lists minimax-m3 and kimi-k2.6 as text-only, so the reporter and the PR shared the same premise, and review passed it through. It is what an unaudited list does over an open domain. So every id was verified individually against NVIDIA per-model documentation ( The 16 vision-capable ids also needed explicit Where I did worse than you. My first two replacement designs were falsified at the audit gate. I proposed deriving text-only as the complement of a vision list (an unclassified id still ends up in neither list, so nothing changes), then a provider-level default-on rule (which would have advertised image input for every embedding, reranker, guard and OCR endpoint NIM serves, since What still is not fixed: an id NVIDIA ships after this snapshot stays unclassified. NIM publishes no modality metadata, so an unknown id carries no signal at all. #956 closes with that bounded scope stated rather than implied. Thank you for the report — the underlying bug was real and is fixed for every model named in the issue. |
…ice repair Two overnight contributor PRs describe real defects the lidge-jun#951-lidge-jun#973 stack does not touch. This unit plans layer 7 as their reconstruction. lidge-jun#964 cannot be carried: five ids in its hand-written text-only list are natively image-capable per NVIDIA's own docs (inkling, minimax-m3, kimi-k2.6, step-3.7-flash, mistral-medium-3.5-128b). A false positive there is silent — the model can read the image, but the proxy substitutes another model's text description. Issue lidge-jun#956's own body carries two of the same errors, so reporter and author shared the premise. 010 inverts the design: maintain the 15 verified vision-capable ids and derive text-only as the complement, so an unclassified new model defaults to sidecar-on rather than to the bug being fixed. lidge-jun#970's premise is right but its diff is oversized: repairService() and 'ocx service repair' already exist here. 020 records the safety proof that matters — repair throws when not installed and the update path runs after 'ocx stop', but stop never deregisters on any of the three platforms. It also closes a hole lidge-jun#970 leaves: bin/ocx.mjs infers service presence from a possibly-stale marker, where repair would throw and lose the managed service. 030 sequences the bottom-up merge and issue closure, including the lidge-jun#954 security-review gate that can legitimately stop the queue.
The A-gate reviewer returned FAIL. Every blocker was reproduced before being accepted; none was rebutted. 001 records the synthesis. B1 killed my own design. I proposed maintaining the 15 vision-capable ids and deriving text-only as their complement, and claimed an unclassified model would default to sidecar-on. It does not — a complement over a static chat-model list leaves an unknown id in neither list, so modelInList returns false and lidge-jun#956 survives verbatim: deepseek-ai/deepseek-v4-flash sidecarWouldRun=true moonshotai/kimi-k2.6 sidecarWouldRun=false brandnew/model-nobody-classified sidecarWouldRun=false I had inverted which list is maintained while keeping the closed world — the same lesson as the three earlier allowlist failures, reproduced while writing the document that cites them. 010 now changes the predicate instead: default-on for the nvidia entry with the vision list as its exception set, so a stale exception list costs one description hop rather than reproducing the bug. B2: removing a native-vision id from noVisionModels is not sufficient. The catalog advertises image input only for list members, so those models would be blocked client-side instead. They need explicit modelInputModalities. B3: src/update/job.ts:775 skips the service refresh entirely on non-elevated Windows — the dashboard path. Its stated reason is that schtasks /create needs UAC, which repair does not call, so the skip must be narrowed or the reporter's own surface stays broken. B4: repairService throws plain Errors and bin/ocx.mjs sees only an exit status, so 'fall back on not-installed' was unimplementable. Re-run diagnoseService() after a failed repair instead of parsing messages. B5: retargeting emits 'edited', which ci.yml does not listen for, so a green check on the same head sha proves nothing about the new merge base. 030 also moves the lidge-jun#964/lidge-jun#970 closure from 'when stack 7 opens' to 'open and green'. The earlier text borrowed a policy from the six carried PRs, which had verified replacement commits already on a branch; this replacement does not exist yet and its first design just failed audit.
…ler honest design Audit round 2 closed B2/B3/B5 and returned FAIL on two P0s. Two consecutive failures on the same surface means root cause, not a third patch of the same shape. R2-B2 is the one that matters: 010 flagged 'non-chat endpoints never reach the predicate' as a thing to confirm rather than assume, and I did not confirm it. It is false. NVIDIA has no discovery filter and shouldExposeRoutedModel rejects only media-generation names, so embeddings, rerankers, guards and OCR all reach planVisionSidecar: nvidia/nv-embedqa-e5-v5 filteredOut=false nvidia/llama-3.1-nemotron-safety-guard-8b-v3 filteredOut=false nvidia/nemotron-ocr-v2 filteredOut=false Under default-on every one of them would advertise image input and burn a sidecar call before failing upstream. Root cause: twelve of the thirteen registry entries declaring noVisionModels pair it with a static models list. NVIDIA is the first asked to classify over an unbounded set, with no modality and no model-kind metadata. An unknown NIM id therefore carries no signal separating a text-only chat model from an embedding endpoint, and no predicate over an id string can recover information the provider does not publish. Draft 1 kept the closed world; draft 2 escaped it but claimed knowledge that does not exist. The design that follows: enumerate the known text-only ids (correcting lidge-jun#964's five false positives), pin the 15 verified vision ids with explicit modelInputModalities so they become usable, leave unknown ids untouched, and record the open-world gap as a stated limitation. Confined to registry.ts with no predicate change, so no consumer edits — the reason this draft is implementable where draft 2 was not. R2-B1 also caught two consumers earlier drafts missed: web-search/index.ts:165, and cli/models.ts:44 which uses raw .includes() instead of modelInList. R2-B3 (bin/ocx.mjs cannot import diagnoseService from TypeScript) was found and fixed before the verdict arrived; 020 already reads startup.serviceInstalled from the status --json subprocess it spawns.
Audit round 3 FAIL. Three failures on one document is LOOP-DOOM territory, so this changes the verification method the design rests on rather than patching the design again. R3-B1: moonshotai/kimi-k2.5 is a sixth false positive in lidge-jun#964's list — NVIDIA documents GIF/JPG/PNG input, four images per prompt, with hosted image_url examples. This is fatal to draft 3's justification, not just a missing entry. Draft 3 argued 'for a known id the classification is real and verifiable' while inheriting ~54 unaudited entries from lidge-jun#964 and calling them known. Finding a sixth immediately after correcting five proves I never verified the remainder. Every carried id now gets verified against NVIDIA docs or dropped; dropping costs today's behavior, assuming costs a silent regression. R3-B2: my registry census was wrong. Counted directly there are 17 entries declaring noVisionModels, not 13, and the two without a static models list are opencode-go and opencode-free — opencode-zen declares none at all. The numbers came from an ad-hoc regex whose entry boundaries were wrong, and I wrote its output into two documents as fact. Same failure as R2-B2, one document later. The information-constraint argument survives and the two real exceptions strengthen it: both classify only known ids, and opencode-free has a -free suffix filter NVIDIA lacks. R3-B3: test 5 asserted that a user's noVisionModels 'wins' over the registry. mergeStringArray unions them, so a user cannot remove a registry entry. Test now asserts additions are preserved. R3-B4: dropped the dated snapshot test. A local date assertion has no NVIDIA input, so it detects elapsed time rather than drift, and its cheapest CI fix is bumping the date without auditing anything. Also: 030 now requires lidge-jun#956 to close with an explicit bounded-scope statement, and 020 records that the status probe runs only on the success path today.
…ersed, 32 dropped 003 made per-id verification a gating step. This is that audit, run against build.nvidia.com model pages and the NIM LLM/Visual API indexes on 2026-08-04. lidge-jun#964 submitted ~64 ids. Fewer than half survive: 26 confirmed text-only (explicit 'Input Modalities: Text') — these ship 6 confirmed image-capable — moved to the vision list 32 unverified or absent from NVIDIA's catalog — dropped The 26 include z-ai/glm-5.2, deepseek-v4-flash/pro and the nemotron-3 family, so the models issue lidge-jun#956 actually names are all fixed. No seventh false positive was found, which is the first evidence the correction has converged rather than merely advanced. The 32 dropped are mostly delisted models — harmless in isolation, since nobody can route to a model NVIDIA no longer serves. But the set includes nvidia/nemotron-nano-3-30b-a3b, a reversed-name typo of the real nvidia/nemotron-3-nano-30b-a3b which the same list also spells correctly, and mistralai/mixtral-8x22b-v0.1 where NVIDIA documents mixtral-8x22b-instruct-v0.1. Half the list was assembled rather than verified; the six reversed entries were the visible damage, this is the extent of it. Kimi is now split correctly across two independent axes: k2.5 and k2.6 join the vision list, k2-thinking and k2-instruct stay text-only, and all four remain in NVIDIA_NIM_KIMI_MODELS for reasoning suppression. google/codegemma-7b verifies while google/codegemma-1.1-7b does not — adjacent names, opposite outcomes, which is why name-based classification was rejected.
The nvidia registry entry declared no noVisionModels, so planVisionSidecar never fired for any NIM model and the catalog never advertised image input. A text-only NIM model therefore either received raw image parts it cannot read, or had attachments blocked client-side. That is issue lidge-jun#956. Two verified lists, both audited per-model against NVIDIA documentation on 2026-08-04 (evidence: devlog/_plan/260804_stack7_service_vision/011_nim_id_audit.md): noVisionModels 26 ids — text-only, sidecar describes their images modelInputModalities 16 ids — natively image-capable, native path, image input advertised explicitly PR lidge-jun#964 proposed ~64 text-only ids. Six are natively image-capable per NVIDIA's own docs — inkling, minimax-m3, kimi-k2.6, kimi-k2.5, step-3.7-flash and mistral-medium-3.5-128b — and listing those is a silent defect: the model can read the image, but the proxy substitutes another model's text description. No error, worse answers, extra cost. Issue lidge-jun#956's body carries two of the same errors. A further 32 of lidge-jun#964's ids have no current NVIDIA page and are dropped rather than assumed text-only. The vision list also needs explicit modelInputModalities. Removing an id from noVisionModels is not enough: the catalog advertises image input only for list members, so a natively-capable model would be published as text-only and the Codex app would block attachments before the native path could run. Unclassified ids are left alone deliberately. NIM publishes no modality metadata and shouldExposeRoutedModel filters only media-generation names, so embeddings, rerankers, guards and OCR endpoints reach this same path — an unknown id carries no signal separating them from a text-only chat model. Defaulting in either direction would be a claim the data does not support. Vision and reasoning stay independent: k2.5/k2.6 join the vision list while k2-thinking/k2-instruct stay text-only, and all four keep reasoning suppression. Red-green: reintroducing lidge-jun#964's kimi-k2.5 entry fails 2 guards; dropping the modalities map fails 3; removing noVisionModels entirely fails 8. Restored: 23 pass / 0 fail.
3 participants
Summary
Fixes #956. The built-in
nvidiaprovider entry insrc/providers/registry.tsdid not declarenoVisionModels, so the vision sidecar never activated for NVIDIA NIM text-only models (deepseek-ai/deepseek-v4-flash,z-ai/glm-5.2,minimaxai/minimax-m3,moonshotai/kimi-k2.6,nvidia/nemotron-3-ultra-550b-a55b, ...). Raw image parts were forwarded to a text-only upstream (400 / silently ignored) or the catalog advertised the model as text-only, making the Codex app block attachments client-side.NIM's
/v1/modelscarries no input-modality metadata, so the registry is the only source of truth for capability classification.Changes
src/providers/registry.ts: addNVIDIA_NIM_NO_VISION_MODELSand wire it into thenvidiaentry asnoVisionModels.meta/llama-3.2-11b/90b-vision-instruct,microsoft/phi-3-vision-128k-instruct,adept/fuyu-8b,google/deplot,microsoft/kosmos-2,nvidia/llama-3.1-nemotron-nano-vl-8b-v1,nvidia/nemotron-nano-12b-v2-vl,nvidia/neva-22b,nvidia/vila,nvidia/nemotron-3-nano-omni-30b-a3b-reasoning(multimodal), plus non-chat endpoints (embeddings, guards, translators, detectors, image/video generators).modelInListsemantics: exact-id matching for slash-namespaced NIM ids; new text-only ids must be appended individually (same convention as the existing NIM kimi lists).With the list in place,
planVisionSidecarreturns a plan for image-carrying requests to these models and the catalog advertisesinputModalitiesincluding"image"(so the app lets attachments reach the sidecar).Tests
tests/nvidia-nim-hardening.test.ts: 4 new regression testsnoVisionModelsvia the router mergeplanVisionSidecarplans for a text-only NIM model with an image, staysundefinedwithout an image, and staysundefinedfor a vision-capable NIM modelVerification
bun test tests/nvidia-nim-hardening.test.ts→ 18 pass / 0 failbun test tests/provider-registry-parity.test.ts tests/vision-anthropic.test.ts tests/claude-sidecar-override.test.ts tests/codex-catalog.test.ts tests/nvidia-nim-hardening.test.ts→ 176 pass / 0 failbun x tsc --noEmit→ cleandevin the same sandbox — the failures are identical and environmental (sandbox blocks outbound network and socket binds; e.g. OAuth replay,/v1/liverelay, andEADDRINUSEon ephemeral ports), so this change introduces no regressions.Notes
The NIM catalog is dynamic; new text-only ids added upstream must be appended to
NVIDIA_NIM_NO_VISION_MODELS(noted in a registry comment). Vision-capable NIM models keep their previous behavior.Summary by CodeRabbit
New Features
Bug Fixes
Tests