Repository navigation
eval: RQE expression trees in e2e tables - #225
Conversation
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
milindsrivastava1997
left a comment
milindsrivastava1997
left a comment
There was a problem hiding this comment.
Code review findings (via Claude Code).
| sort_keys=True) | ||
| if key in unique: | ||
| unique[key] = {**unique[key], "rqes": sorted(unique[key]["rqes"] + q["rqes"])} | ||
| merged = {k: v for k, v in unique[key].items() if k != "expr"} |
There was a problem hiding this comment.
When schema queries that differ only in their RQEs are merged (templates 14, 15 and 17 under --keep-duplicate-queries), expr is dropped. The merged query then has no tree, though any one of its identical RQEs could answer it. That also makes the README line "Each query but rate carries expr" wrong for the §6.3 tables. Keeping the first query's expr would fix it.
There was a problem hiding this comment.
Declining. A merged query lists several RQEs in rqes, and ASAPQuery's --planner saved (ProjectASAP/ASAPQuery#888) refuses any query whose rqes differ from its expr leaves. So keeping the first tree would make these queries fail to convert instead of just lacking a tree. The README line is in e2e-rate10/README.md, whose tables never merge queries, so it holds there. The §6.3 tables don't run against ASAPQuery's data plane, so they need no trees.
| "24h": 86400} | ||
| SERIES_LABELS = ["job", "label_0"] # a classic metric's labels | ||
| # Capabilities an expression-tree leaf can read (no rate). | ||
| EXPR_CAPABILITY = {"sum": "sum", "quantile": "quantile", "cardinality": "cardinality", |
There was a problem hiding this comment.
Nit: EXPR_CAPABILITY is mostly identity mappings. Its only real job is renaming topk to topk_by_value, so a set of leaf capabilities plus that one rename would say so directly.
There was a problem hiding this comment.
Fixed in 2ba089e: LEAF_CAPABILITIES set, and rqe_leaf() renames only topk → topk_by_value.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011gKTCAEz3sJ1Hzde16xxU1
1 participant
Closes #222. Part of #224 / ProjectASAP/ASAPQuery#886.
Every e2e query gets an
exprfield: its RQE expression tree (format in ProjectASAP/ASAPQuery#880). Leaves are self-contained (id,metric,capability,lookback_ms,grouping,phi/k,value)./node over its two quantile leavesratequeries get noexpr(not a leaf capability)--keep-duplicate-queries) gets noexprgroupingis the query's output labels (for top-k: the per-series labels it returns, as ASAPQuery'sQueryRequirements.grouping_labels)Regenerated
rqe-optimizer/results/e2e-rate10/inputs/tables/*with the same command as #217 (no re-solve); each equals the committed table onceexpris removed, so plan files and RQE ids are unchanged. classic 42/46, all 92/96, multigroup 50/50 queries haveexpr(the rest arerate).The §6.3 hydra tables (
autosketch-vs-asap-hydra/inputs/tables) are not regenerated: they come from older exporter code, so re-running changes their RQEs.Tests:
python3 scripts/test_export_autosketch_eval_table.py(34 pass).🤖 Generated with Claude Code