Repository navigation
refactor(document): consolidate five forked PDF extractors onto one canonical path #13893
Description
Activity
Decision needed — orphan wire-in scope.
PR #13924 removes all five forked implementations:
utils/document_parser.pyandutils/document_extractors.pynow delegate to the canonical extractor instead of carrying their own pypdf loops. But they still have zero callers, so the AC "every orphan is wired to a live caller" is not yet met.Wiring them in properly means letting the KB upload endpoint accept the formats
DocumentParseruniquely supports — XLSX, PPTX, ODT, ODS, ODP, ODG. It was built for exactly that (its docstring reads "Universal Document Parser for Knowledge Base"), butALLOWED_EXTENSIONSatapi/knowledge.py:124has never included them.Options:
- Widen
ALLOWED_EXTENSIONSto the fullDocumentParserset — genuine capability gain, wires the orphan for real. Cost: putsodfpy,openpyxlandpython-pptxon the untrusted-upload path. - Keep the current upload surface, leave the orphans delegating-but-uncalled, and track the wire-in separately.
Recommend (1), since an unwired module is the thing we are trying to eliminate and the parsers are already dependencies — but it is a product/security call with real blast radius, so flagging rather than deciding. Not blocking #13924, which is complete for the consolidation itself.
- Widen
- added 5 commits that reference this issue
on Aug 10, 2026 Landed on
Dev_new_guivia #13924.$ git show origin/Dev_new_gui:autobot-backend/media/document/extraction.py | grep -cE 'def extract_pdf|def extract_docx' 2The five forked PDF/DOCX extractors now route through one canonical path.
Worth recording for the next person in
api/knowledge.py: the PR also had to clear 7 pre-existing hardcoded-value violations in that file, none of them on a line it touched — the guard scanned changed files rather than changed lines. That scoping is fixed in #13950/#13962.PR #13924 merged to
Dev_new_gui— commit0b913d1f5, verified in the base:$ git log origin/Dev_new_gui --oneline --grep=13893 0b913d1f5 refactor(document): consolidate five forked PDF extractors onto one canonical path (#13893) (#13924) $ git cat-file -e origin/Dev_new_gui:autobot-backend/media/document/extraction.py && echo present present $ git show origin/Dev_new_gui:autobot-backend/requirements.txt | grep -c '^PyPDF2' 0Met:
- One canonical implementation; the other four delegate to it
-
PyPDF2no longer imported anywhere -
PyPDF2removed fromautobot-backend/requirements.txt - Format breadth preserved (ODT/ODS/ODP/ODG still parse)
- Drive/OneDrive regression-tested
- One page-marker convention chosen (
## Page N)
NOT met — this issue stays open:
- Directory-batch capability reachable
- Every orphan wired to a live caller
utils/document_parser.pyandutils/document_extractors.pyno longer carry forked implementations, but they still have zero callers. Closing now would be partial delivery. The blocker is the scope question posted above — whether to widenALLOWED_EXTENSIONS— which is a product/security call, not an implementation one.Wave 2 (#13884, #13894, #13895) is unblocked by the merge and does not wait on this decision.
Reopening not needed, but recording for the trail: this closed with two acceptance criteria unmet, and they remain unmet in base today.
$ git grep -n "document_parser|document_extractors" origin/Dev_new_gui -- '*.py' \ | grep -v 'utils/document_parser.py|utils/document_extractors.py' ...test_document_extraction_canonical_13893.py:46: "utils/document_parser.py", ...test_document_extraction_canonical_13893.py:47: "utils/document_extractors.py",Both hits are string literals in the anti-fork guard's path list — not imports. The two modules still have zero live callers.
The consolidation itself did ship and is correct, so the remaining work is tracked as #14333 rather than by reopening this. It is gated on the
ALLOWED_EXTENSIONSdecision raised here before merge.- added a commit that references this issue
on Aug 18, 2026 Retro-closure: the two unmet criteria are now satisfied
This closed with two acceptance criteria open — directory-batch capability reachable and every orphan wired to a live caller — which I recorded at the time and tracked as #14333.
#14333 is closed and merged. Verified in base by grepping for imports, not string occurrences (the mistake that made this issue look complete in the first place):
$ git grep -c "from utils.document_extractors import" origin/Dev_new_gui -- 'autobot-backend/knowledge/documents.py' 2 $ git grep -c "from utils.document_parser import" origin/Dev_new_gui -- 'autobot-backend/utils/document_extractors.py' 1Chain:
documents.py(live) →DocumentExtractor→DocumentParser. Both modules that this issue left orphaned are now reachable from a live, non-test path, and no file was deleted.Nothing further is owed on this issue. Recording it here rather than reopening, so the closure is honest in retrospect instead of resting on the merge evidence that was posted originally.
Worth keeping as a pattern note: this closed on "the PR merged", while two ACs were explicitly flagged as unmet in the PR body. Merge evidence answered a different question than the ACs asked.
Part of #13892 — Wave 1. No blockers.
Summary
Five independent PDF text extractors exist. Three are live and two are fully
orphaned (728 lines, zero callers, not even tests). Every fix for the document
path — empty-layer detection, OCR fallback, page provenance — has to ship five
times or, more likely, once and be forgotten in four places.
autobot-backend/api/knowledge.py:1065autobot-backend/media/document/pipeline.py:104media/manager.py:40autobot-backend/knowledge/connectors/content_extraction.py:39gdrive.py:40,onedrive.py:44## Page Nautobot-backend/utils/document_parser.py:124--- Page N ---autobot-backend/utils/document_extractors.py:126Security drift
The connector fork runs PyPDF2, which is unmaintained, while
requirements.txt:23pinspypdf>=6.15.0with the comment "SECURITY UPDATE(improved permission checks)". Google Drive and OneDrive ingestion runs on the
library we deliberately upgraded away from. Consolidation removes this as a
side effect.
Orphan verification
Approach
Per the no-deletion rule the orphans are wired in, not removed — both carry
capability the live paths lack:
utils/document_parser.py— format breadth: DOC/DOCX/XLSX/PPT/PPTX/ODT/ODS/ODP/ODG(
:29-42), a dispatch table (:76), and thread-pool offload (:69).utils/document_extractors.py— directory-batch extraction and format discovery.Survivor should be
media/document/pipeline.py: it already returns a structuredProcessingResultwithconfidence,page_count,document_info, and — uniquely— an
unavailablevserrordistinction (:206-238) that the other four lack andthat the empty-layer work in #13884 needs as its reporting shape.
Acceptance criteria
PyPDF2no longer imported anywhere;grep -rn "PyPDF2" --include=*.pyis cleanPyPDF2removed fromautobot-backend/requirements.txt:28document_parser.pypreserved (ODT/ODS/ODP/ODG still parse)document_extractors.pypreserved and reachableBlocks
The rest of #13892 — every Wave 2 and Wave 3 item lands on this path.