fix: minor issue and order

application/alembic/versions/0028_user_logs_agent_lookup_idx.py

@@ -1,4 +1,4 @@
-"""0027 index the per-agent log lookups so /api/get_user_logs stops timing out.
+"""0028 index the per-agent log lookups so /api/get_user_logs stops timing out.
 
 The per-agent Logs timeline filters ``user_logs`` on two JSONB keys
 (``data->>'api_key'`` / ``data->>'agent_id'``) and ``stack_logs`` on
@@ -27,11 +27,13 @@ bypassed entirely. Indexing the second arm is what actually activates it.
 Indexes are built ``CONCURRENTLY`` in autocommit blocks (following 0018): a
 plain CREATE INDEX would take an ACCESS EXCLUSIVE lock on a 1.3 GB table on a
 live database. The ``user_logs`` builds each detoast the full ~1.16 GB once, so
-they are not instant; CONCURRENTLY keeps them non-blocking. Idempotent both
-ways.
+they are not instant; CONCURRENTLY keeps them non-blocking. Because a cancelled
+concurrent build leaves an INVALID index behind that ``IF NOT EXISTS`` would
+then skip forever, each build first clears such a leftover — see
+``_rebuild_if_invalid``. Idempotent both ways.
 
-Revision ID: 0027_user_logs_agent_lookup_idx
-Revises: 0026_stack_logs_agent_id
+Revision ID: 0028_user_logs_agent_lookup_idx
+Revises: 0027_schedule_run_error_types
 """
 
 from typing import Sequence, Union
@@ -39,8 +41,8 @@ from typing import Sequence, Union
 from alembic import op
 
 
-revision: str = "0027_user_logs_agent_lookup_idx"
-down_revision: Union[str, None] = "0026_stack_logs_agent_id"
+revision: str = "0028_user_logs_agent_lookup_idx"
+down_revision: Union[str, None] = "0027_schedule_run_error_types"
 branch_labels: Union[str, Sequence[str], None] = None
 depends_on: Union[str, Sequence[str], None] = None
 
@@ -67,11 +69,39 @@ _INDEXES = (
 )
 
 
+def _rebuild_if_invalid(name: str) -> None:
+    """Drop ``name`` only when a previous concurrent build left it INVALID.
+
+    A failed/cancelled ``CREATE INDEX CONCURRENTLY`` does not roll back — it
+    leaves an index in the catalog with ``indisvalid = false`` that still owns
+    the name. ``IF NOT EXISTS`` then skips it forever, so the migration reports
+    success while the planner refuses to use the index: a silent no-op. Clearing
+    it first makes the rebuild actually happen.
+
+    The check is deliberately conditional rather than an unconditional drop
+    (0018's approach): the ``user_logs`` builds detoast ~1.16 GB each, so
+    re-running this migration against a healthy database should not pay that
+    cost again. ``to_regclass`` returns NULL for a name that doesn't exist, so
+    the first run selects no row and skips the drop.
+    """
+    invalid = (
+        op.get_bind()
+        .exec_driver_sql(
+            "SELECT NOT indisvalid FROM pg_index "
+            f"WHERE indexrelid = to_regclass('{name}')"
+        )
+        .scalar()
+    )
+    if invalid:
+        op.execute(f"DROP INDEX CONCURRENTLY IF EXISTS {name};")
+
+
 def upgrade() -> None:
     # CONCURRENTLY can't run inside a transaction; the autocommit block lets
     # each build run without locking out writers on a multi-GB live table.
     for name, target, predicate in _INDEXES:
         with op.get_context().autocommit_block():
+            _rebuild_if_invalid(name)
             op.execute(
                 f"CREATE INDEX CONCURRENTLY IF NOT EXISTS {name} "
                 f"ON {target} WHERE {predicate};"