fix(scope-resolution): parse def coordinates after file paths

Anchor coordinate parsing to the known file path so coordinate-like path fragments and private symbol names cannot corrupt closure attribution.

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

@@ -34,6 +34,7 @@ import {
 } from '../../utils/callable-labels.js';
 import { templateConstraintsIdTag } from '../../utils/template-arguments.js';
 import { parameterShapeIdTag } from '../../utils/method-props.js';
+import { definitionIdPosition } from '../utils/definition-id.js';
 /**
  * Labels that may legitimately ANCHOR a CALLS/ACCESSES edge as the
  * source ("caller"). A Variable / Property can be the TARGET of an
@@ -99,9 +100,9 @@ function scopeIsCallableBody(
   range: { startLine: number; startCol: number },
   def: SymbolDefinition,
 ): boolean {
-  const m = def.nodeId.match(/#(\d+):(\d+):/);
-  if (m === null) return false;
-  return Number(m[1]) === range.startLine && Number(m[2]) === range.startCol;
+  const position = definitionIdPosition(def.nodeId, def.filePath);
+  if (position === undefined) return false;
+  return position.line === range.startLine && position.column === range.startCol;
 }
 
 /** Pick the callable that owns `atRange` when multiple overloads share a class scope. */
@@ -165,12 +166,8 @@ function pickCallerCallableDef(
  * Extract the 1-based declaration line from a scope-resolution def id.
  * Shape: `def:<filePath>#<line>:<col>:<...>`; `undefined` when it doesn't match.
  */
-function defStartLine(nodeId: string | undefined): number | undefined {
-  if (nodeId === undefined) return undefined;
-  const m = nodeId.match(/#(\d+):(\d+):/);
-  if (m === null) return undefined;
-  const line = Number(m[1]);
-  return Number.isFinite(line) ? line : undefined;
+function defStartLine(nodeId: string | undefined, filePath: string): number | undefined {
+  return definitionIdPosition(nodeId, filePath)?.line;
 }
 
 /**
@@ -221,7 +218,7 @@ export function resolveDefGraphId(
     // without either side having to model the scope chain. Node ids are
     // 0-based, def ids 1-based. An `AMBIGUOUS_POSITION` tombstone (two
     // callables on one line) falls through to the name-based keys below.
-    const line = defStartLine(def.nodeId);
+    const line = defStartLine(def.nodeId, filePath);
     if (line !== undefined && isPositionQualifiedLocalLabel(def.type)) {
       const simple = simpleNameOf(qn);
       const posHit = nodeLookup.get(positionKey(filePath, def.type, line - 1, simple));

gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts

@@ -26,6 +26,7 @@ import {
   simpleQualifiedName,
 } from '../graph-bridge/ids.js';
 import { resolveInheritanceBaseInScope } from '../scope/walkers.js';
+import { definitionIdPosition } from '../utils/definition-id.js';
 import { narrowOverloadCandidates } from './overload-narrowing.js';
 
 export const MAX_CALLABLE_VALUE_TARGETS = 32;
@@ -815,7 +816,7 @@ const withoutSigil = (name: string): string => name.replace(/^[$@]+/, '');
  */
 function valueBindingPositionKey(def: SymbolDefinition): string | undefined {
   if (!VALUE_BINDING_DEF_TYPES.has(def.type)) return undefined;
-  const line = def.nodeId.match(/#(\d+):(\d+):/)?.[1];
+  const line = definitionIdPosition(def.nodeId, def.filePath)?.line;
   const name = simpleQualifiedName(def);
   if (line === undefined || name === undefined) return undefined;
   return `${def.filePath}\0${line}\0${withoutSigil(name)}`;
@@ -921,8 +922,10 @@ function canonicalizeCallableDeclarations(
 }
 
 function definitionPosition(def: SymbolDefinition): string | undefined {
-  const match = def.nodeId.match(/#(\d+):(\d+):/);
-  return match === null ? undefined : `${def.filePath}\0${match[1]}\0${match[2]}`;
+  const position = definitionIdPosition(def.nodeId, def.filePath);
+  return position === undefined
+    ? undefined
+    : `${def.filePath}\0${position.line}\0${position.column}`;
 }
 
 function declarationSignatureCompatible(
@@ -1003,7 +1006,7 @@ function buildGraphCallableIndexes(graph: KnowledgeGraph): GraphCallableIndexes
 }
 
 function definitionAnchorKey(def: SymbolDefinition): string | undefined {
-  const line = def.nodeId.match(/#(\d+):(\d+):/)?.[1];
+  const line = definitionIdPosition(def.nodeId, def.filePath)?.line;
   const name = simpleName(def.qualifiedName);
   if (line === undefined || name === undefined) return undefined;
   return `${def.filePath}\0${def.type}\0${line}\0${name}`;
@@ -1437,8 +1440,8 @@ function dedupeDefinitions(defs: readonly SymbolDefinition[]): SymbolDefinition[
 }
 
 function definitionStartsAt(def: SymbolDefinition, line: number, column: number): boolean {
-  const match = def.nodeId.match(/#(\d+):(\d+):/);
-  return match !== null && Number(match[1]) === line && Number(match[2]) === column;
+  const position = definitionIdPosition(def.nodeId, def.filePath);
+  return position !== undefined && position.line === line && position.column === column;
 }
 
 function expandMemberTargets(

gitnexus/src/core/ingestion/scope-resolution/utils/definition-id.ts

@@ -0,0 +1,24 @@
+export interface DefinitionIdPosition {
+  readonly line: number;
+  readonly column: number;
+}
+
+/**
+ * Extract coordinates from `def:<filePath>#<line>:<column>:<type>:<name>`.
+ *
+ * File paths are embedded verbatim and may themselves contain fragments such
+ * as `#12:34:`, while names may themselves contain `#` (TypeScript private
+ * members). Anchor on the known file path so neither side can be mistaken for
+ * the declaration separator.
+ */
+export function definitionIdPosition(
+  nodeId: string | undefined,
+  filePath: string,
+): DefinitionIdPosition | undefined {
+  if (nodeId === undefined) return undefined;
+  const prefix = `def:${filePath}#`;
+  if (!nodeId.startsWith(prefix)) return undefined;
+  const match = /^(\d+):(\d+):/.exec(nodeId.slice(prefix.length));
+  if (match === null) return undefined;
+  return { line: Number(match[1]), column: Number(match[2]) };
+}

gitnexus/test/integration/closure-review-findings.test.ts

@@ -18,12 +18,16 @@ import { DIST_WORKER_URL, distWorkerExists } from '../helpers/worker-parse.js';
 vi.setConfig({ testTimeout: 90_000 });
 
 const describeIfWorkerBuilt = distWorkerExists() ? describe : describe.skip;
+const describeIfPathSupportsColon =
+  process.platform === 'win32' ? describe.skip : describeIfWorkerBuilt;
 
 /** Every CALLS edge in a one-file repo, as `src -> dst`, sorted. */
 const callEdges = async (filename: string, source: string): Promise<string[]> => {
   const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-review-'));
   try {
-    fs.writeFileSync(path.join(dir, filename), source, 'utf-8');
+    const filePath = path.join(dir, filename);
+    fs.mkdirSync(path.dirname(filePath), { recursive: true });
+    fs.writeFileSync(filePath, source, 'utf-8');
     const result = await runPipelineFromRepo(dir, () => {}, {
       workerPoolSize: 1,
       workerUrlForTest: DIST_WORKER_URL,
@@ -85,6 +89,20 @@ describeIfWorkerBuilt(
   },
 );
 
+describeIfPathSupportsColon('#2734 — def ids parse coordinates after the file path', () => {
+  it('a path containing #<line>:<column>: still attributes closure calls', async () => {
+    const edges = await callEdges(
+      'src/bugs#12:34:test/a.php',
+      '<?php\nfunction target($x) { return $x; }\nfunction outer() {\n' +
+        '  $handler = function ($x) { return target($x); };\n  return 1;\n}\n',
+    );
+
+    expect(edges).toEqual([
+      'Function:src/bugs#12:34:test/a.php:outer.$handler@3:2 -> Function:src/bugs#12:34:test/a.php:target',
+    ]);
+  });
+});
+
 describeIfWorkerBuilt(
   '#2699 review P1-2 — widening identity to values must not touch class members',
   () => {

gitnexus/test/unit/scope-resolution/callable-value-target-index.test.ts

@@ -33,7 +33,7 @@ const graphOf = (nodes: readonly StubNode[]): KnowledgeGraph => {
 
 /** `line` is 1-based, matching the definition-id convention. */
 const def = (type: string, filePath: string, qualifiedName: string, line: number) => ({
-  nodeId: `${filePath}#${line}:0:${qualifiedName}`,
+  nodeId: `def:${filePath}#${line}:0:${type}:${qualifiedName}`,
   type,
   filePath,
   qualifiedName,
@@ -69,7 +69,7 @@ describe('buildGraphTargetIndex — value bindings join by position', () => {
     // the same construct, so they share file, line and name.
     expect(
       targetsFor([node('Function', 'a.kt', 'handler', 3)], [def('Property', 'a.kt', 'handler', 3)]),
-    ).toEqual(['a.kt#3:0:handler => Function:a.kt:handler']);
+    ).toEqual(['def:a.kt#3:0:Property:handler => Function:a.kt:handler']);
   });
 
   it('REJECTS a value binding that merely shares a name with a callable elsewhere', () => {
@@ -106,12 +106,12 @@ describe('buildGraphTargetIndex — value bindings join by position', () => {
         [node('Function', 'a.php', '$save', 5), node('Function', 'a.php', 'save', 2)],
         [def('Variable', 'a.php', 'save', 5)],
       ),
-    ).toEqual(['a.php#5:0:save => Function:a.php:$save']);
+    ).toEqual(['def:a.php#5:0:Variable:save => Function:a.php:$save']);
   });
 
   it('keeps ordinary callable defs, which never take the positional path', () => {
     expect(
       targetsFor([node('Function', 'a.ts', 'fn', 1)], [def('Function', 'a.ts', 'fn', 1)]),
-    ).toEqual(['a.ts#1:0:fn => Function:a.ts:fn']);
+    ).toEqual(['def:a.ts#1:0:Function:fn => Function:a.ts:fn']);
   });
 });

gitnexus/test/unit/scope-resolution/definition-id.test.ts

@@ -0,0 +1,30 @@
+import { describe, expect, it } from 'vitest';
+import { definitionIdPosition } from '../../../src/core/ingestion/scope-resolution/utils/definition-id.js';
+
+describe('definitionIdPosition', () => {
+  it('reads coordinates after a path containing a coordinate-like fragment (#2734)', () => {
+    expect(
+      definitionIdPosition(
+        'def:src/bugs#12:34:test/a.php#4:2:Function:$handler',
+        'src/bugs#12:34:test/a.php',
+      ),
+    ).toEqual({ line: 4, column: 2 });
+  });
+
+  it('does not mistake a private-name hash for the path separator', () => {
+    expect(definitionIdPosition('def:src/a.ts#4:2:Method:#secret', 'src/a.ts')).toEqual({
+      line: 4,
+      column: 2,
+    });
+  });
+
+  it.each<[string | undefined, string]>([
+    [undefined, 'src/plain.php'],
+    ['def:src/plain.php', 'src/plain.php'],
+    ['def:src/plain.php#x:2:Function:handler', 'src/plain.php'],
+    ['def:src/plain.php#4:x:Function:handler', 'src/plain.php'],
+    ['def:src/other.php#4:2:Function:handler', 'src/plain.php'],
+  ])('rejects malformed or mismatched definition id %s', (nodeId, filePath) => {
+    expect(definitionIdPosition(nodeId, filePath)).toBeUndefined();
+  });
+});