fix(markdown): resolve review findings and keep mention edges across rebuilds - #3587
AstroMined wants to merge 2 commits into
Conversation
…n mention resolution Three findings from the automated review of Graphify-Labs#3562, all reproduced: A dotted span with a non-code extension (README.md, pyproject.toml) was classified as a symbol because only CODE_EXTENSIONS was checked, and every dotted form was reduced to its last segment, so time.sleep matched a repo's own sleep(). The dotted form now keeps its whole chain, the file early-exit also checks DOC_EXTENSIONS, and a bare-name match must carry every qualifier as an owner label or a segment or stem of its source path. _match_cited_file used lstrip("./"), which strips characters, so a .github/... cite could never match by suffix. The leading ./ and ../ segments are now stripped as segments, and the suffix form also matches a root-level file. An explicit ./ or ../ cite that escaped the corpus fell through to the suffix rule and could land on an unrelated copy of the file. It now resolves relative to the document only.
…isting graph _reconcile_markdown_links treats every AST-tier references edge owned by a re-parsed Markdown file as an authored [link](file) and prunes any pair it did not author. A code-span mention is also a references edge owned by its document, so a plain `graphify update` over an existing graph, with or without changes, dropped every mention edge; only a build from scratch kept them. A mention targets a code symbol, never a file representative, so the reconcile now leaves those edges to the AST ownership rule: re-extracting the document regenerates them and re-extracting the code side keeps or drops them with the target node.
![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 2 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_match\_cited\_file changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_match\_cited\_file 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 \{"cited":"'\./'","doc\_file":"'\.mdx'","source\_files":"\{1, 2, 3\}"\}, the old code produced raises AttributeError but the new code produces None. Paste that input straight into a regression test.
Graphify review — findings
Restricts code-span mention resolution so a dotted mention (pkg.Widget, Widget.render) only matches a candidate whose owner chain or source-path segments cover every qualifier, preventing time.sleep from landing on a repo's own sleep; _evidence computes those qualifier labels from contains/method owners and path segments. Classifies dotted spans ending in a code or doc extension (setup.py, README.md) as files while treating other dotted forms as mentions that carry their qualifiers into _code_span_mention. Makes an explicit relative cite (./, ../) in _match_cited_file resolve against the document's directory only and never fall through to the suffix rule, and in watch mode leaves code-span references edges to the AST re-extraction rule rather than the file-representative reconciliation.
Worth a look
- Explicit relative cites no longer use suffix fallback —
graphify/markdown_resolution.py:90· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Stale markdown code-span reference edges are unconditionally preserved —
graphify/watch.py:668· 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 — 493 functions depend on the 134 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 125 callers, 53 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
extract_markdown()— 17 callers, 9 callees - new:
watch()— 5 callers, 7 callees - new:
_reconcile_graph_html()— 6 callers, 5 callees - new:
_reconcile_existing_graph()— 1 callers, 8 callees - new:
resolve_markdown_mentions()— 1 callers, 7 callees - new:
_reconcile_markdown_links()— 1 callers, 7 callees - …and 1 more — each is listed as a finding
Verification — 493 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: 319 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
6 of 283 test file(s) selected (2%) via static blast radius.
tests/test_labeling.py— impacttests/test_languages.py— impacttests/test_markdown_code_spans.py— impact, changed-testtests/test_terraform_modules.py— impacttests/test_watch.py— impacttests/test_watch_manifest_location.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
Behavior changes: \_match\_cited\_file changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_match\_cited\_file 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 \{"cited":"'\./'","doc\_file":"'\.mdx'","source\_files":"\{1, 2, 3\}"\}, the old code produced raises AttributeError but the new code produces None. Paste that input straight into a regression test.
No difference found (not proven): No behavior difference found in \_code\_span\_mention (not a proof).
The verifier ran both versions of \_code\_span\_mention 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 extract\_markdown.
The verifier did not have enough to check extract\_markdown, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
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.
Could not verify: Could not verify \_reconcile\_markdown\_links.
The verifier did not have enough to check \_reconcile\_markdown\_links, 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: parameter `project_root` is annotated `Path` — outside the synthesizable primitive/collection set
· 9 more finding(s) on lines outside this diff (see the check run).
|
Shipped in v0.9.63 (on PyPI). Cherry-picked with authorship preserved. The rebuild-persistence fix (routing the survival test through the real rebuild) and the dotted-qualifier evidence check were both the right calls. Thanks @AstroMined! |
2 participants
Follow-up to #3562, which shipped in 0.9.62 as c805c6f. The two commits pushed to that branch after the automated review were not cherry-picked, so 0.9.62 carries the reviewed defects. This PR rebases them onto v8 unchanged.
fix(markdown): honour dotted qualifiers and explicit relative cites
README.md,pyproject.tomlandtime.sleepno longer match a same-named callable. The file early-exit also checksDOC_EXTENSIONS._match_cited_filestrips./and../as prefixes, so.github/...cites can match by suffix, and a root-level file can match the suffix form../or../cite resolves relative to the document only; it never falls through to the suffix rule.fix(watch): keep code-span mention edges through a rebuild over an existing graph
_reconcile_markdown_linkstreated every AST-tierreferencesedge owned by a re-parsed markdown file as an authored[link](file)and pruned the code-span mention edges, so the firstgraphify updateafter a full build dropped every doc-to-code edge. Mention edges target code symbol nodes rather than file nodes and are now kept.Tests:
tests/test_markdown_code_spans.pycovers each case; the rebuild test fails on v8 without the watch change and passes with it.