diff --git a/.changeset/test-file-hints.md b/.changeset/test-file-hints.md new file mode 100644 index 0000000..98ff003 --- /dev/null +++ b/.changeset/test-file-hints.md @@ -0,0 +1,15 @@ +--- +"@lytics/dev-agent-mcp": patch +"@lytics/dev-agent": patch +--- + +### Features + +- **Test file hints in search results**: `dev_search` now shows related test files (e.g., `utils.test.ts`) after search results. This surfaces test files without polluting semantic search rankings. + +### Design + +- Uses structural matching (`.test.ts`, `.spec.ts` patterns) rather than semantic search +- Keeps semantic search pure - test hints are in a separate "Related test files:" section +- Patterns are configurable for future extensibility via function parameters + diff --git a/PLAN.md b/PLAN.md index 8970494..b77868a 100644 --- a/PLAN.md +++ b/PLAN.md @@ -192,11 +192,42 @@ Git history is valuable context that LLMs can't easily access. We add intelligen | Task | Gap Identified | Priority | Status | |------|----------------|----------|--------| | Diagnostic command suggestions | Baseline provided shell commands for debugging; dev-agent didn't | 🔴 High | 🔲 Todo | -| Test file inclusion hints | Baseline read test files; dev-agent skipped them | 🔴 High | 🔲 Todo | +| Test file inclusion hints | Baseline read test files; dev-agent skipped them | 🔴 High | ✅ Done | | Code example extraction | Baseline included more code snippets in responses | 🟡 Medium | 🔲 Todo | | Exhaustive mode for debugging | Option for thorough exploration vs fast satisficing | 🟡 Medium | 🔲 Todo | | Related files suggestions | "You might also want to check: X, Y, Z" | 🟡 Medium | 🔲 Todo | +### Test File Hints - Design (v0.4.4) + +**Problem:** Benchmark showed baseline Claude read test files, but dev-agent didn't surface them. + +**Root cause:** Test files have *structural* relationship (same name), not *semantic* relationship: +- Searching "authentication middleware" finds `auth/middleware.ts` (semantic match ✓) +- `auth/middleware.test.ts` might NOT appear because test semantics differ +- "describe('AuthMiddleware', () => {...})" doesn't semantically match the query + +**Design decision:** Keep semantic search pure. Add "Related files:" section using filesystem lookup. + +| Approach | Decision | Rationale | +|----------|----------|-----------| +| Filename matching | ✅ Chosen | Fast, reliable, honest about what it is | +| Boost test queries | ❌ Rejected | Might return unrelated tests | +| Index-time metadata | ❌ Rejected | Requires re-index, complex | + +**Phased rollout:** + +| Phase | Tool | Status | +|-------|------|--------| +| 1 (v0.4.4) | `dev_search` | ✅ Done | +| 2 | `dev_refs`, `dev_explore` | 🔲 Todo | +| 3 | `dev_map`, `dev_status` | 🔲 Todo | + +**Implementation (Phase 1):** +- After search results, check filesystem for test siblings +- Patterns: `*.test.ts`, `*.spec.ts`, `__tests__/*.ts` +- Add "Related files:" section to output +- No change to semantic search scoring + ### Tool Description Refinements (Done in v0.4.2) | Task | Status | diff --git a/packages/mcp-server/bin/dev-agent-mcp.ts b/packages/mcp-server/bin/dev-agent-mcp.ts index 97ba037..37c1383 100644 --- a/packages/mcp-server/bin/dev-agent-mcp.ts +++ b/packages/mcp-server/bin/dev-agent-mcp.ts @@ -145,6 +145,7 @@ async function main() { // Create and register adapters const searchAdapter = new SearchAdapter({ repositoryIndexer: indexer, + repositoryPath, defaultFormat: 'compact', defaultLimit: 10, }); diff --git a/packages/mcp-server/src/adapters/built-in/search-adapter.ts b/packages/mcp-server/src/adapters/built-in/search-adapter.ts index 0080965..3ca969a 100644 --- a/packages/mcp-server/src/adapters/built-in/search-adapter.ts +++ b/packages/mcp-server/src/adapters/built-in/search-adapter.ts @@ -5,6 +5,7 @@ import type { RepositoryIndexer } from '@lytics/dev-agent-core'; import { CompactFormatter, type FormatMode, VerboseFormatter } from '../../formatters'; +import { findRelatedTestFiles, formatRelatedFiles } from '../../utils/related-files'; import { ToolAdapter } from '../tool-adapter'; import type { AdapterContext, ToolDefinition, ToolExecutionContext, ToolResult } from '../types'; @@ -17,6 +18,11 @@ export interface SearchAdapterConfig { */ repositoryIndexer: RepositoryIndexer; + /** + * Repository root path (for finding related files) + */ + repositoryPath?: string; + /** * Default format mode */ @@ -26,6 +32,11 @@ export interface SearchAdapterConfig { * Default result limit */ defaultLimit?: number; + + /** + * Include related test files in results + */ + includeRelatedFiles?: boolean; } /** @@ -41,15 +52,19 @@ export class SearchAdapter extends ToolAdapter { }; private indexer: RepositoryIndexer; - private config: Required; + private config: Required> & { + repositoryPath?: string; + }; constructor(config: SearchAdapterConfig) { super(); this.indexer = config.repositoryIndexer; this.config = { repositoryIndexer: config.repositoryIndexer, + repositoryPath: config.repositoryPath, defaultFormat: config.defaultFormat ?? 'compact', defaultLimit: config.defaultLimit ?? 10, + includeRelatedFiles: config.includeRelatedFiles ?? true, }; } @@ -210,11 +225,27 @@ export class SearchAdapter extends ToolAdapter { const formatted = formatter.formatResults(results); + // Find related test files if enabled and repository path is available + let relatedFilesSection = ''; + let relatedFilesCount = 0; + if (this.config.includeRelatedFiles && this.config.repositoryPath && results.length > 0) { + const sourcePaths = results + .map((r) => r.metadata.path) + .filter((p): p is string => typeof p === 'string'); + + if (sourcePaths.length > 0) { + const relatedFiles = await findRelatedTestFiles(sourcePaths, this.config.repositoryPath); + relatedFilesCount = relatedFiles.length; + relatedFilesSection = formatRelatedFiles(relatedFiles); + } + } + const duration_ms = Date.now() - startTime; context.logger.info('Search completed', { query, resultCount: results.length, + relatedFilesCount, tokens: formatted.tokens, duration_ms, }); @@ -224,7 +255,7 @@ export class SearchAdapter extends ToolAdapter { data: { query, format, - content: formatted.content, + content: formatted.content + relatedFilesSection, }, metadata: { tokens: formatted.tokens, @@ -234,6 +265,7 @@ export class SearchAdapter extends ToolAdapter { results_total: results.length, results_returned: Math.min(results.length, limit as number), results_truncated: results.length > (limit as number), + related_files_count: relatedFilesCount, }, }; } catch (error) { diff --git a/packages/mcp-server/src/adapters/types.ts b/packages/mcp-server/src/adapters/types.ts index e14ce17..4c01133 100644 --- a/packages/mcp-server/src/adapters/types.ts +++ b/packages/mcp-server/src/adapters/types.ts @@ -65,6 +65,10 @@ export interface MCPMetadata { results_returned?: number; /** Whether results were truncated due to limits */ results_truncated?: boolean; + + // Related files (optional) + /** Number of related test files found */ + related_files_count?: number; } // Tool Result diff --git a/packages/mcp-server/src/formatters/__tests__/utils.test.ts b/packages/mcp-server/src/formatters/__tests__/utils.test.ts index 595821e..7da44fd 100644 --- a/packages/mcp-server/src/formatters/__tests__/utils.test.ts +++ b/packages/mcp-server/src/formatters/__tests__/utils.test.ts @@ -208,11 +208,11 @@ describe('Formatter Utils', () => { it('should return elapsed time', async () => { const timer = startTimer(); - // Wait a bit - await new Promise((resolve) => setTimeout(resolve, 10)); + // Wait a bit (use 15ms to avoid flaky timing issues) + await new Promise((resolve) => setTimeout(resolve, 15)); const elapsed = timer.elapsed(); - expect(elapsed).toBeGreaterThanOrEqual(10); + expect(elapsed).toBeGreaterThanOrEqual(10); // Allow some timing variance expect(elapsed).toBeLessThan(100); // Should be fast }); diff --git a/packages/mcp-server/src/utils/__tests__/related-files.test.ts b/packages/mcp-server/src/utils/__tests__/related-files.test.ts new file mode 100644 index 0000000..adb80ff --- /dev/null +++ b/packages/mcp-server/src/utils/__tests__/related-files.test.ts @@ -0,0 +1,192 @@ +/** + * Tests for Related Files Utility + */ + +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { + DEFAULT_TEST_PATTERNS, + findRelatedTestFiles, + findTestFile, + formatRelatedFiles, + type RelatedFile, + type TestPatternFn, +} from '../related-files'; + +describe('Related Files Utility', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'related-files-test-')); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + describe('findTestFile', () => { + it('should find .test.ts sibling', async () => { + // Create source and test files + await fs.writeFile(path.join(tempDir, 'utils.ts'), 'export const x = 1;'); + await fs.writeFile(path.join(tempDir, 'utils.test.ts'), 'test("x", () => {});'); + + const result = await findTestFile('utils.ts', tempDir); + + expect(result).not.toBeNull(); + expect(result?.relatedPath).toBe('utils.test.ts'); + expect(result?.type).toBe('test'); + }); + + it('should find .spec.ts sibling', async () => { + await fs.writeFile(path.join(tempDir, 'utils.ts'), 'export const x = 1;'); + await fs.writeFile(path.join(tempDir, 'utils.spec.ts'), 'test("x", () => {});'); + + const result = await findTestFile('utils.ts', tempDir); + + expect(result).not.toBeNull(); + expect(result?.relatedPath).toBe('utils.spec.ts'); + expect(result?.type).toBe('spec'); + }); + + it('should return null for files without tests', async () => { + await fs.writeFile(path.join(tempDir, 'utils.ts'), 'export const x = 1;'); + + const result = await findTestFile('utils.ts', tempDir); + + expect(result).toBeNull(); + }); + + it('should return null for test files (skip self)', async () => { + await fs.writeFile(path.join(tempDir, 'utils.test.ts'), 'test("x", () => {});'); + + const result = await findTestFile('utils.test.ts', tempDir); + + expect(result).toBeNull(); + }); + + it('should return null for spec files (skip self)', async () => { + await fs.writeFile(path.join(tempDir, 'utils.spec.ts'), 'test("x", () => {});'); + + const result = await findTestFile('utils.spec.ts', tempDir); + + expect(result).toBeNull(); + }); + + it('should handle nested directories', async () => { + await fs.mkdir(path.join(tempDir, 'src', 'utils'), { recursive: true }); + await fs.writeFile(path.join(tempDir, 'src', 'utils', 'helper.ts'), 'export const x = 1;'); + await fs.writeFile( + path.join(tempDir, 'src', 'utils', 'helper.test.ts'), + 'test("x", () => {});' + ); + + const result = await findTestFile('src/utils/helper.ts', tempDir); + + expect(result).not.toBeNull(); + expect(result?.relatedPath).toBe('src/utils/helper.test.ts'); + }); + }); + + describe('findRelatedTestFiles', () => { + it('should find test files for multiple sources', async () => { + await fs.writeFile(path.join(tempDir, 'a.ts'), 'export const a = 1;'); + await fs.writeFile(path.join(tempDir, 'a.test.ts'), 'test("a", () => {});'); + await fs.writeFile(path.join(tempDir, 'b.ts'), 'export const b = 1;'); + await fs.writeFile(path.join(tempDir, 'b.test.ts'), 'test("b", () => {});'); + await fs.writeFile(path.join(tempDir, 'c.ts'), 'export const c = 1;'); + // No test for c.ts + + const results = await findRelatedTestFiles(['a.ts', 'b.ts', 'c.ts'], tempDir); + + expect(results).toHaveLength(2); + expect(results.map((r) => r.relatedPath).sort()).toEqual(['a.test.ts', 'b.test.ts']); + }); + + it('should deduplicate source paths', async () => { + await fs.writeFile(path.join(tempDir, 'utils.ts'), 'export const x = 1;'); + await fs.writeFile(path.join(tempDir, 'utils.test.ts'), 'test("x", () => {});'); + + const results = await findRelatedTestFiles(['utils.ts', 'utils.ts', 'utils.ts'], tempDir); + + expect(results).toHaveLength(1); + }); + + it('should return empty array when no tests found', async () => { + await fs.writeFile(path.join(tempDir, 'a.ts'), 'export const a = 1;'); + await fs.writeFile(path.join(tempDir, 'b.ts'), 'export const b = 1;'); + + const results = await findRelatedTestFiles(['a.ts', 'b.ts'], tempDir); + + expect(results).toEqual([]); + }); + + it('should return empty array for empty input', async () => { + const results = await findRelatedTestFiles([], tempDir); + + expect(results).toEqual([]); + }); + }); + + describe('formatRelatedFiles', () => { + it('should format related files as string', () => { + const files: RelatedFile[] = [ + { sourcePath: 'utils.ts', relatedPath: 'utils.test.ts', type: 'test' }, + { sourcePath: 'helper.ts', relatedPath: 'helper.spec.ts', type: 'spec' }, + ]; + + const result = formatRelatedFiles(files); + + expect(result).toContain('Related test files:'); + expect(result).toContain('utils.test.ts'); + expect(result).toContain('helper.spec.ts'); + }); + + it('should return empty string for empty array', () => { + const result = formatRelatedFiles([]); + + expect(result).toBe(''); + }); + + it('should include separator line', () => { + const files: RelatedFile[] = [ + { sourcePath: 'utils.ts', relatedPath: 'utils.test.ts', type: 'test' }, + ]; + + const result = formatRelatedFiles(files); + + expect(result).toContain('---'); + }); + }); + + describe('custom patterns', () => { + it('should support custom test patterns', async () => { + // Create a custom pattern for __tests__ directory + const customPatterns: TestPatternFn[] = [ + ...DEFAULT_TEST_PATTERNS, + (base, ext, dir) => path.join(dir, '__tests__', `${path.basename(base)}${ext}`), + ]; + + await fs.mkdir(path.join(tempDir, '__tests__')); + await fs.writeFile(path.join(tempDir, 'utils.ts'), 'export const x = 1;'); + await fs.writeFile(path.join(tempDir, '__tests__', 'utils.ts'), 'test("x", () => {});'); + + const result = await findTestFile('utils.ts', tempDir, customPatterns); + + expect(result).not.toBeNull(); + expect(result?.relatedPath).toBe('__tests__/utils.ts'); + }); + + it('should use default patterns when none provided', async () => { + await fs.writeFile(path.join(tempDir, 'utils.ts'), 'export const x = 1;'); + await fs.writeFile(path.join(tempDir, 'utils.test.ts'), 'test("x", () => {});'); + + // Call without patterns parameter - should use defaults + const result = await findTestFile('utils.ts', tempDir); + + expect(result).not.toBeNull(); + expect(result?.relatedPath).toBe('utils.test.ts'); + }); + }); +}); diff --git a/packages/mcp-server/src/utils/related-files.ts b/packages/mcp-server/src/utils/related-files.ts new file mode 100644 index 0000000..d53b3ce --- /dev/null +++ b/packages/mcp-server/src/utils/related-files.ts @@ -0,0 +1,154 @@ +/** + * Related Files Utility + * Finds structurally related files (test files, type files, etc.) + * + * Design: Accepts patterns as parameters for future configurability. + * Currently uses simple heuristics (*.test.*, *.spec.*) that match + * common conventions across Jest, Vitest, Mocha, etc. + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; + +/** + * Related file information + */ +export interface RelatedFile { + /** Path to the source file */ + sourcePath: string; + /** Path to the related file */ + relatedPath: string; + /** Type of relationship */ + type: 'test' | 'spec' | 'types' | 'index'; +} + +/** + * Pattern function type for generating test file paths + */ +export type TestPatternFn = (base: string, ext: string, dir: string) => string; + +/** + * Default test file patterns (covers most JS/TS projects) + * - foo.ts -> foo.test.ts + * - foo.ts -> foo.spec.ts + * + * Note: Can be extended via config in future versions to support + * project-specific patterns like __tests__/, tests/, etc. + */ +export const DEFAULT_TEST_PATTERNS: TestPatternFn[] = [ + // Same directory: foo.ts -> foo.test.ts, foo.spec.ts + (base: string, ext: string) => `${base}.test${ext}`, + (base: string, ext: string) => `${base}.spec${ext}`, +]; + +/** + * Check if a file exists + */ +async function fileExists(filePath: string): Promise { + try { + await fs.access(filePath); + return true; + } catch { + return false; + } +} + +/** + * Check if a path looks like a test file + */ +function isTestFile(filePath: string): boolean { + return filePath.includes('.test.') || filePath.includes('.spec.'); +} + +/** + * Find test file for a given source file + * + * @param sourcePath - Path to the source file (relative to repository) + * @param repositoryPath - Root path of the repository + * @param patterns - Test file patterns to check (defaults to DEFAULT_TEST_PATTERNS) + * @returns Related test file if found, null otherwise + */ +export async function findTestFile( + sourcePath: string, + repositoryPath: string, + patterns: TestPatternFn[] = DEFAULT_TEST_PATTERNS +): Promise { + // Skip if already a test file + if (isTestFile(sourcePath)) { + return null; + } + + const ext = path.extname(sourcePath); + const base = sourcePath.slice(0, -ext.length); + const dir = path.dirname(sourcePath); + const fullDir = path.join(repositoryPath, dir); + + for (const pattern of patterns) { + const testPath = pattern(base, ext, fullDir); + const fullTestPath = testPath.startsWith(repositoryPath) + ? testPath + : path.join(repositoryPath, testPath); + + if (await fileExists(fullTestPath)) { + // Return relative path + const relatedPath = path.relative(repositoryPath, fullTestPath); + const type = relatedPath.includes('.spec.') ? 'spec' : 'test'; + return { + sourcePath, + relatedPath, + type, + }; + } + } + + return null; +} + +/** + * Find related test files for multiple source files + * + * @param sourcePaths - Array of source file paths (relative to repository) + * @param repositoryPath - Root path of the repository + * @param patterns - Test file patterns to check (defaults to DEFAULT_TEST_PATTERNS) + * @returns Array of related files found + */ +export async function findRelatedTestFiles( + sourcePaths: string[], + repositoryPath: string, + patterns: TestPatternFn[] = DEFAULT_TEST_PATTERNS +): Promise { + // Deduplicate source paths + const uniquePaths = [...new Set(sourcePaths)]; + + const results = await Promise.all( + uniquePaths.map((sourcePath) => findTestFile(sourcePath, repositoryPath, patterns)) + ); + + // Filter out nulls and deduplicate by relatedPath + const seen = new Set(); + return results.filter((result): result is RelatedFile => { + if (result === null) return false; + if (seen.has(result.relatedPath)) return false; + seen.add(result.relatedPath); + return true; + }); +} + +/** + * Format related files as a string for output + * + * @param relatedFiles - Array of related files + * @returns Formatted string + */ +export function formatRelatedFiles(relatedFiles: RelatedFile[]): string { + if (relatedFiles.length === 0) { + return ''; + } + + const lines = ['', '---', 'Related test files:']; + for (const file of relatedFiles) { + lines.push(` • ${file.relatedPath}`); + } + + return lines.join('\n'); +}