fix(wiki): count each incident edge once in the Audit Trail confidence split - #2635
fix(wiki): count each incident edge once in the Audit Trail confidence split#2635rajarshidattapy wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article 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 \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"\-1","nodes":"\[3, 1, 2\]","label":"'a longer string'","labels":"\{'n': 0, 's': 'x', 'l': \[1, 2\]\}","cohesion":"2\.5","node\_community":"\{'n': 0, 's': 'x', 'l': \…, the old code produced '\# a longer string\\n\\n\> 3 nodes · cohesion 2\.50\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationshi… but the new code produces '\# a longer string\\n\\n\> 3 nodes · cohesion 2\.50\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationshi…. Paste that input straight into a regression test.
Graphify review — findings
This PR changes how the confidence breakdown in a community's "Audit Trail" section is computed in graphify/wiki.py. Instead of iterating over nodes x G.neighbors and reading edge data via edge_data, it now uses G.edges(nodes, data=True) to enumerate incident edges and tally their confidence values. The stated intent is to count each incident edge once (rather than double-counting intra-community edges) and to count parallel MultiGraph edges individually. The test file updates the existing test_community_article_has_audit_trail to assert specific per-confidence counts and percentages (rather than mere substring presence), and adds a new test_audit_trail_counts_parallel_edges_individually test exercising a MultiGraph with parallel edges. The large set of "changed symbols" appears to be mostly test/rationale identifiers surfaced by the diff rather than substantively edited code. Surface area: one function's counting logic in wiki.py plus corresponding test assertions.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 212 functions depend on the 68 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
to_wiki()— 34 callers, 6 callees
Verification — 212 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: 73 function(s) in the blast radius were not formally verified this run
Formal verification
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article 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 \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"\-1","nodes":"\[3, 1, 2\]","label":"'a longer string'","labels":"\{'n': 0, 's': 'x', 'l': \[1, 2\]\}","cohesion":"2\.5","node\_community":"\{'n': 0, 's': 'x', 'l': \…, the old code produced '\# a longer string\\n\\n\> 3 nodes · cohesion 2\.50\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationshi… but the new code produces '\# a longer string\\n\\n\> 3 nodes · cohesion 2\.50\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationshi…. Paste that input straight into a regression test.
· 1 more finding(s) on lines outside this diff (see the check run).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed in v0.9.41 ( |
|
Shipped in v0.9.41 ( |
Fixes #2633.
The bug
_community_articlebuilt the confidence breakdown by walkingnodes × G.neighbors, whichvisits the unordered pair
(a, b)once asa→band again asb→awhen both endpoints arecommunity members. So:
Intra-community edges are overwhelmingly the high-confidence
EXTRACTEDones — that is whatmakes a community a community — while crossing edges are disproportionately the uncertain
ones. The reported split was therefore biased towards confidence, understating the
AMBIGUOUSshare thatARCHITECTURE.mddocuments as "flagged for human review". Thesection exists to size a reviewer's workload and was systematically shrinking it.
The reported repro (3 edges) printed
EXTRACTED: 4 (80%) / AMBIGUOUS: 1 (20%)— a total of5 for 3 edges.
Fix
networkx already reports each incident edge exactly once, so no dedup bookkeeping is needed:
G.edges(nbunch)gives precisely the semantics this section wants — intra-community edgesonce, crossing edges once, self-loops once — and tolerates duplicate or absent entries in
nodes. Net −5 lines, and it drops theseen-set the issue proposed along with theper-pair
edge_datalookup.On "belonging to" vs "touching" the community
The issue asks which is meant. The Expected output settles it: it keeps the crossing
c→xAMBIGUOUS edge in the denominator (1/3, not0/2). That also matches the issue's ownargument — crossing edges are the uncertain ones, so excluding them would understate the
review burden further. Cross-community edges stay in; only the double-count is removed.
Made explicit in a comment so the next reader doesn't have to re-derive it.
One deliberate extra
On a
MultiGraph,edge_datareturns only the first parallel edge, so anAMBIGUOUSedge running alongside an
EXTRACTEDone between the same pair was invisible in the split.G.edges(..., data=True)counts each individually. This is the same class of understatementas the double-count, but it is beyond what was reported and will raise MultiGraph totals
where parallel edges exist. Easy to revert to pair-collapsing if that is not wanted.
Verification
EXTRACTED: 4 (80%),AMBIGUOUS: 1 (20%)EXTRACTED: 2 (67%),AMBIGUOUS: 1 (33%)Matches the Expected output exactly.
test_community_article_has_audit_trailexisted but only asserted"EXTRACTED" in parsing— substring checks, which is why arithmetic this far off went unnoticed. Replaced with the
actual counts and percentages (its fixture is already one intra + one cross edge, i.e. the
defect), plus a new
test_audit_trail_counts_parallel_edges_individuallyfor the MultiGraphbehavior. Both fail before the change and pass after.
Full suite: same 42 pre-existing failures as baseline (Windows/env), no new ones.
Scope
_cross_community_linksneeds no change, as the issue notes — a crossing edge has oneendpoint in
nodesand is naturally counted once. Swept the rest of the codebase for thesame
nodes × neighborspattern used as an edge counter; the neighbor loops inanalyze.py,export.py,serve.py,cluster.pyandbenchmark.pyare traversals orper-node listings, not edge tallies. This was the only affected site.