Repository navigation
fix(media): the document pipeline plain-texts verified office formats instead of parsing or refusing them #16784
Description
What happens
extract_document (autobot-backend/media/document/extraction.py) dispatches on detect_format and falls through to extract_plain_text for anything that is not pdf or docx:
detected = detect_format(raw, mime_type)
if detected == "pdf": return extract_pdf(raw)
if detected == "docx": return extract_docx(raw)
return extract_plain_text(raw)
So a spreadsheet, presentation or OpenDocument file reaching the media document pipeline is decoded as if its ZIP bytes were text. The result is binary noise presented as document text, which then flows onward — into the KB, and into prompts through retrieval.
This is pre-existing behaviour, not a regression: before #16773 those formats were detected as "text" and took the same branch. #16773 only makes it visible, because detect_format now returns xlsx/pptx/odt/ods/odp/odg for them.
Why it is worth fixing separately
utils/document_parser.py already owns working parsers for all six. The media pipeline does not reach them, so the same file parses correctly through one ingest route and produces noise through the other. Silent garbage is worse than a clear refusal: nothing downstream can tell the difference between a bad document and a document this path cannot read.
Options
- Delegate to
DocumentParserfor the six formats, asutils/document_extractors.pyalready does (extract_from_office) — one extraction path per format, nothing forked. - Raise
DocumentExtractionErrornaming the format, so the pipeline's existing_unavailable_resultreports it honestly instead of returning noise.
(1) is preferred: the parsers exist, and the pipeline's callers want the text.
Acceptance criteria
- A verified office/ODF document through the media document pipeline either returns parsed text or a named error — never plain-texted ZIP bytes.
- A test covers at least one such format end to end through
extract_document. - pdf, docx and genuine plain text are unchanged.
Found during
#16773 (content-based format verification), umbrella #16772. Not folded in, to keep that PR's scope clean.
Activity
chore(session): write end-of-session handoff for issue-16784-16785-do…
mrveiss commentedon Sep 16, 2026
Implementation note — read-only prep, no code yet
Claimed by the session that filed it. Still blocked on #16783 (which adds
parse_document_textandSUFFIX_BY_FORMAT). Recording the design now so it is not re-derived, and because one constraint changes the shape of option (1).Confirmed against current
mainextract_documentdispatchespdf→docx→extract_plain_textfor everything else, as the issue says.DocumentExtractionError, andDocumentDependencyErrorsubclasses it, so both error paths are already wired.asyncio.wait_for(asyncio.to_thread(...), timeout=deadline). A delegated office extraction inherits that bound, so unlike Add zip/archive recursive ingestion for KB document uploads #16774 this needs no timeout of its own.The constraint that shapes option (1)
utils/document_parser.pyimports frommedia.document.extraction. A module-level import in the other direction would be a genuine cycle. Nothing inmedia/importsutils/today, so this is a new edge either way.The workable form is a function-local import inside the new helper, deferred to call time when both modules are loaded. That is a deliberate deviation from the normal import placement and should be recorded as such, with the cycle named in a comment, rather than left looking like an oversight.
The alternative — moving the six parsers down into
media/document/and havingutils/document_parser.pydelegate there — is the cleaner layering and matches "consolidate, never fork", but it is a much larger change touching every caller ofDocumentParser. Worth doing eventually; not worth coupling to this fix.Proposed shape
Add to
media/document/extraction.py(currently 519 lines, not size-ratcheted, so there is headroom):extract_office(raw, detected)— writesrawto a temporary file carrying the verified suffix from the siblingzip_formatsmodule, callsparse_document_text, and returns anExtractedDocumentwithformat=detectedandtables_attempted=False. The last is the honest value: the parsers fold spreadsheet and table content into the text rather than populatingtables, and fix(document): _extract_pdf_tables stub returns [] while the result claims confidence 0.95 #13895 added that flag precisely so an emptytablescannot mean both "none present" and "never looked".ImportErrorbecomesDocumentDependencyError, everything else becomesDocumentExtractionError. The office parsers importopenpyxl,python-pptxandodfpylazily, so a missing library is a deployment gap and must not be reported as a bad document. The pipeline already keeps those two apart; this is what lets it.extract_documentgains one branch before the plain-text fallback.Acceptance criteria, mapped
extract_document— at least one real format, driven throughextract_documentitself rather than the helper, so the dispatch is what is under test.One more case worth covering that the criteria do not name: a file whose ZIP members verify as an office format but whose content is corrupt. It must produce the named error, not fall through to plain text — that fall-through is the actual defect being fixed, and a test that only covers healthy files would not catch its return.
mrveiss commentedon Sep 16, 2026
Correction to the note above: the dependency is #16775, not #16783
I wrote that this is blocked on #16783 because that PR adds
parse_document_textandSUFFIX_BY_FORMAT. Only the second half is true.SUFFIX_BY_FORMATandsniff_zip_formatcome from Verify ZIP-based office/ODF formats by content, not extension alone #16773 (PR feat(documents): verify ZIP-based office and ODF formats by content (#16773) #16783). ✅parse_document_textis added by KB upload allowlist omits already-supported office/ODF formats #16775 (PR feat(kb): accept spreadsheet, presentation and OpenDocument uploads (#16775) #16788) — it is absent from the Verify ZIP-based office/ODF formats by content, not extension alone #16773 branch and from currentmain, and present only on the KB upload allowlist omits already-supported office/ODF formats #16775 branch, whereapi/knowledge_office_upload.pyuses it.So the real dependency is #16775, which itself sits on top of #16773. Verified by checking the function's presence on each branch rather than by inference. I caught this when the branch I had cut from the #16783 base did not contain the symbol I had told everyone it would.
The rest of the design note stands unchanged — the import cycle, the
ImportError→DocumentDependencyErrormapping, and the inherited timeout are all unaffected by which branch supplies the helper.The implementation is written and sits on a branch stacked on #16775. It is not pushed: the self-hosted runner queue is backed up and pushing now would add workflow runs to a queue nothing is draining. It goes out when that clears.
mrveiss commentedon Sep 19, 2026
AC verification against merged main (post #17133 vehicle merge)
extraction.pygainedextract_office()(delegates toutils.document_parser.parse_document_text, Option 1 as preferred) andOFFICE_FORMATS(xlsx/pptx/odt/ods/odp/odg).extract_document's dispatch now routes these throughextract_officebefore the plain-text fallback.extract_officeraisesDocumentDependencyErrorif the parser module is missing (checked first, so a deployment gap is never conflated with a bad document per the fix(document): _extract_pdf_tables stub returns [] while the result claims confidence 0.95 #13895 concern) orDocumentExtractionErrornaming the format on parse failure; the plain-text fallback is now unreachable for these 6 formats.office_extraction_16784_test.pycallsextract_document(OFFICE_FIXTURES[fmt])(parametrized across formats), plus explicit failure-path assertions.extract_pdf/extract_docx/extract_plain_textfunction bodies untouched;extract_document's pdf/docx branches are the same lines, with the new office branch inserted only before the final fallback. Same test file assertsextract_document(b"just text")and a docx fixture still work.All 3 satisfied. Closed correctly.