Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@booklore-api/src/main/java/org/booklore/model/dto/CreateBookMarkRequest.java`:
- Around line 24-25: The CreateBookMarkRequest DTO allows pageNumber values <=
0; add a validation constraint so any non-null pageNumber must be >=1 to match
UpdateBookMarkRequest and controller `@Valid` handling. On the pageNumber field in
class CreateBookMarkRequest add the appropriate validation annotation (e.g.,
`@Min`(1)) and import the corresponding javax.validation constraint so POST
/bookmarks rejects 0 or negative pages while still allowing null for non-PDF
bookmarks.

In
`@booklore-api/src/main/resources/db/migration/V136__Add_page_number_to_book_marks.sql`:
- Line 1: The migration currently only adds page_number to book_marks which
leaves a TOCTOU race; modify the migration to also create a composite UNIQUE
index enforcing per-book page uniqueness (e.g. a unique index on (book_id,
page_number) for the book_marks table) and use a partial index that applies only
when page_number IS NOT NULL so existing/null bookmarks aren’t blocked; give the
index a clear name (e.g. unique_book_marks_book_id_page_number) and add that
CREATE UNIQUE INDEX step to the same migration after adding the page_number
column.

In `@frontend/src/app/features/book/model/book.model.ts`:
- Around line 304-307: Update the PdfViewerSetting type to use 'none' instead of
'off' and normalize legacy 'off' values when reading saved prefs; in the
documentOpened$ handler (after the existing setZoomLevel call) apply the saved
spread by calling this.embedPdfBook.setSpreadMode(this.spread) and set
this.spreadMode = this.spread as 'none'|'odd'|'even'; in cycleSpreadMode()
ensure you update both this.spreadMode and this.spread, call
this.embedPdfBook.setSpreadMode(this.spreadMode), and then call
this.updateViewerSetting() so the new spread persists.

In
`@frontend/src/app/features/readers/pdf-reader/components/pdf-sidebar.component.html`:
- Around line 7-8: The close button uses t('title') (the sidebar title) for its
aria-label, so update the button with class "sidebar-close-btn" to use a
dedicated action label (e.g., t('actions.close') or a new i18n key like
readerPdf.sidebar.close) instead of t('title'); keep the (click)="closed.emit()"
behavior and ensure the new key is added to the translation files so screen
readers announce the action as "close" rather than the panel title.

In
`@frontend/src/app/features/readers/pdf-reader/components/pdf-sidebar.component.scss`:
- Around line 195-200: The delete button (.delete-btn) is only revealed on
parent :hover, which hides it from keyboard and touch users; update the
stylesheet so the button is also shown when focused and when the parent gains
focus (use :focus and :focus-within on the same selector that currently uses
:hover), and also ensure .delete-btn itself becomes visible when it receives
focus (add a rule for .delete-btn:focus { opacity: 1 } or equivalent); apply
these same changes to the other block affecting .delete-btn (the similar rules
around the second occurrence) so keyboard and touch users can discover and
activate the control.

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.html`:
- Around line 211-215: Add a persistent accessible name to the search input so
assistive tech still has a label after the placeholder disappears: update the
input element that uses [(ngModel)]="searchQuery" and (input)="onSearchInput()"
to include an aria-label bound to the same translation used for the placeholder
(e.g. [attr.aria-label]="'readerPdf.toolbar.searchPlaceholder' | transloco") or
add a visually hidden <label> that uses that translation; ensure you leave
existing handlers onSearchInput/onSearchNext/closeSearch unchanged.

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.scss`:
- Around line 680-682: The CSS currently forces ".search-bar" to "display: none"
at mobile sizes while the toolbar search button in pdf-reader.component.html
remains tappable; either restore the search input on small screens or
hide/disable the toolbar action. Fix by updating the mobile CSS rule that
contains ".zoom-menu-wrapper, .search-bar" so it only hides ".zoom-menu-wrapper"
(remove ".search-bar" from that selector) or alternately add a mobile-only rule
to hide/disable the toolbar search button element in the pdf-reader component
(the toolbar search button class/id) so the button is not shown/tappable when
".search-bar" is hidden.

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts`:
- Around line 319-330: The annotationEvent$ handler currently only marks
annotations dirty and triggers save/cache; also refresh the sidebar list so
new/edited annotations appear immediately: inside the subscriber (the block
guarded by this.annotationsLoaded && !this.isImportingAnnotations) after
this.annotationCacheSubject.next(), call the function that rebuilds or refreshes
annotationListItems (e.g. this.buildAnnotationListItems() or
this.refreshAnnotationList()—or emit an existing annotationListRefreshSubject)
so that this.annotationListItems is updated on each annotationEvent; ensure the
call runs inside the same ngZone run block and does not run during import.
- Around line 573-579: Handlers like onSidebarNavigateToPage should not set
this.page before calling onPageChange because onPageChange contains a guard that
will immediately short-circuit; remove the direct assignment (e.g., delete
this.page = pageNumber) and instead call this.onPageChange(pageNumber) (keep
other side effects such as this.sidebarOpen = false and
embedPdfBook.scrollToPage(pageNumber) as needed). Apply the same change to
goToFirstPage, goToPreviousPage, goToLastPage, onSliderChange, and onGoToPage so
that onPageChange is responsible for updating this.page and performing
persistence/progress logic.
- Around line 167-202: The handlers registered inside ngZone.runOutsideAngular
(mouseMoveHandler, clickHandler, keydownHandler) mutate bound UI state but do so
outside Angular's zone; wrap any UI-affecting calls—specifically calls to
showChrome() and startChromeAutoHide() in mouseMoveHandler, the assignment
this.isZoomMenuOpen = false in clickHandler, and any calls that update view
state (e.g., closeReader() is already run inside ngZone for the key handler but
double-check) —inside this.ngZone.run(() => { ... }) so change detection runs
and the view updates reliably.

In
`@frontend/src/app/features/readers/pdf-reader/services/embedpdf-book.service.ts`:
- Around line 311-365: The applyWorkerShims function uses an untyped window cast
and leaves patched globals permanent while also unconditionally firing a
synthetic "ready" message; fix by replacing the any cast with a minimal typed
interface (extend Window with __grimmoryShimsApplied and optional original
constructors/flags), store the original Blob and Worker constructors (e.g.,
OrigBlob/OrigWorker) and any state flags (readySent, wasmError) on that typed
window shim, update PatchedBlob/PatchedWorker to set a wasmError flag when a
wasmError message is observed, and only dispatch the fallback ready after
timeout if neither wasmError nor an actual ready was received; finally restore
the originals and clear flags inside destroy() (reference applyWorkerShims,
PatchedBlob, PatchedWorker, w.__grimmoryShimsApplied, readySent, wasmError,
destroy), and also replace the other casts like this.scroll as any with proper
types.

In
`@frontend/src/app/features/readers/pdf-reader/services/pdf-bookmark.service.ts`:
- Around line 22-33: loadBookmarks() currently clears this.bookmarks only on
successful fetch and lets errors bubble up, leaving UI without a safe
empty-state; modify loadBookmarks to handle failures locally by adding a
catchError on the Observable returned by
bookMarkService.getBookmarksForBook(this.bookId) that clears this.bookmarks and
returns of([]) so subscribers always receive an empty array on error; reference
the loadBookmarks method, this.bookmarks Map, and
bookMarkService.getBookmarksForBook to locate where to add the catchError and
the of([]) fallback.

In `@frontend/src/app/features/readers/pdf-reader/utils/annotation-converter.ts`:
- Around line 44-59: serializeAnnotations and restoreArrayBuffers currently use
an `any` cast to access serialized fields; remove those casts and introduce a
local helper interface (e.g., EncodedAnnotation or SerializableAnnotation) that
models the shape added during serialization (ctx?: { data: string;
_dataEncoding?: 'base64' } along with original AnnotationTransferItem fields),
then use that type when mapping/inspecting items in serializeAnnotations and
when restoring in restoreArrayBuffers so you can safely read ctx.data and
ctx._dataEncoding without `any`; update logic to convert ArrayBuffer->base64 and
base64->ArrayBuffer using the helper type to satisfy the eslint rule against
`any` while preserving the existing serialization behavior.
- Around line 68-76: The legacy annotation type mapping in the switch inside
convertLegacyAnnotation (the switch handling numeric legacy types) incorrectly
treats highlights as case 16 and thus drops type 9 annotations; change the
switch so that highlights are handled for the pdf.js value 9
(AnnotationEditorType.HIGHLIGHT = 9) instead of 16, ensure type 9 returns a
proper Highlight import (not null) so highlights aren’t skipped, and update the
nearby comment that lists pdf.js/ngx-extended-pdf-viewer numeric values to
correctly state HIGHLIGHT = 9 (and remove or correct the incorrect “16” claim).

---

Outside diff comments:
In `@booklore-api/src/main/java/org/booklore/service/book/BookMarkService.java`:
- Around line 81-89: updateBookmark currently checks CFI uniqueness but skips
pageNumber, so changing pageNumber can create duplicates; before calling
applyUpdates in BookMarkService.updateBookmark, if request.getPageNumber() !=
null call the same uniqueness check used for CFIs (i.e.
validateNoDuplicateBookmark) passing request.getPageNumber(), the existing
bookmark.getBookId(), bookmark.getUserId(), and bookmarkId as the exclude-id so
the current bookmark is ignored; mirror the CFI-path logic used in
createBookmark/updateBookmark to prevent duplicate page-number bookmarks.

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts`:
- Around line 249-255: Initialize the viewer's spreadMode from the persisted
this.spread when setting up the viewer (so spreadMode reflects the currently
applied preference) and update/persist this.spread whenever cycleSpreadMode()
changes spreadMode; specifically, in the initialization code that reads
pdfPrefs.pdfSettings and myself.userSettings.pdfReaderSetting set spreadMode =
this.spread, and in cycleSpreadMode() after computing the new spreadMode assign
this.spread = spreadMode and write it back into pdfPrefs.pdfSettings.spread (and
call the existing preferences-saving routine used elsewhere to persist the
change). This same fix should be applied in the other occurrence noted (lines
~519-524).

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/app/features/readers/ebook-reader/ebook-reader.component.ts`:
- Around line 484-489: The subscription to viewManager.goTo(linkUrl) in the
go-to-link branch can leak because it lacks the takeUntil(this.destroy$)
teardown used elsewhere; update the call that currently does
this.viewManager.goTo(linkUrl).subscribe() so the observable is piped through
takeUntil(this.destroy$) before subscribing (referencing viewManager.goTo and
this.destroy$ in ebook-reader.component.ts), then keep the existing
this.selectionService.handleAction(action) call as-is.

In
`@frontend/src/app/features/readers/ebook-reader/shared/selection-popup.component.html`:
- Around line 72-77: The tooltip/action are mismatched: the button shows
"removeLink" when linkUrl exists but onDelete() always emits a delete action, so
either change the tooltip to always use t('deleteAnnotation') or make the click
emit a link-removal action; to fix, update the click handler logic in the
component where onDelete() is defined so it checks linkUrl and emits {type:
'removeLink', annotationId: overlappingAnnotationId, linkUrl} when linkUrl is
truthy (and keep emitting {type: 'delete', annotationId:
overlappingAnnotationId} otherwise), and ensure the parent handler that
processes these events (the consumer of onDelete()/the emitted action) is
updated to handle the new 'removeLink' action type; references:
overlappingAnnotationId, linkUrl, onDelete(), emitted action object.

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.html`:
- Around line 29-31: The icon-only buttons in pdf-reader.component.html lack
accessible labels; update each <button> that currently only contains
<app-reader-icon> (the ones wired to dismissDocViewerInfo(), the bookmark toggle
handler, the overflow/open menu handler, and the search close handler) to
include an aria-label that mirrors the action text (or reference an i18n key) —
e.g. aria-label="{{ 'pdf.dismissNotice' | translate }}" for
dismissDocViewerInfo(), aria-label="{{ 'pdf.toggleBookmark' | translate }}" for
the bookmark button, aria-label="{{ 'pdf.moreOptions' | translate }}" for the
overflow/open menu, and aria-label="{{ 'pdf.closeSearch' | translate }}" for the
search close — ensuring the label strings match visible action text or new i18n
keys.

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts`:
- Around line 383-389: The code stores temporary object URLs in pdfBlobUrl when
loading PDFs via fetchAsObjectUrl but never revokes them, causing memory leaks;
before assigning a new pdfBlobUrl in the pdf loading branch (the block that
calls this.fetchAsObjectUrl) call URL.revokeObjectURL on the existing
this.pdfBlobUrl if present, set this.pdfBlobUrl to the new URL, and also revoke
any non-null this.pdfBlobUrl inside destroyBookViewer() and the component
teardown (ngOnDestroy) to ensure all temporary blob URLs are released; reference
the pdfBlobUrl property, fetchAsObjectUrl method, and
destroyBookViewer()/ngOnDestroy lifecycle teardown to locate where to add the
revoke calls.
- Around line 752-770: The mode switch currently flips this.viewerMode before
saving and never stores the setTimeout handles, causing persistAnnotations/save
paths (see persistAnnotations and saveEmbedPdfDocument) to short-circuit and
stale timers to run; fix setViewerMode so it first clears any existing
this.initTimeout, awaits persistAnnotations (when switching to 'document') or
triggers saveEmbedPdfDocument appropriately, only then updates this.viewerMode,
and store the return value of setTimeout in this.initTimeout for both branches
so future clearTimeout calls actually cancel pending inits; also ensure
destroyBookViewer/destroyDocViewerIframe and initDocViewerIframe/initBookViewer
are invoked after the save completes and the timeout handle is set.

In
`@frontend/src/app/features/readers/pdf-reader/services/embedpdf-book.service.ts`:
- Around line 366-367: The teardown currently calls restoreWorkerShims() and
restoreReleasePointerCapture() but fails to restore a patched global
devicePixelRatio; update the code where devicePixelRatio is overridden (the
patch logic around devicePixelRatio) to save the original descriptor/value
(e.g., originalDevicePixelRatioDescriptor or originalDevicePixelRatioValue) when
patching, add a restoreDevicePixelRatio() that re-applies the saved
descriptor/value, and call that restore from destroy() alongside
restoreWorkerShims() and restoreReleasePointerCapture() so the global
window.devicePixelRatio is returned to its original state after the reader
closes.
- Around line 50-53: The destroy() method is completing the shared Subjects
(pageChange$, annotationEvent$, documentOpened$, layoutReady$) which makes the
service unusable after a mode/book switch; either stop calling complete() on
those four streams in destroy() so they remain usable for the service lifetime,
or move their creation out of the readonly class fields and recreate
pageChange$, annotationEvent$, documentOpened$, and layoutReady$ at the start of
init() so each new document gets fresh Subjects; update destroy() or init()
accordingly and keep references to the symbols pageChange$, annotationEvent$,
documentOpened$, layoutReady$, destroy(), and init() when applying the change.

In
`@frontend/src/app/features/readers/pdf-reader/services/pdf-bookmark.service.ts`:
- Around line 67-84: On a 409 duplicate error from
this.bookMarkService.createBookmark, the handler should refresh the local cache
for the affected page (or reload the book's bookmarks) before returning false so
the UI reflects the server state; update the catchError block to, when
error?.status === 409, call the appropriate bookmark-fetching method (e.g., a
method that loads all bookmarks for the current book or a method that fetches
the bookmark for pageNumber), update this.bookmarks.set(pageNumber,
fetchedBookmark) (or repopulate this.bookmarks from the returned list), then
show the existing warning via this.messageService.add and return of(false) so
toggleBookmark() will see the hydrated cache.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.html`:
- Around line 129-142: The zoom label button lacks an accessible name and the
dropdown doesn't close on outside clicks; update the button in
pdf-reader.component (the element that calls toggleZoomMenu() and displays {{
zoom }}) to include an accessible name via aria-label or title (e.g., "Zoom
options, current: {{ zoom }}") and include aria-expanded bound to
isZoomMenuOpen; then implement a backdrop element rendered when isZoomMenuOpen
is true (mirror the overflow menu pattern) that captures clicks and calls
toggleZoomMenu() or a new closeZoomMenu() method to close the zoom dropdown;
ensure the dropdown options (setZoomPreset and zoomPresets loop) remain
keyboard-focusable and that the backdrop is inserted/removed together with
isZoomMenuOpen.
- Around line 244-246: The backdrop div's (keydown.escape) won't fire because it
isn't focusable; update the PDFReaderComponent to handle Escape properly by
moving the escape listener to a focusable element or the component/document:
when isToolbarOverflowOpen becomes true, set focus into the toolbar element
(query the element identified by "toolbar-overflow-menu" via `@ViewChild` in
PDFReaderComponent) and attach a keydown handler there to call
closeToolbarOverflow(), or alternatively add a host/document-level keydown
listener in the component that listens for Escape and calls
closeToolbarOverflow(); ensure the focused element is removed or the listener
unsubscribed when the menu closes.

In
`@frontend/src/app/features/readers/pdf-reader/services/embedpdf-book.service.ts`:
- Around line 654-670: The ResizeObserver created in setupResizeObserver isn't
stored and may not be disconnected if destroy() runs before layoutReady$ emits;
change setupResizeObserver (and the class) to save the created observer (e.g.,
this.resizeObserver) and any subscription (from layoutReady$) as class fields,
stop using an anonymous observer variable, and in destroy() call
this.resizeObserver.disconnect() and unsubscribe the saved subscription (or set
it to null) to ensure deterministic cleanup; keep the existing
zoom/getSpreadMode/requestZoom logic intact.

---

Nitpick comments:
In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.html`:
- Around line 104-118: Extract the duplicated bookmark SVGs used in the bookmark
button (the template that references isCurrentPageBookmarked and
(click)="toggleBookmark()") into a single reusable piece—either an <ng-template>
(e.g., `#bookmarkFilled` and `#bookmarkOutline`) and render with *ngTemplateOutlet
where needed, or create a small BookmarkIconComponent that accepts an input like
[filled]="isCurrentPageBookmarked" and replace the inline SVGs in both desktop
and mobile buttons with the reusable template/component; do the same for theme
toggle icons to eliminate duplication while preserving the existing bindings and
button classes (e.g., class="icon-btn bookmark-btn") and click handler
toggleBookmark().

In
`@frontend/src/app/features/readers/pdf-reader/services/embedpdf-book.service.ts`:
- Around line 172-179: The fallback setTimeout started inside init
(zone.runOutsideAngular -> setTimeout -> zone.run ->
this.documentOpened$.next(...)) should be tracked and cleared in destroy to
avoid firing after teardown; add a class property (e.g., fallbackTimeoutId) to
store the value returned by setTimeout when scheduling the fallback and call
clearTimeout(fallbackTimeoutId) and null it inside destroy() before
completing/disposing documentOpened$ so the timeout callback never runs after
destroy().

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.html`:
- Around line 44-45: The template uses a wrong translation key
'readerPdf.sidebar.title' in the PDF reader button; update the binding in
pdf-reader.component.html to use the existing key 'readerPdf.toolbar.sidebar' so
the button's title resolves to the localized string (locate the button with
class "icon-btn" and the (click)="toggleSidebar()" handler and replace the
translation key accordingly).

In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts`:
- Around line 255-263: Debounced search still fires a queued query after the
search UI is closed; update the stream that listens to searchQuery$ to cancel
stale searches when the search is closed by adding a guard or cancellation
trigger (e.g., filter by an isSearchOpen observable/property or use takeUntil
with a searchClosed Subject emitted from closeSearch()). Specifically, modify
the pipeline around searchQuery$ (the debounceTime(...).subscribe(...) block
that calls embedPdfBook.searchAllPages) so it only proceeds when the search is
open (use filter(() => this.isSearchOpen) or takeUntil(this.searchClose$) tied
to closeSearch()), ensuring no queued debounced searches run after closeSearch()
is called.
- Around line 1132-1168: Footer handlers (goToFirstPage, goToPreviousPage,
goToNextPage, goToLastPage, onSliderChange, onGoToPage) currently always call
onPageChange even when viewerMode !== 'book', causing persisted page state to
drift; update these methods to only call onPageChange when the viewer actually
moves: either (A) short-term: guard the onPageChange calls with if
(this.viewerMode === 'book') so document mode does not update persisted state,
or (B) full fix: when viewerMode === 'document' send a postMessage navigation
command to the iframe and wait for the iframe/page-change sync event before
calling onPageChange (use existing page/totalPages/goToPageInput fields to
validate). Choose one approach and implement it consistently across
goToFirstPage, goToPreviousPage, goToNextPage, goToLastPage, onSliderChange and
onGoToPage.

---

Duplicate comments:
In `@frontend/src/app/features/readers/pdf-reader/pdf-reader.component.html`:
- Around line 130-140: The zoom toggle button currently exposes only the raw
zoom value (zoom) to assistive tech; update the button element that calls
toggleZoomMenu() to include a descriptive aria-label (e.g., "Zoom options" or
similar), bind aria-expanded to isZoomMenuOpen, and optionally add
aria-haspopup="menu" so screen readers know it opens a menu; keep existing
behavior for displaying {{ zoom }} and the existing click handler
(toggleZoomMenu()) and leave the dropdown rendering (zoomPresets, setZoomPreset)
unchanged.

In
`@frontend/src/app/features/readers/pdf-reader/services/embedpdf-book.service.ts`:
- Around line 172-180: The fallback setTimeout started in init() (inside
this.zone.runOutsideAngular -> setTimeout -> this.documentOpened$.next(...)) can
fire after teardown and pollute a new init cycle; store its timer id (from
setTimeout) in a class field when scheduling and clear it during
teardown/unsubscribe (e.g., in the same method that cleans up the
plugin/unsubscribe or ngOnDestroy) by calling clearTimeout on that id; apply the
same change to the duplicate fallback at the other location (the second
setTimeout around lines 346-350) so no stale callbacks emit into a recreated
documentOpened$.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend/src/app/features/readers/ebook-reader/ebook-reader.component.ts`:
- Around line 487-489: The subscription to
viewManager.goTo(linkUrl).pipe(takeUntil(this.destroy$)).subscribe() lacks error
handling and can emit unhandled errors; update the call to handle errors by
providing an error callback to subscribe (or pipe in catchError) that gracefully
handles/logs navigation failures (e.g., call a logger, show user feedback, or
ignore benign errors) and ensures the stream completes cleanly; target the
viewManager.goTo(...) invocation and the subscribe() call so the fix adds an
error handler and any needed cleanup logic tied to destroy$.