From 4dee5897351544942371bd071a2673a74032a4da Mon Sep 17 00:00:00 2001 From: Charis <26616127+charislam@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:08:28 -0400 Subject: [PATCH] fix(studio): stop assistant fabricating database_identifier for notebooks (#49558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary * Fixes [FE-4275](https://linear.app/supabase/issue/FE-4275/assistant-always-creates-notebooks-with-wrong-identifier-first-try): the assistant always created database notebook cells with a fabricated `database_identifier` (`"primary"`, later observed as `""` / `"_primary"` under different prompt wording) instead of omitting the key for the project's primary database, which tripped the tool's reject-and-retry validation on the very first attempt. * Prompt wording alone wasn't reliable — live eval runs against the real model kept substituting a new placeholder every time the prompt was tightened further. * Normalizes an empty-string `database_identifier` to absent at the schema level (`databaseIdentifierSchema` in `notebook-schema.ts`), which is inherited by every schema built from it — the AI SDK's `inputSchema` for `create_notebook`/`update_notebook`, and the write-boundary `writableNotebookSchema` used right before the PUT to the backend. * Adds an eval case (`evals/dataset.ts`) reproducing the original bug, plus unit tests covering schema-level and write-boundary normalization. ## Test plan - [X] `pnpm --filter studio exec tsc --noEmit` passes - [X] `pnpm exec prettier --check` passes on touched files - [X] Unit tests pass: `notebook-schema.test.ts`, `notebook-upsert-mutation.test.ts`, `notebook-tools.test.ts` (104 tests) - [X] Ran the new eval case against the real model 3x before the code fix (0% correctness, fabricated `""`/`"_primary"`) and 3x after (100% correctness) ## Summary by CodeRabbit * **Bug Fixes** * Improved notebook handling of empty database identifiers by treating them as absent. * Ensured notebook requests omit unused database identifier fields. * Added validation guidance for read-replica database identifiers. --- .../content/notebooks/notebook-schema.test.ts | 21 +++++++++++++ .../data/content/notebooks/notebook-schema.ts | 14 ++++++--- .../notebook-upsert-mutation.test.ts | 27 ++++++++++++++++ .../notebooks/notebook-upsert-mutation.ts | 4 +-- apps/studio/evals/dataset.ts | 31 +++++++++++++++++++ apps/studio/lib/ai/prompts.ts | 2 +- 6 files changed, 90 insertions(+), 9 deletions(-) diff --git a/apps/studio/data/content/notebooks/notebook-schema.test.ts b/apps/studio/data/content/notebooks/notebook-schema.test.ts index a459382cbe0..edbcc4bb07e 100644 --- a/apps/studio/data/content/notebooks/notebook-schema.test.ts +++ b/apps/studio/data/content/notebooks/notebook-schema.test.ts @@ -136,6 +136,27 @@ describe('notebookSchema', () => { expect(result.success).toBe(true) }) + it('normalizes an empty-string database_identifier to absent — a model asked to omit it often writes "" instead', () => { + const result = notebookSchema.safeParse({ + schema_version: 1, + cells: [ + { + _tag: 'database_cell', + _id: '1', + sql: 'select 1', + row_limit: 100, + database_identifier: '', + }, + ], + }) + + expect(result.success).toBe(true) + if (!result.success) return + const [cell] = result.data.cells + expect(cell._tag).toBe('database_cell') + expect(cell._tag === 'database_cell' ? cell.database_identifier : undefined).toBeUndefined() + }) + it('rejects a cell with no _id — the backend always assigns one on save', () => { const result = notebookSchema.safeParse({ schema_version: 1, diff --git a/apps/studio/data/content/notebooks/notebook-schema.ts b/apps/studio/data/content/notebooks/notebook-schema.ts index 7b746785656..df9e5449320 100644 --- a/apps/studio/data/content/notebooks/notebook-schema.ts +++ b/apps/studio/data/content/notebooks/notebook-schema.ts @@ -54,12 +54,16 @@ export const timeRangeSchema = z { message: 'must be later than the start of the range', path: ['end'] } ) +// The read-replica `identifier`, or absent for the project's primary. An empty string is +// normalized to absent — models asked to omit this key reliably substitute "" instead of +// leaving it out. +export const databaseIdentifierSchema = z + .string() + .optional() + .transform((value) => (value === '' ? undefined : value)) + export const databaseSourceSchema = z.object({ - /** - * Which database the query runs against: the read-replica `identifier`, or absent - * for the project's primary. - */ - database_identifier: z.string().optional(), + database_identifier: databaseIdentifierSchema, }) export const logsSourceSchema = z.object({ diff --git a/apps/studio/data/content/notebooks/notebook-upsert-mutation.test.ts b/apps/studio/data/content/notebooks/notebook-upsert-mutation.test.ts index df4e8ce5e12..e4e91726e70 100644 --- a/apps/studio/data/content/notebooks/notebook-upsert-mutation.test.ts +++ b/apps/studio/data/content/notebooks/notebook-upsert-mutation.test.ts @@ -98,6 +98,33 @@ describe('createNotebook', () => { createNotebook({ projectRef: 'default', name: 'Bad notebook', content: INVALID_CONTENT }) ).rejects.toThrow() }) + + it('strips an empty-string database_identifier before sending — a caller that skipped the agent schema is not trusted to have already normalized it', async () => { + let sentBody: Record | undefined + addAPIMock({ + method: 'put', + path: '/platform/projects/:ref/content', + response: async ({ request }) => { + sentBody = (await request.json()) as Record + return HttpResponse.json(null) + }, + }) + + await createNotebook({ + projectRef: 'default', + name: 'Signup funnel', + content: { + schema_version: 1, + cells: [ + { _tag: 'database_cell', sql: DATABASE_SQL, row_limit: 100, database_identifier: '' }, + ], + }, + }) + + const content = sentBody?.content as WritableNotebook + const [databaseCell] = content.cells as Array> + expect(databaseCell).not.toHaveProperty('database_identifier') + }) }) describe('upsertNotebook', () => { diff --git a/apps/studio/data/content/notebooks/notebook-upsert-mutation.ts b/apps/studio/data/content/notebooks/notebook-upsert-mutation.ts index e6332ac260a..c3609d7c970 100644 --- a/apps/studio/data/content/notebooks/notebook-upsert-mutation.ts +++ b/apps/studio/data/content/notebooks/notebook-upsert-mutation.ts @@ -17,15 +17,13 @@ function buildNotebookUpsertPayload({ description?: string content: WritableNotebook }): UpsertContentPayload { - writableNotebookSchema.parse(content) - return { id, name, description, type: 'notebook', visibility: 'project', - content, + content: writableNotebookSchema.parse(content), } } diff --git a/apps/studio/evals/dataset.ts b/apps/studio/evals/dataset.ts index 69ad2045d04..245bb23e34c 100644 --- a/apps/studio/evals/dataset.ts +++ b/apps/studio/evals/dataset.ts @@ -645,6 +645,37 @@ export const dataset: AssistantEvalCase[] = [ 'Guards against targeting a non-primary database when the user never asked for a specific one — the cell must omit database_identifier or target the primary, never a replica', }, }, + { + input: { + prompt: + "Create a notebook called 'Primary customer signups' with a query that shows the 20 most recently created customers on my primary database.", + mockTables: { + public: [ + { + name: 'customers', + rls_enabled: true, + columns: [ + { name: 'id', data_type: 'uuid' }, + { name: 'created_at', data_type: 'timestamp with time zone' }, + ], + }, + ], + }, + }, + expected: { + requiredTools: [ + { name: 'create_notebook', input: { name: { equals: 'Primary customer signups' } } }, + ], + forbiddenTools: ['list_databases'], + correctAnswer: + 'Creates the notebook with a database cell selecting the 20 most recent customers and omits the database_identifier key entirely — not present in the cell at all. The primary database does not need a lookup, and database_identifier must never be set to the literal "primary", an empty string, or another guessed value; setting it to "" is a contradiction of "omit the field", not equivalent to omitting it.', + }, + metadata: { + category: ['general_help'], + description: + 'Guards against fabricating the literal primary identifier or making an unnecessary list_databases round trip for a primary-database notebook', + }, + }, { input: { prompt: diff --git a/apps/studio/lib/ai/prompts.ts b/apps/studio/lib/ai/prompts.ts index f659ccd3ea5..c92ea1efe36 100644 --- a/apps/studio/lib/ai/prompts.ts +++ b/apps/studio/lib/ai/prompts.ts @@ -817,7 +817,7 @@ export const NOTEBOOKS_PROMPT = ` - When describing an existing notebook, report each query cell's configuration that changes what it returns — a log cell's time range, a database cell's row limit — and don't count markdown cells as queries. - Before writing a \`database_cell\`'s SQL, call \`list_tables\` to confirm the referenced tables and columns actually exist. Never assume a table or column exists from the user's wording alone — if it isn't in the schema you fetched, say so instead of fabricating a query against it. - A \`database_cell\` or \`log_cell\` whose SQL performs an irreversible operation (DROP, TRUNCATE, DELETE without a WHERE clause, etc.) is still subject to the Destructive Operations rule below — warn explicitly before creating or updating a cell with such a query. Saving it for repeated future use does not make it safer. -- Before setting a \`database_cell\`'s \`database_identifier\` (e.g. to target a read replica the user names or describes), call \`list_databases\` and use one of the identifiers it returns. Never invent one — an unrecognized identifier is rejected. Omit the field entirely to target the project's primary database. +- There is no identifier for the primary database — not \`primary\`, not \`_primary\`, not an empty string, not the project ref, not any other placeholder spelling of "primary". \`database_identifier\` exists solely to name an explicitly requested read replica; the primary database is what you get by leaving the key out of the cell's JSON entirely, so when the user says "primary" or names no database, omit the key. Writing any string into this field — even one that merely gestures at "primary" — is rejected at save time and forces a retry, so get it right the first time. Before setting it for a replica, call \`list_databases\` and use one of the identifiers it returns; never invent one, because an unrecognized identifier is rejected the same way. - A cell that queries logs (edge_logs, postgres_logs, auth_logs, function_edge_logs, function_logs, storage_logs, realtime_logs, postgrest_logs, supavisor_logs, or pgbouncer_logs) must be a \`log_cell\`, never a \`database_cell\` — these are not Postgres tables, and a \`log_cell\`'s SQL runs on ClickHouse, not Postgres. ${CLICKHOUSE_LOGS_COMPLETION_INSTRUCTIONS} ${buildClickhouseLogsSchemaSection()}