feat(cli): label VRAM vs system RAM in ls and up tables#482
Conversation
Add a VRAM column to the detailed Citadel (GPU) listing so total GPU memory is visible alongside system RAM, and suffix the unified picker's memory column with VRAM/RAM so mixed GPU and CPU offerings can't be misread.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughThe PR updates GPU memory display labeling and rendering across two CLI output surfaces: the interactive offering selection table gains a "GB VRAM" suffix, with corresponding layout adjustments, and the detailed offerings table gains a new ChangesGPU Memory Display Enhancement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/basilica-cli/src/cli/handlers/gpu_rental_helpers.rs (1)
887-887: 💤 Low valueHeader spacing doesn't match format column width.
The Memory column uses
{:<12}(12 characters) in the format string, but the header has "Memory" + 7 spaces = 13 characters before the separator.📏 Proposed fix for alignment
style( - " Type │ GPU/CPU │ Provider │ Loc │ Memory │ Price" + " Type │ GPU/CPU │ Provider │ Loc │ Memory │ Price" )Also applies to: 902-902
🤖 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 `@crates/basilica-cli/src/cli/handlers/gpu_rental_helpers.rs` at line 887, The header text spacing is misaligned with the format string "{} │ {:<25} │ {:<15} │ {:<4} │ {:<12} │ {}" — the "Memory" header currently has 7 trailing spaces (13 chars) but the format uses a 12-char column; update the header string(s) (the header row(s) printed near the format string in gpu_rental_helpers.rs) to give "Memory" a total width of 12 (i.e., "Memory" + 6 spaces) so it lines up with the "{:<12}" column; apply the same adjustment to the other header occurrence noted around line 902.
🤖 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 `@crates/basilica-cli/src/cli/handlers/gpu_rental_helpers.rs`:
- Line 828: The display uses per‑GPU VRAM for the community cloud (memory_str
computed from min_memory_gb) but secure cloud shows total VRAM; fix by computing
total VRAM and using that in memory_str: multiply min_memory_gb by the GPU count
variable (e.g., gpu_count or num_gpus) to produce total_vram_gb and format
memory_str from total_vram_gb instead of min_memory_gb; update the memory_str
assignment (symbol: memory_str) and ensure any other place that formats per‑GPU
vs total VRAM (symbols: min_memory_gb, gpu_count/num_gpus) is made consistent.
---
Nitpick comments:
In `@crates/basilica-cli/src/cli/handlers/gpu_rental_helpers.rs`:
- Line 887: The header text spacing is misaligned with the format string "{} │
{:<25} │ {:<15} │ {:<4} │ {:<12} │ {}" — the "Memory" header currently has 7
trailing spaces (13 chars) but the format uses a 12-char column; update the
header string(s) (the header row(s) printed near the format string in
gpu_rental_helpers.rs) to give "Memory" a total width of 12 (i.e., "Memory" + 6
spaces) so it lines up with the "{:<12}" column; apply the same adjustment to
the other header occurrence noted around line 902.
🪄 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
Run ID: 5a6efbf2-fcd3-4cd0-a1e8-282e389cadc0
📒 Files selected for processing (2)
crates/basilica-cli/src/cli/handlers/gpu_rental_helpers.rscrates/basilica-cli/src/output/table_output.rs
Add dedicated VRAM column to `basilica ls` secure-cloud table, label the `up` selector Memory column as `<N> GB (VRAM)` / `<N> GB (RAM)`, and put a space between number and unit (`16 GB` vs `16GB`) across VRAM, RAM, storage, and volume size columns to match SI/NIST style.
Community cloud was displaying per-GPU VRAM while secure cloud showed total VRAM, making multi-GPU offerings appear smaller than they are.
The validation error message was normalized to include a space between
the number and unit ("24 GB") in b5993ad, but the corresponding test
assertion still expected "24GB".
1 participant
basilica lspreviously labelled a singleRAMcolumn on the detailed Citadel (GPU) table, but the value was system memory — while--memory-minfilters on GPU VRAM. Offerings could pass the filter and still display a smaller RAM number, e.g. a shadeform kansascity 1× A100 row showingRAM 64GBwhen the A100 carries 80GB of VRAM.This PR:
VRAMcolumn (total GPU memory:mem_per_gpu × gpu_count) to the detailed Citadel (GPU) table; system RAM staysbasilica upunified picker memory column withVRAMorRAMper row, and widens that column so the suffix doesn't squash the priceSummary by CodeRabbit
Release Notes
New Features
Style