diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/unique-name-properties.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/unique-name-properties.ts index f8957aa95..cce98f126 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/unique-name-properties.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/unique-name-properties.ts @@ -299,6 +299,21 @@ function buildDirectImportMap( return byFile; } +/** Is this Property id qualified by `.`, allowing a position suffix? */ +function idOwnedBy(id: string, owner: string): boolean { + const at = id.indexOf(`:${owner}.`); + if (at === -1) return false; + const rest = id.slice(at + owner.length + 2); + return !rest.includes('.') || rest.indexOf('@') < rest.indexOf('.'); +} + +/** Trailing symbol name of a graph id (`Function:a/b.js:buildB` -> `buildB`). */ +function simpleNameOfGraphId(graphId: string): string { + const tail = graphId.slice(graphId.lastIndexOf(':') + 1); + const at = tail.indexOf('@'); + return at === -1 ? tail : tail.slice(0, at); +} + /** * Pick the single candidate a read in `readingFile` can plausibly mean. * @@ -312,6 +327,8 @@ function narrowToSingleCandidate( candidatesIn: readonly PropertyCandidate[], readingFile: string, importedFiles: ReadonlySet | undefined, + /** Simple name of the callable the site sits in, when it could be resolved. */ + ownerName?: string, ): { readonly id: string; readonly tier: string } | null { // DECLARED ANCHORS FIRST. A return shape is a real definition but a weaker // one: it says "some function builds an object with this key", where a named @@ -321,6 +338,23 @@ function narrowToSingleCandidate( // return shapes were indexed at all. let candidates = candidatesIn; + // A SITE INSIDE ITS OWN RETURN SHAPE BINDS TO ITS OWN KEY. + // + // `export function buildB(row) { return { tickIntervalMs: row.b } }` writes + // the key that IS `buildB.tickIntervalMs`. Ranking declared anchors above + // return shapes is right for a READ through a receiver, but applied here it + // handed that write to a same-named module const (`fnSettings.tickIntervalMs`) + // that `buildB` never touches — a wrong edge, reported at the confident tier, + // and the node the key actually defines was left with no writer at all. + // + // Checked before every other rule because it is evidence rather than ranking: + // the owner qualifier on the candidate id and the enclosing callable are the + // same symbol. Nothing else can outrank that. + if (ownerName !== undefined && ownerName.length > 0) { + const own = candidates.filter((c) => c.fromReturnShape && idOwnedBy(c.id, ownerName)); + if (own.length === 1) return { id: own[0]!.id, tier: 'own-return-shape' }; + } + // PRODUCTION CODE FIRST. A test constructs throwaway shapes with the same // field names as the thing it exercises — measured on the reporting repo, // four of the seven JavaScript anchors for `wickRatio` are in `tests/` — and @@ -339,7 +373,15 @@ function narrowToSingleCandidate( const ranked = declared.length > 0 ? declared : candidates; if (ranked.length === 1) { - return { id: ranked[0]!.id, tier: 'workspace-unique' }; + // TIER HONESTY. `workspace-unique` is a claim that exactly one node in the + // workspace carries this name — a fact about the graph. Reaching one + // survivor by FILTERING (tests out, return shapes down-ranked) is a + // different and weaker claim, and reporting it under the same label told a + // reader "unambiguous workspace-wide match" for an answer that was + // narrowed. The edge is the same; what it is allowed to say about itself is + // not. `narrowed` counts it correctly now too, since that keys off the tier. + const tier = candidatesIn.length === 1 ? 'workspace-unique' : 'ranked'; + return { id: ranked[0]!.id, tier }; } candidates = ranked; @@ -447,10 +489,16 @@ export function emitUniqueNamePropertyAccesses( continue; } + // Resolved BEFORE narrowing: the enclosing callable is evidence the + // ranking needs, not just the edge's source. + const callerGraphId = resolveCallerGraphId(site.inScope, indexes, nodeLookup, site.atRange); + if (callerGraphId === undefined) continue; + const choice = narrowToSingleCandidate( candidates, parsed.filePath, directImports.get(parsed.filePath), + simpleNameOfGraphId(callerGraphId), ); if (choice === null) { ambiguous++; @@ -460,8 +508,6 @@ export function emitUniqueNamePropertyAccesses( const targetId = choice.id; if (choice.tier !== 'workspace-unique') narrowed++; - const callerGraphId = resolveCallerGraphId(site.inScope, indexes, nodeLookup, site.atRange); - if (callerGraphId === undefined) continue; // A property reading itself is not a fact about anything. if (callerGraphId === targetId) continue; diff --git a/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts b/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts index 080212e93..159c0d694 100644 --- a/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts +++ b/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts @@ -259,18 +259,61 @@ describe('JavaScript plain-object property access (A1/A5)', () => { expect(targetOf('readsBeta').some((id) => id.includes('producerAlpha.'))).toBe(false); }); - it('marks the edge as precise, not as name inference', () => { - const reasons = getRelationships(result, 'ACCESSES') - .filter((e) => e.target === 'ambiguousProducedField') - .map((e) => String(e.rel.reason ?? '')); - expect(reasons.every((r) => r.includes('return-shape member'))).toBe(true); - for (const e of getRelationships(result, 'ACCESSES').filter( - (x) => x.target === 'ambiguousProducedField', - )) { + // Scoped to the READERS. The producer itself also touches this field — it + // constructs the key — and that edge is a different claim reached a + // different way (see the own-return-shape case below), so folding both into + // one "every edge must be precise" assertion would either forbid a correct + // write or force it to lie about its tier. + it('marks the reader edges as precise, not as name inference', () => { + const readerEdges = getRelationships(result, 'ACCESSES').filter( + (e) => + e.target === 'ambiguousProducedField' && + (e.source === 'readsAlpha' || e.source === 'readsBeta'), + ); + expect(readerEdges.length).toBeGreaterThan(0); + for (const e of readerEdges) { + expect(String(e.rel.reason ?? '')).toContain('return-shape member'); expect(e.rel.confidence).toBeGreaterThan(0.85); } }); + // Review finding 3, the telemetry half. `workspace-unique` is a claim that + // exactly one node in the workspace carries this name — a fact about the + // graph, and the label a reader trusts most. An answer reached by FILTERING + // (tests down-ranked, return shapes down-ranked) is a weaker claim, and + // reporting it under the same label said "unambiguous workspace-wide match" + // for something that was narrowed. The edge is unchanged; what it is allowed + // to say about itself is not. + it('does not claim workspace-uniqueness for an answer reached by ranking', () => { + // `sharedWithDeclared` is carried by BOTH a declared object key and a + // return-shape key, so the workspace is not unique in it. The read + // resolves only because declared anchors outrank return shapes — a + // ranking decision — and the edge must say so. + const ranked = getRelationships(result, 'ACCESSES').filter( + (e) => e.target === 'sharedWithDeclared' && e.source === 'readsShared', + ); + expect(ranked.length).toBeGreaterThan(0); + for (const e of ranked) { + expect(String(e.rel.reason ?? '')).not.toContain('(workspace-unique)'); + } + }); + + // Review finding 3, the half that was a real wrong edge. A function that + // returns `{ field: … }` WRITES the key that is its own return shape. The + // declared-outranks-return-shape ranking is right for a read through a + // receiver, but applied to this site it handed the write to a same-named + // declared const the producer never touches — and left the node the key + // actually defines with no writer at all. + it('binds a producer writing its own returned key to that key', () => { + const own = getRelationships(result, 'ACCESSES').filter( + (e) => e.source === 'producerAlpha' && e.target === 'ambiguousProducedField', + ); + expect(own.length).toBeGreaterThan(0); + for (const e of own) expect(e.targetFilePath ?? '').toBeTruthy(); + // Never the OTHER producer's key: the owner qualifier is the evidence. + expect(targetOf('producerAlpha').some((id) => id.includes('producerBeta.'))).toBe(false); + }); + // THE BOUND, asserted so the mechanism is not mistaken for something it is // not. A bare parameter has no binding here — typing it needs the CALLER's // type to flow in, which is inter-procedural — so it falls through to name