Deferred from PR #2034 review.
Original reviewer comment: inline comment — #2034 (comment)
Context:
PR #2034 added a liveness check for object-literal-property value-refs ({ resolve: someFn }): the referenced function only gets a calls edge when the property key is independently confirmed to be invoked via member-call syntax (x.resolve(...)) somewhere in the files being processed (collectInvokedPropertyNames in src/domain/graph/builder/call-resolver.ts, mirrored in crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs).
On a full build, "the files being processed" is the whole codebase, so this is exact. On an incremental rebuild (src/domain/graph/builder/incremental.ts's buildCallEdges), it is scoped to only the single file currently being rebuilt. If factory.js defines { resolve: neverCalled } and the only table.resolve(1) call site lives in consumer.js, an incremental rebuild triggered solely on factory.js will not see resolve in invokedPropertyNames and will drop the edge — classifying neverCalled as dead even though it is genuinely reachable. A subsequent full rebuild corrects this, but a dead-code CI check running against an incremental watch build could see a transient false positive.
This is the same category of trade-off already made elsewhere in this codebase's incremental role classification (hasActiveFileSiblings and exported-via-reexport reachability both scope to an affected-file subset rather than the whole graph in crates/codegraph-core/src/graph/classifiers/roles.rs's incremental path — verified by direct code reading, see do_classify_incremental, find_neighbour_files, is_barrel_prod_reachable), so it was accepted as-is for #2034 rather than blocking that PR.
What the fix entails: to close this gap without falling back to a full-codebase scan on every incremental rebuild, the codebase would need a small persistent index — e.g. a invoked_property_names table (or a column/marker on existing calls-shaped edges) that incremental rebuilds insert into/prune from as files are added, changed, or removed, so collectInvokedPropertyNames can query evidence across the whole graph in O(lookup) time rather than only the currently-parsed file(s). This is a real schema/incremental-maintenance change (new persisted state + insert/prune logic on both engines), not a one-line tweak, so it's being tracked separately rather than folded into #2034.
Deferred from PR #2034 review.
Original reviewer comment: inline comment — #2034 (comment)
Context:
PR #2034 added a liveness check for object-literal-property value-refs (
{ resolve: someFn }): the referenced function only gets acallsedge when the property key is independently confirmed to be invoked via member-call syntax (x.resolve(...)) somewhere in the files being processed (collectInvokedPropertyNamesinsrc/domain/graph/builder/call-resolver.ts, mirrored incrates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs).On a full build, "the files being processed" is the whole codebase, so this is exact. On an incremental rebuild (
src/domain/graph/builder/incremental.ts'sbuildCallEdges), it is scoped to only the single file currently being rebuilt. Iffactory.jsdefines{ resolve: neverCalled }and the onlytable.resolve(1)call site lives inconsumer.js, an incremental rebuild triggered solely onfactory.jswill not seeresolveininvokedPropertyNamesand will drop the edge — classifyingneverCalledas dead even though it is genuinely reachable. A subsequent full rebuild corrects this, but a dead-code CI check running against an incremental watch build could see a transient false positive.This is the same category of trade-off already made elsewhere in this codebase's incremental role classification (
hasActiveFileSiblingsand exported-via-reexport reachability both scope to an affected-file subset rather than the whole graph incrates/codegraph-core/src/graph/classifiers/roles.rs's incremental path — verified by direct code reading, seedo_classify_incremental,find_neighbour_files,is_barrel_prod_reachable), so it was accepted as-is for #2034 rather than blocking that PR.What the fix entails: to close this gap without falling back to a full-codebase scan on every incremental rebuild, the codebase would need a small persistent index — e.g. a
invoked_property_namestable (or a column/marker on existingcalls-shaped edges) that incremental rebuilds insert into/prune from as files are added, changed, or removed, socollectInvokedPropertyNamescan query evidence across the whole graph in O(lookup) time rather than only the currently-parsed file(s). This is a real schema/incremental-maintenance change (new persisted state + insert/prune logic on both engines), not a one-line tweak, so it's being tracked separately rather than folded into #2034.