superset icon indicating copy to clipboard operation
superset copied to clipboard

fix(viz): display offset metrics as dashed lines regardless of compar…

Open anumukul opened this issue 6 days ago • 7 comments

SUMMARY

Fixes #36806

This PR fixes a regression where offset metrics (time comparisons like "1 year ago") were not displayed as dashed lines when charts have no dimensions.

Root Cause: The isDerivedSeries function only applied dashed styling when comparison_type === "Values", and hasTimeOffset didn't detect series where the name exactly matched a time offset (which happens when there are no dimensions).

Changes:

  • Removed the restrictive comparison_type === "Values" check in isDerivedSeries.ts
  • Added exact match handling in timeOffset.ts for dimension-less charts
  • Updated tests to reflect new behavior

BEFORE/AFTER

BEFORE image

**AFTER **

image

TESTING INSTRUCTIONS

  1. Create a new Line Chart
  2. Select a dataset with a time column (e.g., birth_names with ds)
  3. Add a metric (e.g., COUNT(*))
  4. Set Time Range to a valid range (e.g., 2005-01-01 to 2008-12-31)
  5. In Advanced Analytics, add Time Comparison: 1 year ago
  6. Do NOT add any dimensions (leave Dimensions empty)
  7. Click "Update Chart"
  8. Verify the offset metric line (1 year ago) appears dashed, not solid

ADDITIONAL INFORMATION

  • [x] Has associated issue: Fixes #36806
  • [ ] Required feature flags: None
  • [x] Changes UI
  • [ ] Includes DB Migration
    • [ ] 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

anumukul avatar Jan 06 '26 10:01 anumukul

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X · Reddit · LinkedIn

Deploy Preview for superset-docs-preview canceled.

Name Link
Latest commit 845701eefc75961a33be499c6ab77bbec603ee02
Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/695cea981190d60008f288ce

netlify[bot] avatar Jan 06 '26 10:01 netlify[bot]

Code Review Agent Run #ff9a91

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 845701e..845701e
    • superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
    • superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts
    • superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/isDerivedSeries.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ 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 [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

bito-code-review[bot] avatar Jan 06 '26 10:01 bito-code-review[bot]

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • [ ] Behavioral change
    The removed guard that required comparison_type === ComparisonType.Values means any chart with a time_compare entry may now mark series as derived. Verify this doesn't enable dashed styling for other comparison types unintentionally. Confirm desired behavior across all supported comparison_type values.

  • [ ] getTimeOffset guard
    The exported helper getTimeOffset assumes series.name is a string (it calls .includes), but it has no internal type guard. While hasTimeOffset now checks the type before calling it, other call sites could call getTimeOffset directly and trigger a runtime error if series.name is undefined or non-string. Consider making getTimeOffset defensive about series.name or narrow its parameter type.

  • [ ] Behavior change
    Removing the restriction that only ComparisonType.Values marks a derived series means series whose names match time offsets will now be treated as derived regardless of the configured comparison_type. This can change visual styling (dashed vs solid lines) in other comparison modes; verify this is intended across chart types and UI surface.

  • [ ] Matching robustness
    The new exact-match check (timeCompare.includes(series.name)) in hasTimeOffset fixes a regression but can still miss matches due to leading/trailing whitespace or minor formatting differences. Also, logic is split between getTimeOffset (pattern matches) and hasTimeOffset (exact match), which may lead to maintenance churn. Consider centralizing and normalizing comparisons (trim/case) and consolidating matching logic.

  • [ ] Viz type clarity
    The shared formData uses VizType.Table. The regression being fixed relates to charts with no dimensions (e.g. line charts); confirm tests reflect the intended chart type or add explicit tests for relevant viz types so behavior for dimension-less charts is validated.

CodeAnt AI finished reviewing your PR.

CodeAnt AI is running Incremental review


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X · Reddit · LinkedIn

Code Review Agent Run #e4959d

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 845701e..94997dd
    • superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
    • superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts
    • superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/isDerivedSeries.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ 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 [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

bito-code-review[bot] avatar Jan 07 '26 12:01 bito-code-review[bot]