Skip to content

Comments

feat: interactive Plotly scatter charts for HTML dashboard#21

Merged
wolfiesch merged 2 commits intomasterfrom
feat/interactive-scatter
Feb 17, 2026
Merged

feat: interactive Plotly scatter charts for HTML dashboard#21
wolfiesch merged 2 commits intomasterfrom
feat/interactive-scatter

Conversation

@wolfiesch
Copy link
Collaborator

@wolfiesch wolfiesch commented Feb 17, 2026

Summary

  • Replace static matplotlib SVG scatter plots with interactive Plotly.js charts in the HTML dashboard
  • Add hover tooltips (library name, pass rate, throughput), box-zoom, pan, and click-to-maximize modal
  • Widen layout to 2800px max-width with responsive breakpoints for 4K/ultrawide monitors
  • Preserve all existing dark-theme aesthetics: zone bands, WolfXL highlighting, capability markers

Changes

File What
pyproject.toml Add plotly>=6.0 dependency
src/excelbench/results/scatter_interactive.py NEW — Plotly renderer (1×3 tier grid + 2×3 feature grid)
src/excelbench/results/html_dashboard.py Use interactive charts, add maximize modal, responsive layout
src/excelbench/results/scatter.py Lighten Score 3 zone band for gridline visibility
tests/test_scatter_interactive.py NEW — 9 smoke tests
uv.lock Auto-updated with plotly 6.5.2

Test plan

  • All 13 tests pass (uv run pytest)
  • Lint clean (uv run ruff check)
  • Dashboard generates successfully (~5.9 MB with inline plotly.js)
  • Visual verification: dark theme, hover tooltips, zoom, maximize modal, zone bands, WolfXL highlighting
  • Tested on 4K monitor — layout fills available width
  • Verify deploy pipeline generates correct HTML

🤖 Generated with Claude Code

Greptile Summary

Replaces static matplotlib SVG scatter plots with interactive Plotly.js charts in the HTML dashboard. The new scatter_interactive.py module reuses the existing data pipeline from scatter.py and renders 1×3 tier and 2×3 feature grids as embeddable HTML fragments. The dashboard gains hover tooltips, box-zoom/pan, a click-to-maximize modal, a wider 2800px layout with 4K/ultrawide responsive breakpoints, and preserves the dark-theme aesthetic.

  • Bug — annotation styling overwrites zone labels: In both _build_tiers_figure and _build_features_figure, the loop that styles subplot titles (for ann in fig.layout.annotations) also overwrites the color-coded score zone annotations ("Score 0"–"Score 3") added by _build_panel, resetting them from their intended colors/sizes to size=16, color='#ededed'.
  • Bug — maximize modal loses chart interactivity: The modal JS uses cloneNode(true) to copy Plotly charts, but this only copies DOM structure, not Plotly's internal state. The maximized chart renders as a static snapshot without hover, zoom, or resize capabilities.
  • Dashboard file size increases to ~5.9 MB due to inline plotly.js bundling (expected trade-off for single-file distribution).

Confidence Score: 2/5

  • Two functional bugs affect visual correctness and the maximize modal feature; recommend fixing before merge.
  • The annotation styling bug silently breaks zone label colors in every rendered chart, and the modal cloneNode issue means the "maximize" feature — a key UX addition in this PR — does not work as intended. Both issues are reproducible and affect end-user experience.
  • Pay close attention to src/excelbench/results/scatter_interactive.py (annotation loop bug at lines 287 and 321) and the modal JS in src/excelbench/results/html_dashboard.py (cloneNode at line 562).

Important Files Changed

Filename Overview
src/excelbench/results/scatter_interactive.py New Plotly scatter renderer. Annotation styling loop at lines 287 and 321 overwrites zone label colors/sizes by iterating all annotations instead of only subplot titles. Otherwise well-structured reuse of existing data pipeline.
src/excelbench/results/html_dashboard.py Dashboard integration for interactive Plotly charts with maximize modal, responsive breakpoints, and layout widening. The modal cloneNode approach breaks Plotly interactivity; charts will appear as static snapshots when maximized. CSS and responsive media queries are well done.
src/excelbench/results/scatter.py Minor color tweak to Score 3 zone band (#133d1f → #0e2916) for better gridline visibility. No functional impact.
pyproject.toml Adds plotly>=6.0 dependency and mypy ignore entries. Clean and correct.
tests/test_scatter_interactive.py 9 smoke tests covering tiers/features HTML output, dark theme colors, hover templates, WolfXL highlighting, zone bands, and from-data API equivalence. Tests depend on fixture files existing on disk (not pytest fixtures), making them non-portable.
uv.lock Auto-updated lockfile adding plotly 6.5.2 and its narwhals dependency.

Flowchart

flowchart TD
    A["render_html_dashboard()"] --> B["Load fidelity & perf JSON"]
    B --> C{"perf data exists?"}
    C -->|Yes| D["_section_scatter(fidelity, perf, heatmap_svg)"]
    C -->|No| E["Skip interactive charts"]
    D --> F["scatter_interactive.py\nrender_interactive_scatter_tiers_from_data()"]
    D --> G["scatter_interactive.py\nrender_interactive_scatter_features_from_data()"]
    F --> H["_build_tiers_figure()\n1×3 subplot grid"]
    G --> I["_build_features_figure()\n2×3 subplot grid"]
    H --> J["_build_panel() × 3\nZone bands + scatter points"]
    I --> K["_build_panel() × 6\nZone bands + scatter points"]
    J --> L["_apply_layout()\nDark theme styling"]
    K --> L
    L --> M["fig.to_html()\nInclude plotly.js in tiers only"]
    M --> N["Embed HTML fragments\nin dashboard"]
    N --> O["Modal JS\nMaximize button + overlay"]
    D --> P["Heatmap SVG\nStill static"]
    E --> P
Loading

Last reviewed commit: 614e465

## Context
The HTML dashboard at excelbench.vercel.app embedded static matplotlib SVGs
for fidelity-vs-throughput scatter plots. These looked good but offered no
interactivity — no tooltips, zoom, or click-to-maximize. Replacing them with
Plotly.js charts adds rich hover info, box-zoom, pan, and a fullscreen modal
while preserving the dark-theme aesthetics.

## Changes
- Add plotly>=6.0 dependency to pyproject.toml (+ mypy ignore)
- Create scatter_interactive.py: Plotly renderer reusing scatter.py data
  pipeline — 1×3 tier grid and 2×3 feature grid with zone bands, threshold
  lines, WolfXL highlighting, and per-point hover templates
- Rewrite _section_scatter() in html_dashboard.py to embed interactive
  Plotly HTML fragments instead of inline SVGs
- Add click-to-maximize modal (⛶ button, ESC/click-outside dismiss) with
  DOM cloneNode for XSS safety
- Widen layout to max-width 2800px with responsive breakpoints at 2000px
  and 3000px for 4K/ultrawide monitors
- Adjust Score 3 zone band color (#133d1f → #0e2916) for gridline visibility
- Add 9 smoke tests for the interactive scatter API

## Impact
- Dashboard HTML grows ~4MB (inline plotly.js); acceptable for a dev dashboard
- Static matplotlib scatter.py unchanged — PNG/SVG generation still works
- All 13 tests pass, lint clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 17, 2026 08:57
@vercel
Copy link

vercel bot commented Feb 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
excelbench Ready Ready Preview, Comment Feb 17, 2026 9:08am

Request Review

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @wolfiesch, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly upgrades the HTML dashboard's data visualization capabilities by integrating interactive Plotly scatter charts. The change moves away from static images to provide users with a more engaging and informative experience, allowing for deeper exploration of fidelity and throughput data. Additionally, the dashboard's layout has been improved to better accommodate modern high-resolution displays, ensuring a consistent and visually appealing presentation.

Highlights

  • Interactive Plotly Charts: Replaced static Matplotlib SVG scatter plots with dynamic Plotly.js charts in the HTML dashboard, enhancing user interaction.
  • Enhanced Chart Interactivity: Implemented hover tooltips displaying library name, pass rate, and throughput, along with box-zoom, pan functionality, and a click-to-maximize modal for detailed viewing.
  • Responsive Dashboard Layout: Widened the dashboard layout to a 2800px maximum width and added responsive breakpoints to optimize viewing on 4K and ultrawide monitors.
  • Aesthetic Preservation: Ensured that all existing dark-theme aesthetics, including zone bands, WolfXL highlighting, and capability markers, are preserved in the new interactive charts.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • pyproject.toml
    • Added 'plotly>=6.0' as a new dependency.
    • Included 'plotly' and 'plotly.*' in the tool.ruff.lint.isort.known-first-party configuration.
  • src/excelbench/results/html_dashboard.py
    • Modified the _section_scatter function to integrate interactive Plotly charts, replacing the previous static SVG rendering.
    • Added CSS styles for interactive chart containers, maximize buttons, and a chart maximization modal.
    • Updated the main container's max-width and padding, and introduced new media queries for responsive design on larger screens (2000px and 3000px widths).
    • Implemented JavaScript for the chart maximize modal functionality, including opening, closing, and re-rendering Plotly charts.
  • src/excelbench/results/scatter.py
    • Adjusted the color of the Score 3 zone band from '#133d1f' to '#0e2916' for improved gridline visibility.
  • src/excelbench/results/scatter_interactive.py
    • Introduced a new module for rendering interactive Plotly scatter plots, including functions for tier and feature-based visualizations.
    • Implemented subplot creation, panel building with score-zone bands, threshold lines, and annotations.
    • Defined shared dark-theme layout settings and a Plotly marker-symbol mapping.
  • tests/test_scatter_interactive.py
    • Added a new test file containing smoke tests for the interactive Plotly scatter renderer.
    • Verified that the generated HTML includes Plotly.js, dark theme colors, hover templates, WolfXL highlighting, and zone bands.
    • Confirmed that the 'from_data' convenience API produces output equivalent to the file-based API.
  • uv.lock
    • Updated the lock file to include the 'plotly' package (version 6.5.2) and its dependency 'narwhals' (version 2.16.0).
Activity
  • All 13 tests passed successfully.
  • Lint checks passed cleanly.
  • The dashboard generated successfully, with an approximate size of 5.9 MB including inline plotly.js.
  • Visual verification confirmed dark theme, hover tooltips, zoom functionality, maximize modal, zone bands, and WolfXL highlighting.
  • The layout was tested on a 4K monitor and observed to fill the available width as intended.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 286 to 288
# Style subplot titles
for ann in fig.layout.annotations:
ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
Copy link
Contributor

Choose a reason for hiding this comment

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

Annotation loop overwrites zone label styles

The comment says "Style subplot titles", but fig.layout.annotations at this point includes not only the 3 subplot titles from make_subplots, but also all score zone annotations ("Score 0"–"Score 3") added by _build_panel. Each zone label was created with a specific size=11 and color-coded font (e.g., #e4484d for Score 0, #e79c12 for Score 1). This loop overwrites all of them with size=16, color='#ededed', making the zone labels lose their distinctive styling.

The same issue occurs in _build_features_figure at lines 321–322.

A fix would be to record how many subplot title annotations exist before building panels, then only restyle those:

Suggested change
# Style subplot titles
for ann in fig.layout.annotations:
ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
# Style subplot titles (only the ones created by make_subplots, not zone labels)
n_subplot_titles = len(tier_titles)
for ann in fig.layout.annotations[:n_subplot_titles]:
ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/excelbench/results/scatter_interactive.py
Line: 286:288

Comment:
**Annotation loop overwrites zone label styles**

The comment says "Style subplot titles", but `fig.layout.annotations` at this point includes not only the 3 subplot titles from `make_subplots`, but also all score zone annotations ("Score 0"–"Score 3") added by `_build_panel`. Each zone label was created with a specific `size=11` and color-coded font (e.g., `#e4484d` for Score 0, `#e79c12` for Score 1). This loop overwrites all of them with `size=16, color='#ededed'`, making the zone labels lose their distinctive styling.

The same issue occurs in `_build_features_figure` at lines 321–322.

A fix would be to record how many subplot title annotations exist before building panels, then only restyle those:

```suggestion
    # Style subplot titles (only the ones created by make_subplots, not zone labels)
    n_subplot_titles = len(tier_titles)
    for ann in fig.layout.annotations[:n_subplot_titles]:
        ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 10e9185. The annotation loop now uses fig.layout.annotations[:n_subplot_titles] to only restyle the subplot titles created by make_subplots, preserving the color-coded zone labels (Score 0 red, Score 1 yellow, etc.) from _build_panel.

Comment on lines 320 to 322

for ann in fig.layout.annotations:
ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
Copy link
Contributor

Choose a reason for hiding this comment

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

Same annotation override bug as in tiers figure

Same issue as in _build_tiers_figure — this loop resyles all annotations (including score zone labels from _build_panel) to size=16 in _DARK_TEXT color, overwriting the color-coded zone label styling.

Suggested change
for ann in fig.layout.annotations:
ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
# Style subplot titles only (not zone labels from _build_panel)
n_subplot_titles = len(feature_titles)
for ann in fig.layout.annotations[:n_subplot_titles]:
ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/excelbench/results/scatter_interactive.py
Line: 320:322

Comment:
**Same annotation override bug as in tiers figure**

Same issue as in `_build_tiers_figure` — this loop resyles all annotations (including score zone labels from `_build_panel`) to `size=16` in `_DARK_TEXT` color, overwriting the color-coded zone label styling.

```suggestion
    # Style subplot titles only (not zone labels from _build_panel)
    n_subplot_titles = len(feature_titles)
    for ann in fig.layout.annotations[:n_subplot_titles]:
        ann.font = dict(size=16, color=_DARK_TEXT, family="system-ui, sans-serif")
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 10e9185. Same fix applied — uses annotations[:n_subplot_titles] slice for the features figure as well.

Comment on lines 561 to 568
/* Clone chart nodes into modal (same-origin Plotly SVG, safe to clone) */
var clone=chart.cloneNode(true);
while(modalBody.firstChild)modalBody.removeChild(modalBody.firstChild);
modalBody.appendChild(clone);
overlay.classList.add('active');
/* Re-render Plotly charts at full size */
var plots=modalBody.querySelectorAll('.js-plotly-plot');
plots.forEach(function(p){if(window.Plotly)window.Plotly.Plots.resize(p);});
Copy link
Contributor

Choose a reason for hiding this comment

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

Cloned Plotly charts lose interactivity in modal

cloneNode(true) copies the DOM structure (including the rendered SVG), but it does not copy Plotly's internal data stored as properties on the element (_fullData, _fullLayout, etc.). The subsequent Plotly.Plots.resize(p) on the cloned node will fail silently or be a no-op because the clone has no Plotly state.

The result is that the maximized chart appears as a static snapshot of the original chart at its pre-modal size — no hover tooltips, no zoom, no pan, and no resize to fill the modal.

Consider using Plotly.newPlot to re-render in the modal using the source chart's data:

var srcPlot = chart.querySelector('.js-plotly-plot');
if (srcPlot && window.Plotly) {
  var clone = document.createElement('div');
  clone.style.width = '100%';
  clone.style.height = '100%';
  modalBody.appendChild(clone);
  Plotly.newPlot(clone, srcPlot.data, srcPlot.layout, srcPlot.config);
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/excelbench/results/html_dashboard.py
Line: 561:568

Comment:
**Cloned Plotly charts lose interactivity in modal**

`cloneNode(true)` copies the DOM structure (including the rendered SVG), but it does not copy Plotly's internal data stored as properties on the element (`_fullData`, `_fullLayout`, etc.). The subsequent `Plotly.Plots.resize(p)` on the cloned node will fail silently or be a no-op because the clone has no Plotly state.

The result is that the maximized chart appears as a static snapshot of the original chart at its pre-modal size — no hover tooltips, no zoom, no pan, and no resize to fill the modal.

Consider using `Plotly.newPlot` to re-render in the modal using the source chart's data:
```js
var srcPlot = chart.querySelector('.js-plotly-plot');
if (srcPlot && window.Plotly) {
  var clone = document.createElement('div');
  clone.style.width = '100%';
  clone.style.height = '100%';
  modalBody.appendChild(clone);
  Plotly.newPlot(clone, srcPlot.data, srcPlot.layout, srcPlot.config);
}
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 10e9185. Replaced cloneNode(true) with Plotly.newPlot() that rebuilds the chart from the original element's .data and .layout properties. The modal chart is now fully interactive with hover, zoom, and pan. Falls back to cloneNode only for non-Plotly content.

Copy link

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

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: 614e465570

ℹ️ 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 on lines 177 to 180
if scatter_dir:
for name in ("scatter_tiers", "scatter_features", "heatmap"):
svg_path = scatter_dir / f"{name}.svg"
if svg_path.exists():
svgs[name] = _namespace_svg_ids(svg_path.read_text(), f"{name}-")
heatmap_path = scatter_dir / "heatmap.svg"
if heatmap_path.exists():
heatmap_svg = _namespace_svg_ids(heatmap_path.read_text(), "heatmap-")

Choose a reason for hiding this comment

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

P2 Badge Keep scatter_dir SVG fallback when perf data is missing

render_html_dashboard now only loads heatmap.svg from scatter_dir, and _section_scatter renders the tier/feature charts only when perf is present, so dashboards generated without a perf JSON silently drop scatter_tiers.svg and scatter_features.svg even if those files exist. This regresses the previous behavior of embedding pre-rendered scatter SVGs from --scatter-dir and breaks workflows that generate scatter assets separately from perf ingestion.

Useful? React with 👍 / 👎.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 10e9185. render_html_dashboard now loads scatter_tiers.svg and scatter_features.svg from scatter_dir and passes them to _section_scatter(). When perf data is unavailable, these static SVGs are embedded as fallback.

Comment on lines 562 to 563
var clone=chart.cloneNode(true);
while(modalBody.firstChild)modalBody.removeChild(modalBody.firstChild);

Choose a reason for hiding this comment

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

P2 Badge Rebuild Plotly modal chart instead of cloning chart DOM

The maximize handler uses cloneNode(true) on the rendered Plotly container, but DOM cloning does not preserve Plotly's runtime bindings/state, so the modal copy can lose hover/zoom/pan behavior and Plotly.Plots.resize may not operate correctly on the clone. This issue appears whenever a user opens a chart in the maximize modal, making the advertised interactive fullscreen view unreliable.

Useful? React with 👍 / 👎.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 10e9185. The modal now uses Plotly.newPlot(plotDiv, srcPlot.data, newLayout, config) instead of cloneNode, which preserves all Plotly runtime bindings. The chart is rebuilt at the modal's full dimensions with autosize:true.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modernizes the ExcelBench HTML dashboard by replacing static matplotlib scatter plots with interactive Plotly.js visualizations. The new charts support hover tooltips, pan, zoom, and full-screen modal viewing. The layout is also expanded to 2800px max-width with responsive breakpoints for 2K/4K monitors, while preserving the existing dark-theme aesthetics including zone bands, WolfXL highlighting, and capability markers.

Changes:

  • Add Plotly dependency (>=6.0) for interactive visualizations
  • Create new scatter_interactive.py module with Plotly renderer implementing 1×3 tier grid and 2×3 feature grid
  • Update HTML dashboard to embed interactive charts with maximize modal functionality and responsive 2K/4K breakpoints
  • Lighten Score 3 zone band color in static scatter.py for better gridline visibility
  • Add comprehensive smoke tests for interactive chart rendering

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
uv.lock Auto-updated dependencies: adds plotly 6.5.2 and narwhals 2.16.0
pyproject.toml Adds plotly>=6.0 dependency and mypy ignore rules
src/excelbench/results/scatter_interactive.py New module implementing interactive Plotly scatter charts matching static matplotlib output
src/excelbench/results/html_dashboard.py Integrates interactive charts, adds maximize modal, expands max-width to 2800px with 2K/4K responsive breakpoints
src/excelbench/results/scatter.py Lightens Score 3 zone band color (#133d1f → #0e2916) for better gridline visibility
tests/test_scatter_interactive.py New test suite with 9 smoke tests covering HTML generation, dark theme, hover templates, WolfXL highlighting, and zone bands

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the HTML dashboard by replacing static matplotlib charts with interactive Plotly charts, significantly improving user experience and adding responsive breakpoints for larger screens. However, a critical Cross-Site Scripting (XSS) vulnerability has been identified due to the unvalidated embedding of an SVG file into the dashboard. Additionally, there are a few suggestions for code improvement: modernizing JavaScript in html_dashboard.py with const/let and arrow functions, refactoring duplicated HTML generation logic in html_dashboard.py, addressing the use of !important in CSS, and adding comments for magic numbers in scatter_interactive.py.


# Static heatmap SVG (unchanged)
if heatmap_svg:
parts.append(f'<h3>Heatmap</h3><div class="svg-wrap">{heatmap_svg}</div>')
Copy link
Contributor

Choose a reason for hiding this comment

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

security-medium medium

The application reads the content of heatmap.svg from the results directory and embeds it directly into the HTML dashboard without sanitization. If an attacker can control the content of this file (e.g., by providing a malicious results directory), they can achieve Cross-Site Scripting (XSS) when the dashboard is opened. It is recommended to sanitize the SVG content using a library like bleach or a dedicated SVG sanitizer before embedding it.

transition:all .15s;line-height:1}
.chart-maximize-btn:hover{background:#282828;color:#ededed;border-color:#51a8ff}
.plotly-chart{width:100%;overflow-x:auto}
.plotly-chart .js-plotly-plot{width:100%!important}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The use of !important in CSS can lead to maintenance issues as it makes styles difficult to override. It's often a sign that the CSS specificity needs to be adjusted.

While it might be necessary here to override inline styles from Plotly, it's worth exploring alternatives first. Could you try increasing the selector's specificity? If !important is truly unavoidable, please add a comment explaining why it's needed.

Comment on lines 534 to 571
(function(){
var overlay=document.createElement('div');
overlay.className='chart-modal-overlay';
var content=document.createElement('div');
content.className='chart-modal-content';
var closeBtn=document.createElement('button');
closeBtn.className='chart-modal-close';
closeBtn.title='Close';
closeBtn.textContent='\u00d7';
var modalBody=document.createElement('div');
modalBody.className='chart-modal-body';
content.appendChild(closeBtn);
content.appendChild(modalBody);
overlay.appendChild(content);
document.body.appendChild(overlay);
var closeModal=function(){
overlay.classList.remove('active');
while(modalBody.firstChild)modalBody.removeChild(modalBody.firstChild);
};
closeBtn.addEventListener('click',closeModal);
overlay.addEventListener('click',function(e){if(e.target===overlay)closeModal();});
document.addEventListener('keydown',function(e){if(e.key==='Escape')closeModal();});
document.querySelectorAll('.chart-maximize-btn').forEach(function(btn){
btn.addEventListener('click',function(){
var wrap=btn.closest('.chart-maximize-wrap');
var chart=wrap?wrap.querySelector('.plotly-chart'):null;
if(!chart)return;
/* Clone chart nodes into modal (same-origin Plotly SVG, safe to clone) */
var clone=chart.cloneNode(true);
while(modalBody.firstChild)modalBody.removeChild(modalBody.firstChild);
modalBody.appendChild(clone);
overlay.classList.add('active');
/* Re-render Plotly charts at full size */
var plots=modalBody.querySelectorAll('.js-plotly-plot');
plots.forEach(function(p){if(window.Plotly)window.Plotly.Plots.resize(p);});
});
});
})();
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This JavaScript block can be modernized by using const and let instead of var, and by using arrow functions for callbacks. This improves code readability and aligns with modern JavaScript best practices.

I've also replaced the while loop for clearing child nodes with modalBody.innerHTML = '', which is more concise and generally performant for this use case.

(function(){
  const overlay=document.createElement('div');
  overlay.className='chart-modal-overlay';
  const content=document.createElement('div');
  content.className='chart-modal-content';
  const closeBtn=document.createElement('button');
  closeBtn.className='chart-modal-close';
  closeBtn.title='Close';
  closeBtn.textContent='\u00d7';
  const modalBody=document.createElement('div');
  modalBody.className='chart-modal-body';
  content.appendChild(closeBtn);
  content.appendChild(modalBody);
  overlay.appendChild(content);
  document.body.appendChild(overlay);
  const closeModal=()=>{
    overlay.classList.remove('active');
    modalBody.innerHTML = '';
  };
  closeBtn.addEventListener('click',closeModal);
  overlay.addEventListener('click', e => {if(e.target===overlay)closeModal();});
  document.addEventListener('keydown', e => {if(e.key==='Escape')closeModal();});
  document.querySelectorAll('.chart-maximize-btn').forEach(btn => {
    btn.addEventListener('click',()=>{
      const wrap=btn.closest('.chart-maximize-wrap');
      const chart=wrap?wrap.querySelector('.plotly-chart'):null;
      if(!chart)return;
      /* Clone chart nodes into modal (same-origin Plotly SVG, safe to clone) */
      const clone=chart.cloneNode(true);
      modalBody.innerHTML = '';
      modalBody.appendChild(clone);
      overlay.classList.add('active');
      /* Re-render Plotly charts at full size */
      const plots=modalBody.querySelectorAll('.js-plotly-plot');
      plots.forEach(p => {if(window.Plotly)window.Plotly.Plots.resize(p);});
    });
  });
})();

Comment on lines +816 to +829
parts.append(
'<h3>By Feature Group</h3>'
f'<div class="chart-maximize-wrap">'
f'<button class="chart-maximize-btn" title="Maximize"'
f' aria-label="Maximize chart">\u26f6</button>'
f'<div class="plotly-chart">{tiers_html}</div></div>'
)
parts.append(
'<h3>Per Feature</h3>'
f'<div class="chart-maximize-wrap">'
f'<button class="chart-maximize-btn" title="Maximize"'
f' aria-label="Maximize chart">\u26f6</button>'
f'<div class="plotly-chart">{features_html}</div></div>'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The HTML structure for wrapping the interactive charts is duplicated for both 'By Feature Group' and 'Per Feature'. To improve maintainability and reduce code duplication, consider extracting this repeated structure into a helper function.

For example:

def _create_chart_html(title: str, chart_content: str) -> str:
    return (
        f'<h3>{title}</h3>'
        f'<div class="chart-maximize-wrap">'
        f'<button class="chart-maximize-btn" title="Maximize"'
        f' aria-label="Maximize chart">\u26f6</button>'
        f'<div class="plotly-chart">{chart_content}</div></div>'
    )

# ... inside _section_scatter ...
parts.append(_create_chart_html('By Feature Group', tiers_html))
parts.append(_create_chart_html('Per Feature', features_html))

font=dict(color=_DARK_TEXT, family="system-ui, sans-serif"),
margin=dict(l=70, r=40, t=90, b=90),
showlegend=False,
height=550 * nrows + 130,
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The calculation for the figure height uses magic numbers (550 and 130). This makes it difficult to understand how the height is determined. Please add a comment to explain what these numbers represent.

Suggested change
height=550 * nrows + 130,
height=550 * nrows + 130, # 550px per subplot row + 130px for title/margins

…vity, SVG fallback

## Context
Automated reviewers (Greptile, Codex, mypy CI) identified 5 issues in PR #21.

## Changes
- Remove unused `type: ignore` comments on plotly imports (mypy CI)
- Cast `fig.to_html()` returns to `str` to satisfy mypy `no-any-return` (4 sites)
- Fix annotation loop to only style subplot titles, not zone labels — use
  `annotations[:n_subplot_titles]` slice instead of iterating all (Greptile)
- Replace `cloneNode(true)` modal with `Plotly.newPlot()` rebuild using
  original chart's `.data` and `.layout` for full interactivity (Greptile+Codex)
- Restore static SVG fallback in `_section_scatter()` when perf data is
  unavailable — loads scatter_tiers.svg and scatter_features.svg from
  scatter_dir (Codex)

## Impact
- CI mypy should now pass (6 errors fixed)
- Maximized charts are fully interactive (hover, zoom, pan)
- Zone label colors preserved (Score 0 red, Score 1 yellow, etc.)
- Dashboards without perf data still show scatter plots if SVGs exist

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@wolfiesch wolfiesch merged commit 6050e7d into master Feb 17, 2026
7 checks passed
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