refactor(books): add background job book submissions - #2189
Open
alexhb1 wants to merge 2 commits into
Open
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe frontend adds book cover and shelf command services, shelf-definition queries, pending shelf overlays, and metadata refresh reconciliation. AppComponent forwards parsed batch-progress websocket notifications to the metadata refresh service. ChangesBook command data layer
Metadata refresh reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/app.component.ts`:
- Around line 145-149: Update setupWebSocketSubscriptions to pipe the metadata
batch progress watch through takeUntilDestroyed(this.destroyRef), and remove
this subscription from the subscriptions cleanup collection. Apply the same
lifecycle pattern to the other subscriptions in setupWebSocketSubscriptions,
eliminating their manual subscriptions-list management while preserving existing
handlers.
In `@frontend/src/app/features/book/data/book-background-submission.service.ts`:
- Around line 22-31: Update requestCoverChanges to handle unsupported
variables.kind values with a default branch that asserts or throws, ensuring
every switch path returns or fails while preserving the existing regenerate and
generate behavior.
In `@frontend/src/app/features/book/data/book-command-pending-state.ts`:
- Around line 59-79: Add direct spec coverage for
injectPendingBookShelfMembership by seeding overlapping assign/unassign
mutations for the same book and shelf in a QueryClient mutation cache. Assert
the resulting PendingShelfMembership reflects last-write-wins, including removal
from the earlier operation’s set and presence in the later operation’s set.
In
`@frontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.ts`:
- Around line 61-70: Replace the hardcoded 5,000 and 10,000 millisecond timer
advances in the metadata refresh submission tests with the exported/shared
quiet-period duration used by the service’s ERROR_QUIET_PERIOD_MS, while
retaining fake-timer control and existing assertions.
In
`@frontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts`:
- Around line 32-36: Update the private refresh method to handle failures from
lastValueFrom(this.http.post(...)) by adding explicit HTTP error normalization
through catchError or a promise catch. Ensure refreshMetadata receives a
normalized failure result or error rather than allowing an unhandled rejection,
while preserving the existing refresh request payload and successful completion
behavior.
- Around line 39-51: The handleBatchProgress method currently uses a single
timer, allowing one task’s progress event to cancel another task’s delayed
reconciliation. Replace the shared errorReconcileTimer handling with a Map keyed
by progress.taskId, clearing or replacing only the incoming task’s timer while
preserving reconciliation behavior for terminal statuses. Add coverage for task
A emitting ERROR followed by task B emitting IN_PROGRESS, ensuring task A’s
delayed reconciliation still runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 98f47461-7472-405d-b170-04cd7089f025
📒 Files selected for processing (18)
frontend/src/app/app.component.spec.tsfrontend/src/app/app.component.tsfrontend/src/app/features/book/data/book-background-submission-keys.tsfrontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
grimmory-tools/grimmory-docs(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Test Suite / Frontend Tests
- GitHub Check: Test Suite / Backend Tests
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ CodeRabbit configuration file
**/*: This project is being developed using current and future-facing technologies:
- Java 25 with --enable-preview (preview features are INTENTIONAL and encouraged)
- Spring Boot 4 (latest major version, check APIs accordingly)
- Jackson 3 (new package: tools.jackson.* instead of com.fasterxml.jackson.*)
- Hibernate 7.3.x (Jakarta Persistence 3.2, new APIs; avoid deprecated Hibernate 5/6 patterns)
- Angular 21 (signals-based reactivity, no NgModules unless legacy)
Grimmory Internal Tools
- epub4j and pdfium4j are our own internal tools developed by the Grimmory team.
- Always verify behavior and API changes against the upstream repositories:
- If you encounter issues with these libraries, check if a fix exists in the upstream grimmory-tools organization.
Metadata Standards and Compliance
- For all metadata writing and parsing logic, double-check against Dublin Core and ANSI standards to ensure perfect official compliance.
- We strictly follow the widespread and official XML-compliant methods for EPUB2, EPUB3, CBX, and PDF formats.
General Java and Spring rules
- ALWAYS prefer modern, idiomatic Java 25 constructs over legacy patterns.
- Preview features (--enable-preview) are enabled and intentional; do NOT flag them as risky unless there is a concrete runtime issue.
- Prefer: records, sealed classes/interfaces, pattern matching (switch expressions, instanceof), structured concurrency (StructuredTaskScope), scoped values, string templates, unnamed patterns/variables.
- Prefer virtual threads (Thread.ofVirtual(), Executors.newVirtualThreadPerTaskExecutor()) over platform threads for I/O-bound work.
- Prefer the new Sequenced Collections API (SequencedCollection, SequencedMap) where applicable.
- Prefer
varfor local variables when the type is obvious from context.- Use stream().toList() instead of stream().collect(Collectors.toList()) for imm...
Files:
frontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/app.component.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.tsfrontend/src/app/features/book/data/book-background-submission-keys.ts
**/*.spec.ts
⚙️ CodeRabbit configuration file
**/*.spec.ts: Angular 21 test review:
- Flag tests with no expect() calls.
- Flag hardcoded async timeouts; prefer fakeAsync/tick or signal-based testing.
- Flag missing fixture.detectChanges() after state mutations.
Files:
frontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.ts
**/*.component.ts
⚙️ CodeRabbit configuration file
**/*.component.ts: Angular 21 component review:
- Must be standalone (standalone: true); flag NgModule-based declarations.
- Prefer OnPush change detection strategy.
- Prefer inject() over constructor DI.
- Localization: use Transloco for all UI strings.
- Prefer Angular Signals (signal, computed, effect) for local/reactive state.
- Flag missing trackBy / track expression in list rendering.
- Prefer
@if/@for/@switchover *ngIf / *ngFor / *ngSwitch structural directives.
Files:
frontend/src/app/app.component.ts
**/*.service.ts
⚙️ CodeRabbit configuration file
**/*.service.ts: Angular 21 service review:
- Prefer providedIn: 'root' unless scope is intentionally limited.
- Prefer inject() over constructor DI.
- Prefer Signals or lightweight RxJS (takeUntilDestroyed) over manual subscription management.
- Flag unsubscribed Observables (missing takeUntilDestroyed or explicit unsubscribe).
- Prefer typed HttpClient responses with explicit error handling (catchError).
- Flag any state mutation outside a defined signal or BehaviorSubject.
Files:
frontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts
🧠 Learnings (5)
📚 Learning: 2026-04-05T21:16:01.715Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.
Applied to files:
frontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/app.component.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.tsfrontend/src/app/features/book/data/book-background-submission-keys.ts
📚 Learning: 2026-04-07T09:28:09.587Z
Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...
Applied to files:
frontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/app.component.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.tsfrontend/src/app/features/book/data/book-background-submission-keys.ts
📚 Learning: 2026-04-11T03:55:57.229Z
Learnt from: zachyale
Repo: grimmory-tools/grimmory PR: 439
File: frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts:178-196
Timestamp: 2026-04-11T03:55:57.229Z
Learning: In this Angular frontend (frontend/src/app/), prefer the team’s reactive i18n “signal/computed” pattern: (1) For individual reactive translated strings, use `translateSignal()` from `jsverse/transloco`. (2) For option/label arrays that must update on language switch, create a single `activeLang` signal with `toSignal(t.langChanges$, { initialValue: t.getActiveLang() })`, then derive the arrays as `computed()` signals that read `activeLang()`. This should avoid manual `langChanges$` subscriptions and any `ngOnDestroy` subscription cleanup; prefer this over subscribing in `ngOnInit` when implementing reactive localization.
Applied to files:
frontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/app.component.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.tsfrontend/src/app/features/book/data/book-background-submission-keys.ts
📚 Learning: 2026-05-18T14:54:39.422Z
Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 1379
File: frontend/src/assets/styles/tailwind.css:3-4
Timestamp: 2026-05-18T14:54:39.422Z
Learning: In the grimmory-tools/grimmory repository, Biome is not used for linting/formatting (no `biome.json` and no Biome dependency in `package.json`). During code reviews, do not raise Biome-related issues or recommend adding/changing `biome.json`/Biome dependencies for formatting or linting in this project.
Applied to files:
frontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/app.component.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.tsfrontend/src/app/features/book/data/book-background-submission-keys.ts
📚 Learning: 2026-06-30T01:30:43.644Z
Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 1881
File: frontend/src/app/shared/components/icon-picker/icon-picker-component.ts:116-119
Timestamp: 2026-06-30T01:30:43.644Z
Learning: In the Grimmory Angular frontend (Angular 21), do not flag `[(ngModel)]` two-way bindings that are bound directly to a `WritableSignal`. Angular supports two-way binding to writable signals, so usages like `[(ngModel)]="svgSearchText"` should be treated as valid and should not be considered an incorrect overwrite of the signal with a plain value. Only flag if the binding target is not a `WritableSignal`.
Applied to files:
frontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/app.component.tsfrontend/src/app/app.component.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.tsfrontend/src/app/features/book/data/book-background-submission-keys.ts
🔇 Additional comments (19)
frontend/src/app/features/metadata/data/metadata-refresh-submission-keys.ts (1)
1-3: LGTM!frontend/src/app/features/metadata/data/metadata-refresh-submission.models.ts (1)
1-3: LGTM!frontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts (1)
25-29: LGTM!Also applies to: 62-67
frontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.ts (1)
17-53: LGTM!Also applies to: 65-75
frontend/src/app/app.component.ts (1)
23-23: LGTM!Also applies to: 48-48
frontend/src/app/app.component.spec.ts (1)
11-11: LGTM!Also applies to: 52-52, 94-94, 127-127, 223-225
frontend/src/app/features/book/data/book-shelf-command-keys.ts (1)
1-9: LGTM!frontend/src/app/features/book/data/book-shelf-command.models.ts (1)
1-9: LGTM!frontend/src/app/features/book/data/shelf-definition.models.ts (1)
1-11: LGTM!frontend/src/app/features/book/data/shelf-definition-query-keys.ts (1)
1-4: LGTM!frontend/src/app/features/book/data/shelf-definition-query.service.ts (1)
1-38: LGTM!frontend/src/app/features/book/data/shelf-definition-query-cache.ts (1)
1-9: LGTM!frontend/src/app/features/book/data/book-command-pending-state.ts (1)
12-18: LGTM!Also applies to: 126-142
frontend/src/app/features/book/data/book-shelf-command.service.ts (1)
1-66: LGTM!frontend/src/app/features/book/data/book-command-pending-state.spec.ts (1)
3-3: LGTM!Also applies to: 12-15, 63-80
frontend/src/app/features/book/data/book-background-submission-keys.ts (1)
1-5: LGTM!frontend/src/app/features/book/data/book-background-submission.models.ts (1)
1-4: LGTM!frontend/src/app/features/book/data/book-background-submission.service.ts (2)
1-8: LGTM!Also applies to: 10-20
33-35: 🩺 Stability & AvailabilityVerify mutation error handling.
postBookIdsrejects directly with Angular HTTP failures vialastValueFrom, and the only visiblechangeCoverspath uses TanStack mutation without anonErrorhandler in the reviewed references. Add an explicit handler for the mutation, or map HTTP failures to the app’s error format before rethrowing.
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/app.component.ts`:
- Around line 145-149: Update setupWebSocketSubscriptions to pipe the metadata
batch progress watch through takeUntilDestroyed(this.destroyRef), and remove
this subscription from the subscriptions cleanup collection. Apply the same
lifecycle pattern to the other subscriptions in setupWebSocketSubscriptions,
eliminating their manual subscriptions-list management while preserving existing
handlers.
In `@frontend/src/app/features/book/data/book-background-submission.service.ts`:
- Around line 22-31: Update requestCoverChanges to handle unsupported
variables.kind values with a default branch that asserts or throws, ensuring
every switch path returns or fails while preserving the existing regenerate and
generate behavior.
In `@frontend/src/app/features/book/data/book-command-pending-state.ts`:
- Around line 59-79: Add direct spec coverage for
injectPendingBookShelfMembership by seeding overlapping assign/unassign
mutations for the same book and shelf in a QueryClient mutation cache. Assert
the resulting PendingShelfMembership reflects last-write-wins, including removal
from the earlier operation’s set and presence in the later operation’s set.
In
`@frontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.ts`:
- Around line 61-70: Replace the hardcoded 5,000 and 10,000 millisecond timer
advances in the metadata refresh submission tests with the exported/shared
quiet-period duration used by the service’s ERROR_QUIET_PERIOD_MS, while
retaining fake-timer control and existing assertions.
In
`@frontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts`:
- Around line 32-36: Update the private refresh method to handle failures from
lastValueFrom(this.http.post(...)) by adding explicit HTTP error normalization
through catchError or a promise catch. Ensure refreshMetadata receives a
normalized failure result or error rather than allowing an unhandled rejection,
while preserving the existing refresh request payload and successful completion
behavior.
- Around line 39-51: The handleBatchProgress method currently uses a single
timer, allowing one task’s progress event to cancel another task’s delayed
reconciliation. Replace the shared errorReconcileTimer handling with a Map keyed
by progress.taskId, clearing or replacing only the incoming task’s timer while
preserving reconciliation behavior for terminal statuses. Add coverage for task
A emitting ERROR followed by task B emitting IN_PROGRESS, ensuring task A’s
delayed reconciliation still runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 98f47461-7472-405d-b170-04cd7089f025
📒 Files selected for processing (18)
frontend/src/app/app.component.spec.tsfrontend/src/app/app.component.tsfrontend/src/app/features/book/data/book-background-submission-keys.tsfrontend/src/app/features/book/data/book-background-submission.models.tsfrontend/src/app/features/book/data/book-background-submission.service.tsfrontend/src/app/features/book/data/book-command-pending-state.spec.tsfrontend/src/app/features/book/data/book-command-pending-state.tsfrontend/src/app/features/book/data/book-shelf-command-keys.tsfrontend/src/app/features/book/data/book-shelf-command.models.tsfrontend/src/app/features/book/data/book-shelf-command.service.tsfrontend/src/app/features/book/data/shelf-definition-query-cache.tsfrontend/src/app/features/book/data/shelf-definition-query-keys.tsfrontend/src/app/features/book/data/shelf-definition-query.service.tsfrontend/src/app/features/book/data/shelf-definition.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission-keys.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.models.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.tsfrontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
grimmory-tools/grimmory-docs(manual)
📜 Review details
🔇 Additional comments (19)
frontend/src/app/features/metadata/data/metadata-refresh-submission-keys.ts (1)
1-3: LGTM!frontend/src/app/features/metadata/data/metadata-refresh-submission.models.ts (1)
1-3: LGTM!frontend/src/app/features/metadata/data/metadata-refresh-submission.service.ts (1)
25-29: LGTM!Also applies to: 62-67
frontend/src/app/features/metadata/data/metadata-refresh-submission.service.spec.ts (1)
17-53: LGTM!Also applies to: 65-75
frontend/src/app/app.component.ts (1)
23-23: LGTM!Also applies to: 48-48
frontend/src/app/app.component.spec.ts (1)
11-11: LGTM!Also applies to: 52-52, 94-94, 127-127, 223-225
frontend/src/app/features/book/data/book-shelf-command-keys.ts (1)
1-9: LGTM!frontend/src/app/features/book/data/book-shelf-command.models.ts (1)
1-9: LGTM!frontend/src/app/features/book/data/shelf-definition.models.ts (1)
1-11: LGTM!frontend/src/app/features/book/data/shelf-definition-query-keys.ts (1)
1-4: LGTM!frontend/src/app/features/book/data/shelf-definition-query.service.ts (1)
1-38: LGTM!frontend/src/app/features/book/data/shelf-definition-query-cache.ts (1)
1-9: LGTM!frontend/src/app/features/book/data/book-command-pending-state.ts (1)
12-18: LGTM!Also applies to: 126-142
frontend/src/app/features/book/data/book-shelf-command.service.ts (1)
1-66: LGTM!frontend/src/app/features/book/data/book-command-pending-state.spec.ts (1)
3-3: LGTM!Also applies to: 12-15, 63-80
frontend/src/app/features/book/data/book-background-submission-keys.ts (1)
1-5: LGTM!frontend/src/app/features/book/data/book-background-submission.models.ts (1)
1-4: LGTM!frontend/src/app/features/book/data/book-background-submission.service.ts (2)
1-8: LGTM!Also applies to: 10-20
33-35: 🩺 Stability & AvailabilityVerify mutation error handling.
postBookIdsrejects directly with Angular HTTP failures vialastValueFrom, and the only visiblechangeCoverspath uses TanStack mutation without anonErrorhandler in the reviewed references. Add an explicit handler for the mutation, or map HTTP failures to the app’s error format before rethrowing.
🛑 Comments failed to post (1)
frontend/src/app/features/book/data/book-command-pending-state.ts (1)
59-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add direct test coverage for the conflict-resolution logic.
injectPendingBookShelfMembershipimplements last-write-wins conflict resolution across concurrent pending mutations for the same book and shelf. The accompanying spec file only exercisesoverlayShelfIdsandoverlayPendingBookState; the merge logic ininjectPendingBookShelfMembershipitself (lines 62-73) has no direct test.Add a test that seeds two overlapping pending mutations (for example, assign shelf 5, then unassign shelf 5) into a
QueryClient's mutation cache and asserts the resultingPendingShelfMembershipreflects the later operation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/features/book/data/book-command-pending-state.ts` around lines 59 - 79, Add direct spec coverage for injectPendingBookShelfMembership by seeding overlapping assign/unassign mutations for the same book and shelf in a QueryClient mutation cache. Assert the resulting PendingShelfMembership reflects last-write-wins, including removal from the earlier operation’s set and presence in the later operation’s set.
imnotjames
reviewed
Aug 1, 2026
7 tasks
imnotjames
reviewed
Aug 1, 2026
imnotjames
approved these changes
Aug 1, 2026
7 tasks
alexhb1
force-pushed
the
epic-browser/07-background-jobs
branch
from
August 8, 2026 15:03
4312ed1 to
c5cfd47
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
New background tasks layer for the new browser's bulk actions, specifically cover regeneration and metadata refreshing.
Linked Issue
Fixes #2187
Changes
Manual Testing Steps
N/A
Screenshots (Optional)
N/A
Additional Context (Optional)
N/A
AI Disclosure
N/A
Checklist
just ui checkandjust api check.Summary by CodeRabbit
New Features
Bug Fixes