Skip to content

[#763] Fix plot count +1: deduplicate and add unique constraint#766

Merged
realproject7 merged 3 commits intomainfrom
task/763-plot-count-fix
Apr 2, 2026
Merged

[#763] Fix plot count +1: deduplicate and add unique constraint#766
realproject7 merged 3 commits intomainfrom
task/763-plot-count-fix

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Migration 00030: Deduplicates any existing (storyline_id, plot_index) duplicates (keeps earliest), adds UNIQUE(storyline_id, plot_index) constraint, re-reconciles all plot counts
  • Indexer/backfill upserts: Changed onConflict from tx_hash,log_index to storyline_id,plot_index with ignoreDuplicates: true — prevents genesis plot being double-inserted by both storyline and plot indexers
  • Root cause: Genesis plot could be indexed twice via different tx_hash/log_index combos, inflating COUNT(*)

Fixes #763

Test plan

  • Migration runs cleanly (dedupe + constraint + reconcile)
  • Plot counts are correct after migration
  • New storyline creation doesn't fail on genesis plot insert
  • Re-indexing a plot doesn't create duplicate rows
  • npm run build passes

🤖 Generated with Claude Code

- Migration 00030: deletes duplicate (storyline_id, plot_index) rows
  keeping the earliest entry, adds unique constraint to prevent
  recurrence, and re-reconciles all plot counts
- Updates all plot upsert calls (indexer, storyline indexer, backfill)
  to use onConflict: "storyline_id,plot_index" with ignoreDuplicates
  instead of tx_hash,log_index — prevents duplicate genesis plots
- Updates reconcile.ts comment to document the constraint dependency

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

vercel Bot commented Apr 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Apr 2, 2026 3:33pm

Request Review

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.

T2b APPROVE

Well-structured fix across migration + indexer routes:

  • Migration: Correct order — dedupe (keep MIN id), add unique constraint, then reconcile. IS DISTINCT FROM in the reconcile UPDATE is a nice touch to skip no-op rows.
  • Upsert changes: Switching conflict key from tx_hash,log_index to storyline_id,plot_index with ignoreDuplicates: true directly addresses the root cause (same plot indexed via different tx combos). Safe because plot data is immutable per index on-chain.
  • Comment in reconcile.ts documents the constraint dependency.

No concerns.

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 unique-constraint direction makes sense for fixing duplicate plot counts, but the current conflict handling introduces a metadata regression for genesis plots.

Findings

  • [high] Genesis plot titles can now be lost permanently. The storyline/backfill genesis insert paths still write the plots row without a title, but this PR changes those inserts to onConflict: "storyline_id,plot_index", ignoreDuplicates: true. That means if the genesis row is created first by the storyline path, the later plot-indexer write that does include title is discarded, leaving the row at the default empty string forever.
    • File: src/app/api/index/storyline/route.ts:162
    • File: src/app/api/cron/backfill/route.ts:292
    • Suggestion: Either include title in the genesis plot rows written by the storyline/backfill paths, or keep the new conflict key but allow merge updates instead of ignoreDuplicates: true so the later richer row can populate missing fields.

Decision

Requesting changes because the current PR fixes the count issue by turning duplicate genesis writes into no-ops, but that also drops legitimate metadata from the surviving row.

Genesis insert paths (storyline indexer, backfill genesis) keep
ignoreDuplicates: true since they insert bare rows without title.
Plot indexer and backfill plot paths use regular upsert so they can
update rows with full data (title, etc.) if a genesis row exists.

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.

Verdict: REQUEST CHANGES

Summary

The new conflict-handling split fixes the future genesis-write race, but the migration still drops metadata from already duplicated genesis plots.

Findings

  • [high] The migration deletes duplicate (storyline_id, plot_index) rows by keeping MIN(id), which will preserve the earlier bare genesis row and delete the later duplicate row that carries the genesis title. The follow-up upsert fix only helps future replays; it does not repair the duplicated rows that already exist and are being deduplicated here.
    • File: supabase/migrations/00030_dedupe_plots_unique_constraint.sql:7
    • File: src/app/api/index/storyline/route.ts:162
    • File: src/app/api/index/plot/route.ts:117
    • Suggestion: Before deleting duplicates, merge non-empty metadata from the duplicate set into the survivor, or choose the survivor from the richer row set instead of blindly keeping the lowest id.

Decision

Requesting changes because the PR still turns an existing data-integrity bug into a persistent metadata-loss bug during migration for already duplicated genesis plots.

Migration now uses DISTINCT ON with ordering that prefers rows with
non-empty title over bare genesis rows, then latest id as tiebreaker.
This preserves title data from the plot indexer instead of keeping
the earlier bare genesis row.

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.

Verdict: APPROVE

Summary

The follow-up migration change resolves the remaining data-loss issue by deduplicating (storyline_id, plot_index) rows in a way that prefers richer genesis rows before the unique constraint is added.

Findings

  • None.

Decision

Approving because the PR now both prevents future duplicate plot rows and preserves the better existing row during migration, which addresses the plot-count bug without the metadata regression from the earlier revision.

@realproject7 realproject7 merged commit 5a2acfa into main Apr 2, 2026
5 checks passed
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.

[Bug] Plot count still shows +1 — investigate and re-reconcile

2 participants