From 8142494401946da4016fd2c1317cf628ed5a4365 Mon Sep 17 00:00:00 2001 From: nftpoetrist <264138787+nftpoetrist@users.noreply.github.com> Date: Sun, 30 Aug 2026 01:55:24 +0300 Subject: [PATCH] fix(tui): render nested todo subtasks via the parent field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../src/__tests__/turnControllerTodos.test.ts | 38 +++++++++++++ ui-tui/src/app/turnController.ts | 8 ++- ui-tui/src/components/todoPanel.tsx | 14 ++--- ui-tui/src/lib/todo.test.ts | 53 ++++++++++++++++++- ui-tui/src/lib/todo.ts | 50 +++++++++++++++++ ui-tui/src/types.ts | 2 + 6 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 ui-tui/src/__tests__/turnControllerTodos.test.ts diff --git a/ui-tui/src/__tests__/turnControllerTodos.test.ts b/ui-tui/src/__tests__/turnControllerTodos.test.ts new file mode 100644 index 0000000000..74d9a6f33b --- /dev/null +++ b/ui-tui/src/__tests__/turnControllerTodos.test.ts @@ -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' }]) + }) +}) diff --git a/ui-tui/src/app/turnController.ts b/ui-tui/src/app/turnController.ts index b81ae4d154..edd67e5281 100644 --- a/ui-tui/src/app/turnController.ts +++ b/ui-tui/src/app/turnController.ts @@ -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)) diff --git a/ui-tui/src/components/todoPanel.tsx b/ui-tui/src/components/todoPanel.tsx index 41196b060b..4bd1955949 100644 --- a/ui-tui/src/components/todoPanel.tsx +++ b/ui-tui/src/components/todoPanel.tsx @@ -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 && ( - {todos.map(todo => { + {todoTree(todos).map(([todo, depth]) => { const tone = todoTone(todo.status) const color = rowColor(t, todo.status) return ( - - {todoGlyph(todo.status)} - {todo.content} - + + + {todoGlyph(todo.status)} + {todo.content} + + ) })} diff --git a/ui-tui/src/lib/todo.test.ts b/ui-tui/src/lib/todo.test.ts index bf8befa2c6..4b1a3933ea 100644 --- a/ui-tui/src/lib/todo.test.ts +++ b/ui-tui/src/lib/todo.test.ts @@ -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] + ]) + }) +}) diff --git a/ui-tui/src/lib/todo.ts b/ui-tui/src/lib/todo.ts index 1846d02fe6..375b92cbb9 100644 --- a/ui-tui/src/lib/todo.ts +++ b/ui-tui/src/lib/todo.ts @@ -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() + 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() + + 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 +} diff --git a/ui-tui/src/types.ts b/ui-tui/src/types.ts index 6e225363cb..4da7cb5882 100644 --- a/ui-tui/src/types.ts +++ b/ui-tui/src/types.ts @@ -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' }