Skip to content

Make sure text splitter is used even for PDF's - #520

Merged
ppinchuk merged 5 commits into
mainfrom
fix/pdf-raw-page-splitting
Aug 5, 2026
Merged

Make sure text splitter is used even for PDF's#520
ppinchuk merged 5 commits into
mainfrom
fix/pdf-raw-page-splitting

Conversation

@ppinchuk

@ppinchuk ppinchuk commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Fixing a bug that sometimes let a giant page through if it was a PDF document that was parsed via MD

@ppinchuk ppinchuk self-assigned this Aug 5, 2026
@ppinchuk
ppinchuk requested a review from castelao as a code owner August 5, 2026 18:34
Copilot AI lite review requested due to automatic review settings August 5, 2026 18:34
@ppinchuk ppinchuk added bugfix Fixed a known bug topic-python-llm Issues/pull requests related to LLMs p-high Priority: high labels Aug 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes an edge case in raw_pages_from_doc where PDF content represented as an MDDocument (with attrs["doc_type"] == "pdf") could bypass text splitting and allow an oversized “page” through to downstream processing.

Changes:

  • Update raw_pages_from_doc to treat only real PDFDocument instances as PDFs (avoiding doc_type-only detection for this path) and add a one-page PDF split failsafe.
  • Add unit tests covering oversized vs. within-budget behavior for both MDDocument and PDFDocument inputs marked as PDF.

Reviewed changes

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

File Description
compass/utilities/parsing.py Adjusts PDF/raw-page handling to ensure text splitting is applied in the problematic scenario.
tests/python/unit/utilities/test_utilities_parsing.py Adds regression tests ensuring oversized PDF-like raw pages are split and within-budget pages are preserved.

Comment thread compass/utilities/parsing.py Outdated
@codecov-commenter

codecov-commenter commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.85%. Comparing base (2ea7b6d) to head (ddb6fa0).

Files with missing lines Patch % Lines
compass/utilities/parsing.py 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
+ Coverage   63.60%   63.85%   +0.24%     
==========================================
  Files          78       78              
  Lines        7430     7437       +7     
  Branches      754      755       +1     
==========================================
+ Hits         4726     4749      +23     
+ Misses       2546     2528      -18     
- Partials      158      160       +2     
Flag Coverage Δ
unittests 63.85% <81.81%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ppinchuk
ppinchuk merged commit 77e7a53 into main Aug 5, 2026
22 checks passed
@ppinchuk
ppinchuk deleted the fix/pdf-raw-page-splitting branch August 5, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixed a known bug p-high Priority: high topic-python-llm Issues/pull requests related to LLMs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants