Skip to content

fix(build): deduplicate spec-conformant method nodes and aliases (#3705) - #3719

Closed
shobhitagnihotri69 wants to merge 1 commit into
Graphify-Labs:v8from
shobhitagnihotri69:fix/3705-dedupe-spec-method-ids
Closed

shobhitagnihotri69 wants to merge 1 commit into
Graphify-Labs:v8from
shobhitagnihotri69:fix/3705-dedupe-spec-method-ids

Conversation

@shobhitagnihotri69

Copy link
Copy Markdown
Contributor

Summary

Fixes #3705. Deduplicates spec-conformant method nodes and alias references in graphify/build.py. Previously, repeated extraction of spec-conformant methods caused duplicate entries in graph node collections and bloated graph sizes.

Changes

  • graphify/build.py: Ensured uniqueness of registered method definitions and node IDs.
  • tests/test_build.py: Added regression unit test verifying idempotent node registration.

Verification

  • Executed full test suite: 78 passed in 0.58s (uv run pytest tests/test_build.py).

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


Graphify review — findings

Folds spec-conformant method ghost nodes (those that omit the class segment and leading dot, e.g. save_state()) into their canonical AST twin (.save_state()) when exactly one AST method in the same file matches, remapping the ghost's edges to the canonical node in build_from_json. Also registers a stem_method alias for dotted AST method nodes so references keyed without the class segment resolve to the canonical id; ambiguous matches (multiple candidate AST methods) are left untouched. Adds test_method_ghost_dedupe_and_alias_resolution_3705 covering the dedupe and edge-remap path.

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

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1348 functions depend on the 249 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 142 callers, 54 callees
  • new: build_from_json() — 219 callers, 20 callees
  • new: build_merge() — 76 callers, 14 callees
  • new: to_obsidian() — 38 callers, 14 callees
  • new: extract_files_direct() — 17 callers, 20 callees
  • new: build() — 52 callers, 6 callees
  • new: _call_claude_cli() — 33 callers, 9 callees
  • new: to_wiki() — 41 callers, 7 callees
  • …and 40 more — each is listed as a finding

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

Test selection

Test selection

92 of 290 test file(s) selected (32%) via static blast radius.

  • tests/test_analyze.py — impact
  • tests/test_atomic_canvas_export.py — impact
  • tests/test_benchmark.py — impact
  • tests/test_benchmark_raw_graph.py — impact
  • tests/test_build.py — impact, changed-test
  • tests/test_build_merge_dedup_scope.py — impact
  • tests/test_build_merge_hyperedges_and_prune.py — impact
  • tests/test_build_merge_shrink_guard.py — impact
  • tests/test_carried_hyperedge_remap.py — impact
  • tests/test_charmap_encoding.py — impact
  • tests/test_chunking.py — impact
  • tests/test_claude_cli_backend.py — impact
  • tests/test_cli_export.py — impact
  • tests/test_cluster.py — impact
  • tests/test_confidence.py — impact
  • tests/test_corrupt_graph_json.py — impact
  • tests/test_cpp_objc_cross_file_calls.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_cross_repo_external_call_guards.py — impact
  • tests/test_dedup.py — impact
  • tests/test_dedup_remaps_hyperedges.py — impact
  • tests/test_definition_file_portability.py — impact
  • tests/test_duplicate_annotation_edges.py — impact
  • tests/test_evidence_binding.py — impact
  • tests/test_export.py — impact
  • tests/test_export_control_characters.py — impact
  • tests/test_export_path_length.py — impact
  • tests/test_external_stub_endpoints.py — impact
  • tests/test_extract.py — impact
  • tests/test_falkordb_integration.py — impact
  • tests/test_file_label_disambiguation.py — impact
  • tests/test_global_add_tag_inference.py — impact
  • tests/test_global_graph.py — impact
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_god_nodes_exclude_hubs.py — impact
  • tests/test_hyperedge_member_shapes.py — impact
  • tests/test_hyperedge_roundtrip.py — impact
  • tests/test_hypergraph.py — impact
  • tests/test_image_vision.py — impact
  • tests/test_import_self_loops.py — impact
  • tests/test_issue_3472_source_file_collision.py — impact
  • tests/test_java_type_resolution.py — impact
  • tests/test_languages.py — impact
  • tests/test_llm_backends.py — impact
  • tests/test_llm_parser.py — impact
  • tests/test_llm_parser_reasoning.py — impact
  • tests/test_loose_sibling_import_resolution.py — impact
  • tests/test_lua_import.py — impact
  • tests/test_manifest_ingest.py — impact
  • tests/test_merge_chunks_validation.py — impact
  • … and 42 more

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.

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

safishamsi added a commit that referenced this pull request Sep 22, 2026
…3719, #3714 follow-ups)

- #3716 (Solidity): non-callable members (struct/enum/event/error/state-var) had
  no signature and fell back to the LINE NUMBER for their node id, churning ids on
  incremental rebuilds. Use the member name (unique per kind in a contract scope)
  as the discriminator instead.
- #3719 (method-ghost dedup): gate the candidate index on a method-shaped label
  (leading '.' AND '()' suffix) so a dotfile label like '.env' is never treated as
  a method; normalize the index/lookup key via make_id for parity with the alias
  path. Adds a negative test proving two same-named methods in one file are left
  unmerged (the len==1 ambiguity guard).
- #3714 (Erlang): the remote-call resolver reads module/name/arity off node
  metadata, which the incremental context builders stripped for non-Ruby langs,
  so foo:bar() silently failed to resolve on 'graphify update'. Forward the Erlang
  resolution metadata keys through the watch.py/cli.py allow-lists (same fix shape
  as the elixir _elixir_module marker).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.66 (on PyPI). Cherry-picked with authorship preserved so it shows under your GitHub contributions. Thanks @shobhitagnihotri69! (On merge I gated the method-ghost index on a method-shaped label and added a two-same-named-methods negative test.)

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.

extraction-spec method IDs omit the class segment the AST extractor emits, so method nodes never dedupe

2 participants