mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 17:35:10 +03:00
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Feature, plus a refactor of the shared logs-rewrite flow. PR 8 of the SQL editor query-source series. Stacked on #48457 — review that one first, and merge this after it. ## What is the current behavior? A `log_sql` snippet runs against the ClickHouse-backed analytics endpoint, but the SQL editor's AI still writes Postgres: inline edits get Postgres system prompts, and the result is run through `sql-formatter`, which mangles ClickHouse backticks and `log_attributes` map lookups. Legacy Logs Explorer saved queries open in the editor as `log_sql` snippets. Those are BigQuery dialect and error against the ClickHouse endpoint the editor runs them on, with no in-editor way out — only the Logs Explorer offered a rewrite. The completion route was also asymmetric. It assembled a schema/code/instruction message for Postgres but forwarded `prompt` verbatim for ClickHouse, so a client wanting ClickHouse had to hand-build the equivalent string. ## What is the new behavior? **Inline AI speaks ClickHouse for logs snippets.** `sqlSourceToDialect` maps a snippet's source to `postgres`/`clickhouse` and `buildCompletionRequestBody` threads it through. For ClickHouse, `useSqlEditorAi` strips code fences from the response and skips `formatSql`. Execution and dialect both follow the snippet type, so a snippet's valid dialect never flips. **Rewrite to ClickHouse in the editor.** A banner offers the rewrite for a logs snippet whose text trips `looksLikeLegacyLogsQuery`, and proposes the result through the editor's existing AI diff view rather than replacing the snippet, so it's accepted or discarded like any other AI edit. Gated on `otelLegacyLogs`: on a non-migrated org the BigQuery text is still correct, so rewriting it would break a working query. The offer is a state machine (`offered` / `rewriting` / `failed` / `noRewriteNeeded` / `dismissed`) with a declarative table of valid transitions, so the states are mutually exclusive by construction and dismissal is terminal. A failure keeps its message and offers a retry; a response identical to the input is reported rather than opening an empty diff. **One place assembles completion prompts.** The route now uses a single template for both dialects, branching only the schema section and — for `intent: 'rewrite'` — the instruction. `lib/ai/clickhouse-logs.ts` is the single home for ClickHouse-logs prompt content, replacing two independently maintained descriptions of the same table. Clients carry no prompt text. **The rewrite flow is shared with the Logs Explorer.** Both surfaces previously hand-rolled the same sequence and had drifted: only one detected a no-op rewrite, they sourced `log_attributes` keys differently, and the Explorer formatted errors with an `as Error` cast. Both now use `useLegacyLogsRewrite` and the same state-driven banner, so the Explorer picks up no-op detection and typed error extraction. **Attribute keys are fetched on submit, not while typing.** The detected source would otherwise feed a reactive query key, making every edit that changed it cost another network call. `useLogsAttributeKeys` is imperative and goes through `queryClient.fetchQuery`, so a source already cached — including by the Explorer header and query panel, which subscribe reactively — is reused. This also closes a gap where inline edits never received keys at all, unlike full rewrites. `getErrorMessage` gains an optional typed fallback and no longer stringifies a bare object into `'[object Object]'`; every existing caller already hand-rolled a fallback, except `QueueSettings`, which interpolated the raw result and now passes one. Nothing here is user-visible until the `sqlEditorLogsSource` flag is enabled. Tests: dialect selection and request-body shape, the ClickHouse prompt content (including that the schema section does not restate the dialect rules), the reducer's valid and invalid transitions, `shouldOfferLegacyLogsRewrite`, on-submit key discovery with cache reuse, and `getErrorMessage`. ## Additional context <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an Assistant banner to help rewrite legacy BigQuery-style logs queries into ClickHouse SQL. * SQL assistance now adapts to the selected query type, including relevant log attribute context. * Rewrite suggestions can be reviewed as editor diffs before being applied. * **Bug Fixes** * Improved rewrite failure handling, retry options, dismissal behavior, and “no rewrite needed” messaging. * Error notifications now provide a clearer fallback message when details are unavailable. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
164 lines
5.7 KiB
TypeScript
164 lines
5.7 KiB
TypeScript
import { act, waitFor } from '@testing-library/react'
|
|
import { delay, http, HttpResponse } from 'msw'
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import {
|
|
INITIAL_LEGACY_LOGS_REWRITE_STATE,
|
|
legacyLogsRewriteReducer as reduce,
|
|
useLegacyLogsRewrite,
|
|
type LegacyLogsRewriteEvent,
|
|
type LegacyLogsRewriteState,
|
|
} from './useLegacyLogsRewrite'
|
|
import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject'
|
|
import { API_URL } from '@/lib/constants'
|
|
import { mswServer } from '@/tests/lib/msw'
|
|
import { renderSqlEditorHook, setupSqlEditorMocks } from '@/tests/lib/sql-editor-test-utils'
|
|
|
|
const run = (
|
|
events: LegacyLogsRewriteEvent[],
|
|
from: LegacyLogsRewriteState = INITIAL_LEGACY_LOGS_REWRITE_STATE
|
|
) => events.reduce(reduce, from)
|
|
|
|
const FAILED: LegacyLogsRewriteEvent = { type: 'rewriteFailed', message: 'boom' }
|
|
|
|
describe('legacyLogsRewriteReducer', () => {
|
|
it('starts out offering the rewrite', () => {
|
|
expect(INITIAL_LEGACY_LOGS_REWRITE_STATE).toEqual({ status: 'offered' })
|
|
})
|
|
|
|
it('requesting a rewrite moves to rewriting, and a proposal returns to offered', () => {
|
|
expect(run([{ type: 'rewriteRequested' }])).toEqual({ status: 'rewriting' })
|
|
expect(run([{ type: 'rewriteRequested' }, { type: 'rewriteProposed' }])).toEqual({
|
|
status: 'offered',
|
|
})
|
|
})
|
|
|
|
it('a failure lands in failed and keeps its message for the UI', () => {
|
|
expect(run([{ type: 'rewriteRequested' }, FAILED])).toEqual({
|
|
status: 'failed',
|
|
message: 'boom',
|
|
})
|
|
})
|
|
|
|
it('a failure is recoverable — the same request retries it', () => {
|
|
expect(run([{ type: 'rewriteRequested' }, FAILED, { type: 'rewriteRequested' }])).toEqual({
|
|
status: 'rewriting',
|
|
})
|
|
})
|
|
|
|
it('an unchanged response waits for acknowledgement instead of retiring silently', () => {
|
|
const noop = run([{ type: 'rewriteRequested' }, { type: 'rewriteNoop' }])
|
|
expect(noop).toEqual({ status: 'noRewriteNeeded' })
|
|
expect(run([{ type: 'dismissed' }], noop)).toEqual({ status: 'dismissed' })
|
|
})
|
|
|
|
it('both outcomes can be dismissed, and neither can be retried into a new outcome', () => {
|
|
const failed = run([{ type: 'rewriteRequested' }, FAILED])
|
|
expect(run([{ type: 'dismissed' }], failed)).toEqual({ status: 'dismissed' })
|
|
// noRewriteNeeded only accepts dismissal — no silent retry.
|
|
const noop = run([{ type: 'rewriteRequested' }, { type: 'rewriteNoop' }])
|
|
expect(run([{ type: 'rewriteRequested' }], noop)).toEqual({ status: 'noRewriteNeeded' })
|
|
})
|
|
|
|
it('dismissal is terminal — nothing resurrects the offer', () => {
|
|
const dismissed = run([{ type: 'dismissed' }])
|
|
expect(dismissed).toEqual({ status: 'dismissed' })
|
|
expect(
|
|
run(
|
|
[
|
|
{ type: 'rewriteRequested' },
|
|
{ type: 'rewriteProposed' },
|
|
FAILED,
|
|
{ type: 'rewriteNoop' },
|
|
],
|
|
dismissed
|
|
)
|
|
).toEqual({ status: 'dismissed' })
|
|
})
|
|
|
|
it('cannot be dismissed mid-rewrite, so a settling request never resurrects it', () => {
|
|
expect(run([{ type: 'rewriteRequested' }, { type: 'dismissed' }])).toEqual({
|
|
status: 'rewriting',
|
|
})
|
|
})
|
|
|
|
it('ignores events that are invalid for the current state', () => {
|
|
// No rewrite in flight to settle.
|
|
expect(run([{ type: 'rewriteProposed' }])).toEqual({ status: 'offered' })
|
|
expect(run([{ type: 'rewriteNoop' }])).toEqual({ status: 'offered' })
|
|
expect(run([FAILED])).toEqual({ status: 'offered' })
|
|
// Already rewriting; a second request is a no-op rather than a restart.
|
|
expect(run([{ type: 'rewriteRequested' }, { type: 'rewriteRequested' }])).toEqual({
|
|
status: 'rewriting',
|
|
})
|
|
})
|
|
})
|
|
|
|
describe('useLegacyLogsRewrite — dismiss', () => {
|
|
// No detectable source, so key discovery is skipped and the only outbound
|
|
// request is the completion call we control below.
|
|
const SQL_WITHOUT_SOURCE = 'select 1 from logs limit 5'
|
|
|
|
/** Keeps a requested rewrite in flight for the duration of the test. */
|
|
function stallTheRewrite() {
|
|
mswServer.use(
|
|
http.post(`${API_URL}/ai/code/complete`, async () => {
|
|
await delay(10_000)
|
|
return HttpResponse.json('select 1 from logs')
|
|
})
|
|
)
|
|
}
|
|
|
|
/**
|
|
* Exposes the resolved project alongside the hook: `requestRewrite` no-ops
|
|
* without a project ref, so tests must wait for that query before asking.
|
|
*/
|
|
async function renderDismissHarness() {
|
|
const onDismissed = vi.fn()
|
|
const utils = renderSqlEditorHook(() => {
|
|
const { data: project } = useSelectedProjectQuery()
|
|
const rewrite = useLegacyLogsRewrite({
|
|
readSql: () => SQL_WITHOUT_SOURCE,
|
|
onProposal: vi.fn(),
|
|
onDismissed,
|
|
})
|
|
return { ...rewrite, projectRef: project?.ref }
|
|
})
|
|
await waitFor(() => expect(utils.result.current.projectRef).toBe('default'))
|
|
return { ...utils, onDismissed }
|
|
}
|
|
|
|
beforeEach(() => {
|
|
setupSqlEditorMocks()
|
|
})
|
|
|
|
it('dismisses from the offer and reports it', async () => {
|
|
const { result, onDismissed } = await renderDismissHarness()
|
|
|
|
await act(async () => {
|
|
result.current.dismiss()
|
|
})
|
|
|
|
expect(result.current.state.status).toBe('dismissed')
|
|
expect(onDismissed).toHaveBeenCalledTimes(1)
|
|
})
|
|
|
|
it('does not report a dismissal the machine rejects mid-rewrite', async () => {
|
|
stallTheRewrite()
|
|
const { result, onDismissed } = await renderDismissHarness()
|
|
|
|
act(() => {
|
|
void result.current.requestRewrite()
|
|
})
|
|
await waitFor(() => expect(result.current.state.status).toBe('rewriting'))
|
|
|
|
await act(async () => {
|
|
result.current.dismiss()
|
|
})
|
|
|
|
// Persisting this would suppress an offer that's still live.
|
|
expect(onDismissed).not.toHaveBeenCalled()
|
|
expect(result.current.state.status).toBe('rewriting')
|
|
})
|
|
})
|