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


##########
superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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.
+ */
+
+/**
+ * Escapes RegExp metacharacters in a string so it can be safely used in a
+ * dynamically created regular expression.
+ *
+ * @param value - The raw string to escape
+ * @returns The escaped string safe for use in `new RegExp(...)`
+ */
+const escapeRegex = (value: string) =>
+  value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+
+/**
+ * Replaces a label inside a compound key only if it appears as a complete
+ * word/token and contains at least one alphabetic character.
+ *
+ * @param key - The source string (typically a compound field name)
+ * @param label - The label to search for as a standalone token
+ * @param replacement - The human-readable value to replace the label with
+ * @returns The transformed key if a valid match exists, otherwise the 
original key
+ */
+const replaceLabelIfExists = (
+  key: string,
+  label: string,
+  replacement: string,
+) => {
+  /**
+   * Logic:
+   *
+   * This function is intentionally stricter than a simple substring replace:
+   * - The label must NOT be part of a larger word (e.g. "account" will NOT 
match
+   *   "testing_account").
+   * - Underscores (`_`) are treated as part of the word.
+   * - Numeric-only matches are ignored (e.g. "12" will NOT match "123").
+   *
+   * If the label is found, only the matched portion is replaced; otherwise,
+   * the original key is returned unchanged.
+   *
+   * Examples:
+   * - replaceLabelIfExists("testing_account 123", "testing_account", 
"Account")
+   *   → "Account 123"
+   * - replaceLabelIfExists("testing_account 123", "account", "Account")
+   *   → "testing_account 123"
+   * - replaceLabelIfExists("123", "12", "X")
+   *   → "123"
+   */
+
+  if (key === label) {
+    return replacement;
+  }
+
+  const escapedLabel = escapeRegex(label);
+  const regex = new RegExp(`(?<!\\w)${escapedLabel}(?!\\w)`, 'g');
+  return regex.test(key) ? key.replace(regex, replacement) : key;

Review Comment:
   **Suggestion:** Logic bug & portability: `replaceLabelIfExists` claims to 
ignore numeric-only labels and to match only tokens containing alphabetic 
characters, but it never checks for alphabetic characters in `label`. Also it 
uses a RegExp with a lookbehind `(?<!\w)`, which can throw a SyntaxError in JS 
environments that don't support lookbehind. Add an explicit alphabetic check 
for `label` and replace lookbehind with a look-for-prefix capturing group so 
the function behaves as documented and works in older runtimes. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Module load can throw SyntaxError in older JS engines.
   - ❌ Column/header humanization may be incorrect for numeric labels.
   - ⚠️ Breaks any UI code importing this util at runtime.
   ```
   </details>
   
   ```suggestion
      */
   
     if (key === label) {
       return replacement;
     }
   
     // Ignore numeric-only labels (documented behavior)
     if (!/[A-Za-z]/.test(label)) {
       return key;
     }
   
     const escapedLabel = escapeRegex(label);
     // Avoid using lookbehind for broader runtime compatibility.
     // Use a capturing prefix group to ensure the label is not preceded by a 
word char.
     const regex = new RegExp(`(^|[^A-Za-z0-9_])${escapedLabel}(?!\\w)`, 'g');
     return regex.test(key)
       ? key.replace(regex, (_match, prefix) => `${prefix}${replacement}`)
       : key;
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. In a runtime that loads the frontend bundle, the module at
   
      
superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap.ts
   
      is evaluated. The regex construction occurs at line 70 (`const regex = new
      RegExp(...)`)
   
      inside the function `replaceLabelIfExists` (lines 39-72).
   
   2. If the environment's JS engine does not support ES2018 lookbehind (for 
example,
   
      older Node/browsers without lookbehind support), the RegExp literal string
   
      containing `(?<!\\w)` will cause a SyntaxError when `new RegExp(...)` is 
executed
   
      (see line 70). This prevents the module from functioning or being 
required.
   
   3. Independently, call the exported function `addLabelMapToVerboseMap` (line 
81)
   
      with inputs that include a numeric-only label:
   
      - Example call (in a small repro script): import addLabelMapToVerboseMap 
from
   
        
'superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap';
   
        addLabelMapToVerboseMap({ '123 12': ['12'] }, { '12': 'Twelve' });
   
      The call path reaches the loop that invokes `replaceLabelIfExists` at 
lines 138-143,
   
      which will construct the lookbehind-based RegExp and attempt to replace 
"12" inside
      "123 12".
   
   4. Expected (per function documentation): numeric-only labels should be 
ignored.
   
      Actual: because there is no alphabetic check, the current implementation 
will
   
      attempt the replacement (or fail earlier with SyntaxError in 
lookbehind-unsupported
      runtimes).
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap.ts
   **Line:** 55:71
   **Comment:**
        *Possible Bug: Logic bug & portability: `replaceLabelIfExists` claims 
to ignore numeric-only labels and to match only tokens containing alphabetic 
characters, but it never checks for alphabetic characters in `label`. Also it 
uses a RegExp with a lookbehind `(?<!\w)`, which can throw a SyntaxError in JS 
environments that don't support lookbehind. Add an explicit alphabetic check 
for `label` and replace lookbehind with a look-for-prefix capturing group so 
the function behaves as documented and works in older runtimes.
   
   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.
   ```
   </details>



##########
superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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.
+ */
+
+/**
+ * Escapes RegExp metacharacters in a string so it can be safely used in a
+ * dynamically created regular expression.
+ *
+ * @param value - The raw string to escape
+ * @returns The escaped string safe for use in `new RegExp(...)`
+ */
+const escapeRegex = (value: string) =>
+  value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+
+/**
+ * Replaces a label inside a compound key only if it appears as a complete
+ * word/token and contains at least one alphabetic character.
+ *
+ * @param key - The source string (typically a compound field name)
+ * @param label - The label to search for as a standalone token
+ * @param replacement - The human-readable value to replace the label with
+ * @returns The transformed key if a valid match exists, otherwise the 
original key
+ */
+const replaceLabelIfExists = (
+  key: string,
+  label: string,
+  replacement: string,
+) => {
+  /**
+   * Logic:
+   *
+   * This function is intentionally stricter than a simple substring replace:
+   * - The label must NOT be part of a larger word (e.g. "account" will NOT 
match
+   *   "testing_account").
+   * - Underscores (`_`) are treated as part of the word.
+   * - Numeric-only matches are ignored (e.g. "12" will NOT match "123").
+   *
+   * If the label is found, only the matched portion is replaced; otherwise,
+   * the original key is returned unchanged.
+   *
+   * Examples:
+   * - replaceLabelIfExists("testing_account 123", "testing_account", 
"Account")
+   *   → "Account 123"
+   * - replaceLabelIfExists("testing_account 123", "account", "Account")
+   *   → "testing_account 123"
+   * - replaceLabelIfExists("123", "12", "X")
+   *   → "123"
+   */
+
+  if (key === label) {
+    return replacement;
+  }
+
+  const escapedLabel = escapeRegex(label);
+  const regex = new RegExp(`(?<!\\w)${escapedLabel}(?!\\w)`, 'g');
+  return regex.test(key) ? key.replace(regex, replacement) : key;
+};
+
+/**
+ * Enriches the verbose map by creating human-readable versions of compound 
field names.
+ *
+ * @param label_map — a mapping of compound keys to arrays of component labels 
(e.g., { "revenue_total_usd": ["revenue", "total", "usd"] })
+ * @param verboseMap — the existing mapping of field names to their display 
labels
+ * @returns an updated verbose map that includes human-readable versions of 
the compound keys
+ */
+const addLabelMapToVerboseMap = (
+  label_map: Record<string, string[]>,
+  verboseMap: Record<string, string> = {},
+): Record<string, string> => {
+  /**
+   * Logic:
+   *
+   * This function takes a mapping of compound field names to their component 
labels
+   * and replaces those labels with their corresponding human-readable values 
from
+   * `verboseMap`, producing display-friendly versions of the compound keys.
+   *
+   * Replacement behavior:
+   * - Each compound key is processed word-by-word (split on spaces).
+   * - Only labels that exist in `verboseMap` are considered.
+   * - Each word is replaced at most once, using `replaceLabelIfExists`, which:
+   *   - Matches only full tokens (no partial matches).
+   *   - Treats underscores (`_`) as part of a token.
+   *   - Is case-sensitive.
+   * - Labels not found in `verboseMap` are left unchanged.
+   *
+   * The original `verboseMap` is preserved and extended with the newly 
generated
+   * human-readable entries.
+   *
+   * Example:
+   * ```ts
+   * label_map = {
+   *   "testing_count, 1 week ago": ["testing_count", "1 week ago"]
+   * }
+   *
+   * verboseMap = {
+   *   testing_count: "Testing Count"
+   * }
+   *
+   * Result:
+   * {
+   *   testing_count: "Testing Count",
+   *   "testing_count, 1 week ago": "Testing Count, 1 week ago"
+   * }
+   * ```
+   */
+  const newVerboseMap: Record<string, string> = {};
+
+  Object.entries(label_map).forEach(([key, labels]) => {
+    if (labels) {
+      const newLabelMap: Record<string, string> = labels
+        .filter(l => verboseMap[l])
+        .reduce(
+          (acc, label) => ({
+            ...acc,
+            [label]: verboseMap[label],
+          }),
+          {},
+        );
+
+      const newKey = key
+        .split(' ')
+        .map(word => {
+          for (const label of Object.keys(newLabelMap)) {
+            const newWord = replaceLabelIfExists(
+              word,
+              label,
+              newLabelMap[label],
+            );
+
+            if (newWord !== word) {
+              return newWord;
+            }
+          }
+
+          return word;
+        })
+        .join(' ');

Review Comment:
   **Suggestion:** Logic bug: the code splits the compound `key` on spaces and 
attempts per-word replacements, which prevents matching multi-word labels 
(e.g., "1 week ago") and can produce incorrect results. Replace per-word 
mapping with applying `replaceLabelIfExists` across the entire `key` for each 
label to correctly handle multi-word labels and punctuation. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Multi-word timeframe labels not humanized.
   - ❌ Dashboard column headers show raw tokens.
   - ⚠️ Affects any UI that consumes verboseMap for display.
   ```
   </details>
   
   ```suggestion
         let newKey = key;
         for (const label of Object.keys(newLabelMap)) {
           newKey = replaceLabelIfExists(newKey, label, newLabelMap[label]);
         }
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. The code that builds `newKey` is at lines 135-152 inside 
addLabelMapToVerboseMap
   
      
(superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap.ts).
   
      It splits the compound key on spaces (line 136) and attempts per-word 
replacement.
   
   2. Reproduce by calling addLabelMapToVerboseMap with:
   
      - label_map = { "testing_count, 1 week ago": ["testing_count", "1 week 
ago"] }
   
      - verboseMap = { testing_count: "Testing Count", "1 week ago": "1 week 
ago (label)" }
   
   3. Execution splits the key "testing_count, 1 week ago" into words and 
passes each single
   word
   
      into replaceLabelIfExists (lines 136-151). The multi-word label "1 week 
ago" is never
   
      matched because it's split across words, so it will not be replaced in 
the generated
      entry.
   
   4. Expected: "Testing Count, 1 week ago (label)". Actual: only single-word 
replacements
   applied,
   
      multi-word components remain unchanged. This reproduces incorrect header 
text
      generation.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/packages/superset-ui-core/src/utils/addLabelMapToVerboseMap.ts
   **Line:** 135:152
   **Comment:**
        *Logic Error: Logic bug: the code splits the compound `key` on spaces 
and attempts per-word replacements, which prevents matching multi-word labels 
(e.g., "1 week ago") and can produce incorrect results. Replace per-word 
mapping with applying `replaceLabelIfExists` across the entire `key` for each 
label to correctly handle multi-word labels and punctuation.
   
   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.
   ```
   </details>



##########
superset-frontend/spec/fixtures/mockTimeSeries.ts:
##########
@@ -0,0 +1,269 @@
+/**
+ * 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 { DatasourceType } from '@superset-ui/core';
+import { EchartsTimeseriesChartProps } from 
'@superset-ui/plugin-chart-echarts';
+import { EchartsMixedTimeseriesProps } from 
'plugins/plugin-chart-echarts/src/MixedTimeseries/types';
+
+export const mockedTimeSeriesProps: EchartsTimeseriesChartProps = {
+  // Datasource metadata
+  datasource: {
+    id: 1,
+    name: 'test_datasource',
+    type: DatasourceType.Table,
+    columnFormats: {},
+    currencyFormats: {},
+    verboseMap: {
+      testing_count: 'Testing count',
+      'SUM(money_for_learning)': 'SUM(money_for_learning)',
+      time_start: 'time_start',
+    },
+  },
+
+  // Query results
+  queriesData: [
+    {
+      label_map: {
+        time_start: ['time_start'],
+        'SUM(money_for_learning)': ['SUM(money_for_learning)'],
+        'SUM(money_for_learning), 1 day ago': [
+          'SUM(money_for_learning)',
+          '1 day ago',
+        ],
+        'SUM(money_for_learning), 1 week ago': [
+          'SUM(money_for_learning)',
+          '1 week ago',
+        ],
+        'SUM(money_for_learning), 1 year ago': [
+          'SUM(money_for_learning)',
+          '1 year ago',
+        ],
+        testing_count: ['testing_count'],
+        'testing_count, 1 day ago': ['testing_count', '1 day ago'],
+        'testing_count, 1 week ago': ['testing_count', '1 week ago'],
+        'testing_count, 1 year ago': ['testing_count', '1 year ago'],
+        'Testing count': ['testing_count'],
+      },
+      colnames: [
+        'time_start',
+        'SUM(money_for_learning)',
+        'SUM(money_for_learning), 1 day ago',
+        'SUM(money_for_learning), 1 week ago',
+        'SUM(money_for_learning), 1 year ago',
+        'testing_count',
+        'testing_count, 1 day ago',
+        'testing_count, 1 week ago',
+        'testing_count, 1 year ago',
+      ],
+      indexnames: [0, 1, 2],
+      coltypes: [2, 0, 0, 0, 1, 0, 0, 0, 1],
+      data: [
+        {
+          time_start: 1533081600001,
+          'SUM(money_for_learning)': 101,
+          'SUM(money_for_learning), 1 day ago': null,
+          'SUM(money_for_learning), 1 week ago': null,
+          'SUM(money_for_learning), 1 year ago': null,
+          testing_count: 1,
+          'testing_count, 1 day ago': null,
+          'testing_count, 1 week ago': null,
+          'testing_count, 1 year ago': null,
+        },
+        {
+          time_start: 1533168000001,
+          'SUM(money_for_learning)': 5791,
+          'SUM(money_for_learning), 1 day ago': 101,
+          'SUM(money_for_learning), 1 week ago': null,
+          'SUM(money_for_learning), 1 year ago': null,
+          testing_count: 131,
+          'testing_count, 1 day ago': 11,
+          'testing_count, 1 week ago': null,
+          'testing_count, 1 year ago': null,
+        },
+      ],
+      result_format: 'json',
+      applied_filters: [
+        {
+          column: 'time_start',
+        },
+      ],
+      rejected_filters: [],
+    },
+  ],
+
+  // Filter and legend state
+  filterState: {
+    selectedValues: [],
+  },
+  legendState: {},
+
+  // Form data (chart configuration)
+  formData: {
+    metrics: [
+      {
+        aggregate: 'SUM',
+        column: {
+          column_name: 'money_for_learning',
+          filterable: true,
+          groupby: true,
+          id: 460,
+          is_dttm: false,
+          type: 'DOUBLE PRECISION',
+          type_generic: 0,
+        },
+        expressionType: 'SIMPLE',
+        hasCustomLabel: false,
+        label: 'SUM(money_for_learning)',
+        optionName: 'metric_olnflt1ak9g_z1vjg3dhmnf',
+      },
+      'testing_count',
+    ],
+  },
+
+  // Hooks
+  hooks: {
+    setDataMask: () => {},
+    setControlValue: () => {},
+    onContextMenu: () => {},
+    onLegendStateChanged: () => {},
+    onLegendScroll: () => {},
+  },
+
+  // UI state
+  inContextMenu: false,
+  emitCrossFilters: true,
+  legendIndex: 0,
+
+  // Raw form data (needed for additional validation)
+  rawFormData: {
+    datasource: '1__table',
+    viz_type: 'echarts_timeseries',
+    x_axis: 'ds',

Review Comment:
   **Suggestion:** The `rawFormData.x_axis` is set to `'ds'` but the query 
column in this fixture is `time_start`; this mismatch will cause code that 
looks up the x-axis column (for drilldown, labeling, or mapping) to not find 
the intended column and show the wrong header or fail to map drill context 
correctly. Set `x_axis` to `'time_start'` so it matches the actual column name 
in `colnames`/`verboseMap`. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Drill/context menu shows wrong column header.
   - ⚠️ Timeseries story/tests produce incorrect snapshots.
   ```
   </details>
   
   ```suggestion
       x_axis: 'time_start',
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open the fixture file at 
superset-frontend/spec/fixtures/mockTimeSeries.ts and inspect
   the rawFormData block (lines 153-158). Note x_axis is set to 'ds' (line 156) 
while the
   fixture's query column is named 'time_start' (see colnames at lines 62-72 
and verboseMap
   at lines 31-35).
   
   2. Any test or story that imports mockedTimeSeriesProps and mounts the 
Echarts
   Timeseries/MixedTimeseries component (the fixture is typed as 
EchartsTimeseriesChartProps
   / EchartsMixedTimeseriesProps via imports at lines 19-21) will pass this 
rawFormData into
   the chart's props.
   
   3. The chart code that maps x-axis values looks up the configured x_axis name
   (rawFormData.x_axis) against queriesData[0].colnames to find the column 
index (colnames
   present at lines 62-72). Because x_axis is 'ds' (not present in colnames), 
the lookup will
   fail and the chart will either fall back to a wrong column or omit the 
x-axis mapping.
   
   4. Observe the symptom: drill/context menu or column header displays the 
wrong header text
   (or drill payload references an incorrect/missing column). This reproduces 
the issue
   described in the PR title (drill by wrong column header text).
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/spec/fixtures/mockTimeSeries.ts
   **Line:** 156:156
   **Comment:**
        *Logic Error: The `rawFormData.x_axis` is set to `'ds'` but the query 
column in this fixture is `time_start`; this mismatch will cause code that 
looks up the x-axis column (for drilldown, labeling, or mapping) to not find 
the intended column and show the wrong header or fail to map drill context 
correctly. Set `x_axis` to `'time_start'` so it matches the actual column name 
in `colnames`/`verboseMap`.
   
   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.
   ```
   </details>



##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -85,11 +88,13 @@ export const useResultsPane = ({
         formData: queryFormData,
         force: queryForce,
         resultFormat: 'json',
-        resultType: 'results',
+        resultType: 'full',
         ownState,
       })
         .then(({ json }) => {
-          setResultResp(ensureIsArray(json.result));
+          const responseArray = ensureIsArray(json.result);
+          const transformedResponseArray = transformTableData(responseArray);
+          setResultResp(transformedResponseArray);
           setResponseError('');
           cache.set(queryFormData, json.result);
           if (queryForce) {

Review Comment:
   **Suggestion:** Inconsistent caching: the code stores the raw `json.result` 
into `cache` but sets the component state to the transformed result; on 
subsequent cache hits you will re-use the untransformed shape which likely 
doesn't match the component's expected shape and will cause rendering errors. 
Store the transformed array in the cache so read/write use the same data shape. 
[logic error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Explore DataTablesPane shows wrong table columns.
   - ❌ Drill-by headers render incorrectly in Explore.
   - ⚠️ Re-render with cached queries causes UI errors.
   ```
   </details>
   
   ```suggestion
               cache.set(queryFormData, transformedResponseArray);
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open the Explore view that renders DataTablesPane which imports this hook:
   
      
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx.
   
   2. Trigger a query such that isRequest === true and cache does NOT yet have 
the key:
   
      the hook enters the code path at the getChartDataRequest call 
(useResultsPane.tsx lines
      ~88-96)
   
      and executes the .then handler at lines 95-99 where cache.set(...) is 
called.
   
   3. The .then handler (useResultsPane.tsx:95-99) transforms json.result via
   transformTableData()
   
      and calls setResultResp(transformedResponseArray) but stores the 
untransformed
      json.result
   
      in cache via cache.set(queryFormData, json.result).
   
   4. Later (same session) when the same queryFormData is used, the early 
cache-read branch
   runs
   
      (useResultsPane.tsx — the useEffect branch that checks if (isRequest &&
      cache.has(queryFormData))),
   
      calling setResultResp(ensureIsArray(cache.get(queryFormData))). Because 
the cached
      value is the
   
      raw json.result (untransformed) the component receives a different data 
shape than it
      expects,
   
      producing mismatched colnames/coltypes or rendering errors in 
SingleQueryResultPane.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx
   **Line:** 100:100
   **Comment:**
        *Logic Error: Inconsistent caching: the code stores the raw 
`json.result` into `cache` but sets the component state to the transformed 
result; on subsequent cache hits you will re-use the untransformed shape which 
likely doesn't match the component's expected shape and will cause rendering 
errors. Store the transformed array in the cache so read/write use the same 
data shape.
   
   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.
   ```
   </details>



##########
superset-frontend/src/explore/components/DataTablesPane/utils.ts:
##########
@@ -0,0 +1,78 @@
+/**
+ * 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 { ensureIsArray, addLabelMapToVerboseMap } from '@superset-ui/core';

Review Comment:
   **Suggestion:** The import of `ensureIsArray` uses a named import but the 
utility is exported as a default from its module; this can cause 
`ensureIsArray` to be undefined at runtime. Import the default and the named 
export correctly so both functions are available. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ DataTablesPane column labels throw TypeError.
   - ⚠️ Explore UI table formatting fails to apply labels.
   - ⚠️ Any view calling transformTableData affected.
   ```
   </details>
   
   ```suggestion
   import ensureIsArray, { addLabelMapToVerboseMap } from '@superset-ui/core';
   ```
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open the file 
superset-frontend/src/explore/components/DataTablesPane/utils.ts and note
   the export of transformTableData at line 28. (file: utils.ts, line 28)
   
   2. In a unit test or browser console, call transformTableData with a 
responseArray that
   contains a label_map entry (any object with label_map present). The call 
site is the
   exported function transformTableData in utils.ts:28.
   
   3. During execution, transformTableData iterates labelMap at lines 41-46 and 
calls
   ensureIsArray(value) at line 42 (Object.entries(labelMap).filter(...)). 
Because
   ensureIsArray is imported as a named import (line 19) but the actual module
   (packages/superset-ui-core/src/utils/ensureIsArray.ts) exports it as default,
   ensureIsArray will be undefined and the call at utils.ts:42 throws TypeError:
   ensureIsArray is not a function.
   
   4. The TypeError surfaces while formatting column labels in DataTablesPane 
(utils.ts),
   breaking table label processing and producing a stack trace pointing to
   superset-frontend/src/explore/components/DataTablesPane/utils.ts lines 
~41-46. This
   reproduces reliably whenever transformTableData is invoked with a response 
that has a
   label_map.
   ```
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset-frontend/src/explore/components/DataTablesPane/utils.ts
   **Line:** 19:19
   **Comment:**
        *Type Error: The import of `ensureIsArray` uses a named import but the 
utility is exported as a default from its module; this can cause 
`ensureIsArray` to be undefined at runtime. Import the default and the named 
export correctly so both functions are available.
   
   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.
   ```
   </details>



-- 
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