Skip to content

Revert "Added reading and writing with JSON for save actions metadata" - #16556

Merged
subhramit merged 1 commit into
mainfrom
revert-15159-fix-for-issue-10371
Aug 13, 2026
Merged

Revert "Added reading and writing with JSON for save actions metadata"#16556
subhramit merged 1 commit into
mainfrom
revert-15159-fix-for-issue-10371

Conversation

@subhramit

@subhramit subhramit commented Aug 13, 2026

Copy link
Copy Markdown
Member

Reverts #15159

Due to internal discussion - change in priorities for v6.0, will continue for v7.0 or next year in the form of a GSoC project.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Revert JSON-based save actions metadata in .bib read/write path

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Revert JSON comment metadata support for save actions in BibTeX parse/write.
• Remove JSON metadata version flag and related Gson-based handling.
• Update tests to validate only legacy jabref-meta save actions comments.
Diagram

graph TD
  A["BibtexParser"] --> B["MetaDataParser"] --> C["MetaData"] --> D["BibDatabaseWriter"] --> E["BibWriter"] --> F[".bib file"]
  G["module-info.java"] --> H["Gson module dep"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Read JSON metadata, but stop writing it
  • ➕ Preserves backward compatibility while still accepting files produced by reverted versions
  • ➕ Reduces churn in user repositories by not re-emitting JSON blocks
  • ➖ Keeps extra parsing complexity and error paths in BibtexParser
  • ➖ Harder to fully remove the experimental format later
2. Versioned JSON schema with explicit migration
  • ➕ Makes format evolution explicit and testable
  • ➕ Can support multiple versions without ad-hoc parsing
  • ➖ More design work upfront (schema/versioning policy, migration rules)
  • ➖ Requires long-term maintenance commitment
3. Store structured metadata outside .bib comments
  • ➕ Avoids embedding complex structured data in BibTeX comment blocks
  • ➕ Reduces risk of other tools mangling metadata
  • ➖ Introduces additional sidecar file management and sync concerns
  • ➖ Potentially breaks workflows expecting single-file portability

Recommendation: Given this is explicitly a revert to address a reported issue, the current approach (fully removing JSON read/write and returning to the legacy jabref-meta representation) is the lowest-risk compatibility move. If JSON metadata is reintroduced later, consider either read-only support first or a versioned schema + migration plan to avoid repeated format reversals.

Files changed (6) +25 / -228

Bug fix (3) +0 / -70
BibDatabaseWriter.javaRemove JSON metadata emission for save actions +0/-30

Remove JSON metadata emission for save actions

• Removes the code path that serialized save actions into a 'jabref-meta-0.1.0' JSON comment block. Metadata writing now only uses the existing key/value comment format.

jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java

BibtexParser.javaRemove parsing of JSON metadata comments +0/-39

Remove parsing of JSON metadata comments

• Eliminates parsing/storage of 'jabref-meta-0.1.0' JSON comment blocks and the subsequent conversion into save actions. The parser now ignores that JSON-specific comment variant entirely.

jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java

MetaData.javaDrop JSON metadata version flag constant +0/-1

Drop JSON metadata version flag constant

• Removes the 'META_FLAG_V1' constant used to identify JSON metadata comment blocks, reflecting the revert of that format.

jablib/src/main/java/org/jabref/model/metadata/MetaData.java

Tests (2) +24 / -157
BibDatabaseWriterTest.javaUpdate save actions writer expectations (no JSON block) +8/-71

Update save actions writer expectations (no JSON block)

• Adjusts the save actions test to assert only the legacy 'jabref-meta: saveActions' output. Removes the round-trip test that ensured JSON metadata was not duplicated.

jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java

BibtexParserTest.javaRemove JSON save actions parsing tests and clean up formatting +16/-86

Remove JSON save actions parsing tests and clean up formatting

• Deletes integration/unit tests covering JSON metadata comment parsing and malformed JSON handling for save actions. Also includes minor test comment and whitespace/blank-line expectation adjustments.

jablib/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java

Other (1) +1 / -1
module-info.javaMake Gson requirement non-transitive +1/-1

Make Gson requirement non-transitive

• Changes the JPMS dependency on Gson from transitive to direct. This reduces accidental exposure of Gson to downstream modules.

jablib/src/main/java/module-info.java

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Arrays.asList used in test 📘 Rule violation ⚙ Maintainability
Description
The changed test uses Arrays.asList(...) instead of the modern immutable List.of(...),
regressing Java 25+ idioms and readability. This increases mutability risk and inconsistency with
project conventions.
Code

jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[R782-784]

        FieldFormatterCleanupActions saveActions = new FieldFormatterCleanupActions(true,
-                List.of(
+                Arrays.asList(
                        new FieldFormatterCleanup(StandardField.TITLE, new LowerCaseFormatter()),
Evidence
PR Compliance ID 2 requires preferring modern Java idioms such as List.of(...). The changed code
constructs the list with Arrays.asList(...) in writeSaveActions().

AGENTS.md: Use the Configured Gradle Toolchain and Modern Java (25+) Idioms
jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[781-786]

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 now uses `Arrays.asList(...)` where modern JabRef style prefers `List.of(...)`.

## Issue Context
The compliance checklist requires using modern Java idioms (Java 25+), including `List.of` over legacy list factories.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[782-786]

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


2. Multiline string built via + 📘 Rule violation ⚙ Maintainability
Description
The changed assertion builds a multi-line expected value via string concatenation instead of a text
block, reducing readability and maintainability. This conflicts with the modern Java idioms
requirement.
Code

jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[R792-795]

+        assertEquals("@Comment{jabref-meta: saveActions:enabled;"
+                + OS.NEWLINE
+                + "title[lower_case]" + OS.NEWLINE
+                + "journal[title_case]" + OS.NEWLINE
Evidence
PR Compliance ID 2 explicitly lists text blocks as a preferred modern Java idiom for multi-line
constants. The assertion constructs the expected output by concatenating multiple string literals
and OS.NEWLINE.

AGENTS.md: Use the Configured Gradle Toolchain and Modern Java (25+) Idioms
jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[791-798]

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 multi-line expected string is constructed using chained `+` concatenation with `OS.NEWLINE` instead of using a Java text block.

## Issue Context
The compliance checklist calls out text blocks as the preferred modern Java construct for multi-line constants.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[792-798]

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


3. {@link} used in /// 📘 Rule violation ⚙ Maintainability
Description
A changed Markdown Javadoc (///) comment uses a Javadoc inline tag {@link ...} instead of
Markdown-style linking. This violates the documentation style requirement and reduces consistency.
Code

jablib/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java[82]

+/// Tests cannot be executed concurrently, because Localization is used at {@link BibtexParser#parseAndAddEntry(String)}
Evidence
PR Compliance ID 5 requires using Markdown Javadoc conventions for /// and avoiding Javadoc inline
tags. The modified comment line contains {@link BibtexParser#parseAndAddEntry(String)}.

AGENTS.md: Remove Commented-Out Code and Avoid Trivial Comments; Use Markdown Javadoc (///) for Multi-line Comments
jablib/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java[80-83]

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 `///` documentation comment uses a Javadoc inline tag (`{@link ...}`) rather than Markdown Javadoc style.

## Issue Context
Project policy requires Markdown Javadoc (`///`) to use Markdown conventions and avoid Javadoc inline tags.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java[80-83]

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


View review recommended (1)
4. Stale JSON metadata persisted 🐞 Bug ≡ Correctness
Description
After the revert, BibtexParser no longer recognizes jabref-meta-0.1.0 JSON metadata comments, so
they are kept as ordinary pre-entry text/epilog and will be written back unchanged. Since
BibDatabaseWriter no longer rewrites JSON metadata, these legacy JSON blocks can become stale and
conflict with the canonical jabref-meta: metadata after subsequent saves.
Code

jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[L425-428]

-        } else if (comment.startsWith(MetaData.META_FLAG_V1)) {
-            parsedJsonMetaData = parseCommentToJson(comment, new ParserResult.Range(startLine, startColumn, line, column));
-
-            // JSON metadata comments are always re-written by JabRef and not stored in the file
Evidence
parseJabRefComment now only strips JabRef-managed comments for jabref-meta: and entrytype flags;
anything else remains in the parser's raw-text buffer. That raw text is later attached to entries
(or epilog) via dumpTextReadSoFarToString(). Meanwhile BibDatabaseWriter.writeMetaData no longer
emits any JSON metadata block, so the preserved legacy JSON comment will never be updated/removed by
the writer and can become stale after edits.

jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[358-409]
jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[321-340]
jablib/src/main/java/org/jabref/logic/exporter/BibDatabaseWriter.java[290-310]

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 parser no longer treats `@Comment{jabref-meta-0.1.0 ...}` as JabRef-managed metadata, so these blocks are preserved as regular comments/epilog and will round-trip unchanged. Because the writer no longer generates/updates these JSON blocks, they can become stale and contradict the actual metadata JabRef writes.

### Issue Context
This PR is a revert of JSON save-actions metadata support. However, files produced while that support existed may still contain `jabref-meta-0.1.0` JSON metadata comments.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[358-409]
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[321-340]

### Suggested change
In `parseJabRefComment`, add a legacy/cleanup branch that detects JSON metadata comments (e.g., prefix `jabref-meta-0.1.0`) and calls `dumpTextReadSoFarToString()` (optionally also add a warning) so these comments are not preserved in `commentsBeforeEntry`/epilog when rewriting files.

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


Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines 782 to 784
FieldFormatterCleanupActions saveActions = new FieldFormatterCleanupActions(true,
List.of(
Arrays.asList(
new FieldFormatterCleanup(StandardField.TITLE, new LowerCaseFormatter()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. arrays.aslist used in test 📘 Rule violation ⚙ Maintainability

The changed test uses Arrays.asList(...) instead of the modern immutable List.of(...),
regressing Java 25+ idioms and readability. This increases mutability risk and inconsistency with
project conventions.
Agent Prompt
## Issue description
The test now uses `Arrays.asList(...)` where modern JabRef style prefers `List.of(...)`.

## Issue Context
The compliance checklist requires using modern Java idioms (Java 25+), including `List.of` over legacy list factories.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[782-786]

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

Comment on lines +792 to +795
assertEquals("@Comment{jabref-meta: saveActions:enabled;"
+ OS.NEWLINE
+ "title[lower_case]" + OS.NEWLINE
+ "journal[title_case]" + OS.NEWLINE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Multiline string built via + 📘 Rule violation ⚙ Maintainability

The changed assertion builds a multi-line expected value via string concatenation instead of a text
block, reducing readability and maintainability. This conflicts with the modern Java idioms
requirement.
Agent Prompt
## Issue description
A multi-line expected string is constructed using chained `+` concatenation with `OS.NEWLINE` instead of using a Java text block.

## Issue Context
The compliance checklist calls out text blocks as the preferred modern Java construct for multi-line constants.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/exporter/BibDatabaseWriterTest.java[792-798]

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

/// Tests for reading whole bib files can be found at {@link org.jabref.logic.importer.fileformat.BibtexImporterTest}
///
/// Tests cannot be executed concurrently, because Localization is used at [BibtexParser#parseAndAddEntry(String)]
/// Tests cannot be executed concurrently, because Localization is used at {@link BibtexParser#parseAndAddEntry(String)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. {@link} used in /// 📘 Rule violation ⚙ Maintainability

A changed Markdown Javadoc (///) comment uses a Javadoc inline tag {@link ...} instead of
Markdown-style linking. This violates the documentation style requirement and reduces consistency.
Agent Prompt
## Issue description
The updated `///` documentation comment uses a Javadoc inline tag (`{@link ...}`) rather than Markdown Javadoc style.

## Issue Context
Project policy requires Markdown Javadoc (`///`) to use Markdown conventions and avoid Javadoc inline tags.

## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java[80-83]

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

} catch (ParseException ex) {
parserResult.addException(new ParserResult.Range(startLine, startColumn, line, column), ex);
}
} else if (comment.startsWith(MetaData.META_FLAG_V1)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

4. Stale json metadata persisted 🐞 Bug ≡ Correctness

After the revert, BibtexParser no longer recognizes jabref-meta-0.1.0 JSON metadata comments, so
they are kept as ordinary pre-entry text/epilog and will be written back unchanged. Since
BibDatabaseWriter no longer rewrites JSON metadata, these legacy JSON blocks can become stale and
conflict with the canonical jabref-meta: metadata after subsequent saves.
Agent Prompt
### Issue description
The parser no longer treats `@Comment{jabref-meta-0.1.0 ...}` as JabRef-managed metadata, so these blocks are preserved as regular comments/epilog and will round-trip unchanged. Because the writer no longer generates/updates these JSON blocks, they can become stale and contradict the actual metadata JabRef writes.

### Issue Context
This PR is a revert of JSON save-actions metadata support. However, files produced while that support existed may still contain `jabref-meta-0.1.0` JSON metadata comments.

### Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[358-409]
- jablib/src/main/java/org/jabref/logic/importer/fileformat/BibtexParser.java[321-340]

### Suggested change
In `parseJabRefComment`, add a legacy/cleanup branch that detects JSON metadata comments (e.g., prefix `jabref-meta-0.1.0`) and calls `dumpTextReadSoFarToString()` (optionally also add a warning) so these comments are not preserved in `commentsBeforeEntry`/epilog when rewriting files.

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

@koppor koppor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We revert and can work on the Qodo-comments later.

I marked as #15159 as freeze to enable re-discovery if we have much time.

@subhramit
subhramit added this pull request to the merge queue Aug 13, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label Aug 13, 2026
Merged via the queue into main with commit f672e04 Aug 13, 2026
95 checks passed
@subhramit
subhramit deleted the revert-15159-fix-for-issue-10371 branch August 13, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants