-
Notifications
You must be signed in to change notification settings - Fork 56
Phase 4: ABCs + ports #333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f28b92c
e4e2613
db16704
f169e1e
9f6fcd0
cce4d0d
b9f66e3
9261de8
54d0fb4
44d09bb
52da016
cb6a976
bdd6194
7c08af2
828d389
d16638a
594f847
91cf4c4
0c738b2
11471ac
503fdc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """ChunkingStrategy ABC + registry.""" | ||
|
|
||
| from openrag.core.chunking.chunking_strategy import ChunkingStrategy | ||
| from openrag.core.chunking.registry import chunking_registry | ||
|
|
||
| __all__ = ["ChunkingStrategy", "chunking_registry"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| """Abstract chunking strategy interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from openrag.core.models.chunk import Chunk | ||
| from openrag.core.models.document import ProcessedDocument | ||
|
|
||
|
|
||
| class ChunkingStrategy(ABC): | ||
| """Base class for all chunking strategies.""" | ||
|
|
||
| @abstractmethod | ||
| def chunk(self, document: ProcessedDocument, partition: str = "default") -> list[Chunk]: | ||
| """Split a processed document into chunks.""" | ||
| ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """Chunking strategy registry.""" | ||
|
|
||
| from openrag.core.chunking.chunking_strategy import ChunkingStrategy | ||
| from openrag.core.utils.registry import Registry | ||
|
|
||
| chunking_registry: Registry[ChunkingStrategy] = Registry("chunking") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """Embedder ABC + registry.""" | ||
|
|
||
| from openrag.core.embeddings.embedder import Embedder | ||
| from openrag.core.embeddings.registry import embedder_registry | ||
|
|
||
| __all__ = ["Embedder", "embedder_registry"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| """Abstract embedder interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
|
|
||
| class Embedder(ABC): | ||
| """Base class for all embedding providers.""" | ||
|
|
||
| @abstractmethod | ||
| async def embed(self, texts: list[str]) -> list[list[float]]: | ||
| """Embed a batch of texts, returning vectors.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def embed_single(self, text: str) -> list[float]: | ||
| """Embed a single text.""" | ||
| ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def dimension(self) -> int: | ||
| """Return the embedding dimension.""" | ||
| ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """Embedder registry.""" | ||
|
|
||
| from openrag.core.embeddings.embedder import Embedder | ||
| from openrag.core.utils.registry import Registry | ||
|
|
||
| embedder_registry: Registry[Embedder] = Registry("embedder") | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,6 @@ | ||||||||||
| """DocumentParser ABC + registry.""" | ||||||||||
|
|
||||||||||
| from openrag.core.indexing.parsers.document_parser import DocumentParser | ||||||||||
| from openrag.core.indexing.parsers.registry import parser_registry | ||||||||||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check if the file exists and examine its contents
cat -n openrag/core/indexing/parsers/__init__.py 2>/dev/null || echo "File not found at specified path"Repository: linagora/openrag Length of output: 328 🏁 Script executed: # Search for how imports are done elsewhere in openrag/ for similar cases
rg "^from (openrag\.|\.)" openrag/core/indexing/parsers/ -n --color=never | head -20Repository: linagora/openrag Length of output: 617 🏁 Script executed: # Check the actual structure of the parsers directory
ls -la openrag/core/indexing/parsers/ 2>/dev/null || echo "Directory not found"Repository: linagora/openrag Length of output: 356 🏁 Script executed: # Look for imports in __init__.py files to understand the pattern used in this codebase
rg "^from" openrag/core/indexing/__init__.py -n --color=never
rg "^from" openrag/components/reranker/__init__.py -n --color=neverRepository: linagora/openrag Length of output: 92 🏁 Script executed: # Search for imports of this __init__.py module
rg "from openrag.core.indexing.parsers import|from .parsers import" --type pythonRepository: linagora/openrag Length of output: 90 🏁 Script executed: # Check if there are any runtime issues or error logs mentioning these imports
rg "openrag.core.indexing.parsers" --type python -c | head -20Repository: linagora/openrag Length of output: 90 🏁 Script executed: # Search for imports of this __init__.py module
rg "from openrag.core.indexing.parsers import|from .parsers import"Repository: linagora/openrag Length of output: 42 🏁 Script executed: # Check all uses of openrag.core.indexing.parsers
rg "openrag\.core\.indexing\.parsers"Repository: linagora/openrag Length of output: 626 🏁 Script executed: # Verify the import behavior - check if there's a setup.py or pyproject.toml that affects path resolution
cat openrag/core/indexing/__init__.py 2>/dev/null | head -20Repository: linagora/openrag Length of output: 42 Replace Both modules are in the same package, and the current Proposed fix-from openrag.core.indexing.parsers.document_parser import DocumentParser
-from openrag.core.indexing.parsers.registry import parser_registry
+from .document_parser import DocumentParser
+from .registry import parser_registryPer coding guidelines, absolute imports from the 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| __all__ = ["DocumentParser", "parser_registry"] | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| """Abstract document parser interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from openrag.core.models.document import Document, ProcessedDocument | ||
|
|
||
|
|
||
| class DocumentParser(ABC): | ||
| """Base class for all document parsers (PDF, text, HTML, image, audio, etc.).""" | ||
|
|
||
| @abstractmethod | ||
| async def parse(self, document: Document) -> ProcessedDocument: | ||
| """Parse a document into text blocks and images.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| def supported_types(self) -> list[str]: | ||
| """Return list of DocumentType values this parser handles.""" | ||
| ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """Document parser registry.""" | ||
|
|
||
| from openrag.core.indexing.parsers.document_parser import DocumentParser | ||
| from openrag.core.utils.registry import Registry | ||
|
|
||
| parser_registry: Registry[DocumentParser] = Registry("parser") |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,6 @@ | ||||||||||
| """LLM ABC + registry.""" | ||||||||||
|
|
||||||||||
| from openrag.core.llm.llm import LLM | ||||||||||
| from openrag.core.llm.registry import llm_registry | ||||||||||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Read-only check: inspect import-root conventions and package configuration without importing project modules.
echo "Existing root-style imports:"
rg -n --type=py -C2 '^\s*from (components|core|utils)\.' openrag || true
echo
echo "openrag-prefixed imports:"
rg -n --type=py -C2 '^\s*from openrag\.' openrag || true
echo
echo "Packaging / PYTHONPATH hints:"
rg -n -C2 'PYTHONPATH|pythonpath|package-dir|packages|uvicorn|module' pyproject.toml setup.cfg setup.py Dockerfile docker-compose.yml .github 2>/dev/null || trueRepository: linagora/openrag Length of output: 50372 Replace these imports with relative imports. Lines 3-4 use Proposed fix-from openrag.core.llm.llm import LLM
-from openrag.core.llm.registry import llm_registry
+from .llm import LLM
+from .registry import llm_registryPer coding guidelines, 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| __all__ = ["LLM", "llm_registry"] | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| """Abstract LLM interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| from abc import ABC, abstractmethod | ||
| from collections.abc import AsyncIterator | ||
|
|
||
| from openrag.core.utils.exceptions import LLMParsingError | ||
|
|
||
|
|
||
| class LLM(ABC): | ||
| """Base class for all LLM providers.""" | ||
|
|
||
| @abstractmethod | ||
| async def generate(self, prompt: str, **kwargs) -> str: | ||
| """Generate a completion for a prompt.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def chat(self, messages: list[dict[str, str]], **kwargs) -> str: | ||
| """Chat completion with message list.""" | ||
| ... | ||
|
|
||
| async def generate_json(self, prompt: str, **kwargs) -> dict: | ||
| """Generate a JSON response. Default: parse generate() output. | ||
|
|
||
| Raises LLMParsingError if the LLM output is not valid JSON. | ||
| """ | ||
| response = await self.generate(prompt, **kwargs) | ||
| text = response.strip() | ||
| try: | ||
| return json.loads(text) | ||
| except json.JSONDecodeError as exc: | ||
| raise LLMParsingError( | ||
| raw_response=text, | ||
| parse_error=str(exc), | ||
| ) from exc | ||
|
Comment on lines
+25
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate that Line 33 can return a list/scalar from Proposed fix text = response.strip()
try:
- return json.loads(text)
+ parsed = json.loads(text)
except json.JSONDecodeError as exc:
raise LLMParsingError(
raw_response=text,
parse_error=str(exc),
) from exc
+ if not isinstance(parsed, dict):
+ raise LLMParsingError(
+ raw_response=text,
+ parse_error=f"Expected JSON object, got {type(parsed).__name__}",
+ )
+ return parsed🤖 Prompt for AI Agents |
||
|
|
||
| async def chat_with_tools( | ||
| self, | ||
| messages: list[dict[str, str]], | ||
| tools: list[dict], | ||
| tool_choice: str | dict = "required", | ||
| **kwargs, | ||
| ) -> dict: | ||
| """Chat completion with function calling. | ||
|
|
||
| Only supported by backends with function calling (e.g., vLLM). | ||
| Default raises NotImplementedError. | ||
| """ | ||
| raise NotImplementedError(f"{type(self).__name__} does not support tool calling") | ||
|
|
||
| async def stream_chat(self, messages: list[dict[str, str]], **kwargs) -> AsyncIterator[str]: | ||
| """Stream chat completion. Default falls back to non-streaming.""" | ||
| result = await self.chat(messages, **kwargs) | ||
| yield result | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """LLM registry.""" | ||
|
|
||
| from openrag.core.llm.llm import LLM | ||
| from openrag.core.utils.registry import Registry | ||
|
|
||
| llm_registry: Registry[LLM] = Registry("llm") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| """Port interfaces — repository ABCs + CatalogStore aggregate root.""" | ||
|
|
||
| from openrag.core.ports.audit_log_repo import AuditLogRepository | ||
| from openrag.core.ports.catalog_store import CatalogStore | ||
| from openrag.core.ports.chunk_repo import ChunkRepository | ||
| from openrag.core.ports.conversation_repo import ConversationRepository | ||
| from openrag.core.ports.document_repo import DocumentRepository | ||
| from openrag.core.ports.entity_repo import EntityRepository | ||
| from openrag.core.ports.idempotency_repo import IdempotencyRepository | ||
| from openrag.core.ports.job_repo import JobRepository | ||
| from openrag.core.ports.model_endpoint_repo import ModelEndpointRepository | ||
| from openrag.core.ports.partition_repo import PartitionRepository | ||
| from openrag.core.ports.preset_repo import PresetRepository | ||
| from openrag.core.ports.prompt_repo import PromptRepository | ||
| from openrag.core.ports.topic_tag_repo import TopicTagRepository | ||
| from openrag.core.ports.user_repo import UserRepository | ||
|
|
||
| __all__ = [ | ||
| "AuditLogRepository", | ||
| "CatalogStore", | ||
| "ChunkRepository", | ||
| "ConversationRepository", | ||
| "DocumentRepository", | ||
| "EntityRepository", | ||
| "IdempotencyRepository", | ||
| "JobRepository", | ||
| "ModelEndpointRepository", | ||
| "PartitionRepository", | ||
| "PresetRepository", | ||
| "PromptRepository", | ||
| "TopicTagRepository", | ||
| "UserRepository", | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| """Audit log repository interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
| from typing import Any | ||
|
|
||
|
|
||
| class AuditLogRepository(ABC): | ||
| """Append-only audit trail.""" | ||
|
|
||
| @abstractmethod | ||
| async def insert( | ||
| self, | ||
| user_id: int | None, | ||
| action: str, | ||
| resource_type: str, | ||
| resource_id: str | None = None, | ||
| details_json: dict | None = None, | ||
| request_id: str | None = None, | ||
| ) -> None: ... | ||
|
|
||
| @abstractmethod | ||
| async def query(self, filters: dict[str, Any], offset: int = 0, limit: int = 50) -> list[dict]: ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| """CatalogStore — aggregate root composing all repository ports. | ||
|
|
||
| Concrete implementations (e.g. PostgresStore) own the connection pool | ||
| and compose per-entity repository instances. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from openrag.core.ports.audit_log_repo import AuditLogRepository | ||
| from openrag.core.ports.chunk_repo import ChunkRepository | ||
| from openrag.core.ports.conversation_repo import ConversationRepository | ||
| from openrag.core.ports.document_repo import DocumentRepository | ||
| from openrag.core.ports.entity_repo import EntityRepository | ||
| from openrag.core.ports.idempotency_repo import IdempotencyRepository | ||
| from openrag.core.ports.job_repo import JobRepository | ||
| from openrag.core.ports.model_endpoint_repo import ModelEndpointRepository | ||
| from openrag.core.ports.partition_repo import PartitionRepository | ||
| from openrag.core.ports.preset_repo import PresetRepository | ||
| from openrag.core.ports.prompt_repo import PromptRepository | ||
| from openrag.core.ports.topic_tag_repo import TopicTagRepository | ||
| from openrag.core.ports.user_repo import UserRepository | ||
|
|
||
|
|
||
| class CatalogStore(ABC): | ||
| """Abstract interface for the relational catalog backing store. | ||
|
|
||
| Concrete implementations (e.g. PostgresStore) live in the services layer. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| async def initialize(self) -> None: ... | ||
|
|
||
| @abstractmethod | ||
| async def shutdown(self) -> None: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def document_repo(self) -> DocumentRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def job_repo(self) -> JobRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def user_repo(self) -> UserRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def prompt_repo(self) -> PromptRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def partition_repo(self) -> PartitionRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def model_endpoint_repo(self) -> ModelEndpointRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def preset_repo(self) -> PresetRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def chunk_repo(self) -> ChunkRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def entity_repo(self) -> EntityRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def topic_tag_repo(self) -> TopicTagRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def conversation_repo(self) -> ConversationRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def audit_log_repo(self) -> AuditLogRepository: ... | ||
|
|
||
| @property | ||
| @abstractmethod | ||
| def idempotency_repo(self) -> IdempotencyRepository: ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| """Chunk repository interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
|
|
||
| class ChunkRepository(ABC): | ||
| """Bulk CRUD operations for text chunks.""" | ||
|
|
||
| @abstractmethod | ||
| async def bulk_insert(self, chunks: list[dict]) -> int: | ||
| """Insert multiple chunks. Returns count of inserted rows.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def get_by_ids(self, chunk_ids: list[str]) -> list[dict]: | ||
| """Batch fetch chunks by IDs.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def get_by_document_id(self, document_id: str) -> list[dict]: | ||
| """Fetch all chunks for a document, ordered by chunk_index.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def delete_by_document_id(self, document_id: str) -> int: | ||
| """Delete all chunks belonging to a document. Returns count.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def delete_by_partition(self, partition: str) -> int: | ||
| """Delete all chunks in a partition. Returns count.""" | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def bm25_search(self, query_text: str, partition: str, top_k: int = 20) -> list[dict]: | ||
| """Full-text search using tsvector column with ts_rank scoring.""" | ||
| ... |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| """Conversation repository interface.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from openrag.core.models.conversation import Conversation, Message | ||
|
|
||
|
|
||
| class ConversationRepository(ABC): | ||
| """CRUD operations for conversations and messages.""" | ||
|
|
||
| @abstractmethod | ||
| async def create_conversation(self, conversation: Conversation) -> Conversation: ... | ||
|
|
||
| @abstractmethod | ||
| async def get_conversation(self, conversation_id: str) -> Conversation | None: ... | ||
|
|
||
| @abstractmethod | ||
| async def list_conversations(self, user_id: int, partition: str | None = None) -> list[Conversation]: ... | ||
|
|
||
| @abstractmethod | ||
| async def delete_conversation(self, conversation_id: str) -> bool: ... | ||
|
|
||
| @abstractmethod | ||
| async def add_message(self, message: Message) -> Message: ... | ||
|
|
||
| @abstractmethod | ||
| async def list_messages(self, conversation_id: str) -> list[Message]: ... | ||
|
Comment on lines
+16
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Carry user and partition scope through item-level conversation operations.
Suggested scoped contract shape- async def get_conversation(self, conversation_id: str) -> Conversation | None: ...
+ async def get_conversation(
+ self,
+ conversation_id: str,
+ user_id: int,
+ partition: str | None = None,
+ ) -> Conversation | None: ...
@@
- async def delete_conversation(self, conversation_id: str) -> bool: ...
+ async def delete_conversation(
+ self,
+ conversation_id: str,
+ user_id: int,
+ partition: str | None = None,
+ ) -> bool: ...
@@
- async def add_message(self, message: Message) -> Message: ...
+ async def add_message(
+ self,
+ message: Message,
+ user_id: int,
+ partition: str | None = None,
+ ) -> Message: ...
@@
- async def list_messages(self, conversation_id: str) -> list[Message]: ...
+ async def list_messages(
+ self,
+ conversation_id: str,
+ user_id: int,
+ partition: str | None = None,
+ ) -> list[Message]: ...As per coding guidelines, use token-based authentication with role-based access control for multi-tenant partition access. 🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: linagora/openrag
Length of output: 292
🏁 Script executed:
# Verify the target modules exist ls -la openrag/core/embeddings/embedder.py ls -la openrag/core/utils/registry.pyRepository: linagora/openrag
Length of output: 223
🏁 Script executed:
Repository: linagora/openrag
Length of output: 720
🏁 Script executed:
Repository: linagora/openrag
Length of output: 715
🏁 Script executed:
# Check if there are any other imports or issues in the file wc -l openrag/core/embeddings/registry.py cat -n openrag/core/embeddings/registry.pyRepository: linagora/openrag
Length of output: 330
Use absolute imports without the
openrag.prefix.Line 3 imports from the same package and should use a relative import. Line 4 imports from a cross-package module and should use an absolute import rooted at
openrag/, without theopenrag.prefix—this will fail at runtime with the current format.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents