fix(onboard): force chat completions API for Ollama#1315
Conversation
|
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:
📝 WalkthroughWalkthroughLocal Ollama onboarding now stops propagating the validated API choice and instead always sets Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ollama 0.19.0 added a /v1/responses endpoint that passes the onboard probe but produces malformed tool calls in the TUI. Force openai-completions for both Ollama code paths, matching the existing vLLM and NIM behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
4974cc8 to
2775cdd
Compare
prekshivyas
left a comment
There was a problem hiding this comment.
Nice catch and clean fix — thanks @yimoj!
- Root cause correctly identified: Ollama 0.19.0's /v1/responses endpoint passes the probe but produces malformed tool calls in practice ✓
- Fix matches the existing vLLM/NIM override pattern ✓
- Applied to both Ollama onboard paths (existing install + brew install) ✓
- Test updated to assert the correct openai-completions value ✓
- Informational log only when actually overriding — good UX ✓
Minor: the 7-line override block is duplicated across both paths, but that matches the existing code style and isn't worth blocking on.
LGTM.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
bin/lib/onboard.js (1)
2787-2794: Correct fix, though there's code duplication with the other Ollama path.This block is identical to lines 2735–2742. The entire Ollama model selection loop is duplicated between the "existing Ollama" and "brew install" paths, so this follows the existing pattern. Consider extracting the shared Ollama onboarding logic into a helper function in a future cleanup to reduce duplication.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/onboard.js` around lines 2787 - 2794, The Ollama onboarding logic that forces chat completions (checks validation.api !== "openai-completions", logs the message, and sets preferredInferenceApi = "openai-completions") is duplicated between the "existing Ollama" and "brew install" code paths; refactor by extracting that shared block into a helper function (e.g., ensureOllamaUsesChatCompletions or setPreferredOllamaInference) and call it from both places, moving the validation.api check, console.log message, and preferredInferenceApi assignment into the new helper to eliminate duplication while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@bin/lib/onboard.js`:
- Around line 2787-2794: The Ollama onboarding logic that forces chat
completions (checks validation.api !== "openai-completions", logs the message,
and sets preferredInferenceApi = "openai-completions") is duplicated between the
"existing Ollama" and "brew install" code paths; refactor by extracting that
shared block into a helper function (e.g., ensureOllamaUsesChatCompletions or
setPreferredOllamaInference) and call it from both places, moving the
validation.api check, console.log message, and preferredInferenceApi assignment
into the new helper to eliminate duplication while preserving behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 854a3254-511a-4689-b136-1b1392e3b87c
📒 Files selected for processing (1)
bin/lib/onboard.js
…ions API Two upstream bug fixes backported from NVIDIA/NemoClaw: - preflight.js: `port || 18789` → `port ?? 18789` so port=0 (OS-assigned) is not incorrectly replaced by the default (upstream NVIDIA#1304) - onboard.js: Force openai-completions for Ollama regardless of probe result. Ollama 0.19.0 /v1/responses endpoint passes probe but produces malformed tool calls in the TUI. Matches existing vLLM/NIM override pattern. Update test assertion that was asserting the buggy value (upstream NVIDIA#1315)
## Summary - Ollama 0.19.0 added a `/v1/responses` endpoint that passes the onboard probe but produces malformed tool calls in the TUI, causing schema validation failures and nonsensical fallback text (NVIDIA#1211) - Force `preferredInferenceApi` to `openai-completions` for both Ollama onboard paths (existing install and brew install), matching the existing vLLM and NIM override pattern - Fix test assertion that was asserting the buggy `openai-responses` value since the validation feature was first added in NVIDIA#648 ## Test plan - [x] `npx vitest run test/onboard-selection.test.js` — all 26 tests pass - [x] `npx vitest run test/onboard.test.js` — all 43 tests pass - [x] Live repro with Ollama 0.19.0 + qwen2.5:0.5b confirms `setupNim()` now returns `openai-completions` - [ ] Manual TUI test: `nemoclaw onboard` with local Ollama, then send a tool-using prompt in the TUI Closes NVIDIA#1211 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Local Ollama onboarding now consistently selects the OpenAI-style completions inference route; an informational message may be logged when a non-chat API is detected. * **Tests** * Onboarding tests updated to expect the enforced completions-route selection for local Ollama. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Yimo <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Prekshi Vyas <[email protected]>
Summary
/v1/responsesendpoint that passes the onboard probe but produces malformed tool calls in the TUI, causing schema validation failures and nonsensical fallback text (openclaw tui emits malformed tool calls on local Ollama (macOS) even when health checks and one-shot agent runs succeed #1211)preferredInferenceApitoopenai-completionsfor both Ollama onboard paths (existing install and brew install), matching the existing vLLM and NIM override patternopenai-responsesvalue since the validation feature was first added in feat: expand provider onboarding and validation #648Test plan
npx vitest run test/onboard-selection.test.js— all 26 tests passnpx vitest run test/onboard.test.js— all 43 tests passsetupNim()now returnsopenai-completionsnemoclaw onboardwith local Ollama, then send a tool-using prompt in the TUICloses #1211
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Signed-off-by: Yimo [email protected]