chore: retire promql_utilities crate, move AggregationType into asap_types - #403
Merged
Merged
Conversation
…types promql_utilities had shrunk to a single type (AggregationType, 162 lines) after prior stages of the sketch-identity unification moved everything else out. asap_types was already the only crate depending on it and already re-exported the type, so the separate crate boundary served no purpose. Moves AggregationType verbatim into crates/asap_types/src/aggregation_type.rs, repoints all import sites across data_plane/control_plane, and deletes the crate. Pure structural move — AggregationType's variants and semantics are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Jul 21, 2026
…ities retirement promql_utilities was deleted in #403 (merged to main after this branch was cut); AggregationType now lives in-crate at asap_types::aggregation_type. Update the one remaining import site picked up by the rebase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5 tasks
zzylol
added a commit
that referenced
this pull request
Jul 21, 2026
…ities retirement promql_utilities was deleted in #403 (merged to main after this branch was cut); AggregationType now lives in-crate at asap_types::aggregation_type. Update the one remaining import site picked up by the rebase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks
zzylol
added a commit
that referenced
this pull request
Jul 22, 2026
…ities retirement promql_utilities was deleted in #403 (merged to main after this branch was cut); AggregationType now lives in-crate at asap_types::aggregation_type. Update the one remaining import site picked up by the rebase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Jul 22, 2026
…ed params + grouping) (#401) * feat(asap_types, data_plane): split AggregationType into AccumulatorSpec Step 5 of the sketch-identity unification (scratchpad/artifacts/enum- unification-plan.md, section 6/8). data_plane's AggregationConfig has represented "which accumulator to run, on what, with what parameters" as three loosely-typed things: a 16-variant AggregationType enum that conflates sketch identity with the keyed/unkeyed axis, a raw aggregation_sub_type string consulted only for two "wrapper" variants, and an untyped parameters: HashMap<String, Value> bag. The real dispatch (accumulator_factory.rs::create_accumulator_updater) was a 14-arm match with a second string-typed dispatch layer underneath it. This adds asap_types::AccumulatorSpec { kind: SummaryKind, params: SummaryParams, grouping: Option<KeyByLabelNames> }, converging data_plane onto the same asap_sketch::SummaryKind/SummaryParams representation control_plane already uses (Stage 3, merged), extended with the keyed/unkeyed grouping axis AggregationType wrongly folded into identity. accumulator_factory.rs now dispatches on AccumulatorSpec.kind instead of the AggregationType + string combo; the SingleSubpopulation/MultipleSubpopulation string-matching arms collapse away now that grouping is a sibling field. Every one of the old 14 match arms' behavior is preserved exactly, including the CMS- heap vs bare-CMS distinction, the bare-CountSketch-shares-CmsAccumulator quirk, and all three fallback/unknown-warning paths (same warning text, same default updater per path). Two decisions worth flagging for review: - Additive, not a replacement. AggregationConfig keeps its aggregation_type/aggregation_sub_type/parameters fields untouched. PolicyFingerprint::from_config hashes those three fields directly and its own module doc calls the byte layout it produces a stability contract ("any such change invalidates every deployed fingerprint and forces a cold-start rebuild") -- so policy_fingerprint.rs is not touched by this change at all. Separately, AggregationType turned out to be read by ~40 files across data_plane/asap_types (query-time capability matching, persistence, reconciliation, index maintenance) well beyond accumulator_factory.rs, so full removal was judged too large to land and review safely in one PR. AccumulatorSpec is computed on demand from AggregationConfig's existing fields via AggregationConfig::accumulator_spec(); full removal of the old fields is follow-up work, not done here. - Three details don't fit asap_sketch's upstream types and still read AggregationConfig/its parameters map directly, documented in accumulator_spec.rs's module doc: min/max direction (SummaryParams:: MinMax carries no fields), HydraKLL's (row, col) tiling grid (SummaryParams::Kll carries only k), and top-k weight_mode (no upstream concept at all). The wire format (aggregationType/aggregationSubType/parameters JSON and YAML keys) is unaffected -- AggregationConfig::from_yaml/from_json still parse those key names generically, unchanged. cargo test -p asap_types: 78 passed (18 new, covering every AggregationType variant's resolution, both wrapper sub_type alias lists, all three error paths, and a fingerprint-stability regression guard). cargo test -p data_plane --lib: 881 passed, 2 ignored -- no change from the pre-existing baseline. cargo build --workspace: clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(asap_types): repoint accumulator_spec.rs import after promql_utilities retirement promql_utilities was deleted in #403 (merged to main after this branch was cut); AggregationType now lives in-crate at asap_types::aggregation_type. Update the one remaining import site picked up by the rebase. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(asap_types): rename WindowType to WindowKind in accumulator_spec.rs tests Picked up by rebasing onto main post-#405 (WindowType retired in favor of asap_ir::WindowKind). Test-only fixture reference, no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
promql_utilitieshad shrunk to a single type (AggregationType) after prior sketch-identity-unification stages moved everything else out of it.asap_typeswas already the only real consumer and already re-exported the type, so the crate boundary added nothing.AggregationType(enum +as_str/is_keyed/is_multi_population_value_type/is_key_agg_type/Display/FromStr/serde impls) verbatim intocrates/asap_types/src/aggregation_type.rs, repoints everypromql_utilities::query_logics::enums::AggregationTypeandasap_types::enums::AggregationTypeimport site acrossdata_plane/control_planetoasap_types::AggregationType, drops the crate from the workspace, and deletes it.AggregationType's variants and behavior are unchanged — no downstream logic touched.Note: this will conflict with #401 (Step 5,
feat/accumulator-spec-split), which still importsAggregationTypefrompromql_utilities. That PR's import will need repointing toasap_types::AggregationTypewhen the two are reconciled.Test plan
cargo build --workspace— cleancargo test -p asap_types -p control_plane -p data_plane --no-run— all lib/bin/test/bench targets compilecargo test -p asap_types -p control_plane -p data_plane --lib— 822 passed; 1 pre-existing failure (optimizer::rules::tests::invalid_sketch_type_override_falls_back_to_default) confirmed to also fail onmainbefore this change, unrelated to this PRrustfmtapplied to touched files only🤖 Generated with Claude Code