Skip to content

refactor: move AccuracyModel to plan-selection - #636

Draft
zzylol wants to merge 1 commit into
stack/cleanup-9-delete-legacy-searchfrom
stack/cleanup-10-accuracy-model-to-plan-selection
Draft

zzylol wants to merge 1 commit into
stack/cleanup-9-delete-legacy-searchfrom
stack/cleanup-10-accuracy-model-to-plan-selection

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

Problem

Q53: only Stage 3 checks an estimate against its accuracy target, but the AccuracyModel trait and DefaultAccuracyModel lived in Stage 1's crate. The legacy search was the reason; #635 deleted it.

Changes

  • New asap_plan_selection::accuracy module, re-exported at the crate root:
    • AccuracyModel with local_guarantee and satisfies.
    • DefaultAccuracyModel, which delegates to logical-optimizer's estimators.
    • PlanningModels now takes this trait. As before, Stage 3 rejects an estimate whose family has no model (local_guarantee returns None).
  • asap_logical_optimizer::accuracy no longer defines a model. It exposes the analytical functions the default delegates to:
    • local_guarantee and sketch_guarantee (made pub)
    • satisfies, the conservative target check moved out of DefaultAccuracyModel::satisfies unchanged
    • The estimator unit tests call these functions directly. The crate root no longer re-exports AccuracyModel / DefaultAccuracyModel.
  • propagate and exact_operation_rule were already dropped in chore: delete the legacy replacement search and CostModel #635 (they depended on deleted modules).
  • Test models repointed to asap_plan_selection::{AccuracyModel, DefaultAccuracyModel}: planner/tests/summary_sharing.rs (UnivMonEvidence) and frontend-promql/tests/univmon_candidates.rs.
  • The stage_pipeline devtool needs no change: it reports the model by name ("DefaultAccuracyModel").
  • The facade re-exports only PlanningModels, so it needs no change either.

Rebase note: rebased onto #627: the plan-selection AccuracyModel keeps answers(statistic, guarantee) next to satisfies, as a default method delegating to the new asap_logical_optimizer::accuracy::answers, and DefaultAccuracyModel delegates it like the other two methods; Stage 3 still calls models.accuracy.answers(...). #621's UnivMon L2 tests now call local_guarantee/satisfies directly.

Stacked on #635.

Test plan

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  • cargo test --workspace --locked: 1280 passed, 0 failed, 14 ignored (after the rebase on main d4869a7; was 1256 passed, 22 ignored)

🤖 Generated with Claude Code

Only Stage 3 checks estimates against accuracy targets, so the
AccuracyModel trait (local_guarantee, satisfies) and DefaultAccuracyModel
move to asap-plan-selection. logical-optimizer keeps the analytical
estimators and exposes local_guarantee, sketch_guarantee and satisfies;
DefaultAccuracyModel delegates to them (Q53).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the stack/cleanup-9-delete-legacy-search branch from f9fc726 to a7a07d8 Compare October 5, 2026 06:21
@zzylol
zzylol force-pushed the stack/cleanup-10-accuracy-model-to-plan-selection branch from 6d28dba to 910df58 Compare October 5, 2026 06:21
This was referenced Oct 5, 2026
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