fix(extract): qualify nested class node IDs with parent class scope

[abhi-byte62]

Fixes #4220

Summary

When extracting nested classes with the same name across different parent classes (e.g. Forward.Params and Backward.Params), node IDs previously collided because _make_id(stem, ".".join(namespace_stack), class_name) did not incorporate the enclosing class scope. This caused the second class definition to be discarded by seen_ids and produced multiple contains edges pointing to a single node.

Changes

• In graphify/extractors/engine.py, prefix class_nid with parent_class_nid when present (except for Ruby which resolves full namespace qualification).
• Added regression tests in tests/test_python_nested_classes.py covering same-named nested classes under different parents, multi-level nesting, and recursive nesting.

[github-actions]

Thanks for the pull request, @abhi-byte62. A maintainer will review it soon.

Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions.

A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic.

[graphify-labs]

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.

Not checked: tests were not run; formal verification proved 0 of 1 changed function(s) (1 not verified).

Formal verification. PR-changed functions: 0/1 verified (0 proven, 0 may-equivalent, 0 distinguished) · 1 not verified (1 unsupported).

Not verified on this run: \_extract\_generic (unsupported).

---

Graphify review — findings

Extends parent-qualified node IDs for nested classes from Python to every language except Ruby, so same-named inner classes under different parents (e.g. Forward.Params and Backward.Params) mint distinct IDs. The qualified ID now uses class_name_for_id, so C# generic arity carries into nested IDs. Ruby keeps the stem-and-namespace ID so reopened classes still merge into one node; new tests cover sibling, deep, and same-name nesting in Python.

Worth a look

• Parent-qualified nested-class IDs now apply to every non-Ruby language, not just Python — graphify/extractors/engine.py:4957 · 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 — 835 functions depend on the 293 functions this change touches.

Health — grade A; 10 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):

• _extract_generic() — 19 callers, 35 callees (high)
• extract_js() — 87 callers, 5 callees (high)
• extract_xaml() — 19 callers, 17 callees (high)
• extract_objc() — 25 callers, 10 callees (high)
• extract_php() — 37 callers, 5 callees (high)
• extract_cpp() — 36 callers, 5 callees (high)
• extract_julia() — 17 callers, 8 callees (high)
• extract_svelte() — 14 callers, 8 callees (high)
• …and 2 more

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

Test selection

Test selection

32 of 362 test file(s) selected (9%) via static blast radius.

• tests/test_astro_extraction.py — impact
• tests/test_build.py — impact
• tests/test_cjs_module_extension.py — impact
• tests/test_cpp_nested_and_cli.py — impact
• tests/test_dotnet.py — impact
• tests/test_extract.py — impact
• tests/test_extract_php_closures.py — impact
• tests/test_import_extension_resolution.py — impact
• tests/test_indirect_call_block_scoped_shadow.py — impact
• tests/test_indirect_dispatch.py — impact
• tests/test_indirect_dispatch_assign_return.py — impact
• tests/test_indirect_dispatch_getattr.py — impact
• tests/test_js_exported_scalar_bindings.py — impact
• tests/test_languages.py — impact
• tests/test_multilang.py — impact
• tests/test_php_unparsed_casts.py — impact
• tests/test_python_inherited_method_calls.py — impact
• tests/test_python_nested_classes.py — impact, changed-test
• tests/test_python_underscore_resolution.py — impact
• tests/test_rationale.py — impact
• tests/test_ruby_resolution.py — impact
• tests/test_scala_context_bounds.py — impact
• tests/test_scala_self_type.py — impact
• tests/test_scala_top_level_binding.py — impact
• tests/test_scala_type_definition.py — impact
• tests/test_svelte_extraction.py — impact
• tests/test_swift_computed_properties.py — impact
• tests/test_swift_protocol_requirements.py — impact
• tests/test_trailing_newline_not_a_syntax_error.py — impact
• tests/test_ts_new_expression_calls.py — impact
• tests/test_typescript_module_extensions.py — impact
• tests/test_vue_extraction.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.

Formal verification

Could not verify: Could not verify \_extract\_generic.

The verifier did not have enough to check \_extract\_generic, 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: parameter `config` is annotated `LanguageConfig` — outside the synthesizable primitive/collection set

[abhi-byte62]

Applying parent-qualified IDs across generic AST languages ensures nested classes don't collide within the same module scope across Java, C++, TypeScript, etc., while Ruby keeps its reopened class behavior.

[safishamsi]

Thanks @abhi-byte62! Two things before this can land:

1. The Python case in [Bug]: Python AST extraction merges same-named nested classes under different parents #4220 is already fixed on v8 (by fix(python): preserve nested class identities by enclosing class #4228), so the three tests here pass with or without this change — they don't exercise what the PR actually alters.
2. The real value is generalizing the parent-scope qualification to C#/Java/Scala/etc., where same-named nested types genuinely still collide. But changing the declaration id shape there also needs the id-reconstruction sites updated to match (e.g. the C# base_list resolver), otherwise inherits/type edges will dangle onto bare-name stubs.

Could you add regression tests for at least C# (same-named nested types + inheriting from a nested type) and one JVM language, and audit/update those resolver sites (or narrow the scope)? Happy to point at the exact spots if useful.

[abhi-byte62]

Thanks for the thorough review and heads up @safishamsi! That makes complete sense regarding the declaration ID shape and type resolution.

Would really appreciate if you could point at the exact resolver spots for C# and the JVM extractors—I'd be happy to audit those sites, update the resolution logic, and add the regression tests for nested types and inheritance.

[safishamsi]

Thanks @abhi-byte62. Python nested-class ids are already qualified by their enclosing class on the current branch (#4220 is fixed there, and your Python tests pass against it without this change). The net new effect here is broadening that to C#/Swift/TS/Java, which collides with the C# id-collision and partial-merge machinery that just landed (#4411/#4415/#4364): it would bypass the underscore-salting and the partial-class merge keys. Closing as superseded for Python and out of scope for the others; a C#-specific change would need to build on the new salting/merge path.

[abhi-byte62]

Thanks for the detailed breakdown @safishamsi! That makes total sense—I hadn't caught the partial-class merge and salting changes that just landed in #4411/#4415. Agree 100% that bypassing that machinery would cause regressions there. Appreciate the review and the pointers!