fix(scope-resolution): allow same-range Module-as-parent for top-level scopes (closes #1086)

[mann1x]

Summary

Closes #1086 — scopeResolution aborts (silently, via extractParsedFile's try/catch) for any C# file whose compilation_unit and single top-level namespace_declaration have identical tree-sitter ranges. The triggering shape is shape-driven, not size-driven: any file with no leading content outside the namespace block AND no trailing newline after the closing } produces coincident ranges. Hit on three real-world *.Designer.cs files in PersistentWindows; the four-case empirical repro (2 fail, 2 ok) is in #1086.

The fix is a single carve-out in the parent-validity contract: a Module may parent a same-range non-Module child. The relationship stays acyclic because the carve-out is direction-asymmetric — only Module-as-outer parents a same-range non-Module, never the reverse.

This is option (b) from the issue (named canParentScope helper at the contract layer). Option (a) (in-place tightening of pass1BuildScopes's while-loop with a sort tie-breaker) was also feasible and slightly smaller, but option (b) keeps the carve-out as a single named, documented helper that lives in both scope-extractor.ts and scope-tree.ts — the two places that previously duplicated the same rangeStrictlyContains and that need to agree on what a well-formed parent edge looks like. Putting the carve-out behind a named helper makes the contract greppable and the asymmetry guard impossible to miss.

Changes

• gitnexus/src/core/ingestion/scope-extractor.ts — new canParentScope(outer, inner, outerKind, innerKind) helper next to rangeStrictlyContains. pass1BuildScopes consults it for parent-finding. Sort tie-breaker added so a same-range Module sorts before a non-Module candidate, ensuring the Module lands on the parent-stack first regardless of tree-sitter capture iteration order.
• gitnexus-shared/src/scope-resolution/scope-tree.ts — mirrors the helper. buildScopeTree's parent-must-contain-child check now uses canParentScope so the validator agrees with the extractor. Error message updated to spell out the new contract: "… does not contain child '...' (allowed: strict containment, or equal-range Module-as-parent)".

rangeStrictlyContains keeps its strict semantics in both files — position-index lookups, hook-side range comparisons, and other call sites are unchanged.

Tests

• New regression fixture test/fixtures/lang-resolution/csharp-namespace-as-root-no-trailing-newline/ — both Models/User.cs and App/Program.cs end exactly on the closing } of their namespace with no trailing newline. ~250 bytes total. Each commit's pre-commit hook verifies the trigger byte (tail -c 1 is 0x7D).
• New csharp.test.ts describe — scope extraction completes for both files (the bug surface), and the cross-file IMPORTS edge resolves through the scope-resolution path with reason: 'csharp-scope: using'. The csharp-scope: reason prefix is the signal that scope-resolution drove the resolution rather than the legacy DAG fallback.
• scope-tree.test.ts — replaced the prior "rejects child ranges identical to the parent" case with three new ones:
  • non-Module parent still rejected at equal range (Namespace-as-parent of same-range Class).
  • Module-as-parent of a same-range non-Module accepted (the scope-extractor: 'Namespace has kind Namespace but no parent' on C# files where compilation_unit and a single top-level namespace_declaration share an identical range #1086 carve-out itself, with getParent/getChildren round-trip).
  • Module-as-parent of another Module still rejected (the asymmetry guard — two same-range Modules would imply two roots / cyclic structure).

Validation

• npx vitest run test/unit/scope-resolution test/integration/resolvers → 2514 passed / 77 skipped / 0 failed (52 test files, 85 s).
• npx tsc --noEmit clean in both gitnexus/ and gitnexus-shared/.
• Pre-commit hook (typecheck) green.
• End-to-end on PersistentWindows after rebuilding from this branch (verification on local Docker image): 3 prior scope extraction failed for *.Designer.cs warnings → 0. Existing post-fix(ingestion): two-channel binding lifecycle (closes #1066) + scope-resolution I8 hardening #1082 baseline is 1113 nodes / 2987 edges / 39 clusters / 97 flows; the 3 newly-recovered files contribute additional cross-file resolution beyond that baseline.

Cross-language note

canParentScope is language-agnostic. Other languages whose query emits (compilation_unit) @scope.module plus a single same-range top-level scope (e.g. Python's module matching a single top-level class_definition on a file with no leading/trailing trivia) can naturally hit the same byte shape on minimal files; this fix applies to all of them uniformly.

Test plan

• npx vitest run test/unit/scope-resolution test/integration/resolvers (52 files / 2514 tests)
• npx tsc --noEmit in both packages
• New regression test fails on the original codebase (verified via empirical 4-case probe in scope-extractor: 'Namespace has kind Namespace but no parent' on C# files where compilation_unit and a single top-level namespace_declaration share an identical range #1086)
• Asymmetry guard exercised by the third new scope-tree.test.ts case
• Maintainer to re-run on a wider corpus if desired

[vercel]

@mann1x is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[magyargergo]

Nice! Thanks for this PR! 🎉

[github-actions]

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

---

PR #1087 — Production Readiness Review

Tasks:

• Read trigger comment and classify request
• Read DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md
• Read scope-extractor.ts and scope-tree.ts
• Read test files and fixtures
• Inspect canParentScope semantics in both locations
• Verify extractor/validator agreement
• Inspect candidate sorting in pass1BuildScopes
• Verify runtime behavior and fixture design
• Assess cross-language impact
• Check CI results
• Produce final review

---

Review bar for this PR

• Issue scope-extractor: 'Namespace has kind Namespace but no parent' on C# files where compilation_unit and a single top-level namespace_declaration share an identical range #1086 is fixed through the real scope-extraction / scope-resolution path, not only by isolated ScopeTree unit behavior.
• The same-range parent carve-out is narrow: only Module may parent a same-range non-Module; all other equal-range parent/child combinations remain invalid.
• scope-extractor.ts and scope-tree.ts implement equivalent canParentScope semantics — extractor and validator cannot disagree on a well-formed tree.
• rangeStrictlyContains keeps strict semantics for all call sites other than canParentScope.
• Sort tie-breaker in pass1BuildScopes ensures the same-range Module lands on the stack first, deterministically.
• canParentScope is language-agnostic; the carve-out benefits all file-root shapes uniformly without widening the contract beyond the intended case.
• Tests cover the regression, the allowed carve-out, both rejected equal-range cases, and prove the real C# scope-resolution path via the reason tag.
• Validation meets the DoD baseline for the touched packages, with documented gaps.

---

Understanding of the change

Issue #1086: scope-resolution silently aborted for any C# file whose compilation_unit (mapped to Module) and single top-level namespace_declaration reported identical tree-sitter byte ranges. The shape is trigger-only when there is no leading content outside the {} body and no trailing newline after the closing }. Under the prior strict-containment rule, pass1BuildScopes would (in capture-iteration order) push the Namespace onto the stack with parent = null, then process Module and pop Namespace since it cannot parent Namespace at the same range — leaving Namespace parentless. buildScopeTree then threw non-module-requires-parent, which extractParsedFile's try/catch swallowed silently, aborting the whole file.

The carve-out: A new helper canParentScope(outer, inner, outerKind, innerKind) is introduced in both files. It returns true for (a) strict containment as before, plus (b) equal-range pairs where outerKind === 'Module' and innerKind !== 'Module'. The fix also adds a sort tie-breaker to ensure Module sorts before a same-range non-Module candidate, so Module is always on the stack when the non-Module scope is processed.

Runtime paths affected: pass1BuildScopes in scope-extractor.ts (parent-finding) and buildScopeTree in gitnexus-shared (parent-validity validation). All downstream scope-resolution consumers of buildScopeTree are unaffected — they only see valid trees.

What must remain unchanged: rangeStrictlyContains strict semantics for all other range logic; the sibling-overlap guard; the Module-as-parent-of-Module rejection; non-Module-as-parent rejection for same-range children.

---

Findings

[medium] canParentScope is duplicated — silent drift risk between extractor and validator

• Category: Contracts / Maintainability
• Files: gitnexus/src/core/ingestion/scope-extractor.ts, gitnexus-shared/src/scope-resolution/scope-tree.ts
• Issue: The helper is identical in both files today. Since scope-extractor.ts already imports from gitnexus-shared (import { buildScopeTree, ... } from 'gitnexus-shared'), it could import canParentScope directly from the shared package. As-is, a future edit to one file that forgets the other produces silent extractor/validator disagreement: the extractor would produce trees the validator rejects (or vice versa), surfacing as ScopeTreeInvariantErrors on files that were previously clean. The PR's cross-file documentation comments reduce the chance of this but do not eliminate it.
• Why it matters here: buildScopeTree is the shared contract layer (Ring 2 SHARED RING2-SHARED-1: Scope / ScopeTree / PositionIndex in gitnexus-shared #912). Its correctness invariants are expected to be authoritative. Duplicating a rule that defines those invariants in the consuming layer creates a two-source-of-truth problem — exactly the class of drift gitnexus-shared/ is meant to prevent.
• Recommended fix: Export canParentScope from gitnexus-shared/src/scope-resolution/scope-tree.ts (or a co-located scope-tree-utils.ts) and import it in scope-extractor.ts. The helper's comment and asymmetry guard then live in exactly one place. This is the smallest correct change that eliminates the drift surface. Fix this →

[medium] gitnexus-web/ typecheck not run — DoD §4.4 gap for a shared-package change

• Category: Validation
• Files: gitnexus-shared/src/scope-resolution/scope-tree.ts, gitnexus-web/
• Issue: DoD §4.4 requires that when gitnexus-shared/ changes, dependent packages typecheck and test after the change — "verify both CLI and web consumers together." The PR runs npx tsc --noEmit in gitnexus/ and gitnexus-shared/ but does not mention cd gitnexus-web && npx tsc -b --noEmit or npm run build in gitnexus-shared/ to verify the dist is current before downstream consumers typecheck. The type-level contract of buildScopeTree is unchanged (same signature, same exported types), so the actual risk of a typecheck failure is low — but the DoD requirement is explicit and the gap is not documented in the PR.
• Why it matters here: Any time gitnexus-shared/ exports change behavior — even privately — CI ci-quality.yml runs the web typecheck. A gap in local validation that CI catches is acceptable; a gap that CI doesn't cover is not.
• Recommended fix: Add cd gitnexus-web && npx tsc -b --noEmit to the PR's validation checklist, or document explicitly why it doesn't apply (e.g., gitnexus-web does not import buildScopeTree or ScopeTreeInvariantError). CI will catch this if it runs, but the PR's validation claims should match what was actually run.

[low] CI main workflow still in progress — local validation unverified by CI

• Category: Validation
• Files: N/A
• Issue: At review time, the main CI workflow (run ID 24956557814) is in_progress. The PR's validation claims (2514 passed / 0 failed) are from a local run, not from a green CI run. The Docker Build & Push job is also still running.
• Why it matters here: DoD §3 ("No false-done. 'Done' is claimed only after the Validation Baseline below has been run or any gap is explicitly named. Green tests on an unrelated path do not constitute validation.") requires CI results to be verifiable before merge. This is not blocking if CI passes, but the review is written before that is confirmed.
• Recommended fix: Wait for CI to complete before merging. No code change needed.

---

Definition of Done check

Item	Status
#1086 fixed through the real scope-extraction / scope-resolution path (not only ScopeTree isolation)	satisfied — integration test pins reason === 'csharp-scope: using', proving the scope-resolution path ran
Carve-out is narrow: only Module-as-outer at equal range; all other equal-range pairs rejected	satisfied — canParentScope condition is outerKind === 'Module' && innerKind !== 'Module' && rangesEqual(...)
Extractor and validator agree on parent-validity semantics	satisfied (currently) — both helpers are byte-for-byte identical; the drift risk is documented under Findings
rangeStrictlyContains keeps strict semantics for all other callers	satisfied — rangeStrictlyContains is unchanged and used only inside canParentScope in both files
Sort tie-breaker is deterministic and ensures Module sorts before same-range non-Module	satisfied — sort explicitly puts a.kind === 'Module' first (return -1) after all positional keys tie
Change is language-agnostic without creating invalid scope trees for non-C# languages	satisfied — carve-out applies uniformly to any Module/non-Module equal-range pair; malformed multi-same-range-sibling cases still hit the sibling-overlap guard
Tests cover regression, carve-out, rejected equal-range cases, and real C# resolver path	satisfied — three new scope-tree.test.ts cases + integration test with reason pin
Validation sufficient for a shared scope-resolution contract change	partially satisfied — gitnexus/ and gitnexus-shared/ validated; gitnexus-web/ typecheck gap per DoD §4.4; CI pending

---

Validation assessment

Tests added / updated:

• scope-tree.test.ts: three new cases — non-Module equal-range rejection (Namespace→Class), Module→Namespace equal-range acceptance with getParent/getChildren round-trip, Module→Module equal-range rejection (asymmetry guard).
• csharp.test.ts: new describe('C# namespace-as-root with no trailing newline (issue #1086)') with two it blocks: class detection proving extraction completed, and IMPORTS edge with reason === 'csharp-scope: using' proving the scope-resolution path.
• Fixtures: csharp-namespace-as-root-no-trailing-newline/ with two source files. Read tool confirms both end on } at the last line with no trailing blank line (consistent with no trailing newline byte, as described by the PR's pre-commit tail -c 1 guard).

Validation claimed by PR:

• npx vitest run test/unit/scope-resolution test/integration/resolvers → 2514 passed / 77 skipped / 0 failed (52 files)
• npx tsc --noEmit clean in gitnexus/ and gitnexus-shared/
• Pre-commit typecheck green
• Local end-to-end on PersistentWindows: 3 prior scope extraction failed for *.Designer.cs warnings → 0

Validation verified from diff / CI evidence:

• Code structure (two canParentScope helpers, sort, test assertions) is consistent with the claimed test results.
• CI: labeler and description checks passed; main CI workflow is in_progress — no CI green confirmation available.

Missing / not-verifiable validation:

• gitnexus-web/ typecheck per DoD §4.4 (not mentioned).
• npm run build in gitnexus-shared/ (not mentioned; the shared dist is consumed by downstream typechecks).
• CI results (pending).

Whether missing validation blocks production readiness:
The gitnexus-web/ gap is low actual risk (type signature of buildScopeTree unchanged, canParentScope is module-private). Pending CI is the more meaningful gating item — the PR should not merge until CI is green.

---

Final verdict

production-ready with minor follow-ups

The fix is semantically correct: canParentScope implements exactly the intended asymmetric carve-out, the sort tie-breaker is deterministic, and the extractor/validator currently agree byte-for-byte. The integration test's reason === 'csharp-scope: using' pin is a meaningful assertion that the scope-resolution path actually ran rather than a legacy fallback — this is the right way to prove the real bug surface is covered. The two medium findings are non-blocking: the gitnexus-web/ typecheck gap carries negligible actual risk given the unchanged type contract, and the helper duplication is documented and cross-referenced. The recommended follow-up is to export canParentScope from gitnexus-shared to eliminate the drift surface once the immediate bug is shipped. Merge should wait for CI to complete.

[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
7578	7478	0	98	287s

✅ All 7478 tests passed

98 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
• bridge-db edge cases > test_openBridgeDbReadOnly_bak_recovery_restores_bridge
• bridge-db edge cases > test_writeBridge_crossLink_with_missing_to_node_silently_skipped
• bridge-db edge cases > test_writeBridge_manifest_grpc_link_with_symbol_uids_persists_queryable_contract_edge

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	75.59%	21966/29058	75.6%	📉 -0.0	🔴 ███████████████░░░░░
Branches	64.52%	14062/21793	64.52%	＝ 0.0	🟢 ████████████░░░░░░░░
Functions	80.81%	2127/2632	80.82%	📉 -0.0	🔴 ████████████████░░░░
Lines	78.4%	19818/25277	78.4%	＝ 0.0	🟢 ███████████████░░░░░

---

_{📋 View full run · Generated by CI}

[magyargergo]

It seems this bug wasn't covered in the legacy path either. We need to be able to define a set of tests that we expect to fail during legacy analysis in the parity tests.

---

Could you please look into this? 🙏 your implementation is nice and i'll merge it in regardless, I just want to turn the CI green.

[magyargergo]

I'm adding this feature now.

[magyargergo]

Opened #1099 to add the legacy-parity expected-failure harness and register this PR's new C# namespace-as-root assertion as a known legacy gap. Once #1099 lands and this PR is updated/re-run, the csharp parity check should stop failing on the legacy-only reason mismatch.

[magyargergo]

@mann1x Please resolve the conflicts from origin/main, I merged your other PR #1085 into main.

[mann1x]

Thanks for the review @magyargergo / Claude — both medium findings addressed and the branch is rebased on the latest main. Summary of changes since the last review:

1. canParentScope duplication (medium) — fixed

Per the recommendation, the helper now lives only in gitnexus-shared/src/scope-resolution/scope-tree.ts (Ring 2 SHARED #912) and is re-exported from gitnexus-shared. scope-extractor.ts imports it instead of holding its own copy. The local rangeStrictlyContains in the extractor was deleted at the same time (it was used only inside the deleted canParentScope); the local rangesEqual stays because it's still consumed by capture-anchor logic at two unrelated sites. Validator and extractor now have a single source of truth — no future drift surface.

Net diff in this commit (d39482d): +7 / -53.

2. gitnexus-web/ typecheck (medium, DoD §4.4) — verified

$ cd gitnexus-web && npx tsc -b --noEmit
$ echo $?
0

Type contract of buildScopeTree is unchanged; canParentScope is a new export but no gitnexus-web code references it. Both CLI and web consumers typecheck clean against the post-refactor gitnexus-shared.

3. Rebased on upstream/main

Branch was DIRTY against the new main (commit 7c3fa58) because PR #1085 (the companion fixture) merged and added a describe('C# frozen-binding collision via using-import (issue #1066 companion)', ...) block at the bottom of test/integration/resolvers/csharp.test.ts. Resolution: keep #1085's describe verbatim, append #1086's describe after it. Both regression suites coexist in the file.

4. Validation

Check	Result
gitnexus-shared npm run build	clean
gitnexus npx tsc --noEmit	clean
gitnexus-web npx tsc -b --noEmit	clean
vitest run test/unit/scope-resolution test/integration/resolvers	2522 passed / 0 failed / 77 skipped (54 files)
vitest run (full suite, in dockerized node:22-trixie-slim per Dockerfile.cli)	7238 passed / 1 failed / 97 skipped

The single failure is test/unit/ignore-service.test.ts > warns on EACCES but does not throw — it asserts that loadIgnoreRules returns null when the file path emits EACCES. The container runs as uid=0, where Linux bypasses POSIX permission checks, so the EACCES condition can't be triggered and loadIgnoreRules returns the loaded rules instead of null. Pre-existing limitation of the privileged-container test environment, completely unrelated to scope-resolution; CI runs as a non-root user and clears it.

Final shape of the PR

Two commits on top of upstream/main:

• fbdf2ed — original fix for scope-extractor: 'Namespace has kind Namespace but no parent' on C# files where compilation_unit and a single top-level namespace_declaration share an identical range #1086 (carve-out + sort tie-breaker + tests + fixture), rebased
• d39482d — refactor addressing the duplication finding

Happy to squash on merge if preferred.

[mann1x]

Correction on the prior comment's "1 failure": I shouldn't have dismissed it as environmental and moved on. It's a real test-stability bug — the loadIgnoreRules — error handling > warns on EACCES but does not throw test in test/unit/ignore-service.test.ts is fragile under uid=0 because root bypasses POSIX read-permission checks, so chmod 000 doesn't trigger EACCES and the assertion expect(result).toBeNull() fails on any privileged test environment.

Sent as a separate one-line PR (extends the existing skipIf(win32) guard with process.getuid?.() === 0): #1108. With that fix applied locally, the dockerized full suite is 7239 passed / 0 failed / 97 skipped — clean.

PR #1087 itself remains pure scope-resolution + the dedup refactor; the test fix is unrelated and properly belongs on its own PR.

[github-actions]

Claude finished @magyargergo's task in 3m 35s —— View job

---

PR #1087 — Production Readiness Review (Round 2)

Tasks:

• Read trigger comment and classify request
• Read DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md
• Read scope-tree.ts (shared) and scope-extractor.ts
• Read gitnexus-shared/src/index.ts for export surface
• Read test files and fixtures
• Inspect canParentScope semantics — both locations
• Verify extractor/validator agreement (single source of truth after refactor)
• Inspect candidate sorting in pass1BuildScopes
• Inspect shared package export surface
• Verify runtime behavior and fixture design
• Assess cross-language impact
• Check CI results
• Produce final review

---

Review bar for this PR

• Issue scope-extractor: 'Namespace has kind Namespace but no parent' on C# files where compilation_unit and a single top-level namespace_declaration share an identical range #1086 is fixed through the real scope-extraction / scope-resolution path, not only by isolated ScopeTree behavior.
• The same-range parent carve-out is narrow: only Module may parent a same-range non-Module; all other equal-range parent/child cases remain invalid.
• scope-extractor.ts and scope-tree.ts use equivalent parent-validity semantics so extraction and validation cannot disagree.
• rangeStrictlyContains keeps strict semantics for all other callers and position/range logic.
• Candidate sorting in pass1BuildScopes is deterministic and ensures the same-range Module is considered before the non-Module child.
• Any gitnexus-shared public export change is intentional, type-safe, and does not expose unstable internals accidentally.
• Tests cover the regression, the allowed carve-out, rejected equal-range cases, and the real C# resolver path.
• Validation is sufficient for a shared scope-resolution contract change.

---

Understanding of the change

Issue #1086: scope-resolution silently aborted for C# files where tree-sitter reports identical byte ranges for compilation_unit (→ Module) and the single top-level namespace_declaration (→ Namespace). This shape arises when a file has no leading content before the namespace block AND no trailing newline after the closing }. Under strict containment, pass1BuildScopes could not recognize Module as a valid parent for a same-range Namespace, so Namespace got parent = null. buildScopeTree then threw non-module-requires-parent, swallowed silently by extractParsedFile's try/catch, aborting the entire file's scope extraction.

The carve-out: canParentScope(outer, inner, outerKind, innerKind) returns true for (a) strict containment as before, plus (b) outerKind === 'Module' && innerKind !== 'Module' && rangesEqual(outer, inner). The direction-asymmetry preserves acyclicity: Module-as-outer parents a same-range non-Module, but the reverse is false, and Module→Module at equal ranges is also rejected. The sort tie-breaker ensures Module lands on the parent stack first regardless of tree-sitter capture order.

What changed since the first review: The previous medium finding (duplication of canParentScope between extractor and validator) has been resolved. canParentScope now lives exclusively in gitnexus-shared/src/scope-resolution/scope-tree.ts and is re-exported from gitnexus-shared/src/index.ts. scope-extractor.ts imports it directly, eliminating the drift surface. The local rangesEqual stays in the extractor for the unrelated anchor-hosting logic at lines 484 and 683 (checking whether a declaration anchor coincides with its innermost scope's range — a different concern from parent-validity).

Runtime paths affected: pass1BuildScopes (parent-finding, extractor) and buildScopeTree (parent-validity validation, shared). All downstream consumers of buildScopeTree are unaffected; they only see valid trees.

---

Findings

[low] Program.cs does not actually trigger coincident ranges — fixture comment is slightly imprecise

• Category: Documentation / Fixture design
• Files: gitnexus/test/fixtures/lang-resolution/csharp-namespace-as-root-no-trailing-newline/App/Program.cs, integration test comment block at line 2551-2552
• Issue: Program.cs has using NoTrailingNewline.Models; before the namespace block. This means compilation_unit starts at the using directive (line 1) while namespace_declaration starts at line 3 — the two ranges are not identical for this file. Only User.cs (which has no leading using) actually triggers the coincident-range shape. The PR comment "both files end on the closing } with no trailing newline" is accurate, but implies both files trigger the bug, when only User.cs does. The test comment at line 2551-2552 says "The fixture mirrors that shape minimally — both files end on the closing } with no trailing newline."
• Why it matters here: The regression test is still correct and strong — if User.cs aborts extraction, User is invisible to the cross-file resolver, and the IMPORTS edge from Program.cs → User.cs with reason === 'csharp-scope: using' would fail. The test catches the real bug. This is a documentation gap, not a correctness gap.
• Recommended fix: No code change needed. The test is sound. If desired, the comment could be tightened to note that User.cs is the file that triggers coincident ranges (no leading using, no trailing newline), while Program.cs tests cross-file resolution after User.cs extraction succeeds.

---

No other production-readiness issues found against the current DoD bar.

---

Definition of Done check

Item	Status
#1086 fixed through the real scope-extraction / scope-resolution path	satisfied — integration test pins reason === 'csharp-scope: using', proving scope-resolution ran; if extraction aborted on User.cs, the IMPORTS edge would be absent or carry a different reason
Carve-out is narrow: only Module-as-outer at equal range; all other equal-range pairs rejected	satisfied — canParentScope condition is outerKind === 'Module' && innerKind !== 'Module' && rangesEqual(outer, inner); Module→Module rejected; Namespace→Class rejected; verified in both canParentScope and unit tests
Extractor and validator agree on parent-validity semantics (single source of truth)	satisfied — canParentScope now lives exclusively in gitnexus-shared/scope-tree.ts and is imported by scope-extractor.ts; no duplicate definition; no drift surface
rangeStrictlyContains keeps strict semantics for all other callers	satisfied — rangeStrictlyContains is unexported, unchanged, and called only inside canParentScope in scope-tree.ts; all other range logic is unaffected
Sort tie-breaker is deterministic and ensures Module sorts before same-range non-Module	satisfied — sort keys: startLine ASC, startCol ASC, endLine DESC, endCol DESC, then Module before non-Module, then 0; deterministic for the intended case
Two same-range non-Module siblings under Module are still rejected	satisfied — rangesOverlap returns true for identical ranges; Pass 3 sibling check catches this; the carve-out cannot be exploited to admit overlapping siblings
gitnexus-shared export is intentional, type-safe, does not expose unstable internals	satisfied — canParentScope takes Range and Scope['kind'], both already in the public API; the export rationale (eliminate duplication) is documented in the PR and in the code comment
Tests cover regression, carve-out, rejected equal-range cases, and real C# resolver path	satisfied — three new scope-tree.test.ts cases (non-Module parent rejected, Module→Namespace accepted with round-trip, Module→Module rejected) plus integration test with reason pin
Validation sufficient for a shared scope-resolution contract change	satisfied — gitnexus-shared built, gitnexus typechecked, gitnexus-web typechecked (exit 0 per last comment + CI ci-quality.yml), full test suite 7478/0 in CI

---

Validation assessment

Tests added / updated:

• scope-tree.test.ts: three new invariant violations cases — Namespace-as-parent of same-range Class rejected; Module-as-parent of same-range Namespace accepted with getParent/getChildren round-trip; Module-as-parent of same-range Module rejected (asymmetry guard).
• csharp.test.ts: new describe('C# namespace-as-root with no trailing newline (issue #1086)') — class detection proving extraction completed for both files; IMPORTS edge with reason === 'csharp-scope: using' proving the scope-resolution path ran.
• Fixtures: csharp-namespace-as-root-no-trailing-newline/Models/User.cs and .../App/Program.cs. User.cs confirmed to end exactly on } with no trailing newline (7 lines, no blank last line). Program.cs has a leading using so its compilation_unit and namespace_declaration are NOT coincident — but this doesn't weaken the test (see Finding above).

Validation claimed by PR (round 2):

• gitnexus-shared npm run build — clean
• gitnexus npx tsc --noEmit — clean
• gitnexus-web npx tsc -b --noEmit — clean (exit 0)
• vitest run test/unit/scope-resolution test/integration/resolvers — 2522 passed / 0 failed / 77 skipped (54 files)

Validation verified from CI evidence:

• CI run 24987210975: 7478 passed / 0 failed / 98 skipped across unit + integration + cross-platform. Typecheck stage: success. E2E: success.
• All coverage metrics stable vs base (no regression).

Missing / not-verifiable validation: None. All DoD §4.4 items are addressed in both the PR comments and CI.

---

Final verdict

production-ready

The round-2 state of this PR is correct. The single medium finding from the first review — canParentScope duplication creating a drift surface between extractor and validator — has been resolved cleanly: the helper now has exactly one definition in gitnexus-shared, is re-exported as a narrow, intentional public API, and is imported by scope-extractor.ts. The gitnexus-web typecheck gap from the first review is also closed. The canParentScope contract is direction-asymmetric by construction, the sort tie-breaker is deterministic, the sibling-overlap guard still catches multi-same-range-child pathology, and the integration test's reason === 'csharp-scope: using' pin is a meaningful signal that the real scope-resolution path ran. CI is fully green (7478/0). The one remaining low finding is a documentation imprecision in the fixture comment that does not affect correctness or test strength.