mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
bb68cc1eb0
* Initial plan * feat(SM-11): extract resolveMemberCall from resolveCallTarget - Create resolveMemberCall(ownerType, methodName, currentFile, ctx, heritageMap?) that uses owner-scoped + MRO resolution only (no fuzzy lookup) - resolveCallTarget delegates member calls (D0 path) to resolveMemberCall - walkMixedChain uses resolveMemberCall for owner-scoped member-call resolution - Add 7 unit tests for resolveMemberCall covering direct, inherited, MRO, null cases, and confidence tier assertions - Export resolveMemberCall for external use Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3b7889a9-5f2f-4572-8904-45084210f10d * fix(SM-11): address PR #744 review Blocking fixes: - B1: Revert unrelated package-lock.json gitnexus-shared addition - B2: Document confidence-tier semantic change on resolveMemberCall Performance / coupling fixes: - S1: walkMixedChain now calls resolveMethodByOwner directly (hot path) to avoid throwaway ResolveResult allocation per chain step - S2: Thread tier from resolveMethodByOwner via { def, tier } tuple; eliminates double ctx.resolve Alignment with semantic-model plan (Phase 3 target): - resolveMethodByOwner now iterates ALL class-like candidates from ctx.resolve, deduplicating matches by nodeId. Absorbs D4's ownerId-filtering into the owner-scoped path. - Handles homonym classes (two Users in different files) without falling through to D1-D4 fuzzy widening - Shared-ancestor MRO walks automatically dedup (both homonyms walk to same base method) - Unified direct-vs-MRO lookup under a single canWalkMRO check Tests added: - T1: Three D0 skip-condition tests via new _resolveCallTargetForTesting internal export (overloadHints, preComputedArgTypes, hasActiveModuleAlias) - T2: Rust qualified-syntax null test (trait-inherited method) + direct impl control - T3: C++ leftmost-base diamond inheritance test - B2 lock-in: cross-file class tier assertion - Homonym disambiguation: only-one-owns-method, both-own-method ambiguity, shared-ancestor MRO convergence Verification: - tsc --noEmit: clean - vitest run test/unit/: 3014 passed - vitest run test/integration/resolvers/: 1746 passed * test(SM-11): address second PR #744 review round + per-language integration tests Review fixes (https://github.com/abhigyanpatwari/GitNexus/pull/744#issuecomment-4211877593): P1 (Performance): Replace Map allocation in resolveMethodByOwner with a firstDef+ambiguous flag pattern. Zero allocation for the common single-candidate case on the hot path — the previous Map approach allocated on every member call regardless of whether deduplication was needed. P2 (Test gap): Strengthen the module-alias D0 skip test with a homonym fixture (two Users in different files). Previously the test passed whether or not D0 was actually bypassed; the new version proves D0 must be skipped by showing that resolveMemberCall directly returns null (ambiguous) but D1-D4 with alias narrowing picks the right one. Also fixes the underlying D2-vs-alias widening interaction: when filteredCandidates was narrowed by module-alias disambiguation, D2 no longer widens back to the full fuzzy pool (introduces aliasNarrowed boolean flag). L1 (Language coverage): Add C# and Kotlin implements-split tests at the resolveMemberCall layer. L2 (Maintainability): Export OverloadHints as @internal so the test can use a direct cast instead of fragile Parameters<...> type inference. Per-language integration tests: - rust-child-extends-parent: Direct impl method resolution via D0 (with honest documentation of the trait-method-as-Function gap that is Phase 5 / SM-16 scope) - java-interface-default-method: User implements Validator with default method resolved via implements-split MRO - csharp-interface-default-method: Same pattern for C# 8.0+ default interface methods - kotlin-interface-default-method: Same pattern for Kotlin interfaces with default implementations - python-multi-level-mro: 3-level C3 linearization (Grandparent ← Parent ← Child) - cpp-diamond-inheritance: Classic diamond (Base ← A, B ← Derived) via leftmost-base MRO Verification: - tsc --noEmit: clean - vitest run test/unit/: 3015 passed - vitest run test/integration/resolvers/: 1763 passed (+17 new per-language tests) * fix(SM-11): Codex adversarial review corrections + deeper D0 fixes Addresses the three high-severity findings from the Codex adversarial review of PR #744 (https://github.com/abhigyanpatwari/GitNexus/pull/744#issuecomment-4212075120), plus four deeper fixes discovered during regression triage. All discovered issues are now addressed end-to-end rather than papered over with tail-return fallbacks. Codex review findings: R1 (C++ diamond): The cpp-diamond-inheritance fixture used non-virtual inheritance, which is genuinely ambiguous in real C++ (two Base subobjects). Changed A and B to use 'virtual public Base' so there's a single shared Base subobject and d.method() is an unambiguous call that the leftmost-base MRO walk correctly resolves. R2 (C# default-interface): The csharp-interface-default-method fixture called user.Validate() via a User-typed variable, but C# does not inherit default interface methods as callable class members — the call is only valid through an interface-typed variable. Changed App.cs to 'IValidator user = new User(...)' which is the idiomatic dispatch pattern. R3 (resolveCallTarget tail-return): When D1-D4 receiver filtering produced zero file-matched and zero owner-matched candidates for a member call, the function fell through to the permissive single-candidate tail return — silently emitting CALLS edges for methods that don't belong to the receiver. Added an explicit null-route inside the D1-D4 block that fires only when both filters yielded 0. R4 (Rust negative assertion): Added the c.trait_only() negative integration test in rust.test.ts demonstrating that direct member calls on Rust structs do not walk trait ancestry. The test now passes because of R3 (previously fell through to the tail return). Regression triage discoveries: 1. D0 was dead code on the sequential pipeline. The sequential path sets overloadHints for every call regardless of whether the method is overloaded, and the original D0 skip condition '!overloadHints && !preComputedArgTypes' was therefore always false. The Java/C#/C++ SM-9/SM-10 inheritance tests were passing ONLY via the tail-return fallback. Fix: narrow the skip to 'overloadHints && filteredCandidates.length > 1' — skip D0 only when there are actually multiple candidates that need overload disambiguation. 2. lookupMethodByOwner couldn't disambiguate arity-differing overloads (e.g. C++ greet() vs greet(string)). With D0 now firing on the sequential path, same-name/different-arity overloads would collapse to an arbitrary first pick. Fix: added an optional argCount parameter to lookupMethodByOwner + lookupMethodByOwnerWithMRO that filters the overload set by parameterCount/requiredParameterCount before the returnType dedup. 3. Python and Rust class methods are captured as Function nodes (not Method) with ownerId set to the class. The methodByOwner index only accepted 'Method' and 'Constructor' types, so Python class methods and Rust trait methods were invisible to D0. Fix: extended the methodByOwner indexing condition to include 'Function' when ownerId is set. This also unlocks the Rust trait-method negative assertion by ensuring the qualified-syntax MRO strategy has something to return null for. 4. D0 was being skipped when a local variable shadowed an imported module name (Python 'from models.c import C; c = C()' creates both a module alias 'c → models/c.py' AND a typed local 'c'). Fix: the D0 skip now gates on 'aliasNarrowed' (a new boolean tracking whether the alias block actually narrowed filteredCandidates) instead of 'hasActiveModuleAlias'. If the method isn't in the aliased module, the receiver is a typed local variable and D0 should run. 5. PHP trait walk missed the HasTimestamps trait because lookupClassByName did not include 'Trait' type. buildHeritageMap uses lookupClassByName to resolve parent names, so 'BaseModel use HasTimestamps' was failing to register an ancestor edge for BaseModel → HasTimestamps. Fix: added 'Trait' to CLASS_TYPES. The trait is now a valid class-like type for heritage resolution (PHP use, Rust impl Trait for Struct, Scala traits). Test updates: - Updated the 'no heritageMap' unit test in call-processor.test.ts to assert the correct null-route behavior instead of the old tail-return fallback. - Added a new unit test asserting Trait inclusion in the class set. - Updated the 'does NOT include other type-like labels' test to remove Trait from its rejection set. Verification: - tsc --noEmit: clean - vitest run test/unit/: 3016 passed (+1 new Trait inclusion test) - vitest run test/integration/resolvers/: 1764 passed (+1 new Rust negative assertion) - Zero regressions --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Gergo Magyar <magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> |
||
|---|---|---|
| .. | ||
| app.py | ||
| child.py | ||
| grandparent.py | ||
| parent.py | ||