fix(scope-resolution): bind a producer's own returned key to itself, and stop claiming uniqueness for a ranked answer

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) <noreply@anthropic.com>
This commit is contained in:
ReidenXerx 2026-08-07 19:31:02 +03:00
parent a76c4c70fe
commit 376de5c3ce
2 changed files with 100 additions and 11 deletions

View file

@ -299,6 +299,21 @@ function buildDirectImportMap(
return byFile;
}
/** Is this Property id qualified by `<owner>.`, 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<string> | 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;

View file

@ -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