From b807eca7fe3ddfaf2ec83600889b491bb00060dc Mon Sep 17 00:00:00 2001 From: klopez4212 Date: Sat, 27 Jun 2026 09:56:46 +0100 Subject: [PATCH] Fix task conversation foundation issues --- .../agents/agentConversations.test.mjs | 78 +++++++++++++++++++ .../src/features/agents/agentConversations.ts | 39 +++++++--- .../src/features/channels/ui/ChannelPane.tsx | 11 --- .../messages/lib/timelineItems.test.mjs | 18 +++++ .../features/messages/lib/timelineItems.ts | 7 +- .../features/messages/ui/MessageTimeline.tsx | 5 +- .../messages/ui/TimelineMessageList.tsx | 9 ++- .../src/features/sidebar/ui/AppSidebar.tsx | 41 +--------- 8 files changed, 144 insertions(+), 64 deletions(-) diff --git a/desktop/src/features/agents/agentConversations.test.mjs b/desktop/src/features/agents/agentConversations.test.mjs index 99f32d7ccd..8cfa2180b1 100644 --- a/desktop/src/features/agents/agentConversations.test.mjs +++ b/desktop/src/features/agents/agentConversations.test.mjs @@ -407,3 +407,81 @@ test("continued conversation marker hides same-second messages after the anchor" assert.deepEqual([...hiddenIds], ["after"]); }); + +test("continued conversation marker with a missing anchor does not hide thread messages", () => { + const root = message({ + body: "Can you look into the data model?", + createdAt: 1, + id: "root", + }); + const reply = message({ + body: "One note before opening.", + createdAt: 3, + id: "reply", + }); + const marker = parseAgentConversationMarker( + markerEvent({ content: { agentReplyId: "missing-reply" }, createdAt: 4 }), + ); + + const hiddenIds = getHiddenAgentConversationMessageIds( + [root, reply], + marker ? [marker] : [], + ); + + assert.deepEqual([...hiddenIds], []); +}); + +test("continued conversation markers keep later task anchors visible", () => { + const root = message({ + body: "Can you look into the data model?", + createdAt: 1, + id: "root", + }); + const firstAnchor = message({ + body: "I'll look into it.", + createdAt: 2, + id: "agent-reply", + pubkey: "agent", + }); + const hiddenReply = message({ + body: "This should live in the first task.", + createdAt: 3, + id: "hidden", + }); + const secondAnchor = message({ + body: "Let's split this into another task.", + createdAt: 4, + id: "second-anchor", + pubkey: "agent", + }); + const laterReply = message({ + body: "This should also be hidden.", + createdAt: 5, + id: "later", + }); + + const firstMarker = parseAgentConversationMarker( + markerEvent({ content: { startedAt: 2 }, createdAt: 2 }), + ); + const secondMarker = parseAgentConversationMarker({ + ...markerEvent({ + content: { agentReplyId: "second-anchor", startedAt: 4 }, + createdAt: 4, + id: "second-marker", + }), + tags: [ + ["h", "channel"], + ["e", "root", "", "root"], + ["e", "second-anchor", "", "agent-reply"], + ["p", "agent"], + ["title", "Second task"], + ], + }); + + const hiddenIds = getHiddenAgentConversationMessageIds( + [root, firstAnchor, hiddenReply, secondAnchor, laterReply], + [firstMarker, secondMarker].filter(Boolean), + ); + + assert.deepEqual([...hiddenIds], ["hidden", "later"]); +}); diff --git a/desktop/src/features/agents/agentConversations.ts b/desktop/src/features/agents/agentConversations.ts index cf93fed4ad..94fe37e2e0 100644 --- a/desktop/src/features/agents/agentConversations.ts +++ b/desktop/src/features/agents/agentConversations.ts @@ -626,26 +626,42 @@ export function getHiddenAgentConversationMessageIds( const messageIndexById = new Map( orderedMessages.map(({ message }, index) => [message.id, index]), ); + const messageById = new Map( + orderedMessages.map(({ message }) => [message.id, message]), + ); + const anchorMessageIdsByThreadRootId = new Map>(); const cutoffByThreadRootId = new Map< string, { - anchorIndex: number | null; - anchorMessageId: string; + anchorIndex: number; startedAt: number; } >(); for (const marker of markers) { + const anchorMessage = messageById.get(marker.agentReplyId); + const anchorIndex = messageIndexById.get(marker.agentReplyId); + if (!anchorMessage || anchorIndex === undefined) { + continue; + } + + const anchorThreadRootId = + anchorMessage.rootId ?? anchorMessage.parentId ?? anchorMessage.id; + if (anchorThreadRootId !== marker.threadRootId) { + continue; + } + + const anchorMessageIds = + anchorMessageIdsByThreadRootId.get(marker.threadRootId) ?? new Set(); + anchorMessageIds.add(marker.agentReplyId); + anchorMessageIdsByThreadRootId.set(marker.threadRootId, anchorMessageIds); + const current = cutoffByThreadRootId.get(marker.threadRootId); const candidate = { - anchorIndex: messageIndexById.get(marker.agentReplyId) ?? null, - anchorMessageId: marker.agentReplyId, + anchorIndex, startedAt: marker.startedAt, }; const isEarlier = - current === undefined || - (candidate.anchorIndex !== null && current.anchorIndex !== null - ? candidate.anchorIndex < current.anchorIndex - : candidate.startedAt < current.startedAt); + current === undefined || candidate.anchorIndex < current.anchorIndex; if (isEarlier) { cutoffByThreadRootId.set(marker.threadRootId, candidate); } @@ -659,12 +675,15 @@ export function getHiddenAgentConversationMessageIds( } const cutoff = cutoffByThreadRootId.get(threadRootId); - if (cutoff === undefined || message.id === cutoff.anchorMessageId) { + if ( + cutoff === undefined || + anchorMessageIdsByThreadRootId.get(threadRootId)?.has(message.id) + ) { continue; } const messageIndex = messageIndexById.get(message.id); - if (cutoff.anchorIndex !== null && messageIndex !== undefined) { + if (messageIndex !== undefined) { if (messageIndex > cutoff.anchorIndex) { hiddenIds.add(message.id); } diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 7bcc0f1d67..08bc59d153 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -242,17 +242,6 @@ export const ChannelPane = React.memo(function ChannelPane({ return true; }, [findLastOwnEditable, messages, onEdit]); - const handleEditLastOwnThreadMessage = React.useCallback((): boolean => { - if (!onEdit) return false; - const scope: TimelineMessage[] = []; - if (threadHeadMessage) scope.push(threadHeadMessage); - for (const entry of threadMessages) scope.push(entry.message); - const target = findLastOwnEditable(scope); - if (!target) return false; - onEdit(target); - return true; - }, [findLastOwnEditable, onEdit, threadHeadMessage, threadMessages]); - const isComposerDisabled = !activeChannel?.isMember || activeChannel.archivedAt !== null || diff --git a/desktop/src/features/messages/lib/timelineItems.test.mjs b/desktop/src/features/messages/lib/timelineItems.test.mjs index 58ea21722b..c8566382d1 100644 --- a/desktop/src/features/messages/lib/timelineItems.test.mjs +++ b/desktop/src/features/messages/lib/timelineItems.test.mjs @@ -86,6 +86,24 @@ test("buildTimelineItems: system messages flatten to a 'system' item", () => { assert.deepEqual(kinds(items), ["day-divider", "message", "system"]); }); +test("buildTimelineItems: can omit only the initial day divider", () => { + const entries = [ + entry({ id: "d1a", createdAt: dayAt(2026, 6, 13) }), + entry({ id: "d1b", createdAt: dayAt(2026, 6, 13, 13) }), + entry({ id: "d2a", createdAt: dayAt(2026, 6, 14) }), + ]; + const { items } = buildTimelineItems(entries, null, { + showInitialDayDivider: false, + }); + + assert.deepEqual(kinds(items), [ + "message", + "message", + "day-divider", + "message", + ]); +}); + test("buildTimelineItems: empty entries produce no items", () => { const { items } = buildTimelineItems([], null); assert.equal(items.length, 0); diff --git a/desktop/src/features/messages/lib/timelineItems.ts b/desktop/src/features/messages/lib/timelineItems.ts index bb20ac2163..0bdd92f75e 100644 --- a/desktop/src/features/messages/lib/timelineItems.ts +++ b/desktop/src/features/messages/lib/timelineItems.ts @@ -30,6 +30,10 @@ export type TimelineItemsResult = { items: TimelineItem[]; }; +type BuildTimelineItemsOptions = { + showInitialDayDivider?: boolean; +}; + /** Stable per-item key, unique across the flattened stream. */ export function getTimelineItemKey(item: TimelineItem): string { return item.key; @@ -47,6 +51,7 @@ function entryRenderKey(entry: MainTimelineEntry): string { export function buildTimelineItems( entries: MainTimelineEntry[], firstUnreadMessageId: string | null, + { showInitialDayDivider = true }: BuildTimelineItemsOptions = {}, ): TimelineItemsResult { const items: TimelineItem[] = []; @@ -66,7 +71,7 @@ export function buildTimelineItems( const renderKey = entryRenderKey(entry); const dayBoundary = dayBoundariesByStartIndex.get(i); - if (dayBoundary) { + if (dayBoundary && (showInitialDayDivider || i !== 0)) { items.push({ kind: "day-divider", key: dayBoundary.key, diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 7c742079f0..3e120638a9 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -94,6 +94,7 @@ type MessageTimelineProps = { searchMatchingMessageIds?: Set; /** The current find-in-channel query string. */ searchQuery?: string; + showInitialDayDivider?: boolean; targetMessageId?: string | null; onTargetReached?: (messageId: string) => void; /** Event id of the oldest unread top-level message at channel open, or null. */ @@ -189,6 +190,7 @@ const MessageTimelineBase = React.forwardRef< searchActiveMessageId = null, searchMatchingMessageIds, searchQuery, + showInitialDayDivider = true, targetMessageId = null, onTargetReached, firstUnreadMessageId = null, @@ -239,7 +241,7 @@ const MessageTimelineBase = React.forwardRef< // scrollTop into the current scroll element during navigation; reusing the // same node across channel routes can leave the newly-loaded message list // painted at a stale offset until the user's next scroll event forces layout. - const scrollContainerDomKey = channelId ?? "none"; + const scrollContainerDomKey = `${channelId ?? "none"}:${layoutShiftKey ?? "none"}`; const timelineBodySurface = selectTimelineBodySurface({ deferredCount: deferredMessages.length, @@ -634,6 +636,7 @@ const MessageTimelineBase = React.forwardRef< searchActiveMessageId={searchActiveMessageId} searchMatchingMessageIds={searchMatchingMessageIds} searchQuery={searchQuery} + showInitialDayDivider={showInitialDayDivider} threadUnreadCounts={threadUnreadCounts} unfollowThreadById={unfollowThreadById} /> diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index e71ed8e868..baf0dc0e38 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -78,6 +78,7 @@ type TimelineMessageListProps = { searchMatchingMessageIds?: Set; /** The current find-in-channel query string. */ searchQuery?: string; + showInitialDayDivider?: boolean; /** Per-thread unread counts keyed by thread root id. */ threadUnreadCounts?: ReadonlyMap; }; @@ -112,6 +113,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ searchActiveMessageId = null, searchMatchingMessageIds, searchQuery, + showInitialDayDivider = true, threadUnreadCounts, unfollowThreadById, }: TimelineMessageListProps) { @@ -168,8 +170,11 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ // The flattened item stream, memoized on the entries and the unread boundary // (the unread divider is its own item, so it shifts subsequent rows). const itemsResult = React.useMemo( - () => buildTimelineItems(entries, firstUnreadMessageId), - [entries, firstUnreadMessageId], + () => + buildTimelineItems(entries, firstUnreadMessageId, { + showInitialDayDivider, + }), + [entries, firstUnreadMessageId, showInitialDayDivider], ); const agentConversationMarkerByMessageId = React.useMemo( () => diff --git a/desktop/src/features/sidebar/ui/AppSidebar.tsx b/desktop/src/features/sidebar/ui/AppSidebar.tsx index 7e08ffc59a..51f4b4e492 100644 --- a/desktop/src/features/sidebar/ui/AppSidebar.tsx +++ b/desktop/src/features/sidebar/ui/AppSidebar.tsx @@ -493,45 +493,8 @@ export function AppSidebar({ return byChannelId; }, [agentConversations]); const isAgentConversationActive = selectedView === "agents"; - const activeAgentConversationChannelId = React.useMemo(() => { - if (!isAgentConversationActive || !selectedAgentConversationId) { - return null; - } - - return ( - agentConversations.find( - (conversation) => conversation.id === selectedAgentConversationId, - )?.channelId ?? null - ); - }, [ - agentConversations, - isAgentConversationActive, - selectedAgentConversationId, - ]); - const displayUnreadChannelIds = React.useMemo(() => { - if ( - !activeAgentConversationChannelId || - !unreadChannelIds.has(activeAgentConversationChannelId) - ) { - return unreadChannelIds; - } - - const next = new Set(unreadChannelIds); - next.delete(activeAgentConversationChannelId); - return next; - }, [activeAgentConversationChannelId, unreadChannelIds]); - const displayUnreadChannelCounts = React.useMemo(() => { - if ( - !activeAgentConversationChannelId || - !unreadChannelCounts.has(activeAgentConversationChannelId) - ) { - return unreadChannelCounts; - } - - const next = new Map(unreadChannelCounts); - next.delete(activeAgentConversationChannelId); - return next; - }, [activeAgentConversationChannelId, unreadChannelCounts]); + const displayUnreadChannelIds = unreadChannelIds; + const displayUnreadChannelCounts = unreadChannelCounts; const sidebarLoadingShape = useSidebarLoadingShape({ activeWorkspaceId: activeWorkspace?.id, currentPubkey,