fix(box-plot): distribute across field not marked as required for datasets without a temporal column by vhogberg · Pull Request #43087 · apache/superset

vhogberg · 2026-08-12T09:55:38Z

SUMMARY

Distribute across is validated with validateNonEmpty, but initialValue returned
[undefined] when the dataset has no temporal column. An array of length 1 passes
validateNonEmpty, so the control was never flagged as required and running the chart
failed with an unclear query error instead. Now an empty array is returned in that case.

const { defaultTemporalColumn } = getTemporalColumns(state.datasource);
return defaultTemporalColumn ? [defaultTemporalColumn] : [];

Added a test in a new controlPanel.test.ts file.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

->

(note that I have renamed the field to "unique identifier" in my fork, not included in the PR)

TESTING INSTRUCTIONS

Create a Box Plot on a dataset with no temporal column -> Distribute across is
marked required immediately now.
Fill in a metric and run -> works.
Repeat on a dataset with a default datetime column -> still auto-populated as before.

ADDITIONAL INFORMATION

Has associated issue: fixes Box Plot: "Distribute across" is not flagged as required when the dataset has no default temporal column #43086
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

…asets without a temporal column

bito-code-review · 2026-08-12T09:55:49Z

Code Review Agent Run #2a8abb

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: ca981ef..ca981ef
- superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts
- superset-frontend/plugins/plugin-chart-echarts/test/BoxPlot/controlPanel.test.ts
Files skipped - 0
Tools
- 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

codeant-ai-for-open-source · 2026-08-12T10:00:54Z

+          const { defaultTemporalColumn } = getTemporalColumns(
+            state.datasource,
+          );
+          return defaultTemporalColumn ? [defaultTemporalColumn] : [];


Suggestion: The helper returns main_dttm_col without verifying that it still exists in the datasource's temporal columns. Datasources can retain a stale main_dttm_col after that column is changed to non-temporal or removed, so this code will populate columns with an invalid distribution column and allow the query to proceed with an invalid field instead of leaving the control empty or selecting a valid temporal column. Ensure defaultTemporalColumn is present in the returned temporal-column list before using it. [api mismatch]

Severity Level: Major ⚠️

- ❌ Box Plot queries fail with stale column metadata. - ⚠️ Explore auto-selects unavailable distribution columns. - ⚠️ Dataset edits can leave main temporal metadata inconsistent.

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts **Line:** 198:201 **Comment:** *Api Mismatch: The helper returns `main_dttm_col` without verifying that it still exists in the datasource's temporal columns. Datasources can retain a stale `main_dttm_col` after that column is changed to non-temporal or removed, so this code will populate `columns` with an invalid distribution column and allow the query to proceed with an invalid field instead of leaving the control empty or selecting a valid temporal column. Ensure `defaultTemporalColumn` is present in the returned temporal-column list before using it. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

This seems like pre-existing behavior of the getTemporalColumns helper, not introduced by this PR, and it affects every caller of the helper. In my opinion it's out of scope for this PR and should be fixed separately if needed in getTemporalColumns itself, so that it's fixed globally.

✅ Customized review instruction saved!

Instruction:

When a finding originates from shared helper behavior that predates the change, report it against the helper for a global fix rather than flagging individual callers.

Applied to:

**/getTemporalColumns*

💡 To manage or update this instruction, visit: CodeAnt AI Settings

bito-code-review · 2026-08-12T10:01:05Z

The flagged issue is correct. The current implementation assumes main_dttm_col is always a valid temporal column, which can lead to invalid state if the datasource metadata is stale. The proposed fix is to verify that defaultTemporalColumn exists before returning it, which is already implemented in the provided PR diff:

          const { defaultTemporalColumn } = getTemporalColumns(
            state.datasource,
          );
          return defaultTemporalColumn ? [defaultTemporalColumn] : [];

This change ensures that if no valid temporal column is found, the control remains empty instead of returning an invalid field. There are no other comments on this PR to address.

superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts

const { defaultTemporalColumn } = getTemporalColumns(
            state.datasource,
          );
          return defaultTemporalColumn ? [defaultTemporalColumn] : [];

codecov · 2026-08-12T10:11:29Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.64%. Comparing base (8f6587d) to head (ca981ef).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #43087   +/-   ##
=======================================
  Coverage   66.64%   66.64%           
=======================================
  Files        2863     2863           
  Lines      162112   162113    +1     
  Branches    37384    37385    +1     
=======================================
+ Hits       108033   108040    +7     
+ Misses      52021    52015    -6     
  Partials     2058     2058

Flag	Coverage Δ
javascript	`73.65% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

rusackas

Matches the repo, has solid tests, CI is green aside from some flakiness (I'll try to fix). LGTM

fix(box-plot): distribute across field not marked as required for dat…

ca981ef

…asets without a temporal column

pull-request-size Bot added the size/M label Aug 12, 2026

dosubot Bot added change:frontend Requires changing the frontend viz:charts:boxplot Related to the Boxplot chart labels Aug 12, 2026

github-actions Bot added the plugins label Aug 12, 2026

codeant-ai-for-open-source Bot reviewed Aug 12, 2026

View reviewed changes

rusackas approved these changes Aug 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(box-plot): distribute across field not marked as required for datasets without a temporal column - #43087

fix(box-plot): distribute across field not marked as required for datasets without a temporal column#43087
vhogberg wants to merge 1 commit into
apache:masterfrom
vhogberg:fix/box-plot-required-field

vhogberg commented Aug 12, 2026 •

edited

Loading

Uh oh!

bito-code-review Bot commented Aug 12, 2026 •

edited

Loading

Code Review Agent Run #2a8abb

Uh oh!

codeant-ai-for-open-source Bot Aug 12, 2026

Uh oh!

vhogberg Aug 12, 2026

Uh oh!

codeant-ai-for-open-source Bot Aug 12, 2026

Uh oh!

bito-code-review Bot commented Aug 12, 2026

Uh oh!

codecov Bot commented Aug 12, 2026

Uh oh!

rusackas left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vhogberg commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #2a8abb

Uh oh!

codeant-ai-for-open-source Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

vhogberg Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

bito-code-review Bot commented Aug 12, 2026

Uh oh!

codecov Bot commented Aug 12, 2026

Codecov Report

Uh oh!

rusackas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vhogberg commented Aug 12, 2026 •

edited

Loading

bito-code-review Bot commented Aug 12, 2026 •

edited

Loading