Skip to content

fix(index): clear the legacy embeddings table on reset and deindex - #688

Merged
plind-junior merged 3 commits into
vouchdev:testfrom
joaovictor91123:fix/index-legacy-embeddings-cleanup
Jul 31, 2026
Merged

fix(index): clear the legacy embeddings table on reset and deindex#688
plind-junior merged 3 commits into
vouchdev:testfrom
joaovictor91123:fix/index-legacy-embeddings-cleanup

Conversation

@joaovictor91123

Copy link
Copy Markdown
Contributor

What changed

index_db.reset() and index_db.deindex() now clear the legacy
embeddings table alongside embedding_index, matching what both
functions' own docstrings already claim to do.

Why

reset()'s docstring: "Leaving stale rows here means semantic search can
return orphaned hits after a reindex." deindex()'s docstring: removes
"the embedding row for any kind." Neither function's executescript/
execute calls actually touched the legacy embeddings table — only
embedding_index. Confirmed with a repro: indexing a row via
index_embedding(), then calling deindex() (or reset()), leaves the
row in embeddings untouched.

search_embeddings (plural, the function that reads the legacy table)
currently has zero callers anywhere in src/ — a pre-existing comment in
tests/test_index_db_embeddings.py already notes this — so this isn't
live search-result pollution today. The concrete, verified-live impact is
narrower: health._check_orphan_embeddings explicitly checks both the
legacy embeddings table and embedding_index, so a leaked row
permanently trips the orphan_embedding fsck warning with no way to
clear it via the documented remedy (reindexing), and state.db grows
unbounded with dead rows over a KB's lifetime. (I initially overstated the
live-search angle in the linked issue and posted a correction there.)

Fixes #687

What might break

Nothing for users with an existing .vouch/ directory — no on-disk
shape, kb.* method, or object model change; this is purely
state.db-internal cleanup (a derived/rebuildable index, never
committed). Behaviorally: vouch index (reset) and artifact deletion now
actually remove the row from both embedding tables instead of one.

VEP

Not applicable — no object model, kb.* method, on-disk layout, bundle
format, or audit-log shape change. state.db is a derived, rebuildable
cache, not part of the on-disk artifact format.

Prior art

This exact defect and fix were previously submitted as #543
(fix(index): clear the legacy embeddings table on reset and deindex)
but closed unmerged on 2026-07-29 purely for going stale against test
(a CHANGELOG.md conflict) — the maintainer's closing comment explicitly
said this wasn't a judgment on the change. Re-verified independently
against current test HEAD: the gap is still live in both functions.

Tests

  • Local make check-equivalent: ruff clean (src + tests); mypy
    clean on index_db.py; all 36 tests/test_index_db_embeddings.py
    cases pass (34 pre-existing + 2 new); tests/test_delete.py and
    tests/test_health.py (both exercise deindex/orphan_embedding)
    pass with no regressions
  • New / changed behaviour has a test —
    test_deindex_removes_the_legacy_embeddings_row,
    test_reset_clears_the_legacy_embeddings_table
  • CHANGELOG.md updated under ## [Unreleased]

reset()'s docstring warns "leaving stale rows here means semantic
search can return orphaned hits after a reindex" and deindex()'s
docstring promises to remove "the embedding row for any kind," but
neither ever touched the legacy embeddings table alongside
embedding_index. every artifact deletion or full reindex leaked a
stale vector row that never gets cleaned up.

search_embeddings (plural, the reader of the legacy table) currently
has no callers in src/, so this isn't live search pollution today -
but health._check_orphan_embeddings explicitly checks both tables, so
a leaked row permanently trips the orphan_embedding fsck warning with
no way to clear it via the documented remedy (reindexing), and grows
state.db unbounded over a kb's lifetime.

add the missing DELETE FROM embeddings to reset()'s executescript, and
the missing per-row delete to deindex(), matching the existing
embedding_index handling in both.

this exact defect and fix were previously submitted as vouchdev#543
(fix(index): clear the legacy embeddings table on reset and deindex),
but that PR was closed unmerged for going stale against a fast-moving
test branch, not for anything wrong with the change; the maintainer's
closing comment explicitly invited a fresh PR.

Fixes vouchdev#687
@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance retrieval context, search, synthesis, and evaluation tests tests and fixtures size: XS less than 50 changed non-doc lines labels Jul 30, 2026
…beddings-cleanup

# Conflicts:
#	CHANGELOG.md
@plind-junior

Copy link
Copy Markdown
Member

clean, and the honesty about scope is the right call — the correction you posted on the issue narrowing this from "live search pollution" to "a permanently stuck orphan_embedding fsck warning plus unbounded state.db growth" makes the change easier to review, not weaker. search_embeddings having no callers in src/ is exactly the sort of thing a reviewer would otherwise spend twenty minutes rediscovering.

checked the upgrade path, since a bare DELETE FROM embeddings inside reset()'s executescript would raise OperationalError on any state.db predating the table: index_db.py:37 creates it with CREATE TABLE IF NOT EXISTS embeddings, run on open, so the table always exists by the time either function runs. no migration concern.

placing the deindex delete outside the if/elif chain is correct and matches the embedding_index delete directly below it — both embedding tables are keyed (kind, id) and neither is one of the per-kind FTS tables, so gating them on kind would be the bug.

the two tests assert through search_embeddings rather than poking the table, which is the right level: they would still pass if the storage detail changed, and they fail for the reason stated.

no objections.

@plind-junior
plind-junior merged commit b32cd23 into vouchdev:test Jul 31, 2026
8 of 11 checks passed
@github-actions github-actions Bot added the ci: failing ci is red label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: failing ci is red docs documentation, specs, examples, and repo guidance retrieval context, search, synthesis, and evaluation size: XS less than 50 changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(index): reset() and deindex() never clear the legacy embeddings table, leaking stale vectors forever

2 participants