diff --git a/gitnexus/bench/emit-persistence/baselines.json b/gitnexus/bench/emit-persistence/baselines.json index 93806b252..0e7f22f34 100644 --- a/gitnexus/bench/emit-persistence/baselines.json +++ b/gitnexus/bench/emit-persistence/baselines.json @@ -1,5 +1,5 @@ { - "fingerprint": "1b9dd0b783899b47067c36511d241860f291ac736e57682b0ece14148e3958ff", + "fingerprint": "580d0585cf4188b4a2c1946716156d6a788de18b3b51fec62f6977e94d4a27b4", "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-route-extractor.ts b/gitnexus/src/core/group/extractors/http-route-extractor.ts index e14b9b701..450cf3841 100644 --- a/gitnexus/src/core/group/extractors/http-route-extractor.ts +++ b/gitnexus/src/core/group/extractors/http-route-extractor.ts @@ -6,6 +6,7 @@ import type { ContractExtractor, CypherExecutor } from '../contract-extractor.js import type { ExtractedContract, RepoHandle } from '../types.js'; import { readSafe } from './fs-utils.js'; import { parseSourceSafe } from '../../tree-sitter/safe-parse.js'; +import { logger } from '../../logger.js'; import { getPluginForFile, HTTP_SCAN_GLOB, @@ -40,13 +41,16 @@ import { // ─── Graph-assisted queries ────────────────────────────────────────── -const HANDLES_ROUTE_QUERY = ` +// Exported so integration tests can run the exact production query against a +// real LadybugDB (guards the Route.method column contract — see +// route-method-roundtrip.test.ts). +export const HANDLES_ROUTE_QUERY = ` 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.responseKeys AS responseKeys, r.reason AS routeSource`; - const FETCHES_QUERY = ` MATCH (callerFile:File)-[r:CodeRelation {type: 'FETCHES'}]->(route:Route) RETURN callerFile.id AS fileId, callerFile.filePath AS filePath, @@ -324,7 +328,16 @@ export class HttpRouteExtractor implements ContractExtractor { let rows: Record[]; try { rows = await db(HANDLES_ROUTE_QUERY); - } catch { + } catch (err) { + // A failure here silently disables the entire graph-assisted HTTP + // provider path (the source-scan fallback still runs and masks most + // of the damage), so surface it at debug level to make a total + // outage observable instead of invisible. + logger.debug( + `[http-route-extractor] HANDLES_ROUTE query failed; graph providers skipped: ${ + err instanceof Error ? err.message : String(err) + }`, + ); return []; } @@ -332,7 +345,14 @@ export class HttpRouteExtractor implements ContractExtractor { const filePath = String(row.filePath ?? ''); const routePath = String(row.routePath ?? ''); const routeSource = String(row.routeSource ?? row.routeReason ?? ''); - let method = methodFromRouteReason(routeSource); + // Prefer the HTTP verb persisted on the Route node by the ingestion + // routes phase (Spring/Laravel framework routes and decorator routes + // carry it). Fall back to parsing it out of the edge reason for + // older indexes or filesystem routes that never stored a method. + const graphMethod = String(row.routeMethod ?? '') + .trim() + .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 @@ -458,7 +478,12 @@ export class HttpRouteExtractor implements ContractExtractor { let rows: Record[]; try { rows = await db(FETCHES_QUERY); - } catch { + } catch (err) { + logger.debug( + `[http-route-extractor] FETCHES query failed; graph consumers skipped: ${ + err instanceof Error ? err.message : String(err) + }`, + ); return []; } for (const row of rows) { diff --git a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts index 561590e05..e9909afbe 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/routes.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/routes.ts @@ -42,6 +42,15 @@ const EXPO_NAV_PATTERNS = [ export interface RouteEntry { filePath: string; source: string; + /** + * HTTP verb for this route when ingestion knows it structurally + * (Spring/Laravel framework routes and decorator routes carry + * `httpMethod`; filesystem-derived routes — Next.js/Expo/PHP file + * routes — do not, so this stays undefined for them). Persisted onto + * the Route node so downstream contract extraction can read the verb + * from the graph instead of re-parsing the handler source. + */ + method?: string; } export interface RoutesOutput { @@ -135,6 +144,33 @@ function escapeRegex(s: string): string { return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); } +/** + * Canonicalize a route's HTTP verb for persistence on the Route node. + * Returns an upper-cased standard method, or `undefined` when the value + * is not a real HTTP verb. Laravel `Route::resource` / `apiResource` + * surface `httpMethod` values like `resource` / `apiResource` (they + * expand to several verbs at runtime), so they must not be stored as a + * method — leaving them `undefined` keeps the column clean and lets the + * contract extractor fall back to its source-scan path for those routes. + */ +const VALID_HTTP_METHODS = new Set([ + 'GET', + 'POST', + 'PUT', + 'PATCH', + 'DELETE', + 'HEAD', + 'OPTIONS', + 'TRACE', + 'CONNECT', +]); + +export function normalizeRouteMethod(raw: string | null | undefined): string | undefined { + if (typeof raw !== 'string') return undefined; + const verb = raw.trim().toUpperCase(); + return VALID_HTTP_METHODS.has(verb) ? verb : undefined; +} + export const routesPhase: PipelinePhase = { name: 'routes', deps: ['parse'], @@ -213,6 +249,7 @@ export const routesPhase: PipelinePhase = { addRoute(routeUrl, { filePath: route.filePath, source: 'framework-route', + method: normalizeRouteMethod(route.httpMethod), }); if (route.routeName && !namedRouteRegistry.has(route.routeName)) { namedRouteRegistry.set(route.routeName, routeUrl); @@ -223,6 +260,7 @@ export const routesPhase: PipelinePhase = { addRoute(url, { filePath: dr.filePath, source: `decorator-${dr.decoratorName}`, + method: normalizeRouteMethod(dr.httpMethod), }); } @@ -232,7 +270,7 @@ export const routesPhase: PipelinePhase = { handlerContents = await readFileContents(ctx.repoPath, handlerPaths); for (const [routeURL, entry] of routeRegistry) { - const { filePath: handlerPath, source: routeSource } = entry; + const { filePath: handlerPath, source: routeSource, method: routeMethod } = entry; const content = handlerContents.get(handlerPath); const { responseKeys, errorKeys } = content @@ -251,6 +289,7 @@ export const routesPhase: PipelinePhase = { properties: { name: routeURL, filePath: handlerPath, + ...(routeMethod ? { method: routeMethod } : {}), ...(responseKeys ? { responseKeys } : {}), ...(errorKeys ? { errorKeys } : {}), ...(middleware && middleware.length > 0 ? { middleware } : {}), diff --git a/gitnexus/src/core/lbug/csv-generator.ts b/gitnexus/src/core/lbug/csv-generator.ts index 0dc6eba94..148375445 100644 --- a/gitnexus/src/core/lbug/csv-generator.ts +++ b/gitnexus/src/core/lbug/csv-generator.ts @@ -374,7 +374,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', + 'id,name,filePath,responseKeys,errorKeys,middleware,method', ); // Tool nodes for MCP tool definitions @@ -553,6 +553,7 @@ export const streamAllCSVsToDisk = async ( escapeCSVField(keysStr), escapeCSVField(errorKeysStr), escapeCSVField(middlewareStr), + escapeCSVField(String(node.properties.method ?? '')), ].join(','), ); break; diff --git a/gitnexus/src/core/lbug/lbug-adapter.ts b/gitnexus/src/core/lbug/lbug-adapter.ts index f5ff27281..124b91070 100644 --- a/gitnexus/src/core/lbug/lbug-adapter.ts +++ b/gitnexus/src/core/lbug/lbug-adapter.ts @@ -1305,7 +1305,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) FROM "${filePath}" ${COPY_CSV_OPTS}`; + return `COPY ${t}(id, name, filePath, responseKeys, errorKeys, middleware, method) 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 4c8cc7cc5..769ecc983 100644 --- a/gitnexus/src/core/lbug/schema.ts +++ b/gitnexus/src/core/lbug/schema.ts @@ -194,6 +194,7 @@ CREATE NODE TABLE Route ( responseKeys STRING[], errorKeys STRING[], middleware STRING[], + method STRING, PRIMARY KEY (id) )`; diff --git a/gitnexus/test/integration/route-method-roundtrip.test.ts b/gitnexus/test/integration/route-method-roundtrip.test.ts new file mode 100644 index 000000000..155bbb86b --- /dev/null +++ b/gitnexus/test/integration/route-method-roundtrip.test.ts @@ -0,0 +1,81 @@ +/** + * Real-LadybugDB round trip for `Route.method` (issue #2138, Part 1). + * + * This is the test the mocked `http-route-graph-method.test.ts` could NOT + * provide: it persists a `Route` node carrying `method` through the actual + * CSV generator + `COPY` path into a real LadybugDB, then runs the exact + * production `HANDLES_ROUTE_QUERY` and asserts the verb comes back. + * + * Before the schema/CSV/COPY columns were added, `HANDLES_ROUTE_QUERY`'s + * `route.method AS routeMethod` failed to bind against the real schema + * (`Binder exception: Cannot find property method for r.`) and the + * extractor's `catch { return [] }` silently swallowed it — so this test + * would have failed (empty rows / throw), pinning the exact regression. + * + * Coverage spans all three persistence points touched by Part 1: + * - `ROUTE_SCHEMA` (schema.ts) — the `method` 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'; + +withTestLbugDB('route-method-roundtrip', (handle) => { + it('persists Route.method through CSV→COPY and HANDLES_ROUTE_QUERY returns it', async () => { + const adapter = await import('../../src/core/lbug/lbug-adapter.js'); + + // 1. Build a graph with a single framework Route node carrying `method`, + // mirroring what the routes phase now emits for a Spring controller. + const graph = buildTestGraph([ + { + id: 'Route:/api/orders', + label: 'Route', + name: '/api/orders', + filePath: 'OrderController.java', + extra: { + method: 'POST', + responseKeys: [], + errorKeys: [], + middleware: [], + }, + }, + ]); + + // 2. Generate CSVs through the real generator (exercises the new + // `method` column in the Route CSV row). + const csvDir = path.join(handle.tmpHandle.dbPath, 'csv-roundtrip'); + const repoDir = path.join(handle.tmpHandle.dbPath, 'repo-roundtrip'); + await fs.mkdir(repoDir, { recursive: true }); + await streamAllCSVsToDisk(graph, repoDir, csvDir); + + // Sanity: the generated route.csv header + row include the method column. + const routeCsv = await fs.readFile(path.join(csvDir, 'route.csv'), 'utf-8'); + expect(routeCsv.split('\n')[0]).toContain('method'); + expect(routeCsv).toContain('POST'); + + // 3. COPY the Route node into the real DB via the production COPY query + // (exercises the new `method` column in getCopyQuery('Route')). + 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 via Cypher. + 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 verb 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!.routeMethod).toBe('POST'); + }); +}); diff --git a/gitnexus/test/unit/blade-template-routes.test.ts b/gitnexus/test/unit/blade-template-routes.test.ts index 9edd6e791..305b1e01d 100644 --- a/gitnexus/test/unit/blade-template-routes.test.ts +++ b/gitnexus/test/unit/blade-template-routes.test.ts @@ -146,6 +146,7 @@ describe('Blade/template static route extraction', () => { expect(output.routeRegistry.get('/admin/orders')).toEqual({ filePath: 'routes/web.php', source: 'framework-route', + method: 'POST', }); const fetchEdges = graph.relationships.filter((rel) => rel.type === 'FETCHES'); diff --git a/gitnexus/test/unit/group/http-route-graph-method.test.ts b/gitnexus/test/unit/group/http-route-graph-method.test.ts new file mode 100644 index 000000000..0162a3678 --- /dev/null +++ b/gitnexus/test/unit/group/http-route-graph-method.test.ts @@ -0,0 +1,229 @@ +/** + * Step A coverage for issue #2138 groundwork: + * `HttpRouteExtractor.extractProvidersGraph` should read the HTTP verb + * persisted on the Route node (`route.method`, surfaced as `routeMethod` + * by HANDLES_ROUTE_QUERY) as the authoritative method, falling back to + * the edge `reason` only for older indexes / filesystem routes that never + * stored a method. + * + * Why this matters: framework routes (Java Spring, Laravel) are emitted + * with `routeSource = 'framework-route'`, which `methodFromRouteReason` + * cannot decode (returns null). Before the Route node carried `method`, + * the graph path had to re-parse the handler source to recover the verb. + * Persisting the verb on the node removes that dependency for the method + * piece (the handler-name piece is addressed separately in Step B). + * + * Harness mirrors http-route-multi-verb.test.ts: the plugin registry, + * fs-utils, and tree-sitter are mocked so we drive the graph rows + * directly without real grammars. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type Parser from 'tree-sitter'; +import type { HttpDetection } from '../../../src/core/group/extractors/http-patterns/types.js'; + +const FILE_DETECTIONS = new Map(); + +vi.mock('../../../src/core/group/extractors/fs-utils.js', () => ({ + readSafe: (_repo: string, _rel: string) => 'stub content', +})); + +vi.mock('../../../src/core/group/extractors/http-patterns/index.js', () => { + return { + HTTP_SCAN_GLOB: '**/*.fake', + getPluginForFile: (rel: string) => ({ + name: 'fake', + language: {}, + scan: (_tree: Parser.Tree) => FILE_DETECTIONS.get(rel) ?? [], + }), + }; +}); + +vi.mock('tree-sitter', () => { + class FakeParser { + setLanguage(_lang: unknown) {} + parse(_src: string) { + return {} as Parser.Tree; + } + } + return { default: FakeParser }; +}); + +import { HttpRouteExtractor } from '../../../src/core/group/extractors/http-route-extractor.js'; + +function detection( + role: 'provider' | 'consumer', + method: string, + p: string, + name: string | null, +): HttpDetection { + return { role, framework: 'test', method, path: p, name, confidence: 0.8 }; +} + +const containsFor = (names: string[]) => + names.map((name) => ({ + uid: `uid-${name}`, + name, + filePath: 'OrderController.java', + labels: ['Method'], + 0: `uid-${name}`, + 1: name, + 2: 'OrderController.java', + 3: ['Method'], + })); + +describe('HttpRouteExtractor — Route.method from graph (Step A / #2138)', () => { + beforeEach(() => { + FILE_DETECTIONS.clear(); + }); + + it('framework-route: uses Route.method when the edge reason cannot decode the verb', async () => { + // Spring controller: reason is the generic 'framework-route', so + // methodFromRouteReason() returns null. The verb must come from the + // Route node's persisted `method` (routeMethod). + FILE_DETECTIONS.set('OrderController.java', [ + detection('provider', 'POST', '/api/orders', 'createOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'OrderController.java', + routePath: '/api/orders', + routeId: 'r1', + routeMethod: 'POST', + routeSource: 'framework-route', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['createOrder']); + 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'); + expect(out[0].contractId).toBe('http::POST::/api/orders'); + }); + + it('framework-route: Route.method disambiguates the handler among multi-verb candidates', async () => { + // Two verbs at the same path in one controller; reason is generic. + // Route.method = PUT must both set the verb AND pick replaceOrder. + FILE_DETECTIONS.set('OrderController.java', [ + detection('provider', 'GET', '/api/orders', 'listOrders'), + detection('provider', 'PUT', '/api/orders', 'replaceOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'OrderController.java', + routePath: '/api/orders', + routeMethod: 'PUT', + routeSource: 'framework-route', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders', 'replaceOrder']); + 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('PUT'); + expect(out[0].symbolName).toBe('replaceOrder'); + }); + + it('case-insensitive: lower-case Route.method is normalized to an upper-case verb', async () => { + FILE_DETECTIONS.set('OrderController.java', [ + detection('provider', 'DELETE', '/api/orders/{param}', 'deleteOrder'), + ]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'OrderController.java', + routePath: '/api/orders/{id}', + routeMethod: 'delete', + routeSource: 'framework-route', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['deleteOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out[0].meta.method).toBe('DELETE'); + }); + + it('backward-compat: missing Route.method falls back to the edge reason (old indexes)', async () => { + // Old index has no `method` on the Route node → routeMethod undefined. + // The decorator reason still decodes the verb as before. + FILE_DETECTIONS.set('routes.ts', [detection('provider', 'GET', '/api/orders', 'listOrders')]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + // no routeMethod field at all + routeSource: 'decorator-Get', + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['listOrders']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + expect(out[0].meta.method).toBe('GET'); + expect(out[0].symbolName).toBe('listOrders'); + }); + + 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')]); + + const db = vi.fn(async (query: string) => { + if (query.includes('HANDLES_ROUTE')) { + return [ + { + fileId: 'f1', + filePath: 'routes.ts', + routePath: '/api/orders', + routeSource: 'framework-route', // undecodable, and no routeMethod + }, + ]; + } + if (query.includes('CONTAINS')) return containsFor(['createOrder']); + return []; + }); + + const out = await new HttpRouteExtractor().extract(db, '/repo', { + name: 'r', + url: 'r', + } as never); + // Single candidate, so its method is adopted (existing behavior); the + // point is that absence of routeMethod does not throw and still works. + expect(out[0].meta.method).toBe('POST'); + }); +});