From 947f939f84c606678156ec7e61eebca3153172f4 Mon Sep 17 00:00:00 2001 From: Leo310 Date: Tue, 13 Oct 2026 16:57:13 +0200 Subject: [PATCH 1/2] fix(indexing): stop re-embedding Excalidraw drawings on every autosave (#541) A drawing was read in full (scene JSON included) on every Excalidraw autosave by both indexers, and embedded as a title-only vector: the back-of-note extraction kept the frontmatter and Excalidraw's "switch to view" banner, so a blank drawing never looked empty. New files were also embedded immediately on create, with no debounce. - Strip frontmatter and the banner from the back of the note; bare `.excalidraw` scenes are non-embeddable (still findable by name). - Notes with no extracted text are skipped instead of embedded title-only, and the verdict is remembered per mtime (`emptyNotes`, like `failedNotes`) so validation does not re-read them on every launch. - The vector create handler goes through the modify debounce. - Drawings use a 30 s debounce in both indexers, so a drawing session costs one re-read once the user stops. - Fix the stale "10s" debounce comment (it is 5 s). Co-Authored-By: Claude --- .../modal/IndexingReportModal.svelte | 2 + src/search/LexicalSearchService.ts | 22 ++- src/stores/dataStore.svelte.ts | 8 + src/types/plugin.ts | 6 + src/utils/fileFiltering.ts | 60 +++++-- src/vectorstore/VectorStoreService.ts | 147 ++++++++++++------ src/vectorstore/types.ts | 2 +- .../LexicalSearchService.validate.test.ts | 21 +++ test/utils/fileFiltering.test.ts | 70 +++++++++ .../VectorStoreService.lazyOpen.test.ts | 87 ++++++++++- 10 files changed, 352 insertions(+), 73 deletions(-) diff --git a/src/components/modal/IndexingReportModal.svelte b/src/components/modal/IndexingReportModal.svelte index b26c8080..5f71fa7a 100644 --- a/src/components/modal/IndexingReportModal.svelte +++ b/src/components/modal/IndexingReportModal.svelte @@ -21,6 +21,7 @@ const skipReasonLabels: Record = { "not-indexed": "Not yet indexed", "read-error": "Failed to read file", "embed-error": "Embedding failed", + empty: "No text to embed", }; const skipReasonIcons: Record = { @@ -29,6 +30,7 @@ const skipReasonIcons: Record = { "not-indexed": "clock", "read-error": "alert-triangle", "embed-error": "zap-off", + empty: "file-x", }; const skippedByReason = $derived.by(() => { diff --git a/src/search/LexicalSearchService.ts b/src/search/LexicalSearchService.ts index 71856156..85196d11 100644 --- a/src/search/LexicalSearchService.ts +++ b/src/search/LexicalSearchService.ts @@ -20,7 +20,13 @@ import { } from "../search/lexicalScoring"; import { Logger } from "../utils/logging"; import { StartupProfiler } from "../utils/startupProfiler"; -import { getIndexableVaultFiles, isIndexableFile, readIndexableContent } from "../utils/fileFiltering"; +import { + DRAWING_MODIFY_DEBOUNCE_MS, + getIndexableVaultFiles, + isExcalidrawPath, + isIndexableFile, + readIndexableContent, +} from "../utils/fileFiltering"; import { MiniSearchService, type AutocompleteCacheSnapshot, @@ -36,7 +42,8 @@ const MAX_SNIPPET_LENGTH = 180; * user types, and every save re-read, re-tokenized and re-added the note; * the embedding path already waits (5 s, a provider round trip per save). The * lexical index is cheap to update, so it waits less — an edit is searchable - * a couple of seconds after the last keystroke. + * a couple of seconds after the last keystroke. Drawings wait longer + * (`DRAWING_MODIFY_DEBOUNCE_MS`). */ const MODIFY_DEBOUNCE_MS = 2_000; @@ -429,10 +436,13 @@ export class LexicalSearchService { this.cancelPendingModify(file.path); this.modifyTimers.set( file.path, - window.setTimeout(() => { - this.modifyTimers.delete(file.path); - void this.reindexModifiedFile(file); - }, MODIFY_DEBOUNCE_MS), + window.setTimeout( + () => { + this.modifyTimers.delete(file.path); + void this.reindexModifiedFile(file); + }, + isExcalidrawPath(file.path) ? DRAWING_MODIFY_DEBOUNCE_MS : MODIFY_DEBOUNCE_MS, + ), ); } diff --git a/src/stores/dataStore.svelte.ts b/src/stores/dataStore.svelte.ts index a8a19737..9b60011e 100644 --- a/src/stores/dataStore.svelte.ts +++ b/src/stores/dataStore.svelte.ts @@ -1449,6 +1449,7 @@ export class PluginDataStore { documentCount?: number; dimensions?: number; failedNotes?: Record; + emptyNotes?: Record; }, ): void { const config = this.#data.embeddingIndexes.find((i) => i.id === indexId); @@ -1478,6 +1479,13 @@ export class PluginDataStore { changed = true; } } + if (stats.emptyNotes !== undefined) { + const next = Object.keys(stats.emptyNotes).length > 0 ? stats.emptyNotes : undefined; + if (next !== undefined || config.emptyNotes !== undefined) { + config.emptyNotes = next; + changed = true; + } + } if (changed) void this.saveSettings(); } diff --git a/src/types/plugin.ts b/src/types/plugin.ts index 107b5ad2..345ee44d 100644 --- a/src/types/plugin.ts +++ b/src/types/plugin.ts @@ -79,6 +79,12 @@ export interface EmbeddingIndexConfig { * that mtime changes; absent when nothing failed. */ failedNotes?: Record; + /** + * Notes with no text to embed (a blank note, a drawing with nothing on the back + * of the note), keyed by path with the mtime they were read at. Same lifecycle + * as `failedNotes`; absent when there are none. + */ + emptyNotes?: Record; } // ============================================================================ diff --git a/src/utils/fileFiltering.ts b/src/utils/fileFiltering.ts index 06df23c6..5db30078 100644 --- a/src/utils/fileFiltering.ts +++ b/src/utils/fileFiltering.ts @@ -130,9 +130,13 @@ const BINARY_TEXT_EXTENSIONS = new Set(["pdf"]); * - images: no text to extract. Embedding them would require a multimodal * embedding model and a separate image pipeline; until that exists they are * pure noise rather than a missing feature. + * - `excalidraw`: a bare Excalidraw scene — JSON shapes, no prose. (The + * `.excalidraw.md` form is a markdown note and is decided by its content: + * see {@link readIndexableContent}.) */ const NON_EMBEDDABLE_EXTENSIONS = new Set([ "base", + "excalidraw", "png", "jpg", "jpeg", @@ -225,6 +229,24 @@ export function isEmbeddableFile(file: TFile): boolean { return !NON_EMBEDDABLE_EXTENSIONS.has(extension); } +/** Returns `true` for an Excalidraw drawing, in either its `.excalidraw.md` or bare `.excalidraw` form. */ +export function isExcalidrawPath(path: string): boolean { + const lower = path.toLowerCase(); + return lower.endsWith(".excalidraw.md") || lower.endsWith(".excalidraw"); +} + +/** + * Debounce before re-indexing a modified Excalidraw drawing. + * + * Excalidraw autosaves every few seconds while the user draws, and each save + * rewrites the whole file — the scene JSON included. The indexers' normal + * debounce (a couple of seconds) fell between those saves, so every one of them + * re-read the full file on the main thread mid-stroke (#541). What a drawing + * contributes to search — its title and the text on the back of the note — rarely + * changes while drawing, so it is picked up once the drawing has been left alone. + */ +export const DRAWING_MODIFY_DEBOUNCE_MS = 30_000; + /** * Enumerate all vault files eligible for indexing (replaces * `vault.getMarkdownFiles()` in search / vectorstore callers). @@ -245,8 +267,9 @@ export function getEmbeddableVaultFiles(vault: Vault): TFile[] { * - `.chat` files: extracts only human/AI message content (not LangChain metadata) * - `.pdf` files: extracts the document text via pdfjs, so a PDF is chunked and * embedded like any other long document rather than reduced to its filename - * - `.excalidraw.md` files: extracts the "back of the note" and text elements, - * stripping the huge embedded JSON drawing data + * - `.excalidraw.md` files: extracts only the user-written "back of the note", + * stripping the frontmatter, Excalidraw's banner and the huge drawing data. + * A drawing nobody wrote anything beside comes back empty. * * The caller is responsible for filtering via `isIndexableFile` * before calling this helper. @@ -662,22 +685,29 @@ function extractMessageContent(msg: unknown): string { */ const RE_EXCALIDRAW_DATA_START = /^#+ (?:Excalidraw Data|Text Elements|Drawing)\s*$/m; +/** The drawing's frontmatter (`excalidraw-plugin: parsed`, tags): plugin bookkeeping. */ +const RE_LEADING_FRONTMATTER = /^---\r?\n[\s\S]*?\r?\n---[ \t]*(?:\r?\n|$)/; + +/** + * The "==⚠ Switch to EXCALIDRAW VIEW … ⚠==" warning Excalidraw writes into every + * drawing for readers who open it as markdown — the same boilerplate in each file. + */ +const RE_EXCALIDRAW_BANNER = /^==⚠.*$/gm; + /** * Extract the user-authored "back of the note" from an `.excalidraw.md` file. - * Strips the frontmatter-tagged Excalidraw sections and the huge JSON drawing. + * + * Strips the frontmatter, the banner and everything from the drawing data on. + * Both of the former used to survive, so a blank drawing looked like a note with + * text and was embedded as boilerplate plus its title — one more vector at the + * similarity noise floor per drawing, re-embedded on every autosave (#541). + * Exported for tests. */ -function extractExcalidrawContent(raw: string): string { +export function extractExcalidrawContent(raw: string): string { const match = RE_EXCALIDRAW_DATA_START.exec(raw); - if (!match) { - // No Excalidraw markers found — treat the whole file as content - return raw; - } - - // Everything before the first Excalidraw section header - let content = raw.substring(0, match.index); - + // Without the data section it is not a drawing in the format we know; keep it all. + let content = match ? raw.substring(0, match.index) : raw; + content = content.replace(RE_LEADING_FRONTMATTER, "").replace(RE_EXCALIDRAW_BANNER, ""); // Strip Obsidian comment markers that Excalidraw uses as section fences - content = content.replace(/%%\s*$/g, "").trim(); - - return content; + return content.replace(/%%\s*$/g, "").trim(); } diff --git a/src/vectorstore/VectorStoreService.ts b/src/vectorstore/VectorStoreService.ts index b95af74d..f3867b22 100644 --- a/src/vectorstore/VectorStoreService.ts +++ b/src/vectorstore/VectorStoreService.ts @@ -27,7 +27,13 @@ import { import { ensureProviderRegistered } from "../providers/registrySync"; import { getData } from "../stores/dataStore.svelte"; import { chunkText } from "../utils/chunkText"; -import { getEmbeddableVaultFiles, isEmbeddableFile, readIndexableContent } from "../utils/fileFiltering"; +import { + DRAWING_MODIFY_DEBOUNCE_MS, + getEmbeddableVaultFiles, + isEmbeddableFile, + isExcalidrawPath, + readIndexableContent, +} from "../utils/fileFiltering"; import { Logger } from "../utils/logging"; import { matchesPathPrefix } from "../utils/pathUtils"; import { isConnectionRefusedError, isDocumentRejectionError, isProviderUnreachableError } from "../lib/transportErrors"; @@ -263,6 +269,16 @@ interface IndexInstance { * `clearStore` drops it, so a rebuild tries everything again. */ failedNotes: Map; + /** + * Notes with no text to embed, keyed by path with the mtime they were read + * at. Embedding one would store a title-only vector — noise-floor similarity + * against any query — so it is left out, and remembered like `failedNotes` + * so validation does not re-read it on every launch. That matters for + * Excalidraw drawings: most never get text beside the drawing, and reading + * one means reading its whole scene (#541). Mirrored in + * `EmbeddingIndexConfig.emptyNotes`. + */ + emptyNotes: Map; progress: IndexingProgress; /** Timestamp (ms) when the current indexing run started, for ETA estimation */ indexingStartedAt: number | null; @@ -509,6 +525,7 @@ export class VectorStoreService { buildPromise: null, activeBulkRun: null, failedNotes: new Map(Object.entries(getData().getEmbeddingIndex(indexId)?.failedNotes ?? {})), + emptyNotes: new Map(Object.entries(getData().getEmbeddingIndex(indexId)?.emptyNotes ?? {})), progress: { isIndexing: false, total: 0, @@ -962,9 +979,9 @@ export class VectorStoreService { vaultPaths.add(file.path); const indexed = indexedMap.get(file.path); if (!indexed) { - // A note the provider rejected at this very mtime is not retried; - // an edit (any mtime change) puts it back in play. - if (inst.failedNotes.get(file.path) === file.stat.mtime) { + // A note the provider rejected, or found empty, at this very mtime + // is not retried; an edit (any mtime change) puts it back in play. + if (this.rememberedSkip(inst, file)) { failedSkipped++; continue; } @@ -986,9 +1003,7 @@ export class VectorStoreService { // dead weight; drop them here rather than carrying them for good. this.pruneFailedNotes(inst, vaultFiles); if (failedSkipped > 0) { - Logger.log( - `[VectorStore] ${inst.indexId}: leaving ${failedSkipped} note(s) the provider rejected unchanged`, - ); + Logger.log(`[VectorStore] ${inst.indexId}: leaving ${failedSkipped} rejected or empty note(s) unchanged`); } const totalUpdates = missingFiles.length + staleFiles.length + orphanedPaths.length; @@ -1102,10 +1117,15 @@ export class VectorStoreService { private registerEvents(): void { const { vault } = this.plugin.app; + // A new file is debounced like an edit. It is usually created empty and + // written to right away — a new note while the user types its first + // line, a new drawing as Excalidraw saves its first strokes — and + // embedding it on the spot cost a provider round trip for content that + // was stale within seconds (#541). this.plugin.registerEvent( - vault.on("create", async (file) => { + vault.on("create", (file) => { if (file instanceof TFile && isEmbeddableFile(file)) { - await this.handleFileCreate(file); + this.handleFileModify(file); } }), ); @@ -1178,20 +1198,9 @@ export class VectorStoreService { } /** - * Handle new file creation — forward to active instances only. - * Inactive instances are marked for re-validation on next use. - */ - private async handleFileCreate(file: TFile): Promise { - for (const inst of this.instances.values()) { - if (!this.canApplyEvent(inst)) continue; - await this.indexDocumentForInstance(inst, file); - void this.notifyStatsChanged(inst); - } - } - - /** - * Handle file modification — debounce re-embedding until 10s of inactivity. - * Inactive instances are marked for re-validation on next use. + * Handle file creation or modification — debounce re-embedding until + * `MODIFY_DEBOUNCE_MS` of inactivity (`DRAWING_MODIFY_DEBOUNCE_MS` for a + * drawing). Inactive instances are marked for re-validation on next use. */ private handleFileModify(file: TFile): void { const existing = this.modifyTimers.get(file.path); @@ -1199,10 +1208,13 @@ export class VectorStoreService { this.modifyTimers.set( file.path, - window.setTimeout(() => { - this.modifyTimers.delete(file.path); - void this.reindexModifiedFile(file); - }, MODIFY_DEBOUNCE_MS), + window.setTimeout( + () => { + this.modifyTimers.delete(file.path); + void this.reindexModifiedFile(file); + }, + isExcalidrawPath(file.path) ? DRAWING_MODIFY_DEBOUNCE_MS : MODIFY_DEBOUNCE_MS, + ), ); } @@ -1538,17 +1550,15 @@ export class VectorStoreService { const { vault } = this.plugin.app; const allFiles = getEmbeddableVaultFiles(vault); - // Categorize files by skip reason. A note the provider rejected at this - // very mtime is skipped here as it is by validation: a store emptied by - // such rejections takes this path on every launch, and would retry them. + // Categorize files by skip reason. A note the provider rejected, or found + // empty, at this very mtime is skipped here as it is by validation: a store + // emptied by such notes takes this path on every launch, and would retry them. const files: TFile[] = []; const skippedFiles: SkippedFile[] = []; for (const file of allFiles) { - const reason = this.getFileSkipReason(file, model.provider); + const reason = this.getFileSkipReason(file, model.provider) ?? this.rememberedSkip(inst, file); if (reason) { skippedFiles.push({ path: file.path, reason }); - } else if (inst.failedNotes.get(file.path) === file.stat.mtime) { - skippedFiles.push({ path: file.path, reason: "embed-error" }); } else { files.push(file); } @@ -1912,6 +1922,11 @@ export class VectorStoreService { try { const mtime = stampForRead(file); const content = await readIndexableContent(vault, file); + if (!content.trim()) { + noteSkipped(file.path, "empty"); + await this.markNoteEmpty(inst, file.path, mtime); + continue; + } const chunks = chunkText(content, file.basename, maxContentLength); pendingNotes.set(file.path, { mtime, expected: chunks.length, vectors: new Map() }); for (const chunk of chunks) { @@ -2054,6 +2069,10 @@ export class VectorStoreService { let chunks: ReturnType; try { const content = await readIndexableContent(this.plugin.app.vault, file); + if (!content.trim()) { + await this.markNoteEmpty(inst, file.path, mtime); + return; + } const maxContentLength = await this.getMaxEmbeddingContentLength(inst, model); chunks = chunkText(content, file.basename, maxContentLength); } catch (error) { @@ -2126,32 +2145,64 @@ export class VectorStoreService { } } + /** + * The note has no text to embed at this mtime. Record it, and drop what the + * store holds for the path — an earlier version that did have text must not + * keep the note searchable by content it no longer has. + */ + private async markNoteEmpty(inst: IndexInstance, path: string, mtime: number): Promise { + inst.failedNotes.delete(path); + if (inst.emptyNotes.get(path) !== mtime) { + inst.emptyNotes.set(path, mtime); + this.persistFailedNotes(inst); + } + try { + await inst.store.remove(path); + } catch (error) { + Logger.warn(`[VectorStore] Could not drop the stored rows of empty note ${path}:`, error); + } + } + + /** Why a not-yet-indexed note is left out at its current mtime, if it was rejected or found empty. */ + private rememberedSkip(inst: IndexInstance, file: TFile): SkipReason | null { + if (inst.failedNotes.get(file.path) === file.stat.mtime) return "embed-error"; + if (inst.emptyNotes.get(file.path) === file.stat.mtime) return "empty"; + return null; + } + private recordNoteFailure(inst: IndexInstance, path: string, mtime: number): void { if (inst.failedNotes.get(path) === mtime) return; inst.failedNotes.set(path, mtime); + inst.emptyNotes.delete(path); this.persistFailedNotes(inst); } private clearNoteFailure(inst: IndexInstance, path: string): void { - if (!inst.failedNotes.delete(path)) return; - this.persistFailedNotes(inst); + const hadFailure = inst.failedNotes.delete(path); + const wasEmpty = inst.emptyNotes.delete(path); + if (hadFailure || wasEmpty) this.persistFailedNotes(inst); } - /** Forget failures of notes that are gone from the vault or have changed since. */ + /** Forget failed and empty verdicts of notes that are gone from the vault or have changed since. */ private pruneFailedNotes(inst: IndexInstance, vaultFiles: TFile[]): void { - if (inst.failedNotes.size === 0) return; + if (inst.failedNotes.size === 0 && inst.emptyNotes.size === 0) return; const current = new Map(vaultFiles.map((file) => [file.path, file.stat.mtime])); let changed = false; - for (const [path, mtime] of inst.failedNotes) { - if (current.get(path) === mtime) continue; - inst.failedNotes.delete(path); - changed = true; + for (const verdicts of [inst.failedNotes, inst.emptyNotes]) { + for (const [path, mtime] of verdicts) { + if (current.get(path) === mtime) continue; + verdicts.delete(path); + changed = true; + } } if (changed) this.persistFailedNotes(inst); } private persistFailedNotes(inst: IndexInstance): void { - getData().updateEmbeddingIndexStats(inst.indexId, { failedNotes: Object.fromEntries(inst.failedNotes) }); + getData().updateEmbeddingIndexStats(inst.indexId, { + failedNotes: Object.fromEntries(inst.failedNotes), + emptyNotes: Object.fromEntries(inst.emptyNotes), + }); } /** @@ -2429,11 +2480,9 @@ export class VectorStoreService { for (const file of allFiles) { if (indexedPaths.has(file.path)) continue; - const reason = this.getFileSkipReason(file, provider); + const reason = this.getFileSkipReason(file, provider) ?? this.rememberedSkip(inst, file); if (reason) { skippedFiles.push({ path: file.path, reason }); - } else if (inst.failedNotes.get(file.path) === file.stat.mtime) { - skippedFiles.push({ path: file.path, reason: "embed-error" }); } else { // File passed filters but isn't indexed yet skippedFiles.push({ path: file.path, reason: "not-indexed" }); @@ -2577,7 +2626,13 @@ export class VectorStoreService { private async clearStore(inst: IndexInstance): Promise { await inst.store.clear(); inst.failedNotes.clear(); - getData().updateEmbeddingIndexStats(inst.indexId, { lastBuiltAt: null, documentCount: 0, failedNotes: {} }); + inst.emptyNotes.clear(); + getData().updateEmbeddingIndexStats(inst.indexId, { + lastBuiltAt: null, + documentCount: 0, + failedNotes: {}, + emptyNotes: {}, + }); } /** diff --git a/src/vectorstore/types.ts b/src/vectorstore/types.ts index b29466e2..782d95eb 100644 --- a/src/vectorstore/types.ts +++ b/src/vectorstore/types.ts @@ -202,7 +202,7 @@ export interface IndexingProgress { } /** Reasons a file can be skipped during indexing */ -export type SkipReason = "excluded" | "privacy" | "not-indexed" | "read-error" | "embed-error"; +export type SkipReason = "excluded" | "privacy" | "not-indexed" | "read-error" | "embed-error" | "empty"; /** A file that was skipped during indexing, with its reason */ export interface SkippedFile { diff --git a/test/search/LexicalSearchService.validate.test.ts b/test/search/LexicalSearchService.validate.test.ts index d483a2f4..f6f9b8f9 100644 --- a/test/search/LexicalSearchService.validate.test.ts +++ b/test/search/LexicalSearchService.validate.test.ts @@ -63,6 +63,8 @@ vi.mock("../../src/vectorstore/MiniSearchService", () => ({ })); vi.mock("../../src/stores/dataStore.svelte", () => ({ getData: () => ({ vaultSlug: "vault-1" }) })); vi.mock("../../src/utils/fileFiltering", () => ({ + DRAWING_MODIFY_DEBOUNCE_MS: 30_000, + isExcalidrawPath: (path: string) => path.endsWith(".excalidraw.md"), getIndexableVaultFiles: () => vaultFiles, isIndexableFile: () => true, isBinaryTextFile: () => false, @@ -219,6 +221,25 @@ describe("LexicalSearchService.validateIndex", () => { expect(addDocument.mock.calls.map(([path]) => path)).toEqual(["b.md"]); }); + it("waits for a drawing to be left alone before re-reading it, not for each autosave (#541)", async () => { + loaded = new Map([["Sketch.excalidraw.md", 1_000]]); + vaultFiles = [file("Sketch.excalidraw.md", 1_000)]; + service = LexicalSearchService.startInitialize(fakePlugin()); + expect(await waitForLexicalSearch()).toBe(true); + await vi.advanceTimersByTimeAsync(1_000); + addDocument.mockClear(); + const hooks = service as unknown as { handleFileModify(file: FakeFile): void }; + + for (let mtime = 2_000; mtime <= 6_000; mtime += 1_000) { + vaultFiles[0].stat.mtime = mtime; + hooks.handleFileModify(vaultFiles[0]); + await vi.advanceTimersByTimeAsync(5_000); + } + expect(addDocument).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(25_000); + expect(addDocument).toHaveBeenCalledTimes(1); + }); + it("leaves an index that matches the vault alone", async () => { loaded = new Map([["a.md", 1_000]]); vaultFiles = [file("a.md", 1_000)]; diff --git a/test/utils/fileFiltering.test.ts b/test/utils/fileFiltering.test.ts index 81618cfa..504a1afe 100644 --- a/test/utils/fileFiltering.test.ts +++ b/test/utils/fileFiltering.test.ts @@ -10,9 +10,11 @@ import { installAgentPathSource } from "../../src/utils/agentPathSource"; installAgentPathSource({ agentFolder: () => "Agents", agentName: () => undefined }); import { + extractExcalidrawContent, isAgentFilePath, isAgentPath, isEmbeddableFile, + isExcalidrawPath, isIndexableFile, readIndexableContent, } from "../../src/utils/fileFiltering"; @@ -458,3 +460,71 @@ describe("rename into a non-embeddable extension", () => { expect(isIndexableFile(after)).toBe(true); }); }); + +/** An `.excalidraw.md` file as the Excalidraw plugin writes it, with `back` as the back of the note. */ +function drawing(back = ""): string { + return [ + "---", + "", + "excalidraw-plugin: parsed", + "tags: [excalidraw]", + "", + "---", + "==⚠ Switch to EXCALIDRAW VIEW in the MORE OPTIONS menu of this document. ⚠== You can decompress Drawing data with the command palette: 'Decompress current Excalidraw file'. For more info check in plugin settings under 'Saving'", + "", + back, + "", + "# Excalidraw Data", + "", + "## Text Elements", + "Label on a box ^aBcD1234", + "", + "%%", + "## Drawing", + "```compressed-json", + "N4KAkARALgngDgUwgLgAQQQDwMYEMA2AlgCYBOuA7hADTgQBuCpAzoQPYB2KqATLZMzYBXUtiRoIACyhQ4zZAHoFAc0JRJQgEYA6bGwC2", + "```", + "%%", + ].join("\n"); +} + +describe("Excalidraw drawings (#541)", () => { + const f = (path: string, extension: string) => ({ path, extension }) as never; + + it("recognises both drawing forms by path", () => { + expect(isExcalidrawPath("Drawings/Sketch.excalidraw.md")).toBe(true); + expect(isExcalidrawPath("Drawings/Sketch.Excalidraw.MD")).toBe(true); + expect(isExcalidrawPath("Drawings/Sketch.excalidraw")).toBe(true); + expect(isExcalidrawPath("Notes/excalidraw.md")).toBe(false); + expect(isExcalidrawPath("Notes/On Excalidraw.md")).toBe(false); + }); + + it("never embeds a bare .excalidraw scene, but keeps it findable by name", () => { + installAgentPathSource({ agentFolder: () => "Agents", agentName: () => undefined }); + expect(isEmbeddableFile(f("Drawings/Sketch.excalidraw", "excalidraw"))).toBe(false); + expect(isIndexableFile(f("Drawings/Sketch.excalidraw", "excalidraw"))).toBe(true); + }); + + it("reduces a blank drawing to nothing — no frontmatter, banner, text elements or scene", () => { + // The frontmatter and banner used to survive, so a blank drawing was embedded + // as boilerplate plus its title on every autosave. + expect(extractExcalidrawContent(drawing())).toBe(""); + }); + + it("keeps only what the user wrote on the back of the note", () => { + const out = extractExcalidrawContent(drawing("Architecture sketch for the [[Sync engine]] rewrite.")); + expect(out).toBe("Architecture sketch for the [[Sync engine]] rewrite."); + }); + + it("handles the older format whose data section starts at # Text Elements, behind a %% fence", () => { + const raw = + "---\nexcalidraw-plugin: parsed\n---\nMy notes\n%%\n# Text Elements\nfoo ^x\n# Drawing\n```json\n{}\n```\n%%"; + expect(extractExcalidrawContent(raw)).toBe("My notes"); + }); + + it("is what readIndexableContent returns for an .excalidraw.md file", async () => { + const vault = { cachedRead: vi.fn(async () => drawing()) } as never; + const file = { path: "Drawings/Sketch.excalidraw.md", extension: "md", stat: { size: 10 } } as never; + expect(await readIndexableContent(vault, file)).toBe(""); + }); +}); diff --git a/test/vectorstore/VectorStoreService.lazyOpen.test.ts b/test/vectorstore/VectorStoreService.lazyOpen.test.ts index bbd450ec..6287de51 100644 --- a/test/vectorstore/VectorStoreService.lazyOpen.test.ts +++ b/test/vectorstore/VectorStoreService.lazyOpen.test.ts @@ -127,6 +127,8 @@ const stores = new Map(); let vaultFiles: FakeFile[] = []; /** Paths whose `readIndexableContent` throws. */ let readFailures = new Set(); +/** Paths whose extracted content is empty (a blank note, a drawing with nothing beside it). */ +let emptyPaths = new Set(); /** Content served for `long.md`, when a test needs a multi-chunk note. */ let longContent = ""; const embedDocuments = vi.fn(async (texts: string[]) => texts.map(() => [1, 0, 0])); @@ -159,6 +161,7 @@ const fakeData = { // Mirror the stats the service wrote, as the real config would. lastBuiltAt: (indexStats.lastBuiltAt as number | undefined) ?? null, failedNotes: indexStats.failedNotes as Record | undefined, + emptyNotes: indexStats.emptyNotes as Record | undefined, }; }, onProviderRenamed: (listener: (oldId: string, newId: string) => Promise | void) => { @@ -210,11 +213,14 @@ vi.mock("../../src/lib/modelMetadataNormalizer", () => ({ hydrateEmbeddingModel: () => ({ maxInputTokens: 8191 }), })); vi.mock("../../src/utils/fileFiltering", () => ({ + DRAWING_MODIFY_DEBOUNCE_MS: 30_000, + isExcalidrawPath: (path: string) => path.endsWith(".excalidraw.md"), getEmbeddableVaultFiles: () => vaultFiles, isEmbeddableFile: () => true, isBinaryTextFile: (f: FakeFile) => f.extension === "pdf", readIndexableContent: async (_vault: unknown, f: FakeFile) => { if (readFailures.has(f.path)) throw new Error(`cannot read ${f.path}`); + if (emptyPaths.has(f.path)) return ""; if (f.path === "long.md" && longContent) return longContent; return `content of ${f.path}`; }, @@ -265,6 +271,7 @@ beforeEach(() => { stores.clear(); vaultFiles = []; readFailures = new Set(); + emptyPaths = new Set(); longContent = ""; indexIds = [INDEX]; fakeData.searchEmbedIndex = INDEX; @@ -657,7 +664,8 @@ describe("bulk embed run", () => { /** The service's private per-file hooks, reached directly: the fake vault registers no events. */ type EventHooks = { handleFileModify(file: FakeFile): void; - handleFileCreate(file: FakeFile): Promise; + /** What a create or modify event runs once its debounce elapses. */ + reindexModifiedFile(file: FakeFile): Promise; handleFileRename(file: FakeFile, oldPath: string): Promise; }; @@ -943,7 +951,7 @@ describe("indexing review follow-ups", () => { // Same on the incremental path. const created = file("c.md"); vaultFiles.push(created); - await (svc as unknown as EventHooks).handleFileCreate(created); + await (svc as unknown as EventHooks).reindexModifiedFile(created); expect(indexStats.failedNotes).toBeUndefined(); // The key is fixed: the next launch indexes every note. @@ -962,7 +970,7 @@ describe("indexing review follow-ups", () => { const unreadable = file("a.md"); vaultFiles.push(unreadable); readFailures.add("a.md"); - await (svc as unknown as EventHooks).handleFileCreate(unreadable); + await (svc as unknown as EventHooks).reindexModifiedFile(unreadable); expect(embedDocuments).not.toHaveBeenCalled(); expect(indexStats.failedNotes).toBeUndefined(); @@ -974,7 +982,7 @@ describe("indexing review follow-ups", () => { store.putNote = async () => { throw new Error("QuotaExceededError"); }; - await (svc as unknown as EventHooks).handleFileCreate(unreadable); + await (svc as unknown as EventHooks).reindexModifiedFile(unreadable); expect(embedDocuments).toHaveBeenCalledTimes(1); expect(indexStats.failedNotes).toBeUndefined(); store.putNote = putNote; @@ -1068,7 +1076,7 @@ describe("indexing review follow-ups", () => { await vi.advanceTimersByTimeAsync(1_000); // nothing to validate const created = file("a.md"); vaultFiles.push(created); - await (svc as unknown as EventHooks).handleFileCreate(created); + await (svc as unknown as EventHooks).reindexModifiedFile(created); expect(embedQuery).not.toHaveBeenCalled(); expect(embedDocuments).toHaveBeenCalledTimes(1); expect(stores.get(INDEX)?.docs.has("a.md#0")).toBe(true); @@ -1215,3 +1223,72 @@ describe("provider rename", () => { expect(oldStore?.docs.size).toBe(0); }); }); + +describe("notes with no text to embed (#541)", () => { + it("are skipped instead of embedded title-only, and not re-read on later launches until they change", async () => { + platform.isMobile = false; + vaultFiles = [file("ok.md", 1_000), file("Sketch.excalidraw.md", 1_000)]; + emptyPaths = new Set(["Sketch.excalidraw.md"]); + const svc = await startService(); + await vi.advanceTimersByTimeAsync(1_000); + + const store = stores.get(INDEX); + expect([...(store?.docs.keys() ?? [])]).toEqual(["ok.md#0"]); + expect(embedDocuments.mock.calls.flat(2).some((text) => String(text).includes("Sketch"))).toBe(false); + expect(indexStats.emptyNotes).toEqual({ "Sketch.excalidraw.md": 1_000 }); + expect(indexStats.failedNotes).toEqual({}); + expect((await svc.getReport(INDEX))?.skippedFiles).toEqual([{ path: "Sketch.excalidraw.md", reason: "empty" }]); + + // Next launch: still absent from the store, but not read or embedded again. + const attempts = embedDocuments.mock.calls.length; + await svc.cleanup(); + service = null; + const relaunched = await startService(); + await vi.advanceTimersByTimeAsync(1_000); + expect(embedDocuments).toHaveBeenCalledTimes(attempts); + expect((await relaunched.getReport(INDEX))?.skippedFiles).toEqual([ + { path: "Sketch.excalidraw.md", reason: "empty" }, + ]); + + // The user writes on the back of the note: it is embedded and the verdict goes. + emptyPaths = new Set(); + vaultFiles[1].stat.mtime = 2_000; + await (relaunched as unknown as EventHooks).reindexModifiedFile(vaultFiles[1]); + expect(stores.get(INDEX)?.docs.get("Sketch.excalidraw.md#0")?.mtime).toBe(2_000); + expect(indexStats.emptyNotes).toEqual({}); + }); + + it("an indexed note that is emptied drops its stale vectors", async () => { + platform.isMobile = false; + vaultFiles = [file("a.md", 1_000)]; + const svc = await startService(); + await vi.advanceTimersByTimeAsync(1_000); + expect(stores.get(INDEX)?.docs.has("a.md#0")).toBe(true); + + emptyPaths = new Set(["a.md"]); + vaultFiles[0].stat.mtime = 2_000; + await (svc as unknown as EventHooks).reindexModifiedFile(vaultFiles[0]); + expect(stores.get(INDEX)?.docs.has("a.md#0")).toBe(false); + expect(indexStats.emptyNotes).toEqual({ "a.md": 2_000 }); + }); + + it("a drawing being drawn is re-read once it is left alone, not on every autosave", async () => { + platform.isMobile = false; + vaultFiles = [file("Sketch.excalidraw.md", 1_000)]; + const svc = await startService(); + await vi.advanceTimersByTimeAsync(1_000); + embedDocuments.mockClear(); + const hooks = svc as unknown as EventHooks; + + // Excalidraw autosaves every few seconds while the user draws. + for (let mtime = 2_000; mtime <= 6_000; mtime += 1_000) { + vaultFiles[0].stat.mtime = mtime; + hooks.handleFileModify(vaultFiles[0]); + await vi.advanceTimersByTimeAsync(5_000); + } + expect(embedDocuments).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(25_000); + expect(embedDocuments).toHaveBeenCalledTimes(1); + expect(stores.get(INDEX)?.docs.get("Sketch.excalidraw.md#0")?.mtime).toBe(6_000); + }); +}); From 88805d50e11ad8e19f433f768bbff3591795ded9 Mon Sep 17 00:00:00 2001 From: Leo310 Date: Tue, 13 Oct 2026 17:09:38 +0200 Subject: [PATCH 2/2] fix(indexing): keep drawing aliases, judge emptiness only on plain-text reads, batch empty-note saves Review follow-ups on #549: - Drawings keep their frontmatter in the extracted text (lexical search reads aliases from it); hasNoEmbeddableText looks past it instead. - An empty extraction from a PDF/.chat or a mobile size-gated file may be a failed read, so it is never remembered as empty. - Empty verdicts found by a bulk run are saved at checkpoints, not per note. Co-Authored-By: Claude --- src/utils/fileFiltering.ts | 41 +++++++++++----- src/vectorstore/VectorStoreService.ts | 28 ++++++++--- test/utils/fileFiltering.test.ts | 48 ++++++++++++++----- .../VectorStoreService.lazyOpen.test.ts | 1 + 4 files changed, 86 insertions(+), 32 deletions(-) diff --git a/src/utils/fileFiltering.ts b/src/utils/fileFiltering.ts index 5db30078..fd2c1462 100644 --- a/src/utils/fileFiltering.ts +++ b/src/utils/fileFiltering.ts @@ -267,9 +267,8 @@ export function getEmbeddableVaultFiles(vault: Vault): TFile[] { * - `.chat` files: extracts only human/AI message content (not LangChain metadata) * - `.pdf` files: extracts the document text via pdfjs, so a PDF is chunked and * embedded like any other long document rather than reduced to its filename - * - `.excalidraw.md` files: extracts only the user-written "back of the note", - * stripping the frontmatter, Excalidraw's banner and the huge drawing data. - * A drawing nobody wrote anything beside comes back empty. + * - `.excalidraw.md` files: extracts the frontmatter and the user-written "back + * of the note", stripping Excalidraw's banner and the huge drawing data. * * The caller is responsible for filtering via `isIndexableFile` * before calling this helper. @@ -346,6 +345,26 @@ async function readIndexableContentRaw(vault: Vault, file: TFile): Promise MOBILE_MAX_READ_BYTES) return false; + const text = isExcalidrawPath(file.path) ? content.replace(RE_LEADING_FRONTMATTER, "") : content; + return text.trim() === ""; +} + /** * Read a text file's raw content, resilient to Obsidian mobile's `cachedRead`. * @@ -685,7 +704,7 @@ function extractMessageContent(msg: unknown): string { */ const RE_EXCALIDRAW_DATA_START = /^#+ (?:Excalidraw Data|Text Elements|Drawing)\s*$/m; -/** The drawing's frontmatter (`excalidraw-plugin: parsed`, tags): plugin bookkeeping. */ +/** A leading frontmatter block. */ const RE_LEADING_FRONTMATTER = /^---\r?\n[\s\S]*?\r?\n---[ \t]*(?:\r?\n|$)/; /** @@ -695,19 +714,15 @@ const RE_LEADING_FRONTMATTER = /^---\r?\n[\s\S]*?\r?\n---[ \t]*(?:\r?\n|$)/; const RE_EXCALIDRAW_BANNER = /^==⚠.*$/gm; /** - * Extract the user-authored "back of the note" from an `.excalidraw.md` file. - * - * Strips the frontmatter, the banner and everything from the drawing data on. - * Both of the former used to survive, so a blank drawing looked like a note with - * text and was embedded as boilerplate plus its title — one more vector at the - * similarity noise floor per drawing, re-embedded on every autosave (#541). - * Exported for tests. + * Extract the frontmatter and the user-authored "back of the note" from an + * `.excalidraw.md` file, stripping the banner and everything from the drawing + * data on. The frontmatter stays for lexical search, which reads `aliases` from + * it; {@link hasNoEmbeddableText} looks past it. Exported for tests. */ export function extractExcalidrawContent(raw: string): string { const match = RE_EXCALIDRAW_DATA_START.exec(raw); // Without the data section it is not a drawing in the format we know; keep it all. - let content = match ? raw.substring(0, match.index) : raw; - content = content.replace(RE_LEADING_FRONTMATTER, "").replace(RE_EXCALIDRAW_BANNER, ""); + const content = (match ? raw.substring(0, match.index) : raw).replace(RE_EXCALIDRAW_BANNER, ""); // Strip Obsidian comment markers that Excalidraw uses as section fences return content.replace(/%%\s*$/g, "").trim(); } diff --git a/src/vectorstore/VectorStoreService.ts b/src/vectorstore/VectorStoreService.ts index f3867b22..6362b844 100644 --- a/src/vectorstore/VectorStoreService.ts +++ b/src/vectorstore/VectorStoreService.ts @@ -30,6 +30,7 @@ import { chunkText } from "../utils/chunkText"; import { DRAWING_MODIFY_DEBOUNCE_MS, getEmbeddableVaultFiles, + hasNoEmbeddableText, isEmbeddableFile, isExcalidrawPath, readIndexableContent, @@ -1887,11 +1888,20 @@ export class VectorStoreService { } }; + // Empty-note verdicts found by this run, not yet in the plugin settings. + let emptiesUnsaved = false; + const saveEmpties = () => { + if (!emptiesUnsaved) return; + emptiesUnsaved = false; + this.persistFailedNotes(inst); + }; + /** Pacing after a batch: a checkpoint every interval of notes, a GC pause otherwise. */ const afterBatch = async (): Promise => { refreshNotice(); if (notesSinceCheckpoint >= BULK_CHECKPOINT_INTERVAL) { notesSinceCheckpoint = 0; + saveEmpties(); await inst.store.flush(); // The notes just flushed are searchable now, so the count the // settings row shows must say so — not only once the run ends, @@ -1922,9 +1932,12 @@ export class VectorStoreService { try { const mtime = stampForRead(file); const content = await readIndexableContent(vault, file); - if (!content.trim()) { + if (hasNoEmbeddableText(file, content)) { noteSkipped(file.path, "empty"); - await this.markNoteEmpty(inst, file.path, mtime); + // Saved at the next checkpoint: a vault of blank drawings would + // otherwise rewrite the plugin settings once per drawing. + await this.markNoteEmpty(inst, file.path, mtime, false); + emptiesUnsaved = true; continue; } const chunks = chunkText(content, file.basename, maxContentLength); @@ -1965,6 +1978,7 @@ export class VectorStoreService { // Notes whose chunks were not all embedded (a stopped run) were never // written; the next validation finds them missing. pendingNotes.clear(); + saveEmpties(); const cancelled = aborted(); if (stopped && !cancelled) { @@ -2069,7 +2083,7 @@ export class VectorStoreService { let chunks: ReturnType; try { const content = await readIndexableContent(this.plugin.app.vault, file); - if (!content.trim()) { + if (hasNoEmbeddableText(file, content)) { await this.markNoteEmpty(inst, file.path, mtime); return; } @@ -2150,11 +2164,11 @@ export class VectorStoreService { * store holds for the path — an earlier version that did have text must not * keep the note searchable by content it no longer has. */ - private async markNoteEmpty(inst: IndexInstance, path: string, mtime: number): Promise { - inst.failedNotes.delete(path); - if (inst.emptyNotes.get(path) !== mtime) { + private async markNoteEmpty(inst: IndexInstance, path: string, mtime: number, persist = true): Promise { + const hadFailure = inst.failedNotes.delete(path); + if (inst.emptyNotes.get(path) !== mtime || hadFailure) { inst.emptyNotes.set(path, mtime); - this.persistFailedNotes(inst); + if (persist) this.persistFailedNotes(inst); } try { await inst.store.remove(path); diff --git a/test/utils/fileFiltering.test.ts b/test/utils/fileFiltering.test.ts index 504a1afe..0fadd749 100644 --- a/test/utils/fileFiltering.test.ts +++ b/test/utils/fileFiltering.test.ts @@ -11,6 +11,7 @@ installAgentPathSource({ agentFolder: () => "Agents", agentName: () => undefined import { extractExcalidrawContent, + hasNoEmbeddableText, isAgentFilePath, isAgentPath, isEmbeddableFile, @@ -505,26 +506,49 @@ describe("Excalidraw drawings (#541)", () => { expect(isIndexableFile(f("Drawings/Sketch.excalidraw", "excalidraw"))).toBe(true); }); - it("reduces a blank drawing to nothing — no frontmatter, banner, text elements or scene", () => { - // The frontmatter and banner used to survive, so a blank drawing was embedded - // as boilerplate plus its title on every autosave. - expect(extractExcalidrawContent(drawing())).toBe(""); - }); + const sketch = { path: "Drawings/Sketch.excalidraw.md", extension: "md", stat: { size: 10 } } as never; - it("keeps only what the user wrote on the back of the note", () => { + it("drops the banner, text elements and scene, keeping frontmatter and the back of the note", () => { const out = extractExcalidrawContent(drawing("Architecture sketch for the [[Sync engine]] rewrite.")); - expect(out).toBe("Architecture sketch for the [[Sync engine]] rewrite."); + expect(out).not.toContain("Switch to EXCALIDRAW VIEW"); + expect(out).not.toContain("Label on a box"); + expect(out).not.toContain("N4KAk"); + // Lexical search reads `aliases` from the frontmatter. + expect(out).toContain("excalidraw-plugin: parsed"); + expect(out.endsWith("Architecture sketch for the [[Sync engine]] rewrite.")).toBe(true); + }); + + it("reads a blank drawing as having no text to embed — its frontmatter is bookkeeping", async () => { + // The frontmatter and banner used to count as text, so a blank drawing was + // embedded as boilerplate plus its title on every autosave. + const vault = { cachedRead: vi.fn(async () => drawing()) } as never; + expect(hasNoEmbeddableText(sketch, await readIndexableContent(vault, sketch))).toBe(true); + expect(hasNoEmbeddableText(sketch, extractExcalidrawContent(drawing("A note")))).toBe(false); }); it("handles the older format whose data section starts at # Text Elements, behind a %% fence", () => { const raw = "---\nexcalidraw-plugin: parsed\n---\nMy notes\n%%\n# Text Elements\nfoo ^x\n# Drawing\n```json\n{}\n```\n%%"; - expect(extractExcalidrawContent(raw)).toBe("My notes"); + expect(extractExcalidrawContent(raw)).toBe("---\nexcalidraw-plugin: parsed\n---\nMy notes"); }); +}); - it("is what readIndexableContent returns for an .excalidraw.md file", async () => { - const vault = { cachedRead: vi.fn(async () => drawing()) } as never; - const file = { path: "Drawings/Sketch.excalidraw.md", extension: "md", stat: { size: 10 } } as never; - expect(await readIndexableContent(vault, file)).toBe(""); +describe("hasNoEmbeddableText", () => { + const f = (path: string, extension: string) => ({ path, extension, stat: { size: 10 } }) as never; + + it("is true for a blank note", () => { + expect(hasNoEmbeddableText(f("Notes/Untitled.md", "md"), " \n")).toBe(true); + expect(hasNoEmbeddableText(f("Notes/Idea.md", "md"), "an idea")).toBe(false); + }); + + it("counts a plain note's frontmatter as text: a task note keeps everything there", () => { + expect(hasNoEmbeddableText(f("Tasks/Pay rent.md", "md"), "---\nstatus: open\ndue: 2026-11-01\n---\n")).toBe( + false, + ); + }); + + it("never takes an empty extraction from a container as a verdict: it may be a failed read", () => { + expect(hasNoEmbeddableText(f("Papers/scan.pdf", "pdf"), "")).toBe(false); + expect(hasNoEmbeddableText(f("Chats/Thread.chat", "chat"), "")).toBe(false); }); }); diff --git a/test/vectorstore/VectorStoreService.lazyOpen.test.ts b/test/vectorstore/VectorStoreService.lazyOpen.test.ts index 6287de51..e672f205 100644 --- a/test/vectorstore/VectorStoreService.lazyOpen.test.ts +++ b/test/vectorstore/VectorStoreService.lazyOpen.test.ts @@ -216,6 +216,7 @@ vi.mock("../../src/utils/fileFiltering", () => ({ DRAWING_MODIFY_DEBOUNCE_MS: 30_000, isExcalidrawPath: (path: string) => path.endsWith(".excalidraw.md"), getEmbeddableVaultFiles: () => vaultFiles, + hasNoEmbeddableText: (_f: FakeFile, content: string) => content.trim() === "", isEmbeddableFile: () => true, isBinaryTextFile: (f: FakeFile) => f.extension === "pdf", readIndexableContent: async (_vault: unknown, f: FakeFile) => {