From 12768717a59ba70cd7c07d8fb3479f6625d1148c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 08:00:28 +0000 Subject: [PATCH] fix: let a queued book's cheap picks download without the rest The automatic check weighed all of a saved item's picks together: a freeleech audiobook and a small ebook waited as long as a large non-freeleech audiobook picked with them couldn't go, which wasted free unsatisfied slots that would have been building ratio. Each pick is now weighed on its own, cheapest first: nothing on MAM, then freeleech, then by size. Each one goes if it fits (unsatisfied slots, buffer, target ratio) alongside the ones already going. Picks that don't fit stay in the queue as the item's remaining releases, with "Waiting for" saying why. They stay queued, not turned off as a failed pick is. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V88DbAixT45r3AJsyFJchY --- shelfmark/core/saved_autoget.py | 109 ++++++++++++------ shelfmark/core/saved_items.py | 17 ++- .../src/components/activity/SavedPanel.tsx | 2 +- tests/core/test_saved_autoget.py | 56 ++++++++- 4 files changed, 141 insertions(+), 43 deletions(-) diff --git a/shelfmark/core/saved_autoget.py b/shelfmark/core/saved_autoget.py index b8564fa0d..d02b604ed 100644 --- a/shelfmark/core/saved_autoget.py +++ b/shelfmark/core/saved_autoget.py @@ -75,11 +75,16 @@ class AutoGetHooks: @dataclass class Decision: - """Whether an item can be got now; if not, why it waits.""" + """Which of an item's picks can be got now, and why the rest wait. - ready: bool - status: str = "" + `ready` holds the indexes of the picks that can go (into the item's releases and + payloads), with `payloads` their payloads to queue. `status` says why the others + wait; it is empty when none do. + """ + + ready: list[int] = field(default_factory=list) payloads: list[dict[str, Any]] = field(default_factory=list) + status: str = "" def enabled() -> bool: @@ -172,51 +177,76 @@ def _unsat_wait(check: mam_account.BufferCheck) -> str: return f"Waiting for {needed} unsatisfied slot{'s' if needed != 1 else ''}" +def _waiting(status: str) -> Decision: + return Decision(status=status) + + def evaluate(item: dict[str, Any], owner: Owner, hooks: AutoGetHooks, run: _RunContext) -> Decision: - """Decide whether one saved item can be downloaded now.""" + """Decide which of a saved item's picks can be downloaded now. + + Each pick is weighed on its own, so a freeleech torrent or a small ebook isn't held + back by a large audiobook picked with it that the ratio can't take yet: the cheapest + go first (nothing on MAM, then freeleech, then by size), each one that still fits + alongside those already going. + """ releases = item.get("releases") or [] payloads = [p for p in item.get("payloads") or [] if isinstance(p, dict)] if not payloads or len(payloads) != len(releases): - return Decision(False, "Save the pick again to download it automatically") + return _waiting("Save the pick again to download it automatically") if not all(hooks.may_download(owner, payload) for payload in payloads): - return Decision(False, "Needs approval: use Get to request it") + return _waiting("Needs approval: use Get to request it") + # Nothing counts against MyAnonamouse here, or nothing can be checked: all go. mam_indexes = [i for i, p in enumerate(payloads) if mam_torrent_id_for_release(p)] - if not mam_indexes: - # Nothing here counts against MyAnonamouse, so nothing to wait for. - return Decision(True, payloads=payloads) - if not mam_account.is_configured(): - return Decision(True, payloads=payloads) + if not mam_indexes or not mam_account.is_configured(): + return Decision(ready=list(range(len(payloads))), payloads=payloads) + free_indexes = [i for i in range(len(payloads)) if i not in mam_indexes] stats, error = run.stats() if stats is None: - return Decision(False, f"Couldn't read the MyAnonamouse account: {error}") + return Decision( + ready=free_indexes, + payloads=[payloads[i] for i in free_indexes], + status=f"Couldn't read the MyAnonamouse account: {error}", + ) # Freeleech can start or end at any time, so ask MAM rather than trusting the save. vip = mam_account.is_vip_class(stats.classname) + checked = list(payloads) for index in mam_indexes: - torrent_id = mam_torrent_id_for_release(payloads[index]) + torrent_id = mam_torrent_id_for_release(checked[index]) if torrent_id is None: continue try: freeleech = mam_account.torrent_is_freeleech(torrent_id, vip=vip) except mam_account.MAM_ERRORS: continue # Keep what the search said; the ratio check covers a wrong guess - payloads[index] = _with_freeleech(payloads[index], freeleech) + checked[index] = _with_freeleech(checked[index], freeleech) + + going: list[int] = [] + reason = "" + for index in sorted(mam_indexes, key=lambda i: mam_charge_bytes_for_release(checked[i])): + trial = [checked[i] for i in [*going, index]] + wait = _pick_wait(trial, stats) + if wait: + reason = reason or wait + continue + going.append(index) + + ready = sorted([*free_indexes, *going]) + return Decision(ready=ready, payloads=[checked[i] for i in ready], status=reason) - check = mam_account.check_buffer(payloads) + +def _pick_wait(trial: list[dict[str, Any]], stats: mam_account.MamStats) -> str | None: + """Why the last of `trial` must wait, with the rest of it going too; None if it may go.""" + check = mam_account.check_buffer(trial) if not check.ok: if not check.unsat_ok: - return Decision(False, _unsat_wait(check)) - return Decision(False, f"Waiting for buffer: {_gib(check.missing_bytes)} short") + return _unsat_wait(check) + return f"Waiting for buffer: {_gib(check.missing_bytes)} short" if not check.checked and check.error: - return Decision(False, f"Couldn't check MyAnonamouse: {check.error}") - - wait = _ratio_wait(stats, sum(mam_charge_bytes_for_release(p) for p in payloads)) - if wait: - return Decision(False, wait) - - return Decision(True, payloads=payloads) + return f"Couldn't check MyAnonamouse: {check.error}" + return _ratio_wait(stats, sum(mam_charge_bytes_for_release(p) for p in trial)) @dataclass @@ -275,28 +305,35 @@ def _check_item(self, item: dict[str, Any], run: _RunContext, report: RunReport) self._service.update(owner_key, item["id"], auto_status=decision.status) return - failed: list[tuple[dict[str, Any], dict[str, Any], str]] = [] releases = item.get("releases") or [] - for pick, payload in zip(releases, decision.payloads, strict=True): - ok, error = self._hooks.queue_release(payload, owner) + payloads = item.get("payloads") or [] + failed: list[int] = [] + error = "" + for index, payload in zip(decision.ready, decision.payloads, strict=True): + ok, queue_error = self._hooks.queue_release(payload, owner) if not ok: - failed.append((pick, payload, error or "Could not queue the download")) - if len(failed) == len(decision.payloads): - error = failed[0][2] + failed.append(index) + error = error or queue_error or "Could not queue the download" + if len(failed) == len(decision.ready): report.waiting[item["id"]] = error self._service.update(owner_key, item["id"], auto_status=f"Couldn't queue: {error}") return + report.queued.append(item["id"]) - if failed: + left = [i for i in range(len(releases)) if i not in decision.ready or i in failed] + if not left: + self._service.delete(owner_key, item["id"]) + else: + if decision.status: + report.waiting[item["id"]] = decision.status self._service.keep_picks( owner_key, item["id"], - releases=[pick for pick, _, _ in failed], - payloads=[payload for _, payload, _ in failed], - last_error=f"Some picks couldn't be queued: {failed[0][2]}", + releases=[releases[i] for i in left], + payloads=[payloads[i] for i in left], + last_error=f"Some picks couldn't be queued: {error}" if failed else None, + auto_status=decision.status, ) - else: - self._service.delete(owner_key, item["id"]) try: self._hooks.notify(item, owner) except Exception: # a notification failure must not undo the download diff --git a/shelfmark/core/saved_items.py b/shelfmark/core/saved_items.py index 96ffc3e3c..d1623c739 100644 --- a/shelfmark/core/saved_items.py +++ b/shelfmark/core/saved_items.py @@ -387,25 +387,32 @@ def keep_picks( *, releases: list[dict[str, Any]], payloads: list[dict[str, Any]], - last_error: str, + last_error: str | None = None, + auto_status: str | None = None, ) -> None: - """After a partly failed automatic download: keep only the picks still to get. + """After an automatic download got some of an item's picks: keep only the rest. - Automatic downloading is turned off so the error is seen before anything retries. + With `last_error` (a pick couldn't be queued), automatic downloading is turned + off so the error is seen before anything retries. Without it, the rest are + simply still waiting for room: they stay queued, with `auto_status` saying why. """ kind = "release" if len(releases) == 1 else "combined" + failed = last_error is not None with self._lock: conn = self._connect() try: conn.execute( """UPDATE saved_items SET kind = CASE WHEN content_type = 'combined' - THEN 'combined' ELSE ? END, releases = ?, payloads = ?, auto_get = 0, - auto_status = NULL, last_error = ?, updated_at = ? + THEN 'combined' ELSE ? END, releases = ?, payloads = ?, auto_get = ?, + auto_status = ?, auto_checked_at = ?, last_error = ?, updated_at = ? WHERE owner = ? AND id = ?""", ( kind, json.dumps(releases), json.dumps(payloads), + 0 if failed else 1, + None if failed else (auto_status or None), + None if failed else now_utc_iso(), last_error, now_utc_iso(), owner, diff --git a/src/frontend/src/components/activity/SavedPanel.tsx b/src/frontend/src/components/activity/SavedPanel.tsx index f50ac1ef6..dc7c72d80 100644 --- a/src/frontend/src/components/activity/SavedPanel.tsx +++ b/src/frontend/src/components/activity/SavedPanel.tsx @@ -368,7 +368,7 @@ const SavedTable = ({

{inLine - ? "Each downloads on its own once there's room, checked in this order: one that doesn't fit yet doesn't hold up the ones after it." + ? "Each release downloads on its own once there's room, checked in this order: one that doesn't fit yet (a big audiobook waiting for your ratio, say) doesn't hold up the others, even those picked with it." : 'Choose releases for a book and it joins the queue, to download once there’s room.'}

diff --git a/tests/core/test_saved_autoget.py b/tests/core/test_saved_autoget.py index ee9e6236a..c425b1328 100644 --- a/tests/core/test_saved_autoget.py +++ b/tests/core/test_saved_autoget.py @@ -138,7 +138,7 @@ def test_gets_an_item_once_it_fits_then_removes_it(service, mam): def test_waits_for_unsatisfied_slots_and_says_how_many(service, mam): - mam.unsat_free = 1 + mam.unsat_free = 0 recorder = Recorder() item_id = _mark(service, [_payload("r1", torrent_id=1), _payload("r2", torrent_id=2)]) report = SavedAutoGetter(service, recorder.hooks()).run_check() @@ -151,6 +151,60 @@ def test_waits_for_unsatisfied_slots_and_says_how_many(service, mam): assert item["auto_get"] is True +def test_picks_that_fit_go_and_the_rest_keep_waiting(service, mam): + # One slot free, two picks: the first goes now, the second waits for the next slot. + mam.unsat_free = 1 + recorder = Recorder() + item_id = _mark(service, [_payload("r1", torrent_id=1), _payload("r2", torrent_id=2)]) + report = SavedAutoGetter(service, recorder.hooks()).run_check() + assert report.queued == [item_id] + assert [p["source_id"] for p in recorder.queued] == ["r1"] + item = service.get_item("user:1", item_id) + assert item is not None + assert [pick["release"]["source_id"] for pick in item["releases"]] == ["r2"] + assert item["auto_get"] is True # Still queued: nothing went wrong + assert item["last_error"] is None + assert item["auto_status"] == "Waiting for 1 unsatisfied slot" + assert recorder.notified == ["Book Title"] + + +def test_freeleech_and_small_picks_are_not_held_back_by_a_large_one(service, mam): + # Ratio 1.5, under the 2.0 target. A freeleech audiobook and a small ebook can go; + # the 40 GB non-freeleech audiobook picked with them waits for the ratio. + mam.stats = _stats(uploaded=150 * GIB, downloaded=100 * GIB) + mam.freeleech[2] = True + recorder = Recorder() + item_id = _mark( + service, + [ + _payload("ebook", torrent_id=1, size=5 * 1024**2), + _payload("fl-audiobook", torrent_id=2, size=30 * GIB), + _payload("big-audiobook", torrent_id=3, size=40 * GIB), + ], + ) + report = SavedAutoGetter(service, recorder.hooks()).run_check() + assert report.queued == [item_id] + assert sorted(p["source_id"] for p in recorder.queued) == ["ebook", "fl-audiobook"] + item = service.get_item("user:1", item_id) + assert item is not None + assert [pick["release"]["source_id"] for pick in item["releases"]] == ["big-audiobook"] + assert item["auto_get"] is True + assert item["auto_status"].startswith("Waiting for freeleech or ratio 2.00") + + +def test_the_cheapest_picks_get_the_free_slots(service, mam): + # One slot: the freeleech pick gets it, though it was picked after the paid one. + mam.unsat_free = 1 + mam.freeleech[2] = True + recorder = Recorder() + _mark( + service, + [_payload("paid", torrent_id=1, size=GIB), _payload("freeleech", torrent_id=2, size=GIB)], + ) + SavedAutoGetter(service, recorder.hooks()).run_check() + assert [p["source_id"] for p in recorder.queued] == ["freeleech"] + + def test_freeleech_goes_even_below_the_target_ratio(service, mam): # Ratio 1.5, under the 2.0 target: a freeleech torrent can't lower it, so it goes. mam.stats = _stats(uploaded=150 * GIB, downloaded=100 * GIB)