codeant-ai-for-open-source[bot] commented on code in PR #40280:
URL: https://github.com/apache/superset/pull/40280#discussion_r3275597937
##########
superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx:
##########
@@ -254,7 +254,9 @@ export default function AddSemanticViewModal({
!schema?.properties ||
Object.keys(schema.properties).length === 0
) {
- // No runtime config needed — fetch views right away
+ // Preserve top-level runtime metadata (e.g. x-singleView) even when
+ // there are no form fields, then fetch views right away.
+ applyRuntimeSchema(schema);
Review Comment:
**Suggestion:** `schema` is treated as optional in the condition
(`!schema?.properties`), but then it is passed directly to
`applyRuntimeSchema`. If the API returns `null`/`undefined` for `json.result`,
`sanitizeSchema` will dereference `schema.properties` and crash. Add a null
guard before calling `applyRuntimeSchema`, and only apply when a real schema
object is present. [null pointer]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Add Semantic View modal crashes on null runtime schemas.
- ⚠️ Semantic layer plugins returning null schemas become unusable.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Dataset List page, which mounts `AddSemanticViewModal` via
`<AddSemanticViewModal />` in
`superset-frontend/src/pages/DatasetList/index.tsx` (around
the block shown at lines 1468–1480 in the tool output).
2. In the modal implementation `AddSemanticViewModal`
(`superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx`),
select a
semantic layer in the "Semantic Layer" `<Select>`; this triggers
`handleLayerChange`
(useCallback starting around lines 88–120 in the tool output), which calls
`SupersetClient.post` to `POST /api/v1/semantic_layer/<uuid>/schema/runtime`
(lines
108–110).
3. On the backend, the `runtime_schema` view in
`superset/semantic_layers/api.py` (lines
669–676) calls `cls.get_runtime_schema(...)` and returns its result as
`payload["result"]`. Configure or implement a semantic layer extension whose
`get_runtime_schema` returns `None`/`null` instead of a JSON dict (violating
but not
enforced by the abstract contract at
`superset-core/src/superset_core/semantic_layers/layer.py:80-88`).
4. With `json.result` now `null`, `handleLayerChange` assigns `const schema =
json.result;` and the condition `if (!schema?.properties ||
Object.keys(schema.properties).length === 0)` (lines 113–117) evaluates
truthy. In this
branch it calls `applyRuntimeSchema(schema);` at diff line 259.
`applyRuntimeSchema`
(lines 64–69) immediately calls `sanitizeSchema(rawSchema)` from
`superset-frontend/src/features/semanticLayers/jsonFormsHelpers.tsx` (lines
8–25), which
dereferences `schema.properties` without a null check. Because `schema` is
`null`, this
causes a runtime TypeError/exception, crashing the Add Semantic View modal
whenever a
misbehaving or future extension returns a null runtime schema.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=85c46f45ea834020a14b3a970d19a960&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=85c46f45ea834020a14b3a970d19a960&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/semanticViews/AddSemanticViewModal.tsx
**Line:** 259:259
**Comment:**
*Null Pointer: `schema` is treated as optional in the condition
(`!schema?.properties`), but then it is passed directly to
`applyRuntimeSchema`. If the API returns `null`/`undefined` for `json.result`,
`sanitizeSchema` will dereference `schema.properties` and crash. Add a null
guard before calling `applyRuntimeSchema`, and only apply when a real schema
object is present.
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%2F40280&comment_hash=e38098ee77923bb500bc99408f080d81b059ffa9a1b178bc458bed8b3dbb6fad&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40280&comment_hash=e38098ee77923bb500bc99408f080d81b059ffa9a1b178bc458bed8b3dbb6fad&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/semanticViews/AddSemanticViewModal.tsx:
##########
@@ -456,6 +458,32 @@ export default function AddSemanticViewModal({
const viewsDisabled =
loadingViews || (!loadingViews && availableViews.length === 0);
+ // When ``x-singleView: true`` the runtime form fully describes a single
+ // semantic view (e.g. a MetricFlow cube). Hide the picker and auto-select
+ // whatever ``get_semantic_views`` returned so the Add button can fire
+ // without an extra user click.
+ const singleViewMode =
+ (runtimeSchema as Record<string, unknown> | null)?.['x-singleView'] ===
+ true;
+
+ useEffect(() => {
+ if (!singleViewMode) return;
+ const namesToAdd = availableViews
+ .filter(v => !v.already_added)
+ .map(v => v.name)
+ .sort((a, b) => a.localeCompare(b))
+ .slice(0, 1);
Review Comment:
**Suggestion:** Auto-selection in single-view mode sorts names
alphabetically before picking one, which can select a different view than the
backend-provided first candidate. This contradicts the intended "first returned
not-yet-added view" behavior and can add the wrong semantic view; remove the
sort and pick the first eligible item in API order. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Wrong semantic view auto-selected in single-view runtime mode.
- ⚠️ Users may add unintended views, causing configuration mistakes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Implement or configure a semantic layer backend whose
`get_runtime_schema` adds a
top-level `"x-singleView": true` flag to its runtime schema (contract
defined at
`superset-core/src/superset_core/semantic_layers/layer.py:80-88`, and
returned via
`runtime_schema` in `superset/semantic_layers/api.py:669-676`). In the
frontend, this
schema is stored as `runtimeSchema` in `AddSemanticViewModal` and used to
compute
`singleViewMode` at diff lines 465–468.
2. From the Dataset List page
(`superset-frontend/src/pages/DatasetList/index.tsx`), open
`AddSemanticViewModal` for that layer and select the semantic layer in the
"Semantic
Layer" `<Select>`. `handleLayerChange` in `AddSemanticViewModal.tsx` (lines
88–120)
fetches the runtime schema and, when there are no runtime fields, calls
`fetchViews(uuid,
{}, gen);` directly after `applyRuntimeSchema` (diff lines 259–261).
`fetchViews` (lines
42–53) posts to `POST /api/v1/semantic_layer/<uuid>/views` and sets
`availableViews` from
`json.result`.
3. On the backend, `views` in `superset/semantic_layers/api.py` (lines
60–62, 75–80)
delegates to `layer.implementation.get_semantic_views(runtime_data)` and
returns the
results in order. The unit test `test_views` at
`superset/tests/unit_tests/semantic_layers/api_test.py:1920-33` asserts that
the first
element of the JSON response preserves backend order (`result[0]["name"] ==
"View A"`).
Configure a backend implementation whose `get_semantic_views` returns
multiple
not-yet-added views in a deliberate order that is not alphabetical (e.g.,
`["Primary
View", "Alternate View"]` where the first element is the intended default).
4. Once `availableViews` is populated and `singleViewMode` is true, the
`useEffect`
introduced at diff lines 469–475 runs. It computes `namesToAdd` as:
```ts
const namesToAdd = availableViews
.filter(v => !v.already_added)
.map(v => v.name)
.sort((a, b) => a.localeCompare(b))
.slice(0, 1);
```
This sorts by `v.name` before taking the first element, so the
lexicographically
smallest name is chosen instead of the first backend-provided candidate.
`setSelectedViewNames` then stores this sorted-first name. When the user
clicks
**Add**, `handleSave` (lines 195–205) filters `availableViews` by
`selectedViewNames`
and posts them to `/api/v1/semantic_view/`, causing the wrong semantic
view
(alphabetically first, not the backend's first candidate) to be created
in single-view
mode.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5c87120e7c7e4381acaf592672461e56&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=5c87120e7c7e4381acaf592672461e56&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/semanticViews/AddSemanticViewModal.tsx
**Line:** 471:475
**Comment:**
*Logic Error: Auto-selection in single-view mode sorts names
alphabetically before picking one, which can select a different view than the
backend-provided first candidate. This contradicts the intended "first returned
not-yet-added view" behavior and can add the wrong semantic view; remove the
sort and pick the first eligible item in API order.
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%2F40280&comment_hash=97bf60e7c34e75f9c0132178b5f73aef24d9c2433b5f447bb1085dffe334c42d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40280&comment_hash=97bf60e7c34e75f9c0132178b5f73aef24d9c2433b5f447bb1085dffe334c42d&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]
