Repository navigation
fix(path): resolve a Type.Member endpoint to the member, not the type (#4884) - #4893
SyedFahad7 wants to merge 3 commits into
Conversation
…Graphify-Labs#4884) A member's label is `.Build()`, so `PdfReport.Build` hit no exact tier and token scoring picked the `PdfReport` class. `path "Exporter.Run" "PdfReport.Build"` then printed a class-to-class route through the file as if it joined the two methods, and `path "Run" "PdfReport.Build"` found nothing, since no edge leads from a method back up to a class. `_find_node_tiers` now resolves `Type.Member` (and `Outer.Inner.Member`) through the type's `method` / `defines` / `contains` edges, after the dotted Python form and only when no node is literally labelled with the query. The hits form the exact tier, so `path`, `explain`, get_node and `query` share them, and several hits are refused with their ids instead of guessed. A `path` endpoint whose type exists but declares no such member fails closed, like a `file::symbol` miss (Graphify-Labs#4264), instead of scoring its way to the type. A namespace-qualified name is refused the same way, listing the bare type's same-named members, since the file rather than the namespace owns the type. Fixes Graphify-Labs#4884 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Syed Fahad <sfahad2904@gmail.com>
…#4884) Extracts the issue's Report.cs and checks that `Exporter.Run` / `Run` reach `PdfReport.Build` through the calls and dispatches_to edges, never through the file's contains edges, for the CLI and the MCP shortest_path tool. Synthetic graphs cover a member in two files, same-type overloads, nested types, direction, a namespace qualifier, a missing member, a literal dotted label, and an owner that is not a type. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Syed Fahad <sfahad2904@gmail.com>
|
Thanks for the pull request, @SyedFahad7. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 2 changed function(s) (2 sampled, not proven).
Formal verification. PR-changed functions: 2/2 verified (0 proven, 2 may-equivalent, 0 distinguished) · 0 not verified.
Graphify review — findings
Adds Type.Member (and Outer.Inner.Member, Type.Member()) endpoint resolution, so path and explain walk the declaring type's method/defines/contains edges to the member instead of scoring onto the type itself. Several same-named members, such as one per implementer or overloads, come back as the exact tier and are refused with their ids rather than guessed. Namespace qualifiers match nothing. When owner direction can't be told apart in an undirected graph without _src/_tgt markers, _membership_neighbors counts every membership neighbour both ways, which can turn members into owners. Fails closed in _resolve_path_endpoint when a type exists but declares no such member, returning a not-found message that lists same-named members of the bare type for retry instead of routing to the class. This refusal runs before ordinary scoring, so dotted endpoints like file names (utils.py) or exact dotted ids can be caught by it rather than reaching their normal match.
Worth a look
- Undirected graphs without markers turn members into owners, causing false refusals and wrong matches —
graphify/serve.py:1670· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- File-name path endpoints like
utils.pycan be refused as missingType.Memberqueries —graphify/serve.py:2158· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Member-miss refusal shadows exact dotted endpoint IDs —
graphify/serve.py:2156· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 710 functions depend on the 195 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
main()— 116 callers, 3 callees - worse:
_resolve_path_endpoint()— 10 callers, 6 callees - worse:
_resolve_single_node()— 18 callers, 3 callees - worse:
_shortest_path_text()— 15 callers, 3 callees - worse:
_find_node_tiers()— 5 callers, 8 callees
Verification — 710 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 598 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
18 of 470 test file(s) selected (4%) via static blast radius.
tests/test_benchmark.py— impacttests/test_benchmark_raw_graph.py— impacttests/test_benchmark_real_query.py— impacttests/test_explain_same_file_tie.py— impacttests/test_extract.py— impacttests/test_file_label_disambiguation.py— impacttests/test_mcp_graph_view_cache.py— impacttests/test_path_cli.py— impacttests/test_path_type_member.py— impact, changed-testtests/test_query_induced_edges.py— impacttests/test_query_mcp_direction.py— impacttests/test_query_names_its_graph.py— impacttests/test_query_seed_order.py— impacttests/test_query_seeds.py— impacttests/test_serve.py— impacttests/test_serve_http.py— impacttests/test_terraform.py— impacttests/test_truncation_full_answer.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
No difference found (not proven): No behavior difference found in \_find\_node\_tiers (not a proof).
The verifier ran both versions of \_find\_node\_tiers on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
No difference found (not proven): No behavior difference found in \_resolve\_path\_endpoint (not a proof).
The verifier ran both versions of \_resolve\_path\_endpoint on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| return msg | ||
|
|
||
|
|
||
| def _find_node_tiers( |
There was a problem hiding this comment.
_find_node_tiers()
fans out to 8 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
…ing (Graphify-Labs#4884) Review follow-up on Graphify-Labs#4893. The `Type.Member` lookup ran ahead of the ordinary tiers, and its not-found refusal only knew that some type of that name existed. A file name has the same shape, so `config.json` resolved to a `Config.Json()` method, and `utils.py` was refused as "no type 'utils' declares 'py'" whenever a `Utils` type existed and no node carried the bare file name as its label. The member lookup now runs after the ordinary tiers, only when no node's own id, label or source path matched exactly, and never for a query that names a source file (case-insensitive, like the source-path tier). The refusal skips the same queries, so it only fires for a genuine missing member. Membership direction now comes from the edge's `_src`/`_tgt` markers, else the stored arc of a directed graph. An undirected edge without markers is skipped instead of counted both ways, which let a member pose as its type's owner: `.Report()` "owned" `Builder`, and `Go.Missing` was refused because `.Go()` looked like a type with members. The CLI loaders that read a graph undirected stamp the markers on every link, so they keep resolving. Tests: a `utils.py` endpoint, `config.json` next to `Config.Json()`, an exact dotted node id, and undirected graphs with and without markers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Syed Fahad <sfahad2904@gmail.com>
…ing (#4884) Review follow-up on #4893. The `Type.Member` lookup ran ahead of the ordinary tiers, and its not-found refusal only knew that some type of that name existed. A file name has the same shape, so `config.json` resolved to a `Config.Json()` method, and `utils.py` was refused as "no type 'utils' declares 'py'" whenever a `Utils` type existed and no node carried the bare file name as its label. The member lookup now runs after the ordinary tiers, only when no node's own id, label or source path matched exactly, and never for a query that names a source file (case-insensitive, like the source-path tier). The refusal skips the same queries, so it only fires for a genuine missing member. Membership direction now comes from the edge's `_src`/`_tgt` markers, else the stored arc of a directed graph. An undirected edge without markers is skipped instead of counted both ways, which let a member pose as its type's owner: `.Report()` "owned" `Builder`, and `Go.Missing` was refused because `.Go()` looked like a type with members. The CLI loaders that read a graph undirected stamp the markers on every link, so they keep resolving. Tests: a `utils.py` endpoint, `config.json` next to `Config.Json()`, an exact dotted node id, and undirected graphs with and without markers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Syed Fahad <sfahad2904@gmail.com> (cherry picked from commit c3ad177)
…els, path weakest-hop confidence, Type.Member path endpoints Covers the SyedFahad7 batch: #4891 and #4889 merged on GitHub; #4892 and #4893 landed via authorship-preserving cherry-pick (their CI matrix was gated pending first-contributor approval, so brought in through the local gate instead). Credit by GitHub PR-author login @SyedFahad7. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Landed in v0.9.98, now on PyPI (pip install graphifyy==0.9.98). Landed via cherry-pick (your CI matrix was gated). A Type.Member path endpoint resolves to the member, not the containing type. Thanks for the fix, @SyedFahad7. |
What does this PR do?
Fixes #4884.
When
graphify pathgot an endpoint inType.Methodform, it resolved it to the class and printed a misleading route. A member's label is.Build(), soPdfReport.Buildmatched none of_find_node_tiers' exact tiers._resolve_path_endpointthen fell through to token scoring, which picked thePdfReportclass. With the issue'sReport.cs:path "Exporter.Run" "PdfReport.Build"printedExporter <--contains-- Report.cs --contains--> PdfReport, a route between two classes through the file, presented as if it joined the two methods.path "Run" "PdfReport.Build"found no path, because no edge leads from a method back up to a class.The change:
_find_node_tiersnow resolvesType.MemberandOuter.Inner.Member(an optional trailing()is allowed) by walking the owner type'smethod,definesandcontainsedges. This only runs after the dotted-Python form ([Bug]: graphify_node silently resolves qualified Python symbols to unrelated semantic matches #4366) finds nothing, and only when no node is literally labelled with the query. The hits are returned as the exact tier, sopath,explain, get_node andqueryall get them. Several hits go through the existing refusal with their candidate ids, rather than one being guessed: the same member name in two files, or overloads in one type._resolve_path_endpointfails closed when the type exists but declares no member of that name, the way afile::symbolmiss does ([Bug]:pathanswers afile::symbolendpoint from a same-name symbol in another file when the file holds no such symbol (exit 0, warning only) #4264). It returns not-found (exit 4) instead of scoring its way to the type._src/_tgtmarkers (graphify path (0.9.30) still prints reversed hops, and answers undirected connectivity while being documented as dependency reach (regression of #849; relates #829/#2074) #2309). SoInner.Outerdoes not resolve just becauseOutercontainsInner.After the fix, both commands from the issue print
.Run() --calls--> .Build() --dispatches_to--> .Build()(2 hops).IReport.BuildandCsvReport.Build()resolve to their own methods.PdfReport.Missingis refused withno type 'PdfReport' declares 'Missing'.This does not depend on the label change in #4883. It matches on the label as it is stored today (
.Build()), and would keep matching a bareBuild()orBuildlabel too.Type of change
Verification & Invariants
The invariant is the README contract: "a name that matches several nodes is refused with the candidates listed rather than guessed". A
Type.Memberendpoint now resolves to that member or is refused. It never resolves silently to the type.What stays the same:
_label_has_literal_exact_matchholds.pkg.mod.funcstill resolves through the module file first.serve.py.Limitations / unsupported cases:
A namespace qualifier (
Acme.Io.Outer.Go) cannot narrow the match. The file owns the type, not the namespace node, so there is no edge to walk. Such a query is refused, and the refusal lists the bare type's same-named members with their ids for a retry.Only
.-separated forms are handled.Type::Memberis still read aspath::symbol.Read the CONTRIBUTING.md guide.
Reproduced the issue and identified the invariant.
Made the smallest fix necessary.
Added a regression test (if bug fix) or isolated boundary test.
Kept the PR description synchronized with the final implementation.
Documented any limitations / unsupported cases explicitly.
How was this tested?
tests/test_path_type_member.pyextracts the issue'sReport.cswith the real C# extractor and runs the CLI and the MCPshortest_pathtool against it. It also uses synthetic graphs for:Without the
serve.pychange, 11 of the 12 tests fail. The one that still passes guards the unchanged fall-through behaviour.Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (No skill fragments touched.)Update (follow-up commit c3ad177, addressing bot review)
Type.Memberlookup now runs only after exact node id, label and source-path matches find nothing, and never for a query that names a source file (utils.pywas previously refused;config.jsonpreviously resolved to aConfig.Json()member)._src/_tgtedge markers, else the stored arc in a directed graph; undirected edges without markers are skipped.tests/test_path_type_member.py17 passed; full suite 8983 passed, 17 skipped, 1 xfailed.