[#322] Fix backfill: missing title, silent data loss, cron auth#325
[#322] Fix backfill: missing title, silent data loss, cron auth#325realproject7 merged 2 commits intomainfrom
Conversation
- Add title field to processPlotChained() DB row (matching direct indexer) - Log IPFS fetch/hash-mismatch failures to backfill_failures table instead of silently skipping events - Include failure count in response JSON - Fail closed on missing CRON_SECRET in production - Add Supabase migration for backfill_failures table - Add backfill_failures type to Database interface Fixes #322 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE
Verified all 3 acceptance items:
- Title field (E3):
title: title || ""added toprocessPlotChained()PlotInsert, destructured from decoded event args — matches deployed contract ABI - Failure logging (E6/E7): Both
processStorylineCreatedandprocessPlotChainednow log IPFS/hash failures tobackfill_failurestable vialogBackfillFailure(). Failure count included in response JSON. Helper function catches its own DB errors (no throw) — won't break the backfill loop - Cron auth (D2):
verifyCron()returnsprocess.env.NODE_ENV !== "production"when no secret — correct fail-closed in prod, permissive in dev
Migration is clean with storyline_id index. TypeScript types match schema. Single commit, well-scoped.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The main backfill fixes are in the right area, but the new failure-logging path still allows permanent silent loss when the backfill_failures insert itself fails. That leaves the audit issue unresolved in an important edge case.
Findings
- [high] Failure logging errors are swallowed, but the cursor still advances.
- File:
src/app/api/cron/backfill/route.ts:54 - Suggestion: Make
logBackfillFailure()propagate insertion errors, or otherwise stop cursor advancement when a failure row cannot be recorded. As written, if the migration is missing or Supabase insert fails transiently, the event is neither indexed nor recorded, andlast_blockstill advances atsrc/app/api/cron/backfill/route.ts:195, recreating the permanent data-loss path from audit #320.
- File:
Decision
Request changes. The PR should only advance the cursor after failed events are durably recorded, otherwise the new table does not actually guarantee recoverability.
If the backfill_failures insert itself fails, throw so the outer catch handles it and increments the error count — prevents silent data loss when the table is missing or DB is unreachable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up fixes the remaining failure-path bug: backfill now fails visibly if backfill_failures cannot be written, instead of silently advancing the cursor. The PR now satisfies the #322 acceptance criteria.
Findings
- [low] The prior blocker is resolved because
logBackfillFailure()now throws on insert errors, so failed events are either durably recorded or counted in the outererrorspath.- File:
src/app/api/cron/backfill/route.ts:54 - Suggestion: None.
- File:
Decision
Approve. Backfilled plots now persist title, failed IPFS/hash events are recorded via the new table, production cron auth fails closed without CRON_SECRET, and local npm run build plus npm run typecheck both pass.
Summary
title: title || ""toprocessPlotChained()DB row, matching the direct indexerbackfill_failurestable instead of silently skipping; failure count included in response JSONverifyCron()now rejects requests whenCRON_SECRETis unset in production (NODE_ENV === "production")supabase/migrations/00015_backfill_failures.sqlcreates the table with an index onstoryline_idbackfill_failurestoDatabaseinterface inlib/supabase.tsFixes #322
Test plan
npm run buildpassesnpm run typecheckpasses