From e51a0732c75ca8650ebef16bb831771b756eae20 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Wed, 14 Oct 2026 03:55:52 -0500 Subject: [PATCH] fix(explore): tests and examples the engine knows stay out of explore's answer codegraph_explore's low-value check (the hard filter, the rank penalty and the comparator key) and the dominant-file, route-file and routing-manifest filter in db/queries.ts each kept a regex list of their own. They missed every test convention isTestFile learned since: C++ `foo_test.cc` and `FooTest.cpp`, Google's `foo_unittest.cc`, LLVM's `unittests/`, `testing/`, `e2e/`, `__mocks__/`, Gradle `foo-test/` modules, Kotlin source sets, and the examples, samples, fixtures and benchmarks that explore's gather already dampens through isTestFile. Both now build on isTestFile, keeping icons/i18n (explore) and generated files (queries), plus the singular `__test__/` the old lists matched. Dropping their lowercase suffix patterns also ends false positives such as `Latest.java` and `contest.cs`. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/explore-low-value-tests.test.ts | 159 ++++++++++++++++++++++ src/db/queries.ts | 25 +--- src/mcp/tools.ts | 25 ++-- 4 files changed, 176 insertions(+), 34 deletions(-) create mode 100644 __tests__/explore-low-value-tests.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c3b4b7947..141cf103db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph sync` and the file watcher now link a route to the page or layout it loads lazily when that file is added after the router was indexed, comes back after being deleted, or gets the component the route renders in a later edit, whether or not the import names the file's extension: React Router's `lazy: () => import('./pages/Team')`, Vue Router's `component: () => import('@/views/Login')` and Angular's `loadComponent: () => import('./home/home.component')`. Before, the route stayed unlinked until the router file changed or the project was re-indexed, so the page looked unused and `codegraph affected` missed the tests that reach it through the router. Re-index after upgrading to pick up links an earlier version missed. - A file named the way Google names its tests, like `wire_format_unittest.cc` in protocolbuffers/protobuf, `logging_unittest.cc` in google/glog or a Python `foo_unittest.py`, now counts as a test, as `wire_format_test.cc` already did. Before, it was taken for production code: search and `codegraph_explore` ranked it alongside the code it tests, `codegraph affected` never listed it, and calls in production code could link to a class or type a unittest declares for itself, as google/breakpad's `stack_frame_entries_.size()` did to a unittest's `StackHelper`. Re-index projects that have such files after upgrading. - A file in a `unittests/` folder, where llvm/llvm-project and google/breakpad keep their unit tests, or in a folder named like a unittest, such as `dcheck_unittest/` in google/glog, now counts as a test whatever its own name says, like LLVM's `CountCopyAndMove.h` helper. Before, such a file was taken for production code unless its name said test, and none of them were kept apart from the program the way a `tests/` folder is: search and `codegraph_explore` ranked them alongside production code, `codegraph affected` missed them, and production code linked to a unit test's own declarations, as LLVM's `Ctx` and `OpInfo` variables did to ones its unit tests declare. A folder named just `unittest/`, like CPython's `Lib/unittest/`, is still production code. Re-index projects that have such folders after upgrading. +- `codegraph_explore` now leaves out of its answers every file that search already treats as a test or an example: a C++ `FooTest.cpp` or `foo_test.cc`, a Google `foo_unittest.cc`, anything in an LLVM-style `unittests/` folder or an `e2e/` or `testing/` folder, and a project's `examples/`, `samples/` and `benchmarks/`. Before, explore went by a shorter list of its own, so a unit test or an example that called the code you asked about could take a place in the answer meant for that code: in protocolbuffers/protobuf, a question about `DescriptorPool` came back with three of its unit tests. The same files are no longer taken for a project's main routes file either, such as an end-to-end suite's mock server. No re-index is needed. - `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now do the same for a Python import under a name of its own, like `from .models import User as UserModel`, `from . import helpers as h` or `from app.pkg import sub as subpkg`: when the module or package is added after the importing file was indexed, comes back after being deleted, or gets the imported name in a later edit, the import and every use of the alias, such as a call, a base class or the class passed as a value, now link to it. Before, they stayed unlinked until the importing file changed or the project was re-indexed. Re-index Python projects after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now also link a Python import of a name a package re-exports, like `from fastapi import FastAPI` through `fastapi/__init__.py`, when that `__init__.py` is added after the importing files were indexed or gains the re-export in a later edit. Before, calls through the name stayed unlinked until each importing file changed or the project was re-indexed. Large projects gain the most: sync also used to skip, in any language, a module or package that more than 500 references were waiting for, and now restoring Django's `django/urls/__init__.py` relinks every file that imports `path` from it. Re-index Python projects after upgrading to pick up links an earlier version missed. diff --git a/__tests__/explore-low-value-tests.test.ts b/__tests__/explore-low-value-tests.test.ts new file mode 100644 index 0000000000..d2b298c07e --- /dev/null +++ b/__tests__/explore-low-value-tests.test.ts @@ -0,0 +1,159 @@ +/** + * codegraph_explore drops test files from its answer, and the dominant-file + * and route-file detectors skip them — but each kept a list of test names of + * its own, so they missed every convention the engine's `isTestFile` learned + * later: LLVM's `unittests/` trees (a helper there has no test-like name), + * Google's `foo_unittest.cc`, a C++ `FooTest.cpp`, `e2e/` folders and the + * examples around a project. On an LLVM-style repo a unit test's harness, named + * like the code under test, took a file slot in the answer. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { ToolHandler } from '../src/mcp/tools'; + +const write = (root: string, files: Record) => { + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } +}; + +const renderedFiles = (text: string): string[] => [...text.matchAll(/^\*\*`([^`]+)`\*\* —/gm)].map((m) => m[1]!); + +describe('codegraph_explore leaves out unit tests the engine knows', () => { + let root = ''; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-explore-low-value-')); + write(root, { + 'src/lexer.h': `#pragma once + +class Lexer { + public: + int nextToken(); +}; +`, + 'src/lexer.cc': `#include "lexer.h" + +int Lexer::nextToken() { return 1; } +`, + 'src/parser.h': `#pragma once + +class Parser { + public: + int parseExpression(const char* text); + int parseStatement(const char* text); +}; +`, + 'src/parser.cc': `#include "parser.h" +#include "lexer.h" + +int Parser::parseExpression(const char* text) { + Lexer lexer; + return lexer.nextToken() + (text ? 1 : 0); +} + +int Parser::parseStatement(const char* text) { + return parseExpression(text) + 1; +} +`, + // Google's naming: the unittest sits beside the code it tests. + 'src/parser_unittest.cc': `#include "parser.h" + +void ParsesExpressions() { + Parser parser; + parser.parseExpression("1 + 2"); + parser.parseStatement("x = 1;"); +} +`, + // LLVM's: a unittests/ tree, with a harness named like what it tests. + 'unittests/Parse/ParserHarness.h': `#pragma once + +#include "../../src/parser.h" + +class ParserHarness { + public: + int parseExpression(const char* text) { return parser.parseExpression(text); } + int parseStatement(const char* text) { return parser.parseStatement(text); } + + private: + Parser parser; +}; +`, + 'unittests/Parse/ParserTest.cpp': `#include "ParserHarness.h" + +int ParsesStatements() { + ParserHarness harness; + return harness.parseStatement("y = 2;") + harness.parseExpression("3"); +} +`, + }); + cg = await CodeGraph.init(root, { index: true }); + }, 60_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + it('answers from the production files, not the unit tests that call them', async () => { + const handler = new ToolHandler(cg); + const result = await handler.execute('codegraph_explore', { query: 'Parser parseExpression parseStatement Lexer nextToken' }); + const files = renderedFiles(result.content?.[0]?.text ?? ''); + expect(files).toContain('src/parser.cc'); + expect(files.filter((f) => /unittests\/|_unittest\.cc$|Test\.cpp$/.test(f))).toEqual([]); + }); +}); + +describe('the route-file detector skips an e2e mock server', () => { + let root = ''; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-route-low-value-')); + write(root, { + 'package.json': JSON.stringify({ name: 'shop', dependencies: { express: '^4.19.0' } }), + 'src/routes.js': `const express = require('express'); +const router = express.Router(); + +function listOrders(req, res) { res.json([]); } +function showOrder(req, res) { res.json({}); } +function createOrder(req, res) { res.status(201).end(); } + +router.get('/orders', listOrders); +router.get('/orders/:id', showOrder); +router.post('/orders', createOrder); + +module.exports = router; +`, + // A stand-in payment provider the end-to-end suite starts. + 'e2e/mock-server.js': `const express = require('express'); +const app = express(); + +function ok(req, res) { res.json({ ok: true }); } + +app.get('/v1/charges', ok); +app.post('/v1/charges', ok); +app.get('/v1/refunds', ok); +app.post('/v1/refunds', ok); +app.get('/v1/balance', ok); + +module.exports = app; +`, + }); + cg = await CodeGraph.init(root, { index: true }); + }, 60_000); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + it("names the app's own routes file", () => { + expect(cg.getTopRouteFile()?.filePath).toBe('src/routes.js'); + }); +}); diff --git a/src/db/queries.ts b/src/db/queries.ts index f6b33befdd..009aef8c6c 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -18,15 +18,15 @@ import { SearchResult, } from '../types'; import { safeJsonParse } from '../utils'; -import { kindBonus, nameMatchBonus, scorePathRelevance } from '../search/query-utils'; +import { isTestFile, kindBonus, nameMatchBonus, scorePathRelevance } from '../search/query-utils'; import { parseQuery, boundedEditDistance } from '../search/query-parser'; import { isGeneratedFile } from '../extraction/generated-detection'; import { splitIdentifierSegments } from '../search/identifier-segments'; import { isModuleKey, referenceNameTail } from './reference-tail'; /** - * Files that should not be candidates for "dominant file" detection: test/spec - * files and tool-generated files. Generated files (`*.pb.go`, `*.pulsar.go`, + * Files that should not be candidates for "dominant file" detection: tests, + * examples and tool-generated files. Generated files (`*.pb.go`, `*.pulsar.go`, * mock outputs, …) often have huge in-file edge counts that dwarf the real * source — etcd's `rpc.pb.go` has 4× the in-file edges of `server.go`. * @@ -37,21 +37,10 @@ import { isModuleKey, referenceNameTail } from './reference-tail'; */ function isLowValueFile(filePath: string, generated?: ReadonlySet): boolean { if (generated?.has(filePath)) return true; - const lp = filePath.toLowerCase(); - return ( - /(?:^|\/)(tests?|__tests?__|spec)\//.test(lp) || - /_test\.go$/.test(lp) || - /(?:^|\/)test_[^/]+\.py$/.test(lp) || - /_test\.py$/.test(lp) || - /_spec\.rb$/.test(lp) || - /_test\.rb$/.test(lp) || - /\.(test|spec)\.[jt]sx?$/.test(lp) || - /(test|spec|tests)\.(java|kt|scala)$/.test(lp) || - /(tests?|spec)\.cs$/.test(lp) || - /tests?\.swift$/.test(lp) || - /_test\.dart$/.test(lp) || - isGeneratedFile(filePath) - ); + // Tests, examples, fixtures and benchmarks by the engine's own reading — + // LLVM's `unittests/`, Google's `foo_unittest.cc`, a C++ `FooTest.cpp` — + // plus the singular `__test__/` some projects use. + return isTestFile(filePath) || /(?:^|\/)__test__\//i.test(filePath) || isGeneratedFile(filePath); } const SQLITE_PARAM_CHUNK_SIZE = 500; diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 8aebac9588..e33cfb8181 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -4659,25 +4659,18 @@ export class ToolHandler { // Test/spec/icon/i18n file detector — used by the pre-floor hard filter, the // rank penalty, and the comparator deprioritization. // - // The directory pattern is anchored at `^` as well as `/`: a repo-ROOT - // `test/` or `spec/` directory (express, cobra, and most of npm/Go) produced - // paths like `test/express.raw.js`, which the old leading-`/` form could - // never match — so express's routing question spent 59% of its envelope on - // three test files while `lib/router/index.js` never rendered. + // Tests are the engine's own reading (`isTestFile`), the one the gather + // above already dampened them by: every ecosystem's test names and folders — + // a repo-ROOT `test/` (express's routing question once spent 59% of its + // envelope on three test files), LLVM's `unittests/`, Google's + // `foo_unittest.cc`, a C++ `FooTest.cpp`, `testing/`, `e2e/` — and the + // examples, samples, fixtures and benchmarks around a project. A list of + // its own here missed every one of those it hadn't been taught. const isLowValue = (p: string) => { const lp = p.toLowerCase(); return ( - /(?:^|\/)(tests?|__tests?__|specs?)\//.test(lp) || - /_test\.go$/.test(lp) || - /(?:^|\/)test_[^/]+\.py$/.test(lp) || - /_test\.py$/.test(lp) || - /_spec\.rb$/.test(lp) || - /_test\.rb$/.test(lp) || - /\.(test|spec)\.[jt]sx?$/.test(lp) || - /(test|spec|tests)\.(java|kt|scala)$/.test(lp) || - /(tests?|spec)\.cs$/.test(lp) || - /tests?\.swift$/.test(lp) || - /_test\.dart$/.test(lp) || + isTestFile(p) || + /(?:^|\/)__test__\//.test(lp) || /\bicons?\b/.test(lp) || /\bi18n\b/.test(lp) );