Repository navigation
fix(sync): an import links an added file whose name a popular import shares - #2666
Merged
Merged
Conversation
…shares Sync retries a failed import 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. A path import is parked under its path's stem, though, and a package's imports share that stem with every file of the 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 a project's own `utils.ts` added after its importers were indexed, or restored after a delete, never linked them: folo's `modules/settings/utils.ts` and `components/ui/modal/stacked/hooks.ts`, and vscode's `base/common/assert.ts`. A key past the ceiling is now looked through for the imports that could name an added file, and those are retried. A JavaScript or TypeScript import names a file by its module (the path branch of resolveViaImport, shared with it as jsModuleFile), by a framework alias, or by the file's own name. What that accepts is no noise, so it is held to the module ceiling, as the specifiers that resolve to an added file already are: bitwarden imports its account.service from 724 files. An import in another language is not accepted, so a crowded key there is skipped as before; a key within the ceiling is retried whole, as it was. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 19, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Sync retries a failed import by what an added file can be imported as (its name, its stem, its folder), and skips a key that more than 500 failed imports share, as noise one file cannot answer (#2392). A path import is parked under its path's stem, so a key holds a project's own imports and every package import that ends the same way:
utilsis the stem ofjotai/utils,@electron-toolkit/utils, folo's own@follow/utils[/utils]workspace specifiers (which codegraph can't follow: it ignores a workspace package'sexportsmap) and of./utils/~/modules/settings/utils. A fresh index on current main already parks 485 failed imports there, andhooksholds 451. Delete and restoreapps/desktop/layer/renderer/src/modules/settings/utils.tsand its 21 imports pushutilspast 500, so the retry skips the key and none of them relink. Hold out every 7th source file instead, and it'shooksthat tips over (539): 88 imports of~/components/ui/modal/stacked/hooksand its neighbours stay unlinked.libs/common/src/auth/abstractions/account.service.ts's 724 importers and every binding and reference that hangs off them.assertholds 5,415 failed imports by tail (theassertpackage's module references and every binding ofimport * as assert from 'assert'). Delete and restoresrc/vs/base/common/assert.tsand its importers stay unlinked.Fix
A path key past the ceiling is now looked through for the imports that could name an added file, and those are retried (
getRetryableFailedImports' newcouldName, built byReferenceResolver.couldNameAnyOf(addedFiles)):resolveViaImport, now shared with it asjsModuleFileso the two can't drift — when a framework's alias resolves it into one, or when its last segment is an added file's name (the file-name strategy).resolvedModuleslookup is. bitwarden needs this: 724 files importlibs/common/src/auth/abstractions/account.service.ts, all of them through specifiers that resolve to it.names(namespace) andresolvedModuleslookups are untouched.Since a crowded key used to be skipped entirely, this can only add retries: a row the test rejects was not retried before either.
Why not park package imports elsewhere (the other idea in the brief)
I measured that split on current main's fresh indexes (package =
isOutOfRepoImport), and it doesn't reach most of the noise:utils(485): 40 rows are package imports, and 445 are folo's own@follow/utilsand@follow/utils/utilsworkspace specifiers, so the key would sit at 445 and pass 500 again with a slightly different change set.hooks(451): not one row is a package import. This is the key that actually tipped over in the hold-out scenario.assert(5,415): 2,692 rows are package module references, but 2,723 are bindings, which would keep it crowded.Re-keying would also change
name_tailin every fresh index (the dead-code report reads it as "maybe referenced"). And a package specifier with a file-like last segment (chart.js) is linked by file name in a fresh index, so moving it would be a convergence risk. The lookup-time test needs no schema change and works on existing indexes.Validation
Arms:
main= 0d33925,fix= this branch; both with the same native kernel built from main's source. Kit:D:\cgwork\cgceil.Re-run on a6cfb52 (after merging #2664/#2665 into the branch): the folo and bitwarden fresh controls are still byte-identical, and the folo/bitwarden hold-out and folo delete-and-restore numbers below are reproduced exactly, line for line.
Regression test —
__tests__/sync-import-retry-crowded-key.test.ts, three cases, each comparing the synced graph's file-to-file imports with a fresh index of the final tree in a second folder: a project'sutils.tsamong 520kitN/utilspackage imports (folo's shape),assert.tsamong 520import * as assert from 'assert'module refs and bindings (vscode's shape), and autils.tsthat 510 files import (bitwarden's shape). All three fail on main and pass here.sync-import-retry.test.tsgains agetRetryableFailedImportsunit test for the crowded-key pass, and its query-plan test now also covers the new per-key read (it stays onidx_unresolved_failed_import_tail/_name).Fresh indexes — controls, byte-identical (
scripts/dump-graph.mjs,cmp): folo, vben, milo-ui, ghostfolio, infrahub, excalidraw, bitwarden, gin (Go), django-realworld (Python) — 9/9 identical. The fix only acts in sync, and thejsModuleFileextraction fromresolveViaImportchanges nothing.Late scenario (every 7th source file held out, the rest indexed, the files restored,
codegraph sync; each arm's synced dump against its own fresh dump, which are identical across arms):~/components/ui/modal/stacked/hooks×69,./hooks, …: thehookskey reached 539)@bitwarden/common/auth/abstractions/account.service×679 and the 1,600 binding/reference edges that follow it,@bitwarden/auth/common×101, …)mapcall under the symbol retry's ceiling. excalidraw's 5 and 2 of bitwarden's areimport … from '../..', which waits under no key; that's a follow-up PR. bitwarden's other lines are Angularrouter.navigatein a spec, metadata-only and retargeted edges, and member calls under the symbol ceiling.FOREIGN KEY constraint failedon both arms. That'scodegraph syncfails with "FOREIGN KEY constraint failed" after d0bb9be5 when a file lazy-loads a workspace package #2565, and fix(sync): record only indexed files as synthesis inputs (#2565) #2574 is open for it.Delete and restore the file from the brief, folo's
apps/desktop/layer/renderer/src/modules/settings/utils.ts, then compare with the fresh index. On current main,utilsholds 485 failed imports in a fresh index, and the file's 21 imports push it past 500, so main leaves all 21 unlinked: 14 through~/modules/settings/utils, 7 through./utilsor../utils. The fix leaves none.vscode, delete and restore
src/vs/base/common/assert.ts(68 files import it): fresh dumps byte-identical across arms. Main's restore sync retries 28 refs and leaves 393 fresh-only lines. The fix retries 84 and leaves 317, recovering 76 lines and losing 0: 56 module imports of…/base/common/assert.js, plus 9assertbindings and 9assert(…)calls that follow them. Every remainingimportsline (127) was re-pointed while the file was gone, not left failed, so no retry sees it, on either arm:…/base/common/assert.jstotest/unit/assert.js, the only otherassert.js.assertto anotherassertfunction.That's an existing re-point gap (an import a sync should move to the file that outranks its current target), not this change's.
Cost and plans (vscode, the restored
assert.ts;assertholds 5,415 failed imports by tail and 5,405 by name, the two crowded lookups):assert.EXPLAIN QUERY PLANfor the per-key read:SEARCH unresolved_refs USING INDEX idx_unresolved_failed_import_tail (reference_kind=? AND name_tail=?), andidx_unresolved_failed_import_namefor the by-name lookup, the same partial indexes as the existing count queries. The query-plan test insync-import-retry.test.tsnow covers it.Overlap
None open. Builds on #2392 (the import retry), #2450 (
module:keys) and theresolvedModules/folder-import lookups on main.🤖 Generated with Claude Code