mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-02 02:11:29 +00:00
* fix(routes): connect decorator routes to their handler function
A Route node's only relationship was HANDLES_ROUTE from its FILE. The graph knew
a route existed and which file declared it, but not which function implemented
it. Two consequences on a 12.4k-file repository with 162 FastAPI routes:
- Every decorated handler was indistinguishable from dead code. Its sole edge
was DEFINES, so a reachability query reported it unreferenced even though the
framework invokes it on every request.
- `route_map` / `api_impact` could only answer at file granularity, and
`processes.ts` routed every route through its `routesWithoutHandlerByFile`
fallback instead of keying by handler.
Two halves of one gap, both already designed for and neither wired:
1. `ExtractedDecoratorRoute.handlerName` is documented as "captured at extraction
where the decorated definition node is in hand", and `resolveRouteHandlerSymbols`
already consumes it to stamp `handlerSymbolId`. Only the Spring extractor ever
set it, so for every decorator-routed framework — FastAPI, Flask, NestJS — it
arrived undefined and 0 of 162 routes carried a handler. A route decorator's
parent IS the decorated definition, so the name is in hand: add
`decoratedDefinitionName` and thread it through. It climbs consecutive
decorators so stacked forms (`@router.get(...)` over `@requires_auth`) resolve,
caps the climb so a malformed tree cannot loop, and returns undefined rather
than guessing — the routes phase already treats a missing name as
"fall back to file-level".
2. With a handler symbol resolved there is finally something to point an edge at.
Emit a definition-level HANDLES_ROUTE alongside the file-level one. The sibling
decorator overlay already does exactly this: `pipeline-phases/tools.ts` anchors
HANDLES_TOOL on the definition the decorator sat on, not its file. Routes were
the outlier.
Kept as one change because the edge is inert without the symbol — emitted from a
branch lacking part 1 it produces zero edges, since `handlerSymbolId` is empty.
Additive, and both existing consumers are unaffected:
`group/extractors/http-route-extractor.ts` types its query `(handlerFile:File)`;
`manifest-extractor.ts` matches an untyped `(handler)` but takes `LIMIT 1` ordered
by `handler.id`, and `File:…` sorts before `Function:…`, so its selected row is
unchanged.
Direction is Function → Route, matching how every other overlay attaches
(MEMBER_OF → Community, STEP_IN_PROCESS → Process, HANDLES_TOOL → Tool: the symbol
is the source). That also keeps it free of schema risk — `Function|Route` is
already declared by the ATTACHMENT rule in `lbug/schema.ts`
(`DEFINITION_ANCHOR_LABELS × ATTACHMENT_TARGET_LABELS`), which that file documents
as deliberate headroom for this case. Route → Function would have needed a new
hand-listed pair, and an undeclared pair aborts `analyze` outright — a failure
that file records having hit four separate times.
Verified on a FastAPI fixture (edges 9 → 11):
api.py (File) -> GET /widgets, POST /widgets [unchanged]
list_widgets (line 10) -> GET /widgets [new]
create_widget (line 15) -> POST /widgets [new]
On the 12.4k-file repository: 161 of 162 routes now resolve to their handler
function, up from 0. The single abstention is `uniqueSymbolId` correctly refusing
to guess where the name is not uniquely resolvable in its file.
`npx tsc --noEmit` clean; schema-pair coverage and route suites pass (196 tests).
* fix(routes): harden decorator handler attribution (#2865)
Keep definition-level route links correct across warm caches and malformed symbol lookups, and avoid per-route group-sync scans. Move Python AST ownership behind the language provider and add end-to-end regression coverage.
Note: full npm test could not complete in this container due unrelated worker startup failures and a stalled retry; targeted route suites, typecheck, format, and lint passed.
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(routes): reuse per-file symbol lookup and drop duplicate warm-cache test
Share extract()'s CONTAINING_QUERY memo with the graph provider path, resolve each route handler once, and fold the decorator-edge warm-cache assertions into the existing FastAPI composed-route round-trip.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Carter LaSalle <carterlasalle@gmail.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
238 lines
9.7 KiB
TypeScript
238 lines
9.7 KiB
TypeScript
/**
|
|
* End-to-end coverage of imported/composed FastAPI route path constants (#2391).
|
|
*
|
|
* `@router.post(API_V1_WIDGETS_GET)` — where the path is an imported constant
|
|
* built by `+`-concatenation in another module — must index as
|
|
* `POST /api/v1/widgets/get` in the ingestion `Route` graph nodes (which drive
|
|
* `route_map` / `api_impact`), NOT as `POST /`. An argument that cannot be folded
|
|
* to a literal is skipped entirely (KTD5 floor), never recorded as `/`.
|
|
*
|
|
* The group HTTP-contract parity, multi-hop chains, the module-collision floor,
|
|
* and the warm-cache guard are added by U5/U6 (see the sibling describe blocks
|
|
* and `http-route-extractor.test.ts`).
|
|
*
|
|
* Fixture: `test/fixtures/fastapi-composed-app/`.
|
|
*/
|
|
|
|
import { describe, it, expect, beforeAll } from 'vitest';
|
|
import path from 'node:path';
|
|
import * as fs from 'node:fs';
|
|
import * as os from 'node:os';
|
|
import Parser from 'tree-sitter';
|
|
import Python from 'tree-sitter-python';
|
|
import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js';
|
|
import type { PipelineResult } from '../../types/pipeline.js';
|
|
import { PYTHON_HTTP_PLUGIN } from '../../src/core/group/extractors/http-patterns/python.js';
|
|
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');
|
|
|
|
describe('FastAPI composed route constants — ingestion pipeline (#2391)', () => {
|
|
let result: PipelineResult;
|
|
|
|
beforeAll(async () => {
|
|
result = await runPipelineFromRepo(FIXTURE, () => {}, {});
|
|
}, 60_000);
|
|
|
|
function routes(): { method: string | undefined; url: string }[] {
|
|
const out: { method: string | undefined; url: string }[] = [];
|
|
result.graph.forEachNode((n) => {
|
|
if (n.label !== 'Route') return;
|
|
const method = n.properties.method;
|
|
out.push({
|
|
method: method === undefined ? undefined : String(method),
|
|
url: String(n.properties.name),
|
|
});
|
|
});
|
|
return out;
|
|
}
|
|
const urls = (): string[] =>
|
|
routes()
|
|
.map((r) => r.url)
|
|
.sort();
|
|
|
|
it('resolves the imported composed constant to its full path', () => {
|
|
expect(routes()).toContainEqual({ method: 'POST', url: '/api/v1/widgets/get' });
|
|
});
|
|
|
|
it('never records a phantom `/` for a non-literal path', () => {
|
|
expect(urls()).not.toContain('/');
|
|
});
|
|
|
|
it('leaves an ordinary string-literal sibling route unchanged', () => {
|
|
expect(routes()).toContainEqual({ method: 'GET', url: '/literal/health' });
|
|
});
|
|
|
|
it('skips an unresolvable constant argument (no Route node, not `/`)', () => {
|
|
// `@router.delete(UNKNOWN_ROUTE_CONST)` — the constant is defined nowhere, so
|
|
// it folds to null and the route is dropped rather than indexed as `DELETE /`.
|
|
expect(routes().some((r) => r.method === 'DELETE')).toBe(false);
|
|
});
|
|
|
|
it('joins an APIRouter(prefix=…) with a resolved composed path', () => {
|
|
// prefixed.py: `router = APIRouter(prefix="/v2")` + `@router.post(COMPOSED)`.
|
|
expect(routes()).toContainEqual({ method: 'POST', url: '/v2/api/v1/widgets/get' });
|
|
});
|
|
|
|
it('keeps two composed routes at distinct paths as distinct nodes', () => {
|
|
const composed = routes().filter((r) => r.url.endsWith('/api/v1/widgets/get'));
|
|
expect(composed.map((r) => r.url).sort()).toEqual([
|
|
'/api/v1/widgets/get',
|
|
'/v2/api/v1/widgets/get',
|
|
]);
|
|
});
|
|
|
|
it('resolves a multi-hop import chain (leaf → mid → base) with an inline concat', () => {
|
|
// deep/base.py ROOT=/root → deep/mid.py MID=ROOT+"/mid" → deep/leaf.py
|
|
// @router.get(MID + "/leaf").
|
|
expect(routes()).toContainEqual({ method: 'GET', url: '/root/mid/leaf' });
|
|
});
|
|
|
|
it('resolves same-named constants in different packages against their OWN package', () => {
|
|
// pkg_a/constants.py SHARED="/a-shared" and pkg_b/constants.py SHARED="/b-shared",
|
|
// each imported via `from .constants import SHARED`. Never crossed (KTD4).
|
|
expect(routes()).toContainEqual({ method: 'GET', url: '/a-shared' });
|
|
expect(routes()).toContainEqual({ method: 'GET', url: '/b-shared' });
|
|
expect(urls().filter((u) => u.endsWith('-shared'))).toEqual(['/a-shared', '/b-shared']);
|
|
});
|
|
|
|
it('snapshots an aliased constant before a later mutation, end-to-end (#2393)', () => {
|
|
// app/snapshot.py: `SNAP = API_V1` (captures "/api/v1") then `API_V1 += "/mutated"`.
|
|
// SNAP's route must be the pre-mutation value, never the mutated one.
|
|
expect(routes()).toContainEqual({ method: 'GET', url: '/api/v1' });
|
|
expect(urls()).not.toContain('/api/v1/mutated');
|
|
});
|
|
});
|
|
|
|
// ─── R4 parity: the group HTTP-contract layer resolves the same paths ─────────
|
|
|
|
describe('FastAPI composed route constants — ingestion↔group parity (#2391 R4)', () => {
|
|
it('group provider paths match the ingestion Route-node paths for composed routes', async () => {
|
|
const ingestion = await runPipelineFromRepo(FIXTURE, () => {}, {});
|
|
const ingestionUrls = new Set<string>();
|
|
ingestion.graph.forEachNode((n) => {
|
|
if (n.label === 'Route') ingestionUrls.add(String(n.properties.name));
|
|
});
|
|
|
|
// Run the group plugin over the same fixture files.
|
|
const files: Record<string, string> = {};
|
|
const walk = (dir: string, rel: string): void => {
|
|
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
|
const abs = path.join(dir, entry.name);
|
|
const r = rel ? `${rel}/${entry.name}` : entry.name;
|
|
if (entry.isDirectory()) walk(abs, r);
|
|
else if (entry.name.endsWith('.py')) files[r] = fs.readFileSync(abs, 'utf8');
|
|
}
|
|
};
|
|
walk(FIXTURE, '');
|
|
const parser = new Parser();
|
|
const parseSource = (p: Parser, src: string): Parser.Tree => {
|
|
p.setLanguage(Python);
|
|
return p.parse(src);
|
|
};
|
|
const ctx = PYTHON_HTTP_PLUGIN.prepareRepo?.({
|
|
files: Object.keys(files),
|
|
parser,
|
|
readFile: (r) => files[r] ?? null,
|
|
parseSource,
|
|
});
|
|
const groupPaths = new Set<string>();
|
|
for (const [rel, src] of Object.entries(files)) {
|
|
for (const d of PYTHON_HTTP_PLUGIN.scan(parseSource(parser, src), ctx, rel)) {
|
|
if (d.role === 'provider') groupPaths.add(d.path);
|
|
}
|
|
}
|
|
|
|
// Every composed route the ingestion side resolved is also a group provider
|
|
// path, and vice versa — the two subsystems agree (R4), including the
|
|
// multi-hop and per-package-collision cases. (The `/v2` APIRouter(prefix)
|
|
// route is emitted by BOTH sides as well — asserted separately above; the
|
|
// four paths below are this block's shared-parity set.)
|
|
for (const composed of ['/api/v1/widgets/get', '/root/mid/leaf', '/a-shared', '/b-shared']) {
|
|
expect(ingestionUrls.has(composed)).toBe(true);
|
|
expect(groupPaths.has(composed)).toBe(true);
|
|
}
|
|
// Neither side invents a phantom `/` for the unresolvable DELETE route.
|
|
expect(ingestionUrls.has('/')).toBe(false);
|
|
expect(groupPaths.has('/')).toBe(false);
|
|
}, 60_000);
|
|
});
|
|
|
|
// ─── Warm parse-cache: composed routes survive the cache serialization ────────
|
|
|
|
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 {
|
|
const cold: ParseCache = {
|
|
version: PARSE_CACHE_VERSION,
|
|
entries: new Map(),
|
|
usedKeys: new Set<string>(),
|
|
storagePath: storageDir,
|
|
onDiskKeys: new Set<string>(),
|
|
};
|
|
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<string>();
|
|
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<string, string> => {
|
|
const out = new Map<string, string>();
|
|
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 });
|
|
}
|
|
}, 120_000);
|
|
});
|