fix: a unique price column is not a foreign-key target

CHANGELOG.md

@@ -5,6 +5,24 @@ and public product updates.
 
 ## Unreleased
 
+### A price column is no longer mistaken for a join target (bug fix)
+
+- Relationship inference treated **any** unique, non-null column as a valid foreign-key target.
+  A small price list has unique prices, so every money column whose values happened to coincide
+  was "explained" as a foreign key into it - measured at 81%, 68% and 54% on a realistic fixture
+- The damage was worse than a wrong edge in the graph. Both endpoints of a relationship are
+  excluded from the semantic model, so a phantom edge **silently deleted the table's real
+  measure**: an invoice table stopped exposing `sum_net_amt` at all, with nothing in the output
+  to say why
+- A non-id target is now credible only when the foreign column names it outright, which is what
+  a genuine natural key looks like (`sku -> sku`, `region_cd -> region_cd`). Uniqueness alone no
+  longer qualifies
+- `isIdLike` moved to `signals.ts` as the single shared definition. The two halves of the engine
+  had disagreed about it: the semantic model already knew that "a unique, non-null column like
+  `amount` is a real measure, not a key", and inference did not
+- Both engine suites are unchanged (18/18 and 25/25), so no real edge was lost, and a regression
+  test pins the fixture that reproduces it
+
 ### Measures now carry their grain (bug fix)
 
 - A measure had no notion of the grain it is valid at, so being handed one could make the agent

ROADMAP.md

@@ -302,15 +302,24 @@ see the measure-grain defect in step 6.2. Build one step at a time:
    With the web app retired this is the interactive surface: the coding agent is the UI.
 6. **Engine defects surfaced by the hard dataset** - open, and worth fixing before more
    semantic-layer work.
-   1. **Numeric value overlap creates phantom foreign keys.** `isKeyCandidate`
+   1. **Numeric value overlap creates phantom foreign keys** - ✅ Fixed (2026-07-26). `isKeyCandidate`
       (`src/core/discovery/relationships.ts:38`) accepts any column that is unique and non-null
       in its own table, so a small lookup table's `list_amt` is a valid FK *target*. Every money
       column whose values coincide with those prices then gets an edge (measured: 81%, 68%, 54%
       on the hard fixture before it was made realistic). The damage is not just a wrong edge in
       the graph: `keyColumns` (`semantic-model.ts:75`) excludes both endpoints of every edge from
       dimensions **and** measures, so the table's real money measure disappears without a word.
-      Candidate fix: require an FK target to look like a key (id-like name, or referenced by a
-      name-similar column), or refuse targets that are themselves measures.
+      **The fix**: a non-id target is only credible when the foreign column names it outright
+      (`nameSimilarity >= STRONG_NAME_SIMILARITY`), which is what a real natural key looks like -
+      `sku -> sku`, `region_cd -> region_cd`. Uniqueness alone no longer qualifies a column as a
+      join target. `isIdLike` moved to `signals.ts` and is now the single definition shared by
+      relationship inference and model derivation, which previously disagreed about it: the
+      semantic model already knew "a unique, non-null column like `amount` is a real measure, not
+      a key", and inference did not.
+      Verified on an isolated fixture (a 4-row price list whose prices coincide with a sales
+      table's line amounts): the phantom edge disappears, the legitimate `sku` join survives at
+      100%, and both `sum_line_amt` and `sum_list_amt` come back. Both engine suites are
+      unchanged (18/18 and 25/25), so no real edge was lost.
    2. **Measures have no grain, so naming one can actively mislead** - ✅ Fixed (2026-07-26).
       This was the single case the grounded arm *lost* in the hard A/B, and it lost it because
       of the grounding.

src/core/discovery/relationships.ts

@@ -5,6 +5,7 @@ import {
   NAME_SIMILARITY_FLOOR,
   OVERLAP_FLOOR,
   STRONG_NAME_SIMILARITY,
+  isIdLike,
   cardinalityShapeScore,
   confidence,
   isTypeCompatible,
@@ -92,6 +93,20 @@ export async function discoverRelationships(
           ) {
             continue;
           }
+
+          // Uniqueness alone does not make a column a key: a small price list has
+          // unique prices, so every money column that happens to match one of them
+          // would be "explained" as a foreign key into it. Worse than a wrong edge,
+          // that silently deletes the measure — both endpoints of a relationship are
+          // excluded from the semantic model. A non-id target is therefore only
+          // credible when the foreign column names it outright, which is what a real
+          // natural key looks like (sku -> sku, region_cd -> region_cd).
+          if (
+            !isIdLike(keyColumn.name, keyProfile.tableName) &&
+            nameScore < STRONG_NAME_SIMILARITY
+          ) {
+            continue;
+          }
           candidates.push({
             keyTable: keyProfile.tableName,
             keyColumn,

src/core/discovery/semantic-model.ts

@@ -6,7 +6,7 @@ import type {
   SemanticMeasure,
   SemanticModel,
 } from "../types/discovery";
-import { singularize, splitTokens } from "./signals";
+import { isIdLike, singularize, splitTokens } from "./signals";
 
 /**
  * Roll inferred relationships into named business entities. Pure — tables +
@@ -81,16 +81,6 @@ function keyColumns(table: string, relationships: Relationship[]): Set<string> {
   return keys;
 }
 
-/**
- * True for id-named columns (surrogate keys). We exclude these from measures/dimensions
- * by name rather than by uniqueness — a unique, non-null column like `amount` is a real
- * measure, not a key, so summing it is meaningful.
- */
-function isIdLike(name: string, table: string): boolean {
-  const lower = name.toLowerCase();
-  return lower === "id" || lower.endsWith("_id") || lower === `${singularize(table)}_id`;
-}
-
 /** Derive a dimension candidate from a non-key column, or null if it isn't one. */
 function dimensionFor(col: ColumnProfile, rowCount: number): SemanticDimension | null {
   if (col.kind === "date") {

src/core/discovery/signals.ts

@@ -82,6 +82,16 @@ export function nameSimilarity(
 }
 
 /** Are two column kinds joinable at all? */
+/**
+ * True for id-named columns (surrogate keys). Deliberately name-based, not
+ * uniqueness-based: a unique, non-null column like `amount` is a real measure, not a
+ * key, so summing it is meaningful and joining on it is not.
+ */
+export function isIdLike(name: string, table: string): boolean {
+  const lower = name.toLowerCase();
+  return lower === "id" || lower.endsWith("_id") || lower === `${singularize(table)}_id`;
+}
+
 export function isTypeCompatible(a: ProfileColumnKind, b: ProfileColumnKind): boolean {
   if (a === b) return true;
   // numbers stored as text vs numeric still routinely join after casting

test/discovery.test.ts

@@ -124,3 +124,54 @@ test("inspect fixtures yields exactly the two true relationships", async () => {
     db.close();
   }
 });
+
+// ---- non-id join targets -------------------------------------------------------
+
+test("a price column is not a join target, so its measure survives", async () => {
+  const { mkdtemp, writeFile } = await import("node:fs/promises");
+  const { tmpdir } = await import("node:os");
+  const { resolveSource } = await import("../src/adapters/cli/source");
+  const { buildSemanticModel } = await import("../src/core/discovery/semantic-model");
+
+  const dir = await mkdtemp(path.join(tmpdir(), "querypad-fk-target-"));
+  // A small price list has unique prices, so it passes the unique + non-null test
+  // that used to be the whole definition of a join target.
+  await writeFile(
+    path.join(dir, "price_list.csv"),
+    "id,sku,list_amt\n1,A,10.00\n2,B,20.00\n3,C,30.00\n4,D,40.00\n"
+  );
+  // line_amt holds those same values by coincidence. It is a measure, not a key.
+  await writeFile(
+    path.join(dir, "sales.csv"),
+    "id,sku,line_amt\n1,A,10.00\n2,B,20.00\n3,A,10.00\n4,C,30.00\n5,D,40.00\n6,B,20.00\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);
+    const keys = rels.map(relationshipKey);
+
+    // The natural key still joins: the foreign column names the target outright.
+    assert.ok(keys.includes("sales.sku->price_list.sku"), `expected the sku join, got ${keys}`);
+    // The money columns must not be mistaken for one, at any confidence.
+    assert.ok(
+      !keys.includes("sales.line_amt->price_list.list_amt"),
+      `value overlap alone must not make a price a join target, got ${keys}`
+    );
+
+    // The damage this prevents: both endpoints of a relationship are dropped from the
+    // semantic model, so a phantom edge silently deletes the table's real measure.
+    const model = buildSemanticModel(tables.map((t) => t.name), rels, 1, profiles);
+    const sale = model.entities.find((e) => e.table === "sales")!;
+    const priceList = model.entities.find((e) => e.table === "price_list")!;
+    assert.ok(sale.measures.some((m) => m.column === "line_amt"), "sales must keep sum_line_amt");
+    assert.ok(
+      priceList.measures.some((m) => m.column === "list_amt"),
+      "price_list must keep sum_list_amt"
+    );
+  } finally {
+    db.close();
+  }
+});