From 52d1f379cbe4acc3453fdb4fcef1063f1f1a29c8 Mon Sep 17 00:00:00 2001 From: Vikranth Reddimasu Date: Fri, 17 Apr 2026 21:41:26 -0400 Subject: [PATCH] feat(wave3): missing actions + error visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wave 3 of 5 from .gstack/qa-reports/PLAN.md. With Waves 1 and 2 in, the product no longer corrupts data and its signature feature renders — but you still can't rename a notebook, can't delete a document, can't see errors once a toast vanishes, and the AI-model-crashed case is a blank stare. This wave closes those gaps. ## Actions that didn't exist - **Rename notebook.** Backend PATCH /notebooks/:id + notebook_store rename_notebook(). Frontend: context menu → inline edit (same pattern as conversations). New notebook creation drops you directly into rename mode so "Untitled notebook" doesn't stick around. - **Delete document.** Backend DELETE /documents?notebook_id=&source_path= removes the chunks from the chroma collection, the summary row, the uploaded file (scoped to the notebook's uploads_dir with the same Path.relative_to guard as preview), and decrements the notebook's source_count/chunk_count via a new clamp-at-zero adjust_counts helper. Frontend: overflow menu on each document card with confirmation. - **Create empty notebook.** "+ New" now calls POST /notebooks/ directly instead of opening a file picker — users can shape a notebook before adding documents. The welcome + upload paths still create a notebook implicitly. - **Zotero is discoverable.** Button lives next to "+ Add" in the Documents section. It was previously only reachable via the command palette. ## Error visibility - **Notification center.** Bell icon in the sidebar footer with an unread badge; opens a slide-over panel listing the last 50 errors, successes, and warnings with relative+absolute timestamps. Toast.tsx now mirrors non-info toasts into this log so "Upload failed" at second 3 is still recoverable at second 30. role="alert" on error toasts so screen readers announce immediately. - **Human error messages.** New utils/errorMessages.ts translates common failure modes — "Request failed with status 422" -> "We couldn't process that. The file may be empty, scanned without OCR, or corrupt." Network errors, 413 size, 415 MIME, 403 scope, 500 server, Ollama-down all get specific copy. Unknown errors fall through to the raw message (truncated). Wired into Sidebar upload/delete/rename + AppShell drop. - **AppShell drop handler no longer aborts the whole batch.** Each file upload is now its own try/catch — file 2 failing doesn't swallow the report on file 1, and the error message names which file failed. ## Reliability nudges - **ConnectionBanner monitors Ollama too.** Previously checked only the backend, so a model crash mid-session was invisible until the chat failed. Now it watches both and shows a distinct amber warn-banner when Ollama is unreachable: "AI model (Ollama) isn't reachable. Start Ollama, then try your message again." - **Banner runs its check on mount**, not only after the first 30s interval tick. And the interval is 15s now, not 30s. ## Verified - cd apps/desktop && npm run build — clean (272 modules, 847ms) - cd backend && uv run ruff check . — clean - cd backend && uv run pytest -q — 33 passed (3 new: rename happy path, rename-missing returns None, adjust_counts clamps to zero) ## Intentionally not in this PR - 3.3 duplicate upload detection — needs ingestion-layer change; punted to Wave 4 alongside other vector_store work - 3.4 real upload progress bar — requires an XHR refactor of the upload path; kept as a separate PR for review surface - 3.6 wizard model selection wiring — needs a backend-side model override API; scoped out for this wave Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/desktop/src/api.ts | 21 + .../src/components/documents/DocumentCard.tsx | 62 ++- .../src/components/documents/documents.css | 37 +- .../src/components/layout/AppShell.tsx | 18 +- .../desktop/src/components/layout/Sidebar.tsx | 418 +++++++++++++----- apps/desktop/src/components/layout/layout.css | 24 + .../src/components/ui/ConnectionBanner.tsx | 85 +++- .../src/components/ui/NotificationCenter.tsx | 128 ++++++ apps/desktop/src/components/ui/Toast.tsx | 42 +- .../src/components/ui/connection-banner.css | 6 + .../src/components/ui/notification-center.css | 241 ++++++++++ apps/desktop/src/store/app-store.ts | 40 ++ apps/desktop/src/utils/errorMessages.ts | 79 ++++ backend/notebooklm_backend/models/notebook.py | 4 + .../notebooklm_backend/routes/documents.py | 61 +++ .../notebooklm_backend/routes/notebooks.py | 26 +- .../services/notebook_store.py | 37 ++ .../services/vector_store.py | 28 ++ backend/tests/test_notebook_store.py | 51 +++ 19 files changed, 1233 insertions(+), 175 deletions(-) create mode 100644 apps/desktop/src/components/ui/NotificationCenter.tsx create mode 100644 apps/desktop/src/components/ui/notification-center.css create mode 100644 apps/desktop/src/utils/errorMessages.ts diff --git a/apps/desktop/src/api.ts b/apps/desktop/src/api.ts index 921a022..8a9e831 100644 --- a/apps/desktop/src/api.ts +++ b/apps/desktop/src/api.ts @@ -125,6 +125,27 @@ export function deleteNotebook(notebookId: string): Promise<{ status: string }> }); } +export function renameNotebook(notebookId: string, title: string): Promise { + return request(`/notebooks/${encodeURIComponent(notebookId)}`, { + method: 'PATCH', + body: JSON.stringify({ title }), + }); +} + +export function deleteDocument( + notebookId: string, + sourcePath: string, +): Promise<{ status: string; chunks_removed: number }> { + const params = new URLSearchParams({ + notebook_id: notebookId, + source_path: sourcePath, + }); + return request<{ status: string; chunks_removed: number }>( + `/documents?${params.toString()}`, + { method: 'DELETE' }, + ); +} + export async function getDocumentPreviewUrl(notebookId: string, sourcePath: string): Promise { const apiBase = await getApiBase(); const params = new URLSearchParams({ diff --git a/apps/desktop/src/components/documents/DocumentCard.tsx b/apps/desktop/src/components/documents/DocumentCard.tsx index 18d8b77..a0f5a14 100644 --- a/apps/desktop/src/components/documents/DocumentCard.tsx +++ b/apps/desktop/src/components/documents/DocumentCard.tsx @@ -1,37 +1,63 @@ import type { DocumentInfo } from '../../types'; import './documents.css'; -const TYPE_COLORS: Record = { - pdf: '#ef4444', - docx: '#3b82f6', - txt: '#6b7280', - md: '#8b5cf6', - pptx: '#f97316', - py: '#22c55e', -}; - interface DocumentCardProps { document: DocumentInfo; onClick?: () => void; + onContextMenu?: (e: React.MouseEvent) => void; + /** Click handler for the explicit overflow-menu button. Separate from + * onContextMenu so a left-click on the "..." button doesn't also open + * the document preview. */ + onMenuClick?: (e: React.MouseEvent) => void; } -export function DocumentCard({ document, onClick }: DocumentCardProps) { +const FILETYPE_LABEL: Record = { + pdf: 'PDF', + docx: 'Word', + md: 'Markdown', + txt: 'Text', + pptx: 'Slides', + py: 'Python', +}; + +export function DocumentCard({ document, onClick, onContextMenu, onMenuClick }: DocumentCardProps) { const ext = document.filename.split('.').pop()?.toLowerCase() ?? ''; - const color = TYPE_COLORS[ext] ?? '#6b7280'; + const label = FILETYPE_LABEL[ext] ?? ext.toUpperCase(); return ( -
-
- {ext.toUpperCase()} -
+
{ + if ((e.key === 'Enter' || e.key === ' ') && onClick) { + e.preventDefault(); + onClick(); + } + }} + > +
{ext.toUpperCase()}
{document.filename} - - {ext === 'pdf' ? 'PDF' : ext === 'docx' ? 'Word' : ext === 'md' ? 'Markdown' : ext === 'txt' ? 'Text' : ext === 'pptx' ? 'Slides' : ext === 'py' ? 'Python' : ext.toUpperCase()} - + {label}
+ {onMenuClick && ( + + )}
); } diff --git a/apps/desktop/src/components/documents/documents.css b/apps/desktop/src/components/documents/documents.css index c8b77ab..a05cf60 100644 --- a/apps/desktop/src/components/documents/documents.css +++ b/apps/desktop/src/components/documents/documents.css @@ -1,11 +1,12 @@ /* ---- DocumentCard ---- */ .document-card { + position: relative; display: flex; align-items: center; gap: var(--space-3); padding: var(--space-2) var(--space-3); - border-radius: var(--radius-sm); + border-radius: var(--radius-pill); cursor: pointer; transition: background var(--duration-fast) var(--ease-out); } @@ -23,15 +24,17 @@ font-size: 9px; font-weight: 700; font-family: var(--font-mono); + font-variant-numeric: tabular-nums; + color: var(--color-text-secondary); min-width: 30px; height: 22px; display: flex; align-items: center; justify-content: center; border-radius: var(--radius-xs); - background: rgba(255, 255, 255, 0.04); + background: var(--color-bg-surface); border: 1px solid var(--color-border); - letter-spacing: 0.02em; + letter-spacing: 0.04em; } .document-card-info { @@ -57,6 +60,34 @@ font-variant-numeric: tabular-nums; } +.document-card-menu-btn { + flex-shrink: 0; + width: 22px; + height: 22px; + padding: 0; + background: transparent; + border: none; + color: var(--color-text-muted); + font-size: 14px; + line-height: 1; + cursor: pointer; + border-radius: var(--radius-pill); + opacity: 0; + transition: opacity var(--duration-fast) var(--ease-out), + color var(--duration-fast) var(--ease-out), + background var(--duration-fast) var(--ease-out); +} + +.document-card:hover .document-card-menu-btn, +.document-card-menu-btn:focus-visible { + opacity: 1; +} + +.document-card-menu-btn:hover { + color: var(--color-text-primary); + background: var(--color-bg-surface); +} + /* ---- DropZone ---- */ .drop-zone { diff --git a/apps/desktop/src/components/layout/AppShell.tsx b/apps/desktop/src/components/layout/AppShell.tsx index 1e08df6..8863919 100644 --- a/apps/desktop/src/components/layout/AppShell.tsx +++ b/apps/desktop/src/components/layout/AppShell.tsx @@ -15,6 +15,7 @@ import { CommandPalette } from '../ui/CommandPalette'; import { KeyboardShortcutsOverlay } from '../ui/KeyboardShortcuts'; import { ZoteroImportDialog } from '../ui/ZoteroImport'; import type { SourceChunk } from '../../types'; +import { humanizeError } from '../../utils/errorMessages'; import './layout.css'; function isWizardComplete(): boolean { @@ -135,19 +136,22 @@ export function AppShell() { const activeNotebook = notebooks.find((nb) => nb.notebook_id === activeNotebookId); const handleGlobalDrop = useCallback(async (files: FileList) => { - try { - for (const file of Array.from(files)) { + for (const file of Array.from(files)) { + try { showToast(`Processing ${file.name}...`); const result = await uploadDocument(file, activeNotebookId || undefined); if (!activeNotebookId) { useAppStore.getState().setActiveNotebookId(result.notebook_id); } - showToast(`${file.name} indexed successfully`, 'success'); + showToast(`${file.name} indexed`, 'success'); + } catch (err) { + // Per-file failure should be specific — the old behavior aborted the + // whole batch and reported only a generic "Upload failed" with no + // filename. + showToast(humanizeError(err, { action: `upload ${file.name}` }), 'error'); } - await refreshNotebooks(); - } catch (err) { - showToast(err instanceof Error ? err.message : 'Upload failed', 'error'); } + await refreshNotebooks(); }, [activeNotebookId, refreshNotebooks]); const welcomeFileRef = useRef(null); @@ -235,7 +239,7 @@ export function AppShell() { return ( <>
- + setZoteroOpen(true)} /> setPendingSuggest(null)} diff --git a/apps/desktop/src/components/layout/Sidebar.tsx b/apps/desktop/src/components/layout/Sidebar.tsx index ad27811..936ab39 100644 --- a/apps/desktop/src/components/layout/Sidebar.tsx +++ b/apps/desktop/src/components/layout/Sidebar.tsx @@ -6,26 +6,37 @@ import { useConversations } from '../../hooks/useConversations'; import { DocumentCard } from '../documents/DocumentCard'; import { showToast } from '../ui/Toast'; import { ConfirmDialog } from '../ui/ConfirmDialog'; -import { deleteNotebook } from '../../api'; +import { NotificationCenter } from '../ui/NotificationCenter'; +import { createNotebook, deleteDocument, deleteNotebook, renameNotebook } from '../../api'; +import { humanizeError } from '../../utils/errorMessages'; import { timeAgo } from '../../utils/timeAgo'; +import type { DocumentInfo } from '../../types'; import './layout.css'; +interface SidebarProps { + onOpenZotero?: () => void; +} + type PendingConfirm = | { kind: 'delete-notebook'; notebookId: string; title: string } - | { kind: 'delete-conversation'; conversationId: string; title: string }; + | { kind: 'delete-conversation'; conversationId: string; title: string } + | { kind: 'delete-document'; notebookId: string; sourcePath: string; filename: string }; + +type RenameTarget = + | { kind: 'notebook'; notebookId: string } + | { kind: 'conversation'; conversationId: string }; -// Lazy Scholar palette: warm, muted tones that match the stone-sage design system const NOTEBOOK_COLORS = [ - '#7c9a82', // sage (primary accent) - '#b8977e', // warm tan - '#8b9eb0', // slate blue - '#c9956b', // amber stone - '#9b8ba0', // dusty mauve - '#7ea898', // sea glass - '#b09070', // clay - '#8a9a70', // olive - '#a08090', // muted rose - '#80a0a0', // teal stone + '#7c9a82', + '#b8977e', + '#8b9eb0', + '#c9956b', + '#9b8ba0', + '#7ea898', + '#b09070', + '#8a9a70', + '#a08090', + '#80a0a0', ]; function notebookColor(id: string): string { @@ -38,34 +49,48 @@ function notebookColor(id: string): string { const MAX_VISIBLE_CONVERSATIONS = 5; -export function Sidebar() { +export function Sidebar({ onOpenZotero }: SidebarProps) { const { notebooks, activeNotebookId, select, refresh: refreshNotebooks } = useNotebooks(); - const { documents, upload } = useDocuments(); - const { conversations, activeConversationId, loadConversation, deleteConversation: deleteConv, renameConversation: renameConv } = useConversations(); + const { documents, upload, refresh: refreshDocuments } = useDocuments(); + const { + conversations, + activeConversationId, + loadConversation, + deleteConversation: deleteConv, + renameConversation: renameConv, + } = useConversations(); const status = useAppStore((s) => s.status); const setPreviewDocument = useAppStore((s) => s.setPreviewDocument); + const fileInputRef = useRef(null); + const renameInputRef = useRef(null); + const [contextMenu, setContextMenu] = useState<{ x: number; y: number; notebookId: string } | null>(null); const [convMenu, setConvMenu] = useState<{ x: number; y: number; conversationId: string } | null>(null); + const [docMenu, setDocMenu] = useState<{ x: number; y: number; document: DocumentInfo } | null>(null); const [showAllConversations, setShowAllConversations] = useState(false); - const [renamingId, setRenamingId] = useState(null); + const [renameTarget, setRenameTarget] = useState(null); const [renameValue, setRenameValue] = useState(''); const [pendingConfirm, setPendingConfirm] = useState(null); - const renameInputRef = useRef(null); + const [isCreating, setIsCreating] = useState(false); useEffect(() => { - if (!contextMenu && !convMenu) return; - const handler = () => { setContextMenu(null); setConvMenu(null); }; + if (!contextMenu && !convMenu && !docMenu) return; + const handler = () => { + setContextMenu(null); + setConvMenu(null); + setDocMenu(null); + }; window.addEventListener('click', handler); return () => window.removeEventListener('click', handler); - }, [contextMenu, convMenu]); + }, [contextMenu, convMenu, docMenu]); useEffect(() => { - if (renamingId && renameInputRef.current) { + if (renameTarget && renameInputRef.current) { renameInputRef.current.focus(); renameInputRef.current.select(); } - }, [renamingId]); + }, [renameTarget]); const askDeleteNotebook = (notebookId: string) => { const nb = notebooks.find((n) => n.notebook_id === notebookId); @@ -87,6 +112,17 @@ export function Sidebar() { setConvMenu(null); }; + const askDeleteDocument = (doc: DocumentInfo) => { + if (!activeNotebookId) return; + setPendingConfirm({ + kind: 'delete-document', + notebookId: activeNotebookId, + sourcePath: doc.source_path, + filename: doc.filename, + }); + setDocMenu(null); + }; + const confirmPending = async () => { if (!pendingConfirm) return; const current = pendingConfirm; @@ -95,46 +131,98 @@ export function Sidebar() { if (current.kind === 'delete-notebook') { try { await deleteNotebook(current.notebookId); - if (activeNotebookId === current.notebookId) { - select(null); - } + if (activeNotebookId === current.notebookId) select(null); await refreshNotebooks(); showToast('Notebook deleted', 'success'); - } catch { - showToast('Failed to delete notebook', 'error'); + } catch (err) { + showToast(humanizeError(err, { action: 'delete notebook' }), 'error'); } } else if (current.kind === 'delete-conversation') { await deleteConv(current.conversationId); + } else if (current.kind === 'delete-document') { + try { + await deleteDocument(current.notebookId, current.sourcePath); + await refreshDocuments(); + await refreshNotebooks(); + showToast(`Removed ${current.filename}`, 'success'); + } catch (err) { + showToast(humanizeError(err, { action: 'delete document' }), 'error'); + } } }; - const handleNewClick = () => { - fileInputRef.current?.click(); + const handleNewNotebook = async () => { + if (isCreating) return; + setIsCreating(true); + try { + const nb = await createNotebook('Untitled notebook'); + await refreshNotebooks(); + select(nb.notebook_id); + // Drop the user straight into rename mode for the new notebook. + setRenameValue(nb.title); + setRenameTarget({ kind: 'notebook', notebookId: nb.notebook_id }); + } catch (err) { + showToast(humanizeError(err, { action: 'create notebook' }), 'error'); + } finally { + setIsCreating(false); + } }; + const handleAddDocumentClick = () => fileInputRef.current?.click(); + const handleFileSelect = async (e: React.ChangeEvent) => { if (!e.target.files?.length) return; try { for (const file of Array.from(e.target.files)) { showToast(`Processing ${file.name}...`); await upload(file); - showToast(`${file.name} indexed successfully`, 'success'); + showToast(`${file.name} indexed`, 'success'); } await refreshNotebooks(); } catch (err) { - showToast(err instanceof Error ? err.message : 'Upload failed', 'error'); + showToast(humanizeError(err, { action: 'upload' }), 'error'); } e.target.value = ''; }; - const handleRenameSubmit = (conversationId: string) => { - if (renameValue.trim()) { - renameConv(conversationId, renameValue.trim()); + const handleRenameSubmit = async () => { + const trimmed = renameValue.trim(); + const target = renameTarget; + setRenameTarget(null); + setRenameValue(''); + if (!trimmed || !target) return; + + if (target.kind === 'conversation') { + renameConv(target.conversationId, trimmed); + } else if (target.kind === 'notebook') { + try { + await renameNotebook(target.notebookId, trimmed); + await refreshNotebooks(); + } catch (err) { + showToast(humanizeError(err, { action: 'rename notebook' }), 'error'); + } } - setRenamingId(null); + }; + + const cancelRename = () => { + setRenameTarget(null); setRenameValue(''); }; + const beginRenameNotebook = (notebookId: string) => { + const nb = notebooks.find((n) => n.notebook_id === notebookId); + setRenameValue(nb?.title || ''); + setRenameTarget({ kind: 'notebook', notebookId }); + setContextMenu(null); + }; + + const beginRenameConversation = (conversationId: string) => { + const conv = conversations.find((c) => c.id === conversationId); + setRenameValue(conv?.title || ''); + setRenameTarget({ kind: 'conversation', conversationId }); + setConvMenu(null); + }; + const visibleConversations = showAllConversations ? conversations : conversations.slice(0, MAX_VISIBLE_CONVERSATIONS); @@ -143,7 +231,13 @@ export function Sidebar() {
{contextMenu && ( -
+
+ @@ -331,6 +479,28 @@ export function Sidebar() {
)} + {docMenu && ( +
+ + +
+ )} + setPendingConfirm(null)} /> + + setPendingConfirm(null)} + /> ); } diff --git a/apps/desktop/src/components/layout/layout.css b/apps/desktop/src/components/layout/layout.css index 8bc0604..ff4d865 100644 --- a/apps/desktop/src/components/layout/layout.css +++ b/apps/desktop/src/components/layout/layout.css @@ -159,6 +159,26 @@ flex-shrink: 0; } +.sidebar-section-actions { + display: flex; + align-items: center; + gap: var(--space-1); +} + +.sidebar-rename-input { + flex: 1; + min-width: 0; + background: var(--color-bg-primary); + border: 1px solid var(--color-accent); + border-radius: var(--radius-pill); + color: var(--color-text-primary); + font-family: var(--font-sans); + font-size: var(--text-sm); + font-weight: 450; + padding: 4px 12px; + outline: none; +} + .sidebar-add-doc-btn { -webkit-app-region: no-drag; background: transparent; @@ -341,6 +361,10 @@ padding: var(--space-3) var(--space-4); border-top: 1px solid var(--color-border); flex-shrink: 0; + display: flex; + align-items: center; + justify-content: space-between; + gap: var(--space-3); -webkit-app-region: no-drag; } diff --git a/apps/desktop/src/components/ui/ConnectionBanner.tsx b/apps/desktop/src/components/ui/ConnectionBanner.tsx index 4f42f1a..a4193c2 100644 --- a/apps/desktop/src/components/ui/ConnectionBanner.tsx +++ b/apps/desktop/src/components/ui/ConnectionBanner.tsx @@ -1,48 +1,85 @@ import { useEffect, useRef, useState } from 'react'; import './connection-banner.css'; +type ConnectionState = 'healthy' | 'backend-down' | 'ollama-down'; + +const BACKEND_HEALTHZ = 'http://127.0.0.1:8000/api/healthz'; +const OLLAMA_VERSION = 'http://127.0.0.1:11434/api/version'; + +/** + * Top-of-window banner that watches both the backend and Ollama. Raw errors + * from a dead model used to land in the chat bubble with no recovery hint; + * now the banner tells you the model is offline and how to recover. + */ export function ConnectionBanner() { - const [disconnected, setDisconnected] = useState(false); + const [state, setState] = useState('healthy'); const [showReconnected, setShowReconnected] = useState(false); - const wasDisconnected = useRef(false); + const wasDown = useRef(false); useEffect(() => { let mounted = true; const check = async () => { + // Backend is the hard blocker — if it's down, don't bother checking + // Ollama because the user can't do anything either way. try { - // Try the default backend URL; the app already resolved it during bootstrap - const res = await fetch('http://127.0.0.1:8000/api/healthz', { - signal: AbortSignal.timeout(5000), - }); + const backendRes = await fetch(BACKEND_HEALTHZ, { signal: AbortSignal.timeout(4000) }); + if (!backendRes.ok) throw new Error('backend-down'); + } catch { if (!mounted) return; - if (res.ok) { - if (wasDisconnected.current) { - setShowReconnected(true); - setTimeout(() => { if (mounted) setShowReconnected(false); }, 3000); - wasDisconnected.current = false; - } - setDisconnected(false); - } else { - wasDisconnected.current = true; - setDisconnected(true); - } + wasDown.current = true; + setState('backend-down'); + return; + } + + // Ollama is a soft blocker — app still lists notebooks, but chat will fail. + try { + const ollamaRes = await fetch(OLLAMA_VERSION, { signal: AbortSignal.timeout(3000) }); + if (!ollamaRes.ok) throw new Error('ollama-down'); } catch { if (!mounted) return; - wasDisconnected.current = true; - setDisconnected(true); + wasDown.current = true; + setState('ollama-down'); + return; } + + if (!mounted) return; + if (wasDown.current) { + setShowReconnected(true); + setTimeout(() => { + if (mounted) setShowReconnected(false); + }, 3000); + wasDown.current = false; + } + setState('healthy'); }; - const interval = setInterval(check, 30000); - return () => { mounted = false; clearInterval(interval); }; + // Run immediately on mount so the user sees status without waiting 30s. + check(); + const interval = setInterval(check, 15000); + return () => { + mounted = false; + clearInterval(interval); + }; }, []); - if (!disconnected && !showReconnected) return null; + if (state === 'healthy' && !showReconnected) return null; + + if (showReconnected) { + return
Reconnected
; + } + + if (state === 'backend-down') { + return ( +
+ Backend isn't responding. Reconnecting… +
+ ); + } return ( -
- {showReconnected ? 'Reconnected' : 'Connection lost. Retrying...'} +
+ AI model (Ollama) isn't reachable. Start Ollama, then try your message again.
); } diff --git a/apps/desktop/src/components/ui/NotificationCenter.tsx b/apps/desktop/src/components/ui/NotificationCenter.tsx new file mode 100644 index 0000000..2ebc21f --- /dev/null +++ b/apps/desktop/src/components/ui/NotificationCenter.tsx @@ -0,0 +1,128 @@ +import { useEffect, useState } from 'react'; +import { useAppStore, type AppNotification } from '../../store/app-store'; +import './notification-center.css'; + +function relativeTime(ts: number): string { + const diffSec = Math.floor((Date.now() - ts) / 1000); + if (diffSec < 45) return 'just now'; + if (diffSec < 90) return '1 min ago'; + const min = Math.floor(diffSec / 60); + if (min < 60) return `${min} min ago`; + const hr = Math.floor(min / 60); + if (hr < 24) return `${hr} hr ago`; + const day = Math.floor(hr / 24); + return `${day} day${day === 1 ? '' : 's'} ago`; +} + +function formatAbsoluteTime(ts: number): string { + return new Date(ts).toLocaleString(); +} + +export function NotificationCenter() { + const notifications = useAppStore((s) => s.notifications); + const markAllRead = useAppStore((s) => s.markAllNotificationsRead); + const clearAll = useAppStore((s) => s.clearNotifications); + const dismiss = useAppStore((s) => s.dismissNotification); + const [open, setOpen] = useState(false); + + const unreadCount = notifications.filter((n) => n.unread).length; + + useEffect(() => { + if (!open) return; + const handler = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + e.preventDefault(); + setOpen(false); + } + }; + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, [open]); + + const handleOpen = () => { + setOpen((v) => { + const next = !v; + if (next && unreadCount > 0) { + // Open → mark everything read so the badge clears. + setTimeout(markAllRead, 0); + } + return next; + }); + }; + + return ( + <> + + + {open && ( + <> +
setOpen(false)} /> +
+
+

Activity

+ {notifications.length > 0 && ( + + )} +
+ {notifications.length === 0 ? ( +
+

Nothing to report.

+

Errors, uploads, and warnings will collect here.

+
+ ) : ( +
    + {notifications.map((n) => ( + dismiss(n.id)} /> + ))} +
+ )} +
+ + )} + + ); +} + +function NotificationRow({ + notification, + onDismiss, +}: { + notification: AppNotification; + onDismiss: () => void; +}) { + return ( +
  • +
    +

    {notification.title}

    + {notification.body &&

    {notification.body}

    } +

    + {relativeTime(notification.timestamp)} +

    +
    + +
    • + ); +} diff --git a/apps/desktop/src/components/ui/Toast.tsx b/apps/desktop/src/components/ui/Toast.tsx index daff0d5..b525b0f 100644 --- a/apps/desktop/src/components/ui/Toast.tsx +++ b/apps/desktop/src/components/ui/Toast.tsx @@ -1,19 +1,52 @@ import { useEffect, useState } from 'react'; +import { useAppStore, type NotificationLevel } from '../../store/app-store'; import './toast.css'; interface ToastItem { id: string; message: string; - type: 'info' | 'success' | 'error'; + type: ToastType; + detail?: string; } +type ToastType = 'info' | 'success' | 'error' | 'warning'; + const MAX_VISIBLE = 3; let toastListeners: ((toast: ToastItem) => void)[] = []; -export function showToast(message: string, type: ToastItem['type'] = 'info') { - const toast: ToastItem = { id: Date.now().toString(), message, type }; +interface ShowToastOptions { + /** Longer-form explanation that lands in the notification center only. */ + detail?: string; + /** Skip writing this to the persistent log. Default: log everything that + * isn't 'info'. */ + ephemeral?: boolean; +} + +export function showToast( + message: string, + type: ToastType = 'info', + options: ShowToastOptions = {}, +) { + const toast: ToastItem = { + id: `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, + message, + type, + detail: options.detail, + }; toastListeners.forEach((fn) => fn(toast)); + + // Mirror into the persistent notification log unless opted out. 'info' + // toasts ("Processing foo.pdf…") aren't worth keeping around; everything + // else lands in the bell so the user can recover a missed message. + const shouldLog = !options.ephemeral && type !== 'info'; + if (shouldLog) { + useAppStore.getState().addNotification({ + level: type as NotificationLevel, + title: message, + body: options.detail, + }); + } } export function ToastContainer() { @@ -27,7 +60,6 @@ export function ToastContainer() { const handler = (toast: ToastItem) => { setToasts((prev) => { const next = [...prev, toast]; - // Stack limit: keep only the newest MAX_VISIBLE return next.slice(-MAX_VISIBLE); }); setTimeout(() => { @@ -49,7 +81,7 @@ export function ToastContainer() { key={toast.id} className={`toast toast-${toast.type}`} onClick={() => dismiss(toast.id)} - role="status" + role={toast.type === 'error' ? 'alert' : 'status'} > {toast.message}
    diff --git a/apps/desktop/src/components/ui/connection-banner.css b/apps/desktop/src/components/ui/connection-banner.css index 83feecc..7da5f51 100644 --- a/apps/desktop/src/components/ui/connection-banner.css +++ b/apps/desktop/src/components/ui/connection-banner.css @@ -24,3 +24,9 @@ background: rgba(74, 222, 128, 0.15); border-color: rgba(74, 222, 128, 0.3); } + +.connection-banner-warn { + background: rgba(217, 146, 90, 0.14); + border-color: rgba(217, 146, 90, 0.32); + color: var(--color-text-primary); +} diff --git a/apps/desktop/src/components/ui/notification-center.css b/apps/desktop/src/components/ui/notification-center.css new file mode 100644 index 0000000..3af7786 --- /dev/null +++ b/apps/desktop/src/components/ui/notification-center.css @@ -0,0 +1,241 @@ +/* Notification center — bell trigger + slide-over panel. Sits in the + * sidebar footer so it's always reachable without being in the user's way. */ + +.notif-trigger { + position: relative; + display: inline-flex; + align-items: center; + justify-content: center; + width: 28px; + height: 28px; + background: transparent; + border: 1px solid var(--color-border); + border-radius: var(--radius-pill); + color: var(--color-text-muted); + font-size: 14px; + cursor: pointer; + transition: background var(--duration-fast) var(--ease-out), + color var(--duration-fast) var(--ease-out), + border-color var(--duration-fast) var(--ease-out); +} + +.notif-trigger:hover { + background: var(--color-bg-hover); + color: var(--color-text-secondary); + border-color: var(--color-border-hover); +} + +.notif-trigger.has-unread { + color: var(--color-cite); + border-color: rgba(217, 146, 90, 0.35); +} + +.notif-bell { + line-height: 1; + font-size: 13px; +} + +.notif-badge { + position: absolute; + top: -4px; + right: -4px; + min-width: 16px; + height: 16px; + padding: 0 4px; + display: inline-flex; + align-items: center; + justify-content: center; + font-family: var(--font-mono); + font-variant-numeric: tabular-nums; + font-size: 10px; + font-weight: 600; + color: #0c0a09; + background: var(--color-cite); + border-radius: var(--radius-pill); + letter-spacing: 0.02em; +} + +.notif-backdrop { + position: fixed; + inset: 0; + background: rgba(0, 0, 0, 0.3); + z-index: 1050; + animation: notifBackdropIn 140ms var(--ease-out); +} + +@keyframes notifBackdropIn { + from { opacity: 0; } + to { opacity: 1; } +} + +.notif-panel { + position: fixed; + top: var(--titlebar-height); + left: var(--space-4); + width: 360px; + max-width: 90vw; + max-height: calc(100vh - var(--titlebar-height) - var(--space-6)); + background: var(--color-bg-secondary); + border: 1px solid var(--color-border); + border-radius: var(--radius-modal); + box-shadow: var(--shadow-lg); + display: flex; + flex-direction: column; + overflow: hidden; + z-index: 1060; + animation: notifPanelIn 180ms var(--ease-out); +} + +@keyframes notifPanelIn { + from { opacity: 0; transform: translateY(-6px); } + to { opacity: 1; transform: translateY(0); } +} + +.notif-panel-header { + display: flex; + align-items: center; + justify-content: space-between; + padding: var(--space-4) var(--space-5) var(--space-3); + border-bottom: 1px solid var(--color-border); +} + +.notif-panel-header h3 { + margin: 0; + font-family: var(--font-sans); + font-size: var(--text-sm); + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.06em; + color: var(--color-text-muted); +} + +.notif-clear { + background: transparent; + border: none; + color: var(--color-text-muted); + font-family: var(--font-sans); + font-size: var(--text-xs); + font-weight: 500; + cursor: pointer; + padding: 4px 8px; + border-radius: var(--radius-pill); + transition: color var(--duration-fast) var(--ease-out), + background var(--duration-fast) var(--ease-out); +} + +.notif-clear:hover { + color: var(--color-text-primary); + background: var(--color-bg-hover); +} + +.notif-empty { + padding: var(--space-6) var(--space-5) var(--space-8); +} + +.notif-empty-headline { + margin: 0 0 var(--space-1); + font-family: var(--font-serif); + font-style: italic; + font-size: var(--text-lg); + color: var(--color-text-secondary); + letter-spacing: -0.01em; +} + +.notif-empty-hint { + margin: 0; + font-size: var(--text-sm); + color: var(--color-text-muted); + line-height: 1.55; +} + +.notif-list { + margin: 0; + padding: 0; + list-style: none; + overflow-y: auto; + flex: 1; +} + +.notif-row { + display: flex; + align-items: flex-start; + gap: var(--space-3); + padding: var(--space-3) var(--space-5); + border-bottom: 1px solid var(--color-border-subtle); + border-left: 2px solid transparent; + transition: background var(--duration-fast) var(--ease-out); +} + +.notif-row:hover { + background: var(--color-bg-hover); +} + +.notif-row.unread { + background: var(--color-bg-hover); +} + +.notif-row-info { border-left-color: var(--color-text-muted); } +.notif-row-success { border-left-color: var(--color-success); } +.notif-row-warning { border-left-color: var(--color-warning); } +.notif-row-error { border-left-color: var(--color-error); } + +.notif-row-main { + flex: 1; + min-width: 0; + display: flex; + flex-direction: column; + gap: 2px; +} + +.notif-row-title { + margin: 0; + font-family: var(--font-sans); + font-size: var(--text-sm); + font-weight: 500; + color: var(--color-text-primary); + line-height: 1.4; +} + +.notif-row-body { + margin: 0; + font-size: var(--text-xs); + color: var(--color-text-secondary); + line-height: 1.55; + word-wrap: break-word; +} + +.notif-row-time { + margin: 0; + font-family: var(--font-mono); + font-size: 10px; + font-variant-numeric: tabular-nums; + color: var(--color-text-muted); + letter-spacing: 0.02em; +} + +.notif-row-dismiss { + flex-shrink: 0; + width: 20px; + height: 20px; + padding: 0; + background: transparent; + border: none; + color: var(--color-text-muted); + font-size: 16px; + line-height: 1; + cursor: pointer; + border-radius: var(--radius-pill); + opacity: 0; + transition: opacity var(--duration-fast) var(--ease-out), + color var(--duration-fast) var(--ease-out), + background var(--duration-fast) var(--ease-out); +} + +.notif-row:hover .notif-row-dismiss { + opacity: 1; +} + +.notif-row-dismiss:hover { + color: var(--color-text-primary); + background: var(--color-bg-hover); +} diff --git a/apps/desktop/src/store/app-store.ts b/apps/desktop/src/store/app-store.ts index 17b17d9..ac56d20 100644 --- a/apps/desktop/src/store/app-store.ts +++ b/apps/desktop/src/store/app-store.ts @@ -2,6 +2,20 @@ import { create } from 'zustand'; import type { BackendConfig, ChatMessage, Conversation, DocumentInfo, Notebook, SourceChunk } from '../types'; +export type NotificationLevel = 'info' | 'success' | 'error' | 'warning'; + +export interface AppNotification { + id: string; + level: NotificationLevel; + title: string; + body?: string; + timestamp: number; + /** True until the user opens the center or dismisses it. */ + unread: boolean; +} + +const MAX_NOTIFICATIONS = 50; + function makeId(): string { // crypto.randomUUID is available in Electron's renderer and modern browsers. if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') { @@ -38,6 +52,9 @@ interface AppState { activeSources: SourceChunk[]; previewDocument: DocumentInfo | null; + // Notifications (persistent log — toasts are ephemeral, these stick around) + notifications: AppNotification[]; + // Actions — connection setStatus: (status: AppState['status']) => void; setConfig: (config: BackendConfig) => void; @@ -77,6 +94,12 @@ interface AppState { toggleSourcePanel: () => void; setSourcePanelOpen: (open: boolean) => void; setPreviewDocument: (doc: DocumentInfo | null) => void; + + // Actions — notifications + addNotification: (n: Omit) => void; + markAllNotificationsRead: () => void; + clearNotifications: () => void; + dismissNotification: (id: string) => void; } export const useAppStore = create((set) => ({ @@ -94,6 +117,7 @@ export const useAppStore = create((set) => ({ sourcePanelOpen: true, activeSources: [], previewDocument: null, + notifications: [], // Connection setStatus: (status) => set({ status }), @@ -144,4 +168,20 @@ export const useAppStore = create((set) => ({ toggleSourcePanel: () => set((state) => ({ sourcePanelOpen: !state.sourcePanelOpen })), setSourcePanelOpen: (open) => set({ sourcePanelOpen: open }), setPreviewDocument: (doc) => set({ previewDocument: doc }), + + // Notifications — newest first, capped so the log doesn't grow unbounded + addNotification: (n) => + set((state) => ({ + notifications: [ + { ...n, id: makeId(), timestamp: Date.now(), unread: true }, + ...state.notifications, + ].slice(0, MAX_NOTIFICATIONS), + })), + markAllNotificationsRead: () => + set((state) => ({ + notifications: state.notifications.map((n) => ({ ...n, unread: false })), + })), + clearNotifications: () => set({ notifications: [] }), + dismissNotification: (id) => + set((state) => ({ notifications: state.notifications.filter((n) => n.id !== id) })), })); diff --git a/apps/desktop/src/utils/errorMessages.ts b/apps/desktop/src/utils/errorMessages.ts new file mode 100644 index 0000000..3adee70 --- /dev/null +++ b/apps/desktop/src/utils/errorMessages.ts @@ -0,0 +1,79 @@ +/** + * Translate raw backend / network errors into sentences a human actually + * wants to read. Uses the original error as a fallback so we never obscure + * unknown failures — just reframe the ones we recognise. + */ + +interface ErrorContext { + /** Short hint about what the user was doing — shown in the message. + * E.g. "upload", "delete", "rename". */ + action?: string; +} + +const KNOWN_PATTERNS: Array<{ test: (msg: string) => boolean; render: (ctx: ErrorContext) => string }> = [ + { + test: (m) => /ECONNREFUSED|Failed to fetch|NetworkError|Network request failed/i.test(m), + render: (ctx) => + ctx.action + ? `The backend isn't reachable, so we couldn't ${ctx.action}. Check that the server is running.` + : "The backend isn't reachable. Check that the server is running.", + }, + { + test: (m) => /timed? out|ETIMEDOUT/i.test(m), + render: (ctx) => + ctx.action + ? `The ${ctx.action} request took too long and was stopped. Try again in a moment.` + : 'The request took too long and was stopped.', + }, + { + test: (m) => /status 413|payload too large|Request Entity Too Large/i.test(m), + render: () => "That file is too large. Split it into smaller documents and try again.", + }, + { + test: (m) => /status 415|Unsupported Media Type|unsupported file/i.test(m), + render: () => "That file type isn't supported. Try a PDF, DOCX, PPTX, TXT, or Markdown file.", + }, + { + test: (m) => /status 422|Unprocessable Entity/i.test(m), + render: () => "We couldn't process that. The file may be empty, scanned without OCR, or corrupt.", + }, + { + test: (m) => /status 403|Access denied|Forbidden/i.test(m), + render: () => "That file is outside this notebook.", + }, + { + test: (m) => /status 404|Not Found/i.test(m), + render: (ctx) => + ctx.action ? `Couldn't find what we needed to ${ctx.action}.` : "Not found.", + }, + { + test: (m) => /status 500|Internal Server Error/i.test(m), + render: (ctx) => + ctx.action + ? `Something went wrong on the server while trying to ${ctx.action}.` + : 'Something went wrong on the server.', + }, + { + test: (m) => /Ollama|ollama|model not found/i.test(m), + render: () => + "The AI model isn't available. Make sure Ollama is running and the model is installed.", + }, +]; + +/** Produce a single-line user-facing message for a caught error. */ +export function humanizeError(err: unknown, context: ErrorContext = {}): string { + const raw = + err instanceof Error + ? err.message + : typeof err === 'string' + ? err + : 'Unknown error'; + + for (const { test, render } of KNOWN_PATTERNS) { + if (test(raw)) return render(context); + } + // Fallback: drop any FastAPI-style JSON detail wrapper and cap length. + const jsonMatch = raw.match(/"detail"\s*:\s*"([^"]+)"/); + if (jsonMatch) return jsonMatch[1]; + return raw.length > 140 ? raw.slice(0, 140) + '…' : raw; +} diff --git a/backend/notebooklm_backend/models/notebook.py b/backend/notebooklm_backend/models/notebook.py index 6874209..8f03939 100644 --- a/backend/notebooklm_backend/models/notebook.py +++ b/backend/notebooklm_backend/models/notebook.py @@ -27,6 +27,10 @@ class CreateNotebookRequest(BaseModel): title: str = "New Notebook" +class RenameNotebookRequest(BaseModel): + title: str = Field(..., min_length=1, max_length=200) + + class IngestionJobStatus(BaseModel): job_id: str notebook_id: str diff --git a/backend/notebooklm_backend/routes/documents.py b/backend/notebooklm_backend/routes/documents.py index da0e30f..dbaefbe 100644 --- a/backend/notebooklm_backend/routes/documents.py +++ b/backend/notebooklm_backend/routes/documents.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import uuid from pathlib import Path @@ -11,6 +12,8 @@ from ..services.notebook_store import NotebookStore from ..models.notebook import NotebookIngestionRequest +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/documents", tags=["documents"]) @@ -151,6 +154,64 @@ async def list_documents( raise HTTPException(status_code=500, detail=f"Failed to list documents: {str(e)}") +@router.delete("") +async def delete_document( + request: Request, + notebook_id: str, + source_path: str, +) -> JSONResponse: + """Remove a single document from a notebook — its chunks from the vector + store, its summary row, its uploaded file, and decrement the notebook's + counts. Notebook itself stays.""" + from urllib.parse import unquote + from ..services.vector_store import VectorStoreManager + + settings: AppConfig = request.app.state.settings + vector_store: VectorStoreManager = request.app.state.vector_store + notebook_store: NotebookStore = request.app.state.notebook_store + + decoded_path = unquote(source_path) + + # 1) Drop chunks from the notebook's chroma collection. + try: + chunks_removed = vector_store.delete_document(notebook_id, decoded_path) + except Exception: + logger.exception("vector_store.delete_document failed") + chunks_removed = 0 + + # 2) Delete the uploaded file if it sits inside this notebook's scoped + # uploads directory. Scope-check is the same pattern as preview. + uploads_dir = (settings.data_dir / "uploads" / notebook_id).resolve() + filename = Path(decoded_path).name + candidate = (uploads_dir / filename).resolve() + if not candidate.exists() and Path(decoded_path).is_absolute(): + abs_candidate = Path(decoded_path).resolve() + try: + abs_candidate.relative_to(uploads_dir) + candidate = abs_candidate + except ValueError: + candidate = uploads_dir / filename # fall through — won't exist + if candidate.exists(): + try: + candidate.relative_to(uploads_dir) + candidate.unlink() + except (ValueError, OSError): + logger.warning("could not unlink file for delete: %s", candidate) + + # 3) Decrement notebook counts by what we actually removed. + if chunks_removed > 0: + notebook_store.adjust_counts(notebook_id, source_delta=-1, chunk_delta=-chunks_removed) + + return JSONResponse( + content={ + "status": "deleted", + "notebook_id": notebook_id, + "source_path": decoded_path, + "chunks_removed": chunks_removed, + } + ) + + @router.get("/preview") async def preview_document( request: Request, diff --git a/backend/notebooklm_backend/routes/notebooks.py b/backend/notebooklm_backend/routes/notebooks.py index fd29fcf..f329e29 100644 --- a/backend/notebooklm_backend/routes/notebooks.py +++ b/backend/notebooklm_backend/routes/notebooks.py @@ -7,7 +7,14 @@ from fastapi import APIRouter, Request -from ..models.notebook import CreateNotebookRequest, IngestionJobStatus, NotebookMetadata +from fastapi import HTTPException + +from ..models.notebook import ( + CreateNotebookRequest, + IngestionJobStatus, + NotebookMetadata, + RenameNotebookRequest, +) from ..services.notebook_store import NotebookStore logger = logging.getLogger(__name__) @@ -37,6 +44,23 @@ async def create_notebook(request: Request, body: CreateNotebookRequest) -> Note return store.upsert_notebook(notebook) +@router.patch("/{notebook_id}", response_model=NotebookMetadata) +async def rename_notebook( + request: Request, + notebook_id: str, + body: RenameNotebookRequest, +) -> NotebookMetadata: + """Rename a notebook. Only `title` is editable today.""" + store: NotebookStore = request.app.state.notebook_store + title = body.title.strip() + if not title: + raise HTTPException(status_code=422, detail="Title cannot be empty") + updated = store.rename_notebook(notebook_id, title) + if updated is None: + raise HTTPException(status_code=404, detail="Notebook not found") + return updated + + @router.delete("/{notebook_id}") async def delete_notebook(request: Request, notebook_id: str) -> dict[str, str]: store: NotebookStore = request.app.state.notebook_store diff --git a/backend/notebooklm_backend/services/notebook_store.py b/backend/notebooklm_backend/services/notebook_store.py index 719d00a..784699d 100644 --- a/backend/notebooklm_backend/services/notebook_store.py +++ b/backend/notebooklm_backend/services/notebook_store.py @@ -228,6 +228,43 @@ def delete_notebook(self, notebook_id: str) -> None: conn.execute("DELETE FROM ingestion_jobs WHERE notebook_id = ?", (notebook_id,)) conn.execute("DELETE FROM notebooks WHERE notebook_id = ?", (notebook_id,)) + def adjust_counts( + self, notebook_id: str, source_delta: int, chunk_delta: int + ) -> NotebookMetadata | None: + """Apply deltas to source_count and chunk_count, clamped at zero. + + Used by the document delete path — removing a document reduces the + notebook's count without full reingestion. Clamps to 0 defensively + so a data inconsistency can't produce negative counts.""" + now = datetime.now(timezone.utc) + with self._connect() as conn: + cursor = conn.execute( + """ + UPDATE notebooks + SET source_count = MAX(0, source_count + ?), + chunk_count = MAX(0, chunk_count + ?), + updated_at = ? + WHERE notebook_id = ? + """, + (source_delta, chunk_delta, now.isoformat(), notebook_id), + ) + if cursor.rowcount == 0: + return None + return self.get_notebook(notebook_id) + + def rename_notebook(self, notebook_id: str, title: str) -> NotebookMetadata | None: + """Update the title and bump updated_at. Returns the updated row, or + None if the notebook doesn't exist.""" + now = datetime.now(timezone.utc) + with self._connect() as conn: + cursor = conn.execute( + "UPDATE notebooks SET title = ?, updated_at = ? WHERE notebook_id = ?", + (title, now.isoformat(), notebook_id), + ) + if cursor.rowcount == 0: + return None + return self.get_notebook(notebook_id) + def list_jobs(self) -> list[IngestionJobStatus]: with self._connect() as conn: rows = conn.execute( diff --git a/backend/notebooklm_backend/services/vector_store.py b/backend/notebooklm_backend/services/vector_store.py index 7588c3f..fca6c6b 100644 --- a/backend/notebooklm_backend/services/vector_store.py +++ b/backend/notebooklm_backend/services/vector_store.py @@ -38,6 +38,34 @@ def delete_notebook_collections(self, notebook_id: str) -> None: # Collection may not exist yet for never-ingested notebooks. pass + def delete_document(self, notebook_id: str, source_path: str) -> int: + """Remove all chunks and the summary for a single document. Returns + the number of chunks removed (0 if the document wasn't indexed).""" + removed = 0 + try: + collection = self.get_collection(notebook_id) + existing = collection.get(where={"source_path": source_path}) + ids = existing.get("ids") or [] + if ids: + collection.delete(ids=ids) + removed = len(ids) + except Exception: + pass + + # Summaries live in a sibling collection; also best-effort. + try: + summaries_collection = self.client.get_collection( + name=self._doc_summaries_collection_name(notebook_id) + ) + existing = summaries_collection.get(where={"source_path": source_path}) + ids = existing.get("ids") or [] + if ids: + summaries_collection.delete(ids=ids) + except Exception: + pass + + return removed + def add_chunks(self, notebook_id: str, chunks: Iterable[TextChunk]) -> int: chunk_list = list(chunks) if not chunk_list: diff --git a/backend/tests/test_notebook_store.py b/backend/tests/test_notebook_store.py index ab52b47..9770c6e 100644 --- a/backend/tests/test_notebook_store.py +++ b/backend/tests/test_notebook_store.py @@ -53,3 +53,54 @@ def test_complete_ingestion_accumulates_counts_across_jobs(tmp_path): assert nb is not None assert nb.source_count == 5 # 3 + 2 assert nb.chunk_count == 19 # 12 + 7 + + +def test_rename_notebook_updates_title_and_returns_row(tmp_path): + settings = AppConfig( + workspace_root=tmp_path, + data_dir=tmp_path / "data", + models_dir=tmp_path / "models", + index_dir=tmp_path / "indexes", + cache_dir=tmp_path / "cache", + ) + store = NotebookStore(settings) + job = store.start_ingestion(NotebookIngestionRequest(path="x.md", title="Old name")) + + updated = store.rename_notebook(job.notebook_id, "New name") + assert updated is not None + assert updated.title == "New name" + + # And the row in list_notebooks reflects the change + notebooks = store.list_notebooks() + assert notebooks[0].title == "New name" + + +def test_rename_notebook_returns_none_when_missing(tmp_path): + settings = AppConfig( + workspace_root=tmp_path, + data_dir=tmp_path / "data", + models_dir=tmp_path / "models", + index_dir=tmp_path / "indexes", + cache_dir=tmp_path / "cache", + ) + store = NotebookStore(settings) + assert store.rename_notebook("not-a-real-id", "Whatever") is None + + +def test_adjust_counts_clamps_to_zero(tmp_path): + """Removing more chunks than exist shouldn't produce negative counts.""" + settings = AppConfig( + workspace_root=tmp_path, + data_dir=tmp_path / "data", + models_dir=tmp_path / "models", + index_dir=tmp_path / "indexes", + cache_dir=tmp_path / "cache", + ) + store = NotebookStore(settings) + job = store.start_ingestion(NotebookIngestionRequest(path="a.md", title="NB")) + store.complete_ingestion(job.job_id, "done", source_count=1, chunk_count=5) + + updated = store.adjust_counts(job.notebook_id, source_delta=-10, chunk_delta=-100) + assert updated is not None + assert updated.source_count == 0 + assert updated.chunk_count == 0