feat(dash): IA redesign — 5-tab shell, inline Details, live TTFT/TPOT, Observe - #62
Merged
Conversation
…se 0) Add tests/dash_characterization.rs freezing current ui::draw behaviour for every existing tab (Overview/Hardware/Instances/Bench/Chat) via TestBackend buffer-text assertions + a squeezed-height no-panic sweep, so the upcoming ActiveTab restructure is regression-guarded. Land the gen_mockups visual target + the 16 baseline mock SVGs the migration renders against. Signed-off-by: Michael Roy <michael.roy@amd.com>
…opt_row (Phase 1) Port the only net-new mock painting onto reusable, unit-tested pure fns with no live wiring yet: - ui/tabs/mod.rs::draw_tab_panel — outlined folder-tab body frame - ui/modal.rs::grey_overlay / draw_logo (btop ROCm, lowercase m) / opt_row draw_logo reuses gradient::lerp3_t instead of a local lerp. Tab hit-testing stays on the single-source compute_chip_layout (unchanged, still [TabChip;5]); the outlined panel is wired in P2-P4. Each helper has a TestBackend test. Signed-off-by: Michael Roy <michael.roy@amd.com>
…ed) (Phase 2) Add ActiveTab::Home as the first tab + ui/tabs/home.rs (state-driven hero GPU gauge/spark, VRAM/TEMP/POWER mini-spark cluster, context-aware Next-step card, Running/Health/Updates tiles; honest placeholders when telemetry absent). The enum edit lands atomically with every literal coupling: next/prev/from_digit, TAB_LABELS (6), compute_chip_layout ([TabChip;6] + offsets), the '1'..='6' digit guard, ui/mod.rs dispatch + footer 1-6, modal.rs draw_help arm, and all coupled tests (tab-cycle, digit, chip-layout, tab_bar_hit, chat-gate). Default stays Overview (additive-then-switch) — P3 repoints it. Signed-off-by: Michael Roy <michael.roy@amd.com>
Final tab model: Home (default) / Action / Observe / Chat. - Observe (ui/tabs/observe.rs) folds the former Overview/Hardware/Instances/Bench surfaces by composing their existing draw fns; selectable list = instances. Amber demo-data banner (F151) is shown iff live daemon telemetry is absent, driven by ConnState/snapshot presence (no hardcoded flag). - Action (ui/tabs/action.rs) is an arrow-nav verb list (Serve/Install/Engines/ Doctor/Updates/Providers) whose rows open the EXISTING managers via the EXISTING seam (KeyAction::Open* -> apply_action -> RocmToolExecutor + ui/approval.rs). No second approval path; no image-generation verb; Optimize + Uninstall are display-only (soon/greyed). - Enum collapse + every literal coupling updated atomically: next/prev/from_digit, '1'..='4' guard, TAB_LABELS(4)/compute_chip_layout([TabChip;4]), ui dispatch, detail+footer+help, slash home/gpu, mouse hit-test, and all coupled tests. - Default + dash entry (apps/rocm/dash.rs) repointed Overview->Home; bare rocm still opens the dash; rocm chat still reaches Chat. KeyAction derives Copy. Invariants held: rocm-core edges=0, single approval path. Signed-off-by: Michael Roy <michael.roy@amd.com>
… (Phase 4) Add four overlays composing the Phase-1 chrome helpers over the stable 4-tab body: - Modal::Menu — btop Esc main menu (grey_overlay + draw_logo + Options/Help/Quit, ↑↓ cycle, Enter activates). Esc opens it only when idle; Chat keeps its Esc meaning; managers/approval are routed upstream so they still own Esc. - Modal::Palette — ':' opens a 'Go to…' command palette routing to the tab switch. - Modal::Options — tabbed General/CPU/GPU/Engines via draw_tab_panel + opt_row; Theme wired to live config, net-new toggles display-with-intent (ponytail). - Modal::GlobalHelp — 2-column keyboard reference (distinct from contextual ?). Header chrome shows 'Esc menu · t theme · ? help'. New KeyActions OpenMenu/ OpenPalette/MenuMove/OptionsTab/MenuActivate + wrap_cursor; cursors menu_sel/ palette_sel/options_tab on AppState. Tests per overlay + Esc-semantics. Signed-off-by: Michael Roy <michael.roy@amd.com>
Add ui/launcher.rs — a minimal front door with a live status strip (GPU + serving, running vs idle variants) over an icon menu: Serve / Set up / Diagnose / Chat / Open full dashboard, plus a display-only Optimize (soon) row. No image-generation verb. Pure draw fn + run_launcher() synchronous pre-screen (ponytail: thin pre-dash loop, escalates into the existing dash run path). Routing: bare interactive rocm -> dash::run_launcher (main.rs launch_default); 'Open full dashboard'/d/Serve/Set up/Diagnose escalate into dash::run (Home), Chat -> run_chat. 'rocm dash' / 'rocm chat' bypass the launcher unchanged. Tests: launcher running/idle render (status strip + >=4 rows, no image verb, soon badge), choice mapping, squeeze; launch_default routing assertion updated. Signed-off-by: Michael Roy <michael.roy@amd.com>
Add ui/dock.rs and a width-gated split in ui/mod.rs: when cols>=180 && rows>=45 the body becomes GPU wall (left ~40) / center tab body (unchanged) / right dock (~52); below the threshold the single-column path is byte-for-byte unchanged (Phase 0 narrow snapshots hold). Right dock is read-only and NEVER a composer: - logs_dock (LOGS) — live job-output ring from AppState.jobs, for Observe/Action. - context_rail (CONTEXT) — RUNNING SERVICES / GPU STATE / RECENT TOOLS from instances/snapshot/jobs, for Home/Chat. - gpu_wall left rail from snapshot telemetry. The mock's interactive-composer rail is NOT ported (grep assistant_dock = 0); the Chat composer stays only in tabs::chat (draw_input untouched); seam unchanged. Tests: triptych extents + is_wide gate, logs/context dock content asserting no composer leak, gpu wall, plus ui::draw wide-shows-dock / narrow-single-column. Signed-off-by: Michael Roy <michael.roy@amd.com>
…or (Phase 7) Add the falsifiable done-bar coverage on top of the migrated surfaces: - States: observe_empty_state, loading_state_connecting_banner, disconnected_banner_present render correct placeholders/banners. - Honesty: demo banner absent on Connected+telemetry, present when telemetry absent; no image-generation verb anywhere in src (grep=0); Optimize soon. - A11y of color: connection status carried as text labels (connected/ disconnected), not color-only. Invariants verified: rocm-core edges=0, single approval path, agent.rs sole rig namer (no rig use outside agent.rs), assistant_dock not ported (=0). Deterministic gate green; clippy 0 warnings; gen_mockups regenerates the 16-scene parity target. Signed-off-by: Michael Roy <michael.roy@amd.com>
…mode, node-throughput hero + activity feed Address design-mock feedback: 1) The outlined folder tabs (draw_tab_panel) are now the LIVE tab chrome in ui::draw, replacing the flat segmented bar — Home/Action/Observe/Chat render as raised folders. Tab hit-testing shares one geometry source (outlined_chip_spans) so clicks land on the painted folder; flat draw_tab_bar removed. 2) Wide layout: the tab panel wraps only the CENTER column (wide_triptych), left-aligned with the center block, with the GPU wall / dock rails beside it (rails align with the center content panel). Previously the bar spanned full width starting at the far left. 3) Home hero title is now 'Node throughput · N × MODEL · ROCm V' (mock parity), falling back to 'Unknown GPU' with no fabricated count/version. 4) New 'Activity · node' block: node-load mini-spark + recent-activity feed derived from running services and recent jobs (honest placeholder when empty). Tests: chip-layout/tab_bar_hit updated to outlined geometry; wide_triptych alignment + narrow fallback; node_throughput_title formatting + Unknown GPU fallback; home activity/title. Gate green, clippy 0, gen_mockups regenerated. Signed-off-by: Michael Roy <michael.roy@amd.com>
A stray sed during the Phase-6 chrome restructure had set footer_area = body (the full-height middle region), so draw_footer painted the keyboard/control legend at the TOP of the body instead of the bottom status row. Point footer_area back at the 1-row bottom region (outer[2]). Regression test asserts the 'quit' legend tail is on the last row and the body legend chips (e.g. 'jump') are absent from the top header band. Signed-off-by: Michael Roy <michael.roy@amd.com>
…ion detail, clickable legend UI polish across the dashboard TUI: - New ui/panel.rs `bento()` helper: the single source of box chrome — rounded corners, a btop-style inline title whose border turns down on each side of the label (descenders), a semantic BoxRole border color, a faint per-role surface tint, and adaptive left/top padding. A compact `popup()` variant preserves manager-overlay geometry. The 4-tab folder panel is intentionally left as-is. - Every bordered box converted to bento with a distinct semantic role so adjacent boxes differ; draw_popup_frame cascades the look to all managers. - Global theme.bg fill in ui::draw so empty space matches the theme background (removes the stray black box below the tabs). - Action tab: detail pane shows a real per-verb summary (grounded in each manager's actual flow) with a focus-aware Start; →/Enter steps focus into the box, a second Enter opens the manager, ←/Esc back out one level. Fully mouse-operable (click a verb, click the detail to step in). - Footer legend is clickable: caps record geometry and dispatch the same KeyAction on click; clickable caps are accent+bold+underlined (non-color affordance) on a luminance-tracking surface for legibility on all themes. Verified: cargo build, clippy --workspace --all-targets -D warnings, fmt, and 520 tests (502 lib + 13 + 5) all pass. Signed-off-by: Michael Roy <michael.roy@amd.com>
- Drop the full-cell descender row under the title; the notch is now just the bracket arcs' own half-cell downward turn (~70% shorter). - Remove the inner spaces so the brackets hug the label: `╮Label╭`. Signed-off-by: Michael Roy <michael.roy@amd.com>
… flatten box backgrounds - Round the tab folders and the body frame they sit in (╭╮╰╯), matching the bento boxes. - Inactive folders now use the frame's border color so they read as one piece; only the active folder is accented. - The active folder's opening corners are drawn in the border color, so the accent turns straight down into the frame and never continues horizontally along the body border. - Drop the per-role surface tint: every box uses the single theme background. Boxes are distinguished by border color (and inner elements), not fills. Signed-off-by: Michael Roy <michael.roy@amd.com>
The active folder's rounded opening corners are drawn in the accent color again, so the active color continues down the sides and curves outward at the corners. Because the corners are rounded arcs (not the old sharp horizontal segments), this reads as an outward flourish rather than a horizontal run; the frame line beyond the corners stays the border color. Signed-off-by: Michael Roy <michael.roy@amd.com>
Replace the single Action tab with two domain tabs (ROCm, Serving) in the ActiveTab enum, widening the IA to five tabs. This is the visible skeleton for the inline-Details redesign; every exhaustive ActiveTab match site is updated in one coherent edit (next/prev/from_digit, digit guard '1'..='5', selection model, render dispatch, footer 1–5, modal help, palette, dock, tab-bar hit-test geometry). ROCm and Serving both render the ported Action verb list as an explicit one-phase placeholder; Phase 2 lands the real per-tab modules + PaneFocus. Also folds in the Home "Next step" card removal already staged in the tree. Tab geometry expands to 5 chips (ROCm=4, Serving=7 cols); chip-offset and tab_bar_hit tests updated for the new extents. Full gate green: build, clippy -D warnings, fmt --check, test -p rocm-dash-tui --test-threads=1 (519 pass / 0 fail). Signed-off-by: Michael Roy <michael.roy@amd.com>
Split the single Action tab into two domain tabs, each an Actions list +
inline Details pane built on a shared renderer (ui/tabs/pane.rs):
- ui/tabs/rocm.rs — Install · Check for updates · Diagnose (doctor) ·
Runtimes · Command runner · Uninstall[dim]
- ui/tabs/serving.rs — Serve · Engines · Running instances/services ·
Providers & keys · Logs · Optimize[soon]
Per-verb summary/steps/cmd are grounded in the real manager flows; verbs map
to the existing KeyAction::Open* seam (no second approval path). Display-only
rows (Uninstall/Optimize) are safe no-ops.
ActionFocus → PaneFocus { Actions, Detail } shared by both tabs; action_sel →
per-tab rocm_sel/serving_sel; KeyAction::Action* → Pane* (FocusDetail/
FocusActions/Activate/Escape/Select). resolve_mouse routes each tab to its own
hit_test. action.rs deleted and unreferenced (tabs::action / mod action /
ActionFocus = 0 matches).
Full gate green: build, clippy -D warnings, fmt --check, test -p
rocm-dash-tui --test-threads=1 (522 pass / 0 fail).
Signed-off-by: Michael Roy <michael.roy@amd.com>
…ails pane The core UX shift: the 12 operational managers' first screens now render INLINE in the ROCm/Serving Details pane instead of as centered popups. Contract (built first, then fanned out): - ui/mod.rs: draw_active_manager() draws the open manager into the Details rect (pane::detail_rect) on ROCm/Serving, or a centered overlay rect when opened from a non-domain tab (Observe hotkeys / Chat slash commands). - app/mod.rs: event-loop Esc back-out arm (should_pane_back_out) closes the inline manager and returns focus to the Actions list. - Each manager's first screen fills its rect via panel::bento (dropped centered_rect + draw_popup_frame on the first-screen path). Sub-popups (folder browser / model picker / import input), gating approvals, and job consoles stay centered via centered_rect. Gating preserved: approval::draw_approval is the sole approval renderer (1 definition), still drawn LAST over the full body, owns the screen. Also retires the per-tab letter hotkeys (w/e/d/u/i/l/…) on ROCm/Serving — the Actions list is the single path there; they remain on Observe. Adversarial multi-lens review (correctness / a11y×15 / nav / approval) run; resolved both HIGH findings + the MEDIUMs: - Esc back-out now defers to the manager when a sub-popup / approval / job console is open (active_overlay_at_root gate) — no more ejecting a manager mid-flow or abandoning a running job. - resolve_mouse swallows body clicks while a manager is open (no click-through to the obscured Actions/Details list). - Footer shows "Esc back out" while inline; help text reworded. Two LOWs (inline-manager focus-brighten, manager hint copy) deferred to P6. Full gate green: build, clippy -D warnings, fmt --check, test -p rocm-dash-tui --test-threads=1 (527 pass / 0 fail). New tests: 12-manager inline render, back-out flow, sub-screen Esc deferral, mouse swallow. Signed-off-by: Michael Roy <michael.roy@amd.com>
Add live first-token / per-output-token latency, mirroring the proven
gen_tps / tokens_per_watt counter-windowing template:
- vllm_prom.rs: read vllm:time_to_first_token_seconds_{sum,count} and
vllm:time_per_output_token_seconds_{sum,count}; carry the cumulative
readings on InstanceSample (ttft_sum_s/ttft_count/tpot_sum_s/tpot_count).
- traits.rs: thread the new sample fields; merge_instance leaves
Instance.ttft_ms/tpot_ms None (derived at the runner).
- metrics.rs: Instance.ttft_ms / tpot_ms : Option<f64> with #[serde(default)]
(NDJSON replay back-compat); extended back-compat + round-trip tests.
- runner.rs: avg_ms_from_histogram() windows successive readings
(Δsum/Δcount × 1000) with a cumulative-average fallback on the first
scrape; per-instance prev maps cleaned up on disappear/failure (mirrors
prev_gen_tokens); sets inst.ttft_ms/tpot_ms.
- demo.rs: synthesize plausible TTFT/TPOT so `rocm dash --demo` shows real
values; the first instance stays None to exercise the honest `—` path.
- Honest fallback everywhere: None when unavailable; never fabricated.
Gate green: build, build -p rocm-dash-daemon, clippy --all-targets -D
warnings, fmt --check, test -p rocm-dash-{tui --test-threads=1,collectors,
core}; daemon windowing + demo tests pass (daemon suite single-threaded —
a pre-existing replay-ring parallelism flake, unrelated to this change).
Signed-off-by: Michael Roy <michael.roy@amd.com>
Reorient Observe around AI-serving efficiency/throughput: - observe.rs: top band = two hero panels — Node efficiency (tok/watt big number + trend sparkline over snapshot history) and Node throughput (Σ gen_tps + total board power W). Below: the AI per-instance table, then the deep hardware/GPU cluster and bench rollup (kept reachable). - instances.rs: new draw_table() — model · tok/s · tok/watt · TTFT · TPOT · power · queue (running/waiting) · kv-cache%, with tok/watt surfaced in accent and honest `—` for every missing Option metric. - efficiency.rs: instance_power_w() — per-instance board power via the same id-normalizing join as tokens_per_watt (None, not 0 W, when unmatched). Color is never the only signal: every value carries a text label + unit. Observe's instances surface is keyboard + scroll-wheel driven (the AI table sits below the hero band, so left-click row-mapping is intentionally not wired). The former card grid + kv heatmap renderers remain (drill-in detail via draw_detail) — flagged for P6 cleanup. Gate green: build, clippy -D warnings, fmt --check, test -p rocm-dash-tui --test-threads=1 (530 pass / 0 fail). New tests: two-hero render, AI table headers+values, honest `—` placeholders, deep hardware/bench reachable. Signed-off-by: Michael Roy <michael.roy@amd.com>
Cross-cutting verification for the 5-tab redesign: - a11y_every_theme_renders_every_tab_with_chrome_intact: renders all 5 tabs under every registered theme (≥15); asserts no panic, the rounded btop tab chrome survives, and labels render. Single background is structurally guaranteed by ui::draw painting the whole frame with theme.bg first. - demo_buffer_dump_all_tabs_inline_details_and_heroes: drives ui::draw over a connected demo snapshot (one live instance + one cold) and confirms every surface — all 5 tabs, inline manager Details on ROCm (install) + Serving (serve wizard), and the Observe heroes with tok/watt + live TTFT (150ms) + honest `—` for the cold instance. Prints DEMO_DUMP excerpts for evidence. - Tidy a stale Action-era label in the draw_tab_panel unit test. Full workspace deterministic gate green except the pre-existing, environment-dependent apps/rocm tui.rs test (running_models_hides_stopped_services_from_main_list), which fails identically at baseline eb10324 (empty diff for apps/rocm/src/tui.rs this run) — attributed per the Phase 6 rule, not a redesign regression. Signed-off-by: Michael Roy <michael.roy@amd.com>
Restore the operational managers as centered modals on every tab. The inline Details-pane render orphaned into a floating box when you switched tabs; the Details bento + Start affordance stay, and Start now opens the manager in the modal. Dim the backdrop (grey_overlay) and paint an opaque card (Clear + theme-bg fill) so body glyphs no longer bleed through the box gaps or the job console's inset ring. Scrolling, routed to whatever the pointer is over: - Job console gains real vertical + horizontal scroll (was hard-coded to 0); the 12 identical draw_job_console branches are centralized into draw_active_manager. Wheel / PgUp-PgDn / arrows pan the open console. - Wide-layout right LOGS dock is scrollable: dock_logs_scroll windowed from the tail (title shows the offset), wheel-routed when hovered. - Domain-tab wheel moves the Actions selection by one row, and only while the pointer is over the Actions column (was Move(+/-3) anywhere, which skipped a row). Tests: re-point the inline-render characterization to centered-modal; add console + dock scroll routing, clamping, and windowing tests. Signed-off-by: Michael Roy <michael.roy@amd.com>
Signed-off-by: Michael Roy <michael.roy@amd.com>
…eaders, lints) The dash redesign predates main's doctor_manager→examine_manager rename (PR #31) and main's hawkeye license-header enforcement. Rebasing --onto main surfaced three classes of gap in code that did not textually conflict: - references to retired symbols (OpenDoctor, doctor_manager, DoctorManagerState) auto-merged into app/mod.rs, tabs/rocm.rs and the characterization test — renamed to the examine_* API, - new files added by the redesign (dock, launcher, panel, the home/observe/ rocm/serving/pane tab modules, gen_mockups, dash_characterization) lacked the MIT SPDX header — added via hawkeye format, - a few fmt/clippy nits (redundant field name, field-reassign-with-default, semicolon-if-nothing-returned) surfaced by the merge. Gate is green: build --all-targets, cargo fmt --check, clippy -D warnings, hawkeye check, and the rocm-dash-tui suite (545 tests). Signed-off-by: Michael Roy <michael.roy@amd.com>
michaelroy-amd
force-pushed
the
dash-ui-polish-work
branch
from
June 27, 2026 05:50
1ef851d to
8385ab6
Compare
michaelroy-amd
enabled auto-merge
June 27, 2026 15:52
Member
Author
|
This is the dash UI redesign |
rominf
requested changes
Jun 29, 2026
rominf
left a comment
rominf
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the large redesign. Requesting changes — a few blocking items plus polish. One category (non-public references in the added docs) I've sent you privately rather than reposting here; the rest is below.
Blocking
- Added
plans/+docs/design/material shouldn't ship in a public repo as-is — severaldocs/design/links resolve outside the repo or to paths that don't exist, andplans/*.mdare internal implementation notes. Specifics sent privately. Please remove theplans/additions and scrub/remove the out-of-repo references indocs/design/. (This also overlaps an in-flight cleanup that removesplans/and some tracker docs — they'll need reconciling.) - Legacy code isn't fully retired.
crates/rocm-dash-tui/src/ui/tabs/overview.rsis orphaned — the new IA has no Overview tab andoverview::drawhas no callers; please delete it and itspub mod overview;.apps/rocm/src/tui.rsis retained only via the_RETAINED_TUI_ENTRYdead-code anchor — either complete its removal here or split it into its own tracked change so this PR doesn't carry a ~45k-line dead module. (tabs/mod.rsdraw_placeholderis also uncalled.) - Esc-menu unusable on short terminals —
ui/modal.rs:408guardsinner.height < 6, but items render frominner.y + 8, so at inner heights 6–8 the menu shows the logo with no selectable Options/Help/Quit. Guard should be ~< 9, with a test in that range.
Should-fix
ui/launcher.rs:239footer advertisest theme, but the loop has nothandler — the key does nothing. Remove the hint or implement it.ui/modal.rs:561global Help shows"1 .. 4"for jump-to-tab, but there are 5 tabs (draw_helpcorrectly says"1 .. 5").app/mod.rs:743/763scroll offsets use... as u16, which wraps when job output exceeds 65,536 lines — use a saturating cast.- Doc-comment misattribution at
app/mod.rs:1226and:1958(///blocks bind to the wrong item, leavingapply_actionundocumented) — add a blank-line separator.
Polish
- Stale "tab" module/doc comments now that Hardware/Instances/Bench are Observe sub-panels (
hardware.rs:1,336,instances.rs:1,494,bench.rs:1,378,overview.rs:5); leftover "Doctor overlay" comments after the examine rename (app/mod.rs:504,1617); staleinitial_tabdoc atapp/mod.rs:54references the removedOverview. Test namesaction_tab_renders_key_labels/slash_gpu_switches_to_hardwareno longer match what they assert. - Test gaps: TPOT value never asserted (fixture hardcodes
tpot_ms = 22.0; only TTFT150mschecked);efficiency.rsinstance_power_wis new public API with no unit tests;instances.rsrendersttft_ms/tpot_mswithout theis_finite()guardtokens_per_wattuses (NaNmspossible). examples/gen_mockups.rsis a separate sketchbook that doesn't call the realui::draw, so the committed SVGs (~1.1 MB) will drift, and a hardcoded version string is baked into output. Consider not committing generated SVGs or adding a CI regen check.
Telemetry math, sign-off, and CI all look good — thanks again.
Removes PR-added internal implementation notes (plans/dash-*), the docs/design/ design notes + ~1.1 MB generated mockup SVGs, and the examples/gen_mockups.rs sketchbook (rominf #1, #10). Scrubs the one surviving non-public reference in shipping code (observe.rs banner doc) and rewords doc comments that linked to the now-deleted example/dir so no dangling references remain. Signed-off-by: Michael Roy <michael.roy@amd.com>
The 5-tab IA has no ActiveTab::Overview and overview::draw had zero callers; tabs/mod.rs::draw_placeholder was likewise uncalled (rominf #2, in-scope part). Removes overview.rs, its pub mod decl, draw_placeholder, and the imports only those needed. tui.rs is deferred to a tracked issue. Signed-off-by: Michael Roy <michael.roy@amd.com>
… scroll, doc fixes - #3: guard the Esc menu on a pure menu_fits() helper so it only renders when the logo AND all three items (Options/Help/Quit) fit; the old inner.height < 6 guard painted the logo with no reachable items at heights 6..=10. New unit test exercises the broken range + boundary. - #4: remove the dead 't theme' launcher footer hint (no t handler). - #5: global Help jump-to-tab now reads '1 .. 5' (5 tabs), matching draw_help. - #6: scroll-offset casts use u16::try_from(..).unwrap_or(u16::MAX) instead of 'as u16' (console v/h, dock logs, bench detail, chat) — no wrap >65535. - #7: split misattributed doc comments so apply_action and clamp_selectors each carry their own doc (and drop a stale gpu_scroll line). Signed-off-by: Michael Roy <michael.roy@amd.com>
…omments and test names - #9 bug: guard ttft_ms/tpot_ms with is_finite() in instances.rs (the shared Observe renderer), matching tokens_per_watt; NaN/Inf now render the em-dash, never 'NaNms'/'infms'. New render test proves it. - #9 test: observe table test now also asserts the TPOT value ('22ms'), not just TTFT ('150ms'). - #9 test: add efficiency::instance_power_w unit tests (match-sum, no-match -> None, empty -> None). - #8: reword Hardware/Instances/Bench 'tab' doc comments to 'Observe sub-panel'; 'Doctor overlay' -> 'examine overlay' (rocm examine job); initial_tab doc says Home (not removed Overview) for rocm dash. - #8: rename action_tab_renders_key_labels -> rocm_and_serving_tabs_render_verb_labels and slash_gpu_switches_to_hardware -> slash_gpu_switches_to_observe to match their assertions; update the referencing parity-map/checklist rows. Signed-off-by: Michael Roy <michael.roy@amd.com>
9 tasks
Member
Author
|
Thanks for the thorough review, @rominf — all actionable items are addressed. The new commits are on this branch; the tui.rs portion of #2 is split into its own PR (#67) as you suggested.
Each change is surgical and traces to a specific item. Happy to adjust anything — and the bootstrap UX change in #67 (it adopts the dash onboarding flow) is worth a look there. |
Resolve conflicts after main advanced (PR #63 cleanup + reroute): - apps/rocm/src/main.rs: keep PR #62's minimal launcher front door for bare `rocm` (dash::run_launcher); drop the dangling docs/tui-retirement-checklist.md reference (deleted on main). - docs/dash-parity-checklist.md, docs/dash-parity-map.md: accept main's deletion (PR #63 removed these tracker docs); the Phase 4 test-name updates to them are moot. Full gate green post-merge: fmt, build, clippy -D warnings, single-thread tests. Signed-off-by: Michael Roy <michael.roy@amd.com>
rominf
approved these changes
Jun 30, 2026
rominf
left a comment
rominf
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed at 351a513 — all the items from my earlier round are addressed and CI is green:
- The non-public docs/planning material and the rendered mockups are removed.
- Orphaned
overview.rs+ unuseddraw_placeholderdeleted. - Short-terminal Esc menu now uses a single
menu_fits()source of truth;t themehint dropped; jump-to-tab hint corrected to1 .. 5. - Scroll offsets saturate instead of wrapping; doc-comments de-orphaned; stale tab/rename comments and test names refreshed.
- New coverage:
is_finite()guards (with NaN/Inf regression test),instance_power_wunit tests, and a TPOT value assertion.
Approving. One follow-up tracked separately (not blocking): apps/rocm/src/tui.rs is still retained behind the _RETAINED_TUI_ENTRY anchor — its actual removal is handled as its own change. Thanks for the thorough turnaround.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redesigns the
rocm-dash-tuidashboard information architecture and serving telemetry. This is the dash IA-redesign work that sat on top of the §1+§2 supergoal stack (PRs #31–#38); it has been rebased clean ontomainso the diff contains only the redesign.Highlights
Home / ROCm / Serving / Observe / Chat, replacing the legacyOverview/Hardware/Instances/Bench/Chatshell, with aPaneFocusmodel andaction.rsretired.de-modal), then returned to overlays (re-modal) with real log scrolling.rocm.docs/design/.Rebase reconciliation
The branch predated
main'sdoctor_manager→examine_managerrename (PR #31) andmain's hawkeye license-header enforcement. A singlefix(dash): reconcile…commit adapts the merged tree: renames retired symbols (OpenDoctor/doctor_manager/DoctorManagerState) to theexamine_*API, adds MIT SPDX headers to the new files, fixes a few fmt/clippy nits, and normalizes mockup file endings.Test plan
cargo build --workspace --all-targetscargo fmt --checkcargo clippy --locked --workspace --all-targets -- -D warningshawkeye checkrocm-dash-tuisuite — 545 tests (lib + characterization + job-bridge)cargo xtask verify-commits)--require-verified) — runs post-push against GitHubtui::tests::running_models_hides_stopped_services_from_main_listfails locally only when a server occupies127.0.0.1:11435; it is an env-dependent legacy-tui.rstest (passes onmainwith the port free).