Skip to content

Commit 6b96a11

Browse files
test+chore: measure pro coverage, move image advice to chat, 100% on new modules
- jest.config: collectCoverageFrom now includes pro/** (measured, gated on proExists); pro carved into its own aggregate threshold group (its authoritative gate is the pro repo CI — core only runs pro-dependent suites, ~18%); new standalone modules (imageModelIntegrity, imageGenAdvice, modelLoadErrors, ImageGenAdviceCard) held to 100%. - Move ImageGenAdviceCard out of the settings modal INTO the chat (ChatMessageArea, beside ModelFailureCard) so a user hitting slow/garbled generations actually sees the tips; add a session dismiss. Discoverability fix. - Tests to 100% on the new modules: imageModelIntegrity IO wrappers (RNFS/unzip boundaries only), ImageModelIncompleteError + guard branches, advice-card dismiss + nullish, and a B false-branch wiring test (loadImageModel REFUSES an incomplete model — asserts the outcome: throws + native load never happens). Stub the new barrel export in the two chat-screen suites' component mocks. - CLAUDE.md: document the coverage direction (measure pro; new code 100%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4c93edc commit 6b96a11

12 files changed

Lines changed: 315 additions & 34 deletions

File tree

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/models/activeModelService.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ jest.mock('../../../src/utils/imageModelIntegrity', () => ({
3434
ensureImageExtractionComplete: jest.fn(async () => {}),
3535
}));
3636

37+
import { validateImageModelDir } from '../../../src/utils/imageModelIntegrity';
38+
import { ImageModelIncompleteError } from '../../../src/services/modelLoadErrors';
39+
3740
const mockLlmService = llmService as jest.Mocked<typeof llmService>;
3841
const mockLocalDreamService = localDreamGeneratorService as jest.Mocked<typeof localDreamGeneratorService>;
3942
const mockHardwareService = hardwareService as jest.Mocked<typeof hardwareService>;
@@ -462,6 +465,19 @@ describe('ActiveModelService Integration', () => {
462465
{ backend: 'auto', cpuOnly: false, attentionVariant: undefined },
463466
);
464467
});
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+
});
465481
});
466482

467483
describe('extreme mode (aggressive) — single-model switching text/image/STT', () => {
Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,41 @@
11
/**
2-
* ImageGenAdviceCard — GPU-path speed/quality guidance. Renders nothing off the mnn path
3-
* or at good settings; shows the right tips (raise steps / lower size / raise size) when
4-
* the live settings warrant it. Drives the REAL store + REAL advice rule.
2+
* ImageGenAdviceCard — in-chat GPU-path speed/quality guidance. Renders nothing off the
3+
* mnn path or at good settings; shows the right tips (raise steps / lower size / raise
4+
* size) when the live settings warrant it; is dismissible. Drives the REAL store + rule.
55
*/
66
import React from 'react';
7-
import { render } from '@testing-library/react-native';
7+
import { render, fireEvent } from '@testing-library/react-native';
88

99
jest.mock('react-native-vector-icons/Feather', () => 'Icon');
10+
jest.mock('../../../src/components/AnimatedPressable', () => {
11+
const { TouchableOpacity } = require('react-native');
12+
return { AnimatedPressable: ({ children, onPress, testID }: any) => (
13+
<TouchableOpacity testID={testID} onPress={onPress}>{children}</TouchableOpacity>
14+
) };
15+
});
1016

11-
import { ImageGenAdviceCard } from '../../../src/components/GenerationSettingsModal/ImageGenAdviceCard';
17+
import { ImageGenAdviceCard } from '../../../src/components/ImageGenAdviceCard';
1218
import { useAppStore } from '../../../src/stores';
1319

14-
const setup = (backend: string | undefined, imageSteps: number, imageWidth: number) => {
20+
const setup = (backend: string | undefined, imageSteps: number | undefined, imageWidth: number | undefined) => {
1521
useAppStore.setState({
1622
downloadedImageModels: backend
1723
? ([{ id: 'img', name: 'M', modelPath: '/m', backend, downloadedAt: '', size: 1 }] as any)
1824
: ([] as any),
1925
activeImageModelId: backend ? 'img' : null,
20-
settings: { ...useAppStore.getState().settings, imageSteps, imageWidth, imageHeight: imageWidth },
26+
settings: { ...useAppStore.getState().settings, imageSteps, imageWidth, imageHeight: imageWidth } as any,
2127
});
2228
};
2329

2430
describe('ImageGenAdviceCard', () => {
2531
it('renders nothing on the NPU (qnn) path', () => {
2632
setup('qnn', 8, 512);
27-
const { queryByTestId } = render(<ImageGenAdviceCard />);
28-
expect(queryByTestId('image-gen-advice')).toBeNull();
33+
expect(render(<ImageGenAdviceCard />).queryByTestId('image-gen-advice')).toBeNull();
2934
});
3035

3136
it('renders nothing at the sweet spot (mnn, 22 steps, 256)', () => {
3237
setup('mnn', 22, 256);
33-
const { queryByTestId } = render(<ImageGenAdviceCard />);
34-
expect(queryByTestId('image-gen-advice')).toBeNull();
38+
expect(render(<ImageGenAdviceCard />).queryByTestId('image-gen-advice')).toBeNull();
3539
});
3640

3741
it('shows the raise-steps tip on the GPU path at low steps', () => {
@@ -44,8 +48,7 @@ describe('ImageGenAdviceCard', () => {
4448

4549
it('shows the lower-size tip when too large (512)', () => {
4650
setup('mnn', 22, 512);
47-
const { getByTestId } = render(<ImageGenAdviceCard />);
48-
expect(getByTestId('image-gen-advice-size')).toBeTruthy();
51+
expect(render(<ImageGenAdviceCard />).getByTestId('image-gen-advice-size')).toBeTruthy();
4952
});
5053

5154
it('shows the raise-size (garbage) tip when below 256 (the 128 case)', () => {
@@ -54,4 +57,19 @@ describe('ImageGenAdviceCard', () => {
5457
expect(getByTestId('image-gen-advice-raise-size')).toBeTruthy();
5558
expect(queryByTestId('image-gen-advice-size')).toBeNull();
5659
});
60+
61+
it('can be dismissed (session) — hides after tapping X', () => {
62+
setup('mnn', 8, 256);
63+
const { getByTestId, queryByTestId } = render(<ImageGenAdviceCard />);
64+
fireEvent.press(getByTestId('image-gen-advice-dismiss'));
65+
expect(queryByTestId('image-gen-advice')).toBeNull();
66+
});
67+
68+
it('treats undefined steps/size as 0 without crashing (nullish fallback branch)', () => {
69+
setup('mnn', undefined, undefined);
70+
// width 0 => not >256 and not (0<256 && >0) => no size tip; steps 0 (<20) => raiseSteps.
71+
const { getByTestId, queryByTestId } = render(<ImageGenAdviceCard />);
72+
expect(getByTestId('image-gen-advice-steps')).toBeTruthy();
73+
expect(queryByTestId('image-gen-advice-raise-size')).toBeNull();
74+
});
5775
});

__tests__/rntl/onboarding/ChatScreenSpotlight.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ jest.mock('../../../src/components', () => ({
151151
},
152152
ThinkingIndicator: () => null,
153153
ModelFailureCard: () => null,
154+
ImageGenAdviceCard: () => null,
154155
ModelSelectorModal: () => null,
155156
GenerationSettingsModal: () => null,
156157
ProjectSelectorSheet: () => null,

__tests__/rntl/screens/ChatScreen.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ jest.mock('../../../src/components', () => ({
296296
},
297297
ThinkingIndicator: () => null,
298298
ModelFailureCard: () => null,
299+
ImageGenAdviceCard: () => null,
299300
ModelSelectorModal: ({ visible, onClose, onSelectModel, onUnloadModel }: any) => {
300301
const { View, Text, TouchableOpacity } = require('react-native');
301302
if (!visible) return null;

__tests__/unit/services/modelLoadErrors.test.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { OverridableMemoryError, isOverridableMemoryError } from '../../../src/services/modelLoadErrors';
1+
import {
2+
OverridableMemoryError, isOverridableMemoryError,
3+
ImageModelIncompleteError, isImageModelIncompleteError,
4+
} from '../../../src/services/modelLoadErrors';
25

36
describe('OverridableMemoryError', () => {
47
it('is a real Error subclass carrying the overridable discriminant', () => {
@@ -26,3 +29,35 @@ describe('OverridableMemoryError', () => {
2629
expect(isOverridableMemoryError({ overridable: false })).toBe(false);
2730
});
2831
});
32+
33+
describe('ImageModelIncompleteError', () => {
34+
it('is a real Error subclass carrying the missing files + discriminant', () => {
35+
const err = new ImageModelIncompleteError(['pos_emb.bin', 'clip_v2.mnn.weight']);
36+
expect(err).toBeInstanceOf(Error);
37+
expect(err).toBeInstanceOf(ImageModelIncompleteError);
38+
expect(err.name).toBe('ImageModelIncompleteError');
39+
expect(err.incompleteModel).toBe(true);
40+
expect(err.missing).toEqual(['pos_emb.bin', 'clip_v2.mnn.weight']);
41+
// The user-facing message names the missing files + says to re-download.
42+
expect(err.message).toContain('pos_emb.bin');
43+
expect(err.message).toContain('re-download');
44+
});
45+
46+
it('isImageModelIncompleteError recognises the class', () => {
47+
expect(isImageModelIncompleteError(new ImageModelIncompleteError(['x']))).toBe(true);
48+
});
49+
50+
it('isImageModelIncompleteError recognises a duck-typed object (survives boundaries)', () => {
51+
expect(isImageModelIncompleteError({ incompleteModel: true, missing: ['x'] })).toBe(true);
52+
});
53+
54+
it('rejects plain errors and non-errors', () => {
55+
expect(isImageModelIncompleteError(new Error('generic'))).toBe(false);
56+
expect(isImageModelIncompleteError('incomplete')).toBe(false);
57+
expect(isImageModelIncompleteError(null)).toBe(false);
58+
expect(isImageModelIncompleteError(undefined)).toBe(false);
59+
expect(isImageModelIncompleteError({ incompleteModel: false })).toBe(false);
60+
// An overridable-memory error must NOT read as incomplete (distinct surfaces).
61+
expect(isImageModelIncompleteError(new OverridableMemoryError('x'))).toBe(false);
62+
});
63+
});
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
/**
2+
* imageModelIntegrity IO wrappers — resolveImageModelDir / validateImageModelDir /
3+
* ensureImageExtractionComplete. The pure rule is covered in imageModelIntegrity.test;
4+
* this drives the RNFS + unzip boundaries (mocked) to cover dir resolution, the coreml
5+
* short-circuit, unreadable dirs, and the re-unzip-once-then-throw gate.
6+
*/
7+
const existing = new Set<string>();
8+
const dirListings: Record<string, Array<{ name: string; size: number; isFile: boolean }>> = {};
9+
10+
const mockExists = jest.fn(async (p: string) => existing.has(p));
11+
const mockReadDir = jest.fn(async (p: string) => {
12+
if (!(p in dirListings)) throw new Error(`ENOENT ${p}`);
13+
return dirListings[p].map(e => ({
14+
name: e.name,
15+
size: e.size,
16+
path: `${p}/${e.name}`,
17+
isFile: () => e.isFile,
18+
isDirectory: () => !e.isFile,
19+
}));
20+
});
21+
jest.mock('react-native-fs', () => ({ exists: (p: string) => mockExists(p), readDir: (p: string) => mockReadDir(p) }));
22+
23+
const mockUnzip = jest.fn(async (_source?: string, _target?: string) => '/done');
24+
jest.mock('react-native-zip-archive', () => ({ unzip: (source: string, target: string) => mockUnzip(source, target) }));
25+
26+
jest.mock('../../../src/utils/logger', () => ({ __esModule: true, default: { log: jest.fn(), warn: jest.fn(), error: jest.fn() } }));
27+
28+
import {
29+
resolveImageModelDir, validateImageModelDir, ensureImageExtractionComplete,
30+
} from '../../../src/utils/imageModelIntegrity';
31+
import { ImageModelIncompleteError } from '../../../src/services/modelLoadErrors';
32+
33+
const COMPLETE_MNN = [
34+
{ name: 'unet.mnn', size: 1, isFile: true }, { name: 'unet.mnn.weight', size: 1, isFile: true },
35+
{ name: 'clip_v2.mnn', size: 1, isFile: true }, { name: 'clip_v2.mnn.weight', size: 1, isFile: true },
36+
{ name: 'vae_decoder.mnn', size: 1, isFile: true }, { name: 'vae_decoder.mnn.weight', size: 1, isFile: true },
37+
{ name: 'pos_emb.bin', size: 1, isFile: true }, { name: 'token_emb.bin', size: 1, isFile: true },
38+
{ name: 'tokenizer.json', size: 1, isFile: true },
39+
];
40+
41+
beforeEach(() => {
42+
jest.clearAllMocks();
43+
existing.clear();
44+
for (const k of Object.keys(dirListings)) delete dirListings[k];
45+
mockUnzip.mockResolvedValue('/done');
46+
});
47+
48+
describe('resolveImageModelDir', () => {
49+
it('returns modelPath itself when the unet marker is directly inside', async () => {
50+
existing.add('/m/unet.mnn');
51+
expect(await resolveImageModelDir('/m', 'mnn')).toBe('/m');
52+
});
53+
54+
it('finds the marker one subdir down (e.g. AbsoluteReality/)', async () => {
55+
existing.add('/m/AbsoluteReality/unet.mnn');
56+
dirListings['/m'] = [{ name: 'AbsoluteReality', size: 0, isFile: false }];
57+
expect(await resolveImageModelDir('/m', 'mnn')).toBe('/m/AbsoluteReality');
58+
});
59+
60+
it('finds the marker two subdirs down (qnn output_512/qnn_models_min)', async () => {
61+
existing.add('/m/output_512/qnn_models_min/unet.bin');
62+
dirListings['/m'] = [{ name: 'output_512', size: 0, isFile: false }];
63+
dirListings['/m/output_512'] = [{ name: 'qnn_models_min', size: 0, isFile: false }];
64+
expect(await resolveImageModelDir('/m', 'qnn')).toBe('/m/output_512/qnn_models_min');
65+
});
66+
67+
it('returns null when no marker is found and skips unreadable subdirs', async () => {
68+
dirListings['/m'] = [{ name: 'empty', size: 0, isFile: false }]; // /m/empty is unreadable (no listing)
69+
expect(await resolveImageModelDir('/m', 'mnn')).toBeNull();
70+
});
71+
72+
it('skips top-level files and non-marker sub-files while descending two levels', async () => {
73+
existing.add('/m/sub/deep/unet.mnn');
74+
dirListings['/m'] = [
75+
{ name: 'readme.txt', size: 5, isFile: true }, // top-level FILE → item.isDirectory() false
76+
{ name: 'sub', size: 0, isFile: false },
77+
];
78+
dirListings['/m/sub'] = [
79+
{ name: 'note.bin', size: 5, isFile: true }, // sub FILE → s.isDirectory() false
80+
{ name: 'deep', size: 0, isFile: false },
81+
];
82+
expect(await resolveImageModelDir('/m', 'mnn')).toBe('/m/sub/deep');
83+
});
84+
85+
it('returns null when modelPath itself is unreadable', async () => {
86+
expect(await resolveImageModelDir('/nope', 'mnn')).toBeNull();
87+
});
88+
89+
it('treats an exists() failure as no-marker (hasMarker swallows the error)', async () => {
90+
mockExists.mockRejectedValueOnce(new Error('io error')); // the top-level marker probe throws
91+
expect(await resolveImageModelDir('/m', 'mnn')).toBeNull(); // then /m is unreadable → null
92+
});
93+
});
94+
95+
describe('validateImageModelDir', () => {
96+
it('coreml short-circuits to complete', async () => {
97+
expect(await validateImageModelDir('/m', 'coreml')).toEqual({ complete: true, missing: [] });
98+
});
99+
100+
it('reports the unet as missing when no model dir resolves', async () => {
101+
const res = await validateImageModelDir('/m', 'mnn');
102+
expect(res.complete).toBe(false);
103+
expect(res.missing).toContain('unet.mnn');
104+
});
105+
106+
it('names unet.bin (not unet.mnn) when a qnn model dir does not resolve', async () => {
107+
const res = await validateImageModelDir('/m', 'qnn');
108+
expect(res.missing).toContain('unet.bin');
109+
});
110+
111+
it('treats a zero/NaN file size as 0 (missing) when mapping the listing', async () => {
112+
existing.add('/m/unet.mnn');
113+
// token_emb.bin present but zero-byte → Number(size)||0 = 0 → counted missing.
114+
dirListings['/m'] = COMPLETE_MNN.map(f => (f.name === 'token_emb.bin' ? { ...f, size: 0 } : f));
115+
const res = await validateImageModelDir('/m', 'mnn');
116+
expect(res.complete).toBe(false);
117+
expect(res.missing).toContain('token_emb.bin');
118+
});
119+
120+
it('passes a complete extraction', async () => {
121+
existing.add('/m/unet.mnn');
122+
dirListings['/m'] = COMPLETE_MNN;
123+
expect(await validateImageModelDir('/m', 'mnn')).toEqual({ complete: true, missing: [] });
124+
});
125+
126+
it('flags the missing files of a partial extraction', async () => {
127+
existing.add('/m/unet.mnn');
128+
dirListings['/m'] = COMPLETE_MNN.filter(f => f.name !== 'pos_emb.bin' && f.name !== 'clip_v2.mnn.weight');
129+
const res = await validateImageModelDir('/m', 'mnn');
130+
expect(res.complete).toBe(false);
131+
expect(res.missing).toEqual(expect.arrayContaining(['pos_emb.bin', 'clip_v2.mnn.weight']));
132+
});
133+
134+
it('reports unreadable when the resolved dir cannot be listed', async () => {
135+
existing.add('/m/unet.mnn');
136+
// marker exists so it resolves to /m, but /m has no listing → readDir throws.
137+
const res = await validateImageModelDir('/m', 'mnn');
138+
expect(res.complete).toBe(false);
139+
expect(res.missing).toContain('<unreadable model dir>');
140+
});
141+
});
142+
143+
describe('ensureImageExtractionComplete', () => {
144+
const opts = (backend?: string) => ({ backend: backend as any, modelDir: '/m', zipPath: '/m.zip', modelId: 'id' });
145+
146+
it('is a no-op for coreml / unknown backends', async () => {
147+
await expect(ensureImageExtractionComplete(opts('coreml'))).resolves.toBeUndefined();
148+
await expect(ensureImageExtractionComplete(opts(undefined))).resolves.toBeUndefined();
149+
expect(mockUnzip).not.toHaveBeenCalled();
150+
});
151+
152+
it('passes without re-unzip when already complete', async () => {
153+
existing.add('/m/unet.mnn');
154+
dirListings['/m'] = COMPLETE_MNN;
155+
await expect(ensureImageExtractionComplete(opts('mnn'))).resolves.toBeUndefined();
156+
expect(mockUnzip).not.toHaveBeenCalled();
157+
});
158+
159+
it('re-unzips ONCE and passes when the retry completes the model', async () => {
160+
existing.add('/m/unet.mnn');
161+
dirListings['/m'] = COMPLETE_MNN.filter(f => f.name !== 'pos_emb.bin'); // incomplete first
162+
mockUnzip.mockImplementation(async () => { dirListings['/m'] = COMPLETE_MNN; return '/done'; }); // repaired on re-unzip
163+
await expect(ensureImageExtractionComplete(opts('mnn'))).resolves.toBeUndefined();
164+
expect(mockUnzip).toHaveBeenCalledTimes(1);
165+
});
166+
167+
it('throws ImageModelIncompleteError when still incomplete after the re-unzip', async () => {
168+
existing.add('/m/unet.mnn');
169+
dirListings['/m'] = COMPLETE_MNN.filter(f => f.name !== 'pos_emb.bin');
170+
// re-unzip is a no-op (still missing pos_emb.bin)
171+
await expect(ensureImageExtractionComplete(opts('mnn'))).rejects.toBeInstanceOf(ImageModelIncompleteError);
172+
expect(mockUnzip).toHaveBeenCalledTimes(1);
173+
});
174+
});

0 commit comments

Comments
 (0)