Skip to content

fix(cluster): honor --resolution on the community split passes - #2388

Open
keith-oak wants to merge 1 commit into
Graphify-Labs:v8from
keith-oak:fix/cluster-split-resolution
Open

fix(cluster): honor --resolution on the community split passes#2388
keith-oak wants to merge 1 commit into
Graphify-Labs:v8from
keith-oak:fix/cluster-split-resolution

Conversation

@keith-oak

Copy link
Copy Markdown

_split_community() calls _partition() without forwarding resolution, so it always re-partitions at the default 1.0. cluster(resolution=...) (the --resolution flag added in 2d783e5) therefore applies to the top-level partition but is silently dropped by both split passes:

  • the oversized-community split (cluster.py:214)
  • the cohesion re-split (cluster.py:223)

So graphify build --resolution 0.5 partitions the graph at 0.5, then re-partitions its largest communities at 1.0 — the opposite granularity to the one requested.

Change

Thread cluster()'s resolution through to both call sites and give _split_community() a resolution: float = 1.0 parameter.

Default behaviour is unchanged — the default is 1.0 either way, which is what the split passes were already hardcoding. Only runs that pass an explicit --resolution change.

Tests

Two tests in tests/test_cluster.py, both verified to fail without the fix:

  • test_split_community_forwards_resolution_to_partition — asserts the kwarg reaches _partition
  • test_cluster_forwards_resolution_to_split_passes — end-to-end through cluster() on a graph that trips the oversized-split path

Full suite: 3904 passed, 36 skipped. Two failures on this branch are pre-existing on a clean v8 checkout and unrelated (test_ollama_retry_cap.py needs the openai extra; test_labeling.py::test_label_communities_batches_when_over_batch_size asserts a batch completion order).

_split_community() called _partition() with the default resolution of 1.0,
so --resolution applied to the top-level partition but was silently dropped
by both split passes (the oversized-community split and the cohesion
re-split). A run with --resolution 0.5 could therefore have its large
communities re-partitioned at 1.0, contradicting the flag.

Thread cluster()'s resolution through to both call sites. Default behaviour
is unchanged (1.0 either way); only explicit --resolution runs differ.

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

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

Graphify reviewed this change.

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


Graphify review — findings

This PR threads the resolution parameter through the community-splitting logic in graphify/cluster.py. Previously _split_community always ran its secondary Leiden partition with the default resolution of 1.0; now cluster() passes its own resolution value down to both the oversized-community and low-cohesion split passes, and _split_community forwards it to _partition. Two new tests verify that the resolution is forwarded from _split_community to _partition and from cluster into the split passes.

No blocking issues surfaced.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 363 functions depend on the 38 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: cluster() — 61 callers, 3 callees

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

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

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.

1 participant