Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .agents/skills/add-api-tests/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ Key helpers:
5. **Options** — every default in `modelOpts`, and every per-call override.
6. **Disposal** — `dispose()` leaves `fakeJsi.liveTensors()` at 0 and
`fakeJsi.liveModels()` empty, and repeated calls do not accumulate scratch
tensors.
tensors. The same has to hold when construction *fails*: allocate through a
`createResourceScope()` and wrap the factory body in `try`/`catch` so a
schema mismatch releases the model instead of stranding it. Add the factory
to `__tests__/tasks/constructionFailure.test.ts`, which drives every one of
them.
7. **Sync/async parity** — `runTaskWorklet(x)` equals `await runTask(x)`.

Only the *weights* are out of scope, not the pipeline that runs on them. Before
Expand Down Expand Up @@ -144,5 +148,8 @@ When adding or changing code under `src/`, verify that:
is intentional (a removal or rename is a breaking change).
- [ ] Any new fake behaviour in `__tests__/support/` is faithful where fidelity
changes an assertion, and its simplifications are commented.
- [ ] No test was made to pass by calling `allowNativeLeaks()` without an
explanation.
- [ ] A new `create<Task>` allocates through `createResourceScope()` and is
listed in `__tests__/tasks/constructionFailure.test.ts`.
- [ ] No test was made to pass by calling `allowNativeLeaks()`. Nothing in the
suite needs it today, so reach for it only when a leak is genuinely the
point of the test, and say why.
29 changes: 21 additions & 8 deletions .agents/skills/add-task-pipeline/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,28 @@ When implementing task constructors like `create<Task>` (e.g. `createClassifier`
const [tReshape, tUint8] = tensors;
```

2. **Immediate `dispose()` Definition**:
- Right after allocating the static tensors, define the `dispose` function immediately. This makes it instantly visible and verifiable that all native memory will be cleaned up:
2. **Allocate Through a Resource Scope**:
- Open a `createResourceScope()` as the first statement of the constructor, take its `dispose` as the pipeline's, and wrap the whole body in `try`/`catch`. `track` every native resource as it is created:
```typescript
const dispose = () => {
tensors.forEach((t) => t.dispose());
preprocessor.dispose();
model.dispose();
};
const scope = createResourceScope();
const dispose = scope.dispose;

try {
const model = scope.track(await wrapAsync(loadModel, runtime)(modelPath));
const { dims } = validateSpec(model.schema, { ... }); // may throw

const tensors = [tensor('float32', shapeA), tensor('float32', shapeB)] as const;
tensors.forEach(scope.track);
const preprocessor = scope.track(createImagePreprocessor(modelOpts, inpShape));

return { runTask, runTaskWorklet, dispose };
} catch (error) {
dispose();
throw error;
}
```
- This is not style. A caller whose `create<Task>` throws never receives a `dispose`, so anything already allocated would be stranded in native memory for the life of the process, and `useModel` re-runs the factory on every config change. The scope makes success and failure share one teardown path.
- Track resources **as they land**, not afterwards. For parallel loads that means `Promise.all([load(a).then(scope.track), load(b).then(scope.track)])`, so one rejecting does not strand the other.

3. **Dynamic Tensors & `try/finally` Pattern**:
- If you must allocate dynamically sized tensors during inference execution (e.g. resizing an output tensor to match the input image dimensions), you must wrap the execution inside a `try {} finally {}` block.
Expand All @@ -51,7 +64,7 @@ When implementing task constructors like `create<Task>` (e.g. `createClassifier`

4. **Pure Helper Functions**:
- Write all auxiliary/helper logic as pure, worklet-compatible functions **outside** the `create<Task>` constructor. Any helper functions invoked inside the worklet executor thread must contain the `'worklet';` directive.
- **Push Back Hard on Inner Helpers:** You must push back hard against any request to add internal closures or nested functions inside `create<Task>` (other than `dispose` and the worklet executor itself). Keep the constructor scope flat to avoid scope leak and dependency chain bugs.
- **Push Back Hard on Inner Helpers:** You must push back hard against any request to add internal closures or nested functions inside `create<Task>` (other than the worklet executor itself). Keep the constructor scope flat to avoid scope leak and dependency chain bugs.

5. **PTE Model Export & Optimizations**:
- **Shift Heavy Ops to PyTorch**: Push complex tensor reshaping, data normalization, activations (e.g. `softmax`), or bounding box decoding into the PyTorch model itself so they execute on native backends (e.g., XNNPACK or CoreML).
Expand Down
5 changes: 4 additions & 1 deletion packages/react-native-executorch/__tests__/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ for free.

Wrap construction in `tracked()` and the harness disposes it at the end of the
test — which also keeps a failing assertion from cascading into a second,
misleading leak error. A test that means to leak calls `allowNativeLeaks()`.
misleading leak error. A test that means to leak calls `allowNativeLeaks()`;
nothing in the suite currently does, including the construction-failure cases,
because a factory that throws part-way now releases what it had allocated
(`src/core/lifetime.ts`).

## Layout

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ exports[`public API surface matches the recorded export list 1`] = `
"createObjectDetector",
"createPaddleOcr",
"createPrivacyFilter",
"createResourceScope",
"createSdxsTextToImage",
"createSemanticSegmenter",
"createStyleTransfer",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { deferred, fakeNet } from '../support/blobUtilMock';
import { cachePathFor } from '../support/cachePath';
import { fakeJsi } from '../support/fakeJsi';
import { STRETCH_PREPROCESSING, exported, imageBuffer, writesOutputs } from '../support/fixtures';
import { allowNativeLeaks } from '../support/setup';

const MODEL_URL = 'https://huggingface.co/software-mansion/model/resolve/v1/model.pte';
const TOKENIZER_URL = 'https://huggingface.co/software-mansion/model/resolve/v1/tokenizer.json';
Expand Down Expand Up @@ -121,11 +120,11 @@ describe('useClassifier', () => {
expect(result.current.error?.message).toMatch(/labels length \(2\)/);
expect(result.current.isReady).toBe(false);

// The failed construction abandons the native model it had already loaded,
// and the hook has no handle to release — this is the app-level shape of
// the leak recorded in `tasks/constructionFailure.test.ts`.
expect(fakeJsi.liveModels()).toEqual([cachePathFor(MODEL_URL)]);
allowNativeLeaks();
// The hook never receives a `dispose` for a pipeline that failed to build,
// so the factory has to have released the model itself. `useModel` re-runs
// the factory on every config change, which is what makes this the
// app-level shape of `tasks/constructionFailure.test.ts`.
expect(fakeJsi.liveModels()).toEqual([]);
});

it('loads nothing while preventLoad is set', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@
* Disposal is idempotent, so tests that assert on `dispose()` explicitly can
* still call it themselves.
*/
type Disposable = { dispose: () => void };
type NativeResource = { dispose: () => void };

const created: Disposable[] = [];
const created: NativeResource[] = [];

/**
* Registers `instance` for disposal at the end of the current test.
* @typeParam T The pipeline type.
* @param instance The pipeline to track.
* @returns The same instance.
*/
export function tracked<T extends Disposable>(instance: T): T {
export function tracked<T extends NativeResource>(instance: T): T {
created.push(instance);
return instance;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { createClassifier } from '../../src/extensions/cv/tasks/classification';
import { fakeJsi } from '../support/fakeJsi';
import { tracked } from '../support/lifetime';
import { STRETCH_PREPROCESSING, exported, imageBuffer, writesOutputs } from '../support/fixtures';
import { allowNativeLeaks } from '../support/setup';

const MODEL_PATH = '/models/classifier.pte';
const LABELS = ['cat', 'dog', 'bird'] as const;
Expand Down Expand Up @@ -46,15 +45,13 @@ describe('createClassifier — model acceptance', () => {
});

await expect(createClassifier(config())).rejects.toThrow(/doesn't match any of the provided/);
allowNativeLeaks(); // see `tasks/constructionFailure.test.ts`
});

it('rejects a labels array that does not match the output dimension', async () => {
registerBatched();
await expect(createClassifier(config(['cat', 'dog']))).rejects.toThrow(
/labels length \(2\) must match model output dimension \(3\)/
);
allowNativeLeaks(); // see `tasks/constructionFailure.test.ts`
});

it('surfaces a load failure', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,47 @@
/**
* What a `create<Task>` factory leaves behind when it throws.
*
* Every factory follows the same shape: load the model (and, for some tasks, a
* tokenizer), validate its schema, pre-allocate the execution tensors, and only
* then hand back a `dispose`. If validation throws, the caller never receives a
* `dispose` — so anything already allocated is unreachable from JavaScript and
* stays alive in native memory for the process's lifetime.
* Every factory follows the same shape: load a model (and, depending on the
* task, a tokenizer, a phonemizer, an LLM runner or a whole nested pipeline),
* validate the schema, pre-allocate the execution tensors, and only then hand
* back a `dispose`. A failure anywhere after the first allocation means the
* caller never receives that `dispose`, so whatever was already allocated is
* unreachable from JavaScript — and native memory is not garbage collected, so
* it stays alive for the rest of the process.
*
* That is what these tests record. They are written as assertions on the
* current* behavior rather than on the desired behavior, so the day a
* factory starts cleaning up after itself they fail loudly and can be flipped,
* instead of quietly passing either way.
* The exposure is not theoretical: `useModel` re-runs its factory whenever the
* config changes, so an app pointed at a mismatched model would leak one
* resource set per attempt.
*
* The exposure is real: `useModel` re-runs its factory whenever the config
* changes, so an app pointed at a mismatched model leaks one native model per
* attempt.
* So every factory releases what it allocated before rethrowing, and this suite
* holds them to it. The assertions are per-resource-kind rather than left to
* the setup file's global leak check, so a failure says which factory and which
* kind of handle, not just that something leaked.
*/
import { f32, method } from '../../src/core/schema';
import { createClassifier } from '../../src/extensions/cv/tasks/classification';
import { createImageEmbedder } from '../../src/extensions/cv/tasks/imageEmbedding';
import { createInstanceSegmenter } from '../../src/extensions/cv/tasks/instanceSegmentation';
import { createKeypointDetector } from '../../src/extensions/cv/tasks/keypointDetection';
import { createObjectDetector } from '../../src/extensions/cv/tasks/objectDetection';
import { createPaddleOcr } from '../../src/extensions/cv/tasks/paddleOcr';
import { createSdxsTextToImage } from '../../src/extensions/cv/tasks/sdxsTextToImage';
import { createSemanticSegmenter } from '../../src/extensions/cv/tasks/semanticSegmentation';
import { createStyleTransfer } from '../../src/extensions/cv/tasks/styleTransfer';
import { createPrivacyFilter } from '../../src/extensions/nlp/tasks/privacyFilter';
import { createTextEmbedder } from '../../src/extensions/nlp/tasks/textEmbedding';
import { createFsmnVoiceActivityDetector } from '../../src/extensions/speech/tasks/fsmnVoiceActivityDetection';
import { createKokoroTextToSpeech } from '../../src/extensions/speech/tasks/kokoroTextToSpeech';
import { createSupertonicTextToSpeech } from '../../src/extensions/speech/tasks/supertonicTextToSpeech';
import { createWhisperSpeechToText } from '../../src/extensions/speech/tasks/whisperSpeechToText';
import { fakeFs } from '../support/blobUtilMock';
import { fakeJsi } from '../support/fakeJsi';
import { STRETCH_PREPROCESSING, exported } from '../support/fixtures';
import { allowNativeLeaks } from '../support/setup';

const MODEL_PATH = '/models/mismatched.pte';
const TOKENIZER_PATH = '/models/tokenizer.json';
const CHARSET_PATH = '/models/charset.json';
const VOICE_PATH = '/models/voice.bin';

/** A schema no task pipeline declares: two inputs, three outputs, wrong ranks. */
const MISMATCHED = exported(method('forward', [f32(9), f32(9)], [f32(9), f32(9), f32(9)]));
Expand All @@ -37,63 +52,149 @@ const CV_OPTS = {
outInterpolation: 'linear',
outNormalizeOpts: { alpha: 255, beta: 0 },
labels: ['a'],
landmarks: ['nose'],
boxFormat: 'xyxy',
defaultIouThreshold: 0.5,
defaultMaskThreshold: 0.5,
defaultConfidenceThreshold: 0.5,
} as const;

const factories = {
classifier: () => createClassifier({ modelPath: MODEL_PATH, modelOpts: CV_OPTS }),
objectDetector: () => createObjectDetector({ modelPath: MODEL_PATH, modelOpts: CV_OPTS }),
semanticSegmenter: () => createSemanticSegmenter({ modelPath: MODEL_PATH, modelOpts: CV_OPTS }),
styleTransfer: () => createStyleTransfer({ modelPath: MODEL_PATH, modelOpts: CV_OPTS }),
imageEmbedder: () => createImageEmbedder({ modelPath: MODEL_PATH, modelOpts: CV_OPTS }),
const cv = { modelPath: MODEL_PATH, modelOpts: CV_OPTS } as const;
const tokenized = { modelPath: MODEL_PATH, tokenizerPath: TOKENIZER_PATH } as const;
const vadModel = {
modelPath: MODEL_PATH,
defaultOptions: {
speechThreshold: 0.5,
minSpeechDurationMs: 100,
minSilenceDurationMs: 100,
speechPadMs: 30,
mergeGapMs: 100,
},
} as const;

/**
* Every factory that allocates something before it can fail, with whatever it
* needs registered so the failure lands on schema validation rather than on the
* load itself.
*
* `createTokenizer` is absent on purpose: it loads a tokenizer and returns, with
* nothing in between that can throw, so it has no window to leak through.
*/
const factories: Record<string, () => Promise<unknown>> = {
classifier: () => createClassifier(cv),
objectDetector: () => createObjectDetector(cv),
semanticSegmenter: () => createSemanticSegmenter(cv),
styleTransfer: () => createStyleTransfer(cv),
imageEmbedder: () => createImageEmbedder(cv),
instanceSegmenter: () => createInstanceSegmenter(cv),
keypointDetector: () => createKeypointDetector(cv),
paddleOcr: () =>
createPaddleOcr({
modelPath: MODEL_PATH,
charsetPath: CHARSET_PATH,
modelOpts: { defaultConfidenceThreshold: 0.5 },
}),
sdxsTextToImage: () => createSdxsTextToImage(tokenized),
textEmbedder: () => createTextEmbedder(tokenized),
privacyFilter: () =>
createPrivacyFilter({
...tokenized,
modelOpts: { labelNames: ['O', 'B-p', 'I-p', 'E-p', 'S-p'], padTokenId: 0 },
}),
whisperSpeechToText: () =>
createWhisperSpeechToText({
...tokenized,
vadModel,
supportedLanguages: ['en'],
} as never),
fsmnVoiceActivityDetector: () => createFsmnVoiceActivityDetector(vadModel),
supertonicTextToSpeech: () =>
createSupertonicTextToSpeech({
name: 'supertonic',
modelPaths: {
durationPredictor: MODEL_PATH,
vectorEstimator: MODEL_PATH,
textEncoder: MODEL_PATH,
vocoder: MODEL_PATH,
},
voices: { v: VOICE_PATH },
} as never),
kokoroTextToSpeech: () =>
createKokoroTextToSpeech({
name: 'kokoro',
modelPaths: { durationPredictor: MODEL_PATH, synthesizer: MODEL_PATH },
phonemizer: { lang: 'en-us' },
voices: { v: VOICE_PATH },
} as never),
};

/** Everything the fake still considers allocated, as one comparable object. */
const stillAllocated = () => ({
models: fakeJsi.liveModels(),
tokenizers: fakeJsi.liveTokenizers(),
runners: fakeJsi.liveRunners(),
phonemizers: fakeJsi.livePhonemizers(),
tensors: fakeJsi.liveTensorDescriptions(),
});

const NOTHING = { models: [], tokenizers: [], runners: [], phonemizers: [], tensors: [] };

describe('create<Task> — schema validation failure', () => {
beforeEach(() => {
fakeJsi.registerModel(MODEL_PATH, { schema: MISMATCHED });
fakeJsi.registerTokenizer(TOKENIZER_PATH, { tokens: ['a', 'b'] });
fakeFs.write(CHARSET_PATH, JSON.stringify(['a', 'b', 'c']));
fakeFs.write(VOICE_PATH, new Uint8Array(1024));
});

it.each(Object.entries(factories))('create%s rejects rather than resolving', async (_n, make) => {
await expect(make()).rejects.toThrow();
});

it.each(Object.entries(factories))(
'create%s rejects with a message naming every variant it tried',
async (_name, factory) => {
await expect(factory()).rejects.toThrow(/doesn't match any of the provided variants/);
allowNativeLeaks();
'create%s releases everything it had allocated',
async (_name, make) => {
await expect(make()).rejects.toThrow();

expect(stillAllocated()).toEqual(NOTHING);
}
);

it.each(Object.entries(factories))(
'create%s abandons the loaded native model (known leak)',
async (_name, factory) => {
await expect(factory()).rejects.toThrow();

expect(fakeJsi.liveModels()).toEqual([MODEL_PATH]);
allowNativeLeaks();
it.each(Object.entries(factories).filter(([name]) => name !== 'kokoroTextToSpeech'))(
'create%s names every variant it tried',
async (_name, make) => {
// A caller should learn what is wrong from the message. Kokoro validates
// two sub-models and reports whichever failed first, so its wording is
// pinned in its own suite instead.
await expect(make()).rejects.toThrow(/doesn't match any of the provided variants/);
}
);

it('leaks one model per failed attempt, the way a re-rendering hook would', async () => {
it('stays clean across repeated attempts, the way a re-rendering hook would', async () => {
// `useModel` re-runs its factory on every config change. Before the
// factories cleaned up, this leaked one model per attempt.
for (const path of ['/a.pte', '/b.pte', '/c.pte']) {
fakeJsi.registerModel(path, { schema: MISMATCHED });
await expect(createClassifier({ modelPath: path, modelOpts: CV_OPTS })).rejects.toThrow();
await expect(createClassifier({ ...cv, modelPath: path })).rejects.toThrow();
}

expect(fakeJsi.liveModels()).toEqual(['/a.pte', '/b.pte', '/c.pte']);
allowNativeLeaks();
expect(stillAllocated()).toEqual(NOTHING);
});
});

describe('create<Task> — load failure', () => {
it.each(Object.entries(factories))(
'create%s leaves nothing allocated when the model itself cannot be loaded',
async (_name, factory) => {
// Nothing registered at MODEL_PATH, so `loadModel` throws before any
// allocation happens — the one failure path that is already clean.
await expect(factory()).rejects.toThrow(/mismatched.pte/);
async (_name, make) => {
// Nothing registered, so the very first load throws. The factory has
// allocated nothing yet, but for the multi-resource pipelines a sibling
// load may already have succeeded.
fakeFs.write(CHARSET_PATH, JSON.stringify(['a', 'b', 'c']));
fakeFs.write(VOICE_PATH, new Uint8Array(1024));

await expect(make()).rejects.toThrow();

expect(fakeJsi.liveModels()).toEqual([]);
expect(fakeJsi.liveTensors()).toBe(0);
expect(stillAllocated()).toEqual(NOTHING);
}
);
});
Loading