fix(cli): apply remap_communities_to_previous in cluster-only path - #1028
Conversation
|
After re-reading The
And Step 5 itself reads:
That language reads to me as "label from scratch using the new community contents", not "reuse the prior labels file". If that's correct, then the existing behavior of If that interpretation matches what you had in mind, two alternatives look more consistent with
Apologies for not raising this in the original PR description — I was anchoring too hard on "watch already does this, mirror it." Could you let me know which direction you'd prefer?
I'm happy to amend this PR in whichever direction you point at — or to close it and open a fresh one if you'd rather start from a clean slate. |
cluster-only re-runs Leiden clustering and then re-applies the existing .graphify_labels.json by raw cid index, which causes labels to attach to clusters whose members are unrelated to the label's original meaning whenever the graph has changed between labeling and re-clustering. Mirror the safety net already present in watch.py:_rebuild_code added in Graphify-Labs#822 for the watch/update paths. Adds a regression test that fails without the fix (label cids become orphaned from graph.json community attributes after re-clustering). Refs: Graphify-Labs#1027 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8762a33 to
bf43efb
Compare
Summary
cluster-onlyre-runs Leiden clustering and then re-applies the existing.graphify_labels.jsonby raw cid index, without invoking theremap_communities_to_previoussafety net that #822 added to thewatch/updatepaths. As a result, labels become misaligned with theactual community contents whenever the graph has changed between the
labeling pass and the
cluster-onlyinvocation — including the officialupdate --no-cluster+cluster-onlyworkflow introduced by #822.Closes #1027.
What changed
Mirror the existing pattern in
graphify/watch.py:_rebuild_code:previous_node_communityfrom_raw(already loaded a few linesabove) using each node's
communityattribute.remap_communities_to_previous(communities, previous_node_community)immediately after
cluster(...)so the existing labels file keepstracking the same conceptual communities by node-overlap, not by raw
cid index.
Diff is contained to the
elif cmd == "cluster-only":branch ingraphify/__main__.py(one import addition, one block right after thecluster(...)call).Test
Added
tests/test_cli_export.py::test_cluster_only_remaps_labels_to_previous_cids.The test tags every node with out-of-band sentinel community ids
(
4242/9999), writes a labels file keyed on those ids, then runsgraphify cluster-only. With the fix, at least one sentinel cid stillappears in the final
graph.jsoncommunity attributes (= remap matchedby node overlap). Without the fix, Leiden renumbers to
0, 1, ...andthe prior labels become orphaned.
Verified locally:
Full suite passes:
Open question
The
remap_communities_to_previoushelper already exists ingraphify.clusterand is used correctly inwatch.py:_rebuild_code.#822's description explicitly mentions:
— but
cluster-onlywas not updated alongside the watch/update path. Theneighbouring code paths re-applying labels (
watch.py:_rebuild_code,update) all go through remap;cluster-onlyis the onlyre-clustering CLI entry point that omitted it.
Was there a design reason to deliberately keep
cluster-onlywithoutthe remap (e.g. a use case where label inheritance is undesirable)?
If so, happy to revise this PR — for example, gate the remap behind an
opt-out flag, or expose a
--reset-labelsswitch instead. Otherwise itlooks like a simple miss when #822 was applied and this PR just mirrors
the existing pattern.
Refs
remap_communities_to_previous: Reduce update churn and stabilize community IDs #822