fix(review): exclude dataflow nodes from review diff + move detection in removed-referenced

.github/workflows/pr-review.yml

@@ -39,9 +39,15 @@ jobs:
           BASELINE_DIR="${RUNNER_TEMP}/cg-baseline-checkout"
           rm -rf "$BASELINE_DIR"
           git worktree add "$BASELINE_DIR" origin/main
+          # Both graphs are built with the PR-head codegraph (installed in
+          # the previous step). Re-installing from the baseline worktree
+          # here would silently clobber the editable install, making the
+          # diff AND the rule evaluation run main's code — PR changes to
+          # the differ/rules/parsers would never take effect in their own
+          # review, and parser changes would dominate the diff as
+          # added/removed-node noise.
           (
             cd "$BASELINE_DIR"
-            pip install -e . --quiet
             codegraph build --no-incremental
             codegraph baseline save --output "$GITHUB_WORKSPACE/.codegraph/baseline.db"
           )

CHANGELOG.md

@@ -119,6 +119,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Review diff no longer churns on dataflow nodes; moved symbols no
+  longer flag as removed.** The graph differ now excludes `VARIABLE` /
+  `PARAMETER` nodes and `DATA_*` edges from review diffs — their
+  qualnames embed line numbers (`pkg.fn.<var:x:42>`), so any edit that
+  shifted lines made every local variable appear removed+added and fired
+  false-positive `removed-referenced` criticals (12+ on a clean
+  typescript.py refactor). Separately, `removed-referenced` now skips
+  symbols whose terminal name reappears in the added set with the same
+  kind — moving `esc`/`pyvisHref` from `app.js` to `ui/helpers.js` with
+  every caller updated was flagged critical three times. Genuine
+  deletions with surviving callers still fire. (Both surfaced by the
+  self-review gate on the wave-2 dashboard/resolver PRs.)
 - **`codegraph init` now writes a robust `.mcp.json`** — uses the absolute
   path to the `codegraph` binary (resolved from the running interpreter's
   bin dir, then `shutil.which`, then bare fallback), passes an explicit

codegraph/review/differ.py

@@ -6,6 +6,13 @@
 
 import networkx as nx
 
+# Sub-symbol granularity introduced by the dataflow pass. Variable and
+# parameter qualnames embed line numbers (``pkg.fn.<var:x:42>``), so any
+# edit that shifts lines makes every local appear removed+added. Review
+# operates on API-level symbols; dataflow plumbing is excluded outright.
+_EXCLUDED_NODE_KINDS = frozenset({"VARIABLE", "PARAMETER"})
+_EXCLUDED_EDGE_KINDS = frozenset({"DATA_ASSIGN", "DATA_ARG", "DATA_RETURN"})
+
 
 @dataclass
 class NodeChange:
@@ -55,7 +62,7 @@ def _kind_str(value: object) -> str:
 def _node_key(attrs: dict[str, Any]) -> _NodeKey | None:
     qualname = str(attrs.get("qualname") or "")
     kind = _kind_str(attrs.get("kind"))
-    if not qualname or not kind:
+    if not qualname or not kind or kind in _EXCLUDED_NODE_KINDS:
         return None
     return (qualname, kind)
 
@@ -99,6 +106,8 @@ def _edge_keys(
     keys: set[tuple[str, str, str]] = set()
     for src, dst, data in graph.edges(data=True):
         kind = _kind_str(data.get("kind"))
+        if kind in _EXCLUDED_EDGE_KINDS:
+            continue
         src_qn = id_map.get(src, src)
         dst_qn = id_map.get(dst, dst)
         keys.add((src_qn, dst_qn, kind))

codegraph/review/rules.py

@@ -275,9 +275,20 @@ def evaluate_rules(
                     )
                 )
         elif when == "removed_referenced":
+            # A removed symbol whose terminal name reappears in the added
+            # set (same kind) was moved, not deleted — callers reference the
+            # new location. PR #73 flagged esc/pyvisHref as critical after
+            # they moved from app.js to ui/helpers.js with all callers
+            # updated.
+            moved = {
+                (c.qualname.rsplit(".", 1)[-1], c.kind)
+                for c in diff.added_nodes
+            }
             for change in diff.removed_nodes:
                 if not _node_matches(rule, change):
                     continue
+                if (change.qualname.rsplit(".", 1)[-1], change.kind) in moved:
+                    continue
                 old_id = _find_node_id(change.qualname, change.kind, old_graph)
                 if old_id is None:
                     continue

tests/test_review_differ.py

@@ -174,3 +174,57 @@ def test_line_shift_AND_signature_change_records_both_in_details() -> None:
     details = diff.modified_nodes[0].details or {}
     assert "signature" in details
     assert "line_start" in details
+
+
+def _add_node(
+    g: nx.MultiDiGraph,
+    qn: str,
+    *,
+    kind: str,
+    file: str = "pkg/foo.py",
+    line: int = 1,
+) -> None:
+    g.add_node(
+        f"node::{qn}",
+        qualname=qn,
+        kind=kind,
+        file=file,
+        line_start=line,
+        signature="",
+    )
+
+
+def test_variable_and_parameter_nodes_are_ignored() -> None:
+    """Regression test: dataflow VARIABLE/PARAMETER qualnames embed line
+    numbers (``pkg.fn.<var:x:42>``), so a pure line shift makes every local
+    look removed+added. PR #74 got 12+ false-positive ``removed-referenced``
+    criticals on refactored locals in typescript.py."""
+    old_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    new_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    _add_func(old_g, "pkg.fn", signature="fn()")
+    _add_func(new_g, "pkg.fn", signature="fn()")
+    _add_node(old_g, "pkg.fn.<var:x:42>", kind="VARIABLE", line=42)
+    _add_node(old_g, "pkg.fn.<param:y:40>", kind="PARAMETER", line=40)
+    _add_node(new_g, "pkg.fn.<var:x:57>", kind="VARIABLE", line=57)
+    _add_node(new_g, "pkg.fn.<param:y:55>", kind="PARAMETER", line=55)
+    diff = diff_graphs(old_g, new_g)
+    assert diff.added_nodes == []
+    assert diff.removed_nodes == []
+    assert diff.modified_nodes == []
+
+
+def test_data_edges_are_ignored() -> None:
+    """DATA_* edges churn with every line edit alongside their var nodes;
+    they are dataflow plumbing, not reviewable API surface."""
+    old_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    new_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    _add_func(old_g, "pkg.fn", signature="fn()")
+    _add_func(new_g, "pkg.fn", signature="fn()")
+    _add_node(new_g, "pkg.fn.<var:x:57>", kind="VARIABLE", line=57)
+    new_g.add_edge(
+        "node::pkg.fn", "node::pkg.fn.<var:x:57>",
+        key="DATA_ASSIGN", kind="DATA_ASSIGN",
+    )
+    diff = diff_graphs(old_g, new_g)
+    assert diff.added_edges == []
+    assert diff.removed_edges == []

tests/test_review_rules.py

@@ -101,3 +101,55 @@ def test_evaluate_rules_with_custom_rule(
     )
     assert findings
     assert all(f.rule_id == "any-add" for f in findings)
+
+
+def _add_fn(
+    g: nx.MultiDiGraph,
+    qn: str,
+    *,
+    file: str = "pkg/foo.py",
+    line: int = 1,
+) -> None:
+    g.add_node(
+        f"node::{qn}",
+        qualname=qn,
+        kind="FUNCTION",
+        file=file,
+        line_start=line,
+        signature=f"{qn.rsplit('.', 1)[-1]}()",
+    )
+
+
+def test_removed_referenced_skips_moved_symbol() -> None:
+    """Regression test: a symbol moved to another module (removed at one
+    qualname, added at another with the same terminal name and kind) must
+    not fire ``removed-referenced``. PR #73 flagged esc/pyvisHref/
+    mermaidThemeVars as critical after they moved from app.js to
+    ui/helpers.js with every caller updated."""
+    old_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    new_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    _add_fn(old_g, "pkg.app.esc", file="app.js")
+    _add_fn(old_g, "pkg.app.render", file="app.js", line=10)
+    old_g.add_edge("node::pkg.app.render", "node::pkg.app.esc", key="CALLS", kind="CALLS")
+    # esc moved to helpers; caller render survives.
+    _add_fn(new_g, "pkg.ui.helpers.esc", file="ui/helpers.js")
+    _add_fn(new_g, "pkg.app.render", file="app.js", line=10)
+    diff = diff_graphs(old_g, new_g)
+    findings = evaluate_rules(diff, new_graph=new_g, old_graph=old_g)
+    assert not [f for f in findings if f.rule_id == "removed-referenced"], (
+        "moved symbol must not be reported as removed-referenced"
+    )
+
+
+def test_removed_referenced_still_fires_for_real_removal() -> None:
+    """Control: genuinely deleting a symbol that callers still reference
+    keeps firing critical."""
+    old_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    new_g: nx.MultiDiGraph = nx.MultiDiGraph()
+    _add_fn(old_g, "pkg.app.esc", file="app.js")
+    _add_fn(old_g, "pkg.app.render", file="app.js", line=10)
+    old_g.add_edge("node::pkg.app.render", "node::pkg.app.esc", key="CALLS", kind="CALLS")
+    _add_fn(new_g, "pkg.app.render", file="app.js", line=10)
+    diff = diff_graphs(old_g, new_g)
+    findings = evaluate_rules(diff, new_graph=new_g, old_graph=old_g)
+    assert [f for f in findings if f.rule_id == "removed-referenced"]