From b278b1ec8a173158fa3dde096fed4bc7dc1a7744 Mon Sep 17 00:00:00 2001 From: Charis <26616127+charislam@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:27:38 -0400 Subject: [PATCH] fix(studio): Debug with Assistant and Copy prompt work (#49690) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary * Resolved hanging buttons in Explorer's QueryResultError panel that were wired to stub no-ops (`buildPrompt={() => ''}`, `onOpenAssistant={() => {}}`). * "Debug with Assistant" now opens a new chat seeded with a real prompt combining the SQL query and error context using existing `buildDebugPromptText` util and `useCreateChat` hook. * "Copy prompt" now copies the same real debug prompt text to clipboard. * Threaded `sql` and query `source` props down through `QueryEditor` → `QueryResultRenderer` → `QueryResultError` while keeping them optional for backward compatibility with other callers like `AssistantNotebookPreviewCell`. ## Test plan - [X] Run `pnpm typecheck` — passes - [X] Run `pnpm lint --filter=studio` — passes - [X] Run `pnpm test:studio --run apps/studio/components/interfaces/Explorer/QueryEditor` — Explorer vitest suite (99 tests across 13 files) passes with no regressions - [X] Manually verify in Explorer: trigger an ad-hoc SQL query that fails, confirm "Debug with Assistant" opens a new chat with the error prompt seeded, and "Copy prompt" copies the prompt to clipboard ## Summary by CodeRabbit - **New Features** - Added “Debug with Assistant” to query errors using the submitted SQL and error details. - Added an option to copy the debugging prompt for easier troubleshooting. - Assistant actions are hidden when query details are unavailable or restricted. - **Bug Fixes** - Ensured query errors reference the SQL that produced them, rather than later editor changes. - **Tests** - Added coverage for assistant debugging, prompt copying, conditional visibility, and self-hosted behavior. --- .../QueryResultError.selfhosted.test.tsx | 82 +++++++++ .../QueryEditor/QueryResultError.test.tsx | 161 ++++++++++++++++++ .../Explorer/QueryEditor/QueryResultError.tsx | 50 +++++- .../QueryEditor/QueryResultRenderer.tsx | 14 +- .../interfaces/Explorer/QueryEditor/index.tsx | 35 +++- .../components/interfaces/Explorer/types.ts | 4 + 6 files changed, 327 insertions(+), 19 deletions(-) create mode 100644 apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.selfhosted.test.tsx create mode 100644 apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.test.tsx diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.selfhosted.test.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.selfhosted.test.tsx new file mode 100644 index 00000000000..7ec4302a2fa --- /dev/null +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.selfhosted.test.tsx @@ -0,0 +1,82 @@ +import { screen } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { QueryResultError } from './QueryResultError' +import { customRender } from '@/tests/lib/custom-render' +import { addAPIMock } from '@/tests/lib/msw' + +const mocks = vi.hoisted(() => ({ + createChat: vi.fn(), + useParams: vi.fn(), +})) + +vi.mock('@/lib/constants', async () => { + const actual = await vi.importActual>('@/lib/constants') + return { ...actual, IS_PLATFORM: false } +}) + +vi.mock('common', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useParams: () => mocks.useParams() } +}) + +vi.mock('../hooks', () => ({ + useCreateChat: () => ({ createChat: mocks.createChat, isCreating: false }), +})) + +vi.mock('@/hooks/misc/useSelectedOrganization', () => ({ + useSelectedOrganizationQuery: () => ({ data: undefined }), +})) + +// Self-hosted has no orgs/billing, so these eligibility queries are expected to never +// resolve (disabled or failing) - the dropdown must not stay hidden waiting on them. +vi.mock('@/data/subscriptions/org-subscription-query', () => ({ + useOrgSubscriptionQuery: () => ({ data: undefined, isSuccess: false }), +})) + +vi.mock('@/data/config/project-settings-v2-query', () => ({ + useProjectSettingsV2Query: () => ({ data: undefined, isSuccess: false }), +})) + +describe('QueryResultError (self-hosted)', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.useParams.mockReturnValue({ ref: 'default' }) + addAPIMock({ + method: 'get', + path: '/platform/projects/:ref', + response: { + id: 1, + ref: 'default', + organization_id: 1, + name: 'Test Project', + status: 'ACTIVE_HEALTHY', + cloud_provider: 'AWS', + region: 'us-east-1', + db_host: 'db.default.supabase.co', + restUrl: 'https://default.supabase.co/rest/v1/', + inserted_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + subscription_id: 'sub_123', + is_branch_enabled: false, + is_physical_backups_enabled: false, + high_availability: false, + integration_source: null, + connectionString: 'postgresql://postgres@localhost:5432/postgres', + is_hibernating: false, + }, + }) + }) + + it('renders the assistant dropdown without waiting on HIPAA eligibility queries', () => { + customRender( + + ) + + expect(screen.getByRole('button', { name: 'Debug with Assistant' })).toBeInTheDocument() + }) +}) diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.test.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.test.tsx new file mode 100644 index 00000000000..574860d4a9f --- /dev/null +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.test.tsx @@ -0,0 +1,161 @@ +import { fireEvent, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { QueryResultError } from './QueryResultError' +import { customRender } from '@/tests/lib/custom-render' +import { addAPIMock } from '@/tests/lib/msw' + +const mocks = vi.hoisted(() => ({ + createChat: vi.fn(), + useParams: vi.fn(), + mockCopyToClipboard: vi.fn(), + useOrgSubscriptionQuery: vi.fn(), + useProjectSettingsV2Query: vi.fn(), +})) + +vi.mock('common', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useParams: () => mocks.useParams() } +}) + +// This file covers the platform-mode HIPAA eligibility gate; the self-hosted bypass is +// covered separately in QueryResultError.selfhosted.test.tsx. +vi.mock('@/lib/constants', async () => { + const actual = await vi.importActual>('@/lib/constants') + return { ...actual, IS_PLATFORM: true } +}) + +// CopyButton and AiAssistantDropdown write via copyToClipboard from 'ui'. Stub just that +// export so we can assert the value handed to the clipboard without depending on jsdom's +// document.hasFocus() / navigator.clipboard. Everything else in 'ui' stays real. +vi.mock('ui', async (importOriginal) => ({ + ...(await importOriginal()), + copyToClipboard: mocks.mockCopyToClipboard, +})) + +vi.mock('../hooks', () => ({ + useCreateChat: () => ({ createChat: mocks.createChat, isCreating: false }), +})) + +vi.mock('@/hooks/misc/useSelectedOrganization', () => ({ + useSelectedOrganizationQuery: () => ({ data: { slug: 'default-org' } }), +})) + +vi.mock('@/data/subscriptions/org-subscription-query', () => ({ + useOrgSubscriptionQuery: () => mocks.useOrgSubscriptionQuery(), +})) + +vi.mock('@/data/config/project-settings-v2-query', () => ({ + useProjectSettingsV2Query: () => mocks.useProjectSettingsV2Query(), +})) + +describe('QueryResultError', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.useParams.mockReturnValue({ ref: 'default' }) + mocks.useOrgSubscriptionQuery.mockReturnValue({ data: undefined, isSuccess: true }) + mocks.useProjectSettingsV2Query.mockReturnValue({ data: undefined, isSuccess: true }) + // useTrack() (invoked by AiAssistantDropdown) reads the selected project to attach + // telemetry context, so the platform project fetch needs a handler even though this + // component doesn't read project data itself. + addAPIMock({ + method: 'get', + path: '/platform/projects/:ref', + response: { + id: 1, + ref: 'default', + organization_id: 1, + name: 'Test Project', + status: 'ACTIVE_HEALTHY', + cloud_provider: 'AWS', + region: 'us-east-1', + db_host: 'db.default.supabase.co', + restUrl: 'https://default.supabase.co/rest/v1/', + inserted_at: '2024-01-01T00:00:00Z', + updated_at: '2024-01-01T00:00:00Z', + subscription_id: 'sub_123', + is_branch_enabled: false, + is_physical_backups_enabled: false, + high_availability: false, + integration_source: null, + connectionString: 'postgresql://postgres@localhost:5432/postgres', + is_hibernating: false, + }, + }) + }) + + it('opens a new assistant chat seeded with the query and error when debugging', () => { + customRender( + + ) + + fireEvent.click(screen.getByRole('button', { name: 'Debug with Assistant' })) + + expect(mocks.createChat).toHaveBeenCalledWith({ + name: 'Debug SQL snippet', + initialMessage: expect.stringContaining('select * from foo;'), + }) + expect(mocks.createChat.mock.calls[0][0].initialMessage).toContain( + 'relation "foo" does not exist' + ) + }) + + it('copies the same debug prompt text via the dropdown', async () => { + const user = userEvent.setup() + customRender( + + ) + + await user.click(screen.getByRole('button', { name: 'More actions' })) + await user.click(await screen.findByText('Copy prompt')) + + expect(mocks.mockCopyToClipboard).toHaveBeenCalledWith( + expect.stringContaining('select * from foo;') + ) + }) + + it('does not render the assistant dropdown when the query is unavailable', () => { + customRender() + + expect(screen.queryByRole('button', { name: 'Debug with Assistant' })).not.toBeInTheDocument() + }) + + it('does not render the assistant dropdown while HIPAA eligibility is still resolving', () => { + mocks.useOrgSubscriptionQuery.mockReturnValue({ data: undefined, isSuccess: false }) + + customRender( + + ) + + expect(screen.queryByRole('button', { name: 'Debug with Assistant' })).not.toBeInTheDocument() + }) + + it('does not render the assistant dropdown when an eligibility query is disabled or failed', () => { + // A disabled or failed query also settles with isSuccess: false forever - same as + // still-loading from this component's point of view, so it stays denied. + mocks.useProjectSettingsV2Query.mockReturnValue({ data: undefined, isSuccess: false }) + + customRender( + + ) + + expect(screen.queryByRole('button', { name: 'Debug with Assistant' })).not.toBeInTheDocument() + }) +}) diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.tsx index 83cc1081489..d83b46ed85f 100644 --- a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultError.tsx @@ -1,9 +1,13 @@ import { useParams } from 'common' import { ExternalLink } from 'lucide-react' import { parseAsBoolean, useQueryState } from 'nuqs' +import { useCallback } from 'react' import { Button, cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui' import { subscriptionHasHipaaAddon } from '../../Billing/Subscription/Subscription.utils' +import { type SqlSnippetSource } from '../../SQLEditor/querySource' +import { buildDebugPromptText } from '../../SQLEditor/SQLEditor.utils' +import { useCreateChat } from '../hooks' import { type QueryResult } from '../types' import { AiAssistantDropdown } from '@/components/ui/AiAssistantDropdown' import CopyButton from '@/components/ui/CopyButton' @@ -12,24 +16,53 @@ import { useProjectSettingsV2Query } from '@/data/config/project-settings-v2-que import { getSqlErrorLines } from '@/data/sql/utils' import { useOrgSubscriptionQuery } from '@/data/subscriptions/org-subscription-query' import { useSelectedOrganizationQuery } from '@/hooks/misc/useSelectedOrganization' -import { DOCS_URL } from '@/lib/constants' +import { DOCS_URL, IS_PLATFORM } from '@/lib/constants' export const QueryResultError = ({ error, autoLimit, + sql, + source, }: { error: NonNullable autoLimit?: QueryResult['autoLimit'] + sql?: string + source?: SqlSnippetSource }) => { const { ref } = useParams() const { data: org } = useSelectedOrganizationQuery() - const { data: subscription } = useOrgSubscriptionQuery({ orgSlug: org?.slug }) - const { data: projectSettings } = useProjectSettingsV2Query({ projectRef: ref }) + const { data: subscription, isSuccess: isSubscriptionResolved } = useOrgSubscriptionQuery({ + orgSlug: org?.slug, + }) + const { data: projectSettings, isSuccess: isProjectSettingsResolved } = useProjectSettingsV2Query( + { + projectRef: ref, + } + ) const hasHipaaAddon = subscriptionHasHipaaAddon(subscription) && projectSettings?.is_sensitive + // Default deny until both eligibility queries have actually succeeded - a disabled or + // failed query also reports isLoading: false, so isLoading can't tell "confirmed no + // addon" apart from "don't know yet", and the assistant sends the SQL and error to an + // LLM. Self-hosted has no HIPAA concept at all (subscriptionHasHipaaAddon short-circuits + // to false there), so there's nothing to wait on outside of platform. + const isCheckingHipaaEligibility = + IS_PLATFORM && (!isSubscriptionResolved || !isProjectSettingsResolved) + + const { createChat, isCreating } = useCreateChat() const [, setShowConnect] = useQueryState('showConnect', parseAsBoolean.withDefault(false)) + const canDebug = sql !== undefined && source !== undefined + + const buildDebugPrompt = useCallback( + () => (canDebug ? buildDebugPromptText(sql, error.message, source) : ''), + [canDebug, sql, error.message, source] + ) + + const handleDebug = () => + createChat({ name: 'Debug SQL snippet', initialMessage: buildDebugPrompt() }) + const isTimeout = error.message?.includes('canceling statement due to statement timeout') || error.message?.includes('upstream request timeout') || @@ -136,15 +169,14 @@ export const QueryResultError = ({ )} - {!hasHipaaAddon && ( - // [Joshen] TODO + {!hasHipaaAddon && !isCheckingHipaaEligibility && canDebug && ( ''} - onOpenAssistant={() => {}} - disabled={false} - loading={false} + buildPrompt={buildDebugPrompt} + onOpenAssistant={handleDebug} + disabled={isCreating} + loading={isCreating} /> )} diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx index 79fbc5a407f..52442928cfb 100644 --- a/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/QueryResultRenderer.tsx @@ -1,3 +1,4 @@ +import { type SqlSnippetSource } from '../../SQLEditor/querySource' import { type QueryResult } from '../types' import { QueryResultChart } from './QueryResultChart' import { QueryResultError } from './QueryResultError' @@ -8,9 +9,18 @@ interface QueryResultRendererProps { result?: QueryResult view?: 'table' | 'chart' chart?: ChartConfig + /** The query that produced `result`, used to build the "Debug with Assistant" prompt on error. */ + sql?: string + source?: SqlSnippetSource } -export const QueryResultRenderer = ({ result, view, chart }: QueryResultRendererProps) => { +export const QueryResultRenderer = ({ + result, + view, + chart, + sql, + source, +}: QueryResultRendererProps) => { const { rows, error, autoLimit } = result ?? {} if (!result) { @@ -18,7 +28,7 @@ export const QueryResultRenderer = ({ result, view, chart }: QueryResultRenderer } if (error) { - return + return } if ((rows ?? []).length === 0) { diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx index 11cfe9b4f2c..7c729cee4e3 100644 --- a/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryEditor/index.tsx @@ -227,13 +227,11 @@ export const QueryEditor = forwardRef(funct ) const { mutateAsync: executeSql, isPending: isExecutingSql } = useExecuteSqlMutation({ - onSuccess: (data) => onResultChange({ rows: data.result }), - onError: (error) => onResultChange({ error }), + onError: () => {}, }) const { mutateAsync: executeLogsSql, isPending: isExecutingLogs } = useExecuteLogsSqlMutation({ - onSuccess: (data) => onResultChange({ rows: data.rows as readonly Record[] }), - onError: (error) => onResultChange({ error }), + onError: () => {}, }) const isResolvingDatabase = @@ -259,6 +257,7 @@ export const QueryEditor = forwardRef(funct } onRun?.() + const querySnapshot = { sql: rawSql, source: query._tag } // [Joshen] This is deliberate to commit the sql, rather than the passed rawSql // As we want to save the cell's content into the store, rather than what's getting run onSqlCommit?.(sql) @@ -267,6 +266,7 @@ export const QueryEditor = forwardRef(funct if (!isOtelLogsEnabled) { onResultChange({ error: { message: "Querying logs isn't available for this project yet." }, + ...querySnapshot, }) return } @@ -276,7 +276,14 @@ export const QueryEditor = forwardRef(funct sql: acceptUntrustedLogsSql(untrustedLogSql(rawSql)), range: resolveLogTimeRange(query.time_range), endpoint: QUERY_SOURCE_REGISTRY.logs.endpoint, - }).catch(() => {}) + }).then( + (data) => + onResultChange({ + rows: data.rows as readonly Record[], + ...querySnapshot, + }), + (error) => onResultChange({ error, ...querySnapshot }) + ) return } @@ -284,7 +291,10 @@ export const QueryEditor = forwardRef(funct const limitedSql = applyAutoLimit(safeSql, rowLimit) if (!isValidConnString(connectionString)) { - onResultChange({ error: { message: 'Unable to run query: Connection string is missing' } }) + onResultChange({ + error: { message: 'Unable to run query: Connection string is missing' }, + ...querySnapshot, + }) return } @@ -296,7 +306,10 @@ export const QueryEditor = forwardRef(funct contextualInvalidation: true, isStatementTimeoutDisabled: true, isRoleImpersonationEnabled: isRoleImpersonationEnabled(roleImpersonationState?.role), - }).catch(() => {}) + }).then( + (data) => onResultChange({ rows: data.result, ...querySnapshot }), + (error) => onResultChange({ error, ...querySnapshot }) + ) } const handleConfirmPendingRun = () => { @@ -584,7 +597,13 @@ export const QueryEditor = forwardRef(funct : 'overflow-x-auto' )} > - + diff --git a/apps/studio/components/interfaces/Explorer/types.ts b/apps/studio/components/interfaces/Explorer/types.ts index 4109350cbae..b1f88985626 100644 --- a/apps/studio/components/interfaces/Explorer/types.ts +++ b/apps/studio/components/interfaces/Explorer/types.ts @@ -1,9 +1,13 @@ +import { type SqlSnippetSource } from '../SQLEditor/querySource' import { type ChartConfig } from '@/data/content/notebooks/notebook-schema' export type QueryResult = { rows?: readonly Record[] error?: { message: string; formattedError?: string } autoLimit?: number + /** The query that was submitted to produce this result, snapshotted at run time. */ + sql?: string + source?: SqlSnippetSource } /**