Repository navigation
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/knowledge_base/parsers/pdf_parser.py" line_range="73" />
<code_context>
)
except Exception:
- # 单个图片提取失败不影响整体
+ # A broken image must not hide other images on the page.
continue
except Exception:
</code_context>
<issue_to_address>
**Image failures go unnoticed**
When pypdf or Pillow raises while retrieving or encoding an image, the per-image `except Exception` discards the failed image and returns the remaining media without recording the exception, so the upload succeeds with missing media and operators have no indication extraction failed.
Log the image/page context and exception before continuing so extraction remains isolated but failures are observable.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: astrbot/core/knowledge_base/parsers/pdf_parser.py:73
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PDFParser saved decoded pixel buffers as image files and guessed MIME types from a single PDF filter. Flate images were unreadable, filter-chained JPEGs were mislabeled, and nested images were omitted.
Use pypdf's public
page.imagesAPI to preserve encoded image bytes and derive the actual extension/MIME type. Isolate image and page failures so remaining images and text still load. Failed extraction now logs the file, page, image (when known), and exception details, addressing the review feedback.Validation
Latest update, Windows / Python 3.12:
python -m pytest tests/test_pdf_parser.py -q: 8 passed.git diff --check: passed.The original patch reproduced five failing cases before the fix and passed its seven-test suite with both pypdf 6.21.2 and the minimum supported 6.1.1. The added logging test was run with the current environment.
No new dependencies or API changes. Existing stored media are not rewritten; affected documents must be parsed again. The full repository suite was not run. Upstream Actions require maintainer approval; local results are not a claim that upstream CI passed.