Skip to content

Add Metabase AI Tools #17673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add Metabase AI Tools #17673

wants to merge 12 commits into from

Conversation

vimtor
Copy link
Contributor

@vimtor vimtor commented Mar 8, 2025

Description

As can be seen in the screencast, the first question I ask uses an existing query stored in Metabase. The second one is not stored, and the AI is able to figure out the PostgreSQL query.

I plan for future contributions to add more DDL queries besides PostgreSQL to improve accuracy.

Screencast

https://share.cleanshot.com/X7QXDL1Z

Checklist

@raycastbot raycastbot added extension fix / improvement Label for PRs with extension's fix improvements extension: metabase Issues related to the metabase extension OP is author The OP of the PR is the author of the extension labels Mar 8, 2025
@raycastbot
Copy link
Collaborator

Thank you for the update! 🎉

Due to our current reduced availability, the initial review may take up to 10-15 business days

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR adds AI tools to the Metabase extension, enabling natural language querying of databases and saved questions through Raycast's AI interface.

  • The tools section in package.json needs evals in the ai section - please add more test cases following the evals documentation
  • API calls in api.ts should handle failed responses with proper error handling before JSON parsing
  • launchCommand in run-query.ts and run-question.ts should be wrapped in try/catch blocks
  • Consider using showFailureToast from @raycast/utils instead of showToast for error handling in the API calls
  • Since there are multiple commands, consider adding "Metabase" as subtitle to each command for better context

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

8 file(s) reviewed, 11 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +51 to +71
export async function runQuery(input: { query: string; databaseId: number }) {
const endpoint = new URL(`/api/dataset`, preferences.instanceUrl);

return fetch(endpoint.toString(), {
method: "POST",
headers: {
accept: "application/json",
"content-type": "application/json",
"x-api-key": preferences.apiToken,
},
body: JSON.stringify({
database: input.databaseId,
type: "native",
native: {
query: input.query,
},
}),
})
.then((res) => res.json() as Promise<{ data: Query }>)
.then((res) => res.data);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: No error handling for failed requests. Should check res.ok before parsing JSON and handle network errors in the catch block. Consider using showFailureToast from @raycast/utils for error handling.

Suggested change
export async function runQuery(input: { query: string; databaseId: number }) {
const endpoint = new URL(`/api/dataset`, preferences.instanceUrl);
return fetch(endpoint.toString(), {
method: "POST",
headers: {
accept: "application/json",
"content-type": "application/json",
"x-api-key": preferences.apiToken,
},
body: JSON.stringify({
database: input.databaseId,
type: "native",
native: {
query: input.query,
},
}),
})
.then((res) => res.json() as Promise<{ data: Query }>)
.then((res) => res.data);
}
export async function runQuery(input: { query: string; databaseId: number }) {
const endpoint = new URL(`/api/dataset`, preferences.instanceUrl);
try {
const response = await fetch(endpoint.toString(), {
method: "POST",
headers: {
accept: "application/json",
"content-type": "application/json",
"x-api-key": preferences.apiToken,
},
body: JSON.stringify({
database: input.databaseId,
type: "native",
native: {
query: input.query,
},
}),
});
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}
const json = await response.json() as { data: Query };
return json.data;
} catch (error) {
showFailureToast(error instanceof Error ? error.message : "Failed to run query");
throw error;
}
}

Comment on lines +91 to +93
accept: "application/json",
contentType: "application/json",
"x-api-key": preferences.apiToken,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Header name should be 'content-type' (hyphenated) instead of 'contentType' to match other requests and HTTP standards

Suggested change
accept: "application/json",
contentType: "application/json",
"x-api-key": preferences.apiToken,
accept: "application/json",
"content-type": "application/json",
"x-api-key": preferences.apiToken,

Comment on lines +48 to +56
"evals": [
{
"input": "@metabase How many users do I have?",
"expected": []
},
{
"input": "@metabase What databases do I have connected?",
"expected": []
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Evals are empty arrays. Need at least one eval with expected output to validate AI behavior. See https://developers.raycast.com/ai/write-evals-for-your-ai-extension

Comment on lines +3 to +4
export default async function () {
const questions = await getQuestions();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: getQuestions() call should be wrapped in try/catch to handle API errors gracefully

Suggested change
export default async function () {
const questions = await getQuestions();
export default async function () {
try {
const questions = await getQuestions();

@@ -0,0 +1,23 @@
import { getQuestions } from "../lib/api";

export default async function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Function lacks TypeScript return type annotation

Comment on lines +32 to +42
try {
const ddl = await runQuery({
query: ddlQueries[database.engine],
databaseId: database.id,
});

schema.tables = JSON.parse(ddl.rows[0][0]);
if (ddl.rows[0][1]) {
schema.enums = JSON.parse(ddl.rows[0][1]);
}
} catch {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: runQuery should be wrapped in try/catch with showFailureToast from @raycast/utils for better error handling

Suggested change
try {
const ddl = await runQuery({
query: ddlQueries[database.engine],
databaseId: database.id,
});
schema.tables = JSON.parse(ddl.rows[0][0]);
if (ddl.rows[0][1]) {
schema.enums = JSON.parse(ddl.rows[0][1]);
}
} catch {
try {
const ddl = await runQuery({
query: ddlQueries[database.engine],
databaseId: database.id,
});
schema.tables = JSON.parse(ddl.rows[0][0]);
if (ddl.rows[0][1]) {
schema.enums = JSON.parse(ddl.rows[0][1]);
}
} catch (error) {
showFailureToast("Failed to fetch database schema", error);

Comment on lines +11 to +14
export default async function (input: Input) {
const result = await runQuery(input);

return { result };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: runQuery should be wrapped in try/catch to handle potential API errors gracefully

Suggested change
export default async function (input: Input) {
const result = await runQuery(input);
return { result };
export default async function (input: Input) {
try {
const result = await runQuery(input);
return { result };
} catch (error) {
throw new Error(`Failed to run query: ${error.message}`);
}

Comment on lines +17 to +19
export const confirmation: Tool.Confirmation<Input> = async (input) => {
const database = await getDatabase(input.databaseId);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: getDatabase call should be wrapped in try/catch to handle API errors

Suggested change
export const confirmation: Tool.Confirmation<Input> = async (input) => {
const database = await getDatabase(input.databaseId);
export const confirmation: Tool.Confirmation<Input> = async (input) => {
try {
const database = await getDatabase(input.databaseId);

Comment on lines +9 to +13
export default async function (input: Input) {
const result = await runQuestion(input);

return { result };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: runQuestion should be wrapped in try/catch to handle potential API errors gracefully

Suggested change
export default async function (input: Input) {
const result = await runQuestion(input);
return { result };
}
export default async function (input: Input) {
try {
const result = await runQuestion(input);
return { result };
} catch (error) {
throw new Error(`Failed to run question: ${error.message}`);
}
}

Comment on lines +36 to +37
value: question.name,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: question.name could be undefined - needs null check like other fields

@vimtor
Copy link
Contributor Author

vimtor commented May 3, 2025

I'm not sure if I should do the AI changes pointed out @pernielsentikaer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Extension extension fix / improvement Label for PRs with extension's fix improvements extension: metabase Issues related to the metabase extension OP is author The OP of the PR is the author of the extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants