Skip to content

[Perf] Bundle related entities in /v1/notifications response#799

Merged
dylanjeffers merged 4 commits into
mainfrom
notifications-bundle-related-entities
May 11, 2026
Merged

[Perf] Bundle related entities in /v1/notifications response#799
dylanjeffers merged 4 commits into
mainfrom
notifications-bundle-related-entities

Conversation

@dylanjeffers
Copy link
Copy Markdown
Contributor

Summary

  • Hydrate users/tracks/playlists referenced by /v1/notifications action data inline under a new related block, eliminating the N+1 client round trips currently needed to render notifications.
  • Mirrors the pattern already used by the comments endpoints (app.queries.Parallel(...) + data / related envelope).
  • Cap actor mining at 1 action per notification group (clients render a single avatar per group, so additional initiator profiles are wasted bytes). Target entities are duplicated across every action in a group, so the cap doesn't drop them.
  • Polymorphic fields (*_item_id, content_id, comment entity_id) are routed to tracks vs. playlists by their sibling discriminator (type / content_type / entity_type).
  • Pure addition to the response shape — existing clients that ignore related keep working.

Test plan

  • go build ./... and go vet ./api/... clean
  • CI runs TestV1Notifications_RelatedEntities (covers hydration of users, tracks, playlists; asserts the per-group actor cap bounds a 5-follower fan-out group)
  • CI runs the existing TestV1Notifications* suite (response is purely additive — existing assertions on data.notifications.* should still pass)
  • Manual sanity check on staging: hit /v1/notifications/{me} and confirm related.users, related.tracks, related.playlists populate for follow / repost / save / comment / tip notifications

Notes

  • IncludeUnlisted: true for the parallel hydration — notifications are recipient-personal and may legitimately reference unlisted tracks the user has a relationship with.
  • Comment entity_id is folded into related.tracks only when entity_type == "Track". The FanClub and Event entity types don't have a hydrator in Parallel today; clients can keep round-tripping for those until we wire them in.

🤖 Generated with Claude Code

dylanjeffers and others added 4 commits May 8, 2026 15:35
Avoid N+1 client round trips by hydrating users/tracks/playlists
referenced by notification action data inline under a `related` key,
matching the pattern used by the comments endpoints.

To prevent fan-out groups (e.g. one notification representing 100 new
followers) from bloating the payload, we mine actor IDs from at most
3 actions per group; target entity IDs are duplicated across every
action so the cap doesn't drop them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Clients render a single avatar per notification group, so hydrating
more actors just bloats the response. Targets are unaffected — they
repeat across every action in a group, so the first action still
carries them.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Original test run logs expired; pushing an empty commit to get
fresh CI output for the failed Go Tests job.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The fixture's 2025-01-XX timestamps fell outside the SQL handler's
90-day initial-load window (the live test runs in mid-2026), so all
seeded notifications got filtered out and the asserted related lists
came back empty. Letting the seed default (time.Now()) take over
keeps the rows inside the window.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dylanjeffers dylanjeffers merged commit 602f7b0 into main May 11, 2026
5 checks passed
@dylanjeffers dylanjeffers deleted the notifications-bundle-related-entities branch May 11, 2026 05:44
dylanjeffers added a commit that referenced this pull request May 11, 2026
## Summary
- [PR #799](#799) added a
`related` block (`{ users, tracks, playlists }`) to the
`/v1/notifications` response in Go but didn't update the swagger spec to
match.
- Without this, generated SDK clients (e.g. `@audius/sdk`) don't see the
field and consumers can't read it in a type-safe way.
- One-line addition — `$ref`s the existing `related` schema (already
defined at line 13146 for other endpoints).

## Test plan
- [x] Spec lints / parses (single property addition under existing
schema)
- [ ] Regenerating SDK clients picks up `related?: Related` on
`NotificationsResponse` (verified locally against the `@audius/sdk`
typescript-fetch generator)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dylanjeffers added a commit to AudiusProject/apps that referenced this pull request May 11, 2026
#14287)

## Summary
- The `/v1/notifications` endpoint now returns a `related` block (`{
users, tracks, playlists }`) hydrated server-side ([api PR
#799](AudiusProject/api#799)). Read those
directly to prime the tan-query cache instead of firing N+1 secondary
fetches per page.
- Drops `useUsers` / `useTracks` / `useCollections` prefetch calls from
`useNotifications`, the per-type ID-collection walker, and the last-page
hold-back gating.
- Drops the `isAllPending` field on the hook result; callers now use the
standard `isPending` directly.

## Changes
- **SDK** — regenerate `NotificationsResponse` model with `related?:
Related` (paired with the swagger fix in [AudiusProject/api PR for
`fix/swagger-notifications-related-field`](https://github.com/AudiusProject/api/pulls)).
One-file regen diff — no incidental churn.
- **`packages/common`** — `useNotifications` rewritten to call
`primeRelatedData({ related: response.related, queryClient })` once per
page. Manual SDK type cast for `getNotifications` removed (proper type
now flows through). `primeRelatedData` extended to also seed playlists
via `primeCollectionData`.
- **`packages/web` + `packages/mobile`** — `NotificationPanel`,
`NotificationPage`, `NotificationList` updated to read `isPending`
directly from the query result.
- **Tooling** — fix `packages/sdk/rollup.config.ts` so `npm run
build:sdk` works on Node 22 (the version pinned by `.nvmrc`): read
`package.json` via `fs.readFileSync` (instead of an ESM JSON import,
which `--configPlugin typescript` downlevels to the unsupported `assert
{ type: 'json' }` syntax) and use a named import for
`@rollup/plugin-babel` (its CJS default-export shim doesn't survive ESM
interop in modern Node).

## Test plan
- [x] `tsc --noEmit` in `packages/common` — refactored code typechecks
(pre-existing unrelated errors only)
- [x] `npm run build:sdk` succeeds on Node 22.21.1 (verified locally;
was failing prior to the rollup fix)
- [ ] Notifications panel renders avatars + track names for follows,
reposts, saves, comments
- [ ] Network tab shows a single `/v1/notifications` request per page
(no follow-up `/users`, `/tracks`, `/playlists` round-trips)
- [ ] Pagination via "Load More" still works

## Notes
- `related` lives at the response root (sibling to `data`), not
per-notification — mirrors the `data`/`related` envelope already used by
the comments endpoints. Per-page cache-seed instead of per-notification.
- This PR depends on the API server returning `related` (already merged
in api #799). The swagger schema catch-up is a separate small PR in the
api repo.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant