diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fb0294..ea9609e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,11 +24,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 second instance listed the main hub's knowledge bases and wrote its vectors elsewhere. When `database.path` is not set, the legacy location is kept, so existing deployments are unaffected. Both resolved paths are now logged at startup. +- Startup now refuses to open an empty registry when the configured `database.path` does not + exist but the legacy `./data/perspicacite.db` still holds knowledge bases. The shipped + `config.example.yml` sets `database.path` to a home-directory location, so a deployment created + before the path was honoured (metadata in the legacy file) would otherwise present an empty KB + list after upgrade. The error names the fix; `PERSPICACITE_ALLOW_NEW_DB=1` opts into a fresh + instance. No data is moved. +- The `embedding_health` probe now samples both ends of a collection instead of a bounded prefix, + so a mid-ingest embedding outage that zero-fills the tail is no longer reported healthy. The + block gained `total_chunks` and `complete`, so a clean sample of a large KB is not mistaken for + a proven-clean KB. ### Added - `GET /api/kb/{name}/stats` reports an `embedding_health` block (`probed_chunks`, - `zero_vector_chunks`, `degraded`) so a poisoned knowledge base is visible without reading - Chroma by hand. + `zero_vector_chunks`, `degraded`, `total_chunks`, `complete`) so a poisoned knowledge base is + visible without reading Chroma by hand. ### Changed - **BREAKING:** the `indicia` and `adapters` optional extras are removed. They required the diff --git a/src/perspicacite/cli.py b/src/perspicacite/cli.py index 8467240..8c156cb 100644 --- a/src/perspicacite/cli.py +++ b/src/perspicacite/cli.py @@ -10,7 +10,7 @@ from perspicacite import __version__ from perspicacite.config import load_config -from perspicacite.config.paths import resolve_session_db_path +from perspicacite.config.paths import guard_session_db_path from perspicacite.logging import get_logger, setup_logging from perspicacite.pipeline.asb.run_ingest import ingest_asb_run as ingest_asb_run_pipeline from perspicacite.pipeline.github.bundle import ContentSpec @@ -260,7 +260,7 @@ async def _create_empty() -> dict[str, Any]: ) return - session_db = session_db or resolve_session_db_path(config) + session_db = session_db or guard_session_db_path(config) chroma_dir = chroma_dir or config.database.chroma_path click.echo(f"Creating knowledge base '{name}' from {from_bibtex}...") @@ -346,7 +346,7 @@ def add_to_kb( if ingest_mode is not None: config.knowledge_base.ingest_mode = ingest_mode - session_db = session_db or resolve_session_db_path(config) + session_db = session_db or guard_session_db_path(config) chroma_dir = chroma_dir or config.database.chroma_path click.echo(f"Adding papers from {from_bibtex} to knowledge base '{name}'...") diff --git a/src/perspicacite/config/paths.py b/src/perspicacite/config/paths.py index b7ad4ee..f96db7f 100644 --- a/src/perspicacite/config/paths.py +++ b/src/perspicacite/config/paths.py @@ -7,6 +7,8 @@ nothing. These helpers give every entry point (web, MCP, CLI) one answer. """ +import os +import sqlite3 from pathlib import Path from perspicacite.config.schema import Config @@ -15,6 +17,19 @@ # to the process working directory. LEGACY_SESSION_DB_PATH = Path("./data/perspicacite.db") +# Set to start a deliberately fresh instance whose configured database does not +# yet exist, bypassing the orphaned-registry guard below. +ALLOW_NEW_DB_ENV = "PERSPICACITE_ALLOW_NEW_DB" + + +class SessionDatabaseMisconfiguredError(RuntimeError): + """The configured session database is absent while a populated one is not. + + Honouring the configuration would open an empty database and hide every + existing knowledge base. Raised at startup so the operator fixes the path + (or opts into a fresh instance) rather than discovering an empty registry. + """ + def resolve_session_db_path(config: Config) -> Path: """Return the KB-metadata SQLite path for this configuration. @@ -27,3 +42,57 @@ def resolve_session_db_path(config: Config) -> Path: if "path" in config.database.model_fields_set: return config.database.path.expanduser() return LEGACY_SESSION_DB_PATH + + +def _holds_knowledge_bases(db_path: Path) -> bool: + """Return True when ``db_path`` is a SQLite file with a non-empty registry.""" + if not db_path.is_file(): + return False + try: + connection = sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) + except sqlite3.Error: + return False + try: + has_table = connection.execute( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='kb_metadata'" + ).fetchone() + if not has_table: + return False + return connection.execute("SELECT 1 FROM kb_metadata LIMIT 1").fetchone() is not None + except sqlite3.Error: + return False + finally: + connection.close() + + +def guard_session_db_path(config: Config) -> Path: + """Resolve the session-DB path, refusing to orphan an existing registry. + + The shipped ``config.example.yml`` sets ``database.path`` to a home-directory + location, while deployments created before the path was honoured keep their + knowledge bases in the legacy working-directory database. Starting against + the configured (empty) path would present an empty KB list and write new + metadata somewhere the vectors do not follow. Rather than move data + implicitly, refuse to start and name the fix. ``PERSPICACITE_ALLOW_NEW_DB=1`` + opts into a deliberately fresh instance. + """ + resolved = resolve_session_db_path(config) + if os.environ.get(ALLOW_NEW_DB_ENV): + return resolved + if resolved.exists() or resolved == LEGACY_SESSION_DB_PATH: + return resolved + if not _holds_knowledge_bases(LEGACY_SESSION_DB_PATH): + return resolved + raise SessionDatabaseMisconfiguredError( + f"configured session database {resolved} does not exist, but " + f"{LEGACY_SESSION_DB_PATH.resolve()} holds existing knowledge bases. " + f"Starting here would present an empty registry. Set database.path to " + f"'{LEGACY_SESSION_DB_PATH}' in your config, or move the file to " + f"{resolved}, or set {ALLOW_NEW_DB_ENV}=1 to start a fresh instance." + ) + + +if __name__ == "__main__": + assert resolve_session_db_path(Config()) == LEGACY_SESSION_DB_PATH + assert guard_session_db_path(Config()) == LEGACY_SESSION_DB_PATH + print("paths smoke check OK") diff --git a/src/perspicacite/mcp/server.py b/src/perspicacite/mcp/server.py index b5768fe..1e23f1f 100644 --- a/src/perspicacite/mcp/server.py +++ b/src/perspicacite/mcp/server.py @@ -33,7 +33,7 @@ from pathlib import Path from typing import Any -from perspicacite.config.paths import resolve_session_db_path +from perspicacite.config.paths import guard_session_db_path from perspicacite.logging import get_logger from perspicacite.pipeline.asb.response import build_asb_response_metadata from perspicacite.pipeline.asb.run_ingest import ingest_asb_run as ingest_asb_run_pipeline @@ -124,7 +124,7 @@ async def initialize(self, config: Any) -> None: ) # Session store - db_path = resolve_session_db_path(config) + db_path = guard_session_db_path(config) db_path.parent.mkdir(parents=True, exist_ok=True) self.session_store = SessionStore(db_path) await self.session_store.init_db() diff --git a/src/perspicacite/web/routers/kb.py b/src/perspicacite/web/routers/kb.py index 6c63aed..f170835 100644 --- a/src/perspicacite/web/routers/kb.py +++ b/src/perspicacite/web/routers/kb.py @@ -753,26 +753,50 @@ async def add_papers_to_kb(name: str, request: KBAddPapersRequest): EMBEDDING_PROBE_LIMIT = 128 +def _fetch_vectors(coll, limit: int, offset: int) -> list: + """Return the embedding vectors of a slice, or an empty list.""" + if limit <= 0: + return [] + got = coll.get(limit=limit, offset=offset, include=["embeddings"]) + vectors = got.get("embeddings") + return list(vectors) if vectors is not None else [] + + def _probe_embedding_health(coll, total_chunks: int) -> dict: - """Sample stored vectors and count the degenerate (zero-norm) ones. + """Sample stored vectors head and tail and count the degenerate (zero-norm) ones. A zero vector sits at cosine distance 1.0 from every other vector, so a KB full of them answers every query with the same passages at the same score. - Sampling a bounded prefix surfaces that; fetching every vector would not be - affordable on a large collection. + A mid-ingest embedding outage leaves a contiguous run of zeros, usually at + the end, which a prefix-only sample would miss; the probe reads both ends. + ``total_chunks`` and ``complete`` are returned so a clean sample of a large + collection is not mistaken for a proven-clean collection. """ - empty = {"probed_chunks": 0, "zero_vector_chunks": 0, "degraded": False} + empty = { + "probed_chunks": 0, + "zero_vector_chunks": 0, + "degraded": False, + "total_chunks": total_chunks, + "complete": total_chunks == 0, + } if not total_chunks: return empty - probe = coll.get(limit=min(total_chunks, EMBEDDING_PROBE_LIMIT), include=["embeddings"]) - vectors = probe.get("embeddings") - if vectors is None or len(vectors) == 0: + if total_chunks <= EMBEDDING_PROBE_LIMIT: + vectors = _fetch_vectors(coll, total_chunks, 0) + else: + head = EMBEDDING_PROBE_LIMIT // 2 + tail = EMBEDDING_PROBE_LIMIT - head + vectors = _fetch_vectors(coll, head, 0) + _fetch_vectors(coll, tail, total_chunks - tail) + if not vectors: return empty zero_count = sum(1 for vector in vectors if is_zero_vector(list(vector))) + probed = len(vectors) return { - "probed_chunks": len(vectors), + "probed_chunks": probed, "zero_vector_chunks": zero_count, "degraded": zero_count > 0, + "total_chunks": total_chunks, + "complete": probed >= total_chunks, } @@ -785,7 +809,7 @@ async def get_kb_stats(name: str): if not kb: return {"error": f"Knowledge base '{name}' not found"} scan_cap = 20000 - embedding_health = {"probed_chunks": 0, "zero_vector_chunks": 0, "degraded": False} + embedding_health = _probe_embedding_health(None, 0) try: coll = app_state.vector_store.client.get_collection(name=kb.collection_name) total_chunks = coll.count() diff --git a/src/perspicacite/web/state.py b/src/perspicacite/web/state.py index 0f694ed..e797c03 100644 --- a/src/perspicacite/web/state.py +++ b/src/perspicacite/web/state.py @@ -11,7 +11,7 @@ import os from typing import Any -from perspicacite.config.paths import resolve_session_db_path +from perspicacite.config.paths import guard_session_db_path from perspicacite.jobs.registry import JobRegistry from perspicacite.memory.session_store import SessionStore from perspicacite.provenance.store import ProvenanceStore @@ -192,7 +192,7 @@ async def initialize(self): logger.info("Agentic orchestrator initialized") # Initialize session store FIRST so RAGEngine can receive it - db_path = resolve_session_db_path(config) + db_path = guard_session_db_path(config) db_path.parent.mkdir(parents=True, exist_ok=True) self.session_store = SessionStore(db_path) await self.session_store.init_db() diff --git a/tests/unit/test_config_database_path.py b/tests/unit/test_config_database_path.py index 0157e16..e30d9a0 100644 --- a/tests/unit/test_config_database_path.py +++ b/tests/unit/test_config_database_path.py @@ -49,6 +49,8 @@ async def test_app_state_opens_the_configured_database(monkeypatch, tmp_path): target = tmp_path / "instance" / "memory.db" monkeypatch.setenv("PERSPICACITE_DB_PATH", str(target)) monkeypatch.setenv("PERSPICACITE_ALLOW_MISSING_LLM_KEYS", "1") + # A throwaway instance whose configured DB does not exist yet. + monkeypatch.setenv("PERSPICACITE_ALLOW_NEW_DB", "1") monkeypatch.setenv("PERSPICACITE_DB_CHROMA_PATH", str(tmp_path / "chroma")) opened: list[Path] = [] @@ -68,3 +70,84 @@ async def init_db(self): assert opened == [target] assert opened[0] != LEGACY_SESSION_DB_PATH + + +def _make_populated_db(path: Path) -> None: + """Create a SQLite file with one kb_metadata row, like a real registry.""" + import sqlite3 + + path.parent.mkdir(parents=True, exist_ok=True) + connection = sqlite3.connect(path) + connection.execute("CREATE TABLE kb_metadata (name TEXT)") + connection.execute("INSERT INTO kb_metadata (name) VALUES ('existing-kb')") + connection.commit() + connection.close() + + +def test_guard_refuses_to_orphan_an_existing_registry(monkeypatch, tmp_path): + """The documented `cp config.example.yml` path must not open an empty DB.""" + from perspicacite.config import paths as paths_module + + legacy = tmp_path / "data" / "perspicacite.db" + _make_populated_db(legacy) + monkeypatch.setattr(paths_module, "LEGACY_SESSION_DB_PATH", legacy) + monkeypatch.delenv(paths_module.ALLOW_NEW_DB_ENV, raising=False) + + config = Config(database={"path": str(tmp_path / "home" / "memory.db")}) + with pytest.raises(paths_module.SessionDatabaseMisconfiguredError) as excinfo: + paths_module.guard_session_db_path(config) + assert str(legacy) in str(excinfo.value) + + +def test_guard_allows_a_deliberately_fresh_instance(monkeypatch, tmp_path): + """PERSPICACITE_ALLOW_NEW_DB opts out of the orphan guard.""" + from perspicacite.config import paths as paths_module + + legacy = tmp_path / "data" / "perspicacite.db" + _make_populated_db(legacy) + monkeypatch.setattr(paths_module, "LEGACY_SESSION_DB_PATH", legacy) + monkeypatch.setenv(paths_module.ALLOW_NEW_DB_ENV, "1") + + target = tmp_path / "home" / "memory.db" + resolved = paths_module.guard_session_db_path(Config(database={"path": str(target)})) + assert resolved == target + + +def test_guard_is_silent_when_no_legacy_registry_exists(monkeypatch, tmp_path): + """A first-ever install has no legacy DB, so the guard must not fire.""" + from perspicacite.config import paths as paths_module + + monkeypatch.setattr(paths_module, "LEGACY_SESSION_DB_PATH", tmp_path / "data" / "absent.db") + monkeypatch.delenv(paths_module.ALLOW_NEW_DB_ENV, raising=False) + + target = tmp_path / "home" / "memory.db" + resolved = paths_module.guard_session_db_path(Config(database={"path": str(target)})) + assert resolved == target + + +def test_guard_ignores_an_empty_legacy_database(monkeypatch, tmp_path): + """A legacy file with no knowledge bases is not worth blocking startup for.""" + from perspicacite.config import paths as paths_module + + legacy = tmp_path / "data" / "perspicacite.db" + legacy.parent.mkdir(parents=True) + legacy.touch() # exists but has no kb_metadata table + monkeypatch.setattr(paths_module, "LEGACY_SESSION_DB_PATH", legacy) + monkeypatch.delenv(paths_module.ALLOW_NEW_DB_ENV, raising=False) + + target = tmp_path / "home" / "memory.db" + resolved = paths_module.guard_session_db_path(Config(database={"path": str(target)})) + assert resolved == target + + +def test_guard_passes_through_when_config_points_at_the_legacy_path(monkeypatch, tmp_path): + """Pinning database.path to the legacy DB is the intended fix, not an error.""" + from perspicacite.config import paths as paths_module + + legacy = tmp_path / "data" / "perspicacite.db" + _make_populated_db(legacy) + monkeypatch.setattr(paths_module, "LEGACY_SESSION_DB_PATH", legacy) + monkeypatch.delenv(paths_module.ALLOW_NEW_DB_ENV, raising=False) + + resolved = paths_module.guard_session_db_path(Config(database={"path": str(legacy)})) + assert resolved == legacy diff --git a/tests/unit/test_embedding_health_probe.py b/tests/unit/test_embedding_health_probe.py new file mode 100644 index 0000000..ce81604 --- /dev/null +++ b/tests/unit/test_embedding_health_probe.py @@ -0,0 +1,67 @@ +"""Regression tests for the embedding-health probe in GET /api/kb/{name}/stats. + +The probe samples stored vectors to flag a knowledge base poisoned with zero +vectors. A mid-ingest embedding outage leaves a contiguous run of zeros, usually +at the end of the collection, so a prefix-only sample would report a heavily +poisoned KB as healthy. The probe reads both ends and states whether the whole +collection was checked, so a sample is never mistaken for a proof. +""" + +import chromadb + +from perspicacite.web.routers.kb import EMBEDDING_PROBE_LIMIT, _probe_embedding_health + +DIMENSION = 8 +_UNIT = [1.0] + [0.0] * (DIMENSION - 1) + + +def _collection(tmp_path, zero_from: int, total: int): + """A Chroma collection of `total` vectors, zero-filled from index `zero_from`.""" + client = chromadb.PersistentClient(path=str(tmp_path)) + coll = client.create_collection(name="probe-kb") + embeddings = [[0.0] * DIMENSION if i >= zero_from else _UNIT for i in range(total)] + coll.add( + ids=[f"c{i}" for i in range(total)], + embeddings=embeddings, + documents=[f"doc {i}" for i in range(total)], + ) + return coll + + +def test_empty_collection_is_complete_and_not_degraded(): + assert _probe_embedding_health(None, 0) == { + "probed_chunks": 0, + "zero_vector_chunks": 0, + "degraded": False, + "total_chunks": 0, + "complete": True, + } + + +def test_small_clean_collection_is_probed_completely(tmp_path): + coll = _collection(tmp_path, zero_from=999, total=20) + health = _probe_embedding_health(coll, 20) + assert health["degraded"] is False + assert health["complete"] is True + assert health["probed_chunks"] == 20 + + +def test_late_poisoning_beyond_the_prefix_is_detected(tmp_path): + """Zeros past the first 128 chunks must still flip degraded to True.""" + total = EMBEDDING_PROBE_LIMIT * 4 + coll = _collection(tmp_path, zero_from=total - 10, total=total) + health = _probe_embedding_health(coll, total) + assert health["degraded"] is True, "tail poisoning must not be reported healthy" + assert health["zero_vector_chunks"] >= 1 + assert health["complete"] is False + assert health["total_chunks"] == total + + +def test_large_clean_sample_is_flagged_incomplete(tmp_path): + """A clean sample of a big KB is not proof the whole KB is clean.""" + total = EMBEDDING_PROBE_LIMIT * 4 + coll = _collection(tmp_path, zero_from=total + 1, total=total) + health = _probe_embedding_health(coll, total) + assert health["degraded"] is False + assert health["complete"] is False + assert health["probed_chunks"] == EMBEDDING_PROBE_LIMIT