Skip to content

fix(indexing): stop re-embedding Excalidraw drawings on every autosave - #549

Open
Leo310 wants to merge 2 commits into
mainfrom
fix/541-excalidraw-indexing
Open

Leo310 wants to merge 2 commits into
mainfrom
fix/541-excalidraw-indexing

Conversation

@Leo310

@Leo310 Leo310 commented Oct 13, 2026

Copy link
Copy Markdown
Member

What

With S2B enabled, every Excalidraw autosave made both indexers re-read the whole drawing file (scene JSON included) on the main thread, and the vector index then embedded it as a title-only vector. The back-of-note extraction kept the frontmatter and Excalidraw's "Switch to EXCALIDRAW VIEW" banner, so a blank drawing never looked empty. New files were also embedded immediately on create, with no debounce. This is the likely cause of the iPad drawing breakage and the "couldn't embed" notice. S2B never writes to the drawing, so file locking was ruled out.

  • The back-of-note extraction now drops the frontmatter and the banner. Bare .excalidraw scenes are non-embeddable but still found by name in lexical search.
  • Notes with no extracted text are skipped instead of being embedded title-only. The verdict is remembered per mtime (emptyNotes, same lifecycle as failedNotes, shown as "No text to embed" in the indexing report), so validation doesn't re-read every drawing on each launch.
  • The vector create handler now goes through the modify debounce.
  • Drawings get a 30 s debounce in both the lexical and vector indexers, so a drawing session costs one re-read after the user stops.
  • Fixed the stale "10s" debounce comment (the value is 5 s).

Fixes #541

How I tested it

Unit tests: drawing extraction (blank, back-of-note text, older format), .excalidraw embeddability, the empty-note verdict surviving a relaunch without re-reading and clearing on edit, an emptied note dropping its stale vectors, and the 30 s drawing debounce on both indexers. bun run check, format, lint and test all pass. Not device-verified: Leo will test on iPad. A dev build is in slot wt2 (S2B WT2).

AI assistance: Claude Code (Opus) wrote the change and the tests from Leo's brief to address #541. Leo reviews the change and tests it on iPad before merging.

Checklist

  • bun run check, bun run format, bun run lint, and bun run test pass locally
  • I tried the change in a real Obsidian vault (or explained above why that isn't applicable): pending Leo's iPad test
  • I read CONTRIBUTING.md, including the section on AI assistance
  • If this adds a provider, a bundled skill, a built-in tool, or changes manifest.json: I noted that the docs site needs updating (n/a)

#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 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the remaining previous finding concerns refreshing unchanged drawing entries after upgrade.

Fix All in Claude CodeFindings

  1. P2 Old drawing entries remain ▶
Fix with agent prompt
### Issue 1
src/utils/fileFiltering.ts:undefined-710
Existing unchanged drawings never get the new extraction rules. Both indexes accept saved entries when their `mtime` matches, and this PR leaves their saved versions unchanged. Blank `.excalidraw.md` drawings already indexed with the banner and frontmatter keep those search entries and vectors until an edit or manual rebuild. Add a one-time refresh for affected drawings and an upgrade test with an existing index.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR reduces repeated indexing during Excalidraw autosaves.

  • Excalidraw search uses the note text, not its drawing boilerplate.
  • Empty notes stay out of vectors until their text changes.
  • Drawing indexes wait until autosaves have settled.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[File created or modified] --> B{Drawing?}
  B -->|Yes| C[Wait 30 seconds after last edit]
  B -->|No| D[Use normal edit delay]
  C --> E[Read searchable text]
  D --> E
  E --> F[Lexical index keeps aliases]
  E --> G{Successful text read with no text?}
  G -->|Yes| H[Remember empty verdict and remove old vectors]
  G -->|No| I[Use normal embedding path]
  H --> J[Bulk run saves verdicts together]
Loading

Reviews (2) · Last reviewed commit: "fix(indexing): keep drawing aliases, jud..." · Reviewed by Greptile

Comment thread src/utils/fileFiltering.ts Outdated
Comment thread src/vectorstore/VectorStoreService.ts Outdated
Comment thread src/utils/fileFiltering.ts Outdated

// 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, "");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Old drawing entries remain

Existing unchanged drawings never get the new extraction rules. Both indexes accept saved entries when their mtime matches, and this PR leaves their saved versions unchanged. Blank .excalidraw.md drawings already indexed with the banner and frontmatter keep those search entries and vectors until an edit or manual rebuild. Add a one-time refresh for affected drawings and an upgrade test with an existing index.

Knowledge Base Used: Vector indexing and storage

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/utils/fileFiltering.ts
Line: 710

Comment:
**Old drawing entries remain**

Existing unchanged drawings never get the new extraction rules. Both indexes accept saved entries when their `mtime` matches, and this PR leaves their saved versions unchanged. Blank `.excalidraw.md` drawings already indexed with the banner and frontmatter keep those search entries and vectors until an edit or manual rebuild. Add a one-time refresh for affected drawings and an upgrade test with an existing index.

**Knowledge Base Used:** [Vector indexing and storage](https://app.greptile.com/your-papa/-/custom-context/knowledge-base/s2b-dev/smart-second-brain/-/docs/vector-indexing-and-storage.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this. Issue #541 comes from the per-autosave work: the full re-read, the re-embed and the create-time embed. That is fixed whether or not old entries are refreshed. A drawing already indexed with the banner keeps that one stale vector until its next edit, when it is re-read under the new rules (and dropped if blank), or until the user rebuilds the index. A one-time refresh would need a persisted migration marker that reaches into both indexes, for a cosmetic noise-floor vector per untouched drawing. That isn't worth the extra machinery in this fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s a fair scope tradeoff. This PR fixes the repeated autosave re-read/re-embed path, while an untouched drawing’s legacy entry remains only until its next edit or an explicit rebuild. A persisted migration across both indexes would add substantial machinery for a noise-floor cleanup, so I’m withdrawing the finding.

Comment thread src/vectorstore/VectorStoreService.ts Outdated
…xt 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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: SSB's indexing / embedding file change watcher breaks Excalidraw plugin

1 participant