codeant-ai-for-open-source[bot] commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r3474594866
##########
superset-frontend/src/core/sqlLab/models.ts:
##########
@@ -62,6 +65,7 @@ export class Tab implements sqlLabType.Tab {
this.schema = schema;
this.editorGetter = editorGetter;
this.panels = panels;
+ this.backendId = backendId;
Review Comment:
**Suggestion:** `backendId` is assigned only from the optional constructor
argument, so it stays `undefined` whenever callers do not pass it explicitly.
In current SQL Lab state bootstrapping, persisted tabs commonly have only `id`
populated (with no `tabViewId`), so extensions reading `Tab.backendId` will not
be able to correlate tabs with backend `tabstateview` rows even though the tab
is backend-backed. Ensure `backendId` is derived for persisted tabs (or update
all constructors/callers to always pass it) so the new public API is
consistently populated. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ SQL Lab extensions cannot correlate preloaded tabs to backend rows.
- ⚠️ Extension features using backendId misbehave for bootstrapped tabs.
- ⚠️ Inconsistent Tab.backendId semantics across different tab creation
paths.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Enable SQL Lab backend persistence so the backend sends `tab_state_ids`
in bootstrap,
then inspect how initial SQL Lab state is built in
`superset-frontend/src/SqlLab/reducers/getInitialState.ts:40-78`. For each
backend tab
(`tab_state_ids.forEach`), `QueryEditor` instances are created with `id:
id.toString()`
but **no `tabViewId` property**, even though `id` is the backend
`tabstateview` row
identifier.
2. Start SQL Lab and obtain the current `sqlLab` Redux slice via
`getSqlLabState()` as
implemented in `superset-frontend/src/core/sqlLab/index.ts:62-65`. The
`queryEditors`
array contains these backend-backed editors with `id` set to the backend row
ID and
`tabViewId === undefined` (confirmed by the test comment "The preloaded
editor has no
tabViewId" in `src/core/sqlLab/sqlLab.test.ts:51-55`).
3. From an extension (or in tests), call `sqlLab.getTabs()`, which is
implemented at
`superset-frontend/src/core/sqlLab/index.ts:104-108`. This iterates over
`queryEditors`
and calls `getTab(qe.id)` for each editor; `getTab` (line 182 onward) reads
`tabViewId`
from the `QueryEditor` and passes it as the `backendId` argument to
`makeTab`:
- `const { name, dbId, catalog, schema, tabViewId } = queryEditor;`
- `return makeTab(id, name, dbId, catalog, schema, false, tabViewId);`
Since these bootstrapped editors have `tabViewId === undefined`,
`makeTab` is called
with `backendId` undefined.
4. `makeTab` constructs a `Tab` instance by calling the `Tab` constructor in
`superset-frontend/src/core/sqlLab/models.ts:22-40` and forwards the
`backendId` argument.
Inside the constructor, the new field is assigned directly at `models.ts:68`
as
`this.backendId = backendId;` with no fallback logic. As a result, for
backend-backed tabs
whose state originated from `tab_state_ids` (where `id` is the backend
`tabstateview`
row), `Tab.backendId` remains `undefined` even though the tab is persisted
and has a
backend row ID. Any extension that relies on `Tab.backendId` to correlate UI
tabs with
`/tabstateview/<id>` rows (instead of re-implementing the `tabViewId ?? id`
fallback used
in `SqlLab/actions/sqlLab.ts`) will be unable to correlate these common
bootstrapped tabs,
breaking the intended correlation contract.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=69df7236ba9b4988bdc7d9b61c790f19&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=69df7236ba9b4988bdc7d9b61c790f19&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/core/sqlLab/models.ts
**Line:** 68:68
**Comment:**
*Api Mismatch: `backendId` is assigned only from the optional
constructor argument, so it stays `undefined` whenever callers do not pass it
explicitly. In current SQL Lab state bootstrapping, persisted tabs commonly
have only `id` populated (with no `tabViewId`), so extensions reading
`Tab.backendId` will not be able to correlate tabs with backend `tabstateview`
rows even though the tab is backend-backed. Ensure `backendId` is derived for
persisted tabs (or update all constructors/callers to always pass it) so the
new public API is consistently populated.
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%2F41285&comment_hash=70e7ac47b75e4e4bf286b31a8c92e6207c4e7b2d7a6b5d88632b2717714e5088&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41285&comment_hash=70e7ac47b75e4e4bf286b31a8c92e6207c4e7b2d7a6b5d88632b2717714e5088&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]
