DEV-1788: unify aggregated slot-type and display-format inference

DECISIONS.md

@@ -116,3 +116,4 @@ implementation detail. Include issue refs when known.
 
 - 2026-08-11 — Deletion, sweep, docs: P-J states 2+3 executed (DEV-1749, PR 6 of 6, closing the DEV-1742 consolidation). Every mechanism PRs 1–5 + DEV-1763 left production-unreferenced (P-J state 1) is now **deleted** together with its pinning tests (state 3), after confirming the desired behaviour is pinned by tests on the new code. Removed from `generator.py`: the five legacy per-path `ValueKey` renderers and the three arithmetic composer shims (`render_value_key` / `render_arithmetic` are the sole paths, P-G); the first/last host-base ranked machinery — `_build_first_last_base_select`, `_build_ranked_subquery_from_planned`, `_has_first_last_aggregate`, the rn-suffix + filtered-rn/match maps and raw-filter-leak fallback in `_build_agg`, `FirstLastRenderState` and its `first_last_state` threading, and the production-dead `is_first_or_last` arm of `_render_cross_model_cte` (first/last is a `RankedAggregatePlan` CTE since DEV-1748, P-C); the Mode-A model-filter qualify chain (`_render_model_filter_sql`, `_qualify_mode_a_sql_filter`, `_render_mode_a_predicate`, `_filter_join_paths`, `_expand_degenerate_derived_root`, `_column_ref_is_derived`, `_predicate_references_derived`) plus the dead `FilterPhase.text_columns` field — the Mode-A door (`ScopeFrame.enter_predicate`, DEV-1745) is the one door, P-A; the four legacy ORDER BY resolvers (`_build_combined_order_by_sql`, `_resolve_combined_order_term`, `_planned_order_by_sql`, `_apply_order_limit_from_planned`) superseded by `resolve_order_term`; `_null_safe_join_pair_sql` (string round-trip superseded by `render/joins.py`, P-I); `_build_transform_sql`+`_SELF_JOIN_TRANSFORMS`, `_build_outer_wrap`+`_strip_trailing_pagination` (planned outer-wrap delegates to `SqlDialect.emit_outer_wrap`, P-H), and `_cte_name_from_alias` (superseded by `naming.cte_name_from_alias`, P-F). Removed from `cross_model_planner.py`: the formula-text re-rooting island (`_local_agg_formula`, `_render_ref_formula`, `_scalar_formula_literal`, `_reroot_ref`, `_host_ref_path`, `_REROOT_BIND_ERRORS`) and two further dead helpers (`_classify_subplan_filters`, `_filter_ref_paths`) — cross-model re-rooting is typed keys end to end (`reroot_aggregate_key` / `reroot_value_key`), no text round-trip, P-E. **B12 (ratified):** `_build_agg`'s dispatch now reads the single `AGG_REGISTRY` classification table (DEV-1744) — `_AGG_FUNCTION_MAP` (with its dead `COUNT_DISTINCT`/`MEDIAN` string values), the second inline class map, and the generator-local stat-name frozenset are gone; the two-phase resolution order (own-inner builders → shared inner + filter wrap → distinct/median/simple) is preserved byte-for-byte, SQL-identical. **Consolidations landed:** transform-op registries single-sourced (`RANK_FAMILY_TRANSFORMS`, `TIME_TRANSFORMS` from `core/formula.py`), the cube identifier regex re-pointed at `core/refs.IDENTIFIER_RE`; the `_bare_column_refs` regex kept for its remaining validation role only. **Deferred to DEV-1777** (pure refactors, no SQL change, carry regression risk needing their own byte-identity checks): step-CTE emission extraction, the throwaway-`ScopeFrame` consolidation + `_resolve_explicit_time_col` dead-branch removal, and the positional-index couplings. The remaining bare-identifier divergences (`schema_drift`, `.isidentifier()` sites) went to DEV-1771. `docs/architecture/sql-generation.md` was rewritten as the P-A – P-J principles document. Every deleted symbol's docstring/comment references were swept to present-tense truth across 16 files. No emitted SQL changed; the full non-integration suite is green (3 xfails, all ticketed — DEV-1752 / DEV-1729 / DEV-1445 — retained per the F18 inventory).
 - 2026-08-12 — Single-source bare-identifier detection narrowed to `schema_drift` only (DEV-1771). Of the three ad-hoc detectors the issue named, only `schema_drift._is_bare_identifier` was re-pointed at the canonical `core/refs.IDENTIFIER_RE` (its char-loop `all(c.isalnum() or c == "_")` accepted non-ASCII in every position; the regex rejects only a non-ASCII *leading* char, since `\w*` still matches Unicode after the ASCII-only lead class). The two `stage_planner` `.isidentifier()` sites (`_saved_model_measure_type`, `_bare_saved_measure_name`) were left as-is: they gate `get_measure(name)` and `ModelMeasure.name` is already ASCII-constrained by `_NAME_PATTERN`, so any string that could match a real measure passes both predicates identically — the flip is observationally a no-op, so no test could fail without it. The two `generator` `.isidentifier()` sites (`_resolve_sql`; the no-bundle branch reached via the `col.name` fallback) were also left as-is: `Column.name`/`Column.sql` are NOT ASCII-constrained, and routing a non-ASCII-leading physical column (e.g. Cyrillic `год`, common in RU/UA schemas) through `_parse()` drops the model-relation qualifier (`m."год"` → bare `год`) — a correctness regression on legitimate input, exactly the issue's "surface and stop" guard. The one shipped behavior change is a drift false-negative: a base column aliasing a bare non-ASCII-leading physical name (`Column(name="year", sql="год")`) reclassifies base→derived, so a dropped physical `год` is no longer flagged by `_diff_sql_table_columns` and is instead scanned as a ref by `_first_dropped_sql_column_ref` (author-accepted; advisory-only, never query correctness). `.match()` (not `fullmatch`) and the retained `.strip()` match the existing `cube`/`dbt`/`osi` reuse sites.
+- 2026-08-16 — Aggregated slot-type and display-format inference share one classifier (DEV-1788, follow-up to DEV-1784's Option A). `aggregated_type` (slot `DataType`) and `_infer_aggregated_format` (response `NumberFormat`) had disagreed on the stat/parametric family: type said `DOUBLE` while format fell through to inherit the source column's format, so `revenue:stddev_samp` was typed `DOUBLE` yet displayed as currency. Both now read a single `classify_aggregation` (`core/enums.py`) returning one of four `AggregationValueClass` buckets, and each function maps the bucket to its own output — no per-name branching survives, so the two axes cannot drift. The four builtin frozensets (`INTEGER_AGGREGATIONS`, `PRESERVING_AGGREGATIONS`, `FLOAT_SOURCE_UNIT_AGGREGATIONS`, `FLOAT_PLAIN_AGGREGATIONS`) partition `BUILTIN_AGGREGATIONS`, pinned by a completeness test; custom/model-defined aggregations hit the `PRESERVING` fallback (inherit type & format), unchanged. **Semantics chosen (Option B, unit-correct):** `avg`/`median`/`weighted_avg`/`percentile`/`stddev*` are `DOUBLE` but keep the source's UNITS, so display format inherits the source (falling back to `FLOAT` when the source has none — keeping type `DOUBLE` and format `FLOAT` coherent for unformatted measures, and confining the change to formatted ones); `corr`/`var*`/`covar*` are dimensionless/squared/product units, so they display as plain `FLOAT` regardless of source. `aggregated_type` is behaviourally unchanged (only restructured). **Net user-visible change, all in `_infer_aggregated_format`:** avg-family of a FORMATTED measure now inherits that format (was `FLOAT`); `corr`/`var*`/`covar*` now `FLOAT` (was inherit); `stddev*`/`percentile` unchanged (already inherited). Drift guard extended to the full four-bucket table and routed through the public callers (`measure_key_type` / `measure_key_format_description`), plus response-metadata assertions for the stat/parametric family.

docs/architecture/sql-generation.md

@@ -287,8 +287,11 @@ confirmed pinned.
   each public result key mapped to its `FieldMetadata(label, format)`.
   `_slot_result_keys` mirrors `_full_alias_for_slot`; only keys actually present
   in the rendered SQL are surfaced. Aggregate formats come from
-  `_infer_aggregated_format` (INTEGER for count/star, FLOAT for avg-family,
-  source-column format for sum/min/max).
+  `_infer_aggregated_format`, which shares one classifier (`classify_aggregation`,
+  DEV-1788) with `aggregated_type` (slot `DataType`) so the type and format axes
+  cannot drift: INTEGER for count/star; plain FLOAT for `corr`/`var`/`covar`;
+  the source column's format (else FLOAT) for the avg-family/`percentile`/`stddev`;
+  the source column's format (else None) for `sum`/`min`/`max`/`first`/`last`.
 
 `FieldMetadata` / `ResponseAttributes` / `_infer_aggregated_format` live here (not
 in `query_engine`) so the module imports nothing from the engine; `query_engine`

slayer/core/enums.py

@@ -3,7 +3,7 @@
 import datetime  # noqa: F401  (kept for downstream imports of TimeGranularity)
 import difflib
 from enum import Enum
-from typing import Any
+from typing import Any, Optional
 
 
 class StrEnum(str, Enum):
@@ -174,15 +174,61 @@ class JoinType(StrEnum):
     "corr", "covar_samp", "covar_pop",
 })
 
-# Aggregations whose result is always an integer count, independent of the
-# source column's type/format. Single source of truth shared by
-# ``aggregated_type`` (slot DataType) and ``_infer_aggregated_format`` (response
-# NumberFormat) so the two cannot drift on the integer bucket; a drift-guard
-# test pins both against it. The float/inherit split for stat/parametric aggs
-# legitimately differs between type and display format (see DEV-1788).
+# Aggregation value classification (DEV-1788). One classifier,
+# ``classify_aggregation``, buckets every aggregation by how its result relates
+# to the source column. Both ``aggregated_type`` (slot DataType) and
+# ``_infer_aggregated_format`` (display NumberFormat) read the bucket and map it
+# to their own output, so the type and format axes cannot drift apart. The four
+# builtin sets partition ``BUILTIN_AGGREGATIONS`` (pinned by a drift-guard test);
+# custom/model-defined aggregations hit the PRESERVING fallback.
+
+# Result is always an integer count, independent of the source column.
 INTEGER_AGGREGATIONS: frozenset[str] = frozenset({
     "count", "count_distinct", "count_distinct_approx",
 })
+# Result is a float in the SAME units as the source (display format inherited).
+FLOAT_SOURCE_UNIT_AGGREGATIONS: frozenset[str] = frozenset({
+    "avg", "weighted_avg", "median", "percentile",
+    "stddev_samp", "stddev_pop",
+})
+# Result is a float in different units (dimensionless / squared / product), so it
+# carries a plain FLOAT format, not the source's units.
+FLOAT_PLAIN_AGGREGATIONS: frozenset[str] = frozenset({
+    "corr", "var_samp", "var_pop", "covar_samp", "covar_pop",
+})
+# Result preserves the source column's type AND format.
+PRESERVING_AGGREGATIONS: frozenset[str] = frozenset({
+    "sum", "min", "max", "first", "last",
+})
+
+
+class AggregationValueClass(StrEnum):
+    """How an aggregation's result relates to its source column, for slot-type
+    and display-format inference (DEV-1788)."""
+
+    COUNT = "count"                            # INT type, INTEGER format
+    PRESERVING = "preserving"                  # source type & format
+    FLOAT_SOURCE_UNITS = "float_source_units"  # DOUBLE type, source format (else FLOAT)
+    FLOAT_PLAIN = "float_plain"                # DOUBLE type, plain FLOAT format
+
+
+def classify_aggregation(
+    *, measure_name: Optional[str], aggregation: str
+) -> AggregationValueClass:
+    """Bucket an aggregation for slot-type / display-format inference.
+
+    ``measure_name == "*"`` (``*:count``) is COUNT; custom/unknown aggregations
+    fall through to PRESERVING (inherit source type & format).
+    """
+    if measure_name == "*":
+        return AggregationValueClass.COUNT
+    if aggregation in INTEGER_AGGREGATIONS:
+        return AggregationValueClass.COUNT
+    if aggregation in FLOAT_SOURCE_UNIT_AGGREGATIONS:
+        return AggregationValueClass.FLOAT_SOURCE_UNITS
+    if aggregation in FLOAT_PLAIN_AGGREGATIONS:
+        return AggregationValueClass.FLOAT_PLAIN
+    return AggregationValueClass.PRESERVING
 
 # DEV-1576: unambiguous aggregation-name aliases that LLM agents routinely
 # emit. ``normalize_aggregation_name`` lowercases the incoming token and maps

slayer/engine/prebound.py

@@ -28,11 +28,15 @@
 
 from __future__ import annotations
 
-from typing import FrozenSet, List, Optional, Tuple
+from typing import List, Optional, Tuple
 
 from pydantic import BaseModel, ConfigDict, Field, model_validator
 
-from slayer.core.enums import DataType, INTEGER_AGGREGATIONS
+from slayer.core.enums import (
+    AggregationValueClass,
+    DataType,
+    classify_aggregation,
+)
 from slayer.core.format import NumberFormat
 from slayer.core.keys import (
     AggregateKey,
@@ -163,36 +167,31 @@ def __getattr__(self, item: str):
 # Key -> slot metadata
 # ---------------------------------------------------------------------------
 
-_FLOAT_AGGREGATIONS: FrozenSet[str] = frozenset({
-    "avg", "weighted_avg", "median",
-    "stddev_samp", "stddev_pop", "var_samp", "var_pop",
-    "corr", "covar_samp", "covar_pop", "percentile",
-})
-
-
 def aggregated_type(
     *,
     model: SlayerModel,
     measure_name: Optional[str],
     aggregation: str,
 ) -> Optional[DataType]:
-    """Type for an aggregated measure slot. Mirrors
-    ``_infer_aggregated_format`` (decision #2 of the Stage B plan):
+    """Type for an aggregated measure slot, via the shared
+    ``classify_aggregation`` (DEV-1788), so it cannot drift from
+    ``_infer_aggregated_format``:
 
-    * ``*:count`` (measure_name=``"*"``) → ``INT``
-    * ``count`` / ``count_distinct`` / ``count_distinct_approx`` → ``INT``
-    * ``avg`` / ``weighted_avg`` / ``median`` / parametric / stat aggs →
+    * ``COUNT`` (``*:count`` / count-family) → ``INT``
+    * ``FLOAT_SOURCE_UNITS`` / ``FLOAT_PLAIN`` (avg-family, stat, parametric) →
       ``DOUBLE``
-    * ``sum`` / ``min`` / ``max`` / ``first`` / ``last`` → inherit from
-      source column type (DOUBLE if absent).
+    * ``PRESERVING`` (sum / min / max / first / last, and custom aggs) → inherit
+      source column type (``None`` if absent).
     """
-    if measure_name == "*":
-        return DataType.INT
-    if aggregation in INTEGER_AGGREGATIONS:
+    cls = classify_aggregation(measure_name=measure_name, aggregation=aggregation)
+    if cls is AggregationValueClass.COUNT:
         return DataType.INT
-    if aggregation in _FLOAT_AGGREGATIONS:
+    if cls in (
+        AggregationValueClass.FLOAT_SOURCE_UNITS,
+        AggregationValueClass.FLOAT_PLAIN,
+    ):
         return DataType.DOUBLE
-    # sum / min / max / first / last — preserve source column type.
+    # PRESERVING — inherit source column type.
     if measure_name is None:
         return None
     col = model.get_column(measure_name)

slayer/engine/response_meta.py

@@ -26,7 +26,7 @@
 import sqlglot
 from pydantic import BaseModel, Field as PydanticField
 
-from slayer.core.enums import INTEGER_AGGREGATIONS
+from slayer.core.enums import AggregationValueClass, classify_aggregation
 from slayer.core.format import NumberFormat, NumberFormatType
 from slayer.core.keys import (
     AggregateKey,
@@ -73,28 +73,28 @@ def _infer_aggregated_format(
     measure_name: str,
     aggregation: str,
 ) -> Optional[NumberFormat]:
-    """Infer NumberFormat for an aggregated measure based on aggregation type and source measure format.
-
-    Rules:
-    - count, count_distinct, count_distinct_approx: always INTEGER
-    - avg, weighted_avg, median: always FLOAT
-    - sum, min, max, first, last: inherit from source measure
-    - *:count (measure_name="*"): INTEGER
+    """Infer the display NumberFormat for an aggregated measure via the shared
+    ``classify_aggregation`` (DEV-1788), so it cannot drift from
+    ``aggregated_type``:
+
+    - COUNT (``*:count`` / count-family): INTEGER
+    - FLOAT_PLAIN (corr / var / covar): plain FLOAT
+    - FLOAT_SOURCE_UNITS (avg-family / percentile / stddev): inherit source
+      format, else FLOAT (the result is fractional even absent source units)
+    - PRESERVING (sum / min / max / first / last, and custom aggs): inherit
+      source format, else None
     """
-    if measure_name == "*":
-        return NumberFormat(type=NumberFormatType.INTEGER)
-
-    if aggregation in INTEGER_AGGREGATIONS:
+    cls = classify_aggregation(measure_name=measure_name, aggregation=aggregation)
+    if cls is AggregationValueClass.COUNT:
         return NumberFormat(type=NumberFormatType.INTEGER)
-
-    if aggregation in ("avg", "weighted_avg", "median"):
+    if cls is AggregationValueClass.FLOAT_PLAIN:
         return NumberFormat(type=NumberFormatType.FLOAT)
 
-    # sum, min, max, first, last: inherit from source column's format
     source_col = model.get_column(measure_name)
     if source_col and source_col.format:
         return source_col.format
-
+    if cls is AggregationValueClass.FLOAT_SOURCE_UNITS:
+        return NumberFormat(type=NumberFormatType.FLOAT)
     return None
 
 
@@ -202,8 +202,9 @@ def _measure_format(
     """Number format for a measure slot.
 
     Aggregate slots inherit via ``_infer_aggregated_format`` (INTEGER for
-    count(-distinct) / star, FLOAT for avg-family, source-column format for
-    sum/min/max). Transform / arithmetic / scalar-call slots default to FLOAT.
+    count(-distinct) / star, plain FLOAT for corr / var / covar, source format
+    for the avg-family / percentile / stddev and for sum / min / max).
+    Transform / arithmetic / scalar-call slots default to FLOAT.
     """
     key = slot.key
     if isinstance(key, AggregateKey):