Compare commits
3 commits
main
...
claude/rel
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
30c6cf5d29 | ||
|
|
6d6d077ed3 | ||
|
|
e1016023c3 |
11 changed files with 259 additions and 24 deletions
6
.github/workflows/ci.yml
vendored
6
.github/workflows/ci.yml
vendored
|
|
@ -29,7 +29,7 @@ jobs:
|
|||
with:
|
||||
python-version: "3.12"
|
||||
cache: pip
|
||||
cache-dependency-path: document-parser/requirements.txt
|
||||
cache-dependency-path: document-parser/requirements-test.txt
|
||||
|
||||
- name: Install system dependencies
|
||||
run: sudo apt-get update && sudo apt-get install -y --no-install-recommends poppler-utils
|
||||
|
|
@ -37,8 +37,8 @@ jobs:
|
|||
- name: Install Python dependencies
|
||||
run: |
|
||||
pip install --upgrade pip
|
||||
pip install -r requirements.txt
|
||||
pip install pytest pytest-asyncio httpx ruff
|
||||
pip install -r requirements-test.txt
|
||||
pip install httpx ruff
|
||||
|
||||
- name: Lint
|
||||
run: ruff check .
|
||||
|
|
|
|||
|
|
@ -8,6 +8,10 @@ from __future__ import annotations
|
|||
|
||||
from dataclasses import dataclass, field
|
||||
|
||||
# US Letter page dimensions (points) — fallback when page size is unknown
|
||||
DEFAULT_PAGE_WIDTH: float = 612.0
|
||||
DEFAULT_PAGE_HEIGHT: float = 792.0
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class PageElement:
|
||||
|
|
|
|||
|
|
@ -36,6 +36,8 @@ from docling_core.types.doc import (
|
|||
)
|
||||
|
||||
from domain.value_objects import (
|
||||
DEFAULT_PAGE_HEIGHT,
|
||||
DEFAULT_PAGE_WIDTH,
|
||||
ConversionOptions,
|
||||
ConversionResult,
|
||||
PageDetail,
|
||||
|
|
@ -50,10 +52,6 @@ logger = logging.getLogger(__name__)
|
|||
# Uses a timeout to prevent a frozen conversion from blocking all others.
|
||||
_converter_lock = threading.Lock()
|
||||
|
||||
# US Letter page dimensions (points) — fallback when page size is unknown
|
||||
_DEFAULT_PAGE_WIDTH = 612.0
|
||||
_DEFAULT_PAGE_HEIGHT = 792.0
|
||||
|
||||
# Default converter (lazy-init on first request)
|
||||
_default_converter: DoclingConverter | None = None
|
||||
|
||||
|
|
@ -175,11 +173,11 @@ def _process_content_item(
|
|||
logger.warning(
|
||||
"Page %d not found in document metadata — using US Letter fallback (%sx%s pt)",
|
||||
page_no,
|
||||
_DEFAULT_PAGE_WIDTH,
|
||||
_DEFAULT_PAGE_HEIGHT,
|
||||
DEFAULT_PAGE_WIDTH,
|
||||
DEFAULT_PAGE_HEIGHT,
|
||||
)
|
||||
pages[page_no] = PageDetail(
|
||||
page_number=page_no, width=_DEFAULT_PAGE_WIDTH, height=_DEFAULT_PAGE_HEIGHT
|
||||
page_number=page_no, width=DEFAULT_PAGE_WIDTH, height=DEFAULT_PAGE_HEIGHT
|
||||
)
|
||||
|
||||
page_height = pages[page_no].height
|
||||
|
|
@ -248,10 +246,10 @@ def _convert_sync(
|
|||
pages_detail = [
|
||||
PageDetail(
|
||||
page_number=i + 1,
|
||||
width=doc.pages[i + 1].size.width if (i + 1) in doc.pages else _DEFAULT_PAGE_WIDTH,
|
||||
width=doc.pages[i + 1].size.width if (i + 1) in doc.pages else DEFAULT_PAGE_WIDTH,
|
||||
height=doc.pages[i + 1].size.height
|
||||
if (i + 1) in doc.pages
|
||||
else _DEFAULT_PAGE_HEIGHT,
|
||||
else DEFAULT_PAGE_HEIGHT,
|
||||
)
|
||||
for i in range(page_count)
|
||||
]
|
||||
|
|
|
|||
|
|
@ -69,13 +69,14 @@ class OpenSearchStore:
|
|||
verify_certs: Whether to verify TLS certificates.
|
||||
"""
|
||||
|
||||
def __init__(self, url: str, *, verify_certs: bool = False) -> None:
|
||||
def __init__(self, url: str, *, verify_certs: bool = False, default_limit: int = 1000) -> None:
|
||||
self._client = AsyncOpenSearch(
|
||||
hosts=[url],
|
||||
use_ssl=url.startswith("https"),
|
||||
verify_certs=verify_certs,
|
||||
ssl_show_warn=False,
|
||||
)
|
||||
self._default_limit = default_limit
|
||||
|
||||
# -- lifecycle -------------------------------------------------------------
|
||||
|
||||
|
|
@ -147,9 +148,11 @@ class OpenSearchStore:
|
|||
index_name: str,
|
||||
doc_id: str,
|
||||
*,
|
||||
limit: int = 1000,
|
||||
limit: int | None = None,
|
||||
) -> list[SearchResult]:
|
||||
"""Retrieve all indexed chunks for a document, ordered by chunk_index."""
|
||||
if limit is None:
|
||||
limit = self._default_limit
|
||||
resp = await self._client.search(
|
||||
index=index_name,
|
||||
body={
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@ import httpx
|
|||
from docling_core.types.doc.base import BoundingBox, CoordOrigin
|
||||
|
||||
from domain.value_objects import (
|
||||
DEFAULT_PAGE_HEIGHT,
|
||||
DEFAULT_PAGE_WIDTH,
|
||||
ConversionOptions,
|
||||
ConversionResult,
|
||||
PageDetail,
|
||||
|
|
@ -31,7 +33,6 @@ from infra.bbox import to_topleft_list
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
_API_PREFIX = "/v1"
|
||||
_DEFAULT_TIMEOUT = 600.0
|
||||
|
||||
# Docling Serve label → our element type
|
||||
_LABEL_MAP = {
|
||||
|
|
@ -60,7 +61,7 @@ class ServeConverter:
|
|||
self,
|
||||
base_url: str,
|
||||
api_key: str | None = None,
|
||||
timeout: float = _DEFAULT_TIMEOUT,
|
||||
timeout: float = 600.0,
|
||||
):
|
||||
self._base_url = base_url.rstrip("/")
|
||||
self._api_key = api_key
|
||||
|
|
@ -192,8 +193,8 @@ def _extract_pages_from_docling_document(doc: dict) -> list[PageDetail]:
|
|||
size = page_data.get("size", {})
|
||||
pages_dict[page_no] = PageDetail(
|
||||
page_number=page_no,
|
||||
width=size.get("width", 612.0),
|
||||
height=size.get("height", 792.0),
|
||||
width=size.get("width", DEFAULT_PAGE_WIDTH),
|
||||
height=size.get("height", DEFAULT_PAGE_HEIGHT),
|
||||
)
|
||||
|
||||
# Process all element arrays
|
||||
|
|
@ -220,8 +221,8 @@ def _add_element(item: dict, pages: dict[int, PageDetail]) -> None:
|
|||
if page_no not in pages:
|
||||
pages[page_no] = PageDetail(
|
||||
page_number=page_no,
|
||||
width=612.0,
|
||||
height=792.0,
|
||||
width=DEFAULT_PAGE_WIDTH,
|
||||
height=DEFAULT_PAGE_HEIGHT,
|
||||
)
|
||||
|
||||
bbox_data = prov.get("bbox", {})
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ class Settings:
|
|||
batch_page_size: int = 0 # 0 = disabled, > 0 = pages per batch
|
||||
opensearch_url: str = "" # empty = disabled
|
||||
embedding_url: str = "" # empty = disabled (e.g. http://localhost:8001)
|
||||
opensearch_default_limit: int = 1000 # max chunks returned by get_chunks
|
||||
embedding_dimension: int = 384 # Granite Embedding 30M / all-MiniLM-L6-v2
|
||||
upload_dir: str = "./uploads"
|
||||
db_path: str = "./data/docling_studio.db"
|
||||
|
|
@ -54,6 +55,10 @@ class Settings:
|
|||
errors.append(f"rate_limit_rpm must be >= 0 (got {self.rate_limit_rpm})")
|
||||
if self.batch_page_size < 0:
|
||||
errors.append(f"batch_page_size must be >= 0 (got {self.batch_page_size})")
|
||||
if self.opensearch_default_limit < 1:
|
||||
errors.append(
|
||||
f"opensearch_default_limit must be >= 1 (got {self.opensearch_default_limit})"
|
||||
)
|
||||
if self.embedding_dimension < 1:
|
||||
errors.append(f"embedding_dimension must be >= 1 (got {self.embedding_dimension})")
|
||||
if self.default_table_mode not in ("accurate", "fast"):
|
||||
|
|
@ -97,6 +102,7 @@ class Settings:
|
|||
batch_page_size=int(os.environ.get("BATCH_PAGE_SIZE", "10")),
|
||||
opensearch_url=os.environ.get("OPENSEARCH_URL", ""),
|
||||
embedding_url=os.environ.get("EMBEDDING_URL", ""),
|
||||
opensearch_default_limit=int(os.environ.get("OPENSEARCH_DEFAULT_LIMIT", "1000")),
|
||||
embedding_dimension=int(os.environ.get("EMBEDDING_DIMENSION", "384")),
|
||||
upload_dir=os.environ.get("UPLOAD_DIR", "./uploads"),
|
||||
db_path=os.environ.get("DB_PATH", "./data/docling_studio.db"),
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ def _build_converter():
|
|||
return ServeConverter(
|
||||
base_url=settings.docling_serve_url,
|
||||
api_key=settings.docling_serve_api_key,
|
||||
timeout=settings.conversion_timeout,
|
||||
)
|
||||
else:
|
||||
from infra.local_converter import LocalConverter
|
||||
|
|
@ -99,7 +100,10 @@ def _build_ingestion_service() -> IngestionService | None:
|
|||
from infra.opensearch_store import OpenSearchStore
|
||||
|
||||
embedding = EmbeddingClient(settings.embedding_url)
|
||||
vector_store = OpenSearchStore(settings.opensearch_url)
|
||||
vector_store = OpenSearchStore(
|
||||
settings.opensearch_url,
|
||||
default_limit=settings.opensearch_default_limit,
|
||||
)
|
||||
config = IngestionConfig(
|
||||
embedding_dimension=settings.embedding_dimension,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -9,11 +9,9 @@ from contextlib import asynccontextmanager
|
|||
|
||||
import aiosqlite
|
||||
|
||||
from infra.settings import settings
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
DB_PATH = settings.db_path
|
||||
DB_PATH = os.environ.get("DB_PATH", "./data/docling_studio.db")
|
||||
|
||||
_SCHEMA = """
|
||||
CREATE TABLE IF NOT EXISTS documents (
|
||||
|
|
|
|||
4
document-parser/requirements-test.txt
Normal file
4
document-parser/requirements-test.txt
Normal file
|
|
@ -0,0 +1,4 @@
|
|||
-r requirements.txt
|
||||
pytest>=8.0.0,<9.0.0
|
||||
pytest-asyncio>=0.23.0,<1.0.0
|
||||
pytestarch>=2.0.0,<3.0.0
|
||||
208
document-parser/tests/test_architecture.py
Normal file
208
document-parser/tests/test_architecture.py
Normal file
|
|
@ -0,0 +1,208 @@
|
|||
"""Hexagonal architecture tests — enforce layer dependency rules.
|
||||
|
||||
Uses pytestarch for inter-layer dependency rules and ast-based import
|
||||
scanning for external (third-party) dependency constraints.
|
||||
|
||||
Rules enforced:
|
||||
- domain -> no import from api, services, infra, persistence
|
||||
- services -> no import from api, infra, persistence
|
||||
- api -> no import from infra, persistence
|
||||
- infra -> no import from api, services
|
||||
- persistence -> no import from api, services, infra
|
||||
- domain -> no import of fastapi, sqlalchemy, httpx, opensearchpy
|
||||
- services -> no import of fastapi
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from pytestarch import Rule, get_evaluable_architecture
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# pytestarch evaluable (project root = document-parser/)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_PROJECT_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
# pytestarch uses the directory name as module prefix when given absolute paths.
|
||||
# We use the directory name to build qualified module references.
|
||||
_PREFIX = _PROJECT_ROOT.name # "document-parser"
|
||||
|
||||
_evaluable = get_evaluable_architecture(str(_PROJECT_ROOT), str(_PROJECT_ROOT))
|
||||
|
||||
|
||||
def _mod(layer: str) -> str:
|
||||
"""Return the fully-qualified pytestarch module name for a layer."""
|
||||
return f"{_PREFIX}.{layer}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helper: collect top-level imports from all .py files in a package
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _collect_imports(package: str) -> set[str]:
|
||||
"""Return the set of top-level module names imported by *package*."""
|
||||
pkg_path = Path(_PROJECT_ROOT) / package
|
||||
imports: set[str] = set()
|
||||
for py_file in pkg_path.rglob("*.py"):
|
||||
tree = ast.parse(py_file.read_text(), filename=str(py_file))
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.Import):
|
||||
for alias in node.names:
|
||||
imports.add(alias.name.split(".")[0])
|
||||
elif isinstance(node, ast.ImportFrom) and node.module:
|
||||
imports.add(node.module.split(".")[0])
|
||||
return imports
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Inter-layer dependency rules (pytestarch)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDomainLayerIsolation:
|
||||
"""domain must not depend on any other layer."""
|
||||
|
||||
@pytest.mark.parametrize("forbidden", ["api", "services", "infra", "persistence"])
|
||||
def test_domain_does_not_import(self, forbidden: str):
|
||||
rule = (
|
||||
Rule()
|
||||
.modules_that()
|
||||
.are_sub_modules_of(_mod("domain"))
|
||||
.should_not()
|
||||
.import_modules_that()
|
||||
.are_sub_modules_of(_mod(forbidden))
|
||||
)
|
||||
rule.assert_applies(_evaluable)
|
||||
|
||||
|
||||
class TestServicesLayerIsolation:
|
||||
"""services may import domain only."""
|
||||
|
||||
@pytest.mark.parametrize("forbidden", ["api", "infra", "persistence"])
|
||||
def test_services_does_not_import(self, forbidden: str):
|
||||
rule = (
|
||||
Rule()
|
||||
.modules_that()
|
||||
.are_sub_modules_of(_mod("services"))
|
||||
.should_not()
|
||||
.import_modules_that()
|
||||
.are_sub_modules_of(_mod(forbidden))
|
||||
)
|
||||
rule.assert_applies(_evaluable)
|
||||
|
||||
|
||||
class TestApiLayerIsolation:
|
||||
"""api may import services and domain, but not infra or persistence."""
|
||||
|
||||
@pytest.mark.parametrize("forbidden", ["infra", "persistence"])
|
||||
def test_api_does_not_import(self, forbidden: str):
|
||||
rule = (
|
||||
Rule()
|
||||
.modules_that()
|
||||
.are_sub_modules_of(_mod("api"))
|
||||
.should_not()
|
||||
.import_modules_that()
|
||||
.are_sub_modules_of(_mod(forbidden))
|
||||
)
|
||||
rule.assert_applies(_evaluable)
|
||||
|
||||
|
||||
class TestInfraLayerIsolation:
|
||||
"""infra may import domain (ports), but not api or services."""
|
||||
|
||||
@pytest.mark.parametrize("forbidden", ["api", "services"])
|
||||
def test_infra_does_not_import(self, forbidden: str):
|
||||
rule = (
|
||||
Rule()
|
||||
.modules_that()
|
||||
.are_sub_modules_of(_mod("infra"))
|
||||
.should_not()
|
||||
.import_modules_that()
|
||||
.are_sub_modules_of(_mod(forbidden))
|
||||
)
|
||||
rule.assert_applies(_evaluable)
|
||||
|
||||
|
||||
class TestPersistenceLayerIsolation:
|
||||
"""persistence may import domain, but not api, services, or infra."""
|
||||
|
||||
@pytest.mark.parametrize("forbidden", ["api", "services", "infra"])
|
||||
def test_persistence_does_not_import(self, forbidden: str):
|
||||
rule = (
|
||||
Rule()
|
||||
.modules_that()
|
||||
.are_sub_modules_of(_mod("persistence"))
|
||||
.should_not()
|
||||
.import_modules_that()
|
||||
.are_sub_modules_of(_mod(forbidden))
|
||||
)
|
||||
rule.assert_applies(_evaluable)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# External dependency rules (ast-based)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_DOMAIN_FORBIDDEN_EXTERNALS = {"fastapi", "sqlalchemy", "httpx", "opensearchpy"}
|
||||
_SERVICES_FORBIDDEN_EXTERNALS = {"fastapi"}
|
||||
|
||||
|
||||
class TestDomainExternalDependencies:
|
||||
"""domain must not import infrastructure-specific third-party libraries."""
|
||||
|
||||
@pytest.mark.parametrize("lib", sorted(_DOMAIN_FORBIDDEN_EXTERNALS))
|
||||
def test_domain_does_not_import_external(self, lib: str):
|
||||
imports = _collect_imports("domain")
|
||||
assert lib not in imports, f"domain imports forbidden external library '{lib}'"
|
||||
|
||||
|
||||
class TestServicesExternalDependencies:
|
||||
"""services must not import web-framework libraries."""
|
||||
|
||||
@pytest.mark.parametrize("lib", sorted(_SERVICES_FORBIDDEN_EXTERNALS))
|
||||
def test_services_does_not_import_external(self, lib: str):
|
||||
imports = _collect_imports("services")
|
||||
assert lib not in imports, f"services imports forbidden external library '{lib}'"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Convention: ports live exclusively in domain.ports
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPortConvention:
|
||||
"""Protocol definitions (ports) must live in domain.ports only."""
|
||||
|
||||
def test_no_protocol_outside_domain_ports(self):
|
||||
"""No Protocol subclass should be defined outside domain/ports.py."""
|
||||
ports_file = Path(_PROJECT_ROOT) / "domain" / "ports.py"
|
||||
for py_file in Path(_PROJECT_ROOT).rglob("*.py"):
|
||||
if py_file == ports_file:
|
||||
continue
|
||||
# Skip test files and __pycache__
|
||||
if "tests" in py_file.parts or "__pycache__" in py_file.parts:
|
||||
continue
|
||||
tree = ast.parse(py_file.read_text(), filename=str(py_file))
|
||||
for node in ast.walk(tree):
|
||||
if isinstance(node, ast.ClassDef):
|
||||
for base in node.bases:
|
||||
base_name = _get_name(base)
|
||||
if base_name == "Protocol":
|
||||
pytest.fail(
|
||||
f"Protocol '{node.name}' defined in {py_file.relative_to(_PROJECT_ROOT)}"
|
||||
f" — ports must live in domain/ports.py"
|
||||
)
|
||||
|
||||
|
||||
def _get_name(node: ast.expr) -> str:
|
||||
"""Extract a simple name from an AST expression node."""
|
||||
if isinstance(node, ast.Name):
|
||||
return node.id
|
||||
if isinstance(node, ast.Attribute):
|
||||
return node.attr
|
||||
return ""
|
||||
|
|
@ -19,6 +19,7 @@ class TestSettingsDefaults:
|
|||
assert s.max_page_count == 0
|
||||
assert s.max_file_size_mb == 50
|
||||
assert s.batch_page_size == 0
|
||||
assert s.opensearch_default_limit == 1000
|
||||
assert s.upload_dir == "./uploads"
|
||||
assert s.db_path == "./data/docling_studio.db"
|
||||
assert "http://localhost:3000" in s.cors_origins
|
||||
|
|
@ -103,6 +104,12 @@ class TestSettingsValidation:
|
|||
with pytest.raises(ValueError, match="lock_timeout must be > 0"):
|
||||
Settings(lock_timeout=0)
|
||||
|
||||
def test_zero_opensearch_default_limit_rejected(self):
|
||||
import pytest
|
||||
|
||||
with pytest.raises(ValueError, match="opensearch_default_limit must be >= 1"):
|
||||
Settings(opensearch_default_limit=0)
|
||||
|
||||
def test_invalid_table_mode_rejected(self):
|
||||
import pytest
|
||||
|
||||
|
|
@ -146,6 +153,7 @@ class TestSettingsFromEnv:
|
|||
monkeypatch.setenv("MAX_PAGE_COUNT", "20")
|
||||
monkeypatch.setenv("MAX_FILE_SIZE_MB", "100")
|
||||
monkeypatch.setenv("BATCH_PAGE_SIZE", "15")
|
||||
monkeypatch.setenv("OPENSEARCH_DEFAULT_LIMIT", "500")
|
||||
monkeypatch.setenv("UPLOAD_DIR", "/data/uploads")
|
||||
monkeypatch.setenv("DB_PATH", "/data/test.db")
|
||||
monkeypatch.setenv("CORS_ORIGINS", "http://a.com, http://b.com")
|
||||
|
|
@ -163,6 +171,7 @@ class TestSettingsFromEnv:
|
|||
assert s.max_page_count == 20
|
||||
assert s.max_file_size_mb == 100
|
||||
assert s.batch_page_size == 15
|
||||
assert s.opensearch_default_limit == 500
|
||||
assert s.upload_dir == "/data/uploads"
|
||||
assert s.db_path == "/data/test.db"
|
||||
assert s.cors_origins == ["http://a.com", "http://b.com"]
|
||||
|
|
|
|||
Loading…
Reference in a new issue