fix(watch): bring the incremental rebuild in line with cluster-only (stale labels, deleted viz)#2218
Open
bobspryn wants to merge 2 commits into
Open
Conversation
…bels Community labels are saved keyed by community id, but re-clustering reassigns those ids: after a rebuild that adds nodes, cid 30 can cover a completely different community and its saved name is then simply wrong. cluster-only already guards this — it validates each reused label against the `.graphify_labels.json.sig` membership fingerprints and re-hubs any community that changed (the case community_member_sigs() was written for). _rebuild_code() skipped that check entirely: it reused every label whose cid was present and hub-filled only the *missing* ones, so stale names survived — and then wrote them back to labels.json, laundering them as current. It also never refreshed the .sig sidecar, so the signatures kept describing an older clustering and drifted out of step with the labels they sit beside, leaving the cluster-only guard nothing accurate to check. Adding ~3.5k nodes to a real graph re-clustered 463 -> 515 communities and mislabeled 162 of them this way: a `domain.audit` namespace reading "ACH / bank payments", a `domain.auth` namespace reading "document-sensitivity.up.sql". Node and edge data stayed correct, so nothing failed loudly — only the names lied. Apply the same signature check in the incremental path, write the sidecar in step with the labels, and print the same "run `graphify label`" notice cluster-only emits so a drifted community set is visible rather than silent.
Crossing MAX_NODES_FOR_VIZ left the project with no graph.html at all: _rebuild_code unlinked the existing file and wrote nothing in its place. The delete on its own is defensible — a kept graph.html would describe an older, smaller graph — but the file is gone before the user reads the message, and the next incremental rebuild silently removes it again, so a repo that grows past the threshold just loses its visualization with no way to keep one. The export path already solved this (Graphify-Labs#1019): over the cap it re-renders the community-aggregation view rather than going without. Do the same here, so the artifact is current AND present instead of current OR present. GRAPHIFY_VIZ_NODE_LIMIT=0 still means "no HTML viz" (CI runners) rather than "aggregate", and if the aggregated render also fails the old skip-and-remove behaviour stands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two places where
_rebuild_code(thegraphify update/ watch path) diverges from whatcluster-onlyandexportalready do carefully. Both are silent — nothing fails, the outputs are just wrong or missing.1. Stale community labels
Labels are persisted keyed by community id, but re-clustering reassigns those ids. After a rebuild that adds nodes, cid 30 can cover a completely different community, and its saved name is then simply wrong.
cluster-onlyguards this: it validates each reused label against the.graphify_labels.json.sigmembership fingerprints and re-hubs any community whose membership changed. That is exactly the casecommunity_member_sigs()documents:_rebuild_codeskipped the check entirely — it reused every label whose cid was present and hub-filled only the missing ones, so stale names survived, then wrote them back tolabels.jsonas though current. It also never refreshed the.sigsidecar, so signatures kept describing an older clustering and drifted out of step with the labels beside them, leaving thecluster-onlyguard nothing accurate to check against.Impact. Found while adding a language extractor. Extraction grew the graph by ~3.5k nodes, re-clustering 463 → 515 communities and mislabeling 162 of them: a
domain.auditnamespace reading"BOB-34 — ACH / bank payments", adomain.authnamespace reading"…document-sensitivity.up.sql". Node and edge data stayed correct — only the names lied, inGRAPH_REPORT.mdand in query output where labels are the orientation cue. The sidecar had drifted to 300 entries against 515 live communities.Fix. Apply the same signature check, write the sidecar in step with the labels, and print the same "run
graphify label" noticecluster-onlyemits. Falls back tocluster-only's community-count heuristic when no sidecar exists.2. A graph that outgrows the viz cap loses
graph.htmlentirelyOver
MAX_NODES_FOR_VIZ,_rebuild_codeunlinked the existinggraph.htmland wrote nothing. The delete on its own is defensible — a kept file would describe an older, smaller graph — but it is gone before the user reads the message, and every later rebuild removes it again, so a project that crosses the threshold just loses its visualization with no way to keep one.export htmlalready solved this (#1019): over the cap it re-renders the community-aggregation view rather than going without. This does the same, so the artifact is current and present rather than current or present.GRAPHIFY_VIZ_NODE_LIMIT=0still means "no HTML viz" (CI runners) rather than "aggregate", and if the aggregated render also fails, the old skip-and-remove behaviour stands.Tests
test_rebuild_code_drops_labels_whose_community_changed— labels a corpus, grows it so clustering shifts, rebuilds, and asserts no surviving label sits on a changed community, plus that the sidecar is rewritten in step.test_rebuild_code_keeps_a_visualization_when_over_the_viz_cap— drops the cap below the graph size but above its community count (the real shape of this bug), assertsgraph.htmlsurvives and is re-rendered, and thatGRAPHIFY_VIZ_NODE_LIMIT=0still disables it outright.Both confirmed to fail on the unfixed file before the change. Full suite on this branch: 3764 passed (3 unrelated
test_ollama.pybackend-detection failures reproduce on a clean checkout — they trip on aGEMINI_API_KEYin the environment).