DEV-1714 Stage 10: windowed measures (_wm_ range-join) rebuilt on ScopeFrame

[ZmeiGorynych]

Reimplements duration-windowed measures (revenue:sum(window='90d')) on the typed planner/generator as a plan-time WindowedAggregatePlan + host-rooted _wm_ range-join CTE (a ScopeFrame client), closing DEV-1496. Stage 10 of the DEV-1703 typed-pipeline redesign.

What changed

• slayer/core/window_duration.py (new) — compact-duration parser, dependency-free so the engine planner validates durations at plan time without importing the SQL layer.
• slayer/engine/planned.py — WindowedAggregatePlan + PlannedQuery.windowed_aggregate_plans.
• slayer/engine/stage_planner.py — window detection, 8 plan-time guards (precedence G1→G8→G3→G4→G5→G7→G6→G2 on the original value-key trees so transform/composite win over hidden), plan construction, and Phase.POST filter reclassification.
• slayer/sql/generator.py — _render_window_measure_cte_from_planned builds the _src self-join subquery (_w_dim_<n> / _w_td_<n> / _w_time / _w_value, CASE-wrapped by Column.filter) with Law-1 join discovery, range-joins to _base (_src._w_time >= bucket_end − window / < bucket_end) via the DEV-1716 dialect strategy, and joins back null-safe on the grain. Wired into _render_with_cross_model_plans so windowed + cross-model measures coexist.

Semantics

• Scope: sum/avg local measures only. Cross-model, transform (input or sibling), arithmetic/composite, hidden filter-only, mixed windowed+plain filters, and non-sum/avg all raise loudly at plan time (DEV-1504 shapes stay guarded, never silently degrade).
• _src row filters: model + WHERE-phase filters apply inside _src; only the typed date_range is stripped (the trailing window must reach rows before the range start). An explicit raw-time-column filter still truncates — documented inconsistency tracked as DEV-1732.
• CAST follows the base path (inferred slot type). NULL-dim groups get a NULL windowed value (F1-style documented — plain = inside the CTE never matches NULL).
• window is a globally reserved aggregation kwarg name (legacy parity).

Tests

• Promotes ~28 pinned strict-xfails (test_sql_generator.py TestFields / TestMultiDialectGeneration / TestCastEmissionNonBasePaths / TestWindowedMeasureGuards, test_carrier_scope_matrix.py) to plain passing tests.
• New: shape-gap + parity pins, loud-error + plan-time-guard matrix (plan_query-level, proving guards fire before rendering), and DuckDB + SQLite integration value tests asserting hand-computed rolling values ({Jan:300, Feb:600, Mar:900}, avg Mar:300, NULL-dim → NULL, March-only date_range still returning 900).
• Full non-integration suite green (8983 passed, 0 failed); ruff clean. Windowed integration tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added rolling windowed sum and average measures with duration-based windows.
  • Supports multiple windows, date-range expansion, filtering, ordering, aliases, and NULL-safe dimension matching.
  • Works across DuckDB and SQLite with dialect-specific interval handling.

• Bug Fixes

  • Added clear planning-time validation for malformed or unsupported window configurations.

• Documentation

  • Documented windowed measure requirements, filtering behavior, supported configurations, and generated query structure.

[linear]

DEV-1714

DEV-1703

[coderabbitai]

📝 Walkthrough

Walkthrough

This change adds typed planning and SQL generation for duration-windowed sum and avg measures. It validates window definitions during planning, renders host-rooted range-join CTEs, preserves filter semantics, supports null-safe grain joins, and adds cross-dialect integration and validation coverage.

Changes

Duration-windowed measures

Layer / File(s)	Summary
Window duration and plan contracts slayer/core/window_duration.py, slayer/engine/planned.py	Adds compact duration parsing and the WindowedAggregatePlan model. PlannedQuery now stores windowed aggregate plans.
Windowed aggregate planning slayer/engine/stage_planner.py	Validates supported windows, resolves time and grain dimensions, classifies filters, and attaches window plans to planned queries.
Window CTE rendering and join-back slayer/sql/generator.py	Renders _wm_ CTEs with scoped joins, interval predicates, aggregation, casts, null-safe grain joins, and combined-level filters.
Validation and documentation tests/integration/test_integration_windowed_measures.py, tests/test_sql_generator.py, tests/test_carrier_scope_matrix.py, docs/architecture/sql-generation.md, docs/concepts/formulas.md, DECISIONS.md	Promotes window tests, adds DuckDB and SQLite integration coverage, validates planning guards and dialect output, and documents the supported behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Query
  participant Planner
  participant SQLGenerator
  participant WindowCTE
  participant CombinedQuery
  Query->>Planner: plan duration-windowed measure
  Planner->>Planner: validate duration, aggregation, scope, and filters
  Planner->>SQLGenerator: provide WindowedAggregatePlan
  SQLGenerator->>WindowCTE: render host-rooted range-join CTE
  WindowCTE->>CombinedQuery: return aggregate and grain aliases
  CombinedQuery->>CombinedQuery: apply POST filters and null-safe joins

Loading

Possibly related PRs

• MotleyAI/slayer#264: Provides the assert_scope_closed validation used by the windowed-measure SQL tests.
• MotleyAI/slayer#265: Introduces ScopeFrame-based join and reference resolution used by window planning and rendering.
• MotleyAI/slayer#267: Provides scoped join discovery and null-safe grain join-back mechanisms used by windowed CTEs.

Suggested reviewers: aivanf

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the Stage 10 implementation of windowed measures with _wm_ range-join CTEs and ScopeFrame.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch egor/dev-1714-dev-1703-stage-10-windowed-measures-_wm_-range-join-rebuilt

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (2)

slayer/sql/generator.py (1)

6428-6438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Carry wm_ctes into the transform-chain prelude for future safety.

The transform branch returns _render_cross_model_transform_chain(prelude_ctes=[("_base", base_cte_sql)] + cm_ctes, ...) and omits wm_ctes, while combined_select_sql already projects and joins each _wm_* CTE. The result would be a statement that references undefined CTEs.

The path is unreachable today, because guard G4 in _guard_windowed_measures rejects any transform that coexists with a windowed measure, so transform_layers is empty whenever windowed_aggregate_plans is non-empty. When DEV-1504 lifts G4, this becomes a live defect. Include the list now, or raise an explicit NotImplementedError here so the combination cannot fail silently.

♻️ Proposed change

         if planned_query.transform_layers:
+            if wm_ctes:
+                raise NotImplementedError(
+                    "DEV-1714 Stage 10: a windowed measure combined with a "
+                    "transform layer is guarded at plan time (G4); the "
+                    "transform chain does not carry `_wm_` CTEs.",
+                )
             return self._render_cross_model_transform_chain(
                 prelude_ctes=[("_base", base_cte_sql)] + cm_ctes,

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/sql/generator.py` around lines 6428 - 6438, Update the transform
branch in the surrounding query-rendering method to include wm_ctes in
prelude_ctes before calling _render_cross_model_transform_chain, preserving the
existing CTE order and ensuring every _wm_* reference in combined_select_sql is
defined.

slayer/engine/stage_planner.py (1)

275-279: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align the stated guard precedence with the execution order.

The docstring states Precedence: G1 → G8 → G3 → G4 → G5 → G7 → G6 → G2 and calls it a contract. The function runs G4 first (line 287), then G5 (line 304), then G1/G8/G3 (line 312), then G7/G6, then G2. For cumsum(revenue:min(window='90d')) the raised error is the G4 transform message, not the G1 non-sum/avg message, so the listed order does not describe observable behavior. test_windowed_transform_input_precedence_not_selected pins G4 over G6, which matches the code, not the list.

Restate the line as the execution order (G4 → G5 → G1 → G8 → G3 → G7 → G6 → G2) so a later reader does not reorder the checks to match the comment.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/engine/stage_planner.py` around lines 275 - 279, Update the guard
precedence statement in the stage planner docstring to match the actual
execution order: G4 → G5 → G1 → G8 → G3 → G7 → G6 → G2. Leave the guard
implementation and surrounding explanation unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/architecture/sql-generation.md`:
- Around line 103-104: In the documentation text around the host-row column
description, keep the `_w_td_<n>` identifier entirely on one line within a
single code span, matching the unbroken `_w_dim_<n>` formatting; change only
this line break.

In `@docs/concepts/formulas.md`:
- Around line 73-75: Revise the formula documentation clause describing windowed
measures so mixed filters that reference both a windowed measure and a plain
aggregate are restricted regardless of whether the windowed measure is selected.
Keep filter-only use as a separate case, since it is already documented by the
preceding clause and is governed independently by _guard_windowed_measures guard
G7.

In `@slayer/engine/planned.py`:
- Around line 278-289: Update the windowed-plan construction flow around
`_build_windowed_plans` and `WindowedAggregatePlan` so `active_td_slot_id` is
never passed as `None`. After `projection.registry.find_by_key(active_td_key)`,
explicitly handle a missing slot by either interning the active time dimension
as a hidden slot or raising the existing G2 validation error message; preserve
the required `window_time_dimension_slot_id: SlotId` contract.

In `@slayer/engine/stage_planner.py`:
- Around line 382-399: The windowed-plan construction loop must fail loudly when
registry.find_by_key(key) returns None instead of continuing. Update the lookup
branch in the function building WindowedAggregatePlan instances to raise an
appropriate error for the missing slot, preserving plan creation only for
successful lookups and preventing the measure from falling back to a plain
aggregate.

---

Nitpick comments:
In `@slayer/engine/stage_planner.py`:
- Around line 275-279: Update the guard precedence statement in the stage
planner docstring to match the actual execution order: G4 → G5 → G1 → G8 → G3 →
G7 → G6 → G2. Leave the guard implementation and surrounding explanation
unchanged.

In `@slayer/sql/generator.py`:
- Around line 6428-6438: Update the transform branch in the surrounding
query-rendering method to include wm_ctes in prelude_ctes before calling
_render_cross_model_transform_chain, preserving the existing CTE order and
ensuring every _wm_* reference in combined_select_sql is defined.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 97664657-f69e-48dc-9c81-ec971998ed3d

📥 Commits

Reviewing files that changed from the base of the PR and between 385d377 and fb30eae.

📒 Files selected for processing (10)

• DECISIONS.md
• docs/architecture/sql-generation.md
• docs/concepts/formulas.md
• slayer/core/window_duration.py
• slayer/engine/planned.py
• slayer/engine/stage_planner.py
• slayer/sql/generator.py
• tests/integration/test_integration_windowed_measures.py
• tests/test_carrier_scope_matrix.py
• tests/test_sql_generator.py

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.8% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

🧹 Nitpick comments (3)

slayer/engine/stage_planner.py (2)

1171-1178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assign a per-plan copy of src_where_ids.

Every plan receives the same list object. Any later in-place mutation of one plan's where_filter_ids would change all plans. Pass a copy per plan to keep the plans independent.

♻️ Proposed refactor

         for wp in windowed_plans:
-            wp.where_filter_ids = src_where_ids
+            wp.where_filter_ids = list(src_where_ids)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/engine/stage_planner.py` around lines 1171 - 1178, Update the
windowed-plans loop around windowed_plans and where_filter_ids so each plan
receives an independent copy of src_where_ids rather than the shared list
object. Preserve the existing filter selection and assignment behavior.

---

393-404: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the shared G2 message into one constant.

This block repeats the exact G2 error text from lines 369-373. The two copies must stay identical, because tests match on the phrase could not resolve its time dimension. A single module-level constant removes the drift risk.

♻️ Proposed refactor

+_WINDOWED_NO_TIME_DIM_MSG = (
+    "Windowed measure could not resolve its time dimension. Add a single "
+    "time_dimensions entry, or set main_time_dimension to select among "
+    "multiple time dimensions."
+)

Then use it at both raise sites:

     if active_td_slot_id is None:
-        raise ValueError(
-            "Windowed measure could not resolve its time dimension. Add a single "
-            "time_dimensions entry, or set main_time_dimension to select among "
-            "multiple time dimensions."
-        )
+        raise ValueError(_WINDOWED_NO_TIME_DIM_MSG)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/engine/stage_planner.py` around lines 393 - 404, Extract the repeated
G2 error text into a single module-level constant in stage_planner.py, then
update both raise sites—including the active_td_slot_id check and the earlier G2
validation—to raise ValueError using that constant. Keep the message exactly
unchanged so existing tests continue matching it.

slayer/sql/generator.py (1)

6466-6488: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared grain join-back emission.

This loop repeats the cross-model join-back loop at lines 6439-6464. Both build the same null-safe LEFT JOIN … ON … string, or fall back to CROSS JOIN. One helper that takes the CTE name and the join-back pairs removes the duplicate logic, so a future change to the join-back shape applies to both paths.

♻️ Proposed refactor

+    def _grain_joinback_sql(
+        self, *, cte_name: str, joinback_pairs: List[Tuple[str, str]],
+    ) -> str:
+        """One grain join-back clause: null-safe LEFT JOIN, or CROSS JOIN when
+        the CTE shares no grain with ``_base``."""
+        if not joinback_pairs:
+            return f"\nCROSS JOIN {cte_name}"
+        join_parts = [
+            self._null_safe_join_pair_sql(
+                left_sql=f'_base.{self._quote_ident(host)}',
+                right_sql=f'{cte_name}.{self._quote_ident(cte_col)}',
+            )
+            for host, cte_col in joinback_pairs
+        ]
+        return f"\nLEFT JOIN {cte_name} ON " + _SQL_AND_JOINER.join(join_parts)

Then both loops reduce to one call each:

         for plan in planned_query.windowed_aggregate_plans:
             cte_name = wm_cte_name_for_plan[plan.aggregate_slot_id]
-            joinback_pairs = wm_joinback_pairs_for_plan.get(
-                plan.aggregate_slot_id, [],
-            )
-            if joinback_pairs:
-                join_parts = [
-                    self._null_safe_join_pair_sql(
-                        left_sql=f'_base.{self._quote_ident(host)}',
-                        right_sql=f'{cte_name}.{self._quote_ident(cte_col)}',
-                    )
-                    for host, cte_col in joinback_pairs
-                ]
-                from_clause_str += (
-                    f"\nLEFT JOIN {cte_name} ON " + _SQL_AND_JOINER.join(join_parts)
-                )
-            else:
-                from_clause_str += f"\nCROSS JOIN {cte_name}"
+            from_clause_str += self._grain_joinback_sql(
+                cte_name=cte_name,
+                joinback_pairs=wm_joinback_pairs_for_plan.get(
+                    plan.aggregate_slot_id, [],
+                ),
+            )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@slayer/sql/generator.py` around lines 6466 - 6488, Extract the repeated
join-back SQL construction into a shared helper near the existing generator
logic, accepting a CTE name and its join-back pairs and returning either the
null-safe LEFT JOIN clause or CROSS JOIN fallback. Update both the cross-model
loop and the windowed-aggregate loop to call this helper, preserving their
existing join inputs and generated SQL.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@slayer/engine/stage_planner.py`:
- Around line 1171-1178: Update the windowed-plans loop around windowed_plans
and where_filter_ids so each plan receives an independent copy of src_where_ids
rather than the shared list object. Preserve the existing filter selection and
assignment behavior.
- Around line 393-404: Extract the repeated G2 error text into a single
module-level constant in stage_planner.py, then update both raise
sites—including the active_td_slot_id check and the earlier G2 validation—to
raise ValueError using that constant. Keep the message exactly unchanged so
existing tests continue matching it.

In `@slayer/sql/generator.py`:
- Around line 6466-6488: Extract the repeated join-back SQL construction into a
shared helper near the existing generator logic, accepting a CTE name and its
join-back pairs and returning either the null-safe LEFT JOIN clause or CROSS
JOIN fallback. Update both the cross-model loop and the windowed-aggregate loop
to call this helper, preserving their existing join inputs and generated SQL.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 60a9a945-c702-4ec2-b684-b6d2fcaaff49

📥 Commits

Reviewing files that changed from the base of the PR and between fb30eae and f8afe91.

📒 Files selected for processing (6)

• DECISIONS.md
• docs/architecture/sql-generation.md
• docs/concepts/formulas.md
• slayer/engine/stage_planner.py
• slayer/sql/generator.py
• tests/test_sql_generator.py

🚧 Files skipped from review as they are similar to previous changes (4)

• docs/concepts/formulas.md
• DECISIONS.md
• docs/architecture/sql-generation.md
• tests/test_sql_generator.py