diff --git a/src/__tests__/model_selection.spec.ts b/src/__tests__/model_selection.spec.ts new file mode 100644 index 0000000..5afcd54 --- /dev/null +++ b/src/__tests__/model_selection.spec.ts @@ -0,0 +1,99 @@ +/** + * Tests for the localStorage-backed model selection persistence helper. + * jsdom provides a real (per-test-class) localStorage. + */ +import { + clearModelSelection, + loadModelSelection, + saveModelSelection +} from '../components/model_selection'; + +describe('loadModelSelection', () => { + beforeEach(() => { + localStorage.clear(); + }); + + it('returns null when no entry exists', () => { + expect(loadModelSelection('foo.ipynb')).toBeNull(); + }); + + it('returns null for an empty notebookPath (defensive)', () => { + expect(loadModelSelection('')).toBeNull(); + }); + + it('returns the parsed entry when storage is valid', () => { + localStorage.setItem( + 'opencode_bridge:model-selection:foo.ipynb', + JSON.stringify({ providerId: 'anthropic', modelId: 'claude-sonnet-4-20250514' }) + ); + expect(loadModelSelection('foo.ipynb')).toEqual({ + providerId: 'anthropic', + modelId: 'claude-sonnet-4-20250514' + }); + }); + + it('returns null for malformed JSON (does not throw)', () => { + localStorage.setItem( + 'opencode_bridge:model-selection:foo.ipynb', + '{not json' + ); + expect(loadModelSelection('foo.ipynb')).toBeNull(); + }); + + it('returns null when the entry is missing required fields', () => { + localStorage.setItem( + 'opencode_bridge:model-selection:foo.ipynb', + JSON.stringify({ providerId: 'anthropic' }) + ); + expect(loadModelSelection('foo.ipynb')).toBeNull(); + }); + + it('keys entries independently per notebookPath', () => { + saveModelSelection('a.ipynb', { providerId: 'p1', modelId: 'm1' }); + saveModelSelection('b.ipynb', { providerId: 'p2', modelId: 'm2' }); + expect(loadModelSelection('a.ipynb')).toEqual({ + providerId: 'p1', + modelId: 'm1' + }); + expect(loadModelSelection('b.ipynb')).toEqual({ + providerId: 'p2', + modelId: 'm2' + }); + }); +}); + +describe('saveModelSelection + clearModelSelection', () => { + beforeEach(() => { + localStorage.clear(); + }); + + it('writes a JSON entry that can be re-read', () => { + saveModelSelection('foo.ipynb', { providerId: 'p1', modelId: 'm1' }); + expect(loadModelSelection('foo.ipynb')).toEqual({ + providerId: 'p1', + modelId: 'm1' + }); + }); + + it('overwrites an existing entry', () => { + saveModelSelection('foo.ipynb', { providerId: 'p1', modelId: 'm1' }); + saveModelSelection('foo.ipynb', { providerId: 'p2', modelId: 'm2' }); + expect(loadModelSelection('foo.ipynb')).toEqual({ + providerId: 'p2', + modelId: 'm2' + }); + }); + + it('clearModelSelection removes the entry', () => { + saveModelSelection('foo.ipynb', { providerId: 'p1', modelId: 'm1' }); + clearModelSelection('foo.ipynb'); + expect(loadModelSelection('foo.ipynb')).toBeNull(); + }); + + it('no-op for an empty notebookPath (defensive)', () => { + expect(() => + saveModelSelection('', { providerId: 'p1', modelId: 'm1' }) + ).not.toThrow(); + expect(() => clearModelSelection('')).not.toThrow(); + }); +}); diff --git a/src/__tests__/opencode_cell_actions.spec.ts b/src/__tests__/opencode_cell_actions.spec.ts index 64c4413..6c139e9 100644 --- a/src/__tests__/opencode_cell_actions.spec.ts +++ b/src/__tests__/opencode_cell_actions.spec.ts @@ -103,6 +103,12 @@ beforeAll(() => { }); }); +beforeEach(() => { + // Each test starts with a clean selection store so persistence tests + // are deterministic. + localStorage.clear(); +}); + // Mock the network-touching API module so jsdom tests don't try to hit // a real notebook server (and to keep the refresh-history logic decoupled // from the network). The cell_actions tests focus on widget behavior. @@ -323,6 +329,122 @@ describe('OpenCodeCellActions', () => { setOpenCodeProviders(null); }); + it('restores the user\'s previously-saved (provider, model) when still valid in the providers payload', () => { + // Seed localStorage with a prior selection that IS valid. + localStorage.setItem( + 'opencode_bridge:model-selection:foo.ipynb', + JSON.stringify({ providerId: 'openai', modelId: 'gpt-5' }) + ); + setOpenCodeProviders({ + providers: [ + { id: 'anthropic', models: { 'claude-sonnet-4-20250514': { id: 'x' } } }, + { id: 'openai', models: { 'gpt-5': { id: 'x' }, 'gpt-4': { id: 'x' } } } + ], + default: { anthropic: 'claude-sonnet-4-20250514' } + }); + const cell = makeFakeCell('x = 1', [], 'foo.ipynb'); + const actions = new OpenCodeCellActions(cell); + Object.defineProperty(actions, 'parent', { value: cell, configurable: true }); + (actions as any).onAfterAttach({} as any); + + (actions.node.querySelector('button') as HTMLButtonElement).click(); + const prompt = (actions as any)._prompt; + const providerSel = prompt.node.querySelector( + '.opencode-provider-select' + ) as HTMLSelectElement; + const modelSel = prompt.node.querySelector( + '.opencode-model-select' + ) as HTMLSelectElement; + // Stored selection was applied, NOT the default (first provider). + expect(providerSel.value).toBe('openai'); + expect(modelSel.value).toBe('gpt-5'); + setOpenCodeProviders(null); + }); + + it('falls back to default when the stored provider is no longer present', () => { + localStorage.setItem( + 'opencode_bridge:model-selection:foo.ipynb', + JSON.stringify({ providerId: 'removed-provider', modelId: 'm1' }) + ); + setOpenCodeProviders({ + providers: [ + { id: 'anthropic', models: { 'claude-sonnet-4-20250514': { id: 'x' } } } + ] + }); + const cell = makeFakeCell('x = 1', [], 'foo.ipynb'); + const actions = new OpenCodeCellActions(cell); + Object.defineProperty(actions, 'parent', { value: cell, configurable: true }); + (actions as any).onAfterAttach({} as any); + + (actions.node.querySelector('button') as HTMLButtonElement).click(); + const providerSel = (actions as any)._prompt.node.querySelector( + '.opencode-provider-select' + ) as HTMLSelectElement; + // Provider is gone → fall back to first available. + expect(providerSel.value).toBe('anthropic'); + setOpenCodeProviders(null); + }); + + it('falls back to default model when the stored model is no longer present under the same provider', () => { + localStorage.setItem( + 'opencode_bridge:model-selection:foo.ipynb', + JSON.stringify({ providerId: 'openai', modelId: 'gpt-99-removed' }) + ); + setOpenCodeProviders({ + providers: [ + { id: 'openai', models: { 'gpt-5': { id: 'x' }, 'gpt-4': { id: 'x' } } } + ], + default: { openai: 'gpt-5' } + }); + const cell = makeFakeCell('x = 1', [], 'foo.ipynb'); + const actions = new OpenCodeCellActions(cell); + Object.defineProperty(actions, 'parent', { value: cell, configurable: true }); + (actions as any).onAfterAttach({} as any); + + (actions.node.querySelector('button') as HTMLButtonElement).click(); + const modelSel = (actions as any)._prompt.node.querySelector( + '.opencode-model-select' + ) as HTMLSelectElement; + // Model is gone → fall back to default[openai] = gpt-5. + expect(modelSel.value).toBe('gpt-5'); + setOpenCodeProviders(null); + }); + + it('saves the new selection to localStorage when the model select changes', () => { + setOpenCodeProviders({ + providers: [ + { id: 'anthropic', models: { 'claude-sonnet-4-20250514': { id: 'x' } } }, + { id: 'openai', models: { 'gpt-5': { id: 'x' } } } + ] + }); + const cell = makeFakeCell('x = 1', [], 'foo.ipynb'); + const actions = new OpenCodeCellActions(cell); + Object.defineProperty(actions, 'parent', { value: cell, configurable: true }); + (actions as any).onAfterAttach({} as any); + + (actions.node.querySelector('button') as HTMLButtonElement).click(); + const prompt = (actions as any)._prompt; + const providerSel = prompt.node.querySelector( + '.opencode-provider-select' + ) as HTMLSelectElement; + const modelSel = prompt.node.querySelector( + '.opencode-model-select' + ) as HTMLSelectElement; + + // Switch provider to openai (whose model list includes gpt-5). + providerSel.value = 'openai'; + providerSel.dispatchEvent(new Event('change')); + // Now change the model select to gpt-5. + modelSel.value = 'gpt-5'; + modelSel.dispatchEvent(new Event('change')); + + const stored = JSON.parse( + localStorage.getItem('opencode_bridge:model-selection:foo.ipynb') || 'null' + ); + expect(stored).toEqual({ providerId: 'openai', modelId: 'gpt-5' }); + setOpenCodeProviders(null); + }); + it('falls back to the first model when default[provider] is not in models', () => { setOpenCodeProviders({ providers: [ diff --git a/src/components/model_selection.ts b/src/components/model_selection.ts new file mode 100644 index 0000000..82f0912 --- /dev/null +++ b/src/components/model_selection.ts @@ -0,0 +1,82 @@ +/** + * Persistence of the user's (provider, model) selection in the inline + * prompt, keyed by notebook path. Stored in localStorage so the + * preference survives panel close/reopen and JupyterLab restart. + * + * The selection is ALWAYS validated against the current providers + * payload before being applied: if the stored provider (or the model + * under it) is no longer present, the caller falls back to its + * default-selection logic. We never silently apply a stale value that + * would result in an empty / disabled