feat(sid): wire find_matching_policies + sids_for_policy into ASAPQueryEngine.execute - #206
Merged
Merged
Conversation
…ryEngine.execute Switches the query engine's candidate → sid resolution to the content-addressed fast path. PRs #203, #204, #205 landed the primitives; this PR integrates them. ## Before vs. after Before, per candidate: 1. `idx.instances_matching(metric, gbk)` walks the `RwLock<HashMap<u64, SketchInstanceMetadata>>` and returns every sid whose metric + group-by-keys match. 2. Per-sid: classify, check capability satisfaction, push to `hit_sids`. After, per candidate: 1. Snapshot the streaming config; build the `PolicyRegistry`. 2. `find_matching_policies(registry, candidate)` — walks the small policy registry (≤ thousands of entries) with the full match predicate (metric + group_by + capability + window + filter). Returns `Vec<PolicyFingerprint>`. 3. For each fp: `idx.sids_for_policy(fp)` — O(1) hash lookup over the reverse index added in PR #203. 4. Same per-sid classify + capability check as before (defensive; fast-path-discovered sids already satisfy the candidate by construction, but UNSET sids reached via the fallback don't). ## Slow-path fallback retained When the fast path yields zero sids — because either: - no policy in the registry matches the candidate (control plane hasn't published one yet), OR - the candidate's sids were registered with `PolicyFingerprint::UNSET` (legacy paths that didn't carry an `AggregationConfig` at ingest, test fixtures, raw mode) — the engine falls back to the metadata walk `instances_matching(metric, gbk)`. The per-sid capability filter below catches mismatches the fast path would have rejected at policy-match time. The fallback can be deleted in a follow-up once every code path populates `policy_fp` and existing on-disk records have aged out. ## Snapshot semantics The streaming-config snapshot is pinned once per query (not per candidate). Hot-reload swaps the underlying `Arc<StreamingConfig>` mid-query are isolated by the snapshot: the query sees the policy set that was active at query start. Same isolation the legacy `streaming_config_snapshot()` call already gave the engine elsewhere. ## Test plan - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` green (`asap_types::capability_matching::tests::avg_finds_sum_and_count` HashMap-iteration flake unchanged) - Existing query-engine tests cover the slow-path fallback (their fixtures register sids with UNSET fp); future tests on the fast path land alongside an end-to-end test that builds a streaming config with matching policies and verifies the fp-keyed lookup is used. 🤖 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
May 14, 2026
…ty-matching flake (#208) Two related cleanups bundled because both touch deterministic sid lookup and they're each small. ## 1. Drop `instances_matching` fallback in ASAPQueryEngine.execute PR #206 wired the content-addressed fast path with a fallback to the legacy `idx.instances_matching(metric, gbk)` walk for sids registered with `PolicyFingerprint::UNSET`. With PRs #203 + #205 populating the fp on every production registration path, the fallback's only consumers are test fixtures and the raw-mode fast-path that the sink already drops. Removing it makes "capability miss" mean exactly one thing — no policy in the registry satisfies the candidate — instead of overloading the miss path between "no policy" and "no sid metadata". ## 2. Make `aggregation_priority` a total order `capability_matching::tests::avg_finds_sum_and_count` had been flaky because `aggregation_priority` returned `Equal` on equal `window_size`, leaving `Vec::sort_by` order dependent on the underlying `HashMap` iteration. The test inserts both a `Sum` and a `CountMinSketch` config for the same metric; `Statistic::Sum` matches both, and when the multi-pop `CountMinSketch` sorted first the downstream key-aggregation lookup (only needed for multi-pop value types) missed and the whole match returned `None`. Sort keys now: 1. Larger `window_size` (coarser windows answer finer-grained queries via re-aggregation). 2. Single-population types beat multi-population (avoids the key-aggregation hunt when both shapes serve the statistic). 3. `aggregation_id()` (the policy fingerprint u64) tie-break — deterministic across runs and hosts. The result for the failing test: `Sum` always wins the Sum-stat candidate, no key-aggregation lookup fires, the function returns `Some(...)` deterministically. ## Engine test update `execute_returns_capability_miss_when_classify_is_ghost` previously asserted the detail string contained "ghost" / "unknown". With the fallback removed, the engine short-circuits at the policy-resolution step (the test fixture's `dd_meta` helper registers with `PolicyFingerprint::UNSET`, so the policy lookup never finds the sid). The `CapabilityMiss` outcome is preserved; the detail-string assertion is dropped since it pinned implementation, not contract. ## Test plan - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` green - [x] `avg_finds_sum_and_count` ran 5× in a row, all pass — flake gone 🤖 Generated with [Claude Code](https://claude.com/claude-code) 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
Switches the query engine's candidate → sid resolution to the content-addressed fast path. PRs #203 / #204 / #205 landed the primitives; this PR integrates them.
Before vs. after
idx.instances_matching(metric, gbk)walks the per-sid metadata mapfind_matching_policies(registry, candidate)→sids_for_policy(fp), both O(1)-amortizedSlow-path fallback retained
When the fast path yields zero sids — no matching policy OR sids carry
PolicyFingerprint::UNSET(legacy test fixtures, raw mode, paths that don't carry anAggregationConfig) — the engine falls back to the legacy walk. Deletable once every path populatespolicy_fpand old records age out.Test plan
cargo check --workspacecleancargo test --workspace --lib --binsgreen🤖 Generated with Claude Code