Fix duplicate CTE name when a query has multiple arithmetic-wrapped transforms

.claude/skills/slayer-query.md

@@ -48,6 +48,8 @@ Each entry in `measures` is either a bare formula string or a `{"formula": ...,
 
 Built-in aggregations: `sum`, `avg`, `min`, `max`, `count`, `count_distinct`, `count_distinct_approx`, `first`, `last`, `weighted_avg`, `median`, `percentile`, `stddev_samp`, `stddev_pop`, `var_samp`, `var_pop`, `corr`, `covar_samp`, `covar_pop`. `count_distinct_approx` is dialect-aware (native approximate-distinct where available, exact `COUNT(DISTINCT)` fallback otherwise). Two-column `corr`/`covar_samp`/`covar_pop` take the second column as a named param: `price:corr(other=quantity)`. `sum` and `avg` accept an optional trailing-window: `revenue:sum(window='30d')`.
 
+For month-over-month / period-over-period growth use `change_pct(x)` (absolute delta: `change(x)`) — both are calendar-aware and partition-safe (the underlying self-join matches on all non-time dimensions, so per-group series reset cleanly). Reach for `time_shift` only when you need the shifted value itself as a term in custom arithmetic or at a different grain (`time_shift(revenue:sum, -1, 'year')` for year-over-year).
+
 `*:count` is always available — no column definition needed. `col:count` counts non-nulls.
 
 Saved named formulas (`SlayerModel.measures`) can be referenced by bare name in any formula context: `{"formula": "aov"}`.

CLAUDE.md

@@ -72,6 +72,7 @@ poetry run ruff check slayer/ tests/
 - **Result column naming**: `revenue:sum` → `orders.revenue_sum` (colon becomes underscore). `*:count` → `orders._count` — the `*` is dropped but the underscore is kept as a leading marker so the alias never collides with a user-defined column literally named `count`. When converting queries to models (`create_model_from_query`), the same colon-to-underscore mapping applies. An explicit `name` on the measure spec overrides the canonical form for **both** simple aggregations and arithmetic/transform formulas — `{"formula": "amount:sum", "name": "rev"}` surfaces as `orders.rev`. This matters most for inner stages of multi-stage `source_queries`, where downstream stages reference inner-stage outputs by the chosen name. **DEV-1443**: when a query measure is renamed via `name`, filters and ORDER BY entries in the same node may reference EITHER the raw colon form (`amount:sum > 100`) OR the user alias (`rev > 100`) — both resolve to the user alias, and a colon-form filter classifies as HAVING. Renaming never changes the legal filter/order form. Two enrichment-time validations on renames: (1) a query measure `name` colliding with a source column on the source model is rejected (the alias-form filter would otherwise silently bind to the source column instead of the aggregate); (2) a rename whose canonical alias (e.g. `revenue_sum`) literally shadows a source column on the same model is also rejected — the colon-form filter would otherwise be ambiguous between the renamed aggregate and the source column. **DEV-1448**: the user-supplied `name` on a *cross-model* aggregated measure (`{"formula": "customers.revenue:sum", "name": "cust_rev"}`) is now honored for the projection alias and the downstream-stage virtual model column. Only the canonical **leaf** of the dotted path is swapped to the user name; the hop path is preserved — same dot-syntax shape as every other multi-hop caller-facing key. So `{"formula": "customers.revenue:sum", "name": "cust_rev"}` surfaces as `orders.customers.cust_rev`, and `{"formula": "customers.regions.population:sum", "name": "region_pop"}` surfaces as `orders.customers.regions.region_pop`. Two caller-facing surfaces: (a) the top-level result-column key uses the hop-preserved form (`orders.customers.cust_rev`); (b) the downstream-stage virtual model column built by `_query_as_model` uses the BARE user name (`cust_rev`) — a special-case in the cross-model loop short-circuits the `__`-flattening of `_alias_to_short` whenever `cm.name` is a bare identifier, so a stage-2 reference like `cust_rev:max` resolves directly. The inner `CrossModelMeasure.measure` (used to build the CTE aggregate expression) intentionally retains the canonical form — only the outer handle is renamed. The canonical-collision guard from DEV-1443 was lifted into a pre-pass so it fires for both local AND cross-model renames symmetrically. Covers `customers.*:count` and `customers.col:count_distinct` cross-model variants. Same-stage **filter** referencing a renamed cross-model measure (`filters=["cust_rev > 100"]` or the colon form `"customers.revenue:sum > 100"`) is NOT auto-resolved — both raise at strict resolution; the SQL generator has no path to route the bare user alias to the cross-model CTE's output column for filter classification, so admitting the filter would emit broken SQL (`WHERE orders.cust_rev > 100` against a column that doesn't exist on the base table). Filter remap stays DEV-1445 territory. **ORDER BY** referencing the bare user alias (`order=[{"column": "cust_rev"}]`) DOES resolve via `SQLGenerator._resolve_order_column`'s `alias_lookup[cm.name] = cm.alias` mapping and emits `ORDER BY "orders.customers.cust_rev"` against the cross-model CTE's output column. Workaround until DEV-1445 lands: restructure as a multi-stage `source_queries` so the cross-model measure becomes a local measure in the downstream stage. See companion tickets DEV-1445 (cross-model filter remap) and DEV-1446 (transform-wrapped inner-ref dedup).
 - **Response attributes**: `SlayerResponse.attributes` is a `ResponseAttributes` with `.dimensions` and `.measures` dicts, each mapping column alias → `FieldMetadata(label, format)`. Split by type so consumers can distinguish dimension metadata from measure metadata.
 - Available formula transforms: cumsum, time_shift, change, change_pct, rank, percent_rank, dense_rank, ntile, first (FIRST_VALUE window ASC), last (FIRST_VALUE window DESC), lag, lead, consecutive_periods. time_shift uses a self-join CTE where the shifted sub-query has the time column expression offset by INTERVAL (calendar-based, gap-safe). change and change_pct are desugared at enrichment time into a hidden time_shift + arithmetic expression. lag/lead use LAG/LEAD window functions directly (more efficient but produce NULLs at edges). Non-transform SQL function calls (`nullif`, `coalesce`, `ln`, `sqrt`, etc.) may also wrap aggregated refs inside arithmetic expressions, e.g. `"*:count / nullif(revenue:max, 0)"` — the call passes through to emitted SQL while the inner refs resolve to their measure aliases
+- **Hidden-transform naming** (DEV-1692): a transform wrapped in arithmetic (`revenue:sum - time_shift(revenue:sum, -1, 'month')`) is hoisted out of the formula under a hidden name. `slayer/core/formula.py::_parse_mixed_arithmetic` mints per-formula placeholders `_t0`, `_t1`, … from a counter that **restarts on every formula parse**, so the `MixedArithmeticField` branch of `_flatten_spec` (`slayer/engine/enrichment.py`) qualifies each one with the owning measure's `field_name` — `_t0_growth`, `_t1_growth_pct` — and then runs it through `_allocate_hidden_name` before handing it to `_add_transform`. The `field_name` qualifier is what separates hoisted names from *each other*, but it is not sufficient on its own: nothing stops a user from also declaring a measure literally named `_t0_growth`, which would claim the same alias and make the hoisted reference silently resolve to that measure (valid SQL, wrong numbers). So allocation is **collision-checked** against every projected name — the query's public measure names (`_public_field_name` over `query.measures`), the selected dimensions and time dimensions (reserved by bare `name`, since they alias as `<model>.<name>` exactly like hoisted transforms do), previously allocated hidden names, and `known_aliases` — appending `_2`, `_3`, … until free. This matters because the name becomes both the self-join CTE name (`shifted_{name}` / `sjoin_{name}` in `slayer/sql/generator.py`) and the projection alias: unqualified, two measures each wrapping a transform both became `_t0`, which surfaced as a duplicate-CTE parser error for `time_shift`/`change` and, for the CTE-less window transforms (`cumsum`, `lag`, …), as *silently wrong values* (the second measure read the first's). The `change`/`change_pct` desugar uses the same scheme (`_ts_{field_name}`) and composes — `change(x) * 2` yields `_ts__t0_{field_name}`. Filter-extracted transforms use a separate `_ft{n}` namespace off a query-wide shared counter, so they never collide either. Extending: any new hoisting path must derive its name from `field_name`, never from a formula-local counter alone
 - **Rank-family transforms** (DEV-1353): `rank`, `percent_rank`, `dense_rank`, and `ntile` are timeless window-function transforms emitted as `RANK() / PERCENT_RANK() / DENSE_RANK() / NTILE(n) OVER (... ORDER BY <measure> DESC)`. They default to **no `PARTITION BY`** (rank across the entire result set, unlike cumsum/lag/lead which auto-partition by query dimensions), and accept an optional `partition_by=col` or `partition_by=[col1, col2]` kwarg to opt into per-partition ranking; the columns referenced must be query dimensions or time dimensions. `ntile` additionally requires `n=<positive int>`. Standard SQL across SQLite (≥3.25), Postgres, DuckDB, MySQL, and ClickHouse — no UDFs needed.
 - Filters can reference computed field names or contain inline transform expressions (e.g., `"change(revenue:sum) > 0"`, `"last(change(revenue:sum)) < 0"`). These are auto-extracted as hidden fields and applied as post-filters on the outer query
 - **Window functions in filters**: filter strings and `ModelMeasure.formula` cannot contain raw `OVER (...)` SQL — SLayer's formula parser is Python-AST-based and rejects with an actionable error pointing at the `rank()` / `first()` / `last()` / `lag()` / `lead()` transforms. Filtering on a `Column` whose `sql` contains a window function is also rejected (DEV-1369; the prior auto-promotion escape hatch from DEV-1336 is removed). For top-N use the inline `rank(<measure>) <= N` transform (or `dense_rank` / `percent_rank` / `ntile(n=<N>)`); for non-standard window expressions, factor them into an earlier stage of a multi-stage `source_queries` model.

docs/concepts/formulas.md

@@ -131,12 +131,12 @@ Functions apply window operations to measures:
 | Function | Description | SQL Generated |
 |----------|-------------|---------------|
 | `cumsum(x)` | Running total over time | `SUM(x) OVER (PARTITION BY dims ORDER BY time)` |
-| `time_shift(x, n)` | Value N periods back/ahead | Self-join CTE with INTERVAL offset |
+| `time_shift(x, n)` | Value N time buckets back/ahead (calendar-aware) | Self-join CTE with INTERVAL offset |
 | `time_shift(x, offset, gran)` | Value from a different time bucket | Self-join CTE with INTERVAL offset |
 | `lag(x, n)` | Value N rows back (window function) | `LAG(x, n) OVER (PARTITION BY dims ORDER BY time)` |
 | `lead(x, n)` | Value N rows ahead (window function) | `LEAD(x, n) OVER (PARTITION BY dims ORDER BY time)` |
-| `change(x)` | Difference from previous period | Desugars to `x - time_shift(x, -1)` |
-| `change_pct(x)` | Percentage change from previous | Desugars to `(x - ts) / ts` where `ts = time_shift(x, -1)` |
+| `change(x)` | Period-over-period difference (partition-safe, resets per group) | Desugars to `x - time_shift(x, -1)` |
+| `change_pct(x)` | Period-over-period % change, e.g. month-over-month growth (partition-safe, resets per group; NULL when the prior period's value is 0 or missing) | Desugars to `CASE WHEN ts != 0 THEN (x - ts) / ts END` where `ts = time_shift(x, -1)` |
 | `consecutive_periods(predicate)` | Current trailing run length where predicate is true | Staged window CTEs with reset groups |
 | `rank(x[, partition_by=...])` | Ranking by value (descending) | `RANK() OVER ([PARTITION BY ...] ORDER BY x DESC)` |
 | `percent_rank(x[, partition_by=...])` | Relative rank in [0, 1] (descending) | `PERCENT_RANK() OVER ([PARTITION BY ...] ORDER BY x DESC)` |
@@ -155,6 +155,14 @@ total per status, not one running total across the whole result set.
 
 `time_shift` uses a **self-join CTE** with an INTERVAL-shifted time column. `change` and `change_pct` are desugared into a hidden `time_shift` + arithmetic expression at query enrichment time. The shifted sub-query applies the time offset everywhere (WHERE, GROUP BY, SELECT), so it can reach outside the current result set — no edge NULLs when the database has the data, and correct handling of gaps in time series.
 
+The self-join matches on **all non-time dimensions as well as the shifted time column** (e.g. `ON base.month = shifted.month AND base.store = shifted.store`), so these transforms are partition-safe: each group's series is compared only against itself, and per-group series reset cleanly. One store's first month is never diffed against another store's last month.
+
+**Intent recipes:**
+
+- Month-over-month / period-over-period growth → `change_pct(revenue:sum)` with a `time_dimensions` entry at the desired granularity. Prefer this over hand-building the ratio from `time_shift`.
+- Absolute period-over-period delta → `change(revenue:sum)`.
+- Comparing against a *different* grain than the query's (e.g. year-over-year on a monthly series), or using the shifted value as a term in custom arithmetic → `time_shift(revenue:sum, -1, 'year')`.
+
 `lag(x, n)` and `lead(x, n)` use SQL `LAG`/`LEAD` window functions directly. They are more efficient but have two trade-offs:
 
 - **Edge NULLs**: the first/last N rows always return NULL since window functions can only see rows within the current result set.

docs/concepts/terminology.md

@@ -52,7 +52,7 @@ Key terms used throughout SLayer documentation and code.
 
 **Self-join vs window-function transforms:**
 
-- `time_shift`, `change`, and `change_pct` use self-join CTEs — they can reach outside the current result set (no edge NULLs) and handle gaps in data correctly. `time_shift(revenue, -1, 'year')` (with granularity) joins on calendar date arithmetic for comparisons like year-over-year.
+- `time_shift`, `change`, and `change_pct` use self-join CTEs — they can reach outside the current result set (no edge NULLs) and handle gaps in data correctly. The join matches on all non-time dimensions as well as the shifted time column, so grouped comparisons are partition-safe and reset per group. `time_shift(revenue, -1, 'year')` (with granularity) joins on calendar date arithmetic for comparisons like year-over-year.
 - `lag(revenue, 1)` / `lead(revenue, 1)` use SQL `LAG`/`LEAD` window functions directly — more efficient, but produce NULLs at the edges and are sensitive to gaps in data.
 
 **Nesting** — Formulas can be nested: `change(cumsum(revenue))` applies `change` to the result of `cumsum`. Each level of nesting generates an additional CTE layer in the SQL.

docs/examples/04_time/time_nb.ipynb

@@ -261,7 +261,7 @@
    "metadata": {
     "jp-MarkdownHeadingCollapsed": true
    },
-   "source": "### time_shift vs lag\n\nWithout a granularity argument, `time_shift(x, -1)` is equivalent to `lag(x, 1)` — it uses SQL's `LAG()` window function to look at the previous row. This is more efficient but blindly walks rows: if the result has a gap (e.g., a missing month), the previous row is the wrong period.\n\nWith a granularity argument (like `'year'` above), `time_shift` uses a calendar-based self-join: gap-safe (it matches on the actual calendar date, so a missing intermediate month produces NULL rather than the wrong value), and the shift can be any amount (not just a multiple of the query granularity). Both forms still return NULL at the start of the window when there's no prior period in the data to match."
+   "source": "### time_shift vs lag\n\n`time_shift` always uses a calendar-based self-join, with or without a granularity argument. Without one, `time_shift(x, -1)` shifts by one bucket of the query's own time granularity; with one (like `'year'` above), the shift can be any calendar amount — not just a multiple of the query granularity. Either way it's gap-safe: the previous value is matched on the actual calendar date, so a missing intermediate month produces NULL rather than the wrong value. The join also matches on all non-time dimensions, so grouped series (e.g. per-store) reset cleanly per group.\n\n`lag(x, 1)` / `lead(x, 1)` use SQL's `LAG()`/`LEAD()` window functions instead — more efficient, but they blindly walk rows: if the result has a gap (e.g., a missing month), the previous row is the wrong period. Both approaches return NULL at the start of the window when there's no prior period in the data to match."
   },
   {
    "cell_type": "markdown",