test(promql-compliance): roll-up suite with fixed planner output by milindsrivastava1997 · Pull Request #850 · ProjectASAP/ASAPQuery

milindsrivastava1997 · 2026-10-13T02:57:09Z

Part 3/5 of #839. Based on main (#847 and #853 merged). Stack: compliance suite (this) → MILP sketches → MILP exact types.

What

planner_output_dir suite field. Its value is a directory holding streaming_config.yaml and inference_config.yaml, relative to the suite file and checked at load time. When it's set, the runner skips the planner: a Compose override puts planner behind a profile that is never activated, and mounts the directory read-only where the planner volume would be. The legacy planner always deploys at the query's own grouping, so this is the only way to exercise roll-ups against Prometheus.

rollup suite and dataset.

Dataset: rollup_latency for 2 services × 10 endpoints, plus an endpoint that only appears at offset 600.
Aggregations: all grouped by (endpoint, service): KLL, DDSketch, Sum (Tumbling and Sliding), MinMax (Tumbling min, Sliding max).
Queries: each one is asked three ways: by (service, endpoint) (no roll-up), by (service), and with no grouping. That's 21 queries, each run as instants at 300/600/900/1200 and as a 300→1200 range.
CI: added to CI_CASES.

Results

On main: the 7 queries at the deployed grouping pass. All 14 roll-ups fail, and the old engine also mislabels the rows (service="e00", an endpoint value zipped under the service label).
On this stack: passed=true, 295/295 evaluations.
Override path checked separately: the quantiles suite passes with its planner output copied into a fixed directory.

Notes

No HLL case: no PromQL query reaches HLL (only SQL COUNT(DISTINCT) does). feat(query-engine): roll up single-subpopulation aggregations to a coarser grouping #848 covers HLL in engine tests.
Configs are hand-written: single-subpopulation Sum and MinMax aren't emitted by any planner for PromQL today.
no_cleanup: the fixed inference config uses it because several queries share each aggregation. A per-query read-count threshold could evict windows that other queries still need.

🤖 Generated with Claude Code

milindsrivastava1997

Automated code review (Claude Code): 8 findings inline.

milindsrivastava1997 · 2026-10-13T03:11:46Z

+  planner:
+    profiles: ["generated-planner-output"]
+  queryengine:
+    depends_on: !override


depends_on: !override requires Docker Compose >= v2.24.4. On older Compose the tag is rejected, or ignored so depends_on merges and queryengine still depends on the profile-disabled planner ("service planner is required by queryengine but is disabled"). Only this suite fails, so the cause is non-obvious. Consider checking docker compose version when PlannerOutputDir is set, or documenting the minimum version in HOW_TO.

Fixed in ba348ee. When fixed planner output is used with Compose, the runner first runs docker compose version --short and fails clearly if it's older than 2.24.4 (composeVersionAtLeast, unit-tested). HOW_TO also documents the minimum.

milindsrivastava1997 · 2026-10-13T03:11:46Z

+// planner's output volume would be.
+const fixedPlannerOutputCompose = `services:
+  planner:
+    profiles: ["generated-planner-output"]


The planner is disabled only by an unactivated profile, and the compose subprocess inherits os.Environ(). A caller with COMPOSE_PROFILES='*' (or this profile) exported re-enables the planner, which then runs without a generated controller config and fails. Consider stripping/overriding COMPOSE_PROFILES in lifecycle.env for fixed-output runs, or using deploy.replicas: 0 / scale: 0 instead of a profile.

Fixed in ba348ee. Fixed-output runs set COMPOSE_PROFILES= in the compose subprocess's environment. It's appended after os.Environ(), so it overrides any inherited value.

milindsrivastava1997 · 2026-10-13T03:11:46Z

+      prometheus:
+        condition: service_healthy
+    volumes:
+      - "${` + fixedPlannerOutputDirEnv + `:?` + fixedPlannerOutputDirEnv + ` is required}:` + plannerOutputMountPath + `:ro"


Short volume syntax with an unescaped host path: a resolved planner_output_dir containing : produces an invalid or misparsed spec even though resolvePlannerOutputDir accepted it. Long syntax (type: bind, source:, target:, read_only: true) avoids this.

Fixed in ba348ee. The override now uses the long bind syntax (type: bind, source, target, read_only: true), so a ':' in the host path stays part of the source. The test asserts the parsed bind.

milindsrivastava1997 · 2026-10-13T03:11:46Z

+	if err := writeEngineConfig(directory); err != nil {
+		return err
+	}
+	if suite.PlannerOutputDir != "" {


In fixed-output mode the override file is written and writePlannerConfig is skipped even when no Compose files are used. HOW_TO says the field has no effect with already-running targets, but here it silently disables planner-config generation/timing validation for those runs. Either gate on Compose being used or update HOW_TO.

Fixed in ba348ee. Fixed-output mode now applies only when Compose files are used. With caller-owned targets, the planner input is generated and validated as usual and no override is written (TestGeneratedConfigsWithoutComposeStillWritePlannerInput). HOW_TO says so.

milindsrivastava1997 · 2026-10-13T03:11:46Z

+		if err != nil {
+			return Suite{}, fmt.Errorf("query suite %q: %w", suite.Name, err)
+		}
+		suite.PlannerOutputDir = resolved


planner_output_dir is resolved to an absolute path and validated only in LoadSuiteFile. A Suite from LoadSuite(contents) keeps the raw relative string, which Run passes as ASAP_FIXED_PLANNER_OUTPUT_DIR; Compose resolves it against the first compose file's dir, not the suite dir. Consider rejecting a relative planner_output_dir in LoadSuite (or in Run) so the invariant "absolute and validated" is enforced at one place.

Fixed in ba348ee. LoadSuite now rejects a relative planner_output_dir (it has no suite file to resolve against) and validates an absolute one. So every loaded Suite holds an absolute, checked path, whichever loader produced it (TestLoadSuiteRequiresAnAbsoluteValidPlannerOutputDir).

milindsrivastava1997 · 2026-10-13T03:11:47Z

+		if _, err := parser.ParseExpr(query.Expr); err != nil {
+			t.Fatalf("parse %q: %v", query.Expr, err)
+		}
+		if _, ok := configured[query.Expr]; !ok {


This only matches suite queries against inference query keys. It doesn't check that each referenced aggregation_id exists in streaming_config.yaml or that grouping labels match the rollup dataset's labels, so a typo there only surfaces as a slow Docker-suite failure. Cheap to add here.

Fixed in ba348ee. The fixture test now also checks that every aggregation_id a query names exists in streaming_config.yaml, and that each aggregation's metric and grouping labels exist in the rollup dataset.

milindsrivastava1997 · 2026-10-13T03:11:47Z

@@ -0,0 +1,113 @@
+# Hand-written in the planner's output format. Every aggregation is a


These hand-written configs aren't tied to the planner/engine schema; if the planner output format changes (as with table_name/value_column), they drift silently. A unit test that round-trips them through the engine's config parser (or generating them from a planner run) would catch that.

Fixed in ba348ee. A new engine unit test (compliance_rollup_planner_output_loads_as_engine_config in asap-query-engine/src/utils/file_io.rs) loads both checked-in files with the engine's own read_inference_config/read_streaming_config and checks that every referenced aggregation resolves. A format change now fails cargo test.

milindsrivastava1997 · 2026-10-13T03:11:47Z

+func TestLoadSuiteFileAcceptsAbsolutePlannerOutputDir(t *testing.T) {
+	outputDirectory := t.TempDir()
+	for _, name := range []string{plannerStreamingConfigFile, plannerInferenceConfigFile} {
+		if err := os.WriteFile(filepath.Join(outputDirectory, name), []byte("{}\n"), 0o600); err != nil {


Nit: this loop duplicates the fixture-writing in writePlannerOutputSuite. Letting the helper take/return the output directory would remove the copy.

Fixed in ba348ee. The duplicated fixture-writing loop is now a shared writePlannerOutputFiles helper.

Add an optional suite-level planner_output_dir holding streaming_config.yaml and inference_config.yaml. When set, the runner validates both files at load time, skips generating the planner input, and adds a Compose override that parks the planner behind an inactive profile and mounts the directory where the query engine reads planner output. This lets the harness exercise engine configs the planner does not produce yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The rollup suite reads KLL, DDSketch, Sum and MinMax aggregations grouped by (service, endpoint), over tumbling and sliding windows, through queries grouped by (service, endpoint), by (service) and with no grouping. The dataset includes an endpoint that starts mid-range. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- Check docker compose >= 2.24.4 (needed for !override) before using it, and document it. - Clear COMPOSE_PROFILES so an inherited value can't re-enable the planner. - Bind the fixed directory with long syntax, so a ':' in its path is safe. - Only skip planner-input generation when a Compose project is used. - LoadSuite rejects a relative planner_output_dir and validates an absolute one, so every loaded suite holds an absolute, checked path. - The checked-in fixture test also checks aggregation ids, metrics and grouping labels; an engine test parses the fixed configs with the engine's own loaders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This was referenced Oct 13, 2026

feat(planner): MILP roll-ups for KLL, DDSketch and HLL #851

Merged

feat(planner): MILP roll-ups for exact sum, count, min and max #852

Merged

milindsrivastava1997 commented Oct 13, 2026

View reviewed changes

milindsrivastava1997 force-pushed the 839-2-engine-rollup branch from 86b14fe to 5bdfa3c Compare October 13, 2026 03:40

milindsrivastava1997 force-pushed the 839-3-compliance-rollup-suite branch from c460836 to 49426b5 Compare October 13, 2026 13:11

milindsrivastava1997 changed the base branch from 839-2-engine-rollup to main October 13, 2026 13:11

milindsrivastava1997 and others added 2 commits October 13, 2026 09:30

milindsrivastava1997 force-pushed the 839-3-compliance-rollup-suite branch from 49426b5 to 901cf84 Compare October 13, 2026 13:33

milindsrivastava1997 merged commit 5844ea8 into main Oct 13, 2026
8 checks passed

		@@ -0,0 +1,113 @@
		# Hand-written in the planner's output format. Every aggregation is a

Conversation

milindsrivastava1997 commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Results

Notes

Uh oh!

milindsrivastava1997 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

milindsrivastava1997 commented Oct 13, 2026 •

edited

Loading