chore(sketch_algebra): re-layer three cost/planning modules out of L4 IR - #398
Merged
Merged
Conversation
Stage 4 of the sketch_algebra re-layering, scoped down after a fresh audit found two of the originally-planned four items were based on a stale premise (sketch_params.rs has 14 real production consumers, not "one"; PhysicalExpr::RawAtEdgeSketchAtBackend/RawAtEdgePrometheusArchive are actively constructed across optimizer/rules, emit/*, and physical/colored_dag/*, not dead) — both dropped from this stage pending a separate, properly-scoped investigation. Three confirmed-safe moves: - Deleted sketch_algebra/schema.rs. SketchStateSchema/SketchStateMetadata had zero real callers — every hit was a stale doc-comment cross-reference, never constructed or consumed anywhere. - Moved SketchCapability/SupportedIntent/default_capability_table/ load_capability_overrides from sketch_algebra::capability into optimizer::cost::sketch_capability. This is the perf/cost-model half of the "four overlapping capability tables" Step 2a originally consolidated — read by the optimizer and physical planner for cost-based plan rewriting, not L4 IR. sketch_algebra::capability keeps the query-side Capability/SketchKindHandle tag and the capability_for semantic bridge, which are genuinely L4-adjacent. - Moved sketch_selection.rs to a top-level control_plane::sketch_selection module. Its one external caller is query_planning.rs; naming the concrete sketch families that satisfy a Capability is a query-planning concern, not L4 IR. No behavior change — all three moves are verbatim relocations with updated doc comments and import paths. - cargo build --workspace: clean - cargo test -p control_plane: 824 passed, 1 pre-existing unrelated failure (invalid_sketch_type_override_falls_back_to_default) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stage 4 of the
sketch_algebrare-layering (Stage 1: ASAPController#141, Stage 2: #396, Stage 3: #397) — scoped down after a fresh audit found two of the originally-planned four items rested on a stale premise:sketch_params.rshas 14 real production consumers (optimizer/cost/wire.rs,optimizer/engine.rs,optimizer/rules/mod.rs,replan.rs, all ofemit/*.rs, all ofphysical/colored_dag/*.rs), not "one real consumer" as the design doc claimed.PhysicalExpr::RawAtEdgeSketchAtBackend/RawAtEdgePrometheusArchiveare actively constructed and matched acrossoptimizer/rules/mod.rs,emit/{mod,telegraf,otap,stage_config}.rs,physical/colored_dag/{emitter,allocator}.rs, and referenced fromdata_plane/asap_types— not dead code. The "duplicate deadBindMode" claim was also wrong:optimizer::cost::wire::BindModeis a single, real, heavily-tested enum that deliberately mirrorsPhysicalExpr's variant names.Both dropped from this stage; a proper investigation into replacing
sketch_params.rswithasap_sketch::SummaryKind/SummaryParamsis separate future work given its real blast radius.Three items whose premises held up:
sketch_algebra/schema.rs.SketchStateSchema/SketchStateMetadatahad zero real callers — every hit was a stale doc-comment cross-reference.SketchCapability/SupportedIntent/default_capability_table/load_capability_overridesfromsketch_algebra::capabilityintooptimizer::cost::sketch_capability— this is the perf/cost-model half of the "four overlapping capability tables" Step 2a originally consolidated, read by the optimizer/physical planner, not L4 IR.sketch_algebra::capabilitykeeps the query-sideCapability/SketchKindHandletag andcapability_for, which are genuinely L4-adjacent.sketch_selection.rsto a top-levelcontrol_plane::sketch_selectionmodule — its one external caller isquery_planning.rs; naming candidate sketch families for aCapabilityis a query-planning concern, not L4 IR.No behavior change — all three moves are verbatim relocations with updated doc comments and import paths.
Test plan
cargo build --workspace— cleancargo test -p control_plane— 824 passed, 1 pre-existing unrelated failure (invalid_sketch_type_override_falls_back_to_default)🤖 Generated with Claude Code