codeant-ai-for-open-source[bot] commented on code in PR #41006:
URL: https://github.com/apache/superset/pull/41006#discussion_r3405745519


##########
superset-frontend/src/core/chat/index.ts:
##########
@@ -0,0 +1,238 @@
+/**
+ * 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.
+ */
+
+/**
+ * @fileoverview Host implementation of the `chat` contribution type.
+ *
+ * Chat is a dedicated contribution type, not a view: extensions register via
+ * the public `chat.registerChat()` and the host owns mounting, open/close
+ * state, and the display mode. Multiple chat extensions may register, but the
+ * host applies singleton resolution — the most-recently-registered chat is
+ * active; disposing it falls back to the previous one.
+ *
+ * Open-state policy across active-chat transitions: when the active chat's
+ * identity changes — a takeover by a different id, disposal falling back to a
+ * different id, or disposal of the last chat — the panel is closed (firing
+ * `onDidClose`) so the incoming chat never mounts into an open state it did
+ * not request. A same-id re-registration is an upgrade in place and keeps the
+ * open state.
+ *
+ * The public namespace (`chat`) is exposed to extensions on
+ * `window.superset`; the other exports are host-internal accessors for
+ * ChatMount and are NOT part of the public `@apache-superset/core` API.
+ */
+
+import { ReactElement } from 'react';
+import type { chat as chatApi } from '@apache-superset/core';
+import { Disposable } from '../models';
+import { createEmitter, createEventEmitter } from '../utils';
+
+type Chat = chatApi.Chat;
+type ChatMode = chatApi.ChatMode;
+
+/** A registered chat: its descriptor plus the host-mountable providers. */
+export interface RegisteredChat {
+  /** The chat descriptor passed to `registerChat`. */
+  chat: Chat;
+  /** Renders the collapsed bubble. Hidden by the host in panel mode. */
+  trigger: () => ReactElement;
+  /** Renders the chat panel, mounted per the current {@link ChatMode}. */
+  panel: () => ReactElement;
+  /**
+   * Unique per registration (a same-id re-registration gets a new one). The
+   * host UI keys mounts and fault containment on it, so a replacement resets
+   * crashed error boundaries instead of inheriting their latched state.
+   */
+  registrationId: number;
+}
+
+/**
+ * Immutable snapshot of the whole chat state, rebuilt on every change.
+ * Returned by reference from `getChatSnapshot` so `useSyncExternalStore`
+ * consumers read registrations, open state, and mode from one consistent
+ * object instead of tearing across separate live reads.
+ */
+export interface ChatSnapshot {
+  /** Monotonic change counter, useful as a memo/effect dependency. */
+  version: number;
+  /** Whether the active chat's panel is open. */
+  open: boolean;
+  /** The current display mode. */
+  mode: ChatMode;
+  /** The active registration, or undefined when none is registered. */
+  active: RegisteredChat | undefined;
+}
+
+/** Registration order is the singleton-resolution order: last entry wins. */
+const registrations: RegisteredChat[] = [];
+
+let panelOpen = false;
+let nextRegistrationId = 1;
+
+const registerEmitter = createEventEmitter<Chat>();
+const unregisterEmitter = createEventEmitter<Chat>();
+const openEmitter = createEventEmitter<Chat>();
+const closeEmitter = createEventEmitter<Chat>();
+const resizePanelEmitter = createEventEmitter<{ width: number }>();
+const modeEmitter = createEmitter<ChatMode>('floating');
+
+/**
+ * Host-internal: resolves the active chat with its providers.
+ * The most-recently-registered chat wins; when it is disposed the previous
+ * registration takes over the slot again.
+ */
+export const getActiveChat = (): RegisteredChat | undefined =>
+  registrations[registrations.length - 1];
+
+let snapshot: ChatSnapshot = {
+  version: 0,
+  open: false,
+  mode: modeEmitter.getCurrent(),
+  active: undefined,
+};
+
+const stateSubscribers = new Set<() => void>();
+
+const notifyState = () => {
+  snapshot = {
+    version: snapshot.version + 1,
+    open: panelOpen,
+    mode: modeEmitter.getCurrent(),
+    active: getActiveChat(),
+  };
+  stateSubscribers.forEach(fn => fn());
+};
+
+export const subscribeToChatState = (listener: () => void): (() => void) => {
+  stateSubscribers.add(listener);
+  return () => {
+    stateSubscribers.delete(listener);
+  };
+};
+
+export const getChatSnapshot = (): ChatSnapshot => snapshot;
+
+/** Closes the panel and fires `onDidClose` with the chat that was closed. */
+const closePanel = (closedChat: Chat) => {
+  panelOpen = false;
+  closeEmitter.fire(closedChat);
+};
+
+const registerChat: typeof chatApi.registerChat = (
+  chat: Chat,
+  trigger: () => ReactElement,
+  panel: () => ReactElement,
+): Disposable => {
+  const previousActive = getActiveChat();
+
+  // Re-registering an id replaces the previous entry and moves it to the
+  // most-recent position, mirroring the view registry's same-id semantics.
+  const existingIndex = registrations.findIndex(r => r.chat.id === chat.id);
+  if (existingIndex !== -1) {
+    registrations.splice(existingIndex, 1);
+  }

Review Comment:
   **Suggestion:** Re-registering the same chat id removes the previous 
registration from the registry but does not emit `onDidUnregisterChat` for the 
displaced registration. Subscribers relying on unregister events for cleanup 
will miss that lifecycle transition and can keep stale state/resources. Emit 
the unregister event for the replaced registration when it is removed. 
[incomplete implementation]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Extensions tracking chat lifecycle miss replacement unregistration.
   - ⚠️ Resource cleanup tied to unregister leaks on chat upgrades.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Observe the chat host implementation in
   `superset-frontend/src/core/chat/index.ts:137-186`, where `registerChat` 
manages the
   `registrations` stack and exposes `onDidUnregisterChat` via 
`unregisterEmitter`.
   
   2. In an extension (or a Jest test), subscribe to unregistration events using
   `chat.onDidUnregisterChat` from the public API
   (`superset-frontend/packages/superset-core/src/chat/index.ts:27-29`), 
implemented by
   `onDidUnregisterChat: unregisterEmitter.event` at 
`src/core/chat/index.ts:225-227`.
   
   3. Call `chat.registerChat({ id: 'acme.chat', name: 'Acme v1' }, trigger, 
panel)` and then
   immediately call `chat.registerChat({ id: 'acme.chat', name: 'Acme v2' }, 
trigger, panel)`
   again with the same `id`. The second call enters the same-id branch at
   `src/core/chat/index.ts:146-149`, where it finds the existing index and 
`splice`s the
   previous registration from `registrations` without firing 
`unregisterEmitter`.
   
   4. Verify that the `onDidUnregisterChat` listener never runs for the 
displaced
   registration: the only place that calls `unregisterEmitter.fire(chat)` is 
inside the
   `Disposable` returned from `registerChat` at 
`src/core/chat/index.ts:169-185`, but in the
   same-id re-registration case that `Disposable` later sees `index === -1` 
(because the
   entry was already spliced at line 148) and returns early at line 173, so the 
previous
   registration is effectively removed without any unregister event.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=24ab3ef20d9545938bf06728e3f1a86e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=24ab3ef20d9545938bf06728e3f1a86e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/core/chat/index.ts
   **Line:** 146:149
   **Comment:**
        *Incomplete Implementation: Re-registering the same chat id removes the 
previous registration from the registry but does not emit `onDidUnregisterChat` 
for the displaced registration. Subscribers relying on unregister events for 
cleanup will miss that lifecycle transition and can keep stale state/resources. 
Emit the unregister event for the replaced registration when it is removed.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41006&comment_hash=c453bb192f89f8add4fd78b873d7079eff8eae9c7c14c7445c8ecf6b3eb8bb39&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41006&comment_hash=c453bb192f89f8add4fd78b873d7079eff8eae9c7c14c7445c8ecf6b3eb8bb39&reaction=dislike'>👎</a>



##########
superset-frontend/src/core/chat/index.ts:
##########
@@ -0,0 +1,238 @@
+/**
+ * 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.
+ */
+
+/**
+ * @fileoverview Host implementation of the `chat` contribution type.
+ *
+ * Chat is a dedicated contribution type, not a view: extensions register via
+ * the public `chat.registerChat()` and the host owns mounting, open/close
+ * state, and the display mode. Multiple chat extensions may register, but the
+ * host applies singleton resolution — the most-recently-registered chat is
+ * active; disposing it falls back to the previous one.
+ *
+ * Open-state policy across active-chat transitions: when the active chat's
+ * identity changes — a takeover by a different id, disposal falling back to a
+ * different id, or disposal of the last chat — the panel is closed (firing
+ * `onDidClose`) so the incoming chat never mounts into an open state it did
+ * not request. A same-id re-registration is an upgrade in place and keeps the
+ * open state.
+ *
+ * The public namespace (`chat`) is exposed to extensions on
+ * `window.superset`; the other exports are host-internal accessors for
+ * ChatMount and are NOT part of the public `@apache-superset/core` API.
+ */
+
+import { ReactElement } from 'react';
+import type { chat as chatApi } from '@apache-superset/core';
+import { Disposable } from '../models';
+import { createEmitter, createEventEmitter } from '../utils';
+
+type Chat = chatApi.Chat;
+type ChatMode = chatApi.ChatMode;
+
+/** A registered chat: its descriptor plus the host-mountable providers. */
+export interface RegisteredChat {
+  /** The chat descriptor passed to `registerChat`. */
+  chat: Chat;
+  /** Renders the collapsed bubble. Hidden by the host in panel mode. */
+  trigger: () => ReactElement;
+  /** Renders the chat panel, mounted per the current {@link ChatMode}. */
+  panel: () => ReactElement;
+  /**
+   * Unique per registration (a same-id re-registration gets a new one). The
+   * host UI keys mounts and fault containment on it, so a replacement resets
+   * crashed error boundaries instead of inheriting their latched state.
+   */
+  registrationId: number;
+}
+
+/**
+ * Immutable snapshot of the whole chat state, rebuilt on every change.
+ * Returned by reference from `getChatSnapshot` so `useSyncExternalStore`
+ * consumers read registrations, open state, and mode from one consistent
+ * object instead of tearing across separate live reads.
+ */
+export interface ChatSnapshot {
+  /** Monotonic change counter, useful as a memo/effect dependency. */
+  version: number;
+  /** Whether the active chat's panel is open. */
+  open: boolean;
+  /** The current display mode. */
+  mode: ChatMode;
+  /** The active registration, or undefined when none is registered. */
+  active: RegisteredChat | undefined;
+}
+
+/** Registration order is the singleton-resolution order: last entry wins. */
+const registrations: RegisteredChat[] = [];
+
+let panelOpen = false;
+let nextRegistrationId = 1;
+
+const registerEmitter = createEventEmitter<Chat>();
+const unregisterEmitter = createEventEmitter<Chat>();
+const openEmitter = createEventEmitter<Chat>();
+const closeEmitter = createEventEmitter<Chat>();
+const resizePanelEmitter = createEventEmitter<{ width: number }>();
+const modeEmitter = createEmitter<ChatMode>('floating');
+
+/**
+ * Host-internal: resolves the active chat with its providers.
+ * The most-recently-registered chat wins; when it is disposed the previous
+ * registration takes over the slot again.
+ */
+export const getActiveChat = (): RegisteredChat | undefined =>
+  registrations[registrations.length - 1];
+
+let snapshot: ChatSnapshot = {
+  version: 0,
+  open: false,
+  mode: modeEmitter.getCurrent(),
+  active: undefined,
+};
+
+const stateSubscribers = new Set<() => void>();
+
+const notifyState = () => {
+  snapshot = {
+    version: snapshot.version + 1,
+    open: panelOpen,
+    mode: modeEmitter.getCurrent(),
+    active: getActiveChat(),
+  };
+  stateSubscribers.forEach(fn => fn());
+};
+
+export const subscribeToChatState = (listener: () => void): (() => void) => {
+  stateSubscribers.add(listener);
+  return () => {
+    stateSubscribers.delete(listener);
+  };
+};
+
+export const getChatSnapshot = (): ChatSnapshot => snapshot;
+
+/** Closes the panel and fires `onDidClose` with the chat that was closed. */
+const closePanel = (closedChat: Chat) => {
+  panelOpen = false;
+  closeEmitter.fire(closedChat);
+};
+
+const registerChat: typeof chatApi.registerChat = (
+  chat: Chat,
+  trigger: () => ReactElement,
+  panel: () => ReactElement,
+): Disposable => {
+  const previousActive = getActiveChat();
+
+  // Re-registering an id replaces the previous entry and moves it to the
+  // most-recent position, mirroring the view registry's same-id semantics.
+  const existingIndex = registrations.findIndex(r => r.chat.id === chat.id);
+  if (existingIndex !== -1) {
+    registrations.splice(existingIndex, 1);
+  }
+
+  const entry: RegisteredChat = {
+    chat,
+    trigger,
+    panel,
+    registrationId: nextRegistrationId,
+  };

Review Comment:
   **Suggestion:** The registry stores the incoming chat descriptor object by 
reference, so a caller can mutate that object after registration and change 
active chat metadata without any host event/state notification. This breaks 
snapshot immutability assumptions and can cause stale UI behavior. Store a 
copied descriptor when registering to decouple host state from caller-owned 
objects. [stale reference]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Chat snapshot may mutate without state version increments.
   - ⚠️ Extensions relying on immutable descriptors see inconsistent metadata.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. `registerChat` in `superset-frontend/src/core/chat/index.ts:137-159` 
builds a
   `RegisteredChat` entry by assigning the caller-provided `chat` descriptor 
directly into
   `entry.chat` (line 152) and pushing that entry into the `registrations` 
array (line 158).
   
   2. An extension calls the public `dashboard.registerChat` API (declared in
   `packages/superset-core/src/chat/index.ts:7-11`) with a mutable descriptor 
object, e.g.
   `const desc = { id: 'acme.chat', name: 'Acme v1' }; chat.registerChat(desc, 
trigger,
   panel);` — the host stores `desc` by reference in `registrations`.
   
   3. Later, the extension mutates `desc`, e.g. `desc.name = 'Acme v2'`, 
without going
   through any host API. Because `entry.chat` points to the same object, the 
active
   registration's descriptor in `registrations` and in the current 
`snapshot.active` (rebuilt
   via `notifyState` at `src/core/chat/index.ts:112-118`) changes in place, but 
no
   `notifyState()` call is made and `snapshot.version` does not increment.
   
   4. Consumers that rely on the snapshot being immutable — e.g. React code 
using
   `useSyncExternalStore(subscribeToChatState, getChatSnapshot)` in
   `superset-frontend/src/components/ChatMount/index.ts:47-55` or future 
extensions reading
   `chat.getChat()` (which returns a shallow copy of `active.chat` at
   `src/core/chat/index.ts:188-192`) — can observe descriptor fields changing 
without a
   corresponding snapshot version bump or lifecycle event, breaking assumptions 
that
   snapshots represent discrete, event-driven state changes.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9610281400704b1e93f642a7a705eb32&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9610281400704b1e93f642a7a705eb32&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/core/chat/index.ts
   **Line:** 151:156
   **Comment:**
        *Stale Reference: The registry stores the incoming chat descriptor 
object by reference, so a caller can mutate that object after registration and 
change active chat metadata without any host event/state notification. This 
breaks snapshot immutability assumptions and can cause stale UI behavior. Store 
a copied descriptor when registering to decouple host state from caller-owned 
objects.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41006&comment_hash=916af861c25569fc27ceede0b7e2623efc3ea37f5292feb5b6e393563d8cc4e5&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41006&comment_hash=916af861c25569fc27ceede0b7e2623efc3ea37f5292feb5b6e393563d8cc4e5&reaction=dislike'>👎</a>



##########
superset-frontend/src/core/dashboard/index.ts:
##########
@@ -0,0 +1,123 @@
+/**
+ * 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.
+ */
+
+/**
+ * Host-internal implementation of the `dashboard` namespace.
+ *
+ * Wraps Redux dashboardInfo and dataMask state and normalizes them into the
+ * stable `DashboardContext` contract. Extensions must not depend on the Redux
+ * slice structure directly.
+ */
+
+import type { dashboard as dashboardApi } from '@apache-superset/core';
+import type { DataMaskStateWithId } from '@superset-ui/core';
+import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate';
+import {
+  UPDATE_DATA_MASK,
+  SET_DATA_MASK_FOR_FILTER_CHANGES_COMPLETE,
+} from 'src/dataMask/actions';
+import { store, RootState } from 'src/views/store';
+import { AnyListenerPredicate } from '@reduxjs/toolkit';
+import getChartIdsFromLayout from 'src/dashboard/util/getChartIdsFromLayout';
+import { createActionListener } from '../utils';
+import { navigation } from '../navigation';
+
+type DashboardContext = dashboardApi.DashboardContext;
+type FilterValue = dashboardApi.FilterValue;
+type ChartSummary = NonNullable<DashboardContext['charts']>[number];
+
+function buildChartSummaries(state: RootState): ChartSummary[] {
+  const slices = state.sliceEntities?.slices ?? {};
+  const layout = state.dashboardLayout?.present ?? {};
+
+  // Only charts actually placed on the dashboard layout — `slices` can also
+  // hold entities that are not on the current dashboard.
+  return getChartIdsFromLayout(layout).map(chartId => {
+    const slice = slices[chartId];
+    return {
+      chartId,
+      chartName: slice?.slice_name ?? '',
+      vizType: slice?.viz_type ?? '',
+      datasourceId: slice?.datasource_id ?? null,
+      datasourceName: slice?.datasource_name ?? null,
+      // Tab-accurate visibility is a deferred phase; every chart on the
+      // dashboard is reported visible for now.
+      isVisible: true,
+    };
+  });
+}
+
+function buildDashboardContext(): DashboardContext | undefined {
+  if (navigation.getPageType() !== 'dashboard') return undefined;
+  // `store.getState()` is already typed as RootState, so the slices below are
+  // read with their real types — the host owns this normalization and must
+  // stay type-safe against slice reshapes.
+  const state = store.getState();
+  const info = state.dashboardInfo;
+  if (!info?.id) return undefined;
+
+  const nativeFilters = state.nativeFilters?.filters ?? {};
+  const dataMask: DataMaskStateWithId = state.dataMask ?? {};
+
+  const filters: FilterValue[] = Object.entries(dataMask)
+    .filter(([id, mask]) => {
+      if (!(id in nativeFilters)) return false;
+      const value = mask?.filterState?.value;
+      return value !== null && value !== undefined;
+    })
+    .map(([id, mask]) => {
+      const raw = mask.filterState?.value;
+      return {
+        filterId: id,
+        label: nativeFilters[id]?.name ?? id,
+        value: Array.isArray(raw) ? [...raw] : raw,
+      };

Review Comment:
   **Suggestion:** The returned filter `value` only clones arrays, but 
non-array object values are returned by reference from Redux state. If an 
extension mutates that object, it mutates store-backed state outside reducers 
and can corrupt dashboard filter state. Return a defensive copy for object 
values as well (for example via `structuredClone`/deep clone for plain 
objects). [stale reference]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Extensions can mutate Redux dataMask filter state directly.
   - ⚠️ Corrupted filter state may break dashboard filtering behavior.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. `buildDashboardContext` in 
`superset-frontend/src/core/dashboard/index.ts:66-99` reads
   `dataMask` from the Redux store (`store.getState()` at line 71) and builds 
the exported
   `filters` array from `Object.entries(dataMask)` at lines 78-91.
   
   2. For each entry it assigns `const raw = mask.filterState?.value;` at line 
85 and then
   sets `value: Array.isArray(raw) ? [...raw] : raw` at line 89. Arrays are 
defensively
   cloned, but any non-array object is passed through by reference into the
   `FilterValue.value` returned to extensions.
   
   3. Tests in `superset-frontend/src/core/dashboard/index.test.ts:22-31` 
explicitly verify
   that array values are defensive copies by mutating `ctx.filters[0].value` 
and checking
   that the underlying Redux `dataMask` array is unchanged, confirming the 
intent that
   `FilterValue.value` must not allow direct mutation of store state.
   
   4. If a filter's `filterState.value` is an object (e.g. a date range `{ 
start, end }` or a
   complex selection), calling `dashboard.getCurrentDashboard()` (implemented at
   `src/core/dashboard/index.ts:106-107`) returns a `DashboardContext` whose
   `filters[0].value` is that same object reference; if an extension then 
mutates
   `ctx.filters[0].value` (for example, `ctx.filters[0].value.start = 
'2024-01-01'`), the
   Redux state at `store.getState().dataMask[filterId].filterState.value` will 
also change,
   because no defensive clone is made for object values in 
`buildDashboardContext`.
   ```
   </details>
   
   [Fix in 
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f4c259d280fd4f51aeca9572cf08b3a5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 | [Fix in VSCode 
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f4c259d280fd4f51aeca9572cf08b3a5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/core/dashboard/index.ts
   **Line:** 85:90
   **Comment:**
        *Stale Reference: The returned filter `value` only clones arrays, but 
non-array object values are returned by reference from Redux state. If an 
extension mutates that object, it mutates store-backed state outside reducers 
and can corrupt dashboard filter state. Return a defensive copy for object 
values as well (for example via `structuredClone`/deep clone for plain objects).
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41006&comment_hash=a095b43988a36a21952f3f0317be3f3bc48c2a54c326cd0b8521d5e72bc88fdc&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41006&comment_hash=a095b43988a36a21952f3f0317be3f3bc48c2a54c326cd0b8521d5e72bc88fdc&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to