From 9954f6fdfd2a52d7188a6311423f1245cc86b8ec Mon Sep 17 00:00:00 2001 From: zm2231 Date: Sun, 22 Mar 2026 20:21:08 -0400 Subject: [PATCH] fix: timeout detection, always-on dim validation, test hardening - Fix timeout detection: AbortSignal.timeout() throws TimeoutError, not AbortError. Timeouts are no longer retried (30s fail, not 93s). - Validate embedding dimensions in both httpEmbed and httpEmbedQuery against config.dimensions or the 384d schema default. When DIMS is unset, the error says 'Set GITNEXUS_EMBEDDING_DIMS=N' to guide users. - Centralize test env var cleanup in afterEach via savedEnv snapshot. - Test mocks use 384d vectors matching schema default. - 4 new tests: timeout not retried, network retry success, query path dim mismatch, unset-dims hint. 23 total, all pass. --- gitnexus/src/core/embeddings/http-client.ts | 37 +++++- gitnexus/test/unit/http-embedder.test.ts | 131 +++++++++++++------- 2 files changed, 121 insertions(+), 47 deletions(-) diff --git a/gitnexus/src/core/embeddings/http-client.ts b/gitnexus/src/core/embeddings/http-client.ts index d0805269a..b16496440 100644 --- a/gitnexus/src/core/embeddings/http-client.ts +++ b/gitnexus/src/core/embeddings/http-client.ts @@ -9,6 +9,7 @@ const HTTP_TIMEOUT_MS = 30_000; const HTTP_MAX_RETRIES = 2; const HTTP_RETRY_BACKOFF_MS = 1_000; const HTTP_BATCH_SIZE = 64; +const DEFAULT_DIMS = 384; interface HttpConfig { baseUrl: string; @@ -105,7 +106,15 @@ const httpEmbedBatch = async ( body: JSON.stringify({ input: batch, model }), }); } catch (err) { - // DNS, timeout, connection errors — add context without leaking the key + // Timeouts should not be retried — the server is unresponsive. + // AbortSignal.timeout() throws DOMException with name 'TimeoutError'. + const isTimeout = err instanceof DOMException && err.name === 'TimeoutError'; + if (isTimeout) { + throw new Error( + `Embedding request timed out after ${HTTP_TIMEOUT_MS}ms (${safeUrl(url)}, batch ${batchIndex})`, + ); + } + // DNS, connection errors — retry with backoff if (attempt < HTTP_MAX_RETRIES) { const delay = HTTP_RETRY_BACKOFF_MS * (attempt + 1); await new Promise(r => setTimeout(r, delay)); @@ -165,13 +174,17 @@ export const httpEmbed = async (texts: string[]): Promise => { const vec = new Float32Array(item.embedding); // Fail fast on dimension mismatch rather than inserting bad vectors // into the FLOAT[N] column which would cause a cryptic Kuzu error. - if (config.dimensions && vec.length !== config.dimensions) { + const expected = config.dimensions ?? DEFAULT_DIMS; + if (vec.length !== expected) { + const hint = config.dimensions + ? 'Update GITNEXUS_EMBEDDING_DIMS to match your model output.' + : `Set GITNEXUS_EMBEDDING_DIMS=${vec.length} to match your model output.`; throw new Error( `Embedding dimension mismatch: endpoint returned ${vec.length}d vector, ` + - `but GITNEXUS_EMBEDDING_DIMS is set to ${config.dimensions}. ` + - `Update GITNEXUS_EMBEDDING_DIMS to match your model output.`, + `but expected ${expected}d. ${hint}`, ); } + allVectors.push(vec); } } @@ -195,5 +208,19 @@ export const httpEmbedQuery = async (text: string): Promise => { if (!items.length) { throw new Error(`Embedding endpoint returned empty response (${safeUrl(url)})`); } - return items[0].embedding; + + const embedding = items[0].embedding; + // Same dimension checks as httpEmbed — catch mismatches before they + // reach the Kuzu FLOAT[N] cast in search queries. + const expected = config.dimensions ?? DEFAULT_DIMS; + if (embedding.length !== expected) { + const hint = config.dimensions + ? 'Update GITNEXUS_EMBEDDING_DIMS to match your model output.' + : `Set GITNEXUS_EMBEDDING_DIMS=${embedding.length} to match your model output.`; + throw new Error( + `Embedding dimension mismatch: endpoint returned ${embedding.length}d vector, ` + + `but expected ${expected}d. ${hint}`, + ); + } + return embedding; }; diff --git a/gitnexus/test/unit/http-embedder.test.ts b/gitnexus/test/unit/http-embedder.test.ts index d35dd2bad..e0e9150be 100644 --- a/gitnexus/test/unit/http-embedder.test.ts +++ b/gitnexus/test/unit/http-embedder.test.ts @@ -1,10 +1,33 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { getEmbeddingDims, isEmbedderReady } from '../../src/mcp/core/embedder.js'; +const ENV_KEYS = [ + 'GITNEXUS_EMBEDDING_URL', + 'GITNEXUS_EMBEDDING_MODEL', + 'GITNEXUS_EMBEDDING_API_KEY', + 'GITNEXUS_EMBEDDING_DIMS', +] as const; + +/** 384d mock vector matching the default schema dimensions. */ +const mockVec = Array.from({ length: 384 }, (_, i) => i / 384); + describe('HTTP embedding backend', () => { + // Save original env state before any test mutates it + const savedEnv = Object.fromEntries( + ENV_KEYS.map(k => [k, process.env[k]]), + ); + afterEach(() => { vi.unstubAllGlobals(); vi.resetModules(); + // Restore env vars to pre-test state so a mid-test throw can't leak + for (const key of ENV_KEYS) { + if (savedEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedEnv[key]; + } + } }); describe('MCP embedder', () => { @@ -21,8 +44,6 @@ describe('HTTP embedding backend', () => { process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; const mod = await import('../../src/mcp/core/embedder.js'); expect(mod.isEmbedderReady()).toBe(true); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('reads custom dimensions from environment', async () => { @@ -31,16 +52,13 @@ describe('HTTP embedding backend', () => { process.env.GITNEXUS_EMBEDDING_DIMS = '1024'; const mod = await import('../../src/mcp/core/embedder.js'); expect(mod.getEmbeddingDims()).toBe(1024); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; - delete process.env.GITNEXUS_EMBEDDING_DIMS; }); it('retries query on transient server error', async () => { process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; - const ok = { ok: true, json: async () => ({ data: [{ embedding: [0.1, 0.2] }] }) }; + const ok = { ok: true, json: async () => ({ data: [{ embedding: mockVec }] }) }; vi.stubGlobal('fetch', vi.fn() .mockResolvedValueOnce({ ok: false, status: 503 }) .mockResolvedValueOnce(ok)); @@ -49,10 +67,8 @@ describe('HTTP embedding backend', () => { const result = await mod.embedQuery('test query'); expect(fetch).toHaveBeenCalledTimes(2); - expect(result).toEqual([0.1, 0.2]); + expect(result).toEqual(mockVec); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); }); @@ -78,16 +94,13 @@ describe('HTTP embedding backend', () => { expect(result).toBeInstanceOf(Float32Array); expect(result.length).toBe(384); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; - delete process.env.GITNEXUS_EMBEDDING_API_KEY; }); it('retries on server error', async () => { process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; - const ok = { ok: true, json: async () => ({ data: [{ embedding: [0.1] }] }) }; + const ok = { ok: true, json: async () => ({ data: [{ embedding: mockVec }] }) }; vi.stubGlobal('fetch', vi.fn() .mockResolvedValueOnce({ ok: false, status: 503 }) .mockResolvedValueOnce(ok)); @@ -96,15 +109,13 @@ describe('HTTP embedding backend', () => { await embedText('test'); expect(fetch).toHaveBeenCalledTimes(2); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('retries on rate limit', async () => { process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; - const ok = { ok: true, json: async () => ({ data: [{ embedding: [0.1] }] }) }; + const ok = { ok: true, json: async () => ({ data: [{ embedding: mockVec }] }) }; vi.stubGlobal('fetch', vi.fn() .mockResolvedValueOnce({ ok: false, status: 429 }) .mockResolvedValueOnce(ok)); @@ -113,8 +124,6 @@ describe('HTTP embedding backend', () => { await embedText('test'); expect(fetch).toHaveBeenCalledTimes(2); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('throws when all retries are exhausted', async () => { @@ -126,8 +135,6 @@ describe('HTTP embedding backend', () => { const { embedText } = await import('../../src/core/embeddings/embedder.js'); await expect(embedText('test')).rejects.toThrow('500'); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('excludes API key from error messages', async () => { @@ -145,9 +152,6 @@ describe('HTTP embedding backend', () => { expect(e.message).not.toContain('Authorization'); } - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; - delete process.env.GITNEXUS_EMBEDDING_API_KEY; }); it('includes abort signal for timeout', async () => { @@ -156,7 +160,7 @@ describe('HTTP embedding backend', () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, - json: async () => ({ data: [{ embedding: [0.1] }] }), + json: async () => ({ data: [{ embedding: mockVec }] }), })); const { embedText } = await import('../../src/core/embeddings/embedder.js'); @@ -165,8 +169,6 @@ describe('HTTP embedding backend', () => { const opts = (fetch as any).mock.calls[0][1]; expect(opts.signal).toBeDefined(); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('splits large inputs into batches', async () => { @@ -175,7 +177,7 @@ describe('HTTP embedding backend', () => { const makeResp = (n: number) => ({ ok: true, - json: async () => ({ data: Array.from({ length: n }, () => ({ embedding: [0.1] })) }), + json: async () => ({ data: Array.from({ length: n }, () => ({ embedding: mockVec })) }), }); vi.stubGlobal('fetch', vi.fn() .mockResolvedValueOnce(makeResp(64)) @@ -187,8 +189,6 @@ describe('HTTP embedding backend', () => { expect(fetch).toHaveBeenCalledTimes(2); expect(results).toHaveLength(70); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('rejects initEmbedder when using HTTP backend', async () => { @@ -198,8 +198,6 @@ describe('HTTP embedding backend', () => { const { initEmbedder } = await import('../../src/core/embeddings/embedder.js'); await expect(initEmbedder()).rejects.toThrow('HTTP mode'); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('rejects getEmbedder when using HTTP backend', async () => { @@ -209,8 +207,6 @@ describe('HTTP embedding backend', () => { const { getEmbedder } = await import('../../src/core/embeddings/embedder.js'); expect(() => getEmbedder()).toThrow('HTTP embedding mode'); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('throws on empty response from endpoint', async () => { @@ -225,8 +221,6 @@ describe('HTTP embedding backend', () => { const mod = await import('../../src/mcp/core/embedder.js'); await expect(mod.embedQuery('test')).rejects.toThrow('empty response'); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('throws when endpoint returns fewer embeddings than texts', async () => { @@ -235,14 +229,12 @@ describe('HTTP embedding backend', () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, - json: async () => ({ data: [{ embedding: [0.1] }] }), + json: async () => ({ data: [{ embedding: mockVec }] }), })); const { embedBatch } = await import('../../src/core/embeddings/embedder.js'); await expect(embedBatch(['text1', 'text2', 'text3'])).rejects.toThrow('1 vectors for 3 texts'); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; }); it('throws on dimension mismatch when GITNEXUS_EMBEDDING_DIMS is set', async () => { @@ -258,9 +250,6 @@ describe('HTTP embedding backend', () => { const { embedText } = await import('../../src/core/embeddings/embedder.js'); await expect(embedText('test')).rejects.toThrow('Embedding dimension mismatch'); - delete process.env.GITNEXUS_EMBEDDING_URL; - delete process.env.GITNEXUS_EMBEDDING_MODEL; - delete process.env.GITNEXUS_EMBEDDING_DIMS; }); }); @@ -274,7 +263,65 @@ describe('HTTP embedding backend', () => { process.env.GITNEXUS_EMBEDDING_DIMS = '1024'; const { EMBEDDING_DIMS } = await import('../../src/core/lbug/schema.js'); expect(EMBEDDING_DIMS).toBe(1024); - delete process.env.GITNEXUS_EMBEDDING_DIMS; + }); + }); + + describe('timeout and network error handling', () => { + it('does not retry on timeout', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + + const timeoutErr = new DOMException('The operation was aborted due to timeout', 'TimeoutError'); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(timeoutErr)); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + await expect(embedText('test')).rejects.toThrow('timed out'); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('retries on network error then succeeds', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + + const ok = { ok: true, json: async () => ({ data: [{ embedding: mockVec }] }) }; + vi.stubGlobal('fetch', vi.fn() + .mockRejectedValueOnce(new TypeError('fetch failed')) + .mockResolvedValueOnce(ok)); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + const result = await embedText('test'); + expect(fetch).toHaveBeenCalledTimes(2); + expect(result).toBeInstanceOf(Float32Array); + }); + }); + + describe('dimension mismatch on query path', () => { + it('throws on explicit dim mismatch in embedQuery', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + process.env.GITNEXUS_EMBEDDING_DIMS = '512'; + + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ data: [{ embedding: mockVec }] }), + })); + + const mod = await import('../../src/mcp/core/embedder.js'); + await expect(mod.embedQuery('test')).rejects.toThrow('dimension mismatch'); + }); + + it('throws with Set hint when GITNEXUS_EMBEDDING_DIMS is unset', async () => { + process.env.GITNEXUS_EMBEDDING_URL = 'http://test:8080/v1'; + process.env.GITNEXUS_EMBEDDING_MODEL = 'test-model'; + + const vec768 = Array.from({ length: 768 }, (_, i) => i / 768); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ data: [{ embedding: vec768 }] }), + })); + + const { embedText } = await import('../../src/core/embeddings/embedder.js'); + await expect(embedText('test')).rejects.toThrow('Set GITNEXUS_EMBEDDING_DIMS=768'); }); }); });