never store a model the picker cannot render
The selected model went blank after a turn: visible only by opening the dropdown.
`officerModel` was coming back as `claude-code` — the bare harness. That is `DEFAULT_MODEL`, used when the
client sends no model and the user has no default. It ROUTES fine (isClaudeModel is a prefix test, and
split('/')[1] is undefined so the CLI picks its own), but it names no catalogue entry, so Radix's
`SelectValue` matched no item and rendered nothing.
It fed itself. A blank picker sends no model, so the next turn fell back to the bare harness again and
overwrote the real value — it destroyed a correctly backfilled `claude-code/opus` within one turn. My own
write path did that, added this afternoon.
Two changes. The server refuses to record a model with no tier: a bare harness means "no explicit choice",
and the honest response is to leave the previous answer alone rather than overwrite it with a non-answer.
And the trigger no longer trusts that its value names an item — an unmatchable value renders as its own
string instead of as an empty control, which also covers a retired model or a vanished OpenCode provider.
Repaired the one corrupted row in place by deleting it, so the transcript family fallback answers for that
session until its next real turn; it now reports claude-code/opus again.
This commit is contained in:
@@ -362,6 +362,22 @@ function createMessageHandler(sessionId: string, model: string, resumeSessionId?
|
|||||||
*/
|
*/
|
||||||
const record = (): void => {
|
const record = (): void => {
|
||||||
if (!transcriptId) return;
|
if (!transcriptId) return;
|
||||||
|
/*
|
||||||
|
Only ever record a model that NAMES A TIER.
|
||||||
|
|
||||||
|
`DEFAULT_MODEL` is the bare harness, `claude-code`, used when the client sends no model and the user has
|
||||||
|
no default. It routes correctly — `isClaudeModel` is a prefix test, and `split('/')[1]` yields undefined
|
||||||
|
so the CLI picks its own model — but it is not an id the catalogue offers, so storing it puts a value in
|
||||||
|
the picker that matches no option and renders as BLANK.
|
||||||
|
|
||||||
|
That failure feeds itself: a blank picker sends no model, so the next turn falls back to the bare
|
||||||
|
harness again and overwrites whatever real value was there. It destroyed a correctly backfilled
|
||||||
|
`claude-code/opus` within one turn.
|
||||||
|
|
||||||
|
A bare harness means "no explicit choice", and the honest response to that is to leave the previous
|
||||||
|
answer alone rather than to overwrite it with a non-answer.
|
||||||
|
*/
|
||||||
|
if (!effective.includes('/')) return;
|
||||||
void setChatSessionModel(transcriptId, effective).catch((err: unknown) => {
|
void setChatSessionModel(transcriptId, effective).catch((err: unknown) => {
|
||||||
logger.warn('Could not record the session model', {
|
logger.warn('Could not record the session model', {
|
||||||
sessionId,
|
sessionId,
|
||||||
|
|||||||
@@ -98,3 +98,22 @@ describe('ModelSelector default selection', () => {
|
|||||||
expect(picked).toEqual([]);
|
expect(picked).toEqual([]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('ModelSelector display of an unknown model', () => {
|
||||||
|
test('a value naming no catalogue entry still shows its name', () => {
|
||||||
|
/*
|
||||||
|
The blank-trigger bug. The server stored `claude-code` — the bare harness fallback used when a turn
|
||||||
|
carries no model — which routes correctly but names no catalogue entry, so `SelectValue` rendered
|
||||||
|
nothing and the trigger went empty. Self-reinforcing, too: a blank picker sends no model, so the next
|
||||||
|
turn fell back to the bare harness again and overwrote the real value.
|
||||||
|
*/
|
||||||
|
mount({ selectedModel: 'claude-code', hasStarted: true });
|
||||||
|
expect(screen.getByTitle('claude-code').textContent).toBe('claude-code');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a known model still renders through the Select', () => {
|
||||||
|
mount({ selectedModel: 'claude-code/opus', hasStarted: true });
|
||||||
|
// No raw-string fallback for a value the catalogue knows — that path is only for unmatchable ones.
|
||||||
|
expect(screen.queryByTitle('claude-code/opus')).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -212,7 +212,21 @@ export function ModelSelector({
|
|||||||
disabled={isBusy}
|
disabled={isBusy}
|
||||||
>
|
>
|
||||||
<SelectTrigger className="h-auto cursor-pointer gap-1 border-0 bg-transparent p-0 text-xs text-muted-foreground shadow-none focus:ring-0">
|
<SelectTrigger className="h-auto cursor-pointer gap-1 border-0 bg-transparent p-0 text-xs text-muted-foreground shadow-none focus:ring-0">
|
||||||
|
{/*
|
||||||
|
`SelectValue` renders the matching ITEM's text, so a value naming no item renders NOTHING —
|
||||||
|
the trigger goes blank and the only way to find out what is selected is to open the dropdown.
|
||||||
|
|
||||||
|
That is reachable whenever the two id spaces disagree: a stored `claude-code` (the bare
|
||||||
|
harness fallback, which routes fine but is not a catalogue entry), a model retired from the
|
||||||
|
list, or an OpenCode model that has since disappeared from its provider. The value is still
|
||||||
|
meaningful to the server in every one of those cases, so the honest display is the raw
|
||||||
|
string rather than an empty control.
|
||||||
|
*/}
|
||||||
|
{displayModel && !availableModels.some((m) => m.id === displayModel) ? (
|
||||||
|
<span title={displayModel}>{displayModel.split('/').pop()}</span>
|
||||||
|
) : (
|
||||||
<SelectValue />
|
<SelectValue />
|
||||||
|
)}
|
||||||
</SelectTrigger>
|
</SelectTrigger>
|
||||||
<SelectContent className="z-[800]" side="top">
|
<SelectContent className="z-[800]" side="top">
|
||||||
{providerModels.map((m) => (
|
{providerModels.map((m) => (
|
||||||
|
|||||||
Reference in New Issue
Block a user