Skip to content

fix(search): discard obsolete embedding work and sweep orphan vectors - #644

Merged
aliasunder merged 13 commits into
mainfrom
fix/obsolete-embedding-writes
Oct 6, 2026
Merged

aliasunder merged 13 commits into
mainfrom
fix/obsolete-embedding-writes

Conversation

@aliasunder

@aliasunder aliasunder commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Pending embedding work can write old chunks or vectors after its source is deleted or replaced. A recreated note can then return its new title with an old snippet, and orphan memory vectors can permanently consume nearest-neighbor capacity.

Committed source versions now travel with queued and rebuild jobs. The index rejects obsolete work before model calls, after model awaits, and before derived writes or tail pruning.

Watcher event tokens reject delayed reads after deletion or a newer change. Watcher operations contain filesystem/index failures; missing-source reads are skipped at debug while real index/model failures remain errors. Rebuild attachment-stat failures are logged and skipped; Canvas links resolve as their content enters the index.

Startup removes vectors with no parent chunk or memory entry, retaining valid caches. Memory templates are created before the initial index scan, so their contents are searchable immediately in a fresh vault. Rebuild file reads are bounded. Attachment-link resolution skips impossible candidates while preserving SQL precedence. Canvas resolution reuses a private catalog with mutation and rollback invalidation. Independent content deletion still runs after metadata deletion fails. Source deletion stays immediate; per-path queue recovery and content-hash reuse remain intact. Embedding versions are an internal API change, with all caller fixtures migrated.

Validation:

  • Full suite: 4,716 passed, 2 skipped; build, lint (0 errors) and knip passed.
  • Controlled note, file, memory and watcher races assert intermediate state while replacements are held.
  • Memory-folder reads are bounded; both limit regressions fail when concurrency is increased.
  • Rebuild stat/read failures and watcher text/PDF recovery have controlled regression coverage.
  • Five caller regressions pin rebuild I/O limits and fail when those limits are bypassed.
  • Eleven regressions fail against the old index; six watcher mutations fail for the intended reasons.
  • Real sqlite-vec checks pin orphan cleanup, idempotence, valid cache retention and nearest-neighbor capacity recovery.

Live validation (deployed test image):

  • Test deployment succeeded on 5439fd09; the recreated container was verified healthy on that exact image revision before and after testing.
  • Public MCP checks passed for note creation, update, deletion and path recreation. A semantic query for the old marker returned only the recreated note’s current title and snippet.
  • Fourteen isolated checks against the deployed image passed with controlled model promises and real SQLite/sqlite-vec: obsolete note/file results, same-mtime recreation, deletion during a memory batch, orphan cleanup, retained vector bytes, nearest-neighbor recovery and repeat-sweep idempotence.
  • A fresh-vault server started from the image indexed all five newly created memory templates; an MCP SDK request over real HTTP found the Agents template immediately.
  • Read-only inspection found zero parentless note, file or memory vectors. Persisted current-boot logs confirmed rebuild, watcher startup and embedding completion.
  • All temporary notes, index rows and image fixtures were cleaned up. The instance remains on the tested image; no release or restore was performed.

🔍 ship-check · ship-check · gpt-6.1-sol

Comment thread src/vault-mcp/search/file-watcher.ts Outdated
@umm-actually

umm-actually Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

umm-actually re-reviewed at 5439fd0

1 new finding(s) posted (24 tracked finding(s) across all runs).

Context notes
  • Priority docs already in context: ARCHITECTURE.md, deploy/remote/README.md
  • Priority docs not included: deploy/railway/README.md, deploy/render/README.md (missing, unreadable, or over budget)

umm-actually · z-ai/glm-5.3-flash

@aliasunder
aliasunder force-pushed the fix/obsolete-embedding-writes branch from 767d4c4 to aada67f Compare October 6, 2026 01:01
Comment thread src/vault-mcp/search/file-watcher.ts
Comment thread src/vault-mcp/search/search-index.ts
Comment thread src/vault-mcp/search/search-index.ts Outdated
@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Index the memory templates the bootstrap creates before the watcher starts
Medium severity · correctness · high confidence

src/vault-mcp/server.ts:152 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: startup runs rebuildFromVault → bootstrapMemoryIfEnabled → startFileWatcher, and the watcher starts with ignoreInitial: true, so files the bootstrap creates (a fresh vault's About Me/ templates) — or anything else created between the rebuild's directory listing and the watcher start — emit no add events and are never upserted. vault_search and vault_memory_recall return nothing for those files until they are edited or the container restarts.

Failure scenario: A user boots the local image against a vault with no About Me/ folder: rebuildFromVault snapshots the vault before the templates exist, bootstrapMemoryIfEnabled then creates About Me/Me.md and the other templates, and startFileWatcher starts afterward with ignoreInitial: true — the templates produce no events, so searches for template content and memory recall over those files stay empty until a template is edited or the container restarts.

Suggested fix
Have bootstrapMemoryDir return the paths it created and upsert each into the index from server.ts (it holds both sides), or re-run the rebuild when the bootstrap created files.

umm-actually · z-ai/glm-5.3-flash

@aliasunder
aliasunder marked this pull request as ready for review October 6, 2026 01:41
@aliasunder

aliasunder commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner Author

Issue comment #6007450065: fixed in 66f0ed4d. Memory bootstrap now runs before the initial rebuild. A real HTTP/SQLite regression uses a custom memory folder, verifies that five template notes were actually created and indexed, searches unique template content, and verifies existing fixture content remains indexed.

Sweep: 20 bootstrap call/definition references repository-wide; one startup order corrected, four production references already correct, fifteen test references. Full suite: 4,513 passed, two skipped; build and lint pass.


🔍 ship-check · pr-monitor · gpt-6.1-sol

Comment thread ARCHITECTURE.md Outdated
@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Reorder the Restart startup-task list to match the new boot order
Low severity · conventions · high confidence

deploy/local/README.md:191 — beyond the diff's line ranges, in code the changes touch or depend on.

The Restart section still says the server rebuilds the search index before it creates the memory template files, but this PR moved the bootstrap ahead of the rebuild so a fresh vault's templates are indexed on first boot. The stale order misdescribes when those templates become searchable.

Failure scenario: A user with a vault that has no About Me/ folder reads the Restart section to check whether the auto-created templates are searchable immediately; the listed order (index rebuilt before templates exist) implies they are not, so the user edits a template or restarts the container to make it findable — a no-op step the reordered code no longer requires.

Suggested fix
State the current order, e.g.: "creates memory template files if the memory folder doesn't exist (skipped when `MEMORY_ENABLED=false` or `READONLY_MODE=true`), builds the search index including them, and starts the file watcher."

umm-actually · deepseek/deepseek-v4.1-flash

@aliasunder

aliasunder commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner Author

Issue comment #6007790578: fixed in 3c9bbbe4. Both local and remote restart guides now list memory template creation before indexing, then watcher startup. The memory/read-only opt-outs remain beside template creation, and the remote guide retains its preceding catch-up sync.

Sweep: eight startup/index/source-of-truth statements repository-wide. Both restart guides and the architecture invariant are corrected; the architecture boot sequence and Render/Railway descriptions already match. Markdown lint and whitespace checks pass; no executable code changed.


🔍 ship-check · pr-monitor · gpt-6.1-sol

Comment thread src/vault-mcp/search/search-index.ts Outdated
Comment thread src/vault-mcp/search/file-watcher.ts
Comment thread src/vault-mcp/search/search-index.ts
Comment thread src/vault-mcp/search/search-index.ts
@umm-actually

umm-actually Bot commented Oct 6, 2026

Copy link
Copy Markdown

Reconcile boot-window vault changes at watcher startup
Medium severity · correctness · medium confidence

src/vault-mcp/search/file-watcher.ts:295 — beyond the diff's line ranges, in code the changes touch or depend on.

Changes landing between the rebuild's directory snapshot and the watcher's first scan are never reconciled: a note or file deleted in that window keeps its FTS, link, and vector rows as a ghost search result, and one modified there keeps serving its pre-edit content, until the next container restart. ignoreInitial suppresses the add/unlink events that would otherwise correct both, and the continuous sync process is already running while the server rebuilds.

Failure scenario: A user deletes a note on their phone while the server is rebuilding its index at boot: the rebuild read and indexed the note, continuous sync deletes the file before the watcher's initial scan, chokidar emits no unlink (the deletion predates the watch and ignoreInitial suppresses the scan's add), and the deleted note stays searchable with its vectors and backlinks until the container is next restarted.

Suggested fix
On the watcher's ready event, reconcile once: for each path in the index (notes, non_md_files, file_content), remove its rows when the file no longer exists on disk, and upsert visible on-disk paths the index lacks — respecting the shared hidden-path predicate — so the boot window closes in both directions.

umm-actually · z-ai/glm-5.3-flash

@aliasunder

aliasunder commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner Author

Issue comment #6008042551: confirmed with real chokidar and SQLite. Changes made after rebuild but before watcher startup are missed: new/edited contents remain unavailable or stale, and deleted contents remain searchable even after a later watcher event succeeds.

This existing startup-handoff gap is separately tracked for a coordinated startup change. Moving the watcher earlier or rebuilding twice alone would leave competing source writes unresolved; this change rejects obsolete embedding jobs after indexed source updates and does not add a startup handoff mechanism.

Sweep: 70 startup/config references repository-wide; the current ordering and gap were verified. The other four findings from this review are fixed with 13 regressions in ac450e12; the combined suite passes 4,528 tests, with two skipped.


🔍 ship-check · pr-monitor · gpt-6.1-sol

Comment thread src/vault-mcp/search/__tests__/file-watcher.test.ts
Comment thread src/__tests__/integration/server-integration.test.ts
Comment thread src/vault-mcp/search/__tests__/file-watcher.test.ts
Comment thread src/vault-mcp/search/__tests__/file-watcher.test.ts
Comment thread src/vault-mcp/search/file-watcher.ts
Comment thread src/vault-mcp/search/file-watcher.ts
Comment thread src/vault-mcp/search/search-index.ts
@aliasunder
aliasunder force-pushed the fix/obsolete-embedding-writes branch from ac450e1 to d115e49 Compare October 6, 2026 03:18
Comment thread src/vault-mcp/search/file-watcher.ts
Comment thread src/vault-mcp/search/search-index.ts
@aliasunder
aliasunder force-pushed the fix/obsolete-embedding-writes branch from d115e49 to c2356ad Compare October 6, 2026 03:45
Comment thread src/vault-mcp/search/file-watcher.ts Outdated
Comment thread src/vault-mcp/search/search-index.ts Outdated
Ship-Check: pr-monitor · gpt-6.1-sol
@aliasunder
aliasunder force-pushed the fix/obsolete-embedding-writes branch from c2356ad to 5439fd0 Compare October 6, 2026 04:41
Comment thread ARCHITECTURE.md
@aliasunder

Copy link
Copy Markdown
Owner Author

Live validation (deployed test image):

  • Test deployment succeeded on 5439fd09; the recreated container was verified healthy on that exact image revision before and after testing.
  • Public MCP checks passed for note creation, update, deletion and path recreation. A semantic query for the old marker returned only the recreated note’s current title and snippet.
  • Fourteen isolated checks against the deployed image passed with controlled model promises and real SQLite/sqlite-vec: obsolete note/file results, same-mtime recreation, deletion during a memory batch, orphan cleanup, retained vector bytes, nearest-neighbor recovery and repeat-sweep idempotence.
  • A fresh-vault server started from the image indexed all five newly created memory templates; an MCP SDK request over real HTTP found the Agents template immediately.
  • Read-only inspection found zero parentless note, file or memory vectors. Persisted current-boot logs confirmed rebuild, watcher startup and embedding completion.
  • All temporary notes, index rows and image fixtures were cleaned up. The instance remains on the tested image; no release or restore was performed.

🔍 ship-check · pr-monitor · gpt-6.1-sol

@aliasunder
aliasunder merged commit 503ffbf into main Oct 6, 2026
19 checks passed
@aliasunder
aliasunder deleted the fix/obsolete-embedding-writes branch October 6, 2026 05:25
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.

1 participant