feat(resolver): extract generator functions as definitions (JS/TS)#1333
Conversation
Register function* declarations and const gen = function*() {} as
first-class function definitions in both the WASM and native engines.
Before this change, generator_function_declaration nodes were absent
from the definition registry, so:
- Calls inside generator bodies were attributed to the file level
- yield* gen8() and other inter-generator calls produced no edges
- The generators jelly-micro fixture scored 0% recall (0/9 named edges)
Changes:
- parser.ts + wasm-worker-entry.ts: add generator_function_declaration
and generator_function (var-declared) to COMMON_QUERY_PATTERNS
- extractors/javascript.ts: walk-path case, handleVariableDecl,
extractConstDeclarators skip guard, export kindMaps, return type map
- extractors/javascript.rs: equivalent changes for native engine parity
- Add generators jelly-micro fixture (9 named edges, 100% recall)
- Add 4 unit tests for generator extraction and yield* call capture
Closes #1319
docs check acknowledged
|
Claude finished @carlos-alm's task —— View job PR Review: feat(resolver): extract generator functions as definitions (JS/TS)
Root Cause AnalysisProblem Being Solved: Generator functions ( Why This Approach: Correct approach — adds Risk Assessment: Low. The change is purely additive: new node types are registered in an existing dispatch table. No existing behavior is modified. Phase 2: Configuration Change DetectionNo linting rules relaxed, no TypeScript strict flags disabled, no test timeouts or coverage thresholds changed. Clean. Phase 3: Test IntegrityTests are new, additive, and appropriately strict. No assertions weakened, no existing tests modified. One minor gap: the test at // Ideally: assert that calls[gen8].enclosingFunction === 'gen9'However the Phase 4: Code QualityWASM path (
Query patterns ( Both files' Rust engine (
One finding worth noting: The No issues found with the Rust parity. The diff is clean. Phase 5: Backlog Compliance
Critical ConcernsNone. The one thing I would flag for awareness (not blocking):
Final Assessment |
Greptile SummaryThis PR fixes a gap in JS/TS graph extraction where
Confidence Score: 5/5Safe to merge — the change is a targeted, additive extension that mirrors every existing function-type handler pattern without touching any shared logic paths. Every modified surface (query patterns, walk-path switch, variable-decl handler, export kind-map, return-type-map, Rust counterparts) receives an identical, symmetric change alongside the existing function_declaration/function_expression cases. The extractConstDeclarators skip-guard is correctly extended to prevent duplicate constant entries. Tests cover declaration-form, variable-form, call attribution with line-range containment, and yield* capture. The benchmark fixture confirms end-to-end 9/9 edge recall. No existing code paths are altered. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph "Before (0% recall)"
A1["function* gen(){}"] -->|"not recognised"| B1["No definition (file-scope attribution)"]
C1["const g = function*(){}"] -->|"not recognised"| D1["No definition (file-scope attribution)"]
end
subgraph "After this PR (100% recall)"
A2["function* gen(){}"] -->|"generator_function_declaration"| B2["Definition registered (walk + query)"]
C2["const g = function*(){}"] -->|"generator_function"| D2["Definition registered (walk + query)"]
B2 --> E2["Calls inside body scoped to generator"]
D2 --> E2
E2 --> F2["yield*/call edges emitted correctly"]
end
subgraph "Touched surfaces"
G["parser.ts COMMON_QUERY_PATTERNS"]
H["wasm-worker-entry.ts COMMON_QUERY_PATTERNS"]
I["javascript.ts walk + export + return-type-map"]
J["javascript.rs match_js_node + handle_var_decl + match_js_return_type_map + handle_export_declaration"]
end
Reviews (7): Last reviewed commit: "Merge branch 'main' into feat/generator-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it('attributes calls inside generator body to the generator', () => { | ||
| const symbols = parseJS(` | ||
| function* gen9() { yield* gen8(); } | ||
| function* gen8() { yield 1; } | ||
| `); | ||
| expect(symbols.definitions).toContainEqual(expect.objectContaining({ name: 'gen9' })); | ||
| expect(symbols.definitions).toContainEqual(expect.objectContaining({ name: 'gen8' })); | ||
| expect(symbols.calls).toContainEqual(expect.objectContaining({ name: 'gen8' })); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The test attributes calls inside generator body to the generator only confirms that gen8 appears somewhere in symbols.calls — it does not assert that the call is scoped to gen9. The original bug was that calls fell through to file-level attribution, and this test would pass even with the old (broken) behavior since the call would still appear in the flat calls list. A stronger assertion would check that no definition with a matching line range contains the call, or use the graph resolution path to confirm the gen9 → gen8 edge actually appears.
There was a problem hiding this comment.
Fixed — rewrote the test to use multi-line generator bodies so line ranges are distinct, then added line-range containment assertions: the call to gen8 must fall within gen9's [line, endLine] range (proving it is attributed to gen9's body), and must NOT fall within gen8's own range. This would have caught the original bug — with the old behavior, generators had no definitions and all calls had file-level attribution outside any definition's range.
Codegraph Impact Analysis12 functions changed → 15 callers affected across 3 files
|
|
Claude finished @carlos-alm's task —— View job Review: Generator attribution test strengthening
The fix flagged by Greptile is already implemented in commit What changed ( The test was rewritten to use multi-line generators so line ranges are non-overlapping, then added two containment assertions: // The call's line must fall within gen9's range
expect(gen8Call!.line).toBeGreaterThanOrEqual(gen9Def!.line);
expect(gen8Call!.line).toBeLessThanOrEqual(gen9Def!.endLine!);
// Negative: the call must NOT fall within gen8's own range
const callIsInsideGen8 = gen8Call!.line >= gen8Def!.line && ...;
expect(callIsInsideGen8).toBe(false);This is exactly what was requested. With the old (broken) behavior, generators had no definitions, so all calls had file-level attribution and Status: No changes needed — the fix is already in the branch and addresses the review feedback. |
1 participant
Summary
function* gen() {}declarations were invisible to the graph — not registered as definitions, so calls inside generator bodies were attributed to the file level andyield* otherGen()edges were never emittedgenerator_function_declarationandgenerator_function(variable-declared) to query patterns in bothparser.tsandwasm-worker-entry.ts; mirror in the walk path and all export/return-type-map handlersmatch_js_node,handle_var_decl,match_js_return_type_map,handle_export_declarationjelly-micro/generatorsfixture: 9 named edges, 100% recall after fix (was 0%)Test plan
npx vitest run tests/parsers/javascript.test.ts— 4 new generator tests, all passnpx vitest run tests/benchmarks/resolution/jelly-micro.test.ts— generators fixture: 100% recall (9/9 TP, 0 FP, 0 FN)npx vitest run --exclude "tests/benchmarks/**"— full suite green (2656 tests)Closes #1319