Allow suffix letter after the numbers in page range

[mikezhanghaozhe]

Relax the page checker regex so that suffix letters are accepted such as "436S-439S".

Related issues and pull requests

Closes #13701

PR Description

Change SINGLE_PAGE_PATTERN to allow suffix letter after the numbers. Update the unit test for both valid and invalid cases.

Steps to test

1. open the following test library: https://github.com/JabRef/jabref-demonstration-libraries/blob/cc066bda1cb494c3994620021445bde9b0b4ec03/chocolate/Chocolate.bib#L40
2. click the entry, Chocolate: Food as Medicine/Medicine as Food.
3. go to "Optional field" and notice that the warning disappears.
4. additionally, cleanup entries for "Pages" will not affect the original page range.

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]

Review Summary by Qodo

Allow suffix letters in page range validation

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Relaxed page range regex to accept suffix letters like "436S-439S"
• Updated SINGLE_PAGE_PATTERN to allow optional suffix after numbers
• Added comprehensive unit tests for valid and invalid suffix cases
• Updated changelog to document the fix

Diagram

flowchart LR
  A["SINGLE_PAGE_PATTERN<br/>Old: [A-Za-z]?\\d*"] -->|"Add suffix support"| B["SINGLE_PAGE_PATTERN<br/>New: [A-Za-z]?\\d*[A-Za-z]?"]
  B -->|"Validates"| C["Page ranges<br/>436S-439S"]
  D["Unit Tests"] -->|"Added cases"| E["Valid: 436S-439S<br/>Invalid: 436SS-439S"]

Loading

File Changes

1. jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java 🐞 Bug fix +1/-1

Add optional suffix letter support to page pattern

• Modified SINGLE_PAGE_PATTERN regex from [A-Za-z]?\\d* to [A-Za-z]?\\d*[A-Za-z]?
• Added support for optional suffix letters after page numbers
• Updated comment to reflect optional prefix, numbers, and optional suffix

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

---

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

Add biblatex suffix letter test cases

• Added test case "436S-439S" to valid page ranges (single dash with suffix)
• Added test case "436SS-439S" to invalid page ranges (multiple suffix letters)
• Ensures biblatex format accepts single suffix letters but rejects multiple

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

---

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

Add bibtex suffix letter test cases

• Added test case "436S--439S" to valid page ranges (double dash with suffix)
• Added test case "436S-439S" to invalid page ranges (single dash forbidden in bibtex)
• Validates bibtex-specific range separator requirements with suffix letters

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

---

View more (1)

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

Document page range suffix letter fix

• Added entry documenting the fix for suffix letters in page ranges
• References issue #13701
• Placed in "Fixed" section for user-visible changes

CHANGELOG.md

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Letter-only pages accepted 🐞 Bug ✓ Correctness

Description

PagesChecker's updated SINGLE_PAGE_PATTERN allows page tokens with no digits when both the optional
prefix and optional suffix letters are present (e.g., "SS"), so integrity checking can silently
accept malformed page ranges like "SS-SS". This exceeds the documented intent of validating “page
numbers or range of numbers”.

Code

jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[14]

+    private static final String SINGLE_PAGE_PATTERN = "[A-Za-z]?\\d*[A-Za-z]?";             // optional prefix, numbers, and optional suffix

Evidence

The full validation regex is anchored (\A…\z) and built by concatenating SINGLE_PAGE_PATTERN for the
first and optional second page token, so any string matched by SINGLE_PAGE_PATTERN is considered a
valid page token. With the new pattern [A-Za-z]?\d*[A-Za-z]?, the input "SS" matches as:
prefix='S', digits='', suffix='S', therefore the checker would treat it as valid even though the
class comment describes the field as containing page numbers/ranges of numbers.

jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[12-35]
jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[47-54]

Agent prompt

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

## Issue description
`PagesChecker` now treats letter-only page tokens like `"SS"` as valid because `SINGLE_PAGE_PATTERN` allows an optional prefix letter + zero digits + an optional suffix letter. This can hide malformed page values from the integrity checker.
### Issue Context
The checker is documented to validate “page numbers or range of numbers”. We still want to accept suffix letters after digits (e.g., `436S-439S`) without opening the door to prefix+suffix with no digits.
### Fix approach (suggested)
- Change `SINGLE_PAGE_PATTERN` to an alternation that:
- allows the legacy prefix-only form (`[A-Za-z]?\d*`) to preserve current behavior for `43+` handling, but
- only allows a suffix letter when at least one digit exists.
For example:

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

---

2. CHANGELOG uses internal wording 📘 Rule violation ✓ Correctness

Description

The CHANGELOG entry uses the internal term pages checker and is grammatically imprecise, making it
less end-user focused. This reduces clarity of the release note for non-developers.

Code

CHANGELOG.md[39]

+- We fixed pages checker to allow suffix letters in the page range like "436S-439S". [#13701](https://github.com/JabRef/jabref/issues/13701)

Evidence

Compliance requires CHANGELOG entries to be end-user focused and user-facing text to be precise and
grammatically correct; the added entry uses internal jargon (pages checker) and awkward wording.

AGENTS.md
CHANGELOG.md[39-39]
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 entry is not clearly end-user focused and uses internal jargon (`pages checker`), with slightly awkward grammar.
## Issue Context
CHANGELOG entries are user-facing release notes and should be understandable and professionally phrased.
## Fix Focus Areas
- CHANGELOG.md[39-39]

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

---

3. Trivial // suffix comment 📘 Rule violation ✓ Correctness

Description

The added // suffix comment restates what the next test value already shows without explaining
intent or rationale. This adds noise and reduces readability in tests.

Code

jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java[43]

+                // suffix

Evidence

The compliance rule asks to avoid trivial/restating comments; // suffix does not explain why this
case matters beyond what the literal already communicates.

AGENTS.md
jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java[42-44]

Agent prompt

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

## Issue description
A newly added test comment (`// suffix`) is trivial and does not explain the rationale for the test case.
## Issue Context
Comments should explain intent/why, not restate what the next line already shows.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java[42-44]

ⓘ 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

[qodo-free-for-open-source-projects]

1. Letter-only pages accepted 🐞 Bug ✓ Correctness

PagesChecker's updated SINGLE_PAGE_PATTERN allows page tokens with no digits when both the optional
prefix and optional suffix letters are present (e.g., "SS"), so integrity checking can silently
accept malformed page ranges like "SS-SS". This exceeds the documented intent of validating “page
numbers or range of numbers”.

Agent Prompt

### Issue description
`PagesChecker` now treats letter-only page tokens like `"SS"` as valid because `SINGLE_PAGE_PATTERN` allows an optional prefix letter + zero digits + an optional suffix letter. This can hide malformed page values from the integrity checker.

### Issue Context
The checker is documented to validate “page numbers or range of numbers”. We still want to accept suffix letters after digits (e.g., `436S-439S`) without opening the door to prefix+suffix with no digits.

### Fix approach (suggested)
- Change `SINGLE_PAGE_PATTERN` to an alternation that:
  - allows the legacy prefix-only form (`[A-Za-z]?\d*`) to preserve current behavior for `43+` handling, but
  - only allows a suffix letter when at least one digit exists.

For example:
```java
private static final String SINGLE_PAGE_PATTERN = "(?:[A-Za-z]?\\d+[A-Za-z]?|[A-Za-z]?\\d*)";
```
This preserves numeric pages and `43+` behavior, allows `436S`, but rejects `SS`.

- Add unit tests to ensure `"SS"` is rejected in both modes (and optionally `"SS-SS"`/`"SS--SS"`).

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/integrity/PagesChecker.java[14-35]
- jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBiblatexTest.java[29-65]
- jablib/src/test/java/org/jabref/logic/integrity/PagesCheckerBibtexTest.java[29-65]

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

[github-actions]

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.

[SaurabhSinghRajput-8777]

/assign-me

[mikezhanghaozhe]

Hi Saurabh, I am still working on this issue.

[subhramit]

please align the comment with the ones below

[subhramit]

LGTM sans one minor comment^ (& changelog conflict - please resolve)

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: ff179b7
▶️ Tests: 10210 executed
⚪️ Checks: 52/52 completed

---

Learn more about TestLens at testlens.app.

[mikezhanghaozhe]

@subhramit Hi, thanks for the approval. I am a bit confused about the current state of the PR. I have resolved the changelog conflicts locally, but I have not pushed that merge yet. Should I still push the changes or it is already handled by the auto-merge? Thanks so much for your guidance!

[subhramit]

@subhramit Hi, thanks for the approval. I am a bit confused about the current state of the PR. I have resolved the changelog conflicts locally, but I have not pushed that merge yet. Should I still push the changes or it is already handled by the auto-merge? Thanks so much for your guidance!

I took the opportunity to resolve it as this PR has been pending for 2 weeks.

[mikezhanghaozhe]

Got it! Thanks for resolving it.