diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 220bf8ad0..0865903db 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed session title generation, commit-message generation, speech-enhancer rewrites, and the online auto-thinking and unexpected-stop classifiers silently truncating on non-reasoning-flagged models that still emit thinking output (e.g. Qwen3 served via llama.cpp) by always sizing the completion budget for backends that ignore `disableReasoning` ([#4355](https://github.com/can1357/oh-my-pi/issues/4355)) + ## [16.3.3] - 2026-07-02 ### Breaking Changes diff --git a/packages/coding-agent/src/auto-thinking/classifier.ts b/packages/coding-agent/src/auto-thinking/classifier.ts index 40b34458a..a6c0540de 100644 --- a/packages/coding-agent/src/auto-thinking/classifier.ts +++ b/packages/coding-agent/src/auto-thinking/classifier.ts @@ -35,13 +35,14 @@ const DIFFICULTY_SYSTEM_PROMPT = prompt.render(difficultySystemPrompt); const MAX_INPUT_CHARS = 6000; const HEAD_CHARS = 4000; const TAIL_CHARS = 2000; -/** The online answer is a single word; keep budgets tiny for non-reasoning backends. */ -const ANSWER_MAX_TOKENS = 8; /** Local classifiers occasionally need more room for chat-template boilerplate. */ const LOCAL_ANSWER_MAX_TOKENS = 16; /** - * Reasoning backends ignore `disableReasoning` on some providers, so reserve - * enough output room for the keyword to still land after unavoidable thinking. + * Online classifier budget. Sized to survive backends that ignore + * `disableReasoning` (e.g. Qwen3 via llama.cpp catalogued `reasoning: false` + * but still emitting thinking): the classifier keyword needs to land after any + * unavoidable thinking preamble. `maxTokens` is a hard cap — non-thinking + * completions still return in a handful of tokens (issue #4355). */ const REASONING_SAFE_MAX_TOKENS = 1024; @@ -85,7 +86,7 @@ async function classifyOnline(input: string, deps: ClassifyDifficultyDeps): Prom } // Resolve metadata after getApiKey so the session-sticky credential is recorded first. const metadata = deps.metadataResolver?.(model.provider); - const maxTokens = model.reasoning ? Math.max(ANSWER_MAX_TOKENS, REASONING_SAFE_MAX_TOKENS) : ANSWER_MAX_TOKENS; + const maxTokens = REASONING_SAFE_MAX_TOKENS; const response = await completeSimple( model, diff --git a/packages/coding-agent/src/session/unexpected-stop-classifier.ts b/packages/coding-agent/src/session/unexpected-stop-classifier.ts index f2cfe1465..5bf7c3456 100644 --- a/packages/coding-agent/src/session/unexpected-stop-classifier.ts +++ b/packages/coding-agent/src/session/unexpected-stop-classifier.ts @@ -16,8 +16,11 @@ const CLASSIFIER_SYSTEM_PROMPT = prompt.render(unexpectedStopClassifierPrompt); */ const ANSWER_MAX_TOKENS = 16; /** - * Reasoning backends ignore `disableReasoning` on some providers, so reserve - * enough output room for the keyword to still land after unavoidable thinking. + * Online classifier budget. Sized to survive backends that ignore + * `disableReasoning` (e.g. Qwen3 via llama.cpp catalogued `reasoning: false` + * but still emitting thinking): the yes/no keyword needs to land after any + * unavoidable thinking preamble. `maxTokens` is a hard cap — non-thinking + * completions still return in a single word (issue #4355). */ const REASONING_SAFE_MAX_TOKENS = 1024; @@ -74,7 +77,7 @@ async function classifyOnline(text: string, deps: ClassifyUnexpectedStopDeps): P throw new Error(`unexpected-stop: no API key for ${model.provider}/${model.id}`); } const metadata = deps.metadataResolver?.(model.provider); - const maxTokens = model.reasoning ? Math.max(ANSWER_MAX_TOKENS, REASONING_SAFE_MAX_TOKENS) : ANSWER_MAX_TOKENS; + const maxTokens = REASONING_SAFE_MAX_TOKENS; const response = await completeSimple( model, diff --git a/packages/coding-agent/src/tts/speech-enhancer.ts b/packages/coding-agent/src/tts/speech-enhancer.ts index e6320baf6..f53f2d188 100644 --- a/packages/coding-agent/src/tts/speech-enhancer.ts +++ b/packages/coding-agent/src/tts/speech-enhancer.ts @@ -23,10 +23,12 @@ import type { Settings } from "../config/settings"; import speechRewritePrompt from "../prompts/system/speech-rewrite.md" with { type: "text" }; const SYSTEM_PROMPT = prompt.render(speechRewritePrompt); -/** Rewrite budget: a paragraph in, a spoken paragraph (usually shorter) out. */ -const ANSWER_MAX_TOKENS = 512; -/** Reasoning backends may burn tokens before the answer despite `disableReasoning`. */ -const REASONING_SAFE_MAX_TOKENS = 1536; +// Rewrite budget: a paragraph in, a spoken paragraph (usually shorter) out. +// Always reserve enough room to survive backends that ignore `disableReasoning` +// (e.g. Qwen3 via llama.cpp catalogued `reasoning: false` but still emitting +// thinking). `maxTokens` is a hard cap — non-thinking completions still return +// in a normal spoken-paragraph budget (issue #4355). +const ANSWER_MAX_TOKENS = 1536; /** Per-block completion deadline before falling back to mechanical cleanup. */ const REWRITE_TIMEOUT_MS = 6000; /** Bound block characters sent to the model (huge diffs/code dumps get elided). */ @@ -89,7 +91,7 @@ export class SpeechEnhancer { }, { apiKey: registry.resolver(model, sessionId), - maxTokens: model.reasoning ? REASONING_SAFE_MAX_TOKENS : ANSWER_MAX_TOKENS, + maxTokens: ANSWER_MAX_TOKENS, disableReasoning: true, metadata, signal: signal ? AbortSignal.any([signal, timeout]) : timeout, diff --git a/packages/coding-agent/src/utils/commit-message-generator.ts b/packages/coding-agent/src/utils/commit-message-generator.ts index 8800e499e..2a242f205 100644 --- a/packages/coding-agent/src/utils/commit-message-generator.ts +++ b/packages/coding-agent/src/utils/commit-message-generator.ts @@ -16,8 +16,12 @@ import { concreteThinkingLevel, toReasoningEffort } from "../thinking"; const COMMIT_SYSTEM_PROMPT = prompt.render(commitSystemPrompt); const MAX_DIFF_CHARS = 4000; -const COMMIT_MAX_TOKENS = 60; -const REASONING_SAFE_MAX_TOKENS = 1024; +// Cover the "backend ignores `disableReasoning`" case unconditionally: the +// static `model.reasoning` catalog flag can't distinguish a thinking model +// declared `reasoning: false` (e.g. Qwen3 served locally via llama.cpp) from +// one that never emits thinking. `maxTokens` is a hard cap — non-thinking +// completions still return in a handful of tokens (issue #4355). +const COMMIT_MAX_TOKENS = 1024; /** File patterns that should be excluded from commit message generation diffs. */ const NOISE_SUFFIXES = [".lock", ".lockb", "-lock.json", "-lock.yaml"]; @@ -101,9 +105,7 @@ export async function generateCommitMessage( if (!apiKey) continue; try { - const maxTokens = candidate.model.reasoning - ? Math.max(COMMIT_MAX_TOKENS, REASONING_SAFE_MAX_TOKENS) - : COMMIT_MAX_TOKENS; + const maxTokens = COMMIT_MAX_TOKENS; const response = await completeSimple( candidate.model, { diff --git a/packages/coding-agent/src/utils/title-generator.ts b/packages/coding-agent/src/utils/title-generator.ts index 5fee0646d..f40911877 100644 --- a/packages/coding-agent/src/utils/title-generator.ts +++ b/packages/coding-agent/src/utils/title-generator.ts @@ -23,8 +23,15 @@ const TITLE_MARKER_INSTRUCTION = prompt.render(titleMarkerInstruction); const DEFAULT_TERMINAL_TITLE = "π"; const TERMINAL_TITLE_CONTROL_CHARS = /[\u0000-\u001f\u007f-\u009f]/g; -const TITLE_MAX_TOKENS = 30; -const REASONING_SAFE_MAX_TOKENS = 1024; +// Cover the "backend ignores `disableReasoning`" case unconditionally: the +// static `model.reasoning` catalog flag can't distinguish a thinking model that +// was declared with `reasoning: false` (e.g. Qwen3 served locally via llama.cpp, +// whose bundled jinja chat template forces `enable_thinking: true`) from one +// that never emits thinking. `maxTokens` is a hard cap, not a target — the +// happy-path completion still returns in a handful of tokens, so raising the +// ceiling costs nothing when thinking is genuinely suppressed and keeps the +// forced `set_title` tool call reachable when it isn't (issue #4355). +const TITLE_MAX_TOKENS = 1024; const SET_TITLE_TOOL_NAME = "set_title"; const setTitleTool: Tool = { @@ -212,11 +219,9 @@ export async function generateTitleOnline( // account_uuid rather than the snapshot-at-call-site value. const metadata = metadataResolver?.(model.provider); - // Title generation is a 3-7 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; + // Title generation is a 3-7 word task, but the ceiling has to survive + // backends that ignore `disableReasoning` (see TITLE_MAX_TOKENS above). + const maxTokens = TITLE_MAX_TOKENS; logger.debug("title-generator: request", { ...modelContext, maxTokens }); const response = await completeSimple( diff --git a/packages/coding-agent/test/title-generator.test.ts b/packages/coding-agent/test/title-generator.test.ts index a3d5b0163..b4b29de82 100644 --- a/packages/coding-agent/test/title-generator.test.ts +++ b/packages/coding-agent/test/title-generator.test.ts @@ -258,6 +258,36 @@ describe("title generator", () => { expect(maxTokens).toBeGreaterThanOrEqual(1024); }); + // Regression for #4355: a model catalogued with `reasoning: false` that + // still emits thinking (e.g. Qwen3 via llama.cpp) must get the same + // reasoning-safe budget, otherwise the forced `set_title` tool call is + // truncated before it can be emitted. + it("uses a reasoning-safe output budget even when the model declares reasoning: false", async () => { + const baseModel = getModelOrThrow("claude-sonnet-4-5"); + const model = { ...baseModel, reasoning: false } as Model; + const completeSimpleMock = vi.spyOn(ai, "completeSimple").mockResolvedValue({ + stopReason: "stop", + content: [ + { + type: "toolCall", + id: "call-title", + name: "set_title", + arguments: { title: "Budget Title" }, + }, + ], + } as never); + + const title = await generateSessionTitle( + "Investigate the resolver", + createRegistry(model), + createSettings(model), + ); + const maxTokens = (completeSimpleMock.mock.calls[0]?.[2] as { maxTokens?: number } | undefined)?.maxTokens; + + expect(title).toBe("Budget Title"); + expect(maxTokens).toBeGreaterThanOrEqual(1024); + }); + it("strips code blocks from the message sent to the model", async () => { const model = getModelOrThrow("claude-sonnet-4-5"); const completeSimpleMock = vi.spyOn(ai, "completeSimple").mockResolvedValue({