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


##########
superset-frontend/src/pages/DashboardBuilderV2/controlValueValidation.ts:
##########
@@ -0,0 +1,111 @@
+/**
+ * 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.
+ */
+
+/**
+ * The one path either half of the Properties panel (the schema-driven form
+ * and the JSON editor) writes a schema-controlled widget's control values
+ * through.
+ *
+ * Both representations edit the same `node.props`, but neither may write to
+ * it directly: a candidate is merged, sent to the backend's
+ * `Widget.validate_control_values` gate — the same gate the
+ * `set_widget_control_values` MCP tool commits through, reached here via its
+ * REST wrapper rather than a second, frontend-authored copy of its rules —
+ * and only committed to the store once that gate accepts it. A rejected
+ * candidate returns its errors and touches nothing: `node.props` is read
+ * here, never written to, until validation has already succeeded.
+ */
+import { SupersetClient } from '@superset-ui/core';
+import { provider } from 'src/core/dashboard/store';
+
+export type ControlValidationError = {
+  loc: (string | number)[];
+  message: string;
+};
+
+export type CommitPropsResult =
+  | { ok: true; values: Record<string, unknown> }
+  | { ok: false; errors: ControlValidationError[] };
+
+/**
+ * SupersetClient rejects a non-2xx response with the raw, unparsed `Response`
+ * object rather than an `Error`, so a plain `String(e)` yields the useless
+ * "[object Response]". Pull the actual `{message}`/`{errors:[...]}` body
+ * Superset sends back (same shape `chartData.ts` handles).
+ */
+export async function describeError(e: unknown): Promise<string> {
+  if (typeof Response !== 'undefined' && e instanceof Response) {
+    try {
+      const body = await e.clone().json();
+      const detail =
+        body?.message ??
+        (Array.isArray(body?.errors)
+          ? body.errors
+              .map((err: { message?: string }) => err.message)
+              .join('; ')
+          : undefined);
+      return detail
+        ? `${e.status} ${e.statusText}: ${detail}`
+        : `${e.status} ${e.statusText}`;
+    } catch {
+      return `${e.status} ${e.statusText}`;
+    }
+  }
+  return e instanceof Error ? e.message : String(e);
+}
+
+async function validateControlValues(
+  widgetType: string,
+  controlValues: Record<string, unknown>,
+): Promise<ControlValidationError[]> {
+  const { json } = await SupersetClient.post({
+    endpoint: `/api/v1/widgets/type/${widgetType}/validate`,
+    jsonPayload: { control_values: controlValues },
+  });
+  return (json as { result: { errors: ControlValidationError[] } }).result
+    .errors;
+}
+
+/**
+ * Merges `delta` onto the node's current props, validates the merged
+ * candidate, and commits it to `node.props` only if the backend accepts it.
+ *
+ * `onBeforeCommit`, if given, runs synchronously immediately before the
+ * `provider.updateProps` call — not part of the merge/validate/commit
+ * contract itself, but the one hook a caller needs to set a flag in the
+ * exact tick its own commit lands (e.g. `SchemaControlPanel` telling its
+ * own resync effect "this `props` change was mine"), without racing the
+ * async validation round-trip to do it.
+ */
+export async function commitWidgetProps(
+  nodeId: string,
+  widgetType: string,
+  delta: Record<string, unknown>,
+  options?: { onBeforeCommit?: () => void },
+): Promise<CommitPropsResult> {
+  const node = provider.getNode(nodeId);
+  const candidate = { ...node?.props, ...delta };
+  const errors = await validateControlValues(widgetType, candidate);
+  if (errors.length > 0) {
+    return { ok: false, errors };
+  }
+  options?.onBeforeCommit?.();
+  provider.updateProps(nodeId, candidate);

Review Comment:
   **Suggestion:** Each invocation snapshots `node.props` before awaiting 
validation, then commits that snapshot later. If two edits are validated 
concurrently, a slower earlier request can call `updateProps` after a newer 
request and overwrite the newer change. Serialize commits or reject stale 
validation results before updating the provider, not only when displaying 
errors. [race condition]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Rapid schema edits can lose newer widget properties.
   - ⚠️ Validation completion order controls final state.
   - ❌ Dashboard Builder edits become nondeterministic.
   ```
   </details>
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8c93d7e4af4a48948796de37d36a631f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8c93d7e4af4a48948796de37d36a631f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/src/pages/DashboardBuilderV2/controlValueValidation.ts
   **Line:** 96:109
   **Comment:**
        *Race Condition: Each invocation snapshots `node.props` before awaiting 
validation, then commits that snapshot later. If two edits are validated 
concurrently, a slower earlier request can call `updateProps` after a newer 
request and overwrite the newer change. Serialize commits or reject stale 
validation results before updating the provider, not only when displaying 
errors.
   
   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%2F43588&comment_hash=dc8cb944e7f45eed9ee6a65b8ae058b2a0f8bff6db011702f19171997dd38f9e&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43588&comment_hash=dc8cb944e7f45eed9ee6a65b8ae058b2a0f8bff6db011702f19171997dd38f9e&reaction=dislike'>👎</a>



##########
superset-frontend/src/core/dashboard/widgets/echartsStructuredChrome.ts:
##########
@@ -0,0 +1,144 @@
+/**
+ * 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.
+ */
+
+/**
+ * The `echarts` widget's second, independent structured layer — chart
+ * chrome (title/legend/tooltip/axis labels) — matching `EchartsChrome`'s
+ * docstring in `superset/widgets/controls.py`: every field is optional and
+ * applies (or not) on its own, regardless of `chartType`/`customize`. A
+ * field left at its default never touches `echartsOptions`; when it does
+ * apply, it merges onto — rather than replaces — the matching section, so
+ * an unmanaged sibling property there (e.g. a hand-authored `legend.orient`)
+ * survives.
+ *
+ * `EchartsChromeValue` is deliberately flat, not grouped into
+ * `title`/`legend`/`tooltip`/`xAxis`/`yAxis` sub-objects — see
+ * `EchartsChrome`'s own docstring: JsonForms' generated control panel only
+ * renders one level of nested-object properties, so a two-level-deep
+ * `chrome.title.text` would render as an empty group with no fields inside.
+ */
+
+export interface EchartsChromeValue {
+  titleText?: string;
+  legendShow?: boolean;
+  legendPosition?: 'top' | 'bottom' | 'left' | 'right' | null;
+  tooltipTrigger?: 'item' | 'axis' | null;
+  xAxisName?: string;
+  xAxisRotate?: number;
+  xAxisFormat?: string;
+  yAxisName?: string;
+  yAxisRotate?: number;
+  yAxisFormat?: string;
+}
+
+// ECharts has no single "position" property on `legend` — placement comes
+// from `top`/`left` (each accepting a keyword or coordinate). This maps the
+// friendlier compass-direction picker onto the pair ECharts actually reads.
+const LEGEND_POSITION: Record<string, { top: string; left: string }> = {
+  top: { top: 'top', left: 'center' },
+  bottom: { top: 'bottom', left: 'center' },
+  left: { top: 'middle', left: 'left' },
+  right: { top: 'middle', left: 'right' },
+};
+
+function asRecord(value: unknown): Record<string, unknown> {
+  return value !== null && typeof value === 'object'
+    ? (value as Record<string, unknown>)
+    : {};

Review Comment:
   **Suggestion:** `asRecord` treats arrays as records. When a raw ECharts 
option uses the valid multi-axis form, applying an axis name, rotation, or 
format spreads the array indexes into an object and returns an object-shaped 
axis, changing the option's meaning and potentially breaking multi-axis 
rendering. Preserve array axes or apply the structured override to the 
appropriate axis entries. [type error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Multi-axis ECharts options lose their array representation.
   - ❌ Structured axis edits can break chart rendering.
   - ⚠️ Raw custom ECharts options commonly support axis arrays.
   ```
   </details>
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4a9afe10f3ca42e78718fbd0ec3789b5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4a9afe10f3ca42e78718fbd0ec3789b5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <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/widgets/echartsStructuredChrome.ts
   **Line:** 60:63
   **Comment:**
        *Type Error: `asRecord` treats arrays as records. When a raw ECharts 
option uses the valid multi-axis form, applying an axis name, rotation, or 
format spreads the array indexes into an object and returns an object-shaped 
axis, changing the option's meaning and potentially breaking multi-axis 
rendering. Preserve array axes or apply the structured override to the 
appropriate axis entries.
   
   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%2F43588&comment_hash=1aa01275737cbe3ea8c8090f73c4f0015c88b03e7845ae29ad81f1ab9439b3a1&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43588&comment_hash=1aa01275737cbe3ea8c8090f73c4f0015c88b03e7845ae29ad81f1ab9439b3a1&reaction=dislike'>👎</a>



##########
superset-frontend/src/pages/DashboardBuilderV2/schemaControlRenderers.tsx:
##########
@@ -93,24 +161,1114 @@ function ColorControl({
   path,
   schema,
   label,
+  description,
 }: ControlProps) {
   const theme = useTheme();
   const value = (data ??
     (schema as Record<string, unknown>).default ??
     theme.colorText) as string;
   return (
-    <Flex align="center" gap="small">
+    <Form.Item label={label} tooltip={description}>
       <input
         type="color"
         value={value}
         onChange={event => handleChange(path, event.target.value)}
       />
-      <Typography.Text>{label}</Typography.Text>
-    </Flex>
+    </Form.Item>
+  );
+}
+
+interface SeriesEntryPropertySchema {
+  type?: string;
+  title?: string;
+  default?: unknown;
+  minimum?: number;
+  maximum?: number;
+  'x-control'?: string;
+  'x-step'?: number;
+}
+
+interface SeriesEntrySchema {
+  properties?: Record<string, SeriesEntryPropertySchema>;
+}
+
+interface SeriesMapSchema {
+  properties?: Record<string, SeriesEntrySchema>;
+}
+
+type SeriesOverrideValue = Record<string, unknown>;
+
+/** Turns a camelCase field key into the lowercase, space-separated phrase
+ * an aria-label reads naturally with (`sizeScale` → `size scale`). */
+function humanizeFieldKey(key: string): string {
+  return key
+    .replace(/([A-Z])/g, ' $1')
+    .toLowerCase()
+    .trim();
+}
+
+/** The value a not-yet-customized series entry starts from, read purely off
+ * its own enriched sub-schema — every widget's per-series entry shape
+ * (Balloons' `{color, sizeScale}`, echarts' `{color, visible, displayName}`,
+ * or any future one) declares its own field defaults; this has no
+ * hard-coded opinion of what those fields are. `fallbackColor` only backs a
+ * `color` field the backend never defaulted — not expected to happen, but a
+ * theme token beats a literal black. */
+export function seriesDefaults(
+  entrySchema: SeriesEntrySchema | undefined,
+  fallbackColor: string,
+): SeriesOverrideValue {
+  const properties = entrySchema?.properties ?? {};
+  const value: SeriesOverrideValue = Object.fromEntries(
+    Object.entries(properties).map(([key, prop]) => [key, prop.default]),
+  );
+  if (value.color === undefined && 'color' in properties) {
+    value.color = fallbackColor;
+  }
+  return value;
+}
+
+/** One property of a customized series entry, rendered by its schema shape
+ * — not by a hard-coded field name — so a new per-series entry model (e.g.
+ * echarts' `{color, visible, displayName}`) needs no renderer of its own:
+ *   - `x-control: "color"` (or the property key `color`) → a color swatch.
+ *   - `type: "boolean"` → a toggle.
+ *   - `type: "number"` / `"integer"` → a bounded numeric input.
+ *   - anything else → a text input.
+ */
+function SeriesEntryPropertyControl({
+  seriesKey,
+  propKey,
+  propSchema,
+  value,
+  onChange,
+}: {
+  seriesKey: string;
+  propKey: string;
+  propSchema: SeriesEntryPropertySchema | undefined;
+  value: unknown;
+  onChange: (next: unknown) => void;
+}): ReactElement {
+  const theme = useTheme();
+  const fieldLabel = `${seriesKey} ${humanizeFieldKey(propKey)}`;
+
+  if (propSchema?.['x-control'] === 'color' || propKey === 'color') {
+    const color = (value as string) || theme.colorText;
+    return (
+      <ColorPicker
+        value={color}
+        onChange={next => onChange(next.toHexString())}
+      >
+        <button
+          type="button"
+          aria-label={t('%s color', seriesKey)}
+          style={{
+            width: 20,
+            height: 20,
+            borderRadius: 4,
+            border: '1px solid rgba(0, 0, 0, 0.15)',
+            background: color,
+            cursor: 'pointer',
+            padding: 0,
+          }}
+        />
+      </ColorPicker>
+    );
+  }
+  if (propSchema?.type === 'boolean') {
+    return (
+      <Switch
+        aria-label={fieldLabel}
+        checked={value !== false}
+        onChange={onChange}
+      />
+    );
+  }
+  if (propSchema?.type === 'number' || propSchema?.type === 'integer') {
+    return (
+      <InputNumber
+        size="small"
+        aria-label={fieldLabel}
+        value={value as number}
+        min={propSchema?.minimum}
+        max={propSchema?.maximum}
+        step={propSchema?.['x-step'] ?? 1}
+        style={{ width: 64 }}
+        onChange={next => onChange(typeof next === 'number' ? next : value)}
+      />
+    );
+  }
+  return (
+    <Input
+      size="small"
+      aria-label={fieldLabel}
+      value={(value as string) ?? ''}
+      onChange={event => onChange(event.target.value)}
+      style={{ width: 140 }}
+    />
+  );
+}
+
+/**
+ * `x-dynamic: true` on a dict-of-objects field (e.g. Balloons'
+ * `customize.series`, one entry per distinct color-dimension value, or
+ * echarts' `customize.series`, one entry per `dataBinding` metric): an
+ * overrides list, collapsed by default, rather than the upstream renderer's
+ * one always-expanded group per possible entry — which turns a real
+ * grouping column into thousands of pixels of identical, unstyled controls.
+ *
+ * The backend enriches this field's schema with one inlined per-key
+ * sub-schema (a title and, where the entry shape has one, a palette-defaulted
+ * `color`), but leaves the *data* untouched until an author actually edits a
+ * value — so "has an entry in `data`" already means "has been customized",
+ * with no comparison against the schema's own defaults needed. Which fields
+ * an entry has, and how each renders, comes entirely from that per-key
+ * sub-schema (see `SeriesEntryPropertyControl`) — this component has no
+ * opinion of its own on the entry shape.
+ */
+function SeriesOverridesControl(props: ControlProps): ReactElement {
+  const { data, handleChange, path, schema, label, description } = props;
+  const theme = useTheme();
+  const seriesSchema = schema as SeriesMapSchema;
+  const keys = useMemo(
+    () => Object.keys(seriesSchema.properties ?? {}),
+    [seriesSchema],
+  );
+  const values = (data ?? {}) as Record<string, SeriesOverrideValue>;
+  const customizedKeys = keys.filter(key => values[key] !== undefined);
+  const availableKeys = keys.filter(key => values[key] === undefined);
+
+  if (keys.length === 0) {
+    return (
+      <Form.Item label={label} tooltip={description}>
+        <Typography.Text type="secondary">
+          {t('No series available to customize yet.')}
+        </Typography.Text>
+      </Form.Item>
+    );
+  }
+
+  const write = (next: Record<string, SeriesOverrideValue>) =>
+    handleChange(path, next);
+
+  const removeOverride = (key: string) => {
+    const next = { ...values };
+    delete next[key];
+    write(next);
+  };
+
+  const addOverride = (key: string) => {
+    write({
+      ...values,
+      [key]: seriesDefaults(seriesSchema.properties?.[key], theme.colorText),
+    });
+  };
+
+  return (
+    <Form.Item label={label} tooltip={description}>
+      <Collapse
+        ghost
+        size="small"
+        items={[
+          {
+            key: 'series-overrides',
+            label: t(
+              '%s series · %s customized',
+              keys.length,
+              customizedKeys.length,
+            ),
+            children: (
+              <Flex vertical gap="small">
+                {customizedKeys.map(key => {
+                  const value = values[key];
+                  const entryProperties =
+                    seriesSchema.properties?.[key]?.properties ?? {};
+                  const otherPropKeys = Object.keys(entryProperties).filter(
+                    propKey => propKey !== 'color',
+                  );
+                  return (
+                    <Flex key={key} align="center" gap="small">
+                      {'color' in entryProperties && (
+                        <SeriesEntryPropertyControl
+                          seriesKey={key}
+                          propKey="color"
+                          propSchema={entryProperties.color}
+                          value={value.color}
+                          onChange={next =>
+                            write({
+                              ...values,
+                              [key]: { ...value, color: next },
+                            })
+                          }
+                        />
+                      )}
+                      <div
+                        style={{
+                          flex: 1,
+                          minWidth: 0,
+                          whiteSpace: 'nowrap',
+                          overflow: 'hidden',
+                          textOverflow: 'ellipsis',
+                        }}
+                      >
+                        {key}
+                      </div>
+                      {otherPropKeys.map(propKey => (
+                        <SeriesEntryPropertyControl
+                          key={propKey}
+                          seriesKey={key}
+                          propKey={propKey}
+                          propSchema={entryProperties[propKey]}
+                          value={value[propKey]}
+                          onChange={next =>
+                            write({
+                              ...values,
+                              [key]: { ...value, [propKey]: next },
+                            })
+                          }
+                        />
+                      ))}
+                      <Button
+                        buttonSize="xsmall"
+                        buttonStyle="link"
+                        aria-label={t('Reset %s to default', key)}
+                        icon={<Icons.CloseOutlined iconSize="s" />}
+                        onClick={() => removeOverride(key)}
+                      />
+                    </Flex>
+                  );
+                })}
+                {availableKeys.length > 0 && (
+                  // Remounted on every pick: rc-select otherwise keeps
+                  // showing the just-picked option's label internally even
+                  // once it's gone from `options` (a controlled `value` of
+                  // `null`/`undefined` doesn't clear that cache on its own —
+                  // see `ReferenceMultiList`'s identical, pre-existing gap).
+                  <Select
+                    key={availableKeys.length}
+                    value={null}
+                    placeholder={t('Add a series override…')}
+                    ariaLabel={t('Add %s override', label)}
+                    options={availableKeys.map(key => ({
+                      value: key,
+                      label: key,
+                    }))}
+                    onChange={next => addOverride(next as string)}
+                    css={{ width: '100%' }}
+                  />
+                )}
+              </Flex>
+            ),
+          },
+        ]}
+      />
+    </Form.Item>
+  );
+}
+
+/**
+ * The dataset id a column/metric-reference control resolves options
+ * against. A JsonForms control only sees its own field's data by default —
+ * the whole node's props reach it through `config.formData`, which
+ * `SchemaControlPanel` populates for exactly this reason (mirroring
+ * `SemanticLayerModal`'s own `config={{ formData }}`).
+ */
+function useBoundDatasetId(props: ControlProps): number | undefined {
+  const formData = (
+    props.config as { formData?: Record<string, unknown> } | undefined
+  )?.formData;
+  const dataBinding = formData?.dataBinding as
+    { datasetId?: number } | undefined;
+  return dataBinding?.datasetId;
+}
+
+/** The sibling `dataBinding.dimensions` list, read the same way. */
+function useBoundDimensions(props: ControlProps): string[] {
+  const formData = (
+    props.config as { formData?: Record<string, unknown> } | undefined
+  )?.formData;
+  const dataBinding = formData?.dataBinding as
+    { dimensions?: string[] } | undefined;
+  return dataBinding?.dimensions ?? [];
+}
+
+/**
+ * True when a column/metric-reference control should fail open to the raw
+ * JSON editor (`CodeControl`) rather than render its picker: no dataset is
+ * bound yet, or the dataset fetch failed. Deliberately does NOT cover the
+ * in-flight loading state (`metadata` still `null`, no `error` yet) — that's
+ * the normal case while a bound dataset's metadata is fetched, and the
+ * picker renders as usual with its own `loading` flag set.
+ */
+function shouldFallBackToCode(
+  datasetId: number | undefined,
+  error: string | null,
+): boolean {
+  return datasetId === undefined || error !== null;
+}
+
+interface ReferenceOption {
+  value: string;
+  label: ReactNode;
+}
+
+const COLUMN_TYPE_BY_HINT: Record<string, number> = {
+  numeric: 0,
+  string: 1,
+  temporal: 2,
+  boolean: 3,
+};
+
+/**
+ * Column options for a `column`/`column-multi` control, filtered by the
+ * field's `x-column-types` hint (omitted means any column type).
+ */
+export function columnOptions(
+  metadata: DatasetMetadata | null,
+  allowedTypes: string[] | undefined,
+): ReferenceOption[] {
+  const allowed = allowedTypes?.map(hint => COLUMN_TYPE_BY_HINT[hint]);
+  return (metadata?.columns ?? [])
+    .filter(
+      column =>
+        !allowed || (column.type !== null && allowed.includes(column.type)),
+    )
+    .map(column => ({
+      value: column.name,
+      label: (
+        <Flex align="center" gap="small">
+          <ColumnTypeLabel type={column.type ?? undefined} />
+          {column.name}
+        </Flex>
+      ),
+    }));
+}
+
+/** A single reference value (column or metric), rendered as a Select. */
+function ReferenceSelect({
+  label,
+  description,
+  value,
+  options,
+  loading,
+  disabled,
+  placeholder,
+  onChange,
+}: {
+  label: string;
+  description: string | undefined;
+  value: string | undefined;
+  options: ReferenceOption[];
+  loading: boolean;
+  disabled: boolean;
+  placeholder?: string;
+  onChange: (next: string | undefined) => void;
+}): ReactElement {
+  return (
+    <Form.Item label={label} tooltip={description}>
+      <Select
+        ariaLabel={label}
+        value={value}
+        onChange={next => onChange((next as string | undefined) ?? undefined)}
+        options={options}
+        loading={loading}
+        disabled={disabled}
+        placeholder={placeholder}
+        allowClear
+        css={{ width: '100%' }}
+      />
+    </Form.Item>
   );
 }
 
-/** Base Semantic-Layer renderers plus the widget-control code/color ones. */
+/**
+ * An ordered list of reference values (columns or metrics): each entry can
+ * be removed or dragged to reorder, and a trailing Select adds one more from
+ * whatever isn't already picked.
+ */
+function ReferenceMultiList({
+  label,
+  description,
+  values,
+  options,
+  loading,
+  disabled,
+  onChange,
+}: {
+  label: string;
+  description: string | undefined;
+  values: string[];
+  options: ReferenceOption[];
+  loading: boolean;
+  disabled: boolean;
+  onChange: (next: string[]) => void;
+}): ReactElement {
+  const dragIndexRef = useRef<number | null>(null);
+  const available = options.filter(option => !values.includes(option.value));
+
+  const move = (from: number, to: number) => {
+    const next = [...values];
+    const [moved] = next.splice(from, 1);
+    next.splice(to, 0, moved);
+    onChange(next);
+  };
+
+  return (
+    <Form.Item label={label} tooltip={description}>
+      <Flex vertical gap="small">
+        {values.map((value, index) => {
+          const option = options.find(candidate => candidate.value === value);
+          return (
+            <Flex
+              key={value}
+              align="center"
+              gap="small"
+              draggable
+              onDragStart={() => {
+                dragIndexRef.current = index;
+              }}
+              onDragOver={event => event.preventDefault()}
+              onDrop={() => {
+                if (
+                  dragIndexRef.current !== null &&
+                  dragIndexRef.current !== index
+                ) {
+                  move(dragIndexRef.current, index);
+                }
+                dragIndexRef.current = null;
+              }}
+            >
+              <Icons.HolderOutlined iconSize="s" />
+              <div style={{ flex: 1 }}>{option?.label ?? value}</div>
+              {/* Keyboard-operable equivalent of the drag handle above: that
+                  handle isn't itself focusable, so reordering — which
+                  decides e.g. the default color dimension — had no
+                  non-mouse path at all. */}
+              <Button
+                buttonSize="xsmall"
+                buttonStyle="link"
+                aria-label={t('Move %s up', value)}
+                disabled={index === 0}
+                icon={<Icons.UpOutlined iconSize="s" />}
+                onClick={() => move(index, index - 1)}
+              />
+              <Button
+                buttonSize="xsmall"
+                buttonStyle="link"
+                aria-label={t('Move %s down', value)}
+                disabled={index === values.length - 1}
+                icon={<Icons.DownOutlined iconSize="s" />}
+                onClick={() => move(index, index + 1)}
+              />
+              <Button
+                buttonSize="xsmall"
+                buttonStyle="link"
+                aria-label={t('Remove %s', value)}
+                icon={<Icons.CloseOutlined iconSize="s" />}
+                onClick={() => onChange(values.filter((_, i) => i !== index))}
+              />
+            </Flex>
+          );
+        })}
+        {available.length > 0 && (
+          // Remounted on every pick — see `SeriesOverridesControl`'s
+          // identical picker for why a controlled `value` of `undefined`
+          // alone doesn't stop rc-select echoing the just-picked label.
+          <Select
+            key={available.length}
+            value={null}
+            placeholder={t('Add field')}
+            ariaLabel={t('Add %s', label)}
+            options={available}
+            loading={loading}
+            disabled={disabled}
+            onChange={next => onChange([...values, next as string])}
+            css={{ width: '100%' }}
+          />
+        )}
+      </Flex>
+    </Form.Item>
+  );
+}
+
+/**
+ * `x-control: "column"` — a single column reference. Falls back to the raw
+ * JSON editor when no dataset is bound (or its fetch failed), or when the
+ * existing value isn't a string — e.g. an object hand-authored into the
+ * field through the Inspector's JSON tab, which `Select` can't render as a
+ * `value` and JsonForms would otherwise crash on.
+ */
+function ColumnControl(props: ControlProps): ReactElement {
+  const datasetId = useBoundDatasetId(props);
+  const { metadata, loading, error } = useDatasetMetadata(datasetId);
+  const allowedTypes = (props.schema as Record<string, unknown>)[
+    'x-column-types'
+  ] as string[] | undefined;
+
+  if (
+    shouldFallBackToCode(datasetId, error) ||
+    (props.data !== undefined && typeof props.data !== 'string')
+  ) {
+    return <CodeControl {...props} />;
+  }
+
+  return (
+    <ReferenceSelect
+      label={props.label}
+      description={props.description}
+      value={props.data as string | undefined}
+      options={columnOptions(metadata, allowedTypes)}
+      loading={loading}
+      disabled={!props.enabled}
+      onChange={value => props.handleChange(props.path, value)}
+    />
+  );
+}
+
+/**
+ * The `colorDimension` field specifically: a column reference, but not to
+ * any column — the widget only colors by a dimension it already groups by
+ * (Balloons' `_color_dimension_must_be_grouped` validator rejects anything
+ * else). Offering all of a dataset's columns, most of which the backend
+ * will reject, taught nothing about which one was actually valid; this
+ * intersects the picker's options with the sibling `dataBinding.dimensions`
+ * instead, and disables it with an explanatory placeholder when there's
+ * nothing grouped yet to color by.
+ */
+function ColorDimensionControl(props: ControlProps): ReactElement {
+  const datasetId = useBoundDatasetId(props);
+  const dimensions = useBoundDimensions(props);
+  const { metadata, loading, error } = useDatasetMetadata(datasetId);
+
+  if (
+    shouldFallBackToCode(datasetId, error) ||
+    (props.data !== undefined && typeof props.data !== 'string')
+  ) {
+    return <CodeControl {...props} />;
+  }
+
+  const options = columnOptions(metadata, undefined).filter(option =>
+    dimensions.includes(option.value),
+  );
+
+  return (
+    <ReferenceSelect
+      label={props.label}
+      description={props.description}
+      value={props.data as string | undefined}
+      options={options}
+      loading={loading}
+      disabled={!props.enabled || dimensions.length === 0}
+      placeholder={
+        dimensions.length === 0 ? t('Group a dimension first') : undefined
+      }
+      onChange={value => props.handleChange(props.path, value)}
+    />
+  );
+}
+
+/**
+ * `x-control: "column-multi"` — an ordered list of column references. Falls
+ * back to the raw JSON editor when no dataset is bound (or its fetch
+ * failed), or when an existing entry isn't a string — e.g. an object
+ * hand-authored into the field through the Inspector's JSON tab, which
+ * `ReferenceMultiList` can't render as a list entry.
+ */
+function ColumnMultiControl(props: ControlProps): ReactElement {
+  const datasetId = useBoundDatasetId(props);
+  const { metadata, loading, error } = useDatasetMetadata(datasetId);
+  const allowedTypes = (props.schema as Record<string, unknown>)[
+    'x-column-types'
+  ] as string[] | undefined;
+  const values = Array.isArray(props.data) ? (props.data as unknown[]) : [];
+  const hasNonStringEntry = values.some(value => typeof value !== 'string');
+
+  if (shouldFallBackToCode(datasetId, error) || hasNonStringEntry) {
+    return <CodeControl {...props} />;
+  }
+
+  return (
+    <ReferenceMultiList
+      label={props.label}
+      description={props.description}
+      values={values as string[]}
+      options={columnOptions(metadata, allowedTypes)}
+      loading={loading}
+      disabled={!props.enabled}
+      onChange={next => props.handleChange(props.path, next)}
+    />
+  );
+}
+
+/**
+ * Metric options for a `metric-multi` control: the dataset's saved metrics,
+ * shown with the same Sigma icon Explore's metric picker uses.
+ */
+export function metricOptions(
+  metadata: DatasetMetadata | null,
+): ReferenceOption[] {
+  return (metadata?.metrics ?? []).map(metric => ({
+    value: metric.name,
+    label: (
+      <Flex align="center" gap="small">
+        <ColumnTypeLabel type="metric" />
+        {metric.verboseName}
+      </Flex>
+    ),
+  }));
+}
+
+/**
+ * True for a metric entry this control has no row to draw at all: neither a
+ * plain saved-metric-name string nor a structurally valid ad-hoc metric
+ * object — e.g. malformed data hand-authored through the JSON tab. Only
+ * this case still drops the *whole* field to the raw JSON editor; a
+ * well-formed mix of saved and ad-hoc entries renders as a mixed list
+ * instead (see `MetricEntryList`).
+ */
+export function isUnrepresentableMetric(value: unknown): boolean {
+  return typeof value !== 'string' && !isDictionaryForAdhocMetric(value);
+}
+
+/** Whether the bound dataset's own settings forbid ad-hoc metrics entirely
+ * (a dataset-level admin setting, not a per-field one) — read the same raw
+ * `extra` JSON the legacy metric editor reads it from. */
+export function disallowsAdhocMetrics(
+  metadata: DatasetMetadata | null,
+): boolean {
+  if (!metadata?.extra) return false;
+  try {
+    return Boolean(
+      (JSON.parse(metadata.extra) as { disallow_adhoc_metrics?: boolean })
+        .disallow_adhoc_metrics,
+    );
+  } catch {
+    return false;
+  }
+}
+
+type MetricEntry = string | CoreAdhocMetric;
+
+/** A metric entry's own display label: a saved metric's verbose name (or
+ * its raw name if the dataset's metric list hasn't loaded/matched yet), or
+ * an ad-hoc metric's own label — computed the same way the legacy editor
+ * derives one (`(AVG)(price)`, etc.) when the author hasn't set a custom one. 
*/
+function metricEntryLabel(
+  entry: MetricEntry,
+  metadata: DatasetMetadata | null,
+): ReactNode {
+  if (typeof entry === 'string') {
+    const known = metadata?.metrics.find(metric => metric.name === entry);
+    return known?.verboseName ?? entry;
+  }
+  return fromCoreAdhocMetric(entry).label;
+}
+
+/** Sentinel option value picked from the "Add field" select to start a new
+ * ad-hoc metric, distinct from any real saved-metric name. */
+const ADD_CUSTOM_METRIC = '__custom_metric__';
+
+/**
+ * An ordered list of metric references, each entry rendered by its own
+ * kind: a saved metric as a plain row (unchanged from `ReferenceMultiList`),
+ * an ad-hoc metric (SIMPLE or SQL) as a row whose label opens
+ * `AdhocMetricEditor` for just that entry. "Add field" offers both a saved
+ * metric to pick and, unless the dataset disallows it, a blank ad-hoc draft.
+ */
+function MetricEntryList({
+  label,
+  description,
+  values,
+  metadata,
+  columns,
+  datasourceId,
+  datasourceType,
+  disallowAdhoc,
+  disabled,
+  onChange,
+}: {
+  label: string;
+  description: string | undefined;
+  values: MetricEntry[];
+  metadata: DatasetMetadata | null;
+  columns: DatasetColumnMeta[];
+  datasourceId: number | undefined;
+  datasourceType: string | undefined;
+  disallowAdhoc: boolean;
+  disabled: boolean;
+  onChange: (next: MetricEntry[]) => void;
+}): ReactElement {
+  const [editingIndex, setEditingIndex] = useState<number | null>(null);
+  const [addingNew, setAddingNew] = useState(false);
+
+  const pickedSavedNames = values.filter(
+    (value): value is string => typeof value === 'string',
+  );
+  const availableSaved = metricOptions(metadata).filter(
+    option => !pickedSavedNames.includes(option.value),
+  );
+  const addOptions = [
+    ...availableSaved,
+    ...(disallowAdhoc
+      ? []
+      : [{ value: ADD_CUSTOM_METRIC, label: t('Custom metric…') }]),
+  ];
+
+  const move = (from: number, to: number) => {
+    const next = [...values];
+    const [moved] = next.splice(from, 1);
+    next.splice(to, 0, moved);
+    onChange(next);
+  };
+
+  return (
+    <Form.Item label={label} tooltip={description}>
+      <Flex vertical gap="small">
+        {values.map((value, index) => {
+          const isAdhoc = typeof value !== 'string';
+          const canEdit = isAdhoc && !disallowAdhoc;
+          const key =
+            typeof value === 'string'
+              ? value
+              : (value.optionName ?? `adhoc-${index}`);
+          // Event handlers only attached at all when `canEdit` — a static
+          // div with an onClick/onKeyDown regardless of role is what the
+          // a11y linter (rightly) objects to, not just a style choice.
+          const interactiveProps = canEdit
+            ? {
+                role: 'button' as const,
+                tabIndex: 0,
+                onClick: () => setEditingIndex(index),
+                onKeyDown: (event: KeyboardEvent<HTMLDivElement>) => {
+                  if (event.key === 'Enter' || event.key === ' ') {
+                    event.preventDefault();
+                    setEditingIndex(index);
+                  }
+                },
+              }
+            : {};
+          const rowLabel = (
+            <div
+              style={{
+                flex: 1,
+                minWidth: 0,
+                cursor: canEdit ? 'pointer' : 'default',
+              }}
+              {...interactiveProps}
+            >
+              <Flex align="center" gap="small">
+                <ColumnTypeLabel type="metric" />
+                {metricEntryLabel(value, metadata)}
+              </Flex>
+            </div>
+          );
+          return (
+            <Flex key={key} align="center" gap="small">
+              <Icons.HolderOutlined iconSize="s" />
+              {isAdhoc ? (
+                <AdhocMetricEditor
+                  value={value}
+                  columns={columns}
+                  datasourceId={datasourceId}
+                  datasourceType={datasourceType}
+                  open={editingIndex === index}
+                  onOpenChange={open => {
+                    // `Popover`'s own `trigger="click"` opens on any click to
+                    // its children regardless of what the row's own onClick
+                    // does — `canEdit` has to gate here too, or a disallowed
+                    // dataset's rows would still open on click.
+                    if (canEdit) setEditingIndex(open ? index : null);
+                  }}
+                  onSave={next => {
+                    const updated = [...values];
+                    updated[index] = next;
+                    onChange(updated);
+                  }}
+                >
+                  {rowLabel}
+                </AdhocMetricEditor>
+              ) : (
+                rowLabel
+              )}
+              <Button
+                buttonSize="xsmall"
+                buttonStyle="link"
+                aria-label={t('Move metric %s up', index + 1)}
+                disabled={index === 0}
+                icon={<Icons.UpOutlined iconSize="s" />}
+                onClick={() => move(index, index - 1)}
+              />
+              <Button
+                buttonSize="xsmall"
+                buttonStyle="link"
+                aria-label={t('Move metric %s down', index + 1)}
+                disabled={index === values.length - 1}
+                icon={<Icons.DownOutlined iconSize="s" />}
+                onClick={() => move(index, index + 1)}
+              />
+              <Button
+                buttonSize="xsmall"
+                buttonStyle="link"
+                aria-label={t('Remove metric %s', index + 1)}
+                icon={<Icons.CloseOutlined iconSize="s" />}
+                onClick={() => onChange(values.filter((_, i) => i !== index))}
+              />
+            </Flex>
+          );
+        })}
+        {addOptions.length > 0 && (
+          <Select
+            key={`${availableSaved.length}-${values.length}`}
+            value={null}
+            placeholder={t('Add field')}
+            ariaLabel={t('Add %s', label)}
+            options={addOptions}
+            disabled={disabled}
+            onChange={next => {
+              if (next === ADD_CUSTOM_METRIC) {
+                setAddingNew(true);
+              } else {
+                onChange([...values, next as string]);
+              }
+            }}
+          />
+        )}
+        {addingNew && (
+          <AdhocMetricEditor
+            value={undefined}
+            columns={columns}
+            datasourceId={datasourceId}
+            datasourceType={datasourceType}
+            open={addingNew}
+            onOpenChange={setAddingNew}
+            onSave={next => {
+              onChange([...values, next]);
+              setAddingNew(false);
+            }}
+          >
+            {/* Zero-size trigger: opening this popover is driven entirely
+                by picking "Custom metric…" above, not by a click here. */}
+            <span />
+          </AdhocMetricEditor>
+        )}
+      </Flex>
+    </Form.Item>
+  );
+}
+
+/**
+ * `x-control: "metric-multi"` — an ordered list of metric references. Falls
+ * back to the raw JSON editor (`CodeControl`) whenever no dataset is bound
+ * (or its fetch failed), or an entry is genuinely unrepresentable (see
+ * `isUnrepresentableMetric`); a well-formed mix of saved and ad-hoc metrics
+ * renders as `MetricEntryList`, not raw JSON.
+ */
+function MetricMultiControl(props: ControlProps): ReactElement {
+  const datasetId = useBoundDatasetId(props);
+  const { metadata, loading, error } = useDatasetMetadata(datasetId);
+  const values = Array.isArray(props.data) ? (props.data as unknown[]) : [];
+  const hasUnrepresentable = values.some(isUnrepresentableMetric);
+
+  if (shouldFallBackToCode(datasetId, error) || hasUnrepresentable) {
+    return <CodeControl {...props} />;
+  }
+
+  return (
+    <MetricEntryList
+      label={props.label}
+      description={props.description}
+      values={values as MetricEntry[]}
+      metadata={metadata}
+      columns={metadata?.columns ?? []}
+      datasourceId={datasetId}
+      datasourceType={metadata?.datasourceType}
+      disallowAdhoc={disallowsAdhocMetrics(metadata)}
+      disabled={!props.enabled || loading}
+      onChange={next => props.handleChange(props.path, next)}
+    />
+  );
+}
+
+/**
+ * The value the dataset picker needs for an already-bound dataset. Bare —
+ * just the id (composite-encoded per below) — while the name is still
+ * resolving: `AsyncSelect` prefers a *labeled* value's own label over a
+ * matching loaded option's, so handing it a stale `String(datasetId)` label
+ * here would overwrite the real name the moment the matching option
+ * arrives, replaying the exact stuck-on-the-numeric-id symptom this control
+ * exists to avoid — instead, letting `AsyncSelect` fall back to the id on
+ * its own leaves it free to prefer a loaded option's label the instant one
+ * matches. Once `tableName` resolves, the explicit label takes over.
+ *
+ * `value` carries the composite `"ds:<id>"` encoding when the Semantic
+ * Layers flag is on, matching what `loadDatasetOptions` encodes its own
+ * options as in that mode (see `resolveDatasetPick` below) — an option and
+ * a bound value in two different encodings never match, which is what
+ * leaves a genuinely-selected dataset showing as unselected.
+ */
+export function toDatasetSelectValue(
+  datasetId: number | undefined,
+  tableName: string | undefined,
+  useSemanticLayers: boolean,
+): { label: string; value: number | string } | number | string | undefined {
+  if (datasetId === undefined) {
+    return undefined;
+  }
+  const value = useSemanticLayers ? toCompositeValue(datasetId) : datasetId;
+  return tableName ? { label: tableName, value } : value;
+}
+
+/**
+ * The numeric dataset id a picked option resolves to, or `undefined` when
+ * the pick should be rejected outright: a semantic view, which
+ * `DataBinding.datasetId` has no way to represent (SIP-182's `kind` is a
+ * connection-level concept, not a per-field one on this schema). Handles
+ * both encodings `loadDatasetOptions` can produce — a plain number when the
+ * Semantic Layers flag is off, or, flag on, a composite `"ds:<id>"` /
+ * `"sv:<id>"` string for every option (not only semantic views: with the
+ * flag on, ordinary datasets are composite-encoded too).
+ */
+export function resolveDatasetPick(
+  value: number | string | undefined,
+): number | undefined {
+  if (value === undefined || typeof value === 'number') {
+    return value;
+  }
+  return kindFromComposite(value) === 'semantic_view'
+    ? undefined
+    : fromCompositeValue(value);
+}
+
+/**
+ * `loadDatasetOptions` filtered down to plain datasets. `DataBinding` has no
+ * way to represent a semantic view (SIP-182's `kind` is a connection-level
+ * concept, not a per-field one on this schema), so rather than let one be
+ * picked and then reject it after the fact — leaving the closed select
+ * showing a value the widget never actually bound — it is never offered.
+ *
+ * Module-level, not a closure defined inside `DatasetControl`: `AsyncSelect`
+ * treats a change in its `options` function's identity as a reason to wipe
+ * its own fetched-options cache, and a fresh arrow function on every render
+ * would do exactly that.
+ *
+ * `totalCount` is passed through unfiltered — it counts datasets and
+ * semantic views together, same as the page `data` was drawn from before
+ * this function's own filter ran. With the Semantic Layers flag on and
+ * enough semantic views sorted ahead of the wanted datasets on the current
+ * search, a page that filters down to nothing still reports more rows
+ * exist, but `AsyncSelect` only requests the next page on scroll — and a
+ * dropdown with nothing to scroll never gets the chance. Narrow (flag off,
+ * the filter is a no-op and this never applies) and not addressed here;
+ * paging forward internally past an empty filtered page, or asking the
+ * backend to exclude semantic views from the query in the first place,
+ * would close it.
+ */
+async function loadDatasetOnlyOptions(
+  search: string,
+  page: number,
+  pageSize: number,
+) {
+  const { data, totalCount } = await loadDatasetOptions(search, page, 
pageSize);
+  return {
+    data: data.filter(option => option.kind !== 'semantic_view'),
+    totalCount,
+  };

Review Comment:
   **Suggestion:** The loader removes semantic views from each page but returns 
the combined endpoint's unfiltered `totalCount`. If the first page contains 
only semantic views, `AsyncSelect` receives no options and cannot scroll to 
trigger the next page even though the count says more results exist, so valid 
datasets later in the result set are permanently unavailable. Continue fetching 
pages until a non-empty filtered page is found or query the backend with 
semantic views excluded. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Valid datasets can disappear from Dashboard V2 selection.
   - ⚠️ Semantic Layers pagination produces empty first pages.
   - ❌ Dataset-aware column and metric controls cannot initialize.
   ```
   </details>
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5581d45af3794b2a94a6f86b5aef64d0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5581d45af3794b2a94a6f86b5aef64d0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/src/pages/DashboardBuilderV2/schemaControlRenderers.tsx
   **Line:** 1166:1175
   **Comment:**
        *Api Mismatch: The loader removes semantic views from each page but 
returns the combined endpoint's unfiltered `totalCount`. If the first page 
contains only semantic views, `AsyncSelect` receives no options and cannot 
scroll to trigger the next page even though the count says more results exist, 
so valid datasets later in the result set are permanently unavailable. Continue 
fetching pages until a non-empty filtered page is found or query the backend 
with semantic views excluded.
   
   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%2F43588&comment_hash=da03b8e5b04f6c6f9e3558cba0b5c4b0a33acf5e3c7178f0d6d164a7e7a3ae0e&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43588&comment_hash=da03b8e5b04f6c6f9e3558cba0b5c4b0a33acf5e3c7178f0d6d164a7e7a3ae0e&reaction=dislike'>👎</a>



##########
superset/mcp_service/widgets/tool/set_widget_control_values.py:
##########
@@ -0,0 +1,107 @@
+# 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.
+
+"""MCP tool: set_widget_control_values"""
+
+from __future__ import annotations
+
+import logging
+from typing import Any, Dict
+
+from superset_core.mcp.decorators import tool, ToolAnnotations
+
+from superset.mcp_service.widgets.node_store import nodes
+from superset.mcp_service.widgets.utils import (
+    resolve_widget,
+    unknown_node_error,
+    unknown_widget_type_error,
+)
+
+logger = logging.getLogger(__name__)
+
+
+def _set_widget_control_values_impl(
+    node_id: str,
+    control_values: Dict[str, Any],
+) -> Dict[str, Any]:
+    """Pure logic: validate-then-commit a widget node's control values.
+
+    Builds a candidate (the node's current ``props`` shallow-merged with
+    ``control_values`` -- new keys override, everything else is preserved,
+    mirroring how the frontend's ``DashboardProvider.updateProps`` merges)
+    without touching the stored node. Validates the candidate through
+    ``Widget.validate_control_values`` -- the same commit-time gate the
+    ``/type/<widget_type>/validate`` REST endpoint uses. Only on success is
+    the node's ``props`` replaced with the candidate, a single dict
+    reassignment, so a validation failure leaves the stored node completely
+    unchanged: there is nothing to roll back because nothing was mutated in
+    place.
+    """
+    node = nodes.get(node_id)
+    if node is None:
+        return unknown_node_error(node_id)
+
+    widget = resolve_widget(node.widget_type)
+    if widget is None:
+        return unknown_widget_type_error(node.widget_type)
+
+    candidate = {**node.props, **control_values}
+    if errors := widget.validate_control_values(candidate):
+        return {"errors": errors}
+
+    # model_validate succeeded once already inside validate_control_values;
+    # this second call is cheap and deterministic on the same input, and is
+    # what gets us the normalized (coerced, alias-keyed) values to return and
+    # store, rather than the raw candidate validate_control_values discards.
+    normalized = widget.controls_class.model_validate(candidate).model_dump(

Review Comment:
   **Suggestion:** `validate_control_values` deliberately returns no errors for 
a falsy candidate, but this code then unconditionally performs strict model 
validation. For an empty node receiving `{}`, `model_validate` raises for 
missing required fields instead of returning the documented structured error 
response. Normalize only after a successful strict validation or handle the 
validation exception and return its errors. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ MCP mutation tool can raise on empty node updates.
   - ⚠️ Callers receive an exception instead of structured errors.
   - ❌ Tool reliability differs from its documented contract.
   ```
   </details>
   
   [![Use CodeAnt 
Skill](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/use-codeant-skill-flat-v2.svg)](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
 [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c5cd97bf56f64def84b21937cf09d9fd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c5cd97bf56f64def84b21937cf09d9fd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/mcp_service/widgets/tool/set_widget_control_values.py
   **Line:** 62:70
   **Comment:**
        *Api Mismatch: `validate_control_values` deliberately returns no errors 
for a falsy candidate, but this code then unconditionally performs strict model 
validation. For an empty node receiving `{}`, `model_validate` raises for 
missing required fields instead of returning the documented structured error 
response. Normalize only after a successful strict validation or handle the 
validation exception and return its errors.
   
   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%2F43588&comment_hash=422a3acbab3fdd22c62b482fe68126827cbebd710fd6e0d40360d406845fe2b2&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43588&comment_hash=422a3acbab3fdd22c62b482fe68126827cbebd710fd6e0d40360d406845fe2b2&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