fix(detect): warn when pypdf extra is missing in extract_pdf_text (#3… - #3710
shobhitagnihotri69 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2377 functions depend on the 124 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 655 callers, 45 callees - new:
_rebuild_code()— 142 callers, 54 callees - new:
detect()— 112 callers, 15 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
save_manifest()— 40 callers, 11 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_files_direct()— 17 callers, 20 callees - new:
extract_xaml()— 19 callers, 17 callees - …and 45 more — each is listed as a finding
Verification — 2377 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 804 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
55 of 291 test file(s) selected (19%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_atomic_writes.py— impacttests/test_build.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_cjs_module_extension.py— impacttests/test_claude_cli_backend.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_evidence_binding.py— impacttests/test_extract.py— impacttests/test_extract_cli.py— impacttests/test_file_slice.py— impacttests/test_ignore_file_encoding.py— impacttests/test_image_vision.py— impacttests/test_import_extension_resolution.py— impacttests/test_incremental_mtime_collision.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_languages.py— impacttests/test_llm_backends.py— impacttests/test_long_path_hashing.py— impacttests/test_manifest_ingest.py— impacttests/test_mcp_ingest.py— impacttests/test_multilang.py— impacttests/test_non_regular_files.py— impacttests/test_office_incremental.py— impacttests/test_office_limits.py— impacttests/test_ollama.py— impacttests/test_out_dir_evidence.py— impacttests/test_oversized_document_slicing.py— impacttests/test_package_json_subpath_imports.py— impacttests/test_pdf_extra_warning.py— impact, changed-testtests/test_pdf_slicing.py— impacttests/test_pdf_token_estimate.py— impacttests/test_phantom_external_import.py— impacttests/test_pipeline.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_stale_prune.py— impacttests/test_swift_computed_properties.py— impacttests/test_swift_protocol_requirements.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impact- … and 5 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify extract\_pdf\_text.
The verifier did not have enough to check extract\_pdf\_text, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 53 more finding(s) on lines outside this diff (see the check run).
Atomic label sidecars (#3853), docx table/all-text extraction (#3833), wiki index-collision (#3818) and escaped wikilink aliases (#3772), community-listing real-node filter (#3836), warn-once on missing pypdf extra (#3710), plus the --help logo banner + app.graphify.com line. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Shipped in v0.9.72 (now on PyPI) via an authorship-preserving cherry-pick, so your commit keeps contributor-graph credit. Thanks @shobhitagnihotri69! Warn (once per run) when the pdf extra is missing, instead of silent empty text. Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.72 |
Summary
Fixes #3702.
extract_pdf_textpreviously swallowed all exceptions silently withexcept Exception: return "". Whenpypdfis not installed, PDF files yielded empty text without any notice, misleading users into believing the parser or LLM dropped the documents.Changes
graphify/detect.py: Explicitly caughtImportErroron missingpypdfand printed a diagnostic warning tosys.stderrpointing touv tool install 'graphifyy[pdf]'(orpip install graphifyy[pdf]).tests/test_pdf_extra_warning.pyverifying that missingpypdfemits the warning to stderr while returning empty string.Testing
tests/test_pdf_extra_warning.pypassed.