Keep previews responsive while switching large libraries#16289
Conversation
PR Summary by QodoKeep entry previews responsive when switching large libraries
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
github-actions
Bot
added
the
status: changes-required
|
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. |
- Avoid a redundant full-database rescan when number cells re-render while the initial group-count load is still in flight - Log group-count update failures at warn instead of debug - Document why group-count tasks go through schedule() (separate pool, keeps the worker pool free for preview rendering) - Force a group-count reload when the display-group-count preference is re-enabled after a skipped recompute - Confine PreviewViewer.update() to the FX thread so the unsynchronized updateSequence guard is safe when entry observables fire from background threads - Drop the now-unused Collector implementation from TreeCollector and the now-unused GroupTreeNode.isSameGroupAs - Cover the recursive grandchild-collision path in TreeCollectorTest - Move the CHANGELOG entry from Fixed to Changed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NW1qh78rm2gy42vtixbmM4
The OFT requirement template (docs/requirements/index.md, AGENTS.md) specifies a "### <title>" heading; the file's older entries predate it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NW1qh78rm2gy42vtixbmM4
The skipped recompute already invalidated the cache; also clear the matched-entries set so rebinding the number cell never briefly shows the outdated count, and cover the disable -> refresh -> re-enable sequence with a test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NW1qh78rm2gy42vtixbmM4
Siedlerchr
added
status: ready-for-review
|
Improves #15962 |
koppor
left a comment
koppor
left a comment
There was a problem hiding this comment.
Not sure what's happening here, but we need to test "in the wild"; thus approving merge to main.
github-actions
Bot
added
the
status: to-be-merged
3 participants
Related issues and pull requests
Meta-issue: #8906
Depends on JabRef/html-to-node#16.
PR Description
This improves switching between large libraries: automatic group paths are merged with temporary indexes, group counts are loaded only when their cells become visible, and group-count work no longer occupies the preview worker pool. Preview updates are rendered atomically and discard stale background results, keeping the selected entry responsive while groups are recalculated.
The group calcuation is now based on a https://de.wikipedia.org/wiki/Trie
This is a draft until html-to-node PR #16 is merged and its
0.2.0release is published.Steps to test
./gradlew -PuseMavenLocal :jabgui:run.The contributor manually performed this scenario with a generated large library. A static screenshot would not demonstrate this performance behavior.
AI usage
OpenAI Codex (model GPT-5) was used for assistance. The contributor reviewed, understood, manually tested, and takes responsibility for this change.
AI CHECKLIST.md walkthrough
1. Code self-review
Nullability and control flow
== null/!= nullchecks — JSpecify annotations (@NullMarked,@Nullable,@NonNull) used instead.Objects.requireNonNull(...)— nullability expressed via JSpecify annotations.@NullMarked(org.jspecify.annotations.NullMarked).Optionalconsumed withifPresent/ifPresentOrElse/map/orElseThrow— neverorElse(unusedValue)nor anisPresent()+get()block.StringUtil.isBlank(...)used instead ofs == null || s.isBlank().Exceptions
catch (Exception e)— only specific exceptions are caught.throw new RuntimeException(...)/IllegalStateException(...)— these tear down the whole application.LOGGER.info("...", e)), not concatenated into the message string.Style and idioms
BibEntryobjects built with withers (withField, notsetField).List.of()/Map.of()/Set.of(),Path.of(),SequencedCollection/SequencedSet, text blocks.Pattern.compile(...)constant, notString.matches(...).org.jabref.logic.util.BackgroundTask, notnew Thread().///) uses Markdown syntax, not JavaDoc inline tags:`code`instead of{@code},[ClassName]instead of{@link}.User-facing text
Localization.langin Java,%prefix in FXML).!; labels do not end with:."...: %0"), not string concatenation.Security
text/htmlresponse — including exception/error messages, not just the success body (XSS).Tests
org.jabref.model/org.jabref.logichave added or updated tests.assertEquals), use plain JUnit asserts (not AssertJ), have no@DisplayName, do not catch exceptions (let them propagate so JUnit reports setup/teardown failures directly), and use@TempDirinstead of manual temp directories.2. Verification commands
Run in this order — cheapest first. Each must pass.
./gradlew :jablib:check(or./gradlew checkfor all modules). The local run completed test execution but Gradle failed collecting a transient test-result file; CI will perform this check../gradlew checkstyleMain checkstyleTest checkstyleJmh../gradlew modernizer../gradlew --no-configuration-cache :rewriteDryRunreports no changes (run./gradlew rewriteRunto fix)../gradlew javadoc.npx markdownlint-cli2 "docs/**/*.md" "*.md"(only if Markdown changed).rewriteRun:docker run -v $(pwd):/github/workspace ghcr.io/leventebajczi/intellij-format:master "*.java" "" ".idea/codeStyles/Project.xml".3. Documentation
CHANGELOG.mdentry added if the change is visible to the user (end-user wording, no extra blank lines). Link the issue if one exists; link the PR only when no issue exists. UseTODOas the placeholder when neither is known yet — never a fake number.TODO(nocloses/fixesfor merely-similar issues).docs/requirements/<area>.mdif the change is a new feature or significant bug fix (skip for refactors, minor fixes, and internal changes).docs/updated if behavior or architecture changed.4. Pull request
.github/PULL_REQUEST_TEMPLATE.md, every section filled.[x],[ ], or[/].gh pr create --body-file <file>(not--body).CHANGELOG.mdused aTODOplaceholder (no issue confidently identified yet — an existing issue link always stays), it was replaced with the real PR-number link after PR creation, then committed and pushed. If an issue is identified or created later, the link is switched to the issue.Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)