fix(review): exclude dataflow nodes from review diff + move detection in removed-referenced#76
Merged
Merged
Conversation
… removed-referenced The wave-2 PRs tripped the self-review gate with two false-positive classes: 1. VARIABLE/PARAMETER nodes (new in the data-edges work) embed line numbers in their qualnames, so any line shift makes every local appear removed+added. A clean typescript.py refactor produced 12+ removed-referenced criticals on its own local variables. The differ now excludes these kinds and DATA_* edges from review diffs - they are dataflow plumbing, not reviewable API surface. 2. removed-referenced fired on symbols that moved modules (esc, pyvisHref, mermaidThemeVars: app.js -> ui/helpers.js) even though every caller was updated. The rule now skips removed nodes whose terminal name reappears in the added set with the same kind. Genuine deletions with surviving callers still fire (control test added). Gate: 746 tests pass, ruff + mypy --strict clean, self-lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
codegraph PR reviewDiff vs main · severity ≤
|
| severity | rule | kind | qualname | file:line | score | message |
|---|---|---|---|---|---|---|
| critical | removed-referenced | graph | codegraph.review.differ._build_node_index.<var:key:76> |
codegraph/review/differ.py:76 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._build_node_index.<var:nid:75> |
codegraph/review/differ.py:75 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._build_node_index.<var:payload:83> |
codegraph/review/differ.py:83 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._edge_keys.<var:dst_qn:103> |
codegraph/review/differ.py:103 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._edge_keys.<var:keys:99> |
codegraph/review/differ.py:99 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._edge_keys.<var:kind:101> |
codegraph/review/differ.py:101 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._edge_keys.<var:src:100> |
codegraph/review/differ.py:100 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._edge_keys.<var:src_qn:102> |
codegraph/review/differ.py:102 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._node_key.<var:kind:57> |
codegraph/review/differ.py:57 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ._node_key.<var:qualname:56> |
codegraph/review/differ.py:56 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:diff:124> |
codegraph/review/differ.py:124 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:key:129> |
codegraph/review/differ.py:129 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:key:173> |
codegraph/review/differ.py:173 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:new_edges:190> |
codegraph/review/differ.py:190 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:new_id_map:188> |
codegraph/review/differ.py:188 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:new_idx:127> |
codegraph/review/differ.py:127 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:old_edges:189> |
codegraph/review/differ.py:189 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:old_id_map:187> |
codegraph/review/differ.py:187 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:old_idx:126> |
codegraph/review/differ.py:126 | 50 | Removed symbol still referenced by callers |
| critical | removed-referenced | graph | codegraph.review.differ.diff_graphs.<var:src_qn:192> |
codegraph/review/differ.py:192 | 50 | Removed symbol still referenced by callers |
Triggered by codegraph CI · last run
The baseline build ran 'pip install -e .' inside the origin/main worktree, silently replacing the PR-head editable install from the previous step. Every subsequent step - the PR graph build, the diff, and the rule evaluation - ran main's codegraph, so PR changes to the differ/rules/parsers never took effect in their own review. This PR's differ fix was itself flagged by the exact var-node noise it removes. Both graphs are now built with the PR-head install, which also keeps diffs apples-to-apples when a PR changes parser output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
codegraph PR reviewDiff vs main · severity ≤
|
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
The wave-2 PRs (#73, #74) tripped the self-review gate (
--fail-on high) with two false-positive classes introduced by the interaction between the new data-edges feature (#68) and the review rules:Dataflow node churn.
VARIABLE/PARAMETERqualnames embed line numbers (pkg.fn.<var:x:42>), so any edit that shifts lines makes every local variable appear removed+added. PR feat(resolve): TS fresh-instance binding + decorator CALLS edges (R2 parity) #74's cleantypescript.pyrefactor produced 12+removed-referencedcriticals on its own locals. The differ now excludes these node kinds andDATA_*edges from review diffs — they're dataflow plumbing, not reviewable API surface (same philosophy as the existing line-shift exclusion documented indiff_graphs).Moves flagged as removals.
removed-referencedfired onesc/pyvisHref/mermaidThemeVarsafter PR refactor(web): split app.js into per-view modules + shared UI helpers #73 moved them fromapp.jstoui/helpers.jswith every caller updated (verified: all callers aliasCGUI.*). The rule now skips removed nodes whose terminal name reappears in the added set with the same kind. A control test asserts genuine deletions with surviving callers still fire critical.Test plan
codegraph lintself-run: 0 findingsUnblocks #73, #74 (and keeps the gate honest for #75, which has a legit finding being addressed separately).
🤖 Generated with Claude Code