GitNexus/gitnexus/test/unit/constant-resolver.test.ts
Gergő Magyar 5f4964b4e6
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
fix: resolve imported/composed FastAPI route path constants (#2391) (#2393)
* feat(routes): add pure Python string-constant resolver (#2391 U1)

* feat(routes): extract Python module constants from tree (#2391 U2)

* feat(routes): capture non-literal FastAPI decorator args + per-file constants, bump parse-cache schema (#2391 U3)

* feat(routes): resolve composed decorator route constants in parse-impl + skip floor (#2391 U4)

* feat(routes): resolve composed FastAPI route constants in group HTTP-contract layer (#2391 U5)

* test(routes): multi-hop, ingestion↔group parity, and warm-cache regression locks (#2391 U6)

* docs(routes): mark the language-agnostic seam for cross-language const resolution (#2391)

* refactor(routes): extract language-agnostic constant-fold core; Python becomes a binding (#2391)

The fold, cycle guard, and depth cap now live in constant-resolver.ts and take a
pluggable ImportResolver. python-const-resolver.ts supplies the Python import
semantics + tree extractor and re-exports the same surface, so no call site
changes. A Spring/Kotlin/C# binding can now reuse the core with its own resolver
(proven by constant-resolver.test.ts driving it with a Java-style resolver).

* fix(routes): treat the constant-fold cycle guard as a recursion stack (#2391)

The `visited` set in `foldName` was added-to but never removed on unwind, so a
constant referenced more than once in a single fold — `A + A`, a reused
separator (`SLASH + PATH + SLASH`), or a diamond `X = P + Q` where P and Q share
a base — tripped the cycle guard on its second occurrence and the whole route
was silently dropped by the skip floor. Pop the guard in `finally` so it tracks
the ACTIVE resolution stack, not every name ever seen: a true cycle (a name
still on the stack) is still caught, but a name that already resolved and popped
folds again. Re-computation stays bounded by MAX_RESOLVE_DEPTH, so no blowup is
reintroduced.

Locked in constant-resolver.test.ts (A+A, reused separator, shared-base
diamond); the pre-existing real-cycle and depth-cap cases still return null.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): make module-constant binding writes mutually exclusive (#2391)

`extractPythonModuleConstants` kept `literals`, `exprs`, and `imports` as three
independent maps: `setName` cleared literals+exprs but never `imports`, and an
import never cleared a prior literal/expr. Since `foldName` checks
literals > exprs > imports regardless of source order, a name that was both
imported and locally (re)assigned kept both bindings and the wrong one won —
`from .c import ROUTE; ROUTE = os.getenv(...)` resolved the STALE import instead
of dropping, a confidently wrong route path (the exact skip-floor invariant
this feature is meant to uphold).

Treat the three maps as one logical namespace: any write to one clears the
other two for that name (via `imports.delete` in `setName` and a `bindImport`
helper), so last-binding-in-source-order wins, matching Python. An import both
imported and dynamically rebound now drops. Folding `+=`/`+` onto an imported
base remains deferred (it drops safely, never a stale value).

Locked in python-const-resolver.test.ts: dynamic-rebind drops, literal-shadows-
import, import-shadows-literal, and `+=`-on-import drops.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): widen the group cost-gate to catch literal-leading concats (#2391)

`NONLITERAL_ROUTE_DECORATOR_RE` required the first decorator argument to START
with an identifier, so a string-literal-leading concat like
`@router.get("/api" + SUFFIX)` never tripped `hasComposedRoute`. When such a
route was the ONLY composed shape in a repo, the group layer left `constantsByFile`
empty and dropped the route, while the ingestion side (which has no gate)
resolved `/api/users` and emitted a Route node — an R4 provider/graph parity break.

Widen the gate to also fire on a string-literal-leading `+`-concat, detected by a
`+` before the closing paren on the decorator line. Gating on the `+` (not merely
a leading quote) keeps a plain literal route `@router.get("/x")` OFF the gate, so a
literal-only repo still pays no parse pass.

Locked in fastapi-composed-provider.test.ts: a sole literal-leading concat now
resolves (parseCalls>0 + provider emitted), plus previously-uncovered
`@app.<verb>(CONST)` EXPR-branch resolution; the literal-only no-parse gate case
still passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): correct package-init and over-deep relative import resolution (#2391)

Two edges in `resolvePythonImport`:

- `from . import X` (empty module after the dots) resolved to a sibling
  `<dir>.py` instead of the package `<dir>/__init__.py`. Resolve the bare-package
  case to `__init__.py`.
- An over-deep relative import (more extra dots than the importing file has
  directory levels) silently clamped `dirOf('')` to `''` and could match an
  unrelated root-level `<name>.py` — a wrong file. Guard with `walk > depth →
  null` so an import that escapes above the repo root drops (skip floor).

Both preserve the exact-match / ambiguity→null behavior for ordinary relative and
absolute imports.

Locked in python-const-resolver.test.ts: `from . import` → `__init__.py` (and
null when absent), and an over-deep import returns null even when the clamped
target file exists.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): bound parseConstOperands recursion depth (#2391)

`parseConstOperands` recursed on `binary_operator` children with no depth bound.
A stack overflow is not currently reachable (tree-sitter caps expression nesting
below the JS stack limit, so it throws on a deep `+`-chain before this runs), but
add a depth guard (cap 64, mirroring the fold engine's MAX_RESOLVE_DEPTH) as
defense-in-depth: a pathological chain now floors to null (skip) rather than
relying on tree-sitter's limit. The `depth` parameter defaults to 0, so all
existing callers are unaffected.

Locked in python-const-resolver.test.ts: a 100-term `+` chain yields no binding
(null) instead of throwing; ordinary short chains still fold.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(routes): read each .py once in buildPythonRepoContext (#2391)

The group repo-context builder read every `.py` file from disk twice: once in
the `include_router` cross-file pre-pass and again in the #2391 constant
cost-gate loop — an unconditional 2x read on every Python repo, on every group
extraction. Hoist a single read pass that populates one `pyContents` map (and
computes the composed-route cost gate); both the include_router pre-pass and the
constant-map pass now consume the cached content. Behavior-preserving — a
literal-only repo still does one read and zero parses.

Covered by the existing group unit + integration suites (R4 parity and
include_router prefix joins unchanged).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(routes): tidy constant-resolver docs and declaration order (#2391)

Three no-behavior nits from the PR #2393 review:
- Name `conditional_expression` (`x if c else y`) in the `parseConstOperands`
  jsdoc list of shapes that deferred to null.
- Move `NONLITERAL_ROUTE_DECORATOR_RE` above `buildPythonRepoContext`, which
  references it — it read as a forward reference before (runtime-safe, but
  confusing).
- Correct the integration-test comment that called `/v2/api/v1/widgets/get`
  "ingestion-only garnish": the group side emits it too (asserted separately);
  the four paths in that block are the shared-parity set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(routes): fold `X += "…"` onto an imported base constant (#2391)

Previously `from .c import BASE; BASE += "/v1"` dropped (the extractor could not
represent "the imported prior value" as an operand without self-referencing X and
tripping the cycle guard). Preserve the imported prior under a synthetic `$imp$N`
key — `$` can never appear in a Python identifier, so it cannot collide with a
real name — and reference it, so the augmented assignment folds to
`<imported BASE>/v1`. Extractor-only: no change to the `Operand` type, the fold
core, or the cache shape, so no SCHEMA_BUMP. An imported base that is itself
unresolvable still drops (skip floor preserved — never a wrong path).

Locked in python-const-resolver.test.ts: single and chained `+=` fold onto an
imported base; an unresolvable base still drops.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(routes): resolve bare decorator constants via the by-name entry (#2391)

The group `resolveExprArg` hand-built `[{ kind: 'ref', name }]` and called
`resolveOperands` for a bare-constant decorator argument — exactly what the
language-agnostic core's `resolveConstant(file, name, repo)` seam does. Call it
directly for the identifier case. This gives the previously test-only by-name
entry point a real production caller (it is the documented reuse seam for future
JVM/other bindings), drops the synthetic operand construction, and lets the now-
unused `Operand` type import go. Behavior-identical — the `+`-concat path still
parses to an operand list and folds via `resolveOperands`.

Guarded by the existing group provider suite (bare-constant and concat cases).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(routes): parse each .py once in buildPythonRepoContext (#2391)

The repo-context builder ran two parse loops — the include_router prefix pre-pass
and the #2391 constant-map pass — so an include_router file in a composed repo was
tree-sitter-parsed twice. Merge them into a single pass that parses each `.py` at
most once and feeds both extractions from the same tree; a file that needs neither
pass is still not parsed at all (cost gates unchanged). Complements the earlier
single-read-pass change (this is the single-parse counterpart).

Behavior-preserving (prefixes, R4 parity, and cost gates verified by the group +
integration suites). Locked with a parseCalls assertion: a file needing both
passes is parsed once, not twice.

Note: a cross-run (cross-process) constant-map cache — the other deferred perf
idea — remains out of scope; it needs disk persistence + invalidation and would
add hashing/IO cost on the common path, so it fails the minimal-change bar this
single-parse dedup meets.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): bound constant-fold work and output to prevent OOM (#2391)

The `finally`-popped cycle guard (recursion-stack semantics) correctly folds
diamonds/repeated refs, but popping the guard removed the accidental work cap the
old seen-ever set provided: a wide shared-descendant DAG re-folds each child once
per reference, and a self-multiplying concat (`X = A + A; A = B + B; …`) builds a
genuinely exponential string. Reviewers reproduced ~16.8M folds escalating to
`RangeError: Invalid string length` and heap OOM — and neither fold call site is
wrapped in try/catch, so it crashed the whole phase rather than dropping the route.

Two complementary bounds, both flooring to null (skip), never a wrong value:
- a never-popped `memo` in `foldName` caps recomputation at O(nodes) (successes
  only — a null may be transient on a cyclic branch);
- a `MAX_FOLD_LENGTH` (8192) cap in `foldExpr` drops a fold whose output grows
  past any real route path, bounding the string size the depth cap does not.

Corrects the prior "≤ 2^8 folds" comment (output grows multiplicatively, not
additively). Locked with a 64^4-fanout construction that now drops in ~ms instead
of OOMing; diamonds/cycles/depth-cap behavior unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): snapshot assignment RHS refs at the assignment line (#2391)

`ROUTE = BASE` was stored as a lazy `ref(BASE)`, resolved against BASE's FINAL
binding. So `ROUTE = BASE; BASE += "/v1"` (or `ROUTE = API; API = "/other"`)
resolved ROUTE to the MUTATED value — a confidently wrong path, since Python
assigns by value at the `ROUTE =` line. This was latent for local constants at
the base of this feature and the `+=`-on-import work extended it to imports.

Snapshot each assignment/`+=` RHS reference to a bound name into that name's
current frozen value at the assignment line (`freeze`/`snapshot`): a literal
value, a copy of the current expr (whose refs are already frozen), or an import
preserved under a `$imp$N` alias. Unbound refs (forward references) stay lazy.
A later rebind of the aliased name can no longer change the earlier binding.
`freeze` also unifies the previous `currentOps` + inline import-alias logic.

Locked in python-const-resolver.test.ts: aliased-import-then-`+=`,
aliased-local-then-`+=`, aliased-local-then-rebind all resolve to the pre-mutation
value; normal reference chains still fold.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): fold group identifier args via resolveOperands for parity (#2391)

Resolving a bare-constant decorator arg through `resolveConstant` entered
`foldName` at depth 0, whereas the ingestion side folds `routePathOperands`
through `resolveOperands([{ref}])`, entering at depth 1. At the MAX_RESOLVE_DEPTH
boundary the group tolerated one more hop than ingestion, so a deep alias/re-export
chain resolved in the group provider set but dropped from the graph Route nodes —
an R4 parity break. Restore the operand-list path in the group so both subsystems
share identical fold-entry depth. (`resolveConstant` reverts to the documented
agnostic-core seam.)

Locked in constant-resolver.test.ts: a 4-hop chain that `resolveOperands([ref])`
drops but `resolveConstant` resolves, documenting why the group must use the
operand-list entry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): match multiline literal-leading concats in the cost gate (#2391)

`NONLITERAL_ROUTE_DECORATOR_RE` used `[^)\n]*` so it only saw a literal-leading
`+`-concat when the `+` was on the same line as the opening quote. A
Black-formatted `@router.get(\n "/api"\n + SUFFIX\n)` therefore failed the gate,
and when it was the only composed route in a repo the group dropped it while
ingestion (which parses the tree, not the raw line) resolved it — an R4 parity
break. Drop the `\n` exclusion: `[^)]*` spans the wrapped argument but stays
bounded by the decorator's own closing paren, so a plain literal route still
never trips the gate.

Locked in fastapi-composed-provider.test.ts with a multiline concat fixture.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(routes): bump SCHEMA_BUMP for changed extractor output + E2E snapshot lock (#2391)

`extractPythonModuleConstants` now emits DIFFERENT `moduleConstants` for the same
source (binding mutual-exclusivity clears stale imports; RHS refs are snapshotted;
`$imp$N` aliases). That output is cached verbatim in the parse cache, so a warm
shard built at the pre-fix version would replay stale — in one case actively
wrong — folded values, and the correctness fixes would silently no-op on upgrade.
Bump SCHEMA_BUMP 11→12 to force re-extraction (same warm-cache-replay class the
original 10→11 bump addressed for the field addition).

Also adds the first end-to-end coverage for the new behavior through the real
ingestion pipeline: app/snapshot.py aliases a constant (`SNAP = API_V1`) then
mutates the source (`API_V1 += "/mutated"`), and the test asserts the Route node
is `/api/v1`, never `/api/v1/mutated` — a case the pure-function unit tests
covered but the pipeline did not.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-07 13:23:05 +01:00

141 lines
5.9 KiB
TypeScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* The language-agnostic constant-fold core (#2391). The exhaustive Python fold
* behavior is pinned in `python-const-resolver.test.ts`; this file proves the
* core is genuinely language-neutral by driving it with a NON-Python (Java-style)
* {@link ImportResolver}, so a future Spring/Kotlin/C# binding can reuse the fold,
* cycle guard, and depth cap by supplying only its own import resolver + extractor.
*/
import { describe, it, expect } from 'vitest';
import {
resolveConstant,
resolveOperands,
type ImportResolver,
type ModuleConstants,
type Operand,
type RepoConstants,
} from '../../src/core/ingestion/route-extractors/constant-resolver.js';
const lit = (value: string): Operand => ({ kind: 'literal', value });
const ref = (name: string): Operand => ({ kind: 'ref', name });
const mc = (parts: {
literals?: Record<string, string>;
exprs?: Record<string, Operand[]>;
imports?: Record<string, { module: string; originalName: string }>;
}): ModuleConstants => ({
literals: new Map(Object.entries(parts.literals ?? {})),
exprs: new Map(Object.entries(parts.exprs ?? {})),
imports: new Map(Object.entries(parts.imports ?? {})),
});
// A deliberately non-Python resolver: JVM-style `com.app.Paths` → `com/app/Paths.java`.
const javaImport: ImportResolver = (_importingFileKey, moduleSpec, repoKeys) => {
const candidate = moduleSpec.replace(/\./g, '/') + '.java';
return repoKeys.has(candidate) ? candidate : null;
};
describe('constant-resolver — language-agnostic core', () => {
it('folds a named constant across a Java-style import chain', () => {
const repo: RepoConstants = new Map([
[
'com/app/Paths.java',
mc({ literals: { API: '/api' }, exprs: { WIDGETS: [ref('API'), lit('/widgets')] } }),
],
[
'com/app/Routes.java',
mc({ imports: { WIDGETS: { module: 'com.app.Paths', originalName: 'WIDGETS' } } }),
],
]);
expect(resolveConstant('com/app/Routes.java', 'WIDGETS', repo, javaImport)).toBe(
'/api/widgets',
);
});
it('folds an inline operand list through the injected resolver', () => {
const repo: RepoConstants = new Map([
['com/app/Paths.java', mc({ literals: { API: '/api' } })],
[
'com/app/Routes.java',
mc({ imports: { API: { module: 'com.app.Paths', originalName: 'API' } } }),
],
]);
expect(
resolveOperands('com/app/Routes.java', [ref('API'), lit('/widgets')], repo, javaImport),
).toBe('/api/widgets');
});
it('floors to null when the resolver cannot pin the import', () => {
const repo: RepoConstants = new Map([
[
'com/app/Routes.java',
mc({ imports: { X: { module: 'com.missing.Paths', originalName: 'X' } } }),
],
]);
expect(resolveConstant('com/app/Routes.java', 'X', repo, javaImport)).toBeNull();
});
it('applies the cycle guard and depth cap independent of the resolver', () => {
const cyclic: RepoConstants = new Map([['m', mc({ exprs: { A: [ref('B')], B: [ref('A')] } })]]);
expect(resolveConstant('m', 'A', cyclic, javaImport)).toBeNull();
const exprs: Record<string, Operand[]> = {};
for (let i = 0; i < 20; i++) exprs[`A${i}`] = [ref(`A${i + 1}`)];
const deep: RepoConstants = new Map([['m', mc({ exprs, literals: { A20: '/end' } })]]);
expect(resolveConstant('m', 'A0', deep, javaImport)).toBeNull();
});
it('folds a constant referenced twice in one expression (not a false cycle) (#2393)', () => {
const repo: RepoConstants = new Map([['m', mc({ literals: { A: '/a' } })]]);
// `A + A` — the second reference must NOT be mistaken for a cycle.
expect(resolveOperands('m', [ref('A'), ref('A')], repo, javaImport)).toBe('/a/a');
});
it('folds a reused separator constant (#2393)', () => {
const repo: RepoConstants = new Map([['m', mc({ literals: { SLASH: '/', PATH: 'p' } })]]);
expect(resolveOperands('m', [ref('SLASH'), ref('PATH'), ref('SLASH')], repo, javaImport)).toBe(
'/p/',
);
});
it('by-name and operand-list entry differ at the depth boundary (#2393 parity)', () => {
// A hop chain that lands exactly at MAX_RESOLVE_DEPTH for the operand-list
// entry (one depth deeper than the by-name entry). This is why the group side
// must fold identifier args via resolveOperands([ref]) — the SAME entry the
// ingestion side uses — not resolveConstant, which would resolve here and
// break ingestion↔group parity at the boundary.
const exprs: Record<string, Operand[]> = {};
for (let i = 0; i < 4; i++) exprs[`A${i}`] = [ref(`A${i + 1}`)];
const repo: RepoConstants = new Map([['m', mc({ exprs, literals: { A4: '/end' } })]]);
expect(resolveOperands('m', [ref('A0')], repo, javaImport)).toBeNull();
expect(resolveConstant('m', 'A0', repo, javaImport)).toBe('/end');
});
it('drops a pathological self-multiplying concat instead of exhausting memory (#2393)', () => {
// Each level references the next 64×, so the true value is 64^4 chars — folding
// it naively blows the heap (RangeError/OOM). The fold-length cap must floor it
// to null (drop). This resolves ~instantly; without the cap it OOMs.
const W = 64;
const exprs: Record<string, Operand[]> = {};
for (let i = 0; i < 4; i++) exprs[`L${i}`] = Array.from({ length: W }, () => ref(`L${i + 1}`));
const repo: RepoConstants = new Map([['m', mc({ exprs, literals: { L4: '/leaf' } })]]);
expect(resolveConstant('m', 'L0', repo, javaImport)).toBeNull();
});
it('folds a diamond where two operands share a common base (#2393)', () => {
const repo: RepoConstants = new Map([
[
'm',
mc({
literals: { BASE: '/base' },
exprs: {
P: [ref('BASE'), lit('/p')],
Q: [ref('BASE'), lit('/q')],
X: [ref('P'), ref('Q')],
},
}),
],
]);
// BASE is reached transitively via both P and Q within X's single fold.
expect(resolveConstant('m', 'X', repo, javaImport)).toBe('/base/p/base/q');
});
});