fix: an ambiguous join is refused instead of guessed

CHANGELOG.md

@@ -5,6 +5,17 @@ and public product updates.
 
 ## Unreleased
 
+### An ambiguous join is refused instead of guessed (bug fix)
+
+- `ensureJoin` matched on the table pair rather than the column, so a table with two foreign keys
+  into the same target - billing region and shipping region - silently joined on whichever edge
+  sorted first. The result looked perfectly ordinary while answering a question the user had not
+  asked
+- It now refuses and names both candidate keys, so the agent can write the SQL it actually means.
+  That matches how the compiler already handles fan-out: refuse rather than guess
+- Both engine suites are unchanged (18/18, 25/25); a regression test pins a fixture where both
+  edges are inferred at 93%
+
 ### Plan-first turn for multi-part questions
 
 - Asked for revenue *and* ticket counts per customer, the agent wrote one query joining both

ROADMAP.md

@@ -369,8 +369,12 @@ see the measure-grain defect in step 6.2. Build one step at a time:
       has zero renames. Unique names rather than refuse-on-ambiguity - which would have matched
       the compiler's usual style - because a catalog keyed by name cannot hold duplicates and
       `resolve_terms` was offering two identical-looking entries pointing at different tables.
-      The remaining half (`ensureJoin` matching on the table pair rather than the column, so two
-      FKs into one target pick whichever edge sorts first) is still open.
+      **The other half is also fixed**: `ensureJoin` matched on the table pair rather than the
+      column, so two FKs into one target (billing vs shipping region) silently joined on
+      whichever edge sorted first - answering a question the user did not ask, with a result that
+      looks perfectly ordinary. It now refuses and names both candidate keys, matching how the
+      compiler already handles fan-out: refuse rather than guess. Reproduced on a fixture where
+      both edges are inferred at 93%, and both engine suites are unchanged (18/18, 25/25).
       **Re-measured after the fix**: the hard A/B is unchanged at **+30.6** (grounded 31/36,
       raw-sql 20/36) across two runs at different code states, which is a useful reproducibility
       signal for the harness itself.

src/core/discovery/compile-metric.ts

@@ -86,11 +86,19 @@ export function compileMetric(
   // Ensure the fact table can reach `target` via a single many-to-one hop (no fan-out).
   const ensureJoin = (target: string, what: string): string | null => {
     if (target === base || joins.has(target)) return null;
-    const forward = relationships.find((r) => r.from.table === base && r.to.table === target);
-    if (forward) {
+    const forward = relationships.filter((r) => r.from.table === base && r.to.table === target);
+    // More than one key into the same table (billing vs shipping region) means the
+    // grouping is genuinely ambiguous. Picking the first edge would answer a question
+    // the user did not ask, and the result would look perfectly ordinary.
+    if (forward.length > 1) {
+      const keys = forward.map((r) => `${base}.${r.from.column}`).join(" or ");
+      return `Cannot use "${what}": ${base} joins ${target} through more than one key (${keys}), so this grouping is ambiguous. Write the SQL directly and pick the one you mean.`;
+    }
+    if (forward.length === 1) {
+      const edge = forward[0];
       joins.set(
         target,
-        `JOIN ${qi(target)} ON ${qi(base)}.${qi(forward.from.column)} = ${qi(target)}.${qi(forward.to.column)}`
+        `JOIN ${qi(target)} ON ${qi(base)}.${qi(edge.from.column)} = ${qi(target)}.${qi(edge.to.column)}`
       );
       return null;
     }

test/discovery.test.ts

@@ -209,3 +209,52 @@ test("colliding measure names are table-qualified on every side", async () => {
     db.close();
   }
 });
+
+test("two keys into the same table make a grouping ambiguous, and it is refused", async () => {
+  const { buildSemanticModel } = await import("../src/core/discovery/semantic-model");
+  const { compileMetric } = await import("../src/core/discovery/compile-metric");
+  const { mkdtemp, writeFile } = await import("node:fs/promises");
+  const { tmpdir } = await import("node:os");
+  const { resolveSource } = await import("../src/adapters/cli/source");
+
+  const dir = await mkdtemp(path.join(tmpdir(), "querypad-ambig-join-"));
+  // `tier` repeats, so it survives the categorical ratio rule and is groupable.
+  await writeFile(
+    path.join(dir, "regions.csv"),
+    "id,name,tier\n1,West,core\n2,East,core\n3,North,edge\n4,South,edge\n5,Central,core\n6,Far,edge\n"
+  );
+  await writeFile(
+    path.join(dir, "orders.csv"),
+    "id,billing_region_id,shipping_region_id,amount\n" +
+      "1,1,2,10.0\n2,1,3,20.0\n3,2,1,30.0\n4,3,4,40.0\n5,4,2,50.0\n6,5,6,60.0\n"
+  );
+
+  const db = await createNodeDb();
+  try {
+    const { tables } = await resolveSource({ folder: dir }).load(db.runner);
+    const profiles = await Promise.all(tables.map((t) => profileTable(t, db.runner, 1)));
+    const rels = await discoverRelationships(profiles, db.runner);
+    // Both foreign keys are real and both are inferred.
+    const keys = rels.map(relationshipKey);
+    assert.ok(keys.includes("orders.billing_region_id->regions.id"), keys.join(","));
+    assert.ok(keys.includes("orders.shipping_region_id->regions.id"), keys.join(","));
+
+    const model = buildSemanticModel(tables.map((t) => t.name), rels, 1, profiles);
+    const compiled = compileMetric(model, rels, {
+      metric: "orders_count",
+      dimensions: ["tier"],
+    });
+
+    // Silently joining on whichever edge sorted first would answer a question the
+    // user did not ask, and the result would look perfectly ordinary.
+    assert.equal(compiled.ok, false, "an ambiguous grouping must not compile");
+    if (!compiled.ok) {
+      assert.match(compiled.error, /more than one key/);
+      // The refusal has to name both candidates, or the agent cannot act on it.
+      assert.match(compiled.error, /billing_region_id/);
+      assert.match(compiled.error, /shipping_region_id/);
+    }
+  } finally {
+    db.close();
+  }
+});