Skip to content

Commit 18de8b8

Browse files
strefethenclaude
strefethen
and
claude
committed
feat(expr): add expression evaluation diagnostics
- Add ExpressionWarning type for collecting resolution failures - Add evaluate_with_diagnostics() returning (Value, Vec<Warning>) - Warn on: missing step, missing output key, unknown namespace, missing input, missing source description - Wire warnings into CriterionEvaluation and TraceCriterionResult - Trace output now includes expression warnings per criterion - Backward compatible: evaluate() still returns Value only Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c658948 commit 18de8b8

File tree

4 files changed

+211
-55
lines changed

4 files changed

+211
-55
lines changed

‎crates/arazzo-expr/src/lib.rs‎

Lines changed: 202 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,21 @@ impl std::fmt::Display for PathError {
2828

2929
impl std::error::Error for PathError {}
3030

31+
/// Warning produced when an expression resolves to `Null` due to a missing key,
32+
/// unknown step, or unrecognised namespace. Collected by
33+
/// [`ExpressionEvaluator::evaluate_with_diagnostics`].
34+
#[derive(Debug, Clone, PartialEq, Eq)]
35+
pub struct ExpressionWarning {
36+
pub expression: String,
37+
pub message: String,
38+
}
39+
40+
impl std::fmt::Display for ExpressionWarning {
41+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
42+
write!(f, "{}: {}", self.expression, self.message)
43+
}
44+
}
45+
3146
static INTERPOLATE_RE: Lazy<Regex> = Lazy::new(|| {
3247
Regex::new(r"\{(\$[^}]+)\}|\$([a-zA-Z_][a-zA-Z0-9_\.]*(?:\[[0-9]+\])*)")
3348
.unwrap_or_else(|err| panic!("failed to compile interpolate regex: {err}"))
@@ -71,122 +86,195 @@ impl ExpressionEvaluator {
7186
}
7287

7388
/// Evaluate an expression and return a dynamic JSON value.
89+
///
90+
/// Missing keys and unknown namespaces silently return `Value::Null`.
91+
/// Use [`evaluate_with_diagnostics`](Self::evaluate_with_diagnostics) to
92+
/// collect warnings about unresolved expressions.
7493
pub fn evaluate(&self, expr: &str) -> Value {
94+
self.evaluate_with_diagnostics(expr).0
95+
}
96+
97+
/// Evaluate an expression, returning both the value and any diagnostic
98+
/// warnings produced when resolution falls back to `Null`.
99+
pub fn evaluate_with_diagnostics(&self, expr: &str) -> (Value, Vec<ExpressionWarning>) {
100+
let mut warnings = Vec::new();
101+
75102
let Some(rest) = expr.strip_prefix('$') else {
76-
return Value::String(expr.to_string());
103+
return (Value::String(expr.to_string()), warnings);
77104
};
78105

79106
if let Some(name) = rest.strip_prefix("env.") {
80-
return Value::String(env::var(name).unwrap_or_default());
107+
return (Value::String(env::var(name).unwrap_or_default()), warnings);
81108
}
82109

83110
if let Some(name) = rest.strip_prefix("inputs.") {
84-
return self.ctx.inputs.get(name).cloned().unwrap_or(Value::Null);
111+
let value = self.ctx.inputs.get(name).cloned().unwrap_or_else(|| {
112+
warnings.push(ExpressionWarning {
113+
expression: expr.to_string(),
114+
message: format!("input \"{name}\" not found in context"),
115+
});
116+
Value::Null
117+
});
118+
return (value, warnings);
85119
}
86120

87121
if let Some(after) = rest.strip_prefix("steps.") {
88122
if let Some((step_id, output_name)) = after.split_once(".outputs.") {
89-
return self
90-
.ctx
91-
.steps
92-
.get(step_id)
93-
.and_then(|outputs| outputs.get(output_name))
94-
.cloned()
95-
.unwrap_or(Value::Null);
123+
let value = match self.ctx.steps.get(step_id) {
124+
Some(outputs) => outputs.get(output_name).cloned().unwrap_or_else(|| {
125+
warnings.push(ExpressionWarning {
126+
expression: expr.to_string(),
127+
message: format!(
128+
"output \"{output_name}\" not found in step \"{step_id}\""
129+
),
130+
});
131+
Value::Null
132+
}),
133+
None => {
134+
warnings.push(ExpressionWarning {
135+
expression: expr.to_string(),
136+
message: format!("step \"{step_id}\" not found in context"),
137+
});
138+
Value::Null
139+
}
140+
};
141+
return (value, warnings);
96142
}
97-
return Value::Null;
143+
warnings.push(ExpressionWarning {
144+
expression: expr.to_string(),
145+
message: "invalid $steps expression: expected $steps.<id>.outputs.<key>"
146+
.to_string(),
147+
});
148+
return (Value::Null, warnings);
98149
}
99150

100151
if rest == "statusCode" {
101-
return self
102-
.ctx
103-
.status_code
104-
.map(|code| json!(code))
105-
.unwrap_or(Value::Null);
152+
return (
153+
self.ctx
154+
.status_code
155+
.map(|code| json!(code))
156+
.unwrap_or(Value::Null),
157+
warnings,
158+
);
106159
}
107160

108161
if rest == "method" {
109-
return self
110-
.ctx
111-
.method
112-
.as_ref()
113-
.map(|m| Value::String(m.clone()))
114-
.unwrap_or(Value::Null);
162+
return (
163+
self.ctx
164+
.method
165+
.as_ref()
166+
.map(|m| Value::String(m.clone()))
167+
.unwrap_or(Value::Null),
168+
warnings,
169+
);
115170
}
116171

117172
if rest == "url" {
118-
return self
119-
.ctx
120-
.url
121-
.as_ref()
122-
.map(|u| Value::String(u.clone()))
123-
.unwrap_or(Value::Null);
173+
return (
174+
self.ctx
175+
.url
176+
.as_ref()
177+
.map(|u| Value::String(u.clone()))
178+
.unwrap_or(Value::Null),
179+
warnings,
180+
);
124181
}
125182

126183
if let Some(after) = rest.strip_prefix("outputs.") {
127184
if let Some((name, pointer)) = after.split_once('#') {
128-
return self
185+
let value = self
129186
.ctx
130187
.outputs
131188
.get(name)
132189
.and_then(|v| v.pointer(pointer))
133190
.cloned()
134191
.unwrap_or(Value::Null);
192+
return (value, warnings);
135193
}
136-
return self.ctx.outputs.get(after).cloned().unwrap_or(Value::Null);
194+
return (
195+
self.ctx.outputs.get(after).cloned().unwrap_or(Value::Null),
196+
warnings,
197+
);
137198
}
138199

139200
if let Some(name) = rest.strip_prefix("request.header.") {
140-
return get_header_case_insensitive(&self.ctx.request_headers, name)
141-
.map(|v| Value::String(v.clone()))
142-
.unwrap_or(Value::Null);
201+
return (
202+
get_header_case_insensitive(&self.ctx.request_headers, name)
203+
.map(|v| Value::String(v.clone()))
204+
.unwrap_or(Value::Null),
205+
warnings,
206+
);
143207
}
144208

145209
if let Some(name) = rest.strip_prefix("request.query.") {
146-
return self
147-
.ctx
148-
.request_query
149-
.get(name)
150-
.map(|v| Value::String(v.clone()))
151-
.unwrap_or(Value::Null);
210+
return (
211+
self.ctx
212+
.request_query
213+
.get(name)
214+
.map(|v| Value::String(v.clone()))
215+
.unwrap_or(Value::Null),
216+
warnings,
217+
);
152218
}
153219

154220
if let Some(name) = rest.strip_prefix("request.path.") {
155-
return self
156-
.ctx
157-
.request_path
158-
.get(name)
159-
.map(|v| Value::String(v.clone()))
160-
.unwrap_or(Value::Null);
221+
return (
222+
self.ctx
223+
.request_path
224+
.get(name)
225+
.map(|v| Value::String(v.clone()))
226+
.unwrap_or(Value::Null),
227+
warnings,
228+
);
161229
}
162230

163231
if let Some(suffix) = rest.strip_prefix("request.body") {
164-
return resolve_body_access(&self.ctx.request_body, suffix);
232+
return (
233+
resolve_body_access(&self.ctx.request_body, suffix),
234+
warnings,
235+
);
165236
}
166237

167238
if let Some(after) = rest.strip_prefix("sourceDescriptions.") {
168239
if let Some(name) = after.strip_suffix(".url") {
169-
return self
240+
let value = self
170241
.ctx
171242
.source_descriptions
172243
.get(name)
173244
.map(|u| Value::String(u.clone()))
174-
.unwrap_or(Value::Null);
245+
.unwrap_or_else(|| {
246+
warnings.push(ExpressionWarning {
247+
expression: expr.to_string(),
248+
message: format!("source description \"{name}\" not found in context"),
249+
});
250+
Value::Null
251+
});
252+
return (value, warnings);
175253
}
176-
return Value::Null;
254+
return (Value::Null, warnings);
177255
}
178256

179257
if let Some(name) = rest.strip_prefix("response.header.") {
180-
return get_header_case_insensitive(&self.ctx.response_headers, name)
181-
.map(|v| Value::String(v.clone()))
182-
.unwrap_or(Value::Null);
258+
return (
259+
get_header_case_insensitive(&self.ctx.response_headers, name)
260+
.map(|v| Value::String(v.clone()))
261+
.unwrap_or(Value::Null),
262+
warnings,
263+
);
183264
}
184265

185266
if let Some(suffix) = rest.strip_prefix("response.body") {
186-
return resolve_body_access(&self.ctx.response_body, suffix);
267+
return (
268+
resolve_body_access(&self.ctx.response_body, suffix),
269+
warnings,
270+
);
187271
}
188272

189-
Value::Null
273+
warnings.push(ExpressionWarning {
274+
expression: expr.to_string(),
275+
message: format!("unknown expression namespace \"${rest}\""),
276+
});
277+
(Value::Null, warnings)
190278
}
191279

192280
/// Evaluate an expression and convert to string with Go-compatible coercions.
@@ -1387,4 +1475,64 @@ mod tests {
13871475
let eval = ExpressionEvaluator::new(EvalContext::default());
13881476
assert_eq!(eval.evaluate("$response.body#/data/0"), Value::Null);
13891477
}
1478+
1479+
#[test]
1480+
fn diagnostics_missing_step_warns() {
1481+
let eval = ExpressionEvaluator::new(EvalContext::default());
1482+
let (value, warnings) = eval.evaluate_with_diagnostics("$steps.missing.outputs.x");
1483+
assert_eq!(value, Value::Null);
1484+
assert_eq!(warnings.len(), 1);
1485+
assert!(warnings[0].message.contains("step \"missing\" not found"));
1486+
}
1487+
1488+
#[test]
1489+
fn diagnostics_missing_output_key_warns() {
1490+
let mut ctx = EvalContext::default();
1491+
ctx.steps.insert(
1492+
"s1".to_string(),
1493+
BTreeMap::from([("a".to_string(), json!(1))]),
1494+
);
1495+
let eval = ExpressionEvaluator::new(ctx);
1496+
let (value, warnings) = eval.evaluate_with_diagnostics("$steps.s1.outputs.nope");
1497+
assert_eq!(value, Value::Null);
1498+
assert_eq!(warnings.len(), 1);
1499+
assert!(warnings[0].message.contains("output \"nope\" not found"));
1500+
}
1501+
1502+
#[test]
1503+
fn diagnostics_valid_expression_no_warnings() {
1504+
let mut ctx = EvalContext::default();
1505+
ctx.inputs.insert("name".to_string(), json!("Alice"));
1506+
let eval = ExpressionEvaluator::new(ctx);
1507+
let (value, warnings) = eval.evaluate_with_diagnostics("$inputs.name");
1508+
assert_eq!(value, json!("Alice"));
1509+
assert!(warnings.is_empty());
1510+
}
1511+
1512+
#[test]
1513+
fn diagnostics_unknown_namespace_warns() {
1514+
let eval = ExpressionEvaluator::new(EvalContext::default());
1515+
let (value, warnings) = eval.evaluate_with_diagnostics("$foo.bar");
1516+
assert_eq!(value, Value::Null);
1517+
assert_eq!(warnings.len(), 1);
1518+
assert!(warnings[0].message.contains("unknown expression namespace"));
1519+
}
1520+
1521+
#[test]
1522+
fn diagnostics_missing_source_description_warns() {
1523+
let eval = ExpressionEvaluator::new(EvalContext::default());
1524+
let (value, warnings) = eval.evaluate_with_diagnostics("$sourceDescriptions.missing.url");
1525+
assert_eq!(value, Value::Null);
1526+
assert_eq!(warnings.len(), 1);
1527+
assert!(warnings[0]
1528+
.message
1529+
.contains("source description \"missing\""));
1530+
}
1531+
1532+
#[test]
1533+
fn evaluate_backward_compat_still_returns_null() {
1534+
let eval = ExpressionEvaluator::new(EvalContext::default());
1535+
assert_eq!(eval.evaluate("$steps.missing.outputs.x"), Value::Null);
1536+
assert_eq!(eval.evaluate("$foo.bar"), Value::Null);
1537+
}
13901538
}

‎crates/arazzo-runtime/src/runtime_core.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,8 @@ pub struct TraceCriterionResult {
515515
#[serde(default, skip_serializing_if = "String::is_empty")]
516516
pub context: String,
517517
pub result: bool,
518+
#[serde(default, skip_serializing_if = "Vec::is_empty")]
519+
pub warnings: Vec<String>,
518520
}
519521

520522
/// Runtime trace record for one step attempt.

‎crates/arazzo-runtime/src/runtime_core/engine_impl.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,7 @@ impl Engine {
616616
condition: evaluation.condition.clone(),
617617
context: evaluation.context_expr.clone(),
618618
result: evaluation.matched,
619+
warnings: evaluation.warnings.iter().map(|w| w.to_string()).collect(),
619620
});
620621
let gate = DebugGateContext {
621622
workflow_id,

‎crates/arazzo-runtime/src/runtime_core/helpers.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ pub(crate) struct CriterionEvaluation {
1010
pub context_expr: String,
1111
pub context_value: Value,
1212
pub error: Option<String>,
13+
pub warnings: Vec<arazzo_expr::ExpressionWarning>,
1314
}
1415

1516
pub(crate) fn extract_xpath(body: &[u8], expr: &str) -> Value {
@@ -56,10 +57,13 @@ pub(crate) fn evaluate_criterion_detailed(
5657
response: Option<&Response>,
5758
) -> CriterionEvaluation {
5859
let type_name = criterion.resolved_type_name();
60+
let mut expr_warnings = Vec::new();
5961
let mut context_value = if criterion.context.trim().is_empty() {
6062
default_criterion_context(response)
6163
} else {
62-
eval.evaluate(&criterion.context)
64+
let (val, warnings) = eval.evaluate_with_diagnostics(&criterion.context);
65+
expr_warnings = warnings;
66+
val
6367
};
6468
let mut error = None;
6569

@@ -105,6 +109,7 @@ pub(crate) fn evaluate_criterion_detailed(
105109
context_expr: criterion.context.clone(),
106110
context_value,
107111
error,
112+
warnings: expr_warnings,
108113
}
109114
}
110115

0 commit comments

Comments
 (0)