fix(explain): render arrows in original edge direction (#853)#854
Closed
spindle79 wants to merge 1 commit into
Closed
fix(explain): render arrows in original edge direction (#853)#854spindle79 wants to merge 1 commit into
spindle79 wants to merge 1 commit into
Conversation
Same structural bug as Graphify-Labs#849 (and the same fix shape). `graphify explain` previously iterated `G.neighbors(nid)` on the undirected loaded graph and printed every connection as `--> X [relation]`, ignoring whether the underlying edge actually pointed out of the queried node or into it. For a queried callee, its callers were rendered as outbound calls — asserting a backwards call graph. Confirmed in the wild on a real codebase: `explain validateSanitySession()` printed `--> createPatchHandler() [calls]` even though the stored edge is `createPatchHandler --calls--> validateSanitySession`. Fix mirrors Graphify-Labs#849: - Force directed interpretation on read (`_raw = {**_raw, "directed": True}`) so the loaded `G` is a `DiGraph` that preserves edge direction. - Iterate successors and predecessors separately, tagging each connection as "out" or "in". - Render `-->` for outbound, `<--` for inbound. A neighbor that has both an inbound AND outbound edge to the queried node (mutual references) is rendered twice — once per edge — which is the truthful directed representation. Tests in `tests/test_explain_cli.py` cover both directions.
Collaborator
|
Thanks for the report! We shipped this fix ourselves in commit 7bb0919 (v0.7.17) — explain arrow direction now correctly shows |
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.
Fixes #853.
Summary
Same structural bug as #849, same fix shape as PR #851 — applied to
graphify explain. Previously every connection rendered as--> X [relation], asserting a backwards call graph whenever the queried node was a callee.Reproducer
Real-world:
explain validateSanitySession()printed--> createPatchHandler() [calls]even though the stored edge iscreatePatchHandler --calls--> validateSanitySession(verified ingraph.jsonand by the file source).validateSanitySession()is a pure leaf utility — all 13 of its connections are inbound — yet every one rendered as outbound.Fix
Same as #851:
_raw = {**_raw, "directed": True}) so the loadedGis aDiGraphpreserving edge direction.G.successors(nid)andG.predecessors(nid)separately, tagging each connection as"out"or"in".-->for outbound,<--for inbound.A neighbor that has both an inbound and outbound edge to the queried node (mutual references) is rendered twice — once per edge. That's the truthful directed representation; the previous undirected
G.neighbors()would have silently collapsed it to one row.Tests
tests/test_explain_cli.py— two tests, both on agraph.jsonfixture whose on-disk shape matches whatgraphify updateproduces (directed: false,links[i].source= caller):test_explain_renders_callers_as_inbound_and_callees_as_outbound— queried node isvalidateSanitySession. Its two callers (createPatchHandler,createEditHandler) must render as<--; its own callee (stableStringify) must render as-->. Asserts the old buggy--> createPatchHandler() [calls]is not in the output.test_explain_renders_pure_caller_as_outbound— converse: queryingcreatePatchHandlermust showvalidateSanitySessionas-->and no<--rows.The fixture writes graph.json directly rather than via
nx.Graph().add_edge()+node_link_data()— the latter emits links in node-insertion order, not edge-argument order, which can silently encode the wrong direction in small fixtures and mask the bug.Test plan
tree-sitter-sql; one Fortran preprocessed test; two Ollama env-var tests) — zero new failures.explain validateSanitySession()now renders all 13 connections as<--. Verified on the same Sanity Studio plugin reproducer used ingraphify pathrenders reversed arrows because graph is undirected (renderer bug, post-#760) #849/fix(path): render arrows in original edge direction (#849) #851.Notes for the reviewer
__main__.pyfor path/explain), but the two changes are independent.directed: falsebecauselinks[].source/.targetare preserved verbatim bynode_link_graphand always represent caller→callee per the existing build conventions — verified empirically on real graphs.