Skip to content

perf(notifications): skip re-enrichment of unchanged notifications - #3102

Merged
afonsojramos merged 4 commits into
gitify-app:mainfrom
joshfree:josh/skip-unchanged-notification-enrichment
Jul 29, 2026
Merged

perf(notifications): skip re-enrichment of unchanged notifications#3102
afonsojramos merged 4 commits into
gitify-app:mainfrom
joshfree:josh/skip-unchanged-notification-enrichment

Conversation

@joshfree

Copy link
Copy Markdown
Contributor

Fixes part of #3101 (Improvement 1 of 3).

Problem

On every poll, getAllNotificationsenrichNotifications
(src/renderer/utils/notifications/notifications.ts) re-fetches subject
details for every visible notification, with no change detection. For N
visible notifications this issues ceil(N / 100) GraphQL detail queries every
interval even when nothing changed.

Change

Cache the enriched subject keyed by account + notification id + updatedAt. On
each poll, notifications whose updatedAt is unchanged reuse the cached subject
and only cache misses are sent to the batched enrichment path. The cached
subject is applied onto the freshly-listed notification so read state and
ordering stay current, and the cache is pruned each cycle to the
currently-present notifications so it stays bounded.

The change lives in the enrichment orchestrator above the adapter, so it is
forge-agnostic and preserves existing behavior for changed notifications.

Tests

Added coverage in notifications.test.ts for the skip-when-unchanged and
re-fetch-when-updatedAt-changes cases.

Cache enriched notification subjects keyed by account + id + updatedAt so
that notifications whose updatedAt is unchanged are not re-fetched on every
poll. Only cache misses are sent to the batched enrichment path, and the
cache is pruned each cycle to the currently-present notifications so it
stays bounded.

Refs gitify-app#3101
@setchy

setchy commented Jul 29, 2026

Copy link
Copy Markdown
Member

Thanks Josh.

One question - does the notification timestamp get updated when enriched details change? I looked into this 12-18 mths ago and have a faint (could be incorrect) recollection that there was something quirky about how the notification list data returns with respect to enriched details.

There was also some interesting notification timestamp behavior as notifications go from your unread inbox, to done/read, to being manually moved back to your inbox - so we'd need to go ensure our cache key handles this, too

@joshfree

joshfree commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Hey @setchy -- thanks so much for taking the time to look at this, and for the thoughtful question — really appreciate it.

On the read/done -> back-to-inbox case: I think the current approach handles it, but you know the notification lifecycle far better than I do, so please tell me if I'm missing the quirk you ran into. The cache only stores the enriched subject and applies it back onto the fresh list item, so read/unread/done state and ordering still come straight from the live list. It also prunes to whatever's currently in the list each cycle, so anything that leaves and returns comes back as a miss and gets re-fetched.

On  updatedAt  not always moving: you're absolutely right, and I'll defer to you here. State changes and new comments bump it reliably, but things like labels, reactions, and milestones can change without it, so those could lag behind.

One idea, entirely subject to your preference: instead of a fixed timer (which is awkward anyway, since to actually save any calls it has to be longer than the poll interval, and that's user-configurable), we could tie freshness to the manual refresh -- background polling uses the cache, an explicit refresh clears it and re-enriches. But I'm very happy to go whichever way you think fits Gitify best.

How would you like me to handle it?

@github-actions github-actions Bot added the refactor Refactoring of existing feature label Jul 29, 2026
@afonsojramos

Copy link
Copy Markdown
Member

Following up on the open questions here, I ran a live experiment against the real notifications API to pin down the lifecycle semantics, since that determines whether the account:id:updatedAt cache key is sound.

Method: a scratch private repo with a workflow_dispatch workflow so github-actions[bot] acts as a second user (creates an issue mentioning me, comments, labels, closes). That generated a disposable notification thread on my account, which I drove through unread → read → resurfaced → closed → done via the REST API, measuring the thread updated_at and the list validators at every step.

Results:

Event thread updated_at
New comment bumps
Issue closed / reopened (state change) bumps
Label added unchanged
Marked read, then new comment arrives same thread id resurfaces as unread with a new updated_at

So the cache key concept holds up:

  • Anything that changes the enriched state (the field our detailed filters actually consume) rotates the key and forces a re-fetch.
  • The read/done → back-to-inbox flow is covered: if the thread returns due to new activity it comes back with a new updated_at (cache miss), and if it merely left the list for a poll it was pruned and re-fetches on return.
  • The known gap is label/milestone/reaction changes, which do not bump updatedAt. Today that is display-only staleness (no filter reads those fields), refreshed whenever the thread next has real activity. Worth keeping in mind if we ever add label-based filters.

While testing this I also found two bugs, and (taking advantage of "maintainers can modify") pushed fixes to this branch:

  1. Multi-account cache wipe. getAllNotifications enriches each account in a separate call, but the prune loop deleted every key not in the current call's list, so with 2+ accounts each account evicted the others' entries and the cache never hit (verified with a regression test: 4 detail fetches where 2 were expected). The prune is now scoped to the polled account's key prefix.
  2. Failed enrichments were cached. The GitHub adapter swallows detail-fetch errors and returns base subject details; those were cached under the unchanged key, so one transient outage would degrade notifications (wrong icons/state) until the thread next changed. The adapter now preserves the original subject reference on failure and the orchestrator skips caching those, so they retry on the next poll.

Both come with regression tests; full suite is green (157 files / 1201 tests), plus format, lint, and tsc.

@joshfree

Copy link
Copy Markdown
Contributor Author

Thank you — this is a fantastic review, and the two fixes (multi-account prune scoping + not caching failed enrichments) are clear improvements. Appreciate you driving the live lifecycle experiment to validate the cache key; the label/milestone display-only caveat is a good one to keep in mind if filters ever read those fields.

@setchy

setchy commented Jul 29, 2026

Copy link
Copy Markdown
Member

Thinking through the labels/milestone/reactions changes - what if we bust the cache on Gitify window open via tanstack query?

That way we

  • cache the enriched state while the window is kept open
  • cache the enriched state while the window is in the background
  • ensure that when the user does view the Gitify inbox that it's updated at that point in time and not drifting...

@afonsojramos
afonsojramos merged commit 2ff02e1 into gitify-app:main Jul 29, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactoring of existing feature

Development

Successfully merging this pull request may close these issues.

3 participants