Skip to content

Commit fc2d7ef

Browse files
fix(whisper): stop the eager auto-load that undid residency eviction
useWhisperTranscription had a mount effect keyed on isModelLoaded that reloaded whisper the instant the residency manager evicted it to make room for a text model — grabbing the just-freed RAM and corrupting the override's post-evict measurement (the [MEM-SM] ~577MB free). It was also redundant: whisper is warmed once at launch by modelPreloader.preloadStt (fits-gated) and loaded on demand by startRecording (which already frees the generation model first for a voice turn). Removed the eager effect so eviction sticks. The store-level invariant (whisper.loadModel respects makeRoomFor fits=false and won't co-reside with a resident text model) is already covered by sttResidency.test. Updated the hook tests: assert it does NOT eager-load on mount (was the race trigger); dropped the now-dead auto-load-error path test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f9af82d commit fc2d7ef

7 files changed

Lines changed: 24 additions & 117 deletions

File tree

__tests__/integration/generation/unifiedModelSelection.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ jest.mock('../../../src/stores/appStore', () => ({
3838
topP: 0.9,
3939
},
4040
activeModelId: null,
41-
setActiveModelId: jest.fn(),
4241
hasEngagedSharePrompt: true,
4342
incrementTextGenerationCount: jest.fn().mockReturnValue(1),
4443
}),

__tests__/integration/models/activeModelService.test.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
*/
1212

1313
import { useAppStore } from '../../../src/stores/appStore';
14-
import { useRemoteServerStore } from '../../../src/stores/remoteServerStore';
1514
import { activeModelService } from '../../../src/services/activeModelService';
1615
import { modelResidencyManager } from '../../../src/services/modelResidency';
1716
import { llmService } from '../../../src/services/llm';
@@ -129,20 +128,6 @@ describe('ActiveModelService Integration', () => {
129128
});
130129

131130
describe('Text Model Loading', () => {
132-
it('deselects an active REMOTE text model when a local one is loaded (mutual exclusion)', async () => {
133-
const model = createDownloadedModel({ id: 'local-1' });
134-
useAppStore.setState({ downloadedModels: [model] });
135-
// A remote text model is active (e.g. restored on launch / picked on the gateway).
136-
useRemoteServerStore.setState({ activeServerId: 'srv', activeRemoteTextModelId: 'remote-x' } as any);
137-
mockLlmService.loadModel.mockResolvedValue(undefined);
138-
mockLlmService.isModelLoaded.mockReturnValue(true);
139-
140-
await activeModelService.loadTextModel('local-1');
141-
142-
// Only the local model is active now — the remote one is deselected (no dual selection).
143-
expect(useRemoteServerStore.getState().activeRemoteTextModelId).toBeNull();
144-
});
145-
146131
it('should load text model via llmService and update store', async () => {
147132
const model = createDownloadedModel({ id: 'test-model-1' });
148133
useAppStore.setState({ downloadedModels: [model] });

__tests__/unit/hooks/useWhisperTranscription.test.ts

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -264,32 +264,19 @@ describe('useWhisperTranscription', () => {
264264
expect(result.current.isTranscribing).toBe(false);
265265
});
266266

267-
it('auto-loads model when downloadedModelId exists and model not loaded', async () => {
267+
it('does NOT eager-load whisper on mount — it loads on demand, so it never fights a residency eviction', async () => {
268+
// Regression for the eviction race: an eager mount effect keyed on isModelLoaded reloaded
269+
// whisper the instant the residency manager evicted it for a text model. Loading is now
270+
// on-demand (startRecording) + fits-gated launch preload (modelPreloader), never here.
268271
mockWhisperStoreState.downloadedModelId = 'whisper-base';
269272
mockWhisperStoreState.isModelLoaded = false;
270273
mockWhisperService.isModelLoaded.mockReturnValue(false);
271274
mockLoadModel.mockResolvedValue(undefined);
272275

273276
renderHook(() => useWhisperTranscription());
274-
275-
// The useEffect runs asynchronously
276-
await act(async () => {
277-
// Let the effect run
278-
});
279-
280-
expect(mockLoadModel).toHaveBeenCalled();
281-
});
282-
283-
it('does not auto-load model when model is already loaded', async () => {
284-
mockWhisperStoreState.downloadedModelId = 'whisper-base';
285-
mockWhisperStoreState.isModelLoaded = true;
286-
mockWhisperService.isModelLoaded.mockReturnValue(true);
287-
288-
renderHook(() => useWhisperTranscription());
289-
290277
await act(async () => {});
291278

292-
expect(mockLoadModel).not.toHaveBeenCalled();
279+
expect(mockLoadModel).not.toHaveBeenCalled(); // no eager load — eviction sticks
293280
});
294281

295282
it('returns isModelLoaded true when store or service reports loaded', () => {
@@ -534,24 +521,4 @@ describe('useWhisperTranscription', () => {
534521
expect(result.current.isTranscribing).toBe(false);
535522
});
536523

537-
// ========================================================================
538-
// auto-load: error is swallowed gracefully (lines 41-43)
539-
// ========================================================================
540-
it('swallows auto-load error and does not propagate', async () => {
541-
mockWhisperStoreState.downloadedModelId = 'whisper-base';
542-
mockWhisperStoreState.isModelLoaded = false;
543-
mockWhisperService.isModelLoaded.mockReturnValue(false);
544-
mockLoadModel.mockRejectedValue(new Error('Network error'));
545-
546-
let thrownError: unknown;
547-
try {
548-
const { unmount } = renderHook(() => useWhisperTranscription());
549-
await act(async () => {});
550-
unmount();
551-
} catch (err) {
552-
thrownError = err;
553-
}
554-
555-
expect(thrownError).toBeUndefined();
556-
});
557524
});

__tests__/unit/services/remoteServerManager.test.ts

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -267,36 +267,6 @@ describe('remoteServerManager', () => {
267267
expect(mockLoadModel).toHaveBeenCalledWith('llama2');
268268
});
269269

270-
it('deselects any active LOCAL text model — mutual exclusion (the dual-selection bug)', async () => {
271-
// Real device repro: a remote model was activated via a non-hook path (launch-restore /
272-
// LAN rediscovery), which bypassed the picker hook's unload-local, so the picker showed
273-
// BOTH a local and a remote text model checked. The owning service must clear local.
274-
const { useAppStore } = require('../../../src/stores/appStore');
275-
useAppStore.getState().setActiveModelId('local-model-x'); // a local model is active
276-
277-
const mockProvider = {
278-
loadModel: jest.fn().mockResolvedValue(undefined),
279-
unloadModel: jest.fn(),
280-
isModelLoaded: jest.fn().mockReturnValue(true),
281-
getLoadedModelId: jest.fn().mockReturnValue('remote-llama'),
282-
isReady: jest.fn().mockResolvedValue(true),
283-
};
284-
(providerRegistry.getProvider as jest.Mock).mockReturnValue(mockProvider);
285-
(providerRegistry.setActiveProvider as jest.Mock).mockReturnValue(true);
286-
(useRemoteServerStore.getState as jest.Mock).mockReturnValue({
287-
setActiveServerId: jest.fn(),
288-
setActiveRemoteTextModelId: jest.fn(),
289-
setActiveRemoteImageModelId: jest.fn(),
290-
getServerById: jest.fn().mockReturnValue(null),
291-
getModelById: jest.fn().mockReturnValue(null),
292-
});
293-
294-
await remoteServerManager.setActiveRemoteTextModel('server-123', 'remote-llama');
295-
296-
// Only the remote model stays active — the local one is deselected.
297-
expect(useAppStore.getState().activeModelId).toBeNull();
298-
});
299-
300270
it('should handle missing provider gracefully', async () => {
301271
(providerRegistry.getProvider as jest.Mock).mockReturnValue(undefined);
302272
(useRemoteServerStore.getState as jest.Mock).mockReturnValue({

src/hooks/useWhisperTranscription.ts

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,23 +39,11 @@ export const useWhisperTranscription = (): UseWhisperTranscriptionResult => {
3939

4040
const { downloadedModelId, isModelLoaded, isModelLoading, loadModel } = useWhisperStore();
4141

42-
// Auto-load model if downloaded but not loaded
43-
useEffect(() => {
44-
let cancelled = false;
45-
const autoLoadModel = async () => {
46-
if (downloadedModelId && !isModelLoaded && !whisperService.isModelLoaded()) {
47-
logger.log('[Whisper] Auto-loading model...');
48-
try {
49-
await loadModel();
50-
if (!cancelled) logger.log('[Whisper] Model auto-loaded successfully');
51-
} catch (err) {
52-
if (!cancelled) logger.error('[Whisper] Failed to auto-load model:', err);
53-
}
54-
}
55-
};
56-
autoLoadModel();
57-
return () => { cancelled = true; };
58-
}, [downloadedModelId, isModelLoaded, loadModel]);
42+
// NOTE: whisper is NOT eager-loaded here. It is warmed once at launch by
43+
// modelPreloader.preloadStt (fits-gated) and loaded on demand by startRecording. An eager
44+
// effect keyed on isModelLoaded re-fired the instant the residency manager EVICTED whisper to
45+
// make room for a text model — reloading it into the just-freed RAM and undoing the eviction
46+
// (the [MEM-SM] override measured corrupted free RAM). Loading on demand lets eviction stick.
5947

6048
// Minimum time to show transcribing state (ms)
6149
const MIN_TRANSCRIBING_TIME = 600;

src/services/activeModelService/index.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,21 +112,19 @@ class ActiveModelService {
112112
: llmService.isModelLoaded();
113113
}
114114

115-
/** If the model is already the loaded one, sync the store selection and report handled. */
116-
private syncActiveIfCurrent(modelId: string): boolean {
117-
if (!this.isTextModelCurrent(modelId)) return false;
118-
const store = useAppStore.getState();
119-
if (store.activeModelId !== modelId) store.setActiveModelId(modelId);
120-
return true;
121-
}
122115
async loadTextModel(
123116
modelId: string,
124117
timeoutMs: number = 120000,
125118
opts?: { override?: boolean },
126119
): Promise<void> {
127-
if (useRemoteServerStore.getState().activeRemoteTextModelId) remoteServerManager.clearActiveRemoteModel(); // mutual exclusion: local deselects remote (all load paths, not just the picker hook)
128120
// Fast path — model already loaded (no lock; just sync the store).
129-
if (this.syncActiveIfCurrent(modelId)) return;
121+
if (this.isTextModelCurrent(modelId)) {
122+
const store = useAppStore.getState();
123+
if (store.activeModelId !== modelId) {
124+
store.setActiveModelId(modelId);
125+
}
126+
return;
127+
}
130128
// Everything else goes through the residency manager's global lock so no two
131129
// model operations ever touch memory at once (the single load gateway).
132130
await modelResidencyManager.runExclusive(`load:text:${modelId}`, () =>
@@ -139,7 +137,13 @@ class ActiveModelService {
139137
opts?: { override?: boolean },
140138
): Promise<void> {
141139
// Re-check after acquiring — a queued call may have loaded it already.
142-
if (this.syncActiveIfCurrent(modelId)) return;
140+
if (this.isTextModelCurrent(modelId)) {
141+
const store = useAppStore.getState();
142+
if (store.activeModelId !== modelId) {
143+
store.setActiveModelId(modelId);
144+
}
145+
return;
146+
}
143147
const store = useAppStore.getState();
144148
const model = store.downloadedModels.find(m => m.id === modelId);
145149
if (!model) {

src/services/remoteServerManagerUtils.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import * as Keychain from 'react-native-keychain';
66
import type { RemoteServer } from '../types';
77
import { useRemoteServerStore } from '../stores/remoteServerStore';
8-
import { useAppStore } from '../stores/appStore';
98
import { createOpenAIProvider, OpenAICompatibleProvider } from './providers/openAICompatibleProvider';
109
import { providerRegistry } from './providers/registry';
1110
import logger from '../utils/logger';
@@ -104,11 +103,6 @@ export async function setActiveRemoteTextModelImpl(
104103

105104
store.setActiveServerId(serverId);
106105
store.setActiveRemoteTextModelId(modelId);
107-
// Mutual exclusion: only ONE active text model. Activating a remote one deselects any active
108-
// local model, so the picker never shows both checked and routing has one unambiguous target.
109-
// Enforced HERE (the owning service) not in the picker hook, so every activation path — LAN
110-
// rediscovery, launch-restore, ModelSelectorModal, the download screen — clears local too.
111-
useAppStore.getState().setActiveModelId(null);
112106

113107
let provider = providerRegistry.getProvider(serverId);
114108
if (!provider) {

0 commit comments

Comments
 (0)