fix(cluster): exclude self-loops from cohesion_score so the ratio sta…fix(cluster): exclude self-loops from cohesion_score so the ratio stays in 0..1 - #3558
Conversation
…ys in 0..1 cohesion_score() counts self-loops in the numerator (number_of_edges) but not in the denominator (n * (n - 1) / 2, distinct node pairs), so the ratio its docstring defines over 0..1 can exceed 1.0. Recursive `calls` self-edges are deliberately preserved by build_from_json, so they reach this code in ordinary graphs: a two-node community holding one recursive function scored 2.0, and report.py prints the value unclamped.
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) tested, no difference found (not proven).
Graphify review — findings
Fixes cohesion_score to subtract self-loops from the edge count so recursive calls self-edges no longer inflate the ratio above 1.0 (a two-node community with one recursive function previously scored 2.0), keeping numerator and denominator both measured over distinct node pairs.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 472 functions depend on the 21 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 116 callers, 51 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_make_graph()— 33 callers, 6 callees - new:
cluster()— 61 callers, 3 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
make_inputs()— 16 callers, 5 callees - new:
suggest_questions()— 11 callers, 4 callees - new:
watch()— 5 callers, 7 callees - …and 11 more — each is listed as a finding
Verification — 472 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: 268 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
13 of 275 test file(s) selected (5%) via static blast radius.
tests/test_analyze.py— impacttests/test_build.py— impacttests/test_cli_export.py— impacttests/test_cluster.py— impacttests/test_community_hub_labels.py— impacttests/test_confidence.py— impacttests/test_export.py— impacttests/test_pipeline.py— impacttests/test_reflect.py— impacttests/test_report.py— impacttests/test_serve.py— impacttests/test_serve_http.py— impacttests/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 cohesion\_score (not a proof).
The verifier ran both versions of cohesion\_score 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.
· 19 more finding(s) on lines outside this diff (see the check run).
|
Shipped in v0.9.62 (now on PyPI: |
Problem
cohesion_score()counts self-loops in the numerator (number_of_edges()) but not inthe denominator (
n * (n - 1) / 2, distinct node pairs). Recursivecallsself-edgesare deliberately preserved by
build_from_json(build.py:1294), so they reach thiscode in ordinary graphs and push the ratio past the 0..1 range the docstring defines.
report.py:289prints the score unclamped, soGRAPH_REPORT.mdcould showCohesion: 2.00.Change
One line in
cohesion_score(): subtractnx.number_of_selfloops(subgraph)from the edgecount, with a comment explaining why the self-loops are there in the first place.
Clamping the output was the other option, but that would hide a miscount rather than fix
it, and it would leave the inflated sub-1.0 scores wrong.
Tests
Three cases added to
tests/test_cluster.py, beside the existing cohesion tests:test_cohesion_score_ignores_self_loops— the 2.0 casetest_cohesion_score_range_with_self_loops— the 0..1 invariant with every node recursivetest_cohesion_score_self_loops_only_is_zero— self-loops alone are not a connectionAll three fail on
mainand pass with the fix. The existingtest_cohesion_score_rangealready asserts the invariant but runs on a fixture with noself-loops, so it could not catch this.
Behaviour change
Communities containing recursive functions now report lower, correct cohesion.
cluster()reads this score atcluster.py:320to decide re-splitting(
< 0.05, communities of 50+ nodes), so a community whose inflated score sat just abovethat threshold could now be split. Checked against a real 2048-node graph with 38
self-loops: 15 of 104 communities report corrected scores, none crosses the re-split
threshold, clustering output unchanged.