From 233cbdc8e5f29cd44aa6e5d4a45930ca0e0c7bc2 Mon Sep 17 00:00:00 2001 From: Charis <26616127+charislam@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:09:44 -0400 Subject: [PATCH] Restore notebook diff preview for completed updates (#49402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This is **PR 3 of 3** in the FE-4243 stack fixing "Notebook update proposal shows 'unapplyable' error for already-completed updates." - Consumes the `previous_content` field added by PR 2 (#49401) to reconstruct diffs for already-applied notebook updates - Restores the diff preview that PR 1 initially dropped — completed updates now show the full before/after instead of a generic "Notebook updated" message - Uses the same diff derivation function called pre-approval, guaranteeing the rendered diff matches what was shown during confirmation - Includes defensive fallback handling for older persisted chats (before `previous_content` existed) and edge cases **Depends on**: PR 2 (#49401) merging first — this PR consumes the `previous_content` field from that server change. Resolves FE-4243 ## Test plan - ✅ 19/19 tests pass in NotebookProposalRenderer.test.tsx (2 confirmed as real regressions) - ✅ 118/118 tests pass in full AIAssistantPanel suite - ✅ Typecheck: clean on modified files - ✅ ESLint: zero errors/warnings on changed files - ✅ Lint ratchet: passes (some rules improved) - ✅ New regression tests cover: delete_cell, insert_cell, missing previous_content, and operations that no longer reconcile - ✅ No notebook fetch in completed update tests (proves no redundant re-fetching) ## Summary by CodeRabbit * **New Features** * Added visual previews showing notebook changes, including inserted and deleted cells, when prior content is available. * Prevented duplicate cells from appearing in update previews. * Retained a compact completion message when change details are unavailable or inconsistent. * Ensured previews are shown only for the relevant notebook. * **Tests** * Added coverage for notebook update previews, deletion and insertion diffs, duplicate prevention, notebook matching, and fallback behavior. --- .../ui/AIAssistantPanel/Message.utils.ts | 6 +- .../NotebookProposalRenderer.test.tsx | 135 ++++++++++++++++++ .../NotebookProposalRenderer.tsx | 26 +++- 3 files changed, 165 insertions(+), 2 deletions(-) diff --git a/apps/studio/components/ui/AIAssistantPanel/Message.utils.ts b/apps/studio/components/ui/AIAssistantPanel/Message.utils.ts index 9867cc27511..1f901c2ccfe 100644 --- a/apps/studio/components/ui/AIAssistantPanel/Message.utils.ts +++ b/apps/studio/components/ui/AIAssistantPanel/Message.utils.ts @@ -2,7 +2,7 @@ import { untrustedSql } from '@supabase/pg-meta' import { z, type SafeParseReturnType } from 'zod' import { notebookOperationsSchema } from '@/data/content/notebooks/notebook-operations' -import { agentNotebookSchema } from '@/data/content/notebooks/notebook-schema' +import { agentNotebookSchema, notebookSchema } from '@/data/content/notebooks/notebook-schema' // Splits markdown into alternating [plain, code, plain, code, ...] segments. // Odd-indexed segments are already inside code spans/fences and should be left alone. @@ -143,6 +143,10 @@ export const updateNotebookInputSchema = z.object({ export const notebookToolOutputSchema = z.object({ id: z.string(), name: z.string() }) +export const updateNotebookToolOutputSchema = notebookToolOutputSchema.extend({ + previous_content: notebookSchema.optional(), +}) + export const rateMessageResponseSchema = z.object({ category: z.enum([ 'sql_generation', diff --git a/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.test.tsx b/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.test.tsx index f48bef2b8c7..f445dec37d2 100644 --- a/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.test.tsx +++ b/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.test.tsx @@ -339,6 +339,141 @@ describe('NotebookProposalRenderer', () => { expect(screen.queryByText("This update can't be applied as written")).not.toBeInTheDocument() }) + it('renders the snapshot-derived diff for a completed delete_cell update, without fetching the notebook', () => { + render( + + ) + + expect(screen.getByText('−1')).toBeInTheDocument() + expect(screen.getByRole('link', { name: 'Open notebook' })).toHaveAttribute( + 'href', + `/project/default/explorer/notebook/${NOTEBOOK_ID}` + ) + }) + + it('renders a single added cell for a completed insert_cell update, not a phantom duplicate', () => { + render( + + ) + + expect(screen.getByText('+1')).toBeInTheDocument() + expect(screen.getAllByRole('button', { name: 'Added Markdown cell' })).toHaveLength(1) + }) + + it('falls back to the compact completed body when previous_content is absent', () => { + render( + + ) + + expect(screen.getByText('Notebook updated: Signup funnel')).toBeInTheDocument() + expect(screen.queryByText("This update can't be applied as written")).not.toBeInTheDocument() + expect(screen.getByRole('link', { name: 'Open notebook' })).toBeInTheDocument() + }) + + it('falls back to the compact completed body when the snapshot no longer matches the operations', () => { + render( + + ) + + expect(screen.getByText('Notebook updated: Signup funnel')).toBeInTheDocument() + expect(screen.queryByText("This update can't be applied as written")).not.toBeInTheDocument() + expect(screen.queryByText('Preview unavailable')).not.toBeInTheDocument() + }) + + it('falls back to the compact completed body when the output id does not match the requested notebook', () => { + render( + + ) + + expect(screen.getByText('Notebook updated: Signup funnel')).toBeInTheDocument() + expect(screen.queryByText('−1')).not.toBeInTheDocument() + }) + it('derives the diff against live content for a denied update', async () => { mockContentItem(mockNotebookRow()) diff --git a/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.tsx b/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.tsx index d2634c6a948..af710fbf15b 100644 --- a/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.tsx +++ b/apps/studio/components/ui/AIAssistantPanel/NotebookProposalRenderer.tsx @@ -13,6 +13,7 @@ import { createNotebookInputSchema, notebookToolOutputSchema, updateNotebookInputSchema, + updateNotebookToolOutputSchema, } from './Message.utils' import { AlertError } from '@/components/ui/AlertError' import { @@ -340,8 +341,31 @@ function UpdateNotebookProposal({ } if (isCompleted) { - const parsedOutput = notebookToolOutputSchema.safeParse(output) + const parsedOutput = updateNotebookToolOutputSchema.safeParse(output) const notebookName = parsedOutput.success ? parsedOutput.data.name : undefined + const isOutputForRequestedNotebook = + parsedOutput.success && parsedOutput.data.id === parsedInput.data.id + const previousContent = isOutputForRequestedNotebook + ? parsedOutput.data.previous_content + : undefined + const diff = previousContent + ? deriveNotebookDiff(previousContent, parsedInput.data.operations) + : undefined + + if (diff?.success) { + return ( + + + + ) + } return (