Conversation
…aphify-Labs#3796) A named tuple type `(int mode, string label)` is a `tuple_type` whose `tuple_element` children carry both a `type` and a `name` field. _csharp_collect_type_refs walked every named child, so the element NAMES were collected as type references, minted as sourceless placeholder nodes and emitted `references` edges — one junk type node per element, repeated in every file declaring such a signature. Only the `type` field of each tuple element should feed type-reference collection; the `name` is an identifier, not a type reference. Fix the collector to descend into the element's type field only. Regression: tests/test_csharp_tuple_type_refs.py asserts no node/edge is minted for element names (all existing C# tests pass).
|
Thanks for the pull request, @KaiyiQuan. 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. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The regression test does not verify that valid tuple element type references are preserved.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes C# named tuple element names being emitted as type references.
Changes:
- Collects only each tuple element’s
typefield. - Adds regression coverage for excluding element names.
Review note: The test should use user-defined element types and verify their references remain collected.
| File | Summary |
|---|---|
tests/test_csharp_tuple_type_refs.py |
Adds named-tuple regression coverage. |
graphify/extractors/engine.py |
Excludes tuple element names from type-reference traversal. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def test_tuple_element_names_are_not_type_references(): | ||
| r = _extract_source(READER_CS) | ||
|
|
||
| # Element NAMES must not be minted as type nodes. | ||
| labels = {n["label"] for n in r["nodes"]} | ||
| assert "mode" not in labels | ||
| assert "label" not in labels | ||
|
|
||
| # And no references edge may target a placeholder minted from a name. | ||
| for e in r["edges"]: | ||
| if e["relation"] != "references": | ||
| continue | ||
| target = next(n for n in r["nodes"] if n["id"] == e["target"]) | ||
| assert target["label"] not in ("mode", "label"), ( | ||
| f"references edge to tuple element name {target['label']}" | ||
| ) |
There was a problem hiding this comment.
Fixed in 6506c32. The regression test now uses user-defined element types (Mode mode, Label label) and asserts both halves: element names mode/label are still excluded, while Mode/Label nodes and their references edges from the declaring member are preserved. All C# tests pass (108 total).
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
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. 11 change(s) tested, no difference found (not proven).
Graphify review — findings
Handles C# named tuple_type nodes in _csharp_collect_type_refs by descending only into each tuple_element's type field, so element names like mode and label no longer get minted as junk type nodes with spurious references edges (#3796).
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 698 functions depend on the 236 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_extract_generic()— 18 callers, 29 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_julia()— 17 callers, 7 callees - new:
extract_cpp()— 29 callers, 3 callees - new:
extract_vue()— 10 callers, 7 callees - new:
walk()— 1 callers, 63 callees - …and 9 more — each is listed as a finding
Verification — 698 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: 635 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
26 of 302 test file(s) selected (9%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_build.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_csharp_tuple_type_refs.py— impact, changed-testtests/test_dotnet.py— impacttests/test_extract.py— impacttests/test_extract_php_closures.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_languages.py— impacttests/test_multilang.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_swift_computed_properties.py— impacttests/test_swift_protocol_requirements.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_typescript_module_extensions.py— impacttests/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
No difference found (not proven): No behavior difference found in \_read\_tsconfig\_aliases (not a proof).
The verifier ran both versions of \_read\_tsconfig\_aliases 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.
No difference found (not proven): No behavior difference found in \_load\_tsconfig\_aliases (not a proof).
The verifier ran both versions of \_load\_tsconfig\_aliases 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 extract.
The verifier did not have enough to check extract, 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 90 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify \_csharp\_collect\_type\_refs.
The verifier did not have enough to check \_csharp\_collect\_type\_refs, 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 `skip` is annotated `frozenset[str] | None` — outside the synthesizable primitive/collection set
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
No difference found (not proven): No behavior difference found in \_to\_absolute\_from\_storage (not a proof).
The verifier ran both versions of \_to\_absolute\_from\_storage 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.
No difference found (not proven): No behavior difference found in load\_manifest (not a proof).
The verifier ran both versions of load\_manifest 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 deduplicate\_entities.
The verifier did not have enough to check deduplicate\_entities, 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 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in save\_manifest (not a proof).
The verifier ran both versions of save\_manifest 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.
No difference found (not proven): No behavior difference found in detect\_incremental (not a proof).
The verifier ran both versions of detect\_incremental 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.
No difference found (not proven): No behavior difference found in \_detect\_url\_type (not a proof).
The verifier ran both versions of \_detect\_url\_type 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 IndexError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in extract\_julia (not a proof).
The verifier ran both versions of extract\_julia 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.
No difference found (not proven): No behavior difference found in extract\_fortran (not a proof).
The verifier ran both versions of extract\_fortran 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.
No difference found (not proven): No behavior difference found in extract\_ocaml (not a proof).
The verifier ran both versions of extract\_ocaml 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.
No difference found (not proven): No behavior difference found in extract\_elixir (not a proof).
The verifier ran both versions of extract\_elixir 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.
· 17 more finding(s) on lines outside this diff (see the check run).
…Graphify-Labs#3796) Per Copilot review: the regression test must prove both halves of the fix — element names are excluded, AND the real element type references are still collected. Add a test using user-defined element types `(Mode mode, Label label)`: asserts Mode/Label nodes and their references edges survive while the names mode/label are not minted.
|
Test coverage strengthened per Copilot review (6506c32): user-defined tuple element types now verified to remain referenced, element names verified excluded. C# suite 108/108 passing. |
There was a problem hiding this comment.
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
Handles C# tuple_type nodes in _csharp_collect_type_refs by recursing only into each tuple_element's type field, so named-tuple element names like mode/label no longer get minted as junk type nodes or emit spurious references edges while the real element types are still collected.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 699 functions depend on the 237 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_extract_generic()— 18 callers, 29 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_julia()— 17 callers, 7 callees - new:
extract_cpp()— 29 callers, 3 callees - new:
extract_vue()— 10 callers, 7 callees - new:
walk()— 1 callers, 63 callees - …and 9 more — each is listed as a finding
Verification — 699 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: 636 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
26 of 302 test file(s) selected (9%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_build.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_csharp_tuple_type_refs.py— impact, changed-testtests/test_dotnet.py— impacttests/test_extract.py— impacttests/test_extract_php_closures.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_languages.py— impacttests/test_multilang.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_self_type.py— impacttests/test_swift_computed_properties.py— impacttests/test_swift_protocol_requirements.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_typescript_module_extensions.py— impacttests/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.
· 17 more finding(s) on lines outside this diff (see the check run).
Security: close the Fortran cpp #include arbitrary-file-read (GHSA-pcc4-rvhr-2pr8), the last Aider/Devin monolith --watch shell sink (#3852), and terraform name/value secret redaction (#3870). Plus Windows watch rebuild locking (#3883), C# tuple element-name refs (#3877), JSX component-usage calls (#3855), and nested scan-root Python import projection (#3867). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Shipped in v0.9.70 (now on PyPI) via an authorship-preserving cherry-pick, so your commit keeps contributor-graph credit. Thanks @KaiyiQuan! Named-tuple element names are no longer collected as C# type references; complements #3815. Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.70 |
Fixes #3796
Problem
A named tuple type
(int mode, string label)parses as atuple_typewhosetuple_elementchildren carry both atypeand anamefield._csharp_collect_type_refswalked every named child, so the element names were collected as type references, minted as sourceless placeholder nodes and emittedreferencesedges — one junk "type" node per element, repeated in every file that declares such a signature (the issue reports five junk nodes from a single 5-element tuple return).Fix
Only the
typefield of eachtuple_elementshould feed type-reference collection; thenameis an identifier, not a type reference. The collector now descends into the element'stypefield only, skipping the name.Tests
tests/test_csharp_tuple_type_refs.py— asserts no node is minted and noreferencesedge targets an element name.