Skip to content

reconnectReverseDepEdges still mis-wires same-named siblings when sibling count also changes (incremental rebuild) #1865

Description

@carlos-alm

Summary

The fix for #1752 (reconnectReverseDepEdges in build-edges.ts / reconnect_reverse_dep_edges in detect_changes.rs) correctly resolves the common case where an incremental rebuild shifts a group of same-(name, kind) sibling declarations in a file (e.g. several object-literal close() {} methods) without changing how many siblings exist, by matching on each target's saved ordinal rank instead of nearest-line proximity.

However, when a same-named/same-kind sibling is also added or removed in the same file edit that shifts the group, the sibling-count check in pickReconnectTarget/pick_reconnect_target intentionally falls back to the pre-#1752 nearest-line heuristic (documented in-code as "a genuinely ambiguous case"). That fallback is exactly the heuristic #1752 proved unreliable once a same-named group shifts far enough — so this specific compound scenario (shift + sibling count change, in the same incremental build) still produces reverse-dep call edges that diverge from a full rebuild of the identical final source.

This is a pre-existing gap, not a regression from the #1752 fix — verified by reproducing the identical divergence against the pre-#1752 code on the same scenario (both produce the same wrong result; the #1752 fix does not make this case worse, it just doesn't cover it).

Reproduction

Using a fixture with 4 functions in one file, each returning an object with its own close() {} method (openA..openD), and 4 reverse-dep caller files each calling close() off one of them:

  1. Full build.
  2. Edit the file: insert an unrelated helper function above all 4 (shifts every close() sibling's line down), and rename one sibling's method away from close (e.g. openB's closeshutdown), changing the sibling count for (name="close", kind="method") in that file from 4 to 3 in the same edit.
  3. Incremental rebuild (default).
  4. Compare the resulting close() call edges (or fn-impact close --json) against a full rebuild (--no-incremental) of the identical final source — they diverge (some callers end up wired to the wrong remaining sibling, or drop an edge entirely).

The same divergence reproduces with a sibling added instead of removed (4 → 5), and reproduces identically on both the WASM/JS engine and the native Rust engine.

Suggested direction

The ordinal-based matching in #1752 assumes the sibling count is a reliable invariant when the identity of individual siblings isn't otherwise recoverable. When the count itself changes in the same build, a more robust identifier would be needed to disambiguate — e.g. hashing each sibling's own body text (a purge+reinsert that doesn't touch a given method's body should preserve that hash even if the group's size changes), rather than positional ordinal or line proximity. This likely requires reusing/extending the existing per-file content-hash infrastructure already used for incremental change detection, applied at the per-declaration granularity instead of per-file.

How this was found

Found while independently verifying the #1752 fix (issue #1752, fixed by the commit "fix: correct blast-radius/fn-impact computation for line-shifted declarations on incremental rebuild") — stress-testing beyond the literal repro (which only shifts otherwise-untouched siblings) with additional scenarios, including one where the sibling count changes in the same edit. Out of scope for #1752 itself, which is about the untouched-sibling-count case described in that issue's literal repro (fully fixed and verified across both engines).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions