GitNexus/gitnexus/test/unit/java-route-const-pipeline-e2e.test.ts
ChunxueLi 3f5fbb05e0
feat(group+ingestion): resolve Java constant-based route paths (@PostMapping(ApiPathConstants.X)) (#2980)
* feat(group): resolve Java constant-based route paths via repo constant map

- prepareRepo builds repo-wide Java constant map (constant-definition files only,
  cheap regex gate; per-file try/catch so one bad file degrades not forfeits)
- bind parser language in prepareRepo (orchestrator hands over a bare Parser)
- scan() lazily overlays the importing file's own import table (extracted from
  the tree already in hand, zero extra parses) before folding operands
- foldJavaOperands resolves qualified refs (Class.CONST) + static imports +
  string concatenation against the merged view; unresolved refs are skipped,
  never guessed

Real-repo validation (winning-winex-opt, 23k Java files):
  providers 2 -> 1701 (1700 source_scan_resolved), cross-links 0 -> 589 exact
Unit: 14/14 (java-route-const-resolver.test.ts)

* fix(review): address bot review findings on PR #2980

- P2-1 (real): spring.ts route loop dropped every @value_expr match — the
  '!valueNode' guard ran before the operand branch, so ingestion emitted zero
  constant-referencing routes. Guard now accepts @value_expr when @value is
  absent; two downstream valueNode dereferences made conditional.
  Added 2 extractor-level regression tests (16 total).
- P2-2 (real): collectSpringTypes copied rawPath:'' for constant routes into
  the shared Spring inheritance view — now skipped there (fold happens in
  scan(); empty-path noise would leak into inheritance-based providers).
- P1-1 (false positive): Java 'static final' allows exactly one initializer
  (duplicate declarations are compile errors), so the Python-style rebinding
  shadowing cleanup does not apply — documented at the site.
- P1-2 (false positive): constant-resolver.ts and prepareDurableParsedFileChunk
  both exist on upstream main (#2391 / parsedfile-store.ts:562); the bot's
  'repository lookup' appears to have compared against a stale index.
- P3: removed dead FQN_CONTROLLER fixture.

Real-repo regression: 589 cross-links / 2423 contracts (was 2424 — the
dropped contract is the empty-path inheritance artifact fixed above).

* docs(cache): note Java constant-route capture set in the SCHEMA_BUMP ledger

The Java constant-route harvest (route-extractors/java-const-resolver.ts +
the spring.ts operand branch + the parse-worker Java constant harvest)
changes the worker capture set: a warm pre-feature cache replays
moduleConstants=0 captures verbatim and silently drops every constant-based
Spring route on unchanged files. After rebasing onto current main the
ledger already sits at 70, whose capture set post-dates and includes this
harvest, so v70 invalidates those caches — no additional bump is needed.

* fix(feign): guard @RequestLine against the constant-valued shape

A constant-valued `@RequestLine(SOME_CONST)` is captured as @value_expr,
not @value, so `valueNode` is undefined in that shape and the literal
dereference crashed the scan. Skip instead — folding verb+path literals
through the constant map is out of scope for this PR.

Found in maintainer review of #2980.

* fix(resolver): bound qualified-ref recursion depth for self/mutual import cycles

Maintainer review point: the qualified branch of resolveJavaConstant
recurses through resolveJavaImport without a guard — a self-import
(X = SelfConsts.X + ...) or a pair of mutually-importing constants
would recurse without bound before reaching the shared fold's
visited-stack, which only guards the bare-name path.

Bound the Java-qualified walk with a depth cap (32) and thread it
through every recursive call. Two regression tests use real repo
shapes (repoOf fixtures): self-import and mutual-import cycles both
terminate with null (skip floor), as before, but promptly.

Also drops the stray machine-local .gitignore entry that rode along
from the fork's dev branch.

* fix(routes): address round-2 review — provider hooks, FQN fold, interface nesting

F1 (High): production harvest silently dropped routes when the constants
class is not named *Constants (e.g. ApiPaths). The content gate is now
SYNTAX-driven (static-final String field or any class import) and lives in
the provider (moduleConstantHeuristic), not a shared-layer regex.

F2: shared ingestion layers no longer branch on language. The harvest and
the qualified-ref fold run through new provider hooks
(extractModuleConstants / foldRoutePathOperands); parse-impl resolves the
provider by filePath (getProviderForFile). Python wires the same hooks for
architecture parity.

F3: multi-segment FQN chains (com.example.ApiPaths.USERS) now flatten
recursively; verified via tree-sitter that the existing query already
captures the whole nested field_access — the gap was resolver-side only.

F4: implicit-final interface semantics no longer leak into nested classes
at type boundaries (JLS 9.5).

F5: nested same-name shadowing now drops the stale entry (rebind-drop,
matching Python #2391 semantics) instead of keeping the first binding.

Tests: 9 new unit tests (27/27) + real-pipeline e2e over a reviewer-shaped
fixture (non-*Constants class, cold run + warm parse-cache replay) — the
exact production gap unit tests missed.

* style: prettier --write on the two touched test files (CI format gate)

* fix(routes): address the open review findings on Java constant route folding

Answers every reproduced finding still open on #2980, plus the defects an
adversarial pass found in the first round of those fixes. The wrong-path group
each turned a *missing* fact into a *wrong* one, which is what this module's
skip-or-correct contract exists to prevent.

Wrong-path fixes

* Escapes were deleted from constant values. tree-sitter-java splits a
  `string_literal` around its `escape_sequence` children, so joining
  `string_fragment`s alone folded `"/user/{id:\\d+}"` — the standard Spring
  path-variable constraint — to `/user/{id:d+}`, and a pure-escape literal to
  the empty string. Worse, the LITERAL path keeps escapes verbatim, so one Java
  route had two irreconcilable spellings. `stringLiteralValue` now reuses
  `unquoteSpringLiteral`, the helper that literal path already uses. Java text
  blocks are excluded: that helper's `"""` arm would hand back the raw block,
  newline and incidental indentation included, so they keep the old skip.

* A constant-valued class prefix produced a truncated route. The new
  `@value_expr` query branches were `method_declaration`-only, so
  `@RequestMapping(ApiPaths.BASE)` left the prefix empty and the method route
  was emitted unprefixed — a path the application does not serve, where the base
  emitted nothing at all. Both subsystems now detect such a class and suppress
  its method routes, the rule `classesWithArrayPrefix` already encodes for the
  array form. The suppression covers ingestion's separate no-argument-mapping
  loop too, without which a bare `@GetMapping` under a constant prefix still
  shipped an empty-path Route while the group emitted nothing.

* A shadowed static import survived a non-foldable rebind. The rebind-drop
  deleted `literals`/`exprs` but not `imports`, so a name both static-imported
  and locally redeclared resolved through the stale import to the imported
  value instead of skipping (#2393's Python defect, reproduced for Java).

* `resolveJavaImport` guessed where its own docstring promised null. The
  nearest-shared-directory tie-break is gone: javac resolves duplicate FQNs by
  classpath order, so proximity can return a src/test fixture copy.

Parity and coverage fixes

* One constant-file gate, exported as `isJavaConstantFile` and used by both the
  ingestion provider and the group `prepareRepo` pre-pass. The two spellings
  disagreed on a constant INTERFACE — implicitly `public static final`, so it
  carries neither keyword — which the group admitted and ingestion rejected, so
  the group published a contract while the graph got no Route node. It is also
  modifier-order agnostic now, and its interface arm requires a String
  assignment so a javadoc mentioning "interface" no longer costs a parse.

* Import ambiguity is measured over constant-DEFINING files on both sides.
  Ingestion's harvest gate also admits import-only files, so handing
  `resolveJavaImport` every repo key let a duplicate FQN that defines nothing
  make ingestion alone floor to skip — reopening the same parity break in the
  same losing direction.

* Python's constant harvest is unconditional again. The gate added here
  required NAME immediately followed by `=`, so it dropped `API: str = "/api"`,
  `API: Final[str] = "/api"` and every composed constant whose RHS starts with
  an identifier — routes that already resolve on main. The worker now treats a
  missing heuristic as "harvest" rather than "skip".

* Enum and record declarations were traversed but never collected, so a
  `static final String` declared in one was absent from the map. The walk still
  descends the whole body, so a type nested in an enum-constant body is kept.

* Constants composed across files through a qualified ref never resolved:
  operands found inside an initializer went to the agnostic core, which only
  knows bare names, so `X = BConsts.Y + "/tail"` floored to null even
  acyclically. The Java binding now folds its own expressions — and carries the
  core's guards with them: a `visited` stack popped on unwind, a memo of
  successes, and `MAX_FOLD_LENGTH`. Without the memo a shared-descendant DAG
  re-folds each child per reference; because a chain of empty strings never
  accumulates output, the length cap could not stop it, and one route over a
  31-line constants file took 11 s at 28 levels on the main thread.

* Dropped the dead `com.java.lang.` type normalization.

Cache

* `SCHEMA_BUMP` 70 -> 72. Leaving it at 70 was justified by "the ledger already
  sits at 70, whose capture set post-dates and includes this harvest" — it does
  not: 70 was cut by fe3d7e56b for #2417/#2891, an ancestor of this base. With
  package.json untouched, `PARSE_CACHE_VERSION` was byte-identical across the
  merge, so every same-version warm cache replayed pre-feature captures and the
  feature was inert. 72 rather than 71 because open PR #3017 already claims 71
  with an identical pin test — the ledger's rule is the next value above every
  in-flight claim, not above origin/main.

Tests

* Regression cover for each fix above, including a gate-level test (the gate
  itself had none), an import-ambiguity test, a text-block test, and a 30-level
  shared-descendant DAG that fails by timeout if the memo is ever removed.
* New `group/java-const-route-parity.test.ts` drives `prepareRepo` + a
  three-argument `scan`. Every existing Spring parity guard calls `scan(tree)`
  with ONE argument, and the plugin drops constant-valued routes without a repo
  context — so those guards were structurally blind to this whole feature.
* The pipeline e2e now proves the warm run is a REPLAY (`usedWorkerPool` false)
  instead of only comparing route sets. It was not one: the test never persisted
  the durable ParsedFile store, so the "warm" run reparsed through the workers
  and would have passed with the cache round-trip completely broken.
* Its dist freshness gate covers every source the pipeline loads, not just
  parse-worker.ts, and prints the loud message the docblock promised.
* The self-import cycle fixture now actually self-imports, so it reaches the
  qualified-ref recursion and its depth cap.
* Removed the dead `WIN_POST_MAPPING` fixture and the claim behind it: Spring
  alias recognition is an exact-name map on this base, so `@WinPostMapping`
  extracts zero routes no matter how its value folds (#2883 is still open).
  Fixtures now use annotations this branch actually recognises.

* fix(routes): widen the Java constant-file gate to match its extractor

Answers the gitnexus-check round on 43a0ff290.

The gate was still narrower than the extractor it feeds, in two ways the
extractor explicitly supports:

* `static final String` was matched as an ADJACENT pair, but the extractor
  scans modifiers independently (`isStaticFinal`), so `static public final
  String PATH = "/x";` — legal Java — was extracted when parsed and never
  parsed, because the gate returned false.
* the type had to be the bare token `String`, but the extractor also accepts
  `java.lang.String`, so `public static final java.lang.String PATH = "/x";`
  was skipped the same way.

Both are the same defect class as the ingestion/group divergence this predicate
was introduced to prevent, one layer down: a cost gate that is narrower than
the thing it gates silently drops facts. The modifier run is now matched as a
span excluding `;{}()`, so every legal order and the qualified type name are
admitted while precision holds — a local `String s = "x"` inside
`static void f() { … }` still does not match, because reaching it from `static`
crosses `(`, `)` and `{`. `final` is deliberately not required: the gate may be
wider than the extractor, never narrower.

Also: the worker's harvest condition moves into `shouldHarvestModuleConstants`
in `language-provider.ts`. The rule that is easy to get backwards — a provider
declaring no `moduleConstantHeuristic` harvests unconditionally — was only
reachable by booting a worker, so the Python tests could assert the extractor
harvests and the provider declares no heuristic while a regression to
`provider.moduleConstantHeuristic?.(content)` still turned the hook off. The
tests now drive the predicate itself, plus the two branches around it.

One finding in that round is not reproducible: the parity helper is not made
unresolvable by its import-only fixture. Every `resolveJavaImport` call site
passes the fold state's `constantKeys` — files with `literals`/`exprs` — not
`repo.keys()`, so a same-FQN class defining nothing creates no ambiguity. That
filtering is what the helper exists to exercise, and the test is green.

---------

Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
2026-08-25 09:41:57 +01:00

264 lines
11 KiB
TypeScript

/**
* #2980 review round-2: COLD and WARM pipeline e2e for the provider-hook
* constant harvest (`extractModuleConstants` / `moduleConstantHeuristic` /
* `foldRoutePathOperands`).
*
* The maintainer's blocking finding: unit tests only exercised worker-gated
* helpers — never the REAL pipeline. A controller referencing constants from
* a class NOT named `*Constants` (e.g. `ApiPaths`) was silently dropped:
* the old content gate `/import ... [\\w.]*Constants/` never matched, the
* constants file never entered the import map, the route resolved to null and
* got skipped.
*
* This file drives the REAL `runChunkedParseAndResolve` with the REAL compiled
* dist worker (vitest auto-falls back to dist/core/ingestion/workers/
* parse-worker.js) over a fixture repo shaped like the reviewer's example:
*
* repo/
* src/main/java/com/example/ApiPaths.java — constants class NOT named
* *Constants (the High bug)
* src/main/java/com/example/UserController.java — @RequestMapping prefix +
* @PostMapping(ApiPaths.X) +
* FQN form + concat over a
* static-imported bare ref
*
* Assertions (both runs):
* - the emitted Route node carries the FOLDED literal path, not the expr;
* - ALL THREE non-literal shapes survive (qualified, FQN-qualified, concat);
* - a phantom `POST ` / empty path never appears (skip floor);
* - the warm run yields the IDENTICAL route set AND is a genuine replay
* (`usedWorkerPool === false`) — the harvest result survives the
* structured-clone cache round trip (ModuleConstants uses Map, exercised
* through mapReplacer/mapReviver). Asserting the route set alone would pass
* on a cache MISS that silently reparsed.
*
* Rebuild gate: this test requires dist/ to be current; when dist/ is stale
* (older than src/) it self-skips with a loud message rather than silently
* asserting against the old binary. (CI builds before vitest, so it runs.)
*/
import { beforeEach, afterEach, describe, expect, it } from 'vitest';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
import { runChunkedParseAndResolve } from '../../src/core/ingestion/pipeline-phases/parse-impl.js';
import { PARSE_CACHE_VERSION, type ParseCache } from '../../src/storage/parse-cache.js';
import {
getDurableParsedFileDir,
pruneAndSaveDurableParsedFileStore,
} from '../../src/storage/parsedfile-store.js';
// ── dist freshness gate ───────────────────────────────────────────────────
// The worker is one emitted file among many: TypeScript emits every module in
// this feature separately, so comparing dist/parse-worker.js against
// src/parse-worker.ts alone passes while the resolver, the Spring extractor or
// the provider behind it are stale — and the test then asserts against the
// PREVIOUS build's harvest. Gate on the newest mtime across every source this
// pipeline actually loads.
const repoRoot = path.resolve(__dirname, '..', '..');
const distWorker = path.join(repoRoot, 'dist', 'core', 'ingestion', 'workers', 'parse-worker.js');
const GATED_SOURCES = [
'core/ingestion/workers/parse-worker.ts',
'core/ingestion/route-extractors/java-const-resolver.ts',
'core/ingestion/route-extractors/constant-resolver.ts',
'core/ingestion/route-extractors/spring.ts',
'core/ingestion/languages/java.ts',
'core/ingestion/languages/python.ts',
'core/ingestion/language-provider.ts',
'core/ingestion/pipeline-phases/parse-impl.ts',
];
const newestSourceMs = Math.max(
...GATED_SOURCES.map((rel) => fs.statSync(path.join(repoRoot, 'src', rel)).mtimeMs),
);
const distStale = !fs.existsSync(distWorker) || fs.statSync(distWorker).mtimeMs < newestSourceMs;
if (distStale) {
// `describe.skip` prints only vitest's ordinary skip marker, so without this
// the docblock's promised "loud message" did not exist and a stale/absent
// dist/ looked like a passing run.
console.warn(
'[#2980 e2e] SKIPPED: dist/ is missing or older than src/ — run `npm run build` to exercise the real pipeline.',
);
}
const maybeDescribe = distStale ? describe.skip : describe;
// ── fixture repo (reviewer's exact High-finding shape) ────────────────────
const API_PATHS = `package com.example.common;
public class ApiPaths {
public static final String USERS = "/api/v1/users";
public static final String ORDERS = "/api/v1/orders";
public static final String V1 = "/api/v1";
}
`;
const USER_CONTROLLER = `package com.example;
import com.example.common.ApiPaths;
import static com.example.common.ApiPaths.V1;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.GetMapping;
@RequestMapping("/users")
public class UserController {
// Qualified ref via a class NOT named *Constants (High finding): the old
// gate dropped the whole route because ApiPaths fails the name pattern.
@PostMapping(ApiPaths.USERS)
public void create() {}
// FQN-qualified form (F3): multi-segment field_access chain.
@GetMapping(com.example.common.ApiPaths.ORDERS)
public void list() {}
// Inline concat with a STATIC-IMPORTED bare ref — the shape this fixture
// used to only claim: it spelled the operand as the full FQN chain, which
// just re-tested the FQN branch above, so bare-name resolution through the
// import table had no coverage anywhere in the suite.
@PostMapping(V1 + "/orders")
public void createOrders() {}
}
`;
let repoDir: string;
let storageDir: string;
function writeFixture(): { path: string; size: number }[] {
const files: [string, string][] = [
['src/main/java/com/example/common/ApiPaths.java', API_PATHS],
['src/main/java/com/example/UserController.java', USER_CONTROLLER],
];
const out: { path: string; size: number }[] = [];
for (const [rel, content] of files) {
const full = path.join(repoDir, rel);
fs.mkdirSync(path.dirname(full), { recursive: true });
fs.writeFileSync(full, content);
out.push({ path: rel, size: Buffer.byteLength(content) });
}
return out;
}
/**
* The parse phase does not emit Route nodes itself — it returns the folded
* `decoratorRoutes` (the routes phase emits them downstream). Asserting on the
* folded paths at THIS seam is exactly the regression the maintainer asked
* for: the worker's harvest → provider heuristic → parse-impl fold, with the
* real dist worker.
*/
type PipelineResult = Awaited<ReturnType<typeof runChunkedParseAndResolve>>;
function foldedRoutesOf(result: PipelineResult): Array<{ path: string; method: string }> {
return (result.allDecoratorRoutes ?? [])
.filter((r) => typeof r.routePath === 'string')
.map((r) => ({ path: r.routePath, method: r.httpMethod }));
}
async function runPipeline(
cache: ParseCache,
files: { path: string; size: number }[],
): Promise<PipelineResult> {
const kg = createKnowledgeGraph();
return await runChunkedParseAndResolve(
kg,
files,
files.map((f) => f.path),
files.length,
repoDir,
Date.now(),
() => {},
{ workerPoolSize: 1, parseCache: cache },
);
}
maybeDescribe('#2980 provider-hook constant harvest — real pipeline (cold + warm)', () => {
beforeEach(() => {
repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gnx-2980-cold-'));
storageDir = path.join(repoDir, '.gitnexus');
});
afterEach(() => {
for (const d of [repoDir]) fs.rmSync(d, { recursive: true, force: true });
});
it('cold run: folds qualified / FQN / concat paths from a non-*Constants class', async () => {
const files = writeFixture();
const cache: ParseCache = {
version: PARSE_CACHE_VERSION,
entries: new Map(),
usedKeys: new Set(),
storagePath: storageDir,
onDiskKeys: new Set(),
};
const result = await runPipeline(cache, files);
expect(result.usedWorkerPool).toBe(true);
const routes = foldedRoutesOf(result);
// All three non-literal shapes resolve to folded literals. (The class-level
// @RequestMapping("/users") prefix join happens in the downstream routes
// phase — at this seam we assert the method-level folded paths.)
const paths = routes.map((r) => r.path).sort();
expect(paths).toContain('/api/v1/users'); // qualified ref via import
expect(paths).toContain('/api/v1/orders'); // FQN multi-segment chain
// The concat route folds to the same literal as the FQN route.
expect(paths.filter((p) => p === '/api/v1/orders').length).toBeGreaterThanOrEqual(2);
// Skip floor: no phantom empty/raw-expr paths.
for (const p of paths) {
expect(p.length).toBeGreaterThan(1);
expect(p).not.toContain('ApiPaths');
expect(p).not.toContain('com.example');
}
}, 120_000);
it('warm run: parse-cache replay yields the identical folded route set', async () => {
const files = writeFixture();
const cache: ParseCache = {
version: PARSE_CACHE_VERSION,
entries: new Map(),
usedKeys: new Set(),
storagePath: storageDir,
onDiskKeys: new Set(),
};
// Run #1 populates the cache; persist it like run-analyze does — BOTH the
// chunk shards and the durable ParsedFile store. `slimParseWorkerResultsForCache`
// blanks `parsedFiles` before writing a shard, so a warm run without the
// durable store cannot replay the chunk and silently falls back to the
// workers — which is what this test used to do while still passing.
const run1 = await runPipeline(cache, files);
const { saveParseCache, pruneCache } = await import('../../src/storage/parse-cache.js');
pruneCache(cache, cache.usedKeys);
const savedKeys = await saveParseCache(storageDir, cache);
expect(savedKeys.length).toBeGreaterThan(0);
await pruneAndSaveDurableParsedFileStore(
getDurableParsedFileDir(storageDir),
PARSE_CACHE_VERSION,
new Set(savedKeys),
);
// Run #2 — warm: every chunk is a cache HIT, no worker spawn, the cached
// ParseWorkerResult (moduleConstants included) is replayed from disk.
const { loadParseCache } = await import('../../src/storage/parse-cache.js');
const warm = await loadParseCache(storageDir);
expect(warm.onDiskKeys).toEqual(new Set(savedKeys));
const run2 = await runPipeline(warm, files);
const cold = foldedRoutesOf(run1)
.map((r) => `${r.method} ${r.path}`)
.sort();
const hot = foldedRoutesOf(run2)
.map((r) => `${r.method} ${r.path}`)
.sort();
expect(hot).toEqual(cold);
expect(hot.length).toBeGreaterThan(0);
// Without this the test proves nothing about the cache: `loadParseCache`
// returns an EMPTY cache on any failure (missing file, corrupt JSON,
// version mismatch) and never throws, so a broken Map round-trip through
// mapReplacer/mapReviver — the exact regression this test exists for —
// would silently reparse through the workers and produce the same routes.
expect(run1.usedWorkerPool).toBe(true);
expect(run2.usedWorkerPool).toBe(false);
}, 120_000);
});