[Fix] Notifications related.users: hydrate with viewer perspective#836
Open
dylanjeffers wants to merge 2 commits into
Open
[Fix] Notifications related.users: hydrate with viewer perspective#836dylanjeffers wants to merge 2 commits into
dylanjeffers wants to merge 2 commits into
Conversation
…uthors
The previous shadow-ban filter on the contest discovery list used
`aggregate_user.score < 0` (AAO output). Two problems:
1. `aggregate_user` has no index covering `score`, so the CTE forced a
full seq scan on every cold call. /v1/events/remix-contests?status=all
was hanging ~22s cold-cache (warm: ~100ms).
2. The AAO signal is a separate moderation lane from the community
karma-reports system that already governs comment visibility.
The two can drift.
Fix: align the contest filter with the comment-visibility filter. A host
is shadow-banned from contest discovery if they authored a comment that
crossed the same `high_karma_reporters` threshold (sum of reporters'
follower_count >= karmaCommentCountThreshold) that hides the comment
itself on v1_track_comments / v1_event_comments. The new CTE
`karma_reported_authors` lifts the comment-level signal up to user_id.
`muted_by_karma` is unchanged — still filters hosts muted by high-karma
users.
`comment_reports` is a small table indexed on `comment_id`, and the new
CTE only adds a hash-join on comments (PK lookup per hkr row), so the
cost is bounded by report volume rather than user-table size — no
sequential scan over millions of aggregate_user rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ed.users The /v1/notifications/:userId related.users block was hydrated with MyID=0 because getMyId reads the (absent) user_id query param. With my_id=0 the SQL short-circuits does_current_user_follow/_subscribe to false, so profile pages opened from a notification showed "Follow" for accounts the viewer already followed. The path-param userId IS the viewer on this endpoint, so thread it through to ParallelParams.MyID instead. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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
/v1/notifications/:userIdbuilds arelated.usersblock viaqueries.Parallel, but passedMyID = app.getMyId(c)— which reads the (typically absent)user_idquery param and defaults to 0. Withmy_id = 0,get_users.sqlshort-circuitsdoes_current_user_follow/does_current_user_subscribetofalsefor every related user.:userIdIS the viewer, so we now passuserIdthrough asMyID.TestV1Notifications_RelatedUsersViewerPerspective) that seeds a follow + two repost notifications and assertsdoes_current_user_followistruefor the followed actor andfalsefor the unfollowed actor inrelated.users. Verified by reverting the fix: the test fails withexpected: true / actual: falseexactly on the follow assertion.The client-side workaround in apps#14361 can be dropped once this ships.
Test plan
go test -count=1 -run 'TestV1Notifications' ./api/— all 13 notification tests passmain's behavior (reverted the fix locally, re-ran, assertion fired)🤖 Generated with Claude Code