codeant-ai-for-open-source[bot] commented on code in PR #45392: URL: https://github.com/apache/superset/pull/45392#discussion_r4298488005
########## superset-frontend/src/core/clientTools/sqlLab/index.ts: ########## @@ -0,0 +1,238 @@ +/** + * 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 type { chat, sqlLab as sqlLabApi } from '@apache-superset/core'; +import { navigation } from 'src/core/navigation'; +import { sqlLab } from 'src/core/sqlLab'; + +const emptySchema = { + type: 'object', + properties: {}, + additionalProperties: false, +}; +const editorIdSchema = { + type: 'object', + properties: { editor_id: { type: 'string', minLength: 1 } }, + required: ['editor_id'], + additionalProperties: false, +}; + +/** Validates inputs even when a consumer invokes a handler without schema validation. */ +function inputObject(input: unknown): Record<string, unknown> { + if (!input || typeof input !== 'object' || Array.isArray(input)) { + throw new Error('Expected an object input'); + } + return input as Record<string, unknown>; +} + +/** Reads a required string, allowing an empty SQL buffer. */ +function stringInput(input: Record<string, unknown>, key: string): string { + const value = input[key]; + if (typeof value !== 'string' || (key !== 'sql' && !value.trim())) { + throw new Error(`Expected a string for "${key}"`); + } + return value; +} + +/** Resolves the active tab at invocation time. */ +function currentTab(): sqlLabApi.Tab { + const tab = sqlLab.getCurrentTab(); + if (!tab) throw new Error('No active SQL editor tab'); + return tab; +} + +/** Resolves a target before APIs that silently ignore missing tab IDs are called. */ +function targetTab(input: unknown): sqlLabApi.Tab { + const editorId = stringInput(inputObject(input), 'editor_id'); + const tab = sqlLab.getTabs().find(item => item.id === editorId); + if (!tab) throw new Error(`SQL editor "${editorId}" not found`); + return tab; +} + +/** Serializes tab metadata without waiting for inactive editors to mount. */ +function tabMetadata(tab: sqlLabApi.Tab) { + return { + editor_id: tab.id, + title: tab.title, + databaseId: tab.databaseId, + catalog: tab.catalog, + schema: tab.schema, + }; +} + +/** SQL Lab tools backed exclusively by the existing frontend API. */ +const sqlLabTools: chat.ClientTool[] = [ + { + name: 'sqlLab__get_current_editor_id', + description: 'Get the ID of the active SQL Lab editor tab.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: () => ({ success: true, editor_id: currentTab().id }), + }, + { + name: 'sqlLab__get_editor_tabs', + description: + 'List open SQL Lab tabs and their database, catalog, and schema. Switch tabs before reading or editing their SQL.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: () => ({ + success: true, + editors: sqlLab.getTabs().map(tabMetadata), + }), + }, + { + name: 'sqlLab__get_databases', + description: 'List database IDs and names already loaded in SQL Lab.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: () => ({ + success: true, + databases: sqlLab.getDatabases().map(({ id, name }) => ({ id, name })), + }), + }, + { + name: 'sqlLab__get_editor_sql', + description: 'Read the active SQL Lab editor SQL, selection, and metadata.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: async () => { + const tab = currentTab(); + const editor = await tab.getEditor(); Review Comment: **Suggestion:** If the tab closes before its editor handle mounts, this `getEditor()` wait never settles; SQL read and update calls then hang instead of returning an error. **Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Resource leak` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=415d43d40d9642a595ea046a4adacf49&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=415d43d40d9642a595ea046a4adacf49&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/clientTools/sqlLab/index.ts **Line:** 116:116 **Comment:** *Resource Leak: If the tab closes before its editor handle mounts, this `getEditor()` wait never settles; SQL read and update calls then hang instead of returning an error. 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%2F45392&comment_hash=8b47b7caf0bed25bca5fb46fd482f3971ca7dd5b69b983eaf43d28e9f3ef3616&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45392&comment_hash=8b47b7caf0bed25bca5fb46fd482f3971ca7dd5b69b983eaf43d28e9f3ef3616&reaction=dislike'>๐</a> ########## superset-frontend/src/core/clientTools/sqlLab/index.ts: ########## @@ -0,0 +1,238 @@ +/** + * 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 type { chat, sqlLab as sqlLabApi } from '@apache-superset/core'; +import { navigation } from 'src/core/navigation'; +import { sqlLab } from 'src/core/sqlLab'; + +const emptySchema = { + type: 'object', + properties: {}, + additionalProperties: false, +}; +const editorIdSchema = { + type: 'object', + properties: { editor_id: { type: 'string', minLength: 1 } }, + required: ['editor_id'], + additionalProperties: false, +}; + +/** Validates inputs even when a consumer invokes a handler without schema validation. */ +function inputObject(input: unknown): Record<string, unknown> { + if (!input || typeof input !== 'object' || Array.isArray(input)) { + throw new Error('Expected an object input'); + } + return input as Record<string, unknown>; +} + +/** Reads a required string, allowing an empty SQL buffer. */ +function stringInput(input: Record<string, unknown>, key: string): string { + const value = input[key]; + if (typeof value !== 'string' || (key !== 'sql' && !value.trim())) { + throw new Error(`Expected a string for "${key}"`); + } + return value; +} + +/** Resolves the active tab at invocation time. */ +function currentTab(): sqlLabApi.Tab { + const tab = sqlLab.getCurrentTab(); + if (!tab) throw new Error('No active SQL editor tab'); + return tab; +} + +/** Resolves a target before APIs that silently ignore missing tab IDs are called. */ +function targetTab(input: unknown): sqlLabApi.Tab { + const editorId = stringInput(inputObject(input), 'editor_id'); + const tab = sqlLab.getTabs().find(item => item.id === editorId); + if (!tab) throw new Error(`SQL editor "${editorId}" not found`); + return tab; +} + +/** Serializes tab metadata without waiting for inactive editors to mount. */ +function tabMetadata(tab: sqlLabApi.Tab) { + return { + editor_id: tab.id, + title: tab.title, + databaseId: tab.databaseId, + catalog: tab.catalog, + schema: tab.schema, + }; +} + +/** SQL Lab tools backed exclusively by the existing frontend API. */ +const sqlLabTools: chat.ClientTool[] = [ + { + name: 'sqlLab__get_current_editor_id', + description: 'Get the ID of the active SQL Lab editor tab.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: () => ({ success: true, editor_id: currentTab().id }), + }, + { + name: 'sqlLab__get_editor_tabs', + description: + 'List open SQL Lab tabs and their database, catalog, and schema. Switch tabs before reading or editing their SQL.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: () => ({ + success: true, + editors: sqlLab.getTabs().map(tabMetadata), + }), + }, + { + name: 'sqlLab__get_databases', + description: 'List database IDs and names already loaded in SQL Lab.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: () => ({ + success: true, + databases: sqlLab.getDatabases().map(({ id, name }) => ({ id, name })), + }), + }, + { + name: 'sqlLab__get_editor_sql', + description: 'Read the active SQL Lab editor SQL, selection, and metadata.', + inputSchema: emptySchema, + annotations: { readOnlyHint: true }, + handler: async () => { + const tab = currentTab(); + const editor = await tab.getEditor(); + return { + success: true, + editor: { + ...tabMetadata(tab), + sql: editor.getValue(), + selectedText: editor.getSelectedText(), + }, + }; + }, + }, + { + name: 'sqlLab__update_editor_sql', + description: + 'Replace the SQL in the active SQL Lab editor without running or saving it as a saved query.', + inputSchema: { + type: 'object', + properties: { sql: { type: 'string' } }, + required: ['sql'], + additionalProperties: false, + }, + annotations: { readOnlyHint: false, destructiveHint: true }, + handler: async input => { + const sql = stringInput(inputObject(input), 'sql'); + const tab = currentTab(); + const editor = await tab.getEditor(); + editor.setValue(sql); + return { success: true, editor_id: tab.id }; + }, + }, + { + name: 'sqlLab__create_editor_tab', + description: + 'Open a SQL Lab tab without executing SQL. Use get_databases to find a database ID.', + inputSchema: { + type: 'object', + properties: { + title: { type: 'string', minLength: 1 }, + databaseId: { type: 'integer', minimum: 1 }, + sql: { type: 'string' }, + catalog: { type: 'string' }, + schema: { type: 'string' }, + }, + required: ['title', 'databaseId'], + additionalProperties: false, + }, + annotations: { readOnlyHint: false, destructiveHint: false }, + handler: async input => { + const args = inputObject(input); + const title = stringInput(args, 'title'); + const { databaseId } = args; + if ( + typeof databaseId !== 'number' || + !Number.isInteger(databaseId) || + !sqlLab.getDatabases().some(db => db.id === databaseId) + ) { + throw new Error('Expected a databaseId available in SQL Lab'); + } + const options: sqlLabApi.CreateTabOptions = { title, databaseId }; Review Comment: **Suggestion:** When `databaseId` differs from the active tab, omitted catalog and schema values are inherited from that tab, leaving the new tab with mismatched database context. **Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Api mismatch` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=24d84c4fbe494646bf9710f397155501&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=24d84c4fbe494646bf9710f397155501&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/clientTools/sqlLab/index.ts **Line:** 174:174 **Comment:** *Api Mismatch: When `databaseId` differs from the active tab, omitted catalog and schema values are inherited from that tab, leaving the new tab with mismatched database context. 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%2F45392&comment_hash=f9b40a960863f1e44b54668314a2c39641c396a200520b05f71aec0505957ec7&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45392&comment_hash=f9b40a960863f1e44b54668314a2c39641c396a200520b05f71aec0505957ec7&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]
