fix(local-inference): image-gen honors the probed GPU vendor instead of always CPU (#10727) - #11654
Conversation
…of always CPU (#10727) The image-gen loader hardcoded `gpu: undefined` in the ImageGenRuntimeProfile, so selectImageGenBackends fell through to sd-cpp CPU on every Linux/Windows box — even NVIDIA, whose backend probeHardware reliably detects via nvidia-smi. A silent CPU fallback with an accelerated path available (the #10727 rule). - Add pure imageGenGpuVendorFromProbeBackend (cuda->nvidia, metal->apple, vulkan->amd, null->undefined) and thread the real probe vendor into the profile; NVIDIA Linux/Windows now reaches the CUDA/TensorRT path. Probe failure degrades gracefully to the platform default with a logged reason (no silent swallow). - SEV-4: the embedding handler's swallowed `probeHardware().catch(() => undefined)` now logs why, so a broken probe on an accelerated box is visible. - SEV-3: correct two false hardware.ts comments (claimed the probe uses capacitor-llama and reports AMD as vulkan — it does neither; it uses nvidia-smi + Apple-Silicon detection and leaves AMD/Intel null at probe). - Tests exercise the REAL mapper the caller uses (not synthetic gpu profiles), asserting NVIDIA reaches CUDA and macOS/AMD paths are unchanged. Known, deliberately deferred: embeddings/image-gen on AMD/Intel still run CPU because probeHardware leaves them null at probe time (no cheap pre-load VRAM query; guessing vulkan risks a hard GPU-open throw with no CPU retry). Routed to a real-GPU-host follow-up rather than shipped blind from a macOS host. Refs #10727. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Reviewed and updated this PR. Decision: keep #10727 open as the broader local-model lifecycle epic, but this code-side slice is correct to land. The implementation only threads facts the existing probe can honestly know: CUDA -> NVIDIA, Metal -> Apple, and unknown/null remains the platform default. It deliberately does not claim AMD/Intel Vulkan until the probe can prove it. I added Local verification on
CI is queued on the new commit; I am enabling auto-merge rather than merging ahead of it. |
… loader's capability gate (#10727) (#11671) Follow-up to the merged #10727 image-gen vendor-threading fix (#11654), which only genuinely helped Windows NVIDIA. A 5-lens adversarial review caught that on Linux+NVIDIA the fix was still a no-op: selectImageGenBackends gated the CUDA head on `profile.sdCpp?.cudaCapable`, positive evidence the real caller never populates, so a Linux NVIDIA box still returned [{sd-cpp, cpu}] — byte-identical to the pre-fix output — and silently ran image-gen on CPU (the primary #10727 target). - Linux+NVIDIA now proposes CUDA first and lets loadSdCppImageGenBackend be the single source of capability truth (it probes the binary and throws ImageGenBackendUnavailableError → clean CPU fall-through when CUDA is unproven). Only demote to CPU-only on explicit evidence the binary CANNOT do CUDA. Same trust-the-loader contract win32 and the AMD/Intel branch already use. - Log each accelerated→next image-gen fall-through so a GPU box degrading to CPU is visible, not silent — the #10727 failure mode one layer below selection. - Correct the SEV-4 embedding probe-failure warn: the fallback is the performance preset (gpuLayers auto), not a 'conservative default'. - Tests: assert CUDA-first on the EXACT real-caller profile shape (no synthetic sdCpp — the shape the merged test masked with an injected cudaCapable flag), add a mocked-probe service seam guarding the wiring, and prove macOS is unaffected. Refs #10727. Co-authored-by: Shaw <shawgotbags@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
2 participants
Summary
Closes the one remaining code-side silent-CPU-fallback in local inference that #10727's audit surfaced beyond the already-merged embedding-probe flip (#10812): image generation ignored the GPU entirely.
LocalInferenceService.loadImageGenBackendhardcodedgpu: undefinedin theImageGenRuntimeProfile, soselectImageGenBackendsfell through tosd-cppCPU on every Linux/Windows box — including NVIDIA, whose backendprobeHardwarealready detects reliably vianvidia-smi(backend: "cuda"). An accelerated path was available and never chosen — exactly the fallback #10727 says to eliminate.What changed
imagegen/backend-selector.ts— new pureimageGenGpuVendorFromProbeBackend(cuda→nvidia,metal→apple,vulkan→amd,null→undefined).service.ts— thread the real probe vendor into the profile via adetectImageGenGpuVendor()helper; NVIDIA Linux/Windows now reaches the CUDA/TensorRT path. A probe failure degrades gracefully to the platform default with a logged reason (not a silent swallow), and image-gen never crashes on a probe error.ensure-local-inference-handler.ts(SEV-4) — the embedding handler's swallowedprobeHardware().catch(() => undefined)now logs why the probe failed, so a broken probe on an accelerated box is visible.hardware.ts(SEV-3) — corrected two false comments that claimed the probe usescapacitor-llamaand reports AMD asvulkan; it does neither (nvidia-smi + Apple-Silicon detection; AMD/Intel leftnullat probe).Evidence (macOS M4 Max, develop worktree)
imagegen/backend-selector.test.ts+hardware.test.ts— 26 pass. New tests drive the real mapper the caller uses (not syntheticgpu:"nvidia"profiles, the gap the audit called out): assert NVIDIA→CUDA at the head of the try-list, win32→TensorRT, and that macOS (mflux/Metal) + AMD/Intel-null paths are unchanged.tsc -p plugins/plugin-local-inference— 0 errors in changed files (satisfies ImageGenRuntimeProfilecompiles).Deliberately deferred (device-gated, not shipped blind)
Embeddings/image-gen on AMD/Intel still run CPU because
probeHardwareleaves themnullat probe time — no cheap pre-load VRAM query, and guessingvulkanfrom a macOS host that can't verify it risks a hard GPU-open throw (there is no TS-side CPU retry). This is documented in-code and belongs on a real AMD/Intel-GPU host (cf. theneeds-human#11339 pattern for the NVIDIA/CUDA validation of #10812). The image-gen path already routesamd/intelto Vulkan the moment the probe can report it.Refs #10727.