fix(app): separate unread state from unread counts

fluxer_app/src/features/read_state/state/ReadStates.test.ts

@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import {beforeEach, describe, expect, it, vi} from 'vitest';
+
+const makeChannel = (id: string) => ({
+	id,
+	type: 0,
+	guildId: 'guild-1',
+	isPrivate: () => false,
+	getGuildId: () => 'guild-1',
+});
+const loadedMessages: Array<{id: string; author: {id: string}}> = [];
+let hasMoreBefore = false;
+let hasNewestMessages = true;
+
+vi.mock('@app/features/channel/state/Channels', () => ({
+	default: {getChannel: (id: string) => makeChannel(id), getBasicChannel: (id: string) => makeChannel(id)},
+}));
+vi.mock('@app/features/messaging/state/MessagingMessages', () => ({
+	default: {
+		getMessages: () => ({
+			get hasMoreBefore() {
+				return hasMoreBefore;
+			},
+			get length() {
+				return loadedMessages.length;
+			},
+			jumpDestinationId: null,
+			hasNewestMessages: () => hasNewestMessages,
+			has: (id: string) => loadedMessages.some((m) => m.id === id),
+			last: () => loadedMessages[loadedMessages.length - 1],
+			forEachBuffered: (cb: (m: unknown) => void) => {
+				for (const m of loadedMessages) cb(m);
+			},
+		}),
+	},
+}));
+vi.mock('@app/features/user/state/Users', () => ({default: {getCurrentUser: () => ({id: 'me'})}}));
+vi.mock('@app/features/relationship/state/Relationships', () => ({default: {isBlocked: () => false}}));
+vi.mock('@app/features/member/state/GuildMembers', () => ({default: {getMember: () => null}}));
+vi.mock('@app/features/user/state/UserGuildSettings', () => ({
+	default: {
+		isEveryoneMentionSuppressed: () => false,
+		isRoleMentionSuppressed: () => false,
+		isGuildOrChannelMuted: () => false,
+	},
+}));
+vi.mock('@app/features/ui/state/Dimension', () => ({default: {channelPinnedToEnd: () => false}}));
+vi.mock('@app/features/notification/state/NotificationAutoAck', () => ({
+	default: {isAutomaticAckEnabled: () => false},
+}));
+vi.mock('@app/features/platform/transport/RestTransport', () => ({http: {post: vi.fn(), get: vi.fn()}}));
+
+const {default: ReadStates} = await import('@app/features/read_state/state/ReadStates');
+
+const ID = {
+	ack: '1519773906704011264',
+	newer: '1519773906708205568',
+};
+
+let nextChannelId = 0;
+
+function seedReadChannel() {
+	const channelId = `channel-${++nextChannelId}`;
+	const state = ReadStates.get(channelId);
+	state.readStateKnown = true;
+	state.ackMessageId = ID.ack;
+	state.lastMessageId = ID.ack;
+	state.unreadCount = 0;
+	state.oldestUnreadMessageId = null;
+	return {channelId, state};
+}
+
+describe('ReadStates unread invariant', () => {
+	beforeEach(() => {
+		loadedMessages.length = 0;
+		hasMoreBefore = false;
+		hasNewestMessages = true;
+	});
+
+	it('never reports a positive unread count without an unread anchor after a passive update', () => {
+		const {channelId} = seedReadChannel();
+		ReadStates.handlePassiveLastMessageUpdates({[channelId]: ID.newer}, 'guild-1');
+		const count = ReadStates.getUnreadCount(channelId);
+		const anchor = ReadStates.getVisualUnreadMessageId(channelId);
+		expect(count > 0).toBe(anchor != null);
+	});
+
+	it('keeps the channel unread for the sidebar even with no anchor to draw a divider at', () => {
+		const {channelId} = seedReadChannel();
+		ReadStates.handlePassiveLastMessageUpdates({[channelId]: ID.newer}, 'guild-1');
+		expect(ReadStates.hasUnread(channelId)).toBe(true);
+	});
+
+	it('clears a stale unread once the server walks the last message id back', () => {
+		const {channelId} = seedReadChannel();
+		ReadStates.handlePassiveLastMessageUpdates({[channelId]: ID.newer}, 'guild-1');
+		expect(ReadStates.hasUnread(channelId)).toBe(true);
+		ReadStates.handlePassiveLastMessageUpdates({[channelId]: ID.ack}, 'guild-1');
+		expect(ReadStates.hasUnread(channelId)).toBe(false);
+		expect(ReadStates.getUnreadCount(channelId)).toBe(0);
+		expect(ReadStates.getVisualUnreadMessageId(channelId)).toBeNull();
+	});
+
+	it('anchors the divider when a window is loaded whose ack sits outside it', () => {
+		const {channelId, state} = seedReadChannel();
+		state.lastMessageId = ID.newer;
+		hasMoreBefore = true;
+		loadedMessages.push({id: ID.newer, author: {id: 'someone'}});
+		ReadStates.handleLoadMessages({channelId, messages: []});
+		expect(ReadStates.getVisualUnreadMessageId(channelId)).toBe(ID.newer);
+		expect(ReadStates.getUnreadCount(channelId) > 0).toBe(true);
+	});
+});

fluxer_app/src/features/read_state/state/ReadStates.ts

@@ -141,16 +141,11 @@ class ReadStates {
 		this.refreshMentionChannel(state.channelId);
 	}
 
-	private refreshUnreadEstimate(state: ReadStateEntry): void {
+	private clearUnreadStateIfRead(state: ReadStateEntry): void {
 		if (!state.hasUnread()) {
 			state.estimated = false;
 			state.unreadCount = 0;
 			state.oldestUnreadMessageId = null;
-			return;
-		}
-		if (state.unreadCount === 0) {
-			state.estimated = true;
-			state.unreadCount = Math.max(1, state.mentionCount);
 		}
 	}
 
@@ -416,7 +411,7 @@ class ReadStates {
 				if (!channelsWithReadState.has(channel.id as ChannelId)) {
 					this.setMentionCount(state, 0);
 				}
-				this.refreshUnreadEstimate(state);
+				this.clearUnreadStateIfRead(state);
 			}
 			this.notifyChange(undefined, {global: true});
 		});
@@ -436,7 +431,7 @@ class ReadStates {
 					state.lastMessageId = channel.last_message_id ?? null;
 					state.lastPinTimestamp = parseTimestamp(channel.last_pin_timestamp);
 					state.storedGuildId = action.guild.id;
-					this.refreshUnreadEstimate(state);
+					this.clearUnreadStateIfRead(state);
 					this.refreshMentionChannel(channel.id);
 				}
 			}
@@ -452,9 +447,11 @@ class ReadStates {
 		if (newestMessage != null && isNewerMessageId(newestMessage.id, state.lastMessageId)) {
 			state.lastMessageId = newestMessage.id;
 		}
-		if (state.hasUnread()) {
+		const landedOnNewestWindow = messages.hasNewestMessages();
+		const landedOnAck = state.ackMessageId != null && messages.jumpDestinationId === state.ackMessageId;
+		if (state.hasUnread() || landedOnNewestWindow || landedOnAck) {
 			state.rebuild();
-			this.refreshUnreadEstimate(state);
+			this.clearUnreadStateIfRead(state);
 		} else if (action.isAfter && state.ackMessageId != null && messages.has(state.ackMessageId, true)) {
 			state.unreadCount += action.messages.length;
 			if (state.oldestUnreadMessageId == null) {
@@ -564,7 +561,7 @@ class ReadStates {
 			state.readStateKnown = true;
 			state.ackMessageId = action.channel.last_message_id;
 		} else if (GUILD_TEXT_BASED_CHANNEL_TYPES.has(action.channel.type) && state.hasUnread()) {
-			this.refreshUnreadEstimate(state);
+			this.clearUnreadStateIfRead(state);
 		}
 		this.notifyChange(action.channel.id);
 	}
@@ -586,10 +583,10 @@ class ReadStates {
 				changed = state.guildId !== guildId;
 				state.storedGuildId = guildId;
 			}
-			if (isNewerMessageId(lastMessageId, state.lastMessageId)) {
+			if (lastMessageId !== state.lastMessageId) {
 				state.lastMessageId = lastMessageId;
 				changed = true;
-				this.refreshUnreadEstimate(state);
+				this.clearUnreadStateIfRead(state);
 			}
 			if (changed) {
 				changedChannels.push(channelId as ChannelId);
@@ -751,7 +748,7 @@ class ReadStates {
 					this.setMentionCount(state, mentionCount);
 				}
 				if (decision.shouldRefreshUnreadEstimate) {
-					this.refreshUnreadEstimate(state);
+					this.clearUnreadStateIfRead(state);
 				}
 				if (decision.shouldNotify) {
 					this.notifyChange(action.channelId);
@@ -1046,7 +1043,7 @@ class ReadStates {
 				state.serverVersion = readState.version ?? state.serverVersion;
 				this.setMentionCount(state, readState.mention_count ?? 0);
 				state.rebuild(null, {recomputeMentions: manual});
-				this.refreshUnreadEstimate(state);
+				this.clearUnreadStateIfRead(state);
 				this.notifyChange(readState.id);
 				continue;
 			}

fluxer_app/src/features/read_state/state/read_states/ReadStateEntry.ts

@@ -274,7 +274,7 @@ export class ReadStateEntry {
 		});
 		const hasUnreadBoundary = foundAckMessage || loadedOlderMessages || !messages.hasMoreBefore;
 		const hasNewestMessages = messages.hasNewestMessages();
-		this.estimated = !hasNewestMessages || !hasUnreadBoundary;
+		this.estimated = !hasNewestMessages || (!hasUnreadBoundary && messages.length === loadedUnreadCount);
 		if (this.estimated) {
 			this.unreadCount = Math.max(previousUnreadCount, loadedUnreadCount);
 		} else {