• Mark bst package as nonnull by default with @NullMarked annotation
• Remove null handling code, enforce non-null contracts throughout
• Replace null values with empty strings in string operations
• Improve null safety in BstVMContext and related classes
• Refactor test access methods and enhance code clarity

Add null safety annotations and remove null handling

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java

Add @NullMarked, refactor stack access API

jablib/src/main/java/org/jabref/logic/bst/BstVM.java

Add null safety annotations to record

jablib/src/main/java/org/jabref/logic/bst/BstVMContext.java

Description

BstVM is now @NullMarked, but render(List, BibDatabase) still documents bibDatabase as
potentially null while its signature does not declare @Nullable. This misaligned null-safety
contract can lead to incorrect usage (passing null) and runtime failures when null is assumed
impossible downstream.

Code

jablib/src/main/java/org/jabref/logic/bst/BstVM.java[R65-68]

 /// @param bibEntries  list of entries to convert
 /// @param bibDatabase (may be null) the bibDatabase used for resolving strings / crossref
 /// @return list of references in plain text form
-    public String render(@NonNull Collection<BibEntry> bibEntries, BibDatabase bibDatabase) {
+    public String render(List<BibEntry> bibEntries, BibDatabase bibDatabase) {

Evidence

PR Compliance ID 48 requires null-safety contracts to be aligned (annotations/contract must match
behavior). In BstVM (now @NullMarked), the render method parameter bibDatabase is implicitly
non-null, yet the Javadoc explicitly states it may be null, creating a contract mismatch.

jablib/src/main/java/org/jabref/logic/bst/BstVM.java[63-75]
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
`BstVM` is annotated `@NullMarked`, but `render(List<BibEntry>, BibDatabase)` still documents `bibDatabase` as “may be null” while the method signature does not declare it as `@Nullable`. This creates a mismatch between documented behavior and the nullness contract implied by `@NullMarked`.
## Issue Context
The compliance rule requires overrides/public APIs to have consistent nullability contracts. If `bibDatabase` is truly optional, it must be declared `@Nullable` and handled safely (e.g., using a default). If it is required, the Javadoc must be updated to remove the “may be null” claim and call sites must not pass null.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bst/BstVM.java[63-89]
- jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[53-60]

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

Description

bstEmpty now throws when the operand is null, but missing entry fields are still represented as null
and pushed onto the stack, so common field empty$ usage can crash the VM. This contradicts
existing tests expecting missing fields to be treated as empty.

Code

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[R398-406]

     Object o1 = stack.pop();
-        if (o1 == null) {
-            LOGGER.trace("null is empty");
-            stack.push(BstVM.TRUE);
-            return;
-        }
-
     if (!(o1 instanceof String s)) {
         throw new BstVMException("Operand does not match function empty$ (line %d)".formatted(ctx.start.getLine()));
     }
-        boolean result = s.trim().isEmpty();
+        boolean result = StringUtil.isBlank(s);
     LOGGER.trace("empty$({}) result: {}", s, result);
     stack.push(result ? BstVM.TRUE : BstVM.FALSE);

Evidence

Missing fields are initialized as null and then pushed onto the VM stack; empty$ now requires a
String and throws for null, while the test suite demonstrates the expected behavior is that a
missing field counts as empty.

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[394-406]
jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[151-160]
jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[198-207]
jablib/src/main/java/org/jabref/logic/bst/BstVMContext.java[24-27]
jablib/src/test/java/org/jabref/logic/bst/BstFunctionsTest.java[224-248]

Agent prompt

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

## Issue description
`empty$` now throws for `null` operands, but the VM still uses `null` to represent missing fields and pushes those nulls onto the stack. This can crash typical `.bst` programs that do `field empty$`.
### Issue Context
- ENTRY fields are initialized to `null` and remain `null` when unresolved.
- `resolveIdentifier` pushes the field value (possibly `null`) onto the stack.
- Existing tests expect missing fields to be treated as empty.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[394-406]
- jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[151-160]
- jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[198-207]
### Suggested change
Restore prior behavior in `bstEmpty`: if operand is `null`, push `TRUE` and return (or otherwise coerce `null` to ""). Keep throwing only for non-null, non-String operands.

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

Description

bstConcat removed the null-to-empty-string normalization and now logs
o1.getClass()/o2.getClass() inside the error path, which will throw NullPointerException if
either operand is null. Since missing fields are still pushed as null, concatenating optional fields
can crash.

Code

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[R192-197]

     Object o2 = stack.pop();
     Object o1 = stack.pop();
-        if (o1 == null) {
-            o1 = "";
-        }
-        if (o2 == null) {
-            o2 = "";
-        }
-
     if (!((o1 instanceof String) && (o2 instanceof String))) {
         LOGGER.error("o1: {} ({})", o1, o1.getClass());
         LOGGER.error("o2: {} ({})", o2, o2.getClass());

Evidence

The visitor still pushes null field values onto the stack; bstConcat’s error logging dereferences
operands (getClass) even when they may be null, and the previous code that converted null to "" was
removed.

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[188-202]
jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[151-160]
jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[198-207]
jablib/src/main/java/org/jabref/logic/bst/BstVMContext.java[24-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
`bstConcat` can throw `NullPointerException` when either operand is `null` because it logs `o1.getClass()`/`o2.getClass()` and no longer normalizes nulls. Missing fields are still represented as `null` and pushed onto the stack.
### Issue Context
BST programs commonly concatenate optional fields; the VM currently uses `null` to represent a missing field value.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[188-202]
### Suggested change
- Reintroduce null-to-empty-string normalization (e.g., treat null as "").
- Make logging null-safe (e.g., log `o1 == null ? "<null>" : o1.getClass()`), or avoid logging `getClass()` on potentially null references.

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

Description

New/updated code uses Optional.orElse("") to convert absence into an empty string (e.g., for
citationKey and preamble). This can mask missing values and is non-idiomatic per the
Optional-usage compliance rule unless the empty-string fallback is handled explicitly/justified.

Code

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[R370-376]

+        public void execute(BstVMVisitor visitor, ParserRuleContext ctx, @Nullable BstEntry bstEntry) {
+            if (bstEntry == null) {
             execute(visitor, ctx);
             return;
         }
-            stack.push(bstEntryContext.entry.getCitationKey().orElse(null));
+            stack.push(bstEntry.entry.getCitationKey().orElse(""));

Evidence

PR Compliance ID 11 forbids introducing orElse("") patterns that turn missing Optional values into
empty strings without clear justification. The changed code pushes an empty string when
citationKey is absent and also defaults missing preamble to "" via orElse("").

AGENTS.md
jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[53-60]
jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[361-377]

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 updated code uses `Optional.orElse("")` to substitute missing values with an empty string. This hides absence and violates the Optional idioms compliance rule unless the fallback is handled explicitly.
## Issue Context
If the BST VM semantics truly require the “null string” to be represented as `""`, handle the absence explicitly (e.g., `ifPresentOrElse(...)` with a clear comment/log), or model the value as an `Optional<String>` until the final push/assignment point.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[53-60]
- jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[361-377]

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

Description

BstFunctions now uses Optional.of(bibDatabase) for preamble extraction, which throws if
bibDatabase is null, even though BstVM.render(..., bibDatabase) still documents that the
database may be null. Calling render(entries, null) will now crash during VM setup.

Code

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[R55-59]

     this.strings = bstVMContext.strings();
     this.integers = bstVMContext.integers();
     this.functions = bstVMContext.functions();
-        this.preamble = Optional.ofNullable(bstVMContext.bibDatabase()).flatMap(BibDatabase::getPreamble).orElse("");
+        this.preamble = Optional.of(bstVMContext.bibDatabase()).flatMap(BibDatabase::getPreamble).orElse("");
     this.stack = bstVMContext.stack();

Evidence

The constructor unconditionally wraps bstVMContext.bibDatabase() with Optional.of(...), while
the render API explicitly states the argument may be null; the two are inconsistent and produce an
NPE if the documented contract is used.

jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[53-60]
jablib/src/main/java/org/jabref/logic/bst/BstVM.java[63-75]

Agent prompt

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

## Issue description
`BstFunctions` assumes `bibDatabase` is non-null (`Optional.of(...)`), but `BstVM.render(..., bibDatabase)` is documented as allowing null. This mismatch creates an easy-to-hit NPE for callers following the documentation.
### Issue Context
Even if current in-repo call sites pass a non-null database, the public API contract still claims null is acceptable.
### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bst/BstFunctions.java[53-60]
- jablib/src/main/java/org/jabref/logic/bst/BstVM.java[63-75]
### Suggested change (choose one)
1) Preserve contract: switch back to `Optional.ofNullable(bstVMContext.bibDatabase())` and annotate the render parameter/context field as `@Nullable` as needed.
2) Tighten contract: update Javadoc to state `bibDatabase` must be non-null and add `Objects.requireNonNull(bibDatabase)` in `render(...)` (and keep `Optional.of(...)`).

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

Description

The added comment /// Trims braces and resolve months restates what the helper does rather than
explaining rationale/constraints. This reduces signal-to-noise in documentation comments.

Code

jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[R98-99]

+    /// Trims braces and resolve months
+    private static String normalizeFieldValue(String content, FieldWriter fieldWriter, Field field) {

Evidence

PR Compliance ID 5 requires comments to explain "why" rather than repeating behavior. The new ///
comment is a simple paraphrase of the method’s implementation.

AGENTS.md
jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[98-99]

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 comment above `normalizeFieldValue(...)` is a "what" comment and does not add rationale.
## Issue Context
Project guideline prefers comments that explain intent/constraints rather than restating code.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/bst/BstVMVisitor.java[98-99]

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