feat(themes): add enhanced validation and error handling with fallback mechanisms #35349

rebenitez1802 · 2025-09-30T23:01:09Z

SUMMARY

Implements enhanced theme validation and error handling for Superset's theme system with comprehensive fallback mechanisms. This PR introduces:

Enhanced theme token validation with detailed error reporting for invalid theme tokens
error recovery with fallback to system default themes when theme application fails
Feature flag controlled validation (ENHANCED_THEME_VALIDATION) to enable/disable advanced validation
Partial theme loading that filters out invalid tokens while preserving valid ones
Improved ThemeModal with real-time validation feedback and user guidance

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

To test live token validation:

Enable enhanced validation:
- Set ENHANCED_THEME_VALIDATION: True in feature flags

ADDITIONAL INFORMATION

Has associated issue:
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

korbit-ai · 2025-09-30T23:01:15Z

Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment /korbit-review.

Your admin can change your review schedule in the Korbit Console

codecov · 2025-09-30T23:17:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.86%. Comparing base (412587a) to head (007fa5d).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #35349       +/-   ##
===========================================
+ Coverage        0   71.86%   +71.86%     
===========================================
  Files           0      589      +589     
  Lines           0    43607    +43607     
  Branches        0     4719     +4719     
===========================================
+ Hits            0    31336    +31336     
- Misses          0    11032    +11032     
- Partials        0     1239     +1239

Flag	Coverage Δ
hive	`46.27% <ø> (?)`
mysql	`70.89% <ø> (?)`
postgres	`70.94% <ø> (?)`
presto	`49.97% <ø> (?)`
python	`71.82% <ø> (?)`
sqlite	`70.54% <ø> (?)`
unit	`100.00% <ø> (?)`

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

☔ View full report in Codecov by Sentry.
📢 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.

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Enhanced theme validation disabled by default ▹ view	🧠 Not in standard
	Repeated Default Object Structure ▹ view	🧠 Not in standard
	Duplicated Feature Flag Logic ▹ view	🧠 Not in standard
	Validation results lost on JSON errors ▹ view	🧠 Not in scope
	Inefficient Set recreation in color validation ▹ view	🧠 Incorrect
	Sequential API calls in theme fallback logic ▹ view
	Unnecessary validation hook execution ▹ view	🧠 Incorrect
	Repeated JSON parsing without memoization ▹ view	🧠 Incorrect
	Inefficient regex pattern creation ▹ view	🧠 Not in standard
	Validation Logic Not Separated from State Management ▹ view	🧠 Not in standard

Files scanned

File Path	Reviewed
superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts	✅
superset-frontend/src/theme/hooks/useThemeValidation.ts	✅
superset-frontend/src/features/themes/ThemeModal.tsx	✅
superset-frontend/src/theme/utils/themeTokenValidation.ts	✅
superset-frontend/src/theme/ThemeController.ts	✅
superset/config.py	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset/config.py

    "EMBEDDABLE_CHARTS": True,
+    # Enhanced theme validation in theme editor with real-time token validation
+    # and partial theme loading for invalid tokens
+    "ENHANCED_THEME_VALIDATION": False,


superset-frontend/src/features/themes/ThemeModal.tsx

+  const isEmptyTheme = (jsonData?: string) => {
+    if (!jsonData?.trim()) return true;
+    try {
+      const parsed = JSON.parse(jsonData);
+      // Check if it's an empty object or array
+      return Object.keys(parsed).length === 0;
+    } catch (e) {
+      return false; // If it's not valid JSON, let other validation handle it
+    }
+  };


superset-frontend/src/features/themes/ThemeModal.tsx

+  const themeValidation = useThemeValidation(currentTheme?.json_data || '', {
+    enabled: !isReadOnly && Boolean(currentTheme?.json_data),
+    themeName: currentTheme?.theme_name || 'Unknown Theme',
+  });


superset-frontend/src/features/themes/ThemeModal.tsx

  const validate = () => {
-    if (isReadOnly) {
+    if (isReadOnly || !currentTheme) {
      setDisableSave(true);
      return;
    }

-    if (
-      currentTheme?.theme_name.length &&
-      currentTheme?.json_data?.length &&
-      isValidJson(currentTheme.json_data)
-    ) {
-      setDisableSave(false);
+    const hasValidName = Boolean(currentTheme?.theme_name?.length);
+    const hasValidJsonData = Boolean(currentTheme?.json_data?.length);
+    const isValidJsonSyntax = isValidJson(currentTheme?.json_data);
+    const isNotEmpty = !isEmptyTheme(currentTheme?.json_data);
+
+    // Basic validation requirements
+    const basicValidation =
+      hasValidName && hasValidJsonData && isValidJsonSyntax && isNotEmpty;
+
+    if (isEnhancedValidationEnabled && currentTheme) {
+      // Enhanced validation: allow saving even with token warnings, but block on JSON syntax errors
+      const enhancedValidation = basicValidation && !themeValidation.hasErrors;
+      setDisableSave(!enhancedValidation);
    } else {
-      setDisableSave(true);
+      // Original validation logic
+      setDisableSave(!basicValidation);
    }
  };


superset-frontend/src/theme/hooks/useThemeValidation.ts

+  const enhancedValidation = useMemo(() => {
+    // Skip if basic validation is disabled or JSON has syntax errors
+    if (!enabled || jsonAnnotations.length > 0 || !debouncedJsonValue?.trim()) {
+      return {
+        annotations: [],
+        validTokenCount: 0,
+        invalidTokenCount: 0,
+        errorMessages: [],
+      };
+    }
+
+    // Only run enhanced validation if feature flag is enabled
+    try {
+      const isEnabled = isFeatureEnabled(FeatureFlag.EnhancedThemeValidation);
+      if (!isEnabled) {
+        return {
+          annotations: [],
+          validTokenCount: 0,
+          invalidTokenCount: 0,
+          errorMessages: [],
+        };
+      }
+    } catch (error) {
+      // Feature flag check failed - assume disabled
+      return {
+        annotations: [],
+        validTokenCount: 0,
+        invalidTokenCount: 0,
+        errorMessages: [],
+      };
+    }


superset-frontend/src/theme/hooks/useThemeValidation.ts

+export function useIsEnhancedValidationEnabled(): boolean {
+  return useMemo(() => {
+    try {
+      return isFeatureEnabled(FeatureFlag.EnhancedThemeValidation);
+    } catch (error) {
+      // Feature flag check failed - assume disabled
+      return false;
+    }
+  }, []);
+}


superset-frontend/src/theme/hooks/useThemeValidation.ts

+    if (!enabled || jsonAnnotations.length > 0 || !debouncedJsonValue?.trim()) {
+      return {
+        annotations: [],
+        validTokenCount: 0,
+        invalidTokenCount: 0,
+        errorMessages: [],
+      };
+    }


superset-frontend/src/theme/hooks/useThemeValidation.ts

+    const propertyPattern = new RegExp(
+      `"${tokenName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}"\\s*:`,
+    );


superset-frontend/src/theme/utils/themeTokenValidation.ts

+  const namedColors = new Set([
+    'transparent',
+    'inherit',
+    'currentColor',
+    'initial',
+    'unset',
+    'black',
+    'white',
+    'red',
+    'green',
+    'blue',
+    'yellow',
+    'cyan',
+    'magenta',
+    'gray',
+    'grey',
+    'darkgray',
+    'darkgrey',
+    'lightgray',
+    'lightgrey',
+    'orange',
+    'purple',
+    'brown',
+    'pink',
+    'lime',
+    'navy',
+    'teal',
+    'silver',
+    'maroon',
+    'olive',
+    'aqua',
+    'fuchsia',
+  ]);


korbit-ai · 2025-10-01T01:58:55Z

superset-frontend/src/theme/ThemeController.ts

+      // Try to fetch theme marked as system default (is_system_default=true)
+      const defaultResponse = await fetch(
+        '/api/v1/theme/?q=(filters:!((col:is_system_default,opr:eq,value:!t)))',
+      );
+      if (defaultResponse.ok) {
+        const data = await defaultResponse.json();
+        if (data.result?.length > 0) {
+          const themeConfig = JSON.parse(data.result[0].json_data);
+          if (themeConfig && typeof themeConfig === 'object') {
+            return themeConfig;
+          }
+        }
+      }
+
+      // Fallback: Try to fetch system theme named 'THEME_DEFAULT'
+      const fallbackResponse = await fetch(
+        '/api/v1/theme/?q=(filters:!((col:theme_name,opr:eq,value:THEME_DEFAULT),(col:is_system,opr:eq,value:!t)))',
+      );


Sequential API calls in theme fallback logic

Tell me more

What is the issue?

The fallback mechanism makes two sequential API calls even when the first call succeeds but returns an empty result set, creating unnecessary network overhead.

Why this matters

When the first API call returns successfully but with no matching themes, the code still makes a second API call. This doubles the network latency and server load during theme recovery scenarios, which are already error conditions that should be handled efficiently.

Suggested change ∙ Feature Preview

Combine both queries into a single API call using OR logic, or add an early return when the first call indicates no themes exist:

// Single API call with OR condition for both system default and named fallback const response = await fetch( '/api/v1/theme/?q=(filters:!((col:is_system_default,opr:eq,value:!t),(col:theme_name,opr:eq,value:THEME_DEFAULT)))', ); if (response.ok) { const data = await response.json(); if (data.result?.length > 0) { // Prioritize system default if available const systemDefault = data.result.find(theme => theme.is_system_default); const theme = systemDefault || data.result[0]; const themeConfig = JSON.parse(theme.json_data); if (themeConfig && typeof themeConfig === 'object') { return themeConfig; } } }

Provide feedback to improve future suggestions

_{💬 Looking for more details? Reply to this comment to chat with Korbit.}

rusackas · 2025-10-01T17:24:18Z

Do we really need the feature flag? I think everyone is in favor of more validation :) Seems like it should be default behavior.

rebenitez1802 · 2025-10-02T14:26:08Z

Do we really need the feature flag? I think everyone is in favor of more validation :) Seems like it should be default behavior.

My thought process was because is not a small iteration and also the rules could require a little more refinement it was safer to put it behind a FF but if everyone agrees I can remove it.

gabotorresruiz · 2025-10-13T22:49:22Z

I really appreciate the effort to improve theme validation, but I’m concerned about the long-term maintainability of this approach. Here are my thoughts:

Ant Design silently ignores invalid tokens by design. It's permissive and forward compatible. This means:

Token typos --> ignored by AntD, defaults are used
"#invalid" values --> ignored by AntD, defaults are used
Unknown tokens from future versions --> continue to work without validation updates

By introducing custom validators (useThemeValidation.ts, themeTokenValidation.ts), we're essentially:

Reimplementing Ant Design's token system within our own validation layer
Adding maintenance overhead. Every Ant Design update would require updating our validators
Risking incompatibility. Valid future tokens could be rejected by our outdated validation

So I think we should keep it simple and only have structural validations:

JSON syntax validation
Empty theme blocking
Algorithm type validation ("dark" | "light" | "system")
Object type validation (token, components)

So the ENHANCED_THEME_VALIDATION feature flag, even though it’s optional, I think that if we make an structural validation, we might not need it anymore.

To sum up, I think we should just simplify to:

Basic structural validation (syntax, type, emptiness)
Runtime fallback if theme loading fails
User notifications for load failures

This gives users clear feedback while avoiding the complexity of duplicating Ant Design’s validation logic.

What do you think? Happy to discuss trade-offs!

pull-request-size bot added the size/XXL label Sep 30, 2025

github-actions bot added the packages label Sep 30, 2025

rebenitez1802 force-pushed the feat/theme-error-handling branch from 95089e9 to a66aea1 Compare September 30, 2025 23:10

rebenitez1802 marked this pull request as ready for review October 1, 2025 01:56

korbit-ai bot reviewed Oct 1, 2025

View reviewed changes

dosubot bot added change:frontend Requires changing the frontend global:theming Related to theming Superset labels Oct 1, 2025

rusackas requested review from michael-s-molina and msyavuz October 1, 2025 17:23

rebenitez1802 added 5 commits October 10, 2025 13:15

[WIP] Theme fallback implementation

365b793

added token validation for themes, added feature flag for this.

4ca6b4f

message in alert changed

f299e53

TypeScript error and test refactor

7289ba4

fix fallback

61323a0

rebenitez1802 force-pushed the feat/theme-error-handling branch from 4204948 to 61323a0 Compare October 10, 2025 16:19

geido added the hold:testing! On hold for testing label Oct 13, 2025

fix tests

007fa5d

feat(themes): add enhanced validation and error handling with fallback mechanisms #35349

Are you sure you want to change the base?

feat(themes): add enhanced validation and error handling with fallback mechanisms #35349

Uh oh!

Conversation

rebenitez1802 commented Sep 30, 2025

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Sep 30, 2025

Uh oh!

codecov bot commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

korbit-ai bot Oct 1, 2025

Choose a reason for hiding this comment

Sequential API calls in theme fallback logic

What is the issue?

Why this matters

Suggested change ∙ Feature Preview

Provide feedback to improve future suggestions

Uh oh!

rusackas commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rebenitez1802 commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gabotorresruiz commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Sep 30, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

rusackas commented Oct 1, 2025 •

edited

Loading

rebenitez1802 commented Oct 2, 2025 •

edited

Loading