Skip to content

Commit 439dcd9

Browse files
zzylolclaude
andcommitted
test(level1): assert candidate membership and account for every rejection
Level 1 declared one expected plan per query while its stated purpose was to enumerate the candidate inventory before pricing. A single expected shape is a selection assertion in structural clothing, and four of the ten queries carried no expectation at all. Replace it with a membership contract. Each query declares the shapes the inventory must expose and how this fixture resolves them: MustBind, or MustReject with a named policy reason. The heap families Planner exposes for spatial-topk and topk-rate are now required to be present *and* refused, which is a positive assertion about the inventory rather than silence about it. Separate policy refusals from defects. A rejection that is neither a declared policy reason nor a recorded defect now fails the test. Three candidates fail with "Planner logical fragment does not match any original query subtree" and one with a schema incompatibility; main is green on these paths, so each is a regression introduced inside the #737 -> #728 stack. They are listed in KNOWN_BINDING_DEFECTS with exact occurrence counts, so a new instance fails and a fixed one forces its entry to be deleted. All must be gone before #775 installs and executes these candidates. Stop committing derived plans. candidates/ and ensembles/ were ~94k lines, are fully reproducible from the test, and every identity in them changes when the Planner pin moves; CI already uploads them on every run. Only the per-query admission reports stay in the tree, and they now reproduce byte-for-byte from this build. Historical synthetic-price exports move to archive/, and the README's hand-copied Planner revision is dropped in favour of reading the pin from Cargo.toml, since the copied one had gone stale. The sort-key contract test now compiles the query instead of reading a committed export, so it no longer depends on plans from an older Planner. A certified companion fixture for the heap candidates is NOT included: the issue-754 generator cannot supply one. Scoped evidence needs topk_selected_lower_bound > topk_excluded_upper_bound, and under the generator's own per-series domain the third- and fourth-ranked series overlap in both groups, while these queries evaluate in real_time scope so the bound must hold across the whole validity window. That needs a generator with non-overlapping per-series domains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1668df1 commit 439dcd9

166 files changed

Lines changed: 452 additions & 51191 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎control_plane/tests/issue754_level1.rs‎

Lines changed: 227 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,131 @@ use serde_json::{json, Value};
1616
mod workload;
1717
use workload::Suite;
1818

19+
/// Level 1 asserts *membership*: which shapes the candidate inventory must
20+
/// expose, and which it must refuse and why. It never names a winner, because
21+
/// ranking is #742's contract and this layer prices nothing.
22+
///
23+
/// A required shape declares how it must resolve. `MustBind` shapes have to
24+
/// reach `AwaitingQuote`; `MustReject` shapes have to appear in the inventory
25+
/// and then fail admission for the stated policy reason. The same shape can be
26+
/// `MustReject` under the strict fixture and `MustBind` under the certified
27+
/// one, so the fixture, not a second test path, carries the difference.
28+
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
29+
enum Resolution {
30+
// No fixture constructs this yet: the issue-754 generator cannot certify a
31+
// heap (see `required_summary_shapes`), so every required shape here is a
32+
// rejection. The variant is the half of the contract a certified fixture
33+
// fills in, and deleting it would delete that contract.
34+
#[allow(dead_code)]
35+
MustBind,
36+
MustReject(PolicyReason),
37+
}
38+
39+
/// Reasons a fixture may legitimately refuse an otherwise well-formed
40+
/// candidate. Anything outside this set is a defect, not a policy decision.
41+
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
42+
enum PolicyReason {
43+
/// The fixture supplies no evidence that certifies the summary readout.
44+
NoCertifiedGuarantee,
45+
/// The realized family cannot meet the query's declared accuracy target.
46+
AccuracyTargetUnmet,
47+
}
48+
49+
impl PolicyReason {
50+
fn matches(self, reason: &str) -> bool {
51+
match self {
52+
Self::NoCertifiedGuarantee => reason.contains("accuracy guarantee"),
53+
Self::AccuracyTargetUnmet => reason.contains("does not satisfy"),
54+
}
55+
}
56+
57+
const ALL: &'static [Self] = &[Self::NoCertifiedGuarantee, Self::AccuracyTargetUnmet];
58+
}
59+
60+
/// Sketch/heap families each query must expose, with how this fixture resolves
61+
/// them. The strict issue-754 fixture certifies no heap, so every heap family
62+
/// is required to be present *and* rejected; that is a positive assertion about
63+
/// the inventory, not silence about it.
64+
///
65+
/// A certified companion fixture cannot be derived from this generator. Scoped
66+
/// evidence needs `topk_selected_lower_bound > topk_excluded_upper_bound`, and
67+
/// under the generator's own per-series domain (`multiplier * base` to
68+
/// `multiplier * (base + modulo)`, as `issue754_workload` reads it for the
69+
/// quantile operands) the third- and fourth-ranked series overlap in both
70+
/// groups: group `a` selects down to 40 while excluding a series reaching 130,
71+
/// and group `b` selects down to 360 while excluding one reaching 850. The
72+
/// pointwise ordering never actually changes, but the queries evaluate in
73+
/// `real_time` scope, so the contract must hold across the whole validity
74+
/// window, not at one instant. Certifying a heap needs a generator with
75+
/// non-overlapping per-series domains.
76+
fn required_summary_shapes(name: &str) -> &'static [(&'static str, Resolution)] {
77+
match name {
78+
"spatial-topk" => &[(
79+
"CountSketchWithHeap",
80+
Resolution::MustReject(PolicyReason::NoCertifiedGuarantee),
81+
)],
82+
"topk-rate" => &[
83+
(
84+
"CmsWithHeap",
85+
Resolution::MustReject(PolicyReason::NoCertifiedGuarantee),
86+
),
87+
(
88+
"CountSketchWithHeap",
89+
Resolution::MustReject(PolicyReason::NoCertifiedGuarantee),
90+
),
91+
],
92+
_ => &[],
93+
}
94+
}
95+
96+
/// Binding defects this fixture reaches today. Level 1 records them rather than
97+
/// tolerating them silently: each entry must still occur exactly `count` times,
98+
/// so a new instance fails the test and a fixed one forces its line to be
99+
/// deleted. `main` is green on these paths, so every entry is a regression
100+
/// introduced inside the #737 → #728 stack, and all of them must be gone before
101+
/// #775 installs and executes these candidates.
102+
const KNOWN_BINDING_DEFECTS: &[(&str, &str, usize)] = &[
103+
(
104+
"grouped-rate",
105+
"Planner logical fragment does not match any original query subtree",
106+
1,
107+
),
108+
(
109+
"grouped-rate",
110+
"physical program has incompatible input or result schema",
111+
1,
112+
),
113+
(
114+
"grouped-temporal-sum",
115+
"Planner logical fragment does not match any original query subtree",
116+
1,
117+
),
118+
(
119+
"spatial-topk",
120+
"Planner logical fragment does not match any original query subtree",
121+
1,
122+
),
123+
];
124+
125+
/// Classify one admission rejection. A reason that is neither a declared policy
126+
/// refusal nor a recorded defect fails the query outright.
127+
fn assert_rejection_is_accounted_for(name: &str, reason: &str) {
128+
if PolicyReason::ALL
129+
.iter()
130+
.any(|policy| policy.matches(reason))
131+
{
132+
return;
133+
}
134+
assert!(
135+
KNOWN_BINDING_DEFECTS
136+
.iter()
137+
.any(|(query, defect, _)| *query == name && reason.contains(defect)),
138+
"{name}: unaccounted binding rejection: {reason}\n\
139+
Add a policy reason if this is a deliberate refusal, or fix the \
140+
defect. Level 1 does not accept unexplained bind failures."
141+
);
142+
}
143+
19144
struct ExpectedPlan {
20145
family: Option<ExpectedFamily>,
21146
partitioning: &'static str,
@@ -152,15 +277,43 @@ fn assert_rate_sort_expression(dag: &asap_types::executable_plan::OwnedPostAsapD
152277
}
153278

154279
// A descending sort over a timestamp or label must fail the Level 1 contract.
280+
/// Compile the topk-rate query and return every ranking DAG the inventory
281+
/// exposes. Reading a committed export instead would pin the contract to plans
282+
/// generated by an older Planner revision; candidate identities do not survive
283+
/// a Planner bump, so the fixture has to be produced by the build under test.
284+
fn compiled_topk_rate_sort_dags() -> Vec<asap_types::executable_plan::OwnedPostAsapDag> {
285+
let case = workload::suite()
286+
.queries
287+
.into_iter()
288+
.find(|case| case.name == "topk-rate")
289+
.expect("topk-rate left the issue-754 suite");
290+
let (request, environment) = workload::input(&case)
291+
.into_physical_compilation_request()
292+
.unwrap();
293+
let mut dags = Vec::new();
294+
for candidate in enumerate_exact_and_materialized_candidates(request).unwrap() {
295+
let Ok(plan) = DeploymentPlanCompiler.compile_promql(candidate, environment.clone()) else {
296+
continue;
297+
};
298+
dags.extend(
299+
plan.query_plan
300+
.selected_dags
301+
.values()
302+
.filter(|dag| {
303+
dag.nodes
304+
.iter()
305+
.any(|node| node.payload["operation"].get("Sort").is_some())
306+
})
307+
.cloned(),
308+
);
309+
}
310+
assert!(!dags.is_empty(), "topk-rate exposes no ranking DAG");
311+
dags
312+
}
313+
155314
#[test]
156315
fn topk_rate_sort_contract_rejects_wrong_value_expression() {
157-
let artifact: Value = serde_json::from_str(include_str!(
158-
"../../docs/evaluation/issue754-human-review/topk-rate.json"
159-
))
160-
.unwrap();
161-
let dag: asap_types::executable_plan::OwnedPostAsapDag =
162-
serde_json::from_value(artifact["query_plan"]["selected_dags"]["compat-query-0"].clone())
163-
.unwrap();
316+
let dag = compiled_topk_rate_sort_dags().remove(0);
164317
assert_rate_sort_expression(&dag);
165318
for column in [0, 2] {
166319
let mut wrong = dag.clone();
@@ -694,15 +847,11 @@ fn issue754_queries_have_valid_physical_plans() {
694847
"root substitutions must not be reported as exhaustive joint search"
695848
);
696849
}
697-
let heap_families: &[&str] = match case.name.as_str() {
698-
"spatial-topk" => &["CountSketchWithHeap"],
699-
"topk-rate" => &["CmsWithHeap", "CountSketchWithHeap"],
700-
_ => &[],
701-
};
850+
let heap_families = required_summary_shapes(&case.name);
702851
let mut heap_roots =
703852
heap_families
704853
.iter()
705-
.map(|family| {
854+
.map(|(family, resolution)| {
706855
let trace = request
707856
.planner_selection_trace
708857
.iter()
@@ -730,11 +879,14 @@ fn issue754_queries_have_valid_physical_plans() {
730879
);
731880
assert!(trace["guarantee"].to_string().contains("topk_max_distinct_items"),
732881
"fixture lacks an enforced bound; cardinality estimates cannot certify a heap");
733-
trace["logical_root_id"].as_str().unwrap().to_owned()
882+
(
883+
trace["logical_root_id"].as_str().unwrap().to_owned(),
884+
*resolution,
885+
)
734886
})
735887
.collect::<Vec<_>>();
736888
if case.name == "topk-rate" {
737-
for family in heap_families {
889+
for (family, resolution) in heap_families {
738890
let trace = request
739891
.planner_selection_trace
740892
.iter()
@@ -758,7 +910,10 @@ fn issue754_queries_have_valid_physical_plans() {
758910
.query
759911
.input_contracts()
760912
.all(|(id, _)| split.materialized_outputs.contains_key(&id)));
761-
heap_roots.push(trace["logical_root_id"].as_str().unwrap().to_owned());
913+
heap_roots.push((
914+
trace["logical_root_id"].as_str().unwrap().to_owned(),
915+
*resolution,
916+
));
762917
}
763918
}
764919
let candidates = enumerate_exact_and_materialized_candidates(request).unwrap();
@@ -777,17 +932,47 @@ fn issue754_queries_have_valid_physical_plans() {
777932
);
778933
match result.status {
779934
CandidateEvaluationStatus::AwaitingQuote => assert!(result.plan_id.is_some()),
780-
CandidateEvaluationStatus::CompilationFailed => assert!(result
781-
.unavailable_reason
782-
.as_ref()
783-
.is_some_and(|reason| !reason.is_empty())),
935+
CandidateEvaluationStatus::CompilationFailed => {
936+
let reason = result.unavailable_reason.as_deref().unwrap_or_default();
937+
assert!(
938+
!reason.is_empty(),
939+
"{}: bind failure without a reason",
940+
case.name
941+
);
942+
assert_rejection_is_accounted_for(&case.name, reason);
943+
}
784944
ref status => panic!("unexpected pre-pricing status: {status:?}"),
785945
}
786946
}
947+
// Each recorded defect must still occur exactly as often as declared:
948+
// a new occurrence and a silently fixed one both fail here.
949+
for (query, defect, count) in KNOWN_BINDING_DEFECTS {
950+
if *query != case.name {
951+
continue;
952+
}
953+
let observed = admission
954+
.iter()
955+
.filter(|result| {
956+
result
957+
.unavailable_reason
958+
.as_deref()
959+
.is_some_and(|reason| reason.contains(defect))
960+
})
961+
.count();
962+
assert_eq!(
963+
observed, *count,
964+
"{}: recorded defect count changed for {defect:?}; \
965+
update KNOWN_BINDING_DEFECTS (delete the entry once fixed)",
966+
case.name
967+
);
968+
}
787969
if let Ok(directory) = std::env::var("ASAP_LEVEL1_ARTIFACT_DIR") {
788-
std::fs::create_dir_all(&directory).unwrap();
970+
// Mirror the committed layout: `admission/` is the contract, so a
971+
// downloaded CI artifact drops straight onto the repository copy.
972+
let admission_dir = std::path::Path::new(&directory).join("admission");
973+
std::fs::create_dir_all(&admission_dir).unwrap();
789974
std::fs::write(
790-
std::path::Path::new(&directory).join(format!("{}.admission.json", case.name)),
975+
admission_dir.join(format!("{}.admission.json", case.name)),
791976
serde_json::to_vec_pretty(&admission).unwrap(),
792977
)
793978
.unwrap();
@@ -880,7 +1065,7 @@ fn issue754_queries_have_valid_physical_plans() {
8801065
case.name
8811066
);
8821067
}
883-
for root_id in heap_roots {
1068+
for (root_id, resolution) in heap_roots {
8841069
let heap = admission
8851070
.iter()
8861071
.filter(|candidate| candidate.logical_root_ids.contains(&root_id))
@@ -889,16 +1074,25 @@ fn issue754_queries_have_valid_physical_plans() {
8891074
!heap.is_empty(),
8901075
"physical heap candidate disappeared before admission"
8911076
);
892-
assert!(
893-
heap.iter().all(|candidate| candidate.status
894-
== CandidateEvaluationStatus::CompilationFailed
895-
&& candidate.total_cost.is_none()
896-
&& candidate
897-
.unavailable_reason
898-
.as_ref()
899-
.is_some_and(|reason| reason.contains("accuracy guarantee"))),
900-
"missing proof must be an explicit admission failure: {heap:?}"
901-
);
1077+
match resolution {
1078+
Resolution::MustBind => assert!(
1079+
heap.iter().all(|candidate| candidate.status
1080+
== CandidateEvaluationStatus::AwaitingQuote
1081+
&& candidate.plan_id.is_some()
1082+
&& candidate.total_cost.is_none()),
1083+
"certified shape must reach pricing unpriced: {heap:?}"
1084+
),
1085+
Resolution::MustReject(policy) => assert!(
1086+
heap.iter().all(|candidate| candidate.status
1087+
== CandidateEvaluationStatus::CompilationFailed
1088+
&& candidate.total_cost.is_none()
1089+
&& candidate
1090+
.unavailable_reason
1091+
.as_deref()
1092+
.is_some_and(|reason| policy.matches(reason))),
1093+
"missing proof must be an explicit {policy:?} admission failure: {heap:?}"
1094+
),
1095+
}
9021096
}
9031097
}
9041098
}

‎docs/design_docs/planning-test-layers.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ ERP collection, feedback-driven replanning and deployment switching are deferred
88

99
| PR | Input | Assertion |
1010
| --- | --- | --- |
11-
| #728: structure | workload and declared execution/accuracy contracts | Every supported candidate has a valid physical DAG or an explicit binding rejection; no prices or winner assertions |
11+
| #728: structure | workload and declared execution/accuracy contracts | Membership: required shapes appear in the inventory and resolve as declared (bind, or refuse for a named policy reason); every other rejection is a recorded defect with a fixed occurrence count. No prices, no winner |
1212
| #742: ranking | same candidates plus synthetic complete quotes | Selection follows costs, reverses with costs, and excludes unavailable candidates |
1313
| #775: execution | workload, synthetic quotes, finite fixture data | Mutate quotes to select each admitted candidate, install and execute it; validate every query result and execution provenance |
1414

@@ -37,6 +37,14 @@ once, ingests its fixture once, and checks every query in that workload.
3737
under the fixture's contracts, not an exhaustive Cartesian product of hypothetical
3838
query plans. Binding rejections remain visible; prices cannot override them.
3939

40+
A rejection is not self-justifying. Level 1 separates *policy* refusals — the
41+
fixture declining to certify a candidate, which are asserted as required
42+
outcomes — from *defects*, which are bugs recorded with exact occurrence counts
43+
so that neither a new instance nor a silent fix can pass unnoticed. Level 1's
44+
enumerated candidate plans and ensembles are CI artifacts, not repository
45+
content: they are fully derived, and their identities change with the Planner
46+
pin. Only the per-query admission reports are committed.
47+
4048
## Synthetic cost scope
4149

4250
Prices are deterministic test inputs, not resource measurements. The execution

0 commit comments

Comments
 (0)