Skip to content

fix(model): default NVIDIA NIM main loop model - #1928

Merged
kevincodex1 merged 4 commits into
Twigpine:mainfrom
jatmn:fix/nvidia-nim-main-loop-default
Jul 10, 2026
Merged

kevincodex1 merged 4 commits into
Twigpine:mainfrom
jatmn:fix/nvidia-nim-main-loop-default

Conversation

@jatmn

@jatmn jatmn commented Jul 10, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes #1925 and addresses the incorrect unknown-model pricing fallback in #1924.

  • Return OPENAI_MODEL for NVIDIA NIM main-loop defaults.
  • Use the NVIDIA NIM descriptor default when OPENAI_MODEL is unset, avoiding an invalid Claude model on the NIM endpoint.
  • Add regression coverage for configured and env-only NIM sessions.
  • Keep unknown-model token accounting and its visible inaccuracy warning, while preventing those models from inheriting an unrelated configured default model's price.

Pricing behavior for unpriced NIM models

This PR does not claim an exact NVIDIA NIM price where the application has no
authoritative per-model rate. An unpriced NIM model continues to use the
application's generic unknown-model estimate: $5 per million input tokens
and $25 per million output tokens. The session output explicitly warns:

costs may be inaccurate due to usage of unknown models

Why it is unknown: NVIDIA's model endpoint returns model IDs, but not
input/output token prices. NVIDIA's public pricing documentation describes
free developer access and GPU-based production licensing, not a per-model
token-price API. LiteLLM's current catalog also has no NIM chat-model price
entries. OpenClaude therefore has no source for a verified NIM token price.

The fix is that this estimate is no longer silently replaced by the price of
whichever model the user configured as their default.

Validation

  • bun test src/utils/model/model.openai-shim-providers.test.ts
  • bun test src/utils/modelCost.modelGate.test.ts
  • bun run typecheck
  • bun run check (build, smoke, and dead-code stages passed; the serial full-test stage has pre-existing unrelated failures, reproduced on unchanged main)

Limitation

The complete serial test suite currently has baseline failures outside this change (discovery cache, shell-governance, and xAI OAuth suites). The focused provider suite and TypeScript check pass.

Summary by CodeRabbit

  • Bug Fixes
    • Improved NVIDIA NIM model selection to honor the configured OpenAI model and use the route’s default when none is specified.
    • Corrected cost estimates for unrecognized models so they use the appropriate unknown-model estimate instead of inheriting pricing from an unrelated default model.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 33 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b9a170cc-51f5-49ac-8b8b-e94b93eda8a1

📥 Commits

Reviewing files that changed from the base of the PR and between 01a67a0 and 43299a8.

📒 Files selected for processing (1)
  • src/utils/model/model.openai-shim-providers.test.ts
📝 Walkthrough

Walkthrough

NVIDIA NIM model resolution now uses the configured model or route default before its hardcoded fallback. Unknown model costs now use the explicit unknown-model estimate instead of inheriting the configured main-loop model’s pricing tier.

Changes

Model behavior corrections

Layer / File(s) Summary
NVIDIA NIM model resolution
src/utils/model/model.ts, src/utils/model/model.openai-shim-providers.test.ts
NVIDIA NIM model selection prefers OPENAI_MODEL, then the route descriptor default, and includes regression tests for both paths.
Unknown-model cost handling
src/utils/modelCost.ts, src/utils/modelCost.modelGate.test.ts
Unknown models always use DEFAULT_UNKNOWN_MODEL_COST, with regression coverage and updated documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • #1924 — The pricing fallback correction directly addresses the issue’s incorrect model-pricing behavior.

Suggested reviewers: kevincodex1, gnanam1990

🚥 Pre-merge checks | ✅ 4 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning modelCost.ts adds unrelated unknown-model pricing behavior that is not required by #1925. Split the pricing fallback changes into a separate PR or link the relevant issue if that work is intended here.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed ⚠️ Warning The only review comment flags a tautological test; it does not discuss provider-routing risk surface or whether it blocks the PR. Add review feedback that explicitly names the provider-routing risk and states whether it is blocking or non-blocking.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Accurately names the NVIDIA NIM default-model fix and matches the main diff.
Description check ✅ Passed The PR description is detailed and mostly matches the template, but Impact and Notes are not filled as separate sections.
Linked Issues check ✅ Passed The NVIDIA NIM main-loop default now uses OPENAI_MODEL or a valid NIM default, matching #1925.
No Hidden Policy Change ✅ Passed PASS: The only behavior change is an explicit NVIDIA NIM default-model branch reusing route metadata; no hidden telemetry, permission, or trust-policy changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@jatmn jatmn self-assigned this Jul 10, 2026
@jatmn jatmn added bug Something isn't working enhancement New feature or request labels Jul 10, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/utils/model/model.openai-shim-providers.test.ts`:
- Around line 323-331: Replace the tautological expectation in the NVIDIA NIM
fallback test with a stable, concrete expected NVIDIA model identifier (or
explicitly assert it differs from the Claude default); do not call
getRouteDefaultModel('nvidia-nim') in the expectation. Keep the setup using
NVIDIA_NIM and CLAUDE_CODE_USE_OPENAI and verify
getDefaultMainLoopModelSetting() returns the intended NIM model id.
🪄 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

Run ID: fe00db77-7b83-430a-bb17-1805990a2054

📥 Commits

Reviewing files that changed from the base of the PR and between 64d164d and 01a67a0.

📒 Files selected for processing (4)
  • src/utils/model/model.openai-shim-providers.test.ts
  • src/utils/model/model.ts
  • src/utils/modelCost.modelGate.test.ts
  • src/utils/modelCost.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: smoke-and-tests (24.11.x)
  • GitHub Check: smoke-and-tests (22)
  • GitHub Check: typecheck
  • GitHub Check: CodeRabbit / Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

Files:

  • src/utils/modelCost.modelGate.test.ts
  • src/utils/model/model.openai-shim-providers.test.ts
  • src/utils/modelCost.ts
  • src/utils/model/model.ts
**

⚙️ CodeRabbit configuration file

**: # AGENTS.md - AI Agent Coding Guide

This guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.

Project Snapshot

OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.

The installed CLI runs on Node.js >=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.

Work Style

  • Keep changes focused on one problem.
  • Prefer existing patterns in the file or nearby module.
  • Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
  • Add or update tests when behavior changes.
  • Update docs when setup, commands, provider behavior, or user-facing behavior changes.
  • For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.

Stack And Conventions

  • TypeScript with strict mode and ESM imports.
  • React + Ink for terminal UI.
  • Bun lockfile and Bun scripts for development workflows.
  • Node runtime for the built CLI.

Common libraries and patterns:

  • chalk for terminal color.
  • commander for CLI argument parsing.
  • execa for child processes.
  • Existing service, provider, settings, permission, and UI patterns over new abstractions.

Repository Map

  • src/commands/ - slash and CLI command implementations.
  • src/components/ - React/Ink UI components.
  • src/services/ - API, MCP, OAuth, wiki, voice, and other service integrations.
  • src/tools/ - tool implementations.
  • src/utils/ - shared utilities.
  • src/integrations/ - provider and model integration metadata.
  • src/entrypoints/ - CLI, MCP, SDK, and generated public types.
  • src/tasks/ - local, remote, workflow, and monitor tas...

Files:

  • src/utils/modelCost.modelGate.test.ts
  • src/utils/model/model.openai-shim-providers.test.ts
  • src/utils/modelCost.ts
  • src/utils/model/model.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • src/utils/modelCost.modelGate.test.ts
  • src/utils/model/model.openai-shim-providers.test.ts
  • src/utils/modelCost.ts
  • src/utils/model/model.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

  • src/utils/modelCost.modelGate.test.ts
  • src/utils/model/model.openai-shim-providers.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/utils/model/model.openai-shim-providers.test.ts
  • src/utils/model/model.ts
🔇 Additional comments (4)
src/utils/model/model.ts (1)

33-33: NVIDIA NIM fallback chain looks correct.

Env → route descriptor default → hardcoded fallback matches the pattern used elsewhere (e.g. minimax) and directly addresses the 404 fallback-to-Claude bug. getRouteDefaultModel is a pure, synchronous lookup with no side effects, so no new network reach or hidden default expansion is introduced — the added fallback is intentional and documented in the comment.

Also applies to: 392-400

src/utils/modelCost.ts (1)

165-168: Fix correctly prevents unknown models from inheriting unrelated pricing.

Unconditionally returning DEFAULT_UNKNOWN_MODEL_COST for unmapped models removes the previous unintended coupling to the configured main-loop model's tier, and the updated comment documents the contract clearly.

Also applies to: 183-184

src/utils/modelCost.modelGate.test.ts (1)

25-46: Good regression test for the pricing fix.

Mocking getDefaultMainLoopModelSetting to return a priced model (claude-haiku-4-5) while asserting the unknown model still gets COST_TIER_5_25 directly proves the bug is fixed and guards against regression.

src/utils/model/model.openai-shim-providers.test.ts (1)

310-321: 🩺 Stability & Availability

No action needed for env cleanup here. The shared beforeEach/afterEach hooks already clear NVIDIA_NIM, CLAUDE_CODE_USE_OPENAI, and OPENAI_MODEL, so these values don’t leak across tests.

			> Likely an incorrect or invalid review comment.

Comment thread src/utils/model/model.openai-shim-providers.test.ts
@jatmn
jatmn marked this pull request as ready for review July 10, 2026 05:53

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevincodex1 kevincodex1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevincodex1
kevincodex1 merged commit 5918e33 into Twigpine:main Jul 10, 2026
5 checks passed
@jatmn
jatmn deleted the fix/nvidia-nim-main-loop-default branch July 11, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants