fix(playwright): address review feedback on de-flake change

Three follow-ups from /review-code on the list-spec de-flake work:

- Revert toast assertions to expect(toast.getSuccess()).toBeVisible().
  The previous switch to toast.getSuccess().waitFor({ state: 'visible' })
  was a no-op: both APIs poll for visibility, so neither catches a fast
  auto-dismiss between polls. The expect() form fails as an assertion
  (clearer diff, counted in reports). Misleading "use waitFor so we
  detect auto-dismiss" comments are gone too.

- Drop redundant waitFor({ state: 'visible' }) calls in BulkSelect's
  selectRow / deselectRow / clickAction. Locator.check()/click() already
  auto-wait for visibility, stability, and enabled state. The
  expect(checkbox).toBeChecked() assertion -- which is the actual race
  guard against React state propagation -- stays.

- Expand the DeleteConfirmationModal.clickDelete docstring to explain
  why it bypasses Modal.clickFooterButton: the footer button label is
  i18n'd, so name-based lookups break in non-English locales.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.github/workflows/claude.yml

@@ -17,12 +17,13 @@ jobs:
     steps:
       - name: Check if user is allowed
         id: check
-        env:
-          COMMENTER: ${{ github.event.comment.user.login }}
         run: |
           # List of allowed users
           ALLOWED_USERS="mistercrunch,rusackas"
 
+          # Get the commenter's username
+          COMMENTER="${{ github.event.comment.user.login }}"
+
           echo "Checking permissions for user: $COMMENTER"
 
           # Check if user is in allowed list
@@ -44,12 +45,9 @@ jobs:
     steps:
       - name: Comment access denied
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
-        env:
-          COMMENTER_LOGIN: ${{ github.event.comment.user.login || github.event.review.user.login || github.event.issue.user.login }}
         with:
           script: |
-            const commenter = process.env.COMMENTER_LOGIN;
-            const message = `👋 Hi @${commenter}!
+            const message = `👋 Hi @${{ github.event.comment.user.login || github.event.review.user.login || github.event.issue.user.login }}!
 
             Thanks for trying to use Claude Code, but currently only certain team members have access to this feature.
 

.github/workflows/codeql-analysis.yml

@@ -41,7 +41,7 @@ jobs:
 
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4
+        uses: github/codeql-action/init@v4
         with:
           languages: ${{ matrix.language }}
           # If you wish to specify custom queries, you can do so here or in a config file.
@@ -53,6 +53,6 @@ jobs:
 
       - name: Perform CodeQL Analysis
         if: steps.check.outputs.python || steps.check.outputs.frontend
-        uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4
+        uses: github/codeql-action/analyze@v4
         with:
           category: "/language:${{matrix.language}}"

superset-frontend/packages/superset-ui-core/src/chart/components/createLoadableRenderer.ts

@@ -17,109 +17,59 @@
  * under the License.
  */
 
-import { ReactElement, useEffect, useRef, useState } from 'react';
+import Loadable from 'react-loadable';
+import { ComponentClass } from 'react';
 
 export type LoadableRendererProps = {
-  onRenderFailure?: (error: Error) => void;
-  onRenderSuccess?: () => void;
+  onRenderFailure?: Function;
+  onRenderSuccess?: Function;
 };
 
-type LoaderMap<Exports> = {
-  [K in keyof Exports]: () => Promise<Exports[K]> | Exports[K];
+const defaultProps = {
+  onRenderFailure() {},
+  onRenderSuccess() {},
 };
 
-export interface LoadingProps {
-  error?: { toString(): string };
-}
+export interface LoadableRenderer<Props>
+  extends
+    ComponentClass<Props & LoadableRendererProps>,
+    Loadable.LoadableComponent {}
 
-export interface LoadableOptions<Props, Exports> {
-  loader: LoaderMap<Exports>;
-  loading: (loadingProps: LoadingProps) => ReactElement | null;
-  render: (loaded: Exports, props: Props) => ReactElement;
-}
+export default function createLoadableRenderer<
+  Props,
+  Exports extends { [key: string]: any },
+>(options: Loadable.OptionsWithMap<Props, Exports>): LoadableRenderer<Props> {
+  const LoadableRenderer = Loadable.Map<Props, Exports>(
+    options,
+  ) as LoadableRenderer<Props>;
 
-export interface LoadableRenderer<Props> {
-  (props: Props & LoadableRendererProps): ReactElement | null;
-  preload: () => Promise<unknown>;
-  displayName?: string;
-}
+  // Extends the behavior of LoadableComponent to provide post-render listeners
+  class CustomLoadableRenderer extends LoadableRenderer {
+    static defaultProps: object;
 
-export default function createLoadableRenderer<Props, Exports>(
-  options: LoadableOptions<Props, Exports>,
-): LoadableRenderer<Props> {
-  let promise: Promise<Exports> | null = null;
-  let cachedResult: Exports | null = null;
-  let cachedError: Error | null = null;
+    componentDidMount() {
+      this.afterRender();
+    }
 
-  const load = (): Promise<Exports> => {
-    if (promise) return promise;
-    const keys = Object.keys(options.loader) as (keyof Exports)[];
-    promise = Promise.all(
-      keys.map(key => Promise.resolve(options.loader[key]())),
-    ).then(
-      values => {
-        const loaded = {} as Exports;
-        keys.forEach((key, i) => {
-          loaded[key] = values[i] as Exports[typeof key];
-        });
-        cachedResult = loaded;
-        return loaded;
-      },
-      err => {
-        cachedError = err instanceof Error ? err : new Error(String(err));
-        throw cachedError;
-      },
-    );
-    return promise;
-  };
+    componentDidUpdate() {
+      this.afterRender();
+    }
 
-  const Renderer: LoadableRenderer<Props> = props => {
-    const [state, setState] = useState<{
-      loaded: Exports | null;
-      error: Error | null;
-    }>(() => ({ loaded: cachedResult, error: cachedError }));
-
-    useEffect(() => {
-      if (state.loaded || state.error) return undefined;
-      let cancelled = false;
-      load().then(
-        loaded => {
-          if (!cancelled) setState({ loaded, error: null });
-        },
-        err => {
-          if (!cancelled) setState({ loaded: null, error: err });
-        },
-      );
-      return () => {
-        cancelled = true;
-      };
-    }, [state.loaded, state.error]);
-
-    // Keep callback refs current without retriggering the post-load effect on
-    // every prop update.
-    const onRenderSuccessRef = useRef(props.onRenderSuccess);
-    const onRenderFailureRef = useRef(props.onRenderFailure);
-    onRenderSuccessRef.current = props.onRenderSuccess;
-    onRenderFailureRef.current = props.onRenderFailure;
-
-    useEffect(() => {
-      if (state.error) {
-        onRenderFailureRef.current?.(state.error);
-      } else if (state.loaded && Object.keys(state.loaded).length > 0) {
-        onRenderSuccessRef.current?.();
+    afterRender() {
+      const { loaded, loading, error } = this.state;
+      const { onRenderFailure, onRenderSuccess } = this.props;
+      if (!loading) {
+        if (error) {
+          (onRenderFailure as Function)(error);
+        } else if (loaded && Object.keys(loaded).length > 0) {
+          (onRenderSuccess as Function)();
+        }
       }
-    }, [state.loaded, state.error]);
-
-    if (state.error) {
-      return options.loading({ error: state.error });
     }
-    if (!state.loaded) {
-      return options.loading({});
-    }
-    return options.render(state.loaded, props as Props);
-  };
+  }
 
-  Renderer.preload = load;
+  CustomLoadableRenderer.defaultProps = defaultProps;
+  CustomLoadableRenderer.preload = LoadableRenderer.preload;
 
-  return Renderer;
+  return CustomLoadableRenderer;
 }

superset-frontend/packages/superset-ui-core/src/components/Label/index.tsx

@@ -38,7 +38,7 @@ export function Label(props: LabelProps) {
   } = props;
 
   const baseColor = getColorVariants(theme, type);
-  const color = baseColor.text;
+  const color = baseColor.active;
   const borderColor = baseColor.border;
   const backgroundColor = baseColor.bg;
 

superset-frontend/playwright/components/ListView/BulkSelect.ts

@@ -17,12 +17,13 @@
  * under the License.
  */
 
-import { Locator, Page } from '@playwright/test';
+import { Locator, Page, expect } from '@playwright/test';
 import { Button, Checkbox, Table } from '../core';
 
 const BULK_SELECT_SELECTORS = {
   CONTROLS: '[data-test="bulk-select-controls"]',
   ACTION: '[data-test="bulk-select-action"]',
+  HEADER_TOGGLE: '[data-test="header-toggle-all"]',
 } as const;
 
 /**
@@ -56,10 +57,17 @@ export class BulkSelect {
   }
 
   /**
-   * Enables bulk selection mode by clicking the toggle button
+   * Enables bulk selection mode by clicking the toggle button.
+   *
+   * Waits for the bulk-select column header checkbox to render so the next
+   * row interaction does not race the table re-render that adds the
+   * checkbox column.
    */
   async enable(): Promise<void> {
     await this.getToggleButton().click();
+    await this.page
+      .locator(BULK_SELECT_SELECTORS.HEADER_TOGGLE)
+      .waitFor({ state: 'visible' });
   }
 
   /**
@@ -72,19 +80,27 @@ export class BulkSelect {
   }
 
   /**
-   * Selects a row's checkbox in bulk select mode
+   * Selects a row's checkbox in bulk select mode.
+   * Asserts the checkbox is checked afterwards so any state-update race
+   * surfaces here rather than as a missing bulk-action button later.
    * @param rowName - The name/text identifying the row to select
    */
   async selectRow(rowName: string): Promise<void> {
-    await this.getRowCheckbox(rowName).check();
+    const checkbox = this.getRowCheckbox(rowName);
+    await checkbox.check();
+    await expect(checkbox.element).toBeChecked();
   }
 
   /**
-   * Deselects a row's checkbox in bulk select mode
+   * Deselects a row's checkbox in bulk select mode.
+   * Mirrors selectRow: asserts the unchecked state so any lingering selection
+   * surfaces here rather than as a stale bulk-action count later.
    * @param rowName - The name/text identifying the row to deselect
    */
   async deselectRow(rowName: string): Promise<void> {
-    await this.getRowCheckbox(rowName).uncheck();
+    const checkbox = this.getRowCheckbox(rowName);
+    await checkbox.uncheck();
+    await expect(checkbox.element).not.toBeChecked();
   }
 
   /**
@@ -107,10 +123,11 @@ export class BulkSelect {
   }
 
   /**
-   * Clicks a bulk action button by name (e.g., "Export", "Delete")
+   * Clicks a bulk action button by name (e.g., "Export", "Delete").
    * @param actionName - The name of the bulk action to click
    */
   async clickAction(actionName: string): Promise<void> {
-    await this.getActionButton(actionName).click();
+    const button = this.getActionButton(actionName);
+    await button.click();
   }
 }

superset-frontend/playwright/components/modals/DeleteConfirmationModal.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { expect } from '@playwright/test';
 import { Modal, Input } from '../core';
 
 /**
@@ -27,7 +28,8 @@ import { Modal, Input } from '../core';
  */
 export class DeleteConfirmationModal extends Modal {
   private static readonly SELECTORS = {
-    CONFIRMATION_INPUT: 'input[type="text"]',
+    CONFIRMATION_INPUT: '[data-test="delete-modal-input"]',
+    CONFIRM_BUTTON: '[data-test="modal-confirm-button"]',
   };
 
   /**
@@ -36,12 +38,16 @@ export class DeleteConfirmationModal extends Modal {
   private get confirmationInput(): Input {
     return new Input(
       this.page,
-      this.body.locator(DeleteConfirmationModal.SELECTORS.CONFIRMATION_INPUT),
+      this.element.locator(
+        DeleteConfirmationModal.SELECTORS.CONFIRMATION_INPUT,
+      ),
     );
   }
 
   /**
    * Fills the confirmation input with the specified text.
+   * Waits for the input to be visible before filling so callers don't race
+   * with the modal's open animation / focus effect.
    *
    * @param confirmationText - The text to type
    * @param options - Optional fill options (timeout, force)
@@ -57,11 +63,25 @@ export class DeleteConfirmationModal extends Modal {
     confirmationText: string,
     options?: { timeout?: number; force?: boolean },
   ): Promise<void> {
+    await this.confirmationInput.element.waitFor({
+      state: 'visible',
+      timeout: options?.timeout,
+    });
     await this.confirmationInput.fill(confirmationText, options);
   }
 
   /**
-   * Clicks the Delete button in the footer
+   * Clicks the Delete button in the footer.
+   *
+   * Targets the confirm button by data-test rather than going through
+   * Modal.clickFooterButton, which finds buttons by their visible text. The
+   * button label is i18n'd ("Delete" / "Supprimer" / …) so name-based lookups
+   * break in non-English locales.
+   *
+   * Also waits for the button to become enabled before clicking: it is
+   * disabled until the confirmation text matches "DELETE", and React's state
+   * update from fillConfirmationInput is asynchronous, so an immediate click
+   * can race the disabled→enabled transition.
    *
    * @param options - Optional click options (timeout, force, delay)
    */
@@ -70,6 +90,10 @@ export class DeleteConfirmationModal extends Modal {
     force?: boolean;
     delay?: number;
   }): Promise<void> {
-    await this.clickFooterButton('Delete', options);
+    const confirmButton = this.element.locator(
+      DeleteConfirmationModal.SELECTORS.CONFIRM_BUTTON,
+    );
+    await expect(confirmButton).toBeEnabled({ timeout: options?.timeout });
+    await confirmButton.click(options);
   }
 }

superset-frontend/playwright/tests/chart/chart-list.spec.ts

@@ -32,6 +32,7 @@ import {
   expectStatusOneOf,
   expectValidExportZip,
 } from '../../helpers/api/assertions';
+import { TIMEOUT } from '../../utils/constants';
 
 /**
  * Extend testWithAssets with chartListPage navigation (beforeEach equivalent).
@@ -62,8 +63,11 @@ test('should delete a chart with confirmation', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify chart is visible in list
-  await expect(chartListPage.getChartRow(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart appears.
+  await expect(chartListPage.getChartRow(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click delete action button
   await chartListPage.clickDeleteAction(chartName);
@@ -81,12 +85,12 @@ test('should delete a chart with confirmation', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
-  // Verify chart is removed from list
-  await expect(chartListPage.getChartRow(chartName)).not.toBeVisible();
+  // Verify chart is removed from list (deleted rows leave the DOM)
+  await expect(chartListPage.getChartRow(chartName)).toHaveCount(0);
 
   // Backend verification: API returns 404
   await expectDeleted(page, ENDPOINTS.CHART, chartId, {
@@ -111,8 +115,11 @@ test('should edit chart name via properties modal', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify chart is visible in list
-  await expect(chartListPage.getChartRow(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart appears.
+  await expect(chartListPage.getChartRow(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click edit action to open properties modal
   await chartListPage.clickEditAction(chartName);
@@ -137,7 +144,7 @@ test('should edit chart name via properties modal', async ({
   // Modal should close
   await propertiesModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
@@ -164,8 +171,11 @@ test('should export a chart as a zip file', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify chart is visible in list
-  await expect(chartListPage.getChartRow(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart appears.
+  await expect(chartListPage.getChartRow(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Set up API response intercept for export endpoint
   const exportResponsePromise = waitForGet(page, ENDPOINTS.CHART_EXPORT);
@@ -202,9 +212,14 @@ test('should bulk delete multiple charts', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify both charts are visible in list
-  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible();
-  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created charts appear.
+  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await chartListPage.clickBulkSelectButton();
@@ -229,13 +244,13 @@ test('should bulk delete multiple charts', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
-  // Verify both charts are removed from list
-  await expect(chartListPage.getChartRow(chart1.name)).not.toBeVisible();
-  await expect(chartListPage.getChartRow(chart2.name)).not.toBeVisible();
+  // Verify both charts are removed from list (deleted rows leave the DOM)
+  await expect(chartListPage.getChartRow(chart1.name)).toHaveCount(0);
+  await expect(chartListPage.getChartRow(chart2.name)).toHaveCount(0);
 
   // Backend verification: Both return 404
   for (const chart of [chart1, chart2]) {
@@ -259,8 +274,11 @@ test('should edit chart name from card view', async ({ page, testAssets }) => {
   await cardListPage.gotoCardView();
   await cardListPage.waitForCardLoad();
 
-  // Verify chart card is visible
-  await expect(cardListPage.getChartCard(chartName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created chart card appears.
+  await expect(cardListPage.getChartCard(chartName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Open card dropdown and click edit
   await cardListPage.clickCardEditAction(chartName);
@@ -285,13 +303,16 @@ test('should edit chart name from card view', async ({ page, testAssets }) => {
   // Modal should close
   await propertiesModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
   // Verify the renamed card appears in card view and old name is gone
-  await expect(cardListPage.getChartCard(newName)).toBeVisible();
-  await expect(cardListPage.getChartCard(chartName)).not.toBeVisible();
+  // (the old card name is removed from the DOM after the rename re-render).
+  await expect(cardListPage.getChartCard(newName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(cardListPage.getChartCard(chartName)).toHaveCount(0);
 
   // Backend verification: API returns updated name
   const response = await apiGetChart(page, chartId);
@@ -304,6 +325,11 @@ test('should bulk export multiple charts', async ({
   chartListPage,
   testAssets,
 }) => {
+  // Chains create×2 → refresh → bulk select → export. Matches the
+  // sibling bulk-delete test's budget so the export response wait below
+  // can exceed the 30s default without hitting the test timeout.
+  test.setTimeout(TIMEOUT.SLOW_TEST);
+
   // Create 2 throwaway charts for bulk export
   const [chart1, chart2] = await Promise.all([
     createTestChart(page, testAssets, test.info(), {
@@ -318,9 +344,14 @@ test('should bulk export multiple charts', async ({
   await chartListPage.goto();
   await chartListPage.waitForTableLoad();
 
-  // Verify both charts are visible in list
-  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible();
-  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created charts appear.
+  await expect(chartListPage.getChartRow(chart1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(chartListPage.getChartRow(chart2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await chartListPage.clickBulkSelectButton();
@@ -329,8 +360,12 @@ test('should bulk export multiple charts', async ({
   await chartListPage.selectChartCheckbox(chart1.name);
   await chartListPage.selectChartCheckbox(chart2.name);
 
-  // Set up API response intercept for export endpoint
-  const exportResponsePromise = waitForGet(page, ENDPOINTS.CHART_EXPORT);
+  // Set up API response intercept BEFORE the click that triggers it.
+  // Exports of multiple charts can take longer than 30s under load,
+  // so use SLOW_TEST instead of the default test-timeout-bound budget.
+  const exportResponsePromise = waitForGet(page, ENDPOINTS.CHART_EXPORT, {
+    timeout: TIMEOUT.SLOW_TEST,
+  });
 
   // Click bulk export action
   await chartListPage.clickBulkAction('Export');

superset-frontend/playwright/tests/dashboard/dashboard-list.spec.ts

@@ -68,33 +68,34 @@ test('should delete a dashboard with confirmation', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify dashboard is visible in list
-  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboard appears.
+  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click delete action button
   await dashboardListPage.clickDeleteAction(dashboardName);
 
   // Delete confirmation modal should appear
   const deleteModal = new DeleteConfirmationModal(page);
-  await deleteModal.waitForVisible();
+  await deleteModal.waitForReady();
 
   // Type "DELETE" to confirm
   await deleteModal.fillConfirmationInput('DELETE');
 
-  // Click the Delete button
+  // Click the Delete button (waits for it to become enabled)
   await deleteModal.clickDelete();
 
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
   // Verify dashboard is removed from list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboardName),
-  ).not.toBeVisible();
+  await expect(dashboardListPage.getDashboardRow(dashboardName)).toHaveCount(0);
 
   // Backend verification: API returns 404
   await expectDeleted(page, ENDPOINTS.DASHBOARD, dashboardId, {
@@ -119,8 +120,11 @@ test('should export a dashboard as a zip file', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify dashboard is visible in list
-  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboard appears.
+  await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Set up API response intercept for export endpoint
   const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT);
@@ -157,13 +161,14 @@ test('should bulk delete multiple dashboards', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify both dashboards are visible in list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard1.name),
-  ).toBeVisible();
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard2.name),
-  ).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboards appear.
+  await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(dashboardListPage.getDashboardRow(dashboard2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await dashboardListPage.clickBulkSelectButton();
@@ -188,17 +193,17 @@ test('should bulk delete multiple dashboards', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
-  // Verify both dashboards are removed from list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard1.name),
-  ).not.toBeVisible();
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard2.name),
-  ).not.toBeVisible();
+  // Verify both dashboards are removed from list (deleted rows leave the DOM)
+  await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toHaveCount(
+    0,
+  );
+  await expect(dashboardListPage.getDashboardRow(dashboard2.name)).toHaveCount(
+    0,
+  );
 
   // Backend verification: Both return 404
   for (const dashboard of [dashboard1, dashboard2]) {
@@ -213,6 +218,11 @@ test('should bulk export multiple dashboards', async ({
   dashboardListPage,
   testAssets,
 }) => {
+  // Chains create×2 → refresh → bulk select → export. Matches the
+  // sibling bulk-delete test's budget so the export response wait below
+  // can exceed the 30s default without hitting the test timeout.
+  test.setTimeout(TIMEOUT.SLOW_TEST);
+
   // Create 2 throwaway dashboards for bulk export
   const [dashboard1, dashboard2] = await Promise.all([
     createTestDashboard(page, testAssets, test.info(), {
@@ -227,25 +237,30 @@ test('should bulk export multiple dashboards', async ({
   await dashboardListPage.goto();
   await dashboardListPage.waitForTableLoad();
 
-  // Verify both dashboards are visible in list
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard1.name),
-  ).toBeVisible();
-  await expect(
-    dashboardListPage.getDashboardRow(dashboard2.name),
-  ).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dashboards appear.
+  await expect(dashboardListPage.getDashboardRow(dashboard1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(dashboardListPage.getDashboardRow(dashboard2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
-  // Enable bulk select mode
+  // Enable bulk select mode (waits for the checkbox column to render)
   await dashboardListPage.clickBulkSelectButton();
 
-  // Select both dashboards
+  // Select both dashboards (each call asserts the checkbox is checked)
   await dashboardListPage.selectDashboardCheckbox(dashboard1.name);
   await dashboardListPage.selectDashboardCheckbox(dashboard2.name);
 
-  // Set up API response intercept for export endpoint
-  const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT);
+  // Set up API response intercept BEFORE the click that triggers it.
+  // Exports of multiple dashboards can take longer than 30s under load,
+  // so use SLOW_TEST instead of the default test-timeout-bound budget.
+  const exportResponsePromise = waitForGet(page, ENDPOINTS.DASHBOARD_EXPORT, {
+    timeout: TIMEOUT.SLOW_TEST,
+  });
 
-  // Click bulk export action
+  // Click bulk export action (waits for the action button to render)
   await dashboardListPage.clickBulkAction('Export');
 
   // Wait for export API response and validate zip contains both dashboards
@@ -293,12 +308,12 @@ test.describe('import dashboard', () => {
       label: `Dashboard ${dashboardId}`,
     });
 
-    // Refresh to confirm dashboard is no longer in the list
+    // Refresh to confirm dashboard is no longer in the list (deleted rows leave the DOM)
     await dashboardListPage.goto();
     await dashboardListPage.waitForTableLoad();
-    await expect(
-      dashboardListPage.getDashboardRow(dashboardName),
-    ).not.toBeVisible();
+    await expect(dashboardListPage.getDashboardRow(dashboardName)).toHaveCount(
+      0,
+    );
 
     // Click the import button
     await dashboardListPage.clickImportButton();
@@ -350,7 +365,7 @@ test.describe('import dashboard', () => {
     // Modal should close on success
     await importModal.waitForHidden({ timeout: TIMEOUT.FILE_IMPORT });
 
-    // Verify success toast appears
+    // Verify success toast appears.
     const toast = new Toast(page);
     await expect(toast.getSuccess()).toBeVisible({ timeout: 10000 });
 
@@ -358,10 +373,11 @@ test.describe('import dashboard', () => {
     await dashboardListPage.goto();
     await dashboardListPage.waitForTableLoad();
 
-    // Verify dashboard appears in list
-    await expect(
-      dashboardListPage.getDashboardRow(dashboardName),
-    ).toBeVisible();
+    // The list query is asynchronous; allow extra time on slow CI before the
+    // freshly-imported dashboard appears.
+    await expect(dashboardListPage.getDashboardRow(dashboardName)).toBeVisible({
+      timeout: TIMEOUT.API_RESPONSE,
+    });
 
     // Track for cleanup: look up the reimported dashboard by title
     const reimported = await getDashboardByName(page, dashboardName);

superset-frontend/playwright/tests/dataset/dataset-list.spec.ts

@@ -107,8 +107,11 @@ test('should delete a dataset with confirmation', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify dataset is visible in list
-  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dataset appears.
+  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click delete action button
   await datasetListPage.clickDeleteAction(datasetName);
@@ -126,14 +129,13 @@ test('should delete a dataset with confirmation', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears with correct message
+  // Verify success toast appears with correct message.
   const toast = new Toast(page);
-  const successToast = toast.getSuccess();
-  await expect(successToast).toBeVisible();
+  await expect(toast.getSuccess()).toBeVisible();
   await expect(toast.getMessage()).toContainText('Deleted');
 
-  // Verify dataset is removed from list
-  await expect(datasetListPage.getDatasetRow(datasetName)).not.toBeVisible();
+  // Verify dataset is removed from list (deleted rows leave the DOM)
+  await expect(datasetListPage.getDatasetRow(datasetName)).toHaveCount(0);
 
   // Verify via API that dataset no longer exists (404)
   await expectDeleted(page, ENDPOINTS.DATASET, datasetId, {
@@ -155,10 +157,13 @@ test('should duplicate a dataset with new name', async ({
   );
   const duplicateName = `duplicate_${Date.now()}_${test.info().parallelIndex}`;
 
-  // Navigate to list and verify original dataset is visible
+  // Navigate to list and verify original dataset is visible.
+  // The list query is asynchronous; allow extra time on slow CI.
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
-  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible();
+  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Set up response intercept to capture duplicate dataset ID
   const duplicateResponsePromise = waitForPost(
@@ -201,9 +206,14 @@ test('should duplicate a dataset with new name', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify both datasets exist in list
-  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible();
-  await expect(datasetListPage.getDatasetRow(duplicateName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // duplicate appears alongside the original.
+  await expect(datasetListPage.getDatasetRow(originalName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(datasetListPage.getDatasetRow(duplicateName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // API Verification: Fetch both datasets via detail API for consistent comparison
   // (list API may return undefined for fields that detail API returns as null)
@@ -256,6 +266,11 @@ test('should export multiple datasets via bulk select action', async ({
   datasetListPage,
   testAssets,
 }) => {
+  // Chains create×2 → refresh → bulk select → export. Matches the
+  // sibling bulk-delete test's budget so the export response wait below
+  // can exceed the 30s default without hitting the test timeout.
+  test.setTimeout(TIMEOUT.SLOW_TEST);
+
   // Create 2 throwaway datasets for bulk export
   const [dataset1, dataset2] = await Promise.all([
     createTestDataset(page, testAssets, test.info(), {
@@ -270,9 +285,14 @@ test('should export multiple datasets via bulk select action', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify both datasets are visible in list
-  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible();
-  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created datasets appear.
+  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await datasetListPage.clickBulkSelectButton();
@@ -281,8 +301,12 @@ test('should export multiple datasets via bulk select action', async ({
   await datasetListPage.selectDatasetCheckbox(dataset1.name);
   await datasetListPage.selectDatasetCheckbox(dataset2.name);
 
-  // Set up API response intercept for export endpoint
-  const exportResponsePromise = waitForGet(page, ENDPOINTS.DATASET_EXPORT);
+  // Set up API response intercept BEFORE the click that triggers it.
+  // Exports of multiple datasets can take longer than 30s under load,
+  // so use SLOW_TEST instead of the default test-timeout-bound budget.
+  const exportResponsePromise = waitForGet(page, ENDPOINTS.DATASET_EXPORT, {
+    timeout: TIMEOUT.SLOW_TEST,
+  });
 
   // Click bulk export action
   await datasetListPage.clickBulkAction('Export');
@@ -312,8 +336,11 @@ test('should edit dataset name via modal', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify dataset is visible in list
-  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created dataset appears.
+  await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Click edit action to open modal
   await datasetListPage.clickEditAction(datasetName);
@@ -348,7 +375,7 @@ test('should edit dataset name via modal', async ({
   // Modal should close
   await editModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible({ timeout: 10000 });
 
@@ -377,9 +404,14 @@ test('should bulk delete multiple datasets', async ({
   await datasetListPage.goto();
   await datasetListPage.waitForTableLoad();
 
-  // Verify both datasets are visible in list
-  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible();
-  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible();
+  // The list query is asynchronous; allow extra time on slow CI before the
+  // freshly-created datasets appear.
+  await expect(datasetListPage.getDatasetRow(dataset1.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
+  await expect(datasetListPage.getDatasetRow(dataset2.name)).toBeVisible({
+    timeout: TIMEOUT.API_RESPONSE,
+  });
 
   // Enable bulk select mode
   await datasetListPage.clickBulkSelectButton();
@@ -404,13 +436,13 @@ test('should bulk delete multiple datasets', async ({
   // Modal should close
   await deleteModal.waitForHidden();
 
-  // Verify success toast appears
+  // Verify success toast appears.
   const toast = new Toast(page);
   await expect(toast.getSuccess()).toBeVisible();
 
-  // Verify both datasets are removed from list
-  await expect(datasetListPage.getDatasetRow(dataset1.name)).not.toBeVisible();
-  await expect(datasetListPage.getDatasetRow(dataset2.name)).not.toBeVisible();
+  // Verify both datasets are removed from list (deleted rows leave the DOM)
+  await expect(datasetListPage.getDatasetRow(dataset1.name)).toHaveCount(0);
+  await expect(datasetListPage.getDatasetRow(dataset2.name)).toHaveCount(0);
 
   // Verify via API that datasets no longer exist (404)
   await expectDeleted(page, ENDPOINTS.DATASET, dataset1.id, {
@@ -455,10 +487,10 @@ test.describe('import dataset', () => {
       label: `Dataset ${datasetId}`,
     });
 
-    // Refresh to confirm dataset is no longer in the list
+    // Refresh to confirm dataset is no longer in the list (deleted rows leave the DOM)
     await datasetListPage.goto();
     await datasetListPage.waitForTableLoad();
-    await expect(datasetListPage.getDatasetRow(datasetName)).not.toBeVisible();
+    await expect(datasetListPage.getDatasetRow(datasetName)).toHaveCount(0);
 
     // Click the import button
     await datasetListPage.clickImportButton();
@@ -507,7 +539,7 @@ test.describe('import dataset', () => {
     // Modal should close on success
     await importModal.waitForHidden({ timeout: TIMEOUT.FILE_IMPORT });
 
-    // Verify success toast appears
+    // Verify success toast appears.
     const toast = new Toast(page);
     await expect(toast.getSuccess()).toBeVisible({ timeout: 10000 });
 
@@ -515,8 +547,11 @@ test.describe('import dataset', () => {
     await datasetListPage.goto();
     await datasetListPage.waitForTableLoad();
 
-    // Verify dataset appears in list
-    await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible();
+    // The list query is asynchronous; allow extra time on slow CI before the
+    // freshly-imported dataset appears.
+    await expect(datasetListPage.getDatasetRow(datasetName)).toBeVisible({
+      timeout: TIMEOUT.API_RESPONSE,
+    });
 
     // Track for cleanup: the dataset import API returns {"message": "OK"}
     // with no ID, so look up the reimported dataset by name.

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/htmlTextFilterValueGetter.test.ts

@@ -1,83 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import { ValueGetterParams } from '@superset-ui/core/components/ThemedAgGridReact';
-import htmlTextFilterValueGetter, {
-  htmlTextComparator,
-} from './htmlTextFilterValueGetter';
-
-const makeParams = (value: unknown): ValueGetterParams =>
-  ({
-    data: { foo: value },
-    colDef: { field: 'foo' },
-  }) as unknown as ValueGetterParams;
-
-test('htmlTextFilterValueGetter extracts visible text from HTML anchor', () => {
-  expect(
-    htmlTextFilterValueGetter(
-      makeParams(
-        '<a href="https://jira.example.com/123/S18_3232">S18_3232</a>',
-      ),
-    ),
-  ).toBe('S18_3232');
-});
-
-test('htmlTextFilterValueGetter strips nested HTML markup', () => {
-  expect(
-    htmlTextFilterValueGetter(
-      makeParams('<div><strong>Hello</strong> <em>World</em></div>'),
-    ),
-  ).toBe('Hello World');
-});
-
-test('htmlTextFilterValueGetter passes plain strings through', () => {
-  expect(htmlTextFilterValueGetter(makeParams('plain value'))).toBe(
-    'plain value',
-  );
-});
-
-test('htmlTextFilterValueGetter passes non-string values through', () => {
-  expect(htmlTextFilterValueGetter(makeParams(42))).toBe(42);
-  expect(htmlTextFilterValueGetter(makeParams(null))).toBeNull();
-  expect(htmlTextFilterValueGetter(makeParams(undefined))).toBeUndefined();
-});
-
-test('htmlTextComparator orders by visible text, not raw HTML', () => {
-  // URL prefixes (zzz vs bbb) would flip the order under raw-HTML sort,
-  // but the visible labels (S700_4002 vs S72_3212) sort the other way.
-  const left = '<a href="https://jira.example.com/zzz/S700_4002">S700_4002</a>';
-  const right = '<a href="https://jira.example.com/bbb/S72_3212">S72_3212</a>';
-  expect(htmlTextComparator(left, right)).toBeLessThan(0);
-});
-
-test('htmlTextComparator handles nulls and numbers', () => {
-  expect(htmlTextComparator(null, null)).toBe(0);
-  expect(htmlTextComparator(null, 'x')).toBeLessThan(0);
-  expect(htmlTextComparator('x', null)).toBeGreaterThan(0);
-  expect(htmlTextComparator(1, 2)).toBeLessThan(0);
-  expect(htmlTextComparator(2, 1)).toBeGreaterThan(0);
-});
-
-test('htmlTextComparator preserves default codepoint ordering for plain strings', () => {
-  // AG Grid's default string comparator orders by codepoint, so 'Z' (90)
-  // sorts before 'a' (97). A locale-aware comparator would flip this —
-  // verify we match the default so plain string columns are unaffected.
-  expect(htmlTextComparator('Z', 'a')).toBeLessThan(0);
-  expect(htmlTextComparator('a', 'Z')).toBeGreaterThan(0);
-  expect(htmlTextComparator('apple', 'banana')).toBeLessThan(0);
-});

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/htmlTextFilterValueGetter.ts

@@ -1,74 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import { isProbablyHTML, sanitizeHtml } from '@superset-ui/core';
-import { ValueGetterParams } from '@superset-ui/core/components/ThemedAgGridReact';
-
-const stripHtmlToText = (html: string): string => {
-  const doc = new DOMParser().parseFromString(sanitizeHtml(html), 'text/html');
-  return (doc.body.textContent || '').trim();
-};
-
-// Cache the comparator-ready form per raw string. Both the HTML-detection
-// step (`isProbablyHTML`, which itself invokes DOMParser for HTML-looking
-// values) and the extraction step (`stripHtmlToText`, also DOMParser) are
-// expensive; sort runs `O(n log n)` comparator calls against the same set
-// of cell values. Memoizing the combined detection + extraction means each
-// unique cell value pays the cost once per session. Module-level scope;
-// bounded by the count of unique string cell values seen.
-const comparableTextCache = new Map<string, string>();
-
-const toComparableText = (raw: string): string => {
-  const cached = comparableTextCache.get(raw);
-  if (cached !== undefined) return cached;
-  const normalized = isProbablyHTML(raw) ? stripHtmlToText(raw) : raw;
-  comparableTextCache.set(raw, normalized);
-  return normalized;
-};
-
-/**
- * Returns the visible-text representation of an HTML cell value so AG Grid
- * filters and sort operate on what the user sees, not the underlying markup.
- * Pass-through for non-HTML values.
- */
-const htmlTextFilterValueGetter = (params: ValueGetterParams) => {
-  const raw = params.data?.[params.colDef.field as string];
-  return typeof raw === 'string' ? toComparableText(raw) : raw;
-};
-
-/**
- * Comparator that mirrors AG Grid's default string comparator (codepoint
- * order, nulls first), but extracts visible text from HTML values first
- * so HTML cells sort by their displayed label. Plain (non-HTML) values
- * pass through unchanged, preserving default ordering — e.g. 'Z' still
- * sorts before 'a' as it does under the default comparator.
- */
-export const htmlTextComparator = (a: unknown, b: unknown): number => {
-  const toText = (v: unknown) =>
-    typeof v === 'string' ? toComparableText(v) : v;
-  const aT = toText(a);
-  const bT = toText(b);
-  if (aT == null && bT == null) return 0;
-  if (aT == null) return -1;
-  if (bT == null) return 1;
-  if (typeof aT === 'number' && typeof bT === 'number') return aT - bT;
-  if (aT === bT) return 0;
-  return aT < bT ? -1 : 1;
-};
-
-export default htmlTextFilterValueGetter;

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts

@@ -32,9 +32,6 @@ import {
 } from '../types';
 import getCellClass from './getCellClass';
 import filterValueGetter from './filterValueGetter';
-import htmlTextFilterValueGetter, {
-  htmlTextComparator,
-} from './htmlTextFilterValueGetter';
 import dateFilterComparator from './dateFilterComparator';
 import DateWithFormatter from './DateWithFormatter';
 import { getAggFunc } from './getAggFunc';
@@ -320,24 +317,6 @@ export const useColDefs = ({
         ...(isPercentMetric && {
           filterValueGetter,
         }),
-        ...(dataType === GenericDataType.String &&
-          !serverPagination && {
-            // HTML cells (e.g. anchor markup) are rendered by TextCellRenderer
-            // via dangerouslySetInnerHTML; without these the filter and sort
-            // operate on raw HTML so the URL inside the markup dictates order
-            // and the "Contains" filter matches against the raw HTML string.
-            //
-            // Gated on !serverPagination: in server-pagination mode sort and
-            // filter are both delegated to the backend (which sees raw HTML
-            // in the database), so applying the visible-text getter only on
-            // the client would create a mismatch where the typed filter
-            // value is stripped client-side but the server query still
-            // operates on the raw HTML. Server-paginated tables with HTML
-            // columns are out of scope for this fix and would require
-            // server-side handling.
-            filterValueGetter: htmlTextFilterValueGetter,
-            comparator: htmlTextComparator,
-          }),
         ...(dataType === GenericDataType.Temporal && {
           // Use dateFilterValueGetter so AG Grid correctly identifies null dates for blank filter
           filterValueGetter: dateFilterValueGetter,

superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts

@@ -285,6 +285,8 @@ export default function transformProps(
   }
   const labelProps = {
     color: theme.colorText,
+    textBorderColor: theme.colorBgBase,
+    textBorderWidth: 1,
   };
   const traverse = (
     treeNodes: TreeNode[],

superset-frontend/plugins/plugin-chart-echarts/test/Sunburst/transformProps.test.ts

@@ -1,53 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-import { ChartProps } from '@superset-ui/core';
-import { supersetTheme } from '@apache-superset/core/theme';
-import { EchartsSunburstChartProps } from '../../src/Sunburst/types';
-import transformProps from '../../src/Sunburst/transformProps';
-
-const formData = {
-  colorScheme: 'bnbColors',
-  datasource: '3__table',
-  groupby: ['category'],
-  metric: 'sum__value',
-};
-
-const chartProps = new ChartProps({
-  formData,
-  width: 800,
-  height: 600,
-  queriesData: [
-    {
-      data: [
-        { category: 'A', sum__value: 10 },
-        { category: 'B', sum__value: 20 },
-      ],
-    },
-  ],
-  theme: supersetTheme,
-});
-
-test('series label has no textBorderColor or textBorderWidth', () => {
-  const { echartOptions } = transformProps(
-    chartProps as EchartsSunburstChartProps,
-  );
-  const series = (echartOptions as any).series[0];
-  expect(series.label).not.toHaveProperty('textBorderColor');
-  expect(series.label).not.toHaveProperty('textBorderWidth');
-});

superset-frontend/src/components/Chart/chartAction.ts

@@ -781,16 +781,6 @@ export function exploreJSON(
         handleChartDataResponse(response, json, useLegacyApi),
       )
       .then(queriesResponse => {
-        // Drop stale responses: if a newer query has started for this chart,
-        // its controller will have replaced ours in state, so ignore this
-        // response to avoid clobbering newer data with older results.
-        if (key != null) {
-          const currentController =
-            getState().charts?.[key]?.queryController;
-          if (currentController && currentController !== controller) {
-            return undefined;
-          }
-        }
         (queriesResponse as QueryData[]).forEach(
           (resultItem: QueryData & { applied_filters?: JsonObject[] }) =>
             dispatch(
@@ -836,16 +826,6 @@ export function exploreJSON(
             );
           }
 
-          // Drop stale failures the same way we drop stale successes,
-          // so a slow earlier request can't mark a newer one as failed.
-          if (key != null) {
-            const currentController =
-              getState().charts?.[key]?.queryController;
-            if (currentController && currentController !== controller) {
-              return undefined;
-            }
-          }
-
           if (isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) {
             // In async mode we just pass the raw error response through
             return dispatch(

superset-frontend/src/components/Chart/chartActions.test.ts

@@ -156,78 +156,6 @@ describe('chart actions', () => {
       .mockImplementation((data: unknown) => Promise.resolve(data));
   });
 
-  test('should drop stale success dispatches when a newer controller has replaced ours in state', async () => {
-    const chartKey = 'stale_success_test';
-    const formData: Partial<QueryFormData> = {
-      slice_id: 456,
-      datasource: 'table__1',
-      viz_type: 'table',
-    };
-    // A controller belonging to a *newer* in-flight request, already stored
-    // in state by the time this thunk's response resolves.
-    const newerController = new AbortController();
-    const state: MockState = {
-      charts: {
-        [chartKey]: {
-          queryController: newerController,
-        },
-      },
-      common: {
-        conf: {
-          SUPERSET_WEBSERVER_TIMEOUT: 60,
-        },
-      },
-    };
-    const getState = jest.fn(() => state);
-    const dispatchMock = jest.fn();
-    const getChartDataRequestSpy = jest
-      .spyOn(actions, 'getChartDataRequest')
-      .mockResolvedValue({
-        response: { status: 200 } as Response,
-        json: { result: [{ data: [{ stale: true }] }] },
-      });
-    const handleChartDataResponseSpy = jest
-      .spyOn(actions, 'handleChartDataResponse')
-      .mockResolvedValue([{ data: [{ stale: true }] }]);
-    const updateDataMaskSpy = jest
-      .spyOn(dataMaskActions, 'updateDataMask')
-      .mockReturnValue({ type: 'UPDATE_DATA_MASK' } as ReturnType<
-        typeof dataMaskActions.updateDataMask
-      >);
-    const getQuerySettingsStub = jest
-      .spyOn(exploreUtils, 'getQuerySettings')
-      .mockReturnValue([false, () => {}] as unknown as ReturnType<
-        typeof exploreUtils.getQuerySettings
-      >);
-
-    try {
-      const thunkAction = actions.exploreJSON(
-        formData as QueryFormData,
-        false,
-        undefined,
-        chartKey,
-      );
-      await thunkAction(
-        dispatchMock as unknown as actions.ChartThunkDispatch,
-        getState as unknown as () => actions.RootState,
-        undefined,
-      );
-
-      // CHART_UPDATE_STARTED is fine (it ran before the gate),
-      // but CHART_UPDATE_SUCCEEDED must NOT have fired with the stale data.
-      const dispatchedTypes = dispatchMock.mock.calls.map(
-        ([action]) => action?.type,
-      );
-      expect(dispatchedTypes).toContain(actions.CHART_UPDATE_STARTED);
-      expect(dispatchedTypes).not.toContain(actions.CHART_UPDATE_SUCCEEDED);
-    } finally {
-      getChartDataRequestSpy.mockRestore();
-      handleChartDataResponseSpy.mockRestore();
-      updateDataMaskSpy.mockRestore();
-      getQuerySettingsStub.mockRestore();
-    }
-  });
-
   test('should defer abort of previous controller to avoid Redux state mutation', async () => {
     jest.useFakeTimers();
     const chartKey = 'defer_abort_test';

superset-frontend/src/embedded/index.tsx

@@ -18,7 +18,7 @@
  */
 import 'src/public-path';
 
-import { lazy, Suspense, useEffect } from 'react';
+import { lazy, StrictMode, Suspense, useEffect } from 'react';
 import { createRoot, type Root } from 'react-dom/client';
 import { BrowserRouter as Router, Route } from 'react-router-dom';
 import { Global } from '@emotion/react';
@@ -197,7 +197,11 @@ function start() {
       if (!root) {
         root = createRoot(appMountPoint);
       }
-      root.render(<EmbeddedApp />);
+      root.render(
+        <StrictMode>
+          <EmbeddedApp />
+        </StrictMode>,
+      );
     },
     err => {
       // something is most likely wrong with the guest token; reset the guard

superset-frontend/src/explore/components/ExploreContentPopover.tsx

@@ -22,18 +22,12 @@ export const ExplorePopoverContent = styled.div`
   .edit-popover-resize {
     transform: scaleX(-1);
     float: right;
-    margin-top: ${({ theme }) => theme.margin}px;
-    margin-right: ${({ theme }) => theme.marginXXS * -1}px;
+    margin-top: ${({ theme }) => theme.sizeUnit * 4}px;
+    margin-right: ${({ theme }) => theme.sizeUnit * -1}px;
     color: ${({ theme }) => theme.colorIcon};
     cursor: nwse-resize;
   }
   .filter-sql-editor {
     border: ${({ theme }) => theme.colorBorder} solid thin;
   }
-  && .ant-tabs-nav {
-    margin-bottom: ${({ theme }) => theme.marginSM}px;
-  }
-  && .ant-form-item {
-    margin-bottom: ${({ theme }) => theme.marginXS}px;
-  }
 `;

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx

@@ -78,6 +78,14 @@ interface AdhocFilterEditPopoverState {
 }
 
 const FilterPopoverContentContainer = styled.div`
+  .adhoc-filter-edit-tabs > .nav-tabs {
+    margin-bottom: ${({ theme }) => theme.sizeUnit * 2}px;
+
+    & > li > a {
+      padding: ${({ theme }) => theme.sizeUnit}px;
+    }
+  }
+
   #filter-edit-popover {
     max-width: none;
   }
@@ -89,17 +97,21 @@ const FilterPopoverContentContainer = styled.div`
   .filter-edit-clause-section {
     display: flex;
     flex-direction: row;
-    gap: ${({ theme }) => theme.marginMD}px;
+    gap: ${({ theme }) => theme.sizeUnit * 5}px;
+  }
+
+  .adhoc-filter-simple-column-dropdown {
+    margin-top: ${({ theme }) => theme.sizeUnit * 5}px;
   }
 `;
 
 const FilterActionsContainer = styled.div`
-  margin-top: ${({ theme }) => theme.marginXS}px;
+  margin-top: ${({ theme }) => theme.sizeUnit * 2}px;
 `;
 
 const LayerSelectContainer = styled.div`
-  margin-top: ${({ theme }) => theme.marginXS}px;
-  margin-bottom: ${({ theme }) => theme.marginXXL}px;
+  margin-top: ${({ theme }) => theme.sizeUnit * 2}px;
+  margin-bottom: ${({ theme }) => theme.sizeUnit * 12}px;
 `;
 
 export default class AdhocFilterEditPopover extends Component<

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx

@@ -523,7 +523,8 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
   const subjectComponent = (
     <Select
       css={{
-        marginBottom: theme.marginXS,
+        marginTop: theme.sizeUnit * 4,
+        marginBottom: theme.sizeUnit * 4,
       }}
       data-test="select-element"
       options={columns.map(column => ({
@@ -564,7 +565,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
         >
           <SelectWithLabel
             css={css`
-              margin-top: ${theme.marginXS}px;
+              margin-top: ${theme.sizeUnit * 4}px;
             `}
             labelText={labelText}
             options={suggestions}
@@ -580,7 +581,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
         >
           <div
             css={css`
-              margin-top: ${theme.marginXS}px;
+              margin-top: ${theme.sizeUnit * 4}px;
             `}
           />
           <Input

superset-frontend/src/explore/components/controls/LayerConfigsControl/LayerConfigsPopoverContent.tsx

@@ -64,6 +64,7 @@ export const StyledCloseButton = styled(Button)`
     color: ${theme.colorPrimaryText};
     font-size: ${theme.fontSizeSM}px;
     font-weight: ${theme.fontWeightStrong};
+    text-transform: uppercase;
     min-width: ${theme.sizeUnit * 36};
     min-height: ${theme.sizeUnit * 8};
     box-shadow: none;
@@ -112,6 +113,7 @@ export const StyledSaveButton = styled(Button)`
     color: ${theme.colorTextLightSolid};
     font-size: ${theme.fontSizeSM}px;
     font-weight: ${theme.fontWeightStrong};
+    text-transform: uppercase;
     min-width: ${theme.sizeUnit * 36};
     min-height: ${theme.sizeUnit * 8};
     box-shadow: none;

superset-frontend/src/explore/components/controls/MapViewControl/MapViewControl.tsx

@@ -36,6 +36,7 @@ export const StyledExtentButton = styled(Button)`
     color: ${theme.colorPrimaryText};
     font-size: ${theme.fontSizeSM}px;
     font-weight: ${theme.fontWeightStrong};
+    text-transform: uppercase;
     min-width: ${theme.sizeUnit * 36};
     min-height: ${theme.sizeUnit * 8};
     box-shadow: none;

superset-frontend/src/views/index.tsx

@@ -18,6 +18,7 @@
  */
 import 'src/public-path';
 
+import { StrictMode } from 'react';
 import { createRoot } from 'react-dom/client';
 import { logging } from '@apache-superset/core/utils';
 import initPreamble from 'src/preamble';
@@ -31,7 +32,11 @@ if (appMountPoint) {
       await initPreamble();
     } finally {
       const { default: App } = await import(/* webpackMode: "eager" */ './App');
-      root.render(<App />);
+      root.render(
+        <StrictMode>
+          <App />
+        </StrictMode>,
+      );
     }
   })().catch(err => {
     logging.error('Unhandled error during app initialization', err);

superset-frontend/src/views/menu.tsx

@@ -20,6 +20,7 @@ import 'src/public-path';
 
 // Menu App. Used in views that do not already include the Menu component in the layout.
 // eg, backend rendered views
+import { StrictMode } from 'react';
 import { Provider } from 'react-redux';
 import { createRoot } from 'react-dom/client';
 import { BrowserRouter } from 'react-router-dom';
@@ -45,24 +46,26 @@ const emotionCache = createCache({
 });
 
 const app = (
-  <CacheProvider value={emotionCache}>
-    <ThemeProvider theme={theme}>
-      <Provider store={store}>
-        <BrowserRouter>
-          <QueryParamProvider
-            adapter={ReactRouter5Adapter}
-            options={{
-              searchStringToObject: querystring.parse,
-              objectToSearchString: (object: Record<string, any>) =>
-                querystring.stringify(object, { encode: false }),
-            }}
-          >
-            <Menu data={menu} />
-          </QueryParamProvider>
-        </BrowserRouter>
-      </Provider>
-    </ThemeProvider>
-  </CacheProvider>
+  <StrictMode>
+    <CacheProvider value={emotionCache}>
+      <ThemeProvider theme={theme}>
+        <Provider store={store}>
+          <BrowserRouter>
+            <QueryParamProvider
+              adapter={ReactRouter5Adapter}
+              options={{
+                searchStringToObject: querystring.parse,
+                objectToSearchString: (object: Record<string, any>) =>
+                  querystring.stringify(object, { encode: false }),
+              }}
+            >
+              <Menu data={menu} />
+            </QueryParamProvider>
+          </BrowserRouter>
+        </Provider>
+      </ThemeProvider>
+    </CacheProvider>
+  </StrictMode>
 );
 
 const menuMountPoint = document.getElementById('app-menu');

superset/app.py

@@ -71,6 +71,22 @@ def create_app(
             # value of app_root so things work out of the box
             if not app.config["STATIC_ASSETS_PREFIX"]:
                 app.config["STATIC_ASSETS_PREFIX"] = app_root
+            # Prefix APP_ICON path with subdirectory root for subdirectory deployments
+            if (
+                app.config.get("APP_ICON", "").startswith("/static/")
+                and app_root != "/"
+            ):
+                app.config["APP_ICON"] = f"{app_root}{app.config['APP_ICON']}"
+                # Also update theme tokens for subdirectory deployments
+                for theme_key in ("THEME_DEFAULT", "THEME_DARK"):
+                    theme = app.config[theme_key]
+                    token = theme.get("token", {})
+                    # Update brandLogoUrl if it points to /static/
+                    if token.get("brandLogoUrl", "").startswith("/static/"):
+                        token["brandLogoUrl"] = f"{app_root}{token['brandLogoUrl']}"
+                    # Update brandLogoHref if it's the default "/"
+                    if token.get("brandLogoHref") == "/":
+                        token["brandLogoHref"] = app_root
             if app.config["APPLICATION_ROOT"] == "/":
                 app.config["APPLICATION_ROOT"] = app_root
 

superset/mcp_service/auth.py

@@ -516,47 +516,6 @@ def _cleanup_session_on_error() -> None:
         logger.warning("Error cleaning up session after exception: %s", e)
 
 
-def _remove_session_safe() -> None:
-    """Remove the scoped SQLAlchemy session, tolerating SSL/connection errors.
-
-    Thread-pool workers reuse threads across requests.  Before each tool call
-    the session is removed to prevent a prior request's thread-local session
-    from leaking into the next one.  If the underlying DBAPI connection died
-    between requests (e.g. RDS SSL idle-timeout or max-connection-age), the
-    rollback implicit in ``session.close()`` raises a ``DBAPIError`` subclass
-    (``OperationalError`` for psycopg2, ``InterfaceError`` for some other
-    drivers).
-
-    When that happens:
-    1. Invalidate the dead connection so the pool discards it (rather than
-       returning a broken connection to the next caller).
-    2. Retry ``remove()`` to deregister the session from the scoped registry.
-
-    The tool call still proceeds because a fresh connection will be obtained
-    on the next DB access.
-    """
-    from sqlalchemy.exc import DBAPIError
-
-    from superset.extensions import db
-
-    try:
-        db.session.remove()
-    except DBAPIError as exc:
-        logger.warning(
-            "Connection error during pre-call session cleanup "
-            "(likely SSL/idle timeout); invalidating connection and retrying: %s",
-            exc,
-        )
-        try:
-            db.session.invalidate()
-        except Exception as invalidate_exc:
-            logger.debug(
-                "Could not invalidate session after connection error: %s",
-                invalidate_exc,
-            )
-        db.session.remove()  # retry: session deregisters cleanly after invalidation
-
-
 def mcp_auth_hook(tool_func: F) -> F:  # noqa: C901
     """
     Authentication and authorization decorator for MCP tools.
@@ -679,7 +638,9 @@ def mcp_auth_hook(tool_func: F) -> F:  # noqa: C901
                 # still be bound to a different tenant's DB engine. Removing it here
                 # ensures the next DB access creates a fresh session bound to the
                 # correct engine for the current request.
-                _remove_session_safe()
+                from superset.extensions import db
+
+                db.session.remove()
                 user = _setup_user_context()
 
                 # No Flask context - this is a FastMCP internal operation

superset/mcp_service/chart/tool/get_chart_data.py

@@ -46,10 +46,7 @@ from superset.mcp_service.chart.schemas import (
     GetChartDataRequest,
     PerformanceMetadata,
 )
-from superset.mcp_service.utils import (
-    escape_llm_context_delimiters,
-    sanitize_for_llm_context,
-)
+from superset.mcp_service.utils import sanitize_for_llm_context
 from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
 from superset.mcp_service.utils.oauth2_utils import (
     build_oauth2_redirect_message,
@@ -202,12 +199,8 @@ async def get_chart_data(  # noqa: C901
 
         if not chart:
             await ctx.warning("Chart not found: identifier=%s" % (request.identifier,))
-            safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
             return ChartError(
-                error=(
-                    f"No chart found with identifier: {safe_id}."
-                    " Use list_charts to get valid chart IDs."
-                ),
+                error=f"No chart found with identifier: {request.identifier}",
                 error_type="NotFound",
             )
 

superset/mcp_service/chart/tool/get_chart_preview.py

@@ -47,10 +47,7 @@ from superset.mcp_service.chart.schemas import (
     URLPreview,
     VegaLitePreview,
 )
-from superset.mcp_service.utils import (
-    escape_llm_context_delimiters,
-    sanitize_for_llm_context,
-)
+from superset.mcp_service.utils import sanitize_for_llm_context
 from superset.mcp_service.utils.oauth2_utils import (
     build_oauth2_redirect_message,
     OAUTH2_CONFIG_ERROR_MESSAGE,
@@ -1204,22 +1201,8 @@ async def _get_chart_preview_internal(  # noqa: C901
 
         if not chart:
             await ctx.warning("Chart not found: identifier=%s" % (request.identifier,))
-            is_form_data_key = (
-                isinstance(request.identifier, str)
-                and len(request.identifier) > 8
-                and not request.identifier.isdigit()
-            )
-            if is_form_data_key:
-                recovery = (
-                    "If using a form_data_key, it may have expired — "
-                    "use generate_explore_link to get a fresh key, "
-                    "or use list_charts to find a saved chart by ID."
-                )
-            else:
-                recovery = "Use list_charts to get valid chart IDs."
-            safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
             return ChartError(
-                error=f"No chart found with identifier: {safe_id}. {recovery}",
+                error=f"No chart found with identifier: {request.identifier}",
                 error_type="NotFound",
             )
 

superset/mcp_service/chart/tool/update_chart.py

@@ -47,7 +47,6 @@ from superset.mcp_service.chart.schemas import (
     PerformanceMetadata,
     UpdateChartRequest,
 )
-from superset.mcp_service.utils import escape_llm_context_delimiters
 from superset.mcp_service.utils.oauth2_utils import (
     build_oauth2_redirect_message,
     OAUTH2_CONFIG_ERROR_MESSAGE,
@@ -338,18 +337,17 @@ async def update_chart(  # noqa: C901
             chart = find_chart_by_identifier(request.identifier)
 
         if not chart:
-            safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
-            not_found_msg = (
-                f"No chart found with identifier: {safe_id}."
-                " Use list_charts to get valid chart IDs."
-            )
             return GenerateChartResponse.model_validate(
                 {
                     "chart": None,
                     "error": {
                         "error_type": "NotFound",
-                        "message": not_found_msg,
-                        "details": not_found_msg,
+                        "message": (
+                            f"No chart found with identifier: {request.identifier}"
+                        ),
+                        "details": (
+                            f"No chart found with identifier: {request.identifier}"
+                        ),
                     },
                     "success": False,
                     "schema_version": "2.0",

superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py

@@ -334,10 +334,7 @@ def _find_and_authorize_dashboard(
             dashboard=None,
             dashboard_url=None,
             position=None,
-            error=(
-                f"Dashboard with ID {dashboard_id} not found."
-                " Use list_dashboards to get valid dashboard IDs."
-            ),
+            error=f"Dashboard with ID {dashboard_id} not found",
         )
 
     try:
@@ -395,10 +392,7 @@ def add_chart_to_existing_dashboard(
                     dashboard=None,
                     dashboard_url=None,
                     position=None,
-                    error=(
-                        f"Chart with ID {request.chart_id} not found."
-                        " Use list_charts to get valid chart IDs."
-                    ),
+                    error=f"Chart with ID {request.chart_id} not found",
                 )
 
             # Validate dataset access for the chart.

superset/mcp_service/dashboard/tool/generate_dashboard.py

@@ -230,10 +230,7 @@ def generate_dashboard(  # noqa: C901
                 return GenerateDashboardResponse(
                     dashboard=None,
                     dashboard_url=None,
-                    error=(
-                        f"Charts not found: {sorted(missing_chart_ids)}."
-                        " Use list_charts to get valid chart IDs."
-                    ),
+                    error=f"Charts not found: {list(missing_chart_ids)}",
                 )
 
             # Validate dataset access for each chart.

superset/mcp_service/dataset/tool/query_dataset.py

@@ -183,10 +183,7 @@ async def query_dataset(  # noqa: C901
         if dataset is None:
             await ctx.error("Dataset not found: identifier=%s" % (request.dataset_id,))
             return DatasetError.create(
-                error=(
-                    f"No dataset found with identifier: {request.dataset_id}."
-                    " Use list_datasets to get valid dataset IDs."
-                ),
+                error=f"No dataset found with identifier: {request.dataset_id}",
                 error_type="NotFound",
             )
 

superset/mcp_service/sql_lab/tool/execute_sql.py

@@ -100,10 +100,7 @@ async def execute_sql(request: ExecuteSqlRequest, ctx: Context) -> ExecuteSqlRes
                 )
                 return ExecuteSqlResponse(
                     success=False,
-                    error=(
-                        f"Database with ID {request.database_id} not found."
-                        " Use list_databases to get valid database IDs."
-                    ),
+                    error=f"Database with ID {request.database_id} not found",
                     error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR.value,
                 )
 

superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py

@@ -103,8 +103,7 @@ def open_sql_lab_with_context(
             database = DatabaseDAO.find_by_id(request.database_connection_id)
         if not database:
             error_message = (
-                f"Database with ID {request.database_connection_id} not found."
-                " Use list_databases to get valid database IDs."
+                f"Database with ID {request.database_connection_id} not found"
             )
             return _sanitize_sql_lab_response_for_llm_context(
                 SqlLabResponse(

superset/utils/encrypt.py

@@ -99,29 +99,14 @@ class SecretsMigrator:
 
     def discover_encrypted_fields(self) -> dict[str, dict[str, EncryptedType]]:
         """
-        Iterates over ORM-mapped tables, looking for EncryptedType columns
-        along the way. Builds up a dict of
+        Iterates over SqlAlchemy's metadata, looking for EncryptedType
+        columns along the way. Builds up a dict of
         table_name -> dict of col_name: enc type instance
-
-        Superset's ORM models inherit from Flask-AppBuilder's declarative base
-        (`flask_appbuilder.Model`), whose MetaData is distinct from
-        `db.metadata`. We combine both sources so encrypted columns are found
-        regardless of which base a model uses. FAB's metadata takes precedence
-        when a table name appears in both registries.
-
-        :return: mapping of table name to a dict of {column name: EncryptedType}
+        :return:
         """
-        from flask_appbuilder import (  # pylint: disable=import-outside-toplevel
-            Model as FABModel,
-        )
-
         meta_info: dict[str, Any] = {}
 
-        tables: dict[str, Any] = dict(FABModel.metadata.tables)
         for table_name, table in self._db.metadata.tables.items():
-            tables.setdefault(table_name, table)
-
-        for table_name, table in tables.items():
             for col_name, col in table.columns.items():
                 if isinstance(col.type, EncryptedType):
                     cols = meta_info.get(table_name, {})

tests/integration_tests/utils/encrypt_tests.py

@@ -89,21 +89,6 @@ class EncryptedFieldTest(SupersetTestCase):
                         " encrypted_field_factory"
                     )
 
-    def test_discover_encrypted_fields_finds_dbs_table(self):
-        """
-        Ensure discover_encrypted_fields finds the encrypted columns on the
-        dbs table (password, encrypted_extra, server_cert). This guards
-        against db.metadata diverging from db.Model.metadata.
-        """
-        migrator = SecretsMigrator("")
-        encrypted_fields = migrator.discover_encrypted_fields()
-        assert "dbs" in encrypted_fields, (
-            "dbs table not found in encrypted fields — "
-            "discover_encrypted_fields may be using the wrong MetaData instance"
-        )
-        dbs_cols = set(encrypted_fields["dbs"].keys())
-        assert {"password", "encrypted_extra", "server_cert"}.issubset(dbs_cols)
-
     def test_lazy_key_resolution(self):
         """
         Verify that the encryption key is resolved lazily at runtime,

tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py

@@ -298,8 +298,7 @@ class TestOpenSqlLabWithContext:
                 field_path=("title",),
             )
             assert response.error == sanitize_for_llm_context(
-                "Database with ID 404 not found."
-                " Use list_databases to get valid database IDs.",
+                "Database with ID 404 not found",
                 field_path=("error",),
             )
         finally:

tests/unit_tests/mcp_service/test_auth_user_resolution.py

@@ -409,50 +409,6 @@ def test_mcp_auth_hook_removes_stale_db_session_in_sync_wrapper(app) -> None:
     assert result == "fresh"
 
 
-def test_sync_wrapper_handles_ssl_error_on_pre_call_remove(app) -> None:
-    """sync_wrapper tolerates OperationalError from db.session.remove() before the call.
-
-    If the underlying DBAPI connection died between requests (e.g. RDS SSL
-    idle-timeout), the rollback implicit in session.close() raises
-    OperationalError.  _remove_session_safe() should:
-    - Log a warning
-    - Call session.invalidate() to mark the dead connection for pool discard
-    - Retry session.remove() so the registry is clean
-    - Allow the tool to run successfully
-    """
-    from sqlalchemy.exc import OperationalError as SAOperationalError
-
-    fresh_user = _make_mock_user("fresh")
-
-    def dummy_tool() -> str:
-        """Dummy sync tool."""
-        return g.user.username
-
-    wrapped = mcp_auth_hook(dummy_tool)
-
-    with app.test_request_context():
-        g.user = fresh_user
-        with patch("superset.extensions.db") as mock_db:
-            mock_db.session.remove.side_effect = [
-                SAOperationalError(
-                    "SSL connection has been closed unexpectedly", None, None
-                ),
-                None,  # second call succeeds
-            ]
-
-            with patch(
-                "superset.mcp_service.auth.get_user_from_request",
-                return_value=fresh_user,
-            ):
-                result = wrapped()
-
-    assert result == "fresh"
-    assert mock_db.session.invalidate.called, "invalidate() must be called on SSL error"
-    assert mock_db.session.remove.call_count == 2, (
-        "remove() must be retried after SSL error"
-    )
-
-
 # -- default_user_resolver --