chore(train): land vehicle, 4 approved PRs

.session/HANDOFF-issue-16784-16785-document-format-gaps.md

@@ -0,0 +1,22 @@
+# Handoff: issue-16784-16785-document-format-gaps
+status: partial
+pr: #16790
+base_at_push: c6af9cf0d
+gates: pre-commit=PASS on every commit | pre-push=PASS (document_extractors_16785_test.py) | file-size ratchet=PASS | CI: 0 failures at head 327d092cf, some checks still pending at handoff — not polled further, per this repo's "never wait on CI" rule
+needs_rebase_before_merge: no (just refreshed against current origin/main via merge, not rebase, to keep any future review stamp valid as a merge-commit re-read)
+remaining:
+  - #16785 (this PR, #16790) is otherwise complete: all 4 acceptance criteria met, 7 tests, body reheadered to the repo's required Thinking Path / What Changed / Verification / Model Used sections plus a Single-issue rationale line. Just needs review + merge.
+  - #16784 is NOT in this PR. It is blocked_by #16773 (recorded natively on GitHub, not just here) — #16773 (PR #16783, owned by peer session autobot-ai-42) extends detect_format() to distinguish the six office/ODF ZIP formats, which #16784's fix needs to route on. #16773 was still unmerged and being refreshed by 42 as of this handoff. Once it lands, #16784 goes in its own PR: dispatch the six office/ODF formats from extract_document() (media/document/extraction.py) to DocumentParser (utils/document_parser.py, already has working parsers for all six via utils/document_extractors.py's extract_from_office), and decide explicitly what an *unverified* ZIP does (42's stated view: keep falling through to extract_plain_text, since a plain ZIP is not a document and a hard failure there would be a new refusal, not a fix).
+  - Umbrella #16772 has two more children not staffed to this session: #16775 (KB upload allowlist gap, staffed to autobot-ai-42) and #16786 (.doc format advertised but always fails — filed, needs an owner call on whether to add an OLE2 reader, not staffed to anyone as of this handoff).
+worktree: .worktrees/issue-16784-16785-document-format-gaps  (safe to remove after #16790 merges; #16784's future PR will need a fresh worktree once #16773 lands, this one should not be reused for it since it's tied to #16790's already-pushed history)
+
+done:
+  - DocumentExtractor.SUPPORTED_FORMATS gained .csv (joined the existing "text" category), plus new "json"/"html" categories. New extract_from_json() mirrors api.knowledge._extract_file_content's exact structure including its edge case (JSONDecodeError falls back to raw text; non-UTF-8 bytes still raise, since the fallback only runs after JSONDecodeError). New extract_from_html() imports and calls api.knowledge._sanitize_html_content at call time rather than reimplementing it.
+  - get_supported_extensions()/is_supported_format() pick the three up automatically (both already iterate SUPPORTED_FORMATS generically) — knowledge/documents.py's directory-discovery docstring updated to match what's actually handled now.
+  - Recorded #16784 blocked_by #16773 as a native GitHub dependency edge (not just prose), after coordinating with autobot-ai-42 (who owns media/document/ for #16773) rather than duplicating their in-progress detection work or editing their file mid-flight.
+  - changelog/unreleased/16785-document-extractor-csv-json-html.md added.
+
+notes:
+  - Ledger: this session (autobot-ai-01) holds active claims on issues 16707, 16708, 16784, 16785 and worktree issue-16707-16708-kb-bugfix-batch as of handoff. Session is ending; treat these as available to re-claim rather than waiting out the TTL.
+  - Fleet context at handoff (from cross-session coordination this session, not independently re-verified — confirm before acting on it): autobot-ai-71 is coordinating the merge queue and has been requiring a fresh-base refresh before merge even for trivial-looking dependency-only drift, deliberately, for consistency; autobot-ai-42 owns knowledge/facts.py, media/document/zip_formats.py (#16773) and api/knowledge.py (#16775); autobot-ai-21 owns services/knowledge/service.py and advanced_rag_optimizer.py.
+  - A separate, small piece of work never landed: a /research pass on document-conversion patterns (comparing AutoBot against an anonymized reference work per this repo's research-skill rules) produced docs/research/document-to-markdown-conversion-pipeline.md and issues #16772 (umbrella)/#16773/#16774/#16775. The doc file itself was written directly to the main tree by mistake (not a worktree), then salvaged out to /tmp/claude-1000/-home-martins-AutoBot-Ai-AutoBot-AI/06a96e9f-de2b-45f3-a873-7de8247a847a/scratchpad/research-doc-salvage/ when the worktree ceiling (15/15) blocked creating a proper worktree to commit it from. It has never been committed to the repo. No branch exists for it, so no handoff file covers it — flagging here instead. Needs: a worktree once the ceiling has room, add both salvaged files (_index.md's one new line, the new document-to-markdown-conversion-pipeline.md file), commit, push, small doc-only PR.

autobot-backend/api/knowledge.py

@@ -46,6 +46,7 @@
     Request,
 )
 
+from api.knowledge_office_upload import OFFICE_EXTENSIONS, extract_office_upload, verified_upload_extension
 from api.schemas_knowledge import (
     AddFactsRequest,
     AddUrlRequest,
@@ -123,7 +124,7 @@
 # File upload constants (Issue #549 Code Review)
 MAX_FILE_SIZE_MB = 10
 MAX_FILE_SIZE_BYTES = MAX_FILE_SIZE_MB * 1024 * 1024
-ALLOWED_EXTENSIONS = {".txt", ".md", ".pdf", ".docx", ".json", ".csv", ".html"}
+ALLOWED_EXTENSIONS = {".txt", ".md", ".pdf", ".docx", ".json", ".csv", ".html"} | OFFICE_EXTENSIONS  # #16775
 
 # Import RAG Agent for enhanced search capabilities
 try:
@@ -921,10 +922,9 @@ async def _fetch_and_extract_url(url: str, fallback_title: str) -> "tuple[str, s
 
     from autobot_shared.security.ssrf_guard import SSRFError, fetch_safe_url
 
-    # Imported locally rather than at module scope: `config` is already a local
-    # name in add_watch_folder (a WatchFolderConfig), so a module-level import
-    # would be silently shadowed there. Kept outside the try so an ImportError
-    # surfaces as itself instead of as a fetch failure.
+    # Imported locally rather than at module scope: `config` is already a local name in add_watch_folder (a
+    # WatchFolderConfig), so a module-level import would be silently shadowed there. Kept outside the try so an
+    # ImportError surfaces as itself instead of as a fetch failure.
     from autobot_shared.ssot_config import config
 
     try:
@@ -1088,9 +1088,10 @@ def _extract_file_content(filename: str, file_content: bytes) -> "tuple[str, Ext
     Raises:
         HTTPException: If file cannot be parsed or library is missing
     """
-    import os
+    ext = verified_upload_extension(filename, file_content)  # #16773: the bytes outrank the name
 
-    ext = os.path.splitext(filename.lower())[1]
+    if ext in OFFICE_EXTENSIONS:
+        return extract_office_upload(filename, file_content, ext), None
 
     if ext in {".txt", ".md", ".csv"}:
         return file_content.decode("utf-8", errors="replace"), None
@@ -1251,10 +1252,9 @@ async def upload_file_to_knowledge(
     category = form.get("category", "uploads")
     tags = _parse_upload_tags(form.get("tags", "[]"))
 
-    # #14754: _extract_file_content does blocking CPU work — PDF parsing plus
-    # pdfplumber layout analysis on every page — and this handler is async, so a
-    # large upload held the worker's event loop for the whole extraction and
-    # stalled every other coroutine on it, health endpoints included.
+    # #14754: _extract_file_content does blocking CPU work — PDF parsing plus pdfplumber layout analysis on every page
+    # — and this handler is async, so a large upload held the worker's event loop for the whole extraction and stalled
+    # every other coroutine on it, health endpoints included.
     from media.document.ocr import extraction_timeout
 
     _deadline = extraction_timeout()
@@ -1264,9 +1264,8 @@ async def upload_file_to_knowledge(
             timeout=_deadline,
         )
     except asyncio.TimeoutError:
-        # The deadline is what makes the offload safe: to_thread frees the loop
-        # but the default executor's slots are process-wide and shared with the
-        # OCR path, so an extraction that never returns holds one indefinitely.
+        # The deadline is what makes the offload safe: to_thread frees the loop but the default executor's slots are
+        # process-wide and shared with the OCR path, so an extraction that never returns holds one indefinitely.
         # Reported as a rejected upload rather than left to hang (#14754).
         logger.warning("Extraction of %s exceeded %ss", filename, _deadline)
         raise HTTPException(
@@ -2325,10 +2324,9 @@ async def get_facts_by_category(
     try:
         category_fact_ids, category_totals = await _fetch_category_fact_ids(kb, categories_to_fetch, limit, offset)
         if not category_totals:
-            # Issue #12394: fall back only when NO category index exists at all.
-            # (Previously this checked `category_fact_ids`, which is also empty
-            # for a legitimate out-of-range page on an existing index, wrongly
-            # triggering the expensive full-keyspace SCAN fallback.)
+            # Issue #12394: fall back only when NO category index exists at all. (Previously this checked
+            # `category_fact_ids`, which is also empty for a legitimate out-of-range page on an existing index,
+            # wrongly triggering the expensive full-keyspace SCAN fallback.)
             logger.warning("No category indexes - falling back to SCAN method")
             return await _get_facts_by_category_legacy(kb, category, limit, offset)
 

autobot-backend/api/knowledge_office_upload.py

@@ -0,0 +1,77 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""Spreadsheet, presentation and OpenDocument uploads for the KB (#16775).
+
+``DocumentParser`` has carried working parsers for these formats all along; the upload
+form's allowlist simply never named them, so a user could not upload a file the backend
+could already read. This is the routing that closes that gap -- no new extraction logic.
+
+It deliberately does not extend the allowlist to the legacy binary formats: ``.doc`` and
+``.ppt`` are OLE2 containers, and the parsers behind them (python-docx, python-pptx) read
+only the OOXML ones. Advertising them would accept a valid file and then report it
+corrupt, which is #16786, an owner decision of its own. ``.odg`` is left out too --
+drawings carry almost no extractable text, and this endpoint stores flattened text only.
+
+Format is resolved from content first (#16773), so a renamed upload reaches the parser
+its bytes call for rather than the one its name claims.
+"""
+
+from __future__ import annotations
+
+import os
+import tempfile
+from pathlib import Path
+
+from fastapi import HTTPException
+
+from autobot_shared.logging_manager import get_logger
+from media.document.zip_formats import verified_suffix
+
+logger = get_logger(__name__)
+
+#: Handled through DocumentParser. Every one is a ZIP container, so #16773's content
+#: verification covers the whole set.
+OFFICE_EXTENSIONS = {".xlsx", ".pptx", ".odt", ".ods", ".odp"}
+
+
+def verified_upload_extension(filename: str, file_content: bytes) -> str:
+    """The extension to dispatch on: what the bytes say, else what the name says (#16773).
+
+    The allowlist upstream still rules on the *name* — this decides only which parser
+    reads an already-accepted upload, so a mislabeled file is parsed correctly instead
+    of being handed to the wrong parser.
+    """
+    named = os.path.splitext(filename.lower())[1]
+    verified = verified_suffix(file_content)
+    if verified and verified != named:
+        logger.info("Upload %s: content is %s, name says %s — parsing as %s", filename, verified, named, verified)
+        return verified
+    return named
+
+
+def extract_office_upload(filename: str, file_content: bytes, ext: str) -> str:
+    """Text of an uploaded spreadsheet, presentation or OpenDocument file.
+
+    ``DocumentParser`` reads from a path, so the bytes land in a temp file carrying the
+    *verified* suffix. Runs blocking work on purpose: the caller
+    (``api.knowledge._extract_file_content``) is already off the event loop in
+    ``asyncio.to_thread``.
+
+    Raises:
+        HTTPException: 400 when no candidate parser could read the document, matching
+            how the pdf and docx helpers next to the caller report a bad upload.
+    """
+    from utils.document_parser import parse_document_text
+
+    with tempfile.NamedTemporaryFile(suffix=ext) as handle:
+        handle.write(file_content)
+        handle.flush()
+        text, metadata = parse_document_text(Path(handle.name))
+
+    if not metadata.get("extraction_success"):
+        detail = metadata.get("extraction_error") or "no parser could read it"
+        raise HTTPException(status_code=400, detail=f"Failed to parse {ext.lstrip('.')} file: {detail}")
+    logger.info("Parsed %s upload %s as %s (%d chars)", ext, filename, metadata.get("format"), len(text))
+    return text

autobot-backend/api/knowledge_office_upload_16775_test.py

@@ -0,0 +1,113 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+# AutoBot - AI-Powered Automation Platform
+# Author: mrveiss
+"""The KB upload form accepts the office formats the backend already parses (#16775).
+
+``DocumentParser`` has carried these parsers all along; only the upload allowlist left
+them out, so a user could not upload a file the backend could read by another path.
+
+Dispatch goes through the verified format (#16773), not the filename, so widening the
+allowlist does not reintroduce the extension-trust this same surface just removed.
+"""
+
+from __future__ import annotations
+
+import zipfile
+from io import BytesIO
+from unittest.mock import patch
+
+import pytest
+from fastapi import HTTPException
+
+from api.knowledge_office_upload import OFFICE_EXTENSIONS, extract_office_upload, verified_upload_extension
+
+
+def _ooxml_bytes(part: str) -> bytes:
+    buffer = BytesIO()
+    with zipfile.ZipFile(buffer, "w") as archive:
+        archive.writestr("[Content_Types].xml", "<Types/>")
+        archive.writestr(part, "<xml/>")
+    return buffer.getvalue()
+
+
+_XLSX = _ooxml_bytes("xl/workbook.xml")
+
+
+def test_the_allowlist_gained_the_formats_the_parsers_already_read():
+    from api.knowledge import ALLOWED_EXTENSIONS
+
+    assert OFFICE_EXTENSIONS <= ALLOWED_EXTENSIONS
+    assert {".xlsx", ".pptx", ".odt", ".ods", ".odp"} <= ALLOWED_EXTENSIONS
+
+
+def test_the_legacy_binary_formats_stay_out():
+    """.doc/.ppt are OLE2; python-docx and python-pptx read only the OOXML ones.
+
+    Advertising them would accept a valid file and then call it corrupt — #16786, which
+    needs an owner call on whether to add a real OLE2 reader or stop advertising it.
+    """
+    from api.knowledge import ALLOWED_EXTENSIONS
+
+    assert ".doc" not in ALLOWED_EXTENSIONS
+    assert ".ppt" not in ALLOWED_EXTENSIONS
+
+
+def test_a_renamed_upload_dispatches_on_its_content():
+    assert verified_upload_extension("notes.txt", _XLSX) == ".xlsx"
+
+
+def test_an_honest_upload_keeps_its_own_extension():
+    assert verified_upload_extension("budget.xlsx", _XLSX) == ".xlsx"
+
+
+def test_a_non_archive_upload_keeps_its_name():
+    assert verified_upload_extension("notes.txt", b"plain words") == ".txt"
+
+
+def test_an_office_upload_is_routed_to_the_parser():
+    from api.knowledge import _extract_file_content
+
+    with patch("api.knowledge.extract_office_upload", return_value="sheet text") as office:
+        content, extracted = _extract_file_content("budget.xlsx", _XLSX)
+
+    assert (content, extracted) == ("sheet text", None)
+    assert office.call_args.args[2] == ".xlsx"
+
+
+def test_a_mislabeled_office_upload_reaches_the_right_parser():
+    """The failure the allowlist widening must not reintroduce: trusting the name."""
+    from api.knowledge import _extract_file_content
+
+    with patch("api.knowledge.extract_office_upload", return_value="sheet text") as office:
+        content, _ = _extract_file_content("notes.txt", _XLSX)
+
+    assert content == "sheet text"
+    assert office.call_args.args[2] == ".xlsx"
+
+
+def test_a_plain_text_upload_is_untouched():
+    from api.knowledge import _extract_file_content
+
+    assert _extract_file_content("notes.txt", b"hello there") == ("hello there", None)
+
+
+def test_extract_office_upload_returns_the_parsed_text():
+    parsed = ("Sheet1\nrevenue\t42", {"extraction_success": True, "format": ".xlsx"})
+
+    with patch("utils.document_parser.parse_document_text", return_value=parsed) as parse:
+        text = extract_office_upload("budget.xlsx", _XLSX, ".xlsx")
+
+    assert text == "Sheet1\nrevenue\t42"
+    assert parse.call_args.args[0].suffix == ".xlsx", "the temp file carries the verified suffix"
+
+
+def test_an_unparseable_office_upload_is_a_400():
+    failed = ("", {"extraction_success": False, "extraction_error": "not a workbook"})
+
+    with patch("utils.document_parser.parse_document_text", return_value=failed):
+        with pytest.raises(HTTPException) as raised:
+            extract_office_upload("budget.xlsx", _XLSX, ".xlsx")
+
+    assert raised.value.status_code == 400
+    assert "not a workbook" in raised.value.detail

autobot-backend/knowledge/documents.py

@@ -144,8 +144,9 @@ async def add_document_from_file(
         ``read_text(encoding="utf-8")``, which reads a PDF as UTF-8 — raising on
         the binary header or, worse, storing mojibake. Extraction now goes
         through DocumentExtractor, so the advertised formats are the handled
-        ones: PDF, DOC/DOCX, plain text, and the spreadsheet / presentation /
-        OpenDocument set it delegates to DocumentParser.
+        ones: PDF, DOC/DOCX, plain text (including CSV #16785), JSON and HTML
+        (#16785), and the spreadsheet / presentation / OpenDocument set it
+        delegates to DocumentParser.
 
         Args:
             file_path: Path to the file

autobot-backend/media/document/extraction.py

@@ -39,6 +39,7 @@
 from autobot_shared.env_utils import blank_to_none
 from autobot_shared.logging_manager import get_logger
 from autobot_shared.ssot_config import config
+from media.document.zip_formats import sniff_zip_format
 
 logger = get_logger(__name__)
 
@@ -311,8 +312,15 @@ def detect_format(raw: bytes, mime_type: str = "") -> str:
     mime = (mime_type or "").lower()
     if raw[:4] == _PDF_MAGIC:
         return "pdf"
-    if raw[:2] == _ZIP_MAGIC and _DOCX_MARKER in raw[:_DOCX_SNIFF_BYTES]:
-        return "docx"
+    if raw[:2] == _ZIP_MAGIC:
+        # #16773: all seven office/ODF formats carry the same PK prefix, so the prefix
+        # cannot tell them apart -- the archive's own members can. The marker sniff below
+        # still covers a truncated upload, whose central directory has not arrived yet.
+        verified = sniff_zip_format(raw)
+        if verified:
+            return verified
+        if _DOCX_MARKER in raw[:_DOCX_SNIFF_BYTES]:
+            return "docx"
     if "pdf" in mime:
         return "pdf"
     if "docx" in mime or "officedocument.wordprocessingml" in mime: