perf(issues): stop full timeline re-render on every WS event (#2329)

* perf(issues): stop full timeline re-render on every WS event (MUL-1941)

Two compounding causes made every Comment/reply WS event re-render every
sibling thread on the issue detail page — visible during AI streaming as
a flash across all 10 nested replies under a parent and as the green
reply-input losing its draft.

1) `useCreateComment.onSettled` invalidated the timeline query, forcing a
   full `GET /timeline` refetch on every comment submit. The response
   replaced every entry's reference even when the content was unchanged,
   poisoning every downstream React.memo. The `comment:created` WS
   broadcast already keeps the cache fresh and `useWSReconnect` invalidates
   on disconnect, so the redundant refetch had no upside. Drop it.

2) The `timelineView` useMemo passed the full `repliesByParent: Map` to
   every CommentCard. Each WS event rebuilt the Map (new ref), so React.memo
   on CommentCard fell back to a re-render for *every* card, not just the
   one whose thread changed. Replace the Map prop with a per-thread
   `replies: TimelineEntry[]` slice, precomputed once via
   `collectThreadReplies` and stabilized against the prior render — when a
   thread's flat list is shallow-equal to last time, reuse the previous
   array reference so unrelated cards keep their memo.

ResolvedThreadBar gets the same `replies` prop, so the collapsed count +
author list still match the expanded view without re-walking the graph.

Verified: pnpm typecheck + pnpm test for @multica/views and @multica/core
(334 + 214 tests, all passing).

Co-authored-by: multica-agent <github@multica.ai>

* fix(realtime): mark timeline stale without refetching active queries (MUL-1941)

Per GPT-Boy's review on PR #2329: dropping `useCreateComment.onSettled`'s
invalidate wasn't enough. The global `useRealtimeSync` runs in WSProvider
for the lifetime of the app and re-invalidates the timeline on every
`comment:created` / `comment:updated` / `comment:deleted` /
`comment:resolved` / `comment:unresolved` / `activity:created` /
`reaction:added` / `reaction:removed` event. With `staleTime: Infinity` on
the QueryClient default, the active timeline query refetches on every
invalidate — replacing every entry's reference and busting the per-thread
memoization the prior commit just put in place.

Switch the global handler's `invalidateQueries` to `refetchType: "none"`.
Active observers now stay fresh via the granular `setQueryData` handlers
in `useIssueTimeline`; inactive issues' caches are still marked stale, so
when IssueDetail mounts later, `refetchOnMount` triggers a fresh fetch
the same way it did before.

`comment:resolved` / `comment:unresolved` previously had no granular
handler — only the global invalidate kept the cache in sync. Add
useWSEvent handlers in `useIssueTimeline` that replace the matching
entry via `commentToTimelineEntry`, and extend that helper to carry the
resolved_at / resolved_by_type / resolved_by_id fields so resolved state
survives the round-trip (it was silently dropped on every
`comment:updated` too — fixed as a side effect).

Tests: 3 new cases covering resolved / unresolved / cross-issue isolation
in the timeline hook. All 337 + 214 unit tests + full monorepo typecheck
pass.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Bohan Jiang
2026-05-09 17:20:08 +08:00
committed by GitHub
parent d713b57072
commit 807201086c
7 changed files with 302 additions and 30 deletions

View File

@@ -336,9 +336,13 @@ export function useCreateComment(issueId: string) {
return [...old, entry];
});
},
onSettled: () => {
qc.invalidateQueries({ queryKey: issueKeys.timeline(issueId) });
},
// No onSettled invalidate. The `comment:created` WS broadcast keeps
// the timeline cache fresh after a successful create, and reconnect
// recovery in useIssueTimeline already invalidates if the connection
// dropped. Re-fetching on every submit replaces every entry's
// reference, which forces every memoized CommentCard subtree to
// re-render (visible as a flash across sibling threads during AI
// streaming).
});
}

View File

@@ -332,12 +332,25 @@ export function useRealtimeSync(
// --- Timeline event handlers (global fallback) ---
// These events are also handled granularly by useIssueTimeline when
// IssueDetail is mounted. This global handler ensures the timeline cache
// is invalidated even when IssueDetail is unmounted, so stale data
// isn't served on next mount (staleTime: Infinity relies on this).
// IssueDetail is mounted. This global handler exists to mark the
// timeline cache stale for issues whose IssueDetail is *not* mounted,
// so stale data isn't served on next mount (staleTime: Infinity, set on
// the QueryClient default, relies on this).
//
// `refetchType: "none"` is the load-bearing detail: without it, an
// active IssueDetail observer would refetch the entire timeline on
// every comment / activity / reaction event. The refetch replaces
// every entry's reference and busts React.memo on every CommentCard
// subtree (visible during AI streaming as a flash across all sibling
// threads, MUL-1941). Inactive observers don't refetch either way;
// when IssueDetail mounts later, the stale flag triggers the refetch
// through `refetchOnMount`. Active observers stay fresh via the
// granular setQueryData handlers in `useIssueTimeline`.
const invalidateTimeline = (issueId: string) => {
qc.invalidateQueries({ queryKey: issueKeys.timeline(issueId) });
qc.invalidateQueries({
queryKey: issueKeys.timeline(issueId),
refetchType: "none",
});
};
const unsubCommentCreated = ws.on("comment:created", (p) => {

View File

@@ -35,7 +35,6 @@ import { FileUploadButton } from "@multica/ui/components/common/file-upload-butt
import { useFileUpload } from "@multica/core/hooks/use-file-upload";
import { api } from "@multica/core/api";
import { ReplyInput } from "./reply-input";
import { collectThreadReplies } from "./thread-utils";
import type { TimelineEntry, Attachment } from "@multica/core/types";
import { useCommentCollapseStore } from "@multica/core/issues/stores";
import { useT } from "../../i18n";
@@ -47,7 +46,14 @@ import { useT } from "../../i18n";
interface CommentCardProps {
issueId: string;
entry: TimelineEntry;
allReplies: Map<string, TimelineEntry[]>;
/**
* Flat list of every nested reply under this thread root, in render order.
* Computed once in `issue-detail.tsx`'s `timelineView` and stabilized so
* the array reference only changes when *this* thread's replies change —
* an unrelated thread receiving a new reply must NOT bust this card's
* memo. Passing the full Map here used to do exactly that.
*/
replies: TimelineEntry[];
currentUserId?: string;
/**
* True when the current user is a workspace owner/admin and can therefore
@@ -363,7 +369,7 @@ function CommentRow({
function CommentCardImpl({
issueId,
entry,
allReplies,
replies,
currentUserId,
canModerate = false,
onReply,
@@ -427,10 +433,10 @@ function CommentCardImpl({
}
};
// Collect all nested replies recursively into a flat list. Helper is
// shared with ResolvedThreadBar so the collapsed count matches what the
// expanded card renders.
const allNestedReplies = collectThreadReplies(entry.id, allReplies);
// The parent precomputes the flat thread (using collectThreadReplies),
// memoizes by thread, and stabilizes the array reference, so we render
// straight from `replies` instead of re-walking the graph on every render.
const allNestedReplies = replies;
const replyCount = allNestedReplies.length;
const contentPreview = (entry.content ?? "").replace(/\n/g, " ").slice(0, 80);

View File

@@ -45,6 +45,7 @@ import { ProjectPicker } from "../../projects/components/project-picker";
import { CommentCard } from "./comment-card";
import { CommentInput } from "./comment-input";
import { ResolvedThreadBar } from "./resolved-thread-bar";
import { collectThreadReplies } from "./thread-utils";
import { AgentLiveCard } from "./agent-live-card";
import { ExecutionLogSection } from "./execution-log-section";
import { useQuery } from "@tanstack/react-query";
@@ -152,6 +153,23 @@ function formatTokenCount(n: number): string {
return String(n);
}
// Stable reference for threads with no replies. Inline `[]` would create a
// new array on every render and bust React.memo on CommentCard / ResolvedThreadBar.
const EMPTY_REPLIES: TimelineEntry[] = [];
// Shallow array equality by element identity. Used to reuse the previous
// render's per-thread reply slice when nothing in *this* thread changed,
// even if the surrounding `timeline` array was rebuilt by a WS event in
// some unrelated thread.
function shallowEqualEntries(a: TimelineEntry[], b: TimelineEntry[]): boolean {
if (a === b) return true;
if (a.length !== b.length) return false;
for (let i = 0; i < a.length; i++) {
if (a[i] !== b[i]) return false;
}
return true;
}
function TimelineSkeleton() {
return (
<div className="mt-4 flex flex-col gap-3">
@@ -315,10 +333,16 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
[clearResolvedExpand, toggleResolveComment],
);
// Memoized timeline grouping. The same Map / groups references are reused
// across re-renders that don't change `timeline`, so React.memo on
// CommentCard can skip re-rendering when the only thing that moved was
// unrelated parent state (e.g. composer draft, sidebar toggle).
// Memoized timeline grouping. Each render rebuilds the per-parent map from
// the latest timeline, then pre-flattens each thread's reply subtree into a
// dedicated `threadReplies` slice per root. Slices are stabilized against
// the previous render via `prevThreadRepliesRef`: if a thread's flat list
// is shallow-equal to the previous one, we reuse the previous array so
// React.memo on CommentCard / ResolvedThreadBar can short-circuit. Without
// this, every WS event (including reactions, edits, AI streaming on an
// unrelated thread) hands every card a brand-new prop reference and forces
// every thread subtree to re-render in lockstep.
const prevThreadRepliesRef = useRef<Map<string, TimelineEntry[]>>(new Map());
const timelineView = useMemo(() => {
// Group entries: top-level = activities + root comments; replies are
// bucketed under their parent's id and rendered nested inside CommentCard.
@@ -336,6 +360,22 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
}
}
// Pre-flatten each top-level comment's thread subtree (parent + every
// descendant in render order). Reuse the previous array reference when
// the thread is unchanged so unrelated CommentCards keep their memo.
const prevThreadReplies = prevThreadRepliesRef.current;
const threadReplies = new Map<string, TimelineEntry[]>();
for (const root of topLevel) {
if (root.type !== "comment") continue;
const fresh = collectThreadReplies(root.id, repliesByParent);
const previous = prevThreadReplies.get(root.id);
threadReplies.set(
root.id,
previous && shallowEqualEntries(previous, fresh) ? previous : fresh,
);
}
prevThreadRepliesRef.current = threadReplies;
// Coalesce consecutive activities from the same actor + action.
// - task_completed / task_failed: no time limit (these repeat across runs)
// - all other actions: within a 2-minute window
@@ -375,7 +415,7 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
}
}
return { repliesByParent, groups };
return { threadReplies, groups };
}, [timeline]);
const {
@@ -1039,7 +1079,7 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
<div key={entry.id} id={`comment-${entry.id}`}>
<ResolvedThreadBar
entry={entry}
repliesByParent={timelineView.repliesByParent}
replies={timelineView.threadReplies.get(entry.id) ?? EMPTY_REPLIES}
onExpand={() => toggleResolvedExpand(entry.id, true)}
/>
</div>
@@ -1050,7 +1090,7 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
<CommentCard
issueId={id}
entry={entry}
allReplies={timelineView.repliesByParent}
replies={timelineView.threadReplies.get(entry.id) ?? EMPTY_REPLIES}
currentUserId={user?.id}
canModerate={canModerateComments}
onReply={submitReply}

View File

@@ -2,29 +2,27 @@ import { CheckCircle2, ChevronRight } from "lucide-react";
import { useActorName } from "@multica/core/workspace/hooks";
import { Card } from "@multica/ui/components/ui/card";
import type { TimelineEntry } from "@multica/core/types";
import { collectThreadReplies } from "./thread-utils";
import { useT } from "../../i18n";
interface ResolvedThreadBarProps {
/** The resolved root comment. */
entry: TimelineEntry;
/**
* Full reply graph keyed by parent_id. The bar walks the graph recursively
* so the count + author list match what CommentCard would render in the
* expanded view (direct-children-only would undercount nested replies).
* Flat list of every nested reply under this thread root. Precomputed by
* `issue-detail.tsx`'s `timelineView` from the same walk that CommentCard
* uses, so the count + author list match what the expanded view renders
* (direct-children-only would undercount nested replies).
*/
repliesByParent: Map<string, TimelineEntry[]>;
replies: TimelineEntry[];
onExpand: () => void;
}
const MAX_NAMED_AUTHORS = 2;
export function ResolvedThreadBar({ entry, repliesByParent, onExpand }: ResolvedThreadBarProps) {
export function ResolvedThreadBar({ entry, replies, onExpand }: ResolvedThreadBarProps) {
const { t } = useT("issues");
const { getActorName } = useActorName();
const replies = collectThreadReplies(entry.id, repliesByParent);
const authorKeys = new Set<string>();
const authors: Array<{ type: string; id: string }> = [];
for (const e of [entry, ...replies]) {

View File

@@ -201,4 +201,172 @@ describe("useIssueTimeline", () => {
// setQueryData should not have been invoked for a non-matching issue.
expect(cacheUpdates.last).toBeNull();
});
// The global useRealtimeSync handler now uses refetchType: "none" for
// timeline events, which means useIssueTimeline must own the granular
// cache update for every event that mutates the timeline — including
// comment:resolved / comment:unresolved. Without these handlers the
// resolve toggle on a thread root would only update the cache when the
// user remounts IssueDetail (the stale flag triggers a refetch), so the
// bar/expanded view would lag the click by a navigation cycle.
it("comment:resolved updates the matching entry in place with the new resolved fields", () => {
queryState.data = [
{
type: "comment",
id: "c1",
actor_type: "member",
actor_id: "u",
content: "hello",
parent_id: null,
created_at: "2026-05-06T01:00:00Z",
updated_at: "2026-05-06T01:00:00Z",
reactions: [],
attachments: [],
resolved_at: null,
resolved_by_type: null,
resolved_by_id: null,
},
{
type: "comment",
id: "c2",
actor_type: "member",
actor_id: "u",
content: "untouched",
parent_id: null,
created_at: "2026-05-06T02:00:00Z",
updated_at: "2026-05-06T02:00:00Z",
reactions: [],
attachments: [],
resolved_at: null,
resolved_by_type: null,
resolved_by_id: null,
},
];
renderHook(() => useIssueTimeline("issue-1", "user-1"));
const handler = wsHandlers.get("comment:resolved");
expect(handler).toBeDefined();
act(() => {
handler!({
comment: {
id: "c1",
issue_id: "issue-1",
author_type: "member",
author_id: "u",
content: "hello",
parent_id: null,
created_at: "2026-05-06T01:00:00Z",
updated_at: "2026-05-06T01:00:00Z",
type: "comment",
reactions: [],
attachments: [],
resolved_at: "2026-05-06T03:00:00Z",
resolved_by_type: "member",
resolved_by_id: "u",
},
});
});
const updated = cacheUpdates.last as Array<{
id: string;
resolved_at: string | null;
resolved_by_type: string | null;
resolved_by_id: string | null;
}>;
expect(updated.map((e) => e.id)).toEqual(["c1", "c2"]);
expect(updated[0]!.resolved_at).toBe("2026-05-06T03:00:00Z");
expect(updated[0]!.resolved_by_type).toBe("member");
expect(updated[0]!.resolved_by_id).toBe("u");
// Sibling entry must not change (identity preserved by .map).
expect(updated[1]!.resolved_at).toBeNull();
});
it("comment:unresolved clears the resolved fields on the matching entry", () => {
queryState.data = [
{
type: "comment",
id: "c1",
actor_type: "member",
actor_id: "u",
content: "hello",
parent_id: null,
created_at: "2026-05-06T01:00:00Z",
updated_at: "2026-05-06T01:00:00Z",
reactions: [],
attachments: [],
resolved_at: "2026-05-06T03:00:00Z",
resolved_by_type: "member",
resolved_by_id: "u",
},
];
renderHook(() => useIssueTimeline("issue-1", "user-1"));
const handler = wsHandlers.get("comment:unresolved");
expect(handler).toBeDefined();
act(() => {
handler!({
comment: {
id: "c1",
issue_id: "issue-1",
author_type: "member",
author_id: "u",
content: "hello",
parent_id: null,
created_at: "2026-05-06T01:00:00Z",
updated_at: "2026-05-06T01:00:00Z",
type: "comment",
reactions: [],
attachments: [],
resolved_at: null,
resolved_by_type: null,
resolved_by_id: null,
},
});
});
const updated = cacheUpdates.last as Array<{
id: string;
resolved_at: string | null;
}>;
expect(updated[0]!.resolved_at).toBeNull();
});
it("comment:resolved ignores events from other issues", () => {
queryState.data = [
{
type: "comment",
id: "c1",
actor_type: "member",
actor_id: "u",
content: "hello",
parent_id: null,
created_at: "2026-05-06T01:00:00Z",
updated_at: "2026-05-06T01:00:00Z",
reactions: [],
attachments: [],
resolved_at: null,
resolved_by_type: null,
resolved_by_id: null,
},
];
renderHook(() => useIssueTimeline("issue-1", "user-1"));
const handler = wsHandlers.get("comment:resolved");
act(() => {
handler!({
comment: {
id: "c1",
issue_id: "different-issue",
author_type: "member",
author_id: "u",
content: "hello",
parent_id: null,
created_at: "2026-05-06T01:00:00Z",
updated_at: "2026-05-06T01:00:00Z",
type: "comment",
reactions: [],
attachments: [],
resolved_at: "2026-05-06T03:00:00Z",
resolved_by_type: "member",
resolved_by_id: "u",
},
});
});
expect(cacheUpdates.last).toBeNull();
});
});

View File

@@ -15,6 +15,8 @@ import type {
CommentCreatedPayload,
CommentUpdatedPayload,
CommentDeletedPayload,
CommentResolvedPayload,
CommentUnresolvedPayload,
ActivityCreatedPayload,
ReactionAddedPayload,
ReactionRemovedPayload,
@@ -50,6 +52,9 @@ function commentToTimelineEntry(c: Comment): TimelineEntry {
comment_type: c.type,
reactions: c.reactions ?? [],
attachments: c.attachments ?? [],
resolved_at: c.resolved_at,
resolved_by_type: c.resolved_by_type,
resolved_by_id: c.resolved_by_id,
};
}
@@ -118,6 +123,44 @@ export function useIssueTimeline(issueId: string, userId?: string) {
),
);
// Granular handlers for comment:resolved / comment:unresolved. The payload
// carries the full Comment with the new resolved_at/resolved_by_* fields,
// which `commentToTimelineEntry` already preserves, so the existing
// entry can simply be replaced in place. Without these handlers the only
// path that updated the cache was `useRealtimeSync`'s global invalidate,
// which forces a full timeline refetch and busts every CommentCard memo.
useWSEvent(
"comment:resolved",
useCallback(
(payload: unknown) => {
const { comment } = payload as CommentResolvedPayload;
if (comment.issue_id !== issueId) return;
qc.setQueryData<TLCache>(issueKeys.timeline(issueId), (old) =>
old?.map((e) =>
e.id === comment.id ? commentToTimelineEntry(comment) : e,
),
);
},
[qc, issueId],
),
);
useWSEvent(
"comment:unresolved",
useCallback(
(payload: unknown) => {
const { comment } = payload as CommentUnresolvedPayload;
if (comment.issue_id !== issueId) return;
qc.setQueryData<TLCache>(issueKeys.timeline(issueId), (old) =>
old?.map((e) =>
e.id === comment.id ? commentToTimelineEntry(comment) : e,
),
);
},
[qc, issueId],
),
);
useWSEvent(
"comment:deleted",
useCallback(