From b4b1bbfc7b389fb3f1821e86719aa2a3290de1e0 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:21:24 +0200 Subject: [PATCH 01/10] test(worker-bus): export v2 duckdb/inference handlers + parity gate tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither workers/v2/duckdb.worker.ts's handlers nor workers/v2/inference.worker.ts's handleInference were exported or unit-tested against real logic (only webllm.worker.ts had that, tests/unit/webllmWorkerHandler.test.ts) despite both already being registered as live WorkerBus v2 pools with zero production callers — docs/adr/0014-worker-generation-duplication.md's deferred migration needs these to actually match v1 before any consumer cuts over. Testing surfaced a real gap: initDuckDb()'s OPFS-unavailable catch block silently swallowed the failure, unlike v1's out-of-band OPFS_FALLBACK message that lets the UI warn users their analytics won't persist. Fixed by threading ctx.emitProgress into initDuckDb() and emitting an 'opfs-fallback' progress stage from the catch block — reuses the existing progress channel WebLLM already uses for download progress, no new message-protocol/schema change needed. First PR of the ADR-0014 consolidation sprint (5 PRs total, see .claude/plans/worker-generation-v1-vs-enumerated-fox.md). Co-Authored-By: Claude Sonnet 5 --- tests/unit/duckdbWorkerHandler.test.ts | 233 ++++++++++++++++++++ tests/unit/inferenceWorkerHandlerV2.test.ts | 174 +++++++++++++++ workers/v2/duckdb.worker.ts | 35 ++- workers/v2/inference.worker.ts | 4 +- 4 files changed, 436 insertions(+), 10 deletions(-) create mode 100644 tests/unit/duckdbWorkerHandler.test.ts create mode 100644 tests/unit/inferenceWorkerHandlerV2.test.ts diff --git a/tests/unit/duckdbWorkerHandler.test.ts b/tests/unit/duckdbWorkerHandler.test.ts new file mode 100644 index 00000000..c1e0f10e --- /dev/null +++ b/tests/unit/duckdbWorkerHandler.test.ts @@ -0,0 +1,233 @@ +// @vitest-environment jsdom +// QNBS-v3: parity gate (PR 0, worker-generation consolidation, docs/adr/0014-worker-generation- +// duplication.md) — unit tests for the v2 DuckDB worker's handler functions, exercised +// directly (not via a real worker/WorkerBus) against a mocked @duckdb/duckdb-wasm, to +// catch response-shape / OPFS-fallback-signal drift from v1 before any consumer cuts over. + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { WorkerHandlerContext } from '../../packages/worker-bus/src/workerBootstrap'; + +class MockWorker { + postMessage = vi.fn(); + terminate = vi.fn(); + addEventListener = vi.fn(); +} +vi.stubGlobal('Worker', MockWorker); + +// QNBS-v3: vi.hoisted — the worker module is imported statically below, so the mock factory runs +// during the hoisted import phase; plain consts would hit a TDZ ReferenceError. +const { + mockSelectBundle, + mockInstantiate, + mockRegisterFileHandle, + mockConnect, + mockDbTerminate, + MockAsyncDuckDB, +} = vi.hoisted(() => { + const mockInstantiate = vi.fn(); + const mockRegisterFileHandle = vi.fn(); + const mockConnect = vi.fn(); + const mockDbTerminate = vi.fn(); + // QNBS-v3: must be a `function`, not an arrow — arrow functions aren't constructable, and + // `initDuckDb()` calls `new AsyncDuckDB(...)`. + const MockAsyncDuckDB = vi.fn().mockImplementation(function MockAsyncDuckDBCtor() { + return { + instantiate: mockInstantiate, + registerFileHandle: mockRegisterFileHandle, + connect: mockConnect, + terminate: mockDbTerminate, + }; + }); + return { + mockSelectBundle: vi.fn(), + mockInstantiate, + mockRegisterFileHandle, + mockConnect, + mockDbTerminate, + MockAsyncDuckDB, + }; +}); + +vi.mock('@duckdb/duckdb-wasm', () => ({ + AsyncDuckDB: MockAsyncDuckDB, + selectBundle: mockSelectBundle, + ConsoleLogger: vi.fn(), + DuckDBDataProtocol: { BROWSER_FSACCESS: 'browser-fsaccess' }, +})); + +// QNBS-v3: imported AFTER the mock is declared so the worker picks up the mocked module. +import { + handleExec, + handleQuery, + handleShutdown, + initDuckDb, +} from '../../workers/v2/duckdb.worker'; + +function makeCtx(overrides: Partial = {}): WorkerHandlerContext { + return { + taskId: 't1', + taskType: 'db.duckdb.query', + payload: {}, + signal: new AbortController().signal, + emitProgress: vi.fn(), + ...overrides, + }; +} + +function stubOpfsDirectory(getFileHandle: ReturnType): void { + Object.defineProperty(globalThis.navigator, 'storage', { + value: { getDirectory: vi.fn().mockResolvedValue({ getFileHandle }) }, + configurable: true, + }); +} + +function stubNoOpfs(): void { + Object.defineProperty(globalThis.navigator, 'storage', { + value: undefined, + configurable: true, + }); +} + +// QNBS-v3: always include `close` — the shared beforeEach calls the real handleShutdown() to +// reset module state, which unconditionally calls `connection?.close()`. +function mockConnection(overrides: { query?: ReturnType } = {}) { + return { query: overrides.query ?? vi.fn(), close: vi.fn().mockResolvedValue(undefined) }; +} + +describe('duckdb.worker handlers', () => { + beforeEach(async () => { + // QNBS-v3: connection/db are module-scoped in duckdb.worker.ts and persist across tests in + // this file — reset via handleShutdown() before each test so "not initialized" + // assertions aren't polluted by a previous test's initDuckDb() call. + await handleShutdown(); + vi.clearAllMocks(); + mockSelectBundle.mockResolvedValue({ mainModule: 'mvp.wasm', mainWorker: 'mvp.worker.js' }); + mockInstantiate.mockResolvedValue(undefined); + }); + + afterEach(() => { + stubNoOpfs(); + }); + + describe('initDuckDb', () => { + it('attaches OPFS persistence and does not emit opfs-fallback when it succeeds', async () => { + const connectionQuery = vi.fn().mockResolvedValue(undefined); + mockConnect.mockResolvedValue(mockConnection({ query: connectionQuery })); + mockRegisterFileHandle.mockResolvedValue(undefined); + stubOpfsDirectory(vi.fn().mockResolvedValue({})); + const emitProgress = vi.fn(); + + await initDuckDb(emitProgress); + + expect(connectionQuery).toHaveBeenCalledWith(expect.stringContaining('ATTACH')); + expect(emitProgress).not.toHaveBeenCalled(); + }); + + it('emits opfs-fallback and still connects when registerFileHandle throws', async () => { + mockConnect.mockResolvedValue(mockConnection()); + mockRegisterFileHandle.mockRejectedValue(new Error('registerFileHandle denied')); + stubOpfsDirectory(vi.fn().mockResolvedValue({})); + const emitProgress = vi.fn(); + + await initDuckDb(emitProgress); + + expect(emitProgress).toHaveBeenCalledWith('opfs-fallback', 1, 'registerFileHandle denied'); + // Falls back to a plain (in-memory) connect() despite the OPFS failure. + expect(mockConnect).toHaveBeenCalledTimes(1); + }); + + it('does not emit opfs-fallback when OPFS is simply unsupported (no navigator.storage)', async () => { + mockConnect.mockResolvedValue(mockConnection()); + stubNoOpfs(); + const emitProgress = vi.fn(); + + await initDuckDb(emitProgress); + + expect(emitProgress).not.toHaveBeenCalled(); + expect(mockConnect).toHaveBeenCalledTimes(1); + }); + + it('works when emitProgress is omitted (registerTaskHandler always passes ctx.emitProgress, but keep the param optional/safe)', async () => { + mockConnect.mockResolvedValue( + mockConnection({ query: vi.fn().mockRejectedValue(new Error('boom')) }), + ); + mockRegisterFileHandle.mockResolvedValue(undefined); + stubOpfsDirectory(vi.fn().mockResolvedValue({})); + + await expect(initDuckDb()).resolves.toBeUndefined(); + }); + }); + + describe('handleQuery', () => { + it('throws when DuckDB is not initialized', async () => { + await expect(handleQuery(makeCtx({ payload: { sql: 'select 1' } }))).rejects.toThrow( + 'DuckDB not initialized', + ); + }); + + it('returns a raw array of plain-object rows (not a {ok,rows} wrapper)', async () => { + const row = { toJSON: () => ({ id: 1 }) }; + mockConnect.mockResolvedValue( + mockConnection({ query: vi.fn().mockResolvedValue({ toArray: () => [row] }) }), + ); + stubNoOpfs(); + await initDuckDb(); + + const result = await handleQuery(makeCtx({ payload: { sql: 'select 1' } })); + + expect(result).toEqual([{ id: 1 }]); + }); + + it('throws (does not resolve {ok:false}) when the query rejects', async () => { + mockConnect.mockResolvedValue( + mockConnection({ query: vi.fn().mockRejectedValue(new Error('bad sql')) }), + ); + stubNoOpfs(); + await initDuckDb(); + + await expect(handleQuery(makeCtx({ payload: { sql: 'garbage' } }))).rejects.toThrow( + 'bad sql', + ); + }); + }); + + describe('handleExec', () => { + it('throws when DuckDB is not initialized', async () => { + await expect( + handleExec(makeCtx({ payload: { sql: 'insert into t values (1)' } })), + ).rejects.toThrow('DuckDB not initialized'); + }); + + it('returns {ok:true} on success', async () => { + const connectionQuery = vi.fn().mockResolvedValue(undefined); + mockConnect.mockResolvedValue(mockConnection({ query: connectionQuery })); + stubNoOpfs(); + await initDuckDb(); + + const result = await handleExec(makeCtx({ payload: { sql: 'create table t (id int)' } })); + + expect(result).toEqual({ ok: true }); + expect(connectionQuery).toHaveBeenCalledWith('create table t (id int)'); + }); + }); + + describe('handleShutdown', () => { + it('closes the connection and terminates the db, returning {ok:true}', async () => { + const close = vi.fn().mockResolvedValue(undefined); + mockConnect.mockResolvedValue({ query: vi.fn(), close }); + stubNoOpfs(); + await initDuckDb(); + + const result = await handleShutdown(); + + expect(result).toEqual({ ok: true }); + expect(close).toHaveBeenCalled(); + expect(mockDbTerminate).toHaveBeenCalled(); + + // A query after shutdown must throw 'DuckDB not initialized' again. + await expect(handleQuery(makeCtx({ payload: { sql: 'select 1' } }))).rejects.toThrow( + 'DuckDB not initialized', + ); + }); + }); +}); diff --git a/tests/unit/inferenceWorkerHandlerV2.test.ts b/tests/unit/inferenceWorkerHandlerV2.test.ts new file mode 100644 index 00000000..aadb8001 --- /dev/null +++ b/tests/unit/inferenceWorkerHandlerV2.test.ts @@ -0,0 +1,174 @@ +// @vitest-environment jsdom +// QNBS-v3: parity gate (PR 0, worker-generation consolidation, docs/adr/0014-worker-generation- +// duplication.md) — unit tests for the v2 inference worker's handler, exercised directly +// (not via a real worker/WorkerBus) against a mocked @huggingface/transformers, to catch +// drift from v1 (workers/inference.worker.ts) before any consumer cuts over. File name is +// distinct from the existing v1 tests/unit/inferenceWorker.test.ts until v1 is deleted. + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { WorkerHandlerContext } from '../../packages/worker-bus/src/workerBootstrap'; + +const { mockPipelineFactory } = vi.hoisted(() => ({ mockPipelineFactory: vi.fn() })); + +vi.mock('@huggingface/transformers', () => ({ + pipeline: mockPipelineFactory, +})); + +// QNBS-v3: imported AFTER the mock is declared so the worker picks up the mocked factory. +import { handleInference } from '../../workers/v2/inference.worker'; + +function makeCtx(overrides: Partial = {}): WorkerHandlerContext { + return { + taskId: 't1', + taskType: 'inference.embed', + payload: { task: 'feature-extraction', modelId: 'm', input: 'hello world' }, + signal: new AbortController().signal, + emitProgress: vi.fn(), + ...overrides, + }; +} + +describe('inference.worker (v2) handleInference', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('throws Aborted when the signal is already aborted', async () => { + const controller = new AbortController(); + controller.abort(); + await expect(handleInference(makeCtx({ signal: controller.signal }))).rejects.toThrow( + 'Aborted', + ); + expect(mockPipelineFactory).not.toHaveBeenCalled(); + }); + + it('throws Aborted when the signal aborts between model load and inference', async () => { + const controller = new AbortController(); + const pipe = vi.fn(); + mockPipelineFactory.mockImplementation(async () => { + controller.abort(); + return pipe; + }); + + await expect( + handleInference( + makeCtx({ + signal: controller.signal, + payload: { task: 'feature-extraction', modelId: 'abort-model', input: 'x' }, + }), + ), + ).rejects.toThrow('Aborted'); + expect(pipe).not.toHaveBeenCalled(); + }); + + it('feature-extraction: normalizes a Float32Array result to a flat number[]', async () => { + const pipe = vi.fn().mockResolvedValue(new Float32Array([0.1, 0.2, 0.3])); + mockPipelineFactory.mockResolvedValue(pipe); + const emitProgress = vi.fn(); + + const result = await handleInference( + makeCtx({ + payload: { task: 'feature-extraction', modelId: 'embed-1', input: 'x' }, + emitProgress, + }), + ); + + expect(result).toEqual([Math.fround(0.1), Math.fround(0.2), Math.fround(0.3)]); + expect(pipe).toHaveBeenCalledWith( + 'x', + expect.objectContaining({ pooling: 'mean', normalize: true }), + ); + expect(emitProgress).toHaveBeenNthCalledWith(1, 'loading', 0.2, 'Loading model'); + expect(emitProgress).toHaveBeenNthCalledWith(2, 'inference', 0.6, 'Running inference'); + expect(emitProgress).toHaveBeenNthCalledWith(3, 'done', 1.0, 'Complete'); + }); + + it('feature-extraction: normalizes a {data: Float32Array} result to a flat number[]', async () => { + const pipe = vi.fn().mockResolvedValue({ data: new Float32Array([1, 2]) }); + mockPipelineFactory.mockResolvedValue(pipe); + + const result = await handleInference( + makeCtx({ payload: { task: 'feature-extraction', modelId: 'embed-2', input: 'x' } }), + ); + + expect(result).toEqual([1, 2]); + }); + + it('feature-extraction: normalizes a plain iterable result to a flat number[]', async () => { + const pipe = vi.fn().mockResolvedValue([4, 5, 6]); + mockPipelineFactory.mockResolvedValue(pipe); + + const result = await handleInference( + makeCtx({ payload: { task: 'feature-extraction', modelId: 'embed-3', input: 'x' } }), + ); + + expect(result).toEqual([4, 5, 6]); + }); + + it('sentiment-analysis: returns "LABEL:score" formatted to 4 decimals', async () => { + const pipe = vi.fn().mockResolvedValue([{ label: 'POSITIVE', score: 0.5 }]); + mockPipelineFactory.mockResolvedValue(pipe); + + const result = await handleInference( + makeCtx({ payload: { task: 'sentiment-analysis', modelId: 'sent-1', input: 'great!' } }), + ); + + expect(result).toBe('POSITIVE:0.5000'); + expect(pipe).toHaveBeenCalledWith('great!', {}); + }); + + it('sentiment-analysis: defaults to NEUTRAL:0.0000 when the pipeline result is empty', async () => { + const pipe = vi.fn().mockResolvedValue([]); + mockPipelineFactory.mockResolvedValue(pipe); + + const result = await handleInference( + makeCtx({ payload: { task: 'sentiment-analysis', modelId: 'sent-2', input: 'x' } }), + ); + + expect(result).toBe('NEUTRAL:0.0000'); + }); + + it('summarization: returns summary_text', async () => { + const pipe = vi.fn().mockResolvedValue([{ summary_text: 'a short summary' }]); + mockPipelineFactory.mockResolvedValue(pipe); + + const result = await handleInference( + makeCtx({ payload: { task: 'summarization', modelId: 'sum-1', input: 'long text...' } }), + ); + + expect(result).toBe('a short summary'); + }); + + it('text-generation: returns generated_text', async () => { + const pipe = vi.fn().mockResolvedValue([{ generated_text: 'once upon a time' }]); + mockPipelineFactory.mockResolvedValue(pipe); + + const result = await handleInference( + makeCtx({ payload: { task: 'text-generation', modelId: 'gen-1', input: 'prompt' } }), + ); + + expect(result).toBe('once upon a time'); + }); + + it('passes inferenceOptions through to the pipeline call for non-embedding tasks', async () => { + const pipe = vi.fn().mockResolvedValue([{ generated_text: 'x' }]); + mockPipelineFactory.mockResolvedValue(pipe); + + await handleInference( + makeCtx({ + payload: { + task: 'text-generation', + modelId: 'gen-2', + input: 'prompt', + inferenceOptions: { max_new_tokens: 32 }, + }, + }), + ); + + expect(pipe).toHaveBeenCalledWith('prompt', { max_new_tokens: 32 }); + }); +}); diff --git a/workers/v2/duckdb.worker.ts b/workers/v2/duckdb.worker.ts index 228e384e..2743ce7c 100644 --- a/workers/v2/duckdb.worker.ts +++ b/workers/v2/duckdb.worker.ts @@ -42,7 +42,11 @@ const SELF_HOSTED_BUNDLES = { }, }; -async function initDuckDb(): Promise { +// QNBS-v3: exported for tests/unit/duckdbWorkerHandler.test.ts (parity gate, PR 0 of the +// worker-generation consolidation) — verifies real handler logic without a live worker. +export async function initDuckDb( + emitProgress?: (stage: string, progress: number, message?: string) => void, +): Promise { const { AsyncDuckDB, selectBundle, ConsoleLogger } = await getDuckDb(); const bundle = await selectBundle(SELF_HOSTED_BUNDLES); @@ -68,7 +72,16 @@ async function initDuckDb(): Promise { ); connection = await newDb.connect(); await connection.query("ATTACH 'worldscript_analytics.duckdb' AS analytics (TYPE duckdb)"); - } catch { + } catch (opfsErr) { + // QNBS-v3: mirrors legacy workers/duckdbWorker.ts's out-of-band OPFS_FALLBACK message — + // v2 has no bare postMessage escape hatch from inside a task handler, so this + // reuses the existing progress channel instead (duckdbClient's INIT adapter + // listens for the 'opfs-fallback' stage and forwards it to setOpfsFallbackHandler). + emitProgress?.( + 'opfs-fallback', + 1, + opfsErr instanceof Error ? opfsErr.message : String(opfsErr), + ); connection = await newDb.connect(); } } else { @@ -78,7 +91,7 @@ async function initDuckDb(): Promise { db = newDb; } -async function handleQuery(ctx: WorkerHandlerContext): Promise { +export async function handleQuery(ctx: WorkerHandlerContext): Promise { const { payload } = ctx; const req = payload as { sql?: string }; if (!connection) throw new Error('DuckDB not initialized'); @@ -86,7 +99,7 @@ async function handleQuery(ctx: WorkerHandlerContext): Promise { return result.toArray().map((row: { toJSON(): Record }) => row.toJSON()); } -async function handleExec(ctx: WorkerHandlerContext): Promise { +export async function handleExec(ctx: WorkerHandlerContext): Promise { const { payload } = ctx; const req = payload as { sql?: string }; if (!connection) throw new Error('DuckDB not initialized'); @@ -94,7 +107,7 @@ async function handleExec(ctx: WorkerHandlerContext): Promise { return { ok: true }; } -async function handleShutdown(): Promise { +export async function handleShutdown(): Promise { await connection?.close(); await db?.terminate(); connection = null; @@ -102,10 +115,14 @@ async function handleShutdown(): Promise { return { ok: true }; } -registerTaskHandler('db.duckdb.init', async () => { - await initDuckDb(); - return { ok: true }; -}, ['db.duckdb']); +registerTaskHandler( + 'db.duckdb.init', + async (ctx) => { + await initDuckDb(ctx.emitProgress); + return { ok: true }; + }, + ['db.duckdb'], +); registerTaskHandler('db.duckdb.query', handleQuery, ['db.duckdb']); registerTaskHandler('db.duckdb.exec', handleExec, ['db.duckdb']); diff --git a/workers/v2/inference.worker.ts b/workers/v2/inference.worker.ts index 261eb717..a0eb7e3a 100644 --- a/workers/v2/inference.worker.ts +++ b/workers/v2/inference.worker.ts @@ -42,7 +42,9 @@ async function loadPipeline(task: string, modelId: string, quantized = true) { }); } -async function handleInference(ctx: WorkerHandlerContext): Promise { +// QNBS-v3: exported for tests/unit/inferenceWorkerHandlerV2.test.ts (parity gate, PR 0 of the +// worker-generation consolidation) — verifies real handler logic without a live worker. +export async function handleInference(ctx: WorkerHandlerContext): Promise { const { payload, signal, emitProgress } = ctx; const req = payload as { task: string; From bcc41c2757ba6fd9ba6e1d10d3a7fb3dddf7a47f Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:44:35 +0200 Subject: [PATCH 02/10] fix(worker-bus): close leaked OPFS connection on ATTACH failure + QNBS-v3 format CodeRabbit review on #286 found a real bug: initDuckDb()'s OPFS catch block overwrote `connection` with the fallback connection without closing the first (partially-attached) one, leaking a DuckDB connection on every OPFS-attach failure. Fixed by tracking the OPFS attempt in its own local variable and closing it in the catch path before falling back. New regression test asserts close() is called. Also collapsed several multi-line QNBS-v3 comments in workers/v2/duckdb.worker.ts, workers/v2/inference.worker.ts, and both new test files to the required single bracketed-line format. Co-Authored-By: Claude Sonnet 5 --- tests/unit/duckdbWorkerHandler.test.ts | 37 +++++++++++++-------- tests/unit/inferenceWorkerHandlerV2.test.ts | 8 ++--- workers/v2/duckdb.worker.ts | 18 +++++----- workers/v2/inference.worker.ts | 3 +- 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/tests/unit/duckdbWorkerHandler.test.ts b/tests/unit/duckdbWorkerHandler.test.ts index c1e0f10e..5ebe5fa3 100644 --- a/tests/unit/duckdbWorkerHandler.test.ts +++ b/tests/unit/duckdbWorkerHandler.test.ts @@ -1,8 +1,5 @@ // @vitest-environment jsdom -// QNBS-v3: parity gate (PR 0, worker-generation consolidation, docs/adr/0014-worker-generation- -// duplication.md) — unit tests for the v2 DuckDB worker's handler functions, exercised -// directly (not via a real worker/WorkerBus) against a mocked @duckdb/duckdb-wasm, to -// catch response-shape / OPFS-fallback-signal drift from v1 before any consumer cuts over. +// QNBS-v3: [Parity gate for the v2 DuckDB worker handlers (docs/adr/0014) — catches response-shape/OPFS-fallback drift from v1 before any consumer cuts over.] import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { WorkerHandlerContext } from '../../packages/worker-bus/src/workerBootstrap'; @@ -14,8 +11,7 @@ class MockWorker { } vi.stubGlobal('Worker', MockWorker); -// QNBS-v3: vi.hoisted — the worker module is imported statically below, so the mock factory runs -// during the hoisted import phase; plain consts would hit a TDZ ReferenceError. +// QNBS-v3: [vi.hoisted — the mock factory runs during the hoisted import phase, so plain consts would hit a TDZ ReferenceError.] const { mockSelectBundle, mockInstantiate, @@ -28,8 +24,7 @@ const { const mockRegisterFileHandle = vi.fn(); const mockConnect = vi.fn(); const mockDbTerminate = vi.fn(); - // QNBS-v3: must be a `function`, not an arrow — arrow functions aren't constructable, and - // `initDuckDb()` calls `new AsyncDuckDB(...)`. + // QNBS-v3: [Must be a `function`, not an arrow — arrow functions aren't constructable, and initDuckDb() calls `new AsyncDuckDB(...)`.] const MockAsyncDuckDB = vi.fn().mockImplementation(function MockAsyncDuckDBCtor() { return { instantiate: mockInstantiate, @@ -55,7 +50,7 @@ vi.mock('@duckdb/duckdb-wasm', () => ({ DuckDBDataProtocol: { BROWSER_FSACCESS: 'browser-fsaccess' }, })); -// QNBS-v3: imported AFTER the mock is declared so the worker picks up the mocked module. +// QNBS-v3: [Imported after the mock is declared so the worker picks up the mocked module.] import { handleExec, handleQuery, @@ -88,17 +83,14 @@ function stubNoOpfs(): void { }); } -// QNBS-v3: always include `close` — the shared beforeEach calls the real handleShutdown() to -// reset module state, which unconditionally calls `connection?.close()`. +// QNBS-v3: [Always include `close` — the shared beforeEach calls handleShutdown() to reset state, which unconditionally calls `connection?.close()`.] function mockConnection(overrides: { query?: ReturnType } = {}) { return { query: overrides.query ?? vi.fn(), close: vi.fn().mockResolvedValue(undefined) }; } describe('duckdb.worker handlers', () => { beforeEach(async () => { - // QNBS-v3: connection/db are module-scoped in duckdb.worker.ts and persist across tests in - // this file — reset via handleShutdown() before each test so "not initialized" - // assertions aren't polluted by a previous test's initDuckDb() call. + // QNBS-v3: [connection/db are module-scoped and persist across tests — reset first so "not initialized" assertions aren't polluted by a prior test's initDuckDb().] await handleShutdown(); vi.clearAllMocks(); mockSelectBundle.mockResolvedValue({ mainModule: 'mvp.wasm', mainWorker: 'mvp.worker.js' }); @@ -136,6 +128,23 @@ describe('duckdb.worker handlers', () => { expect(mockConnect).toHaveBeenCalledTimes(1); }); + it('closes the partially-initialized OPFS connection before falling back when ATTACH fails', async () => { + const opfsClose = vi.fn().mockResolvedValue(undefined); + const opfsQuery = vi.fn().mockRejectedValue(new Error('ATTACH failed')); + mockConnect + .mockResolvedValueOnce({ query: opfsQuery, close: opfsClose }) + .mockResolvedValueOnce(mockConnection()); + mockRegisterFileHandle.mockResolvedValue(undefined); + stubOpfsDirectory(vi.fn().mockResolvedValue({})); + const emitProgress = vi.fn(); + + await initDuckDb(emitProgress); + + expect(opfsClose).toHaveBeenCalled(); + expect(emitProgress).toHaveBeenCalledWith('opfs-fallback', 1, 'ATTACH failed'); + expect(mockConnect).toHaveBeenCalledTimes(2); + }); + it('does not emit opfs-fallback when OPFS is simply unsupported (no navigator.storage)', async () => { mockConnect.mockResolvedValue(mockConnection()); stubNoOpfs(); diff --git a/tests/unit/inferenceWorkerHandlerV2.test.ts b/tests/unit/inferenceWorkerHandlerV2.test.ts index aadb8001..07f16091 100644 --- a/tests/unit/inferenceWorkerHandlerV2.test.ts +++ b/tests/unit/inferenceWorkerHandlerV2.test.ts @@ -1,9 +1,5 @@ // @vitest-environment jsdom -// QNBS-v3: parity gate (PR 0, worker-generation consolidation, docs/adr/0014-worker-generation- -// duplication.md) — unit tests for the v2 inference worker's handler, exercised directly -// (not via a real worker/WorkerBus) against a mocked @huggingface/transformers, to catch -// drift from v1 (workers/inference.worker.ts) before any consumer cuts over. File name is -// distinct from the existing v1 tests/unit/inferenceWorker.test.ts until v1 is deleted. +// QNBS-v3: [Parity gate for the v2 inference worker handler (docs/adr/0014) — catches drift from v1 before any consumer cuts over; named distinctly from tests/unit/inferenceWorker.test.ts until v1 is deleted.] import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { WorkerHandlerContext } from '../../packages/worker-bus/src/workerBootstrap'; @@ -14,7 +10,7 @@ vi.mock('@huggingface/transformers', () => ({ pipeline: mockPipelineFactory, })); -// QNBS-v3: imported AFTER the mock is declared so the worker picks up the mocked factory. +// QNBS-v3: [Imported after the mock is declared so the worker picks up the mocked factory.] import { handleInference } from '../../workers/v2/inference.worker'; function makeCtx(overrides: Partial = {}): WorkerHandlerContext { diff --git a/workers/v2/duckdb.worker.ts b/workers/v2/duckdb.worker.ts index 2743ce7c..891886ea 100644 --- a/workers/v2/duckdb.worker.ts +++ b/workers/v2/duckdb.worker.ts @@ -42,8 +42,7 @@ const SELF_HOSTED_BUNDLES = { }, }; -// QNBS-v3: exported for tests/unit/duckdbWorkerHandler.test.ts (parity gate, PR 0 of the -// worker-generation consolidation) — verifies real handler logic without a live worker. +// QNBS-v3: [Exported for tests/unit/duckdbWorkerHandler.test.ts to verify real handler logic without a live worker.] export async function initDuckDb( emitProgress?: (stage: string, progress: number, message?: string) => void, ): Promise { @@ -58,6 +57,8 @@ export async function initDuckDb( const useOpfs = await isOPFSSupported(); if (useOpfs) { + // QNBS-v3: [Tracked separately from `connection` so a failed ATTACH can close this partial connection instead of leaking it.] + let opfsConnection: import('@duckdb/duckdb-wasm').AsyncDuckDBConnection | null = null; try { const { DuckDBDataProtocol } = await getDuckDb(); const opfsRoot = await navigator.storage.getDirectory(); @@ -70,13 +71,14 @@ export async function initDuckDb( DuckDBDataProtocol.BROWSER_FSACCESS, true, ); - connection = await newDb.connect(); - await connection.query("ATTACH 'worldscript_analytics.duckdb' AS analytics (TYPE duckdb)"); + opfsConnection = await newDb.connect(); + await opfsConnection.query( + "ATTACH 'worldscript_analytics.duckdb' AS analytics (TYPE duckdb)", + ); + connection = opfsConnection; } catch (opfsErr) { - // QNBS-v3: mirrors legacy workers/duckdbWorker.ts's out-of-band OPFS_FALLBACK message — - // v2 has no bare postMessage escape hatch from inside a task handler, so this - // reuses the existing progress channel instead (duckdbClient's INIT adapter - // listens for the 'opfs-fallback' stage and forwards it to setOpfsFallbackHandler). + // QNBS-v3: [No bare postMessage from inside a task handler — OPFS-unavailable is surfaced via the progress channel duckdbClient's INIT adapter listens on instead.] + await opfsConnection?.close(); emitProgress?.( 'opfs-fallback', 1, diff --git a/workers/v2/inference.worker.ts b/workers/v2/inference.worker.ts index a0eb7e3a..88633733 100644 --- a/workers/v2/inference.worker.ts +++ b/workers/v2/inference.worker.ts @@ -42,8 +42,7 @@ async function loadPipeline(task: string, modelId: string, quantized = true) { }); } -// QNBS-v3: exported for tests/unit/inferenceWorkerHandlerV2.test.ts (parity gate, PR 0 of the -// worker-generation consolidation) — verifies real handler logic without a live worker. +// QNBS-v3: [Exported for tests/unit/inferenceWorkerHandlerV2.test.ts to verify real handler logic without a live worker.] export async function handleInference(ctx: WorkerHandlerContext): Promise { const { payload, signal, emitProgress } = ctx; const req = payload as { From bc371b36e9890d880d2b77ab42237776cc32bd24 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:59:57 +0200 Subject: [PATCH 03/10] feat(duckdb): migrate duckdbClient onto WorkerBus v2, delete v1 worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication (ADR-0014). services/duckdb/duckdbClient.ts's public API (init/query/ exec/shutdown/terminate/setOpfsFallbackHandler) is unchanged — its 5 real consumers (useDuckDb.ts, duckdbAnalytics.ts, duckdbMigration.ts, ragVectorMigration.ts, telemetryService.ts) need no changes. Only the internals swapped from a raw `new Worker(duckdbWorker.ts)` to routing through the shared WorkerBus v2 'duckdb' pool via a new ensureDuckDbPool() (mirrors ensureWebLlmPool()'s decoupling from the enableWorkerBusV2 flag — analytics is core, not experimental infra). The v2 worker returns raw values and throws on failure, unlike v1's {ok,rows,error} wrapper — the adapter in duckdbClient.ts's send() translates at that one boundary instead of touching ~20 call sites across 5 files. WorkerBus's default 2-retry is disabled per-call (retryPolicy: {maxRetries:0}) to avoid stacking under duckdbAnalytics.ts's existing withDuckDbRetry() app-level retry. Two things surfaced while wiring this up, both fixed here: - WorkerBus.shutdown() tears down all 4 pools with no way to scope to just 'duckdb' — added WorkerBus.terminatePool(poolId) so duckdbClient.terminate() doesn't kill in-flight inference/webllm/ plugin work on the shared bus. - send() is async and awaits ensureDuckDbPool() before a task handle exists; naively attaching the AbortSignal listener after that await (like a first draft did) can lose a same-tick abort, unlike v1's synchronous Promise-executor version. Fixed by attaching the listener synchronously up front and applying it once the handle is ready — regression-tested. workers/duckdbWorker.ts deleted; features/featureCatalog.ts's enableDuckDbAnalytics entry retargeted to workers/v2/duckdb.worker.ts. Co-Authored-By: Claude Sonnet 5 --- features/featureCatalog.ts | 2 +- packages/worker-bus/src/workerBus.ts | 15 ++ packages/worker-bus/tests/workerBus.test.ts | 24 ++ services/duckdb/duckdbClient.ts | 157 +++++++----- services/workerBusManager.ts | 12 + tests/unit/duckdbClient.test.ts | 264 ++++++++++++++------ workers/duckdbWorker.ts | 219 ---------------- 7 files changed, 332 insertions(+), 361 deletions(-) delete mode 100644 workers/duckdbWorker.ts diff --git a/features/featureCatalog.ts b/features/featureCatalog.ts index 52933b29..d78b9485 100644 --- a/features/featureCatalog.ts +++ b/features/featureCatalog.ts @@ -430,7 +430,7 @@ const RAW_FEATURE_CATALOG: CatalogEntryInput[] = [ description: 'Conditionally shows DuckDB query tab', }, ], - implementedIn: ['services/duckdb/', 'workers/duckdbWorker.ts', 'hooks/useDuckDb.ts'], + implementedIn: ['services/duckdb/', 'workers/v2/duckdb.worker.ts', 'hooks/useDuckDb.ts'], drifts: [], }, diff --git a/packages/worker-bus/src/workerBus.ts b/packages/worker-bus/src/workerBus.ts index b99bb73e..84b45627 100644 --- a/packages/worker-bus/src/workerBus.ts +++ b/packages/worker-bus/src/workerBus.ts @@ -195,6 +195,21 @@ export class WorkerBus { this.progress.clear(); } + /** + * Terminate all workers in a single named pool without shutting down the rest of the bus. + * QNBS-v3: unlike shutdown() (tears down every pool), this only affects `poolId` — added so + * duckdbClient's terminate()/shutdown() can stop the DuckDB worker without also killing + * in-flight inference/webllm/plugin work on the shared bus (docs/adr/0014-worker-generation- + * duplication.md's migration follow-up). The pool is removed from the registry; a later task + * routed to its capabilities fails with NO_POOL until the bus re-registers it. + */ + async terminatePool(poolId: string): Promise { + const pool = this.pools.get(poolId); + if (!pool) return; + await pool.terminateAll(); + this.pools.delete(poolId); + } + // --------------------------------------------------------------------------- // Internal // --------------------------------------------------------------------------- diff --git a/packages/worker-bus/tests/workerBus.test.ts b/packages/worker-bus/tests/workerBus.test.ts index 45f494ba..74ae557c 100644 --- a/packages/worker-bus/tests/workerBus.test.ts +++ b/packages/worker-bus/tests/workerBus.test.ts @@ -209,6 +209,30 @@ describe('WorkerBus', () => { expect(pool).toBeUndefined(); }); + it('terminatePool terminates and removes only the named pool, leaving others intact', async () => { + bus.registerPool('other', ['db.duckdb'], { + maxWorkers: 1, + minWorkers: 1, + idleTimeoutMs: 120_000, + workerScript: '/other.worker.js', + capabilities: ['db.duckdb'], + labels: {}, + }); + const pools = (bus as unknown as { pools: Map }).pools; + const terminateAllSpy = vi.spyOn(pools.get('fake')!, 'terminateAll'); + vi.spyOn(pools.get('other')!, 'acquire').mockReturnValue(new Promise(() => {})); + + await bus.terminatePool('fake'); + + expect(terminateAllSpy).toHaveBeenCalled(); + expect(pools.get('fake')).toBeUndefined(); + expect(pools.get('other')).toBeDefined(); + }); + + it('terminatePool is a no-op for an unknown pool id', async () => { + await expect(bus.terminatePool('does-not-exist')).resolves.toBeUndefined(); + }); + it('progress callback receives emitted progress', async () => { const progressUpdates: Array<{ stage: string; progress: number }> = []; let capturedTaskId = ''; diff --git a/services/duckdb/duckdbClient.ts b/services/duckdb/duckdbClient.ts index 5751f0d4..1315002f 100644 --- a/services/duckdb/duckdbClient.ts +++ b/services/duckdb/duckdbClient.ts @@ -1,80 +1,102 @@ -// QNBS-v3: Singleton proxy for the DuckDB-WASM worker. -// Exposes typed query/exec/init/shutdown helpers with AbortSignal cancellation. -// Worker is instantiated lazily on first call to init(). - -import type { - DuckDbRequest, - DuckDbRequestType, - DuckDbResponse, - DuckDbWorkerEvent, -} from '../../workers/duckdbWorker'; - -let worker: Worker | null = null; -const pendingResolvers = new Map void>(); -let messageIdCounter = 0; -// QNBS-v3: Settable by useDuckDb to surface OPFS fallback state in Redux. -let opfsFallbackCb: ((reason: string) => void) | null = null; +// QNBS-v3: Singleton proxy for DuckDB analytics — routes through the shared WorkerBus v2 'duckdb' +// pool (docs/adr/0014-worker-generation-duplication.md migration, superseded by +// ADR-0015). Public API is unchanged from the v1-backed client (init/query/exec/ +// shutdown/terminate/setOpfsFallbackHandler) so all 5 consumers (useDuckDb.ts, +// duckdbAnalytics.ts, duckdbMigration.ts, ragVectorMigration.ts, telemetryService.ts) +// need no changes — only the transport underneath swapped. + +import { ensureDuckDbPool, getWorkerBus } from '../workerBusManager'; + +export type DuckDbRequestType = 'INIT' | 'QUERY' | 'EXEC' | 'SHUTDOWN'; + +export interface DuckDbResponse { + messageId: string; + ok: boolean; + rows?: Record[]; + error?: string; + latencyMs?: number; +} + +const TASK_TYPE: Record = { + INIT: 'db.duckdb.init', + QUERY: 'db.duckdb.query', + EXEC: 'db.duckdb.exec', + SHUTDOWN: 'db.duckdb.shutdown', +}; +let messageIdCounter = 0; function generateMessageId(): string { return `duckdb-${Date.now()}-${++messageIdCounter}`; } -function getWorker(): Worker { - if (!worker) { - // QNBS-v3: Vite import.meta.url pattern — same as inference.worker.ts. - worker = new Worker(new URL('../../workers/duckdbWorker.ts', import.meta.url), { - type: 'module', - }); - worker.addEventListener('message', (event: MessageEvent) => { - const data = event.data as DuckDbResponse | DuckDbWorkerEvent; - // QNBS-v3: OPFS_FALLBACK is out-of-band — no messageId resolver to call. - if ((data as DuckDbWorkerEvent).type === 'OPFS_FALLBACK') { - opfsFallbackCb?.((data as DuckDbWorkerEvent).reason); - return; - } - const { messageId } = data as DuckDbResponse; - const resolve = pendingResolvers.get(messageId); - if (resolve) { - pendingResolvers.delete(messageId); - resolve(data as DuckDbResponse); - } - }); - worker.addEventListener('error', (event) => { - // Reject all pending promises on fatal worker error - const error = event.message ?? 'DuckDB worker crashed'; - for (const [id, resolve] of pendingResolvers) { - resolve({ messageId: id, ok: false, error }); - } - pendingResolvers.clear(); - worker = null; - }); - } - return worker; +// QNBS-v3: Settable by useDuckDb to surface OPFS fallback state in Redux. +let opfsFallbackCb: ((reason: string) => void) | null = null; + +function errorMessage(err: unknown): string { + const code = (err as { code?: string } | undefined)?.code; + if (code === 'CIRCUIT_OPEN') return 'DuckDB temporarily unavailable (circuit open)'; + return err instanceof Error ? err.message : String(err); } -function send( +async function send( type: DuckDbRequestType, sql?: string, params?: readonly unknown[], signal?: AbortSignal, ): Promise { const messageId = generateMessageId(); - const w = getWorker(); + const start = Date.now(); - return new Promise((resolve) => { - pendingResolvers.set(messageId, resolve); + // QNBS-v3: attach synchronously, before the `await` below — v1 set up its abort listener + // inside a synchronous Promise executor, so a caller aborting in the same tick as the + // call was always caught. This function is async and awaits ensureDuckDbPool() before + // a handle exists, which opens a gap; a same-tick abort must not be lost inside it. + let abortedEarly = signal?.aborted ?? false; + let handleRef: { cancel: (reason?: string) => void } | undefined; + signal?.addEventListener( + 'abort', + () => { + abortedEarly = true; + handleRef?.cancel('Aborted'); + }, + { once: true }, + ); - if (signal) { - signal.addEventListener('abort', () => { - w.postMessage({ type: 'WORKER_CANCEL', messageId }); - pendingResolvers.delete(messageId); - resolve({ messageId, ok: false, error: 'Aborted' }); - }); - } + const bus = await ensureDuckDbPool(); + if (!bus) return { messageId, ok: false, error: 'WorkerBus v2 unavailable' }; + + // QNBS-v3: v1 never retried a failed QUERY/EXEC (caller decided) — disabling the bus's default + // 2-retry here keeps that behavior instead of silently stacking under + // duckdbAnalytics.ts's own withDuckDbRetry() app-level retry (app/listenerMiddleware.ts). + const handle = bus.enqueue< + { sql?: string | undefined; params?: readonly unknown[] | undefined }, + unknown + >( + TASK_TYPE[type], + { sql, params }, + { + capabilities: ['db.duckdb'], + retryPolicy: { maxRetries: 0 }, + onProgress: (p) => { + if (p.stage === 'opfs-fallback') opfsFallbackCb?.(p.message ?? 'OPFS unavailable'); + }, + }, + ); + handleRef = handle; + if (abortedEarly) handle.cancel('Aborted'); - const req: DuckDbRequest = { messageId, type, sql, params }; - w.postMessage(req); - }); + try { + const raw = await handle.result; + const latencyMs = Date.now() - start; + if (type === 'QUERY') { + return { messageId, ok: true, rows: raw as Record[], latencyMs }; + } + return { messageId, ok: true, latencyMs }; + } catch (err) { + const latencyMs = Date.now() - start; + if (signal?.aborted) return { messageId, ok: false, error: 'Aborted', latencyMs }; + return { messageId, ok: false, error: errorMessage(err), latencyMs }; + } } export const duckdbClient = { @@ -98,14 +120,17 @@ export const duckdbClient = { return send('SHUTDOWN', undefined, undefined, signal); }, - /** Terminate the worker immediately (no flush). */ + /** + * Terminate the DuckDB pool immediately (no flush). + * QNBS-v3: scoped via WorkerBus.terminatePool('duckdb') so this doesn't tear down the shared + * bus's other pools (inference/webllm/plugin) — bus.shutdown() would kill all of them. + */ terminate(): void { - worker?.terminate(); - worker = null; - pendingResolvers.clear(); + const bus = getWorkerBus(); + void bus?.terminatePool('duckdb'); }, - /** Register a callback invoked when the worker falls back to in-memory (OPFS unavailable). */ + /** Register a callback invoked when DuckDB falls back to in-memory (OPFS unavailable). */ setOpfsFallbackHandler(cb: ((reason: string) => void) | null): void { opfsFallbackCb = cb; }, diff --git a/services/workerBusManager.ts b/services/workerBusManager.ts index eccf3f18..2e973c24 100644 --- a/services/workerBusManager.ts +++ b/services/workerBusManager.ts @@ -164,6 +164,18 @@ export async function ensureWebLlmPool(): Promise { return _bus; } +/** + * Ensure the DuckDB worker pool is available, initializing the WorkerBus on demand. + * QNBS-v3: mirrors ensureWebLlmPool()'s decoupling from `enableWorkerBusV2` — DuckDB analytics + * defaults on and was never gated by that flag in the v1 worker it replaces + * (docs/adr/0014-worker-generation-duplication.md), so toggling an experimental infra flag off + * must not silently break analytics. Returns null only if init failed. + */ +export async function ensureDuckDbPool(): Promise { + if (_bus === null) await initWorkerBus(); + return _bus; +} + /** * Shut down the WorkerBus v2 and terminate all worker threads. * Called when the feature flag is disabled or the app unmounts. diff --git a/tests/unit/duckdbClient.test.ts b/tests/unit/duckdbClient.test.ts index ee1726b4..c592b26c 100644 --- a/tests/unit/duckdbClient.test.ts +++ b/tests/unit/duckdbClient.test.ts @@ -1,130 +1,244 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -// QNBS-v3: duckdbClient instantiates a Worker via new URL(…, import.meta.url). -// We mock the Worker constructor so no real worker is spun up in jsdom. +// QNBS-v3: duckdbClient now routes through WorkerBus v2 (workers/v2/duckdb.worker.ts) instead of +// spawning a raw Worker directly — mock services/workerBusManager's ensureDuckDbPool()/ +// getWorkerBus() instead of the global Worker constructor. + +const { mockEnqueue, mockTerminatePool, mockEnsureDuckDbPool, mockGetWorkerBus } = vi.hoisted( + () => ({ + mockEnqueue: vi.fn(), + mockTerminatePool: vi.fn(), + mockEnsureDuckDbPool: vi.fn(), + mockGetWorkerBus: vi.fn(), + }), +); + +vi.mock('../../services/workerBusManager', () => ({ + ensureDuckDbPool: mockEnsureDuckDbPool, + getWorkerBus: mockGetWorkerBus, +})); -const mockPostMessage = vi.fn(); -const mockTerminate = vi.fn(); -const mockAddEventListener = vi.fn(); +const { duckdbClient } = await import('../../services/duckdb/duckdbClient'); -class MockWorker { - postMessage = mockPostMessage; - terminate = mockTerminate; - addEventListener = mockAddEventListener; +function deferred() { + let resolve!: (v: T) => void; + let reject!: (e: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; } -vi.stubGlobal('Worker', MockWorker); +function makeHandle(result: Promise, cancel: ReturnType = vi.fn()) { + return { taskId: 't1', result, progress: (async function* () {})(), cancel }; +} -// Re-import after stubbing so the module captures MockWorker -const { duckdbClient } = await import('../../services/duckdb/duckdbClient'); +function makeBus() { + return { enqueue: mockEnqueue, terminatePool: mockTerminatePool }; +} beforeEach(() => { vi.clearAllMocks(); - // Reset internal worker state between tests - duckdbClient.terminate(); + mockEnsureDuckDbPool.mockResolvedValue(makeBus()); + mockGetWorkerBus.mockReturnValue(makeBus()); }); describe('duckdbClient.init', () => { - it('posts an INIT message with a messageId', async () => { - const promise = duckdbClient.init(); + it('enqueues db.duckdb.init with the db.duckdb capability and returns {ok:true}', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.resolve({ ok: true }))); + + const res = await duckdbClient.init(); - // Simulate worker responding synchronously via the message event listener - const listenerCall = mockAddEventListener.mock.calls.find(([ev]) => ev === 'message'); - expect(listenerCall).toBeDefined(); - const onMessage = listenerCall![1] as (e: MessageEvent) => void; + expect(mockEnqueue).toHaveBeenCalledWith( + 'db.duckdb.init', + expect.anything(), + expect.objectContaining({ capabilities: ['db.duckdb'] }), + ); + expect(res.ok).toBe(true); + }); - // Retrieve the messageId from the postMessage call - const req = mockPostMessage.mock.calls[0]?.[0] as { messageId: string; type: string }; - expect(req.type).toBe('INIT'); + it('returns {ok:false} without enqueuing when the pool is unavailable', async () => { + mockEnsureDuckDbPool.mockResolvedValue(null); - onMessage({ data: { messageId: req.messageId, ok: true } } as MessageEvent); - const res = await promise; + const res = await duckdbClient.init(); - expect(res.ok).toBe(true); - expect(res.messageId).toBe(req.messageId); + expect(res).toEqual(expect.objectContaining({ ok: false, error: 'WorkerBus v2 unavailable' })); + expect(mockEnqueue).not.toHaveBeenCalled(); }); }); describe('duckdbClient.query', () => { - it('posts a QUERY message and returns rows', async () => { - const promise = duckdbClient.query('SELECT 1 AS n'); + it('enqueues db.duckdb.query and wraps the raw row array as {ok:true, rows}', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.resolve([{ n: 1 }]))); - const listenerCall = mockAddEventListener.mock.calls.find(([ev]) => ev === 'message'); - const onMessage = listenerCall![1] as (e: MessageEvent) => void; - const req = mockPostMessage.mock.calls[0]?.[0] as { messageId: string; type: string }; + const res = await duckdbClient.query('SELECT 1 AS n'); - expect(req.type).toBe('QUERY'); + const [taskType, payload] = mockEnqueue.mock.calls[0] as [string, unknown]; + expect(taskType).toBe('db.duckdb.query'); + expect(payload).toEqual({ sql: 'SELECT 1 AS n', params: undefined }); + expect(res).toEqual(expect.objectContaining({ ok: true, rows: [{ n: 1 }] })); + }); - onMessage({ - data: { messageId: req.messageId, ok: true, rows: [{ n: 1 }] }, - } as MessageEvent); + it('returns {ok:false, error} (not a thrown exception) when the task rejects', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(new Error('bad sql')))); - const res = await promise; - expect(res.ok).toBe(true); - expect(res.rows).toEqual([{ n: 1 }]); + const res = await duckdbClient.query('garbage'); + + expect(res).toEqual(expect.objectContaining({ ok: false, error: 'bad sql' })); + }); + + it('maps a CIRCUIT_OPEN error code to a friendly message', async () => { + const err = Object.assign(new Error('circuit open'), { code: 'CIRCUIT_OPEN' }); + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(err))); + + const res = await duckdbClient.query('SELECT 1'); + + expect(res).toEqual( + expect.objectContaining({ + ok: false, + error: 'DuckDB temporarily unavailable (circuit open)', + }), + ); + }); + + it('disables WorkerBus-level retries (retryPolicy maxRetries:0) to avoid stacking under withDuckDbRetry', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.resolve([]))); + + await duckdbClient.query('SELECT 1'); + + expect(mockEnqueue).toHaveBeenCalledWith( + 'db.duckdb.query', + expect.anything(), + expect.objectContaining({ retryPolicy: { maxRetries: 0 } }), + ); }); }); describe('duckdbClient.exec', () => { - it('posts an EXEC message', async () => { - const promise = duckdbClient.exec('CREATE TABLE t (id INTEGER)'); - - const listenerCall = mockAddEventListener.mock.calls.find(([ev]) => ev === 'message'); - const onMessage = listenerCall![1] as (e: MessageEvent) => void; - const req = mockPostMessage.mock.calls[0]?.[0] as { messageId: string; type: string }; + it('enqueues db.duckdb.exec and returns {ok:true}', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.resolve({ ok: true }))); - expect(req.type).toBe('EXEC'); + const res = await duckdbClient.exec('CREATE TABLE t (id INTEGER)'); - onMessage({ data: { messageId: req.messageId, ok: true } } as MessageEvent); - const res = await promise; + expect((mockEnqueue.mock.calls[0] as [string])[0]).toBe('db.duckdb.exec'); expect(res.ok).toBe(true); }); }); describe('duckdbClient.shutdown', () => { - it('posts a SHUTDOWN message', async () => { - const promise = duckdbClient.shutdown(); + it('enqueues db.duckdb.shutdown', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.resolve({ ok: true }))); - const listenerCall = mockAddEventListener.mock.calls.find(([ev]) => ev === 'message'); - const onMessage = listenerCall![1] as (e: MessageEvent) => void; - const req = mockPostMessage.mock.calls[0]?.[0] as { messageId: string; type: string }; + const res = await duckdbClient.shutdown(); - expect(req.type).toBe('SHUTDOWN'); - - onMessage({ data: { messageId: req.messageId, ok: true } } as MessageEvent); - const res = await promise; + expect((mockEnqueue.mock.calls[0] as [string])[0]).toBe('db.duckdb.shutdown'); expect(res.ok).toBe(true); }); }); describe('duckdbClient.terminate', () => { - it('calls worker.terminate()', async () => { - // Ensure the worker is created by making a call first - const promise = duckdbClient.query('SELECT 1'); - const listenerCall = mockAddEventListener.mock.calls.find(([ev]) => ev === 'message'); - const onMessage = listenerCall![1] as (e: MessageEvent) => void; - const req = mockPostMessage.mock.calls[0]?.[0] as { messageId: string }; - onMessage({ data: { messageId: req.messageId, ok: true, rows: [] } } as MessageEvent); - await promise; - + it('terminates only the "duckdb" pool, not the whole shared bus', () => { duckdbClient.terminate(); - expect(mockTerminate).toHaveBeenCalled(); + + expect(mockTerminatePool).toHaveBeenCalledWith('duckdb'); + }); + + it('is a no-op when the bus was never initialized', () => { + mockGetWorkerBus.mockReturnValue(null); + + expect(() => duckdbClient.terminate()).not.toThrow(); + expect(mockTerminatePool).not.toHaveBeenCalled(); }); }); describe('duckdbClient abort', () => { - it('posts WORKER_CANCEL when AbortSignal fires', async () => { + it('cancels the task handle when the AbortSignal fires mid-flight', async () => { + const cancel = vi.fn(); + const { promise: resultPromise, reject } = deferred(); + mockEnqueue.mockReturnValue(makeHandle(resultPromise, cancel)); + const controller = new AbortController(); - void duckdbClient.query('SELECT 1', undefined, controller.signal); + const resPromise = duckdbClient.query('SELECT 1', undefined, controller.signal); + controller.abort(); + + // QNBS-v3: cancel() fires after the async ensureDuckDbPool()/enqueue() chain settles, not + // synchronously — unlike v1's Promise-executor version. waitFor tolerates that gap + // without hardcoding a specific number of microtask ticks. + await vi.waitFor(() => expect(cancel).toHaveBeenCalledWith('Aborted')); - const req = mockPostMessage.mock.calls[0]?.[0] as { messageId: string }; + reject(Object.assign(new Error('Task was cancelled'), { code: 'CANCELLED' })); + const res = await resPromise; + + expect(res).toEqual(expect.objectContaining({ ok: false, error: 'Aborted' })); + }); + + it('registers the abort listener synchronously, so an already-fired-before-enqueue abort is not lost', async () => { + const cancel = vi.fn(); + const { promise: resultPromise } = deferred(); + mockEnqueue.mockReturnValue(makeHandle(resultPromise, cancel)); + + const controller = new AbortController(); controller.abort(); + void duckdbClient.query('SELECT 1', undefined, controller.signal); - // The cancel message should be posted after abort - const cancelMsg = mockPostMessage.mock.calls.find( - ([m]) => (m as { type: string }).type === 'WORKER_CANCEL', + await vi.waitFor(() => expect(cancel).toHaveBeenCalledWith('Aborted')); + }); + + it('does not miss an abort fired in the same tick as the call, before ensureDuckDbPool resolves', async () => { + const cancel = vi.fn(); + const { promise: resultPromise } = deferred(); + mockEnqueue.mockReturnValue(makeHandle(resultPromise, cancel)); + // QNBS-v3: regression guard for the real timing bug this PR fixed — the abort listener must + // be attached before the `await ensureDuckDbPool()` inside send() yields, so an + // abort fired synchronously right after the call (same tick, no microtask flush + // in between) still gets caught once the handle exists. + let resolveEnsurePool!: (bus: ReturnType) => void; + mockEnsureDuckDbPool.mockReturnValue( + new Promise((res) => { + resolveEnsurePool = res; + }), ); - expect(cancelMsg).toBeDefined(); - expect((cancelMsg![0] as { messageId: string }).messageId).toBe(req.messageId); + + const controller = new AbortController(); + void duckdbClient.query('SELECT 1', undefined, controller.signal); + controller.abort(); + resolveEnsurePool(makeBus()); + + await vi.waitFor(() => expect(cancel).toHaveBeenCalledWith('Aborted')); + }); +}); + +describe('duckdbClient OPFS fallback', () => { + it('forwards the opfs-fallback progress stage to setOpfsFallbackHandler', async () => { + let capturedOnProgress: ((p: { stage: string; message?: string }) => void) | undefined; + mockEnqueue.mockImplementation((_type, _payload, opts) => { + capturedOnProgress = opts.onProgress; + return makeHandle(Promise.resolve({ ok: true })); + }); + const onFallback = vi.fn(); + duckdbClient.setOpfsFallbackHandler(onFallback); + + await duckdbClient.init(); + capturedOnProgress?.({ stage: 'opfs-fallback', message: 'OPFS unavailable in private mode' }); + + expect(onFallback).toHaveBeenCalledWith('OPFS unavailable in private mode'); + duckdbClient.setOpfsFallbackHandler(null); + }); + + it('ignores non-opfs-fallback progress stages', async () => { + let capturedOnProgress: ((p: { stage: string; message?: string }) => void) | undefined; + mockEnqueue.mockImplementation((_type, _payload, opts) => { + capturedOnProgress = opts.onProgress; + return makeHandle(Promise.resolve({ ok: true })); + }); + const onFallback = vi.fn(); + duckdbClient.setOpfsFallbackHandler(onFallback); + + await duckdbClient.init(); + capturedOnProgress?.({ stage: 'loading', message: 'irrelevant' }); + + expect(onFallback).not.toHaveBeenCalled(); + duckdbClient.setOpfsFallbackHandler(null); }); }); diff --git a/workers/duckdbWorker.ts b/workers/duckdbWorker.ts deleted file mode 100644 index 30899b36..00000000 --- a/workers/duckdbWorker.ts +++ /dev/null @@ -1,219 +0,0 @@ -/// -// QNBS-v3: Off-main-thread DuckDB-WASM worker. OPFS persistence with in-memory fallback. -// Uses duckdb-eh bundle (no SharedArrayBuffer required — no COOP/COEP needed). -// Message protocol mirrors inference.worker.ts for consistency. - -export type DuckDbRequestType = 'INIT' | 'QUERY' | 'EXEC' | 'MIGRATE' | 'SHUTDOWN'; - -// QNBS-v3: Out-of-band event emitted when OPFS attach fails — no messageId, fires independently. -export interface DuckDbWorkerEvent { - type: 'OPFS_FALLBACK'; - reason: string; -} - -export interface DuckDbRequest { - messageId: string; - type: DuckDbRequestType; - sql?: string | undefined; - params?: readonly unknown[] | undefined; -} - -export interface DuckDbResponse { - messageId: string; - ok: boolean; - rows?: Record[]; - error?: string; - latencyMs?: number; -} - -// QNBS-v3: Lazy module reference — DuckDB bundle is ~2 MB; only load when INIT is received. -let duckdbModule: typeof import('@duckdb/duckdb-wasm') | null = null; -let db: import('@duckdb/duckdb-wasm').AsyncDuckDB | null = null; -let connection: import('@duckdb/duckdb-wasm').AsyncDuckDBConnection | null = null; - -async function getDuckDb() { - if (!duckdbModule) { - duckdbModule = await import('@duckdb/duckdb-wasm'); - } - return duckdbModule; -} - -// QNBS-v3: Security guard — only process messages from the same origin (mirrors inference.worker.ts). -function isTrustedWorkerMessage(event: MessageEvent): boolean { - return event.origin === '' || event.origin === globalThis.location?.origin; -} - -async function isOPFSSupported(): Promise { - try { - const root = await navigator.storage?.getDirectory?.(); - if (!root) return false; - await root.getFileHandle('__duckdb_opfs_test__', { create: true }); - return true; - } catch { - return false; - } -} - -// QNBS-v3 (F-09): self-hosted, same-origin DuckDB-WASM assets (scripts/copy-duckdb-assets.mjs) replacing an unversioned CDN that was already dead under worker-src 'self' blob:' CSP; absolute path so resolution is unambiguous from either the main thread or this nested worker context. -const DUCKDB_ASSET_BASE = `${import.meta.env.BASE_URL}duckdb/`; -const SELF_HOSTED_BUNDLES = { - mvp: { - mainModule: `${DUCKDB_ASSET_BASE}duckdb-mvp.wasm`, - mainWorker: `${DUCKDB_ASSET_BASE}duckdb-browser-mvp.worker.js`, - }, - eh: { - mainModule: `${DUCKDB_ASSET_BASE}duckdb-eh.wasm`, - mainWorker: `${DUCKDB_ASSET_BASE}duckdb-browser-eh.worker.js`, - }, -}; - -async function initDuckDb(): Promise { - const { AsyncDuckDB, selectBundle, ConsoleLogger } = await getDuckDb(); - - const bundle = await selectBundle(SELF_HOSTED_BUNDLES); - const logger = new ConsoleLogger(); - // QNBS-v3: bundle.mainWorker can be null for non-browser bundles; guard before constructing Worker. - if (!bundle.mainWorker) throw new Error('DuckDB bundle has no worker URL'); - const worker = new Worker(bundle.mainWorker); - const newDb = new AsyncDuckDB(logger, worker); - await newDb.instantiate(bundle.mainModule); - - // QNBS-v3: OPFS for persistence when available; fall back to in-memory (private browsing, old Safari). - const useOpfs = await isOPFSSupported(); - if (useOpfs) { - try { - const { DuckDBDataProtocol } = await getDuckDb(); - const opfsRoot = await navigator.storage.getDirectory(); - const fileHandle = await opfsRoot.getFileHandle('worldscript_analytics.duckdb', { - create: true, - }); - await newDb.registerFileHandle( - 'worldscript_analytics.duckdb', - fileHandle, - DuckDBDataProtocol.BROWSER_FSACCESS, - true, - ); - connection = await newDb.connect(); - await connection.query("ATTACH 'worldscript_analytics.duckdb' AS analytics (TYPE duckdb)"); - } catch (opfsErr) { - // QNBS-v3: Notify main thread so the hook can set persistence-mode badge in Redux. - self.postMessage({ - type: 'OPFS_FALLBACK', - reason: opfsErr instanceof Error ? opfsErr.message : String(opfsErr), - } satisfies DuckDbWorkerEvent); - connection = await newDb.connect(); - } - } else { - connection = await newDb.connect(); - } - - db = newDb; -} - -async function handleQuery(req: DuckDbRequest): Promise { - const start = Date.now(); - if (!connection) { - return { messageId: req.messageId, ok: false, error: 'DuckDB not initialized', latencyMs: 0 }; - } - try { - const result = await connection.query(req.sql ?? ''); - const rows = result.toArray().map((row: { toJSON(): Record }) => row.toJSON()); - return { messageId: req.messageId, ok: true, rows, latencyMs: Date.now() - start }; - } catch (err) { - return { - messageId: req.messageId, - ok: false, - error: err instanceof Error ? err.message : String(err), - latencyMs: Date.now() - start, - }; - } -} - -async function handleExec(req: DuckDbRequest): Promise { - const start = Date.now(); - if (!connection) { - return { messageId: req.messageId, ok: false, error: 'DuckDB not initialized', latencyMs: 0 }; - } - try { - await connection.query(req.sql ?? ''); - return { messageId: req.messageId, ok: true, latencyMs: Date.now() - start }; - } catch (err) { - return { - messageId: req.messageId, - ok: false, - error: err instanceof Error ? err.message : String(err), - latencyMs: Date.now() - start, - }; - } -} - -async function handleShutdown(req: DuckDbRequest): Promise { - try { - await connection?.close(); - await db?.terminate(); - connection = null; - db = null; - return { messageId: req.messageId, ok: true }; - } catch (err) { - return { - messageId: req.messageId, - ok: false, - error: err instanceof Error ? err.message : String(err), - }; - } -} - -// QNBS-v3: In-flight AbortController map (mirrors inference.worker.ts; DuckDB cancellation via close/reopen). -const abortMap = new Map(); - -self.addEventListener('message', (event: MessageEvent) => { - if (!isTrustedWorkerMessage(event)) return; - - // QNBS-v3: Check raw type as string before narrowing to DuckDbRequest so WORKER_CANCEL is reachable. - const raw = event.data as { type?: string; messageId?: string }; - - if (raw.type === 'WORKER_CANCEL' && raw.messageId) { - abortMap.get(raw.messageId)?.abort(); - abortMap.delete(raw.messageId); - return; - } - - const req = event.data as DuckDbRequest; - - const controller = new AbortController(); - abortMap.set(req.messageId, controller); - - let task: Promise; - - switch (req.type) { - case 'INIT': - task = initDuckDb() - .then(() => ({ messageId: req.messageId, ok: true })) - .catch((err: unknown) => ({ - messageId: req.messageId, - ok: false, - error: err instanceof Error ? err.message : String(err), - })); - break; - case 'QUERY': - task = handleQuery(req); - break; - case 'EXEC': - task = handleExec(req); - break; - case 'SHUTDOWN': - task = handleShutdown(req); - break; - default: - task = Promise.resolve({ - messageId: req.messageId, - ok: false, - error: `Unknown request type: ${String(req.type)}`, - }); - } - - void task.then((response) => { - abortMap.delete(req.messageId); - self.postMessage(response); - }); -}); From dbebf5cba7ee064986f17b59eec07d9d51bba819 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:15:39 +0200 Subject: [PATCH 04/10] fix(worker-bus): make OPFS-connection cleanup best-effort on ATTACH failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's second pass on #286 flagged this as an outside-diff-range comment (not a resolvable inline thread) on bcc41c27's own fix: the new `await opfsConnection?.close()` call could itself reject, which would skip emitProgress('opfs-fallback', ...) and the fallback newDb.connect(), and replace the original ATTACH error with the close failure — turning a graceful degrade into a hard init failure. Wrapped the cleanup in its own try/catch (best-effort, logged via console.warn, never blocks the fallback path) so a failing close() can't prevent the fallback connection or swallow the real error. New regression test asserts both still happen when close() rejects. Co-Authored-By: Claude Sonnet 5 --- tests/unit/duckdbWorkerHandler.test.ts | 18 ++++++++++++++++++ workers/v2/duckdb.worker.ts | 7 ++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/unit/duckdbWorkerHandler.test.ts b/tests/unit/duckdbWorkerHandler.test.ts index 5ebe5fa3..b942e2ec 100644 --- a/tests/unit/duckdbWorkerHandler.test.ts +++ b/tests/unit/duckdbWorkerHandler.test.ts @@ -145,6 +145,24 @@ describe('duckdb.worker handlers', () => { expect(mockConnect).toHaveBeenCalledTimes(2); }); + it('still falls back and emits the original OPFS error when close() itself rejects', async () => { + const opfsClose = vi.fn().mockRejectedValue(new Error('close also failed')); + const opfsQuery = vi.fn().mockRejectedValue(new Error('ATTACH failed')); + mockConnect + .mockResolvedValueOnce({ query: opfsQuery, close: opfsClose }) + .mockResolvedValueOnce(mockConnection()); + mockRegisterFileHandle.mockResolvedValue(undefined); + stubOpfsDirectory(vi.fn().mockResolvedValue({})); + const emitProgress = vi.fn(); + + await expect(initDuckDb(emitProgress)).resolves.toBeUndefined(); + + expect(opfsClose).toHaveBeenCalled(); + // The original ATTACH error reaches emitProgress, not the close() failure. + expect(emitProgress).toHaveBeenCalledWith('opfs-fallback', 1, 'ATTACH failed'); + expect(mockConnect).toHaveBeenCalledTimes(2); + }); + it('does not emit opfs-fallback when OPFS is simply unsupported (no navigator.storage)', async () => { mockConnect.mockResolvedValue(mockConnection()); stubNoOpfs(); diff --git a/workers/v2/duckdb.worker.ts b/workers/v2/duckdb.worker.ts index 891886ea..d7a5880a 100644 --- a/workers/v2/duckdb.worker.ts +++ b/workers/v2/duckdb.worker.ts @@ -77,8 +77,13 @@ export async function initDuckDb( ); connection = opfsConnection; } catch (opfsErr) { + // QNBS-v3: [Cleanup is best-effort — a rejecting close() must not block the fallback below or replace the original OPFS error.] + try { + await opfsConnection?.close(); + } catch (closeErr) { + console.warn('[duckdb.worker] Failed to close partial OPFS connection', closeErr); + } // QNBS-v3: [No bare postMessage from inside a task handler — OPFS-unavailable is surfaced via the progress channel duckdbClient's INIT adapter listens on instead.] - await opfsConnection?.close(); emitProgress?.( 'opfs-fallback', 1, From 871cfbaa3c0afdeb7b2a15614a017ab361f14534 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:27:08 +0200 Subject: [PATCH 05/10] feat(ai): migrate localEmbeddingService onto WorkerBus v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third of 5 PRs consolidating the v1/WorkerBus-v2 worker duplication (ADR-0014). services/ai/localEmbeddingService.ts's public API (embedText/embedBatch/cosineSimilarity/clearEmbeddingCache) is unchanged — its consumers (localRagService.ts, crossProjectIndexService.ts, ragPromptAssembly.ts, ragVectorMigration.ts, proForgeMemoryBank.ts, loraEvaluationService.ts, loraDatasetBuilder.ts, inferenceGateway.ts) need no changes. Internals swapped from a dedicated `new Worker(inference.worker.ts)` instance to the shared WorkerBus v2 'inference' pool via ensureInferencePool() (same decoupling-from- enableWorkerBusV2 pattern as ensureWebLlmPool/ensureDuckDbPool). v2's handler already throws on failure (matching embedText's existing throw-on-failure contract), so no response-shape translation is needed here — just swap the transport. Deleted ~70 lines of hand-rolled health-check/restart/backoff logic (30s ping, exponential backoff up to 5 attempts), superseded by WorkerPool's generic health check. Capped the shared 'inference' pool's maxWorkers to 2 (was MAX_WORKERS_INFERENCE=4) in workerBusManager.ts: each pool replica independently loads its own transformers.js pipeline with no cross-replica cache sharing, so 4 concurrent workers under a burst (e.g. embedBatch's micro-batching) could mean 4x the model memory footprint. workers/inference.worker.ts (v1) is NOT deleted yet — localNlpService.ts still uses it directly; removal is PR 4 of this sequence, after that migration lands too. Co-Authored-By: Claude Sonnet 5 --- services/ai/localEmbeddingService.ts | 145 ++--------------------- services/workerBusManager.ts | 3 +- tests/unit/localEmbeddingService.test.ts | 127 ++++++++++---------- 3 files changed, 77 insertions(+), 198 deletions(-) diff --git a/services/ai/localEmbeddingService.ts b/services/ai/localEmbeddingService.ts index b3bb9782..990ae993 100644 --- a/services/ai/localEmbeddingService.ts +++ b/services/ai/localEmbeddingService.ts @@ -1,9 +1,6 @@ -// QNBS-v3: Semantic embedding service — routes to inference.worker.ts via WorkerBus channels. -// Uses Xenova/all-MiniLM-L6-v2 (384-dim, L2-normalized) for semantic RAG and cross-project search. -// Adapted from CannaGuide-2025 embeddingService.ts patterns. +// QNBS-v3: [Semantic embedding service — routes to workers/v2/inference.worker.ts via WorkerBus v2 (docs/adr/0014 migration). Xenova/all-MiniLM-L6-v2, 384-dim, L2-normalized.] -// QNBS-v3: logger import was missing — restartWorker() references logger.error on restart-limit. -import { logger } from '../logger'; +import { ensureInferencePool } from '../workerBusManager'; const EMBEDDING_MODEL = 'Xenova/all-MiniLM-L6-v2'; const MAX_INPUT_CHARS = 512; @@ -22,95 +19,6 @@ function makeCacheKey(text: string): string { export type EmbeddingVector = Float32Array; -// QNBS-v3: The inference worker is loaded lazily to avoid a 2 MB bundle on app start. -let workerInstance: Worker | null = null; -// QNBS-v3: Health check — 30s ping interval; worker restarts if no pong within PONG_TIMEOUT_MS. -const PING_INTERVAL_MS = 30_000; -const PONG_TIMEOUT_MS = 5_000; -let pingTimer: ReturnType | null = null; -let pongTimeoutTimer: ReturnType | null = null; - -// QNBS-v3: Exponential backoff for worker restart to prevent infinite spin on permanent failure. -let restartAttemptCount = 0; -const MAX_RESTART_ATTEMPTS = 5; -const MAX_RESTART_BACKOFF_MS = 60_000; - -function getRestartBackoffMs(): number { - return Math.min(2 ** restartAttemptCount * 1000, MAX_RESTART_BACKOFF_MS); -} - -function clearWorkerHealthTimers(): void { - if (pingTimer) { - clearInterval(pingTimer); - pingTimer = null; - } - if (pongTimeoutTimer) { - clearTimeout(pongTimeoutTimer); - pongTimeoutTimer = null; - } -} - -function restartWorker(): void { - clearWorkerHealthTimers(); - if (workerInstance) { - workerInstance.terminate(); - workerInstance = null; - } - if (import.meta.env?.DEV) { - console.warn('[localEmbeddingService] Inference worker restarted (missed health check pong)'); - } - restartAttemptCount++; - if (restartAttemptCount > MAX_RESTART_ATTEMPTS) { - logger.error( - `[localEmbeddingService] Worker restart limit (${MAX_RESTART_ATTEMPTS}) reached. ` + - 'Embedding service is offline. Reload the app to retry.', - ); - return; - } - const backoff = getRestartBackoffMs(); - if (backoff > 0) { - setTimeout(() => startWorkerHealthCheck(), backoff); - } else { - startWorkerHealthCheck(); - } -} - -function startWorkerHealthCheck(): void { - if (typeof Worker === 'undefined') return; - pingTimer = setInterval(() => { - const w = workerInstance; - if (!w) return; - w.postMessage({ type: 'WORKER_PING' }); - pongTimeoutTimer = setTimeout(() => { - // QNBS-v3: No pong received — worker is dead or hung; restart with backoff. - restartWorker(); - }, PONG_TIMEOUT_MS); - - const pongHandler = (ev: MessageEvent<{ type?: string }>) => { - if (ev.data?.type === 'WORKER_PONG') { - // QNBS-v3: Successful pong resets the restart counter. - restartAttemptCount = 0; - if (pongTimeoutTimer) { - clearTimeout(pongTimeoutTimer); - pongTimeoutTimer = null; - } - w.removeEventListener('message', pongHandler); - } - }; - w.addEventListener('message', pongHandler); - }, PING_INTERVAL_MS); -} - -function getWorker(): Worker { - if (!workerInstance) { - workerInstance = new Worker(new URL('../../workers/inference.worker.ts', import.meta.url), { - type: 'module', - }); - startWorkerHealthCheck(); - } - return workerInstance; -} - function truncate(text: string): string { if (text.length <= MAX_INPUT_CHARS) return text; // QNBS-v3: Silent truncation — warn in dev builds only. @@ -129,30 +37,15 @@ function l2Normalize(vec: number[]): EmbeddingVector { return new Float32Array(vec.map((v) => v / magnitude)); } -function postToWorker( - task: string, - modelId: string, - input: string, -): Promise<{ ok: boolean; result?: number[]; error?: string }> { - return new Promise((resolve) => { - const messageId = `emb-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`; - const worker = getWorker(); - - const handler = (event: MessageEvent) => { - const data = event.data as { - messageId: string; - ok: boolean; - result?: number[]; - error?: string; - }; - if (data.messageId !== messageId) return; - worker.removeEventListener('message', handler); - resolve(data); - }; - - worker.addEventListener('message', handler); - worker.postMessage({ messageId, task, modelId, input }); - }); +async function requestEmbedding(task: string, modelId: string, input: string): Promise { + const bus = await ensureInferencePool(); + if (!bus) throw new Error('WorkerBus v2 unavailable'); + const handle = bus.enqueue<{ task: string; modelId: string; input: string }, number[]>( + 'inference.embed', + { task, modelId, input }, + { capabilities: ['inference.embed'] }, + ); + return handle.result; } export async function embedText(text: string): Promise { @@ -167,11 +60,8 @@ export async function embedText(text: string): Promise { return cached; } - const response = await postToWorker('feature-extraction', EMBEDDING_MODEL, truncated); - if (!response.ok || !response.result) { - throw new Error(response.error ?? 'embedding failed'); - } - const vector = l2Normalize(response.result); + const raw = await requestEmbedding('feature-extraction', EMBEDDING_MODEL, truncated); + const vector = l2Normalize(raw); // QNBS-v3: Evict the oldest (first) entry when at capacity before inserting. if (embeddingCache.size >= EMBEDDING_CACHE_MAX) { @@ -206,12 +96,3 @@ export function cosineSimilarity(a: EmbeddingVector, b: EmbeddingVector): number // Both vectors are already L2-normalised, so cosine = dot product return Math.max(-1, Math.min(1, dot)); } - -// QNBS-v3: Used in testing to reset the worker instance without affecting production code. -export function _resetWorkerForTest(): void { - clearWorkerHealthTimers(); - if (workerInstance) { - workerInstance.terminate(); - workerInstance = null; - } -} diff --git a/services/workerBusManager.ts b/services/workerBusManager.ts index 2e973c24..ede6b7d6 100644 --- a/services/workerBusManager.ts +++ b/services/workerBusManager.ts @@ -87,7 +87,8 @@ async function doInitWorkerBus(): Promise { poolId: 'inference', capabilities: ['inference.text', 'inference.embed'], options: { - maxWorkers: MAX_WORKERS_INFERENCE, + // QNBS-v3: [Capped below MAX_WORKERS_INFERENCE — each replica loads its own transformers.js pipeline (no cross-replica cache sharing), so 4 concurrent workers could mean 4x the model memory footprint under a burst.] + maxWorkers: Math.min(2, MAX_WORKERS_INFERENCE), minWorkers: MIN_WORKERS, idleTimeoutMs: WORKER_IDLE_TIMEOUT_MS, workerScript: inferenceUrl, diff --git a/tests/unit/localEmbeddingService.test.ts b/tests/unit/localEmbeddingService.test.ts index 314b1f06..45310d42 100644 --- a/tests/unit/localEmbeddingService.test.ts +++ b/tests/unit/localEmbeddingService.test.ts @@ -1,99 +1,96 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -// QNBS-v3: Self-resolving MockWorker — when postMessage is called it immediately fires -// all registered 'message' handlers with a configurable response. This avoids -// async timing issues with concurrent Promise.all batches in embedBatch. -let mockResponse: { ok: boolean; result?: number[]; error?: string } = { - ok: true, - result: [0.5, 0.5], -}; - -let postMessageCalls: Array<{ messageId: string; input: string; task: string }> = []; - -class MockWorker { - private handlers: Array<(e: MessageEvent) => void> = []; - - addEventListener(_type: string, handler: (e: MessageEvent) => void) { - this.handlers.push(handler); - } - - removeEventListener(_type: string, handler: (e: MessageEvent) => void) { - const idx = this.handlers.indexOf(handler); - if (idx >= 0) this.handlers.splice(idx, 1); - } - - postMessage(msg: { messageId: string; input: string; task: string }) { - postMessageCalls.push(msg); - // QNBS-v3: Respond synchronously so Promise.all batches resolve without microtask tricks. - const response = { ...mockResponse, messageId: msg.messageId }; - const event = { data: response } as MessageEvent; - for (const handler of [...this.handlers]) { - handler(event); - } - } +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// QNBS-v3: [localEmbeddingService now routes through WorkerBus v2 — mock ensureInferencePool() instead of the global Worker constructor.] + +const { mockEnqueue, mockEnsureInferencePool } = vi.hoisted(() => ({ + mockEnqueue: vi.fn(), + mockEnsureInferencePool: vi.fn(), +})); - terminate() {} +vi.mock('../../services/workerBusManager', () => ({ + ensureInferencePool: mockEnsureInferencePool, +})); + +const { clearEmbeddingCache, cosineSimilarity, embedBatch, embedText } = await import( + '../../services/ai/localEmbeddingService' +); + +function makeHandle(result: Promise) { + return { taskId: 't1', result, progress: (async function* () {})(), cancel: vi.fn() }; } -vi.stubGlobal('Worker', MockWorker); +function makeBus() { + return { enqueue: mockEnqueue }; +} -import { - _resetWorkerForTest, - cosineSimilarity, - embedBatch, - embedText, -} from '../../services/ai/localEmbeddingService'; +let requestCalls: Array<{ task: string; modelId: string; input: string }> = []; +let nextResult: number[] = [0.5, 0.5]; beforeEach(() => { - _resetWorkerForTest(); - mockResponse = { ok: true, result: [0.5, 0.5] }; - postMessageCalls = []; -}); - -afterEach(() => { - _resetWorkerForTest(); + vi.clearAllMocks(); + clearEmbeddingCache(); + requestCalls = []; + nextResult = [0.5, 0.5]; + mockEnsureInferencePool.mockResolvedValue(makeBus()); + mockEnqueue.mockImplementation((_taskType: string, payload: unknown) => { + requestCalls.push(payload as { task: string; modelId: string; input: string }); + return makeHandle(Promise.resolve(nextResult)); + }); }); // ─── embedText ────────────────────────────────────────────────────────────── describe('embedText', () => { - it('returns a Float32Array for a successful worker response', async () => { - mockResponse = { ok: true, result: [0.5, 0.5] }; + it('returns a Float32Array for a successful task result', async () => { + nextResult = [0.5, 0.5]; const vec = await embedText('Hello world'); expect(vec).toBeInstanceOf(Float32Array); expect(vec.length).toBe(2); }); it('L2-normalises the returned vector (magnitude ≈ 1)', async () => { - mockResponse = { ok: true, result: [3, 4] }; // magnitude = 5 → normalised to [0.6, 0.8] + nextResult = [3, 4]; // magnitude = 5 → normalised to [0.6, 0.8] const vec = await embedText('Normalise me'); const magnitude = Math.sqrt(vec[0]! ** 2 + vec[1]! ** 2); expect(magnitude).toBeCloseTo(1, 5); }); - it('throws when worker returns ok:false with error message', async () => { - mockResponse = { ok: false, error: 'OOM' }; + it('propagates the error message when the task rejects', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(new Error('OOM')))); await expect(embedText('fail case')).rejects.toThrow('OOM'); }); - it('throws with generic message when error field is absent', async () => { - mockResponse = { ok: false }; - await expect(embedText('fail case 2')).rejects.toThrow('embedding failed'); + it('throws WorkerBus v2 unavailable without enqueuing when the pool is unavailable', async () => { + mockEnsureInferencePool.mockResolvedValue(null); + await expect(embedText('fail case 2')).rejects.toThrow('WorkerBus v2 unavailable'); + expect(mockEnqueue).not.toHaveBeenCalled(); }); it('truncates input silently when text exceeds 512 chars', async () => { await embedText('a'.repeat(600)); - expect(postMessageCalls[0]?.input.length).toBe(512); + expect(requestCalls[0]?.input.length).toBe(512); }); it('does not truncate input at exactly 512 chars', async () => { await embedText('b'.repeat(512)); - expect(postMessageCalls[0]?.input.length).toBe(512); + expect(requestCalls[0]?.input.length).toBe(512); }); - it('sends feature-extraction task to worker', async () => { + it('enqueues inference.embed with the feature-extraction task and inference.embed capability', async () => { await embedText('test'); - expect(postMessageCalls[0]?.task).toBe('feature-extraction'); + expect(requestCalls[0]?.task).toBe('feature-extraction'); + expect(mockEnqueue).toHaveBeenCalledWith( + 'inference.embed', + expect.anything(), + expect.objectContaining({ capabilities: ['inference.embed'] }), + ); + }); + + it('returns the cached vector on a second call for the same text without re-enqueuing', async () => { + const first = await embedText('cache me'); + const second = await embedText('cache me'); + expect(second).toBe(first); + expect(mockEnqueue).toHaveBeenCalledTimes(1); }); }); @@ -101,7 +98,7 @@ describe('embedText', () => { describe('embedBatch', () => { it('returns an array of Float32Arrays, one per input', async () => { - mockResponse = { ok: true, result: [0.5, 0.5] }; + nextResult = [0.5, 0.5]; const results = await embedBatch(['a', 'b', 'c']); expect(results).toHaveLength(3); for (const vec of results) { @@ -110,16 +107,16 @@ describe('embedBatch', () => { }); it('returns correct count for 9 texts (spans two micro-batches of 8+1)', async () => { - mockResponse = { ok: true, result: [0.1, 0.2] }; + nextResult = [0.1, 0.2]; const texts = Array.from({ length: 9 }, (_, i) => `text-${i}`); const results = await embedBatch(texts); expect(results).toHaveLength(9); }); - it('sends exactly N worker messages for N input texts', async () => { - mockResponse = { ok: true, result: [0.1, 0.2] }; + it('sends exactly N enqueue calls for N input texts', async () => { + nextResult = [0.1, 0.2]; await embedBatch(['x', 'y', 'z']); - expect(postMessageCalls).toHaveLength(3); + expect(requestCalls).toHaveLength(3); }); it('returns empty array for empty input', async () => { From 36c5f7cb553fc1751c4fc65551d683a111ec5acf Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 21:23:14 +0200 Subject: [PATCH 06/10] fix(worker-bus): pool-termination lifecycle, idle-respawn recovery, SQL param binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real findings from CodeRabbit's review of #287 (duckdbClient migration onto WorkerBus v2), all confirmed against current code before fixing: 1. CRITICAL — params silently dropped (workers/v2/duckdb.worker.ts). duckdbClient.send() enqueues {sql, params}, but handleQuery/ handleExec only read req.sql and called connection.query(sql) directly, ignoring params entirely and leaving an unbound-query/ SQL-injection surface flagged by static analysis. This was already live: services/ai/telemetryService.ts's writeToDuckDb() passes real bound params ([taskType, backend, modelId, latencyMs, success]) that were being silently discarded. Fixed via DuckDB-WASM prepared statements (connection.prepare(sql) + stmt.query(...params) + stmt.close()) when params is non-empty; unchanged direct connection.query(sql) otherwise. Needed a narrow structural type (PreparableConnection) since tsgo doesn't resolve AsyncDuckDBConnection.prepare() through this package's nested `export *` chain, despite it being present in the package's own .d.ts — same class of tsgo/external-package gap CLAUDE.md already documents for the transformers.js path alias. 2. MAJOR — DuckDB pool worker losing its connection on respawn (services/duckdb/duckdbClient.ts). The duckdb pool's minWorkers:0 lets its worker idle-terminate; a freshly spawned replacement worker's module state has no `connection` until INIT runs again on it, so the next QUERY/EXEC after a respawn failed with "DuckDB not initialized". Fixed by transparently re-sending INIT once and retrying the original call when send() sees that specific error message (bounded to one retry, INIT/SHUTDOWN excluded from the check to avoid any recursive loop). 3. MAJOR — terminatePool() had no lifecycle contract (packages/worker-bus/src/workerBus.ts + services/workerBusManager.ts). Removing a pool didn't settle tasks already routed to it — their result promises would await a RESULT message from a worker that no longer exists. Fixed by tracking task->pool assignment (new taskPools map, set/cleared around runTask's pool-acquire phase) and cancelling matching tasks before deletion. Separately, ensureDuckDbPool() assumed a non-null bus always has every pool, so any call after duckdbClient.terminate() would fail with NO_POOL forever; added WorkerBus.hasPool() and a re-registration path (duckDbPoolOptions()/reRegisterDuckDbPool(), shared with doInitWorkerBus() to avoid option drift between the two registration sites). Co-Authored-By: Claude Sonnet 5 --- README.md | 8 +- packages/worker-bus/src/workerBus.ts | 24 ++++++ packages/worker-bus/tests/workerBus.test.ts | 37 +++++++++ services/duckdb/duckdbClient.ts | 19 ++++- services/workerBusManager.ts | 42 +++++++--- tests/unit/duckdbClient.test.ts | 65 +++++++++++++++ tests/unit/duckdbWorkerHandler.test.ts | 90 +++++++++++++++++++++ tests/unit/workerBusManager.test.ts | 57 +++++++++++-- workers/v2/duckdb.worker.ts | 55 ++++++++++--- 9 files changed, 363 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 0e0d34b9..99dfe67f 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ IndexedDB v8 PWA v3.0 i18n 19 locales — 2849 keys - 5807+ tests / 529 files + 5807+ tests / 531 files Codecov Coverage License MIT CI Status @@ -471,7 +471,7 @@ The Settings → AI panel shows a live GPU status badge with adapter details and | **Document Export** | docx + jszip | Word-compatible `.docx` generation (lazy-loaded) | | **PWA** | Service Worker + Web App Manifest v3 | Offline support, installability, Workbox chunking | | **i18n** | Custom React Context (`I18nContext.tsx`) | 2849 keys × 19 locales (de/en/es/fr/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu Beta); EN fallback; `localStorage` persistence | -| **Testing** | Vitest 4.x (5807+ tests / 529 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) | +| **Testing** | Vitest 4.x (5807+ tests / 531 files) + Playwright E2E | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow) | | **Code Quality** | Biome (lint + format) + TypeScript 7 (tsgo) strict | `--error-on-warnings` in CI; zero `any` policy | | **Visualization** | Force-directed graph | Interactive character relationship network | | **Desktop** | Tauri v2 | Cross-platform installer; auto-updater via `latest.json` | @@ -508,7 +508,7 @@ WorldScript-Studio/ │ ├── sw.js # PWA Service Worker │ └── manifest.json # PWA Web App Manifest v3 ├── tests/ -│ ├── unit/ # Vitest unit tests (5807+ tests, 529 files) — count spans tests/, components/, packages/*/tests/, not just this folder +│ ├── unit/ # Vitest unit tests (5807+ tests, 531 files) — count spans tests/, components/, packages/*/tests/, not just this folder │ │ ├── ai/ # aiSmallModules, aiCoreFallbackPaths │ │ └── settings/ # WebLlmPanel, AiSections │ └── e2e/ # Playwright specs + helpers.ts @@ -667,7 +667,7 @@ The main pipeline is [`.github/workflows/ci.yml`](.github/workflows/ci.yml). Opt | `scorecard` | weekly + `main` push | OpenSSF Scorecard — SARIF uploaded to GitHub Code Scanning | **Current test metrics (2026-07-28):** -- **5807+ unit tests** across **529 test files** — all passing +- **5807+ unit tests** across **531 test files** — all passing - Coverage thresholds: lines ≥ 74 · branches ≥ 60 · functions ≥ 67 · statements ≥ 72 — enforced in CI (see Codecov badge for live metrics) - i18n: **2849 keys × 19 locales** (en/de/fr/es/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu Beta) diff --git a/packages/worker-bus/src/workerBus.ts b/packages/worker-bus/src/workerBus.ts index 84b45627..f7e4b964 100644 --- a/packages/worker-bus/src/workerBus.ts +++ b/packages/worker-bus/src/workerBus.ts @@ -32,6 +32,8 @@ export class WorkerBus { private readonly dlq: DeadLetterQueue; private readonly progress = new ProgressEmitter(); private readonly tokens = new Map(); + // QNBS-v3: [Tracks which pool each in-flight task is running on, so terminatePool() can cancel exactly the affected tasks instead of leaving their result promises hanging forever.] + private readonly taskPools = new Map(); private readonly listeners = new Set(); private running = false; private pumpScheduled = false; @@ -49,6 +51,11 @@ export class WorkerBus { this.schedulePump(); } + // QNBS-v3: [Lets callers (e.g. ensureDuckDbPool()) detect a pool removed via terminatePool() and re-register it, instead of assuming a non-null bus always has every pool.] + hasPool(poolId: string): boolean { + return this.pools.has(poolId); + } + registerPool( poolId: string, capabilities: readonly WorkerCapability[], @@ -202,10 +209,14 @@ export class WorkerBus { * in-flight inference/webllm/plugin work on the shared bus (docs/adr/0014-worker-generation- * duplication.md's migration follow-up). The pool is removed from the registry; a later task * routed to its capabilities fails with NO_POOL until the bus re-registers it. + * QNBS-v3: [Cancels every in-flight task routed to this pool before removing it — otherwise their result promises would await a RESULT message from a worker that's already been terminated and never arrives.] */ async terminatePool(poolId: string): Promise { const pool = this.pools.get(poolId); if (!pool) return; + for (const [taskId, taskPoolId] of this.taskPools) { + if (taskPoolId === poolId) this.cancel(taskId, 'Pool terminated'); + } await pool.terminateAll(); this.pools.delete(poolId); } @@ -235,6 +246,19 @@ export class WorkerBus { }; } + this.taskPools.set(task.taskId, pool.poolId); + try { + return await this.runOnPool(task, token, pool); + } finally { + this.taskPools.delete(task.taskId); + } + } + + private async runOnPool( + task: WorkerTask, + token: CancellationToken, + pool: WorkerPool, + ): Promise> { const worker = await pool.acquire(token.signal); const startedAt = performance.now(); const queueTimeMs = Math.round(startedAt - task.createdAt); diff --git a/packages/worker-bus/tests/workerBus.test.ts b/packages/worker-bus/tests/workerBus.test.ts index 74ae557c..f1f9c580 100644 --- a/packages/worker-bus/tests/workerBus.test.ts +++ b/packages/worker-bus/tests/workerBus.test.ts @@ -233,6 +233,43 @@ describe('WorkerBus', () => { await expect(bus.terminatePool('does-not-exist')).resolves.toBeUndefined(); }); + it('terminatePool cancels only the in-flight tasks routed to that pool', async () => { + // QNBS-v3: [beforeEach's mocked acquire() ignores the abort signal entirely (returns a + // promise that never settles); override it here with a signal-aware implementation + // mirroring WorkerPool's real waitForIdle() so cancellation can actually be observed + // unsticking the caller, not just verified as "cancel() was called".] + const fakePool = (bus as unknown as { pools: Map }).pools.get('fake')!; + vi.spyOn(fakePool, 'acquire').mockImplementation( + (signal) => + new Promise((_resolve, reject) => { + signal?.addEventListener('abort', () => reject(new Error('Aborted')), { once: true }); + }), + ); + bus.registerPool('other', ['db.duckdb'], { + maxWorkers: 1, + minWorkers: 1, + idleTimeoutMs: 120_000, + workerScript: '/other.worker.js', + capabilities: ['db.duckdb'], + labels: {}, + }); + const otherPool = (bus as unknown as { pools: Map }).pools.get('other')!; + vi.spyOn(otherPool, 'acquire').mockReturnValue(new Promise(() => {})); + + const fakeHandle = bus.enqueue('test.task', {}, { capabilities: ['inference.text'] }); + const otherHandle = bus.enqueue('other.task', {}, { capabilities: ['db.duckdb'] }); + otherHandle.result.catch(() => {}); + + const cancelSpy = vi.spyOn(bus, 'cancel'); + await bus.terminatePool('fake'); + + expect(cancelSpy).toHaveBeenCalledTimes(1); + expect(cancelSpy).toHaveBeenCalledWith(fakeHandle.taskId, 'Pool terminated'); + // Without the fix, this task would hang forever at pool.acquire() — terminatePool() + // cancelling it lets the result promise settle instead. + await expect(fakeHandle.result).rejects.toThrow('Aborted'); + }); + it('progress callback receives emitted progress', async () => { const progressUpdates: Array<{ stage: string; progress: number }> = []; let capturedTaskId = ''; diff --git a/services/duckdb/duckdbClient.ts b/services/duckdb/duckdbClient.ts index 1315002f..b561fe72 100644 --- a/services/duckdb/duckdbClient.ts +++ b/services/duckdb/duckdbClient.ts @@ -43,6 +43,7 @@ async function send( sql?: string, params?: readonly unknown[], signal?: AbortSignal, + isReinitRetry = false, ): Promise { const messageId = generateMessageId(); const start = Date.now(); @@ -95,7 +96,23 @@ async function send( } catch (err) { const latencyMs = Date.now() - start; if (signal?.aborted) return { messageId, ok: false, error: 'Aborted', latencyMs }; - return { messageId, ok: false, error: errorMessage(err), latencyMs }; + const message = errorMessage(err); + // QNBS-v3: [The duckdb pool's worker can idle-terminate (minWorkers:0) or crash-restart; a + // freshly spawned worker's module state has no `connection` until INIT runs again. + // Transparently re-init once and retry, instead of surfacing a confusing "not + // initialized" error for what looks like a routine idle-timeout respawn.] + if ( + !isReinitRetry && + type !== 'INIT' && + type !== 'SHUTDOWN' && + message === 'DuckDB not initialized' + ) { + const reinit = await send('INIT', undefined, undefined, signal, true); + if (reinit.ok) { + return send(type, sql, params, signal, true); + } + } + return { messageId, ok: false, error: message, latencyMs }; } } diff --git a/services/workerBusManager.ts b/services/workerBusManager.ts index 2e973c24..a8004663 100644 --- a/services/workerBusManager.ts +++ b/services/workerBusManager.ts @@ -46,6 +46,27 @@ export async function initWorkerBus(): Promise { } } +// QNBS-v3: [Shared with reRegisterDuckDbPool() below so a pool removed via terminatePool() gets re-registered with identical options, not a drifted copy.] +async function duckDbPoolOptions() { + const { WORKER_IDLE_TIMEOUT_MS } = await import('@domain/worker-bus'); + return { + // QNBS-v3: DuckDB is single-writer — minWorkers 0 keeps the thread alive only while in use. + maxWorkers: 1, + minWorkers: 0, + idleTimeoutMs: WORKER_IDLE_TIMEOUT_MS, + workerScript: new URL('../workers/v2/duckdb.worker.ts', import.meta.url).href, + capabilities: ['db.duckdb'] as const, + labels: { pool: 'duckdb', version: 'v2' }, + }; +} + +/** Re-register the 'duckdb' pool if it was removed via terminatePool() — a no-op if it's already present. */ +async function reRegisterDuckDbPool(bus: WorkerBus): Promise { + if (bus.hasPool('duckdb')) return; + const options = await duckDbPoolOptions(); + bus.registerPool('duckdb', options.capabilities, options); +} + async function doInitWorkerBus(): Promise { try { const { @@ -78,7 +99,6 @@ async function doInitWorkerBus(): Promise { // QNBS-v3: new URL(path, import.meta.url) lets Vite emit the worker script as a proper // asset URL. The .ts extension is allowed — Vite transforms it during build. const inferenceUrl = new URL('../workers/v2/inference.worker.ts', import.meta.url).href; - const duckdbUrl = new URL('../workers/v2/duckdb.worker.ts', import.meta.url).href; // QNBS-v3: P1-1 — dedicated WebLLM (WebGPU) worker. Separate pool keeps @mlc-ai/web-llm out // of the transformers.js worker bundle and isolates the GPU lifecycle. const webllmUrl = new URL('../workers/v2/webllm.worker.ts', import.meta.url).href; @@ -96,18 +116,11 @@ async function doInitWorkerBus(): Promise { }, }); + const duckdbOptions = await duckDbPoolOptions(); registry.register({ poolId: 'duckdb', - // QNBS-v3: DuckDB is single-writer — minWorkers 0 keeps the thread alive only while in use. - capabilities: ['db.duckdb'], - options: { - maxWorkers: 1, - minWorkers: 0, - idleTimeoutMs: WORKER_IDLE_TIMEOUT_MS, - workerScript: duckdbUrl, - capabilities: ['db.duckdb'], - labels: { pool: 'duckdb', version: 'v2' }, - }, + capabilities: duckdbOptions.capabilities, + options: duckdbOptions, }); // QNBS-v3: P1-1 — WebLLM pool. maxWorkers:1 (single heavy GPU consumer; tab-leader election @@ -172,7 +185,12 @@ export async function ensureWebLlmPool(): Promise { * must not silently break analytics. Returns null only if init failed. */ export async function ensureDuckDbPool(): Promise { - if (_bus === null) await initWorkerBus(); + if (_bus === null) { + await initWorkerBus(); + return _bus; + } + // QNBS-v3: [terminatePool('duckdb') can remove the pool while the bus itself stays alive — re-register it here instead of assuming a non-null bus always has every pool.] + await reRegisterDuckDbPool(_bus); return _bus; } diff --git a/tests/unit/duckdbClient.test.ts b/tests/unit/duckdbClient.test.ts index c592b26c..c5790a92 100644 --- a/tests/unit/duckdbClient.test.ts +++ b/tests/unit/duckdbClient.test.ts @@ -113,6 +113,71 @@ describe('duckdbClient.query', () => { expect.objectContaining({ retryPolicy: { maxRetries: 0 } }), ); }); + + describe('auto-reinit on "DuckDB not initialized"', () => { + // QNBS-v3: [Regression coverage for the idle-respawn/crash-restart gap — a fresh duckdb pool + // worker has no `connection` until INIT runs again on it.] + it('re-inits once and retries the original query, then succeeds', async () => { + let queryCallCount = 0; + mockEnqueue.mockImplementation((taskType: string) => { + if (taskType === 'db.duckdb.query') { + queryCallCount++; + if (queryCallCount === 1) { + return makeHandle(Promise.reject(new Error('DuckDB not initialized'))); + } + return makeHandle(Promise.resolve([{ n: 1 }])); + } + if (taskType === 'db.duckdb.init') { + return makeHandle(Promise.resolve({ ok: true })); + } + throw new Error(`unexpected taskType ${taskType}`); + }); + + const res = await duckdbClient.query('SELECT 1 AS n'); + + expect(res).toEqual(expect.objectContaining({ ok: true, rows: [{ n: 1 }] })); + expect(queryCallCount).toBe(2); + const initCalls = mockEnqueue.mock.calls.filter(([t]) => t === 'db.duckdb.init'); + expect(initCalls).toHaveLength(1); + }); + + it('attempts exactly one reinit — does not loop when the retry also fails', async () => { + mockEnqueue.mockImplementation((taskType: string) => { + if (taskType === 'db.duckdb.query') { + return makeHandle(Promise.reject(new Error('DuckDB not initialized'))); + } + if (taskType === 'db.duckdb.init') { + return makeHandle(Promise.resolve({ ok: true })); + } + throw new Error(`unexpected taskType ${taskType}`); + }); + + const res = await duckdbClient.query('SELECT 1'); + + expect(res).toEqual(expect.objectContaining({ ok: false, error: 'DuckDB not initialized' })); + const initCalls = mockEnqueue.mock.calls.filter(([t]) => t === 'db.duckdb.init'); + expect(initCalls).toHaveLength(1); + }); + + it('does not reinit-retry unrelated error messages', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(new Error('syntax error')))); + + const res = await duckdbClient.query('garbage'); + + expect(res).toEqual(expect.objectContaining({ ok: false, error: 'syntax error' })); + const initCalls = mockEnqueue.mock.calls.filter(([t]) => t === 'db.duckdb.init'); + expect(initCalls).toHaveLength(0); + }); + + it('does not recursively reinit when INIT itself fails with "DuckDB not initialized"', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(new Error('DuckDB not initialized')))); + + await duckdbClient.init(); + + const initCalls = mockEnqueue.mock.calls.filter(([t]) => t === 'db.duckdb.init'); + expect(initCalls).toHaveLength(1); + }); + }); }); describe('duckdbClient.exec', () => { diff --git a/tests/unit/duckdbWorkerHandler.test.ts b/tests/unit/duckdbWorkerHandler.test.ts index b942e2ec..18f8db18 100644 --- a/tests/unit/duckdbWorkerHandler.test.ts +++ b/tests/unit/duckdbWorkerHandler.test.ts @@ -216,6 +216,67 @@ describe('duckdb.worker handlers', () => { 'bad sql', ); }); + + it('binds params via a prepared statement instead of dropping them', async () => { + const row = { toJSON: () => ({ id: 1 }) }; + const stmtQuery = vi.fn().mockResolvedValue({ toArray: () => [row] }); + const stmtClose = vi.fn().mockResolvedValue(undefined); + const prepare = vi.fn().mockResolvedValue({ query: stmtQuery, close: stmtClose }); + const rawQuery = vi.fn(); + mockConnect.mockResolvedValue({ + query: rawQuery, + close: vi.fn().mockResolvedValue(undefined), + prepare, + }); + stubNoOpfs(); + await initDuckDb(); + + const result = await handleQuery( + makeCtx({ payload: { sql: 'select * from t where id = ?', params: [1] } }), + ); + + expect(prepare).toHaveBeenCalledWith('select * from t where id = ?'); + expect(stmtQuery).toHaveBeenCalledWith(1); + expect(stmtClose).toHaveBeenCalled(); + expect(rawQuery).not.toHaveBeenCalled(); + expect(result).toEqual([{ id: 1 }]); + }); + + it('closes the prepared statement even when the bound query rejects', async () => { + const stmtQuery = vi.fn().mockRejectedValue(new Error('bind failed')); + const stmtClose = vi.fn().mockResolvedValue(undefined); + const prepare = vi.fn().mockResolvedValue({ query: stmtQuery, close: stmtClose }); + mockConnect.mockResolvedValue({ + query: vi.fn(), + close: vi.fn().mockResolvedValue(undefined), + prepare, + }); + stubNoOpfs(); + await initDuckDb(); + + await expect( + handleQuery(makeCtx({ payload: { sql: 'select ?', params: ['x'] } })), + ).rejects.toThrow('bind failed'); + expect(stmtClose).toHaveBeenCalled(); + }); + + it('does not use a prepared statement when params is an empty array', async () => { + const row = { toJSON: () => ({ id: 1 }) }; + const rawQuery = vi.fn().mockResolvedValue({ toArray: () => [row] }); + const prepare = vi.fn(); + mockConnect.mockResolvedValue({ + query: rawQuery, + close: vi.fn().mockResolvedValue(undefined), + prepare, + }); + stubNoOpfs(); + await initDuckDb(); + + await handleQuery(makeCtx({ payload: { sql: 'select 1', params: [] } })); + + expect(prepare).not.toHaveBeenCalled(); + expect(rawQuery).toHaveBeenCalledWith('select 1'); + }); }); describe('handleExec', () => { @@ -236,6 +297,35 @@ describe('duckdb.worker handlers', () => { expect(result).toEqual({ ok: true }); expect(connectionQuery).toHaveBeenCalledWith('create table t (id int)'); }); + + it('binds params via a prepared statement instead of dropping them', async () => { + const stmtQuery = vi.fn().mockResolvedValue(undefined); + const stmtClose = vi.fn().mockResolvedValue(undefined); + const prepare = vi.fn().mockResolvedValue({ query: stmtQuery, close: stmtClose }); + const rawQuery = vi.fn(); + mockConnect.mockResolvedValue({ + query: rawQuery, + close: vi.fn().mockResolvedValue(undefined), + prepare, + }); + stubNoOpfs(); + await initDuckDb(); + + const result = await handleExec( + makeCtx({ + payload: { + sql: 'insert into ai_telemetry values (?, ?, ?, ?, ?)', + params: ['task', 'backend', 'model', 12, true], + }, + }), + ); + + expect(prepare).toHaveBeenCalledWith('insert into ai_telemetry values (?, ?, ?, ?, ?)'); + expect(stmtQuery).toHaveBeenCalledWith('task', 'backend', 'model', 12, true); + expect(stmtClose).toHaveBeenCalled(); + expect(rawQuery).not.toHaveBeenCalled(); + expect(result).toEqual({ ok: true }); + }); }); describe('handleShutdown', () => { diff --git a/tests/unit/workerBusManager.test.ts b/tests/unit/workerBusManager.test.ts index 20cf41a7..151f3717 100644 --- a/tests/unit/workerBusManager.test.ts +++ b/tests/unit/workerBusManager.test.ts @@ -4,10 +4,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // QNBS-v3: vi.hoisted ensures these references are available inside the vi.mock factory // which is hoisted before all imports. -const { mockShutdown, mockRegisterPool, mockInstall, MockWorkBus, MockRegistry } = vi.hoisted( - () => { +const { mockShutdown, mockRegisterPool, mockHasPool, mockInstall, MockWorkBus, MockRegistry } = + vi.hoisted(() => { const mockShutdown = vi.fn().mockResolvedValue(undefined); const mockRegisterPool = vi.fn(); + // QNBS-v3: [Defaults to true (pool present) so existing tests that don't care about + // re-registration behavior are unaffected; the dedicated ensureDuckDbPool tests + // override this per-case.] + const mockHasPool = vi.fn(() => true); const mockInstall = vi.fn(); // QNBS-v3: Regular function (not arrow) so new MockWorkBus() works as a constructor. @@ -15,6 +19,7 @@ const { mockShutdown, mockRegisterPool, mockInstall, MockWorkBus, MockRegistry } const MockWorkBus = vi.fn(function (this: Record) { this['shutdown'] = mockShutdown; this['registerPool'] = mockRegisterPool; + this['hasPool'] = mockHasPool; this['enqueue'] = vi.fn(); this['cancel'] = vi.fn(() => true); this['getTelemetry'] = vi.fn(() => ({ @@ -39,9 +44,8 @@ const { mockShutdown, mockRegisterPool, mockInstall, MockWorkBus, MockRegistry } this['install'] = mockInstall; }); - return { mockShutdown, mockRegisterPool, mockInstall, MockWorkBus, MockRegistry }; - }, -); + return { mockShutdown, mockRegisterPool, mockHasPool, mockInstall, MockWorkBus, MockRegistry }; + }); vi.mock('@domain/worker-bus', () => ({ WorkerBus: MockWorkBus, @@ -69,6 +73,8 @@ describe('workerBusManager', () => { MockRegistry.mockClear(); mockShutdown.mockClear(); mockRegisterPool.mockClear(); + mockHasPool.mockClear(); + mockHasPool.mockReturnValue(true); mockInstall.mockClear(); }); @@ -143,4 +149,45 @@ describe('workerBusManager', () => { await initWorkerBus(); expect(getLegacyAdapter()).not.toBeNull(); }); + + describe('ensureDuckDbPool', () => { + it('initializes the bus when not yet running', async () => { + const { ensureDuckDbPool, isWorkerBusReady } = await import( + '../../services/workerBusManager' + ); + const bus = await ensureDuckDbPool(); + expect(bus).not.toBeNull(); + expect(isWorkerBusReady()).toBe(true); + }); + + it('does not re-register the pool when it is already present', async () => { + const { initWorkerBus, ensureDuckDbPool } = await import('../../services/workerBusManager'); + await initWorkerBus(); + mockRegisterPool.mockClear(); + mockHasPool.mockReturnValue(true); + + await ensureDuckDbPool(); + + expect(mockHasPool).toHaveBeenCalledWith('duckdb'); + expect(mockRegisterPool).not.toHaveBeenCalled(); + }); + + it('re-registers the duckdb pool when the bus is alive but the pool was removed', async () => { + // QNBS-v3: [Simulates the gap terminatePool('duckdb') can leave — the bus stays non-null + // but the pool is gone; ensureDuckDbPool must not just trust `_bus !== null`.] + const { initWorkerBus, ensureDuckDbPool } = await import('../../services/workerBusManager'); + await initWorkerBus(); + mockRegisterPool.mockClear(); + mockHasPool.mockReturnValue(false); + + const bus = await ensureDuckDbPool(); + + expect(bus).not.toBeNull(); + expect(mockRegisterPool).toHaveBeenCalledWith( + 'duckdb', + expect.arrayContaining(['db.duckdb']), + expect.objectContaining({ workerScript: expect.stringContaining('duckdb.worker') }), + ); + }); + }); }); diff --git a/workers/v2/duckdb.worker.ts b/workers/v2/duckdb.worker.ts index d7a5880a..56c89b2c 100644 --- a/workers/v2/duckdb.worker.ts +++ b/workers/v2/duckdb.worker.ts @@ -1,15 +1,15 @@ /// -// QNBS-v3: WorkerBus v2 DuckDB worker. Wraps legacy duckdbWorker.ts logic -// in the typed bootstrap protocol. +// QNBS-v3: [Sole DuckDB worker generation post-consolidation (docs/adr/0014); formerly wrapped the now-deleted legacy workers/duckdbWorker.ts.] +import type { AsyncDuckDB, AsyncDuckDBConnection } from '@duckdb/duckdb-wasm'; import { registerTaskHandler, type WorkerHandlerContext, } from '../../packages/worker-bus/src/workerBootstrap'; let duckdbModule: typeof import('@duckdb/duckdb-wasm') | null = null; -let db: import('@duckdb/duckdb-wasm').AsyncDuckDB | null = null; -let connection: import('@duckdb/duckdb-wasm').AsyncDuckDBConnection | null = null; +let db: AsyncDuckDB | null = null; +let connection: AsyncDuckDBConnection | null = null; async function getDuckDb() { if (!duckdbModule) { @@ -29,7 +29,7 @@ async function isOPFSSupported(): Promise { } } -// QNBS-v3 (F-09): self-hosted, same-origin DuckDB-WASM assets — see workers/duckdbWorker.ts for the full rationale (unpinned CDN URL, supply-chain trust, already CSP-dead code). +// QNBS-v3 (F-09): [Self-hosted, same-origin DuckDB-WASM assets — replaces an unpinned CDN URL that was already CSP-dead (supply-chain trust); see docs/adr/0013-csp-wasm-and-blob-frames.md.] const DUCKDB_ASSET_BASE = `${import.meta.env.BASE_URL}duckdb/`; const SELF_HOSTED_BUNDLES = { mvp: { @@ -58,7 +58,7 @@ export async function initDuckDb( const useOpfs = await isOPFSSupported(); if (useOpfs) { // QNBS-v3: [Tracked separately from `connection` so a failed ATTACH can close this partial connection instead of leaking it.] - let opfsConnection: import('@duckdb/duckdb-wasm').AsyncDuckDBConnection | null = null; + let opfsConnection: AsyncDuckDBConnection | null = null; try { const { DuckDBDataProtocol } = await getDuckDb(); const opfsRoot = await navigator.storage.getDirectory(); @@ -98,19 +98,50 @@ export async function initDuckDb( db = newDb; } +interface DuckDbQueryResult { + toArray(): Array<{ toJSON(): Record }>; +} + +interface PreparableConnection { + prepare(sql: string): Promise<{ + query(...params: unknown[]): Promise; + close(): Promise; + }>; +} + +// QNBS-v3: [tsgo doesn't resolve AsyncDuckDBConnection.prepare() through this package's nested +// `export *` chain (present and correct in @duckdb/duckdb-wasm's own .d.ts, verified +// directly) — a narrow structural type sidesteps the gap, the same class of tsgo/external- +// package resolution issue CLAUDE.md documents for the transformers.js path alias.] +function asPreparable(conn: AsyncDuckDBConnection): PreparableConnection { + return conn as unknown as PreparableConnection; +} + +// QNBS-v3: [Bind via a prepared statement when params are present — a raw connection.query(sql) silently drops them, leaving an unbound query and a SQL-injection surface.] +async function runSql(sql: string, params?: readonly unknown[]): Promise { + if (!connection) throw new Error('DuckDB not initialized'); + if (params && params.length > 0) { + const stmt = await asPreparable(connection).prepare(sql); + try { + return await stmt.query(...params); + } finally { + await stmt.close(); + } + } + return connection.query(sql); +} + export async function handleQuery(ctx: WorkerHandlerContext): Promise { const { payload } = ctx; - const req = payload as { sql?: string }; - if (!connection) throw new Error('DuckDB not initialized'); - const result = await connection.query(req.sql ?? ''); + const req = payload as { sql?: string; params?: readonly unknown[] }; + const result = await runSql(req.sql ?? '', req.params); return result.toArray().map((row: { toJSON(): Record }) => row.toJSON()); } export async function handleExec(ctx: WorkerHandlerContext): Promise { const { payload } = ctx; - const req = payload as { sql?: string }; - if (!connection) throw new Error('DuckDB not initialized'); - await connection.query(req.sql ?? ''); + const req = payload as { sql?: string; params?: readonly unknown[] }; + await runSql(req.sql ?? '', req.params); return { ok: true }; } From 776ff4e6a6a9a3221424e50255ee734bb3a74474 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:31:06 +0200 Subject: [PATCH 07/10] fix(ai): add the missing ensureInferencePool export that broke prod builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of both Vercel deployment failures on this branch/PR chain: services/ai/localEmbeddingService.ts imports and calls ensureInferencePool() from workerBusManager.ts, but that function was never actually added there — only ensureDuckDbPool() was. Vitest never caught this because localEmbeddingService.test.ts mocks the entire workerBusManager module (vi.mock(...) replaces it wholesale), so the real file's missing export was invisible to that suite. tsgo's typecheck also passed clean locally for reasons still unclear (worth a follow-up look), but rolldown's stricter static bundling in the actual `pnpm run build:edge` (Vercel's exact build command) correctly failed hard with [MISSING_EXPORT] — reproduced locally with `NODE_OPTIONS=--max-old-space-size=3072 pnpm run build:edge`, which is what actually surfaced this. Added ensureInferencePool(), mirroring ensureDuckDbPool()'s shape (decoupled from enableWorkerBusV2, re-registers the pool if it was removed via terminatePool() while the bus stays alive). Refactored doInitWorkerBus()'s inline 'inference' pool registration to share the same inferencePoolOptions() the new function uses, avoiding the same kind of drift the duckdb pool options already guard against. New tests in workerBusManager.test.ts import the *real* module (only @domain/worker-bus is mocked) — this exact suite would have caught the missing export immediately, unlike the fully-mocked consumer-side test. Verified fixed via a full local `pnpm run build:edge` reproduction of Vercel's exact build command (previously failed with [MISSING_EXPORT], now builds clean). Co-Authored-By: Claude Sonnet 5 --- services/workerBusManager.ts | 56 +++++++++++++++++++++-------- tests/unit/workerBusManager.test.ts | 47 ++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/services/workerBusManager.ts b/services/workerBusManager.ts index 7fe74030..b2223c21 100644 --- a/services/workerBusManager.ts +++ b/services/workerBusManager.ts @@ -67,13 +67,35 @@ async function reRegisterDuckDbPool(bus: WorkerBus): Promise { bus.registerPool('duckdb', options.capabilities, options); } +// QNBS-v3: [Shared with reRegisterInferencePool() below, same rationale as duckDbPoolOptions().] +async function inferencePoolOptions() { + const { MAX_WORKERS_INFERENCE, MIN_WORKERS, WORKER_IDLE_TIMEOUT_MS } = await import( + '@domain/worker-bus' + ); + return { + // QNBS-v3: [Capped below MAX_WORKERS_INFERENCE — each replica loads its own transformers.js pipeline (no cross-replica cache sharing), so 4 concurrent workers could mean 4x the model memory footprint under a burst.] + maxWorkers: Math.min(2, MAX_WORKERS_INFERENCE), + minWorkers: MIN_WORKERS, + idleTimeoutMs: WORKER_IDLE_TIMEOUT_MS, + workerScript: new URL('../workers/v2/inference.worker.ts', import.meta.url).href, + capabilities: ['inference.text', 'inference.embed'] as const, + labels: { pool: 'inference', version: 'v2' }, + }; +} + +/** Re-register the 'inference' pool if it was removed via terminatePool() — a no-op if already present. */ +async function reRegisterInferencePool(bus: WorkerBus): Promise { + if (bus.hasPool('inference')) return; + const options = await inferencePoolOptions(); + bus.registerPool('inference', options.capabilities, options); +} + async function doInitWorkerBus(): Promise { try { const { WorkerBus, WorkerRegistry, MAX_WORKERS_INFERENCE, - MIN_WORKERS, WORKER_IDLE_TIMEOUT_MS, CIRCUIT_BREAKER_THRESHOLD, CIRCUIT_BREAKER_RECOVERY_MS, @@ -96,25 +118,15 @@ async function doInitWorkerBus(): Promise { const registry = new WorkerRegistry(); - // QNBS-v3: new URL(path, import.meta.url) lets Vite emit the worker script as a proper - // asset URL. The .ts extension is allowed — Vite transforms it during build. - const inferenceUrl = new URL('../workers/v2/inference.worker.ts', import.meta.url).href; // QNBS-v3: P1-1 — dedicated WebLLM (WebGPU) worker. Separate pool keeps @mlc-ai/web-llm out // of the transformers.js worker bundle and isolates the GPU lifecycle. const webllmUrl = new URL('../workers/v2/webllm.worker.ts', import.meta.url).href; + const inferenceOptions = await inferencePoolOptions(); registry.register({ poolId: 'inference', - capabilities: ['inference.text', 'inference.embed'], - options: { - // QNBS-v3: [Capped below MAX_WORKERS_INFERENCE — each replica loads its own transformers.js pipeline (no cross-replica cache sharing), so 4 concurrent workers could mean 4x the model memory footprint under a burst.] - maxWorkers: Math.min(2, MAX_WORKERS_INFERENCE), - minWorkers: MIN_WORKERS, - idleTimeoutMs: WORKER_IDLE_TIMEOUT_MS, - workerScript: inferenceUrl, - capabilities: ['inference.text', 'inference.embed'], - labels: { pool: 'inference', version: 'v2' }, - }, + capabilities: inferenceOptions.capabilities, + options: inferenceOptions, }); const duckdbOptions = await duckDbPoolOptions(); @@ -195,6 +207,22 @@ export async function ensureDuckDbPool(): Promise { return _bus; } +/** + * Ensure the shared local-inference worker pool is available, initializing the WorkerBus on + * demand. QNBS-v3: mirrors ensureDuckDbPool()/ensureWebLlmPool()'s decoupling from + * `enableWorkerBusV2` — embeddings/NLP were never gated by that flag in the v1 worker they + * replace, so toggling an experimental infra flag off must not silently break RAG/cross-project + * search. Returns null only if init failed. + */ +export async function ensureInferencePool(): Promise { + if (_bus === null) { + await initWorkerBus(); + return _bus; + } + await reRegisterInferencePool(_bus); + return _bus; +} + /** * Shut down the WorkerBus v2 and terminate all worker threads. * Called when the feature flag is disabled or the app unmounts. diff --git a/tests/unit/workerBusManager.test.ts b/tests/unit/workerBusManager.test.ts index 151f3717..7781e221 100644 --- a/tests/unit/workerBusManager.test.ts +++ b/tests/unit/workerBusManager.test.ts @@ -190,4 +190,51 @@ describe('workerBusManager', () => { ); }); }); + + describe('ensureInferencePool', () => { + // QNBS-v3: [Imports the real module (only @domain/worker-bus is mocked) — unlike + // localEmbeddingService.test.ts's fully-mocked workerBusManager, this actually + // exercises the export surface. This exact suite would have caught PR #288's + // missing-export bug (ensureInferencePool was imported but never defined).] + it('initializes the bus when not yet running', async () => { + const { ensureInferencePool, isWorkerBusReady } = await import( + '../../services/workerBusManager' + ); + const bus = await ensureInferencePool(); + expect(bus).not.toBeNull(); + expect(isWorkerBusReady()).toBe(true); + }); + + it('does not re-register the pool when it is already present', async () => { + const { initWorkerBus, ensureInferencePool } = await import( + '../../services/workerBusManager' + ); + await initWorkerBus(); + mockRegisterPool.mockClear(); + mockHasPool.mockReturnValue(true); + + await ensureInferencePool(); + + expect(mockHasPool).toHaveBeenCalledWith('inference'); + expect(mockRegisterPool).not.toHaveBeenCalled(); + }); + + it('re-registers the inference pool when the bus is alive but the pool was removed', async () => { + const { initWorkerBus, ensureInferencePool } = await import( + '../../services/workerBusManager' + ); + await initWorkerBus(); + mockRegisterPool.mockClear(); + mockHasPool.mockReturnValue(false); + + const bus = await ensureInferencePool(); + + expect(bus).not.toBeNull(); + expect(mockRegisterPool).toHaveBeenCalledWith( + 'inference', + expect.arrayContaining(['inference.text', 'inference.embed']), + expect.objectContaining({ workerScript: expect.stringContaining('inference.worker') }), + ); + }); + }); }); From 451918035f517bc5d72858f016480690dae4fa55 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:51:37 +0200 Subject: [PATCH 08/10] feat(ai): migrate localNlpService onto WorkerBus v2, delete v1 inference worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth and final migration PR of the worker-generation consolidation (ADR-0014). services/ai/localNlpService.ts's public API (analyzeSentiment/summarizeText/classifyWritingTopic) is unchanged — confirmed zero real production callers beyond its own tests and a barrel re-export in services/ai/index.ts, so this is the lowest-blast-radius of the three migrations. Internals swapped from a dedicated `new Worker(inference.worker.ts)` instance to the shared WorkerBus v2 'inference' pool via ensureInferencePool() (added in the previous PR), routing to the 'inference.text' capability. analyzeSentiment/summarizeText already had their own app-level graceful-degrade behavior on failure (NEUTRAL sentiment / truncated text) — that's preserved unchanged via try/catch around the new throw-on-failure requestInference() call, replacing the old ok:false-check. With both localEmbeddingService.ts (previous PR) and this migrated, workers/inference.worker.ts (v1) has no remaining callers — deleted, along with its dedicated test (tests/unit/inferenceWorker.test.ts). tests/unit/localNlpService.test.ts rewritten against a workerBusManager mock; tests/unit/services/localNlpService.test.ts deleted as a near-total duplicate of the rewritten suite (same module, overlapping classifyWritingTopic/analyzeSentiment coverage, the comprehensive file already had everything it did). Verified via typecheck and a full local `pnpm run build:edge` reproduction of Vercel's exact build command (the lesson from the previous PR's missing-export incident) — both clean. Co-Authored-By: Claude Sonnet 5 --- services/ai/localNlpService.ts | 98 ++++------- services/ai/pipelineLruCache.ts | 6 +- services/voice/wasmSttEngine.ts | 2 +- tests/unit/inferenceWorker.test.ts | 151 ---------------- tests/unit/localNlpService.test.ts | 185 ++++++++------------ tests/unit/services/localNlpService.test.ts | 110 ------------ workers/inference.worker.ts | 174 ------------------ 7 files changed, 119 insertions(+), 607 deletions(-) delete mode 100644 tests/unit/inferenceWorker.test.ts delete mode 100644 tests/unit/services/localNlpService.test.ts delete mode 100644 workers/inference.worker.ts diff --git a/services/ai/localNlpService.ts b/services/ai/localNlpService.ts index 825bbccd..c3e1db21 100644 --- a/services/ai/localNlpService.ts +++ b/services/ai/localNlpService.ts @@ -1,6 +1,6 @@ -// QNBS-v3: Local NLP service — sentiment, summarization, topic classification via inference.worker.ts. -// Adapted from CannaGuide-2025 nlpService.ts patterns for creative-writing context. -// All inference runs off-main-thread; falls back gracefully when worker unavailable. +// QNBS-v3: [Local NLP service — sentiment/summarization via workers/v2/inference.worker.ts through WorkerBus v2 (docs/adr/0014 migration).] + +import { ensureInferencePool } from '../workerBusManager'; export interface SentimentResult { label: 'POSITIVE' | 'NEGATIVE' | 'NEUTRAL'; @@ -12,74 +12,59 @@ const SENTIMENT_MODEL = 'Xenova/distilbert-base-uncased-finetuned-sst-2-english' const SUMMARIZATION_MODEL = 'Xenova/distilbart-cnn-6-6'; const MAX_SUMMARY_TOKENS = 150; -let workerInstance: Worker | null = null; - -function getWorker(): Worker { - if (!workerInstance) { - workerInstance = new Worker(new URL('../../workers/inference.worker.ts', import.meta.url), { - type: 'module', - }); - } - return workerInstance; -} - -function postToWorker( +async function requestInference( task: string, modelId: string, input: string, inferenceOptions?: Record, -): Promise<{ ok: boolean; result?: string; error?: string }> { - return new Promise((resolve) => { - const messageId = `nlp-${Date.now()}-${Math.random().toString(36).slice(2, 7)}`; - const worker = getWorker(); - - const handler = (event: MessageEvent) => { - const data = event.data as { - messageId: string; - ok: boolean; - result?: string; - error?: string; - }; - if (data.messageId !== messageId) return; - worker.removeEventListener('message', handler); - resolve(data); - }; - - worker.addEventListener('message', handler); - worker.postMessage({ messageId, task, modelId, input, inferenceOptions }); - }); +): Promise { + const bus = await ensureInferencePool(); + if (!bus) throw new Error('WorkerBus v2 unavailable'); + const handle = bus.enqueue< + { task: string; modelId: string; input: string; inferenceOptions?: Record }, + string + >( + 'inference.text', + { task, modelId, input, inferenceOptions }, + { capabilities: ['inference.text'] }, + ); + return handle.result; } export async function analyzeSentiment(text: string): Promise { const capped = text.slice(0, 512); // model input limit - const response = await postToWorker('sentiment-analysis', SENTIMENT_MODEL, capped); - if (!response.ok || !response.result) { - return { label: 'NEUTRAL', score: 0.5, normalized: 0 }; - } + try { + const result = await requestInference('sentiment-analysis', SENTIMENT_MODEL, capped); - // Worker returns "LABEL:score" string (see inference.worker.ts sentiment handler) - const [labelRaw, scoreRaw] = response.result.split(':'); - const rawLabel = (labelRaw ?? 'NEUTRAL').toUpperCase(); - const score = parseFloat(scoreRaw ?? '0.5'); + // Worker returns "LABEL:score" string (see workers/v2/inference.worker.ts sentiment branch) + const [labelRaw, scoreRaw] = result.split(':'); + const rawLabel = (labelRaw ?? 'NEUTRAL').toUpperCase(); + const score = parseFloat(scoreRaw ?? '0.5'); - const label: SentimentResult['label'] = - rawLabel === 'POSITIVE' ? 'POSITIVE' : rawLabel === 'NEGATIVE' ? 'NEGATIVE' : 'NEUTRAL'; + const label: SentimentResult['label'] = + rawLabel === 'POSITIVE' ? 'POSITIVE' : rawLabel === 'NEGATIVE' ? 'NEGATIVE' : 'NEUTRAL'; - const normalized = label === 'POSITIVE' ? score : label === 'NEGATIVE' ? -score : 0; + const normalized = label === 'POSITIVE' ? score : label === 'NEGATIVE' ? -score : 0; - return { label, score, normalized }; + return { label, score, normalized }; + } catch { + // QNBS-v3: [Graceful degrade — same fallback v1 returned on any worker failure.] + return { label: 'NEUTRAL', score: 0.5, normalized: 0 }; + } } export async function summarizeText(text: string, maxLength = MAX_SUMMARY_TOKENS): Promise { const capped = text.slice(0, 1024); - const response = await postToWorker('summarization', SUMMARIZATION_MODEL, capped, { - max_new_tokens: maxLength, - do_sample: false, - }); - if (!response.ok || !response.result) return text.slice(0, 280); // graceful degrade - return response.result; + try { + return await requestInference('summarization', SUMMARIZATION_MODEL, capped, { + max_new_tokens: maxLength, + do_sample: false, + }); + } catch { + return text.slice(0, 280); // graceful degrade + } } // QNBS-v3: Zero-shot topic classification for creative-writing genres using sentiment model heuristic. @@ -108,10 +93,3 @@ export async function classifyWritingTopic(text: string): Promise { return bestGenre; } - -export function _resetWorkerForTest(): void { - if (workerInstance) { - workerInstance.terminate(); - workerInstance = null; - } -} diff --git a/services/ai/pipelineLruCache.ts b/services/ai/pipelineLruCache.ts index 029b6ee5..7aac4ad9 100644 --- a/services/ai/pipelineLruCache.ts +++ b/services/ai/pipelineLruCache.ts @@ -1,8 +1,8 @@ /** * Shared LRU cache for loaded inference pipelines (transformers.js / ONNX). - * QNBS-v3: Phase 2.3 — extracted from the byte-identical LRU loops that were duplicated in - * `workers/inference.worker.ts` and `workers/v2/inference.worker.ts`. Two real fixes - * over the inlined version: + * QNBS-v3: Phase 2.3 — extracted from byte-identical LRU loops duplicated across the v1/v2 worker + * generations (v1 since deleted, docs/adr/0014); only `workers/v2/inference.worker.ts` + * uses this now. Two real fixes over the inlined version: * 1. **dispose-on-evict** — evicted pipelines now `dispose()`, closing the VRAM/RAM * leak (same bug-class as the WebLLM eviction fix in AUDIT 2026-06-01 #1). * 2. **in-flight dedup** — concurrent loads of the same key share one promise, so the diff --git a/services/voice/wasmSttEngine.ts b/services/voice/wasmSttEngine.ts index 6a044f21..a7077b21 100644 --- a/services/voice/wasmSttEngine.ts +++ b/services/voice/wasmSttEngine.ts @@ -61,7 +61,7 @@ export class WasmSttEngine implements SttEngine { typedEnv.backends.onnx.wasm.proxy = false; } // QNBS-v3: cast pipeline to a loose signature — v3's typed overload union is too large to - // represent (TS2590). Same pattern as workers/inference.worker.ts loadPipeline(). + // represent (TS2590). Same pattern as workers/v2/inference.worker.ts's loadPipeline(). const createPipeline = pipeline as ( task: string, model: string, diff --git a/tests/unit/inferenceWorker.test.ts b/tests/unit/inferenceWorker.test.ts deleted file mode 100644 index 72e55b1b..00000000 --- a/tests/unit/inferenceWorker.test.ts +++ /dev/null @@ -1,151 +0,0 @@ -import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; - -// QNBS-v3: Relative path mock — same resolution trick as aiCoreFallbackPaths.test.ts. -const mockPipelineFn = vi.hoisted(() => vi.fn()); - -vi.mock( - '../../packages/ai-core/node_modules/@huggingface/transformers/dist/transformers.web.js', - () => ({ - pipeline: mockPipelineFn, - }), -); - -// Simulate worker globals. postMessages are pushed to `posted` for assertion. -const posted: unknown[] = []; -const messageListeners: Array<(e: MessageEvent) => void> = []; - -vi.stubGlobal('self', { - addEventListener: (_type: string, handler: (e: MessageEvent) => void) => { - messageListeners.push(handler); - }, - postMessage: (msg: unknown) => { - posted.push(msg); - }, - location: { origin: '' }, -}); -vi.stubGlobal('navigator', { hardwareConcurrency: 4 }); - -function sendMessage(data: unknown) { - const event = { data, origin: '' } as MessageEvent; - for (const listener of messageListeners) listener(event); -} - -// Wait for a message to appear in `posted` -function waitForResponse(timeout = 5000): Promise { - return new Promise((resolve, reject) => { - const deadline = Date.now() + timeout; - const check = () => { - if (posted.length > 0) { - resolve(posted.shift()); - } else if (Date.now() > deadline) { - reject(new Error('waitForResponse timeout')); - } else { - setTimeout(check, 20); - } - }; - check(); - }); -} - -beforeAll(async () => { - await import('../../workers/inference.worker'); - await vi.waitFor(() => messageListeners.length > 0, { timeout: 2000 }); -}); - -describe('inference.worker', () => { - beforeEach(() => { - posted.length = 0; - mockPipelineFn.mockReset(); - }); - - afterEach(() => { - posted.length = 0; - }); - - it('registers a message listener on import', () => { - expect(messageListeners.length).toBeGreaterThan(0); - }); - - it('returns inference result for text-generation', async () => { - const mockPipe = vi.fn().mockResolvedValue([{ generated_text: 'Once upon a time' }]); - mockPipelineFn.mockResolvedValue(mockPipe); - - sendMessage({ - messageId: 'msg-1', - task: 'text-generation', - modelId: 'test-model-textgen-001', - input: 'Once', - }); - - const response = (await waitForResponse()) as { - messageId: string; - ok: boolean; - result: string; - }; - expect(response.messageId).toBe('msg-1'); - expect(response.ok).toBe(true); - expect(response.result).toBe('Once upon a time'); - }); - - it('returns feature-extraction result as number[]', async () => { - const vec = new Float32Array([0.1, 0.2, 0.3]); - const mockPipe = vi.fn().mockResolvedValue(vec); - mockPipelineFn.mockResolvedValue(mockPipe); - - sendMessage({ - messageId: 'emb-1', - task: 'feature-extraction', - modelId: 'test-model-embedding-001', - input: 'Hello world', - }); - - const response = (await waitForResponse()) as { ok: boolean; result: number[] }; - expect(response.ok).toBe(true); - expect(Array.isArray(response.result)).toBe(true); - expect((response.result as number[]).length).toBe(3); - }); - - it('returns error response when pipeline throws', async () => { - mockPipelineFn.mockRejectedValue(new Error('OOM error')); - - sendMessage({ - messageId: 'err-1', - task: 'text-generation', - modelId: 'test-model-error-001', - input: 'test', - }); - - const response = (await waitForResponse()) as { ok: boolean; error: string }; - expect(response.ok).toBe(false); - expect(response.error).toContain('OOM'); - }); - - it('returns sentiment label for sentiment-analysis task', async () => { - const mockPipe = vi.fn().mockResolvedValue([{ label: 'POSITIVE', score: 0.9932 }]); - mockPipelineFn.mockResolvedValue(mockPipe); - - sendMessage({ - messageId: 'sent-1', - task: 'sentiment-analysis', - modelId: 'test-model-sentiment-001', - input: 'I love writing!', - }); - - const response = (await waitForResponse()) as { ok: boolean; result: string }; - expect(response.ok).toBe(true); - expect(response.result).toMatch(/POSITIVE/); - }); - - it('ignores WORKER_CANCEL messages without posting a response', async () => { - sendMessage({ type: 'WORKER_CANCEL', messageId: 'nonexistent-cancel-test' }); - await new Promise((r) => setTimeout(r, 100)); - expect(posted).toHaveLength(0); - }); - - it('responds with WORKER_PONG when receiving WORKER_PING', async () => { - sendMessage({ type: 'WORKER_PING' }); - const response = (await waitForResponse()) as { type: string; ts: number }; - expect(response.type).toBe('WORKER_PONG'); - expect(typeof response.ts).toBe('number'); - }); -}); diff --git a/tests/unit/localNlpService.test.ts b/tests/unit/localNlpService.test.ts index 8856b01d..08b9515e 100644 --- a/tests/unit/localNlpService.test.ts +++ b/tests/unit/localNlpService.test.ts @@ -1,163 +1,132 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -// QNBS-v3: Arrow functions cannot be constructors — use hoisted vi.fn() refs + class stub. -const workerPostMessage = vi.hoisted(() => vi.fn()); -const workerAddEventListener = vi.hoisted(() => vi.fn()); -const workerRemoveEventListener = vi.hoisted(() => vi.fn()); -const workerTerminate = vi.hoisted(() => vi.fn()); - -// QNBS-v3: Class-based Worker stub so `new Worker(...)` in source doesn't throw. -class MockWorker { - postMessage = workerPostMessage; - addEventListener = workerAddEventListener; - removeEventListener = workerRemoveEventListener; - terminate = workerTerminate; -} +import { beforeEach, describe, expect, it, vi } from 'vitest'; -vi.stubGlobal('Worker', MockWorker); +// QNBS-v3: [localNlpService now routes through WorkerBus v2 — mock ensureInferencePool() instead of the global Worker constructor. Consolidates the former tests/unit/services/localNlpService.test.ts, which duplicated most of this coverage.] -import { - _resetWorkerForTest, - analyzeSentiment, - classifyWritingTopic, - summarizeText, -} from '../../services/ai/localNlpService'; +const { mockEnqueue, mockEnsureInferencePool } = vi.hoisted(() => ({ + mockEnqueue: vi.fn(), + mockEnsureInferencePool: vi.fn(), +})); -beforeEach(() => { - _resetWorkerForTest(); - workerPostMessage.mockReset(); - workerAddEventListener.mockReset(); - workerRemoveEventListener.mockReset(); -}); +vi.mock('../../services/workerBusManager', () => ({ + ensureInferencePool: mockEnsureInferencePool, +})); -afterEach(() => { - _resetWorkerForTest(); -}); +const { analyzeSentiment, classifyWritingTopic, summarizeText } = await import( + '../../services/ai/localNlpService' +); -// Helper: fire the message handler captured by the last addEventListener call -function resolveWorkerMessage(responseOverride?: { - ok?: boolean; - result?: string | undefined; - error?: string; -}) { - const [, handler] = - workerAddEventListener.mock.calls[workerAddEventListener.mock.calls.length - 1] ?? []; - if (typeof handler !== 'function') throw new Error('No handler registered'); - - const lastMsg = workerPostMessage.mock.calls[workerPostMessage.mock.calls.length - 1]?.[0] as { - messageId: string; - }; - - handler({ - data: { - messageId: lastMsg?.messageId ?? 'nlp-test', - ok: true, - result: 'POSITIVE:0.95', - ...responseOverride, - }, - } as MessageEvent); +function makeHandle(result: Promise) { + return { taskId: 't1', result, progress: (async function* () {})(), cancel: vi.fn() }; } -// ─── analyzeSentiment ────────────────────────────────────────────────────────── +function makeBus() { + return { enqueue: mockEnqueue }; +} + +let requestCalls: Array<{ + task: string; + modelId: string; + input: string; + inferenceOptions?: Record; +}> = []; +let nextResult = 'POSITIVE:0.95'; + +beforeEach(() => { + vi.clearAllMocks(); + requestCalls = []; + nextResult = 'POSITIVE:0.95'; + mockEnsureInferencePool.mockResolvedValue(makeBus()); + mockEnqueue.mockImplementation((_taskType: string, payload: unknown) => { + requestCalls.push(payload as (typeof requestCalls)[number]); + return makeHandle(Promise.resolve(nextResult)); + }); +}); + +// ─── analyzeSentiment ──────────────────────────────────────────────────────── describe('analyzeSentiment', () => { it('parses POSITIVE result correctly', async () => { - const promise = analyzeSentiment('I love writing!'); - resolveWorkerMessage({ result: 'POSITIVE:0.95' }); - const r = await promise; + nextResult = 'POSITIVE:0.95'; + const r = await analyzeSentiment('I love writing!'); expect(r.label).toBe('POSITIVE'); expect(r.score).toBeCloseTo(0.95, 3); expect(r.normalized).toBeCloseTo(0.95, 3); }); it('parses NEGATIVE result correctly', async () => { - const promise = analyzeSentiment('I hate this.'); - resolveWorkerMessage({ result: 'NEGATIVE:0.88' }); - const r = await promise; + nextResult = 'NEGATIVE:0.88'; + const r = await analyzeSentiment('I hate this.'); expect(r.label).toBe('NEGATIVE'); expect(r.score).toBeCloseTo(0.88, 3); expect(r.normalized).toBeCloseTo(-0.88, 3); }); it('parses NEUTRAL result with normalized = 0', async () => { - const promise = analyzeSentiment('The document exists.'); - resolveWorkerMessage({ result: 'NEUTRAL:0.6' }); - const r = await promise; + nextResult = 'NEUTRAL:0.6'; + const r = await analyzeSentiment('The document exists.'); expect(r.label).toBe('NEUTRAL'); expect(r.normalized).toBe(0); }); - it('returns NEUTRAL fallback on worker error', async () => { - const promise = analyzeSentiment('anything'); - resolveWorkerMessage({ ok: false, result: undefined, error: 'OOM' }); - const r = await promise; + it('returns NEUTRAL fallback when the task rejects', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(new Error('OOM')))); + const r = await analyzeSentiment('anything'); expect(r.label).toBe('NEUTRAL'); expect(r.score).toBe(0.5); expect(r.normalized).toBe(0); }); - it('returns NEUTRAL fallback on empty result', async () => { - const promise = analyzeSentiment('anything'); - resolveWorkerMessage({ ok: true, result: undefined }); - const r = await promise; - expect(r.label).toBe('NEUTRAL'); - }); - - it('caps input text to 512 chars before sending', async () => { - const promise = analyzeSentiment('x'.repeat(600)); - resolveWorkerMessage(); - await promise; - const posted = workerPostMessage.mock.calls[0]?.[0] as { input: string }; - expect(posted.input.length).toBe(512); + it('caps input text to 512 chars before enqueuing', async () => { + await analyzeSentiment('x'.repeat(600)); + expect(requestCalls[0]?.input.length).toBe(512); }); - it('maps unknown label to NEUTRAL', async () => { - const promise = analyzeSentiment('weird text'); - resolveWorkerMessage({ result: 'GARBAGE:0.7' }); - const r = await promise; + it('maps an unknown label to NEUTRAL', async () => { + nextResult = 'GARBAGE:0.7'; + const r = await analyzeSentiment('weird text'); expect(r.label).toBe('NEUTRAL'); expect(r.normalized).toBe(0); }); + + it('enqueues inference.text with the inference.text capability', async () => { + await analyzeSentiment('test'); + expect(requestCalls[0]?.task).toBe('sentiment-analysis'); + expect(mockEnqueue).toHaveBeenCalledWith( + 'inference.text', + expect.anything(), + expect.objectContaining({ capabilities: ['inference.text'] }), + ); + }); }); -// ─── summarizeText ────────────────────────────────────────────────────────── +// ─── summarizeText ──────────────────────────────────────────────────────────── describe('summarizeText', () => { - it('returns worker result on success', async () => { - const promise = summarizeText('A long piece of text about storytelling and craft.'); - resolveWorkerMessage({ result: 'Short summary.' }); - const result = await promise; + it('returns the worker result on success', async () => { + nextResult = 'Short summary.'; + const result = await summarizeText('A long piece of text about storytelling and craft.'); expect(result).toBe('Short summary.'); }); - it('falls back to text.slice(0, 280) when worker fails', async () => { + it('falls back to text.slice(0, 280) when the task rejects', async () => { + mockEnqueue.mockReturnValue(makeHandle(Promise.reject(new Error('unavailable')))); const text = 'b'.repeat(400); - const promise = summarizeText(text); - resolveWorkerMessage({ ok: false, result: undefined }); - const result = await promise; + const result = await summarizeText(text); expect(result).toBe(text.slice(0, 280)); }); - it('caps input to 1024 chars before sending to worker', async () => { - const promise = summarizeText('c'.repeat(2000)); - resolveWorkerMessage({ result: 'ok' }); - await promise; - const posted = workerPostMessage.mock.calls[0]?.[0] as { input: string }; - expect(posted.input.length).toBe(1024); + it('caps input to 1024 chars before enqueuing', async () => { + await summarizeText('c'.repeat(2000)); + expect(requestCalls[0]?.input.length).toBe(1024); }); - it('passes max_new_tokens option from maxLength param', async () => { - const promise = summarizeText('some text', 200); - resolveWorkerMessage({ result: 'ok' }); - await promise; - const posted = workerPostMessage.mock.calls[0]?.[0] as { - inferenceOptions: { max_new_tokens: number }; - }; - expect(posted.inferenceOptions.max_new_tokens).toBe(200); + it('passes max_new_tokens from the maxLength param', async () => { + await summarizeText('some text', 200); + expect(requestCalls[0]?.inferenceOptions?.['max_new_tokens']).toBe(200); }); }); -// ─── classifyWritingTopic ──────────────────────────────────────────────────── +// ─── classifyWritingTopic ───────────────────────────────────────────────────── describe('classifyWritingTopic', () => { it('returns Fantasy for fantasy-keyword text', async () => { diff --git a/tests/unit/services/localNlpService.test.ts b/tests/unit/services/localNlpService.test.ts deleted file mode 100644 index 56e38911..00000000 --- a/tests/unit/services/localNlpService.test.ts +++ /dev/null @@ -1,110 +0,0 @@ -/** - * Tests for services/ai/localNlpService.ts - * QNBS-v3: Tests classifyWritingTopic (pure keyword matching) and the worker-path functions - * with a mocked Worker that returns controlled responses. - */ - -import { afterEach, describe, expect, it, vi } from 'vitest'; - -// --------------------------------------------------------------------------- -// Mock Worker -// --------------------------------------------------------------------------- - -class MockWorker { - private handlers: ((event: MessageEvent) => void)[] = []; - - postMessage(data: { messageId: string; task: string }) { - // Simulate async response - const responseData = { - messageId: data.messageId, - ok: true, - result: 'POSITIVE:0.9', - }; - setTimeout(() => { - for (const h of this.handlers) { - h(new MessageEvent('message', { data: responseData })); - } - }, 0); - } - - addEventListener(_type: string, handler: (event: MessageEvent) => void) { - this.handlers.push(handler); - } - - removeEventListener(_type: string, handler: (event: MessageEvent) => void) { - this.handlers = this.handlers.filter((h) => h !== handler); - } - - terminate() { - this.handlers = []; - } -} - -vi.stubGlobal('Worker', MockWorker); - -// --------------------------------------------------------------------------- -// Import after mocks -// --------------------------------------------------------------------------- - -import { - _resetWorkerForTest, - analyzeSentiment, - classifyWritingTopic, -} from '../../../services/ai/localNlpService'; - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe('classifyWritingTopic', () => { - it('classifies dragon-related text as Fantasy', async () => { - const result = await classifyWritingTopic('The dragon soared over the magical realm'); - expect(result).toBe('Fantasy'); - }); - - it('classifies space-related text as SciFi', async () => { - const result = await classifyWritingTopic('The spaceship launched into the alien galaxy'); - expect(result).toBe('SciFi'); - }); - - it('classifies thriller keywords', async () => { - const result = await classifyWritingTopic('The detective uncovered a conspiracy and murder'); - expect(result).toBe('Thriller'); - }); - - it('classifies romance keywords', async () => { - const result = await classifyWritingTopic( - 'Their love blossomed and they shared a kiss at the wedding', - ); - expect(result).toBe('Romance'); - }); - - it('returns General Fiction when no keywords match', async () => { - const result = await classifyWritingTopic('She walked to the store and bought bread.'); - expect(result).toBe('General Fiction'); - }); - - it('picks the genre with the most keyword matches', async () => { - // 2 Fantasy keywords vs 1 SciFi - const result = await classifyWritingTopic('A dragon and a wizard met a robot'); - expect(result).toBe('Fantasy'); - }); - - it('is case-insensitive', async () => { - const result = await classifyWritingTopic('DRAGON AND MAGIC'); - expect(result).toBe('Fantasy'); - }); -}); - -describe('analyzeSentiment', () => { - afterEach(() => { - _resetWorkerForTest(); - }); - - it('returns parsed sentiment from worker response', async () => { - const result = await analyzeSentiment('This is wonderful!'); - expect(result.label).toBe('POSITIVE'); - expect(result.score).toBe(0.9); - expect(result.normalized).toBeCloseTo(0.9); - }); -}); diff --git a/workers/inference.worker.ts b/workers/inference.worker.ts deleted file mode 100644 index 6ab6e34a..00000000 --- a/workers/inference.worker.ts +++ /dev/null @@ -1,174 +0,0 @@ -/// -// QNBS-v3: Off-main-thread inference worker for @huggingface/transformers v3 (ONNX backend). -// Uses WorkerBus request/response protocol with messageId correlation. -// Adapted from CannaGuide-2025 inference.worker.ts patterns for creative-writing tasks. - -import { PipelineLruCache } from '../services/ai/pipelineLruCache'; - -export type WorkerTaskType = - | 'text-generation' - | 'feature-extraction' - | 'sentiment-analysis' - | 'summarization'; - -export interface InferenceRequest { - messageId: string; - task: WorkerTaskType; - modelId: string; - input: string; - pipelineOptions?: { quantized?: boolean }; - inferenceOptions?: { - max_new_tokens?: number; - do_sample?: boolean; - temperature?: number; - return_full_text?: boolean; - }; -} - -export interface InferenceResponse { - messageId: string; - ok: boolean; - result?: string | number[]; - error?: string; - latencyMs?: number; -} - -// QNBS-v3: Phase 2.3 — shared LRU now disposes evicted pipelines (closes a VRAM leak) and dedups -// concurrent same-model loads. `dispose()` is best-effort; absent on some backends. -const pipelineCache = new PipelineLruCache({ - // QNBS-v3: return the (possibly async) dispose result; PipelineLruCache catches sync/async failure. - dispose: (pipe) => (pipe as { dispose?: () => void | Promise }).dispose?.(), -}); - -let transformersModule: { pipeline: (...args: unknown[]) => Promise } | null = null; - -// QNBS-v3: Lazy import — avoids loading the 2 MB transformers bundle until first inference request. -async function getTransformers() { - if (!transformersModule) { - // Dynamic import so the worker bundle is only loaded when actually needed. - const mod = await import('@huggingface/transformers'); - transformersModule = mod as unknown as typeof transformersModule; - } - return transformersModule!; -} - -// QNBS-v3: Security guard — only process messages from the same origin. -function isTrustedWorkerMessage(event: MessageEvent): boolean { - // In a dedicated worker, event.origin is '' (empty string) for same-origin posts. - // We reject messages with non-empty origins that don't match our scope. - return event.origin === '' || event.origin === globalThis.location?.origin; -} - -async function loadPipeline(task: WorkerTaskType, modelId: string, quantized = true) { - const cacheKey = `${task}::${modelId}`; - return pipelineCache.getOrLoad(cacheKey, async () => { - const { pipeline } = await getTransformers(); - // QNBS-v3: Device auto-selection — webgpu if available, else wasm. - const device = - typeof globalThis.navigator !== 'undefined' && 'gpu' in globalThis.navigator - ? 'webgpu' - : 'wasm'; - // QNBS-v3: transformers.js v3 replaced `quantized: boolean` with `dtype`; q8 ≈ old quantized=true. - return (pipeline as (task: string, model: string, opts: unknown) => Promise)( - task, - modelId, - { dtype: quantized ? 'q8' : 'fp32', device }, - ); - }); -} - -async function runInference( - req: InferenceRequest, - signal?: AbortSignal, -): Promise { - const start = Date.now(); - try { - if (signal?.aborted) throw new Error('Aborted'); - const pipe = await loadPipeline(req.task, req.modelId, req.pipelineOptions?.quantized ?? true); - if (signal?.aborted) throw new Error('Aborted'); - - const opts = req.inferenceOptions ?? {}; - let rawResult: unknown; - - if (req.task === 'feature-extraction') { - rawResult = await (pipe as (input: string, opts: unknown) => Promise)(req.input, { - pooling: 'mean', - normalize: true, - ...opts, - }); - } else { - rawResult = await (pipe as (input: string, opts: unknown) => Promise)( - req.input, - opts, - ); - } - - // Normalise result to string or number[] - let result: string | number[]; - if (req.task === 'feature-extraction') { - // Flatten Float32Array / nested array to plain number[] - const flat = rawResult as { data?: Float32Array } | Float32Array | number[]; - if (flat instanceof Float32Array) { - result = Array.from(flat); - } else if ( - 'data' in (flat as object) && - (flat as { data: Float32Array }).data instanceof Float32Array - ) { - result = Array.from((flat as { data: Float32Array }).data); - } else { - result = Array.from(flat as Iterable); - } - } else { - // text-generation, summarization, sentiment-analysis → extract text / label - const arr = rawResult as Array<{ - generated_text?: string; - summary_text?: string; - label?: string; - score?: number; - }>; - if (req.task === 'sentiment-analysis') { - result = `${arr[0]?.label ?? 'NEUTRAL'}:${(arr[0]?.score ?? 0).toFixed(4)}`; - } else { - result = arr[0]?.generated_text ?? arr[0]?.summary_text ?? ''; - } - } - - return { messageId: req.messageId, ok: true, result, latencyMs: Date.now() - start }; - } catch (err) { - return { - messageId: req.messageId, - ok: false, - error: err instanceof Error ? err.message : String(err), - latencyMs: Date.now() - start, - }; - } -} - -// QNBS-v3: In-flight AbortController map so callers can cancel long-running inference. -const abortMap = new Map(); - -self.addEventListener('message', (event: MessageEvent) => { - if (!isTrustedWorkerMessage(event)) return; - - const data = event.data as { type?: string; messageId?: string } & InferenceRequest; - - // QNBS-v3: Health check ping — reply with pong so the host can detect worker liveness. - if (data.type === 'WORKER_PING') { - self.postMessage({ type: 'WORKER_PONG', ts: Date.now() }); - return; - } - - if (data.type === 'WORKER_CANCEL' && data.messageId) { - abortMap.get(data.messageId)?.abort(); - abortMap.delete(data.messageId); - return; - } - - const controller = new AbortController(); - abortMap.set(data.messageId, controller); - - void runInference(data, controller.signal).then((response) => { - abortMap.delete(data.messageId); - self.postMessage(response); - }); -}); From aeaee844b9c8066a1eece1aaae72eb6d55468adc Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 23:10:23 +0200 Subject: [PATCH 09/10] docs: ADR-0015, CLAUDE.md/AGENTS.md doc-truth, TODO/AUDIT closeout, run log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes out the worker-generation consolidation sprint (PRs #286-288, #290) at the documentation layer. - New docs/adr/0015-worker-generation-consolidation.md — the decision record for the completed migration (target generation, per-workload changes, parity gaps found and fixed, deliberately-unchanged WebLLM-fallback-shape rationale, the timeoutMs follow-up). - docs/adr/0014-worker-generation-duplication.md: status line changed to "Superseded by 0015" (body untouched, per this repo's own "supersede, don't edit history" convention). - docs/adr/README.md: index gained rows 0010-0015 — 0010-0014 were pre-existing files missing from the index before this sprint, unrelated drift fixed opportunistically while editing the file. - CLAUDE.md/AGENTS.md: directory map, WorkerBus v2 section, DuckDB section, and Known Technical Debt all updated to the post-migration state. Also fixed 2 pre-existing doc-truth bugs found while reading the WorkerBus v2 source directly: WorkerHandlerContext.emitProgress is a flat function, not context.progress.emit(...); there is no 'background' priority tier, only critical > high > normal > low. - TODO.md: F-14 entry marked done with the PR chain and correction- loop findings. Also caught and fixed a genuinely stale, unrelated entry while in the file — "Tag v1.24.2 + publish release" was still marked open, but v1.24.2 was already tagged/released (verified via git tag/gh release list). - AUDIT.md: F-14 and F-11 (the same stale-tag issue) findings updated to resolved. - CHANGELOG.md: [Unreleased] section added, covering the full sprint (Changed/Fixed/Removed). - New docs/audit/WS-RUN-LOG-2026-07-29-worker-consolidation.md — execution log in this repo's established WS-RUN-LOG format: baseline, the 4-PR breakdown, every correction-loop finding with its fix, the Vercel missing-export incident, final verification, and what's deliberately left unresolved (timeoutMs enforcement; why local tsgo typecheck didn't catch the missing export). Verified: pnpm exec tsx scripts/audit-feature-parity.ts (0 drifts), checkDocMetrics.test.ts + featureCatalog.test.ts (28/28), typecheck clean. Co-Authored-By: Claude Sonnet 5 --- AGENTS.md | 15 +-- AUDIT.md | 4 +- CHANGELOG.md | 45 ++++++++ CLAUDE.md | 12 ++- TODO.md | 24 +++-- .../adr/0014-worker-generation-duplication.md | 2 +- .../0015-worker-generation-consolidation.md | 99 +++++++++++++++++ docs/adr/README.md | 6 ++ ...RUN-LOG-2026-07-29-worker-consolidation.md | 100 ++++++++++++++++++ 9 files changed, 283 insertions(+), 24 deletions(-) create mode 100644 docs/adr/0015-worker-generation-consolidation.md create mode 100644 docs/audit/WS-RUN-LOG-2026-07-29-worker-consolidation.md diff --git a/AGENTS.md b/AGENTS.md index e31b362b..f928363d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,7 +120,7 @@ WorldScript-Studio/ │ ├── unit/ # Vitest tests (co-located naming convention) │ ├── e2e/ # Playwright specs (CI-only) │ └── setup.ts # Global Vitest setup -├── workers/ # Web Workers: inference, duckdb, plugin, v2 inference/duckdb +├── workers/ # Web Workers: plugin.worker.ts; v2/ is the sole worker generation since ADR-0015 (inference, duckdb, webllm) ├── scripts/ # Build/deploy helpers (i18n bundle, SW version sync, bundle budget, edge build) ├── infra/low-end-ci/ # Local CI stack for constrained hardware ├── src-tauri/ # Tauri 2 desktop app (Rust) @@ -425,7 +425,7 @@ Edge builds run `scripts/build-edge.mjs` which sets `DEPLOY_TARGET=edge` and pat ### Local Inference - 4-layer stack: WebLLM → ONNX Runtime Web → Transformers.js → heuristics fallback. -- `services/localAiFacade.ts` wraps WebLLM via `packages/ai-core` + `workers/inference.worker.ts`. +- `services/localAiFacade.ts` wraps WebLLM via `packages/ai-core` + `workers/v2/inference.worker.ts`. - Tab-leader election via BroadcastChannel prevents multi-tab GPU contention. - **Adaptive AI Engine** (`services/ai/adaptiveAiEngine.ts`) — runtime hardware-aware backend/model selection. Gated by `enableAdaptiveAiEngine` flag. - **Device Profiler** (`services/ai/localAiDeviceProfiler.ts`) — WebGPU/WebNN/NPU/battery detection, 30s TTL cache. @@ -448,19 +448,20 @@ Edge builds run `scripts/build-edge.mjs` which sets `DEPLOY_TARGET=edge` and pat ### WorkerBus v2 -Central orchestration layer for all background worker tasks. Messages use short kind literals (`TASK`, `CANCEL`, `PING`, `PONG`, `PROGRESS`, `RESULT`) validated by Zod. +Central orchestration layer for all background worker tasks — since ADR-0015, the **sole** worker generation (v1's `workers/duckdbWorker.ts`/`workers/inference.worker.ts` deleted). Messages use short kind literals (`TASK`, `CANCEL`, `PING`, `PONG`, `PROGRESS`, `RESULT`) validated by Zod. -- `packages/worker-bus/src/` — WorkerBus, WorkerPool, PriorityTaskQueue, CircuitBreaker, DeadLetterQueue, ProtocolHandler, workerBootstrap, constants, schemas. -- `services/workerBusManager.ts` — singleton lifecycle; registers `inference` and `duckdb` pools. +- `packages/worker-bus/src/` — WorkerBus (`hasPool()`/`terminatePool()` for scoped pool lifecycle), WorkerPool, PriorityTaskQueue (`critical > high > normal > low`, no `background` tier), CircuitBreaker, DeadLetterQueue, ProtocolHandler (unused by the live `runTask()` path), workerBootstrap (`WorkerHandlerContext.emitProgress(stage, progress, message?)` — a flat function, not `context.progress.emit(...)`), constants, schemas. +- `services/workerBusManager.ts` — singleton lifecycle; registers `inference`, `duckdb`, `webllm`, `plugin` pools. `ensureDuckDbPool()`/`ensureInferencePool()`/`ensureWebLlmPool()` force-init and re-register a pool removed via `terminatePool()`, decoupled from `enableWorkerBusV2` (these are core features, not experimental infra). - `services/hybridRouter.ts` — routes to Web Worker pool or Rust TaskSupervisor (Tauri only) when `enableRustCompute` is on. - `services/legacyWorkerBusAdapter.ts` — shims old `@domain/ai-core` WorkerBus API onto v2. - `services/tauriTaskBridge.ts` — `invokeRustTask()`, `isRustComputeAvailable()` (60s TTL ping cache). - Feature flags: `enableWorkerBusV2` (on by default), `enableRustCompute` (on by default; effective on Tauri desktop only). -- v2 workers: `workers/v2/inference.worker.ts` (text + embed via Hugging Face transformers), `workers/v2/duckdb.worker.ts` (init/query/exec/shutdown). +- Workers: `workers/v2/inference.worker.ts` (text + embed via Hugging Face transformers, prepared-statement-free), `workers/v2/duckdb.worker.ts` (init/query/exec/shutdown, binds params via DuckDB-WASM prepared statements), `workers/v2/webllm.worker.ts` (WebGPU, ADR-0005), `workers/plugin.worker.ts` (sandboxed plugin execution, outside `v2/` but on the same protocol). +- **Known gap:** `timeoutMs` is not enforced anywhere in the live `runTask()` path (no timer rejects a hung task) — pre-existing, tracked in ADR-0015/TODO.md. ### DuckDB Analytics -- `workers/duckdbWorker.ts` runs DuckDB-WASM off main thread (OPFS persistence → in-memory fallback). +- `workers/v2/duckdb.worker.ts` runs DuckDB-WASM off main thread via the shared `duckdb` pool (OPFS persistence → in-memory fallback). `duckdbClient.ts` is an adapter over `ensureDuckDbPool()`/`bus.enqueue()` that auto-reinits on a respawned worker's "not initialized" error. - `services/duckdb/duckdbClient.ts` is a singleton proxy with AbortSignal and init retry. - Schema (`duckdbSchema.ts`): 10 tables + 5 views including `rag_chunks` (FLOAT[384]), `cross_project_index`, `codex_*`. - Gated behind `featureFlagsSlice.enableDuckDbAnalytics` (on by default). diff --git a/AUDIT.md b/AUDIT.md index 3a9f45da..8a75b5fc 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -39,10 +39,10 @@ change landed — all 14 findings CONFIRMED, no baseline drift, base SHA `f5f9c1 | F-08 | 🟠 P1 | Tauri `connect-src` missing LanguageTool's port and the Hugging Face hosts WebLLM/Transformers.js resolve models from | Fixed; scope widened during verification — the LanguageTool port was missing on **all 5** surfaces, not just Tauri; the real weight-file CDN (`us.aws.cdn.hf.co`) traced empirically via `curl`, not guessed | | F-09 | 🟠 P1 | DuckDB-WASM loaded from an unversioned, floating-`latest` third-party CDN — already unreachable under the (correctly-scoped) `worker-src` CSP, so this was dead code, not just a supply-chain risk | Self-hosted from the pinned npm dependency via `scripts/copy-duckdb-assets.mjs` (gitignored, ~72 MB, never committed) | | F-10 | 🟡 P2 | Two conflicting production URLs; the in-app link and the Italian locale pointed at a dead domain | Empirically resolved (live 200 vs. 404 check) — no maintainer input needed. Unified into a `PRODUCTION_URL` constant + new drift gate | -| F-11 | 🟡 P2 | Release/tag drift — `v1.24.1` tag 5 commits behind `HEAD` | Version bumped to `1.24.2` in this sprint (WS-7); tag/release publish remains a maintainer action | +| F-11 | 🟡 P2 | Release/tag drift — `v1.24.1` tag 5 commits behind `HEAD` | Version bumped to `1.24.2` in this sprint (WS-7); `v1.24.2` tagged and published 2026-07-29 | | F-12 | 🟡 P2 | Storybook built 3× per CI run; `ci.yml` header still said "StoryCraft Studio" | Deduped to 1×, header fixed. Bonus: the Storybook test-runner's flags were invalid for the installed CLI version, so it had never actually executed a single story on any prior CI run — fixed, and its `\|\| true` (which was itself hiding the failure) replaced with `continue-on-error: true` | | F-13 | 🟡 P2 | Coverage ratchet stale since 2026-06-06; 4 non-blocking CI gates with no stated exit criterion | Ratchet raised to CI-measured values (see quality-gate line above); `docs/CI.md` gained an exit-criteria table for all 4 | -| F-14 | 🟡 P2 | Two live worker generations (v1 + WorkerBus v2) for DuckDB/inference, confirmed via real call-site tracing | Documented, not consolidated — [ADR-0014](docs/adr/0014-worker-generation-duplication.md); a dedicated migration sprint is the correct scope for consolidation | +| F-14 | 🟡 P2 | Two live worker generations (v1 + WorkerBus v2) for DuckDB/inference, confirmed via real call-site tracing | **Resolved.** Consolidated as the dedicated migration sprint [ADR-0014](docs/adr/0014-worker-generation-duplication.md) called for — [ADR-0015](docs/adr/0015-worker-generation-consolidation.md), PRs #286–288/#290. Both v1 files deleted; correction loops on the migration PRs also caught and fixed 4 real bugs the untested v2 files had drifted on (dropped SQL params, pool-termination hangs, respawn connection loss, a missing export that broke production builds) | ### Post-mortem: why no gate caught F-01 for two months diff --git a/CHANGELOG.md b/CHANGELOG.md index eb4c64cd..6fe70a2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,51 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +> Worker-generation consolidation sprint: the v1/WorkerBus-v2 duplication [ADR-0014](docs/adr/0014-worker-generation-duplication.md) +> deferred is now closed out. Executed as 4 stacked PRs (#286–288, #290); see +> [ADR-0015](docs/adr/0015-worker-generation-consolidation.md) for the full decision record. + +### Changed + +- **DuckDB analytics and local inference (embeddings + NLP) now route through WorkerBus v2**, + replacing dedicated `new Worker(...)` instances per service. `services/duckdb/duckdbClient.ts`, + `services/ai/localEmbeddingService.ts`, and `services/ai/localNlpService.ts` keep their exact + public APIs — an adapter pattern, not a consumer rewrite, so none of their real callers needed + changes. `ensureDuckDbPool()`/`ensureInferencePool()` added to `services/workerBusManager.ts`, + decoupled from `enableWorkerBusV2` (mirrors the existing `ensureWebLlmPool()` pattern — these are + core features, not experimental infra). +- The shared `inference` WorkerBus pool's `maxWorkers` capped at 2 (was 4) — each replica + independently loads its own transformers.js pipeline with no cross-replica cache sharing, so 4 + concurrent workers under a burst could mean 4x the model memory footprint. + +### Fixed + +- **DuckDB query/exec parameters were silently dropped**, executing an unbound query — already live + and breaking `services/ai/telemetryService.ts`'s parameterized `INSERT` writes. Fixed via + DuckDB-WASM prepared statements. +- **`WorkerBus.terminatePool()` could leave tasks hanging forever** — removing a pool didn't settle + tasks already routed to it. Fixed by tracking task→pool assignment and cancelling matching tasks + before removal; a pool removed this way is now also transparently re-registered on next use + instead of failing every subsequent call. +- **A respawned DuckDB worker lost its connection**, failing every query after an idle timeout with + "DuckDB not initialized." Fixed by transparently re-initializing once and retrying. +- **DuckDB's OPFS-unavailable fallback silently swallowed the failure**, giving users no warning + their analytics wouldn't persist across reloads (private browsing, old Safari) — restored via the + existing progress channel. A follow-up fix made the fallback's own cleanup step best-effort, so a + secondary failure there can no longer block the fallback path itself. +- **A missing `ensureInferencePool` export broke production builds** on Vercel (rolldown's + `[MISSING_EXPORT]`) — not caught by Vitest (the consumer test mocks the whole module) or by local + `tsgo` typecheck. Root-caused by reproducing Vercel's *exact* build command + (`pnpm run build:edge`, which differs from the plain `pnpm run build` used for an earlier, + misleading local repro attempt) and fixed by adding the missing function. + +### Removed + +- `workers/duckdbWorker.ts` and `workers/inference.worker.ts` (the v1 worker generation) — both + workloads now exclusively use `workers/v2/duckdb.worker.ts` and `workers/v2/inference.worker.ts`. + ## [1.24.2] — 2026-07-29 > CSP functional-truth, desktop crypto, and documentation-truth hardening sprint diff --git a/CLAUDE.md b/CLAUDE.md index d8ee4dfd..d488e55b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,8 +107,9 @@ locales/ → i18n source JSON — 19 locales (de/en/es/fr/it/ar/he/el/j tests/ → unit/ (Vitest) + e2e/ (Playwright); shared E2E helpers in tests/e2e/helpers.ts types/ → Supplemental TypeScript definitions (duckdb-wasm-worker.d.ts, tauri-plugins.d.ts) types.ts → Core shared interfaces and types (root level) -workers/ → inference.worker.ts (@huggingface/transformers v3), duckdbWorker.ts (DuckDB-WASM) - v2/ → WorkerBus v2 workers: inference.worker.ts, duckdb.worker.ts, webllm.worker.ts (P1-1, @mlc-ai/web-llm) +workers/ → plugin.worker.ts (sandboxed plugin execution, P0-2) + v2/ → sole worker generation since ADR-0015: inference.worker.ts (@huggingface/transformers v3), + duckdb.worker.ts (DuckDB-WASM), webllm.worker.ts (P1-1, @mlc-ai/web-llm) infra/low-end-ci/ → Local CI stack: Forgejo + act + systemd units + bash scripts scripts/ → Build/deploy helpers (sync-deploy-base, cf-pages-deploy, graphify-update, etc.) ``` @@ -188,7 +189,7 @@ Wrap each major view root with `components/ui/ViewErrorBoundary.tsx` — provide ### DuckDB Analytics -`workers/duckdbWorker.ts` off main thread (OPFS → in-memory fallback). `duckdbClient.ts`: singleton, init retry 3× backoff. Schema: 10 tables + 5 views incl. `rag_chunks` (FLOAT[384]). Gate all paths behind `enableDuckDbAnalytics`. Dual-write via `duckdbListenerLoader.ts` (dynamically imported). `ragVectorMigration.ts`: FLOAT[64]→FLOAT[384] upgrade. `useDuckDb.ts` 30s timeout; `useAnalytics.ts` parallelizes 4 queries. +`workers/v2/duckdb.worker.ts` off main thread via the shared WorkerBus v2 `duckdb` pool (OPFS → in-memory fallback, ADR-0015). `duckdbClient.ts`: adapter over `ensureDuckDbPool()`/`bus.enqueue()`, auto-reinits on a respawned worker's "not initialized" error. Schema: 10 tables + 5 views incl. `rag_chunks` (FLOAT[384]). Gate all paths behind `enableDuckDbAnalytics`. Dual-write via `duckdbListenerLoader.ts` (dynamically imported). `ragVectorMigration.ts`: FLOAT[64]→FLOAT[384] upgrade. `useDuckDb.ts` 30s timeout; `useAnalytics.ts` parallelizes 4 queries. ### Logging @@ -218,7 +219,7 @@ Real-time P2P via Yjs + `packages/collab-transport`. Signaling AES-256-GCM/PBKDF ### WorkerBus v2 (`packages/worker-bus`) -Central orchestration layer for all background tasks. Key files: `workerBus.ts` (orchestrator, priority queue + circuit breakers), `workerPool.ts` (auto-scaling `MIN`→`MAX_WORKERS_INFERENCE`, idle timeout), `taskQueue.ts` (heap: `critical > high > normal > low > background`), `circuitBreaker.ts` (per-worker health gate), `deadLetterQueue.ts`, `protocolHandler.ts` (typed postMessage + version negotiation), `workerBootstrap.ts` (`registerTaskHandler` inside worker scripts, `WorkerHandlerContext.progress.emit`). +Central orchestration layer for all background tasks — since ADR-0015, the **sole** worker generation (no more v1/v2 duplication). Key files: `workerBus.ts` (orchestrator, priority queue + circuit breakers, `hasPool()`/`terminatePool()` for scoped pool lifecycle), `workerPool.ts` (auto-scaling `MIN`→`MAX_WORKERS_INFERENCE`, idle timeout), `taskQueue.ts` (priority queue: `critical > high > normal > low` — no `background` tier), `circuitBreaker.ts` (per-worker health gate), `deadLetterQueue.ts`, `protocolHandler.ts` (typed postMessage + version negotiation, currently unused by the live `runTask()` path — not wired up), `workerBootstrap.ts` (`registerTaskHandler` inside worker scripts; handlers receive `WorkerHandlerContext.emitProgress(stage, progress, message?)`, a flat function, not `context.progress.emit(...)`). **All constants** re-exported from `constants.ts`. **Schemas** (Zod) in `schemas.ts` gate cross-thread messages. After changes: `pnpm exec vitest run tests/unit/workerBus`. @@ -405,7 +406,8 @@ Feature-specific implementation patterns (Plot Board, ProForge Pipeline, scene-l See `AUDIT.md` and `TODO.md`. Key items: - `app/listenerMiddleware.ts` — redux-undo `StateWithHistory` typing at boundaries. -- `workers/inference.worker.ts` — `@huggingface/transformers` v3 path alias in `tsconfig.json`; if alias breaks, restore `@ts-expect-error`. +- `workers/v2/inference.worker.ts` (v1 deleted, ADR-0015) — `@huggingface/transformers` v3 path alias in `tsconfig.json`; if alias breaks, restore `@ts-expect-error`. +- **`WorkerBus.runTask()` doesn't enforce `timeoutMs`** — confirmed via inspection, no timer on either the bus or worker-side rejects a hung task. Pre-existing, tracked in ADR-0015/TODO.md, not yet fixed. - **DS-5:** Delete legacy bridge block from `index.css` — deferred until DS-1 verified in production. - **B-1 (IDB encryption):** Passphrase UX complete (`IdbUnlockModal`, `PassphraseModal`). Actual IDB read/write integration for stores is Phase 4 (service-layer only currently). - **B-2 (Voice WASM):** Engine + download UI shipped. Remaining: E2E integration test coverage. diff --git a/TODO.md b/TODO.md index 16856070..04ee4221 100644 --- a/TODO.md +++ b/TODO.md @@ -23,13 +23,19 @@ Status: 🔄 in progress | ⬜ open | ✅ done - ✅ **DuckDB-WASM self-hosted** — replaced an unversioned, already-CSP-dead third-party CDN. - ✅ **Doc-truth fixes** — fabricated `tauri-plugin-stronghold` claim removed; canonical Vercel URL unified with a new drift gate; CI Storybook 3×→1× + a genuinely broken test-runner invocation fixed. - ✅ **Coverage ratchet** raised to CI-measured values (L79/F72/B65/S77). -- ⬜ **Worker-generation consolidation (F-14, new this sprint)** — both a v1 and a WorkerBus-v2 - generation of the DuckDB and local-inference workers are live simultaneously - (`docs/adr/0014-worker-generation-duplication.md`). Needs a dedicated migration sprint: decide - the target generation (v2, per `CLAUDE.md`'s own architecture framing), plan the 3 v1 call-site - migrations (`services/duckdb/duckdbClient.ts`, `services/ai/localNlpService.ts`, - `services/ai/localEmbeddingService.ts`), then remove the v1 worker files. Until then, any worker- - level fix (CSP, protocol, error handling) must be checked against **both** generations. +- ✅ **Worker-generation consolidation (F-14)** — the dedicated migration sprint + `docs/adr/0014-worker-generation-duplication.md` deferred, executed as 4 stacked PRs: + #286 (parity gate + real handler tests for the previously-untested v2 workers), #287 (DuckDB — + `duckdbClient.ts` migrated, `workers/duckdbWorker.ts` deleted), #288 (embeddings — + `localEmbeddingService.ts` migrated), #290 (NLP — `localNlpService.ts` migrated, + `workers/inference.worker.ts` deleted, closing out v1 entirely). Decision record: + `docs/adr/0015-worker-generation-consolidation.md` (supersedes 0014). Correction loops on #287/#288 + surfaced and fixed real bugs the original v2 files had never been exercised against: DuckDB + `params` were silently dropped (already live in `telemetryService.ts`'s parameterized writes), + `terminatePool()` could leave tasks hanging forever, a respawned DuckDB worker lost its + connection, and `ensureInferencePool()` was imported but never defined (broke Vercel's prod + build — see the entry below). **Merge status:** #286 merged; #287/#288/#290 open, stacked, CI + green — merge in order before the next release. - ✅ **Pre-existing, unrelated E2E a11y finding surfaced during this sprint's CI runs** — `tests/e2e/a11y.spec.ts` "writer version control panel has no serious axe violations" failed deterministically (`color-contrast` 4.47 vs. 4.5 required, `--sc-accent` `#92400e` over its own @@ -38,8 +44,8 @@ Status: 🔄 in progress | ⬜ open | ✅ done active-toggle classes reused `--sc-accent` for both background tint and text instead of the already-vetted `--nav-background-active`/`--nav-text-active` token pair; switched all 3 occurrences (Writer VC desktop/mobile + mobile ProForge, same shared pattern). -- ⬜ **Tag `v1.24.2` + publish the GitHub Release** — maintainer action (STOP-AND-ASK per this - sprint's own hard rules); the version-bump commit is in, the tag/release is not. +- ✅ **Tag `v1.24.2` + publish the GitHub Release** — stale entry, already done (verified via + `git tag`/`gh release list`: `v1.24.2` tagged and published 2026-07-29T11:25:42Z). ## v1.24 — Post-release feature-flag refinement (2026-06-21) diff --git a/docs/adr/0014-worker-generation-duplication.md b/docs/adr/0014-worker-generation-duplication.md index bd08a051..24086609 100644 --- a/docs/adr/0014-worker-generation-duplication.md +++ b/docs/adr/0014-worker-generation-duplication.md @@ -1,6 +1,6 @@ # ADR 0014 — Two live worker generations (v1 and WorkerBus v2) -- **Status:** Accepted (documented as known debt — consolidation explicitly out of scope this sprint) +- **Status:** Superseded by [[0015-worker-generation-consolidation]] — migrated 2026-07-29 - **Date:** 2026-07-29 - **Deciders:** Maintainer + Claude Code - **Context tags:** workers, architecture, tech-debt diff --git a/docs/adr/0015-worker-generation-consolidation.md b/docs/adr/0015-worker-generation-consolidation.md new file mode 100644 index 00000000..cc54f7b1 --- /dev/null +++ b/docs/adr/0015-worker-generation-consolidation.md @@ -0,0 +1,99 @@ +# ADR 0015 — Worker-generation consolidation: v1 retired, WorkerBus v2 is the sole generation + +- **Status:** Accepted (shipped) +- **Date:** 2026-07-29 +- **Deciders:** Maintainer + Claude Code +- **Context tags:** workers, architecture, tech-debt +- **Supersedes:** [[0014-worker-generation-duplication]] + +## Context + +[[0014-worker-generation-duplication]] documented, but deliberately deferred, consolidating two +live worker generations (legacy `workers/*.worker.ts` vs. WorkerBus v2's `workers/v2/*.worker.ts`) +for the same two workloads — DuckDB analytics and local inference (embeddings/NLP). That ADR named +the target generation (v2, per the codebase's own architecture framing) and the exact 3 v1 call +sites to migrate, but explicitly scoped the actual migration out as "a dedicated migration sprint." +This ADR is that migration, executed as 4 stacked PRs. + +## Decision + +**v2 is now the sole worker generation for DuckDB analytics and local inference.** Both legacy +files are deleted: + +| Workload | v1 file (deleted) | v2 file (sole generation) | Migrated in | +|---|---|---|---| +| DuckDB analytics | `workers/duckdbWorker.ts` | `workers/v2/duckdb.worker.ts` | PR #287 | +| Embeddings | `workers/inference.worker.ts` | `workers/v2/inference.worker.ts` (`inference.embed`) | PR #288 | +| Local NLP | `workers/inference.worker.ts` | `workers/v2/inference.worker.ts` (`inference.text`) | PR #290 | + +Each of the 3 v1 call sites (`services/duckdb/duckdbClient.ts`, `services/ai/localEmbeddingService.ts`, +`services/ai/localNlpService.ts`) kept its exact public API — an adapter pattern, not a consumer +rewrite — so none of their real callers (`useDuckDb.ts`, `duckdbAnalytics.ts`, `duckdbMigration.ts`, +`ragVectorMigration.ts`, `telemetryService.ts`, `localRagService.ts`, `crossProjectIndexService.ts`, +`ragPromptAssembly.ts`, `proForgeMemoryBank.ts`, `loraEvaluationService.ts`, `loraDatasetBuilder.ts`, +`inferenceGateway.ts`) needed any changes. `services/workerBusManager.ts` gained +`ensureDuckDbPool()`/`ensureInferencePool()`, mirroring `ensureWebLlmPool()`'s decoupling from +`enableWorkerBusV2` (ADR-0005) — these are core features (DuckDB analytics defaults on; RAG/ +cross-project search are permanent core behaviour), not experimental infra, so toggling that flag +off must not silently break them. + +### Parity gaps found and fixed before cutover (not assumed away) + +The v2 worker files had zero production callers before this migration — registered as live pools +but never exercised by real traffic — and had genuinely drifted from v1 in ways a parity-test PR +(#286) and two review-comment correction loops caught before they could reach users: + +1. **`initDuckDb()`'s OPFS-unavailable catch block silently swallowed the failure**, unlike v1's + out-of-band `OPFS_FALLBACK` message that lets the UI warn users their analytics won't persist. + Fixed by threading `emitProgress` into `initDuckDb()` and emitting an `'opfs-fallback'` progress + stage; a follow-up review pass found the fix itself could abort the fallback path if the + partial connection's `close()` rejected — wrapped in its own best-effort try/catch. +2. **`handleQuery`/`handleExec` silently dropped `params`**, executing an unbound query — already + live and breaking `telemetryService.ts`'s parameterized `INSERT` (a real, shipping bug caught + only by this migration, not a hypothetical). Fixed via DuckDB-WASM prepared statements + (`connection.prepare(sql)` → `stmt.query(...params)` → `stmt.close()`). +3. **A respawned duckdb-pool worker had no `connection`** (the pool's `minWorkers:0` lets it + idle-terminate; a fresh worker's module state starts blank), so any QUERY/EXEC after a respawn + failed with "DuckDB not initialized." Fixed by transparently re-sending INIT once and retrying + the original call when `duckdbClient.ts` sees that specific error. +4. **`WorkerBus.terminatePool()` had no lifecycle contract** — removing a pool didn't settle tasks + already routed to it (their result promises would await a RESULT message from a worker that no + longer exists), and `ensureDuckDbPool()` assumed a non-null bus always has every pool, so any + call after `duckdbClient.terminate()` would fail with `NO_POOL` forever. Fixed by tracking + task→pool assignment and cancelling matching tasks on `terminatePool()`, plus a re-registration + path (`reRegisterDuckDbPool()`/`reRegisterInferencePool()`) shared with initial registration. +5. **`ensureInferencePool()` was imported by `localEmbeddingService.ts` but never defined** — a + genuine missing-export regression that broke production builds (caught by Vercel's build, not + by Vitest, since the consumer test fully mocks the module, or by local `tsgo` typecheck, for + reasons not fully understood). Fixed by adding the function; see the "not yet resolved" item + below and `TODO.md`'s incident write-up. + +### Deliberately not changed + +WebLLM's worker-first-with-main-thread-fallback shape (ADR-0005) was **not** extended to DuckDB or +embedding/NLP — that shape exists because a real main-thread fallback engine exists for WebLLM +(`runLocalTextGeneration`'s ONNX→Transformers.js→heuristic chain). No such fallback exists for +DuckDB-WASM or transformers.js pipelines in this repo; failures still surface as errors and callers +degrade exactly as they did against v1 (empty query results, heuristic sentiment/summary fallback). + +## Consequences + +- **Positive:** the "any worker-level fix must be checked against both generations" tax ADR-0014 + described is retired — `workers/duckdbWorker.ts` and `workers/inference.worker.ts` no longer + exist. `packages/worker-bus`'s generic `WorkerPool` health-check also replaced ~70 lines of + hand-rolled ping/backoff duplicated per v1 service. +- **Negative (accepted):** the `inference` pool's `maxWorkers` was capped at 2 (was + `MAX_WORKERS_INFERENCE`=4) because each pool replica independently loads its own transformers.js + pipeline with no cross-replica cache sharing — a resource tradeoff to revisit from WorkerBus's own + telemetry (`peakLatencyMs`/`errorRate`) if it proves too conservative. +- **Not yet resolved:** `WorkerBus.runTask()`'s live path does not enforce `timeoutMs` — confirmed + via code inspection, no timer anywhere rejects a hung task, on either the bus or the worker-side + bootstrap. Pre-existing (not introduced by this migration), tracked as a follow-up in `TODO.md` + rather than expanded into this migration's scope. + +## References + +- [[0014-worker-generation-duplication]] — the deferred decision this ADR completes +- [[0005-webllm-worker-offload]] — the `ensureXPool()`/worker-first pattern this migration reused +- PRs #286 (parity gate), #287 (DuckDB), #288 (embeddings), #290 (NLP + v1 deletion) +- `TODO.md`'s "Vercel deployment failures root-caused + fixed" entry — the missing-export incident diff --git a/docs/adr/README.md b/docs/adr/README.md index 802d88b7..14e4d557 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -15,6 +15,12 @@ than editing history. | [0007](0007-plugin-sandbox-model.md) | Plugin Sandbox Model | Accepted | | [0008](0008-local-first-data-model.md) | Local-first data model: Yjs document as source of truth | Accepted | | [0009](0009-xstate-workflow-orchestration.md) | XState for complex workflow orchestration (Redux + RTK Query + XState) | Accepted | +| [0010](0010-languagetool-self-hosted.md) | Self-hosted LanguageTool grammar checking via the editor overlay | Accepted | +| [0011](0011-ai-heuristic-fallbacks.md) | AI heuristic fallbacks via a registry + provider-layer seam | Accepted | +| [0012](0012-local-server-connectivity-tauri-http.md) | Local AI server connectivity: route localhost HTTP through the Tauri HTTP plugin | Accepted | +| [0013](0013-csp-wasm-and-blob-frames.md) | CSP `'wasm-unsafe-eval'` and `frame-src blob:` | Accepted | +| [0014](0014-worker-generation-duplication.md) | Two live worker generations (v1 and WorkerBus v2) | Superseded by 0015 | +| [0015](0015-worker-generation-consolidation.md) | Worker-generation consolidation: v1 retired, WorkerBus v2 is the sole generation | Accepted | **Format:** Context → Decision → Consequences (incl. rejected alternatives). Keep each ADR to one decision. Link related records with `[[slug]]`. diff --git a/docs/audit/WS-RUN-LOG-2026-07-29-worker-consolidation.md b/docs/audit/WS-RUN-LOG-2026-07-29-worker-consolidation.md new file mode 100644 index 00000000..20983a1b --- /dev/null +++ b/docs/audit/WS-RUN-LOG-2026-07-29-worker-consolidation.md @@ -0,0 +1,100 @@ +# WS Run Log — 2026-07-29 Worker-Generation Consolidation Sprint + +Tracks execution of the deferred [ADR-0014](../adr/0014-worker-generation-duplication.md) migration +(F-14 from the CSP/crypto/doc-truth sprint's finding table), completed the same day as `v1.24.2`'s +release. Decision record: [ADR-0015](../adr/0015-worker-generation-consolidation.md). + +## Baseline + +- **HEAD at start:** `dc14bc09` (`chore: reconcile stale TODO.md entry + add Renovate storybook + grouping (#285)`), tag `v1.24.2` already published at this point. +- **Plan file:** `.claude/plans/worker-generation-v1-vs-enumerated-fox.md`. +- **Scope handed off from ADR-0014:** target generation v2 (per the repo's own architecture + framing); 3 named v1 call sites (`services/duckdb/duckdbClient.ts`, + `services/ai/localNlpService.ts`, `services/ai/localEmbeddingService.ts`); no migration plan or + timeline beyond "a dedicated migration sprint." + +## Execution — 4 stacked PRs + +| PR | Branch | Scope | Status at log time | +|---|---|---|---| +| #286 | `feat/worker-v2-parity-gate` | Export v2 `duckdb`/`inference` worker handlers, add handler-level tests exercising real logic (previously untested — zero production callers before this sprint), fix a real OPFS-fallback-signal gap found in the process | **Merged** (squash, `ea879e41`) | +| #287 | `feat/worker-v2-duckdb-migration` | Migrate `duckdbClient.ts` onto WorkerBus v2; delete `workers/duckdbWorker.ts`; add `WorkerBus.terminatePool()`/`hasPool()` | Open, CI green, correction loop quiescent | +| #288 | `feat/worker-v2-embedding-migration` | Migrate `localEmbeddingService.ts`; cap `inference` pool `maxWorkers` at 2 | Open, CI green (after the missing-export incident below) | +| #290 | `feat/worker-v2-nlp-migration` | Migrate `localNlpService.ts`; delete `workers/inference.worker.ts` (v1 fully retired); consolidate a duplicate test file | Open, CI green | + +Each PR kept the migrated service's public API stable (adapter pattern) — no consumer call site +outside the 3 named services needed changes across the whole sprint. + +## Correction-loop findings (CodeRabbit), all confirmed real and fixed before merge + +**PR #286** (2 waves): +1. QNBS-v3 comment-format nitpicks (multi-line → required single bracketed line) — 3 files. +2. **Major:** `initDuckDb()`'s OPFS-unavailable catch block silently swallowed the failure instead + of surfacing it (v1 had an out-of-band `OPFS_FALLBACK` message; v2 had none). Fixed via the + existing progress channel. +3. **Major** (second wave, outside-diff-range comment on the PR#286 fix itself): the new + `opfsConnection?.close()` cleanup could itself reject and abort the fallback path, undoing the + fix it was part of. Fixed by making cleanup best-effort (its own try/catch). + +**PR #287** (1 wave, 4 findings): +1. **Critical:** `handleQuery`/`handleExec` silently dropped `params` — an already-live bug + breaking `services/ai/telemetryService.ts`'s parameterized `INSERT`. Fixed via DuckDB-WASM + prepared statements. +2. **Major:** a respawned `duckdb` pool worker had no `connection` (fresh module state after + `minWorkers:0` idle-termination), failing every query with "DuckDB not initialized." Fixed via + transparent re-init-and-retry. +3. **Major:** `WorkerBus.terminatePool()` didn't settle tasks already routed to the removed pool — + their result promises would await forever. Fixed via task→pool tracking + cancellation. +4. **Major:** `ensureDuckDbPool()` assumed a non-null bus always has every pool, so any call after + `terminatePool('duckdb')` would fail `NO_POOL` permanently. Fixed via `hasPool()` + + re-registration. + +**PR #288 → production incident (not a review-bot finding — a real deployment failure):** +`services/ai/localEmbeddingService.ts` imported `ensureInferencePool` from `workerBusManager.ts`, +which was never actually defined there. Invisible to Vitest (the consumer test mocks the whole +module) and to local `tsgo` typecheck (root cause not fully understood — flagged as a follow-up). +Two consecutive Vercel deployments failed with only a generic error; the first local repro attempt +(`pnpm run build`) misleadingly succeeded because **it is not the command Vercel actually runs** +(`vercel.json`'s `buildCommand` is `pnpm run build:edge`, which sets `DEPLOY_TARGET=edge` and runs +`scripts/sync-deploy-base.mjs` first). Reproducing the *exact* command +(`NODE_OPTIONS=--max-old-space-size=3072 pnpm run build:edge`) immediately surfaced rolldown's +`[MISSING_EXPORT]` error. Fixed by adding `ensureInferencePool()`; verified via a second clean +`build:edge` run before closing out. `vercel.json`'s `git.deploymentEnabled` was briefly set to +`false` mid-investigation (PR #289) and reverted once root-caused — net diff of that PR is a +`TODO.md` incident write-up, no functional change. + +## Final verification (this doc's own sprint, PR #291 — docs cleanup) + +```bash +$ pnpm exec tsx scripts/audit-feature-parity.ts +=== Summary === +Total flags: 22 +Critical errors: 0 +Warnings: 0 +✓ Audit passed — all flags consistent +``` + +- `docs/adr/0015-worker-generation-consolidation.md` — new ADR, supersedes 0014. +- `docs/adr/0014-worker-generation-duplication.md` — status line changed to `Superseded by 0015` + (body left untouched, per this repo's own "supersede, don't edit history" convention). +- `docs/adr/README.md` — index gained rows 0010–0015 (0010–0014 were pre-existing files missing + from the index before this sprint; unrelated drift, fixed opportunistically). +- `CLAUDE.md`/`AGENTS.md` — directory map, WorkerBus v2 section, DuckDB section, Known Technical + Debt all updated; fixed 2 pre-existing doc-truth bugs found while reading the WorkerBus v2 source + (`WorkerHandlerContext.emitProgress` is a flat function, not `context.progress.emit(...)`; no + `background` priority tier exists, only `critical > high > normal > low`). +- `TODO.md` — F-14 entry marked done with the PR chain; a genuinely stale "Tag v1.24.2" entry + (already tagged/released, unrelated to this sprint) also caught and fixed while in the file. +- `AUDIT.md` — F-14 and F-11 (same stale-tag issue) findings updated to resolved. +- `CHANGELOG.md` — `[Unreleased]` section added (Changed/Fixed/Removed) covering the full sprint. + +## Known, deliberately unresolved + +- **`WorkerBus.runTask()` doesn't enforce `timeoutMs`** — confirmed via inspection (no timer on + either the bus or worker-side bootstrap rejects a hung task). Pre-existing, not introduced by + this sprint; scoped out to keep this migration bounded to what ADR-0014 named. Tracked in + ADR-0015 and `TODO.md`. +- **Why local `tsgo` typecheck didn't catch the missing-export bug** — genuinely unresolved; worth + a follow-up look at whether a project-reference or cache-behavior gap exists, since it's the kind + of thing that could mask a real error again. From dcf4144f6153cc27c94c724ffd9e470298b470e9 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 29 Jul 2026 23:46:21 +0200 Subject: [PATCH 10/10] fix(docs): don't recommend @ts-expect-error as a suppression fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on #291 correctly flagged that the Known Technical Debt line I edited (only updating the file path to workers/v2/inference.worker.ts) still carried a pre-existing recommendation to "restore @ts-expect-error" if the transformers.js path alias breaks — in direct tension with this repo's own suppression-ratchet policy documented a few sections above in the same file. Reworded to point at fixing the alias/type declaration directly instead, consistent with the structural-type workaround this same sprint used for a different tsgo/external-package resolution gap (workers/v2/duckdb.worker.ts's PreparableConnection, PR #287) rather than reaching for a suppression. Co-Authored-By: Claude Sonnet 5 --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index d488e55b..a89858f3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -406,7 +406,7 @@ Feature-specific implementation patterns (Plot Board, ProForge Pipeline, scene-l See `AUDIT.md` and `TODO.md`. Key items: - `app/listenerMiddleware.ts` — redux-undo `StateWithHistory` typing at boundaries. -- `workers/v2/inference.worker.ts` (v1 deleted, ADR-0015) — `@huggingface/transformers` v3 path alias in `tsconfig.json`; if alias breaks, restore `@ts-expect-error`. +- `workers/v2/inference.worker.ts` (v1 deleted, ADR-0015) — `@huggingface/transformers` v3 path alias in `tsconfig.json`; if the alias breaks, fix the path alias or the package's type declaration directly — do not suppress with `@ts-expect-error` (conflicts with the suppression-ratchet policy above). - **`WorkerBus.runTask()` doesn't enforce `timeoutMs`** — confirmed via inspection, no timer on either the bus or worker-side rejects a hung task. Pre-existing, tracked in ADR-0015/TODO.md, not yet fixed. - **DS-5:** Delete legacy bridge block from `index.css` — deferred until DS-1 verified in production. - **B-1 (IDB encryption):** Passphrase UX complete (`IdbUnlockModal`, `PassphraseModal`). Actual IDB read/write integration for stores is Phase 4 (service-layer only currently).