Repository navigation
fix/explorer saveable dashboards - #2385
Conversation
joeharris76
commented
Sep 26, 2026
- feat(explorer): add localStorage dashboard store for saved chart views
- feat(explorer): user-saveable chart dashboards
Dashboards are named collections of saved chart views; each view stores the URL that reproduces it, since chart selection, sort, filters, and anchors already live in explorer URL state. Payloads are versioned and validated entry-wise so one corrupt dashboard never takes down the rest.
Add dashboards: named collections of saved chart views persisted to localStorage. Chart state already lives in URL state, so each saved view stores the reproducing URL plus the chart id. Save controls sit in the chart panel header and on long-layout chart cards; the new /results/dashboards/ page manages dashboards (create/rename/delete) and views (open/rename/remove), and the Overview links in. Stored payloads are versioned and validated entry-wise; saved URLs are constrained to site paths.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdef56bb52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Render a SaveChartView control on every long-layout chart figure so charts on the Compare and ResultDetail pages can be saved, matching the tabs layout. Anchor all eight saveable analysis cards on the benchmark page and force-open whichever card a saved-view URL names, so reopening a saved percentile, CDF, histogram, phase, trend, or cost view restores that chart instead of landing on closed cards.
localStorage.setItem can throw when storage is unavailable, blocked, or over quota. The writeStorage catch previously discarded the failure while callers still returned the mutated model, so the next load read stale storage and the saved dashboard disappeared. Keep the latest snapshot in a module-scoped variable on write failure and have readStorage prefer it, so the session keeps seeing what it saved until a successful write clears the fallback.
…ards - Reject protocol-relative and backslash URLs in saved views so a seeded payload can never turn a view link into an open redirect. - Access window.localStorage inside try blocks so SecurityError in private browsing falls back to memory instead of crashing. - Tombstone deletes while writes fail so the stale stored copy cannot resurrect through the memory merge. - Regression tests for all three.
|
Visual CI status: the /results/* capture diffs are real data changes, not flakes. The PR tree renders the current corpus (16 benchmarks, 244 published runs, POWER SCORE/GEOMEAN/NORMALIZED COST columns); the exact-base baseline built from f100589 renders the withdrawn-cloud-TPC-H corpus (22 benchmarks, 253 published runs, GEOMEAN-only table). No code change needed for this — the captures will converge once the corpus-consuming PRs ahead land and the baseline rebuilds from the new tip. Recommend holding queue until then; will re-verify visual after the next develop tip baseline. |
|
Visual diagnostics note (run 36240711404, Sep 26 head): the 14 changed captures are exactly /results/, /results/benchmarks/, /results/platforms/* at all viewports — the three routes this PR touches (Dashboards nav entry plus per-chart save controls on benchmark pages). Refreshed onto current develop in 5ffeceb; the fresh visual run on the new head is the binding comparison. Threads: all three review findings addressed (save controls in both layouts, per-chart anchors, in-memory session fallback with tombstones). |
|
Visual re-verified (run 36323032202, head 5ffeceb): 12 changed captures, a subset of the 14 noted Sep 26 — /results/, /results/benchmarks/, /results/platforms/* at all viewports. Same corpus-drift cause per your assessment above; no code change. Holding for baseline convergence as you recommended. |
|
Maintainer review: refreshed onto develop (merge 876ee46) so the visual gate compares against the current baseline set. Zero open threads. CI re-running on the refreshed head; the /results/* captures will re-compare and any genuine UI delta from the dashboard feature will need a baseline update. Enqueuing after green. |
|
Status note (takeover sweep): the remaining failures are Public-site visual regression (changed /results/ captures at all four viewports) and its acceptance gate. The UI change is intentional in this PR, so the visual baselines need regeneration per the repo visual-baseline policy, then re-push. No code fix needed beyond the baseline update. |
|
The public-site visual comparison found changed captures on |