From 376de5c3ce67a1f45cf3beabfae55be803ff7b7e Mon Sep 17 00:00:00 2001 From: ReidenXerx Date: Fri, 7 Aug 2026 19:31:02 +0300 Subject: [PATCH] fix(scope-resolution): bind a producer's own returned key to itself, and stop claiming uniqueness for a ranked answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding 3, accepting the two defects it demonstrates and declining the remedy it proposes. Both halves are mutation-verified. 1. A SITE INSIDE ITS OWN RETURN SHAPE NOW 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 correct for a READ through a receiver, but applied to this site it handed the write to a same-named module const that `buildB` never touches — a wrong edge — while the node the key actually defines was left with no writer at all. Both halves wrong from one rule applied to the wrong shape. 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 outranks that. 2. THE TIER NO LONGER LIES. `workspace-unique` is a claim that exactly one node in the workspace carries the 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 it was reported under the same label. The edge is unchanged; what it is allowed to say about itself is not. `narrowed` now counts these correctly too, since it keys off the tier. WHAT I AM NOT DOING, and why. The review proposes dropping the same-file and imported-file tiers "and keeping only genuine workspace-uniqueness". That would revert the measured R2 result taking backend readers of `exitMinAtrMult` from 0 to 24. Workspace uniqueness was already measured too strict on that repo: the field carries 26 Property definitions — 16 in one-off scripts, 7 in the frontend, one in a test, and exactly one in the backend that reads it. Strict uniqueness declines all 24. The alternative suggestion — require the receiver to bind to the owning object — has the same effect by another route: the population this pass exists for is the untyped option bag, whose receiver binds to nothing. Requiring a binding turns the pass off for its own use case. So the two demonstrated defects are fixed and the capability around them is kept, at half confidence, naming its inference in the reason string, and honoured only where `fieldFallbackOnMethodLookup` allows. The R3-5 precision test needed rescoping rather than relaxing: it asserted that EVERY edge to the contested field is a precise return-shape edge, which the producer's own (correct, name-tier) write now violates. It asserts the reader edges are precise and the producer's write binds to its own key — two different claims reached two different ways, which is what the code now models. Co-Authored-By: Claude Opus 5 (1M context) --- .../passes/unique-name-properties.ts | 52 +++++++++++++++- .../javascript-object-properties.test.ts | 59 ++++++++++++++++--- 2 files changed, 100 insertions(+), 11 deletions(-) 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