mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(native-chat): keep the Claude model picker rendered
The Claude picker rendered nothing until the per-host `list_models` probe returned, so it popped in ~1s after mount and never appeared at all when the probe failed — an old CLI without `list_models`, no `claude` on PATH, or an older remote runtime whose response omits `catalogOrigin`. Restore the version-neutral family seed as the starting list; discovery still replaces it wholesale on success, so a host with a real catalog never shows an obsolete hardcoded row. Separately, the tracked model could fall outside the active list: the terminal header scrape yields family ids (`opus`) while a current CLI lists `opus[1m]` and no plain `opus`. That blanked the picker trigger and dropped the model's effort and fast-mode controls. Reconcile the tracked id into the active list once, so the snapshot, the appliers, and typed command recording all see a labelled, operable row for it.
This commit is contained in:
@@ -8,6 +8,7 @@ import type {
|
||||
} from '../../../../shared/native-chat-session-options'
|
||||
import type * as nativeChatAgentProfiles from '../../../../shared/native-chat-agent-profiles'
|
||||
import { clearNativeChatSessionOptionCacheForTests } from './native-chat-session-option-cache'
|
||||
import { clearNativeChatModelEnrichmentForTests } from './native-chat-session-option-enrichment'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
cancelPendingSends: vi.fn(),
|
||||
@@ -138,6 +139,7 @@ describe('NativeChatComposer', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
clearNativeChatSessionOptionCacheForTests()
|
||||
clearNativeChatModelEnrichmentForTests()
|
||||
mocks.fieldProps = null
|
||||
mocks.modelSwitchOutcome = 'applied'
|
||||
mocks.draftScopeKeys.length = 0
|
||||
@@ -318,6 +320,59 @@ describe('NativeChatComposer', () => {
|
||||
expect(mocks.setDraft).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('renders the Claude model picker while host discovery is still pending', () => {
|
||||
mocks.discoverCommitMessageModels.mockReturnValue(new Promise(() => {}))
|
||||
render(
|
||||
<NativeChatComposer
|
||||
terminalTabId="tab-1"
|
||||
paneKey="tab-1:leaf-1"
|
||||
targetPtyId="pty-1"
|
||||
agent="claude"
|
||||
readTerminalScreen={() => null}
|
||||
/>
|
||||
)
|
||||
|
||||
expect(mocks.fieldProps?.sessionOptionsSnapshot?.[0]).toMatchObject({
|
||||
id: 'model',
|
||||
kind: {
|
||||
choices: expect.arrayContaining([
|
||||
expect.objectContaining({ value: 'opus', label: 'Opus' }),
|
||||
expect.objectContaining({ value: 'sonnet', label: 'Sonnet' })
|
||||
])
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps the Claude model picker when an older remote runtime omits the catalog origin', async () => {
|
||||
mocks.discoverCommitMessageModels.mockResolvedValue({
|
||||
success: true,
|
||||
defaultModelId: 'sonnet',
|
||||
models: [{ id: 'sonnet', label: 'Sonnet' }]
|
||||
})
|
||||
render(
|
||||
<NativeChatComposer
|
||||
terminalTabId="tab-1"
|
||||
paneKey="tab-1:leaf-1"
|
||||
targetPtyId="pty-1"
|
||||
agent="claude"
|
||||
readTerminalScreen={() => null}
|
||||
/>
|
||||
)
|
||||
|
||||
await waitFor(() => expect(mocks.discoverCommitMessageModels).toHaveBeenCalled())
|
||||
await act(async () => undefined)
|
||||
|
||||
expect(mocks.fieldProps?.sessionOptionsSnapshot?.[0]).toMatchObject({
|
||||
id: 'model',
|
||||
kind: {
|
||||
choices: expect.arrayContaining([
|
||||
expect.objectContaining({ value: 'fable', label: 'Fable' }),
|
||||
expect.objectContaining({ value: 'haiku', label: 'Haiku' })
|
||||
])
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('shows the model already selected in the Claude TUI when chat opens', async () => {
|
||||
mocks.getMainBufferSnapshot.mockResolvedValue({
|
||||
data: 'Claude Code v2.1.211\r\nOpus 4.8 with medium effort · API Usage Billing',
|
||||
|
||||
@@ -9,7 +9,7 @@ import { createNativeChatPtySessionOptions } from './native-chat-pty-session-opt
|
||||
describe('native chat PTY session options', () => {
|
||||
beforeEach(() => clearNativeChatSessionOptionCacheForTests())
|
||||
|
||||
it('hides the model picker until the host catalog is known', () => {
|
||||
it('renders nothing when no model list exists at all', () => {
|
||||
const surface = createNativeChatPtySessionOptions({
|
||||
agent: 'claude',
|
||||
scopeKey: 'pty-1',
|
||||
@@ -20,6 +20,28 @@ describe('native chat PTY session options', () => {
|
||||
expect(surface.getSnapshot()).toEqual([])
|
||||
})
|
||||
|
||||
it('renders the version-neutral seed picker before any host catalog arrives', () => {
|
||||
const surface = createNativeChatPtySessionOptions({
|
||||
agent: 'claude',
|
||||
scopeKey: 'pty-1',
|
||||
mode: 'live',
|
||||
dispatchCommand: vi.fn()
|
||||
})!
|
||||
|
||||
expect(surface.getSnapshot()[0]).toMatchObject({
|
||||
id: 'model',
|
||||
valueSource: 'unknown',
|
||||
kind: {
|
||||
choices: [
|
||||
expect.objectContaining({ value: 'fable', label: 'Fable' }),
|
||||
expect.objectContaining({ value: 'opus', label: 'Opus' }),
|
||||
expect.objectContaining({ value: 'sonnet', label: 'Sonnet' }),
|
||||
expect.objectContaining({ value: 'haiku', label: 'Haiku' })
|
||||
]
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('uses model and effort reported by the live Claude terminal', () => {
|
||||
const surface = createNativeChatPtySessionOptions({
|
||||
agent: 'claude',
|
||||
@@ -583,7 +605,7 @@ describe('native chat PTY session options', () => {
|
||||
expect(surface.getSnapshot()[0]).toMatchObject({ valueSource: 'unknown' })
|
||||
})
|
||||
|
||||
it('tracks an unavailable Claude model without advertising it as a choice', () => {
|
||||
it('passes an unknown persisted model through as a literal choice', () => {
|
||||
seedNativeChatAppliedSessionOptions('pty-1', 'claude', {
|
||||
model: 'future-model'
|
||||
})
|
||||
@@ -596,7 +618,59 @@ describe('native chat PTY session options', () => {
|
||||
const model = surface.getSnapshot()[0]
|
||||
expect(model.kind).toMatchObject({
|
||||
currentValue: 'future-model',
|
||||
choices: expect.not.arrayContaining([{ value: 'future-model', label: 'future-model' }])
|
||||
choices: expect.arrayContaining([{ value: 'future-model', label: 'future-model' }])
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps a tracked alias selectable when the host catalog omits it', async () => {
|
||||
seedNativeChatAppliedSessionOptions('pty-1', 'claude', {
|
||||
model: 'opus',
|
||||
effort: 'xhigh'
|
||||
})
|
||||
const dispatch = vi.fn()
|
||||
const surface = createNativeChatPtySessionOptions({
|
||||
agent: 'claude',
|
||||
scopeKey: 'pty-1',
|
||||
// Why: current CLIs list `opus[1m]` and no plain `opus`.
|
||||
initialModels: [
|
||||
{ id: 'opus[1m]', label: 'Opus (1M context)', options: [] },
|
||||
{ id: 'sonnet', label: 'Sonnet', options: [] }
|
||||
],
|
||||
mode: 'live',
|
||||
dispatchCommand: dispatch
|
||||
})!
|
||||
|
||||
expect(surface.getSnapshot()[0].kind).toMatchObject({
|
||||
currentValue: 'opus',
|
||||
choices: expect.arrayContaining([
|
||||
{ value: 'opus', label: 'Opus', description: expect.any(String) }
|
||||
])
|
||||
})
|
||||
expect(surface.getSnapshot().find(({ id }) => id === 'effort')).toMatchObject({
|
||||
settable: true,
|
||||
kind: { currentValue: 'xhigh' }
|
||||
})
|
||||
|
||||
await surface.setOption('effort', 'high')
|
||||
|
||||
expect(dispatch).toHaveBeenCalledWith('/effort high')
|
||||
})
|
||||
|
||||
it('drops the reconciled row once the tracked model moves onto the host catalog', async () => {
|
||||
seedNativeChatAppliedSessionOptions('pty-1', 'claude', { model: 'opus' })
|
||||
const surface = createNativeChatPtySessionOptions({
|
||||
agent: 'claude',
|
||||
scopeKey: 'pty-1',
|
||||
initialModels: [{ id: 'opus[1m]', label: 'Opus (1M context)', options: [] }],
|
||||
mode: 'live',
|
||||
dispatchCommand: vi.fn()
|
||||
})!
|
||||
|
||||
await surface.setOption('model', 'opus[1m]')
|
||||
|
||||
expect(surface.getSnapshot()[0].kind).toMatchObject({
|
||||
currentValue: 'opus[1m]',
|
||||
choices: [{ value: 'opus[1m]', label: 'Opus (1M context)' }]
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import {
|
||||
getAgentSessionOptionCatalog,
|
||||
type AgentSessionOptionCatalog,
|
||||
type CatalogModel
|
||||
} from '../../../../shared/agent-session-option-catalog'
|
||||
import type { AgentType } from '../../../../shared/agent-status-types'
|
||||
@@ -11,7 +12,8 @@ import type {
|
||||
import {
|
||||
createNativeChatSessionOptionRecord,
|
||||
readNativeChatSessionOptionCache,
|
||||
writeNativeChatSessionOptionCache
|
||||
writeNativeChatSessionOptionCache,
|
||||
type NativeChatSessionOptionRecord
|
||||
} from './native-chat-session-option-cache'
|
||||
import { createSessionOptionAppliers } from './native-chat-session-option-apply'
|
||||
import {
|
||||
@@ -47,6 +49,24 @@ export type CreateNativeChatPtySessionOptionsArgs = {
|
||||
onDraftValuesChanged?: (values: Record<string, SessionOptionValue>) => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Why: the tracked model can sit outside the active list — a persisted default,
|
||||
* or an alias this host's CLI no longer lists. Keeping a row for it preserves
|
||||
* the labelled selection and the model's own options instead of blanking both.
|
||||
*/
|
||||
function withTrackedModel(
|
||||
catalog: AgentSessionOptionCatalog,
|
||||
models: readonly CatalogModel[],
|
||||
record: NativeChatSessionOptionRecord
|
||||
): CatalogModel[] {
|
||||
const trackedId = typeof record.model?.value === 'string' ? record.model.value : null
|
||||
if (!trackedId || models.some((model) => model.id === trackedId)) {
|
||||
return [...models]
|
||||
}
|
||||
const seeded = catalog.models.find((model) => model.id === trackedId)
|
||||
return [...models, seeded ?? { id: trackedId, label: trackedId, options: [] }]
|
||||
}
|
||||
|
||||
export function createNativeChatPtySessionOptions(
|
||||
args: CreateNativeChatPtySessionOptionsArgs
|
||||
): NativeChatPtySessionOptionsSurface | null {
|
||||
@@ -65,12 +85,12 @@ export function createNativeChatPtySessionOptions(
|
||||
if (args.reportedValues && applyNativeChatReportedSessionOptions(record, args.reportedValues)) {
|
||||
writeNativeChatSessionOptionCache(args.scopeKey, record)
|
||||
}
|
||||
const activeModels = (): CatalogModel[] => withTrackedModel(catalog, models, record)
|
||||
let snapshot = buildNativeChatSessionOptionSnapshot({
|
||||
catalog,
|
||||
models,
|
||||
models: activeModels(),
|
||||
record,
|
||||
mode: args.mode,
|
||||
includeTrackedModelChoice: args.agent !== 'claude'
|
||||
mode: args.mode
|
||||
})
|
||||
const listeners = new Set<(value: SessionOptionDescriptor[]) => void>()
|
||||
|
||||
@@ -78,10 +98,9 @@ export function createNativeChatPtySessionOptions(
|
||||
writeNativeChatSessionOptionCache(args.scopeKey, record)
|
||||
snapshot = buildNativeChatSessionOptionSnapshot({
|
||||
catalog,
|
||||
models,
|
||||
models: activeModels(),
|
||||
record,
|
||||
mode: args.mode,
|
||||
includeTrackedModelChoice: args.agent !== 'claude'
|
||||
mode: args.mode
|
||||
})
|
||||
for (const listener of listeners) {
|
||||
listener(snapshot)
|
||||
@@ -126,7 +145,7 @@ export function createNativeChatPtySessionOptions(
|
||||
const appliers = createSessionOptionAppliers({
|
||||
mode: args.mode,
|
||||
catalog,
|
||||
getModels: () => models,
|
||||
getModels: activeModels,
|
||||
getRecord: () => record,
|
||||
dispatchCommand: args.dispatchCommand,
|
||||
onAgentPicker: args.onAgentPicker,
|
||||
@@ -148,7 +167,7 @@ export function createNativeChatPtySessionOptions(
|
||||
recordOutgoingCommand: (command) => {
|
||||
const result = recordNativeChatSessionOptionCommand({
|
||||
catalog,
|
||||
models,
|
||||
models: activeModels(),
|
||||
record,
|
||||
command,
|
||||
persist
|
||||
|
||||
@@ -118,21 +118,19 @@ export function buildNativeChatSessionOptionSnapshot(args: {
|
||||
models: readonly CatalogModel[]
|
||||
record: NativeChatSessionOptionRecord
|
||||
mode: NativeChatSessionOptionMode
|
||||
includeTrackedModelChoice: boolean
|
||||
}): SessionOptionDescriptor[] {
|
||||
const { catalog, models, record, mode } = args
|
||||
if (models.length === 0) {
|
||||
return []
|
||||
}
|
||||
const modelTracked = record.model
|
||||
const listedModelChoices = models.map(({ id, label, description }) => ({
|
||||
// Why: callers reconcile the tracked model into `models`, so every listed row
|
||||
// is a real choice and the trigger never shows a value without one.
|
||||
const modelChoices = models.map(({ id, label, description }) => ({
|
||||
value: id,
|
||||
label,
|
||||
...(description ? { description } : {})
|
||||
}))
|
||||
const modelChoices = args.includeTrackedModelChoice
|
||||
? choiceWithCurrent(listedModelChoices, modelTracked)
|
||||
: listedModelChoices
|
||||
const modelSettable = settableState({ mode, apply: catalog.modelApply })
|
||||
const modelAction = actionForApply(catalog.modelApply, modelTracked, mode)
|
||||
const snapshot: SessionOptionDescriptor[] = [
|
||||
|
||||
@@ -54,12 +54,15 @@ export function useNativeChatSessionOptions(args: {
|
||||
agent,
|
||||
scopeKey,
|
||||
...(targetPtyId ? { fallbackScopeKey: terminalTabId } : {}),
|
||||
initialModels: discoveryContext
|
||||
? (readNativeChatEnrichedModels(agent, discoveryContext.hostKey) ??
|
||||
(agent === 'claude' ? [] : undefined))
|
||||
: agent === 'claude'
|
||||
? []
|
||||
: undefined,
|
||||
// Why: the catalog seed carries version-neutral family labels, so it is
|
||||
// safe on every host while the once-per-host probe runs or after it fails
|
||||
// — without it the whole picker would pop in late or never appear.
|
||||
...(discoveryContext
|
||||
? {
|
||||
initialModels:
|
||||
readNativeChatEnrichedModels(agent, discoveryContext.hostKey) ?? undefined
|
||||
}
|
||||
: {}),
|
||||
mode: targetPtyId ? 'live' : 'draft',
|
||||
reportedValues,
|
||||
dispatchCommand,
|
||||
|
||||
Reference in New Issue
Block a user