From ff78ef78bc6de4b1dd86155decedeae29887d6b6 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Tue, 6 Oct 2026 15:13:16 -0500 Subject: [PATCH] fix(sync): an import links the file it names when that file appears later A sync retried parked failed refs by name_tail (#1240), but an import names a file, a folder or a namespace, not a symbol, so the lookup never found one: the tail of `package:app/b.dart` was `dart`, of `inc/db.php` `php`, of `./req` `/req`. Most languages never parked theirs at all: an import that named no file reached the importing file's own `import` node through matchByQualifiedName (its qualified name is the module path) and resolved there, so `./x` in TS/JS, `import pkg.mod`, `#include "a/b.h"`, ``, a C# `using` of an outside namespace, Go module paths and R, Solidity, Svelte and Vue imports were consumed by an edge to themselves. A Lua `require` bound the local it is assigned to. The import linked only once the importer changed, or on a full index. - resolveOne's target gate drops an `imports` ref's own import statement, so the ref parks as failed (#2388 did this for Dart alone). - A Lua / Luau `require` resolves to a module file or nothing. - An import written as a path is parked under its last segment's stem (`package:app/b.dart` -> `b`, `./z.js` -> `z`): src/db/reference-tail.ts. - Sync retries the failed imports an ADDED file can satisfy: by its name, stem and folder (importPathKeys), as a failed import's tail or whole name, and by the changed files' namespace / module names as a whole name (C# `using Foo.Bar`, Java `import com.z.*`). Same per-key ceiling. - Schema v12: two partial indexes over failed imports, which the planner needs to avoid reading every failed call that shares a tail; the migration rewrites the tails of path imports parked by an older version. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/sync-import-retry.test.ts | 425 ++++++++++++++++++++++++++++ docs/viewer-launch-changelog.md | 2 + src/db/migrations.ts | 24 +- src/db/queries.ts | 90 ++++-- src/db/reference-tail.ts | 57 ++++ src/db/schema.sql | 6 + src/extraction/index.ts | 5 + src/index.ts | 15 + src/resolution/import-resolver.ts | 10 + src/resolution/index.ts | 23 +- src/resolution/name-matcher.ts | 2 + 12 files changed, 630 insertions(+), 30 deletions(-) create mode 100644 __tests__/sync-import-retry.test.ts create mode 100644 src/db/reference-tail.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d4d89d3484..9d900662d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Dart, an annotation now links to the constant or constructor it names, read the way it is written: `@riverpod` reaches Riverpod's `const riverpod = Riverpod();`, `@Riverpod(keepAlive: true)` the `Riverpod` class, an annotation written through a class, like `@SomeClass.value` or `@SomeClass.named(…)`, that class's own constant, enum value or constructor, and `@meta.immutable` only what the `meta` import brings in. Before, an annotation was matched like a function decorator, so a method, getter or function that only shared its name won: every `@riverpod` provider in Riverpod's own repository was linked to an unrelated getter called `riverpod`, and `callers` and impact for the real annotation missed them. Re-index Dart projects after upgrading. - 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. ## [1.6.2] - 2026-10-03 diff --git a/__tests__/sync-import-retry.test.ts b/__tests__/sync-import-retry.test.ts new file mode 100644 index 0000000000..ce836ffe89 --- /dev/null +++ b/__tests__/sync-import-retry.test.ts @@ -0,0 +1,425 @@ +/** + * A sync links an import whose file appears after the importer was indexed. + * + * Sync retries parked failed refs by name tail (#1240), but an import names a + * file, a folder or a namespace, not a symbol, so the symbol-name lookup never + * found it: the tail of `package:app/b.dart` was `dart`, of `inc/db.php` + * `php`, of `./foo` `/foo`. Most languages never even parked theirs: a module + * path that named no file reached the importing file's own `import` node by + * its qualified name, and that edge was never revisited. A Lua `require` took + * the local it is assigned to. Either way the import linked only once the + * importer itself changed, or on a full index. + */ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { DatabaseConnection } from '../src/db'; +import { CURRENT_SCHEMA_VERSION, getCurrentVersion, runMigrations } from '../src/db/migrations'; +import { QueryBuilder } from '../src/db/queries'; +import { importPathKeys } from '../src/db/reference-tail'; +import type { Node } from '../src/types'; + +interface Case { + name: string; + initial: Record; + added: Record; + /** The importing file. */ + from: string; + /** What its imports reach once `added` exists: ` `. */ + expected: string[]; +} + +const CASES: Case[] = [ + { + name: 'Dart', + initial: { + 'pubspec.yaml': 'name: app\n', + 'lib/a.dart': "import 'package:app/b.dart';\nimport 'package:kit/kit.dart';\nimport 'c.dart';\nimport 'src/d.dart';\nexport 'src/e.dart';\npart 'a.g.dart';\n\nvoid main() {}\n", + }, + added: { + 'lib/b.dart': 'class B {}\n', + 'packages/kit/pubspec.yaml': 'name: kit\n', + 'packages/kit/lib/kit.dart': 'class Kit {}\n', + 'lib/c.dart': 'class C {}\n', + 'lib/src/d.dart': 'class D {}\n', + 'lib/src/e.dart': 'class E {}\n', + // A generated part, written by build_runner after the library was indexed. + 'lib/a.g.dart': "part of 'a.dart';\n\nclass _Generated {}\n", + }, + from: 'lib/a.dart', + expected: ['file lib/b.dart', 'file packages/kit/lib/kit.dart', 'file lib/c.dart', 'file lib/src/d.dart', 'file lib/src/e.dart', 'file lib/a.g.dart'], + }, + { + name: 'TypeScript', + initial: { + 'src/a.ts': "import { X } from './x';\nimport './side';\nimport * as NS from './ns';\nimport { Y } from './dir';\nimport { Z } from './z.js';\nimport type { T } from './types';\nexport function run() { return [X, NS, Y, Z]; }\nexport type U = T;\n", + }, + added: { + 'src/x.ts': 'export const X = 1;\n', + 'src/side.ts': 'console.log(1);\n', + 'src/ns.ts': 'export const member = 1;\n', + 'src/dir/index.ts': 'export const Y = 2;\n', + 'src/z.ts': 'export const Z = 3;\n', + 'src/types.d.ts': 'export interface T { a: number }\n', + }, + from: 'src/a.ts', + expected: ['file src/x.ts', 'file src/side.ts', 'file src/ns.ts', 'file src/dir/index.ts', 'file src/z.ts', 'file src/types.d.ts'], + }, + { + name: 'JavaScript', + initial: { + 'a.js': "import './side';\nimport def from './def.js';\nconst req = require('./req');\nconst lib = require('./lib');\nmodule.exports = { def, req, lib };\n", + }, + added: { + 'side.js': 'console.log(1);\n', + 'def.js': 'export default function def() {}\n', + 'req.js': 'module.exports = 1;\n', + 'lib/index.js': 'module.exports = 2;\n', + }, + from: 'a.js', + expected: ['file side.js', 'file def.js', 'file req.js', 'file lib/index.js'], + }, + { + name: 'C', + initial: { 'src/main.c': '#include "a/b.h"\n#include "c.h"\n#include \nint main(void) { return 0; }\n' }, + added: { 'src/a/b.h': 'int b(void);\n', 'src/c.h': 'int c(void);\n' }, + from: 'src/main.c', + expected: ['file src/a/b.h', 'file src/c.h'], + }, + { + name: 'C++', + initial: { 'src/main.cpp': '#include "util/u.hpp"\n#include "w.h"\nint main() { return 0; }\n' }, + added: { 'src/util/u.hpp': 'namespace util { int u(); }\n', 'src/w.h': 'int w();\n' }, + from: 'src/main.cpp', + expected: ['file src/util/u.hpp', 'file src/w.h'], + }, + { + name: 'Objective-C', + initial: { 'Foo.m': '#import "Bar.h"\n#import "sub/Baz.h"\n@implementation Foo\n@end\n' }, + added: { 'Bar.h': '@interface Bar\n@end\n', 'sub/Baz.h': '@interface Baz\n@end\n' }, + from: 'Foo.m', + expected: ['file Bar.h', 'file sub/Baz.h'], + }, + { + name: 'PHP', + initial: { 'index.php': " ok.\n' }, + added: { 'src/b.hrl': '-define(B, 1).\n', 'include/c.hrl': '-define(C, 1).\n' }, + from: 'src/a.erl', + expected: ['file src/b.hrl', 'file include/c.hrl'], + }, + { + name: 'COBOL', + initial: { + 'MAIN.cbl': ' IDENTIFICATION DIVISION.\n PROGRAM-ID. MAIN.\n DATA DIVISION.\n WORKING-STORAGE SECTION.\n COPY CUSTREC.\n PROCEDURE DIVISION.\n STOP RUN.\n', + }, + added: { 'CUSTREC.cpy': ' 01 CUST-REC.\n 05 CUST-ID PIC 9(5).\n' }, + from: 'MAIN.cbl', + expected: ['file CUSTREC.cpy'], + }, + { + name: 'Java (a package wildcard)', + initial: { 'src/com/x/A.java': 'package com.x;\nimport com.z.*;\npublic class A {}\n' }, + added: { 'src/com/z/Z.java': 'package com.z;\npublic class Z {}\n' }, + from: 'src/com/x/A.java', + expected: ['namespace src/com/z/Z.java'], + }, + { + name: 'C# (a using of a namespace declared later)', + initial: { 'A.cs': 'using Foo.Bar;\nnamespace App { class A {} }\n' }, + added: { 'Baz.cs': 'namespace Foo.Bar { class Baz {} }\n' }, + from: 'A.cs', + expected: ['namespace Baz.cs'], + }, + { + name: 'Svelte', + initial: { 'src/App.svelte': "\n\n" }, + added: { 'src/Child.svelte': '

child

\n' }, + from: 'src/App.svelte', + expected: ['file src/Child.svelte'], + }, + { + name: 'Vue', + initial: { 'src/App.vue': "\n\n" }, + added: { 'src/Child.vue': '\n' }, + from: 'src/App.vue', + expected: ['file src/Child.vue'], + }, +]; + +let root = ''; +let cg: CodeGraph | undefined; + +afterEach(() => { + cg?.destroy(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; +}); + +function write(files: Record): 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); + } +} + +/** ` ` for every `imports` edge out of `file`, plus how many land on one of its own import statements. */ +function importsOf(file: string): { targets: string[]; ownStatements: number } { + const graph = cg!; + const ids = graph.getNodesInFile(file).map((n) => n.id); + const targets = new Set(); + let ownStatements = 0; + for (const edge of graph.getOutgoingEdgesFrom(ids, ['imports'])) { + const target = graph.getNode(edge.target); + if (!target) continue; + if (target.filePath === file) { + if (target.kind === 'import') ownStatements++; + continue; + } + targets.add(`${target.kind} ${target.filePath}`); + } + return { targets: [...targets].sort(), ownStatements }; +} + +describe('sync links an import of a file that appears after the importer', () => { + it.each(CASES)('$name', async (c) => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-import-')); + write(c.initial); + cg = await CodeGraph.init(root, { index: true }); + + // Unresolved, the import parks as failed — not as an edge to itself. + const before = importsOf(c.from); + expect(before.ownStatements).toBe(0); + for (const target of c.expected) expect(before.targets).not.toContain(target); + + write(c.added); + const result = await cg.sync(); + expect(result.filesAdded).toBe(Object.keys(c.added).length); + + // The importer did not change: only the retry of its parked import links it. + const synced = importsOf(c.from); + expect(synced.targets).toEqual(expect.arrayContaining(c.expected)); + expect(synced.ownStatements).toBe(0); + expect(cg.getPendingReferenceCount()).toBe(0); + + await cg.indexAll(); + expect(importsOf(c.from)).toEqual(synced); + }, 60_000); +}); + +describe('a removed file', () => { + it('relinks its unchanged importers when it is restored', async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-import-')); + write({ + 'pubspec.yaml': 'name: app\n', + 'lib/a.dart': "import 'package:app/b.dart';\n\nvoid main() {}\n", + 'lib/b.dart': 'class B {}\n', + }); + cg = await CodeGraph.init(root, { index: true }); + expect(importsOf('lib/a.dart').targets).toEqual(['file lib/b.dart']); + + fs.rmSync(path.join(root, 'lib/b.dart')); + expect((await cg.sync()).filesRemoved).toBe(1); + expect(importsOf('lib/a.dart')).toEqual({ targets: [], ownStatements: 0 }); + expect(cg.getPendingReferenceCount()).toBe(0); + + write({ 'lib/b.dart': 'class B {}\n' }); + expect((await cg.sync()).filesAdded).toBe(1); + expect(importsOf('lib/a.dart').targets).toEqual(['file lib/b.dart']); + }, 60_000); +}); + +describe('importPathKeys', () => { + it('names a file and its folder, whole and up to the first dot', () => { + expect(importPathKeys('lib/src/d.dart')).toEqual(['src', 'd.dart', 'd']); + expect(importPathKeys('src/types.d.ts')).toEqual(['src', 'types.d.ts', 'types']); + expect(importPathKeys('pkg/sub/__init__.py')).toEqual(['sub', '__init__.py', '__init__']); + expect(importPathKeys('vendor/gopkg.in/yaml.v3/yaml.go')).toEqual(['yaml.v3', 'yaml', 'yaml.go']); + expect(importPathKeys('main.c')).toEqual(['main.c', 'main']); + expect(importPathKeys('.github/.eslintrc.js')).toEqual(['.github', '.eslintrc.js', '.eslintrc']); + }); +}); + +describe('getRetryableFailedImports', () => { + let db: DatabaseConnection | undefined; + + afterEach(() => { + db?.close(); + db = undefined; + }); + + function setup(): QueryBuilder { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-import-')); + db = DatabaseConnection.initialize(path.join(root, 'test.db')); + const queries = new QueryBuilder(db.getDb()); + const failed = (id: string, referenceName: string, referenceKind: 'imports' | 'calls' = 'imports') => { + const node: Node = { id, kind: 'file', name: id, qualifiedName: id, filePath: id, language: 'dart', + startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: Date.now() }; + queries.insertNode(node); + queries.insertUnresolvedRef({ fromNodeId: id, referenceName, referenceKind, line: 1, column: 0, filePath: id, language: 'dart' }); + queries.markReferencesFailed([{ fromNodeId: id, referenceName, referenceKind }]); + }; + failed('a.dart', 'package:app/b.dart'); + failed('c.dart', '../b.dart'); + failed('d.dart', 'src/b.dart'); + failed('e.dart', 'b.dart'); + failed('f.dart', 'x.b', 'calls'); + failed('g.cs', 'Foo.Bar'); + failed('h.ts', 'useState'); + return queries; + } + + const names = (refs: Array<{ referenceName: string }>) => refs.map((r) => r.referenceName).sort(); + + it('finds an import by the stem of its path or by its whole name, and nothing else', () => { + const queries = setup(); + expect(names(queries.getRetryableFailedImports(['b']))).toEqual(['../b.dart', 'package:app/b.dart', 'src/b.dart']); + expect(names(queries.getRetryableFailedImports(['b.dart']))).toEqual(['b.dart']); + expect(queries.getRetryableFailedImports(['b', 'b.dart', 'b'])).toHaveLength(4); + }); + + it('finds an import by a node name only when the name is not also its tail', () => { + const queries = setup(); + // `using Foo.Bar` waits for the namespace node of that name. + expect(names(queries.getRetryableFailedImports([], ['Foo.Bar', 'useState', 'b']))).toEqual(['Foo.Bar']); + // A path key still reaches a plain name. + expect(names(queries.getRetryableFailedImports(['useState']))).toEqual(['useState']); + }); + + it('skips a key that more failed imports share than the ceiling', () => { + const queries = setup(); + expect(queries.getRetryableFailedImports(['b'], [], 2)).toEqual([]); + expect(names(queries.getRetryableFailedImports(['b', 'b.dart'], [], 2))).toEqual(['b.dart']); + expect(queries.getRetryableFailedImports(['b'], [], 3)).toHaveLength(3); + }); +}); + +describe('schema v12', () => { + let db: DatabaseConnection | undefined; + + afterEach(() => { + vi.restoreAllMocks(); + db?.close(); + db = undefined; + }); + + function fixture(): QueryBuilder { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-import-')); + db = DatabaseConnection.initialize(path.join(root, 'test.db')); + const queries = new QueryBuilder(db.getDb()); + queries.insertNode({ id: 'a', kind: 'file', name: 'a.dart', qualifiedName: 'lib/a.dart', filePath: 'lib/a.dart', + language: 'dart', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 }); + for (const [referenceName, referenceKind, tail] of [ + ['package:app/b.dart', 'imports', 'dart'], + ['c.h', 'imports', 'h'], + ['lists::map/2', 'calls', 'map'], + ] as const) { + queries.insertUnresolvedRef({ fromNodeId: 'a', referenceName, referenceKind, line: 1, column: 0, filePath: 'lib/a.dart', language: 'dart' }); + db.getDb().prepare("UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE reference_name = ?").run(tail, referenceName); + } + return queries; + } + + const tails = () => db!.getDb().prepare('SELECT reference_name AS name, name_tail AS tail FROM unresolved_refs ORDER BY id').all(); + + it('rewrites the tail of a path import parked by an older version, and replays cleanly', () => { + fixture(); + db!.getDb().exec(`DROP INDEX idx_unresolved_failed_import_tail; + DROP INDEX idx_unresolved_failed_import_name; + DELETE FROM schema_versions WHERE version >= 12; + INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (11, 0, 'legacy fixture');`); + db!.close(); + db = DatabaseConnection.open(path.join(root, 'test.db')); + expect(getCurrentVersion(db.getDb())).toBe(CURRENT_SCHEMA_VERSION); + const indexes = db.getDb().prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND name LIKE 'idx_unresolved_failed_import_%' ORDER BY name").all(); + expect(indexes).toEqual([{ name: 'idx_unresolved_failed_import_name' }, { name: 'idx_unresolved_failed_import_tail' }]); + const migrated = tails(); + expect(migrated).toEqual([ + { name: 'package:app/b.dart', tail: 'b' }, + { name: 'c.h', tail: 'h' }, + { name: 'lists::map/2', tail: 'map' }, + ]); + + db.getDb().exec('DELETE FROM schema_versions WHERE version >= 12'); + runMigrations(db.getDb(), 11); + expect(tails()).toEqual(migrated); + }); + + it('looks failed imports up through their own indexes', () => { + const queries = fixture(); + 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); + const lookups = prepare.mock.calls.map(([sql]) => sql as string).filter((sql) => sql.includes("reference_kind = 'imports'")); + prepare.mockRestore(); + expect(lookups.length).toBeGreaterThan(0); + for (const sql of lookups) { + const plan = db!.getDb().prepare(`EXPLAIN QUERY PLAN ${sql}`).all('k').map((row) => (row as { detail: string }).detail).join('; '); + expect(plan).toMatch(/USING (COVERING )?INDEX idx_unresolved_failed_import_(tail|name)/); + } + }); +}); diff --git a/docs/viewer-launch-changelog.md b/docs/viewer-launch-changelog.md index 84ed815051..0342462797 100644 --- a/docs/viewer-launch-changelog.md +++ b/docs/viewer-launch-changelog.md @@ -207,6 +207,8 @@ These describe `codegraph ui` and its screens. They were taken out of `## [Unrel - **`codegraph ui` on Windows moves on from a port another program holds.** Windows refuses a port that another program keeps for itself, or one inside a range it reserves, with a different error than a port that is simply in use, so the viewer gave up at the first such port instead of trying the next one, and blamed ports below 1024 needing elevated privileges — a rule Windows doesn't have. It now moves on to the next free port, and when none is left, or the port you pinned with `--port` is refused, it says what Windows did and how to list the reserved ranges. Thanks @ijbranch for the report and @sx4im. (#2299) +- **A file's outside imports are listed in more languages.** The file screen lists the imports that name nothing in your project, like a third-party package or a system header. In C and C++, Objective-C, JavaScript and TypeScript, Python, C#, Lua, R, Solidity, Svelte and Vue, an import that named no file had been linked to its own import statement, so it was listed nowhere. Re-index after upgrading. + ## Entries whose graph and `codegraph_explore` half already shipped The release notes carry a reworded version of each of these. Keep only the viewer half at launch. diff --git a/src/db/migrations.ts b/src/db/migrations.ts index bb7b84bf53..5191519b59 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -5,11 +5,12 @@ */ import { SqliteDatabase } from './sqlite-adapter'; +import { referenceNameTail } from './reference-tail'; /** * Current schema version */ -export const CURRENT_SCHEMA_VERSION = 11; +export const CURRENT_SCHEMA_VERSION = 12; /** * Migration definition @@ -216,6 +217,27 @@ const migrations: Migration[] = [ `); }, }, + { + version: 12, + description: 'Retry a failed import when sync adds the file it names: path tails and failed-import indexes', + up: (db) => { + // Partial over failed imports, so both stay small. Keep the definitions + // in lockstep with schema.sql. + db.exec(` + CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_tail ON unresolved_refs(reference_kind, name_tail) WHERE status = 'failed' AND reference_kind = 'imports'; + CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_name ON unresolved_refs(reference_kind, reference_name) WHERE status = 'failed' AND reference_kind = 'imports'; + `); + // A path import parked before this version carries its dotted tail — + // the extension or a path fragment — which no file's keys match. + // Rewrite it to the path tail a failed import is parked under now. + // Idempotent: a rewritten tail rewrites to itself. + const update = db.prepare('UPDATE unresolved_refs SET name_tail = ? WHERE id = ?'); + const rows = db + .prepare("SELECT id, reference_name FROM unresolved_refs WHERE status = 'failed' AND reference_kind = 'imports' AND reference_name LIKE '%/%'") + .all() as Array<{ id: number; reference_name: string }>; + for (const row of rows) update.run(referenceNameTail(row.reference_name, 'imports'), row.id); + }, + }, ]; /** diff --git a/src/db/queries.ts b/src/db/queries.ts index dd0160319b..b51034dfdc 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -22,6 +22,7 @@ import { kindBonus, nameMatchBonus, scorePathRelevance } from '../search/query-u import { parseQuery, boundedEditDistance } from '../search/query-parser'; import { isGeneratedFile } from '../extraction/generated-detection'; import { splitIdentifierSegments } from '../search/identifier-segments'; +import { referenceNameTail } from './reference-tail'; /** * Files that should not be candidates for "dominant file" detection: test/spec @@ -164,21 +165,6 @@ interface UnresolvedRefRow { name_tail: string; } -/** - * Last segment of a (possibly dotted/qualified) reference name — the part a - * new symbol's plain node name could match: 'util.greet' → 'greet', - * 'mod::fn' → 'fn', 'greet' → 'greet'. Written to unresolved_refs.name_tail - * when a ref is marked failed, so the #1240 retry lookup can match dotted - * refs against newly-added node names. - */ -function referenceNameTail(referenceName: string): string { - // Erlang refs carry a written arity (`f/1`, `mod::fn/2` — #1610); the tail a - // new symbol's plain name could match is the arity-less function name. - const base = referenceName.replace(/\/\d{1,3}$/, '') || referenceName; - const idx = Math.max(base.lastIndexOf('.'), base.lastIndexOf(':')); - return idx >= 0 ? base.slice(idx + 1) : base; -} - /** * Convert database row to Node object */ @@ -3739,7 +3725,7 @@ export class QueryBuilder { let changed = 0; const markMany = this.db.transaction((items: typeof refs) => { for (const ref of items) { - changed += stmt.run(referenceNameTail(ref.referenceName), ref.fromNodeId, ref.referenceName, ref.referenceKind).changes; + changed += stmt.run(referenceNameTail(ref.referenceName, ref.referenceKind), ref.fromNodeId, ref.referenceName, ref.referenceKind).changes; } }); markMany(refs); @@ -3754,7 +3740,7 @@ export class QueryBuilder { * can differ per call site (receiver-type inference reads the ref's line), * so a sibling must not inherit this row's failure. */ - markReferencesFailedByRowIds(refs: Array<{ rowId: number; referenceName: string }>): number { + markReferencesFailedByRowIds(refs: Array<{ rowId: number; referenceName: string; referenceKind: string }>): number { if (refs.length === 0) return 0; const stmt = this.db.prepare( "UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE id = ?" @@ -3762,7 +3748,7 @@ export class QueryBuilder { let changed = 0; const markMany = this.db.transaction((items: typeof refs) => { for (const ref of items) { - changed += stmt.run(referenceNameTail(ref.referenceName), ref.rowId).changes; + changed += stmt.run(referenceNameTail(ref.referenceName, ref.referenceKind), ref.rowId).changes; } }); markMany(refs); @@ -3824,6 +3810,66 @@ export class QueryBuilder { })); } + /** + * Failed `imports` refs a sync should retry once files appear. An import + * names a file, a folder or a namespace, which the symbol lookup above + * cannot match, so it is found two ways: + * - by `pathKeys` (`importPathKeys` of each added file), as its tail or its + * whole name: `package:app/b.dart` waits for a file named `b.dart`, a + * bare `#include "b.h"` or `require 'db.php'` is the file's whole name; + * - by `names` (the changed files' namespaces and modules), as its whole + * name when its tail is something else: a C# `using Foo.Bar` waits for + * the namespace node `Foo.Bar`. An import whose tail is the name is the + * symbol lookup's, under that lookup's ceiling. + * Same per-name ceiling as {@link getRetryableFailedReferences}, counted + * per key. + */ + getRetryableFailedImports(pathKeys: string[], names: string[] = [], perNameCeiling: number = 500): UnresolvedReference[] { + const failedImports = "status = 'failed' AND reference_kind = 'imports'"; + const lookups = [ + { keys: pathKeys, column: 'name_tail', where: failedImports }, + { keys: pathKeys, column: 'reference_name', where: failedImports }, + { keys: names, column: 'reference_name', where: `${failedImports} AND name_tail != reference_name` }, + ]; + const rows = new Map(); + for (const { keys, column, where } 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[] = []; + 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(','); + const counts = this.db + .prepare(`SELECT ${column} AS key, COUNT(*) AS count FROM unresolved_refs WHERE ${where} AND ${column} IN (${placeholders}) GROUP BY ${column}`) + .all(...chunk) as Array<{ key: string; count: number }>; + for (const row of counts) { + if (row.count <= perNameCeiling) retryKeys.push(row.key); + } + } + // Pass 2: load the surviving rows; a row two lookups find is kept once. + for (let i = 0; i < retryKeys.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = retryKeys.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const chunkRows = this.db + .prepare(`SELECT * FROM unresolved_refs WHERE ${where} AND ${column} IN (${placeholders})`) + .all(...chunk) as UnresolvedRefRow[]; + for (const row of chunkRows) 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, + })); + } + /** * Resolution edges whose TARGET symbol is named one of `names` — the edges a * sync must re-resolve after `names` gained or lost a definition (CG-33). @@ -3935,16 +3981,18 @@ export class QueryBuilder { /** * Distinct node names present in the given files — the symbol names a sync * pass uses to look up retryable failed refs after those files changed. + * `kinds` narrows them to nodes of those kinds. */ - getNodeNamesByFiles(filePaths: string[]): string[] { + getNodeNamesByFiles(filePaths: string[], kinds?: readonly NodeKind[]): string[] { if (filePaths.length === 0) return []; + const kindFilter = kinds ? ` AND kind IN (${kinds.map(() => '?').join(',')})` : ''; const names = new Set(); for (let i = 0; i < filePaths.length; i += SQLITE_PARAM_CHUNK_SIZE) { const chunk = filePaths.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); const placeholders = chunk.map(() => '?').join(','); const rows = this.db - .prepare(`SELECT DISTINCT name FROM nodes WHERE file_path IN (${placeholders})`) - .all(...chunk) as Array<{ name: string }>; + .prepare(`SELECT DISTINCT name FROM nodes WHERE file_path IN (${placeholders})${kindFilter}`) + .all(...chunk, ...(kinds ?? [])) as Array<{ name: string }>; for (const row of rows) names.add(row.name); } return [...names]; diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts new file mode 100644 index 0000000000..4c7ed319b0 --- /dev/null +++ b/src/db/reference-tail.ts @@ -0,0 +1,57 @@ +/** + * The keys sync's failed-ref retry (#1240) matches on: the tail a failed ref + * is parked under, and the names a newly added file can be imported as. + * Shared by the query layer, which writes and reads them, and the migration + * that rewrote the tails parked before path imports had one of their own. + */ + +/** + * Last segment of a (possibly dotted/qualified) reference name — the part a + * new symbol's plain node name could match: 'util.greet' → 'greet', + * 'mod::fn' → 'fn', 'greet' → 'greet'. Written to unresolved_refs.name_tail + * when a ref is marked failed, so the #1240 retry lookup can match dotted + * refs against newly-added node names. + * + * An import written as a path names a FILE, so its tail is the path's last + * segment cut at its first dot — `package:app/b.dart` → 'b', `./foo` → 'foo', + * `inc/db.php` → 'db' — the stem {@link importPathKeys} takes from a file + * that could satisfy it. The dotted tail was the extension ('dart', 'php') or + * a path fragment ('/foo'), which no file ever matched. + */ +export function referenceNameTail(referenceName: string, referenceKind?: string): string { + if (referenceKind === 'imports') { + const path = referenceName.replace(/\/+$/, ''); + const slash = path.lastIndexOf('/'); + const stem = slash >= 0 ? fileNameStem(path.slice(slash + 1)) : ''; + // `..` and `.` name a folder only through the importing file's location. + if (!/^\.*$/.test(stem)) return stem; + } + // Erlang refs carry a written arity (`f/1`, `mod::fn/2` — #1610); the tail a + // new symbol's plain name could match is the arity-less function name. + const base = referenceName.replace(/\/\d{1,3}$/, '') || referenceName; + const idx = Math.max(base.lastIndexOf('.'), base.lastIndexOf(':')); + return idx >= 0 ? base.slice(idx + 1) : base; +} + +/** A file or folder name up to its first dot: 'b.dart' → 'b', 'types.d.ts' → 'types', '.eslintrc.js' → '.eslintrc'. */ +function fileNameStem(name: string): string { + const dot = name.indexOf('.', 1); + return dot > 0 ? name.slice(0, dot) : name; +} + +/** + * What an import of the file at `filePath` can be written as, for the failed + * imports a sync that adds the file retries: its name (`b.h`, a bare include) + * and stem (`b`, the tail of `a/b.h`, `./b`, `pkg.b`), and the same two for + * its folder, which an import names for its index file (`./ui` → + * `ui/index.ts`, `pkg` → `pkg/__init__.py`) or as a Go package. + */ +export function importPathKeys(filePath: string): string[] { + const parts = filePath.split('/'); + const keys = new Set(); + for (const name of parts.slice(-2)) { + keys.add(name); + keys.add(fileNameStem(name)); + } + return [...keys]; +} diff --git a/src/db/schema.sql b/src/db/schema.sql index 91ae4805ba..6aff9163c1 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -197,6 +197,12 @@ CREATE INDEX IF NOT EXISTS idx_unresolved_file_path ON unresolved_refs(file_path CREATE INDEX IF NOT EXISTS idx_unresolved_from_name ON unresolved_refs(from_node_id, reference_name); CREATE INDEX IF NOT EXISTS idx_unresolved_status ON unresolved_refs(status); CREATE INDEX IF NOT EXISTS idx_unresolved_failed_tail ON unresolved_refs(name_tail) WHERE status = 'failed'; +-- Sync's failed-import retry looks failed imports up by tail and by whole +-- name. The leading reference_kind is what makes the planner pick these over +-- idx_unresolved_failed_tail / idx_unresolved_name, whose per-key averages +-- hide how many failed calls a common tail (`index`, `types`) has. +CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_tail ON unresolved_refs(reference_kind, name_tail) WHERE status = 'failed' AND reference_kind = 'imports'; +CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_name ON unresolved_refs(reference_kind, reference_name) WHERE status = 'failed' AND reference_kind = 'imports'; CREATE INDEX IF NOT EXISTS idx_edges_provenance ON edges(provenance); -- Sync's third-file wiring lookup must not scan every synthesized edge. -- CASE short-circuits malformed metadata; keep these expressions identical diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 8b7da4c2cf..4d768ddda9 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -145,6 +145,8 @@ export interface SyncResult { */ durationMs: number; changedFilePaths?: string[]; + /** The part of `changedFilePaths` that was not indexed before this sync. */ + addedFilePaths?: string[]; /** Paths not absorbed because reading or extraction failed; retain for status/retry. */ failedFilePaths?: string[]; /** @@ -3302,6 +3304,7 @@ export class ExtractionOrchestrator { let filesRemoved = 0; let nodesUpdated = 0; const changedFilePaths: string[] = []; + const addedFilePaths: string[] = []; // `file\0name` definition pairs for the files this sync touches, sampled // BEFORE their nodes are replaced/deleted. Compared against the post-store // pairs below to derive `definitionDelta` (CG-33). @@ -3476,6 +3479,7 @@ export class ExtractionOrchestrator { onFileChange?.(filePath, content); filesToIndex.push(filePath); changedFilePaths.push(filePath); + addedFilePaths.push(filePath); filesAdded++; } else if (tracked.contentHash !== contentHash || neverParsed) { onFileChange?.(filePath, content); @@ -3545,6 +3549,7 @@ export class ExtractionOrchestrator { nodesUpdated, durationMs: Date.now() - startTime, changedFilePaths: changedFilePaths.length > 0 ? changedFilePaths : undefined, + ...(addedFilePaths.length > 0 ? { addedFilePaths } : {}), ...(failedFilePaths.length > 0 ? { failedFilePaths } : {}), definitionDelta: definitionDelta.length > 0 ? definitionDelta : undefined, }; diff --git a/src/index.ts b/src/index.ts index 7e7a1dbf9d..4d852af522 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,6 +30,7 @@ import { import { DatabaseConnection, getDatabasePath, removeDatabaseFiles } from './db'; import { WalCheckpointValve, resolveWalValveMb } from './db/wal-valve'; import { QueryBuilder } from './db/queries'; +import { importPathKeys } from './db/reference-tail'; import { isInitialized, createDirectory, @@ -989,6 +990,20 @@ export class CodeGraph { const retryable = this.queries.getRetryableFailedReferences( this.queries.getNodeNamesByFiles(result.changedFilePaths) ); + // A failed import waits for a file, a folder or a namespace, not a + // symbol: `package:app/b.dart` for a file named `b.dart`, `./ui` + // for `ui/index.ts`, `using Foo.Bar` for that namespace's node. + // Look those up by what the ADDED files can be imported as (a + // modified file was already there for any import of it), and by + // the namespaces and modules the changed files declare. + const retryRows = new Set(retryable.map((ref) => ref.rowId)); + const importRetry = this.queries.getRetryableFailedImports( + (result.addedFilePaths ?? []).flatMap(importPathKeys), + this.queries.getNodeNamesByFiles(result.changedFilePaths, ['namespace', 'module']) + ); + for (const ref of importRetry) { + if (!retryRows.has(ref.rowId)) retryable.push(ref); + } if (retryable.length > 0) { options.onProgress?.({ phase: 'resolving', diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 4c3d5570dc..bf5dab41b0 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -905,6 +905,16 @@ export function isDartImportRef(ref: UnresolvedRef): boolean { return ref.language === 'dart' && ref.referenceKind === 'imports'; } +/** + * Is this a Lua / Luau `require`? It names a module file (see + * resolveLuaRequire) or a module from outside the project, never a symbol: + * the name-matcher bound `local lfs = require "lfs"` and `local Signal = + * require(script.Parent.Signal)` to the local each is assigned to. + */ +export function isLuaRequireRef(ref: UnresolvedRef): boolean { + return (ref.language === 'lua' || ref.language === 'luau') && ref.referenceKind === 'imports'; +} + /** * Resolve a PHP include/require path to a project-relative file path. * diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 88451f6193..41a15843ec 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -29,7 +29,7 @@ import { clearDartLibraryMemos } from './dart-libraries'; import { clearVbnetReceiverMemos, isVbMemberRead, isVbPathCall, matchVbMemberRead, matchVbPathCall } from './vbnet-receivers'; import { gateTypeParameter, clearTypeParameterMemos } from './type-parameters'; import { gateDartLocal, clearDartLocalScopeMemos } from './dart-local-scope'; -import { resolveViaImport, resolvePhpImportedStaticCall, resolvePhpQualifiedClassRef, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isDartImportRef, isJsPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; +import { resolveViaImport, resolvePhpImportedStaticCall, resolvePhpQualifiedClassRef, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isDartImportRef, isLuaRequireRef, isJsPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; import { ResolverPool, minRefsForPool, shouldEngageAdaptively } from './resolver-pool'; import { resolveAliasBinding } from './alias-binding'; import { detectFrameworks } from './frameworks'; @@ -1359,8 +1359,9 @@ export class ReferenceResolver { // Nix static path imports are file references for the same reason — // falling through would let "./x.nix" name-match an unrelated node. So // is a Dart import's URI: `package:flutter/foundation.dart` matched by - // its last segment went to riverpod's own foundation.dart. - if (isPhpIncludePathRef(ref) || isCobolCopybookRef(ref) || isNixPathImportRef(ref) || isDartImportRef(ref) || ref.language === 'terraform') { + // its last segment went to riverpod's own foundation.dart. And a Lua + // `require`, whose leaf matched the local it is assigned to. + if (isPhpIncludePathRef(ref) || isCobolCopybookRef(ref) || isNixPathImportRef(ref) || isDartImportRef(ref) || isLuaRequireRef(ref) || ref.language === 'terraform') { return candidates.length > 0 ? candidates.reduce((best, curr) => curr.confidence > best.confidence ? curr : best @@ -1544,13 +1545,13 @@ export class ReferenceResolver { * ref's line), so a sibling must not inherit this row's failure (#1269). */ private static partitionFailedCleanup(unresolved: UnresolvedRef[]): { - byRowId: Array<{ rowId: number; referenceName: string }>; + byRowId: Array<{ rowId: number; referenceName: string; referenceKind: string }>; legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }>; } { - const byRowId: Array<{ rowId: number; referenceName: string }> = []; + const byRowId: Array<{ rowId: number; referenceName: string; referenceKind: string }> = []; const legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }> = []; for (const r of unresolved) { - if (r.rowId != null) byRowId.push({ rowId: r.rowId, referenceName: r.referenceName }); + if (r.rowId != null) byRowId.push({ rowId: r.rowId, referenceName: r.referenceName, referenceKind: r.referenceKind }); else legacyKeys.push({ fromNodeId: r.fromNodeId, referenceName: r.referenceName, @@ -2993,7 +2994,8 @@ export class ReferenceResolver { * framework, import, name-match, chain, CFML component path. * * For `imports`: the target must be importable. A member that only exists - * inside a type never is. + * inside a type never is, and neither is the import statement the reference + * was written in. * * For `extends`/`implements`, it cannot be describing a real supertype when: * @@ -3029,9 +3031,14 @@ export class ReferenceResolver { } // An `imports` reference names something importable — never a member that - // only exists inside a type. + // only exists inside a type, nor its own import statement. A module path + // that names no file (`./missing`, `stdio.h`, `pkg.mod`, a C# `using` of + // an outside namespace) reaches the statement by its qualified name; as an + // edge to itself it is never retried, while a failed ref is retried by the + // sync that adds the file it names. if (ref.referenceKind === 'imports') { const target = this.nodeById(result.targetNodeId); + if (target?.kind === 'import' && target.filePath === ref.filePath) return null; return target && !isImportableKind(target.kind) ? null : result; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index d2f07797e7..d86047ebd3 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -7206,6 +7206,8 @@ export function matchByQualifiedName( let candidates = keepForRef(context.getNodesByQualifiedName(ref.referenceName)); // A C# `using X.Y;` names a namespace: one the project declares, else it is // the file's own (external) using — never another file's using of that name. + // That own using ends the lookup here, and resolveOne drops it (an import + // never names its own statement), so the ref parks as failed. if (ref.language === 'csharp' && ref.referenceKind === 'imports') { const namespaces = candidates.filter((n) => n.kind === 'namespace'); candidates = namespaces.length > 0 ? preferCallSiteFile(namespaces, ref.filePath).slice(0, 1)