Skip to content
Merged
2 changes: 2 additions & 0 deletions .claude/skills/slayer-query.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ A `SlayerQuery` is a JSON/dict object. The same shape works across the REST API,

`order[].column` is the short alias (`count`, `revenue_sum`) — not the colon form.

**Ordering by something you don't project.** `order` may name an undeclared column/aggregate ("top-N by X, show only Y, Z"). An **aggregate** (`amount:sum`, `customers.revenue:sum`) is computed hidden, sorted on, and stripped from the result. A **raw row column** is orderable only in a raw-rows query (`distinct_dimension_values: false`); in a grouped/dedup query it's rejected (HTTP 400 — add it to `dimensions` or order by an aggregate of it). A **joined** row column and an inline **transform/composite** (`change(amount:sum)`) are also rejected — project it, or declare it as a measure and order by that.

**Dim-only queries deduplicate.** A query with no measures and at least one dimension or time-dimension auto-emits `GROUP BY <dim/td aliases>` and returns the distinct combinations. The `GROUP BY` is applied before `LIMIT`, so a row cap can't silently drop unique tuples. To opt out, set `"distinct_dimension_values": false` on the query — emits raw rows (no top-level `GROUP BY`), with WHERE / ORDER BY / LIMIT applied as usual. Any measure reference in `measures` / `filters` / `order` raises `DistinctDimensionValuesError` in this mode.

## Measures — colon aggregation
Expand Down
1 change: 1 addition & 0 deletions DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@ implementation detail. Include issue refs when known.
- 2026-08-03 — DEV-1692 time_shift de-collision (DEV-1713): the fix has two halves, both in the TYPED pipeline's `_emit_time_shift_ctes_for_planned` (the hoisted placeholder name `_time_shift_inner` repeats across arithmetic-wrapped shifts). A per-generation `AliasAllocator` (reserve every deterministic CTE name, allocate the `shifted_`/`sjoin_`/`step`/`cp_` families around them) makes CTE NAMES unique, AND the hidden slot's projected value alias is allocated uniquely (`_time_shift_inner`, `_time_shift_inner_2`, …) so downstream arithmetic (resolved by slot id) reads each shift's own value instead of collapsing both onto the first — the value corruption the duplicate CTE name had masked. User-facing (`public_aliases`) shift aliases are already unique and left untouched.
- 2026-08-03 — BigQuery scope-validator carve-out removed (DEV-1713, closing the DEV-1705 inherited item): with BigQuery naming/mangling finalized, no real BigQuery generator output makes sqlglot raise `TypeError` on parse (the dotted-alias shapes are collapsed to `___` before validation, and the stale calendar-time_shift INTERVAL round-trip bug no longer reproduces). The `_SQLGLOT_TYPEERROR_DIALECTS` skip-set is deleted from `scope_check.py` and all three test harnesses; BigQuery output is now scope-validated and CTE-collision-checked like every other dialect, and a parse `TypeError` propagates for every dialect with no exception. Verified empirically by running the full BigQuery test surface with the skip-set emptied — only the self-referential monkeypatch test (which forced a TypeError) reacted; zero real residual, so no follow-up ticket.
- 2026-08-03 — CTE-name collision detection stays case-sensitive for now (DEV-1713 Codex review, deferred to DEV-1726): `AliasAllocator` / `assert_unique_cte_names` compare CTE names by exact string, but generated CTE names are emitted unquoted and case-fold on Postgres/Snowflake/Redshift — so two user measure aliases differing only in case, both generating CTEs, can still collide there. Deferred rather than fixed in Stage 9 because it is pre-existing (pre-DEV-1713 the names weren't deduped at all), an edge case, and the correct fix is dialect-aware (fold only for case-folding dialects; a blanket case-insensitive dedup would wrongly merge genuinely-distinct names on case-sensitive BigQuery / quoted SQLite). Tracked in DEV-1726.
- 2026-08-03 — Order-only hidden slots + plan-time order/partition validations (DEV-1712, DEV-1703 Stage 8). An ORDER BY ref that is not a declared dimension/measure is classified at **plan time** in `stage_planner.plan_query` (right after `_bucket_slots`): an **aggregate** (local or cross-model) always materialises hidden and orders — never rejected; a **local row column** is allowed only in a raw-rows query (`distinct_dimension_values=False`, no grouping) and emits a SPLIT `orders.<col>` reference in the generator's `_apply_order_limit_from_planned` (the old `NotImplementedError` becomes a defensive internal assertion — the plan-time pass guarantees only that shape reaches it); a **grouped** local row column raises `ValueError` (not in GROUP BY — add it to dims or order by an aggregate of it); a **joined** row column raises `UnresolvableOrderColumnError`; an inline **transform/composite** (`change(amount:sum)`, only reachable as `raw_formula` — composite arithmetic is unexpressible via `OrderItem.column`, Pydantic rejects it) raises `ValueError` pointing at "declare it as a measure" — full support deferred to **DEV-1733** with strict-xfail future tests + a matching worktree. The grouping predicate is planner-semantic: `bool(agg_slots) or (dims/tds present and distinct_dimension_values)` — a hidden order aggregate that induces grouping counts, so a row column in that same query is correctly rejected. **Hidden cross-model aggregate trim** (DEV-1495 bug 2): an order-only CMA gets `hidden=True`/`public_alias=None` from the planner; the generator's combined-projection loop skips it (`public_aliases=[]`) **only when there is no transform chain** — a hidden CMA feeding a `cumsum(...)` step must stay projected for the step CTE to consume (the transform outer-wrap does the public-vs-hidden trim there). Its ORDER BY term is CTE-qualified (`_cm_*."<alias>"`) since the bare alias is no longer projected. The DEV-1495-bug-1 malformed-alias half (`orders.customers._sum`) was already fixed by Stage 9's naming module — only the projection leak remained. **partition_by grain guard** (DEV-1497): a pre-intern pass (`planning.rewrite_rank_partition_keys`, mirroring `lower_sugar_transforms`' identity-preserving rebuild) validates every rank-family (`rank`/`dense_rank`/`percent_rank`/`ntile`) `partition_by` key resolves to a query dimension/time-dimension by **exact ValueKey membership** — the typed binder resolves `partition_by` to a ValueKey before validation, so the legacy string-matching ambiguity can't arise. A time-dimension **source column** is rewritten to its `TimeTruncKey` so `PARTITION BY` uses the truncated bucket (not the raw timestamp, which had silently widened the GROUP BY grain and emitted a duplicate alias); a non-dimension raises `ValueError` naming the transform + column + available dims (the legacy `enrichment._resolve_rank_partition` message, restored). The 7 DEV-1645 Flavor-A ORDER-BY unit pins (split-not-composite for unprojected sort keys; `UnresolvableOrderColumnError` for joined sort keys) were made green by porting main's legacy `_OrderColRef`/`_order_split_sql`/`_resolve_order_column` fix (lost in the Stage-0 merge) into the legacy generator — throwaway parity, deleted with the legacy stack in Stage 11 — so `tests/parity_xfails.py` is now empty (the DEV-1485 end-state). Deliberate divergence from main: the typed pipeline rejects a grouped raw-row order at plan time (HTTP 400) instead of emitting SQL the database rejects at execution.
2 changes: 1 addition & 1 deletion docs/concepts/formulas.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ Combine with a filter to get "top N":

**Ranking within a partition (`partition_by=`):**

To rank within groups instead of across the whole result set, pass `partition_by=` referencing one or more **query dimensions** (or time dimensions). The columns must already be grouped on — partitioning by a column that's not a dimension errors at enrichment time.
To rank within groups instead of across the whole result set, pass `partition_by=` referencing one or more **query dimensions** (or time dimensions). The columns must already be grouped on — partitioning by a column that's not a dimension errors at plan time (HTTP 400). Naming a query time-dimension partitions by its truncated bucket, not the raw timestamp.

```json
{
Expand Down
21 changes: 21 additions & 0 deletions docs/concepts/queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,27 @@ A sort specification: `column` is the short alias (`status`, `revenue_sum`, `*:c

Via MCP: `{"column": "*:count", "direction": "desc"}`

### Ordering by something you don't project

`order` may reference a column or aggregate that is **not** declared as a dimension/measure — the classic "top-N by metric X, display only Y, Z" pattern:

```json
{"source_model": "orders", "dimensions": ["status"], "measures": [{"formula": "*:count"}],
"order": [{"column": "amount:sum", "direction": "desc"}], "limit": 10}
```

The `amount:sum` aggregate is computed as a hidden column, sorted on, and **stripped from the result** — the response projects only `status` and `_count`. This works for local aggregates, cross-model aggregates (`customers.revenue:sum`), and inner-stage columns re-aggregated in a later DAG stage (`customers__revenue_sum:max`).

What each shape of an *undeclared* order target does:

| Order target | Behavior |
| --- | --- |
| An aggregate (`amount:sum`, `customers.revenue:sum`) | Computed hidden, sorted on, stripped from the result. Always allowed. |
| A raw row column, in a **raw-rows** query (`distinct_dimension_values: false`, no measures) | Sorted on directly (`ORDER BY orders.created_at`). |
| A raw row column, in an **aggregated / dedup** query | Rejected (HTTP 400): it isn't in the `GROUP BY`. Add it to `dimensions`, or order by an aggregate of it (`created_at:max`). |
| A **joined** row column (`customers.regions.name`) not projected | Rejected (HTTP 400): project it (add to `dimensions`) or order by a projected field. |
| An inline **transform / composite** (`change(amount:sum)`, `revenue:sum / cnt:sum`) not declared | Rejected (HTTP 400): declare it as a measure (optionally with a `name`) and order by that. |

## Response

Query results are returned as a `SlayerResponse`:
Expand Down
58 changes: 58 additions & 0 deletions slayer/engine/planning.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
column_path,
normalize_scalar,
)
from slayer.core.formula import RANK_FAMILY_TRANSFORMS
from slayer.core.refs import agg_kwarg_canonical_str, canonical_agg_name
from slayer.engine.binding import BoundExpr, BoundFilter
from slayer.engine.planned import SlotId, ValueSlot
Expand Down Expand Up @@ -401,6 +402,63 @@
return key


def rewrite_rank_partition_keys(key: ValueKey, rewrite_fn) -> ValueKey:

Check failure on line 405 in slayer/engine/planning.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 26 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=MotleyAI_slayer&issues=AZ_IiRFf7ZloDB7cAhbk&open=AZ_IiRFf7ZloDB7cAhbk&pullRequest=274
"""Walk ``key``; for every rank-family ``TransformKey`` carrying an
explicit ``partition_by`` (non-empty ``partition_keys``), replace those
keys with ``rewrite_fn(transform_key)`` (DEV-1497).

``rewrite_fn`` receives the whole ``TransformKey`` and returns a new
``frozenset`` of partition keys — validating that each resolves to a query
dimension / time-dimension and rewriting a time-dimension source column to
its ``TimeTruncKey`` bucket. It may raise ``ValueError`` for a partition
column that is not a query dimension.

Identity-preserving (mirrors :func:`lower_sugar_transforms`): parents are
rebuilt only where a child changed, so this runs BEFORE interning without
churning unrelated slots. Reaches rank transforms nested in composite
measures (``ArithmeticKey`` / ``ScalarCallKey``) and in filter predicates
(comparisons are ``ArithmeticKey``).
"""
if isinstance(key, TransformKey):
new_input = rewrite_rank_partition_keys(key.input, rewrite_fn)
updates: dict = {}
if new_input is not key.input:
updates["input"] = new_input
if key.op in RANK_FAMILY_TRANSFORMS and key.partition_keys:
new_pk = rewrite_fn(key)
if new_pk != key.partition_keys:
updates["partition_keys"] = new_pk
return key.model_copy(update=updates) if updates else key
if isinstance(key, ArithmeticKey):
new_ops = tuple(rewrite_rank_partition_keys(op, rewrite_fn) for op in key.operands)
if all(a is b for a, b in zip(new_ops, key.operands)):
return key
return ArithmeticKey(op=key.op, operands=new_ops)
if isinstance(key, ScalarCallKey):
new_args = tuple(
rewrite_rank_partition_keys(a, rewrite_fn)
if isinstance(a, _SLOTTABLE_KIND + (ArithmeticKey, ScalarCallKey, BetweenKey))
else a
for a in key.args
)
if all(a is b for a, b in zip(new_args, key.args)):
return key
return ScalarCallKey(name=key.name, args=new_args)
if isinstance(key, BetweenKey):
new_col = rewrite_rank_partition_keys(key.column, rewrite_fn)
new_low = rewrite_rank_partition_keys(key.low, rewrite_fn)
new_high = rewrite_rank_partition_keys(key.high, rewrite_fn)
if new_col is key.column and new_low is key.low and new_high is key.high:
return key
return BetweenKey(column=new_col, low=new_low, high=new_high)
if isinstance(key, InKey):
new_col = rewrite_rank_partition_keys(key.column, rewrite_fn)
if new_col is key.column:
return key
return InKey(column=new_col, values=key.values, negated=key.negated)
return key


def desugar_change_pct(key: TransformKey) -> ArithmeticKey:
"""``change_pct(x)`` → ``(x - time_shift(x, periods=-1)) /
NULLIF(time_shift(x, periods=-1), 0)``.
Expand Down
Loading