refactor(edges): drop 608 lines of shadowed duplicate code - #52
Merged
Conversation
edge_extractor.py defines four module-level functions twice. Python keeps the last definition, so the first copy of each has never executed: _node_text 660-662 shadowed by 1282-1283 _find_enclosing_type 671-680 shadowed by 1286-1294 _find_enclosing_callable 683-692 shadowed by 1297-1305 _add_type_relation_edges 695-1279 shadowed by 1308-1371 The last of those is the interesting one. It spans 585 lines because a block was indented one level too deep during the extract-methods refactor, nesting a second copy of ten EdgeExtractor methods inside it as local functions -- including a second _extract_call_edges carrying the same `for n in []` defect as the real one. None of it is reachable. This deletes the shadowed copies, which is a no-op by construction: the interpreter was already discarding them. Verified by extracting each live callable's source before and after -- all five are byte-identical. The surviving copies had lost their docstrings when they were duplicated, so those are carried back over. That is the only content change. edge_extractor.py: 1371 -> 759 lines. EdgeExtractor itself (lines 31-657) is untouched. Re: #10 -- the split this issue asks for has already happened; extract_edges delegates to _extract_containment_edges / _extract_import_edges / _extract_call_edges and six more. The issue also points at ast_rag/ast_parser.py, which no longer exists. What remained was this debris from that refactor. Suite unchanged from main: 3 failed, 174 passed, 2 xfailed (same three pre-existing failures, addressed separately in #51).
Collaborator
Author
|
CI on this PR is red at the Ruff is unpinned and Because lint runs before #51 fixes that and is passing. Once it lands, this PR should go green on a re-run; I've verified all three CI steps locally in the meantime. |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Re: #10
What's here
edge_extractor.pydefines four module-level functions twice. Python keeps the last definition, so the first copy of each has never executed:_node_text_find_enclosing_type_find_enclosing_callable_add_type_relation_edgesThe last one is the interesting case. It spans 585 lines because a block was indented one level too deep during the extract-methods refactor, nesting a second copy of ten
EdgeExtractormethods inside it as local functions — including a second_extract_call_edgescarrying the samefor n in []defect as the real one (that's the bug fixed in #50). None of it is reachable.edge_extractor.py: 1371 → 759 lines.EdgeExtractoritself (lines 31–657) is untouched.Why this is safe
Deleting a shadowed definition is a no-op by construction — the interpreter was already discarding it. I verified rather than asserted it: extracted the source of each live callable via
inspect.getsourcebefore and after, and all five are byte-identical (ignoring docstrings)._containment_edge_kindis included as a control — it has no duplicate and is untouched.The one content change
When these functions were duplicated, the surviving copies lost their docstrings — the documented originals were the ones being discarded. So I carried the four docstrings back onto the live copies. That's the only thing in this diff that isn't a deletion, and it's why the numbers are −608/+4 rather than a pure −608.
On #10 itself
The split that issue asks for has already happened —
extract_edgesdelegates to_extract_containment_edges,_extract_import_edges,_extract_call_edgesand six more. The issue also points atast_rag/ast_parser.py, which no longer exists. So I've treated it as "clean up what that refactor left behind" rather than redoing it. Happy to close it as done, or leave it open if you had further decomposition in mind — the remaining length is inEdgeExtractor's per-language branches, which could go further if you want.Verification
Identical to
main— same three pre-existing failures, which #51 fixes. This branch is independent of both #50 and #51 and touches no line either of them touches.