From 4adf96efa413b59b8c8e038f076a9556205c83fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 25 Aug 2026 18:33:38 +0000 Subject: [PATCH] the picker stopped overruling the default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit a new chat offered fable. not displayed it — SELECTED it, sent it, and the server honoured the choice it was given. the composer's fallback was providerModels[0], the first entry in the catalogue, which is not a default but an ordering accident. the file already carried a comment about the day fable was added above opus and every resumed session came back claiming fable; the same accident then quietly defeated the whole server-side default. so the fallback prefers the platform's default and uses catalogue order only when that model is not in the provider's list at all — which is every opencode model, where there is no officer-wide default to prefer. and a test, because a comment cannot hold a constant mirrored across two packages. it asserts the client's copy equals the server's, and that the value names a tier rather than the bare harness. both of the ways this default has been defeated are now failures a test catches: a drifted mirror, and a stored harness read as a choice. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- src/servers/api/chat/default-model.test.ts | 54 +++++++++++++++++++ .../apps/Chat/components/ModelSelector.tsx | 14 ++++- .../officerdev/src/apps/Chat/types.ts | 15 ++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 src/servers/api/chat/default-model.test.ts diff --git a/src/servers/api/chat/default-model.test.ts b/src/servers/api/chat/default-model.test.ts new file mode 100644 index 00000000..39d7d315 --- /dev/null +++ b/src/servers/api/chat/default-model.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from 'bun:test'; +import { CHAT_DEFAULT_MODEL, DEFAULT_MODEL, isTierlessModel } from './websocket'; +import { CHAT_DEFAULT_MODEL as CLIENT_CHAT_DEFAULT_MODEL } from 'officerdev'; + +/** + * The default model, and the two ways it has already been defeated. + * + * Both failures were silent and both cost real money: every unconfigured surface ran Opus while the code + * said Sonnet. Neither was visible in a diff — one was a constant in another package, the other was a + * value stored in somebody's settings. + */ + +describe('the client and server agree on the default', () => { + /* + A comment cannot hold this. `apps/Chat/types.ts` mirrors the server's constant because the picker has + to SHOW what the server would choose — and if it shows something else it also sends it, at which point + the server honours the client and the "default" never applies. That is not hypothetical: the client + fell back to the catalogue's first entry, which was `fable`, and did exactly that. + */ + test('the mirrored constant matches', () => { + expect(CLIENT_CHAT_DEFAULT_MODEL).toBe(CHAT_DEFAULT_MODEL); + }); + + test('it names a tier rather than the bare harness', () => { + expect(CHAT_DEFAULT_MODEL).toContain('/'); + expect(isTierlessModel(CHAT_DEFAULT_MODEL)).toBe(false); + }); +}); + +describe('isTierlessModel', () => { + /* + `claude-code` is a harness, and it is a legitimate value — `decideResume` assumes it when neither side + knows which sidecar a session belongs to. As an answer to "which model", it means "whatever the CLI + picks", which is Opus. Accounts have it stored from before that distinction existed, which is why the + default appeared to do nothing on a machine where it was set. + */ + test('the bare harness is not a model choice', () => { + expect(isTierlessModel(DEFAULT_MODEL)).toBe(true); + expect(isTierlessModel('claude-code')).toBe(true); + }); + + test('nothing chosen is not a model choice either', () => { + expect(isTierlessModel(null)).toBe(true); + expect(isTierlessModel(undefined)).toBe(true); + expect(isTierlessModel('')).toBe(true); + }); + + test('a real tier is', () => { + expect(isTierlessModel('claude-code/sonnet')).toBe(false); + expect(isTierlessModel('claude-code/opus[1m]')).toBe(false); + // OpenCode ids carry a provider prefix of their own, so they are tiered by the same test. + expect(isTierlessModel('opencode/some-model')).toBe(false); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/components/ModelSelector.tsx b/src/workspaces/officerdev/src/apps/Chat/components/ModelSelector.tsx index e14af62f..0ea4f19c 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/ModelSelector.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/ModelSelector.tsx @@ -3,6 +3,7 @@ import { CirclePlay, ClipboardList, FolderOpen } from 'lucide-react'; import { toneText } from '@/components/Data'; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; import type { ModelOption } from 'state/useModels'; +import { CHAT_DEFAULT_MODEL } from '../types'; import { getProviderDisplayName } from 'state/useModels'; import type { ChatMessage } from '../types'; import { UsagePopover } from './UsagePopover'; @@ -88,7 +89,18 @@ export function ModelSelector({ const activeProvider = availableModels.find((m) => m.id === displayModel)?.provider ?? providers[0]; const providerModels = availableModels.filter((m) => m.provider === activeProvider); - const fallbackModelId = providerModels[0]?.id ?? null; + /* + The platform's default first, and the catalogue's order only as a last resort. + + `providerModels[0]` is not a default — it is whichever entry happens to be listed first, and the + comment below records what that cost once already. It cost it again on 2026-08-25: with the server + defaulting to Sonnet, this line selected `fable`, sent it, and the server honoured the choice it was + given. A default the client can silently overrule is not a default. + + Falls back to the first entry only when the platform default is not in this provider's list at all — + which is every OpenCode model, where there is no Officer-wide default to prefer. + */ + const fallbackModelId = providerModels.find((m) => m.id === CHAT_DEFAULT_MODEL)?.id ?? providerModels[0]?.id ?? null; /* Pick a default only for a chat that has not started. diff --git a/src/workspaces/officerdev/src/apps/Chat/types.ts b/src/workspaces/officerdev/src/apps/Chat/types.ts index e8269ef6..231fb297 100644 --- a/src/workspaces/officerdev/src/apps/Chat/types.ts +++ b/src/workspaces/officerdev/src/apps/Chat/types.ts @@ -1,3 +1,18 @@ +/** + * What an unstarted chat offers when nothing else has been chosen. + * + * **Mirrors `CHAT_DEFAULT_MODEL` in `src/servers/api/chat/websocket.ts` and must not drift from it.** Same + * arrangement as `COMMANDER_DEFAULT_MODEL` in `apps/CommandHQ/types.ts`, and for the same reason: the + * server decides, but the picker has to show the same answer or it silently sends a different one. + * + * The composer used to fall back to `providerModels[0]` — the first entry in the catalogue. That is not a + * default, it is an ordering accident, and the file already carried a comment about the day `fable` was + * added above `opus` and every resumed session came back claiming fable. The same accident then defeated + * the server-side default entirely: the client selected fable, SENT fable, and the server honoured what it + * was told. + */ +export const CHAT_DEFAULT_MODEL = 'claude-code/sonnet'; + export type Attachment = | { type: 'webpage'; url: string; title: string; content: string; attachmentId: string; loading?: boolean } | { type: 'image'; filename: string; dataUrl: string; attachmentId: string; loading?: boolean }