refactor(ingestion): address code-review findings on object-literal owner resolution

Multi-agent code review on the prior commit surfaced 7 actionable findings,
all walked through and applied here. None change observable behavior for
issue #1358's fix; all harden correctness, predicate stability, and test
signal.

- #1 (P1 / 3-reviewer corroboration): Case 5 in receiver-bound-calls.ts no
  longer hand-builds graph.addRelationship + a dedup key. New
  tryEmitEdgeWithExplicitTargetId in edges.ts takes a pre-resolved target
  id (the canonical Method nodeId from the parser) and reuses every
  invariant of tryEmitEdge: dedup-key format, collapse-flag honoring,
  caller-id resolution, rel-id shape, mapReferenceKindToEdgeType for
  read/write ACCESSES. This also lands the adversarial reviewer's "F2"
  follow-up (hardcoded type: 'CALLS' for non-call sites) for free.

- #2 (P2 cross-reviewer): findValueBindingInScope's predicate inverted
  from denylist ("not class-like and not callable") to explicit allowlist
  matching reconcileOwnership's registration set:
  Const | Variable | Property | Static. Extracted as isOwnableValueLabel
  so future NodeLabel additions require an explicit opt-in.

- #6 (P2): walkScopeChain<T>() extracted; both findClassBindingInScope
  and findValueBindingInScope now route through it. Local scope.bindings
  are exhausted BEFORE lookupBindingsAt (imported/augmented) at every
  scope level — preserves JavaScript lexical scoping where a local const
  shadows an imported binding of the same name. Behavior was already
  correct in findClassBindingInScope but was implicit; now it is the
  walker's explicit, documented contract.

- #7 (P2): scope-walker duplication closed. findClassBindingInScope and
  findValueBindingInScope reduce to thin wrappers over walkScopeChain
  with their respective predicate. findClassBindingInScope keeps its
  qualifiedNames + dotted-name fallback tail.

- #3 (P2): parse-worker.ts hoists `const ownerId = enclosingClassId ??
  objectLiteralOwnerInfo?.ownerId` once before the symbol push, dropping
  the duplicated coalesce + `as string` cast. Matches the cast-free
  pattern at parsing-processor.ts:793. HAS_METHOD emit site reuses the
  same hoisted local.

- #4 (P2): object-literal-owner-resolution.test.ts Test A's CALLS-edge
  assertion no longer matches by name alone. .toEqual now pins the
  canonical target id (Method:src/service.ts:getUser#1 via generateId),
  confidence (0.85), and reason ('import-resolved'). A regression that
  emits the edge at confidence=0, with the wrong reason, or against a
  phantom Method node now fails the test.

- #5 (P2): worker-parity test adds a CI tripwire — when CI=1 and
  dist/parse-worker.js is missing, throw at module top with a clear
  message. Locally, skipIf(!hasDistWorker) keeps the fast-iteration
  experience; CI cannot pass with U3 (worker-path ownerId) unverified.

Verification: tsc --noEmit clean. Targeted regression sweep on
ast-helpers-object-literal-binding (13), object-literal-owner-resolution
(9), has-method (60), cross-file-binding (40) — 122/122 pass. Full unit
sweep: 6056/6056. Integration suite: 1 pre-existing Windows-flake in
worker-pool.test.ts (passes 28/28 in isolation) unrelated to this diff.

gitnexus/src/core/ingestion/scope-resolution/graph-bridge/edges.ts

@@ -98,3 +98,54 @@ export function tryEmitEdge(
   });
   return true;
 }
+
+/**
+ * Variant of `tryEmitEdge` that takes a pre-resolved target graph id
+ * instead of resolving it from a `SymbolDefinition`. Used by the
+ * value-receiver-owner bridge (`receiver-bound-calls.ts` Case 5) where
+ * the picked owner-indexed method def carries no `qualifiedName` (object
+ * literals have no class owner to seed it) and therefore cannot
+ * round-trip through `resolveDefGraphId`. The def's `nodeId` IS the
+ * canonical graph node id (written by the parse phase), so the caller
+ * passes it directly.
+ *
+ * All other invariants of `tryEmitEdge` apply: dedup key shape, collapse
+ * flag honoring, edge-type mapping, caller-id resolution.
+ */
+export function tryEmitEdgeWithExplicitTargetId(
+  graph: KnowledgeGraph,
+  scopes: ScopeResolutionIndexes,
+  nodeLookup: GraphNodeLookup,
+  site: {
+    readonly inScope: ScopeId;
+    readonly atRange: { startLine: number; startCol: number };
+    readonly kind: string;
+  },
+  targetGraphId: string,
+  reason: string,
+  seen: Set<string>,
+  confidence = 0.85,
+  collapseByCallerTarget = false,
+): boolean {
+  const callerGraphId = resolveCallerGraphId(site.inScope, scopes, nodeLookup);
+  const edgeType = mapReferenceKindToEdgeType(site.kind as Reference['kind']);
+  if (callerGraphId === undefined) return false;
+  if (edgeType === undefined) return false;
+
+  const useCollapsed = collapseByCallerTarget && edgeType === 'CALLS';
+  const dedupKey = useCollapsed
+    ? `${edgeType}:${callerGraphId}->${targetGraphId}`
+    : `${edgeType}:${callerGraphId}->${targetGraphId}:${site.atRange.startLine}:${site.atRange.startCol}`;
+  if (seen.has(dedupKey)) return false;
+  seen.add(dedupKey);
+
+  graph.addRelationship({
+    id: `rel:${dedupKey}`,
+    sourceId: callerGraphId,
+    targetId: targetGraphId,
+    type: edgeType,
+    confidence,
+    reason,
+  });
+  return true;
+}

gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts

@@ -54,9 +54,9 @@ import {
   findValueBindingInScope,
   isClassLike,
 } from '../scope/walkers.js';
-import { tryEmitEdge } from '../graph-bridge/edges.js';
+import { tryEmitEdge, tryEmitEdgeWithExplicitTargetId } from '../graph-bridge/edges.js';
 import { resolveCompoundReceiverClass } from '../passes/compound-receiver.js';
-import { resolveCallerGraphId, resolveDefGraphId } from '../graph-bridge/ids.js';
+import { resolveDefGraphId } from '../graph-bridge/ids.js';
 import {
   narrowOverloadCandidates,
   isOverloadAmbiguousAfterNormalization,
@@ -730,9 +730,10 @@ export function emitReceiverBoundCalls(
       //
       // Object-literal methods do not carry a `qualifiedName` (no class
       // owner to seed it), so the picked def cannot round-trip through
-      // `tryEmitEdge` → `resolveDefGraphId`. We emit directly using
-      // `picked.nodeId` (already the canonical graph node id, written by
-      // the legacy parse phase).
+      // `tryEmitEdge` → `resolveDefGraphId`. We use
+      // `tryEmitEdgeWithExplicitTargetId` instead, passing `picked.nodeId`
+      // directly — same dedup-key shape, collapse-flag honoring, and
+      // caller resolution as `tryEmitEdge`.
       const valueDef = findValueBindingInScope(site.inScope, receiverName, scopes);
       if (valueDef !== undefined) {
         const ownerGraphId =
@@ -743,31 +744,27 @@ export function emitReceiverBoundCalls(
           continue;
         }
         if (picked !== undefined) {
-          const callerGraphId = resolveCallerGraphId(site.inScope, scopes, nodeLookup);
-          if (callerGraphId !== undefined) {
-            const reason =
-              site.kind === 'write' || site.kind === 'read'
-                ? site.kind
-                : picked.filePath !== parsed.filePath
-                  ? 'import-resolved'
-                  : 'global';
-            const confidence = site.kind === 'write' || site.kind === 'read' ? 1.0 : 0.85;
-            const dedupKey = `CALLS:${callerGraphId}->${picked.nodeId}:${site.atRange.startLine}:${site.atRange.startCol}`;
-            if (!seen.has(dedupKey)) {
-              seen.add(dedupKey);
-              graph.addRelationship({
-                id: `rel:${dedupKey}`,
-                sourceId: callerGraphId,
-                targetId: picked.nodeId,
-                type: 'CALLS',
-                confidence,
-                reason,
-              });
-              emitted++;
-            }
-            handledSites.add(siteKey);
-            continue;
-          }
+          const reason =
+            site.kind === 'write' || site.kind === 'read'
+              ? site.kind
+              : picked.filePath !== parsed.filePath
+                ? 'import-resolved'
+                : 'global';
+          const confidence = site.kind === 'write' || site.kind === 'read' ? 1.0 : 0.85;
+          const ok = tryEmitEdgeWithExplicitTargetId(
+            graph,
+            scopes,
+            nodeLookup,
+            site,
+            picked.nodeId,
+            reason,
+            seen,
+            confidence,
+            collapse,
+          );
+          if (ok) emitted++;
+          handledSites.add(siteKey);
+          continue;
         }
       }
     }

gitnexus/src/core/ingestion/scope-resolution/scope/walkers.ts

@@ -165,28 +165,9 @@ export function findClassBindingInScope(
   receiverName: string,
   scopes: ScopeResolutionIndexes,
 ): SymbolDefinition | undefined {
-  let currentId: ScopeId | null = startScope;
-  const visited = new Set<ScopeId>();
-  while (currentId !== null) {
-    if (visited.has(currentId)) return undefined;
-    visited.add(currentId);
-    const scope = scopes.scopeTree.getScope(currentId);
-    if (scope === undefined) return undefined;
+  const local = walkScopeChain(startScope, receiverName, scopes, (def) => isClassLike(def.type));
+  if (local !== undefined) return local;
 
-    const localBindings = scope.bindings.get(receiverName);
-    if (localBindings !== undefined) {
-      for (const b of localBindings) {
-        if (isClassLike(b.def.type)) return b.def;
-      }
-    }
-
-    const importedBindings = lookupBindingsAt(currentId, receiverName, scopes);
-    for (const b of importedBindings) {
-      if (isClassLike(b.def.type)) return b.def;
-    }
-
-    currentId = scope.parent;
-  }
   // Fallback for languages (Go) where namespace-style imports don't
   // create scope bindings: resolve via QualifiedNameIndex. Only fires
   // when the scope-chain walk found nothing; single-match wins.
@@ -212,17 +193,31 @@ export function findClassBindingInScope(
 }
 
 /**
- * Look up a value-binding (non-class-like, non-callable) by name in
+ * Predicate for value-receiver bridge: the labels for which
+ * `reconcileOwnership` registers methods/fields under the def's
+ * `nodeId` as the `ownerId`. Explicit allowlist so future NodeLabel
+ * additions (Module, Namespace, TypeAlias, EnumMember, etc.) do NOT
+ * silently widen the bridge — adding a new ownerable label requires
+ * touching both this predicate and `reconcileOwnership`.
+ *
+ * See: `scope-resolution/pipeline/reconcile-ownership.ts` Property /
+ * Variable / Const / Static registration block.
+ */
+export function isOwnableValueLabel(t: string): boolean {
+  return t === 'Const' || t === 'Variable' || t === 'Property' || t === 'Static';
+}
+
+/**
+ * Look up a value-binding (Const/Variable/Property/Static) by name in
  * the given scope's chain. Used by the value-receiver-owner bridge
  * for object-literal services such as:
  *
  *   export const fooService = { getUser(id) {...} };
  *
  * where `fooService` is a `Const`/`Variable` whose `nodeId` is the
- * `ownerId` of the member method but where neither `findClassBindingInScope`
- * (rejects non-class-like) nor `findReceiverTypeBinding` (no typeBinding for
- * an unannotated literal) finds it. Returns the first non-class-like,
- * non-callable binding match.
+ * `ownerId` of the member method. Neither `findClassBindingInScope`
+ * (rejects non-class-like) nor `findReceiverTypeBinding` (no typeBinding
+ * for an unannotated literal) finds it.
  *
  * Mirrors `findClassBindingInScope` exactly; only the accepted def-type
  * predicate differs.
@@ -231,6 +226,27 @@ export function findValueBindingInScope(
   startScope: ScopeId,
   receiverName: string,
   scopes: ScopeResolutionIndexes,
+): SymbolDefinition | undefined {
+  return walkScopeChain(startScope, receiverName, scopes, (def) => isOwnableValueLabel(def.type));
+}
+
+/**
+ * Generic scope-chain walker. Walks from `startScope` toward the root,
+ * consulting both the local `scope.bindings` channel and the dual-source
+ * `lookupBindingsAt` view (finalized + augmented). At each scope, local
+ * bindings are exhausted BEFORE imported/augmented bindings — preserves
+ * JavaScript-style lexical scoping where a local `const x` shadows an
+ * imported `x` of the same name.
+ *
+ * Returns the first binding `def` matching `predicate`. Cycles in the
+ * scope graph terminate the walk (defensive — should not occur in
+ * well-formed inputs).
+ */
+function walkScopeChain(
+  startScope: ScopeId,
+  name: string,
+  scopes: ScopeResolutionIndexes,
+  predicate: (def: SymbolDefinition) => boolean,
 ): SymbolDefinition | undefined {
   let currentId: ScopeId | null = startScope;
   const visited = new Set<ScopeId>();
@@ -240,19 +256,18 @@ export function findValueBindingInScope(
     const scope = scopes.scopeTree.getScope(currentId);
     if (scope === undefined) return undefined;
 
-    const isValueLike = (t: string): boolean =>
-      !isClassLike(t) && t !== 'Function' && t !== 'Method' && t !== 'Constructor';
-
-    const localBindings = scope.bindings.get(receiverName);
+    // Local first: a `const x` in this scope shadows any imported `x`.
+    const localBindings = scope.bindings.get(name);
     if (localBindings !== undefined) {
       for (const b of localBindings) {
-        if (isValueLike(b.def.type)) return b.def;
+        if (predicate(b.def)) return b.def;
       }
     }
 
-    const importedBindings = lookupBindingsAt(currentId, receiverName, scopes);
+    // Then imported/augmented bindings — only consulted when no local match.
+    const importedBindings = lookupBindingsAt(currentId, name, scopes);
     for (const b of importedBindings) {
-      if (isValueLike(b.def.type)) return b.def;
+      if (predicate(b.def)) return b.def;
     }
 
     currentId = scope.parent;

gitnexus/src/core/ingestion/workers/parse-worker.ts

@@ -2311,6 +2311,7 @@ const processFileGroup = (
       });
 
       // enclosingClassId already computed above (before nodeId generation)
+      const ownerId = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId;
 
       result.symbols.push({
         filePath: file.path,
@@ -2327,9 +2328,7 @@ const processFileGroup = (
         ...(classTemplateArguments !== undefined && classTemplateArguments.length > 0
           ? { templateArguments: classTemplateArguments }
           : {}),
-        ...((enclosingClassId ?? objectLiteralOwnerInfo?.ownerId)
-          ? { ownerId: (enclosingClassId ?? objectLiteralOwnerInfo?.ownerId) as string }
-          : {}),
+        ...(ownerId !== undefined ? { ownerId } : {}),
         visibility: methodProps.visibility as string | undefined,
         isStatic: methodProps.isStatic as boolean | undefined,
         isReadonly: methodProps.isReadonly as boolean | undefined,
@@ -2362,12 +2361,11 @@ const processFileGroup = (
       });
 
       // ── HAS_METHOD / HAS_PROPERTY: link member to enclosing class ──
-      const ownerIdForMemberEdge = enclosingClassId ?? objectLiteralOwnerInfo?.ownerId ?? null;
-      if (ownerIdForMemberEdge) {
+      if (ownerId !== undefined) {
         const memberEdgeType = nodeLabel === 'Property' ? 'HAS_PROPERTY' : 'HAS_METHOD';
         result.relationships.push({
-          id: generateId(memberEdgeType, `${ownerIdForMemberEdge}->${nodeId}`),
-          sourceId: ownerIdForMemberEdge,
+          id: generateId(memberEdgeType, `${ownerId}->${nodeId}`),
+          sourceId: ownerId,
           targetId: nodeId,
           type: memberEdgeType,
           confidence: 1.0,

gitnexus/test/integration/object-literal-owner-resolution.test.ts

@@ -50,6 +50,19 @@ const DIST_WORKER = path.resolve(
 );
 const hasDistWorker = fs.existsSync(DIST_WORKER);
 
+// CI tripwire: worker-parity test (Test B below) silently skips when
+// `dist/parse-worker.js` is missing. That's fine locally — devs may not
+// have run `npm run build` — but on CI a missing dist would mean U3
+// (worker-path ownerId emission) is unverified. Fail hard so a missing
+// dist surfaces as a red build, not a green test with a silent skip.
+// Locally, run `npm run build` before this suite to exercise worker mode.
+if (!hasDistWorker && process.env.CI) {
+  throw new Error(
+    'dist/parse-worker.js missing on CI — worker-parity test would silently skip. ' +
+      'Ensure the build runs before this suite.',
+  );
+}
+
 /** Materialise a tiny fixture repo on disk. Returns the absolute repo root. */
 function writeFixture(files: Record<string, string>): string {
   const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gnx-objlit-'));
@@ -124,10 +137,26 @@ describe('object-literal owner resolution — sequential pipeline (PR #1718)', (
     expect(fooServiceNode!.id).toBe(expectedNodeId);
   });
 
-  it('emits a CALLS edge from caller to getUser (issue #1358 fix)', () => {
+  it('emits a CALLS edge from caller to getUser with the expected target/confidence/reason (issue #1358 fix)', () => {
     const calls = getRelationships(result, 'CALLS');
-    const callerToGetUser = calls.filter((e) => e.source === 'caller' && e.target === 'getUser');
-    expect(callerToGetUser.length).toBe(1);
+    const callerToGetUser = calls
+      .filter((e) => e.source === 'caller' && e.target === 'getUser')
+      .map((e) => ({
+        targetId: e.rel.targetId,
+        confidence: e.rel.confidence,
+        reason: e.rel.reason,
+      }));
+
+    // The Method node id encodes arity disambiguation (#1 = one-arity overload).
+    // Pin the canonical id so a regression that targets a phantom node fails.
+    const expectedTargetId = generateId('Method', 'src/service.ts:getUser#1');
+    expect(callerToGetUser).toEqual([
+      {
+        targetId: expectedTargetId,
+        confidence: 0.85,
+        reason: 'import-resolved',
+      },
+    ]);
   });
 });