fix(extractors): resolve 3 silent contract mis-resolution bugs (#793)

[magyargergo]

Closes #793

Addresses three high-severity findings from the Codex adversarial review of the new group extractor surface. All three bugs cause silent contract mis-resolution — no exceptions, just wrong contract IDs baked into the graph that downstream cross-impact bridge queries then join through.

Summary

• F1 — manifest-extractor.ts: resolveSymbol passed the full METHOD::path contract string through normalizeRoutePath, producing /GET::/api/orders which can never match Route.name. Every explicit-method manifest link silently fell back to a synthetic UID. Fix: new parseHttpContract() helper strips the METHOD:: prefix before path normalization. buildContractId is unchanged.
• F2 — http-route-extractor.ts: graph-assisted backfill (both provider and consumer paths) used path-only detections.find(), so multi-verb same-URL files attached the wrong verb/handler to provider rows and inferred the wrong verb on FETCHES consumer edges. Fix: require path+method match when method is known; skip backfill when method is unknown and multiple detections share the same path.
• F3 — grpc-extractor.ts: resolveProtoConflict seeded bestScore = -1 and only replaced on strict >, so all-zero-score ties silently selected candidates[0]. Fix: compute all scores, count ties at the top score, return null on ambiguity. Caller skips contract emission and warns with service name + candidate proto paths.

Test plan

• gitnexus/test/unit/group/manifest-extractor.test.ts — explicit-method manifest links now resolve to real Route nodes
• gitnexus/test/unit/group/http-route-multi-verb.test.ts (new) — multi-verb same-URL backfill picks the right verb or refuses
• gitnexus/test/unit/group/grpc-extractor.test.ts — tie/ambiguous proto resolution refuses rather than picking arbitrarily
• All 73 tests across the three suites pass
• Pre-commit eslint + prettier + typecheck clean

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
gitnexus	Ready	Preview, Comment	Apr 14, 2026 6:44am

[magyargergo]

@ivkond Would you be able to review these changes?

[github-actions]

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	success	tsc --noEmit
✅ Tests	success	unit tests, 3 platforms
✅ E2E	success	gitnexus-web changes only

Test Results

Tests	Passed	Failed	Skipped	Duration
6311	6214	0	97	244s

✅ All 6214 tests passed

97 test(s) skipped — expand for details

• Swift MethodExtractor > isTypeDeclaration > recognizes class_declaration
• Swift MethodExtractor > isTypeDeclaration > recognizes protocol_declaration
• Swift MethodExtractor > isTypeDeclaration > rejects import_declaration
• Swift MethodExtractor > visibility > extracts public method
• Swift MethodExtractor > visibility > extracts private method
• Swift MethodExtractor > visibility > defaults to internal when no modifier
• Swift MethodExtractor > protocol methods > marks protocol method as abstract
• Swift MethodExtractor > static and class methods > detects static func as isStatic
• Swift MethodExtractor > static and class methods > detects class func as isStatic
• Swift MethodExtractor > parameters > extracts parameters with types and default values
• Swift MethodExtractor > return type > extracts return type from -> annotation
• Swift MethodExtractor > annotations > extracts @objc attribute
• Swift MethodExtractor > isFinal > detects final func
• Swift MethodExtractor > isFinal > is false when not final
• Swift MethodExtractor > isAsync > detects async func
• Swift MethodExtractor > isOverride > detects override method
• buildTypeEnv > constructor inference (Tier 1 fallback) > lookupClassByName regression coverage > Swift lookupClassByName regression coverage > Swift cross-file constructor inference uses lookupClassByName
• buildTypeEnv > constructor inference (Tier 1 fallback) > lookupClassByName regression coverage > Swift lookupClassByName regression coverage > Swift explicit init inference uses lookupClassByName
• buildTypeEnv > constructor inference (Tier 1 fallback) > lookupClassByName regression coverage > Swift lookupClassByName regression coverage > Swift cross-file constructor inference does not bind plain functions
• buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature
• 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()
• Swift implicit imports (cross-file visibility) > detects UserService class in Models.swift
• Swift implicit imports (cross-file visibility) > resolves UserService() constructor call across files (no explicit import)
• Swift implicit imports (cross-file visibility) > resolves service.fetchUser() member call across files
• Swift implicit imports (cross-file visibility) > creates IMPORTS edges between files in the same module
• Swift extension deduplication > detects Product class
• Swift extension deduplication > resolves Product() constructor despite extension creating duplicate class node
• Swift extension deduplication > resolves product.save() to Product.swift (primary definition)
• Swift constructor call fallback (no new keyword) > resolves OCRService() as constructor call across files
• Swift constructor call fallback (no new keyword) > resolves ocr.recognize() member call via constructor-inferred type
• Swift export visibility (internal vs private) > resolves PublicService() constructor across files
• Swift export visibility (internal vs private) > resolves internalHelper() across files (internal = module-scoped)
• Swift if let / guard let binding resolution > detects User and Repo classes
• Swift if let / guard let binding resolution > resolves user.save() inside if-let to User#save
• Swift if let / guard let binding resolution > resolves repo.save() inside guard-let to Repo#save
• Swift if let / guard let binding resolution > user.save() in if-let does NOT resolve to Repo#save
• Swift await / try expression unwrapping > resolves user.save() via await fetchUser() return type
• Swift await / try expression unwrapping > resolves repo.save() via try parseRepo() return type
• Swift await / try expression unwrapping > detects fetchUser and parseRepo as functions
• Swift for-in loop element type inference > detects User and Repo classes
• Swift for-in loop element type inference > creates implicit import edges between files
• Swift field-type resolution > detects classes and their properties
• Swift field-type resolution > emits HAS_PROPERTY edges from class to field
• Swift field-type resolution > resolves field-chain call user.address.save() → Address#save
• Swift field-type resolution > emits ACCESSES edges for field reads in chains
• Swift field-type resolution > populates field metadata (visibility, declaredType) on Property nodes
• Swift call-result binding > resolves call-result-bound method call user.save() → User#save
• Swift call-result binding > getUser() is present as a defined function
• Swift call-result binding > emits processUser -> getUser CALLS edge for let-assigned free function call
• Swift method enrichment > detects Animal protocol and Dog class
• Swift method enrichment > emits IMPLEMENTS edge Dog -> Animal
• Swift method enrichment > emits HAS_METHOD edges for Dog methods
• Swift method enrichment > marks protocol Animal.speak as isAbstract
• Swift method enrichment > marks Dog.speak as NOT isAbstract
• Swift method enrichment > marks breathe as isFinal
• Swift method enrichment > marks classify as isStatic
• Swift method enrichment > captures @objc annotation on breathe
• Swift method enrichment > populates parameterTypes for classify(_ name: String)
• Swift method enrichment > records parameterCount for classify
• Swift method enrichment > records returnType for speak
• Swift method enrichment > resolves dog.speak() CALLS edge
• Swift method enrichment > resolves Dog.classify("dog") CALLS edge
• Swift abstract dispatch > detects Repository protocol and SqlRepository class
• Swift abstract dispatch > emits IMPLEMENTS edge SqlRepository -> Repository
• Swift abstract dispatch > emits HAS_METHOD edges for Repository.find and Repository.save
• Swift abstract dispatch > emits HAS_METHOD edges for SqlRepository.find and SqlRepository.save
• Swift abstract dispatch > marks base Repository.find as isAbstract
• Swift abstract dispatch > marks base Repository.save as isAbstract
• Swift abstract dispatch > marks concrete SqlRepository.find as NOT isAbstract
• Swift abstract dispatch > resolves repo.find(id: 42) CALLS edge
• Swift abstract dispatch > resolves repo.save(entity: user) CALLS edge
• Swift abstract dispatch > populates parameterTypes for Repository.find
• Swift abstract dispatch > populates parameterTypes for Repository.save
• Swift abstract dispatch > records returnType for SqlRepository.find
• Swift abstract dispatch > emits METHOD_IMPLEMENTS edges from SqlRepository methods → Repository protocol methods
• Swift overloaded method disambiguation > detects 2 distinct find Method nodes on SqlRepository
• Swift overloaded method disambiguation > emits METHOD_IMPLEMENTS edges for both find overloads
• Swift overloaded method disambiguation > emits METHOD_IMPLEMENTS edge for save
• Swift overloaded method disambiguation > emits exactly 3 METHOD_IMPLEMENTS edges total
• Swift Child extends Parent — inherited method resolution (SM-9) > detects Parent and Child classes
• Swift Child extends Parent — inherited method resolution (SM-9) > resolves c.parentMethod() to Parent.parentMethod via first-wins MRO walk

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	72.82%	16775/23036	72.74%	📈 +0.1	🟢 ██████████████░░░░░░
Branches	61.74%	10695/17320	61.61%	📈 +0.1	🟢 ████████████░░░░░░░░
Functions	77.83%	1566/2012	77.72%	📈 +0.1	🟢 ███████████████░░░░░
Lines	75.2%	15230/20251	75.16%	📈 +0.0	🟢 ███████████████░░░░░

---

_{📋 View full run · Generated by CI}

[Copilot]

Pull request overview

Fixes three extractor-edge-case bugs that caused silent contract mis-resolution in the group graph (manifest HTTP METHOD::path parsing, HTTP multi-verb same-path backfill, and gRPC proto conflict resolution), with targeted unit coverage to prevent regressions.

Changes:

• Add parseHttpContract() and strip METHOD:: before normalizing/looking up HTTP routes in ManifestExtractor.
• Disambiguate graph-assisted HTTP backfill by requiring method+path matches when method is known, and refusing to guess when ambiguous.
• Make gRPC proto conflict resolution return null on ambiguous ties (with a warning) and skip emitting contracts in that case.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
gitnexus/src/core/group/extractors/manifest-extractor.ts	Adds helper to parse METHOD::path and fixes HTTP route lookup normalization.
gitnexus/src/core/group/extractors/http-route-extractor.ts	Fixes graph-assisted provider/consumer backfill to avoid .find() path-only mis-attribution in multi-verb cases.
gitnexus/src/core/group/extractors/grpc-extractor.ts	Refuses ambiguous proto conflict resolution and prevents emitting contracts on ambiguity.
gitnexus/test/unit/group/manifest-extractor.test.ts	Adds regression/edge-case coverage for explicit-method HTTP manifest contracts.
gitnexus/test/unit/group/http-route-multi-verb.test.ts	New suite covering multi-verb same-path backfill behavior (provider + consumer).
gitnexus/test/unit/group/grpc-extractor.test.ts	Adds tie/ambiguity tests for proto resolution and extraction behavior when ambiguous.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[magyargergo]

@claude is this pr ready for production? Is this following clean code principles? Are there any performance bottlenecks? Is the code structured in a way that will unlock easy contributions in the future?

[github-actions]

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

---

PR Review

Tasks:

• Gather context and understand the request
• Read changed source files
• Analyze for production readiness
• Analyze for clean code principles
• Analyze for performance bottlenecks
• Analyze for contribution-friendliness
• Post full review

---

Verdict: Not quite ready — one unresolved bug + CI failure needs addressing

The fixes for F1 and F3 are solid and the test coverage is well-designed. F2 has a partial regression that needs one more change before this is safe to merge.

---

CI Failure

The CI failure is not a typecheck failure (the CI report comment above was slightly misleading — typecheck actually passed). The real failure is a Prettier formatting issue in two files unrelated to this PR:

[warn] gitnexus-web/src/hooks/useAutoScroll.ts
[warn] gitnexus-web/test/unit/use-auto-scroll.test.tsx

These are pre-existing unformatted files from a different PR. This PR shouldn't be blocked by them, but the CI Gate will reject the merge until they're fixed (either here or separately). The author should coordinate with whoever owns those files or fix the formatting as a separate commit.

---

Bug: F2 Fix Is Incomplete (Significant)

Copilot's first comment identifies a real regression that the PR author should address.

gitnexus/src/core/group/extractors/http-route-extractor.ts:280–289

// handlerName is null when match is undefined (ambiguous multi-verb case)
if (match) {
  if (!method) method = match.method;
  handlerName = match.name;
}
// ...
const picked = pickSymbolUid(syms, handlerName);  // ← called with null

When match is undefined due to ambiguity (multiple candidates + unknown method), handlerName stays null. pickSymbolUid then falls back to pool[0] — the first Function/Method symbol in the file — silently attributing the wrong handler. The guard against .find() mis-attribution was added for the path filter, but the CONTAINS-based symbol enrichment still picks arbitrarily.

The fix is straightforward: skip the symbol enrichment entirely when the match was ambiguous, or use a sentinel that pickSymbolUid treats as "no preference." Fix this →

---

Bug: buildContractId Casing Inconsistency (Minor)

gitnexus/src/core/group/extractors/manifest-extractor.ts:287

case 'http': {
  if (/^[A-Za-z]+::/.test(contract)) return `http::${contract}`; // preserves original casing
  return `http::*::${contract}`;
}

buildContractId passes through whatever casing the user wrote (e.g. get::/api/orders → http::get::/api/orders), while parseHttpContract normalizes the method to uppercase. A user who writes get::/api/orders in group.yaml gets a different contractId than one who writes GET::/api/orders, which fragments cross-impact join matching. Fix this →

---

Clean Code Assessment: Strong ✅

The changes follow clean code principles well:

• Comments explain the why, not the what. The inline comments are exemplary — especially the docstring on manifestSymbolUid explaining the two reasons for synthetic UIDs, and the normalizeRoutePath explaining what it mirrors.
• Each function has a single responsibility. parseHttpContract, resolveProtoConflict, normalizeHttpPath, normalizeConsumerPath are all narrow and focused.
• resolveProtoConflict is now pure — takes inputs, returns output or null. Good.
• buildContractId's never exhaustiveness guard at line 298–300 is a great pattern that will catch any new ContractType variant at compile time.
• One notable confusion: pickSymbolUid uses both named row properties (r.uid) and positional index access (r[0], r[1]). This dual-mode is a legacy holdover that confuses contributors. It's not introduced by this PR but it's worth a cleanup ticket.

---

Performance Assessment: No New Bottlenecks ✅

• longestSharedSegmentRun (grpc-extractor.ts:207) is O(a·b·min(a,b)) per candidate — technically cubic in path depth, but proto files have short, shallow paths and conflicts are rare. Acceptable.
• stripProtoCommentsAndStrings builds a string[] of the full file length. Fine for proto files (typically small). No issue.
• The CONTAINS_QUERY is still called once per HANDLES_ROUTE/FETCHES row in the graph-assisted paths. This is a potential N+1 query pattern for large repos, but it predates this PR.
• The cachedDetections Map correctly avoids double-parsing files. Good.

---

Contribution-Friendliness Assessment: Good ✅

• The top-of-file doc comment in http-route-extractor.ts ("Adding a new language for Strategy B is a one-file edit") is exactly the kind of guide new contributors need.
• http-route-multi-verb.test.ts demonstrates the mock setup pattern clearly with inline comments — contributors can follow it.
• The parseHttpContract edge cases (::/api/orders, GET::) are documented in the JSDoc — makes the decision logic transparent.
• The never exhaustiveness check in buildContractId makes adding new contract types safe and self-documenting.
• Suggestion: the comment in resolveProtoConflict ("Ties (including all-zero ties) would otherwise silently merge unrelated services…") explains the old bug but doesn't explain the current behavior. A brief "Returns null to force the caller to skip contract emission" would help future maintainers.

---

Summary

Area	Status
Production readiness	⚠️ Not yet — F2 partial regression + CI formatter failure
Clean code	✅ Strong — good naming, comments, and structure
Performance	✅ No new bottlenecks introduced
Contribution-friendliness	✅ Good — plugin extension point is documented, tests are clear

Must fix before merging:

1. The ambiguous-match case in extractProvidersGraph still passes null to pickSymbolUid, which silently picks the first symbol in the file (Copilot comment Add support for Ollama as a local inference backend #1).

Should fix before merging:
2. buildContractId casing inconsistency for lowercase METHOD:: prefixes (Copilot comment #2).
3. Prettier failures in gitnexus-web need to be resolved (can be a separate PR/commit).