Skip to content

feat(#72): SPI v1 slice 1c-ii.e — inline arrow handler synthesis#110

Merged
mohanagy merged 1 commit into
mainfrom
feat/spi-v1-slice-1c-ii.e-express-inline
May 11, 2026
Merged

feat(#72): SPI v1 slice 1c-ii.e — inline arrow handler synthesis#110
mohanagy merged 1 commit into
mainfrom
feat/spi-v1-slice-1c-ii.e-express-inline

Conversation

@mohanagy
Copy link
Copy Markdown
Owner

@mohanagy mohanagy commented May 11, 2026

Builds on slices 1c-ii.b/c/d (Express factory/route/middleware). Closes the synthesis gap left open by 1c-ii.c and 1c-ii.d.

Summary

When the route/middleware argument is an `ArrowFunction` or `FunctionExpression` instead of an `Identifier` — the canonical Express idiom `app.get('/p', (req, res) => {...})` — the detector now mints a synthetic `SpiSymbol` for the anonymous callback.

Synthesis property Value
`kind` `'function'` (closest `SpiSymbolKind` for a callable)
`id` `symbol:<file_id>/function/..L` — deterministic across builds
`range` the handler node's own source range
`framework_role` `'express_route'` for HTTP methods, `'express_middleware'` for `.use(...)`
`exported` `false` (anonymous callbacks are never exported)

Implementation notes

  • `DetectExpressFrameworkContext` grows a `symbols: SpiSymbol[]` field — the flat list buildSpi sorts and returns. Without this, synthesized symbols would live only in the per-file map and never make it back into the final SemanticProgramIndex. `build.ts` passes the parent `symbols` array through.
  • Route synthesis emits a `route_handler` edge from binding to synthetic symbol (parity with named-handler routes). Middleware synthesis emits no edge (parity with named-handler middleware's tag-only design).
  • The receiver-identity check from slice 1c-ii.c still gates synthesis: an inner-scope shadowed local won't trigger synthesis even if its inline argument is an arrow.

What's deferred to slice 1c-ii.f (v0.14.0 trigger)

  • `route_path` metadata on synthetic symbols. Requires extending `SpiSymbol` with a `framework_metadata?: Record<string, unknown>` field (open question 3 in the SPI v1 design doc).
  • Mounted-router prefix resolution: `app.use('/api', usersRouter)` should propagate the `/api` prefix down to every route registered on `usersRouter`.
  • Full byte-equivalent port of the remaining surface in `extract/frameworks/express.ts` for demo-repo parity.

Test plan

  • `npm run typecheck` — clean
  • `npm run build` — clean
  • `npm run test:run` — 94 files / 1636 tests pass (7 new for synthesis + 1629 pre-existing)
  • Tests cover: arrow synthesis with deterministic name pattern, `route_handler` edge emission, multiple inline handlers in the same file (distinct symbols), plain function-expression handlers, inline middleware synthesis, shadowed-receiver-still-skips, end-to-end projection (synthetic symbol → ExtractionNode with `framework: 'express'`, `node_kind: 'route'`).
  • Two pre-existing 1c-ii.c/d "deferred to 1c-ii.e" skip-assertions are flipped to assert the now-delivered behavior. Same prompts, inverted expectations.
  • CI must pass on Ubuntu/macOS/Windows matrix before merge.
  • Note: Windows-Node-20 timeout flakiness from PR feat(#72): SPI v1 slice 1c-ii.d — Express middleware detection #109 may recur; not caused by this change.

Refs #72, #107, #108, #109.

Summary by CodeRabbit

  • New Features

    • Enhanced detection of inline route handlers and middleware callbacks in Express applications. Inline arrow functions and function expressions are now properly recognized and indexed, providing more complete framework mapping.
  • Tests

    • Updated test suite to validate synthetic symbol generation for inline Express handlers.

Review Change Stack

Closes the synthesis gap left open by slices 1c-ii.c and 1c-ii.d. When the route/middleware argument is an ArrowFunction or FunctionExpression instead of an Identifier — the canonical Express idiom (app.get('/p', (req, res) => {...})) — the detector now mints a synthetic SpiSymbol for the anonymous callback.

Synthesis details:

* kind: 'function' (closest SpiSymbolKind for a callable). * id: symbol:<file_id>/function/<binding>.<method>.L<line>. Deterministic across builds for the same source. Collisions at the same line on the same binding/method are rare; if encountered the existing symbol is reused. * range: the handler node's own source range. * framework_role: 'express_route' for HTTP method calls, 'express_middleware' for app.use(...). * Pushed to BOTH the flat ctx.symbols list (so buildSpi's sort/return picks it up) and ctx.symbolsByFile[fileId] (so the per-file index stays consistent).

DetectExpressFrameworkContext gains a symbols: SpiSymbol[] field. build.ts passes the parent symbols array through. Without this, synthesized symbols would only live in the per-file map and never make it back into the final SemanticProgramIndex.

Edge emission for synthetic routes: same route_handler edge from the binding to the synthetic symbol, with confidence 'high' and source 'framework-decorator'. Middleware synthesis emits no edge (parity with slice 1c-ii.d's tag-only design).

Lexical-shadow defense still applies: the receiver-identity check from slice 1c-ii.c gates whether we even attempt synthesis. If the receiver is an inner-scope shadowed local, no synthesis happens.

Tests: 7 new in the inline-handler-synthesis suite. Cover arrow synthesis with deterministic name pattern, route_handler edge emission, multiple inline handlers in the same file (each gets its own symbol), plain function-expression handlers, inline middleware synthesis, shadowed-receiver still skips, end-to-end projection (synthetic symbol → ExtractionNode with framework=express, node_kind=route).

Plus 2 pre-existing 'inline handler is deferred' tests updated to assert the now-delivered behavior (they were skip-assertions that became inverted once slice 1c-ii.e closed the gap).

What's left for slice 1c-ii.f (the v0.14.0 trigger): route_path metadata on synthetic symbols (requires extending SpiSymbol with a framework_metadata field), mounted-router prefix resolution (app.use('/api', usersRouter) → routes mounted at /api/...), full byte-equivalent port of the remaining surface in extract/frameworks/express.ts.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

📝 Walkthrough

Walkthrough

PR #110 extends Express framework detection in the SPI v1 type-checker phase to synthesize deterministic SpiSymbol instances for inline route handlers and middleware callbacks. The context now receives the full symbols array; middleware and route detectors mint synthetic symbols for inline expressions, tag them appropriately, and insert them into both the flat symbols list and per-file index. Tests verify synthesis, deterministic naming, distinct symbols per handler, and correct projection to extraction nodes.

Changes

Express inline handler synthesis

Layer / File(s) Summary
Context type update
src/pipeline/spi/framework-express.ts
DetectExpressFrameworkContext gains a symbols: SpiSymbol[] field for storing synthesized inline-handler symbols in lockstep with symbolsByFile.
Context threading in build phase
src/pipeline/spi/build.ts
detectExpressFramework call in addTypeCheckerEdges now receives the full symbols array alongside sourceFile, fileId, edges, and checker.
Synthetic symbol creation infrastructure
src/pipeline/spi/framework-express.ts
New mintSyntheticHandlerSymbol and rangeOfNode helpers create deterministic function-kind SpiSymbols from file id, binding name, method, and start line; symbols are inserted into ctx.symbols and ctx.symbolsByFile with re-use detection.
Middleware inline callback synthesis
src/pipeline/spi/framework-express.ts
emitMiddlewareForCall resolves Identifier arguments as before and mints synthesized SpiSymbols for arrow/function-expression arguments, tagging them as express_middleware.
Route inline handler synthesis
src/pipeline/spi/framework-express.ts
emitRouteForCall resolves named-handler Identifiers (existing behavior) or mints synthetic symbols for inline arrow/function expressions; creates route_handler edges from the binding symbol.
Role assignment refinement
src/pipeline/spi/framework-express.ts
Middleware framework-role assignment condition tightened to proceed only when framework_role is unset or explicitly express_middleware, preventing overwrites of other express_* roles.
Test updates and synthesis validation
tests/unit/spi-framework-express.test.ts
Inline route handler and middleware tests updated to assert synthesis behavior. New test block verifies synthetic symbol minting, deterministic naming, distinct symbols for multiple handlers, plain function expression support, middleware synthesis, suppression when receiver is shadowed, and correct ExtractionNode projection (framework=express, framework_role=express_route, node_kind=route).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • mohanagy/graphify-ts#109: Main PR extends middleware detection logic that PR #109 introduced, adding full symbols-array threading and deterministic inline-handler synthesis.
  • mohanagy/graphify-ts#108: Main PR builds directly on PR #108's framework-detector signature and route-handler resolution, adding symbols parameter and inline-handler minting infrastructure.
  • mohanagy/graphify-ts#106: Main PR synthesizes and tags framework_role metadata that PR #106's projector consumes to set ExtractionNode framework annotations.

Poem

🐰 Inline handlers, now they're born,
Synthetic symbols, neat and warm,
From arrow code, we mint them clear,
Deterministic names appear!
Route and middleware, tagged just right,
The graph is growing, burning bright. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically refers to the main feature: SPI v1 slice 1c-ii.e implementing inline arrow handler synthesis for Express framework detection.
Description check ✅ Passed The description is comprehensive and well-structured, covering summary, implementation details, test plan, and deferred work. It addresses the template requirements though formatted differently.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/spi-v1-slice-1c-ii.e-express-inline

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/pipeline/spi/framework-express.ts (1)

264-313: 💤 Low value

Potential ID collision for multiple inline handlers on the same line.

The deterministic ID symbol:${fileId}/function/${binding}.${method}.L${line} will collide if two distinct inline handlers appear on the same line with the same binding and method (e.g., app.get("/a", ()=>{}); app.get("/b", ()=>{}) on one line). The comment acknowledges this but states "the second push is a no-op thanks to the seenEdgeKeys dedupe downstream" — however, mintSyntheticHandlerSymbol itself returns the first symbol for the second handler (line 294-296), meaning both route registrations point to the same SpiSymbol.

This is an accepted edge case per the comment, but worth noting that the two callbacks will share identity in the SPI. If column-level disambiguation is ever needed, the ID scheme would need adjustment.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pipeline/spi/framework-express.ts` around lines 264 - 313, The current
deterministic id in mintSyntheticHandlerSymbol can collide for multiple inline
handlers on the same line; fix by incorporating column info into the synthetic
symbol name/id (e.g., include range.start.character or a small hash of
handler.pos) so each handler on the same line gets a unique id; update the
constructed name and id (the variables name and id inside
mintSyntheticHandlerSymbol) to append the column (or short hash) and keep the
rest of the logic (symbolsByFile dedupe and framework_role update) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/pipeline/spi/framework-express.ts`:
- Around line 264-313: The current deterministic id in
mintSyntheticHandlerSymbol can collide for multiple inline handlers on the same
line; fix by incorporating column info into the synthetic symbol name/id (e.g.,
include range.start.character or a small hash of handler.pos) so each handler on
the same line gets a unique id; update the constructed name and id (the
variables name and id inside mintSyntheticHandlerSymbol) to append the column
(or short hash) and keep the rest of the logic (symbolsByFile dedupe and
framework_role update) unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 849fd075-be77-436d-ab53-519a598ec724

📥 Commits

Reviewing files that changed from the base of the PR and between fdd3a1f and 2dd1de8.

📒 Files selected for processing (3)
  • src/pipeline/spi/build.ts
  • src/pipeline/spi/framework-express.ts
  • tests/unit/spi-framework-express.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant