fix(analyze): god_nodes honours exclude_hubs_percentile (#3205) - #3239
fix(analyze): god_nodes honours exclude_hubs_percentile (#3205)#3239abhay-codes07 wants to merge 1 commit into
Conversation
…s#3205) --exclude-hubs only ever reached cluster()'s community-resolution step; god_nodes() had no exclusion parameter at all, so running the ranking at every percentile returned identical output and utility hubs stayed on top regardless of the setting. god_nodes() now takes exclude_hubs_percentile with cluster()'s exact threshold computation (degrees sorted ascending, idx = max(0, int(n*pct/100)-1), degree > degrees[idx] is a hub), so one setting suppresses hubs in the ranking and in community resolution alike. Wired through every surface that already had the flag or should: the god-nodes CLI command gains --exclude-hubs, the extract and cluster-only paths thread the percentile they already parsed for cluster(), and the MCP god_nodes tool accepts exclude_hubs_percentile. Default None keeps the historical ranking byte-identical.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds an exclude_hubs_percentile parameter to god_nodes that drops any node whose degree exceeds the given percentile of the degree distribution, computed with the same threshold formula cluster() uses so one setting suppresses utility hubs in both the ranking and community resolution; None keeps the historical ranking unchanged. Wires it through the god-nodes CLI command via --exclude-hubs/--exclude-hubs= (rejecting non-numeric values with a usage error), through the MCP god_nodes tool's exclude_hubs_percentile input, and reuses the percentile already passed to cluster() when the cluster/analyze flows compute their god-node lists.
Worth a look
- Out-of-range hub percentile can raise IndexError —
graphify/analyze.py:123· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 984 functions depend on the 220 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 113 callers, 50 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
to_json()— 56 callers, 7 callees - new:
dispatch_command()— 2 callers, 123 callees - new:
generate()— 32 callers, 7 callees - new:
_make_graph()— 32 callers, 6 callees - new:
to_html()— 17 callers, 11 callees - new:
_query_graph_text()— 20 callers, 9 callees - …and 27 more — each is listed as a finding
Verification — 984 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: 912 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in god\_nodes (not a proof).
The verifier ran both versions of god\_nodes 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.
Could not verify: Could not verify dispatch\_command.
The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_build\_server.
The verifier did not have enough to check \_build\_server, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous
· 1 grounded finding(s) anchored inline below; 34 more finding(s) on lines outside this diff (see the check run).
|
|
||
|
|
||
| def god_nodes(G: nx.Graph, top_n: int = 10) -> list[dict]: | ||
| def god_nodes(G: nx.Graph, top_n: int = 10, |
There was a problem hiding this comment.
god_nodes()
32 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Closes #3205.
The problem
--exclude-hubsonly ever reachedcluster()'s community-resolution step.god_nodes()had no exclusion parameter at all — the reporter ran the ranking at every percentile value on a ~14k-node graph and got identical output every time, with utility hubs pinned to the top regardless of the setting.The change
god_nodes()gainsexclude_hubs_percentileusingcluster()'s exact threshold computation (degrees sorted ascending,idx = max(0, int(n·pct/100) − 1), anything with degree abovedegrees[idx]is a hub), so the one setting suppresses hubs in the ranking and in community resolution alike. DefaultNonekeeps the historical ranking byte-identical.Wired through every surface:
god-nodesCLI command gains--exclude-hubs N(both flag forms; a non-numeric value is a usage error);extractandcluster-onlypaths pass the percentile they already parsed forcluster(), so--exclude-hubson a build now shapes the report's god-node list too;god_nodestool acceptsexclude_hubs_percentilein its schema.Tests
tests/test_god_nodes_exclude_hubs.py— 7 tests on a hub-and-spokes graph (hub label chosen outside_BUILTIN_NOISE_LABELSso the test isolates the percentile mechanism): the default ranking is unchanged; the percentile suppresses the hub without emptying the ranking; the exclusion set matchescluster()'s formula computed independently; percentile 100 excludes nothing; the signature default staysNone; and the CLI takes both flag forms with a usage error for garbage. With the fix reverted, 6 of 7 fail. The analyze suites are unchanged (56 passed together); the full suite matches the freshv8(0.9.53) baseline.