{{ message }}
fix(roles): classify methods and functions with active siblings as leaf#1602
Merged
Conversation
Three call-site patterns cause false-positive dead-unresolved classification
when codegraph cannot trace the call relationship:
1. Interface dispatch via conditional property access:
`if (v.enterFunction) v.enterFunction(fn, ctx)` — the call resolves to
the property accessor, not the concrete method implementation, so the
method gets fanIn=0. Affects enterFunction/exitFunction/enterNode/exitNode/
finish methods in cfg-visitor.ts, complexity-visitor.ts, dataflow-visitor.ts.
2. Logical-or function defaults:
`const fn = options._fetchLatest || fetchLatestVersion` — the function
reference is a value reference, not a call site, so no call edge is produced.
Requires fanOut>0 as evidence the function is non-trivial. Affects
fetchLatestVersion in update-check.ts.
3. Handler-table object property callbacks:
`{ handle: handleReturn }` — the function stored as a property value
generates no call edge when dispatched via `h.handle(...)`.
Fix: Extend the existing hasActiveFileSiblings heuristic (previously only
applied to constant kind) to also cover method and function kinds. When the
same file contains at least one callable connected to the graph:
- method kinds: always classified as leaf (interface implementations)
- function kinds with fanOut>0: classified as leaf (non-trivial helpers
used as value references)
The fanOut>0 guard for function kinds prevents genuinely inert dead functions
(fanOut=0) from being promoted to leaf, preserving dead-code detection for
the common case.
Both the TypeScript classifier and the mirrored Rust classifier are updated.
Also updates a stale unit test that expected dead-entry for execute() in CLI
files, which was corrected to entry by #1585.
Closes #1586
-- docs check acknowledged
Contributor
…cture.ts buildActiveFilesSet now returns two sets: activeFiles (fan_in>0 || fan_out>0) for annotation-only kinds, and calledActiveFiles (fan_in>0 only) for method/ function kinds. A function or method with fanIn=0, fanOut>0 as the sole callable in its file no longer adds its own file to the active set and thus no longer promotes itself to leaf.
… classifier compute_active_files now returns (active_files, called_active_files): the first uses fan_in>0||fan_out>0 for annotation-only kinds; the second uses fan_in>0 only. classify_rows passes called_active_files for method/function has_active_siblings checks, preventing a function or method with fan_in=0, fan_out>0 as the sole callable in its file from promoting itself to leaf.
…negative Two new integration-path tests verify that a function or method with fanIn=0, fanOut>0 as the sole callable in its file is classified as dead-unresolved rather than leaf, covering the scenario identified in the Greptile review.
…est) Merge resolves three conflicts: - roles.rs: adopt ANNOTATION_ONLY_KINDS constant from main's refactor while keeping the two-set compute_active_files return value from this branch - tests/graph/classifiers/roles.test.ts: keep both the improved test name/comment and the non-Commander dead-entry boundary test from this branch - tests/unit/roles.test.ts: keep both the self-sibling boundary tests from this branch and the incremental-path #1583 test from main
Contributor
Author
|
Addressed: P1 self-sibling false-negative in both TS and Rust classifiers Both bugs identified in the Greptile review have been fixed: P1 — The function now returns two sets instead of one:
A function or method with fanIn=0, fanOut>0 as the sole callable in its file no longer adds its own file to the active set, so it stays P1 — Same fix mirrored symmetrically: New boundary tests Two new integration-path tests added to
All 63 roles tests pass (unit + graph classifier + integration). |
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the
hasActiveFileSiblingsheuristic in the role classifier to covermethodandfunctionkinds, eliminating three false-positivedead-unresolvedpatterns discovered during Titan audit phase (grind):Pattern 2 — Interface dispatch via conditional property access: Methods implementing interfaces like
Visitorare dispatched viaif (v.enterFunction) v.enterFunction(fn, ctx). Codegraph resolves the call to the property accessor, not the concrete method implementation, so the method getsfanIn=0. Affected:enterFunction,exitFunction,enterNode,exitNode,finishincfg-visitor.ts,complexity-visitor.ts,dataflow-visitor.ts.Pattern 3 — Logical-or function defaults:
const fn = options._fetchLatest || fetchLatestVersion— a function reference as a fallback value is not a call site, so no call edge is produced. Affected:fetchLatestVersioninupdate-check.ts. AfanOut > 0guard ensures only non-trivial helpers (those that call something) are promoted, preserving detection of genuinely inert dead functions.Pattern 1 — Handler-table object property callbacks (partially covered): Functions stored as object property values and dispatched via
h.handle(...)also benefit when the file has active siblings.Both the TypeScript classifier (
src/graph/classifiers/roles.ts+src/features/structure.ts) and the mirrored Rust classifier (crates/codegraph-core/src/graph/classifiers/roles.rs) are updated symmetrically.Also fixes a stale test that expected
dead-entryforexecute()in CLI command files — corrected toentryby bug(roles): Commander.js action callbacks (command.execute, command.validate) classified as dead-entry #1585 but the test was not updated.Test plan
classifyRolesunit tests pass (30 existing + 5 new + 1 updated for bug(roles): Commander.js action callbacks (command.execute, command.validate) classified as dead-entry #1585 correction)classifyNodeRolesintegration tests passCloses #1586