Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9a862e8
feat(documents): verify ZIP-based office and ODF formats by content (…
mrveiss Sep 16, 2026
586a9c3
chore: claim worktree for #16784, #16785
mrveiss Sep 16, 2026
7bc1f19
feat(kb): accept spreadsheet, presentation and OpenDocument uploads (…
mrveiss Sep 16, 2026
3250fbd
feat(knowledge): DocumentExtractor handles .csv/.json/.html like the …
mrveiss Sep 16, 2026
6ac93b8
chore(documents): merge main into the #16773 format-verification bran…
mrveiss Sep 16, 2026
327d092
docs(changelog): add fragment for #16785 (DocumentExtractor csv/json/…
mrveiss Sep 16, 2026
590681f
chore: merge main into #16790 (refresh behind by #16597, #16761)
mrveiss Sep 16, 2026
93c5155
chore(session): write end-of-session handoff for issue-16784-16785-do…
mrveiss Sep 16, 2026
2e12386
fix(session): short SHA in the handoff — a 40-hex string trips detect…
mrveiss Sep 16, 2026
9456e40
Merge branch 'main' into issue-16773-zip-format-verify
mrveiss Sep 16, 2026
8ca011e
Merge branch 'issue-16773-zip-format-verify' into issue-16775-kb-uplo…
mrveiss Sep 17, 2026
ab0782a
Merge branch 'main' into issue-16784-16785-document-format-gaps
mrveiss Sep 17, 2026
6d92ede
Merge branch 'main' into issue-16773-zip-format-verify
mrveiss Sep 17, 2026
6d448cc
Merge branch 'main' into issue-16784-16785-document-format-gaps
mrveiss Sep 17, 2026
0d07b8a
Merge branch 'main' into issue-16773-zip-format-verify
mrveiss Sep 17, 2026
c2bfc01
Merge branch 'issue-16773-zip-format-verify' into issue-16775-kb-uplo…
mrveiss Sep 17, 2026
8434d84
Merge branch 'main' into issue-16784-16785-document-format-gaps
mrveiss Sep 17, 2026
dcbe52d
Merge branch 'main' into issue-16773-zip-format-verify
mrveiss Sep 17, 2026
016c221
Merge branch 'main' into issue-16773-zip-format-verify
github-actions[bot] Sep 18, 2026
4ed3c20
Merge branch 'main' into issue-16784-16785-document-format-gaps
mrveiss Sep 18, 2026
ca95302
Merge branch 'main' into issue-16773-zip-format-verify
mrveiss Sep 18, 2026
c5f51c0
Merge branch 'main' into issue-16784-16785-document-format-gaps
mrveiss Sep 18, 2026
cfb0d49
Merge branch 'issue-16773-zip-format-verify' into issue-16775-kb-uplo…
mrveiss Sep 18, 2026
76bbf2a
chore(vehicle): merge #16783 ca95302a2b1ef01b68e88a53eff702475d7e24a0…
mrveiss Sep 18, 2026
2d8efc1
chore(vehicle): merge #16788 cfb0d4932a33a0969fd2d219e1910c1f8dc37171…
mrveiss Sep 18, 2026
b4c5702
chore(vehicle): merge #16790 c5f51c0522dfcec86a7804d6f3d33e95408b0998…
mrveiss Sep 18, 2026
9884eca
style(vehicle): lint-only fixes for code-quality on docparser vehicle…
mrveiss Sep 18, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .session/HANDOFF-issue-16784-16785-document-format-gaps.md
Original file line number Diff line number Diff line change
@@ -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.
35 changes: 17 additions & 18 deletions autobot-backend/api/knowledge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1088,9 +1088,11 @@ 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 = os.path.splitext(filename.lower())[1]
ext = verified_upload_extension(filename, file_content) # #16773: the bytes outrank the name

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
Expand Down Expand Up @@ -1251,10 +1253,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()
Expand All @@ -1264,9 +1265,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(
Expand Down Expand Up @@ -2325,10 +2325,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)

Expand Down
77 changes: 77 additions & 0 deletions autobot-backend/api/knowledge_office_upload.py
Original file line number Diff line number Diff line change
@@ -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
113 changes: 113 additions & 0 deletions autobot-backend/api/knowledge_office_upload_16775_test.py
Original file line number Diff line number Diff line change
@@ -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
5 changes: 3 additions & 2 deletions autobot-backend/knowledge/documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 10 additions & 2 deletions autobot-backend/media/document/extraction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading