Conversation
External changes to a library file are merged into the open library without asking; the review dialog only appears for an item that was changed differently in memory and on disk. The autosave preference is relabeled accordingly, since both directions together keep library and file the same. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GWmXyN3GUNhfF9fSo1UNrt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GWmXyN3GUNhfF9fSo1UNrt
PR Summary by QodoSynchronize local libraries with external file changes
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GWmXyN3GUNhfF9fSo1UNrt
Code Review by Qodo
1. Queued scans survive synchronization
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| BackgroundTask.wrap(() -> scanner.scanForChanges(scannedBaseline)) | ||
| .onSuccess(triage -> synchronize(scannedBaseline, triage)) | ||
| .onFailure(e -> LOGGER.error("Error while synchronizing with the library file", e)) |
There was a problem hiding this comment.
5. Concurrent edits get overwritten 🐞 Bug ≡ Correctness
A change classified as disk-only on the background thread is applied later without revalidating the live database. If the user edits the same field before the success callback runs, the accepted EntryChange silently replaces that newer edit with the scanned disk value.
Agent Prompt
## Issue description
Automatic synchronization applies background-scan results without checking for in-memory edits made after triage, allowing newer user edits to be overwritten.
## Issue Context
The diff retains references to live entries, while classification and application occur at different times and on different threads.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[209-245]
- jabgui/src/main/java/org/jabref/gui/collab/ChangeScanner.java[49-52]
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[85-139]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in c7bb88b: only the parse and the two-way diff run in the background; the classification against the baseline now happens on the FX thread immediately before the accepted changes are applied, so an edit made in the meantime is seen as an in-memory change and either merged field by field or sent to review instead of being overwritten.
| .filter(base -> remote.getCitationKey() | ||
| .map(key -> base.getCitationKey().filter(key::equals).isPresent()) | ||
| .orElseGet(() -> sameContent(base, remote))) | ||
| .findFirst(); |
There was a problem hiding this comment.
6. Citation renames remain unpaired 🐞 Bug ≡ Correctness
pairSplitEntries cannot pair a delete/add caused by a disk-side citation-key rename because find requires citation-key equality whenever the added entry has a key. Such a rename can consequently be applied as a separate deletion and insertion, losing entry identity or creating a duplicate while the original deletion awaits review.
Agent Prompt
## Issue description
Split entry changes caused by citation-key renames cannot be paired because lookup requires the renamed key to equal the baseline key.
## Issue Context
The two-way diff may emit an EntryDelete and EntryAdd when similarity is too low. Pairing must identify the common baseline entry even when its citation key changed.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[166-191]
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[239-245]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in c7bb88b: when no baseline entry carries the added entry's citation key, the lookup falls back to matching the remaining content, so a key renamed on disk is paired with its in-memory entry and applied as one change. Test citationKeyChangedOnDiskIsMergedIntoTheSameEntry added.
| case MetadataChange metadataChange -> { | ||
| metaDataSide = sideOf(metaData, serialize(local.getMetaData(), citationKeyPatterns), serialize(metadataChange.getMetaDataDiff().getNewMetaData(), citationKeyPatterns), Objects::equals); | ||
| yield metaDataSide; |
There was a problem hiding this comment.
7. Encoding changes are discarded 🐞 Bug ≡ Correctness
Metadata triage compares serialized maps that omit MetaData.encoding, although MetaDataDiff detects encoding changes. An encoding-only disk change is therefore classified as memory-only and silently dropped instead of being synchronized.
Agent Prompt
## Issue description
Encoding-only external metadata changes disappear because the baseline representation does not include encoding.
## Issue Context
MetaDataDiff compares encoding, but MetaDataSerializer's map omits it, causing both baseline and remote values used by sideOf to appear unchanged.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[112-114]
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[266-268]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
🤖 Generated with Claude Code
Fixed in c7bb88b: the baseline's metadata snapshot now includes the encoding, so an encoding change on disk is classified and applied like any other metadata change. Test encodingChangedOnDiskIsAccepted added.
# Conflicts: # docs/requirements/ux.md
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GWmXyN3GUNhfF9fSo1UNrt
A scan overtaken by a newer file change discards its result, and changes are sorted on the FX thread right before they are applied, so no edit can slip in between. Entries whose citation key changed on disk are paired with their in-memory entry, encoding changes count as metadata changes, and switching synchronization on for an open, unmodified library takes effect immediately. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GWmXyN3GUNhfF9fSo1UNrt
| if (!enabled) { | ||
| baseline = null; |
There was a problem hiding this comment.
1. Queued scans survive synchronization 📘 Rule violation ☼ Reliability
Disabling synchronization or unregistering the monitor does not invalidate scans already queued on the executor, because their callbacks check only the scan generation and retain the previously captured baseline. A stale callback can therefore automatically apply disk changes, update dirty state, and notify listeners after synchronization was disabled, the tab was closed, the monitor was replaced, or the component was disposed, rather than routing the changes through external-change review.
Agent Prompt
## Issue description
Queued synchronization scans remain valid after synchronization is disabled or the monitor is unregistered. Their asynchronous callbacks can consequently mutate the library after the user disabled synchronization, the tab closed, the monitor was replaced, or the component was disposed.
## Issue Context
The existing generation check detects only a newer scan. Disabling synchronization clears the current baseline but leaves the callback's captured baseline intact, while `unregister()` removes listeners without invalidating pending work or recording an inactive lifecycle state.
Invalidate pending scans when synchronization preferences or monitor lifecycle state change, or add an active/current-mode guard that every asynchronous callback checks before performing work. If synchronization has been disabled, do not perform the automatic merge; route the changes through the external-change review path instead.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[198-207]
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[230-257]
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[260-277]
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[301-308]
- jabgui/src/main/java/org/jabref/gui/LibraryTab.java[697-715]
- jabgui/src/main/java/org/jabref/gui/LibraryTab.java[799-810]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (value == null) { | ||
| merged.clearField(field); | ||
| } else { | ||
| merged.setField(field, value); |
There was a problem hiding this comment.
2. Merged entries use mutators 📘 Rule violation ⚙ Maintainability
mergeFields constructs a new BibEntry and populates it with setField, contrary to the required wither-style construction pattern. This makes construction look like an observable edit and deviates from the repository's value-update convention.
Agent Prompt
## Issue description
The newly constructed merged `BibEntry` is populated through `setField` instead of the required `withField` construction pattern.
## Issue Context
Retain the existing handling for removed fields, but use the construction-oriented wither for non-null field values.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[237-247]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| synchronized (database) { | ||
| LibraryBaseline updated = captureBaseline(); | ||
| if (updated != null) { | ||
| updated.keepUnresolved(scannedBaseline, unresolved); | ||
| } | ||
| baseline = updated; |
There was a problem hiding this comment.
3. Invalid scan erases baseline 🐞 Bug ≡ Correctness
An invalid, empty, or unreadable external file produces an empty change list, after which synchronize still replaces the baseline with the current in-memory library. This records unsaved edits as already synchronized, so a later valid disk change to the same field is classified as disk-only and can overwrite the edit without review.
Agent Prompt
## Issue description
Invalid, empty, or unreadable file parses return an empty change list, but synchronization treats that result as successful and advances the baseline. Unsaved in-memory divergence can consequently be forgotten and overwritten by a later disk update.
## Issue Context
Distinguish a successful comparison with no changes from a scan that could not produce a valid database. Only advance the synchronization baseline after a valid comparison.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/ChangeScanner.java[36-63]
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[236-247]
- jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java[261-274]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| case BibTexStringRename stringRename -> { | ||
| boolean oldUntouchedInMemory = Objects.equals(strings.get(stringRename.getOldString().getName()), stringRename.getOldString().getContent()); | ||
| yield oldUntouchedInMemory && !strings.containsKey(stringRename.getNewString().getName()) ? Side.DISK : Side.BOTH; |
There was a problem hiding this comment.
4. String rename collision ignored 🐞 Bug ≡ Correctness
Disk-side string renames are classified as disk-only by checking only whether the destination name existed in the baseline, not whether that name was added in memory afterward. The rename is then automatically accepted despite colliding with the unsaved in-memory string, potentially leaving duplicate names or an unapplied merge reported as successful.
Agent Prompt
## Issue description
A disk rename to a string name independently added in memory is classified as disk-only because rename triage checks only the baseline namespace. Automatic application then collides with the current string.
## Issue Context
Classify a rename as disk-only only when the old string is unchanged in memory and the destination name is absent from both the baseline and the current in-memory database. Add coverage for same-content and different-content destination strings added after the baseline.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/LibraryBaseline.java[125-134]
- jabgui/src/main/java/org/jabref/gui/collab/stringrename/BibTexStringRename.java[28-36]
- jabgui/src/test/java/org/jabref/gui/collab/LibraryBaselineTest.java[202-225]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit c7bb88b |
|
This pull requests was closed without merging. You have been unassigned from the respective issue #8431. In case you closed the PR for yourself, you can re-open it. Please also check After submission of a pull request in CONTRIBUTING.md. |
1 participant
Summary
🤖 Stacked on #11879 (base branch
fix-sync); only the last commits belong to this PR.With "Synchronize local libraries with their files" (the former autosave option) enabled, changes made to the library file by another program are now merged into the open library automatically, including field-level merges of entries also edited in JabRef. The "External changes detected" review is only offered when the same item was changed differently in memory and on disk, or deleted on one side and changed on the other.
Analogies: like honey, external edits now flow in smoothly instead of sticking in a dialog; like chocolate, the merge takes the best of both sides; like the moon, the review still shows up, but only when the two sides truly eclipse each other.
jabref-contrib-policy:4.2:reviewed:okSteps to test
Enable "Synchronize local libraries with their files" in Preferences → General → Saving.
Open a
.bibfile in JabRef and edit it in a text editor: change a field of one entry, add a new entry, save.Observe: the table updates within a few seconds, a "Merged 2 change(s) from the library file" notification appears, no review dialog.
Change a field of an entry in JabRef, then change the same field to a different value in the text editor.
Observe: "External changes detected" with "Review changes", offering the usual merge dialog for that entry only.
Delete an entry in JabRef, then change that entry in the text editor: the review is offered; delete it in the text editor instead: nothing is asked.
Related issues and pull requests
Closes #8431
Stacked on #11879.
User documentation: JabRef/user-documentation#672
AI usage
Claude Code (model claude-fable-5-1), AIL4: implementation, tests, and manual GUI test driven by the AI, directed and reviewed by the author.
AI CHECKLIST.md walkthrough
1. Code self-review
Nullability and control flow
== null/!= nullchecks — absence of an item on one side of the three-way comparison is modelled asnull, as in the Git merge rules being reused.Objects.requireNonNull(...).@NullMarked.Optionalconsumed withifPresent/map/orElseThrow.StringUtil.isBlank(...).Exceptions
catch (Exception e).throw new RuntimeException(...).Style and idioms
BibEntryobjects built with withers.BackgroundTask.User-facing text
!; labels do not end with:.Security
Tests
LibraryBaselineTest).@DisplayName, no caught exceptions.2. Verification commands
:jabgui:test --tests org.jabref.gui.collab.*andLocalizationConsistencyTestpass.checkstyleMain checkstyleTest.modernizer.rewriteRunreports no changes.javadoc.markdownlint-cli2on the changed Markdown.3. Documentation
CHANGELOG.mdentry added.docs/requirements/ux.md.4. Pull request
gh pr create --body-file.TODOplaceholder.Checklist
CHANGELOG.mddescribing the change from the user's point of view (if the change is visible to the user)🤖 Generated with Claude Code