From a3fdb920a81b22206b07cbd43ca75b329fa1221e Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 11 Oct 2026 23:19:11 +0000 Subject: [PATCH] fix(sync): a Python import of a name a package re-exports links when the package appears later (#2507) `from fastapi import FastAPI` reaches `fastapi/applications.py` through `fastapi/__init__.py`, which re-exports it and declares no node named `FastAPI`. While that `__init__.py` was missing, or before it gained the re-export, the calls through the name failed and were parked under the name, which the package file that then appears or changes never carries. A fresh index linked them; a sync never did. An unaliased from-import of a module in the project's own folders is now parked under the module's key, as an aliased one is (#2501): a relative import, or an absolute one whose dotted path, or the package holding it, is a project folder at any depth. An outside package's names (`from django.db.models import Q`) keep their tails, since on netbox they would crowd 'module:models', 'module:urls' and others past the retry ceiling. #2450's by-name lookup still finds every parked ref by its own name. Two limits made the restored package's importers wait anyway: - A module key gets a retry ceiling of its own, 5,000 rather than the 500 a name tail has. A large population under a module key is every importer of a package that was missing, not noise: fastapi's 1,069 `from fastapi import ...` refs, Django's 2,257 under 'module:urls'. - The CG-33 rebind's per-name ceiling counts only the edges that existed before the retry. Relinking every `from django.urls import path` pushed `path` over it, so the `path` edges the sync's new definitions moved were never rebound; the retry's own edges are still rebound under a name that stays below it, as before. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../sync-python-import-binding-retry.test.ts | 167 +++++++++++++++++- src/db/queries.ts | 32 +++- src/db/reference-tail.ts | 5 + src/extraction/index.ts | 7 +- src/index.ts | 8 +- src/resolution/import-resolver.ts | 67 ++++++- 7 files changed, 264 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d6e83499..31ab29da8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A file in a `unittests/` folder, where llvm/llvm-project and google/breakpad keep their unit tests, or in a folder named like a unittest, such as `dcheck_unittest/` in google/glog, now counts as a test whatever its own name says, like LLVM's `CountCopyAndMove.h` helper. Before, such a file was taken for production code unless its name said test, and none of them were kept apart from the program the way a `tests/` folder is: search and `codegraph_explore` ranked them alongside production code, `codegraph affected` missed them, and production code linked to a unit test's own declarations, as LLVM's `Ctx` and `OpInfo` variables did to ones its unit tests declare. A folder named just `unittest/`, like CPython's `Lib/unittest/`, is still production code. Re-index projects that have such folders after upgrading. - `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now do the same for a Python import under a name of its own, like `from .models import User as UserModel`, `from . import helpers as h` or `from app.pkg import sub as subpkg`: when the module or package is added after the importing file was indexed, comes back after being deleted, or gets the imported name in a later edit, the import and every use of the alias, such as a call, a base class or the class passed as a value, now link to it. Before, they stayed unlinked until the importing file changed or the project was re-indexed. Re-index Python projects after upgrading to pick up links an earlier version missed. +- `codegraph sync` and the file watcher now also link a Python import of a name a package re-exports, like `from fastapi import FastAPI` through `fastapi/__init__.py`, when that `__init__.py` is added after the importing files were indexed or gains the re-export in a later edit. Before, calls through the name stayed unlinked until each importing file changed or the project was re-indexed. Large projects gain the most: sync also used to skip, in any language, a module or package that more than 500 references were waiting for, and now restoring Django's `django/urls/__init__.py` relinks every file that imports `path` from it. Re-index Python projects after upgrading to pick up links an earlier version missed. - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading. - In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading. - In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector>` or `Striped>`, now links to the method of the class it is declared as. Before, the nested arguments hid that class: in RocksDB, `files_marked_for_compaction_.clear()` on such an `autovector` linked to an unrelated `CompactionInputFiles::clear` that only shares words with the variable's name, and `mutex_.Get(key)` on a `Striped` linked to nothing. Re-index C++ projects after upgrading. diff --git a/__tests__/sync-python-import-binding-retry.test.ts b/__tests__/sync-python-import-binding-retry.test.ts index 870d022d4..087380ded 100644 --- a/__tests__/sync-python-import-binding-retry.test.ts +++ b/__tests__/sync-python-import-binding-retry.test.ts @@ -12,6 +12,12 @@ * importer changed or the project was indexed again. #2450 parked the ES-module * bindings under their module; a Python import names its module by a dotted * path, relative to the importing file's package when it starts with dots. + * + * An unaliased name has the same gap when a package re-exports it: + * `from fastapi import FastAPI` reaches `fastapi/applications.py` through + * `fastapi/__init__.py`, which declares no node named `FastAPI`. So a package + * `__init__.py` that appears after its importers, or gains the re-export, + * left them unlinked too. */ import { describe, it, expect, afterEach } from 'vitest'; import * as fs from 'fs'; @@ -19,6 +25,7 @@ import * as os from 'os'; import * as path from 'path'; import CodeGraph from '../src/index'; import { DatabaseConnection, getDatabasePath } from '../src/db'; +import { QueryBuilder } from '../src/db/queries'; import { moduleReferenceKeys } from '../src/db/reference-tail'; type Files = Record; @@ -104,6 +111,46 @@ const BY_NAME: Files = { }; const BY_NAME_ADDED: Files = { 'app/schemas.py': 'class FormSchema:\n pass\n' }; +/** + * Unaliased imports of what a package re-exports from the modules that declare + * it: absolutely (`from shop import Order`) and from the package above + * (`from .. import Order`). The package renames what it re-exports, so nothing + * named `Order` or `price` exists for a name lookup to find. + */ +const REEXPORT_IMPORTERS: Files = { + 'shop/models.py': 'class _Order:\n pass\n', + 'shop/services/__init__.py': '', + 'shop/services/pricing.py': 'def base_price():\n return 1\n', + 'shop/api/__init__.py': '', + 'shop/api/views.py': 'from .. import Order\n\n\ndef view():\n return Order()\n', + 'checkout.py': 'from shop import Order, price\n\n\ndef run():\n return Order(), price()\n', +}; +const REEXPORT_PACKAGE: Files = { + 'shop/__init__.py': 'from .models import _Order as Order\nfrom .services.pricing import base_price as price\n', +}; +const REEXPORT_LINKS = [ + 'Order: imports file checkout.py -> class shop/models.py::_Order', + 'Order: imports file shop/__init__.py -> class shop/models.py::_Order', + 'Order: imports file shop/api/views.py -> class shop/models.py::_Order', + 'Order: instantiates function checkout.py -> class shop/models.py::_Order', + 'Order: instantiates function shop/api/views.py -> class shop/models.py::_Order', + 'price: calls function checkout.py -> function shop/services/pricing.py::base_price', + 'price: imports file checkout.py -> function shop/services/pricing.py::base_price', + 'price: imports file shop/__init__.py -> function shop/services/pricing.py::base_price', +]; + +/** fastapi's shape: `from fastapi import FastAPI` through `from .applications import FastAPI as FastAPI`. */ +const SAME_NAME: Files = { + 'kit/applications.py': 'class Kit:\n pass\n', + 'app.py': 'from kit import Kit\n\n\ndef make():\n return Kit()\n', +}; +const SAME_NAME_PACKAGE: Files = { 'kit/__init__.py': 'from .applications import Kit as Kit\n' }; +const SAME_NAME_LINKS = [ + 'Kit: imports file app.py -> class kit/applications.py::Kit', + 'Kit: imports file kit/__init__.py -> class kit/applications.py::Kit', + 'Kit: instantiates function app.py -> class kit/applications.py::Kit', +]; + let roots: string[] = []; let graphs: CodeGraph[] = []; @@ -145,9 +192,9 @@ function edgesOf(graph: CodeGraph): string[] { .sort(); } -/** The {@link LINKS} the graph has. */ -function linksOf(graph: CodeGraph): string[] { - const names = new Set(LINKS.map((link) => link.slice(0, link.indexOf(':')))); +/** The links of `expected`'s reference names the graph has ({@link LINKS} by default). */ +function linksOf(graph: CodeGraph, expected: string[] = LINKS): string[] { + const names = new Set(expected.map((link) => link.slice(0, link.indexOf(':')))); const ids = graph.getFiles().flatMap((file) => graph.getNodesInFile(file.path)).map((n) => n.id); const links: string[] = []; for (const edge of graph.getOutgoingEdgesFrom(ids)) { @@ -162,9 +209,13 @@ function linksOf(graph: CodeGraph): string[] { /** The final tree, indexed from scratch in a folder of its own: what the sync has to match. */ async function fullIndexOf(files: Files): Promise { + return edgesOf(await freshGraphOf(files)); +} + +async function freshGraphOf(files: Files): Promise { const root = tempRoot(); write(root, files); - return edgesOf(await index(root)); + return index(root); } describe('sync links references through a Python import alias to their module', () => { @@ -229,21 +280,64 @@ describe('sync links references through a Python import alias to their module', }, 120_000); }); +describe('sync links an unaliased Python import through the package that re-exports it', () => { + it('when the package is added after its importers', async () => { + const root = tempRoot(); + write(root, REEXPORT_IMPORTERS); + const graph = await index(root); + expect(linksOf(graph, REEXPORT_LINKS)).toEqual([]); + + write(root, REEXPORT_PACKAGE); + const result = await graph.sync(); + expect(result.filesAdded).toBe(1); + expect(result.filesModified).toBe(0); + expect(linksOf(graph, REEXPORT_LINKS)).toEqual(REEXPORT_LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...REEXPORT_IMPORTERS, ...REEXPORT_PACKAGE })); + }, 120_000); + + it('when an edit gives the package the re-export', async () => { + const root = tempRoot(); + write(root, { ...REEXPORT_IMPORTERS, 'shop/__init__.py': '' }); + const graph = await index(root); + + write(root, REEXPORT_PACKAGE); + expect((await graph.sync()).filesModified).toBe(1); + expect(linksOf(graph, REEXPORT_LINKS)).toEqual(REEXPORT_LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...REEXPORT_IMPORTERS, ...REEXPORT_PACKAGE })); + }, 120_000); + + it('when it re-exports the name its module declares', async () => { + // While the package is missing, the import itself is linked to `Kit` by + // name, which a full index links through the package: the same link. + const root = tempRoot(); + write(root, SAME_NAME); + const graph = await index(root); + + write(root, SAME_NAME_PACKAGE); + expect((await graph.sync()).filesAdded).toBe(1); + expect(linksOf(graph, SAME_NAME_LINKS)).toEqual(SAME_NAME_LINKS); + expect(linksOf(await freshGraphOf({ ...SAME_NAME, ...SAME_NAME_PACKAGE }), SAME_NAME_LINKS)).toEqual(SAME_NAME_LINKS); + }, 120_000); +}); + describe('a failed reference through a Python import alias', () => { it('waits for its module, and every other reference keeps its own tail', async () => { const root = tempRoot(); write(root, { ...IMPORTERS, ...BY_NAME, - // An unaliased import, a standard-library alias and a member read - // through an alias wait for the name they use. + ...REEXPORT_IMPORTERS, + // An unaliased import of an outside package, a standard-library alias + // and a member read through an alias wait for the name they use; an + // unaliased import of the project's own module waits for its module. 'app/other.py': [ 'from .models import Group', + 'from django.db.models import Q', 'from collections import OrderedDict as OD', 'from .models import User as UserModel', '', 'def build():', - ' return Group(), OD(), UserModel.create()', + ' return Group(), Q(), OD(), UserModel.create()', '', ].join('\n'), }); @@ -273,7 +367,13 @@ describe('a failed reference through a Python import alias', () => { // Still found by its own name too: sync's lookup by whole name. expect(tailOf('app/form.py', 'FormSchema', 'extends')).toBe('module:lib'); - expect(tailOf('app/other.py', 'Group', 'calls')).toBe('Group'); + // An unaliased import of a module in the project's own folders. + expect(tailOf('app/other.py', 'Group', 'calls')).toBe('module:models'); + expect(tailOf('checkout.py', 'Order', 'calls')).toBe('module:shop'); + expect(tailOf('checkout.py', 'price', 'imports')).toBe('module:shop'); + expect(tailOf('shop/api/views.py', 'Order', 'calls')).toBe('module:shop'); + + expect(tailOf('app/other.py', 'Q', 'calls')).toBe('Q'); expect(tailOf('app/other.py', 'OD', 'calls')).toBe('OD'); expect(tailOf('app/other.py', 'UserModel.create', 'calls')).toBe('create'); @@ -287,3 +387,54 @@ describe('a failed reference through a Python import alias', () => { } }, 60_000); }); + +describe('the retry ceiling', () => { + let db: DatabaseConnection | undefined; + + afterEach(() => { + db?.close(); + db = undefined; + }); + + /** 600 failed calls of `FastAPI` parked under 'module:fastapi', and 600 of `get` under their name. */ + function fixture(): QueryBuilder { + db = DatabaseConnection.initialize(path.join(tempRoot(), 'test.db')); + const queries = new QueryBuilder(db.getDb()); + queries.insertNode({ id: 'f', kind: 'file', name: 'app.py', qualifiedName: 'app.py', filePath: 'app.py', + language: 'python', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 }); + const refs = Array.from({ length: 1200 }, (_, i) => ({ + fromNodeId: 'f', referenceName: i < 600 ? 'FastAPI' : 'get', referenceKind: 'calls' as const, + line: i + 1, column: 0, filePath: 'app.py', language: 'python' as const, + })); + queries.insertUnresolvedRefsBatch(refs); + db.getDb().exec("UPDATE unresolved_refs SET status = 'failed', name_tail = CASE reference_name WHEN 'FastAPI' THEN 'module:fastapi' ELSE 'get' END"); + return queries; + } + + it('lets every importer of a package that appeared retry, and still skips a common name', () => { + const queries = fixture(); + // Every importer of the package waits for it: a large population is not noise. + expect(queries.getRetryableFailedReferences(['module:fastapi'])).toHaveLength(600); + // A name shared by 600 failed calls is, as before; so is the by-name lookup of parked rows. + expect(queries.getRetryableFailedReferences(['get'])).toHaveLength(0); + expect(queries.getRetryableFailedReferences(['FastAPI'])).toHaveLength(0); + // The module ceiling still bounds the work. + expect(queries.getRetryableFailedReferences(['module:fastapi'], 500, 599)).toHaveLength(0); + }); + + it("leaves the edges a sync's retry just made out of the rebind's count, not out of the rebind", () => { + // Django: relinking every `from django.urls import path` must not stop a + // new definition of `path` from rebinding the calls it may now answer. + const queries = fixture(); + queries.insertNode({ id: 'p', kind: 'variable', name: 'path', qualifiedName: 'path', filePath: 'conf.py', + language: 'python', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 }); + const edge = (line: number) => ({ source: 'f', target: 'p', kind: 'calls' as const, line, column: 0, + metadata: { refName: 'path', resolvedBy: 'exact-match', confidence: 0.7 } }); + queries.insertEdges([edge(1)]); + const beforeRetry = queries.getMaxEdgeId(); + queries.insertEdges(Array.from({ length: 600 }, (_, i) => edge(i + 2))); + + expect(queries.getResolutionEdgesByTargetName(['path'])).toHaveLength(0); + expect(queries.getResolutionEdgesByTargetName(['path'], 500, beforeRetry)).toHaveLength(601); + }); +}); diff --git a/src/db/queries.ts b/src/db/queries.ts index de9b1dc8c..e5a2cccf0 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -22,7 +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'; +import { isModuleKey, referenceNameTail } from './reference-tail'; /** * Files that should not be candidates for "dominant file" detection: test/spec @@ -3841,8 +3841,14 @@ export class QueryBuilder { * reference still waits for its own name too, looked up by its whole name: * a fresh index links a binding whose module never resolves it by that * name alone, to whichever declaration carries it. + * + * A module key has a ceiling of its own, `perModuleCeiling`. What waits + * under it imports a module of that name, so a large population is not + * noise: it is every importer of a package whose `__init__.py` was missing + * (`from fastapi import FastAPI` across a whole test suite), which the file + * that just appeared resolves at once. The ceiling only bounds the work. */ - getRetryableFailedReferences(names: string[], perNameCeiling: number = 500): UnresolvedReference[] { + getRetryableFailedReferences(names: string[], perNameCeiling: number = 500, perModuleCeiling: number = 5000): UnresolvedReference[] { if (names.length === 0) return []; const lookups = [ @@ -3863,7 +3869,8 @@ export class QueryBuilder { .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); + const ceiling = column === 'name_tail' && isModuleKey(row.key) ? perModuleCeiling : perNameCeiling; + if (row.count <= ceiling) retryKeys.push(row.key); } } @@ -4018,6 +4025,11 @@ export class QueryBuilder { return changed; } + /** The highest edge id so far: edges inserted later have higher ids (AUTOINCREMENT). */ + getMaxEdgeId(): number { + return (this.db.prepare('SELECT COALESCE(MAX(id), 0) AS id FROM edges').get() as { id: number }).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). @@ -4041,15 +4053,24 @@ export class QueryBuilder { * population the name is generic (`get`, `clear`, …), one definition changing * won't flip most of them, and rebinding an arbitrary subset is both wasted * work and incoherent coverage. + * + * With `countedUpToEdgeId`, only edges up to that id count toward the + * ceiling: the ones a sync's failed-ref retry made just now were resolved + * against the graph this sync built, and relinking every `from django.urls + * import path` must not push `path` over the ceiling and leave the edges the + * new definitions moved unrebound. They are still returned under a name + * that stays below it. */ getResolutionEdgesByTargetName( names: string[], - perNameCeiling: number = 500 + perNameCeiling: number = 500, + countedUpToEdgeId?: number ): Array { if (names.length === 0) return []; // Pass 1: per-name edge counts, chunked under the SQLite parameter limit. const keep: string[] = []; + const counted = countedUpToEdgeId === undefined ? '' : 'AND e.id <= ?'; for (let i = 0; i < names.length; i += SQLITE_PARAM_CHUNK_SIZE) { const chunk = names.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); const placeholders = chunk.map(() => '?').join(','); @@ -4060,9 +4081,10 @@ export class QueryBuilder { JOIN nodes tgt ON tgt.id = e.target WHERE tgt.name IN (${placeholders}) AND (e.provenance IS NULL OR e.provenance != 'heuristic') + ${counted} GROUP BY tgt.name` ) - .all(...chunk) as Array<{ name: string; count: number }>; + .all(...chunk, ...(countedUpToEdgeId === undefined ? [] : [countedUpToEdgeId])) as Array<{ name: string; count: number }>; for (const row of counts) { if (row.count <= perNameCeiling) keep.push(row.name); } diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts index 2bdd6646a..9ec033f34 100644 --- a/src/db/reference-tail.ts +++ b/src/db/reference-tail.ts @@ -73,6 +73,11 @@ const MODULE_REFERENCE = /^(?:layout:)?(?:lazy-import:(.+)|import:(.+)#[^#]+)$/; /** What a module reference's tail starts with. No symbol's name does, so a lookup by symbol names never finds one. */ const MODULE_KEY = 'module:'; +/** Whether a tail or lookup key waits for a module (`module:Team`) rather than for a name. */ +export function isModuleKey(key: string): boolean { + return key.startsWith(MODULE_KEY); +} + /** * The tail of a reference that waits for a module rather than for a name: the * module path's stem behind 'module:' — `./pages/Team` → 'module:Team', diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 8566e0398..42ff58f04 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -3208,14 +3208,15 @@ export class ExtractionOrchestrator { * - edges whose source is in a file this sync already re-extracted are * skipped — their references were re-resolved from scratch moments ago; * - very common names are skipped by the per-name ceiling in - * `getResolutionEdgesByTargetName`. + * `getResolutionEdgesByTargetName`, counted over the edges up to + * `countedUpToEdgeId` when given (those before the sync's failed-ref retry). * * Returns the number of references resurrected. */ - resurrectStaleResolutionEdges(definitionDelta: string[], changedFilePaths: string[]): number { + resurrectStaleResolutionEdges(definitionDelta: string[], changedFilePaths: string[], countedUpToEdgeId?: number): number { if (definitionDelta.length === 0) return 0; const alreadyFresh = new Set(changedFilePaths); - const candidates = this.queries.getResolutionEdgesByTargetName(definitionDelta); + const candidates = this.queries.getResolutionEdgesByTargetName(definitionDelta, undefined, countedUpToEdgeId); const edgeIds: number[] = []; const refs: UnresolvedReference[] = []; diff --git a/src/index.ts b/src/index.ts index 6fa4b8b69..75e32ad28 100644 --- a/src/index.ts +++ b/src/index.ts @@ -998,6 +998,10 @@ export class CodeGraph { // Resolve references if files were updated const filesChanged = result.filesAdded > 0 || result.filesModified > 0; + // The last edge before the failed-ref retry below: the rebind's + // per-name ceiling counts only edges up to it (see + // resurrectStaleResolutionEdges). + let edgesBeforeRetry: number | undefined; if (filesChanged) { if (result.changedFilePaths) { // Scope resolution to changed files (git fast path — bounded set) @@ -1037,6 +1041,7 @@ export class CodeGraph { // an edit can satisfy it as well as an added file. On a sync // where no failed ref matches, this is one indexed lookup. const tRetry = Date.now(); + edgesBeforeRetry = this.queries.getMaxEdgeId(); const retryable = this.queries.getRetryableFailedReferences([...new Set([ ...this.queries.getNodeNamesByFiles(result.changedFilePaths), ...result.changedFilePaths.flatMap(moduleReferenceKeys), @@ -1122,7 +1127,8 @@ export class CodeGraph { const tRebind = Date.now(); const rebound = this.orchestrator.resurrectStaleResolutionEdges( result.definitionDelta, - result.changedFilePaths ?? [] + result.changedFilePaths ?? [], + edgesBeforeRetry ); if (process.env.CODEGRAPH_SYNTH_TIMINGS) { console.error( diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index ba9face41..c3b66202b 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -323,6 +323,7 @@ export function clearImportResolverMemos(context: ResolutionContext): void { cobolCopybookIndexes.delete(context); pythonModuleFileMemos.delete(context); PY_MODULE_SYMBOLS.delete(context); + PROJECT_FOLDER_SUFFIXES.delete(context); } export function resolveImportPath( @@ -3151,22 +3152,76 @@ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext * import a submodule of it (`helpers` is `app/helpers.py`), whose keys hold * the package's name too (moduleReferenceKeys). * - * An unaliased import keeps its tail: the module, or the one it re-exports the - * name from, declares that name. So does an import of the standard library. An - * installed package's import can't be told from a module the project doesn't + * An installed package's alias can't be told from a module the project doesn't * have yet (`from settings import Config as Cfg`), so it waits for its module - * too, and a sync still finds it by its own name. + * too, and a sync still finds it by its own name. An import of the standard + * library keeps its tail. + * + * So does an unaliased `import x`, and an unaliased `from fastapi import + * FastAPI` unless its module is in one of the project's own folders + * ({@link isProjectPythonModule}). The name is the one the module has, but a + * package often re-exports it from the module that declares it + * (`fastapi/__init__.py`: `from .applications import FastAPI as FastAPI`), and + * a package `__init__.py` that appears or gains the re-export declares no node + * of that name. An outside package's names stay off the module keys: every + * `from django.db.models import Q` would crowd 'module:models' past the retry + * ceiling, and the project's own modules with it. */ function pythonImportBindingTail(ref: UnresolvedRef, context: ResolutionContext): string | undefined { if (!IDENTIFIER.test(ref.referenceName)) return undefined; const binding = context.getImportMappings(ref.filePath, 'python').find((m) => m.localName === ref.referenceName); - if (!binding) return undefined; + if (!binding || isExternalImport(binding.source, 'python', context)) return undefined; const declared = binding.isNamespace ? binding.source.slice(binding.source.lastIndexOf('.') + 1) : binding.exportedName; - if (declared === binding.localName || isExternalImport(binding.source, 'python', context)) return undefined; + if (declared === binding.localName && (binding.isNamespace || !isProjectPythonModule(binding.source, context))) return undefined; const modulePath = pythonModulePath(binding.source, ref.filePath); return modulePath ? moduleTail(modulePath) || undefined : undefined; } +/** + * Whether a Python import's module is in one of the project's own folders: a + * relative one is, and an absolute one when a folder of the project ends in its + * dotted path (`app/api` for `app.api`, the package) or in the package holding + * it (`app` for `app.api`, a module file), at any depth, as `src/` layouts put + * it. A folder, not a module: `fastapi/__init__.py` is what goes missing, while + * `fastapi/` keeps its other files. + */ +function isProjectPythonModule(source: string, context: ResolutionContext): boolean { + if (source.startsWith('.')) return true; + const folders = projectFolderSuffixes(context); + const modulePath = source.replace(/\./g, '/'); + const slash = modulePath.lastIndexOf('/'); + return folders.has(modulePath) || (slash > 0 && folders.has(modulePath.slice(0, slash))); +} + +/** + * Per-context memo: every trailing run of folder names the project's files sit + * in (`a/b/c.py` gives 'a/b', 'b' and 'a'). The file set is that of the + * resolution pass; dropped by clearImportResolverMemos. + */ +const PROJECT_FOLDER_SUFFIXES = new WeakMap>(); + +function projectFolderSuffixes(context: ResolutionContext): Set { + let suffixes = PROJECT_FOLDER_SUFFIXES.get(context); + if (suffixes) return suffixes; + const folders = new Set(); + for (const file of context.getAllFiles()) { + for (let i = file.lastIndexOf('/'); i > 0; i = file.lastIndexOf('/', i - 1)) { + const folder = file.slice(0, i); + if (folders.has(folder)) break; + folders.add(folder); + } + } + suffixes = new Set(); + for (const folder of folders) { + for (let i = -1; ; i = folder.indexOf('/', i + 1)) { + suffixes.add(folder.slice(i + 1)); + if (folder.indexOf('/', i + 1) === -1) break; + } + } + PROJECT_FOLDER_SUFFIXES.set(context, suffixes); + return suffixes; +} + /** * A Python import's module as a project path: `app.models` → 'app/models'; * from `app/sub/views.py`, `.models` → 'app/sub/models', `..` → 'app' and