diff --git a/alembic/versions/0108_download_revisit_days.py b/alembic/versions/0108_download_revisit_days.py new file mode 100644 index 0000000..daac686 --- /dev/null +++ b/alembic/versions/0108_download_revisit_days.py @@ -0,0 +1,54 @@ +"""download_revisit_days — how far back a tick keeps looking for EDITED posts. + +Operator, 2026-09-23, pointing at a Floppystack post: *"this post has been +updated as he implements hot fixes — any chance we have a way to scan for or +see updated posts so we can update ours to match and pull the new attachments +and pictures etc."* + +A tick stopped after 20 contiguous already-have-it items. That is the right +instinct and the wrong unit: a post edited three days after publication sits +well below twenty seen items, so the walk turned around before reaching it. The +walk now needs BOTH a run of seen items and a post older than this many days +before it stops. + +A settings row rather than a constant (rule 25) because the right window is a +property of the CREATOR, not of FabledCurator — one artist appends hotfix +builds for a fortnight, another never touches a post again. 0 turns the revisit +off entirely and restores the pure count early-out. + +30 days is the operator's own number, 2026-09-23. + +Revision ID: 0108 +Revises: 0107 +Create Date: 2026-09-23 + +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0108" +down_revision: Union[str, None] = "0107" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # server_default so the existing single settings row gets the window without + # a data migration — and so an install that predates this column reads 30 + # rather than 0. 0 is a real, meaningful value here (revisit off), so the + # column must never be allowed to arrive at it by omission. + op.add_column( + "import_settings", + sa.Column( + "download_revisit_days", + sa.Integer(), + nullable=False, + server_default="30", + ), + ) + + +def downgrade() -> None: + op.drop_column("import_settings", "download_revisit_days") diff --git a/backend/app/api/settings.py b/backend/app/api/settings.py index 751e60e..4d90f3d 100644 --- a/backend/app/api/settings.py +++ b/backend/app/api/settings.py @@ -36,6 +36,7 @@ _EDITABLE_FIELDS = ( "download_validate_files", "download_schedule_default_seconds", "download_event_retention_days", + "download_revisit_days", "download_failure_warning_threshold", "series_suggest_enabled", "series_suggest_threshold", @@ -113,6 +114,12 @@ async def update_import_settings(): v = body["download_schedule_default_seconds"] if not isinstance(v, int) or isinstance(v, bool) or v < 60 or v > 86400: return _bad_int("download_schedule_default_seconds", 60, 86400) + # 0 is a real value (revisit off), so the floor is 0, not 1 — and the + # ceiling is a year, past which a "tick" is a backfill wearing a hat. + if "download_revisit_days" in body: + v = body["download_revisit_days"] + if not isinstance(v, int) or isinstance(v, bool) or v < 0 or v > 365: + return _bad_int("download_revisit_days", 0, 365) if "download_event_retention_days" in body: v = body["download_event_retention_days"] if not isinstance(v, int) or isinstance(v, bool) or v < 1 or v > 3650: diff --git a/backend/app/models/import_settings.py b/backend/app/models/import_settings.py index 3227724..7ac2cbd 100644 --- a/backend/app/models/import_settings.py +++ b/backend/app/models/import_settings.py @@ -68,6 +68,16 @@ class ImportSettings(Base): Integer, nullable=False, default=90, server_default="90", ) + # How far back a routine tick keeps looking after it has run out of new + # posts, so a creator who EDITS an older post to attach a hotfix build is + # still reached (ingest_core.DEFAULT_REVISIT_DAYS carries the reasoning). + # A knob rather than a constant because how long a creator keeps editing is + # a property of the creator, not of FabledCurator: 0 turns the revisit off + # and restores the pure count early-out. + download_revisit_days: Mapped[int] = mapped_column( + Integer, nullable=False, default=30, + server_default="30", + ) download_failure_warning_threshold: Mapped[int] = mapped_column( Integer, nullable=False, default=5, server_default="5", diff --git a/backend/app/services/download_backends.py b/backend/app/services/download_backends.py index 99b5cca..e2382f7 100644 --- a/backend/app/services/download_backends.py +++ b/backend/app/services/download_backends.py @@ -24,6 +24,7 @@ import asyncio from pathlib import Path from .gallery_dl import DownloadResult, ErrorType +from .ingest_core import DEFAULT_REVISIT_DAYS from .patreon_ingester import PatreonIngester from .patreon_resolver import extract_vanity, resolve_campaign_id_for_source from .platforms import known_platform_keys @@ -83,6 +84,7 @@ async def run_download( mode: str | None, gdl, sync_session_factory, + revisit_days: int = DEFAULT_REVISIT_DAYS, ) -> tuple[DownloadResult, str | None]: """Uniform download across backends — the download counterpart to `verify_source_credential`, so this module is the ONE place that knows how @@ -106,7 +108,7 @@ async def run_download( ), None if uses_native_ingester(platform): return await _run_native_ingester( - ctx, source_config, mode, gdl, sync_session_factory + ctx, source_config, mode, gdl, sync_session_factory, revisit_days ) result = await gdl.download( url=ctx["url"], @@ -146,6 +148,7 @@ def _campaign_resolution_error(platform: str, url: str) -> str: async def _run_native_ingester( ctx: dict, source_config, mode: str | None, gdl, sync_session_factory, + revisit_days: int = DEFAULT_REVISIT_DAYS, ) -> tuple[DownloadResult, str | None]: """Run the native ingester for a native platform in a worker thread (sync requests/subprocess). Patreon resolves a campaign id from the vanity URL; @@ -210,6 +213,9 @@ async def _run_native_ingester( mode=mode, resume_cursor=source_config.resume_cursor, time_budget_seconds=source_config.timeout, + # How far back a tick keeps looking for EDITED posts. The ingester + # applies it to ticks only; a backfill ignores it. + revisit_days=revisit_days, posts_base=int(overrides.get("_backfill_posts", 0)), # plan #709: live progress writes to this running event mid-walk. event_id=ctx.get("event_id"), diff --git a/backend/app/services/download_service.py b/backend/app/services/download_service.py index 67a5690..825bb82 100644 --- a/backend/app/services/download_service.py +++ b/backend/app/services/download_service.py @@ -39,6 +39,7 @@ from .gallery_dl import ( walk_completed, ) from .importer import Importer +from .ingest_core import DEFAULT_REVISIT_DAYS from .platforms import auth_type_for from .scheduler_service import set_platform_cooldown @@ -60,6 +61,7 @@ class DownloadService: importer: Importer, cred_service: CredentialService, sync_session_factory=None, + revisit_days: int = DEFAULT_REVISIT_DAYS, ): self.async_session = async_session self.sync_session = sync_session @@ -71,6 +73,12 @@ class DownloadService: # the multi-minute walk — see PatreonIngester). Only the patreon branch # of phase 2 uses it; gallery-dl sources leave it None. self.sync_session_factory = sync_session_factory + # ImportSettings.download_revisit_days — how far back a tick keeps + # looking for EDITED posts (ingest_core.DEFAULT_REVISIT_DAYS). Passed in + # rather than read here because the task already loads the settings row + # for rate_limit/validate_files, and a second load on every download + # would be the same row twice for one number. + self.revisit_days = revisit_days async def download_source(self, source_id: int) -> int: """Returns DownloadEvent.id. Idempotent: in-flight events are returned as-is.""" @@ -178,6 +186,7 @@ class DownloadService: return await run_download( ctx=ctx, source_config=source_config, skip_value=skip_value, mode=mode, gdl=self.gdl, sync_session_factory=self.sync_session_factory, + revisit_days=self.revisit_days, ) async def _phase1_setup(self, source_id: int) -> dict[str, Any]: diff --git a/backend/app/services/ingest_core.py b/backend/app/services/ingest_core.py index 2d71882..8105baf 100644 --- a/backend/app/services/ingest_core.py +++ b/backend/app/services/ingest_core.py @@ -31,6 +31,7 @@ import json import logging import time from collections.abc import Callable +from datetime import UTC, datetime, timedelta from sqlalchemy import delete, func, select, text from sqlalchemy.dialects.postgresql import insert as pg_insert @@ -51,6 +52,33 @@ log = logging.getLogger(__name__) # per-file HEADs. Headroom against paywalled/undownloadable items interleaving. _TICK_SEEN_THRESHOLD = 20 +# How far back a tick keeps looking even once everything is already-have-it — +# the REVISIT WINDOW. Operator, 2026-09-23, holding up a Floppystack post: +# *"this post has been updated as he implements hot fixes — any chance we have a +# way to scan for or see updated posts so we can update ours to match and pull +# the new attachments and pictures etc."* +# +# A creator who edits a three-day-old post to append a hotfix build was +# structurally unreachable: that post sits twenty-odd already-seen items down +# the feed, so the count early-out above fired before the walk ever got to it. +# Not a bug in the early-out — a COUNT cannot express "recent". +# +# So the early-out now needs BOTH conditions: the run of already-seen items AND +# a post published before the horizon. Strictly a widening. Two properties this +# shape has and a plain "walk the last N days" would not: +# +# * window 0 is exactly the old behaviour, so the feature has an off switch +# that costs nothing to reason about; +# * no window can make a tick stop EARLIER than it used to. A source paused +# for months has an unseen backlog stretching well past any horizon, and +# the walk still runs to the end of it — the horizon is a FLOOR on how far +# to look, never a ceiling. +# +# The live value is `ImportSettings.download_revisit_days` (rule 25 — an +# operator tuning how far back their creators edit should not need a redeploy). +# This is the fallback for a caller that passes none. +DEFAULT_REVISIT_DAYS = 30 + # plan #705 #7: after this many failed download/validate attempts a media is # "dead-lettered" and skipped on routine tick/backfill walks (recovery still # re-attempts it). Stops a permanently-broken media re-erroring forever. @@ -78,6 +106,28 @@ _LIVE_PROGRESS_INTERVAL = 5.0 _CANARY_MIN_SAMPLE = 30 +def _parse_published(raw: object) -> datetime | None: + """An ISO-8601 post date from either native client, as aware UTC. + + Patreon's `published_at` is tz-aware with a `Z` or `+00:00` offset; + SubscribeStar's is NAIVE (`_parse_ss_datetime` renders a parsed local + timestamp with no zone). A naive value is read as UTC — the alternative is + discarding it, and a post whose date we refuse to read is a post the revisit + window can never reach. + + Anything unparseable returns None, which reads downstream as "not provably + recent" and leaves the walk on its pre-revisit behaviour. Never raises: a + date we cannot read must not fail a walk that is otherwise working. + """ + if not isinstance(raw, str) or not raw.strip(): + return None + try: + parsed = datetime.fromisoformat(raw.strip().replace("Z", "+00:00")) + except ValueError: + return None + return parsed if parsed.tzinfo else parsed.replace(tzinfo=UTC) + + class Ingester: """Generic native-ingest orchestration. Subclass with a platform adapter (see the module docstring) — or construct directly with the keyword seams.""" @@ -132,11 +182,17 @@ class Ingester: resume_cursor: str | None = None, time_budget_seconds: float = 870.0, seen_threshold: int = _TICK_SEEN_THRESHOLD, + revisit_days: int = DEFAULT_REVISIT_DAYS, posts_base: int = 0, event_id: int | None = None, ) -> DownloadResult: """Walk + download for one source, returning a gallery-dl-shaped result. + `revisit_days` is the tick's revisit window (see DEFAULT_REVISIT_DAYS): + inside it a tick neither early-outs nor trusts the post-record gate, so + a post edited after we first captured it is re-read and its new + attachments downloaded. 0 turns the window off. + `mode` is "tick" | "backfill" | "recovery" | "recapture". Recovery bypasses the tier-1 seen-ledger AND the dead-letter ledger (tier-2 disk still skips kept files). Recapture (#830) is the cheap "re-grab post @@ -179,6 +235,21 @@ class Ingester: # no media download, no post-record stub. Absent on stub/not-yet-migrated # clients → nothing is ever treated as gated. post_is_gated = getattr(self.client, "post_is_gated", None) + # The revisit window (see DEFAULT_REVISIT_DAYS). `post_meta` is an + # existing client seam — both native clients already implement it, for + # a preview sample whose caller has since gone, so this needed no new + # contract, only a live consumer for one. Absent seam, an unreadable + # date or a window of 0 → `horizon` never matches and the walk behaves + # exactly as it did before 2026-09-23. + # + # The window applies to TICKS only. A backfill is gated on purpose + # (capture each post once) and `recapture` mode already exists for the + # operator-driven "re-read every body" pass; a horizon there would be a + # third overlapping answer to a question that has two. + post_meta = getattr(self.client, "post_meta", None) + horizon: datetime | None = None + if mode == "tick" and revisit_days > 0 and post_meta is not None: + horizon = datetime.now(UTC) - timedelta(days=revisit_days) start = time.monotonic() last_live = start # plan #709: last live-progress write timestamp log_lines: list[str] = [] @@ -210,6 +281,12 @@ class Ingester: # absolute across chunks instead of an inflating sum. posts_processed # stays the gross per-chunk count used for the run summary. chunk_new_posts = 0 + # Posts inside the revisit window that we had already captured, and the + # media those revisits turned up. Reported in the run summary — the + # operator's ask was to SEE the updated posts, not only to end up with + # their files ("so we can update ours to match"). + revisited = 0 + revisit_downloads = 0 consecutive_seen = 0 emitted_cursor: str | None = None reached_bottom = False @@ -322,6 +399,20 @@ class Ingester: # resume_cursor None, so everything counts. if not (resume_cursor and page_cursor == resume_cursor): chunk_new_posts += 1 + # Inside the revisit window? Computed per post rather than + # "stop once one post is old" because the feed is only MOSTLY + # date-ordered — a pinned or re-pinned post can sit above older + # ones, and one such post must not end the walk. + in_window = False + if horizon is not None: + published = _parse_published((post_meta(post) or {}).get("date")) + in_window = published is not None and published >= horizon + # Set by the post-record block below when this post was already + # captured on an earlier walk. Stays False when the platform has + # no post-record seam, so the revisit accounting simply reports + # nothing rather than guessing. + post_already_recorded = False + downloaded_before = downloaded # Tier-gated post (#874): the account can't fully view it, so # Patreon serves only blurred locked-preview media. Skip it # ENTIRELY — no media download AND no post-record stub (operator @@ -353,11 +444,31 @@ class Ingester: set() if recapture_records else self._seen_keys(source_id, [pkey]) ) - if pkey not in already: - rec = write_post_record(post, artist_slug) - posts_recorded += 1 - if rec.body_chars: - posts_with_body += 1 + post_already_recorded = pkey in already + # A post inside the revisit window is re-read even + # though the gate has it: that gate's whole job is to + # stop us paying for a post twice, and an EDITED post is + # not the same post. `revisit=True` keeps the cost at + # zero requests — the downloader re-reads the body from + # the feed response already in hand and declines to + # write at all if that body came back empty, so a + # detail-fetched body is never overwritten by a blank. + if not post_already_recorded or in_window: + rec = write_post_record( + post, artist_slug, revisit=post_already_recorded, + ) + if not post_already_recorded: + # FIRST captures only feed the #862 body canary. + # A revisit legitimately comes back empty — a + # post whose body only ever arrived from the + # detail endpoint has none in the feed, and the + # downloader declines to write it. Counting + # those into the sample would walk the canary + # toward firing on healthy ticks, which is the + # one thing a drift alarm must never do. + posts_recorded += 1 + if rec.body_chars: + posts_with_body += 1 if rec.path is not None: post_records.append(str(rec.path)) self._mark_seen(source_id, [(pkey, ppid)]) @@ -367,7 +478,8 @@ class Ingester: # a 0-char body is the "why is this one empty" answer. log_lines.append( f" post {ppid} [{rec.post_type or '?'}] " - f"body: {rec.body_chars} chars" + + ("re-read, " if post_already_recorded else "") + + f"body: {rec.body_chars} chars" + ("" if rec.body_chars else " — EMPTY") + (f" — {rec.title}" if rec.title else "") ) @@ -451,7 +563,15 @@ class Ingester: to_fail.append((key, media_item.post_id, outcome.error or "error")) # An error neither advances nor resets the run-of-seen. - if mode == "tick" and consecutive_seen >= seen_threshold: + # `not in_window` is the revisit window's half of the + # early-out: a run of already-seen items is only permission + # to stop once the walk is BELOW the horizon. Both halves, + # never either alone — see DEFAULT_REVISIT_DAYS. + if ( + mode == "tick" + and not in_window + and consecutive_seen >= seen_threshold + ): early_out = True break @@ -465,6 +585,21 @@ class Ingester: if to_fail: self._record_failures(source_id, to_fail) + # An already-captured post that yielded NEW media is an edited + # post — the operator's Floppystack case, and the one thing in + # this walk worth naming individually in the run log. The media + # half needed no new detection: `extract_media` reads the media + # list off the live feed response, so a hotfix build appended + # last night is simply a ledger key we have never seen. + new_here = downloaded - downloaded_before + if post_already_recorded and new_here: + revisited += 1 + revisit_downloads += new_here + log_lines.append( + f" post {post.get('id')} — updated: " + f"{new_here} new file(s)" + ) + # plan #709: time-throttled live progress to the running event so # the Downloads view ticks ~every 5s, independent of page size. now = time.monotonic() @@ -527,6 +662,12 @@ class Ingester: # visible in the Raw stdout (e.g. "bodies 3/180" reads as off). + (f", bodies {posts_with_body}/{posts_recorded}" if posts_recorded else "") + (f", {gated_skipped} gated-skipped" if gated_skipped else "") + # Only when it happened: on a quiet tick this is 0 and saying so + # every run would bury the times it is not. + + ( + f", {revisited} post(s) updated ({revisit_downloads} new file(s))" + if revisited else "" + ) + (", reached end" if reached_bottom else "") + (", time-boxed" if budget_hit else "") ) diff --git a/backend/app/services/patreon_client.py b/backend/app/services/patreon_client.py index 07e2c9c..d968ae8 100644 --- a/backend/app/services/patreon_client.py +++ b/backend/app/services/patreon_client.py @@ -483,8 +483,14 @@ class PatreonClient: @staticmethod def post_meta(post: dict) -> dict: - """Title + published date for a post — for the preview sample (plan #708 - B4). Part of the client contract `ingest_core.Ingester.preview` calls.""" + """Title + published date for a post. Part of the client contract. + + Written for a preview sample (plan #708 B4) whose caller has since gone; + as of 2026-09-23 its consumer is the core's REVISIT WINDOW, which needs + a post's date to know whether a tick is still inside it. Both native + clients answer in the same shape — an ISO-8601 string under `date`, or + None — so the core reads a date without knowing the platform. + """ attrs = post.get("attributes") or {} title = attrs.get("title") published = attrs.get("published_at") diff --git a/backend/app/services/patreon_downloader.py b/backend/app/services/patreon_downloader.py index 47132e7..c588b3d 100644 --- a/backend/app/services/patreon_downloader.py +++ b/backend/app/services/patreon_downloader.py @@ -340,7 +340,7 @@ class PatreonDownloader(BaseNativeDownloader): def _write_sidecar_data( self, post: dict, sidecar_path: Path, *, source_url: str | None = None, - minimal: bool = False, + minimal: bool = False, detail_fetch: bool = True, ) -> Path: """Serialize the post's metadata to `sidecar_path`. The post-only record (`write_post_record`) writes the FULL post (body/title/date/url); the @@ -364,7 +364,13 @@ class PatreonDownloader(BaseNativeDownloader): # dict — so a multi-image post fetches detail at most once, the post-record # body-length read reuses it, and a fully-seen post (no fresh download → no # sidecar write) never pays the extra GET. - if (not content or not content.strip()) and self._content_fetcher: + # `detail_fetch=False` on a REVISIT (a post inside the tick's revisit + # window that we already captured): re-read the body from the feed + # response we are holding and pay nothing. Without this a 30-day window + # would buy one detail GET per body-less post per tick, forever — a + # per-creator cost that grows with how prolific they are, to re-fetch a + # body we already stored. + if (not content or not content.strip()) and self._content_fetcher and detail_fetch: fetched = self._content_fetcher(str(post.get("id") or "")) if fetched: content = fetched @@ -386,7 +392,9 @@ class PatreonDownloader(BaseNativeDownloader): sidecar_path.write_text(json.dumps(data, indent=2)) return sidecar_path - def write_post_record(self, post: dict, artist_slug: str) -> PostRecordOutcome: + def write_post_record( + self, post: dict, artist_slug: str, *, revisit: bool = False, + ) -> PostRecordOutcome: """Write a post-ONLY sidecar (no media file) for a media-less post, so the importer can still upsert the Post + its body — text posts often hold the only copy of an external link. Named `_post.json`: the @@ -397,6 +405,18 @@ class PatreonDownloader(BaseNativeDownloader): Returns a PostRecordOutcome (path None when the post has no id) carrying the captured body's shape — post_type + final char count — so the engine can log per-post handling without re-reading the post itself. + + `revisit=True` is the tick re-reading a post it already captured + (ingest_core's revisit window, #...). Two differences, both about not + making an update cost more than it is worth: + + * no detail-fetch — the body comes from the feed response already in + hand, so a revisit costs zero requests; + * a body that comes back empty writes NOTHING and returns `path=None`. + On a first capture an empty body is the truth about the post; on a + revisit it usually just means this post's body only ever came from + the detail endpoint we just declined to call, and writing it would + blank a stored body to say something we never learned. """ attrs = post.get("attributes") or {} title = attrs.get("title") if isinstance(attrs.get("title"), str) else None @@ -406,9 +426,17 @@ class PatreonDownloader(BaseNativeDownloader): return PostRecordOutcome( path=None, post_type=post_type, title=title, body_chars=0, ) + if revisit: + feed_body = post_body_html(attrs) + if not (isinstance(feed_body, str) and feed_body.strip()): + return PostRecordOutcome( + path=None, post_type=post_type, title=title, body_chars=0, + ) post_dir = self.images_root / artist_slug / "patreon" / post_dir_name(post) post_dir.mkdir(parents=True, exist_ok=True) - path = self._write_sidecar_data(post, post_dir / "_post.json") + path = self._write_sidecar_data( + post, post_dir / "_post.json", detail_fetch=not revisit, + ) # _write_sidecar_data has by now memoized any detail-fetched body onto # post["attributes"]["content"], so re-read it for the FINAL char count. body = (post.get("attributes") or {}).get("content") diff --git a/backend/app/services/subscribestar_client.py b/backend/app/services/subscribestar_client.py index 9ae848a..2589e79 100644 --- a/backend/app/services/subscribestar_client.py +++ b/backend/app/services/subscribestar_client.py @@ -742,8 +742,15 @@ class SubscribeStarClient: @staticmethod def post_meta(post: dict) -> dict: - """Title + date for the preview sample. Title is synthesized from the body - (SubscribeStar has no title field).""" + """Title + date. Title is None — SubscribeStar has no title field, and + the importer synthesizes one from the body. + + `date` is the contract the core's REVISIT WINDOW reads (2026-09-23): + ISO-8601 or None. NAIVE here, because `_parse_ss_datetime` renders a + parsed local timestamp with no zone; the core reads a naive date as UTC + rather than discarding it, since a date it refuses to read is a post + the window can never reach. + """ attrs = post.get("attributes") or {} return {"title": None, "date": attrs.get("published_at")} diff --git a/backend/app/services/subscribestar_downloader.py b/backend/app/services/subscribestar_downloader.py index 9845a3b..fadb89e 100644 --- a/backend/app/services/subscribestar_downloader.py +++ b/backend/app/services/subscribestar_downloader.py @@ -184,10 +184,20 @@ class SubscribeStarDownloader(BaseNativeDownloader): sidecar_path.write_text(json.dumps(data, indent=2)) return sidecar_path - def write_post_record(self, post: dict, artist_slug: str) -> PostRecordOutcome: + def write_post_record( + self, post: dict, artist_slug: str, *, revisit: bool = False, + ) -> PostRecordOutcome: """Write the post-first `_post.json` (body/links/metadata) — the sole writer of the post record on the native path. SubscribeStar's body is - already in the feed HTML, so no detail-fetch is needed.""" + already in the feed HTML, so no detail-fetch is needed. + + `revisit=True` is the tick re-reading a post it already captured + (ingest_core's revisit window). The no-detail-fetch half of that + contract is free here — there is no detail endpoint — but the + don't-blank-a-stored-body half still applies: a chunk that parsed with + no content must not overwrite a body we already have. Same guarantee as + the Patreon downloader, for the same reason, so a walk behaves the same + on both platforms.""" attrs = post.get("attributes") or {} title = attrs.get("title") if isinstance(attrs.get("title"), str) else None post_type = attrs.get("post_type") if isinstance(attrs.get("post_type"), str) else None @@ -196,6 +206,12 @@ class SubscribeStarDownloader(BaseNativeDownloader): return PostRecordOutcome( path=None, post_type=post_type, title=title, body_chars=0, ) + if revisit: + body = attrs.get("content") + if not (isinstance(body, str) and body.strip()): + return PostRecordOutcome( + path=None, post_type=post_type, title=title, body_chars=0, + ) post_dir = self.images_root / artist_slug / "subscribestar" / post_dir_name(post) post_dir.mkdir(parents=True, exist_ok=True) path = self._write_sidecar_data(post, post_dir / "_post.json") diff --git a/backend/app/tasks/download.py b/backend/app/tasks/download.py index 49af07e..65b2664 100644 --- a/backend/app/tasks/download.py +++ b/backend/app/tasks/download.py @@ -177,6 +177,10 @@ def download_source(self, source_id: int, _serialize_waits: int = 0) -> int: settings = ImportSettings.load_sync(sync_session) rate_limit = settings.download_rate_limit_seconds validate_files = settings.download_validate_files + # How far back a tick keeps looking for EDITED posts. Read here + # with the other downloader knobs, off the row this block is + # already holding open. + revisit_days = settings.download_revisit_days gdl = GalleryDLService( images_root=IMAGES_ROOT, @@ -207,6 +211,7 @@ def download_source(self, source_id: int, _serialize_waits: int = 0) -> int: # the walk). Same factory the importer's sync session # comes from — a different DB connection per checkout. sync_session_factory=SyncFactory, + revisit_days=revisit_days, ) return await svc.download_source(source_id) finally: diff --git a/frontend/src/components/subscriptions/SettingsTab.vue b/frontend/src/components/subscriptions/SettingsTab.vue index 16bb78b..c94d496 100644 --- a/frontend/src/components/subscriptions/SettingsTab.vue +++ b/frontend/src/components/subscriptions/SettingsTab.vue @@ -153,6 +153,24 @@ + + + +
+ A routine check keeps looking back this far after it runs out of + new posts, so a creator who edits an older post to add a file is + still picked up. New attachments download; the post text is + re-read from the same page, costing nothing extra. 0 turns this + off. Default 30. +
+
+
{{ importStore.settingsError }} @@ -209,6 +227,7 @@ const dl = reactive({ download_schedule_default_seconds: 28800, download_event_retention_days: 90, download_failure_warning_threshold: 5, + download_revisit_days: 30, extdl_mega_enabled: true, extdl_gdrive_enabled: true, extdl_mediafire_enabled: true, diff --git a/tests/test_api_settings_downloader.py b/tests/test_api_settings_downloader.py index b784263..426d5f6 100644 --- a/tests/test_api_settings_downloader.py +++ b/tests/test_api_settings_downloader.py @@ -65,3 +65,41 @@ async def test_extdl_toggle_rejects_non_bool(client): "/api/settings/import", json={"extdl_gdrive_enabled": "nope"} ) assert resp.status_code == 400 + + +# -- download_revisit_days: how far back a tick looks for EDITED posts -------- + + +@pytest.mark.asyncio +async def test_revisit_window_defaults_to_thirty_days(client): + body = await (await client.get("/api/settings/import")).get_json() + assert body["download_revisit_days"] == 30 + + +@pytest.mark.asyncio +async def test_revisit_window_is_settable(client): + resp = await client.patch( + "/api/settings/import", json={"download_revisit_days": 7} + ) + assert resp.status_code == 200 + assert (await resp.get_json())["download_revisit_days"] == 7 + + +@pytest.mark.asyncio +async def test_zero_is_accepted_because_it_is_the_off_switch(client): + """0 turns the revisit off and restores the pure count early-out. Pinned + because the obvious bounds check for a "days" field is `>= 1`, and that + would take the off switch away without anything failing.""" + resp = await client.patch( + "/api/settings/import", json={"download_revisit_days": 0} + ) + assert resp.status_code == 200 + assert (await resp.get_json())["download_revisit_days"] == 0 + + +@pytest.mark.asyncio +async def test_a_negative_window_is_refused(client): + resp = await client.patch( + "/api/settings/import", json={"download_revisit_days": -1} + ) + assert resp.status_code == 400 diff --git a/tests/test_patreon_downloader.py b/tests/test_patreon_downloader.py index a26ffb5..2f56d12 100644 --- a/tests/test_patreon_downloader.py +++ b/tests/test_patreon_downloader.py @@ -658,6 +658,53 @@ def test_write_post_record_writes_enriched_post_only_sidecar(tmp_path): assert rec.body_chars == len("

text post body

") +# The revisit contract (2026-09-23): a tick re-reading a post it already +# captured, so an edit made after first capture reaches us. Both halves exist to +# stop an update costing more than it is worth. + + +def test_a_revisit_re_reads_the_body_without_paying_for_a_detail_fetch(tmp_path): + """A 30-day window would otherwise buy one detail GET per body-less post per + tick, forever — a per-creator cost that grows with how prolific they are, to + re-fetch a body we already stored.""" + calls: list[str] = [] + + def _fetcher(post_id: str) -> str: + calls.append(post_id) + return "

detail body

" + + dl = PatreonDownloader( + images_root=tmp_path, cookies_path=None, validate=False, + session=_FakeSession(), content_fetcher=_fetcher, + ) + post = _post() + post["attributes"]["content"] = "

feed body

" + rec = dl.write_post_record(post, "artist-x", revisit=True) + + assert calls == [] + assert json.loads(rec.path.read_text())["content"] == "

feed body

" + + +def test_a_revisit_with_an_empty_body_writes_nothing_at_all(tmp_path): + """The data-loss guard. On a FIRST capture an empty body is the truth about + the post; on a revisit it usually means the body only ever came from the + detail endpoint we just declined to call. Writing it would blank a stored + body to say something we never learned.""" + dl = PatreonDownloader( + images_root=tmp_path, cookies_path=None, validate=False, + session=_FakeSession(), content_fetcher=lambda _pid: "

detail

", + ) + post = _post() + post["attributes"]["content"] = "" + rec = dl.write_post_record(post, "artist-x", revisit=True) + + assert rec.path is None + assert rec.body_chars == 0 + # Not "wrote an empty file" — nothing was written, so a record captured on + # an earlier walk is still exactly what it was. + assert not list(tmp_path.rglob("_post.json")) + + def test_write_post_record_none_without_post_id(tmp_path): dl = PatreonDownloader( images_root=tmp_path, cookies_path=None, validate=False, diff --git a/tests/test_patreon_ingester.py b/tests/test_patreon_ingester.py index 59c05b7..2363a6a 100644 --- a/tests/test_patreon_ingester.py +++ b/tests/test_patreon_ingester.py @@ -7,6 +7,8 @@ real CDN. The ledger is real (a sync sessionmaker bound to the test engine), so the tier-1 skip and the idempotent mark-seen run against actual rows. """ +from datetime import UTC, datetime, timedelta + import pytest from sqlalchemy import func, select from sqlalchemy.orm import sessionmaker @@ -50,9 +52,15 @@ class _FakeClient: """Stub PatreonClient. `pages` is a list of (page_cursor, [posts]); each post is (post_id, [MediaItem]). `raise_on_first` lets a test trip drift.""" - def __init__(self, pages, raise_exc=None, empty_body=False, gated=None): + def __init__(self, pages, raise_exc=None, empty_body=False, gated=None, + published=None): self._pages = pages self._raise_exc = raise_exc + # {post_id: ISO-8601 published_at} for the revisit window. Absent → the + # date reads as None, which is how EVERY test written before the window + # existed keeps its old behaviour: an unreadable date is never inside + # the horizon, so the count early-out stands alone. + self._published = dict(published or {}) # empty_body simulates a body-field schema break: every post comes back # with no content (the #862 canary's trip condition). self._empty_body = empty_body @@ -89,7 +97,10 @@ class _FakeClient: return post["_media"] def post_meta(self, post): - return {"title": post.get("id"), "date": None} + return { + "title": post.get("id"), + "date": self._published.get(str(post.get("id") or "")), + } @staticmethod def post_is_gated(post): @@ -113,6 +124,7 @@ class _FakeDownloader: self.error = set(error or ()) self.download_calls = 0 self.post_records = 0 + self.post_revisits = 0 def download_post(self, post, media_items, artist_slug, *, is_seen, should_stop=lambda: False, recapture=False): @@ -146,8 +158,19 @@ class _FakeDownloader: outcomes.append(MediaOutcome(media=m, status="downloaded", path=p, error=None)) return outcomes - def write_post_record(self, post, artist_slug): + def write_post_record(self, post, artist_slug, *, revisit=False): self.post_records += 1 + if revisit: + self.post_revisits += 1 + attrs_ = post.get("attributes") or {} + body_ = attrs_.get("content") + # Mirrors the real downloaders' revisit contract: a re-read whose body + # came back empty writes NOTHING rather than blanking a stored one. + if revisit and not (isinstance(body_, str) and body_.strip()): + return PostRecordOutcome( + path=None, post_type=attrs_.get("post_type"), + title=attrs_.get("title"), body_chars=0, + ) p = self.tmp_path / f"{post.get('id')}__post.json" p.write_text("{}") attrs = post.get("attributes") or {} @@ -1114,3 +1137,289 @@ async def test_body_canary_silent_below_min_sample(source_id, sync_engine, tmp_p ) assert result.success is True assert result.error_type is None + + +# --- the revisit window: reaching posts that were EDITED after capture ------- +# +# Operator, 2026-09-23, on a Floppystack post: *"this post has been updated as +# he implements hot fixes — any chance we have a way to scan for or see updated +# posts so we can update ours to match and pull the new attachments and +# pictures etc."* +# +# A tick stopped after N contiguous already-have-it items, so an edit to a +# three-day-old post was structurally unreachable: it sits well below twenty +# seen items. The early-out now needs BOTH that run AND a post published before +# the horizon. + + +def _iso(days_ago): + return (datetime.now(UTC) - timedelta(days=days_ago)).isoformat() + + +def _seed_seen(sync_engine, source_id, media): + factory = sessionmaker(sync_engine, expire_on_commit=False) + with factory() as s: + for m in media: + s.add(PatreonSeenMedia( + source_id=source_id, filehash=_ledger_key(m), post_id=m.post_id, + )) + s.commit() + + +@pytest.mark.asyncio +async def test_a_run_of_seen_items_does_not_stop_a_tick_inside_the_window( + source_id, sync_engine, tmp_path, +): + """The bug itself. Three all-seen posts, threshold 2 — the old walk turned + around at the second and never looked at the third, which is exactly where + an edited post lives.""" + seen = [_media(f"p{i}", 1) for i in range(1, 4)] + _seed_seen(sync_engine, source_id, seen) + + client = _FakeClient( + [(None, [(m.post_id, [m]) for m in seen])], + published={m.post_id: _iso(3) for m in seen}, + ) + ing = _ingester(sync_engine, tmp_path, client, _FakeDownloader(tmp_path)) + + result = ing.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", seen_threshold=2, + revisit_days=30, + ) + + assert result.success is True + assert client.consumed_posts == 3 + + +@pytest.mark.asyncio +async def test_the_early_out_still_fires_once_the_walk_is_below_the_horizon( + source_id, sync_engine, tmp_path, +): + """The other half, and the one that keeps a tick cheap. Same three posts, + published outside the window → the count early-out stands exactly as it + did. Asserted beside the test above because the window is only correct if + BOTH conditions are required; either one alone is a different feature.""" + seen = [_media(f"p{i}", 1) for i in range(1, 4)] + _seed_seen(sync_engine, source_id, seen) + + client = _FakeClient( + [(None, [(m.post_id, [m]) for m in seen])], + published={m.post_id: _iso(90) for m in seen}, + ) + ing = _ingester(sync_engine, tmp_path, client, _FakeDownloader(tmp_path)) + + result = ing.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", seen_threshold=2, + revisit_days=30, + ) + + assert result.success is True + assert client.consumed_posts == 2 + + +@pytest.mark.asyncio +async def test_a_window_of_zero_is_the_behaviour_that_shipped_before_it( + source_id, sync_engine, tmp_path, +): + """The off switch. Recent posts, a window of 0 → the walk stops on the + count alone, so an operator who wants the old cheap tick has one.""" + seen = [_media(f"p{i}", 1) for i in range(1, 4)] + _seed_seen(sync_engine, source_id, seen) + + client = _FakeClient( + [(None, [(m.post_id, [m]) for m in seen])], + published={m.post_id: _iso(1) for m in seen}, + ) + ing = _ingester(sync_engine, tmp_path, client, _FakeDownloader(tmp_path)) + + ing.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", seen_threshold=2, + revisit_days=0, + ) + + assert client.consumed_posts == 2 + + +@pytest.mark.asyncio +async def test_a_backfill_ignores_the_window_because_it_never_early_outs( + source_id, sync_engine, tmp_path, +): + """Stated so the window cannot quietly acquire a second job. A backfill + walks to the bottom regardless, and `recapture` is the mode that re-reads + every body — a horizon there would be a third answer to a two-answer + question.""" + seen = [_media(f"p{i}", 1) for i in range(1, 4)] + _seed_seen(sync_engine, source_id, seen) + + client = _FakeClient( + [(None, [(m.post_id, [m]) for m in seen])], + published={m.post_id: _iso(90) for m in seen}, + ) + downloader = _FakeDownloader(tmp_path) + ing = _ingester(sync_engine, tmp_path, client, downloader) + + ing.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="backfill", seen_threshold=2, + revisit_days=30, + ) + + assert client.consumed_posts == 3 + assert downloader.post_revisits == 0 + + +@pytest.mark.asyncio +async def test_an_edited_post_inside_the_window_downloads_its_new_attachment( + source_id, sync_engine, tmp_path, +): + """The operator's case, end to end. + + Walk one captures a post with one file. The creator then edits it to attach + a hotfix build. Walk two must download that file AND name the post as + updated — the ask was to SEE which posts changed, not only to end up with + their bytes. + + The detection half needed nothing new: `extract_media` reads the media list + off the LIVE feed response every walk, so a newly attached file is simply a + ledger key we have never seen. Only reaching the post was missing. + """ + original = _media("p1", 1) + client1 = _FakeClient( + [(None, [("p1", [original])])], published={"p1": _iso(3)}, + ) + ing1 = _ingester(sync_engine, tmp_path, client1, _FakeDownloader(tmp_path)) + ing1.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + hotfix = _media("p1", 2) + client2 = _FakeClient( + [(None, [("p1", [original, hotfix])])], published={"p1": _iso(3)}, + ) + downloader2 = _FakeDownloader(tmp_path) + ing2 = _ingester(sync_engine, tmp_path, client2, downloader2) + result = ing2.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + assert result.files_downloaded == 1 + assert "1 post(s) updated (1 new file(s))" in result.stdout + assert "post p1 — updated: 1 new file(s)" in result.stdout + # The post record was re-read, not skipped by the capture gate. + assert downloader2.post_revisits == 1 + + +@pytest.mark.asyncio +async def test_a_post_below_the_horizon_keeps_its_capture_gate( + source_id, sync_engine, tmp_path, +): + """The gate still does its job everywhere the window does not reach — else + the window would have quietly become "re-read every post forever".""" + old = _media("pold", 1) + client1 = _FakeClient([(None, [("pold", [old])])], published={"pold": _iso(90)}) + ing1 = _ingester(sync_engine, tmp_path, client1, _FakeDownloader(tmp_path)) + ing1.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + client2 = _FakeClient([(None, [("pold", [old])])], published={"pold": _iso(90)}) + downloader2 = _FakeDownloader(tmp_path) + ing2 = _ingester(sync_engine, tmp_path, client2, downloader2) + ing2.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + assert downloader2.post_records == 0 + + +@pytest.mark.asyncio +async def test_a_revisit_that_finds_nothing_new_is_not_reported_as_an_update( + source_id, sync_engine, tmp_path, +): + """Most revisits find nothing — that is the normal case, and a run summary + claiming "1 post(s) updated" on every tick would train the operator to stop + reading it (lesson: a signal that is always on is not a signal).""" + m = _media("p1", 1) + client1 = _FakeClient([(None, [("p1", [m])])], published={"p1": _iso(3)}) + ing1 = _ingester(sync_engine, tmp_path, client1, _FakeDownloader(tmp_path)) + ing1.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + client2 = _FakeClient([(None, [("p1", [m])])], published={"p1": _iso(3)}) + downloader2 = _FakeDownloader(tmp_path) + ing2 = _ingester(sync_engine, tmp_path, client2, downloader2) + result = ing2.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + assert result.files_downloaded == 0 + assert "updated" not in result.stdout + assert downloader2.post_revisits == 1 + + +@pytest.mark.asyncio +async def test_revisits_do_not_feed_the_body_drift_canary( + source_id, sync_engine, tmp_path, +): + """#862's canary fails a run that recorded a meaningful sample of posts and + got a body from NONE of them. A revisit legitimately comes back empty — the + post's body only ever arrived from the detail endpoint, which a revisit + declines to call — so counting revisits into that sample would walk the + alarm toward firing on healthy ticks. First captures only.""" + posts = [(f"c{i}", []) for i in range(_CANARY_MIN_SAMPLE)] + published = {pid: _iso(3) for pid, _ in posts} + + client1 = _FakeClient([(None, posts)], published=published) + ing1 = _ingester(sync_engine, tmp_path, client1, _FakeDownloader(tmp_path)) + first = ing1.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + assert first.success is True + + # Second walk: every post is a revisit, and every body comes back empty. + client2 = _FakeClient([(None, posts)], published=published, empty_body=True) + ing2 = _ingester(sync_engine, tmp_path, client2, _FakeDownloader(tmp_path)) + second = ing2.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", revisit_days=30, + ) + + assert second.success is True + assert second.error_type is not ErrorType.API_DRIFT + + +@pytest.mark.asyncio +async def test_a_post_whose_date_will_not_parse_falls_back_to_the_count( + source_id, sync_engine, tmp_path, +): + """A date we cannot read must not fail the walk, and must not be guessed + into the window. It reads as "not provably recent", which leaves that post + on the behaviour it had before the window existed.""" + seen = [_media(f"p{i}", 1) for i in range(1, 4)] + _seed_seen(sync_engine, source_id, seen) + + client = _FakeClient( + [(None, [(m.post_id, [m]) for m in seen])], + published={m.post_id: "last Tuesday" for m in seen}, + ) + ing = _ingester(sync_engine, tmp_path, client, _FakeDownloader(tmp_path)) + + result = ing.run( + source_id=source_id, campaign_id="c1", artist_slug="ingest", + url="https://patreon.com/ingest", mode="tick", seen_threshold=2, + revisit_days=30, + ) + + assert result.success is True + assert client.consumed_posts == 2 diff --git a/tests/test_subscribestar_native.py b/tests/test_subscribestar_native.py index 0545a7b..5b09809 100644 --- a/tests/test_subscribestar_native.py +++ b/tests/test_subscribestar_native.py @@ -453,6 +453,29 @@ def test_write_post_record(tmp_path): assert rec.body_chars > 0 +def test_a_revisit_with_an_empty_body_writes_nothing(tmp_path): + """The revisit contract's second half, which applies here even though the + first (no detail-fetch) is free on SubscribeStar — there is no detail + endpoint. A chunk that parsed with no content must not overwrite a body we + already have, so a walk behaves the same on both platforms.""" + dl = _downloader(tmp_path) + post = _post("222") + post["attributes"]["content"] = "" + rec = dl.write_post_record(post, "artist-x", revisit=True) + + assert rec.path is None + assert rec.body_chars == 0 + assert not list(tmp_path.rglob("_post.json")) + + +def test_a_revisit_that_still_has_a_body_writes_it(tmp_path): + dl = _downloader(tmp_path) + rec = dl.write_post_record(_post("333"), "artist-x", revisit=True) + + assert rec.path is not None + assert "body text" in json.loads(rec.path.read_text())["content"] + + def test_skip_seen_does_not_download(tmp_path): session = _FakeSession() dl = _downloader(tmp_path, session=session)