mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-10 03:27:59 +00:00
fix(scope-resolution): resolve the import map by point lookup so the seal cannot empty it
Review finding 4, reproduced end-to-end by two lanes: the same commit and the same repo produced a DIFFERENT graph depending on `GITNEXUS_DISK_SCOPE_INDEX`. `buildDirectImportMap` built `scopeToFile` by walking `parsed.scopes`. The out-of-core seal replaces `emitParsedFiles` with a scope-STRIPPED copy — that is its documented contract, scopes are reachable only via `scopeTree.getScope` afterwards — so under the seal the map came out empty, every `directImports` lookup returned undefined, and tier-2 narrowing died repo-wide. The reporting is the worse half. The loss surfaced as `ambiguous`, which means "several candidates and the pass refused to choose". The truth was "the evidence was discarded one function earlier". A reader acting on that would go looking for better receiver typing to fix a problem that was not there. This is the SECOND consumer of `parsed.scopes` on this branch to hit the seal. The first was hoisted above it. This one is converted to the point lookup instead, which is the stronger fix: a point lookup survives the seal by contract, so there is no ordering left for a future edit to get wrong. The parity assertion that would have caught it now exists. The sealed harness in `javascript-const-references` already ran the fixture both ways, but every assertion in it pinned ONE field's readers — which is exactly how a second instance slipped in, since no assertion happened to cover a narrowed name. It now also compares the WHOLE ACCESSES edge set between the two runs, as a sorted diff so a failure names the edges that moved, with a non-empty guard so two empty sets cannot compare equal and assert nothing. Mutation-verified: forcing the map empty fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
690470865e
commit
a76c4c70fe
2 changed files with 43 additions and 10 deletions
|
|
@ -263,19 +263,28 @@ function candidatesForLanguage(
|
|||
* narrow every read to nothing.
|
||||
*/
|
||||
function buildDirectImportMap(
|
||||
parsedFiles: readonly ParsedFile[],
|
||||
indexes: ScopeResolutionIndexes,
|
||||
finalized: FinalizedImportView,
|
||||
): ReadonlyMap<string, ReadonlySet<string>> {
|
||||
const scopeToFile = new Map<string, string>();
|
||||
for (const parsed of parsedFiles) {
|
||||
for (const scope of parsed.scopes) {
|
||||
scopeToFile.set(scope.id, parsed.filePath);
|
||||
}
|
||||
}
|
||||
|
||||
// Resolve each importing scope to its file by POINT LOOKUP on the scope tree,
|
||||
// not by walking `parsed.scopes`.
|
||||
//
|
||||
// The out-of-core seal replaces `emitParsedFiles` with a scope-STRIPPED copy —
|
||||
// `scopes: []` for every file — and that is the documented contract: after the
|
||||
// seal, scopes are reachable only via `scopeTree.getScope`. Building the map
|
||||
// from `parsed.scopes` therefore produced an EMPTY map under
|
||||
// `GITNEXUS_DISK_SCOPE_INDEX=1`, every `directImports` lookup returned
|
||||
// undefined, tier-2 narrowing died, and — worst of it — the loss was
|
||||
// misreported as `ambiguous`: "several candidates, refused to choose" when the
|
||||
// truth was "the evidence was discarded one function earlier". Same commit,
|
||||
// same repo, two different graphs depending on an env var.
|
||||
//
|
||||
// This is the SECOND consumer of `parsed.scopes` on this branch to hit the
|
||||
// seal. The first was hoisted above it; this one is converted to the point
|
||||
// lookup instead, which is stronger — there is no ordering left to get wrong.
|
||||
const byFile = new Map<string, Set<string>>();
|
||||
for (const [scopeId, edges] of finalized.imports) {
|
||||
const fromFile = scopeToFile.get(scopeId);
|
||||
const fromFile = indexes.scopeTree.getScope(scopeId)?.filePath;
|
||||
if (fromFile === undefined) continue;
|
||||
for (const edge of edges) {
|
||||
if (edge.targetFile === null || edge.targetFile === fromFile) continue;
|
||||
|
|
@ -392,7 +401,7 @@ export function emitUniqueNamePropertyAccesses(
|
|||
const directImports =
|
||||
finalized === undefined
|
||||
? new Map<string, ReadonlySet<string>>()
|
||||
: buildDirectImportMap(parsedFiles, finalized);
|
||||
: buildDirectImportMap(indexes, finalized);
|
||||
|
||||
let emitted = 0;
|
||||
let ambiguous = 0;
|
||||
|
|
|
|||
|
|
@ -91,6 +91,30 @@ describe('JavaScript module-scope const references (A2)', () => {
|
|||
expect([...sealedReaders()].sort()).toEqual([...readersOfConst()].sort());
|
||||
});
|
||||
|
||||
// WHOLESALE parity, not one field's readers.
|
||||
//
|
||||
// The two assertions around this one each pin a single name, and that is how
|
||||
// a second instance of the same defect got in: a different consumer of
|
||||
// `parsed.scopes` (`buildDirectImportMap`) was also reading scope-stripped
|
||||
// files under the seal, tier-2 import narrowing died repo-wide, and every
|
||||
// targeted assertion here still passed because none of them covered a
|
||||
// narrowed name. Comparing the whole ACCESSES set is the only shape that
|
||||
// notices a loss nobody thought to name.
|
||||
//
|
||||
// Reported as a sorted diff rather than a bare count so a failure says WHICH
|
||||
// edges moved.
|
||||
it('produces an identical ACCESSES edge set to the unsealed run', () => {
|
||||
const edgeSet = (r: PipelineResult): string[] =>
|
||||
getRelationships(r, 'ACCESSES')
|
||||
.map((e) => `${e.source} -> ${e.target} (${e.rel.reason})`)
|
||||
.sort();
|
||||
const unsealed = edgeSet(result);
|
||||
// Guard the guard: an empty set on both sides would compare equal and
|
||||
// assert nothing.
|
||||
expect(unsealed.length).toBeGreaterThan(0);
|
||||
expect(edgeSet(sealed)).toEqual(unsealed);
|
||||
});
|
||||
|
||||
it('still withholds the block-local edge', () => {
|
||||
// The filter must fail OPEN when scopes are unavailable, not be disabled:
|
||||
// the block-local exclusion is a correctness property, not an optimization.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue