Skip to content

Harden Semantic Scholar PDF downloads - #65

Open
turgaybulut wants to merge 11 commits into
openags:mainfrom
turgaybulut:fix/harden-semantic-pdf-downloads
Open

Harden Semantic Scholar PDF downloads#65
turgaybulut wants to merge 11 commits into
openags:mainfrom
turgaybulut:fix/harden-semantic-pdf-downloads

Conversation

@turgaybulut

@turgaybulut turgaybulut commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Treat Semantic Scholar openAccessPdf.url as a candidate URL instead of assuming it is always a direct PDF.
  • Validate downloaded bytes/content type before saving or parsing as PDF.
  • Add PMCID-based Europe PMC/PMC PDF fallbacks and Europe PMC full-text XML fallback for reads.
  • Remove invalid cached PDF files before retrying downloads.
  • Add regression tests for publisher 403s, HTML responses saved as PDFs, PMC article URL fallback, and invalid cache replacement.

Root Cause

Semantic Scholar can return publisher, DOI, or PMC article URLs in openAccessPdf.url. Some of those URLs return HTML challenge/landing pages or HTTP 403 responses rather than PDF bytes. The previous Semantic connector wrote successful HTTP 200 responses directly to .pdf files and then handed them to PyPDF, which produced errors such as invalid pdf header: b"<!doc" and Stream has ended unexpectedly.

Impact

The Semantic connector now fails cleanly when a URL is not a PDF, avoids leaving bogus cached PDFs behind, and recovers common open-access cases by using PMCID metadata to fetch PDFs from Europe PMC.

Validation

  • Semantic connector tests pass.
  • The originally failing Semantic Scholar IDs now download real PDF files and read extracts text successfully.
  • Additional live checks covered direct PDF URLs, PMC article URL fallback, DOI landing URL with PMCID fallback, and DOI landing URL without PMCID.

@turgaybulut
turgaybulut marked this pull request as ready for review April 27, 2026 15:59
@universea
universea requested a review from Copilot April 27, 2026 23:29

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.

Pull request overview

This PR hardens the Semantic Scholar connector’s PDF download and read paths by treating openAccessPdf.url as a candidate (not guaranteed to be a direct PDF), validating responses before caching/parsing, and adding Europe PMC/PMC fallbacks using PMCID metadata.

Changes:

  • Add candidate URL selection and robust PDF validation (header/content-type checks), plus invalid-cache removal and retry logic.
  • Add PMCID-based fallbacks (Europe PMC rendered PDF, PMC PDF endpoint) and Europe PMC full-text XML fallback for read_paper.
  • Add regression tests covering 403/HTML responses, PMC URL handling, and invalid cached PDF replacement.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
paper_search_mcp/academic_platforms/semantic.py Implements candidate URL download flow with validation, PMCID fallbacks, cache cleanup, and XML full-text fallback for reads.
tests/test_semantic.py Adds mocked regression tests for forbidden/HTML download responses, PMC fallbacks, and invalid cache replacement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated
Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated
Comment thread tests/test_semantic.py Outdated

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py
Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated
Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated
Comment thread paper_search_mcp/academic_platforms/semantic.py

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py
Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated

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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated
Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated
Comment thread paper_search_mcp/academic_platforms/semantic.py

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.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py

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.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py Outdated

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.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_semantic.py

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.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread paper_search_mcp/academic_platforms/semantic.py
Comment thread paper_search_mcp/academic_platforms/semantic.py
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.

2 participants