Skip to content

docs(roles): correct hasActiveFileSiblings JSDoc to match population logic#2044

Open
carlos-alm wants to merge 1 commit into
fix/issue-1909-lua-wasm-extractor-missing-eval-computed-keyfrom
fix/issue-1911-stale-doc-comment-roleclassificationnode
Open

docs(roles): correct hasActiveFileSiblings JSDoc to match population logic#2044
carlos-alm wants to merge 1 commit into
fix/issue-1909-lua-wasm-extractor-missing-eval-computed-keyfrom
fix/issue-1911-stale-doc-comment-roleclassificationnode

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • RoleClassificationNode.hasActiveFileSiblings (src/graph/classifiers/roles.ts) had a JSDoc comment claiming the field is "populated only for constant and TYPE_DEF_KINDS nodes; undefined for regular callables (functions, methods, classes, etc.)".
  • That's stale: buildClassifierInput in src/features/structure.ts also populates it for method/function kinds, using a different source set (calledActiveFiles, gated on fanIn > 0 rather than fanIn > 0 || fanOut > 0) to avoid a self-sibling false positive where a function would count its own fan-out as evidence of an "active sibling".
  • The stale comment made the function/method-specific branches in classifyUnreferencedNode (the fanOut > 0 rescue heuristic from Dispatch-table function references (resolve: fn) inconsistently flagged dead-unresolved depending on unrelated fanOut #1771) read as unreachable dead code to a future reader, when they are not.
  • Rewrote the JSDoc to describe both population groups and their distinct source sets accurately. No mirrored Rust doc comment exists for this (the native side takes has_active_file_siblings as a plain function parameter, not a documented struct field), so this is TS-only.

Test plan

  • Added a regression test (tests/unit/roles.test.ts) exercising the real population path (classifyNodeRolesbuildActiveFilesSet/buildClassifierInput) for the positive method/function promotion case — a function with fanIn=0, fanOut>0 gets promoted to leaf when the file has a genuine other active sibling (fanIn>0). This complements the existing self-sibling negative-boundary tests (bug(roles): three untraced call-site patterns cause dead-unresolved false positives (handler-table, conditional-interface-dispatch, logical-or default) #1586) which only covered the "must NOT promote itself" case.
  • npx vitest run tests/unit/roles.test.ts tests/graph/classifiers/roles.test.ts — 69/69 pass
  • npm run lint — clean on touched files
  • npm run build — clean
  • npm test (full suite) — 3610 passed, 6 failed, all 6 failures (plus 19 "failed" test files) exclusively due to a pre-existing, unrelated environment condition in this fresh worktree: Cannot find module '@optave/codegraph-darwin-arm64' (native addon not built/linked for this worktree — a known gotcha, not something this docs-only change touches). No failure is related to role classification; codegraph diff-impact --staged confirms the change is isolated to the JSDoc comment.

Fixes #1911

…logic

RoleClassificationNode.hasActiveFileSiblings claimed to be populated only
for constant/TYPE_DEF_KINDS nodes, undefined for regular callables. In
reality buildClassifierInput (features/structure.ts) also populates it
for method/function kinds via a separate self-sibling-safe source set
(calledActiveFiles), which classifyUnreferencedNode relies on to rescue
functions/methods referenced only as values (logical-or defaults,
interface-dispatch methods). The stale comment made those branches read
as dead code.

Added a regression test exercising the real population path (not just
the pure classifier) for the positive method/function promotion case,
complementing the existing self-sibling negative-boundary tests.
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR corrects a stale JSDoc comment on RoleClassificationNode.hasActiveFileSiblings in src/graph/classifiers/roles.ts to accurately document both population groups (constant/TYPE_DEF_KINDS and method/function), including their distinct source sets and the rationale for the narrower fanIn > 0-only check for callables. A regression test is added to cover the positive promotion case (a function with fanIn=0, fanOut>0 being promoted to leaf when a genuine active sibling exists in the same file).

Confidence Score: 5/5

Safe to merge — the change is a JSDoc correction and a complementary regression test with no logic modifications.

The rewritten JSDoc accurately reflects the implementation in buildActiveFilesSet/buildClassifierInput (verified against the source). The new test covers the previously undocumented positive promotion case and follows the same setup conventions as adjacent tests. No production logic is touched.

No files require special attention.

Important Files Changed

Filename Overview
src/graph/classifiers/roles.ts JSDoc-only change: rewrites the hasActiveFileSiblings comment to accurately describe both population groups and their distinct source sets; no logic changes.
tests/unit/roles.test.ts Adds a meaningful regression test for the positive method/function promotion path, verifying isolatedHelper (fanIn=0, fanOut=1) is promoted to leaf when a genuine active sibling with fanIn>0 exists in the same file.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[buildActiveFilesSet] --> B[activeFiles\nfanIn>0 OR fanOut>0\nnon-annotation-only]
    A --> C[calledActiveFiles\nfanIn>0 only\nnon-annotation-only]

    D[buildClassifierInput] --> E{r.kind in\nANNOTATION_ONLY_KINDS?}
    E -- yes\nconstant / TYPE_DEF_KINDS --> F[activeFiles.has file\nhasActiveFileSiblings]
    E -- no --> G{r.kind === method\nor function?}
    G -- yes --> H[calledActiveFiles.has file\nhasActiveFileSiblings\nstrict fanIn>0 only]
    G -- no\ne.g. class --> I[undefined]

    B --> F
    C --> H
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[buildActiveFilesSet] --> B[activeFiles\nfanIn>0 OR fanOut>0\nnon-annotation-only]
    A --> C[calledActiveFiles\nfanIn>0 only\nnon-annotation-only]

    D[buildClassifierInput] --> E{r.kind in\nANNOTATION_ONLY_KINDS?}
    E -- yes\nconstant / TYPE_DEF_KINDS --> F[activeFiles.has file\nhasActiveFileSiblings]
    E -- no --> G{r.kind === method\nor function?}
    G -- yes --> H[calledActiveFiles.has file\nhasActiveFileSiblings\nstrict fanIn>0 only]
    G -- no\ne.g. class --> I[undefined]

    B --> F
    C --> H
Loading

Reviews (1): Last reviewed commit: "docs(roles): correct hasActiveFileSiblin..." | Re-trigger Greptile

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