Commit graph

2 commits

Author SHA1 Message Date
henry201605
8bef64baa1
feat(ingestion/routes): give Route nodes a (method, url) identity (#2289) (#2302)
* feat(ingestion/routes): give Route nodes a (method, url) identity (#2289)

Route node identity was URL-only, so a same-URL multi-verb pair
(GET /x + POST /x) collapsed into a single node and the second verb's
handler and execution flow were silently lost. Route identity is now
(method, url) via routeNodeKey(method, url): a known, specific verb keys
as "METHOD url", while a method-less route (filesystem routes — Next.js /
Expo / PHP — and Laravel resource/apiResource) or a wildcard "*" route
(e.g. Django function views) falls back to URL-only. The fallback is
byte-identical to the previous URL-only ids, so only genuine
declaration-style multi-verb routes split into separate nodes.

The identity key is shared across the three phases that must agree on the
Route node id:
- routes phase: registry key + node id + handler-symbol lookup; the Route
  node still carries the bare URL as its display name.
- call-processor: resolveRouteHandlerSymbols re-keyed by identity so each
  verb resolves its own handler; a verb-less fetch() consumer matches by
  URL and connects to every Route node at that URL (one per verb).
- processes phase: ENTRY_POINT_OF targets the identity-keyed node id.

Bumps INCREMENTAL_SCHEMA_VERSION 4 -> 5: persisted pre-v5 Route nodes use
the old url-only ids, so an incremental top-up would strand them alongside
new composite-keyed nodes — force a full re-analyze instead.

Part of #2280.

* fix(ingestion/routes): address PR #2302 review (P1/P2/P3)

P1 — Schema v5 fast-path bypass (run-analyze.ts):
  Adds a schemaVersion-mismatch guard above the alreadyUpToDate early-return,
  mirroring the pdgModeMismatch slot. Without it, a same-commit re-analyze on
  a pre-v5 stamp returned alreadyUpToDate without ever reaching the
  isIncremental gate, defeating the v5 schema bump's migration intent.
  Regression test covers: analyze (stamps v5) → meta downgrade to v4 → same
  commit re-analyze must NOT early-return and meta restamps to v5.

P2 — ENTRY_POINT_OF handler-aware linking (processes.ts):
  Pre-fix routesByFile fanned every same-file Route to every same-file
  process, cross-wiring same-file GET/POST handlers. Now reads handlerSymbolId
  off the Route graph node (the source of truth routes.ts stamps) into
  routesByHandlerId, with a routesWithoutHandlerByFile fallback — mirrors the
  Tool linking precedent 10 lines below. Two regression tests: weak form
  (only one handler has a process; sibling verb does not get spuriously
  attached) and strong form (both handlers form distinct processes; each
  Route links to exactly its own entryPoint, 2 edges not pre-fix 4).

P2 — Roundtrip composite-id (route-{method,handler-symbol}-roundtrip):
  Both tests now seed the Route node with
  generateId('Route', routeNodeKey('POST', '/api/orders')) and run the
  Cypher MATCH against the composite id, exercising the literal-space-in-id
  through CSV→COPY→HANDLES_ROUTE_QUERY. A space-in-id escape regression
  would surface here instead of being silently swallowed by the extractor's
  catch.

P3 — doc-drift + test if:
  - route-path.ts:4 — header updated to "(method, url) via routeNodeKey"
  - java.ts:684 — drop "Route nodes are URL-keyed"; #2289 closes that gap
  - manifest-extractor.ts:196 — explicit that Route node *id* is composite
    while route.name remains the bare URL
  - multi-verb-route-identity.test.ts:88 — forEachRelationship+if rewritten
    as a .filter().map() chain (no test-level conditional). New
    route-process-linking tests are also if-free.

Validation: tsc clean, prettier clean, 9 touched suites / 43 tests pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(ingestion/routes): drop routes.ts re-export, fix CI fast-path tests

Two follow-ups on PR #2302's CHANGES_REQUESTED review:

1. Drop `routes.ts` re-export of `normalizeExtractedRoutePath` /
   `normalizeRouteMethod` / `routeNodeKey` (per @magyargergo's inline
   comment at routes.ts:153 — the symbols already live in
   `route-extractors/route-path.ts` and consumers should import them
   from the source, not via a routes-phase indirection that was kept
   only as a compat shim during the #2289 refactor). Updated the two
   remaining callers (blade-template-routes / spring-route-extractor-
   parity tests) to import directly from `route-extractors/route-path.js`.
   `call-processor.ts` and `processes.ts` already import from the source.

2. Fix two `run-analyze.test.ts` fast-path tests that started failing
   on CI after the schema-version mismatch guard landed (
   "creates .gitnexus/.gitignore on the already-up-to-date fast path"
   and "reports isPrimaryBranch false for an up-to-date non-primary
   branch"). The test fixtures hand-built a RepoMeta with NO
   schemaVersion field; with the guard now checking
   `existingMeta.schemaVersion !== INCREMENTAL_SCHEMA_VERSION`, that
   pre-versioning shape was treated as a mismatch and forced a rebuild,
   short-circuiting the fast path the tests exercise. Stamp the current
   schemaVersion on those fixtures so they reflect the post-#2289 meta
   shape production actually writes (`runFullAnalysis` always stamps
   the field on git repos — see meta save site).

Validation: tsc clean, prettier clean, 11 touched suites / 80 tests pass.

---------

Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-26 07:59:46 +01:00
henry201605
a691dcb320
feat(routes): persist HTTP method on Route nodes (#2138 part 1/2) (#2234)
* feat(routes): persist HTTP method on Route nodes

Part 1 of 2 for issue #2138 (skip redundant HTTP provider source-scan).

The ingestion routes phase already knows each route's HTTP verb —
`ExtractedRoute.httpMethod` (Spring/Laravel framework routes) and
`ExtractedDecoratorRoute.httpMethod` (decorator routes) — but dropped it
when creating the Route graph node. As a result `HttpRouteExtractor`'s
graph-assisted path could not recover the verb for `framework-route`
sources (whose edge `reason` is undecodable by `methodFromRouteReason`)
and had to fall back to re-scanning the handler source.

Changes:
- routes phase: carry `httpMethod` into `RouteEntry` and persist it as
  `Route.method` (filesystem-derived Next.js/Expo/PHP routes have no
  structural verb, so they stay method-less).
- HttpRouteExtractor: HANDLES_ROUTE query now returns `route.method`;
  `extractProvidersGraph` prefers it and falls back to the edge reason
  for older indexes / method-less routes (fail-open, fully backward
  compatible).
- tests: graph-method precedence, multi-verb handler disambiguation via
  the persisted verb, case normalization, and old-index fallback.

This change is intentionally NOT a performance optimization on its own:
the graph path still parses handler files to recover the handler *name*.
Eliminating that parse (and thus the redundant source-scan #2138 targets)
requires linking HANDLES_ROUTE to the handler symbol, which lands in
Part 2. This PR is the data-completeness groundwork for that.

Refs #2138

* test: account for new Route.method in blade route-registry assertion

The routes phase now persists httpMethod onto RouteEntry/Route nodes, so
the strict toEqual on the framework-route registry entry must include the
new method field.

* fix(routes): persist Route.method end-to-end + real-lbug round-trip test

Addresses review on #2234 (magyargergo + tri-review): the prior commit
read `route.method` in HANDLES_ROUTE_QUERY but never added the column to
the schema/persistence path, so against a real LadybugDB the query failed
to bind (`Cannot find property method for r.`) and the `catch { return [] }`
silently swallowed it — regressing the graph-assisted HTTP provider path.

- schema: add `method STRING` to ROUTE_SCHEMA.
- csv-generator: write `method` in the Route CSV row (header + row, column
  order aligned with the COPY statement).
- lbug-adapter: add `method` to getCopyQuery('Route').
- routes phase: normalizeRouteMethod() canonicalizes the verb to upper-case
  and skips non-verbs — Laravel resource/apiResource carry httpMethod
  values like `resource`/`apiResource`, which must not land a junk method.
- http-route-extractor: log at debug when the HANDLES_ROUTE / FETCHES graph
  query throws, so a total graph-provider outage is observable instead of
  silently swallowed. Export HANDLES_ROUTE_QUERY for the round-trip test.
- tests: add a real-lbug round-trip (graph -> CSV -> COPY -> HANDLES_ROUTE_QUERY)
  asserting the verb persists and reads back; update the blade registry
  assertion for the normalized (upper-case) method.

Refs #2138

* fix(csv): coerce Route.method to string for escapeCSVField typecheck

node.properties.method is typed unknown (not a declared property), so
`x || ''` stayed unknown and failed tsc against escapeCSVField's
string|number param. Coerce explicitly with String(... ?? '').

* test(bench): regenerate emit-persistence fingerprint for Route.method column

Adding the method column to route.csv changes the byte-identity
fingerprint of the emit-persistence benchmark (the synthetic graph's
route.csv header now includes 'method'). scaling_ratio unchanged (~0.9,
linear); this is the documented regenerate-on-legitimate-emit-change
path. Streaming baseline (BasicBlock/PDG) is unaffected.

---------

Co-authored-by: henry <zhangwei2017@unipus.cn>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-06-20 06:30:16 +01:00