Skip to content

fix(db): notification_deliveries table name collides with an unrelated schema used by the currently-unwired src/review/alerts.ts anomaly-alert port #8901

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/db/schema.ts:1145-1169 (canonical notification_deliveries table: id, dedup_key, channel, recipient_login, event_type, repo_full_name, pull_number, title, body, deeplink, actor_login, status, created_at, delivered_at, read_at, from migrations/0031_notification_subscriptions.sql) collides in name with src/review/alerts.ts:223-255, which does raw INSERT INTO notification_deliveries (id, project, target_id, notification_key, status) ... ON CONFLICT(project, target_id, notification_key) DO NOTHING -- a completely different shape, assuming columns (project, target_id, notification_key) and a unique constraint that don't exist on the real table. runAnomalyAlerts currently has zero callers anywhere in the codebase (confirmed via grep), so this hasn't fired yet, but it is a complete, working function awaiting a cron call site, and the moment it's wired, every invocation with Discord-notify enabled throws immediately at the first INSERT.

Requirements

Rename the table src/review/alerts.ts writes to (e.g. alert_dedup_claims) and add its own migration with the (project, target_id, notification_key) unique index it actually needs, so it no longer collides with the existing notification_deliveries schema. This fix is independent of whether runAnomalyAlerts ever gets wired to a cron path -- it's a latent schema-collision landmine regardless.

Deliverables

  • A new migration creates a distinctly-named table (e.g. alert_dedup_claims) with the (project, target_id, notification_key) unique index alerts.ts needs
  • src/review/alerts.ts writes to the new table instead of notification_deliveries
  • A test confirming runAnomalyAlerts's dedup-claim insert now succeeds against the new table's real schema

All of the above Deliverables are required in the same PR unless the deliverable text itself states otherwise.

Test Coverage Requirements

src/** -- 99%+ patch coverage, branch-counted, on the new migration and the updated write path.

Expected Outcome

src/review/alerts.ts writes to a correctly-shaped table with its own real unique constraint, so if/when it's wired to a cron path in the future, it no longer throws immediately on its first insert due to a name collision with an unrelated schema.

Links & Resources

  • src/db/schema.ts:1145-1169
  • migrations/0031_notification_subscriptions.sql
  • src/review/alerts.ts:223-255

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions