Skip to content

Add migration to reconcile stale plot_count data#260

Merged
realproject7 merged 2 commits intomainfrom
task/259-reconcile-plot-counts-migration
Mar 17, 2026
Merged

Add migration to reconcile stale plot_count data#260
realproject7 merged 2 commits intomainfrom
task/259-reconcile-plot-counts-migration

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Add supabase/migrations/00013_reconcile_plot_counts.sql to fix existing stale storylines.plot_count and last_plot_time values
  • Idempotent UPDATE using COUNT(*) and MAX(block_timestamp) from plots — only updates rows where values differ
  • Companion to PR Fix plot_count: reconcile storyline after plot upsert #258 which fixed the code path going forward

Fixes #259

Test plan

  • Migration updates stale storyline rows with correct plot_count and last_plot_time
  • Running migration twice produces no additional changes (idempotent)
  • npm run typecheck passes

🤖 Generated with Claude Code

Idempotent UPDATE that sets storylines.plot_count and last_plot_time
from COUNT(*) and MAX(block_timestamp) on the plots table. Only
updates rows where values differ from computed aggregates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST CHANGES — one blocking issue.

Blocking: != doesn't match NULL values — stale rows with NULL last_plot_time will be skipped.

Line 16: s.plot_count != sub.cnt OR s.last_plot_time != sub.latest

In PostgreSQL, NULL != value evaluates to NULL (not TRUE), so any storyline where last_plot_time is currently NULL (never set) won't be updated by this migration — which is likely the most common stale case.

Fix: use IS DISTINCT FROM instead of !=:

WHERE s.storyline_id = sub.storyline_id
  AND (s.plot_count IS DISTINCT FROM sub.cnt
    OR s.last_plot_time IS DISTINCT FROM sub.latest);

IS DISTINCT FROM treats NULL as a comparable value and returns TRUE when one side is NULL and the other isn't. Same applies to plot_count if it could be NULL.

Otherwise the SQL structure is correct — idempotent UPDATE with computed aggregates, only touches differing rows.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The migration is close, but the WHERE predicate uses SQL != comparisons that do not handle NULL correctly.

Findings

  • [medium] Rows with NULL in storylines.last_plot_time can be skipped even when they need repair. In PostgreSQL, NULL != sub.latest evaluates to NULL, not TRUE, so the whole AND (...) condition may fail. Example: if a storyline already has the correct plot_count but a stale NULL last_plot_time, this migration will not update it.
    • File: supabase/migrations/00013_reconcile_plot_counts.sql:16
    • Suggestion: use IS DISTINCT FROM for both comparisons: s.plot_count IS DISTINCT FROM sub.cnt OR s.last_plot_time IS DISTINCT FROM sub.latest.

Decision

Request changes. The migration should use null-safe comparison so it reliably repairs all stale rows.

NULL != value evaluates to NULL in Postgres, silently skipping stale
rows with NULL last_plot_time. IS DISTINCT FROM handles NULLs correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

NULL-safety fix confirmed: IS DISTINCT FROM on both comparisons. Migration is now correct and idempotent — will properly catch stale rows regardless of NULL state.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The follow-up patch fixes the migration predicate by using null-safe comparisons. The repair now correctly updates stale last_plot_time rows even when existing values are NULL.

Findings

  • [info] No blocking issues found in the updated patch.

Decision

Approve. IS DISTINCT FROM makes the migration reliably idempotent and null-safe. CI was still pending when reviewed.

@realproject7 realproject7 merged commit 6fc1611 into main Mar 17, 2026
1 check passed
realproject7 added a commit that referenced this pull request Mar 23, 2026
Adds $PLOT as a nav link pointing to /token page, consistent with
existing nav link styling and responsive behavior.

Fixes #260

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@realproject7 realproject7 mentioned this pull request Mar 23, 2026
4 tasks
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.

[chore] Add migration to reconcile stale plot_count data

2 participants