fix(python): make multi-segment suffix fallback deterministic (#1253)

gitnexus/src/core/ingestion/languages/python/import-target.ts

@@ -88,7 +88,8 @@ export function resolvePythonImportTarget(
  * Precedence order:
  *  1. Workspace-root direct hit (`<pathLike>.py`, `<pathLike>/__init__.py`).
  *  2. Closest-ancestor match walking up from the importer's directory.
- *  3. Suffix fallback (first match).
+ *  3. Suffix fallback (deterministic: fewest path segments, then
+ *     lexicographic on the normalized path).
  *
  * Root wins over ancestor by construction — if both `services/sync.py` and
  * `backend/services/sync.py` exist, `backend/routers/cron.py`'s
@@ -129,18 +130,39 @@ function resolveAbsoluteFromFiles(
     }
   }
 
-  // Existing suffix-match fallback (preserved for monorepo/nested-repo
-  // layouts that don't share a directory ancestor with the importer).
+  // Suffix-match fallback (preserved for monorepo/nested-repo layouts
+  // that don't share a directory ancestor with the importer).
+  //
+  // Tie-break order when multiple files match the same suffix:
+  //  1. Fewest path segments (shorter, more canonical paths win — `lib/x.py`
+  //     beats `tooling/extras/x.py`).
+  //  2. Lexicographic order over the normalized path (final stable
+  //     tiebreak independent of file-set insertion order).
+  //
+  // Without an explicit tie-break the previous implementation returned
+  // the first match in `Set` iteration order, which depended on file
+  // ingestion order and produced non-deterministic edges across runs in
+  // multi-directory collision repos.
   const suffixFile = `/${directFile}`;
   const suffixPkg = `/${directPkg}`;
-  let suffixMatch: string | null = null;
+  const matches: { raw: string; norm: string }[] = [];
   for (const raw of allFilePaths) {
-    const f = raw.replace(/\\/g, '/');
-    if (suffixMatch === null && (f.endsWith(suffixFile) || f.endsWith(suffixPkg))) {
-      suffixMatch = raw;
+    const norm = raw.replace(/\\/g, '/');
+    if (norm.endsWith(suffixFile) || norm.endsWith(suffixPkg)) {
+      matches.push({ raw, norm });
     }
   }
-  return suffixMatch;
+  if (matches.length === 0) return null;
+  if (matches.length === 1) return matches[0].raw;
+  matches.sort((a, b) => {
+    const aDepth = a.norm.split('/').length;
+    const bDepth = b.norm.split('/').length;
+    if (aDepth !== bDepth) return aDepth - bDepth;
+    if (a.norm < b.norm) return -1;
+    if (a.norm > b.norm) return 1;
+    return 0;
+  });
+  return matches[0].raw;
 }
 
 /**

gitnexus/test/integration/resolvers/helpers.ts

@@ -12,6 +12,16 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly<Record<string, Readonly
   csharp: new Set([
     'emits the using-import edge App/Program.cs -> Models/User.cs through the scope-resolution path',
   ]),
+  python: new Set([
+    // Suffix-fallback lex tiebreak depends on the registry-primary
+    // resolver's deterministic sort. The legacy resolver returns the
+    // first match in `Set` iteration order, which is insertion-order
+    // dependent and not aligned with this guarantee. Backporting the
+    // sort to legacy is out of scope.
+    'picks the lexicographically smaller path on equal-depth ties',
+    'binds the call to alpha/services/sync.py, not omega',
+    'lex tiebreak still picks alpha/services/sync.py with reversed file-write order',
+  ]),
 };
 
 type ResolverParityEnv = Readonly<Record<string, string | undefined>>;

gitnexus/test/integration/resolvers/python.test.ts

@@ -1,13 +1,14 @@
 /**
  * Python: relative imports + class inheritance + ambiguous module disambiguation
  */
-import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { describe, expect, beforeAll, afterAll } from 'vitest';
 import path from 'path';
 import fs from 'node:fs';
 import os from 'node:os';
 import {
   FIXTURES,
   CROSS_FILE_FIXTURES,
+  createResolverParityIt,
   getRelationships,
   getNodesByLabel,
   getNodesByLabelFull,
@@ -16,6 +17,11 @@ import {
   type PipelineResult,
 } from './helpers.js';
 
+// Mirrors `csharp.test.ts`: skips tests in `LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.python`
+// when the legacy-resolver parity sweep runs (`REGISTRY_PRIMARY_PYTHON=0`). For the
+// default registry-primary CI run this is a transparent passthrough to vitest's `it`.
+const it = createResolverParityIt('python');
+
 function writeFixtureRepo(root: string, files: Record<string, string>): void {
   for (const [relPath, content] of Object.entries(files)) {
     const fullPath = path.join(root, relPath);
@@ -623,6 +629,225 @@ def handler():
   });
 });
 
+// ---------------------------------------------------------------------------
+// Suffix-fallback determinism: when both root + ancestor walk miss but the
+// suffix scan finds multiple candidates in unrelated trees, the resolver
+// must pick the same file regardless of file-set insertion order. The
+// previous implementation returned the first match in `Set` iteration
+// order, which depended on file ingestion order and produced flapping
+// edges across runs in multi-directory collision repos.
+//
+// Tie-break order: fewest path segments, then lexicographic.
+// ---------------------------------------------------------------------------
+
+describe('Python multi-segment resolution: suffix fallback determinism', () => {
+  let repoDir: string;
+  let result: PipelineResult;
+
+  beforeAll(async () => {
+    repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-determinism-'));
+    writeFixtureRepo(repoDir, {
+      // Importer's package. The `app/services/marker.py` file makes the
+      // `services` segment gate-pass under the ancestor-bounded
+      // `hasRepoCandidate` check, but `app/services/sync.py` is
+      // intentionally absent so the ancestor walk misses and the suffix
+      // fallback fires.
+      'app/services/marker.py': `def _marker(): return True
+`,
+      'app/main.py': `from services.sync import handler
+
+def boot():
+    return handler()
+`,
+      // Two suffix candidates outside the importer's ancestor tree.
+      // `lib/services/sync.py` has 3 path segments, the alternative has
+      // 4 — the deterministic pick is `lib/services/sync.py`.
+      'lib/services/sync.py': `def handler():
+    return "lib"
+`,
+      'tooling/extras/services/sync.py': `def handler():
+    return "tooling"
+`,
+    });
+    result = await runPipelineFromRepo(repoDir, () => {});
+  }, 60000);
+
+  afterAll(() => {
+    if (repoDir !== undefined) fs.rmSync(repoDir, { recursive: true, force: true });
+  });
+
+  it('picks the shortest-path candidate (lib/services/sync.py) and only that one', () => {
+    const imports = getRelationships(result, 'IMPORTS').filter(
+      (i) => i.sourceFilePath === 'app/main.py',
+    );
+
+    const libEdge = imports.find((i) => i.targetFilePath === 'lib/services/sync.py');
+    expect(libEdge).toBeDefined();
+
+    const toolingEdge = imports.find((i) => i.targetFilePath === 'tooling/extras/services/sync.py');
+    expect(toolingEdge).toBeUndefined();
+  });
+
+  it('binds the call to the deterministic pick, not the alternate copy', () => {
+    const calls = getRelationships(result, 'CALLS').filter(
+      (c) => c.sourceFilePath === 'app/main.py' && c.target === 'handler',
+    );
+    expect(calls.length).toBe(1);
+    expect(calls[0].targetFilePath).toBe('lib/services/sync.py');
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Lexicographic tiebreak: when two suffix candidates have the same
+// directory depth, the lexicographically smaller path wins. Without this,
+// equal-depth collisions would still depend on file-set insertion order.
+// ---------------------------------------------------------------------------
+
+describe('Python multi-segment resolution: suffix fallback lexicographic tiebreak', () => {
+  let repoDir: string;
+  let result: PipelineResult;
+
+  beforeAll(async () => {
+    repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-lex-tiebreak-'));
+    writeFixtureRepo(repoDir, {
+      // Same gate-passing pattern as the determinism test — non-init
+      // marker file makes the `services` segment satisfy
+      // `hasRepoCandidate` for an importer at `app/main.py`.
+      'app/services/marker.py': `def _marker(): return True
+`,
+      'app/main.py': `from services.sync import handler
+
+def boot():
+    return handler()
+`,
+      // Both candidates have depth 3, so directory-depth alone cannot
+      // disambiguate. Lexicographic order picks `alpha/...` over
+      // `omega/...` regardless of which file was ingested first.
+      'alpha/services/sync.py': `def handler():
+    return "alpha"
+`,
+      'omega/services/sync.py': `def handler():
+    return "omega"
+`,
+    });
+    result = await runPipelineFromRepo(repoDir, () => {});
+  }, 60000);
+
+  afterAll(() => {
+    if (repoDir !== undefined) fs.rmSync(repoDir, { recursive: true, force: true });
+  });
+
+  it('picks the lexicographically smaller path on equal-depth ties', () => {
+    const imports = getRelationships(result, 'IMPORTS').filter(
+      (i) => i.sourceFilePath === 'app/main.py',
+    );
+
+    const alphaEdge = imports.find((i) => i.targetFilePath === 'alpha/services/sync.py');
+    expect(alphaEdge).toBeDefined();
+
+    const omegaEdge = imports.find((i) => i.targetFilePath === 'omega/services/sync.py');
+    expect(omegaEdge).toBeUndefined();
+  });
+
+  it('binds the call to alpha/services/sync.py, not omega', () => {
+    const calls = getRelationships(result, 'CALLS').filter(
+      (c) => c.sourceFilePath === 'app/main.py' && c.target === 'handler',
+    );
+    expect(calls.length).toBe(1);
+    expect(calls[0].targetFilePath).toBe('alpha/services/sync.py');
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Insertion-order independence: re-runs the depth and lexicographic
+// scenarios with the candidate files written in reverse order. The
+// deterministic sort in `resolveAbsoluteFromFiles` should pick the same
+// winner regardless. If a future refactor accidentally drops the sort
+// and falls back to `Set` insertion order, these tests pin the
+// regression directly.
+// ---------------------------------------------------------------------------
+
+describe('Python multi-segment resolution: suffix fallback insertion-order independence', () => {
+  let depthRepoDir: string;
+  let lexRepoDir: string;
+  let depthResult: PipelineResult;
+  let lexResult: PipelineResult;
+
+  beforeAll(async () => {
+    // Depth scenario, files written in reverse order: tooling first, lib
+    // second. `writeFixtureRepo` iterates in object-property insertion
+    // order, and the pipeline scanner's directory traversal is also
+    // affected by mtime/inode order on most filesystems. The expected
+    // winner is still `lib/services/sync.py` (depth 3 < depth 4).
+    depthRepoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-determinism-rev-'));
+    writeFixtureRepo(depthRepoDir, {
+      'tooling/extras/services/sync.py': `def handler():
+    return "tooling"
+`,
+      'lib/services/sync.py': `def handler():
+    return "lib"
+`,
+      'app/services/marker.py': `def _marker(): return True
+`,
+      'app/main.py': `from services.sync import handler
+
+def boot():
+    return handler()
+`,
+    });
+    depthResult = await runPipelineFromRepo(depthRepoDir, () => {});
+
+    // Lexicographic scenario, files written in reverse order: omega first.
+    // Expected winner is still `alpha/services/sync.py`.
+    lexRepoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-python-suffix-lex-rev-'));
+    writeFixtureRepo(lexRepoDir, {
+      'omega/services/sync.py': `def handler():
+    return "omega"
+`,
+      'alpha/services/sync.py': `def handler():
+    return "alpha"
+`,
+      'app/services/marker.py': `def _marker(): return True
+`,
+      'app/main.py': `from services.sync import handler
+
+def boot():
+    return handler()
+`,
+    });
+    lexResult = await runPipelineFromRepo(lexRepoDir, () => {});
+  }, 120000);
+
+  afterAll(() => {
+    if (depthRepoDir !== undefined) fs.rmSync(depthRepoDir, { recursive: true, force: true });
+    if (lexRepoDir !== undefined) fs.rmSync(lexRepoDir, { recursive: true, force: true });
+  });
+
+  it('depth tiebreak still picks lib/services/sync.py with reversed file-write order', () => {
+    const imports = getRelationships(depthResult, 'IMPORTS').filter(
+      (i) => i.sourceFilePath === 'app/main.py',
+    );
+
+    const libEdge = imports.find((i) => i.targetFilePath === 'lib/services/sync.py');
+    expect(libEdge).toBeDefined();
+
+    const toolingEdge = imports.find((i) => i.targetFilePath === 'tooling/extras/services/sync.py');
+    expect(toolingEdge).toBeUndefined();
+  });
+
+  it('lex tiebreak still picks alpha/services/sync.py with reversed file-write order', () => {
+    const imports = getRelationships(lexResult, 'IMPORTS').filter(
+      (i) => i.sourceFilePath === 'app/main.py',
+    );
+
+    const alphaEdge = imports.find((i) => i.targetFilePath === 'alpha/services/sync.py');
+    expect(alphaEdge).toBeDefined();
+
+    const omegaEdge = imports.find((i) => i.targetFilePath === 'omega/services/sync.py');
+    expect(omegaEdge).toBeUndefined();
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Re-export chain: from .base import X barrel pattern via __init__.py
 // ---------------------------------------------------------------------------