Skip to content

fix(onboard): honor provider model env for Ollama#6873

Open
HwangJohn wants to merge 3 commits into
NVIDIA:mainfrom
HwangJohn:codex/6869-ollama-model-env
Open

fix(onboard): honor provider model env for Ollama#6873
HwangJohn wants to merge 3 commits into
NVIDIA:mainfrom
HwangJohn:codex/6869-ollama-model-env

Conversation

@HwangJohn

@HwangJohn HwangJohn commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Honor NEMOCLAW_PROVIDER_MODEL as a compatibility fallback for Ollama onboarding while keeping NEMOCLAW_MODEL as the preferred model env. Interactive Ollama selection now defaults to the requested model when that model appears in the rendered menu, instead of falling back to the memory-based or alphabetically-first default.

Related Issue

Fixes #6869

Changes

  • Add a shared requested-model env reader that prefers NEMOCLAW_MODEL and falls back to NEMOCLAW_PROVIDER_MODEL for non-interactive provider selection.
  • Pass the requested model env into interactive Ollama model prompting only when no route-compatibility lock already controls the model.
  • Match the requested default against rendered Ollama menu entries with existing Ollama model-reference normalization, and keep the memory-based default when the requested model is not shown.
  • Document the NEMOCLAW_PROVIDER_MODEL compatibility fallback for Ollama and add regression coverage for env precedence, interactive defaults, and hidden requested models.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Contributor self-review focused on onboarding/inference env precedence and validation; the change does not read credentials, alter policy, or expose new secret material. Maintainer review is still required before merge.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable — npm run check:diff passed on DGX Spark Linux/aarch64 against the committed diff
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli src/lib/inference/ollama/proxy.test.ts src/lib/onboard/setup-nim-ollama.test.ts src/lib/onboard/providers.test.ts passed locally on Windows and on DGX Spark Linux/aarch64 (71 passed)
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — not applicable; this is a focused onboarding/inference env handling change covered by targeted tests, docs build, CLI build, and diff gate
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only) — npm run docs passed on DGX Spark Linux/aarch64 with 0 errors; Fern reported existing hidden warnings/upgrade notice, so this box is left unchecked
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: HwangJohn angelic805@gmail.com

Summary by CodeRabbit

  • New Features
    • Added compatibility support for configuring a provider model via NEMOCLAW_PROVIDER_MODEL.
    • Enhanced interactive Ollama onboarding: if a requested “safe” model tag exists in the current Ollama model menu, it’s now used as the default selection.
  • Bug Fixes
    • Improved Ollama model selection to match models by reference reliably and to calculate the default selection index correctly.
    • When the requested model isn’t available in the installed-model menu, the app now consistently falls back to the memory-based default.
  • Documentation
    • Updated Ollama onboarding docs to cover NEMOCLAW_PROVIDER_MODEL and the interactive/non-interactive default-model fallback behavior.

Signed-off-by: HwangJohn <angelic805@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 38775cfa-f403-4766-9885-c2661d44910b

📥 Commits

Reviewing files that changed from the base of the PR and between 9e7bca9 and f3965ee.

📒 Files selected for processing (1)
  • src/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/onboard.ts

📝 Walkthrough

Walkthrough

Ollama onboarding now resolves model requests from NEMOCLAW_MODEL or NEMOCLAW_PROVIDER_MODEL, propagates validated interactive defaults, and matches requested model references against installed Ollama options. Tests and documentation cover precedence, fallback, and prompt selection behavior.

Changes

Ollama model selection

Layer / File(s) Summary
Environment model resolution
src/lib/onboard/providers.ts, src/lib/onboard/providers.test.ts
Adds primary and compatibility model environment resolution, applies it to hosted staging and non-interactive validation, and tests precedence and fallback behavior.
Interactive default propagation
src/lib/onboard/setup-nim-ollama.ts, src/lib/onboard/setup-nim-ollama.test.ts
Derives prompt defaults from environment variables for interactive Ollama setup and passes them through model selection, including null behavior when unavailable.
Prompt model matching and documentation
src/lib/inference/ollama/proxy.ts, src/lib/inference/ollama/proxy.test.ts, src/lib/onboard.ts, docs/inference/set-up-ollama.mdx
Validates prompt defaults, matches installed models by reference, recalculates the default index, and documents compatibility fallback and interactive selection rules.

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

Possibly related PRs

Suggested labels: provider: ollama, area: onboarding, area: inference, bug-fix

Suggested reviewers: cv, ericksoa

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: honoring the provider model env for Ollama onboarding.
Linked Issues check ✅ Passed The implementation and tests address #6869 by honoring the requested provider model in interactive and non-interactive Ollama onboarding, with fallback preserved.
Out of Scope Changes check ✅ Passed The edits are confined to Ollama onboarding docs, logic, and regression tests, with no obvious unrelated changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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/lib/onboard/setup-nim-ollama.test.ts`:
- Around line 112-115: Update the injected process.env in the fallback test to
explicitly set NEMOCLAW_MODEL to undefined, ensuring the fallback uses
NEMOCLAW_PROVIDER_MODEL regardless of the host environment. Preserve the
existing environment spread and provider-model setup.
🪄 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: 41f9d7eb-e396-4b07-892b-ad24a6d3ebb4

📥 Commits

Reviewing files that changed from the base of the PR and between 20130dd and eae13ba.

📒 Files selected for processing (8)
  • docs/inference/set-up-ollama.mdx
  • src/lib/inference/ollama/proxy.test.ts
  • src/lib/inference/ollama/proxy.ts
  • src/lib/onboard.ts
  • src/lib/onboard/providers.test.ts
  • src/lib/onboard/providers.ts
  • src/lib/onboard/setup-nim-ollama.test.ts
  • src/lib/onboard/setup-nim-ollama.ts

Comment thread src/lib/onboard/setup-nim-ollama.test.ts
Signed-off-by: HwangJohn <angelic805@gmail.com>
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions
Status: Canonical ledger: 0 blocker(s), 1 warning(s), 0 suggestion(s).

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 1 warning · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 fewer warning, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy, onboard-repair, onboard-resume, cloud-onboard

1 optional E2E recommendation
  • ollama-auth-proxy
1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Document the current consumer and lifecycle of the provider-model fallback

  • Location: src/lib/onboard/providers.ts:313
  • Category: architecture
  • Problem: The new `NEMOCLAW_PROVIDER_MODEL` compatibility resolver broadens supported configuration across non-interactive selection, hosted staging, and the interactive Ollama default, but neither its comments nor the user documentation names the current automation consumer that cannot provide `NEMOCLAW_MODEL`, explains why that source cannot be corrected here, or establishes a removal condition.
  • Impact: An unowned compatibility input becomes a permanent public configuration contract and can preserve behavior for unknown callers, increasing future selection-path complexity and making safe removal or precedence changes harder.
  • Recommendation: Name the currently supported consumer and its input contract, explain why it must use `NEMOCLAW_PROVIDER_MODEL`, and record a bounded removal/migration condition; otherwise remove or narrowly scope the fallback to the demonstrated consumer.
  • Verification: Read `src/lib/onboard/providers.ts:313-328` and `docs/inference/set-up-ollama.mdx:175-183`; confirm a named current consumer and retirement condition are recorded for this fallback.
  • Test coverage: Add a contract test for the named current consumer showing it supplies only `NEMOCLAW_PROVIDER_MODEL` and resolves the intended model while `NEMOCLAW_MODEL` retains documented precedence.
  • Evidence: `getRequestedModelEnv` introduces `NEMOCLAW_PROVIDER_MODEL` as a general fallback at `src/lib/onboard/providers.ts:313-320`. The resolver is consumed by hosted staging, non-interactive selection, and interactive Ollama prompt-default handling. `docs/inference/set-up-ollama.mdx:175` calls it a compatibility fallback for unspecified automation but identifies neither a consumer nor retirement condition. The linked issue is contributor-authored and has no maintainer decision; it cannot establish the compatibility contract.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: HwangJohn <angelic805@gmail.com>
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.

[All Platforms][Onboard] NEMOCLAW_PROVIDER_MODEL ignored — Ollama onboard auto-selects the alphabetically-first model

1 participant