From 355811635d29e8efffea324b5c581c701a78896a Mon Sep 17 00:00:00 2001 From: Ed_ Date: Mon, 15 Jun 2026 23:48:25 -0400 Subject: [PATCH] fix(rag): handle None metadata in get_all_indexed_paths and non-empty numpy in dim check Two bugs in src/rag_engine.py were causing 'NoneType object has no attribute get' in the live_gui RAG tests (test_rag_phase4_final_verify, test_rag_phase4_stress): 1. _validate_collection_dim_result:148 Old: if not embeddings or len(embeddings) == 0: New: if embeddings is None or len(embeddings) == 0: The 'if not embeddings' check raises ValueError('The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()') when 'embeddings' is a non-empty numpy array (which is the normal case after documents are upserted). The exception is caught by the outer 'except Exception' which returns a non-ok Result, causing __init__ to set self.collection = None. Subsequent 'get_all_indexed_paths()' then fails with 'NoneType has no attribute get' on self.collection.get(). 2. get_all_indexed_paths:334 Old: return list(set(m.get('path') for m in res['metadatas'] if m.get('path'))) New: return list(set(m['path'] for m in res['metadatas'] if m is not None and m.get('path'))) When chromadb returns 'metadatas=[None, ...]' (documents upserted without metadata), 'm.get('path')' fails with AttributeError on the first None element. Adds 'm is not None' guard. Both fixes are defensive: the conditions that trigger them (orphan docs without metadata, non-empty embeddings arrays) are normal valid states that the old code couldn't handle. New file: tests/test_rag_sync_none_error.py 3 unit tests covering both bugs: - test_dim_check_does_not_raise_on_non_empty_ndarray - test_get_all_indexed_paths_handles_none_metadata - test_get_all_indexed_paths_returns_paths_with_metadata Verified: - 3/3 focused tests pass - test_rag_phase4_final_verify.py::test_phase4_final_verify PASSES (was failing) - test_rag_phase4_stress.py::test_rag_large_codebase_verification_sim PASSES (was failing) - test_rag_visual_sim.py::test_rag_full_lifecycle_sim PASSES (still passing) --- src/rag_engine.py | 4 +- tests/test_rag_sync_none_error.py | 81 +++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 tests/test_rag_sync_none_error.py diff --git a/src/rag_engine.py b/src/rag_engine.py index 3504c244..e98541ae 100644 --- a/src/rag_engine.py +++ b/src/rag_engine.py @@ -147,7 +147,7 @@ class RAGEngine: if not res: return Result(data=None) embeddings = res.get("embeddings") if isinstance(res, dict) else None - if not embeddings or len(embeddings) == 0: + if embeddings is None or len(embeddings) == 0: return Result(data=None) existing_dim = len(embeddings[0]) expected_dim = len(self.embedding_provider.embed(["__rag_dim_check__"])[0]) @@ -331,7 +331,7 @@ class RAGEngine: res = self.collection.get(include=["metadatas"]) if not res or not res["metadatas"]: return [] - return list(set(m.get("path") for m in res["metadatas"] if m.get("path"))) + return list(set(m["path"] for m in res["metadatas"] if m is not None and m.get("path"))) def delete_documents_by_path(self, file_paths: List[str]): if not self.config.enabled or self.collection == "mock": diff --git a/tests/test_rag_sync_none_error.py b/tests/test_rag_sync_none_error.py new file mode 100644 index 00000000..cab328ce --- /dev/null +++ b/tests/test_rag_sync_none_error.py @@ -0,0 +1,81 @@ +from __future__ import annotations +import os +import shutil +import tempfile +import pytest + +from src.rag_engine import RAGEngine +from src.models import RAGConfig, VectorStoreConfig + + +LOCAL_EMBED_DIM = 384 + + +@pytest.fixture +def temp_workspace(): + tmp = tempfile.mkdtemp() + try: + yield tmp + finally: + for _ in range(5): + try: + shutil.rmtree(tmp, ignore_errors=False) + break + except (OSError, PermissionError): + import time + time.sleep(0.2) + else: + shutil.rmtree(tmp, ignore_errors=True) + + +def _make_chroma_collection_with_orphan_no_metadata(db_path, collection_name, dim=LOCAL_EMBED_DIM): + from src.rag_engine import _get_chromadb + chromadb_module, _ = _get_chromadb() + client = chromadb_module.PersistentClient(path=db_path) + col = client.get_or_create_collection(name=collection_name) + emb = [[0.01 * i for i in range(dim)]] + col.upsert(ids=["orphan"], embeddings=emb, documents=["orphan doc"]) + return client, col + + +def _build_engine(workspace, collection_name): + cfg = RAGConfig( + enabled=True, + embedding_provider="local", + vector_store=VectorStoreConfig(provider="chroma", collection_name=collection_name), + ) + return RAGEngine(cfg, workspace) + + +def test_dim_check_does_not_raise_on_non_empty_ndarray(temp_workspace): + db_path = os.path.abspath(os.path.join(temp_workspace, ".slop_cache", "chroma_test_dim")) + os.makedirs(db_path, exist_ok=True) + _make_chroma_collection_with_orphan_no_metadata(db_path, "test_dim") + + engine = _build_engine(temp_workspace, "test_dim") + assert engine.collection is not None + + +def test_get_all_indexed_paths_handles_none_metadata(temp_workspace): + db_path = os.path.abspath(os.path.join(temp_workspace, ".slop_cache", "chroma_test_orphan")) + os.makedirs(db_path, exist_ok=True) + _make_chroma_collection_with_orphan_no_metadata(db_path, "test_orphan") + + engine = _build_engine(temp_workspace, "test_orphan") + paths = engine.get_all_indexed_paths() + assert paths == [] + + +def test_get_all_indexed_paths_returns_paths_with_metadata(temp_workspace): + db_path = os.path.abspath(os.path.join(temp_workspace, ".slop_cache", "chroma_test_with_meta")) + os.makedirs(db_path, exist_ok=True) + from src.rag_engine import _get_chromadb + chromadb_module, _ = _get_chromadb() + client = chromadb_module.PersistentClient(path=db_path) + col = client.get_or_create_collection(name="test_with_meta") + emb = [[0.01 * i for i in range(LOCAL_EMBED_DIM)]] + col.upsert(ids=["doc1"], embeddings=emb, documents=["doc1"], metadatas=[{"path": "f.txt", "chunk": 0}]) + + engine = _build_engine(temp_workspace, "test_with_meta") + paths = engine.get_all_indexed_paths() + assert "f.txt" in paths