[milindsrivastava1997]

Closes #997.

Why

Production planning (the memo / joint search) always builds LogicalCandidate with choice: Vec::new() and pass2: None. Only the test helper explicit_assignment_plans filled them, and its one caller never read them. Every type, field, and label that existed to carry or display that metadata was dead weight that each refactor still had to keep compiling.

What

Removes the dead Pass 1 choice / Pass 2 metadata and everything that only fed it. No planner output changes: the same roots are produced in the same order, and production labels already took the roots-only branch.

How

• LogicalCandidate keeps only id and roots.
• stage1_candidates_of_choice returns the roots directly. Its internal Pass 2 enumeration (Pass2List) no longer threads a Rules record through every rule; deduplication and ordering already depended only on roots.
• With Rules gone, RollupPass2Candidate.rollup_candidates and TopKReusePass2Candidate.topk_reuse_candidates had no readers, so they are removed too. The structs stay because many tests read their .roots.
• devtools candidate_labels keeps only the connected-DAG label. choice_label, pass2_label, and six helpers they alone used are deleted.
• Developer docs no longer mention Pass2Candidate.

Key code interfaces added / modified

crates/plan-selection/src/search/pipeline.rs

/// Logical roots of one archived candidate; the physical plan it produced
/// names it by `id`. Stays.
pub struct LogicalCandidate {
    pub id: String,                                                     // unchanged
    pub roots: Result<Vec<QueryRoot>, PerComputationCandidateError>,     // unchanged
    // removed: pub choice: Vec<usize>
    // removed: pub pass2: Option<Pass2Candidate>
}

crates/logical-asap-optimization/src/pass2/workload_candidates.rs

/// The composed candidate's roots (unless not viable), then each of its
/// Pass 2 candidates' roots. Used by single-query pruning
/// (`alone_violation`) and the `explicit_assignment_plans` test helper.
pub fn stage1_candidates_of_choice<Id: Clone>(
    per_computation: &PerComputationCandidates<Id>,
    entries: &[QueryWorkloadEntry],
    choice: &[usize],
    viable: &dyn Fn(&[QueryRoot]) -> bool,
) -> Vec<Result<Vec<QueryRoot>, PerComputationCandidateError>>; // was Vec<Stage1Candidate>

// removed: pub struct Pass2Candidate { from_pass1_position, shared_builds, pane_candidates,
//          shared_pane_chains, rollup_candidates, topk_reuse_candidates, sharing_candidates }
// removed: pub struct Stage1Candidate { choice, roots, pass2 }
// removed (private): fn choice_candidates(.., from: usize, ..), struct Rules, Pass2::failed

crates/logical-asap-optimization/src/pass2/asap_aware_cse.rs

/// A rollup-rule Pass 2 candidate. Stays.
pub struct RollupPass2Candidate {
    pub roots: Result<Vec<QueryRoot>, PerComputationCandidateError>, // unchanged
    // removed: pub rollup_candidates: Vec<RollupCandidate>
}
/// An exact top-k reuse Pass 2 candidate. Stays.
pub struct TopKReusePass2Candidate {
    pub roots: Result<Vec<QueryRoot>, PerComputationCandidateError>, // unchanged
    // removed: pub topk_reuse_candidates: Vec<TopKReuseCandidate>
}

crates/devtools/src/candidate_labels.rs, planning_stages.rs

pub fn candidate_labels(run: &PlanningStagesRun) -> BTreeMap<String, String>; // unchanged signature
// removed: pub fn choice_label<Id>(inventory: &PerComputationCandidates<Id>, choice: &[usize]) -> String
// removed: pub fn pass2_label(pass2: &Pass2Candidate, pass1_roots: &[QueryRoot], candidate_roots: &[QueryRoot]) -> String
// removed (private): readers_in, readers, same_computation, is_pane_chain_of, same_pane_chain, state_name
// removed: pub use ...::Pass2Candidate (planning_stages.rs)

End-to-end usage (unchanged for users): stage_pipeline --promql 'sum(a)' --promql 'max(a)' → run_stage1_to_stage3_with archives LogicalCandidate { id, roots } → candidate_labels labels each from its roots → the viewer JSON's display_name. Internally, alone_violation calls stage1_candidates_of_choice and plans each returned roots. No numbers change.

Major algorithm changes

None. The Pass 2 enumeration in workload_candidates visits the same rules in the same order and deduplicates on the same flattened roots; it just stops recording which rules produced each candidate.

Before this PR

LogicalCandidate carried choice and pass2: Option<Pass2Candidate>. Production always left them empty. stage1_candidates_of_choice built a Stage1Candidate with a full Pass2Candidate record that only the test helper copied, and nothing read it. devtools had about 370 lines of labelling code for those fields that production never reached.

After this PR

Logical candidates are { id, roots } everywhere, and stage1_candidates_of_choice returns roots. A new Pass 2 rule no longer needs a Pass2Candidate field or a devtools label (extend-asap-aware-mapping.md updated). Net −548 lines.

Evidence

Architecture-only removal; no behavior or performance change to measure. Evidence is the test suite below and git grep showing no remaining readers.

Verification

• Unit tests: cargo test -p asap-logical-asap-optimization -p asap-plan-selection -p asap-devtools -p asap-frontend-promql pass. a_rollup_is_also_built_over_tumbling_panes is rewritten to check roots instead of pass2 metadata: it verifies that some Stage 1 candidate builds the fine sum as a single PaneChain that the coarse sum also reads (the coarse sum rolls up from it). Checked by mutation: it fails when the rollup branch no longer applies window composition.
• End-to-end tests: cargo test -p asap-integration-tests passes. In one full run, memo_covers_whole_workload_pass2::topk_reuse failed (memo cost above whole-workload). It then passed 3/3 when rerun alone. That test compares a time-limited MILP search against a 0.02% margin, so it is timing-sensitive under load, and this PR doesn't touch the memo search. Tracked in Planner tests depend on wall-clock search budgets and flake under load #1067. stage_pipeline::scans_projecting_different_labels_are_shared flaked the same way in one pre-commit run (passed 2/2 alone).
• Other checks: cargo fmt, cargo clippy --workspace --all-targets clean; pre-commit cargo test passed.

Snapshot changes

None.

Architectural decisions

• Kept RollupPass2Candidate / TopKReusePass2Candidate as one-field structs rather than returning bare roots. Unwrapping them would touch many tests that read .roots and buy nothing.
• stage1_candidates_of_choice returns Vec<Result<..>> rather than a one-field Stage1Candidate, since all its callers want only roots.
• Rewrote the rollup-over-panes test rather than deleting it, to keep its coverage.

Limitations and follow-up

• Other Pass 2 struct fields that only tests read (e.g. SummaryCapabilityPass2Candidate.shared_builds, WindowCompositionPass2Candidate.pane_candidates) are kept.
• .agents/skills/asap-code-review/SKILL.md still cites Pass2Candidate as a historical refactor example. Left as is.
• The pre-commit hook trimmed trailing whitespace in planner-layering.md, which is most of that file's diff.

Human review — do not complete with an agent

• The MVP boundary is correct.
• New conceptual layers or public interfaces are necessary.
• The before/after description matches the intended product behavior.
• Human reviewer:
• Decision and rationale:

🤖 Generated with Claude Code