refactor: fix all project-wide quality issues found by full audit

GeNa8880 · GeNa8880 · commit f3e9d345b9a8 · 2026-03-06T12:16:57.000+01:00
- Fix 5 failing multilingual tests (rate limiter accumulation across tests)
- Add autouse fixture to reset rate limiter between tests
- Fix all ruff lint errors (E402, F401, I001, E501)
- Fix ruff formatting for 5 test files
- Fix all 6 mypy errors (pydantic-settings call-arg, arg-type, assignment)
- Add mypy, vulture, interrogate to dev dependencies
- Remove unused import in test_api_rate_limiting.py
diff --git a/.gitignore b/.gitignore
@@ -44,3 +44,4 @@ CLAUDE.md
 docs/audit-fixes.md
 docs/audit-technical-spec.md
 .hypothesis/
+AUDIT-PLAN.md
diff --git a/pyproject.toml b/pyproject.toml
@@ -35,6 +35,9 @@ dev = [
     "schemathesis>=3.38",
     "bandit>=1.7.0",
     "pip-audit>=2.7.0",
+    "mypy>=1.19.1",
+    "vulture>=2.15",
+    "interrogate>=1.7.0",
 ]
 
 [build-system]
diff --git a/src/rag_engine/api/app.py b/src/rag_engine/api/app.py
@@ -10,7 +10,7 @@
 from rag_engine.models.config import Settings
 from rag_engine.utils.logging import setup_logging
 
-settings = Settings()
+settings = Settings()  # type: ignore[call-arg]
 
 
 def create_app() -> FastAPI:
@@ -25,7 +25,7 @@ def create_app() -> FastAPI:
 
     # Rate limiting
     app.state.limiter = limiter
-    app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)
+    app.add_exception_handler(RateLimitExceeded, _rate_limit_exceeded_handler)  # type: ignore[arg-type]
 
     app.add_middleware(
         CORSMiddleware,
diff --git a/src/rag_engine/api/dependencies.py b/src/rag_engine/api/dependencies.py
@@ -12,7 +12,7 @@
 from rag_engine.storage.knowledge_graph import KnowledgeGraphStore
 from rag_engine.storage.qdrant_store import QdrantStore
 
-_settings = Settings()
+_settings = Settings()  # type: ignore[call-arg]
 _logger = structlog.get_logger()
 
 VALID_ID_PATTERN = re.compile(r"^[a-zA-Z0-9_-]{1,128}$")
diff --git a/src/rag_engine/api/routes/health.py b/src/rag_engine/api/routes/health.py
@@ -6,7 +6,7 @@
 from rag_engine.models.health import HealthResponse, HealthStatus
 
 router = APIRouter(tags=["system"])
-settings = Settings()
+settings = Settings()  # type: ignore[call-arg]
 
 
 @router.get("/health", response_model=HealthResponse)
diff --git a/src/rag_engine/ingestion/parsers/docx_parser.py b/src/rag_engine/ingestion/parsers/docx_parser.py
@@ -21,7 +21,7 @@ def parse(self, file_path: Path) -> str:
             Concatenated text from all paragraphs.
         """
         try:
-            document = Document(file_path)
+            document = Document(str(file_path))
         except Exception as exc:
             raise IngestionError(f"Failed to open DOCX: {file_path}") from exc
 
diff --git a/src/rag_engine/services/embedding.py b/src/rag_engine/services/embedding.py
@@ -25,7 +25,7 @@ def __init__(self, model_name: str = "intfloat/multilingual-e5-large") -> None:
             model_name: HuggingFace model identifier.
         """
         self._model_name = model_name
-        self._model = None
+        self._model: Any = None
 
     @property
     def model(self) -> Any:
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -12,15 +12,16 @@
 )
 settings.load_profile(os.getenv("HYPOTHESIS_PROFILE", "default"))
 
-import pytest
-from fastapi.testclient import TestClient
-from qdrant_client import QdrantClient
+import pytest  # noqa: E402
+from fastapi.testclient import TestClient  # noqa: E402
+from qdrant_client import QdrantClient  # noqa: E402
 
-from rag_engine.api.app import create_app
-from rag_engine.services.gdpr import GDPRService
-from rag_engine.storage.bm25_store import BM25Store
-from rag_engine.storage.knowledge_graph import KnowledgeGraphStore
-from rag_engine.storage.qdrant_store import QdrantStore
+from rag_engine.api.app import create_app  # noqa: E402
+from rag_engine.api.routes.rate_limit import limiter  # noqa: E402
+from rag_engine.services.gdpr import GDPRService  # noqa: E402
+from rag_engine.storage.bm25_store import BM25Store  # noqa: E402
+from rag_engine.storage.knowledge_graph import KnowledgeGraphStore  # noqa: E402
+from rag_engine.storage.qdrant_store import QdrantStore  # noqa: E402
 
 TEST_API_KEY = "test-api-key"
 
@@ -65,8 +66,15 @@ def gdpr_service(
     return GDPRService(bm25_store, graph_store, qdrant_store)
 
 
+@pytest.fixture(autouse=True)
+def _fresh_rate_limiter():
+    """Reset the rate limiter before each test to avoid cross-test 429 errors."""
+    limiter.reset()
+    yield
+
+
 @pytest.fixture
 def test_client() -> TestClient:
-    """FastAPI test client."""
+    """FastAPI test client with a fresh rate limiter."""
     app = create_app()
     return TestClient(app)
diff --git a/tests/integration/test_api_rate_limiting.py b/tests/integration/test_api_rate_limiting.py
@@ -6,8 +6,6 @@
 from slowapi.errors import RateLimitExceeded
 from slowapi.util import get_remote_address
 
-from rag_engine.api.dependencies import verify_api_key
-
 
 class TestAPIRateLimiting:
     """Verify rate limiter returns 429 when limits are exceeded."""
diff --git a/tests/integration/test_api_schema.py b/tests/integration/test_api_schema.py
@@ -1,9 +1,9 @@
 """API schema validation tests using Schemathesis."""
 
 import schemathesis
+from schemathesis.openapi import from_asgi
 from slowapi import Limiter
 from slowapi.util import get_remote_address
-from schemathesis.openapi import from_asgi
 
 from rag_engine.api.app import create_app
 from rag_engine.api.routes import rate_limit
diff --git a/tests/integration/test_concurrent_tenants.py b/tests/integration/test_concurrent_tenants.py
@@ -34,13 +34,19 @@ def test_three_tenants_isolated(self) -> None:
         # Each tenant finds their own keyword
         for tenant, (_, query) in tenants.items():
             results = retriever.search(
-                tenant, query, language="en", search_type=SearchType.BM25,
+                tenant,
+                query,
+                language="en",
+                search_type=SearchType.BM25,
             )
             assert len(results) >= 1, f"{tenant} should find '{query}'"
 
         # tenant-beta must not see tenant-alpha data
         results = retriever.search(
-            "tenant-beta", "rockets", language="en", search_type=SearchType.BM25,
+            "tenant-beta",
+            "rockets",
+            language="en",
+            search_type=SearchType.BM25,
         )
         assert len(results) == 0, "tenant-beta must not see tenant-alpha data"
 
@@ -53,7 +59,10 @@ def ingest_and_search(args):
             bm25.add_documents(tenant, "en", [(f"{tenant}-doc", 0, text)])
             graph.add_documents(tenant, [(f"{tenant}-doc", 0, text)])
             results = retriever.search(
-                tenant, query, language="en", search_type=SearchType.BM25,
+                tenant,
+                query,
+                language="en",
+                search_type=SearchType.BM25,
             )
             return tenant, len(results)
 
@@ -84,12 +93,18 @@ def test_deleting_one_tenant_preserves_others(self) -> None:
 
         # Deleted tenant should have no results
         results = retriever.search(
-            "del-t", "Data to delete", language="en", search_type=SearchType.BM25,
+            "del-t",
+            "Data to delete",
+            language="en",
+            search_type=SearchType.BM25,
         )
         assert len(results) == 0
 
         # Other tenant should still have results
         results = retriever.search(
-            "keep-t", "Data to keep", language="en", search_type=SearchType.BM25,
+            "keep-t",
+            "Data to keep",
+            language="en",
+            search_type=SearchType.BM25,
         )
         assert len(results) >= 1
diff --git a/tests/integration/test_gdpr_full_flow.py b/tests/integration/test_gdpr_full_flow.py
@@ -40,7 +40,9 @@ def test_upload_search_delete_verify(self) -> None:
 
         # 2. Search — should find the document
         results = retriever.search(
-            "gdpr-tenant", "Sensitive personal data", language="en",
+            "gdpr-tenant",
+            "Sensitive personal data",
+            language="en",
             search_type=SearchType.BM25,
         )
         assert len(results) >= 1
@@ -52,7 +54,9 @@ def test_upload_search_delete_verify(self) -> None:
 
         # 4. Verify — search must return nothing
         results = retriever.search(
-            "gdpr-tenant", "Sensitive personal data", language="en",
+            "gdpr-tenant",
+            "Sensitive personal data",
+            language="en",
             search_type=SearchType.BM25,
         )
         assert len(results) == 0
@@ -79,7 +83,9 @@ def test_tenant_erasure_removes_all_documents(self) -> None:
 
         # Verify docs are searchable
         results = retriever.search(
-            "erasure-tenant", "Document number", language="en",
+            "erasure-tenant",
+            "Document number",
+            language="en",
             search_type=SearchType.BM25,
         )
         assert len(results) >= 1
@@ -90,7 +96,9 @@ def test_tenant_erasure_removes_all_documents(self) -> None:
 
         # Verify nothing remains
         results = retriever.search(
-            "erasure-tenant", "Document number", language="en",
+            "erasure-tenant",
+            "Document number",
+            language="en",
             search_type=SearchType.BM25,
         )
         assert len(results) == 0
diff --git a/tests/integration/test_hybrid_retrieval_quality.py b/tests/integration/test_hybrid_retrieval_quality.py
@@ -51,12 +51,20 @@ def test_hybrid_score_exceeds_pure_vector(self) -> None:
 
         vector_results = qdrant.search("t1", query_vec, top_k=4)
         hybrid = retriever.search(
-            "t1", "Machine Learning", language="en",
-            search_type=SearchType.HYBRID, vector_results=vector_results, top_k=4,
+            "t1",
+            "Machine Learning",
+            language="en",
+            search_type=SearchType.HYBRID,
+            vector_results=vector_results,
+            top_k=4,
         )
         pure_vector = retriever.search(
-            "t1", "Machine Learning", language="en",
-            search_type=SearchType.VECTOR, vector_results=vector_results, top_k=4,
+            "t1",
+            "Machine Learning",
+            language="en",
+            search_type=SearchType.VECTOR,
+            vector_results=vector_results,
+            top_k=4,
         )
 
         # Hybrid should return results (BM25 + graph contribute)
@@ -74,16 +82,25 @@ def test_hybrid_returns_more_results_than_single_method(self) -> None:
         retriever = HybridRetriever(bm25, graph)
 
         bm25_only = retriever.search(
-            "t1", "Machine Learning healthcare", language="en",
-            search_type=SearchType.BM25, top_k=10,
+            "t1",
+            "Machine Learning healthcare",
+            language="en",
+            search_type=SearchType.BM25,
+            top_k=10,
         )
         graph_only = retriever.search(
-            "t1", "Machine Learning healthcare", language="en",
-            search_type=SearchType.GRAPH, top_k=10,
+            "t1",
+            "Machine Learning healthcare",
+            language="en",
+            search_type=SearchType.GRAPH,
+            top_k=10,
         )
         hybrid = retriever.search(
-            "t1", "Machine Learning healthcare", language="en",
-            search_type=SearchType.HYBRID, top_k=10,
+            "t1",
+            "Machine Learning healthcare",
+            language="en",
+            search_type=SearchType.HYBRID,
+            top_k=10,
         )
 
         # Hybrid should return at least as many results as either source alone
@@ -96,8 +113,11 @@ def test_hybrid_deduplicates_across_sources(self) -> None:
 
         retriever = HybridRetriever(bm25, graph)
         hybrid = retriever.search(
-            "t1", "Machine Learning", language="en",
-            search_type=SearchType.HYBRID, top_k=10,
+            "t1",
+            "Machine Learning",
+            language="en",
+            search_type=SearchType.HYBRID,
+            top_k=10,
         )
 
         doc_chunk_keys = [(r.document_id, r.chunk_text) for r in hybrid]
diff --git a/tests/integration/test_multilingual_search.py b/tests/integration/test_multilingual_search.py
@@ -26,8 +26,12 @@ def _upload(self, client: TestClient, text: str, tenant: str, lang: str) -> dict
     def _search(self, client: TestClient, query: str, tenant: str, language: str = "en") -> dict:
         resp = client.post(
             "/api/v1/documents/search",
-            json={"query": query, "tenant_id": tenant, "search_type": "bm25",
-                  "language": language},
+            json={
+                "query": query,
+                "tenant_id": tenant,
+                "search_type": "bm25",
+                "language": language,
+            },
             headers=self._headers,
         )
         assert resp.status_code == 200
diff --git a/tests/unit/test_property.py b/tests/unit/test_property.py
@@ -10,7 +10,6 @@
 from rag_engine.models.search import SearchQuery
 from rag_engine.storage.bm25_store import BM25Store
 
-
 # --- Chunker properties ---
 
 
@@ -138,7 +137,9 @@ def test_tenant_id_validation(id_str: str) -> None:
     assert sq.tenant_id == id_str
 
 
-@given(id_str=st.text(min_size=1, max_size=20).filter(lambda s: not re.match(r"^[a-zA-Z0-9_-]+$", s)))
+@given(
+    id_str=st.text(min_size=1, max_size=20).filter(lambda s: not re.match(r"^[a-zA-Z0-9_-]+$", s))
+)
 @settings(max_examples=50)
 def test_tenant_id_rejects_invalid(id_str: str) -> None:
     """Strings with invalid characters should be rejected."""
diff --git a/uv.lock b/uv.lock

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,9 @@ dev = [`
`35`	`35`	`"schemathesis>=3.38",`
`36`	`36`	`"bandit>=1.7.0",`
`37`	`37`	`"pip-audit>=2.7.0",`
	`38`	`+ "mypy>=1.19.1",`
	`39`	`+ "vulture>=2.15",`
	`40`	`+ "interrogate>=1.7.0",`
`38`	`41`	`]`
`39`	`42`
`40`	`43`	`[build-system]`