sadpandajoe commented on code in PR #45392:
URL: https://github.com/apache/superset/pull/45392#discussion_r4304053027
##########
superset-frontend/src/core/sqlLab/sqlLab.test.ts:
##########
@@ -548,6 +553,92 @@ test('getTabs returns all open tabs', () => {
expect(tabs[0].title).toBe('Untitled Query 1');
});
+test.each(['switch_editor_tab', 'close_editor_tab'])(
+ '%s can address an unvisited persisted tab',
+ async name => {
+ const persisted = { ...makeSecondEditor(), dbId: undefined, loaded: false
};
+ mockStore.dispatch({ type: ADD_QUERY_EDITOR, queryEditor: persisted });
Review Comment:
`ADD_QUERY_EDITOR` already makes the persisted tab the active one, so the
`switch_editor_tab` case here passes even if `setActiveTab` ignored tabs whose
`databaseId` is `undefined`. Could this re-activate `EDITOR_ID` after adding
the persisted tab, so the final `getCurrentTab()` assertion actually proves the
switch?
##########
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 ?? null,
+ 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);
Review Comment:
If the model rewrites the editor with the SQL it already contains (say the
buffer shows `SELECT 1` and it calls `update_editor_sql({ sql: 'SELECT 1' })`),
Ace's `setValue` emits a removal change and then an insertion change.
`EditorWrapper.onChangeText` accepts the empty string, but then drops the
insertion because it equals the render-time `sql`, so the queued local state
and Redux SQL end up empty and the next render clears the buffer while the tool
reports success. Would skipping the `setValue` call when `editor.getValue() ===
sql` avoid that?
--
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]
