Skip to content

Commit 3937057

Browse files
dmealingclaude
andcommitted
fix(codegen-ts): #368 — the origin.first refusal broke a legal mutual 1:1
extract-view-spec's origin.first correlation threw whenever findReferencesBetween(base, childEntity).length > 1. That walk is BIDIRECTIONAL (find-reference.ts walks [[a,b],[b,a]]), so a mutual 1:1 — Customer.primaryAddressRef -> Address PLUS Address.customerRef -> Customer — returned two entries and hard-failed codegen. That is not #368 ambiguity: the two references point in OPPOSITE directions, the surrounding code explicitly supports both (`referenceHolder: "source" | "target"`), and findReferenceBetween's own contract calls mutual 1:1 "rare, but legal". The refusal is now grouped by HOLDER and fires only when a SINGLE holder declares two or more references onto the other side — the shape the first-match genuinely cannot choose within. The message names that holder. A mutual 1:1 keeps the documented first-match behaviour. Tests: the new mutual-1:1 case asserts extractViewSpec does NOT throw and still resolves source-held on primary_address_id (verified to FAIL pre-fix with exactly the reported error); the existing same-holder ambiguity test still asserts the throw, and now also pins the holder name in the message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lfd8nat1WcSpXffd8YetiC
1 parent bb4cac2 commit 3937057

2 files changed

Lines changed: 88 additions & 2 deletions

File tree

‎server/typescript/packages/codegen-ts/src/projection/extract-view-spec.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,10 +1189,24 @@ function buildSelectSpec(
11891189
// branch to match _validateViaPath/_inferViaSingleHop) — out of scope for #368's
11901190
// silent-first-match fix; the message below reflects the real, narrower remedy.
11911191
const refs = findReferencesBetween(base, childEntity);
1192-
if (refs.length > 1) {
1192+
// findReferencesBetween walks BOTH directions ([base,child] then [child,base]),
1193+
// so ">1 entry" is not by itself #368 ambiguity: a mutual 1:1 —
1194+
// Customer.primaryAddressRef -> Address PLUS Address.customerRef -> Customer —
1195+
// returns two entries pointing in OPPOSITE directions. That shape is legal
1196+
// (findReferenceBetween's own contract calls it "rare, but legal", and the
1197+
// referenceHolder: "source" | "target" branch below exists to serve it), and
1198+
// refusing it broke codegen for a model #368 says nothing about. The real
1199+
// ambiguity is two references declared by the SAME holder onto the other side —
1200+
// that is what the first-match below cannot choose between.
1201+
const ambiguousHolder = [base, childEntity].find(
1202+
(holder) => refs.filter((r) => r.holder === holder).length > 1,
1203+
);
1204+
if (ambiguousHolder !== undefined) {
1205+
const ambiguous = refs.filter((r) => r.holder === ambiguousHolder);
11931206
throw new Error(
11941207
`origin.first correlation from "${base.name}" to "${childEntity.name}" is ambiguous: ` +
1195-
`${refs.map((r) => r.referenceIdentity.name).join(", ")}. ` +
1208+
`"${ambiguousHolder.name}" declares ${ambiguous.length} identity.reference nodes ` +
1209+
`onto the other side (${ambiguous.map((r) => r.referenceIdentity.name).join(", ")}). ` +
11961210
`origin.first's own @via is not consulted for this correlation — reduce to a ` +
11971211
`single identity.reference between these two entities.`,
11981212
);

‎server/typescript/packages/codegen-ts/test/projection/reference-ambiguity.test.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,10 +431,82 @@ describe("extractViewSpec — reference ambiguity (#368)", () => {
431431
}
432432

433433
expect(message).toMatch(/origin\.first correlation from "Team" to "Match" is ambiguous:.*homeTeamRef.*awayTeamRef/s);
434+
// The refusal names the HOLDER that declares both, because that is the shape it
435+
// cannot choose within (see the mutual-1:1 test below for the shape it must not
436+
// refuse).
437+
expect(message).toContain('"Match" declares 2 identity.reference nodes');
434438
// #368 round 2 sibling check: origin.first has no relationship node to attach
435439
// @sourceRefField to at all (the correlation is derived from @of alone), so this
436440
// message must never suggest it — confirming it stays dead-end-free alongside the
437441
// buildJoinTree fix above.
438442
expect(message).not.toContain("@sourceRefField");
439443
});
444+
445+
test("a mutual 1:1 — one reference per holder, pointing opposite ways — is NOT ambiguous", async () => {
446+
// findReferencesBetween walks BOTH directions ([a,b] then [b,a]), so a mutual 1:1
447+
// returns TWO entries. A bare `refs.length > 1` refusal therefore hard-failed
448+
// codegen on a shape #368 says nothing about: the two references point in OPPOSITE
449+
// directions, the surrounding code explicitly supports both (`referenceHolder:
450+
// "source" | "target"`), and findReferenceBetween's own contract calls mutual 1:1
451+
// "rare, but legal". Only a SINGLE holder declaring two-or-more is #368 ambiguity.
452+
const root = await load([
453+
{
454+
"object.entity": {
455+
name: "Customer",
456+
children: [
457+
{ "source.rdb": { "@table": "customers" } },
458+
{ "field.int": { name: "id" } },
459+
{ "field.int": { name: "primaryAddressId" } },
460+
{ "identity.primary": { name: "id", "@fields": "id" } },
461+
// Customer -> Address
462+
{ "identity.reference": { name: "primaryAddressRef", "@fields": "primaryAddressId", "@references": "Address" } },
463+
// Lets the loader's single-hop-@via inference succeed for origin.first.
464+
{ "relationship.association": { name: "addresses", "@objectRef": "Address", "@cardinality": "many" } },
465+
],
466+
},
467+
},
468+
{
469+
"object.entity": {
470+
name: "Address",
471+
children: [
472+
{ "source.rdb": { "@table": "addresses" } },
473+
{ "field.int": { name: "id" } },
474+
{ "field.int": { name: "customerId" } },
475+
{ "identity.primary": { name: "id", "@fields": "id" } },
476+
// Address -> Customer: the other half of the mutual 1:1.
477+
{ "identity.reference": { name: "customerRef", "@fields": "customerId", "@references": "Customer" } },
478+
],
479+
},
480+
},
481+
{
482+
"object.projection": {
483+
name: "CustomerSummary",
484+
children: [
485+
{ "source.rdb": { "@kind": "view", "@table": "v_customer_summary" } },
486+
{ "field.int": { name: "id", extends: "Customer.id" } },
487+
{ "identity.primary": { name: "id", extends: "Customer.id" } },
488+
{
489+
"field.int": {
490+
name: "latestAddressId",
491+
children: [
492+
{ "origin.first": { "@of": "Address.id", "@orderBy": ["id:desc"] } },
493+
],
494+
},
495+
},
496+
],
497+
},
498+
},
499+
]);
500+
501+
const projection = root.objects().find((o) => o.name === "CustomerSummary")!;
502+
// The assertion is that this does NOT throw.
503+
const spec = extractViewSpec(projection, root, { columnNamingStrategy: "snake_case" });
504+
const col = spec.selectSpec.columns.find((c) => c.fieldName === "latestAddressId");
505+
expect(col).toBeDefined();
506+
expect(col!.kind).toBe("first");
507+
// Documented first-match behaviour is preserved: `a` is walked first, so the
508+
// base-side reference wins and the correlation is source-held.
509+
expect((col as { referenceHolder: string }).referenceHolder).toBe("source");
510+
expect((col as { fkColumn: string }).fkColumn).toBe("primary_address_id");
511+
});
440512
});

0 commit comments

Comments
 (0)