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
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,27 @@ and public product updates.

## Unreleased

### 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
*worse*: told that "net revenue" is `inv.net_amt`, it summed that column after joining down to
`inv_line` and double-counted every invoice across its lines (2,520.50 instead of 1,074). This
was the one case the grounded arm lost in the hard A/B, and it lost it because of the grounding
- `SemanticMeasure` now carries `grain`, and the grounding context states it plainly: *"measures
are per inv row; joining InvLine repeats each inv row, so aggregate at that grain instead of
summing across the join"*. The warning is derived from each entity's existing `has_many`, so it
needs no new inference
- **Measured**: the target case went **0/3 to 3/3** and the hard A/B delta went **+22.2 to +30.6
points** (grounded 31/36 = 86.1%, 10/12 cases; raw-sql 20/36 = 55.6%). The control arm gets no
grounding context, so it is an unchanged control across both runs
- The fan-out case also stopped double-counting (its wrong answer of 8,156 is gone), though it now
fails on a grading artifact: the agent answers a two-part question with two queries and the row
grader sees only the last. That case is queued to be reframed
- Reported alongside: `hard-safety-no-write` regressed 2/3 to 0/3 in the grounded arm with its
same known failure mode. Nothing in this change touches that path and the control moved by one
run on identical inputs, so it reads as variance on a boundary case rather than a caused
regression - but it is recorded rather than smoothed over

### A harder trap dataset, and a moat claim that now holds

- New `evals/dataset-hard/` (11 tables, deliberately bad naming) plus a committed glossary at
Expand Down
14 changes: 7 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ suite has produced:
| | dataset | grounded | raw-sql | delta |
|---|---|---|---|---|
| run pass rate | original | 29/36 (80.6%) | 28/36 (77.8%) | +2.8 |
| run pass rate | **hard** | **29/36 (80.6%)** | **21/36 (58.3%)** | **+22.2** |
| mean tool steps | hard | **1.5** | 4.5 | ~60% fewer |
| run pass rate | **hard** | **31/36 (86.1%)** | **20/36 (55.6%)** | **+30.6** |
| mean tool steps | hard | **1.7** | 4.5 | ~60% fewer |

On the original 7-table dataset the grounding buys **nothing measurable on accuracy**: +2.8
points is inside the noise floor, the two metrics disagree on direction, and 8 of 12 cases pass
Expand All @@ -262,11 +262,11 @@ On the hard dataset it wins clearly, and for a legible reason: every one of the
failures traces to the same thing - it does not exclude void invoices, a business rule the
schema cannot express and only the glossary carries. Two cases go 3/3 versus **0/3**.

Two marks against it, kept in the open: grounding *lost* one case by summing an invoice-grain
measure at line grain (measures carry no grain - see ROADMAP step 6.2), and both arms fail the
fan-out case identically, so grounding does not prevent fan-out once the agent hand-writes SQL.
Every number is printed with validity checks - turn-budget exhaustion and baseline controls -
that must be read before the score.
This suite has also caught the product making the agent *worse*: grounding once lost a case by
summing an invoice-grain measure at line grain, because measures carried no notion of grain.
Fixing that (ROADMAP step 6.2) moved the delta from +22.2 to +30.6 - which is the whole point of
keeping a scoreboard. Every number is printed with validity checks - turn-budget exhaustion and
baseline controls - that must be read before the score.

## `explain`: justify every join

Expand Down
32 changes: 25 additions & 7 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,9 @@ competitor is cloud/warehouse-native.
hard enough to tell. On the original trap dataset grounding moved the run pass rate by +2.8 points,
inside the noise floor, with the two metrics disagreeing on direction - a null result. On a dataset
built with opaque keys, natural-key joins and business rules the schema cannot express, the same
comparison gives **+22.2 points (80.6% vs 58.3%)** with both metrics agreeing, and the control's
comparison gives **+30.6 points (86.1% vs 55.6%)** with both metrics agreeing, and the control's
failures collapse to a single cause: it does not know the rules only a glossary carries.
Efficiency held in both runs (**1.5 vs 4.5** mean tool steps here, ~60% fewer). The honest
Efficiency held in every run (**1.7 vs 4.5** mean tool steps here, ~60% fewer). The honest
qualifier: on easy schemas the semantic layer buys nothing on accuracy, and it can even mislead -
see the measure-grain defect in step 6.2. Build one step at a time:

Expand Down Expand Up @@ -311,8 +311,9 @@ see the measure-grain defect in step 6.2. Build one step at a time:
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.
2. **Measures have no grain, so naming one can actively mislead.** This is the single
case the grounded arm *lost* in the hard A/B, and it lost it because of the grounding.
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.
The glossary names `inv.net_amt` as "net revenue"; asked to break revenue down by product
category the agent reached for that measure and summed it after joining down to
`inv_line`, double-counting each invoice across its lines (2520.5 instead of 1074 for
Expand All @@ -321,9 +322,26 @@ see the measure-grain defect in step 6.2. Build one step at a time:
`agg` and `column` but nothing about the grain it is valid at, `query_metric` refuses
cross-grain joins only inside its own compiler, and `compile-metric.ts:99` cannot do the
two hops this question needs, so the agent falls through to hand-written SQL with a
measure it has no safe way to use. Candidate fix: record each measure's grain (its base
table's key) and surface it in the context, so "sum_net_amt is per invoice" is something
the agent can read.
measure it has no safe way to use.
**The fix**: `SemanticMeasure` now carries `grain` (the table it is counted once per), and
`buildAskContext` states it where the agent reads - "measures are per inv row; joining
InvLine repeats each inv row, so aggregate at that grain instead of summing across the
join". The warning is derived from each entity's existing `has_many`, so it costs no new
inference.
**Measured on the hard A/B** (same configuration; the control arm receives no grounding
context, so it is an unchanged control): the target case went **0/3 to 3/3**, and the
overall delta went **+22.2 to +30.6 points** (grounded 31/36 = 86.1%, 10/12 cases;
raw-sql 20/36 = 55.6%, 6/12). The fan-out case also stopped double-counting: its failure
changed from `8156` (a 4x inflation) to a grading artifact, see below.
**Two things not to read past.** (a) `hard-safety-no-write` regressed in the grounded arm,
2/3 to 0/3, with the same known failure mode (answering 18 = 20 minus the 2 void invoices,
i.e. simulating the deletion). Nothing in this change touches that path, the control arm
moved 21/36 to 20/36 on identical inputs, and the case was never reliably passing - so this
is most consistent with variance on a boundary case, though a longer context diluting
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
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
Expand Down
9 changes: 9 additions & 0 deletions src/core/agent/ask-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,15 @@ function renderEntities(model: SemanticModel): string {
if (entity.measures.length > 0) {
const measures = entity.measures.map((m) => `${m.name} (${m.agg})${annotation(m)}`).join(", ");
lines.push(` measures: ${measures}`);
// A measure is one value per its own row. Joining a has_many child multiplies
// those rows, so summing afterwards silently double-counts — the failure mode
// is invisible in the result, which is exactly why it has to be said here.
if (entity.hasMany.length > 0) {
lines.push(
` measures are per ${entity.table} row; joining ${entity.hasMany.join(" or ")} ` +
`repeats each ${entity.table} row, so aggregate at that grain instead of summing across the join`
);
}
}
return lines.join("\n");
});
Expand Down
6 changes: 4 additions & 2 deletions src/core/discovery/semantic-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,14 @@ function enrichEntity(
): { dimensions: SemanticDimension[]; measures: SemanticMeasure[] } {
const keys = keyColumns(table, relationships);
const dimensions: SemanticDimension[] = [];
const measures: SemanticMeasure[] = [{ name: `${table}_count`, agg: "count" }];
// Every measure is valid at one row per this table; carrying that explicitly is
// what lets the agent see when a join would double-count it.
const measures: SemanticMeasure[] = [{ name: `${table}_count`, agg: "count", grain: table }];

for (const col of profile.columns) {
if (keys.has(col.name) || isIdLike(col.name, table)) continue;
if (col.kind === "numeric") {
measures.push({ name: `sum_${col.name}`, agg: "sum", column: col.name });
measures.push({ name: `sum_${col.name}`, agg: "sum", column: col.name, grain: table });
continue;
}
const dim = dimensionFor(col, profile.rowCount);
Expand Down
6 changes: 6 additions & 0 deletions src/core/types/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ export interface SemanticMeasure {
name: string;
agg: "count" | "sum";
column?: string;
/**
* The table this measure is counted once per. Summing it after joining a table
* that has many rows per grain row double-counts, which is invisible in the
* result and easy for an agent to do when it hand-writes SQL.
*/
grain?: string;
/** Human/AI-authored description (empty until enriched from a glossary). */
description?: string;
/**
Expand Down
43 changes: 43 additions & 0 deletions test/glossary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,46 @@ test("prepareDataset applies glossary.json as curation, like verdicts.json", asy
db.close();
}
});

// ---- measure grain -------------------------------------------------------------

test("measures carry the grain they are valid at, and the context warns about it", async () => {
const { buildSemanticModel } = await import("../src/core/discovery/semantic-model");
const { buildAskContext } = await import("../src/core/agent/ask-context");
const { createNodeDb } = await import("../src/engine/duckdb/connection");
const { resolveSource } = await import("../src/adapters/cli/source");
const { profileTable } = await import("../src/engine/duckdb/profile");
const { discoverRelationships } = await import("../src/core/discovery/relationships");

const dir = await mkdtemp(path.join(tmpdir(), "querypad-grain-"));
await writeFile(path.join(dir, "orders.csv"), "id,total\n1,100.0\n2,250.0\n3,75.0\n");
await writeFile(
path.join(dir, "order_lines.csv"),
"id,order_id,qty\n1,1,2\n2,1,3\n3,2,1\n4,3,5\n5,3,1\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 model = buildSemanticModel(tables.map((t) => t.name), rels, 1, profiles);

const orders = model.entities.find((e) => e.table === "orders")!;
const total = orders.measures.find((m) => m.column === "total");
assert.equal(total?.grain, "orders", "a measure is one value per its own table's row");
assert.equal(orders.measures.find((m) => m.agg === "count")?.grain, "orders");

// The warning must name the child that would repeat the rows: summing orders.total
// after joining order_lines double-counts, and nothing in the result shows it.
const context = buildAskContext({ tables, relationships: rels, semanticModel: model });
assert.match(context, /measures are per orders row/);
assert.match(context, /joining OrderLine repeats each orders row/);

// A leaf entity has no has_many, so it gets no warning to ignore.
const lines = model.entities.find((e) => e.table === "order_lines")!;
assert.equal(lines.hasMany.length, 0);
} finally {
db.close();
}
});