Skip to content

fix/explorer saveable dashboards - #2385

Merged
joeharris76 merged 10 commits into
developfrom
fix/explorer-saveable-dashboards
Oct 4, 2026
Merged

joeharris76 merged 10 commits into
developfrom
fix/explorer-saveable-dashboards

Conversation

@joeharris76

Copy link
Copy Markdown
Collaborator
  • 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread results-explorer/src/components/ChartPanel.tsx
Comment thread results-explorer/src/components/SummaryChartOverview.tsx
Comment thread results-explorer/src/lib/dashboards.ts Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-26T02:49:00.576644Z bdef56b PR opened
ℹ️ 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" or "@codex security review".

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.
@joeharris76

Copy link
Copy Markdown
Collaborator Author

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.

@joeharris76

Copy link
Copy Markdown
Collaborator Author

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).

@joeharris76

Copy link
Copy Markdown
Collaborator Author

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.

@joeharris76

Copy link
Copy Markdown
Collaborator Author

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.

@joeharris76

Copy link
Copy Markdown
Collaborator Author

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.

@joeharris76

Copy link
Copy Markdown
Collaborator Author

The public-site visual comparison found changed captures on /results/ at all four viewports (390, 768, 1280, 1600). This PR edits results-explorer/src/pages/Home.tsx and the dashboard components, so the change is likely intended. It needs maintainer acceptance: review the public-site-visual-diagnostics-* artifact from the failed run, then set repository variables APPROVED_HEAD_SHA=13a2808b2a420bef1700c651273f300f0204cf7e and APPROVAL_REASON to a review note, and rerun the failed workflow. Auto-merge is disabled until then.

@joeharris76
joeharris76 added this pull request to the merge queue Oct 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 1, 2026
@joeharris76
joeharris76 added this pull request to the merge queue Oct 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 1, 2026
@joeharris76
joeharris76 added this pull request to the merge queue Oct 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 1, 2026
@joeharris76
joeharris76 added this pull request to the merge queue Oct 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 1, 2026
@joeharris76
joeharris76 merged commit 80ff373 into develop Oct 4, 2026
71 of 75 checks passed
@joeharris76
joeharris76 deleted the fix/explorer-saveable-dashboards branch October 4, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant