Prepare to refactror worker model list loader.
Browse files
plans/2025-08-01-model-filtering/4-worker-refactoring-async-iterator.md
ADDED
|
@@ -0,0 +1,105 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
## Goal
|
| 2 |
+
|
| 3 |
+
Move the core list/classify pipeline out of `src/worker/boot-worker.js` into a small, well-scoped async-iterator action module `src/worker/actions/list-chat-models.js`. The module will implement the pipeline as an `async function*` that yields plain JSON progress objects; the worker (`boot-worker.js`) will keep a minimal wrapper that forwards yielded objects to the main thread via `postMessage`, and will cancel the task by exiting the iterator (call `iterator.return()`).
|
| 4 |
+
|
| 5 |
+
This refactor should keep code short and pragmatic, make the core logic easily unit-testable, and keep message/I/O concerns isolated to the wrapper.
|
| 6 |
+
|
| 7 |
+
## Final state / success criteria
|
| 8 |
+
|
| 9 |
+
- `src/worker/actions/list-chat-models.js` exists and exports `async function* listChatModelsIterator(params = {})`.
|
| 10 |
+
- The iterator contains listing, prefilter, promise-pool config fetch, classification logic, yields incremental progress objects, and throws on irrecoverable errors.
|
| 11 |
+
- The generator manages per-request AbortControllers and cleans them up in `finally` when iteration ends early (iterator.return()).
|
| 12 |
+
- `src/worker/boot-worker.js` imports the iterator and provides a 10–20 line wrapper that iterates and forwards yields via `self.postMessage({ id, type: 'progress', ...delta })`, sends final response or error, and stores an `activeTasks` abort function that calls `iterator.return()`.
|
| 13 |
+
- `src/app/worker-connection.js` does not double-send requests; only one control message is sent per invocation.
|
| 14 |
+
- All yielded objects are JSON-serializable and follow the agreed shapes.
|
| 15 |
+
|
| 16 |
+
## Yield contract (canonical shapes)
|
| 17 |
+
|
| 18 |
+
All yields must be plain JSON-serializable objects. The wrapper will not inspect internals beyond forwarding the object via `postMessage`.
|
| 19 |
+
|
| 20 |
+
- { status: 'listing_done', totalFound: number }
|
| 21 |
+
- { status: 'prefiltered', survivors: number }
|
| 22 |
+
- { modelId: string, status: 'config_fetching' }
|
| 23 |
+
- { modelId: string, status: 'classified', data: ModelEntry }
|
| 24 |
+
- { modelId: string, status: 'error', data: { message: string, code?: string } } // non-fatal per-model
|
| 25 |
+
- { status: 'done', models: ModelEntry[], meta: { fetched:number, filtered:number, errors:[] } }
|
| 26 |
+
|
| 27 |
+
ModelEntry minimal shape (for `classified` / final models):
|
| 28 |
+
- { id, model_type?, architectures?, classification?, confidence?, fetchStatus? }
|
| 29 |
+
|
| 30 |
+
Design rule: yield final `{ status: 'done' }` explicitly; do not rely on generator return value for final payload.
|
| 31 |
+
|
| 32 |
+
## Implementation steps (exact, minimal)
|
| 33 |
+
|
| 34 |
+
1) Create the action file
|
| 35 |
+
|
| 36 |
+
- File: `src/worker/actions/list-chat-models.js`
|
| 37 |
+
- Export: `export async function* listChatModelsIterator(params = {})`
|
| 38 |
+
- Move the core logic from `boot-worker.js` into this file: listing pagination, prefilter, promise-pool config fetcher (with short per-request controllers), classification helpers.
|
| 39 |
+
- Replace any `self.postMessage(...)` calls with `yield` of the appropriate shape described above.
|
| 40 |
+
- For fatal errors (e.g. listing failure), throw a plain Error with an optional `code` property, e.g. `const e = new Error('listing failed'); e.code = 'listing_failed'; throw e;`.
|
| 41 |
+
|
| 42 |
+
2) Cancellation and cleanup inside the generator (important)
|
| 43 |
+
|
| 44 |
+
- Maintain a `const inFlight = new Set()` of per-request AbortControllers.
|
| 45 |
+
- Use a small helper `fetchWithController(url, init)` that:
|
| 46 |
+
- creates `const c = new AbortController(); inFlight.add(c);`
|
| 47 |
+
- performs `fetch(url, { ...init, signal: c.signal })`
|
| 48 |
+
- in a finally block does `inFlight.delete(c)`.
|
| 49 |
+
- Surround the main body with a `try { ... } finally { for (const c of inFlight) c.abort(); }` so when the caller calls `iterator.return()` the generator's finally runs and aborts in-flight requests.
|
| 50 |
+
- Do not yield a special cancelled object inside finally unless you want the wrapper to forward it; be mindful that yields inside finally will be observed by the caller. If you want a single `{ cancelled:true }` response from the wrapper, prefer the wrapper to send it after `iterator.return()` completes.
|
| 51 |
+
|
| 52 |
+
3) Helper functions
|
| 53 |
+
|
| 54 |
+
- Move `fetchConfigForModel` and `classifyModel` into the new module. Keep them small and synchronous where possible. They should return JSON-serializable values only.
|
| 55 |
+
|
| 56 |
+
4) Replace inline handler in `boot-worker.js` with thin wrapper
|
| 57 |
+
|
| 58 |
+
- Import the iterator:
|
| 59 |
+
- `import { listChatModelsIterator } from './actions/list-chat-models.js';`
|
| 60 |
+
- Replace the long `handleListChatModels` body with:
|
| 61 |
+
- Create iterator: `const iterator = listChatModelsIterator(params);`
|
| 62 |
+
- Register abort: `activeTasks.set(id, { abort: () => { try { iterator.return(); } catch(e) {} } });`
|
| 63 |
+
- For-await-of the iterator and forward yields:
|
| 64 |
+
- `for await (const delta of iterator) { self.postMessage(Object.assign({ id, type: 'progress' }, delta)); if (delta.status === 'done') { self.postMessage({ id, type: 'response', result: { models: delta.models, meta: delta.meta } }); break; } }`
|
| 65 |
+
- Catch thrown errors from the iterator and send `self.postMessage({ id, type: 'error', error: String(err), code: err.code || null })`.
|
| 66 |
+
- Finally: `activeTasks.delete(id);` and if the iterator exited without a final `done` and without throwing, send `self.postMessage({ id, type: 'response', result: { cancelled: true } });`.
|
| 67 |
+
|
| 68 |
+
5) Fix main-thread double-send (one-line)
|
| 69 |
+
|
| 70 |
+
- Edit `src/app/worker-connection.js`'s `listChatModels` to avoid both `worker.postMessage(msg)` and also calling `send({ type: 'listChatModels', params })`. Use a single path: prefer the `send()` helper which sets the pending map. Remove the duplicate call.
|
| 71 |
+
|
| 72 |
+
6) Tests & smoke checks
|
| 73 |
+
|
| 74 |
+
- Manual smoke test:
|
| 75 |
+
- Start server and call UI flow to trigger `listChatModels`. Verify a stream of `progress` messages and a final `response` with `models` and `meta`.
|
| 76 |
+
- Trigger `cancelListChatModels` quickly and verify the wrapper sends `{ cancelled: true }` and generator's finally cleaned up (no lingering network tasks).
|
| 77 |
+
- Cause a fatal listing error (e.g., simulate network down) and verify `type:'error'` arrives with error string and code.
|
| 78 |
+
- Unit test ideas:
|
| 79 |
+
- Mock `fetch` to return known pages and config.jsons and iterate the generator directly in node, asserting yielded items and final result.
|
| 80 |
+
- Test iterator.return() from a caller and assert finally executed (e.g., manipulate an `inFlight` spy or set a flag inside finally).
|
| 81 |
+
|
| 82 |
+
## Practical caveats & guidance
|
| 83 |
+
|
| 84 |
+
- Backpressure: many model-level yields may flood the main thread. If the UI is overwhelmed, coalesce `classified` yields in the wrapper (simple time-window batching, e.g. 50-100ms). Keep this as an optimization only if necessary.
|
| 85 |
+
- Yielding inside `finally`: if you yield summary data during cleanup, the wrapper will forward those yields. If you prefer a single `{ cancelled:true }` response, avoid yielding in finally and let the wrapper send the cancellation response after `iterator.return()` finishes.
|
| 86 |
+
- Error objects are not serializable; thrown Errors should be caught by wrapper and stringified. For structured error codes, attach a `code` property to the Error before throwing.
|
| 87 |
+
- Keep import paths relative and test in the worker context; worker modules use the same resolution as code running in worker thread.
|
| 88 |
+
|
| 89 |
+
## Minimal timeline
|
| 90 |
+
|
| 91 |
+
- Implement the new file and wrapper: 20–45 minutes.
|
| 92 |
+
- Smoke test and fix small issues: 10–20 minutes.
|
| 93 |
+
- Optional unit test: 10–20 minutes.
|
| 94 |
+
|
| 95 |
+
## Rollout steps
|
| 96 |
+
|
| 97 |
+
1. Implement `src/worker/actions/list-chat-models.js` and wire wrapper in `boot-worker.js`.
|
| 98 |
+
2. Run smoke test in browser; fix any message shape mismatches.
|
| 99 |
+
3. Fix main-thread duplicate send and re-test.
|
| 100 |
+
4. Add one small unit test for iterator.abort cleanup.
|
| 101 |
+
|
| 102 |
+
---
|
| 103 |
+
|
| 104 |
+
This plan keeps the code minimal, uses language-native iterator cancellation, isolates side-effects to the generator, and leaves the worker wrapper trivial. Proceed to implement when ready.
|
| 105 |
+
|