diff --git a/agents-docs/LESSONS.md b/agents-docs/LESSONS.md index a0b58ee..122f941 100644 --- a/agents-docs/LESSONS.md +++ b/agents-docs/LESSONS.md @@ -25,6 +25,13 @@ Durable rules for AI agents working on this project. Read this file at session s ## Lessons +### Re-clear visible notification channels after recompute [notifications] [startup] + +- **Trigger:** fixing startup unread badges by only changing read-marker writes or initial hydration. +- **Rule:** also check later `loadMessagesSuccess` and `syncMessages` recomputes, and re-clear the focused visible channel after applying derived unread counts. +- **Why:** the startup-selected server can load or sync messages after it was marked read, reintroducing a channel unread badge even though the user is viewing that channel. +- **Example:** `NotificationsService.refreshRoomUnreadFromMessages(...)` should clear `activeChannelId` for `currentRoom` after recalculating counts from a startup message batch. + ### Disambiguate nested chat cards [chat] [ui] - **Trigger:** removing a visual treatment from chat history when a system message has both an outer row wrapper and an inner pill/card. diff --git a/toju-app/src/app/domains/notifications/README.md b/toju-app/src/app/domains/notifications/README.md index 16b4222..27f6fb1 100644 --- a/toju-app/src/app/domains/notifications/README.md +++ b/toju-app/src/app/domains/notifications/README.md @@ -143,6 +143,8 @@ Unread state is modeled as a combination of persisted read markers plus in-memor Important design constraint: unread counters are intentionally not persisted. The persisted state stores only the user-controlled settings and the read markers needed to derive unread counts again. +Read markers are written with the same server-adjusted clock that chat messages use, so a visible channel does not become unread again after reload when the signaling server clock is ahead of the local desktop clock. + ### Why baselines exist The domain must avoid marking an entire historical backlog as unread the first time a room appears. @@ -176,6 +178,7 @@ Unread is cleared by channel, not globally. - `markCurrentChannelReadIfActive()` only runs when the window is focused and the document is visible. - If a live message arrives in the currently visible text channel, the domain immediately marks that channel read instead of incrementing unread. +- When a startup message load or sync batch recomputes unread counts for the viewed server, the currently visible text channel is re-cleared and its read marker advances to the newest visible message in that batch. - Window focus and document visibility changes both clear taskbar attention and mark the active channel read. - `pruneUnreadState()` removes deleted or non-text channels from the unread maps and re-derives room totals from channel totals. diff --git a/toju-app/src/app/domains/notifications/application/services/notifications.service.spec.ts b/toju-app/src/app/domains/notifications/application/services/notifications.service.spec.ts index 18b472a..ffebd2e 100644 --- a/toju-app/src/app/domains/notifications/application/services/notifications.service.spec.ts +++ b/toju-app/src/app/domains/notifications/application/services/notifications.service.spec.ts @@ -10,6 +10,7 @@ import type { User } from '../../../../shared-kernel'; import { NotificationAudioService } from '../../../../core/services/notification-audio.service'; +import { TimeSyncService } from '../../../../core/services/time-sync.service'; import { DatabaseService } from '../../../../infrastructure/persistence'; import { selectActiveChannelId, @@ -33,6 +34,15 @@ const alice: User = { }; describe('NotificationsService', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(1_000); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + it('keeps channel read markers when startup room metadata has no channels yet', async () => { const roomWithoutChannels = createRoom({ channels: [] }); const context = createServiceContext({ @@ -53,12 +63,92 @@ describe('NotificationsService', () => { expect(context.service.settings().lastReadByChannel['room-1']?.['channel-1']).toBe(100); }); + + it('uses the server-adjusted clock when marking the active channel read', async () => { + const room = createRoom(); + const context = createServiceContext({ + activeChannelId: 'channel-1', + currentRoom: room, + currentUser: alice, + savedRooms: [room], + settings: { + ...createDefaultNotificationSettings(), + roomBaselines: { 'room-1': 0 } + }, + timeNow: 3_000 + }); + + await context.service.initialize(); + + expect(context.service.settings().lastReadByChannel['room-1']?.['channel-1']).toBe(3_000); + }); + + it('keeps the visible active channel read when startup message loads recompute unread', async () => { + const room = createRoom(); + const context = createServiceContext({ + activeChannelId: 'channel-1', + currentRoom: room, + currentUser: alice, + savedRooms: [room], + settings: { + ...createDefaultNotificationSettings(), + roomBaselines: { 'room-1': 0 } + }, + timeNow: 100 + }); + + await context.service.initialize(); + + context.service.refreshRoomUnreadFromMessages('room-1', [createMessage({ channelId: 'channel-1', senderId: 'bob', timestamp: 200 })]); + + expect(context.service.channelUnreadCount('room-1', 'channel-1')).toBe(0); + expect(context.service.roomUnreadCount('room-1')).toBe(0); + }); + + it('does not compute unread from messages before it is initialised', () => { + const room = createRoom(); + const context = createServiceContext({ + currentUser: alice, + savedRooms: [room], + settings: createDefaultNotificationSettings() + }); + + context.service.refreshRoomUnreadFromMessages('room-1', [createMessage({ channelId: 'channel-1', senderId: 'bob', timestamp: 5_000 })]); + + expect(context.service.roomUnreadCount('room-1')).toBe(0); + expect(context.service.channelUnreadCount('room-1', 'channel-1')).toBe(0); + }); + + it('hydrates unread counts from persisted read markers on initialize', async () => { + const room = createRoom(); + const context = createServiceContext({ + activeChannelId: 'other', + currentRoom: null, + currentUser: alice, + dbMessages: [createMessage({ channelId: 'channel-1', senderId: 'bob', timestamp: 5_000 })], + savedRooms: [room], + settings: { + ...createDefaultNotificationSettings(), + roomBaselines: { 'room-1': 0 } + }, + timeNow: 10_000 + }); + + await context.service.initialize(); + + expect(context.service.channelUnreadCount('room-1', 'channel-1')).toBe(1); + expect(context.service.roomUnreadCount('room-1')).toBe(1); + }); }); interface ServiceContextOptions { + activeChannelId?: string | null; + currentRoom?: Room | null; currentUser: User | null; + dbMessages?: Message[]; savedRooms: Room[]; settings: NotificationsSettings; + timeNow?: number; } interface ServiceContext { @@ -68,8 +158,8 @@ interface ServiceContext { function createServiceContext(options: ServiceContextOptions): ServiceContext { const currentUser = signal(options.currentUser); const savedRooms = signal(options.savedRooms); - const currentRoom = signal(null); - const activeChannelId = signal(null); + const currentRoom = signal(options.currentRoom ?? null); + const activeChannelId = signal(options.activeChannelId ?? null); let storedSettings = options.settings; @@ -78,7 +168,7 @@ function createServiceContext(options: ServiceContextOptions): ServiceContext { { provide: DatabaseService, useValue: { - getMessagesSince: vi.fn(async (): Promise => []) + getMessagesSince: vi.fn(async (): Promise => options.dbMessages ?? []) } }, { @@ -96,6 +186,12 @@ function createServiceContext(options: ServiceContextOptions): ServiceContext { play: vi.fn() } }, + { + provide: TimeSyncService, + useValue: { + now: vi.fn(() => options.timeNow ?? Date.now()) + } + }, { provide: NotificationSettingsStorageService, useValue: { @@ -154,3 +250,18 @@ function createRoom(overrides: Partial = {}): Room { ...overrides } as Room; } + +function createMessage(overrides: Partial = {}): Message { + return { + id: 'message-1', + roomId: 'room-1', + channelId: 'channel-1', + senderId: 'bob', + senderName: 'Bob', + content: 'hello', + timestamp: 1, + reactions: [], + isDeleted: false, + ...overrides + }; +} diff --git a/toju-app/src/app/domains/notifications/application/services/notifications.service.ts b/toju-app/src/app/domains/notifications/application/services/notifications.service.ts index db3df8a..e97d4d1 100644 --- a/toju-app/src/app/domains/notifications/application/services/notifications.service.ts +++ b/toju-app/src/app/domains/notifications/application/services/notifications.service.ts @@ -8,6 +8,7 @@ import { import { Store } from '@ngrx/store'; import type { Message, Room } from '../../../../shared-kernel'; import { NotificationAudioService, AppSound } from '../../../../core/services/notification-audio.service'; +import { TimeSyncService } from '../../../../core/services/time-sync.service'; import { DatabaseService } from '../../../../infrastructure/persistence'; import { selectActiveChannelId, @@ -47,6 +48,7 @@ export class NotificationsService { private readonly store = inject(Store); private readonly db = inject(DatabaseService); private readonly audio = inject(NotificationAudioService); + private readonly timeSync = inject(TimeSyncService); private readonly desktopNotifications = inject(DesktopNotificationService); private readonly storage = inject(NotificationSettingsStorageService); @@ -80,6 +82,7 @@ export class NotificationsService { this.registerWindowListeners(); this.registerWindowStateListener(); this.syncRoomCatalog(this.savedRooms()); + await this.hydrateUnreadCounts(this.savedRooms()); this.markCurrentChannelReadIfActive(); } @@ -150,6 +153,10 @@ export class NotificationsService { } refreshRoomUnreadFromMessages(roomId: string, messages: Message[]): void { + if (!this.initialised) { + return; + } + const room = getRoomById(this.savedRooms(), roomId); if (!room) { @@ -169,6 +176,7 @@ export class NotificationsService { } })); + this.markVisibleChannelReadFromMessages(roomId, messages); this.syncWindowAttention(); } @@ -462,8 +470,8 @@ export class NotificationsService { this.syncWindowAttention(); } - private markChannelRead(roomId: string, channelId: string, timestamp = Date.now()): void { - const nextReadAt = Math.max(timestamp, Date.now(), this.getChannelLastReadAt(roomId, channelId)); + private markChannelRead(roomId: string, channelId: string, timestamp = this.timeSync.now()): void { + const nextReadAt = Math.max(timestamp, this.timeSync.now(), this.getChannelLastReadAt(roomId, channelId)); this.setSettings({ ...this._settings(), @@ -497,6 +505,23 @@ export class NotificationsService { this.syncWindowAttention(); } + private markVisibleChannelReadFromMessages(roomId: string, messages: Message[]): void { + if (!this._windowFocused() || !this._documentVisible() || this.currentRoom()?.id !== roomId) { + return; + } + + const channelId = this.activeChannelId() || DEFAULT_TEXT_CHANNEL_ID; + const latestVisibleMessageTimestamp = messages.reduce((latestTimestamp, message) => { + if (resolveMessageChannelId(message) !== channelId) { + return latestTimestamp; + } + + return Math.max(latestTimestamp, message.timestamp); + }, 0); + + this.markChannelRead(roomId, channelId, latestVisibleMessageTimestamp); + } + private getChannelLastReadAt(roomId: string, channelId: string): number { return this._settings().lastReadByChannel[roomId]?.[channelId] ?? this._settings().roomBaselines[roomId] diff --git a/toju-app/src/app/domains/notifications/domain/logic/notification.logic.spec.ts b/toju-app/src/app/domains/notifications/domain/logic/notification.logic.spec.ts new file mode 100644 index 0000000..6364986 --- /dev/null +++ b/toju-app/src/app/domains/notifications/domain/logic/notification.logic.spec.ts @@ -0,0 +1,71 @@ +import type { Message, Room } from '../../../../shared-kernel'; +import { createDefaultNotificationSettings } from '../models/notification.model'; +import { calculateUnreadForRoom } from './notification.logic'; + +function createRoom(overrides: Partial = {}): Room { + return { + id: 'room-1', + name: 'Room', + hostId: 'host', + isPrivate: false, + createdAt: 0, + userCount: 1, + channels: [{ id: 'text-1', name: 'lobby', type: 'text', position: 0 }], + ...overrides + }; +} + +function createMessage(overrides: Partial = {}): Message { + return { + id: 'message-1', + roomId: 'room-1', + senderId: 'bob', + senderName: 'Bob', + content: 'hello', + timestamp: 100, + reactions: [], + isDeleted: false, + ...overrides + }; +} + +describe('calculateUnreadForRoom', () => { + it('ignores messages whose channel is not part of the room catalog', () => { + const room = createRoom(); + const settings = { + ...createDefaultNotificationSettings(), + roomBaselines: { 'room-1': 0 } + }; + const messages = [createMessage({ id: 'm-1', channelId: 'general', timestamp: 200 })]; + const counts = calculateUnreadForRoom(room, messages, settings, new Set()); + + expect(counts.roomCount).toBe(0); + expect(counts.channelCounts['general']).toBeUndefined(); + }); + + it('counts messages for known text channels', () => { + const room = createRoom(); + const settings = { + ...createDefaultNotificationSettings(), + roomBaselines: { 'room-1': 0 } + }; + const messages = [createMessage({ id: 'm-1', channelId: 'text-1', timestamp: 200 })]; + const counts = calculateUnreadForRoom(room, messages, settings, new Set()); + + expect(counts.roomCount).toBe(1); + expect(counts.channelCounts['text-1']).toBe(1); + }); + + it('treats empty channel ids as the default channel when the room has no catalog', () => { + const room = createRoom({ channels: [] }); + const settings = { + ...createDefaultNotificationSettings(), + roomBaselines: { 'room-1': 0 } + }; + const messages = [createMessage({ id: 'm-1', channelId: undefined, timestamp: 200 })]; + const counts = calculateUnreadForRoom(room, messages, settings, new Set()); + + expect(counts.roomCount).toBe(1); + expect(counts.channelCounts['general']).toBe(1); + }); +}); diff --git a/toju-app/src/app/domains/notifications/domain/logic/notification.logic.ts b/toju-app/src/app/domains/notifications/domain/logic/notification.logic.ts index b3f8137..6bca426 100644 --- a/toju-app/src/app/domains/notifications/domain/logic/notification.logic.ts +++ b/toju-app/src/app/domains/notifications/domain/logic/notification.logic.ts @@ -130,11 +130,15 @@ export function calculateUnreadForRoom( const channelId = resolveMessageChannelId(message); + if (!(channelId in channelCounts)) { + continue; + } + if (message.timestamp <= getChannelLastReadAt(settings, room.id, channelId)) { continue; } - channelCounts[channelId] = (channelCounts[channelId] ?? 0) + 1; + channelCounts[channelId] += 1; } return {