Skip to content

fix(pdf): keep PDF URI links in ScrapingResult.links (#2374) - #2375

Merged
ntohidi merged 1 commit into
unclecode:developfrom
Hugo-DDT:fix/2374-pdf-links
Oct 20, 2026
Merged

ntohidi merged 1 commit into
unclecode:developfrom
Hugo-DDT:fix/2374-pdf-links

Conversation

@Hugo-DDT

Copy link
Copy Markdown

Summary

Fixes #2374

PDFContentScrapingStrategy.scrap() collected the PDF's URI annotations as links = {"urls": [{"url": ..., "page": ...}]} and passed that to ScrapingResult. ScrapingResult.links is a Links model with only internal / external lists of Link items (keyed by href), so pydantic dropped the unknown urls key. The processor found the links, the result said success=True, and links came back empty.

The strategy now builds Links itself:

  • each target is classified against the PDF URL's domain with the same get_base_domain / is_external_url helpers the HTML scraper uses. A local PDF has no domain, so all of its absolute links are external; mailto: etc. stay external as in HTML.
  • relative targets in a remote PDF are resolved against the PDF URL.
  • a target that appears on several pages is listed once (first occurrence), like HTML links.

Link has no page field, so the page number is not kept on the link. I left the shared Link model alone so HTML results don't change shape; the page is still visible in cleaned_html (data-page divs). I'm happy to add a page field in a follow-up if you'd like one.

Before / after with the issue's repro (one-page PDF, one URI annotation):

before: scraping links: {'internal': [], 'external': []}
after:  scraping links: {'internal': [], 'external': [{'href': 'https://example.org/reference', ..., 'base_domain': 'example.org', ...}]}

List of files changed and why

  • crawl4ai/processors/pdf/__init__.py: add _build_links() and use it in scrap() in place of the {"urls": [...]} dict.
  • tests/unit/test_pdf_scraping_links_2374.py: regression tests on generated PDFs (pypdf only, no network): a local PDF's link survives as external; a remote PDF URL splits same-domain vs other-domain / mailto: links and dedupes a target repeated on two pages; the links survive model_dump() (what arun puts on CrawlResult.links).
  • docs/md_v2/advanced/pdf-parsing.md: the docs described links["urls"]; describe links.internal / links.external instead.

How Has This Been Tested?

  • pytest tests/unit/test_pdf_scraping_links_2374.py: 3 failed on develop (2efcccc), 3 passed with the fix.
  • pytest tests/unit/test_pdf_download_limits.py tests/unit/test_pdf_html_escaping.py tests/unit/test_pdf_page_number_race.py tests/unit/test_crawl_result_default_shape.py: pass.
  • pytest tests/unit: 202 passed, 6 skipped; the 6 failures in test_browser_tls_sandbox_flags.py fail the same way on develop without this change (no browser in my environment).
  • ruff check --select F clean on the changed files; new test file formatted with black.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added/updated unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

PDFContentScrapingStrategy.scrap() built links as {"urls": [{"url", "page"}]},
but ScrapingResult.links is a Links model with only internal/external lists
of Link items keyed by href, so pydantic dropped the unknown key and every
URI annotation the PDF processor extracted was lost while success=True.

Build Links directly: classify each target against the PDF URL's domain the
way the HTML scraper does (a local PDF has no domain, so its absolute links
are external), resolve relative targets against a remote PDF URL, and report
each target once. Update the PDF parsing docs to describe the shape.
Copilot AI balanced review requested due to automatic review settings October 15, 2026 12:04

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ntohidi ntohidi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @Hugo-DDT, this looks good. Putting the links into Links with the same domain helpers the HTML scraper uses is the right fix, and keeping Link unchanged means HTML results stay the same. Tests pass on my side too. Dropping the page number is fine for now. Approving.

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.

3 participants