smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): honored explicit local providers.tinyModel choice in title generation

Browse Source 
When the user set `providers.tinyModel` to a local key, `generateSessionTitle`
still raced local against the online `smol` path with a 10 s timeout and
silently fired the online request whenever the local worker returned `null`
(unknown key, model not downloaded, transformers.js failure). The online path
resolves the `smol` role through `priority.json` (haiku → flash → mini → …);
with an `OPENROUTER_API_KEY` picked up from env, that silently billed
OpenRouter without consent.

Drop the race entirely for local choices: honor the user's setting, log a
warning on local failure, leave the session untitled. The `raceFirstNonNull`
helper and `TITLE_LOCAL_FALLBACK_DELAY_MS` had no other consumer and are
removed; the obsolete \"silently bills online when local fails\" tests are
flipped into regressions that lock the no-fallback contract, including the
unknown-key path (e.g. \"ollama:gpt-oss\") which previously also leaked
straight through to the online billing path.

Fixes #3187
This commit is contained in:
roboomp
2026-06-21 10:51:04 +00:00
parent 35508c48f7
commit 853e8abda4
3 changed files with 65 additions and 256 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed session-title generation silently falling back to the online `smol` model (and billing whatever provider held the resolved API key — OpenRouter in the reporter's case) when the user had explicitly configured a **local** `providers.tinyModel`: `generateSessionTitle` raced local against online with a 10s timeout and fired the online request immediately whenever the local worker returned `null` (unknown key, model not downloaded, transformers.js failure). Now an explicit local-model choice is honored end-to-end — on local failure the session is left untitled with a `logger.warn` instead of billing the smol fallback ([#3187](https://github.com/can1357/oh-my-pi/issues/3187))
## [16.1.10] - 2026-06-21
### Added
packages/coding-agent/src/utils/title-generator.ts
+31 -99
View File
@@ -12,7 +12,7 @@ import type { Settings } from "../config/settings";
import titleMarkerInstruction from "../prompts/system/title-marker-instruction.md" with { type: "text" };
import titleSystemPrompt from "../prompts/system/title-system.md" with { type: "text" };
import titleMarkerSystemPrompt from "../prompts/system/title-system-marker.md" with { type: "text" };
import { ONLINE_TINY_TITLE_MODEL_KEY } from "../tiny/models";
import { isTinyTitleLocalModelKey, ONLINE_TINY_TITLE_MODEL_KEY } from "../tiny/models";
import { formatTitleUserMessage, isLowSignalTitleInput, normalizeGeneratedTitle } from "../tiny/text";
import { tinyTitleClient } from "../tiny/title-client";
@@ -23,7 +23,6 @@ const TITLE_MARKER_INSTRUCTION = prompt.render(titleMarkerInstruction);
const DEFAULT_TERMINAL_TITLE = "π";
const TERMINAL_TITLE_CONTROL_CHARS = /[\u0000-\u001f\u007f-\u009f]/g;
export const TITLE_LOCAL_FALLBACK_DELAY_MS = 10_000;
const TITLE_MAX_TOKENS = 30;
const REASONING_SAFE_MAX_TOKENS = 1024;
const SET_TITLE_TOOL_NAME = "set_title";
@@ -81,78 +80,6 @@ function getTitleModel(registry: ModelRegistry, settings: Settings, currentModel
return undefined;
}
export async function raceFirstNonNull<T>(
primary: Promise<T | null>,
startFallback: () => Promise<T | null>,
delayMs: number = TITLE_LOCAL_FALLBACK_DELAY_MS,
onPrimaryWinAfterFallback?: () => void,
): Promise<T | null> {
const { promise, resolve } = Promise.withResolvers<T | null>();
let resolved = false;
let primarySettled = false;
let fallbackStarted = false;
let fallbackSettled = false;
const resolveOnce = (value: T | null): void => {
if (resolved) return;
resolved = true;
resolve(value);
};
const maybeResolveNull = (): void => {
if (primarySettled && fallbackStarted && fallbackSettled) resolveOnce(null);
};
const startFallbackOnce = (): void => {
if (fallbackStarted || resolved) return;
fallbackStarted = true;
let fallback: Promise<T | null>;
try {
fallback = startFallback();
} catch {
fallbackSettled = true;
maybeResolveNull();
return;
}
void fallback.then(
value => {
fallbackSettled = true;
if (value !== null) resolveOnce(value);
else maybeResolveNull();
},
() => {
fallbackSettled = true;
maybeResolveNull();
},
);
};
const timer = setTimeout(startFallbackOnce, delayMs);
void primary.then(
value => {
primarySettled = true;
clearTimeout(timer);
if (value !== null) {
if (fallbackStarted) onPrimaryWinAfterFallback?.();
resolveOnce(value);
return;
}
startFallbackOnce();
maybeResolveNull();
},
() => {
primarySettled = true;
clearTimeout(timer);
startFallbackOnce();
maybeResolveNull();
},
);
try {
return await promise;
} finally {
clearTimeout(timer);
}
}
/**
* Generate a title for a session based on the first user message.
*
@@ -200,36 +127,41 @@ export async function generateSessionTitle(
);
}
const onlineAbortController = new AbortController();
const localTitlePromise = titleSystemPrompt
? tinyTitleClient.generate(tinyModel, firstMessage, { systemPrompt: titleSystemPrompt })
: tinyTitleClient.generate(tinyModel, firstMessage);
const localTitle = localTitlePromise.then(
title => title || null,
err => {
logger.warn("title-generator: local model error", {
// User explicitly picked a local tiny model. NEVER fall back to the online
// smol path (issue #3187): the smol role resolves through priority.json and
// silently bills whatever provider holds the resolved API key — OpenRouter
// in the reporter's case, leaking real credits without consent. If the
// local worker fails (unknown key, download missing, transformers.js
// crash, abort), leave the session untitled; the next user turn retries.
if (!isTinyTitleLocalModelKey(tinyModel)) {
logger.warn("title-generator: unknown local tiny model; skipping title (will not fall back to online)", {
sessionId,
model: tinyModel,
reason: "unknown-local-model",
});
return null;
}
try {
const localTitle = titleSystemPrompt
? await tinyTitleClient.generate(tinyModel, firstMessage, { systemPrompt: titleSystemPrompt })
: await tinyTitleClient.generate(tinyModel, firstMessage);
if (!localTitle) {
logger.warn("title-generator: local tiny model produced no title; skipping (no online fallback)", {
sessionId,
model: tinyModel,
error: err instanceof Error ? err.message : String(err),
reason: "local-no-output",
});
return null;
},
);
const startOnline = (): Promise<string | null> =>
generateTitleOnline(
firstMessage,
registry,
settings,
}
return localTitle;
} catch (err) {
logger.warn("title-generator: local tiny model errored; skipping (no online fallback)", {
sessionId,
currentModel,
metadataResolver,
onlineAbortController.signal,
titleSystemPrompt,
);
return raceFirstNonNull(localTitle, startOnline, TITLE_LOCAL_FALLBACK_DELAY_MS, () => {
onlineAbortController.abort();
});
model: tinyModel,
error: err instanceof Error ? err.message : String(err),
});
return null;
}
}
export async function generateTitleOnline(
packages/coding-agent/test/tiny-title-generator.test.ts
+30 -157
View File
@@ -1,5 +1,5 @@
import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test";
import type { Api, AssistantMessage, Model } from "@oh-my-pi/pi-ai";
import type { Api, Model } from "@oh-my-pi/pi-ai";
import * as ai from "@oh-my-pi/pi-ai";
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
import { isSubcommand } from "@oh-my-pi/pi-coding-agent/cli-commands";
@@ -22,17 +22,9 @@ import {
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 {
generateSessionTitle,
raceFirstNonNull,
TITLE_LOCAL_FALLBACK_DELAY_MS,
} from "@oh-my-pi/pi-coding-agent/utils/title-generator";
import { generateSessionTitle } from "@oh-my-pi/pi-coding-agent/utils/title-generator";
import type { Subprocess } from "bun";
async function flushMicrotasks(turns = 4): Promise<void> {
for (let i = 0; i < turns; i += 1) await Promise.resolve();
}
function getModelOrThrow(id: string): Model<Api> {
const model = getBundledModel("anthropic", id);
if (!model) throw new Error(`Expected model ${id}`);
@@ -114,102 +106,6 @@ afterEach(() => {
vi.restoreAllMocks();
});
describe("raceFirstNonNull", () => {
it("resolves with local result without starting fallback", async () => {
let fallbackStarted = false;
const title = await raceFirstNonNull(
Promise.resolve("Local Title"),
() => {
fallbackStarted = true;
return Promise.resolve("Online Title");
},
TITLE_LOCAL_FALLBACK_DELAY_MS,
);
expect(title).toBe("Local Title");
expect(fallbackStarted).toBe(false);
});
it("starts fallback after the hardcoded delay", async () => {
vi.useFakeTimers();
const local = Promise.withResolvers<string | null>();
let fallbackStarted = false;
const result = raceFirstNonNull(
local.promise,
() => {
fallbackStarted = true;
return Promise.resolve("Online Title");
},
TITLE_LOCAL_FALLBACK_DELAY_MS,
);
await flushMicrotasks();
expect(fallbackStarted).toBe(false);
vi.advanceTimersByTime(TITLE_LOCAL_FALLBACK_DELAY_MS - 1);
await flushMicrotasks();
expect(fallbackStarted).toBe(false);
vi.advanceTimersByTime(1);
await flushMicrotasks();
expect(fallbackStarted).toBe(true);
await expect(result).resolves.toBe("Online Title");
local.resolve(null);
});
it("starts fallback immediately when local fails", async () => {
let fallbackStarted = false;
const title = await raceFirstNonNull(
Promise.reject(new Error("local failed")),
() => {
fallbackStarted = true;
return Promise.resolve("Online Title");
},
TITLE_LOCAL_FALLBACK_DELAY_MS,
);
expect(title).toBe("Online Title");
expect(fallbackStarted).toBe(true);
});
it("returns null only after local and fallback return null", async () => {
const title = await raceFirstNonNull(
Promise.resolve(null),
() => Promise.resolve(null),
TITLE_LOCAL_FALLBACK_DELAY_MS,
);
expect(title).toBeNull();
});
it("runs the loser-cancel callback when local wins after fallback starts", async () => {
vi.useFakeTimers();
const local = Promise.withResolvers<string | null>();
const fallback = Promise.withResolvers<string | null>();
let fallbackStarted = false;
let cancelCount = 0;
const result = raceFirstNonNull(
local.promise,
() => {
fallbackStarted = true;
return fallback.promise;
},
TITLE_LOCAL_FALLBACK_DELAY_MS,
() => {
cancelCount += 1;
},
);
vi.advanceTimersByTime(TITLE_LOCAL_FALLBACK_DELAY_MS);
await flushMicrotasks();
expect(fallbackStarted).toBe(true);
local.resolve("Local Title");
await expect(result).resolves.toBe("Local Title");
expect(cancelCount).toBe(1);
fallback.resolve(null);
});
});
describe("tiny title generator routing", () => {
it("keeps online-only behavior when Tiny Model is Online", async () => {
const model = getModelOrThrow("claude-sonnet-4-5");
@@ -264,10 +160,10 @@ describe("tiny title generator routing", () => {
expect(online).not.toHaveBeenCalled();
});
it("starts online fallback immediately when local returns null", async () => {
it("does NOT fall back to online when local returns null (issue #3187)", async () => {
const model = getModelOrThrow("claude-sonnet-4-5");
vi.spyOn(tinyTitleClient, "generate").mockResolvedValue(null);
const online = mockOnlineTitle("Online Title");
const local = vi.spyOn(tinyTitleClient, "generate").mockResolvedValue(null);
const online = mockOnlineTitle("Billed Online Title");
const title = await generateSessionTitle(
"Investigate fallback",
@@ -275,63 +171,40 @@ describe("tiny title generator routing", () => {
createSettings(model, "lfm2-350m"),
);
expect(title).toBe("Online Title");
expect(online).toHaveBeenCalledTimes(1);
expect(title).toBeNull();
expect(local).toHaveBeenCalledTimes(1);
expect(online).not.toHaveBeenCalled();
});
it("aborts the online request when delayed local generation wins", async () => {
vi.useFakeTimers();
it("does NOT fall back to online when local throws", async () => {
const model = getModelOrThrow("claude-sonnet-4-5");
const local = Promise.withResolvers<string | null>();
const onlineHold = Promise.withResolvers<AssistantMessage>();
let onlineSignal: AbortSignal | undefined;
vi.spyOn(tinyTitleClient, "generate").mockReturnValue(local.promise);
vi.spyOn(ai, "completeSimple").mockImplementation((_model, _context, options) => {
onlineSignal = options?.signal;
return onlineHold.promise;
});
vi.spyOn(tinyTitleClient, "generate").mockRejectedValue(new Error("worker crashed"));
const online = mockOnlineTitle("Billed Online Title");
const result = generateSessionTitle(
"Investigate cancellation",
createRegistry(model),
createSettings(model, "lfm2-350m"),
);
vi.advanceTimersByTime(TITLE_LOCAL_FALLBACK_DELAY_MS);
await flushMicrotasks();
expect(onlineSignal?.aborted).toBe(false);
local.resolve("Local Title");
await expect(result).resolves.toBe("Local Title");
expect(onlineSignal?.aborted).toBe(true);
onlineHold.resolve({ stopReason: "abort", content: [] } as never);
});
it("keeps local generation alive when the delayed online fallback wins", async () => {
vi.useFakeTimers();
const model = getModelOrThrow("claude-sonnet-4-5");
const local = Promise.withResolvers<string | null>();
let localSettled = false;
void local.promise.then(() => {
localSettled = true;
});
vi.spyOn(tinyTitleClient, "generate").mockReturnValue(local.promise);
mockOnlineTitle("Online Title");
const result = generateSessionTitle(
"Investigate background download",
const title = await generateSessionTitle(
"Investigate crash",
createRegistry(model),
createSettings(model, "lfm2-700m"),
);
vi.advanceTimersByTime(TITLE_LOCAL_FALLBACK_DELAY_MS);
await flushMicrotasks();
await expect(result).resolves.toBe("Online Title");
expect(localSettled).toBe(false);
expect(title).toBeNull();
expect(online).not.toHaveBeenCalled();
});
local.resolve("Late Local Title");
await flushMicrotasks();
expect(localSettled).toBe(true);
it("does NOT call the local worker or online path for an unknown tinyModel key", async () => {
const model = getModelOrThrow("claude-sonnet-4-5");
const local = vi.spyOn(tinyTitleClient, "generate").mockResolvedValue("Late Local");
const online = mockOnlineTitle("Billed Online Title");
const title = await generateSessionTitle(
"Investigate unknown",
createRegistry(model),
createSettings(model, "ollama:gpt-oss"),
);
expect(title).toBeNull();
expect(local).not.toHaveBeenCalled();
expect(online).not.toHaveBeenCalled();
});
});