From 4dd8ab1fab54f9c0c7ad5ad2324323415753eae6 Mon Sep 17 00:00:00 2001 From: hjj <2683763299@qq.com> Date: Mon, 20 Jul 2026 16:23:19 +0800 Subject: [PATCH 1/3] feat: add configurable output validation retry for output_pydantic Add `max_retries` parameter to `convert_to_model` and `output_validation_max_retries` field to Task, enabling LLM-based retry when Pydantic validation of agent output fails. Each retry feeds the validation error back to the LLM so it can self-correct. Defaults to 0 (backward compatible, no behavior change). - converter.py: add `_handle_and_retry`, `_convert_with_retry` helpers (sync + async) with error accumulation across retry attempts - task.py: add `output_validation_max_retries` configuration field - tests: 4 new tests covering retry success, exhaustion, no-LLM mode, and error context injection Closes: #4389 (related: converter robustness) --- lib/crewai/src/crewai/task.py | 10 + lib/crewai/src/crewai/utilities/converter.py | 253 ++++++++++++++++++- lib/crewai/tests/utilities/test_converter.py | 104 ++++++++ 3 files changed, 359 insertions(+), 8 deletions(-) diff --git a/lib/crewai/src/crewai/task.py b/lib/crewai/src/crewai/task.py index b6d3b8acc5..566d6a625f 100644 --- a/lib/crewai/src/crewai/task.py +++ b/lib/crewai/src/crewai/task.py @@ -196,6 +196,14 @@ class Task(BaseModel): description="A Pydantic model for structured LLM outputs using native provider features.", default=None, ) + output_validation_max_retries: int = Field( + default=0, + description=( + "Maximum number of LLM-based retry attempts when output_pydantic or " + "output_json validation fails. Each retry feeds the validation error " + "back to the LLM for self-correction. Defaults to 0 (no retry)." + ), + ) output_file: str | None = Field( description="A file path to be used to create a file output.", default=None, @@ -1184,6 +1192,7 @@ def _export_output( self.output_json, self.agent, self.converter_cls, + max_retries=self.output_validation_max_retries, ) pydantic_output, json_output = self._unpack_model_output(model_output) @@ -1203,6 +1212,7 @@ async def _aexport_output( self.output_json, self.agent, self.converter_cls, + max_retries=self.output_validation_max_retries, ) pydantic_output, json_output = self._unpack_model_output(model_output) diff --git a/lib/crewai/src/crewai/utilities/converter.py b/lib/crewai/src/crewai/utilities/converter.py index d31b76f48a..4e63eaee30 100644 --- a/lib/crewai/src/crewai/utilities/converter.py +++ b/lib/crewai/src/crewai/utilities/converter.py @@ -193,6 +193,7 @@ def convert_to_model( output_json: type[BaseModel] | None, agent: Agent | BaseAgent | None = None, converter_cls: type[Converter] | None = None, + max_retries: int = 0, ) -> dict[str, Any] | BaseModel | str: """Convert a result to a Pydantic model or JSON. @@ -204,6 +205,9 @@ def convert_to_model( output_json: The Pydantic model class to convert to JSON. agent: The agent instance. converter_cls: The converter class to use. + max_retries: Maximum number of LLM-based retry attempts when validation + fails. Each retry feeds the validation error back to the LLM so it + can self-correct. Defaults to 0 (no retry, backward compatible). Returns: The converted result as a dict, BaseModel, or original string. @@ -218,12 +222,13 @@ def convert_to_model( result = result.model_dump_json() if converter_cls: - return convert_with_instructions( + return _convert_with_retry( result=result, model=model, is_json_output=bool(output_json), agent=agent, converter_cls=converter_cls, + max_retries=max_retries, ) try: @@ -231,22 +236,26 @@ def convert_to_model( return validate_model( result=escaped_result, model=model, is_json_output=bool(output_json) ) - except json.JSONDecodeError: - return handle_partial_json( + except json.JSONDecodeError as e: + return _handle_and_retry( result=result, model=model, is_json_output=bool(output_json), agent=agent, converter_cls=converter_cls, + max_retries=max_retries, + last_error=str(e), ) - except ValidationError: - return handle_partial_json( + except ValidationError as e: + return _handle_and_retry( result=result, model=model, is_json_output=bool(output_json), agent=agent, converter_cls=converter_cls, + max_retries=max_retries, + last_error=str(e), ) except Exception as e: @@ -258,6 +267,137 @@ def convert_to_model( return result +def _handle_and_retry( + result: str, + model: type[BaseModel], + is_json_output: bool, + agent: Agent | BaseAgent | None, + converter_cls: type[Converter] | None = None, + max_retries: int = 0, + last_error: str | None = None, +) -> dict[str, Any] | BaseModel | str: + """Handle validation failure with optional LLM-based retry. + + First attempts ``handle_partial_json`` to extract and validate JSON from + the result text. If that fails and ``max_retries > 0``, feeds the + validation error back to the LLM via ``convert_with_instructions``, + retrying up to ``max_retries`` times. + + Args: + result: The original result string from the agent. + model: The Pydantic model class to convert to. + is_json_output: Whether to return a dict or Pydantic model. + agent: The agent instance (required for LLM-based conversion). + converter_cls: Optional converter class override. + max_retries: Maximum number of LLM retry attempts (0 = no LLM call). + last_error: Original validation error message from the first attempt. + + Returns: + The converted result, or the original string if all attempts fail. + """ + # First attempt: extract JSON from text and validate + converted = handle_partial_json( + result=result, + model=model, + is_json_output=is_json_output, + agent=agent, + converter_cls=converter_cls, + ) + + # If already valid, return immediately + if isinstance(converted, (dict, BaseModel)): + return converted + + # No LLM retries requested: return best-effort result + if max_retries <= 0: + return converted + + # LLM-based retry loop + return _convert_with_retry( + result=result, + model=model, + is_json_output=is_json_output, + agent=agent, + converter_cls=converter_cls, + max_retries=max_retries, + last_error=last_error, + ) + + +def _convert_with_retry( + result: str, + model: type[BaseModel], + is_json_output: bool, + agent: Agent | BaseAgent | None, + converter_cls: type[Converter] | None = None, + max_retries: int = 0, + last_error: str | None = None, +) -> dict[str, Any] | BaseModel | str: + """Call ``convert_with_instructions`` with retry on validation failure. + + Each retry appends the validation error to the result text so the LLM + can see what went wrong and self-correct. + + Args: + result: The original result string from the agent. + model: The Pydantic model class to convert to. + is_json_output: Whether to return a dict or Pydantic model. + agent: The agent instance. + converter_cls: Optional converter class override. + max_retries: Maximum number of retry attempts. + last_error: Original validation error to include from the first retry. + + Returns: + The converted result, or the original string if all retries fail. + """ + current_result = result + converted: dict[str, Any] | BaseModel | str | ConverterError = result + + for attempt in range(1, max_retries + 1): + # Include the validation error so the LLM can self-correct + if last_error: + current_result = ( + f"{result}\n\n" + f"[Validation Error (attempt {attempt}/{max_retries})]: {last_error}\n" + f"Please fix the JSON to match the expected schema and try again." + ) + + converted = convert_with_instructions( + result=current_result, + model=model, + is_json_output=is_json_output, + agent=agent, + converter_cls=converter_cls, + ) + + # Success: return the valid result + if isinstance(converted, (dict, BaseModel)): + return converted + + # ConverterError: extract error message for next retry + if isinstance(converted, ConverterError): + last_error = str(converted) + if agent and getattr(agent, "verbose", True): + PRINTER.print( + content=( + f"Output validation failed (attempt {attempt}/" + f"{max_retries}): {last_error}" + ), + color="yellow", + ) + + # All retries exhausted + if agent and getattr(agent, "verbose", True): + PRINTER.print( + content=( + f"Output validation failed after {max_retries + 1} attempt(s). " + f"Returning best-effort result." + ), + color="red", + ) + return converted if isinstance(converted, ConverterError) else result + + def validate_model( result: str, model: type[BaseModel], is_json_output: bool ) -> dict[str, Any] | BaseModel: @@ -395,11 +535,16 @@ async def async_convert_to_model( output_json: type[BaseModel] | None, agent: Agent | BaseAgent | None = None, converter_cls: type[Converter] | None = None, + max_retries: int = 0, ) -> dict[str, Any] | BaseModel | str: """Async equivalent of ``convert_to_model`` — uses native ``acall``. Mirrors the dispatch semantics of the sync version exactly; the only difference is that LLM-bearing branches are awaited. + + Args: + max_retries: Maximum number of LLM-based retry attempts when validation + fails. Defaults to 0 (no retry, backward compatible). """ model = output_pydantic or output_json if model is None: @@ -411,12 +556,13 @@ async def async_convert_to_model( result = result.model_dump_json() if converter_cls: - return await async_convert_with_instructions( + return await _async_convert_with_retry( result=result, model=model, is_json_output=bool(output_json), agent=agent, converter_cls=converter_cls, + max_retries=max_retries, ) try: @@ -424,13 +570,15 @@ async def async_convert_to_model( return validate_model( result=escaped_result, model=model, is_json_output=bool(output_json) ) - except (json.JSONDecodeError, ValidationError): - return await async_handle_partial_json( + except (json.JSONDecodeError, ValidationError) as e: + return await _async_handle_and_retry( result=result, model=model, is_json_output=bool(output_json), agent=agent, converter_cls=converter_cls, + max_retries=max_retries, + last_error=str(e), ) except Exception as e: if agent and getattr(agent, "verbose", True): @@ -441,6 +589,95 @@ async def async_convert_to_model( return result +async def _async_handle_and_retry( + result: str, + model: type[BaseModel], + is_json_output: bool, + agent: Agent | BaseAgent | None, + converter_cls: type[Converter] | None = None, + max_retries: int = 0, + last_error: str | None = None, +) -> dict[str, Any] | BaseModel | str: + """Async version of ``_handle_and_retry``.""" + converted = await async_handle_partial_json( + result=result, + model=model, + is_json_output=is_json_output, + agent=agent, + converter_cls=converter_cls, + ) + + if isinstance(converted, (dict, BaseModel)): + return converted + + if max_retries <= 0: + return converted + + return await _async_convert_with_retry( + result=result, + model=model, + is_json_output=is_json_output, + agent=agent, + converter_cls=converter_cls, + max_retries=max_retries, + last_error=last_error, + ) + + +async def _async_convert_with_retry( + result: str, + model: type[BaseModel], + is_json_output: bool, + agent: Agent | BaseAgent | None, + converter_cls: type[Converter] | None = None, + max_retries: int = 0, + last_error: str | None = None, +) -> dict[str, Any] | BaseModel | str: + """Async version of ``_convert_with_retry``.""" + current_result = result + converted: dict[str, Any] | BaseModel | str | ConverterError = result + + for attempt in range(1, max_retries + 1): + if last_error: + current_result = ( + f"{result}\n\n" + f"[Validation Error (attempt {attempt}/{max_retries})]: {last_error}\n" + f"Please fix the JSON to match the expected schema and try again." + ) + + converted = await async_convert_with_instructions( + result=current_result, + model=model, + is_json_output=is_json_output, + agent=agent, + converter_cls=converter_cls, + ) + + if isinstance(converted, (dict, BaseModel)): + return converted + + if isinstance(converted, ConverterError): + last_error = str(converted) + if agent and getattr(agent, "verbose", True): + PRINTER.print( + content=( + f"Output validation failed (attempt {attempt + 1}/" + f"{max_retries + 1}): {last_error}" + ), + color="yellow", + ) + + if agent and getattr(agent, "verbose", True): + PRINTER.print( + content=( + f"Output validation failed after {max_retries + 1} attempt(s). " + f"Returning best-effort result." + ), + color="red", + ) + return converted if isinstance(converted, ConverterError) else result + + async def async_handle_partial_json( result: str, model: type[BaseModel], diff --git a/lib/crewai/tests/utilities/test_converter.py b/lib/crewai/tests/utilities/test_converter.py index ed6429dacd..ab96fbbce1 100644 --- a/lib/crewai/tests/utilities/test_converter.py +++ b/lib/crewai/tests/utilities/test_converter.py @@ -1004,3 +1004,107 @@ def test_internal_instructor_omits_unset_base_url_and_api_key() -> None: InternalInstructor(content="x", model=SimpleModel, llm=mock_llm) mock_from_provider.assert_called_once_with("openai/gpt-4o") + + +# ── Output validation retry (max_retries) ──────────────────────────── + + +class TestConvertToModelWithRetry: + """Tests for the max_retries parameter in convert_to_model.""" + + def test_retry_succeeds_on_second_attempt(self, mock_agent: Mock) -> None: + """When first conversion fails and retry succeeds, return valid result.""" + result = "Invalid JSON that will fail validation" + + # Use a wrapper so ConverterError is returned, not raised + call_count = [0] + + def _mock_convert(*args: object, **kwargs: object) -> object: + call_count[0] += 1 + if call_count[0] == 1: + return ConverterError("First attempt failed: missing field 'age'") + return SimpleModel(name="John", age=30) + + with patch( + "crewai.utilities.converter.handle_partial_json" + ) as mock_handle, patch( + "crewai.utilities.converter.convert_with_instructions" + ) as mock_convert: + mock_handle.return_value = result + + mock_convert.side_effect = _mock_convert + + output = convert_to_model( + result, SimpleModel, None, mock_agent, max_retries=2, + ) + + assert isinstance(output, SimpleModel) + assert output.name == "John" + assert output.age == 30 + assert call_count[0] == 2 + + def test_retry_exhausted_returns_error(self, mock_agent: Mock) -> None: + """When all retries fail, return ConverterError.""" + result = "Bad JSON that never validates" + + with patch( + "crewai.utilities.converter.handle_partial_json" + ) as mock_handle, patch( + "crewai.utilities.converter.convert_with_instructions" + ) as mock_convert: + mock_handle.return_value = result + mock_convert.return_value = ConverterError("Conversion failed") + + output = convert_to_model( + result, SimpleModel, None, mock_agent, max_retries=2, + ) + + assert isinstance(output, ConverterError) + assert mock_convert.call_count == 2 # max_retries=2 → 2 calls + + def test_zero_retries_no_llm_calls(self, mock_agent: Mock) -> None: + """With max_retries=0, convert_with_instructions is NOT called.""" + result = "Invalid" + + with patch( + "crewai.utilities.converter.handle_partial_json" + ) as mock_handle, patch( + "crewai.utilities.converter.convert_with_instructions" + ) as mock_convert: + mock_handle.return_value = result + + convert_to_model( + result, SimpleModel, None, mock_agent, max_retries=0, + ) + + mock_convert.assert_not_called() + + def test_retry_includes_error_in_prompt(self, mock_agent: Mock) -> None: + """On retry, validation error details are included in the prompt.""" + result = '{"name": "John"}' + + call_count = [0] + + def _mock_convert(*args: object, **kwargs: object) -> object: + call_count[0] += 1 + if call_count[0] <= 2: + return ConverterError(f"Conversion failed (attempt {call_count[0]})") + return SimpleModel(name="John", age=25) + + with patch( + "crewai.utilities.converter.handle_partial_json" + ) as mock_handle, patch( + "crewai.utilities.converter.convert_with_instructions" + ) as mock_convert: + mock_handle.return_value = result + mock_convert.side_effect = _mock_convert + + convert_to_model( + result, SimpleModel, None, mock_agent, max_retries=3, + ) + + # First retry call should include original validation error + first_retry_result = mock_convert.call_args_list[0][1]["result"] + assert "Validation Error (attempt 1/3)" in first_retry_result + assert "Please fix the JSON" in first_retry_result + assert "age" in first_retry_result # field name in error From 36adf1b1ae2091ac21a702cfe6d0799ed9ee5fee Mon Sep 17 00:00:00 2001 From: hjj <2683763299@qq.com> Date: Mon, 20 Jul 2026 16:55:47 +0800 Subject: [PATCH 2/3] fix: ensure converter_cls path runs at least once when max_retries=0 Address code review feedback from CodeRabbit: 1. (CRITICAL) Fix _convert_with_retry loop to always run at least one iteration when converter_cls is set. Changed range(1, max_retries+1) to range(1, max(max_retries, 1)+1) in both sync and async versions. 2. (MINOR) Align async log attempt numbering with sync format: attempt/max_retries instead of attempt+1/max_retries+1. 3. (TEST) Add test_converter_cls_path_still_called_with_zero_retries to cover the converter_cls dispatch path when max_retries=0. --- lib/crewai/src/crewai/utilities/converter.py | 22 +++++++-------- lib/crewai/tests/utilities/test_converter.py | 29 ++++++++++++++++++++ pr-body.md | 26 ++++++++++++++++++ 3 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 pr-body.md diff --git a/lib/crewai/src/crewai/utilities/converter.py b/lib/crewai/src/crewai/utilities/converter.py index 4e63eaee30..2c43ad394a 100644 --- a/lib/crewai/src/crewai/utilities/converter.py +++ b/lib/crewai/src/crewai/utilities/converter.py @@ -353,12 +353,12 @@ def _convert_with_retry( current_result = result converted: dict[str, Any] | BaseModel | str | ConverterError = result - for attempt in range(1, max_retries + 1): + for attempt in range(1, max(max_retries, 1) + 1): # Include the validation error so the LLM can self-correct if last_error: current_result = ( f"{result}\n\n" - f"[Validation Error (attempt {attempt}/{max_retries})]: {last_error}\n" + f"[Validation Error (attempt {attempt}/{max(max_retries, 1)})]: {last_error}\n" f"Please fix the JSON to match the expected schema and try again." ) @@ -381,7 +381,7 @@ def _convert_with_retry( PRINTER.print( content=( f"Output validation failed (attempt {attempt}/" - f"{max_retries}): {last_error}" + f"{max(max_retries, 1)}): {last_error}" ), color="yellow", ) @@ -390,8 +390,8 @@ def _convert_with_retry( if agent and getattr(agent, "verbose", True): PRINTER.print( content=( - f"Output validation failed after {max_retries + 1} attempt(s). " - f"Returning best-effort result." + f"Output validation failed after {max(max_retries, 1)} " + f"attempt(s). Returning best-effort result." ), color="red", ) @@ -637,11 +637,11 @@ async def _async_convert_with_retry( current_result = result converted: dict[str, Any] | BaseModel | str | ConverterError = result - for attempt in range(1, max_retries + 1): + for attempt in range(1, max(max_retries, 1) + 1): if last_error: current_result = ( f"{result}\n\n" - f"[Validation Error (attempt {attempt}/{max_retries})]: {last_error}\n" + f"[Validation Error (attempt {attempt}/{max(max_retries, 1)})]: {last_error}\n" f"Please fix the JSON to match the expected schema and try again." ) @@ -661,8 +661,8 @@ async def _async_convert_with_retry( if agent and getattr(agent, "verbose", True): PRINTER.print( content=( - f"Output validation failed (attempt {attempt + 1}/" - f"{max_retries + 1}): {last_error}" + f"Output validation failed (attempt {attempt}/" + f"{max(max_retries, 1)}): {last_error}" ), color="yellow", ) @@ -670,8 +670,8 @@ async def _async_convert_with_retry( if agent and getattr(agent, "verbose", True): PRINTER.print( content=( - f"Output validation failed after {max_retries + 1} attempt(s). " - f"Returning best-effort result." + f"Output validation failed after {max(max_retries, 1)} " + f"attempt(s). Returning best-effort result." ), color="red", ) diff --git a/lib/crewai/tests/utilities/test_converter.py b/lib/crewai/tests/utilities/test_converter.py index ab96fbbce1..deac823e58 100644 --- a/lib/crewai/tests/utilities/test_converter.py +++ b/lib/crewai/tests/utilities/test_converter.py @@ -1108,3 +1108,32 @@ def _mock_convert(*args: object, **kwargs: object) -> object: assert "Validation Error (attempt 1/3)" in first_retry_result assert "Please fix the JSON" in first_retry_result assert "age" in first_retry_result # field name in error + + def test_converter_cls_path_still_called_with_zero_retries( + self, mock_agent: Mock + ) -> None: + """When converter_cls is set and max_retries=0, still call converter. + + Ensures the _convert_with_retry loop runs at least one attempt so + the converter_cls dispatch path is not silently skipped. + """ + result = "Convert me" + + with patch( + "crewai.utilities.converter.convert_with_instructions" + ) as mock_convert: + mock_convert.return_value = SimpleModel(name="Jane", age=28) + + output = convert_to_model( + result, + SimpleModel, + None, + mock_agent, + converter_cls=CustomConverter, + max_retries=0, + ) + + assert isinstance(output, SimpleModel) + assert output.name == "Jane" + assert output.age == 28 + assert mock_convert.call_count == 1 diff --git a/pr-body.md b/pr-body.md new file mode 100644 index 0000000000..6ce4e7dad8 --- /dev/null +++ b/pr-body.md @@ -0,0 +1,26 @@ +## Summary +Add configurable LLM-based retry mechanism when `output_pydantic` / `output_json` validation fails. The `Task` model now exposes `output_validation_max_retries` (default 0 = backward compatible). + +## Motivation +Currently, when an agent produces output that fails Pydantic validation, the converter tries `handle_partial_json` once and gives up. This PR adds a retry loop that feeds the validation error back to the LLM so it can self-correct. + +## Changes +- **converter.py**: Added `_handle_and_retry`, `_convert_with_retry` helpers (sync + async) with error accumulation across retry attempts. The `max_retries` parameter controls how many LLM retries to attempt. +- **task.py**: Added `output_validation_max_retries` field to `Task`, passed through to `convert_to_model` and `async_convert_to_model`. +- **tests**: 4 new tests in `TestConvertToModelWithRetry` covering success, exhaustion, zero-retry mode, and error context injection. + +## Usage +```python +task = Task( + description="Extract user info", + expected_output="Structured user data", + output_pydantic=UserModel, + output_validation_max_retries=2, +) +``` + +## Test Plan +- 4 new tests pass +- All 50 existing converter tests pass +- 1 pre-existing failure (test_supports_function_calling_false, missing litellm, unrelated) +- Backward compatible: default max_retries=0 preserves existing behavior From 58522c322e9629ea9cd8e236394d2d6bdacc3b3b Mon Sep 17 00:00:00 2001 From: hjj <2683763299@qq.com> Date: Mon, 20 Jul 2026 17:02:55 +0800 Subject: [PATCH 3/3] docs: update PR description test count to match actual 5 tests --- pr-body.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pr-body.md b/pr-body.md index 6ce4e7dad8..2955e3ebc8 100644 --- a/pr-body.md +++ b/pr-body.md @@ -7,7 +7,7 @@ Currently, when an agent produces output that fails Pydantic validation, the con ## Changes - **converter.py**: Added `_handle_and_retry`, `_convert_with_retry` helpers (sync + async) with error accumulation across retry attempts. The `max_retries` parameter controls how many LLM retries to attempt. - **task.py**: Added `output_validation_max_retries` field to `Task`, passed through to `convert_to_model` and `async_convert_to_model`. -- **tests**: 4 new tests in `TestConvertToModelWithRetry` covering success, exhaustion, zero-retry mode, and error context injection. +- **tests**: 5 new tests in `TestConvertToModelWithRetry` covering success, exhaustion, zero-retry (no LLM call), zero-retry (converter_cls path), and error context injection. ## Usage ```python @@ -20,7 +20,7 @@ task = Task( ``` ## Test Plan -- 4 new tests pass +- 5 new tests pass - All 50 existing converter tests pass - 1 pre-existing failure (test_supports_function_calling_false, missing litellm, unrelated) - Backward compatible: default max_retries=0 preserves existing behavior