feat(notifications): comment, mention, thread, reaction, fan_club_text_post triggers#851
Merged
Merged
Conversation
raymondjacobson
force-pushed
the
api/comment-notifications
branch
from
81b3585 to
55da6d1
Compare
…t_post triggers
Adds the five comment-related notification triggers that apps'
src/tasks/entity_manager/entities/comment.py creates directly from
Python during ManageEntity processing. The ETL handlers in
go-openaudio (pkg/etl/processors/entity_manager/comment_*.go) already
write the source rows — comments, comment_mentions, comment_threads,
comment_reactions — but the user-facing notifications had no Go
equivalent. handle_comment.sql here previously only updated
aggregate_track.comment_count.
Closes the largest remaining notification gap on the path to shutting
off the Python discovery indexer.
New trigger files (one notification type each, all on the same
table-per-trigger pattern as handle_comment_remix_contest_update.sql):
handle_comment_notification.sql → `comment`
Fires on comments INSERT, deferred. Notifies entity owner (track
owner / event host / fan-club artist) of a new top-level comment.
Skips: self-comment, replies (comment_threads exists), owner-
mentioned (comment_mentions for owner — they get comment_mention
instead), and comment_notification_settings / muted_users mutes.
handle_comment_mention.sql → `comment_mention`
Fires on comment_mentions INSERT. Notifies the mentioned user.
Skips: self-mention, mention has muted the commenter, owner is
mentioned AND owner muted notifications on the entity.
handle_comment_thread.sql → `comment_thread`
Fires on comment_threads INSERT. Notifies the parent comment
author. Skips: self-reply, parent author muted the thread or
the replier.
handle_comment_reaction.sql → `comment_reaction`
Fires on comment_reactions INSERT. Notifies the comment author.
Skips: self-react, comment author muted notifications on the
comment or the reacter, plus apps' track_owner_mention_mute
when the commenter is the entity owner.
handle_fan_club_text_post.sql → `fan_club_text_post`
Fires on comments INSERT (FanClub entity_type), deferred. Fans
out to (followers ∪ artist-coin holders) − {artist}. One row per
recipient with specifier=recipient_id (unique constraint on
(group_id, specifier) dedupes).
Why DEFERRABLE INITIALLY DEFERRED on the comments INSERT triggers:
"Top-level" is determined by the absence of comment_threads for this
comment_id, and "owner is mentioned" by the presence of a
comment_mentions row. Both sibling rows are inserted AFTER the comments
row in the same indexer transaction. Same pattern as
handle_comment_remix_contest_update.sql.
Intentionally deferred (matches apps but not ported here): the karma-
based mute check that drops a notification when SUM(follower_count) of
users who muted the commenter exceeds a threshold (1.7M prod, 4k dev).
Keeps the triggers localized; the threshold lives in apps' config not
the DB. If notification noise becomes a problem we can fold it in.
Notification payload shapes match apps verbatim (specifier, group_id,
data) so existing notification readers / clients keep working.
Schema dump regeneration follows in a separate commit (cf. 4da78ab
for the handle_comment_remix_contest_update precedent).
Tests (api/v1_comment_notifications_test.go — 9 tests, all DB-backed):
- TestCommentNotification_NotifiesTrackOwner — happy path: track owner
receives `comment` with the correct group_id and payload
- TestCommentNotification_SkipsSelfComment — self-comment no-op
- TestCommentNotification_SkipsReply — reply inserted with comment_threads
in the same tx → deferred trigger correctly skips
- TestCommentMention_NotifiesMentionedUser — mentioned user gets
`comment_mention` with entity_user_id / comment_user_id
- TestCommentMention_SkipsWhenMentionedMutedCommenter — muted_users gates
- TestCommentThread_NotifiesParentAuthor — parent author gets
`comment_thread`, specifier = reply comment_id
- TestCommentReaction_NotifiesCommentAuthor — author gets
`comment_reaction`, specifier = reacter user_id
- TestFanClubTextPost_FansOutToFollowersAndCoinHolders — recipients =
follower ∪ coin holder, deduped via UNION; artist excluded
- TestFanClubTextPost_SkipsFanComments — only artist's own top-level
posts trigger fan-out
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds handle_comment_{notification,mention,thread,reaction} and
handle_fan_club_text_post functions + their triggers to sql/01_schema.sql
so the test-schema template includes them and the comment-notification
tests pass. Also picks up the 0204 case-insensitivity view fix that main's
dump was missing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
raymondjacobson
force-pushed
the
api/comment-notifications
branch
from
55da6d1 to
17be5c8
Compare
…base CREATE DATABASE ... TEMPLATE fails with SQLSTATE 55006 when any session is connected to the source template. go test runs each package as its own process, so the package-level testMutex can't prevent one process holding a template connection open while another clones it. Connect to the postgres maintenance DB instead so no test process ever holds a template connection. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
4 tasks
raymondjacobson
added a commit
that referenced
this pull request
May 29, 2026
…e used (#880) ## Summary The actual fix for the `IndexChallengesJob` wedge that #877 was supposed to enable. #877's partial GIN was correct — but the trigger SQL couldn't reach it. ## Root cause The `on_user_challenge` and `on_challenge_disbursement` triggers both do per-row lookups against the 8 GB / ~23.5 M-row `notification` table to dedupe reward / cooldown notifications, e.g.: ```sql SELECT id FROM notification WHERE type = 'reward_in_cooldown' AND new.user_id = ANY(user_ids) AND timestamp >= (new.completed_at - interval '1 hour') ``` **PostgreSQL's GIN operator class supports `@>`, `&&`, `<@` — but NOT `scalar = ANY(array)`.** Even with the full `ix_notification` GIN, and the partial GIN added by `0210` (#877), every trigger call fell back to a parallel sequential scan of the entire 8 GB table. Confirmed in prod on cd94ede: ``` -> Parallel Seq Scan on notification (cost=0.00..963930.62) Filter: type='reward_in_cooldown' AND scalar = ANY(user_ids) Rows Removed by Filter: 7,846,045 Execution: 13,640 ms ``` And `pg_stat_user_indexes` showed `ix_notification_cooldown_user_ids.idx_scan = 0` since it was built — completely unused. ## Fix Rewrite the predicate to the canonical `@>` form. Semantics are identical (both test array membership), but only `@>` is GIN-eligible. Same EXPLAIN on the same row, with the same data: | Form | Plan | Execution | |---|---|---| | `new.user_id = ANY(user_ids)` (before) | Parallel Seq Scan | **13,640 ms** | | `user_ids @> ARRAY[new.user_id]` (after) | Bitmap Index Scan on `ix_notification_cooldown_user_ids` | **2 ms** | Three call sites updated: | File | What it does | |---|---| | `ddl/functions/handle_user_challenges.sql` | `reward_in_cooldown` dedupe path of `handle_on_user_challenge()` — fires on every `is_complete=true` write to `user_challenges` | | `ddl/functions/handle_challenge_disbursements.sql` (×2) | `challenge_reward` dedupe in both `handle_challenge_disbursement()` (legacy table) and `handle_sol_reward_disbursement()` (new indexer's table) | Schema dump (`sql/01_schema.sql`) and migration tracker checksums updated to match. ## Impact - **Challenge job**: per-upsert trigger cost drops from ~13 s → ~2 ms. The `IndexChallengesJob` first-tick wedge clears once a fresh backend picks up the new function (so a `core-indexer` pod restart after this deploys is the last manual step, unless the deploy itself replaces the pod). - **Rewards / disbursements**: per-disbursement trigger cost drops by the same factor; the rewards attester is no longer rate-limited by trigger latency. - **All other `cooldown_days > 0` challenges** (`p`, `u`, the Phase 2 ones, etc.) get the same speedup whenever they fire the trigger. ## Out of scope The wider codebase has **~50 other `= any(user_ids)` occurrences** across other trigger functions (notification triggers added in #851, etc.). Same anti-pattern — same fix. Worth a separate sweep PR; I left it out here to keep this one minimal and reviewable. ## Test plan - [x] `go build ./...`, `go vet ./...` clean (no Go changes; sanity check). - [x] Confirmed in prod via `EXPLAIN (ANALYZE, BUFFERS)` that the `@>` form picks `ix_notification_cooldown_user_ids` and completes in 2 ms (vs 13.6 s for the `= ANY` form). - [x] Function checksums in `sql/03_migration_tracker.sql` updated so `pg_migrate.sh` re-applies them on deploy. - [x] `sql/01_schema.sql` updated in lockstep so a fresh test template reflects the new function bodies. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
raymondjacobson
added a commit
that referenced
this pull request
May 29, 2026
## Summary Adds the three timer-driven notification creators the legacy Python discovery-provider celery beat produced, as scheduled parity jobs in the `core-indexer`. This closes the remaining functional gap between the vendored Go ETL indexer and the Python indexer for notifications: event-driven notifications are already handled by DB triggers (#851/#870), and these cover the ones that fire on elapsed time rather than an indexed entity. - **EngagementNotificationsJob** (`claimable_reward`): promotes 7-day-cooldown challenges to `claimable_reward` once their cooldown elapses and they're still undisbursed. Complements the `handle_on_user_challenge` trigger, which only emits an immediate `claimable_reward` for `cooldown_days = 0` and `reward_in_cooldown` for `cooldown_days > 0`. Hence the `cooldown_days = 7` filter, matching Python's hardcoded check. Scheduled every 10m. - **ListenStreakReminderJob** (`listen_streak_reminder`): reminds users in the 42-43h window after their last listen (6h of slack before the 48h streak breaks). Tight 1-2m window under `env=stage` for end-to-end testing, matching Python. Scheduled every 10s. - **RemixContestNotificationsJob**: the four `fan`/`artist` `remix_contest` `ended` / `ending_soon` notifications, with the same audience rules (remixers, host followers, parent-track favoriters, event subscribers; host excluded from fan types). Scheduled every 30s. Each job is a set-based `INSERT ... SELECT ... ON CONFLICT (group_id, specifier) DO NOTHING` for idempotency via `uq_notification`, and is wired into `startParityJobs`. Mirrors the corresponding `apps/` Python tasks. ## Test plan - [x] `go test ./jobs/...` — new tests cover the engagement full pipeline + exclusions (cooldown/disbursement/not-elapsed/incomplete), listen-streak window boundaries + idempotency, and remix-contest ended/ending-soon audiences with host exclusion. - [x] Tests account for the `handle_on_user_challenge` and `handle_event` triggers that also fire on seed (verified no overlap with the asserted notification types). - [x] `go vet ./jobs/... ./indexer/...` clean; full `jobs` package passes. - [ ] Observe in stage that the jobs emit the expected notifications on the real schedule. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the five comment-related notification triggers that apps'
src/tasks/entity_manager/entities/comment.pycreates directly during ManageEntity processing. The ETL handlers ingo-openaudio/pkg/etl/processors/entity_manager/comment_*.goalready write the source rows (comments,comment_mentions,comment_threads,comment_reactions) — but the user-facing notification rows had no Go equivalent.handle_comment.sqlhere previously only updatedaggregate_track.comment_count.Closes the largest remaining notification gap on the path to shutting off the Python discovery indexer (per the broader parity audit).
Triggers (one notification type per file)
handle_comment_notification.sqlcommentcommentsINSERThandle_comment_mention.sqlcomment_mentioncomment_mentionsINSERThandle_comment_thread.sqlcomment_threadcomment_threadsINSERThandle_comment_reaction.sqlcomment_reactioncomment_reactionsINSERThandle_fan_club_text_post.sqlfan_club_text_postcommentsINSERT (FanClub)Each follows the established repo pattern (one file per trigger, sibling of
handle_comment_remix_contest_update.sql). All useON CONFLICT DO NOTHINGagainstnotification.uq_notification(group_id, specifier)for dedup. Notification shape —specifier,group_id,datapayload — matches apps verbatim so existing readers/clients keep working.Why DEFERRABLE INITIALLY DEFERRED
`handle_comment_notification` and `handle_fan_club_text_post` need to look at `comment_threads` (to detect replies) and `comment_mentions` (to detect owner mentions). Those sibling rows are inserted after the `comments` row in the same indexer transaction. Same problem and same fix as `handle_comment_remix_contest_update.sql`.
Intentional gap: karma-based mute
Apps drops the notification when
SUM(follower_count)of users who muted the commenter exceeds a configured threshold (1.7M prod, 4k dev). Not ported here — keeps triggers localized and the threshold lives in apps' config, not the DB. Worst-case impact is "spammy commenters notify a few more users than they would have under apps." If that becomes a real noise problem we fold it in.Skip semantics (mirror apps)
comment— skips self-comment, replies (they getcomment_thread), owner-mentioned (they getcomment_mention),comment_notification_settings.is_muted,muted_userscomment_mention— skips self-mention, mention-muted-commenter, owner-mentioned-with-owner-mutecomment_thread— skips self-reply, parent author muted thread or repliercomment_reaction— skips self-react, comment-author mute, plus apps'track_owner_mention_mutewhen commenter is entity ownerfan_club_text_post— only artist's own top-level posts fan out; recipients = followers ∪ artist-coin holders, excluding artistSchema dump
Regeneration follows in a separate commit, matching 4da78ab for
handle_comment_remix_contest_update.Test plan
9 DB-backed tests against the
test_apitemplate, all passing locally:`go build ./...` and `go vet ./api/...` clean.
🤖 Generated with Claude Code