From 0880bbb97d9377ba59517bff2f13fca0cbdcb151 Mon Sep 17 00:00:00 2001 From: ReidenXerx Date: Fri, 7 Aug 2026 04:31:56 +0300 Subject: [PATCH] feat(scope-resolution): resolve members through a call result's return shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The question three rounds of reports could not answer, and the one narrowing must refuse by design: a field produced by SEVERAL functions. A read of `spike.wickRatio` could mean any producer, so name inference correctly declines and no amount of tier-tuning changes that. It needs evidence, not inference. The evidence existed in two halves that had never been joined. The call-result type binding (`const alert = formatSpikeAlert(row)` binds `alert` to a TypeRef whose rawName is the callee) predates all of this work; it simply had nothing to resolve to when the callee returned an anonymous literal, because an anonymous literal named nothing. R3-4 gave it a name. Joining them: const alert = formatSpikeAlert(row); alert.wickRatio -> Property:...:formatSpikeAlert.wickRatio Precise, at ordinary emission confidence, and it works EXACTLY where narrowing cannot: several producers sharing a field name stop being competitors because the receiver says which one. Runs before the name fallback and claims its sites, so a precise answer is never second-guessed by a name match. Measured on the reporting repo: 1,410 precise edges, and all six fields round 3 verified OUT-OF-SAMPLE go from 0 backend readers to 7, 11, 10, 7, 6 and 14. Round 3 scored 0/6 on that set; this is 6/6. The bound is asserted, not just documented: a read off a BARE PARAMETER has no binding here, because typing it needs the caller's type to flow in — that is inter-procedural and genuinely larger. Those reads still fall through to name inference and are still reported when it declines. The fixture has two producers sharing a field name precisely so the test cannot pass by name matching, and mutation-checking the owner lookup fails it. No SCHEMA_BUMP: this is scope resolution, not parse-time capture, so a warm cache already carries everything it reads. Noted in the ledger because the reflex on this branch has been to bump, and an unnecessary bump costs every user a full re-parse. Co-Authored-By: Claude Opus 5 (1M context) --- .../passes/return-shape-members.ts | 151 ++++++++++++++++++ .../scope-resolution/pipeline/run.ts | 21 ++- gitnexus/src/storage/parse-cache.ts | 4 + .../return-shape-typed.js | 32 ++++ .../javascript-object-properties.test.ts | 45 ++++++ 5 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts create mode 100644 gitnexus/test/fixtures/lang-resolution/javascript-object-properties/return-shape-typed.js diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts new file mode 100644 index 000000000..567387fff --- /dev/null +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/return-shape-members.ts @@ -0,0 +1,151 @@ +/** + * PRECISE member resolution through a call result's RETURN SHAPE (R3-5). + * + * The last unanswered question from three rounds of blind-spot reports was + * "who reads `wickRatio`?", where the field is produced by several functions + * that each return an anonymous object containing it. Name inference must + * refuse that — a read of `spike.wickRatio` could mean any producer, and a + * wrong edge in the pre-edit safety gate is worse than a missing one — so no + * amount of narrowing gets there. It needs EVIDENCE instead of inference. + * + * The evidence already exists in two halves that had never been joined: + * + * 1. The call-result type binding. `const alert = formatSpikeAlert(row)` + * binds `alert` to a `TypeRef` whose `rawName` is the callee. That + * machinery predates this work; it simply had nothing to resolve to when + * the callee returned an anonymous literal, because an anonymous literal + * named nothing. + * 2. R3-4 gave it a name. A returned literal's keys are now owned by the + * producing function, so `formatSpikeAlert.wickRatio` is a real symbol. + * + * Joining them turns a refusal into a precise answer: + * + * const alert = formatSpikeAlert(row); + * alert.wickRatio → Property:…:formatSpikeAlert.wickRatio + * + * and it works for exactly the case narrowing cannot: several producers sharing + * a field name are no longer competitors, because the receiver says WHICH one. + * That is why this runs before the unique-name fallback and registers its sites + * as handled — a precise answer must never be second-guessed by a name match. + * + * BOUND, deliberately. This only fires where the value is BOUND to a name the + * type binding could attach to. A field read off a bare parameter + * (`function f(spike) { return spike.wickRatio }`) still has no receiver type + * here, because typing it requires the CALLER's type to flow in — that is + * inter-procedural and genuinely larger. Those reads keep falling through to + * name inference, and keep being reported when it declines. + */ + +import type { ParsedFile } from 'gitnexus-shared'; +import type { KnowledgeGraph } from '../../../graph/types.js'; +import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import type { GraphNodeLookup } from '../graph-bridge/node-lookup.js'; +import { resolveCallerGraphId } from '../graph-bridge/ids.js'; +import { findReceiverTypeBinding } from '../scope/walkers.js'; +import { callableFlowSiteKey } from './callable-value-flow.js'; +import type { PropertyNameIndex } from './unique-name-properties.js'; + +/** + * Confidence for a return-shape member. This is a PRECISE resolution — the + * receiver's binding names the producing function and the member is owned by + * it — so it carries the ordinary emission confidence, not the reduced tier + * name inference uses. Nothing here is guessed. + */ +const RETURN_SHAPE_CONFIDENCE = 0.9; + +const EDGE_REASON = 'scope-resolution: return-shape member'; + +export interface ReturnShapeMemberStats { + /** ACCESSES edges resolved through a call result's return shape. */ + readonly emitted: number; + /** + * Sites where the receiver WAS typed to a producer but that producer owns no + * member of this name. Reported rather than dropped: it means the read and + * the shape disagree, which is either a stale field name or a producer this + * pass mis-attributed, and both are worth seeing. + */ + readonly memberNotOnShape: number; +} + +/** + * Does this Property node id name `.`? + * + * Ids carry an optional position suffix for function-local symbols + * (`…:buildFlat.field@33:4`), so the owner segment is matched up to a `@` or + * the end rather than by equality. + */ +function idNamesMember(id: string, owner: string, member: string): boolean { + const needle = `:${owner}.${member}`; + const at = id.indexOf(needle); + if (at === -1) return false; + const after = id.slice(at + needle.length); + return after.length === 0 || after.startsWith('@'); +} + +export function emitReturnShapeMemberAccesses( + graph: KnowledgeGraph, + indexes: ScopeResolutionIndexes, + parsedFiles: readonly ParsedFile[], + nodeLookup: GraphNodeLookup, + /** Sites a precise pass already owns — never re-resolved here. */ + skipSites: ReadonlySet, + propertyNameIndex: PropertyNameIndex, + /** Sites this pass resolves, so the name fallback leaves them alone. */ + handledSink: Set, +): ReturnShapeMemberStats { + let emitted = 0; + let memberNotOnShape = 0; + const seen = new Set(); + + for (const parsed of parsedFiles) { + for (const site of parsed.referenceSites) { + if (site.kind !== 'read' && site.kind !== 'write') continue; + const receiver = site.explicitReceiver?.name; + if (receiver === undefined || receiver.length === 0) continue; + const siteKey = callableFlowSiteKey(parsed.filePath, site.atRange); + if (skipSites.has(siteKey)) continue; + + // The receiver's binding names the PRODUCER, not a class. That is the + // whole point: `formatSpikeAlert` is a function, and before R3-4 there + // was nothing named after it to look a member up on. + const typeRef = findReceiverTypeBinding(site.inScope, receiver, indexes); + const producer = typeRef?.rawName; + if (producer === undefined || producer.length === 0) continue; + + const candidates = propertyNameIndex.get(site.name); + if (candidates === undefined) continue; + const owned = candidates.filter((c) => idNamesMember(c.id, producer, site.name)); + // Exactly one, or nothing. Two nodes claiming `.` would + // mean the id qualifier failed to separate them, and picking between them + // would be the guess this pass exists to avoid. + if (owned.length !== 1) { + if (owned.length === 0) memberNotOnShape++; + continue; + } + const target = owned[0]!; + + const callerGraphId = resolveCallerGraphId(site.inScope, indexes, nodeLookup, site.atRange); + if (callerGraphId === undefined) continue; + if (callerGraphId === target.id) continue; + + const dedupKey = `ACCESSES:${callerGraphId}->${target.id}:${site.atRange.startLine}:${site.atRange.startCol}`; + if (seen.has(dedupKey)) continue; + seen.add(dedupKey); + + graph.addRelationship({ + id: `rel:${dedupKey}`, + sourceId: callerGraphId, + targetId: target.id, + type: 'ACCESSES', + confidence: RETURN_SHAPE_CONFIDENCE, + reason: `${EDGE_REASON}: ${site.kind}`, + evidence: [], + }); + // Claim the site so the name fallback cannot re-answer it differently. + handledSink.add(siteKey); + emitted++; + } + } + + return { emitted, memberNotOnShape }; +} diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index f13e43505..35b07a157 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -77,9 +77,11 @@ import { } from '../passes/property-dispatch.js'; import { emitReferencesViaLookup } from '../graph-bridge/references-to-edges.js'; import { + buildPropertyNameIndex, emitUniqueNamePropertyAccesses, type PropertyNameIndex, } from '../passes/unique-name-properties.js'; +import { emitReturnShapeMemberAccesses } from '../passes/return-shape-members.js'; import { emitImportedValueReferences } from '../passes/imported-value-refs.js'; import { createCalleeIdAccumulator, @@ -1053,6 +1055,23 @@ export function runScopeResolution( // TS-anchor case R3-1 was filed about — the identical defect, mirrored. // `reportOnly` counts without emitting: no edge, no inference, no change to // what the opt-out protects. + // PRECISE first (R3-5). A call result's return shape names WHICH producer a + // receiver holds, so it answers exactly the case name inference must refuse: + // several functions returning the same field name. Sites it resolves are + // added to the skip set, so the fallback below never second-guesses them. + const sharedPropertyIndex = input.prebuiltPropertyNameIndex ?? buildPropertyNameIndex(graph); + const returnShapeMembers = callableFlowOnly + ? { emitted: 0, memberNotOnShape: 0 } + : emitReturnShapeMemberAccesses( + graph, + indexes, + emitParsedFiles, + postHeritageNodeLookup, + uniqueNameSkipSites, + sharedPropertyIndex, + uniqueNameSkipSites, + ); + const nameFallbackDisabled = provider.fieldFallbackOnMethodLookup === false; const uniqueNameProperties = callableFlowOnly ? { @@ -1070,7 +1089,7 @@ export function runScopeResolution( postHeritageNodeLookup, uniqueNameSkipSites, finalized, - input.prebuiltPropertyNameIndex, + sharedPropertyIndex, nameFallbackDisabled, ); diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 0b8535df8..f0a3f0036 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -300,6 +300,10 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // is a different capture set wearing the same number — which is exactly what // the note above records for 33/34. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. +// 49 -> 50 is NOT needed for R3-5: that pass is scope-resolution, not +// parse-time capture, so a warm cache replays ParsedFiles that already carry +// everything it reads. Recorded because the reflex on this branch has been to +// bump, and a bump nobody needs still forces every user a full re-parse. const SCHEMA_BUMP = 49; const GITNEXUS_PKG_VERSION = (() => { try { diff --git a/gitnexus/test/fixtures/lang-resolution/javascript-object-properties/return-shape-typed.js b/gitnexus/test/fixtures/lang-resolution/javascript-object-properties/return-shape-typed.js new file mode 100644 index 000000000..67cdae6b9 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/javascript-object-properties/return-shape-typed.js @@ -0,0 +1,32 @@ +// R3-5: TWO producers returning the same field name — the case name inference +// must refuse, because `x.ambiguousProducedField` alone cannot say which shape +// is meant. The receiver's binding says which, so this resolves precisely. +export function producerAlpha(row) { + return { + ambiguousProducedField: row.a, + }; +} + +export function producerBeta(row) { + return { + ambiguousProducedField: row.b, + }; +} + +// BOUND to the call result, so the type binding attaches. +export function readsAlpha(row) { + const shaped = producerAlpha(row); + return shaped.ambiguousProducedField; +} + +export function readsBeta(row) { + const shaped = producerBeta(row); + return shaped.ambiguousProducedField; +} + +// THE BOUND of the mechanism: a bare parameter has no binding here, because +// typing it needs the CALLER's type to flow in. This must stay unresolved and +// fall through to name inference, which will refuse it (two producers). +export function readsUnbound(shaped) { + return shaped.ambiguousProducedField; +} diff --git a/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts b/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts index 56ba7a455..080212e93 100644 --- a/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts +++ b/gitnexus/test/integration/resolvers/javascript-object-properties.test.ts @@ -235,6 +235,51 @@ describe('JavaScript plain-object property access (A1/A5)', () => { }); }); + // R3-5. The question three rounds of reports could not answer: a field + // produced by SEVERAL functions. Name inference must refuse it — the name + // alone cannot say which shape is meant — so this replaces inference with + // evidence, joining the call-result type binding (which already existed) to + // the return-shape owner (which R3-4 created). + describe('return-shape members via the call result (R3-5)', () => { + const targetOf = (source: string): string[] => + getRelationships(result, 'ACCESSES') + .filter((e) => e.target === 'ambiguousProducedField' && e.source === source) + .map((e) => String((e.rel as { targetId?: string }).targetId ?? '')); + + it('resolves to the producer the receiver actually holds', () => { + expect(targetOf('readsAlpha').some((id) => id.includes('producerAlpha.'))).toBe(true); + expect(targetOf('readsBeta').some((id) => id.includes('producerBeta.'))).toBe(true); + }); + + // The discriminator. Both producers own a field of this name, so a name + // match cannot tell them apart — getting the RIGHT one is only possible + // because the receiver's binding names the producer. + it('does not cross the two producers', () => { + expect(targetOf('readsAlpha').some((id) => id.includes('producerBeta.'))).toBe(false); + 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', + )) { + expect(e.rel.confidence).toBeGreaterThan(0.85); + } + }); + + // 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 + // inference, which refuses because two producers share the name. + it('leaves an unbound receiver to name inference, which refuses it', () => { + expect(targetOf('readsUnbound')).toEqual([]); + }); + }); + // R2. Strict workspace uniqueness was measurably too blunt: in the reporting // repo `exitMinAtrMult` had 26 definitions, 16 of them in one-off scripts the // backend has no relationship with, so every backend read was refused because