Skip to content

fix(migrations): resolve the #548/#549 collision that made the digest timezone fix inert - #563

Merged
guillermoscript merged 1 commit into
masterfrom
fix/digest-migration-collision-549
Jul 26, 2026
Merged

fix(migrations): resolve the #548/#549 collision that made the digest timezone fix inert#563
guillermoscript merged 1 commit into
masterfrom
fix/digest-migration-collision-549

Conversation

@guillermoscript

Copy link
Copy Markdown
Owner

What

Fixes two problems that only exist in the combination of #548 (keyset pagination for get_daily_digest_candidates) and #549 (the tenant-local streak-nudge comparison). Both PRs merged cleanly on their own within hours of each other; neither was reviewed against the other's result.

Follow-up to #559. Relates to #548, #549.

Why

1. Duplicate migration version. 20260726140000_daily_digest_candidates_pagination.sql and 20260726140000_league_rollover_catchup_and_bands.sql share a version, so a fresh db reset would try to record two ledger rows under one primary key. The league migration is independent of everything around it, so it moves to 20260726140500 rather than reordering anything.

2. The #549 digest fix was inert — the bug was live while the fix looked applied. #548 replaced the zero-argument function with a three-argument keyset-paginated one, and deliberately DROPped the zero-arg version, with a comment explaining that an overload makes a zero-arg call ambiguous (PostgREST answers 300).

#549's migration then re-created exactly that zero-arg function. It sorts after #548's, so a fresh reset ended up with both. The cron passes three named arguments, so it kept resolving to the paginated definition — which still carried the = CURRENT_DATE - 1 UTC comparison that #549 exists to remove. Meanwhile #549's own function sat unused.

Net effect on a fresh database: the LATAM streak-nudge bug is still live, the unit tests still pass (they cover isStreakAtRisk, which is correct), and the migration that was supposed to fix it is present and applied. Nothing surfaces the problem.

The #549 migration now replaces the paginated signature, carrying #548's cursor predicate and ORDER BY over verbatim, so a single function has both fixes.

How to QA

The two migrations are applied in file order inside a rolled-back transaction, then asserted on — BEGIN; + both migration files + the assertion below + ROLLBACK;:

-- exactly one overload, it has the #549 fix, and it kept #548's pagination
SELECT count(*) FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace
 WHERE n.nspname = 'public' AND p.proname = 'get_daily_digest_candidates';  -- must be 1

Result:

NOTICE:  PASS: exactly 1 signature get_daily_digest_candidates(uuid,uuid,integer)
         - has #549 fix AND keeps #548 pagination

Also checked:

  • No duplicate migration versions remain: ls supabase/migrations/*.sql | xargs -n1 basename | cut -d_ -f1 | sort | uniq -d returns empty.
  • npm run test:unit passes 502/502.
  • npm run typecheck clean.

A full db reset was deliberately not run: four other worktrees are active against the shared local stack and a reset would wipe it out from under them.

Screenshots / GIF

N/A - two migration files.

Checklist

Note on cloud

Cloud never had this problem. When applying #549 I checked the live definition first, found the paginated signature, and patched that rather than applying the repo's zero-arg file - so production has carried both fixes on a single function since then. This PR brings the repo in line with what cloud already runs.

… timezone fix inert

#548 (keyset pagination for get_daily_digest_candidates) and #549 (the
tenant-local streak-nudge comparison) were developed in parallel and landed
within hours of each other. Each merged cleanly on its own; together they left
master in a state neither PR was reviewed in.

Two problems.

1. Duplicate migration version. Both `daily_digest_candidates_pagination` and
   `league_rollover_catchup_and_bands` were stamped 20260726140000, so a fresh
   `db reset` would try to record two rows under one primary key. The league
   migration is independent of everything around it, so it moves to
   20260726140500 rather than reordering anything.

2. The #549 digest fix was inert. #548 replaced the zero-argument function with
   a three-argument keyset-paginated one, and explicitly DROPped the zero-arg
   version because an overload makes a zero-arg call ambiguous (PostgREST 300).
   #549's migration then re-created that zero-arg function. Because it sorts
   after #548's, a fresh reset ended up with both: the cron passes three named
   arguments so it kept resolving to the paginated definition, which still
   carried the `= CURRENT_DATE - 1` UTC comparison #549 exists to remove — while
   #549's own function sat unused. The bug was live and the fix looked applied.

The #549 migration now replaces the paginated signature, carrying #548's cursor
and ORDER BY over verbatim, so one function has both fixes.

Verified by applying both migrations in file order inside a rolled-back
transaction: exactly one `get_daily_digest_candidates(uuid,uuid,integer)`
survives, it contains `>= CURRENT_DATE - 2`, and it retains `_after_tenant_id`.
502/502 unit tests and typecheck green.

Cloud was already patched directly against the live paginated definition, so it
has been correct throughout; this brings the repo in line with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RGCEptT7pgWwRiMdB3TUCM
@guillermoscript
guillermoscript merged commit 0165cc7 into master Jul 26, 2026
2 checks passed
@guillermoscript
guillermoscript deleted the fix/digest-migration-collision-549 branch July 26, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant