feat(sid): migrate PrecomputedOutput + sinks to PolicyFingerprint - #197
Merged
Merged
Conversation
PR 4 of the merged-sid-identity chain. Threads `PolicyFingerprint`
through the precompute output → sink path while keeping
`aggregation_id` as a transitional fallback.
## What
- `PrecomputedOutput` gains a `policy_fp: PolicyFingerprint` field.
`#[serde(default)]` on read means records persisted before this
PR deserialise as `PolicyFingerprint::UNSET` (the all-zero
sentinel introduced here).
- New constructors `PrecomputedOutput::new_with_policy_fp` and
`new_backfilled_with_policy_fp` take both the legacy `aggregation_id`
and the content-addressed `policy_fp`. The pre-existing
`new` / `new_backfilled` continue to compile and leave `policy_fp`
as the sentinel — used by the raw-mode fast-path that doesn't
carry a source `AggregationConfig`.
- `SketchStoreSink::append_to_index` now resolves the source config
via `PolicyRegistry::get(policy_fp)` when the output carries a
non-sentinel fp; falls back to the legacy
`StreamingConfig::get_aggregation_config(aggregation_id)` lookup
otherwise.
- Precompute worker (`worker.rs`) — 7 group-state-driven emit sites
migrated to `new_with_policy_fp`. The fp is computed once from
`state.config` and threaded through each emit.
- Backfill processor (`processor.rs`) — backfilled-window emit
migrated to `new_backfilled_with_policy_fp`.
- Raw-mode fast-path (`process_samples_raw`) stays on the legacy
constructor with a code comment explaining the sentinel. The
raw mode synthesises an `aggregation_id` without an associated
`AggregationConfig`, so the fp can't be derived at emit time;
the sink's fallback handles it.
## Dual-keyed invariant
`StreamingConfig.aggregation_configs: HashMap<u64, AggregationConfig>`
remains the source of truth. `PolicyRegistry` is a derived view
(introduced in PR 3). The two lookup paths can never disagree —
when the fp path returns a config, the agg_id path returns the
same one (modulo the agg_id key itself).
## Test plan
- [x] `cargo check --workspace` clean
- [x] `cargo test --workspace --lib --bins` — all green at the
time of commit (one pre-existing HashMap-iteration-order
flake in `asap_types::capability_matching::tests::avg_finds_sum_and_count`
is unrelated; documented for follow-up).
## Next
- PR 5: delete `aggregation_id` from `AggregationConfig` and the
YAML schema; control plane stops minting u64 ids; `policy_fp`
becomes the only handle on the wire.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Aug 22, 2026
…ing (#443) ASAPController was renamed to ASAPPlanner, and its main branch has moved far past the rev this repo was pinned to: the flat asap-ir/asap-l2/ asap-sketch/asap-plan crate split was consolidated into asap-types (pre_asap/post_asap modules) + asap-aware-mapping, and several IR shapes changed underneath. This re-pins to current ASAPPlanner main and adapts every downstream consumer so the workspace builds and passes tests against it. Dependency changes - control_plane, crates/asap_types, data_plane Cargo.toml: repoint git deps from ASAPController to ASAPPlanner at current main (cb70086b4c4a7ba89baf2516be81d0b192137a3a); replace asap-ir/asap-l2/ asap-sketch/asap-plan with asap-types (aliased locally as `planner-types` via Cargo's `package = "..."` to avoid a path collision with this repo's own crates/asap_types) and asap-aware-mapping. Upstream IR changes adapted to - SummaryKind/SummaryParams split into per-family ExactKind/ExactParams (exact accumulators) and SketchKind/SketchParams (approximate sketches); SummaryAgg's kind+params fields collapsed into one `family: SummaryFamilyType` enum. Every call site across sketch_algebra, optimizer, emit, physical, and data_plane's query engine updated for the split/collapse. - QueryExpr::Window removed upstream (never had a real producer); TimeRange{range,child} was already the actual canonical shape, so every Window match arm across window_fusion.rs, optimizer/engine.rs, physical/{allocator,planner}.rs, colored_dag/*, asap_tier_*.rs, and query_parser is rewritten against TimeRange. window_fusion.rs's recognize_windowed_sketch rewrite is a genuine bug fix, not just a rename: the old Window-matching code was dead against real traffic. - QueryExpr::LetBinding/Ref removed (workload-level CSE representation is gone upstream). control_plane's own dead CSE consumers (optimizer/cse.rs, intent_algebra/lower.rs) are deleted to match -- both were confirmed to have zero live callers (PR #428 switched intent_algebra to asap_frontend_promql::lower_promql directly). CommonSubexprElim (R8) is changed to always return None, with the removed representation documented inline; this is correctness- preserving, it only gives up an optimization pass. - L2Expr/L3Expr/Expr<C> folded directly into QueryExpr itself; L3Scalar renamed ScalarValue; Predicate<C> changed from Predicate(pub L3Expr) to Predicate<C>(pub Box<QueryExpr<C>>). expr_ir.rs now re-exports the upstream scalar types plus type aliases (L2Expr = UnresolvedQueryExpr, L3Expr = QueryExpr) instead of defining its own. - implement_tree_in_with(expr, &BindingScope, cost_model) collapsed to implement_tree_with(expr, cost_model) (BindingScope removed); all call sites updated. - Aggregate.aggs field renamed to Aggregate.measures. Vendored locally (upstream deleted, this repo still needs them) - summary_exec.rs (data_plane): SummaryExecutor trait + execute(), deleted upstream (ASAPPlanner#190/#197). Ported from the pre-deletion crates/sketch/src/exec.rs, adapted so find_candidates takes `family: &SummaryFamilyType` instead of separate kind/params. - WindowKind (crates/asap_types/src/enums.rs): deleted upstream alongside QueryExpr::Window: ASAPPlanner's scope (batch workload planning) has no use for tumbling/sliding/session flush semantics, but this repo's streaming aggregation config still does. Same shape as the old re-export, so its ~145 call sites needed no changes. - Flat SummaryKind/SummaryParams (crates/asap_types/src/ accumulator_spec.rs): upstream's split into ExactKind/SketchKind has no single type spanning both anymore, but this repo's AccumulatorSpec dispatch (~25 call sites in precompute_engine::accumulator_factory) never needed that distinction -- it's purely "which concrete Rust accumulator struct to build." Vendored as the same 14-variant shape the pre-split type had, with From impls to convert from upstream's ExactKind/SketchKind where this repo's other code needs to bridge between them (Materialization.kind/params, BackendAggregation's backend-facing fields -- both genuinely span exact+sketch). Testing - cargo check --workspace --all-targets: clean. - control_plane lib: 706 passed, 1 failed (pre-existing -- see below). - data_plane lib: 927 passed, 0 failed. - asap_types lib: 42 passed, 0 failed. - data_plane/control_plane integration suites: same pattern, all passing except the same 1 pre-existing failure plus 2 already- ignored tests tracked as ASAPQuery-backend#431. Pre-existing failures (not regressions -- verified against unmodified main via a detached git worktree before touching anything): - optimizer::rules::tests::invalid_sketch_type_override_falls_back_to_default: fails identically on unmodified main. bind_workload_typed_with_item_filter's override-re-derivation logic re-derives `statistic` to match the override's natural family BEFORE validity is checked, making the "invalid override falls back to default" path unreachable for the 5 canonical sketch families -- a genuine pre-existing logic bug. - controller_plan_to_query_full_roundtrip_{cms,count_sketch}_with_heap_topk: fail identically on unmodified main; matches two adjacent #[ignore]d tests in the same file already documented as a known gap tracked at ASAPQuery-backend#431. Not in scope - A repo-wide sweep of ASAPController -> ASAPPlanner prose in comments/ docs was only done in files this migration otherwise touched; a full sweep across untouched files is left as follow-up. 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
PR 4 of the merged-sid-identity chain. Threads
PolicyFingerprintthrough the precompute output → sink path while keepingaggregation_idas a transitional fallback.Surface changes
PrecomputedOutputgainspolicy_fp: PolicyFingerprint.#[serde(default)]ensures forward-compat with pre-PR-4 on-disk records (deserialise as thePolicyFingerprint::UNSETsentinel).PrecomputedOutput::new_with_policy_fp+new_backfilled_with_policy_fptake both ids. Existingnew/new_backfilledcontinue to compile (raw-mode keeps using them).SketchStoreSink::append_to_indexresolves the config viaPolicyRegistry::get(policy_fp)when non-sentinel; falls back to legacyaggregation_idlookup otherwise.AggregationConfigat emit time; sink fallback handles it).Dual-keyed
StreamingConfig.aggregation_configsremains the source of truth. The two lookup paths derive from the same source and can never disagree. PR 5 retires the legacy field.Test plan
cargo check --workspacecleancargo test --workspace --lib --binsgreenasap_types::capability_matching::tests::avg_finds_sum_and_count(HashMap-iteration-order-dependent) is unrelated to this PR — needs its own fix in a follow-up.🤖 Generated with Claude Code