Merge PR #6469: fix(tui): prewarmed tiny-title worker off the first-submit hot path (@roboomp)
This commit is contained in:
can1357
@@ -15,6 +15,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed spilled tool-output artifact descriptors leaking on error/abort paths. `OutputSink.dump()` was the only path that closed the spill `Bun.FileSink`, but the bash and Python executors re-throw on failure and their `finally` blocks never closed the sink, so a large-output command that errored leaked the artifact descriptor until an unrelated read (e.g. a `SKILL.md` load) hit `EMFILE`. `OutputSink` now exposes an idempotent `dispose()` that closes the sink exactly once, wired into every executor's `finally` ([#6463](https://github.com/can1357/oh-my-pi/issues/6463)).
|
||||
### Fixed
|
||||
|
||||
- Fixed the first submitted prompt stalling while the local tiny-title worker started: the interactive submit handler now paints the pending user row before starting title generation, and startup prewarms an idle, unref'd worker so the first submit reuses a live subprocess instead of paying spawn latency ahead of the first frame ([#6462](https://github.com/can1357/oh-my-pi/issues/6462)).
|
||||
|
||||
## [17.1.0] - 2026-07-24
|
||||
|
||||
|
||||
@@ -834,45 +834,6 @@ export class InputController {
|
||||
// First, move any pending bash components to chat
|
||||
this.ctx.flushPendingBashComponents();
|
||||
|
||||
// AgentSession.prompt() consumes registered extension commands locally.
|
||||
// Classify them here because title generation starts before prompt dispatch.
|
||||
const extensionCommandSpace = text.indexOf(" ");
|
||||
const isLocalExtensionCommand =
|
||||
text.startsWith("/") &&
|
||||
runner?.getCommand(extensionCommandSpace === -1 ? text.slice(1) : text.slice(1, extensionCommandSpace)) !==
|
||||
undefined;
|
||||
|
||||
// Auto-generate a session title while the session is still unnamed.
|
||||
// Greetings / acknowledgements / empty input carry no task, so they are
|
||||
// skipped deterministically (no model invoked, no download-progress UI)
|
||||
// and the session stays unnamed — the next user message gets a fresh
|
||||
// chance, so titling defers past "hi" instead of latching onto it.
|
||||
if (
|
||||
!isLocalExtensionCommand &&
|
||||
!this.ctx.sessionManager.getSessionName() &&
|
||||
!$env.PI_NO_TITLE &&
|
||||
!isLowSignalTitleInput(text)
|
||||
) {
|
||||
this.#showTinyTitleDownloadProgress(this.ctx.settings.get("providers.tinyModel"));
|
||||
this.ctx.session
|
||||
.generateTitle(text)
|
||||
.then(async title => {
|
||||
// Re-check: a concurrent attempt for an earlier message may have
|
||||
// already named the session. Don't clobber it. Terminal title and
|
||||
// accent updates fire from the onSessionNameChanged listener.
|
||||
if (title && !this.ctx.sessionManager.getSessionName()) {
|
||||
await this.ctx.sessionManager.setSessionName(title, "auto");
|
||||
}
|
||||
})
|
||||
.catch(err => {
|
||||
logger.warn("title-generator: uncaught auto-title error", {
|
||||
sessionId: this.ctx.session.sessionId,
|
||||
reason: "uncaught-auto-title-error",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
if (this.ctx.onInputCallback) {
|
||||
// Include any pending images from clipboard paste
|
||||
this.ctx.editor.imageLinks = undefined;
|
||||
@@ -892,6 +853,9 @@ export class InputController {
|
||||
imageLinks: inputImageLinks,
|
||||
streamingBehavior: "steer",
|
||||
});
|
||||
// Start titling only after the optimistic row painted, so the local
|
||||
// tiny-title worker's subprocess spawn never blocks the first frame.
|
||||
this.#maybeStartTitleGeneration(text);
|
||||
|
||||
this.ctx.onInputCallback(submission);
|
||||
} else {
|
||||
@@ -906,6 +870,7 @@ export class InputController {
|
||||
const images = inputImages && inputImages.length > 0 ? [...inputImages] : undefined;
|
||||
this.ctx.editor.pendingImages = [];
|
||||
this.ctx.editor.pendingImageLinks = [];
|
||||
this.#maybeStartTitleGeneration(text);
|
||||
try {
|
||||
await this.ctx.withLocalSubmission(
|
||||
text,
|
||||
@@ -935,6 +900,49 @@ export class InputController {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Kick off session-title generation while the session is still unnamed.
|
||||
* Invoked AFTER the optimistic user row is painted so the local tiny-title
|
||||
* worker's subprocess spawn never lands ahead of the first frame (issue #6462).
|
||||
* Skips slash extension commands (consumed locally by AgentSession.prompt()),
|
||||
* already-named sessions, PI_NO_TITLE, and low-signal greetings — no model or
|
||||
* download UI for those.
|
||||
*/
|
||||
#maybeStartTitleGeneration(text: string): void {
|
||||
const runner = this.ctx.session.extensionRunner;
|
||||
const extensionCommandSpace = text.indexOf(" ");
|
||||
const isLocalExtensionCommand =
|
||||
text.startsWith("/") &&
|
||||
runner?.getCommand(extensionCommandSpace === -1 ? text.slice(1) : text.slice(1, extensionCommandSpace)) !==
|
||||
undefined;
|
||||
if (
|
||||
isLocalExtensionCommand ||
|
||||
this.ctx.sessionManager.getSessionName() ||
|
||||
$env.PI_NO_TITLE ||
|
||||
isLowSignalTitleInput(text)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
this.#showTinyTitleDownloadProgress(this.ctx.settings.get("providers.tinyModel"));
|
||||
this.ctx.session
|
||||
.generateTitle(text)
|
||||
.then(async title => {
|
||||
// Re-check: a concurrent attempt for an earlier message may have
|
||||
// already named the session. Don't clobber it. Terminal title and
|
||||
// accent updates fire from the onSessionNameChanged listener.
|
||||
if (title && !this.ctx.sessionManager.getSessionName()) {
|
||||
await this.ctx.sessionManager.setSessionName(title, "auto");
|
||||
}
|
||||
})
|
||||
.catch(err => {
|
||||
logger.warn("title-generator: uncaught auto-title error", {
|
||||
sessionId: this.ctx.session.sessionId,
|
||||
reason: "uncaught-auto-title-error",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/** Submit editor text to the focused subagent session (chat-only focus policy). */
|
||||
async #submitToFocusedSession(text: string, streamingBehavior: "steer" | "followUp"): Promise<void> {
|
||||
const target = this.ctx.viewSession;
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
} from "@oh-my-pi/pi-tui";
|
||||
import { isInsideTerminalMultiplexer } from "@oh-my-pi/pi-tui/terminal-capabilities";
|
||||
import {
|
||||
$env,
|
||||
APP_NAME,
|
||||
adjustHsv,
|
||||
formatNumber,
|
||||
@@ -113,6 +114,7 @@ import { STTController, type SttState } from "../stt";
|
||||
import { discoverTitleSystemPromptFile, resolvePromptInput } from "../system-prompt";
|
||||
import { formatTaskId } from "../task/render";
|
||||
import type { ConfiguredThinkingLevel } from "../thinking";
|
||||
import { tinyTitleClient } from "../tiny/title-client";
|
||||
import type { LspStartupServerInfo } from "../tools";
|
||||
import { normalizeLocalScheme } from "../tools/path-utils";
|
||||
import { replaceTabs, TRUNCATE_LENGTHS, truncateToWidth } from "../tools/render-utils";
|
||||
@@ -1014,6 +1016,15 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
this.isInitialized = true;
|
||||
this.ui.requestRender(true);
|
||||
|
||||
// Prewarm the local tiny-title worker off the submit hot path: spawn it
|
||||
// now, idle and unref'd, so the first submit reuses a live subprocess
|
||||
// instead of paying spawn latency ahead of the first frame (issue #6462).
|
||||
// No-ops for the online default and for already-named sessions that will
|
||||
// not be titled.
|
||||
if (!$env.PI_NO_TITLE && !this.sessionManager.getSessionName()) {
|
||||
tinyTitleClient.prewarm(this.settings.get("providers.tinyModel"));
|
||||
}
|
||||
|
||||
// Initialize hooks with TUI-based UI context
|
||||
await this.initHooksAndCustomTools();
|
||||
|
||||
|
||||
@@ -196,6 +196,28 @@ export class TinyTitleClient {
|
||||
return () => this.#progressListeners.delete(listener);
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn the tiny-model worker ahead of first use without loading any model.
|
||||
* Called from idle TUI startup so the first {@link generate} reuses a live,
|
||||
* unref'd subprocess instead of paying subprocess-spawn latency on the submit
|
||||
* hot path (issue #6462). No-ops for online / non-local keys and for models
|
||||
* already marked failed. A no-op `ping` round-trips the transport to fault in
|
||||
* the worker's module graph; no pending request is registered, so
|
||||
* {@link #syncWorkerRef} leaves the worker unref'd and idle sessions still exit.
|
||||
*/
|
||||
prewarm(modelKey: string): void {
|
||||
if (!isTinyTitleLocalModelKey(modelKey) || this.#failedModels.has(modelKey)) return;
|
||||
try {
|
||||
const worker = this.#ensureWorker();
|
||||
worker.send({ type: "ping", id: String(++this.#nextRequestId) });
|
||||
} catch (error) {
|
||||
logger.debug("tiny-title: prewarm failed", {
|
||||
modelKey,
|
||||
error: error instanceof Error ? error.message : String(error),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async generate(modelKey: string, message: string, signal?: AbortSignal): Promise<string | null>;
|
||||
async generate(modelKey: string, message: string, options?: TinyTitleGenerateOptions): Promise<string | null>;
|
||||
async generate(
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { InteractiveMode } from "@oh-my-pi/pi-coding-agent/modes/interactive-mode";
|
||||
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { tinyTitleClient } from "@oh-my-pi/pi-coding-agent/tiny/title-client";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
// Issue #6462: the first submit used to spawn the local tiny-title worker
|
||||
// synchronously ahead of the first frame, and title generation started before
|
||||
// the optimistic user row painted. Startup now prewarms an idle worker, and the
|
||||
// submit handler paints the pending row before kicking off titling.
|
||||
describe("InteractiveMode tiny-title prewarm", () => {
|
||||
let authStorage: AuthStorage;
|
||||
let mode: InteractiveMode;
|
||||
let session: AgentSession;
|
||||
let tempDir: TempDir;
|
||||
// Titling (and thus the prewarm gate) is disabled when PI_NO_TITLE is set,
|
||||
// which the test env exports globally. Clear it per-test and restore after.
|
||||
let previousNoTitle: string | undefined;
|
||||
|
||||
beforeAll(() => {
|
||||
initTheme();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
// Keep ProcessTerminal.start() from writing escape queries to the real
|
||||
// terminal; the test only drives the mode API, not real terminal I/O.
|
||||
vi.spyOn(process.stdout, "write").mockReturnValue(true);
|
||||
vi.spyOn(process.stdin, "resume").mockReturnValue(process.stdin);
|
||||
vi.spyOn(process.stdin, "pause").mockReturnValue(process.stdin);
|
||||
vi.spyOn(process.stdin, "setEncoding").mockReturnValue(process.stdin);
|
||||
if (typeof process.stdin.setRawMode === "function") {
|
||||
vi.spyOn(process.stdin, "setRawMode").mockReturnValue(process.stdin);
|
||||
}
|
||||
|
||||
previousNoTitle = Bun.env.PI_NO_TITLE;
|
||||
delete Bun.env.PI_NO_TITLE;
|
||||
|
||||
resetSettingsForTest();
|
||||
tempDir = TempDir.createSync("@pi-interactive-mode-title-prewarm-");
|
||||
await Settings.init({ inMemory: true, cwd: tempDir.path() });
|
||||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||||
const modelRegistry = new ModelRegistry(authStorage);
|
||||
const model = modelRegistry.find("anthropic", "claude-sonnet-4-5");
|
||||
if (!model) {
|
||||
throw new Error("Expected claude-sonnet-4-5 to exist in registry");
|
||||
}
|
||||
|
||||
session = new AgentSession({
|
||||
agent: new Agent({
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
}),
|
||||
sessionManager: SessionManager.create(tempDir.path(), tempDir.path()),
|
||||
settings: Settings.isolated(),
|
||||
modelRegistry,
|
||||
});
|
||||
mode = new InteractiveMode(session, "test", undefined, () => {}, [], undefined, undefined);
|
||||
// A real fs.watch on repo HEAD in a parallel Bun worker can trip a Bun
|
||||
// SIGTRAP in the suite; this contract does not need branch watching.
|
||||
vi.spyOn(mode.statusLine, "watchBranch").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
mode?.stop();
|
||||
vi.restoreAllMocks();
|
||||
await session?.dispose();
|
||||
authStorage?.close();
|
||||
tempDir?.removeSync();
|
||||
resetSettingsForTest();
|
||||
if (previousNoTitle === undefined) delete Bun.env.PI_NO_TITLE;
|
||||
else Bun.env.PI_NO_TITLE = previousNoTitle;
|
||||
});
|
||||
|
||||
it("prewarms the configured local worker on startup for an unnamed session", async () => {
|
||||
session.settings.set("providers.tinyModel", "lfm2-350m");
|
||||
const prewarm = vi.spyOn(tinyTitleClient, "prewarm").mockImplementation(() => {});
|
||||
|
||||
await mode.init();
|
||||
|
||||
expect(prewarm).toHaveBeenCalledWith("lfm2-350m");
|
||||
});
|
||||
|
||||
it("does not prewarm when the session is already named", async () => {
|
||||
session.settings.set("providers.tinyModel", "lfm2-350m");
|
||||
vi.spyOn(mode.sessionManager, "getSessionName").mockReturnValue("resumed-session");
|
||||
const prewarm = vi.spyOn(tinyTitleClient, "prewarm").mockImplementation(() => {});
|
||||
|
||||
await mode.init();
|
||||
|
||||
expect(prewarm).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("paints the pending user row before starting title generation", async () => {
|
||||
await mode.init();
|
||||
|
||||
const order: string[] = [];
|
||||
vi.spyOn(mode, "startPendingSubmission").mockImplementation(input => {
|
||||
order.push("pending-row");
|
||||
return { text: input.text, cancelled: false, started: false };
|
||||
});
|
||||
const generateTitle = vi.spyOn(session, "generateTitle").mockImplementation(async () => {
|
||||
order.push("title-gen");
|
||||
return null;
|
||||
});
|
||||
const onInput = vi.fn();
|
||||
mode.onInputCallback = onInput;
|
||||
|
||||
await mode.editor.onSubmit?.("investigate the failing title worker");
|
||||
|
||||
expect(order).toEqual(["pending-row", "title-gen"]);
|
||||
expect(generateTitle).toHaveBeenCalledWith("investigate the failing title worker");
|
||||
expect(onInput).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,7 @@ import { isSubcommand } from "@oh-my-pi/pi-coding-agent/cli-commands";
|
||||
import { getDefault, getEnumValues, getUi } from "@oh-my-pi/pi-coding-agent/config/settings-schema";
|
||||
import { TinyTitleDownloadProgressComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tiny-title-download-progress";
|
||||
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import type { RefCountedWorkerHandle } from "@oh-my-pi/pi-coding-agent/subprocess/worker-client";
|
||||
import {
|
||||
TINY_MODEL_DEVICE_DEFAULT,
|
||||
TINY_MODEL_DEVICE_SETTING_OPTIONS,
|
||||
@@ -21,7 +22,12 @@ import {
|
||||
TINY_TITLE_MODEL_OPTIONS,
|
||||
TINY_TITLE_MODEL_VALUES,
|
||||
} from "@oh-my-pi/pi-coding-agent/tiny/models";
|
||||
import { createTinyTitleSubprocess, tinyTitleClient } from "@oh-my-pi/pi-coding-agent/tiny/title-client";
|
||||
import {
|
||||
createTinyTitleSubprocess,
|
||||
TinyTitleClient,
|
||||
tinyTitleClient,
|
||||
} from "@oh-my-pi/pi-coding-agent/tiny/title-client";
|
||||
import type { TinyTitleWorkerInbound, TinyTitleWorkerOutbound } from "@oh-my-pi/pi-coding-agent/tiny/title-protocol";
|
||||
import { generateSessionTitle } from "@oh-my-pi/pi-coding-agent/utils/title-generator";
|
||||
import type { Subprocess } from "bun";
|
||||
|
||||
@@ -199,6 +205,92 @@ describe("tiny title generator routing", () => {
|
||||
});
|
||||
});
|
||||
|
||||
interface FakeTinyWorker {
|
||||
handle: RefCountedWorkerHandle<TinyTitleWorkerInbound, TinyTitleWorkerOutbound>;
|
||||
sent: TinyTitleWorkerInbound[];
|
||||
refCount: number;
|
||||
emit(message: TinyTitleWorkerOutbound): void;
|
||||
}
|
||||
|
||||
function createFakeTinyWorker(): FakeTinyWorker {
|
||||
const sent: TinyTitleWorkerInbound[] = [];
|
||||
let onMessage: ((message: TinyTitleWorkerOutbound) => void) | undefined;
|
||||
const worker: FakeTinyWorker = {
|
||||
sent,
|
||||
refCount: 0,
|
||||
emit(message) {
|
||||
onMessage?.(message);
|
||||
},
|
||||
handle: {
|
||||
send(message) {
|
||||
sent.push(message);
|
||||
},
|
||||
onMessage(handler) {
|
||||
onMessage = handler;
|
||||
return () => {
|
||||
onMessage = undefined;
|
||||
};
|
||||
},
|
||||
onError() {
|
||||
return () => {};
|
||||
},
|
||||
async terminate() {},
|
||||
ref() {
|
||||
worker.refCount++;
|
||||
},
|
||||
unref() {
|
||||
worker.refCount--;
|
||||
},
|
||||
},
|
||||
};
|
||||
return worker;
|
||||
}
|
||||
|
||||
describe("tiny title prewarm", () => {
|
||||
it("spawns one idle worker that the first generate reuses (issue #6462)", async () => {
|
||||
const workers: FakeTinyWorker[] = [];
|
||||
let spawnCount = 0;
|
||||
const client = new TinyTitleClient(() => {
|
||||
spawnCount++;
|
||||
const worker = createFakeTinyWorker();
|
||||
workers.push(worker);
|
||||
return worker.handle;
|
||||
});
|
||||
|
||||
client.prewarm("lfm2-350m");
|
||||
|
||||
expect(spawnCount).toBe(1);
|
||||
// No pending request registered, so the prewarmed worker is never
|
||||
// referenced and never blocks process exit.
|
||||
expect(workers[0]?.refCount).toBe(0);
|
||||
// A no-op ping warms the transport without loading a model.
|
||||
expect(workers[0]?.sent).toEqual([{ type: "ping", id: expect.any(String) }]);
|
||||
|
||||
const generated = client.generate("lfm2-350m", "Investigate routing");
|
||||
// The first submit reuses the prewarmed worker — no second spawn.
|
||||
expect(spawnCount).toBe(1);
|
||||
|
||||
const request = workers[0]?.sent.find(message => message.type === "generate");
|
||||
expect(request?.type).toBe("generate");
|
||||
workers[0]?.emit({ type: "title", id: request?.id ?? "", title: "Routing" });
|
||||
|
||||
expect(await generated).toBe("Routing");
|
||||
await client.terminate();
|
||||
});
|
||||
|
||||
it("does not spawn a worker for the online default", () => {
|
||||
let spawnCount = 0;
|
||||
const client = new TinyTitleClient(() => {
|
||||
spawnCount++;
|
||||
return createFakeTinyWorker().handle;
|
||||
});
|
||||
|
||||
client.prewarm("online");
|
||||
|
||||
expect(spawnCount).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("tiny title subprocess", () => {
|
||||
it("does not inherit worker output into the interactive terminal", async () => {
|
||||
const calls: TinyWorkerSpawnCall[] = [];
|
||||
|
||||
Reference in New Issue
Block a user