feat(ingestion): Link object literal methods to exported bindings

[luyua9]

Summary

Fixes #1358.

TypeScript/JavaScript object literal shorthand methods such as:

export const fooService = {
  async getUser(id: string) {
    return findUser(id);
  },
};

were parsed as top-level Method nodes with only DEFINES edges from the file. The exported object binding (fooService) had no edge to those methods, so graph/impact queries could not discover fooService.getUser.

This PR adds a narrow AST helper for object-literal method owners and uses it in both parsing paths to emit HAS_METHOD from the exported object binding to shorthand object methods.

Validation

• npx vitest run test/unit/parsing-worker-fallback.test.ts
• git diff --check

[vercel]

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

A member of the Team first needs to authorize it.

[github-actions]

Claude finished @magyargergo's task in 6m 44s —— View job

---

PR #1718 Production-Readiness Review

Reviewer: Adversarial production-readiness review — ingestion, AST parsing, graph relationships, scope-resolution migration safety.

Task List

• Read repo docs (AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md, ARCHITECTURE.md, TESTING.md, DoD.md)
• Determine TS/JS migration ownership (legacy DAG vs scope-resolution)
• Inspect all 4 changed files line-by-line
• Inspect related language provider and scope-resolution files
• Check ID generation, label logic, and symbol table consistency
• Verify caller/impact resolution path
• Check sequential vs worker path parity
• Run hygiene/Unicode checks
• Verify CI status
• Formulate findings and final verdict

---

Review Bar

The bar is: every item in §§ 2.1–2.7 of DoD.md relevant to this change, plus the 12-item PR-specific DoD from the task briefing. The weakest production-critical lane determines the verdict.

---

Current PR State

One commit (9a27e6c), 4 changed TypeScript files, +122/−8. Clean branch — no unrelated merge churn, no formatting noise, no generated artifacts.

Branch hygiene: clean feature/fix PR.

---

Understanding of the Change

The PR adds a new helper findObjectLiteralBindingInfo in ast-helpers.ts that walks the AST up from a method_definition inside an object node, finds the enclosing variable_declarator, and returns the owner's graph node ID. Both the sequential path (parsing-processor.ts) and the worker path (parse-worker.ts) are extended to call this helper when a Method node has no enclosing class, and to emit a HAS_METHOD relationship from the owning exported Const/Variable binding to the method. One test verifies the happy path in sequential fallback mode.

---

Architecture / Migration Ownership Finding

TypeScript and JavaScript are both migrated to scope-resolution (RFC #909). Both providers implement emitScopeCaptures (typescript.ts:316, javascript would be :379 per search results). A scope-extractor-bridge.ts and full scope-resolver directories exist for both.

However: the scope-resolution path is explicitly documented as additive to, not a replacement of, the legacy DAG. scope-extractor-bridge.ts:24 states: "Scope-based resolution is the new path under construction — it must not destabilize the legacy DAG. Legacy parsing on the same file continues unaffected." The WorkerExtractedData interface in parsing-processor.ts:83–84 confirms: "parsedFiles are additive to the legacy DAG path."

The structural edges (DEFINES, HAS_METHOD, HAS_PROPERTY) are exclusively built by the legacy DAG. The scope-resolution path produces ParsedFile artifacts for CALLS resolution. The fix to the legacy DAG for structural HAS_METHOD edges is therefore correct placement — this is not a migration boundary violation.

---

Files Inspected

• gitnexus/src/core/ingestion/utils/ast-helpers.ts (full)
• gitnexus/src/core/ingestion/parsing-processor.ts (full)
• gitnexus/src/core/ingestion/workers/parse-worker.ts (partial: imports + ~lines 2050–2370)
• gitnexus/test/unit/parsing-worker-fallback.test.ts (full)
• gitnexus/src/core/ingestion/scope-extractor-bridge.ts (full)
• gitnexus/src/core/ingestion/scope-extractor.ts (header)
• gitnexus/src/core/ingestion/languages/typescript.ts (partial)
• gitnexus/src/core/ingestion/languages/typescript/scope-resolver.ts (full)
• gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts (lines 358–431)
• gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts (partial)
• gitnexus/src/core/ingestion/call-processor.ts (partial)
• gitnexus/src/core/ingestion/tree-sitter-queries.ts (relevant snippets)
• DoD.md, ARCHITECTURE.md (full)

---

CI / Checks Status

Of 9 workflow runs, 8 are action_required (require maintainer authorization to run for a fork PR) and 1 is success (conventional labeler). The required gates — CI (typecheck, unit tests, lint), CodeQL, Gitleaks, Dependency Review — have not executed against this commit. This is not the PR author's fault (fork auth gate), but it means zero CI validation has been confirmed.

---

Findings

---

Finding 1 — BLOCKER: Caller/Impact Resolution Remains Broken (Lane 3)

Severity: CRITICAL
File: parsing-processor.ts:785–795, parse-worker.ts:2321
Evidence:

// parsing-processor.ts:793
symbolTable.add(file.path, nodeName, nodeId, nodeLabel, {
  ...
  ownerId: enclosingClassId ?? undefined,   // ← object literal ownerId is NOT set
  ...
});

// parse-worker.ts:2321
...(enclosingClassId ? { ownerId: enclosingClassId } : {}),  // ← same gap

When an object literal method is parsed, enclosingClassId is null (no class enclosure — that's the problem). The PR adds a HAS_METHOD graph edge from the exported binding to the method, but it does not set ownerId for the method in the symbol table. The legacy call-processor (call-processor.ts) resolves fooService.getUser(id) via receiverTypeName + owner-indexed symbol table lookup. Without an ownerId, getUser is not in the owner index, and receiverTypeName for a lowercase-receiver object literal binding defaults to undefined (the capital-initial heuristic at call-processor.ts:1160 doesn't fire for fooService). The scope-resolution path (receiver-bound-calls.ts) also uses model.methods.lookupAllByOwner(ownerId, ...) — same gap.

Production risk: Issue #1358's core symptom — gitnexus_impact reporting false LOW risk for calls like credentialService.verifyPassword(...) — is not fixed. The structural HAS_METHOD edge exists in the graph but CALLS edges from callers are still not created, leaving impact traversal blind to real callers.

Recommended fix: Also set ownerId to objectLiteralOwnerInfo.ownerId in the symbol table for methods resolved via the object literal path. Additionally, extend the receiver-type resolution chain so that when a binding resolves to an exported object (Const/Variable node with type 'object-literal' or equivalent), member calls against it can be indexed and traversed.

Blocks merge: YES.

---

Finding 2 — BLOCKER: Boundary Guard Is Ineffective for Local Object Literals (Lane 6)

Severity: HIGH
File: ast-helpers.ts:432–466
Evidence:

while (current) {
  if (current.type === 'object') sawObjectLiteral = true;

  if (sawObjectLiteral && current.type === 'variable_declarator') {
    // ← returns here for the local case
    const nameNode = current.childForFieldName?.('name');
    ...
    return { ownerId: ..., ownerName: ... };
  }

  // ← boundary check never reached for local case
  if (current !== node && (FUNCTION_NODE_TYPES... || CLASS_CONTAINER_TYPES...)) {
    return null;
  }
  current = current.parent;
}

For a local object literal like:

export function processAll() {
  const handler = { async run(x: string) { return fetch(x); } };
}

The AST parent walk from run is: method_definition → object → variable_declarator (for handler). The variable_declarator is found at step 3. The enclosing function_declaration boundary sits at step 5 (lexical_declaration → statement_block → function_declaration) and is never reached. The tree-sitter query captures all lexical_declaration nodes regardless of scope depth, so Const:foo.ts:handler already exists in the graph. Result: handler.HAS_METHOD.run is emitted for a local, non-exported binding — a false positive.

The same false attribution fires for:

• Object literals inside class constructors / methods
• Nested object literals: export const s = { nested: { method() {} } } — method gets linked directly to s, skipping nested
• Objects returned from factory functions

Production risk: Incorrect HAS_METHOD edges inflate the apparent API surface of local bindings and misattribute nested object methods to their top-level ancestor. Impact queries for s.method would erroneously include it in the blast radius of s.

Recommended fix: After finding the variable_declarator, walk its ancestor chain and return null if any FUNCTION_NODE_TYPES or CLASS_CONTAINER_TYPES ancestor appears before the root (or a top-level export_statement / program). Alternatively, restructure the loop so the boundary check always precedes the variable_declarator check (with special-casing for the starting node).

Blocks merge: YES.

---

Finding 3 — BLOCKER: Tests Do Not Cover the Issue's Production Symptom (Lane 5)

Severity: HIGH
File: test/unit/parsing-worker-fallback.test.ts:46–88
Evidence:

The new test asserts:

• fooService node exists as Const ✓
• getUser and saveUser are Method nodes ✓
• HAS_METHOD edges exist from fooService to both methods ✓

Missing assertions:

• No test for a consumer file (import { fooService } from ...; fooService.getUser(id)) with a CALLS edge reaching the method.
• No test for impact({target: "getUser", direction: "upstream"}) returning callers.
• No negative test: a const handler = { run() {} } inside a function body should NOT emit HAS_METHOD (proves the boundary guard works).
• No worker-mode test: the test calls processParsing without a workerPool argument, exercising only sequential fallback. Worker mode (the production code path for repos above the dispatch threshold) is unverified.

Per DoD.md §2.7: "Tests cover the real changed path — they would fail if behavior, wiring, or contracts were broken." A green test on the structural edge alone does not prove the production symptom (false LOW risk from impact) is fixed.

Blocks merge: YES (directly tied to Finding 1 — the CALLS gap cannot be verified without an end-to-end test).

---

Finding 4 — MEDIUM: ownerName Field Is Dead Code

Severity: LOW
File: ast-helpers.ts:417–418, ast-helpers.ts:450
Evidence:

export interface ObjectLiteralBindingInfo {
  ownerId: string;
  ownerName: string;   // ← declared
}
// ...
return {
  ownerId: generateId(ownerLabel, `${filePath}:${nameNode.text}`),
  ownerName: nameNode.text,   // ← set
};

Neither call site in parsing-processor.ts nor parse-worker.ts reads ownerName:

const ownerIdForMemberEdge = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId ?? null;
// objectLiteralOwnerInfo.ownerName is never accessed

DoD.md §2.3 prohibits dead code. This is a minor cleanup but violates the "smallest correct solution" rule.

Recommended fix: Remove the ownerName field from the interface and return value.

Blocks merge: NO (minor cleanup; can be addressed in the same PR or follow-up).

---

Finding 5 — MEDIUM: Owner Label Heuristic Inconsistency for let

Severity: LOW
File: ast-helpers.ts:447
Evidence:

const ownerLabel = declaration?.type === 'variable_declaration' ? 'Variable' : 'Const';

var declarations use variable_declaration → Variable. Both const and let use lexical_declaration → heuristic returns Const. This is internally consistent with the tree-sitter query ((lexical_declaration ...) @definition.const captures both). However, let fooService = { ... } would generate Const:file.ts:fooService both from the query capture and from this heuristic, which is semantically confusing (the variable is mutable). This is pre-existing behavior in the query design, not introduced by this PR.

Blocks merge: NO (pre-existing; not a new regression).

---

Finding 6 — CI Not Verified

Severity: HIGH
Evidence: 8 of 9 workflows are action_required (fork authorization gate). The main CI run (typecheck, unit tests, lint), CodeQL, Gitleaks, and Dependency Review have not run. The PR's own validation instructions (npx vitest run test/unit/parsing-worker-fallback.test.ts) cannot be confirmed from CI. A maintainer must authorize the CI runs before this can be considered for merge.

Blocks merge: YES (conditional — CI must be run and pass).

---

Hidden Unicode / Hygiene

• No bidirectional control characters found in changed files (grep clean).
• Non-ASCII characters in ast-helpers.ts are pre-existing em-dashes and arrows in comments (—, →). These are ordinary punctuation, not executable code. Clean.
• git diff --check not executable in this environment, but the branch appears whitespace-clean.

---

PR-Specific Assessment Sections

Migration Boundary

Pass with caveat. Both TS and JS have emitScopeCaptures, making them scope-resolution-migrated. However, the legacy DAG path remains the sole producer of structural graph edges (DEFINES, HAS_METHOD, HAS_PROPERTY). The fix correctly targets the legacy DAG structural path. The risk is that scope-resolution CALLS paths also need fixing (owner indexing via ownerId) — which this PR omits.

Object Owner Edge Correctness

Partial. For the straightforward export const fooService = { method() {} } case, the ID (Const:file.ts:fooService) matches the tree-sitter query's @definition.const label derivation. The ID is stable and correct for exported file-scope bindings. Fails for local bindings inside functions and for nested object literals (Finding 2).

Caller / Impact Correctness

Fails. The central issue — callers cannot be discovered via impact({target: "getUser", direction: "upstream"}) — is not fixed. ownerId is absent from the symbol table for object-literal methods, so the receiver-type call resolution chain cannot link fooService.getUser(id) to getUser (Finding 1).

Sequential vs Worker Path Parity

Structural parity achieved. Both parsing-processor.ts and parse-worker.ts are identically extended. However, only the sequential path is tested. The worker path is not covered by any test in this PR.

Test Adequacy

Insufficient for the stated DoD. Covers exactly one happy-path structural assertion in sequential mode. Missing: CALLS integration, impact traversal, worker mode, negative case (local binding), nested object literal, consumer file. DoD.md §2.7 requires tests that would fail if wiring or contracts broke — the current test would not catch the CALLS gap or the boundary guard bug.

Hidden Unicode / Hygiene

Clean. No bidirectional controls. Pre-existing em-dashes in comments are benign.

CI / Security

Unverified. All required CI checks are pending fork authorization. Cannot confirm typecheck, lint, or test results. Vercel deployment warning (authorization required for fork) is not a merge blocker per project policy.

---

Final Verdict

Not production-ready.

The PR correctly identifies the problem and places the structural fix in the right code layer (legacy DAG for HAS_METHOD edges). However, it only solves half of the issue: the CALLS resolution gap that causes false LOW impact scores remains open (Finding 1). The ownerId is not propagated to the symbol table for object-literal methods, so receiver-bound call-processor and scope-resolution passes cannot connect fooService.getUser(id) call sites to the getUser node. Additionally, the boundary guard in findObjectLiteralBindingInfo is ineffective (Finding 2) — local object literals inside functions would generate incorrect HAS_METHOD edges, creating false-positive graph connectivity. The test suite does not verify the production symptom and has no negative or worker-mode coverage (Finding 3). The weakest lanes are Findings 1 and 2 (caller resolution and nested scope correctness), which must be resolved before this PR can close issue #1358 in production.

[magyargergo]

@luyua9 can you please look into claude's findings? 🙏

[magyargergo]

@luyua9 any update?

[magyargergo]

Addressed Claude's adversarial production-readiness review in e9531cee.

Findings closure

F1 — Caller / impact resolution (BLOCKER → fixed). The review's recommended fix (setting ownerId on the symbol-table entry) was necessary but not sufficient: scope-resolution's receiver-bound-calls.ts only consults class-like or type-annotated bindings, so a lowercase value receiver like fooService with no type annotation never reaches owner-indexed lookup even when ownerId is correct.

Fix is in two parts:

• parsing-processor.ts:793 + parse-worker.ts:2321 — symbol-table ownerId now falls back to objectLiteralOwnerInfo?.ownerId (the original review recommendation).
• receiver-bound-calls.ts — new Case 5 (value-receiver bridge) after Case 4: resolves the receiver name via a new findValueBindingInScope walker, translates the scope-resolution Variable def to the canonical graph node id, and emits the CALLS edge against the owner-indexed method registry. Const was also added to isLinkableLabel in node-lookup.ts so the graph-bridge sees these nodes during lookup table construction.

Implemented in the scope-resolution path only — the legacy DAG (call-processor.ts) was intentionally not touched: TS/JS are scope-resolution migrated per RFC #909, so the bug fix lives where the production resolution flows.

F2 — Boundary guard (BLOCKER → fixed). findObjectLiteralBindingInfo rewritten as an explicit two-phase walk:

• Phase A tracks objectDepth; returns null for nested literals (objectDepth > 1) and for function/class containers reached before the owning variable_declarator (catches IIFE patterns).
• Phase B walks the declarator's own ancestors and returns null on any function, class, or block-statement container (statement_block, if_statement, for_*, while_*, try_statement, catch_clause, switch_*, with_statement) before reaching program / export_statement. This closes the additional block-scope leak surfaced during deepening (if (cond) { const handler = {...} } at file scope no longer emits a false-positive HAS_METHOD).

F3 — Test coverage (BLOCKER → fixed). Two new integration test files:

• test/integration/ast-helpers-object-literal-binding.test.ts (13 cases) — pins helper semantics against the real tree-sitter TypeScript parser: happy paths (export const, bare const, var), function/arrow/class-constructor boundaries, nested literals, block scope (if / for-of / try), IIFE wrapping, and assignment without variable_declarator (no throw).
• test/integration/object-literal-owner-resolution.test.ts (9 cases) — drives runPipelineFromRepo against an on-disk fixture repo (matches the codebase's existing integration pattern from cross-file-binding.test.ts):
  • Test A: sequential pipeline emits Const:fooService → HAS_METHOD → Method:getUser AND Function:caller → CALLS → Method:getUser. This is the directly testable equivalent of issue TypeScript: export const fooService = { async method() {} } produces zero edges (no HAS_PROPERTY, no incoming CALLS) #1358's production symptom — impact upstream traversal is a graph walk over CALLS, so the edge's existence is the impact-API proof.
  • Test B: worker-mode pipeline (real createWorkerPool via workerThresholdsForTest) emits identical HAS_METHOD and CALLS edge sets to sequential. Gated on hasDistWorker.
  • Test C: local-scoped object literal inside a function body emits zero false-positive HAS_METHOD edges and no ownerId on the method node.
  • Test D: nested object literal binds only the top-level method to its outer Const; the nested method is not attributed (safe under-approximation, preserves pre-feat(ingestion): Link object literal methods to exported bindings #1718 behavior for that edge case).

All assertions use exact .toBe(N) / .toEqual([...]) per the project's testing conventions — no toBeGreaterThanOrEqual, no .toContain shortcuts.

F4 — Dead code (fixed). ObjectLiteralBindingInfo.ownerName removed; both call sites only read ownerId.

F5 — let → Const label (pre-existing, deferred). Acknowledged in the helper's docstring; preserved as-is. Tree-sitter lexical_declaration capture design predates this PR; changing it requires a separate audit of downstream label consumers.

F6 — CI auth gate. This commit pushes from a fork, so the same fork-PR authorization gate still applies. Maintainer can authorize the workflow runs to verify on CI.

Local verification

• tsc --noEmit: clean.
• npm run build: produces fresh dist/parse-worker.js (needed by worker-parity test B).
• vitest run test/integration/: 2958 passed, 13 skipped (skips are pre-existing platform/CI-specific gates).
• vitest run test/unit/: 6056 passed, 30 skipped.
• Targeted regression sweep on has-method.test.ts + cross-file-binding.test.ts: 100/100 pass.

Files changed

src/core/ingestion/parsing-processor.ts
src/core/ingestion/scope-resolution/graph-bridge/node-lookup.ts
src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts
src/core/ingestion/scope-resolution/scope/walkers.ts
src/core/ingestion/utils/ast-helpers.ts
src/core/ingestion/workers/parse-worker.ts
test/integration/ast-helpers-object-literal-binding.test.ts (new)
test/integration/object-literal-owner-resolution.test.ts   (new)

[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
9451	9450	0	1	416s

✅ All 9450 tests passed

1 test(s) skipped — expand for details

• buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	78.15%	31116/39811	N/A%	—	🟢 ███████████████░░░░░
Branches	66.69%	19817/29711	N/A%	—	🟢 █████████████░░░░░░░
Functions	82.19%	3171/3858	N/A%	—	🟢 ████████████████░░░░
Lines	81.52%	28068/34428	N/A%	—	🟢 ████████████████░░░░

---

_{📋 View full run · Generated by CI}

[github-actions]

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

---

PR #1718 Adversarial Production-Readiness Review

Reviewer: Adversarial production-readiness review — ingestion, AST parsing, graph relationships, scope-resolution, worker parity, false-positive suppression.

Task List

• Post initial comment
• Read repo docs (DoD.md, AGENTS.md, GUARDRAILS.md, CONTRIBUTING.md)
• Verify branch hygiene (git log, merge commit inspection)
• Inspect all 9 changed files line-by-line
• Inspect related files (tree-sitter-queries, scope-extractor, language providers, graph-bridge ids/edges, walkers, symbol-table, method-registry, semantic-model)
• Search for key terms (findObjectLiteralBindingInfo, ownerId, HAS_METHOD, findValueBindingInScope, tryEmitEdge, isLinkableLabel, etc.)
• Verify CI status
• Run Unicode/hidden-character hygiene checks
• Assess owner ID label consistency (const/let/var)
• Assess worker path parity
• Assess scope-resolution value-receiver bridge (false positives, direct edge emission)
• Assess issue TypeScript: export const fooService = { async method() {} } produces zero edges (no HAS_PROPERTY, no incoming CALLS) #1358 scope (class-instance pattern, ACCESSES edges)
• Formulate findings and final verdict

---

Review Bar for This PR

DoD.md §§2.1–2.7 (correctness, placement, design, contracts, security, performance, tests). PR-specific bar from the trigger: all 12 items — owner discovered only for importable file-scope exported bindings; ownerId in both paths; HAS_METHOD in both paths; scope-resolution CALLS resolution for imported receivers; end-to-end CALLS edge as issue #1358 proof; negative cases for local/nested/block-scoped/IIFE patterns; TS/JS parity; existing resolution not regressed; direct edge emission preserves graph bridge invariants; CI passes; branch hygiene acceptable; no unrelated area touched.

---

Problem Being Solved

Issue #1358: export const fooService = { async getUser(id) {} } produced Method:getUser with no incoming CALLS edges and no ownership relationship, causing gitnexus_impact to return false LOW risk even for production chokepoints with live call sites. Both the structural (HAS_METHOD) and caller-resolution (CALLS) gaps needed fixing.

---

Current PR State

Two feature commits: 9a27e6c (original fix, luyua9) and e9531ce (review follow-up, magyargergo), plus 4 merge-from-main commits. 9 changed files total. Head SHA: e9531ce. All CI: 9/9 completed, 9/9 success.

---

Merge Status and Mergeability

CI: all green. Verified: main CI run (26211101572) — typecheck tsc --noEmit, unit tests (9379 passed, 1 skipped), E2E; scope-parity (TS + JS parity gates); Gitleaks; CodeQL; Dependency Review; Docker build; PR description check. No pending or failing checks. Merge is not blocked by CI. Conflict/mergeability status: branch is merge-safe based on clean CI and the verified merge commits.

---

Repository History Considered

• Issue TypeScript: export const fooService = { async method() {} } produces zero edges (no HAS_PROPERTY, no incoming CALLS) #1358 — primary bug report and acceptance criteria (object-literal shorthand methods, zero edges)
• Issue Meta: Pure Scope-Based Resolution in the Semantic Model #909 / RFC Meta: Pure Scope-Based Resolution in the Semantic Model #909 — pure scope-resolution migration; verified both TypeScript and JavaScript are in MIGRATED_LANGUAGES in registry-primary-flag.ts:72,77. This confirms the scope-resolution path is the production call-resolution path for both languages. The legacy DAG not being touched in call-processor.ts is correct per architecture.
• Issue TypeScript: export const fooService = { async method() {} } produces zero edges (no HAS_PROPERTY, no incoming CALLS) #1358 class-instance sub-case — class FooService { method() {} }; export const fooService = new FooService();. PR explicitly scopes to object-literal shorthand only. This sub-case remains a separate issue.
• PR fix(workers): resilient + zero-copy ingestion worker pool — prevent analyze hangs on TS-root-scale loads #1693 (worker resilience) — merged into main and brought in via merge commit 3960648; conflict resolution in parsing-processor.ts verified correct.

---

Branch Hygiene Assessment

Acceptable. Four merge-from-main commits (8721f61, bbc9568, bafeaa6, 3960648) bring in unrelated main changes (worker resilience, eval-server fixes, FTS repair, etc.). Inspected merge commit 3960648 against both parents: the conflict in parsing-processor.ts was resolved cleanly — loggedQuarantineByPool and objectLiteralOwnerInfo code coexist correctly. The PR-specific diffs (git diff parent2..merge for each PR-specific file) match the intended changes exactly. No unrelated churn in the 9 PR-specific files.

---

Understanding of the Change

Structural fix (legacy DAG path — ast-helpers.ts, parsing-processor.ts, parse-worker.ts):

• New helper findObjectLiteralBindingInfo does a two-phase AST walk from a method_definition node. Phase A tracks objectDepth and finds the enclosing variable_declarator (returns null for nested literals, function/class containers, and IIFEs). Phase B walks the declarator's ancestors and returns null if any function, class, or block-statement container precedes program/export_statement. Returns { ownerId: generateId(label, filePath + ':' + name) }.
• Both parsing-processor.ts:793 and parse-worker.ts:2330–2331 now set ownerId on the symbol table entry for classless Method nodes to objectLiteralOwnerInfo?.ownerId.
• Both emit HAS_METHOD from the owning Const/Variable graph node to the method node.

Scope-resolution fix (receiver-bound-calls.ts, walkers.ts, node-lookup.ts):

• New Case 5 value-receiver bridge: after Cases 1–4 fail for lowercase receivers like fooService, findValueBindingInScope resolves the receiver as a non-class-like Variable def. resolveDefGraphId translates this to the canonical Const:filePath:name graph node ID (via simple-key fallback). model.methods.lookupAllByOwner(ownerGraphId, memberName) finds the method because ownerId matches. Edge emitted directly with picked.nodeId (= canonical graph node ID from the legacy parse phase).
• isLinkableLabel extended with 'Const' so buildGraphNodeLookup registers Const nodes.
• New findValueBindingInScope walker mirrors findClassBindingInScope but accepts non-class-like, non-callable defs.

---

Findings

---

Finding 1 — NOTED: Scope-Resolution Type Label Mismatch (Latent, Non-Blocking)

Severity: LOW

Evidence (verified):

scope-extractor.ts:696 maps both 'const' and 'variable' capture kinds to the label 'Variable' via normalizeNodeLabel. So a scope-resolution def for export const fooService = { ... } has def.type = 'Variable'. But findObjectLiteralBindingInfo generates the ownerId as generateId('Const', 'filePath:name') — using 'Const' because variable_declarator.parent.type === 'lexical_declaration'. This means:

In resolveDefGraphId(filePath, valueDef, nodeLookup):

• valueDef.qualifiedName = 'fooService', valueDef.type = 'Variable'
• Qualified key tried: <q>:filePath::Variable::fooService → MISS (graph node is Const)
• Falls back to simple key: filePath::fooService → HIT → returns Const:filePath:fooService ✓

The lookup succeeds via the simple-key fallback, and the owner graph ID resolves correctly. Verified by the integration test passing end-to-end. This is accidental correctness — if two bindings share the same simple name in the same file, or if the simple-key fallback were removed, this could silently break. However it works correctly under current codebase conventions.

Recommended follow-up: Consider either making normalizeNodeLabel('const') return 'Const' rather than 'Variable' (broader change requiring audit), or adding an explicit 'Variable'-to-'Const' translation step in Case 5 when the resolved value def has type === 'Variable' and the owner graph ID ends with Const:.

Blocks merge: NO. Integration tests prove correctness on the actual path.

---

Finding 2 — NOTED: Case 5 Hardcodes type: 'CALLS' for All Site Kinds (Latent, Non-Blocking)

Severity: LOW

Evidence (verified):

receiver-bound-calls.ts:762 emits type: 'CALLS' unconditionally in Case 5, regardless of site.kind. For read/write sites, tryEmitEdge would emit type: 'ACCESSES' (edges.ts:36–37). The dedup key also uses the CALLS: prefix hardcoded.

Why this is benign in practice: pickOverload checks model.methods.lookupAllByOwner first. For object-literal methods, only Method nodes are registered under the owner's ownerId (because objectLiteralOwnerInfo fires only when nodeLabel === 'Method' per parsing-processor.ts:536). For read/write access to a non-method property of an object literal, model.fields.lookupFieldByOwner returns undefined (no HAS_PROPERTY edges were added for non-method properties). So picked is undefined and Case 5 skips. No CALLS edge is emitted for ACCESSES scenarios.

Recommended follow-up: Use mapReferenceKindToEdgeType(site.kind) and match the dedup key prefix to the actual edge type, or gate Case 5 on site.kind === 'call' only if ACCESSES semantics for object-literal properties are not needed.

Blocks merge: NO. Harmless under current data model.

---

Finding 3 — INFO: JavaScript .js Files Have No Explicit Integration Test

Severity: INFO

Evidence (verified):

object-literal-owner-resolution.test.ts uses .ts fixture files only. The fix applies to both TypeScript and JavaScript because:

1. findObjectLiteralBindingInfo uses tree-sitter AST nodes that are identical in JS and TS grammars (method_definition, object, variable_declarator)
2. emitReceiverBoundCalls (Case 5) is language-agnostic, called from the shared pipeline/run.ts:353
3. registry-primary-flag.ts:77 confirms JavaScript is in MIGRATED_LANGUAGES
4. The CI scope-parity gate for JavaScript (scope-parity / javascript parity) passed ✅

Blocks merge: NO. CI scope-parity gate covers JavaScript at the parity level.

---

Finding 4 — INFO: Issue #1358 Class-Instance Sub-Case Out of Scope

Severity: INFO

Evidence: Issue #1358 mentions both export const fooService = { method() {} } and class FooService { method() {} }; export const fooService = new FooService(). The PR description explicitly scopes to "object-literal method shorthand" and tests only that pattern. The class-instance singleton sub-case is not fixed and not claimed as fixed. No test covers it.

Recommended action: Add a tracking comment in the PR linking to a follow-up issue for the class-instance singleton pattern, or update the PR description to explicitly document the remaining sub-case. Not required for this PR to merge.

Blocks merge: NO. Partial fix is documented. The PR does not claim to fully close #1358 for the class-instance pattern.

---

Finding 5 — CONFIRMED CLEAN: Direct Edge Emission Invariants

Severity: CONFIRMED OK

Case 5's direct graph.addRelationship (lines 758–765) was flagged as a candidate bypass of graph bridge invariants. Verified:

• Graph node IDs: callerGraphId comes from resolveCallerGraphId (same as tryEmitEdge). picked.nodeId is the symbol-table nodeId passed by parsing-processor.ts (= generateId(nodeLabel, filePath + ':' + name)) — the canonical legacy-phase graph node ID.
• Dedup key format: CALLS:${callerGraphId}->${picked.nodeId}:${line}:${col} matches tryEmitEdge's format for call sites exactly (${edgeType}:${callerGraphId}->${targetGraphId}:${line}:${col}).
• Edge ID: rel:${dedupKey} matches tryEmitEdge's rel:${dedupKey}.
• handledSites: set on the same path, preventing emitReferencesViaLookup double-emission ✓
• seen: prevents within-pass duplication ✓

The reason for not using tryEmitEdge directly (stated in the comment at lines 729–735) is that object-literal method defs don't have class-qualified qualifiedName, so resolveDefGraphId would fall through to simple-key. This works but the comment claims it "cannot round-trip" which is slightly overstated — it could, via the simple-key path. The direct emission is semantically equivalent for call sites and is correct.

Blocks merge: NO.

---

PR-Specific Assessment Sections

Lane 1 — Parser Ownership

Status: PASS.

findObjectLiteralBindingInfo two-phase walk inspected line-by-line (ast-helpers.ts:468–528). Phase A correctly increments objectDepth on object nodes, exits on variable_declarator with depth >= 1 (rejecting depth > 1), and returns null on function/class containers before the declarator (IIFEs). Phase B correctly rejects function, class, and all BLOCK_SCOPE_BOUNDARY_TYPES ancestors between the declarator and program/export_statement. The ownerLabel heuristic (variable_declaration → 'Variable', otherwise → 'Const') matches getLabelFromCaptures and the tree-sitter query capture names (@definition.const for lexical_declaration, @definition.variable for variable_declaration).

Tested: 13 integration cases in ast-helpers-object-literal-binding.test.ts against a real tree-sitter TypeScript parser. All boundary conditions covered.

Residual risk: Object.assign({}, { method() {} }) pattern would falsely attribute (Phase A finds the declarator via an intermediate call_expression). Not listed in the PR-specific DoD and not a new regression relative to pre-PR behavior. Acceptable known limitation.

Lane 2 — Graph Structural

Status: PASS.

Both parsing-processor.ts:813–826 and parse-worker.ts:2365–2378 emit HAS_METHOD using ownerIdForMemberEdge = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId ?? null. Edge IDs use generateId(memberEdgeType, ownerIdForMemberEdge + '->' + nodeId) — deterministic and dedup-safe. Reason field correctly identifies object-literal origin vs class-origin edges. Code is structurally symmetric between the two paths.

Lane 3 — Symbol-Table

Status: PASS.

parsing-processor.ts:793: ownerId: enclosingClassId ?? objectLiteralOwnerInfo?.ownerId ?? undefined ✓
parse-worker.ts:2330–2331: ...((enclosingClassId ?? objectLiteralOwnerInfo?.ownerId) ? { ownerId: ... } : {}) ✓

Both paths set ownerId to the canonical graph node ID. The method registry keys on ownerNodeId\0methodName, so model.methods.lookupAllByOwner(ownerGraphId, 'getUser') finds the method correctly when ownerGraphId = generateId('Const', 'filePath:fooService').

Lane 4 — Scope-Resolution

Status: PASS (with noted type-mismatch, Finding 1).

Case 5 flow verified end-to-end:

1. findValueBindingInScope (walkers.ts:230–261) correctly walks scopes including imported bindings via lookupBindingsAt — imported fooService is visible at the consumer's scope ✓
2. resolveDefGraphId succeeds via simple-key fallback (type mismatch noted in Finding 1 but correct in practice) ✓
3. model.methods.lookupAllByOwner(ownerGraphId, memberName) finds the method ✓
4. picked.nodeId = canonical graph node ID ✓
5. CALLS edge emitted with consistent dedup key and edge ID format ✓
6. handledSites prevents double-emission ✓

Lane 5 — Worker Parity

Status: PASS.

Sequential and worker code paths are structurally identical for objectLiteralOwnerInfo. Integration test B (object-literal-owner-resolution.test.ts:136–178) compares HAS_METHOD and CALLS edge sets between sequential and worker pipeline runs, asserting expect(workerEdges).toEqual(seqEdges). Gated on hasDistWorker (CI builds dist/parse-worker.js before running integration tests).

Lane 6 — Regression/False-Positive

Status: PASS.

Negative cases proven by tests:

• Local const inside function body → null (Phase B rejects function_declaration ancestor) ✓
• Local const inside arrow function → null (Phase A rejects arrow_function before declarator) ✓
• Local const inside class constructor → null (Phase A rejects class body container) ✓
• Nested object literal method → null (Phase A rejects objectDepth > 1) ✓
• Top-level method alongside nested literal → correctly binds to outer ✓
• Block-scoped inside if body → null (Phase B rejects statement_block) ✓
• Block-scoped inside for-of body → null ✓
• Block-scoped inside try body → null ✓
• IIFE-wrapped → null (Phase A rejects arrow_function before declarator) ✓
• Assignment expression (no declarator) → null without throwing ✓
• Integration test C: local literal in function body emits no HAS_METHOD and ownerId = undefined on the method node ✓
• Integration test D: nested literal binds only top-level method to outer, not the nested method ✓

Lane 7 — CI/Test

Status: PASS.

Tests cover the real runtime path — no mocks bypass ingestion or graph construction. Assertions use exact toBe/toEqual per DoD §2.7. object-literal-owner-resolution.test.ts:127–131 proves the CALLS edge exists (the canonical proof for issue #1358's impact-analysis fix). parsing-worker-fallback.test.ts:147–188 proves the structural assertion in sequential mode.

All 9 CI checks pass on the head commit. 9379/9379 tests passed (1 pre-existing skip on Ruby block parameter inference, unrelated).

Lane 8 — Branch Hygiene

Status: PASS.

Merge commit 3960648 (most recent, bringing in PR #1693 worker resilience) verified: parsing-processor.ts conflict resolved correctly — loggedQuarantineByPool from main and objectLiteralOwnerInfo from PR branch coexist without interference. No unrelated changes appear in the 9 PR-specific files when comparing git diff parent2..merge for each merge commit.

---

Back-and-Forth Avoided by Verifying

• Owner ID label consistency (const/let/var): verified getLabelFromCaptures, normalizeNodeLabel, tree-sitter queries, and findObjectLiteralBindingInfo all agree — lexical_declaration → 'Const', variable_declaration → 'Variable'. No mismatch for supported forms.
• resolveDefGraphId path for Const: verified it works via simple-key fallback despite the scope-resolution type being 'Variable'. Confirmed by passing integration test.
• picked.nodeId correctness: traced symbolTable.add(nodeId) → SymbolDefinition.nodeId → model.methods.lookupAllByOwner → picked.nodeId. All the same canonical graph node ID from generateId(nodeLabel, filePath + ':' + name).
• Worker dispatch threshold: workerThresholdsForTest: { minFiles: 1, minBytes: 1 } in test B forces worker mode for the 2-file fixture — confirmed the worker path actually runs.
• JavaScript coverage: receiver-bound-calls.ts is called from the shared pipeline, not from the TypeScript-specific scope-resolver. JS is covered generically.
• Merge commit conflict resolution: inspected git diff parent1..merge vs git diff parent2..merge for the files touched by the PR. Resolution is correct.

---

Open Questions

None. All verification points were resolvable from the repository.

---

Final Verdict

production-ready with minor follow-ups

The PR correctly and completely fixes the production symptom reported in issue #1358 for the export const fooService = { method() {} } pattern: HAS_METHOD edges are emitted in both the sequential and worker parse paths, ownerId is correctly set in the symbol table, and Case 5 in receiver-bound-calls.ts connects cross-file call sites to the object-literal method nodes, producing CALLS edges that make gitnexus_impact traversal accurate. All originally-blocking findings from the prior review (F1: caller resolution, F2: boundary guard, F3: test coverage, F4: dead code) are addressed and verified. The two noted findings (type-label mismatch that resolves via simple-key fallback; CALLS hardcoding for read/write sites) are benign under the current data model and do not affect the fix's correctness. CI is green across all 9 required gates with 9379 tests passing. Branch hygiene is acceptable.