Conversation
Explicit group membership was lost across restarts because the `groups` field had no Hayagriva key. Sidecar renames now follow "Auto rename files if entry changes", with a library-level override in the General properties tab that also applies to .bib libraries. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
PR Summary by QodoPersist folder groups and add per-library rename control
AI Description
Diagram
High-Level Assessment
Files changed (22)
|
Code Review by Qodo
1.
|
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
| } | ||
|
|
||
| storeKeywordSeparator(metaData); | ||
| autoRenameFilesOnChangeProperty.getValue().ifPresentOrElse(metaData::setAutoRenameFilesOnChange, metaData::clearAutoRenameFilesOnChange); |
There was a problem hiding this comment.
1. Shared libraries keep stale rename rules 🐞 Bug ≡ Correctness
storeSettings clears the optional override, but MetaDataSerializer then omits its key while shared metadata persistence only upserts emitted keys and remote parsing never clears an absent key. When a user switches back to the global preference in a shared library, other clients can indefinitely retain the previous override and continue renaming or not renaming linked files against the current setting.
Agent Prompt
## Issue description
Clearing `autoRenameFilesOnChange` omits it from serialized metadata, but shared SQL persistence does not delete omitted ordinary keys and parsing into an existing `MetaData` does not clear an absent override. Other shared-library clients therefore retain the previous library-specific rename behavior.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/libraryproperties/general/GeneralPropertiesViewModel.java[125-125]
- jablib/src/main/java/org/jabref/logic/shared/DBMSProcessor.java[624-637]
- jablib/src/main/java/org/jabref/logic/importer/util/MetaDataParser.java[164-171]
## Recommended Fix
Treat absence of this optional key as an explicit clear throughout shared synchronization: delete the stored SQL metadata row and notify clients when a complete serialized snapshot omits it, and clear the existing `MetaData` value before or while applying a fetched snapshot that lacks it. Add a shared-database synchronization test covering a transition from an explicit override to the global setting.
ⓘ 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
Not done: pre-existing shared-DB limitation for every clearable key (keyword separator, file directory, git flags), not specific to this override. Separate PR if wanted.
|
Code review by qodo was updated up to the latest commit 030485f |
|
Code review by qodo was updated up to the latest commit 030485f |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
|
Code review by qodo was updated up to the latest commit 1f4c552 |
…ange dialog Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
| AutoRenameFileOnEntryChange.isEnabled(databaseContext, preferences.getFilePreferences()) | ||
| ? FileUtil.createFileNameFromPattern(databaseContext.getDatabase(), entry, preferences.getFilePreferences().getFileNamePattern()) | ||
| : Optional.empty(); |
There was a problem hiding this comment.
9. Long names split folder file pairs 🐞 Bug ≡ Correctness
The directory-library fileNameGenerator returns the raw pattern basename while AutoRenameFileOnEntryChange sends the same entry through LinkedFileHandler, which truncates the PDF filename to 255 characters. When an enabled library override processes a sufficiently long generated name, the PDF moves to the truncated basename before SidecarWriteBack looks for it under the old basename, so only the sidecar receives the untruncated name and the pair is no longer associated.
Agent Prompt
## Issue description
Directory libraries can pass an untruncated pattern result to sidecar write-back while the ordinary linked-file renamer truncates the corresponding PDF filename. Long generated names can therefore split an equally named PDF and sidecar pair.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDirectoryLibraryAction.java[166-169]
- jablib/src/main/java/org/jabref/logic/directorylibrary/SidecarWriteBack.java[191-203]
- jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java[245-255]
## Recommended Fix
Centralize sanitization and length limiting for generated pair basenames, accounting for both PDF and sidecar extensions, and use the same resulting basename in the ordinary linked-file renamer and directory sidecar write-back. Add a directory-library test with a generated name exceeding the filesystem limit and verify that the PDF, sidecar, catalog, and entry link all retain one matching basename.
ⓘ 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: the entry-change renamer now skips folder libraries, so only the write-back renames the pair (one basename, no overlap).
|
Code review by qodo was updated up to the latest commit 176c3db |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
| if ("true".equalsIgnoreCase(value) || "false".equalsIgnoreCase(value)) { | ||
| metaData.setAutoRenameFilesOnChange(Boolean.parseBoolean(value)); | ||
| } else { | ||
| LOGGER.warn("Ignoring invalid value '{}' for {}", value, MetaData.AUTO_RENAME_FILES_ON_CHANGE); |
There was a problem hiding this comment.
8. Invalid overrides preserve prior renames 🐞 Bug ≡ Correctness
MetaDataParser.parse logs an invalid autoRenameFilesOnChange value but leaves the existing override on the supplied MetaData unchanged. When a shared library previously held a valid override and a remote update supplies a malformed value, applyRemoteMetaData parses into that same object and file renaming continues under the stale rule instead of falling back to the global preference.
Agent Prompt
## Issue description
Malformed `autoRenameFilesOnChange` metadata is logged and ignored without clearing an override already present on the supplied `MetaData`. Shared metadata updates parse into the existing object, so the prior rename rule remains active rather than reverting to the global preference.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/importer/util/MetaDataParser.java[164-171]
- jablib/src/test/java/org/jabref/logic/importer/util/MetaDataParserTest.java[117-123]
## Recommended Fix
Call `metaData.clearAutoRenameFilesOnChange()` when the supplied value is invalid, while retaining the warning. Extend the malformed-value test to parse into a `MetaData` instance that already has an override and assert that the override is cleared.
ⓘ 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: malformed value clears the override; test parses into a MetaData that already had one.
|
Code review by qodo was updated up to the latest commit 73ef5ad |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
|
Code review by qodo was updated up to the latest commit 7e49b42 |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt
| public void setAutoRenameFilesOnChange(boolean autoRenameFilesOnChange) { | ||
| this.autoRenameFilesOnChange = autoRenameFilesOnChange; | ||
| postChange(); |
There was a problem hiding this comment.
1. Rename override calls hide intent 📘 Rule violation ⚙ Maintainability
setAutoRenameFilesOnChange(boolean) introduces a public mode-setting API whose call sites express the two alternatives only as true and false. Parser, properties, and test callers now depend on this flag, so later changes must interpret each literal or computed boolean rather than invoke an intention-revealing operation.
Agent Prompt
## Issue description
The new public metadata API accepts a boolean mode flag, making callers encode enable and disable operations as boolean values.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/model/metadata/MetaData.java[189-200]
## Recommended Fix
Replace the boolean setter with separate intention-revealing enable and disable methods while retaining the clear method for global fallback. Update parser, properties, and test call sites to select the appropriate named operation.
ⓘ 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
Not done: boolean setter matches the existing setGitAutoPull(boolean) family; a differing API for one flag would be inconsistent.
| metaData.getAutoRenameFilesOnChange().ifPresent( | ||
| autoRename -> stringyMetaData.put(MetaData.AUTO_RENAME_FILES_ON_CHANGE, List.of(autoRename.toString()))); |
There was a problem hiding this comment.
2. Shared libraries keep cleared overrides 🐞 Bug ≡ Correctness
MetaDataSerializer.getSerializedStringMap omits an unset override, while DBMSProcessor.setSharedMetaData only upserts keys present in that serialized map and deletes absent group-tree keys only. When a user selects “Use global preference” in a shared library, the previous database row remains and is fetched back into reopened and remote clients, restoring the old explicit setting.
Agent Prompt
## Issue description
Clearing the library-specific auto-rename override omits its key from serialization, but shared metadata persistence never deletes that obsolete key. The stale database value is subsequently fetched and reapplied to clients.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/exporter/MetaDataSerializer.java[57-58]
- jablib/src/main/java/org/jabref/logic/shared/DBMSProcessor.java[624-650]
- jablib/src/main/java/org/jabref/logic/shared/DBMSSynchronizer.java[556-565]
## Recommended Fix
When a complete shared metadata snapshot omits the auto-rename override, delete its existing database row and emit the metadata notification. Also ensure applying a fetched snapshot clears the in-memory override when that key is absent, and add shared-database tests covering explicit-to-global transitions.
ⓘ 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
Not done: same pre-existing shared-DB gap for every clearable key (keyword separator, file directory, git flags), see earlier thread. Separate PR if wanted.
|
Code review by qodo was updated up to the latest commit 7b2b3d7 |
|
Code review by qodo was updated up to the latest commit 7b2b3d7 |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…reopen Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
🤖 Adding an entry to a group in a folder library was lost on restart, because explicit membership lives in the
groupsfield and that field had no sidecar key; it is now written as a JabRef extension key. Sidecar and PDF pairs are no longer renamed on every metadata change: renames follow the global "Auto rename files if entry changes" preference, and the General tab of the library properties can override it per library (also for.biblibraries).jabref-contrib-policy:4.2:reviewed:okAnalogies
Like honey, group membership now sticks to the sidecar instead of running off at restart. Like chocolate, the rename knob comes in three squares: global, always, never. And like the moon, the library override only shows its face when someone deliberately turns it.
Steps to test
.biblibrary: file renames follow the library setting instead of the global one.No screenshot: the new row is a plain label + combo box below "Keyword separator" in the General tab.
Related issues and pull requests
Stacked on #760 (base branch
directory-split). Closes NAAI usage
Claude Code (model claude-fable-5-1), AIL4.
AI CHECKLIST.md walkthrough
1. Code self-review
Nullability and control flow
== null/!= nullchecks —@Nullable Booleanread throughOptional.ofNullable.Objects.requireNonNull(...).Optionalconsumed withifPresent/ifPresentOrElse/map/orElse.Exceptions
catch (Exception e).throw new RuntimeException(...)/IllegalStateException(...).Style and idioms
HayagrivaMapping.toBibEntryand the existing synchronizer fixtures, no hand-built entries beyond the existing pattern.Optionalchains, method references.///).User-facing text
Use global preference (%0),Yes,No; existingAuto rename files if entry changesreused.%0placeholder for the current global value.Security
Tests
HayagrivaEntryWriterTest.mergeWritesGroupMembershipAndReadsItBack,MetaDataSerializerTest.serializeAutoRenameFilesOnChangeOverride,MetaDataParserTest.parsesAutoRenameFilesOnChangeOverride;DirectoryLibrarySynchronizerTest.userGroupsFromMirrorMetadataAreRestoredAtOpenextended to membership across a restart (fails without the fix, checked by mutation).@DisplayName,@TempDir.2. Verification commands
./gradlew :jablib:test --tests '*HayagrivaEntryWriterTest' --tests '*MetaDataSerializerTest' --tests '*MetaDataParserTest' --tests '*DirectoryLibrarySynchronizerTest' --tests '*LocalizationConsistencyTest'./gradlew :jabgui:compileJava :jablib:checkstyleMain :jablib:checkstyleTest :jabgui:checkstyleMain./gradlew modernizer— part of CI.:rewriteRun— part of CI../gradlew traceRequirements3. Documentation
CHANGELOG.mdentry for the library override (visible in.biblibraries too); the sidecar fix itself is part of the unreleased feature.req~directory-library.pattern-renamebumped to~2(auto-rename opt-in), markers updated.4. Pull request
.github/PULL_REQUEST_TEMPLATE.md, every section filled.--body-file.TODOplaceholder inCHANGELOG.md.Checklist
CHANGELOG.mddescribing the change from the user's point of view (if the change is visible to the user)🤖 Generated with Claude Code
https://claude.ai/code/session_01H7bhV1r2H2XZvjHrYQkiJt