Merge dev into main for v0.1.9 release

[serabi]

Summary

• fix(docker): set HF_HOME for faster-whisper model caching
• test(bookfusion): cover client internals
• chore(git): ignore private local notes directory
• fix(git): keep cleanup scripts from deleting untracked files
• chore(git): strip private-only files from dev
• chore(git): add versioned branch workflow tooling
• fix(storyteller): handle DNS failures cleanly
• fix(kosync): use datetime.min instead of 0 for last_updated fallback
• fix: address Macroscope PR Release: security hardening, KoSync refactor, suggestions, Grimmory rename #53 review findings
• fix: address pre-release review findings
• feat: proactive suggestion flow + cross-ebook pairings
• fix: align Hardcover API usage with docs, cache read IDs
• fix: security audit fixes + refactors

Note

Rename Booklore to Grimmory and release v0.1.9

• Renames the Booklore integration to Grimmory throughout the codebase: environment variables (BOOKLORE_* → GRIMMORY_*), DB table (booklore_books → grimmory_books), ORM model (BookloreBook → GrimmoryBook), sync client, DI container, API client, and all UI labels/icons.
• Adds a database migration (5308a8e2c930) that renames the table and column, updates states.client_name rows, and migrates settings keys; the migration is reversible.
• Migrates all internal ID references from abs_id/book.abs_id to the numeric book.id across alignment, write-tracker, BookFusion, Storyteller, TBR, and suggestion services.
• Adds KosyncService, DebounceManager, TokenBucketRateLimiter, KoReaderXPathService, and LocatorSearchService as new service/utility classes; refactors EbookParser to delegate XPath and text-search to those services.
• Adds a KoSync admin API and /kosync-documents management page; suggestion operations are now scoped by (source_id, source); cover proxying caches fetched images locally and serves from cache on upstream failure.
• Rewrites large blocks of frontend JavaScript into external files (batch-match.js, match.js, bookfusion.js, logs.js, suggestions.js, confirm-modal.js, kosync-documents.js) and removes equivalent inline <script> blocks from templates.
• Risk: record_write and is_own_write parameter renamed from abs_id to book_id; any keyword callers will break. EbookParser.get_character_delta is removed. PendingSuggestion.audiobook_count property is removed. The settings_page.test_connection endpoint now expects 'grimmory'/'grimmory2' instead of 'booklore'/'booklore2'.

📊 Macroscope summarized fa59918. 71 files reviewed, 28 issues evaluated, 16 issues filtered, 0 comments posted

🗂️ Filtered Issues

src/api/grimmory_client.py — 0 comments posted, 2 evaluated, 2 filtered

• line 357: Direct dictionary access book["id"] at line 357 will raise KeyError if any book in the API response lacks an "id" field. The code iterates over all_books_list which contains all books from the API batch, but doesn't guard against missing IDs. Should use book.get("id") with appropriate handling for None. [ Posting failed ]
• line 411: Direct dictionary access detail["id"] at line 411 will raise KeyError if the detail dict lacks an "id" key. Line 395 uses detail.get("id") safely, but line 411 uses direct bracket access. Should use detail.get("id") and handle the case where it's None. [ Out of scope ]

src/api/hardcover_client.py — 0 comments posted, 1 evaluated, 1 filtered

• line 811: When cached_read_id is provided, the code sets started_at and finished_at to the literal string "cached" (line 811). This causes two bugs: (1) The truthy string prevents the auto-population logic on lines 834 and 838 from ever running since not "cached" is False, so start/finish dates are never auto-set. (2) The invalid string "cached" is then passed to the GraphQL mutation variables (lines 863-864, 885-886) which expect a date type, likely causing API validation errors or data corruption. The sentinel should be None (falsy) rather than "cached" (truthy string). [ Posting failed ]

src/blueprints/bookfusion_bp.py — 0 comments posted, 1 evaluated, 1 filtered

• line 269: At line 269, reading_svc.update_status(abs_id, target_status, container) passes abs_id (a string like "bf-123") but ReadingService.update_status expects a numeric book_id and internally calls self.database_service.get_book_by_id(book_id). This will fail to find the book and return {"success": False, "error": "Book not found"}, silently failing to update the status. Should use book.id instead of abs_id. [ Out of scope ]

src/services/kosync_service.py — 0 comments posted, 1 evaluated, 1 filtered

• line 761: On line 761, int(latest_state.last_updated) will raise a TypeError because datetime objects cannot be directly converted to int. The code should call .timestamp() first, like elsewhere in the file (e.g., line 90: int(doc.timestamp.timestamp())). This should be int(latest_state.last_updated.timestamp()). [ Failed validation ]

src/services/suggestion_service.py — 0 comments posted, 3 evaluated, 2 filtered

• line 560: If self.storyteller_client.get_all_positions_bulk() returns None instead of raising an exception, the call to positions.items() on line 560 will raise AttributeError: 'NoneType' object has no attribute 'items'. This error occurs outside the try/except block. Add a guard like positions = ... or {} on line 554 or check for None before iterating. [ Cross-file consolidated ]
• line 938: The refactoring removed the status preservation logic that existed in the old code. Previously, when updating an existing suggestion, the code preserved the status field if it was 'hidden' or 'dismissed': status='hidden' if existing and getattr(existing, 'status', None) in ('hidden', 'dismissed') else 'pending'. Now the PendingSuggestion is created without explicitly passing the status parameter, which defaults to 'pending'. This means that if a user dismisses or hides a suggestion, and then a rescan runs, the suggestion will be recreated with status='pending', overwriting the user's preference and resurfacing suggestions they already dismissed. [ Posting failed ]

src/utils/koreader_xpath.py — 0 comments posted, 3 evaluated, 3 filtered

• line 60: In generate_xpath, when position falls exactly on a spine boundary gap (the separator space between spine items), the generator expression on line 59 finds no matching item and falls back to spine_map[-1]. This results in local_pos = position - target_item["start"] being negative (e.g., if position=5 is in a gap and spine_map[-1] starts at 6000, local_pos=-5995). The subsequent _find_text_node call with a large negative local_pos will incorrectly return the first text node of the wrong spine item. This produces an XPath pointing to the end of the book instead of the correct location. The root cause is the 1-character gap in spine_map construction, but generate_xpath could defend against it by checking for negative local_pos values. [ Out of scope ]
• line 216: In _hybrid_anchor_to_lxml, the LXML text iteration order does not match BS4's document order, causing incorrect occurrence counting. The code checks el.text then iterates el's direct children to check child.tail. For HTML like <p>Hello <b>world</b> foo</p>, LXML iteration processes: p.text ("Hello "), then b.tail (" foo") when visiting p, then b.text ("world") when visiting b. This yields order ["Hello ", " foo", "world"]. But BS4's find_all(string=True) returns document order ["Hello ", "world", " foo"]. This mismatch means when occurrence_index is calculated from BS4, the wrong LXML element may be selected, producing an incorrect XPath. [ Posting failed ]
• line 395: The fallback logic at lines 395-399 for BS4 Tag elements is unreachable. This method is only called with lxml elements (from _nearest_crengine_anchor, _build_sentence_level_chapter_fallback_xpath, etc.), so isinstance(element, Tag) will always be False. If the lxml xpath lookup fails or returns no non-empty text nodes, the fallback won't help and the method will return None regardless. This is dead code rather than a bug, but if the intent was to support both lxml and BS4 elements, the lxml-only .xpath() call at line 388 would fail for BS4 elements before reaching the fallback anyway. [ Posting failed ]

src/utils/transcriber.py — 0 comments posted, 1 evaluated, 1 filtered

• line 131: The slice ebook_text[: max(len(ebook_text), len(smil_text_raw) * 2)] is a no-op because max(len(ebook_text), ...) is always >= len(ebook_text), so the slice returns the entire string. The comment on line 129 states the intent is to limit to "first 50k chars to save time", but max defeats this optimization entirely. Should likely use min instead of max to actually truncate the text. [ Out of scope ]

static/js/batch-match.js — 0 comments posted, 2 evaluated, 1 filtered

• line 61: Similarly for the button text, when hasAudiobook is false with both hasStoryteller and hasEbook true, the condition on line 61 (!hasAudiobook && hasEbook) matches and sets the button to "Add Ebook Only to Queue", even though storyteller is also selected. The else clause on line 65-66 with "Add to Queue" should handle this combined case, but it's never reached. [ Posting failed ]

tests/test_background_job_service.py — 0 comments posted, 4 evaluated, 3 filtered

• line 92: Mock setup uses db.get_books_by_status.return_value = [book] which returns the same book for ALL status queries (both "pending" and "failed_retry_later"). Other similar tests in this file (lines 115-117, 133-135, 151-153) correctly use side_effect with a lambda to return different values based on the status argument. This test could pass for the wrong reasons if the service happens to have guards that prevent double-processing, but it doesn't actually isolate the "pending book" behavior it claims to test. [ Failed validation ]
• line 200: Test claims "book becomes active" in docstring but never asserts book.status == "active". The test only verifies the phases were called, not the actual outcome claimed. Compare with test_success_updates_book_and_job (line 591) which properly asserts book.status == "active". [ Cross-file consolidated ]
• line 614: In test_success_updates_book_and_job, the assertion assert book.ebook_filename == "test.epub" on line 614 does not actually verify that the service sets this field. The _make_book() helper already initializes ebook_filename="test.epub" by default, so the assertion passes regardless of whether _phase_alignment modifies book.ebook_filename. To properly test that the service sets this field from the epub path, the book should be created with a different initial value, e.g., _make_book(ebook_filename="original.epub"). [ Posting failed ]

tests/test_matching_errors.py — 0 comments posted, 5 evaluated, 1 filtered

• line 267: The test claims to verify that suggestions with empty matches are "filtered out" but only asserts response.status_code == 200. There is no assertion verifying that the filtering actually occurred (e.g., checking that only one suggestion appears in the response, or that suggestion_no_matches was excluded). If the route returns 200 but doesn't actually filter out empty-match suggestions, this test would pass as a false positive. [ Posting failed ]

Summary by CodeRabbit

• Chores
  • Updated container image configuration for Hugging Face integration
  • Enhanced test coverage for BookFusion API integration

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 701cb3d9-2d68-4e8a-aa83-627d24e3a604

📥 Commits

Reviewing files that changed from the base of the PR and between 7e4b655 and 07141f5.

📒 Files selected for processing (3)

• .gitignore
• Dockerfile
• tests/test_bookfusion_client.py

---

📝 Walkthrough

Walkthrough

A new test suite for the BookFusion client is introduced, covering multipart body construction, authentication helpers, parsing utilities, and API interactions. Docker configuration is updated to manage Hugging Face cache locations, and the private directory is added to .gitignore.

Changes

Cohort / File(s)	Summary
Build & Configuration .gitignore, Dockerfile	.gitignore excludes private/ directory. Dockerfile sets HF_HOME=/data/huggingface environment variable and creates corresponding cache directory with proper ownership.
Test Suite tests/test_bookfusion_client.py	New comprehensive test module validating BookFusion client internals: multipart body encoding, Calibre auth headers and digest generation, frontmatter/highlight parsing, client configuration gating, API connectivity, library/highlight fetching, and end-to-end sync flow with database persistence.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🧪 New tests bloom and Docker caches align,
With Hugging Face paths and private/ resign,
BookFusion now checked from every angle,
A suite of assertions, no room for the wrangle!

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}