Repository navigation
fix(markdown): a bare method name needs its class cited (#4609) - #4617
nothariharan wants to merge 2 commits into
Conversation
…s#4609) `resolve_markdown_mentions` linked any bare, parenthesis-less code span to the corpus's single `_callable` carrying that label, and methods qualify. A prose word in a code span (`redis` the server, `data`, `message`) then became `heading --references--> SomeClass.word()` at INFERRED 0.95, on a method that has nothing to do with the sentence. On the reporter's corpus 44 of 1,906 mention edges were words of this kind. A bare single-name span is now resolved in three passes. Explicit evidence (a cited path, a dotted qualifier chain, or a trailing `()`) resolves as before. Bare mentions that do not name a method keep resolving, and their targets are recorded per document. A bare mention that names a method is kept only when its owner class is already cited in the same document, so `redis` no longer links to `Cache.redis` while `successWithWarnings` next to `HandlerResult` still does. Ambiguity, built-ins and the case-sensitive label match are unchanged. Tests cover the issue repro, the owner-cited case, the `()` case, a free function, and the cross-document guard.
github-actions
Bot
added
the
contributor
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
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 1 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 1 changed function(s) (1 sampled, not proven).
Formal verification. PR-changed functions: 1/1 verified (0 proven, 1 may-equivalent, 0 distinguished) · 0 not verified.
Graphify review — findings
Tightens markdown mention resolution so a bare, parenthesis-less method name like redis only links to Cache.redis when the same document also cites the owning class. A trailing (), a dotted form, or a cited path still resolves on its own, and bare class or free-function names keep matching as before. The resolver now runs in passes, with _add recording each document's cited targets as owner evidence for the method pass, and that bookkeeping is where a class edge that already exists can leave a later method mention without evidence.
Worth a look
- Owner evidence not recorded when the class edge already exists, so method mentions are dropped on rebuild —
graphify/markdown_resolution.py:233· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 470 functions depend on the 44 functions this change touches.
Health — grade A; 9 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
_rebuild_code()— 187 callers, 60 callees (high)dispatch_command()— 5 callers, 134 callees (high)main()— 110 callers, 3 callees (high)_run_cli()— 11 callers, 8 callees (high)watch()— 6 callers, 10 callees (high)_reconcile_existing_graph()— 1 callers, 11 callees (medium)resolve_markdown_mentions()— 1 callers, 7 callees (medium)_reconcile_markdown_links()— 1 callers, 7 callees (medium)- …and 1 more
Verification — 470 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: 216 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
2 of 436 test file(s) selected (0%) via static blast radius.
tests/test_markdown_code_spans.py— impact, changed-testtests/test_watch.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 resolve\_markdown\_mentions (not a proof).
The verifier ran both versions of resolve\_markdown\_mentions 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.
The review bot flagged a real hazard in the owner-evidence bookkeeping: a mention whose `references` edge is already in the graph returned early, so the document never recorded that it cites the target. A later bare method mention in the same document then lost its owner and was dropped. `_add` now records the cited target before it decides whether the edge has to be written, so the evidence survives an edge that is already there. The pipeline cannot reach that state today (the resolver is handed a fresh edge list, and the incremental context only carries contains/method/ inherits/imports edges), so this is hardening rather than a fix for a reachable failure. The regression test drives the resolver directly with a pre-existing class edge.
This branch has not been deployed
1 participant
Fixes #4609.
What breaks
Any bare, parenthesis-less code span was linked to the corpus's single node carrying that label, and methods carry labels too. So a docs sentence like
became
Queue --references--> Cache.redisat INFERRED / 0.95 — an edge to a method the sentence never mentions. On the reporter's corpus 44 of 1,906 mention edges were plain English words (data,message,request,previous,moved,integrations,signature).Fix
resolve_markdown_mentionsnow resolves bare single-name spans in three passes:()— resolves exactly as before. These targets are recorded per document.So
redisno longer links toCache.redis, while the case the issue says should keep its edge still does:Ambiguity handling, language built-ins, case-sensitive label matching, path-qualified resolution and the extraction cache are untouched. No schema or cache-format change: the resolver reads the span text already stored in
context.Tests
Added to
tests/test_markdown_code_spans.py:redisrepro yields no edgesuccessWithWarningsnext toHandlerResultstill resolvesredis()with parentheses still resolvesdeploy) still resolvespython -m pytest tests/test_markdown_code_spans.py tests/test_watch.py tests/test_cache.py -q→ 288 passed, 8 skipped.ruff checkclean.Follow-up: the review bot's owner-evidence finding
The Graphify review bot flagged, correctly, that
_addreturned early when a mention'sreferencesedge was already in the graph — so the document never recorded that it cites the target, and a later bare method mention in the same document lost its owner evidence and was dropped.The state it needs is not reachable in the pipeline today: the resolver is handed a fresh edge list, and the incremental context only carries
contains/method/inherits/importsedges, so noreferencesedge can pre-exist at resolver time (verified by instrumentingrun_language_resolversduring an incremental rebuild: 0referencesedges in the edge list on every call). It is still a real hazard in the bookkeeping, so_addnow records the cited target before deciding whether the edge has to be written, and a regression test drives the resolver directly with a pre-existing class edge.