retire(data_plane): remove shadow_compare.rs and sketch_reducer.rs

control_plane/docs/design-target-architecture.md

@@ -242,16 +242,18 @@ and readout, through the five trait methods:
   `SummaryExpr::Logical` escape hatch) — this deployment's fallback path
   for anything that reaches serving time without a summary decision.
 
-**Target: `storage_engines/sketch_db/query/sketch_reducer.rs` is retired
-once `SummaryExecutor` reaches parity.** Not before — per this repo's own
-rollout doc, cutover requires confidence data from a shadow-mode
-comparison period (compute both, log discrepancies, serve the legacy
-answer) before the legacy path can be deleted; the doc's own
-"Rollout design" section already specifies this precisely. This doc
-doesn't relitigate that plan — it just confirms the plan's target state
-(a single `SummaryExecutor`-driven serving path) is exactly what
-ASAPController's own interface is designed to make possible, not a
-deployment-specific detour from it.
+**`storage_engines/sketch_db/query/sketch_reducer.rs` and
+`asap_query_engine/shadow_compare.rs` are retired.** Neither was any more
+"ground truth" than `SummaryExecutor` itself — `shadow_compare.rs`'s job
+(validate `SummaryExecutor` against the legacy reducer before cutover)
+was done once the cutover landed (Part A, #427), and keeping the legacy
+reducer around after that only meant two independently-planned answering
+mechanisms could silently disagree with each other, not that either was
+more trustworthy. Shapes `SummaryExecutor` self-excludes before binding
+(`rate()`/`irate()`, `topk(K, sum by(...)(rate(...)))`, keyed-CMS
+point-estimate, and the outer-exact/summary composition gaps below) now
+fail over to archive directly — there is no legacy fallback left, by
+design, not because a rollout step is still pending.
 
 ## 4. What this means for current code — gap against this target
 
@@ -262,19 +264,22 @@ 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 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. |
+| Serving | Single `SummaryExecutor` impl is the live path | **Closed.** `data_plane`'s `summary_executor.rs` implements the trait fully and is the default-on, *sole* live path (`ASAP_SUMMARY_EXECUTOR_LIVE` default flipped from off to on, #427; the legacy `sketch_reducer.rs` and diagnostic `shadow_compare.rs` are both retired). Shapes it self-excludes before binding (`rate()`/`irate()`, `topk(K, sum by(...)(rate(...)))`, keyed-CMS point-estimate) and shapes it structurally can't realize yet (composed exact/summary aggregation in either nesting order — [ASAPController#171](https://github.com/ProjectASAP/ASAPController/issues/171), e.g. `max/avg by (zone) (quantile_over_time(...))`) fail over to archive directly, with no local workaround. |
 
 **Net reading**: L1–L4 and the serving-time cutover are all now 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; L1 has since closed the same way
-(#428), and the serving-time cutover is done for the shapes
-`SummaryExecutor` covers (default-on, #427). `sketch_reducer.rs` is not
-pending deletion — it's the permanent, intentional fallback for shapes
-`SummaryExecutor` self-excludes before binding. L5 should **not** shrink
-— it's this deployment's own, permanent responsibility per
-ASAPController's own "no `asap-physical` crate" status; its only
-remaining gap is the low-priority naming/trait-alignment noted above.
+(#428), and the serving-time cutover is fully done (#427) — `sketch_reducer.rs`
+and `shadow_compare.rs` are both retired, not just superseded. The
+remaining serving-time gaps (rate/topk-over-rate/keyed-CMS,
+ASAPController#171's composed exact/summary shapes) are genuine upstream
+L4 limitations tracked in ASAPController, not something this repo routes
+around locally — same category as the already-tracked `TopK { accuracy:
+Exact }` gap (ASAPController#151). L5 should **not** shrink — it's this
+deployment's own, permanent responsibility per ASAPController's own "no
+`asap-physical` crate" status; its only remaining gap is the low-priority
+naming/trait-alignment noted above.
 
 ## 5. Open questions (carried from `data_plane/docs/l4node-plan-executor-design.md`, mostly resolved)
 

control_plane/src/sketch_algebra/cost_model.rs

@@ -380,6 +380,77 @@ impl CostModel for ForcedFamilyCostModel {
     }
 }
 
+/// A `CostModel` that forces both the family AND the exact parameters
+/// for whichever intent it's asked to rank/size, falling back to an
+/// inner accuracy-driven [`ControlPlaneCostModel`] when nothing was
+/// observed for the candidates on offer.
+///
+/// This is the seam `data_plane`'s live-serving re-binding path
+/// (`l4_lowering.rs`) needs: planning already decided a family + params
+/// for a metric (that decision is what's actually registered in the
+/// `SketchStore`), so serving-time re-parsing the same query must
+/// reproduce EXACTLY that plan, not size a fresh one from a guessed
+/// accuracy target (`ForcedFamilyCostModel` above forces the family but
+/// still re-derives params from `eps`/`delta` — the wrong tool here,
+/// since re-deriving is exactly what caused the mismatch this type
+/// exists to avoid; see `control_plane/docs/design-target-architecture.md`'s
+/// "planning vs serving" split). `observed` is `None` whenever this
+/// query's metric has no registered sid at all — `rank_candidates`/
+/// `size_params` then fall back to the accuracy-driven default, which
+/// won't match anything registered either way, so the outcome
+/// (`find_candidates` finds nothing) is unchanged.
+pub struct ObservedFamilyCostModel {
+    inner: ControlPlaneCostModel,
+    observed: Option<(SummaryKind, SummaryParams)>,
+}
+
+impl ObservedFamilyCostModel {
+    pub fn new(
+        workload_accuracy: AccuracyTarget,
+        observed: Option<(SummaryKind, SummaryParams)>,
+    ) -> Self {
+        Self {
+            inner: ControlPlaneCostModel::new(workload_accuracy),
+            observed,
+        }
+    }
+}
+
+impl CostModel for ObservedFamilyCostModel {
+    fn rank_candidates(&self, intent: &AggIntent, candidates: &[SummaryKind]) -> Vec<SummaryKind> {
+        match &self.observed {
+            Some((kind, _)) if candidates.contains(kind) => vec![kind.clone()],
+            _ => self.inner.rank_candidates(intent, candidates),
+        }
+    }
+
+    fn size_params(
+        &self,
+        kind: SummaryKind,
+        intent: &AggIntent,
+        eps: f64,
+        delta: f64,
+    ) -> SummaryParams {
+        match &self.observed {
+            Some((okind, oparams)) if *okind == kind => oparams.clone(),
+            _ => self.inner.size_params(kind, intent, eps, delta),
+        }
+    }
+
+    fn realize_extension(&self, ext_kind: &str, payload: &serde_json::Value) -> Implementation {
+        self.inner.realize_extension(ext_kind, payload)
+    }
+
+    fn readout_extension(
+        &self,
+        ext_kind: &str,
+        payload: &serde_json::Value,
+        col: &ColumnRef,
+    ) -> SketchQuery {
+        self.inner.readout_extension(ext_kind, payload, col)
+    }
+}
+
 /// Map an ε rank-error budget to a KLL stream-size `k`. Verbatim port of
 /// `bind_kll_quantile.rs::kll_k_for_eps` — power-of-two rungs (200, 400,
 /// 800, 2048, 8192) so the in-tree `algebra::directory` continues to

control_plane/src/sketch_algebra/lower.rs

@@ -34,6 +34,7 @@
 use std::rc::Rc;
 
 use asap_plan::bind::implement_tree_in_with;
+use asap_plan::cost_model::CostModel;
 use thiserror::Error;
 
 use crate::intent_algebra::{AggIntent, BindingScope, QueryExpr};
@@ -51,22 +52,43 @@ pub enum BindingError {
 }
 
 /// Lower an L3 `QueryExpr` to L4/L5 under the supplied workload-level
-/// accuracy target. The result is always [`PhysicalExpr::Committed`] —
-/// this walk never picks a Phase ε.1 backend/archive placement; that's a
-/// separate, later L5 decision (`optimizer::cost::wire`).
+/// accuracy target, via [`ControlPlaneCostModel`] (this deployment's
+/// planning-time family/sizing preferences). The result is always
+/// [`PhysicalExpr::Committed`] — this walk never picks a Phase ε.1
+/// backend/archive placement; that's a separate, later L5 decision
+/// (`optimizer::cost::wire`).
 pub fn bind_query_expr(
     expr: &QueryExpr,
     accuracy: AccuracyTarget,
 ) -> Result<PhysicalExpr, BindingError> {
-    Ok(PhysicalExpr::Committed(bind_recursive(expr, &accuracy)?))
+    let cost_model = ControlPlaneCostModel::new(accuracy);
+    bind_query_expr_with_cost_model(expr, &cost_model)
 }
 
-fn bind_recursive(expr: &QueryExpr, accuracy: &AccuracyTarget) -> Result<L4Plan, BindingError> {
+/// Like [`bind_query_expr`], but with an explicitly supplied [`CostModel`]
+/// instead of the default planning-time [`ControlPlaneCostModel`].
+///
+/// This is the seam serving-time re-binding needs: `data_plane`'s
+/// live-serving path (`l4_lowering.rs`) must NOT re-derive a family/params
+/// choice independently of what was actually planned — it looks up what's
+/// really registered in the `SketchStore` and hands in a cost model that
+/// echoes that back, so the resulting `L4Node` matches reality by
+/// construction rather than by a coincidental accuracy-target match. See
+/// `control_plane/docs/design-target-architecture.md`'s "planning vs
+/// serving" split.
+pub fn bind_query_expr_with_cost_model(
+    expr: &QueryExpr,
+    cost_model: &dyn CostModel,
+) -> Result<PhysicalExpr, BindingError> {
+    Ok(PhysicalExpr::Committed(bind_recursive(expr, cost_model)?))
+}
+
+fn bind_recursive(expr: &QueryExpr, cost_model: &dyn CostModel) -> Result<L4Plan, BindingError> {
     match expr {
         QueryExpr::LetBinding { name, expr, child } => Ok(L4Plan::LetBinding {
             name: BindingName::new(name.as_str()),
-            expr: Rc::new(bind_recursive(expr, accuracy)?),
-            child: Rc::new(bind_recursive(child, accuracy)?),
+            expr: Rc::new(bind_recursive(expr, cost_model)?),
+            child: Rc::new(bind_recursive(child, cost_model)?),
         }),
         QueryExpr::Ref { name } => Ok(L4Plan::Ref {
             name: BindingName::new(name.as_str()),
@@ -108,7 +130,7 @@ fn bind_recursive(expr: &QueryExpr, accuracy: &AccuracyTarget) -> Result<L4Plan,
                     child: agg_child.clone(),
                 }),
             };
-            bind_recursive(&pushed, accuracy)
+            bind_recursive(&pushed, cost_model)
         }
 
         // `AggIntent::Count { accuracy: Exact }` — `boundary::implementation_for`'s
@@ -139,8 +161,7 @@ fn bind_recursive(expr: &QueryExpr, accuracy: &AccuracyTarget) -> Result<L4Plan,
 
         _ => {
             let rewritten = rewrite_rate_to_increase(expr);
-            let cost_model = ControlPlaneCostModel::new(accuracy.clone());
-            let node = implement_tree_in_with(&rewritten, &BindingScope::default(), &cost_model)?;
+            let node = implement_tree_in_with(&rewritten, &BindingScope::default(), cost_model)?;
             Ok(L4Plan::Summary(node))
         }
     }

control_plane/src/sketch_algebra/mod.rs

@@ -44,6 +44,6 @@ pub use capability::{capability_for, Capability, SketchKindHandle};
 pub use capability_matching::{
     classify_demo_metric, is_valid_pair, pick_family, AccuracyPreference, StatisticClass,
 };
-pub use lower::{bind_query_expr, BindingError};
+pub use lower::{bind_query_expr, bind_query_expr_with_cost_model, BindingError};
 pub use matcher::SummaryFamilyMatcher;
 pub use physical_expr::{L4Plan, PhysicalExpr};