Deferred from PR #1917 review.
Original reviewer comment: #1917 (comment)
Context:
emitDirectCallEdgesForCall in src/domain/graph/builder/stages/build-edges.ts gates the dyn=0→dyn=1 upgrade (for a dynamicKind-tagged call referring to an already-recorded target) on callLine < dynZeroEntry.line — the incoming call must be strictly earlier than the recorded one.
Greptile correctly identified a real gap: when a bare decorator and its call-expression sibling are on the exact same source line (e.g. @Log @Log() class Foo {}), the query phase collects @Log() first (dyn=0), and the walk phase collects the bare @Log afterward (dynamicKind=reflection) — but since both are on line N, N < N is false and the upgrade never happens. Native's single-pass source-order walk would correctly keep this as dyn=1, so this is a real WASM/native divergence.
Why the suggested fix (<=) is unsafe: Changing the comparison to callLine <= dynZeroEntry.line fixes the decorator case above, but reintroduces #1687 for the symmetric case: f(); f.call({}); written on a single line. Both the direct f() call and the .call() reflection call are collected in the same query phase (unlike decorators), so .call/.apply/.bind never legitimately need the upgrade when on a later or equal line — that's a genuinely separate, later reference to the same target, and native's dedup (first-recorded-wins) correctly leaves it dyn=0. With N <= N also true here, <= would incorrectly flip this back to dyn=1, undoing the #1687 fix for the same-line case.
Root cause: Call.line (src/types.ts) only carries line granularity, not column/byte offset, and there's no signal distinguishing "collected in the query phase" from "collected in the walk-phase, appended out of order" (see the two-phase collection note in buildFileCallEdges, #1683). Neither < nor <= alone can correctly resolve both scenarios from line alone, since they collide on the exact same value.
Possible fixes for whoever picks this up:
- Add column/byte-offset granularity to
Call (bigger blast radius — touches every extractor + the native mirror + types.ts), or
- Thread an explicit "collected in walk-phase" boolean/tag through to
emitDirectCallEdgesForCall so the upgrade only loosens to <= for walk-phase-sourced entries (decorators), keeping strict < for query-phase entries (.call/.apply/.bind).
Option 2 is likely the smaller, more targeted fix.
Deferred from PR #1917 review.
Original reviewer comment: #1917 (comment)
Context:
emitDirectCallEdgesForCallinsrc/domain/graph/builder/stages/build-edges.tsgates the dyn=0→dyn=1 upgrade (for a dynamicKind-tagged call referring to an already-recorded target) oncallLine < dynZeroEntry.line— the incoming call must be strictly earlier than the recorded one.Greptile correctly identified a real gap: when a bare decorator and its call-expression sibling are on the exact same source line (e.g.
@Log @Log() class Foo {}), the query phase collects@Log()first (dyn=0), and the walk phase collects the bare@Logafterward (dynamicKind=reflection) — but since both are on line N,N < Nis false and the upgrade never happens. Native's single-pass source-order walk would correctly keep this as dyn=1, so this is a real WASM/native divergence.Why the suggested fix (
<=) is unsafe: Changing the comparison tocallLine <= dynZeroEntry.linefixes the decorator case above, but reintroduces #1687 for the symmetric case:f(); f.call({});written on a single line. Both the directf()call and the.call()reflection call are collected in the same query phase (unlike decorators), so.call/.apply/.bindnever legitimately need the upgrade when on a later or equal line — that's a genuinely separate, later reference to the same target, and native's dedup (first-recorded-wins) correctly leaves it dyn=0. WithN <= Nalso true here,<=would incorrectly flip this back to dyn=1, undoing the #1687 fix for the same-line case.Root cause:
Call.line(src/types.ts) only carries line granularity, not column/byte offset, and there's no signal distinguishing "collected in the query phase" from "collected in the walk-phase, appended out of order" (see the two-phase collection note inbuildFileCallEdges, #1683). Neither<nor<=alone can correctly resolve both scenarios fromlinealone, since they collide on the exact same value.Possible fixes for whoever picks this up:
Call(bigger blast radius — touches every extractor + the native mirror +types.ts), oremitDirectCallEdgesForCallso the upgrade only loosens to<=for walk-phase-sourced entries (decorators), keeping strict<for query-phase entries (.call/.apply/.bind).Option 2 is likely the smaller, more targeted fix.