Skip to content

Adding Nvidia NIM Api key as one of the provider#960

Open
skyhighbg22-jpg wants to merge 3 commits intoGitlawb:mainfrom
skyhighbg22-jpg:main
Open

Adding Nvidia NIM Api key as one of the provider#960
skyhighbg22-jpg wants to merge 3 commits intoGitlawb:mainfrom
skyhighbg22-jpg:main

Conversation

@skyhighbg22-jpg
Copy link
Copy Markdown

@skyhighbg22-jpg skyhighbg22-jpg commented Apr 30, 2026

Adds Nividia NIM API Key as one of the provider and makes it very beginner friendly

- Add NIMClient class with simple API key-only authentication
- Support both string prompts and chat format conversations
- Include example usage script with basic and advanced examples
- Update README with comprehensive documentation
@skyhighbg22-jpg skyhighbg22-jpg changed the title Merge branch 'main' of https://github.com/Gitlawb/openclaude Adding Nvidia NIM Api key as one of the provider Apr 30, 2026
Comment thread README.md
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would suggest putting all of your files in a dedicated subfolder

Copy link
Copy Markdown
Collaborator

@gnanam1990 gnanam1990 left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to put this together, but this PR has some structural issues that I think mean it can't be merged as-is. Wanted to be specific about what's blocking and how a usable NVIDIA NIM provider would actually look in this codebase, in case you (or someone else) wants to take another pass at it.

Blockers

  1. README.md is replaced, not extended. The diff shows -356 / +28 on README.md — the entire OpenClaude README (badges, sponsors, quick-start, provider list, source-build guide, VS Code extension docs, etc.) is wiped and replaced with a short Python tutorial. That alone makes this unmergeable.
  2. Python files in a TypeScript/Bun repo. OpenClaude is TypeScript shipped via Bun (bun run build, bun test). Adding src/nim_client.py and example_usage.py introduces a Python dependency the project doesn't have and won't load at runtime. None of these files are reachable from the existing CLI.
  3. No integration into the provider system. A real provider PR needs to plug into:
    • src/utils/model/providers.ts (provider registration and routing)
    • src/utils/providerProfile.ts / providerProfiles.ts (profile system, env var wiring)
    • src/services/api/openaiShim.ts (since NIM exposes an OpenAI-compatible endpoint, this is almost certainly where it should hook in)
    • Tests under src/utils/*.test.ts and src/services/api/*.test.ts
  4. Branch is main. PRing from your fork's main makes follow-up commits messy. A feature branch like feat/nvidia-nim-provider is much easier to iterate on.
  5. No validation evidence. No bun test output, no bun run build confirmation, no manual smoke against NIM.

Suggested approach if you want to retry

NVIDIA NIM exposes an OpenAI-compatible API at https://integrate.api.nvidia.com/v1. That means you almost certainly do not need a new client class — you can wire it up by:

  1. Adding NVIDIA_API_KEY env var handling (similar to XAI_API_KEY or MINIMAX_API_KEY).
  2. Adding NVIDIA as a recognized base URL in the provider registry / profile system.
  3. Adding it to the /provider setup wizard.
  4. A regression test that exercises the OpenAI-compatible shim against a mocked NIM endpoint.

Take a look at how MiniMax (#492) or Mistral were added — they're a much closer template than building a separate client.

Closing this isn't strictly necessary if you'd rather rework it in place, but I'd recommend opening a new PR from a feature branch with the integration approach above. Happy to help review whichever route you pick.

@gnanam1990
Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @skyhighbg22-jpg, and apologies for the back-and-forth on this one. I owe you a correction on my earlier review and a clearer picture of what would unblock it.

Correction

In my previous review I called out "Python files in a TypeScript/Bun repo" as a blocker. That was wrong — we already have a tracked python/ directory with ollama_provider.py, atomic_chat_provider.py, smart_router.py, a requirements.txt, and a python/tests/ suite that PR #477 wired into CI. Python helpers are a real, supported track in this project. Sorry for the noise on that point.

What's still blocking

  1. README is replaced rather than extended. The diff takes README.md from 384 lines to 41, removing badges, sponsors, the quick-start, the full provider list, the source-build guide, the VS Code extension docs, and the community section. This needs to be reverted; NIM docs would be very welcome as a new section appended to the existing README.

  2. example_usage.py doesn't run as written. from nim_client import NIMClient raises ModuleNotFoundError because the module lives at src/nim_client.py and isn't on sys.path. I confirmed this locally.

  3. File placement doesn't match the existing Python pattern. If we're going the Python route, the file should live at python/nim_provider.py (next to ollama_provider.py) with tests under python/tests/test_nim_provider.py. That way CI's existing pytest job covers it. @adomaskn flagged the subfolder point too — python/ is the specific folder to use.

  4. Conventions of the existing helpers aren't followed. python/ollama_provider.py is a good template:

    • httpx.AsyncClient rather than sync requests
    • Configuration via env vars (e.g. NVIDIA_API_KEY, optional NVIDIA_BASE_URL, NVIDIA_MODEL) rather than a hardcoded constructor arg
    • A module docstring showing the .env keys
    • A pytest with a mocked HTTP response, mirroring python/tests/test_ollama_provider.py
  5. Users still can't select NIM from the CLI after this lands. There are no references to nim/NVIDIA anywhere in src/utils/model/, src/utils/providerProfile*.ts, or src/services/api/openaiShim.ts. Since NIM is OpenAI-compatible, the user-facing path is almost certainly an openaiShim profile + NVIDIA_API_KEY env var rather than a separate client. MiniMax (feat: add MiniMax provider support #492) is the closest template for that wiring. This can be a follow-up PR if you'd rather keep this one focused on the Python helper.

  6. Branch hygiene. This PR is from your fork's main and is currently 24 commits behind upstream main (status: CONFLICTING). A rebased feature branch (e.g. feat/nvidia-nim-provider) would make iteration much smoother.

Smaller suggestion

The hardcoded model="nim/llama3-8b-instruct" default will go stale quickly — NIM hosts dozens of models. Consider making it env-configurable with no opinionated default, similar to how BIG_MODEL / SMALL_MODEL work for the Ollama helper.

Happy to re-review as soon as the README is restored and the file moves into python/ — that alone gets this much closer. Thanks again for the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants