refactor(stage1): move Pass 1's memo population into pass1; forbid pass1 → pass2 imports (#1096)

crates/logical-asap-optimization/src/lib.rs

@@ -88,8 +88,8 @@ pub use pass1::{
     PerComputationLogicalMemoCandidatesError,
 };
 // Pass 2: its entry and its error.
+pub use pass1::memo_population::CandidatePopulationError;
 pub use pass2::logical_memo_candidates::generate_logical_asap_memo_candidates;
-pub use pass2::memo::candidates::CandidatePopulationError;
 pub use pass2::memo::state_groups::Pass2RuleError;
 // The CandidateLogicalASAPDAGs a memo stands for (developer tools'
 // reference pricing).

crates/logical-asap-optimization/src/pass1.rs

@@ -7,6 +7,8 @@
 /// Stage 1, Pass 1's entry and output: the per-computation candidates on
 /// the memo groups ([`PerComputationLogicalMemoCandidates`](asap_types::stages::PerComputationLogicalMemoCandidates)).
 pub(crate) mod logical_memo_candidates;
+/// Pass 1's candidates on Stage 0's memo groups (`try_populate_pass1`).
+pub(crate) mod memo_population;
 /// Stage 1, Pass 1's computation targets, their candidates and their
 /// composition. No execution timing is assigned: that is a Stage 2
 /// materialization decision. Its target list

crates/logical-asap-optimization/src/pass1/logical_memo_candidates.rs

@@ -13,12 +13,12 @@ use asap_accuracy::sizing::{
 };
 use asap_accuracy::{AccuracyEvidenceProvider, AccuracyModel};
 
+use super::memo_population::try_populate_pass1;
+use super::memo_population::CandidatePopulationError;
 use crate::deployment_capabilities::SupportedSummary;
 use crate::pass1::per_computation_candidates::{
     generate_per_computation_candidates, PerComputationCandidateError,
 };
-use crate::pass2::memo::candidates::CandidatePopulationError;
-use crate::pass2::memo::groups::try_populate_pass1;
 use asap_types::stages::{
     DagOfLogicalMemoGroups, Memo, PerComputationLogicalMemoCandidates, PrunedCandidate,
 };

crates/logical-asap-optimization/src/pass2/memo/groups.rs → crates/logical-asap-optimization/src/pass1/memo_population.rs

@@ -11,7 +11,7 @@
 //!
 //! A Pass 1 target's aggregate group gets one candidate per combination of
 //! its measures' Pass 1 candidates: the aggregate alone composed by
-//! [`crate::pass1::per_computation_candidates::compose_logical_candidate`] with that choice, every target beneath it
+//! [`super::per_computation_candidates::compose_logical_candidate`] with that choice, every target beneath it
 //! exact. Its local nodes are the composed nodes above the first node
 //! equal to one of the group's descendant groups, which are its children.
 //! The local nodes read each child group through its node (the same
@@ -38,18 +38,28 @@ use asap_types::stages::HowToComputeAggregateMeasure;
 use std::collections::{HashMap, HashSet};
 use std::rc::Rc;
 
-use crate::pass1::per_computation_candidates::{
+use super::per_computation_candidates::{
     compose_logical_candidate_with_accuracy_budget_parts, PerComputationCandidates,
 };
 use asap_types::ir::schema::FieldDataType;
 use asap_types::ir::{ASAPOp, OperatorNode, QueryRoot};
 
-use super::candidates::CandidatePopulationError;
 use asap_types::stages::{
     CandidateForMemoGroup, DagOfLogicalMemoGroups, Memo, MemoGroup, MemoGroupId,
     OriginOfCandidateForMemoGroup,
 };
 
+/// A Pass 1 inventory does not belong to the initial logical memo.
+#[derive(Debug, thiserror::Error, PartialEq, Eq)]
+pub enum CandidatePopulationError {
+    #[error("Pass 1 root dependencies differ from the initial logical memo")]
+    RootDependenciesChanged,
+    #[error("Pass 1 target {target} is not a valid measure of an initial aggregate group")]
+    InvalidTarget { target: usize },
+    #[error("Pass 1 target {target} references missing absorbed target {absorbed}")]
+    InvalidAbsorption { target: usize, absorbed: usize },
+}
+
 /// Populate implementation candidates without changing initial group identities.
 /// The inventory must have walked `initial`'s groups
 /// (`PerComputationCandidates::walked`); its targets name them by id.
@@ -308,8 +318,7 @@ fn compose_alone(
     targets: &[usize],
     choice: &[usize],
     accuracy_budget_parts: usize,
-) -> Result<Vec<QueryRoot>, crate::pass1::per_computation_candidates::PerComputationCandidateError>
-{
+) -> Result<Vec<QueryRoot>, super::per_computation_candidates::PerComputationCandidateError> {
     let mut kept: Vec<usize> = targets.to_vec();
     for &t in targets {
         if let Some((_, Some(absorbs))) = per_computation.targets[t].computation_for(choice[t]) {
@@ -770,7 +779,7 @@ mod tests {
     }
 
     /// The sketch summaries of the first query of `queries` composed with
-    /// every target at its `algorithm` summary, by [`crate::pass1::per_computation_candidates::compose_logical_candidate`]:
+    /// every target at its `algorithm` summary, by [`super::per_computation_candidates::compose_logical_candidate`]:
     /// as the exhaustive search sizes them.
     fn composed_sketches(
         queries: &[&str],

crates/logical-asap-optimization/src/pass1/per_computation_candidates.rs

@@ -239,7 +239,7 @@ impl PerComputationCandidates {
     /// and each query's root over the same groups. Its targets' group ids
     /// then name `dag`'s groups.
     pub(crate) fn walked(&self, dag: &DagOfLogicalMemoGroups) -> bool {
-        use crate::pass2::memo::NodeToStructuralIdCache;
+        use super::memo_population::NodeToStructuralIdCache;
         let mut structural_ids = NodeToStructuralIdCache::default();
         let group_of: HashMap<*const OperatorNode, MemoGroupId> = self
             .logical_nodes

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

@@ -1,12 +1,8 @@
 //! The logical memo's candidates, rules and enumeration, over the memo
 //! groups Stage 0 builds (`crate::stage0`).
 
-/// Logical implementation candidates populated after initial identity construction.
-pub(crate) mod candidates;
 /// The candidate DAGs a memo stands for: choices and their composition.
 pub(crate) mod enumeration;
-pub(crate) mod groups;
-pub(crate) use groups::NodeToStructuralIdCache;
 pub(crate) mod rules;
 /// Shared finite state configuration enumeration.
 pub(crate) mod shared_configurations;

crates/logical-asap-optimization/src/pass2/memo/rules/rollup.rs

@@ -313,8 +313,8 @@ pub(crate) fn add_window_rollup_candidates(memo: &mut Memo) -> Result<usize, Pas
 
 #[cfg(test)]
 mod tests {
+    use crate::pass1::memo_population::try_populate_pass1;
     use crate::pass1::per_computation_candidates::generate_per_computation_candidates;
-    use crate::pass2::memo::groups::try_populate_pass1;
     use crate::stage0::generate_dag_of_logical_memo_groups;
     use crate::test_support::{lower_promql, sharing_rules_and_shared_state_groups};
     use asap_types::ir::properties::AccuracyTarget;

crates/logical-asap-optimization/src/pass2/memo/rules/shared_configuration_readers.rs

@@ -503,8 +503,8 @@ fn compatible(left: &StateNeed, right: &StateNeed) -> bool {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::pass1::memo_population::try_populate_pass1;
     use crate::pass1::per_computation_candidates::generate_per_computation_candidates;
-    use crate::pass2::memo::groups::try_populate_pass1;
     use crate::stage0::generate_dag_of_logical_memo_groups;
     use crate::test_support::{entry_with, lower_promql};
     use asap_types::ir::properties::AccuracyTarget;

crates/logical-asap-optimization/src/pass2/memo/state_groups.rs

@@ -442,8 +442,8 @@ fn replace_node(
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::pass1::memo_population::try_populate_pass1;
     use crate::pass1::per_computation_candidates::generate_per_computation_candidates;
-    use crate::pass2::memo::groups::try_populate_pass1;
     use crate::stage0::generate_dag_of_logical_memo_groups;
     use asap_types::ir::properties::AccuracyTarget;
     use asap_types::ir::QueryRoot;

crates/logical-asap-optimization/src/pass2/memo/window_candidates.rs

@@ -252,8 +252,8 @@ fn candidate_signature(candidate: &CandidateForMemoGroup) -> Result<String, serd
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::pass1::memo_population::try_populate_pass1;
     use crate::pass1::per_computation_candidates::generate_per_computation_candidates;
-    use crate::pass2::memo::groups::try_populate_pass1;
     use crate::stage0::generate_dag_of_logical_memo_groups;
     use crate::test_support::{entry_with, lower_promql};
     use asap_types::ir::properties::AccuracyTarget;

crates/logical-asap-optimization/src/test_support.rs

@@ -7,6 +7,7 @@ use std::rc::Rc;
 // Stage 1 internals other crates' tests call: Pass 1's working form and its
 // population step, single-candidate composition, Stage 0 hash-consing, and
 // the Pass 2 rule entry points those tests build memos with.
+pub use crate::pass1::memo_population::try_populate_pass1;
 pub use crate::pass1::per_computation_candidates::{
     compose_logical_candidate, generate_per_computation_candidates, PerComputationCandidates,
 };
@@ -15,7 +16,6 @@ pub use crate::pass2::asap_aware_cse::{
     window_composition_pass2_candidates, window_composition_shared_chain_candidates,
     with_union_scan_labels,
 };
-pub use crate::pass2::memo::groups::try_populate_pass1;
 pub use crate::pass2::memo::window_candidates::add_local_window_composition;
 pub use crate::stage0::cse::share_common_sub_dags;
 

crates/logical-asap-optimization/tests/pass_layering.rs

@@ -0,0 +1,36 @@
+//! Pass 2 builds on Pass 1, never the reverse (#1096): Pass 1's production
+//! code does not import `crate::pass2`. Test modules may use Pass 2 to build
+//! fixtures. Pass 2 → Pass 1 is still allowed (#1191).
+
+use std::path::Path;
+
+/// Pass 1's production source does not reference `pass2`.
+#[test]
+fn pass1_does_not_import_pass2() {
+    let src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
+    let mut files = vec![src.join("pass1.rs")];
+    for entry in std::fs::read_dir(src.join("pass1")).unwrap() {
+        let path = entry.unwrap().path();
+        // `*_tests.rs` files are `#[cfg(test)]` modules.
+        if !path.to_string_lossy().ends_with("_tests.rs") {
+            files.push(path);
+        }
+    }
+    let mut offending = Vec::new();
+    for file in files {
+        let text = std::fs::read_to_string(&file).unwrap();
+        // ponytail: a file's production code ends at its `#[cfg(test)] mod tests`;
+        // parse with `syn` if inline `#[cfg(test)]` items ever need excluding.
+        let production = text.split("#[cfg(test)]\nmod tests").next().unwrap();
+        for (n, line) in production.lines().enumerate() {
+            if line.contains("pass2") {
+                offending.push(format!("{}:{}: {}", file.display(), n + 1, line.trim()));
+            }
+        }
+    }
+    assert!(
+        offending.is_empty(),
+        "Pass 1 imports Pass 2:\n{}",
+        offending.join("\n")
+    );
+}

docs/design_docs/proposals/planner-layering.md

@@ -310,7 +310,7 @@ rule's origin, and gives each reader a candidate that reads that group.
 
 | `OriginOfCandidateForMemoGroup` | Added by | Candidates |
 |---|---|---|
-| `Pass1` | `try_populate_pass1` (`pass2/memo/groups.rs`) | Every Pass 1 choice of the group's measures: exact, exact accumulator, each summary family and size, and fused computations. Each summary choice comes at every `accuracy_budget_parts` from 1 to the most approximate inputs a query reaching the group has. A fused candidate implements the groups it absorbs only when no other reader needs them; otherwise they stay groups of their own. |
+| `Pass1` | `try_populate_pass1` (`pass1/memo_population.rs`) | Every Pass 1 choice of the group's measures: exact, exact accumulator, each summary family and size, and fused computations. Each summary choice comes at every `accuracy_budget_parts` from 1 to the most approximate inputs a query reaching the group has. A fused candidate implements the groups it absorbs only when no other reader needs them; otherwise they stay groups of their own. |
 | `TumblingWindows` | Window composition: `add_local_window_composition` and `extend_local_window_composition` (`pass2/memo/window_candidates.rs`) | A window summary computed from tumbling windows, at each tumbling-window width the reader's evaluation grid allows. The tumbling-window chain stays private to the candidate. |
 | `WidenedScans` | Scan widening: `add_scan_label_candidates` (`pass2/memo/rules/scan_labels.rs`) | A reader of a metric's scans widened to a shared label set, read through a `Project`. |
 | `SummaryInput` | `factor_summary_inputs` (`pass2/memo/rules/summary_inputs.rs`) | Canonical value preprocessing feeding a summary build. |
@@ -524,7 +524,7 @@ candidates is in `crates/plan-selection/src/` (`select.rs`, `groups/`, `milp/`).
 | Step | Entry point | Path |
 |---|---|---|
 | Stage 0 | `generate_dag_of_logical_memo_groups` → `DagOfLogicalMemoGroups`, `MemoGroup`, `MemoGroupId` | `stage0.rs` (the types: `asap_types::stages`); hash-consing is `share_common_sub_dags` in `stage0/cse.rs` |
-| Pass 1 | `generate_per_computation_logical_memo_candidates` → `PerComputationLogicalMemoCandidates` (its `Memo` and its `PrunedCandidate<HowToComputeAggregateMeasure>`s; `ComputationTarget` is Pass 1's working type, in `asap_types::stages`), the one entry: inside it, the computation targets (walking the memo groups), the summaries the deployment does not support and those no query can use pruned, the candidates attached to the groups (`try_populate_pass1`), then the accuracy-scoped sizes | `pass1/logical_memo_candidates.rs`; `pass1/per_computation_candidates.rs`; `pass2/memo/groups.rs` |
+| Pass 1 | `generate_per_computation_logical_memo_candidates` → `PerComputationLogicalMemoCandidates` (its `Memo` and its `PrunedCandidate<HowToComputeAggregateMeasure>`s; `ComputationTarget` is Pass 1's working type, in `asap_types::stages`), the one entry: inside it, the computation targets (walking the memo groups), the summaries the deployment does not support and those no query can use pruned, the candidates attached to the groups (`try_populate_pass1`), then the accuracy-scoped sizes | `pass1/logical_memo_candidates.rs`; `pass1/per_computation_candidates.rs`; `pass1/memo_population.rs` |
 | Memo types | `DagOfLogicalMemoGroups`, `PerComputationLogicalMemoCandidates`, `HowToComputeAggregateMeasure`, Pass 1's working types (`ComputationTarget`, `FusedComputation`, `HavingEntry`, `HavingPlacement`), `Memo`, `CandidateForMemoGroup`, `OriginOfCandidateForMemoGroup`, `LogicalASAPMemoCandidates`, `CandidateGenerationTimeout`, `root_groups`, `queries_rooted_at` | `crates/types/src/stages/` (`asap_types::stages`) |
 | Pass 2 | `generate_logical_asap_memo_candidates` → `LogicalASAPMemoCandidates`, from Pass 1's output. It calls in this order: 1. window composition for every legal tumbling-window width (`add_local_window_composition`); 2–9. the rules in `pass2/memo/rules/`: scan widening, summary inputs, top-k reuse, finer aggregate, rollup, window rollup, summary results, state configurations; 10. `add_shared_summary_state_groups`, the shared builds | `pass2/logical_memo_candidates.rs` (`LogicalASAPMemoCandidates` itself: `asap_types::stages`); `pass2/memo/window_candidates.rs`, `pass2/memo/rules/`, `pass2/memo/state_groups.rs`, `pass2/memo/shared_configurations.rs` |
 | Memo to candidate DAGs | `enumerate`, `build_logical_asap_dags_from_chosen_candidates`, `ChosenCandidatePerMemoGroup`, `AllChosenCandidatesPerMemoGroup`, `ErrorBuildingLogicalASAPDagsFromChosenCandidates` | `pass2/memo/enumeration.rs` (re-exported at the crate root) |