From 24af75eb4dbeaab7688ac619fd5a2722de86e33b Mon Sep 17 00:00:00 2001 From: Fran BlakIA Date: Sat, 8 Aug 2026 13:18:01 +0200 Subject: [PATCH 1/4] chore(lint): fijar el conjunto de reglas de ruff y sanear el estilo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ruff se estaba ejecutando con su conjunto por defecto, que cambia entre versiones: un CI que dependa de él se rompe solo el día que alguien actualiza la herramienta. Se fija un select explícito (E, F, I, UP, PIE, SIM) en pyproject. Con esas reglas había 140 avisos. Este commit corrige los mecánicos: anotaciones PEP 585/604, orden de imports, placeholders innecesarios e imports sin usar. Los prompts quedan exentos de E501 mediante per-file-ignores: son datos, y partir una línea de prompt para que quepa en 88 columnas cambia el texto que recibe el modelo. --- pyproject.toml | 27 +++++++++++++++++++ src/core/exceptions.py | 2 -- src/core/interfaces/chunker.py | 11 ++++---- src/core/interfaces/embedder.py | 7 ++--- src/core/interfaces/llm.py | 4 +-- src/core/interfaces/parser.py | 5 ++-- src/core/schemas/chunk.py | 10 +++---- src/core/schemas/document.py | 9 ++++--- .../embeddings/openai_embedder.py | 6 ++--- src/services/agent_service.py | 8 +++--- src/services/context_builder.py | 5 ++-- src/settings.py | 26 ++++++++---------- 12 files changed, 68 insertions(+), 52 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a5cb2c0..6d16414 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,3 +38,30 @@ dev = [ [tool.pytest.ini_options] asyncio_mode = "auto" asyncio_default_fixture_loop_scope = "function" + +[tool.ruff] +target-version = "py310" + +[tool.ruff.lint] +# Conjunto explícito y no el de por defecto: los valores por defecto de ruff +# cambian entre versiones, y un CI que depende de ellos se rompe solo el día +# que alguien actualiza la herramienta. +select = [ + "E", # pycodestyle + "F", # pyflakes + "I", # isort + "UP", # pyupgrade + "PIE", # flake8-pie + "SIM", # flake8-simplify +] + +[tool.ruff.lint.per-file-ignores] +# Los ejemplos priorizan la legibilidad didáctica sobre el estilo idiomático. +"examples/*" = ["E501"] +# Los prompts son datos, no código. Partir una línea de prompt para que quepa +# en 88 columnas cambia el texto que recibe el modelo y empeora la lectura del +# propio prompt, que es lo que aquí importa revisar. +"src/core/prompts.py" = ["E501"] +"src/services/agent_service.py" = ["E501"] +"src/endpoints/cli/main.py" = ["E501"] +"src/core/interfaces/parser.py" = ["E501"] diff --git a/src/core/exceptions.py b/src/core/exceptions.py index e335490..4cb354d 100644 --- a/src/core/exceptions.py +++ b/src/core/exceptions.py @@ -8,8 +8,6 @@ class RepositoryError(Exception): """Base exception for repository operations.""" - pass - class DocumentSaveError(RepositoryError): """Raised when a document fails to save to the database.""" diff --git a/src/core/interfaces/chunker.py b/src/core/interfaces/chunker.py index 8a29b5f..5055927 100644 --- a/src/core/interfaces/chunker.py +++ b/src/core/interfaces/chunker.py @@ -1,6 +1,6 @@ from abc import ABC, abstractmethod from dataclasses import dataclass -from typing import Any, Dict, List, Optional +from typing import Any @dataclass @@ -9,8 +9,8 @@ class RawChunk: content: str index: int - metadata: Dict[str, Any] - token_count: Optional[int] = None + metadata: dict[str, Any] + token_count: int | None = None class IChunker(ABC): @@ -18,8 +18,8 @@ class IChunker(ABC): @abstractmethod async def chunk_document( - self, content: str, title: str, source: str, docling_doc: Optional[Any] = None - ) -> List[RawChunk]: + self, content: str, title: str, source: str, docling_doc: Any | None = None + ) -> list[RawChunk]: """ Split a document into chunks. @@ -32,4 +32,3 @@ async def chunk_document( Returns: List of RawChunk objects. """ - pass diff --git a/src/core/interfaces/embedder.py b/src/core/interfaces/embedder.py index e5cf59b..8f3676d 100644 --- a/src/core/interfaces/embedder.py +++ b/src/core/interfaces/embedder.py @@ -1,16 +1,13 @@ from abc import ABC, abstractmethod -from typing import List class IEmbedder(ABC): """Interface for embedding generation providers.""" @abstractmethod - async def get_embedding(self, text: str) -> List[float]: + async def get_embedding(self, text: str) -> list[float]: """Generate embedding for a single string.""" - pass @abstractmethod - async def get_embeddings(self, texts: List[str]) -> List[List[float]]: + async def get_embeddings(self, texts: list[str]) -> list[list[float]]: """Generate embeddings for a batch of strings.""" - pass diff --git a/src/core/interfaces/llm.py b/src/core/interfaces/llm.py index 0e82ae8..ff549e4 100644 --- a/src/core/interfaces/llm.py +++ b/src/core/interfaces/llm.py @@ -1,6 +1,7 @@ from abc import ABC, abstractmethod +from collections.abc import AsyncIterator from dataclasses import dataclass, field -from typing import Any, AsyncIterator +from typing import Any @dataclass @@ -27,7 +28,6 @@ async def generate_response( self, system_prompt: str, user_prompt: str, stream: bool = False ) -> AsyncIterator[str] | str: """Generate a response from the LLM.""" - pass def supports_tools(self) -> bool: """Check if the provider supports function calling / tools. diff --git a/src/core/interfaces/parser.py b/src/core/interfaces/parser.py index ba6b77d..36965f7 100644 --- a/src/core/interfaces/parser.py +++ b/src/core/interfaces/parser.py @@ -1,12 +1,12 @@ from abc import ABC, abstractmethod -from typing import Any, Optional +from typing import Any class IParser(ABC): """Interface for document parsing.""" @abstractmethod - async def parse(self, file_path: str) -> tuple[str, Optional[Any]]: + async def parse(self, file_path: str) -> tuple[str, Any | None]: """ Parse a document and return its content as markdown and an optional raw document object. @@ -16,4 +16,3 @@ async def parse(self, file_path: str) -> tuple[str, Optional[Any]]: Returns: Tuple of (markdown_content, raw_document). """ - pass diff --git a/src/core/schemas/chunk.py b/src/core/schemas/chunk.py index aa0f7f6..824d914 100644 --- a/src/core/schemas/chunk.py +++ b/src/core/schemas/chunk.py @@ -1,5 +1,5 @@ from datetime import datetime -from typing import Any, Dict, List, Optional +from typing import Any from pydantic import BaseModel, Field @@ -11,11 +11,11 @@ class Chunk(BaseModel): The id field is database-agnostic - transformations happen in repositories. """ - id: Optional[str] = None + id: str | None = None document_id: str content: str - embedding: Optional[List[float]] = None + embedding: list[float] | None = None chunk_index: int - metadata: Dict[str, Any] = Field(default_factory=dict) - token_count: Optional[int] = None + metadata: dict[str, Any] = Field(default_factory=dict) + token_count: int | None = None created_at: datetime = Field(default_factory=datetime.now) diff --git a/src/core/schemas/document.py b/src/core/schemas/document.py index 2568e5f..1afc5fd 100644 --- a/src/core/schemas/document.py +++ b/src/core/schemas/document.py @@ -1,14 +1,17 @@ from datetime import datetime -from typing import Dict, Any, Optional +from typing import Any + from pydantic import BaseModel, Field + class Document(BaseModel): """Domain model for a source document.""" - id: Optional[str] = Field(None, alias="_id") + + id: str | None = Field(None, alias="_id") title: str source: str content: str - metadata: Dict[str, Any] = Field(default_factory=dict) + metadata: dict[str, Any] = Field(default_factory=dict) created_at: datetime = Field(default_factory=datetime.now) class Config: diff --git a/src/infrastructure/embeddings/openai_embedder.py b/src/infrastructure/embeddings/openai_embedder.py index f22cb4c..52c8dba 100644 --- a/src/infrastructure/embeddings/openai_embedder.py +++ b/src/infrastructure/embeddings/openai_embedder.py @@ -1,5 +1,3 @@ -from typing import List - import openai from src.core.interfaces.embedder import IEmbedder @@ -12,11 +10,11 @@ def __init__(self, api_key: str, model: str, base_url: str): self.client = openai.AsyncOpenAI(api_key=api_key, base_url=base_url) self.model = model - async def get_embedding(self, text: str) -> List[float]: + async def get_embedding(self, text: str) -> list[float]: response = await self.client.embeddings.create(model=self.model, input=text) return response.data[0].embedding - async def get_embeddings(self, texts: List[str]) -> List[List[float]]: + async def get_embeddings(self, texts: list[str]) -> list[list[float]]: if not texts: return [] diff --git a/src/services/agent_service.py b/src/services/agent_service.py index 47af92c..a82bda0 100644 --- a/src/services/agent_service.py +++ b/src/services/agent_service.py @@ -11,8 +11,8 @@ """ import logging +from collections.abc import AsyncIterator from dataclasses import dataclass, field -from typing import AsyncIterator, List from src.core.interfaces.llm import ILLMProvider from src.core.schemas.search import SearchHit @@ -28,7 +28,7 @@ class AgentResult: stream: AsyncIterator[str] searched: bool = False search_query: str | None = None - matches: List[SearchHit] = field(default_factory=list) + matches: list[SearchHit] = field(default_factory=list) async def collect(self) -> str: """Consume the stream and return the full response as text.""" @@ -157,7 +157,7 @@ async def _react_loop( full_system = f"{react_system}\n\nResponse rules:\n{system_prompt}" scratchpad = "" - last_matches: List[SearchHit] = [] + last_matches: list[SearchHit] = [] last_search_query: str | None = None for step in range(self.max_steps): @@ -236,7 +236,7 @@ async def _stream() -> AsyncIterator[str]: return _stream() return response - def _format_observation(self, hits: List[SearchHit]) -> str: + def _format_observation(self, hits: list[SearchHit]) -> str: if not hits: return "No relevant documents were found." diff --git a/src/services/context_builder.py b/src/services/context_builder.py index 3dba9d7..4c3863c 100644 --- a/src/services/context_builder.py +++ b/src/services/context_builder.py @@ -8,7 +8,6 @@ import logging from dataclasses import dataclass, field -from typing import List from src.core.schemas.search import SearchHit @@ -30,7 +29,7 @@ class ContextResult: """Result from context building.""" context: str - citations: List[Citation] = field(default_factory=list) + citations: list[Citation] = field(default_factory=list) truncated: bool = False total_hits: int = 0 included_hits: int = 0 @@ -56,7 +55,7 @@ def __init__( self.max_chars = max_chars self.max_per_document = max_per_document - def build(self, hits: List[SearchHit]) -> ContextResult: + def build(self, hits: list[SearchHit]) -> ContextResult: """Build context from search hits. Args: diff --git a/src/settings.py b/src/settings.py index 2c297c5..0032597 100644 --- a/src/settings.py +++ b/src/settings.py @@ -1,7 +1,5 @@ """Settings configuration for MongoDB RAG Agent.""" -from typing import Optional - from dotenv import load_dotenv from pydantic import Field from pydantic_settings import BaseSettings @@ -19,9 +17,7 @@ class Settings(BaseSettings): ) # MongoDB Configuration - mongodb_uri: Optional[str] = Field( - None, description="MongoDB Atlas connection string" - ) + mongodb_uri: str | None = Field(None, description="MongoDB Atlas connection string") mongodb_database: str = Field(default="rag_db", description="MongoDB database name") @@ -44,8 +40,8 @@ class Settings(BaseSettings): ) # Supabase Configuration - supabase_url: Optional[str] = Field(None, description="Supabase project URL") - supabase_key: Optional[str] = Field(None, description="Supabase API key") + supabase_url: str | None = Field(None, description="Supabase project URL") + supabase_key: str | None = Field(None, description="Supabase API key") # LLM Configuration (Generic OpenAI-compatible) llm_api_key: str = Field(..., description="API key for the LLM provider") @@ -91,14 +87,14 @@ def load_settings() -> Settings: settings = Settings() # Validation based on db_type - if settings.db_type == "mongo": - if not settings.mongodb_uri: - raise ValueError("MONGODB_URI is required when DB_TYPE is 'mongo'") - elif settings.db_type == "supabase": - if not settings.supabase_url or not settings.supabase_key: - raise ValueError( - "SUPABASE_URL and SUPABASE_KEY are required when DB_TYPE is 'supabase'" - ) + if settings.db_type == "mongo" and not settings.mongodb_uri: + raise ValueError("MONGODB_URI is required when DB_TYPE is 'mongo'") + elif settings.db_type == "supabase" and ( + not settings.supabase_url or not settings.supabase_key + ): + raise ValueError( + "SUPABASE_URL and SUPABASE_KEY are required when DB_TYPE is 'supabase'" + ) return settings except Exception as e: From 40cf60e7bb9fe327081fc289b3976aea936bb261 Mon Sep 17 00:00:00 2001 From: Fran BlakIA Date: Sat, 8 Aug 2026 13:18:19 +0200 Subject: [PATCH 2/4] =?UTF-8?q?refactor(core)!:=20separar=20acceso=20destr?= =?UTF-8?q?uctivo=20y=20mover=20el=20umbral=20a=20la=20aplicaci=C3=B3n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cierra tres problemas de diseño abiertos en el propio repositorio. #12 — clean_all() colgaba de IRepository, así que cualquier consumidor que solo necesitara leer o guardar recibía además la capacidad de borrar la base de datos entera. Se extrae a IAdminRepository, que ya existía pero no se estaba usando. Los adaptadores concretos implementan ambos puertos; los consumidores dependen del estrecho. IngestService recibe el puerto de administración de forma opcional y clean() falla con un mensaje claro si no se le inyectó: la ingesta no necesita permisos destructivos para hacer su trabajo. #14 — el umbral de similitud vivía como estado del adaptador de Supabase, que lo aplicaba en silencio por encima de lo que pidiera quien llamaba. Pasa a viajar en SearchOptions desde la capa de aplicación, de modo que el mismo repositorio puede atender una consulta estricta y otra permisiva sin reconfigurarse. El adaptador conserva solo una constante de último recurso, no una política. #18 — docling_chunker importaba docling y transformers en la cabecera del módulo, así que el extra opcional 'ingestion' dejaba de serlo en cuanto algo importaba el paquete. Las importaciones pasan a __init__, con un ImportError explicativo. Se corrige además un 'except:' desnudo en mongo_repository que se tragaba cualquier excepción, incluida KeyboardInterrupt, y convertía un fallo de serialización en un document_id mal tipado que solo se manifestaba más tarde como un join que no casaba nada. 13 tests nuevos cubren los tres límites. BREAKING CHANGE: IRepository ya no expone clean_all(). Quien la necesite debe depender de IAdminRepository. SupabaseRepository ya no acepta 'threshold' en el constructor. --- src/bootstrap.py | 17 +- src/core/dtos/__init__.py | 6 +- src/core/interfaces/admin_repository.py | 2 - src/core/interfaces/repository.py | 31 +-- .../database/mongo_repository.py | 39 +++- .../database/supabase_repository.py | 75 ++++--- .../ingestion/docling_chunker.py | 28 ++- .../ingestion/docling_parser.py | 26 +-- src/services/ingest_service.py | 31 ++- src/services/rag_service.py | 37 +++- tests/conftest.py | 52 +++-- tests/test_agent_service.py | 11 +- tests/test_context_builder.py | 3 +- tests/test_rag_service.py | 10 +- tests/test_repository_boundaries.py | 192 ++++++++++++++++++ 15 files changed, 431 insertions(+), 129 deletions(-) create mode 100644 tests/test_repository_boundaries.py diff --git a/src/bootstrap.py b/src/bootstrap.py index bab4bac..7fcd5ee 100644 --- a/src/bootstrap.py +++ b/src/bootstrap.py @@ -1,3 +1,4 @@ +from src.core.dtos import SearchOptions from src.services.ingest_service import IngestService from src.services.rag_service import RAGService from src.settings import load_settings @@ -15,7 +16,6 @@ def _get_repository(settings): return SupabaseRepository( url=settings.supabase_url, key=settings.supabase_key, - threshold=settings.semantic_match_threshold, ) else: from src.infrastructure.database.mongo_repository import MongoRepository @@ -53,7 +53,12 @@ def bootstrap_rag_service() -> RAGService: max_per_document=2, ) - return RAGService(repository, llm, embedder, context_builder) + # The similarity threshold is retrieval policy, so it is configured here + # in the application layer and travels with each query, instead of being + # baked into the adapter at construction time. + default_options = SearchOptions(threshold=settings.semantic_match_threshold) + + return RAGService(repository, llm, embedder, context_builder, default_options) def bootstrap_ingest_service() -> IngestService: @@ -61,8 +66,8 @@ def bootstrap_ingest_service() -> IngestService: repository = _get_repository(settings) from src.infrastructure.embeddings.openai_embedder import OpenAIEmbedder - from src.infrastructure.ingestion.docling_parser import DoclingParser from src.infrastructure.ingestion.docling_chunker import DoclingChunker + from src.infrastructure.ingestion.docling_parser import DoclingParser embedder = OpenAIEmbedder( api_key=settings.embedding_api_key, @@ -73,7 +78,11 @@ def bootstrap_ingest_service() -> IngestService: parser = DoclingParser() chunker = DoclingChunker(max_tokens=settings.embedding_dimension) - return IngestService(repository, embedder, parser, chunker) + # The concrete repository also implements IAdminRepository, so ingestion + # gets destructive access explicitly rather than by accident. + return IngestService( + repository, embedder, parser, chunker, admin_repository=repository + ) def bootstrap_agent_service(): diff --git a/src/core/dtos/__init__.py b/src/core/dtos/__init__.py index 4f4b7c3..220a2a7 100644 --- a/src/core/dtos/__init__.py +++ b/src/core/dtos/__init__.py @@ -5,7 +5,7 @@ """ from dataclasses import dataclass, field -from typing import Any, List +from typing import Any from src.core.schemas.search import SearchHit, SearchType @@ -28,7 +28,7 @@ class SearchResponse: """Response from a search operation.""" query: str - hits: List[SearchHit] + hits: list[SearchHit] total_hits: int search_type: SearchType @@ -47,5 +47,5 @@ class ContextResult: """Result from context building with citations.""" context: str - citations: List[Citation] = field(default_factory=list) + citations: list[Citation] = field(default_factory=list) truncated: bool = False diff --git a/src/core/interfaces/admin_repository.py b/src/core/interfaces/admin_repository.py index 0a1d780..2db1643 100644 --- a/src/core/interfaces/admin_repository.py +++ b/src/core/interfaces/admin_repository.py @@ -23,7 +23,6 @@ async def clean_all(self) -> None: Use with caution - this operation is irreversible. Typically used for testing or resetting the database. """ - pass @abstractmethod async def get_stats(self) -> dict: @@ -32,4 +31,3 @@ async def get_stats(self) -> dict: Returns: Dict with keys like 'document_count', 'chunk_count', 'storage_bytes'. """ - pass diff --git a/src/core/interfaces/repository.py b/src/core/interfaces/repository.py index 15f13b8..fc22880 100644 --- a/src/core/interfaces/repository.py +++ b/src/core/interfaces/repository.py @@ -6,7 +6,6 @@ """ from abc import ABC, abstractmethod -from typing import List from src.core.schemas.chunk import Chunk from src.core.schemas.document import Document @@ -18,6 +17,12 @@ class IRepository(ABC): All methods are async to allow non-blocking I/O operations. Implementations should handle connection management internally. + + Destructive operations live in ``IAdminRepository``, not here. Anything + that depends on this interface can persist and retrieve, and cannot wipe + the database -- a capability nothing in the retrieval path needs, and one + that is far too easy to reach for by accident when it sits on the same + object. """ @abstractmethod @@ -34,10 +39,9 @@ async def save_document(self, document: Document) -> str: Raises: DocumentSaveError: If the document fails to save. """ - pass @abstractmethod - async def save_chunks(self, chunks: List[Chunk]) -> None: + async def save_chunks(self, chunks: list[Chunk]) -> None: """Save a batch of document chunks. Args: @@ -46,12 +50,11 @@ async def save_chunks(self, chunks: List[Chunk]) -> None: Raises: ChunkSaveError: If chunks fail to save. """ - pass @abstractmethod async def semantic_search( - self, vector: List[float], limit: int, threshold: float | None = None - ) -> List[SearchHit]: + self, vector: list[float], limit: int, threshold: float | None = None + ) -> list[SearchHit]: """Perform semantic vector search. Args: @@ -66,10 +69,9 @@ async def semantic_search( Raises: SearchError: If database query fails. """ - pass @abstractmethod - async def text_search(self, query: str, limit: int) -> List[SearchHit]: + async def text_search(self, query: str, limit: int) -> list[SearchHit]: """Perform full-text keyword search. Args: @@ -83,16 +85,3 @@ async def text_search(self, query: str, limit: int) -> List[SearchHit]: Raises: SearchError: If database query fails. """ - pass - - @abstractmethod - async def clean_all(self) -> None: - """Clear all documents and chunks. - - Use with caution - this operation is irreversible. - Typically used for testing or resetting the database. - - Raises: - RepositoryError: If cleanup fails. - """ - pass diff --git a/src/infrastructure/database/mongo_repository.py b/src/infrastructure/database/mongo_repository.py index 69ec089..f5a3e9e 100644 --- a/src/infrastructure/database/mongo_repository.py +++ b/src/infrastructure/database/mongo_repository.py @@ -1,10 +1,11 @@ import logging from datetime import datetime -from typing import List from bson import ObjectId +from bson.errors import InvalidId from motor.motor_asyncio import AsyncIOMotorClient +from src.core.interfaces.admin_repository import IAdminRepository from src.core.interfaces.repository import IRepository from src.core.schemas.chunk import Chunk from src.core.schemas.document import Document @@ -13,8 +14,12 @@ logger = logging.getLogger(__name__) -class MongoRepository(IRepository): - """MongoDB implementation of the repository interface.""" +class MongoRepository(IRepository, IAdminRepository): + """MongoDB implementation of the repository and admin interfaces. + + Implementing both is fine for a concrete adapter: what matters is that + consumers depend on the narrower interface they actually need. + """ def __init__( self, uri: str, db_name: str, doc_collection: str, chunk_collection: str @@ -29,25 +34,32 @@ async def save_document(self, document: Document) -> str: result = await self.documents.insert_one(doc_dict) return str(result.inserted_id) - async def save_chunks(self, chunks: List[Chunk]) -> None: + async def save_chunks(self, chunks: list[Chunk]) -> None: if not chunks: return chunk_dicts = [] for chunk in chunks: cd = chunk.model_dump(exclude={"id"}, by_alias=True, mode="json") - # Ensure document_id is ObjectId if it's a string from Mongo + # Ensure document_id is an ObjectId when it arrives as a string. + # The catch is narrow on purpose: a bare 'except' here also + # swallowed KeyboardInterrupt and any genuine bug in serialisation, + # turning them into a silently mistyped document_id that only + # surfaced later as a join that matched nothing. try: cd["document_id"] = ObjectId(cd["document_id"]) - except: - pass + except (InvalidId, TypeError): + logger.warning( + "document_id %r is not a valid ObjectId; storing it unchanged", + cd.get("document_id"), + ) chunk_dicts.append(cd) await self.chunks.insert_many(chunk_dicts, ordered=False) async def semantic_search( - self, vector: List[float], limit: int, threshold: float | None = None - ) -> List[SearchHit]: + self, vector: list[float], limit: int, threshold: float | None = None + ) -> list[SearchHit]: index_name = "vector_index" pipeline = [ { @@ -101,7 +113,7 @@ async def semantic_search( ) return results - async def text_search(self, query: str, limit: int) -> List[SearchHit]: + async def text_search(self, query: str, limit: int) -> list[SearchHit]: index_name = "text_index" pipeline = [ { @@ -162,5 +174,12 @@ async def clean_all(self) -> None: await self.chunks.delete_many({}) await self.documents.delete_many({}) + async def get_stats(self) -> dict: + """Return document and chunk counts.""" + return { + "document_count": await self.documents.count_documents({}), + "chunk_count": await self.chunks.count_documents({}), + } + async def close(self): self.client.close() diff --git a/src/infrastructure/database/supabase_repository.py b/src/infrastructure/database/supabase_repository.py index 29636fc..43633aa 100644 --- a/src/infrastructure/database/supabase_repository.py +++ b/src/infrastructure/database/supabase_repository.py @@ -8,11 +8,11 @@ import asyncio import logging -from typing import List from supabase import Client, create_client from src.core.exceptions import ChunkSaveError, DocumentSaveError, SearchError +from src.core.interfaces.admin_repository import IAdminRepository from src.core.interfaces.repository import IRepository from src.core.schemas.chunk import Chunk from src.core.schemas.document import Document @@ -20,17 +20,23 @@ logger = logging.getLogger(__name__) +# Last-resort value when no caller supplies a threshold. It is deliberately a +# module constant and not instance state: the similarity cut-off is a retrieval +# policy that belongs to the application layer (``SearchOptions``), and an +# adapter that stores its own copy silently overrides whatever the caller +# decided. +DEFAULT_SEMANTIC_THRESHOLD = 0.3 -class SupabaseRepository(IRepository): + +class SupabaseRepository(IRepository, IAdminRepository): """Supabase/PostgreSQL implementation of the repository interface. Uses pgvector for semantic search and PostgreSQL full-text search. All sync operations are wrapped with asyncio.to_thread() to be non-blocking. """ - def __init__(self, url: str, key: str, threshold: float = 0.3): + def __init__(self, url: str, key: str): self.client: Client = create_client(url, key) - self.threshold = threshold async def save_document(self, document: Document) -> str: """Save a document and return its ID. @@ -51,7 +57,7 @@ async def save_document(self, document: Document) -> str: ) return str(result.data[0]["id"]) - async def save_chunks(self, chunks: List[Chunk]) -> None: + async def save_chunks(self, chunks: list[Chunk]) -> None: """Save a batch of document chunks using upsert. Uses upsert with (document_id, chunk_index) constraint to prevent @@ -72,9 +78,11 @@ async def save_chunks(self, chunks: List[Chunk]) -> None: # Use upsert to handle re-ingestion without duplicates result = await asyncio.to_thread( - lambda: self.client.table("chunks") - .upsert(chunk_dicts, on_conflict="document_id,chunk_index") - .execute() + lambda: ( + self.client.table("chunks") + .upsert(chunk_dicts, on_conflict="document_id,chunk_index") + .execute() + ) ) if not result.data: @@ -85,19 +93,25 @@ async def save_chunks(self, chunks: List[Chunk]) -> None: ) async def semantic_search( - self, vector: List[float], limit: int, threshold: float | None = None - ) -> List[SearchHit]: + self, vector: list[float], limit: int, threshold: float | None = None + ) -> list[SearchHit]: """Perform semantic search using pgvector via RPC. Args: vector: Query embedding vector. limit: Maximum results to return. - threshold: Optional override for similarity threshold. + threshold: Similarity threshold for this query. Comes from + ``SearchOptions`` in the application layer. When omitted, the + adapter falls back to ``DEFAULT_SEMANTIC_THRESHOLD`` rather + than to configured state of its own: the adapter no longer + holds a policy, only a last-resort constant. Returns: List of SearchHit with semantic_score populated. """ - effective_threshold = threshold if threshold is not None else self.threshold + effective_threshold = ( + threshold if threshold is not None else DEFAULT_SEMANTIC_THRESHOLD + ) rpc_params = { "query_embedding": vector, "match_threshold": effective_threshold, @@ -105,7 +119,9 @@ async def semantic_search( } logger.debug( - f"Semantic search: vector_dim={len(vector)}, threshold={effective_threshold}" + "Semantic search: vector_dim=%s, threshold=%s", + len(vector), + effective_threshold, ) try: @@ -136,7 +152,7 @@ async def semantic_search( ) return hits - async def text_search(self, query: str, limit: int) -> List[SearchHit]: + async def text_search(self, query: str, limit: int) -> list[SearchHit]: """Perform full-text search using PostgreSQL RPC. Returns: @@ -179,19 +195,33 @@ async def text_search(self, query: str, limit: int) -> List[SearchHit]: ) return hits + async def get_stats(self) -> dict: + """Return document and chunk counts.""" + docs = await asyncio.to_thread( + lambda: self.client.table("documents").select("id", count="exact").execute() + ) + chunks = await asyncio.to_thread( + lambda: self.client.table("chunks").select("id", count="exact").execute() + ) + return {"document_count": docs.count, "chunk_count": chunks.count} + async def clean_all(self) -> None: """Clear all documents and chunks from Supabase.""" await asyncio.to_thread( - lambda: self.client.table("chunks") - .delete() - .neq("id", "00000000-0000-0000-0000-000000000000") - .execute() + lambda: ( + self.client.table("chunks") + .delete() + .neq("id", "00000000-0000-0000-0000-000000000000") + .execute() + ) ) await asyncio.to_thread( - lambda: self.client.table("documents") - .delete() - .neq("id", "00000000-0000-0000-0000-000000000000") - .execute() + lambda: ( + self.client.table("documents") + .delete() + .neq("id", "00000000-0000-0000-0000-000000000000") + .execute() + ) ) async def close(self) -> None: @@ -200,4 +230,3 @@ async def close(self) -> None: Supabase client doesn't require explicit closing, but this hook exists for interface consistency. """ - pass diff --git a/src/infrastructure/ingestion/docling_chunker.py b/src/infrastructure/ingestion/docling_chunker.py index 0e7f92c..6d86d87 100644 --- a/src/infrastructure/ingestion/docling_chunker.py +++ b/src/infrastructure/ingestion/docling_chunker.py @@ -1,8 +1,15 @@ -import logging -from typing import Any, Dict, List, Optional +"""Docling-backed chunker. + +Docling and transformers are heavy optional dependencies (they pull in the +whole model stack). They are imported inside ``__init__`` rather than at module +level so that importing this module -- which happens through package imports +and during test collection -- does not require the ``ingestion`` extra to be +installed. Anyone who actually constructs a ``DoclingChunker`` needs them, and +gets a clear message if they are missing. +""" -from transformers import AutoTokenizer -from docling.chunking import HybridChunker +import logging +from typing import Any from src.core.interfaces.chunker import IChunker, RawChunk @@ -24,6 +31,15 @@ def __init__( max_tokens: Maximum tokens per chunk. model_id: Tokenizer model ID. """ + try: + from docling.chunking import HybridChunker + from transformers import AutoTokenizer + except ImportError as exc: # pragma: no cover - depends on the extra + raise ImportError( + "DoclingChunker requires the optional 'ingestion' extra. " + "Install it with: pip install -e '.[ingestion]'" + ) from exc + self.max_tokens = max_tokens self.tokenizer = AutoTokenizer.from_pretrained(model_id) self.chunker = HybridChunker( @@ -32,8 +48,8 @@ def __init__( logger.info(f"DoclingChunker initialized (max_tokens={max_tokens})") async def chunk_document( - self, content: str, title: str, source: str, docling_doc: Optional[Any] = None - ) -> List[RawChunk]: + self, content: str, title: str, source: str, docling_doc: Any | None = None + ) -> list[RawChunk]: """Chunk a document using Docling's HybridChunker.""" if not content.strip(): return [] diff --git a/src/infrastructure/ingestion/docling_parser.py b/src/infrastructure/ingestion/docling_parser.py index 3d48773..e308675 100644 --- a/src/infrastructure/ingestion/docling_parser.py +++ b/src/infrastructure/ingestion/docling_parser.py @@ -1,7 +1,7 @@ import logging import os from pathlib import Path -from typing import Any, Optional +from typing import Any from src.core.interfaces.parser import IParser @@ -15,7 +15,7 @@ def __init__(self): """Initialize the Docling parser.""" self._initialized = False - async def parse(self, file_path: str) -> tuple[str, Optional[Any]]: + async def parse(self, file_path: str) -> tuple[str, Any | None]: """ Parse a document using Docling. @@ -52,7 +52,9 @@ async def parse(self, file_path: str) -> tuple[str, Optional[Any]]: from docling.document_converter import DocumentConverter logger.info( - f"Converting {file_ext} file using Docling: {os.path.basename(file_path)}" + "Converting %s file using Docling: %s", + file_ext, + os.path.basename(file_path), ) # In a real production scenario, we might want to reuse the converter @@ -72,7 +74,7 @@ async def parse(self, file_path: str) -> tuple[str, Optional[Any]]: logger.error(f"Failed to convert {file_path} with Docling: {e}") logger.warning(f"Falling back to raw text extraction for {file_path}") try: - with open(file_path, "r", encoding="utf-8") as f: + with open(file_path, encoding="utf-8") as f: return (f.read(), None) except Exception: return ( @@ -83,18 +85,18 @@ async def parse(self, file_path: str) -> tuple[str, Optional[Any]]: # Text-based formats else: try: - with open(file_path, "r", encoding="utf-8") as f: + with open(file_path, encoding="utf-8") as f: return (f.read(), None) except UnicodeDecodeError: - with open(file_path, "r", encoding="latin-1") as f: + with open(file_path, encoding="latin-1") as f: return (f.read(), None) - async def _transcribe_audio(self, file_path: str) -> tuple[str, Optional[Any]]: + async def _transcribe_audio(self, file_path: str) -> tuple[str, Any | None]: """Transcribe audio file using Whisper ASR via Docling.""" try: + from docling.datamodel import asr_model_specs from docling.datamodel.base_models import InputFormat from docling.datamodel.pipeline_options import AsrPipelineOptions - from docling.datamodel import asr_model_specs from docling.document_converter import AudioFormatOption, DocumentConverter from docling.pipeline.asr_pipeline import AsrPipeline @@ -125,8 +127,6 @@ async def _transcribe_audio(self, file_path: str) -> tuple[str, Optional[Any]]: return (markdown_content, result.document) except Exception as e: - logger.error(f"Failed to transcribe {file_path} with Whisper ASR: {e}") - return ( - f"[Error: Could not transcribe audio file {os.path.basename(file_path)}]", - None, - ) + logger.error("Failed to transcribe %s with Whisper ASR: %s", file_path, e) + filename = os.path.basename(file_path) + return (f"[Error: Could not transcribe audio file {filename}]", None) diff --git a/src/services/ingest_service.py b/src/services/ingest_service.py index b2e2cdf..55659a6 100644 --- a/src/services/ingest_service.py +++ b/src/services/ingest_service.py @@ -1,11 +1,12 @@ import logging import os -from typing import Dict, Any +from typing import Any +from src.core.interfaces.admin_repository import IAdminRepository +from src.core.interfaces.chunker import IChunker from src.core.interfaces.embedder import IEmbedder -from src.core.interfaces.repository import IRepository from src.core.interfaces.parser import IParser -from src.core.interfaces.chunker import IChunker +from src.core.interfaces.repository import IRepository from src.core.schemas.chunk import Chunk from src.core.schemas.document import Document @@ -21,6 +22,7 @@ def __init__( embedder: IEmbedder, parser: IParser, chunker: IChunker, + admin_repository: IAdminRepository | None = None, ): """ Initialize IngestService. @@ -35,8 +37,9 @@ def __init__( self.embedder = embedder self.parser = parser self.chunker = chunker + self._admin_repository = admin_repository - async def ingest_file(self, file_path: str, metadata: Dict[str, Any] = None): + async def ingest_file(self, file_path: str, metadata: dict[str, Any] = None): """ Process and save a single file. @@ -97,6 +100,20 @@ def _extract_title(self, content: str, file_path: str) -> str: return line[2:].strip() return os.path.splitext(os.path.basename(file_path))[0] - async def clean(self): - """Wipe all documents and chunks from the repository.""" - await self.repository.clean_all() + async def clean(self) -> None: + """Wipe all documents and chunks from the repository. + + Requires an ``IAdminRepository`` to have been injected. Ingestion does + not need destructive access to do its job, so it is not granted by + default: a caller that wants to wipe the database has to ask for that + capability explicitly when wiring the service. + + Raises: + RuntimeError: If no admin repository was provided. + """ + if self._admin_repository is None: + raise RuntimeError( + "clean() requires an admin repository. Construct IngestService " + "with admin_repository=... to enable destructive operations." + ) + await self._admin_repository.clean_all() diff --git a/src/services/rag_service.py b/src/services/rag_service.py index 48d22b6..d0900c2 100644 --- a/src/services/rag_service.py +++ b/src/services/rag_service.py @@ -1,6 +1,7 @@ import logging -from typing import AsyncIterator, List +from collections.abc import AsyncIterator +from src.core.dtos import SearchOptions from src.core.interfaces.embedder import IEmbedder from src.core.interfaces.llm import ILLMProvider from src.core.interfaces.repository import IRepository @@ -19,28 +20,44 @@ def __init__( llm: ILLMProvider, embedder: IEmbedder, context_builder: ContextBuilder = None, + default_options: SearchOptions | None = None, ): self.repository = repository self.llm = llm self.embedder = embedder self.context_builder = context_builder or ContextBuilder() + # Retrieval policy lives here, in the application layer, not inside the + # adapter. Callers may override it per query. + self.default_options = default_options or SearchOptions() async def search( - self, query: str, limit: int = 5, search_type: SearchType = SearchType.HYBRID - ) -> tuple[List[SearchHit], str]: + self, + query: str, + limit: int = 5, + search_type: SearchType = SearchType.HYBRID, + options: SearchOptions | None = None, + ) -> tuple[list[SearchHit], str]: """Orchestrate search across multiple methods and merge results. Args: query: Search query (should be pre-optimized by caller). limit: Maximum number of results to return. search_type: Type of search to perform. + options: Per-query retrieval options. Falls back to the service + default. The similarity threshold travels from here down to + the adapter, so the same repository can serve a strict query + and a permissive one without being reconfigured. Returns: Tuple of (hits, query_used). """ + opts = options or self.default_options + if search_type == SearchType.SEMANTIC: vector = await self.embedder.get_embedding(query) - results = await self.repository.semantic_search(vector, limit) + results = await self.repository.semantic_search( + vector, limit, opts.threshold + ) return results, query elif search_type == SearchType.TEXT: results = await self.repository.text_search(query, limit) @@ -49,7 +66,9 @@ async def search( logger.debug(f"Hybrid search with query: {query}") vector = await self.embedder.get_embedding(query) - semantic_results = await self.repository.semantic_search(vector, limit * 2) + semantic_results = await self.repository.semantic_search( + vector, limit * 2, opts.threshold + ) text_results = await self.repository.text_search(query, limit * 2) merged = self._reciprocal_rank_fusion(semantic_results, text_results) logger.debug(f"Hybrid search merged into {len(merged)} results") @@ -57,10 +76,10 @@ async def search( def _reciprocal_rank_fusion( self, - semantic_hits: List[SearchHit], - text_hits: List[SearchHit], + semantic_hits: list[SearchHit], + text_hits: list[SearchHit], k: int = 60, - ) -> List[SearchHit]: + ) -> list[SearchHit]: """Merge search results using Reciprocal Rank Fusion. Creates NEW SearchHit objects with fusion_score set. @@ -111,7 +130,7 @@ def _reciprocal_rank_fusion( async def answer( self, query: str, system_prompt: str, limit: int = 5 - ) -> tuple[AsyncIterator[str] | str, List[SearchHit], str]: + ) -> tuple[AsyncIterator[str] | str, list[SearchHit], str]: """Find relevant info and generate an answer. Returns: diff --git a/tests/conftest.py b/tests/conftest.py index 73d405a..192a481 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,25 +1,25 @@ """Pytest fixtures and mocks for testing.""" import pytest -from typing import List, Optional -from src.core.interfaces.llm import ILLMProvider, ToolResponse, ToolCall -from src.core.interfaces.repository import IRepository +from src.core.interfaces.admin_repository import IAdminRepository from src.core.interfaces.embedder import IEmbedder +from src.core.interfaces.llm import ILLMProvider, ToolCall, ToolResponse +from src.core.interfaces.repository import IRepository from src.core.schemas.chunk import Chunk from src.core.schemas.search import SearchHit +from src.services.agent_service import AgentService from src.services.context_builder import ContextBuilder from src.services.rag_service import RAGService -from src.services.agent_service import AgentService class MockLLMWithTools(ILLMProvider): """LLM that supports tools and follows a scripted response sequence.""" - def __init__(self, responses: Optional[List[ToolResponse]] = None): - self.responses: List[ToolResponse] = responses if responses is not None else [] + def __init__(self, responses: list[ToolResponse] | None = None): + self.responses: list[ToolResponse] = responses if responses is not None else [] self.call_index = 0 - self.call_history: List[tuple] = [] + self.call_history: list[tuple] = [] def supports_tools(self) -> bool: return True @@ -46,7 +46,7 @@ class MockLLMNoTools(ILLMProvider): def __init__(self, classifier_response: str = "SEARCH"): self.classifier_response = classifier_response - self.call_history: List[tuple] = [] + self.call_history: list[tuple] = [] def supports_tools(self) -> bool: return False @@ -65,16 +65,16 @@ class MockRepository(IRepository): def __init__( self, - semantic_results: Optional[List[SearchHit]] = None, - text_results: Optional[List[SearchHit]] = None, + semantic_results: list[SearchHit] | None = None, + text_results: list[SearchHit] | None = None, ): - self._semantic_results: List[SearchHit] = ( + self._semantic_results: list[SearchHit] = ( semantic_results if semantic_results is not None else [] ) - self._text_results: List[SearchHit] = ( + self._text_results: list[SearchHit] = ( text_results if text_results is not None else [] ) - self.search_history: List[tuple] = [] + self.search_history: list[tuple] = [] async def save_document(self, document) -> str: return "doc-test-id" @@ -83,26 +83,38 @@ async def save_chunks(self, chunks) -> None: pass async def semantic_search( - self, vector: List[float], limit: int, threshold: Optional[float] = None - ) -> List[SearchHit]: - self.search_history.append(("semantic", limit)) + self, vector: list[float], limit: int, threshold: float | None = None + ) -> list[SearchHit]: + # The threshold is recorded so tests can assert that retrieval policy + # actually reaches the adapter instead of being silently dropped. + self.search_history.append(("semantic", limit, threshold)) return self._semantic_results[:limit] - async def text_search(self, query: str, limit: int) -> List[SearchHit]: + async def text_search(self, query: str, limit: int) -> list[SearchHit]: self.search_history.append(("text", query, limit)) return self._text_results[:limit] + +class FakeAdminRepository(IAdminRepository): + """Admin double. Records that destructive access was actually used.""" + + def __init__(self) -> None: + self.clean_all_calls = 0 + async def clean_all(self) -> None: - pass + self.clean_all_calls += 1 + + async def get_stats(self) -> dict: + return {"document_count": 0, "chunk_count": 0} class MockEmbedder(IEmbedder): """Mock embedder that returns fixed vectors.""" - async def get_embedding(self, text: str) -> List[float]: + async def get_embedding(self, text: str) -> list[float]: return [0.1] * 1536 - async def get_embeddings(self, texts: List[str]) -> List[List[float]]: + async def get_embeddings(self, texts: list[str]) -> list[list[float]]: return [[0.1] * 1536 for _ in texts] diff --git a/tests/test_agent_service.py b/tests/test_agent_service.py index 1f8f71e..377a2f5 100644 --- a/tests/test_agent_service.py +++ b/tests/test_agent_service.py @@ -1,17 +1,18 @@ """Tests for AgentService ReAct implementation.""" import pytest -from src.services.agent_service import AgentService, AgentResult -from src.services.rag_service import RAGService -from src.core.interfaces.llm import ToolResponse, ToolCall + +from src.core.interfaces.llm import ToolCall, ToolResponse from src.core.schemas.chunk import Chunk from src.core.schemas.search import SearchHit +from src.services.agent_service import AgentResult, AgentService from src.services.context_builder import ContextBuilder +from src.services.rag_service import RAGService from tests.conftest import ( - MockLLMWithTools, + MockEmbedder, MockLLMNoTools, + MockLLMWithTools, MockRepository, - MockEmbedder, ) diff --git a/tests/test_context_builder.py b/tests/test_context_builder.py index 4991453..0a66d47 100644 --- a/tests/test_context_builder.py +++ b/tests/test_context_builder.py @@ -1,9 +1,8 @@ """Tests for ContextBuilder.""" -import pytest -from src.services.context_builder import ContextBuilder, ContextResult from src.core.schemas.chunk import Chunk from src.core.schemas.search import SearchHit +from src.services.context_builder import ContextBuilder, ContextResult class TestContextBuilder: diff --git a/tests/test_rag_service.py b/tests/test_rag_service.py index 9ce15ff..ac9058a 100644 --- a/tests/test_rag_service.py +++ b/tests/test_rag_service.py @@ -1,11 +1,12 @@ """Tests for RAGService hybrid search and generation.""" import pytest -from src.services.rag_service import RAGService -from src.core.schemas.search import SearchType, SearchHit + from src.core.schemas.chunk import Chunk +from src.core.schemas.search import SearchHit, SearchType from src.services.context_builder import ContextBuilder -from tests.conftest import MockRepository, MockEmbedder, MockLLMWithTools +from src.services.rag_service import RAGService +from tests.conftest import MockEmbedder, MockLLMWithTools, MockRepository class TestHybridSearch: @@ -65,7 +66,8 @@ async def test_semantic_only_search(self): hits, _ = await rag.search("test", limit=5, search_type=SearchType.SEMANTIC) assert len(hits) == 1 - assert repo.search_history == [("semantic", 5)] + # The default SearchOptions threshold travels down to the adapter. + assert repo.search_history == [("semantic", 5, 0.3)] @pytest.mark.asyncio async def test_text_only_search(self): diff --git a/tests/test_repository_boundaries.py b/tests/test_repository_boundaries.py new file mode 100644 index 0000000..9417495 --- /dev/null +++ b/tests/test_repository_boundaries.py @@ -0,0 +1,192 @@ +"""Tests for the boundaries between retrieval and administration. + +These cover the two design fixes that motivated issues #12 and #14: + +* Destructive access is not part of ``IRepository``. Anything that only needs + to persist and retrieve cannot wipe the database. +* The similarity threshold is retrieval policy owned by the application layer + and carried per query, not configuration baked into an adapter. +""" + +import inspect + +import pytest + +from src.core.dtos import SearchOptions +from src.core.interfaces.admin_repository import IAdminRepository +from src.core.interfaces.repository import IRepository +from src.core.schemas.chunk import Chunk +from src.core.schemas.search import SearchHit, SearchType +from src.services.context_builder import ContextBuilder +from src.services.rag_service import RAGService +from tests.conftest import FakeAdminRepository, MockEmbedder, MockRepository + + +def _hit() -> SearchHit: + return SearchHit( + chunk=Chunk(id="c1", document_id="d1", content="test", chunk_index=0), + document_title="Test document", + document_source="test.md", + semantic_score=0.9, + ) + + +class TestAdminSeparation: + """Issue #12: clean_all() must not hang off the retrieval interface.""" + + def test_retrieval_interface_has_no_destructive_method(self): + assert not hasattr(IRepository, "clean_all"), ( + "clean_all() is back on IRepository. Destructive access belongs to " + "IAdminRepository so that retrieval consumers cannot reach it." + ) + + def test_admin_interface_owns_destructive_method(self): + assert hasattr(IAdminRepository, "clean_all") + assert hasattr(IAdminRepository, "get_stats") + + def test_retrieval_interface_keeps_its_own_methods(self): + for name in ("save_document", "save_chunks", "semantic_search", "text_search"): + assert hasattr(IRepository, name) + + def test_concrete_adapters_declare_both_ports(self): + """A concrete adapter may implement both; consumers depend on one.""" + from src.infrastructure.database import mongo_repository, supabase_repository + + for module, cls_name in ( + (mongo_repository, "MongoRepository"), + (supabase_repository, "SupabaseRepository"), + ): + cls = getattr(module, cls_name) + bases = inspect.getmro(cls) + assert IRepository in bases, f"{cls_name} must implement IRepository" + assert IAdminRepository in bases, ( + f"{cls_name} must implement IAdminRepository" + ) + + +class TestIngestServiceDestructiveAccess: + """Ingestion only gets destructive powers when they are asked for.""" + + @pytest.mark.asyncio + async def test_clean_fails_loudly_without_an_admin_repository(self): + from src.services.ingest_service import IngestService + + service = IngestService( + repository=MockRepository(), + embedder=MockEmbedder(), + parser=None, + chunker=None, + ) + + with pytest.raises(RuntimeError, match="admin repository"): + await service.clean() + + @pytest.mark.asyncio + async def test_clean_delegates_to_the_admin_repository(self): + from src.services.ingest_service import IngestService + + admin = FakeAdminRepository() + service = IngestService( + repository=MockRepository(), + embedder=MockEmbedder(), + parser=None, + chunker=None, + admin_repository=admin, + ) + + await service.clean() + + assert admin.clean_all_calls == 1 + + +class TestThresholdIsApplicationPolicy: + """Issue #14: the threshold travels with the query.""" + + @pytest.mark.asyncio + async def test_default_options_reach_the_adapter(self): + repo = MockRepository(semantic_results=[_hit()]) + rag = RAGService(repo, None, MockEmbedder(), ContextBuilder()) + + await rag.search("q", limit=3, search_type=SearchType.SEMANTIC) + + assert repo.search_history == [("semantic", 3, 0.3)] + + @pytest.mark.asyncio + async def test_per_query_options_override_the_default(self): + repo = MockRepository(semantic_results=[_hit()]) + rag = RAGService(repo, None, MockEmbedder(), ContextBuilder()) + + await rag.search( + "q", + limit=3, + search_type=SearchType.SEMANTIC, + options=SearchOptions(threshold=0.85), + ) + + assert repo.search_history == [("semantic", 3, 0.85)] + + @pytest.mark.asyncio + async def test_service_level_default_is_configurable(self): + repo = MockRepository(semantic_results=[_hit()]) + rag = RAGService( + repo, + None, + MockEmbedder(), + ContextBuilder(), + default_options=SearchOptions(threshold=0.5), + ) + + await rag.search("q", limit=2, search_type=SearchType.SEMANTIC) + + assert repo.search_history == [("semantic", 2, 0.5)] + + @pytest.mark.asyncio + async def test_hybrid_search_also_carries_the_threshold(self): + repo = MockRepository(semantic_results=[_hit()], text_results=[_hit()]) + rag = RAGService(repo, None, MockEmbedder(), ContextBuilder()) + + await rag.search( + "q", + limit=2, + search_type=SearchType.HYBRID, + options=SearchOptions(threshold=0.7), + ) + + semantic_calls = [c for c in repo.search_history if c[0] == "semantic"] + assert semantic_calls == [("semantic", 4, 0.7)] + + def test_the_adapter_no_longer_stores_a_threshold(self): + """The adapter keeps a last-resort constant, never instance state.""" + from src.infrastructure.database import supabase_repository + + assert hasattr(supabase_repository, "DEFAULT_SEMANTIC_THRESHOLD") + params = inspect.signature( + supabase_repository.SupabaseRepository.__init__ + ).parameters + assert "threshold" not in params, ( + "SupabaseRepository takes a threshold again. Retrieval policy " + "belongs to SearchOptions, not to adapter construction." + ) + + +class TestOptionalIngestionExtra: + """Issue #18: the heavy ingestion stack must stay optional.""" + + def test_docling_chunker_module_imports_without_the_extra(self): + """Importing the module must not require docling or transformers.""" + import importlib + + module = importlib.import_module("src.infrastructure.ingestion.docling_chunker") + assert hasattr(module, "DoclingChunker") + + def test_module_does_not_import_the_extra_at_top_level(self): + from pathlib import Path + + source = Path("src/infrastructure/ingestion/docling_chunker.py").read_text() + header = source.split("class DoclingChunker")[0] + for dependency in ("from docling", "from transformers"): + assert dependency not in header, ( + f"'{dependency}' is imported at module level again. It must be " + "imported inside __init__ so the module loads without the " + "'ingestion' extra." + ) From 66e931da86f5dc6d8a3d15828bde2e9412febee1 Mon Sep 17 00:00:00 2001 From: Fran BlakIA Date: Sat, 8 Aug 2026 13:18:30 +0200 Subject: [PATCH 3/4] =?UTF-8?q?chore:=20a=C3=B1adir=20licencia=20MIT,=20CI?= =?UTF-8?q?=20y=20eliminar=20el=20script=20de=20depuraci=C3=B3n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - LICENSE (MIT): sin ella nadie podía reutilizar legalmente el código de un repositorio cuyo propósito declarado es enseñar el patrón. - CI con ruff y pytest sobre Python 3.10 y 3.12. Se instala sin el extra 'ingestion' a propósito, para que el CI verifique que el proyecto se importa y se testea sin las dependencias pesadas. - Se elimina debug_db.py, un script de depuración suelto en la raíz. - Insignias de CI y licencia en el README. --- .github/workflows/ci.yml | 48 ++++++++++++++++++ LICENSE | 21 ++++++++ README.md | 3 ++ debug_db.py | 62 ----------------------- src/infrastructure/llm/openai_provider.py | 4 +- 5 files changed, 74 insertions(+), 64 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 LICENSE delete mode 100644 debug_db.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..a9313ee --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,48 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + test: + name: Lint y tests + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + python-version: ['3.10', '3.12'] + + steps: + - uses: actions/checkout@v4 + + - name: Instalar uv + uses: astral-sh/setup-uv@v5 + with: + enable-cache: true + + # Un entorno por versión de la matriz. Sin él, uv resolvería contra el + # Python del sistema y las dos jobs correrían sobre el mismo intérprete. + - name: Crear entorno con Python ${{ matrix.python-version }} + run: uv venv --python ${{ matrix.python-version }} + + # Sin el extra 'ingestion' a propósito: docling y transformers son + # opcionales, y el CI comprueba justamente que el proyecto se importa y + # se testea sin ellos. + - name: Instalar dependencias + run: uv pip install -e . 'pytest>=8.3' 'pytest-asyncio>=0.24' 'ruff>=0.8' + + - name: Comprobar formato + run: uv run ruff format --check src/ tests/ + + - name: Comprobar lint + run: uv run ruff check src/ tests/ + + - name: Ejecutar tests + run: uv run pytest -q diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..559583c --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Francisco Manuel Olmedo Cortés + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index 549b013..e19fc90 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,8 @@ # Hybrid RAG Agent - Clean Architecture +[![CI](https://github.com/FullFran/Hybrid-RAG-example/actions/workflows/ci.yml/badge.svg)](https://github.com/FullFran/Hybrid-RAG-example/actions/workflows/ci.yml) +[![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](LICENSE) + Modern and modular RAG (Retrieval-Augmented Generation) system designed under **Clean Architecture** principles. This system enables intelligent document retrieval with total independence from infrastructure providers (Database, LLM, or Embeddings). ## 🏛️ Architecture: Clean RAG Design diff --git a/debug_db.py b/debug_db.py deleted file mode 100644 index 4a56db5..0000000 --- a/debug_db.py +++ /dev/null @@ -1,62 +0,0 @@ -import asyncio -import os - -from dotenv import load_dotenv -from supabase import create_client - -load_dotenv() -url = os.getenv("SUPABASE_URL") -key = os.getenv("SUPABASE_KEY") -client = create_client(url, key) - - -async def check(): - print("Checking database counts...") - docs = client.table("documents").select("count", count="exact").execute() - chunks = client.table("chunks").select("count", count="exact").execute() - - print(f"Documents: {docs.count}") - print(f"Chunks: {chunks.count}") - - if chunks.count > 0: - print("\nSampling first chunk:") - sample = client.table("chunks").select("content, embedding").limit(1).execute() - if sample.data: - content = sample.data[0].get("content", "") - emb = sample.data[0].get("embedding") - print(f"Content: {content[:100]}...") - print(f"Has Embedding: {emb is not None}") - if emb: - # If it's a string, try to parse it - if isinstance(emb, str): - print(f"Embedding is STRING (length: {len(emb)})") - import json - - try: - emb_list = json.loads(emb) - print(f"Parsed as list of length: {len(emb_list)}") - except: - print("Failed to parse string as JSON") - else: - print(f"Embedding is {type(emb)} (length: {len(emb)})") - - # Test RPC - print( - "\nTesting Semantic Search RPC with threshold 0.0 (to see anything)..." - ) - rpc_params = { - "query_embedding": emb_list if isinstance(emb, str) else emb, - "match_threshold": 0.0, - "match_count": 5, - } - rpc_res = client.rpc("match_chunks", rpc_params).execute() - print(f"RPC found {len(rpc_res.data)} matches at 0.0 threshold") - if rpc_res.data: - for i, m in enumerate(rpc_res.data): - print( - f" {i + 1}. Sim: {m.get('similarity')} - {m.get('doc_title')}" - ) - - -if __name__ == "__main__": - asyncio.run(check()) diff --git a/src/infrastructure/llm/openai_provider.py b/src/infrastructure/llm/openai_provider.py index 1f7b6c6..aa7d1e3 100644 --- a/src/infrastructure/llm/openai_provider.py +++ b/src/infrastructure/llm/openai_provider.py @@ -1,5 +1,5 @@ import json -from typing import AsyncIterator, List +from collections.abc import AsyncIterator import openai @@ -29,7 +29,7 @@ async def generate_response( ) return response.choices[0].message.content - async def _stream_response(self, messages: List[dict]) -> AsyncIterator[str]: + async def _stream_response(self, messages: list[dict]) -> AsyncIterator[str]: stream = await self.client.chat.completions.create( model=self.model, messages=messages, stream=True ) From 25011bf8ba759ea67337dc7ae7aa96201e65a8d1 Mon Sep 17 00:00:00 2001 From: Fran BlakIA Date: Sat, 8 Aug 2026 13:20:42 +0200 Subject: [PATCH 4/4] fix(ci): disparar el workflow en la rama por defecto real MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit El filtro apuntaba a 'main', pero la rama por defecto de este repositorio es 'dev-fran', así que el workflow no llegaba a ejecutarse en ninguna PR. El disparador de pull_request pasa a no filtrar por rama: una PR debe validarse apunte a donde apunte. --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9313ee..1fde4df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,10 @@ name: CI on: push: - branches: [main] + # dev-fran es la rama por defecto de este repositorio, no main. + branches: [dev-fran, main] + # Sin filtro de rama: una PR debe validarse apunte a donde apunte. pull_request: - branches: [main] permissions: contents: read