mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
Restore notebook diff preview for completed updates (#49402)
## 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) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
8920439569
commit
233cbdc8e5
3 files changed
+165
-2
No files matched your search
@@ -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',
|
||||
|
||||
@@ -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(
|
||||
<NotebookProposalRenderer
|
||||
mode="update"
|
||||
state="output-available"
|
||||
input={{
|
||||
id: NOTEBOOK_ID,
|
||||
expected_updated_at: '2024-01-01T00:00:00.000Z',
|
||||
operations: [{ _tag: 'delete_cell', cell_id: 'cell-1' }],
|
||||
}}
|
||||
output={{
|
||||
id: NOTEBOOK_ID,
|
||||
name: 'Signup funnel',
|
||||
previous_content: {
|
||||
schema_version: 1,
|
||||
cells: [
|
||||
{ _tag: 'markdown_cell', _id: 'cell-1', text: 'hello' },
|
||||
{ _tag: 'markdown_cell', _id: 'cell-2', text: 'world' },
|
||||
],
|
||||
},
|
||||
}}
|
||||
/>
|
||||
)
|
||||
|
||||
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(
|
||||
<NotebookProposalRenderer
|
||||
mode="update"
|
||||
state="output-available"
|
||||
input={{
|
||||
id: NOTEBOOK_ID,
|
||||
expected_updated_at: '2024-01-01T00:00:00.000Z',
|
||||
operations: [
|
||||
{
|
||||
_tag: 'insert_cell',
|
||||
after_cell_id: 'cell-1',
|
||||
cell: { _tag: 'markdown_cell', text: 'new section' },
|
||||
},
|
||||
],
|
||||
}}
|
||||
output={{
|
||||
id: NOTEBOOK_ID,
|
||||
name: 'Signup funnel',
|
||||
previous_content: {
|
||||
schema_version: 1,
|
||||
cells: [{ _tag: 'markdown_cell', _id: 'cell-1', text: 'hello' }],
|
||||
},
|
||||
}}
|
||||
/>
|
||||
)
|
||||
|
||||
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(
|
||||
<NotebookProposalRenderer
|
||||
mode="update"
|
||||
state="output-available"
|
||||
input={{
|
||||
id: NOTEBOOK_ID,
|
||||
expected_updated_at: '2024-01-01T00:00:00.000Z',
|
||||
operations: [{ _tag: 'delete_cell', cell_id: 'cell-1' }],
|
||||
}}
|
||||
output={{ id: NOTEBOOK_ID, name: 'Signup funnel' }}
|
||||
/>
|
||||
)
|
||||
|
||||
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(
|
||||
<NotebookProposalRenderer
|
||||
mode="update"
|
||||
state="output-available"
|
||||
input={{
|
||||
id: NOTEBOOK_ID,
|
||||
expected_updated_at: '2024-01-01T00:00:00.000Z',
|
||||
operations: [{ _tag: 'delete_cell', cell_id: 'missing' }],
|
||||
}}
|
||||
output={{
|
||||
id: NOTEBOOK_ID,
|
||||
name: 'Signup funnel',
|
||||
previous_content: {
|
||||
schema_version: 1,
|
||||
cells: [{ _tag: 'markdown_cell', _id: 'cell-1', text: 'hello' }],
|
||||
},
|
||||
}}
|
||||
/>
|
||||
)
|
||||
|
||||
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(
|
||||
<NotebookProposalRenderer
|
||||
mode="update"
|
||||
state="output-available"
|
||||
input={{
|
||||
id: NOTEBOOK_ID,
|
||||
expected_updated_at: '2024-01-01T00:00:00.000Z',
|
||||
operations: [{ _tag: 'delete_cell', cell_id: 'cell-1' }],
|
||||
}}
|
||||
output={{
|
||||
id: 'some-other-notebook-id',
|
||||
name: 'Signup funnel',
|
||||
previous_content: {
|
||||
schema_version: 1,
|
||||
cells: [
|
||||
{ _tag: 'markdown_cell', _id: 'cell-1', text: 'hello' },
|
||||
{ _tag: 'markdown_cell', _id: 'cell-2', text: 'world' },
|
||||
],
|
||||
},
|
||||
}}
|
||||
/>
|
||||
)
|
||||
|
||||
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())
|
||||
|
||||
|
||||
@@ -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 (
|
||||
<NotebookConfirm
|
||||
mode="update"
|
||||
confirmState={confirmState}
|
||||
footerAction={footerAction}
|
||||
message={`Assistant wants to update "${notebookName}"`}
|
||||
onApprove={onApprove}
|
||||
onDeny={onDeny}
|
||||
>
|
||||
<AssistantNotebookPreview entries={diff.entries} mode="update" title={notebookName} />
|
||||
</NotebookConfirm>
|
||||
)
|
||||
}
|
||||
|
||||
return (
|
||||
<NotebookConfirm
|
||||
|
||||
Reference in new issue
Block a user