Skip to content

fix(affected): accept qualified seeds, exit nonzero on a miss or tie - #4134

Open
krishhgg wants to merge 1 commit into
Graphify-Labs:v8from
krishhgg:fix/affected-qualified-seeds
Open

krishhgg wants to merge 1 commit into
Graphify-Labs:v8from
krishhgg:fix/affected-qualified-seeds

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 6, 2026

Copy link
Copy Markdown

What does this PR do?

graphify affected had no way to name one of two same-named methods, and it reported a failed lookup as success. On httpx, Client and AsyncClient both define .send() in httpx/_client.py:

  • affected "Client.send", affected "httpx/_client.py::Client.send" and affected ".send()" print No unique node match for ... on stdout and exit 0. A script reads that as "nothing depends on this".
  • affected send resolves to a sourceless stub Send, left by a send: Send annotation in tests/conftest.py, and lists its 10 annotation users. Three real definitions are named send.

The change is in seed resolution in graphify/affected.py and in the affected branch of graphify/cli.py. The traversal, affected_nodes, is unchanged.

Qualified seeds. Class.method, path::Class.method, path::function and path::Class now resolve.

  • The path goes through the same _as_repo_relative normalization as a file-path seed and restricts the node's own source_file. The exact spelling wins. A case- or Unicode-insensitive match is used only when no file has the exact spelling, and when it matches several files the result is a tie.
  • The class must own the member through a method or contains edge. Direction is read from the _src/_tgt markers the way serve.py reads them. Label text alone never picks the owner.
  • When several owners with the requested name own a matching member, the result is a tie.
  • A qualified query is answered from its scope before any unrestricted label, so a Markdown heading # Client.send cannot stand in for the method. A miss inside a path scope is final.
  • A two-level chain such as Outer.Inner.run is refused with a hint to use Inner.run, path::Inner.run or a node id.

Fail closed. Resolution runs in tiers: exact id, qualified, exact label, bare name, source path, substring. A tier resolves the seed only when it has exactly one candidate. A miss prints No unique node match for X to stderr and exits 1. A tie prints each candidate's label, file:line and node id to stderr and exits 1. After a name tier ties, the substring tier no longer runs.

Real definitions beat stubs. In each tier, source-backed nodes outrank sourceless ones. A method label such as .send() also answers to send. Other labels keep their leading dot, so a .config file does not tie with Config.

format_affected raises a new SeedResolutionError instead of returning the miss text, and the CLI turns it into exit 1. resolve_seed keeps its str | None return. explain and path resolve through serve._find_node_tiers and are unchanged.

Behavior changes users will see

I resolved every node label under 80 characters, plus its bare forms, with v8 and with this branch (httpx: 3,235 seeds, Flask: 3,134).

  • A miss or a tie now exits 1 and prints to stderr. One existing test expected exit 0 on a miss and now expects 1.
  • A bare name shared by a function and methods is now a tie: 10 httpx names (get, post, put, ...) and 16 Flask names. get() and httpx/_api.py::get still resolve to the function.
  • Stubs no longer answer for names that real definitions carry. 14 seeds moved from a stub to a real node and 9 to a candidate list. Flask's affected copy used to resolve to the stdlib copy stub and now resolves to AppContext.copy. Flask's before_app_request now resolves to Blueprint.before_app_request, which has no inbound edges; v8's stub carried one decorator use that the extractor had bound to it.
  • 102 seeds that used to miss now resolve (44 httpx, 58 Flask), almost all bare method names that one class defines.
  • I also tried every owned member in both graphs as Owner.member and path::Owner.member. None resolved to the wrong node. The 168 misses are httpx Markdown headings whose own labels contain a dot.

Limitations

  • One ownership level only, and inherited members are not followed. Name the class that defines the method.
  • A member whose own label contains a dot (some Markdown headings) cannot be named as Owner.member. Use its node id.
  • A :: left half with no separator, no matching file and whitespace in it is treated as prose, as on v8. Adding ./ makes it a path.
  • A left half that names an extensionless file in the graph is a path. So a root file named Invoice scopes Invoice::Line to itself; v8 resolved that query by substring.
  • A plain file-path seed (no ::) keeps v8's case-insensitive comparison.
  • affected_nodes on an undirected graph still reads direction from edge iteration order, as on v8. The CLI always loads a directed graph, so only library callers are affected.
  • Seen while testing and not changed here: explain "Client.send" on httpx returns a rationale node, and path "no_such_symbol_xyz" httpx_models_response routes from a docs node and exits 0.

Related: #3485 (path::Symbol for explain), #3913 and #3935 (path endpoint refusal), #1669 (member seeding), #2309 (_src/_tgt markers), #2706 and #2707 (path-form seeds).

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

Invariant: affected walks from exactly one node, and only from a node the query names unambiguously. A query that names no node, or several, exits nonzero with the reason on stderr. The resolver never picks one of several candidates, and a sourceless stub never answers for a name that source-backed definitions carry. A path:: query whose left half has a separator or names a file never resolves outside that file.

Persisted state: none. affected reads graph.json and writes nothing, and extraction is untouched. Fixture graphs built by v8 and by this branch have identical node and edge sets, and an incremental update matched a clean rebuild.

  • 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?

Linux only. Windows was not tested. The path half of a path:: seed goes through the same _as_repo_relative helper as a file-path seed, but I did not run it on Windows.

# new tests against unmodified v8
uv run --offline pytest tests/test_affected_cli.py -q
20 failed, 21 passed    (the 21 include guards for lookups that already worked on v8)

# with this PR
uv run --offline pytest tests/test_affected_cli.py tests/test_affected_member_seed.py -q
45 passed

# full suite; the same 30 tests fail on unmodified v8 (erlang, r, solidity and
# vbnet extractors plus the ollama retry tests, all from missing optional dependencies)
uv run --offline pytest tests -q
v8:     30 failed, 6452 passed, 105 skipped
branch: 30 failed, 6474 passed, 105 skipped

uv run --offline ruff check .    All checks passed!
uv run --offline pyright         637 errors on v8 and on this branch

# httpx (2,239 nodes), graphify affected <seed>
seed                            v8                           this PR
Client.send                     miss, exit 0                 .send() L879, 9 hits, exit 0
httpx/_client.py::Client.send   miss, exit 0                 .send() L879, 9 hits, exit 0
AsyncClient.send                miss, exit 0                 .send() L1594, 9 hits, exit 0
send                            stub Send, 10 hits, exit 0   3 candidates, exit 1
.send()                         miss, exit 0                 2 candidates, exit 1
get_environment_proxies         9 hits, exit 0               9 hits, exit 0
httpx/_utils.py                 98 hits, exit 0              98 hits, exit 0
httpx/_missing.py::Client.send  miss, exit 0                 miss, exit 1
no_such_symbol_xyz              miss, exit 0                 miss, exit 1
$ graphify affected send
Ambiguous seed send: 3 nodes match.
- .send() httpx/_client.py:L879 (id: httpx_client_client_send)
- .send() httpx/_client.py:L1594 (id: httpx_client_asyncclient_send)
- send() httpx/_transports/asgi.py:L148 (id: httpx_transports_asgi_asgitransport_handle_async_request_send)
Pass one of the ids above, or narrow the seed as Class.method or path::symbol.
Edge-case fixtures through the CLI (v8 vs this PR)
owner stub (Handle annotation), handle.send:                         miss, exit 0     ->  nested send(), exit 0
unrelated class Handle in other.py, pkg.py::handle.send:             miss, exit 0     ->  nested send(), exit 0
files a.py and A.py, a.py::Bar:                                      miss, exit 0     ->  miss, exit 1
files a.py and A.py, a.py::Foo:                                      miss, exit 0     ->  Foo, exit 0
composed and decomposed café.py, crossed query:                      miss, exit 0     ->  miss, exit 1
files ' a.py' and 'a.py', ' a.py::run':                              miss, exit 0     ->  run() in ' a.py', exit 0
files 'my client.py' and 'My Client.py', MY CLIENT.PY::Client.send:  miss, exit 0     ->  2 candidates, exit 1
heading '# Client.send', Client.send:                                heading, exit 0  ->  .send(), exit 0
heading '# missing.py::Client.send', same query:                     heading, exit 0  ->  miss, exit 1
heading '# my pkg/missing.py::Client.send', same query:              heading, exit 0  ->  miss, exit 1
heading tree '# Client' / '## send', Client.send:                    miss, exit 0     ->  2 candidates, exit 1
heading tree, pkg.py::Client.send:                                   miss, exit 0     ->  .send() and its caller, exit 0
headings '# Outer.Inner' and '# Outer.Inner.run', Outer.Inner.run:   heading, exit 0  ->  miss + hint, exit 1
Ruby Billing::Invoice::Line, Invoice::Line:                          substring match  ->  same

Qualified resolution on graphs with many same-named owners stays linear: 2,000 classes named Client resolve Client.send in 0.03 s, and a 16,000-edge undirected graph resolves in 0.12 s. Two tests cap the ownership lookups and whole-graph edge scans.

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments. (Not applicable: no skill fragments changed.)
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables).
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python).
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

`graphify affected` could not name one of two same-named methods, and
a failed lookup printed "No unique node match" on stdout with exit 0.
On httpx, `Client.send` and `httpx/_client.py::Client.send` missed,
and `send` resolved to a sourceless `Send` stub left by an annotation.

Seed resolution now runs in tiers (exact id, qualified, exact label,
bare name, source path, substring) and resolves only when a tier has
exactly one candidate. `Class.method`, `path::Class.method`,
`path::function` and `path::Class` are accepted. The class must own
the member through a `method` or `contains` edge, and the path
restricts the node's own source_file, exact spelling first.
Source-backed nodes outrank sourceless stubs. A miss or a tie prints
to stderr and exits 1, and a tie lists its candidates.

affected_nodes, explain and path are unchanged.

Related: Graphify-Labs#3485, Graphify-Labs#3913, Graphify-Labs#1669, Graphify-Labs#2706, Graphify-Labs#2707.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@krishhgg
krishhgg requested a review from safishamsi as a code owner October 6, 2026 00:32
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Thanks for the pull request, @krishhgg. 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.

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. 2 change(s) alter behavior, breaking input(s) attached. PR-changed functions: 3/4 verified (0 proven, 1 may-equivalent, 2 distinguished) · 1 not verified (1 vacuous).

Behavior changes: format\_affected changes behavior, here is the input that shows it.

The verifier found a concrete input on which format\_affected behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"graph":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\]\), \_g\.add\_edges\_from\(\[\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","query":"'\\\\t\\\\n'"\}, the old code produced 'No unique node match for \\t\\n' but the new code produces raises SeedResolutionError. Paste that input straight into a regression test.

Behavior changes: resolve\_seed changes behavior, here is the input that shows it.

The verifier found a concrete input on which resolve\_seed behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"graph":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","query":"'\(\)'","root":"\_\_import\_\_\('pathlib'\)\.Path\('a\.txt'\)"\}, the old code produced None but the new code produces raises KeyError. Paste that input straight into a regression test.

Not verified on this run: dispatch\_command (vacuous: never exercised).


Graphify review — findings

Extends affected seed resolution to accept Class.method and path::symbol queries alongside labels, node ids, and file paths, reading ownership from method/contains edges via _src/_tgt markers so undirected graphs keep the right direction. An ambiguous or unmatched seed now raises SeedResolutionError, listing up to 20 candidates instead of guessing; this covers several files that match a path only after case or Unicode normalization and nested Outer.Inner.run qualifiers, which are refused. Source-backed definitions win over sourceless annotation stubs, and owner lookups on undirected graphs come from a single indexed edge pass rather than a full scan per member.

Worth a look

  • format_affected now raises instead of returning a 'No unique node match' string — graphify/affected.py:628 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review

Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 484 functions depend on the 214 functions this change touches.

Health — this change adds coupling hotspots:

  • new: main() — 104 callers, 3 callees
  • new: dispatch_command() — 2 callers, 128 callees
  • new: _refresh_stale_skills() — 25 callers, 4 callees
  • new: format_affected() — 8 callers, 10 callees
  • new: resolve_seed_candidates() — 6 callers, 7 callees
  • new: _run_cli() — 6 callers, 7 callees
  • new: _stale_graph_sources() — 7 callers, 6 callees
  • new: _run_hook_guard() — 4 callers, 10 callees
  • …and 2 more — each is listed as a finding

Verification — 484 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: 484 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

37 of 334 test file(s) selected (11%) via static blast radius.

  • tests/test_affected_cli.py — impact, changed-test
  • tests/test_affected_member_seed.py — impact
  • tests/test_agents_platform.py — impact
  • tests/test_codebuddy.py — impact
  • tests/test_corrupt_graph_json.py — impact
  • tests/test_dedup_shrink_refuses_force_write.py — impact
  • tests/test_devin.py — impact
  • tests/test_explain_cli.py — impact
  • tests/test_extract_cli.py — impact
  • tests/test_global_add_tag_inference.py — impact
  • tests/test_god_nodes_cli.py — impact
  • tests/test_hollow_chunks_arm_shrink_guard.py — impact
  • tests/test_hook_guard_token_match.py — impact
  • tests/test_hook_out_of_project_paths.py — impact
  • tests/test_hook_strict.py — impact
  • tests/test_incomplete_build_guard.py — impact
  • tests/test_indirect_call_nested_closure_shadow.py — impact
  • tests/test_indirect_dispatch.py — impact
  • tests/test_indirect_dispatch_assign_return.py — impact
  • tests/test_indirect_dispatch_getattr.py — impact
  • tests/test_install.py — impact
  • tests/test_install_references.py — impact
  • tests/test_install_version_warning.py — impact
  • tests/test_js_dynamic_import_affected.py — impact
  • tests/test_js_dynamic_imports.py — impact
  • tests/test_merge_chunks_validation.py — impact
  • tests/test_multigraph_diagnostics.py — impact
  • tests/test_no_dedup_flag.py — impact
  • tests/test_partial_cache.py — impact
  • tests/test_path_cli.py — impact
  • tests/test_query_cli.py — impact
  • tests/test_query_induced_edges.py — impact
  • tests/test_scala_self_type.py — impact
  • tests/test_skill_auto_refresh.py — impact
  • tests/test_skill_version_warning.py — impact
  • tests/test_stale_prune.py — impact
  • tests/test_unverified_semantic_shrink.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.

Docs that may be stale (advisory)

…and 10 more.

Formal verification

Behavior changes: format\_affected changes behavior, here is the input that shows it.

The verifier found a concrete input on which format\_affected behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"graph":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\]\), \_g\.add\_edges\_from\(\[\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","query":"'\\\\t\\\\n'"\}, the old code produced 'No unique node match for \\t\\n' but the new code produces raises SeedResolutionError. Paste that input straight into a regression test.

Behavior changes: resolve\_seed changes behavior, here is the input that shows it.

The verifier found a concrete input on which resolve\_seed behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"graph":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","query":"'\(\)'","root":"\_\_import\_\_\('pathlib'\)\.Path\('a\.txt'\)"\}, the old code produced None but the new code produces raises KeyError. Paste that input straight into a regression test.

No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).

The verifier ran both versions of \_run\_cli 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.

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 40 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

· 1 grounded finding(s) anchored inline below; 9 more finding(s) on lines outside this diff (see the check run).

Comment thread graphify/affected.py
yield exact_source_matches


def resolve_seed_candidates(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — resolve_seed_candidates()

fans out to 7 callees (efferent coupling); 6 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

@krishhgg

krishhgg commented Oct 6, 2026

Copy link
Copy Markdown
Author

On the advisory: format_affected now raises SeedResolutionError on a miss or a tie instead of returning No unique node match for X. That is deliberate. Its only caller in the repo is the affected branch of cli.py, which turns the error into a stderr message and exit 1. Returning the text is what let a miss print as a normal report with exit 0. If you'd rather keep the string return for library callers, I can add a keyword such as raise_on_unresolved=False by default and have the CLI pass True.

The coupling note on resolve_seed_candidates() is the tier dispatcher: it calls one helper per tier (exact id, qualified, exact label, bare name, source path, substring) plus the source-backed filter.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant