From c538cac7732c3faf377838e043b288ca926524d2 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Sun, 22 Feb 2026 13:35:22 -0500 Subject: [PATCH 1/6] feat: support fork/tree topology in handoff chain data model - Thread.handoffChildIds: string[] replaces singular handoffChildId (legacy field kept for backward compat, set to last child seen) - ThreadChain.descendantsTree: recursive ThreadChainNode[] for branching (flat descendants[] kept for UI compat) - ThreadStack.topology: adjacency maps (rootId, childToParent, parentToChildren) - Fix threadChain.ts: was reading thread.relationships which getThreads() never populates; now uses handoffParentId/handoffChildIds - All new fields are optional/additive; no UI changes needed Amp-Thread-ID: https://ampcode.com/threads/T-019c869d-7987-71df-99a5-e64a57114bee Co-authored-by: Amp --- server/lib/threadChain.ts | 121 ++++++++++++++++++++------------------ server/lib/threadCrud.ts | 10 ++-- shared/types.ts | 15 +++++ src/utils/threadStacks.ts | 34 ++++++++++- 4 files changed, 118 insertions(+), 62 deletions(-) diff --git a/server/lib/threadChain.ts b/server/lib/threadChain.ts index c8a3166..875d79e 100644 --- a/server/lib/threadChain.ts +++ b/server/lib/threadChain.ts @@ -1,79 +1,88 @@ -import type { Thread, ThreadChain, ChainThread, ThreadRelationship } from '../../shared/types.js'; +import type { Thread, ThreadChain, ChainThread, ThreadChainNode } from '../../shared/types.js'; import { runAmp, stripAnsi } from './utils.js'; import { getThreads } from './threadCrud.js'; -import { isHandoffRelationship } from './threadTypes.js'; + +function toChainThread(t: Thread, comment?: string): ChainThread { + return { + id: t.id, + title: t.title, + lastUpdated: t.lastUpdated, + workspace: t.workspace, + ...(comment != null ? { comment } : {}), + }; +} export async function getThreadChain(threadId: string): Promise { const { threads } = await getThreads({ limit: 1000 }); const threadMap = new Map(threads.map((t) => [t.id, t])); + // Build parent-to-children index from all threads' handoffParentId + const parentToChildren = new Map(); + for (const t of threads) { + if (t.handoffParentId && threadMap.has(t.handoffParentId)) { + const existing = parentToChildren.get(t.handoffParentId); + if (existing) { + existing.push(t.id); + } else { + parentToChildren.set(t.handoffParentId, [t.id]); + } + } + } + + // Walk up to collect ancestors (linear path to root) const ancestors: ChainThread[] = []; const visited = new Set([threadId]); - - interface ThreadWithRelationships extends Thread { - relationships?: ThreadRelationship[]; + let currentId = threadId; + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- runtime guard + while (true) { + const thread = threadMap.get(currentId); + const parentId = thread?.handoffParentId; + if (!parentId || visited.has(parentId)) break; + const parentThread = threadMap.get(parentId); + if (!parentThread) break; + visited.add(parentId); + ancestors.unshift(toChainThread(parentThread)); + currentId = parentId; } - function findAncestors(id: string): void { - const thread = threadMap.get(id) as ThreadWithRelationships | undefined; - if (!thread?.relationships) return; - - for (const rel of thread.relationships) { - if (isHandoffRelationship(rel) && rel.role === 'parent' && !visited.has(rel.threadID)) { - visited.add(rel.threadID); - const parentThread = threadMap.get(rel.threadID); - if (parentThread) { - ancestors.unshift({ - id: parentThread.id, - title: parentThread.title, - lastUpdated: parentThread.lastUpdated, - workspace: parentThread.workspace, - comment: rel.comment, - }); - findAncestors(rel.threadID); - } - } + // Build descendants tree (recursive, supports forks) + function buildDescendantNode(id: string): ThreadChainNode | null { + const t = threadMap.get(id); + if (!t) return null; + const childIds = parentToChildren.get(id) || []; + const children: ThreadChainNode[] = []; + for (const childId of childIds) { + if (visited.has(childId)) continue; + visited.add(childId); + const node = buildDescendantNode(childId); + if (node) children.push(node); } + return { thread: toChainThread(t), children }; } - const descendants: ChainThread[] = []; - - function findDescendants(id: string): void { - const thread = threadMap.get(id) as ThreadWithRelationships | undefined; - if (!thread?.relationships) return; + const descendantsTree: ThreadChainNode[] = []; + const flatDescendants: ChainThread[] = []; + const directChildIds = parentToChildren.get(threadId) || []; + for (const childId of directChildIds) { + if (visited.has(childId)) continue; + visited.add(childId); + const node = buildDescendantNode(childId); + if (node) descendantsTree.push(node); + } - for (const rel of thread.relationships) { - if (isHandoffRelationship(rel) && rel.role === 'child' && !visited.has(rel.threadID)) { - visited.add(rel.threadID); - const childThread = threadMap.get(rel.threadID); - if (childThread) { - descendants.push({ - id: childThread.id, - title: childThread.title, - lastUpdated: childThread.lastUpdated, - workspace: childThread.workspace, - comment: rel.comment, - }); - findDescendants(rel.threadID); - } - } + // BFS flatten the tree for backward-compatible flat descendants list + function flattenTree(nodes: ThreadChainNode[]): void { + for (const node of nodes) { + flatDescendants.push(node.thread); + flattenTree(node.children); } } - - findAncestors(threadId); - findDescendants(threadId); + flattenTree(descendantsTree); const currentThread = threadMap.get(threadId); - const current: ChainThread | null = currentThread - ? { - id: currentThread.id, - title: currentThread.title, - lastUpdated: currentThread.lastUpdated, - workspace: currentThread.workspace, - } - : null; + const current: ChainThread | null = currentThread ? toChainThread(currentThread) : null; - return { ancestors, current, descendants }; + return { ancestors, current, descendants: flatDescendants, descendantsTree }; } interface HandoffResult { diff --git a/server/lib/threadCrud.ts b/server/lib/threadCrud.ts index f73d54a..72fff97 100644 --- a/server/lib/threadCrud.ts +++ b/server/lib/threadCrud.ts @@ -127,18 +127,19 @@ export async function getThreads({ // Extract handoff relationships const relationships = data.relationships || []; let handoffParentId: string | null = null; - let handoffChildId: string | null = null; + const handoffChildIds: string[] = []; for (const rel of relationships) { if (isHandoffRelationship(rel)) { if (rel.role === 'child') { // "I am the child" → threadID is my parent handoffParentId = rel.threadID; } else { - // "I am the parent" → threadID is my child (use last one seen) - handoffChildId = rel.threadID; + // "I am the parent" → threadID is my child + handoffChildIds.push(rel.threadID); } } } + const uniqueChildIds = [...new Set(handoffChildIds)]; // Extract touched files from tool uses const touchedFiles = new Set(); @@ -167,7 +168,8 @@ export async function getThreads({ repo, touchedFiles: [...touchedFiles], handoffParentId, - handoffChildId, + handoffChildId: uniqueChildIds[uniqueChildIds.length - 1] ?? null, + handoffChildIds: uniqueChildIds, }; } catch (e) { const error = e as Error; diff --git a/shared/types.ts b/shared/types.ts index d600f93..9b65f3b 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -19,7 +19,9 @@ export interface Thread { autoInvoke?: boolean; // Handoff relationship IDs (derived from relationships for quick access) handoffParentId?: string | null; + /** @deprecated Use handoffChildIds instead. Kept for backward compat (last child seen). */ handoffChildId?: string | null; + handoffChildIds?: string[]; } export interface RelatedThread { @@ -49,10 +51,16 @@ export interface ChainThread { comment?: string; } +export interface ThreadChainNode { + thread: ChainThread; + children: ThreadChainNode[]; +} + export interface ThreadChain { ancestors: ChainThread[]; current: ChainThread | null; descendants: ChainThread[]; + descendantsTree?: ThreadChainNode[]; } export interface FileEdit { @@ -99,9 +107,16 @@ export type SortField = 'lastUpdated' | 'title' | 'messages' | 'status' | 'conte export type SortDirection = 'asc' | 'desc'; // Thread stacking (for grouping handoff chains) +export interface ThreadStackTopology { + rootId: string; + childToParent: Record; + parentToChildren: Record; +} + export interface ThreadStack { head: Thread; ancestors: Thread[]; // ordered from newest to oldest (head's parent first) + topology?: ThreadStackTopology; } export interface ThreadsResult { diff --git a/src/utils/threadStacks.ts b/src/utils/threadStacks.ts index 90cf026..39a66da 100644 --- a/src/utils/threadStacks.ts +++ b/src/utils/threadStacks.ts @@ -1,4 +1,4 @@ -import type { Thread, ThreadStack } from '../types'; +import type { Thread, ThreadStack, ThreadStackTopology } from '../types'; export interface ThreadListEntry { kind: 'thread' | 'stack'; @@ -85,10 +85,40 @@ export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { } if (ancestors.length > 0) { + // Build topology restricted to this stack's members + const childToParentLocal: Record = {}; + const parentToChildrenLocal: Record = {}; + let rootId = head.id; + + for (const member of chainMembers) { + const pid = childToParent.get(member.id); + if (pid && visited.has(pid)) { + childToParentLocal[member.id] = pid; + if (!parentToChildrenLocal[pid]) { + parentToChildrenLocal[pid] = []; + } + parentToChildrenLocal[pid].push(member.id); + } + } + + // Find root: the member with no parent in this stack + for (const member of chainMembers) { + if (!childToParentLocal[member.id]) { + rootId = member.id; + break; + } + } + + const topology: ThreadStackTopology = { + rootId, + childToParent: childToParentLocal, + parentToChildren: parentToChildrenLocal, + }; + entries.push({ kind: 'stack', thread: head, - stack: { head, ancestors }, + stack: { head, ancestors, topology }, }); } else { entries.push({ From 6c12ebcaaebbce4e553b7ec0c68b0b568bd9070e Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Sun, 22 Feb 2026 13:53:18 -0500 Subject: [PATCH 2/6] test: add fork topology assertions for threadStacks and threadChain - threadStacks.test.ts: assert topology adjacency maps on 'multiple children' and 'fan-out' tests (rootId, parentToChildren forks, childToParent edges) - threadChain.test.ts: new test file with 6 tests covering linear chain, fork descendants tree, mid-chain queries, unknown thread, and flat-vs-tree consistency Amp-Thread-ID: https://ampcode.com/threads/T-019c869d-7987-71df-99a5-e64a57114bee Co-authored-by: Amp --- server/lib/threadChain.test.ts | 173 +++++++++++++++++++++++++++++++++ src/utils/threadStacks.test.ts | 29 ++++++ 2 files changed, 202 insertions(+) create mode 100644 server/lib/threadChain.test.ts diff --git a/server/lib/threadChain.test.ts b/server/lib/threadChain.test.ts new file mode 100644 index 0000000..9da42f6 --- /dev/null +++ b/server/lib/threadChain.test.ts @@ -0,0 +1,173 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Thread, ThreadChainNode } from '../../shared/types.js'; + +vi.mock('./threadCrud.js', () => ({ + getThreads: vi.fn(), +})); + +import { getThreadChain } from './threadChain.js'; +import { getThreads } from './threadCrud.js'; + +const mockedGetThreads = vi.mocked(getThreads); + +function makeThread(overrides: Partial & { id: string }): Thread { + return { + title: `Thread ${overrides.id}`, + lastUpdated: '2 hours ago', + visibility: 'Private' as const, + messages: 5, + ...overrides, + }; +} + +function collectNodeIds(nodes: ThreadChainNode[]): string[] { + const ids: string[] = []; + for (const node of nodes) { + ids.push(node.thread.id); + ids.push(...collectNodeIds(node.children)); + } + return ids; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('getThreadChain', () => { + it('returns empty chain for a thread with no relationships', async () => { + mockedGetThreads.mockResolvedValue({ + threads: [makeThread({ id: 'solo' })], + nextCursor: null, + hasMore: false, + }); + + const chain = await getThreadChain('solo'); + expect(chain.ancestors).toEqual([]); + expect(chain.current?.id).toBe('solo'); + expect(chain.descendants).toEqual([]); + expect(chain.descendantsTree).toEqual([]); + }); + + it('returns linear ancestors for a simple chain', async () => { + mockedGetThreads.mockResolvedValue({ + threads: [ + makeThread({ id: 'root' }), + makeThread({ id: 'middle', handoffParentId: 'root' }), + makeThread({ id: 'leaf', handoffParentId: 'middle' }), + ], + nextCursor: null, + hasMore: false, + }); + + const chain = await getThreadChain('leaf'); + expect(chain.ancestors.map((a) => a.id)).toEqual(['root', 'middle']); + expect(chain.current?.id).toBe('leaf'); + expect(chain.descendants).toEqual([]); + expect(chain.descendantsTree).toEqual([]); + }); + + it('returns flat descendants and tree for a fork', async () => { + // root → mid → [child-a, child-b] + mockedGetThreads.mockResolvedValue({ + threads: [ + makeThread({ id: 'root' }), + makeThread({ id: 'mid', handoffParentId: 'root' }), + makeThread({ id: 'child-a', handoffParentId: 'mid' }), + makeThread({ id: 'child-b', handoffParentId: 'mid' }), + ], + nextCursor: null, + hasMore: false, + }); + + const chain = await getThreadChain('root'); + expect(chain.ancestors).toEqual([]); + expect(chain.current?.id).toBe('root'); + + // Flat descendants should contain all 3 + const descIds = chain.descendants.map((d) => d.id); + expect(descIds).toContain('mid'); + expect(descIds).toContain('child-a'); + expect(descIds).toContain('child-b'); + + // Tree: root's direct child is mid, mid has two children + expect(chain.descendantsTree).toHaveLength(1); + const midNode = chain.descendantsTree?.[0]; + expect(midNode?.thread.id).toBe('mid'); + expect(midNode?.children).toHaveLength(2); + const childIds = midNode?.children.map((c) => c.thread.id) ?? []; + expect(childIds).toContain('child-a'); + expect(childIds).toContain('child-b'); + }); + + it('builds tree from middle of a chain', async () => { + // root → mid → [child-a → grandchild, child-b] + mockedGetThreads.mockResolvedValue({ + threads: [ + makeThread({ id: 'root' }), + makeThread({ id: 'mid', handoffParentId: 'root' }), + makeThread({ id: 'child-a', handoffParentId: 'mid' }), + makeThread({ id: 'child-b', handoffParentId: 'mid' }), + makeThread({ id: 'grandchild', handoffParentId: 'child-a' }), + ], + nextCursor: null, + hasMore: false, + }); + + const chain = await getThreadChain('mid'); + + // Ancestors: just root + expect(chain.ancestors.map((a) => a.id)).toEqual(['root']); + expect(chain.current?.id).toBe('mid'); + + // Descendants tree: two children of mid + expect(chain.descendantsTree).toHaveLength(2); + const allDescIds = collectNodeIds(chain.descendantsTree ?? []); + expect(allDescIds).toContain('child-a'); + expect(allDescIds).toContain('child-b'); + expect(allDescIds).toContain('grandchild'); + + // child-a should have grandchild as its child in the tree + const childANode = chain.descendantsTree?.find((n) => n.thread.id === 'child-a'); + expect(childANode?.children).toHaveLength(1); + expect(childANode?.children[0]?.thread.id).toBe('grandchild'); + + // child-b is a leaf + const childBNode = chain.descendantsTree?.find((n) => n.thread.id === 'child-b'); + expect(childBNode?.children).toEqual([]); + }); + + it('returns null current for unknown thread id', async () => { + mockedGetThreads.mockResolvedValue({ + threads: [makeThread({ id: 'other' })], + nextCursor: null, + hasMore: false, + }); + + const chain = await getThreadChain('nonexistent'); + expect(chain.current).toBeNull(); + expect(chain.ancestors).toEqual([]); + expect(chain.descendants).toEqual([]); + }); + + it('flat descendants match tree contents', async () => { + // Verify the flat list and tree contain the same threads + // root → [a → [a1, a2], b] + mockedGetThreads.mockResolvedValue({ + threads: [ + makeThread({ id: 'root' }), + makeThread({ id: 'a', handoffParentId: 'root' }), + makeThread({ id: 'b', handoffParentId: 'root' }), + makeThread({ id: 'a1', handoffParentId: 'a' }), + makeThread({ id: 'a2', handoffParentId: 'a' }), + ], + nextCursor: null, + hasMore: false, + }); + + const chain = await getThreadChain('root'); + const flatIds = chain.descendants.map((d) => d.id).sort(); + const treeIds = collectNodeIds(chain.descendantsTree ?? []).sort(); + expect(flatIds).toEqual(treeIds); + expect(flatIds).toEqual(['a', 'a1', 'a2', 'b']); + }); +}); diff --git a/src/utils/threadStacks.test.ts b/src/utils/threadStacks.test.ts index 8cca751..4e605c7 100644 --- a/src/utils/threadStacks.test.ts +++ b/src/utils/threadStacks.test.ts @@ -133,6 +133,18 @@ describe('buildThreadStacks', () => { expect(ancestorIds).toContain('parent'); expect(ancestorIds).toContain('child1'); expect(ancestorIds).toContain('child2'); + + // Topology captures the fork: parent has 3 children + const topo = at(entries, 0).stack?.topology; + expect(topo).toBeDefined(); + expect(topo?.rootId).toBe('parent'); + expect(topo?.parentToChildren['parent']).toHaveLength(3); + expect(topo?.parentToChildren['parent']).toContain('child1'); + expect(topo?.parentToChildren['parent']).toContain('child2'); + expect(topo?.parentToChildren['parent']).toContain('child3'); + expect(topo?.childToParent['child1']).toBe('parent'); + expect(topo?.childToParent['child2']).toBe('parent'); + expect(topo?.childToParent['child3']).toBe('parent'); }); it('handles fan-out: parent with children that also have children', () => { @@ -160,6 +172,23 @@ describe('buildThreadStacks', () => { // leaf is most recent expect(at(entries, 0).thread.id).toBe('leaf'); expect(getStackSize(at(entries, 0))).toBe(4); + + // Topology captures the full tree including fork at root + const topo = at(entries, 0).stack?.topology; + expect(topo).toBeDefined(); + expect(topo?.rootId).toBe('root'); + // root has two children (branch-a and branch-b) + expect(topo?.parentToChildren['root']).toHaveLength(2); + expect(topo?.parentToChildren['root']).toContain('branch-a'); + expect(topo?.parentToChildren['root']).toContain('branch-b'); + // branch-a has one child (leaf) + expect(topo?.parentToChildren['branch-a']).toEqual(['leaf']); + // branch-b is a leaf (no children entry) + expect(topo?.parentToChildren['branch-b']).toBeUndefined(); + // child-to-parent edges + expect(topo?.childToParent['branch-a']).toBe('root'); + expect(topo?.childToParent['branch-b']).toBe('root'); + expect(topo?.childToParent['leaf']).toBe('branch-a'); }); }); From edaac0c901b73ce5e6acfbe6e208c87da0edb094 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Sun, 22 Feb 2026 14:16:23 -0500 Subject: [PATCH 3/6] refactor: replace flat descendants with descendantsTree in ThreadChain - Remove ThreadChain.descendants (flat ChainThread[]) - Make ThreadChain.descendantsTree required (was optional) - Drop flattenTree() from threadChain.ts (no longer needed) - Update ThreadChainContent.tsx: recursive DescendantNodes renderer with depth-based indentation and GitFork icon for branches - Update useThreadDiscovery.ts: chainCount uses descendantsTree.length - Update tests to assert on tree structure only Amp-Thread-ID: https://ampcode.com/threads/T-019c869d-7987-71df-99a5-e64a57114bee Co-authored-by: Amp --- server/lib/threadChain.test.ts | 42 ++++++------ server/lib/threadChain.ts | 12 +--- shared/types.ts | 3 +- src/components/ThreadChainContent.tsx | 65 ++++++++++++++----- .../ThreadDiscovery/useThreadDiscovery.ts | 2 +- 5 files changed, 73 insertions(+), 51 deletions(-) diff --git a/server/lib/threadChain.test.ts b/server/lib/threadChain.test.ts index 9da42f6..ba8ff53 100644 --- a/server/lib/threadChain.test.ts +++ b/server/lib/threadChain.test.ts @@ -44,7 +44,6 @@ describe('getThreadChain', () => { const chain = await getThreadChain('solo'); expect(chain.ancestors).toEqual([]); expect(chain.current?.id).toBe('solo'); - expect(chain.descendants).toEqual([]); expect(chain.descendantsTree).toEqual([]); }); @@ -62,11 +61,10 @@ describe('getThreadChain', () => { const chain = await getThreadChain('leaf'); expect(chain.ancestors.map((a) => a.id)).toEqual(['root', 'middle']); expect(chain.current?.id).toBe('leaf'); - expect(chain.descendants).toEqual([]); expect(chain.descendantsTree).toEqual([]); }); - it('returns flat descendants and tree for a fork', async () => { + it('returns tree with fork for multiple children', async () => { // root → mid → [child-a, child-b] mockedGetThreads.mockResolvedValue({ threads: [ @@ -83,20 +81,20 @@ describe('getThreadChain', () => { expect(chain.ancestors).toEqual([]); expect(chain.current?.id).toBe('root'); - // Flat descendants should contain all 3 - const descIds = chain.descendants.map((d) => d.id); - expect(descIds).toContain('mid'); - expect(descIds).toContain('child-a'); - expect(descIds).toContain('child-b'); - // Tree: root's direct child is mid, mid has two children expect(chain.descendantsTree).toHaveLength(1); - const midNode = chain.descendantsTree?.[0]; + const midNode = chain.descendantsTree[0]; expect(midNode?.thread.id).toBe('mid'); expect(midNode?.children).toHaveLength(2); const childIds = midNode?.children.map((c) => c.thread.id) ?? []; expect(childIds).toContain('child-a'); expect(childIds).toContain('child-b'); + + // All 3 descendants are in the tree + const allIds = collectNodeIds(chain.descendantsTree); + expect(allIds).toContain('mid'); + expect(allIds).toContain('child-a'); + expect(allIds).toContain('child-b'); }); it('builds tree from middle of a chain', async () => { @@ -121,18 +119,18 @@ describe('getThreadChain', () => { // Descendants tree: two children of mid expect(chain.descendantsTree).toHaveLength(2); - const allDescIds = collectNodeIds(chain.descendantsTree ?? []); + const allDescIds = collectNodeIds(chain.descendantsTree); expect(allDescIds).toContain('child-a'); expect(allDescIds).toContain('child-b'); expect(allDescIds).toContain('grandchild'); // child-a should have grandchild as its child in the tree - const childANode = chain.descendantsTree?.find((n) => n.thread.id === 'child-a'); + const childANode = chain.descendantsTree.find((n) => n.thread.id === 'child-a'); expect(childANode?.children).toHaveLength(1); expect(childANode?.children[0]?.thread.id).toBe('grandchild'); // child-b is a leaf - const childBNode = chain.descendantsTree?.find((n) => n.thread.id === 'child-b'); + const childBNode = chain.descendantsTree.find((n) => n.thread.id === 'child-b'); expect(childBNode?.children).toEqual([]); }); @@ -146,11 +144,10 @@ describe('getThreadChain', () => { const chain = await getThreadChain('nonexistent'); expect(chain.current).toBeNull(); expect(chain.ancestors).toEqual([]); - expect(chain.descendants).toEqual([]); + expect(chain.descendantsTree).toEqual([]); }); - it('flat descendants match tree contents', async () => { - // Verify the flat list and tree contain the same threads + it('tree contains all descendants in a complex fork', async () => { // root → [a → [a1, a2], b] mockedGetThreads.mockResolvedValue({ threads: [ @@ -165,9 +162,14 @@ describe('getThreadChain', () => { }); const chain = await getThreadChain('root'); - const flatIds = chain.descendants.map((d) => d.id).sort(); - const treeIds = collectNodeIds(chain.descendantsTree ?? []).sort(); - expect(flatIds).toEqual(treeIds); - expect(flatIds).toEqual(['a', 'a1', 'a2', 'b']); + const treeIds = collectNodeIds(chain.descendantsTree).sort(); + expect(treeIds).toEqual(['a', 'a1', 'a2', 'b']); + + // Verify tree structure: root has 2 direct children + expect(chain.descendantsTree).toHaveLength(2); + const nodeA = chain.descendantsTree.find((n) => n.thread.id === 'a'); + expect(nodeA?.children).toHaveLength(2); + const nodeB = chain.descendantsTree.find((n) => n.thread.id === 'b'); + expect(nodeB?.children).toEqual([]); }); }); diff --git a/server/lib/threadChain.ts b/server/lib/threadChain.ts index 875d79e..65cce5b 100644 --- a/server/lib/threadChain.ts +++ b/server/lib/threadChain.ts @@ -61,7 +61,6 @@ export async function getThreadChain(threadId: string): Promise { } const descendantsTree: ThreadChainNode[] = []; - const flatDescendants: ChainThread[] = []; const directChildIds = parentToChildren.get(threadId) || []; for (const childId of directChildIds) { if (visited.has(childId)) continue; @@ -70,19 +69,10 @@ export async function getThreadChain(threadId: string): Promise { if (node) descendantsTree.push(node); } - // BFS flatten the tree for backward-compatible flat descendants list - function flattenTree(nodes: ThreadChainNode[]): void { - for (const node of nodes) { - flatDescendants.push(node.thread); - flattenTree(node.children); - } - } - flattenTree(descendantsTree); - const currentThread = threadMap.get(threadId); const current: ChainThread | null = currentThread ? toChainThread(currentThread) : null; - return { ancestors, current, descendants: flatDescendants, descendantsTree }; + return { ancestors, current, descendantsTree }; } interface HandoffResult { diff --git a/shared/types.ts b/shared/types.ts index 9b65f3b..0bd2d94 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -59,8 +59,7 @@ export interface ThreadChainNode { export interface ThreadChain { ancestors: ChainThread[]; current: ChainThread | null; - descendants: ChainThread[]; - descendantsTree?: ThreadChainNode[]; + descendantsTree: ThreadChainNode[]; } export interface FileEdit { diff --git a/src/components/ThreadChainContent.tsx b/src/components/ThreadChainContent.tsx index 0c22004..5b551bf 100644 --- a/src/components/ThreadChainContent.tsx +++ b/src/components/ThreadChainContent.tsx @@ -1,11 +1,55 @@ -import { ChevronRight } from 'lucide-react'; -import type { ThreadChain, ChainThread, Thread } from '../types'; +import { ChevronRight, GitFork } from 'lucide-react'; +import type { ThreadChain, ThreadChainNode, ChainThread, Thread } from '../types'; interface ThreadChainContentProps { chain: ThreadChain; onOpenThread: (thread: Thread) => void; } +function DescendantNodes({ + nodes, + depth, + onClickThread, +}: { + nodes: ThreadChainNode[]; + depth: number; + onClickThread: (t: ChainThread) => void; +}) { + return ( + <> + {nodes.map((node) => ( +
+
0 ? { marginLeft: depth * 16 } : undefined} + > + {depth > 0 && nodes.length > 1 && } + +
+ {node.children.length > 0 && ( + <> +
+ +
+ + + )} +
+ ))} + + ); +} + export function ThreadChainContent({ chain, onOpenThread }: ThreadChainContentProps) { const handleClick = (t: ChainThread) => { onOpenThread({ @@ -42,7 +86,7 @@ export function ThreadChainContent({ chain, onOpenThread }: ThreadChainContentPr Current {chain.current.title} - {chain.descendants.length > 0 && ( + {chain.descendantsTree.length > 0 && (
@@ -50,20 +94,7 @@ export function ThreadChainContent({ chain, onOpenThread }: ThreadChainContentPr )} - {chain.descendants.map((t, i) => ( -
- - {i < chain.descendants.length - 1 && ( -
- -
- )} -
- ))} + ); diff --git a/src/components/ThreadDiscovery/useThreadDiscovery.ts b/src/components/ThreadDiscovery/useThreadDiscovery.ts index 6767314..b4b8b17 100644 --- a/src/components/ThreadDiscovery/useThreadDiscovery.ts +++ b/src/components/ThreadDiscovery/useThreadDiscovery.ts @@ -257,7 +257,7 @@ export function useThreadDiscovery({ if (!signal.aborted) { setChain(data); const chainCount = data - ? (data.ancestors?.length || 0) + (data.descendants?.length || 0) + ? (data.ancestors?.length || 0) + (data.descendantsTree?.length || 0) : 0; setSummary((prev) => ({ ...prev, chainCount })); } From b25c7e75d84a8fc9234a74382674e53fd188b128 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Sun, 22 Feb 2026 15:20:45 -0500 Subject: [PATCH 4/6] Remove deprecated handoffChildId field The plural handoffChildIds (added in PR #59) fully replaces it. Removed from Thread interface and threadCrud assignment. Amp-Thread-ID: https://ampcode.com/threads/T-019c8702-28e9-73ba-b5e4-496c1dc11651 Co-authored-by: Amp --- server/lib/threadCrud.ts | 1 - shared/types.ts | 2 -- 2 files changed, 3 deletions(-) diff --git a/server/lib/threadCrud.ts b/server/lib/threadCrud.ts index 72fff97..9261f92 100644 --- a/server/lib/threadCrud.ts +++ b/server/lib/threadCrud.ts @@ -168,7 +168,6 @@ export async function getThreads({ repo, touchedFiles: [...touchedFiles], handoffParentId, - handoffChildId: uniqueChildIds[uniqueChildIds.length - 1] ?? null, handoffChildIds: uniqueChildIds, }; } catch (e) { diff --git a/shared/types.ts b/shared/types.ts index 0bd2d94..f3eba1d 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -19,8 +19,6 @@ export interface Thread { autoInvoke?: boolean; // Handoff relationship IDs (derived from relationships for quick access) handoffParentId?: string | null; - /** @deprecated Use handoffChildIds instead. Kept for backward compat (last child seen). */ - handoffChildId?: string | null; handoffChildIds?: string[]; } From 5079ead44fa98f8e692bb032f03976c41d585b70 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Mon, 23 Feb 2026 11:23:42 -0500 Subject: [PATCH 5/6] feat: root-as-head stack rendering with tree expansion - buildThreadStacks() now uses root (no parent) as head instead of most-recent - Added lastActiveDate to ThreadStack for sort ordering - Added descendants field (DFS tree-ordered) alongside deprecated ancestors - Re-sort entries by lastActiveDate so active stacks float to top - New StackTree component renders expanded stacks as indented tree via topology - ThreadRow supports stackDepth prop for depth-based indentation - Stack head Updated column shows most recently active thread's time - KanbanView uses getLastActiveThread() for column placement and time display - Removed mutable visited Set from StackTree render (React strict mode fix) - All three views (table, kanban, cards) use tree rendering with depth - 258 tests pass, typecheck + lint clean Amp-Thread-ID: https://ampcode.com/threads/T-019c8b24-9f53-7038-a667-56ab9bee122f Co-authored-by: Amp --- shared/types.ts | 4 +- src/components/DetailCardView/ThreadCard.tsx | 48 ++++-- src/components/DetailCardView/index.tsx | 3 +- src/components/KanbanView.tsx | 65 +++++--- src/components/ThreadList/StackTree.tsx | 106 ++++++++++++ src/components/ThreadList/ThreadRow.tsx | 13 +- src/components/ThreadList/index.tsx | 51 +++--- src/components/ThreadList/types.ts | 2 + src/utils/threadStacks.test.ts | 163 +++++++++++++++---- src/utils/threadStacks.ts | 127 +++++++++++---- 10 files changed, 462 insertions(+), 120 deletions(-) create mode 100644 src/components/ThreadList/StackTree.tsx diff --git a/shared/types.ts b/shared/types.ts index 19f94d2..b3f30e7 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -112,7 +112,9 @@ export interface ThreadStackTopology { export interface ThreadStack { head: Thread; - ancestors: Thread[]; // ordered from newest to oldest (head's parent first) + ancestors: Thread[]; // @deprecated - use descendants instead (kept for backward compat) + descendants: Thread[]; // tree-ordered via DFS from root's children + lastActiveDate?: string; // most recent lastUpdatedDate across all members topology?: ThreadStackTopology; } diff --git a/src/components/DetailCardView/ThreadCard.tsx b/src/components/DetailCardView/ThreadCard.tsx index 8960c6b..52d7ac4 100644 --- a/src/components/DetailCardView/ThreadCard.tsx +++ b/src/components/DetailCardView/ThreadCard.tsx @@ -12,7 +12,7 @@ import { } from 'lucide-react'; import { ThreadStatusBadge } from '../ThreadStatusBadge'; import { LinkedIssueBadge } from '../LinkedIssue'; -import type { Thread, ThreadMetadata, ThreadStatus } from '../../types'; +import type { Thread, ThreadMetadata, ThreadStatus, ThreadStackTopology } from '../../types'; function formatRelativeTime(dateStr: string): string { const date = new Date(dateStr); @@ -42,7 +42,18 @@ export interface ThreadCardProps { isExpanded?: boolean; onToggleExpand?: () => void; isStackChild?: boolean; - stackAncestors?: Thread[]; + stackDescendants?: Thread[]; + topology?: ThreadStackTopology; +} + +function getCardDepth(nodeId: string, topology: ThreadStackTopology): number { + let depth = 0; + let current = nodeId; + while (topology.childToParent[current]) { + depth++; + current = topology.childToParent[current]; + } + return Math.max(0, depth - 1); } function getFilename(path: string): string { @@ -61,7 +72,8 @@ export function ThreadCard({ isExpanded, onToggleExpand, isStackChild, - stackAncestors, + stackDescendants, + topology, }: ThreadCardProps) { const meta = metadata[thread.id]; const status = meta?.status || 'active'; @@ -180,35 +192,35 @@ export function ThreadCard({ )} - {hasStack && isExpanded && stackAncestors && stackAncestors.length > 0 && ( + {hasStack && isExpanded && stackDescendants && stackDescendants.length > 0 && (
- {stackAncestors.map((ancestor) => { - const ancestorMeta = metadata[ancestor.id]; + {stackDescendants.map((desc) => { + const descMeta = metadata[desc.id]; + const depth = topology ? getCardDepth(desc.id, topology) : 0; return (
onContinue(ancestor)} + key={desc.id} + className={`detail-card stack-child status-${descMeta?.status || 'active'}`} + style={depth > 0 ? { marginLeft: depth * 12 } : undefined} + onClick={() => onContinue(desc)} >
onStatusChange?.(ancestor.id, s)} + threadId={desc.id} + status={descMeta?.status || 'active'} + onStatusChange={(s) => onStatusChange?.(desc.id, s)} compact />
-

{ancestor.title}

+

{desc.title}

- - {formatRelativeTime(ancestor.lastUpdated)} - - {ancestor.cost !== undefined && ( + {formatRelativeTime(desc.lastUpdated)} + {desc.cost !== undefined && ( - ~${ancestor.cost.toFixed(2)} + ~${desc.cost.toFixed(2)} )}
diff --git a/src/components/DetailCardView/index.tsx b/src/components/DetailCardView/index.tsx index 800bd3b..aa9af9e 100644 --- a/src/components/DetailCardView/index.tsx +++ b/src/components/DetailCardView/index.tsx @@ -131,7 +131,8 @@ export function DetailCardView({ stackSize={stackSize} isExpanded={isExpanded} onToggleExpand={() => toggleStackExpand(entry.thread.id)} - stackAncestors={entry.stack?.ancestors} + stackDescendants={entry.stack?.descendants} + topology={entry.stack?.topology} /> ); })} diff --git a/src/components/KanbanView.tsx b/src/components/KanbanView.tsx index 1003286..8e1c050 100644 --- a/src/components/KanbanView.tsx +++ b/src/components/KanbanView.tsx @@ -1,7 +1,24 @@ import { memo, useMemo, useState, useCallback } from 'react'; import { ExternalLink, GitBranch, ChevronRight, ChevronDown, Layers } from 'lucide-react'; import type { Thread, ThreadMetadata, ThreadStatus } from '../types'; -import { buildThreadStacks, getStackSize, type ThreadListEntry } from '../utils/threadStacks'; +import { + buildThreadStacks, + getStackSize, + getLastActiveThread, + type ThreadListEntry, +} from '../utils/threadStacks'; + +function getDepth(nodeId: string, topology: import('../types').ThreadStackTopology): number { + let depth = 0; + let current = nodeId; + while (topology.childToParent[current]) { + depth++; + current = topology.childToParent[current]; + } + // Subtract 1 because root is depth 0 and its children are depth 1, + // but root is already shown as the head card + return Math.max(0, depth - 1); +} interface KanbanCardProps { thread: Thread; @@ -13,8 +30,10 @@ interface KanbanCardProps { stackSize?: number; isExpanded?: boolean; onToggleExpand?: () => void; - stackAncestors?: Thread[]; + stackDescendants?: Thread[]; + topology?: import('../types').ThreadStackTopology; allMetadata?: Record; + displayLastUpdated?: string; } const KanbanCard = memo(function KanbanCard({ @@ -26,8 +45,10 @@ const KanbanCard = memo(function KanbanCard({ stackSize, isExpanded, onToggleExpand, - stackAncestors, + stackDescendants, + topology, allMetadata, + displayLastUpdated, }: KanbanCardProps) { const hasStack = stackSize && stackSize > 1; @@ -69,7 +90,7 @@ const KanbanCard = memo(function KanbanCard({ )}
- {thread.lastUpdated} + {displayLastUpdated || thread.lastUpdated} {thread.contextPercent !== undefined && ( 80 ? 'warning' : ''}`}> {thread.contextPercent}% @@ -104,31 +125,31 @@ const KanbanCard = memo(function KanbanCard({
- {hasStack && isExpanded && stackAncestors && stackAncestors.length > 0 && ( + {hasStack && isExpanded && stackDescendants && stackDescendants.length > 0 && (
- {stackAncestors.map((ancestor) => { - const ancestorMeta = allMetadata?.[ancestor.id]; - const ancestorStatus = ancestorMeta?.status || 'active'; + {stackDescendants.map((desc) => { + const descMeta = allMetadata?.[desc.id]; + const descStatus = descMeta?.status || 'active'; + const depth = topology ? getDepth(desc.id, topology) : 0; return (
0 ? { marginLeft: depth * 12 } : undefined} role="button" tabIndex={0} - onClick={() => onContinue(ancestor)} + onClick={() => onContinue(desc)} onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); - onContinue(ancestor); + onContinue(desc); } }} > -
{ancestor.title}
+
{desc.title}
- {ancestor.lastUpdated} - - {ancestorStatus} - + {desc.lastUpdated} + {descStatus}
); @@ -187,7 +208,10 @@ export const KanbanView = memo(function KanbanView({ }; for (const entry of entries) { - const status = metadata[entry.thread.id]?.status || 'active'; + // Use last active thread's status for column placement so stacks + // appear in the column matching their most recent activity + const activeThread = getLastActiveThread(entry); + const status = metadata[activeThread.id]?.status || 'active'; result[status].push(entry); } @@ -232,6 +256,7 @@ export const KanbanView = memo(function KanbanView({ {columnData[status].map((entry) => { const stackSize = getStackSize(entry); const isExpanded = expandedStacks.has(entry.thread.id); + const lastActive = entry.kind === 'stack' ? getLastActiveThread(entry) : undefined; return ( toggleStackExpand(entry.thread.id)} - stackAncestors={entry.stack?.ancestors} + stackDescendants={entry.stack?.descendants} + topology={entry.stack?.topology} allMetadata={metadata} + displayLastUpdated={lastActive?.lastUpdated} /> ); })} diff --git a/src/components/ThreadList/StackTree.tsx b/src/components/ThreadList/StackTree.tsx new file mode 100644 index 0000000..207c4d9 --- /dev/null +++ b/src/components/ThreadList/StackTree.tsx @@ -0,0 +1,106 @@ +import React from 'react'; +import type { Thread, ThreadMetadata, ThreadStatus, ThreadStackTopology } from '../../types'; +import { ThreadRow } from './ThreadRow'; + +interface StackTreeProps { + topology: ThreadStackTopology; + threadMap: Map; + metadata: Record; + threadLabelObjects?: Record; + selectedIds: Set; + focusedThreadId?: string; + onContinue: (thread: Thread) => void; + onArchive: (thread: Thread) => void; + onDelete: (thread: Thread) => void; + onStatusChange?: (threadId: string, status: ThreadStatus) => void; + onSelect: (threadId: string, shiftKey: boolean) => void; +} + +function StackTreeNodes({ + parentId, + depth, + topology, + threadMap, + ...passthrough +}: { + parentId: string; + depth: number; + topology: ThreadStackTopology; + threadMap: Map; + metadata: Record; + threadLabelObjects?: Record; + selectedIds: Set; + focusedThreadId?: string; + onContinue: (thread: Thread) => void; + onArchive: (thread: Thread) => void; + onDelete: (thread: Thread) => void; + onStatusChange?: (threadId: string, status: ThreadStatus) => void; + onSelect: (threadId: string, shiftKey: boolean) => void; +}) { + const childIds = topology.parentToChildren[parentId]; + if (!childIds) return null; + + // Sort children by lastUpdatedDate desc for deterministic ordering + const sorted = [...childIds].sort((a, b) => { + const ta = threadMap.get(a); + const tb = threadMap.get(b); + const dateA = ta?.lastUpdatedDate ? Date.parse(ta.lastUpdatedDate) : 0; + const dateB = tb?.lastUpdatedDate ? Date.parse(tb.lastUpdatedDate) : 0; + return (Number.isFinite(dateB) ? dateB : 0) - (Number.isFinite(dateA) ? dateA : 0); + }); + + return ( + <> + {sorted.map((childId) => { + const thread = threadMap.get(childId); + if (!thread) return null; + + return ( + + + + + ); + })} + + ); +} + +export function StackTree(props: StackTreeProps) { + return ( + + ); +} diff --git a/src/components/ThreadList/ThreadRow.tsx b/src/components/ThreadList/ThreadRow.tsx index c882014..d3724b5 100644 --- a/src/components/ThreadList/ThreadRow.tsx +++ b/src/components/ThreadList/ThreadRow.tsx @@ -30,6 +30,8 @@ export const ThreadRow = memo(function ThreadRow({ isExpanded, onToggleExpand, isStackChild, + stackDepth, + displayLastUpdated, }: ThreadRowProps) { const hasStack = stackSize && stackSize > 1; @@ -87,7 +89,14 @@ export const ThreadRow = memo(function ThreadRow({ {stackSize} )} - {isStackChild && } + {isStackChild && ( + 1 ? { marginLeft: (stackDepth - 1) * 16 } : undefined + } + /> + )} {thread.title} {metadata?.linked_issue_url && ( @@ -97,7 +106,7 @@ export const ThreadRow = memo(function ThreadRow({ e.stopPropagation()}> - {thread.lastUpdated} + {displayLastUpdated || thread.lastUpdated} {thread.contextPercent !== undefined ? ( 80 ? 'context-warning' : ''}> diff --git a/src/components/ThreadList/index.tsx b/src/components/ThreadList/index.tsx index d90ff0b..6b34750 100644 --- a/src/components/ThreadList/index.tsx +++ b/src/components/ThreadList/index.tsx @@ -14,7 +14,8 @@ import { useThreadListSelection } from './useThreadListSelection'; import { useThreadListKeyboard } from './useThreadListKeyboard'; import { PAGE_SIZE } from './constants'; import type { ThreadListProps, BulkAction } from './types'; -import { buildThreadStacks, getStackSize } from '../../utils/threadStacks'; +import { buildThreadStacks, getStackSize, getLastActiveThread } from '../../utils/threadStacks'; +import { StackTree } from './StackTree'; import { CostInfoTip } from '../CostInfoTip'; // Re-exports for external consumers @@ -91,12 +92,21 @@ export const ThreadList = memo(function ThreadList({ for (const entry of paginatedEntries) { result.push(entry.thread); if (entry.kind === 'stack' && entry.stack && expandedStacks.has(entry.thread.id)) { - result.push(...entry.stack.ancestors); + result.push(...entry.stack.descendants); } } return result; }, [paginatedEntries, expandedStacks]); + // Build thread map for StackTree (all threads by id) + const threadMapForStacks = useMemo(() => { + const map = new Map(); + for (const t of threads) { + map.set(t.id, t); + } + return map; + }, [threads]); + const { selectedIds, toggleSelect, selectAll, clearSelection, isAllSelected, isSomeSelected } = useThreadListSelection({ threads, paginatedThreads }); @@ -236,6 +246,8 @@ export const ThreadList = memo(function ThreadList({ {paginatedEntries.map((entry) => { const stackSize = getStackSize(entry); const isExpanded = expandedStacks.has(entry.thread.id); + const lastActiveThread = + entry.kind === 'stack' ? getLastActiveThread(entry) : undefined; return ( @@ -253,26 +265,23 @@ export const ThreadList = memo(function ThreadList({ stackSize={stackSize} isExpanded={isExpanded} onToggleExpand={() => toggleStackExpand(entry.thread.id)} + displayLastUpdated={lastActiveThread?.lastUpdated} /> - {entry.kind === 'stack' && - entry.stack && - isExpanded && - entry.stack.ancestors.map((ancestor) => ( - - ))} + {entry.kind === 'stack' && entry.stack?.topology && isExpanded && ( + + )} ); })} diff --git a/src/components/ThreadList/types.ts b/src/components/ThreadList/types.ts index c31bef2..5f1612f 100644 --- a/src/components/ThreadList/types.ts +++ b/src/components/ThreadList/types.ts @@ -37,4 +37,6 @@ export interface ThreadRowProps { isExpanded?: boolean; onToggleExpand?: () => void; isStackChild?: boolean; + stackDepth?: number; + displayLastUpdated?: string; } diff --git a/src/utils/threadStacks.test.ts b/src/utils/threadStacks.test.ts index 4e605c7..e0e5b50 100644 --- a/src/utils/threadStacks.test.ts +++ b/src/utils/threadStacks.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from 'vitest'; import type { Thread } from '../types'; -import { buildThreadStacks, flattenEntries, getStackSize } from './threadStacks'; +import { + buildThreadStacks, + flattenEntries, + getStackSize, + getLastActiveThread, +} from './threadStacks'; function at(arr: T[], index: number): T { const item = arr[index]; @@ -32,7 +37,7 @@ describe('buildThreadStacks', () => { expect(at(entries, 0).stack).toBeUndefined(); }); - it('groups parent-child into a stack', () => { + it('groups parent-child into a stack with root as head', () => { const threads = [ makeThread({ id: 'parent', lastUpdatedDate: '2025-01-01T00:00:00Z' }), makeThread({ @@ -44,22 +49,25 @@ describe('buildThreadStacks', () => { const entries = buildThreadStacks(threads); expect(entries).toHaveLength(1); expect(at(entries, 0).kind).toBe('stack'); - // Child is more recent, so it becomes head - expect(at(entries, 0).thread.id).toBe('child'); - expect(at(entries, 0).stack?.ancestors).toHaveLength(1); - expect(at(entries, 0).stack?.ancestors[0]?.id).toBe('parent'); + // Root (parent) is head + expect(at(entries, 0).thread.id).toBe('parent'); + expect(at(entries, 0).stack?.descendants).toHaveLength(1); + expect(at(entries, 0).stack?.descendants[0]?.id).toBe('child'); + // lastActiveDate is the child's date (most recent) + expect(at(entries, 0).stack?.lastActiveDate).toBe('2025-01-02T00:00:00Z'); }); - it('picks most recently updated thread as head', () => { + it('picks root (no parent) as head, not most recent', () => { const threads = [ makeThread({ id: 'old', lastUpdatedDate: '2025-01-01T00:00:00Z' }), makeThread({ id: 'new', handoffParentId: 'old', lastUpdatedDate: '2025-01-10T00:00:00Z' }), ]; const entries = buildThreadStacks(threads); - expect(at(entries, 0).thread.id).toBe('new'); + expect(at(entries, 0).thread.id).toBe('old'); + expect(at(entries, 0).stack?.lastActiveDate).toBe('2025-01-10T00:00:00Z'); }); - it('builds a 3-thread chain', () => { + it('builds a 3-thread chain with root as head', () => { const threads = [ makeThread({ id: 'a', lastUpdatedDate: '2025-01-01T00:00:00Z' }), makeThread({ id: 'b', handoffParentId: 'a', lastUpdatedDate: '2025-01-02T00:00:00Z' }), @@ -68,8 +76,10 @@ describe('buildThreadStacks', () => { const entries = buildThreadStacks(threads); expect(entries).toHaveLength(1); expect(at(entries, 0).kind).toBe('stack'); - expect(at(entries, 0).thread.id).toBe('c'); - expect(at(entries, 0).stack?.ancestors).toHaveLength(2); + expect(at(entries, 0).thread.id).toBe('a'); + expect(at(entries, 0).stack?.descendants).toHaveLength(2); + // DFS order from root: b then c + expect(at(entries, 0).stack?.descendants.map((d) => d.id)).toEqual(['b', 'c']); }); it('handles unrelated threads as separate entries', () => { @@ -125,14 +135,15 @@ describe('buildThreadStacks', () => { const entries = buildThreadStacks(threads); expect(entries).toHaveLength(1); expect(at(entries, 0).kind).toBe('stack'); - // Most recently updated child becomes head - expect(at(entries, 0).thread.id).toBe('child3'); - // All others are ancestors (sorted by recency) - expect(at(entries, 0).stack?.ancestors).toHaveLength(3); - const ancestorIds = at(entries, 0).stack?.ancestors.map((a) => a.id) ?? []; - expect(ancestorIds).toContain('parent'); - expect(ancestorIds).toContain('child1'); - expect(ancestorIds).toContain('child2'); + // Root (parent) is head + expect(at(entries, 0).thread.id).toBe('parent'); + // All children are descendants + expect(at(entries, 0).stack?.descendants).toHaveLength(3); + const descendantIds = at(entries, 0).stack?.descendants.map((d) => d.id) ?? []; + // DFS order: sorted by lastUpdatedDate desc (child3, child2, child1) + expect(descendantIds).toEqual(['child3', 'child2', 'child1']); + // lastActiveDate is child3's date + expect(at(entries, 0).stack?.lastActiveDate).toBe('2025-01-04T00:00:00Z'); // Topology captures the fork: parent has 3 children const topo = at(entries, 0).stack?.topology; @@ -169,38 +180,66 @@ describe('buildThreadStacks', () => { const entries = buildThreadStacks(threads); expect(entries).toHaveLength(1); expect(at(entries, 0).kind).toBe('stack'); - // leaf is most recent - expect(at(entries, 0).thread.id).toBe('leaf'); + // Root is head + expect(at(entries, 0).thread.id).toBe('root'); expect(getStackSize(at(entries, 0))).toBe(4); + // DFS order: branch-a subtree is more recent (leaf=Jan5), so branch-a comes first + // branch-a (Jan 2), leaf (Jan 5), branch-b (Jan 3) + // Wait: children of root sorted by lastUpdatedDate desc. branch-b=Jan3, branch-a=Jan2 + // But branch-a has child leaf=Jan5. Children sort is by their OWN date, not subtree. + // So root's children sorted desc: branch-b (Jan3), branch-a (Jan2) + // DFS: branch-b, then branch-a, leaf + const descendantIds = at(entries, 0).stack?.descendants.map((d) => d.id) ?? []; + expect(descendantIds).toEqual(['branch-b', 'branch-a', 'leaf']); + // Topology captures the full tree including fork at root const topo = at(entries, 0).stack?.topology; expect(topo).toBeDefined(); expect(topo?.rootId).toBe('root'); - // root has two children (branch-a and branch-b) expect(topo?.parentToChildren['root']).toHaveLength(2); expect(topo?.parentToChildren['root']).toContain('branch-a'); expect(topo?.parentToChildren['root']).toContain('branch-b'); - // branch-a has one child (leaf) expect(topo?.parentToChildren['branch-a']).toEqual(['leaf']); - // branch-b is a leaf (no children entry) expect(topo?.parentToChildren['branch-b']).toBeUndefined(); - // child-to-parent edges expect(topo?.childToParent['branch-a']).toBe('root'); expect(topo?.childToParent['branch-b']).toBe('root'); expect(topo?.childToParent['leaf']).toBe('branch-a'); }); + + it('sorts entries by lastActiveDate desc so active stacks float to top', () => { + const threads = [ + // Stack 1: root old, child recent + makeThread({ id: 'stack1-root', lastUpdatedDate: '2025-01-01T00:00:00Z' }), + makeThread({ + id: 'stack1-child', + handoffParentId: 'stack1-root', + lastUpdatedDate: '2025-01-10T00:00:00Z', + }), + // Standalone: mid-range date + makeThread({ id: 'standalone', lastUpdatedDate: '2025-01-05T00:00:00Z' }), + ]; + const entries = buildThreadStacks(threads); + // Stack1 has lastActiveDate Jan 10, standalone has Jan 5 + // Stack1 should come first + expect(at(entries, 0).thread.id).toBe('stack1-root'); + expect(at(entries, 1).thread.id).toBe('standalone'); + }); }); describe('flattenEntries', () => { it('returns threads in order when no stacks expanded', () => { - const threads = [makeThread({ id: 'a' }), makeThread({ id: 'b' })]; + const threads = [ + makeThread({ id: 'a', lastUpdatedDate: '2025-01-02T00:00:00Z' }), + makeThread({ id: 'b', lastUpdatedDate: '2025-01-01T00:00:00Z' }), + ]; const entries = buildThreadStacks(threads); const flat = flattenEntries(entries, new Set()); + // Sorted by lastUpdatedDate desc expect(flat.map((t) => t.id)).toEqual(['a', 'b']); }); - it('includes ancestors when stack is expanded', () => { + it('includes descendants when stack is expanded', () => { const threads = [ makeThread({ id: 'parent', lastUpdatedDate: '2025-01-01T00:00:00Z' }), makeThread({ @@ -210,12 +249,14 @@ describe('flattenEntries', () => { }), ]; const entries = buildThreadStacks(threads); - const headId = at(entries, 0).thread.id; // 'child' is head + const headId = at(entries, 0).thread.id; // 'parent' is head (root) + expect(headId).toBe('parent'); const flat = flattenEntries(entries, new Set([headId])); expect(flat).toHaveLength(2); + expect(flat.map((t) => t.id)).toEqual(['parent', 'child']); }); - it('does not include ancestors when stack is collapsed', () => { + it('does not include descendants when stack is collapsed', () => { const threads = [ makeThread({ id: 'parent', lastUpdatedDate: '2025-01-01T00:00:00Z' }), makeThread({ @@ -228,6 +269,31 @@ describe('flattenEntries', () => { const flat = flattenEntries(entries, new Set()); expect(flat).toHaveLength(1); }); + + it('produces tree-ordered output for fan-out stacks', () => { + const threads = [ + makeThread({ id: 'root', lastUpdatedDate: '2025-01-01T00:00:00Z' }), + makeThread({ + id: 'child-a', + handoffParentId: 'root', + lastUpdatedDate: '2025-01-02T00:00:00Z', + }), + makeThread({ + id: 'child-b', + handoffParentId: 'root', + lastUpdatedDate: '2025-01-03T00:00:00Z', + }), + makeThread({ + id: 'grandchild', + handoffParentId: 'child-a', + lastUpdatedDate: '2025-01-04T00:00:00Z', + }), + ]; + const entries = buildThreadStacks(threads); + const flat = flattenEntries(entries, new Set(['root'])); + // root, then DFS: child-b (Jan3, sorted first), child-a (Jan2), grandchild (Jan4) + expect(flat.map((t) => t.id)).toEqual(['root', 'child-b', 'child-a', 'grandchild']); + }); }); describe('getStackSize', () => { @@ -236,7 +302,7 @@ describe('getStackSize', () => { expect(getStackSize(at(entries, 0))).toBe(1); }); - it('returns count including ancestors for a stack', () => { + it('returns count including descendants for a stack', () => { const threads = [ makeThread({ id: 'a', lastUpdatedDate: '2025-01-01T00:00:00Z' }), makeThread({ id: 'b', handoffParentId: 'a', lastUpdatedDate: '2025-01-02T00:00:00Z' }), @@ -246,3 +312,40 @@ describe('getStackSize', () => { expect(getStackSize(at(entries, 0))).toBe(3); }); }); + +describe('getLastActiveThread', () => { + it('returns the thread itself for a non-stack entry', () => { + const entries = buildThreadStacks([ + makeThread({ id: 'a', lastUpdatedDate: '2025-01-01T00:00:00Z' }), + ]); + expect(getLastActiveThread(at(entries, 0)).id).toBe('a'); + }); + + it('returns the most recently updated thread in a stack', () => { + const threads = [ + makeThread({ id: 'root', lastUpdatedDate: '2025-01-01T00:00:00Z' }), + makeThread({ + id: 'child', + handoffParentId: 'root', + lastUpdatedDate: '2025-01-05T00:00:00Z', + }), + ]; + const entries = buildThreadStacks(threads); + // Head is root, but last active is child + expect(at(entries, 0).thread.id).toBe('root'); + expect(getLastActiveThread(at(entries, 0)).id).toBe('child'); + }); + + it('returns root if root is most recent', () => { + const threads = [ + makeThread({ id: 'root', lastUpdatedDate: '2025-01-10T00:00:00Z' }), + makeThread({ + id: 'child', + handoffParentId: 'root', + lastUpdatedDate: '2025-01-01T00:00:00Z', + }), + ]; + const entries = buildThreadStacks(threads); + expect(getLastActiveThread(at(entries, 0)).id).toBe('root'); + }); +}); diff --git a/src/utils/threadStacks.ts b/src/utils/threadStacks.ts index 39a66da..ee2cc81 100644 --- a/src/utils/threadStacks.ts +++ b/src/utils/threadStacks.ts @@ -6,6 +6,41 @@ export interface ThreadListEntry { stack?: ThreadStack; } +function parseDate(dateStr: string | undefined): number { + if (!dateStr) return 0; + const t = Date.parse(dateStr); + return Number.isFinite(t) ? t : 0; +} + +/** DFS walk from a node's children, producing tree-ordered Thread[] (excludes the node itself). */ +function dfsDescendants( + nodeId: string, + parentToChildrenMap: Record, + threadMap: Map, + visited: Set, +): Thread[] { + const result: Thread[] = []; + const childIds = parentToChildrenMap[nodeId]; + if (!childIds) return result; + + // Sort children by lastUpdatedDate desc for deterministic ordering + const sorted = [...childIds].sort((a, b) => { + const ta = threadMap.get(a); + const tb = threadMap.get(b); + return parseDate(tb?.lastUpdatedDate) - parseDate(ta?.lastUpdatedDate); + }); + + for (const childId of sorted) { + if (visited.has(childId)) continue; + const child = threadMap.get(childId); + if (!child) continue; + visited.add(childId); + result.push(child); + result.push(...dfsDescendants(childId, parentToChildrenMap, threadMap, visited)); + } + return result; +} + export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { const threadMap = new Map(); for (const t of threads) { @@ -29,7 +64,7 @@ export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { } } - // Collect all members of each tree, picking the most recently updated as head + // Collect all members of each tree, using the root as head const inStack = new Set(); const entries: ThreadListEntry[] = []; @@ -68,27 +103,28 @@ export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { } } - // Pick the most recently updated thread as head - chainMembers.sort((a, b) => { - const dateA = new Date(a.lastUpdatedDate || 0).getTime(); - const dateB = new Date(b.lastUpdatedDate || 0).getTime(); - return dateB - dateA; - }); - - const head = chainMembers[0]; - if (!head) continue; - const ancestors = chainMembers.slice(1); - // Mark all as processed for (const member of chainMembers) { inStack.add(member.id); } - if (ancestors.length > 0) { + // Find root: the member with no parent in this stack + let root: Thread | undefined; + for (const member of chainMembers) { + if (!childToParent.has(member.id)) { + root = member; + break; + } + } + if (!root) { + root = chainMembers[0]; + } + if (!root) continue; + + if (chainMembers.length > 1) { // Build topology restricted to this stack's members const childToParentLocal: Record = {}; const parentToChildrenLocal: Record = {}; - let rootId = head.id; for (const member of chainMembers) { const pid = childToParent.get(member.id); @@ -101,33 +137,53 @@ export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { } } - // Find root: the member with no parent in this stack - for (const member of chainMembers) { - if (!childToParentLocal[member.id]) { - rootId = member.id; - break; - } - } - const topology: ThreadStackTopology = { - rootId, + rootId: root.id, childToParent: childToParentLocal, parentToChildren: parentToChildrenLocal, }; + // Compute lastActiveDate (max lastUpdatedDate across all members) + let maxDate = 0; + let lastActiveDateStr: string | undefined; + for (const member of chainMembers) { + const d = parseDate(member.lastUpdatedDate); + if (d > maxDate) { + maxDate = d; + lastActiveDateStr = member.lastUpdatedDate; + } + } + + // Build tree-ordered descendants via DFS from root (excludes root) + const dfsVisited = new Set([root.id]); + const descendants = dfsDescendants(root.id, parentToChildrenLocal, threadMap, dfsVisited); + entries.push({ kind: 'stack', - thread: head, - stack: { head, ancestors, topology }, + thread: root, + stack: { + head: root, + ancestors: descendants, + descendants, + lastActiveDate: lastActiveDateStr, + topology, + }, }); } else { entries.push({ kind: 'thread', - thread: head, + thread: root, }); } } + // Re-sort entries by lastActiveDate desc so active stacks float to top + entries.sort((a, b) => { + const dateA = a.stack?.lastActiveDate ?? a.thread.lastUpdatedDate; + const dateB = b.stack?.lastActiveDate ?? b.thread.lastUpdatedDate; + return parseDate(dateB) - parseDate(dateA); + }); + return entries; } @@ -139,7 +195,7 @@ export function flattenEntries( for (const entry of entries) { result.push(entry.thread); if (entry.kind === 'stack' && entry.stack && expandedStackIds.has(entry.thread.id)) { - result.push(...entry.stack.ancestors); + result.push(...entry.stack.descendants); } } return result; @@ -147,7 +203,22 @@ export function flattenEntries( export function getStackSize(entry: ThreadListEntry): number { if (entry.kind === 'stack' && entry.stack) { - return 1 + entry.stack.ancestors.length; + return 1 + entry.stack.descendants.length; } return 1; } + +/** Get the most recently updated thread in a stack (for kanban column placement, status display). */ +export function getLastActiveThread(entry: ThreadListEntry): Thread { + if (entry.kind !== 'stack' || !entry.stack) return entry.thread; + let best = entry.thread; + let bestDate = parseDate(best.lastUpdatedDate); + for (const d of entry.stack.descendants) { + const date = parseDate(d.lastUpdatedDate); + if (date > bestDate) { + best = d; + bestDate = date; + } + } + return best; +} From 927fcd2f6ad6ec68d7889a464d3a9ba574d474d3 Mon Sep 17 00:00:00 2001 From: Charlie Croom Date: Mon, 23 Feb 2026 12:27:50 -0500 Subject: [PATCH 6/6] refactor: extract buildHandoffGraph to shared/utils and remove deprecated ancestors alias - Extract duplicated graph-building logic (threadMap, childToParent, parentToChildren) from threadChain.ts and threadStacks.ts into a shared buildHandoffGraph() helper in shared/utils.ts - Remove deprecated 'ancestors' field from ThreadStack type (was an alias for 'descendants') Amp-Thread-ID: https://ampcode.com/threads/T-019c8b82-ed31-7099-b07a-2fc28341ad52 Co-authored-by: Amp --- server/lib/threadChain.ts | 16 ++-------------- shared/types.ts | 1 - shared/utils.ts | 32 ++++++++++++++++++++++++++++++++ src/utils/threadStacks.ts | 24 ++---------------------- 4 files changed, 36 insertions(+), 37 deletions(-) diff --git a/server/lib/threadChain.ts b/server/lib/threadChain.ts index 90274aa..38ef9f5 100644 --- a/server/lib/threadChain.ts +++ b/server/lib/threadChain.ts @@ -1,6 +1,7 @@ import { readFile } from 'fs/promises'; import { join } from 'path'; import type { Thread, ThreadChain, ChainThread, ThreadChainNode } from '../../shared/types.js'; +import { buildHandoffGraph } from '../../shared/utils.js'; import { runAmp, stripAnsi } from './utils.js'; import { getThreads } from './threadCrud.js'; import { THREADS_DIR, isHandoffRelationship, type ThreadFile } from './threadTypes.js'; @@ -9,20 +10,7 @@ import { getThreadMetadata, updateLinkedIssue } from './database.js'; export async function getThreadChain(threadId: string): Promise { const { threads } = await getThreads({ limit: 1000 }); - const threadMap = new Map(threads.map((t) => [t.id, t])); - - // Build parent-to-children index from all threads' handoffParentId - const parentToChildren = new Map(); - for (const t of threads) { - if (t.handoffParentId && threadMap.has(t.handoffParentId)) { - const existing = parentToChildren.get(t.handoffParentId); - if (existing) { - existing.push(t.id); - } else { - parentToChildren.set(t.handoffParentId, [t.id]); - } - } - } + const { threadMap, parentToChildren } = buildHandoffGraph(threads); // Walk up to collect ancestor IDs (linear path to root) const ancestorIds: string[] = []; diff --git a/shared/types.ts b/shared/types.ts index b3f30e7..719b598 100644 --- a/shared/types.ts +++ b/shared/types.ts @@ -112,7 +112,6 @@ export interface ThreadStackTopology { export interface ThreadStack { head: Thread; - ancestors: Thread[]; // @deprecated - use descendants instead (kept for backward compat) descendants: Thread[]; // tree-ordered via DFS from root's children lastActiveDate?: string; // most recent lastUpdatedDate across all members topology?: ThreadStackTopology; diff --git a/shared/utils.ts b/shared/utils.ts index a539dd8..5d896ca 100644 --- a/shared/utils.ts +++ b/shared/utils.ts @@ -1,3 +1,5 @@ +import type { Thread } from './types.js'; + let msgCounter = 0; export function generateId(): string { @@ -12,3 +14,33 @@ export function stripAnsi(str: string): string { // eslint-disable-next-line no-control-regex return str.replace(/\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])/g, ''); } + +export interface HandoffGraph { + threadMap: Map; + childToParent: Map; + parentToChildren: Map; +} + +export function buildHandoffGraph(threads: Thread[]): HandoffGraph { + const threadMap = new Map(); + for (const t of threads) { + threadMap.set(t.id, t); + } + + const parentToChildren = new Map(); + const childToParent = new Map(); + + for (const t of threads) { + if (t.handoffParentId && threadMap.has(t.handoffParentId)) { + childToParent.set(t.id, t.handoffParentId); + const existing = parentToChildren.get(t.handoffParentId); + if (existing) { + existing.push(t.id); + } else { + parentToChildren.set(t.handoffParentId, [t.id]); + } + } + } + + return { threadMap, childToParent, parentToChildren }; +} diff --git a/src/utils/threadStacks.ts b/src/utils/threadStacks.ts index ee2cc81..9d2da8b 100644 --- a/src/utils/threadStacks.ts +++ b/src/utils/threadStacks.ts @@ -1,4 +1,5 @@ import type { Thread, ThreadStack, ThreadStackTopology } from '../types'; +import { buildHandoffGraph } from '../../shared/utils'; export interface ThreadListEntry { kind: 'thread' | 'stack'; @@ -42,27 +43,7 @@ function dfsDescendants( } export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { - const threadMap = new Map(); - for (const t of threads) { - threadMap.set(t.id, t); - } - - // Build bidirectional links (only for threads in our current list) - // A parent can have multiple children (fan-out handoffs) - const parentToChildren = new Map(); - const childToParent = new Map(); - - for (const t of threads) { - if (t.handoffParentId && threadMap.has(t.handoffParentId)) { - childToParent.set(t.id, t.handoffParentId); - const existing = parentToChildren.get(t.handoffParentId); - if (existing) { - existing.push(t.id); - } else { - parentToChildren.set(t.handoffParentId, [t.id]); - } - } - } + const { threadMap, childToParent, parentToChildren } = buildHandoffGraph(threads); // Collect all members of each tree, using the root as head const inStack = new Set(); @@ -163,7 +144,6 @@ export function buildThreadStacks(threads: Thread[]): ThreadListEntry[] { thread: root, stack: { head: root, - ancestors: descendants, descendants, lastActiveDate: lastActiveDateStr, topology,