refactor(types): rename AggregationConfig::aggregation_id() to policy_fp_u64() (closes #272 step 5) - #294
Merged
Conversation
…_fp_u64() (closes #272 step 5) The `aggregation_id()` accessor was a vestigial alias from the pre-PR-5 era when `aggregation_id` was a controller-allocated counter id distinct from the content fingerprint. After PR 5 collapsed identity into `PolicyFingerprint::from_config`, the accessor's only job became `self.policy_fingerprint().as_u64()` — returning the same u64 used as the `StreamingConfig` map key (`HashMap<u64, AggregationConfig>`). The misleading name implied a counter id; the value IS the policy fingerprint. Sweep: - `pub fn aggregation_id()` -> `pub fn policy_fp_u64()` on `AggregationConfig`. Doc comment refreshed to call out the content-addressed semantic explicitly. - All 55 call sites mechanically rekeyed via sed across asap_types (capability_matching, streaming_config, policy_fingerprint, aggregation_config tests) and data_plane (drivers/ingest/otel, drivers/query/servers/http, precompute_engine/{output_sink,worker}, query_engines/asap_query_engine/engine, storage_engines/sketch_db/{backfill/{mod,processor,service}, lifecycle/eviction}, storage_engines/types/hot_reload_config, tests/test_utilities/engine_factories). - Tracing field names inside `find_compatible_aggregation` (`agg_id=`, `chosen_agg_id=`, `value_agg_id=`, `key_agg_id=`) renamed to `policy_fp=` / `chosen_policy_fp=` / `value_policy_fp=` / `key_policy_fp=` for log-side consistency. - Two test-fn renames mirror the accessor rename: `aggregation_id_accessor_equals_fingerprint_u64` in `aggregation_config.rs` and `policy_fingerprint.rs` -> the `policy_fp_u64_*` form. Out of scope (deliberate, per #272 step 5): - `AggregationIdInfo` struct fields (`aggregation_id_for_key` / `aggregation_id_for_value`): typed accessors (`policy_fp_for_key()` / `policy_fp_for_value()`) already exist; the underlying u64 field names are stable wire-adjacent surface. - `StreamingConfig::{get_aggregation_config, contains, Index}` parameter name `aggregation_id: u64`: separate public API. - `control_plane/`: rg confirms zero `.aggregation_id()` call sites and no `pub fn aggregation_id` defs there — accessor was data-plane / asap_types only. - Local `let agg_id = ...` bindings in B7.6/B7.7-settled files (output_sink, engine_factories, backfill/mod): too deeply tangled with downstream uses to rename in this PR; values are now correctly fingerprints regardless of binding name. Verification: - `cargo build --all`: clean. - `cargo test -p asap_types --lib`: 60/60 pass. - `cargo test -p data_plane --lib`: 743/743 pass (matches #292 baseline). - `cargo test -p control_plane --lib --bins`: 748 + 28 pass (matches baseline). - `rg "\.aggregation_id\(\)" .`: 0 hits. - `rg "pub fn aggregation_id" .`: 0 hits. Co-Authored-By: Claude Opus 4.7 (1M context) <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
Issue #272 step 5: "Delete
pub fn aggregation_id()onAggregationConfigonce all consumers are gone."Post B7.6 (#284) / B7.7 (#286), every site that used the
aggregation_id()accessor as a(agg_id, group_key)bucket key has been rekeyed tosid: u64. The remaining ~55 call sites are legitimate policy-fingerprint uses (e.g.StreamingConfig'sHashMap<u64, AggregationConfig>keys, capability matching'sAggregationIdInfo, debug log fields).The misleading name (
aggregation_id) is what's left to fix — the value has always beenself.policy_fingerprint().as_u64(). This PR renames the accessor to make that explicit, matching the existingPolicyFingerprint::as_u64()convention.pub fn aggregation_id()->pub fn policy_fp_u64()onAggregationConfig(with refreshed doc-comment).asap_typesanddata_plane.find_compatible_aggregationrenamed (agg_id=->policy_fp=,value_agg_id=->value_policy_fp=, etc.) so logs stay consistent.aggregation_id_accessor_equals_fingerprint_u64->policy_fp_u64_*).Sweep inventory (15 files, 66 insertions / 66 deletions)
crates/asap_types/src/aggregation_config.rs(accessor + test)crates/asap_types/src/capability_matching.rs(priority sort + 4 debug fields + 13 test-helper sites)crates/asap_types/src/policy_fingerprint.rs(1 test)crates/asap_types/src/streaming_config.rs(2 sites)data_plane/src/drivers/ingest/otel.rs(4 sites)data_plane/src/drivers/query/servers/http.rs(1 site)data_plane/src/precompute_engine/{output_sink, worker}.rs(1 + 1 doc + 1 site)data_plane/src/query_engines/asap_query_engine/engine.rs(1 site)data_plane/src/storage_engines/sketch_db/backfill/{mod, processor, service}.rs(1 + 7 + 4)data_plane/src/storage_engines/sketch_db/lifecycle/eviction.rs(1)data_plane/src/storage_engines/types/hot_reload_config.rs(1)data_plane/src/tests/test_utilities/engine_factories.rs(8)Out of scope (per #272 step 5)
AggregationIdInfostruct fields (aggregation_id_for_{key,value}) — typed accessorspolicy_fp_for_{key,value}()already exist.StreamingConfig::{get_aggregation_config, contains, Index}parameter namedaggregation_id: u64— separate public API.control_plane/—rgconfirms zero.aggregation_id()calls and nopub fn aggregation_iddefs (theaggregation_id: Stringfield on controller emit messages is unrelated).let agg_id = ...bindings in B7.6/B7.7-settled files (output_sink, engine_factories, backfill/mod) — values are now correctly fingerprints; renaming the bindings would touch B7-settled diff territory unnecessarily.Test plan
cargo build --all: clean.cargo test -p asap_types --lib: 60/60 pass.cargo test -p data_plane --lib: 743/743 pass (matches feat(query): compose rate-over-Sum into ExactAgg(Sum) candidate (multinode topk dispatch) #292 baseline).cargo test -p control_plane --lib --bins: 748 + 28 pass (matches baseline).rg \"\\.aggregation_id\\(\\)\" .: 0 hits.rg \"pub fn aggregation_id\" .: 0 hits.Closes #272 step 5.
🤖 Generated with Claude Code