villebro commented on code in PR #43093:
URL: https://github.com/apache/superset/pull/43093#discussion_r3814615345
##########
superset-frontend/src/core/chat/ChatProvider.ts:
##########
@@ -29,6 +30,62 @@ import { createValueEventEmitter, createEventEmitter } from
'../utils';
type Chat = chatApi.Chat;
type DisplayMode = chatApi.DisplayMode;
+type ClientTool = chatApi.ClientTool;
+type ClaudeToolSpec = chatApi.ClaudeToolSpec;
+type ClientToolsFormat = chatApi.ClientToolsFormat;
+
+// The real value backing @apache-superset/core's `declare const
+// ClientToolsFormat` — that package only ever declares (see its own docs on
+// why this isn't a TS `enum`); this is the actual object attached to
+// `window.superset.chat.ClientToolsFormat` (re-exported from ./index).
+export const ClientToolsFormat = {
+ Claude: 'claude',
+ AgUi: 'ag-ui',
+ CopilotKit: 'copilot-kit',
+ Codex: 'codex',
+} as const;
+
+// AgUi/CopilotKit/Codex have no real transform below — see
+// @apache-superset/core's ClientToolsFormat docs for why (no framework in
+// this codebase actually talks to any of them yet, so there's no verified
+// target shape to convert to). Throwing a clear, named error beats either
+// silently returning the native ClientTool[] (wrong shape, and callers can
+// already get that from a plain getTools()) or returning an empty array
+// (looks like "this source has no tools" instead of "this format isn't
+// implemented").
+function notYetImplemented(
+ formatKey: keyof typeof ClientToolsFormat,
+): () => never {
+ return () => {
+ throw new Error(
+ `[Superset] chat.getTools(chat.ClientToolsFormat.${formatKey}) is ` +
+ 'not yet implemented — no framework in this codebase talks to ' +
+ 'this format yet, so there is no verified target shape to convert ' +
+ 'to. Add a real transform to CLIENT_TOOLS_FORMATTERS in ' +
+ 'ChatProvider.ts once there is one to verify against, rather than ' +
+ 'guessing at it here.',
+ );
+ };
+}
+
+// One entry per ClientToolsFormat member — see that constant's own docs for
+// why only Claude has a real transform. Keeping each target's transform (or
+// placeholder) here, keyed by the same object, is what makes adding a real
+// one later a single changed entry rather than a change to getTools()
+// itself.
+const CLIENT_TOOLS_FORMATTERS: {
+ [K in ClientToolsFormat]: (tools: ClientTool[]) => unknown[];
+} = {
+ [ClientToolsFormat.Claude]: (tools: ClientTool[]): ClaudeToolSpec[] =>
+ tools.map(tool => ({
+ name: tool.name,
+ description: tool.description,
+ input_schema: tool.inputSchema,
+ })),
+ [ClientToolsFormat.AgUi]: notYetImplemented('AgUi'),
+ [ClientToolsFormat.CopilotKit]: notYetImplemented('CopilotKit'),
+ [ClientToolsFormat.Codex]: notYetImplemented('Codex'),
+};
Review Comment:
Are we expecting to add support for these in the near future? If not, should
we make Claude format a hard requirement for now, and introduce this
generalization later once we have full visibility into what these alternative
formats actually look like? This seems borderline Big Design Up Front at the
risk of these other formats not becoming generally supported in the long term..
##########
superset-frontend/packages/superset-core/src/chat/index.ts:
##########
@@ -151,6 +151,182 @@ export declare const onDidChangeDisplayMode:
Event<DisplayMode>;
*/
export declare const onDidResizePanel: Event<{ width: number }>;
-// TODO: client actions API — tool availability functions will be added here
-// once the client_actions SIP is finalized. The chat namespace is the
-// intended integration point between the two SIPs.
+/**
+ * A client-side (frontend) tool the chat agent can call — see the "Client
+ * Tools" SIP. Unlike a backend/MCP-server tool, its handler runs in the
+ * browser and can read/mutate whatever is currently on screen (e.g. the
+ * Dashboard v2 canvas), so it works entirely off local state with no network
+ * round trip of its own.
+ *
+ * Contributed either by the host itself (built-in "core" tools) or by an
+ * extension, via {@link registerClientTool} called directly from its own
+ * module — same as {@link registerChat}/`commands.registerCommand`.
+ */
+export interface ClientTool {
+ /**
+ * Name WITHOUT your extension's own prefix, e.g. `dashboard__do_thing` —
+ * when {@link registerClientTool}/{@link registerClientTools} is called
+ * from your extension's own module (the normal case: `import { chat }
+ * from '@apache-superset/core'` inside your extension), the host
+ * automatically qualifies it with your extension id, so it's registered
+ * (and addressed in a tool call) as
`<your-extension-id>.dashboard__do_thing`.
+ * This only applies to calls made through that per-extension binding —
+ * host-internal code (this codebase's own "core" tools) isn't
+ * extension-scoped, so it manages its own prefix explicitly instead (see
+ * {@link registerClientTool}'s own docs).
+ */
+ name: string;
+ /** Describes what the tool does, so the LLM agent knows when to call it. */
+ description: string;
+ /** JSON Schema for the tool's input, e.g. `{ type: 'object', properties: {}
}`. */
+ inputSchema: Record<string, unknown>;
+ /** Invoked with the model's tool-call arguments; return value is reported
back as the tool result. */
+ handler: (input: unknown) => Promise<unknown> | unknown;
+}
+
+/**
+ * Registers a single client-side tool the chat agent can call — mirrors
+ * `commands.registerCommand`: a direct, imperative call an extension makes
+ * from its own module (typically its `./index` entry), not a declarative
+ * `extension.json` pointer the host resolves for you. Called from your
+ * extension's own module, your extension id is automatically prepended to
+ * `tool.name` (see {@link ClientTool.name}'s own docs) — write just
+ * `dashboard__do_thing`, not `my-extension.dashboard__do_thing`. Registering
+ * a second tool under the same (already-qualified) name overwrites the
+ * first (logged), and disposing the returned Disposable unregisters it.
+ *
+ * @example
+ * ```typescript
+ * import { chat } from '@apache-superset/core';
+ *
+ * chat.registerClientTool({
+ * name: 'dashboard__do_thing',
+ * description: '...',
+ * inputSchema: { type: 'object', properties: {} },
+ * handler: () => ({ success: true }),
+ * });
+ * ```
+ */
+export declare function registerClientTool(tool: ClientTool): Disposable;
+
+/**
+ * Registers a list of client-side tools in one call — equivalent to calling
+ * {@link registerClientTool} once per entry (including its automatic
+ * extension-id prefixing), but without writing that loop yourself. Disposing
+ * the returned Disposable unregisters every tool in the list.
+ *
+ * @example
+ * ```typescript
+ * import { chat } from '@apache-superset/core';
+ *
+ * chat.registerClientTools(getMyTools(chat));
+ * ```
+ */
+export declare function registerClientTools(tools: ClientTool[]): Disposable;
+
+/**
+ * A target AI-service wire format `getTools()` can convert to — see
+ * {@link getTools}'s overloads. Each member's transform lives alongside
+ * `ChatProvider.getTools()`'s implementation
+ * (`superset-frontend/src/core/chat/ChatProvider.ts`); adding a new member
+ * here means adding one case there, not redesigning anything.
+ *
+ * A plain `const` object + derived union, not a TS `enum` — this file only
+ * ever *declares* values (the real object lives in host code and is
+ * attached to `window.superset.chat`, same as every function below), and a
+ * real `enum`'s nominal typing doesn't structurally match a differently-built
+ * object the way this does, which would make the two copies incompatible.
+ *
+ * `Claude` is the only member with a real, verified transform —
+ * `devaigateway-provider`'s Anthropic SDK call, and `chat`'s own backend
+ * `ToolSpec`, both expect exactly {@link ClaudeToolSpec}'s shape.
+ * `AgUi`/`CopilotKit`/`Codex` are placeholders: nothing in this codebase
+ * talks to any of those frameworks today, so there is no real tool-spec
+ * shape here to convert to yet, and `getTools()` throws if one of them is
+ * passed (see its own docs) rather than guessing at an unverified shape.
+ * Replace a placeholder's `ChatProvider.ts` case with a real transform once
+ * that framework's actual expected shape is known.
+ */
+export declare const ClientToolsFormat: {
+ readonly Claude: 'claude';
+ readonly AgUi: 'ag-ui';
+ readonly CopilotKit: 'copilot-kit';
+ readonly Codex: 'codex';
+};
Review Comment:
So many formats already.. :sadpanda:
--
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]
