Skip to content

Commit 737c04a

Browse files
olivermontesclaude
andcommitted
fix(models): preserve model selections when switching providers
- Add session-based sync tracking (syncedProvidersInSession Set) - Only apply minimal save logic to providers synced in current session - Preserve selectedModelIds when sync returns empty results - Protect toggle/selection methods from clearing existing selections - Skip background sync for providers without API keys configured Fixes issue where selected models from inactive providers were lost when switching between providers or during background syncs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 48ee09e commit 737c04a

File tree

1 file changed

+84
-35
lines changed

1 file changed

+84
-35
lines changed

src/renderer/services/modelService.ts

Lines changed: 84 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ class ModelServiceImpl {
2323
// Classification cache for O(1) lookups
2424
private classificationCache = new Map<string, ModelClassification>();
2525

26+
// Track which providers have been synced in this session
27+
// This prevents losing models from inactive providers when saving
28+
private syncedProvidersInSession = new Set<string>();
29+
2630
// Initialize with default providers and load from storage
2731
async initialize(): Promise<void> {
2832
// Prevent double initialization from React StrictMode
@@ -221,9 +225,14 @@ class ModelServiceImpl {
221225

222226
// 1. Refresh dynamic providers if needed (smart sync)
223227
// We don't want to block UI, so we trust cache but trigger background sync if very old
228+
// Only sync providers that have API keys configured to avoid empty results
224229
const now = Date.now();
225230
this.providers.forEach(provider => {
231+
// Skip providers without API key (except openrouter which works without key)
232+
const hasCredentials = provider.apiKey || provider.type === 'openrouter';
233+
226234
if (provider.modelSource === 'dynamic' &&
235+
hasCredentials &&
227236
(!provider.lastModelSync || (now - provider.lastModelSync > 1000 * 60 * 5))) {
228237
// Trigger sync but don't await it to keep UI snappy
229238
// If it's critical, the user can refresh or we can await
@@ -298,8 +307,12 @@ class ModelServiceImpl {
298307
model.isSelected = selected;
299308

300309
// Update selectedModelIds for dynamic providers
310+
// Use Set operations to preserve existing IDs not in current models list
301311
if (provider.modelSource === 'dynamic') {
302-
provider.selectedModelIds = provider.models.filter(m => m.isSelected).map(m => m.id);
312+
const currentModelIds = new Set(provider.models.map(m => m.id));
313+
const existingSelectedIds = (provider.selectedModelIds || []).filter(id => !currentModelIds.has(id));
314+
const newSelectedIds = provider.models.filter(m => m.isSelected).map(m => m.id);
315+
provider.selectedModelIds = [...existingSelectedIds, ...newSelectedIds];
303316
}
304317

305318
await this.saveProviders();
@@ -318,8 +331,12 @@ class ModelServiceImpl {
318331
});
319332

320333
// Update selectedModelIds for dynamic providers
334+
// Use Set operations to preserve existing IDs not in current models list
321335
if (provider.modelSource === 'dynamic') {
322-
provider.selectedModelIds = provider.models.filter(m => m.isSelected).map(m => m.id);
336+
const currentModelIds = new Set(provider.models.map(m => m.id));
337+
const existingSelectedIds = (provider.selectedModelIds || []).filter(id => !currentModelIds.has(id));
338+
const newSelectedIds = provider.models.filter(m => m.isSelected).map(m => m.id);
339+
provider.selectedModelIds = [...existingSelectedIds, ...newSelectedIds];
323340
}
324341

325342
await this.saveProviders();
@@ -358,8 +375,12 @@ class ModelServiceImpl {
358375
provider.models.push(newModel);
359376

360377
// Update selectedModelIds for dynamic providers
378+
// Use Set operations to preserve existing IDs not in current models list
361379
if (provider.modelSource === 'dynamic') {
362-
provider.selectedModelIds = provider.models.filter(m => m.isSelected).map(m => m.id);
380+
const currentModelIds = new Set(provider.models.map(m => m.id));
381+
const existingSelectedIds = (provider.selectedModelIds || []).filter(id => !currentModelIds.has(id));
382+
const newSelectedIds = provider.models.filter(m => m.isSelected).map(m => m.id);
383+
provider.selectedModelIds = [...existingSelectedIds, ...newSelectedIds];
363384
}
364385

365386
await this.saveProviders();
@@ -373,8 +394,9 @@ class ModelServiceImpl {
373394
provider.models = provider.models.filter(m => m.id !== modelId || !m.userDefined);
374395

375396
// Update selectedModelIds for dynamic providers
397+
// Also remove the model from selectedModelIds if it was there
376398
if (provider.modelSource === 'dynamic') {
377-
provider.selectedModelIds = provider.models.filter(m => m.isSelected).map(m => m.id);
399+
provider.selectedModelIds = (provider.selectedModelIds || []).filter(id => id !== modelId);
378400
}
379401

380402
await this.saveProviders();
@@ -535,7 +557,26 @@ class ModelServiceImpl {
535557

536558
// Update provider models: combine synced models with user-defined models
537559
provider.models = [...models, ...userDefinedModels];
538-
provider.selectedModelIds = provider.models.filter(m => m.isSelected).map(m => m.id);
560+
561+
// Only update selectedModelIds if we actually got new models from sync
562+
// This prevents losing saved selections when sync fails or returns empty
563+
if (models.length > 0) {
564+
provider.selectedModelIds = provider.models.filter(m => m.isSelected).map(m => m.id);
565+
// Mark provider as synced in this session (only if we got actual models)
566+
this.syncedProvidersInSession.add(provider.id);
567+
}
568+
// If no new models but we have user-defined models, update to include their IDs
569+
else if (userDefinedModels.length > 0) {
570+
const userDefinedSelectedIds = userDefinedModels.filter(m => m.isSelected).map(m => m.id);
571+
// Preserve existing selectedModelIds and add user-defined
572+
provider.selectedModelIds = [
573+
...(provider.selectedModelIds || []).filter(id => !userDefinedModels.some(m => m.id === id)),
574+
...userDefinedSelectedIds
575+
];
576+
}
577+
// If sync returned empty and no user-defined, preserve existing selectedModelIds
578+
// (don't clear them - they'll be restored when sync succeeds later)
579+
539580
provider.lastModelSync = Date.now();
540581
await this.saveProviders();
541582

@@ -558,38 +599,46 @@ class ModelServiceImpl {
558599
private async saveProviders(): Promise<void> {
559600
try {
560601
// For dynamic providers, save selected model IDs + minimal model data with classification
602+
// Only apply minimal save logic to providers that have been synced in this session
561603
const providersToSave = this.providers.map(provider => {
562604
if (provider.modelSource === 'dynamic') {
563-
// Extract selected model IDs
564-
const selectedModelIds = provider.models
565-
.filter(m => m.isSelected === true)
566-
.map(m => m.id);
567-
568-
// Save minimal model data for selected models (id + classification only)
569-
// This allows main process to access classification without full sync
570-
const selectedModelsMinimal = provider.models
571-
.filter(m => m.isSelected === true && !m.userDefined)
572-
.map(m => ({
573-
id: m.id,
574-
name: m.name,
575-
provider: m.provider,
576-
category: m.category,
577-
computedCapabilities: m.computedCapabilities,
578-
taskType: m.taskType,
579-
userDefined: false,
580-
isAvailable: true,
581-
contextLength: 0,
582-
capabilities: []
583-
}));
584-
585-
// Also include user-defined models (full data)
586-
const userDefinedModels = provider.models.filter(m => m.userDefined);
587-
588-
return {
589-
...provider,
590-
selectedModelIds,
591-
models: [...selectedModelsMinimal, ...userDefinedModels],
592-
};
605+
// Only apply minimal save to providers synced in this session
606+
// This prevents losing models from inactive providers that haven't been synced
607+
if (this.syncedProvidersInSession.has(provider.id)) {
608+
// Extract selected model IDs
609+
const selectedModelIds = provider.models
610+
.filter(m => m.isSelected === true)
611+
.map(m => m.id);
612+
613+
// Save minimal model data for selected models (id + classification only)
614+
// This allows main process to access classification without full sync
615+
const selectedModelsMinimal = provider.models
616+
.filter(m => m.isSelected === true && !m.userDefined)
617+
.map(m => ({
618+
id: m.id,
619+
name: m.name,
620+
provider: m.provider,
621+
category: m.category,
622+
computedCapabilities: m.computedCapabilities,
623+
taskType: m.taskType,
624+
userDefined: false,
625+
isAvailable: true,
626+
contextLength: 0,
627+
capabilities: []
628+
}));
629+
630+
// Also include user-defined models (full data)
631+
const userDefinedModels = provider.models.filter(m => m.userDefined);
632+
633+
return {
634+
...provider,
635+
selectedModelIds,
636+
models: [...selectedModelsMinimal, ...userDefinedModels],
637+
};
638+
}
639+
// For providers not synced in this session, preserve current state from storage
640+
// This prevents losing models from inactive providers
641+
return provider;
593642
}
594643
// For user-defined providers (cloud), save full model data
595644
return provider;

0 commit comments

Comments
 (0)