fix(config): refuse startup that would orphan the KB registry; probe both ends for zero vectors#36
Merged
Merged
Conversation
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.
Two defects surfaced by an adversarial audit of
mainafter PRs #32–#35 merged. Each is a follow-up to code those PRs introduced.1. Startup can silently orphan an existing KB registry (blocking)
PR #33 made the session/KB-metadata SQLite path resolve from
config.database.path. Before that, every entry point hardcoded./data/perspicacite.dband ignored the config key. The shippedconfig.example.yml— the documented starting point (cp config.example.yml config.yml) — setsdatabase.pathto~/.local/share/perspicacite/memory.db. So a deployment created before #33 keeps its knowledge bases in the legacy working-directory file, while its config names a home-directory path that does not exist. After #33, honouring the config opens an empty database andlist_kbs()returns an empty list: every knowledge base vanishes from the UI, and new metadata is written where the vectors do not follow.#33's guard only defended the case where
database.pathis unset; the documented config sets it explicitly, so the guard did not fire for exactly the mainstream upgrade path.guard_session_db_pathnow refuses to start when the configured database is absent and the legacy./data/perspicacite.dbstill holds a non-emptykb_metadatatable. The error names both paths and the fix. No data is moved — relocating a production registry implicitly is riskier than a loud stop.PERSPICACITE_ALLOW_NEW_DB=1opts into a deliberately fresh instance. Wired into the web app, the MCP server and the two CLI entry points.Verified against the real production registry and config: the guard raises with the correct message and paths, and the escape hatch bypasses it.
2. The embedding-health probe missed tail poisoning (advisory)
PR #32 added
embedding_healthtoGET /api/kb/{name}/stats, sampling a bounded prefix (first 128 chunks). A mid-ingest embedding outage leaves a contiguous run of zero vectors, usually at the end of a collection, which a prefix-only sample reports as healthy. The probe now reads both ends, and the block gainedtotal_chunksandcompleteso a clean sample of a large collection is not mistaken for a proven-clean collection.Tests
test_config_database_path.py: guard raises on the orphaning scenario; the escape hatch, an absent legacy DB, an empty legacy DB, and a config pinned at the legacy path all pass through.test_embedding_health_probe.py: zeros beyond the first 128 chunks flipdegradedto true; a clean large sample is reportedcomplete: false.CI's unit suite: 2433 passed, 5 skipped, 0 failed.
ruffclean on the new module;mypyclean onconfig/paths.py.