From 1b2373425baae68f1eb8f0d84328f99e90c357a8 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 7 Jun 2026 08:42:51 +0200 Subject: [PATCH] feat(packages/coding-agent): set 30fps stream and stabilized read groups - Reduced shimmer-driven UI animations and borders to a 30fps cadence. - Collapsed consecutive read tool calls into one finalized transcript block. - Tracked visible assistant content to finalize and reset read groups correctly. - Updated running task rows to static bullet markers with shimmer-only subagent labels. --- docs/theme.md | 2 +- packages/coding-agent/CHANGELOG.md | 6 +++ packages/coding-agent/src/config/settings.ts | 22 ++++++++++- .../src/modes/components/read-tool-group.ts | 16 ++++++++ .../src/modes/components/tool-execution.ts | 37 ++++++++++++------- .../src/modes/controllers/event-controller.ts | 25 +++++++++---- .../src/modes/interactive-mode.ts | 9 ++++- .../src/modes/theme/theme-schema.json | 2 +- .../coding-agent/src/modes/theme/theme.ts | 8 ++-- .../src/modes/utils/ui-helpers.ts | 15 +++++++- packages/coding-agent/src/task/render.ts | 18 ++++++++- packages/coding-agent/src/tui/output-block.ts | 8 ++-- 12 files changed, 130 insertions(+), 38 deletions(-) diff --git a/docs/theme.md b/docs/theme.md index 738ab3b5b..36be98c4e 100644 --- a/docs/theme.md +++ b/docs/theme.md @@ -85,7 +85,7 @@ If omitted, export code derives defaults from resolved theme colors. - `symbols.preset` sets a theme-level default symbol set. - `symbols.overrides` can override individual `SymbolKey` values. -- `symbols.spinnerFrames` overrides the loading spinner frames. Accepts either a flat `string[]` (applied to both spinner types) or an object `{ "status"?: string[], "activity"?: string[] }` to override each type independently. Any type not specified falls back to the symbol preset's default frames. `status` drives the ~12.5fps spinner used by loaders and tool-execution indicators; `activity` drives the ~60fps spinner used by markdown progress bars and similar high-frequency UI. +- `symbols.spinnerFrames` overrides the loading spinner frames. Accepts either a flat `string[]` (applied to both spinner types) or an object `{ "status"?: string[], "activity"?: string[] }` to override each type independently. Any type not specified falls back to the symbol preset's default frames. `status` drives the ~12.5fps spinner used by loaders and tool-execution indicators; `activity` drives the ~30fps spinner used by markdown progress bars and similar high-frequency UI. Runtime precedence: diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 17ac573fe..9c89f2648 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -17,6 +17,9 @@ ### Changed +- Changed shimmer-driven TUI animations (working text, pending bash/eval borders, and theme activity-spinner documentation) to render at 30fps instead of 60fps. +- Changed running `task` tool agent rows to use a static `•` marker and shimmer only the subagent name, leaving descriptions, stats, and nested tool detail text solid while removing the rotating status glyph from those rows. +- Changed settings singleton method access to reuse bound methods for the active instance instead of allocating a new bound function on every `settings.get` lookup. - Changed plan-mode approval to keep the drafted `local://-plan.md` file at its original name as the canonical plan path, so approved plans are no longer renamed when leaving plan mode - Changed plan-mode write enforcement so only `local://` artifact files are writable during planning, blocking working-tree edits and allowing scratch or draft plan files in the local artifact area - Changed the `todo` tool result renderer to stop redrawing every phase's full task list on each update: when a multi-phase list is rendered collapsed (the default, not manually expanded), only phases the latest update touched — the phase holding the in_progress task, any phase with a just-completed task, and phases named by the ops that ran (`init` counts as touching all) — render their tasks; untouched phases collapse to a one-line `N. Name done/total` summary. When call args are unavailable (e.g. transcript rebuilds) it falls back to the in_progress/completed-transition signals, and the manual expand toggle still shows every task. Also dropped the blank separator line previously inserted between phases. @@ -38,6 +41,9 @@ ### Fixed +- Fixed the working-status shimmer to opt into the loader's 30fps animated-message repaint path while keeping both the status spinner and pending bash/eval tool spinners on their normal 80 ms glyph cadence. +- Fixed consecutive `read` tool calls failing to collapse into a single grouped block when a reasoning model emits one read per completion (`[thinking, read]`). The read group was reset on every assistant `message_start`, so each read rendered as its own one-entry `Read …` line; now a read run accretes across completions and is broken only by a rendered non-empty text/thinking block, a non-read tool, or a user/IRC message — matching the transcript-rebuild path. `ReadToolGroupComponent` now reports its live/finalized state so the growing `Read (N)` header repaints correctly on native-scrollback (risk) terminals. + - Fixed the `task` tool shared-context brief rendering raw Markdown headings (`# Goal`, `# Constraints`) inside framed call/result blocks instead of using the normal Markdown renderer. - Fixed the animated pending border on `bash`/`eval` blocks leaving a frozen dark "bar" segment behind after a backgrounded command finalized through the async update path. Once a command is auto-backgrounded (`details.async.state === "running"`) the block stays "partial" in the TUI until the async job-manager delivers the final result, but it also gets committed to native scrollback — so a mid-sweep shimmer frame baked a stray darkened border segment into the committed copy. The border now stops animating (and the 60fps redraw loop stops) the moment a block enters the backgrounded state, so the committed frame is a clean static border. - Fixed cold `omp` launch to clear native terminal history on the first paint, avoiding a once-per-launch duplicate welcome/transcript copy before the normal session replay. diff --git a/packages/coding-agent/src/config/settings.ts b/packages/coding-agent/src/config/settings.ts index 432519460..6f51f703d 100644 --- a/packages/coding-agent/src/config/settings.ts +++ b/packages/coding-agent/src/config/settings.ts @@ -240,11 +240,13 @@ export class Settings { return promise.then( instance => { globalInstance = instance; + clearBoundSettingsMethods(); globalInstancePromise = Promise.resolve(instance); return instance; }, error => { globalInstance = null; + clearBoundSettingsMethods(); throw error; }, ); @@ -978,6 +980,13 @@ export function onHindsightScopeChanged(cb: () => void): () => void { let globalInstance: Settings | null = null; let globalInstancePromise: Promise | null = null; +let boundSettingsInstance: Settings | null = null; +let boundSettingsMethods = new Map(); + +function clearBoundSettingsMethods(): void { + boundSettingsInstance = null; + boundSettingsMethods = new Map(); +} export function isSettingsInitialized(): boolean { return globalInstance !== null; @@ -990,6 +999,7 @@ export function isSettingsInitialized(): boolean { export function resetSettingsForTest(): void { globalInstance = null; globalInstancePromise = null; + clearBoundSettingsMethods(); } /** @@ -1001,9 +1011,17 @@ export const settings = new Proxy({} as Settings, { if (!globalInstance) { throw new Error("Settings not initialized. Call Settings.init() first."); } - const value = (globalInstance as unknown as Record)[prop]; + if (boundSettingsInstance !== globalInstance) { + clearBoundSettingsMethods(); + boundSettingsInstance = globalInstance; + } + const value = (globalInstance as unknown as Record)[prop]; if (typeof value === "function") { - return value.bind(globalInstance); + const cached = boundSettingsMethods.get(prop); + if (cached) return cached; + const bound = value.bind(globalInstance); + boundSettingsMethods.set(prop, bound); + return bound; } return value; }, diff --git a/packages/coding-agent/src/modes/components/read-tool-group.ts b/packages/coding-agent/src/modes/components/read-tool-group.ts index fc686ed75..5952b0b12 100644 --- a/packages/coding-agent/src/modes/components/read-tool-group.ts +++ b/packages/coding-agent/src/modes/components/read-tool-group.ts @@ -81,6 +81,14 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa #text: Text; #expanded = false; #showContentPreview: boolean; + // A read group accretes entries across multiple assistant completions for as + // long as the run of reads is uninterrupted. While it is the active group it + // must stay in the transcript's repaintable live region — its header line + // re-layouts from `Read ` to `Read (N)` + tree as entries arrive, so a + // frozen snapshot taken on a risk terminal would strand the single-entry form + // (see TranscriptContainer / NativeScrollbackLiveRegion). The controller calls + // `finalize()` once the run breaks so the block can commit to native scrollback. + #finalized = false; constructor(options: ReadToolGroupOptions = {}) { super(); @@ -90,6 +98,14 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa this.#updateDisplay(); } + isTranscriptBlockFinalized(): boolean { + return this.#finalized; + } + + finalize(): void { + this.#finalized = true; + } + updateArgs(args: ReadRenderArgs, toolCallId?: string): void { if (!toolCallId) return; const basePath = args.file_path || args.path || ""; diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index e2d481db2..aee73cc06 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -133,12 +133,12 @@ export interface ToolExecutionHandle { setExpanded(expanded: boolean): void; } -/** Drive pending-tool redraws at ~60fps so the animated border sweep is smooth. - * The TUI already throttles at its 16ms `MIN_RENDER_INTERVAL_MS`, so this is the - * natural upper bound and static frames diff to a no-op redraw at ~zero cost. */ -const SPINNER_RENDER_INTERVAL_MS = 16; +/** Drive pending-tool redraws at 30fps so the animated border sweep stays + * smooth without spending twice the frame budget. The TUI throttles at the same + * cadence, and static frames diff to a no-op redraw at ~zero cost. */ +const SPINNER_RENDER_INTERVAL_MS = 1000 / 30; /** Advance the spinner glyph at its classic ~12.5fps step, decoupled from the - * 60fps render cadence (mirrors `Loader`). */ + * render cadence (mirrors `Loader`). */ const SPINNER_GLYPH_ADVANCE_MS = 80; // Stable per-instance counter so each tool execution's inline images get a @@ -436,17 +436,28 @@ export class ToolExecutionComponent extends Container { !isBackgroundAsyncRunning; const needsSpinner = isStreamingArgs || isPartialTask || isPendingExecBlock; if (needsSpinner && !this.#spinnerInterval) { - this.#lastSpinnerAdvanceAt = performance.now(); + const now = performance.now(); + const frameCount = theme.spinnerFrames.length; + this.#lastSpinnerAdvanceAt = now; + if (frameCount > 0 && this.#spinnerFrame === undefined) { + this.#spinnerFrame = 0; + this.#renderState.spinnerFrame = 0; + } this.#spinnerInterval = setInterval(() => { const now = performance.now(); const frameCount = theme.spinnerFrames.length; - // Redraw at ~60fps for a smooth border sweep, but only step the spinner - // glyph at its classic ~12.5fps cadence. The TUI throttles renders at - // 16ms and the differ drops no-op redraws, so the extra ticks are free. - if (frameCount > 0 && now - this.#lastSpinnerAdvanceAt >= SPINNER_GLYPH_ADVANCE_MS) { - this.#spinnerFrame = ((this.#spinnerFrame ?? -1) + 1) % frameCount; - this.#renderState.spinnerFrame = this.#spinnerFrame; - this.#lastSpinnerAdvanceAt = now; + // Redraw at 30fps for a smooth border sweep, but keep the spinner + // glyph phase-locked to its classic ~12.5fps cadence. Advancing the + // anchor by elapsed frames instead of resetting to `now` avoids the + // 30fps timer quantizing the glyph down to one step every three ticks. + if (frameCount > 0) { + const elapsed = now - this.#lastSpinnerAdvanceAt; + if (elapsed >= SPINNER_GLYPH_ADVANCE_MS) { + const steps = Math.floor(elapsed / SPINNER_GLYPH_ADVANCE_MS); + this.#spinnerFrame = ((this.#spinnerFrame ?? 0) + steps) % frameCount; + this.#renderState.spinnerFrame = this.#spinnerFrame; + this.#lastSpinnerAdvanceAt += steps * SPINNER_GLYPH_ADVANCE_MS; + } } this.#ui.requestRender(); }, SPINNER_RENDER_INTERVAL_MS); diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 06190ae38..084ee4d29 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -44,7 +44,13 @@ type AgentSessionEventHandlers = { export class EventController { #lastReadGroup: ReadToolGroupComponent | undefined = undefined; - #lastThinkingCount = 0; + // Count of visible assistant content blocks (rendered non-empty text/thinking) + // already seen in the current streaming message. A newly appearing one breaks + // the read run: the rendered reasoning/answer is a visual separator, so reads + // after it start a fresh group. Empty/absent thinking — common when a model + // emits one read per completion — does not break it, so a run of consecutive + // reads collapses into one group even across completion boundaries. + #lastVisibleBlockCount = 0; #renderedCustomMessages = new Set(); #lastIntent: string | undefined = undefined; #backgroundToolCallIds = new Set(); @@ -103,6 +109,7 @@ export class EventController { } #resetReadGroup(): void { + this.#lastReadGroup?.finalize(); this.#lastReadGroup = undefined; } @@ -208,6 +215,7 @@ export class EventController { this.#lastIntent = undefined; this.#readToolCallArgs.clear(); this.#readToolCallAssistantComponents.clear(); + this.#resetReadGroup(); this.#assistantMessageStreaming = false; this.#lastAssistantComponent = undefined; // Restore the previous turn's inline error in the transcript before dropping @@ -298,9 +306,8 @@ export class EventController { this.ctx.addMessageToChat(event.message); this.ctx.ui.requestRender(); } else if (event.message.role === "assistant") { - this.#lastThinkingCount = 0; this.#assistantMessageStreaming = true; - this.#resetReadGroup(); + this.#lastVisibleBlockCount = 0; this.ctx.streamingComponent = new AssistantMessageComponent( undefined, this.ctx.hideThinkingBlock, @@ -356,14 +363,15 @@ export class EventController { this.ctx.streamingMessage = event.message; this.ctx.streamingComponent.updateContent(this.ctx.streamingMessage); - const thinkingCount = this.ctx.streamingMessage.content.filter( - content => content.type === "thinking" && content.thinking.trim(), + const visibleBlockCount = this.ctx.streamingMessage.content.filter( + content => + (content.type === "text" && content.text.trim().length > 0) || + (content.type === "thinking" && content.thinking.trim().length > 0), ).length; - if (thinkingCount > this.#lastThinkingCount) { + if (visibleBlockCount > this.#lastVisibleBlockCount) { this.#resetReadGroup(); - this.#lastThinkingCount = thinkingCount; + this.#lastVisibleBlockCount = visibleBlockCount; } - for (const content of this.ctx.streamingMessage.content) { if (content.type !== "toolCall") continue; if (content.name === "read") { @@ -685,6 +693,7 @@ export class EventController { ); this.#readToolCallArgs.clear(); this.#readToolCallAssistantComponents.clear(); + this.#resetReadGroup(); this.#lastAssistantComponent = undefined; this.ctx.ui.requestRender(); this.#scheduleIdleCompaction(); diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 56b763d28..0b0e42641 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -20,7 +20,7 @@ import { modelsAreEqual, type UsageReport, } from "@oh-my-pi/pi-ai"; -import type { Component, EditorTheme, OverlayHandle, SlashCommand } from "@oh-my-pi/pi-tui"; +import type { Component, EditorTheme, LoaderMessageColorFn, OverlayHandle, SlashCommand } from "@oh-my-pi/pi-tui"; import { Container, clearRenderCache, @@ -2659,13 +2659,18 @@ export class InteractiveMode implements InteractiveModeContext { ensureLoadingAnimation(): void { if (!this.loadingAnimation) { this.statusContainer.clear(); + const messageColorFn = ((message: string) => + renderWorkingMessage(message, this.#getWorkingMessageAccent())) as LoaderMessageColorFn & { + animated: true; + }; + messageColorFn.animated = true; this.loadingAnimation = new Loader( this.ui, spinner => { const accent = this.#getWorkingMessageAccent(); return accent ? `${accent.main}${spinner}\x1b[39m` : theme.fg("accent", spinner); }, - message => renderWorkingMessage(message, this.#getWorkingMessageAccent()), + messageColorFn, this.#defaultWorkingMessage, getSymbolTheme().spinnerFrames, ); diff --git a/packages/coding-agent/src/modes/theme/theme-schema.json b/packages/coding-agent/src/modes/theme/theme-schema.json index e401ca0c1..3fd9972b4 100644 --- a/packages/coding-agent/src/modes/theme/theme-schema.json +++ b/packages/coding-agent/src/modes/theme/theme-schema.json @@ -406,7 +406,7 @@ } }, "spinnerFrames": { - "description": "Override the spinner frames. Use a flat array to set both `status` and `activity`, or an object to override each independently. Frames are advanced ~12.5fps for status spinners and ~60fps for activity spinners.", + "description": "Override the spinner frames. Use a flat array to set both `status` and `activity`, or an object to override each independently. Frames are advanced ~12.5fps for status spinners and ~30fps for activity spinners.", "oneOf": [ { "type": "array", diff --git a/packages/coding-agent/src/modes/theme/theme.ts b/packages/coding-agent/src/modes/theme/theme.ts index 963bade66..ed0d46d24 100644 --- a/packages/coding-agent/src/modes/theme/theme.ts +++ b/packages/coding-agent/src/modes/theme/theme.ts @@ -2443,10 +2443,10 @@ function getHighlightColors(t: Theme): NativeHighlightColors { * switch (which always reassigns `theme`) must invalidate every entry. * * Why this exists: animated tool blocks (eval/bash) repaint their box on every - * ~16ms border-shimmer frame, and markdown re-lexes on every streamed delta. - * Without memoization each frame re-tokenizes an unchanged code body through the - * Rust FFI — ~26ms for 100 lines, ~40ms for 150 — overrunning the 16ms frame - * budget and starving the spinner/render timers (the "TUI freeze"). + * ~33ms border-shimmer frame, and markdown re-lexes on every streamed delta. + * Without memoization each frame can re-tokenize an unchanged code body through + * the Rust FFI — ~26ms for 100 lines, ~40ms for 150 — consuming or overrunning + * the 33ms frame budget and starving the spinner/render timers (the "TUI freeze"). */ const HIGHLIGHT_CACHE_MAX = 256; const highlightCache = new LRUCache({ max: HIGHLIGHT_CACHE_MAX }); diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 5b663c5e9..fdf6b2876 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -336,7 +336,15 @@ export class UiHelpers { if (assistantComponent) { assistantComponent.setUsageInfo(message.usage); } - readGroup = null; + const hasVisibleAssistantContent = message.content.some( + content => + (content.type === "text" && content.text.trim().length > 0) || + (content.type === "thinking" && content.thinking.trim().length > 0), + ); + if (hasVisibleAssistantContent) { + readGroup?.finalize(); + readGroup = null; + } const isAbortedSilently = message.stopReason === "aborted" && isSilentAbort(message.errorMessage); const hasErrorStop = !isAbortedSilently && (message.stopReason === "aborted" || message.stopReason === "error"); @@ -384,6 +392,7 @@ export class UiHelpers { continue; } + readGroup?.finalize(); readGroup = null; const tool = this.ctx.session.getToolByName(content.name); const renderArgs = @@ -471,6 +480,10 @@ export class UiHelpers { } } + // The trailing read run has no following break to close it; finalize so the + // rebuilt group commits to native scrollback like every other historical block. + readGroup?.finalize(); + // Render deferred messages (compaction summaries) at the bottom so they're visible for (const message of deferredMessages) { this.ctx.addMessageToChat(message, options); diff --git a/packages/coding-agent/src/task/render.ts b/packages/coding-agent/src/task/render.ts index 0975c66ea..1bb94527f 100644 --- a/packages/coding-agent/src/task/render.ts +++ b/packages/coding-agent/src/task/render.ts @@ -11,6 +11,7 @@ import { formatNumber } from "@oh-my-pi/pi-utils"; import { settings } from "../config/settings"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import { formatContextUsage } from "../modes/components/status-line/context-thresholds"; +import { shimmerEnabled, shimmerText } from "../modes/theme/shimmer"; import { getMarkdownTheme, type Theme } from "../modes/theme/theme"; import { formatBadge, @@ -628,11 +629,24 @@ function renderAgentProgress( const description = progress.description?.trim(); const displayId = formatTaskId(progress.id); const titlePart = description ? `${theme.bold(displayId)}: ${description}` : displayId; - let statusLine = `${prefix ? `${prefix} ` : ""}${theme.fg(iconColor, icon)} ${theme.fg("accent", titlePart)}`; + const indent = prefix ? `${prefix} ` : ""; + let statusLine: string; + if (progress.status === "running") { + const bullet = theme.fg("accent", "•"); + const name = shimmerEnabled() + ? shimmerText(displayId, theme) + : theme.fg("accent", description ? theme.bold(displayId) : displayId); + statusLine = `${indent}${bullet} ${name}`; + if (description) { + statusLine += theme.fg("accent", `: ${description}`); + } + } else { + statusLine = `${indent}${theme.fg(iconColor, icon)} ${theme.fg("accent", titlePart)}`; + } // Show retry-blocked badge so the parent immediately sees that a child // is sleeping on a provider 429, not silently progressing. Wins over the - // generic running spinner because "we're waiting on a quota window" is + // generic running marker because "we're waiting on a quota window" is // the operationally meaningful state. if (progress.retryState && progress.status === "running") { statusLine += ` ${formatBadge("retrying", "warning", theme)}`; diff --git a/packages/coding-agent/src/tui/output-block.ts b/packages/coding-agent/src/tui/output-block.ts index 848d92969..d80344f68 100644 --- a/packages/coding-agent/src/tui/output-block.ts +++ b/packages/coding-agent/src/tui/output-block.ts @@ -37,7 +37,7 @@ export function isFramedBlockComponent(component: Component): boolean { return (component as FramedBlockComponent)[FRAMED_BLOCK_COMPONENT] === true; } -const BORDER_SHIMMER_TICK_MS = 16; +const BORDER_SHIMMER_TICK_MS = 1000 / 30; /** Duration of one full left↔right↔left bounce of the bottom-edge segment, in * ms. Position is derived from the wall clock against this fixed cycle so a * resize only nudges the segment proportionally instead of teleporting it. */ @@ -46,9 +46,9 @@ const BORDER_BOUNCE_MS = 3000; const BORDER_SEGMENT_LEN = 8; /** - * Monotonic frame counter for animated borders, quantized to the TUI's ~16ms - * render cap so the cache key advances once per ~60fps frame — fine enough for a - * smooth segment sweep, coarse enough to coalesce multiple render passes that + * Monotonic frame counter for animated borders, quantized to the TUI's ~30fps + * render cap so the cache key advances once per animation frame — fine enough + * for a smooth segment sweep, coarse enough to coalesce multiple render passes * land inside the same frame. */ export function borderShimmerTick(): number {