Fix wrong author and year when importing PDFs with creator-app metadata - #16247
Fix wrong author and year when importing PDFs with creator-app metadata#16247koppor wants to merge 11 commits into
Conversation
The document information dictionary of office-produced PDFs names the person who exported the file, not the authors of the work. The merge importer now verifies author candidates against the text of the leading pages, prefers text-confirmed candidates, and drops a single-person creator-style author that appears nowhere in the document. Also restrict content-importer year extraction to plausible values so postal codes and URL digit runs are no longer imported as year. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
the
status: changes-required
PR Summary by QodoFix PDF import: avoid exporter-account authors and implausible years
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Relocate the leading-page text extraction and author-validation helpers from PdfMergeMetadataImporter into PdfContentImporter, the class that already owns reading text from a PDF and reasoning about authors. The merge importer now only invokes them. Behavior is unchanged. Also use Markdown links in the new /// Javadoc and parameterize the uniform single-candidate cross-check cases in the offline test with @CsvSource. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Your pull request conflicts with the target branch. Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. |
Add JavaDoc to mergeCandidates and rename its misleading `documentText` parameter to `leadingPagesText` (extractLeadingPagesText only reads the first two pages). Mark PdfContentImporter @NullMarked and annotate the leading-pages-text parameter @nullable, consuming it with Objects.requireNonNullElse instead of an ad-hoc `== null` check in the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI's IntelliJ formatter cannot handle a wrapped @PARAM in /// Javadoc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI's IntelliJ formatter column-aligns consecutive @PARAM descriptions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the real paper/creator names with obvious placeholders (Alice Doe, Bob Smith, Eve Void, ...), keeping every property each case tests (diacritics, hyphenation, substring-not-word, exotic XMP separators, multi-person). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The requested changes were not addressed for 3 days. Please follow-up in the next 7 days or your PR will be automatically closed. You can check the contributing guidelines for hints on the pull request process. |
github-actions
Bot
added
the
status: stale
2 participants
Related issues and pull requests
No existing issue found — searched JabRef/jabref and jabref-koppor for reports about wrong authors/years from PDF document properties.
🤖 PR Description
Importing a PDF exported from an office suite took the author from the PDF document properties, which usually contain the account name of whoever exported the file — not the authors of the work (real-world example: a SummerSoc 2018 paper by Krieger, Breitenbücher, Képes, and Leymann imported as author "Markus Richter", year 7056).
PdfMergeMetadataImporternow cross-checks author candidates against the text of the PDF's leading pages: an unconfirmed author is replaced by a text-confirmed candidate, or dropped when it appears nowhere in the document (guarded so JabRef-written XMP metadata survives re-import).PdfContentImporteradditionally only extracts plausible years, so digit runs like postal codes or URL fragments are no longer imported asyear.Analogies
Like honey, this PR combines the output of many workers (importer candidates) and filters out what does not belong in the comb; like chocolate, it removes a bitter surprise hidden under a trustworthy-looking wrapper (metadata that is confidently wrong); and like the moon, the imported entry now only reflects light that actually comes from the document itself.
jabref-contrib-policy:4.2:reviewed:ok
Steps to test
pdfinfo/document properties show it).author= the document-properties name (e.g. "Markus Richter") and possibly a nonsenseyear(e.g.7056from the postal code "70569 Stuttgart" on the first page).yearis only set to a plausible value.Result of importing the SummerSoc 2018 example PDF (docinfo author "Markus Richter"; entry ends up with no author instead, correct title, year 2018 instead of 7056):
AI usage
Claude Code (model claude-fable-5)
AI CHECKLIST.md walkthrough
1. Code self-review
Nullability and control flow
== null/!= nullchecks — JSpecify annotations (@NullMarked,@Nullable,@NonNull) used instead.Objects.requireNonNull(...)— nullability expressed via JSpecify annotations.@NullMarked(org.jspecify.annotations.NullMarked). — no new top-level production class; only a private nested record.Optionalconsumed withifPresent/ifPresentOrElse/map/orElseThrow— neverorElse(unusedValue)nor anisPresent()+get()block.StringUtil.isBlank(...)used instead ofs == null || s.isBlank(). — no null-or-blank checks needed;isBlank()only called on guaranteed non-null strings.Exceptions
catch (Exception e)— only specific exceptions are caught.throw new RuntimeException(...)/IllegalStateException(...)— these tear down the whole application.LOGGER.info("...", e)), not concatenated into the message string.Style and idioms
BibEntryobjects built with withers (withField, notsetField).List.of()/Map.of()/Set.of(),Path.of(),SequencedCollection/SequencedSet, text blocks.Pattern.compile(...)constant, notString.matches(...).org.jabref.logic.util.BackgroundTask, notnew Thread(). — no background work added.///) uses Markdown syntax, not JavaDoc inline tags:`code`instead of{@code},[ClassName]instead of{@link}.User-facing text
Localization.langin Java,%prefix in FXML). — no user-facing strings added.!; labels do not end with:."...: %0"), not string concatenation.Security
text/htmlresponse (XSS). — no HTTP/HTML output touched.Tests
org.jabref.model/org.jabref.logichave added or updated tests. — newPdfMergeMetadataImporterOfflineTest(11 tests) plus updated/added cases inPdfContentPartialImporterTest.assertEquals), use plain JUnit asserts (not AssertJ), have no@DisplayName, do not catch exceptions, and use@TempDirinstead of manual temp directories.2. Verification commands
./gradlew :jablib:check— passed (with xvfb)../gradlew checkstyleMain checkstyleTest checkstyleJmh— passed for:jablib(only module touched)../gradlew modernizer— passed for:jablib../gradlew --no-configuration-cache rewriteDryRunreports no changes../gradlew javadoc—:jablib:javadocpassed.npx markdownlint-cli2—CHANGELOG.md(the only Markdown changed): 0 errors.intellij-formatcontainer — Docker not usable on this machine; used IntelliJidea formatwith.idea/codeStyles/Project.xmlinstead (same code style).3. Documentation
CHANGELOG.mdentry added (end-user wording; PR link, since no issue exists).closesreference.docs/requirements/<area>.md— heuristic bug fix, not a new feature.docs/— no behavior/architecture change at the documented level.4. Pull request
.github/PULL_REQUEST_TEMPLATE.md, every section filled.[x],[ ], or[/].gh pr create --body-file <file>(not--body).CHANGELOG.mdTODOplaceholder was replaced with the real PR-number link after PR creation, then committed and pushed.Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)🤖 Generated with Claude Code