feat(detected): replace suggestions with detected books#57
feat(detected): replace suggestions with detected books#57
Conversation
Track ABS and KoSync activity even when no pairing candidates\nexist so the dashboard can move to a true detected-book queue.\nKeep legacy suggestions intact while adding schema, repository,\nand backend detection support.
…o DetectedBook - Set SUGGESTIONS_ENABLED default to 'false' to disable legacy rescan - Add get_all_ebook_filenames() to DetectedRepository - Update sync_manager cache cleanup to use DetectedBook instead of PendingSuggestion - PendingSuggestion table remains for upgrade compatibility
- Remove /api/suggestions/* endpoints from api.py (7 routes removed) - Remove dismissSuggestion from dashboard.js - Remove clearStaleSuggestions from settings.js - Remove Clear Stale Suggestions button from settings.html - Keep serialize_suggestion import for matching_bp.py and dashboard.py (still used) - Keep suggestions.html, suggestions.js, suggestions.css as orphaned files (can delete later) Upgrade safe: PendingSuggestion table remains in database.
Delete suggestions.html, suggestions.js, suggestions.css - no longer referenced Keeps settings.html toggle for SUGGESTIONS_ENABLED env var (still used for rescan)
- Remove legacy suggestion banner from dashboard.py - Remove suggestions toggle from settings.html - Remove serialize_suggestion and _has_bookfusion_evidence from helpers.py - Remove unused imports from api.py, dashboard.py, matching_bp.py
- Add missing imports to dashboard.py (get_container, get_abs_service, etc.) - Add serialize_detected_book function to helpers.py - Import get_service_web_url and get_hardcover_book_url from service_url_helper
Stop creating PendingSuggestion rows for ABS detection and keep ABS progress discovery on the DetectedBook path only. Also pin mistune to 3.2.0 so the dev image can build again.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 50 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR replaces the suggestions system with a detected books system across the entire codebase. It adds database schema and models for tracking detected books, removes all suggestions-related API endpoints and UI, introduces detected books API endpoints and dashboard display, and updates services to persist detected book state and resolve them when books are mapped. Changes
Possibly related PRs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| if existing.status == "dismissed" and detected_book.status == "detected": | ||
| detected_book.status = "dismissed" |
There was a problem hiding this comment.
🟠 High db/detected_repository.py:43
The docstring promises "preserving dismissed status" but the implementation only preserves it when the incoming book has status == "detected". If a dismissed book is updated with status == "resolved", line 60 overwrites existing.status and the dismissal is lost. Consider removing the and detected_book.status == "detected" condition on line 43 so dismissed status is preserved regardless of incoming status.
- if existing.status == "dismissed" and detected_book.status == "detected":
+ if existing.status == "dismissed":🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/db/detected_repository.py around lines 43-44:
The docstring promises "preserving dismissed status" but the implementation only preserves it when the incoming book has `status == "detected"`. If a dismissed book is updated with `status == "resolved"`, line 60 overwrites `existing.status` and the dismissal is lost. Consider removing the `and detected_book.status == "detected"` condition on line 43 so dismissed status is preserved regardless of incoming status.
Evidence trail:
src/db/detected_repository.py lines 33-60 at REVIEWED_COMMIT:
- Line 34: docstring "Upsert a detected book while preserving dismissed status."
- Lines 43-44: conditional preservation `if existing.status == "dismissed" and detected_book.status == "detected": detected_book.status = "dismissed"`
- Line 60: unconditional overwrite `existing.status = detected_book.status`
| /* Suggestion banner dismiss */ | ||
| function dismissSuggestion(sourceId, source, btn) { | ||
| /* Detected card dismiss */ | ||
| function dismissDetected(sourceId, source, btn) { |
There was a problem hiding this comment.
🟠 High js/dashboard.js:857
Line 862 concatenates sourceId directly into the selector string without escaping, so sourceId values containing CSS special characters like ", \, or ] cause the selector to be malformed and return null instead of finding the card. The rest of this codebase uses CSS.escape() for this (lines 243, 344, 687). Consider escaping sourceId with CSS.escape() before concatenation.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file static/js/dashboard.js around line 857:
Line 862 concatenates `sourceId` directly into the selector string without escaping, so `sourceId` values containing CSS special characters like `"`, `\`, or `]` cause the selector to be malformed and return `null` instead of finding the card. The rest of this codebase uses `CSS.escape()` for this (lines 243, 344, 687). Consider escaping `sourceId` with `CSS.escape()` before concatenation.
Evidence trail:
static/js/dashboard.js lines 855-870 (line 862 shows unescaped sourceId in selector), lines 240-250 (line 243 shows CSS.escape usage), lines 341-350 (line 344 shows CSS.escape usage), lines 684-692 (line 687 shows CSS.escape usage) at REVIEWED_COMMIT in https://github.com/serabi/pagekeeper
There was a problem hiding this comment.
🟢 Low
test_create_book now only asserts against the return value of save_book, so it passes even if save_book returns the input object without persisting to the database. The lines that retrieved the record from the database and verified it exists were removed. Consider restoring get_book_by_abs_id verification to ensure the test actually checks persistence.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file tests/test_database_service_integration.py around line 63:
`test_create_book` now only asserts against the return value of `save_book`, so it passes even if `save_book` returns the input object without persisting to the database. The lines that retrieved the record from the database and verified it exists were removed. Consider restoring `get_book_by_abs_id` verification to ensure the test actually checks persistence.
Evidence trail:
tests/test_database_service_integration.py lines 63-79 (REVIEWED_COMMIT) shows current test only asserting against save_book return value. git_diff MERGE_BASE..REVIEWED_COMMIT shows removal of lines 82-85 which called get_book_by_abs_id(test_abs_id) and asserted the retrieved book was not None and had correct abs_id.
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/index.html (1)
104-142:⚠️ Potential issue | 🟠 MajorRender detected books even when there are no mapped books.
This new block is still inside the outer
{% if mappings %}gate at Line 38, so a user with only detected ABS/KoSync activity falls straight into the empty state and never sees these cards.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/index.html` around lines 104 - 142, The detected-books block guarded by "{% if detected_books %} ... {% endif %}" is currently nested inside the outer "{% if mappings %}" condition so users with only detected ABS/KoSync activity never see it; move the entire detected-books block (the "{% if detected_books %}" through its matching "{% endif %}") out of the outer "{% if mappings %}" scope so it renders independently, ensuring the template still checks detected_books but is not conditioned on "mappings" being truthy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@alembic/versions/t9u0v1w2x3y4_merge_detected_books_head.py`:
- Line 8: The import line "from typing import Sequence" is mis-ordered per
ruff/isort; update the imports in the migration file so imports are grouped and
alphabetized according to project rules (standard library, third-party, local)
and place "from typing import Sequence" into the correct group, or run
isort/ruff --fix on the file to automatically reorder and format the import
block; ensure the symbol Sequence remains imported and tests/CI pass.
In `@src/blueprints/dashboard.py`:
- Around line 11-20: The import block in src.blueprints.dashboard.py includes
unused symbols causing a Ruff lint failure; remove get_book_or_404,
get_grimmory_client, and serialize_detected_book from the import list (leave
find_grimmory_metadata, get_abs_service, get_container, get_database_service,
get_enabled_grimmory_server_ids, and get_grimmory_client only if actually used)
so only used helpers are imported; update the from src.blueprints.helpers import
(...) line to no longer reference the three unused names to resolve the lint
error.
- Around line 395-417: The current block swallows exceptions from
database_service.get_active_detected_books and duplicates mapping logic; change
it to call database_service.get_active_detected_books(...) inside a try/except
that catches exceptions but logs the error (including exception details) instead
of silently passing, and build detected_books by reusing the existing
serialize_detected_book(d) helper for each item (replace the manual dict
construction with serialize_detected_book(d)); ensure the logged message clearly
identifies the failure (e.g., "failed to load active detected books") and
includes the exception/stack for diagnostics.
In `@src/blueprints/helpers.py`:
- Around line 566-581: The new serialize_detected_book serializer was added but
callers still import/use the old name serialize_suggestion (e.g.,
tests/test_helpers.py and any other modules), causing import failures; either
update all imports and call sites to use serialize_detected_book and its field
names, or add a backwards-compatibility alias in this module (define
serialize_suggestion = serialize_detected_book) and update tests to use the new
name over time—search for serialize_suggestion, serialize_*suggestion*, and any
test helpers and replace them with serialize_detected_book or add the alias to
helpers.py to avoid breakage.
In `@src/db/detected_repository.py`:
- Around line 108-127: get_all_ebook_filenames currently only collects filenames
from DetectedBook.matches entries, so DetectedBook.ebook_filename values (used
by cleanup_cache keep-list) are omitted; update get_all_ebook_filenames to also
add detected.ebook_filename when present (e.g., after computing matches =
detected.matches or [] add a check like if detected.ebook_filename:
filenames.add(detected.ebook_filename)), keeping the existing session.expunge
behavior and returning the combined set so cleanup_cache receives row-level
ebook filenames too.
- Around line 33-68: In save_detected_book: when upserting an existing
DetectedBook, preserve a prior "resolved" status the same way "dismissed" is
preserved—i.e., if existing.status is "resolved" (or in general in
["dismissed","resolved"]) and the incoming detected_book.status is "detected"
then do not overwrite it; set detected_book.status (or keep existing.status) to
the existing value before assigning existing.status = detected_book.status so
resolved entries are not reopened; update the conditional that now checks only
for "dismissed" to include "resolved" and ensure existing.status is ultimately
preserved when appropriate.
In `@src/services/kosync_service.py`:
- Around line 626-639: The current guard around percentage (0.01 <= percentage
<= 0.70) prevents both queuing new suggestions and updating existing detected
progress; move the detected-book update outside that guard so progress is
refreshed for later PUTs. Specifically, keep calling
self._container.suggestion_service().queue_kosync_suggestion(...) only when
percentage is within the 0.01–0.70 range, but always call
self._db.get_detected_book(doc_hash, source="kosync") and, if present, set
detected.progress_percentage = float(percentage) and
self._db.save_detected_book(detected) regardless of the threshold; preserve the
try/except around the suggestion call or around both operations as appropriate
and reference suggestion_service(), queue_kosync_suggestion(),
get_detected_book(), save_detected_book(), and detected.progress_percentage to
locate the code.
In `@src/services/suggestion_service.py`:
- Around line 493-503: The call to self._upsert_detected_book always sets
progress_percentage=0.0 which wipes out real KoSync progress on refresh;
instead, fetch the current stored progress for this source/source_id (e.g., via
the existing get/fetch method for detected books) and pass that existing value
into _upsert_detected_book, or pass None and update
_upsert_detected_book/save_detected_book to only overwrite progress when a
non-None value is provided; update the call site (where _upsert_detected_book is
invoked) to stop hardcoding 0.0 and ensure
_upsert_detected_book/save_detected_book respect a None/no-op value so refreshes
do not reset progress.
In `@src/sync_manager.py`:
- Around line 241-242: cleanup_cache currently only protects filenames from
auto-discovery matches (via database_service.get_all_ebook_filenames()), but
detected-book rows that only set ebook_filename are omitted and may be deleted;
update cleanup_cache() to also fetch and include detected-only ebook filenames
by calling the database_service method that returns ebook_filename values for
detected rows (e.g., database_service.get_all_detected_ebook_filenames() or add
such a helper) and add those filenames to valid_filenames before deleting cached
files so detected EPUBs are preserved.
In `@static/js/dashboard.js`:
- Around line 859-863: The DOM removal only matches by data-source-id and can
remove the wrong card when different providers share the same sourceId; update
the selector used in the fetch success handler to match both attributes
(data-source-id and data-source) so it uniquely targets the DetectedBook tuple
(sourceId, source). In the fetch .then callback where you build var card =
document.querySelector('.detected-card[data-source-id="' + sourceId + '"]'),
change that selector to also include the data-source attribute (using the same
source variable or default 'abs' as used in the request) so it selects
'.detected-card[data-source-id="..."][data-source="..."]' before removing the
element.
In `@templates/index.html`:
- Around line 127-131: The template renders an "Add as Ebook" link even when
d.ebook_filename is empty, producing a dead
/match?ebook_filename=&action=ebook_only link; update the kosync branch of the
template to conditionally render the second anchor only when d.ebook_filename is
truthy (non-empty) by checking d.ebook_filename before emitting the <a> tag so
the "Add as Ebook" button is hidden when no ebook filename exists.
In `@tests/test_database_service_integration.py`:
- Around line 82-127: The new assertions referencing test_abs_id were
accidentally left inside test_resolve_detected_book_scoped_by_source where
test_abs_id is not defined; move the retrieval/assertion lines that use
test_abs_id back into the original test_create_book or alternatively define and
set test_abs_id earlier in test_resolve_detected_book_scoped_by_source so the
variable exists; locate uses of test_abs_id in the test file (look for the
symbol test_abs_id and the functions test_resolve_detected_book_scoped_by_source
and test_create_book) and either relocate the assertions to test_create_book or
initialize test_abs_id appropriately to restore correct test scope and coverage.
In `@tests/test_suggestion_logic.py`:
- Around line 89-90: Update the test to remove the legacy PendingSuggestion DB
write assertion: delete or stop asserting
mock_db.save_pending_suggestion.assert_called_once(), and retain/ensure the
detected-book expectation by keeping
mock_db.save_detected_book.assert_called_once() (or equivalent assert that
save_detected_book was called exactly once); this aligns the test with the PR
that removed the pending-suggestion write path.
---
Outside diff comments:
In `@templates/index.html`:
- Around line 104-142: The detected-books block guarded by "{% if detected_books
%} ... {% endif %}" is currently nested inside the outer "{% if mappings %}"
condition so users with only detected ABS/KoSync activity never see it; move the
entire detected-books block (the "{% if detected_books %}" through its matching
"{% endif %}") out of the outer "{% if mappings %}" scope so it renders
independently, ensuring the template still checks detected_books but is not
conditioned on "mappings" being truthy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 93f656c3-b54f-433d-b7b6-4a1bd70df3ef
⛔ Files ignored due to path filters (1)
scripts/git/README.mdis excluded by!**/*.md
📒 Files selected for processing (30)
.githooks/pre-pushalembic/versions/s1t2u3v4w5x6_add_detected_books_table.pyalembic/versions/t9u0v1w2x3y4_merge_detected_books_head.pyrequirements.txtscripts/git/create-draft-branch.shscripts/git/promote.shscripts/git/setup-worktrees.shscripts/git/sync-private-mirrors.shsrc/blueprints/api.pysrc/blueprints/dashboard.pysrc/blueprints/helpers.pysrc/blueprints/matching_bp.pysrc/db/database_service.pysrc/db/detected_repository.pysrc/db/models.pysrc/services/kosync_service.pysrc/services/suggestion_service.pysrc/sync_manager.pystatic/css/kosync.cssstatic/css/suggestions.cssstatic/js/dashboard.jsstatic/js/settings.jsstatic/js/suggestions.jstemplates/index.htmltemplates/partials/navbar.htmltemplates/settings.htmltemplates/suggestions.htmltests/test_database_service_integration.pytests/test_queue_suggestion.pytests/test_suggestion_logic.py
💤 Files with no reviewable changes (9)
- templates/partials/navbar.html
- .githooks/pre-push
- static/js/settings.js
- templates/suggestions.html
- templates/settings.html
- static/css/suggestions.css
- scripts/git/create-draft-branch.sh
- static/js/suggestions.js
- scripts/git/sync-private-mirrors.sh
| # Active detected books — for dashboard detected section | ||
| detected_books = [] | ||
| try: | ||
| active_detected = database_service.get_active_detected_books(limit=10) | ||
| for d in active_detected: | ||
| detected_books.append( | ||
| { | ||
| "id": d.id, | ||
| "source": d.source, | ||
| "source_id": d.source_id, | ||
| "title": d.title, | ||
| "author": d.author, | ||
| "cover_url": d.cover_url, | ||
| "progress_percentage": d.progress_percentage, | ||
| "first_detected_at": d.first_detected_at.isoformat() if d.first_detected_at else None, | ||
| "last_seen_at": d.last_seen_at.isoformat() if d.last_seen_at else None, | ||
| "matches": d.matches, | ||
| "device": d.device, | ||
| "ebook_filename": d.ebook_filename, | ||
| } | ||
| ) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Don’t swallow detected-book load failures.
If get_active_detected_books() starts failing during rollout, the dashboard quietly degrades to an empty list and gives you nothing in logs to diagnose it. Reusing serialize_detected_book() here also removes the duplicated mapping code.
Suggested change
try:
active_detected = database_service.get_active_detected_books(limit=10)
- for d in active_detected:
- detected_books.append(
- {
- "id": d.id,
- "source": d.source,
- "source_id": d.source_id,
- "title": d.title,
- "author": d.author,
- "cover_url": d.cover_url,
- "progress_percentage": d.progress_percentage,
- "first_detected_at": d.first_detected_at.isoformat() if d.first_detected_at else None,
- "last_seen_at": d.last_seen_at.isoformat() if d.last_seen_at else None,
- "matches": d.matches,
- "device": d.device,
- "ebook_filename": d.ebook_filename,
- }
- )
- except Exception:
- pass
+ detected_books = [serialize_detected_book(d) for d in active_detected]
+ except Exception:
+ logger.exception("Failed to load detected books for dashboard")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/blueprints/dashboard.py` around lines 395 - 417, The current block
swallows exceptions from database_service.get_active_detected_books and
duplicates mapping logic; change it to call
database_service.get_active_detected_books(...) inside a try/except that catches
exceptions but logs the error (including exception details) instead of silently
passing, and build detected_books by reusing the existing
serialize_detected_book(d) helper for each item (replace the manual dict
construction with serialize_detected_book(d)); ensure the logged message clearly
identifies the failure (e.g., "failed to load active detected books") and
includes the exception/stack for diagnostics.
| def serialize_detected_book(d): | ||
| """Serialize DetectedBook for template context.""" | ||
| return { | ||
| "id": d.id, | ||
| "source": d.source, | ||
| "source_id": d.source_id, | ||
| "title": d.title, | ||
| "author": d.author, | ||
| "cover_url": d.cover_url, | ||
| "progress_percentage": d.progress_percentage, | ||
| "first_detected_at": d.first_detected_at.isoformat() if d.first_detected_at else None, | ||
| "last_seen_at": d.last_seen_at.isoformat() if d.last_seen_at else None, | ||
| "matches": d.matches, | ||
| "device": d.device, | ||
| "ebook_filename": d.ebook_filename, | ||
| } |
There was a problem hiding this comment.
Detected serializer migration is incomplete across helper consumers
serialize_detected_book is added, but existing imports/usages of removed suggestion helpers (e.g., tests/test_helpers.py importing serialize_suggestion) must be migrated in the same PR or CI will fail on import/use paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/blueprints/helpers.py` around lines 566 - 581, The new
serialize_detected_book serializer was added but callers still import/use the
old name serialize_suggestion (e.g., tests/test_helpers.py and any other
modules), causing import failures; either update all imports and call sites to
use serialize_detected_book and its field names, or add a
backwards-compatibility alias in this module (define serialize_suggestion =
serialize_detected_book) and update tests to use the new name over time—search
for serialize_suggestion, serialize_*suggestion*, and any test helpers and
replace them with serialize_detected_book or add the alias to helpers.py to
avoid breakage.
| def save_detected_book(self, detected_book): | ||
| """Upsert a detected book while preserving dismissed status.""" | ||
| filters = [ | ||
| DetectedBook.source_id == detected_book.source_id, | ||
| DetectedBook.source == detected_book.source, | ||
| ] | ||
| with self.get_session() as session: | ||
| existing = session.query(DetectedBook).filter(*filters).first() | ||
| now = datetime.now(UTC) | ||
| if existing: | ||
| if existing.status == "dismissed" and detected_book.status == "detected": | ||
| detected_book.status = "dismissed" | ||
|
|
||
| if detected_book.title: | ||
| existing.title = detected_book.title | ||
| if detected_book.author: | ||
| existing.author = detected_book.author | ||
| if detected_book.cover_url: | ||
| existing.cover_url = detected_book.cover_url | ||
| if detected_book.matches_json is not None: | ||
| existing.matches_json = detected_book.matches_json | ||
| if detected_book.device: | ||
| existing.device = detected_book.device | ||
| if detected_book.ebook_filename: | ||
| existing.ebook_filename = detected_book.ebook_filename | ||
|
|
||
| existing.progress_percentage = detected_book.progress_percentage | ||
| existing.status = detected_book.status | ||
| existing.last_seen_at = detected_book.last_seen_at or now | ||
| if existing.first_detected_at is None: | ||
| existing.first_detected_at = detected_book.first_detected_at or now | ||
|
|
||
| session.flush() | ||
| session.refresh(existing) | ||
| session.expunge(existing) | ||
| return existing |
There was a problem hiding this comment.
❓ Verification inconclusive
In SQLAlchemy ORM, when a mapped column is defined as Column(String(20), default="detected"), is the Python attribute populated when the model instance is constructed, or only during INSERT/flush?
In SQLAlchemy ORM, when a mapped column is defined as Column(String(20), default="detected") in a standard declarative mapping (not using MappedAsDataclass or native dataclasses), the Python attribute is not populated with the default value when the model instance is constructed. The default is applied only during the INSERT statement (i.e., during flush/commit), when no value is provided for that column.
Citations:
- 1: Understanding default and default_factory in mapped_column() sqlalchemy/sqlalchemy#11372
- 2: https://stackoverflow.com/questions/13791487/how-to-apply-column-defaults-before-a-commit-in-sqlalchemy
- 3: https://stackoverflow.com/questions/14002631/why-isnt-sqlalchemy-default-column-value-available-before-object-is-committed
- 4: allow the possibility of column-level "default" values to populate before persistence (new event + recipe) sqlalchemy/sqlalchemy#1311
Script executed:
# First, let's see the DetectedBook model definition
find . -name "models.py" -path "*/db/*" | head -5Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
./src/db/models.py
Script executed:
# Find calls to save_detected_book to see how detected_book objects are created
rg "save_detected_book" -A 3 -B 3Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
tests/test_database_service_integration.py- self.assertEqual(saved_book.title, "Test Book Creation")
tests/test_database_service_integration.py- self.assertEqual(saved_book.status, "active")
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: def test_save_detected_book_creates_and_updates(self):
tests/test_database_service_integration.py- """Detected books upsert by source and source_id."""
tests/test_database_service_integration.py- from src.db.models import DetectedBook
tests/test_database_service_integration.py-
--
tests/test_database_service_integration.py- progress_percentage=0.25,
tests/test_database_service_integration.py- cover_url="/cover/1",
tests/test_database_service_integration.py- )
tests/test_database_service_integration.py: saved = self.db_service.save_detected_book(first)
tests/test_database_service_integration.py- self.assertEqual(saved.title, "Detected Title")
tests/test_database_service_integration.py- self.assertAlmostEqual(saved.progress_percentage, 0.25)
tests/test_database_service_integration.py-
--
tests/test_database_service_integration.py- author="Author Two",
tests/test_database_service_integration.py- progress_percentage=0.55,
tests/test_database_service_integration.py- )
tests/test_database_service_integration.py: updated = self.db_service.save_detected_book(second)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- self.assertEqual(updated.id, saved.id)
tests/test_database_service_integration.py- self.assertEqual(updated.title, "Detected Title Updated")
--
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- abs_detected = DetectedBook(source="abs", source_id="shared-id", title="ABS", progress_percentage=0.2)
tests/test_database_service_integration.py- kosync_detected = DetectedBook(source="kosync", source_id="shared-id", title="KOSync", progress_percentage=0.3)
tests/test_database_service_integration.py: self.db_service.save_detected_book(abs_detected)
tests/test_database_service_integration.py: self.db_service.save_detected_book(kosync_detected)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- self.assertTrue(self.db_service.resolve_detected_book("shared-id", source="abs"))
tests/test_database_service_integration.py-
--
tests/test_suggestion_logic.py-
tests/test_suggestion_logic.py- # Assert
tests/test_suggestion_logic.py- self.mock_db.save_pending_suggestion.assert_called_once()
tests/test_suggestion_logic.py: self.mock_db.save_detected_book.assert_called_once()
tests/test_suggestion_logic.py-
tests/test_suggestion_logic.py- def test_suggestion_ignored_when_hidden(self):
tests/test_suggestion_logic.py- """Test that suggestions are NOT created if they were previously hidden."""
--
tests/test_queue_suggestion.py- }
tests/test_queue_suggestion.py- self.manager.queue_suggestion("book-789")
tests/test_queue_suggestion.py- self.mock_abs.get_item_details.assert_called_once_with("book-789")
tests/test_queue_suggestion.py: self.mock_db.save_detected_book.assert_called_once()
tests/test_queue_suggestion.py-
tests/test_queue_suggestion.py- def test_thread_safety_prevents_duplicate(self):
tests/test_queue_suggestion.py- """Second concurrent call for same ID should be skipped."""
--
src/db/detected_repository.py- session.expunge(item)
src/db/detected_repository.py- return items
src/db/detected_repository.py-
src/db/detected_repository.py: def save_detected_book(self, detected_book):
src/db/detected_repository.py- """Upsert a detected book while preserving dismissed status."""
src/db/detected_repository.py- filters = [
src/db/detected_repository.py- DetectedBook.source_id == detected_book.source_id,
--
src/services/kosync_service.py- detected = self._db.get_detected_book(doc_hash, source="kosync")
src/services/kosync_service.py- if detected:
src/services/kosync_service.py- detected.progress_percentage = float(percentage)
src/services/kosync_service.py: self._db.save_detected_book(detected)
src/services/kosync_service.py- except Exception as e:
src/services/kosync_service.py- logger.debug(f"KOSync detected-book update failed for {doc_hash[:8]}...: {e}")
src/services/kosync_service.py-
--
src/services/suggestion_service.py- device=device,
src/services/suggestion_service.py- ebook_filename=ebook_filename,
src/services/suggestion_service.py- )
src/services/suggestion_service.py: return self.database_service.save_detected_book(detected)
src/services/suggestion_service.py-
src/services/suggestion_service.py- def _get_bookfusion_context(self) -> dict:
src/services/suggestion_service.py- try:
Script executed:
# Look at the DetectedBook model definition
fd "models.py" -path "*/db/*" -exec cat {} \;Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 2
stderr:
error: 'h' isn't a valid value for '--type <filetype>'
[possible values: file, directory, symlink, executable, empty, socket, pipe]
For more information try '--help'
Script executed:
# Search for DetectedBook instantiation patterns
rg "DetectedBook\(" -A 2 -B 2Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
tests/test_database_service_integration.py- from src.db.models import DetectedBook
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: first = DetectedBook(
tests/test_database_service_integration.py- source="abs",
tests/test_database_service_integration.py- source_id="detected-1",
--
tests/test_database_service_integration.py- self.assertAlmostEqual(saved.progress_percentage, 0.25)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: second = DetectedBook(
tests/test_database_service_integration.py- source="abs",
tests/test_database_service_integration.py- source_id="detected-1",
--
tests/test_database_service_integration.py- from src.db.models import DetectedBook
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: abs_detected = DetectedBook(source="abs", source_id="shared-id", title="ABS", progress_percentage=0.2)
tests/test_database_service_integration.py: kosync_detected = DetectedBook(source="kosync", source_id="shared-id", title="KOSync", progress_percentage=0.3)
tests/test_database_service_integration.py- self.db_service.save_detected_book(abs_detected)
tests/test_database_service_integration.py- self.db_service.save_detected_book(kosync_detected)
--
src/db/models.py-
src/db/models.py-
src/db/models.py:class DetectedBook(Base):
src/db/models.py- """Model for external books with real progress that are not yet in PageKeeper."""
src/db/models.py-
--
src/db/models.py- def __repr__(self):
src/db/models.py- return (
src/db/models.py: f"<DetectedBook(id={self.id}, source='{self.source}', source_id='{self.source_id}', "
src/db/models.py- f"status='{self.status}')>"
src/db/models.py- )
--
src/services/suggestion_service.py- ebook_filename: str | None = None,
src/services/suggestion_service.py- ):
src/services/suggestion_service.py: detected = DetectedBook(
src/services/suggestion_service.py- source=source,
src/services/suggestion_service.py- source_id=source_id,
Script executed:
# Read the DetectedBook model definition
cat src/db/models.py | grep -A 30 "class DetectedBook"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
class DetectedBook(Base):
"""Model for external books with real progress that are not yet in PageKeeper."""
__tablename__ = "detected_books"
__table_args__ = (UniqueConstraint("source_id", "source", name="uq_detected_books_source_id_source"),)
id = Column(Integer, primary_key=True, autoincrement=True)
source = Column(String(50), default="abs", nullable=False)
source_id = Column(String(255), nullable=False)
title = Column(String(500), nullable=True)
author = Column(String(500), nullable=True)
cover_url = Column(String(500), nullable=True)
progress_percentage = Column(Float, default=0.0, nullable=False)
first_detected_at = Column(DateTime, default=utc_now)
last_seen_at = Column(DateTime, default=utc_now, onupdate=utc_now)
status = Column(String(20), default="detected")
matches_json = Column(Text, nullable=True)
device = Column(String(128), nullable=True)
ebook_filename = Column(String(500), nullable=True)
def __init__(
self,
source_id: str,
title: str,
progress_percentage: float,
author: str = None,
cover_url: str = None,
status: str = "detected",
source: str = "abs",
matches_json: str = None,
device: str = None,
Script executed:
# Look at the suggestion_service to see how DetectedBook is created
rg -A 15 "detected = DetectedBook" src/services/suggestion_service.pyRepository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
detected = DetectedBook(
source=source,
source_id=source_id,
title=title or source_id,
author=author or "",
cover_url=cover_url,
progress_percentage=max(0.0, min(progress_percentage, 1.0)),
matches_json=json.dumps(matches or []),
device=device,
ebook_filename=ebook_filename,
)
return self.database_service.save_detected_book(detected)
def _get_bookfusion_context(self) -> dict:
try:
bf_books = list(self.database_service.get_bookfusion_books() or [])
Script executed:
# Check the kosync_service usage more carefully
rg -B 5 -A 5 "get_detected_book.*kosync" src/services/kosync_service.pyRepository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
suggestion_svc.queue_kosync_suggestion(
doc_hash,
filename=kosync_doc.filename,
device=device,
)
detected = self._db.get_detected_book(doc_hash, source="kosync")
if detected:
detected.progress_percentage = float(percentage)
self._db.save_detected_book(detected)
except Exception as e:
logger.debug(f"KOSync detected-book update failed for {doc_hash[:8]}...: {e}")
Script executed:
# Get the full __init__ method of DetectedBook
sed -n '/class DetectedBook/,/def [^_]/p' src/db/models.py | head -80Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
class DetectedBook(Base):
"""Model for external books with real progress that are not yet in PageKeeper."""
__tablename__ = "detected_books"
__table_args__ = (UniqueConstraint("source_id", "source", name="uq_detected_books_source_id_source"),)
id = Column(Integer, primary_key=True, autoincrement=True)
source = Column(String(50), default="abs", nullable=False)
source_id = Column(String(255), nullable=False)
title = Column(String(500), nullable=True)
author = Column(String(500), nullable=True)
cover_url = Column(String(500), nullable=True)
progress_percentage = Column(Float, default=0.0, nullable=False)
first_detected_at = Column(DateTime, default=utc_now)
last_seen_at = Column(DateTime, default=utc_now, onupdate=utc_now)
status = Column(String(20), default="detected")
matches_json = Column(Text, nullable=True)
device = Column(String(128), nullable=True)
ebook_filename = Column(String(500), nullable=True)
def __init__(
self,
source_id: str,
title: str,
progress_percentage: float,
author: str = None,
cover_url: str = None,
status: str = "detected",
source: str = "abs",
matches_json: str = None,
device: str = None,
ebook_filename: str = None,
):
self.source = source
self.source_id = source_id
self.title = title
self.author = author
self.cover_url = cover_url
self.progress_percentage = progress_percentage
self.status = status
self.matches_json = matches_json
self.device = device
self.ebook_filename = ebook_filename
self.first_detected_at = utc_now()
self.last_seen_at = utc_now()
`@property`
def matches(self):
Script executed:
# Also check if there are any other places creating DetectedBook without status
rg "DetectedBook(" --no-heading | sort | uniqRepository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stderr:
regex parse error:
DetectedBook(
^
error: unclosed group
Script executed:
# Search for DetectedBook instantiation patterns without escaping issues
rg "DetectedBook\s*\(" -A 8 | grep -v "^--$"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
tests/test_database_service_integration.py: first = DetectedBook(
tests/test_database_service_integration.py- source="abs",
tests/test_database_service_integration.py- source_id="detected-1",
tests/test_database_service_integration.py- title="Detected Title",
tests/test_database_service_integration.py- author="Author One",
tests/test_database_service_integration.py- progress_percentage=0.25,
tests/test_database_service_integration.py- cover_url="/cover/1",
tests/test_database_service_integration.py- )
tests/test_database_service_integration.py- saved = self.db_service.save_detected_book(first)
tests/test_database_service_integration.py: second = DetectedBook(
tests/test_database_service_integration.py- source="abs",
tests/test_database_service_integration.py- source_id="detected-1",
tests/test_database_service_integration.py- title="Detected Title Updated",
tests/test_database_service_integration.py- author="Author Two",
tests/test_database_service_integration.py- progress_percentage=0.55,
tests/test_database_service_integration.py- )
tests/test_database_service_integration.py- updated = self.db_service.save_detected_book(second)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: abs_detected = DetectedBook(source="abs", source_id="shared-id", title="ABS", progress_percentage=0.2)
tests/test_database_service_integration.py: kosync_detected = DetectedBook(source="kosync", source_id="shared-id", title="KOSync", progress_percentage=0.3)
tests/test_database_service_integration.py- self.db_service.save_detected_book(abs_detected)
tests/test_database_service_integration.py- self.db_service.save_detected_book(kosync_detected)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- self.assertTrue(self.db_service.resolve_detected_book("shared-id", source="abs"))
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- resolved = self.db_service.get_detected_book("shared-id", source="abs")
tests/test_database_service_integration.py- still_active = self.db_service.get_detected_book("shared-id", source="kosync")
tests/test_database_service_integration.py- self.assertEqual(resolved.status, "resolved")
src/db/models.py:class DetectedBook(Base):
src/db/models.py- """Model for external books with real progress that are not yet in PageKeeper."""
src/db/models.py-
src/db/models.py- __tablename__ = "detected_books"
src/db/models.py- __table_args__ = (UniqueConstraint("source_id", "source", name="uq_detected_books_source_id_source"),)
src/db/models.py-
src/db/models.py- id = Column(Integer, primary_key=True, autoincrement=True)
src/db/models.py- source = Column(String(50), default="abs", nullable=False)
src/db/models.py- source_id = Column(String(255), nullable=False)
src/db/models.py: f"<DetectedBook(id={self.id}, source='{self.source}', source_id='{self.source_id}', "
src/db/models.py- f"status='{self.status}')>"
src/db/models.py- )
src/db/models.py-
src/db/models.py-
src/db/models.py-class Setting(Base):
src/db/models.py- """
src/db/models.py- Setting model storing application configuration.
src/db/models.py- """
src/services/suggestion_service.py: detected = DetectedBook(
src/services/suggestion_service.py- source=source,
src/services/suggestion_service.py- source_id=source_id,
src/services/suggestion_service.py- title=title or source_id,
src/services/suggestion_service.py- author=author or "",
src/services/suggestion_service.py- cover_url=cover_url,
src/services/suggestion_service.py- progress_percentage=max(0.0, min(progress_percentage, 1.0)),
src/services/suggestion_service.py- matches_json=json.dumps(matches or []),
src/services/suggestion_service.py- device=device,
Script executed:
# Check the test cases to understand the actual behavior
cat tests/test_database_service_integration.py | grep -A 20 "test_save_detected_book_creates_and_updates"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
def test_save_detected_book_creates_and_updates(self):
"""Detected books upsert by source and source_id."""
from src.db.models import DetectedBook
first = DetectedBook(
source="abs",
source_id="detected-1",
title="Detected Title",
author="Author One",
progress_percentage=0.25,
cover_url="/cover/1",
)
saved = self.db_service.save_detected_book(first)
self.assertEqual(saved.title, "Detected Title")
self.assertAlmostEqual(saved.progress_percentage, 0.25)
second = DetectedBook(
source="abs",
source_id="detected-1",
title="Detected Title Updated",
author="Author Two",
Script executed:
# Check if resolve_detected_book exists and what it does
rg "resolve_detected_book|status.*resolved" -A 3 -B 3Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
tests/test_database_service_integration.py- self.assertEqual(updated.author, "Author Two")
tests/test_database_service_integration.py- self.assertAlmostEqual(updated.progress_percentage, 0.55)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: def test_resolve_detected_book_scoped_by_source(self):
tests/test_database_service_integration.py- """Resolving a detected book only affects the matching source row."""
tests/test_database_service_integration.py- from src.db.models import DetectedBook
tests/test_database_service_integration.py-
--
tests/test_database_service_integration.py- self.db_service.save_detected_book(abs_detected)
tests/test_database_service_integration.py- self.db_service.save_detected_book(kosync_detected)
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: self.assertTrue(self.db_service.resolve_detected_book("shared-id", source="abs"))
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- resolved = self.db_service.get_detected_book("shared-id", source="abs")
tests/test_database_service_integration.py- still_active = self.db_service.get_detected_book("shared-id", source="kosync")
tests/test_database_service_integration.py: self.assertEqual(resolved.status, "resolved")
tests/test_database_service_integration.py- self.assertEqual(still_active.status, "detected")
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- def test_delete_book(self):
--
src/services/kosync_service.py- )
src/services/kosync_service.py- self._db.save_book(book, is_new=True)
src/services/kosync_service.py- self._db.link_kosync_document(doc_hash, book.id, book.abs_id)
src/services/kosync_service.py: self._db.resolve_detected_book(doc_hash, source="kosync")
src/services/kosync_service.py- self._db.resolve_suggestion(doc_hash)
src/services/kosync_service.py- logger.info(
src/services/kosync_service.py- f"Auto-created book '{match['title']}' from exact title match (abs_id={match['abs_id']})"
--
src/services/kosync_service.py- )
src/services/kosync_service.py- self._db.save_book(book, is_new=True)
src/services/kosync_service.py- self._db.link_kosync_document(doc_hash, book.id, book.abs_id)
src/services/kosync_service.py: self._db.resolve_detected_book(doc_hash, source="kosync")
src/services/kosync_service.py- self._db.resolve_suggestion(doc_hash)
src/services/kosync_service.py- logger.info(f"Created ebook-only book: {book.id} '{title}'" + (f" -> {epub_filename}" if epub_filename else ""))
src/services/kosync_service.py-
--
src/blueprints/matching_bp.py- database_service.resolve_suggestion(abs_id)
src/blueprints/matching_bp.py- database_service.resolve_suggestion(kosync_doc_id)
src/blueprints/matching_bp.py- # Also resolve detected entries
src/blueprints/matching_bp.py: database_service.resolve_detected_book(abs_id, source="abs")
src/blueprints/matching_bp.py- if kosync_doc_id:
src/blueprints/matching_bp.py: database_service.resolve_detected_book(kosync_doc_id, source="kosync")
src/blueprints/matching_bp.py- try:
src/blueprints/matching_bp.py- device_doc = database_service.get_kosync_doc_by_filename(ebook_filename)
src/blueprints/matching_bp.py- if device_doc and device_doc.document_hash != kosync_doc_id:
src/blueprints/matching_bp.py- database_service.resolve_suggestion(device_doc.document_hash)
src/blueprints/matching_bp.py: database_service.resolve_detected_book(device_doc.document_hash, source="kosync")
src/blueprints/matching_bp.py- except Exception as e:
src/blueprints/matching_bp.py- logger.warning(f"Failed to check/resolve device hash: {e}")
src/blueprints/matching_bp.py-
--
src/blueprints/matching_bp.py- abs_service.add_to_collection(abs_id, current_app.config["ABS_COLLECTION_NAME"])
src/blueprints/matching_bp.py- attempt_hardcover_automatch(container, book)
src/blueprints/matching_bp.py- database_service.resolve_suggestion(abs_id)
src/blueprints/matching_bp.py: database_service.resolve_detected_book(abs_id, source="abs")
src/blueprints/matching_bp.py- return redirect(url_for("dashboard.index"))
src/blueprints/matching_bp.py-
src/blueprints/matching_bp.py- # --- Ebook-only import (no audiobook required) ---
--
src/blueprints/matching_bp.py- # Resolve any suggestions involving these source IDs
src/blueprints/matching_bp.py- if kosync_doc_id:
src/blueprints/matching_bp.py- database_service.resolve_suggestion(kosync_doc_id, source="kosync")
src/blueprints/matching_bp.py: database_service.resolve_detected_book(kosync_doc_id, source="kosync")
src/blueprints/matching_bp.py- if storyteller_uuid:
src/blueprints/matching_bp.py- database_service.resolve_suggestion(storyteller_uuid, source="storyteller")
src/blueprints/matching_bp.py: database_service.resolve_detected_book(storyteller_uuid, source="storyteller")
src/blueprints/matching_bp.py- if ebook_filename:
src/blueprints/matching_bp.py- database_service.resolve_suggestion(ebook_filename, source="grimmory")
src/blueprints/matching_bp.py- return redirect(url_for("dashboard.index"))
--
src/blueprints/matching_bp.py- except Exception as e:
src/blueprints/matching_bp.py- logger.warning(f"Grimmory add_to_shelf failed for '{sanitize_log_data(ebook_filename)}': {e}")
src/blueprints/matching_bp.py- database_service.resolve_suggestion(kosync_doc_id)
src/blueprints/matching_bp.py: database_service.resolve_detected_book(kosync_doc_id, source="kosync")
src/blueprints/matching_bp.py- return redirect(url_for("dashboard.index"))
src/blueprints/matching_bp.py-
src/blueprints/matching_bp.py- # --- Attach audiobook to ebook-only book ---
--
src/blueprints/matching_bp.py- raise
src/blueprints/matching_bp.py- attempt_hardcover_automatch(container, new_book)
src/blueprints/matching_bp.py- database_service.resolve_suggestion(abs_id)
src/blueprints/matching_bp.py: database_service.resolve_detected_book(abs_id, source="abs")
src/blueprints/matching_bp.py- if new_book.kosync_doc_id:
src/blueprints/matching_bp.py- database_service.resolve_suggestion(new_book.kosync_doc_id)
src/blueprints/matching_bp.py: database_service.resolve_detected_book(new_book.kosync_doc_id, source="kosync")
src/blueprints/matching_bp.py- return redirect(url_for("dashboard.index"))
src/blueprints/matching_bp.py-
src/blueprints/matching_bp.py- # --- Standard flow (requires audiobook) ---
--
src/blueprints/matching_bp.py- abs_service.add_to_collection(item["abs_id"], current_app.config["ABS_COLLECTION_NAME"])
src/blueprints/matching_bp.py- attempt_hardcover_automatch(container, book)
src/blueprints/matching_bp.py- database_service.resolve_suggestion(item["abs_id"])
src/blueprints/matching_bp.py: database_service.resolve_detected_book(item["abs_id"], source="abs")
src/blueprints/matching_bp.py- continue
src/blueprints/matching_bp.py-
src/blueprints/matching_bp.py- # Handle ebook-only queue items
--
src/blueprints/matching_bp.py- ensure_kosync_document(book, database_service)
src/blueprints/matching_bp.py- if kosync_doc_id:
src/blueprints/matching_bp.py- database_service.resolve_suggestion(kosync_doc_id)
src/blueprints/matching_bp.py: database_service.resolve_detected_book(kosync_doc_id, source="kosync")
src/blueprints/matching_bp.py- continue
src/blueprints/matching_bp.py-
src/blueprints/matching_bp.py- book, error = _create_book_mapping(
--
src/blueprints/api.py-
src/blueprints/api.py-
src/blueprints/api.py-@api_bp.route("/api/detected/<source_id>/resolve", methods=["POST"])
src/blueprints/api.py:def resolve_detected_book(source_id):
src/blueprints/api.py- """Mark a detected book as resolved (added to library)."""
src/blueprints/api.py- database_service = get_database_service()
src/blueprints/api.py- source = request.args.get("source", "abs")
src/blueprints/api.py- if source not in _VALID_SUGGESTION_SOURCES:
src/blueprints/api.py- return jsonify({"success": False, "error": "Invalid source"}), 400
src/blueprints/api.py: if database_service.resolve_detected_book(source_id, source=source):
src/blueprints/api.py- return jsonify({"success": True})
src/blueprints/api.py- return jsonify({"success": False, "error": "Not found"}), 404
src/blueprints/api.py-
--
src/db/detected_repository.py- detected.last_seen_at = datetime.now(UTC)
src/db/detected_repository.py- return True
src/db/detected_repository.py-
src/db/detected_repository.py: def resolve_detected_book(self, source_id, source="abs"):
src/db/detected_repository.py- with self.get_session() as session:
src/db/detected_repository.py- detected = (
src/db/detected_repository.py- session.query(DetectedBook)
--
src/db/detected_repository.py- )
src/db/detected_repository.py- if not detected:
src/db/detected_repository.py- return False
src/db/detected_repository.py: detected.status = "resolved"
src/db/detected_repository.py- detected.last_seen_at = datetime.now(UTC)
src/db/detected_repository.py- return True
src/db/detected_repository.py-
Script executed:
# Look at the full save_detected_book implementation
cat src/db/detected_repository.py | grep -A 50 "def save_detected_book"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
def save_detected_book(self, detected_book):
"""Upsert a detected book while preserving dismissed status."""
filters = [
DetectedBook.source_id == detected_book.source_id,
DetectedBook.source == detected_book.source,
]
with self.get_session() as session:
existing = session.query(DetectedBook).filter(*filters).first()
now = datetime.now(UTC)
if existing:
if existing.status == "dismissed" and detected_book.status == "detected":
detected_book.status = "dismissed"
if detected_book.title:
existing.title = detected_book.title
if detected_book.author:
existing.author = detected_book.author
if detected_book.cover_url:
existing.cover_url = detected_book.cover_url
if detected_book.matches_json is not None:
existing.matches_json = detected_book.matches_json
if detected_book.device:
existing.device = detected_book.device
if detected_book.ebook_filename:
existing.ebook_filename = detected_book.ebook_filename
existing.progress_percentage = detected_book.progress_percentage
existing.status = detected_book.status
existing.last_seen_at = detected_book.last_seen_at or now
if existing.first_detected_at is None:
existing.first_detected_at = detected_book.first_detected_at or now
session.flush()
session.refresh(existing)
session.expunge(existing)
return existing
session.add(detected_book)
session.flush()
session.refresh(detected_book)
session.expunge(detected_book)
return detected_book
def dismiss_detected_book(self, source_id, source="abs"):
with self.get_session() as session:
detected = (
session.query(DetectedBook)
.filter(
DetectedBook.source_id == source_id,
DetectedBook.source == source,
)
Script executed:
# See if a resolved book can be fetched and updated via kosync
rg "get_detected_book" -B 3 -A 5Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- self.assertTrue(self.db_service.resolve_detected_book("shared-id", source="abs"))
tests/test_database_service_integration.py-
tests/test_database_service_integration.py: resolved = self.db_service.get_detected_book("shared-id", source="abs")
tests/test_database_service_integration.py: still_active = self.db_service.get_detected_book("shared-id", source="kosync")
tests/test_database_service_integration.py- self.assertEqual(resolved.status, "resolved")
tests/test_database_service_integration.py- self.assertEqual(still_active.status, "detected")
tests/test_database_service_integration.py-
tests/test_database_service_integration.py- def test_delete_book(self):
tests/test_database_service_integration.py- """Test deleting a book record with cascading deletes for states and hardcover details."""
--
tests/test_suggestion_logic.py- self.mock_db.get_all_books.return_value = []
tests/test_suggestion_logic.py- self.mock_db.get_pending_suggestion.return_value = None
tests/test_suggestion_logic.py- self.mock_db.suggestion_exists.return_value = False
tests/test_suggestion_logic.py: self.mock_db.get_detected_book.return_value = None
tests/test_suggestion_logic.py-
tests/test_suggestion_logic.py- # Prepare successful suggestion creation mocks
tests/test_suggestion_logic.py- self.mock_abs.get_item_details.return_value = {
tests/test_suggestion_logic.py- "media": {"metadata": {"title": "Test Book", "authorName": "Author"}}
tests/test_suggestion_logic.py- }
--
tests/test_queue_suggestion.py-
tests/test_queue_suggestion.py- def test_creates_suggestion_for_new_book(self):
tests/test_queue_suggestion.py- self.mock_db.suggestion_exists.return_value = False
tests/test_queue_suggestion.py: self.mock_db.get_detected_book.return_value = None
tests/test_queue_suggestion.py- self.mock_abs.get_item_details.return_value = {
tests/test_queue_suggestion.py- "media": {"metadata": {"title": "Test Book", "authorName": "Author"}}
tests/test_queue_suggestion.py- }
tests/test_queue_suggestion.py- self.manager.queue_suggestion("book-789")
tests/test_queue_suggestion.py- self.mock_abs.get_item_details.assert_called_once_with("book-789")
--
tests/test_queue_suggestion.py- def test_thread_safety_prevents_duplicate(self):
tests/test_queue_suggestion.py- """Second concurrent call for same ID should be skipped."""
tests/test_queue_suggestion.py- self.mock_db.suggestion_exists.return_value = False
tests/test_queue_suggestion.py: self.mock_db.get_detected_book.return_value = None
tests/test_queue_suggestion.py-
tests/test_queue_suggestion.py- # Simulate first call in-flight
tests/test_queue_suggestion.py- self.manager.suggestion_service._suggestion_in_flight.add("book-dup")
tests/test_queue_suggestion.py- self.manager.queue_suggestion("book-dup")
tests/test_queue_suggestion.py-
--
tests/test_queue_suggestion.py-
tests/test_queue_suggestion.py- def test_cleans_up_in_flight_on_error(self):
tests/test_queue_suggestion.py- self.mock_db.suggestion_exists.return_value = False
tests/test_queue_suggestion.py: self.mock_db.get_detected_book.return_value = None
tests/test_queue_suggestion.py- self.mock_abs.get_item_details.side_effect = Exception("boom")
tests/test_queue_suggestion.py-
tests/test_queue_suggestion.py- self.manager.queue_suggestion("book-err")
tests/test_queue_suggestion.py-
tests/test_queue_suggestion.py- # Should clean up despite error
--
src/blueprints/api.py-
src/blueprints/api.py-
src/blueprints/api.py-@api_bp.route("/api/detected", methods=["GET"])
src/blueprints/api.py:def get_detected_books():
src/blueprints/api.py- """Return active detected books."""
src/blueprints/api.py- database_service = get_database_service()
src/blueprints/api.py- try:
src/blueprints/api.py- detected = database_service.get_active_detected_books(limit=50)
src/blueprints/api.py- results = []
--
src/db/detected_repository.py-class DetectedRepository(BaseRepository):
src/db/detected_repository.py- ACTIVE_STATUSES = ("detected",)
src/db/detected_repository.py-
src/db/detected_repository.py: def get_detected_book(self, source_id, source="abs"):
src/db/detected_repository.py- return self._get_one(
src/db/detected_repository.py- DetectedBook,
src/db/detected_repository.py- DetectedBook.source_id == source_id,
src/db/detected_repository.py- DetectedBook.source == source,
src/db/detected_repository.py- )
--
src/services/suggestion_service.py-
src/services/suggestion_service.py- def _detected_book_is_dismissed(self, source_id: str, source: str = "abs") -> bool:
src/services/suggestion_service.py- """Return True when a dismissed detected entry should stay hidden."""
src/services/suggestion_service.py: detected = self.database_service.get_detected_book(source_id, source=source)
src/services/suggestion_service.py- return bool(detected and detected.status == "dismissed")
src/services/suggestion_service.py-
src/services/suggestion_service.py- def _get_storyteller_books_with_progress(self, mapped_uuids: set | None = None) -> list[dict]:
src/services/suggestion_service.py- """Fetch Storyteller books with 1-70% progress, excluding already-mapped UUIDs."""
src/services/suggestion_service.py- if not self.storyteller_client or not self.storyteller_client.is_configured():
--
src/services/kosync_service.py- filename=kosync_doc.filename,
src/services/kosync_service.py- device=device,
src/services/kosync_service.py- )
src/services/kosync_service.py: detected = self._db.get_detected_book(doc_hash, source="kosync")
src/services/kosync_service.py- if detected:
src/services/kosync_service.py- detected.progress_percentage = float(percentage)
src/services/kosync_service.py- self._db.save_detected_book(detected)
src/services/kosync_service.py- except Exception as e:
src/services/kosync_service.py- logger.debug(f"KOSync detected-book update failed for {doc_hash[:8]}...: {e}")
Script executed:
# Check if there's a scenario where kosync updates a resolved book
cat src/services/kosync_service.py | grep -B 10 -A 10 "get_detected_book"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
self._db.save_kosync_document(kosync_doc)
if 0.01 <= percentage <= 0.70:
try:
suggestion_svc = self._container.suggestion_service()
suggestion_svc.queue_kosync_suggestion(
doc_hash,
filename=kosync_doc.filename,
device=device,
)
detected = self._db.get_detected_book(doc_hash, source="kosync")
if detected:
detected.progress_percentage = float(percentage)
self._db.save_detected_book(detected)
except Exception as e:
logger.debug(f"KOSync detected-book update failed for {doc_hash[:8]}...: {e}")
# Update linked book if exists
linked_book = None
if kosync_doc.linked_book_id:
linked_book = self._db.get_book_by_id(kosync_doc.linked_book_id)
Script executed:
# Check if there's a scenario where a book moves from resolved back to detected
cat src/db/detected_repository.py | grep -B 5 -A 15 "def get_active_detected_books"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
DetectedBook,
DetectedBook.source_id == source_id,
DetectedBook.source == source,
)
def get_active_detected_books(self, limit=None):
with self.get_session() as session:
query = (
session.query(DetectedBook)
.filter(DetectedBook.status.in_(self.ACTIVE_STATUSES))
.order_by(DetectedBook.last_seen_at.desc())
)
if limit is not None:
query = query.limit(limit)
items = query.all()
for item in items:
session.expunge(item)
return items
def save_detected_book(self, detected_book):
"""Upsert a detected book while preserving dismissed status."""
Script executed:
# Look at ACTIVE_STATUSES to understand which statuses should be displayed
rg "ACTIVE_STATUSES" -B 2 -A 5Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
src/db/detected_repository.py-
src/db/detected_repository.py-class DetectedRepository(BaseRepository):
src/db/detected_repository.py: ACTIVE_STATUSES = ("detected",)
src/db/detected_repository.py-
src/db/detected_repository.py- def get_detected_book(self, source_id, source="abs"):
src/db/detected_repository.py- return self._get_one(
src/db/detected_repository.py- DetectedBook,
src/db/detected_repository.py- DetectedBook.source_id == source_id,
--
src/db/detected_repository.py- query = (
src/db/detected_repository.py- session.query(DetectedBook)
src/db/detected_repository.py: .filter(DetectedBook.status.in_(self.ACTIVE_STATUSES))
src/db/detected_repository.py- .order_by(DetectedBook.last_seen_at.desc())
src/db/detected_repository.py- )
src/db/detected_repository.py- if limit is not None:
src/db/detected_repository.py- query = query.limit(limit)
src/db/detected_repository.py- items = query.all()
--
src/db/detected_repository.py- session.query(DetectedBook)
src/db/detected_repository.py- .filter(
src/db/detected_repository.py: DetectedBook.status.in_(self.ACTIVE_STATUSES),
src/db/detected_repository.py- DetectedBook.matches_json.isnot(None),
src/db/detected_repository.py- )
src/db/detected_repository.py- .all()
src/db/detected_repository.py- )
src/db/detected_repository.py- filenames = set()
Script executed:
# Double-check: are all DetectedBook instantiations either passing status or using the default?
rg "DetectedBook\s*\(" -A 10 | grep -E "(DetectedBook|status=)" | head -50Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
tests/test_database_service_integration.py: first = DetectedBook(
tests/test_database_service_integration.py: second = DetectedBook(
tests/test_database_service_integration.py: abs_detected = DetectedBook(source="abs", source_id="shared-id", title="ABS", progress_percentage=0.2)
tests/test_database_service_integration.py: kosync_detected = DetectedBook(source="kosync", source_id="shared-id", title="KOSync", progress_percentage=0.3)
src/db/models.py:class DetectedBook(Base):
src/db/models.py: f"<DetectedBook(id={self.id}, source='{self.source}', source_id='{self.source_id}', "
src/db/models.py- f"status='{self.status}')>"
src/services/suggestion_service.py: detected = DetectedBook(
Script executed:
# Final check: verify that all DetectedBook creations in suggestion_service pass a status value or rely on the default
cat src/services/suggestion_service.py | grep -B 2 -A 12 "detected = DetectedBook"Repository: serabi/pagekeeper
Repository: serabi/pagekeeper
Exit code: 0
stdout:
ebook_filename: str | None = None,
):
detected = DetectedBook(
source=source,
source_id=source_id,
title=title or source_id,
author=author or "",
cover_url=cover_url,
progress_percentage=max(0.0, min(progress_percentage, 1.0)),
matches_json=json.dumps(matches or []),
device=device,
ebook_filename=ebook_filename,
)
return self.database_service.save_detected_book(detected)
Preserve "resolved" status during upsert; currently only "dismissed" is preserved.
When save_detected_book updates an existing row, it unconditionally overwrites status with the incoming value. Since all new DetectedBook instances default to status="detected", this reopens previously resolved entries during progress updates.
For example, when kosync syncs progress on a resolved book, the update resets it to "detected" and it reappears on the dashboard. Preserve "resolved" alongside "dismissed":
🐛 Suggested fix
existing = session.query(DetectedBook).filter(*filters).first()
now = datetime.now(UTC)
if existing:
- if existing.status == "dismissed" and detected_book.status == "detected":
- detected_book.status = "dismissed"
+ if existing.status in {"dismissed", "resolved"} and detected_book.status == "detected":
+ detected_book.status = existing.status
if detected_book.title:
existing.title = detected_book.title
@@
- existing.status = detected_book.status
+ existing.status = detected_book.status if detected_book.status != "detected" else existing.status🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/db/detected_repository.py` around lines 33 - 68, In save_detected_book:
when upserting an existing DetectedBook, preserve a prior "resolved" status the
same way "dismissed" is preserved—i.e., if existing.status is "resolved" (or in
general in ["dismissed","resolved"]) and the incoming detected_book.status is
"detected" then do not overwrite it; set detected_book.status (or keep
existing.status) to the existing value before assigning existing.status =
detected_book.status so resolved entries are not reopened; update the
conditional that now checks only for "dismissed" to include "resolved" and
ensure existing.status is ultimately preserved when appropriate.
| # From Detected Books (covers auto-discovery matches) | ||
| valid_filenames.update(self.database_service.get_all_ebook_filenames()) |
There was a problem hiding this comment.
Cache cleanup can drop still-relevant detected EPUBs
Line 242 only protects filenames returned from detected matches. Detected rows that only have ebook_filename (no matches yet) are not protected, so cleanup_cache() can delete files still needed by the detected-books flow.
Proposed hardening
- # From Detected Books (covers auto-discovery matches)
- valid_filenames.update(self.database_service.get_all_ebook_filenames())
+ # From Detected Books (covers auto-discovery matches)
+ valid_filenames.update(self.database_service.get_all_ebook_filenames())
+ # Also keep direct detected ebook filenames even when matches are empty
+ for detected in self.database_service.get_active_detected_books():
+ if detected.ebook_filename:
+ valid_filenames.add(detected.ebook_filename)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/sync_manager.py` around lines 241 - 242, cleanup_cache currently only
protects filenames from auto-discovery matches (via
database_service.get_all_ebook_filenames()), but detected-book rows that only
set ebook_filename are omitted and may be deleted; update cleanup_cache() to
also fetch and include detected-only ebook filenames by calling the
database_service method that returns ebook_filename values for detected rows
(e.g., database_service.get_all_detected_ebook_filenames() or add such a helper)
and add those filenames to valid_filenames before deleting cached files so
detected EPUBs are preserved.
| fetch('/api/detected/' + encodeURIComponent(sourceId) + '/dismiss?source=' + encodeURIComponent(source || 'abs'), { method: 'POST' }) | ||
| .then(function(r) { | ||
| if (!r.ok) throw new Error('Failed to dismiss'); | ||
| var card = document.getElementById('suggestion-card-' + sourceId); | ||
| var card = document.querySelector('.detected-card[data-source-id="' + sourceId + '"]'); | ||
| if (card) { |
There was a problem hiding this comment.
Key the DOM removal by both source and source_id.
DetectedBook is unique by (source_id, source), but this selector only uses data-source-id. If two providers reuse the same ID, a successful dismiss removes the wrong card from the DOM.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@static/js/dashboard.js` around lines 859 - 863, The DOM removal only matches
by data-source-id and can remove the wrong card when different providers share
the same sourceId; update the selector used in the fetch success handler to
match both attributes (data-source-id and data-source) so it uniquely targets
the DetectedBook tuple (sourceId, source). In the fetch .then callback where you
build var card = document.querySelector('.detected-card[data-source-id="' +
sourceId + '"]'), change that selector to also include the data-source attribute
(using the same source variable or default 'abs' as used in the request) so it
selects '.detected-card[data-source-id="..."][data-source="..."]' before
removing the element.
| {% if d.source == 'abs' %} | ||
| <a href="/match?abs_id={{ d.source_id }}&search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Add to Library</a> | ||
| {% elif d.source == 'kosync' %} | ||
| <a href="/match?search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Match</a> | ||
| <a href="/match?ebook_filename={{ d.ebook_filename|urlencode if d.ebook_filename }}&action=ebook_only" class="btn btn-sm btn-secondary">Add as Ebook</a> |
There was a problem hiding this comment.
Hide “Add as Ebook” when no ebook filename is known.
KoSync detections can be created from device-only progress, so d.ebook_filename can legitimately be empty. In that case this renders a dead /match?ebook_filename=&action=ebook_only link.
Suggested change
{% elif d.source == 'kosync' %}
<a href="/match?search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Match</a>
- <a href="/match?ebook_filename={{ d.ebook_filename|urlencode if d.ebook_filename }}&action=ebook_only" class="btn btn-sm btn-secondary">Add as Ebook</a>
+ {% if d.ebook_filename %}
+ <a href="/match?ebook_filename={{ d.ebook_filename|urlencode }}&action=ebook_only" class="btn btn-sm btn-secondary">Add as Ebook</a>
+ {% endif %}
{% else %}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {% if d.source == 'abs' %} | |
| <a href="/match?abs_id={{ d.source_id }}&search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Add to Library</a> | |
| {% elif d.source == 'kosync' %} | |
| <a href="/match?search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Match</a> | |
| <a href="/match?ebook_filename={{ d.ebook_filename|urlencode if d.ebook_filename }}&action=ebook_only" class="btn btn-sm btn-secondary">Add as Ebook</a> | |
| {% if d.source == 'abs' %} | |
| <a href="/match?abs_id={{ d.source_id }}&search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Add to Library</a> | |
| {% elif d.source == 'kosync' %} | |
| <a href="/match?search={{ d.title|urlencode }}" class="btn btn-sm btn-primary">Match</a> | |
| {% if d.ebook_filename %} | |
| <a href="/match?ebook_filename={{ d.ebook_filename|urlencode }}&action=ebook_only" class="btn btn-sm btn-secondary">Add as Ebook</a> | |
| {% endif %} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/index.html` around lines 127 - 131, The template renders an "Add as
Ebook" link even when d.ebook_filename is empty, producing a dead
/match?ebook_filename=&action=ebook_only link; update the kosync branch of the
template to conditionally render the second anchor only when d.ebook_filename is
truthy (non-empty) by checking d.ebook_filename before emitting the <a> tag so
the "Add as Ebook" button is hidden when no ebook filename exists.
| self.mock_db.save_pending_suggestion.assert_called_once() | ||
| self.mock_db.save_detected_book.assert_called_once() |
There was a problem hiding this comment.
Assertion still expects the removed PendingSuggestion write path
Line 89 enforces a legacy DB write that this PR is removing. Keep the detected-book assertion and drop the pending-suggestion creation expectation.
Suggested test expectation update
- self.mock_db.save_pending_suggestion.assert_called_once()
+ self.mock_db.save_pending_suggestion.assert_not_called()
self.mock_db.save_detected_book.assert_called_once()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.mock_db.save_pending_suggestion.assert_called_once() | |
| self.mock_db.save_detected_book.assert_called_once() | |
| self.mock_db.save_pending_suggestion.assert_not_called() | |
| self.mock_db.save_detected_book.assert_called_once() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_suggestion_logic.py` around lines 89 - 90, Update the test to
remove the legacy PendingSuggestion DB write assertion: delete or stop asserting
mock_db.save_pending_suggestion.assert_called_once(), and retain/ensure the
detected-book expectation by keeping
mock_db.save_detected_book.assert_called_once() (or equivalent assert that
save_detected_book was called exactly once); this aligns the test with the PR
that removed the pending-suggestion write path.
Summary
DetectedBook, remove the final ABSPendingSuggestionpath, and pinmistuneto3.2.0so the dev image builds againTesting
Note
Replace suggestions system with detected books across API, dashboard, and database
PendingSuggestion-based suggestions system with a newDetectedBookmodel, adding adetected_bookstable via Alembic migration and aDetectedRepositoryfor CRUD operations including upsert with status preservation./api/suggestions/*endpoints with/api/detected,/api/detected/<source_id>/dismiss, and/api/detected/<source_id>/resolvein api.py.SuggestionServiceto upsertDetectedBookrecords instead ofPendingSuggestionentries; detected-book creation now runs regardless ofSUGGESTIONS_ENABLEDunless the entry is dismissed.detected_books) with source-specific match/add actions, replacing the 'New Suggestions' banner.SyncManagernow usesget_all_ebook_filenames()from detected books instead of iterating actionable suggestion matches./api/suggestions/*endpoints, the/suggestionspage, and the settings controls for suggestions are removed with no backward compatibility.📊 Macroscope summarized 9291c26. 23 files reviewed, 4 issues evaluated, 1 issue filtered, 3 comments posted
🗂️ Filtered Issues
src/blueprints/dashboard.py — 0 comments posted, 1 evaluated, 1 filtered
bl_meta.raw_metadata_dict. Ifbl_metais notNonebutbl_meta.raw_metadata_dictisNone, calling.get("id")onNonewill raiseAttributeError. The same pattern is handled correctly inresolve_book_coversusing(grimmory_meta.raw_metadata_dict or {}).get("id"). [ Out of scope ]Summary by CodeRabbit
Release Notes
New Features
Refactor
Chores