mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-05 01:25:33 -03:00
fix(update): decouple price capture from the metadata TTL
Found in a real instance: after enabling price tracking, a normal "Check updates" captured exactly one price out of 718 models, so the alerts panel looked broken while the log said the refresh completed. Price capture only ran when the version list was re-fetched, so it inherited the metadata TTL: with 24 h metadata and 24 h price TTLs, only the handful of models whose metadata happened to be stale that round 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 - a failed attempt now 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 Verified by copying a real instance's update DB into a sandbox and running a non-forced check: bulk metadata fetches 0 (version lists entirely from cache) while priced versions went 1 -> 20 and the panel listed 19 alerts.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user