Skip to content

fix(extract): collapse byte-identical duplicate edges at emission (#3251) - #3263

Open
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:fix/duplicate-annotation-edges
Open

fix(extract): collapse byte-identical duplicate edges at emission (#3251)#3263
abhay-codes07 wants to merge 1 commit into
Graphify-Labs:v8from
abhay-codes07:fix/duplicate-annotation-edges

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

Closes #3251.

The problem

def two_params(a: Path, b: Path) emits the (two_params → Path, references, L4, parameter_type) edge twice — one per annotation occurrence. The copies are identical in every field, so diagnose_extraction counts one under exact_duplicate_edges and build's dedup then drops it: the graph ends up correct, but the run reports a GRAPH HEALTH WARNING with no actionable cause, exactly as the issue describes.

The disease is not Python-specific. Every language block in _extract_generic emits per-occurrence, and neither add_edge nor the raw appends de-duplicate — C#'s void Copy(Widget a, Widget b) double-emits the same way, and s.AddScoped<IZeta, Box<IZeta>>() (same type in two generic-argument positions of one call) trips the identical warning.

The change

The engine's existing clean-edges pass now collapses byte-identical edges — same full payload after json.dumps(sort_keys=True) — keeping the first occurrence. Anything differing in any field survives untouched: two locations, two contexts at one location (parameter_type vs return_type), differing metadata. This matches the issue's expected contract ("one references edge per (source, target, relation, location, context) tuple") and only removes what build's dedup was already discarding after the health check had counted it.

One existing test asserted the duplicate: test_csharp_call_site_generic_args.py counted two identical IZeta edges from AddScoped<IZeta, Box<IZeta>>() to prove both generic-argument positions were walked. Those two occurrences are one relationship at one location (and were themselves tripping the #3251 warning); the assertion now checks the edge exists exactly once plus the Box argument link, which still fails against the original call-site bug (which emitted nothing).

Tests

tests/test_duplicate_annotation_edges.py — 4 tests: the issue's exact Python repro yields one edge per relationship; the C# witness for the cross-language claim; diagnose_extraction reports exact_duplicate_edges: 0 on the repro corpus; and a survival control proving different locations and different contexts keep every edge. With the fix reverted, 3 of 4 fail (the survival control rightly keeps passing). The extraction/diagnostics slice is unchanged (871 passed; the one failure, test_c_include_out_of_root_target_id_is_deterministic_across_checkout_paths, fails identically on clean v8 on this Windows machine); the full suite matches a fresh same-version v8 baseline.

Copilot AI lite review requested due to automatic review settings September 1, 2026 18:20

@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.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Collapses byte-identical duplicate edges during _extract_generic's edge-cleaning pass, so a signature annotating the same type twice (def f(a: Path, b: Path)) emits one references edge instead of one per occurrence; edges differing in any field (source_location, context, metadata) still all survive. This kills the spurious exact_duplicate_edges GRAPH HEALTH WARNING from diagnose_extraction (build already dropped the copies). Updates the C# generic-args test to expect a single IZeta edge while still asserting the Box argument links, and adds test_duplicate_annotation_edges covering Python, C#, the diagnostics count, and the survival of differing edges.

No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 655 functions depend on the 229 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _extract_generic() — 18 callers, 25 callees
  • new: extract_js() — 85 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: extract_objc() — 27 callers, 9 callees
  • new: extract_julia() — 17 callers, 7 callees
  • new: extract_cpp() — 27 callers, 3 callees
  • new: extract_vue() — 10 callers, 7 callees
  • new: walk() — 1 callers, 57 callees
  • …and 8 more — each is listed as a finding

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

Formal verification

Could not verify: Could not verify \_extract\_generic.

The verifier did not have enough to check \_extract\_generic, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

· 16 more finding(s) on lines outside this diff (see the check run).

Copilot AI 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.

Pull request overview

This PR addresses #3251 by collapsing byte-identical duplicate edges during extraction so diagnose_extraction no longer reports non-actionable exact_duplicate_edges health warnings for repeated type mentions at the same source location/context.

Changes:

  • Deduplicate byte-identical edges during the engine “clean edges” pass.
  • Add a focused regression test suite covering repeated-annotation duplicates and ensuring differing locations/contexts still produce distinct edges.
  • Update the existing C# generic-args call-site test to expect one IZeta edge after extraction-level dedupe.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
graphify/extractors/engine.py Adds byte-identical edge dedupe while producing clean_edges.
tests/test_duplicate_annotation_edges.py New tests reproducing #3251 and asserting duplicates are eliminated while distinct edges survive.
tests/test_csharp_call_site_generic_args.py Updates expectations for C# generic-arg extraction to account for dedupe.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +56 to +58
r = extract([tmp_path / "Svc.cs"], cache_root=tmp_path)
dupes = {k: v for k, v in _edge_counts(r).items() if v > 1}
assert dupes == {}, f"exact duplicate edges emitted: {dupes}"
for edge in edges:
src, tgt = edge["source"], edge["target"]
if src in valid_ids and (tgt in valid_ids or edge["relation"] in ("imports", "imports_from", "re_exports")):
payload = json.dumps(edge, sort_keys=True, default=str)
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.

Python AST: a repeated parameter annotation emits an exact-duplicate edge, flagged as a health warning then silently dropped

2 participants