Conversation
Rename Audiobookshelf → Audiobooks, Sync Clients → Ebooks. Move Storyteller to Audiobooks tab, Hardcover to Tools tab. Rename "Booklore 2" to "Booklore — Secondary".
Extract duplicated nav markup from 6 templates into a shared Jinja partial (templates/partials/navbar.html). All pages now render the same responsive header with hamburger menu, consistent links, and stricter conditional checks for library icons.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
✏️ Tip: You can disable in-progress messages and the fortune message in your review settings. WalkthroughThis PR refactors template navigation by extracting hardcoded navbar markup from six templates into a new reusable navbar partial with conditional rendering. The settings template is restructured, renaming navigation tabs (Audiobookshelf → Audiobooks, Sync Clients → Ebooks), introducing an Ebooks section, and adding Hardcover.app configuration blocks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
templates/partials/navbar.html (1)
8-12: Expose menu state to assistive tech on toggle.Line 8 introduces an interactive nav toggle, but the script does not update
aria-expanded. Addaria-controlsand keep expanded state in sync when the menu opens/closes.Proposed fix
- <button class="nav-toggle" id="nav-toggle" aria-label="Toggle navigation"> + <button class="nav-toggle" id="nav-toggle" aria-label="Toggle navigation" aria-controls="nav-menu" aria-expanded="false"> <span></span><span></span><span></span> </button> @@ -<script> -document.getElementById('nav-toggle')?.addEventListener('click', function() { - document.getElementById('nav-menu')?.classList.toggle('open'); - this.classList.toggle('active'); -}); -</script> +<script> +const navToggle = document.getElementById('nav-toggle'); +const navMenu = document.getElementById('nav-menu'); + +navToggle?.addEventListener('click', function () { + const isOpen = navMenu?.classList.toggle('open'); + this.classList.toggle('active'); + this.setAttribute('aria-expanded', String(!!isOpen)); +}); +</script>Also applies to: 55-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/partials/navbar.html` around lines 8 - 12, The nav toggle button (id="nav-toggle") currently lacks ARIA linkage and expanded state; add aria-controls="nav-menu" to the button and ensure its aria-expanded attribute is initialized to "false" (or reflects current visibility of the element with id="nav-menu"), then update aria-expanded to "true"/"false" inside the existing toggle handler whenever the menu (class "nav-menu"/id "nav-menu") is opened or closed; apply the same changes to the second toggle instance so assistive tech sees the controlled element and the correct expanded state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@templates/partials/navbar.html`:
- Around line 8-12: The nav toggle button (id="nav-toggle") currently lacks ARIA
linkage and expanded state; add aria-controls="nav-menu" to the button and
ensure its aria-expanded attribute is initialized to "false" (or reflects
current visibility of the element with id="nav-menu"), then update aria-expanded
to "true"/"false" inside the existing toggle handler whenever the menu (class
"nav-menu"/id "nav-menu") is opened or closed; apply the same changes to the
second toggle instance so assistive tech sees the controlled element and the
correct expanded state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
templates/batch_match.htmltemplates/index.htmltemplates/logs.htmltemplates/match.htmltemplates/partials/navbar.htmltemplates/settings.htmltemplates/shelfmark.html
Merge pull request #3 from serabi/dev
* Fix remaining book_id migration issues, guard Booklore cache, scope suggestions (#20) Completes the ABS ID decoupling by fixing service/repository methods that still used abs_id as lookup keys, removing 19 dead backward-compat methods, and cleaning up unnecessary abs_id parameters. Key changes: - Fix reading_stats, alignment, storyteller, dashboard lookups to use book_id - Guard Booklore cache loading behind is_configured() for unconfigured instances - Scope suggestion operations by (source_id, source) composite key with unique index migration, preventing collisions across ABS/KoSync/Booklore - Remove dead is_hash_linked_to_device methods from kosync and suggestion repos - Add 14 new tests for book_id resolution, suggestion scoping, and alignment ops All 458 tests passing. * Fix database upgrade safety issues from v0.1.4 compatibility review - Guard save_state() against double-NULL book_id/abs_id lookup - Isolate per-column error handling in _ensure_model_columns - Log orphaned rows in nullable table backfill migration - Remove dead delete_hardcover_details_by_book_id method * fix: sort imports to satisfy ruff I001 * Fix abs_id→book_id migration gaps from CodeRabbit review (#50) Fixes 6 issues found during v0.1.5 PR review: - Restore _rdAbsId JS variable in reading_detail.html (all action buttons broken) - Key KoSync debounce, poll cache, and write-suppression by book.id not abs_id (ebook-only books have abs_id=None, collapsing all into one dict entry) - Fix link_kosync_document to set linked_abs_id for backward compat; query linked/unlinked docs by linked_book_id (the canonical FK) - Guard get_book_by_abs_id(None) with early return - Gate Base.metadata.create_all() on migration success * Fix ebook card display: portrait covers instead of square Change .resource-card from forced 1:1 aspect ratio to portrait layout matching audiobook cards. Add 2:3 aspect ratio to .resource-cover-container with object-fit: cover. Increase resource grid min column width to 160px to match audiobook grid. Ghost cards retain compact centered layout. * Smart mode defaults: auto-detect available services Default to Ebook Only mode when ABS is not configured. Detect all ebook sources (Booklore, CWA, ABS ebook libs, local /books mount). Disable mode buttons that have no backing service. Update subtitle from "ABS library" to "audiobook library". * Rename ABS-specific methods to generic audiobook names get_abs_title → get_audiobook_title, get_abs_author → get_audiobook_author. Change fallback source label from "abs" to "unknown" in suggestion serialization. These methods extract generic metadata, not ABS-specific. * Extract _create_book_mapping helper to deduplicate match logic Single match POST and batch process_queue shared ~130 lines of identical logic (Booklore lookup, KOSync, hash preservation, duplicate merge, Hardcover, Booklore shelf, Storyteller, suggestion resolution). Both now call _create_book_mapping(), reducing net code by ~100 lines. * Add ebook-only support to batch match Allow adding ebook-only items to the batch queue (no audiobook required). Add ebook-only processing branch in process_queue. Update JS to enable "Add to Queue" when ebook is selected without audiobook. Queue items now use a generic queue_key for dedup. * Clean up dead code and unnecessary getattr usage Replace getattr() with direct attribute access in _copy_book_merge_metadata since existing_book is always a SQLAlchemy Book model. Update docstring for get_audiobook_author to remove ABS-specific language. * Fix missing BookFusion covers and broken onerror fallback Skip ABS cover proxy for bf- prefixed books (always 404'd), deduplicate dashboard cover waterfall into resolve_book_covers(), fix onerror chain so placeholder shows when KoSync fallback also fails, add branded BookFusion placeholder logo. * Consolidate suggestion serializer and remove dismissed status Move _serialize_suggestion into helpers.py as shared utility, removing duplicate definitions from api.py and matching_bp.py. Unify dismissed → hidden status throughout suggestion_repository. Allow suggestion rescan to proceed when ABS is unconfigured (BookFusion-only setups). Pass storyteller_configured flag to match/batch_match templates. * Hide Storyteller UI when unconfigured and fix ABS cover proxy fallback Conditionally hide Storyteller column in match/batch_match when the integration is not configured. ABS cover proxy now falls back to using the raw book_ref as abs_id when no book record exists, allowing direct ABS ID lookups without a mapped book. * Improve suggestions page UX with inline refresh and modal errors Replace location.reload() after rescan/link actions with inline data refresh via refreshSuggestionsData(). Replace alert() calls with showErrorToast() using the app's confirm modal. Update copy to be source-agnostic ("unmapped book pairings" instead of "audiobook"). * Cache book metadata (author/subtitle) locally and clean up helpers Add author and subtitle columns to Book model so these fields survive ABS outages. Dashboard opportunistically refreshes from live ABS data and falls back to cached values when disconnected. All book creation sites in matching_bp now populate author/subtitle from ABS metadata. Extract shared helpers (find_booklore_metadata, attempt_hardcover_automatch) to reduce duplication across dashboard, reading, and matching blueprints. Remove dead getattr calls for columns that have model defaults. Also includes ABS cover proxy local caching for offline resilience. * Show service logo placeholder when book cover is unavailable Add placeholder_logo field to mapping/book data dicts, determined by primary source (BookFusion, Booklore, or Audiobookshelf). Display the logo in all cover placeholder divs across dashboard, reading log, reading detail, and backlog cards. * Deduplicate placeholder_logo logic, fix cover proxy streaming, and fix N+1 query Extract resolve_placeholder_logo() into cover_resolver.py and return it from resolve_book_covers(), removing duplicate 4-branch conditionals from dashboard.py and reading_bp.py. Drop unnecessary stream=True from cover proxy requests that immediately buffer via .content. Bulk-fetch Hardcover details on the reading page to avoid per-book N+1 queries. * De-center ABS on batch match page and fix BookFusion enabled check Hide the audiobook column on batch match when ABS isn't configured, adapting section numbers, hints, and status text to be service-agnostic. Fix BookFusionClient.is_configured() to respect BOOKFUSION_ENABLED, matching the pattern used by all other service clients. * Hide Suggestions nav link and guard route when ABS is not configured Suggestions require Audiobookshelf to produce results. Gate the nav link on abs_url and redirect /suggestions to dashboard when ABS is unavailable. Also conditionally hide BookFusion filter/stat on the suggestions page when BookFusion is not enabled. * Update suggestions page description wording * Frontend overhaul + testing gaps: extract inline JS, unify modals, add 260 tests Frontend: - Create shared utils.js (escapeHtml, debounce, toggleHiddenSection) - Create unified confirm-modal.js with PKModal API (confirm, confirmForm, alert) - Create shared confirm_modal.html partial, replacing 5 duplicate modal blocks - Extract inline JS from 5 templates into external files: suggestions.html (445 lines), bookfusion.html (833 lines), logs.html (655 lines), match.html (363 lines), batch_match.html (162 lines) - Use PK_PAGE_DATA pattern for Jinja2→JS data bridging - Consolidate .btn-error into .btn-danger - Wire dashboard.js to use shared PKModal via legacy bridge Testing (461 → 721 tests): - Expand conftest.py with canonical MockContainer, pytest fixtures, test helpers - Add env var save/restore to flask_app fixture to prevent test pollution - New blueprint tests: bookfusion routes (56), logs routes (27), dashboard errors (7) - New service tests: BackgroundJobService (30), ReadingDateService (30), BookMetadataService (12), ReadingService (10), ClientPoller (8) - New integration tests: settings hot-reload (10), sync concurrency (7) - New error path tests: helpers (12), matching (9), reading bp (8), API (17) * Address code review findings: error handling, modal convention, dead code - bookfusion.js: capture error parameter in all 12 catch blocks, surface error messages to users instead of generic "Error" text - confirm-modal.js: add null guard in _resolve() when modal partial is missing from the page - dashboard.js: replace native confirm()/alert() with PKModal.confirm() and PKModal.alert() per project convention - logs.js: remove undeclared lastLogTimestamp variable (dead code from extraction), replace with shownLogs.clear() - reading_service.py: add warning log to pull_started_at catch block that was silently falling back to today's date * Address code review findings: null guards, offset bug, import sorting, dead code - Guard PKModal public methods against missing modal partial - Fix double-increment of currentOffset in logs loadMore handler - Add null guards in batch-match.js and utils.js DOM access - Remove unused preselectedEb variable in match.js - Guard against book.abs_id being None in cover proxy - Default mock_abs_client.is_configured to False in test fixtures - Wrap flask_app fixture in try/finally for safe teardown - Remove unused imports and variables in test files - Fix ruff I001 import sorting across all affected files * Address remaining review findings: unify modals, fix state detection, clean up patterns - Replace native confirm()/alert()/prompt() with PKModal on tbr-detail, dashboard, reading-detail, and settings pages - Migrate settings.js custom modal system to shared PKModal API - Fix dashboard refreshPaused flag never resetting on button-close - Replace fragile textContent.includes() state detection in logs.js with filterPending boolean flag - Add visible error display for live log fetch failures - Fix double JSON.stringify in suggestions.js BookFusion flow * KoSync system overhaul: service extraction, document management, bug fixes Major refactoring and feature additions for the KoSync subsystem: Service extraction: - Extract 375 lines of business logic from kosync_server.py into new KosyncService class (src/services/kosync_service.py) - Decompose _try_find_epub_by_hash (151 lines) into 3 focused methods - Remove dead code: _hash_cache, unused repository methods KoSync Document Management page (/kosync-documents): - New page accessible from Settings > KoSync tab - Three sections: Healthy, Needs Attention, Stale (30+ days) - Actions: Link to Book (search), Link to Self, Create Book, Clear Hash, Unlink, Delete - Rich context: book titles, time-ago indicators, device vs bot labels - Dashboard "Pending Identification" section for unlinked hashes with reading progress Bug fixes: - Fix sync direction inversion: mixed text-matched and percentage-fallback normalization could elect wrong leader (Entitled at 39% over Booklore at 45%) - Fix Booklore get_text_from_current_state using wrong filename - Fix Booklore 2 not showing as pairing option when only BL2 enabled - Fix Booklore crash on books with no ebook filename - Fix ebook-only books showing as unlinked (linked_abs_id vs linked_book_id) - Fix Link to Self sending empty body (Flask 400) - Fix external KoSync server missing credential fields and secret handling - Prevent orphaned hashes by creating KosyncDocument on every book save Improvements: - Rename abs-kosync-bot to pagekeeper-bot, centralize in constants.py - Remove legacy bot names (book-stitch, book-sync) - Redesign KoSync settings tab: sync source at top, conditional sections - Auto-create books for exact ABS title matches (skip suggestion approval) - Downgrade noisy no-progress warnings to debug - Include book title in Instant Sync log message - Add external KoSync server credential fields (KOSYNC_SERVER_USER/KEY) * Address code review findings: security, data integrity, dead code Critical fixes: - XSS: Replace |safe with |tojson for JSON in templates (kosync_documents, suggestions) — prevents script injection via book titles - Path traversal: Use Path().name to strip directory components from Booklore filenames before cache write, add is_safe_path_within check - Data integrity: Unlink/delete endpoints now clear book.kosync_doc_id to prevent recreating orphaned hashes - Primary key mutation: Never mutate document_hash on cached doc — delete old record and create new one instead Other fixes: - Sanitize book.title in Instant Sync and Booklore log messages - Settings: server-side radio state for builtin/external KoSync mode - _test_kosync uses urlparse-based external detection for credential selection - _is_external uses urlparse instead of fragile string matching - Null guards in toggleKosyncSourceMode JS - CSS: unquote Outfit font-family - Remove dead get_all_books() call in Booklore discovery - Strip server_id prefix from Booklore-cached filenames for title derivation - Remove unused json import from matching_bp * Address Macroscope review: linked_abs_id, LibraryService args, null guard, dead code * Address remaining review findings: atomicity, metadata consistency, dead code - Fix book_id None producing malformed "server_id:None" in Booklore search - Return cached_doc.filename for consistent Booklore path format - Reorder resolve-orphan to register hash before mutating books - Populate filename on existing KosyncDocument when linking - Add filename to register_hash_for_book new-doc path - Expand localhost detection to cover localhost/::1 variants - Add KOSYNC_SERVER_KEY to secret reveal whitelist - Add credential fallback for external KoSync test - Log instead of swallowing exceptions in _find_epub_in_db - Extract _serialize_document helper to deduplicate listing code - Fix isnot(None) style to match repo convention * Fix null guard, prefix stripping, enable toggle visibility, secret classification - Use getattr for raw_metadata_dict to handle None values safely - Only strip numeric prefixes (Booklore server IDs) from cached filenames - Move KoSync Enable toggle to always-visible Sync Source section - Remove KOSYNC_SERVER_USER from SECRET_SETTING_KEYS * Decompose EbookParser god class into focused modules Extract KoReaderXPathService and LocatorSearchService from the 1,140-line EbookParser, reducing it to a 335-line facade. The two new stateless services receive (full_text, spine_map) as arguments, making them independently testable without file I/O or mocking. - koreader_xpath.py: XPath generation/resolution, get_perfect_ko_xpath broken into 3 phases (text node location, hybrid BS4→LXML anchor mapping, BS4 structural fallback) - locator_search.py: text search (anchor/exact/normalized/fuzzy), CFI resolution, Storyteller/Readium locator resolution - Remove dead code: get_character_delta(), _has_text_content() - Add 46 new unit tests (23 xpath, 23 locator search) - Zero caller/DI changes — EbookParser facade preserves the public API * Refactor KoSync server: split monolith, extract utilities, add tests Split the 788-line kosync_server.py into focused modules: - kosync_server.py (123 lines): thin protocol route handlers - kosync_admin.py (284 lines): dashboard management routes - kosync_auth.py (121 lines): shared auth decorators Extract reusable utilities: - rate_limiter.py: TokenBucketRateLimiter (thread-safe token bucket) - debounce_manager.py: DebounceManager (PUT event debouncing) Move PUT/GET business logic from route handlers to KosyncService: - handle_put_progress: validation, furthest-wins, save, link, activity flag - handle_get_progress: 4-step lookup chain - resolve_best_progress: sibling doc selection + state fallback Eliminate module-level globals — services stored in Flask app.config, matching the existing blueprint helpers pattern. Add 38 new tests (20 service, 7 rate limiter, 11 debounce). Update 3 existing tests for new architecture. * Security audit: fix unauthenticated secret endpoint, path traversal, XSS, add hardening - Wire up _is_secret_request_authorized() in get_secret() (was defined but never called) - Add is_safe_path_within() checks in covers endpoint and ebook cache fallback - Apply sanitize_html filter to TBR description template - Add X-Frame-Options and X-Content-Type-Options security headers - Run container as non-root appuser in Dockerfile * fix(security,perf): defusedxml, dead code cleanup, N+1 query, DOM perf Security: - Replace xml.etree.ElementTree with defusedxml in CWA client and SMIL extractor to prevent XML entity expansion attacks - Remove dead session['is_admin'] check from secret auth function - Mask ABS API token in stream URL log messages - Set SESSION_COOKIE_SAMESITE=Lax and SESSION_COOKIE_HTTPONLY=True Performance: - Fix N+1 query in /api/processing-status using existing bulk method - Remove redundant rglob("*") fallback in resolve_book_path - Add 150ms debounce to reading page search input - Use DocumentFragment for dashboard sort to batch DOM reflows - Skip sorting hidden dashboard grids Infrastructure: - Add defusedxml==0.7.1 to requirements - Run test container as root for pip install compatibility * fix: SMIL indentation bug, restore secret reveal auth, sort hidden grids Address PR #3 review feedback from Macroscope: - Fix incorrect nesting of relative timestamp handler inside absolute block - Restore session-based admin check for browser secret reveal endpoint - Remove offsetParent guard so hidden dashboard grids get sorted * fix: remove unused variable to pass ruff CI * fix(security): sanitize KoSync endpoint inputs for Snyk XSS findings Validate doc_id format on GET progress, type-check request body on PUT. * fix(security): use make_response with explicit content type to break Snyk taint chain * fix: address PR #4 review findings from Macroscope - Guard suggestion cleanup when kept_ids is empty (ABS outage safety) - Move thread-start check inside lock in debounce_manager (race fix) - Remove offset addition on empty text nodes in xpath resolver - Iterate sentence tags in document order, not tag priority order - Map dismissed status to hidden in serialize_suggestion - Fix refreshPaused scoping: move to module scope in dashboard.js - Re-read localStorage in suggestions.js viewport change handler * fix: distinguish Storyteller-only from ebook-only in batch match UI * fix: map dismissed status to hidden in serialize_suggestion * fix: handle IPv4-mapped IPv6 addresses in private IP check
* Security audit fixes + refactors (#4) * Fix remaining book_id migration issues, guard Booklore cache, scope suggestions (#20) Completes the ABS ID decoupling by fixing service/repository methods that still used abs_id as lookup keys, removing 19 dead backward-compat methods, and cleaning up unnecessary abs_id parameters. Key changes: - Fix reading_stats, alignment, storyteller, dashboard lookups to use book_id - Guard Booklore cache loading behind is_configured() for unconfigured instances - Scope suggestion operations by (source_id, source) composite key with unique index migration, preventing collisions across ABS/KoSync/Booklore - Remove dead is_hash_linked_to_device methods from kosync and suggestion repos - Add 14 new tests for book_id resolution, suggestion scoping, and alignment ops All 458 tests passing. * Fix database upgrade safety issues from v0.1.4 compatibility review - Guard save_state() against double-NULL book_id/abs_id lookup - Isolate per-column error handling in _ensure_model_columns - Log orphaned rows in nullable table backfill migration - Remove dead delete_hardcover_details_by_book_id method * fix: sort imports to satisfy ruff I001 * Fix abs_id→book_id migration gaps from CodeRabbit review (#50) Fixes 6 issues found during v0.1.5 PR review: - Restore _rdAbsId JS variable in reading_detail.html (all action buttons broken) - Key KoSync debounce, poll cache, and write-suppression by book.id not abs_id (ebook-only books have abs_id=None, collapsing all into one dict entry) - Fix link_kosync_document to set linked_abs_id for backward compat; query linked/unlinked docs by linked_book_id (the canonical FK) - Guard get_book_by_abs_id(None) with early return - Gate Base.metadata.create_all() on migration success * Fix ebook card display: portrait covers instead of square Change .resource-card from forced 1:1 aspect ratio to portrait layout matching audiobook cards. Add 2:3 aspect ratio to .resource-cover-container with object-fit: cover. Increase resource grid min column width to 160px to match audiobook grid. Ghost cards retain compact centered layout. * Smart mode defaults: auto-detect available services Default to Ebook Only mode when ABS is not configured. Detect all ebook sources (Booklore, CWA, ABS ebook libs, local /books mount). Disable mode buttons that have no backing service. Update subtitle from "ABS library" to "audiobook library". * Rename ABS-specific methods to generic audiobook names get_abs_title → get_audiobook_title, get_abs_author → get_audiobook_author. Change fallback source label from "abs" to "unknown" in suggestion serialization. These methods extract generic metadata, not ABS-specific. * Extract _create_book_mapping helper to deduplicate match logic Single match POST and batch process_queue shared ~130 lines of identical logic (Booklore lookup, KOSync, hash preservation, duplicate merge, Hardcover, Booklore shelf, Storyteller, suggestion resolution). Both now call _create_book_mapping(), reducing net code by ~100 lines. * Add ebook-only support to batch match Allow adding ebook-only items to the batch queue (no audiobook required). Add ebook-only processing branch in process_queue. Update JS to enable "Add to Queue" when ebook is selected without audiobook. Queue items now use a generic queue_key for dedup. * Clean up dead code and unnecessary getattr usage Replace getattr() with direct attribute access in _copy_book_merge_metadata since existing_book is always a SQLAlchemy Book model. Update docstring for get_audiobook_author to remove ABS-specific language. * Fix missing BookFusion covers and broken onerror fallback Skip ABS cover proxy for bf- prefixed books (always 404'd), deduplicate dashboard cover waterfall into resolve_book_covers(), fix onerror chain so placeholder shows when KoSync fallback also fails, add branded BookFusion placeholder logo. * Consolidate suggestion serializer and remove dismissed status Move _serialize_suggestion into helpers.py as shared utility, removing duplicate definitions from api.py and matching_bp.py. Unify dismissed → hidden status throughout suggestion_repository. Allow suggestion rescan to proceed when ABS is unconfigured (BookFusion-only setups). Pass storyteller_configured flag to match/batch_match templates. * Hide Storyteller UI when unconfigured and fix ABS cover proxy fallback Conditionally hide Storyteller column in match/batch_match when the integration is not configured. ABS cover proxy now falls back to using the raw book_ref as abs_id when no book record exists, allowing direct ABS ID lookups without a mapped book. * Improve suggestions page UX with inline refresh and modal errors Replace location.reload() after rescan/link actions with inline data refresh via refreshSuggestionsData(). Replace alert() calls with showErrorToast() using the app's confirm modal. Update copy to be source-agnostic ("unmapped book pairings" instead of "audiobook"). * Cache book metadata (author/subtitle) locally and clean up helpers Add author and subtitle columns to Book model so these fields survive ABS outages. Dashboard opportunistically refreshes from live ABS data and falls back to cached values when disconnected. All book creation sites in matching_bp now populate author/subtitle from ABS metadata. Extract shared helpers (find_booklore_metadata, attempt_hardcover_automatch) to reduce duplication across dashboard, reading, and matching blueprints. Remove dead getattr calls for columns that have model defaults. Also includes ABS cover proxy local caching for offline resilience. * Show service logo placeholder when book cover is unavailable Add placeholder_logo field to mapping/book data dicts, determined by primary source (BookFusion, Booklore, or Audiobookshelf). Display the logo in all cover placeholder divs across dashboard, reading log, reading detail, and backlog cards. * Deduplicate placeholder_logo logic, fix cover proxy streaming, and fix N+1 query Extract resolve_placeholder_logo() into cover_resolver.py and return it from resolve_book_covers(), removing duplicate 4-branch conditionals from dashboard.py and reading_bp.py. Drop unnecessary stream=True from cover proxy requests that immediately buffer via .content. Bulk-fetch Hardcover details on the reading page to avoid per-book N+1 queries. * De-center ABS on batch match page and fix BookFusion enabled check Hide the audiobook column on batch match when ABS isn't configured, adapting section numbers, hints, and status text to be service-agnostic. Fix BookFusionClient.is_configured() to respect BOOKFUSION_ENABLED, matching the pattern used by all other service clients. * Hide Suggestions nav link and guard route when ABS is not configured Suggestions require Audiobookshelf to produce results. Gate the nav link on abs_url and redirect /suggestions to dashboard when ABS is unavailable. Also conditionally hide BookFusion filter/stat on the suggestions page when BookFusion is not enabled. * Update suggestions page description wording * Frontend overhaul + testing gaps: extract inline JS, unify modals, add 260 tests Frontend: - Create shared utils.js (escapeHtml, debounce, toggleHiddenSection) - Create unified confirm-modal.js with PKModal API (confirm, confirmForm, alert) - Create shared confirm_modal.html partial, replacing 5 duplicate modal blocks - Extract inline JS from 5 templates into external files: suggestions.html (445 lines), bookfusion.html (833 lines), logs.html (655 lines), match.html (363 lines), batch_match.html (162 lines) - Use PK_PAGE_DATA pattern for Jinja2→JS data bridging - Consolidate .btn-error into .btn-danger - Wire dashboard.js to use shared PKModal via legacy bridge Testing (461 → 721 tests): - Expand conftest.py with canonical MockContainer, pytest fixtures, test helpers - Add env var save/restore to flask_app fixture to prevent test pollution - New blueprint tests: bookfusion routes (56), logs routes (27), dashboard errors (7) - New service tests: BackgroundJobService (30), ReadingDateService (30), BookMetadataService (12), ReadingService (10), ClientPoller (8) - New integration tests: settings hot-reload (10), sync concurrency (7) - New error path tests: helpers (12), matching (9), reading bp (8), API (17) * Address code review findings: error handling, modal convention, dead code - bookfusion.js: capture error parameter in all 12 catch blocks, surface error messages to users instead of generic "Error" text - confirm-modal.js: add null guard in _resolve() when modal partial is missing from the page - dashboard.js: replace native confirm()/alert() with PKModal.confirm() and PKModal.alert() per project convention - logs.js: remove undeclared lastLogTimestamp variable (dead code from extraction), replace with shownLogs.clear() - reading_service.py: add warning log to pull_started_at catch block that was silently falling back to today's date * Address code review findings: null guards, offset bug, import sorting, dead code - Guard PKModal public methods against missing modal partial - Fix double-increment of currentOffset in logs loadMore handler - Add null guards in batch-match.js and utils.js DOM access - Remove unused preselectedEb variable in match.js - Guard against book.abs_id being None in cover proxy - Default mock_abs_client.is_configured to False in test fixtures - Wrap flask_app fixture in try/finally for safe teardown - Remove unused imports and variables in test files - Fix ruff I001 import sorting across all affected files * Address remaining review findings: unify modals, fix state detection, clean up patterns - Replace native confirm()/alert()/prompt() with PKModal on tbr-detail, dashboard, reading-detail, and settings pages - Migrate settings.js custom modal system to shared PKModal API - Fix dashboard refreshPaused flag never resetting on button-close - Replace fragile textContent.includes() state detection in logs.js with filterPending boolean flag - Add visible error display for live log fetch failures - Fix double JSON.stringify in suggestions.js BookFusion flow * KoSync system overhaul: service extraction, document management, bug fixes Major refactoring and feature additions for the KoSync subsystem: Service extraction: - Extract 375 lines of business logic from kosync_server.py into new KosyncService class (src/services/kosync_service.py) - Decompose _try_find_epub_by_hash (151 lines) into 3 focused methods - Remove dead code: _hash_cache, unused repository methods KoSync Document Management page (/kosync-documents): - New page accessible from Settings > KoSync tab - Three sections: Healthy, Needs Attention, Stale (30+ days) - Actions: Link to Book (search), Link to Self, Create Book, Clear Hash, Unlink, Delete - Rich context: book titles, time-ago indicators, device vs bot labels - Dashboard "Pending Identification" section for unlinked hashes with reading progress Bug fixes: - Fix sync direction inversion: mixed text-matched and percentage-fallback normalization could elect wrong leader (Entitled at 39% over Booklore at 45%) - Fix Booklore get_text_from_current_state using wrong filename - Fix Booklore 2 not showing as pairing option when only BL2 enabled - Fix Booklore crash on books with no ebook filename - Fix ebook-only books showing as unlinked (linked_abs_id vs linked_book_id) - Fix Link to Self sending empty body (Flask 400) - Fix external KoSync server missing credential fields and secret handling - Prevent orphaned hashes by creating KosyncDocument on every book save Improvements: - Rename abs-kosync-bot to pagekeeper-bot, centralize in constants.py - Remove legacy bot names (book-stitch, book-sync) - Redesign KoSync settings tab: sync source at top, conditional sections - Auto-create books for exact ABS title matches (skip suggestion approval) - Downgrade noisy no-progress warnings to debug - Include book title in Instant Sync log message - Add external KoSync server credential fields (KOSYNC_SERVER_USER/KEY) * Address code review findings: security, data integrity, dead code Critical fixes: - XSS: Replace |safe with |tojson for JSON in templates (kosync_documents, suggestions) — prevents script injection via book titles - Path traversal: Use Path().name to strip directory components from Booklore filenames before cache write, add is_safe_path_within check - Data integrity: Unlink/delete endpoints now clear book.kosync_doc_id to prevent recreating orphaned hashes - Primary key mutation: Never mutate document_hash on cached doc — delete old record and create new one instead Other fixes: - Sanitize book.title in Instant Sync and Booklore log messages - Settings: server-side radio state for builtin/external KoSync mode - _test_kosync uses urlparse-based external detection for credential selection - _is_external uses urlparse instead of fragile string matching - Null guards in toggleKosyncSourceMode JS - CSS: unquote Outfit font-family - Remove dead get_all_books() call in Booklore discovery - Strip server_id prefix from Booklore-cached filenames for title derivation - Remove unused json import from matching_bp * Address Macroscope review: linked_abs_id, LibraryService args, null guard, dead code * Address remaining review findings: atomicity, metadata consistency, dead code - Fix book_id None producing malformed "server_id:None" in Booklore search - Return cached_doc.filename for consistent Booklore path format - Reorder resolve-orphan to register hash before mutating books - Populate filename on existing KosyncDocument when linking - Add filename to register_hash_for_book new-doc path - Expand localhost detection to cover localhost/::1 variants - Add KOSYNC_SERVER_KEY to secret reveal whitelist - Add credential fallback for external KoSync test - Log instead of swallowing exceptions in _find_epub_in_db - Extract _serialize_document helper to deduplicate listing code - Fix isnot(None) style to match repo convention * Fix null guard, prefix stripping, enable toggle visibility, secret classification - Use getattr for raw_metadata_dict to handle None values safely - Only strip numeric prefixes (Booklore server IDs) from cached filenames - Move KoSync Enable toggle to always-visible Sync Source section - Remove KOSYNC_SERVER_USER from SECRET_SETTING_KEYS * Decompose EbookParser god class into focused modules Extract KoReaderXPathService and LocatorSearchService from the 1,140-line EbookParser, reducing it to a 335-line facade. The two new stateless services receive (full_text, spine_map) as arguments, making them independently testable without file I/O or mocking. - koreader_xpath.py: XPath generation/resolution, get_perfect_ko_xpath broken into 3 phases (text node location, hybrid BS4→LXML anchor mapping, BS4 structural fallback) - locator_search.py: text search (anchor/exact/normalized/fuzzy), CFI resolution, Storyteller/Readium locator resolution - Remove dead code: get_character_delta(), _has_text_content() - Add 46 new unit tests (23 xpath, 23 locator search) - Zero caller/DI changes — EbookParser facade preserves the public API * Refactor KoSync server: split monolith, extract utilities, add tests Split the 788-line kosync_server.py into focused modules: - kosync_server.py (123 lines): thin protocol route handlers - kosync_admin.py (284 lines): dashboard management routes - kosync_auth.py (121 lines): shared auth decorators Extract reusable utilities: - rate_limiter.py: TokenBucketRateLimiter (thread-safe token bucket) - debounce_manager.py: DebounceManager (PUT event debouncing) Move PUT/GET business logic from route handlers to KosyncService: - handle_put_progress: validation, furthest-wins, save, link, activity flag - handle_get_progress: 4-step lookup chain - resolve_best_progress: sibling doc selection + state fallback Eliminate module-level globals — services stored in Flask app.config, matching the existing blueprint helpers pattern. Add 38 new tests (20 service, 7 rate limiter, 11 debounce). Update 3 existing tests for new architecture. * Security audit: fix unauthenticated secret endpoint, path traversal, XSS, add hardening - Wire up _is_secret_request_authorized() in get_secret() (was defined but never called) - Add is_safe_path_within() checks in covers endpoint and ebook cache fallback - Apply sanitize_html filter to TBR description template - Add X-Frame-Options and X-Content-Type-Options security headers - Run container as non-root appuser in Dockerfile * fix(security,perf): defusedxml, dead code cleanup, N+1 query, DOM perf Security: - Replace xml.etree.ElementTree with defusedxml in CWA client and SMIL extractor to prevent XML entity expansion attacks - Remove dead session['is_admin'] check from secret auth function - Mask ABS API token in stream URL log messages - Set SESSION_COOKIE_SAMESITE=Lax and SESSION_COOKIE_HTTPONLY=True Performance: - Fix N+1 query in /api/processing-status using existing bulk method - Remove redundant rglob("*") fallback in resolve_book_path - Add 150ms debounce to reading page search input - Use DocumentFragment for dashboard sort to batch DOM reflows - Skip sorting hidden dashboard grids Infrastructure: - Add defusedxml==0.7.1 to requirements - Run test container as root for pip install compatibility * fix: SMIL indentation bug, restore secret reveal auth, sort hidden grids Address PR #3 review feedback from Macroscope: - Fix incorrect nesting of relative timestamp handler inside absolute block - Restore session-based admin check for browser secret reveal endpoint - Remove offsetParent guard so hidden dashboard grids get sorted * fix: remove unused variable to pass ruff CI * fix(security): sanitize KoSync endpoint inputs for Snyk XSS findings Validate doc_id format on GET progress, type-check request body on PUT. * fix(security): use make_response with explicit content type to break Snyk taint chain * fix: address PR #4 review findings from Macroscope - Guard suggestion cleanup when kept_ids is empty (ABS outage safety) - Move thread-start check inside lock in debounce_manager (race fix) - Remove offset addition on empty text nodes in xpath resolver - Iterate sentence tags in document order, not tag priority order - Map dismissed status to hidden in serialize_suggestion - Fix refreshPaused scoping: move to module scope in dashboard.js - Re-read localStorage in suggestions.js viewport change handler * fix: distinguish Storyteller-only from ebook-only in batch match UI * fix: map dismissed status to hidden in serialize_suggestion * fix: handle IPv4-mapped IPv6 addresses in private IP check * Align Hardcover API usage with docs, cache read IDs (#6) * Fix remaining book_id migration issues, guard Booklore cache, scope suggestions (#20) Completes the ABS ID decoupling by fixing service/repository methods that still used abs_id as lookup keys, removing 19 dead backward-compat methods, and cleaning up unnecessary abs_id parameters. Key changes: - Fix reading_stats, alignment, storyteller, dashboard lookups to use book_id - Guard Booklore cache loading behind is_configured() for unconfigured instances - Scope suggestion operations by (source_id, source) composite key with unique index migration, preventing collisions across ABS/KoSync/Booklore - Remove dead is_hash_linked_to_device methods from kosync and suggestion repos - Add 14 new tests for book_id resolution, suggestion scoping, and alignment ops All 458 tests passing. * Fix database upgrade safety issues from v0.1.4 compatibility review - Guard save_state() against double-NULL book_id/abs_id lookup - Isolate per-column error handling in _ensure_model_columns - Log orphaned rows in nullable table backfill migration - Remove dead delete_hardcover_details_by_book_id method * Fix abs_id→book_id migration gaps from CodeRabbit review (#50) Fixes 6 issues found during v0.1.5 PR review: - Restore _rdAbsId JS variable in reading_detail.html (all action buttons broken) - Key KoSync debounce, poll cache, and write-suppression by book.id not abs_id (ebook-only books have abs_id=None, collapsing all into one dict entry) - Fix link_kosync_document to set linked_abs_id for backward compat; query linked/unlinked docs by linked_book_id (the canonical FK) - Guard get_book_by_abs_id(None) with early return - Gate Base.metadata.create_all() on migration success * Smart mode defaults: auto-detect available services Default to Ebook Only mode when ABS is not configured. Detect all ebook sources (Booklore, CWA, ABS ebook libs, local /books mount). Disable mode buttons that have no backing service. Update subtitle from "ABS library" to "audiobook library". * Fix missing BookFusion covers and broken onerror fallback Skip ABS cover proxy for bf- prefixed books (always 404'd), deduplicate dashboard cover waterfall into resolve_book_covers(), fix onerror chain so placeholder shows when KoSync fallback also fails, add branded BookFusion placeholder logo. * Consolidate suggestion serializer and remove dismissed status Move _serialize_suggestion into helpers.py as shared utility, removing duplicate definitions from api.py and matching_bp.py. Unify dismissed → hidden status throughout suggestion_repository. Allow suggestion rescan to proceed when ABS is unconfigured (BookFusion-only setups). Pass storyteller_configured flag to match/batch_match templates. * Hide Storyteller UI when unconfigured and fix ABS cover proxy fallback Conditionally hide Storyteller column in match/batch_match when the integration is not configured. ABS cover proxy now falls back to using the raw book_ref as abs_id when no book record exists, allowing direct ABS ID lookups without a mapped book. * Show service logo placeholder when book cover is unavailable Add placeholder_logo field to mapping/book data dicts, determined by primary source (BookFusion, Booklore, or Audiobookshelf). Display the logo in all cover placeholder divs across dashboard, reading log, reading detail, and backlog cards. * Deduplicate placeholder_logo logic, fix cover proxy streaming, and fix N+1 query Extract resolve_placeholder_logo() into cover_resolver.py and return it from resolve_book_covers(), removing duplicate 4-branch conditionals from dashboard.py and reading_bp.py. Drop unnecessary stream=True from cover proxy requests that immediately buffer via .content. Bulk-fetch Hardcover details on the reading page to avoid per-book N+1 queries. * KoSync system overhaul: service extraction, document management, bug fixes Major refactoring and feature additions for the KoSync subsystem: Service extraction: - Extract 375 lines of business logic from kosync_server.py into new KosyncService class (src/services/kosync_service.py) - Decompose _try_find_epub_by_hash (151 lines) into 3 focused methods - Remove dead code: _hash_cache, unused repository methods KoSync Document Management page (/kosync-documents): - New page accessible from Settings > KoSync tab - Three sections: Healthy, Needs Attention, Stale (30+ days) - Actions: Link to Book (search), Link to Self, Create Book, Clear Hash, Unlink, Delete - Rich context: book titles, time-ago indicators, device vs bot labels - Dashboard "Pending Identification" section for unlinked hashes with reading progress Bug fixes: - Fix sync direction inversion: mixed text-matched and percentage-fallback normalization could elect wrong leader (Entitled at 39% over Booklore at 45%) - Fix Booklore get_text_from_current_state using wrong filename - Fix Booklore 2 not showing as pairing option when only BL2 enabled - Fix Booklore crash on books with no ebook filename - Fix ebook-only books showing as unlinked (linked_abs_id vs linked_book_id) - Fix Link to Self sending empty body (Flask 400) - Fix external KoSync server missing credential fields and secret handling - Prevent orphaned hashes by creating KosyncDocument on every book save Improvements: - Rename abs-kosync-bot to pagekeeper-bot, centralize in constants.py - Remove legacy bot names (book-stitch, book-sync) - Redesign KoSync settings tab: sync source at top, conditional sections - Auto-create books for exact ABS title matches (skip suggestion approval) - Downgrade noisy no-progress warnings to debug - Include book title in Instant Sync log message - Add external KoSync server credential fields (KOSYNC_SERVER_USER/KEY) * Address remaining review findings: error handling, atomicity, TypeError guard * fix(hardcover): align API usage with Hardcover docs, cache read IDs - Replace undocumented `public` field with `privacy_setting_id` on lists - Add `search_by_asin` using dedicated `editions.asin` field - Prefer `reading_format_id` over undocumented format string fields - Cache `user_book_read_id` to skip extra API call per progress update - Add `distinct_on: book_id` to user_books queries per docs - Increase request timeout from 10s to 20s (server allows 30s) - Clamp ratings to 0-5 range with 0.5 increments - Add `get_book_series` method for series metadata - Extract dominant color from `cached_image` into `cover_color` * fix: address PR #6 review findings - Add author/subtitle to save_book update_attrs (silent field drops) - Prefer linked_book_id over linked_abs_id in KoSync lookups (ebook-only support) - Persist book before passing to _get_or_create_user_book (id=None guard) - Remove max(ts_gap, 1) clamp in alignment sentinel detection - Fix test_matching_errors to mock get_kosync_id_for_ebook (correct target) - Set linked_book_id=None on MagicMock kosync docs in tests * fix: address low-priority PR #6 review findings - Cast title/authors to str in bookfusion upload (type safety) - Use coalesce in get_latest_jobs_bulk join (NULL timestamp handling) - Replace sleep with threading.Event in concurrency test (deterministic) - Remove early break in auto_link_by_title (link all matches) * Proactive suggestion flow + cross-ebook pairings (#8) * Rename Booklore to Grimmory (#7) * refactor: rename Booklore to Grimmory (#49) Upstream project renamed from Booklore to Grimmory. Update all references across the codebase: classes, env vars (BOOKLORE_* to GRIMMORY_*), API routes, DB schema, templates, CSS, JS, tests, and documentation. Alembic migration renames table, columns, constraints, and migrates stored data values. Legacy booklore_cache.json fallback preserved for migration compatibility. Closes #49 * style: apply ruff format to touched files * fix: yield fixture to keep env patches active, construct valid ServiceState - test_grimmory_client: change return to yield so patch.dict stays active during test execution - abs_sync_client: construct ServiceState with all required fields in get_fallback_text instead of passing a bare dict * feat(suggestions): dashboard banner, navbar badge, KoSync triggers Surface pending suggestions proactively instead of requiring manual navigation to /suggestions. Add KoSync as a suggestion trigger source so ebooks started on KOReader can find matching ABS audiobooks. Dashboard banner: - Shows top 3 high-confidence suggestions with cover, match info - Map Now / Dismiss actions with source-aware API calls - Responsive grid, card removal animation, badge updates Navbar badge: - Pending suggestion count on every page via global template context - Lightweight COUNT query avoids loading full suggestion objects KoSync suggestion trigger: - queue_kosync_suggestion() derives title from filename, fuzzy-matches against ABS audiobooks via existing _find_abs_audiobook_matches() - Fires in handle_put_progress() when auto-discovery is unavailable - Source badge and correct abs_id mapping URLs on suggestions page Cleanup: - Extract _search_live_candidates(), _dedupe_matches() from 117-line _create_suggestion() - Named constant _MIN_CANDIDATE_SCORE replacing magic 0.72 - DRY _transition_status() for hide/unhide/ignore repository methods - Extract _has_bookfusion_evidence() helper - CSS design tokens for confidence chip colors * feat(suggestions): cross-ebook pairings, decouple from ABS Support suggestion pairings between any two ebook sources (Storyteller, Grimmory, KoSync) without requiring ABS as an anchor. Cross-ebook detection: - _build_ebook_source_candidates() collects Storyteller/Grimmory/KoSync books into searchable candidate lists - _check_cross_ebook_suggestions() matches ebook sources against each other with title-based dedup against existing ABS suggestions - Shared progress helpers extracted from _check_reverse_suggestions() Refactoring: - _save_reverse_suggestion() -> _save_suggestion_with_merge() accepting any source type, not just ABS - _cover_url_for() helper replaces 4 hardcoded /api/cover-proxy/ URLs - Consolidated KoSync suggestion creation from kosync_service into suggestion_service.queue_kosync_suggestion() - Removed dead audiobook_count property from PendingSuggestion model - Filter abs_audiobook provenance entries from serialized matches ABS gate removal: - Suggestions page, dashboard banner, navbar badge/link all work without ABS configured (still gated by SUGGESTIONS_ENABLED) Security: - Source allowlist validation on all suggestion API endpoints - BookFusion link endpoint supports non-ABS sources - Extended clear_stale_suggestions() for all source types * fix: address PR review findings from Macroscope - Use current_app instead of out-of-scope app in inject_global_vars - Set source-specific fields when creating books for non-ABS sources - Fix ABS match cover URL in KoSync suggestion flow * fix: source-aware book lookup and consistent title normalization - Add get_book_by_storyteller_uuid to book repository - Route non-ABS BookFusion link lookups by source type - Use _normalize_title consistently for all ABS title indexing * fix: set kosync_doc_id instead of ebook_filename for kosync source books * fix: remove unused mapped_kosync_ids variable * fix: address pre-release review findings (#9) - Allow KoSync suggestions without ABS client (cross-ebook fallback) - Fix stale suggestion cleanup guard (check all_abs_books, not kept_ids) - Fix TOCTOU race in save_pending_suggestion (single-session atomic) - Fix match score asymmetry: use full title weight when author absent - Expand _SECRET_ENV_VARS to cover all 12 secret keys - Fix redaction order in _test_conn_error (redact before truncate) - Add try/except to DebounceManager poll loop - Simplify KoSync elif to else, remove stale booklore sort key ref - Extract author from matched candidate for KoSync suggestions - Remove dead code: redundant is_suggestion_ignored, unused variables * fix: address Macroscope PR #53 review findings - suggestion_repository: filter NULL abs_id in NOT IN subquery - hardcover_client: prevent cached read_id from overwriting dates - bookfusion_bp: return 404 when abs_id provided but book missing - bookfusion_bp: handle JSON null in title/authors without str() - koreader_xpath: fix offset calculation when falling back to parent - koreader_xpath: fix BS4/LXML text ordering divergence in hybrid anchor - kosync_service: use calendar.timegm for correct UTC timestamp - kosync_service: return early on discovery error to avoid misleading log - test_koreader_xpath: fix overly permissive assertion * fix(kosync): use datetime.min instead of 0 for last_updated fallback Prevents TypeError when comparing datetime against int in max(). * fix(storyteller): handle DNS failures cleanly * chore(git): add versioned branch workflow tooling * chore(git): strip private-only files from dev * fix(git): keep cleanup scripts from deleting untracked files * chore(git): ignore private local notes directory * test(bookfusion): cover client internals * fix(docker): set HF_HOME for faster-whisper model caching
Summary
templates/partials/navbar.html)<header>with hamburger menu, consistent links (Settings, Logs, Batch, Add Book), and stricter conditional checks for library icons (Booklore, Shelfmark){% include 'partials/navbar.html' %}Test plan
/from every pageSummary by CodeRabbit