Skip to content

eval: RQE expression trees for random-manifest tables - #228

Merged
milindsrivastava1997 merged 8 commits into
eval/random-manifestfrom
227-eval-rqe-expression-trees-for-random-manifest-tables
Oct 15, 2026
Merged

milindsrivastava1997 merged 8 commits into
eval/random-manifestfrom
227-eval-rqe-expression-trees-for-random-manifest-tables

Conversation

@milindsrivastava1997

Copy link
Copy Markdown
Contributor

Closes #227. Part of ProjectASAP/ASAPQuery#886. Stacked on #226, with #225 merged in for rqe_leaf().

--random-manifest tables get an expr on each e2e query. Each tree's values come from the manifest's RQE, and its shape from the RQE's capability plus a full match on the query's PromQL. The PromQL by labels must equal the RQE's grouping. The shapes:

  • avg by (G) (quantile_over_time(phi, data[W])) → {"agg":"avg","by":G,"of":<per-series quantile leaf>}
  • topk by (G) (k, sum_over_time(...)) → top-k leaf with per-series grouping, topk_by: G, k (a global topk leaves out topk_by)
  • sum by (G) (sum_over_time(...)) and spatial quantile by (G) → one leaf grouped by G
  • SQL cardinality → a leaf on events with value. These queries don't reach a table yet, because random_manifest_table keeps only common-promql.
  • counter-based top-k, rate and other shapes → no expr

Checked on the workload-01 pilot:

  • the table equals the committed pilot eval_table.json apart from expr (and the input hash/revision);
  • converted with ASAPQuery's --planner saved (#888 + topk_by, plus af8888a from feat(asap-tools): add accelerated random E2E workload artifacts ASAPQuery#879 for the per-series avg PromQL text), all 7 leaves get an rqe_configs entry: the coarse sum rolls up from the fine sum, the 3 quantiles share one DDSketch, and the 2 top-k leaves read their heaps;
  • streaming config unchanged; inference config differs only by rqe_configs.

Tests: python3 scripts/test_export_autosketch_eval_table.py (43 pass).

🤖 Generated with Claude Code

milindsrivastava1997 and others added 3 commits October 14, 2026 18:29
Every e2e query gets `expr`, its RQE expression tree: a leaf per RQE it
reads (id, metric or table, capability, lookback_ms, grouping, phi/k/value),
and a `/` node for template 10. Rate queries, and schema queries merged
across several RQEs (--keep-duplicate-queries), get none.

Regenerate the e2e-rate10 tables: only `expr` is added, so the RQE ids and
plans are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1
…-e2e-tables' into 227-eval-rqe-expression-trees-for-random-manifest-tables
--random-manifest e2e queries carry `expr` (#227, ProjectASAP/ASAPQuery#886):
avg by (G) of per-series quantile_over_time is an avg over a per-series
quantile leaf; topk by (G) of sum_over_time a per-series top-k leaf with
`topk_by` G (left out for a global top-k); sum by (G) of sum_over_time and
spatial quantile by (G) a leaf at G; SQL cardinality a leaf on its table
with `value`. Counter-gated top-k and rate have none. Leaf ids are the
table's RQE ids; values come from the manifest's RQE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1

@milindsrivastava1997 milindsrivastava1997 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review findings (via Claude Code).

Comment thread scripts/export_autosketch_eval_table.py Outdated
return {"agg": "avg", "by": grouping,
"of": leaf("quantile", series_labels, phi=rqe["phi"])}
if shape == "quantile":
return leaf("quantile", grouping, phi=rqe["phi"])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For quantile by (G) (phi, data) the leaf is grouped by G, but random_manifest_table still builds every quantile RQE per series (grouping = names, min_N sized per series). The tree reads a G-level quantile from an RQE planned for per-series accuracy. No test covers this path; the spatial-quantile test calls random_expr on its own.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d79ddb0: the spatial quantile by (G) shape and its test are removed (no seed-862 query has it; #226 now also rejects any quantile that isn't an avg of per-series quantiles).

Comment thread scripts/export_autosketch_eval_table.py Outdated
else:
return None
by = [name.strip() for name in (match["by"] or "").split(",") if name.strip()]
assert by == grouping, f"{query['id']}: by {by} is not its grouping {grouping}"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by == grouping compares lists in order, so sum by (service, region) with grouping_labels ['region', 'service'] aborts --random-manifest, though the query is valid. Comparing sets (like the Rust validator's label_set) would accept it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by d79ddb0: the tree is built from grouping_labels directly, so this code no longer compares label lists. #226's own quantile check still compares in order, which is harmless today because rqe_query() renders labels in grouping_labels order.

Comment thread scripts/export_autosketch_eval_table.py Outdated
if capability == "cardinality" and query.get("sql") is not None:
return leaf("cardinality", grouping, value=rqe["value"])
for shape, pattern in RANDOM_EXPR_SHAPES.get(capability, []):
match = re.fullmatch(pattern, query.get("promql") or "")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This re-parses the manifest's PromQL with regexes to recover capability, grouping, phi and k, which the RQE fields already hold (asap-tools' rqe_query() builds the same Query from them alone). Any change to the PromQL text in random_workloads.py (phi formatting, a without clause, spacing) silently drops the tree. Building the tree from the RQE fields would be shorter and sturdier.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d79ddb0: random_expr builds the tree from the RQE fields only (quantile → avg over a per-series leaf, top-k → leaf with topk_by = grouping, sum → leaf grouped by G). It no longer parses PromQL. Checked against rqe_query()'s rendering.

Comment thread scripts/export_autosketch_eval_table.py Outdated
return rqe_leaf(rqe_id, rqe["metric"], capability, rqe["lookback_ms"] / 1000, grouping,
**extra)

if capability == "cardinality" and query.get("sql") is not None:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch can't run from the only caller: random_manifest_table skips every query not marked common-promql, and every cardinality query is SQL-only. Consider deleting it along with test_sql_cardinality_is_a_leaf_on_its_table.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d79ddb0: the SQL-cardinality branch and its test are deleted; it can come back when random_manifest_table includes SQL-only queries.

return base if metrics == 1 else f"{base}/metrics={metrics}"


def rqe_leaf(rqe, metric, capability, lookback_s, grouping, phi=None, k=None, value=None,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR merges #225 into the #226 branch and then changes rqe_leaf (adds topk_by), so its diff repeats all of #225. Merge order needs to be 225 and 226 into main, then retarget this PR to main; otherwise reviewers see 225's changes twice and the branches conflict.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed: this is a stacked PR. Plan: merge #225 and #226 into main, then retarget this PR to main (or merge main into it), and the diff shrinks to its own change. Latest #225 and #226 are merged in (a360ded, c313c1f).

milindsrivastava1997 and others added 5 commits October 14, 2026 21:31
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1
…-e2e-tables' into 227-eval-rqe-expression-trees-for-random-manifest-tables

# Conflicts:
#	scripts/export_autosketch_eval_table.py
…val-rqe-expression-trees-for-random-manifest-tables

# Conflicts:
#	scripts/export_autosketch_eval_table.py
#	scripts/test_export_autosketch_eval_table.py
random_expr() builds each tree from the manifest RQE's capability,
grouping_labels, phi and k, as asap-tools' rqe_query() renders them:
quantile is avg by G of a per-series quantile leaf, topk_by_value a
per-series top-k leaf ranked within G, sum a leaf at G. No PromQL
regexes, so no list-order label comparison; the spatial quantile shape
(read as a group quantile from a per-series RQE) and the unreachable SQL
cardinality branch go. A test builds seed-862 workload-01's table and
checks every tree names its query's RQE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1
…for-random-manifest-tables

Takes #226's shared-RQE e2e entries and gives every entry, including one
reading a shared RQE, its tree (e2e_query()).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1
@milindsrivastava1997
milindsrivastava1997 merged commit d46f219 into eval/random-manifest Oct 15, 2026
milindsrivastava1997 added a commit that referenced this pull request Oct 15, 2026
#225's rqe_leaf() conflicts with #228's, which adds topk_by; kept #228's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milindsrivastava1997 added a commit that referenced this pull request Oct 15, 2026
* eval: export ASAPQuery random workload manifests

--random-manifest writes the eval table of an ASAPQuery random workload
(asap-tools random_workloads.py, ProjectASAP/ASAPQuery#879): its
common-promql queries as schema RQEs on metric `data`, so a fine sum
can serve a coarse one, quantiles per series (avg of per-series
quantiles), and top-k heaps below the curves' 1,000 keys read there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* eval: address review of --random-manifest

- closed_labels() takes a label list, so random manifests reuse it: a
  child-only grouping counts its parents' groups instead of failing.
- Other capabilities are left out (logged); a quantile that isn't an avg
  of per-series quantiles, an accuracy_sla other than 0.95, and a rate
  that doesn't divide 1000 are refused.
- RQE ids carry the exact range; duplicate ids are refused.
- --random-manifest refuses the flags it would ignore.

Workload 01's table is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* eval: one RQE for random-manifest queries reading the same one

Per-series quantiles that differ only in their avg's grouping read the
same per-series RQE; ASAPQuery's --planner saved refuses one leaf naming
two RQEs. They now share the first query's id. Workloads 03, 04, 06, 08
and 09 of seed 862 hit this; workload 01's table is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* eval: address second review of --random-manifest

- RQEs below the curves' first N (1e3 items per group) are left out
  (logged), as schema_queries() does.
- A sub-second interval is a ValueError, not an assert; a bool rate is
  refused; divides_1000() is shared with --samples-per-sec.
- A repeated query is one e2e entry.
- The curve flags are refused with --random-manifest.
- Each RQE entry is built once; one table writer for both modes.

Seed 862's ten tables are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* eval: address third review of --random-manifest

- The curves' first-N check skips exact families, which read no curve.
- A lookback that isn't whole scrapes, or a query reading other than one
  RQE, is a ValueError naming the query.
- check_unique_ids() is shared with build(); the e2e dedup is a set.

Seed 862's ten tables are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* eval: RQE expression trees for random-manifest tables (#228)

* eval: RQE expression trees in e2e tables

Every e2e query gets `expr`, its RQE expression tree: a leaf per RQE it
reads (id, metric or table, capability, lookback_ms, grouping, phi/k/value),
and a `/` node for template 10. Rate queries, and schema queries merged
across several RQEs (--keep-duplicate-queries), get none.

Regenerate the e2e-rate10 tables: only `expr` is added, so the RQE ids and
plans are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1

* eval: RQE expression trees for random-manifest tables

--random-manifest e2e queries carry `expr` (#227, ProjectASAP/ASAPQuery#886):
avg by (G) of per-series quantile_over_time is an avg over a per-series
quantile leaf; topk by (G) of sum_over_time a per-series top-k leaf with
`topk_by` G (left out for a global top-k); sum by (G) of sum_over_time and
spatial quantile by (G) a leaf at G; SQL cardinality a leaf on its table
with `value`. Counter-gated top-k and rate have none. Leaf ids are the
table's RQE ids; values come from the manifest's RQE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1

* eval: leaf capabilities as a set, renaming only topk

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1

* eval: random-manifest trees from RQE fields, not PromQL

random_expr() builds each tree from the manifest RQE's capability,
grouping_labels, phi and k, as asap-tools' rqe_query() renders them:
quantile is avg by G of a per-series quantile leaf, topk_by_value a
per-series top-k leaf ranked within G, sum a leaf at G. No PromQL
regexes, so no list-order label comparison; the spatial quantile shape
(read as a group quantile from a per-series RQE) and the unreachable SQL
cardinality branch go. A test builds seed-862 workload-01's table and
checks every tree names its query's RQE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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