Repository navigation
Conversation
github-actions
Bot
added
the
size:XL
serabi
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| payload = request.get_json(silent=True) or {} | ||
| ids = [int(i) for i in payload.get("ids", []) if str(i).isdigit()] |
There was a problem hiding this comment.
🟡 Medium blueprints/api.py:265
Malformed JSON bodies such as [], "text", or {"ids": null} make libby_tbr_dismiss raise AttributeError or TypeError, returning HTTP 500 instead of a 4xx client error. Validate that payload is a dictionary and ids is a list before iterating.
| payload = request.get_json(silent=True) or {} | |
| ids = [int(i) for i in payload.get("ids", []) if str(i).isdigit()] | |
| if not isinstance(payload, dict): | |
| return jsonify({"success": False, "detail": "Invalid JSON payload"}), 400 | |
| raw_ids = payload.get("ids", []) | |
| if not isinstance(raw_ids, list): | |
| return jsonify({"success": False, "detail": "'ids' must be an array"}), 400 | |
| ids = [int(i) for i in raw_ids if str(i).isdigit()] |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/blueprints/api.py around lines 265-266:
Malformed JSON bodies such as `[]`, `"text"`, or `{"ids": null}` make `libby_tbr_dismiss` raise `AttributeError` or `TypeError`, returning HTTP 500 instead of a 4xx client error. Validate that `payload` is a dictionary and `ids` is a list before iterating.
| def get_text_from_current_state(self, book: Book, state: ServiceState) -> str | None: | ||
| # Percentage-only locator: text extraction happens via the shared | ||
| # percentage path in the sync manager when needed. | ||
| return None |
There was a problem hiding this comment.
🟠 High sync_clients/libby_sync_client.py:217
Libby progress is never propagated to follower clients when Libby wins leader election: get_text_from_current_state() returns None, so SyncManager._execute_sync_update() saves the Libby snapshot as the new baseline and returns before updating ABS, KoSync, Storyteller, or Hardcover. Return usable leader text here or add an explicit percentage-only leader path so the position is propagated before the baseline is advanced.
Also found in 1 other location(s)
src/utils/di_container.py:241
Registering
Libbyas a sync client exposes a nonfunctional leader path:LibbySyncClient.can_be_leader()returnsTrue, butget_text_from_current_state()always returnsNone. When Libby has the changed/furthest position,SyncManager._execute_sync_update()therefore only persists the Libby snapshot and returns without updating any follower clients. As a result, Libby progress is observed but never propagated to ABS, KoSync, Storyteller, or Hardcover.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/sync_clients/libby_sync_client.py around line 217:
Libby progress is never propagated to follower clients when Libby wins leader election: `get_text_from_current_state()` returns `None`, so `SyncManager._execute_sync_update()` saves the Libby snapshot as the new baseline and returns before updating ABS, KoSync, Storyteller, or Hardcover. Return usable leader text here or add an explicit percentage-only leader path so the position is propagated before the baseline is advanced.
Also found in 1 other location(s):
- src/utils/di_container.py:241 -- Registering `Libby` as a sync client exposes a nonfunctional leader path: `LibbySyncClient.can_be_leader()` returns `True`, but `get_text_from_current_state()` always returns `None`. When Libby has the changed/furthest position, `SyncManager._execute_sync_update()` therefore only persists the Libby snapshot and returns without updating any follower clients. As a result, Libby progress is observed but never propagated to ABS, KoSync, Storyteller, or Hardcover.
| previous = self._last_reading_time.get(psn) | ||
| self._last_reading_time[psn] = current_rt | ||
| if previous is None or current_rt <= previous: | ||
| return | ||
| delta = current_rt - previous | ||
| if delta < READING_TIME_DELTA_THRESHOLD: | ||
| return |
There was a problem hiding this comment.
🟡 Medium services/libby_service.py:179
Reading-time accumulated in increments below READING_TIME_DELTA_THRESHOLD is permanently discarded, so repeated 120-second increases never produce a journal entry. _last_reading_time[psn] is advanced before the threshold check; retain the previous timestamp for sub-threshold deltas and advance it only once the accumulated delta is journaled.
previous = self._last_reading_time.get(psn)
- self._last_reading_time[psn] = current_rt
- if previous is None or current_rt <= previous:
+ if previous is None or current_rt <= previous:
+ self._last_reading_time[psn] = current_rt
return
delta = current_rt - previous
if delta < READING_TIME_DELTA_THRESHOLD:
return
+ self._last_reading_time[psn] = current_rt🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/services/libby_service.py around lines 179-185:
Reading-time accumulated in increments below `READING_TIME_DELTA_THRESHOLD` is permanently discarded, so repeated 120-second increases never produce a journal entry. `_last_reading_time[psn]` is advanced before the threshold check; retain the previous timestamp for sub-threshold deltas and advance it only once the accumulated delta is journaled.
| return dict(self._positions) or None | ||
|
|
||
| poll_secs = self._poll_secs() | ||
| if self._positions and now - self._last_fetch < max(poll_secs, MIN_FETCH_GAP): |
There was a problem hiding this comment.
🟡 Medium sync_clients/libby_sync_client.py:86
A successful refresh with no readable positions is not throttled, so every subsequent sync immediately calls get_active_loans() and the position endpoints again instead of waiting max(poll_secs, MIN_FETCH_GAP). Because line 86 only checks the interval when self._positions is non-empty, empty or unreadable accounts can continuously hammer Libby; base the throttle solely on _last_fetch.
| if self._positions and now - self._last_fetch < max(poll_secs, MIN_FETCH_GAP): | |
| if now - self._last_fetch < max(poll_secs, MIN_FETCH_GAP): | |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/sync_clients/libby_sync_client.py around line 86:
A successful refresh with no readable positions is not throttled, so every subsequent sync immediately calls `get_active_loans()` and the position endpoints again instead of waiting `max(poll_secs, MIN_FETCH_GAP)`. Because line 86 only checks the interval when `self._positions` is non-empty, empty or unreadable accounts can continuously hammer Libby; base the throttle solely on `_last_fetch`.
| ) | ||
| logger.info("Libby: journalled %d min for '%s'", minutes, book.title) | ||
| except Exception as e: | ||
| logger.warning("Libby: journal write failed for book %s: %s", book.id, e) |
There was a problem hiding this comment.
🟡 Medium services/libby_service.py:199
A transient add_reading_journal failure permanently drops the reading-time interval: _last_reading_time[psn] is already advanced at line 180, so the next refresh treats the same delta as observed and never retries it. Restore the previous baseline when the journal write fails (or update the baseline only after a successful write).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/services/libby_service.py around line 199:
A transient `add_reading_journal` failure permanently drops the reading-time interval: `_last_reading_time[psn]` is already advanced at line 180, so the next refresh treats the same delta as observed and never retries it. Restore the previous baseline when the journal write fails (or update the baseline only after a successful write).
| linked_psns = {getattr(b, "libby_psn_key", None) for b in self.db.get_all_books() or []} | ||
| for loan in loans: | ||
| psn = loan["psn_key"] | ||
| if psn in linked_psns: | ||
| continue | ||
|
|
||
| book = self._find_book_for_loan(loan) | ||
| if book is None: | ||
| self._queue_match_suggestion(loan) | ||
| continue | ||
|
|
||
| book.libby_psn_key = psn | ||
| self.db.save_book(book) |
There was a problem hiding this comment.
🟠 High services/libby_service.py:66
A second active loan that resolves to the same PageKeeper book overwrites the first loan's libby_psn_key, so the first loan loses position synchronization and is not represented by a match suggestion. linked_psns contains only values from before the loop and does not exclude books selected earlier in this pass; track matched books after each save and queue later duplicate matches instead of reusing them.
linked_psns = {getattr(b, "libby_psn_key", None) for b in self.db.get_all_books() or []}
+ matched_book_ids = set()
for loan in loans:
psn = loan["psn_key"]
if psn in linked_psns:
continue
book = self._find_book_for_loan(loan)
- if book is None:
+ if book is None or id(book) in matched_book_ids:
self._queue_match_suggestion(loan)
continue
book.libby_psn_key = psn
self.db.save_book(book)
+ matched_book_ids.add(id(book))🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/services/libby_service.py around lines 66-78:
A second active loan that resolves to the same PageKeeper book overwrites the first loan's `libby_psn_key`, so the first loan loses position synchronization and is not represented by a match suggestion. `linked_psns` contains only values from before the loop and does not exclude books selected earlier in this pass; track matched books after each save and queue later duplicate matches instead of reusing them.
| passport = self._get_cached_passport(loan) | ||
| if not passport: | ||
| return None, None | ||
| possession = self.libby_client.get_possession(passport["urls"]["possession"]) |
There was a problem hiding this comment.
🟠 High sync_clients/libby_sync_client.py:147
A partial passport without `urls[
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/sync_clients/libby_sync_client.py around line 147:
A partial passport without `urls[
| """Explicitly merge reviewed holds into tbr_items. Returns count.""" | ||
| merged = 0 | ||
| for suggestion in self.db.get_all_pending_suggestions() or []: | ||
| if suggestion.id not in suggestion_ids or suggestion.source != "libby_hold": |
There was a problem hiding this comment.
🟡 Medium services/libby_service.py:237
merge_tbr_items re-adds a TBR row when called for an already merged or dismissed suggestion, creating duplicates and reporting it as newly merged. Restrict this operation to suggestions whose status is pending so retries are idempotent.
- if suggestion.id not in suggestion_ids or suggestion.source != "libby_hold":
+ if suggestion.id not in suggestion_ids or suggestion.source != "libby_hold" or suggestion.status != "pending":🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/services/libby_service.py around line 237:
`merge_tbr_items` re-adds a TBR row when called for an already merged or dismissed suggestion, creating duplicates and reporting it as newly merged. Restrict this operation to suggestions whose `status` is `pending` so retries are idempotent.
| self._positions = fresh | ||
| self._last_fetch = time.time() | ||
| self._consecutive_failures = 0 |
There was a problem hiding this comment.
🟡 Medium sync_clients/libby_sync_client.py:105
When the loan list succeeds but every per-loan position lookup fails, this cycle clears the previously valid _positions cache and resets _consecutive_failures, so Libby progress disappears for the poll interval and the advertised backoff/auto-disable logic never runs. Preserve the cache and register a failed cycle when loans are present but no positions were obtained.
- self._positions = fresh
- self._last_fetch = time.time()
- self._consecutive_failures = 0
+ if loans and not fresh:
+ self._register_failure()
+ return dict(self._positions) or None
+ self._positions = fresh
+ self._last_fetch = time.time()
+ self._consecutive_failures = 0🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/sync_clients/libby_sync_client.py around lines 105-107:
When the loan list succeeds but every per-loan position lookup fails, this cycle clears the previously valid `_positions` cache and resets `_consecutive_failures`, so Libby progress disappears for the poll interval and the advertised backoff/auto-disable logic never runs. Preserve the cache and register a failed cycle when loans are present but no positions were obtained.
| GrimmoryAudio=grimmory_audio_sync_client, | ||
| Grimmory2Audio=grimmory_audio_sync_client_2, | ||
| Hardcover=hardcover_sync_client, | ||
| Libby=libby_sync_client, |
There was a problem hiding this comment.
🟡 Medium utils/di_container.py:241
Registering Libby enables targeted sync, where LibbySyncClient._targeted_position() iterates get_active_loans() without handling its documented None failure result. A transient /chip/sync failure therefore raises TypeError instead of returning no state; guard the result in _targeted_position() before iterating.
Also found in 3 other location(s)
src/api/libby_client.py:421
get_active_loansnow returnsNonewhen sync fetching fails, but_targeted_positionstill immediately iterates its result in a dict comprehension. A transient/chip/syncnetwork or HTTP failure during a manual per-book sync therefore raisesTypeError: 'NoneType' object is not iterableinstead of returning no position; guardNonein that caller (as the bulk path already does).
src/sync_clients/libby_sync_client.py:233
_targeted_positionassumesget_active_loans()is iterable, but that method explicitly returnsNonewhen/chip/syncfails. A manual sync orClientPollercall during a Libby network/auth failure therefore raisesTypeErrorin the dictionary comprehension instead of returning no state; repeated polling will keep failing per book rather than using the client's failure/backoff handling.
src/sync_clients/libby_sync_client.py:135
_targeted_position(which calls this method when no bulk context is supplied) iteratesself.libby_client.get_active_loans()without handling its documentedNonefailure result. On a network/API failure, the dict comprehension raisesTypeError: 'NoneType' object is not iterable, turning an expected fetch failure into an exception during a manual per-book sync.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/utils/di_container.py around line 241:
Registering `Libby` enables targeted sync, where `LibbySyncClient._targeted_position()` iterates `get_active_loans()` without handling its documented `None` failure result. A transient `/chip/sync` failure therefore raises `TypeError` instead of returning no state; guard the result in `_targeted_position()` before iterating.
Also found in 3 other location(s):
- src/api/libby_client.py:421 -- `get_active_loans` now returns `None` when sync fetching fails, but `_targeted_position` still immediately iterates its result in a dict comprehension. A transient `/chip/sync` network or HTTP failure during a manual per-book sync therefore raises `TypeError: 'NoneType' object is not iterable` instead of returning no position; guard `None` in that caller (as the bulk path already does).
- src/sync_clients/libby_sync_client.py:233 -- `_targeted_position` assumes `get_active_loans()` is iterable, but that method explicitly returns `None` when `/chip/sync` fails. A manual sync or `ClientPoller` call during a Libby network/auth failure therefore raises `TypeError` in the dictionary comprehension instead of returning no state; repeated polling will keep failing per book rather than using the client's failure/backoff handling.
- src/sync_clients/libby_sync_client.py:135 -- `_targeted_position` (which calls this method when no bulk context is supplied) iterates `self.libby_client.get_active_loans()` without handling its documented `None` failure result. On a network/API failure, the dict comprehension raises `TypeError: 'NoneType' object is not iterable`, turning an expected fetch failure into an exception during a manual per-book sync.
…t/libby-matching # Conflicts: # src/sync_clients/libby_sync_client.py # src/utils/di_container.py # tests/conftest.py
github-actions
Bot
added
size:L
Part 3/3 of the Libby integration.
Closes #131
What
book.libby_psn_keyautomatically, title/author fuzzy at ≥90 next, unmatched loans join the pending_suggestions review queuelibby_holdreview-queue suggestions; merging is always explicit (/api/libby/tbr/merge) creatingtbr_items, dismiss marks without TBR. Nothing auto-adds.statistics.readingTimedeltas >5 min journal as progress notes attributed to the observation dayNote
Add
LibbyServicefor loan matching, lifecycle, TBR curation, and reading-time backfillLibbyServiceto enrich Libby loans with OverDrive Thunder metadata (ISBNs, cover URLs) and match them to existing books via exact ISBN then fuzzy title/author (rapidfuzz score ≥ 90), queuing unmatched loans as pending suggestions.libby_holdsuggestions and addsPOST /api/libby/tbr/mergeandPOST /api/libby/tbr/dismissendpoints to convert or dismiss them into TBR items with sourcelibby.LibbyServiceinto the DI container and triggersrefreshfromLibbySyncClient.refresh_positionsafter positions update.handle_lifecycleautomatically unlinks books whoselibby_psn_keyleaves the active loan set and writes a journal note; verify behavior in libby_service.py matches expectations for expired loans.Macroscope summarized d0c9591.