Skip to content

fix(notifications): paginate and index the notification list endpoints#564

Draft
tagpro wants to merge 1 commit into
maxdorninger:masterfrom
tagpro:fix/notifications-pagination
Draft

fix(notifications): paginate and index the notification list endpoints#564
tagpro wants to merge 1 commit into
maxdorninger:masterfrom
tagpro:fix/notifications-pagination

Conversation

@tagpro

@tagpro tagpro commented Jul 11, 2026

Copy link
Copy Markdown

Problem

The notifications tab is very slow to load. Both list endpoints return the entire notification table:

# media_manager/notification/repository.py
stmt = select(Notification).order_by(Notification.timestamp.desc())   # no limit/offset

So every visit to the tab fetches every row, pydantic-validates each one, serialises the whole list to JSON and ships it to the browser. On the instance where I hit this, the notification table had ~437,000 rows — the tab was loading all of them on every page load and on every 30s poll.

The table also has no index beyond the primary key on id, so the ORDER BY timestamp DESC sorts the whole table on every load, and the unread query (WHERE read = false) is a full scan.

Changes

Backend

  • GET /notification and GET /notification/unread now take limit (default 50, le=200) and offset (ge=0), threaded through the service into .limit()/.offset() in the SQL. Ordering is unchanged (timestamp descending).
  • The total number of matching notifications is returned in an X-Total-Count response header (and exposed via CORS) so the UI knows how many pages there are.
  • GET /notification takes an optional read filter. The UI previously built its "read" list by fetching everything and filtering client-side, which is not possible once the endpoint is paginated.
  • New migration (b3d51c7fa9e2, chained off e60ae827ed98) adds an index on timestamp DESC for the ORDER BY, and a partial index (WHERE read = false) for the unread query.
  • New PATCH /notification/read marks every unread notification as read in a single UPDATE. "Mark All as Read" used to work by looping a PATCH over every notification the client held — that only worked because the client held all of them, and would silently mark just the current page now.

Frontend

  • The notifications page loads a page of 50 with a "Load more" button and a "Showing N of M" count, for both the unread and read sections. Read notifications — the bulk of the table — are not fetched at all until that section is expanded.

Tests

  • Repository tests covering: limit caps the number returned, offset skips correctly, ordering is timestamp descending, the unread query returns only unread rows and respects limit/offset, and the bulk mark-as-read reaches past the loaded page. Seeded with more rows than the page size so pagination is actually exercised.
  • This adds a small pytest harness (tests/, async SQLite via aiosqlite, plus a test dependency group), since there wasn't one yet.

Not in this PR

The reason that instance had 437k notifications is a separate root cause: identical failure notifications are written every import cycle, so the table floods. That's worth fixing on its own; this PR is limited to making the endpoint bounded and indexed, which is what makes the tab slow regardless of how the rows got there.

Verification

  • uv run pytest — 14 passed
  • uv run ruff check ./media_manager — clean
  • npm run lint (in web/) — clean
  • Drove the router end-to-end against a seeded DB: default page returns 50 rows with X-Total-Count, limit=500 is rejected with 422, offset pages don't overlap, the unread page never contains read rows, and PATCH /notification/read empties the unread list.
  • alembic upgrade e60ae827ed98:b3d51c7fa9e2 --sql renders the expected DDL, including the partial index.

🤖 Generated with Claude Code

https://claude.ai/code/session_019wrZPxJDip5x3mzjWnpwos

The notification tab loaded slowly because both list endpoints returned the
entire notification table. GET /notification and GET /notification/unread ran
`select(Notification).order_by(timestamp desc)` with no limit, so every page
load fetched every row, pydantic-validated each one, serialised the lot to JSON
and shipped it to the browser. On the instance this was found on the table had
~437,000 rows.

The table also had no index beyond the primary key on `id`, so the ORDER BY
sorted the whole table on every load and the unread filter was a full scan.

Backend:
- Both list endpoints now take `limit` (default 50, max 200) and `offset`
  (>= 0), threaded through the service into `.limit()/.offset()` in the SQL.
  Ordering stays timestamp descending. The number of matching notifications is
  returned in the `X-Total-Count` response header (exposed via CORS) so the UI
  can paginate.
- GET /notification takes an optional `read` filter, so the UI can page through
  read notifications rather than fetching everything and filtering client-side.
- New migration indexes the table for these queries: an index on `timestamp`
  descending for the ORDER BY, and a partial index for the unread query.
- New PATCH /notification/read marks every unread notification read in a single
  statement. The UI used to do this by looping a PATCH over every notification
  it held, which only worked because it held all of them.

Frontend:
- The notifications page now loads a page of 50 with "load more", and does not
  fetch read notifications at all until that section is expanded.

Tests:
- Repository tests covering the limit cap, offset, timestamp-descending
  ordering, the unread query, and the bulk mark-as-read, seeded with more rows
  than the page size so pagination is actually exercised.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wrZPxJDip5x3mzjWnpwos
@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 99aa9312-e138-48ae-a6cb-a7265d475e94

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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