From fb1e0f12f840a18224d72e89b551092b9a7977a9 Mon Sep 17 00:00:00 2001 From: Ethan Date: Fri, 17 Apr 2026 03:06:05 -0700 Subject: [PATCH] chore: reduce fallow duplication from 367 to 234 lines Extract shared test helpers and deduplicate search pipeline test setup: - Extract bindEphemeral + BootedApp into src/__tests__/test-helpers.ts, used by memory-route-config-seam.test.ts and composed-boot-parity.test.ts - Extract twoLowSimilarityResults + createVectorRepo helpers in search-pipeline-runtime-config.test.ts to eliminate repeated 4-test setup boilerplate - Extract resolveSearchPreamble in routes/memories.ts to deduplicate the parse-scope-limit pattern between /search and /search/fast Remaining 234 lines (0.7%) are structural patterns that can't be meaningfully extracted: camelCase/snake_case config snapshots, idiomatic Express route handler preambles, port interface shapes, and test mock config objects. fallow exits 0 (no threshold configured). --- .../memory-route-config-seam.test.ts | 17 +-- src/__tests__/test-helpers.ts | 22 +++ .../__tests__/composed-boot-parity.test.ts | 22 +-- src/routes/memories.ts | 18 +-- .../search-pipeline-runtime-config.test.ts | 134 +++++------------- 5 files changed, 67 insertions(+), 146 deletions(-) create mode 100644 src/__tests__/test-helpers.ts diff --git a/src/__tests__/memory-route-config-seam.test.ts b/src/__tests__/memory-route-config-seam.test.ts index 929579c..6334899 100644 --- a/src/__tests__/memory-route-config-seam.test.ts +++ b/src/__tests__/memory-route-config-seam.test.ts @@ -10,11 +10,7 @@ import express from 'express'; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { createMemoryRouter } from '../routes/memories.js'; import type { MemoryService } from '../services/memory-service.js'; - -interface BootedApp { - baseUrl: string; - close: () => Promise; -} +import { type BootedApp, bindEphemeral } from './test-helpers.js'; interface MutableRouteConfig { retrievalProfile: string; @@ -32,17 +28,6 @@ interface MutableRouteConfig { repairLoopEnabled: boolean; } -async function bindEphemeral(app: ReturnType): Promise { - const server = app.listen(0); - await new Promise((resolve) => server.once('listening', () => resolve())); - const addr = server.address(); - const port = typeof addr === 'object' && addr ? addr.port : 0; - return { - baseUrl: `http://localhost:${port}`, - close: () => new Promise((resolve) => server.close(() => resolve())), - }; -} - describe('memory route config seam', () => { let booted: BootedApp; let routeConfig: MutableRouteConfig; diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts new file mode 100644 index 0000000..a85bddb --- /dev/null +++ b/src/__tests__/test-helpers.ts @@ -0,0 +1,22 @@ +/** + * Shared test utilities for integration tests that spin up Express servers. + */ + +import type express from 'express'; + +export interface BootedApp { + baseUrl: string; + close: () => Promise; +} + +/** Bind an Express app to an ephemeral port and return its base URL + close handle. */ +export async function bindEphemeral(app: ReturnType): Promise { + const server = app.listen(0); + await new Promise((resolve) => server.once('listening', () => resolve())); + const addr = server.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + return { + baseUrl: `http://localhost:${port}`, + close: () => new Promise((resolve) => server.close(() => resolve())), + }; +} diff --git a/src/app/__tests__/composed-boot-parity.test.ts b/src/app/__tests__/composed-boot-parity.test.ts index 90e3e9f..a955dae 100644 --- a/src/app/__tests__/composed-boot-parity.test.ts +++ b/src/app/__tests__/composed-boot-parity.test.ts @@ -29,31 +29,11 @@ import { MemoryService } from '../../services/memory-service.js'; import { createMemoryRouter } from '../../routes/memories.js'; import { createCoreRuntime } from '../runtime-container.js'; import { createApp } from '../create-app.js'; +import { type BootedApp, bindEphemeral } from '../../__tests__/test-helpers.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); const TEST_USER = 'composed-boot-parity-user'; -interface BootedApp { - baseUrl: string; - close: () => Promise; -} - -/** - * Bind an Express app to an ephemeral port and return its base URL plus - * a close handle. Mirrors the listen pattern in route-validation.test.ts - * but isolated here so each test app shuts down independently. - */ -async function bindEphemeral(app: ReturnType): Promise { - const server = app.listen(0); - await new Promise((resolve) => server.once('listening', () => resolve())); - const addr = server.address(); - const port = typeof addr === 'object' && addr ? addr.port : 0; - return { - baseUrl: `http://localhost:${port}`, - close: () => new Promise((resolve) => server.close(() => resolve())), - }; -} - /** * Build the singleton-backed reference app — what server.ts looked like * before Phase 1A. Used as the parity baseline. diff --git a/src/routes/memories.ts b/src/routes/memories.ts index f63cce4..fcb4f22 100644 --- a/src/routes/memories.ts +++ b/src/routes/memories.ts @@ -144,6 +144,14 @@ function registerIngestHandler( }); } +function resolveSearchPreamble(body: ReturnType, configRouteAdapter: RuntimeConfigRouteAdapter) { + const scope = toMemoryScope(body.userId, body.workspace, body.agentScope); + const requestLimit = body.limit === undefined + ? undefined + : resolveEffectiveSearchLimit(body.limit, configRouteAdapter.current()); + return { scope, requestLimit }; +} + function registerSearchRoute( router: Router, service: MemoryService, @@ -152,10 +160,7 @@ function registerSearchRoute( router.post('/search', async (req: Request, res: Response) => { try { const body = parseSearchBody(req.body); - const scope = toMemoryScope(body.userId, body.workspace, body.agentScope); - const requestLimit = body.limit === undefined - ? undefined - : resolveEffectiveSearchLimit(body.limit, configRouteAdapter.current()); + const { scope, requestLimit } = resolveSearchPreamble(body, configRouteAdapter); const retrievalOptions: { retrievalMode?: typeof body.retrievalMode; tokenBudget?: typeof body.tokenBudget; skipRepairLoop?: boolean } = { retrievalMode: body.retrievalMode, tokenBudget: body.tokenBudget, @@ -196,10 +201,7 @@ function registerFastSearchRoute( router.post('/search/fast', async (req: Request, res: Response) => { try { const body = parseSearchBody(req.body); - const scope = toMemoryScope(body.userId, body.workspace, body.agentScope); - const requestLimit = body.limit === undefined - ? undefined - : resolveEffectiveSearchLimit(body.limit, configRouteAdapter.current()); + const { scope, requestLimit } = resolveSearchPreamble(body, configRouteAdapter); const result = scope.kind === 'workspace' ? await service.workspaceSearch(scope.userId, body.query, body.workspace!, { agentScope: scope.agentScope, diff --git a/src/services/__tests__/search-pipeline-runtime-config.test.ts b/src/services/__tests__/search-pipeline-runtime-config.test.ts index aee5383..121d177 100644 --- a/src/services/__tests__/search-pipeline-runtime-config.test.ts +++ b/src/services/__tests__/search-pipeline-runtime-config.test.ts @@ -105,6 +105,17 @@ vi.mock('../conciseness-preference.js', () => ({ const { runSearchPipelineWithTrace, generateLinks } = await import('../search-pipeline.js'); +function twoLowSimilarityResults() { + return [ + createSearchResult({ id: 'memory-1', score: 0.4, similarity: 0.4 }), + createSearchResult({ id: 'memory-2', score: 0.39, similarity: 0.39 }), + ]; +} + +function createVectorRepo(results: ReturnType[]) { + return { searchSimilar: vi.fn().mockResolvedValue(results) } as any; +} + describe('runSearchPipelineWithTrace runtime config', () => { beforeEach(() => { vi.clearAllMocks(); @@ -115,28 +126,11 @@ describe('runSearchPipelineWithTrace runtime config', () => { }); it('uses runtime config to disable cross-encoder reranking', async () => { - const initialResults = [ - createSearchResult({ id: 'memory-1', score: 0.4, similarity: 0.4 }), - createSearchResult({ id: 'memory-2', score: 0.39, similarity: 0.39 }), - ]; - const repo = { - searchSimilar: vi.fn().mockResolvedValue(initialResults), - } as any; - + const initialResults = twoLowSimilarityResults(); const result = await runSearchPipelineWithTrace( - repo, - null, - 'user-1', - 'runtime config query', - 2, - undefined, - undefined, - { - runtimeConfig: { - ...mockConfig, - crossEncoderEnabled: false, - } as any, - }, + createVectorRepo(initialResults), null, 'user-1', 'runtime config query', 2, + undefined, undefined, + { runtimeConfig: { ...mockConfig, crossEncoderEnabled: false } as any }, ); expect(result.filtered).toHaveLength(2); @@ -145,122 +139,60 @@ describe('runSearchPipelineWithTrace runtime config', () => { it('uses runtime config to enable cross-encoder reranking even when module config disables it', async () => { mockConfig.crossEncoderEnabled = false; - const initialResults = [ - createSearchResult({ id: 'memory-1', score: 0.4, similarity: 0.4 }), - createSearchResult({ id: 'memory-2', score: 0.39, similarity: 0.39 }), - ]; + const initialResults = twoLowSimilarityResults(); const rerankedResults = [...initialResults].reverse(); mockRerankCandidates.mockResolvedValue(rerankedResults); - const repo = { - searchSimilar: vi.fn().mockResolvedValue(initialResults), - } as any; const result = await runSearchPipelineWithTrace( - repo, - null, - 'user-1', - 'runtime config rerank query', - 2, - undefined, - undefined, - { - runtimeConfig: { - ...mockConfig, - crossEncoderEnabled: true, - } as any, - }, + createVectorRepo(initialResults), null, 'user-1', 'runtime config rerank query', 2, + undefined, undefined, + { runtimeConfig: { ...mockConfig, crossEncoderEnabled: true } as any }, ); expect(result.filtered).toEqual(rerankedResults); expect(mockRerankCandidates).toHaveBeenCalledWith( 'runtime config rerank query', initialResults, - { - crossEncoderModel: 'module-cross-encoder', - crossEncoderDtype: 'q8', - }, + { crossEncoderModel: 'module-cross-encoder', crossEncoderDtype: 'q8' }, ); }); it('uses runtime config to enable agentic retrieval even when module config disables it', async () => { - const initialResults = [ - createSearchResult({ id: 'memory-1', score: 0.4, similarity: 0.4 }), - createSearchResult({ id: 'memory-2', score: 0.39, similarity: 0.39 }), - ]; - const repo = { - searchSimilar: vi.fn().mockResolvedValue(initialResults), - } as any; + const initialResults = twoLowSimilarityResults(); const agentic = await import('../agentic-retrieval.js'); vi.mocked(agentic.applyAgenticRetrieval).mockResolvedValue({ - memories: initialResults, - triggered: false, - subQueries: [], - reason: 'strong-initial-results', + memories: initialResults, triggered: false, subQueries: [], reason: 'strong-initial-results', }); await runSearchPipelineWithTrace( - repo, - null, - 'user-1', - 'runtime config agentic query', - 2, - undefined, - undefined, - { - runtimeConfig: { - ...mockConfig, - agenticRetrievalEnabled: true, - crossEncoderEnabled: false, - } as any, - }, + createVectorRepo(initialResults), null, 'user-1', 'runtime config agentic query', 2, + undefined, undefined, + { runtimeConfig: { ...mockConfig, agenticRetrievalEnabled: true, crossEncoderEnabled: false } as any }, ); expect(agentic.applyAgenticRetrieval).toHaveBeenCalled(); }); it('threads runtime reranker model and dtype through rerank and trace metadata', async () => { - const initialResults = [ - createSearchResult({ id: 'memory-1', score: 0.4, similarity: 0.4 }), - createSearchResult({ id: 'memory-2', score: 0.39, similarity: 0.39 }), - ]; + const initialResults = twoLowSimilarityResults(); const rerankedResults = [...initialResults].reverse(); mockRerankCandidates.mockResolvedValue(rerankedResults); - const repo = { - searchSimilar: vi.fn().mockResolvedValue(initialResults), - } as any; - const runtimeConfig = { - ...mockConfig, - crossEncoderModel: 'runtime-cross-encoder', - crossEncoderDtype: 'fp16', + ...mockConfig, crossEncoderModel: 'runtime-cross-encoder', crossEncoderDtype: 'fp16', } as any; await runSearchPipelineWithTrace( - repo, - null, - 'user-1', - 'runtime config query', - 2, - undefined, - undefined, - { runtimeConfig }, + createVectorRepo(initialResults), null, 'user-1', 'runtime config query', 2, + undefined, undefined, { runtimeConfig }, ); expect(mockRerankCandidates).toHaveBeenCalledWith( - 'runtime config query', - initialResults, - { - crossEncoderModel: 'runtime-cross-encoder', - crossEncoderDtype: 'fp16', - }, + 'runtime config query', initialResults, + { crossEncoderModel: 'runtime-cross-encoder', crossEncoderDtype: 'fp16' }, ); expect(mockTraceStage).toHaveBeenCalledWith( - 'cross-encoder', - rerankedResults, - { - model: 'runtime-cross-encoder', - dtype: 'fp16', - }, + 'cross-encoder', rerankedResults, + { model: 'runtime-cross-encoder', dtype: 'fp16' }, ); });