feat(examples): example charts for migrated legacy vizzes + thumbnail capture tool - #42328
Draft
rusackas wants to merge 191 commits into
Draft
feat(examples): example charts for migrated legacy vizzes + thumbnail capture tool#42328rusackas wants to merge 191 commits into
rusackas wants to merge 191 commits into
Conversation
Working documents for the remove-legacy-viz-pipeline feature branch; stripped before final merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… leftovers (#41715) Co-authored-by: Claude Code <noreply@anthropic.com>
…1717) Co-authored-by: Claude Code <noreply@anthropic.com>
…41716) Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
…41721) Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
… rose (#41732) Co-authored-by: Claude Code <noreply@anthropic.com>
…egacy plugin (#41728) Co-authored-by: Claude Code <noreply@anthropic.com>
…41730) Co-authored-by: Claude Code <noreply@anthropic.com>
…drop the legacy plugin (#41738) Co-authored-by: Claude Code <noreply@anthropic.com>
… review Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…es (#41751) Co-authored-by: Claude Code <noreply@anthropic.com>
…rt data pipeline (#41750) Co-authored-by: Claude Code <noreply@anthropic.com>
… drop the working trackers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Remove dangling package-lock.json link entries for the removed legacy-* plugin workspaces and regenerate entries for their renamed plugin-chart-* counterparts; npm ci was failing with EUSAGE because the lockfile referenced workspace paths that no longer exist on disk. - Re-parent the bubble-chart-to-echarts migration (d4e5f6a7b8c9) onto the current single migration head (3a8e6f2c1b95) instead of a now-superseded ancestor, resolving the "Multiple head revisions" alembic error that was failing test-sqlite/test-postgres/test-mysql/cypress/playwright. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sed layer loading Multi.tsx now resolves sublayer buildQuery/transformProps via the chart plugin registries instead of the removed legacy explore.ts helper; update the color test's mocks to register deck_scatter/deck_arc stubs and mock SupersetClient.post (matching the new fetch call) instead of .get. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
down_revision on d4e5f6a7b8c9 still pointed at 3a8e6f2c1b95, which master's add_subjects_tables migration (b1c2d3e4f5a6) also forked from, leaving two alembic heads and breaking `superset db upgrade` in every DB-backed CI job. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
down_revision on d4e5f6a7b8c9 pointed at b1c2d3e4f5a6 (add_subjects_tables), which already has a child on master (56cd24c07170 -> 8f3a1b2c4d5e). That left two alembic heads and broke `superset db upgrade` in every DB-backed CI job. Chain the migration after 8f3a1b2c4d5e, master's real current head, instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
…ipeline Resolves conflicts between the async-job-cancellation feature added on master and this branch's removal of the legacy explore_json pipeline: kept the cancellation feature (_register_cancellable_job, cancel_job, is_job_cancelled, the task_id/SIGUSR1 revoke wiring, and its tests) and dropped the parts of it that only applied to submit_explore_json_job / load_explore_json_into_cache, which this branch already deleted. Also merged master's get_request_json_body() hardening in views/utils.py and its abort-signal wiring in chartAction.ts (adapted to this branch's already-simplified, useLegacyApi-free handleChartDataResponse signature).
…ipeline Resolves a conflict in ExploreChartPanel/index.tsx between master's new standalone-download-control feature and this branch's earlier removal of the raw core Alert import in favor of the ExploreAlert component: kept master's URL_PARAMS/getUrlParam/StandaloneDownloadControl wiring, dropped the reintroduced Alert import since it's unused on this branch.
…ipeline The only real conflict was an import-list collision in Timeseries/transformProps.ts (this branch's DTTM_ALIAS next to master's DataRecordValue); kept both. Country-map's regenerated GeoJSON files and notebook edits from master merged cleanly via rename detection onto this branch's already-renamed plugin-chart-country-map/ package, with no overlapping edits to reconcile.
The merge onto master left two migration heads: this branch's 88360afb61ed (migrate_compare_chart_to_echarts) and master's e7d93a524ff6 (add_purge_audit_log), from parallel migration work landing on both sides. Adds the standard no-op merge revision joining them, generated via `superset db merge` and verified with a clean `superset db upgrade` from an empty database. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Enforce DECK_MULTI_MAX_SLICES client-side before fanning out
per-layer metadata/data requests, mirroring the cap the removed
viz.py pipeline used to enforce server-side.
- Accumulate autozoom features by slice_id (bucketed by viz_type)
instead of overwriting by viz_type, so two layers sharing a
viz_type no longer clobber each other's points.
- ChartClient.loadQueryData now posts the built query context at
the top level of the request body (as /api/v1/chart/data expects)
instead of nesting it under a query_context key, and unwraps the
{result: [...]} response shape correctly.
- migrate_viz downgrade no longer discards a hand-edited
query_context that lacks a "queries" key; the whole context is
backed up and restored verbatim in that case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ngrade Addresses @sadpandajoe's follow-up review comment on #41714: an original query_context with "queries": [] backs up as a falsy-but- present empty list, not None. downgrade_slice's truthiness check (`if queries_bak:`) treated that the same as "no context was ever stored" and set query_context to None, discarding the slice's original datasource and form_data instead of restoring it. Also fixes the same ambiguity at the source: QUERIES_BAK_FIELD_NAME defaulted to {} rather than None when absent from form_data, which could be misread as a real (if malformed) backup rather than "key not present". Changed to the natural None default so "no backup" and "an empty list backup" stay distinguishable, and dedented the query_context assignment that was incorrectly nested inside the "form_data" in query_context check (a context missing "form_data" would restore params/viz_type but silently leave query_context at its upgraded value). Also updates the stale UPDATING.md note for this PR: the percent- re-basing and deck_multi autozoom limitations it warned about were both resolved earlier in this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ipeline Routine rebase; the only conflict was package-lock.json, regenerated via npm install against the already-cleanly-merged package.json. Everything else (package.json, plugin-chart-partition's package.json, superset/views/base.py, superset/views/core.py, tests/integration_tests/security_tests.py) merged automatically.
…complete The lockfile committed while resolving the last rebase conflict against master was missing several real transitive dependencies (preact, @react-spring/*, polished, react-ace, @deck.gl/widgets), which failed npm ci in CI's frontend-build/docker-build with "Missing: X from lock file" errors, cascading into pre-commit/cypress/playwright/docker failures. Root-caused to two compounding issues on the machine that regenerated it: a corrupted local npm cache (npm cache verify found and cleaned up 643 bad entries) and the --legacy-peer-deps flag, which does not auto-install a peer dependency's own transitive deps the way a plain npm install does -- @deck.gl/widgets is itself a peer dependency, and preact is one of its own dependencies, so it was silently dropped. Regenerated cleanly with a plain npm install after clearing the cache; verified node_modules/preact, node_modules/@react-spring/core, and node_modules/@deck.gl/widgets are all present this time.
…ipeline A second, smaller conflict landed while pushing the previous rebase fix: master's Prettier-to-Oxfmt migration touched every plugin package.json, colliding with this branch's own renames/deletions of the legacy-prefixed packages. Resolved: - Two modify/delete conflicts (legacy-plugin-chart-rose, legacy-preset-chart-nvd3): kept deleted. Both packages are fully removed on this branch with no replacement package (Rose now lives inside plugin-chart-echarts/src/Rose/). - Five package.json field-reordering conflicts (chord, country-map, paired-t-test, parallel-coordinates, world-map): master's migration reordered/added fields (keywords, homepage) on the old legacy- prefixed paths while this branch had independently reordered the same renamed files. Reconstructed each to the canonical field order already used by sibling packages that merged cleanly (partition, echarts). - One content conflict in paired-t-test/src/TTestTable.tsx: discarded master's dead legacy reactable-based sortConfig block, already fully replaced by this branch's own COMPARATORS object earlier in the same file. package-lock.json regenerated via a clean npm install (no --legacy-peer-deps, after clearing corrupted npm cache entries) to correctly reflect both the dependency-tree changes from this rebase and the Oxfmt migration's own devDependency swap (prettier/eslint- plugin-prettier out, oxfmt in).
# Conflicts: # superset-frontend/package-lock.json
…merge revision master added a new migration head (f3a8c1d2e9b7) branching from the same revision already folded into 9d744c5dd981, producing two heads again.
… HEAD # Conflicts: # UPDATING.md # superset-frontend/package-lock.json # superset-frontend/package.json # superset-frontend/packages/superset-ui-core/src/chart/clients/ChartClient.ts # superset-frontend/packages/superset-ui-core/src/chart/components/StatefulChart.test.tsx # superset-frontend/playwright/components/dashboard/index.ts # superset-frontend/plugins/legacy-plugin-chart-chord/src/transformProps.ts # superset-frontend/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx # superset-frontend/plugins/legacy-plugin-chart-rose/src/images/example1-dark.jpg # superset-frontend/plugins/legacy-plugin-chart-rose/src/images/example1.jpg # superset-frontend/plugins/legacy-plugin-chart-rose/src/images/example2-dark.jpg # superset-frontend/plugins/legacy-plugin-chart-rose/src/images/example2.jpg # superset-frontend/plugins/legacy-plugin-chart-rose/src/index.ts # superset-frontend/plugins/legacy-plugin-chart-rose/src/transformProps.ts # superset-frontend/plugins/legacy-plugin-chart-world-map/types/external.d.ts # superset-frontend/plugins/legacy-preset-chart-nvd3/src/Bubble/index.ts # superset-frontend/plugins/plugin-chart-calendar/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-calendar/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-calendar/src/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-chord/package.json # superset-frontend/plugins/plugin-chart-chord/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-chord/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-chord/src/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-chord/test/index.test.ts # superset-frontend/plugins/plugin-chart-country-map/package.json # superset-frontend/plugins/plugin-chart-country-map/src/countries/chile.geojson # superset-frontend/plugins/plugin-chart-country-map/src/countries/fiji.geojson # superset-frontend/plugins/plugin-chart-country-map/src/countries/french_polynesia.geojson # superset-frontend/plugins/plugin-chart-country-map/src/countries/philippines_regions.geojson # superset-frontend/plugins/plugin-chart-country-map/src/countries/united_states_minor_outlying_islands.geojson # superset-frontend/plugins/plugin-chart-country-map/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-country-map/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-country-map/src/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/example-dark.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/example.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/thumbnail.png # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/index.ts # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts # superset-frontend/plugins/plugin-chart-echarts/src/Rose/controlPanel.tsx # superset-frontend/plugins/plugin-chart-echarts/src/Rose/images/example1-dark.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Rose/images/example1.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Rose/images/example2-dark.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Rose/images/example2.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Rose/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-echarts/src/Rose/index.ts # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/EchartsTimePivot.tsx # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/example-dark.jpg # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/example.jpg # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/thumbnail.png # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/index.ts # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/transformProps.ts # superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx # superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts # superset-frontend/plugins/plugin-chart-echarts/src/index.ts # superset-frontend/plugins/plugin-chart-echarts/test/TimePivot/transformProps.test.ts # superset-frontend/plugins/plugin-chart-horizon/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-horizon/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-horizon/src/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-paired-t-test/package.json # superset-frontend/plugins/plugin-chart-paired-t-test/src/buildQuery.ts # superset-frontend/plugins/plugin-chart-paired-t-test/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-paired-t-test/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-paired-t-test/test/buildQuery.test.ts # superset-frontend/plugins/plugin-chart-parallel-coordinates/package.json # superset-frontend/plugins/plugin-chart-parallel-coordinates/src/buildQuery.ts # superset-frontend/plugins/plugin-chart-parallel-coordinates/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-parallel-coordinates/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-parallel-coordinates/src/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-partition/src/buildQuery.ts # superset-frontend/plugins/plugin-chart-partition/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-partition/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-partition/src/images/thumbnailLarge.png # superset-frontend/plugins/plugin-chart-rose/CHANGELOG.md # superset-frontend/plugins/plugin-chart-rose/README.md # superset-frontend/plugins/plugin-chart-rose/package.json # superset-frontend/plugins/plugin-chart-rose/src/ReactRose.tsx # superset-frontend/plugins/plugin-chart-rose/src/Rose.ts # superset-frontend/plugins/plugin-chart-rose/src/controlPanel.tsx # superset-frontend/plugins/plugin-chart-rose/src/images/example1-dark.jpg # superset-frontend/plugins/plugin-chart-rose/src/images/example1.jpg # superset-frontend/plugins/plugin-chart-rose/src/images/example2-dark.jpg # superset-frontend/plugins/plugin-chart-rose/src/images/example2.jpg # superset-frontend/plugins/plugin-chart-rose/src/index.ts # superset-frontend/plugins/plugin-chart-rose/src/stories/Rose.stories.tsx # superset-frontend/plugins/plugin-chart-rose/src/stories/data.ts # superset-frontend/plugins/plugin-chart-rose/src/transformProps.ts # superset-frontend/plugins/plugin-chart-rose/tsconfig.json # superset-frontend/plugins/plugin-chart-rose/types/external.d.ts # superset-frontend/plugins/plugin-chart-world-map/package.json # superset-frontend/plugins/plugin-chart-world-map/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-world-map/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-world-map/src/images/thumbnailLarge.png # superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.color.test.tsx # superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.test.tsx # superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx # superset-frontend/plugins/preset-chart-deckgl/src/Multi/buildQuery.ts # superset-frontend/scripts/check-custom-rules.js # superset-frontend/src/components/Chart/chartAction.ts # superset-frontend/src/explore/components/ExploreChartPanel/index.tsx # superset-frontend/src/visualizations/presets/MainPreset.ts # superset-frontend/tsconfig.json # superset/async_events/async_query_manager.py # superset/migrations/versions/2026-07-02_21-00_d4e5f6a7b8c9_migrate_bubble_chart_to_echarts.py # superset/views/utils.py # tests/unit_tests/async_events/async_query_manager_tests.py
# Conflicts: # superset-frontend/plugins/plugin-chart-calendar/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-calendar/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-chord/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-chord/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-country-map/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-country-map/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/example-dark.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/example.jpg # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-echarts/src/Bullet/images/thumbnail.png # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/example-dark.jpg # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/example.jpg # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-echarts/src/TimePivot/images/thumbnail.png # superset-frontend/plugins/plugin-chart-horizon/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-horizon/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-paired-t-test/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-paired-t-test/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-parallel-coordinates/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-parallel-coordinates/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-partition/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-partition/src/images/thumbnail.png # superset-frontend/plugins/plugin-chart-world-map/src/images/thumbnail-dark.png # superset-frontend/plugins/plugin-chart-world-map/src/images/thumbnail.png # superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx # superset-frontend/plugins/preset-chart-deckgl/src/Multi/MultiV1.test.tsx # superset/migrations/shared/migrate_viz/base.py # tests/unit_tests/migrations/viz/upgrade_malformed_query_context_test.py
…4 misc-chart thumbnails Addresses rusackas's own leftover checklist on #42328: - Waterfall: pointed at a new quarterly_sales_delta dataset (Q2 2003 - Q2 2005 quarter-over-quarter sales change) instead of raw quarterly totals, which are always positive and rendered as all-green bars regardless of whether sales actually grew or shrank. The delta metric produces genuine green/red bars. - Tree: added a Territory > Country > City sales hierarchy (41 nodes, 3 levels) built from cleaned_sales_data, replacing the tiny 12-node USA-only example that shipped with this PR. - Horizon: new Population Growth Horizon example (20 most populous countries' annual population growth rate, 1960-2014) replacing the old 7-band/2.5-year example that left most of the thumbnail empty; 20 bands at the default 25px row height now fill the full 512px canvas and show real oscillation (several countries dip negative). - deck.gl Heatmap: fixed a real bug -- color_scheme_type was set to 'default_palette', which isn't a valid value in this plugin's control panel (only 'fixed_color'/'linear_palette' are), so getColorRange() fell through every switch case and the layer silently rendered with an undefined colorRange. Set it to 'linear_palette' with the 'fire' scheme, and raised row_limit from 5,000 to 50,000 (of 261k available rows). Recaptured thumbnails for exactly these 4 viz types (light + dark, deck.gl heatmap dark correctly skipped as canvas/theme-invariant) via capture-viz-thumbnails.spec.ts against a local build with the new examples loaded, and visually confirmed each one before committing.
…dashboard Addresses rusackas's leftover note on #42328: "The handlebars showcase tab idea (keep the logo, add a Misc tab of CSS-heavy handlebars examples)." - Kept the existing Sales Handlebars List example (the Superset logo demo) untouched, per "keep the logo". - Added a new "Sales Handlebars Showcase" chart: a product-line leaderboard rendered as cards, using the plugin's actual CSS-heavy capabilities -- the dedicated "CSS Styles" field for card/badge/gradient-bar styling, the registered `formatNumber` helper, and just-handlebars-helpers' conditional (`gt`, `eq`) and math (`sum`, `division`, `multiplication`) helpers for rank medals (top 3), tier badges (Top Seller/Strong/Growing), and proportional progress bars. The card markup, rank, and badge text are plain text/emoji and always render; the colors/shape/bars come from the CSS field and require an administrator to allow `style` elements via HTML_SANITIZATION_SCHEMA_EXTENSIONS (server-side sanitization strips `<style>` by default) -- same caveat the control's own tooltip warns about, documented in the chart's description. - Restructured misc_charts/dashboard.yaml from a flat grid into two tabs ("Charts" holding all existing content unchanged, "Handlebars Showcase" holding the new chart), matching the TABS/TAB position-JSON schema real Superset-authored tabbed dashboards use (e.g. sales_dashboard). Verified against a local build with the new content loaded: the "Charts" tab still renders every pre-existing chart, and the new tab renders the leaderboard correctly in both light and dark app themes (screenshotted, not just structurally validated) -- caught and fixed one real issue this way, a rank-number color that inherited the app's dark-mode text color and went low-contrast against the card background. Not part of this change: the handlebars plugin's own thumbnail.png gallery art (the crawler has no VIZ_TYPE_THUMBNAILS entry for handlebars by design, to preserve the existing logo art) and the separate "gallery diversification" backlog item from the same PR comment.
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Agent Run #8c488a
Actionable Suggestions - 2
-
superset/examples/deckgl_demo/charts/Deck.gl_Hexagons.yaml - 1
- Viewport values diverge from storybook · Line 49-53
-
superset/examples/misc_charts/charts/Sales_Calendar_Heatmap.yaml - 1
- Duplicate viz_type field · Line 41-47
Additional Suggestions - 7
-
superset/examples/misc_charts/charts/Sales_Handlebars_Showcase.yaml - 2
-
Hardcoded magic number in template · Line 43-43The value `3919616` appears to be a magic number representing a hardcoded maximum. If actual sales data exceeds this value, progress bars will overflow. Consider deriving this from the data dynamically, similar to how `@index` is used for rankings.
-
Duplicate viz_type field · Line 148-148Remove one of the duplicate `viz_type: handlebars` entries to avoid redundancy.
-
-
superset/examples/misc_charts/charts/Population_Partition.yaml - 1
-
Duplicate viz_type field · Line 39-39The `viz_type` field is defined twice: once inside `params` (line 39) and again as a top-level field (line 44). All other chart files in this directory only define `viz_type` at the top level. Remove the duplicate inside params.
-
-
superset/examples/misc_charts/charts/Sales_Pivot_Highlights.yaml - 1
-
Inconsistent operator value format · Line 28-28The `operator: None` uses YAML null; migration file `2021-08-03_15-36_143b6f2815da_migrate_pivot_table_v2_heatmaps_to_new_.py` shows the established pattern uses string `"None"`. Consistency with codebase precedent suggests quoting the value.
-
-
superset/examples/misc_charts/charts/Product_Line_Time_Table.yaml - 1
-
Duplicate viz_type field · Line 72-77The `viz_type: time_table` at line 77 is redundant with line 72. Top-level `viz_type` was deprecated in favor of the one inside `params`. Several other charts in this directory (e.g., `Monthly_Sales_Line.yaml`) follow this pattern with only the params-level field.
-
-
superset/examples/misc_charts/charts/Sales_Handlebars_List.yaml - 1
-
Duplicate field in params · Line 41-41The `viz_type: handlebars` field inside `params` (line 41) is redundant since it's already defined at the top level (line 46). Remove the duplicate to avoid potential confusion about which value takes precedence.
-
-
superset/examples/misc_charts/charts/Quarterly_Sales_Steps.yaml - 1
-
Inconsistent legend visibility setting · Line 34-34The `show_legend` is set to `false` while other sales step charts (e.g., Population_Step_Line) use `true`. This may reduce discoverability for multi-series data unless legend is intentionally suppressed.
-
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
superset/examples/misc_charts/charts/Game_Sales_Radar.yaml - 1
- Duplicate YAML field · Line 63-63
-
superset/examples/misc_charts/datasets/sales_territory_hierarchy.yaml - 1
- Nullable int column type mismatch · Line 34-44
Review Details
-
Files reviewed - 46 · Commit Range:
b5df673..4f77538- superset-frontend/playwright/tests/tools/capture-viz-thumbnails.spec.ts
- superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Line/index.ts
- superset/examples/deckgl_demo/charts/Deck.gl_Hexagons.yaml
- superset/examples/deckgl_demo/charts/Deck.gl_Scatterplot.yaml
- superset/examples/misc_charts/charts/Deck_gl_Contour.yaml
- superset/examples/misc_charts/charts/Deck_gl_Heatmap.yaml
- superset/examples/misc_charts/charts/Game_Sales_Radar.yaml
- superset/examples/misc_charts/charts/Life_Expectancy_Histogram.yaml
- superset/examples/misc_charts/charts/Monthly_Sales_Line.yaml
- superset/examples/misc_charts/charts/Monthly_Sales_Smooth.yaml
- superset/examples/misc_charts/charts/Parallel_Coordinates.yaml
- superset/examples/misc_charts/charts/Point_Cluster_Map.yaml
- superset/examples/misc_charts/charts/Population_Funnel.yaml
- superset/examples/misc_charts/charts/Population_Growth_Horizon.yaml
- superset/examples/misc_charts/charts/Population_Nightingale_Rose.yaml
- superset/examples/misc_charts/charts/Population_Paired_t_Test.yaml
- superset/examples/misc_charts/charts/Population_Partition.yaml
- superset/examples/misc_charts/charts/Population_Percent_Change.yaml
- superset/examples/misc_charts/charts/Population_Smooth_Line.yaml
- superset/examples/misc_charts/charts/Population_Step_Line.yaml
- superset/examples/misc_charts/charts/Population_Sunburst.yaml
- superset/examples/misc_charts/charts/Population_Time_Series.yaml
- superset/examples/misc_charts/charts/Population_Treemap.yaml
- superset/examples/misc_charts/charts/Product_Line_Chord.yaml
- superset/examples/misc_charts/charts/Product_Line_Donut.yaml
- superset/examples/misc_charts/charts/Product_Line_Time_Table.yaml
- superset/examples/misc_charts/charts/Quarterly_Sales_Steps.yaml
- superset/examples/misc_charts/charts/Quarterly_Sales_Waterfall.yaml
- superset/examples/misc_charts/charts/Rural_Population_Gauge.yaml
- superset/examples/misc_charts/charts/Sales_Calendar_Heatmap.yaml
- superset/examples/misc_charts/charts/Sales_Grid_Heatmap.yaml
- superset/examples/misc_charts/charts/Sales_Handlebars_List.yaml
- superset/examples/misc_charts/charts/Sales_Handlebars_Showcase.yaml
- superset/examples/misc_charts/charts/Sales_Mixed_Chart.yaml
- superset/examples/misc_charts/charts/Sales_Period_Pivot.yaml
- superset/examples/misc_charts/charts/Sales_Pivot_Highlights.yaml
- superset/examples/misc_charts/charts/Sales_Stacked_Area.yaml
- superset/examples/misc_charts/charts/Sales_Stacked_Bars.yaml
- superset/examples/misc_charts/charts/Sales_Summary_Table.yaml
- superset/examples/misc_charts/charts/Sales_Territory_Tree.yaml
- superset/examples/misc_charts/charts/Sales_YoY_Big_Number.yaml
- superset/examples/misc_charts/charts/Total_Sales_Bullet.yaml
- superset/examples/misc_charts/dashboard.yaml
- superset/examples/misc_charts/datasets/quarterly_sales_delta.yaml
- superset/examples/misc_charts/datasets/sales_territory_hierarchy.yaml
- superset/migrations/versions/2026-08-06_14-30_aaed1198e21f_merge_databend_sslmode_with_pivot_.py
-
Files skipped - 1
- superset-frontend/package.json - Reason: Filter setting
-
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
Contributor
|
Two things:
The thumbnail tool and example loading otherwise look solid: gated behind CAPTURE_THUMBNAILS so it never runs in CI, charts discovered via API rather than hardcoded IDs, and the UUID-keyed import path with auto-discovered parquets is idempotent on re-run. |
aaed1198e21f duplicated master's own resolution (d7cecc48bd55) of the same two heads (4f145192b583, c4a1b8e2d739); nothing in this branch's chain depends on it, so it can be dropped outright. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
Author
|
@sha174n good catch on both. Dropped aaed1198e21f, |
Contributor
Code Review Agent Run #1ca1ebActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
xAxis.interval was set directly from the xAxisLabelInterval control
('auto'/'0'), but xAxis.interval forces echarts' IntervalScale into a
fixed-tick-spacing mode that expects a number and crashes the axis
"nice" tick calculation. Every bubble_v2 chart hit this at its default
control value; there was no existing test coverage for this file.
The interval belongs nested under axisLabel, where it only controls how
many labels are skipped, matching how every other echarts chart in this
codebase (e.g. Timeseries) wires it. Added a regression test.
Found while building out live thumbnail capture for example dashboards
(this PR) -- bubble_v2 apparently never had a real dashboard-placed
chart to exercise this path before.
deck_geojson had zero chart on any example dashboard, needing an example dataset with a serialized GeoJSON Feature column -- something no existing example data had. Derived a `geojson` column for sf_population_polygons (already used by the deck.gl demo dashboard for deck_polygon/deck_contour) from its existing `contour` point-list column, wrapping each zip code's boundary as a proper GeoJSON Feature. Added the chart to Misc Charts and to the dashboard's cross-filter scope list. Also carries the world_health bubble_v2 migration + time_range fix already sent separately to master (#43152); duplicated here so this branch's own capture tooling and CI have working data independent of that PR's merge timing.
…cing hand-made art with live thumbnails The capture tool discovered ~53 viz types across the example dashboards but only ever captured one representative chart per type into thumbnail.png; every gallery slot beyond that was hand-made art that predated any of this, unrelated to what the example dashboards actually contain. Two of those hand-made images turned out to be faked outright: ag-grid-table's and cartodiagram's "dark" variants were a naive full-image color inversion of the light one rather than a real dark-theme render (confirmed by pixel-diffing every light/dark pair in the repo -- these two matched a naive invert at 85-99%, every genuine capture matched at under 2%). To fix this without silently losing the old art or shipping anything dishonest: 1. Every existing thumbnail/gallery image was renamed with a custom_thumb_ prefix (plugin imports updated to match), so it's preserved and clearly labeled as hand-made rather than captured. 2. The capture script (capture-viz-thumbnails.spec.ts) was generalized: it now captures every distinct chart per viz type, not just one -- the preferred (or alphabetically-first) chart becomes the picker thumbnail, and every other distinct chart fills the next already-declared exampleGallery slot, via a new VIZ_TYPE_GALLERY map mirroring VIZ_TYPE_THUMBNAILS. Also fixed a pre-existing bug where VIZ_TYPE_THUMBNAILS.rose pointed at plugin-chart-rose, a package consolidated into plugin-chart-echarts/src/Rose long ago. 3. Ran it for real against a live instance with examples loaded (including the new Deck.gl GeoJson example). Every plugin whose real capture succeeded had its import flipped from the custom_thumb_ file back to the plain, freshly-captured one. 4. For deck.gl chart types, the dark "capture" is confirmed genuinely pixel-identical to light (they render static map tiles that don't respond to prefers-color-scheme) rather than skipped/left stale, so the plain dark file is an honest copy of the real light capture instead of mismatched old art. Four viz types still have zero chart on any example dashboard and keep their custom_thumb_ imports as an honest label rather than a graduated (but fake) real capture: - cartodiagram: needs a hand-crafted embedded sub-chart config - deck_multi: sub-layers are referenced by raw DB-assigned chart ids with no UUID-based remap path through the YAML importer -- a real gap needing an importer/exporter fix, not something a fixture alone can solve - pop_kpi, ag-grid-table: gated behind ChartPluginsExperimental / AgGridTableEnabled and unregistered by default, so a default-visible dashboard chart would just show every regular user a broken card
Member
Author
|
Update: the capture tool now grabs every distinct chart per viz type instead of one representative, so gallery slots with multiple examples show real variety. Ran it for real against a live instance this time, which surfaced a couple of things worth calling out. First, digging into coverage turned up two of the existing "dark" thumbnails that were flat-out faked: Second, running this live against real dashboards found two actual bugs, unrelated to any of this thumbnail work but only surfaced by it: Bubble's transformProps had On the inventory you asked about, four viz types still have zero chart on any example dashboard:
|
# Conflicts: # superset/examples/world_health/charts/Life_Expectancy_VS_Rural.yaml
Sales Territory Tree Horizontal reuses the same sales_territory_hierarchy dataset as the existing radial Sales Territory Tree example, but with layout: orthogonal / orient: LR so the gallery shows tree_chart's other common orientation instead of two renders of the same radial chart.
Life Expectancy Histogram had the same bug as the earlier Bubble chart fix: its adhoc filter pointed at 2014, which has zero SP_DYN_LE00_IN data in the world_health dataset, so the thumbnail showed an empty chart. Moved it to 2011, which has real data. Deck.gl GeoJson rendered as a single solid-colored blob: the global fill_color_picker (alpha=1) always overrides any per-feature fill color, and line_width defaulted to ~1 meter, invisible at city zoom, so none of the 25 zip-code boundaries were visible. Baked a population-ranked choropleth fill-color into each zip code's GeoJSON Feature, set fill_color_picker's alpha to 0 so the per-feature colors show through, switched the stroke to a visible white pixel-width line, and turned off the extruded flag since there's no per-feature elevation data backing it.
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
4 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
Two additions in support of the legacy viz pipeline removal (#41714):
1. Example charts for every viz type that had none — plus purpose-built thumbnail examples. The Misc Charts example dashboard grows to 38 charts: examples for every migrated legacy visualization (calendar heatmap, nightingale rose, partition, paired t-test, bullet, time-series period pivot, percent-change line, time table), every previously-unexemplified type (generic/smooth/stepped time series, handlebars, deck.gl contour and heatmap, point cluster map, GeoJSON), and a set of charts tuned specifically to make good gallery thumbnails (simpler chord, YoY big number, triangular funnel, multi-range gauge, compact heatmap grid, bell-curve histogram, dual-axis mixed chart, labeled donut, video-game radar, sunburst with total, stacked area/bars, marker lines, high-cardinality treemap, quarterly waterfall, multi-metric table, cell-highlighted pivot). Almost all on existing example datasets; the Waterfall, Tree, and GeoJson examples do add small new data (
quarterly_sales_delta,sales_territory_hierarchyparquet + dataset YAMLs, and ageojsoncolumn derived from the deck.gl demo's existing polygon data) since nothing in the existing data had a delta/hierarchy/GeoJSON shape to show off those chart types.2. A Playwright thumbnail-capture crawler (
npm run playwright:thumbnails, gated behindCAPTURE_THUMBNAILS=1). It discovers every chart on every dashboard via the API and captures every distinct chart per viz type — not just one representative — at 512×512 for the picker thumbnail, filling every already-declaredexampleGalleryslot with additional distinct charts so multi-image galleries show real variety instead of the same chart resized (VIZ_TYPE_GALLERY, mirroring the existingVIZ_TYPE_THUMBNAILSmap). Every existing thumbnail/gallery image predating this crawler was renamed with acustom_thumb_prefix (plugin imports updated to match) so it's clearly labeled as hand-made art rather than a live capture; two of those turned out to be outright faked (ag-grid-tableandcartodiagram's "dark" variants were a naive full-image color inversion of the light one, not a real dark-theme render — confirmed by pixel-diffing every light/dark pair in the repo). Every plugin whose real capture succeeded had its import flipped from thecustom_thumb_file back to the plain, freshly-captured one; for deck.gl chart types (static map tiles that don't respond toprefers-color-scheme), the dark file is an honest copy of the real light capture rather than mismatched old art.Also fixed two real bugs surfaced by actually running this against a live instance:
Bubble/transformProps.tssetxAxis.intervalat the wrong config level, crashing echarts' axis tick calculation for every bubble_v2 chart at its default control value (zero prior test coverage — added a regression test).VIZ_TYPE_THUMBNAILS.rosepointed at a package (plugin-chart-rose) consolidated intoplugin-chart-echarts/src/Roselong ago.A third bug (the
world_healthbubble_v2 example itself was still on the long-removed legacybubbleviz_type, and itstime_rangepointed at a year with no life-expectancy data) was found and fixed independently onmasteras #43152, and is duplicated here so this branch's own tooling and CI aren't blocked on that PR's merge timing.FOLLOW-UPS (deliberately not covered here)
Viz types still with zero chart on any example dashboard:
pop_kpiandag-grid-tableare feature-flag-gated (ChartPluginsExperimental/AgGridTableEnabled); their plugins are unregistered on default deployments, where example charts would render as errors. Revisit if/when the flags graduate.deck_multistores sub-layers as raw DB-assigned chart ids indeck_slices, which the YAML importer can't remap since there's no UUID-based reference path; needs an importer/exporter feature, not just a fixture.cartodiagramembeds a serialized sub-chart config (selected_chart); needs careful hand-crafting.Thumbnail polish that needs better data or ideas:
custom_thumb_art because only 1-2 distinct dashboard charts of that viz type exist to fill them — adding more variant examples would let the crawler fill these with real captures.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
The refreshed thumbnails are the diff — best reviewed in the Files view image previews.
TESTING INSTRUCTIONS
superset load-exampleson this branch; open the Misc Charts and World Bank's Data dashboards and confirm the charts render (including the new Deck.gl GeoJson and the fixed bubble chart).npm run playwright:thumbnailsagainst the running instance and confirm plugin thumbnails regenerate.ADDITIONAL INFORMATION
🤖 Generated with Claude Code