Skip to content

test: port the legacy-search callers outside logical-optimizer to Stage 1 - #633

Draft
zzylol wants to merge 1 commit into
stack/cleanup-7-stage1-helpersfrom
stack/cleanup-8-port-legacy-callers
Draft

zzylol wants to merge 1 commit into
stack/cleanup-7-stage1-helpersfrom
stack/cleanup-8-port-legacy-callers

Conversation

@zzylol

@zzylol zzylol commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Stack: #574 → #620 → #618 → #621 → #627 → #625 → #628 → #632 → #634 → #616 → #617 → #622 → #624 → #629 → #630 → #631 → #633 → #635 → #636 → #637

Rebased: univmon_candidates.rs keeps #621's assertion that only L2 is certified (RelativeValue), on the Stage 1 candidates.

Problem

The legacy search (ASAPStrategies, search_workload*, explain_replacements, retain_exact) still had callers outside logical-optimizer: one devtool and a set of frontend, integration and executor tests. It cannot be deleted (Q68 (a), step 7c) until they move to Stage 1 or are deleted.

Changes

One decision per file. Shared helpers: stage1_candidates(root) in executor/tests/common/mod.rs and frontend-promql/tests/support.rs composes every Pass 1 choice for one query (up to 4096) and skips those that do not compose. stage1_plan(root) in the frontend-promql support composes the first summarized choice, or else the pass-through choice.

File Decision
devtools/src/bin/sketch_coverage.rs Rebuilt on Pass 1. Runs enumerate_local_logical_candidates per lowered query. It prints the sketch alternatives Pass 1 offers for each query (a Hydra alternative prints as Hydra(<algorithm>)), and per corpus the fraction of lowered queries with at least one. The CSE-shareable metric is dropped: Stage 1's identical-expression sharing is a workload variant, not a per-query replacement. Over every corpus: 135/2178 lowered queries get a sketch alternative at ε = 0.01.
frontend-promql/tests/observability/promql_corpus.rs, metrics_observability.rs Ported to Stage 1 (stage1_plan). Pass 1 must accept every lowered query, and the query must compose to a candidate. Totals: docs 19 summarized / 28 unchanged / 0 errors; testdata 354 / 721 / 0; every metrics-observability corpus 0 errors. A choice can legitimately fail to compose, for example a Top-K heap over a closed schema or a summary over two sources. Selection skips such choices, so the probe does not count them as errors.
frontend-promql/tests/count_planning.rs Ported to Stage 1 candidates. The Hydra test now checks that Pass 1 offers a Hydra alternative and that selection does not pick it (renamed grouped_count_offers_hydra_but_selection_keeps_per_group_state). Its input carries the series identity: Hydra hashes a non-null item, and PromQL labels are nullable.
frontend-promql/tests/univmon_candidates.rs Ported to Stage 1 candidates. The synthetic AccuracyModel is gone. The shared count is lowered at an approximate target, because Pass 1 offers no UnivMon for an exact count (the legacy search did). Stage 1 nodes carry no guarantee, so the "no calibrated bound" check uses DefaultAccuracyModel::local_guarantee.
integration-tests/tests/promql_numeric_regressions.rs Ported to Stage 1. plan takes each target's first alternative other than pass-through; the weighted-heap test takes the heap that absorbs the inner aggregate. Dropped: the node-guarantee assertions (Stage 1 sets none; the ratio-certification gap is listed in #623) and the Top-K evidence provider. The two-metric guarded expression became avg_over_time(a[5m]) + avg_over_time(a[5m] offset 5m): Pass 1 offers no summary over two sources. A signed sum's Count-Min heap is now checked to carry no non-negative proof, not to be absent; Stage 3 rejects it.
integration-tests/tests/precompute_raw_samples.rs Ported: candidates are the Stage 1 compositions that Stage 3 could admit. Count-Min over unproven weights is skipped.
integration-tests/tests/cse.rs Deleted. It pins the legacy search's memo groups. Its share-vs-recompute coverage is already listed in #623, and identical-sub-DAG merging is covered by pass2::identical_expressions's tests.
frontend-sql/tests/pearson_corr.rs Ported: the exact plan is Stage 1's pass-through composition. Pass 1 offers only pass-through for corr. The exact-guarantee assertion is dropped.
executor/tests/weighted_topk_binding.rs Ported where Stage 1 has the realization. Kept: grouped weighted top-k binding (see Findings, now #[ignore]), direct per-series rate heap with dynamic labels, maintained rate heap. These run on Count Sketch only, because Stage 1 proves no sign for rate values and the executor rejects a Count-Min heap without one. The evaluation-timestamp assertions are dropped, because Stage 1's heap evaluation outputs no timestamp column. Deleted: physical_binding_does_not_impose_an_accuracy_acceptance_policy (it only varied legacy evidence), spatial_topk_exposes_signed_heap_candidate_over_complete_snapshot (legacy current_series_topk_candidates only), and direct_rate_topk_exposes_heap_candidates_with_complete_series_identity (a closed catalog schema with no series identity has no Top-K item in Stage 1).
executor/tests/planspace_series_identity_heap.rs Kept selection_never_commits_a_series_identity_heap (already on plan_stages). Deleted the four tests of the legacy search's whole-root proposals.
executor/tests/precompute_candidates.rs Ported: the grouped-rate candidates come from Stage 1.
executor/tests/deployment_computation.rs Ported: the Count-Min count(up) candidate comes from Stage 1.
executor/tests/current_series_heap.rs, deployment_computation.rs (population_dag), precompute_candidates.rs (population_topk_*), frontend-sql/tests/maintained_population.rs, frontend-promql/tests/maintained_population_horizon.rs Unchanged. They call MaintainedPopulationStrategy::candidate directly, not the search. The plan for 7c is to keep candidate and drop only its ReplacementStrategy impl.

Findings: Stage 1 gaps these ports exposed (not fixed here)

  1. Bug: the whole-expression heap (topk by(job)(k, sum by(service, job)(rate(...)))) keeps the outer top-k's partition keys. Those keys index the inner aggregate's output, but the heap is built over the raw rates. Here by(job) became key 0 = ts, and the output schema loses job. The legacy search remapped the keys. planner_weighted_topk_binds_at_either_deployment_phase is #[ignore]d with this reason.
  2. Stage 1's heap evaluation outputs no evaluation-timestamp column; the legacy realization did.
  3. No non-negativity proof for rate values, so no Count-Min heap over a rate.
  4. No Top-K item identity for a closed schema without the series identity.
  5. No summary over an input with two sources.
  6. Hydra over PromQL needs the series identity column (labels are nullable).

Stacked on #631.

Test plan

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  • cargo test --workspace --locked: 1508 passed, 0 failed, 15 ignored (after the rebase on main d4869a7; was 1484 passed, 23 ignored)
  • cargo run -p asap-devtools --bin sketch_coverage -- --data-ingestion-interval-ms 1000 runs

🤖 Generated with Claude Code

…ge 1

The sketch_coverage devtool and the frontend, integration and executor
tests that drove ASAPStrategies / search_workload / retain_exact now take
their candidates from Stage 1 (Pass 1 inventory and composition) or the
stage pipeline. Tests that only pinned legacy search behaviour are
deleted, among them integration-tests/tests/cse.rs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the stack/cleanup-7-stage1-helpers branch from a35b447 to c41b508 Compare October 5, 2026 06:21
@zzylol
zzylol force-pushed the stack/cleanup-8-port-legacy-callers branch from 94a6d35 to b0b37f1 Compare October 5, 2026 06:21
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