fix(LICENSE-01): make pymupdf/pymupdf4llm optional, ship non-AGPL default path#2083
Open
dimitri-tombroff wants to merge 6 commits into
Open
fix(LICENSE-01): make pymupdf/pymupdf4llm optional, ship non-AGPL default path#2083dimitri-tombroff wants to merge 6 commits into
dimitri-tombroff wants to merge 6 commits into
Conversation
…ault path pymupdf/pymupdf4llm (AGPL-3.0/Artifex dual-licensed) were mandatory dependencies of knowledge-flow-backend, imported unconditionally at module load in three sites regardless of config, so every default build/image shipped AGPL code. Move pymupdf/pymupdf4llm to an optional `pymupdf` extra. Default paths now use pypdf/markitdown (lite processors) and pypdfium2 (PPTX slide rendering, already docling's own PDF backend) instead of fitz. pdf_markdown_processor's extractor fallback and schema default flip from pymupdf to docling; the pymupdf path is now a lazy import with a clear actionable error if selected without the extra installed. Verified: default `uv sync --extra dev` installs no fitz/pymupdf/ pymupdf4llm; full test suite (597 tests) and code-quality pass with pymupdf absent; functional smoke test confirms extraction and PNG rendering both work without it installed. Closes #1950.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR remediates the AGPL packaging risk in knowledge-flow-backend by making pymupdf/pymupdf4llm truly optional and switching default PDF/PPTX ingestion paths to non-AGPL engines, with corresponding schema + documentation updates to reflect the new defaults and opt-in behavior.
Changes:
- Moved
pymupdf+pymupdf4llminto an explicit optional extra ([project.optional-dependencies].pymupdf), and updateduv.lockaccordingly. - Updated PDF/PPTX processors to avoid unconditional
fitz/pymupdfimports; default extraction now favorsdocling/pypdf/markitdown, and PPTX slide rendering usespypdfium2. - Regenerated config/Helm JSON schemas and updated licensing documentation to state the default distribution ships with no AGPL code.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates licensing note to reflect pymupdf is opt-in and default distribution contains no AGPL code. |
| docs/swift/data/id-legend.yaml | Marks LICENSE-01 as done and records the implemented remediation details. |
| docs/swift/COPYLEFT-DEPENDENCIES.md | Updates LICENSE-01 status/details to “resolved” and documents the new default/opt-in behavior. |
| deploy/charts/fred/values.schema.json | Regenerated Helm values schema: extractor default flipped to docling + licensing note added. |
| apps/knowledge-flow-backend/config/schema/configuration.schema.json | Regenerated config schema: extractor default flipped to docling + licensing note added. |
| apps/knowledge-flow-backend/uv.lock | Removes pymupdf* from default deps and adds pymupdf extra; adds pypdfium2. |
| apps/knowledge-flow-backend/pyproject.toml | Makes pymupdf* optional via pymupdf extra; adds pypdfium2 as a direct dependency. |
| apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/pptx_markdown_processor/utils/pptx_slide_renderer.py | Replaces fitz rendering with pypdfium2 rendering for slide PNG generation. |
| apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/pdf_markdown_processor/pymupdf_processor.py | Adds “optional/AGPL” context and pyright ignore for optional import. |
| apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/pdf_markdown_processor/pdf_markdown_processor.py | Lazily imports PyMuPdfExtractor only when configured; default/fallback now docling. |
| apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/lightweight_markdown_processor/lite2_pdf_to_md_processor.py | Removes unconditional fitz/pymupdf4llm imports; makes pymupdf4llm opportunistic and default path markitdown/pypdf. |
| apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/lightweight_markdown_processor/lite_pdf_to_md_processor.py | Replaces page-wise extraction from fitz to pypdf for the deprecated lite path. |
| apps/knowledge-flow-backend/knowledge_flow_backend/common/structures.py | Flips PdfPipelineConfig.extractor default to docling and updates description to reflect opt-in pymupdf. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1a7fdddf6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- pptx_slide_renderer.py: explicitly close() the PdfBitmap after saving, not just the page — relying on GC for native pypdfium2 memory caused high transient memory usage when rendering many slides (Copilot). - lite2_pdf_to_md_processor.py: replace the assert-based None guard on pymupdf4llm with an explicit RuntimeError — assertions are stripped under python -O, which would turn this into an unchecked None-deref (Copilot). - pdf_markdown_processor.py: introduce ExtractorConfigurationError and re-raise it (instead of catching it) in convert_file_to_markdown. The actionable error _build_extractor raises when `extractor: pymupdf` is configured without the extra installed was being caught by the generic extraction-failure handler and silently answered with a markitdown fallback — defeating the point of failing loudly on misconfiguration (Codex). Added regression tests for both the raise and the propagation.
…l-pymupdf' into worktree-1950-license-01-optional-pymupdf
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/lightweight_markdown_processor/lite_pdf_to_md_processor.py:273
PdfReader(str(file_path))can leavereader.streamopen until GC runs. Use awith file_path.open('rb')context manager so the file descriptor is closed deterministically (especially important in long-running ingestion workers).
reader = pypdf.PdfReader(str(file_path))
if len(reader.pages) <= 0:
logger.warning("LitePdfMarkdownProcessor: PDF %s has no pages.", file_path)
return False
return True
apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/lightweight_markdown_processor/lite_pdf_to_md_processor.py:288
PdfReader(str(file_path))can keep an open file handle onreader.streamuntil GC runs. Prefer awith file_path.open('rb')context manager so the file is closed deterministically after reading metadata.
reader = pypdf.PdfReader(str(file_path))
info = reader.metadata or {}
return {
"title": info.get("/Title") or None,
"author": info.get("/Author") or None,
apps/knowledge-flow-backend/knowledge_flow_backend/core/processors/input/lightweight_markdown_processor/lite2_pdf_to_md_processor.py:230
PdfReader(str(file_path))can keep an open file handle onreader.streamuntil GC runs. Prefer awith file_path.open('rb')context manager so the file is always closed after metadata extraction.
reader = pypdf.PdfReader(str(file_path))
info = reader.metadata or {}
return {
"title": info.get("/Title") or None,
"author": info.get("/Author") or None,
…ymupdf - lite_pdf_to_md_processor.py / lite2_pdf_to_md_processor.py: pypdf.PdfReader(str(path)) keeps its internal file handle open until GC runs — accumulates file descriptors in a long-running worker. Open via `with open(path, "rb") as f` and pass the handle instead, matching the pattern pdf_markdown_processor.py already used (Copilot, 5 sites across both files). - test_pdf_markdown_processor.py: the ExtractorConfigurationError regression test assumed pymupdf4llm is never installed. Branch on importlib.util.find_spec so the test asserts the actionable error when the extra is absent (CI default) and asserts a working PyMuPdfExtractor when a developer has opted in locally (Copilot).
| | ID | Package(s) | License | Where | Severity | Status | | ||
| | --- | --- | --- | --- | --- | --- | | ||
| | `LICENSE-01` | `pymupdf`, `pymupdf4llm` | AGPL-3.0 (dual, Artifex commercial alt.) | `knowledge-flow-backend` PDF pipeline (primary extractor in 3 of 4 usage sites) | **High** | Decided: make optional, non-AGPL default path. Tracked in issue #1950, milestone `swift-golive`. Further plugin separation deferred to `INGEST-01` | | ||
| | `LICENSE-01` | `pymupdf`, `pymupdf4llm` | AGPL-3.0 (dual, Artifex commercial alt.) | `knowledge-flow-backend` PDF pipeline — now an opt-in `pymupdf` extra, not installed by default | **Resolved** | Fixed 2026-07-23 (issue #1950): optional dependency, non-AGPL default path (docling/pypdf/pypdfium2). Default build/image contains zero AGPL code, verified. Further plugin separation deferred to `INGEST-01` | |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
docs/swift/COPYLEFT-DEPENDENCIES.md:45
- In the summary table, the “Severity” column should represent risk level (e.g. High/Low), but LICENSE-01 now shows Resolved, which is a status. This makes the table inconsistent with the other rows and the “How to read this document” section.
| ID | Package(s) | License | Where | Severity | Status |
| --- | --- | --- | --- | --- | --- |
| `LICENSE-01` | `pymupdf`, `pymupdf4llm` | AGPL-3.0 (dual, Artifex commercial alt.) | `knowledge-flow-backend` PDF pipeline — now an opt-in `pymupdf` extra, not installed by default | **Resolved** | Fixed 2026-07-23 (issue #1950): optional dependency, non-AGPL default path (docling/pypdf/pypdfium2). Default build/image contains zero AGPL code, verified. Further plugin separation deferred to `INGEST-01` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pymupdf/pymupdf4llm(AGPL-3.0/Artifex dual-licensed) were mandatory dependencies ofknowledge-flow-backend, imported unconditionally at module load in three sites regardless of config — every default build/image shipped AGPL code.pymupdfextra. Default paths now usepypdf/markitdown(lite PDF processors) andpypdfium2(PPTX slide rendering — already docling's own PDF backend) instead offitz.pdf_markdown_processor.py's extractor fallback and thePdfPipelineConfig.extractorschema default both flip frompymupdftodocling; the pymupdf path is now a lazy import that raises a clear, actionable error if selected without the extra installed.Test plan
make code-quality(ruff, bandit, basedpyright) passesmake test— 597 tests passuv sync --extra dev(the default dev/CI/Docker install) pulls in nofitz/pymupdf/pymupdf4llmmarkitdownand PDF-page-to-PNG rendering viapypdfium2both work correctly withpymupdfuninstalledCloses #1950.