From f8b71b4a3cfddebcde102b1016fa451f1e1c82ad Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 2 Aug 2026 00:27:41 +0000 Subject: [PATCH] fix(tui): compacted linear branch continuations - Kept single-child chains aligned with their branch head while preserving real branch depth. - Removed disconnected terminal-branch anchors from TUI and HTML tree renderers. - Updated nested tree regressions for compact connector alignment. Fixes #7332 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/export/html/template.js | 33 +++-------- .../src/modes/components/tree-selector.ts | 55 +++++-------------- .../tree-selector-chain-gutter-2298.test.ts | 40 ++++++-------- ...e-selector-last-branch-gutter-2325.test.ts | 34 +++++------- 5 files changed, 55 insertions(+), 111 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 11d4830cc..ab853d766 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed heavily branched conversation trees shifting linear continuations into disconnected gutter columns and accumulating unnecessary indentation ([#7332](https://github.com/can1357/oh-my-pi/issues/7332)). + ## [17.2.4] - 2026-08-01 ### Added diff --git a/packages/coding-agent/src/export/html/template.js b/packages/coding-agent/src/export/html/template.js index f3a4be6ab..16969170e 100644 --- a/packages/coding-agent/src/export/html/template.js +++ b/packages/coding-agent/src/export/html/template.js @@ -204,7 +204,7 @@ containsActive.set(node, has); } - // Stack: [node, indent, justBranched, showConnector, isLast, gutters, isVirtualRootChild] + // Stack: [node, indent, showConnector, isLast, gutters, isVirtualRootChild] const stack = []; // Add roots (prioritize branch containing active leaf) @@ -213,11 +213,11 @@ ); for (let i = orderedRoots.length - 1; i >= 0; i--) { const isLast = i === orderedRoots.length - 1; - stack.push([orderedRoots[i], multipleRoots ? 1 : 0, multipleRoots, multipleRoots, isLast, [], multipleRoots]); + stack.push([orderedRoots[i], multipleRoots ? 1 : 0, multipleRoots, isLast, [], multipleRoots]); } while (stack.length > 0) { - const [node, indent, justBranched, showConnector, isLast, gutters, isVirtualRootChild] = stack.pop(); + const [node, indent, showConnector, isLast, gutters, isVirtualRootChild] = stack.pop(); result.push({ node, indent, showConnector, isLast, gutters, isVirtualRootChild, multipleRoots }); @@ -229,18 +229,9 @@ Number(containsActive.get(b)) - Number(containsActive.get(a)) ); - // Calculate child indent (matches tree-selector.ts) - let childIndent; - if (multipleChildren) { - // Parent branches: children get +1 - childIndent = indent + 1; - } else if (justBranched && indent > 0) { - // First generation after a branch: +1 for visual grouping - childIndent = indent + 1; - } else { - // Single-child chain: stay flat - childIndent = indent; - } + // Only branch points add visual depth. Linear continuations remain + // aligned with the branch head's content. + const childIndent = multipleChildren ? indent + 1 : indent; // Build gutters for children const connectorDisplayed = showConnector && !isVirtualRootChild; @@ -253,7 +244,7 @@ // Add children in reverse order for stack for (let i = orderedChildren.length - 1; i >= 0; i--) { const childIsLast = i === orderedChildren.length - 1; - stack.push([orderedChildren[i], childIndent, multipleChildren, multipleChildren, childIsLast, childGutters, false]); + stack.push([orderedChildren[i], childIndent, multipleChildren, childIsLast, childGutters, false]); } } @@ -268,13 +259,6 @@ const displayIndent = multipleRoots ? Math.max(0, indent - 1) : indent; const connector = showConnector && !isVirtualRootChild ? (isLast ? '└─ ' : '├─ ') : ''; const connectorPosition = connector ? displayIndent - 1 : -1; - // Chain rows (no connector of their own) under a last-sibling (`└─`) - // branch stay anchored by a vertical drawn one level right of the - // suppressed gutter — below the branch head's content — never in the - // `└─` corner column itself (#2298, #2325). Chains under `├─` heads - // are already anchored by the sibling line (`show: true` gutter). - const nearestGutter = !connector ? gutters[gutters.length - 1] : undefined; - const chainAnchorLevel = nearestGutter && !nearestGutter.show ? nearestGutter.position + 1 : -1; const totalChars = displayIndent * 3; const prefixChars = []; @@ -287,9 +271,6 @@ // Standard tree semantics: `│` only while more siblings continue // below (`show`), space below a `└─`. prefixChars.push(posInLevel === 0 && gutter.show ? '│' : ' '); - } else if (level === chainAnchorLevel) { - // Chain anchor for rows under a `└─` branch head. - prefixChars.push(posInLevel === 0 ? '│' : ' '); } else if (connector && level === connectorPosition) { if (posInLevel === 0) { prefixChars.push(isLast ? '└' : '├'); diff --git a/packages/coding-agent/src/modes/components/tree-selector.ts b/packages/coding-agent/src/modes/components/tree-selector.ts index fc7e76e1b..ebf89b8d1 100644 --- a/packages/coding-agent/src/modes/components/tree-selector.ts +++ b/packages/coding-agent/src/modes/components/tree-selector.ts @@ -144,13 +144,12 @@ class TreeList implements Component { const result: FlatNode[] = []; this.#toolCallMap.clear(); - // Indentation rules: - // - At indent 0: stay at 0 unless parent has >1 children (then +1) - // - At indent 1: children always go to indent 2 (visual grouping of subtree) - // - At indent 2+: stay flat for single-child chains, +1 only if parent branches + // A real branch point adds one indentation level. Linear conversation + // chains retain that level so their text stays aligned with the branch + // head instead of drifting right after every fork. - // Stack items: [node, indent, justBranched, showConnector, isLast, gutters, isVirtualRootChild] - type StackItem = [SessionTreeNode, number, boolean, boolean, boolean, GutterInfo[], boolean]; + // Stack items: [node, indent, showConnector, isLast, gutters, isVirtualRootChild] + type StackItem = [SessionTreeNode, number, boolean, boolean, GutterInfo[], boolean]; const stack: StackItem[] = []; // Determine which subtrees contain the active leaf (to sort current branch first) @@ -188,11 +187,11 @@ class TreeList implements Component { const orderedRoots = [...roots].sort((a, b) => Number(containsActive.get(b)) - Number(containsActive.get(a))); for (let i = orderedRoots.length - 1; i >= 0; i--) { const isLast = i === orderedRoots.length - 1; - stack.push([orderedRoots[i], multipleRoots ? 1 : 0, multipleRoots, multipleRoots, isLast, [], multipleRoots]); + stack.push([orderedRoots[i], multipleRoots ? 1 : 0, multipleRoots, isLast, [], multipleRoots]); } while (stack.length > 0) { - const [node, indent, justBranched, showConnector, isLast, gutters, isVirtualRootChild] = stack.pop()!; + const [node, indent, showConnector, isLast, gutters, isVirtualRootChild] = stack.pop()!; // Extract tool calls from assistant messages for later lookup const entry = node.entry; @@ -227,18 +226,9 @@ class TreeList implements Component { return [...prioritized, ...rest]; })(); - // Calculate child indent - let childIndent: number; - if (multipleChildren) { - // Parent branches: children get +1 - childIndent = indent + 1; - } else if (justBranched && indent > 0) { - // First generation after a branch: +1 for visual grouping - childIndent = indent + 1; - } else { - // Single-child chain: stay flat - childIndent = indent; - } + // Only branch points add visual depth. Linear continuations remain + // aligned with the branch head's content. + const childIndent = multipleChildren ? indent + 1 : indent; // Build gutters for children // If this node showed a connector, add a gutter entry for descendants @@ -255,15 +245,7 @@ class TreeList implements Component { // Add children in reverse order for (let i = orderedChildren.length - 1; i >= 0; i--) { const childIsLast = i === orderedChildren.length - 1; - stack.push([ - orderedChildren[i], - childIndent, - multipleChildren, - multipleChildren, - childIsLast, - childGutters, - false, - ]); + stack.push([orderedChildren[i], childIndent, multipleChildren, childIsLast, childGutters, false]); } } @@ -516,16 +498,8 @@ class TreeList implements Component { const renderedIndent = Math.min(displayIndent, maxIndentLevels); const scrollOffset = displayIndent - renderedIndent; const connectorPositionDisplay = hasConnector ? renderedIndent - 1 : -1; - // Chain rows (no connector of their own) under a last-sibling (`└─`) - // branch stay anchored by a vertical drawn one level RIGHT of the - // suppressed gutter — the column where the row's own connector would - // sit, directly below the branch head's content. Drawing it in the - // `└─` column itself contradicts the corner and leaves dangling, - // drifting verticals once the chain branches deeper (#2298, #2325). - // Chains under `├─` heads need no extra anchor: the sibling line - // (`show: true` gutter) already ties them to their branch. - const nearestGutter = !hasConnector ? flatNode.gutters[flatNode.gutters.length - 1] : undefined; - const chainAnchorLevel = nearestGutter && !nearestGutter.show ? nearestGutter.position + 1 : -1; + // Linear rows reuse their branch head's depth. Existing sibling + // gutters remain visible; terminal gutters remain terminated. // Build prefix char by char, placing gutters and connector at their positions const totalChars = renderedIndent * 3; @@ -545,9 +519,6 @@ class TreeList implements Component { } else { prefixChars.push(" "); } - } else if (originalLevel === chainAnchorLevel) { - // Chain anchor for rows under a `└─` branch head. - prefixChars.push(posInLevel === 0 ? theme.tree.vertical : " "); } else if (hasConnector && level === connectorPositionDisplay) { // Connector at this level if (posInLevel === 0) { diff --git a/packages/coding-agent/test/modes/components/tree-selector-chain-gutter-2298.test.ts b/packages/coding-agent/test/modes/components/tree-selector-chain-gutter-2298.test.ts index 82235f274..79d0abcae 100644 --- a/packages/coding-agent/test/modes/components/tree-selector-chain-gutter-2298.test.ts +++ b/packages/coding-agent/test/modes/components/tree-selector-chain-gutter-2298.test.ts @@ -37,17 +37,15 @@ function renderStripped(tree: SessionTreeNode[], leafId: string, width = 120): s return selector.render(width).map(line => Bun.stripANSI(line)); } -describe("issue #2298: chain rows under last-sibling branches keep their gutter", () => { +describe("issue #7332: linear branch continuations stay compact", () => { beforeAll(async () => { await themeModule.initTheme(false, undefined, undefined, "dark", "light"); }); - // The bug rendered the conversation chain under a `└─` branch with bare - // spaces, breaking the visual flow back to the parent message. The fix - // anchors chain descendants (rows without their own connector) with a `│` - // one level right of the suppressed gutter — directly below the branch - // head's content — never in the `└─` corner column itself (#2325). - it("draws the inherited `│` for chain descendants of a last-sibling branch", () => { + // Linear continuations should align with their branch head. Indenting them + // another level leaves the vertical anchor disconnected from the branch + // connector and makes heavily branched conversations drift right. + it("aligns descendants of a last sibling with the branch head", () => { const root = makeNode("user", "original"); const rootAsst = makeNode("assistant", "resp", root.entry.id); root.children.push(rootAsst); @@ -79,15 +77,13 @@ describe("issue #2298: chain rows under last-sibling branches keep their gutter" const branch1Row = findRow("user: branch1 head"); expect(branch1Row).toMatch(/└─\s+user: branch1 head/); - // Each chain descendant of branch1 must stay anchored by a `│` drawn - // below the branch head's content (one level right of the `└─` - // connector). Before #2298 these rows rendered as bare spaces and the - // chain floated unanchored; after #2325 the anchor must not sit in the - // `└─` corner column, which would dangle below the terminal branch. + // The terminal branch has no continuing sibling gutter. Its linear + // descendants align with the branch head instead of inventing a + // disconnected vertical one level farther right. for (const needle of ["assistant: chain-asst-1", "user: chain-user-2"]) { const row = findRow(needle); - expect(row).not.toMatch(/^\s{2}│/); - expect(row).toMatch(/^\s{5}│\s+\S/); + expect(row).not.toContain("│"); + expect(row).toMatch(/^\s{5}\S/); } }); @@ -128,24 +124,20 @@ describe("issue #2298: chain rows under last-sibling branches keep their gutter" expect(row).toMatch(/[├└]─/); } - // Linear continuations of those branched grandchildren are chain rows. - // c is not the last sibling, so its sibling line (`│` in c's connector - // column) anchors the continuation. d is the last sibling (`└─`), so its - // continuation is anchored one level further right instead — never in - // d's own corner column (#2325), and never in the suppressed branch1 - // column. This is the nested case from the PR review. + // Linear continuations stay at their branch head's content depth. The + // non-last branch keeps its sibling gutter; the terminal branch needs + // no synthetic anchor. { const row = rendered.find(line => line.includes("c continuation")); if (!row) throw new Error("row containing c continuation not rendered"); expect(row).not.toMatch(/^\s{2}│/); - expect(row).toMatch(/^\s{5}│/); + expect(row).toMatch(/^\s{5}│\s{2}\S/); } { const row = rendered.find(line => line.includes("d continuation")); if (!row) throw new Error("row containing d continuation not rendered"); - expect(row).not.toMatch(/^\s{2}│/); - expect(row).not.toMatch(/^\s{5}│/); - expect(row).toMatch(/^\s{8}│/); + expect(row).not.toContain("│"); + expect(row).toMatch(/^\s{8}\S/); } }); }); diff --git a/packages/coding-agent/test/modes/components/tree-selector-last-branch-gutter-2325.test.ts b/packages/coding-agent/test/modes/components/tree-selector-last-branch-gutter-2325.test.ts index 9d4b1f60b..cfd8ebe46 100644 --- a/packages/coding-agent/test/modes/components/tree-selector-last-branch-gutter-2325.test.ts +++ b/packages/coding-agent/test/modes/components/tree-selector-last-branch-gutter-2325.test.ts @@ -47,15 +47,14 @@ function renderStripped(tree: SessionTreeNode[], leafId: string, width = 120): s return selector.render(width).map(line => Bun.stripANSI(line)); } -// Issue #2325 tree shape: a parent that branches into several sub-sessions -// where the LAST sibling (`└─`) carries a chain of flattened message rows -// that itself branches again deeper down. -describe("issue #2325: connectors terminate at `└─` and chain columns stay stable", () => { +// A terminal branch whose linear chain branches again must keep every row at +// its logical depth without reviving the terminal gutter or drifting right. +describe("issue #7332: terminal branch chains keep compact alignment", () => { beforeAll(async () => { await themeModule.initTheme(false, undefined, undefined, "dark", "light"); }); - it("renders no vertical in the `└─` corner column and keeps chain rows on one anchor column", () => { + it("aligns chain rows with their branch heads and terminates last-sibling gutters", () => { counter = 0; const root = makeNode("user", "proceed with implementation"); const asst = chain(root, ["assistant", "resp"]); @@ -83,27 +82,24 @@ describe("issue #2325: connectors terminate at `└─` and chain columns stay s // b3 is the last sibling: its connector is `└─` at column 2. expect(findRow("user: second review head")).toMatch(/^\s{2}└─ \S/); - // Chain rows under the `└─` head: the corner column (col 2) must stay - // blank — no `│` running down from the `└─` — and every chain row is - // anchored by `│` on the same column, one level right (below the head's - // content). Exact prefix: 5 spaces, `│`, 2 spaces, then content. + // Chain rows under the `└─` head align with its content. They neither + // revive the terminated gutter nor create a disconnected anchor farther + // right. for (const needle of ["assistant: fix-asst", "user: fix it all", "assistant: rev-asst"]) { const row = findRow(needle); - expect(row).not.toMatch(/^\s{2}│/); - expect(row).toMatch(/^\s{5}│\s{2}\S/); + expect(row).not.toContain("│"); + expect(row).toMatch(/^\s{5}\S/); } - // The deeper branch point keeps stable columns: connectors sit directly - // below the chain content column (col 8), with nothing dangling in the - // outer corner columns. - expect(findRow("user: review the fixes")).toMatch(/^\s{8}├─ \S/); - expect(findRow("user: other thread")).toMatch(/^\s{8}└─ \S/); + // The deeper branch point advances one level from the compact chain. + expect(findRow("user: review the fixes")).toMatch(/^\s{5}├─ \S/); + expect(findRow("user: other thread")).toMatch(/^\s{5}└─ \S/); - // Continuations of the non-last grandchild ride its sibling line at the - // same column (col 8) — no drift back into outer columns. + // Continuations of the non-last grandchild stay aligned with that + // grandchild while its sibling gutter remains visible. for (const needle of ["user: all findings done", "user: still have findings"]) { const row = findRow(needle); - expect(row).toMatch(/^\s{8}│\s{5}\S/); + expect(row).toMatch(/^\s{5}│\s{2}\S/); } }); });