Skip to content

refactor(planner): Stage 1 no longer depends on the cost model - #582

Draft
zzylol wants to merge 4 commits into
stack/509-d1-stage-pipeline-facadefrom
stack/509-d2-stage1-without-cost
Draft

zzylol wants to merge 4 commits into
stack/509-d1-stage-pipeline-facadefrom
stack/509-d2-stage1-without-cost

Conversation

@zzylol

@zzylol zzylol commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Why

#572 ("Update (2026-10-04)") records decision Q36(a): following #509, only Stage 3 uses the cost model. After #581 the planner facade already runs Stage 1 → Stage 2 → Stage 3, but the Stage 1 modules still imported cost_model and recurrence: candidate generation asked a CostModel how to order and size sketches, and the legacy whole-workload selection lived inside replacement.rs. That coupling also blocks moving Stage 1 into its own crate (Phase B, step B2).

What

Stage 1 no longer depends on the cost model. Planner facade behavior is unchanged: the #581 tests pass and stage_pipeline regenerates the Example 1 fixture byte for byte.

Before this PR (production imports):

replacement        ──► cost_model, recurrence
exact_composition  ──► cost_model
grouping           ──► cost_model
cost_model         ──► replacement            (cycle)

After this PR:

replacement        ──► plan_selection::candidate_selection   (GlobalSelection, TargetSubDAGSelection; a B2 blocker, see below)
exact_composition  ──► replacement, accuracy
grouping           ──► replacement, accuracy
plan_selection::candidate_selection ──► cost_model, recurrence, replacement
cost_model         ──► replacement

A new test, stage1_cost_independence, fails if a Stage 1 module imports cost_model or recurrence outside #[cfg(test)].

How

  1. Cost-free candidate generation. realizations_for_intent(intent) lists sketch candidates in summary_candidates order and sizes them with accuracy::estimators::size_params. CostModel::size_params defaulted to the same function. Extension intents stay PassThrough, which was the previous default. CandidatePlanningInputs drops cost, and realize_child drops its cost argument.

  2. Cost-free strategies. Strategies are now built without a cost model:

    • ASAPStrategies::default() and HydraGroupingStrategy::default();
    • ExactCompositionStrategy, now a unit struct;
    • new_with_planning_inputs*, without the cost argument;
    • default_strategies_with_evidence(evidence).

    The strategy-level runtime-support check is dropped, because selection already requires positive support evidence before it commits a composition.

  3. Pure move. The cost- and recurrence-dependent selection block moves from replacement.rs to the new plan_selection/candidate_selection.rs (plan_selection.rs becomes plan_selection/mod.rs), together with its tests. Only visibility and imports change. Its deletion is tracked in Pass 2 sharing and Stage 1 coverage parity with the retired MajorPass #580.

  4. Call sites. Call sites in integration-tests, asap-physical-operators, frontend-promql, planner and devtools use the new constructors, and docs/develop_docs no longer describes the removed hooks.

Moved to plan_selection::candidate_selection (1,718 production lines and 22 tests)

  • ReplacementSubDAG::runtime_support_evidence
  • CandidateLogicalASAPDAGs::{cost_sorted, cost_sorted_with_recurrence, recurrence_profiles, global_selection, global_selection_with_recurrence}
  • RecurrenceProfileMap, RankedTargetSubDAGCandidates, rank_group, cse_preference, realize_one, sketch_kind_of, summary_grouping
  • TargetSubDAGSelection, CompositionDecision, GlobalSelection (with assemble_*), composition_options, is_automatically_selectable, decide_*, multiplier, ReferenceDAG, topological_order, and their helpers

These are still used by tests, devtools dag_export --post-asap and integration-tests, so they are moved rather than deleted. The public types are still re-exported at the crate root.

Deleted

Item Evidence or reason
CandidateLogicalASAPDAGs::recurrence_profiles_from_workload No caller in the workspace
CostModel::{size_params, realize_extension, evaluation_extension} Stage 1 no longer consults them, and they have no other caller
ASAPStrategies::{new, default_cost_model}, HydraGroupingStrategy::{new, default_cost_model}, ExactCompositionStrategy::{new, default_cost_model}, default_strategies_with Replaced by the cost-free constructors (default_strategies_with(m) equals default_strategies() once the cost model is gone)

Ten tests were deleted, each because it exercised only a removed hook:

Test Removed hook
cost_model::size_params_default_body_matches_default_size_params custom size_params
cost_model::custom_cost_model_can_override_sizing_independently_of_ranking custom size_params
replacement::extension_intent_realizes_via_custom_cost_model custom realize_extension
exact_composition::a_runtime_without_the_capability_gets_no_candidate strategy-level support evidence
integration-tests exact_composition::a_runtime_without_mixed_execution_gets_no_composition_candidates strategy-level support evidence (selection-time exclusion is still covered by unknown_runtime_capability_keeps_candidate_but_prevents_selection)
replacement::realize_with_custom_cost_model_overrides_default_summary_choice strategy-level ranking in realize_child
replacement::custom_cost_model_still_enumerates_every_candidate_not_just_its_own_pick strategy-level ranking (enumeration is still covered by approximate_quantile_enumerates_every_summary_candidate)
grouping::custom_cost_model_cannot_enable_unproven_hydra_kll strategy-level ranking (still covered by quantile_has_no_hydra_candidate_without_a_modeled_error_bound)
explanation::custom_cost_model_changes_the_reported_sketch_kind strategy-level ranking
empirical_cost::public_cost_model_changes_replacement_order_without_changing_guarantees strategy-level ranking and size_params

One test was adapted: replacement::enumerating_the_targets_candidates_does_not_leak_into_a_nested_aggregate now expects the inner aggregate's static order [Kll, DDSketch] instead of a custom cost model's order.

Behavior note (legacy search path only). A custom cost model no longer reorders or resizes candidates during generation. It still ranks them at selection time through rank_candidates, candidate_cost and the other hooks. The in-tree production models (PhysicalPlanCostModel, the dag_export export model) already used the identity ranking.

Remaining B2 blockers (moving Stage 1 into a logical-optimizer crate)

  1. Stage 1 imports Stage 3: replacement.rs:383 imports plan_selection::candidate_selection::{GlobalSelection, TargetSubDAGSelection}. enumerate_candidate_dags* builds them at replacement.rs:4240, replacement.rs:4249 and replacement.rs:4258 (assemble_target) to assemble the unpriced candidate inventory.

  2. candidate_selection.rs adds inherent impls on Stage 1 types: impl ReplacementSubDAG (candidate_selection.rs:34) and impl CandidateLogicalASAPDAGs (:53, :199, :1211). These are not allowed across crates. They also use Stage 1 internals that are only pub(crate):

    • the fields groups, order and composition_plans;
    • PreparedComposition;
    • cse_candidate_pair, direct_child_counts and realize_child (candidate_selection.rs:28-29).

    Resolved by Pass 2 sharing and Stage 1 coverage parity with the retired MajorPass #580.

  3. The shared accuracy module is used by both Stage 1 and Stage 3, and it imports Stage 1:

    • accuracy/mod.rs:31 imports exact_composition::ExactOperation;
    • accuracy/estimators/mod.rs:160 uses replacement::accuracy_budget;
    • accuracy/composition.rs:453 uses the private function_rules.

    It has to move with Stage 1, or be split.

  4. cost_model.rs:52-53, :858 and :875 call the pub(crate) replacement::realize_child. The direction is fine, but this needs a public API once Stage 1 is a separate crate.

No Stage 1 module imports Stage 2 (physical_candidates) or the facade (pass).

Gate

  • cargo fmt --all --check: passes.
  • cargo clippy --workspace --all-targets --all-features -- -D warnings: passes.
  • cargo test --workspace: 1,524 passed and 12 ignored. That is the 1,533 from feat(planner): run the #509 stage pipeline with tree-DP selection; retire MajorPass #581, less the 10 deleted tests, plus the new guard.
  • python3 -m unittest discover -s tools/dag-viewer -p test_render.py: 29 tests OK, 6 skipped.
  • stage_pipeline --example planner-layering-1 regenerates tools/dag-viewer/examples/planner-layering-example1.json byte for byte.

Part of #509. Follows #572 (decision Q36(a)). Follow-up: #580.

🤖 Generated with Claude Code

zzylol and others added 4 commits October 4, 2026 01:29
Move the cost- and recurrence-dependent selection over
`CandidateLogicalASAPDAGs` (cost ranking, recurrence profiles, global
selection, selected-DAG assembly, runtime support evidence) and its tests
from `replacement` into `plan_selection::candidate_selection`. The code
is unchanged; only visibility and imports were adjusted.
`plan_selection.rs` becomes `plan_selection/mod.rs`.

`recurrence_profiles_from_workload` is deleted rather than moved: it has
no caller anywhere in the workspace.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t model

`realizations_for_intent` lists sketch candidates in `summary_candidates`
order and sizes them with the analytical `accuracy::estimators::size_params`,
which is what `CostModel::size_params` defaulted to. Extension intents stay
PassThrough, the previous default.

- `CandidatePlanningInputs` drops its `cost` field; `realize_child` drops its
  cost argument.
- `ASAPStrategies` and `HydraGroupingStrategy` are built with `default()`
  or `new_with_planning_inputs*` without a cost model; `ASAPStrategies::new`,
  the `default_cost_model` constructors and `default_strategies_with` are
  removed, and `default_strategies_with_evidence` takes only the evidence.
- `ExactCompositionStrategy` is a unit struct and no longer checks runtime
  support; selection already requires positive support evidence.
- `CostModel` loses `size_params`, `realize_extension` and
  `evaluation_extension`.

Tests that exercised only the removed hooks are deleted; call sites across
the workspace use the new constructors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Scans the Stage 1 modules' non-test code and fails on any import of
`cost_model` or `recurrence`, including names `lib.rs` re-exports from them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Update the developer docs and the `CostModel` trait doc: strategies take no
cost model, sketch sizing is analytical, extension intents stay
pass-through, and the cost model is consulted only at selection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant