Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/test-file-hints.md
Original file line number Diff line number Diff line change
@@ -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

33 changes: 32 additions & 1 deletion PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
1 change: 1 addition & 0 deletions packages/mcp-server/bin/dev-agent-mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ async function main() {
// Create and register adapters
const searchAdapter = new SearchAdapter({
repositoryIndexer: indexer,
repositoryPath,
defaultFormat: 'compact',
defaultLimit: 10,
});
Expand Down
36 changes: 34 additions & 2 deletions packages/mcp-server/src/adapters/built-in/search-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -17,6 +18,11 @@ export interface SearchAdapterConfig {
*/
repositoryIndexer: RepositoryIndexer;

/**
* Repository root path (for finding related files)
*/
repositoryPath?: string;

/**
* Default format mode
*/
Expand All @@ -26,6 +32,11 @@ export interface SearchAdapterConfig {
* Default result limit
*/
defaultLimit?: number;

/**
* Include related test files in results
*/
includeRelatedFiles?: boolean;
}

/**
Expand All @@ -41,15 +52,19 @@ export class SearchAdapter extends ToolAdapter {
};

private indexer: RepositoryIndexer;
private config: Required<SearchAdapterConfig>;
private config: Required<Omit<SearchAdapterConfig, 'repositoryPath'>> & {
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,
};
}

Expand Down Expand Up @@ -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,
});
Expand All @@ -224,7 +255,7 @@ export class SearchAdapter extends ToolAdapter {
data: {
query,
format,
content: formatted.content,
content: formatted.content + relatedFilesSection,
},
metadata: {
tokens: formatted.tokens,
Expand All @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions packages/mcp-server/src/adapters/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions packages/mcp-server/src/formatters/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
});

Expand Down
192 changes: 192 additions & 0 deletions packages/mcp-server/src/utils/__tests__/related-files.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
Loading