Repository: deepset-ai/haystack
Pull request: fix(JsonSchemaValidator): fix recursive loop and general LLM (claude, mistral...) compatibility
### Related Issues

- fixes https://github.com/deepset-ai/haystack/issues/7457 https://github.com/deepset-ai/haystack/issues/7455

### Proposed Changes:

- Claude Compatibility: modified the behaviour so that (i) error template is now a single message with generated json, error and schema (ii) make it so that validated messages are always "Assistant" chatmessage (for next pipeline step) and validation_errors are always "User" chatmessage (for LLM loops)
- Recursive Loop in type conversion: used Claude OPUS to automatically generate a fix based on the written issue.

### How did you test it?

Tested on my personal use-case and it solved my issues.

### Notes for the reviewer

The behaviour is modified to only include the last messages from the conversation and not the whole history of messages (less cost for long pipeline loops, not necessary to have previous messages).

For the auto-generated fix for recursive, maybe the bug comes from the fact that sometimes `json.loads(value)` output a string and needs to be called twice to get the actual dict/list in the string. This is weird, but I've seen it happen. I'm not sure about the fundamental difference to be honest. Maybe it doesn't work for nested json.

### Checklist

- I have read the [contributors guidelines](https://github.com/deepset-ai/haystack/blob/main/CONTRIBUTING.md) and the [code of conduct](https://github.com/deepset-ai/haystack/blob/main/code_of_conduct.txt)
- I have updated the related issue with new insights and changes
- I added unit tests and updated the docstrings
- I've used one of the [conventional commit types](https://www.conventionalcommits.org/en/v1.0.0/) for my PR title: `fix:`, `feat:`, `build:`, `chore:`, `ci:`, `docs:`, `style:`, `refactor:`, `perf:`, `test:`.
- I documented my code
- I ran [pre-commit hooks](https://github.com/deepset-ai/haystack/blob/main/CONTRIBUTING.md#installation) and fixed any issue


Problem statement:
JsonSchemaValidator: unintended primitive value conversion in `_recursive_json_to_object` method

**Describe the bug**

The current implementation of the `_recursive_json_to_object` method within the `JsonSchemaValidator` class inadvertently converts primitive string values to their respective data types (e.g., converting numeric strings to integers) when processing JSON content. This behavior occurs during the `json.loads(value)` step, where the method does not distinguish between primitive values and JSON objects or arrays. As a result, string values that represent numeric or boolean data are automatically converted to their corresponding data types, leading to potential mismatches with the expected data types defined in the JSON schema.

**Expected behavior**

The correct behavior should ensure that the `_recursive_json_to_object` method maintains the original data types of primitive values as specified in the input JSON content. This requires the method to identify and preserve primitive string values (e.g., numeric strings, boolean strings) without converting them to other data types during the parsing process. Only non-primitive values (i.e., values that represent actual JSON objects or arrays) should be parsed and converted into their respective complex types. This approach will ensure that the JSON content's integrity is maintained, and schema validations are performed accurately according to the specified data types.

**Additional context**

The issue highlights the need for a more nuanced parsing mechanism within the `_recursive_json_to_object` method that can accurately differentiate between primitive and non-primitive values. This distinction is critical for applications that rely on strict data type validations against a JSON schema, where preserving the original data type of each value is essential for successful validation. 

A possible solution involves enhancing the parsing logic to check the result of `json.loads(value)` and only proceed with converting the value if it is indeed a non-primitive data type (i.e., a dictionary or list). If the parsed value is a primitive data type (e.g., integer, float, boolean), the original string value should be retained. Implementing this solution will address the unintended data type conversion issue, thereby improving the functionality and reliability of the `JsonSchemaValidator` in handling JSON content with strict type requirements.

Hints from issue discussion:

Commit message: fix test

Patch to review:
diff --git a/haystack/components/validators/json_schema.py b/haystack/components/validators/json_schema.py
index 109b0ad341..75661688af 100644
--- a/haystack/components/validators/json_schema.py
+++ b/haystack/components/validators/json_schema.py
@@ -70,13 +70,14 @@ def run(self, messages: List[ChatMessage]) -> dict:
 
     # Default error description template
     default_error_template = (
-        "The JSON content in the next message does not conform to the provided schema.\n"
+        "The following generated JSON does not conform to the provided schema.\n"
+        "Generated JSON: {failing_json}\n"
         "Error details:\n- Message: {error_message}\n"
         "- Error Path in JSON: {error_path}\n"
         "- Schema Path: {error_schema_path}\n"
         "Please match the following schema:\n"
         "{json_schema}\n"
-        "and provide the corrected JSON content ONLY."
+        "and provide the corrected JSON content ONLY. Please do not output anything else than the raw corrected JSON string, this is the most important part of the task. Don't use any markdown and don't add any comment."
     )
 
     def __init__(self, json_schema: Optional[Dict[str, Any]] = None, error_template: Optional[str] = None):
@@ -142,7 +143,7 @@ def run(
                 else:
                     validate(instance=content, schema=validation_schema)
 
-            return {"validated": messages}
+            return {"validated": [last_message]}
         except ValidationError as e:
             error_path = " -> ".join(map(str, e.absolute_path)) if e.absolute_path else "N/A"
             error_schema_path = " -> ".join(map(str, e.absolute_schema_path)) if e.absolute_schema_path else "N/A"
@@ -150,10 +151,14 @@ def run(
             error_template = error_template or self.default_error_template
 
             recovery_prompt = self._construct_error_recovery_message(
-                error_template, str(e), error_path, error_schema_path, validation_schema
+                error_template,
+                str(e),
+                error_path,
+                error_schema_path,
+                validation_schema,
+                failing_json=last_message.content,
             )
-            complete_message_list = [ChatMessage.from_user(recovery_prompt)] + messages
-            return {"validation_error": complete_message_list}
+            return {"validation_error": [ChatMessage.from_user(recovery_prompt)]}
 
     def _construct_error_recovery_message(
         self,
@@ -162,6 +167,7 @@ def _construct_error_recovery_message(
         error_path: str,
         error_schema_path: str,
         json_schema: Dict[str, Any],
+        failing_json: str,
     ) -> str:
         """
         Constructs an error recovery message using a specified template or the default one if none is provided.
@@ -171,6 +177,7 @@ def _construct_error_recovery_message(
         :param error_path: The path in the JSON content where the error occurred.
         :param error_schema_path: The path in the JSON schema where the error occurred.
         :param json_schema: The JSON schema against which the content is validated.
+        :param failing_json: The generated invalid JSON string.
         """
         error_template = error_template or self.default_error_template
 
@@ -179,6 +186,7 @@ def _construct_error_recovery_message(
             error_path=error_path,
             error_schema_path=error_schema_path,
             json_schema=json_schema,
+            failing_json=failing_json,
         )
 
     def _is_openai_function_calling_schema(self, json_schema: Dict[str, Any]) -> bool:
@@ -208,11 +216,10 @@ def _recursive_json_to_object(self, data: Any) -> Any:
                 if isinstance(value, str):
                     try:
                         json_value = json.loads(value)
-                        new_dict[key] = (
-                            self._recursive_json_to_object(json_value)
-                            if isinstance(json_value, (dict, list))
-                            else json_value
-                        )
+                        if isinstance(json_value, (dict, list)):
+                            new_dict[key] = self._recursive_json_to_object(json_value)
+                        else:
+                            new_dict[key] = value  # Preserve the original string value
                     except json.JSONDecodeError:
                         new_dict[key] = value
                 elif isinstance(value, dict):
diff --git a/releasenotes/notes/fix-recursive-json-schema-validator-cdb7684de3c75e4e.yaml b/releasenotes/notes/fix-recursive-json-schema-validator-cdb7684de3c75e4e.yaml
new file mode 100644
index 0000000000..c24e687005
--- /dev/null
+++ b/releasenotes/notes/fix-recursive-json-schema-validator-cdb7684de3c75e4e.yaml
@@ -0,0 +1,8 @@
+---
+enhancements:
+  - |
+    Made JSON schema validator compatible with all LLM by switching error template handling to a single user message.
+    Also reduce cost by only including last error instead of full message history.
+fixes:
+  - |
+    Fix recursive JSON type conversion in the schema validator to be less aggressive (no infinite recursion).
diff --git a/test/components/validators/test_json_schema.py b/test/components/validators/test_json_schema.py
index 034bf6b962..39e9c78587 100644
--- a/test/components/validators/test_json_schema.py
+++ b/test/components/validators/test_json_schema.py
@@ -4,11 +4,10 @@
 import json
 from typing import List
 
-from haystack import component, Pipeline
-from haystack.components.validators import JsonSchemaValidator
-
 import pytest
 
+from haystack import Pipeline, component
+from haystack.components.validators import JsonSchemaValidator
 from haystack.dataclasses import ChatMessage
 
 
@@ -110,10 +109,9 @@ def test_validates_multiple_messages_against_json_schema(self, json_schema_githu
         ]
 
         result = validator.run(messages, json_schema_github_compare)
-
         assert "validated" in result
-        assert len(result["validated"]) == 2
-        assert result["validated"] == messages
+        assert len(result["validated"]) == 1
+        assert result["validated"][0] == messages[1]
 
     #  Validates a message against an OpenAI function calling schema successfully.
     def test_validates_message_against_openai_function_calling_schema(
@@ -142,8 +140,8 @@ def test_validates_multiple_messages_against_openai_function_calling_schema(
         result = validator.run(messages, json_schema_github_compare_openai)
 
         assert "validated" in result
-        assert len(result["validated"]) == 2
-        assert result["validated"] == messages
+        assert len(result["validated"]) == 1
+        assert result["validated"][0] == messages[1]
 
     #  Constructs a custom error recovery message when validation fails.
     def test_construct_custom_error_recovery_message(self):
@@ -155,10 +153,11 @@ def test_construct_custom_error_recovery_message(self):
             "- Schema Path: {error_schema_path}\n"
             "Please match the following schema:\n"
             "{json_schema}\n"
+            "Failing Json: {failing_json}\n"
         )
 
         recovery_message = validator._construct_error_recovery_message(
-            new_error_template, "Error message", "Error path", "Error schema path", {"type": "object"}
+            new_error_template, "Error message", "Error path", "Error schema path", {"type": "object"}, "Failing Json"
         )
 
         expected_recovery_message = (
@@ -167,6 +166,7 @@ def test_construct_custom_error_recovery_message(self):
             "- Schema Path: Error schema path\n"
             "Please match the following schema:\n"
             "{'type': 'object'}\n"
+            "Failing Json: Failing Json\n"
         )
         assert recovery_message == expected_recovery_message
 
@@ -201,5 +201,5 @@ def run(self):
         pipe.connect("message_producer", "schema_validator")
         result = pipe.run(data={"schema_validator": {"json_schema": json_schema_github_compare}})
         assert "validation_error" in result["schema_validator"]
-        assert len(result["schema_validator"]["validation_error"]) > 1
+        assert len(result["schema_validator"]["validation_error"]) == 1
         assert "Error details" in result["schema_validator"]["validation_error"][0].content


Code review comments:
diff_hunk: @@ -142,18 +143,22 @@ def run(
                 else:
                     validate(instance=content, schema=validation_schema)
 
-            return {"validated": messages}
+            return {"validated": [last_message]}
path: haystack/components/validators/json_schema.py
text: @user2:
@user1-science looks good but isn't this changing the method contract, do you remember why this change?

@author:
By method contract you mean signature ?
The type is the same List[ChatMessage] but now this list is always of length 1.
The reason I did this is to not have exponentional cost by passing all message history to the LLM each time. Imagine your pipe does 20 loops, you used 20+19+18+17+16... (20x(20+1)/2) 220 messages worth of tokens.
By passing only the last error (which should be the only needed to fix the json schema) you would only use 20 messages worth of tokens

@user2:
Ok I see; but one can also argue that previous failure attempts are useful as well. We should provide some top_k last messages argument, wdyt @user1-science ?

@author:
> Ok I see; but one can also argue that previous failure attempts are useful as well. We should provide some top_k last messages argument, wdyt @user1-science ?

I'm sorry I completly missed to answer you and then went on holiday. I sounds like a good idea but might require a bit more coding ahah. Not sure if I have the space in the next few days for this (maybe during next week)