fix(tui): shared walking-viewport policy for collapsed todos

The first pass anchored a slice on the active task, which still showed completed rows, kept the active item mid-window, and gave the two views divergent selection logic. Per reviewer, replace it with one shared policy both collapsed views run.

selectCollapsedTodos (todo.ts) omits completed/abandoned, pulls every active task (in_progress or subagent-matched pending) to the head in todo order, fills remaining rows with following pending tasks, and emits '… N more active todos' when active work alone exceeds the cap; it falls back to closed tasks for a settled phase so HUD persistence still renders. renderTreeList gains a trailingSummary primitive so item selection lives in the todo domain. The transient tool result reaches live subagent matches via setActiveTodoDescriptionsProvider, wired from interactive mode's observer registry, so both views share the active set.

Fixes #5873
This commit is contained in:
roboomp
2026-07-17 18:03:13 +00:00
parent e6fd29ec21
commit 939d6761f7
6 changed files with 244 additions and 132 deletions
+1 -1
View File
@@ -4,7 +4,7 @@
### Fixed
- Fixed collapsed todo views hiding the in-progress task in large phases. Both the transient `Todo` tool result and the sticky `Todos` HUD now anchor their collapsed window on the active task (or a subagent-matched pending task), keeping it visible with two-sided `… N more` summaries regardless of its position ([#5873](https://github.com/can1357/oh-my-pi/issues/5873)).
- Fixed collapsed todo views hiding the in-progress task in large phases. Both the transient `Todo` tool result and the sticky `Todos` HUD now share one walking-viewport policy: completed/abandoned tasks are omitted, every active task (the in-progress one, or a pending task a live subagent is executing) is pulled to the head in todo order, remaining rows fill with the following pending tasks, and an explicit `… N more active todos` summary is shown when active work alone exceeds the preview cap ([#5873](https://github.com/can1357/oh-my-pi/issues/5873)).
## [17.0.2] - 2026-07-17
@@ -123,7 +123,12 @@ import type { LspStartupServerInfo } from "../tools";
import { normalizeLocalScheme } from "../tools/path-utils";
import { replaceTabs, TRUNCATE_LENGTHS, truncateToWidth } from "../tools/render-utils";
import { setAutoQaConsentHandler } from "../tools/report-tool-issue";
import { formatPhaseDisplayName, todoMatchesAnyDescription } from "../tools/todo";
import {
formatPhaseDisplayName,
selectCollapsedTodos,
setActiveTodoDescriptionsProvider,
todoMatchesAnyDescription,
} from "../tools/todo";
import { ToolError } from "../tools/tool-errors";
import { vocalizer } from "../tts/vocalizer";
import { renderTreeList } from "../tui/tree-list";
@@ -951,6 +956,9 @@ export class InteractiveMode implements InteractiveModeContext {
this.#observerRegistry.onChange(kind => {
this.#scheduleObserverUiSync(kind);
});
// Let the transient todo tool result light up pending todos executed by a
// live subagent, matching the sticky HUD's active set (#5873).
setActiveTodoDescriptionsProvider(() => this.#getActiveSubagentDescriptions());
// Load initial todos
await this.#loadTodoList();
@@ -1895,24 +1903,27 @@ export class InteractiveMode implements InteractiveModeContext {
const isMatched = (todo: TodoItem): boolean =>
activeDescs.length > 0 && todoMatchesAnyDescription(todo.content, activeDescs);
// Task subtree for a phase. Collapsed previews the first open tasks — the
// stage's `done/total` makes the hidden count obvious, so there is no
// "… more" row; expanded lists every task.
// Task subtree for a phase. Collapsed runs the shared walking-viewport
// policy (completed/abandoned omitted, active work pulled to the head,
// then following pending tasks) so the HUD and the transient tool result
// can never disagree about the current work (#5873). Expanded lists all.
const renderTasks = (phase: TodoPhase): string[] => {
const open = phase.tasks.filter(t => t.status === "pending" || t.status === "in_progress");
const base = expanded ? phase.tasks : open.length > 0 ? open : phase.tasks;
// Anchor the collapsed window on the active work — the in-progress task,
// else the first subagent-matched pending task — so it stays visible
// instead of being dropped by a fixed head slice.
let anchorIdx = base.findIndex(t => t.status === "in_progress");
if (anchorIdx < 0) anchorIdx = base.findIndex(t => isMatched(t));
if (expanded) {
return renderTreeList(
{
items: phase.tasks,
expanded: true,
renderItem: todo => this.#formatTodoLine(todo, "", isMatched(todo)),
},
theme,
);
}
const selection = selectCollapsedTodos(phase.tasks, isMatched, activeTaskCap);
return renderTreeList(
{
items: base,
expanded,
maxCollapsed: activeTaskCap,
items: selection.items,
itemType: "task",
anchorIndex: anchorIdx >= 0 ? anchorIdx : undefined,
trailingSummary: selection.summary,
renderItem: todo => this.#formatTodoLine(todo, "", isMatched(todo)),
},
theme,
+123 -17
View File
@@ -12,7 +12,7 @@ import type { ToolSession } from "../sdk";
import type { SessionEntry } from "../session/session-entries";
import { framedBlock, renderStatusLine, renderTreeList } from "../tui";
import { normalizePathLikeInput, resolveToCwd } from "./path-utils";
import { formatErrorDetail, PREVIEW_LIMITS } from "./render-utils";
import { formatErrorDetail, formatMoreItems, PREVIEW_LIMITS, pluralize } from "./render-utils";
// =============================================================================
// Types
@@ -214,6 +214,77 @@ export function todoMatchesAnyDescription(content: string, descriptions: readonl
return false;
}
/**
* A todo the collapsed viewport treats as current work: the literal
* `in_progress` task or a pending task a live subagent is executing. Both
* collapsed views (transient tool result + sticky HUD) run this same policy so
* they can never disagree about what the agent is doing (#5873).
*/
function isActiveTodo<T extends { status: TodoStatus }>(task: T, isMatched: (task: T) => boolean): boolean {
return task.status === "in_progress" || (task.status === "pending" && isMatched(task));
}
/** Result of {@link selectCollapsedTodos}: the rows to render plus an optional
* summary line (empty string ⇒ no summary row). */
export interface CollapsedTodoSelection<T> {
items: T[];
summary: string;
}
/**
* Walking-viewport selection for a phase's collapsed todo preview (#5873).
*
* Policy, applied to `tasks` in todo order:
* 1. While the phase has open work, completed/abandoned tasks are omitted. A
* phase with no open tasks left falls back to its closed tasks so the sticky
* HUD's closed-todo persistence still has something to render.
* 2. Every active task (in-progress, or pending matched to a live subagent) is
* placed at the head in stable todo order — never dropped for lying outside
* an ordinary window.
* 3. Remaining rows up to `cap` are filled with the pending tasks that follow
* the first active one, in todo order (falling back to leading pending tasks
* when no active task exists), so a freshly-promoted task leads the preview.
* 4. When active tasks alone exceed `cap`, only the first `cap` active tasks are
* shown and the summary counts the hidden *active* todos, never replacing
* them with unrelated pending rows.
*
* The summary otherwise counts the remaining tasks in the display base. Returns
* the whole base with an empty summary when it already fits.
*/
export function selectCollapsedTodos<T extends { status: TodoStatus }>(
tasks: T[],
isMatched: (task: T) => boolean,
cap: number,
): CollapsedTodoSelection<T> {
const open = tasks.filter(task => task.status === "pending" || task.status === "in_progress");
// No open work: fall back to the closed tasks so a settled phase still
// renders (HUD closed-todo persistence). Closed tasks are never active.
const base = open.length > 0 ? open : tasks;
if (base.length <= cap) return { items: base, summary: "" };
const active = base.filter(task => isActiveTodo(task, isMatched));
if (active.length >= cap) {
const hiddenActive = active.length - cap;
return {
items: active.slice(0, cap),
summary: hiddenActive > 0 ? `… ${hiddenActive} more active ${pluralize("todo", hiddenActive)}` : "",
};
}
// Fill trailing rows with tasks following the first active one, so the
// promoted/current task leads and its successors follow in todo order.
const firstActiveIdx = active.length > 0 ? base.indexOf(active[0]) : 0;
const fill: T[] = [];
for (let i = firstActiveIdx; i < base.length && active.length + fill.length < cap; i++) {
const task = base[i];
if (isActiveTodo(task, isMatched)) continue;
fill.push(task);
}
const items = [...active, ...fill];
const hidden = base.length - items.length;
return { items, summary: hidden > 0 ? formatMoreItems(hidden, "todo") : "" };
}
function resolveTaskOrError(
phases: TodoPhase[],
content: string | undefined,
@@ -755,6 +826,7 @@ function formatTodoLine(
prefix: string,
completionKeys: Set<string>,
frame: number | undefined,
matched = false,
): string {
const checkbox = uiTheme.checkbox;
switch (item.status) {
@@ -771,7 +843,9 @@ function formatTodoLine(
case "abandoned":
return uiTheme.fg("error", `${prefix}${checkbox.unchecked} ${strikethroughText(item.content)}`);
default:
return uiTheme.fg("dim", `${prefix}${checkbox.unchecked} ${item.content}`);
// A pending todo lit by a live subagent match renders accent, matching
// the sticky HUD's convention (#5873).
return uiTheme.fg(matched ? "accent" : "dim", `${prefix}${checkbox.unchecked} ${item.content}`);
}
}
@@ -822,6 +896,21 @@ function formatPhaseSummary(phase: TodoPhase, oneBasedIndex: number, uiTheme: Th
return `${name}${uiTheme.fg("dim", ` ${done}/${total}`)}`;
}
/**
* Live subagent descriptions the transient tool result uses to detect
* pending todos being executed by an in-flight subagent, so its collapsed
* viewport surfaces the same active work the sticky HUD does (#5873). Wired
* once by interactive mode from its observer registry; returns `[]` outside an
* interactive session (tests, SDK, transcript rebuilds), where only literal
* `in_progress` counts as active.
*/
let activeTodoDescriptionsProvider: () => readonly string[] = () => [];
/** Wire the live-subagent description source for {@link todoToolRenderer}. */
export function setActiveTodoDescriptionsProvider(provider: () => readonly string[]): void {
activeTodoDescriptionsProvider = provider;
}
export const todoToolRenderer = {
renderCall(args: TodoRenderArgs, options: RenderResultOptions, uiTheme: Theme): Component {
// `args` is the raw partially-parsed JSON from the streaming tool-call
@@ -903,6 +992,12 @@ export const todoToolRenderer = {
// a single task flip doesn't redraw every phase's full task list. The
// manual expand toggle (and the no-signal fallback) still shows all.
const touched = expanded || !multiPhase ? null : computeTouchedPhases(args, phases, completedTasks);
// A pending todo counts as active work when an in-flight subagent is
// executing it — the transient result surfaces the same active set the
// sticky HUD does (#5873). Empty outside an interactive session.
const activeDescs = expanded ? [] : activeTodoDescriptionsProvider();
const isMatched = (task: TodoItem): boolean =>
activeDescs.length > 0 && todoMatchesAnyDescription(task.content, activeDescs);
const bodyLines: string[] = [];
for (let p = 0; p < phases.length; p++) {
const phase = phases[p];
@@ -914,21 +1009,32 @@ export const todoToolRenderer = {
bodyLines.push(uiTheme.fg("accent", chalk.bold(formatPhaseDisplayName(phase.name, p + 1))));
}
const completionKeys = completionKeysByPhase.get(phase.name) ?? EMPTY_COMPLETION_KEYS;
// Anchor the collapsed window on the in-progress task so it stays
// visible even mid-phase; tail truncation would otherwise hide it.
const activeIdx = phase.tasks.findIndex(task => task.status === "in_progress");
const treeLines = renderTreeList(
{
items: phase.tasks,
expanded,
maxCollapsed: PREVIEW_LIMITS.COLLAPSED_ITEMS,
itemType: "todo",
truncateFrom: "start",
anchorIndex: activeIdx >= 0 ? activeIdx : undefined,
renderItem: todo => formatTodoLine(todo, uiTheme, "", completionKeys, spinnerFrame),
},
uiTheme,
);
// Collapsed: walking viewport — completed/abandoned omitted, active
// work (in-progress / subagent-matched) pulled to the head, then
// following pending tasks (#5873). Expanded: every task in order.
const treeLines = expanded
? renderTreeList(
{
items: phase.tasks,
expanded,
itemType: "todo",
renderItem: todo => formatTodoLine(todo, uiTheme, "", completionKeys, spinnerFrame),
},
uiTheme,
)
: (() => {
const selection = selectCollapsedTodos(phase.tasks, isMatched, PREVIEW_LIMITS.COLLAPSED_ITEMS);
return renderTreeList(
{
items: selection.items,
itemType: "todo",
trailingSummary: selection.summary,
renderItem: todo =>
formatTodoLine(todo, uiTheme, "", completionKeys, spinnerFrame, isMatched(todo)),
},
uiTheme,
);
})();
for (const line of treeLines) {
bodyLines.push(`${indent}${line}`);
}
+17 -30
View File
@@ -18,13 +18,13 @@ export interface TreeListOptions<T> {
maxCollapsedLines?: number;
itemType?: string;
truncateFrom?: "start" | "end";
/** Index (into `items`) of the task that MUST stay visible when collapsed —
* the in-progress task or a subagent-matched pending task. When set, the
* collapsed window slides to include it, emitting leading/trailing
* `… N more` summaries on whichever side is truncated. Bounds the preview by
* `maxCollapsed` items; ignores `maxCollapsedLines`. Ignored when expanded or
* when everything already fits. */
anchorIndex?: number;
/** Caller-supplied trailing summary line. When set (and not expanded),
* `renderTreeList` renders exactly the provided `items` (the caller has
* already applied its own selection/cap) and appends this text as the
* final `└` row, with the last item using `├`. Empty string renders the
* items with no summary. Bypasses the built-in truncation/`maxCollapsed`
* path. */
trailingSummary?: string;
/** Called once per item with `isLast: false` during budget calculation;
* line count MUST NOT vary based on `isLast`. */
renderItem: (item: T, context: TreeContext) => string | string[];
@@ -43,27 +43,14 @@ export function renderTreeList<T>(options: TreeListOptions<T>, theme: Theme): st
const maxItems = expanded ? items.length : Math.min(items.length, maxCollapsed);
const linesBudget = !expanded && maxCollapsedLines !== undefined ? maxCollapsedLines : Infinity;
// Anchored collapse: keep a specific item (in-progress / subagent-matched
// task) visible by sliding a `maxItems`-wide window over it, with two-sided
// `… N more` summaries. Edge truncation can only keep a head or tail slice,
// so an active item in the middle would otherwise vanish.
if (
!expanded &&
options.anchorIndex !== undefined &&
options.anchorIndex >= 0 &&
options.anchorIndex < items.length &&
items.length > maxItems
) {
const half = Math.floor((maxItems - 1) / 2);
const winStart = Math.min(Math.max(options.anchorIndex - half, 0), items.length - maxItems);
const winEnd = winStart + maxItems;
const before = winStart;
const after = items.length - winEnd;
// Caller-driven collapse: render exactly the provided items (the caller
// already picked/capped them) plus an optional trailing summary row. The
// walking-viewport todo policy uses this so item selection lives in the
// todo domain, not here.
if (!expanded && options.trailingSummary !== undefined) {
const summary = options.trailingSummary;
const lines: string[] = [];
if (before > 0) {
lines.push(`${theme.fg("dim", theme.tree.branch)} ${theme.fg("muted", formatMoreItems(before, itemType))}`);
}
for (let i = winStart; i < winEnd; i++) {
for (let i = 0; i < items.length; i++) {
const rendered = renderItem(items[i], {
index: i,
isLast: false,
@@ -74,7 +61,7 @@ export function renderTreeList<T>(options: TreeListOptions<T>, theme: Theme): st
});
const itemLines = Array.isArray(rendered) ? rendered : rendered ? [rendered] : [];
if (itemLines.length === 0) continue;
const isLast = after === 0 && i === winEnd - 1;
const isLast = summary === "" && i === items.length - 1;
const prefix = `${theme.fg("dim", getTreeBranch(isLast, theme))} `;
const continuePrefix = `${theme.fg("dim", getTreeContinuePrefix(isLast, theme))}`;
lines.push(`${prefix}${replaceTabs(itemLines[0]!)}`);
@@ -82,8 +69,8 @@ export function renderTreeList<T>(options: TreeListOptions<T>, theme: Theme): st
lines.push(`${continuePrefix}${replaceTabs(itemLines[j]!)}`);
}
}
if (after > 0) {
lines.push(`${theme.fg("dim", theme.tree.last)} ${theme.fg("muted", formatMoreItems(after, itemType))}`);
if (summary !== "") {
lines.push(`${theme.fg("dim", theme.tree.last)} ${theme.fg("muted", summary)}`);
}
return lines;
}
+77 -2
View File
@@ -6,7 +6,9 @@ import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
import {
nextActionableTask,
resolveTodoMarkdownPath,
selectCollapsedTodos,
TODO_STRIKE_HOLD_FRAMES,
type TodoItem,
type TodoPhase,
TodoTool,
todoMatchesAnyDescription,
@@ -420,8 +422,9 @@ describe("todoToolRenderer.renderResult phase collapsing", () => {
task: "a1",
});
const rendered = Bun.stripANSI(component.render(100).join("\n"));
// Active phase renders its full task list.
expect(rendered).toContain("a1");
// Active phase's collapsed viewport omits the completed task and shows the
// promoted current one (#5873).
expect(rendered).not.toContain("a1");
expect(rendered).toContain("a2");
// Untouched phases collapse: headers + progress counts, no task contents.
expect(rendered).toContain("II. Beta");
@@ -465,6 +468,78 @@ describe("todoToolRenderer.renderResult phase collapsing", () => {
});
});
describe("selectCollapsedTodos walking viewport (#5873)", () => {
const mk = (n: number, inProgress: number[]): TodoItem[] =>
Array.from({ length: n }, (_, i) => ({
content: `Task ${i + 1}`,
status: inProgress.includes(i + 1) ? "in_progress" : "pending",
}));
const never = () => false;
const contents = (sel: { items: TodoItem[] }) => sel.items.map(t => t.content);
it("starts at the sole in-progress task and fills with following tasks", () => {
const sel = selectCollapsedTodos(mk(14, [6]), never, 8);
expect(contents(sel)).toEqual([
"Task 6",
"Task 7",
"Task 8",
"Task 9",
"Task 10",
"Task 11",
"Task 12",
"Task 13",
]);
expect(sel.summary).toContain("6 more todos");
});
it("omits completed and abandoned tasks in collapsed mode", () => {
const tasks: TodoItem[] = [
{ content: "done", status: "completed" },
{ content: "dropped", status: "abandoned" },
{ content: "current", status: "in_progress" },
{ content: "next", status: "pending" },
];
const sel = selectCollapsedTodos(tasks, never, 5);
expect(contents(sel)).toEqual(["current", "next"]);
expect(sel.summary).toBe("");
});
it("places every subagent-matched todo at the head in todo order", () => {
const tasks = mk(14, []); // all pending
const matched = (t: TodoItem) => t.content === "Task 3" || t.content === "Task 9";
const sel = selectCollapsedTodos(tasks, matched, 5);
// Both matched actives lead, then following pending fill from the first active.
expect(contents(sel).slice(0, 2)).toEqual(["Task 3", "Task 9"]);
expect(contents(sel)).toHaveLength(5);
});
it("caps active todos and counts the hidden actives in the summary", () => {
const tasks = mk(10, []);
const matched = (t: TodoItem) =>
["Task 1", "Task 2", "Task 3", "Task 4", "Task 5", "Task 6", "Task 7"].includes(t.content);
const sel = selectCollapsedTodos(tasks, matched, 5);
expect(contents(sel)).toEqual(["Task 1", "Task 2", "Task 3", "Task 4", "Task 5"]);
expect(sel.summary).toBe("… 2 more active todos");
// No unrelated pending rows leak in.
expect(contents(sel).some(c => ["Task 8", "Task 9", "Task 10"].includes(c))).toBe(false);
});
it("returns the whole open set with no summary when it fits", () => {
const sel = selectCollapsedTodos(mk(3, [2]), never, 5);
expect(contents(sel)).toEqual(["Task 1", "Task 2", "Task 3"]);
expect(sel.summary).toBe("");
});
it("falls back to closed tasks when the phase has no open work", () => {
const tasks: TodoItem[] = [
{ content: "done a", status: "completed" },
{ content: "done b", status: "completed" },
];
const sel = selectCollapsedTodos(tasks, never, 5);
expect(contents(sel)).toEqual(["done a", "done b"]);
});
});
describe("todoToolRenderer.renderCall malformed-args regression (#2005)", () => {
// Reporter saw `TypeError: args?.ops?.map is not a function` against
// Xiaomi Token Plan's Anthropic protocol because `parseStreamingJson`
@@ -288,71 +288,4 @@ describe("renderTreeList maxCollapsedLines", () => {
expect(collapsed[3]).toBe("└ d");
expect(collapsed[4]).toBe(" d2");
});
describe("anchorIndex", () => {
const tasks = Array.from({ length: 14 }, (_, i) => `Task ${i + 1}`);
it("keeps a mid-list anchored item visible with two-sided summaries", () => {
const collapsed = renderTreeList(
{ items: tasks, expanded: false, maxCollapsed: 8, itemType: "todo", anchorIndex: 5, renderItem: t => t },
stubTheme,
);
expect(collapsed.some(l => l.includes("Task 6"))).toBe(true);
expect(collapsed[0]).toContain("more todos");
expect(collapsed.at(-1)).toContain("more todos");
// Window holds exactly maxCollapsed items plus both summary rows.
expect(collapsed).toHaveLength(10);
});
it("counts hidden items correctly on each side", () => {
const collapsed = renderTreeList(
{ items: tasks, expanded: false, maxCollapsed: 8, itemType: "todo", anchorIndex: 5, renderItem: t => t },
stubTheme,
);
// anchor 5, half = floor(7/2) = 3 → window [2,10): Task 3..Task 10.
expect(collapsed[0]).toContain("2 more todos");
expect(collapsed.at(-1)).toContain("4 more todos");
});
it("clamps the window to the tail when the anchor is near the end", () => {
const collapsed = renderTreeList(
{ items: tasks, expanded: false, maxCollapsed: 8, itemType: "todo", anchorIndex: 13, renderItem: t => t },
stubTheme,
);
expect(collapsed.some(l => l.includes("Task 14"))).toBe(true);
expect(collapsed[0]).toContain("6 more todos");
// No trailing summary — the window reaches the last item.
expect(collapsed.at(-1)).not.toContain("more");
expect(collapsed.at(-1)).toContain("└");
});
it("clamps the window to the head when the anchor is near the start", () => {
const collapsed = renderTreeList(
{ items: tasks, expanded: false, maxCollapsed: 8, itemType: "todo", anchorIndex: 0, renderItem: t => t },
stubTheme,
);
expect(collapsed.some(l => l.includes("Task 1"))).toBe(true);
expect(collapsed[0]).not.toContain("more");
expect(collapsed.at(-1)).toContain("6 more todos");
});
it("ignores the anchor when every item already fits", () => {
const items = ["a", "b", "c"];
const collapsed = renderTreeList(
{ items, expanded: false, maxCollapsed: 8, itemType: "todo", anchorIndex: 1, renderItem: t => t },
stubTheme,
);
expect(collapsed).toHaveLength(3);
expect(collapsed.some(l => l.includes("more"))).toBe(false);
});
it("ignores the anchor in expanded mode", () => {
const expandedLines = renderTreeList(
{ items: tasks, expanded: true, maxCollapsed: 8, itemType: "todo", anchorIndex: 5, renderItem: t => t },
stubTheme,
);
expect(expandedLines).toHaveLength(14);
expect(expandedLines.some(l => l.includes("more"))).toBe(false);
});
});
});