refactor(coding-agent): restructured loading animation lifecycle into ensureLoadingAnimation
- Extracted loading animation lifecycle management into centralized ensureLoadingAnimation() method. - Consolidated duplicate loading animation setup across event-controller and interactive-mode into single reusable method. - Updated showError() to properly clean up loading animation state when errors occur. - Added comprehensive test coverage for InputController escape key behavior with optimistic submission.
This commit is contained in:
@@ -1,6 +1,14 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
### Added
|
||||
|
||||
- Added `ensureLoadingAnimation()` method to manage loading animation lifecycle and prevent duplicate spinners
|
||||
|
||||
### Changed
|
||||
|
||||
- Refactored loading animation initialization to use centralized `ensureLoadingAnimation()` method in event and input controllers
|
||||
- Updated `showError()` to properly clean up loading animation state when errors occur
|
||||
|
||||
## [13.9.12] - 2026-03-09
|
||||
### Added
|
||||
|
||||
@@ -107,19 +107,7 @@ export class EventController {
|
||||
this.ctx.retryLoader = undefined;
|
||||
this.ctx.statusContainer.clear();
|
||||
}
|
||||
if (this.ctx.loadingAnimation) {
|
||||
this.ctx.loadingAnimation.stop();
|
||||
}
|
||||
this.ctx.statusContainer.clear();
|
||||
this.ctx.loadingAnimation = new Loader(
|
||||
this.ctx.ui,
|
||||
spinner => theme.fg("accent", spinner),
|
||||
text => theme.fg("muted", text),
|
||||
`Working… (esc to interrupt)`,
|
||||
getSymbolTheme().spinnerFrames,
|
||||
);
|
||||
this.ctx.statusContainer.addChild(this.ctx.loadingAnimation);
|
||||
this.ctx.applyPendingWorkingMessage();
|
||||
this.ctx.ensureLoadingAnimation();
|
||||
this.ctx.ui.requestRender();
|
||||
break;
|
||||
|
||||
|
||||
@@ -339,6 +339,7 @@ export class InputController {
|
||||
};
|
||||
this.ctx.addMessageToChat(optimisticMessage);
|
||||
this.ctx.editor.setText("");
|
||||
this.ctx.ensureLoadingAnimation();
|
||||
this.ctx.ui.requestRender();
|
||||
|
||||
this.ctx.onInputCallback({ text, images });
|
||||
|
||||
@@ -5,8 +5,8 @@
|
||||
import * as path from "node:path";
|
||||
import { type Agent, type AgentMessage, ThinkingLevel } from "@oh-my-pi/pi-agent-core";
|
||||
import type { AssistantMessage, ImageContent, Message, Model, UsageReport } from "@oh-my-pi/pi-ai";
|
||||
import type { Component, Loader, SlashCommand } from "@oh-my-pi/pi-tui";
|
||||
import { Container, Markdown, ProcessTerminal, Spacer, Text, TUI } from "@oh-my-pi/pi-tui";
|
||||
import type { Component, SlashCommand } from "@oh-my-pi/pi-tui";
|
||||
import { Container, Loader, Markdown, ProcessTerminal, Spacer, Text, TUI } from "@oh-my-pi/pi-tui";
|
||||
import { APP_NAME, getProjectDir, hsvToRgb, isEnoent, logger, postmortem } from "@oh-my-pi/pi-utils";
|
||||
import chalk from "chalk";
|
||||
import { KeybindingsManager } from "../config/keybindings";
|
||||
@@ -46,7 +46,14 @@ import { SSHCommandController } from "./controllers/ssh-command-controller";
|
||||
import { OAuthManualInputManager } from "./oauth-manual-input";
|
||||
import { setMermaidRenderCallback } from "./theme/mermaid-cache";
|
||||
import type { Theme } from "./theme/theme";
|
||||
import { getEditorTheme, getMarkdownTheme, onTerminalAppearanceChange, onThemeChange, theme } from "./theme/theme";
|
||||
import {
|
||||
getEditorTheme,
|
||||
getMarkdownTheme,
|
||||
getSymbolTheme,
|
||||
onTerminalAppearanceChange,
|
||||
onThemeChange,
|
||||
theme,
|
||||
} from "./theme/theme";
|
||||
import type { CompactionQueuedMessage, InteractiveModeContext, TodoItem, TodoPhase } from "./types";
|
||||
import { UiHelpers } from "./utils/ui-helpers";
|
||||
|
||||
@@ -849,6 +856,12 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
|
||||
showError(message: string): void {
|
||||
this.optimisticUserMessageSignature = undefined;
|
||||
this.#pendingWorkingMessage = undefined;
|
||||
if (this.loadingAnimation) {
|
||||
this.loadingAnimation.stop();
|
||||
this.loadingAnimation = undefined;
|
||||
this.statusContainer.clear();
|
||||
}
|
||||
this.#uiHelpers.showError(message);
|
||||
}
|
||||
|
||||
@@ -856,6 +869,22 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
this.#uiHelpers.showWarning(message);
|
||||
}
|
||||
|
||||
ensureLoadingAnimation(): void {
|
||||
if (!this.loadingAnimation) {
|
||||
this.statusContainer.clear();
|
||||
this.loadingAnimation = new Loader(
|
||||
this.ui,
|
||||
spinner => theme.fg("accent", spinner),
|
||||
text => theme.fg("muted", text),
|
||||
this.#defaultWorkingMessage,
|
||||
getSymbolTheme().spinnerFrames,
|
||||
);
|
||||
this.statusContainer.addChild(this.loadingAnimation);
|
||||
}
|
||||
|
||||
this.applyPendingWorkingMessage();
|
||||
}
|
||||
|
||||
setWorkingMessage(message?: string): void {
|
||||
if (message === undefined) {
|
||||
this.#pendingWorkingMessage = undefined;
|
||||
|
||||
@@ -128,6 +128,7 @@ export interface InteractiveModeContext {
|
||||
flushPendingModelSwitch(): Promise<void>;
|
||||
setWorkingMessage(message?: string): void;
|
||||
applyPendingWorkingMessage(): void;
|
||||
ensureLoadingAnimation(): void;
|
||||
isKnownSlashCommand(text: string): boolean;
|
||||
addMessageToChat(message: AgentMessage, options?: { populateHistory?: boolean }): void;
|
||||
renderSessionContext(
|
||||
|
||||
@@ -0,0 +1,149 @@
|
||||
import { describe, expect, it, vi } from "bun:test";
|
||||
import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller";
|
||||
import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types";
|
||||
|
||||
type FakeEditor = {
|
||||
onEscape?: () => void;
|
||||
onSubmit?: (text: string) => Promise<void>;
|
||||
shouldBypassAutocompleteOnEscape?: () => boolean;
|
||||
onCtrlC?: () => void;
|
||||
onCtrlD?: () => void;
|
||||
onCtrlZ?: () => void;
|
||||
onShiftTab?: () => void;
|
||||
onCtrlP?: () => void;
|
||||
onShiftCtrlP?: () => void;
|
||||
onAltP?: () => void;
|
||||
onCtrlL?: () => void;
|
||||
onCtrlR?: () => void;
|
||||
onQuestionMark?: () => void;
|
||||
onCtrlV?: () => void;
|
||||
onCopyPrompt?: () => void;
|
||||
onAltUp?: () => void;
|
||||
onChange?: (text: string) => void;
|
||||
setText(text: string): void;
|
||||
getText(): string;
|
||||
addToHistory(text: string): void;
|
||||
setCustomKeyHandler(key: string, handler: () => void): void;
|
||||
};
|
||||
|
||||
function createContext(): {
|
||||
ctx: InteractiveModeContext;
|
||||
editor: FakeEditor;
|
||||
spies: {
|
||||
abort: ReturnType<typeof vi.fn>;
|
||||
addMessageToChat: ReturnType<typeof vi.fn>;
|
||||
clearQueue: ReturnType<typeof vi.fn>;
|
||||
ensureLoadingAnimation: ReturnType<typeof vi.fn>;
|
||||
onInputCallback: ReturnType<typeof vi.fn>;
|
||||
requestRender: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
} {
|
||||
let editorText = "";
|
||||
const abort = vi.fn();
|
||||
const addMessageToChat = vi.fn();
|
||||
const clearQueue = vi.fn(() => ({ steering: [], followUp: [] }));
|
||||
const onInputCallback = vi.fn();
|
||||
const requestRender = vi.fn();
|
||||
const editor: FakeEditor = {
|
||||
setText(text: string) {
|
||||
editorText = text;
|
||||
},
|
||||
getText() {
|
||||
return editorText;
|
||||
},
|
||||
addToHistory: vi.fn(),
|
||||
setCustomKeyHandler: vi.fn(),
|
||||
};
|
||||
|
||||
let ctx!: InteractiveModeContext;
|
||||
const ensureLoadingAnimation = vi.fn(() => {
|
||||
ctx.loadingAnimation = {} as InteractiveModeContext["loadingAnimation"];
|
||||
});
|
||||
|
||||
ctx = {
|
||||
editor: editor as unknown as InteractiveModeContext["editor"],
|
||||
ui: { requestRender } as unknown as InteractiveModeContext["ui"],
|
||||
loadingAnimation: undefined,
|
||||
autoCompactionLoader: undefined,
|
||||
retryLoader: undefined,
|
||||
autoCompactionEscapeHandler: undefined,
|
||||
retryEscapeHandler: undefined,
|
||||
session: {
|
||||
isStreaming: false,
|
||||
isCompacting: false,
|
||||
isGeneratingHandoff: false,
|
||||
isBashRunning: false,
|
||||
isPythonRunning: false,
|
||||
queuedMessageCount: 0,
|
||||
messages: [],
|
||||
extensionRunner: undefined,
|
||||
abort,
|
||||
clearQueue,
|
||||
} as unknown as InteractiveModeContext["session"],
|
||||
sessionManager: {
|
||||
getSessionName: () => "existing session",
|
||||
} as unknown as InteractiveModeContext["sessionManager"],
|
||||
keybindings: {
|
||||
getKeys: () => [],
|
||||
} as unknown as InteractiveModeContext["keybindings"],
|
||||
pendingImages: [],
|
||||
isBashMode: false,
|
||||
isPythonMode: false,
|
||||
optimisticUserMessageSignature: undefined,
|
||||
onInputCallback,
|
||||
addMessageToChat,
|
||||
ensureLoadingAnimation,
|
||||
flushPendingBashComponents: vi.fn(),
|
||||
updatePendingMessagesDisplay: vi.fn(),
|
||||
updateEditorBorderColor: vi.fn(),
|
||||
showDebugSelector: vi.fn(),
|
||||
toggleTodoExpansion: vi.fn(),
|
||||
handleHotkeysCommand: vi.fn(),
|
||||
handleSTTToggle: vi.fn(),
|
||||
showTreeSelector: vi.fn(),
|
||||
showUserMessageSelector: vi.fn(),
|
||||
showSessionSelector: vi.fn(),
|
||||
} as unknown as InteractiveModeContext;
|
||||
|
||||
return {
|
||||
ctx,
|
||||
editor,
|
||||
spies: {
|
||||
abort,
|
||||
addMessageToChat,
|
||||
clearQueue,
|
||||
ensureLoadingAnimation,
|
||||
onInputCallback,
|
||||
requestRender,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("InputController escape behavior", () => {
|
||||
it("arms escape immediately for optimistic submissions", async () => {
|
||||
const { ctx, editor, spies } = createContext();
|
||||
const controller = new InputController(ctx);
|
||||
|
||||
controller.setupKeyHandlers();
|
||||
controller.setupEditorSubmitHandler();
|
||||
await editor.onSubmit?.("hello");
|
||||
|
||||
expect(spies.ensureLoadingAnimation).toHaveBeenCalledTimes(1);
|
||||
expect(spies.addMessageToChat).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
role: "user",
|
||||
attribution: "user",
|
||||
content: [{ type: "text", text: "hello" }],
|
||||
}),
|
||||
);
|
||||
expect(spies.onInputCallback).toHaveBeenCalledWith({ text: "hello", images: undefined });
|
||||
expect(spies.requestRender).toHaveBeenCalledTimes(1);
|
||||
expect(ctx.optimisticUserMessageSignature).toBe("hello\u00000");
|
||||
expect(editor.getText()).toBe("");
|
||||
expect(editor.shouldBypassAutocompleteOnEscape?.()).toBe(true);
|
||||
|
||||
editor.onEscape?.();
|
||||
expect(spies.clearQueue).toHaveBeenCalledTimes(1);
|
||||
expect(spies.abort).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user