Skip to content

Commit c0f144b

Browse files
authored
fix: preserve parent thread anchors on partial list refresh (#494)
1 parent e7a8ad3 commit c0f144b

7 files changed

Lines changed: 356 additions & 2 deletions

File tree

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ Use project aliases for frontend imports:
7676

7777
For broader path maps, use `docs/codebase-map.md`.
7878

79+
## Thread Hierarchy Invariants
80+
81+
- `setThreads` reconciliation must preserve incoming order while retaining required local anchors (active/processing/ancestor summaries) when payloads are partial.
82+
- Never resurrect hidden threads during reconciliation (`hiddenThreadIdsByWorkspace` still wins).
83+
- `useThreadRows` renders children under parents only when parent summaries are present in the visible list; missing parent summaries will promote children to roots.
84+
7985
## Follow-up Behavior Map
8086

8187
For Queue vs Steer follow-up behavior, start here:

src/features/threads/hooks/threadReducer/threadLifecycleSlice.ts

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,11 +312,66 @@ export function reduceThreadLifecycle(
312312
case "setThreads": {
313313
const hidden = state.hiddenThreadIdsByWorkspace[action.workspaceId] ?? {};
314314
const visibleThreads = action.threads.filter((thread) => !hidden[thread.id]);
315+
const existingThreads = state.threadsByWorkspace[action.workspaceId] ?? [];
316+
const existingById = new Map(
317+
existingThreads.map((thread) => [thread.id, thread] as const),
318+
);
319+
const reconciled = [...visibleThreads];
320+
const includedIds = new Set(reconciled.map((thread) => thread.id));
321+
const freshenAnchorSummary = (summary: ThreadSummary) => {
322+
const lastMessageTimestamp =
323+
state.lastAgentMessageByThread[summary.id]?.timestamp ?? 0;
324+
const processingStartedAt =
325+
state.threadStatusById[summary.id]?.processingStartedAt ?? 0;
326+
const nextUpdatedAt = Math.max(
327+
summary.updatedAt ?? 0,
328+
lastMessageTimestamp,
329+
processingStartedAt,
330+
);
331+
if (nextUpdatedAt <= (summary.updatedAt ?? 0)) {
332+
return summary;
333+
}
334+
return {
335+
...summary,
336+
updatedAt: nextUpdatedAt,
337+
};
338+
};
339+
const appendExistingAnchor = (threadId: string | null | undefined) => {
340+
if (!threadId || hidden[threadId] || includedIds.has(threadId)) {
341+
return;
342+
}
343+
const summary = existingById.get(threadId);
344+
if (!summary) {
345+
return;
346+
}
347+
reconciled.push(freshenAnchorSummary(summary));
348+
includedIds.add(threadId);
349+
};
350+
351+
const activeThreadId = state.activeThreadIdByWorkspace[action.workspaceId];
352+
appendExistingAnchor(activeThreadId);
353+
existingThreads.forEach((thread) => {
354+
if (state.threadStatusById[thread.id]?.isProcessing) {
355+
appendExistingAnchor(thread.id);
356+
}
357+
});
358+
359+
const seedThreadIds = [...includedIds];
360+
seedThreadIds.forEach((threadId) => {
361+
const visited = new Set<string>([threadId]);
362+
let parentId = state.threadParentById[threadId];
363+
while (parentId && !visited.has(parentId)) {
364+
visited.add(parentId);
365+
appendExistingAnchor(parentId);
366+
parentId = state.threadParentById[parentId];
367+
}
368+
});
369+
315370
return {
316371
...state,
317372
threadsByWorkspace: {
318373
...state.threadsByWorkspace,
319-
[action.workspaceId]: visibleThreads,
374+
[action.workspaceId]: reconciled,
320375
},
321376
threadSortKeyByWorkspace: {
322377
...state.threadSortKeyByWorkspace,

src/features/threads/hooks/useThreadActions.test.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ describe("useThreadActions", () => {
8181
threadsByWorkspace: {},
8282
activeThreadIdByWorkspace: {},
8383
activeTurnIdByThread: {},
84+
threadParentById: {},
8485
threadListCursorByWorkspace: {},
8586
threadStatusById: {},
8687
threadSortKey: "updated_at",
@@ -755,6 +756,50 @@ describe("useThreadActions", () => {
755756
});
756757
});
757758

759+
it("uses fresh fetched data for active anchors outside top thread target", async () => {
760+
const data = Array.from({ length: 21 }, (_, index) => ({
761+
id: `thread-${index + 1}`,
762+
cwd: workspace.path,
763+
preview: `Thread ${index + 1} fresh`,
764+
updated_at: 5000 - index,
765+
}));
766+
vi.mocked(listThreads).mockResolvedValue({
767+
result: {
768+
data,
769+
nextCursor: null,
770+
},
771+
});
772+
vi.mocked(getThreadTimestamp).mockImplementation((thread) => {
773+
const value = (thread as Record<string, unknown>).updated_at as number;
774+
return value ?? 0;
775+
});
776+
777+
const { result, dispatch } = renderActions({
778+
threadsByWorkspace: {
779+
"ws-1": [{ id: "thread-21", name: "Thread 21 stale", updatedAt: 10 }],
780+
},
781+
activeThreadIdByWorkspace: { "ws-1": "thread-21" },
782+
});
783+
784+
await act(async () => {
785+
await result.current.listThreadsForWorkspace(workspace);
786+
});
787+
788+
const setThreadsAction = dispatch.mock.calls
789+
.map(([action]) => action)
790+
.find(
791+
(action) => action.type === "setThreads" && action.workspaceId === "ws-1",
792+
);
793+
expect(setThreadsAction).toBeTruthy();
794+
if (!setThreadsAction || setThreadsAction.type !== "setThreads") {
795+
return;
796+
}
797+
expect(setThreadsAction.threads).toHaveLength(21);
798+
expect(setThreadsAction.threads[20]?.id).toBe("thread-21");
799+
expect(setThreadsAction.threads[20]?.name).toBe("Thread 21 fresh");
800+
expect(setThreadsAction.threads[20]?.updatedAt).toBe(4980);
801+
});
802+
758803
it("lists threads once and distributes results across workspaces", async () => {
759804
vi.mocked(listThreads).mockResolvedValue({
760805
result: {

src/features/threads/hooks/useThreadActions.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ type UseThreadActionsOptions = {
5656
threadsByWorkspace: ThreadState["threadsByWorkspace"];
5757
activeThreadIdByWorkspace: ThreadState["activeThreadIdByWorkspace"];
5858
activeTurnIdByThread: ThreadState["activeTurnIdByThread"];
59+
threadParentById: ThreadState["threadParentById"];
5960
threadListCursorByWorkspace: ThreadState["threadListCursorByWorkspace"];
6061
threadStatusById: ThreadState["threadStatusById"];
6162
threadSortKey: ThreadListSortKey;
@@ -84,6 +85,7 @@ export function useThreadActions({
8485
threadsByWorkspace,
8586
activeThreadIdByWorkspace,
8687
activeTurnIdByThread,
88+
threadParentById,
8789
threadListCursorByWorkspace,
8890
threadStatusById,
8991
threadSortKey,
@@ -644,10 +646,54 @@ export function useThreadActions({
644646
return aId.localeCompare(bId);
645647
});
646648
}
649+
const summaryById = new Map<string, ThreadSummary>();
650+
uniqueThreads.forEach((thread, index) => {
651+
const summary = buildThreadSummary(workspace.id, thread, index);
652+
if (!summary) {
653+
return;
654+
}
655+
summaryById.set(summary.id, summary);
656+
});
647657
const summaries = uniqueThreads
648658
.slice(0, THREAD_LIST_TARGET_COUNT)
649-
.map((thread, index) => buildThreadSummary(workspace.id, thread, index))
659+
.map((thread) => summaryById.get(String(thread?.id ?? "")) ?? null)
650660
.filter((entry): entry is ThreadSummary => Boolean(entry));
661+
const includedIds = new Set(summaries.map((thread) => thread.id));
662+
const appendFreshAnchor = (threadId: string | null | undefined) => {
663+
if (!threadId || includedIds.has(threadId)) {
664+
return;
665+
}
666+
const summary = summaryById.get(threadId);
667+
if (!summary) {
668+
return;
669+
}
670+
summaries.push(summary);
671+
includedIds.add(threadId);
672+
};
673+
appendFreshAnchor(activeThreadIdByWorkspace[workspace.id]);
674+
const workspaceThreadIds = new Set<string>([
675+
...Array.from(summaryById.keys()),
676+
...(threadsByWorkspace[workspace.id] ?? []).map((thread) => thread.id),
677+
]);
678+
const activeThreadId = activeThreadIdByWorkspace[workspace.id];
679+
if (activeThreadId) {
680+
workspaceThreadIds.add(activeThreadId);
681+
}
682+
workspaceThreadIds.forEach((threadId) => {
683+
if (threadStatusById[threadId]?.isProcessing) {
684+
appendFreshAnchor(threadId);
685+
}
686+
});
687+
const seedThreadIds = [...includedIds];
688+
seedThreadIds.forEach((threadId) => {
689+
const visited = new Set<string>([threadId]);
690+
let parentId = threadParentById[threadId];
691+
while (parentId && !visited.has(parentId)) {
692+
visited.add(parentId);
693+
appendFreshAnchor(parentId);
694+
parentId = threadParentById[parentId];
695+
}
696+
});
651697
dispatch({
652698
type: "setThreads",
653699
workspaceId: workspace.id,
@@ -703,8 +749,12 @@ export function useThreadActions({
703749
onDebug,
704750
onSubagentThreadDetected,
705751
onThreadCodexMetadataDetected,
752+
activeThreadIdByWorkspace,
753+
threadParentById,
706754
threadActivityRef,
755+
threadStatusById,
707756
threadSortKey,
757+
threadsByWorkspace,
708758
updateThreadParent,
709759
],
710760
);

src/features/threads/hooks/useThreads.integration.test.tsx

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,4 +1683,96 @@ describe("useThreads UX integration", () => {
16831683
]);
16841684
expect(unpinnedRows.map((row) => row.thread.id)).toEqual(["thread-b"]);
16851685
});
1686+
1687+
it("keeps parent rows anchored when refresh only returns subagent children", async () => {
1688+
vi.mocked(listThreads)
1689+
.mockResolvedValueOnce({
1690+
result: {
1691+
data: [
1692+
{
1693+
id: "thread-parent-anchor",
1694+
preview: "Parent",
1695+
updated_at: 2000,
1696+
cwd: workspace.path,
1697+
},
1698+
{
1699+
id: "thread-child-anchor",
1700+
preview: "Child",
1701+
updated_at: 3000,
1702+
cwd: workspace.path,
1703+
source: {
1704+
subAgent: {
1705+
thread_spawn: {
1706+
parent_thread_id: "thread-parent-anchor",
1707+
depth: 1,
1708+
},
1709+
},
1710+
},
1711+
},
1712+
],
1713+
nextCursor: null,
1714+
},
1715+
})
1716+
.mockResolvedValueOnce({
1717+
result: {
1718+
data: [
1719+
{
1720+
id: "thread-child-anchor",
1721+
preview: "Child",
1722+
updated_at: 3500,
1723+
cwd: workspace.path,
1724+
source: {
1725+
subAgent: {
1726+
thread_spawn: {
1727+
parent_thread_id: "thread-parent-anchor",
1728+
depth: 1,
1729+
},
1730+
},
1731+
},
1732+
},
1733+
],
1734+
nextCursor: null,
1735+
},
1736+
});
1737+
1738+
const { result } = renderHook(() =>
1739+
useThreads({
1740+
activeWorkspace: workspace,
1741+
onWorkspaceConnected: vi.fn(),
1742+
}),
1743+
);
1744+
1745+
await act(async () => {
1746+
await result.current.listThreadsForWorkspace(workspace);
1747+
});
1748+
1749+
await waitFor(() => {
1750+
expect(result.current.threadParentById["thread-child-anchor"]).toBe(
1751+
"thread-parent-anchor",
1752+
);
1753+
});
1754+
1755+
await act(async () => {
1756+
await result.current.listThreadsForWorkspace(workspace);
1757+
});
1758+
1759+
expect(vi.mocked(listThreads)).toHaveBeenCalledTimes(2);
1760+
expect(result.current.threadsByWorkspace["ws-1"]?.map((thread) => thread.id)).toEqual(
1761+
["thread-child-anchor", "thread-parent-anchor"],
1762+
);
1763+
1764+
const { result: threadRowsResult } = renderHook(() =>
1765+
useThreadRows(result.current.threadParentById),
1766+
);
1767+
const rows = threadRowsResult.current.getThreadRows(
1768+
result.current.threadsByWorkspace["ws-1"] ?? [],
1769+
true,
1770+
"ws-1",
1771+
() => null,
1772+
);
1773+
expect(rows.unpinnedRows.map((row) => [row.thread.id, row.depth])).toEqual([
1774+
["thread-parent-anchor", 0],
1775+
["thread-child-anchor", 1],
1776+
]);
1777+
});
16861778
});

src/features/threads/hooks/useThreads.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,7 @@ export function useThreads({
548548
threadsByWorkspace: state.threadsByWorkspace,
549549
activeThreadIdByWorkspace: state.activeThreadIdByWorkspace,
550550
activeTurnIdByThread: state.activeTurnIdByThread,
551+
threadParentById: state.threadParentById,
551552
threadListCursorByWorkspace: state.threadListCursorByWorkspace,
552553
threadStatusById: state.threadStatusById,
553554
threadSortKey,

0 commit comments

Comments
 (0)