codeant-ai-for-open-source[bot] commented on code in PR #42535:
URL: https://github.com/apache/superset/pull/42535#discussion_r3676011874
##########
superset-frontend/src/features/semanticLayers/jsonFormsHelpers.tsx:
##########
@@ -342,38 +366,53 @@ export function MultiEnumControl(props: ControlProps) {
(arraySchema.items as Record<string, unknown>) ??
({} as Record<string, unknown>);
- const enumValues = (itemsSchema.enum as unknown[]) ?? [];
- const enumNames =
- (itemsSchema['x-enumNames'] as string[]) ?? enumValues.map(String);
+ // No fallback allocations out here: a fresh ``[]`` per render would give
+ // the memo new deps every time. Fallbacks live inside the memo.
+ const enumValues = Array.isArray(itemsSchema.enum)
+ ? (itemsSchema.enum as unknown[])
+ : undefined;
+ const enumNames = Array.isArray(itemsSchema['x-enumNames'])
+ ? (itemsSchema['x-enumNames'] as string[])
+ : undefined;
- const options = enumValues.map((value, index) => ({
- value: value as string | number,
- label: enumNames[index] ?? String(value),
- }));
+ // Memoized: the host form passes a fresh ``config`` to every control on
+ // each change, so without the memo a catalog of N options is rebuilt on
+ // every one of the user's M selections (O(N·M)).
+ const options = useMemo(
+ () =>
+ (enumValues ?? []).map((value, index) => ({
+ value: value as string | number,
+ label: enumNames?.[index] ?? String(value),
+ })),
+ [enumValues, enumNames],
+ );
const value = Array.isArray(props.data) ? (props.data as unknown[]) : [];
const tooltip = (props.uischema?.options as Record<string, unknown>)
?.tooltip as string | undefined;
return (
- <Form.Item label={props.label} tooltip={tooltip}>
+ <FormItem label={props.label} tooltip={tooltip}>
<Select
+ ariaLabel={props.label || undefined}
mode="multiple"
value={value as (string | number)[]}
onChange={next => props.handleChange(props.path, next)}
options={options}
- style={{ width: '100%' }}
disabled={!props.enabled}
loading={!!refreshingSchema}
allowClear
- optionFilterProp="label"
+ // Stability fix only: the wrapped Select's bulk select-all/clear
+ // affordance is out of scope for this control (sc-107832).
+ allowSelectAll={false}
+ optionFilterProps={['label']}
Review Comment:
**Suggestion:** Restricting filtering to `label` removes the wrapped
Select's default `value` search field. For enum fields with human-readable
`x-enumNames`, users who know the stored metric identifier or ID can no longer
find the option by that value. Preserve both `label` and `value` in the filter
properties. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dynamic enum fields cannot search stored identifiers.
- ⚠️ ID-based discovery fails when labels differ from values.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3a2d802a099c444ab0d5039bee40186c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3a2d802a099c444ab0d5039bee40186c&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/features/semanticLayers/jsonFormsHelpers.tsx
**Line:** 409:409
**Comment:**
*Api Mismatch: Restricting filtering to `label` removes the wrapped
Select's default `value` search field. For enum fields with human-readable
`x-enumNames`, users who know the stored metric identifier or ID can no longer
find the option by that value. Preserve both `label` and `value` in the filter
properties.
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%2F42535&comment_hash=914b3528fab648917c36eb329f93ae1f2efbf7e5c4dc3b16e121c38246a732f1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42535&comment_hash=914b3528fab648917c36eb329f93ae1f2efbf7e5c4dc3b16e121c38246a732f1&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/semanticLayers/jsonFormsHelpers.tsx:
##########
@@ -342,38 +366,53 @@ export function MultiEnumControl(props: ControlProps) {
(arraySchema.items as Record<string, unknown>) ??
({} as Record<string, unknown>);
- const enumValues = (itemsSchema.enum as unknown[]) ?? [];
- const enumNames =
- (itemsSchema['x-enumNames'] as string[]) ?? enumValues.map(String);
+ // No fallback allocations out here: a fresh ``[]`` per render would give
+ // the memo new deps every time. Fallbacks live inside the memo.
+ const enumValues = Array.isArray(itemsSchema.enum)
+ ? (itemsSchema.enum as unknown[])
+ : undefined;
+ const enumNames = Array.isArray(itemsSchema['x-enumNames'])
+ ? (itemsSchema['x-enumNames'] as string[])
+ : undefined;
- const options = enumValues.map((value, index) => ({
- value: value as string | number,
- label: enumNames[index] ?? String(value),
- }));
+ // Memoized: the host form passes a fresh ``config`` to every control on
+ // each change, so without the memo a catalog of N options is rebuilt on
+ // every one of the user's M selections (O(N·M)).
+ const options = useMemo(
+ () =>
+ (enumValues ?? []).map((value, index) => ({
+ value: value as string | number,
+ label: enumNames?.[index] ?? String(value),
+ })),
+ [enumValues, enumNames],
+ );
Review Comment:
**Suggestion:** JSON Schema enums may legally contain booleans or `null`,
but this option builder passes those values directly while asserting they are
`string | number`. The wrapped Select and its value-handling helpers use the
narrower value contract, so such options can be rejected, rendered incorrectly,
or fail to round-trip their original type through `handleChange`. Normalize or
explicitly support the full set of JSON Schema enum value types before using
this renderer. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Boolean and null enum choices may not round-trip.
- ⚠️ JSON Forms fields can diverge from backend schema values.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6979cfb4deeb4546a3af5377a77e3bdb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6979cfb4deeb4546a3af5377a77e3bdb&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/features/semanticLayers/jsonFormsHelpers.tsx
**Line:** 381:388
**Comment:**
*Type Error: JSON Schema enums may legally contain booleans or `null`,
but this option builder passes those values directly while asserting they are
`string | number`. The wrapped Select and its value-handling helpers use the
narrower value contract, so such options can be rejected, rendered incorrectly,
or fail to round-trip their original type through `handleChange`. Normalize or
explicitly support the full set of JSON Schema enum value types before using
this renderer.
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%2F42535&comment_hash=45a3e0a374846082091c97046f30f49b062253796e0ec004d8f25492e6a08b6b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42535&comment_hash=45a3e0a374846082091c97046f30f49b062253796e0ec004d8f25492e6a08b6b&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]
