fix(index): recover an index an incompatible schema wrote - #169
Open
Hasnain2430 wants to merge 1 commit into
Open
Hasnain2430 wants to merge 1 commit into
Hasnain2430 wants to merge 1 commit into
Conversation
An index whose generations were written by an earlier index schema could
not be used or discarded. `clear`, reindexing, and `status` all read the
active snapshot first, and that read validates every generation manifest,
so each one failed with the same error:
IndexSchemaError: Manifest for generation ... is invalid.
index_schema_version
Input should be 7 [input_value=6]
Recovery meant editing repository files by hand or importing the media
into another repository.
Add a supported recovery path:
- `read_active_document` reads the active snapshot without validating the
generations it points at. The snapshot document is still checked for
integrity and identity; only the manifests are left unread, because a
reset is about to discard them.
- `describe_active_generations` reports what a reset would give up, so a
caller can tell the user before anything is destroyed.
- `discard_generations` publishes an empty snapshot whatever the
generations hold, leaving retained generation directories on disk as
`clear` does.
- `IndexStorage.drop_all_collections` deletes every collection in the
store. Publishing an empty snapshot is not enough on its own: a
collection keeps the embedding dimensions it was created with, and an
incompatible store can hold collections this configuration no longer
names.
- `vidxp index reset` exposes this, and the incompatible-schema error now
names that command instead of leaving the user without a next step.
Collections are dropped before the metadata is replaced. A failure part
way through then leaves the repository exactly as it was, so the command
can be retried and still report what it removed.
Imported media is never touched, so indexing it again restores search.
`status` was blocked as well, which the report did not mention, so an
affected repository could not even describe its own state.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
Closes #167.
Summary
An index whose generations were written by an earlier index schema could not be used or discarded.
clear, reindexing, andstatusall read the active snapshot first, and that read validates every generation manifest, so each one failed the same way. Recovery meant editing repository files by hand or importing the media into another repository.This adds a supported recovery path:
LocalSnapshotRepository.read_active_documentreads the active snapshot without validating the generations it points at. The snapshot document is still checked for integrity and identity; only the manifests are left unread, because a reset is about to discard them.describe_active_generationsreports what a reset would give up, so the user is told before anything is destroyed.discard_generationspublishes an empty snapshot whatever the generations hold, leaving retained generation directories on disk exactly ascleardoes.IndexStorage.drop_all_collectionsdeletes every collection in the store. As the issue notes, publishing an empty snapshot is not enough on its own: a collection keeps the embedding dimensions it was created with, and an incompatible store can hold collections the current configuration no longer names.vidxp index resetexposes this. The incompatible-schema error now names that command instead of leaving the user without a next step.Imported media is never touched, so indexing it again restores search.
Ordering matters here. Collections are dropped before the metadata is replaced. A failure part way through then leaves the repository exactly as it was, so the command can be retried and still report what it removed. I had this the other way round first, and a mid-way failure partially applied and lost its own report.
One addition to the report:
statusis blocked as well, which the issue did not mention. An affected repository could not even describe its own state, which is why the error message change is part of this rather than a follow-up.Decisions that are yours, not mine
ports.pygains aresetmethod onIndexBackend. CONTRIBUTING says ownership-boundary changes need maintainer review, so I am flagging it rather than burying it. It returnsdict[str, Any], consistent with the existingstatus()anddescribe()on that protocol, and the application layer converts it to a typedIndexResetResult.index resetcommand rather thanclear --force. Reset is genuinely more destructive than clear: it deletes vector collections, which clear deliberately does not. Overloadingclearwith a flag that changes how much it destroys seemed worse than a separate, explicitly named command. Happy to reshape it if you would rather have the flag.Validation
Windows 11, Python 3.12.14,
scenemodel artifacts prepared. Exit codes read directly, never through a pipe.Reproduction first. Two real clips imported and indexed with the
scenecapability againstgoogle/siglip2-base-patch16-224, then the active snapshot's generation manifests rewritten to the previousindex_schema_versionwith the snapshot and pointer hashes recomputed — what a repository looks like after an intentional schema change. Note this is a simulation of the upgrade: it reaches the same validation and the same code paths, but it does not literally run old code and then new code.Before the change, on that repository:
vidxp index statusIndexSchemaErrorvidxp index clear --yesIndexSchemaErrorvidxp index bulk --all --modality sceneIndexSchemaErrorAfter the change, the same repository:
vidxp index reset --yesscene, and deleted thescenecollectionvidxp media list --jsonvidxp index status --jsonstate: empty— the repository describes itself againvidxp index bulk --all --modality scenevidxp search scene "colour bars" --jsonclearstill refuses an incompatible index, unchanged — it validates by design, andresetis the operation that does not.Edge cases exercised as real commands: a brand-new repository with no index at all returns
No generated index data was found.and exit 0 (this failed with a nonsensical incompatible-schema error until I fixed it —IndexStorage(create=False)treats an empty store as a missing one), and reset on an already-cleared repository is safe to repeat.Tests —
uv run --no-sync python -m unittest discover -s tests -p "test_index_reset.py"→Ran 10 tests ... OK. They cover: the incompatible index blockingread_active,clearandstatus; the document still being readable;describe_active_generationsbeing read-only and repeatable; discard recovering the repository; an empty repository reporting no work; a compatible index being unaffected by the new read path; anddrop_all_collectionsremoving collections the configuration does not name.Written as
unittest.TestCasedeliberately, since CI discovers tests withpython -m unittest discoverand would not collect module-level pytest functions (#156).Known test gap, stated rather than hidden:
LocalIndexBackend.reset— the orchestration that sequences dropping collections and discarding metadata — is covered by the end-to-end runs above but has no unit test of its own. The layers underneath it (repository, storage, application) each do.Full suite —
uv run --no-sync python -m pytest -q:Lint —
uvx "ruff~=0.16.1" check src tests→ all checks passed, including--select RUF100.Documentation —
npx --yes markdownlint-cli2@0.23.2 "INSTALLATION_GUIDE.md"→ exit 0. No links added, solycheewas not run locally; please let the documentation workflow cover it.