Skip to content

feat: return type inference, doc-comment parsing, and per-language type extractors#284

Merged
magyargergo merged 20 commits into
mainfrom
feat/type-resolution-phase3
Mar 15, 2026
Merged

feat: return type inference, doc-comment parsing, and per-language type extractors#284
magyargergo merged 20 commits into
mainfrom
feat/type-resolution-phase3

Conversation

@magyargergo

@magyargergo magyargergo commented Mar 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds return-type-based call resolution, doc-comment type extraction (JSDoc, PHPDoc, YARD), and per-language type extractors across all 12 supported languages. Includes a major refactor consolidating constructor binding scanners into the existing LanguageTypeConfig pattern, plus targeted fixes from 7 automated code review passes.

Return Type Inference

  • Added returnType to SymbolDefinition and wired it through parsing-processorsymbol-tablecall-processor
  • When var = callee() and callee has a known return type, the variable is bound to that type
  • Wrapper unwrapping: Promise<User>User, User | nullUser, *UserUser, models.UserUser
  • Only triggers for unambiguous callees (1 candidate) — conservative by design

Doc-Comment Type Extraction

  • PHP: PHPDoc @return ClassName and @param Type $name parsing, handles ?Type, Type|null, namespaced types. Now also supports alternate format @param $name Type
  • JS/TS: JSDoc @returns {Type} and @param {Type} name parsing, with sanitizeReturnType preserving generics for downstream unwrapping
  • Ruby: YARD @return [Type] and @param name [Type] parsing, qualified type handling (Models::UserUser). Now also supports alternate format @param [Type] name and bracket-balanced generic splitting (Hash<Symbol, User> no longer breaks on inner commas)

Per-Language Constructor Binding Scanners

  • Added scanConstructorBinding to all 12 languages (TS/JS, Java, Kotlin, C#, Go, Rust, PHP, C++, Swift, Python, Ruby)
  • Each scanner captures var = callee() patterns where no explicit type annotation exists
  • Consolidated from a separate dispatch map into LanguageTypeConfigtype-env.ts shrank by 305 lines

Generic Type Argument Extraction

  • New extractGenericTypeArgs() utility: List<User>['User'], Map<String, User>['String', 'User']
  • Handles TS/Java/Kotlin/C#/Rust generic syntax

Ruby Type Extractor

  • Full ruby.ts extractor replacing the previous empty stub
  • YARD annotations, qualified types, nullable handling, constructor inference (User.new)

C# Null-Conditional Call Support

  • Added tree-sitter query pattern for user?.Save() via conditional_access_expressionmember_binding_expression
  • Added member_binding_expression to MEMBER_ACCESS_NODE_TYPES and receiver extraction in extractReceiverName
  • Upgraded known-gap test to positive assertion with full integration test suite (5 tests)

Security Hardening

  • escapeValue in kuzu-adapter.ts now escapes \n and \r to prevent Cypher query structure breaks from untrusted type strings

Bug Fixes (7 review passes)

  • JS/TS: sanitizeReturnType preserves Promise<User> for async return type resolution
  • JS/TS: Decorator nodes skipped in collectJsDocParams backward sibling walk
  • PHP: #[Attribute] nodes between PHPDoc and method no longer break extraction
  • PHP: $this/self/static receiver disambiguation via receiverClassName
  • C#: Scanner fixes for method signature extraction and cross-file resolution
  • Go: selector_expression fix in return type extraction
  • Java: Fixed any type in scanner → SyntaxNode | null
  • Ruby: Bracket-balanced YARD type splitting (fixes Hash<Symbol, User> comma bug)
  • Ruby: YARD alternate @param [Type] name format support
  • Swift: navigation_suffix unwrapping in scanConstructorBinding
  • Rust: Lifetime parameter skipping in generic return type extraction (Ref<'_, User>User)
  • Nested generics: Bracket-balanced extractFirstGenericArg for Promise<Map<string, User>>

Deferred Items (Follow-Up PRs)

Collected from 7 code review passes and reconciled against branch state. Full plan: docs/plans/2026-03-15-feat-type-resolution-deferred-items-plan.md

Next PR: Correctness & Language Gaps

  • Overloaded method receiverKey collision — same-name overloads share composite key, needs scope-aware prefix-scan
  • Kotlin cross-package return-type inference — root cause: resolveJvmMemberImport() requires 3+ segments but import models.getUser has 2 (import resolution gap in jvm.ts:82)
  • Ruby chained constructors (User.new.tap {}, User.new.freeze) — outer call hides .new from scanner

Future PRs: Architecture Items (only with evidence of real missed edges)

  • For-loop variable typing (requires extractGenericTypeArgs + new LanguageTypeConfig hook)
  • Chained property access a.b.c() (requires TypeEnv redesign, depth limit 3)
  • Block-level type narrowing (requires scope model changes)
  • C++/Rust/Swift/C# conditional binding patterns (if let, guard let, is pattern matching)
  • Python class-level annotations (dataclass fields)

Infrastructure

  • extractGenericTypeArgs has 0 production callers (60+219 LOC) — keep for for-loop typing or delete
  • tree-sitter-swift Node 22 build fix (all 11 Swift tests skipped)
  • Extract collectPrecedingComments shared helper (dedup ~40 LOC across TS/PHP/Ruby)

Test Plan

  • 147 unit tests pass (type-env.test.ts)
  • 67 unit tests pass (call-processor.test.ts)
  • 40 unit tests pass (call-form.test.ts)
  • 30 unit tests pass (method-signature.test.ts)
  • 52 C# integration tests pass (5 new: null-conditional)
  • 70 Ruby integration tests pass (6 new: YARD generics + alternate format)
  • TypeScript compilation clean (tsc --noEmit)
  • 115 files changed, 4184 insertions, 512 deletions

…YARD type extractor

Three architectural improvements to the type resolution system:

1. Return type inference — wire extractMethodSignature returnType through
   SymbolDefinition into call-processor. When var = callee() and callee
   has a known return type, bind var to that type. Handles Promise<T>
   unwrapping, nullable stripping, pointer/reference removal.

2. Generic type argument extraction — new extractGenericTypeArgs() utility
   that extracts type parameters from List<User> → ['User']. Handles
   TS/Java/Kotlin/C#/Rust generic syntax. Building block for for-loop
   variable typing.

3. Ruby dedicated type extractor — replaces the stub with YARD annotation
   parsing (@param name [Type]), handling qualified types, nullable types,
   and singleton methods. Ruby now has real type resolution.

Unit tests: 127 → 192+ (type-env) + 65 (symbol-table, call-processor) + 18 (generics)
Integration tests: 8+ new test cases with fixtures across TS/Python/Go/Java/Ruby
@vercel

vercel Bot commented Mar 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gitnexus Ready Ready Preview, Comment Mar 15, 2026 6:50pm

Request Review

@github-actions

github-actions Bot commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Unit Tests success 3 platforms
✅ Integration success 3 OS x 4 groups = 12 jobs

Test Results

Suite Tests Passed Failed Skipped Duration
Unit 1350 1350 0 0 8s
Integration 825 808 0 17 52s
Total 2175 2158 0 17 60s

✅ All 2158 tests passed

17 test(s) skipped — expand for details

Integration:

  • Swift constructor-inferred type resolution > detects User and Repo classes, both with save methods
  • Swift constructor-inferred type resolution > resolves user.save() to Models/User.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > resolves repo.save() to Models/Repo.swift via constructor-inferred type
  • Swift constructor-inferred type resolution > emits exactly 2 save() CALLS edges (one per receiver type)
  • Swift self resolution > detects User and Repo classes, each with a save function
  • Swift self resolution > resolves self.save() inside User.process to User.save, not Repo.save
  • Swift parent resolution > detects BaseModel and User classes plus Serializable protocol
  • Swift parent resolution > emits EXTENDS edge: User → BaseModel
  • Swift parent resolution > emits IMPLEMENTS edge: User → Serializable (protocol conformance)
  • Swift cross-file User.init() inference > resolves user.save() via User.init(name:) inference
  • Swift cross-file User.init() inference > resolves user.greet() via User.init(name:) inference
  • Swift return type inference > detects User class and getUser function
  • Swift return type inference > detects save function on User (Swift class methods are Function nodes)
  • Swift return type inference > resolves user.save() to User#save via return type of getUser() -> User
  • Swift return-type inference via function return type > resolves user.save() to User#save via return type of getUser()
  • Swift return-type inference via function return type > user.save() does NOT resolve to Repo#save
  • Swift return-type inference via function return type > resolves repo.save() to Repo#save via return type of getRepo()

Code Coverage

Combined (Unit + Integration)

Metric Coverage Covered Base Delta Status
Statements 51.54% 4767/9249 40.51% 📈 +11.0 🟢 ██████████░░░░░░░░░░
Branches 44.71% 2975/6653 35.89% 📈 +8.8 🟢 ████████░░░░░░░░░░░░
Functions 53.35% 485/909 41.65% 📈 +11.7 🟢 ██████████░░░░░░░░░░
Lines 53.17% 4342/8166 41.86% 📈 +11.3 🟢 ██████████░░░░░░░░░░
Coverage breakdown by test suite

Unit Tests

Metric Coverage Covered Base Delta Status
Statements 41.96% 3881/9249 40.51% 📈 +1.5 🟢 ████████░░░░░░░░░░░░
Branches 37.39% 2488/6653 35.89% 📈 +1.5 🟢 ███████░░░░░░░░░░░░░
Functions 42.46% 386/909 41.65% 📈 +0.8 🟢 ████████░░░░░░░░░░░░
Lines 43.27% 3534/8166 41.86% 📈 +1.4 🟢 ████████░░░░░░░░░░░░

Integration Tests

Metric Coverage Covered Base Delta Status
Statements 22.71% 2101/9249 40.51% 📉 -17.8 🔴 ████░░░░░░░░░░░░░░░░
Branches 17.18% 1143/6653 35.89% 📉 -18.7 🔴 ███░░░░░░░░░░░░░░░░░
Functions 25.85% 235/909 41.65% 📉 -15.8 🔴 █████░░░░░░░░░░░░░░░
Lines 23.83% 1946/8166 41.86% 📉 -18.0 🔴 ████░░░░░░░░░░░░░░░░

📋 View full run · Generated by CI

@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 5m 52s —— View job


Code Review: Constructor Type Resolution Analysis

Tasks:

  • Read trigger comment and understand request
  • Read changed source files (call-processor.ts, ruby.ts, shared.ts, symbol-table.ts, type-env.ts, parse-worker.ts)
  • Analyze constructor type resolution logic for gaps/bugs
  • Review what this PR improved vs previous state
  • Identify next steps and remaining gaps

What This PR Improved

Before Phase 3: Type resolution depended entirely on explicit type annotations (e.g. user: User) found by AST node walking. Ruby had an empty stub with no YARD support. Variable bindings from function return values were never used.

After Phase 3:

  1. Return type inference: extractMethodSignature now captures returnType from AST nodes (for typed languages) and stores it in SymbolDefinition. When var = callee() is scanned and callee resolves to a single function with a known return type, the variable is bound to that type — enabling var.method() resolution without explicit annotations.
  2. Ruby YARD extractor: Real ruby.ts replaces the empty stub. Parses @param name [Type] from comment nodes before method definitions, handles qualified types (Models::User → User) and nullable (String, nil → String). Works in concert with the existing extractRubyConstructorBinding scanner in type-env.ts.
  3. Generic type args: extractGenericTypeArgs() now properly extracts type parameters for future for-loop variable typing (Task 3.3 prerequisite).
  4. Cross-file constructor constant binding: The new integration tests confirm SERVICE = UserService.new; SERVICE.process() resolves correctly via the scanner → SymbolTable verification → fileReceiverTypes pipeline.

Issues Found

🔴 Critical: TypeScript/JavaScript has no scanner — const user = getUser() return type inference cannot work end-to-end

Location: gitnexus/src/core/ingestion/type-env.ts:440CONSTRUCTOR_BINDING_SCANNERS

The return type inference path in processCallsFromExtracted only fires for entries in constructorBindings. These bindings are produced by the language-specific CONSTRUCTOR_BINDING_SCANNERS. TypeScript and JavaScript have no scanner, so const user = getUser() never produces a binding entry.

The ts-return-type-inference/ fixture (app.ts calls getUser() then user.save()) is never exercised in integration tests — searching all integration test files finds zero references to these fixtures. The unit tests for return type inference work because they manually supply constructorBindings.

// app.ts — this fixture doesn't work end-to-end:
const user = getUser('alice'); // ← no TS scanner to capture this binding
user.save();                   // ← never resolved via return type inference

The same applies to go-return-type-inference/, java-return-type-inference/ and any other language without a scanner entry. Python does work since its scanner captures all assignment nodes generically.

Fix this →

🟡 Bug: ruby.ts::extractInitializer — constant assignments silently fail, misleading comment

Location: gitnexus/src/core/ingestion/type-extractors/ruby.ts:183-185

// Support both local variables (identifier) and constants (SERVICE = UserService.new)
if (left.type !== 'identifier' && left.type !== 'constant') return;
const varName = extractVarName(left);
if (!varName || env.has(varName)) return;  // ← returns undefined for 'constant' nodes!

extractVarName in shared.ts handles identifier, simple_identifier, variable_name, name, variable_declarator, and mut_pattern — but NOT constant. So for SERVICE = UserService.new, extractVarName(left) returns undefined and the initializer exits without updating env. The comment claiming constant support is incorrect.

The net behavior is still correct only because extractRubyConstructorBinding in type-env.ts uses left.text directly (not extractVarName), so the scanner path handles it. But locally-defined classes like SERVICE = UserService.new where UserService is in the same file will unnecessarily go through the cross-file scanner verification path rather than being resolved locally.

Fix this →

🟡 Gap: Ruby Models::User.new — namespaced constructor receiver ignored entirely

Location: gitnexus/src/core/ingestion/type-env.ts:432 and gitnexus/src/core/ingestion/type-extractors/ruby.ts:192

Both extractRubyConstructorBinding and extractInitializer guard:

if (!receiver || receiver.type !== 'constant') return;

In tree-sitter-ruby, Models::User.new parses the receiver as a scope_resolution node (Models::User), not a constant. So qualified namespace constructors are silently dropped — no binding is created and no warning is emitted. This is a silent resolution failure for any Ruby codebase using namespaced classes.

🟡 Gap: extractReturnTypeName doesn't handle Ruby :: qualified type names

Location: gitnexus/src/core/ingestion/call-processor.ts:411-415

// Handle qualified names: models.User → User, com.example.User → User
if (text.includes('.')) {
  text = text.split('.').pop()!;
}
// ...
if (!/^[A-Z_]\w*$/.test(text)) return undefined;  // ← 'Models::User' fails this regex

Ruby uses :: for namespacing, not .. A YARD annotation like @return [Models::User] (if it were captured — see next point) would fail the ^[A-Z_]\w*$ regex because :: is not a word character, causing the function to return undefined. The qualifier stripping logic only handles dot-notation languages.

🟡 Gap: Ruby @return YARD annotation not extracted — return type inference impossible for Ruby methods

Location: gitnexus/src/core/ingestion/type-extractors/ruby.ts and gitnexus/src/core/ingestion/utils.ts:617

extractMethodSignature looks for return_type and type_annotation AST nodes — neither exists in Ruby's dynamically-typed AST. The ruby.ts extractor only parses @param YARD annotations, not @return. So even with a fixture like:

# @return [User]
def get_user(name)
  User.new(name)
end

...the returnType field of that method's SymbolDefinition will be undefined. Return type inference (user = get_user(); user.save()) will never fire for Ruby functions.

🟡 Gap: Ruby scanner only captures .new calls — user = get_user() won't produce a binding

Location: gitnexus/src/core/ingestion/type-env.ts:424

extractRubyConstructorBinding checks method.text !== 'new' implicitly — it only matches the pattern where right has a method field equal to 'new'. Plain function calls like user = get_user() produce no binding. Python's scanner captures ALL call assignments by contrast.

This is the Ruby-specific gap that blocks the equivalent of the python-return-type-inference pattern from working in Ruby.

🟢 Minor: Four fixture directories have no integration tests

ts-return-type-inference/, python-return-type-inference/, go-return-type-inference/, java-return-type-inference/ exist but no integration test file references them (confirmed by search). The Ruby ruby-yard-annotations/ fixture is properly tested.


Summary of What Works vs. What Doesn't

Pattern TS Python Java Go Ruby
var: Type = ... (explicit annotation) ❌ (no annotations)
var = new Class() ✅ (callForm) N/A ✅ (via User.new)
var = fn() return type inference ❌ no scanner ❌ no scanner ❌ no scanner ❌ no scanner + no @return
@param name [Type] (YARD) N/A N/A N/A N/A ✅ new
Namespaced constructor (Ns::Class.new) N/A N/A N/A N/A

Next Steps (from PR description + analysis)

Immediate follow-ups from this PR:

  1. Add integration tests for the 4 existing fixtures (ts-return-type-inference, python-return-type-inference, go-return-type-inference, java-return-type-inference) — the fixtures were created but never exercised.
  2. Add TypeScript/Java/Go scanners that capture untyped var = fn() assignments so return type inference can fire for those languages.
  3. Fix Ruby namespaced constructor: Handle scope_resolution receiver type in both extractRubyConstructorBinding and extractInitializer.
  4. Add Ruby @return YARD parsing in ruby.ts to populate returnType in SymbolDefinition — needed for return type inference from Ruby functions to work at all.

Planned Phase 3 remaining (from PR):

  • 3.2: Chained property access (this.repo.save()) — TypeEnv redesign needed
  • 3.3: For-loop variable typing — now unblocked by 3.1 + 3.4 being in this PR
  • 3.5: Block-level type narrowing — scope model changes needed

| Branch

…namespaced constructors

- Remove collection types (List, Array, Vec, Set) from WRAPPER_GENERICS to prevent
  false CALLS edges (e.g. List<User> no longer unwraps to User)
- Add :: qualifier handling in extractReturnTypeName for Ruby/C++/Rust namespaced types
- Add Ruby `constant` and `scope_resolution` node types to shared extractors
- Extract shared extractRubyConstructorAssignment helper (dedup type-env.ts + ruby.ts)
- Add integration tests for return type inference: Python, TypeScript, Go, Java, Ruby
- Add Ruby namespaced constructor fixture (Models::UserService.new)
- Add unit tests for collection reclassification and :: qualifiers
…rn type inference tests

Add CONSTRUCTOR_BINDING_SCANNERS for 6 missing languages, completing
return type inference coverage across all 11 supported languages:

- TypeScript/JS: variable_declarator with call_expression, unwraps await
- Go: short_var_declaration single-assignment (skips multi-return, new/make)
- Java: local_variable_declaration with `var` type + method_invocation
- C#: variable_declaration with implicit_type (var) + invocation_expression
- Rust: let_declaration without type annotation, handles mut_pattern
- PHP: assignment_expression with function_call_expression

Also adds property_identifier to extractSimpleTypeName for qualified
member calls (repo.getUser → getUser), fixing namespaced constructor
inference that was previously a known limitation.

Integration tests added for all 11 languages with correct label
assertions (Function vs Method per language's tree-sitter queries).
…geTypeConfig

Eliminates the parallel dispatch map in type-env.ts by moving all 11
constructor binding scanners into their respective type-extractors/*.ts
files as `scanConstructorBinding` on LanguageTypeConfig.

- Add ConstructorBindingScanner type to types.ts
- Add shared helpers: hasTypeAnnotation, unwrapAwait, extractCalleeName
- Move scanners to typescript.ts, jvm.ts, python.ts, php.ts, go.ts,
  rust.ts, swift.ts, c-cpp.ts, csharp.ts, ruby.ts
- Fix `any` types in C# scanner → SyntaxNode | null
- Delete ~300 lines from type-env.ts (CONSTRUCTOR_BINDING_SCANNERS map)
- Update buildTypeEnv to use config.scanConstructorBinding

All 143 type-env unit tests and all 10 language integration suites pass.
… comment

- Remove unused extractCalleeName import from jvm.ts
- Fix (c: any) → (c: SyntaxNode) in Java scanner
- Update stale CONSTRUCTOR_BINDING_SCANNERS reference in ruby.ts comment
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 6m 10s —— View job


Code Review: Constructor Type Resolution — Deep Analysis

Tasks:

  • Read trigger comment and understand request
  • Read key source files for constructor type resolution
  • Review what changed vs. main and how it improved type resolution
  • Find uncovered/suspicious gaps in constructor type resolution
  • Cross-reference with previous Claude comment findings
  • Identify next steps

Corrections to the Previous Review Comment

The previous Claude comment contained several inaccuracies — corrected here:

Claim Status
"TS/JS has no scanner" WRONGtypescript.ts has scanConstructorBinding for variable_declarator with call_expression
"Ruby extractVarName doesn't handle constant" WRONGshared.ts:100 includes 'constant' in the type check
"Ruby namespaced Models::User.new dropped" WRONGextractRubyConstructorAssignment in shared.ts:207–213 handles scope_resolution receivers
"4 fixture dirs have no integration tests" WRONG — all 4 fixtures (ts-, python-, go-, java-return-type-inference) have integration tests
"extractReturnTypeName can't handle :: qualified types" WRONGcall-processor.ts:414 uses `text.split(/::

What This PR Actually Improved

Phase 3 brought three concrete advances:

  1. Return type inference pipelineextractMethodSignature now captures returnType into SymbolDefinition. When var = callee() is scanned, processCallsFromExtracted looks up the callee's return type (for single-candidate callees) and binds the variable to that type. Receiver calls from that variable then resolve correctly.

  2. Ruby YARD @param extractionruby.ts replaces the empty stub with real YARD parsing (@param name [Type]). Qualified (Models::User → User) and nullable (String, nil → String) types are handled. Constructor inference for locally-known classes via User.new works.

  3. scanConstructorBinding refactor — All 11 per-language scanner functions were moved from a separate dispatch map in type-env.ts into their respective LanguageTypeConfig objects in the type-extractor files. type-env.ts shrank from 684 to 379 lines with no behavioral change.


Real Issues Found

🔴 Critical: C# and PHP integration tests are false positives

C# — member_access_expression not handled in scanner

csharp.ts:133 extracts the callee name from an invocation_expression via:

const func = value.firstNamedChild;  // member_access_expression for svc.GetUser()
const calleeName = extractSimpleTypeName(func);  // → undefined!

member_access_expression is not in extractSimpleTypeName's handled type list (shared.ts:17–28). For the fixture pattern var user = svc.GetUser("alice"), the RHS is an invocation_expression whose first named child is member_access_expression { svc . GetUser }. Since extractSimpleTypeName returns undefined for this node type, the scanner returns undefined — no binding is created for user.

The integration test at csharp.test.ts:589 passes only because Save is the sole method with that name in the fixture project — it resolves via fuzzy match, not return type inference. The PR description's claim that "C#'s CONSTRUCTOR_BINDING_SCANNER binds var user = svc.GetUser()" is incorrect for this common pattern.

Fix this →

PHP — member_call_expression skipped by scanner

php.ts:122 guards:

if (right.type !== 'function_call_expression') return undefined;

In tree-sitter-php, $this->getUser("alice") parses as member_call_expression, not function_call_expression. The fixture at php-return-type/app/Services/UserService.php:13 uses exactly this pattern — $user = $this->getUser("alice"). The scanner silently skips it.

Again the test at php.test.ts:726 passes only because save is unique in the fixture. True return type inference for PHP method calls ($obj->method()) does not work.

Fix this →

🟡 Gap: Go multi-return pattern — most idiomatic Go is uncovered

go.ts:151 explicitly limits to single-LHS assignments:

if (leftIds.length !== 1 || leftIds[0].type !== 'identifier') return undefined;

The dominant Go idiom for constructor-like calls is user, err := GetUser(), which is a 2-element LHS. This is never captured. The Go fixture (go-return-type-inference/cmd/main.go) avoids this by using a single-return GetUser(*User) — an uncommon Go pattern. The test therefore doesn't validate real-world Go code.

🟡 Gap: Ruby @return YARD annotation not parsed

ruby.ts parses @param name [Type] but has no @return [Type] extraction. extractMethodSignature in utils.ts looks for return_type and type_annotation AST nodes — neither exists in Ruby's dynamically-typed AST. As a result, SymbolDefinition.returnType is always undefined for Ruby methods. The return type inference path in call-processor.ts:476 (callableDefs[0].returnType) will never fire for Ruby functions.

The ruby-return-type/ fixture exists (added in this PR) but no integration test exercises it — this is the one fixture genuinely without a test.

Fix this →

🟡 Gap: Swift — 11 integration tests all skipped

Every Swift integration test introduced in this PR is skipped (visible in CI). The swift.ts scanner has scanConstructorBinding for property_declaration with call_expression, but the extractInitializer has a classNames.has(calleeName) guard that requires the cross-file SymbolTable lookup to succeed. This is the same pattern as Kotlin, which works — but some Swift-specific AST difference is likely causing the class lookup to fail silently.

🟡 Bug: C# scanner only checks firstNamedChild of variable_declarator

csharp.ts:119:

const nameNode = declarator.childForFieldName('name') ?? declarator.firstNamedChild;
if (!nameNode || nameNode.type !== 'identifier') return undefined;

In tree-sitter-c-sharp, the variable_declarator's name node may have type identifier but when multiple declarators exist (e.g., var a = f1(), b = f2()), only the first declarator is checked (line 116: break after first match). Multi-declarator var statements with different factory calls produce only one binding.


Summary Table (Updated)

Pattern TS JS Python Java Kotlin Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a n/a
var = new Class()
var = fn() return inference ✅* ✅** ⚠️ bug ⚠️ bug ❌ no @return ❌ skipped
var = obj.method() return inference n/a n/a n/a n/a ❌ bug ❌ bug
YARD @param n/a n/a n/a n/a n/a n/a n/a n/a n/a ✅ new n/a

* Java var-typed declarations only
** Go single-return only; user, err := fn() not captured


Recommended Next Steps (Priority Order)

  1. Fix C# member_access_expression: Add 'member_access_expression' to extractSimpleTypeName's qualified-names block, then add a disambiguating test (two classes with same method name) to prove it works.

  2. Fix PHP member_call_expression: Extend scanConstructorBinding to also handle member_call_expression (method calls on objects). Update the fixture to avoid the false-positive ambiguity.

  3. Investigate Swift skips: Debug why extractInitializer doesn't bind Swift constructor assignments in cross-file scenarios. Likely a classNames.has() lookup failure.

  4. Add Ruby @return extraction: Wire YARD @return [Type] through extractMethodSignatureSymbolDefinition.returnType to enable return type inference for Ruby.

  5. Go multi-return: Consider extracting the first variable from user, err := fn() patterns (common Go idiom). Requires carefully avoiding false bindings from err-like variable names.

  6. Add ruby-return-type integration test: The fixture exists but has no test suite.


| Branch

…re extraction, and cross-file resolution

Addresses code review findings on PR #284:

C# scanner (csharp.ts):
- Fix type node lookup: iterate children instead of childForFieldName('type')
  which returns undefined in tree-sitter-c-sharp
- Fix initializer lookup: handle direct invocation_expression children
  (no equals_value_clause wrapper in tree-sitter-c-sharp)

C# return type extraction (utils.ts):
- Add 'returns' field check to extractMethodSignature — tree-sitter-c-sharp
  uses 'returns', not 'type', for method return types

C# cross-file resolution (call-processor.ts + fixture):
- Add constructor binding verification to sequential processCalls path
  (was only in the worker processCallsFromExtracted path)
- Add ReturnType.csproj to csharp-return-type fixture
- Update fixture namespaces to use ReturnType.Models/ReturnType.Services
  prefix (matches real C# project conventions)

PHP scanner (php.ts):
- Extend scanConstructorBinding to handle member_call_expression
  ($this->getUser() patterns), not just function_call_expression

Shared (shared.ts):
- Add member_access_expression to extractSimpleTypeName qualified-names
  block (C# method calls like svc.GetUser())

Tests:
- Add Repo.cs/Repo.php disambiguation fixtures (two Save methods)
- Strengthen C# and PHP return type tests with hard disambiguation assertions
- Add C# scanner unit tests and return type extraction test
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 4m 42s —— View job


Code Review: Constructor Type Resolution — Third Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read key source files (call-processor.ts, type-env.ts, type-extractors/*.ts, shared.ts, ruby.ts)
  • Verify/challenge previous comment claims with fresh eyes on the actual code
  • Find new uncovered gaps in constructor type resolution
  • Summarize what changed, what works, what doesn't, and next steps

Corrections to the Second Review Comment

The second comment identified real bugs in C# and PHP, but both were already fixed in the code on this branch:

Second Comment Claim Actual Status
"C# member_access_expression not handled in extractSimpleTypeName" WRONGshared.ts:20 explicitly lists member_access_expression in the qualified-names block
"PHP member_call_expression skipped by scanner" WRONGphp.ts:128–132 explicitly handles member_call_expression with right.childForFieldName('name')

Both of those "critical" findings describe code that was already fixed before the second review was written. The existing integration tests for C# and PHP likely pass for the right reason, not just fuzzy matching.


What This PR Actually Improved (Accurate Summary)

Before Phase 3 After Phase 3
returnType never captured in SymbolDefinition extractMethodSignature now captures returnType for all typed languages
Ruby type extractor was an empty stub Real YARD @param parsing in ruby.ts
CONSTRUCTOR_BINDING_SCANNERS was a separate 305-line dispatch map in type-env.ts Merged into LanguageTypeConfig.scanConstructorBinding per language
No cross-file verification path for return types processCallsFromExtracted now falls back to returnType when callee isn't a Class

Real Issues Found (Not in Previous Comments)

🔴 Critical: Ruby ruby-return-type integration test is a false positive

File: test/integration/resolvers/ruby.test.ts:618 and test/fixtures/lang-resolution/ruby-return-type/

The test comment at ruby.test.ts:593–596 states:

"For plain function calls like get_user(), inference works via the scanner + SymbolTable return type lookup."

This claim is incorrect. Two independent layers both fail:

Layer 1 — Scanner gap: ruby.ts:186 sets scanConstructorBinding: extractRubyConstructorAssignment. That function only matches user = ClassName.new patterns (shared.ts:203–204). For user = get_user('alice'), the right side is a call node where method.text === 'get_user''new' → returns undefined at shared.ts:204. No binding is emitted.

Layer 2 — No returnType on Ruby methods: Even if a binding were emitted, extractMethodSignature in utils.ts only reads static AST fields (return_type, type_annotation, Go result, Rust return_type, etc.). Ruby's dynamic AST has none of these. SymbolDefinition.returnType is always undefined for Ruby methods. The guard at call-processor.ts:507 (callableDefs[0].returnType) will never fire.

Why the test passes: The fixture has exactly one save method (in models.rb). The call resolves via fuzzy match on the unique method name, not via return-type inference. The test is exercising the wrong code path.

Fix this →

🟡 Gap: Go selector_expression missing from extractSimpleTypeName

File: type-extractors/go.ts:159 and type-extractors/shared.ts:17–22

The Go scanner at go.ts:155–159 runs:

const func = rightExprs[0].childForFieldName('function');
const calleeName = extractSimpleTypeName(func);

For a package-qualified call like user := models.NewUser(name), tree-sitter-go parses the function field as a selector_expression { models . NewUser }. But extractSimpleTypeName in shared.ts:17–22 handles member_expression (JS) and member_access_expression (C#/C++) but NOT selector_expression (Go's package-qualified access). So extractSimpleTypeName(selector_expression) returns undefined, the binding is silently dropped.

This is a significant gap because package-qualified factory calls (pkg.NewFoo(), models.NewUser()) are idiomatic Go. The existing fixture uses single-package direct calls (NewUser(...)) specifically to avoid this path.

// Works:  user := NewUser(name)
// Broken: user := models.NewUser(name)  ← selector_expression not handled

Fix this →

🟡 Gap: Ruby scanConstructorBinding is narrower than Python's — plain function calls unscanned

File: type-extractors/ruby.ts:186 vs type-extractors/python.ts:80–103

Python's scanConstructorBinding (python.ts:80) captures all assignment nodes with any call RHS:

if (right.type !== 'call') return undefined;  // catches ANY function call

Ruby's scanner (extractRubyConstructorAssignment) only captures the .new pattern. This means user = get_user() in Ruby never produces a binding for SymbolTable verification, while the equivalent in Python does. Ruby is the only language with a scanner that's strictly narrower than a plain call capture.

The fix would require two parts: (1) broaden scanConstructorBinding in ruby.ts to capture any assignment with a call RHS, AND (2) populate SymbolDefinition.returnType for Ruby methods via YARD @return annotation parsing.

🟡 Gap: Ruby @return YARD annotation never parsed → returnType always undefined

File: type-extractors/ruby.ts and utils.ts:617–663

extractMethodSignature in utils.ts only reads static typed AST fields. Ruby methods have none. ruby.ts defines YARD_PARAM_RE and parses @param, but there is no @return [Type] extraction. The ruby-return-type/models.rb fixture defines:

def get_user(name)
  User.new(name)
end

No YARD @return [User] comment is present, so there's nothing to parse even if the parser existed. The fixture itself is set up to fail at the annotation level before the scanner level.

Fix this →

🟡 Gap: Java scanner limited to var keyword — method_invocation inside non-var declarations missed

File: type-extractors/jvm.ts:71–86

scanJavaConstructorBinding guards typeNode.text !== 'var' (line 75), so it only fires for Java 10+ type-inferred declarations. For UserService svc = factory.create() (explicit type), extractJavaDeclaration correctly binds svc → UserService from the type node directly. This is by design and correct.

However, there's a subtler gap: the scanner checks value.type !== 'method_invocation' but Java has another call form: value.type === 'object_creation_expression' which is already skipped (line 81). What about MethodInvocation vs InvocationExpression? The scanner requires exactly method_invocation, which is the tree-sitter-java node type for foo.bar(). A standalone call like create() (no receiver) in Java also parses as method_invocation. This looks correct. Not a bug, just noting the intentional scope constraint.

🟢 Swift — skipped due to unavailable parser, not code bugs

All 11 Swift integration tests are skipped because isLanguageAvailable(SupportedLanguages.Swift) returns false on Node 22 (tree-sitter-swift has known build issues). The skip logic is correct — describe.skipIf(!swiftAvailable). The Swift code itself (swift.ts) looks structurally sound based on static analysis. The scanConstructorBinding in swift.ts:81–107 correctly handles both User(name:) and User.init(name:) patterns.


Summary Table (Accurate Third Pass)

Pattern TS/JS Python Java Kotlin Go Rust C# PHP Ruby Swift
Explicit annotation n/a
var = new Class() / Class.new
var = fn() return-type inference ✅ (var only) ❌ double gap ⚠️ skipped
var = pkg.fn() package-qualified ✅ (attribute) n/a ❌ selector_expression ⚠️ skipped
YARD @return → return type n/a n/a n/a n/a n/a n/a n/a n/a ❌ never set n/a
Go user, err := fn() multi-return n/a n/a n/a n/a ❌ intentional n/a n/a n/a n/a n/a

Recommended Next Steps (Priority Order)

  1. Harden the ruby-return-type fixture — Add a second class with a save method to the fixture so the test actually requires type disambiguation (exposing the false positive). Then fix the two underlying gaps.

  2. Fix selector_expression in extractSimpleTypeName — A one-line addition to shared.ts:22 unlocks package-qualified Go factory calls. Also add field_identifier to the child-type allowlist at line 24-26. Low risk, high impact for Go codebases.

  3. Add Ruby @return YARD parsing — Extend collectYardParams (or add a sibling) to parse @return [Type] and thread through extractMethodSignatureSymbolDefinition.returnType. Required before Ruby return-type inference can work at all.

  4. Broaden Ruby scanConstructorBinding — After readme #3, change scanConstructorBinding in ruby.ts from extractRubyConstructorAssignment to a function that also captures plain call assignments user = get_user() (matching Python's approach).

  5. Go multi-return considerationuser, err := fn() is idiomatic Go. Extracting only the first LHS for error-idiom patterns would be safe (err, _ are well-known discard names). Worth a separate small PR.


| Branch

…or PHP, JS, Ruby

Add ReturnTypeExtractor to LanguageTypeConfig interface with implementations
for Ruby (YARD @return), PHP (PHPDoc @return), and JS/TS (JSDoc @returns).
The fallback is wired in both parsing-processor and parse-worker paths,
activating only when extractMethodSignature finds no AST-based return type.

Also add doc-comment @param type extraction for PHP and JS/TS, following
Ruby's existing collectYardParams pattern. This enables parameter.method()
resolution in loosely-typed codebases using PHPDoc @param or JSDoc @param.

Additional fixes from PR #284 code review:
- Go: add selector_expression + field_identifier to extractSimpleTypeName
  (enables package-qualified factory calls like models.NewUser())
- Ruby: broaden scanConstructorBinding to capture plain call assignments
  (user = get_user()) in addition to Class.new patterns
- Ruby: harden return-type fixture with disambiguation (two save methods)

Test coverage: +14 new integration tests across Go, Ruby, PHP, JS/TS
@magyargergo

Copy link
Copy Markdown
Collaborator Author

Review Response: Third-Pass Analysis (claude[bot])

All findings verified and addressed in commit 36ca3a3.

Fixes Applied

Finding Status What Changed
🔴 Ruby ruby-return-type false positive Fixed Split fixture into models.rb + repo.rb (two save methods), added YARD @return [User]/@return [Repo] annotations, broadened scanConstructorBinding to capture plain call assignments
🟡 Go selector_expression missing Fixed Added selector_expression + field_identifier to extractSimpleTypeName in shared.ts. New test: models.NewUser() cross-package factory call
🟡 Ruby @return YARD never parsed Fixed Added ReturnTypeExtractor type to LanguageTypeConfig, implemented in ruby.ts, wired fallback in both parsing-processor.ts and parse-worker.ts
🟡 Ruby scanner narrower than Python Fixed scanConstructorBinding now captures both User.new and get_user() patterns

Additional Work (beyond review scope)

Extended the ReturnTypeExtractor pattern to PHP and JS/TS:

Language extractReturnType @param doc-comment parsing
Ruby YARD @return [Type] Already had collectYardParams
PHP PHPDoc @return Type New: PHPDoc @param Type $name
JS/TS JSDoc @returns {Type} New: JSDoc @param {Type} name

Test Results

Suite Count New
Unit (type-env + method-signature) 175
Ruby integration 61 +3
Go integration 43 +1
PHP integration 66 +5
TS/JS integration 77 +5

Corrections Confirmed

The second review's "critical" findings were indeed already fixed on this branch:

  • C# member_access_expression — present at shared.ts:20
  • PHP member_call_expression — handled at php.ts:128-132

@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 9m 42s —— View job


Code Review: Constructor Type Resolution — Fourth Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read latest source files (post-36ca3a3 commit changes)
  • Verify fixes from third review are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Summarize what now works, what's still broken, and next steps

Verification of Third Review Fixes (Commit 36ca3a3)

All four fixes claimed in the PR author's response are confirmed correct:

Claimed Fix Verified
Ruby ruby-return-type false positive — added repo.rb (two save methods) ✅ Confirmed: models.rb has User#save, repo.rb has Repo#save, test checks for ≥2 save methods and verifies correct target file
Go selector_expression + field_identifier added to extractSimpleTypeName ✅ Confirmed: shared.ts:22-28 has both additions; go-return-type-inference fixture tests models.NewUser("bob") cross-package call
Ruby @return YARD parsing added via extractReturnType ✅ Confirmed: ruby.ts:189-211, wired in parse-worker.ts:1052-1057
Ruby scanConstructorBinding broadened to capture plain call assignments ✅ Confirmed: ruby.ts:219-236 tries .new first, then falls through to plain call capture

What Genuinely Works After Phase 3 + 36ca3a3

Pattern TS JS Python Java Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a n/a
var = new Class()
var = fn() return inference ✅ (var only) ⚠️ skipped
var = pkg.fn() qualified fixed n/a ⚠️ skipped
Doc-comment return annotation n/a ✅ JSDoc n/a n/a n/a n/a ✅ PHPDoc new YARD n/a
@param doc annotation ✅ JSDoc ✅ JSDoc n/a n/a n/a n/a ✅ PHPDoc new YARD n/a

New Issues Found (Not in Previous Reviews)

🟡 Bug: normalizeJsDocType strips Promise<User>Promise before storage — async JS functions produce wrong bindings

Location: gitnexus/src/core/ingestion/type-extractors/typescript.ts:29 + typescript.ts:169

normalizeJsDocType strips generic wrappers to their base type:

// Strip generic wrapper: Promise<User> → Promise (base type, not inner)
const genericMatch = type.match(/^(\w+)\s*</);
if (genericMatch) type = genericMatch[1];

This is correct for @param {Array<User>} (binding the param to Array). But for @returns {Promise<User>}, normalizeJsDocType('Promise<User>') returns 'Promise'. This is then stored as SymbolDefinition.returnType = 'Promise'.

In call-processor.ts:508, extractReturnTypeName('Promise') is then called. Since 'Promise' starts with uppercase and is not in PRIMITIVE_TYPES, it returns 'Promise'. The variable is bound to type 'Promise' — completely wrong.

extractReturnTypeName already handles this correctly when it receives the full string: extractReturnTypeName('Promise<User>')WRAPPER_GENERICS.has('Promise') → unwraps → 'User'. But normalizeJsDocType strips the generic before it ever reaches that function.

Affected path: JavaScript files with async functions annotated @returns {Promise<User>} (common in real-world JS codebases). TypeScript is unaffected because fetchUserAsync(): Promise<User> has the AST return type Promise<User> captured directly by extractMethodSignature without passing through normalizeJsDocType.

The existing js-jsdoc-return-type fixture avoids this because @returns {User} (no wrapper) — the tests pass but don't exercise the async path.

Fix: In extractReturnType (typescript.ts:164-174), return match[1] directly (optionally stripping just the JSDoc ?/! prefix), rather than calling normalizeJsDocType. Let extractReturnTypeName in call-processor.ts handle the wrapper unwrapping. @param annotation handling is a separate path (collectJsDocParams) and is unaffected.

Fix this →

🟡 Gap: PHP 8+ #[Attribute] nodes between PHPDoc and method break extractReturnType

Location: gitnexus/src/core/ingestion/type-extractors/php.ts:211-218

The comment walker stops at the first named non-comment sibling:

while (sibling) {
  if (sibling.type === 'comment') {
    const match = PHPDOC_RETURN_RE.exec(sibling.text);
    if (match) return normalizePhpType(match[1]);
  } else if (sibling.isNamed) break;  // ← stops at attribute_list
  sibling = sibling.previousSibling;
}

For PHP 8+ code with Symfony/Laravel attributes:

/** @return User */
#[Route('/user')]   // ← attribute_list node (isNamed = true) → BREAKS the walker
public function getUser() { ... }  // ← no explicit return type

The attribute_list is a named sibling between the PHPDoc block and the method. The walker reaches attribute_list, which isNamed → breaks, never finding the @return annotation. The same pattern affects the Ruby and JS/TS extractReturnType walkers for the analogous decorator patterns (Python @decorator, JS @decorator).

Note: this only affects untyped PHP methods. If getUser(): User has an explicit return type, extractMethodSignature captures it and extractReturnType is never called. The gap is for legacy untyped PHP code with attributes, which is a realistic PHP 8 migration scenario.

Fix: Allow skipping past known attribute/decorator node types (attribute_list, attribute) in the walker loop rather than breaking on them.

Fix this →

🟡 Gap: PHP member_call_expression scanner discards receiver — multiple classes with same method name causes no binding

Location: gitnexus/src/core/ingestion/type-extractors/php.ts:195-199

For $user = $this->getUser("alice"), the scanner captures only calleeName = 'getUser' (method name without receiver):

if (right.type === 'member_call_expression') {
  const methodName = right.childForFieldName('name');
  if (!methodName) return undefined;
  return { varName: left.text, calleeName: methodName.text };  // ← only method name
}

In call-processor.ts:507, callableDefs.length === 1 is required. If two classes in a project both have getUser() methods, the SymbolTable lookup returns 2 candidates → no binding. The existing php-return-type fixture passes only because UserService is the sole class with getUser(). Any PHP project with e.g. both UserService::getUser() and AdminService::getUser() would silently fail receiver type inference for the $this->getUser() pattern.

This is also present in Java's scanJavaConstructorBinding (captures only methodName.text from method_invocation), but is more impactful in PHP since PHP method calls commonly go through $this->.

There's no clean fix without receiver-type context (the receiver is $this, which requires knowing the enclosing class, which is available via findEnclosingClass). A targeted improvement: include the enclosing class name as a qualifier in the callee lookup when the receiver is $this/self/static.


Previously Identified Gaps That Remain

These were documented in earlier reviews and are still present (not introduced by this PR, and not fixed yet):

Gap Status
Go user, err := fn() multi-return — only first LHS captured for single-LHS Intentional/deferred
Java scanJavaConstructorBinding limited to var declarations Intentional design
Swift — all 11 tests skipped (tree-sitter-swift build failure on Node 22) Infrastructure, not code bug
Kotlin scanner can't handle val user = factory.getUser() qualified calls Acceptable simplification

Previous Review Accuracy Summary

Review Finding Accuracy
1st review "TS/JS has no scanner" ❌ Wrong — typescript.ts has scanConstructorBinding
1st review "Ruby constant not in extractVarName" ❌ Wrong — shared.ts:104 includes it
1st review "4 fixture dirs have no integration tests" ❌ Wrong — all have tests
2nd review "C# member_access_expression not in extractSimpleTypeName" ❌ Wrong — shared.ts:20 has it
2nd review "PHP member_call_expression skipped" ❌ Wrong — php.ts:195 handles it
3rd review "Ruby ruby-return-type is a false positive" ✅ Correct — fixed in 36ca3a3
3rd review "Go selector_expression missing" ✅ Correct — fixed in 36ca3a3
3rd review "Ruby @return YARD never parsed" ✅ Correct — fixed in 36ca3a3
3rd review "Ruby scanner narrower than Python" ✅ Correct — fixed in 36ca3a3

The third review was accurate. Commit 36ca3a3 addressed all real issues found.


Recommended Next Steps (Priority Order)

  1. Fix normalizeJsDocType for @returns pathextractReturnType in typescript.ts:169 should not run normalizeJsDocType (which strips wrapper generics). Instead, pass the raw type string to extractReturnTypeName in call-processor.ts, which already has WRAPPER_GENERICS logic. Add a JS fixture with @returns {Promise<User>} to verify.

  2. Allow attribute/decorator nodes in extractReturnType walkers — Extend the skip set in the comment-walker loop in php.ts (and the same pattern in typescript.ts, ruby.ts) to skip attribute_list/attribute nodes instead of breaking on them. This is a one-liner fix per extractor.

  3. PHP $this->method() receiver disambiguation — Use findEnclosingClass (already in php.ts) to qualify calleeName with the enclosing class name when the receiver is $this/self/static, reducing false disambiguation failures.


| Branch

…er disambiguation

Three fixes from fourth-pass code review on PR #284:

1. JSDoc `@returns {Promise<User>}` no longer stripped to `Promise` — extractReturnType
   now uses sanitizeReturnType (preserves generics) instead of normalizeJsDocType
   (which stripped them before extractReturnTypeName could unwrap WRAPPER_GENERICS).

2. PHP 8+ `#[Attribute]` and JS `@decorator` nodes no longer break doc-comment walkers.
   Both extractReturnType and collect*Params functions now skip attribute_list/decorator
   nodes instead of breaking on them as named siblings.

3. PHP `$this->method()` now provides receiverClassName for disambiguation.
   When two classes define the same method, the enclosing class narrows candidates
   via ownerId matching in call-processor, preventing false no-binding results.
Repository owner deleted a comment from claude Bot Mar 15, 2026
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 8m 20s —— View job


Code Review: Constructor Type Resolution — Fifth Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read latest source files (post-fda19a9 commit)
  • Verify 4th review fixes are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Summarize what now works, what's still broken, and next steps

Corrections to the Fourth Review Comment

All four findings from the fourth review were legitimate and are confirmed fixed in commit fda19a9:

Fourth Review Claim Status
"normalizeJsDocType strips Promise<User> before extractReturnTypeName can unwrap it" FixedsanitizeReturnType replaces normalizeJsDocType for the @returns path; it preserves generic wrappers
"PHP attribute_list between PHPDoc and method breaks comment walker" FixedSKIP_NODE_TYPES = { 'attribute_list', 'attribute' } at php.ts:70-71
"PHP $this->method() scanner discards receiver — disambiguation fails when two classes share a method name" FixedscanConstructorBinding in php.ts:199-211 now passes receiverClassName (enclosing class) for $this/self/static receivers, and call-processor.ts:509-515 narrows candidates by owner class
"Ruby false positive: ruby-return-type fixture has only one save method — test doesn't prove disambiguation" Fixedrepo.rb added with Repo#save, broadened scanner, YARD @return parsed

The parsing-processor.ts also has the extractReturnType fallback (line 220-225), mirroring parse-worker.ts:1052-1057 — both code paths are covered.


What This PR Has Accomplished (Accurate Fifth-Pass Summary)

After commit fda19a9, the resolution matrix is:

Pattern TS JS Python Java Kotlin Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a n/a
var = new Class()
var = fn() return inference ✅ (var) ✅ new ⚠️ skipped
var = pkg.fn() qualified n/a fixed n/a ⚠️ skipped
await fn() async inference ✅ new n/a n/a n/a n/a n/a n/a n/a n/a n/a
$this->fn() receiver hint n/a n/a n/a n/a n/a n/a n/a n/a new n/a n/a
Doc-comment return type JSDoc JSDoc n/a n/a n/a n/a n/a PHPDoc new PHPDoc new YARD new n/a

New Issues Found (Not in Previous Reviews)

🔴 Bug: sanitizeReturnType corrupts @returns {Promise<models.User>} — dot inside angle brackets

Location: gitnexus/src/core/ingestion/type-extractors/typescript.ts:172-173

const dotIdx = type.lastIndexOf('.');
if (dotIdx >= 0) type = type.slice(dotIdx + 1);  // ← finds dot INSIDE angle brackets

For @returns {Promise<models.User>}, lastIndexOf('.') finds the dot in models.User at position 14 inside the generic brackets. type becomes 'User>' (with trailing >). extractReturnTypeName('User>') fails the ^[A-Z_]\w*$ regex and returns undefined — silent type inference failure.

The existing test at typescript.test.ts:968 uses @returns {Promise<User>} (no dot inside generic), which has dotIdx = -1 and passes correctly. The bug is masked by the test fixture design.

Why the dot-stripping is unnecessary: extractReturnTypeName in call-processor.ts:444-447 already handles qualified names:

if (text.includes('::') || text.includes('.')) {
  text = text.split(/::|\./).pop()!;
}

So sanitizeReturnType can safely return 'models.User' and extractReturnTypeName will reduce it to 'User'. The dot-stripping in sanitizeReturnType is both redundant and harmful for the generic-wrapper case.

Safe fix: Remove lines 172-173 from sanitizeReturnType. With this change, 'Promise<models.User>' is passed intact to extractReturnTypeName, which correctly unwraps to 'User'.

Fix this →

🟡 Gap: JS/TS async return type tests lack negative assertions

Location: gitnexus/test/integration/resolvers/typescript.test.ts:968-982

The models.js fixture has both User#save and Repo#save. The test only checks:

// Positive: some save call from processUser targets models.js
const saveCall = calls.find(c =>
  c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('models.js'),
);
expect(saveCall).toBeDefined();

Since both methods live in models.js, this assertion passes whether type inference correctly returns User#save or falls back to fuzzy-matching both User#save and Repo#save. A broken type inference path (e.g., if the Promise unwrapping regressed) would still pass the test.

Compare to the PHP test at php.test.ts:736-741 which correctly adds:

// Negative: must NOT resolve to wrong class
const repoSave = calls.find(c =>
  c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('Repo.php'),
);
expect(repoSave).toBeUndefined();

Fix: Add a negative assertion to the JS async test: expect(calls.find(c => c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('models.js') && c.targetNodeId.includes('Repo'))).toBeUndefined(). Or simplify by moving each class into its own file (one user.js with User#save, one repo.js with Repo#save) so the targetFilePath check suffices to confirm disambiguation.

Fix this →

🟡 Gap: Ruby scanConstructorBinding — constant LHS (SERVICE = get_user()) not captured

Location: gitnexus/src/core/ingestion/type-extractors/ruby.ts:229

if (left.type !== 'identifier') return undefined;  // ← skips 'constant' LHS for plain calls

SERVICE = UserService.new is correctly handled by extractRubyConstructorAssignment (which handles both identifier and constant LHS in shared.ts:203). However, SERVICE = get_user() (constant LHS, plain function call) falls through to the plain-call path at ruby.ts:229, which guards identifier only. The constant assignment would be silently dropped.

This is a narrow gap — constant Ruby assignments from plain factory calls are uncommon in practice. The .new path covers the dominant constructor pattern.

🟡 Observation: Go multi-return user, err := GetUser() — still intentionally excluded

Location: gitnexus/src/core/ingestion/type-extractors/go.ts:151-152

// Single assignment only — skip multi-return like `user, err := GetUser()`
if (leftIds.length !== 1 || leftIds[0].type !== 'identifier') return undefined;

The Go scanner intentionally skips multi-return patterns. This is the dominant Go factory-call idiom. A targeted improvement would be to extract the first LHS variable when the second is err or _ (standard Go error-idiom discard names). This doesn't block the PR but is the most impactful remaining gap for real-world Go codebases.


Previous Review Accuracy Tracking (All 4 Passes)

Review Finding Accuracy
1st review "TS/JS has no scanner" ❌ Wrong
1st review "Ruby constant not in extractVarName" ❌ Wrong
1st review "4 fixture dirs have no integration tests" ❌ Wrong
2nd review "C# member_access_expression not in extractSimpleTypeName" ❌ Wrong
2nd review "PHP member_call_expression skipped" ❌ Wrong
3rd review "Ruby false positive, Go selector_expression, Ruby @return" ✅ All correct — fixed in 36ca3a3
4th review "normalizeJsDocType strips Promise<User>, PHP attribute walker, PHP $this receiver" ✅ All correct — fixed in fda19a9
5th review (this) sanitizeReturnType corrupts Promise<models.User>; JS async tests lack negative assertions

The codebase is in genuinely good shape. The third and fourth review passes were accurate; the fixes were well-implemented. The only confirmed new bug is the sanitizeReturnType dot-stripping edge case — a one-line fix.


Recommended Next Steps (Priority Order)

  1. Fix sanitizeReturnType — Remove 2 lines (dotIdx/slice at typescript.ts:172-173). Low risk, high correctness. Add a fixture test with @returns {Promise<models.User>} to lock this in.

  2. Strengthen JS async test assertions — Split models.js into separate files or add negative assertions. Prevents silent regressions in the Promise-unwrapping path.

  3. Go multi-return (user, err := GetUser()) — Consider extracting first LHS when second is err/_. High real-world impact for Go codebases.

  4. Ruby constant LHS for plain calls — Change ruby.ts:229 to if (left.type !== 'identifier' && left.type !== 'constant') return undefined. Minor gap, easy fix.

  5. Swift — All 11 tests still skipped due to tree-sitter-swift build issues on Node 22. Infrastructure issue, not a code bug.


| | Branch

@magyargergo magyargergo changed the title feat: Phase 3 type resolution — return type inference, generic args, Ruby YARD feat: return type inference, doc-comment parsing, and per-language type extractors Mar 15, 2026
…tant receiver

- Remove redundant dot-path stripping from sanitizeReturnType that corrupted
  qualified names inside generics (e.g. Promise<models.User> → User>)
- Split JS async fixture into separate files and add negative assertions
  to properly verify disambiguation (mirroring PHP test pattern)
- Accept 'constant' node type in Ruby scanConstructorBinding for factory
  call assignments (SERVICE = build_service())
- Add 'constant' to SIMPLE_RECEIVER_TYPES so extractReceiverName handles
  Ruby constant receivers (SERVICE.process)
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 7m 2s —— View job


Code Review: Constructor Type Resolution — Eighth Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read latest source files (post-5caaae9 commit)
  • Verify 7th review fixes are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Summarize what now works, what's still broken, and next steps

Verification of Seventh Review Fixes (Commit 5caaae9)

All three items from the seventh review are confirmed addressed:

Claimed Fix Verified
Swift scanConstructorBindingsuffix?.type === 'simple_identifier' wrong for tree-sitter-swift ✅ Fixed: swift.ts:108-116 now checks both simple_identifier AND navigation_suffix, unwrapping the inner lastNamedChild for the method name
Rust lifetime parameters (Ref<'_, User>) — extractFirstGenericArg split on inner comma ✅ Fixed: extractFirstTypeArg at call-processor.ts:481-492 iterates through lifetime tokens ('a, '_, 'static) and skips them to find the first real type arg
Kotlin return-type disambiguation unproven — no competing save() methods ✅ Acknowledged: Repo.kt added with Repo#save, lenient assertion documents the known gap, test now proves User#save is not spuriously emitted

What This PR Has Accomplished (Eighth-Pass Summary)

After commit 5caaae9, the complete resolution matrix:

Pattern TS JS Python Java Kotlin Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a n/a
var = new Class() / Class.new
var = fn() return inference ✅ (var) ⚠️ known gap ⚠️ skipped
var, err = fn() multi-return n/a n/a n/a n/a n/a n/a n/a n/a n/a n/a
var = pkg.fn() qualified n/a ⚠️ skipped
var = service.fn() receiver call n/a n/a ⚠️ skipped
$this->fn() receiver hint n/a n/a n/a n/a n/a n/a n/a n/a n/a n/a
Doc-comment return type JSDoc ✅ JSDoc ✅ n/a n/a n/a n/a n/a PHPDoc ✅ PHPDoc ✅ YARD ✅ n/a
Nested wrapper W<R<T,E>> n/a n/a n/a n/a
Rust lifetime wrappers Ref<'_, T> n/a n/a n/a n/a n/a n/a n/a n/a n/a n/a

New Issues Found

🟡 Gap: Kotlin return type inference — extractMethodSignature likely misses Kotlin return types

Location: gitnexus/src/core/ingestion/utils.ts:662-668

The Kotlin known gap (documented in kotlin.test.ts:484-513) has a likely root cause that hasn't been identified in previous reviews.

extractMethodSignature reads return types via a generic scan:

for (const child of node.children) {
  if (child.type === 'type_annotation' || child.type === 'return_type') {
    const typeNode = child.children.find((c) => c.isNamed);
    if (typeNode) returnType = typeNode.text;
  }
}

In tree-sitter-kotlin, fun getUser(name: String): User may NOT wrap the return type in a type_annotation node. Instead, the : User portion is likely parsed as a user_type direct child of function_declaration (after the colon punctuation token) — similar to how Go uses a result field (handled explicitly at line 619-634) and Rust uses a return_type field (handled explicitly at line 638-643).

If Kotlin return types are not type_annotation nodes, then callableDefs[0].returnType is always undefined in verifyConstructorBindings, and the return type inference chain never fires for any Kotlin function. This would explain why the test says disambiguation "does not yet resolve competing same-named methods."

The fix would be a Kotlin-specific case in extractMethodSignature, analogous to the Go and Rust cases already there:

// Kotlin: function return type is a user_type direct child (after ':' punctuation)
if (!returnType) {
  for (const child of node.children) {
    if (child.type === 'user_type' || child.type === 'nullable_type') {
      returnType = child.text;
      break;
    }
  }
}

However, this must be guarded to only apply to Kotlin functions (not accidentally applied to other languages), and requires verifying the actual tree-sitter-kotlin AST structure for function declarations.

Fix this →

🟡 Gap: Java scanJavaConstructorBinding discards receiver — var user = service.getUser() fails when two classes share the method name

Location: gitnexus/src/core/ingestion/type-extractors/jvm.ts:82-85

if (value.type !== 'method_invocation') return undefined;
const methodName = value.childForFieldName('name');
if (!methodName) return undefined;
return { varName: nameNode.text, calleeName: methodName.text };

For var user = adminService.getUser("alice"), calleeName = 'getUser' with no receiver info. If UserService and AdminService both have getUser(), callableDefs.length > 1 → no binding. The PHP $this->method() issue received receiverClassName disambiguation (now at php.ts:199-211). Java instance method calls with var declarations have no equivalent — the receiver variable's type would need to be looked up in the local TypeEnv, a two-pass problem not solvable at the scanner level.

This is a more fundamental architectural gap than a simple bug. Explicit Java typing (UserService user = service.getUser()) is already handled by extractJavaDeclaration. The var path has this limitation for method calls on non-this receivers.

🟡 Gap: Rust scanConstructorBinding fires for let user = User::default() — produces unhelpful calleeName: 'default'

Location: gitnexus/src/core/ingestion/type-extractors/rust.ts:174-177

The scanner excludes ::new calls (returns undefined) since extractInitializer handles them. But ::default() is not excluded:

if (func.type === 'scoped_identifier') {
  const methodName = func.lastNamedChild;
  if (methodName?.text === 'new') return undefined;  // ← only 'new', not 'default'
}
const calleeName = extractSimpleTypeName(func);  // returns 'default' for User::default()

For let user = User::default(), the scanner produces { calleeName: 'default' }. In verifyConstructorBindings, ctx.resolve('default', filePath) finds many candidates (Rust's Default trait is widely implemented) → callableDefs.length > 1 → no binding. The extractInitializer path in typeEnv already handles User::default() correctly for same-file resolution, so this is a harmless inefficiency (slightly wrong scanner input, but no false binding created). Cross-file User::default() where User is in another file would also not work via the scanner — but neither does it via extractInitializer since that is same-file only.

The safe fix is a one-liner: add || methodName?.text === 'default' to the exclusion check, matching extractInitializer's behavior at rust.ts:125.

Fix this →


Previous Review Accuracy Tracking (All 8 Passes)

Review Findings Accuracy
1st TS/JS no scanner; Ruby extractVarName gaps; 4 fixtures untested ❌ All wrong — features already existed
2nd C# member_access_expression; PHP member_call_expression ❌ Both wrong — already handled
3rd Ruby false positive; Go selector_expression; Ruby @return ✅ All correct — fixed in 36ca3a3
4th normalizeJsDocType strips generics; PHP attribute walker; PHP $this receiver ✅ All correct — fixed in fda19a9
5th sanitizeReturnType dot corruption; async test false positives; Ruby constant LHS ✅ All correct — fixed in 9fbbe2d
6th Sync JSDoc false positive; Ruby factory call false positive; nested wrapper generics ✅ All correct — fixed in 6465b8d
7th Kotlin disambiguation unproven; Swift navigation_suffix; Rust lifetime wrapper ✅ All correct — fixed in 5caaae9
8th (this) Kotlin extractMethodSignature gap; Java receiver disambiguation; Rust ::default()

The pattern is clear: reviews 3–7 all found genuine issues that were promptly fixed. Reviews 1–2 were wrong because they hadn't read the code carefully enough. The codebase is in excellent shape with only three remaining gaps, all of which are either architectural limitations or minor inefficiencies.


Recommended Next Steps (Priority Order)

  1. Investigate Kotlin extractMethodSignature — The root cause of the known gap is likely the utils.ts return type scan missing Kotlin's AST structure. Parse fun getUser(name: String): User {} with tree-sitter-kotlin to confirm the node type, then add a specific extraction case. Changing the lenient test to a strict assertion will confirm the fix.

  2. Rust ::default() exclusion — One-line fix. Add || methodName?.text === 'default' at rust.ts:176. Prevents the scanner from generating useless cross-file bindings for common Rust trait methods.

  3. Java var + method receiver disambiguation — Architectural gap. Would require two-pass scanning (scan TypeEnv first, then use it to qualify the scanner's output). Best addressed as a separate follow-up issue rather than in this PR.

  4. Swift infrastructure — All 11 tests remain skipped due to tree-sitter-swift build failure on Node 22. Infrastructure issue beyond PR scope.


| Branch

…Doc alternate order, escapeValue hardening

- Add C# null-conditional call support (user?.Save()): tree-sitter query for
  conditional_access_expression, member_binding_expression in MEMBER_ACCESS_NODE_TYPES,
  receiver extraction via conditional_access_expression parent walk
- Fix Ruby YARD type parsing for nested generics (Hash<Symbol, User>): replace
  naive split(',') with bracket-balanced splitter respecting <> depth
- Add alternate YARD format (@param [Type] name) alongside standard (@param name [Type])
- Add alternate PHPDoc format (@param $name Type) alongside standard (@param Type $name)
- Harden escapeValue in kuzu-adapter.ts: escape \n and \r to prevent Cypher injection
- Integration tests: C# null-conditional fixture (5 tests), Ruby YARD generics fixture (6 tests)
- Unit tests: PHPDoc alternate order (2 tests), C# null-conditional call-form (updated)
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 10m 55s —— View job


Code Review: Constructor Type Resolution — Ninth Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Check for new commits since 8th pass
  • Read key source files (call-processor.ts, type-extractors/*.ts, utils.ts, rust.ts, jvm.ts, lbug-adapter.ts)
  • Verify 8th review fixes are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Summarize what now works, what's still broken, and next steps

New Commit Since 8th Pass: ec4dca4

One new commit lands since the 8th review (5caaae9):

Feature Status
C# null-conditional calls user?.Save(): tree-sitter query + member_binding_expression in MEMBER_ACCESS_NODE_TYPES + receiver extraction via conditional_access_expression parent walk ✅ Correctly implemented
Ruby YARD bracket-balanced generic splitting — Hash<Symbol, User> no longer breaks on inner comma ✅ Correctly implemented
Ruby alternate YARD @param [Type] name format ✅ Correctly implemented; params.has(paramName) guard gives standard format priority
PHP alternate PHPDoc @param $name Type format ✅ Correctly implemented; both regex lastIndex properly reset before each use
escapeValue \n/\r hardening in lbug-adapter.ts ⚠️ Partially applied — see below

Verification of 8th Review Fixes (Commit 5caaae9)

All three items from the 8th review are confirmed correctly addressed:

Claimed Fix Verified
Swift scanConstructorBindingsuffix?.type === 'simple_identifier' wrong, should unwrap navigation_suffix swift.ts:108-116 now checks both simple_identifier AND navigation_suffix, with lastNamedChild unwrapping
Rust lifetime parameters Ref<'_, User>extractFirstGenericArg split on inner comma extractFirstTypeArg at call-processor.ts:481-492 iterates past 'lifetime tokens
Kotlin disambiguation proof — lenient test with Repo.kt added ✅ Lenient assertions correctly document the known gap without false failures

New Issues Found

🔴 Bug: lbug-adapter.ts batch upsert escapeValue missing \n/\r hardening — same Cypher injection vector as CREATE path

Location: gitnexus/src/core/lbug/lbug-adapter.ts:425-430

Commit ec4dca4 hardened the escapeValue in the CREATE path (line 364) to escape \n and \r:

// Line 364 — correctly hardened ✓
return `'${String(v).replace(/\\/g, '\\\\').replace(/'/g, "''")
           .replace(/\n/g, '\\n').replace(/\r/g, '\\r')}'`;

But the MERGE/upsert path at line 425 uses a separate escapeValue closure that was not updated:

// Line 425-430 — NOT hardened ❌
const escapeValue = (v: any): string => {
  if (v === null || v === undefined) return 'NULL';
  if (typeof v === 'number') return String(v);
  return `'${String(v).replace(/\\/g, '\\\\').replace(/'/g, "''")}'`;
  //                                         ↑ missing \n and \r escaping
};

The MERGE path is used by the batch upsert function and passes the same content fields (multiline source code) through escapeValue. A file containing raw newlines in its indexed content — e.g. any source file — will produce a malformed Cypher query when upserted via this path. The \n and \r escaping is identical to what was added to line 364; this is a missed instance of the same fix.

Fix this →

🟡 Gap: Rust ::default() still not excluded from scanConstructorBinding

Location: gitnexus/src/core/ingestion/type-extractors/rust.ts:176

First identified in the 8th review, still present:

if (func.type === 'scoped_identifier') {
  const methodName = func.lastNamedChild;
  if (methodName?.text === 'new') return undefined;  // ← 'default' not excluded
}

For let user = User::default(), the scanner produces { calleeName: 'default' }. In verifyConstructorBindings, ctx.resolve('default', filePath) returns many candidates (Rust's Default trait is widely implemented) → callableDefs.length > 1 → no binding. This is a harmless no-op (no false binding is created), but it wastes a cross-file lookup for every ::default() call. extractInitializer at rust.ts:125 already excludes default correctly — the scanner should match.

The one-liner fix: change methodName?.text === 'new' to methodName?.text === 'new' || methodName?.text === 'default'.

Fix this →

🟡 Gap: Kotlin return type inference — root cause confirmed as 2-segment import resolution

Location: gitnexus/src/core/ingestion/resolvers/jvm.ts:82

The known Kotlin gap (lenient test at kotlin.test.ts:490-513) has a confirmed root cause:

const segments = importPath.split('.');
if (segments.length < 3) return null;  // ← drops 2-segment imports

The fixture's App.kt uses import models.getUser (2 segments: ['models', 'getUser']). This returns null, so getUser is never linked from models/User.kt into services/App.kt's import graph. When scanKotlinConstructorBinding produces { varName: 'user', calleeName: 'getUser' } and verifyConstructorBindings calls ctx.resolve('getUser', 'services/App.kt'), the function may not be found as a cross-file candidate — no binding is produced, and user.save() is not disambiguated.

The guard segments.length < 3 was designed to detect member imports like com.example.Constants.VALUE (3+ segments where the last is a member name). Short package imports like models.getUser or io.getUser — 2-segment Kotlin top-level function imports — are incorrectly dropped. This is a pre-existing gap documented in the PR's deferred items.


What the Latest Commit (ec4dca4) Correctly Added

The C# null-conditional implementation is structurally sound:

  • Tree-sitter query at tree-sitter-queries.ts:370-373 captures invocation_expression > conditional_access_expression > member_binding_expression > identifier @call.name
  • MEMBER_ACCESS_NODE_TYPES now includes 'member_binding_expression' so the name node is recognized
  • extractReceiverName at utils.ts:867-873 walks from member_binding_expressionconditional_access_expressionfirstNamedChild to get the receiver identifier
  • The fixture csharp-null-conditional/App.cs uses User user = new User() (explicit type, Tier 0), so the test correctly validates call resolution via type-env (not scanner), which is the appropriate path for this pattern

Accuracy Tracking (All 9 Passes)

Review Findings Accuracy
1st TS/JS no scanner; Ruby extractVarName; 4 fixtures untested ❌ All wrong
2nd C# member_access_expression; PHP member_call_expression missing ❌ Both wrong
3rd Ruby false positive; Go selector_expression; Ruby @return missing ✅ All correct — fixed in 36ca3a3
4th normalizeJsDocType strips generics; PHP attribute walker; PHP $this receiver ✅ All correct — fixed in fda19a9
5th sanitizeReturnType dot corruption; async test false positives; Ruby constant LHS ✅ All correct — fixed in 9fbbe2d
6th Sync JSDoc false positive; Ruby factory false positive; nested wrapper generics ✅ All correct — fixed in 6465b8d
7th Kotlin disambiguation unproven; Swift navigation_suffix; Rust lifetime wrapper ✅ All correct — fixed in 5caaae9
8th Kotlin extractMethodSignature gap (unconfirmed); Java receiver disambiguation; Rust ::default() ✅ Rust ::default() confirmed unfixed; Java is architectural; Kotlin root cause confirmed as import resolution
9th (this) lbug-adapter.ts batch escapeValue missing \n/\r; Rust ::default() still unfixed; Kotlin import resolution confirmed

Resolution Matrix (Post-ec4dca4)

Pattern TS JS Python Java Kotlin Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a n/a
var = new Class() / Class.new
var = fn() return inference ✅ (var) ⚠️ import gap ⚠️ skipped
var, err = fn() multi-return n/a n/a n/a n/a n/a n/a n/a n/a n/a n/a
var?.method() null-conditional n/a n/a n/a n/a n/a n/a n/a new n/a n/a n/a
var = pkg.fn() qualified n/a ⚠️ skipped
$this->fn() receiver hint n/a n/a n/a n/a n/a n/a n/a n/a n/a n/a
Doc-comment return type JSDoc ✅ JSDoc ✅ n/a n/a n/a n/a n/a PHPDoc ✅ PHPDoc ✅ YARD ✅ n/a
@param [Hash<Symbol, T>] YARD generics n/a n/a n/a n/a n/a n/a n/a n/a n/a new n/a
@param [Type] name alternate YARD n/a n/a n/a n/a n/a n/a n/a n/a n/a new n/a
@param $name Type alternate PHPDoc n/a n/a n/a n/a n/a n/a n/a n/a new n/a n/a
Nested wrapper W<R<T,E>> n/a n/a n/a n/a
Rust Ref<'_, T> lifetime wrappers n/a n/a n/a n/a n/a n/a n/a n/a n/a n/a

Recommended Next Steps (Priority Order)

  1. Fix lbug-adapter.ts batch escapeValue — Add .replace(/\n/g, '\\n').replace(/\r/g, '\\r') to the escapeValue at line 429. Same fix as line 368, missed instance. High correctness impact for any codebase with multiline file content in the graph.

  2. Fix Rust ::default() exclusion — One-liner at rust.ts:176. No test needed; the existing behavior is a no-op (no false binding), but wastes a lookup.

  3. Fix Kotlin 2-segment import resolution — Relax the segments.length < 3 guard in resolvers/jvm.ts:82 to handle package.memberName patterns (2 segments, lowercase last segment). Once fixed, update the kotlin-return-type test from lenient assertions to strict expect(userSave.targetFilePath).toContain('User.kt') assertions.

  4. Swift infrastructure — All 11 tests remain skipped due to tree-sitter-swift build failure on Node 22. Infrastructure issue beyond PR scope.


| | Branch

Verifies that classes using only @staticmethod/@classmethod have HAS_METHOD
edges connecting them to their child methods. This was the root cause of
issue #289 where context() and impact() returned empty for such classes.

Tests cover: HAS_METHOD edge emission, unique static method resolution
(create_user, delete_user), and ambiguous same-named method handling
(find_user on both UserService and AdminService — safely refused).
…er exclusion

- Apply \n/\r escaping to batch upsert escapeValue in lbug-adapter.ts:429
  (missed instance of the CREATE-path fix from ec4dca4)
- Exclude Rust ::default() from scanConstructorBinding to match
  extractInitializer behavior — avoids wasted cross-file lookups on
  the broadly-implemented Default trait
- Unit tests: 2 new scanner exclusion tests (::default and ::new)
- Integration tests: 6 new Rust ::default() constructor resolution tests
  with disambiguation fixture (User::default vs Repo::default)
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 5m 16s —— View job


Code Review: Constructor Type Resolution — Tenth Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read latest source files (post-1daebc0 commit)
  • Verify 9th review fixes are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Summarize what now works, what's still broken, and next steps

Verification of Ninth Review Fixes (Commits 1daebc0, bd30951)

All items from the 9th review are confirmed addressed:

Claimed Fix Verified
lbug-adapter.ts batch escapeValue missing \n/\r ✅ Fixed: lbug-adapter.ts:429 now has .replace(/\n/g, '\\n').replace(/\r/g, '\\r') alongside the existing quote escaping
Rust ::default() not excluded from scanConstructorBinding ✅ Fixed: rust.ts:176 now checks methodName?.text === 'new' || methodName?.text === 'default' matching extractInitializer's behavior
Kotlin 2-segment import resolution — import models.getUser dropped Acknowledged as known gap: jvm.ts:82 still guards segments.length < 3. Lenient test at kotlin.test.ts:490-513 correctly documents this.

New Issues Found

🔴 Bug: C# scanConstructorBinding doesn't unwrap await_expression — async binding missed

Location: gitnexus/src/core/ingestion/type-extractors/csharp.ts:134

if (value.type !== 'invocation_expression') return undefined;

For var user = await svc.GetUserAsync("alice"), tree-sitter-c-sharp parses the RHS as await_expression { invocation_expression }. The scanner checks value.type !== 'invocation_expression' but receives await_expression → returns undefined. No binding is emitted for the variable.

The TypeScript scanner (typescript.ts:89–92) handles this correctly with explicit await unwrapping:

let value = declarator.childForFieldName('value');
if (value?.type === 'await_expression') {
  value = value.firstNamedChild;
}

C# async/await is idiomatic — var user = await repo.GetByIdAsync(id) is the standard pattern in ASP.NET. Any C# codebase using async repository or service patterns will silently miss all constructor bindings from async calls.

Fix: Add the same await_expression unwrap used in typescript.ts:89–92 to csharp.ts:124–131, between where value is extracted from equals_value_clause and where value.type is checked.

Fix this →

🟡 Bug: Rust scanConstructorBinding doesn't unwrap .await — async binding missed

Location: gitnexus/src/core/ingestion/type-extractors/rust.ts:171

const value = node.childForFieldName('value');
if (!value || value.type !== 'call_expression') return undefined;

In Rust, let user = get_user_async().await parses as a let_declaration with value being an await_expression (.await is postfix in Rust), not a call_expression. The check value.type !== 'call_expression' returns undefined. No binding is produced.

let user = get_user().await is idiomatic Rust async code inside async fn blocks. This gap means all Rust async bindings are silently dropped by the scanner.

Fix: After extracting value from node.childForFieldName('value'), add unwrapping for await_expression: if (value?.type === 'await_expression') { value = value.firstNamedChild; }. Use unwrapAwait from shared.ts:240 which already exists for exactly this purpose.

Fix this →

🟡 Gap: PHP backslash-namespaced return types fail extractReturnTypeName

Location: gitnexus/src/core/ingestion/call-processor.ts:528–537 and gitnexus/src/core/ingestion/type-extractors/php.ts:57–65

When extractMethodSignature (via the generic loop at utils.ts:661–669) captures a PHP typed function's return type, it stores the raw text from the AST. For function getUser(): \App\Models\User, the return_type child's text is \App\Models\User. This is then passed to extractReturnTypeName.

extractReturnTypeName handles :: and . separators:

if (text.includes('::') || text.includes('.')) {
  text = text.split(/::|\./).pop()!;
}

But PHP uses \ as its namespace separator. \App\Models\User doesn't match :: or ., so text remains \App\Models\User. The final regex check /^[A-Z_]\w*$/ fails (backslash is not \w) and extractReturnTypeName returns undefined.

normalizePhpType in php.ts:57–65 correctly handles this with type.split('\\'), but that function is only called in extractReturnType (PHPDoc path). The AST-captured typed PHP returns never go through normalizePhpType.

In practice, PHP 7+ codebases using strict type declarations (function getUser(): \Models\User) with fully-qualified namespaces will silently fail return type inference. Code that uses use imports (most modern PHP) writes function getUser(): User directly, which works. This affects legacy or strictly-namespaced PHP code.

Fix this →

🟢 Observation: fallbackRelationshipInserts inline Cypher templates lack \n/\r escaping

Location: gitnexus/src/core/lbug/lbug-adapter.ts:307–311

The fallbackRelationshipInserts function (error-recovery path for failed batch COPY operations) uses inline template literals:

await conn.query(`
  MATCH (a:... {id: '${fromId.replace(/'/g, "''")}' }),
        (b:... {id: '${toId.replace(/'/g, "''")}' })
  CREATE (a)-[:CALLS {type: '${relType}', ... reason: '${reason.replace(/'/g, "''")}', ...}]->(b)
`);

These escape ''' but not \n/\r. The main escapeValue in batchInsertNodesToLbug was hardened in commit 1daebc0, but this separate fallback path was missed. In practice, fromId and toId are node identifiers (unlikely to contain newlines), and reason is a short descriptor. The risk is low since this is the error-recovery fallback, but it's the same injection vector that was just fixed in the batch path.


What Has Improved Since PR Opening (Accurate Summary)

After commits through 1daebc0, the complete resolution matrix is:

Pattern TS/JS Python Java Kotlin Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a
var = new Class() / Class.new
var = fn() return inference ✅ (var) ⚠️ import gap ⚠️ skipped
var = pkg.fn() qualified n/a ⚠️ skipped
var = await fn() async n/a n/a n/a n/a ❌ new ❌ new n/a n/a n/a
var, err = fn() multi-return n/a n/a n/a n/a n/a n/a n/a n/a n/a
var?.method() null-conditional n/a n/a n/a n/a n/a n/a n/a n/a n/a
$this->fn() receiver hint n/a n/a n/a n/a n/a n/a n/a n/a n/a
Doc-comment return type JSDoc ✅ n/a n/a n/a n/a n/a PHPDoc ✅ PHPDoc ✅ YARD ✅ n/a
Nested wrapper W<R<T,E>> n/a n/a
Rust Ref<'_, T> lifetime wrappers n/a n/a n/a n/a n/a n/a n/a n/a n/a

Accuracy Tracking (All 10 Passes)

Review Findings Accuracy
1st TS/JS no scanner; Ruby extractVarName gaps; 4 fixtures untested ❌ All wrong
2nd C# member_access_expression; PHP member_call_expression ❌ Both wrong
3rd Ruby false positive; Go selector_expression; Ruby @return missing ✅ All correct — fixed in 36ca3a3
4th normalizeJsDocType strips generics; PHP attribute walker; PHP $this receiver ✅ All correct — fixed in fda19a9
5th sanitizeReturnType dot corruption; async test false positives; Ruby constant LHS ✅ All correct — fixed in 9fbbe2d
6th Sync JSDoc false positive; Ruby factory false positive; nested wrapper generics ✅ All correct — fixed in 6465b8d
7th Kotlin disambiguation unproven; Swift navigation_suffix; Rust lifetime wrapper ✅ All correct — fixed in 5caaae9
8th Kotlin extractMethodSignature (unconfirmed); Java receiver disambiguation; Rust ::default() ✅ Rust ::default() fixed in 1daebc0; Kotlin root cause confirmed as import resolution
9th lbug-adapter.ts batch escapeValue missing; Rust ::default(); Kotlin import resolution escapeValue and ::default() fixed in 1daebc0
10th (this) C# await not unwrapped; Rust .await not unwrapped; PHP \-separated namespace types

Recommended Next Steps (Priority Order)

  1. Fix C# async await unwrapping — Add if (value?.type === 'await_expression') { value = value.firstNamedChild; } to csharp.ts:131–134, matching the TS scanner pattern. Add a C# async fixture with two classes sharing an async method to prove disambiguation works.

  2. Fix Rust async .await unwrapping — Use the existing unwrapAwait from shared.ts:240 (or replicate the two-line pattern) in rust.ts:171. Add a Rust async fixture with Future<User> return types.

  3. Fix PHP backslash namespace in extractReturnTypeName — Add \\ to the namespace separator handling at call-processor.ts:529. Extend the condition to text.includes('::') || text.includes('.') || text.includes('\\').

  4. Fix Kotlin 2-segment import resolution — Relax jvm.ts:82 guard from segments.length < 3 to also handle 2-segment Kotlin top-level function imports (models.getUser). Once fixed, update the lenient kotlin.test.ts:490 assertions to strict positive assertions.

  5. Swift infrastructure — All 11 tests remain skipped due to tree-sitter-swift build failure on Node 22. Infrastructure issue.


| Branch

@magyargergo

Copy link
Copy Markdown
Collaborator Author

10th Pass — All 4 Findings Fixed

# Finding Fix
1 C# scanConstructorBinding misses await_expression Added await_expression to value extraction loop + unwrapAwait in csharp.ts
2 Rust scanConstructorBinding misses .await Applied unwrapAwait from shared.ts in rust.ts
3 PHP \-namespaced return types fail extractReturnTypeName Extended namespace separator handling in call-processor.ts to include \
4 fallbackRelationshipInserts missing \n/\r escaping Added inline esc helper with full escaping in lbug-adapter.ts

Tests added:

  • 2 unit tests for Rust .await and C# await unwrapping in type-env.test.ts
  • 3 unit tests for PHP backslash namespace in call-processor.test.ts
  • 3 C# async integration tests with new csharp-async-binding fixture
  • 4 Rust async integration tests with new rust-async-binding fixture

All passing: 1349 unit tests, 61 Rust integration, 55 C# integration, 74 PHP integration.

…caping

- C# scanConstructorBinding: unwrap await_expression to find invocation_expression
  (var user = await svc.GetUserAsync() now produces constructor binding)
- Rust scanConstructorBinding: unwrap .await postfix via shared unwrapAwait helper
  (let user = get_user().await now produces constructor binding)
- extractReturnTypeName: handle PHP backslash namespace separator (\App\Models\User → User)
- fallbackRelationshipInserts: match batch escapeValue hardening with \n/\r escaping

Tests: 2 unit (type-env), 3 unit (call-processor), 7 integration (csharp+rust), 7 fixtures
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 4m 42s —— View job


Claude finished @magyargergo's task ——


Code Review: Constructor Type Resolution — Eleventh Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read latest source files (post-c3b635a commit)
  • Verify 10th review fixes are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Post final analysis

Verification of Tenth Review Fixes (Commit c3b635a)

All four fixes from the tenth review are confirmed correctly implemented:

Claimed Fix Verified
C# scanConstructorBinding missing await_expression unwrap csharp.ts:133-135: value = unwrapAwait(value) is called before the invocation_expression type check
Rust scanConstructorBinding missing .await unwrap rust.ts:171: unwrapAwait(node.childForFieldName('value')) replaces the bare field access
PHP \-separated namespace types fail extractReturnTypeName call-processor.ts:529: text.includes('\\') added to condition; [.\\] in the split regex
fallbackRelationshipInserts missing \n/\r escaping lbug-adapter.ts:307: esc helper added with .replace(/\n/g, '\\n').replace(/\r/g, '\\r')

New Issues Found

🔴 Bug: csharp-async-binding test is a false positive — Save is unique, both services return same type

Location: gitnexus/test/fixtures/lang-resolution/csharp-async-binding/OrderService.cs:5

Both UserService.GetUserAsync and OrderService.GetUserAsync return Task<User> (not Task<User> and Task<Order> as one might expect). Looking at OrderService.cs:

public async Task<User> GetUserAsync(string name)  // ← returns User, NOT Order

There is only one Save() method in the fixture — User.Save() in User.cs. There is no competing Order.Save(). As a result:

  1. ctx.resolve('GetUserAsync', filePath) returns 2 candidates (one from each service), so callableDefs.length === 2verifyConstructorBindings produces no binding (the callableDefs.length === 1 guard at call-processor.ts:105 fails).
  2. Save is the only method with that name, so it resolves via fuzzy matching regardless of whether the binding was produced.
  3. The test at line 689 (resolves user.Save() after await to User#Save) passes without the await unwrapping doing any work.

The test title says "competing Save methods" but there's exactly one. This is the same false-positive pattern found and fixed in the 5th/6th reviews for the JS sync and Ruby fixture tests.

A correct test would need OrderService.GetUserAsync to return Task<Order>, an Order.cs file with its own Order.Save(), and a negative assertion ensuring user.Save() does NOT resolve to Order.Save().

Fix this →

🔴 Bug: rust-async-binding test is a false positive — both save methods in the same file

Location: gitnexus/test/fixtures/lang-resolution/rust-async-binding/src/models.rs

Both User::save and Repo::save are defined in the same models.rs file. The test checks:

const saveCall = calls.find(c =>
  c.target === 'save' && c.source === 'process_user' && c.targetFilePath.includes('models'),
);
expect(saveCall).toBeDefined();

Since both structs live in models.rs, targetFilePath.includes('models') is true for both — the assertion cannot distinguish whether the correct struct's save was hit. There are no negative assertions confirming process_user does NOT resolve to Repo::save.

Compare to the rust-return-type-inference fixture (correctly split into user.rs/repo.rs from the 6th review fix) — this new fixture did not apply the same lesson.

Fix this →

🟡 Gap: C# scanConstructorBinding discards receiver for non-this calls — multi-service disambiguation impossible

Location: gitnexus/src/core/ingestion/type-extractors/csharp.ts:139-143

For var user = await userSvc.GetUserAsync("alice"), after unwrapAwait the scanner sees invocation_expression with func = member_access_expression { userSvc . GetUserAsync }. extractSimpleTypeName(func) returns only the method name 'GetUserAsync' — the receiver userSvc is discarded.

When two classes share GetUserAsync, callableDefs.length > 1 → no binding. PHP handles this via receiverClassName when receiver is $this/self. For C#, when the receiver is an arbitrary typed variable (not this), the receiver's type would require a TypeEnv lookup — a two-pass problem that can't be solved at scanner level.

This means var user = await serviceA.GetUser() and var order = await serviceB.GetUser() can never be disambiguated via return type inference when GetUser exists on multiple classes. This is the dominant async DI pattern in ASP.NET. It's an architectural gap, shared with Java (documented in jvm.ts:85), but newly relevant for the C# async path.

🟡 Remaining Gap: Kotlin return type inference — extractMethodSignature likely misses Kotlin return types

Location: gitnexus/src/core/ingestion/utils.ts:662-669

This was flagged in the 8th review. The Kotlin test at kotlin.test.ts:484 now uses a lenient assertion acknowledging the gap. Root cause: extractMethodSignature looks for type_annotation or return_type children, but tree-sitter-kotlin places the fun getUser(): User return type differently than TypeScript. The extractKotlinDeclaration extractor explicitly looks for user_type nodes — suggesting Kotlin's AST differs from what extractMethodSignature expects.

Until extractMethodSignature has a Kotlin-specific return type extraction case, SymbolDefinition.returnType will be undefined for all Kotlin functions, and the lenient test will remain lenient forever.

🟢 Observation: Go scanConstructorBinding multi-return second-var list is narrower than the full idiom

Location: gitnexus/src/core/ingestion/type-extractors/go.ts:155-160

The multi-return handler checks secondVar.text === '_' || err || ok || error. Go also uses:

  • user, cancel := ctx.WithCancel(...)cancel is not a discard name
  • user, closer := ... — similar pattern
  • Any two-return factory function where the second isn't an error

These are dropped silently. This is a narrow gap compared to the main user, err := fn() pattern that is now handled, but represents real-world Go code using context cancellation or cleanup functions alongside user objects.


What the PR Has Achieved (Accurate Summary After 10 Review Passes)

The codebase has improved dramatically across 10 iterations. The core type resolution pipeline now works correctly for:

Pattern Status
var: T = ... explicit annotations ✅ All 11 languages
var = new Class() constructor inference ✅ All 11 languages
var = fn() return type inference ✅ 9/11 (Kotlin known gap, Swift skipped)
var = await fn() / let x = fn().await ✅ C# + Rust (new in c3b635a)
var, err := fn() Go multi-return ✅ New in 6465b8d
@param / @return doc-comment types ✅ JSDoc, PHPDoc, YARD
Nested wrapper generics W<R<T,E>> ✅ Balanced-bracket extraction
Rust lifetime wrappers Ref<'_, T> ✅ Lifetime-skipping extractor
PHP \App\Models\User backslash namespaces ✅ New in c3b635a
C# user?.Save() null-conditional ✅ New in ec4dca4

Accuracy Tracking (All 11 Passes)

Review Findings Accuracy
1st TS/JS no scanner; Ruby gaps; 4 fixtures untested ❌ All wrong
2nd C# member_access_expression; PHP member_call_expression ❌ Both wrong
3rd Ruby false positive; Go selector_expression; Ruby @return ✅ All correct — fixed
4th normalizeJsDocType strips generics; PHP attribute walker; PHP $this ✅ All correct — fixed
5th sanitizeReturnType dot corruption; async test false positives; Ruby constant LHS ✅ All correct — fixed
6th Sync JSDoc false positive; Ruby factory false positive; nested wrapper generics ✅ All correct — fixed
7th Kotlin disambiguation unproven; Swift navigation_suffix; Rust lifetime wrapper ✅ All correct — fixed
8th Kotlin extractMethodSignature gap; Java receiver disambiguation; Rust ::default() ✅ Rust ::default() fixed; others confirmed
9th lbug-adapter.ts batch escapeValue missing; Rust ::default(); Kotlin root cause ✅ All correct — fixed
10th C# await unwrap; Rust .await unwrap; PHP \ namespace; fallback escaping ✅ All correct — fixed in c3b635a
11th (this) C# async test false positive; Rust async test false positive; Kotlin extractMethodSignature gap remains

Recommended Next Steps (Priority Order)

  1. Fix csharp-async-binding fixture and test — Change OrderService.GetUserAsync to return Task<Order>, add Order.cs with Order.Save(), add a negative assertion. This is the same one-time fix applied to JS and Ruby fixtures in passes 5–6.

  2. Fix rust-async-binding fixture and test — Split models.rs into user.rs/repo.rs, add negative assertions. Direct copy of the rust-return-type-inference fix from pass 6.

  3. Investigate Kotlin extractMethodSignature — Check tree-sitter-kotlin's AST for fun getUser(): User to confirm whether the return type appears as a type_annotation child or directly as user_type. If the latter, add a Kotlin-specific extraction case in utils.ts (like the existing Go and Rust cases at lines 619-643). Once fixed, the lenient test becomes strict.

  4. C# receiver disambiguation (architectural) — For var user = await serviceA.GetUser() where multiple classes share GetUser, the scanner currently discards the receiver. A targeted improvement: when the receiver is a typed variable already in the local TypeEnv, use its type to qualify the callee lookup — analogous to PHP's receiverClassName for $this. Requires a two-pass approach within the scanner or a post-scanner narrowing step.


| Branch

@magyargergo

Copy link
Copy Markdown
Collaborator Author

Eleventh Pass Response — Both False-Positive Tests Fixed

Findings evaluation:

Finding Verdict Action
🔴 C# csharp-async-binding false positive Confirmed — only one Save(), both services returned Task<User> Fixed
🔴 Rust rust-async-binding false positive Confirmed — both structs in same models.rs, no negative assertions Fixed
🟡 C# receiver disambiguation gap Architectural limitation matching Java's known gap in jvm.ts:85 Acknowledged, out of scope
🟡 Kotlin extractMethodSignature gap Known, lenient test exists Out of scope for this pass
🟢 Go multi-return second-var narrowness Minor edge case (cancel, closer) Not actionable now

C# async-binding fix:

  • Added Order.cs with Order.Save() method
  • Changed OrderService.GetUserAsyncGetOrderAsync returning Task<Order> (not Task<User>)
  • Added negative assertion: user.Save() does NOT resolve to Order#Save
  • All 56 C# integration tests pass

Rust async-binding fix:

  • Split models.rs into user.rs and repo.rs (matching rust-default-constructor fixture pattern)
  • Updated main.rs to mod user; mod repo; with async fn for process_user/process_repo
  • Added strict negative assertions: user.save() must NOT resolve to repo.rs and vice versa
  • All 62 Rust integration tests pass

Both fixes follow the same disambiguation pattern established in passes 5–6 for JS and Ruby fixtures: separate files per class/struct + explicit negative assertions proving cross-contamination doesn't occur.

… and negative assertions

C# fixture: add Order.cs with Order.Save(), change OrderService to return
Task<Order> via GetOrderAsync, add negative assertion proving user.Save()
does not resolve to Order#Save.

Rust fixture: split models.rs into user.rs/repo.rs, make process_user and
process_repo async fn, add bidirectional negative assertions proving no
cross-contamination between User#save and Repo#save.
@magyargergo

Copy link
Copy Markdown
Collaborator Author

@claude try to find uncovered or suspicious places in the code that doesn't/won't work for constructor's type resolution. Also review previous comments and review what's changed and how it improved the type resolution and what's the next steps are

@claude

claude Bot commented Mar 15, 2026

Copy link
Copy Markdown

Claude finished @magyargergo's task in 7m 55s —— View job


Code Review: Constructor Type Resolution — Twelfth Pass Analysis

Tasks:

  • Read trigger comment and understand request
  • Read latest source files (post-bb420d2 commit)
  • Verify 11th review fixes are correctly implemented
  • Find new uncovered gaps in constructor type resolution
  • Cross-reference all previous comment findings
  • Post final analysis

Verification of Eleventh Review Fixes (Commit bb420d2)

Both fixes from the 11th review are confirmed correctly applied:

Claimed Fix Verified
C# csharp-async-binding — both services returned Task<User>, only one Save() ✅ Fixed: OrderService.GetOrderAsync now returns Task<Order>, Order.cs has Order.Save(), Program.cs calls both user.Save() and order.Save() separately
Rust rust-async-binding — both structs in same models.rs, no negative assertions ✅ Fixed: split into user.rs/repo.rs, process_user/process_repo in main.rs, strict expect(wrongSave).toBeUndefined() at rust.test.ts:793

New Issues Found

🔴 Bug: C# async-binding negative assertion is logically broken

Location: gitnexus/test/integration/resolvers/csharp.test.ts:698–710

The 11th pass correctly fixed the fixture, but the negative assertion remains logically unsound:

it('user.Save() does NOT resolve to Order#Save', () => {
  const wrongSave = calls.find(c =>
    c.target === 'Save' && c.source === 'Main' && c.targetFilePath.includes('Order.cs'),
  );
  // If both are resolved, at least one must be from the correct file
  if (wrongSave) {
    const correctSave = calls.find(c =>
      c.target === 'Save' && c.source === 'Main' && c.targetFilePath.includes('User.cs'),
    );
    expect(correctSave).toBeDefined();
  }
});

order.Save() is also called from Main and correctly resolves to Order#Save (in Order.cs). So wrongSave is always found (the legitimate Main → Order#Save edge exists). The if (wrongSave) branch always executes and then just confirms correctSave also exists — which it always will. The test passes regardless of whether user.Save() was incorrectly resolved to both User#Save AND Order#Save.

Compare to the Rust fix which correctly uses strict expect(wrongSave).toBeUndefined() — that works because process_user and process_repo are separate source functions.

Fix: Replace Main with separate ProcessUser() and ProcessOrder() methods in the fixture, then check c.source === 'ProcessUser' to make the negative assertion meaningful — the same pattern the Rust test now uses.

Fix this →

🟡 Bug: extractReturnTypeName returns bare wrapper type names instead of undefined

Location: gitnexus/src/core/ingestion/call-processor.ts:494

The WRAPPER_GENERICS set (Task, Promise, Future, Option, Result, etc.) is only checked inside the genericMatch branch — when the type string contains <...>. For bare wrapper type names without a type argument, the function falls through to the uppercase identifier check and returns the wrapper name itself:

// genericMatch requires the pattern ^(\w+)\s*<(.+)>$
// Does NOT match bare 'Task', 'Promise', 'Option', etc.
const genericMatch = text.match(/^(\w+)\s*<(.+)>$/);
if (genericMatch) { /* WRAPPER_GENERICS checked here only */ }

// Falls through to:
if (!/^[A-Z_]\w*$/.test(text)) return undefined;
return text;  // ← 'Task', 'Promise', 'Option' all pass this check

Impact: For C# public Task GetAsync() (void-like, no generic argument), extractReturnTypeName('Task') returns 'Task'. The scanner creates a binding result → 'Task'. In verifyConstructorBindings, ctx.resolve('Task', filePath) is called — for most projects this resolves to nothing (system types not indexed), so the binding silently fails. However, if user code defines a class named Task, Option, or Result (legitimate in Rust/functional-style code), a false binding is created.

The same applies to Go function declarations returning error (caught by PRIMITIVE_TYPES), but not to Rust None, Kotlin Nothing, or any wrapper type used bare.

Fix: After the genericMatch block, add a check: if (WRAPPER_GENERICS.has(text)) return undefined; to prevent bare wrapper type names from creating bindings.

Fix this →

🟡 Gap: JSDoc optional @param {Type} [optionalName] not captured

Location: gitnexus/src/core/ingestion/type-extractors/typescript.ts:36

const JSDOC_PARAM_RE = /@param\s*\{([^}]+)\}\s+(\w+)/g;
//                                                ^^^^  only captures simple identifiers

The JSDoc convention for optional parameters uses brackets: @param {User} [user] or @param {User} [user=null]. The (\w+) pattern requires a plain word character sequence and will not match [user] or [user=null]. Any function with optional parameters documented via JSDoc will have those parameter type bindings silently dropped.

This means that in JavaScript codebases using JSDoc to document optional typed parameters, the type resolution for those parameters won't work even when @param {Type} [name] is correctly documented.

Fix: Update JSDOC_PARAM_RE to optionally match bracketed parameter names: replace (\w+) with \[?(\w+) (or a more complete pattern that handles default values like [name=default]).

Fix this →

🟢 Observation: Kotlin extractMethodSignature gap — still documented but unfixed

Location: gitnexus/src/core/ingestion/utils.ts:661–669 and gitnexus/test/integration/resolvers/kotlin.test.ts:490

The lenient Kotlin test documents a known gap: fun getUser(): User return types are not captured by extractMethodSignature because it only looks for type_annotation and return_type children, but Kotlin uses user_type directly. The extractKotlinDeclaration extractor (which correctly uses findChildByType(varDecl, 'user_type')) confirms this AST difference.

Until a Kotlin-specific extraction case is added to extractMethodSignature (analogous to the existing Go result field and Rust return_type field cases at lines 618–643), SymbolDefinition.returnType will remain undefined for all Kotlin functions, and the lenient test will never become strict.


Accuracy Tracking (All 12 Passes)

Review Findings Accuracy
1st TS/JS no scanner; Ruby extractVarName gaps; 4 fixtures untested ❌ All wrong
2nd C# member_access_expression; PHP member_call_expression ❌ Both wrong
3rd Ruby false positive; Go selector_expression; Ruby @return missing ✅ All correct — fixed in 36ca3a3
4th normalizeJsDocType strips generics; PHP attribute walker; PHP $this receiver ✅ All correct — fixed in fda19a9
5th sanitizeReturnType dot corruption; async test false positives; Ruby constant LHS ✅ All correct — fixed in 9fbbe2d
6th Sync JSDoc false positive; Ruby factory false positive; nested wrapper generics ✅ All correct — fixed in 6465b8d
7th Kotlin disambiguation unproven; Swift navigation_suffix; Rust lifetime wrapper ✅ All correct — fixed in 5caaae9
8th Kotlin extractMethodSignature; Java receiver disambiguation; Rust ::default() ✅ Rust fixed; Java architectural; Kotlin confirmed root cause
9th lbug-adapter.ts batch escapeValue; Rust ::default(); Kotlin import resolution ✅ All correct — fixed in 1daebc0
10th C# await unwrap; Rust .await unwrap; PHP \ namespace; fallback escaping ✅ All correct — fixed in c3b635a
11th C# async fixture false positive; Rust async fixture false positive; Kotlin extractMethodSignature ✅ C#/Rust fixtures fixed in bb420d2; Kotlin acknowledged
12th (this) C# negative assertion logically broken; bare wrapper type returns wrong value; JSDoc optional params

Final Resolution Matrix (Post-bb420d2)

Pattern TS/JS Python Java Kotlin Go Rust C# PHP Ruby Swift
var: T = ... explicit n/a
var = new Class() / Class.new
var = fn() return inference ✅ (var) ⚠️ known gap ⚠️ skipped
var = await fn() / .await n/a n/a n/a n/a n/a n/a n/a
var, err := fn() multi-return n/a n/a n/a n/a n/a n/a n/a n/a n/a
var?.method() null-conditional n/a n/a n/a n/a n/a n/a n/a n/a n/a
$this->fn() receiver hint n/a n/a n/a n/a n/a n/a n/a n/a n/a
Doc-comment return type JSDoc ✅ n/a n/a n/a n/a n/a PHPDoc ✅ PHPDoc ✅ YARD ✅ n/a
Bare wrapper type names (Task, Promise) ⚠️ returns name ⚠️ returns name ⚠️ returns name
JSDoc @param {Type} [optionalName] ❌ not captured

Recommended Next Steps (Priority Order)

  1. Fix C# async-binding negative assertion — Split Main into ProcessUser()/ProcessOrder() in the fixture. This enables a strict expect(wrongSave).toBeUndefined() on c.source === 'ProcessUser', proving true disambiguation rather than just "both edges exist."

  2. Fix bare wrapper type in extractReturnTypeName — Add if (WRAPPER_GENERICS.has(text)) return undefined; after the generic match block. Prevents extractReturnTypeName('Task') from returning 'Task', which creates a useless binding. Low risk, high correctness.

  3. Fix JSDoc optional parameter regex — Change (\w+) to match [optionalName] form in JSDOC_PARAM_RE. Common in real-world JSDoc APIs.

  4. Kotlin extractMethodSignature return type — Add a Kotlin-specific extraction for function return type (user_type child of function_declaration) analogous to the Go result field and Rust return_type field cases. Once fixed, update the lenient kotlin.test.ts:490 assertions to strict disambiguation checks.

  5. Swift infrastructure — All 11 Swift tests remain skipped due to tree-sitter-swift build failure on Node 22. Infrastructure issue beyond PR scope.


| Branch |

… optional params

- Split Program.cs Main into ProcessUser/ProcessOrder so negative
  assertions use strict toBeUndefined() (matching Rust pattern)
- Guard bare wrapper types (Task, Promise, Option…) in
  extractReturnTypeName — return undefined instead of the wrapper name
- Update JSDOC_PARAM_RE to capture @param {Type} [optionalName] syntax
@magyargergo magyargergo merged commit 6c18ae0 into main Mar 15, 2026
18 of 19 checks passed
@magyargergo magyargergo deleted the feat/type-resolution-phase3 branch March 15, 2026 18:49
motolese pushed a commit to motolese/datamoto-gitnexus that referenced this pull request Apr 23, 2026
…pe extractors (abhigyanpatwari#284)

* feat: Phase 3 — return type inference, generic args extraction, Ruby YARD type extractor

Three architectural improvements to the type resolution system:

1. Return type inference — wire extractMethodSignature returnType through
   SymbolDefinition into call-processor. When var = callee() and callee
   has a known return type, bind var to that type. Handles Promise<T>
   unwrapping, nullable stripping, pointer/reference removal.

2. Generic type argument extraction — new extractGenericTypeArgs() utility
   that extracts type parameters from List<User> → ['User']. Handles
   TS/Java/Kotlin/C#/Rust generic syntax. Building block for for-loop
   variable typing.

3. Ruby dedicated type extractor — replaces the stub with YARD annotation
   parsing (@param name [Type]), handling qualified types, nullable types,
   and singleton methods. Ruby now has real type resolution.

Unit tests: 127 → 192+ (type-env) + 65 (symbol-table, call-processor) + 18 (generics)
Integration tests: 8+ new test cases with fixtures across TS/Python/Go/Java/Ruby

* fix: Phase 3 gaps — WRAPPER_GENERICS correctness, Ruby :: qualifier, namespaced constructors

- Remove collection types (List, Array, Vec, Set) from WRAPPER_GENERICS to prevent
  false CALLS edges (e.g. List<User> no longer unwraps to User)
- Add :: qualifier handling in extractReturnTypeName for Ruby/C++/Rust namespaced types
- Add Ruby `constant` and `scope_resolution` node types to shared extractors
- Extract shared extractRubyConstructorAssignment helper (dedup type-env.ts + ruby.ts)
- Add integration tests for return type inference: Python, TypeScript, Go, Java, Ruby
- Add Ruby namespaced constructor fixture (Models::UserService.new)
- Add unit tests for collection reclassification and :: qualifiers

* feat: Phase 4 — CONSTRUCTOR_BINDING_SCANNERS for all languages + return type inference tests

Add CONSTRUCTOR_BINDING_SCANNERS for 6 missing languages, completing
return type inference coverage across all 11 supported languages:

- TypeScript/JS: variable_declarator with call_expression, unwraps await
- Go: short_var_declaration single-assignment (skips multi-return, new/make)
- Java: local_variable_declaration with `var` type + method_invocation
- C#: variable_declaration with implicit_type (var) + invocation_expression
- Rust: let_declaration without type annotation, handles mut_pattern
- PHP: assignment_expression with function_call_expression

Also adds property_identifier to extractSimpleTypeName for qualified
member calls (repo.getUser → getUser), fixing namespaced constructor
inference that was previously a known limitation.

Integration tests added for all 11 languages with correct label
assertions (Function vs Method per language's tree-sitter queries).

* refactor: merge CONSTRUCTOR_BINDING_SCANNERS into per-language LanguageTypeConfig

Eliminates the parallel dispatch map in type-env.ts by moving all 11
constructor binding scanners into their respective type-extractors/*.ts
files as `scanConstructorBinding` on LanguageTypeConfig.

- Add ConstructorBindingScanner type to types.ts
- Add shared helpers: hasTypeAnnotation, unwrapAwait, extractCalleeName
- Move scanners to typescript.ts, jvm.ts, python.ts, php.ts, go.ts,
  rust.ts, swift.ts, c-cpp.ts, csharp.ts, ruby.ts
- Fix `any` types in C# scanner → SyntaxNode | null
- Delete ~300 lines from type-env.ts (CONSTRUCTOR_BINDING_SCANNERS map)
- Update buildTypeEnv to use config.scanConstructorBinding

All 143 type-env unit tests and all 10 language integration suites pass.

* fix: remove unused import, fix any type in Java scanner, update stale comment

- Remove unused extractCalleeName import from jvm.ts
- Fix (c: any) → (c: SyntaxNode) in Java scanner
- Update stale CONSTRUCTOR_BINDING_SCANNERS reference in ruby.ts comment

* fix: C# and PHP return type inference — scanner fixes, method signature extraction, and cross-file resolution

Addresses code review findings on PR abhigyanpatwari#284:

C# scanner (csharp.ts):
- Fix type node lookup: iterate children instead of childForFieldName('type')
  which returns undefined in tree-sitter-c-sharp
- Fix initializer lookup: handle direct invocation_expression children
  (no equals_value_clause wrapper in tree-sitter-c-sharp)

C# return type extraction (utils.ts):
- Add 'returns' field check to extractMethodSignature — tree-sitter-c-sharp
  uses 'returns', not 'type', for method return types

C# cross-file resolution (call-processor.ts + fixture):
- Add constructor binding verification to sequential processCalls path
  (was only in the worker processCallsFromExtracted path)
- Add ReturnType.csproj to csharp-return-type fixture
- Update fixture namespaces to use ReturnType.Models/ReturnType.Services
  prefix (matches real C# project conventions)

PHP scanner (php.ts):
- Extend scanConstructorBinding to handle member_call_expression
  ($this->getUser() patterns), not just function_call_expression

Shared (shared.ts):
- Add member_access_expression to extractSimpleTypeName qualified-names
  block (C# method calls like svc.GetUser())

Tests:
- Add Repo.cs/Repo.php disambiguation fixtures (two Save methods)
- Strengthen C# and PHP return type tests with hard disambiguation assertions
- Add C# scanner unit tests and return type extraction test

* feat: per-language ReturnTypeExtractor + doc-comment @param parsing for PHP, JS, Ruby

Add ReturnTypeExtractor to LanguageTypeConfig interface with implementations
for Ruby (YARD @return), PHP (PHPDoc @return), and JS/TS (JSDoc @returns).
The fallback is wired in both parsing-processor and parse-worker paths,
activating only when extractMethodSignature finds no AST-based return type.

Also add doc-comment @param type extraction for PHP and JS/TS, following
Ruby's existing collectYardParams pattern. This enables parameter.method()
resolution in loosely-typed codebases using PHPDoc @param or JSDoc @param.

Additional fixes from PR abhigyanpatwari#284 code review:
- Go: add selector_expression + field_identifier to extractSimpleTypeName
  (enables package-qualified factory calls like models.NewUser())
- Ruby: broaden scanConstructorBinding to capture plain call assignments
  (user = get_user()) in addition to Class.new patterns
- Ruby: harden return-type fixture with disambiguation (two save methods)

Test coverage: +14 new integration tests across Go, Ruby, PHP, JS/TS

* fix: JSDoc async return type, PHP attribute walkers, and $this receiver disambiguation

Three fixes from fourth-pass code review on PR abhigyanpatwari#284:

1. JSDoc `@returns {Promise<User>}` no longer stripped to `Promise` — extractReturnType
   now uses sanitizeReturnType (preserves generics) instead of normalizeJsDocType
   (which stripped them before extractReturnTypeName could unwrap WRAPPER_GENERICS).

2. PHP 8+ `#[Attribute]` and JS `@decorator` nodes no longer break doc-comment walkers.
   Both extractReturnType and collect*Params functions now skip attribute_list/decorator
   nodes instead of breaking on them as named siblings.

3. PHP `$this->method()` now provides receiverClassName for disambiguation.
   When two classes define the same method, the enclosing class narrows candidates
   via ownerId matching in call-processor, preventing false no-binding results.

* fix: sanitizeReturnType dot corruption, JS test assertions, Ruby constant receiver

- Remove redundant dot-path stripping from sanitizeReturnType that corrupted
  qualified names inside generics (e.g. Promise<models.User> → User>)
- Split JS async fixture into separate files and add negative assertions
  to properly verify disambiguation (mirroring PHP test pattern)
- Accept 'constant' node type in Ruby scanConstructorBinding for factory
  call assignments (SERVICE = build_service())
- Add 'constant' to SIMPLE_RECEIVER_TYPES so extractReceiverName handles
  Ruby constant receivers (SERVICE.process)

* fix: nested generic arg splitting, JS/Ruby test false positives

- Replace naive comma split in extractReturnTypeName with bracket-balanced
  extractFirstGenericArg so nested types like Future<Result<User, Error>>
  unwrap correctly instead of producing malformed "Result<User"
- Add CompletableFuture to WRAPPER_GENERICS for Java async unwrapping
- Split js-jsdoc-return-type fixture models.js into user.js/repo.js and
  add negative assertions to prove disambiguation (not just file match)
- Split ruby-constant-factory-call fixture into separate service files
  and add negative assertions against AdminService resolution

* fix: review findings — receiverClassName parity, Rust wrappers, Go multi-return, Kotlin/Swift qualified calls

P1: Sequential path now includes receiverClassName narrowing for PHP
$this->method() disambiguation (was missing vs worker path).

P2: Added Rc/Arc/Weak/MutexGuard/Cow + 6 more Rust Deref types to
WRAPPER_GENERICS (Box excluded — Java Swing collision). Extended
Kotlin/Swift scanners to handle navigation_expression callees.
Added Go multi-return support (user, err := f()) with blank/_/err/ok
guard + AST-level first-return extraction in extractMethodSignature.

P3: Extracted shared verifyConstructorBindings() eliminating 60 lines
of duplication between sequential and worker paths. Added return-type
inference integration tests for C++, Rust, Swift with competing
methods and negative disambiguation assertions.

* fix: Swift navigation_suffix unwrapping, Rust lifetime skipping, Kotlin disambiguation tests

- Swift scanConstructorBinding: handle tree-sitter wrapping qualified
  identifiers in navigation_suffix nodes
- Add extractFirstTypeArg to skip Rust lifetime parameters ('a, '_)
  when unwrapping wrapper generics like Ref<'_, User>
- Kotlin tests: add Repo class fixture with competing save() methods
  to prove disambiguation; assert no spurious edges on known gap
- Remove tree-sitter-kotlin from optionalDependencies (now regular dep)

* fix: C# null-conditional calls, Ruby YARD bracket-balanced split, PHPDoc alternate order, escapeValue hardening

- Add C# null-conditional call support (user?.Save()): tree-sitter query for
  conditional_access_expression, member_binding_expression in MEMBER_ACCESS_NODE_TYPES,
  receiver extraction via conditional_access_expression parent walk
- Fix Ruby YARD type parsing for nested generics (Hash<Symbol, User>): replace
  naive split(',') with bracket-balanced splitter respecting <> depth
- Add alternate YARD format (@param [Type] name) alongside standard (@param name [Type])
- Add alternate PHPDoc format (@param $name Type) alongside standard (@param Type $name)
- Harden escapeValue in kuzu-adapter.ts: escape \n and \r to prevent Cypher injection
- Integration tests: C# null-conditional fixture (5 tests), Ruby YARD generics fixture (6 tests)
- Unit tests: PHPDoc alternate order (2 tests), C# null-conditional call-form (updated)

* test: add Python static/classmethod integration tests (issue abhigyanpatwari#289)

Verifies that classes using only @staticmethod/@classmethod have HAS_METHOD
edges connecting them to their child methods. This was the root cause of
issue abhigyanpatwari#289 where context() and impact() returned empty for such classes.

Tests cover: HAS_METHOD edge emission, unique static method resolution
(create_user, delete_user), and ambiguous same-named method handling
(find_user on both UserService and AdminService — safely refused).

* fix: lbug batch escapeValue newline hardening, Rust ::default() scanner exclusion

- Apply \n/\r escaping to batch upsert escapeValue in lbug-adapter.ts:429
  (missed instance of the CREATE-path fix from ec4dca4)
- Exclude Rust ::default() from scanConstructorBinding to match
  extractInitializer behavior — avoids wasted cross-file lookups on
  the broadly-implemented Default trait
- Unit tests: 2 new scanner exclusion tests (::default and ::new)
- Integration tests: 6 new Rust ::default() constructor resolution tests
  with disambiguation fixture (User::default vs Repo::default)

* fix: C#/Rust async await unwrap, PHP backslash namespace, fallback escaping

- C# scanConstructorBinding: unwrap await_expression to find invocation_expression
  (var user = await svc.GetUserAsync() now produces constructor binding)
- Rust scanConstructorBinding: unwrap .await postfix via shared unwrapAwait helper
  (let user = get_user().await now produces constructor binding)
- extractReturnTypeName: handle PHP backslash namespace separator (\App\Models\User → User)
- fallbackRelationshipInserts: match batch escapeValue hardening with \n/\r escaping

Tests: 2 unit (type-env), 3 unit (call-processor), 7 integration (csharp+rust), 7 fixtures

* fix: C#/Rust async-binding test false positives — add competing types and negative assertions

C# fixture: add Order.cs with Order.Save(), change OrderService to return
Task<Order> via GetOrderAsync, add negative assertion proving user.Save()
does not resolve to Order#Save.

Rust fixture: split models.rs into user.rs/repo.rs, make process_user and
process_repo async fn, add bidirectional negative assertions proving no
cross-contamination between User#save and Repo#save.

* fix: C# async-binding broken assertion, bare wrapper type leak, JSDoc optional params

- Split Program.cs Main into ProcessUser/ProcessOrder so negative
  assertions use strict toBeUndefined() (matching Rust pattern)
- Guard bare wrapper types (Task, Promise, Option…) in
  extractReturnTypeName — return undefined instead of the wrapper name
- Update JSDOC_PARAM_RE to capture @param {Type} [optionalName] syntax

* fix: update symbol and relationship counts in documentation
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.

1 participant