fix(coding-agent): handled discard no-op resolution and dynamic expand hints

- Updated tool expand-hint rendering to use `expandKeyHint()`, which pulls the `app.tools.expand` binding and formats collapsed previews as `<key>: Expand`.
- Updated related tests in render utils and TUI regressions to assert the new hint string for default and remapped bindings.
- Added a resolve-tool regression test and changelog note covering `action: "discard"` with no pending action as a successful cancellation.
This commit is contained in:
can1357
2026-06-07 03:21:25 +02:00
parent 8df5718a66
commit cd2eb7ed28
6 changed files with 71 additions and 10 deletions
+1
View File
@@ -12,6 +12,7 @@
- Fixed search, grep, and edit output rendering so repeated directory group blank-line boundaries no longer break nested path/link reconstruction
- Fixed `omp dry-balance --bench` flooding the terminal with staircased, duplicated spinner/status lines (and an indented summary) when the tty has ONLCR/OPOST disabled (raw mode). The interactive progress region separated rows with a bare LF and repositioned with a column-preserving `\x1b[<n>A` cursor-up, both of which only land at column 0 when the terminal translates LF→CRLF; with that translation off, every 80 ms redraw cascaded down and to the right into scrollback. The live region now carriage-returns before every cleared row, terminates each row with CRLF, and caps each row to the terminal width so a wrapped line cannot desync the cursor-up from the logical line count.
- Fixed inconsistent vertical spacing between transcript blocks: some blocks (tool results from `search`/`find` and other renderer-backed tools) rendered with a doubled gap (a leading `Spacer` plus the content box's own `paddingY`), while others (the grouped `read` card, file-mention lists, IRC cards) rendered with no gap at all. Vertical spacing is now owned entirely by the chat renderer: `TranscriptContainer` strips each block's plain-blank top/bottom edges and inserts exactly one blank line between consecutive blocks, so every block is separated by a single consistent gap regardless of which component produced it. Individual components (assistant/user/tool/read-group/bash/eval/skill/custom/hook/compaction/branch/todo-reminder/plan-review messages) no longer emit their own leading `Spacer`/`paddingY` for separation, and multi-row groups (IRC cards, file-mention lists, completed-job batches, and the bordered command/`/changelog`/`/context`/version/OAuth/debug panels) are wrapped as single `TranscriptBlock` children so the renderer spaces them as one unit. Background-colored box padding is preserved as block-internal design.
- Fixed `resolve` with `action: "discard"` surfacing a hard `isError` "No pending action to resolve" failure to the model when the agent asked to cancel a staged action (e.g. an `ast_edit` preview) but nothing was pending. A discard is a request to reach the "no staged change" end-state, which already holds in that case, so it is now honored as a successful cancellation (`"Nothing to discard; no pending action remains."` with `details.action: "discard"`) instead of an error. `action: "apply"` with no pending action still errors.
## [15.10.0] - 2026-06-06
@@ -198,7 +198,7 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa
/**
* Add a code-cell content preview below the entry summary.
* When collapsed: shows first COLLAPSED_PREVIEW_LINES lines with "… N more lines (Ctrl+O for more)" hint.
* When collapsed: shows first COLLAPSED_PREVIEW_LINES lines with a "… N more lines ⟨<key>: Expand⟩" hint.
* When expanded: shows full content.
*/
#addContentPreview(entry: ReadEntry): void {
@@ -10,8 +10,9 @@ import * as path from "node:path";
import type { ToolCallContext } from "@oh-my-pi/pi-agent-core";
import type { Ellipsis } from "@oh-my-pi/pi-natives";
import type { Component } from "@oh-my-pi/pi-tui";
import { replaceTabs, truncateToWidth } from "@oh-my-pi/pi-tui";
import { getKeybindings, replaceTabs, truncateToWidth } from "@oh-my-pi/pi-tui";
import { pluralize } from "@oh-my-pi/pi-utils";
import { formatKeyHints, type KeyId } from "../config/keybindings";
import { settings } from "../config/settings";
import type { Theme } from "../modes/theme/theme";
import { Hasher } from "../tui/utils";
@@ -75,8 +76,16 @@ export const TRUNCATE_LENGTHS = {
SHORT: 40,
} as const;
/** Standard expand hint text */
export const EXPAND_HINT = "(Ctrl+O for more)";
/** Keybinding action that toggles tool-output expansion. */
const EXPAND_ACTION = "app.tools.expand";
/** Fallback key when no binding is resolvable (e.g. outside an interactive session). */
const DEFAULT_EXPAND_KEY: KeyId = "ctrl+o";
/** Human-readable key currently bound to tool-output expansion, e.g. `Ctrl+O`. */
export function expandKeyHint(): string {
const keys = getKeybindings().getKeys(EXPAND_ACTION);
return formatKeyHints(keys.length > 0 ? keys : [DEFAULT_EXPAND_KEY]);
}
// =============================================================================
// Text Truncation Utilities
@@ -150,7 +159,7 @@ export function formatStatusIcon(status: ToolUIStatus, theme: Theme, spinnerFram
export function formatExpandHint(theme: Theme, expanded?: boolean, hasMore?: boolean): string {
if (expanded) return "";
if (hasMore === false) return "";
return theme.fg("dim", wrapBrackets(EXPAND_HINT, theme));
return theme.fg("dim", wrapBrackets(`${expandKeyHint()}: Expand`, theme));
}
/**
@@ -1,16 +1,20 @@
import { beforeAll, describe, expect, it } from "bun:test";
import { afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test";
import * as os from "node:os";
import * as path from "node:path";
import { getThemeByName, initTheme, theme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
import { KeybindingsManager } from "@oh-my-pi/pi-coding-agent/config/keybindings";
import { getThemeByName, initTheme, theme, type Theme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
import {
dedupeParseErrors,
expandKeyHint,
formatCodeFrameLine,
formatDiagnostics,
formatErrorMessage,
formatExpandHint,
formatParseErrors,
formatScreenshot,
truncateDiffByHunk,
} from "@oh-my-pi/pi-coding-agent/tools/render-utils";
import { getKeybindings, setKeybindings } from "@oh-my-pi/pi-tui";
describe("parse error formatting", () => {
it("deduplicates parse errors while preserving order", () => {
@@ -282,3 +286,39 @@ describe("formatErrorMessage (F4 sanitization)", () => {
expect(out).toContain("Unknown error");
});
});
describe("formatExpandHint / expandKeyHint", () => {
// Plain stub: `fg` is a passthrough and brackets are literal `[`/`]`, so the
// rendered hint is deterministic regardless of the active theme's bracket glyphs.
const plainTheme = {
fg: (_color: unknown, text: string) => text,
format: { bracketLeft: "[", bracketRight: "]" },
} as unknown as Theme;
let previous: ReturnType<typeof getKeybindings>;
beforeEach(() => {
previous = getKeybindings();
});
afterEach(() => {
setKeybindings(previous);
});
it("reports the default tool-output expand key", () => {
setKeybindings(KeybindingsManager.inMemory());
expect(expandKeyHint()).toBe("Ctrl+O");
// Single bracket pair from the theme, no double-wrapping around the key.
expect(formatExpandHint(plainTheme, false, true)).toBe("[Ctrl+O: Expand]");
});
it("tracks a user remap of the expand binding", () => {
setKeybindings(KeybindingsManager.inMemory({ "app.tools.expand": "alt+e" }));
expect(expandKeyHint()).toBe("Alt+E");
expect(formatExpandHint(plainTheme, false, true)).toBe("[Alt+E: Expand]");
});
it("renders nothing when expanded or there is no more content", () => {
setKeybindings(KeybindingsManager.inMemory());
expect(formatExpandHint(plainTheme, true, true)).toBe("");
expect(formatExpandHint(plainTheme, false, false)).toBe("");
});
});
@@ -35,6 +35,17 @@ describe("ResolveTool", () => {
);
});
it("treats discard with no pending action as a successful cancellation, not an error", async () => {
const tool = new ResolveTool(createSession());
const result = await tool.execute("call-discard-none", {
action: "discard",
reason: "Abandoning the staged edit.",
});
expect(result.isError ?? false).toBe(false);
expect(getText(result)).toContain("Nothing to discard");
expect(result.details).toMatchObject({ action: "discard", reason: "Abandoning the staged edit." });
});
it("discards pending action and clears store", async () => {
let discardedReason: string | undefined;
const handler = async (input: unknown) => {
+3 -3
View File
@@ -1784,7 +1784,7 @@ describe("TUI terminal-state regressions", () => {
const tui = new TUI(term);
const collapsedLines = [
"frame-top",
"code preview … 16 more lines ⟨(Ctrl+O for more)⟩",
"code preview … 16 more lines ⟨Ctrl+O: Expand⟩",
"output preview … 106 more lines (ctrl+o to expand)",
...rows("json-", 10),
"status",
@@ -1835,7 +1835,7 @@ describe("TUI terminal-state regressions", () => {
const tui = new TUI(term);
const component = new MutableLinesComponent([
"frame-top",
"code preview … 16 more lines ⟨(Ctrl+O for more)⟩",
"code preview … 16 more lines ⟨Ctrl+O: Expand⟩",
"output preview … 106 more lines (ctrl+o to expand)",
...rows("json-", 10),
"status",
@@ -1880,7 +1880,7 @@ describe("TUI terminal-state regressions", () => {
const tui = new TUI(term);
const component = new MutableLinesComponent([
"frame-top",
"code preview … 16 more lines ⟨(Ctrl+O for more)⟩",
"code preview … 16 more lines ⟨Ctrl+O: Expand⟩",
"output preview … 106 more lines (ctrl+o to expand)",
...rows("json-", 10),
"status",