fix(ai): resolve llama.cpp backend search path so on-device AI actually loads a model (I75) - #91
Conversation
…ly loads a model (I75) 1.8.0/I73 fixed the sidecar failing to spawn at all. It still crash-looped after spawning: the pinned llama.cpp b9095 prebuilds dlopen their CPU/Metal backend as a separate library, and ggml's loader for that only checks the executable's own directory and the process's current working directory — never PATH/DYLD_FALLBACK_LIBRARY_PATH/LD_LIBRARY_PATH, which only cover the binary's linked imports. Spawn the sidecar with its companion-library directory as its working directory so the backend is actually found. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe llama-server child process now uses its runtime directory as its working directory, allowing GGML backend libraries to be located during model loading. The changelog and audit ledger document the fix and its cross-platform verification. Changesllama-server backend loading
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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-tauri/src/commands/sidecar.rs`:
- Around line 556-566: Run the required validation gates for the Rust change:
repository-wide Prettier, src-tauri cargo fmt --check, cargo test, and cargo
clippy. Fix any reported formatting or lint issues, including existing Prettier
failures, and rerun all gates until they pass.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75319681-5a7d-44fd-bb04-293b3e72b75f
📒 Files selected for processing (3)
CHANGELOG.mdISSUES.mdsrc-tauri/src/commands/sidecar.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Rust (macOS)
- GitHub Check: Rust (Windows)
🧰 Additional context used
📓 Path-based instructions (3)
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src-tauri/**/*.rs: Rust changes must preserve compatibility with persisted local data and peer-facing contracts.
Rust changes requirecargo test,cargo fmt --check, andcargo clippybefore review.
Files:
src-tauri/src/commands/sidecar.rs
**/*.{ts,tsx,js,jsx,css,json,md,rs,toml,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Run repository formatting before committing multi-file or subagent work; Prettier formatting must pass for the whole tree.
Files:
src-tauri/src/commands/sidecar.rsCHANGELOG.mdISSUES.md
CHANGELOG.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update
CHANGELOG.mdas part of every release.
Files:
CHANGELOG.md
🔇 Additional comments (3)
src-tauri/src/commands/sidecar.rs (1)
556-566: LGTM!ISSUES.md (1)
89-89: LGTM!CHANGELOG.md (1)
21-38: LGTM!
| // I75: the prebuilt llama.cpp binary is a GGML_BACKEND_DL build — | ||
| // ggml-cpu-*/metal/blas are separate libs it dlopen()s at startup, | ||
| // not linked imports. ggml's loader (ggml_backend_load_best) globs | ||
| // exactly two places for those: the executable's own directory and | ||
| // the process's current working directory — never PATH / | ||
| // DYLD_FALLBACK_LIBRARY_PATH / LD_LIBRARY_PATH, which only satisfy | ||
| // the *linked* imports (llama.dll/ggml-base.dll etc, resolved below). | ||
| // Without this, the child starts and logs its banner, then dies with | ||
| // "no backends are loaded" the moment it tries to load a model. | ||
| command = command.current_dir(dir); | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
npx prettier --check .
(
cd src-tauri
cargo fmt --check
cargo test
cargo clippy
)Repository: scotej/studyvis
Length of output: 439
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Changed Prettier-warned files vs repository tracked:"
npx prettier --check CHANGELOG.md ISSUES.md
echo
echo "Rust toolchain / src-tauri status:"
(
if command -v cargo >/dev/null 2>&1; then
cargo --version || true
source .cargo/config.toml 2>/dev/null || true
(
cd src-tauri
cargo fmt --check || true
cargo test --no-run --message-format=short || true
cargo clippy -- -D warnings --no-deps || true
) || true
else
echo "cargo not available"
fi
)
echo
echo "Relevant git diff stat/name-only:"
git diff --stat || true
git diff --name-only || trueRepository: scotej/studyvis
Length of output: 492
Run the required validation gates before merging.
This PR changes Rust code, so the required gates are: repository-wide Prettier, src-tauri/cargo fmt --check, src-tauri/cargo test, and src-tauri/cargo clippy. Prettier currently reports formatting issues in repository files that need to be addressed.
🤖 Prompt for 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.
In `@src-tauri/src/commands/sidecar.rs` around lines 556 - 566, Run the required
validation gates for the Rust change: repository-wide Prettier, src-tauri cargo
fmt --check, cargo test, and cargo clippy. Fix any reported formatting or lint
issues, including existing Prettier failures, and rerun all gates until they
pass.
Source: Coding guidelines
Summary
A same-day follow-up to 1.8.0/I73. That fixed the sidecar failing to spawn
at all (wrong exe path). This fixes the very next link in the same chain: the
sidecar now spawns fine and then crash-loops, because the model never
actually loads.
b9095prebuilds areGGML_BACKEND_DLbuilds — theCPU (and, on Apple Silicon, Metal) backend ships as a separate library
(
ggml-cpu-*.dll/libggml-cpu.dylib/libggml-metal.dylib) that ggmldlopen()s at startup rather than links directly.ggml_backend_load_best(ggml/src/ggml-backend-reg.cpp:479-489at tagb9095, verified against the actual pinned source) globs exactly twoplaces for that: the executable's own directory, and the process's current
working directory. It never consults
PATH/DYLD_FALLBACK_LIBRARY_PATH/
LD_LIBRARY_PATH.binary's linked imports (
llama.dll,ggml-base.dll, …) — which is whythe process starts and logs its banner — but it never satisfies the dlopen
glob above.
ggml_backend_reg_count()stays 0, model load fails with "nobackends are loaded", and the crash-restart watcher gives up after 4
identical failures. Confirmed against a friend's real Windows
llama-server.logand against the actualllama-b9095-bin-win-cpu-x64.zip/
llama-b9095-bin-macos-arm64.tar.gzrelease contents.Fix
spawn_llamanow also sets the child's working directory to the sameruntime-library directory already used for the env-var prepend
(
Command::current_dir, present ontauri-plugin-shell2.3.5), sincefs::current_path()is in ggml's search path. One code path covers bothengine sources (bundled, and the managed install where the runtime dir
already equals the binary's own directory) and all three platforms.
Checked for side effects: StudyVis's
spawn_llamaargs(
--host/--port/--ctx-size/--n-gpu-layers/--model/--mmproj) neverpass
--pathor--slot-save-path— the two llama-server options with aCWD-relative default — so changing the CWD doesn't affect anything else the
server does.
model_path/mmproj_pathare always absolute (file-picker- ordata-dir-derived, gated by
is_file()before spawn).Test plan
b9095source(
ggml-backend-reg.cpp) and the real release archives for bothWindows and macOS.
on CWD.
src-tauri/binaries/has no fetched engine on this Linux dev host.Covered by CI (
cargo test/clippy/fmt) on the PR, plus a liveWindows/macOS walk before the release is published.
Fixes I75 (
ISSUES.md). CHANGELOG.md carries the 1.8.1 section this PR needsin place before
Release prepcan gate on it.🤖 Generated with Claude Code
Summary by CodeRabbit