Skip to content

fix(go): retain interface method signature references - #3737

Closed
oleksii-tumanov wants to merge 2 commits into
Graphify-Labs:v8from
oleksii-tumanov:codex/go-interface-signature-references
Closed

oleksii-tumanov wants to merge 2 commits into
Graphify-Labs:v8from
oleksii-tumanov:codex/go-interface-signature-references

Conversation

@oleksii-tumanov

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #3736.

Use the existing Go signature-reference helper for interface methods, so their
parameter and result types get the same links as concrete methods.

Include named interface methods in the existing case-collision scan. This keeps
signatures such as Run(InputA) and run(InputB) attached to distinct methods,
using the current ID policy rather than introducing a new one.

No changes to embedding, type constraints, call resolution, top-level type
identity, or the helper's existing variadic-parameter behavior.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

How was this tested?

pytest tests/test_go_interface_methods.py tests/test_go_qualified_resolution.py tests/test_go_builtin_call_targets.py tests/test_multilang.py tests/test_languages.py -q
518 passed

pytest tests/ -q --tb=short
5862 passed, 14 skipped

Tests cover named/unnamed parameters, pointer and grouped results, builtin
filtering, concrete-method parity, source ownership, and cold/warm extraction.
Case-only method tests also check declaration-order independence, owner isolation,
and stable exported IDs when a private sibling is removed.
Ruff, all five skillgen checks, and graphify update . --no-cluster passed.

Graphify-specific checklist

  • I added or updated tests for behavior changes.
  • I updated documentation or confirmed that no documentation is needed.
  • I updated generated skill artifacts when changing their source fragments.
  • I considered compatibility across supported Python versions.
  • I confirmed that no API keys, generated graph data, or local-only files are included.

@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

Emits references edges from Go interface method nodes to their parameter and return types by calling emit_go_method_refs on each method_elem, so interface signatures now carry the same type references as their concrete implementations. Registers case-only method-name collisions in case_groups during _scan_declarations so distinct interface methods differing only by case keep separate signature owners with stable IDs regardless of declaration order. Adds tests covering signature type references on both interfaces and structs, warm-extraction stability, case-collision owner separation, and preservation of exported method IDs when a private sibling is removed.

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

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 48 functions depend on the 31 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract_go() — 19 callers, 7 callees
  • new: walk() — 1 callers, 8 callees

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

Test selection

Test selection

3 of 290 test file(s) selected (1%) via static blast radius.

  • tests/test_go_interface_methods.py — impact, changed-test
  • tests/test_languages.py — impact
  • tests/test_multilang.py — impact

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.

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

@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 @oleksii-tumanov!

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.

Go interface methods omit parameter and result type references

2 participants