diff --git a/core/completion/litellm_completion.py b/core/completion/litellm_completion.py index ee75dbe0..8f550164 100644 --- a/core/completion/litellm_completion.py +++ b/core/completion/litellm_completion.py @@ -18,6 +18,10 @@ logger = logging.getLogger(__name__) +# litellm's default aiohttp transport caches one ClientSession per client for an hour, then drops it without +# closing it, which logs "Unclosed client session" from both the API and the worker. httpx doesn't. +litellm.disable_aiohttp_transport = True + def get_system_message(inline_citations: bool = False) -> Dict[str, str]: """Return the standard system message for Morphik's query agent.""" diff --git a/core/embedding/litellm_embedding.py b/core/embedding/litellm_embedding.py index 5130c0a6..0b7c08c9 100644 --- a/core/embedding/litellm_embedding.py +++ b/core/embedding/litellm_embedding.py @@ -9,6 +9,11 @@ from core.models.chunk import Chunk logger = logging.getLogger(__name__) + +# litellm's default aiohttp transport caches one ClientSession per client for an hour, then drops it without +# closing it, which logs "Unclosed client session" from both the API and the worker. httpx doesn't. +litellm.disable_aiohttp_transport = True + PGVECTOR_MAX_DIMENSIONS = 2000 # Maximum dimensions for pgvector # OpenAI embedding models reject the whole request if any single input exceeds 8192 tokens. # Chunks are sized in characters, and CJK text costs ~2 tokens per character, so a diff --git a/core/parser/morphik_parser.py b/core/parser/morphik_parser.py index 315d272b..9462a1b5 100644 --- a/core/parser/morphik_parser.py +++ b/core/parser/morphik_parser.py @@ -1,3 +1,4 @@ +import asyncio import base64 import io import logging @@ -366,29 +367,42 @@ def _is_fast_excel_file(self, filename: str) -> bool: @staticmethod def _office_suffix(filename: str) -> Optional[str]: suffix = Path(filename).suffix.lower() - if suffix in {".docx", ".doc", ".pptx", ".ppt"}: + if suffix in {".docx", ".doc", ".pptx", ".ppt", ".xls", ".rtf", ".odt", ".odp", ".ods"}: return suffix return None + # Docling only reads the OOXML formats, so LibreOffice converts these first. + _LEGACY_OFFICE_TARGETS = { + ".doc": "docx", + ".rtf": "docx", + ".odt": "docx", + ".ppt": "pptx", + ".odp": "pptx", + ".xls": "xlsx", + ".ods": "xlsx", + } + @staticmethod - def _convert_office_to_pdf_bytes(file: bytes, suffix: str) -> Optional[bytes]: - """Convert Office bytes to PDF bytes for fallback parsing.""" + def _convert_with_libreoffice(file: bytes, suffix: str, target: str) -> Optional[bytes]: + """Convert document bytes to the `target` format (pdf, docx, pptx, xlsx) with LibreOffice.""" if not shutil.which("soffice"): - logger.warning("LibreOffice (soffice) not found in PATH; cannot run Office-to-PDF parse fallback.") + logger.warning("LibreOffice (soffice) not found in PATH; cannot convert %s to %s.", suffix, target) return None with tempfile.NamedTemporaryFile(suffix=suffix, delete=False) as temp_input: temp_input.write(file) temp_input_path = temp_input.name - with tempfile.TemporaryDirectory() as output_dir: + with tempfile.TemporaryDirectory() as output_dir, tempfile.TemporaryDirectory() as profile_dir: try: result = subprocess.run( [ "soffice", + # A private profile per call. Concurrent conversions sharing the default one fail. + f"-env:UserInstallation={Path(profile_dir).as_uri()}", "--headless", "--convert-to", - "pdf", + target, "--outdir", output_dir, temp_input_path, @@ -399,22 +413,24 @@ def _convert_office_to_pdf_bytes(file: bytes, suffix: str) -> Optional[bytes]: ) if result.returncode != 0: logger.warning( - "LibreOffice fallback conversion failed: %s", + "LibreOffice %s to %s conversion failed: %s", + suffix, + target, result.stderr.strip() or result.stdout.strip(), ) return None - pdf_path = Path(output_dir) / f"{Path(temp_input_path).stem}.pdf" - if not pdf_path.exists() or pdf_path.stat().st_size == 0: - logger.warning("LibreOffice fallback conversion produced no PDF content") + output_path = Path(output_dir) / f"{Path(temp_input_path).stem}.{target}" + if not output_path.exists() or output_path.stat().st_size == 0: + logger.warning("LibreOffice %s to %s conversion produced no output", suffix, target) return None - return pdf_path.read_bytes() + return output_path.read_bytes() except subprocess.TimeoutExpired: - logger.warning("LibreOffice fallback conversion timed out") + logger.warning("LibreOffice %s to %s conversion timed out", suffix, target) return None except Exception as e: - logger.warning("LibreOffice fallback conversion failed unexpectedly: %s", e) + logger.warning("LibreOffice %s to %s conversion failed unexpectedly: %s", suffix, target, e) return None finally: try: @@ -422,6 +438,22 @@ def _convert_office_to_pdf_bytes(file: bytes, suffix: str) -> Optional[bytes]: except OSError: pass + @classmethod + def _convert_office_to_pdf_bytes(cls, file: bytes, suffix: str) -> Optional[bytes]: + """Convert Office bytes to PDF bytes for fallback parsing.""" + return cls._convert_with_libreoffice(file, suffix, "pdf") + + async def _convert_legacy_office(self, file: bytes, filename: str) -> Tuple[bytes, str]: + """Convert .doc, .ppt, .xls and similar to the OOXML format Docling reads. Other files pass through.""" + suffix = Path(filename).suffix.lower() + target = self._LEGACY_OFFICE_TARGETS.get(suffix) + if not target: + return file, filename + converted = await asyncio.to_thread(self._convert_with_libreoffice, file, suffix, target) + if not converted: + return file, filename + return converted, f"{Path(filename).stem}.{target}" + @staticmethod def _build_deep_pdf_converter() -> DocumentConverter: """Build an uncached Docling converter for expensive fallback parsing.""" @@ -670,6 +702,8 @@ async def _parse_document(self, file: bytes, filename: str) -> Tuple[Dict[str, A text = file.decode("latin-1") return {}, text + file, filename = await self._convert_legacy_office(file, filename) + # Fast path for Excel files — openpyxl is orders of magnitude faster than Docling if self._is_fast_excel_file(filename): try: diff --git a/core/routes/documents.py b/core/routes/documents.py index 95c4bb5b..521001a6 100644 --- a/core/routes/documents.py +++ b/core/routes/documents.py @@ -156,15 +156,13 @@ async def get_document(document_id: str, auth: AuthContext = Depends(verify_toke Returns the :class:`Document` metadata if found or raises 404. """ - try: - doc = await document_service.db.get_document(document_id, auth) - logger.debug(f"Found document: {doc}") - if not doc: - raise HTTPException(status_code=404, detail="Document not found") - return doc - except HTTPException as e: - logger.error(f"Error getting document: {e}") - raise e + doc = await document_service.db.get_document(document_id, auth) + if not doc: + # Clients poll documents that may since have been deleted, so a miss is not a server error. + logger.debug(f"Document {document_id} not found") + raise HTTPException(status_code=404, detail="Document not found") + logger.debug(f"Found document: {doc}") + return doc @router.get("/{document_id}/status", response_model=Dict[str, Any]) diff --git a/core/tests/unit/test_legacy_office_parsing.py b/core/tests/unit/test_legacy_office_parsing.py new file mode 100644 index 00000000..8ced5b42 --- /dev/null +++ b/core/tests/unit/test_legacy_office_parsing.py @@ -0,0 +1,109 @@ +"""Legacy Office files (.doc, .ppt, .xls, ...) are converted with LibreOffice before Docling sees them.""" + +import shutil +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from core.parser import morphik_parser +from core.parser.morphik_parser import MorphikParser + +pytestmark = pytest.mark.unit + + +def _soffice_works() -> bool: + if not shutil.which("soffice"): + return False + try: + return subprocess.run(["soffice", "--version"], capture_output=True, timeout=60).returncode == 0 + except (OSError, subprocess.TimeoutExpired): + return False + + +@pytest.fixture +def parser(monkeypatch): + monkeypatch.setattr( + morphik_parser, "get_settings", lambda: SimpleNamespace(REGISTERED_MODELS={}, PARSER_MODE="local") + ) + parser = MorphikParser() + parser.docling_calls = [] + + async def docling(file, filename): + parser.docling_calls.append((file, filename)) + return "docling text" + + parser._parse_document_local = docling + return parser + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "filename,target", + [("policy.doc", "docx"), ("notes.rtf", "docx"), ("deck.ppt", "pptx"), ("slides.odp", "pptx")], +) +async def test_legacy_files_reach_docling_converted(parser, monkeypatch, filename, target): + conversions = [] + + def convert(file, suffix, to): + conversions.append((file, suffix, to)) + return b"converted" + + monkeypatch.setattr(MorphikParser, "_convert_with_libreoffice", staticmethod(convert)) + + _, text = await parser.parse_file_to_text(b"legacy", filename) + + assert text == "docling text" + assert conversions == [(b"legacy", Path(filename).suffix, target)] + assert parser.docling_calls == [(b"converted", f"{Path(filename).stem}.{target}")] + + +@pytest.mark.asyncio +async def test_converted_xls_uses_fast_excel_path(parser, monkeypatch): + monkeypatch.setattr(MorphikParser, "_convert_with_libreoffice", staticmethod(lambda file, suffix, to: b"xlsx")) + monkeypatch.setattr(parser, "_parse_excel_to_markdown", lambda file: f"sheet from {file.decode()}") + + _, text = await parser.parse_file_to_text(b"legacy", "budget.xls") + + assert text == "sheet from xlsx" + assert parser.docling_calls == [] + + +@pytest.mark.asyncio +async def test_failed_conversion_falls_back_to_original_file(parser, monkeypatch): + monkeypatch.setattr(MorphikParser, "_convert_with_libreoffice", staticmethod(lambda file, suffix, to: None)) + + await parser.parse_file_to_text(b"legacy", "policy.doc") + + assert parser.docling_calls == [(b"legacy", "policy.doc")] + + +@pytest.mark.asyncio +async def test_modern_files_are_not_converted(parser, monkeypatch): + def convert(*args): + raise AssertionError("should not convert") + + monkeypatch.setattr(MorphikParser, "_convert_with_libreoffice", staticmethod(convert)) + + await parser.parse_file_to_text(b"modern", "policy.docx") + + assert parser.docling_calls == [(b"modern", "policy.docx")] + + +@pytest.mark.skipif(not _soffice_works(), reason="LibreOffice is not installed") +def test_libreoffice_converts_doc_to_docx(tmp_path): + source = tmp_path / "policy.txt" + source.write_text("Break time policy") + subprocess.run( + ["soffice", f"-env:UserInstallation={(tmp_path / 'profile').as_uri()}", "--headless", "--convert-to", "doc"] + + ["--outdir", str(tmp_path), str(source)], + check=True, + capture_output=True, + timeout=120, + ) + + docx = MorphikParser._convert_with_libreoffice((tmp_path / "policy.doc").read_bytes(), ".doc", "docx") + + assert docx is not None + assert docx[:2] == b"PK" # OOXML files are zip archives diff --git a/ee/ui-component/components/documents/DocumentsSection.tsx b/ee/ui-component/components/documents/DocumentsSection.tsx index 09213ac4..bbf339cf 100644 --- a/ee/ui-component/components/documents/DocumentsSection.tsx +++ b/ee/ui-component/components/documents/DocumentsSection.tsx @@ -349,6 +349,11 @@ const DocumentsSection = React.forwardRef< const updatedDoc = await response.json(); return updatedDoc; } + if (response.status === 404) { + // Deleted while processing. Refreshing drops it from the list, which stops polling it. + console.log(`Document ${doc.external_id} no longer exists`); + return { deleted: true }; + } return null; }); @@ -357,7 +362,9 @@ const DocumentsSection = React.forwardRef< // Check if any documents have changed status let hasChanges = false; updatedDocs.forEach(updatedDoc => { - if (updatedDoc && updatedDoc.system_metadata?.status !== "processing") { + if (updatedDoc?.deleted) { + hasChanges = true; + } else if (updatedDoc && updatedDoc.system_metadata?.status !== "processing") { hasChanges = true; console.log(`Document ${updatedDoc.external_id} status changed to: ${updatedDoc.system_metadata?.status}`); }