feat(shared): DefIndex / ModuleScopeIndex / QualifiedNameIndex (#913, RFC #909 Ring 2 SHARED)

[magyargergo]

Closes #913. Part of RFC #909 Ring 2 SHARED.

Three flat O(1) indexes + pure build functions over per-file artifacts. Contract-only; no runtime wiring yet — consumers (#915 SCC finalize, #917 Registry lookups, #919 ScopeExtractor materialization) import when they land.

What ships

Index	Purpose	Consumer
DefIndex	DefId → SymbolDefinition materialization	#917 — materializes Registry.lookup DefId hits
ModuleScopeIndex	filePath → moduleScopeId for cross-file hops	#915 — SCC finalize link pass
QualifiedNameIndex	qualifiedName → DefId[] cross-kind fast path	#917, #916 — Registry qualified fast path + resolveTypeRef dotted fallback

Each follows the same shape:

• buildXIndex(flatInput): XIndex — pure function, side-effect-free
• Public interface: readonly Map + get / has / size accessors
• First-write-wins on id / filePath collisions (upstream-bug signal, not a silent overwrite)
• Frozen bucket arrays / EMPTY sentinel so callers can iterate without null checks

Design decisions

Decoupled from ScopeTree (which doesn't exist yet — #912). Each index takes a flat input (SymbolDefinition[] or { filePath, moduleScopeId }[]) rather than a scope tree. #919's ScopeExtractor or #915's finalize algorithm will flatten their state into these inputs when they're ready. #913 ships independently.

QualifiedNameIndex stores DefId[], not a single DefId. Partial classes (C#), method overloads sharing a qname, and accidental cross-kind collisions can legitimately share a qualified name. Callers at Registry.lookup filter the returned array by acceptedKinds and rank the survivors.

First-write-wins on collisions. DefId is nodeId (unique by construction), so a collision means upstream parsed the same symbol twice. Dropping the later record preserves the first-stable id the pipeline may already reference — easier to debug than silent overwrite.

QualifiedNameIndex pair-dedup. Same (qname, DefId) appearing twice in the input contributes once. Distinct DefIds sharing a qname accumulate in input order for deterministic ranking.

Tests — 23 in gitnexus/test/unit/scope-resolution/

def-index.test.ts (6): empty, single def, multiple distinct, first-write-wins collision, missing id → undefined, direct byId iteration.

module-scope-index.test.ts (6): empty, single entry, multiple files, first-write-wins on duplicate filePath, missing → undefined, direct iteration.

qualified-name-index.test.ts (11): empty, single qnamed def, partial classes accumulate, input-order preservation, qname separation, skip undefined/empty qname, pair dedup, cross-kind indexing, frozen-empty-array on miss, direct iteration.

Verification

• gitnexus-shared + gitnexus build clean (tsc + scripts/build.js)
• test/unit/scope-resolution: 23/23 pass
• test/unit/model + test/unit/shadow + test/unit/scope-resolution combined: 129/129 pass — no regressions

Unblocks

• RING2-SHARED-6: ClassRegistry / MethodRegistry / FieldRegistry + evidence composition #917 (Registry.lookup implementations) — now has DefIndex.get for materializing DefId hits + QualifiedNameIndex for the qualified fast path
• RING2-SHARED-4: SCC-aware finalize algorithm with bounded fixpoint #915 (SCC finalize) — now has ModuleScopeIndex for the O(1) file → moduleScope lookup in the link pass
• RING2-PKG-1: ScopeExtractor driver (tree-sitter + provider hooks → shared ScopeTree) #919 (ScopeExtractor driver) — now has a target shape for its materialization output

Test plan

• gitnexus-shared build (cd gitnexus-shared && npm run build)
• gitnexus full build (cd gitnexus && node scripts/build.js)
• New tests: 23/23 pass (vitest run test/unit/scope-resolution)
• Adjacent suites: 129/129 pass (model + shadow + scope-resolution)

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
gitnexus	Ready	Preview, Comment	Apr 18, 2026 2:41pm

[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
6657	6560	0	97	256s

✅ All 6560 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	73.25%	17890/24423	73.25%	＝ 0.0	🟢 ██████████████░░░░░░
Branches	62.3%	11384/18271	62.3%	＝ 0.0	🟢 ████████████░░░░░░░░
Functions	77.84%	1694/2176	77.84%	＝ 0.0	🟢 ███████████████░░░░░
Lines	75.8%	16215/21390	75.8%	＝ 0.0	🟢 ███████████████░░░░░

---

_{📋 View full run · Generated by CI}

[magyargergo]

@claude Act as a senior reviewer for GitNexus. Your job is to determine whether this PR is production-ready for this repo, not to give a generic code review.

You are reviewing a PR in the GitNexus monorepo:

• gitnexus/ → CLI + MCP
• gitnexus-web/
• gitnexus-shared/

Your task has 2 phases, in this exact order:

PHASE 1 — DEFINE THE BAR
Before reviewing the diff, establish a concise repo-specific definition of “production-ready” for GitNexus, based only on the repo docs and the affected area.
Keep this definition practical and reviewable. Do not invent standards that are not grounded in the repo.

PHASE 2 — REVIEW THE PR AGAINST THAT BAR
Review the actual diff only after defining the bar.
Stay tightly scoped to the changed code and its direct consequences.

---

CONTEXT TO LOAD FIRST
Read these before reviewing:

• AGENTS.md
• GUARDRAILS.md
• CONTRIBUTING.md
• TESTING.md
• ARCHITECTURE.md

Additional context:

• If the diff touches ingestion, also read: ARCHITECTURE.md § Pipeline Phase DAG
• PR [group/sync] Fix ManifestExtractor never called — config.links always produced 0 cross-links #827 diff area is primarily: gitnexus/src/core/group/ (sync + manifest extractor) plus associated tests

---

PRIMARY OBJECTIVE
Decide whether this PR is safe, correct, maintainable, and operationally acceptable to merge into production for GitNexus.

Do not optimize for completeness at the expense of signal.
Do not pad the review.
Do not propose unrelated refactors.
Do not restate the PR description unless needed for verification.

---

REVIEW RULES

• Every finding must be grounded in specific evidence from the diff or directly relevant surrounding code.
• Every finding must include path:line.
• If you make a behavioral claim, cite the code that proves it.
• If you make a performance claim, explain the mechanism.
• If something cannot be verified from the diff alone, explicitly say so.
• Distinguish clearly between:
  • verified issue
  • plausible risk
  • unverified concern
• Avoid vague wording like “might be better” or “could be improved” unless you explain exactly why.
• Keep the review focused on this PR’s scope only.

For each finding, assign one severity:

• BLOCKING → must be fixed before merge
• NON-BLOCKING → valid issue, but merge may still be acceptable
• NIT → stylistic/minor, not merge-relevant

---

REPO-SPECIFIC REVIEW CHECKLIST
Use these exact headings.

1. Correctness & functional completeness

Check:

• Does the implementation actually satisfy the PR claim?
  • ManifestExtractor is truly invoked
  • config.links produces non-zero cross-links where expected
• Resolver contracts are preserved:
  • resolveSymbol remains exact-match
  • label-scoped Cypher remains correct per contract type
  • flag any regression toward fuzzy or unscoped matching
• Graph schema integrity is preserved:
  • no silent changes to node labels
  • no silent changes to edge types
  • no silent changes to ID generation (generateId)
• Call out any missing wiring, partial integration, dead branch, or mismatch between tests and runtime behavior

2. Code clarity & clean code

Check:

• naming quality
• local cohesion
• dead code
• unnecessary abstraction
• hidden control flow
• confusing indirection
• adherence to repo conventions:
  • direct imports from gitnexus-shared
  • no barrel re-export regressions
  • no // removed comments
  • no unused re-exports
• no drive-by refactors outside stated scope per CONTRIBUTING.md and GUARDRAILS.md § Scope

3. Test coverage & change safety

Evaluate against TESTING.md:

• Are there unit tests under gitnexus/test/unit/ covering the newly wired path?
• Is there a regression guard for 0-link → N-link behavior?
• Are assertions meaningful rather than tautological?
• Are fixtures realistic for manifest inputs?
• If memoization/cache was introduced, is there a test proving hit/miss behavior and correctness?
• Is there evidence the expected validation path would pass for staged gitnexus/ files?
  • tsc --noEmit
  • vitest run --project default
    If not verifiable, say exactly what is missing.

4. Performance

Inspect for:

• hot-path overhead in ingestion/group sync
• excess allocations per manifest entry
• redundant Cypher round-trips
• missed batching or missed parallelism (Promise.all) where it materially matters
• O(n²) or repeated lookup patterns on large repos
• memoization tradeoffs:
  • correctness
  • invalidation
  • bounded vs unbounded memory growth
    Do not speculate casually; explain the mechanism and likely impact.

5. Operational risk

Check:

• Windows/cross-platform safety:
  • stream lifecycle
  • FD/file handle lifecycle
  • path separator assumptions
  • anything resembling prior ENOTEMPTY-style lifecycle regressions
• LadybugDB single-writer invariant is preserved
• Embeddings preservation:
  • no silent breakage of --embeddings
  • .gitnexus/meta.json.stats.embeddings not silently zeroed by changed paths
• MCP contracts remain compatible:
  • group_*
  • query
  • context
  • impact
  • detect_changes
  • rename
  • cypher
    Flag any schema or contract break without migration note
• staleness behavior still triggers correctly (gitnexus/src/mcp/staleness.ts)
• rollback safety:
  • can this PR be reverted safely without re-analyze?
  • if not, explain why

6. Maintainability

Check:

• Does the change respect Pipeline Phase DAG boundaries?
• Is cross-phase coupling being introduced?
• Are manifest-extractor entry points discoverable from group/sync.ts?
• Would the next contributor know where to extend this?
• Is shared logic placed appropriately:
  • in gitnexus-shared/
  • or gitnexus/src/core/**/utils.ts
  • and not duplicated between CLI and web bridge (server/api.ts)?

---

NON-NEGOTIABLES TO VERIFY
From GUARDRAILS.md / CONTRIBUTING.md:

• no secrets, tokens, or machine-specific paths committed
• no blind find-and-replace renames
• if a symbol rename occurred, rename MCP tool or equivalent careful review was used
• upstream impact was considered for modified shared symbols
• no HIGH/CRITICAL impact left unaddressed
• PR title follows conventional commit style + [area] Short description
• PolyForm Noncommercial license headers/metadata untouched

---

OUTPUT FORMAT

Production-ready bar for this PR

Give a concise repo-specific definition in 5–10 bullets.

Findings

For each finding, use this structure exactly:

[BLOCKING|NON-BLOCKING|NIT] Short title

• Evidence: path:line, path:line
• Why it matters: ...
• Recommendation: ...
• Confidence: High / Medium / Low

Only include findings that are real and supported.
If there are no findings in a section, say:

• No issues found in this area based on the diff and reviewed context.

What I could not fully verify

List any behavior you could not prove from the diff, tests, or available context.

Final verdict

Choose exactly one:

• ✅ Ready to merge — no blocking issues.
• ⚠️ Merge after addressing blocking items.
• ❌ Not ready — fundamental concerns.

If verdict is not ready, list the blocking items in priority order.

---

IMPORTANT REVIEW BEHAVIOR

• Be strict on correctness and contracts.
• Be conservative about operational risk.
• Be explicit about uncertainty.
• Do not suggest unrelated refactors.
• Do not turn nits into blockers.
• Do not reward intent; review actual implementation.

---

You must also ensure we are on track with the RFC plan! Check the issue tickets involved!

[github-actions]

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

---

PR Review: feat(shared): DefIndex / ModuleScopeIndex / QualifiedNameIndex (#913)

Tasks

• Post initial comment
• Read AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, TESTING.md, ARCHITECTURE.md
• Read changed files (indexes + tests)
• Check RFC Meta: Pure Scope-Based Resolution in the Semantic Model #909 / issue RING2-SHARED-2: DefIndex / ModuleScopeIndex / QualifiedNameIndex build functions #913 alignment
• Define production-ready bar (Phase 1)
• Review diff against bar (Phase 2)
• Post final verdict

---

Production-ready bar for this PR

This PR ships pure shared-package index types + build functions in gitnexus-shared/src/scope-resolution/. The bar is:

• Correctness: Each build function must satisfy its RFC §3.1 contract (O(1) lookup, correct collision policy, no silent data loss). First-write-wins must preserve the first-stable id the pipeline references.
• RFC alignment: Must match RFC Meta: Pure Scope-Based Resolution in the Semantic Model #909 Ring 2 SHARED scope exactly — contract-only is acceptable; no runtime wiring required at this ring.
• No cross-phase coupling: Build functions must be pure (no side effects, no DB interaction, no ingestion references). Pipeline Phase DAG must not be violated.
• Shared-package placement: Types and functions must live in gitnexus-shared/src/scope-resolution/, exported from gitnexus-shared/src/index.ts with correct type/value split. Tests must import from gitnexus-shared directly.
• Test coverage: gitnexus/test/unit/scope-resolution/ must cover the collision policy, miss semantics, mutation-guard (frozen arrays), and direct Map iteration. Assertions must use reference identity where applicable.
• Build + type safety: tsc --noEmit clean, vitest passing. CI evidence is sufficient.
• Non-negotiables: No secrets/machine paths, no blind renames, PR title in conventional-commit format, PolyForm license untouched.
• Rollback safety: Pure additive — no existing symbols modified. Must be reversible without re-analyze.

---

Findings

1. Correctness & functional completeness

No issues found in this area based on the diff and reviewed context.

All three contracts verified:

• DefIndex: buildDefIndex builds O(1) Map<DefId, SymbolDefinition>, first-write-wins via byId.has(def.nodeId) guard (def-index.ts:41). Correct.
• ModuleScopeIndex: buildModuleScopeIndex builds O(1) Map<string, ScopeId>, first-write-wins via byFilePath.has(filePath) guard (module-scope-index.ts:44). Correct.
• QualifiedNameIndex: buildQualifiedNameIndex accumulates DefId[] per qname, pair-deduplicates via \0-separated Set key (qualified-name-index.ts:54), freezes each bucket with Object.freeze(v.slice()) (qualified-name-index.ts:69) before exposing. EMPTY sentinel is a module-level frozen constant (qualified-name-index.ts:77). Correct and mutation-safe.
• No wiring is missing — contract-only is the stated and correct scope for Ring 2 SHARED.
• No graph schema mutations, no node label/edge type changes. No generateId changes.

2. Code clarity & clean code

No issues found in this area based on the diff and reviewed context.

• Naming is consistent across all three files: build{X}Index, {X}Index interface, freezeIndex internal helper. Easy to pattern-match for future contributors.
• No dead code, no // removed comments, no drive-by refactors.
• index.ts:66-72: Type/value split is correct — interfaces use export type, functions use export. ModuleScopeEntry is exported (index.ts:70) as required by consumers.
• Tests import from gitnexus-shared directly — correct per repo convention (def-index.test.ts:9, module-scope-index.test.ts:7, qualified-name-index.test.ts:11).

NIT — freezeIndex name implies stronger guarantee than delivered

• Evidence: def-index.ts:49-61, module-scope-index.ts:52-64
• Why it matters: freezeIndex wraps the Map in a TypeScript ReadonlyMap accessor but does not call Object.freeze() on the underlying Map. The function name implies full immutability; in practice it's a type-level guarantee only. A caller that casts away ReadonlyMap can mutate the backing store. (QualifiedNameIndex is fine — its buckets are Object.freezed, but the outer Map is not.)
• Recommendation: Either rename to wrapIndex / createIndex, or add Object.freeze(byId) before wrapping (cheap, consistent with types.ts:15 lifecycle contract).
• Confidence: High (verified by reading the implementation — no Object.freeze call on the Map in def-index.ts or module-scope-index.ts)

3. Test coverage & change safety

No blocking issues. One minor gap noted below.

• 23 tests across 3 files, all in gitnexus/test/unit/scope-resolution/ ✓
• Assertions use reference identity (toBe(def)) where appropriate, not just structural equality ✓
• First-write-wins is explicitly tested in all three suites (def-index.test.ts:45-52, module-scope-index.test.ts:42-48, qualified-name-index.test.ts:84-88) ✓
• Frozen empty-array mutation guard tested at qualified-name-index.test.ts:107-112 (.push() throws) ✓
• byId / byFilePath / byQualifiedName Map iteration tested in all three suites ✓
• CI confirms 6560/6560 tests pass ✓

NIT — Frozen-bucket test covers miss path only; hit-path bucket frozen status not explicitly asserted

• Evidence: qualified-name-index.test.ts:107-112 tests the EMPTY sentinel path; the Object.freeze(v.slice()) bucket (qualified-name-index.ts:69) for an existing key is not explicitly checked for mutation resistance.
• Why it matters: A regression that removes the Object.freeze on existing-key buckets would go undetected.
• Recommendation: Add one assertion like expect(() => (idx.get('app.User') as string[]).push('x')).toThrow() in the "accumulates distinct DefIds" test. Not blocking.
• Confidence: High

4. Performance

No issues found in this area based on the diff and reviewed context.

• All three builders are O(n) in input size. No O(n²) patterns.
• QualifiedNameIndex allocates three structures: seenPairs Set, byQualifiedName mutable accumulator, frozen read-only Map (qualified-name-index.ts:47-72). For realistic symbol tables (tens of thousands of defs) this is acceptable — all three are GC'd after the caller discards the builder scope. The double-map copy is intentional for mutation safety and not a hotspot concern.
• No Cypher round-trips, no DB interaction, no I/O.

5. Operational risk

No issues found in this area based on the diff and reviewed context.

• Pure in-memory operations: no streams, no file handles, no FD lifecycle to manage.
• No LadybugDB writes. No embeddings touched. No MCP tool signatures changed.
• Rollback is trivially safe: this PR is purely additive. Reverting removes three new files and eight new export lines from index.ts. No existing symbols modified. No re-analyze required.

Plausible risk — filePath key in ModuleScopeIndex is not normalized

• Evidence: module-scope-index.ts:43-45 — keys are stored as provided, no path normalization.
• Why it matters: On Windows, a caller using \ separators and another using / would produce misses. This is a plausible risk, not a verified issue — the current PR is contract-only with no runtime wiring. Whether path normalization is the caller's responsibility or the index's is an architectural choice that should be decided before the first consumer (RING2-SHARED-4: SCC-aware finalize algorithm with bounded fixpoint #915) wires this up.
• Recommendation: Add a comment to buildModuleScopeIndex JSDoc noting that filePath keys must be pre-normalized by callers (consistent separators, no trailing slashes). No code change required in this PR.
• Confidence: Medium (cannot verify from this diff — depends on how RING2-SHARED-4: SCC-aware finalize algorithm with bounded fixpoint #915 feeds paths)

6. Maintainability

No issues found in this area based on the diff and reviewed context.

• Placed correctly in gitnexus-shared/src/scope-resolution/. Exported from index.ts. Consistent with adjacent Ring 1/2 modules.
• Each file's JSDoc cites RFC §, Ring number, issue number, and consuming PR numbers — excellent traceability.
• Identical structural shape across all three indexes makes the pattern learnable and extensible.
• No cross-phase coupling. No CLI/web bridge duplication.

Non-negotiables

Check	Status
No secrets / machine paths	✅
No blind renames	✅ (pure additions)
PR title: conventional commit	✅ feat(shared): ...
PolyForm license untouched	✅
CI: tsc + tests	✅ 6560/6560 pass
HIGH/CRITICAL impact addressed	✅ (no shared symbol modifications)

RFC #909 Ring plan check

Ring	Issue	Status
Ring 1	#910 scope types, #911 LanguageProvider hooks	✅ merged
Ring 2 SHARED	#913 indexes (this PR)	⏳ this PR
Ring 2 SHARED	#918 shadow diff/aggregate	✅ merged
Ring 2 SHARED	#912 ScopeTree	pending (not blocking #913)
Ring 2 consumers	#915 SCC finalize, #917 Registry.lookup, #919 ScopeExtractor	pending (consume this PR)

This PR correctly ships indexes independently of #912 (ScopeTree) by taking flat inputs rather than a tree — intentional and correctly stated in the PR description. Plan is on track.

---

What I could not fully verify

1. filePath normalization contract — whether RING2-SHARED-4: SCC-aware finalize algorithm with bounded fixpoint #915's SCC finalize will feed paths with the same separator convention that callers of ModuleScopeIndex.get() will use. Unverifiable without RING2-SHARED-4: SCC-aware finalize algorithm with bounded fixpoint #915's diff.
2. \0 separator safety in QualifiedNameIndex pair keys — DefId is string with no enforcement against null bytes in the type system. In practice nodeIds are constructed from file paths + identifiers + positions and would never contain \0, but this is an undocumented assumption (qualified-name-index.ts:54).
3. Downstream consumer behavior — whether RING2-SHARED-6: ClassRegistry / MethodRegistry / FieldRegistry + evidence composition #917, RING2-SHARED-4: SCC-aware finalize algorithm with bounded fixpoint #915, RING2-PKG-1: ScopeExtractor driver (tree-sitter + provider hooks → shared ScopeTree) #919 will filter QualifiedNameIndex results by acceptedKinds as expected. Only verifiable when those PRs land.

---

Final verdict

✅ Ready to merge — no blocking issues.

The implementation is correct, clean, well-tested, RFC-aligned, and purely additive. CI is green. Two NITs (rename freezeIndex or add Object.freeze, add hit-path frozen-bucket test) are valid but not merge-blocking. The filePath normalization concern should be documented before #915 lands.