Repository navigation
fix(review): IDB-aware invalidation, mark-read rollback + review cleanups - #36
Merged
Merged
Conversation
…nups Post-waves high-effort code review (range 5d288cc..HEAD) — fixes the confirmed findings: Correctness - The IDB pref layer silently defeated react-query invalidation: GitHubIssueModal invalidated ['issues','search'] but the queryFn reads the issueSearch: pref first, so a closed/commented issue resurrected for up to 5 minutes. New db.clearPrefsByPrefix() is now called before invalidating; SentryIssueModal's inline equivalent reuses it. The helper's doc makes the contract explicit for future mutations. - Mark-as-read had no rollback: the optimistic removal also persisted to IDB, so a failed PATCH left local state permanently out of sync with GitHub. onError now drops the notifications pref and refetches to re-sync with the server. Cleanups (review's reuse/efficiency angles) - CopyButton now uses the useFlash hook instead of duplicating its timer logic. - searchIssues paginates at 100/page (GraphQL search max) — half the requests for the same 200 cap. - Notifications grouping computes each group's recency once (first item of the desc-sorted list) instead of a Math.max scan per sort comparison. Co-Authored-By: Claude Opus 4.8 (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
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.
High-effort code review over the four audit waves (
5d288cc..HEAD): 3 finder agents (line-by-line, removed-behavior + cross-file, cleanup angles), candidates verified against the code. 2 confirmed bug classes fixed + 3 cleanups applied. The refuted candidates (normal TTL semantics, speculative future-change risks) were dropped.🔴 Confirmed & fixed
1. The IDB pref layer silently defeated react-query invalidation
GitHubIssueModal.refresh()invalidated['issues','search']— but the queryFn reads theissueSearch:IDB pref first, which was still fresh → close/comment an issue and the feed resurrects the old state for up to 5 minutes. (Sentry resolve already did this right by dropping its prefix — the pattern existed, it just wasn't shared.)Fix: new
db.clearPrefsByPrefix(), called before invalidating;SentryIssueModalreuses it; the helper's docstring makes the mutations-must-drop-the-pref contract explicit for everything future.2. Mark-as-read had no rollback
The optimistic removal also persisted to IDB — a failed PATCH showed an error but left local state permanently desynced from GitHub.
Fix:
onErrordrops thenotifications:pref and refetches, re-syncing with the server.🧹 Cleanups (from the review's reuse/efficiency angles)
CopyButtonnow uses theuseFlashhook instead of duplicating its timer logic.searchIssuespaginates at 100/page (GraphQL search max) — half the requests for the same 200 cap.Math.maxscan per sort comparison.Verified clean (no action)
Relay hardening, demo branches, pagination loops (cursor always advances, caps hold),
useFlashadoptions, Dexie v4 drop (zero remaining readers).npm run checkgreen · 29/29 tests.🤖 Generated with Claude Code