From 4e434075188b08a2a4ef547b3a0aad1b3b1ec5df Mon Sep 17 00:00:00 2001 From: WiseGuru <42100212+WiseGuru@users.noreply.github.com> Date: Mon, 1 Jun 2026 18:32:16 -0700 Subject: [PATCH] Max tokens warning clarification --- CLAUDE.md | 2 ++ src/llm/anthropic.ts | 4 ++-- src/llm/index.ts | 27 +++++++++++++++++++++++++++ src/llm/openai.ts | 21 ++++++++++++++++++--- 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ae485e5..241b996 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -262,6 +262,8 @@ Per [.editorconfig](.editorconfig): tabs (width 4), LF, UTF-8, final newline. Ma - **`saveManyKeys` is a silent no-op when locked.** When `mode === 'passphrase'` and `unlockedKey === null`, `saveManyKeys` does nothing. This is deliberate: unrelated `plugin.saveSettings()` calls (e.g. user changes a model dropdown) would otherwise persist empty `apiKey` values for every profile, wiping the on-disk encrypted bag. The UI prevents this by disabling key fields and gating all pipeline entry points on `encryptionStatus.locked`. `saveKey` (single-key write) still throws so callers can react. - **Keep zxcvbn lazy and the strength API async.** [src/passphrase-strength.ts](src/passphrase-strength.ts) pulls `@zxcvbn-ts/*` via dynamic `import()` specifically so the ~1.6 MB of dictionaries are not parsed/constructed at plugin load (measured: ~9 ms startup vs ~47 ms with a static import). Do not "simplify" it back to a top-level static import or a synchronous `evaluatePassphrase` — that re-adds the cost to every Obsidian launch on every device. The async ripples (modal `updateStrength` race guard, `await isPassphraseAcceptable` in `changeEncryptionMode`) are intentional. `warmPassphraseStrength()` is fired on passphrase-modal open to hide the one-time build behind the modal animation. - **`maxTokens` has two settings-tab views over one value.** `LLMConfig.maxTokens` (per profile, default `2560`) is the single source of truth. The normal-area "Maximum note length" dropdown (`renderNoteLength` in [src/settings/tab.ts](src/settings/tab.ts)) frames it in minutes via `NOTE_LENGTH_PRESETS` at `TOKENS_PER_MIN = 256` (≈150 wpm × ~1.3 tokens/word, padded ~20% for headings/bullets/`Speaker X:` labels; ~10 min → 2560). The Advanced "LLM max tokens" text field edits the same number raw; a value not matching a preset surfaces in the dropdown as a "Custom (N tokens, ~M min)" option. The dropdown calls `this.display()` on change so Advanced reflects the new number; the raw text field does not redraw (focus preservation), so the dropdown updates on the next full render. The Anthropic adapter's `config.maxTokens > 0 ? ... : 2560` fallback ([src/llm/anthropic.ts](src/llm/anthropic.ts)) must stay in sync with the default. The cap is on **output** tokens, so it bounds note length, not input. +- **`maxTokens` over a model's output cap is remapped to a friendly error.** When the requested output cap exceeds a model's max output tokens, Anthropic and OpenAI return a cryptic HTTP 400. `remapOutputLimitError` ([src/llm/index.ts](src/llm/index.ts)), applied as a `.catch()` on the `complete()` POST in [src/llm/anthropic.ts](src/llm/anthropic.ts) and [src/llm/openai.ts](src/llm/openai.ts), detects that specific 400 (body names `max_tokens`/`max_completion_tokens` + a "maximum/too large/at most/exceed" phrase) and rethrows a `ProviderError` pointing at the "Maximum note length" setting; all other errors pass through unchanged. Its `never` return preserves the awaited type. Gemini silently clamps `maxOutputTokens` instead of erroring, so it gets no remap (and can therefore truncate without warning on long notes). +- **OpenAI reasoning models need `max_completion_tokens`, not `max_tokens`.** [src/llm/openai.ts](src/llm/openai.ts) `usesCompletionTokens(id, model)` switches the param name to `max_completion_tokens` when `id === 'openai'` and the model matches `/^(o\d|gpt-5)/i` (o1/o3/o4 + gpt-5 families), which reject the legacy `max_tokens`. Scoped to the first-party `openai` id only; `openai-compatible` and `mistral` keep `max_tokens`, so a reasoning model proxied behind an openai-compatible endpoint is a known gap. This only fixes the token param; o1-mini/o1-preview also reject `system` messages, which is not handled. - **No baked-in model defaults.** Both profiles ship with `model: ""`. The modal renders an inline setup card that blocks recording/paste until the active profile has a provider, model, key, and (for `openai-compatible`) base URL. If you add a provider, do not bake a default model string; surface it as placeholder hint text. - **`openai-compatible` base URL asymmetry** (literal interpretation of the spec): transcription appends `/v1/audio/transcriptions` to a *root* URL (`http://localhost:8080`); LLM appends `/chat/completions` to a URL that *already includes* `/v1` (`http://localhost:11434/v1`). The settings UI hint text and setup card both guide users; do not "normalize" one to match the other. - **`setHeading()` instead of manual `

`** inside settings tabs. `obsidianmd/settings-tab/no-manual-html-headings` forbids manual headings. Same applies anywhere else inside a settings tab that needs a section header. In [src/settings/tab.ts](src/settings/tab.ts), section headings go through the `sectionHeading(parent, name, icon)` helper, which builds the `setHeading()` Setting and prepends a Lucide icon (via `setIcon`) to its `nameEl` styled by `.rewrite-heading-icon`. It returns the `Setting` so callers that attach a status badge (profile, shared core) still reach `nameEl`. Add new headings via this helper, not a bare `new Setting(...).setHeading()`, so they keep an icon. diff --git a/src/llm/anthropic.ts b/src/llm/anthropic.ts index eeac218..ffbb474 100644 --- a/src/llm/anthropic.ts +++ b/src/llm/anthropic.ts @@ -1,6 +1,6 @@ import { LLMConfig } from '../types'; import { jsonGet, jsonPost } from '../http'; -import { LLMProvider } from './index'; +import { LLMProvider, remapOutputLimitError } from './index'; interface MessagesResponse { content?: Array<{ type?: string; text?: string }>; @@ -42,7 +42,7 @@ export function createAnthropicLLM(): LLMProvider { ...ANTHROPIC_HEADERS, }, signal, - ); + ).catch(remapOutputLimitError); const firstText = response.content?.find((block) => block.type === 'text' && typeof block.text === 'string'); if (!firstText || typeof firstText.text !== 'string') { throw new Error(`anthropic: response missing text content (stop_reason=${response.stop_reason ?? 'unknown'})`); diff --git a/src/llm/index.ts b/src/llm/index.ts index bbe7bcf..9b3b69c 100644 --- a/src/llm/index.ts +++ b/src/llm/index.ts @@ -1,8 +1,35 @@ import { LLMConfig, LLMProviderID } from '../types'; +import { ProviderError } from '../http'; import { createAnthropicLLM } from './anthropic'; import { createOpenAILLM } from './openai'; import { createGeminiLLM } from './gemini'; +// Anthropic and OpenAI reject (HTTP 400) a request whose output cap exceeds the +// model's max output tokens, with a body naming `max_tokens` / +// `max_completion_tokens` and a "maximum / too large / at most" phrase. The raw +// message is cryptic, so detect that specific case and rethrow with an actionable +// pointer to the "Maximum note length" setting. Everything else passes through +// unchanged. Used as a `.catch()` handler, so its `never` return keeps the awaited +// value's type intact. (Gemini silently clamps maxOutputTokens instead of erroring, +// so it needs no remap.) +export function remapOutputLimitError(e: unknown): never { + if ( + e instanceof ProviderError && + e.status === 400 && + /max_tokens|max_completion_tokens/i.test(e.body) && + /maximum|at most|too large|exceed|output token/i.test(e.body) + ) { + throw new ProviderError( + e.provider, + e.status, + e.body, + `${e.provider}: the requested note length exceeds this model's output limit. ` + + 'Lower "Maximum note length" in settings, or choose a model with a higher output cap.', + ); + } + throw e; +} + export interface LLMProvider { readonly id: LLMProviderID; complete( diff --git a/src/llm/openai.ts b/src/llm/openai.ts index 7461f22..0a1849b 100644 --- a/src/llm/openai.ts +++ b/src/llm/openai.ts @@ -1,6 +1,6 @@ import { LLMConfig, LLMProviderID } from '../types'; import { jsonGet, jsonPost } from '../http'; -import { LLMProvider } from './index'; +import { LLMProvider, remapOutputLimitError } from './index'; interface ChatCompletionResponse { choices?: Array<{ @@ -12,6 +12,15 @@ interface ModelsListResponse { data?: Array<{ id?: unknown }>; } +// OpenAI's reasoning models (o1/o3/o4 families, gpt-5 family) reject the legacy +// `max_tokens` param and require `max_completion_tokens`. Scoped to id === 'openai' +// because only the first-party endpoint enforces this; openai-compatible servers +// and Mistral keep `max_tokens`. A proxied reasoning model behind openai-compatible +// is the known gap (documented in CLAUDE.md). +function usesCompletionTokens(id: LLMProviderID, model: string): boolean { + return id === 'openai' && /^(o\d|gpt-5)/i.test(model.trim()); +} + export function createOpenAILLM(id: LLMProviderID): LLMProvider { const provider: LLMProvider = { id, @@ -31,14 +40,20 @@ export function createOpenAILLM(id: LLMProviderID): LLMProvider { { role: 'user', content: userMessage }, ], }; - if (config.maxTokens > 0) body.max_tokens = config.maxTokens; + if (config.maxTokens > 0) { + if (usesCompletionTokens(id, config.model)) { + body.max_completion_tokens = config.maxTokens; + } else { + body.max_tokens = config.maxTokens; + } + } const response = await jsonPost( id, url, body, { Authorization: `Bearer ${config.apiKey}` }, signal, - ); + ).catch(remapOutputLimitError); const content = response.choices?.[0]?.message?.content; if (typeof content !== 'string') { throw new Error(`${id}: response missing message content`);