fix(tui): render nested todo subtasks via the parent field
CLI, ACP, and the desktop app all got nested-subtask rendering (the optional `parent` field on a todo item), but the TUI never did. Its TodoItem type had no `parent` field, parseTodos() in turnController.ts dropped it even if the tool payload sent it, and TodoPanel rendered the list with a flat map() and a single fixed indent — a session using nested subtasks showed every subtask at the same visual level as its parent, with no hierarchy cue, in the terminal UI. - types.ts: add the optional `parent` field to TodoItem, matching apps/desktop/src/lib/todos.ts's TodoItem exactly. - turnController.ts: parseTodos() now preserves parent (trimmed, dropped if empty or self-referential), the same normalization desktop's parseArray() applies. - lib/todo.ts: port todoTree() from apps/desktop/src/lib/todos.ts verbatim — same DFS-with-depth algorithm, same dangling/cycle handling, so both surfaces render identical hierarchy from the same `parent` field. - todoPanel.tsx: render todoTree(todos) instead of a flat map(), with per-row indentation scaled by depth (capped at 4 levels, mirroring desktop's status-row.tsx cap).
This commit is contained in:
38
ui-tui/src/__tests__/turnControllerTodos.test.ts
Normal file
38
ui-tui/src/__tests__/turnControllerTodos.test.ts
Normal file
@@ -0,0 +1,38 @@
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import { turnController } from '../app/turnController.js'
|
||||
import { getTurnState, resetTurnState } from '../app/turnStore.js'
|
||||
|
||||
// turnController.recordTodos() parses the raw `todo` tool payload into
|
||||
// TodoItem[]. Nested subtasks (apps/desktop's `parent` field) must survive
|
||||
// this parse — the TUI todo panel renders hierarchy from it via todoTree().
|
||||
describe('turnController.recordTodos — preserves the parent field', () => {
|
||||
beforeEach(() => {
|
||||
resetTurnState()
|
||||
turnController.fullReset()
|
||||
})
|
||||
|
||||
it('keeps parent on a valid nested subtask', () => {
|
||||
turnController.recordTodos([
|
||||
{ content: 'Ship feature', id: 'wp1', status: 'in_progress' },
|
||||
{ content: 'Write tests', id: 't1', parent: 'wp1', status: 'pending' }
|
||||
])
|
||||
|
||||
expect(getTurnState().todos).toEqual([
|
||||
{ content: 'Ship feature', id: 'wp1', status: 'in_progress' },
|
||||
{ content: 'Write tests', id: 't1', parent: 'wp1', status: 'pending' }
|
||||
])
|
||||
})
|
||||
|
||||
it('drops a self-referential parent instead of keeping a self-loop', () => {
|
||||
turnController.recordTodos([{ content: 'x', id: 'a', parent: 'a', status: 'pending' }])
|
||||
|
||||
expect(getTurnState().todos).toEqual([{ content: 'x', id: 'a', status: 'pending' }])
|
||||
})
|
||||
|
||||
it('omits parent entirely when absent, matching pre-nesting payloads', () => {
|
||||
turnController.recordTodos([{ content: 'x', id: 'a', status: 'pending' }])
|
||||
|
||||
expect(getTurnState().todos).toEqual([{ content: 'x', id: 'a', status: 'pending' }])
|
||||
})
|
||||
})
|
||||
@@ -66,10 +66,14 @@ const parseTodos = (value: unknown): null | TodoItem[] => {
|
||||
return null
|
||||
}
|
||||
|
||||
const id = String(row.id ?? '').trim()
|
||||
const parent = String(row.parent ?? '').trim()
|
||||
|
||||
return {
|
||||
content: String(row.content ?? '').trim(),
|
||||
id: String(row.id ?? '').trim(),
|
||||
status
|
||||
id,
|
||||
status,
|
||||
...(parent && parent !== id ? { parent } : {})
|
||||
}
|
||||
})
|
||||
.filter((item): item is TodoItem => Boolean(item?.id && item.content))
|
||||
|
||||
@@ -2,7 +2,7 @@ import { Box, Text } from '@hermes/ink'
|
||||
import { memo, useState } from 'react'
|
||||
|
||||
import { countPendingTodos } from '../lib/liveProgress.js'
|
||||
import { todoGlyph, todoTone } from '../lib/todo.js'
|
||||
import { todoGlyph, todoTone, todoTree } from '../lib/todo.js'
|
||||
import type { Theme } from '../theme.js'
|
||||
import type { TodoItem } from '../types.js'
|
||||
|
||||
@@ -75,15 +75,17 @@ export const TodoPanel = memo(function TodoPanel({
|
||||
|
||||
{!effectiveCollapsed && (
|
||||
<Box flexDirection="column" marginLeft={2}>
|
||||
{todos.map(todo => {
|
||||
{todoTree(todos).map(([todo, depth]) => {
|
||||
const tone = todoTone(todo.status)
|
||||
const color = rowColor(t, todo.status)
|
||||
|
||||
return (
|
||||
<Text color={color} dim={tone === 'dim'} key={todo.id}>
|
||||
<Text color={color}>{todoGlyph(todo.status)} </Text>
|
||||
{todo.content}
|
||||
</Text>
|
||||
<Box key={todo.id} marginLeft={Math.min(depth, 4) * 2}>
|
||||
<Text color={color} dim={tone === 'dim'}>
|
||||
<Text color={color}>{todoGlyph(todo.status)} </Text>
|
||||
{todo.content}
|
||||
</Text>
|
||||
</Box>
|
||||
)
|
||||
})}
|
||||
</Box>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { todoGlyph, todoTone } from './todo.js'
|
||||
import { todoGlyph, todoTone, todoTree } from './todo.js'
|
||||
|
||||
describe('todoGlyph', () => {
|
||||
it('uses fixed-width ASCII markers so the active row does not render wide or emoji-like', () => {
|
||||
@@ -19,3 +19,54 @@ describe('todoTone', () => {
|
||||
expect(todoTone('in_progress')).toBe('active')
|
||||
})
|
||||
})
|
||||
|
||||
describe('todoTree', () => {
|
||||
it('orders parents before children with depths', () => {
|
||||
const tree = todoTree([
|
||||
{ content: 'WP1', id: 'wp1', status: 'in_progress' },
|
||||
{ content: 'WP2', id: 'wp2', status: 'pending' },
|
||||
{ content: 'T1', id: 't1', parent: 'wp1', status: 'pending' },
|
||||
{ content: 'T2', id: 't2', parent: 'wp1', status: 'pending' }
|
||||
])
|
||||
|
||||
expect(tree.map(([t, d]) => [t.id, d])).toEqual([
|
||||
['wp1', 0],
|
||||
['t1', 1],
|
||||
['t2', 1],
|
||||
['wp2', 0]
|
||||
])
|
||||
})
|
||||
|
||||
it('degrades dangling and self parents to roots', () => {
|
||||
const tree = todoTree([
|
||||
{ content: 'A', id: 'a', parent: 'ghost', status: 'pending' },
|
||||
{ content: 'B', id: 'b', parent: 'b', status: 'pending' }
|
||||
])
|
||||
|
||||
expect(tree.map(([t, d]) => [t.id, d])).toEqual([
|
||||
['a', 0],
|
||||
['b', 0]
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps cycle members instead of dropping them', () => {
|
||||
const tree = todoTree([
|
||||
{ content: 'A', id: 'a', parent: 'b', status: 'pending' },
|
||||
{ content: 'B', id: 'b', parent: 'a', status: 'pending' }
|
||||
])
|
||||
|
||||
expect(tree.map(([t]) => t.id).sort()).toEqual(['a', 'b'])
|
||||
})
|
||||
|
||||
it('flattens a todo list with no parents unchanged, all at depth 0', () => {
|
||||
const tree = todoTree([
|
||||
{ content: 'A', id: 'a', status: 'pending' },
|
||||
{ content: 'B', id: 'b', status: 'completed' }
|
||||
])
|
||||
|
||||
expect(tree.map(([t, d]) => [t.id, d])).toEqual([
|
||||
['a', 0],
|
||||
['b', 0]
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,3 +7,53 @@ export const todoGlyph = (status: TodoItem['status']) =>
|
||||
|
||||
export const todoTone = (status: TodoItem['status']): TodoTone =>
|
||||
status === 'in_progress' ? 'active' : status === 'pending' ? 'body' : 'dim'
|
||||
|
||||
/** DFS order of a (possibly nested) todo list: [item, depth] pairs, parents
|
||||
* before children. Dangling/cyclic parents degrade to depth 0. Mirrors
|
||||
* apps/desktop/src/lib/todos.ts's todoTree() so both surfaces render the
|
||||
* same hierarchy from the same `parent` field. */
|
||||
export function todoTree(todos: readonly TodoItem[]): [TodoItem, number][] {
|
||||
const ids = new Set(todos.map(t => t.id))
|
||||
const kids = new Map<string, TodoItem[]>()
|
||||
const roots: TodoItem[] = []
|
||||
|
||||
for (const t of todos) {
|
||||
if (t.parent && ids.has(t.parent) && t.parent !== t.id) {
|
||||
const list = kids.get(t.parent) ?? []
|
||||
list.push(t)
|
||||
kids.set(t.parent, list)
|
||||
} else {
|
||||
roots.push(t)
|
||||
}
|
||||
}
|
||||
|
||||
const out: [TodoItem, number][] = []
|
||||
const seen = new Set<string>()
|
||||
|
||||
const walk = (item: TodoItem, depth: number) => {
|
||||
if (seen.has(item.id)) {
|
||||
return
|
||||
}
|
||||
|
||||
seen.add(item.id)
|
||||
out.push([item, depth])
|
||||
|
||||
for (const kid of kids.get(item.id) ?? []) {
|
||||
walk(kid, depth + 1)
|
||||
}
|
||||
}
|
||||
|
||||
for (const root of roots) {
|
||||
walk(root, 0)
|
||||
}
|
||||
|
||||
// Cycle members never reach a root — append them flat so nothing is lost.
|
||||
for (const t of todos) {
|
||||
if (!seen.has(t.id)) {
|
||||
seen.add(t.id)
|
||||
out.push([t, 0])
|
||||
}
|
||||
}
|
||||
|
||||
return out
|
||||
}
|
||||
|
||||
@@ -9,6 +9,8 @@ export interface ActiveTool {
|
||||
export interface TodoItem {
|
||||
content: string
|
||||
id: string
|
||||
/** Optional id of another item — renders this as a nested subtask. */
|
||||
parent?: string
|
||||
status: 'cancelled' | 'completed' | 'in_progress' | 'pending'
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user