Skip to content

feat(controller): drop aggregationId from asapquery_backend YAML emission - #152

Merged
zzylol merged 1 commit into
mainfrom
feat/controller-drop-agg-id-emission
May 13, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/controller-drop-agg-id-emission

Conversation

@zzylol

@zzylol zzylol commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Phase 5 M2.2 — completes the controller→backend aggregation_id handoff.
  • controller::emit::asapquery_backend::generate_streaming_config_yaml no longer writes the aggregationId field.
  • deterministic_agg_id helper + its dedicated test are removed (~25 LOC).
  • Updated the in-process feedback-loop test comment in data_plane::asap_query_engine::engine to reflect the new contract.

Backend-side derivation of the id (via compute_agg_config_id from M2.1 / PR #151) over the same fields the emitter writes makes the explicit aggregationId field redundant. Existing YAML fixtures + tests that mint explicit ids still work — from_yaml_data honors them for backwards compatibility.

Test plan

  • New emitter test emitted_yaml_omits_aggregation_id asserts the field is absent from output.
  • cargo test -p controller --lib emit::asapquery_backend — 5/5 pass.
  • cargo test -p controller --lib — 710/710 pass.
  • cargo test -p data_plane --lib capability_miss_feedback_loop_closes — both feedback-loop tests pass; explicit-id round-trip still verified.
  • cargo check --workspace clean.

What's next

M2.3: migrate SchemaRegistry / SketchStore from agg_id-keyed to sid-keyed lookup (~362 references across 38 files; deeper refactor — needs decomposition).

🤖 Generated with Claude Code

…sion

Phase 5 M2.2 — completes the controller→backend aggregation_id handoff.
`controller::emit::asapquery_backend::generate_streaming_config_yaml`
no longer writes the `aggregationId` field; the
`deterministic_agg_id` helper (and its dedicated test) is removed.

The backend's `AggregationConfig::from_yaml_data` now derives the id
deterministically via `compute_agg_config_id` (introduced in M2.1 /
PR #151) over the same fields the emitter writes, so the explicit
field was redundant.

In-tree YAML fixtures + the in-process feedback-loop test still mint
explicit ids; that path is exercised unchanged because explicit
`aggregationId` is honored for backwards compatibility.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 60f7149 into main May 13, 2026
zzylol added a commit that referenced this pull request May 13, 2026
Schema retirement #2 of 5. The `/api/v1/db/timeline` handler now
reads from `state.sketch_index` (always attached) via
`sketch_db::query::timeline::timeline_for_metric` (PR #183) instead
of `state.schemas.timeline_for_metric`.

User-visible behavior:

- Endpoint always works (no more 503 when schema-registry isn't
  wired — the sid catalog is always present).
- `agg_id` field carries a stable content-derived signature id
  (xxh64 of `metric + agg_kind + group_by_keys`) instead of the
  controller-emitted `agg_id` (which is gone after M2.2 / PR #152).
- Segments now reflect the sid catalog directly; reconfigure
  semantics propagate once the next sub-PR (lifecycle reconcile)
  lands.

Test impact:

- `test_get_timeline_without_registry_returns_503` rewritten to
  `..._with_no_sids_returns_empty_200` matching the new semantics.
- `test_get_timeline_returns_segments_after_reconfigure` ignored
  (uses POST /streaming-config → SchemaRegistry::reconcile, which
  doesn't yet propagate to the sid catalog; re-enabled in the next
  sub-PR with the lifecycle reconcile).

789/792 lib tests pass (3 ignored — 2 pre-existing + this one).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the feat/controller-drop-agg-id-emission 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