Skip to content

Commit 2bac9dd

Browse files
prosdevclaude
andcommitted
refactor(core): parse once per file, not 3x — extractors take pre-computed AST
Address review findings: 1. runQueries doing too much: extracted runAllAstQueries() that parses once and runs all 12 queries. Extractors now take a pre-computed Map<string, number> instead of matcher+filePath (pure functions). 2. 3x parsing eliminated: analyzeFileFromIndex and analyzeFileWithDocs call runAllAstQueries once, pass results to all 3 extractors. 3. Extractors are now synchronous pure functions — easier to test, no async overhead, no hidden parsing. Tests updated to match new signatures. Same 51 test count, same coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fe09e23 commit 2bac9dd

3 files changed

Lines changed: 81 additions & 91 deletions

File tree

.claude/scratchpad.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
- Vue/Svelte SFC support — `.vue`/`.svelte` files have embedded `<script lang="ts">` blocks. Would need script block extraction before tree-sitter parsing. Lower priority — co-located `.ts` files in those projects already get full analysis.
1818
- Swap `WasmPatternMatcher` to `@ast-grep/napi` if bulk scanning perf becomes an issue (~4x faster native Rust). Interface is ready; implementation is mechanical.
1919

20+
## Test Gaps
21+
22+
- **InspectAdapter integration test with PatternMatcher.** The InspectAdapter test constructs without a `patternMatcher` — the AST path is never exercised through the MCP layer. Needs a test that constructs `InspectAdapter` with `createPatternMatcher()`, mocks the search service, calls `execute()`, and verifies AST-enhanced results flow through. Requires mock search service setup — larger integration test scope.
23+
2024
## Notes
2125

2226
- Both pattern analysis paths (index vs scan) must use the same pure extractors from 1.1 to avoid drift. Test this explicitly.

packages/core/src/pattern-matcher/__tests__/wasm-matcher.test.ts

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
extractImportStyleWithAst,
1717
extractTypeCoverageFromSignatures,
1818
extractTypeCoverageWithAst,
19+
runAllAstQueries,
1920
} from '../../services/pattern-analysis-service';
2021
import {
2122
ALL_QUERIES,
@@ -404,50 +405,49 @@ describe('extractErrorHandlingWithAst', () => {
404405

405406
it('throw + try-catch → mixed', async () => {
406407
const source = 'try { validate(x); } catch (e) { throw new Error("failed"); }';
407-
const result = await extractErrorHandlingWithAst(source, 'test.ts', matcher);
408-
expect(result.style).toBe('mixed');
408+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
409+
expect(extractErrorHandlingWithAst(source, ast).style).toBe('mixed');
409410
});
410411

411412
it('throw only → throw', async () => {
412413
const source = 'throw new Error("bad");';
413-
const result = await extractErrorHandlingWithAst(source, 'test.ts', matcher);
414-
expect(result.style).toBe('throw');
414+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
415+
expect(extractErrorHandlingWithAst(source, ast).style).toBe('throw');
415416
});
416417

417418
it('try-catch without throw → falls through to regex', async () => {
418-
// try-catch alone is a mechanism, not a style — falls through to regex
419419
const source = 'try { x(); } catch (e) { console.log(e); }';
420-
const result = await extractErrorHandlingWithAst(source, 'test.ts', matcher);
420+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
421421
// regex sees no throw/Result either → 'unknown'
422-
expect(result.style).toBe('unknown');
422+
expect(extractErrorHandlingWithAst(source, ast).style).toBe('unknown');
423423
});
424424

425-
it('Result<T> from regex + try-catch from AST → mixed', async () => {
425+
it('Result<T> from regex + try-catch from AST → result', async () => {
426426
const source = 'function f(): Result<string> { try { return ok(); } catch { return err(); } }';
427-
const result = await extractErrorHandlingWithAst(source, 'test.ts', matcher);
428-
// regex detects Result<T>, AST detects try-catch but no throw → regex returns 'result'
429-
// then the extractor sees hasThrow=false, hasResultRegex=true → returns regex result
430-
expect(result.style).toBe('result');
427+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
428+
expect(extractErrorHandlingWithAst(source, ast).style).toBe('result');
431429
});
432430

433-
it('promise.catch only → unknown (no style mapping)', async () => {
431+
it('promise.catch only → unknown', async () => {
434432
const source = 'fetch("/api").catch(log);';
435-
const result = await extractErrorHandlingWithAst(source, 'test.ts', matcher);
436-
expect(result.style).toBe('unknown');
433+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
434+
expect(extractErrorHandlingWithAst(source, ast).style).toBe('unknown');
437435
});
438436

439-
it('matcher=undefined → identical to regex', async () => {
437+
it('empty AST map → identical to regex', () => {
440438
const source = 'throw new Error("bad"); function f(): Result<string> {}';
441-
const withMatcher = await extractErrorHandlingWithAst(source, 'test.ts', undefined);
439+
const result = extractErrorHandlingWithAst(source, new Map());
442440
const regexOnly = extractErrorHandlingFromContent(source);
443-
expect(withMatcher).toEqual(regexOnly);
441+
expect(result).toEqual(regexOnly);
444442
});
445443

446-
it('unsupported extension → falls back to regex', async () => {
444+
it('unsupported extension → runAllAstQueries returns empty → regex', async () => {
447445
const source = 'throw new Error("bad");';
448-
const result = await extractErrorHandlingWithAst(source, 'test.py', matcher);
449-
const regexOnly = extractErrorHandlingFromContent(source);
450-
expect(result).toEqual(regexOnly);
446+
const ast = await runAllAstQueries(source, 'test.py', matcher);
447+
expect(ast.size).toBe(0); // unsupported language
448+
expect(extractErrorHandlingWithAst(source, ast)).toEqual(
449+
extractErrorHandlingFromContent(source)
450+
);
451451
});
452452
});
453453

@@ -460,29 +460,29 @@ describe('extractImportStyleWithAst', () => {
460460

461461
it('static ESM + dynamic import → esm', async () => {
462462
const source = 'import { foo } from "./bar";\nconst m = await import("./baz");';
463-
const result = await extractImportStyleWithAst(source, 'test.ts', matcher);
463+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
464+
const result = extractImportStyleWithAst(source, ast);
464465
expect(result.style).toBe('esm');
465-
// importCount should include the dynamic import
466466
expect(result.importCount).toBeGreaterThan(1);
467467
});
468468

469469
it('require detected by AST → cjs', async () => {
470470
const source = 'const fs = require("fs");';
471-
const result = await extractImportStyleWithAst(source, 'test.ts', matcher);
472-
expect(result.style).toBe('cjs');
471+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
472+
expect(extractImportStyleWithAst(source, ast).style).toBe('cjs');
473473
});
474474

475475
it('mixed: ESM import + require → mixed', async () => {
476476
const source = 'import foo from "./bar";\nconst fs = require("fs");';
477-
const result = await extractImportStyleWithAst(source, 'test.ts', matcher);
478-
expect(result.style).toBe('mixed');
477+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
478+
expect(extractImportStyleWithAst(source, ast).style).toBe('mixed');
479479
});
480480

481-
it('matcher=undefined → identical to regex', async () => {
481+
it('empty AST map → identical to regex', () => {
482482
const source = 'import foo from "./bar";';
483-
const withMatcher = await extractImportStyleWithAst(source, 'test.ts', undefined);
484-
const regexOnly = extractImportStyleFromContent(source);
485-
expect(withMatcher).toEqual(regexOnly);
483+
expect(extractImportStyleWithAst(source, new Map())).toEqual(
484+
extractImportStyleFromContent(source)
485+
);
486486
});
487487
});
488488

@@ -498,44 +498,45 @@ describe('extractTypeCoverageWithAst', () => {
498498
'const add = (a: number, b: number): number => a + b;',
499499
'function greet(name: string): string { return name; }',
500500
].join('\n');
501-
const result = await extractTypeCoverageWithAst(source, 'test.ts', matcher, []);
501+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
502+
const result = extractTypeCoverageWithAst(source, ast, []);
502503
expect(result.coverage).toBe('full');
503504
expect(result.annotatedCount).toBe(2);
504505
expect(result.totalCount).toBe(2);
505506
});
506507

507-
it('some functions typed → partial (accurate denominator)', async () => {
508+
it('some functions typed → minimal (accurate denominator)', async () => {
508509
const source = [
509510
'const typed = (a: number): number => a;',
510511
'const untyped = (a) => a;',
511512
'function alsoUntyped(x) { return x; }',
512513
].join('\n');
513-
const result = await extractTypeCoverageWithAst(source, 'test.ts', matcher, []);
514+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
515+
const result = extractTypeCoverageWithAst(source, ast, []);
514516
expect(result.coverage).toBe('minimal'); // 1 of 3
515517
expect(result.annotatedCount).toBe(1);
516518
expect(result.totalCount).toBe(3);
517519
});
518520

519521
it('no functions → none', async () => {
520522
const source = 'const x = 1;';
521-
const result = await extractTypeCoverageWithAst(source, 'test.ts', matcher, []);
522-
expect(result.coverage).toBe('none');
523+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
524+
expect(extractTypeCoverageWithAst(source, ast, []).coverage).toBe('none');
523525
});
524526

525527
it('AST detects arrows that signatures miss', async () => {
526-
// Signatures from index don't include arrow functions
527528
const source = 'const add = (a: number, b: number): number => a + b;';
528-
const signatures: string[] = []; // index has no signatures for this
529-
const result = await extractTypeCoverageWithAst(source, 'test.ts', matcher, signatures);
529+
const ast = await runAllAstQueries(source, 'test.ts', matcher);
530+
const result = extractTypeCoverageWithAst(source, ast, []);
530531
expect(result.annotatedCount).toBe(1);
531532
expect(result.totalCount).toBe(1);
532533
expect(result.coverage).toBe('full');
533534
});
534535

535-
it('matcher=undefined → identical to regex signatures', async () => {
536+
it('empty AST map → identical to regex signatures', () => {
536537
const signatures = ['function foo(x: string): number'];
537-
const withMatcher = await extractTypeCoverageWithAst('', 'test.ts', undefined, signatures);
538-
const regexOnly = extractTypeCoverageFromSignatures(signatures);
539-
expect(withMatcher).toEqual(regexOnly);
538+
expect(extractTypeCoverageWithAst('', new Map(), signatures)).toEqual(
539+
extractTypeCoverageFromSignatures(signatures)
540+
);
540541
});
541542
});

packages/core/src/services/pattern-analysis-service.ts

Lines changed: 32 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,8 @@
77

88
import * as fs from 'node:fs/promises';
99
import * as path from 'node:path';
10-
import {
11-
ERROR_HANDLING_QUERIES,
12-
IMPORT_STYLE_QUERIES,
13-
TYPE_COVERAGE_QUERIES,
14-
} from '../pattern-matcher/rules';
15-
import type { PatternMatcher, PatternMatchRule } from '../pattern-matcher/wasm-matcher';
10+
import { ALL_QUERIES } from '../pattern-matcher/rules';
11+
import type { PatternMatcher } from '../pattern-matcher/wasm-matcher';
1612
import { resolveLanguage } from '../pattern-matcher/wasm-matcher';
1713
import { scanRepository } from '../scanner';
1814
import type { Document } from '../scanner/types';
@@ -110,34 +106,34 @@ export function extractTypeCoverageFromSignatures(signatures: string[]): TypeAnn
110106
}
111107

112108
// ========================================================================
113-
// AST-Enhanced Extractors — use PatternMatcher when available, regex fallback
109+
// AST-Enhanced Extractors — accept pre-computed AST results, regex fallback
114110
// ========================================================================
115111

116112
/**
117-
* Run AST queries if matcher and language are available, else return empty map.
113+
* Run all AST queries in a single parse. Call once per file, pass results
114+
* to each extractor. Avoids parsing the same source 3 times.
115+
*
116+
* Returns empty map when matcher or filePath is unavailable (regex fallback).
118117
*/
119-
async function runAstQueries(
118+
export async function runAllAstQueries(
120119
content: string,
121120
filePath: string | undefined,
122-
matcher: PatternMatcher | undefined,
123-
queries: PatternMatchRule[]
121+
matcher: PatternMatcher | undefined
124122
): Promise<Map<string, number>> {
125123
if (!matcher || !filePath) return new Map();
126124
const language = resolveLanguage(filePath);
127125
if (!language) return new Map();
128-
return matcher.match(content, language, queries);
126+
return matcher.match(content, language, ALL_QUERIES);
129127
}
130128

131129
/**
132-
* Extract error handling using AST (preferred) + regex (fallback/supplement).
130+
* Extract error handling using pre-computed AST results + regex fallback.
133131
*/
134-
export async function extractErrorHandlingWithAst(
132+
export function extractErrorHandlingWithAst(
135133
content: string,
136-
filePath?: string,
137-
matcher?: PatternMatcher
138-
): Promise<ErrorHandlingPattern> {
134+
ast: Map<string, number>
135+
): ErrorHandlingPattern {
139136
const regex = extractErrorHandlingFromContent(content);
140-
const ast = await runAstQueries(content, filePath, matcher, ERROR_HANDLING_QUERIES);
141137

142138
if (ast.size === 0) return regex;
143139

@@ -158,15 +154,13 @@ export async function extractErrorHandlingWithAst(
158154
}
159155

160156
/**
161-
* Extract import style using AST (preferred) + regex (fallback/supplement).
157+
* Extract import style using pre-computed AST results + regex fallback.
162158
*/
163-
export async function extractImportStyleWithAst(
159+
export function extractImportStyleWithAst(
164160
content: string,
165-
filePath?: string,
166-
matcher?: PatternMatcher
167-
): Promise<ImportStylePattern> {
161+
ast: Map<string, number>
162+
): ImportStylePattern {
168163
const regex = extractImportStyleFromContent(content);
169-
const ast = await runAstQueries(content, filePath, matcher, IMPORT_STYLE_QUERIES);
170164

171165
if (ast.size === 0) return regex;
172166

@@ -192,17 +186,14 @@ export async function extractImportStyleWithAst(
192186
}
193187

194188
/**
195-
* Extract type coverage using AST (preferred) + regex signatures (supplement).
189+
* Extract type coverage using pre-computed AST results + regex signatures.
196190
*/
197-
export async function extractTypeCoverageWithAst(
191+
export function extractTypeCoverageWithAst(
198192
content: string,
199-
filePath?: string,
200-
matcher?: PatternMatcher,
193+
ast: Map<string, number>,
201194
signatures?: string[]
202-
): Promise<TypeAnnotationPattern> {
203-
// Start with signature-based detection (from index or ts-morph)
195+
): TypeAnnotationPattern {
204196
const regex = extractTypeCoverageFromSignatures(signatures ?? []);
205-
const ast = await runAstQueries(content, filePath, matcher, TYPE_COVERAGE_QUERIES);
206197

207198
if (ast.size === 0) return regex;
208199

@@ -287,18 +278,15 @@ export class PatternAnalysisService {
287278
.map((d) => (d.metadata.signature as string) || '')
288279
.filter(Boolean);
289280

290-
const [importStyle, errorHandling, typeAnnotations] = await Promise.all([
291-
extractImportStyleWithAst(content, filePath, this.config.patternMatcher),
292-
extractErrorHandlingWithAst(content, filePath, this.config.patternMatcher),
293-
extractTypeCoverageWithAst(content, filePath, this.config.patternMatcher, signatures),
294-
]);
281+
// Parse once, run all 12 AST queries, pass results to each extractor
282+
const ast = await runAllAstQueries(content, filePath, this.config.patternMatcher);
295283

296284
return {
297285
fileSize: { lines, bytes },
298286
testing,
299-
importStyle,
300-
errorHandling,
301-
typeAnnotations,
287+
importStyle: extractImportStyleWithAst(content, ast),
288+
errorHandling: extractErrorHandlingWithAst(content, ast),
289+
typeAnnotations: extractTypeCoverageWithAst(content, ast, signatures),
302290
};
303291
}
304292

@@ -389,18 +377,15 @@ export class PatternAnalysisService {
389377
.map((d) => d.metadata.signature || '')
390378
.filter(Boolean);
391379

392-
const [importStyle, errorHandling, typeAnnotations] = await Promise.all([
393-
extractImportStyleWithAst(content, filePath, this.config.patternMatcher),
394-
extractErrorHandlingWithAst(content, filePath, this.config.patternMatcher),
395-
extractTypeCoverageWithAst(content, filePath, this.config.patternMatcher, signatures),
396-
]);
380+
// Parse once, run all 12 AST queries, pass results to each extractor
381+
const ast = await runAllAstQueries(content, filePath, this.config.patternMatcher);
397382

398383
return {
399384
fileSize: { lines: content.split('\n').length, bytes: stat.size },
400385
testing,
401-
importStyle,
402-
errorHandling,
403-
typeAnnotations,
386+
importStyle: extractImportStyleWithAst(content, ast),
387+
errorHandling: extractErrorHandlingWithAst(content, ast),
388+
typeAnnotations: extractTypeCoverageWithAst(content, ast, signatures),
404389
};
405390
}
406391

0 commit comments

Comments
 (0)