scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-09 03:20:56 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix: sort unexpected properties before comparing repair attempts

Browse source 
Addresses Copilot review feedback on the repeated-failure check.

serde_json runs with preserve_order, and jsonschema builds the
additionalProperties `unexpected` list by walking the instance in
document order. So the same leftover keys emitted in a different order
produced a different Vec and compared as a different problem, which
suppressed the "unchanged from your previous repair" nudge.

Sorting at capture also makes the MAX_UNEXPECTED_PROPERTIES truncation
pick the same subset every time instead of an order-dependent one, and
stabilizes the rendered message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-08-01 09:18:38 -04:00
parent 0e0dfe4f9d
commit c738130e53
No known key found for this signature in database
1 changed files with 30 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
lib/components/fabro-workflow/src/handler/structured_output.rs
View file

@ -102,13 +102,17 @@ impl SchemaValidationIssue {
.map_or_else(|| property.to_string(), str::to_owned),
},
ValidationErrorKind::AdditionalProperties { unexpected } => {
// `unexpected` arrives in the order the model emitted the keys,
// so sort before truncating. That keeps the retained subset and
// the rendered message stable, and lets two attempts that left
// the same keys in place compare equal whatever order they used.
let total = unexpected.len();
let mut sorted = unexpected.clone();
sorted.sort_unstable();
sorted.truncate(MAX_UNEXPECTED_PROPERTIES);
SchemaValidationIssueDetail::AdditionalProperties {
total: unexpected.len(),
unexpected: unexpected
.iter()
.take(MAX_UNEXPECTED_PROPERTIES)
.cloned()
.collect(),
unexpected: sorted,
total,
}
}
_ => SchemaValidationIssueDetail::Other {
@ -969,6 +973,26 @@ mod tests {
);
}
#[test]
fn the_same_unexpected_properties_in_a_new_order_are_still_unchanged() {
let schema = schema(serde_json::json!({
"type": "object",
"additionalProperties": false,
"properties": {
"findings": { "type": "array" }
}
}));
let previous = validate_response_text(&schema, r#"{"beta":1,"alpha":1}"#).unwrap_err();
let current = validate_response_text(&schema, r#"{"alpha":1,"beta":1}"#).unwrap_err();
let repair = current.repair_message(&schema, Some(&previous));
assert!(
repair.contains("unchanged from your previous repair"),
"unexpected repair message: {repair}",
);
}
#[test]
fn a_different_problem_at_the_same_location_is_not_called_unchanged() {
let schema = schema(serde_json::json!({