refactor: address review feedback — remove dart.ts shim, JSDoc language field, update ARCHITECTURE.md

- Add JSDoc to ImportResolutionConfig.language clarifying it's
  documentation-only metadata not used by the factory
- Remove dart.ts legacy shim (was only kept for backward-compat tests)
- Rewrite dart-import-resolver.test.ts to test production strategies
  (dartPackageStrategy/dartRelativeStrategy) directly, including full
  factory composition via dartImportConfig
- Fix lint warning (no-explicit-any) by using buildSuffixIndex in makeCtx
- Update ARCHITECTURE.md to mention import-resolvers/configs/ as the
  extension point for per-language import resolution

Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/53f09a4f-1ff1-4a3e-a29c-fda9cdb4c4ef

Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-04-16 16:35:35 +00:00 • committed by GitHub
parent 84a028f5f1
commit e83caf4c9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 112 additions and 81 deletions

View file

@ -59,7 +59,7 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`).
| Embeddings | `src/core/embeddings/` + `src/core/run-analyze.ts` |
| Wiki generation | `src/core/wiki/` |
| Language support | `src/core/ingestion/languages/` + `tree-sitter-queries.ts` + `gitnexus-shared/src/languages.ts` |
| Import resolution | `src/core/ingestion/import-processor.ts` + `model/resolution-context.ts` |
| Import resolution | `src/core/ingestion/import-processor.ts` + `import-resolvers/configs/` + `model/resolution-context.ts` |
| Call resolution/MRO | `src/core/ingestion/call-processor.ts` + `model/resolve.ts` |
| Type extraction | `src/core/ingestion/type-extractors/` |
| Worker pool | `src/core/ingestion/workers/` |
@ -178,6 +178,8 @@ Per-language tree-sitter queries use different AST node names but produce the **
### Import resolution
Per-language import resolution uses the **configs + factory** pattern (like call/method/class extractors). Each language declares an `ImportResolutionConfig` in `import-resolvers/configs/`, listing an ordered chain of `ImportResolverStrategy` functions. `createImportResolver()` (in `resolver-factory.ts`) composes them: first non-null result wins. Low-level helpers shared across strategies live alongside the configs in `import-resolvers/` (e.g. `go.ts`, `rust.ts`, `python.ts`).
Unified 3-tier algorithm (`model/resolution-context.ts`), per-language `importSemantics` controls which tier activates:
| Tier | Confidence | Mechanism |

View file

@ -1,51 +0,0 @@
/**
* Dart import resolution — internal helpers.
*
* Strategies live in configs/dart.ts.
* This file is kept for backward compatibility with tests that import
* resolveDartImport directly.
*/
import type { ImportResult, ResolveCtx } from './types.js';
import { resolveStandard } from './standard.js';
import { SupportedLanguages } from 'gitnexus-shared';
/**
* Legacy monolithic Dart import resolver — kept for backward compatibility with
* existing tests. New code should use createImportResolver(dartImportConfig).
*/
export function resolveDartImport(
rawImportPath: string,
filePath: string,
ctx: ResolveCtx,
): ImportResult {
// Strip surrounding quotes from configurable_uri capture
const stripped = rawImportPath.replace(/^['"]|['"]$/g, '');
// Skip dart: SDK imports (dart:async, dart:io, etc.)
if (stripped.startsWith('dart:')) return null;
// Local package: imports → resolve to lib/<path>
if (stripped.startsWith('package:')) {
const slashIdx = stripped.indexOf('/');
if (slashIdx === -1) return null;
const relPath = stripped.slice(slashIdx + 1);
const candidates = [`lib/${relPath}`, relPath];
const files: string[] = [];
for (const candidate of candidates) {
for (const fp of ctx.allFileList) {
if (fp.endsWith('/' + candidate) || fp === candidate) {
files.push(fp);
break;
}
}
if (files.length > 0) break;
}
if (files.length > 0) return { kind: 'files', files };
return null;
}
// Relative imports — use standard resolution.
const relPath = stripped.startsWith('.') ? stripped : './' + stripped;
return resolveStandard(relPath, filePath, ctx, SupportedLanguages.Dart);
}

View file

@ -70,6 +70,12 @@ export type ImportResolverStrategy = ImportResolverFn;
* The factory (`createImportResolver`) chains them: first non-null result wins.
*/
export interface ImportResolutionConfig {
/**
* Documentation-only metadata identifying which language this config serves.
* **Not used by `createImportResolver`** — the factory only iterates `strategies`.
* Useful for logging, debugging, and compile-time exhaustiveness checks when
* mapping `SupportedLanguages → ImportResolutionConfig` in language providers.
*/
readonly language: SupportedLanguages;
readonly strategies: readonly ImportResolverStrategy[];
}

View file

@ -1,15 +1,35 @@
/**
* Unit tests for the production Dart import resolution strategies.
*
* Tests dartPackageStrategy and dartRelativeStrategy from configs/dart.ts —
* the actual strategies composed by createImportResolver(dartImportConfig).
*
* Key behavioral note:
* - dartPackageStrategy returns { kind: 'files', files: [] } (absorbing sentinel)
* for dart: SDK imports and unresolved external packages, which stops the
* strategy chain. This differs from the old monolithic resolver that returned null.
* Both produce zero import edges at runtime (applyImportResult treats them identically).
*/
import { describe, it, expect } from 'vitest';
import { resolveDartImport } from '../../src/core/ingestion/import-resolvers/dart.js';
import {
dartPackageStrategy,
dartRelativeStrategy,
} from '../../src/core/ingestion/import-resolvers/configs/dart.js';
import { createImportResolver } from '../../src/core/ingestion/import-resolvers/resolver-factory.js';
import { dartImportConfig } from '../../src/core/ingestion/import-resolvers/configs/dart.js';
import type { ResolveCtx } from '../../src/core/ingestion/import-resolvers/types.js';
import { buildSuffixIndex } from '../../src/core/ingestion/import-resolvers/utils.js';
function makeCtx(files: string[]): ResolveCtx {
const allFileList = files;
const normalizedFileList = files.map((f) => f.toLowerCase());
const normalizedFileList = files.map((f) => f.replace(/\\/g, '/'));
const index = buildSuffixIndex(normalizedFileList, allFileList);
return {
allFilePaths: new Set(files),
allFileList,
normalizedFileList,
index: undefined as any,
index,
resolveCache: new Map(),
configs: {
tsconfigPaths: null,
@ -21,59 +41,113 @@ function makeCtx(files: string[]): ResolveCtx {
};
}
describe('Dart import resolver', () => {
// ---------------------------------------------------------------------------
// dartPackageStrategy — absorbs SDK / external package imports
// ---------------------------------------------------------------------------
describe('dartPackageStrategy', () => {
describe('dart: SDK imports', () => {
it('skips dart:async', () => {
const result = resolveDartImport("'dart:async'", 'lib/main.dart', makeCtx([]));
expect(result).toBeNull();
it('absorbs dart:async with empty-files sentinel', () => {
const result = dartPackageStrategy("'dart:async'", 'lib/main.dart', makeCtx([]));
expect(result).toEqual({ kind: 'files', files: [] });
});
it('skips dart:io', () => {
const result = resolveDartImport("'dart:io'", 'lib/main.dart', makeCtx([]));
expect(result).toBeNull();
it('absorbs dart:io with empty-files sentinel', () => {
const result = dartPackageStrategy("'dart:io'", 'lib/main.dart', makeCtx([]));
expect(result).toEqual({ kind: 'files', files: [] });
});
});
describe('package: imports', () => {
it('resolves local package import to lib/', () => {
const ctx = makeCtx(['lib/models/user.dart', 'lib/main.dart']);
const result = resolveDartImport("'package:my_app/models/user.dart'", 'lib/main.dart', ctx);
const result = dartPackageStrategy(
"'package:my_app/models/user.dart'",
'lib/main.dart',
ctx,
);
expect(result).toEqual({ kind: 'files', files: ['lib/models/user.dart'] });
});
it('returns null for external package imports', () => {
it('absorbs external package imports with empty-files sentinel', () => {
const ctx = makeCtx(['lib/main.dart']);
const result = resolveDartImport("'package:http/http.dart'", 'lib/main.dart', ctx);
expect(result).toBeNull();
const result = dartPackageStrategy("'package:http/http.dart'", 'lib/main.dart', ctx);
expect(result).toEqual({ kind: 'files', files: [] });
});
it('returns null for malformed package import (no slash)', () => {
const result = resolveDartImport("'package:http'", 'lib/main.dart', makeCtx([]));
expect(result).toBeNull();
it('absorbs malformed package import (no slash) with empty-files sentinel', () => {
const result = dartPackageStrategy("'package:http'", 'lib/main.dart', makeCtx([]));
expect(result).toEqual({ kind: 'files', files: [] });
});
});
describe('relative imports', () => {
it('resolves relative import via standard resolver', () => {
const ctx = makeCtx(['lib/models/user.dart', 'lib/main.dart']);
const result = resolveDartImport("'models/user.dart'", 'lib/main.dart', ctx);
// resolveStandard handles relative path resolution
// The exact result depends on the standard resolver — we just check it doesn't crash
expect(result === null || result?.kind === 'files').toBe(true);
it('returns null for relative imports (chains to dartRelativeStrategy)', () => {
const ctx = makeCtx([]);
const result = dartPackageStrategy("'models.dart'", 'lib/main.dart', ctx);
expect(result).toBeNull();
});
});
describe('quote stripping', () => {
it('strips single quotes', () => {
const ctx = makeCtx([]);
const result = resolveDartImport("'dart:core'", 'lib/main.dart', ctx);
expect(result).toBeNull(); // dart: is skipped, proving quotes were stripped
const result = dartPackageStrategy("'dart:core'", 'lib/main.dart', makeCtx([]));
// dart: is absorbed, proving quotes were stripped
expect(result).toEqual({ kind: 'files', files: [] });
});
it('strips double quotes', () => {
const ctx = makeCtx([]);
const result = resolveDartImport('"dart:core"', 'lib/main.dart', ctx);
expect(result).toBeNull();
const result = dartPackageStrategy('"dart:core"', 'lib/main.dart', makeCtx([]));
expect(result).toEqual({ kind: 'files', files: [] });
});
});
});
// ---------------------------------------------------------------------------
// dartRelativeStrategy — bare relative paths via standard resolution
// ---------------------------------------------------------------------------
describe('dartRelativeStrategy', () => {
it('resolves bare relative path by prepending ./', () => {
const ctx = makeCtx(['lib/models/user.dart', 'lib/main.dart']);
const result = dartRelativeStrategy("'./models/user.dart'", 'lib/main.dart', ctx);
expect(result).toEqual({ kind: 'files', files: ['lib/models/user.dart'] });
});
it('returns null for unresolvable relative import', () => {
const ctx = makeCtx([]);
const result = dartRelativeStrategy("'nonexistent.dart'", 'lib/main.dart', ctx);
expect(result).toBeNull();
});
});
// ---------------------------------------------------------------------------
// Full resolver — dartImportConfig composed via factory
// ---------------------------------------------------------------------------
describe('Dart import resolver (full config)', () => {
const resolve = createImportResolver(dartImportConfig);
it('absorbs dart: SDK imports', () => {
const result = resolve("'dart:async'", 'lib/main.dart', makeCtx([]));
expect(result).toEqual({ kind: 'files', files: [] });
});
it('resolves local package: import', () => {
const ctx = makeCtx(['lib/models/user.dart']);
const result = resolve("'package:my_app/models/user.dart'", 'lib/main.dart', ctx);
expect(result).toEqual({ kind: 'files', files: ['lib/models/user.dart'] });
});
it('absorbs external package: import', () => {
const ctx = makeCtx(['lib/main.dart']);
const result = resolve("'package:http/http.dart'", 'lib/main.dart', ctx);
expect(result).toEqual({ kind: 'files', files: [] });
});
it('resolves relative import via second strategy', () => {
const ctx = makeCtx(['lib/models/user.dart', 'lib/main.dart']);
const result = resolve("'./models/user.dart'", 'lib/main.dart', ctx);
expect(result === null || result?.kind === 'files').toBe(true);
});
});