feat(data_plane): default ASAP_SUMMARY_EXECUTOR_LIVE to on

control_plane/docs/design-target-architecture.md

@@ -262,34 +262,32 @@ deployment-specific detour from it.
 | L3 | Zero local `QueryExpr`/`AggIntent`/`Schema` definitions | Already true — `intent_algebra/{agg_intent,query_expr,relational,schema,expr_ir}.rs` are thin re-export shims with only genuinely-local residues (`Frequency` extension helpers, `PerPartitionWrap`, PromQL-ergonomic `LabelFilter`). `intent_algebra/lower.rs` (~1000 lines) remains real local code — deliberately, for two documented reasons with no ASAPController equivalent (multi-agg fusion, the windowed-Count-as-Frequency heuristic). **Effectively closed modulo `lower.rs`'s two documented exceptions.** |
 | L4 | One `CostModel` impl; `Rc<L4Node>` used directly | `sketch_algebra::cost_model::ControlPlaneCostModel` + `sketch_algebra::lower::bind_query_expr` (delegating to `implement_tree_in_with`) already match this shape. `sketch_algebra::matcher::SummaryFamilyMatcher` is the `Matcher` impl this section's serving-time §3 depends on. **Effectively closed** — `PhysicalExpr`/`L4Plan` is a thin, acceptable L5-placement wrapper around `Rc<L4Node>`, not a competing L4 algebra. |
 | L5 | Full local `PhysicalPlanner`/`TopologyDescriptor`/`StageAllocator` impl | `physical/colored_dag/*` + `emit/*` already implement this shape structurally, just not against the trait names above (no literal `PhysicalPlanner` trait exists in this repo — the free functions/structs are the de facto impl). Low-priority gap: naming/trait-alignment, not missing functionality. |
-| Serving | Single `SummaryExecutor` impl is the live path | `data_plane`'s `summary_executor.rs` implements the trait fully, but is not yet the live path — `engine.rs`'s query-serving entry point still calls the legacy flat `SketchReducer`/`capability_for`-based dispatch. `live_serve.rs`/`shadow_compare.rs` exist as the rollout mechanism (both env-flag-gated, off by default). **Rollout in progress, not complete.** |
+| Serving | Single `SummaryExecutor` impl is the live path | `data_plane`'s `summary_executor.rs` implements the trait fully and is **now the default-on live path** (`ASAP_SUMMARY_EXECUTOR_LIVE` default flipped from off to on — the grouping-ambiguity blocker below is resolved via `Reduction`, and both unit + e2e tests already proved correctness for the covered shapes). `sketch_reducer.rs` remains the permanent fallback for shapes this executor self-excludes before binding (`rate()`/`irate()`, `topk(K, sum by(...)(rate(...)))`, keyed-CMS point-estimate) — **not** legacy debt pending deletion, an intentional, indefinite split. |
 
 **Net reading**: L2–L4 are substantially already at target — the earlier
 instinct that "`intent_algebra`/`sketch_algebra` should be unnecessary
 once connected to ASAPController" is correct and largely *already true*
-for L2–L4, not a still-open gap. The two real, still-open items are L1
-(adopt `asap-frontend-promql`, retiring `query_parser/` outright) and the
-serving-time cutover (finish the `SummaryExecutor` rollout, then retire
-`sketch_reducer.rs`). L5 should **not** shrink — it's this deployment's
-own, permanent responsibility per ASAPController's own "no `asap-physical`
+for L2–L4, not a still-open gap. The serving-time cutover is done for the
+shapes `SummaryExecutor` covers (default-on); the one real, still-open
+item is L1 (adopt `asap-frontend-promql`, retiring `query_parser/`
+outright). L5 should **not** shrink — it's this deployment's own,
+permanent responsibility per ASAPController's own "no `asap-physical`
 crate" status.
 
-## 5. Open questions (carried from `data_plane/docs/l4node-plan-executor-design.md`, still unresolved)
+## 5. Open questions (carried from `data_plane/docs/l4node-plan-executor-design.md`, mostly resolved)
 
-These block the serving-time cutover in §3, not the L1 adoption in §4 —
-listed here because both docs describe the same target and shouldn't
+Listed here because both docs describe the same target and shouldn't
 drift into two different pictures of what's still open:
 
-1. **Grouping ambiguity for empty, sketch-family `by`.** PR #169's
-   `l3-intent-algebra.md` interface section resolves this at the type
-   level — `Reduction::{Reduce(GroupKeys), PerEntity}` is exactly the
-   upstream IR signal this repo's design doc flagged as missing (see
-   `Aggregate.reduction`'s doc: "an implementer must branch on `Reduce`
-   vs. `PerEntity` there, not guess from an empty key list"). **This
-   should now be resolvable** — `find_candidates`'s `reduction` parameter
-   already carries the distinction; the open work is wiring
-   `data_plane`'s `find_candidates` impl to actually branch on it instead
-   of the empty-key heuristic the current draft implementation uses.
+1. **Grouping ambiguity for empty, sketch-family `by`. RESOLVED.** PR
+   #169's `l3-intent-algebra.md` interface section resolves this at the
+   type level — `Reduction::{Reduce(GroupKeys), PerEntity}` is exactly
+   the upstream IR signal this repo's design doc flagged as missing.
+   `data_plane`'s `find_candidates` implementation (`summary_executor.rs`'s
+   `resolve_group_key`) already branches on the real `Reduction` value,
+   not an empty-key heuristic — confirmed by reading the current code,
+   not assumed. This was the stated blocker for the serving-time cutover
+   in §3/Part A; it's why that cutover was safe to default-on already.
 2. **Outer-fold family of gaps** (`topk(K, sum by (...) (rate(m[r])))`,
    stacking an outer exact statistic on a sketch/exact-agg readout) —
    still open, still a cross-repo IR design question per the original doc.

data_plane/src/query_engines/asap_query_engine/live_serve.rs

@@ -21,16 +21,30 @@ const LIVE_ACCURACY: AccuracyTarget = AccuracyTarget::Epsilon(0.01);
 
 /// Whether the actual serving cutover is enabled for this process.
 /// Mirrors `shadow_compare::shadow_summary_executor_enabled`'s exact
-/// mechanics, own flag, own default (off) — this is a materially
-/// riskier switch than shadow mode (it changes what's served, not just
-/// what's logged), so it must never be implied by the shadow flag.
+/// mechanics and own flag — this is a materially riskier switch than
+/// shadow mode (it changes what's served, not just what's logged), so
+/// it must never be implied by the shadow flag.
+///
+/// Default flipped to **on** (control_plane/docs/design-target-architecture.md
+/// §4/Part A): both unit tests and a real HTTP-level e2e test
+/// (`live_serve_hll_global_count_merges_across_sids`,
+/// `live_serve_actually_answers_ddsketch_quantile`) already prove
+/// correctness for the shapes `try_serve_from_summary_executor` covers,
+/// and the grouping-ambiguity problem that gated this default off
+/// (empty-`by` ambiguity) is resolved via the real `Reduction::{Reduce,
+/// PerEntity}` IR signal, not a heuristic. The env var stays as a kill
+/// switch (`ASAP_SUMMARY_EXECUTOR_LIVE=0`/`false`/`off`), not removed —
+/// shapes this executor self-excludes before binding (`rate()`/`irate()`,
+/// `topk(K, sum by(...)(rate(...)))`, keyed-CMS point-estimate) still
+/// fall through to the legacy `SketchReducer` path unconditionally,
+/// regardless of this flag.
 pub fn summary_executor_live_enabled() -> bool {
     std::env::var("ASAP_SUMMARY_EXECUTOR_LIVE")
         .map(|v| {
             let v = v.trim();
-            v == "1" || v.eq_ignore_ascii_case("true") || v.eq_ignore_ascii_case("on")
+            !(v == "0" || v.eq_ignore_ascii_case("false") || v.eq_ignore_ascii_case("off"))
         })
-        .unwrap_or(false)
+        .unwrap_or(true)
 }
 
 /// Try to serve `query` entirely from `SummaryExecutor`. Returns `None`
@@ -195,7 +209,26 @@ mod tests {
     }
 
     #[test]
-    fn flag_off_never_serves() {
+    fn flag_explicitly_off_never_serves() {
+        // Kill switch: an explicit off-spelling still disables live-serve
+        // even though the default (unset) is now on.
+        let _guard = set_live_env("0");
+        let idx = ddsketch_fixture();
+        let result = try_serve_from_summary_executor(
+            &idx,
+            "quantile_over_time(0.99, latency_ms[1m])",
+            1_000,
+            2_000,
+            true,
+        );
+        assert!(result.is_none(), "flag explicitly off must never serve");
+    }
+
+    #[test]
+    fn unset_flag_defaults_to_serving() {
+        // Default flipped to on (design-target-architecture.md §4/Part A)
+        // -- an unset env var must serve, not fall back to the legacy
+        // path, for a shape this executor already proves safe.
         let _guard = clear_live_env();
         let idx = ddsketch_fixture();
         let result = try_serve_from_summary_executor(
@@ -205,7 +238,7 @@ mod tests {
             2_000,
             true,
         );
-        assert!(result.is_none(), "flag off must never serve");
+        assert!(result.is_some(), "unset flag must default to serving");
     }
 
     #[test]