refactor(import-target): make the key-shape rule a type, drop three censuses

Cleanup pass over the #2911 review-fix commits. No behaviour change: all 85
per-language fingerprints, every `resolved` and every `distinct_outcomes` are
byte-identical, and the targeted suite is 1851/1851.

MEASURED — `byBasename` was 71% empty array slots

`byBasename` holds roughly one bucket per file, and building each with `[]`
followed by `push` makes V8 grow the backing store to its 16-slot minimum, so
every single-file bucket retained 15 empty pointer slots. Constructing the
one-element bucket directly is byte-identical in contents and 5.50 -> 1.60 MiB
at 32000 `.py` paths. The bench arm reads 10543848 -> 6360936 B (-39.7%);
`heap_reading_bytes.python` and its ceiling are re-recorded. The same edit
shares one `{ raw, norm }` between both maps instead of allocating a second
literal for every `__init__.py`.

THE RULE THAT COST A TIMING RATIO TO FIND IS NOW A COMPILE ERROR

`perFileSet`'s key is narrowed from `object` to
`ReadonlySet<string> | readonly ParsedFile[]`. Reintroducing the #2911 defect
shape — a memo keyed on an array materialized from the file set — now fails
with TS2345 instead of silently minting a fresh `WeakMap` key per import while
traversing the Set zero extra times, which every scan-counting guard reads as
green at its correct value.

That also retires the header's hand-maintained roster of `ParsedFile[]`-keyed
call sites, which listed three — this PR added a fourth in `395c707d4` and did
not update it. A census inside a comment warning that censuses go stale, stale
inside one commit. The header now names shapes; the compiler names sites.

Two more claims that had drifted from their code:

- `per-file-set.ts` asserted "No index derived from the file set is keyed on an
  ARRAY materialized from it". `configs/swift.ts` is, deliberately, with its
  reasons written down. Two files in one directory disagreeing is worse than
  either; the rule now states what the type rejects and names the exception.
- `SuffixIndex.getFilesInDir`'s doc explained that it returns the index's own
  bucket by reference. True of `buildSuffixIndex`; the other implementation of
  that interface, in `languages/php/import-target.ts`, returns a filtered copy.
  The interface now carries only the caller-facing contract (`readonly`, do not
  mutate) and the sharing rationale moved onto the implementation it describes.
- The contract test still described Python as having "NO memo on this key".
  `parsedFileByPath` landed in `395c707d4`; the floor of 1 is now its single
  build rather than a per-import scan.

DEDUP

`importerDirOf` replaces four copies of `replace / lastIndexOf / slice` — two in
production, where one was a memo KEY and the other a memo's query argument, so
the two per-directory memos in one index agreed only by inspection. The tests
keep their own verbatim derivation on purpose: importing production's would
make the key lookup agree by construction and hide a regression.

`buildParsedFiles` maps through `probeFile` instead of repeating its 7-field
literal 900 lines away; `requireNumericBudget` and `expectNoOrphanKeys` replace
three and three copies, with every per-arm `why` kept per-arm. The two Python
memo guards collapse onto shared arms in `test/helpers/counting-file-set.ts` —
1847 tests before and after, and both still go red under mutation.

SKIPPED, with reasons: dropping `normSet` for bucket scans (trades O(1) probes
on the hot path for ~1.6 MB against a 6.4 MB reading); measuring heap for all
17 languages (+9 s and a design decision, not a cleanup); `readonly` on the
five sibling resolvers' array parameters and the `getDirMap` slice/join rewrite
(both correct, both outside this diff).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NtGZN6YSNU8chALSnKYHBp
This commit is contained in:
Gergo Magyar 2026-08-10 12:49:51 +00:00
parent f3d467949c
commit 66dbfafa36
14 changed files with 759 additions and 509 deletions

File diff suppressed because one or more lines are too long

View file

@ -834,17 +834,45 @@ function buildFiles(lang, fileCount, pad, shape) {
}
/**
* The `ParsedFile[]` the orchestrator threads beside the path set, for the two
* languages whose hook declares a `context` — see `CONTEXT_LANGS`.
* ONE `ParsedFile`, and the ONE place in this file that spells that shape.
*
* CARRIES THE FIELDS THE RESOLVERS READ AND NOTHING ELSE, deliberately.
* `filesByDirectory` reads `filePath`; PHP's declaring-file filter reads
* `localDefs[].type` and `localDefs[].qualifiedName`; Python's
* `pythonFileExportsName` reads `localDefs[].qualifiedName`. `scopes`,
* `parsedImports` and `referenceSites` are on the real shape and are inert on
* this path, and this array is rebuilt inside every timed pass (see `newPass`),
* so filling them would charge the RESOLUTION arms for extraction work that
* happens in another phase entirely.
* this path, and the timed corpora are rebuilt inside every pass (see
* `newPass`), so filling them would charge the RESOLUTION arms for extraction
* work that happens in another phase entirely.
*
* `nodeId` is inert as well — checked, not assumed: neither
* `php/import-target.ts` nor `python/import-target.ts` mentions it, and they are
* the two modules `resolveOne` enters. It is minted anyway because it is on the
* real shape, and its spelling is therefore free to be uniform.
*
* Both callers come through here — `buildParsedFiles` for the timed and heap
* corpora, `CONTEXT_PROBE` for the `context` arm's hand-built ones. It used to
* be spelled out twice, ~900 lines apart, differing only in that `nodeId`; this
* is an untyped `.mjs`, so nothing would have failed at build if `ParsedFile`
* grew a field and only one of the two copies learned about it.
*/
const probeFile = (filePath, defs) => ({
filePath,
moduleScope: filePath,
scopes: [],
parsedImports: [],
localDefs: defs.map(([type, qualifiedName], n) => ({
nodeId: `${filePath}#${n}`,
filePath,
type,
qualifiedName,
})),
referenceSites: [],
});
/**
* The `ParsedFile[]` the orchestrator threads beside the path set, for the two
* languages whose hook declares a `context` — see `CONTEXT_LANGS`.
*
* Two defs per file, and both are real shapes rather than padding. PHP keeps
* classes and functions in SEPARATE symbol tables, so `App\Ns7\File7` naming
@ -870,17 +898,12 @@ function buildParsedFiles(lang, files) {
const parent = slash < 0 ? '' : filePath.slice(0, slash);
const owner = parent.slice(parent.lastIndexOf('/') + 1);
const qualifiedName = lang === 'php' ? `App\\${owner}\\${stem}` : `${owner}.${stem}`;
parsedFiles.push({
filePath,
moduleScope: filePath,
scopes: [],
parsedImports: [],
localDefs: [
{ nodeId: `${filePath}#c`, filePath, type: 'Class', qualifiedName },
{ nodeId: `${filePath}#f`, filePath, type: 'Function', qualifiedName },
],
referenceSites: [],
});
parsedFiles.push(
probeFile(filePath, [
['Class', qualifiedName],
['Function', qualifiedName],
]),
);
}
return parsedFiles;
}
@ -1772,23 +1795,6 @@ function measureHeap(lang) {
};
}
/** One hand-built `ParsedFile` for the `context` arm's corpora. Same fields
* `buildParsedFiles` fills, with the defs named explicitly because these two
* corpora exist to produce ONE specific disagreement each. */
const probeFile = (filePath, defs) => ({
filePath,
moduleScope: filePath,
scopes: [],
parsedImports: [],
localDefs: defs.map(([type, qualifiedName], n) => ({
nodeId: `${filePath}#${n}`,
filePath,
type,
qualifiedName,
})),
referenceSites: [],
});
/**
* The `context` arm's corpora — one per `CONTEXT_LANGS` entry, each a handful
* of files carrying ONE import whose answer DIFFERS between the production
@ -2014,6 +2020,58 @@ if (!CHECK) {
const baseline = JSON.parse(fs.readFileSync(BASELINE_PATH, 'utf-8'));
const failures = [];
/**
* PRESENCE, for one budget, in the one place that spells the reason.
*
* A missing budget is a DELETED GATE, not a passing arm: `got > undefined` is
* `false`, `ceiling * undefined` is `NaN` and `bytes < NaN` is `false`, so every
* comparison in this file answers "within budget" for every possible
* measurement the moment its key stops being a number. Each of the three call
* sites below is one deleted key away from a silent no-op, and the run still
* prints PASS.
*
* `Number.isFinite` rather than `typeof === 'number'`: over JSON input the two
* agree (JSON cannot express NaN or Infinity), and the stricter one is the one
* whose name says what the gate needs.
*
* The two per-site facts stay the caller's, because they are what a triager acts
* on: `reads` is the comparison that silently stopped gating, quoted, and
* `scope` is what deleting this one key actually costs — a single arm, or all
* eight at once. Only the shared framing and the shared trailing sentence live
* here. Returns the message rather than pushing it, so the timing loop can
* `continue` past a budget it must not then compare against.
*/
const requireNumericBudget = ({ key, value, reads, scope }) =>
Number.isFinite(value)
? null
: `no numeric ${key} in baselines.json — a missing budget is a DELETED GATE, not a passing ` +
`arm: the comparison it gates reads \`${reads}\`, which is false for every possible ` +
`measurement. ${scope} Deterministic: a re-run will not change it.`;
/**
* The REVERSE direction of a reconciliation: every key declared in `label` that
* `codeList` does not name.
*
* The forward direction ("the code has an arm with no budget") is a presence
* check inside whichever loop iterates the code's list. This is the other way
* round — a budget, a baseline block or a registry row for an arm that is never
* measured — and no forward check can see it, because the thing it names is
* exactly the thing nothing iterates.
*
* `codeListName` and `why` stay the caller's: which list is authoritative and
* what the orphan costs are the two facts that differ between the three arms,
* and flattening them would leave a triager with a name and no reading of it.
*/
function expectNoOrphanKeys(label, declaredKeys, codeList, codeListName, why) {
for (const key of declaredKeys) {
if (codeList.includes(key)) continue;
failures.push(
`${label} has an entry for '${key}', which is not in ${codeListName} — ${why} ` +
`Deterministic: a re-run will not change it.`,
);
}
}
/** The corpus-shape facts asserted for one timing scale. */
const SCALE_SHAPE = {
fields: ['files', 'imports', 'resolved', 'distinct_outcomes', 'fingerprint'],
@ -2120,21 +2178,21 @@ for (const lang of LANGS) {
},
];
for (const check of timingChecks) {
// PRESENCE FIRST, because `got > undefined` is `false`: without this, going
// from a budget to no budget is going from a gate to no gate, and the run
// still prints PASS. All five maps are complete today, which is exactly
// when the check is worth adding — every one of the four per-language
// lookups above is one deleted key away from a silent no-op. The heap arm
// HAD THE SAME HOLE and this comment used to deny it: iterating the
// BASELINE's keys protects that loop against a deleted MEASUREMENT, which
// is a different thing from a deleted BUDGET. See `heapBudgetChecks`.
if (typeof check.budget !== 'number') {
failures.push(
`${lang}: no ${check.label} budget — baselines.json has no numeric ${check.key}. ` +
`A missing budget is a DELETED GATE, not a passing arm: the comparison below reads ` +
`\`${check.got} > undefined\`, which is false for every possible measurement. ` +
`Deterministic: a re-run will not change it.`,
);
// PRESENCE FIRST — see `requireNumericBudget` for why. All five maps are
// complete today, which is exactly when the check is worth having: every one
// of the four per-language lookups above is one deleted key away from a
// silent no-op. The heap arm HAD THE SAME HOLE and the comment here used to
// deny it: iterating the BASELINE's keys protects that loop against a
// deleted MEASUREMENT, which is a different thing from a deleted BUDGET.
// See `heapBudgetChecks`.
const missing = requireNumericBudget({
key: check.key,
value: check.budget,
reads: `${check.got} > undefined`,
scope: `That leaves ${lang}'s ${check.label} arm ungated.`,
});
if (missing !== null) {
failures.push(`${lang}: ${missing}`);
continue;
}
if (check.got > check.budget) {
@ -2211,23 +2269,19 @@ for (const lang of LANGS) {
// five timing budgets get it — and the reason the comment up there used to give
// for the heap arm not needing it was wrong. Iterating the baseline's keys
// protects the loop below against a deleted MEASUREMENT (`heap == null`, right
// there); it does nothing about a deleted BUDGET. `ceiling * undefined` is
// `NaN` and `bytes_large < NaN` is false, `ratio > undefined` is false: these
// two keys are scalars rather than per-language maps, so deleting either is one
// keystroke that silently disables that arm for ALL EIGHT languages at once.
// That makes them the widest-blast-radius keys in this file, not the safest.
// there); it does nothing about a deleted BUDGET. These two keys are scalars
// rather than per-language maps, so deleting either is one keystroke that
// silently disables that arm for ALL EIGHT languages at once. That makes them
// the widest-blast-radius keys in this file, not the safest — which is what
// their `scope` sentence says and the per-language ones do not.
const heapBudgetChecks = [
{ key: 'heap_floor_fraction', value: baseline.heap_floor_fraction, reads: 'bytes_large < NaN' },
{ key: 'heap_ratio_budget', value: baseline.heap_ratio_budget, reads: 'ratio > undefined' },
];
const heapArmScope = `This one key gates all ${HEAP_LANGS.length} heap arms at once.`;
for (const check of heapBudgetChecks) {
if (Number.isFinite(check.value)) continue;
failures.push(
`no numeric ${check.key} in baselines.json — a missing budget is a DELETED GATE, not a ` +
`passing arm: the comparison below reads \`${check.reads}\`, which is false for every ` +
`possible measurement. This one key gates all ${HEAP_LANGS.length} heap arms at once. ` +
`Deterministic: a re-run will not change it.`,
);
const missing = requireNumericBudget({ ...check, scope: heapArmScope });
if (missing !== null) failures.push(missing);
}
// And EXACT KEY EQUALITY between `HEAP_LANGS` and the two per-language heap
@ -2242,13 +2296,13 @@ const heapBudgetMaps = [
['heap_reading_bytes', baseline.heap_reading_bytes],
];
for (const [key, map] of heapBudgetMaps) {
for (const lang of Object.keys(map ?? {})) {
if (HEAP_LANGS.includes(lang)) continue;
failures.push(
`baselines.json ${key} has an entry for '${lang}', which is not in HEAP_LANGS — the bench ` +
`budgets a heap arm it does not measure. Deterministic: a re-run will not change it.`,
);
}
expectNoOrphanKeys(
`baselines.json ${key}`,
Object.keys(map ?? {}),
HEAP_LANGS,
'HEAP_LANGS',
'the bench budgets a heap arm it does not measure.',
);
}
// The same reverse direction for the context arm. The forward direction (a
@ -2257,15 +2311,13 @@ for (const [key, map] of heapBudgetMaps) {
// other way round — a baseline block for a language the bench hands no context
// is a gate over an arm that is never measured, and `armShapes` would never
// look at it.
for (const lang of Object.keys(baseline.languages)) {
if (baseline.languages[lang].context === undefined) continue;
if (CONTEXT_LANGS.includes(lang)) continue;
failures.push(
`baselines.json languages.${lang} has a context block, but '${lang}' is not in ` +
`CONTEXT_LANGS — the bench pins an arm it does not run. Deterministic: a re-run will not ` +
`change it.`,
);
}
expectNoOrphanKeys(
'baselines.json languages.*.context',
Object.keys(baseline.languages).filter((lang) => baseline.languages[lang].context !== undefined),
CONTEXT_LANGS,
'CONTEXT_LANGS',
'the bench pins an arm it does not run.',
);
// Driven by HEAP_LANGS, the CODE's list, exactly as the timing arms iterate
// LANGS — so a deleted budget key is a presence failure rather than a language
@ -2275,17 +2327,22 @@ for (const lang of Object.keys(baseline.languages)) {
for (const lang of HEAP_LANGS) {
const ceiling = baseline.heap_ceiling_bytes?.[lang];
const reading = baseline.heap_reading_bytes?.[lang];
for (const [key, value] of [
['heap_ceiling_bytes', ceiling],
['heap_reading_bytes', reading],
// `reads` names the comparison each key gates further down: the ceiling is
// compared directly, the reading only after `reading * heap_floor_fraction`
// has turned a missing one into `NaN`.
for (const [key, value, reads] of [
['heap_ceiling_bytes', ceiling, 'bytes_large > undefined'],
['heap_reading_bytes', reading, 'bytes_large < NaN'],
]) {
if (Number.isFinite(value)) continue;
failures.push(
`${lang}: no numeric ${key}.${lang} in baselines.json — a missing budget is a DELETED ` +
`GATE, not a passing arm, and this loop iterates HEAP_LANGS precisely so that deleting ` +
`the key fails here instead of dropping ${lang} out of the gate. Deterministic: a ` +
`re-run will not change it.`,
);
const missing = requireNumericBudget({
key: `${key}.${lang}`,
value,
reads,
scope:
`This loop iterates HEAP_LANGS precisely so that deleting the key fails here instead ` +
`of dropping ${lang} out of the gate.`,
});
if (missing !== null) failures.push(`${lang}: ${missing}`);
}
const heap = report[lang]?.heap;
if (heap == null) {
@ -2382,13 +2439,16 @@ for (const language of registeredLanguages) {
`resolveOne) and a baselines.json entry. Deterministic: a re-run will not change it.`,
);
}
for (const language of benchedLanguages) {
if (registeredLanguages.includes(language)) continue;
failures.push(
`LANG_REGISTRY names '${language}', which is not in SCOPE_RESOLVERS — this bench is gating a ` +
`resolver the pipeline no longer registers. Deterministic: a re-run will not change it.`,
);
}
// The reverse half is the same loop as the two above it, so it goes through the
// same helper. Only the FORWARD half stays written out: its message is a
// five-step remediation for adding a language, which no shared framing carries.
expectNoOrphanKeys(
'LANG_REGISTRY',
benchedLanguages,
registeredLanguages,
'SCOPE_RESOLVERS',
'this bench is gating a resolver the pipeline no longer registers.',
);
// The SAME reconciliation for `CONTEXT_LANGS`, against the registry rather than
// against a claim in a comment. `run.ts` passes the fifth argument to every

View file

@ -22,33 +22,36 @@
* does not grow across runs. There is no invalidation rule to get wrong because
* there is nothing to invalidate — a new file set is a new key.
*
* `K extends object` because the key is whatever object the pass keeps stable.
* Stability for the pass is the only property any of them needs — but WHICH
* object is the key decides whether the guards above can see the memo fail, and
* only one shape is instrumentable by them:
* The KEY TYPE is constrained rather than described, because which object is
* the key decides whether the guards above can see the memo fail, and a prose
* list of call sites is the thing this file elsewhere tells you not to write.
* `K` admits exactly the two shapes the orchestrator keeps stable for a pass:
*
* - `ReadonlySet<string>`, the pass's file set — every index derived from the
* file set, including the derived header-closure sets that
* `languages/{c,cpp}/scope-resolver.ts` memoize inside an outer per-file-set
* memo. Defeating one of these means copying the SET, which re-traverses it,
* which the `CountingSet` instrument (`test/helpers/counting-file-set.ts`)
* reads as a scan count rising with the import count.
* - `readonly ParsedFile[]`, the pass's parsed-file array — `languages/c/
* static-linkage.ts`, `languages/cpp/file-local-linkage.ts` and
* `languages/php/import-target.ts`. Not derived from the file set at all, so
* the file-set guards do not reach them; these key on the array the
* orchestrator already threads through the pass, and their contract is that
* same pass-through discipline.
* - `ReadonlySet<string>`, the pass's file set — every index derived from it,
* including the derived header-closure sets that `languages/{c,cpp}/
* scope-resolver.ts` memoize inside an outer per-file-set memo. Defeating
* one of these means copying the SET, which re-traverses it, which the
* `CountingSet` instrument (`test/helpers/counting-file-set.ts`) reads as a
* scan count rising with the import count.
* - `readonly ParsedFile[]`, the pass's parsed-file array. Not derived from
* the file set at all, so the file-set guards do not reach them; these key
* on the array the orchestrator already threads through the pass, and their
* contract is that same pass-through discipline. The instrument that CAN see
* them counts element reads on that array — `countedParsedFiles`, beside
* `CountingSet`, driven by the contract test's `minimumParsedFileReads`.
*
* No index derived from the file set is keyed on an ARRAY materialized from it.
* `import-resolvers/csharp.ts` was, until #2911, and the reason it is worth
* stating as a rule: copying an array mints a fresh `WeakMap` key while
* traversing the Set zero extra times, so every scan-counting guard stays green
* at its correct value while the index rebuilds once per import. That failure
* is invisible to the whole instrument family above and was caught only by a
* timing ratio in `bench/import-target/`. Derive the array inside the builder
* from `getWorkspaceFileIndex(allFilePaths)` instead, and the Set stays the
* single key shape and the single thing that has to be passed through.
* A THIRD shape — an array materialized from the file set — is what the type
* exists to reject. `import-resolvers/csharp.ts` used one until #2911, and it
* is worth a compile error rather than a rule: copying an array mints a fresh
* `WeakMap` key while traversing the Set zero extra times, so every
* scan-counting guard stays green at its correct value while the index rebuilds
* once per import. That failure is invisible to the whole instrument family
* above and was caught only by a timing ratio in `bench/import-target/`. Derive
* the array inside the builder from `getWorkspaceFileIndex(allFilePaths)`
* instead. `string[]` is not assignable to `K`, so the shape cannot come back
* silently — `configs/swift.ts` keeps the one hand-rolled `WeakMap` on
* `ctx.allFileList` in the tree, deliberately and with its reasons written
* down, and it is deliberately NOT on this primitive.
*
* `T extends object` is deliberate, chosen over probing `has` before `get`.
* `WeakMap.get` returning `undefined` cannot distinguish "not built yet" from
@ -64,7 +67,9 @@
* Inert for the builders here — each is a pure, total pass over the file set —
* and the safer of the two behaviours if that ever stops being true.
*/
export function perFileSet<K extends object, T extends object>(
import type { ParsedFile } from 'gitnexus-shared';
export function perFileSet<K extends ReadonlySet<string> | readonly ParsedFile[], T extends object>(
build: (key: K) => T,
): (key: K) => T {
const cache = new WeakMap<K, T>();

View file

@ -52,10 +52,24 @@ import { perFileSet } from './per-file-set.js';
* computed it — `norm.split('/').slice(0, -1).join('/')`, which for a path
* without a separator is `''` (a root-level importer, whose chain is empty).
*/
export function importerAncestors(index: PythonFileIndex, fromFile: string): readonly string[] {
/**
* The importer's own directory, normalized — the key BOTH per-directory memos
* below are stored under.
*
* One exported derivation rather than one per accessor: the two memos live in
* the same index and must agree on what "the importer's directory" is, and a
* caller that already holds the directory (the bare-import tier computes it for
* its own proximity check) should not pay for it twice. It was three copies of
* `replace / lastIndexOf / slice` across two modules before, byte-identical by
* inspection and by nothing else.
*/
export function importerDirOf(fromFile: string): string {
const norm = fromFile.replace(/\\/g, '/');
const lastSlash = norm.lastIndexOf('/');
const importerDir = lastSlash === -1 ? '' : norm.slice(0, lastSlash);
return lastSlash === -1 ? '' : norm.slice(0, lastSlash);
}
export function importerAncestors(index: PythonFileIndex, importerDir: string): readonly string[] {
const memoized = index.ancestorsByDir.get(importerDir);
if (memoized !== undefined) return memoized;
const built = buildImporterAncestors(importerDir);
@ -158,14 +172,24 @@ export const getPythonFileIndex = perFileSet(
if (!norm.endsWith('.py')) continue;
normSet.add(norm);
// ONE entry object per file, shared by both buckets below: a package file
// lands in `byBasename` and `byInitParent`, and two literals for the same
// `(raw, norm)` pair cost ~40 B each on every `__init__.py`.
const entry = { raw, norm };
const lastSlash = norm.lastIndexOf('/');
const base = lastSlash >= 0 ? norm.slice(lastSlash + 1) : norm;
let bucket = byBasename.get(base);
if (bucket === undefined) {
bucket = [];
byBasename.set(base, bucket);
}
bucket.push({ raw, norm });
// `set(base, [entry])` rather than `set(base, [])` then `push`: an empty
// array literal that is immediately pushed to makes V8 grow the backing
// store to its 16-slot minimum, so every bucket holding ONE file retains
// 15 empty pointer slots — 128 B — for the whole pass. `byBasename` has
// roughly one bucket per file, which made that the dominant term in this
// index: measured 5.50 MiB against 1.60 MiB for the one-element form at
// 32 000 `.py` paths, byte-identical contents. Same shape as
// `languages/php/import-target.ts`'s directory buckets.
const bucket = byBasename.get(base);
if (bucket === undefined) byBasename.set(base, [entry]);
else bucket.push(entry);
// Package files also get a parent-keyed bucket so a `pkg.sub` lookup hits
// only `…/sub/__init__.py` candidates, not every `__init__.py` (P2b).
@ -175,12 +199,9 @@ export const getPythonFileIndex = perFileSet(
const parentName = parentSlash >= 0 ? dir.slice(parentSlash + 1) : dir;
if (parentName) {
const initKey = `${parentName}/__init__.py`;
let ib = byInitParent.get(initKey);
if (ib === undefined) {
ib = [];
byInitParent.set(initKey, ib);
}
ib.push({ raw, norm });
const ib = byInitParent.get(initKey);
if (ib === undefined) byInitParent.set(initKey, [entry]);
else ib.push(entry);
}
}

View file

@ -9,6 +9,7 @@
import {
getPythonFileIndex,
importerBarePrefixes,
importerDirOf,
pythonSegmentAbsent,
} from './python-file-index.js';
import { tryResolveWithExtensions } from './utils.js';
@ -69,13 +70,11 @@ export function resolvePythonImportInternal(
const index = getPythonFileIndex(allFiles);
if (pythonSegmentAbsent(index, pathLike)) return null;
// Normalize for Windows backslashes. `lastIndexOf` rather than
// `split('/').slice(0, -1).join('/')`: identical for every input (a path with
// no separator has no directory, which is `''` both ways) without the
// per-import array of one element per path component.
const norm = currentFile.replace(/\\/g, '/');
const lastSlash = norm.lastIndexOf('/');
const importerDir = lastSlash === -1 ? '' : norm.slice(0, lastSlash);
// One derivation, shared with the index's other per-directory memo — see
// `importerDirOf`. It replaced `split('/').slice(0, -1).join('/')`: identical
// for every input (a path with no separator has no directory, which is `''`
// both ways) without the per-import array of one element per path component.
const importerDir = importerDirOf(currentFile);
// Proximity check — only applies when the importer lives in a subdirectory.
// Root-level importers (importerDir === '') skip straight to the ancestor

View file

@ -97,23 +97,18 @@ export interface SuffixIndex {
/**
* Get all files in a directory suffix.
*
* The directory map behind this is built on the FIRST call and memoized —
* see `buildSuffixIndex`. Callers that never ask a directory question never
* pay for it.
* `readonly` is the CONTRACT, and it is the contract for every implementation
* of this interface, not a description of any one of them: an implementation
* is free to return its own bucket by reference, so callers must treat the
* result as shared and never `sort`/`splice` it in place. The compiler now
* refuses that at the call site. Whether a given implementation shares or
* copies is its own business and documented where it is built —
* `buildSuffixIndex` shares, the root-anchored parity index in
* `languages/php/import-target.ts` returns a filtered copy.
*
* Returns the index's OWN bucket, by reference, and `readonly` is the
* contract rather than a comment about one: the map is built once and then
* held for the whole pass, so an in-place `sort`/`splice` would corrupt every
* later import in that pass, and the compiler now refuses it at the call
* site. Sharing beats copying here because no caller keeps the array — of the
* four (see `dirMap` below) two only measure it (`configs/python.ts` reads
* `.length`, `php.ts` reads `[0]`) and two build a fresh array from it
* (`jvm.ts` flatMaps, `csharp.ts` pushes into its own `results`) — so a
* defensive copy would allocate a whole bucket per import on the path this
* index exists to keep flat. `package-dir-index.ts` reached the same
* conclusion for the same reason and states it the same way, with read-only
* containers plus one copy where a bucket genuinely LEAVES (`sortedRootFiles`);
* `configs/swift.ts` is that leaving case and copies.
* Implementations that memoize should note the directory map behind this may
* be built on the FIRST call rather than up front, so a caller that never
* asks a directory question never pays for it — see `buildSuffixIndex`.
*/
getFilesInDir(dirSuffix: string, extension: string): readonly string[];
}
@ -330,6 +325,18 @@ export function buildSuffixIndex(
return {
get: (suffix: string) => getExactMap().get(suffix),
getInsensitive: (suffix: string) => getLowerMap().get(suffix.toLowerCase()),
// THIS implementation shares: it hands back `dirMap`'s own bucket rather
// than a copy. The map is built on first query and then held for the whole
// pass, so the window in which a mutating caller could corrupt later
// imports is the whole pass — which is why the interface makes the result
// `readonly` and the compiler refuses the mutation at the call site.
//
// Sharing beats copying because no caller keeps the array: two only measure
// it and two build a fresh array from it, so a defensive copy would
// allocate a whole bucket per import on the path this index exists to keep
// flat. `package-dir-index.ts` reached the same conclusion the same way —
// read-only containers, plus one copy where a bucket genuinely LEAVES
// (`sortedRootFiles`), which is the case `configs/swift.ts` is in.
getFilesInDir: (dirSuffix: string, extension: string) => {
return getDirMap().get(`${dirSuffix}:${extension}`) || [];
},

View file

@ -12,7 +12,11 @@
import type { ParsedFile, ParsedImport, WorkspaceIndex } from 'gitnexus-shared';
import { perFileSet } from '../../import-resolvers/per-file-set.js';
import { getPythonFileIndex, importerAncestors } from '../../import-resolvers/python-file-index.js';
import {
getPythonFileIndex,
importerAncestors,
importerDirOf,
} from '../../import-resolvers/python-file-index.js';
import { resolvePythonImportInternal } from '../../import-resolvers/python.js';
export interface PythonResolveContext {
@ -284,7 +288,7 @@ function resolveAbsoluteFromFiles(
// The chain comes from `importerAncestors`, which builds it ONCE per importer
// directory per pass. Rebuilding it here — one `slice(0, i).join('/')` per
// path component, on every import — was half of the depth quadratic in #2913.
for (const ancestor of importerAncestors(index, fromFile)) {
for (const ancestor of importerAncestors(index, importerDirOf(fromFile))) {
if (mayBeModule) {
const candidateFile = `${ancestor}/${directFile}`;
if (allFilePaths.has(candidateFile)) return candidateFile;
@ -394,7 +398,7 @@ function hasRepoCandidate(
// only be a directory prefix if `seg` names a directory under the non-empty
// parent `A`, so a miss here means the old loop would have missed too.
if (!index.nestedDirNames.has(leadingSegment)) return false;
for (const ancestor of importerAncestors(index, fromFile)) {
for (const ancestor of importerAncestors(index, importerDirOf(fromFile))) {
if (index.dirPrefixes.has(`${ancestor}/${prefix}`)) return true;
}
return false;

View file

@ -57,10 +57,14 @@
*
* `expectDistinctFileSetsGetOwnIndex` below is the one arm of those guards that
* is identical in every language once the four values that differ are named, so
* it lives here beside the instrument it reads rather than in each guard.
* it lives here beside the instrument it reads rather than in each guard. The
* `ChainMemoArm` section at the bottom applies the same rule to the guards that
* watch a MEMO instead of a scan count — the two Python importer-chain guards,
* which this instrument provably cannot see (their headers say why) and which
* were arm-for-arm the same suite written twice.
*/
import { expect } from 'vitest';
import type { ParsedFile } from 'gitnexus-shared';
import type { ParsedFile, ParsedImport } from 'gitnexus-shared';
import type { ScopeResolver } from '../../src/core/ingestion/scope-resolution/contract/scope-resolver.js';
export class CountingSet extends Set<string> {
@ -245,3 +249,240 @@ export function expectDistinctFileSetsGetOwnIndex(arm: DistinctFileSetArm): void
expect(a.scans).toBe(arm.expectedScans);
expect(b.scans).toBe(arm.expectedScans);
}
// ─── Python import shapes ────────────────────────────────────────────────────
/**
* `from <targetRaw> import Widget`. The shape that makes
* `resolvePythonImportTarget` run the package-attribute probe
* (`pythonFileExportsName`, the `context.parsedFiles` reader) ahead of the
* submodule fallback — so it is the shape that re-enters the resolver and pays
* the importer's chain TWICE, and the shape `pythonImportedSubmoduleTarget`
* fires for.
*
* The default the adapter synthesizes when `context` is absent is a `namespace`
* import, and that shape never reaches the probe. A guard that means to measure
* either leg therefore has to pass this one explicitly.
*/
export const pythonNamedImport = (targetRaw: string): ParsedImport => ({
kind: 'named',
localName: 'Widget',
importedName: 'Widget',
targetRaw,
});
/** `import <targetRaw>` — the single-walk shape, and the adapter's default. */
export const pythonNamespaceImport = (targetRaw: string): ParsedImport => ({
kind: 'namespace',
localName: '_',
importedName: '_',
targetRaw,
});
/**
* ONE array per file that uses it, never a fresh `[]` per call:
* `parsedFileByPath` memoizes on its identity, and a new array per import would
* mint a `WeakMap` key per import for a channel these guards are not measuring
* (`countedParsedFiles` above is the instrument for that one). Empty, so
* `pythonFileExportsName` answers false and the package-vs-submodule precedence
* never fires — the walk, not the precedence, is what the numbers measure.
*/
export const NO_PARSED_FILES: readonly ParsedFile[] = [];
// ─── the Python importer-chain memo guards ───────────────────────────────────
/**
* What one resolution answered, as the chain-memo arms read it: a path, a path
* list (the `ScopeResolver` signature allows one), or `null`. The two values
* the non-vacuity pairing rule counts are `arm.hitResult` and `null`.
*/
export type ChainMemoResult = string | readonly string[] | null;
/**
* Everything that differs between the two Python importer-chain memo guards:
* `test/unit/import-resolvers/python-importer-prefixes.test.ts`
* (`bareImportPrefixesByDir`) and
* `test/unit/scope-resolution/python/python-importer-ancestors.test.ts`
* (`ancestorsByDir`).
*
* The two memos hold DIFFERENT SEQUENCES under the same key — self included or
* not, workspace root included or not, empty components kept or dropped; see
* `importerBarePrefixes`'s header for why neither guard can be deleted in
* favour of the other. But each guard is the same four arms over the same
* importer corpus once these four values are named, so the arms live here and
* each guard supplies its own four.
*/
export interface ChainMemoArm {
/** The memo under test, read off the pass's per-file-set index. */
readonly memoOf: (files: ReadonlySet<string>) => ReadonlyMap<string, readonly string[]>;
/**
* Drives a production surface `perImporter` times from `fromFile`, with
* spellings that reach the memo, and answers what each call resolved to.
* Exactly one call per invocation must answer `arm.hitResult`, and at least
* one must answer `null`. Must pass `files` THROUGH: both memos are keyed on
* its identity, so a copy here would measure nothing.
*/
readonly drive: (
files: Set<string>,
fromFile: string,
perImporter: number,
) => readonly ChainMemoResult[];
/**
* The verbatim pre-change chain builder, which is the specification: the memo
* agreeing with it is what makes the change a hoist rather than a behaviour
* change.
*/
readonly legacyChain: (fromFile: string) => readonly string[];
/** What the one must-resolve spelling in `drive` answers, once per importer. */
readonly hitResult: string;
}
/** Every file that issues an import in the chain-memo arms. */
export const CHAIN_MEMO_IMPORTERS: readonly string[] = [
'svc/a/one.py',
'svc/a/two.py',
'svc/b/one.py',
'deep/x/y/z/one.py',
'root.py',
];
/** Four directories for those five importers — `svc/a` holds two of them. */
export const CHAIN_MEMO_IMPORTER_DIRS: readonly string[] = ['svc/a', 'svc/b', 'deep/x/y/z', ''];
/** The directory two importers share, which is where identity is measured. */
const SHARED_DIR = 'svc/a';
const SHARED_DIR_IMPORTERS: readonly string[] = ['svc/a/one.py', 'svc/a/two.py'];
/** Imports one directory issues before its chain's identity is re-read. */
const CHAIN_IDENTITY_REPEATS = 40;
/** A sorted copy, so a key set is compared without depending on fill order. */
export const sortedStrings = (values: Iterable<string>): string[] => [...values].sort();
/**
* The importer directory both memos are keyed on, derived exactly as the
* pre-change inline code derived it — `''` for a path with no separator.
*/
const importerDirOf = (fromFile: string): string =>
fromFile.replace(/\\/g, '/').split('/').slice(0, -1).join('/');
/**
* The path-shape space an importer chain has to be correct over, as ONE table
* both guards run: they enumerate the same space and nothing kept the two
* copies in lockstep.
*
* Each `why` names the SHAPE, not what either chain does with it, because the
* two chains do different things with several of these rows — the bare-prefix
* chain KEEPS the empty component an absolute path or a doubled separator
* produces and the ancestor chain drops it, and that difference decides real
* resolutions. Which is why every arm below compares against the guard's own
* `legacyChain` rather than against a shared expectation.
*/
export const IMPORTER_PATH_SHAPES: readonly { readonly fromFile: string; readonly why: string }[] =
[
{ fromFile: 'svc/a/one.py', why: 'a two-component directory' },
{ fromFile: 'deep/x/y/z/one.py', why: 'a four-component directory' },
{ fromFile: 'root.py', why: 'a workspace-root importer' },
{ fromFile: '/abs/svc/a/one.py', why: 'an absolute path (leading empty component)' },
{ fromFile: 'svc//a/one.py', why: 'a doubled separator (empty component)' },
{ fromFile: 'svc\\a\\one.py', why: 'Windows separators' },
{ fromFile: 'trailing/', why: 'a path ending in a separator' },
];
/**
* The gate: N imports from five importers over four directories leave FOUR
* entries, for every N. That is "the chain work is O(1) amortized after the
* first import from a given directory", stated as a number. A chain rebuilt per
* import cannot be memoized at all (size 0); a chain keyed on the importing
* FILE reads five.
*
* Paired with the non-vacuity assertions every guard in this family states: a
* perfect memo count is equally true of an adapter that resolves nothing.
*/
export function expectOneChainPerImporterDir(
arm: ChainMemoArm,
files: Set<string>,
perImporter: number,
): void {
const resolved: ChainMemoResult[] = [];
for (const fromFile of CHAIN_MEMO_IMPORTERS) {
resolved.push(...arm.drive(files, fromFile, perImporter));
}
expect(arm.memoOf(files).size).toBe(CHAIN_MEMO_IMPORTER_DIRS.length);
expect(sortedStrings(arm.memoOf(files).keys())).toEqual(sortedStrings(CHAIN_MEMO_IMPORTER_DIRS));
expect(resolved.filter((value) => value === arm.hitResult)).toHaveLength(
CHAIN_MEMO_IMPORTERS.length,
);
expect(resolved.filter((value) => value === null).length).toBeGreaterThan(0);
}
/**
* The stored-object arm: a memo that stores a FRESH chain on every import posts
* a perfect size while doing all of the work again, so the size gate above is
* paired with reference identity across many later imports from the same
* directory — issued from BOTH files in it, so a chain keyed on the importing
* file would be replaced rather than reused.
*
* Contents are asserted FIRST: `toBe` against an absent entry would pass on
* `undefined === undefined` if the memo were deleted outright.
*/
export function expectSameChainObjectReused(arm: ChainMemoArm, files: Set<string>): void {
const [firstImporter] = SHARED_DIR_IMPORTERS;
arm.drive(files, firstImporter, 1);
const first = arm.memoOf(files).get(SHARED_DIR);
expect(first).toEqual(arm.legacyChain(firstImporter));
for (const fromFile of SHARED_DIR_IMPORTERS) {
arm.drive(files, fromFile, CHAIN_IDENTITY_REPEATS);
}
expect(arm.memoOf(files).get(SHARED_DIR)).toBe(first);
}
/**
* The legacy-equality arm for one path shape: what the memo stored under
* `fromFile`'s directory is what the pre-change inline code built for it.
*
* Returns the memoized chain, so a guard whose memo feeds a SECOND consumer can
* go on to assert that consumer's derived form of it.
*/
export function expectMemoizedChainMatchesLegacy(
arm: ChainMemoArm,
files: Set<string>,
fromFile: string,
): readonly string[] {
arm.drive(files, fromFile, 1);
const chain = arm.memoOf(files).get(importerDirOf(fromFile));
expect(chain).toEqual(arm.legacyChain(fromFile));
return chain ?? [];
}
/**
* The distinct-file-set arm: two independently built file sets each get their
* own memo — equal in content, never the same object, neither leaking into the
* other. The two are driven interleaved, so a memo keyed on anything but the
* Set's identity shows up here as a SHARED entry rather than as a stale one.
*/
export function expectDistinctFileSetsGetOwnChainMemo(
arm: ChainMemoArm,
a: Set<string>,
b: Set<string>,
perImporter: number,
): void {
for (const fromFile of CHAIN_MEMO_IMPORTERS) {
arm.drive(a, fromFile, perImporter);
arm.drive(b, fromFile, perImporter);
}
const memoA = arm.memoOf(a);
const memoB = arm.memoOf(b);
expect(memoA).not.toBe(memoB);
expect(memoA.get(SHARED_DIR)).not.toBe(memoB.get(SHARED_DIR));
expect(memoA.get(SHARED_DIR)).toEqual(memoB.get(SHARED_DIR));
expect(memoA.size).toBe(CHAIN_MEMO_IMPORTER_DIRS.length);
expect(memoB.size).toBe(CHAIN_MEMO_IMPORTER_DIRS.length);
}

View file

@ -54,22 +54,41 @@ const { resolveImportTarget } = csharpScopeResolver;
const FROM_FILE = 'App/Program.cs';
/** Where a workspace's padded filler files go, and what is appended after them. */
interface WorkspaceLayout {
/** Directory the filler files live in. */
readonly dir: string;
/** Basename stem the filler files are numbered from. */
readonly stem: string;
/** The files the resolutions actually target, appended in order. */
readonly extras: readonly string[];
}
/**
* `fileCount` filler files under `layout.dir`, then `layout.extras`. The filler
* is what makes a traversal expensive enough for a per-`using` rebuild to be a
* different number rather than a different constant; the counting instrument
* reads the traversals either way.
*/
function buildWorkspace(fileCount: number, layout: WorkspaceLayout): CountingSet {
const files: string[] = [];
for (let i = 0; i < fileCount; i++) {
files.push(`${layout.dir}/${layout.stem}${String(i).padStart(5, '0')}.cs`);
}
return new CountingSet([...files, ...layout.extras]);
}
/**
* A synthetic C# solution with no `.csproj` discovered, which is the leg #2878
* moved onto the indexes. `App/Models/User.cs` answers the whole-path lookup,
* `App/Services/` answers the namespace-directory lookup, and `Domain/Order.cs`
* is reachable only after progressive prefix stripping.
*/
function buildWorkspace(fileCount: number): CountingSet {
const files: string[] = [];
for (let i = 0; i < fileCount; i++) {
files.push(`App/Services/Service${String(i).padStart(5, '0')}.cs`);
}
files.push('App/Models/User.cs');
files.push('Domain/Order.cs');
files.push('App/Program.cs');
return new CountingSet(files);
}
const NO_CSPROJ_LAYOUT: WorkspaceLayout = {
dir: 'App/Services',
stem: 'Service',
extras: ['App/Models/User.cs', 'Domain/Order.cs', 'App/Program.cs'],
};
/** The one `.csproj` config that puts the adapter on the csproj leg. */
const CSPROJ_CONFIG: CsharpResolutionConfig = {
@ -88,18 +107,15 @@ const CSPROJ_CONFIG: CsharpResolutionConfig = {
* A layout where the first two legs answer would leave the index unbuilt and
* the build count blind to the very copy it is here to catch.
*/
function buildCsprojWorkspace(fileCount: number): CountingSet {
const files: string[] = [];
for (let i = 0; i < fileCount; i++) {
files.push(`src/MyApp/Models/Entity${String(i).padStart(5, '0')}.cs`);
}
files.push('App/Program.cs');
return new CountingSet(files);
}
const CSPROJ_LAYOUT: WorkspaceLayout = {
dir: 'src/MyApp/Models',
stem: 'Entity',
extras: ['App/Program.cs'],
};
describe('C# import resolution — index reuse across usings (#2878)', () => {
it('builds each index once for many usings over a stable file set', () => {
const files = buildWorkspace(300);
const files = buildWorkspace(300, NO_CSPROJ_LAYOUT);
const resolved: (string | readonly string[] | null)[] = [];
for (let i = 0; i < 200; i++) {
@ -126,7 +142,7 @@ describe('C# import resolution — index reuse across usings (#2878)', () => {
it('a distinct file set gets its own indexes (no stale cross-run reuse)', () => {
expectDistinctFileSetsGetOwnIndex({
resolveImportTarget,
buildWorkspace: () => buildWorkspace(20),
buildWorkspace: () => buildWorkspace(20, NO_CSPROJ_LAYOUT),
targetRaw: 'App.Models.User',
fromFile: FROM_FILE,
resolutionConfig: undefined,
@ -138,7 +154,7 @@ describe('C# import resolution — index reuse across usings (#2878)', () => {
});
it('still resolves real usings correctly (the perf test is not vacuous)', () => {
const files = buildWorkspace(5);
const files = buildWorkspace(5, NO_CSPROJ_LAYOUT);
// Whole-path match on the namespace path.
expect(resolveImportTarget('App.Models.User', FROM_FILE, files, undefined)).toBe(
@ -161,7 +177,7 @@ describe('C# import resolution — index reuse across usings (#2878)', () => {
describe('C# import resolution — index reuse on the csproj leg (#2911)', () => {
it('builds each index once for many usings over a stable file set', () => {
const files = buildCsprojWorkspace(300);
const files = buildWorkspace(300, CSPROJ_LAYOUT);
const resolved: (string | readonly string[] | null)[] = [];
for (let i = 0; i < 200; i++) {
@ -188,7 +204,7 @@ describe('C# import resolution — index reuse on the csproj leg (#2911)', () =>
it('a distinct file set gets its own indexes (no stale cross-run reuse)', () => {
expectDistinctFileSetsGetOwnIndex({
resolveImportTarget,
buildWorkspace: () => buildCsprojWorkspace(20),
buildWorkspace: () => buildWorkspace(20, CSPROJ_LAYOUT),
targetRaw: 'App.Models',
fromFile: FROM_FILE,
resolutionConfig: CSPROJ_CONFIG,

View file

@ -1,7 +1,8 @@
/**
* Production-path regression guard for PR #1918 review finding P1.
*
* The Python file index (`getPythonFileIndex` in `import-target.ts`) is
* The Python file index (`getPythonFileIndex` in
* `import-resolvers/python-file-index.ts`) is
* memoized on the `allFilePaths` Set identity via a WeakMap. The registry-
* primary path reaches it through `pythonScopeResolver.resolveImportTarget`
* (the orchestrator adapter) — NOT by calling `resolvePythonImportTarget`

View file

@ -40,13 +40,22 @@
* pipeline's, via `ImportTargetWorkspace`'s shared `ResolveCtx`). They thread
* different objects around the same Set, and the memo is keyed on the Set.
*
* `legacyPrefixes` / `legacyImporterDir` are verbatim copies of the pre-change
* inline code, in the house style of `python-importer-ancestors.test.ts`: they
* are the specification, and the memo agreeing with them is what makes this a
* hoist rather than a behaviour change.
* `legacyPrefixes` is a verbatim copy of the pre-change inline code, in the
* house style of `python-importer-ancestors.test.ts`: it is the specification,
* and the memo agreeing with it is what makes this a hoist rather than a
* behaviour change.
*
* The four arms themselves live in `test/helpers/counting-file-set.ts`, beside
* the other import-target scaffolding: this guard and the #2913 one are the
* same suite over the same importer corpus once four values are named (the
* memo, the drive, the legacy builder, the hit), and they were previously
* written out twice. The two chains still DIFFER — this one keeps the empty
* components an absolute path or a doubled separator produces, and #2913's
* drops them — which is why `legacyChain` is per-guard and the shared path-shape
* table names shapes rather than expectations.
*/
import { describe, expect, it } from 'vitest';
import type { ParsedFile, ParsedImport } from 'gitnexus-shared';
import type { ParsedImport } from 'gitnexus-shared';
import type { ImportResolutionContext } from '../../../src/core/ingestion/scope-resolution/contract/scope-resolver.js';
import { pythonScopeResolver } from '../../../src/core/ingestion/languages/python/scope-resolver.js';
import { resolvePythonImportInternal } from '../../../src/core/ingestion/import-resolvers/python.js';
@ -61,10 +70,22 @@ import type {
ImportResult,
ResolveCtx,
} from '../../../src/core/ingestion/import-resolvers/types.js';
import {
IMPORTER_PATH_SHAPES,
NO_PARSED_FILES,
expectDistinctFileSetsGetOwnChainMemo,
expectMemoizedChainMatchesLegacy,
expectOneChainPerImporterDir,
expectSameChainObjectReused,
pythonNamedImport,
pythonNamespaceImport,
type ChainMemoArm,
type ChainMemoResult,
} from '../../helpers/counting-file-set.js';
const { resolveImportTarget } = pythonScopeResolver;
// ─── verbatim pre-change implementations ─────────────────────────────────────
// ─── verbatim pre-change implementation ──────────────────────────────────────
/** The prefix sequence the inline walk materialized on every import. */
function legacyPrefixes(currentFile: string): string[] {
@ -78,11 +99,6 @@ function legacyPrefixes(currentFile: string): string[] {
return prefixes;
}
/** The importer directory the inline code derived, and now the memo KEY. */
function legacyImporterDir(currentFile: string): string {
return currentFile.replace(/\\/g, '/').split('/').slice(0, -1).join('/');
}
// ─── surfaces ────────────────────────────────────────────────────────────────
/**
@ -109,35 +125,20 @@ function makeResolveCtx(allFilePaths: Set<string>): ResolveCtx {
};
}
const namespaceImport = (targetRaw: string): ParsedImport => ({
kind: 'namespace',
localName: '_',
importedName: '_',
targetRaw,
});
/** The kind that re-enters `resolvePythonImportTarget` and walks twice. */
const namedImport = (targetRaw: string): ParsedImport => ({
kind: 'named',
localName: 'Widget',
importedName: 'Widget',
targetRaw,
});
/**
* ONE array, not a fresh `[]` per call: `parsedFileByPath` memoizes on its
* identity, and a new array per import would mint a `WeakMap` key per import
* for a channel this file is not measuring. Empty, so `pythonFileExportsName`
* answers false and the package-vs-submodule precedence never fires — the
* walk, not the precedence, is under test here.
*/
const NO_PARSED_FILES: readonly ParsedFile[] = [];
const ctxFor = (parsedImport: ParsedImport): ImportResolutionContext => ({
parsedFiles: NO_PARSED_FILES,
parsedImport,
});
/**
* An `ImportResult` as the arms read it: the resolved path, or `null` for a
* miss. A stop-the-chain result carrying no files joins to `''`, which is
* neither the hit nor a miss — the same distinction the pre-collapse arm drew
* with `value?.kind === 'files' && value.files.join() === HIT_RESULT`.
*/
const resolvedPath = (result: ImportResult): ChainMemoResult =>
result === null ? null : result.files.join();
// ─── the workspace the surfaces are driven against ───────────────────────────
/**
@ -164,91 +165,79 @@ const WORKSPACE: readonly string[] = [
'root.py',
];
/** Every file that issues an import below. */
const IMPORTERS: readonly string[] = [
'svc/a/one.py',
'svc/a/two.py',
'svc/b/one.py',
'deep/x/y/z/one.py',
'root.py',
];
/** Four directories for five importers — `svc/a` holds two of them. */
const IMPORTER_DIRS: readonly string[] = ['svc/a', 'svc/b', 'deep/x/y/z', ''];
const HIT_TARGET = 'shared';
const HIT_RESULT = 'shared.py';
/** Survives the absence proof, then walks the whole chain and misses. */
/** Survives the absence proof, then walks the whole chain and misses — the
* dotted tier's suffix fallback picks it up as `elsewhere/deep/probe.py`. */
const WALK_TARGET = 'probe';
/** …and is then picked up by the dotted tier's suffix fallback. */
const WALK_RESULT = 'elsewhere/deep/probe.py';
/** Provably absent: retired before the walk, so it never touches the memo. */
const ABSENT_TARGET = 'ghostmod';
const sorted = (values: Iterable<string>): string[] => [...values].sort();
/**
* The spelling sequence BOTH surfaces are driven with, `perImporter` rounds of
* it plus the one spelling that must resolve: one target that walks the whole
* chain and misses, one that the absence proof retires before the walk, and one
* that hits at the workspace root. No spelling is varied per round — the Python
* chain keeps no per-target cache, so a repeated target really is re-resolved.
*
* `resolve` is the only thing that differs between the orchestrator adapter and
* the import-resolver pipeline; everything about the sequence is shared, which
* is why the two used to be the same loop written twice.
*/
function drive<T>(
fromFile: string,
perImporter: number,
resolve: (targetRaw: string, fromFile: string) => T,
): T[] {
const out: T[] = [];
for (let i = 0; i < perImporter; i++) {
for (const target of [WALK_TARGET, ABSENT_TARGET]) out.push(resolve(target, fromFile));
}
out.push(resolve(HIT_TARGET, fromFile));
return out;
}
/** The ORCHESTRATOR ADAPTER — the surface that pays the walk twice. */
const adapterArm = (mkImport: (targetRaw: string) => ParsedImport): ChainMemoArm => ({
memoOf: prefixMemo,
drive: (files, fromFile, perImporter) =>
drive(fromFile, perImporter, (target, from) =>
resolveImportTarget(target, from, files, undefined, ctxFor(mkImport(target))),
),
legacyChain: legacyPrefixes,
hitResult: HIT_RESULT,
});
/**
* Drives the ORCHESTRATOR ADAPTER, `perImporter` times per importer, with three
* spellings: one that walks the whole chain and misses, one that is retired by
* the absence proof before the walk, and one that hits at the workspace root.
* No spelling is varied per iteration — the Python chain keeps no per-target
* cache, so a repeated target really is re-resolved.
* The import-resolver pipeline surface. One `ResolveCtx` per drive rather than
* one for the run, on purpose: everything the strategy reads off it is derived
* from the same Set (and `pythonImportStrategy` only ever WRITES
* `resolveCache`), so a fresh ctx around the same Set is exactly the "different
* objects, same Set" case the memo has to survive.
*/
function driveAdapter(
files: ReadonlySet<string>,
perImporter: number,
mkImport: (targetRaw: string) => ParsedImport,
): (string | readonly string[] | null)[] {
const out: (string | readonly string[] | null)[] = [];
for (const fromFile of IMPORTERS) {
for (let i = 0; i < perImporter; i++) {
for (const target of [WALK_TARGET, ABSENT_TARGET]) {
out.push(resolveImportTarget(target, fromFile, files, undefined, ctxFor(mkImport(target))));
}
}
out.push(
resolveImportTarget(HIT_TARGET, fromFile, files, undefined, ctxFor(mkImport(HIT_TARGET))),
const strategyArm: ChainMemoArm = {
memoOf: prefixMemo,
drive: (files, fromFile, perImporter) => {
const ctx = makeResolveCtx(files);
return drive(fromFile, perImporter, (target, from) =>
resolvedPath(pythonImportStrategy(target, from, ctx)),
);
}
return out;
}
/** The same drive against the import-resolver pipeline surface. */
function driveStrategy(ctx: ResolveCtx, perImporter: number): ImportResult[] {
const out: ImportResult[] = [];
for (const fromFile of IMPORTERS) {
for (let i = 0; i < perImporter; i++) {
out.push(pythonImportStrategy(WALK_TARGET, fromFile, ctx));
out.push(pythonImportStrategy(ABSENT_TARGET, fromFile, ctx));
}
out.push(pythonImportStrategy(HIT_TARGET, fromFile, ctx));
}
return out;
}
},
legacyChain: legacyPrefixes,
hitResult: HIT_RESULT,
};
describe('Python bare-import prefix memo', () => {
it.each([
{ perImporter: 1, kind: 'namespace', mkImport: namespaceImport },
{ perImporter: 40, kind: 'namespace', mkImport: namespaceImport },
{ perImporter: 1, kind: 'named', mkImport: namedImport },
{ perImporter: 40, kind: 'named', mkImport: namedImport },
{ perImporter: 1, kind: 'namespace', mkImport: pythonNamespaceImport },
{ perImporter: 40, kind: 'namespace', mkImport: pythonNamespaceImport },
{ perImporter: 1, kind: 'named', mkImport: pythonNamedImport },
{ perImporter: 40, kind: 'named', mkImport: pythonNamedImport },
])(
'holds one chain per importer DIRECTORY, not per import — $perImporter x $kind',
({ perImporter, mkImport }) => {
const files = new Set(WORKSPACE);
const resolved = driveAdapter(files, perImporter, mkImport);
// The gate. Five importers over four directories, any number of imports:
// four entries. A chain rebuilt per import cannot be memoized at all
// (size 0); a chain keyed on the importing FILE reads five.
expect(prefixMemo(files).size).toBe(IMPORTER_DIRS.length);
expect(sorted(prefixMemo(files).keys())).toEqual(sorted(IMPORTER_DIRS));
// Non-vacuity: a perfect memo count is equally true of an adapter that
// resolves nothing.
expect(resolved.filter((value) => value === HIT_RESULT)).toHaveLength(IMPORTERS.length);
expect(resolved.filter((value) => value === null).length).toBeGreaterThan(0);
expectOneChainPerImporterDir(adapterArm(mkImport), new Set(WORKSPACE), perImporter);
},
);
@ -258,76 +247,24 @@ describe('Python bare-import prefix memo', () => {
])(
'holds one chain per importer DIRECTORY on the import-resolver surface too — $label',
({ perImporter }) => {
const files = new Set(WORKSPACE);
const ctx = makeResolveCtx(files);
const resolved = driveStrategy(ctx, perImporter);
expect(prefixMemo(files).size).toBe(IMPORTER_DIRS.length);
expect(sorted(prefixMemo(files).keys())).toEqual(sorted(IMPORTER_DIRS));
expect(
resolved.filter((value) => value?.kind === 'files' && value.files.join() === HIT_RESULT),
).toHaveLength(IMPORTERS.length);
expect(resolved.filter((value) => value === null).length).toBeGreaterThan(0);
expectOneChainPerImporterDir(strategyArm, new Set(WORKSPACE), perImporter);
},
);
it('reuses the SAME chain object, rather than rebuilding and re-storing it', () => {
const files = new Set(WORKSPACE);
resolveImportTarget(
WALK_TARGET,
'svc/a/one.py',
files,
undefined,
ctxFor(namedImport(WALK_TARGET)),
);
const first = prefixMemo(files).get('svc/a');
// Contents first: `toBe` against an absent entry would pass on
// `undefined === undefined` if the memo were deleted outright.
expect(first).toEqual(['svc/', '']);
for (let i = 1; i <= 40; i++) {
resolveImportTarget(
WALK_TARGET,
'svc/a/one.py',
files,
undefined,
ctxFor(namedImport(WALK_TARGET)),
);
resolveImportTarget(
HIT_TARGET,
'svc/a/two.py',
files,
undefined,
ctxFor(namespaceImport(HIT_TARGET)),
);
}
expect(prefixMemo(files).get('svc/a')).toBe(first);
expectSameChainObjectReused(adapterArm(pythonNamedImport), new Set(WORKSPACE));
});
it.each([
{ fromFile: 'svc/a/one.py', why: 'a two-component directory' },
{ fromFile: 'deep/x/y/z/one.py', why: 'a four-component directory' },
{ fromFile: 'root.py', why: 'a workspace-root importer (root-only chain)' },
{ fromFile: '/abs/svc/a/one.py', why: 'an absolute path (empty components KEPT)' },
{ fromFile: 'svc//a/one.py', why: 'a doubled separator (empty component KEPT)' },
{ fromFile: 'svc\\a\\one.py', why: 'Windows separators' },
{ fromFile: 'trailing/', why: 'a path ending in a separator' },
])('memoizes the chain the pre-change code built — $why', ({ fromFile }) => {
const files = new Set(WORKSPACE);
resolveImportTarget(
WALK_TARGET,
fromFile,
files,
undefined,
ctxFor(namespaceImport(WALK_TARGET)),
);
const chain = prefixMemo(files).get(legacyImporterDir(fromFile));
expect(chain).toEqual(legacyPrefixes(fromFile));
});
it.each(IMPORTER_PATH_SHAPES)(
'memoizes the chain the pre-change code built — $why',
({ fromFile }) => {
expectMemoizedChainMatchesLegacy(
adapterArm(pythonNamespaceImport),
new Set(WORKSPACE),
fromFile,
);
},
);
it.each([
{ perImporter: 2, label: 'two imports per importer' },
@ -335,20 +272,12 @@ describe('Python bare-import prefix memo', () => {
])(
'gives a distinct file set its own memo (no leak across passes) — $label',
({ perImporter }) => {
const a = new Set(WORKSPACE);
const b = new Set(WORKSPACE);
driveAdapter(a, perImporter, namedImport);
driveAdapter(b, perImporter, namedImport);
const memoA = prefixMemo(a);
const memoB = prefixMemo(b);
expect(memoA).not.toBe(memoB);
expect(memoA.get('svc/a')).not.toBe(memoB.get('svc/a'));
expect(memoA.get('svc/a')).toEqual(memoB.get('svc/a'));
expect(memoA.size).toBe(IMPORTER_DIRS.length);
expect(memoB.size).toBe(IMPORTER_DIRS.length);
expectDistinctFileSetsGetOwnChainMemo(
adapterArm(pythonNamedImport),
new Set(WORKSPACE),
new Set(WORKSPACE),
perImporter,
);
},
);
@ -370,7 +299,7 @@ describe('Python bare-import prefix memo', () => {
}
paths.push('shared.py');
const files = new Set(paths);
const resolved: (string | readonly string[] | null)[] = [];
const resolved: ChainMemoResult[] = [];
for (let d = 0; d < dirs; d++) {
for (let i = 0; i < importsPerDir; i++) {
@ -380,7 +309,7 @@ describe('Python bare-import prefix memo', () => {
`pkg${d}/nest/file0.py`,
files,
undefined,
ctxFor(namedImport(HIT_TARGET)),
ctxFor(pythonNamedImport(HIT_TARGET)),
),
);
}
@ -462,7 +391,7 @@ describe('Python bare-import ancestor walk — resolution', () => {
// strategy must agree: all three reach the same walk.
expect(resolvePythonImportInternal(fromFile, 'user', set)).toBe(expected);
expect(
resolveImportTarget('user', fromFile, set, undefined, ctxFor(namespaceImport('user'))),
resolveImportTarget('user', fromFile, set, undefined, ctxFor(pythonNamespaceImport('user'))),
).toBe(expected);
expect(pythonImportStrategy('user', fromFile, makeResolveCtx(set))).toEqual({
kind: 'files',

View file

@ -43,19 +43,19 @@
* adapter declares three or four parameters and cannot observe a fifth. Which
* ones those are is not a number to maintain here: the per-language
* `minimumParsedFileReads` floor in the table IS the record, and it is what a
* new reader has to change. What the `parsedFileReads` arm gates differs by
* language, and only the first of these is a memo:
* new reader has to change. Both languages that read this key memoize on it,
* and the two memos fail differently:
*
* - PHP: the `filesByDirectory` memo, `perFileSet`-keyed on the `parsedFiles`
* array. Defeat it and every import rebuilds a `Map<dirAlias,
* ParsedFile[]>`; the arm reads 603 against 9 (proven by mutation).
* - Python: there is NO memo on this key — `pythonFileExportsName` runs a
* `parsedFiles.find` for each import whose package probe resolves, and the
* misses here do not get that far, so one resolving import costs one scan.
* The arm pins exactly that: the read stays off the per-import path.
* Widening the fixture's `missTarget` to a spelling whose package DOES
* resolve puts it on that path and the arm reads 603 against 9, so it is a
* live gate rather than a `0 === 0`.
* - Python: `parsedFileByPath`, keyed the same way, behind
* `pythonFileExportsName`. It is built by the FIRST import whose package
* probe resolves — one pass over the array — and every later one is a
* `Map.get`, so the floor of 1 is that single build and the equality half
* is what proves it does not repeat. Before that memo the same call was a
* `parsedFiles.find` per resolving import, which is the shape #2901
* removed on the file-set key; this arm is why it cannot come back.
* - Every other language: `0 === 0`, recorded as a floor of 0. A new reader
* arrives with that floor already in place and is caught by the equality
* half, which needs no per-language knowledge at all.
@ -119,7 +119,11 @@ import type { ParsedImport } from 'gitnexus-shared';
import { SCOPE_RESOLVERS } from '../../../src/core/ingestion/scope-resolution/pipeline/registry.js';
import type { ScopeResolver } from '../../../src/core/ingestion/scope-resolution/contract/scope-resolver.js';
import type { ComposerConfig } from '../../../src/core/ingestion/language-config.js';
import { CountingSet, countedParsedFiles } from '../../helpers/counting-file-set.js';
import {
CountingSet,
countedParsedFiles,
pythonNamedImport,
} from '../../helpers/counting-file-set.js';
/**
* The minimum a language needs for one `resolveImportTarget` call to reach
@ -233,13 +237,6 @@ const PHP_FUNCTION_IMPORT = (targetRaw: string): ParsedImport => ({
* submodule fallback. The default the adapter synthesizes when `context` is
* absent is a `namespace` import, and that shape never reaches the probe.
*/
const PYTHON_NAMED_IMPORT = (targetRaw: string): ParsedImport => ({
kind: 'named',
localName: 'Widget',
importedName: 'Widget',
targetRaw,
});
const FIXTURES: ReadonlyMap<SupportedLanguages, ImportTargetFixture> = new Map<
SupportedLanguages,
ImportTargetFixture
@ -255,10 +252,11 @@ const FIXTURES: ReadonlyMap<SupportedLanguages, ImportTargetFixture> = new Map<
resolutionConfig: undefined,
missTarget: (i) => `realpkg.ghost${i}`,
hitTarget: 'realpkg.widget',
parsedImport: PYTHON_NAMED_IMPORT,
parsedImport: pythonNamedImport,
minimumScans: 1,
// `pythonFileExportsName`'s `parsedFiles.find` on the one import whose
// package probe resolves — the misses never get that far.
// The one build of `parsedFileByPath`, triggered by the single import
// whose package probe resolves — the misses never get that far, and
// every later resolver is a `Map.get` rather than another pass.
minimumParsedFileReads: 1,
},
],

View file

@ -44,9 +44,14 @@
* simply stopped working would post a perfect flat line.
*/
import { describe, expect, it } from 'vitest';
import type { ParsedFile, ParsedImport } from 'gitnexus-shared';
import type { ParsedImport } from 'gitnexus-shared';
import { pythonScopeResolver } from '../../../../src/core/ingestion/languages/python/scope-resolver.js';
import { resolvePythonImportInternal } from '../../../../src/core/ingestion/import-resolvers/python.js';
import {
NO_PARSED_FILES,
pythonNamedImport,
pythonNamespaceImport,
} from '../../../helpers/counting-file-set.js';
const { resolveImportTarget } = pythonScopeResolver;
@ -61,23 +66,9 @@ class ProbeCountingSet extends Set<string> {
}
}
/** One array for the whole file: `parsedFileByPath` memoizes on its identity,
* and it is empty, so the package-export precedence never fires and the walk
* is what the numbers measure. */
const NO_PARSED_FILES: readonly ParsedFile[] = [];
const namespaceImport = (targetRaw: string): ParsedImport => ({
kind: 'namespace',
localName: '_',
importedName: '_',
targetRaw,
});
const namedImport = (targetRaw: string): ParsedImport => ({
kind: 'named',
localName: 'Widget',
importedName: 'Widget',
targetRaw,
});
/** `import x as y`. The third kind, and the only one of the three that is not
* shared with the memo guards — it reaches the same package-attribute probe
* `pythonNamedImport` does, and both arms below assert they cost the same. */
const aliasImport = (targetRaw: string): ParsedImport => ({
kind: 'alias',
localName: 'w',
@ -162,8 +153,8 @@ const DOTTED_SUBMODULE_PROBES: readonly number[] = [5, 6, 8, 12, 20];
describe('Python import probe count', () => {
it.each([
{ kind: 'import x (namespace)', mkImport: namespaceImport },
{ kind: 'from x import y (named)', mkImport: namedImport },
{ kind: 'import x (namespace)', mkImport: pythonNamespaceImport },
{ kind: 'from x import y (named)', mkImport: pythonNamedImport },
{ kind: 'import x as y (alias)', mkImport: aliasImport },
])('costs the same for every import KIND — single-segment, resolving — $kind', ({ mkImport }) => {
const counted = DEPTHS.map((depth) => probeCount(mkImport, depth, PRESENT_TARGET));
@ -175,8 +166,16 @@ describe('Python import probe count', () => {
});
it.each([
{ kind: 'import x (namespace)', mkImport: namespaceImport, expected: DOTTED_NAMESPACE_PROBES },
{ kind: 'from x import y (named)', mkImport: namedImport, expected: DOTTED_SUBMODULE_PROBES },
{
kind: 'import x (namespace)',
mkImport: pythonNamespaceImport,
expected: DOTTED_NAMESPACE_PROBES,
},
{
kind: 'from x import y (named)',
mkImport: pythonNamedImport,
expected: DOTTED_SUBMODULE_PROBES,
},
{ kind: 'import x as y (alias)', mkImport: aliasImport, expected: DOTTED_SUBMODULE_PROBES },
])(
'runs the package tail ONCE for a dotted target that misses — $kind',
@ -191,8 +190,8 @@ describe('Python import probe count', () => {
);
it.each([
{ kind: 'import x (namespace)', mkImport: namespaceImport },
{ kind: 'from x import y (named)', mkImport: namedImport },
{ kind: 'import x (namespace)', mkImport: pythonNamespaceImport },
{ kind: 'from x import y (named)', mkImport: pythonNamedImport },
{ kind: 'import x as y (alias)', mkImport: aliasImport },
])('retires a provably absent segment in a CONSTANT probe count — $kind', ({ mkImport }) => {
const counted = DEPTHS.map((depth) => probeCount(mkImport, depth, ABSENT_TARGET));
@ -207,7 +206,9 @@ describe('Python import probe count', () => {
// Control for the arm above: the same instrument, the same corpus, the same
// depths, one different spelling — and the count triples across the range.
// So a flat line means the walk was skipped, not that nothing is counted.
const present = DEPTHS.map((depth) => probeCount(namedImport, depth, PRESENT_TARGET).probes);
const present = DEPTHS.map(
(depth) => probeCount(pythonNamedImport, depth, PRESENT_TARGET).probes,
);
expect(present).toEqual(PRESENT_PROBES);
expect(new Set(present).size).toBe(DEPTHS.length);
});

View file

@ -40,12 +40,31 @@
* in the house style of `import-target-index-parity.test.ts`: they are the
* specification, and the memo agreeing with them is what makes this a hoist
* rather than a behaviour change.
*
* The four memo arms live in `counting-file-set.ts` beside the other
* import-target scaffolding, because this guard and the bare-prefix one
* (`test/unit/import-resolvers/python-importer-prefixes.test.ts`) are the same
* suite over the same importer corpus once four values are named (the memo, the
* drive, the legacy builder, the hit). The two CHAINS still differ — this one
* drops the empty components an absolute path or a doubled separator produces
* and the other keeps them — so `legacyChain` stays per-guard and the shared
* path-shape table names shapes rather than expectations.
*/
import { describe, expect, it } from 'vitest';
import type { ParsedImport } from 'gitnexus-shared';
import { pythonScopeResolver } from '../../../../src/core/ingestion/languages/python/scope-resolver.js';
import { getPythonFileIndex } from '../../../../src/core/ingestion/import-resolvers/python-file-index.js';
import { countedParsedFiles } from '../../../helpers/counting-file-set.js';
import {
IMPORTER_PATH_SHAPES,
countedParsedFiles,
expectDistinctFileSetsGetOwnChainMemo,
expectMemoizedChainMatchesLegacy,
expectOneChainPerImporterDir,
expectSameChainObjectReused,
sortedStrings,
type ChainMemoArm,
type ChainMemoResult,
} from '../../../helpers/counting-file-set.js';
const { resolveImportTarget } = pythonScopeResolver;
@ -104,8 +123,6 @@ function specNestedDirNames(dirPrefixes: ReadonlySet<string>): Set<string> {
return names;
}
const sorted = (values: Iterable<string>): string[] => [...values].sort();
// ─── the workspace the adapter is driven against ─────────────────────────────
/**
@ -124,18 +141,6 @@ const WORKSPACE: readonly string[] = [
'root.py',
];
/** Every file that issues an import below, and the directory it sits in. */
const IMPORTERS: readonly string[] = [
'svc/a/one.py',
'svc/a/two.py',
'svc/b/one.py',
'deep/x/y/z/one.py',
'root.py',
];
/** Four directories for five importers — `svc/a` holds two of them. */
const IMPORTER_DIRS: readonly string[] = ['svc/a', 'svc/b', 'deep/x/y/z', ''];
/**
* One import that must resolve, so a memo count is never the count of an
* adapter that has stopped resolving anything (the pairing rule every guard in
@ -146,7 +151,7 @@ const HIT_TARGET = 'outer.nested.mod';
const HIT_RESULT = 'outer/nested/mod.py';
/**
* Drives the ORCHESTRATOR ADAPTER, `perImporter` times per importer, with two
* Drives the ORCHESTRATOR ADAPTER `perImporter` times from `fromFile`, with two
* spellings that between them enter the memo from both call sites:
* - `nested.ghost{i}` — reaches `hasRepoCandidate`'s ancestor walk and misses.
* Spelled differently every iteration, so nothing upstream can answer it
@ -157,84 +162,54 @@ const HIT_RESULT = 'outer/nested/mod.py';
* `one.py` has to be a real basename somewhere or the walk is skipped
* before it starts, and the Python chain keeps no per-target cache, so a
* repeated spelling really is re-resolved.
* …then the one spelling that must resolve.
*/
function driveAdapter(
files: ReadonlySet<string>,
function driveImporter(
files: Set<string>,
fromFile: string,
perImporter: number,
): (string | readonly string[] | null)[] {
const out: (string | readonly string[] | null)[] = [];
for (const fromFile of IMPORTERS) {
for (let i = 0; i < perImporter; i++) {
out.push(resolveImportTarget(`nested.ghost${i}`, fromFile, files, undefined, undefined));
out.push(resolveImportTarget('outer.one', fromFile, files, undefined, undefined));
}
out.push(resolveImportTarget(HIT_TARGET, fromFile, files, undefined, undefined));
): ChainMemoResult[] {
const out: ChainMemoResult[] = [];
for (let i = 0; i < perImporter; i++) {
out.push(resolveImportTarget(`nested.ghost${i}`, fromFile, files, undefined, undefined));
out.push(resolveImportTarget('outer.one', fromFile, files, undefined, undefined));
}
out.push(resolveImportTarget(HIT_TARGET, fromFile, files, undefined, undefined));
return out;
}
const ancestorArm: ChainMemoArm = {
memoOf: (files) => getPythonFileIndex(files).ancestorsByDir,
drive: driveImporter,
legacyChain: legacyAncestorChain,
hitResult: HIT_RESULT,
};
describe('Python importer-ancestor memo (#2913)', () => {
it.each([
{ perImporter: 1, label: 'one import per importer' },
{ perImporter: 40, label: 'forty imports per importer' },
])('holds one chain per importer DIRECTORY, not per import — $label', ({ perImporter }) => {
const files = new Set(WORKSPACE);
const resolved = driveAdapter(files, perImporter);
// The gate. Five importers over four directories, any number of imports:
// four entries. A chain rebuilt per import cannot be memoized at all
// (size 0); a chain keyed on the importing FILE reads five.
expect(getPythonFileIndex(files).ancestorsByDir.size).toBe(IMPORTER_DIRS.length);
expect(sorted(getPythonFileIndex(files).ancestorsByDir.keys())).toEqual(sorted(IMPORTER_DIRS));
// Non-vacuity: a perfect memo count is equally true of an adapter that
// resolves nothing.
expect(resolved.filter((value) => value === HIT_RESULT)).toHaveLength(IMPORTERS.length);
expect(resolved.filter((value) => value === null).length).toBeGreaterThan(0);
expectOneChainPerImporterDir(ancestorArm, new Set(WORKSPACE), perImporter);
});
it('reuses the SAME chain object, rather than rebuilding and re-storing it', () => {
const files = new Set(WORKSPACE);
resolveImportTarget('nested.ghost0', 'svc/a/one.py', files, undefined, undefined);
const first = getPythonFileIndex(files).ancestorsByDir.get('svc/a');
// Contents first: `toBe` against an absent entry would pass on
// `undefined === undefined` if the memo were deleted outright.
expect(first).toEqual(['svc/a', 'svc']);
for (let i = 1; i <= 40; i++) {
resolveImportTarget(`nested.ghost${i}`, 'svc/a/one.py', files, undefined, undefined);
resolveImportTarget('outer.one', 'svc/a/two.py', files, undefined, undefined);
}
expect(getPythonFileIndex(files).ancestorsByDir.get('svc/a')).toBe(first);
expectSameChainObjectReused(ancestorArm, new Set(WORKSPACE));
});
it.each([
{ fromFile: 'svc/a/one.py', why: 'a two-component directory' },
{ fromFile: 'deep/x/y/z/one.py', why: 'a four-component directory' },
{ fromFile: 'root.py', why: 'a workspace-root importer (no ancestors)' },
{ fromFile: '/abs/svc/a/one.py', why: 'an absolute path (leading empty part dropped)' },
{ fromFile: 'svc//a/one.py', why: 'a doubled separator (empty part dropped)' },
{ fromFile: 'svc\\a\\one.py', why: 'Windows separators' },
{ fromFile: 'trailing/', why: 'a path ending in a separator' },
])('memoizes the chain the pre-#2913 code built — $why', ({ fromFile }) => {
const files = new Set(WORKSPACE);
resolveImportTarget('nested.ghost', fromFile, files, undefined, undefined);
it.each(IMPORTER_PATH_SHAPES)(
'memoizes the chain the pre-#2913 code built — $why',
({ fromFile }) => {
const chain = expectMemoizedChainMatchesLegacy(ancestorArm, new Set(WORKSPACE), fromFile);
const norm = fromFile.replace(/\\/g, '/');
const lastSlash = norm.lastIndexOf('/');
const importerDir = lastSlash === -1 ? '' : norm.slice(0, lastSlash);
const chain = getPythonFileIndex(files).ancestorsByDir.get(importerDir);
// Both consumers' chains, from the one memo: `resolveAbsoluteFromFiles`
// walked these directories, `hasRepoCandidate` walked the same directories
// with `/<segment>/` appended.
expect(chain).toEqual(legacyAncestorChain(fromFile));
expect((chain ?? []).map((ancestor) => `${ancestor}/nested/`)).toEqual(
legacyAncestorPrefixes(fromFile, 'nested'),
);
});
// Both consumers' chains, from the one memo: `resolveAbsoluteFromFiles`
// walked these directories, `hasRepoCandidate` walked the same directories
// with `/<segment>/` appended.
expect(chain.map((ancestor) => `${ancestor}/nested/`)).toEqual(
legacyAncestorPrefixes(fromFile, 'nested'),
);
},
);
it.each([
{ why: 'relative paths sharing directories', files: WORKSPACE },
@ -251,8 +226,8 @@ describe('Python importer-ancestor memo (#2913)', () => {
// The build now walks separators from the deepest outward and stops at
// the first prefix already present. Skipping the rest is only sound
// because a prefix is always stored with all of its own ancestors.
expect(sorted(index.dirPrefixes)).toEqual(sorted(legacy));
expect(sorted(index.nestedDirNames)).toEqual(sorted(specNestedDirNames(legacy)));
expect(sortedStrings(index.dirPrefixes)).toEqual(sortedStrings(legacy));
expect(sortedStrings(index.nestedDirNames)).toEqual(sortedStrings(specNestedDirNames(legacy)));
});
/**
@ -275,7 +250,7 @@ describe('Python importer-ancestor memo (#2913)', () => {
const paths = ['pkg/__init__.py', ...modules, 'app/main.py'];
const workspace = countedParsedFiles(paths);
const files = new Set(paths);
const resolved: (string | readonly string[] | null)[] = [];
const resolved: ChainMemoResult[] = [];
for (let i = 0; i < imports; i++) {
const targetRaw = `pkg.m${i % modules.length}`;
@ -307,17 +282,10 @@ describe('Python importer-ancestor memo (#2913)', () => {
const a = new Set(WORKSPACE);
const b = new Set(WORKSPACE);
driveAdapter(a, 2);
driveAdapter(b, 2);
expectDistinctFileSetsGetOwnChainMemo(ancestorArm, a, b, 2);
const indexA = getPythonFileIndex(a);
const indexB = getPythonFileIndex(b);
expect(indexA).not.toBe(indexB);
expect(indexA.ancestorsByDir).not.toBe(indexB.ancestorsByDir);
expect(indexA.ancestorsByDir.get('svc/a')).not.toBe(indexB.ancestorsByDir.get('svc/a'));
expect(indexA.ancestorsByDir.get('svc/a')).toEqual(indexB.ancestorsByDir.get('svc/a'));
expect(indexA.ancestorsByDir.size).toBe(IMPORTER_DIRS.length);
expect(indexB.ancestorsByDir.size).toBe(IMPORTER_DIRS.length);
// The whole per-file-set index, not only the memo inside it: the WeakMap is
// keyed on the Set, so two Sets can never share one index.
expect(getPythonFileIndex(a)).not.toBe(getPythonFileIndex(b));
});
});