Skip to content

fix(detect): warn when pypdf extra is missing in extract_pdf_text (#3… - #3710

Closed
shobhitagnihotri69 wants to merge 1 commit into
Graphify-Labs:v8from
shobhitagnihotri69:fix/3702-pdf-extra-warning
Closed

shobhitagnihotri69 wants to merge 1 commit into
Graphify-Labs:v8from
shobhitagnihotri69:fix/3702-pdf-extra-warning

Conversation

@shobhitagnihotri69

Copy link
Copy Markdown
Contributor

Summary

Fixes #3702.

extract_pdf_text previously swallowed all exceptions silently with except Exception: return "". When pypdf is not installed, PDF files yielded empty text without any notice, misleading users into believing the parser or LLM dropped the documents.

Changes

  • In graphify/detect.py: Explicitly caught ImportError on missing pypdf and printed a diagnostic warning to sys.stderr pointing to uv tool install 'graphifyy[pdf]' (or pip install graphifyy[pdf]).
  • Preserved the broad fallback exception handler for corrupt/unsupported PDF payloads.
  • Added regression test tests/test_pdf_extra_warning.py verifying that missing pypdf emits the warning to stderr while returning empty string.

Testing

  • tests/test_pdf_extra_warning.py passed.
  • Existing token-count and PDF-slicing tests pass cleanly.

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 — impact
  • tests/test_atomic_writes.py — impact
  • tests/test_build.py — impact
  • tests/test_charmap_encoding.py — impact
  • tests/test_chunking.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_claude_cli_backend.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_detect.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_evidence_binding.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cli.py — impact
  • tests/test_file_slice.py — impact
  • tests/test_ignore_file_encoding.py — impact
  • tests/test_image_vision.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_incremental_mtime_collision.py — impact
  • tests/test_indirect_call_block_scoped_shadow.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_js_exported_scalar_bindings.py — impact
  • tests/test_languages.py — impact
  • tests/test_llm_backends.py — impact
  • tests/test_long_path_hashing.py — impact
  • tests/test_manifest_ingest.py — impact
  • tests/test_mcp_ingest.py — impact
  • tests/test_multilang.py — impact
  • tests/test_non_regular_files.py — impact
  • tests/test_office_incremental.py — impact
  • tests/test_office_limits.py — impact
  • tests/test_ollama.py — impact
  • tests/test_out_dir_evidence.py — impact
  • tests/test_oversized_document_slicing.py — impact
  • tests/test_package_json_subpath_imports.py — impact
  • tests/test_pdf_extra_warning.py — impact, changed-test
  • tests/test_pdf_slicing.py — impact
  • tests/test_pdf_token_estimate.py — impact
  • tests/test_phantom_external_import.py — impact
  • tests/test_pipeline.py — impact
  • tests/test_python_underscore_resolution.py — impact
  • tests/test_rationale.py — impact
  • tests/test_ruby_resolution.py — impact
  • tests/test_scala_self_type.py — impact
  • tests/test_stale_prune.py — impact
  • tests/test_swift_computed_properties.py — impact
  • tests/test_swift_protocol_requirements.py — impact
  • tests/test_trailing_newline_not_a_syntax_error.py — impact
  • tests/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).

safishamsi added a commit that referenced this pull request Sep 29, 2026
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]>
@safishamsi

Copy link
Copy Markdown
Member

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

@safishamsi safishamsi closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extract_pdf_text silently returns "" when the optional pdf extra is missing — every PDF becomes an empty document

2 participants