From fceff5e6b65d5038edca42321781a07b0cd68934 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 7 Jun 2026 04:30:29 +0200 Subject: [PATCH] feat(coding-agent): overhauled plan-review overlay with TOC sidebar - Added section-based plan parsing with per-section delete, undo, and annotate. - Added Tab/Shift+Tab focus regions and per-line scroll navigation. - Added Refine feedback loop emitting annotations back to the model. - Added `OverlayOptions.fullscreen` borrowing the terminal's alt screen buffer. --- packages/coding-agent/CHANGELOG.md | 2 +- .../src/modes/components/overlay-box.ts | 55 ++ .../modes/components/plan-review-overlay.ts | 607 ++++++++++++++++-- .../src/modes/components/plan-toc.ts | 138 ++++ .../src/modes/interactive-mode.ts | 45 +- .../test/interactive-mode-plan-review.test.ts | 95 +++ .../components/plan-review-overlay.test.ts | 199 ++++++ .../test/modes/components/plan-toc.test.ts | 91 +++ packages/tui/CHANGELOG.md | 1 + packages/tui/src/tui.ts | 113 ++++ packages/tui/test/render-regressions.test.ts | 73 +++ 11 files changed, 1339 insertions(+), 80 deletions(-) create mode 100644 packages/coding-agent/src/modes/components/plan-toc.ts create mode 100644 packages/coding-agent/test/modes/components/plan-toc.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bfd13ffe7..83325513c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,7 +11,7 @@ - Changed non-agent API operations (title and commit-message generation) to use session-aware API key resolution with refresh-first auth retries, refreshing the active credential before rotating to another account - Changed the directory grouping for `find`, `search`, `ast_grep`, `ast_edit`, and `lsp` diagnostics from a single flat `# dir/` heading per immediate directory to a multi-level tree that folds the common path prefix into one heading. Previously every group repeated the full directory path — so results rooted outside cwd printed the absolute prefix (e.g. `/Users/me/proj/`) on every heading and nested directories were never collapsed. Now a single-child directory chain folds into one heading (`# packages/pkg/src/`, including an absolute root for out-of-cwd results), subdirectories nest one `#` deeper (`## nested/` → `### child.ts`), and each directory's own files are listed before its subdirectories. TUI hyperlink reconstruction tracks the nested directory stack across the whole output so file and code-frame links keep resolving to the correct absolute paths. -- Changed the plan-mode approval surface from an inline transcript block plus a separate bottom selector into a single fullscreen overlay (like `/copy`). The overlay owns its entire content via `ScrollView`: the plan renders once as Markdown and scrolls inside the outlined box (PageUp/PageDown, g/G), with the approval options and the model-tier slider beneath it. ↑/↓ move the option cursor, ←/→ drive the slider, Enter confirms, the external-editor key opens the plan, and Esc cancels — so a tall plan no longer competes with the selector for vertical space or clips its head on ED3-risk terminals. +- Changed the plan-mode approval surface from an inline transcript block plus a separate bottom selector into a single fullscreen overlay (like `/copy`) and overhauled its navigation. The overlay now renders the plan per-section through `ScrollView` (line-level ↑/↓ scroll, Shift+↑/↓ to scroll faster, PgUp/PgDn, g/G) with no stray per-line `…`, and — when the terminal is wide enough and the plan has ≥2 headings — shows a compact VS Code-style Contents sidebar (the redundant plan-title heading is omitted). Focus moves between regions with Tab/Shift+Tab (and flows at the edges: Down past the last section or the bottom of the body drops into the approval options; Up steps back), while the sidebar glows to track the scrolled section. The sidebar can fast-jump between sections, delete a section (with `u` undo), and annotate sections with feedback (`a`) that is submitted back to the model when the operator picks "Refine plan"; ←/→ always drive the model-tier slider, Enter confirms, the external-editor key opens the plan, and Esc cancels. The overlay borrows the terminal's alternate screen buffer for its lifetime (`fullscreen` overlay), so the transcript stays put on the normal screen instead of bleeding through scrollback behind the modal. - Changed the interactive controllers (command, MCP, selector, extension-UI, event), debug panels, and the status/error/warning helpers to render chat output through `ctx.present(...)` instead of appending to `chatContainer` and calling `ui.requestRender()` directly; transcript rebuilds dispose live blocks via `ctx.resetTranscript()` so animated blocks' timers stop on reset. ### Fixed diff --git a/packages/coding-agent/src/modes/components/overlay-box.ts b/packages/coding-agent/src/modes/components/overlay-box.ts index 52b9f2dc0..e427e6a25 100644 --- a/packages/coding-agent/src/modes/components/overlay-box.ts +++ b/packages/coding-agent/src/modes/components/overlay-box.ts @@ -51,3 +51,58 @@ export function row(content: string, width: number): string { const box = theme.boxSharp; return `${paint(box.vertical)} ${fit(content, Math.max(0, width - 4))} ${paint(box.vertical)}`; } + +/** + * Column index (0-based) of the inner divider for a two-column layout whose + * sidebar content area is `sidebarWidth` columns wide. The layout is + * `│ sidebar │ body │` with a single-column inset on every side, so the divider + * vertical sits at `sidebarWidth + 3` and the body content area is + * {@link splitBodyWidth} columns. + */ +function splitDividerCol(sidebarWidth: number): number { + return sidebarWidth + 3; +} + +/** Body content width for a two-column overlay of total `width`. */ +export function splitBodyWidth(width: number, sidebarWidth: number): number { + return Math.max(0, width - sidebarWidth - 7); +} + +/** Top border carrying the title, split by a `┬` over the column divider. */ +export function topBorderSplit(width: number, title: string, sidebarWidth: number): string { + const box = theme.boxSharp; + const dividerCol = splitDividerCol(sidebarWidth); + const leftLen = Math.max(0, dividerCol - 1); + const rightLen = Math.max(0, width - 2 - dividerCol); + let left: string; + if (!title) { + left = paint(box.topLeft + box.horizontal.repeat(leftLen)); + } else { + const shown = truncateToWidth(` ${title} `, Math.max(0, leftLen - 1)); + const fillWidth = Math.max(0, leftLen - 1 - visibleWidth(shown)); + left = + paint(box.topLeft + box.horizontal) + + theme.bold(theme.fg("accent", shown)) + + paint(box.horizontal.repeat(fillWidth)); + } + return left + paint(box.teeDown + box.horizontal.repeat(rightLen) + box.topRight); +} + +/** Section rule that closes the sidebar column with a `┴` over the divider. */ +export function dividerSplit(width: number, sidebarWidth: number): string { + const box = theme.boxSharp; + const dividerCol = splitDividerCol(sidebarWidth); + const leftLen = Math.max(0, dividerCol - 1); + const rightLen = Math.max(0, width - 2 - dividerCol); + return paint( + box.teeRight + box.horizontal.repeat(leftLen) + box.teeUp + box.horizontal.repeat(rightLen) + box.teeLeft, + ); +} + +/** A two-column content row: `│ sidebar │ body │`, each inset by one column. */ +export function splitRow(sidebar: string, body: string, width: number, sidebarWidth: number): string { + const box = theme.boxSharp; + const bodyWidth = splitBodyWidth(width, sidebarWidth); + const bar = paint(box.vertical); + return `${bar} ${fit(sidebar, sidebarWidth)} ${bar} ${fit(body, bodyWidth)} ${bar}`; +} diff --git a/packages/coding-agent/src/modes/components/plan-review-overlay.ts b/packages/coding-agent/src/modes/components/plan-review-overlay.ts index 59b7e7fb5..e3f7d6292 100644 --- a/packages/coding-agent/src/modes/components/plan-review-overlay.ts +++ b/packages/coding-agent/src/modes/components/plan-review-overlay.ts @@ -1,35 +1,78 @@ /** - * Fullscreen plan-review overlay. Replaces the old inline `PlanReviewBlock` - * (which lived in the transcript and competed with the approval selector for - * vertical space, clipping tall plans). The overlay owns its entire content: the - * plan is rendered once via {@link Markdown} and windowed through a - * {@link ScrollView}, while the approval options (plus the optional model-tier - * slider) sit beneath it inside the same outlined box — one self-contained - * surface in the spirit of the `/copy` picker. + * Fullscreen plan-review overlay. The overlay owns its entire content: the plan + * is split into sections (preamble + one per heading), each rendered through its + * own {@link Markdown} and windowed by a {@link ScrollView}, while the approval + * options (plus the optional model-tier slider) sit beneath inside the same + * outlined box — one self-contained surface in the spirit of the `/copy` picker. * - * Key map mirrors the plan-mode hook selector so muscle memory carries over: - * ↑/↓ (j/k) move the option cursor, ←/→ (h/l) drive the slider, Enter confirms, - * Esc cancels, and the external-editor key opens the plan. PageUp/PageDown and - * g/G scroll the plan body. + * When the terminal is wide enough and the plan has ≥2 headings, a Contents + * sidebar appears: it tracks the scrolled section with an accent "glow", and — + * when focused — lets the operator jump between sections, delete a section + * (with undo), and annotate sections with feedback that feeds the Refine loop. + * + * Focus regions (`toc`/`body`/`actions`) cycle with Tab/Shift+Tab; arrows move + * within the focused region and step left into the sidebar. The default focus is + * `actions`, so the muscle memory of the old single-target overlay carries over: + * ↑/↓ select options, Enter confirms, ←/→ drives the slider when there is no + * sidebar, g/G + PgUp/PgDn scroll, and the external-editor key opens the plan. */ -import { type Component, Markdown, type MarkdownTheme, matchesKey, ScrollView } from "@oh-my-pi/pi-tui"; +import { + type Component, + Ellipsis, + Input, + Markdown, + type MarkdownTheme, + matchesKey, + ScrollView, + truncateToWidth, + visibleWidth, +} from "@oh-my-pi/pi-tui"; import { getMarkdownTheme, theme } from "../theme/theme"; import { matchesAppExternalEditor, matchesSelectCancel, matchesSelectDown, - matchesSelectPageDown, - matchesSelectPageUp, matchesSelectUp, } from "../utils/keybinding-matchers"; import type { HookSelectorSlider } from "./hook-selector"; -import { bottomBorder, divider, row, topBorder } from "./overlay-box"; +import { + bottomBorder, + divider, + dividerSplit, + fit, + row, + splitBodyWidth, + splitRow, + topBorder, + topBorderSplit, +} from "./overlay-box"; +import { joinPlanSections, parsePlanSections, sectionDeletionSpan } from "./plan-toc"; import { renderSegmentTrack } from "./segment-track"; /** Title shown in the overlay's top border. */ const OVERLAY_TITLE = "Plan Review"; /** Minimum plan-body rows kept visible even on short terminals. */ const MIN_BODY_ROWS = 3; +/** Sidebar gates: enough headings, a wide terminal, and a usable body column. */ +const SIDEBAR_MIN_HEADINGS = 2; +const SIDEBAR_MIN_TOTAL_WIDTH = 64; +const SIDEBAR_MIN_BODY_WIDTH = 40; + +type Focus = "toc" | "body" | "actions"; + +interface OverlaySection { + level: number; + title: string; + raw: string; + md: Markdown; + annotations: string[]; +} + +/** Undo snapshot: the joined plan text plus annotations aligned by section. */ +interface UndoEntry { + text: string; + annotations: string[][]; +} export interface PlanReviewOverlayCallbacks { /** Invoked with the chosen option label (never a disabled one). */ @@ -38,6 +81,10 @@ export interface PlanReviewOverlayCallbacks { onCancel: () => void; /** Invoked when the external-editor key is pressed (overlay stays open). */ onExternalEditor?: () => void; + /** Invoked with the new full plan text after an in-overlay delete/undo. */ + onPlanEdited?: (content: string) => void; + /** Invoked with the Refine feedback markdown whenever annotations change. */ + onFeedbackChange?: (feedback: string) => void; } export interface PlanReviewOverlayOptions { @@ -46,46 +93,64 @@ export interface PlanReviewOverlayOptions { options: string[]; /** Indices into `options` that render dimmed and cannot be selected. */ disabledIndices?: number[]; - /** Footer hint line; falls back to a generic nav hint when omitted. */ + /** Trailing footer hint (cancel hint); the overlay prepends dynamic help. */ helpText?: string; /** Initially highlighted option index. */ initialIndex?: number; /** Optional model-tier slider rendered between the plan body and options. */ slider?: HookSelectorSlider; + /** Display label for the external-editor key, surfaced in the footer help. */ + externalEditorLabel?: string; } -/** Default footer hint when the caller supplies none. */ -const DEFAULT_HELP = "up/down select enter confirm pgup/pgdn scroll esc cancel"; +/** Default trailing footer hint when the caller supplies none. */ +const DEFAULT_HELP_SUFFIX = "esc cancel"; export class PlanReviewOverlay implements Component { - #md: Markdown; #mdTheme: MarkdownTheme; #scrollView: ScrollView; + #sections: OverlaySection[] = []; + #toc: number[] = []; + /** Shallowest level among ToC entries, used to flatten indentation. */ + #tocBaseLevel = 1; + #sectionOffsets: number[] = []; + #undo: UndoEntry[] = []; + #options: string[]; #disabled: Set; - #helpText: string; + #helpSuffix: string; + #externalEditorLabel: string | undefined; #promptTitle: string | undefined; #selectedIndex: number; #slider: HookSelectorSlider | undefined; #sliderIndex: number; + #focus: Focus = "actions"; + #tocCursor = 0; + #sidebarShown = false; + #pendingScrollToToc = false; + + #annotating = false; + #input: Input; + constructor( planContent: string, options: PlanReviewOverlayOptions, private readonly callbacks: PlanReviewOverlayCallbacks, ) { this.#mdTheme = getMarkdownTheme(); - this.#md = new Markdown(planContent, 1, 0, this.#mdTheme); this.#scrollView = new ScrollView([], { height: MIN_BODY_ROWS, scrollbar: "auto", + ellipsis: Ellipsis.Omit, theme: { track: t => theme.fg("dim", t), thumb: t => theme.fg("accent", t) }, }); this.#options = options.options; this.#disabled = new Set( (options.disabledIndices ?? []).filter(i => Number.isInteger(i) && i >= 0 && i < this.#options.length), ); - this.#helpText = options.helpText ?? DEFAULT_HELP; + this.#helpSuffix = options.helpText ?? DEFAULT_HELP_SUFFIX; + this.#externalEditorLabel = options.externalEditorLabel; this.#promptTitle = options.promptTitle; this.#selectedIndex = this.#coerceIndex(options.initialIndex ?? 0); if (options.slider && options.slider.segments.length > 0) { @@ -94,17 +159,54 @@ export class PlanReviewOverlay implements Component { } else { this.#sliderIndex = 0; } + this.#input = new Input(); + this.#input.setUseTerminalCursor(false); + this.#input.onSubmit = value => this.#submitAnnotation(value); + this.#input.onEscape = () => this.#exitAnnotate(); + this.#setSections(planContent); } invalidate(): void { - this.#md.invalidate(); + for (const section of this.#sections) section.md.invalidate(); } /** Swap the displayed plan (e.g. after an external-editor round-trip) and - * reset the scroll position so the operator starts at the top. */ + * reset scroll/focus so the operator starts at the top. Does not emit + * `onPlanEdited` (the editor round-trip already persisted the file). */ setPlanContent(planContent: string): void { - this.#md.setText(planContent); + this.#setSections(planContent); this.#scrollView.scrollToTop(); + this.#tocCursor = 0; + this.#recomputeFeedback(); + } + + #setSections(planContent: string): void { + this.#sections = parsePlanSections(planContent).map(section => ({ + level: section.level, + title: section.title, + raw: section.raw, + md: new Markdown(section.raw, 1, 0, this.#mdTheme), + annotations: [] as string[], + })); + this.#rebuildToc(); + this.#tocCursor = Math.min(this.#tocCursor, Math.max(0, this.#toc.length - 1)); + } + + #rebuildToc(): void { + const headings: number[] = []; + for (let i = 0; i < this.#sections.length; i++) { + if (this.#sections[i]!.level >= 1) headings.push(i); + } + // Drop the plan's title from the ToC: a single shallowest heading at the + // top of the document is the plan name itself ("we know it's the plan"), + // so listing it adds noise. Plans with several top-level sections keep + // them all. + let minLevel = Number.POSITIVE_INFINITY; + for (const i of headings) minLevel = Math.min(minLevel, this.#sections[i]!.level); + const topLevel = headings.filter(i => this.#sections[i]!.level === minLevel); + const titleIndex = topLevel.length === 1 && headings[0] === topLevel[0] ? topLevel[0] : -1; + this.#toc = headings.filter(i => i !== titleIndex); + this.#tocBaseLevel = this.#toc.length > 0 ? Math.min(...this.#toc.map(i => this.#sections[i]!.level)) : 1; } /** Clamp `index` to range, then walk to the nearest enabled option so the @@ -119,6 +221,12 @@ export class PlanReviewOverlay implements Component { return clamped; } + /** First enabled option index (or -1 when none), used to detect the "top". */ + #firstEnabledIndex(): number { + for (let i = 0; i < this.#options.length; i++) if (!this.#disabled.has(i)) return i; + return -1; + } + /** Move the option cursor by `delta`, skipping disabled rows, stopping at the * list edge. */ #moveSelection(delta: number): void { @@ -136,6 +244,7 @@ export class PlanReviewOverlay implements Component { } } + /** Step the slider by `delta`, clamped to its edges (narrow-terminal mode). */ #moveSlider(delta: number): void { const slider = this.#slider; if (!slider) return; @@ -145,35 +254,269 @@ export class PlanReviewOverlay implements Component { slider.onChange?.(next); } + #confirmSelection(): void { + const index = this.#selectedIndex; + if (index >= 0 && index < this.#options.length && !this.#disabled.has(index)) { + this.callbacks.onPick(this.#options[index]!); + } + } + handleInput(keyData: string): void { + if (this.#annotating) { + this.#input.handleInput(keyData); + return; + } if (matchesSelectCancel(keyData)) { this.callbacks.onCancel(); return; } - if (matchesSelectUp(keyData) || keyData === "k") { - this.#moveSelection(-1); - } else if (matchesSelectDown(keyData) || keyData === "j") { - this.#moveSelection(1); - } else if (matchesKey(keyData, "left") || (this.#slider && keyData === "h")) { - this.#moveSlider(-1); - } else if (matchesKey(keyData, "right") || (this.#slider && keyData === "l")) { - this.#moveSlider(1); - } else if (matchesKey(keyData, "enter") || matchesKey(keyData, "return") || keyData === "\n") { - const index = this.#selectedIndex; - if (index >= 0 && index < this.#options.length && !this.#disabled.has(index)) { - this.callbacks.onPick(this.#options[index]!); - } - } else if (matchesSelectPageUp(keyData) || matchesKey(keyData, "pageUp")) { - this.#scrollView.page(-1); - } else if (matchesSelectPageDown(keyData) || matchesKey(keyData, "pageDown")) { - this.#scrollView.page(1); - } else if (keyData === "g") { - this.#scrollView.scrollToTop(); - } else if (keyData === "G") { - this.#scrollView.scrollToBottom(); - } else if (this.callbacks.onExternalEditor && matchesAppExternalEditor(keyData)) { + if (this.callbacks.onExternalEditor && matchesAppExternalEditor(keyData)) { this.callbacks.onExternalEditor(); + return; } + if (matchesKey(keyData, "tab") || keyData === "\t") { + this.#cycleRegion(1); + return; + } + if (matchesKey(keyData, "shift+tab") || keyData === "\x1b[Z") { + this.#cycleRegion(-1); + return; + } + switch (this.#focus) { + case "actions": + this.#handleActions(keyData); + return; + case "body": + this.#handleBody(keyData); + return; + case "toc": + this.#handleToc(keyData); + return; + } + } + + #cycleRegion(direction: number): void { + // Sidebar is skipped from the cycle when it is not shown. + const regions: Focus[] = this.#sidebarShown ? ["toc", "body", "actions"] : ["body", "actions"]; + const current = regions.indexOf(this.#focus); + const base = current < 0 ? regions.length - 1 : current; + this.#setFocus(regions[(base + direction + regions.length) % regions.length]!); + } + + #setFocus(focus: Focus): void { + this.#focus = focus; + if (focus === "toc") this.#tocCursor = this.#deriveTocCursorFromScroll(); + } + + #handleActions(data: string): void { + // Left/right always drive the slider. The sidebar sits beside the body + // (above this row), not the slider, so stealing left for it would strand + // the operator unable to step the model tier back — reach the ToC via Tab. + const isLeft = matchesKey(data, "left") || (this.#slider !== undefined && data === "h"); + const isRight = matchesKey(data, "right") || (this.#slider !== undefined && data === "l"); + if (isLeft) { + this.#moveSlider(-1); + return; + } + if (isRight) { + this.#moveSlider(1); + return; + } + if (matchesSelectUp(data) || data === "k") { + if (this.#selectedIndex === this.#firstEnabledIndex()) this.#setFocus("body"); + else this.#moveSelection(-1); + return; + } + if (matchesSelectDown(data) || data === "j") { + this.#moveSelection(1); + return; + } + if (matchesKey(data, "enter") || matchesKey(data, "return") || data === "\n") { + this.#confirmSelection(); + return; + } + this.#handleBodyScroll(data); + } + + #handleBody(data: string): void { + if (matchesKey(data, "left") || data === "h") { + if (this.#sidebarShown) this.#setFocus("toc"); + return; + } + if ( + matchesKey(data, "right") || + data === "l" || + matchesKey(data, "enter") || + matchesKey(data, "return") || + data === "\n" + ) { + this.#setFocus("actions"); + return; + } + // Vertical nav flows between regions at the edges: scrolling off the bottom + // drops into the actions ("next step"); scrolling off the top steps back up + // to the ToC. + if (matchesSelectUp(data) || data === "k") { + if (this.#scrollView.getScrollOffset() <= 0 && this.#sidebarShown) this.#setFocus("toc"); + else this.#scrollView.scroll(-1); + return; + } + if (matchesSelectDown(data) || data === "j") { + if (this.#scrollView.getScrollOffset() >= this.#scrollView.getMaxScrollOffset()) this.#setFocus("actions"); + else this.#scrollView.scroll(1); + return; + } + this.#handleBodyScroll(data); + } + + /** + * Shared scroll dispatch for body + actions focus. Delegates standard keys + * (Arrows, Shift+Arrow fast-scroll, PgUp/PgDn, Home/End) to the ScrollView, + * then adds the vim g/G jumps. Plain Arrow/k/j are consumed by the callers + * before this runs, so here it only ever sees the paging/fast keys. + */ + #handleBodyScroll(data: string): void { + if (this.#scrollView.handleScrollKey(data)) return; + if (data === "g") this.#scrollView.scrollToTop(); + else if (data === "G") this.#scrollView.scrollToBottom(); + } + + #handleToc(data: string): void { + if (matchesSelectUp(data) || data === "k") { + this.#moveTocCursor(-1); + return; + } + if (matchesSelectDown(data) || data === "j") { + // Past the last section, fall through to the actions ("next step"). + if (this.#tocCursor >= this.#toc.length - 1) this.#setFocus("actions"); + else this.#moveTocCursor(1); + return; + } + if ( + matchesKey(data, "right") || + data === "l" || + matchesKey(data, "enter") || + matchesKey(data, "return") || + data === "\n" + ) { + this.#setFocus("body"); + return; + } + if (data === "d" || matchesKey(data, "delete")) { + this.#deleteSelectedSection(); + return; + } + if (data === "a") { + this.#startAnnotate(); + return; + } + if (data === "u") { + this.#undoLast(); + return; + } + } + + #moveTocCursor(delta: number): void { + if (this.#toc.length === 0) return; + const next = Math.max(0, Math.min(this.#toc.length - 1, this.#tocCursor + delta)); + if (next === this.#tocCursor) return; + this.#tocCursor = next; + this.#scrubBodyToToc(); + } + + /** Scroll the body so the selected ToC section's heading sits at the top. */ + #scrubBodyToToc(): void { + const sectionIndex = this.#toc[this.#tocCursor]; + if (sectionIndex === undefined) return; + const offset = this.#sectionOffsets[sectionIndex]; + if (offset !== undefined) this.#scrollView.setScrollOffset(offset); + } + + /** Greatest ToC position whose section starts at or above the scroll offset. */ + #deriveTocCursorFromScroll(): number { + if (this.#toc.length === 0) return 0; + const scrollOffset = this.#scrollView.getScrollOffset(); + let current = 0; + for (let i = 0; i < this.#sections.length; i++) { + if ((this.#sectionOffsets[i] ?? 0) <= scrollOffset) current = i; + else break; + } + let pos = 0; + for (let p = 0; p < this.#toc.length; p++) { + if (this.#toc[p]! <= current) pos = p; + else break; + } + return pos; + } + + #pushUndo(): void { + this.#undo.push({ + text: joinPlanSections(this.#sections), + annotations: this.#sections.map(section => [...section.annotations]), + }); + } + + #deleteSelectedSection(): void { + const sectionIndex = this.#toc[this.#tocCursor]; + if (sectionIndex === undefined) return; + const span = sectionDeletionSpan(this.#sections, sectionIndex); + if (span.length === 0) return; + this.#pushUndo(); + for (let i = span.length - 1; i >= 0; i--) this.#sections.splice(span[i]!, 1); + this.#rebuildToc(); + this.#tocCursor = Math.min(this.#tocCursor, Math.max(0, this.#toc.length - 1)); + this.#pendingScrollToToc = true; + this.callbacks.onPlanEdited?.(joinPlanSections(this.#sections)); + this.#recomputeFeedback(); + } + + #undoLast(): void { + const entry = this.#undo.pop(); + if (!entry) return; + this.#setSections(entry.text); + for (let i = 0; i < this.#sections.length; i++) { + this.#sections[i]!.annotations = entry.annotations[i] ? [...entry.annotations[i]!] : []; + } + this.#tocCursor = Math.min(this.#tocCursor, Math.max(0, this.#toc.length - 1)); + this.#pendingScrollToToc = true; + this.callbacks.onPlanEdited?.(joinPlanSections(this.#sections)); + this.#recomputeFeedback(); + } + + #startAnnotate(): void { + if (this.#toc[this.#tocCursor] === undefined) return; + this.#annotating = true; + this.#input.setValue(""); + } + + #submitAnnotation(value: string): void { + this.#annotating = false; + const note = value.trim(); + const sectionIndex = this.#toc[this.#tocCursor]; + if (note && sectionIndex !== undefined) { + this.#pushUndo(); + this.#sections[sectionIndex]!.annotations.push(note); + this.#recomputeFeedback(); + } + this.#input.setValue(""); + } + + #exitAnnotate(): void { + this.#annotating = false; + this.#input.setValue(""); + } + + #recomputeFeedback(): void { + const annotated = this.#sections.filter(section => section.level >= 1 && section.annotations.length > 0); + let feedback = ""; + if (annotated.length > 0) { + feedback = "Refinement feedback on the plan:\n"; + for (const section of annotated) { + feedback += `\n## ${section.title}\n`; + for (const note of section.annotations) feedback += `- ${note}\n`; + } + } + this.callbacks.onFeedbackChange?.(feedback); } #renderSliderLines(): string[] { @@ -191,47 +534,173 @@ export class PlanReviewOverlay implements Component { } #renderOptionLines(): string[] { + const active = this.#focus === "actions"; return this.#options.map((label, i) => { - const isSelected = i === this.#selectedIndex; + const selected = i === this.#selectedIndex; const isDisabled = this.#disabled.has(i); - const cursor = isSelected ? theme.fg("accent", `${theme.nav.cursor} `) : " "; + // The cursor marks the selected option; it dims when actions are not the + // focused region so the active region's highlight stays unambiguous. + const cursor = selected ? theme.fg(active ? "accent" : "dim", `${theme.nav.cursor} `) : " "; const text = isDisabled ? theme.fg("dim", label) - : isSelected + : selected && active ? theme.bold(theme.fg("accent", label)) : theme.fg("text", label); return cursor + text; }); } + #buildHelp(): string { + const sep = " · "; + const parts: string[] = []; + switch (this.#focus) { + case "actions": + parts.push("↑↓ select", "⏎ confirm"); + if (this.#slider) parts.push("◂▸ model"); + break; + case "toc": + parts.push("↑↓ section", "⏎ open", "a annotate", "d delete", "u undo"); + break; + case "body": + parts.push("↑↓ scroll", "⇧ faster", "pgup/pgdn", "g/G ends"); + break; + } + parts.push("tab regions"); + if (this.#externalEditorLabel && this.#focus !== "toc") parts.push(`${this.#externalEditorLabel} editor`); + parts.push(this.#helpSuffix); + return parts.join(sep); + } + + /** Build the concatenated body lines and record each section's start row. */ + #buildBody(bodyContentWidth: number): string[] { + const lines: string[] = []; + const offsets: number[] = new Array(this.#sections.length); + for (let i = 0; i < this.#sections.length; i++) { + const section = this.#sections[i]!; + offsets[i] = lines.length; + const rendered = section.md.render(bodyContentWidth); + if (section.level >= 1 && section.annotations.length > 0 && rendered.length > 0) { + lines.push(rendered[0]!); + for (const note of section.annotations) { + lines.push(`${theme.fg("warning", "▎ ")}${theme.fg("dim", "note: ")}${theme.fg("accent", note)}`); + } + for (let k = 1; k < rendered.length; k++) lines.push(rendered[k]!); + } else { + for (const line of rendered) lines.push(line); + } + } + this.#sectionOffsets = offsets; + return lines; + } + + #sidebarWidthFor(width: number): number { + return Math.max(18, Math.min(30, Math.round(width * 0.24))); + } + + #sidebarVisible(width: number): boolean { + if (this.#toc.length < SIDEBAR_MIN_HEADINGS) return false; + if (width < SIDEBAR_MIN_TOTAL_WIDTH) return false; + return splitBodyWidth(width, this.#sidebarWidthFor(width)) >= SIDEBAR_MIN_BODY_WIDTH; + } + + #renderSidebarLines(regionRows: number, sidebarWidth: number): string[] { + // No "Contents" label and no plan-title entry: the box title already says + // "Plan Review", so the sidebar is just the bare list of sections, VS + // Code-style. Window the entries around the cursor. + const lines: string[] = []; + const slots = Math.max(0, regionRows); + const total = this.#toc.length; + let start = 0; + if (total > slots) { + start = Math.max(0, Math.min(this.#tocCursor - Math.floor(slots / 2), total - slots)); + } + for (let r = 0; r < slots; r++) { + const p = start + r; + lines.push(p < total ? this.#renderTocEntry(p, sidebarWidth) : ""); + } + return lines; + } + + #renderTocEntry(p: number, width: number): string { + const section = this.#sections[this.#toc[p]!]!; + const highlighted = p === this.#tocCursor; + const selected = highlighted && this.#focus === "toc"; + const glow = highlighted && this.#focus !== "toc"; + // Compact, VS Code-like rows: a single-column gutter, one space of indent + // per nesting level, then the title and an annotation marker. + const indent = " ".repeat(Math.max(0, section.level - this.#tocBaseLevel)); + const ann = section.annotations.length > 0 ? " ✎" : ""; + const avail = Math.max(0, width - 1 - indent.length - visibleWidth(ann)); + const title = truncateToWidth(section.title || "(untitled)", avail, Ellipsis.Unicode); + const body = indent + title + ann; + // Single-column gutter glyph: a cursor `›` on the focused selection, an + // accent bar `▎` on the current scrolled section, otherwise blank. The + // glyph keeps the cursor legible even where the selection background is + // subtle; the focused row also gets the full-row highlight. + const gutter = selected ? "›" : glow ? "▎" : " "; + const line = gutter + body; + if (selected) return theme.bg("selectedBg", theme.bold(fit(line, width))); + if (glow) return theme.fg("accent", line); + return theme.fg("muted", line); + } + + #renderFooterLines(innerWidth: number): string[] { + if (this.#annotating) { + const section = this.#sections[this.#toc[this.#tocCursor]!]; + const title = section?.title ?? ""; + const caption = `${theme.fg("dim", "Annotate")} ${theme.fg("accent", `‹${title}›`)}`; + return [caption, this.#input.render(innerWidth)[0] ?? ""]; + } + return [theme.fg("dim", this.#buildHelp())]; + } + render(width: number): string[] { const termHeight = process.stdout.rows || 40; + const sidebarShown = this.#sidebarVisible(width); + this.#sidebarShown = sidebarShown; + const sidebarWidth = sidebarShown ? this.#sidebarWidthFor(width) : 0; const innerWidth = Math.max(1, width - 4); + const bodyContentWidth = sidebarShown ? splitBodyWidth(width, sidebarWidth) : innerWidth; - const planLines = this.#md.render(innerWidth); const sliderLines = this.#renderSliderLines(); const optionLines = this.#renderOptionLines(); const promptLines = this.#promptTitle ? [theme.bold(theme.fg("accent", this.#promptTitle))] : []; + const footerLines = this.#renderFooterLines(innerWidth); - // Chrome rows around the scrollable body: top border, divider, prompt, - // slider, options, divider, footer, bottom border. - const chrome = 5 + promptLines.length + sliderLines.length + optionLines.length; - const viewport = Math.max(MIN_BODY_ROWS, termHeight - chrome); + // Chrome rows: top border, two dividers, bottom border, plus the + // prompt/slider/option/footer rows between them. + const chrome = 4 + promptLines.length + sliderLines.length + optionLines.length + footerLines.length; + const regionRows = Math.max(MIN_BODY_ROWS, termHeight - chrome); - this.#scrollView.setLines(planLines); - this.#scrollView.setHeight(viewport); - const body = this.#scrollView.render(innerWidth); + const bodyLines = this.#buildBody(bodyContentWidth); + this.#scrollView.setLines(bodyLines); + this.#scrollView.setHeight(regionRows); + if (this.#pendingScrollToToc) { + this.#pendingScrollToToc = false; + this.#scrubBodyToToc(); + } + if (this.#focus !== "toc") this.#tocCursor = this.#deriveTocCursorFromScroll(); + const body = this.#scrollView.render(bodyContentWidth); - return [ - topBorder(width, OVERLAY_TITLE), - ...body.map(line => row(line, width)), - divider(width), - ...promptLines.map(line => row(line, width)), - ...sliderLines.map(line => row(line, width)), - ...optionLines.map(line => row(line, width)), - divider(width), - row(theme.fg("dim", this.#helpText), width), - bottomBorder(width), - ]; + const out: string[] = []; + if (sidebarShown) { + const sidebar = this.#renderSidebarLines(regionRows, sidebarWidth); + out.push(topBorderSplit(width, OVERLAY_TITLE, sidebarWidth)); + for (let i = 0; i < regionRows; i++) { + out.push(splitRow(sidebar[i] ?? "", body[i] ?? "", width, sidebarWidth)); + } + out.push(dividerSplit(width, sidebarWidth)); + } else { + out.push(topBorder(width, OVERLAY_TITLE)); + for (const line of body) out.push(row(line, width)); + out.push(divider(width)); + } + for (const line of promptLines) out.push(row(line, width)); + for (const line of sliderLines) out.push(row(line, width)); + for (const line of optionLines) out.push(row(line, width)); + out.push(divider(width)); + for (const line of footerLines) out.push(row(line, width)); + out.push(bottomBorder(width)); + return out; } } diff --git a/packages/coding-agent/src/modes/components/plan-toc.ts b/packages/coding-agent/src/modes/components/plan-toc.ts new file mode 100644 index 000000000..03ab43c0c --- /dev/null +++ b/packages/coding-agent/src/modes/components/plan-toc.ts @@ -0,0 +1,138 @@ +/** + * Pure heading/section parser for the plan-review overlay. It splits a plan's + * markdown into a flat list of sections — a leading preamble (text before the + * first heading) followed by one entry per ATX heading — preserving the exact + * source bytes of each section so the overlay can render, reorder-free delete, + * and round-trip the document without a full markdown re-render. + * + * No TUI dependencies: this module is unit-tested in isolation. + */ + +/** ATX heading: 1-6 `#`, required whitespace, a title, optional closing `#`s. */ +const HEADING_RE = /^(#{1,6})[ \t]+(.+?)[ \t]*#*[ \t]*$/; +/** Opening/closing code fence run (``` or ~~~), allowing up to 3 lead spaces. */ +const FENCE_RE = /^ {0,3}(`{3,}|~{3,})(.*)$/; + +export interface PlanSection { + /** `0` = preamble (no heading, no ToC entry); `1..6` = heading depth. */ + level: number; + /** Plain-text heading label with inline markdown lightly stripped. */ + title: string; + /** Exact source slice for this section, including its trailing newline(s). */ + raw: string; +} + +/** + * Collapse inline markdown emphasis/link/code syntax to readable text. This is + * a deliberately light strip (not a full markdown render) just so ToC entries + * read cleanly — `**Goal** & [docs](x)` becomes `Goal & docs`. + */ +export function stripInlineMarkdown(text: string): string { + let out = text; + // Images first (so the link pass below does not eat the `(url)`), then links. + out = out.replace(/!\[([^\]]*)\]\([^)]*\)/g, "$1"); + out = out.replace(/\[([^\]]*)\]\([^)]*\)/g, "$1"); + out = out.replace(/\[([^\]]*)\]\[[^\]]*\]/g, "$1"); + // Autolinks `` keep their URL as the readable text. + out = out.replace(/<([^>\s]+)>/g, "$1"); + // Inline code, then bold/italic/strikethrough emphasis runs. + out = out.replace(/`([^`]+)`/g, "$1"); + out = out.replace(/(\*\*|__)(.+?)\1/g, "$2"); + out = out.replace(/(\*|_)(.+?)\1/g, "$2"); + out = out.replace(/~~(.+?)~~/g, "$1"); + return out.replace(/\s+/g, " ").trim(); +} + +/** + * Split `text` into preamble + heading sections. `#` characters inside fenced + * code blocks are never treated as headings. Concatenating every section's + * `raw` reproduces the original text exactly. + */ +export function parsePlanSections(text: string): PlanSection[] { + const lines = text.split("\n"); + // Character offset of each line start so section `raw` can slice the source. + const offsets: number[] = new Array(lines.length); + let cursor = 0; + for (let i = 0; i < lines.length; i++) { + offsets[i] = cursor; + cursor += lines[i]!.length + 1; // +1 for the "\n" join separator + } + + // Heading line indices (start of each heading section), with metadata. + const heads: { line: number; level: number; title: string }[] = []; + let fenceChar: string | null = null; + let fenceLen = 0; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + const fence = FENCE_RE.exec(line); + if (fenceChar === null) { + if (fence) { + fenceChar = fence[1]![0]!; + fenceLen = fence[1]!.length; + } + // Opening-fence lines are body, not headings. + if (fence) continue; + } else { + // Inside a fence: only a matching-or-longer run of the same char closes. + if (fence && fence[1]![0] === fenceChar && fence[1]!.length >= fenceLen && fence[2]!.trim() === "") { + fenceChar = null; + fenceLen = 0; + } + continue; + } + const heading = HEADING_RE.exec(line); + if (heading) { + heads.push({ line: i, level: heading[1]!.length, title: stripInlineMarkdown(heading[2]!) }); + } + } + + const sections: PlanSection[] = []; + const sliceRaw = (startLine: number, endLine: number): string => { + const startOffset = offsets[startLine]!; + const endOffset = endLine < lines.length ? offsets[endLine]! : text.length; + return text.slice(startOffset, endOffset); + }; + + // Preamble: everything before the first heading (only when non-empty). + const firstHeadLine = heads.length > 0 ? heads[0]!.line : lines.length; + if (firstHeadLine > 0) { + const raw = sliceRaw(0, firstHeadLine); + if (raw.length > 0) sections.push({ level: 0, title: "", raw }); + } + + for (let h = 0; h < heads.length; h++) { + const head = heads[h]!; + const endLine = h + 1 < heads.length ? heads[h + 1]!.line : lines.length; + sections.push({ level: head.level, title: head.title, raw: sliceRaw(head.line, endLine) }); + } + + return sections; +} + +/** + * Concatenate every section's `raw` back into a single document and guarantee a + * single trailing newline. Inverse of {@link parsePlanSections} for any input + * that already ends with a newline. + */ +export function joinPlanSections(sections: readonly PlanSection[]): string { + let joined = ""; + for (const section of sections) joined += section.raw; + if (joined.length === 0) return ""; + return joined.endsWith("\n") ? joined : `${joined}\n`; +} + +/** + * Indices to remove when deleting `sections[index]`: the heading itself plus + * every following section nested deeper than it (its sub-headings). The + * preamble (level 0) is never a deletion target and yields an empty span. + */ +export function sectionDeletionSpan(sections: readonly PlanSection[], index: number): number[] { + const target = sections[index]; + if (!target || target.level === 0) return []; + const span = [index]; + for (let j = index + 1; j < sections.length; j++) { + if (sections[j]!.level > target.level) span.push(j); + else break; + } + return span; +} diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index c596e9df2..d831e9ba8 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -1689,6 +1689,8 @@ export class InteractiveMode implements InteractiveModeContext { helpText?: string; disabledIndices?: number[]; onExternalEditor?: () => void; + onPlanEdited?: (content: string) => void; + onFeedbackChange?: (feedback: string) => void; initialIndex?: number; }, extra?: { slider?: HookSelectorSlider }, @@ -1712,11 +1714,14 @@ export class InteractiveMode implements InteractiveModeContext { helpText: dialogOptions?.helpText, initialIndex: dialogOptions?.initialIndex, slider: extra?.slider, + externalEditorLabel: this.keybindings.getDisplayString("app.editor.external") || undefined, }, { onPick: choice => finish(choice), onCancel: () => finish(undefined), onExternalEditor: dialogOptions?.onExternalEditor, + onPlanEdited: dialogOptions?.onPlanEdited, + onFeedbackChange: dialogOptions?.onFeedbackChange, }, ); this.#planReviewOverlay = overlay; @@ -1725,6 +1730,7 @@ export class InteractiveMode implements InteractiveModeContext { width: "100%", maxHeight: "100%", margin: 0, + fullscreen: true, }); this.ui.setFocus(overlay); this.ui.requestRender(); @@ -1756,14 +1762,6 @@ export class InteractiveMode implements InteractiveModeContext { } } - #getPlanReviewHelpText(): string { - const externalEditorKey = this.keybindings.getDisplayString("app.editor.external"); - if (!externalEditorKey) { - return "up/down select enter confirm pgup/pgdn scroll esc cancel"; - } - return `up/down select enter confirm pgup/pgdn scroll ${externalEditorKey.toLowerCase()} open in editor esc cancel`; - } - #getPlanApprovalContextUsage(): ContextUsage | undefined { const executionModel = this.#planModePreviousModelState?.model ?? this.session.model; const contextWindow = executionModel?.contextWindow; @@ -2280,7 +2278,14 @@ export class InteractiveMode implements InteractiveModeContext { }, } : undefined; - const helpText = slider ? `${this.#getPlanReviewHelpText()} ◂/▸ model` : this.#getPlanReviewHelpText(); + // The overlay now owns the dynamic, focus-aware help line; the caller only + // supplies the trailing cancel hint. + const helpText = "esc cancel"; + // In-overlay edits (section deletes/undo) and section annotations. Deletes + // update `editedContent` (and mirror to disk); annotations build `feedback` + // that the Refine branch re-prompts the model with. + let editedContent: string | undefined; + let feedback = ""; const choice = await this.showPlanReview( planContent, @@ -2289,6 +2294,13 @@ export class InteractiveMode implements InteractiveModeContext { { helpText, onExternalEditor: () => void this.#openPlanInExternalEditor(planFilePath), + onPlanEdited: content => { + editedContent = content; + void Bun.write(this.#resolvePlanFilePath(planFilePath), content); + }, + onFeedbackChange: value => { + feedback = value; + }, disabledIndices: keepContextDisabled ? [PLAN_KEEP_CONTEXT_OPTION_INDEX] : undefined, }, { slider }, @@ -2297,7 +2309,10 @@ export class InteractiveMode implements InteractiveModeContext { if (choice === "Approve and execute" || choice === "Approve and compact context" || choice === keepContextLabel) { const finalPlanFilePath = details.finalPlanFilePath || planFilePath; try { - const latestPlanContent = await this.#readPlanFile(planFilePath); + // Prefer in-overlay edits (already in memory) over a disk re-read; the + // `onPlanEdited` write is fire-and-forget, so reading the file here could + // race ahead of it. + const latestPlanContent = editedContent ?? (await this.#readPlanFile(planFilePath)); if (!latestPlanContent) { this.showError(`Plan file not found at ${planFilePath}`); return; @@ -2328,6 +2343,16 @@ export class InteractiveMode implements InteractiveModeContext { } return; } + + if (choice === "Refine plan") { + // Section annotations entered in the overlay become a refinement prompt + // re-submitted to the model. With no annotations, fall back to today's + // behavior: close the overlay and let the operator type their own. + if (feedback.trim() && this.onInputCallback) { + this.onInputCallback(this.startPendingSubmission({ text: feedback })); + } + return; + } } /** diff --git a/packages/coding-agent/test/interactive-mode-plan-review.test.ts b/packages/coding-agent/test/interactive-mode-plan-review.test.ts index 8c270905d..7867b3538 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -146,6 +146,101 @@ describe("InteractiveMode plan review rendering", () => { expect(review.mock.calls[1]?.[0]).not.toContain("First plan"); }); + it("re-prompts the model with annotation feedback when Refine is chosen", async () => { + const planFilePath = "local://PLAN.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nbody"); + + mode.planModeEnabled = true; + mode.planModePlanFilePath = planFilePath; + const feedback = "Refinement feedback on the plan:\n\n## Goal\n- needs detail\n"; + // The overlay reports annotation feedback through onFeedbackChange before the + // operator picks "Refine plan". + vi.spyOn(mode, "showPlanReview").mockImplementation(async (_plan, _title, _options, dialogOptions) => { + dialogOptions?.onFeedbackChange?.(feedback); + return "Refine plan"; + }); + const startSpy = vi + .spyOn(mode, "startPendingSubmission") + .mockReturnValue({ text: feedback, cancelled: false, started: false }); + const onInput = vi.fn(); + mode.onInputCallback = onInput; + + await mode.handlePlanApproval({ + planFilePath, + planExists: true, + title: "PLAN", + finalPlanFilePath: "local://PLAN.md", + }); + + expect(startSpy).toHaveBeenCalledWith(expect.objectContaining({ text: expect.stringContaining("needs detail") })); + expect(onInput).toHaveBeenCalledTimes(1); + }); + + it("Refine with no annotations does not re-prompt the model", async () => { + const planFilePath = "local://PLAN.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nbody"); + + mode.planModeEnabled = true; + mode.planModePlanFilePath = planFilePath; + vi.spyOn(mode, "showPlanReview").mockResolvedValue("Refine plan"); + const startSpy = vi.spyOn(mode, "startPendingSubmission"); + const onInput = vi.fn(); + mode.onInputCallback = onInput; + + await mode.handlePlanApproval({ + planFilePath, + planExists: true, + title: "PLAN", + finalPlanFilePath: "local://PLAN.md", + }); + + expect(startSpy).not.toHaveBeenCalled(); + expect(onInput).not.toHaveBeenCalled(); + }); + + it("approves with in-overlay edits and mirrors them to the plan file", async () => { + const planFilePath = "local://PLAN.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\noriginal body\n"); + + mode.planModeEnabled = true; + mode.planModePlanFilePath = planFilePath; + const edited = "# Plan\n\nedited body\n"; + vi.spyOn(mode, "showPlanReview").mockImplementation(async (_plan, _title, _options, dialogOptions) => { + dialogOptions?.onPlanEdited?.(edited); + return "Approve and execute"; + }); + vi.spyOn(mode, "handleClearCommand").mockResolvedValue(); + const promptSpy = vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); + + await mode.handlePlanApproval({ + planFilePath, + planExists: true, + title: "PLAN", + finalPlanFilePath: "local://PLAN.md", + }); + + // The synthetic plan-approved prompt carries the in-overlay edit, not the + // stale on-disk content (preferring editedContent avoids the write race). + const call = promptSpy.mock.calls.find(isPlanApprovedCall); + expect(call).toBeDefined(); + expect(call?.[0] as string).toContain("edited body"); + expect(call?.[0] as string).not.toContain("original body"); + // onPlanEdited mirrored the edit to the plan file. + expect(await Bun.file(resolvedPlanPath).text()).toContain("edited body"); + }); + it("offers approve-and-keep-context as a distinct plan approval path", async () => { const planFilePath = "local://PLAN.md"; const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { diff --git a/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts b/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts index 49c2cba61..25728a7d7 100644 --- a/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts +++ b/packages/coding-agent/test/modes/components/plan-review-overlay.test.ts @@ -11,6 +11,8 @@ const DOWN = "\x1b[B"; const LEFT = "\x1b[D"; const RIGHT = "\x1b[C"; const ENTER = "\r"; +const TAB = "\t"; +const SHIFT_DOWN = "\x1b[1;2B"; const CANCEL = "\x07"; // ctrl+g, remapped to tui.select.cancel below let darkTheme = await getThemeByName("dark"); @@ -192,4 +194,201 @@ describe("PlanReviewOverlay", () => { expect(out).toContain("brand new body"); expect(out).not.toContain("para 199"); }); + + // Plan with ≥2 headings + nesting, wide enough for the sidebar at width 80. + const SECTION_PLAN = + "# Overview\n\nintro body\n\n## Goal\n\ngoal body\n\n## Steps\n\nstep body\n\n# Risks\n\nrisk body\n"; + it("renders no per-line ellipsis in the plan body", () => { + const overlay = new PlanReviewOverlay( + "# Plan\n\nshort line one\n\nshort line two", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const body = render(overlay) + .split("\n") + .filter(line => line.includes("short line")); + expect(body.length).toBeGreaterThan(0); + for (const line of body) expect(line).not.toContain("…"); + }); + + it("shows a header-less section sidebar and cycles focus regions with Tab", () => { + const overlay = new PlanReviewOverlay( + SECTION_PLAN, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const out = render(overlay); + // Two-column split chrome (┬ joins the title rule over the divider) and the + // bare section list — no "Contents" label. + expect(out).toContain("┬"); + expect(out).not.toContain("Contents"); + expect(out).toContain("Overview"); + // Tab into the ToC region surfaces its focus-specific help. + overlay.handleInput(TAB); + const tocFocused = render(overlay); + expect(tocFocused).toContain("a annotate"); + expect(tocFocused).toContain("d delete"); + }); + + it("omits the single plan-title heading from the ToC", () => { + // One shallow H1 title + two H2 sections: the title is redundant in the ToC. + const overlay = new PlanReviewOverlay( + "# Plan: build the thing\n\nintro\n\n## Design\n\nd\n\n## Rollout\n\nr\n", + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const sidebar = render(overlay) + .split("\n") + .map(line => line.split("│")[1] ?? "") + .join("\n"); + expect(sidebar).toContain("Design"); + expect(sidebar).toContain("Rollout"); + expect(sidebar).not.toContain("build the thing"); + }); + + it("flows past the end of a region into the actions on Down", () => { + const overlay = new PlanReviewOverlay( + SECTION_PLAN, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + render(overlay); + overlay.handleInput(TAB); // -> toc, first section + // Walk to the last ToC entry, then one more Down drops into the actions. + for (let i = 0; i < 10; i++) overlay.handleInput(DOWN); + const out = render(overlay); + // Actions focus restores the option cursor highlight + actions help. + expect(out).toContain("⏎ confirm"); + expect(out).not.toContain("a annotate"); + }); + + it("scrolls the body exactly one line per keystroke in body focus", () => { + // Tall enough to overflow any test viewport, so the body genuinely scrolls. + const rows = Array.from({ length: 400 }, (_, i) => `L${String(i).padStart(3, "0")}`).join("\n"); + const overlay = new PlanReviewOverlay( + `# Plan\n\n\`\`\`\n${rows}\n\`\`\`\n`, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const visibleRows = (): string[] => + render(overlay) + .split("\n") + .map(line => line.match(/L\d\d\d/)?.[0]) + .filter((m): m is string => m !== undefined); + render(overlay); // first render loads the body lines into the ScrollView + overlay.handleInput(TAB); // actions -> body (no sidebar with a single heading) + // Scroll well past the heading/fence so the window is pure code rows. + for (let i = 0; i < 12; i++) overlay.handleInput(DOWN); + const before = visibleRows(); + overlay.handleInput(DOWN); + const after = visibleRows(); + // One-line scroll: the window advances by exactly one row. + expect(after[0]).toBe(before[1]); + }); + + it("jumps the body to a section when the ToC cursor moves", () => { + const overlay = new PlanReviewOverlay( + SECTION_PLAN, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + render(overlay); + overlay.handleInput(TAB); // -> toc (Overview) + overlay.handleInput(DOWN); // -> Goal, scrubbing the body to it + const body = render(overlay).split("\n").slice(1, 5).join(" "); + expect(body).toContain("Goal"); + }); + + it("deletes the selected section and restores it with undo", () => { + const onPlanEdited = vi.fn(); + const overlay = new PlanReviewOverlay( + SECTION_PLAN, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn(), onPlanEdited }, + ); + render(overlay); + overlay.handleInput(TAB); // -> toc (Overview) + overlay.handleInput(DOWN); // -> Goal + overlay.handleInput("d"); + expect(onPlanEdited).toHaveBeenCalled(); + const edited = onPlanEdited.mock.calls.at(-1)?.[0] as string; + expect(edited).not.toContain("## Goal"); + expect(edited).toContain("## Steps"); + expect(render(overlay)).not.toContain("goal body"); + + overlay.handleInput("u"); + const restored = render(overlay); + expect(restored).toContain("Goal"); + expect(restored).toContain("goal body"); + }); + + it("annotates a section and emits feedback for the Refine loop", () => { + const onFeedbackChange = vi.fn(); + const overlay = new PlanReviewOverlay( + SECTION_PLAN, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn(), onFeedbackChange }, + ); + render(overlay); + overlay.handleInput(TAB); // -> toc (Overview) + overlay.handleInput("a"); // annotate Overview + for (const ch of "needs detail") overlay.handleInput(ch); + overlay.handleInput(ENTER); // submit + const out = render(overlay); + expect(out).toContain("needs detail"); // callout in the body + expect(out).toContain("✎"); // marker in the sidebar + expect(onFeedbackChange).toHaveBeenCalled(); + const feedback = onFeedbackChange.mock.calls.at(-1)?.[0] as string; + expect(feedback).toContain("Overview"); + expect(feedback).toContain("needs detail"); + }); + + it("drives the slider with both arrows even when a sidebar is present", () => { + const changes: number[] = []; + const slider: HookSelectorSlider = { + caption: "continue with", + index: 0, + segments: [{ label: "default" }, { label: "slow", detail: "opus" }], + onChange: index => changes.push(index), + }; + const overlay = new PlanReviewOverlay( + SECTION_PLAN, + { promptTitle: "next", options: APPROVAL_OPTIONS, slider }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + render(overlay); // establish the sidebar + // The sidebar sits beside the body, not the slider, so left must still step + // the tier back instead of being stolen to focus the ToC. + overlay.handleInput(RIGHT); // 0 -> 1 + overlay.handleInput(LEFT); // 1 -> 0 + expect(changes).toEqual([1, 0]); + // Focus stayed on the actions region (left did not jump to the ToC). + expect(render(overlay)).not.toContain("a annotate"); + }); + + it("fast-scrolls the body with Shift+Arrow", () => { + const rows = Array.from({ length: 400 }, (_, i) => `L${String(i).padStart(3, "0")}`).join("\n"); + const overlay = new PlanReviewOverlay( + `# Plan\n\n\`\`\`\n${rows}\n\`\`\`\n`, + { promptTitle: "next", options: APPROVAL_OPTIONS }, + { onPick: vi.fn(), onCancel: vi.fn() }, + ); + const firstRow = (): number => + Number( + ( + render(overlay) + .split("\n") + .map(line => line.match(/L\d\d\d/)?.[0]) + .find((m): m is string => m !== undefined) ?? "L000" + ).slice(1), + ); + render(overlay); // first render loads the body lines into the ScrollView + overlay.handleInput(TAB); // -> body + // Scroll into the pure-code region so the leading heading rows don't skew + // the absolute row math, then compare a single step to a Shift step. + for (let i = 0; i < 10; i++) overlay.handleInput(DOWN); + const base = firstRow(); + overlay.handleInput(SHIFT_DOWN); // Shift+Down — fastScrollLines (5) at once + expect(firstRow() - base).toBe(5); + }); }); diff --git a/packages/coding-agent/test/modes/components/plan-toc.test.ts b/packages/coding-agent/test/modes/components/plan-toc.test.ts new file mode 100644 index 000000000..16bb5b461 --- /dev/null +++ b/packages/coding-agent/test/modes/components/plan-toc.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from "bun:test"; +import { + joinPlanSections, + type PlanSection, + parsePlanSections, + sectionDeletionSpan, + stripInlineMarkdown, +} from "@oh-my-pi/pi-coding-agent/modes/components/plan-toc"; + +const titles = (sections: readonly PlanSection[]): string[] => sections.map(s => s.title); +const levels = (sections: readonly PlanSection[]): number[] => sections.map(s => s.level); + +describe("parsePlanSections", () => { + it("splits a preamble and one section per ATX heading, tracking depth", () => { + const sections = parsePlanSections("intro\n\n# Overview\n\nbody\n\n## Goal\n\ngoal\n\n# Risks\n\nrisk\n"); + expect(levels(sections)).toEqual([0, 1, 2, 1]); + expect(titles(sections)).toEqual(["", "Overview", "Goal", "Risks"]); + // Preamble is the only level-0 section; it carries no title. + expect(sections[0]!.raw).toBe("intro\n\n"); + }); + + it("emits no preamble section when the document opens with a heading", () => { + const sections = parsePlanSections("# Top\n\nbody\n"); + expect(levels(sections)).toEqual([1]); + expect(sections[0]!.title).toBe("Top"); + }); + + it("does not treat '#' inside fenced code blocks as a heading", () => { + const sections = parsePlanSections("# Real\n\n```\n# not a heading\n```\n\n~~~\n## also not\n~~~\n"); + expect(levels(sections)).toEqual([1]); + expect(titles(sections)).toEqual(["Real"]); + }); + + it("requires whitespace after the hashes, so '#tag' is body text", () => { + const sections = parsePlanSections("#tag is not a heading\nmore body\n"); + expect(levels(sections)).toEqual([0]); + }); + + it("strips inline markdown and closing hashes from titles", () => { + const sections = parsePlanSections("## **Goal** & [docs](http://x) ##\n\nbody\n"); + expect(sections[0]!.title).toBe("Goal & docs"); + }); +}); + +describe("joinPlanSections", () => { + it("round-trips a newline-terminated document", () => { + const text = "intro\n\n# A\n\nbody a\n\n## A1\n\nnested\n\n# B\n\nbody b\n"; + expect(joinPlanSections(parsePlanSections(text))).toBe(text); + }); + + it("guarantees a single trailing newline when the source lacks one", () => { + expect(joinPlanSections(parsePlanSections("# A\n\nbody"))).toBe("# A\n\nbody\n"); + }); + + it("returns an empty string for an empty document", () => { + expect(joinPlanSections(parsePlanSections(""))).toBe(""); + }); +}); + +describe("sectionDeletionSpan", () => { + const sections = parsePlanSections("intro\n\n# A\n\na\n\n## A1\n\na1\n\n## A2\n\na2\n\n# B\n\nb\n"); + // Indices: 0 preamble, 1 A(L1), 2 A1(L2), 3 A2(L2), 4 B(L1) + + it("removes a heading together with its deeper-nested children", () => { + expect(sectionDeletionSpan(sections, 1)).toEqual([1, 2, 3]); + }); + + it("removes only a leaf section", () => { + expect(sectionDeletionSpan(sections, 2)).toEqual([2]); + }); + + it("never targets the preamble", () => { + expect(sectionDeletionSpan(sections, 0)).toEqual([]); + }); + + it("joining the surviving sections drops the deleted subtree", () => { + const span = new Set(sectionDeletionSpan(sections, 1)); + const survivors = sections.filter((_, i) => !span.has(i)); + const result = joinPlanSections(survivors); + expect(result).toContain("# B"); + expect(result).not.toContain("# A"); + expect(result).not.toContain("## A1"); + }); +}); + +describe("stripInlineMarkdown", () => { + it("collapses emphasis, code, links, and whitespace to readable text", () => { + expect(stripInlineMarkdown("**bold** _it_ `code` [t](u)")).toBe("bold it code t"); + expect(stripInlineMarkdown("a b\tc")).toBe("a b c"); + }); +}); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 8183b6b00..307befe93 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -12,6 +12,7 @@ - Added `Loader.dispose()` to stop the loader animation timer when the component is disposed - Added a `ScrollView` `ellipsis` option (defaults to `Ellipsis.Unicode`) so callers that pre-wrap content to width can pass `Ellipsis.Omit` and suppress the stray per-line `…` that lands on trailing padding. - Added `ScrollView.handleScrollKey()` plus a `fastScrollLines` option so every scroll view gets shared navigation keys, including Shift+Arrow to scroll faster. +- Added `OverlayOptions.fullscreen`: while the topmost visible overlay sets it, the engine borrows the terminal's alternate screen buffer for the overlay's lifetime and paints only the modal there — no ED3, no transcript re-commit — so the transcript stays untouched on the normal screen and is not scrollable behind the modal. ## [15.10.0] - 2026-06-06 diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index eda1f4854..6e7c4a13d 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -306,6 +306,17 @@ export interface OverlayOptions { * Called each render cycle with current terminal dimensions. */ visible?: (termWidth: number, termHeight: number) => boolean; + + // === Fullscreen === + /** + * Borrow the terminal's alternate screen buffer for this overlay's lifetime + * (vim/less idiom). While the topmost visible overlay sets this, the engine + * paints only the modal on the alt screen and emits no ED3 / scrollback + * bytes, so the transcript on the normal screen stays untouched and is not + * scrollable behind the modal. Defaults off — all other overlays are + * unchanged and still draw over the transcript on the normal screen. + */ + fullscreen?: boolean; } /** @@ -498,6 +509,16 @@ export class TUI extends Container { #resizeEventPending = false; #stopped = false; + // Transient alternate-screen state for a fullscreen overlay. While active, the + // engine paints only the modal on the alt buffer and leaves every + // normal-screen accounting field (#previousLines, #viewportTopRow, …) + // untouched, so exiting reconciles cleanly against the terminal-restored + // normal screen. #altPreviousLines is the last alt frame, for repaint-skip. + #altActive = false; + #altPreviousLines: string[] = []; + #altEnterWidth = 0; + #altEnterHeight = 0; + // Overlay stack for modal components rendered on top of base content overlayStack: { component: Component; @@ -953,6 +974,13 @@ export class TUI extends Container { } stop(): void { + // Leave the alt buffer first so the teardown cursor math below runs against + // the restored normal screen (which #previousLines still describes). + if (this.#altActive) { + this.terminal.write("\x1b[?1049l"); + this.#altActive = false; + this.#altPreviousLines = []; + } if (TERMINAL.imageProtocol === ImageProtocol.Kitty) { for (const id of this.#imageBudget.takeAllTransmittedIds()) { this.terminal.write(encodeKittyDeleteImage(id)); @@ -1506,6 +1534,36 @@ export class TUI extends Container { // fit results stay valid across the compose pass and every emitter re-fit. this.#fitLineCache.clear(); + // Fullscreen alt-screen short-circuit. While the topmost visible overlay + // requests it, borrow the terminal's alternate buffer (saved/restored by + // the terminal around 1049h/1049l) and paint only the modal there. This + // touches no normal-screen accounting field, so the transcript on the + // normal screen stays untouched and unscrollable behind the modal, and + // exiting reconciles cleanly against the terminal-restored screen. + const wantAlt = this.#wantsAltScreen(); + if (wantAlt && !this.#altActive) { + this.terminal.write("\x1b[?1049h"); + this.terminal.hideCursor(); + this.#altActive = true; + this.#altPreviousLines = []; + this.#altEnterWidth = width; + this.#altEnterHeight = height; + } else if (!wantAlt && this.#altActive) { + this.terminal.write("\x1b[?1049l"); + this.#altActive = false; + this.#altPreviousLines = []; + // A resize while on the alt buffer reflowed the terminal's saved normal + // screen; it no longer matches #previousLines, so force the geometry + // rebuild path instead of a stale diff. + if (width !== this.#altEnterWidth || height !== this.#altEnterHeight) { + this.#resizeEventPending = true; + } + } + if (this.#altActive) { + this.#renderAltFrame(width, height); + return; + } + // 1. Compose the frame. Bracket the transcript render so the image budget // observes every inline image in display order (overlays carry none). this.#imageBudget.beginPass(); @@ -2613,6 +2671,61 @@ export class TUI extends Container { this.#commit(lines, width, height, viewportTop, toRow); } + /** Topmost visible overlay requests the alternate-screen buffer. */ + #wantsAltScreen(): boolean { + for (let i = this.overlayStack.length - 1; i >= 0; i--) { + const entry = this.overlayStack[i]!; + if (!this.#isOverlayVisible(entry)) continue; + return entry.options?.fullscreen === true; + } + return false; + } + + /** + * Compose and paint a single fullscreen overlay frame on the alt buffer. + * Cursor markers are stripped (the modal draws its own in-band caret and + * keeps the hardware cursor hidden), and only the modal is composited over a + * blank base — the transcript is never touched while the alt buffer is up. + */ + #renderAltFrame(width: number, height: number): void { + const base: string[] = new Array(Math.max(0, height)).fill(""); + let lines = this.#compositeOverlays(base, width, height); + this.#extractCursorPosition(lines, height); + lines = this.#fitLinesToWidth(this.#applyLineResets(lines), width); + this.#emitAltFrame(lines, width, height); + } + + /** + * Full per-row viewport rewrite on the alt buffer. Emits only sync-output + * brackets, a cursor home, and per-row rewrites — never ED3, append-tail, or + * any native-scrollback byte, so it is fully isolated from the planner and + * #commit. The hardware cursor stays hidden (it is never re-shown here). + */ + #emitAltFrame(lines: string[], width: number, height: number): void { + const fitted: string[] = new Array(height); + for (let r = 0; r < height; r++) fitted[r] = this.#fitLineToWidth(lines[r] ?? "", width); + // Skip an identical repaint (the modal is mostly static between keystrokes). + if (this.#altPreviousLines.length === height) { + let same = true; + for (let r = 0; r < height; r++) { + if (fitted[r] !== this.#altPreviousLines[r]) { + same = false; + break; + } + } + if (same) return; + } + let buffer = `${this.#paintBeginSequence}\x1b[H`; + for (let r = 0; r < height; r++) { + if (r > 0) buffer += "\r\n"; + buffer += this.#lineRewriteSequence(fitted[r], width); + } + buffer += this.#paintEndSequence; + this.terminal.write(buffer); + this.#altPreviousLines = fitted; + this.#fullRedrawCount += 1; + } + /** * Foreground-stream live-region paint for ED3-risk terminals with an * unobservable viewport. Commits the newly-sealed chunk to native scrollback diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index dcb6a06b8..d94e77f69 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -3209,6 +3209,79 @@ describe("TUI terminal-state regressions", () => { }); }); + describe("fullscreen overlay alt-screen", () => { + it("enters the alt buffer on show, leaves it on hide, and emits no ED3 while modal", async () => { + const term = new VirtualTerminal(40, 8, 200); + const writes = captureWrites(term); + const tui = new TUI(term); + tui.addChild(new MutableLinesComponent(rows("base-", 8))); + + try { + tui.start(); + await settle(term); + + const showFrom = writes.length; + const handle = tui.showOverlay(new MutableLinesComponent(["MODAL-0", "MODAL-1"]), { + anchor: "bottom-center", + width: "100%", + maxHeight: "100%", + margin: 0, + fullscreen: true, + }); + await settle(term); + + const modalWrites = writes.slice(showFrom).join(""); + // Borrowed the alternate screen buffer … + expect(modalWrites).toContain("\x1b[?1049h"); + // … and never erased scrollback (ED3) or otherwise touched the transcript. + expect(modalWrites).not.toContain("\x1b[3J"); + expect(visible(term).some(line => line.includes("MODAL-0"))).toBeTrue(); + + const hideFrom = writes.length; + handle.hide(); + await settle(term); + + expect(writes.slice(hideFrom).join("")).toContain("\x1b[?1049l"); + // Transcript is back on the normal screen after leaving the alt buffer. + expect(visible(term).some(line => line.includes("base-"))).toBeTrue(); + expect(visible(term).some(line => line.includes("MODAL-0"))).toBeFalse(); + } finally { + tui.stop(); + } + }); + + it("leaves native scrollback untouched across the modal lifetime", async () => { + const term = new VirtualTerminal(40, 6, 200); + const tui = new TUI(term); + // Base transcript overflows the viewport, so rows land in scrollback. + tui.addChild(new MutableLinesComponent(rows("base-", 24))); + + try { + tui.start(); + await settle(term); + const scrollbackBefore = term.getScrollBuffer().map(line => line.trimEnd()); + + const handle = tui.showOverlay(new MutableLinesComponent(["MODAL"]), { + anchor: "bottom-center", + width: "100%", + maxHeight: "100%", + margin: 0, + fullscreen: true, + }); + await settle(term); + handle.hide(); + await settle(term); + + // The modal borrowed/returned the alt buffer without rewriting the + // normal screen's scrollback — the transcript a reader scrolled up to + // see is identical before and after. + expect(term.getScrollBuffer().map(line => line.trimEnd())).toEqual(scrollbackBefore); + } finally { + tui.stop(); + } + }); + }); + describe("stress scenarios", () => { it("rapid content mutations converge to final expected screen", async () => { const term = new VirtualTerminal(30, 8);