Skip to content

fix(csharp): walk primary-constructor parameters for references and calls - #2836

Closed
brobl2008 wants to merge 1 commit into
Graphify-Labs:v8from
brobl2008:fix/csharp-primary-constructor-params
Closed

fix(csharp): walk primary-constructor parameters for references and calls#2836
brobl2008 wants to merge 1 commit into
Graphify-Labs:v8from
brobl2008:fix/csharp-primary-constructor-params

Conversation

@brobl2008

Copy link
Copy Markdown
Contributor

Fixes #2829.

Problem

A primary constructor (class Foo(IBar bar), C# 12+) declares its dependencies on the type
declaration itself rather than in a field or a property, so neither the field_declaration nor
the property_declaration handler ever saw them. Two edges were lost as a result:

  • the parameter's type got no references edge, and
  • because the name was never registered in csharp_field_types,
    _csharp_method_receiver_types could not type the receiver, so calls through it
    (bar.Baz()) lost their calls edge too.

The practical effect is that a class disappears from affected on the interface it depends on —
not a degraded answer but a confident, empty one.

This is the C# twin of #2063 (Kotlin primary_constructor / class_parameters). Scala already
had the equivalent handling in the same file; C# never got it.

Distinct from #2624 / #2676, which cover explicit type arguments at a call site. The repro
here contains no generics, and #2676 notes it "does not involve primary constructor parameter
resolution." The severity also differs: a #2624 case keeps its references[field] edge and stays
discoverable, whereas this one loses every edge.

Fix

A class_declaration / record_declaration / struct_declaration branch that walks the
parameter_list, registers name -> type for the receiver tables, and emits the references
edge — placed directly after the Scala class_parameters branch it mirrors.

Grammar note worth flagging for review: the parameter list is an unnamed child of the
declaration, so child_by_field_name("parameters") returns None and the children have to be
scanned. The Pascal-case and type-parameter guards match the existing field_declaration rule,
so class G<T>(IDep dep, T item) yields a ref to IDep and no phantom T.

Tests

Four cases added to tests/test_csharp_member_calls.py, each verified to fail on unpatched
HEAD and pass with the patch
:

Test Covers
test_primary_constructor_parameter_emits_references_edge the missing references edge
test_primary_constructor_parameter_resolves_member_calls the missing calls edge
test_record_positional_parameter_emits_references_edge positional record parameters
test_primary_constructor_type_parameter_is_not_referenced T item yields no phantom node

The three C# suites pass 83/83.

Full suite:

baseline (0738af3):  23 failed, 4248 passed, 159 skipped
this branch:         24 failed, 4251 passed, 159 skipped

The one differing entry is tests/test_labeling.py::test_label_communities_batches_when_over_batch_size,
which is flaky on pristine HEAD — run in isolation on unmodified 0738af3 it failed 4 of 5
times, versus 1 of 5 with this branch applied. It is unrelated to C# extraction and this change
does not touch it. Every other failure is pre-existing and identical across both runs
(test_terraform.py, test_skillgen.py, test_ollama_retry_cap.py, test_install_references.py).

Effect on a real graph

Re-extracting a 103-file C# project that uses primary-constructor DI: 3,129 → 3,174 edges
(+30 references, +15 calls). affected on one interface went from 1 node to 16, now
correctly including the service class that injects it and each of its delegating methods.

…alls

A primary constructor (`class Foo(IBar bar)`, C# 12+) declares its
dependencies on the type declaration itself rather than in a field or a
property, so neither the field_declaration nor the property_declaration
handler ever saw them. The parameter type got no references edge, and
because the name was never registered in csharp_field_types,
_csharp_method_receiver_types could not type the receiver, so calls made
through it (`bar.Baz()`) lost their calls edge as well.

Add a class_declaration / record_declaration / struct_declaration branch
that walks the parameter_list, registers name -> type for the receiver
tables, and emits the references edge - mirroring the Scala
class_parameters branch directly above it; Kotlin's equivalent is Graphify-Labs#2063.
Grammar note: the list is an unnamed child of the declaration, so
child_by_field_name("parameters") returns None and the children must be
scanned.

Pascal-case-only and type-parameter guards match the existing
field_declaration rule, so `T item` yields no phantom node.

Fixes Graphify-Labs#2829

Co-Authored-By: Claude Opus 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.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.

Formal verification. 3 change(s) tested, no difference found (not proven).


Graphify review — findings

Reverts the 0.9.46 changelog entries and drops Common Lisp support — removes the [commonlisp] extra from README, deletes the specific-beats-generic edge-relation guard and _GENERIC_RELATIONS in build.py, drops hyperedge member rewiring on dedup, and removes the orphaned external-import stub sweep during source pruning. Reverts corresponding test cases across test_languages, test_serve, test_detect, test_csharp_member_calls, and test_hooks, and rolls the version back to 0.9.45 (unreleased).

Worth a look

  • Generic relation can overwrite a specific edge relationgraphify/build.py:1230 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
  • Removed public hyperedges keyword from deduplicate_entitiesgraphify/dedup.py:460 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
  • Common Lisp extensions no longer dispatch to an extractorgraphify/extract.py:4924 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
  • Common Lisp extractor removed from public extractor registrygraphify/extractors/__init__.py:37 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
  • normalize_id loses caseless-stable/idempotence guarantee for combining-mark sequencesgraphify/ids.py:48 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 4588 functions depend on the 2374 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 464 callers, 41 callees
  • new: _rebuild_code() — 98 callers, 51 callees
  • new: build_from_json() — 153 callers, 18 callees
  • new: detect() — 99 callers, 15 callees
  • new: deduplicate_entities() — 63 callers, 20 callees
  • new: build_merge() — 43 callers, 14 callees
  • new: save_semantic_cache() — 51 callers, 9 callees
  • new: _extract_generic() — 18 callers, 24 callees
  • …and 141 more — each is listed as a finding

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

Formal verification

Could not verify: Could not verify build.

The verifier did not have enough to check build, 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: not verifiable: all 9 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify build\_from\_json.

The verifier did not have enough to check build\_from\_json, 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: not verifiable: all 6 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify build\_merge.

The verifier did not have enough to check build\_merge, 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: parameter `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set

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: 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)

Could not verify: Could not verify \_load\_dir\_own\_ignore.

The verifier did not have enough to check \_load\_dir\_own\_ignore, 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: parameter `d` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_load\_graphifyignore.

The verifier did not have enough to check \_load\_graphifyignore, 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: parameter `root` is annotated `Path` — 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_java\_annotation\_names.

The verifier did not have enough to check \_java\_annotation\_names, 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: not verifiable: all 182 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)

Could not verify: Could not verify \_java\_collect\_type\_refs.

The verifier did not have enough to check \_java\_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: parameter `skip` is annotated `frozenset[str] | None` — outside the synthesizable primitive/collection set

Could not verify: Could not verify extract\_markdown.

The verifier did not have enough to check extract\_markdown, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_resolve\_go\_type\_references.

The verifier did not have enough to check \_resolve\_go\_type\_references, 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: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set

Could not verify: Could not verify \_resolve\_java\_type\_references.

The verifier did not have enough to check \_resolve\_java\_type\_references, 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: non-vacuity: domain too small (only 1 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous

No difference found (not proven): No behavior difference found in normalize\_id (not a proof).

The verifier ran both versions of normalize\_id 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 \_search\_tokens (not a proof).

The verifier ran both versions of \_search\_tokens 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 \_subgraph\_to\_text (not a proof).

The verifier ran both versions of \_subgraph\_to\_text 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.

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

@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.47 via authorship-preserving cherry-pick, with added builtin-not-fabricated and struct primary-ctor tests. Thanks @brobl2008! Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.47

@safishamsi safishamsi closed this Aug 19, 2026
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.

C#: primary-constructor parameters are never walked, so constructor-injected dependencies produce no edges

2 participants