[cli] Preserve Ruby singleton_class context in sequential parsing

[ideepakchauhan7]

Closes #652

Summary

• split sequential Ruby owner lookup from method-extraction owner lookup in parsing-processor.ts
• skip singleton_class for enclosing class resolution, but still use the singleton_class node when the MethodExtractor needs to detect class << self static methods
• add a sequential-path Ruby regression test that covers from_habitat ownership and isStatic behavior with skipWorkers: true

Why

The sequential parser was inconsistent with the worker path for Ruby class << self blocks. It could either return the singleton_class wrapper for owner lookup or lose the static-context signal needed by the MethodExtractor. This change keeps those two concerns separate so sequential fallback preserves both correct ownership and static detection.

Verification

• ./node_modules/.bin/vitest run test/integration/resolvers/ruby.test.ts
• ./node_modules/.bin/tsc --noEmit
• node dist/cli/index.js impact processParsingSequential --direction upstream --repo GitNexus
• node dist/cli/index.js impact seqFindEnclosingClassNode --direction upstream --repo GitNexus
• detect_changes(scope: "all", repo: "GitNexus") via LocalBackend

Risk

• low blast radius from impact analysis on the touched sequential parser symbols
• detect_changes reported medium because the sequential parser participates in shared processParsing flows, but the code change is limited to Ruby singleton_class handling in the sequential fallback path

[vercel]

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

A member of the Team first needs to authorize it.

[Copilot]

Pull request overview

This PR fixes Ruby class << self handling in the sequential parsing fallback so it matches the intended behavior: methods keep the correct enclosing owner (outer class/module) while still preserving the singleton_class context required for isStatic enrichment.

Changes:

• Update sequential enclosing-class traversal to skip Ruby singleton_class nodes for owner/class resolution.
• Introduce a separate sequential traversal for method extraction ownership that can still return singleton_class so isStatic is computed correctly.
• Add an integration regression test forcing the sequential path (skipWorkers: true) to validate both HAS_METHOD ownership and isStatic for from_habitat.

Reviewed changes

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

File	Description
gitnexus/src/core/ingestion/parsing-processor.ts	Splits sequential “enclosing class” vs “method owner for extraction” logic to handle Ruby singleton_class correctly.
gitnexus/test/integration/resolvers/ruby.test.ts	Adds sequential-path regression coverage for class << self ownership and static detection.

---

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

[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
6221	6124	0	97	242s

✅ All 6124 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.62%	16528/22759	72.61%	📈 +0.0	🟢 ██████████████░░░░░░
Branches	61.54%	10586/17201	61.54%	＝ 0.0	🟢 ████████████░░░░░░░░
Functions	77.49%	1532/1977	77.47%	📈 +0.0	🟢 ███████████████░░░░░
Lines	75.03%	15003/19994	75.03%	＝ 0.0	🟢 ███████████████░░░░░

---

_{📋 View full run · Generated by CI}

[xkonjin]

Code Review: Ruby singleton_class sequential parsing fix

Solid, targeted fix. Splitting the node-finder into two helpers with distinct roles is the right call.

What works well:

• seqFindEnclosingClassNode now correctly skips singleton_class for owner and name resolution, while seqFindMethodOwnerNode uses it so isStatic extraction still fires. Good separation of concerns.
• Cache key updated correctly to methodOwnerNode.id.
• Integration test assertions are precise and cover the edge case.

Minor suggestions:

1. seqFindMethodOwnerNode name does not signal it returns a class node. Consider seqFindEnclosingClassOrModuleNode for consistency with the parallel function, to avoid misleading future readers.
2. No coverage for class << self at file-root. The current equals null walk-out is likely correct, but making it explicit via a comment or edge-case test would harden it.
3. NOOP_SYMBOL_TABLE_SEQ comment is slightly stale ("for FieldExtractorContext" when it is now used more broadly). Low priority.

Bugs: None.
Security: N/A.
Test coverage: End-to-end integration test added. Could add a unit-level test for the new helper, but not a blocker.

Verdict: LGTM with minor nits.

[ideepakchauhan7]

Thanks! I addressed the nits in the latest update: renamed the helper for clarity, clarified the file-root class << self behavior in a comment, and refreshed the NOOP_SYMBOL_TABLE_SEQ comment.

[magyargergo]

@ideepakchauhan7 Can you please resolve the merge conflict?

[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 1s —— View job

---

PR Review: Ruby singleton_class sequential parsing fix

Tasks

• Gather context from PR description and comments
• Read changed files
• Assess production readiness
• Review clean code principles
• Check for performance bottlenecks
• Evaluate contributor friendliness

---

Production Readiness

Not quite — there is an unresolved merge conflict (flagged by @magyargergo above). That must be resolved before merging. CI was green on the last commit, so the logic itself is sound.

Beyond the conflict: the fix is correct and safe to ship once merged. The change is surgical — it only affects the sequential fallback path, which is already a lower-priority code path (workers are used whenever available).

---

Clean Code Assessment

What's done well:

• The separation of concerns is the right call. seqFindEnclosingClassNode (for owner/name resolution, skips singleton_class) and seqFindEnclosingMethodContainerNode (for method extraction, returns singleton_class so isStatic is preserved) have clearly distinct responsibilities.
• Comments at both call sites explain why two different helpers are used — this is the most important documentation a reader needs.
• Cache key is correctly updated to methodOwnerNode.id to match the new helper.
• The NOOP_SYMBOL_TABLE_SEQ comment update is a nice housekeeping win.

Concerns:

1. seqFindEnclosingMethodContainerNode name is misleading. The function returns the first CLASS_CONTAINER_TYPES node — which can be a singleton_class, class, module, etc. The name says "method container" but it's really "raw enclosing container (including singleton_class)". The reviewer @xkonjin suggested seqFindEnclosingClassOrModuleNode for consistency with the parallel function. This matters for maintainability — a future contributor would need to read the body to understand the difference between the two nearly identical helpers.

   Fix this →

2. findEnclosingClassInfo in ast-helpers.ts already handles singleton_class (lines 292–307), walking up to the enclosing class/module. This means seqFindEnclosingClassNode duplicates that intent — but only for the sequential path. The two approaches are technically consistent today, but they could drift. A comment cross-referencing ast-helpers.ts:292 would make this explicit.

3. File-root class << self is handled silently. The comment at line 245–246 is good, but there is no test for this edge case. @xkonjin flagged this too. If a user has a Ruby file with only a top-level class << self block (rare but valid), the sequential path will silently emit no owner — which differs from erroring, which is acceptable, but should be documented in a test.

---

Performance

No bottlenecks introduced. The changes are neutral or slightly positive:

• Both traversal helpers are O(depth) walks — identical complexity to before.
• The cache key change (classNode.id → methodOwnerNode.id) is correct. In the class << self case, methodOwnerNode is now the singleton_class node rather than the outer class, meaning the cache is keyed on the more specific node. This is actually better — it avoids a scenario where methods from different singleton_class blocks inside the same class would collide in the cache (unlikely but theoretically possible with nested or multiple class << self blocks).
• No new data structures, no new passes over the AST.

---

Contributor Friendliness

Moderate. The intent is clear from the PR description and the call-site comments, but there's a discoverability issue for future contributors:

• The two helpers (seqFindEnclosingClassNode vs seqFindEnclosingMethodContainerNode) are defined 19 lines apart and look almost identical. Without reading both carefully, a contributor might use the wrong one. A short JSDoc on each explaining which call site should use which would pay dividends.
• The relationship between these sequential helpers and findEnclosingClassInfo in ast-helpers.ts is not documented. The shared logic is spread across two files with no cross-reference.
• Test structure is good — the new describe block is clearly named, uses skipWorkers: true to force the path under test, and covers both ownership and isStatic. Easy to extend.

---

Summary

Dimension	Assessment
Production readiness	⚠️ Blocked by merge conflict; logic is correct
Clean code	✅ Good separation of concerns; minor naming and doc gaps
Performance	✅ No regressions; cache key improvement
Contributor friendliness	⚠️ Two nearly identical helpers need better naming/docs

Recommended before merging:

1. Resolve the merge conflict
2. Rename seqFindEnclosingMethodContainerNode to something that signals it includes singleton_class (e.g. seqFindRawEnclosingContainerNode)
3. (Optional but valuable) Add a JSDoc comment to each helper clarifying which path should use which

[ideepakchauhan7]

@magyargergo I resolved the merge conflict in the latest update and pushed the branch.

[magyargergo]

@claude I think we are violating the language boundaries here. We need a much better architecture for this thus language providers become responsible for defining this. Need a good architecture diagram so parsing processor just defines what and language providers defines precisely how.

[github-actions]

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

---

Architectural Analysis: Language Boundary Violations

Tasks

• Gather context from PR description and comments
• Read changed files and understand current architecture
• Identify language-boundary violations
• Propose architecture where language providers own language-specific logic
• Create architecture diagram

---

You Are Right — There Are Boundary Violations

This PR both fixes a real bug and exposes a deeper architectural problem that predates it. Ruby-specific AST knowledge has leaked into three shared, language-agnostic files. Here is the full picture.

---

Where the Violations Live

1. method-extractors/generic.ts:20 — Hardcoded language-specific node types

/** Owner node types where member functions are effectively static (JVM/Ruby semantics). */
const STATIC_OWNER_TYPES = new Set(['companion_object', 'object_declaration', 'singleton_class']);

singleton_class is Ruby-only. companion_object and object_declaration are Kotlin-only. This set should not exist in a file named "generic". The information about which container node types imply isStatic belongs in each language's MethodExtractionConfig.

2. utils/ast-helpers.ts:292–307 — Ruby node handling in the shared AST utility (worker path)

// Ruby singleton_class (class << self): walk up to the enclosing class/module
// to inherit its name. singleton_class has no name field...
if (current.type === 'singleton_class') {
  let ancestor = current.parent;
  while (ancestor) { ... }
}

findEnclosingClassInfo is a generic utility used by every language. It now contains an explicit Ruby-specific branch. This pattern will grow — the next language with an anonymous owner wrapper will add another if block here.

3. parsing-processor.ts:249 — Ruby container skip logic in the sequential parser (this PR)

function seqFindEnclosingClassNode(node: SyntaxNode): SyntaxNode | null {
  ...
  if (current.type === 'singleton_class') {   // ← Ruby-specific
    current = current.parent;
    continue;
  }
  ...
}

The sequential parser now contains language-specific awareness of Ruby's singleton_class. The worker path duplicates the same logic in ast-helpers.ts. Two diverging implementations of the same language rule is exactly the kind of drift that causes bugs like #652 in the first place.

---

Current Architecture (What vs. How — Broken)

┌─────────────────────────────────────────────────────────────────┐
│  parsing-processor.ts (sequential)          ast-helpers.ts      │
│                                                                  │
│  seqFindEnclosingClassNode()         findEnclosingClassInfo()    │
│    └─ if singleton_class → skip ★       └─ if singleton_class   │
│                                              → walk up ★        │
│  method-extractors/generic.ts                                    │
│    └─ STATIC_OWNER_TYPES = {                                     │
│         'singleton_class',   ★                                   │
│         'companion_object',  ★                                   │
│         'object_declaration' ★                                   │
│       }                                                          │
│                                                                  │
│  ★ = language-specific knowledge leaked into generic files       │
└─────────────────────────────────────────────────────────────────┘

---

Proposed Architecture

The LanguageProvider contract should be the single source of truth for every language-specific AST decision. Two new hooks cover the violations:

┌─────────────────────────────────────────────────────────────────┐
│  LanguageProvider (interface) — owns "how"                      │
│                                                                  │
│  + resolveEnclosingOwner?(                                       │
│      containerNode: SyntaxNode                                   │
│    ): SyntaxNode | null                                          │
│      └─ Ruby: if singleton_class → return enclosing class/module│
│      └─ Others: return null (no override)                        │
│                                                                  │
│  MethodExtractionConfig                                          │
│  + staticOwnerTypes?: ReadonlySet<string>                        │
│      └─ Ruby:   new Set(['singleton_class'])                     │
│      └─ Kotlin: new Set(['companion_object','object_declaration'])│
│      └─ Others: undefined (no static containers)                 │
└─────────────────────────────────────────────────────────────────┘

┌─────────────────────────────────────────────────────────────────┐
│  parsing-processor.ts — defines "what", delegates "how"         │
│                                                                  │
│  seqFindEnclosingOwnerNode(node, provider):                      │
│    1. Walk up CLASS_CONTAINER_TYPES (generic)                    │
│    2. For each container hit:                                    │
│       - call provider.resolveEnclosingOwner(container)          │
│       - if returns a node → use that (language resolved it)     │
│       - if returns null  → keep walking                         │
│       - if undefined     → return current node (default)        │
│                                                                  │
│  method-extractors/generic.ts:                                   │
│    STATIC_OWNER_TYPES ← replaced by config.staticOwnerTypes     │
│                                                                  │
│  ast-helpers.ts:                                                 │
│    findEnclosingClassInfo → remove singleton_class branch        │
│    (callers pass the already-resolved node from the provider)    │
└─────────────────────────────────────────────────────────────────┘

---

Data Flow After the Fix

                        parsing-processor.ts
                              │
                  ┌───────────▼────────────┐
                  │  seqFindEnclosingOwner │  ← generic walk, no lang knowledge
                  │  (node, provider)      │
                  └───────────┬────────────┘
                              │ hits a CLASS_CONTAINER_TYPES node
                              ▼
                  provider.resolveEnclosingOwner(containerNode)
                              │
              ┌───────────────┴───────────────┐
              │ Ruby                          │ All others
              │ singleton_class?              │ return undefined
              │   → walk to parent class      │ (use the node as-is)
              │ else → undefined              │
              └───────────────────────────────┘
                              │
                  ┌───────────▼────────────┐
                  │ methodExtractor.extract│  ← receives final owner node
                  │ (ownerNode, context)   │
                  └───────────┬────────────┘
                              │
                  config.staticOwnerTypes.has(ownerNode.type)
                              │
              ┌───────────────┴───────────────┐
              │ Ruby                          │ Kotlin
              │ singleton_class → isStatic    │ companion_object → isStatic
              └───────────────────────────────┘

---

Migration Checklist

For the contributor who implements this:

• Add resolveEnclosingOwner?(node: SyntaxNode): SyntaxNode | null to LanguageProviderConfig in language-provider.ts
• Implement it in languages/ruby.ts: singleton_class → walk to parent class/module
• Replace STATIC_OWNER_TYPES in method-extractors/generic.ts with config.staticOwnerTypes?: ReadonlySet<string>
• Move singleton_class into rubyMethodConfig.staticOwnerTypes in method-extractors/configs/ruby.ts
• Move companion_object/object_declaration into Kotlin's MethodExtractionConfig
• Remove the singleton_class branch from ast-helpers.ts:findEnclosingClassInfo — call sites should now pass the provider-resolved node
• Collapse seqFindEnclosingClassNode and seqFindRawEnclosingContainerNode into a single seqFindEnclosingOwnerNode(node, provider) in parsing-processor.ts

---

Recommendation for This PR

Don't block the merge on this. The bug (#652) is real, the fix is correct, and CI is green. The architectural cleanup is a larger, separate initiative and should be a follow-up issue.

What I'd recommend:

1. Merge this PR as-is once the merge conflict is confirmed resolved
2. Open a follow-up issue titled something like "Refactor: move language-specific container node logic into LanguageProvider" that references this analysis
3. Link the new issue from the comment thread here so the context is preserved

The violation in generic.ts:STATIC_OWNER_TYPES predates this PR and is the most important one to fix — it is the root cause of the worker/sequential divergence, because the generic extractor embeds language knowledge that each language's config should own.

[magyargergo]

The comment I asked from claude will not block your PR, I'm just curious of something.

[magyargergo]

Thank you for your contribution!