Skip to content

feat(plan): extend CostModel with a size_params hook - #146

Merged
zzylol merged 1 commit into
mainfrom
feat/costmodel-size-params-hook
Jul 21, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/costmodel-size-params-hook

Conversation

@zzylol

@zzylol zzylol commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • CostModel::rank_candidates lets a deployment reorder which SummaryKind answers an AggIntent, but SummaryParams sizing (k/width/depth/precision) was hardcoded inline in boundary::bind_summary_with, unreachable by any cost model.
  • Adds CostModel::size_params(kind, intent, eps, delta) -> SummaryParams, with a default body delegating to the newly-extracted boundary::default_size_params (the exact same formulas that were inline before — no behavior change for DefaultCostModel or any existing caller).
  • Motivation: ASAPQuery-backend's control_plane has its own KLL/CMS/HLL parameter-sizing logic (in some cases tied to a downstream catalog's discrete supported-K rungs, not a continuous formula) that it needs to keep using while still adopting implement_tree_in_with's IR construction (schema/col/by plumbing) instead of forking it. This is the same "one extension point per deployment-specific decision" pattern rank_candidates already established, one level deeper.

Test plan

  • cargo test -p asap-plan — 26 passed (24 pre-existing + 2 new: size_params_default_body_matches_default_size_params, custom_cost_model_can_override_sizing_independently_of_ranking)
  • cargo test --workspace — all green, no regressions
  • cargo clippy -p asap-plan --all-targets — clean

🤖 Generated with Claude Code

`CostModel::rank_candidates` already lets a deployment reorder which
`SummaryKind` answers an intent, but `SummaryParams` sizing (k / width /
depth / precision) was still hardcoded in `boundary::bind_summary_with`,
unreachable by any cost model. A deployment with its own accuracy-bound
math (or a downstream catalog that only recognizes discrete parameter
rungs) had no extension point short of forking `implementation_for_with`.

Add `CostModel::size_params`, defaulting to the now-extracted
`boundary::default_size_params` (byte-for-byte the same formulas
`bind_summary_with` used inline before) so `DefaultCostModel` and every
existing caller keep today's behavior unchanged. A deployment can now
override ranking, sizing, or both, independently.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol
zzylol merged commit 3bac1d0 into main Jul 21, 2026
1 check passed
@zzylol
zzylol deleted the feat/costmodel-size-params-hook branch July 21, 2026 22:18
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