feat(warm_catalog_stats): retire sweep gate as default, compute hourly regardless

MPCAutofill/MPCAutofill/settings.py

@@ -647,7 +647,7 @@
 # constant), same convention as STAGE_E_STREAMING_ENABLED above, so a self-hoster who doesn't run
 # sweeps at all - and therefore never wants the warm skipped over a ledger row it doesn't
 # understand - can turn the gate off with one env var and no code change or redeploy.
-WARM_CATALOG_STATS_SWEEP_GATE_ENABLED = env.bool("WARM_CATALOG_STATS_SWEEP_GATE_ENABLED", default=True)
+WARM_CATALOG_STATS_SWEEP_GATE_ENABLED = env.bool("WARM_CATALOG_STATS_SWEEP_GATE_ENABLED", default=False)
 
 # WARM_CATALOG_STATS_SWEEP_STALE_AFTER_HOURS - how old a RUNNING PilotRunLedger row has to be
 # before the gate stops trusting it and lets the warm run anyway (the guard against a crashed

MPCAutofill/cardpicker/management/commands/warm_catalog_stats.py

@@ -35,36 +35,29 @@
 docstring). A cron/schedule run that silently writes nowhere and reports success is exactly the
 bug issue #538 exists to prevent, and this command must not repeat it a second way.
 
-**SKIPS (exit 0, never an error) while a catalog sweep is in flight (owner ruling 2026-07-29).**
-A sweep (`PilotRunLedger` row with `status=RUNNING` - see that model's own docstring: "the
-durable, queryable record", created RUNNING at start, updated COMPLETED/FAILED at end) genuinely
-contends for the same database this command's five aggregates query, so this command checks for
-one BEFORE computing anything and, if found, skips the entire run rather than compute against a
-catalog mid-mutation. This is the exact same "not configured" vs "broken" split
-`warm_artist_external_links` already draws between an opt-in gate (quiet, exit-0 skip) and a
-genuine fetch failure (`CommandError`, non-zero) - a skip here is equally NOT an error: it is this
-command correctly declining to run, not a failure to run. The cache is left completely untouched
-on skip, same rule this command already follows on a genuine failure - never a partial or zeroed
-blob, stale-but-correct beats fresh-but-wrong here just as much as it does on the failure path.
-
-Guarded against a crashed sweep that never reaches COMPLETED/FAILED and would otherwise leave a
-`RUNNING` row - and therefore this gate - stuck forever: a `RUNNING` row older than
-`settings.WARM_CATALOG_STATS_SWEEP_STALE_AFTER_HOURS` (default comfortably above a full sweep's
-measured ~7.0h floor - see that setting's own comment in `MPCAutofill/MPCAutofill/settings.py` for the exact
-number and reasoning) is ignored and does not block this run. Both the gate itself
-(`settings.WARM_CATALOG_STATS_SWEEP_GATE_ENABLED`) and the staleness bound are settings-driven,
-tunable without a migration or a code change - see those two settings' own comments.
-
-The skip message names the specific blocking run (`run_id`, `started_at`, and how long it has
-been running) precisely so a frozen stats page is diagnosable from the command's own log output in
-one command, without a separate query against `PilotRunLedger`.
-
-**Consequence, stated plainly (owner ruling 2026-07-29): gating the WHOLE warm run this way means
-the stats page can be up to ~7h stale during a full sweep**, not the roughly-hourly cadence the
-schedule alone implies - see `docs/features/catalog-stats.md`'s "Sweep gate" section for the full
-write-up. This is an accepted trade, not an oversight: a fast/slow split (skip only the panels
-that actually read tables a sweep mutates, keep warming the rest) would avoid this staleness, and
-is a known, deliberately deferred follow-up - not designed or built here.
+**Sweep gate (owner ruling 2026-07-29, RETIRED as default 2026-08-03).**
+The original ruling gated the warm run while a catalog sweep held a `PilotRunLedger` row with
+`status=RUNNING` within the staleness bound, on the premise that a heavy batch sweep contended
+for the same database. That premise no longer holds under the streaming micro-batch sweep design,
+where a `RUNNING` row is perpetually present — the gate was freezing the stats page indefinitely.
+
+**Owner's ruling, 2026-08-03 (reverses the 2026-07-29 gate ruling): the gate is RETIRED as the
+default.** The command now computes all five panels on every hourly run, sweep or no sweep,
+because: (a) the aggregations are MVCC-safe — plain SELECTs never block on or are blocked by the
+streaming sweep's tiny micro-batch INSERTs; (b) the stats pipeline already isolates sweep
+artifacts — `runHistory` filters rows with `anonymous_id=SLOW_PATH_ANONYMOUS_ID,
+skip_reason=SLOW_PATH_TO_REVIEW_SKIP_REASON` (see `catalog_stats.py` lines ~391-396), and the
+vote panels count only human sources, so mid-sweep numbers are stable and correct; (c) measured
+full compute of all five panels takes ~9s (last ungated run 2026-08-02T16:00:22Z→16:00:31Z),
+trivial load once an hour.
+
+**The gate is now OPT-IN**, controlled by the settings flag
+`WARM_CATALOG_STATS_SWEEP_GATE_ENABLED` (default `False`). When opted in (set to `True` via
+env var), the exact 2026-07-29 skip behaviour is preserved: a RUNNING row within the staleness
+bound skips the entire run (exit 0, cache untouched, same warning text as before), and a RUNNING
+row older than `WARM_CATALOG_STATS_SWEEP_STALE_AFTER_HOURS` (default 12h) is ignored as a
+crashed-sweep guard. Both the gate flag and the staleness bound remain settings-driven, tunable
+without a migration or a code change.
 """
 
 from datetime import timedelta
@@ -96,10 +89,11 @@ def _find_blocking_sweep() -> Optional[PilotRunLedger]:
 class Command(BaseCommand):
     help = (
         "Recomputes all five Proposal F catalog-stats panels and writes the catalog-stats cache "
-        "blob. Intended for an hourly django-q2 schedule. Skips cleanly (exit 0) while a catalog "
-        "sweep is in flight (PilotRunLedger status=RUNNING within the staleness bound), leaving "
-        "the cache untouched - see this command's own module docstring for the accepted "
-        "up-to-~7h staleness trade this creates. On any other failure, also leaves the existing "
+        "blob. Intended for an hourly django-q2 schedule. By default (sweep gate disabled) "
+        "computes all five panels on every run, sweep or no sweep - see this command's own module "
+        "docstring for the 2026-08-03 retirement rationale. When the sweep gate is opt-in enabled "
+        "(WARM_CATALOG_STATS_SWEEP_GATE_ENABLED=True), skips cleanly (exit 0) while a catalog "
+        "sweep is in flight, leaving the cache untouched. On any failure, also leaves the existing "
         "cache untouched and exits non-zero."
     )
 
@@ -121,6 +115,8 @@ def handle(self, *args: Any, **kwargs: Any) -> None:
                     )
                 )
                 return
+        else:
+            self.stdout.write("Sweep gate: disabled (default) — computing all five panels.")
 
         try:
             blob = warm_catalog_stats_cache()

MPCAutofill/cardpicker/tests/test_catalog_stats.py

@@ -725,9 +725,9 @@ def test_migration_reverses_and_reapplies_cleanly(self, db):
 
 class TestWarmCatalogStatsSweepGate:
     """
-    Tests for `warm_catalog_stats`'s sweep gate (owner ruling 2026-07-29 - see that command's own
-    module docstring, and `docs/features/catalog-stats.md`'s "Sweep gate" section, for the full
-    skip/staleness mechanism and the accepted up-to-~7h staleness trade it creates).
+    Tests for `warm_catalog_stats`'s sweep gate - the 2026-07-29 skip/staleness mechanism,
+    now opt-in (default off, 2026-08-03 retirement). See that command's own module docstring
+    and `docs/features/catalog-stats.md`'s "Sweep gate" section.
 
     Every scenario here calls the real management command end-to-end (`call_command`, not the
     gate helper directly) so a regression in either the gate check itself OR its wiring into
@@ -737,7 +737,8 @@ class TestWarmCatalogStatsSweepGate:
     def test_running_sweep_within_staleness_bound_skips_and_leaves_cache_untouched(self, db, capsys):
         """The core skip guarantee: exit 0 (no exception), a clear message naming the blocking
         run, and the cache left byte-for-byte as it was - not merely "the command returned", the
-        cache must still hold exactly the PRIOR blob, proving no recompute happened at all."""
+        cache must still hold exactly the PRIOR blob, proving no recompute happened at all.
+        Requires the gate to be explicitly enabled since it is off by default (2026-08-03)."""
         call_command("warm_catalog_stats")
         good_cache = caches[SHARED_CACHE_ALIAS].get(CACHE_KEY)
         assert good_cache is not None
@@ -748,7 +749,8 @@ def test_running_sweep_within_staleness_bound_skips_and_leaves_cache_untouched(s
             status=PilotRunLedger.Status.RUNNING,
         )
 
-        call_command("warm_catalog_stats")  # must not raise - a skip is exit 0, never an error
+        with override_settings(WARM_CATALOG_STATS_SWEEP_GATE_ENABLED=True):
+            call_command("warm_catalog_stats")  # must not raise - a skip is exit 0, never an error
         output = capsys.readouterr().out
 
         assert "sweep-in-flight-abc123" in output

MPCAutofill/cardpicker/tests/test_warm_catalog_stats.py

@@ -0,0 +1,169 @@
+"""
+Tests for `warm_catalog_stats` management command sweep gate behaviour after the
+2026-08-03 retirement (gate OFF by default, opt-in via settings flag). Covers the four
+scenarios the retirement PR requires plus one default-behaviour guard.
+
+Mirrors the existing `TestWarmCatalogStatsSweepGate` class in `test_catalog_stats.py`
+and the migration-test style of `test_warm_artist_external_links_schedule.py`.
+"""
+
+import datetime as dt
+from unittest.mock import patch
+
+import pytest
+
+from django.core.cache import caches
+from django.core.management import CommandError, call_command
+from django.test import override_settings
+from django.utils import timezone
+
+from cardpicker.catalog_stats import CACHE_KEY, SHARED_CACHE_ALIAS
+from cardpicker.models import PilotRunLedger
+
+_TEST_CACHES = {
+    "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"},
+    "shared": {
+        "BACKEND": "django.core.cache.backends.locmem.LocMemCache",
+        "LOCATION": "test-warm-catalog-stats-shared",
+    },
+}
+
+
+@pytest.fixture(autouse=True)
+def _shared_cache_configured():
+    with override_settings(CACHES=_TEST_CACHES):
+        caches["default"].clear()
+        caches[SHARED_CACHE_ALIAS].clear()
+        yield
+        caches["default"].clear()
+        caches[SHARED_CACHE_ALIAS].clear()
+
+
+def _create_run(
+    run_id: str,
+    command: str = "local_identify_printing_tags",
+    status: str = PilotRunLedger.Status.COMPLETED,
+    started_at: dt.datetime = None,
+    finished_at: dt.datetime = None,
+    votes_written=None,
+) -> PilotRunLedger:
+    run = PilotRunLedger.objects.create(run_id=run_id, command=command, status=status, votes_written=votes_written)
+    if finished_at is not None:
+        run.finished_at = finished_at
+        run.save(update_fields=["finished_at"])
+    if started_at is not None:
+        PilotRunLedger.objects.filter(pk=run.pk).update(started_at=started_at)
+    run.refresh_from_db()
+    return run
+
+
+@pytest.mark.django_db
+class TestWarmCatalogStatsDefaultBehaviour:
+    """Default behaviour: gate OFF (2026-08-03 retirement)."""
+
+    def test_running_sweep_does_not_block_when_gate_is_off_by_default(self, capsys):
+        """A RUNNING PilotRunLedger within the stale bound does NOT block — all
+        five panels computed, generatedAt bumped, cache written. This is the core
+        fix for the bug that froze the stats page indefinitely under the streaming
+        sweep's perpetually-RUNNING ledger row."""
+        _create_run(
+            run_id="perpetual-streaming-sweep",
+            command="stage_e_streaming_dispatch",
+            status=PilotRunLedger.Status.RUNNING,
+        )
+
+        call_command("warm_catalog_stats")
+        output = capsys.readouterr().out
+
+        assert "Sweep gate: disabled (default)" in output
+        assert "Catalog-stats cache warmed" in output
+
+        cached = caches[SHARED_CACHE_ALIAS].get(CACHE_KEY)
+        assert cached is not None
+        assert cached["generatedAt"] is not None
+
+    def test_no_sweep_running_computes_normally_by_default(self, capsys):
+        """Absent any RUNNING row at all, default behaviour is the standard full
+        compute — this is the same path as when a sweep is running, but worth
+        guarding explicitly so a future gate change cannot silently break the
+        no-sweep path."""
+        call_command("warm_catalog_stats")
+        output = capsys.readouterr().out
+
+        assert "Sweep gate: disabled (default)" in output
+        assert "Catalog-stats cache warmed" in output
+
+        cached = caches[SHARED_CACHE_ALIAS].get(CACHE_KEY)
+        assert cached is not None
+        assert cached["generatedAt"] is not None
+
+
+@pytest.mark.django_db
+class TestWarmCatalogStatsGateOptIn:
+    """Opt-in gate behaviour (gate ON — the explicit override restores the
+    2026-07-29 skip semantics for emergency conservatism)."""
+
+    def test_running_sweep_within_bound_skips_when_gate_enabled(self, capsys):
+        """Opt-in: a RUNNING row within the staleness bound blocks the run,
+        cache untouched, exit 0 — exact 2026-07-29 behaviour preserved."""
+        call_command("warm_catalog_stats")
+        good_cache = caches[SHARED_CACHE_ALIAS].get(CACHE_KEY)
+        assert good_cache is not None
+
+        _create_run(
+            run_id="opt-in-blocking-sweep",
+            command="local_identify_printing_tags",
+            status=PilotRunLedger.Status.RUNNING,
+        )
+
+        with override_settings(WARM_CATALOG_STATS_SWEEP_GATE_ENABLED=True):
+            call_command("warm_catalog_stats")
+        output = capsys.readouterr().out
+
+        assert "opt-in-blocking-sweep" in output
+        assert "skip" in output.lower()
+        assert caches[SHARED_CACHE_ALIAS].get(CACHE_KEY) == good_cache
+
+    def test_stale_sweep_does_not_block_when_gate_enabled(self):
+        """Opt-in staleness guard preserved: a RUNNING row older than the
+        staleness bound is ignored and the warm computes normally. This is the
+        crashed-sweep guard that prevents a RUNNING row left behind by a crash
+        from freezing the page permanently."""
+        from django.conf import settings
+
+        stale_started_at = timezone.now() - dt.timedelta(hours=settings.WARM_CATALOG_STATS_SWEEP_STALE_AFTER_HOURS + 1)
+        _create_run(
+            run_id="crashed-sweep-nobody-finished",
+            command="local_identify_printing_tags",
+            status=PilotRunLedger.Status.RUNNING,
+            started_at=stale_started_at,
+        )
+
+        with override_settings(WARM_CATALOG_STATS_SWEEP_GATE_ENABLED=True):
+            call_command("warm_catalog_stats")
+
+        cached = caches[SHARED_CACHE_ALIAS].get(CACHE_KEY)
+        assert cached is not None
+        assert cached["generatedAt"] is not None
+
+
+@pytest.mark.django_db
+class TestWarmCatalogStatsFailureSemantics:
+    """Failure semantics independent of gate state."""
+
+    def test_computation_failure_leaves_cache_untouched_and_raises_command_error(self):
+        """If compute_catalog_stats raises, the previous cache blob survives
+        byte-for-byte and the command exits with CommandError — same contract
+        regardless of gate state."""
+        call_command("warm_catalog_stats")
+        good_cache = caches[SHARED_CACHE_ALIAS].get(CACHE_KEY)
+        assert good_cache is not None
+
+        with patch(
+            "cardpicker.catalog_stats.compute_catalog_stats",
+            side_effect=RuntimeError("boom"),
+        ):
+            with pytest.raises(CommandError, match="left untouched"):
+                call_command("warm_catalog_stats")
+
+        assert caches[SHARED_CACHE_ALIAS].get(CACHE_KEY) == good_cache