Skip to content

fix: affected silently returns nothing for equivalent path forms - #2707

Open
phudayyy wants to merge 1 commit into
Graphify-Labs:v8from
phudayyy:fix/affected-path-forms
Open

fix: affected silently returns nothing for equivalent path forms#2707
phudayyy wants to merge 1 commit into
Graphify-Labs:v8from
phudayyy:fix/affected-path-forms

Conversation

@phudayyy

Copy link
Copy Markdown

Fixes #2706.

resolve_seed compares the query to the stored source_file as a plain string, so a file can be named three ways and only one resolves:

graphify affected src/x.py       -> 16 results, exit 0
graphify affected ./src/x.py     ->  0 results, exit 0
graphify affected /abs/src/x.py  ->  0 results, exit 0
graphify affected typo.py        ->  0 results, exit 0

The last two lines are the problem: a blast-radius tool answering "nothing depends on this" is an answer people act on, and here it is indistinguishable both from a genuine zero and from a typo — same empty list, same exit 0, nothing on stderr. ./ is what shell completion produces; an absolute path is what any script passes.

The change

Normalise the query to repo-relative form for the source_file comparison only:

  • the label branches above keep the query verbatim, so label resolution is untouched
  • non-path queries pass through unchanged — Path("myFunc()").as_posix() is "myFunc()"
  • an absolute path rooted outside the repo is left alone rather than reduced to its basename, which would match an unrelated file of the same name

30 lines, most of them the comment explaining why.

Verification

test_affected_resolves_equivalent_path_forms covers the three spellings. Red without the change, green with it — checked by reverting only graphify/affected.py and re-running.

Full suite, both ways:

failed passed
without patch 5 4252
with patch 5 4253

Identical failures, all in tests/test_ollama_retry_cap.py — no ollama in this environment — plus the one new test.

How it was found

Measuring affected against an independently built import graph (Python ast, no tree-sitter) on a ~1,400-node Python corpus. The probe passed absolute paths and measured recall 0.000 for every module, which is the same failure one layer up.

`resolve_seed` compares the query to the stored `source_file` as a plain string,
so a file can be named three ways and only one of them resolves:

    graphify affected src/x.py     -> 16 results, exit 0
    graphify affected ./src/x.py   ->  0 results, exit 0
    graphify affected /abs/src/x.py ->  0 results, exit 0
    graphify affected typo.py      ->  0 results, exit 0

The last two lines are the problem. A blast-radius tool answering "nothing
depends on this" is an answer people act on, and here it is indistinguishable
both from a genuine zero and from a typo — same empty list, same exit 0, no
warning. `./` is what shell completion produces and an absolute path is what any
script passes.

Found while measuring `affected` against an independently built import graph:
the probe passed absolute paths and measured recall 0.000 for every module,
which is the same failure one layer up.

Fix: normalise the query to repo-relative form for the `source_file` comparison
only. The label branches above keep the query verbatim, and non-path queries are
unaffected -- `Path("myFunc()").as_posix()` is `"myFunc()"`. An absolute path
rooted outside the repo is left alone rather than guessed at by basename, which
would match an unrelated file of the same name.

Test: `test_affected_resolves_equivalent_path_forms` is red without the change
and green with it.

Suite: 5 failed / 4252 passed without the patch, 5 failed / 4253 passed with it
-- identical failures, all in `tests/test_ollama_retry_cap.py` (no ollama in this
environment), plus the one new test.

@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 2 advisory finding(s) below merit a look before merge.

Formal verification. 1 change(s) tested, no difference found (not proven).


Graphify review — findings

This PR modifies resolve_seed in graphify/affected.py to normalize path-shaped seed queries into a repo-relative form via a new _as_repo_relative helper, so that queries like ./pkg/foo.py, absolute paths, and pkg/foo.py are matched against the graph's stored repo-relative source_file values. Non-path (label) queries are intended to pass through unchanged. A new test in tests/test_affected_cli.py asserts that these three equivalent path forms all resolve to the same node.

Worth a look

  • Absolute path query fails when cwd is not the repo rootgraphify/affected.py:82 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • _as_repo_relative uses cwd instead of repo root, so absolute paths only resolve when cwd is the repo rootgraphify/affected.py:84 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 195 functions depend on the 41 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: resolve_seed() — 11 callers, 4 callees

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

Formal verification

No difference found (not proven): No behavior difference found in resolve\_seed (not a proof).

The verifier ran both versions of resolve\_seed 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 more finding(s) on lines outside this diff (see the check run).

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.

affected silently returns nothing when the path is spelled differently (./ or absolute)

1 participant