Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@ and public product updates.

## Unreleased

### Measure names are unique, and the fan-out case is gradeable (bug fix)

- Two tables with the same numeric column produced the same measure name, and the metric compiler
resolved a name by scanning entities in order - so it silently computed the first table's
measure. **This was live**: the committed hard dataset already had `sum_net_amt` on both `inv`
and `inv_staging`, and its engine cases passed only because `inv` sorts before `inv_staging`
- Measure names are now unique across the model, table-qualifying *every* side of a collision so
the outcome never depends on table order. Names that do not collide are untouched
- The eval's fan-out case asked for two numbers, the agent answered with two queries, and the row
grader only saw the last one - it was failing for harness reasons. Reframed to one row per
customer: a single gradeable result set that keeps the trap sharp
- **Re-measured**: 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 reproducibility signal for the harness itself.
The reframed fan-out case still fails, but now for a real reason - an exact 2x fan-out
inflation rather than a grading artifact

### 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.
Expand Down
25 changes: 21 additions & 4 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,30 @@ see the measure-grain defect in step 6.2. Build one step at a time:
attention cannot be ruled out at n=3. (b) `hard-fanout-revenue-and-cases` now fails on
`column count 1, expected 2`: the agent answers the two-part question with two separate
queries and the row grader only sees the last one. That is a **wrong case for this grader**,
queued to be reframed, not an agent error.
3. **Duplicate measure names resolve silently.** Two tables with an `amount` column both
queued to be reframed, not an agent error. **Reframed and re-measured (2026-07-26)**: it now
asks for one row per customer with a support case, a single gradeable result set. It still
fails 0/3, but now for a real reason - `[2, 3405]` against an expected `[1702.5, 2]`, i.e.
exactly the 2x fan-out inflation for a customer with two tickets. The grain warning added in
6.2 appears in the context and did not prevent it, so a passive warning is not enough here.
3. **Duplicate measure names resolve silently** - ✅ Fixed (2026-07-26). Two tables with an `amount` column both
produce a measure named `sum_amount`, and `findMeasure` (`compile-metric.ts:33`) returns the
first by entity order. Same for duplicate dimension names, and `ensureJoin` matches on the
table pair rather than the column, so two FKs into one target pick whichever edge sorts
first. Deliberately left out of the hard dataset: it is an engine ambiguity to fix, not a
grounding trap to grade.
first.
**It was not hypothetical**: the committed hard dataset already had `sum_net_amt` on both
`inv` and `inv_staging`, and its engine cases were passing only because `inv` sorts before
`inv_staging` - the measurement infrastructure had a coin flip in it.
**The fix**: measure names are unique across the model, table-qualifying *every* side of a
collision so the outcome never depends on table order (`inv_sum_net_amt`,
`inv_staging_sum_net_amt`). Names that do not collide are untouched, so the original dataset
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.
**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.
7. Short planning/decomposition for multi-part questions (bounded).
8. **Native desktop app** (decided 2026-07-25) — the flagship product surface.
A native macOS app (Swift + AppKit) embeds a libghostty terminal pane running
Expand Down
4 changes: 2 additions & 2 deletions evals/cases/agent-hard.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@
{
"id": "hard-fanout-revenue-and-cases",
"trap": "fan-out",
"question": "For Acme Corp, show its net revenue and how many support cases it has opened.",
"expectedSql": "SELECT (SELECT ROUND(SUM(i.net_amt), 2) FROM inv i JOIN acct a ON i.acct_id = a.id WHERE a.cust_id = c.id AND i.status <> 'void') AS revenue, (SELECT COUNT(*) FROM tkt t WHERE t.cust_id = c.id) AS cases FROM cust_master c WHERE c.nm = 'Acme Corp'"
"question": "For every customer that has opened at least one support case, show the customer name, their net revenue, and how many support cases they opened.",
"expectedSql": "SELECT c.nm, COALESCE((SELECT ROUND(SUM(i.net_amt), 2) FROM inv i JOIN acct a ON i.acct_id = a.id WHERE a.cust_id = c.id AND i.status <> 'void'), 0) AS revenue, (SELECT COUNT(*) FROM tkt t WHERE t.cust_id = c.id) AS cases FROM cust_master c WHERE EXISTS (SELECT 1 FROM tkt t WHERE t.cust_id = c.id)"
},
{
"id": "hard-safety-no-write",
Expand Down
76 changes: 61 additions & 15 deletions evals/cases/engine-hard.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,84 +72,130 @@
{
"id": "hard-entity-inv",
"kind": "entity",
"describe": "Inv exposes the authoritative money measure and its status/date dimensions",
"describe": "Inv exposes the authoritative money measure (table-qualified: inv_staging has net_amt too) and its status/date dimensions",
"entity": "Inv",
"table": "inv",
"measures": ["inv_count", "sum_net_amt"],
"dimensions": ["status", "issued_on"]
"measures": [
"inv_count",
"inv_sum_net_amt"
],
"dimensions": [
"status",
"issued_on"
]
},
{
"id": "hard-entity-cust-master",
"kind": "entity",
"describe": "CustMaster derives the soft-delete flag as a groupable dimension",
"entity": "CustMaster",
"table": "cust_master",
"measures": ["cust_master_count"],
"dimensions": ["is_active", "signed_on"]
"measures": [
"cust_master_count"
],
"dimensions": [
"is_active",
"signed_on"
]
},
{
"id": "hard-entity-tkt",
"kind": "entity",
"describe": "Tkt exposes severity as a dimension",
"entity": "Tkt",
"table": "tkt",
"measures": ["tkt_count"],
"dimensions": ["sev"]
"measures": [
"tkt_count"
],
"dimensions": [
"sev"
]
},
{
"id": "hard-entity-inv-line",
"kind": "entity",
"describe": "InvLine derives measures over its numeric columns",
"entity": "InvLine",
"table": "inv_line",
"measures": ["inv_line_count", "sum_qty", "sum_unit_amt"]
"measures": [
"inv_line_count",
"sum_qty",
"sum_unit_amt"
]
},
{
"id": "hard-metric-revenue-by-status",
"kind": "metric",
"describe": "the money measure compiles grouped by its own table's dimension",
"metric": { "metric": "sum_net_amt", "dimensions": ["status"] }
"metric": {
"metric": "inv_sum_net_amt",
"dimensions": [
"status"
]
}
},
{
"id": "hard-metric-count-by-status",
"kind": "metric",
"describe": "an invoice count compiles grouped by status",
"metric": { "metric": "inv_count", "dimensions": ["status"] }
"metric": {
"metric": "inv_count",
"dimensions": [
"status"
]
}
},
{
"id": "hard-metric-refuses-payment-fanout",
"kind": "metric",
"describe": "revenue grouped by a payment date would fan out invoices across payments",
"metric": { "metric": "sum_net_amt", "dimensions": ["paid_on"] },
"metric": {
"metric": "inv_sum_net_amt",
"dimensions": [
"paid_on"
]
},
"expectRefusal": true
},
{
"id": "hard-metric-refuses-ticket-fanout",
"kind": "metric",
"describe": "counting customers by ticket severity would fan out customers across tickets",
"metric": { "metric": "cust_master_count", "dimensions": ["sev"] },
"metric": {
"metric": "cust_master_count",
"dimensions": [
"sev"
]
},
"expectRefusal": true
},
{
"id": "hard-metric-refuses-multi-hop",
"kind": "metric",
"describe": "revenue by ticket severity is more than one hop and must be refused rather than guessed",
"metric": { "metric": "sum_net_amt", "dimensions": ["sev"] },
"metric": {
"metric": "inv_sum_net_amt",
"dimensions": [
"sev"
]
},
"expectRefusal": true
},
{
"id": "hard-metric-refuses-unknown",
"kind": "metric",
"describe": "an undefined metric is refused with the available list",
"metric": { "metric": "gross_margin" },
"metric": {
"metric": "gross_margin"
},
"expectRefusal": true
},
{
"id": "hard-term-revenue",
"kind": "term",
"describe": "GLOSSARY-BACKED: 'revenue' reaches the opaque net_amt measure, which shares no token with sum_net_amt",
"term": "revenue",
"resolvesTo": "sum_net_amt"
"resolvesTo": "inv_sum_net_amt"
},
{
"id": "hard-term-customer",
Expand Down
28 changes: 28 additions & 0 deletions src/core/discovery/semantic-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,33 @@ function enrichEntity(
return { dimensions, measures };
}

/**
* Make measure names unique across the whole model, table-qualifying every side of a
* collision. Two tables with an `amount` column both produce `sum_amount`, and the
* metric compiler resolves a name by scanning entities in order — so it silently
* computed the first table's measure, and term resolution offered two entries a user
* could not tell apart. A catalog keyed by name cannot hold duplicates.
*
* Both sides are renamed, not just the later one, so the outcome never depends on
* table order. Names that do not collide are left alone.
*/
function disambiguateMeasures(entities: SemanticEntity[]): void {
const owners = new Map<string, SemanticEntity[]>();
for (const entity of entities) {
for (const measure of entity.measures) {
owners.set(measure.name, [...(owners.get(measure.name) ?? []), entity]);
}
}

for (const [name, holders] of owners) {
if (holders.length < 2) continue;
for (const entity of holders) {
const measure = entity.measures.find((m) => m.name === name);
if (measure) measure.name = `${entity.table}_${name}`;
}
}
}

export function buildSemanticModel(
tableNames: string[],
relationships: Relationship[],
Expand All @@ -162,6 +189,7 @@ export function buildSemanticModel(
hasOne: [],
};
});
disambiguateMeasures(entities);
const byTable = new Map(entities.map((entity) => [entity.table, entity]));

for (const rel of relationships) {
Expand Down
34 changes: 34 additions & 0 deletions test/discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,37 @@ test("a price column is not a join target, so its measure survives", async () =>
db.close();
}
});

test("colliding measure names are table-qualified on every side", async () => {
const { buildSemanticModel } = await import("../src/core/discovery/semantic-model");
const { buildTermCatalog } = await import("../src/core/discovery/term-catalog");
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-dupe-measure-"));
// Two unrelated tables that both have an `amount` column.
await writeFile(path.join(dir, "sales.csv"), "id,amount\n1,10.0\n2,20.0\n");
await writeFile(path.join(dir, "refunds.csv"), "id,amount\n1,5.0\n2,7.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 model = buildSemanticModel(tables.map((t) => t.name), [], 1, profiles);

const names = model.entities.flatMap((e) => e.measures.map((m) => m.name));
assert.equal(new Set(names).size, names.length, `measure names must be unique, got ${names}`);
// Both sides are qualified, so the result never depends on table order.
assert.ok(names.includes("sales_sum_amount"), names.join(","));
assert.ok(names.includes("refunds_sum_amount"), names.join(","));
assert.ok(!names.includes("sum_amount"), "the bare colliding name must not survive");

// The catalog is keyed by term, so duplicates there were unresolvable by a user.
const catalog = buildTermCatalog(model);
const measureTerms = catalog.filter((e) => e.kind === "measure").map((e) => e.term);
assert.equal(new Set(measureTerms).size, measureTerms.length);
} finally {
db.close();
}
});