fix(sync): an import links an added file whose name a popular import shares

CHANGELOG.md

@@ -70,6 +70,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - In Dart, an annotation is no longer lost when a comment sits between it and the declaration, like `@override` followed by `// ignore: must_call_super`, a `///` comment written below the annotations, or `@riverpod` followed by an `// expect_lint: …` line. Before, the comment hid the annotation, so a provider written that way was missing from the annotation's `callers` and impact. Re-index Dart projects after upgrading.
 - In Dart, a call written through an import prefix, like `http.get(…)` after `import 'package:http/http.dart' as http;`, or `utils.Listener()` and `kit.Box.create()` through a prefix of your own, now links to what the library imported with that prefix exports, following its `export` chains, `show` and `hide`, and to nothing when that library is a package outside your project. Before, the prefix was taken for a variable, so `http.get(…)` could link to an unrelated `Http` class's `get` method, and calls made through a prefix into your own libraries were missing from `callers` and impact. Re-index Dart projects after upgrading.
 - `codegraph sync` and the file watcher now link an import to the file it names when that file is added after the importing file was indexed, or comes back after being deleted: `import 'package:app/b.dart';` in Dart, `import './side'` in JavaScript or TypeScript, `#include "net/socket.h"` in C and C++, `require 'inc/db.php'` in PHP, `import pkg.mod` in Python, a Lua `require`, or a C# `using` of a namespace declared later. Before, the import stayed unlinked until the importing file itself changed or the project was re-indexed, so `codegraph affected` and the files listed as using the new file missed it. Re-index after upgrading.
+- `codegraph sync` and the file watcher now also link the imports of an added or restored JavaScript or TypeScript file whose name hundreds of other imports share, like a project's own `utils.ts` among `jotai/utils` and `@scope/pkg/utils` imports, or `assert.ts` in a project full of `import * as assert from 'assert'`. Before, a name that common was skipped as noise no single file could answer, so in a large project the file's own `./utils` and `~/settings/utils` imports stayed unlinked until the importing files changed or the project was re-indexed.
 - `codegraph sync` and the file watcher now link a navigation call to its page when the page or route is added after the call was indexed, or comes back after being removed: `history.push('/login')` and `navigate('/login')` with React Router, `router.push('/login')` and `redirect('/login')` in Next.js, Expo Router, Vue Router, TanStack Router and SvelteKit (`goto('/login')`), and `this.router.navigate(['/login'])` and `routerLink="/login"` in Angular. Before, the call stayed unlinked, or linked to a catch-all or `:id` page that answered it in the meantime, until its own file changed or the project was re-indexed, so the step from that call to the page it opens was missing or wrong. A route whose path changes without its own file changing, like one built from a path constant kept in another file, now also lets go of the calls that named its old path. Re-index after upgrading.
 - JSON files in a `templates` or `sections` folder are now read as Shopify Liquid only inside a Shopify theme, where the folder that contains `templates` or `sections` also has `layout/theme.liquid` or `config/settings_schema.json`. Before, all such JSON was, so a .NET project template's `template.json`, a schematic or a CMS's content showed up in the index as an empty Liquid file, and one that lists `sections` could be linked to an unrelated Liquid file of the same name. Shopify themes, including one kept in a subfolder of a repository, are indexed as before, and the next sync removes JSON that an earlier version indexed this way.
 - In a repository that holds more than one Shopify theme, a theme's `{% render %}`, `{% include %}` and `{% section %}` tags and the sections its JSON templates list now link only to that theme's own snippets and sections, which is where Shopify looks for them. Before, a section or snippet the theme doesn't have was linked to another theme's file of the same name, and a theme kept in a subfolder could link to the root theme's copy instead of its own. A reference the theme can't satisfy now links to nothing. Projects with a single theme are indexed as before. Re-index projects with several Shopify themes after upgrading.

__tests__/sync-import-retry-crowded-key.test.ts

@@ -0,0 +1,129 @@
+/**
+ * A sync links an import of a file that appears later, even when the key the
+ * import waits under is one a popular import shares.
+ *
+ * Sync looks a failed import up by what an added file can be imported as — its
+ * name, its stem, its folder — and skips a key more than 500 failed imports
+ * share, as noise one file cannot answer. But a path import is parked under
+ * its path's stem, which a package's imports share with every file of that
+ * name: `utils` is the stem of every `jotai/utils` and `@scope/pkg/utils` as
+ * much as of a project's own `./utils`, and `assert` the name of every
+ * binding of `import * as assert from 'assert'`. In a large project those
+ * crowded the key past the ceiling, so the project's own `utils.ts`, added
+ * after its importers were indexed or restored after a delete, never linked
+ * until the importers changed or the project was re-indexed.
+ */
+import { describe, it, expect, afterEach } from 'vitest';
+import * as fs from 'fs';
+import * as os from 'os';
+import * as path from 'path';
+import CodeGraph from '../src/index';
+
+interface Case {
+  name: string;
+  initial: Record<string, string>;
+  added: Record<string, string>;
+  /** `imports` edges the sync must make: `<from file> -> <kind> <file>`. */
+  expected: string[];
+}
+
+/** One file importing `count` package modules whose paths all end in `/utils`. */
+function packageUtils(count: number): string {
+  return Array.from({ length: count }, (_, i) => `import 'kit${i}/utils';\n`).join('');
+}
+
+const CASES: Case[] = [
+  {
+    name: "a project's utils.ts among package subpaths named utils",
+    initial: {
+      'package.json': JSON.stringify({ name: 'app', dependencies: Object.fromEntries(Array.from({ length: 520 }, (_, i) => [`kit${i}`, '1.0.0'])) }),
+      'src/vendor.ts': packageUtils(520),
+      'src/settings/view.ts': "import { format } from './utils';\nexport const view = () => format();\n",
+      'src/settings/panel/list.ts': "import { format } from '../utils';\nexport const list = () => format();\n",
+    },
+    added: { 'src/settings/utils.ts': "export function format() { return ''; }\n" },
+    expected: [
+      'src/settings/view.ts -> file src/settings/utils.ts',
+      'src/settings/panel/list.ts -> file src/settings/utils.ts',
+    ],
+  },
+  {
+    name: "a project's assert.ts among bindings of the assert package",
+    initial: {
+      'package.json': JSON.stringify({ name: 'app', devDependencies: { assert: '2.0.0' } }),
+      // Each statement is a module reference to the package and a binding,
+      // both waiting under `assert`: 520 in all.
+      'test/all.test.ts': Array.from({ length: 260 }, () => "import * as assert from 'assert';\n").join('') + 'assert.ok(true);\n',
+      'src/common/strings.ts': "import { ok } from './assert';\nexport const check = (s: string) => ok(s.length > 0);\n",
+    },
+    added: { 'src/common/assert.ts': 'export function ok(value: boolean) { return value; }\n' },
+    expected: ['src/common/strings.ts -> file src/common/assert.ts'],
+  },
+  {
+    // bitwarden imports its `account.service` from 724 files: not noise at all.
+    name: 'a file more than 500 imports name',
+    initial: Object.fromEntries(Array.from({ length: 510 }, (_, i) => [
+      `src/features/f${i}.ts`,
+      `import { format } from '../shared/utils';\nexport const f${i} = () => format();\n`,
+    ])),
+    added: { 'src/shared/utils.ts': "export function format() { return ''; }\n" },
+    expected: ['src/features/f0.ts -> file src/shared/utils.ts', 'src/features/f509.ts -> file src/shared/utils.ts'],
+  },
+];
+
+const roots: string[] = [];
+const graphs: CodeGraph[] = [];
+
+afterEach(() => {
+  // Close before removing: a held database makes the unlink EBUSY on Windows.
+  for (const graph of graphs.splice(0)) graph.destroy();
+  for (const dir of roots.splice(0)) fs.rmSync(dir, { recursive: true, force: true });
+});
+
+function tempRoot(): string {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-crowded-key-'));
+  roots.push(dir);
+  return dir;
+}
+
+function write(root: string, files: Record<string, string>): void {
+  for (const [rel, content] of Object.entries(files)) {
+    fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
+    fs.writeFileSync(path.join(root, rel), content);
+  }
+}
+
+/** Every `imports` edge between two files of the project: `<from file> -> <kind> <file>`. */
+function fileImports(graph: CodeGraph): string[] {
+  const edges = new Set<string>();
+  for (const file of graph.getFiles()) {
+    const ids = graph.getNodesInFile(file.path).map((n) => n.id);
+    for (const edge of graph.getOutgoingEdgesFrom(ids, ['imports'])) {
+      const target = graph.getNode(edge.target);
+      if (target && target.filePath !== file.path) edges.add(`${file.path} -> ${target.kind} ${target.filePath}`);
+    }
+  }
+  return [...edges].sort();
+}
+
+describe('sync links an import under a key a popular import crowds', () => {
+  it.each(CASES)('$name', async (c) => {
+    const root = tempRoot();
+    write(root, c.initial);
+    const graph = await CodeGraph.init(root, { index: true });
+    graphs.push(graph);
+    for (const edge of c.expected) expect(fileImports(graph)).not.toContain(edge);
+
+    write(root, c.added);
+    expect((await graph.sync()).filesAdded).toBe(Object.keys(c.added).length);
+    const synced = fileImports(graph);
+    expect(synced).toEqual(expect.arrayContaining(c.expected));
+
+    // A fresh index of the same tree, in a folder of its own, links the same.
+    const fresh = tempRoot();
+    write(fresh, { ...c.initial, ...c.added });
+    const oracle = await CodeGraph.init(fresh, { index: true });
+    graphs.push(oracle);
+    expect(synced).toEqual(fileImports(oracle));
+  }, 120_000);
+});

__tests__/sync-import-retry.test.ts

@@ -362,6 +362,20 @@ describe('getRetryableFailedImports', () => {
     expect(names(queries.getRetryableFailedImports(['b', 'b.dart'], [], 2))).toEqual(['b.dart']);
     expect(queries.getRetryableFailedImports(['b'], [], 3)).toHaveLength(3);
   });
+
+  it('looks a key past the ceiling through for the imports that could name an added file', () => {
+    const queries = setup();
+    const only = (...accepted: string[]) => (ref: { referenceName: string }) => accepted.includes(ref.referenceName);
+    // Three imports wait under `b`, one more than the ceiling.
+    expect(names(queries.getRetryableFailedImports(['b'], [], 2, [], undefined, only('../b.dart')))).toEqual(['../b.dart']);
+    // What the test accepts is no noise: the module ceiling holds it.
+    expect(names(queries.getRetryableFailedImports(['b'], [], 1, [], 2, only('../b.dart', 'src/b.dart')))).toEqual(['../b.dart', 'src/b.dart']);
+    expect(queries.getRetryableFailedImports(['b'], [], 1, [], 1, only('../b.dart', 'src/b.dart'))).toEqual([]);
+    // A key within the ceiling is retried whole, whatever the test says.
+    expect(queries.getRetryableFailedImports(['b'], [], 3, [], undefined, only())).toHaveLength(3);
+    // A namespace waits for its node, not a file: the test is not asked.
+    expect(names(queries.getRetryableFailedImports([], ['Foo.Bar'], 0, [], undefined, only('Foo.Bar')))).toEqual([]);
+  });
 });
 
 describe('schema v12', () => {
@@ -420,6 +434,9 @@ describe('schema v12', () => {
     const prepare = vi.spyOn(db!.getDb(), 'prepare');
     // The fixture's tails are old ones: `dart` finds a row by tail, `c.h` by name.
     expect(queries.getRetryableFailedImports(['dart', 'c.h'], ['Foo.Bar'])).toHaveLength(2);
+    // A key past the ceiling is read whole for `couldName` to look through: at
+    // a ceiling of 0 both keys are, and what it accepts is retried.
+    expect(queries.getRetryableFailedImports(['dart', 'c.h'], [], 0, [], undefined, () => true)).toHaveLength(2);
     const lookups = prepare.mock.calls.map(([sql]) => sql as string).filter((sql) => sql.includes("reference_kind = 'imports'"));
     prepare.mockRestore();
     expect(lookups.length).toBeGreaterThan(0);

src/db/queries.ts

@@ -4126,28 +4126,48 @@ export class QueryBuilder {
    * The other lookups share the per-name ceiling of
    * {@link getRetryableFailedReferences}, counted per key; this one has the
    * module ceiling, since a specifier that resolves to the file is no noise.
+   *
+   * A path key past the ceiling is not all noise, though: `utils` is the stem
+   * of a project's own `./utils` and of every `jotai/utils` and
+   * `@scope/pkg/utils` alike, and `assert` the name of every binding of
+   * `import * as assert from 'assert'`. Such a key is retried for the
+   * imports `couldName` accepts, the ones that could name a file the sync
+   * added: no noise either, so within the module ceiling.
    */
   getRetryableFailedImports(
     pathKeys: string[],
     names: string[] = [],
     perNameCeiling: number = 500,
     resolvedModules: string[] = [],
     perModuleCeiling: number = 5000,
+    couldName?: (ref: UnresolvedReference) => boolean,
   ): UnresolvedReference[] {
     const failedImports = "status = 'failed' AND reference_kind = 'imports'";
     const lookups = [
-      { keys: pathKeys, column: 'name_tail', where: failedImports, ceiling: perNameCeiling },
-      { keys: pathKeys, column: 'reference_name', where: failedImports, ceiling: perNameCeiling },
+      { keys: pathKeys, column: 'name_tail', where: failedImports, ceiling: perNameCeiling, couldName },
+      { keys: pathKeys, column: 'reference_name', where: failedImports, ceiling: perNameCeiling, couldName },
       { keys: names, column: 'reference_name', where: `${failedImports} AND name_tail != reference_name`, ceiling: perNameCeiling },
       // A module specifier that now resolves to an added file: no noise, so
       // the module ceiling, as for a module key.
       { keys: resolvedModules, column: 'reference_name', where: failedImports, ceiling: perModuleCeiling },
     ];
+    const toReference = (row: UnresolvedRefRow): UnresolvedReference => ({
+      fromNodeId: row.from_node_id,
+      referenceName: row.reference_name,
+      referenceKind: row.reference_kind as EdgeKind,
+      line: row.line,
+      column: row.col,
+      candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined,
+      filePath: row.file_path,
+      language: row.language as Language,
+      rowId: row.id,
+    });
     const rows = new Map<number, UnresolvedRefRow>();
-    for (const { keys, column, where, ceiling } of lookups) {
+    for (const { keys, column, where, ceiling, couldName } of lookups) {
       const unique = [...new Set(keys)].filter((key) => key.length > 0);
       // Pass 1: per-key counts, chunked under the SQLite parameter limit.
       const retryKeys: string[] = [];
+      const crowdedKeys: string[] = [];
       for (let i = 0; i < unique.length; i += SQLITE_PARAM_CHUNK_SIZE) {
         const chunk = unique.slice(i, i + SQLITE_PARAM_CHUNK_SIZE);
         const placeholders = chunk.map(() => '?').join(',');
@@ -4156,6 +4176,7 @@ export class QueryBuilder {
           .all(...chunk) as Array<{ key: string; count: number }>;
         for (const row of counts) {
           if (row.count <= ceiling) retryKeys.push(row.key);
+          else if (couldName) crowdedKeys.push(row.key);
         }
       }
       // Pass 2: load the surviving rows; a row two lookups find is kept once.
@@ -4167,19 +4188,18 @@ export class QueryBuilder {
           .all(...chunk) as UnresolvedRefRow[];
         for (const row of chunkRows) rows.set(row.id, row);
       }
+      // A key past the ceiling: only the imports that could name an added file.
+      for (const key of crowdedKeys) {
+        const keyRows = this.db
+          .prepare(`SELECT * FROM unresolved_refs WHERE ${where} AND ${column} IN (?)`)
+          .all(key) as UnresolvedRefRow[];
+        const named = keyRows.filter((row) => couldName!(toReference(row)));
+        if (named.length > perModuleCeiling) continue;
+        for (const row of named) rows.set(row.id, row);
+      }
     }
 
-    return [...rows.values()].map((row) => ({
-      fromNodeId: row.from_node_id,
-      referenceName: row.reference_name,
-      referenceKind: row.reference_kind as EdgeKind,
-      line: row.line,
-      column: row.col,
-      candidates: row.candidates ? safeJsonParse(row.candidates, undefined) : undefined,
-      filePath: row.file_path,
-      language: row.language as Language,
-      rowId: row.id,
-    }));
+    return [...rows.values()].map(toReference);
   }
 
   /**

src/index.ts

@@ -1078,11 +1078,16 @@ export class CodeGraph {
               .filter((ref) => ref.filePath !== undefined && ref.language !== undefined &&
                 resolvesToAdded(ref.referenceName, ref.filePath, ref.language));
             const retryRows = new Set(retryable.map((ref) => ref.rowId));
+            // A key a popular import shares (`utils`, `react`, `assert`) is
+            // past the ceiling, and is still looked through for the imports
+            // that could name an added file.
             const importRetry = this.queries.getRetryableFailedImports(
               (result.addedFilePaths ?? []).flatMap(importPathKeys),
               this.queries.getNodeNamesByFiles(result.changedFilePaths, ['namespace', 'module']),
               undefined,
-              resolvedModules
+              resolvedModules,
+              undefined,
+              this.resolver.couldNameAnyOf([...addedFiles])
             );
             for (const ref of [...importRetry, ...folderImports]) {
               if (!retryRows.has(ref.rowId)) {