diff --git a/document-parser/api/analyses.py b/document-parser/api/analyses.py index 3d91805..4fbf988 100644 --- a/document-parser/api/analyses.py +++ b/document-parser/api/analyses.py @@ -3,16 +3,24 @@ from __future__ import annotations import logging +from typing import Annotated -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, Depends, HTTPException, Request from api.schemas import AnalysisResponse, CreateAnalysisRequest -from services import analysis_service +from services.analysis_service import AnalysisService logger = logging.getLogger(__name__) router = APIRouter(prefix="/api/analyses", tags=["analyses"]) +def _get_service(request: Request) -> AnalysisService: + return request.app.state.analysis_service + + +ServiceDep = Annotated[AnalysisService, Depends(_get_service)] + + def _to_response(job) -> AnalysisResponse: return AnalysisResponse( id=job.id, @@ -30,7 +38,7 @@ def _to_response(job) -> AnalysisResponse: @router.post("", response_model=AnalysisResponse) -async def create_analysis(body: CreateAnalysisRequest): +async def create_analysis(body: CreateAnalysisRequest, service: ServiceDep): """Create a new analysis job for a document.""" if not body.documentId or not body.documentId.strip(): raise HTTPException(status_code=400, detail="documentId is required") @@ -40,7 +48,7 @@ async def create_analysis(body: CreateAnalysisRequest): pipeline_opts = body.pipelineOptions.model_dump() try: - job = await analysis_service.create(body.documentId, pipeline_options=pipeline_opts) + job = await service.create(body.documentId, pipeline_options=pipeline_opts) except ValueError as e: raise HTTPException(status_code=404, detail=str(e)) from e @@ -48,24 +56,24 @@ async def create_analysis(body: CreateAnalysisRequest): @router.get("", response_model=list[AnalysisResponse]) -async def list_analyses(): +async def list_analyses(service: ServiceDep): """List all analysis jobs.""" - jobs = await analysis_service.find_all() + jobs = await service.find_all() return [_to_response(j) for j in jobs] @router.get("/{job_id}", response_model=AnalysisResponse) -async def get_analysis(job_id: str): +async def get_analysis(job_id: str, service: ServiceDep): """Get a single analysis job.""" - job = await analysis_service.find_by_id(job_id) + job = await service.find_by_id(job_id) if not job: raise HTTPException(status_code=404, detail="Analysis not found") return _to_response(job) @router.delete("/{job_id}", status_code=204) -async def delete_analysis(job_id: str): +async def delete_analysis(job_id: str, service: ServiceDep): """Delete an analysis job.""" - deleted = await analysis_service.delete(job_id) + deleted = await service.delete(job_id) if not deleted: raise HTTPException(status_code=404, detail="Analysis not found") diff --git a/document-parser/domain/ports.py b/document-parser/domain/ports.py new file mode 100644 index 0000000..bc36c9b --- /dev/null +++ b/document-parser/domain/ports.py @@ -0,0 +1,24 @@ +"""Domain ports — abstract interfaces that infrastructure must implement. + +These protocols define what the domain NEEDS, not how it's done. +Infrastructure adapters (local Docling, Docling Serve, etc.) implement these. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Protocol + +if TYPE_CHECKING: + from domain.value_objects import ConversionOptions, ConversionResult + + +class DocumentConverter(Protocol): + """Port for document conversion. + + Any implementation (local Docling lib, remote Docling Serve, mock, etc.) + must satisfy this contract. + """ + + async def convert( + self, file_path: str, options: ConversionOptions, + ) -> ConversionResult: ... diff --git a/document-parser/domain/value_objects.py b/document-parser/domain/value_objects.py new file mode 100644 index 0000000..b4c5959 --- /dev/null +++ b/document-parser/domain/value_objects.py @@ -0,0 +1,51 @@ +"""Domain value objects — pure data structures for document conversion. + +These types define the contract between the domain and infrastructure layers. +They have ZERO external dependencies (no docling, no HTTP, no DB). +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + + +@dataclass +class PageElement: + type: str + bbox: list[float] + content: str + level: int = 0 + + +@dataclass +class PageDetail: + page_number: int + width: float + height: float + elements: list[PageElement] = field(default_factory=list) + + +@dataclass +class ConversionOptions: + do_ocr: bool = True + do_table_structure: bool = True + table_mode: str = "accurate" + do_code_enrichment: bool = False + do_formula_enrichment: bool = False + do_picture_classification: bool = False + do_picture_description: bool = False + generate_picture_images: bool = False + generate_page_images: bool = False + images_scale: float = 1.0 + + def is_default(self) -> bool: + return self == ConversionOptions() + + +@dataclass +class ConversionResult: + page_count: int + content_markdown: str + content_html: str + pages: list[PageDetail] + skipped_items: int = 0 diff --git a/document-parser/infra/__init__.py b/document-parser/infra/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/document-parser/domain/parsing.py b/document-parser/infra/local_converter.py similarity index 55% rename from document-parser/domain/parsing.py rename to document-parser/infra/local_converter.py index 7e7b4bb..efd4a5a 100644 --- a/document-parser/domain/parsing.py +++ b/document-parser/infra/local_converter.py @@ -1,15 +1,16 @@ -"""Docling document extraction logic — pure domain, no HTTP concerns. +"""Local Docling converter — runs Docling as a Python library in-process. -Wraps the Docling library to convert documents and extract structured -per-page elements with bounding boxes and hierarchy levels. +This adapter implements the DocumentConverter port using the Docling library +directly. It wraps the blocking DocumentConverter in asyncio.to_thread for +non-blocking execution. """ from __future__ import annotations +import asyncio import contextlib import logging import threading -from dataclasses import dataclass, field from docling.datamodel.base_models import InputFormat from docling.datamodel.pipeline_options import ( @@ -17,7 +18,8 @@ from docling.datamodel.pipeline_options import ( TableFormerMode, TableStructureOptions, ) -from docling.document_converter import DocumentConverter, PdfFormatOption +from docling.document_converter import DocumentConverter as DoclingConverter +from docling.document_converter import PdfFormatOption from docling_core.types.doc import ( CodeItem, DocItem, @@ -33,10 +35,16 @@ from docling_core.types.doc import ( ) from domain.bbox import to_topleft_list +from domain.value_objects import ( + ConversionOptions, + ConversionResult, + PageDetail, + PageElement, +) logger = logging.getLogger(__name__) -# Thread lock — DocumentConverter is not thread-safe +# Thread lock — DoclingConverter is not thread-safe _converter_lock = threading.Lock() # US Letter page dimensions (points) — fallback when page size is unknown @@ -44,61 +52,13 @@ _DEFAULT_PAGE_WIDTH = 612.0 _DEFAULT_PAGE_HEIGHT = 792.0 # Default converter (lazy-init on first request) -_default_converter: DocumentConverter | None = None - - -# --------------------------------------------------------------------------- -# Domain value objects -# --------------------------------------------------------------------------- - -@dataclass -class PageElement: - type: str - bbox: list[float] - content: str - level: int = 0 - - -@dataclass -class PageDetail: - page_number: int - width: float - height: float - elements: list[PageElement] = field(default_factory=list) - - -@dataclass -class ConversionOptions: - do_ocr: bool = True - do_table_structure: bool = True - table_mode: str = "accurate" - do_code_enrichment: bool = False - do_formula_enrichment: bool = False - do_picture_classification: bool = False - do_picture_description: bool = False - generate_picture_images: bool = False - generate_page_images: bool = False - images_scale: float = 1.0 - - def is_default(self) -> bool: - return self == ConversionOptions() - - -@dataclass -class ConversionResult: - page_count: int - content_markdown: str - content_html: str - pages: list[PageDetail] - skipped_items: int = 0 +_default_converter: DoclingConverter | None = None # --------------------------------------------------------------------------- # Element type detection # --------------------------------------------------------------------------- -# Mapping from Docling type to element type string. -# Order matters: most specific types before their parents. _ELEMENT_TYPE_MAP: list[tuple[type, str]] = [ (TableItem, "table"), (PictureItem, "picture"), @@ -113,7 +73,6 @@ _ELEMENT_TYPE_MAP: list[tuple[type, str]] = [ def _get_element_type(item: DocItem) -> str: - """Determine element type via isinstance on Docling's type hierarchy.""" for cls, type_name in _ELEMENT_TYPE_MAP: if isinstance(item, cls): return type_name @@ -124,51 +83,50 @@ def _get_element_type(item: DocItem) -> str: # Pipeline factory # --------------------------------------------------------------------------- -def build_converter(options: ConversionOptions | None = None) -> DocumentConverter: - """Build a DocumentConverter with the given pipeline options.""" - opts = options or ConversionOptions() - +def _build_docling_converter(options: ConversionOptions) -> DoclingConverter: table_options = TableStructureOptions( do_cell_matching=True, - mode=TableFormerMode.ACCURATE if opts.table_mode == "accurate" else TableFormerMode.FAST, + mode=TableFormerMode.ACCURATE if options.table_mode == "accurate" else TableFormerMode.FAST, ) pipeline_options = PdfPipelineOptions( - do_ocr=opts.do_ocr, - do_table_structure=opts.do_table_structure, + do_ocr=options.do_ocr, + do_table_structure=options.do_table_structure, table_structure_options=table_options, - do_code_enrichment=opts.do_code_enrichment, - do_formula_enrichment=opts.do_formula_enrichment, - do_picture_classification=opts.do_picture_classification, - do_picture_description=opts.do_picture_description, - generate_page_images=opts.generate_page_images, - generate_picture_images=opts.generate_picture_images, - images_scale=opts.images_scale, + do_code_enrichment=options.do_code_enrichment, + do_formula_enrichment=options.do_formula_enrichment, + do_picture_classification=options.do_picture_classification, + do_picture_description=options.do_picture_description, + generate_page_images=options.generate_page_images, + generate_picture_images=options.generate_picture_images, + images_scale=options.images_scale, ) - return DocumentConverter( + return DoclingConverter( format_options={ InputFormat.PDF: PdfFormatOption(pipeline_options=pipeline_options), } ) -def get_default_converter() -> DocumentConverter: +def _get_default_converter() -> DoclingConverter: global _default_converter if _default_converter is None: - _default_converter = build_converter() + _default_converter = _build_docling_converter(ConversionOptions()) return _default_converter +def _select_converter(options: ConversionOptions) -> DoclingConverter: + if options.is_default(): + return _get_default_converter() + return _build_docling_converter(options) + + # --------------------------------------------------------------------------- # Page extraction # --------------------------------------------------------------------------- -def extract_pages_detail(doc_result) -> tuple[list[PageDetail], int]: - """Extract per-page element details with bounding boxes from Docling result. - - Returns (pages, skipped_count) for transparent error reporting. - """ +def _extract_pages_detail(doc_result) -> tuple[list[PageDetail], int]: pages: dict[int, PageDetail] = {} document = doc_result.document skipped = 0 @@ -193,7 +151,6 @@ def extract_pages_detail(doc_result) -> tuple[list[PageDetail], int]: def _process_content_item( item: DocItem | GroupItem, level: int, pages: dict[int, PageDetail], ) -> bool: - """Process a single content item and add it to the appropriate page.""" if isinstance(item, GroupItem): return True @@ -204,9 +161,6 @@ def _process_content_item( try: page_no = prov.page_no if page_no not in pages: - # Fallback: page was not found in document.pages (corrupted PDF or - # Docling edge case). US Letter dimensions are used as a safe default. - # This may cause slight bbox misalignment on non-Letter pages (e.g. A4). logger.warning( "Page %d not found in document metadata — using US Letter fallback (%sx%s pt)", page_no, _DEFAULT_PAGE_WIDTH, _DEFAULT_PAGE_HEIGHT, @@ -242,49 +196,27 @@ def _process_content_item( # --------------------------------------------------------------------------- -# Main conversion entry point +# Synchronous conversion (called via asyncio.to_thread) # --------------------------------------------------------------------------- -def _select_converter(options: ConversionOptions) -> DocumentConverter: - """Return the cached default converter or build a custom one.""" - if options.is_default(): - return get_default_converter() - return build_converter(options) - - -def _build_fallback_pages(doc, page_count: int) -> list[PageDetail]: - """Create empty PageDetail entries when extraction yields nothing.""" - return [ - PageDetail( - page_number=i + 1, - 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, - ) - for i in range(page_count) - ] - - -def convert_document( - file_path: str, - options: ConversionOptions | None = None, -) -> ConversionResult: - """Convert a document and return structured results. - - This is the main entry point for document parsing. Runs synchronously - (caller should use asyncio.to_thread for non-blocking execution). - """ - opts = options or ConversionOptions() - +def _convert_sync(file_path: str, options: ConversionOptions) -> ConversionResult: with _converter_lock: - conv = _select_converter(opts) + conv = _select_converter(options) result = conv.convert(file_path) doc = result.document page_count = len(doc.pages) - pages_detail, skipped = extract_pages_detail(result) + pages_detail, skipped = _extract_pages_detail(result) if not pages_detail and page_count > 0: - pages_detail = _build_fallback_pages(doc, page_count) + pages_detail = [ + PageDetail( + page_number=i + 1, + 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, + ) + for i in range(page_count) + ] if skipped > 0: logger.info("Parsed: %d pages, %d items skipped", page_count, skipped) @@ -296,3 +228,16 @@ def convert_document( pages=pages_detail, skipped_items=skipped, ) + + +# --------------------------------------------------------------------------- +# Public adapter class +# --------------------------------------------------------------------------- + +class LocalConverter: + """Adapter that runs Docling locally as a Python library.""" + + async def convert( + self, file_path: str, options: ConversionOptions, + ) -> ConversionResult: + return await asyncio.to_thread(_convert_sync, file_path, options) diff --git a/document-parser/infra/serve_converter.py b/document-parser/infra/serve_converter.py new file mode 100644 index 0000000..d85e4a5 --- /dev/null +++ b/document-parser/infra/serve_converter.py @@ -0,0 +1,234 @@ +"""Remote Docling Serve converter — delegates conversion via HTTP. + +This adapter implements the DocumentConverter port by calling a remote +Docling Serve instance's REST API (v1). + +API contract based on docling-serve source code: +- Options are sent as individual multipart form fields (not a JSON blob) +- Response contains document.md_content, document.html_content, document.json_content +- json_content is a serialized DoclingDocument with texts[], tables[], pictures[] +- Bounding boxes use {l, t, r, b, coord_origin} format +""" + +from __future__ import annotations + +import json +import logging +import mimetypes +from pathlib import Path + +import httpx + +from domain.value_objects import ( + ConversionOptions, + ConversionResult, + PageDetail, + PageElement, +) + +logger = logging.getLogger(__name__) + +_API_PREFIX = "/v1" +_DEFAULT_TIMEOUT = 600.0 + +# Docling Serve label → our element type +_LABEL_MAP = { + "table": "table", + "picture": "picture", + "figure": "picture", + "title": "title", + "section_header": "section_header", + "list_item": "list", + "formula": "formula", + "code": "code", + "caption": "text", + "footnote": "text", + "page_header": "text", + "page_footer": "text", + "paragraph": "text", + "text": "text", + "reference": "text", +} + + +class ServeConverter: + """Adapter that delegates document conversion to a remote Docling Serve instance.""" + + def __init__( + self, + base_url: str, + api_key: str | None = None, + timeout: float = _DEFAULT_TIMEOUT, + ): + self._base_url = base_url.rstrip("/") + self._api_key = api_key + self._timeout = timeout + + def _headers(self) -> dict[str, str]: + headers: dict[str, str] = {} + if self._api_key: + headers["X-Api-Key"] = self._api_key + return headers + + async def convert( + self, file_path: str, options: ConversionOptions, + ) -> ConversionResult: + """Convert a document by uploading it to Docling Serve.""" + path = Path(file_path) + content_type = mimetypes.guess_type(path.name)[0] or "application/octet-stream" + + form_data = _build_form_data(options) + url = f"{self._base_url}{_API_PREFIX}/convert/file" + + async with httpx.AsyncClient(timeout=self._timeout) as client: + with open(path, "rb") as f: + response = await client.post( + url, + files={"files": (path.name, f, content_type)}, + data=form_data, + headers=self._headers(), + ) + + response.raise_for_status() + result_data = response.json() + + return _parse_response(result_data) + + async def health_check(self) -> bool: + """Check if Docling Serve is reachable.""" + try: + async with httpx.AsyncClient(timeout=10.0) as client: + resp = await client.get( + f"{self._base_url}/version", + headers=self._headers(), + ) + return resp.status_code == 200 + except httpx.HTTPError: + logger.warning("Docling Serve health check failed at %s", self._base_url, exc_info=True) + return False + + +def _build_form_data(options: ConversionOptions) -> dict[str, str | list[str]]: + """Build form fields matching Docling Serve's multipart form contract. + + Array fields (to_formats) are sent as lists — httpx encodes them as + repeated form keys (to_formats=md&to_formats=html&to_formats=json). + """ + return { + "to_formats": ["md", "html", "json"], + "do_ocr": str(options.do_ocr).lower(), + "do_table_structure": str(options.do_table_structure).lower(), + "table_mode": options.table_mode, + "do_code_enrichment": str(options.do_code_enrichment).lower(), + "do_formula_enrichment": str(options.do_formula_enrichment).lower(), + "do_picture_classification": str(options.do_picture_classification).lower(), + "do_picture_description": str(options.do_picture_description).lower(), + "include_images": str(options.generate_picture_images).lower(), + "images_scale": str(options.images_scale), + } + + +def _parse_response(data: dict) -> ConversionResult: + """Parse Docling Serve v1 ConvertDocumentResponse into our domain ConversionResult.""" + document = data.get("document", {}) + + content_md = document.get("md_content") or "" + content_html = document.get("html_content") or "" + + # json_content contains the full DoclingDocument with pages, elements, bboxes + json_content = document.get("json_content") + if isinstance(json_content, str): + try: + json_content = json.loads(json_content) + except json.JSONDecodeError: + logger.warning("Failed to parse json_content as JSON, ignoring structured data") + json_content = None + + pages: list[PageDetail] = [] + if json_content: + pages = _extract_pages_from_docling_document(json_content) + + page_count = len(pages) if pages else 1 + + return ConversionResult( + page_count=page_count, + content_markdown=content_md, + content_html=content_html, + pages=pages, + ) + + +def _extract_pages_from_docling_document(doc: dict) -> list[PageDetail]: + """Extract pages with elements from a serialized DoclingDocument. + + DoclingDocument structure: + - pages: {page_no: {size: {width, height}}} + - texts: [{label, text, prov: [{page_no, bbox: {l,t,r,b,coord_origin}}]}] + - tables: [{label, prov: [...], data: {...}}] + - pictures: [{label, prov: [...]}] + """ + pages_dict: dict[int, PageDetail] = {} + + # Build page dimensions + for page_key, page_data in doc.get("pages", {}).items(): + page_no = int(page_key) + 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), + ) + + # Process all element arrays + for item in doc.get("texts", []): + _add_element(item, pages_dict) + + for item in doc.get("tables", []): + _add_element(item, pages_dict) + + for item in doc.get("pictures", []): + _add_element(item, pages_dict) + + return sorted(pages_dict.values(), key=lambda p: p.page_number) + + +def _add_element(item: dict, pages: dict[int, PageDetail]) -> None: + """Add an element from a DoclingDocument array to the correct page.""" + label = item.get("label", "text") + element_type = _LABEL_MAP.get(label, "text") + content = item.get("text", "") or "" + + for prov in item.get("prov", []): + page_no = prov.get("page_no", 1) + if page_no not in pages: + pages[page_no] = PageDetail( + page_number=page_no, width=612.0, height=792.0, + ) + + bbox_data = prov.get("bbox", {}) + bbox = _extract_bbox(bbox_data, pages[page_no].height) + + pages[page_no].elements.append( + PageElement(type=element_type, bbox=bbox, content=content, level=0) + ) + + +def _extract_bbox(bbox_data: dict, page_height: float) -> list[float]: + """Extract and normalize bbox to TOPLEFT [l, t, r, b] format.""" + if not isinstance(bbox_data, dict): + return [0.0, 0.0, 0.0, 0.0] + + left = bbox_data.get("l", 0.0) + top = bbox_data.get("t", 0.0) + right = bbox_data.get("r", 0.0) + bottom = bbox_data.get("b", 0.0) + coord_origin = bbox_data.get("coord_origin", "TOPLEFT") + + if coord_origin == "BOTTOMLEFT": + # In BOTTOMLEFT: top has higher y, bottom has lower y + # In TOPLEFT: flip both — new_top = page_height - old_top + new_top = page_height - top + new_bottom = page_height - bottom + top, bottom = new_top, new_bottom + + return [left, top, right, bottom] diff --git a/document-parser/infra/settings.py b/document-parser/infra/settings.py new file mode 100644 index 0000000..0b19c54 --- /dev/null +++ b/document-parser/infra/settings.py @@ -0,0 +1,30 @@ +"""Centralized application settings — loaded from environment variables.""" + +from __future__ import annotations + +import os +from dataclasses import dataclass, field + + +@dataclass(frozen=True) +class Settings: + conversion_engine: str = "local" # "local" or "remote" + docling_serve_url: str = "http://localhost:5001" + docling_serve_api_key: str | None = None + conversion_timeout: int = 600 + upload_dir: str = "./uploads" + db_path: str = "./data/docling_studio.db" + cors_origins: list[str] = field(default_factory=lambda: ["http://localhost:3000", "http://localhost:5173"]) + + @classmethod + def from_env(cls) -> Settings: + cors_raw = os.environ.get("CORS_ORIGINS", "http://localhost:3000,http://localhost:5173") + return cls( + conversion_engine=os.environ.get("CONVERSION_ENGINE", "local"), + docling_serve_url=os.environ.get("DOCLING_SERVE_URL", "http://localhost:5001"), + docling_serve_api_key=os.environ.get("DOCLING_SERVE_API_KEY"), + conversion_timeout=int(os.environ.get("CONVERSION_TIMEOUT", "600")), + upload_dir=os.environ.get("UPLOAD_DIR", "./uploads"), + db_path=os.environ.get("DB_PATH", "./data/docling_studio.db"), + cors_origins=[o.strip() for o in cors_raw.split(",")], + ) diff --git a/document-parser/main.py b/document-parser/main.py index 7e29776..20a5478 100644 --- a/document-parser/main.py +++ b/document-parser/main.py @@ -1,14 +1,17 @@ """Docling Studio — unified FastAPI backend. -Single service replacing both the Spring Boot backend and the document parser. -Provides document management (upload, CRUD), analysis orchestration (async Docling -processing), and PDF preview — all backed by SQLite. +Single service providing document management (upload, CRUD), analysis +orchestration (async Docling processing), and PDF preview — all backed +by SQLite. + +Conversion engine is selected via CONVERSION_ENGINE env var: +- "local" → Docling runs in-process as a Python library (default) +- "remote" → delegates to a Docling Serve instance via HTTP """ from __future__ import annotations import logging -import os from contextlib import asynccontextmanager from fastapi import FastAPI @@ -16,7 +19,9 @@ from fastapi.middleware.cors import CORSMiddleware from api.analyses import router as analyses_router from api.documents import router as documents_router +from infra.settings import Settings from persistence.database import init_db +from services.analysis_service import AnalysisService logging.basicConfig( level=logging.INFO, @@ -24,12 +29,45 @@ logging.basicConfig( ) logger = logging.getLogger(__name__) +# --------------------------------------------------------------------------- +# Settings & dependency wiring +# --------------------------------------------------------------------------- + +settings = Settings.from_env() + + +def _build_converter(): + """Build the converter adapter based on configuration.""" + if settings.conversion_engine == "remote": + from infra.serve_converter import ServeConverter + logger.info("Using remote Docling Serve at %s", settings.docling_serve_url) + return ServeConverter( + base_url=settings.docling_serve_url, + api_key=settings.docling_serve_api_key, + ) + else: + from infra.local_converter import LocalConverter + logger.info("Using local Docling converter") + return LocalConverter() + + +def _build_analysis_service() -> AnalysisService: + converter = _build_converter() + return AnalysisService( + converter=converter, + conversion_timeout=settings.conversion_timeout, + ) + + +# --------------------------------------------------------------------------- +# FastAPI app +# --------------------------------------------------------------------------- @asynccontextmanager async def lifespan(app: FastAPI): - """Startup: initialize database. Shutdown: nothing special needed.""" await init_db() - logger.info("Docling Studio backend ready") + app.state.analysis_service = _build_analysis_service() + logger.info("Docling Studio backend ready (engine=%s)", settings.conversion_engine) yield @@ -39,20 +77,14 @@ app = FastAPI( lifespan=lifespan, ) -# CORS — configurable via env, defaults for local dev -allowed_origins = os.environ.get( - "CORS_ORIGINS", "http://localhost:3000,http://localhost:5173" -).split(",") - app.add_middleware( CORSMiddleware, - allow_origins=[o.strip() for o in allowed_origins], + allow_origins=settings.cors_origins, allow_credentials=True, allow_methods=["GET", "POST", "DELETE", "OPTIONS"], allow_headers=["Content-Type", "Authorization"], ) -# Mount routers app.include_router(documents_router) app.include_router(analyses_router) @@ -60,4 +92,4 @@ app.include_router(analyses_router) @app.get("/health") def health(): """Health check endpoint.""" - return {"status": "ok"} + return {"status": "ok", "engine": settings.conversion_engine} diff --git a/document-parser/persistence/analysis_repo.py b/document-parser/persistence/analysis_repo.py index a16367d..f8b3df7 100644 --- a/document-parser/persistence/analysis_repo.py +++ b/document-parser/persistence/analysis_repo.py @@ -2,10 +2,19 @@ from __future__ import annotations +from datetime import datetime + from domain.models import AnalysisJob, AnalysisStatus from persistence.database import get_connection +def _parse_dt(value: str | None) -> datetime | None: + """Parse an ISO-format datetime string back into a datetime object.""" + if not value: + return None + return datetime.fromisoformat(value) + + def _row_to_job(row) -> AnalysisJob: return AnalysisJob( id=row["id"], @@ -15,9 +24,9 @@ def _row_to_job(row) -> AnalysisJob: content_html=row["content_html"], pages_json=row["pages_json"], error_message=row["error_message"], - started_at=row["started_at"], - completed_at=row["completed_at"], - created_at=row["created_at"], + started_at=_parse_dt(row["started_at"]), + completed_at=_parse_dt(row["completed_at"]), + created_at=_parse_dt(row["created_at"]) or datetime.now(), document_filename=row["filename"] if "filename" in row.keys() else None, # noqa: SIM118 ) diff --git a/document-parser/persistence/document_repo.py b/document-parser/persistence/document_repo.py index 3649dc1..868b24d 100644 --- a/document-parser/persistence/document_repo.py +++ b/document-parser/persistence/document_repo.py @@ -2,11 +2,16 @@ from __future__ import annotations +from datetime import datetime + from domain.models import Document from persistence.database import get_connection def _row_to_document(row) -> Document: + created = row["created_at"] + if isinstance(created, str): + created = datetime.fromisoformat(created) return Document( id=row["id"], filename=row["filename"], @@ -14,7 +19,7 @@ def _row_to_document(row) -> Document: file_size=row["file_size"], page_count=row["page_count"], storage_path=row["storage_path"], - created_at=row["created_at"], + created_at=created, ) diff --git a/document-parser/requirements.txt b/document-parser/requirements.txt index 46d50d0..4b8cc74 100644 --- a/document-parser/requirements.txt +++ b/document-parser/requirements.txt @@ -6,3 +6,4 @@ python-multipart>=0.0.12 pdf2image>=1.17.0,<2.0.0 pillow>=10.0.0,<11.0.0 aiosqlite>=0.20.0,<1.0.0 +httpx>=0.27.0,<1.0.0 diff --git a/document-parser/services/analysis_service.py b/document-parser/services/analysis_service.py index 7a4d71a..2059506 100644 --- a/document-parser/services/analysis_service.py +++ b/document-parser/services/analysis_service.py @@ -1,108 +1,125 @@ -"""Analysis service — async document parsing orchestration.""" +"""Analysis service — async document parsing orchestration. + +Uses an injected DocumentConverter (port) so the service is decoupled +from the conversion implementation (local Docling lib vs remote Docling Serve). +""" from __future__ import annotations import asyncio +import functools import json import logging from dataclasses import asdict +from typing import TYPE_CHECKING from domain.models import AnalysisJob -from domain.parsing import ConversionOptions, ConversionResult, convert_document +from domain.value_objects import ConversionOptions, ConversionResult + +if TYPE_CHECKING: + from domain.ports import DocumentConverter from persistence import analysis_repo, document_repo logger = logging.getLogger(__name__) -# Maximum time (seconds) allowed for a single document conversion. -CONVERSION_TIMEOUT = int(__import__("os").environ.get("CONVERSION_TIMEOUT", "600")) + +class AnalysisService: + """Orchestrates document analysis using an injected converter.""" + + def __init__(self, converter: DocumentConverter, conversion_timeout: int = 600): + self._converter = converter + self._conversion_timeout = conversion_timeout + + async def create(self, document_id: str, *, pipeline_options: dict | None = None) -> AnalysisJob: + """Create a new analysis job and launch background processing.""" + doc = await document_repo.find_by_id(document_id) + if not doc: + raise ValueError(f"Document not found: {document_id}") + + job = AnalysisJob(document_id=document_id) + job.document_filename = doc.filename + await analysis_repo.insert(job) + + task = asyncio.create_task( + self._run_analysis(job.id, doc.storage_path, doc.filename, pipeline_options) + ) + task.add_done_callback(functools.partial(_on_task_done, job_id=job.id)) + + return job + + async def find_all(self) -> list[AnalysisJob]: + return await analysis_repo.find_all() + + async def find_by_id(self, job_id: str) -> AnalysisJob | None: + return await analysis_repo.find_by_id(job_id) + + async def delete(self, job_id: str) -> bool: + return await analysis_repo.delete(job_id) + + async def _run_analysis( + self, job_id: str, file_path: str, filename: str, pipeline_options: dict | None = None, + ) -> None: + """Background task: run conversion and update job status.""" + try: + job = await analysis_repo.find_by_id(job_id) + if not job: + logger.error("Analysis job %s not found", job_id) + return + + job.mark_running() + await analysis_repo.update_status(job) + logger.info("Analysis started: %s (file: %s)", job_id, filename) + + options = ConversionOptions(**(pipeline_options or {})) + + result: ConversionResult = await asyncio.wait_for( + self._converter.convert(file_path, options), + timeout=self._conversion_timeout, + ) + + pages_json = json.dumps([asdict(p) for p in result.pages]) + + job.mark_completed( + markdown=result.content_markdown, + html=result.content_html, + pages_json=pages_json, + ) + await analysis_repo.update_status(job) + + if result.page_count: + await document_repo.update_page_count(job.document_id, result.page_count) + + logger.info("Analysis completed: %s (%d pages)", job_id, result.page_count) + + except TimeoutError: + logger.error("Analysis timed out after %ds: %s", self._conversion_timeout, job_id) + await _mark_failed(job_id, f"Conversion timed out after {self._conversion_timeout}s") + + except Exception as e: + logger.exception("Analysis failed: %s", job_id) + await _mark_failed(job_id, str(e)) -async def create(document_id: str, *, pipeline_options: dict | None = None) -> AnalysisJob: - """Create a new analysis job and launch background processing.""" - doc = await document_repo.find_by_id(document_id) - if not doc: - raise ValueError(f"Document not found: {document_id}") - - job = AnalysisJob(document_id=document_id) - job.document_filename = doc.filename - await analysis_repo.insert(job) - - # Fire background task with error logging callback - task = asyncio.create_task( - _run_analysis(job.id, doc.storage_path, doc.filename, pipeline_options) - ) - task.add_done_callback(_on_task_done) - - return job +_background_tasks: set[asyncio.Task] = set() -def _on_task_done(task: asyncio.Task) -> None: - """Log unhandled exceptions from background analysis tasks.""" +def _on_task_done(task: asyncio.Task, *, job_id: str) -> None: + """Log unhandled exceptions from background analysis tasks and mark job as FAILED.""" if task.cancelled(): - logger.warning("Analysis task was cancelled") + logger.warning("Analysis task was cancelled: %s", job_id) + _schedule_mark_failed(job_id, "Task was cancelled") return exc = task.exception() if exc: - logger.error("Unhandled exception in analysis task: %s", exc, exc_info=True) + logger.error("Unhandled exception in analysis task %s: %s", job_id, exc, exc_info=True) + _schedule_mark_failed(job_id, str(exc)) -async def find_all() -> list[AnalysisJob]: - return await analysis_repo.find_all() - - -async def find_by_id(job_id: str) -> AnalysisJob | None: - return await analysis_repo.find_by_id(job_id) - - -async def delete(job_id: str) -> bool: - return await analysis_repo.delete(job_id) - - -async def _run_analysis( - job_id: str, file_path: str, filename: str, pipeline_options: dict | None = None, -) -> None: - """Background task: run Docling conversion and update job status.""" - try: - job = await analysis_repo.find_by_id(job_id) - if not job: - logger.error("Analysis job %s not found", job_id) - return - - job.mark_running() - await analysis_repo.update_status(job) - logger.info("Analysis started: %s (file: %s)", job_id, filename) - - # Build conversion options from pipeline dict - options = ConversionOptions(**(pipeline_options or {})) - - # Run blocking Docling conversion in a thread with timeout - result: ConversionResult = await asyncio.wait_for( - asyncio.to_thread(convert_document, file_path, options), - timeout=CONVERSION_TIMEOUT, - ) - - pages_json = json.dumps([asdict(p) for p in result.pages]) - - job.mark_completed( - markdown=result.content_markdown, - html=result.content_html, - pages_json=pages_json, - ) - await analysis_repo.update_status(job) - - # Update document page count if available - if result.page_count: - await document_repo.update_page_count(job.document_id, result.page_count) - - logger.info("Analysis completed: %s (%d pages)", job_id, result.page_count) - - except TimeoutError: - logger.error("Analysis timed out after %ds: %s", CONVERSION_TIMEOUT, job_id) - await _mark_failed(job_id, f"Conversion timed out after {CONVERSION_TIMEOUT}s") - - except Exception as e: - logger.exception("Analysis failed: %s", job_id) - await _mark_failed(job_id, str(e)) +def _schedule_mark_failed(job_id: str, error: str) -> None: + """Schedule _mark_failed as a tracked background task.""" + t = asyncio.ensure_future(_mark_failed(job_id, error)) + _background_tasks.add(t) + t.add_done_callback(_background_tasks.discard) async def _mark_failed(job_id: str, error: str) -> None: diff --git a/document-parser/services/document_service.py b/document-parser/services/document_service.py index f36825d..cba3d5c 100644 --- a/document-parser/services/document_service.py +++ b/document-parser/services/document_service.py @@ -70,10 +70,14 @@ async def delete(doc_id: str) -> bool: # Delete associated analyses first (cascade) await analysis_repo.delete_by_document(doc_id) - # Delete file from disk + # Delete file from disk (only if inside UPLOAD_DIR) try: - if os.path.exists(doc.storage_path): - os.unlink(doc.storage_path) + real_path = os.path.realpath(doc.storage_path) + real_upload_dir = os.path.realpath(UPLOAD_DIR) + if real_path.startswith(real_upload_dir + os.sep) and os.path.exists(real_path): + os.unlink(real_path) + elif os.path.exists(doc.storage_path): + logger.warning("Refused to delete file outside upload dir: %s", doc.storage_path) except OSError: logger.warning("Could not delete file: %s", doc.storage_path) diff --git a/document-parser/tests/test_analysis_service.py b/document-parser/tests/test_analysis_service.py new file mode 100644 index 0000000..ec26641 --- /dev/null +++ b/document-parser/tests/test_analysis_service.py @@ -0,0 +1,71 @@ +"""Tests for AnalysisService — focus on _on_task_done callback (bug #1 fix).""" + +from __future__ import annotations + +import asyncio +from unittest.mock import AsyncMock, patch + +import pytest + +from services.analysis_service import _on_task_done + + +class TestOnTaskDone: + """Bug #1: _on_task_done must call _mark_failed when the task raises.""" + + @pytest.mark.asyncio + async def test_exception_marks_job_failed(self): + """When a background task raises, the job should be marked FAILED.""" + job_id = "job-123" + + # Create a task that raises + async def failing_task(): + raise RuntimeError("boom") + + task = asyncio.create_task(failing_task()) + await asyncio.sleep(0) # let the task fail + + with patch("services.analysis_service._mark_failed", new_callable=AsyncMock) as mock_mark: + _on_task_done(task, job_id=job_id) + # ensure_future schedules it; give the event loop a tick + await asyncio.sleep(0) + + mock_mark.assert_called_once_with(job_id, "boom") + + @pytest.mark.asyncio + async def test_cancelled_task_marks_job_failed(self): + """When a background task is cancelled, the job should be marked FAILED.""" + job_id = "job-456" + + async def slow_task(): + await asyncio.sleep(999) + + import contextlib + + task = asyncio.create_task(slow_task()) + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + + with patch("services.analysis_service._mark_failed", new_callable=AsyncMock) as mock_mark: + _on_task_done(task, job_id=job_id) + await asyncio.sleep(0) + + mock_mark.assert_called_once_with(job_id, "Task was cancelled") + + @pytest.mark.asyncio + async def test_successful_task_does_not_mark_failed(self): + """When a background task succeeds, _mark_failed should not be called.""" + job_id = "job-789" + + async def ok_task(): + return "done" + + task = asyncio.create_task(ok_task()) + await task + + with patch("services.analysis_service._mark_failed", new_callable=AsyncMock) as mock_mark: + _on_task_done(task, job_id=job_id) + await asyncio.sleep(0) + + mock_mark.assert_not_called() diff --git a/document-parser/tests/test_api_endpoints.py b/document-parser/tests/test_api_endpoints.py index da10af9..173dd30 100644 --- a/document-parser/tests/test_api_endpoints.py +++ b/document-parser/tests/test_api_endpoints.py @@ -1,6 +1,6 @@ """Tests for FastAPI API endpoints using TestClient.""" -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from fastapi.testclient import TestClient @@ -14,11 +14,23 @@ def client(): return TestClient(app, raise_server_exceptions=False) +@pytest.fixture +def mock_analysis_service(client): + """Inject a mock AnalysisService into app.state for the duration of the test.""" + mock_svc = MagicMock() + original = getattr(app.state, "analysis_service", None) + app.state.analysis_service = mock_svc + yield mock_svc + app.state.analysis_service = original + + class TestHealthEndpoint: def test_health(self, client): resp = client.get("/health") assert resp.status_code == 200 - assert resp.json() == {"status": "ok"} + data = resp.json() + assert data["status"] == "ok" + assert "engine" in data class TestDocumentEndpoints: @@ -102,11 +114,10 @@ class TestDocumentEndpoints: class TestAnalysisEndpoints: - @patch("services.analysis_service.find_all", new_callable=AsyncMock) - def test_list_analyses(self, mock_find_all, client): - mock_find_all.return_value = [ + def test_list_analyses(self, client, mock_analysis_service): + mock_analysis_service.find_all = AsyncMock(return_value=[ AnalysisJob(id="j1", document_id="d1", document_filename="test.pdf"), - ] + ]) resp = client.get("/api/analyses") assert resp.status_code == 200 @@ -117,11 +128,10 @@ class TestAnalysisEndpoints: assert data[0]["documentFilename"] == "test.pdf" assert data[0]["status"] == "PENDING" - @patch("services.analysis_service.find_by_id", new_callable=AsyncMock) - def test_get_analysis(self, mock_find, client): + def test_get_analysis(self, client, mock_analysis_service): job = AnalysisJob(id="j1", document_id="d1", document_filename="test.pdf") job.mark_running() - mock_find.return_value = job + mock_analysis_service.find_by_id = AsyncMock(return_value=job) resp = client.get("/api/analyses/j1") assert resp.status_code == 200 @@ -129,31 +139,28 @@ class TestAnalysisEndpoints: assert data["status"] == "RUNNING" assert data["startedAt"] is not None - @patch("services.analysis_service.find_by_id", new_callable=AsyncMock) - def test_get_analysis_not_found(self, mock_find, client): - mock_find.return_value = None + def test_get_analysis_not_found(self, client, mock_analysis_service): + mock_analysis_service.find_by_id = AsyncMock(return_value=None) resp = client.get("/api/analyses/missing") assert resp.status_code == 404 - @patch("services.analysis_service.create", new_callable=AsyncMock) - def test_create_analysis(self, mock_create, client): - mock_create.return_value = AnalysisJob( + def test_create_analysis(self, client, mock_analysis_service): + mock_analysis_service.create = AsyncMock(return_value=AnalysisJob( id="j1", document_id="d1", document_filename="test.pdf", - ) + )) resp = client.post("/api/analyses", json={"documentId": "d1"}) assert resp.status_code == 200 data = resp.json() assert data["id"] == "j1" assert data["documentId"] == "d1" - mock_create.assert_called_once_with("d1", pipeline_options=None) + mock_analysis_service.create.assert_called_once_with("d1", pipeline_options=None) - @patch("services.analysis_service.create", new_callable=AsyncMock) - def test_create_analysis_with_pipeline_options(self, mock_create, client): - mock_create.return_value = AnalysisJob( + def test_create_analysis_with_pipeline_options(self, client, mock_analysis_service): + mock_analysis_service.create = AsyncMock(return_value=AnalysisJob( id="j2", document_id="d1", document_filename="test.pdf", - ) + )) resp = client.post("/api/analyses", json={ "documentId": "d1", @@ -174,7 +181,7 @@ class TestAnalysisEndpoints: data = resp.json() assert data["id"] == "j2" - call_kwargs = mock_create.call_args + call_kwargs = mock_analysis_service.create.call_args opts = call_kwargs.kwargs["pipeline_options"] assert opts["do_ocr"] is False assert opts["table_mode"] == "fast" @@ -182,12 +189,11 @@ class TestAnalysisEndpoints: assert opts["generate_picture_images"] is True assert opts["images_scale"] == 2.0 - @patch("services.analysis_service.create", new_callable=AsyncMock) - def test_create_analysis_with_partial_pipeline_options(self, mock_create, client): + def test_create_analysis_with_partial_pipeline_options(self, client, mock_analysis_service): """Pipeline options should use defaults for unspecified fields.""" - mock_create.return_value = AnalysisJob( + mock_analysis_service.create = AsyncMock(return_value=AnalysisJob( id="j3", document_id="d1", document_filename="test.pdf", - ) + )) resp = client.post("/api/analyses", json={ "documentId": "d1", @@ -195,38 +201,35 @@ class TestAnalysisEndpoints: }) assert resp.status_code == 200 - opts = mock_create.call_args.kwargs["pipeline_options"] + opts = mock_analysis_service.create.call_args.kwargs["pipeline_options"] assert opts["do_ocr"] is False # Defaults assert opts["do_table_structure"] is True assert opts["table_mode"] == "accurate" assert opts["do_code_enrichment"] is False - @patch("services.analysis_service.create", new_callable=AsyncMock) - def test_create_analysis_document_not_found(self, mock_create, client): - mock_create.side_effect = ValueError("Document not found: d99") + def test_create_analysis_document_not_found(self, client, mock_analysis_service): + mock_analysis_service.create = AsyncMock(side_effect=ValueError("Document not found: d99")) resp = client.post("/api/analyses", json={"documentId": "d99"}) assert resp.status_code == 404 - def test_create_analysis_empty_document_id(self, client): + def test_create_analysis_empty_document_id(self, client, mock_analysis_service): resp = client.post("/api/analyses", json={"documentId": ""}) assert resp.status_code == 400 - def test_create_analysis_whitespace_document_id(self, client): + def test_create_analysis_whitespace_document_id(self, client, mock_analysis_service): resp = client.post("/api/analyses", json={"documentId": " "}) assert resp.status_code == 400 - @patch("services.analysis_service.delete", new_callable=AsyncMock) - def test_delete_analysis(self, mock_delete, client): - mock_delete.return_value = True + def test_delete_analysis(self, client, mock_analysis_service): + mock_analysis_service.delete = AsyncMock(return_value=True) resp = client.delete("/api/analyses/j1") assert resp.status_code == 204 - @patch("services.analysis_service.delete", new_callable=AsyncMock) - def test_delete_analysis_not_found(self, mock_delete, client): - mock_delete.return_value = False + def test_delete_analysis_not_found(self, client, mock_analysis_service): + mock_analysis_service.delete = AsyncMock(return_value=False) resp = client.delete("/api/analyses/missing") assert resp.status_code == 404 diff --git a/document-parser/tests/test_pipeline_options.py b/document-parser/tests/test_pipeline_options.py index dd48a86..f402fdc 100644 --- a/document-parser/tests/test_pipeline_options.py +++ b/document-parser/tests/test_pipeline_options.py @@ -11,10 +11,12 @@ from docling.datamodel.pipeline_options import ( TableFormerMode, ) -from domain.parsing import ( - ConversionOptions, - build_converter, - convert_document, +from domain.value_objects import ConversionOptions +from infra.local_converter import ( + _build_docling_converter as build_converter, +) +from infra.local_converter import ( + _convert_sync as convert_document, ) # --------------------------------------------------------------------------- @@ -30,7 +32,7 @@ class TestBuildConverter: return fmt_opt.pipeline_options def test_defaults(self): - conv = build_converter() + conv = build_converter(ConversionOptions()) opts = self._get_pipeline_options(conv) assert opts.do_ocr is True assert opts.do_table_structure is True @@ -131,8 +133,8 @@ class TestBuildConverter: class TestConvertDocumentRouting: """Verify convert_document uses default converter for default opts, custom otherwise.""" - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_default_converter_with_all_defaults(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -143,13 +145,13 @@ class TestConvertDocumentRouting: mock_conv.convert.return_value = mock_result mock_get_default.return_value = mock_conv - convert_document("/tmp/test.pdf") + convert_document("/tmp/test.pdf", ConversionOptions()) mock_get_default.assert_called_once() mock_build.assert_not_called() - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_ocr_disabled(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -165,8 +167,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once() mock_get_default.assert_not_called() - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_table_mode_fast(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -182,8 +184,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once_with(opts) - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_code_enrichment_on(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -199,8 +201,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once_with(opts) - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_formula_enrichment_on(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -215,8 +217,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once() - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_picture_options_on(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -231,8 +233,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once() - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_generate_images_on(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -247,8 +249,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once() - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_uses_custom_converter_when_images_scale_changed(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -264,8 +266,8 @@ class TestConvertDocumentRouting: mock_build.assert_called_once_with(opts) - @patch("domain.parsing.get_default_converter") - @patch("domain.parsing.build_converter") + @patch("infra.local_converter._get_default_converter") + @patch("infra.local_converter._build_docling_converter") def test_forwards_all_options_to_build_converter(self, mock_build, mock_get_default): mock_conv = MagicMock() mock_result = MagicMock() @@ -312,25 +314,21 @@ class TestServiceForwardsPipelineOptions: @patch("services.analysis_service.document_repo") @patch("services.analysis_service.analysis_repo") - @patch("services.analysis_service._run_analysis") @pytest.mark.asyncio async def test_create_passes_pipeline_options_to_run( - self, mock_run, mock_analysis_repo, mock_doc_repo, mock_doc, + self, mock_analysis_repo, mock_doc_repo, mock_doc, ): mock_doc_repo.find_by_id = AsyncMock(return_value=mock_doc) mock_analysis_repo.insert = AsyncMock() - # Patch _run_analysis as a coroutine that we can inspect - mock_run.return_value = None - from services import analysis_service + mock_converter = AsyncMock() + from services.analysis_service import AnalysisService + svc = AnalysisService(converter=mock_converter) opts = {"do_ocr": False, "table_mode": "fast"} - # We need to patch asyncio.create_task to capture the coroutine args with patch("services.analysis_service.asyncio.create_task") as mock_task: - await analysis_service.create("d1", pipeline_options=opts) - - # create_task should have been called with _run_analysis(...) + await svc.create("d1", pipeline_options=opts) mock_task.assert_called_once() @patch("services.analysis_service.document_repo") @@ -342,32 +340,36 @@ class TestServiceForwardsPipelineOptions: mock_doc_repo.find_by_id = AsyncMock(return_value=mock_doc) mock_analysis_repo.insert = AsyncMock() - from services import analysis_service + mock_converter = AsyncMock() + from services.analysis_service import AnalysisService + svc = AnalysisService(converter=mock_converter) with patch("services.analysis_service.asyncio.create_task") as mock_task: - await analysis_service.create("d1") + await svc.create("d1") mock_task.assert_called_once() @patch("services.analysis_service.analysis_repo") @patch("services.analysis_service.document_repo") - @patch("services.analysis_service.convert_document") @pytest.mark.asyncio async def test_run_analysis_forwards_options_to_convert( - self, mock_convert, mock_doc_repo, mock_analysis_repo, mock_job, + self, mock_doc_repo, mock_analysis_repo, mock_job, ): - from domain.parsing import ConversionResult, PageDetail + from domain.value_objects import ConversionResult, PageDetail mock_analysis_repo.find_by_id = AsyncMock(return_value=mock_job) mock_analysis_repo.update_status = AsyncMock() mock_doc_repo.update_page_count = AsyncMock() - mock_convert.return_value = ConversionResult( + + mock_converter = AsyncMock() + mock_converter.convert.return_value = ConversionResult( page_count=1, content_markdown="# Test", content_html="
Text
", + "json_content": { + "pages": {"1": {"size": {"width": 612.0, "height": 792.0}}}, + "texts": [ + { + "label": "title", + "text": "Title", + "prov": [{"page_no": 1, "bbox": {"l": 10, "t": 20, "r": 200, "b": 40, "coord_origin": "TOPLEFT"}}], + }, + { + "label": "paragraph", + "text": "Text", + "prov": [{"page_no": 1, "bbox": {"l": 10, "t": 50, "r": 200, "b": 70, "coord_origin": "TOPLEFT"}}], + }, + ], + "tables": [], + "pictures": [], + }, + } + } + result = _parse_response(data) + assert len(result.pages[0].elements) == 2 + assert result.pages[0].elements[0].type == "title" + assert result.pages[0].elements[0].content == "Title" + assert result.pages[0].elements[0].bbox == [10, 20, 200, 40] + assert result.pages[0].elements[1].type == "text" + + def test_multi_page(self): + data = { + "document": { + "md_content": "", + "html_content": "", + "json_content": { + "pages": { + "1": {"size": {"width": 612.0, "height": 792.0}}, + "2": {"size": {"width": 595.0, "height": 842.0}}, + }, + "texts": [], "tables": [], "pictures": [], + }, + } + } + result = _parse_response(data) + assert result.page_count == 2 + assert result.pages[1].width == 595.0 + + def test_no_json_content(self): + data = { + "document": { + "md_content": "text", + "html_content": "text
", + } + } + result = _parse_response(data) + assert result.content_markdown == "text" + assert result.pages == [] + assert result.page_count == 1 + + def test_json_content_as_string(self): + json_doc = { + "pages": {"1": {"size": {"width": 612.0, "height": 792.0}}}, + "texts": [], "tables": [], "pictures": [], + } + data = { + "document": { + "md_content": "", + "html_content": "", + "json_content": json.dumps(json_doc), + } + } + result = _parse_response(data) + assert result.page_count == 1 + + def test_json_content_malformed_string_falls_back(self): + """Bug #5: malformed JSON string in json_content must not crash.""" + data = { + "document": { + "md_content": "# Hello", + "html_content": "