Skip to content

fix(view): correct remote-mode node counter, spacing, and Topology alignment#241

Merged
inureyes merged 1 commit into
mainfrom
fix/remote-view-polish
May 26, 2026
Merged

fix(view): correct remote-mode node counter, spacing, and Topology alignment#241
inureyes merged 1 commit into
mainfrom
fix/remote-view-polish

Conversation

@inureyes

Copy link
Copy Markdown
Member

Summary

Three remote-mode all-smi view bugs from this session, plus a small README polish.

1. Node counter inflated by reserved tabs

The dashboard derived total_nodes as state.tabs.len() - 1, only subtracting "All". The real remote-mode tab layout is [All, Users, Topology, host1, …] (set in remote_collector, ssh_strategy, replay_collector, runner, and event_handler), so a 50-host cluster rendered as 50/52.

Fix: added is_reserved_tab / host_tab_count helpers in src/ui/tabs.rs and switched draw_system_view to count only host tabs. The helper centralizes the reserved-tab list so future cluster-level tabs can't silently inflate the denominator again.

2. Live Statistics ↔ Tabs collision

The remote sparkline panel's last row sat directly above the tab strip, hurting readability. Added one spacer line at the tail of draw_remote_sparkline_panel (inside the empty-history early-return so it only emits when the panel actually renders) and bumped LayoutCalculator::calculate_header_lines from 5 to 6 to keep the content-area math in sync.

3. Topology tab box borders zig-zagging

center() in topology::graph_render measured field width with str::len(), but every edge label uses (U+2500 — 3 bytes / 1 cell). For center("── NV ──", 13) it saw len == 16 ≥ 13 and returned the label untouched at 8 visible cells — 5 short of the cell width. The right border then slipped left and the NUMA box looked broken.

Fix: switched to s.chars().count(), which equals display width for the ASCII + single-cell box-drawing characters this module emits, and documented the assumption.

Sample render after the fix (all four lines 54 cells):

┌────────────────────── NUMA 0 ──────────────────────┐
│   [GPU 0]      [GPU 1]      [GPU 2]      [GPU 3]   │
│  ── NV5 ──    ── NV5 ──    ── NV5 ──               │
└────────────────────────────────────────────────────┘

4. README download shields

Both Crates.io and GitHub badges rendered as bare downloads | N, giving no visual cue which count came from which source. They now read crates.io downloads and GitHub downloads.

Tests

  • cargo test --lib — 1033 passed, 0 failed.
  • cargo clippy --lib --tests -- -D warnings — clean.

Regression coverage added in this PR:

  • ui::tabs::tests::host_tab_count_excludes_reserved_tabs — asserts the real [All, Users, Topology, host-0..host-49] shape counts as 50.
  • ui::tabs::tests::is_reserved_tab_matches_cluster_level_tabs — pins the reserved-tab list.
  • ui::topology::graph_render::tests::center_uses_display_width_not_byte_length — pins every edge label (──, ── NV ──, ── NV5 ──, ── NSW ──, ── PXB ──, ── NODE ──, ── SYS ──) to its requested cell width.
  • ui::topology::graph_render::tests::render_numa_box_keeps_borders_aligned_with_unicode_edge_labels — integration check that every border-bearing line of a rendered box has identical visible width.

Risk

Low. Pure rendering / arithmetic fixes; no behavior change for local mode or for the data pipeline. Header-line budget bumped by 1 row so the content area shrinks by one row when Live Statistics is visible.

…ignment

Three independent issues in `all-smi view`'s remote mode plus a README polish in one bundle:

1. **Node counter inflated by reserved tabs.** The dashboard derived `total_nodes` as `state.tabs.len() - 1`, only subtracting the "All" tab. The real tab layout — set in `remote_collector`, `ssh_strategy`, `replay_collector`, `runner` and `event_handler` — is `[All, Users, Topology, host1, …]`, so 50 hosts rendered as `50/52`. Added `is_reserved_tab` / `host_tab_count` helpers in `src/ui/tabs.rs` and switched `draw_system_view` to count only host tabs. The helper is reusable and centralizes the reserved-tab list so future cluster-level tabs can't silently inflate the denominator again.

2. **Live Statistics ↔ Tabs collision.** The remote sparkline panel ended its last row and the tab strip started on the immediately following line, hurting readability on dense clusters. Added one spacer line at the tail of `draw_remote_sparkline_panel` (placed inside the empty-history early-return so it only emits when the panel actually renders) and bumped the matching budget in `LayoutCalculator::calculate_header_lines` from 5 to 6 so the content-area math stays in sync.

3. **Topology tab box borders zig-zagging.** `center()` in `topology::graph_render` measured field width with `str::len()`, but every edge label is built from the unicode `─` (U+2500, 3 bytes / 1 cell), so `center("── NV ──", 13)` saw `len == 16 ≥ 13` and returned the label untouched at 8 visible cells — five short of the cell width. The right border `│` then slipped left and the NUMA box looked broken. Swapped to `s.chars().count()`, which equals display width for the ASCII + single-cell box-drawing characters this module emits, and documented the assumption.

Regression tests:

- `host_tab_count_excludes_reserved_tabs` and `is_reserved_tab_matches_cluster_level_tabs` in `ui::tabs::tests` — assert 50 hosts in the real `[All, Users, Topology, host-0..host-49]` shape count as 50, and pin the reserved-tab list.
- `center_uses_display_width_not_byte_length` in `ui::topology::graph_render::tests` — pins every edge label (`──`, `── NV ──`, `── NV5 ──`, `── NSW ──`, `── PXB ──`, `── NODE ──`, `── SYS ──`) to the requested cell width.
- `render_numa_box_keeps_borders_aligned_with_unicode_edge_labels` — integration check that every border-bearing line of a rendered box has identical visible width.

Also relabels the README download shields so each badge surfaces its source — previously both Crates.io and GitHub badges rendered as bare "downloads | N", which gave no visual cue which count came from which source. They now read "crates.io downloads" and "GitHub downloads".

Full library suite (1033 tests) passes; clippy clean.
@inureyes inureyes added type:bug Something isn't working mode:view View mode related status:review Under review priority:medium Medium priority issue labels May 26, 2026
@inureyes inureyes merged commit 710be77 into main May 26, 2026
4 checks passed
@inureyes inureyes deleted the fix/remote-view-polish branch May 26, 2026 08:01
@inureyes inureyes added status:done Completed and removed status:review Under review labels May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mode:view View mode related priority:medium Medium priority issue status:done Completed type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant