Skip to content

feat(emit): controller emits *WithHeap aggregationType when with_heap is set - #259

Merged
zzylol merged 1 commit into
mainfrom
controller-emit-heap-bearing-aggtype
May 16, 2026
Merged

zzylol merged 1 commit into
mainfrom
controller-emit-heap-bearing-aggtype

Conversation

@zzylol

@zzylol zzylol commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the controller-emit gap documented in PR #258's follow-up section. The planner already knew when a CMS / CountSketch binding wanted a heap (bind_cms_with_heap_on_topk, BindCountSketchOnTopK); the emit didn't surface it. Now it does.

What changed

  • CmsParams gains a with_heap: bool field (parallel to the existing CountSketchParams.with_heap). bind_cms_with_heap_on_topk sets true; BindCmsOnCount sets false. All other constructors default to false.
  • sketch_kind_to_backend_type becomes context-aware — takes (SketchKind, &SketchParams) and promotes to CountMinSketchWithHeap / CountSketchWithHeap when the params side carries with_heap: true.
  • Tests 8 + 9 drop their in-place JSON rewrites — the controller now plans a working heap-bearing pipeline end-to-end. They now verify the controller emits the heap-bearing aggregationType directly.
  • Test 7 (heap-less CountSketch soft-check) updated: the canonical TopK metric top_endpoint_qps now correctly emits as CountSketchWithHeap even with SketchType::CountSketch override (the metric-name TopK routing wins). Soft-check still passes — wire-format ingest is unchanged.
  • Refreshed two adjacent fixtures for the new function signature: phase_b_backend_agg_type_strings_for_every_sketch_kind (exercises both with_heap=false / true for CMS and CountSketch) and backend_json_handles_topk_and_pointcount_readouts (asserts the promoted aggregationType for topk readouts).

Test plan

  • cargo test --test e2e_controller_plans_and_backend_serves — all 9 pass with no in-test JSON patches
  • cargo test --workspace --lib — 1549 pass, 0 fail
  • Test 8 / Test 9 verify the controller-emitted aggregationType binds end-to-end through the analyzer + policy match + reducer

🤖 Generated with Claude Code

… is set

Adds a `with_heap: bool` field to `CmsParams` (parallel to the
existing `CountSketchParams.with_heap`) so the planner's CMS-Heap
binding (`bind_cms_with_heap_on_topk`) can carry the heap signal
all the way through the L4 IR to the L5 emit. `BindCmsOnCount` sets
`with_heap: false`; the topk binder sets `true`.

`sketch_kind_to_backend_type` now takes `(SketchKind, &SketchParams)`
and promotes to `CountMinSketchWithHeap` / `CountSketchWithHeap`
when the params side carries `with_heap: true`. The backend's
`policy_capability` keys on these strings to return
`FrequencyTopk(*WithHeap)`, which is what the analyzer's `topk(...)`
candidate binds against — the missing piece that previously forced
test fixtures to patch the streaming-config JSON in-place.

Test 8 + Test 9 in `e2e_controller_plans_and_backend_serves.rs`
drop the in-test JSON rewrites and now verify the controller emits
the heap-bearing aggregationType directly. Test 7 (heap-less
CountSketch soft-check on `top_endpoint_qps`) is updated to reflect
that the canonical TopK metric now correctly surfaces as
`CountSketchWithHeap` in the streaming-config — the soft-check's
real signal (wire-format ingest works) is unchanged.

Refreshed adjacent fixtures:
- `phase_b_backend_agg_type_strings_for_every_sketch_kind` now
  exercises both with_heap=false and with_heap=true for CMS and
  CountSketch.
- `backend_json_handles_topk_and_pointcount_readouts` asserts the
  promoted aggregationType for the topk readout case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 2aa360d into main May 16, 2026
@zzylol
zzylol deleted the controller-emit-heap-bearing-aggtype branch July 17, 2026 20:05
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