fix: address review feedback for SQL notebooks

- Wrap JSON.parse in try/catch for corrupt pinned_result data
- Fix IPC type mismatches: update/duplicate/updateCell return actual data
- Remove non-functional Run All button from notebook editor
- Replace confirm() dialog with immediate delete + toast notification
- Remove dead notebook placeholder branch in tab-query-editor
- Remove duplicate react-markdown/remark-gfm from root package.json

apps/desktop/src/main/notebook-storage.ts

@@ -46,13 +46,22 @@ function rowToNotebook(row: NotebookRow): Notebook {
   }
 }
 
+function parsePinnedResult(raw: string): PinnedResult | null {
+  try {
+    return JSON.parse(raw) as PinnedResult
+  } catch {
+    log.warn('Corrupt pinned_result JSON, falling back to null')
+    return null
+  }
+}
+
 function rowToCell(row: CellRow): NotebookCell {
   return {
     id: row.id,
     notebookId: row.notebook_id,
     type: row.type,
     content: row.content,
-    pinnedResult: row.pinned_result ? (JSON.parse(row.pinned_result) as PinnedResult) : null,
+    pinnedResult: row.pinned_result ? parsePinnedResult(row.pinned_result) : null,
     order: row.order_index,
     createdAt: row.created_at,
     updatedAt: row.updated_at
@@ -111,16 +120,14 @@ export class NotebookStorage {
   }
 
   getNotebook(id: string): NotebookWithCells | null {
-    const notebookRow = this.db
-      .prepare('SELECT * FROM notebooks WHERE id = ?')
-      .get(id) as NotebookRow | undefined
+    const notebookRow = this.db.prepare('SELECT * FROM notebooks WHERE id = ?').get(id) as
+      | NotebookRow
+      | undefined
 
     if (!notebookRow) return null
 
     const cellRows = this.db
-      .prepare(
-        'SELECT * FROM notebook_cells WHERE notebook_id = ? ORDER BY order_index ASC'
-      )
+      .prepare('SELECT * FROM notebook_cells WHERE notebook_id = ? ORDER BY order_index ASC')
       .all(id) as CellRow[]
 
     return {
@@ -144,9 +151,9 @@ export class NotebookStorage {
   }
 
   updateNotebook(id: string, input: UpdateNotebookInput): Notebook | null {
-    const existing = this.db
-      .prepare('SELECT * FROM notebooks WHERE id = ?')
-      .get(id) as NotebookRow | undefined
+    const existing = this.db.prepare('SELECT * FROM notebooks WHERE id = ?').get(id) as
+      | NotebookRow
+      | undefined
     if (!existing) return null
 
     const now = Date.now()
@@ -154,9 +161,7 @@ export class NotebookStorage {
     const folder = input.folder !== undefined ? input.folder : existing.folder
 
     this.db
-      .prepare(
-        'UPDATE notebooks SET title = ?, folder = ?, updated_at = ? WHERE id = ?'
-      )
+      .prepare('UPDATE notebooks SET title = ?, folder = ?, updated_at = ? WHERE id = ?')
       .run(title, folder, now, id)
 
     return rowToNotebook(
@@ -227,9 +232,9 @@ export class NotebookStorage {
   }
 
   updateCell(id: string, input: UpdateCellInput): NotebookCell | null {
-    const existing = this.db
-      .prepare('SELECT * FROM notebook_cells WHERE id = ?')
-      .get(id) as CellRow | undefined
+    const existing = this.db.prepare('SELECT * FROM notebook_cells WHERE id = ?').get(id) as
+      | CellRow
+      | undefined
     if (!existing) return null
 
     const now = Date.now()
@@ -254,9 +259,9 @@ export class NotebookStorage {
   }
 
   deleteCell(id: string): void {
-    const cell = this.db
-      .prepare('SELECT * FROM notebook_cells WHERE id = ?')
-      .get(id) as CellRow | undefined
+    const cell = this.db.prepare('SELECT * FROM notebook_cells WHERE id = ?').get(id) as
+      | CellRow
+      | undefined
     if (!cell) return
     this.db.prepare('DELETE FROM notebook_cells WHERE id = ?').run(id)
     this.touchNotebook(cell.notebook_id)

apps/desktop/src/preload/index.d.ts

@@ -430,9 +430,7 @@ interface DataPeekApi {
     onEvent: (callback: (event: PgNotificationEvent) => void) => () => void
     onStatus: (callback: (status: PgNotificationConnectionStatus) => void) => () => void
     reconnect: (connectionId: string) => Promise<IpcResponse<void>>
-    getStatus: (
-      connectionId: string
-    ) => Promise<IpcResponse<PgNotificationConnectionStatus | null>>
+    getStatus: (connectionId: string) => Promise<IpcResponse<PgNotificationConnectionStatus | null>>
     getAllStatuses: () => Promise<IpcResponse<PgNotificationConnectionStatus[]>>
   }
   health: {
@@ -466,11 +464,11 @@ interface DataPeekApi {
     list: () => Promise<IpcResponse<Notebook[]>>
     get: (id: string) => Promise<IpcResponse<NotebookWithCells>>
     create: (input: CreateNotebookInput) => Promise<IpcResponse<Notebook>>
-    update: (id: string, updates: UpdateNotebookInput) => Promise<IpcResponse<void>>
+    update: (id: string, updates: UpdateNotebookInput) => Promise<IpcResponse<Notebook>>
     delete: (id: string) => Promise<IpcResponse<void>>
-    duplicate: (id: string, connectionId: string) => Promise<IpcResponse<NotebookWithCells>>
+    duplicate: (id: string, connectionId: string) => Promise<IpcResponse<Notebook>>
     addCell: (notebookId: string, input: AddCellInput) => Promise<IpcResponse<NotebookCell>>
-    updateCell: (cellId: string, updates: UpdateCellInput) => Promise<IpcResponse<void>>
+    updateCell: (cellId: string, updates: UpdateCellInput) => Promise<IpcResponse<NotebookCell>>
     deleteCell: (cellId: string) => Promise<IpcResponse<void>>
     reorderCells: (notebookId: string, cellIds: string[]) => Promise<IpcResponse<void>>
   }

apps/desktop/src/preload/index.ts

@@ -319,21 +319,19 @@ const api = {
   },
   // Notebooks management
   notebooks: {
-    list: (): Promise<IpcResponse<Notebook[]>> =>
-      ipcRenderer.invoke('notebooks:list'),
+    list: (): Promise<IpcResponse<Notebook[]>> => ipcRenderer.invoke('notebooks:list'),
     get: (id: string): Promise<IpcResponse<NotebookWithCells>> =>
       ipcRenderer.invoke('notebooks:get', id),
     create: (input: CreateNotebookInput): Promise<IpcResponse<Notebook>> =>
       ipcRenderer.invoke('notebooks:create', input),
-    update: (id: string, updates: UpdateNotebookInput): Promise<IpcResponse<void>> =>
+    update: (id: string, updates: UpdateNotebookInput): Promise<IpcResponse<Notebook>> =>
       ipcRenderer.invoke('notebooks:update', { id, updates }),
-    delete: (id: string): Promise<IpcResponse<void>> =>
-      ipcRenderer.invoke('notebooks:delete', id),
-    duplicate: (id: string, connectionId: string): Promise<IpcResponse<NotebookWithCells>> =>
+    delete: (id: string): Promise<IpcResponse<void>> => ipcRenderer.invoke('notebooks:delete', id),
+    duplicate: (id: string, connectionId: string): Promise<IpcResponse<Notebook>> =>
       ipcRenderer.invoke('notebooks:duplicate', { id, connectionId }),
     addCell: (notebookId: string, input: AddCellInput): Promise<IpcResponse<NotebookCell>> =>
       ipcRenderer.invoke('notebooks:add-cell', { notebookId, input }),
-    updateCell: (cellId: string, updates: UpdateCellInput): Promise<IpcResponse<void>> =>
+    updateCell: (cellId: string, updates: UpdateCellInput): Promise<IpcResponse<NotebookCell>> =>
       ipcRenderer.invoke('notebooks:update-cell', { cellId, updates }),
     deleteCell: (cellId: string): Promise<IpcResponse<void>> =>
       ipcRenderer.invoke('notebooks:delete-cell', cellId),
@@ -572,11 +570,8 @@ const api = {
       ipcRenderer.on('pg-notify:event', handler)
       return () => ipcRenderer.removeListener('pg-notify:event', handler)
     },
-    onStatus: (
-      callback: (status: PgNotificationConnectionStatus) => void
-    ): (() => void) => {
-      const handler = (_: unknown, status: PgNotificationConnectionStatus): void =>
-        callback(status)
+    onStatus: (callback: (status: PgNotificationConnectionStatus) => void): (() => void) => {
+      const handler = (_: unknown, status: PgNotificationConnectionStatus): void => callback(status)
       ipcRenderer.on('pg-notify:status', handler)
       return () => ipcRenderer.removeListener('pg-notify:status', handler)
     },

apps/desktop/src/renderer/src/components/notebook-editor.tsx

@@ -1,5 +1,5 @@
 import { useState, useCallback, useEffect, useRef } from 'react'
-import { Plus, Play } from 'lucide-react'
+import { Plus } from 'lucide-react'
 import { Button, cn } from '@data-peek/ui'
 import { useNotebookStore } from '@/stores/notebook-store'
 import { useConnectionStore } from '@/stores/connection-store'
@@ -58,9 +58,7 @@ export function NotebookEditor({ tab }: NotebookEditorProps) {
           ? cells[insertAfterIndex].order + 0.5
           : cells.length
       addCell(activeNotebook.id, { type, content: '', order })
-      setFocusedCellIndex(
-        insertAfterIndex !== undefined ? insertAfterIndex + 1 : cells.length
-      )
+      setFocusedCellIndex(insertAfterIndex !== undefined ? insertAfterIndex + 1 : cells.length)
     },
     [activeNotebook, cells, addCell]
   )
@@ -202,24 +200,12 @@ export function NotebookEditor({ tab }: NotebookEditorProps) {
           <Plus className="size-3" />
           Note
         </Button>
-
-        <Button
-          variant="ghost"
-          size="sm"
-          className="h-7 gap-1.5 text-xs"
-          onClick={() => setFocusedCellIndex(0)}
-        >
-          <Play className="size-3" />
-          Run All
-        </Button>
       </div>
 
       <div className="flex-1 overflow-y-auto px-4 py-4 pb-16">
         {cells.length === 0 ? (
           <div className="flex flex-col items-center justify-center py-16 gap-3">
-            <p className="text-sm text-muted-foreground">
-              Empty notebook. Add your first cell.
-            </p>
+            <p className="text-sm text-muted-foreground">Empty notebook. Add your first cell.</p>
             <div className="flex gap-2">
               <Button
                 variant="outline"
@@ -251,9 +237,7 @@ export function NotebookEditor({ tab }: NotebookEditorProps) {
                   connectionId={connectionId ?? ''}
                   isFocused={focusedCellIndex === index}
                   onFocus={() => setFocusedCellIndex(index)}
-                  onRunAndAdvance={() =>
-                    setFocusedCellIndex(Math.min(index + 1, cells.length - 1))
-                  }
+                  onRunAndAdvance={() => setFocusedCellIndex(Math.min(index + 1, cells.length - 1))}
                   onDelete={() => handleDeleteCell(cell.id, index)}
                 />
                 <InsertPoint onInsert={(type) => handleAddCell(type, index)} />

apps/desktop/src/renderer/src/components/notebook-sidebar.tsx

@@ -22,7 +22,7 @@ import {
 } from '@data-peek/ui'
 
 import { useNotebookStore } from '@/stores/notebook-store'
-import { useConnectionStore, useTabStore } from '@/stores'
+import { useConnectionStore, useTabStore, notify } from '@/stores'
 import type { Notebook } from '@shared/index'
 
 export function NotebookSidebar() {
@@ -47,7 +47,10 @@ export function NotebookSidebar() {
 
   const handleCreate = async () => {
     if (!activeConnectionId) return
-    const nb = await createNotebook({ title: 'Untitled Notebook', connectionId: activeConnectionId })
+    const nb = await createNotebook({
+      title: 'Untitled Notebook',
+      connectionId: activeConnectionId
+    })
     if (nb) {
       createNotebookTab(nb.connectionId, nb.id, nb.title)
     }
@@ -57,10 +60,9 @@ export function NotebookSidebar() {
     createNotebookTab(nb.connectionId, nb.id, nb.title)
   }
 
-  const handleDelete = async (id: string) => {
-    if (confirm('Are you sure you want to delete this notebook?')) {
-      await deleteNotebook(id)
-    }
+  const handleDelete = async (nb: Notebook) => {
+    await deleteNotebook(nb.id)
+    notify.success('Notebook deleted', `"${nb.title}" was removed.`)
   }
 
   const toggleFolder = (folder: string) => {
@@ -109,7 +111,7 @@ export function NotebookSidebar() {
             <span>Open notebook</span>
           </DropdownMenuItem>
           <DropdownMenuSeparator />
-          <DropdownMenuItem className="text-red-400" onClick={() => handleDelete(nb.id)}>
+          <DropdownMenuItem className="text-red-400" onClick={() => handleDelete(nb)}>
             <Trash2 className="text-red-400" />
             <span>Delete</span>
           </DropdownMenuItem>

apps/desktop/src/renderer/src/components/tab-query-editor.tsx

@@ -1156,15 +1156,6 @@ export function TabQueryEditor({ tabId }: TabQueryEditorProps) {
     return <HealthMonitor tabId={tabId} />
   }
 
-  // Notebook tabs are rendered by their own component (placeholder until NotebookEditor exists)
-  if (tab.type === 'notebook') {
-    return (
-      <div className="flex flex-1 items-center justify-center text-muted-foreground">
-        <span className="text-sm">Notebook: {tab.notebookId}</span>
-      </div>
-    )
-  }
-
   // Get statement results for multi-statement queries
   const statementResults = getAllStatementResults(tabId)
   const activeStatementResult = getActiveStatementResult(tabId)

apps/desktop/src/renderer/src/stores/notebook-store.ts

@@ -128,8 +128,7 @@ export const useNotebookStore = create<NotebookState>((set, get) => ({
     try {
       const result = await window.api.notebooks.duplicate(id, connectionId)
       if (result.success && result.data) {
-        const { cells: _cells, ...notebookMeta } = result.data
-        set((state) => ({ notebooks: [notebookMeta, ...state.notebooks] }))
+        set((state) => ({ notebooks: [result.data!, ...state.notebooks] }))
       } else {
         console.error('Failed to duplicate notebook:', result.error)
       }
@@ -160,9 +159,7 @@ export const useNotebookStore = create<NotebookState>((set, get) => ({
   updateCellContent: (cellId, content) => {
     set((state) => {
       if (!state.activeNotebook) return state
-      const cells = state.activeNotebook.cells.map((c) =>
-        c.id === cellId ? { ...c, content } : c
-      )
+      const cells = state.activeNotebook.cells.map((c) => (c.id === cellId ? { ...c, content } : c))
       return { activeNotebook: { ...state.activeNotebook, cells } }
     })
   },
@@ -204,7 +201,9 @@ export const useNotebookStore = create<NotebookState>((set, get) => ({
         set((state) => {
           if (!state.activeNotebook || state.activeNotebook.id !== notebookId) return state
           const cellMap = new Map(state.activeNotebook.cells.map((c) => [c.id, c]))
-          const cells = cellIds.map((id) => cellMap.get(id)).filter(Boolean) as typeof state.activeNotebook.cells
+          const cells = cellIds
+            .map((id) => cellMap.get(id))
+            .filter(Boolean) as typeof state.activeNotebook.cells
           return { activeNotebook: { ...state.activeNotebook, cells } }
         })
       } else {

package.json

@@ -54,8 +54,6 @@
     ]
   },
   "dependencies": {
-    "papaparse": "^5.5.3",
-    "react-markdown": "^10.1.0",
-    "remark-gfm": "^4.0.1"
+    "papaparse": "^5.5.3"
   }
 }