Skip to content

fix(cli): resources show suppresses the phantom GPU on GPU-less machines - #275

Merged
saadqbal merged 1 commit into
developfrom
fix/266-resources-phantom-gpu
Jul 14, 2026
Merged

fix(cli): resources show suppresses the phantom GPU on GPU-less machines#275
saadqbal merged 1 commit into
developfrom
fix/266-resources-phantom-gpu

Conversation

@saadqbal

@saadqbalsaadqbal commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

What

Found during a proactive pre-bugbot review of the #266 promote. On a CPU-only host, tracebloc resources (show) reports a phantom GPU:

  • The chart stamps GPU_REQUESTS/GPU_LIMITS = nvidia.com/gpu=1 as literal container env on every install, so ParseTraining returns HasGPU=true.
  • renderResources then prints per training run: up to N CPU · M GiB · 1 GPU even though the machine has no GPU — and under --verbose it also prints gpu: none detected, contradicting itself.

The set path already normalizes exactly this (phantomGPU := current.HasGPU && !machineHasGPU, Bugbot #241); the show path didn't.

Fix

Drop train.HasGPU when the node read succeeded and the node exposes no GPU — the same nodeErr == nil && len(machine.GPU) == 0 guard already used for the "none detected" detail. A node-read failure leaves it untouched (can't confirm absence; the capacity line already says "unavailable").

Verification

  • gofmt/vet clean; go test ./internal/cli/ -run TestRenderResources green.
  • New TestRenderResources_PhantomGPUSuppressed (no-GPU node + phantom GPU_LIMITS env) — fails before the fix (prints · 1 GPU), passes after. TestRenderResources_GPUSurfaced (real GPU node) still shows the GPU.

Once merged to develop, promote PR #266 picks it up.

🤖 Generated with Claude Code


Note

Low Risk
Read-only display fix in the CLI resources view with a focused regression test; no cluster mutation or auth changes.

Overview
On CPU-only hosts, tracebloc resources could still show 1 GPU on the per-run ceiling because chart-default GPU_LIMITS env makes ParseTraining set HasGPU, which clashed with verbose gpu: none detected.

renderResources now clears train.HasGPU when node capacity was read successfully and the machine has no GPU devices—the same guard as the verbose detail and aligned with resources set phantom-GPU handling (Bugbot #241). If node listing fails, behavior is unchanged.

Adds TestRenderResources_PhantomGPUSuppressed for a no-GPU node with phantom GPU_LIMITS env.

Reviewed by Cursor Bugbot for commit 7331df1. Bugbot is set up for automated code reviews on this repo. Configure here.

The chart stamps GPU_REQUESTS/GPU_LIMITS=nvidia.com/gpu=1 as literal container
env on every install, even CPU-only hosts, so ParseTraining reports a phantom
HasGPU=true. `resources show` then printed "per training run: up to N CPU · M
GiB · 1 GPU" on a machine with no GPU — and under --verbose ALSO printed
"gpu: none detected", contradicting itself.
Normalize train.HasGPU to false when the node read succeeded and exposes no GPU
(the same guard already used for the "none detected" detail), mirroring the set
path's phantom-GPU handling (Bugbot #241). A node-read failure leaves it as-is,
since we can't confirm absence. Adds a regression test.
Pre-empts the Cursor Bugbot "phantom GPU" class of finding on #266.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
Contributor

👋 Heads-up — Code review queue is at 36 / 30

Above the WIP limit. The team convention is to review existing PRs before opening new work.

Open PRs currently in Code review (oldest first):

  • averaging-service#181 — feat(weights): normalize-on-read — mixed cycles average instead of rejecting (SEC-03 §8 step 2) · author: @shujaatTracebloc · no reviewer assigned
  • averaging-service#182 — feat(weights): averaging writes SafeTensors (TF + PyTorch) — SEC-03 §8 step 4 · author: @shujaatTracebloc · no reviewer assigned
  • backend#1079 — feat(global_meta): edge dataset_meta exposure + attributes contract at ingest (#924 G4a) · author: @divyasinghds · no reviewer assigned
  • backend#1086 — docs(rfc): SafeTensors weight-format migration — SEC-03 Phase 1 (RFC 0004) · author: @shujaatTracebloc · no reviewer assigned
  • backend#1093 — chore(deps): bump django from 5.2.14 to 5.2.15 · author: @dependabot · no reviewer assigned
  • backend#1095 — feat(experiment): configurable preprocessing knobs incl. tabular scaler — RFC 0003 L1 + L1b (#1094) · author: @LukasWodka · no reviewer assigned
  • backend#1100 — fix(boot): pin SDK install to tracebloc==0.11.2, drop 404 dev line (#1098) · author: @LukasWodka · reviewer: @saqlainsyed007
  • backend#1105 — perf(api): query micro-fixes — notifications N+1, cached data_scientist, composite index, sampling (#975) · author: @aptracebloc · no reviewer assigned
  • cli#266 — main - > enhance CLI features and tests · author: @saadqbal · no reviewer assigned
  • client-runtime#179 — Staging -> Resync ingest.v1 schema for time_series_classification and improve pod watch · author: @saadqbal · no reviewer assigned

Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.)

@saadqbal
saadqbal merged commit 5af9a80 into developJul 14, 2026
21 checks passed
@saadqbal
saadqbal deleted the fix/266-resources-phantom-gpu branch July 14, 2026 11:55

@LukasWodkaLukasWodka 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.

Reviewed — correct and well-scoped. ✅

The guard only drops HasGPU when the node read succeeded and confirmed no GPU (nodeErr == nil && len(machine.GPU) == 0), so a node-read failure correctly leaves it untouched (can't prove absence — the capacity line already says "unavailable"). Mirrors the set path's #241 normalization, and the new test genuinely fails-before/passes-after on the · 1 GPU assertion.

Two tiny test nits (non-blocking):

  • In TestRenderResources_PhantomGPUSuppressed, the first assertion strings.Contains(out, "GPU") && !strings.Contains(out, "none detected") is effectively a no-op here: under WithVerbose(true) the raw-env dump prints GPU_LIMITS (so Contains("GPU") is already true) andnone detected is present, so the && never fires. The · 1 GPU check is the one actually carrying the regression — worth leaning on that (and maybe dropping the first).
  • Optional: also assert the per-run line's positive shape (ends at GiB), so a future refactor that drops the GPU clause differently is still pinned.

LGTM to merge.


Review assisted by Claude Code.

Sign up for freeto 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.

3 participants

@saadqbal@LukasWodka@shujaatTracebloc