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
44 changes: 44 additions & 0 deletions packages/asil-improvement-loop/src/__tests__/scanner.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, expect, it } from 'vitest';
import {
normalizePath,
isTestFile,
scanCodebase,
scanTestFailures,
scanTypeErrors,
Expand Down Expand Up @@ -172,6 +173,49 @@ describe('scanner', () => {
const foo = tasks.find((t) => t.filePaths[0]?.includes('foo.ts'));
expect(foo?.severity).toBe('medium');
});

it('skips markers found in test files — they are fixtures, not tasks (precision fix)', async () => {
const grepOut = [
'./packages/foo/src/__tests__/foo.test.ts:1:// DOMAIN_QUESTION: fixture data',
'./packages/foo/src/scanner.spec.ts:2:// TODO: spec fixture',
'./packages/foo/src/real.ts:9:// TODO: a genuine task',
].join('\n');
const runner = mockRunner([{ match: () => true, stdout: grepOut }]);
const tasks = await scanTodos('/repo', { runner, fs: mockFileReader() });
// Only the non-test file yields a task.
expect(tasks.length).toBe(1);
expect(tasks[0]?.filePaths[0]).toBe('packages/foo/src/real.ts');
});

it('greps with a comment-anchored marker pattern (no bare string-literal matches)', async () => {
const seen: string[][] = [];
const runner = mockRunner([
{
match: (cmd, args) => {
if (cmd === 'grep') seen.push(args);
return cmd === 'grep';
},
stdout: '',
},
]);
await scanTodos('/repo', { runner, fs: mockFileReader() });
const pattern = seen[0]?.find((a) => a.includes('DOMAIN_QUESTION')) ?? '';
// The pattern must require a line-start comment opener before the
// marker, so `'DOMAIN_QUESTION'` / `/DOMAIN_QUESTION:/` don't match.
expect(pattern).toContain('(//|\\*|#)');
expect(pattern.startsWith('^')).toBe(true);
});
});

describe('isTestFile', () => {
it('matches __tests__ dirs and .test/.spec files; not plain source', () => {
expect(isTestFile('packages/x/src/__tests__/a.ts')).toBe(true);
expect(isTestFile('src/a.test.ts')).toBe(true);
expect(isTestFile('src/a.spec.tsx')).toBe(true);
expect(isTestFile('a.test.py')).toBe(false); // py uses test_ prefix, not .test.
expect(isTestFile('src/a.ts')).toBe(false);
expect(isTestFile('src/scanner.ts')).toBe(false);
});
});

describe('scanCoverageGaps', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/asil-improvement-loop/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export {
scanCoverageGaps,
scanDeadCode,
normalizePath,
isTestFile,
stableTaskId,
toRepoRelative,
type ScanResult,
Expand Down
27 changes: 26 additions & 1 deletion packages/asil-improvement-loop/src/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,15 @@ export async function scanTodos(
...GREP_EXCLUDE_DIRS,
...includes,
'-E',
'(TODO|FIXME|HACK|DOMAIN_QUESTION)[: ]',
// Require the marker to be the FIRST token of a line-start comment
// (`//`, `#`, or jsdoc `*`). This is ASIL's own DOMAIN_QUESTION
// convention and it eliminates the false positives a live grind
// surfaced: marker strings inside string literals / regexes
// (e.g. `'DOMAIN_QUESTION'`, `/DOMAIN_QUESTION:/`) and mid-comment
// mentions are no longer matched — only genuine actionable markers.
// Trade-off: trailing comments (`code; // TODO`) aren't matched,
// consistent with how DOMAIN_QUESTION markers are already detected.
'^[ \\t]*(//|\\*|#)[ \\t]*(TODO|FIXME|HACK|DOMAIN_QUESTION)[: ]',
'.',
],
{ cwd: repoRoot },
Expand All @@ -173,6 +181,10 @@ export async function scanTodos(
const [, file, ln, txt] = m;
if (!file) continue;
const key = normalizePath(file);
// Skip test files — a TODO/FIXME/DOMAIN_QUESTION marker in a test is
// almost always test DATA (fixtures exercising the scanner itself),
// not an actionable task. (Live-grind precision finding.)
if (isTestFile(key)) continue;
const arr = byFile.get(key) ?? [];
arr.push(`:${ln} — ${txt?.trim() ?? ''}`);
byFile.set(key, arr);
Expand Down Expand Up @@ -341,6 +353,19 @@ function shortPath(p: string): string {
return p.replace(/^.*\//, '');
}

/**
* True for files that are tests: anything under a `__tests__/` directory
* or named `*.test.*` / `*.spec.*`. Used to keep TODO/FIXME markers that
* are test FIXTURES (data exercising the scanner) out of the task queue.
*/
export function isTestFile(p: string): boolean {
return (
p.includes('/__tests__/') ||
p.startsWith('__tests__/') ||
/\.(test|spec)\.[cm]?[jt]sx?$/.test(p)
);
}

/**
* Convert an absolute path under `repoRoot` to a repo-relative path.
* Coverage tools (vitest's istanbul reporter, coverage.py) emit
Expand Down
Loading