Dashboard visual redesign: make Mission Control deliver its value#286
Merged
Conversation
Body text moves to IBM Plex Sans (mono stays pinned on tables, stats, ids, logs, micro-labels); prose table columns render sans. Provenance badges scale up: live reads active, committed reads amber/archival. Bigger gate cells and a .verdict pill class for ship-gate verdicts. Fixes undefined var(--bg)/--danger/--radius-sm, sticky th background mismatch, Syne-as-body tooltip font; adds focus-visible outlines and a --terminal-bg token. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
The parity skill documents a compiled/interpreted sidebar toggle (localStorage slm-mode) that main.tsx never implemented — the four compiled pages were dead code. useMode() + a compiled page map bring it back with interpreted as default. Adds an amber shell banner when /api/system reports no live outputs/ (committed-snapshot cold start), and makes sidebar nav real links (href + aria-current). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
Overview now answers the 5-second questions first: a HeroStrip with the ship-gate verdict for the current reference (GATES PASS/FAIL n/n, or an honest NO GATE EVIDENCE), run id + provenance + deployment state, live job/dispatch counts, and CTAs — then Live jobs / Remote dispatches cards, then the existing insight surfaces. One shared fetch composition (hero.ts) and shared views (HeroStrip, JobLines, DispatchLines) feed both renderers, so compiled and interpreted stay at parity by construction; overview.openui mirrors the new structure and the e2e spec asserts the new title, verdict, and mode toggle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
Empty accepts an optional CTA chip; the gate editor's no-metrics empty links to Smoke and the dispatches empty links to Experiments, in both renderers. Checkpoints now actually uses its navigate prop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe dashboard now presents mission-control hero and in-flight status, derives comparison and smoke metrics from ship-gate policy, supports compiled/interpreted rendering, updates styling and generated assets, and expands end-to-end and backend coverage. ChangesDashboard metric contract
Mission control data and rendering
Frontend presentation and validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Overview
participant fetchHero
participant DashboardAPIs
participant HeroStrip
Overview->>fetchHero: poll hero state
fetchHero->>DashboardAPIs: fetch overview, jobs, and dispatches
fetchHero->>DashboardAPIs: fetch gate verdict for primary run
fetchHero-->>Overview: return HeroData
Overview->>HeroStrip: render mission-control status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
The gate policy is the canonical statement of what training optimizes, but the Overview aggregated a hardcoded four-metric set that silently omitted component_type_recall — the semantic-density floor added so empty programs can't game the gates — from the comparison table, the aggregate score that crowns the strongest experiment, and the weakest-metric insight. The smoke canary likewise hardcoded a 0.66 parse threshold. gate_metric_keys() now derives the lever set from DEFAULT_SHIP_GATES; comparison rows carry lever-keyed metrics plus metric_columns for the UI, so adding or dropping a gate lever re-shapes every metric surface without a dashboard edit (pinned by a new test). The smoke gate pill reads its lever + threshold from /api/gates/policy, and the Overview hero names the failing gate levers next to a FAIL verdict. Both renderers share the same composition, keeping compiled/interpreted parity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/apps/dashboard/src/pages/Overview.tsx (1)
25-28: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse one set of mission-control polls.
useHero()refetches/api/overview,/api/jobs, and/api/dispatches, duplicating all three polls already started here. Derive the compiled hero from these responses or expose one shared hook; reservefetchHero()for interpreted/tool usage.🤖 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 `@src/apps/dashboard/src/pages/Overview.tsx` around lines 25 - 28, Remove the duplicate useHero poll from Overview and derive the compiled hero data using the existing data, jobsRaw, and dispRaw responses from the three usePoll calls. Reuse the shared response interpretation logic where available, while keeping fetchHero reserved for interpreted/tool usage.src/apps/dashboard/src/pages/Smoke.tsx (1)
100-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer the
metricLabelhelper for consistency.Instead of manually replacing underscores with spaces, you can use the
metricLabelhelper introduced inmetrics.tsto ensure consistent capitalization and to benefit from any future label overrides.
src/apps/dashboard/src/pages/Smoke.tsx#L100-L100: importmetricLabelfrom../metricsand changegate.lever.replace(/_/g, " ")tometricLabel(gate.lever).src/apps/dashboard/src/interpret/toolProvider.ts#L219-L219: importmetricLabelfrom../metricsand changelever.replace(/_/g, " ")tometricLabel(lever).🤖 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 `@src/apps/dashboard/src/pages/Smoke.tsx` at line 100, Replace manual lever label formatting with the shared metricLabel helper. In src/apps/dashboard/src/pages/Smoke.tsx:100-100, import metricLabel from ../metrics and use it for gate.lever; make the same import and replacement for lever in src/apps/dashboard/src/interpret/toolProvider.ts:219-219.
🤖 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 `@src/apps/dashboard/src/components.tsx`:
- Around line 573-576: Update the cancellation button handler in the jobs
component to await the postJSON request, track its pending state to disable the
button during cancellation, and surface request failures to the user. Keep the
job in the displayed list when cancellation fails, and clear the pending state
after completion or error.
- Around line 509-514: The run reference anchor in the component should provide
native link behavior and keyboard accessibility. Update the `<a>` rendered
around `ref.run_id` to include an appropriate href using the encoded run ID,
while preserving the existing navigation behavior and placeholder rendering when
no run ID exists.
In `@src/apps/dashboard/src/hero.ts`:
- Around line 38-42: Update the Promise.all requests in the hero data-loading
flow to stop converting API failures into empty fallback objects. Let errors
propagate or use an explicit stale/unavailable state for getJSON calls to
/api/overview, /api/jobs, and /api/dispatches, ensuring the UI distinguishes
unavailable data from valid zero-state results.
- Around line 84-87: Update the inflight dispatch count in the hero data
construction to count only active remote dispatches, filtering the jobs returned
by dispatches() before calculating the length. Exclude completed and failed
historical jobs while preserving the existing activeJobs and execution values.
In `@src/apps/dashboard/src/main.tsx`:
- Around line 89-104: Update FreshnessBanner to use an API-provided
live-evidence/provenance signal rather than outputs_present, which only reflects
directory existence. Expose that signal from Readers.system() based on whether
displayed data has live evidence, return it from /api/system, and have
FreshnessBanner show the committed-snapshot banner when live evidence is absent
while preserving its loading and error behavior.
- Around line 143-149: Update the sidebar link onClick handler to intercept
navigation only for unmodified primary clicks; allow Ctrl/Cmd-clicks and other
modified clicks to retain the browser’s default link behavior for opening real
links in new tabs. Keep navigate(r.path) for intercepted clicks and preserve the
existing active-link rendering.
In `@src/apps/dashboard/src/pages/Smoke.tsx`:
- Around line 113-118: The missing-value checks in the Smoke.tsx gate renderer
and toolProvider.ts threshold logic must handle both null and undefined. Update
the checks at src/apps/dashboard/src/pages/Smoke.tsx lines 113-118 and
src/apps/dashboard/src/interpret/toolProvider.ts lines 229-234 to use == null
for r.parse/headline and gate.threshold/threshold, preserving the existing
placeholder behavior when either value is missing.
In `@src/slm_training/web/observability.py`:
- Around line 174-186: Update the insight cache fingerprint wherever it is
constructed to include both the current policy derived from gate_metric_keys()
and the comparison evidence, in addition to the existing reference fingerprint.
Ensure policy changes and new experiment evidence invalidate cached best-run,
failure-count, and signature findings while preserving the existing
displayed-row behavior.
- Line 195: Normalize reference metrics to the same gate_metric_keys() set used
by _suite_metrics(), and require every key to be present before calculating
scores or deltas. Withhold comparisons when either side lacks a required lever,
preventing averages across different metric dimensions and ensuring missing
component_type_recall data cannot be ranked as complete.
---
Nitpick comments:
In `@src/apps/dashboard/src/pages/Overview.tsx`:
- Around line 25-28: Remove the duplicate useHero poll from Overview and derive
the compiled hero data using the existing data, jobsRaw, and dispRaw responses
from the three usePoll calls. Reuse the shared response interpretation logic
where available, while keeping fetchHero reserved for interpreted/tool usage.
In `@src/apps/dashboard/src/pages/Smoke.tsx`:
- Line 100: Replace manual lever label formatting with the shared metricLabel
helper. In src/apps/dashboard/src/pages/Smoke.tsx:100-100, import metricLabel
from ../metrics and use it for gate.lever; make the same import and replacement
for lever in src/apps/dashboard/src/interpret/toolProvider.ts:219-219.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f349aba-a31f-489f-a270-a4243c7cf6e3
📒 Files selected for processing (23)
src/apps/dashboard/index.htmlsrc/apps/dashboard/src/components.tsxsrc/apps/dashboard/src/hero.tssrc/apps/dashboard/src/interpret/library.tsxsrc/apps/dashboard/src/interpret/toolProvider.tssrc/apps/dashboard/src/main.tsxsrc/apps/dashboard/src/metrics.tssrc/apps/dashboard/src/pages/Checkpoints.tsxsrc/apps/dashboard/src/pages/Overview.tsxsrc/apps/dashboard/src/pages/Smoke.tsxsrc/apps/dashboard/src/styles.csssrc/slm_training/web/observability.pysrc/slm_training/web/static/app/assets/index-BsLWuLmr.csssrc/slm_training/web/static/app/assets/index-Dl3zSttZ.jssrc/slm_training/web/static/app/assets/index-ONBBdknd.csssrc/slm_training/web/static/app/index.htmlsrc/slm_training/web/static/openui/MANIFEST.jsonsrc/slm_training/web/static/openui/overview.openuisrc/slm_training/web/static/openui/smoke.openuisrc/slm_training/web/static/tokens.csstests/e2e/dashboard.spec.tstests/test_web/test_control_plane.pytests/test_web/test_overview_insights.py
👮 Files not reviewed due to content moderation or server errors (1)
- src/slm_training/web/static/app/assets/index-Dl3zSttZ.js
Comment on lines
+89
to
+104
| /** Amber shell banner when every read is committed-snapshot fallback. */ | ||
| function FreshnessBanner() { | ||
| const [outputsPresent, setOutputsPresent] = useState<boolean | null>(null); | ||
| useEffect(() => { | ||
| getJSON<{ outputs_present?: boolean }>("/api/system") | ||
| .then((sys) => setOutputsPresent(sys.outputs_present !== false)) | ||
| .catch(() => setOutputsPresent(null)); | ||
| }, []); | ||
| if (outputsPresent !== false) return null; | ||
| return ( | ||
| <div className="freshness-banner" role="status"> | ||
| <span className="prov prov-committed">snapshot</span> | ||
| Committed snapshot data — no live <span className="mono">outputs/</span> on this host. Run{" "} | ||
| <span className="mono">serve_playground</span> locally for live evidence. | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Base the freshness banner on live evidence, not directory existence.
Readers.system() sets outputs_present from self.outputs.exists(). An empty or stale outputs/ directory therefore suppresses this banner even when every displayed value is committed fallback. Expose an actual live-evidence/provenance signal and use that here.
🤖 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 `@src/apps/dashboard/src/main.tsx` around lines 89 - 104, Update
FreshnessBanner to use an API-provided live-evidence/provenance signal rather
than outputs_present, which only reflects directory existence. Expose that
signal from Readers.system() based on whether displayed data has live evidence,
return it from /api/system, and have FreshnessBanner show the committed-snapshot
banner when live evidence is absent while preserving its loading and error
behavior.
Comment on lines
+113
to
+118
| gate: (r) => | ||
| r.parse === undefined || gate.threshold === undefined ? ( | ||
| <span className="hint">—</span> | ||
| ) : ( | ||
| <StatusPill value={r.parse >= gate.threshold} label={r.parse >= gate.threshold ? "pass" : "fail"} /> | ||
| ), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use == null to properly catch missing metric values.
Metric values fetched from the server can be null if they were not computed. Using strict equality (=== undefined) fails to catch null, causing the threshold check to evaluate null >= threshold (which implicitly coerces null to 0 and evaluates to an incorrect pass or fail).
src/apps/dashboard/src/pages/Smoke.tsx#L113-L118: update the check tor.parse == null || gate.threshold == null.src/apps/dashboard/src/interpret/toolProvider.ts#L229-L234: update the check toheadline == null || threshold == null.
📍 Affects 2 files
src/apps/dashboard/src/pages/Smoke.tsx#L113-L118(this comment)src/apps/dashboard/src/interpret/toolProvider.ts#L229-L234
🤖 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 `@src/apps/dashboard/src/pages/Smoke.tsx` around lines 113 - 118, The
missing-value checks in the Smoke.tsx gate renderer and toolProvider.ts
threshold logic must handle both null and undefined. Update the checks at
src/apps/dashboard/src/pages/Smoke.tsx lines 113-118 and
src/apps/dashboard/src/interpret/toolProvider.ts lines 229-234 to use == null
for r.parse/headline and gate.threshold/threshold, preserving the existing
placeholder behavior when either value is missing.
Comment on lines
+174
to
+186
| def gate_metric_keys() -> list[str]: | ||
| """The aggregate metric levers, in ship-gate policy order. | ||
|
|
||
| The ship-gate policy is the canonical statement of what training is | ||
| optimizing; deriving the dashboard's metric set from it means a policy | ||
| change (adding/dropping a lever) propagates to every metric surface. | ||
| """ | ||
| keys: list[str] = [] | ||
| for mins in DEFAULT_SHIP_GATES.values(): | ||
| for key in mins: | ||
| if key not in keys: | ||
| keys.append(key) | ||
| return keys |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate cached insights when policy or comparison evidence changes.
These findings now depend on gate_metric_keys(), but the cache identity still includes only the reference fingerprint. A policy update or new experiment can leave best-run, failure-count, and signature findings stale while the displayed rows update. Include the policy and comparison evidence in the cache fingerprint.
🤖 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 `@src/slm_training/web/observability.py` around lines 174 - 186, Update the
insight cache fingerprint wherever it is constructed to include both the current
policy derived from gate_metric_keys() and the comparison evidence, in addition
to the existing reference fingerprint. Ensure policy changes and new experiment
evidence invalidate cached best-run, failure-count, and signature findings while
preserving the existing displayed-row behavior.
- Job cancel awaits the request, disables the chip while pending, and surfaces failures instead of dropping the promise. - Hero run link gets a real href (keyboard-accessible); sidebar and hero links intercept only unmodified primary clicks so Ctrl/Cmd-click still opens a new tab. - fetchHero flags failed core reads as 'EVIDENCE UNAVAILABLE' instead of rendering a fake zero state, and counts only running/queued dispatches as in flight. - Smoke gate checks use == null so a null metric can't coerce to 0 and fabricate a verdict; smoke labels use the shared metricLabel helper. - observability: reference metrics normalize onto the gate levers and vs-reference deltas compare only levers both sides report (no cross-vocabulary averages); the insight-cache fingerprint includes the gate policy so lever changes invalidate cached findings (row-evidence churn intentionally still does not, per the documented cache note); outputs_present treats an empty outputs/ as a cold start. Deliberately skipped: deduplicating Overview's hero poll — the shared fetch composition is what guarantees compiled/interpreted parity, and the duplicate reads are cheap local GETs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
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.
What / why
A critical design review of the dashboard found the visual system solid (moss/ember tokens, shared primitives) but failing its stated value — "one pane of glass" grounded in honest evidence, dogfooding the OpenUI DSL:
ProvenanceBadgewas 0.62rem muted text, and nothing said globally when everything on screen is committed-snapshot fallback (the default on a fresh checkout / Vercel).main.tsxalways rendered interpreted mode — four compiled pages were dead code.var(--bg),var(--danger),var(--radius-sm)referenced but undefined; sticky table-header background mismatch; tooltips set in the display font; sidebar nav<a>withouthref; bare empty states.Changes
.verdictpill for ship-gate verdicts; focus-visible outlines; token bug fixes;--terminal-bgtoken.useMode()(localStorage "slm-mode",data-modeon:root) exactly as thedashboard-openui-parityskill documents, with a sidebar segmented control. Interpreted stays the default; all six compiled pages smoke-loaded./api/systemreports no liveoutputs/, an amber shell banner says the whole dashboard is committed-snapshot data.HeroStripanswers the 5-second questions first: gate verdict for the current reference (GATES PASS/FAIL n/n, or an honestNO GATE EVIDENCE), run id + provenance + deployment state, in-flight job/dispatch counts, CTAs; then Live jobs / Remote dispatches cards; insights and comparisons follow. One shared fetch composition (hero.ts) and shared views feed both renderers, so parity holds by construction.overview.openuimirrors the structure.Emptyaccepts an optional CTA chip; the gate editor's no-metrics empty links to Smoke, the dispatches empty links to Experiments, in both modes.MANIFEST.jsonregenerated.Verification
scripts/validate_page_dsl.py --check✅;pytest tests/test_web72 passed, 1 skipped ✅;tsc --noEmit+ vite build ✅..verdict, and the mode toggle. The one failure ("editing a gate threshold re-evaluates live") expects the first run to initially show GATES PASS — it fails identically against the pre-change bundle frommainon this host because the liveoutputs/data genuinely fails gates; data-dependent and pre-existing, not a regression.🤖 Generated with Claude Code
https://claude.ai/code/session_01WLQiLHopeRPDhNgJ6Pwp8H
Generated by Claude Code
Summary by CodeRabbit
New Features
Style
Tests