Skip to content

Split god-nodes ranking by in-degree and out-degree (#2488) - #3633

Open
ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-2488-god-nodes-in-out-split
Open

ayushcodes10 wants to merge 3 commits into
Graphify-Labs:v8from
ayushcodes10:fix-2488-god-nodes-in-out-split

Conversation

@ayushcodes10

Copy link
Copy Markdown
Contributor

Summary

Fixes #2488. `god-nodes` ranked purely by total degree. On a directed graph, in-degree and out-degree answer opposite questions: high in-degree means many things depend on this node (breaking it breaks the rest), high out-degree means the node itself depends on many things (it breaks easily). Collapsing them into one number ranked a heavily-relied-on module (e.g. an auth guard with 27 inbound edges) right next to a config object like `tsconfig.json`'s `compilerOptions` (mostly outbound edges into its own children) as though comparable — and let wide, shallow config trees crowd the top of the ranking.

Fix

  • `god_nodes()` takes a new `sort_by` parameter (`"total"` / `"in"` / `"out"`, default `"total"` — unchanged historical behavior). Every result on a directed graph now carries `in_degree`/`out_degree` regardless of which sort was requested, so a caller can always show the split. An undirected graph has no in/out concept: `sort_by` is silently ignored and results carry neither key (backward compatible — every existing caller of `god_nodes()` accesses specific dict keys, so the additive keys are harmless).
  • The CLI gains `--by total|in|out`, documented in `graphify --help`, and text output now shows the split (`27 edges (27 in / 1 out)`) whenever the graph is directed.
  • Smaller related fix from the same issue: when several distinct nodes share one printed label, the text output now names the count (`[3 nodes share this label]`) instead of the ranking looking like a single unambiguous, actionable line.

Test plan

  • New tests in `tests/test_analyze.py`: directed graphs carry `in_degree`/`out_degree`, undirected graphs carry neither, sorting by `in`/`out` ranks correctly, and an unrecognized `sort_by` degrades to total rather than raising.
  • New tests in `tests/test_god_nodes_cli.py`: text output shows the in/out split, `--by` works end to end (including an invalid value erroring cleanly), and the shared-label ambiguity marker fires.
  • Confirmed every existing consumer of `god_nodes()` (`report.py`, `serve.py`'s MCP tool, `watch.py`, `llm.py`'s community-labeling prompt) accesses specific dict keys only — the new optional keys don't affect any of them.
  • Full suite: `python3 -m pytest -q` — 5630 passed, 68 skipped, no regressions.

🤖 Generated with Claude Code

ayushcodes10 and others added 3 commits September 17, 2026 20:03
god_nodes ranked purely by total degree. On a directed graph, in
degree and out degree answer opposite questions: high in degree
means many things depend on this node and it breaks the rest if it
changes, high out degree means the node itself depends on many
things and breaks easily. Collapsing them into one number ranked a
heavily relied on module next to a config file with many outbound
only edges as though they were comparable, and let wide shallow
config trees crowd the top of the list.

god_nodes now takes a sort_by parameter of total, in, or out
(defaulting to total, the historical behavior), and every result on
a directed graph carries in_degree and out_degree regardless of the
chosen sort so a caller can always show the split. An undirected
graph has no in or out concept, so sort_by is ignored there and
results carry neither key.

The CLI gains a by flag accepting total, in, or out, and prints the
split alongside the total in text output, plus a small related fix:
when several distinct nodes share one printed label, the output now
names the count so the ambiguity is visible instead of looking like
one safe to act on line.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers a directed graph carrying in_degree/out_degree on every
result, an undirected graph carrying neither key, sorting by total,
in, and out, an unrecognized sort falling back to total instead of
raising, the CLI text output showing the split, the by flag end to
end including an invalid value, and the shared label ambiguity
marker.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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

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. 2 change(s) tested, no difference found (not proven).


Graphify review — findings

Splits god_nodes ranking into in-degree and out-degree on directed graphs, attaching in_degree/out_degree to every result and letting a new --by total|in|out flag pick the ranking metric (defaulting to total). On undirected graphs in/out has no meaning, so sort_by is ignored and those keys are omitted. Text output now appends the in/out split and flags when a printed label is shared by more than one node so an ambiguous line doesn't look actionable.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 995 functions depend on the 407 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 129 callers, 54 callees
  • new: to_obsidian() — 38 callers, 14 callees
  • new: to_json() — 56 callers, 7 callees
  • new: dispatch_command() — 2 callers, 125 callees
  • new: generate() — 34 callers, 7 callees
  • new: _make_graph() — 33 callers, 6 callees
  • new: to_html() — 17 callers, 11 callees
  • new: god_nodes() — 37 callers, 3 callees
  • …and 19 more — each is listed as a finding

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

Test selection

Test selection

286 of 286 test file(s) selected (100%) via static blast radius.

Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.

  • tests/test_affected_cli.py — impact, full-run-safety
  • tests/test_affected_member_seed.py — full-run-safety
  • tests/test_agents_platform.py — impact, full-run-safety
  • tests/test_analyze.py — impact, changed-test, full-run-safety
  • tests/test_anthropic_custom_endpoint.py — full-run-safety
  • tests/test_antigravity_install.py — full-run-safety
  • tests/test_apm_fallback_version.py — full-run-safety
  • tests/test_architecture_doc.py — full-run-safety
  • tests/test_astro_extraction.py — full-run-safety
  • tests/test_astro_import_ids.py — full-run-safety
  • tests/test_atomic_canvas_export.py — impact, full-run-safety
  • tests/test_atomic_version_stamp.py — full-run-safety
  • tests/test_atomic_writes.py — impact, full-run-safety
  • tests/test_backend_env_isolation.py — full-run-safety
  • tests/test_backend_extras.py — full-run-safety
  • tests/test_benchmark.py — full-run-safety
  • tests/test_benchmark_raw_graph.py — full-run-safety
  • tests/test_build.py — impact, full-run-safety
  • tests/test_build_merge_dedup_scope.py — full-run-safety
  • tests/test_build_merge_hyperedges_and_prune.py — full-run-safety
  • tests/test_build_merge_shrink_guard.py — full-run-safety
  • tests/test_builtin_global_type_refs.py — full-run-safety
  • tests/test_cache.py — full-run-safety
  • tests/test_callflow_html.py — full-run-safety
  • tests/test_cargo_introspect.py — full-run-safety
  • tests/test_carried_hyperedge_remap.py — impact, full-run-safety
  • tests/test_case_sensitive_resolution.py — full-run-safety
  • tests/test_charmap_encoding.py — full-run-safety
  • tests/test_chunking.py — full-run-safety
  • tests/test_cjs_module_extension.py — full-run-safety
  • tests/test_claude_cli_backend.py — full-run-safety
  • tests/test_claude_md.py — full-run-safety
  • tests/test_cli_broken_pipe.py — full-run-safety
  • tests/test_cli_export.py — impact, full-run-safety
  • tests/test_cli_help.py — full-run-safety
  • tests/test_cluster.py — full-run-safety
  • tests/test_codebuddy.py — impact, full-run-safety
  • tests/test_community_hub_labels.py — full-run-safety
  • tests/test_community_labels_skill.py — impact, full-run-safety
  • tests/test_confidence.py — impact, full-run-safety
  • tests/test_corrupt_graph_json.py — full-run-safety
  • tests/test_cpp_nested_and_cli.py — full-run-safety
  • tests/test_cpp_objc_cross_file_calls.py — full-run-safety
  • tests/test_cpp_preprocess.py — full-run-safety
  • tests/test_cross_extension_reexport_self_cycle.py — impact, full-run-safety
  • tests/test_cross_language_call_resolution.py — full-run-safety
  • tests/test_cross_repo_external_call_guards.py — full-run-safety
  • tests/test_cross_repo_member_calls.py — full-run-safety
  • tests/test_cross_repo_shared_types.py — full-run-safety
  • tests/test_csharp_call_site_generic_args.py — full-run-safety
  • … and 236 more

non-code file(s) changed (CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)

changed code file(s) with no mapped test (CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests

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 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: no capturable inputs from the test suite; property tier: 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)

No difference found (not proven): No behavior difference found in \_run\_cli (not a proof).

The verifier ran both versions of \_run\_cli 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.

· 1 grounded finding(s) anchored inline below; 26 more finding(s) on lines outside this diff (see the check run).

Comment thread graphify/analyze.py
@@ -107,7 +107,8 @@ def _is_json_key_node(G: nx.Graph, node_id: str) -> bool:


def god_nodes(G: nx.Graph, top_n: int = 10,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regressiongod_nodes()

37 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

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.

god-nodes: total degree hides the difference between a critical dependency and a config tree

1 participant