fix(desktop): derive unread state from NIP-RS + relay catch-up only#598
Closed
tlongwell-block wants to merge 1 commit into
Closed
fix(desktop): derive unread state from NIP-RS + relay catch-up only#598tlongwell-block wants to merge 1 commit into
tlongwell-block wants to merge 1 commit into
Conversation
…badges survive restart
useUnreadChannels computes unread as 'latest > readMarker', where latest is
an in-session Map populated only by the live subscription. Two facts combine
to wipe the comparison on every restart:
1. The live sub subscribes with since: now, so it doesn't backfill.
2. Channel.lastMessageAt from the backend is always null (ChannelRecord has
no last_message_at field), so the seed-from-channel-metadata effect is a
no-op today.
Net effect: every channel's 'latest' is undefined on startup, the predicate
returns false, and badges disappear. Read markers via NIP-RS were never
actually wiped — they just had nothing left to be compared against.
Fix: persist the latestByChannelRef Map to localStorage per pubkey, hydrate
on identity setup, and write through whenever the map advances. The write
path is already filtered to UNREAD_TRIGGER_KINDS and external authors
upstream in useLiveChannelUpdates, so we don't create phantom unread from
edits/reactions/system events or from the user's own messages.
The deeper fix is to have the backend populate Channel.last_message_at via
the existing sprout-db get_last_message_at_bulk and feed it through the
list-channels handler; the seeding effect in useUnreadChannels is already
wired for that. Filing as follow-up.
Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com>
tlongwell-block
force-pushed
the
fix/persist-unread-latest-message
branch
from
014519c to
4650272
Compare
tlongwell-block
changed the title
Collaborator
Author
|
Force-push after design pivot broke this PR's branch link. Continuation in #599 — same branch, new content. Closing this one. |
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.
What
Make GUI unread state depend solely on (a) the NIP-RS read marker and (b) a per-channel relay catch-up query that asks "are there messages newer than this marker?" — no localStorage cache, no
Channel.last_message_atdependency, no schema extension.Why
useUnreadChannelshad two halves: a NIP-RS read marker (which persists fine, locally and cross-device) and an in-memorylatestByChannelRefMap that got built up only from the live subscription (since: now). On every restart the Map reset to empty, the comparisonlatest > readMarkeralways returnedfalse, and badges silently disappeared until new live activity hit the channel.The original fix in this branch persisted the Map to
localStorage. Tyler's review surfaced a better model: the relay already knows what's in each channel, so just ask it. The NIP-RS spec is explicit thatcontextscarries read positions — channel "latest" is the wrong shape for that blob. And we don't need any client-side latest cache if we can query "newer than marker?" on demand.How
latestMessageStorage.tsmodule added earlier in this branch.Channel.lastMessageAtopportunistic backfill effect and the read-state seeding effect fromuseUnreadChannels. Backend never populatedlast_message_at, both effects were dead branches.since: readMarker + 1(strict-newer; client-side> readAtbelt-and-suspenders),limit: 1000, in parallel. Take the max external-author event timestamp per channel; feed the existing predicate.channelIdsKeystring so React Query refetches with identical contents don't churn. Optimistic claim ofcaughtUpChannelsRefprevents duplicate REQs from re-renders; cleanup releases claims so the next effect run retries; individual failed fetches release their own claim so transient relay errors don't suppress unread for the rest of the session.isCancelledguard around the post-settle mutation handles identity reset mid-flight.markChannelUnreadfalls back tolatestByChannelRef.current.get(channelId)whenChannel.lastMessageAtis null (always today), so right-click "mark unread" syncs across devices when we've observed any external message.CHANNEL_MESSAGE_EVENT_KINDSfromshared/constants/kindssouseLiveChannelUpdatesand the catch-up share one source of truth for "unread trigger kinds."The live subscription remains
since: now— unchanged.Verification
pnpm --dir desktop typecheck✅pnpm --dir desktop check(biome + file-size) ✅Reviewers
Reviewed in #sprout-bugs by @max. Two rounds of review caught: (1)
limit: 1is unsafe when self-author filter discards the only returned event, (2) the original retry path left failed channels permanently "caught up" — both fixed. Approved at v3.Closes
The "unread indicators disappear on restart" regression Tyler reported in #sprout-bugs.