Skip to content

Focus pdf-to-bibtex skill on paper metadata and fix jabkit convert stdout output - #16292

Merged
Siedlerchr merged 6 commits into
mainfrom
split-pdf-to-bibtex-skill
Jul 21, 2026
Merged

Focus pdf-to-bibtex skill on paper metadata and fix jabkit convert stdout output#16292
Siedlerchr merged 6 commits into
mainfrom
split-pdf-to-bibtex-skill

Conversation

@koppor

@koppor koppor commented Jul 20, 2026

Copy link
Copy Markdown
Member

Related issues and pull requests

Related to the skills introduced in #16162. The counterpart skill for extracting a paper's reference list will be part of #16186, which ships the required jabkit pdf extract-references command.

PR Description

🤖 The pdf-to-bibtex skill mixed two different user goals: producing a BibTeX entry describing a PDF paper (pdfMerged and friends) and extracting the entries a paper cites (pdfBibiliography). The latter is not a registered import format — jabkit convert --input-format pdfBibiliography fails with "Unknown import format" — so the skill now scopes itself to the paper's own metadata and no longer documents that format id. The separate doi-to-bibtex workflow step is dropped as well, because pdfMerged already resolves DOI/arXiv/ISBN identifiers found in the PDF and merges the fetched metadata into the entry (verified: a test PDF gained publisher, pages, series, and month from the Crossref lookup).

While verifying the documented commands, jabkit convert without --output turned out to print org.jabref.model.database.BibDatabase@17932d9b instead of BibTeX (regression from #15913). It now prints the library via ExportService.printDatabaseContextToStdOut, the same path all other subcommands use, with a JUnit test guarding it.

Analogies

Like honey, this change is the product of many small verifications distilled into something that keeps: every documented flag was run against the real binary. Like chocolate, it is better with less filler — the skill lost a redundant lookup step and a format that never worked. And like the moon, the skill now shows only one face: the side that describes the paper itself, while the reference-list side orbits in #16186.

jabref-contrib-policy:4.2:reviewed​:ok

Steps to test

  1. Build jabkit: ./gradlew :jabkit:installDist
  2. Run jabkit/build/install/jabkit/bin/jabkit -p convert --input <some>.bib --input-format bibtex (no --output). Before this PR it printed org.jabref.model.database.BibDatabase@...; now it prints the BibTeX entries.
  3. Run jabkit -p convert --input <paper>.pdf --input-format pdfMerged on a paper with a DOI and observe fields (publisher, pages, month, ...) merged in from the DOI lookup — matching the updated skill text.
  4. Run jabkit convert --input <paper>.pdf --input-format pdfBibiliography and observe "Unknown import format" — the reason the format id was removed from the skill.

AI usage

Claude Code (model claude-fable-5)

AI CHECKLIST.md walkthrough

1. Code self-review

Nullability and control flow

  • [/] No == null / != null checks — the only null check (outputFile == null) is pre-existing picocli option handling, unchanged in kind.
  • No Objects.requireNonNull(...) — nullability expressed via JSpecify annotations.
  • [/] New classes annotated with @NullMarked — no new classes.
  • [/] Optional consumed with ifPresent / ifPresentOrElse / map / orElseThrow — no Optional in the diff.
  • [/] StringUtil.isBlank(...) used instead of s == null || s.isBlank() — no string blank checks in the diff.

Exceptions

  • No catch (Exception e) — no catch blocks added.
  • No throw new RuntimeException(...) / IllegalStateException(...).
  • Logged exceptions are passed as the last logger argument — no logging added.

Style and idioms

  • [/] New BibEntry objects built with withers — no BibEntry construction in main code.
  • Modern Java used — test uses List.of().
  • [/] Regexes use a precompiled Pattern.compile(...) constant — no regexes.
  • [/] Background work uses org.jabref.logic.util.BackgroundTask — no background work.
  • No commented-out code, no trivial comments, no AI-disclosure comments in source.
  • [/] Markdown Javadoc (///) uses Markdown syntax — no Javadoc changed.

User-facing text

  • [/] All user-facing text localized — no new user-facing strings (stdout output is data, matching the other subcommands).
  • [/] Sentence case; no trailing !; labels do not end with : — no labels.
  • [/] Variance expressed with placeholders — no new messages.

Security

  • [/] User-controlled data HTML-escaped in text/html responses — no HTML output.

Tests

  • Behavior changes have added or updated tests — ConvertTest.noOutputPrintsBibtexToStdout covers the stdout fix.
  • Tests assert object contents, use plain JUnit asserts, no @DisplayName, do not catch exceptions, use @TempDir.

2. Verification commands

  • ./gradlew :jabkit:test (module of the change) — passes; :jablib:check not run, no jablib change.
  • ./gradlew :jabkit:checkstyleMain :jabkit:checkstyleTest — passes (no jmh sources in jabkit).
  • ./gradlew :jabkit:modernizer — passes.
  • ./gradlew rewriteRun run before committing; a subsequent run reports no changes.
  • [/] ./gradlew javadoc — no Javadoc changed; module javadoc unaffected by the 4-line diff.
  • npx markdownlint-cli2 on changed Markdown — CHANGELOG.md clean; remaining MD060 table-style hits in skills/** are pre-existing on main and outside the CI lint scope (docs/**/*.md *.md).
  • [/] IntelliJ format container — formatting matches surrounding code after rewriteRun.

3. Documentation

  • CHANGELOG.md entry added for the user-visible jabkit convert fix, linking this PR.
  • Searched jabref and jabref-koppor issues for the stdout regression and the skill split — no confident match, PR link used.
  • [/] Requirement in docs/requirements/ — bug fix and doc scoping, no new feature.
  • [/] Developer documentation under docs/ — no architecture change.

4. Pull request

  • PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
  • All checklist items kept and marked.
  • All HTML comments removed from the PR body.
  • PR created with gh pr create --body-file.
  • CHANGELOG PR link verified against the actual PR number after creation.

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • I manually tested my changes in running JabRef (jabkit CLI built via :jabkit:installDist; before/after stdout output and the pdfMerged DOI enrichment verified by hand)
  • I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • [/] I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository — jabkit skills live in this repository; docs.jabref.org does not document jabkit convert stdout behavior

🤖 Generated with Claude Code

…dout output

The pdf-to-bibtex skill mixed two different tasks: getting a BibTeX entry
describing a PDF paper (pdfMerged and friends) and extracting the entries a
paper cites (pdfBibiliography). The latter is not a registered import format
("Unknown import format: pdfBibiliography"), so its table row is removed and
the skill is scoped to the paper's own metadata.

The separate doi-to-bibtex workflow step is dropped: pdfMerged already looks
up DOI/arXiv/ISBN identifiers found in the PDF and merges the fetched
metadata into the entry.

While verifying the documented commands, jabkit convert without --output
turned out to print the BibDatabase object reference instead of BibTeX;
it now uses ExportService.printDatabaseContextToStdOut like the other
subcommands.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEJvTaG41iVdG5o1hEyBfS
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Scope pdf-to-bibtex skill to paper metadata; fix jabkit convert stdout export

🐞 Bug fix 📝 Documentation 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Fix jabkit convert to export BibTeX to stdout when --output is omitted
• Add a regression test guarding stdout export behavior for convert
• Refocus pdf-to-bibtex skill docs on paper metadata (not reference-list extraction)
Diagram

graph TD
  U["CLI user"] --> C["jabkit convert"] --> I["ImportService (parse)"] --> X["ExportService (export)"]
  X --> O["Stdout (no --output)"]
  X --> F[("Output file")]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Override/adjust `BibDatabase.toString()` for nicer stdout output
  • ➕ Would make accidental System.out.println(database) look less broken in multiple places
  • ➖ Conflates debugging representation with a stable export format
  • ➖ Hard to honor export preferences/format (BibTeX vs other outputs)
  • ➖ Still wouldn’t align with how other jabkit subcommands print DatabaseContext
2. Implement a dedicated stdout export path inside `Convert` (bypassing `ExportService`)
  • ➕ Keeps Convert self-contained
  • ➕ Could tailor exactly what gets printed for convert
  • ➖ Duplicates export logic and risks drifting from other commands
  • ➖ Must manually respect export preferences (e.g., metadata, ordering) already handled by ExportService
3. Keep documenting `pdfBibiliography` and register it as a real import format
  • ➕ Would support the “extract cited references” workflow under convert
  • ➖ Bigger functional change than documentation fix; likely needs importer registration + implementation alignment
  • ➖ Already planned as separate work; increases review surface and risk in this PR

Recommendation: Current approach is the best tradeoff: reuse ExportService.printDatabaseContextToStdOut(...) so stdout output matches established export behavior and preferences, and keep the pdf-to-bibtex skill documentation aligned with currently supported import formats. The alternatives either duplicate export logic, rely on fragile stringification, or expand scope into reference extraction work that belongs in a dedicated change set.

Files changed (5) +35 / -15

Bug fix (1) +4 / -3
Convert.javaPrint converted library to stdout via 'ExportService' when no '--output' +4/-3

Print converted library to stdout via 'ExportService' when no '--output'

• Creates a single 'ExportService' instance and uses 'printDatabaseContextToStdOut(...)' when '--output' is omitted, instead of printing the raw 'BibDatabase' object. Reuses the same service instance for file export to avoid inconsistent output paths.

jabkit/src/main/java/org/jabref/toolkit/commands/Convert.java

Tests (1) +23 / -0
ConvertTest.javaAdd regression test asserting BibTeX is printed to stdout without '--output' +23/-0

Add regression test asserting BibTeX is printed to stdout without '--output'

• Introduces 'noOutputPrintsBibtexToStdout', configuring export preferences and asserting stdout contains a BibTeX entry header after running 'convert' without '--output'. Adds required exporter/save-order imports for the setup.

jabkit/src/test/java/org/jabref/toolkit/commands/ConvertTest.java

Documentation (3) +8 / -12
CHANGELOG.mdDocument fix for 'jabkit convert' stdout export regression +1/-0

Document fix for 'jabkit convert' stdout export regression

• Adds a changelog entry noting that running 'jabkit convert' without '--output' now prints the converted library instead of a Java object reference.

CHANGELOG.md

SKILL.mdUpdate jabkit skill summary to reflect PDF→BibTeX focus +1/-1

Update jabkit skill summary to reflect PDF→BibTeX focus

• Adjusts the jabkit skill front-matter description to describe converting PDF papers into BibTeX entries (rather than extracting references from PDFs).

skills/users/jabkit/SKILL.md

SKILL.mdRefocus pdf-to-bibtex skill on paper metadata; remove unsupported format/docs +6/-11

Refocus pdf-to-bibtex skill on paper metadata; remove unsupported format/docs

• Clarifies that the skill produces a BibTeX entry describing the PDF itself and is not for extracting a paper’s cited reference list. Removes the undocumented/unsupported 'pdfBibiliography' format id and drops the redundant DOI-to-BibTeX workflow step by documenting 'pdfMerged' enrichment behavior.

skills/users/pdf-to-bibtex/SKILL.md

koppor added a commit that referenced this pull request Jul 20, 2026
Splits the reference-list use case out of the pdf-to-bibtex skill (see
PR #16292, which scopes that skill to the paper's own metadata): the new
skill documents jabkit pdf extract-references, and the jabkit skill's
command table and examples now list the subcommand.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEJvTaG41iVdG5o1hEyBfS
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Stdout ignores output-format ✓ Resolved 🐞 Bug ≡ Correctness
Description
When --output is omitted, convert always prints via printDatabaseContextToStdOut, which
hardcodes BibTeX output and bypasses --output-format validation, so --output-format=html (or
even an invalid format) still emits BibTeX while the status line claims the requested format.
Code

jabkit/src/main/java/org/jabref/toolkit/commands/Convert.java[R57-61]

+        ExportService exportService = ExportService.create(jabKit.cliPreferences, sharedOptions.porcelain);
+
  if (outputFile == null) {
-            System.out.println(parserResult.getDatabase());
+            exportService.printDatabaseContextToStdOut(parserResult.getDatabaseContext());
      return 0;
Evidence
Convert uses outputFormat for its status message and file-export path, but the no---output
path always calls a BibTeX-only stdout printer. That printer uses BibDatabaseWriter directly and
never consults outputFormat/exporter selection, so stdout output cannot match non-BibTeX
--output-format requests and invalid formats won’t be rejected.

jabkit/src/main/java/org/jabref/toolkit/commands/Convert.java[40-65]
jabkit/src/main/java/org/jabref/toolkit/service/ExportService.java[83-97]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`jabkit convert` now prints to stdout via `ExportService.printDatabaseContextToStdOut(...)` when `--output` is omitted. That path ignores `--output-format` (and does not validate it), so the CLI can claim “Converting … to html” while still emitting BibTeX.
### Issue Context
- `Convert` defines `--output-format` and uses it for file export, but not for stdout.
- `ExportService.printDatabaseContextToStdOut` is BibTeX-specific (uses `BibDatabaseWriter`) and does not accept a format.
### Fix Focus Areas
- jabkit/src/main/java/org/jabref/toolkit/commands/Convert.java[40-65]
- jabkit/src/main/java/org/jabref/toolkit/service/ExportService.java[83-97]
### Suggested fix approach
Pick one of these and add a regression test:
1) **Restrict stdout to BibTeX**: if `outputFile == null` and `outputFormat` is not `bibtex`, fail with a usage error (and also fail for unknown formats). Also consider adjusting the non-porcelain status line to reflect the actual emitted format.
2) **Honor stdout formats**: implement an `ExportService.exportParserResultToStdOut(ParserResult, String format)` that exports using the selected exporter and writes the result to stdout (e.g., via a temp file + streaming to stdout), ensuring no extra “Exporting …” chatter pollutes stdout in non-porcelain mode.
Add a test that runs `convert` without `--output` but with a non-bibtex `--output-format` and asserts either correct format output or a deterministic usage error.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread jabkit/src/main/java/org/jabref/toolkit/commands/Convert.java Outdated
Siedlerchr and others added 5 commits July 20, 2026 20:33
Adds a regression test asserting exit code 2 and the "Unknown export
format" message when jabkit convert without --output is given an unknown
--output-format, and extends the CHANGELOG entry to cover the
--output-format handling on stdout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEJvTaG41iVdG5o1hEyBfS
@Siedlerchr
Siedlerchr added this pull request to the merge queue Jul 21, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Jul 21, 2026
Merged via the queue into main with commit afae906 Jul 21, 2026
60 checks passed
@Siedlerchr
Siedlerchr deleted the split-pdf-to-bibtex-skill branch July 21, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants