diff --git a/gitnexus/src/core/group/extractors/http-patterns/python.ts b/gitnexus/src/core/group/extractors/http-patterns/python.ts index 7d8bd99af..7a0fa2f7f 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/python.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/python.ts @@ -1137,13 +1137,17 @@ export const PYTHON_HTTP_PLUGIN: HttpLanguagePlugin = { name: 'python-http', language: Python, // routeCoverage intentionally LEFT at the default 'partial' (#2138 Part 2). - // It would be a no-op even if set to 'complete': FastAPI decorator routes set - // no handlerName (generic worker path) and Django sets methodName: null, so no - // Python file ever resolves a handlerSymbolId and none would be parse-skipped. - // Declaring 'complete' now is only a latent trap for the moment a follow-up - // gives FastAPI routes a handlerName. `hasConsumerSignals` is kept (and is a - // true superset of scan()'s consumer shapes) so the precondition already holds - // when Python is later flipped to 'complete'. + // 'complete' is now an active data-loss risk rather than a no-op: FastAPI and + // Flask decorator routes do carry a handlerName (Python's + // `decoratorRouteHandlerName` hook reads the `decorated_definition`), so their + // files can resolve every handlerSymbolId and become parse-skip candidates. + // The flag asserts more than that — it asserts ingestion emits a Route node + // for EVERY provider route this scan() finds, and it does not: Flask's + // imperative `add_url_rule('/p', view_func=handler)` registration below has no + // ingestion counterpart, so skipping a file that mixes it with resolved + // decorator routes would drop those providers. `hasConsumerSignals` is kept + // (and is a true superset of scan()'s consumer shapes) so the consumer half of + // the precondition already holds once provider parity is closed. // Consumer signals scan() can detect: `requests.`/`requests.request`, // `httpx` (sync/async client), the `uri=`/`url=` keyword/variable wrapper // calls, plus aiohttp/urllib. Conservative — over-matching only costs a parse. diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index 6aa4c8476..5def6a9a9 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -621,6 +621,7 @@ export class HttpRouteExtractor implements ContractExtractor { dbExecutor, getDetections, resolveDetectionSymbol, + loadFileSymbols, coveredFiles, ) : []; @@ -690,6 +691,7 @@ export class HttpRouteExtractor implements ContractExtractor { db: CypherExecutor, getDetections: (rel: string) => Promise, resolveSymbol: (filePath: string, d: HttpDetection) => Promise, + loadFileSymbols: (filePath: string) => Promise[]>, coveredFiles?: Set, ): Promise { const out: ExtractedContract[] = []; @@ -749,15 +751,11 @@ export class HttpRouteExtractor implements ContractExtractor { if (!method) method = 'GET'; symbolUid = handlerSymbolId; if (filePath) { - try { - const syms = await db(CONTAINING_QUERY, { filePath }); - const hit = syms.find((s) => String(s.uid ?? s[0]) === handlerSymbolId); - if (hit) { - symbolName = String(hit.name ?? hit[1]) || symbolName; - symPath = String(hit.filePath ?? hit[2]) || filePath; - } - } catch { - /* keep the authoritative uid + basename fallback */ + const syms = await loadFileSymbols(filePath); + const hit = syms.find((s) => String(s.uid ?? s[0]) === handlerSymbolId); + if (hit) { + symbolName = String(hit.name ?? hit[1]) || symbolName; + symPath = String(hit.filePath ?? hit[2]) || filePath; } } } else { diff --git a/gitnexus/src/core/ingestion/language-provider.ts b/gitnexus/src/core/ingestion/language-provider.ts index 3ca60f372..ae992b12a 100644 --- a/gitnexus/src/core/ingestion/language-provider.ts +++ b/gitnexus/src/core/ingestion/language-provider.ts @@ -413,6 +413,28 @@ interface LanguageProviderConfig { lineOffset: number, ) => ExtractedDecoratorRoute[]; + /** + * Name of the function a route decorator captured by the worker's generic + * `@decorator` query applies to, given the decorator's own AST node. + * + * The worker knows a decorator is a route decorator but not how this + * language's grammar attaches it to a definition, so it hands the node over + * unchanged and takes whatever the language returns. Only languages that + * declare route handlers through the generic decorator captures need this; + * languages with a dedicated {@link extractDecoratorRoutes} extractor + * (JS/TS via `nest.ts`, Java via `spring.ts`) already set + * `ExtractedDecoratorRoute.handlerName` there and should leave this undefined. + * + * Implementations must read their own decorated-definition shape directly and + * return undefined for anything else — never climb ancestors to find a name, + * since a decorator that is not attached to a function has no handler and a + * borrowed enclosing name resolves `handlerSymbolId` to the wrong symbol. The + * routes phase treats undefined as "fall back to the file-level edge". + * + * Default: undefined (no handler name from generic decorator captures). + */ + readonly decoratorRouteHandlerName?: (decoratorNode: SyntaxNode) => string | undefined; + /** * Collect a project-wide, language-agnostic view of route-defining * class/interface declarations (`SharedSpringType`) from a parsed file. diff --git a/gitnexus/src/core/ingestion/languages/python.ts b/gitnexus/src/core/ingestion/languages/python.ts index ca40a4566..ce8d0fa90 100644 --- a/gitnexus/src/core/ingestion/languages/python.ts +++ b/gitnexus/src/core/ingestion/languages/python.ts @@ -45,6 +45,7 @@ import { import { extractDjangoRoutes } from '../route-extractors/django.js'; import { discoverDjangoRootUrls } from '../route-extractors/django-root-discovery.js'; import { extractPythonModuleConstants } from '../route-extractors/python-const-resolver.js'; +import { pythonDecoratorRouteHandlerName } from '../route-extractors/python-decorator-handler.js'; const BUILT_INS: ReadonlySet = new Set([ 'print', @@ -143,6 +144,7 @@ export const pythonProvider = defineLanguage({ discoverDjangoRootUrls(files, contentMap, reader), extractRoutes: (tree, filePath, reader, parser) => parser ? extractDjangoRoutes(tree, filePath, parser, reader) : [], + decoratorRouteHandlerName: pythonDecoratorRouteHandlerName, labelOverride: pythonFunctionDefinitionLabel, // ── RFC #909 Ring 3: scope-based resolution hooks (RFC §5) ────────── diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index e93fa033c..42bf639d2 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -336,32 +336,31 @@ export const routesPhase: PipelinePhase = { let handlerContents: Map | undefined; if (routeRegistry.size > 0) { - const handlerPathFor = (routeKey: string, entry: RouteEntry): string => { - if (entry.source !== DATA_ROUTE_TABLE_SOURCE) return entry.filePath; - const handlerSymbolId = routeHandlerSymbols.get(routeKey); - const resolvedPath = handlerSymbolId - ? ctx.graph.getNode(handlerSymbolId)?.properties.filePath - : undefined; - return typeof resolvedPath === 'string' ? resolvedPath : entry.filePath; - }; - const handlerPaths = [...routeRegistry].map(([key, entry]) => handlerPathFor(key, entry)); - handlerContents = await readFileContents(ctx.repoPath, handlerPaths); + // Resolve once so content attribution, the route stamp, and the edge use + // the same live graph node. Pre-seeded routes never own handler symbols. + const routes = [...routeRegistry].map(([routeKey, entry]) => { + const id = preSeededKeys.has(routeKey) ? undefined : routeHandlerSymbols.get(routeKey); + const node = id ? ctx.graph.getNode(id) : undefined; + const handlerSymbol = id && node ? { id, node } : undefined; + const resolvedPath = + entry.source === DATA_ROUTE_TABLE_SOURCE + ? handlerSymbol?.node.properties.filePath + : undefined; + const handlerPath = typeof resolvedPath === 'string' ? resolvedPath : entry.filePath; + return { routeKey, entry, handlerSymbol, handlerPath }; + }); + handlerContents = await readFileContents( + ctx.repoPath, + routes.map(({ handlerPath }) => handlerPath), + ); - for (const [routeKey, entry] of routeRegistry) { + for (const { routeKey, entry, handlerSymbol, handlerPath } of routes) { const { source: routeSource, method: routeMethod, url } = entry; - const handlerPath = handlerPathFor(routeKey, entry); const content = handlerContents.get(handlerPath); - // A pre-seeded route can never legitimately appear in - // `routeHandlerSymbols`, so a key that does is a route that LOST (#3049). - const handlerSymbolId = preSeededKeys.has(routeKey) - ? undefined - : routeHandlerSymbols.get(routeKey); + const handlerSymbolId = handlerSymbol?.id; const analysisContent = entry.source === DATA_ROUTE_TABLE_SOURCE && content - ? handlerSymbolContent( - content, - handlerSymbolId ? ctx.graph.getNode(handlerSymbolId) : undefined, - ) + ? handlerSymbolContent(content, handlerSymbol?.node) : content; const { responseKeys, errorKeys } = analysisContent @@ -397,6 +396,19 @@ export const routesPhase: PipelinePhase = { confidence: 1.0, reason: routeSource, }); + + // Keep the file edge for existing extractor queries; add the live + // definition edge for explicit handler-level traversal. + if (handlerSymbolId) { + ctx.graph.addRelationship({ + id: generateId('HANDLES_ROUTE', `${handlerSymbolId}->${routeNodeId}`), + sourceId: handlerSymbolId, + targetId: routeNodeId, + type: 'HANDLES_ROUTE', + confidence: 1.0, + reason: routeSource, + }); + } } if (isDev) { diff --git a/gitnexus/src/core/ingestion/route-extractors/python-decorator-handler.ts b/gitnexus/src/core/ingestion/route-extractors/python-decorator-handler.ts new file mode 100644 index 000000000..fca06c857 --- /dev/null +++ b/gitnexus/src/core/ingestion/route-extractors/python-decorator-handler.ts @@ -0,0 +1,19 @@ +/** + * Return the function name attached to a Python decorator's immediate + * `decorated_definition`; reject every other shape rather than climbing. + */ + +import type { SyntaxNode } from '../utils/ast-helpers.js'; + +export function pythonDecoratorRouteHandlerName(decoratorNode: SyntaxNode): string | undefined { + const decorated = decoratorNode.parent; + if (decorated === null || decorated.type !== 'decorated_definition') return undefined; + + // `async def` is still a `function_definition` in tree-sitter-python (the + // `async` keyword is an anonymous child), so async handlers need no branch. + const definition = decorated.childForFieldName('definition'); + if (!definition || definition.type !== 'function_definition') return undefined; + + const name = definition.childForFieldName('name')?.text; + return name !== undefined && name.length > 0 ? name : undefined; +} diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 7c46c6011..071b31784 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -1795,12 +1795,14 @@ const processFileGroup = ( const httpMethod = ['GET', 'POST', 'PUT', 'DELETE', 'PATCH'].includes(method) ? method : 'GET'; + const handlerName = provider.decoratorRouteHandlerName?.(decoratorNode); const base = { filePath: file.path, httpMethod, decoratorName, lineNumber: decoratorNode.startPosition.row + lineOffset, ...(decoratorReceiver ? { decoratorReceiver } : {}), + ...(handlerName ? { handlerName } : {}), }; if (decoratorArgStr) { // String-literal path (the fast path, unchanged). Empty-string diff --git a/gitnexus/src/core/lbug/schema.ts b/gitnexus/src/core/lbug/schema.ts index cc0cc78fd..3c6ec0131 100644 --- a/gitnexus/src/core/lbug/schema.ts +++ b/gitnexus/src/core/lbug/schema.ts @@ -371,7 +371,7 @@ const DEFINITION_ANCHOR_LABELS: readonly NodeTableName[] = NODE_TABLES.filter( * | `Annotation` | `frameworks/spring/conditionals.ts` CONDITIONAL_ON | `resolveDefGraphId` / `resolveCallerGraphId` | * | `Community` | `pipeline-phases/communities.ts` MEMBER_OF | Leiden membership, `isCommunitySymbol`-gated | * | `Process` | `pipeline-phases/processes.ts` STEP_IN_PROCESS | trace step node | - * | `Route` | `pipeline-phases/routes.ts` HANDLES_ROUTE | `generateId('File', handlerPath)` — a literal | + * | `Route` | `pipeline-phases/routes.ts` HANDLES_ROUTE | `generateId('File', handlerPath)` — a literal — plus, when the route's handler resolves, that definition | * | `Tool` | `pipeline-phases/tools.ts` HANDLES_TOOL | `handlerNodeId` — whatever definition the decorator sat on | * | `File` | `languages/vue/scope-resolver.ts` BINDS_EVENT_HANDLER | handler node | * | `Record` | `cobol-processor.ts` × 8 external-resource sites | `scopedCallerLookup` | @@ -383,25 +383,35 @@ const DEFINITION_ANCHOR_LABELS: readonly NodeTableName[] = NODE_TABLES.filter( * {@link DEFINITION_ANCHOR_LABELS} × this set covers all four plus every * sibling the same emitters can reach. * - * TWO TARGETS ARE LABEL-GATED TODAY, and the cross product over-declares for - * them ON PURPOSE (~47 of the 182 attachment pairs are unreachable right now): + * TWO TARGETS USE ONLY PART OF THEIR CROSS PRODUCT TODAY, and it over-declares + * for them ON PURPOSE (~45 of the 182 attachment pairs have no emitter that can + * reach them right now): * - `Community` — `isCommunitySymbol` (`community-processor.ts`) admits only * `Function` / `Class` / `Method` / `Interface` as members, so the other 22 * anchors cannot source a MEMBER_OF edge until that predicate widens. - * - `Route` — HANDLES_ROUTE sources `generateId('File', handlerPath)`, a - * literal `File`, so every non-`File` anchor is headroom. + * - `Route` — HANDLES_ROUTE always sources the literal + * `generateId('File', handlerPath)`, and additionally the route's HANDLER + * DEFINITION whenever `routeHandlerSymbols` produced an id that + * `ctx.graph.getNode` confirms is in the graph. That second anchor is a + * lookup result the emitter never label-checks; handler resolution returns + * `Function` / `Method` definitions today, so `Route` is no longer a + * `File`-only target and the remaining anchors are headroom. * * Those pairs stay declared because the two sides of the error are not * symmetric: an UNDECLARED pair makes LadybugDB reject the edge and aborts * `analyze` outright on a user's repo, while an unused DECLARED pair costs * almost nothing — `bench/schema-pairs` measured the pre-#2801 332→450 growth - * (118 pairs, of which these ~47 are a part) at 0.93–1.05×, i.e. inside + * (118 pairs, of which these ~45 are a part) at 0.93–1.05×, i.e. inside * run-to-run noise. #2801's 11 generated `Record` pairs bring the total to 461. * Its Windows measurements and noise caveats live in the benchmark README; the * operational production ceiling is the checked 1.5× budget. * Every one of the four aborts above came from re-narrowing a set to what one * predicate looked like it allowed — so a reading of `isCommunitySymbol` is not - * grounds to shrink this. Widening either predicate is then a no-op here. + * grounds to shrink this. `Route` is the worked example in the other direction: + * the definition-level HANDLES_ROUTE edge started emitting `Function|Route` / + * `Method|Route` with no DDL change, because those pairs were already declared. + * Widening `isCommunitySymbol`, or handler resolution returning a further + * label, is likewise a no-op here. * * `Route` / `Tool` being excluded as ANCHORS (see {@link NON_DEFINITION_LABELS}) * is likewise a SIZE choice, not something derived from a rule: they do source diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 1dcd4701f..bd3b961ec 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -705,7 +705,18 @@ import { copyV8CacheIfPresent, tryLoadV8Cache, writeV8CacheFile } from './v8-sid // pre-change cache would have served as zero. origin/main at allocation is 88. // RE-CHECK AGAINST origin/main AND OPEN PRs IMMEDIATELY BEFORE MERGING — this // entry was allocated 83 first, and five bumps landed upstream before it merged. -const SCHEMA_BUMP = 89; +// 89 -> 90 (#2865): route decorator captures now carry `handlerName` in +// `decoratorRoutes`, which is what lets `resolveRouteHandlerSymbols` stamp +// `handlerSymbolId` and the routes phase emit the definition-level +// HANDLES_ROUTE edge. The field is minted in the parse WORKER and persisted +// verbatim, so a warm v89 cache replays handler-less decorator routes: every +// route loses its definition-level association on incremental analyze while +// every cold-run test passes — the inert-feature trap the entries above record. +// 89 is now taken by merged #3128. 90 is the next free value above origin/main +// (89) and above every in-flight claim found by scanning open PRs' +// parse-cache.ts at their exact head SHAs (highest other open claim was still +// ≤88). RE-CHECK AGAINST origin/main AND OPEN PRs IMMEDIATELY BEFORE MERGING. +const SCHEMA_BUMP = 90; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/integration/data-route-table-pipeline.test.ts b/gitnexus/test/integration/data-route-table-pipeline.test.ts index adff70250..0965a80b3 100644 --- a/gitnexus/test/integration/data-route-table-pipeline.test.ts +++ b/gitnexus/test/integration/data-route-table-pipeline.test.ts @@ -75,6 +75,11 @@ describe('data-driven route table ingestion', () => { const source = result.graph.getNode(rel.sourceId); const target = result.graph.getNode(rel.targetId); if (source === undefined || target === undefined) return; + // A resolved route also carries a definition-level edge from the handler + // symbol itself; this assertion is about the file-level edge, which is the + // one `http-route-extractor.ts` queries. The definition edges are pinned + // by the next test. + if (source.label !== 'File') return; handled.push({ filePath: String(source.properties.filePath), method: target.properties.method as string | undefined, @@ -106,6 +111,29 @@ describe('data-driven route table ingestion', () => { ]); }); + it('also links every resolved route from the handler definition it stamped', () => { + // The definition-level edge must agree with `Route.handlerSymbolId` — both + // come from the same graph-resolved symbol — and its source must be a real + // definition node, not a File. + const definitionEdges = new Map(); + result.graph.forEachRelationship((rel) => { + if (rel.type !== 'HANDLES_ROUTE' || rel.reason !== DATA_ROUTE_TABLE_SOURCE) return; + const source = result.graph.getNode(rel.sourceId); + if (source === undefined || source.label === 'File') return; + definitionEdges.set(rel.targetId, rel.sourceId); + }); + + const resolved = routes().filter((route) => route.handler !== undefined); + expect(resolved).toHaveLength(3); + result.graph.forEachNode((node) => { + if (node.label !== 'Route') return; + const handlerSymbolId = node.properties.handlerSymbolId as string | undefined; + if (handlerSymbolId === undefined) return; + expect(definitionEdges.get(node.id), String(node.properties.name)).toBe(handlerSymbolId); + expect(result.graph.getNode(handlerSymbolId)?.label).toMatch(/^(Function|Method)$/); + }); + }); + it('extracts response shapes from each resolved handler only', () => { const getUsers = routes().find((route) => route.path === '/users' && route.method === 'GET'); const postUsers = routes().find((route) => route.path === '/users' && route.method === 'POST'); diff --git a/gitnexus/test/integration/fastapi-composed-route-constants.test.ts b/gitnexus/test/integration/fastapi-composed-route-constants.test.ts index e0701e37c..89e6bcb45 100644 --- a/gitnexus/test/integration/fastapi-composed-route-constants.test.ts +++ b/gitnexus/test/integration/fastapi-composed-route-constants.test.ts @@ -27,7 +27,13 @@ import { loadParseCache, saveParseCache, PARSE_CACHE_VERSION, + pruneCache, + type ParseCache, } from '../../src/storage/parse-cache.js'; +import { + getDurableParsedFileDir, + pruneAndSaveDurableParsedFileStore, +} from '../../src/storage/parsedfile-store.js'; const FIXTURE = path.resolve(__dirname, '..', 'fixtures', 'fastapi-composed-app'); @@ -164,35 +170,67 @@ describe('FastAPI composed route constants — ingestion↔group parity (#2391 R // ─── Warm parse-cache: composed routes survive the cache serialization ──────── -describe('FastAPI composed route constants — warm parse-cache (#2391 SCHEMA_BUMP)', () => { - it('re-resolves the composed route on an all-hit warm run after a save/load round-trip', async () => { +describe('FastAPI composed routes — warm parse-cache (#2391, #2865)', () => { + it('replays composed paths and handler metadata on an all-hit warm run', async () => { const storageDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-composed-warm-')); try { - // Run #1 populates the parse cache. - const cold = { + const cold: ParseCache = { version: PARSE_CACHE_VERSION, entries: new Map(), usedKeys: new Set(), + storagePath: storageDir, + onDiskKeys: new Set(), }; - await runPipelineFromRepo(FIXTURE, () => {}, { parseCache: cold }); - - // Force the JSON round-trip (mapReplacer/mapReviver) the real warm path uses - // — this is where the new `moduleConstants` Maps and `routePathExpr` fields - // must survive, or a warm re-analyze silently drops the composed route. - await saveParseCache(storageDir, cold); - const warm = await loadParseCache(storageDir); - expect(warm).not.toBeNull(); - - const result = await runPipelineFromRepo(FIXTURE, () => {}, { - parseCache: warm ?? undefined, + const coldResult = await runPipelineFromRepo(FIXTURE, () => {}, { + parseCache: cold, + workerPoolSize: 1, }); + expect(coldResult.usedWorkerPool).toBe(true); + + pruneCache(cold, cold.usedKeys); + const savedKeys = await saveParseCache(storageDir, cold); + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storageDir), + PARSE_CACHE_VERSION, + new Set(savedKeys), + ); + const warm = await loadParseCache(storageDir); + const replay = await runPipelineFromRepo(FIXTURE, () => {}, { + parseCache: warm, + workerPoolSize: 1, + }); + expect(replay.usedWorkerPool).toBe(false); + const urls = new Set(); - result.graph.forEachNode((n) => { + replay.graph.forEachNode((n) => { if (n.label === 'Route') urls.add(String(n.properties.name)); }); expect(urls.has('/api/v1/widgets/get')).toBe(true); expect(urls.has('/root/mid/leaf')).toBe(true); expect(urls.has('/')).toBe(false); + + const handlers = (pipeline: PipelineResult): Map => { + const out = new Map(); + pipeline.graph.forEachNode((node) => { + if (node.label !== 'Route') return; + const id = node.properties.handlerSymbolId; + if (typeof id === 'string') out.set(String(node.properties.name), id); + }); + return out; + }; + const coldHandlers = handlers(coldResult); + expect(coldHandlers.get('/api/v1/widgets/get')).toMatch(/create_widget/); + expect(handlers(replay)).toEqual(coldHandlers); + + const handlerId = coldHandlers.get('/api/v1/widgets/get'); + const routeId = [...replay.graph.iterNodes()].find( + (node) => node.label === 'Route' && node.properties.name === '/api/v1/widgets/get', + )?.id; + expect( + [...replay.graph.iterRelationshipsByType('HANDLES_ROUTE')].some( + (edge) => edge.sourceId === handlerId && edge.targetId === routeId, + ), + ).toBe(true); } finally { fs.rmSync(storageDir, { recursive: true, force: true }); } diff --git a/gitnexus/test/unit/group/http-route-graph-method.test.ts b/gitnexus/test/unit/group/http-route-graph-method.test.ts index 0391c28ce..44daf6756 100644 --- a/gitnexus/test/unit/group/http-route-graph-method.test.ts +++ b/gitnexus/test/unit/group/http-route-graph-method.test.ts @@ -248,6 +248,91 @@ describe('HttpRouteExtractor — Route.method from graph (Step A / #2138)', () = expect(out[0].symbolName).toBe('createOrder'); }); + it('fast path: CONTAINING_QUERY runs once per file, not once per resolved route', async () => { + // The fast path resolves a handler per ROUTE but CONTAINING_QUERY is a + // per-FILE lookup. Three resolved routes in one controller must therefore + // execute it exactly once — otherwise a group sync re-scans the same file + // for every route it registers. + const ids = ['listOrders', 'createOrder', 'deleteOrder'].map( + (name) => [name, `Method:OrderController.java:OrderController.${name}#0`] as const, + ); + const symbols = ids.map(([name, uid]) => ({ + uid, + name, + filePath: 'OrderController.java', + startLine: 10, + endLine: 12, + labels: ['Method'], + 0: uid, + 1: name, + 2: 'OrderController.java', + })); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return ids.map(([name, uid], i) => ({ + fileId: 'f1', + filePath: 'OrderController.java', + routePath: `/api/orders/${name}`, + routeId: `r${i}`, + routeMethod: 'GET', + handlerSymbolId: uid, + routeSource: 'framework-route', + })); + } + if (query.includes('UNION ALL')) return symbols; + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + + const containingCalls = db.mock.calls.filter(([query]) => query.includes('UNION ALL')); + expect(containingCalls).toHaveLength(1); + // Memoization must not cost resolution: every route still names its handler. + expect(out.map((c) => c.symbolName).sort()).toEqual([ + 'createOrder', + 'deleteOrder', + 'listOrders', + ]); + }); + + it('fast path: a failed CONTAINING_QUERY is cached as empty and not retried per route', async () => { + // Failures previously fell through to the uid + basename fallback per route; + // memoizing must keep that fallback while collapsing the retries. + const ids = ['listOrders', 'createOrder'].map( + (name) => [name, `Method:OrderController.java:OrderController.${name}#0`] as const, + ); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return ids.map(([name, uid], i) => ({ + fileId: 'f1', + filePath: 'OrderController.java', + routePath: `/api/orders/${name}`, + routeId: `r${i}`, + routeMethod: 'GET', + handlerSymbolId: uid, + routeSource: 'framework-route', + })); + } + if (query.includes('UNION ALL')) throw new Error('boom'); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + + expect(db.mock.calls.filter(([query]) => query.includes('UNION ALL'))).toHaveLength(1); + expect(out.map((c) => c.symbolUid).sort()).toEqual(ids.map(([, uid]) => uid).sort()); + // The authoritative uid survives; only the display name falls back. + for (const contract of out) expect(contract.symbolName).toBe('OrderController.java'); + }); + it('backward-compat: no Route.method and undecodable reason stays at conservative GET', async () => { FILE_DETECTIONS.set('routes.ts', [detection('provider', 'POST', '/api/orders', 'createOrder')]); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 6f73c2dba..058f12217 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -244,12 +244,17 @@ describe('PARSE_CACHE_VERSION', () => { // collided, because each re-checked once and neither re-checked after the // other moved — which is why the rule is re-applied AT MERGE, not when the // number is picked. - it('pins SCHEMA_BUMP to 89 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(89); + // Moved 89 -> 90 for #2865's decorator-route `handlerName` after #3128 + // merged and took 89. origin/main is 89; 90 is the next free value and + // still unused by other open PRs' parse-cache.ts heads — the same + // collision the paragraph above describes, caught this time by re-checking + // at merge. + it('pins SCHEMA_BUMP to 90 so concurrent bumps cannot silently collide (#2766, #3015, #3088, #2885, #3128, #2865)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(90); expect(PARSE_CACHE_BUCKET_COUNT).toBe(128); for (const taken of [ 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, - 82, 83, 84, 85, 86, 87, 88, + 82, 83, 84, 85, 86, 87, 88, 89, ]) { expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).not.toBe(taken); } diff --git a/gitnexus/test/unit/python-decorator-handler-name.test.ts b/gitnexus/test/unit/python-decorator-handler-name.test.ts new file mode 100644 index 000000000..8a358063f --- /dev/null +++ b/gitnexus/test/unit/python-decorator-handler-name.test.ts @@ -0,0 +1,96 @@ +/** + * Pins Python's `decoratorRouteHandlerName` provider hook against real + * tree-sitter-python trees. + * + * The hook feeds `ExtractedDecoratorRoute.handlerName`, which the routes phase + * turns into `handlerSymbolId` and a definition-level `HANDLES_ROUTE` edge. Two + * failure directions matter and both are asserted here: + * + * • too little — a plain module function, a method, a stacked-decorator run, + * or an `async def` must all yield the decorated function's name, or every + * Flask/FastAPI handler silently degrades to a file-level edge; + * • too much — a class-attached route decorator must yield nothing. A class + * does not handle a request, and returning its name would resolve + * `handlerSymbolId` to the wrong symbol. + */ + +import { describe, it, expect } from 'vitest'; +import Parser from 'tree-sitter'; +import Python from 'tree-sitter-python'; +import { pythonDecoratorRouteHandlerName } from '../../src/core/ingestion/route-extractors/python-decorator-handler.js'; +import type { SyntaxNode } from '../../src/core/ingestion/utils/ast-helpers.js'; + +const parser = new Parser(); +parser.setLanguage(Python); + +/** Every `decorator` node in `src`, in source order. */ +function decorators(src: string): SyntaxNode[] { + const found: SyntaxNode[] = []; + const walk = (node: SyntaxNode): void => { + if (node.type === 'decorator') found.push(node); + for (const child of node.children) walk(child); + }; + walk(parser.parse(src).rootNode); + return found; +} + +/** Handler names the hook reports for each decorator in `src`. */ +const handlerNames = (src: string): Array => + decorators(src).map((node) => pythonDecoratorRouteHandlerName(node)); + +describe('pythonDecoratorRouteHandlerName', () => { + it('names the module-level function a route decorator sits on', () => { + expect(handlerNames('@router.get("/widgets")\ndef list_widgets(): pass\n')).toEqual([ + 'list_widgets', + ]); + }); + + it('names a method inside a class', () => { + expect( + handlerNames('class WidgetView:\n @router.post("/widgets")\n def create(self): pass\n'), + ).toEqual(['create']); + }); + + it('names the same function for every decorator in a stacked run', () => { + // tree-sitter-python puts all decorators of a run under one + // `decorated_definition`, so no ancestor walk is needed to reach the + // definition past the sibling decorator. + expect(handlerNames('@router.get("/me")\n@requires_auth\ndef whoami(): pass\n')).toEqual([ + 'whoami', + 'whoami', + ]); + }); + + it('names an async handler (`async def` is still a function_definition)', () => { + expect(handlerNames('@app.get("/health")\nasync def health(): pass\n')).toEqual(['health']); + }); + + it('returns undefined for a class-attached route decorator', () => { + expect(handlerNames('@router.get("/widgets")\nclass WidgetResource: pass\n')).toEqual([ + undefined, + ]); + }); + + it('does not climb out of a class body to borrow the enclosing class name', () => { + // The decorator's parent here is the class body's `decorated_definition` + // holding a class, not a function. An ancestor walk would have found + // `Outer`; direct-shape ownership reports nothing. + expect(handlerNames('class Outer:\n @router.get("/x")\n class Inner: pass\n')).toEqual([ + undefined, + ]); + }); + + it('names the real def when a commented-out def precedes it', () => { + // Python applies the decorator to the next real definition; the comment is + // not a definition, so `real_handler` is the correct answer. + expect( + handlerNames('@router.get("/x")\n# def old_handler(): pass\ndef real_handler(): pass\n'), + ).toEqual(['real_handler']); + }); + + it('returns undefined for a non-route decorator context with no decorated definition', () => { + // A bare decorator with no following definition is an ERROR/partial parse; + // the hook must not invent a name from whatever the parent happens to be. + expect(handlerNames('@router.get("/x")\n')).toEqual([undefined]); + }); +}); diff --git a/gitnexus/test/unit/route-handler-definition-edge.test.ts b/gitnexus/test/unit/route-handler-definition-edge.test.ts new file mode 100644 index 000000000..a668529cc --- /dev/null +++ b/gitnexus/test/unit/route-handler-definition-edge.test.ts @@ -0,0 +1,192 @@ +/** + * Routes phase: the definition-level `HANDLES_ROUTE` edge and the + * `Route.handlerSymbolId` stamp, both gated on the handler symbol EXISTING in + * the graph. + * + * `routeHandlerSymbols` carries `SemanticModel` node ids, which are a + * resolution claim rather than proof that the definition node reached + * `ctx.graph`. Two directions are pinned here: + * + * • resolved — a live `Function` / `Method` node gets BOTH the file-level edge + * (what `http-route-extractor.ts` queries) and the definition-level edge, so + * a HANDLES_ROUTE traversal reaches the handler and not just its file; + * • dangling — an id with no node in the graph falls back to exactly the + * file-level behavior a route with no handler resolution has: file-level + * edge only, no `handlerSymbolId` stamp, and no edge sourced on a node that + * does not exist. + */ +import { describe, expect, it } from 'vitest'; +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import type { NodeLabel } from 'gitnexus-shared'; +import { routesPhase } from '../../src/core/ingestion/pipeline-phases/routes.js'; +import { routeNodeKey } from '../../src/core/ingestion/route-extractors/route-path.js'; +import { DATA_ROUTE_TABLE_SOURCE } from '../../src/core/ingestion/route-extractors/data-route-table.js'; +import type { ParseOutput } from '../../src/core/ingestion/pipeline-phases/parse.js'; +import { createKnowledgeGraph } from '../../src/core/graph/graph.js'; +import type { KnowledgeGraph } from '../../src/core/graph/types.js'; +import { generateId } from '../../src/lib/utils.js'; + +const CONTROLLER = 'src/orders/controller.ts'; +const TABLE_FILE = 'src/routes/table.ts'; +const HANDLER_FILE = 'src/handlers/orders.ts'; +const HANDLER_SOURCE = `export function createOrder() {\n return { id: 1 };\n}\n`; + +interface RunOptions { + /** Files to materialize in the temp repo, path → contents. */ + readonly files?: Readonly>; + /** Handler definition node to seed into the graph before the phase runs. */ + readonly handlerNode?: { id: string; label: NodeLabel; filePath: string }; + readonly routeHandlerSymbols: ReadonlyMap; + readonly extractedRoutes?: readonly unknown[]; + readonly decoratorRoutes?: readonly unknown[]; +} + +async function runRoutesPhase( + options: RunOptions, +): Promise<{ graph: KnowledgeGraph; repoPath: string }> { + const repoPath = await fs.mkdtemp(path.join(os.tmpdir(), 'gitnexus-route-handler-edge-')); + const files = options.files ?? { [CONTROLLER]: HANDLER_SOURCE }; + for (const [filePath, contents] of Object.entries(files)) { + await fs.mkdir(path.join(repoPath, path.dirname(filePath)), { recursive: true }); + await fs.writeFile(path.join(repoPath, filePath), contents); + } + + const graph = createKnowledgeGraph(); + if (options.handlerNode) { + graph.addNode({ + id: options.handlerNode.id, + label: options.handlerNode.label, + properties: { + name: 'createOrder', + filePath: options.handlerNode.filePath, + startLine: 0, + endLine: 2, + }, + }); + } + + const parseOutput = { + allPaths: Object.keys(files), + allFetchCalls: [], + allFetchWrapperDefs: [], + allExtractedRoutes: options.extractedRoutes ?? [], + allDecoratorRoutes: options.decoratorRoutes ?? [], + routeHandlerSymbols: options.routeHandlerSymbols, + } as unknown as ParseOutput; + + try { + await routesPhase.execute( + { repoPath, graph, onProgress: () => {}, pipelineStart: Date.now() }, + new Map([['parse', { phaseName: 'parse', output: parseOutput, durationMs: 0 }]]), + ); + return { graph, repoPath }; + } finally { + await fs.rm(repoPath, { recursive: true, force: true }); + } +} + +/** A single framework route: `POST /orders`, declared in `CONTROLLER`. */ +const frameworkRoute = (filePath = CONTROLLER) => ({ + filePath, + httpMethod: 'post', + routePath: '/orders', + routeName: null, + controllerName: null, + methodName: null, + middleware: [], + prefix: null, + lineNumber: 1, +}); + +const POST_ORDERS = routeNodeKey('POST', '/orders'); +const ROUTE_ID = generateId('Route', POST_ORDERS); + +/** `HANDLES_ROUTE` source ids pointing at the route node, de-duplicated. */ +const handlesRouteSources = (graph: KnowledgeGraph, routeNodeId = ROUTE_ID): string[] => [ + ...new Set( + graph.relationships + .filter((rel) => rel.type === 'HANDLES_ROUTE' && rel.targetId === routeNodeId) + .map((rel) => rel.sourceId), + ), +]; + +describe('routes phase — definition-level HANDLES_ROUTE', () => { + it.each(['Function', 'Method'])( + 'emits both the file-level and the definition-level edge for a live %s handler', + async (label) => { + const handlerId = `${label}:${CONTROLLER}:createOrder`; + const { graph } = await runRoutesPhase({ + handlerNode: { id: handlerId, label, filePath: CONTROLLER }, + routeHandlerSymbols: new Map([[POST_ORDERS, handlerId]]), + extractedRoutes: [frameworkRoute()], + }); + + expect(graph.getNode(ROUTE_ID)?.properties.handlerSymbolId).toBe(handlerId); + expect(handlesRouteSources(graph).sort()).toEqual( + [generateId('File', CONTROLLER), handlerId].sort(), + ); + }, + ); + + it('falls back to file-level attribution when the handler id has no node in the graph', async () => { + // Resolution claimed a symbol the graph does not hold. The stamp is the + // extractor's fast path and the edge would source on an absent node, so + // neither may be emitted. + const { graph } = await runRoutesPhase({ + routeHandlerSymbols: new Map([[POST_ORDERS, `Method:${CONTROLLER}:ghost`]]), + extractedRoutes: [frameworkRoute()], + }); + + const route = graph.getNode(ROUTE_ID); + expect(route, 'the route itself must survive a dangling handler id').toBeTruthy(); + expect(route?.properties).not.toHaveProperty('handlerSymbolId'); + expect(handlesRouteSources(graph)).toEqual([generateId('File', CONTROLLER)]); + }); + + it('resolves the data-route-table handler path from the graph, and falls back when it dangles', async () => { + const routeKey = routeNodeKey('GET', '/orders'); + const routeNodeId = generateId('Route', routeKey); + const handlerId = `Function:${HANDLER_FILE}:createOrder`; + const dataRoute = { + filePath: TABLE_FILE, + routePath: '/orders', + httpMethod: 'GET', + decoratorName: 'data-route-table', + source: DATA_ROUTE_TABLE_SOURCE, + lineNumber: 1, + handlerName: 'createOrder', + }; + const files = { + [TABLE_FILE]: `export const routes = [{ path: '/orders', method: 'GET', handler: createOrder }];\n`, + [HANDLER_FILE]: HANDLER_SOURCE, + }; + + const resolved = await runRoutesPhase({ + files, + handlerNode: { id: handlerId, label: 'Function', filePath: HANDLER_FILE }, + routeHandlerSymbols: new Map([[routeKey, handlerId]]), + decoratorRoutes: [dataRoute], + }); + const resolvedRoute = resolved.graph.getNode(routeNodeId); + expect(resolvedRoute?.properties.filePath).toBe(HANDLER_FILE); + expect(resolvedRoute?.properties.handlerSymbolId).toBe(handlerId); + expect(handlesRouteSources(resolved.graph, routeNodeId).sort()).toEqual( + [generateId('File', HANDLER_FILE), handlerId].sort(), + ); + + const dangling = await runRoutesPhase({ + files, + routeHandlerSymbols: new Map([[routeKey, handlerId]]), + decoratorRoutes: [dataRoute], + }); + const danglingRoute = dangling.graph.getNode(routeNodeId); + // No handler node to read a path from, so the declaring file stands in. + expect(danglingRoute?.properties.filePath).toBe(TABLE_FILE); + expect(danglingRoute?.properties).not.toHaveProperty('handlerSymbolId'); + expect(handlesRouteSources(dangling.graph, routeNodeId)).toEqual([ + generateId('File', TABLE_FILE), + ]); + }); +});