fix: detect Spark Ollama CPU fallback#4108
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Ollama runtime CPU-only probing via /api/ps and integrates it into validateOllamaModel for DGX Spark; extends systemd override to optionally set OLLAMA_LLM_LIBRARY=cuda_v13 and enable the service; adds Docker gateway TOML generation and tests for these behaviors. ChangesOllama DGX Spark Runtime Detection and Configuration
Sequence Diagram(s)sequenceDiagram
participant ValidateOllamaModel
participant probeOllamaRuntimeModelStatus
participant OllamaAPI as Ollama /api/ps
ValidateOllamaModel->>probeOllamaRuntimeModelStatus: request model status for selected model
probeOllamaRuntimeModelStatus->>OllamaAPI: GET /api/ps
OllamaAPI-->>probeOllamaRuntimeModelStatus: JSON list of models
probeOllamaRuntimeModelStatus-->>ValidateOllamaModel: return status (probed, loaded, cpuOnly, processor, sizeVram)
ValidateOllamaModel->>ValidateOllamaModel: fail if cpuOnly on Spark
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
PR Review AdvisorFindings: 1 needs attention, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: None Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
Selective E2E Results —
|
| Job | Result |
|---|---|
| gpu-e2e | ⏭️ skipped |
Selective E2E Results —
|
| Job | Result |
|---|---|
| gpu-double-onboard-e2e | ⏭️ skipped |
| gpu-e2e | ⏭️ skipped |
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/lib/onboard.ts`:
- Around line 7096-7099: The call to ensureOllamaLoopbackSystemdOverride is
adding extra lines; collapse it by removing the temporary const overrideState
and use the function call inline where overrideState is used (or assign its
result into an existing nearby variable) so behavior stays identical but the
three-line declaration is reclaimed; reference
ensureOllamaLoopbackSystemdOverride and overrideState to locate and inline the
call.
🪄 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: afadabce-240d-42c5-9635-ada7b1999ac9
📒 Files selected for processing (5)
src/lib/inference/local.test.tssrc/lib/inference/local.tssrc/lib/onboard.tssrc/lib/onboard/ollama-systemd.tstest/onboard-selection.test.ts
|
❌ Brev E2E (gpu): FAILED on branch |
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/lib/onboard/docker-driver-gateway-launch.ts`:
- Around line 253-255: The code sets env.OPENSHELL_GATEWAY_CONFIG = configPath
after writeDockerDriverGatewayConfig but never forwards it into the container;
update the code that builds the Docker run arguments (where the container
args/flags are assembled—the function that composes the `--env` flags for the
launched process) to add `--env OPENSHELL_GATEWAY_CONFIG` (with the same
configPath) so the container receives the variable, and update the launch test
to assert that the produced command includes `--env OPENSHELL_GATEWAY_CONFIG`
(i.e., add an assertion checking the presence of that flag in the test that
validates the Docker launch arguments).
🪄 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: 60237dd5-9324-4c78-8147-cf9d3647d421
📒 Files selected for processing (2)
src/lib/onboard/docker-driver-gateway-launch.test.tssrc/lib/onboard/docker-driver-gateway-launch.ts
Selective E2E Results — ✅ All requested jobs passedRun: 26319732229
|
|
❌ Brev E2E (gpu): FAILED on branch |
Selective E2E Results — ✅ All requested jobs passedRun: 26319964530
|
Selective E2E Results — ✅ All requested jobs passedRun: 26320429784
|
|
✅ Brev E2E (gpu): PASSED on branch |
Summary
/api/psOLLAMA_LLM_LIBRARY=cuda_v13systemd override when that backend is installedTest Plan
npm run build:clinpm run typecheck:clinpx vitest run src/lib/inference/local.test.ts test/onboard-selection.test.ts --testTimeout 20000npx vitest run src/lib/inference/local.test.ts --testTimeout 20000npx vitest run src/lib/inference/local.test.ts test/onboard-selection.test.ts -t "runtime status|CPU-only|GPU memory|adds Spark CUDA v13" --testTimeout 20000git diff --checkNote: local pre-commit/pre-push full CLI coverage hooks failed in unrelated tests on this machine, including the missing
nemoclaw/node_modules/json5fixture path; pushed with--no-verifyafter focused validation passed.Summary by CodeRabbit
New Features
Bug Fixes
Tests