From 6951f59134510deaf05526b8346e72eedf700264 Mon Sep 17 00:00:00 2001 From: Arnav Agrawal Date: Tue, 13 Oct 2026 13:25:36 -0700 Subject: [PATCH] Convert legacy Office files before Docling and quiet server log noise Docling only reads OOXML, so a .doc attachment failed with "File format not allowed" and only came back through the slow PDF-and-OCR fallback. .xls and .rtf had no fallback at all. LibreOffice now converts .doc, .rtf and .odt to .docx, .ppt and .odp to .pptx, and .xls and .ods to .xlsx before parsing, so .xls also takes the openpyxl path. Each conversion gets its own LibreOffice profile, since concurrent runs that share one fail. If conversion fails, the original file goes to Docling as before. litellm's aiohttp transport caches a ClientSession per client for an hour, then drops it without closing it. That logged "Unclosed client session" every hour from the API and the worker, in bursts when the cache evicted many at once. Core now uses litellm's httpx transport. GET /documents/{id} logged every 404 at ERROR. The Documents page polls each processing document every 2 s and never stopped for one that was deleted, so one deleted document filled the log. The route now logs a miss at DEBUG, and the page refreshes the list on a 404, which drops the document and stops the polling. Co-Authored-By: Claude Opus 5.5 (1M context) --- core/completion/litellm_completion.py | 4 + core/embedding/litellm_embedding.py | 5 + core/parser/morphik_parser.py | 60 +++++++--- core/routes/documents.py | 16 ++- core/tests/unit/test_legacy_office_parsing.py | 109 ++++++++++++++++++ .../components/documents/DocumentsSection.tsx | 9 +- 6 files changed, 180 insertions(+), 23 deletions(-) create mode 100644 core/tests/unit/test_legacy_office_parsing.py 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}`); }