diff --git a/docs/plans/paid-model-price-tracking.md b/docs/plans/paid-model-price-tracking.md index 8555e6b6..17f393f2 100644 --- a/docs/plans/paid-model-price-tracking.md +++ b/docs/plans/paid-model-price-tracking.md @@ -652,3 +652,22 @@ Recorded options for mature models, deliberately **not** implemented: "convenience" — the page route is demonstrably unreliable (one host challenges non-browser clients, the other two hide mature models from anonymous visitors), so a supported field is the only way for any third-party tool to show prices for the models where creators monetize most. + +### 11.11 Price capture is independent of the metadata TTL + +Found in a real instance: after enabling price tracking, a normal "Check updates" captured exactly +**one** price out of 718 models. The price capture only ran when the *version list* was re-fetched +(`refresh_succeeded`), so it inherited the metadata TTL: with 24 h metadata and 24 h price TTLs, only +the 6 models whose metadata happened to be stale were ever priced. + +The cached record already carries the gate, so the price pass now runs off whichever version list is +available — freshly fetched or stored — and applies the result without touching `last_checked_at`, so +a price-only pass cannot silently extend the metadata TTL. Two related semantics: + +* A **failed** attempt (`price_check_attempted_at`) satisfies the price TTL, so a mature model whose + page no host will serve is not retried on every single update check. +* An explicitly **forced** check re-prices within the TTL (`_should_fetch_prices(..., force=True)`). + +Verified by copying a real instance's update DB into a sandbox and running a non-forced check: +`bulk metadata fetches: 0` (version lists came entirely from cache) while priced versions went +**1 → 20** and the panel listed 19 alerts. diff --git a/py/services/model_update_service.py b/py/services/model_update_service.py index 10fdce4e..81811a20 100644 --- a/py/services/model_update_service.py +++ b/py/services/model_update_service.py @@ -1472,16 +1472,33 @@ class ModelUpdateService: # Optional price capture. Runs after the version list is known so only the # models that actually carry a gate pay for a second (page) request, and # stays outside the lock along with the other network I/O. + # + # Deliberately independent of the metadata TTL: the cached record already + # carries the gate, so enabling price tracking (or letting the *price* TTL + # lapse) must not wait for the version list to go stale. Otherwise a user + # who turns the feature on prices only the handful of models that happened + # to need a metadata refresh that round. + priced_versions: Optional[List[ModelVersionRecord]] = None + price_candidates: Optional[Sequence[ModelVersionRecord]] = None + if refresh_succeeded and isinstance(fetched_versions, list) and fetched_versions: + price_candidates = fetched_versions + elif existing is not None and existing.versions: + price_candidates = existing.versions + if ( - refresh_succeeded - and isinstance(fetched_versions, list) - and fetched_versions + price_candidates and metadata_provider is not None - and self._should_fetch_prices(fetched_versions, existing) - ): - fetched_versions = await self._apply_model_prices( - metadata_provider, model_id, fetched_versions + and self._should_fetch_prices( + price_candidates, existing, force=force_refresh ) + ): + price_candidates = await self._apply_model_prices( + metadata_provider, model_id, price_candidates + ) + if refresh_succeeded and isinstance(fetched_versions, list): + fetched_versions = list(price_candidates) + else: + priced_versions = list(price_candidates) if fallback_attempted: if refresh_succeeded and isinstance(fetched_versions, list): @@ -1547,6 +1564,11 @@ class ModelUpdateService: local_base_models=local_base_models, ) else: + if priced_versions is not None and existing is not None: + # Metadata came from the cache, but the prices did not: keep + # them without touching last_checked_at, so the metadata TTL is + # not silently extended by a price-only pass. + existing = replace(existing, versions=priced_versions) record = self._merge_with_local_versions( existing, normalized_local, @@ -2187,6 +2209,8 @@ class ModelUpdateService: self, remote_versions: Sequence[ModelVersionRecord], existing: Optional[ModelUpdateRecord], + *, + force: bool = False, ) -> bool: """Whether this model needs a price fetch this round. @@ -2194,10 +2218,18 @@ class ModelUpdateService: nothing. A stored price is refreshed once its TTL lapses, and immediately when the gate itself changed (a new end date or sale window is a reason to believe the price moved). + + A *failed* attempt counts as an attempt: without that, a mature model whose + page no host will serve would be retried on every single update check. + ``force`` (the user asked explicitly) overrides the TTL. """ if not self._price_tracking_enabled(): return False + if force: + return any( + self._has_structural_gate(version) for version in remote_versions + ) existing_map = ( {version.version_id: version for version in existing.versions} @@ -2213,9 +2245,10 @@ class ModelUpdateService: stored = existing_map.get(remote_version.version_id) if stored is None: return True - if stored.price_checked_at is None: + anchor = stored.price_checked_at or stored.price_check_attempted_at + if anchor is None: return True - if (now - stored.price_checked_at) >= ttl: + if (now - anchor) >= ttl: return True if (stored.paid_access or None) != (remote_version.paid_access or None): return True diff --git a/tests/services/test_model_update_service.py b/tests/services/test_model_update_service.py index 33d15d57..6842be57 100644 --- a/tests/services/test_model_update_service.py +++ b/tests/services/test_model_update_service.py @@ -1373,6 +1373,9 @@ class FakeSettings: def get(self, key, default=None): return self._values.get(key, default) + def set(self, key, value): + self._values[key] = value + class PriceProvider(DummyProvider): """DummyProvider that can also serve prices.""" @@ -1955,3 +1958,82 @@ async def test_unavailable_marker_clears_when_the_version_becomes_free(tmp_path) assert record.versions[0].price_check_attempted_at is None assert service.count_unavailable_prices("lora") == 0 + + +def _long_ttl_service(tmp_path, **settings): + """A service whose metadata TTL does not lapse during the test.""" + + return ModelUpdateService( + str(tmp_path / "updates.sqlite"), + ttl_seconds=86400, + settings_manager=FakeSettings(settings), + ) + + +@pytest.mark.asyncio +async def test_prices_are_fetched_even_when_the_version_list_is_fresh(tmp_path): + """Enabling price tracking must not wait for the metadata TTL. + + The cached record already carries the gate, so a fresh version list is no + reason to skip the price: otherwise turning the feature on prices only the + handful of models that happened to need a metadata refresh that round. + """ + + service = _long_ttl_service(tmp_path, price_tracking_enabled=False) + scanner = DummyScanner(LOCAL_RAW_DATA) + provider = PriceProvider(GATED_RESPONSE, prices=PRICE_PAYLOAD) + + await service.refresh_for_model_type("lora", scanner, provider) + assert provider.price_calls == 0 + metadata_calls = provider.calls + checked_at_before = (await service.get_record("lora", 1)).last_checked_at + + service._settings.set("price_tracking_enabled", True) + await service.refresh_for_model_type("lora", scanner, provider) + + record = await service.get_record("lora", 1) + version = next(v for v in record.versions if v.version_id == 12) + + # The version list came from the cache this round ... + assert provider.calls == metadata_calls + # ... and the price was captured anyway. + assert provider.price_calls == 1 + assert version.price_buzz == 250 + assert version.price_checked_at is not None + assert version.price_check_attempted_at is not None + # A price-only pass must not extend the metadata TTL. + assert record.last_checked_at == checked_at_before + + +@pytest.mark.asyncio +async def test_failed_price_attempt_is_not_retried_within_the_ttl(tmp_path): + """A mature model whose page no host will serve must not cost two requests + on every single update check.""" + + service = _long_ttl_service(tmp_path, price_tracking_enabled=True) + scanner = DummyScanner(LOCAL_RAW_DATA) + provider = PriceProvider(GATED_RESPONSE, prices=None) + + await service.refresh_for_model_type("lora", scanner, provider) + assert provider.price_calls == 1 + + await service.refresh_for_model_type("lora", scanner, provider) + + assert provider.price_calls == 1 + assert service.count_unavailable_prices("lora") == 1 + + +@pytest.mark.asyncio +async def test_forced_refresh_reprices_within_the_ttl(tmp_path): + service = _long_ttl_service(tmp_path, price_tracking_enabled=True) + scanner = DummyScanner(LOCAL_RAW_DATA) + provider = PriceProvider(GATED_RESPONSE, prices=PRICE_PAYLOAD) + + await service.refresh_for_model_type("lora", scanner, provider) + assert provider.price_calls == 1 + + await service.refresh_for_model_type( + "lora", scanner, provider, force_refresh=True + ) + + assert provider.price_calls == 2