diff --git a/src/renderer/src/components/notebook/SourcePanel.tsx b/src/renderer/src/components/notebook/SourcePanel.tsx index 5df7a18..cbcfba5 100644 --- a/src/renderer/src/components/notebook/SourcePanel.tsx +++ b/src/renderer/src/components/notebook/SourcePanel.tsx @@ -42,6 +42,9 @@ import { requestAppendExcerpt } from './note/appendExcerptCommand' // 添加来源类型 type AddSourceType = 'file' | 'url' | 'text' | 'note' +/** “/home/me/notes.pdf” → “notes.pdf”。渲染进程没有 node 的 `path`。 */ +const fileName = (filePath: string): string => filePath.split(/[\\/]/).pop() || filePath + // 添加来源弹窗组件 interface AddSourceModalProps { type: AddSourceType @@ -451,6 +454,22 @@ export default function SourcePanel(): ReactElement { [notebookId, currentNote, createNote, t] ) + /** + * 导入失败必须说出来（#146）。 + * + * store 把失败当 **返回值**交回来（`{ success: false, error }`），不是抛异常；只要调用点 + * 不检查它，失败就和成功长得一模一样：面板保持空状态，用户以为上传成功了。 + * + * `reason` 用于代替主进程的原因（空笔记有专门的文案）。 + */ + const reportImportFailure = useCallback( + (name: string, result: { success: boolean; error?: string }, reason?: string): void => { + if (result.success) return + toast.error(t('importFailed', { name, error: reason ?? result.error ?? t('unknownError') })) + }, + [t] + ) + // 处理文件上传 const handleFileUpload = useCallback(async () => { if (!notebookId) return @@ -463,10 +482,13 @@ export default function SourcePanel(): ReactElement { const files = await selectFiles() for (const filePath of files) { - await addDocumentFromFile(notebookId, filePath) + // 一个文件失败不能影响其余文件，但也不能被吞掉： + // 吞掉它，用户看到的就是“什么都没发生” + const result = await addDocumentFromFile(notebookId, filePath) + reportImportFailure(fileName(filePath), result) } setShowAddMenu(false) - }, [notebookId, hasEmbeddingModel, selectFiles, addDocumentFromFile, t]) + }, [notebookId, hasEmbeddingModel, selectFiles, addDocumentFromFile, reportImportFailure, t]) // 处理 URL 导入 const handleUrlImport = useCallback( @@ -480,10 +502,11 @@ export default function SourcePanel(): ReactElement { return } - await addDocumentFromUrl(notebookId, data.url) + const result = await addDocumentFromUrl(notebookId, data.url) + reportImportFailure(data.url, result) setModalType(null) }, - [notebookId, hasEmbeddingModel, addDocumentFromUrl, t] + [notebookId, hasEmbeddingModel, addDocumentFromUrl, reportImportFailure, t] ) // 处理文本粘贴 @@ -498,14 +521,15 @@ export default function SourcePanel(): ReactElement { return } - await addDocument(notebookId, { + const result = await addDocument(notebookId, { title: data.title, type: 'text', content: data.content }) + reportImportFailure(data.title, result) setModalType(null) }, - [notebookId, hasEmbeddingModel, addDocument, t] + [notebookId, hasEmbeddingModel, addDocument, reportImportFailure, t] ) // 处理笔记导入 @@ -520,21 +544,15 @@ export default function SourcePanel(): ReactElement { return } - try { - await addNoteToKnowledge(notebookId, data.noteId) - setModalType(null) - } catch (error) { - // 检查是否是空笔记错误 - const errorMessage = (error as Error).message || '' - if (errorMessage.toLowerCase().includes('empty')) { - alert(t('emptyNoteCannotImport')) - } else { - alert(errorMessage || t('embeddingFailed', { title: '' })) - } - setModalType(null) - } + const noteTitle = notes.find((note) => note.id === data.noteId)?.title ?? data.noteId + const result = await addNoteToKnowledge(notebookId, data.noteId) + // 空笔记是唯一需要换文案的原因，其余由主进程给出 + const isEmpty = (result.error ?? '').toLowerCase().includes('empty') + reportImportFailure(noteTitle, result, isEmpty ? t('emptyNoteCannotImport') : undefined) + + setModalType(null) }, - [notebookId, hasEmbeddingModel, addNoteToKnowledge, t] + [notebookId, hasEmbeddingModel, addNoteToKnowledge, notes, reportImportFailure, t] ) // 处理删除文档 diff --git a/src/renderer/src/locales/en-US/ui.json b/src/renderer/src/locales/en-US/ui.json index a3a19a0..4c42cd5 100644 --- a/src/renderer/src/locales/en-US/ui.json +++ b/src/renderer/src/locales/en-US/ui.json @@ -33,6 +33,8 @@ "resizeNotes": "Resize the notes panel. Use the arrow keys, Shift for a larger step, Home or End for the limits.", "addSource": "Add Source", "uploadFile": "Upload File", + "importFailed": "Import failed for {{name}}: {{error}}", + "unknownError": "unknown reason", "pasteText": "Paste Text", "importUrl": "Import URL", "importNote": "Import Note", diff --git a/src/renderer/src/locales/zh-CN/ui.json b/src/renderer/src/locales/zh-CN/ui.json index 475a927..bce5b38 100644 --- a/src/renderer/src/locales/zh-CN/ui.json +++ b/src/renderer/src/locales/zh-CN/ui.json @@ -32,6 +32,8 @@ "resizeNotes": "调整笔记宽度。用方向键移动，按住 Shift 步长更大，Home / End 到两端。", "addSource": "添加来源", "uploadFile": "上传文件", + "importFailed": "「{{name}}」导入失败：{{error}}", + "unknownError": "未知原因", "pasteText": "粘贴文本", "importUrl": "导入网页", "importNote": "导入笔记", diff --git a/src/renderer/src/store/knowledgeStore.ts b/src/renderer/src/store/knowledgeStore.ts index 2b04552..883e8c8 100644 --- a/src/renderer/src/store/knowledgeStore.ts +++ b/src/renderer/src/store/knowledgeStore.ts @@ -67,6 +67,29 @@ interface KnowledgeStore { selectFiles: () => Promise } +/** + * 一次导入尝试结束之后（成功或失败）收尾。 + * + * 无论成败都要重新读列表：失败时这一行可能已经以 `status: 'failed'` 落库了，而只刷新成功 + * 的路径会让这次失败在界面上**完全不存在** —— 面板保持空状态，用户以为上传成功了（#146）。 + * 失败同时记进 `error`：抛异常和返回 `{success:false}` 都是失败，没道理只记前者。 + */ +async function refreshAfterImport( + notebookId: string, + result: { success: boolean; error?: string }, + set: (partial: Partial) => void, + get: () => KnowledgeStore +): Promise { + await get().loadDocuments(notebookId) + await get().loadStats(notebookId) + + set({ + isIndexing: false, + indexProgress: null, + error: result.success ? null : (result.error ?? null) + }) +} + export const useKnowledgeStore = create()((set, get) => ({ // 初始状态 documents: [], @@ -116,15 +139,12 @@ export const useKnowledgeStore = create()((set, get) => ({ set({ isIndexing: true, error: null }) try { const result = await window.api.knowledge.addDocument(notebookId, options) - if (result.success) { - await get().loadDocuments(notebookId) - await get().loadStats(notebookId) - } - set({ isIndexing: false, indexProgress: null }) + await refreshAfterImport(notebookId, result, set, get) return result } catch (error) { - set({ isIndexing: false, indexProgress: null, error: (error as Error).message }) - return { success: false, error: (error as Error).message } + const message = (error as Error).message + await refreshAfterImport(notebookId, { success: false, error: message }, set, get) + return { success: false, error: message } } }, @@ -133,15 +153,12 @@ export const useKnowledgeStore = create()((set, get) => ({ set({ isIndexing: true, error: null }) try { const result = await window.api.knowledge.addDocumentFromFile(notebookId, filePath) - if (result.success) { - await get().loadDocuments(notebookId) - await get().loadStats(notebookId) - } - set({ isIndexing: false, indexProgress: null }) + await refreshAfterImport(notebookId, result, set, get) return result } catch (error) { - set({ isIndexing: false, indexProgress: null, error: (error as Error).message }) - return { success: false, error: (error as Error).message } + const message = (error as Error).message + await refreshAfterImport(notebookId, { success: false, error: message }, set, get) + return { success: false, error: message } } }, @@ -150,15 +167,12 @@ export const useKnowledgeStore = create()((set, get) => ({ set({ isIndexing: true, error: null }) try { const result = await window.api.knowledge.addDocumentFromUrl(notebookId, url) - if (result.success) { - await get().loadDocuments(notebookId) - await get().loadStats(notebookId) - } - set({ isIndexing: false, indexProgress: null }) + await refreshAfterImport(notebookId, result, set, get) return result } catch (error) { - set({ isIndexing: false, indexProgress: null, error: (error as Error).message }) - return { success: false, error: (error as Error).message } + const message = (error as Error).message + await refreshAfterImport(notebookId, { success: false, error: message }, set, get) + return { success: false, error: message } } }, @@ -167,15 +181,12 @@ export const useKnowledgeStore = create()((set, get) => ({ set({ isIndexing: true, error: null }) try { const result = await window.api.knowledge.addNote(notebookId, noteId) - if (result.success) { - await get().loadDocuments(notebookId) - await get().loadStats(notebookId) - } - set({ isIndexing: false, indexProgress: null }) + await refreshAfterImport(notebookId, result, set, get) return result } catch (error) { - set({ isIndexing: false, indexProgress: null, error: (error as Error).message }) - return { success: false, error: (error as Error).message } + const message = (error as Error).message + await refreshAfterImport(notebookId, { success: false, error: message }, set, get) + return { success: false, error: message } } }, diff --git a/test/knowledgeStore.test.ts b/test/knowledgeStore.test.ts new file mode 100644 index 0000000..e9f8f5a --- /dev/null +++ b/test/knowledgeStore.test.ts @@ -0,0 +1,161 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { useKnowledgeStore } from '../src/renderer/src/store/knowledgeStore.ts' + +/** + * A failed import has to leave a trace (#146). + * + * Every entry point in `SourcePanel` discards the store's return value — the store + * reports failure by *returning* `{ success: false, error }` rather than throwing — + * and the store only reloaded the library when the import had succeeded. So a failed + * import produced nothing at all: no error, no row, and a panel that still showed its + * empty state, which reads as "the upload worked and the app is not showing it". + * + * The store's half of the fix is what these tests pin: after *any* attempt the + * library is re-read (a `status: 'failed'` row may already be persisted and is + * exactly the feedback the reader needs) and the reason is recorded. + * + * Importing renderer code means typechecking it, which is why `tsconfig.test.json` + * also includes `src/preload/index.d.ts` — that is where `window.api` is declared. + */ + +interface CallLog { + getDocuments: number + getStats: number +} + +interface ApiOptions { + /** What the library contains when re-read. */ + documents?: Array> + /** When set, every add resolves with `{ success: false, error }`. */ + failsWith?: string + /** When set, every add rejects, the way a failed IPC round trip does. */ + rejectsWith?: string +} + +function installKnowledgeApi(options: ApiOptions = {}): CallLog { + const log: CallLog = { getDocuments: 0, getStats: 0 } + const documents = options.documents ?? [] + + const add = async (): Promise<{ success: boolean; documentId?: string; error?: string }> => { + if (options.rejectsWith) throw new Error(options.rejectsWith) + if (options.failsWith) return { success: false, error: options.failsWith } + return { success: true, documentId: 'doc_new' } + } + + const knowledge = { + getDocuments: async (): Promise => { + log.getDocuments += 1 + return documents + }, + getStats: async (): Promise => { + log.getStats += 1 + return null + }, + addDocument: add, + addDocumentFromFile: add, + addDocumentFromUrl: add, + addNote: add + } + + ;(globalThis as unknown as { window: unknown }).window = { api: { knowledge } } + return log +} + +const resetStore = (): void => { + useKnowledgeStore.setState({ + documents: [], + stats: null, + error: null, + isIndexing: false, + isLoading: false, + documentsLoaded: false, + indexProgress: null + }) +} + +const state = (): ReturnType => useKnowledgeStore.getState() + +/** The four ways a source gets in, all of which used to discard their result. */ +const entryPoints: Array<{ + name: string + add: (notebookId: string) => Promise<{ success: boolean; error?: string }> +}> = [ + { + name: 'pasted text', + add: (notebookId) => + state().addDocument(notebookId, { title: 'notes', type: 'text', content: 'content' }) + }, + { + name: 'a file', + add: (notebookId) => state().addDocumentFromFile(notebookId, '/tmp/notes.exe') + }, + { + name: 'a URL', + add: (notebookId) => state().addDocumentFromUrl(notebookId, 'https://example.invalid') + }, + { name: 'a note', add: (notebookId) => state().addNoteToKnowledge(notebookId, 'note_1') } +] + +for (const entry of entryPoints) { + test(`${entry.name}: a failed import still re-reads the library and records the reason`, async () => { + resetStore() + + // The row a failure *after* the source row exists leaves behind: this is the + // feedback the reader needs, and it can only appear if the list is re-read. + const log = installKnowledgeApi({ + documents: [{ id: 'doc_1', title: 'broken.pdf', status: 'failed' }], + failsWith: 'Unsupported file type: exe' + }) + + const result = await entry.add('notebook_1') + + assert.equal(result.success, false, 'a failed import reported success') + assert.equal(state().error, 'Unsupported file type: exe', 'the reason was not recorded') + assert.equal( + log.getDocuments, + 1, + 'the library was not re-read, so the failed row stays invisible' + ) + assert.equal(log.getStats, 1, 'the stats were not re-read') + assert.equal(state().documents.length, 1, 'the failed row did not reach the list') + assert.equal(state().documents[0].status, 'failed', 'the row lost its status') + assert.equal(state().isIndexing, false, 'the panel is still showing the import as running') + }) +} + +test('a successful import re-reads the library and clears an earlier failure', async () => { + resetStore() + + installKnowledgeApi({ failsWith: 'No chunks generated from document' }) + await state().addDocumentFromFile('notebook_1', '/tmp/scanned.pdf') + assert.equal(state().error, 'No chunks generated from document', 'the failure was not recorded') + + const log = installKnowledgeApi({ documents: [{ id: 'doc_2', status: 'indexed' }] }) + const result = await state().addDocumentFromFile('notebook_1', '/tmp/notes.pdf') + + assert.equal(result.success, true, 'a successful import reported failure') + assert.equal(state().error, null, 'the previous failure was left behind') + assert.equal(log.getDocuments, 1, 'the library was not re-read') + assert.equal(state().documents.length, 1, 'the imported document is not in the list') +}) + +test('an import whose IPC call rejects is reported and still re-reads the library', async () => { + resetStore() + + // A rejection can happen after the main process has already written the source + // row (a reload, an error at the process boundary), so the failure path cannot + // assume that nothing changed. + const log = installKnowledgeApi({ + documents: [{ id: 'doc_3', status: 'failed' }], + rejectsWith: 'An object could not be cloned' + }) + + const result = await state().addDocumentFromFile('notebook_1', '/tmp/notes.pdf') + + assert.equal(result.success, false, 'a rejected import reported success') + assert.equal(state().error, 'An object could not be cloned', 'the rejection was not recorded') + assert.equal(log.getDocuments, 1, 'the library was not re-read after a rejection') + assert.equal(state().documents.length, 1, 'the list did not pick up what really happened') + assert.equal(state().isIndexing, false, 'the panel is still showing the import as running') +}) diff --git a/tsconfig.test.json b/tsconfig.test.json index 78344c0..2b7c2da 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -1,6 +1,6 @@ { "extends": "./tsconfig.node.json", - "include": ["test/**/*.ts"], + "include": ["test/**/*.ts", "src/preload/index.d.ts"], "compilerOptions": { "composite": false, "noEmit": true,