fix(coding-agent): surfaced title generation failures
Logged structured session-title generation skip and failure outcomes with session/model context, and prevented credential lookup errors from escaping into the caller's swallowed promise path. Added regression coverage for missing and failing title credentials. Fixes #1892
This commit is contained in:
@@ -19,6 +19,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed auto session-title generation failures being swallowed without an actionable diagnostic. Title generation now logs structured start, missing-model/API-key, provider-error, empty-result, and exception outcomes with the session id and resolved title model; the interactive auto-title caller also logs uncaught persistence/generation errors instead of dropping them. ([#1892](https://github.com/can1357/oh-my-pi/issues/1892))
|
||||
|
||||
- Fixed a streamed assistant message freezing at a partial prefix (e.g. only "Nat" of "Natives built, now…") on ED3-risk terminals (Ghostty/kitty/iTerm2/Alacritty), with the final text appearing only after a resize. `TranscriptContainer` freezes each non-live block by replaying its last live render, but render coalescing can finalize a block's content and append the next block within the same throttled frame — so the block was sealed at its stale mid-stream snapshot and never repainted until the next `thaw`. The block that was live on the previous render is now recomputed once on the live→frozen transition, sealing it at its final content.
|
||||
|
||||
- Fixed ACP/RPC stdio startup so protocol frames are no longer consumed as one-shot piped prompt input before the JSON-RPC transport starts.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import * as fs from "node:fs/promises";
|
||||
import type { AutocompleteProvider, SlashCommand } from "@oh-my-pi/pi-tui";
|
||||
import { $env, sanitizeText } from "@oh-my-pi/pi-utils";
|
||||
import { $env, logger, sanitizeText } from "@oh-my-pi/pi-utils";
|
||||
import { getRoleInfo } from "../../config/model-registry";
|
||||
import { isSettingsInitialized, settings } from "../../config/settings";
|
||||
import { renderSegmentTrack } from "../../modes/components/segment-track";
|
||||
@@ -406,7 +406,13 @@ export class InputController {
|
||||
}
|
||||
}
|
||||
})
|
||||
.catch(() => {});
|
||||
.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) {
|
||||
|
||||
@@ -149,7 +149,10 @@ export async function generateSessionTitle(
|
||||
// tiny title model can't reliably decline trivial input, so this happens
|
||||
// deterministically before any model is invoked; the caller retries on the
|
||||
// next user message while the session stays unnamed.
|
||||
if (isLowSignalTitleInput(firstMessage)) return null;
|
||||
if (isLowSignalTitleInput(firstMessage)) {
|
||||
logger.debug("title-generator: skipped low-signal input", { sessionId, reason: "low-signal" });
|
||||
return null;
|
||||
}
|
||||
|
||||
const tinyModel = settings.get("providers.tinyModel");
|
||||
if (tinyModel === ONLINE_TINY_TITLE_MODEL_KEY) {
|
||||
@@ -159,7 +162,14 @@ export async function generateSessionTitle(
|
||||
const onlineAbortController = new AbortController();
|
||||
const localTitle = tinyTitleClient.generate(tinyModel, firstMessage).then(
|
||||
title => title || null,
|
||||
() => null,
|
||||
err => {
|
||||
logger.warn("title-generator: local model error", {
|
||||
sessionId,
|
||||
model: tinyModel,
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return null;
|
||||
},
|
||||
);
|
||||
const startOnline = (): Promise<string | null> =>
|
||||
generateTitleOnline(
|
||||
@@ -188,49 +198,48 @@ export async function generateTitleOnline(
|
||||
): Promise<string | null> {
|
||||
const model = getTitleModel(registry, settings, currentModel);
|
||||
if (!model) {
|
||||
logger.debug("title-generator: no title model found");
|
||||
logger.warn("title-generator: no title model found", { sessionId, reason: "no-title-model" });
|
||||
return null;
|
||||
}
|
||||
|
||||
const userMessage = formatTitleUserMessage(firstMessage);
|
||||
|
||||
const apiKey = await registry.getApiKey(model, sessionId);
|
||||
if (!apiKey) {
|
||||
logger.debug("title-generator: no API key for smol model", {
|
||||
provider: model.provider,
|
||||
id: model.id,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
// Resolve metadata after getApiKey so the session-sticky credential for this
|
||||
// request is already recorded; metadataResolver can then return the correct
|
||||
// account_uuid rather than the snapshot-at-call-site value.
|
||||
const metadata = metadataResolver?.(model.provider);
|
||||
|
||||
// Title generation is a 3-6 word task, but some reasoning backends ignore
|
||||
// disableReasoning. Keep the normal cheap budget for non-reasoning models
|
||||
// while reserving enough output room for reasoning models to still emit
|
||||
// the forced tool call after any unavoidable thinking tokens.
|
||||
const maxTokens = model.reasoning ? Math.max(TITLE_MAX_TOKENS, REASONING_SAFE_MAX_TOKENS) : TITLE_MAX_TOKENS;
|
||||
const request = {
|
||||
model: `${model.provider}/${model.id}`,
|
||||
systemPrompt: TITLE_SYSTEM_PROMPT,
|
||||
userMessage,
|
||||
maxTokens,
|
||||
const modelName = `${model.provider}/${model.id}`;
|
||||
const modelContext = {
|
||||
sessionId,
|
||||
provider: model.provider,
|
||||
id: model.id,
|
||||
model: modelName,
|
||||
};
|
||||
logger.debug("title-generator: request", request);
|
||||
logger.debug("title-generator: start", modelContext);
|
||||
|
||||
try {
|
||||
const apiKey = await registry.getApiKey(model, sessionId);
|
||||
if (!apiKey) {
|
||||
logger.warn("title-generator: no API key", { ...modelContext, reason: "missing-api-key" });
|
||||
return null;
|
||||
}
|
||||
// Resolve metadata after getApiKey so the session-sticky credential for this
|
||||
// request is already recorded; metadataResolver can then return the correct
|
||||
// account_uuid rather than the snapshot-at-call-site value.
|
||||
const metadata = metadataResolver?.(model.provider);
|
||||
|
||||
// Title generation is a 3-6 word task, but some reasoning backends ignore
|
||||
// disableReasoning. Keep the normal cheap budget for non-reasoning models
|
||||
// while reserving enough output room for reasoning models to still emit
|
||||
// the forced tool call after any unavoidable thinking tokens.
|
||||
const maxTokens = model.reasoning ? Math.max(TITLE_MAX_TOKENS, REASONING_SAFE_MAX_TOKENS) : TITLE_MAX_TOKENS;
|
||||
logger.debug("title-generator: request", { ...modelContext, maxTokens });
|
||||
|
||||
const response = await completeSimple(
|
||||
model,
|
||||
{
|
||||
systemPrompt: [request.systemPrompt],
|
||||
messages: [{ role: "user", content: request.userMessage, timestamp: Date.now() }],
|
||||
systemPrompt: [TITLE_SYSTEM_PROMPT],
|
||||
messages: [{ role: "user", content: userMessage, timestamp: Date.now() }],
|
||||
tools: [setTitleTool],
|
||||
},
|
||||
{
|
||||
apiKey,
|
||||
maxTokens: request.maxTokens,
|
||||
maxTokens,
|
||||
disableReasoning: true,
|
||||
toolChoice: { type: "tool", name: SET_TITLE_TOOL_NAME },
|
||||
metadata,
|
||||
@@ -239,8 +248,9 @@ export async function generateTitleOnline(
|
||||
);
|
||||
|
||||
if (response.stopReason === "error") {
|
||||
logger.debug("title-generator: response error", {
|
||||
model: request.model,
|
||||
logger.warn("title-generator: response error", {
|
||||
...modelContext,
|
||||
reason: "provider-response-error",
|
||||
stopReason: response.stopReason,
|
||||
errorMessage: response.errorMessage,
|
||||
});
|
||||
@@ -249,8 +259,18 @@ export async function generateTitleOnline(
|
||||
|
||||
const title = normalizeGeneratedTitle(extractGeneratedTitle(response.content));
|
||||
|
||||
logger.debug("title-generator: response", {
|
||||
model: request.model,
|
||||
if (!title) {
|
||||
logger.debug("title-generator: no title returned", {
|
||||
...modelContext,
|
||||
reason: "model-returned-none",
|
||||
usage: response.usage,
|
||||
stopReason: response.stopReason,
|
||||
});
|
||||
return null;
|
||||
}
|
||||
|
||||
logger.debug("title-generator: success", {
|
||||
...modelContext,
|
||||
title,
|
||||
usage: response.usage,
|
||||
stopReason: response.stopReason,
|
||||
@@ -258,8 +278,9 @@ export async function generateTitleOnline(
|
||||
|
||||
return title;
|
||||
} catch (err) {
|
||||
logger.debug("title-generator: error", {
|
||||
model: request.model,
|
||||
logger.warn("title-generator: error", {
|
||||
...modelContext,
|
||||
reason: "exception",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
});
|
||||
return null;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as ai from "@oh-my-pi/pi-ai";
|
||||
import { type Api, getBundledModel, type Model } from "@oh-my-pi/pi-ai";
|
||||
import { logger } from "@oh-my-pi/pi-utils";
|
||||
import { generateSessionTitle } from "../src/utils/title-generator";
|
||||
|
||||
function getModelOrThrow(id: string): Model<Api> {
|
||||
@@ -116,6 +117,65 @@ describe("title generator", () => {
|
||||
expect(completeSimpleMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("logs and returns null when title credentials are missing", async () => {
|
||||
const model = getModelOrThrow("claude-sonnet-4-5");
|
||||
const completeSimpleMock = vi.spyOn(ai, "completeSimple");
|
||||
const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {});
|
||||
|
||||
const title = await generateSessionTitle(
|
||||
"Investigate the resolver",
|
||||
{
|
||||
getAvailable: () => [model],
|
||||
getApiKey: async () => undefined,
|
||||
} as never,
|
||||
createSettings(model),
|
||||
"session-1",
|
||||
);
|
||||
|
||||
expect(title).toBeNull();
|
||||
expect(completeSimpleMock).not.toHaveBeenCalled();
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
"title-generator: no API key",
|
||||
expect.objectContaining({
|
||||
sessionId: "session-1",
|
||||
provider: model.provider,
|
||||
id: model.id,
|
||||
reason: "missing-api-key",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("logs and returns null when title credential lookup throws", async () => {
|
||||
const model = getModelOrThrow("claude-sonnet-4-5");
|
||||
const completeSimpleMock = vi.spyOn(ai, "completeSimple");
|
||||
const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {});
|
||||
|
||||
const title = await generateSessionTitle(
|
||||
"Investigate the resolver",
|
||||
{
|
||||
getAvailable: () => [model],
|
||||
getApiKey: async () => {
|
||||
throw new Error("credential lookup failed");
|
||||
},
|
||||
} as never,
|
||||
createSettings(model),
|
||||
"session-2",
|
||||
);
|
||||
|
||||
expect(title).toBeNull();
|
||||
expect(completeSimpleMock).not.toHaveBeenCalled();
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
"title-generator: error",
|
||||
expect.objectContaining({
|
||||
sessionId: "session-2",
|
||||
provider: model.provider,
|
||||
id: model.id,
|
||||
reason: "exception",
|
||||
error: "credential lookup failed",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses a reasoning-safe output budget for reasoning models", async () => {
|
||||
const model = getModelOrThrow("claude-sonnet-4-5");
|
||||
const completeSimpleMock = vi.spyOn(ai, "completeSimple").mockResolvedValue({
|
||||
|
||||
Reference in New Issue
Block a user