Skip to content

Commit 79e5e44

Browse files
Merge pull request #498 from off-grid-ai/fix/load-anyway-all-model-types
fix: offer "Load Anyway" memory override for all model types (image parity)
2 parents b226246 + d5ee6c5 commit 79e5e44

70 files changed

Lines changed: 11361 additions & 85 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ Always write **both** unit tests and integration tests for new features and sign
134134

135135
Do not consider a feature complete with only unit tests. Integration tests catch wiring bugs, incorrect data flow between layers, and lifecycle issues that unit tests miss.
136136

137+
### Coverage (core AND pro, 100% on new code)
138+
139+
**Coverage must include the `pro/` submodule, not just `src/`.** `jest.config.js` `collectCoverageFrom` collects from `pro/**` whenever the submodule is checked out (gated on `proExists`, so open-core CI without pro still runs). Pro features (TTS/audio, MCP, and other paid surfaces) are exercised by the pro-dependent suites in this repo's `__tests__/` — they must be *measured*, never invisible to coverage. Do not add code to `pro/` without its coverage counting.
140+
141+
**All NEW code ships at 100% - statements, branches, functions, AND lines.** Every new branch/condition (each side of every `if`/ternary/`??`/`||`, each catch, each early return) needs a test that exercises it. Enforce it with a per-file `coverageThreshold` entry at 100 for each new standalone module (core or pro); for a *changed* legacy file, cover every new/changed branch even though the whole file isn't held to 100. New code with an uncovered branch is not done.
142+
137143
**Use mocks very sparingly - a green suite must mean the real thing works, not that a mock returned what it was told.** Mock only what you genuinely cannot run in the test environment (native modules, the network, the device clock). Everything else - the service under test, the stores it writes, the logic across layers - runs for real. A test that mocks the very thing it is asserting (so it would pass even if the implementation were deleted) is worse than no test: it hides the broken behaviour behind a false green. Prefer driving the real class/store/reducer and asserting the observable outcome. When you must stub a boundary, keep the stub dumb (return plain data) and let the real logic on top of it do the work. If a behaviour can only be proven by mocking out the behaviour, that is the signal to test it at a higher layer (integration) or on-device (Provit) instead.
138144

139145
**Design to SOLID with real abstraction layers (not incidental ones).** These are the same rules as the Architecture section above, restated as a standing expectation for every change: one responsibility per module (SRP); callers depend on an interface/service, never on a concrete implementation or a `kind===`/`instanceof`/`Platform.OS`-mechanism branch (DIP); a new implementation (engine, provider, backend) drops in behind the existing seam with zero caller changes (OCP); any implementation is substitutable through the interface (LSP); interfaces are segregated so an implementation never stubs methods it can't support. The abstraction layer must be a genuine owning seam - a service that owns the state machine, resources, and side-effects - not a thin pass-through that leaks the concretes upward. If a fix would add a second concrete branch in a caller, build/extend the seam instead.

__tests__/integration/audio/streamingStateMachine.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ jest.mock('../../../pro/audio/ttsStore', () => ({
5050
import { useTTSStore } from '../../../pro/audio/ttsStore';
5151
import {
5252
feedStreamingText, finishStreamingText, resetStreamingSpeech, isStreamingSpeechActive, _setSpeakTimeoutForTest,
53+
stopStreamingSpeechForTurn,
5354
} from '../../../pro/audio/streamingSpeech';
5455
import { _setSmSink, type SmEvent } from '../../../pro/audio/ttsLog';
5556

@@ -218,3 +219,49 @@ describe('streaming state machine — reset always reclaims the lock', () => {
218219
expect((mockEngine.speak.mock.calls as unknown as string[][]).map((c) => c[0])).toContain('Fresh.');
219220
});
220221
});
222+
223+
// The device bug: pausing a streaming auto-speak fed the paused engine more segments,
224+
// which timed out and tripped the 2-failure "engine wedged → release" path — unloading
225+
// the engine so ALL later playback died. "Stop" (stopStreamingSpeechForTurn) must abort
226+
// cleanly: no wedge, no release, and remaining tokens for the turn must NOT re-engage.
227+
describe('streaming state machine — user stops mid-stream', () => {
228+
it('aborts cleanly without wedging/releasing the engine, and suppresses the rest of the turn', async () => {
229+
speakMode = 'hang'; // engine can't complete (as if paused) — the exact device condition
230+
feedStreamingText('One. Two. Three. ');
231+
await flush();
232+
expect(isStreamingSpeechActive()).toBe(true);
233+
expect(mockEngine.speak).toHaveBeenCalledTimes(1); // segment 1 in flight (hung)
234+
235+
stopStreamingSpeechForTurn(); // user hits STOP mid-segment
236+
237+
// Let the hung speak time out; the orphaned drain must exit, not advance/wedge.
238+
await new Promise((r) => setTimeout(r, 80));
239+
await flush();
240+
241+
expect(names()).toContain('stopStreamingSpeechForTurn (user stop — suppress rest of turn)');
242+
expect(names()).not.toContain('stream drain ABORT: engine wedged → release for fresh remount');
243+
expect(mockEngine.release).not.toHaveBeenCalled();
244+
expect(mockEngine.speak).toHaveBeenCalledTimes(1); // never advanced to segment 2
245+
expect(isStreamingSpeechActive()).toBe(false);
246+
247+
// More tokens on the SAME turn must not restart speech.
248+
feedStreamingText('One. Two. Three. Four. ');
249+
await flush();
250+
expect(isStreamingSpeechActive()).toBe(false);
251+
expect(mockEngine.speak).toHaveBeenCalledTimes(1);
252+
});
253+
254+
it('a new turn (resetStreamingSpeech) clears the suppression and streams again', async () => {
255+
feedStreamingText('Alpha. ');
256+
await flush();
257+
stopStreamingSpeechForTurn();
258+
feedStreamingText('Alpha. Beta. ');
259+
await flush();
260+
expect(isStreamingSpeechActive()).toBe(false); // still suppressed this turn
261+
262+
resetStreamingSpeech(); // audio.stop fires this at the next turn
263+
feedStreamingText('Gamma. ');
264+
await flush();
265+
expect(isStreamingSpeechActive()).toBe(true);
266+
});
267+
});

__tests__/integration/generation/imageGenerationFlow.test.ts

Lines changed: 154 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import {
2020
} from '../../utils/testHelpers';
2121
import { createONNXImageModel, createGeneratedImage, createMessage } from '../../utils/factories';
2222
import { Message } from '../../../src/types';
23+
import { useModelFailureStore } from '../../../src/stores/modelFailureStore';
24+
import { OverridableMemoryError } from '../../../src/services/modelLoadErrors';
2325

2426
// Mock the services
2527
jest.mock('../../../src/services/localDreamGenerator');
@@ -456,8 +458,9 @@ describe('Image Generation Flow Integration', () => {
456458

457459
await imageGenerationService.generateImage({ prompt: 'Test' });
458460

459-
// Should have tried to load model
460-
expect(mockActiveModelService.loadImageModel).toHaveBeenCalledWith('img-model-1');
461+
// Should have tried to load model (override opts threaded through — undefined on
462+
// the normal path, { override: true } only on a Load-Anyway retry).
463+
expect(mockActiveModelService.loadImageModel).toHaveBeenCalledWith('img-model-1', undefined, undefined);
461464
});
462465

463466
it('should reload model if threads changed', async () => {
@@ -1581,6 +1584,154 @@ describe('Image Generation Flow Integration', () => {
15811584
// A successful generation warms the model so the notice never shows again.
15821585
expect(useAppStore.getState().warmedImageModels).toContain(imageModel.id);
15831586
});
1587+
1588+
it('once steps advance on a first run, the label says "Generating" — NOT "GPU optimization in progress"', async () => {
1589+
const imageModel = setupImageModelState();
1590+
useAppStore.setState({ warmedImageModels: [] }); // first run
1591+
useAppStore.setState({ settings: { ...useAppStore.getState().settings, imageUseOpenCL: false } });
1592+
mockLocalDreamService.isModelLoaded.mockResolvedValue(true);
1593+
mockLocalDreamService.getLoadedThreads.mockReturnValue(4);
1594+
// Fire progress past the first step so we exercise the mid-generation label.
1595+
mockLocalDreamService.generateImage.mockImplementation(async (_p, onProgress) => {
1596+
onProgress?.({ step: 1, totalSteps: 20, progress: 0.05 });
1597+
onProgress?.({ step: 6, totalSteps: 20, progress: 0.3 });
1598+
return { id: 'img-1', prompt: 'test', imagePath: '/path/img.png', width: 512, height: 512, steps: 20, seed: 1, modelId: imageModel.id, createdAt: new Date().toISOString() };
1599+
});
1600+
1601+
const statusUpdates: (string | null)[] = [];
1602+
const unsub = imageGenerationService.subscribe(s => { if (s.status) statusUpdates.push(s.status); });
1603+
await imageGenerationService.generateImage({ prompt: 'a dog' });
1604+
unsub();
1605+
1606+
// The misleading "GPU optimization in progress (N/steps)" must be gone...
1607+
expect(statusUpdates.some(s => s?.includes('GPU optimization in progress'))).toBe(false);
1608+
// ...replaced by an honest "Generating image (6/20)" (with a one-time optimize aside).
1609+
const midStep = statusUpdates.find(s => s?.includes('6/20'));
1610+
expect(midStep).toContain('Generating image');
1611+
expect(midStep).toContain('one-time');
1612+
});
1613+
});
1614+
1615+
// ============================================================================
1616+
// Load Anyway override parity: an OVERRIDABLE memory-gate failure on image load
1617+
// must surface the same "Load Anyway" the text path has, and invoking it must
1618+
// re-run the load forcing past the budget. Before the fix, imageGenerationService
1619+
// stringified the typed OverridableMemoryError, so the override was never offered.
1620+
// ============================================================================
1621+
describe('size floor (never generate at a garbage sub-256 resolution)', () => {
1622+
it('floors a stale 128 setting up to 256 before it reaches the native pipeline', async () => {
1623+
const imageModel = setupImageModelState();
1624+
// Simulate the on-device state: user had dragged size down to 128 (garbage for SD1.5).
1625+
useAppStore.setState({ settings: { ...useAppStore.getState().settings, imageWidth: 128, imageHeight: 128 } });
1626+
mockActiveModelService.getActiveModels.mockReturnValue({
1627+
text: { model: null, isLoaded: false, isLoading: false },
1628+
image: { model: imageModel, isLoaded: true, isLoading: false },
1629+
});
1630+
1631+
await imageGenerationService.generateImage({ prompt: 'a dog' });
1632+
1633+
expect(mockLocalDreamService.generateImage).toHaveBeenCalledWith(
1634+
expect.objectContaining({ width: 256, height: 256 }),
1635+
expect.any(Function),
1636+
expect.any(Function),
1637+
);
1638+
});
1639+
1640+
it('passes a valid 512 through unchanged', async () => {
1641+
const imageModel = setupImageModelState();
1642+
useAppStore.setState({ settings: { ...useAppStore.getState().settings, imageWidth: 512, imageHeight: 512 } });
1643+
mockActiveModelService.getActiveModels.mockReturnValue({
1644+
text: { model: null, isLoaded: false, isLoading: false },
1645+
image: { model: imageModel, isLoaded: true, isLoading: false },
1646+
});
1647+
1648+
await imageGenerationService.generateImage({ prompt: 'a dog' });
1649+
1650+
expect(mockLocalDreamService.generateImage).toHaveBeenCalledWith(
1651+
expect.objectContaining({ width: 512, height: 512 }),
1652+
expect.any(Function),
1653+
expect.any(Function),
1654+
);
1655+
});
1656+
});
1657+
1658+
describe('Load Anyway override (overridable memory gate)', () => {
1659+
beforeEach(() => useModelFailureStore.getState().clear());
1660+
1661+
it('offers Load Anyway on an overridable gate, and the retry forces the load with { override: true }', async () => {
1662+
setupImageModelState();
1663+
// Model not resident → a load is attempted; the gate rejects it as overridable,
1664+
// then the forced retry succeeds.
1665+
mockLocalDreamService.isModelLoaded.mockResolvedValue(false);
1666+
mockActiveModelService.loadImageModel
1667+
.mockRejectedValueOnce(
1668+
new OverridableMemoryError('Not enough memory to load Test Model. Free up space or choose a smaller model.'),
1669+
)
1670+
.mockResolvedValue();
1671+
1672+
const result = await imageGenerationService.generateImage({ prompt: 'a fox in snow' });
1673+
expect(result).toBeNull();
1674+
1675+
const failure = useModelFailureStore.getState().failures.find(f => f.modelType === 'image');
1676+
expect(failure).toBeDefined();
1677+
// The discriminant survived the layers (this is the exact regression).
1678+
expect(failure!.overridable).toBe(true);
1679+
expect(typeof failure!.onLoadAnyway).toBe('function');
1680+
1681+
// Invoke "Load Anyway" → it must re-attempt the load FORCING past the budget.
1682+
failure!.onLoadAnyway!();
1683+
for (let i = 0; i < 6; i++) await flushPromises();
1684+
1685+
expect(mockActiveModelService.loadImageModel).toHaveBeenLastCalledWith(
1686+
'img-model-1',
1687+
undefined,
1688+
{ override: true },
1689+
);
1690+
});
1691+
1692+
it('stops offering Load Anyway once the override retry also fails (no repeated no-op)', async () => {
1693+
setupImageModelState();
1694+
mockLocalDreamService.isModelLoaded.mockResolvedValue(false);
1695+
// First load: an overridable gate. The override retry hits the survival floor, so
1696+
// the service reports it as a NON-overridable hard limit (a plain Error) — the exact
1697+
// behavior checkImageModelCanLoad now produces under { override: true }.
1698+
mockActiveModelService.loadImageModel
1699+
.mockRejectedValueOnce(
1700+
new OverridableMemoryError('Not enough memory to load Test Model. Free up space or choose a smaller model.'),
1701+
)
1702+
.mockRejectedValue(
1703+
new Error('Not enough memory to load Test Model, even after freeing other models. Close other apps or choose a smaller model.'),
1704+
);
1705+
1706+
await imageGenerationService.generateImage({ prompt: 'a fox' });
1707+
const first = useModelFailureStore.getState().failures.find(f => f.modelType === 'image');
1708+
expect(first!.overridable).toBe(true);
1709+
expect(typeof first!.onLoadAnyway).toBe('function');
1710+
1711+
// Press "Load Anyway" → the forced retry fails as a hard limit.
1712+
first!.onLoadAnyway!();
1713+
for (let i = 0; i < 6; i++) await flushPromises();
1714+
1715+
// The card must NOT keep offering "Load Anyway" — the action would be a no-op.
1716+
const after = useModelFailureStore.getState().failures.find(f => f.modelType === 'image');
1717+
expect(after).toBeDefined();
1718+
expect(after!.overridable).toBeFalsy();
1719+
expect(after!.onLoadAnyway).toBeUndefined();
1720+
});
1721+
1722+
it('does NOT offer Load Anyway for a NON-overridable load failure (false branch)', async () => {
1723+
setupImageModelState();
1724+
mockLocalDreamService.isModelLoaded.mockResolvedValue(false);
1725+
mockActiveModelService.loadImageModel.mockRejectedValue(new Error('Pipeline failed: model corrupt'));
1726+
1727+
const result = await imageGenerationService.generateImage({ prompt: 'a fox' });
1728+
expect(result).toBeNull();
1729+
1730+
const failure = useModelFailureStore.getState().failures.find(f => f.modelType === 'image');
1731+
expect(failure).toBeDefined();
1732+
expect(failure!.overridable).toBeFalsy();
1733+
expect(failure!.onLoadAnyway).toBeUndefined();
1734+
});
15841735
});
15851736

15861737
describe('_ensureImageModelLoaded with null activeImageModelId', () => {
@@ -1590,7 +1741,7 @@ describe('Image Generation Flow Integration', () => {
15901741
mockLocalDreamService.getLoadedModelPath.mockResolvedValue(null);
15911742
mockLocalDreamService.getLoadedThreads.mockReturnValue(4);
15921743

1593-
const result = await (imageGenerationService as any)._ensureImageModelLoaded(null, fakeModel, 4);
1744+
const result = await (imageGenerationService as any)._ensureImageModelLoaded(null, fakeModel, { desiredThreads: 4 });
15941745

15951746
expect(result).toBe(false);
15961747
expect(imageGenerationService.getState().error).toBe('No image model selected');

__tests__/integration/models/activeModelService.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ import { createDownloadedModel, createONNXImageModel, createDeviceInfo } from '.
2727
jest.mock('../../../src/services/llm');
2828
jest.mock('../../../src/services/localDreamGenerator');
2929
jest.mock('../../../src/services/hardware');
30+
// Integrity is a boundary for these residency/memory tests (the model files aren't laid
31+
// down on a real disk here). The completeness rule has its own dedicated tests.
32+
jest.mock('../../../src/utils/imageModelIntegrity', () => ({
33+
validateImageModelDir: jest.fn(async () => ({ complete: true, missing: [] })),
34+
ensureImageExtractionComplete: jest.fn(async () => {}),
35+
}));
36+
37+
import { validateImageModelDir } from '../../../src/utils/imageModelIntegrity';
38+
import { ImageModelIncompleteError } from '../../../src/services/modelLoadErrors';
3039

3140
const mockLlmService = llmService as jest.Mocked<typeof llmService>;
3241
const mockLocalDreamService = localDreamGeneratorService as jest.Mocked<typeof localDreamGeneratorService>;
@@ -456,6 +465,19 @@ describe('ActiveModelService Integration', () => {
456465
{ backend: 'auto', cpuOnly: false, attentionVariant: undefined },
457466
);
458467
});
468+
469+
it('refuses an INCOMPLETE model — throws ImageModelIncompleteError and never reaches native load (B respects the verdict)', async () => {
470+
const broken = createONNXImageModel({ id: 'img-broken', backend: 'mnn' });
471+
useAppStore.setState({ downloadedImageModels: [broken], settings: { imageThreads: 4 } as any });
472+
mockLocalDreamService.isModelLoaded.mockResolvedValue(false);
473+
// Force the REAL loadImageModel to see an INCOMPLETE verdict from the integrity
474+
// boundary. Assert the CONSEQUENCE (throws + native load never happens), not the call —
475+
// this catches a caller that queries integrity but ignores `complete: false`.
476+
(validateImageModelDir as jest.Mock).mockResolvedValueOnce({ complete: false, missing: ['pos_emb.bin', 'clip_v2.mnn.weight'] });
477+
478+
await expect(activeModelService.loadImageModel('img-broken')).rejects.toBeInstanceOf(ImageModelIncompleteError);
479+
expect(mockLocalDreamService.loadModel).not.toHaveBeenCalled();
480+
});
459481
});
460482

461483
describe('extreme mode (aggressive) — single-model switching text/image/STT', () => {

__tests__/integration/models/imageDownloadRecovery.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ const mockModelManager = {
1212
addDownloadedImageModel: jest.fn(),
1313
};
1414

15+
// Integrity is a boundary for this recovery test (it exercises the resume/move
16+
// orchestration, not the completeness rule — that has its own dedicated tests).
17+
jest.mock('../../../src/utils/imageModelIntegrity', () => ({
18+
validateImageModelDir: jest.fn(async () => ({ complete: true, missing: [] })),
19+
ensureImageExtractionComplete: jest.fn(async () => {}),
20+
}));
21+
1522
jest.mock('../../../src/services/backgroundDownloadService', () => ({
1623
backgroundDownloadService: mockBackgroundDownloadService,
1724
}));

0 commit comments

Comments
 (0)