Allow roman numerals, forward slash and sequentes in page checker by eduardo-kurek · Pull Request #15497 · JabRef/jabref

eduardo-kurek · 2026-04-04T17:58:04Z

Related issues and pull requests

Closes #15457

PR Description

In this PR, i implemented the following features in page checker:

Add some test cases for invalid ranges, such as 10-, -10, 10--, --10, +10, 10+-10;
Fix potential bugs with empty numerals. Before, the numeral regex pattern was \\d*, which allows empty numbers, causing strings like "SS" to be considered as PREFIX and SUFFIX. Now the pattern is \\d+;
Add support to sequens and sequentes. I saw this in biblatex documentation, p. 292 sq;
Add support to "/" separator;
Add support to roman numerals.

Steps to test

Create a new library;
Change it to biblatex in Library > Library Properties > General > Library Mode
Reopen the library
Add an example entry;
Select the entry and go to the Optional Fields tab.
Insert the following text into pages: 1/10, 2f., 2sqq, iv, iv-X, XX-C, ivxlcdm
See that there is no warning

Checklist

I own the copyright of the code submitted and I license it under the MIT license
I manually tested my changes in running JabRef (always required)
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

qodo-free-for-open-source-projects · 2026-04-04T17:58:34Z

Review Summary by Qodo

Support Roman numerals and BibLaTeX formats in page checker

✨ Enhancement 🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Add support for Roman numerals in page ranges (i, iv, X, etc.)
• Add support for forward slash separator and Latin continuity suffixes (f., ff., sq., sqq.)
• Refactor page validation regex patterns for BibTeX and BibLaTeX formats
• Fix page prefix extraction to handle Roman numerals and mixed formats
• Add comprehensive test cases for new page formats and invalid ranges

Diagram

flowchart LR
  A["Page Checker Input"] --> B["Updated Regex Patterns"]
  B --> C["Roman Numerals Support"]
  B --> D["Forward Slash Separator"]
  B --> E["Sequens/Sequentes Support"]
  C --> F["BracketedPattern Methods"]
  D --> F
  E --> F
  F --> G["firstPage/lastPage/pagePrefix"]
  G --> H["Citation Key Generation"]

File Changes

1. jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java ✨ Enhancement +33/-17

Enhanced page validation patterns for multiple formats

• Refactored regex patterns to support Roman numerals, forward slash separator, and Latin continuity
 modifiers
• Added public constants for ROMAN_NUMBER, DECIMAL_NUMBER, and PAGE_NUMBER patterns
• Introduced SEQUENS_AND_SEQUENTES pattern for f., ff., sq., sqq. suffixes
• Updated BIBLATEX_SEPARATOR to include forward slash (/)
• Restructured page validation expressions for both BibTeX and BibLaTeX formats

jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java

2. jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java ✨ Enhancement +83/-22

Support Roman numerals in page extraction methods

• Added imports for Comparator and Map utilities
• Imported PagesChecker to use public regex patterns
• Refactored firstPage() method to use getPageNumberTokens() and support Roman numerals
• Refactored lastPage() method to handle Roman numeral comparison
• Added pagePrefix() method with enhanced logic for detecting decimal and Roman number prefixes
• Implemented romanToInteger() helper method for Roman numeral conversion
• Added pageTokenComparator() for comparing both decimal and Roman numerals

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java

3. jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java 🧪 Tests +17/-0

Add Roman numeral tests for page extraction
• Added romanentry test fixture with Roman numeral pages (SXX--ivS)
• Added test for firstPage() with Roman numerals
• Added test for lastPage() with Roman numerals
jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java

View more (4)

4. jablib/src/test/java/org/jabref/logic/citationkeypattern/CitationKeyGeneratorTest.java 🧪 Tests +6/-3

Update page prefix and lastpage test expectations
• Updated pagePrefix() test cases to reflect new behavior with Roman numerals
• Fixed expected results for edge cases (L--27, L)
• Added test cases for mixed prefix and Roman numeral formats (S5-K10, Siv-KXX)
• Added test case for lastPage() with mixed Roman and decimal numerals (ivS--SXXY)
jablib/src/test/java/org/jabref/logic/citationkeypattern/CitationKeyGeneratorTest.java

5. jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java 🧪 Tests +18/-2

Expand BibLaTeX page format test coverage

• Added test cases for prefix-only formats (S436-S439)
• Added test cases for prefix and suffix combinations (S436S-S439S)
• Added test cases for forward slash separator (A10S/90, 1/10)
• Added test cases for sequens and sequentes variations (1f, 1ff, 1 f., 1 sq., etc.)
• Added test cases for Roman numerals in various formats (i, ivxlcdm, i-vi, VII-xii)
• Added invalid range test cases (10-, -10, 10--, --10, +10, 10+-10)

jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java

6. jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBibtexTest.java 🧪 Tests +14/-2

Expand BibTeX page format test coverage

• Added test cases for prefix-only formats (S436--S439)
• Added test cases for prefix and suffix combinations (S436S--S439S)
• Added test cases for Roman numerals (i, ivxlcdm, i--vi, VII--xii)
• Added invalid range test cases (10-, -10, 10--, --10, +10, 10+-10)

jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBibtexTest.java

7. CHANGELOG.md 📝 Documentation +2/-0

Document page checker enhancements in changelog

• Added two entries documenting the fix for pages checker to support BibLaTeX-specific formats
• Mentions support for Roman numerals, forward slashes, and Latin continuity suffixes
• References issue #15457

CHANGELOG.md

qodo-free-for-open-source-projects · 2026-04-04T17:58:34Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (4) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

2. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

3. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

View more (15)

4. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

5. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

6. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

7. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

8. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

9. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

10. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

11. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

12. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

13. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

14. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

15. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

16. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

17. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

18. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

19. bibentryRomanExpansionFirsPageTest typo 📘 Rule violation ⚙ Maintainability

Description

A new test method name contains a spelling mistake (Firs instead of First), reducing readability
and discoverability. This violates the naming/code style requirement to avoid typos.

Code

jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[R438-442]

+    @Test
+    void bibentryRomanExpansionFirsPageTest() {
+        BracketedPattern pattern = new BracketedPattern("[year]_[auth]_[firstpage]");
+        assertEquals("2017_Kitsune_iv", pattern.expand(romanentry));
+    }

Evidence
PR Compliance ID 3 requires correctly spelled names following project conventions. The newly added
test method is named bibentryRomanExpansionFirsPageTest, which contains a typo.
AGENTS.md
jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[438-442]

Agent prompt

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

## Issue description
The test method name contains a typo (`Firs`), which should be corrected to `First`.
## Issue Context
Consistent, correctly spelled identifiers improve maintainability and searchability.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[438-442]

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

20. Roman regex matches words 🐞 Bug ≡ Correctness

Description

BracketedPattern pagePrefix/firstPage/lastPage use PagesChecker.ROMAN_NUMBER to find roman tokens
anywhere, so page strings containing ordinary words (e.g., "article 123") can yield incorrect
prefixes/tokens and thus wrong generated citation keys.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1057]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;

Evidence
PagesChecker.ROMAN_NUMBER is defined as [ivxlcdmIVXLCDM]+, which can match substrings inside
normal words. BracketedPattern’s tokenization
(Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages)) and pagePrefix
(Pattern.compile(PagesChecker.ROMAN_NUMBER)) both use these patterns without requiring page-token
boundaries, so a pages value like "article 123" (explicitly handled as a real-world input in
citation-style conversion) would match the "icl" substring inside "article" and can influence
pagePrefix/lastPage results. Citation keys are generated during save using
CitationKeyGenerator (extends BracketedPattern) without sanitizing pages values, and integrity
checking only emits messages (does not rewrite fields).
jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[14-21]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1057]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1098-1123]
jablib/src/main/java/org/jabref/logic/citationstyle/JabRefItemDataProvider.java[133-138]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[407-413]
jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[205-212]
jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[405-416]
jablib/src/main/java/org/jabref/logic/integrity/FieldChecker.java[12-27]

Agent prompt

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

## Issue description
`BracketedPattern.firstPage/lastPage/pagePrefix` currently detect roman numerals using regex searches that can match **inside ordinary words** (because `ROMAN_NUMBER` is `[ivxlcdmIVXLCDM]+` and is used with `find()`), which can produce incorrect expansions for citation key patterns like `[pageprefix]`/`[lastpage]`.
### Issue Context
The codebase already acknowledges pages values like `"article 123"` (handled in `JabRefItemDataProvider`), so the pages field is not guaranteed to be purely numeric/range-like.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1057]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1098-1123]
- jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[14-21]
- jablib/src/main/java/org/jabref/logic/citationstyle/JabRefItemDataProvider.java[133-138]
### Fix approach (guidance)
- Update token extraction and prefix detection to only consider roman/decimal sequences that appear as **page-number tokens** (e.g., bounded by start/end or typical separators like comma, whitespace, `-`, `--`, `�`, `+`, `/`), rather than any substring match.
- Add a regression test demonstrating that pages like `"article 123"` do **not** yield roman-derived tokens/prefixes.

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

21. bibentryRomanExpansionFirsPageTest typo 📘 Rule violation ⚙ Maintainability

Description

A new test method name contains a spelling mistake (Firs instead of First), reducing readability
and discoverability. This violates the naming/code style requirement to avoid typos.

Code

jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[R438-442]

+    @Test
+    void bibentryRomanExpansionFirsPageTest() {
+        BracketedPattern pattern = new BracketedPattern("[year]_[auth]_[firstpage]");
+        assertEquals("2017_Kitsune_iv", pattern.expand(romanentry));
+    }

Evidence
PR Compliance ID 3 requires correctly spelled names following project conventions. The newly added
test method is named bibentryRomanExpansionFirsPageTest, which contains a typo.
AGENTS.md
jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[438-442]

Agent prompt

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

## Issue description
The test method name contains a typo (`Firs`), which should be corrected to `First`.
## Issue Context
Consistent, correctly spelled identifiers improve maintainability and searchability.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[438-442]

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

View more (4)

22. Roman regex matches words 🐞 Bug ≡ Correctness

Description

BracketedPattern pagePrefix/firstPage/lastPage use PagesChecker.ROMAN_NUMBER to find roman tokens
anywhere, so page strings containing ordinary words (e.g., "article 123") can yield incorrect
prefixes/tokens and thus wrong generated citation keys.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1057]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;

Evidence

PagesChecker.ROMAN_NUMBER is defined as [ivxlcdmIVXLCDM]+, which can match substrings inside
normal words. BracketedPattern’s tokenization
(Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages)) and pagePrefix
(Pattern.compile(PagesChecker.ROMAN_NUMBER)) both use these patterns without requiring page-token
boundaries, so a pages value like "article 123" (explicitly handled as a real-world input in
citation-style conversion) would match the "icl" substring inside "article" and can influence
pagePrefix/lastPage results. Cita...

eduardo-kurek · 2026-04-04T18:07:06Z

The pipeline for guard-review is failling, idk why. Since i didnt change worflow files. Please notify me if i have to do something else to fix this.

eduardo-kurek · 2026-04-04T19:17:24Z

The errros in Unit tests from jabgui is happening in main too. Also, it is not related to my task

qodo-free-for-open-source-projects · 2026-04-04T19:17:53Z

Review Summary by Qodo

Support Roman numerals and BibLaTeX page formats

✨ Enhancement 🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Add support for Roman numerals in page ranges
• Add support for forward slash separator and sequens/sequentes suffixes
• Refactor page number parsing to handle mixed decimal and Roman formats
• Add comprehensive test cases for valid and invalid page formats

Diagram

flowchart LR
  A["Page Input<br/>1-10, iv-xx, 1/10"] --> B["PagesChecker<br/>Regex Patterns"]
  B --> C["Roman Number<br/>Pattern"]
  B --> D["Decimal Number<br/>Pattern"]
  B --> E["Separators<br/>-, --, /, +"]
  B --> F["Sequens/Sequentes<br/>f, ff, sq, sqq"]
  C --> G["BracketedPattern<br/>Page Processing"]
  D --> G
  E --> G
  F --> G
  G --> H["firstPage<br/>lastPage<br/>pagePrefix"]
  H --> I["Citation Key<br/>Generation"]

File Changes

1. jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java ✨ Enhancement +33/-17

Extend page patterns for Roman numerals and separators

• Refactored regex patterns to support Roman numerals and new separators
• Added public constants for ROMAN_NUMBER, DECIMAL_NUMBER, and PAGE_NUMBER
• Added support for forward slash / as page range separator
• Added support for sequens/sequentes suffixes (f, ff, sq, sqq)
• Restructured PAGES_EXP_BIBTEX and PAGES_EXP_BIBLATEX patterns for clarity

jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java

2. jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java ✨ Enhancement +81/-24

Add Roman numeral support to page extraction

• Added imports for Comparator, Map, and PagesChecker
• Replaced NOT_DECIMAL_DIGIT pattern with DIGITS_PATTERN and PAGE_NUMBER_PATTERN
• Implemented romanToInteger() method to convert Roman numerals to integers
• Refactored firstPage() to use new getPageNumberTokens() and pageTokenComparator()
• Enhanced pagePrefix() to handle both decimal and Roman numerals with proper prefix extraction
• Updated lastPage() to support Roman numeral comparison

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java

3. jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java 🧪 Tests +17/-0

Add Roman numeral citation key tests
• Added romanentry test fixture with Roman numeral pages SXX--ivS
• Added test bibentryRomanExpansionFirsPageTest() for first page extraction
• Added test bibentryRomanExpansionLastPageTest() for last page extraction
jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java

View more (4)

4. jablib/src/test/java/org/jabref/logic/citationkeypattern/CitationKeyGeneratorTest.java 🧪 Tests +6/-3

Update page prefix and last page tests
• Updated pagePrefixData() test cases to reflect new behavior for invalid ranges
• Added test cases for mixed prefix and Roman numeral pages (S5-K10, Siv-KXX)
• Updated lastPageData() with test case for Roman numerals (ivS--SXXY)
jablib/src/test/java/org/jabref/logic/citationkeypattern/CitationKeyGeneratorTest.java

5. jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java 🧪 Tests +18/-2

Expand BibLaTeX page format test coverage

• Added test cases for prefix-only pages (S436-S439)
• Added test cases for prefix and suffix combinations (S436S-S439S)
• Added test cases for forward slash separator (A10S/90, 1/10)
• Added test cases for sequens/sequentes suffixes (1f, 1ff, 1 f., 1sq, 1sqq)
• Added test cases for Roman numerals (i, ivxlcdm, IVXLCDM, iS, i-vi, VII-xii)
• Added invalid range test cases to rejection list

jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java

6. jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBibtexTest.java 🧪 Tests +14/-2

Expand BibTeX page format test coverage

• Added test cases for prefix-only pages (S436--S439)
• Added test cases for prefix and suffix combinations (S436S--S439S)
• Added test cases for Roman numerals (i, ivxlcdm, IVXLCDM, iS, i--vi, VII--xii)
• Added invalid range test cases to rejection list

jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBibtexTest.java

7. CHANGELOG.md 📝 Documentation +1/-0

Document page checker enhancements

• Added entry documenting fix for pages checker to support BibLaTeX-specific formats
• Mentions Roman numerals, forward slashes, and Latin continuity suffixes support

CHANGELOG.md

qodo-free-for-open-source-projects · 2026-04-04T19:17:54Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. ~~Objects.requireNonNull added to pages~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

New code introduces Objects.requireNonNull(pages) instead of expressing nullness via JSpecify
annotations, adding runtime checks and noise. This violates the repository’s null-safety approach
standardization requirement.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1045]
+        Objects.requireNonNull(pages);

Evidence
PR Compliance ID 8 forbids introducing Objects.requireNonNull and requires nullness to be
expressed using JSpecify annotations instead. The modified methods firstPage, pagePrefix, and
lastPage now call Objects.requireNonNull(pages).
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1046]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1102]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1134]

Agent prompt

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

## Issue description
`Objects.requireNonNull(pages)` was introduced in page-related helpers; this conflicts with the project’s requirement to use JSpecify nullness annotations instead of runtime null checks.
## Issue Context
The affected methods already document “may not be null” and should express that via JSpecify/package null-marking rather than `Objects.requireNonNull`.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1044-1049]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1097-1124]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1132-1139]

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

2. ~~Pattern.compile used inside methods~~ ☑ 📘 Rule violation ➹ Performance

Description

New code compiles regex patterns and uses String.matches(...) at runtime in hot paths instead of
using precompiled Pattern constants. This violates the guideline to prefer compiled patterns and
avoid repeated regex compilation/matching overhead.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1066]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;
+    }
+
+    private static Comparator<String> pageTokenComparator() {
+        return Comparator.comparing(token -> {
+            if (token.matches("\\d+")) {
+                return new BigInteger(token);
+            }
+            return BigInteger.valueOf(romanToInteger(token));
+        });

Evidence
PR Compliance ID 13 requires precompiled Pattern usage instead of ad-hoc regex matching (including
String.matches). The new code compiles patterns inside getPageNumberTokens/pagePrefix and uses
token.matches("\\d+") inside the comparator.
AGENTS.md
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

Agent prompt

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

## Issue description
Regex patterns are compiled inside methods and `String.matches(...)` is used in the comparator; this repeats regex compilation/evaluation and violates the project rule to use precompiled `Pattern` constants.
## Issue Context
`getPageNumberTokens` and `pagePrefix` can be called frequently during citation key generation; precompiling patterns improves performance and readability.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1066]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1100-1102]

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

3. ~~Duplicate #15457 changelog entry~~ ☑ 📘 Rule violation ⚙ Maintainability

Description

CHANGELOG.md contains two nearly identical bullet points for the same fix (#15457), creating
inconsistent and unprofessional release notes. This should be reduced to a single, consistently
worded entry.

Code

CHANGELOG.md[R50-51]

+- We fixed pages checker to allow biblatex specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)
+- We fixed pages checker to allow BibLaTeX-specific formats, including Roman numerals, forward slashes, and Latin continuity suffixes (f., ff., sq., sqq.). [#15457](https://github.com/JabRef/jabref/issues/15457)

Evidence

PR Compliance ID 46 requires professional, precise, consistently formatted release notes. Two
consecutive bullets describe the same change for #15457 with differing capitalization/wording, which
is inconsistent formatting.

CHANGELOG.md[50-51]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
The changelog includes a duplicated bullet for issue #15457 with inconsistent wording/capitalization.
## Issue Context
Release notes should be professional and consistently formatted; each fix should appear once.
## Fix Focus Areas
- CHANGELOG.md[50-51]

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

4. bibentryRomanExpansionFirsPageTest typo 📘 Rule violation ⚙ Maintainability

Description

A new test method name contains a spelling mistake (Firs instead of First), reducing readability
and discoverability. This violates the naming/code style requirement to avoid typos.

Code

jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[R438-442]

+    @Test
+    void bibentryRomanExpansionFirsPageTest() {
+        BracketedPattern pattern = new BracketedPattern("[year]_[auth]_[firstpage]");
+        assertEquals("2017_Kitsune_iv", pattern.expand(romanentry));
+    }

Evidence
PR Compliance ID 3 requires correctly spelled names following project conventions. The newly added
test method is named bibentryRomanExpansionFirsPageTest, which contains a typo.
AGENTS.md
jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[438-442]

Agent prompt

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

## Issue description
The test method name contains a typo (`Firs`), which should be corrected to `First`.
## Issue Context
Consistent, correctly spelled identifiers improve maintainability and searchability.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/citationkeypattern/BracketedPatternTest.java[438-442]

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

5. Roman regex matches words 🐞 Bug ≡ Correctness

Description

BracketedPattern pagePrefix/firstPage/lastPage use PagesChecker.ROMAN_NUMBER to find roman tokens
anywhere, so page strings containing ordinary words (e.g., "article 123") can yield incorrect
prefixes/tokens and thus wrong generated citation keys.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1051-1057]

+    private static List<String> getPageNumberTokens(String pages) {
+        Matcher matcher = Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages);
+        List<String> tokens = new ArrayList<>();
+        while (matcher.find()) {
+            tokens.add(matcher.group());
+        }
+        return tokens;

Evidence
PagesChecker.ROMAN_NUMBER is defined as [ivxlcdmIVXLCDM]+, which can match substrings inside
normal words. BracketedPattern’s tokenization
(Pattern.compile(PagesChecker.PAGE_NUMBER).matcher(pages)) and pagePrefix
(Pattern.compile(PagesChecker.ROMAN_NUMBER)) both use these patterns without requiring page-token
boundaries, so a pages value like "article 123" (explicitly handled as a real-world input in
citation-style conversion) would match the "icl" substring inside "article" and can influence
pagePrefix/lastPage results. Citation keys are generated during save using
CitationKeyGenerator (extends BracketedPattern) without sanitizing pages values, and integrity
checking only emits messages (does not rewrite fields).
jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[14-21]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1057]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1098-1123]
jablib/src/main/java/org/jabref/logic/citationstyle/JabRefItemDataProvider.java[133-138]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[407-413]
jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[205-212]
jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[405-416]
jablib/src/main/java/org/jabref/logic/integrity/FieldChecker.java[12-27]

Agent prompt

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

## Issue description
`BracketedPattern.firstPage/lastPage/pagePrefix` currently detect roman numerals using regex searches that can match **inside ordinary words** (because `ROMAN_NUMBER` is `[ivxlcdmIVXLCDM]+` and is used with `find()`), which can produce incorrect expansions for citation key patterns like `[pageprefix]`/`[lastpage]`.
### Issue Context
The codebase already acknowledges pages values like `"article 123"` (handled in `JabRefItemDataProvider`), so the pages field is not guaranteed to be purely numeric/range-like.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1051-1057]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1098-1123]
- jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[14-21]
- jablib/src/main/java/org/jabref/logic/citationstyle/JabRefItemDataProvider.java[133-138]
### Fix approach (guidance)
- Update token extraction and prefix detection to only consider roman/decimal sequences that appear as **page-number tokens** (e.g., bounded by start/end or typical separators like comma, whitespace, `-`, `--`, `�`, `+`, `/`), rather than any substring match.
- Add a regression test demonstrating that pages like `"article 123"` do **not** yield roman-derived tokens/prefixes.

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

6. Inefficient roman numeral conversion 🐞 Bug ➹ Performance ⭐ New

Description

romanToInteger rebuilds a Map on every call and accumulates into an int before converting to
BigInteger, which is unnecessary work and can overflow for pathological inputs. This can be
simplified by using a static map and a wider accumulator type.

Code

jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[R1068-1088]

+    private static int romanToInteger(String s) {
+        Map<Character, Integer> va = Map.of(
+                'I', 1, 'V', 5, 'X', 10,
+                'L', 50, 'C', 100, 'D', 500, 'M', 1000
+        );
+        String roman = s.toUpperCase();
+        int res = 0;
+        for (int i = 0; i < roman.length(); i++) {
+            int v1 = va.get(roman.charAt(i));
+            if (i + 1 < roman.length()) {
+                int v2 = va.get(roman.charAt(i + 1));
+                if (v1 >= v2) {
+                    res += v1;
+                } else {
+                    res -= v1;
+                }
+            } else {
+                res += v1;
+            }
+        }
+        return res;

Evidence
romanToInteger allocates Map.of(...) each invocation and returns int; the caller immediately
wraps it into BigInteger.valueOf(...) for comparisons.
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1059-1065]
jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1068-1088]

Agent prompt

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

### Issue description
`romanToInteger` creates a new `Map.of(...)` on every call and uses an `int` accumulator even though results are immediately wrapped into `BigInteger`. This is avoidable allocation and has theoretical overflow risk for very long inputs.

### Issue Context
This method is called during citation key expansion when `[firstpage]`/`[lastpage]` are used.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1059-1065]
- jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java[1068-1088]

### Implementation direction
- Hoist the roman-value lookup map into a `private static final Map<Character, Integer>`.
- Use a `long` accumulator (or directly build a `BigInteger`) and only convert once, avoiding repeated boxing/allocation.

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

ⓘ The new review experience is currently in Beta. Learn more

InAnYan · 2026-04-05T12:22:16Z

+    }
+
+    private static int romanToInteger(String s) {
+        Map<Character, Integer> va = Map.of(


I think this could be moved as a constant

Sure! Already updated it.

InAnYan

While I'm not an expert in BibLaTeX standard, the code looks good and the tests don't fail

calixtus · 2026-04-05T19:04:59Z

+
+    private static int romanToInteger(String s) {
+        String roman = s.toUpperCase();
+        int res = 0;


Please unabbreviate 'res'. Its not immediatley clear what that means

calixtus · 2026-04-05T19:06:23Z

+        boolean foundDecimal = decimalMatcher.find();
+        int decimalStart = foundDecimal ? decimalMatcher.start() : -1;
+
+        boolean foundRoman = romanMatcher.find();
+        int romanStart = foundRoman ? romanMatcher.start() : -1;


Please put a short comment here about why -1

Hi, i will put it!

eduardo-kurek · 2026-04-05T20:47:45Z

The error on CHANGELOG is related to JBang

calixtus

Moving forward here. Thanks for your contribution

eduardo-kurek added 14 commits April 4, 2026 11:22

Refactor regex and add new tests

c80c28f

Add page checker for separator '/'

b2d0e12

Add support to sequens and sequents

d35e7e7

Add support to roman numerals

865c3da

Allow romand numerals, forward slash and sequentes in page checker

092bd43

Remove comment

acccd44

Fix comments from qodo

707a319

Fix citation for roman firstpage

21281cd

Fix citation for roman lastpage

27763e0

Fix pageprefix for roman numerals

1239147

Trigger actions

df75c4e

Fix lastPage and pagePrefix

59fd704

Fix checkstyle

c99f5e8

Triggering CI after force push

9a88f43

github-actions Bot added the component: lint label Apr 4, 2026

qodo-free-for-open-source-projects Bot reviewed Apr 4, 2026

View reviewed changes

Comment thread jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java Outdated

Comment thread jablib/src/main/java/org/jabref/logic/citationkeypattern/BracketedPattern.java

Comment thread CHANGELOG.md Outdated

github-actions Bot added the status: no-bot-comments label Apr 4, 2026

Fix qodo comments

b05ec6c

github-actions Bot mentioned this pull request Apr 4, 2026

Cleanup Entries do not support broader page formats #15457

Closed

2 tasks

Remove null check

3c46035

eduardo-kurek marked this pull request as draft April 4, 2026 18:46

eduardo-kurek marked this pull request as ready for review April 4, 2026 19:17

InAnYan reviewed Apr 5, 2026

View reviewed changes

InAnYan previously approved these changes Apr 5, 2026

View reviewed changes

Refactor in romanToInteger function

a0ca98d

eduardo-kurek dismissed InAnYan’s stale review via a0ca98d April 5, 2026 18:57

calixtus reviewed Apr 5, 2026

View reviewed changes

eduardo-kurek and others added 2 commits April 5, 2026 16:22

Add comment explaining the -1 in ternary operation

48f8b07

Merge branch 'main' into fix-for-issue-15457

2db517e

github-actions Bot added status: changes-required Pull requests that are not yet complete and removed status: no-bot-comments labels Apr 5, 2026

eduardo-kurek requested review from InAnYan and calixtus April 5, 2026 20:47

Refactor romanToInteger method to use 'value' variable

7118bf2

github-actions Bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Apr 6, 2026

calixtus approved these changes Apr 6, 2026

View reviewed changes

calixtus added this pull request to the merge queue Apr 6, 2026

github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Apr 6, 2026

Merged via the queue into JabRef:main with commit 8684f3f Apr 6, 2026
53 checks passed

Uh oh!

Conversation

eduardo-kurek commented Apr 4, 2026

Related issues and pull requests

PR Description

Steps to test

Checklist

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 4, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eduardo-kurek commented Apr 4, 2026

Uh oh!

eduardo-kurek commented Apr 4, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 4, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

InAnYan Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

eduardo-kurek Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

InAnYan left a comment

Choose a reason for hiding this comment

Uh oh!

calixtus Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

calixtus Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

eduardo-kurek Apr 5, 2026

Choose a reason for hiding this comment

Uh oh!

eduardo-kurek commented Apr 5, 2026

Uh oh!

calixtus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

qodo-free-for-open-source-projects Bot commented Apr 4, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Apr 4, 2026 •

edited

Loading