From c738130e531c8ef6b08e2d9f1bc002eec87a0f91 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Sat, 1 Aug 2026 09:18:38 -0400
Subject: [PATCH] fix: sort unexpected properties before comparing repair
 attempts

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>
---
 .../src/handler/structured_output.rs          | 36 +++++++++++++++----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/lib/components/fabro-workflow/src/handler/structured_output.rs b/lib/components/fabro-workflow/src/handler/structured_output.rs
index a208dd2cc..845c7b070 100644
--- a/lib/components/fabro-workflow/src/handler/structured_output.rs
+++ b/lib/components/fabro-workflow/src/handler/structured_output.rs
@@ -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!({