diff --git a/gitnexus/bench/emit-persistence/baselines.json b/gitnexus/bench/emit-persistence/baselines.json index 55307c2fc..ce67aafdf 100644 --- a/gitnexus/bench/emit-persistence/baselines.json +++ b/gitnexus/bench/emit-persistence/baselines.json @@ -1,5 +1,5 @@ { - "fingerprint": "4cc418ea87b6d20a68b5c1139f35d81820b715c63de0ec812e73e2135f5b00b1", + "fingerprint": "b169463b7d02185d757b6d8601db6215ac6e7b2a20e52fb0f1276cc153836bd4", "scaling_budget": 1.8, "max_ms_large": 1000, "_note": "fingerprint = sha256 over per-file digests (filename + sha256(file bytes)), entry list sorted — binds each emitted line to its file so a row routed to the WRONG pair file changes the hash, AND catches within-file row reordering (file bytes hashed as-written). Byte-identity gate for #2203 U2/U3. NOTE: a future change that legitimately reorders emit (without changing the node/edge SET) will trip --check; regenerate then. scaling_budget bounds (t_large/t_small)/(LARGE/SMALL): observed ~0.95-1.05 (linear); 1.8 tolerates disk-I/O timing noise on CI while still catching an O(n^2) re-regression (~4x). max_ms_large=1000ms is a coarse absolute backstop (observed ~200ms) that catches a gross uniform slowdown the ratio gate misses; generous so CI host noise won't flake it. Regenerate via `node --import tsx bench/emit-persistence/measure.mjs`." diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 7d70864cd..c62d9f937 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -676,6 +676,26 @@ function scanSpringProject(files: readonly HttpScanInput[]): HttpFileDetections[ export const JAVA_HTTP_PLUGIN: HttpLanguagePlugin = { name: 'java-http', language: Java, + // routeCoverage intentionally LEFT at the default 'partial' (#2138 Part 2). + // The graph provider set is a strict *subset* of this scan()'s provider set — + // ingestion does NOT emit a Route node for (1) array-form `@GetMapping({...})`, + // (2) interface-inherited Spring routes, or (3) the 2nd verb of a same-URL + // GET+POST pair (Route nodes are URL-keyed). Declaring 'complete' here would + // let the parse-skip drop those group-only providers. Java flips to 'complete' + // only once ingestion provider extraction matches this scan (a follow-up: + // array-form query branch + interface-inheritance emission + per-verb Route + // identity). `hasConsumerSignals` below is kept ready for that flip. + // Consumer signals this plugin's scan() can detect: RestTemplate / WebClient / + // OkHttp / Java-HttpClient / Apache-HttpClient call sites, OpenFeign + // (`@FeignClient` + `@RequestLine`) interfaces, and Spring 6 HTTP Interface + // `@(Get|...)Exchange` / `@HttpExchange`. A provider-covered file containing + // any of these must still be parsed so its consumer contracts are not dropped + // (ingestion emits no FETCHES for Java). Conservative by design. + hasConsumerSignals(content) { + return /\brestTemplate\b|\bwebClient\b|Request\.Builder|HttpRequest|HttpMethod\.|new\s+Http(Get|Post|Put|Delete|Patch)\b|@RequestLine|@FeignClient|Exchange/.test( + content, + ); + }, scan(tree) { const out: HttpDetection[] = []; diff --git a/gitnexus/src/core/group/extractors/http-patterns/php.ts b/gitnexus/src/core/group/extractors/http-patterns/php.ts index c1c40a09c..a2042cfd4 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/php.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/php.ts @@ -130,6 +130,17 @@ function isHttpUrlLiteral(path: string): boolean { export const PHP_HTTP_PLUGIN: HttpLanguagePlugin = { name: 'php-http', language: PHP.php_only, + // Laravel `Route::(...)` definitions are emitted as Route nodes by + // ingestion, so the graph is authoritative for PHP providers (#2138 Part 2). + routeCoverage: 'complete', + // Consumer signals scan() can detect: Laravel `Http::`, Guzzle client + // `->get/post/.../request(...)`, and `file_get_contents` of an HTTP URL. A + // provider-covered file with any of these must still be parsed (ingestion + // emits no FETCHES for PHP). Conservative — the `->verb(` shape over-matches + // ordinary method calls, which only costs a parse, never data. + hasConsumerSignals(content) { + return /Http::|file_get_contents|->\s*(get|post|put|delete|patch|request)\s*\(/i.test(content); + }, scan(tree) { const out: HttpDetection[] = []; diff --git a/gitnexus/src/core/group/extractors/http-patterns/python.ts b/gitnexus/src/core/group/extractors/http-patterns/python.ts index 6408f0eca..cadaacc32 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/python.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/python.ts @@ -920,6 +920,22 @@ function joinPrefix(prefix: string, route: string): string { 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'. + // 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. + hasConsumerSignals(content) { + return /\brequests\s*\.|\bhttpx\b|\baiohttp\b|\burllib\b|\burlopen\b|\buri\s*=|\burl\s*=/.test( + content, + ); + }, prepareRepo({ files, parser, readFile, parseSource }): RepoContext { return buildPythonRepoContext(files, parser, readFile, parseSource); }, diff --git a/gitnexus/src/core/group/extractors/http-patterns/types.ts b/gitnexus/src/core/group/extractors/http-patterns/types.ts index fb4ab09cb..22f6597ba 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/types.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/types.ts @@ -78,6 +78,43 @@ export interface HttpLanguagePlugin { name: string; /** tree-sitter grammar object (passed to the shared parser). */ language: unknown; + /** + * Whether ingestion is known to emit a `Route` graph node for EVERY + * provider route in this language (Spring/FastAPI/Laravel annotations are + * extracted into Route nodes during parse). When `'complete'`, the + * orchestrator may skip the source-scan + tree-sitter parse for a file whose + * graph provider routes all resolved a handler symbol (#2138 Part 2) — the + * graph is authoritative, the scan would only re-discover the same routes. + * + * Defaults to `'partial'` (the safe assumption): the source scan always runs, + * so a language whose ingestion coverage is incomplete never loses routes. + * This is a deliberate, per-language trust assertion — set it only for + * languages whose route ingestion is provably complete. + */ + routeCoverage?: 'complete' | 'partial'; + /** + * Cheap, parse-free pre-check used by the parse-skip optimization (#2138 + * Part 2). Given a file's raw source text, return `false` ONLY when the file + * provably contains no outbound-HTTP (consumer) call that this plugin's + * `scan()` would detect; return `true` on any doubt. + * + * Why it exists: `routeCoverage: 'complete'` asserts *provider* Route-node + * completeness only. A provider-covered file may ALSO be a consumer (e.g. a + * Spring `@RestController` that calls `restTemplate`/`webClient`, a Laravel + * controller using Guzzle, a FastAPI handler calling `requests`/`httpx`). + * Ingestion's `FETCHES` edges are JS/TS-only, so the graph cannot back up + * those server-side consumers — they come solely from the source scan. The + * orchestrator may therefore skip a provider-covered file's parse only when + * this returns `false`; otherwise the file is still scanned so its consumer + * contracts are not dropped. + * + * MUST be implemented by any plugin whose `scan()` can emit `'consumer'` + * detections AND that declares `routeCoverage: 'complete'`; otherwise that + * language's provider-covered files are never parse-skipped (safe, no win). + * The check is intentionally conservative — over-matching only costs a parse + * that could have been skipped; it never drops data. + */ + hasConsumerSignals?(content: string): boolean; /** * Optional pre-pass: walk the relevant files in the repo and produce * an opaque context that `scan` can use to resolve cross-file facts. diff --git a/gitnexus/src/core/group/extractors/http-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index 450cf3841..722161e20 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -49,6 +49,7 @@ MATCH (handlerFile:File)-[r:CodeRelation {type: 'HANDLES_ROUTE'}]->(route:Route) RETURN handlerFile.id AS fileId, handlerFile.filePath AS filePath, route.name AS routePath, route.id AS routeId, route.method AS routeMethod, + route.handlerSymbolId AS handlerSymbolId, route.responseKeys AS responseKeys, r.reason AS routeSource`; const FETCHES_QUERY = ` @@ -282,22 +283,57 @@ export class HttpRouteExtractor implements ContractExtractor { }; const files = await getScannedFiles(); - await collectProjectDetections(files); + // Run the graph provider pass FIRST. After #2138 Part 2 it reads handler + // symbols from the graph (no source parse for resolved routes), so it can + // report which files are fully graph-covered BEFORE we decide what to + // parse. Files fully covered by a `routeCoverage: 'complete'` language are + // candidates to skip the source scan + tree-sitter parse — but only their + // *providers* are graph-authoritative; the consumer-safety gate below + // removes any candidate that still needs scanning for outbound calls. + const coveredFiles = new Set(); const graphProviders = - dbExecutor != null ? await this.extractProvidersGraph(dbExecutor, getDetections) : []; - // Source scan always runs to capture routes in languages/files not covered - // by graph edges; the glob and per-file parse results are cached above. + dbExecutor != null + ? await this.extractProvidersGraph(dbExecutor, getDetections, coveredFiles) + : []; + + // Consumer-safety gate (#2138 Part 2): `extractProvidersGraph` marks a file + // covered on *provider* grounds (all HANDLES_ROUTE rows resolved + a + // `routeCoverage: 'complete'` language). But a provider-covered file may also + // be a *consumer* (a controller that calls RestTemplate/WebClient/Guzzle/ + // requests/...), and ingestion emits no FETCHES edges for those server-side + // languages — the graph can't back them up. So a covered file is only truly + // safe to skip (parse) when its plugin can PROVE, from a cheap parse-free + // text scan, that it holds no such consumer call. Anything else (a positive + // signal, no `hasConsumerSignals` hook, or an unreadable file) stays in the + // scan set so its consumer contracts are preserved. + for (const f of [...coveredFiles]) { + const plugin = getPluginForFile(f); + const content = readSafe(repoPath, f); + const provenNoConsumer = + content != null && typeof plugin?.hasConsumerSignals === 'function' + ? plugin.hasConsumerSignals(content) === false + : false; + if (!provenNoConsumer) coveredFiles.delete(f); + } + + // Everything the graph did not fully cover still gets a full source scan + // (fail-open: partial-coverage languages, unresolved routes, and graph-less + // runs all land here). + const scanFiles = files.filter((f) => !coveredFiles.has(f)); + + await collectProjectDetections(scanFiles); + const providers = this.mergeGraphAndSourceContracts( graphProviders, - await this.extractProvidersSourceScan(files, getDetections), + await this.extractProvidersSourceScan(scanFiles, getDetections), ); const graphConsumers = dbExecutor != null ? await this.extractConsumersGraph(dbExecutor, getDetections) : []; const consumers = this.mergeGraphAndSourceContracts( graphConsumers, - await this.extractConsumersSourceScan(files, getDetections), + await this.extractConsumersSourceScan(scanFiles, getDetections), ); return [...providers, ...consumers]; @@ -323,8 +359,14 @@ export class HttpRouteExtractor implements ContractExtractor { private async extractProvidersGraph( db: CypherExecutor, getDetections: (rel: string) => Promise, + coveredFiles?: Set, ): Promise { const out: ExtractedContract[] = []; + // Per-file coverage tracking (#2138 Part 2): a file is "fully graph-covered" + // when every one of its HANDLES_ROUTE rows resolved a handlerSymbolId AND its + // language plugin declares `routeCoverage: 'complete'`. Such files can skip + // the source scan + parse entirely — the graph is authoritative for them. + const fileAllResolved = new Map(); let rows: Record[]; try { rows = await db(HANDLES_ROUTE_QUERY); @@ -354,67 +396,90 @@ export class HttpRouteExtractor implements ContractExtractor { .toUpperCase(); let method = (graphMethod || null) ?? methodFromRouteReason(routeSource); - // Look up handler name (and backfill method if missing) from the - // plugin's scan of the handler file. This replaces the old - // regex-based `inferMethodFromFileScan` and `pickJavaHandlerName` - // helpers — tree-sitter gives both pieces of information - // structurally. Always run the lookup: even when method is set by - // `methodFromRouteReason`, we still need the handler name. - const detections = filePath ? await getDetections(filePath) : []; - const providerDetections = detections.filter((d) => d.role === 'provider'); - let handlerName: string | null = null; - const normalizedRoute = normalizeHttpPath(routePath); - // Candidates share the same normalized path. When multiple - // detections at the same path exist (e.g. GET + POST /api/orders - // in one router), a blind `.find()` silently returned the first - // verb — attaching the wrong handler and, when method was not - // already pinned by the route reason, the wrong method too. - // Disambiguate by method when we know it; refuse to guess when - // we don't. - const candidates = providerDetections.filter( - (d) => normalizeHttpPath(d.path) === normalizedRoute, - ); - let match: (typeof candidates)[number] | undefined; - const ambiguousCandidates = !method && candidates.length > 1; - if (method) { - match = candidates.find((d) => d.method === method); - } else if (candidates.length === 1) { - match = candidates[0]; + const handlerSymbolId = String(row.handlerSymbolId ?? '').trim(); + const fileId = row.fileId ?? row[0]; + // Track per-file resolution for the parse-skip coverage set: a file stays + // "all resolved" only while every one of its rows carries a handlerSymbolId. + if (filePath) { + const prev = fileAllResolved.get(filePath); + fileAllResolved.set(filePath, (prev ?? true) && handlerSymbolId.length > 0); } - // else: multiple candidates + unknown method → leave match - // undefined so handlerName stays null and skip symbol - // enrichment below, keeping the file-basename fallback instead - // of letting pickSymbolUid silently pick the first Function / - // Method in the file (which reintroduces the mis-attribution - // we were trying to avoid). Method stays at the conservative - // 'GET' default set below. - if (match) { - if (!method) method = match.method; - handlerName = match.name; - } - if (!method) method = 'GET'; - - const pathNorm = normalizeHttpPath(routePath); - const cid = contractIdFor(method, pathNorm); + const pathNormEarly = normalizeHttpPath(routePath); let symbolUid = ''; let symbolName = path.basename(filePath) || 'handler'; let symPath = filePath; - const fileId = row.fileId ?? row[0]; - if (fileId && !ambiguousCandidates) { - try { - const syms = await db(CONTAINS_QUERY, { fileId }); - if (syms.length > 0) { - const picked = pickSymbolUid(syms, handlerName); - symbolUid = picked.uid; - symbolName = picked.name; - symPath = picked.filePath || filePath; + + if (handlerSymbolId) { + // Fast path (Part 2, #2138): the handler symbol was resolved during + // ingestion and persisted on the Route node, so we read it straight + // from the graph and SKIP the `getDetections()` source-scan/parse the + // legacy path needed just to recover the handler name. CONTAINS is a + // cheap graph query (no tree-sitter parse) used only to surface the + // handler's display name/path; the uid is authoritative regardless. + if (!method) method = 'GET'; + symbolUid = handlerSymbolId; + if (fileId) { + try { + const syms = await db(CONTAINS_QUERY, { fileId }); + 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 */ + } + } + } else { + // Legacy fallback (old index / unresolved handler): recover the handler + // name from the plugin's scan of the handler file (this parses source). + // Always run the lookup: even when method is set, we still need the name. + const detections = filePath ? await getDetections(filePath) : []; + const providerDetections = detections.filter((d) => d.role === 'provider'); + let handlerName: string | null = null; + // Candidates share the same normalized path. When multiple detections at + // the same path exist (GET + POST /api/orders in one router), a blind + // `.find()` silently returned the first verb — attaching the wrong + // handler/method. Disambiguate by method when known; refuse to guess. + const candidates = providerDetections.filter( + (d) => normalizeHttpPath(d.path) === pathNormEarly, + ); + let match: (typeof candidates)[number] | undefined; + const ambiguousCandidates = !method && candidates.length > 1; + if (method) { + match = candidates.find((d) => d.method === method); + } else if (candidates.length === 1) { + match = candidates[0]; + } + // else: multiple candidates + unknown method → leave match undefined so + // handlerName stays null and we skip symbol enrichment, keeping the + // file-basename fallback rather than letting pickSymbolUid pick the + // first Function/Method (which reintroduces mis-attribution). + if (match) { + if (!method) method = match.method; + handlerName = match.name; + } + if (!method) method = 'GET'; + + if (fileId && !ambiguousCandidates) { + try { + const syms = await db(CONTAINS_QUERY, { fileId }); + if (syms.length > 0) { + const picked = pickSymbolUid(syms, handlerName); + symbolUid = picked.uid; + symbolName = picked.name; + symPath = picked.filePath || filePath; + } + } catch { + /* ignore */ } - } catch { - /* ignore */ } } + const pathNorm = pathNormEarly; + const cid = contractIdFor(method, pathNorm); + out.push({ contractId: cid, type: 'http', @@ -432,6 +497,18 @@ export class HttpRouteExtractor implements ContractExtractor { }, }); } + + // Populate the parse-skip coverage set: files whose every provider route + // resolved a handler symbol AND whose language declares complete ingestion + // route coverage. Fail-open — any unresolved row or a 'partial' language + // leaves the file out, so it still gets a full source scan. + if (coveredFiles) { + for (const [fp, allResolved] of fileAllResolved) { + if (allResolved && getPluginForFile(fp)?.routeCoverage === 'complete') { + coveredFiles.add(fp); + } + } + } return out; } diff --git a/gitnexus/src/core/ingestion/call-processor.ts b/gitnexus/src/core/ingestion/call-processor.ts index b5b258dcd..20aacffd5 100644 --- a/gitnexus/src/core/ingestion/call-processor.ts +++ b/gitnexus/src/core/ingestion/call-processor.ts @@ -21,7 +21,9 @@ import { generateId } from '../../lib/utils.js'; import type { SymbolDefinition } from 'gitnexus-shared'; import { yieldToEventLoop } from './utils/event-loop.js'; import type { ExtractedRoute, ExtractedFetchCall } from './workers/parse-worker.js'; +import type { ExtractedDecoratorRoute } from './workers/parse-worker.js'; import { normalizeFetchURL, routeMatches } from './route-extractors/nextjs.js'; +import { normalizeExtractedRoutePath } from './route-extractors/route-path.js'; import { extractReturnTypeName } from './type-extractors/shared.js'; const MAX_EXPORTS_PER_FILE = 500; @@ -243,6 +245,83 @@ export const processRoutesFromExtracted = async ( onProgress?.(extractedRoutes.length, extractedRoutes.length); }; +/** + * Resolve each route's handler to a real symbol UID, keyed by the normalized + * route URL (the same key the routes phase uses for the `Route` node). This is + * the Part 2 (#2138) groundwork that lets `HttpRouteExtractor.extractProvidersGraph` + * read the handler symbol from the graph instead of re-parsing source via + * `getDetections()`. + * + * Two route shapes, one resolution target — `(filePath, name) → nodeId`: + * - Laravel framework routes (`ExtractedRoute`) carry `controllerName` + + * `methodName`; resolve the controller (qualified-first) then the method in + * the controller's own file (mirrors `processRoutesFromExtracted`). + * - Decorator routes (`ExtractedDecoratorRoute`, e.g. Spring/FastAPI) carry + * `handlerName` (the decorated method, captured at extraction); resolve it + * directly in the route's own file. + * + * First-writer-wins per URL, matching the routes phase's dedup (it keeps the + * first route registered for a URL and counts the rest as duplicates). The first + * route to claim a URL reserves it **even when its handler is unresolvable**, so + * a later same-URL route can never stamp its handler onto the first route's Route + * node (the routes phase made that first route the node-winner). Routes whose + * handler cannot be *uniquely* resolved (no name, zero matches, or an ambiguous + * same-name match) carry no `handlerSymbolId`; the extractor then falls back to + * source scan for that route (fail-open, no regression, never a wrong handler). + */ +export function resolveRouteHandlerSymbols( + model: SemanticModel, + extractedRoutes: readonly ExtractedRoute[], + decoratorRoutes: readonly ExtractedDecoratorRoute[], +): Map { + const out = new Map(); + // URLs already claimed by an earlier route (resolved or not). Mirrors the + // routes phase `addRoute` first-writer-wins so the handler we stamp always + // belongs to the route that actually won the Route node. + const claimed = new Set(); + + // Resolve a single same-file symbol by name, refusing to guess on ambiguity: + // exactly one match → its nodeId; zero or many → undefined (fail-open). + const uniqueSymbolId = (filePath: string, name: string): string | undefined => { + const defs = model.symbols.lookupExactAll(filePath, name); + return defs.length === 1 ? defs[0]?.nodeId : undefined; + }; + + const claim = (routePath: string | null, prefix: string | null, symbolId: string | undefined) => { + if (!routePath) return; + const url = normalizeExtractedRoutePath(routePath, prefix); + if (claimed.has(url)) return; // first-writer-wins: later same-URL routes can't override + claimed.add(url); + if (symbolId) out.set(url, symbolId); + }; + + // Laravel framework routes — controller class + method name. + for (const route of extractedRoutes) { + let methodId: string | undefined; + if (route.controllerName && route.methodName) { + let controllerDef: SymbolDefinition | undefined; + if (route.controllerQualifiedName) { + controllerDef = resolveControllerByQualifiedName(model, route.controllerQualifiedName); + } + if (!controllerDef) { + const controllerDefs = model.types.lookupClassByName(route.controllerName); + if (controllerDefs.length === 1) controllerDef = controllerDefs[0]; + } + if (controllerDef) methodId = uniqueSymbolId(controllerDef.filePath, route.methodName); + } + claim(route.routePath, route.prefix ?? null, methodId); + } + + // Decorator routes (Spring / FastAPI / generic) — the decorated handler in + // the route's own file. + for (const dr of decoratorRoutes) { + const handlerId = dr.handlerName ? uniqueSymbolId(dr.filePath, dr.handlerName) : undefined; + claim(dr.routePath, dr.prefix ?? null, handlerId); + } + + return out; +} + /** Common method names on response/data objects that are NOT property accesses */ // Properties/methods to ignore when extracting consumer accessed keys from `data.X` patterns. // Avoids false positives from Fetch API, Array, Object, Promise, and DOM access on variables diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts index c652e255f..0c8f5cdac 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse-impl.ts @@ -40,6 +40,7 @@ import { DEFAULT_PDG_MAX_FUNCTION_LINES } from '../cfg/collect.js'; import type { WorkerExtractedData } from '../parsing-processor.js'; import { processRoutesFromExtracted, + resolveRouteHandlerSymbols, buildExportedTypeMapFromGraph, type ExportedTypeMap, } from '../call-processor.js'; @@ -370,6 +371,10 @@ export async function runChunkedParseAndResolve( allToolDefs: ExtractedToolDef[]; allORMQueries: ExtractedORMQuery[]; bindingAccumulator: BindingAccumulator; + /** Route URL → resolved handler symbol UID (Part 2, #2138). Lets the routes + * phase stamp `handlerSymbolId` on Route nodes so contract extraction can + * read the handler from the graph instead of re-parsing source. */ + routeHandlerSymbols: ReadonlyMap; /** SemanticModel populated during parse — scope-resolution reads its * TypeRegistry / MethodRegistry / SymbolTable indexes. */ model: MutableSemanticModel; @@ -1282,6 +1287,13 @@ export async function runChunkedParseAndResolve( 'parse-impl-return', `exportedTypeMap=${exportedTypeMap.size} parsedFiles=${allParsedFiles.length} nodes=${graph.nodeCount}`, ); + // Part 2 (#2138): resolve each route's handler to a real symbol UID now that + // the model is fully populated and decorator-route prefixes are finalized. + const routeHandlerSymbols = resolveRouteHandlerSymbols( + model, + allExtractedRoutes, + allDecoratorRoutes, + ); return { exportedTypeMap, allFetchCalls, @@ -1291,6 +1303,7 @@ export async function runChunkedParseAndResolve( allToolDefs, allORMQueries, bindingAccumulator, + routeHandlerSymbols, model, // Whether a worker pool was actually constructed for this run. False means // no pool was needed: a warm all-cache-hit run replays cached worker output diff --git a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts index 1875d9911..08da0068a 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/parse.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/parse.ts @@ -51,6 +51,9 @@ export interface ParseOutput { readonly allDecoratorRoutes: readonly ExtractedDecoratorRoute[]; readonly allToolDefs: readonly ExtractedToolDef[]; readonly allORMQueries: readonly ExtractedORMQuery[]; + /** Route URL → resolved handler symbol UID (Part 2, #2138). Consumed by the + * routes phase to stamp `handlerSymbolId` on Route nodes. */ + readonly routeHandlerSymbols: ReadonlyMap; bindingAccumulator: BindingAccumulator; /** SemanticModel populated during parse — scope-resolution reads its * TypeRegistry / MethodRegistry / SymbolTable indexes. */ diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index 4c45e6232..aea78ecd3 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -29,6 +29,7 @@ import { compiledMatcherMatchesRoute, } from '../route-extractors/middleware.js'; import { processNextjsFetchRoutes } from '../call-processor.js'; +import { normalizeExtractedRoutePath } from '../route-extractors/route-path.js'; import { generateId } from '../../../lib/utils.js'; import { readFileContents } from '../filesystem-walker.js'; import { isDev } from '../utils/env.js'; @@ -133,17 +134,13 @@ export function extractTemplateStaticFetchCalls( return calls; } -export function normalizeExtractedRoutePath(routePath: string, prefix: string | null): string { - const pathPart = routePath.trim().replace(/^\/+/, '').replace(/\/+$/g, ''); - const prefixPart = prefix?.trim().replace(/^\/+/, '').replace(/\/+$/g, ''); - const joined = prefixPart ? `/${prefixPart}${pathPart ? `/${pathPart}` : ''}` : `/${pathPart}`; - return joined.replace(/\/+/g, '/') || '/'; -} - function escapeRegex(s: string): string { return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } +// Re-exported for existing consumers/tests that import it from the routes phase. +export { normalizeExtractedRoutePath }; + /** * Canonicalize a route's HTTP verb for persistence on the Route node. * Returns an upper-cased standard method, or `undefined` when the value @@ -189,6 +186,7 @@ export const routesPhase: PipelinePhase = { allFetchWrapperDefs, allExtractedRoutes, allDecoratorRoutes, + routeHandlerSymbols, } = getPhaseOutput(deps, 'parse'); // Local copy — routes phase must not mutate upstream ParseOutput @@ -287,6 +285,7 @@ export const routesPhase: PipelinePhase = { const middleware = mwResult?.chain; const routeNodeId = generateId('Route', routeURL); + const handlerSymbolId = routeHandlerSymbols.get(routeURL); ctx.graph.addNode({ id: routeNodeId, label: 'Route', @@ -294,6 +293,7 @@ export const routesPhase: PipelinePhase = { name: routeURL, filePath: handlerPath, ...(routeMethod ? { method: routeMethod } : {}), + ...(handlerSymbolId ? { handlerSymbolId } : {}), ...(responseKeys ? { responseKeys } : {}), ...(errorKeys ? { errorKeys } : {}), ...(middleware && middleware.length > 0 ? { middleware } : {}), diff --git a/gitnexus/src/core/ingestion/route-extractors/route-path.ts b/gitnexus/src/core/ingestion/route-extractors/route-path.ts new file mode 100644 index 000000000..907574163 --- /dev/null +++ b/gitnexus/src/core/ingestion/route-extractors/route-path.ts @@ -0,0 +1,21 @@ +/** + * Shared route-path normalization. + * + * Extracted from the routes phase so both the routes phase (which creates the + * `Route` graph node, keyed by the normalized URL) and the parse phase (which + * resolves each route's handler symbol and needs the SAME key to associate the + * resolved id back to the route) can compute an identical route URL without a + * phase-to-phase import cycle. Pure string logic, no dependencies. + */ + +/** + * Join a route's path with its (optional) prefix into a normalized, + * leading-slash URL used as the Route node identity. Collapses duplicate + * slashes and strips trailing ones; an empty result degrades to `/`. + */ +export function normalizeExtractedRoutePath(routePath: string, prefix: string | null): string { + const pathPart = routePath.trim().replace(/^\/+/, '').replace(/\/+$/g, ''); + const prefixPart = prefix?.trim().replace(/^\/+/, '').replace(/\/+$/g, ''); + const joined = prefixPart ? `/${prefixPart}${pathPart ? `/${pathPart}` : ''}` : `/${pathPart}`; + return joined.replace(/\/+/g, '/') || '/'; +} diff --git a/gitnexus/src/core/ingestion/route-extractors/spring.ts b/gitnexus/src/core/ingestion/route-extractors/spring.ts index 4476e55cc..a92d09afe 100644 --- a/gitnexus/src/core/ingestion/route-extractors/spring.ts +++ b/gitnexus/src/core/ingestion/route-extractors/spring.ts @@ -139,6 +139,9 @@ export function extractSpringRoutes( if (routePath === null) continue; const enclosingClass = findEnclosingClass(node); const classPrefix = enclosingClass ? (prefixByClassId.get(enclosingClass.id) ?? '') : ''; + // `node` is the annotated `method_declaration`; its name field is the + // handler method name (resolved to a symbol UID later by the routes phase). + const handlerName = node.childForFieldName('name')?.text; routes.push({ filePath, @@ -147,6 +150,7 @@ export function extractSpringRoutes( decoratorName: ann, lineNumber: annNode.startPosition.row + lineOffset, ...(classPrefix ? { prefix: classPrefix } : {}), + ...(handlerName ? { handlerName } : {}), }); } diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 436833a5a..ee5c59a9f 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -317,6 +317,16 @@ export interface ExtractedDecoratorRoute { * absent ⇒ no prefix applies. */ prefix?: string | null; + /** + * Name of the handler the route decorator sits on (the decorated + * method/function — e.g. `create` for `@PostMapping("/orders") Order create()`). + * Captured at extraction where the decorated definition node is in hand, so + * the routes phase can resolve it to a real handler symbol UID via the + * SemanticModel (same `(filePath, name) → nodeId` lookup Laravel routes use). + * Absent when the extractor could not identify the decorated definition; + * resolution then falls back (the Route node simply carries no handlerSymbolId). + */ + handlerName?: string; } export interface ExtractedToolDef { diff --git a/gitnexus/src/core/lbug/csv-generator.ts b/gitnexus/src/core/lbug/csv-generator.ts index a03a81ada..04bf0fd5d 100644 --- a/gitnexus/src/core/lbug/csv-generator.ts +++ b/gitnexus/src/core/lbug/csv-generator.ts @@ -381,7 +381,7 @@ export const streamAllCSVsToDisk = async ( // Route nodes for API endpoint mapping const routeWriter = new BufferedCSVWriter( path.join(csvDir, 'route.csv'), - 'id,name,filePath,responseKeys,errorKeys,middleware,method', + 'id,name,filePath,responseKeys,errorKeys,middleware,method,handlerSymbolId', ); // Tool nodes for MCP tool definitions @@ -561,6 +561,7 @@ export const streamAllCSVsToDisk = async ( escapeCSVField(errorKeysStr), escapeCSVField(middlewareStr), escapeCSVField(String(node.properties.method ?? '')), + escapeCSVField(String(node.properties.handlerSymbolId ?? '')), ].join(','), ); break; diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index 903493f8f..698c98fdd 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -1337,7 +1337,7 @@ export const getCopyQuery = (table: NodeTableName, filePath: string): string => return `COPY ${t}(id, name, filePath, startLine, endLine, level, content, description) FROM "${filePath}" ${COPY_CSV_OPTS}`; } if (table === 'Route') { - return `COPY ${t}(id, name, filePath, responseKeys, errorKeys, middleware, method) FROM "${filePath}" ${COPY_CSV_OPTS}`; + return `COPY ${t}(id, name, filePath, responseKeys, errorKeys, middleware, method, handlerSymbolId) FROM "${filePath}" ${COPY_CSV_OPTS}`; } if (table === 'Tool') { return `COPY ${t}(id, name, filePath, description) FROM "${filePath}" ${COPY_CSV_OPTS}`; diff --git a/gitnexus/src/core/lbug/schema.ts b/gitnexus/src/core/lbug/schema.ts index 7fd19f5bf..d04ca42b4 100644 --- a/gitnexus/src/core/lbug/schema.ts +++ b/gitnexus/src/core/lbug/schema.ts @@ -195,6 +195,7 @@ CREATE NODE TABLE Route ( errorKeys STRING[], middleware STRING[], method STRING, + handlerSymbolId STRING, PRIMARY KEY (id) )`; diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index c460a480d..cf1263831 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -55,7 +55,7 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // the main thread (the #1983 OOM). Because the two stores share this version, // any future change to the `ParsedFile` serialization shape MUST bump // SCHEMA_BUMP so both invalidate in lockstep. -const SCHEMA_BUMP = 6; // #2082 M2: cfgSideChannel gained bindings + per-block statement facts +const SCHEMA_BUMP = 7; // #2138 Part 2: ExtractedDecoratorRoute gained `handlerName` (route handler symbol resolution) const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/integration/route-handler-symbol-roundtrip.test.ts b/gitnexus/test/integration/route-handler-symbol-roundtrip.test.ts new file mode 100644 index 000000000..e633841c9 --- /dev/null +++ b/gitnexus/test/integration/route-handler-symbol-roundtrip.test.ts @@ -0,0 +1,77 @@ +/** + * Real-LadybugDB round trip for `Route.handlerSymbolId` (issue #2138, Part 2). + * + * The Part-1 analogue (`route-method-roundtrip.test.ts`) pins `Route.method`; + * this pins the second persisted Route column added in Part 2. It persists a + * `Route` node carrying `handlerSymbolId` through the real CSV generator + the + * production `COPY` path into a real LadybugDB, then runs the exact production + * `HANDLES_ROUTE_QUERY` and asserts the handler UID round-trips. + * + * Covers the three persistence points touched by Part 2's U2: + * - `ROUTE_SCHEMA` (schema.ts) — the `handlerSymbolId` column must exist + * - the Route CSV row (csv-generator.ts) — the value must be written + * - `getCopyQuery('Route')` (lbug-adapter.ts) — the COPY must load it + */ +import { it, expect } from 'vitest'; +import path from 'path'; +import fs from 'fs/promises'; +import { withTestLbugDB } from '../helpers/test-indexed-db.js'; +import { buildTestGraph } from '../helpers/test-graph.js'; +import { streamAllCSVsToDisk } from '../../src/core/lbug/csv-generator.js'; +import { HANDLES_ROUTE_QUERY } from '../../src/core/group/extractors/http-route-extractor.js'; + +const HANDLER_UID = 'Method:OrderController.java:create'; + +withTestLbugDB('route-handler-symbol-roundtrip', (handle) => { + it('persists Route.handlerSymbolId through CSV→COPY and HANDLES_ROUTE_QUERY returns it', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // 1. Route node carrying a resolved handlerSymbolId (what the routes phase + // now stamps when resolveRouteHandlerSymbols resolves the handler). + const graph = buildTestGraph([ + { + id: 'Route:/api/orders', + label: 'Route', + name: '/api/orders', + filePath: 'OrderController.java', + extra: { + method: 'POST', + handlerSymbolId: HANDLER_UID, + responseKeys: [], + errorKeys: [], + middleware: [], + }, + }, + ]); + + // 2. Generate CSVs through the real generator. + const csvDir = path.join(handle.tmpHandle.dbPath, 'csv-handler-roundtrip'); + const repoDir = path.join(handle.tmpHandle.dbPath, 'repo-handler-roundtrip'); + await fs.mkdir(repoDir, { recursive: true }); + await streamAllCSVsToDisk(graph, repoDir, csvDir); + + // Sanity: route.csv header + row include the handlerSymbolId column/value. + const routeCsv = await fs.readFile(path.join(csvDir, 'route.csv'), 'utf-8'); + expect(routeCsv.split('\n')[0]).toContain('handlerSymbolId'); + expect(routeCsv).toContain(HANDLER_UID); + + // 3. COPY the Route node via the production COPY query. + const routeCsvPath = path.join(csvDir, 'route.csv').replace(/\\/g, '/'); + await adapter.executeQuery(adapter.getCopyQuery('Route', routeCsvPath)); + + // 4. Seed the handler File node + HANDLES_ROUTE edge. + await adapter.executeQuery( + `CREATE (:File {id: 'File:OrderController.java', name: 'OrderController.java', filePath: 'OrderController.java'})`, + ); + await adapter.executeQuery( + `MATCH (f:File {id: 'File:OrderController.java'}), (r:Route {id: 'Route:/api/orders'}) + CREATE (f)-[:CodeRelation {type: 'HANDLES_ROUTE', confidence: 1.0, reason: 'framework-route', step: 0}]->(r)`, + ); + + // 5. Run the EXACT production query and assert the handler UID round-trips. + const rows = (await adapter.executeQuery(HANDLES_ROUTE_QUERY)) as Record[]; + const row = rows.find((r) => String(r.routePath) === '/api/orders'); + expect(row, 'HANDLES_ROUTE_QUERY returned no row for the seeded route').toBeTruthy(); + expect(row!.handlerSymbolId).toBe(HANDLER_UID); + }); +}); diff --git a/gitnexus/test/integration/route-parse-skip.test.ts b/gitnexus/test/integration/route-parse-skip.test.ts new file mode 100644 index 000000000..18845585b --- /dev/null +++ b/gitnexus/test/integration/route-parse-skip.test.ts @@ -0,0 +1,251 @@ +/** + * #2138 Part 2 · parse-skip proof + P1 regression guards. + * + * The win: for a file whose provider routes are fully covered by the graph in a + * `routeCoverage: 'complete'` language, `HttpRouteExtractor` skips the source + * scan AND the tree-sitter parse — the graph is authoritative. We spy the real + * `parseSourceSafe` to COUNT parses (deterministic, not wall-time). + * + * PHP/Laravel is the language used for the *win* scenarios: ingestion's Laravel + * route extraction is a superset of the group PHP scan, so PHP is `'complete'`. + * + * Java is deliberately `'partial'` (the graph provider set is a strict subset of + * the group Java scan — array-form, interface-inherited, and same-URL multi-verb + * routes have no graph Route node). The Java cases below are REGRESSION GUARDS: + * they prove those group-only routes survive because Java is never parse-skipped. + * If someone flips Java to `'complete'` without making ingestion provider- + * complete, these tests fail — exactly the #2138 P1 data-loss class. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +// Count real parses by wrapping the actual parseSourceSafe. +const parseCalls: string[] = []; +vi.mock('../../src/core/tree-sitter/safe-parse.js', async (importActual) => { + const actual = await importActual(); + return { + ...actual, + parseSourceSafe: (parser: unknown, src: unknown) => { + parseCalls.push(typeof src === 'string' ? src : ''); + return (actual.parseSourceSafe as (p: unknown, s: unknown) => unknown)(parser, src); + }, + }; +}); + +import { HttpRouteExtractor } from '../../src/core/group/extractors/http-route-extractor.js'; + +const repo = { name: 'r', url: 'r' } as never; + +beforeEach(() => { + parseCalls.length = 0; +}); + +function mkRepo(files: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'route-parse-skip-')); + for (const [name, content] of Object.entries(files)) { + fs.writeFileSync(path.join(dir, name), content); + } + return dir; +} + +/** HANDLES_ROUTE rows from a compact spec; CONTAINS/FETCHES return empty. */ +function makeDb( + rows: Array<{ file: string; routePath: string; method: string; resolved: boolean }>, +) { + return vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return rows.map((r, i) => ({ + fileId: `File:${r.file}`, + filePath: r.file, + routePath: r.routePath, + routeMethod: r.method, + handlerSymbolId: r.resolved ? `Method:${r.file}:h${i}` : '', + routeSource: 'framework-route', + })); + } + return []; // CONTAINS (basename fallback is fine) + FETCHES (no consumers) + }); +} + +const providerPaths = (out: Awaited>) => + out.filter((c) => c.role === 'provider').map((c) => `${c.meta.method}::${c.meta.path}`); + +// ── PHP / Laravel — the `'complete'` language where the skip engages ────────── + +const ROUTES_A = ` { + it('baseline: with no graph, every PHP file is parsed', async () => { + const dir = mkRepo({ 'routes_a.php': ROUTES_A, 'routes_b.php': ROUTES_B }); + try { + const out = await new HttpRouteExtractor().extract(null, dir, repo); + expect(providerPaths(out)).toEqual( + expect.arrayContaining(['GET::/api/a/list', 'POST::/api/b/make']), + ); + expect(parseCalls.length).toBeGreaterThanOrEqual(2); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('fully covered: zero PHP files parsed (the win)', async () => { + const dir = mkRepo({ 'routes_a.php': ROUTES_A, 'routes_b.php': ROUTES_B }); + try { + const out = await new HttpRouteExtractor().extract( + makeDb([ + { file: 'routes_a.php', routePath: '/api/a/list', method: 'GET', resolved: true }, + { file: 'routes_b.php', routePath: '/api/b/make', method: 'POST', resolved: true }, + ]), + dir, + repo, + ); + const providers = out.filter((c) => c.role === 'provider'); + expect(providers.map((c) => c.meta.path)).toEqual( + expect.arrayContaining(['/api/a/list', '/api/b/make']), + ); + expect(providers.every((c) => c.meta.extractionStrategy === 'graph_assisted')).toBe(true); + expect(parseCalls.length).toBe(0); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('mixed: an unresolved route falls back to a scan; the resolved file stays skipped', async () => { + const dir = mkRepo({ 'routes_a.php': ROUTES_A, 'routes_b.php': ROUTES_B }); + try { + await new HttpRouteExtractor().extract( + makeDb([ + { file: 'routes_a.php', routePath: '/api/a/list', method: 'GET', resolved: true }, + { file: 'routes_b.php', routePath: '/api/b/make', method: 'POST', resolved: false }, + ]), + dir, + repo, + ); + expect(parseCalls.some((s) => s.includes('/api/b/make'))).toBe(true); // B scanned + expect(parseCalls.some((s) => s.includes('/api/a/list'))).toBe(false); // A skipped + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('provider-covered file that ALSO calls out is still parsed (consumer not dropped)', async () => { + // routes_c.php is a Laravel provider AND a Laravel Http:: consumer. + const ROUTES_C = ` c.role === 'provider' && c.meta.path === '/api/c/list')).toBe(true); + // The Http:: consumer lives only in source — it MUST survive because the + // consumer signal kept the file in the scan set (so it was parsed). + expect(parseCalls.some((s) => s.includes('/api/inventory'))).toBe(true); + expect(out.some((c) => c.role === 'consumer' && c.meta.path === '/api/inventory')).toBe(true); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +// ── Java — `'partial'`, so the P1 group-only shapes must never be dropped ───── + +describe('HttpRouteExtractor — Java parse-skip P1 regression guards (#2138)', () => { + it('array-form @GetMapping({"/a","/b"}) survives a co-located resolved route', async () => { + const AC = `package com.example; +import org.springframework.web.bind.annotation.*; +@RestController +public class AController { + @GetMapping("/covered") public Object covered() { return null; } + @GetMapping({"/a","/b"}) public Object multi() { return null; } +} +`; + const dir = mkRepo({ 'AController.java': AC }); + try { + // Graph resolves only /covered (ingestion has no array-form Route node). + const out = await new HttpRouteExtractor().extract( + makeDb([ + { file: 'AController.java', routePath: '/covered', method: 'GET', resolved: true }, + ]), + dir, + repo, + ); + const paths = providerPaths(out); + // The array-form routes are graph-only-absent but survive via source scan. + expect(paths).toEqual(expect.arrayContaining(['GET::/a', 'GET::/b'])); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('same-URL multi-verb (GET+POST /orders) keeps both verbs', async () => { + const OC = `package com.example; +import org.springframework.web.bind.annotation.*; +@RestController +public class OrderController { + @GetMapping("/orders") public Object list() { return null; } + @PostMapping("/orders") public Object make() { return null; } +} +`; + const dir = mkRepo({ 'OrderController.java': OC }); + try { + // Ingestion's URL-keyed Route node collapses to one verb; resolve only GET. + const out = await new HttpRouteExtractor().extract( + makeDb([ + { file: 'OrderController.java', routePath: '/orders', method: 'GET', resolved: true }, + ]), + dir, + repo, + ); + const paths = providerPaths(out); + expect(paths).toEqual(expect.arrayContaining(['GET::/orders', 'POST::/orders'])); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('interface-inherited Spring route survives on the implementing controller', async () => { + const IFACE = `package com.example; +import org.springframework.web.bind.annotation.*; +@RequestMapping("/orders") +public interface OrderApi { + @GetMapping("/{id}") Object get(Long id); +} +`; + const CTRL = `package com.example; +import org.springframework.web.bind.annotation.*; +@RestController +public class OrderController implements OrderApi { + @GetMapping("/direct") public Object direct() { return null; } + public Object get(Long id) { return null; } +} +`; + const dir = mkRepo({ 'OrderApi.java': IFACE, 'OrderController.java': CTRL }); + try { + // Graph resolves only the controller-direct route; the inherited route is + // composed only by the group scanProject pass. + const out = await new HttpRouteExtractor().extract( + makeDb([ + { file: 'OrderController.java', routePath: '/direct', method: 'GET', resolved: true }, + ]), + dir, + repo, + ); + const paths = providerPaths(out); + expect(paths).toEqual(expect.arrayContaining(['GET::/orders/{param}'])); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/integration/spring-route-pipeline.test.ts b/gitnexus/test/integration/spring-route-pipeline.test.ts index 27d721c52..018ead80f 100644 --- a/gitnexus/test/integration/spring-route-pipeline.test.ts +++ b/gitnexus/test/integration/spring-route-pipeline.test.ts @@ -104,4 +104,27 @@ describe('Spring @RequestMapping route ingestion pipeline', () => { const userRoutes = handlesRouteEdges.filter((e) => e.filePath.includes('UserController.java')); expect(userRoutes.length).toBeGreaterThanOrEqual(1); }); + + it('resolves the decorated handler method to a symbol UID on the Route node (Part 2 #2138)', () => { + // Find the Route node for the @GetMapping("/list") handler. + let routeNode: { properties: Record } | undefined; + result.graph.forEachNode((n) => { + if (n.label === 'Route' && n.properties.name === '/api/users/list') { + routeNode = n; + } + }); + expect(routeNode, 'Route node /api/users/list should exist').toBeTruthy(); + + // U0+U1: the decorated handler (listUsers) was captured and resolved to a + // real symbol UID, stamped on the Route node. + const handlerSymbolId = routeNode!.properties.handlerSymbolId; + expect(handlerSymbolId, 'Route node should carry handlerSymbolId').toBeTruthy(); + + // The id resolves to the listUsers handler symbol in UserController.java. + const handler = result.graph.getNode(String(handlerSymbolId)); + expect(handler, 'handlerSymbolId should resolve to a graph node').toBeTruthy(); + expect(handler!.properties.name).toBe('listUsers'); + expect(String(handler!.properties.filePath)).toContain('UserController.java'); + expect(['Method', 'Function']).toContain(handler!.label); + }); }); diff --git a/gitnexus/test/unit/blade-template-routes.test.ts b/gitnexus/test/unit/blade-template-routes.test.ts index 305b1e01d..231248184 100644 --- a/gitnexus/test/unit/blade-template-routes.test.ts +++ b/gitnexus/test/unit/blade-template-routes.test.ts @@ -131,6 +131,7 @@ describe('Blade/template static route extraction', () => { }, ], allDecoratorRoutes: [], + routeHandlerSymbols: new Map(), } as unknown as ParseOutput; const output = await routesPhase.execute( diff --git a/gitnexus/test/unit/group/http-consumer-signals.test.ts b/gitnexus/test/unit/group/http-consumer-signals.test.ts new file mode 100644 index 000000000..087524cfe --- /dev/null +++ b/gitnexus/test/unit/group/http-consumer-signals.test.ts @@ -0,0 +1,76 @@ +/** + * Unit tests for `HttpLanguagePlugin.hasConsumerSignals` (#2138 Part 2). + * + * The parse-skip consumer-safety gate skips a provider-covered file only when + * its plugin proves (parse-free) the file has no outbound-HTTP call its `scan()` + * would detect. The contract (`types.ts`) requires `hasConsumerSignals` to be a + * SUPERSET of every consumer shape `scan()` emits — otherwise a covered file + * with an undetected consumer call would be wrongly parse-skipped and its + * consumer contract dropped. These tests pin that superset relationship per + * language with the exact idioms each `scan()` matches. + */ +import { describe, it, expect } from 'vitest'; +import { JAVA_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/java.js'; +import { PHP_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/php.js'; +import { PYTHON_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/python.js'; + +const has = (plugin: { hasConsumerSignals?: (s: string) => boolean }, src: string): boolean => { + if (!plugin.hasConsumerSignals) throw new Error('plugin has no hasConsumerSignals'); + return plugin.hasConsumerSignals(src); +}; + +describe('Java hasConsumerSignals — superset of scan() consumer idioms', () => { + it.each([ + ['RestTemplate', 'restTemplate.getForObject("/api/x", X.class);'], + ['WebClient short-form', 'webClient.get().uri("/api/x").retrieve();'], + ['WebClient exchange', 'webClient.method(HttpMethod.GET).uri("/x");'], + ['OkHttp', 'new Request.Builder().url("/api/x").build();'], + ['Java HttpClient', 'HttpRequest.newBuilder().uri(URI.create("/x")).GET();'], + ['Apache HttpGet', 'new HttpGet("/api/x");'], + ['OpenFeign @FeignClient', '@FeignClient(name="svc") interface C {}'], + ['OpenFeign @RequestLine', '@RequestLine("GET /users/{id}")'], + ['Spring HTTP Interface @GetExchange', '@GetExchange("/api/x") Object x();'], + ])('detects %s', (_label, src) => { + expect(has(JAVA_HTTP_PLUGIN, src)).toBe(true); + }); + + it('returns false for a pure provider controller (no outbound calls)', () => { + const src = `@RestController @RequestMapping("/api/a") +class AController { @GetMapping("/list") Object list() { return null; } }`; + expect(has(JAVA_HTTP_PLUGIN, src)).toBe(false); + }); +}); + +describe('PHP hasConsumerSignals — superset of scan() consumer idioms', () => { + it.each([ + ['Laravel Http facade', "Http::get('/api/x');"], + ['Guzzle member call', "$client->post('/api/x', []);"], + ['file_get_contents', "file_get_contents('https://x/api');"], + ])('detects %s', (_label, src) => { + expect(has(PHP_HTTP_PLUGIN, src)).toBe(true); + }); + + it('returns false for a pure Laravel route file (provider only)', () => { + expect(has(PHP_HTTP_PLUGIN, "Route::get('/api/a/list', 'AController@list');")).toBe(false); + }); +}); + +describe('Python hasConsumerSignals — superset of scan() consumer idioms', () => { + it.each([ + ['requests verb', 'requests.get("/api/x")'], + ['requests.request', 'requests.request("GET", "/api/x")'], + ['httpx', 'client = httpx.AsyncClient()'], + ['aiohttp', 'async with aiohttp.ClientSession() as s: ...'], + ['urllib', 'urllib.request.urlopen("/api/x")'], + ['uri= keyword', 'do_call(uri="/api/x")'], + ['url= keyword', 'do_call(url="/api/x")'], + ])('detects %s', (_label, src) => { + expect(has(PYTHON_HTTP_PLUGIN, src)).toBe(true); + }); + + it('returns false for a pure FastAPI provider (decorator route only)', () => { + const src = `@router.get("/api/x") +async def handler(): return {}`; + expect(has(PYTHON_HTTP_PLUGIN, src)).toBe(false); + }); +}); 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 0162a3678..d392bce9c 100644 --- a/gitnexus/test/unit/group/http-route-graph-method.test.ts +++ b/gitnexus/test/unit/group/http-route-graph-method.test.ts @@ -200,6 +200,53 @@ describe('HttpRouteExtractor — Route.method from graph (Step A / #2138)', () = expect(out[0].symbolName).toBe('listOrders'); }); + it('fast path: Route.handlerSymbolId resolves the handler without any source scan', async () => { + // Deliberately leave FILE_DETECTIONS empty: if the extractor still resolves + // the handler, it MUST have used the persisted handlerSymbolId (the graph + // fast path), not a plugin scan of the source. + const HID = 'Method:OrderController.java:OrderController.createOrder#0'; + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'OrderController.java', + routePath: '/api/orders', + routeMethod: 'POST', + handlerSymbolId: HID, + routeSource: 'framework-route', + }, + ]; + } + if (query.includes('CONTAINS')) { + return [ + { + uid: HID, + name: 'createOrder', + filePath: 'OrderController.java', + labels: ['Method'], + 0: HID, + 1: 'createOrder', + 2: 'OrderController.java', + 3: ['Method'], + }, + ]; + } + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out).toHaveLength(1); + expect(out[0].meta.method).toBe('POST'); + // The persisted symbol id is authoritative; name/path come from the cheap + // CONTAINS graph query (no source parse). + expect(out[0].symbolUid).toBe(HID); + expect(out[0].symbolName).toBe('createOrder'); + }); + 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/resolve-route-handler-symbols.test.ts b/gitnexus/test/unit/resolve-route-handler-symbols.test.ts new file mode 100644 index 000000000..4a214991b --- /dev/null +++ b/gitnexus/test/unit/resolve-route-handler-symbols.test.ts @@ -0,0 +1,148 @@ +/** + * Direct unit tests for `resolveRouteHandlerSymbols` (#2138 Part 2). + * + * Pins the P2 fixes from the review: + * - ambiguity → fail-open: a same-name lookup returning ≠1 yields NO + * handlerSymbolId (never an arbitrary `[0]` guess). + * - first-writer-wins reservation: the first route to claim a URL reserves it + * even when its handler is unresolvable, so a later same-URL route can't + * stamp its handler onto the (node-winning) first route's slot. + * - happy path: a uniquely-resolvable handler is stamped, keyed by the + * normalized URL. + */ +import { describe, it, expect } from 'vitest'; +import { createSemanticModel } from '../../src/core/ingestion/model/index.js'; +import { resolveRouteHandlerSymbols } from '../../src/core/ingestion/call-processor.js'; +import type { ExtractedDecoratorRoute } from '../../src/core/ingestion/workers/parse-worker.js'; +import type { ExtractedRoute } from '../../src/core/ingestion/route-extractors/laravel.js'; + +const FILE = 'src/OrderController.java'; + +function decoratorRoute(overrides: Partial = {}): ExtractedDecoratorRoute { + return { + filePath: FILE, + routePath: '/orders', + httpMethod: 'GET', + decoratorName: 'GetMapping', + lineNumber: 1, + handlerName: 'list', + ...overrides, + }; +} + +describe('resolveRouteHandlerSymbols — decorator routes', () => { + it('uniquely-resolvable handler is stamped, keyed by normalized URL', () => { + const model = createSemanticModel(); + model.symbols.add(FILE, 'list', 'method:OrderController.list', 'Method'); + + const out = resolveRouteHandlerSymbols(model, [], [decoratorRoute()]); + + expect(out.get('/orders')).toBe('method:OrderController.list'); + }); + + it('ambiguous same-name handler (overloads) → fail-open, no stamp', () => { + const model = createSemanticModel(); + // Two same-(file,name) defs → lookupExactAll returns 2 → refuse to guess. + model.symbols.add(FILE, 'list', 'method:OrderController.list#1', 'Method'); + model.symbols.add(FILE, 'list', 'method:OrderController.list#2', 'Method'); + + const out = resolveRouteHandlerSymbols(model, [], [decoratorRoute()]); + + expect(out.has('/orders')).toBe(false); + }); + + it('unknown handler name → fail-open, no stamp', () => { + const model = createSemanticModel(); // nothing registered + + const out = resolveRouteHandlerSymbols(model, [], [decoratorRoute({ handlerName: 'ghost' })]); + + expect(out.has('/orders')).toBe(false); + }); + + it('same-URL collision: an unresolvable first route reserves the slot so a later resolvable route cannot stamp it', () => { + const model = createSemanticModel(); + // Only the SECOND route's handler exists in the model. + model.symbols.add(FILE, 'second', 'method:OrderController.second', 'Method'); + + const out = resolveRouteHandlerSymbols( + model, + [], + [ + // First route at /orders is unresolvable (no such symbol) — but it is the + // route the routes phase makes the Route-node winner, so its slot must be + // reserved (empty), NOT filled by the later same-URL route. + decoratorRoute({ handlerName: 'first_missing' }), + decoratorRoute({ handlerName: 'second' }), + ], + ); + + // Reservation holds: the URL carries no (wrong) handler. Pre-fix this would + // have stamped `method:OrderController.second` onto the first route's node. + expect(out.has('/orders')).toBe(false); + }); + + it('first-writer-wins among resolvable same-URL routes', () => { + const model = createSemanticModel(); + model.symbols.add(FILE, 'winner', 'method:OrderController.winner', 'Method'); + model.symbols.add(FILE, 'loser', 'method:OrderController.loser', 'Method'); + + const out = resolveRouteHandlerSymbols( + model, + [], + [decoratorRoute({ handlerName: 'winner' }), decoratorRoute({ handlerName: 'loser' })], + ); + + expect(out.get('/orders')).toBe('method:OrderController.winner'); + }); +}); + +describe('resolveRouteHandlerSymbols — Laravel framework routes', () => { + const CTRL = 'app/Http/Controllers/OrderController.php'; + + function laravelRoute(overrides: Partial = {}): ExtractedRoute { + return { + filePath: 'routes/web.php', + httpMethod: 'get', + routePath: '/orders', + routeName: null, + controllerName: 'OrderController', + methodName: 'index', + middleware: [], + prefix: null, + lineNumber: 1, + ...overrides, + }; + } + + it('resolvable controller + unique method → stamped', () => { + const model = createSemanticModel(); + model.symbols.add(CTRL, 'OrderController', 'class:OrderController', 'Class'); + model.symbols.add(CTRL, 'index', 'method:OrderController.index', 'Method', { + ownerId: 'class:OrderController', + }); + + const out = resolveRouteHandlerSymbols(model, [laravelRoute()], []); + + expect(out.get('/orders')).toBe('method:OrderController.index'); + }); + + it('ambiguous controller short-name (>1) → fail-open, no stamp', () => { + const model = createSemanticModel(); + model.symbols.add( + 'app/A/OrderController.php', + 'OrderController', + 'class:A.OrderController', + 'Class', + ); + model.symbols.add( + 'app/B/OrderController.php', + 'OrderController', + 'class:B.OrderController', + 'Class', + ); + + const out = resolveRouteHandlerSymbols(model, [laravelRoute()], []); + + expect(out.has('/orders')).toBe(false); + }); +});