From 2b6f1d5069fd4e0c883e135e4ec3e9f3544c0919 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 18:25:52 +0300 Subject: [PATCH 01/10] feat(config): add constraint fields to OutputField schema model --- src/conductor/config/schema.py | 93 +++++++++++++++++++- tests/test_config/test_schema.py | 146 +++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+), 1 deletion(-) diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 2f78b654..aabab7d8 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -6,6 +6,7 @@ from __future__ import annotations +import re from typing import Any, Literal, get_args from urllib.parse import urlparse @@ -99,15 +100,105 @@ class OutputField(BaseModel): properties: dict[str, OutputField] | None = None """For object types, the schema of object properties.""" + enum: list[Any] | None = None + """Allowed values for scalar types.""" + + pattern: str | None = None + """Regular expression pattern for string types.""" + + minimum: float | None = None + """Minimum value for number types.""" + + maximum: float | None = None + """Maximum value for number types.""" + + minLength: int | None = None + """Minimum length for string types.""" + + maxLength: int | None = None + """Maximum length for string types.""" + + required: bool = True + """Whether the field is required when used as an object property.""" + + nullable: bool = False + """Whether the field value may be null.""" + @model_validator(mode="after") def validate_type_specific_fields(self) -> OutputField: - """Ensure type-specific fields are properly set.""" + """Ensure type-specific fields are properly set and consistent.""" if self.type == "array" and self.items is None: # Items are optional but recommended for arrays pass if self.type == "object" and self.properties is None: # Properties are optional but recommended for objects pass + + # String-only constraints. + if self.type != "string": + for field_name in ("pattern", "minLength", "maxLength"): + value = getattr(self, field_name) + if value is not None: + raise ValueError(f"{field_name} can only be set when type is 'string'") + + # Number-only constraints. + if self.type != "number": + for field_name in ("minimum", "maximum"): + value = getattr(self, field_name) + if value is not None: + raise ValueError(f"{field_name} can only be set when type is 'number'") + + # Enum validation. + if self.enum is not None: + if self.type in ("array", "object"): + raise ValueError("enum can only be set for scalar types") + + if len(self.enum) == 0: + raise ValueError("enum must contain at least one value") + + if any(value is None for value in self.enum): + raise ValueError( + "enum cannot contain null; use nullable: true to allow null values" + ) + + type_checks = { + "string": lambda x: isinstance(x, str), + "number": lambda x: isinstance(x, int | float) and not isinstance(x, bool), + "boolean": lambda x: isinstance(x, bool), + } + check = type_checks.get(self.type) + if check is not None and not all(check(value) for value in self.enum): + raise ValueError( + f"enum values must match the declared type '{self.type}'" + ) + + # String length validation. + if self.minLength is not None and self.minLength < 0: + raise ValueError("minLength must be non-negative") + if self.maxLength is not None and self.maxLength < 0: + raise ValueError("maxLength must be non-negative") + if ( + self.minLength is not None + and self.maxLength is not None + and self.minLength > self.maxLength + ): + raise ValueError("minLength cannot be greater than maxLength") + + # Number range validation. + if ( + self.minimum is not None + and self.maximum is not None + and self.minimum > self.maximum + ): + raise ValueError("minimum cannot be greater than maximum") + + # Pattern compilation. + if self.pattern is not None: + try: + re.compile(self.pattern) + except re.error as exc: + raise ValueError(f"pattern is not a valid regular expression: {exc}") from exc + return self diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index b947c7e4..38e3800b 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -109,6 +109,152 @@ def test_object_output_with_properties(self) -> None: assert "name" in output.properties assert output.properties["name"].type == "string" + def test_output_field_constraint_defaults(self) -> None: + """Test that new constraint fields have the expected defaults.""" + output = OutputField(type="string") + assert output.enum is None + assert output.pattern is None + assert output.minimum is None + assert output.maximum is None + assert output.minLength is None + assert output.maxLength is None + assert output.required is True + assert output.nullable is False + + def test_output_field_constraint_happy_path(self) -> None: + """Test that all constraint fields are accepted on matching types.""" + output = OutputField.model_validate( + { + "type": "string", + "enum": ["a", "b"], + "pattern": "^a", + "minLength": 1, + "maxLength": 5, + "required": True, + "nullable": False, + } + ) + assert output.type == "string" + assert output.enum == ["a", "b"] + assert output.pattern == "^a" + assert output.minLength == 1 + assert output.maxLength == 5 + assert output.required is True + assert output.nullable is False + + def test_output_field_model_dump_round_trip(self) -> None: + """Test model_dump preserves constraint fields for reconstruction.""" + original = OutputField.model_validate( + { + "type": "number", + "enum": [1, 2, 3], + "minimum": 0, + "maximum": 10, + "required": False, + "nullable": True, + } + ) + dumped = original.model_dump() + rebuilt = OutputField.model_validate(dumped) + assert rebuilt.type == "number" + assert rebuilt.enum == [1, 2, 3] + assert rebuilt.minimum == 0 + assert rebuilt.maximum == 10 + assert rebuilt.required is False + assert rebuilt.nullable is True + + def test_output_field_string_constraints_reject_non_string(self) -> None: + """Test that pattern, minLength, and maxLength are rejected for non-string types.""" + for field_name, value in [ + ("pattern", "^a"), + ("minLength", 1), + ("maxLength", 5), + ]: + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "number", field_name: value}) + assert field_name in str(exc_info.value) + + def test_output_field_number_constraints_reject_non_number(self) -> None: + """Test that minimum and maximum are rejected for non-number types.""" + for field_name, value in [("minimum", 0.0), ("maximum", 10.0)]: + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", field_name: value}) + assert field_name in str(exc_info.value) + + def test_output_field_enum_rejects_array_and_object(self) -> None: + """Test that enum is rejected for array and object types.""" + for scalar_type in ("array", "object"): + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": scalar_type, "enum": ["a"]}) + assert "enum" in str(exc_info.value) + + def test_output_field_empty_enum_rejected(self) -> None: + """Test that an empty enum list is rejected.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", "enum": []}) + assert "enum" in str(exc_info.value) + + def test_output_field_negative_length_rejected(self) -> None: + """Test that negative minLength and maxLength values are rejected.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", "minLength": -1}) + assert "minLength" in str(exc_info.value) + + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", "maxLength": -1}) + assert "maxLength" in str(exc_info.value) + + def test_output_field_min_length_exceeds_max_length_rejected(self) -> None: + """Test that minLength greater than maxLength is rejected.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate( + {"type": "string", "minLength": 5, "maxLength": 1} + ) + assert "minLength" in str(exc_info.value) + assert "maxLength" in str(exc_info.value) + + def test_output_field_minimum_exceeds_maximum_rejected(self) -> None: + """Test that minimum greater than maximum is rejected.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate( + {"type": "number", "minimum": 10, "maximum": 0} + ) + assert "minimum" in str(exc_info.value) + assert "maximum" in str(exc_info.value) + + def test_output_field_invalid_pattern_rejected(self) -> None: + """Test that a pattern that does not compile as a regex is rejected.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", "pattern": "["}) + assert "pattern" in str(exc_info.value) + + def test_output_field_enum_type_mismatch_rejected(self) -> None: + """Test that enum entries must match the declared scalar type.""" + # String field with a numeric enum entry. + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", "enum": ["a", 1]}) + assert "enum" in str(exc_info.value) + + # Number field with a string enum entry (booleans also count as invalid). + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "number", "enum": [1, True]}) + assert "enum" in str(exc_info.value) + + # Boolean field with a string enum entry. + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "boolean", "enum": [True, "false"]}) + assert "enum" in str(exc_info.value) + + def test_output_field_enum_null_rejected(self) -> None: + """Test that None values inside enum are rejected with a precise message.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate( + {"type": "string", "enum": ["a", None], "nullable": True} + ) + assert "enum cannot contain null; use nullable: true to allow null values" in str( + exc_info.value + ) + class TestRouteDef: """Tests for RouteDef model.""" From f073c9533514242a62c668b18c5f64f4b3dead05 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 18:49:31 +0300 Subject: [PATCH 02/10] feat(providers): emit output constraint keywords in shared schema builders --- src/conductor/providers/_schema.py | 53 ++++- tests/test_providers/test_output_schema.py | 221 +++++++++++++++++++++ 2 files changed, 270 insertions(+), 4 deletions(-) diff --git a/src/conductor/providers/_schema.py b/src/conductor/providers/_schema.py index 98171321..04b59280 100644 --- a/src/conductor/providers/_schema.py +++ b/src/conductor/providers/_schema.py @@ -65,14 +65,35 @@ def build_json_schema_field( schema: dict[str, Any] = {"type": field.type} + if field.nullable: + schema["type"] = [field.type, "null"] + if field.description: schema["description"] = field.description + if field.enum is not None: + schema["enum"] = field.enum + + if field.type == "string": + if field.pattern is not None: + schema["pattern"] = field.pattern + if field.minLength is not None: + schema["minLength"] = field.minLength + if field.maxLength is not None: + schema["maxLength"] = field.maxLength + elif field.type == "number": + if field.minimum is not None: + schema["minimum"] = field.minimum + if field.maximum is not None: + schema["maximum"] = field.maximum + if field.type == "object" and field.properties: schema["properties"] = build_json_schema_properties( field.properties, depth=depth + 1, max_depth=max_depth ) - schema["required"] = list(field.properties.keys()) + required = [name for name, prop in field.properties.items() if prop.required] + if required: + schema["required"] = required if field.type == "array" and field.items: schema["items"] = build_json_schema_field(field.items, depth=depth + 1, max_depth=max_depth) @@ -129,14 +150,38 @@ def build_prompt_schema_field( _check_depth(depth, max_depth) schema: dict[str, Any] = {"type": field.type} - if field.description: - schema["description"] = field.description + if field.nullable: + schema["type"] = [field.type, "null"] + + description = field.description + if description and field.required is False: + description += " (optional)" + if description: + schema["description"] = description + + if field.enum is not None: + schema["enum"] = field.enum + + if field.type == "string": + if field.pattern is not None: + schema["pattern"] = field.pattern + if field.minLength is not None: + schema["minLength"] = field.minLength + if field.maxLength is not None: + schema["maxLength"] = field.maxLength + elif field.type == "number": + if field.minimum is not None: + schema["minimum"] = field.minimum + if field.maximum is not None: + schema["maximum"] = field.maximum if field.type == "object" and field.properties: schema["properties"] = build_prompt_schema_properties( field.properties, depth=depth + 1, max_depth=max_depth ) - schema["required"] = list(field.properties.keys()) + required = [name for name, prop in field.properties.items() if prop.required] + if required: + schema["required"] = required if field.type == "array" and field.items: schema["items"] = build_prompt_schema_field( diff --git a/tests/test_providers/test_output_schema.py b/tests/test_providers/test_output_schema.py index 1946ddf7..f8b95c77 100644 --- a/tests/test_providers/test_output_schema.py +++ b/tests/test_providers/test_output_schema.py @@ -13,6 +13,7 @@ from unittest.mock import AsyncMock from conductor.config.schema import OutputField +from conductor.providers._schema import build_json_schema_field, build_prompt_schema_field from conductor.providers.claude_agent_sdk import _build_output_format from conductor.providers.copilot import CopilotProvider from conductor.providers.hermes import _build_prompt_schema @@ -756,3 +757,223 @@ def test_golden_assertion_detects_missing_description(self) -> None: } actual = _serialize(provider._build_prompt_schema(schema_without_description)) assert actual != EXPECTED_COPILOT_RICH_SCHEMA + + +class TestSharedSchemaBuilder: + """Tests for the provider-neutral JSON and prompt schema builders.""" + + def test_json_schema_emits_all_constraint_keywords(self) -> None: + """JSON-schema builder must emit enum, pattern, minLength, and maxLength + for string fields, minimum and maximum for number fields, render nullable + fields as a type array, and exclude optional properties from the required + array.""" + field = OutputField( + type="object", + description="root", + properties={ + "status": OutputField( + type="string", + description="status", + enum=["ok", "fail"], + pattern="^[a-z]+$", + minLength=1, + maxLength=10, + ), + "score": OutputField( + type="number", + description="score", + minimum=0.0, + maximum=100.0, + ), + "count": OutputField( + type="number", + description="count", + nullable=True, + ), + "optional_note": OutputField( + type="string", + description="optional note", + required=False, + ), + "nested": OutputField( + type="object", + description="nested", + required=False, + properties={ + "flag": OutputField( + type="boolean", + description="flag", + required=False, + ), + }, + ), + }, + ) + + actual = build_json_schema_field(field) + + assert actual == { + "type": "object", + "description": "root", + "properties": { + "status": { + "type": "string", + "description": "status", + "enum": ["ok", "fail"], + "pattern": "^[a-z]+$", + "minLength": 1, + "maxLength": 10, + }, + "score": { + "type": "number", + "description": "score", + "minimum": 0.0, + "maximum": 100.0, + }, + "count": { + "type": ["number", "null"], + "description": "count", + }, + "optional_note": { + "type": "string", + "description": "optional note", + }, + "nested": { + "type": "object", + "description": "nested", + "properties": { + "flag": { + "type": "boolean", + "description": "flag", + }, + }, + }, + }, + "required": ["status", "score", "count"], + } + + def test_json_schema_omits_required_when_all_properties_optional(self) -> None: + """An object whose properties are all optional must not include a required + key in the JSON-schema fragment.""" + field = OutputField( + type="object", + description="all optional", + properties={ + "first": OutputField(type="string", required=False), + "second": OutputField(type="number", required=False), + }, + ) + + actual = build_json_schema_field(field) + + assert "required" not in actual + assert actual == { + "type": "object", + "description": "all optional", + "properties": { + "first": {"type": "string"}, + "second": {"type": "number"}, + }, + } + + def test_json_schema_never_emits_default_keys(self) -> None: + """Optional object fields must not leak pydantic-core default values into + the schema fragment at any nesting level.""" + field = OutputField( + type="object", + required=False, + properties={ + "leaf": OutputField(type="string", required=False), + }, + ) + + actual = build_json_schema_field(field) + + def _no_default_keys(value: Any) -> bool: + if isinstance(value, dict): + return "default" not in value and all( + _no_default_keys(v) for v in value.values() + ) + if isinstance(value, list): + return all(_no_default_keys(item) for item in value) + return True + + assert _no_default_keys(actual) + + def test_prompt_schema_emits_constraints_and_optional_suffix(self) -> None: + """Prompt-schema builder must emit the same constraint keywords as the JSON + builder and append ' (optional)' to descriptions of optional fields that + have a real description.""" + field = OutputField( + type="object", + description="root", + properties={ + "status": OutputField( + type="string", + description="status", + enum=["ok", "fail"], + pattern="^[a-z]+$", + minLength=1, + maxLength=10, + ), + "score": OutputField( + type="number", + description="score", + minimum=0.0, + maximum=100.0, + ), + "optional_note": OutputField( + type="string", + description="optional note", + required=False, + ), + }, + ) + + actual = build_prompt_schema_field(field) + + assert actual == { + "type": "object", + "description": "root", + "properties": { + "status": { + "type": "string", + "description": "status", + "enum": ["ok", "fail"], + "pattern": "^[a-z]+$", + "minLength": 1, + "maxLength": 10, + }, + "score": { + "type": "number", + "description": "score", + "minimum": 0.0, + "maximum": 100.0, + }, + "optional_note": { + "type": "string", + "description": "optional note (optional)", + }, + }, + "required": ["status", "score"], + } + + def test_prompt_schema_omits_description_for_optional_field_without_description(self) -> None: + """An optional field without a description must not synthesize a description + key in the prompt schema fragment.""" + field = OutputField( + type="object", + properties={ + "opt": OutputField(type="string", required=False), + }, + ) + + actual = build_prompt_schema_field(field) + + assert "description" not in actual["properties"]["opt"] + assert actual == { + "type": "object", + "properties": { + "opt": {"type": "string"}, + }, + } From bc69065a86c0f498e4ca24a16710b39743eb8425 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 19:51:00 +0300 Subject: [PATCH 03/10] feat(executor,config): enforce output constraints and reject root-level optional fields --- src/conductor/config/schema.py | 28 +++ src/conductor/executor/output.py | 64 ++++++ .../test_config/test_root_optional_output.py | 154 +++++++++++++++ tests/test_executor/test_output.py | 182 ++++++++++++++++++ 4 files changed, 428 insertions(+) create mode 100644 tests/test_config/test_root_optional_output.py diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index aabab7d8..842c77f0 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -3019,3 +3019,31 @@ def validate_references(self) -> WorkflowConfig: ) return self + + @model_validator(mode="after") + def validate_root_level_output_required(self) -> WorkflowConfig: + """Reject top-level optional output fields on agents and for-each agents. + + Object properties may still be optional; the policy only applies to the + root output dict of an agent definition. + """ + for agent in self.agents: + if agent.output: + for field_name, field in agent.output.items(): + if not field.required: + raise ValueError( + f"Agent '{agent.name}' output field '{field_name}': " + "root-level output fields cannot be optional " + "(required: false is only allowed inside object properties)" + ) + for for_each_group in self.for_each: + agent = for_each_group.agent + if agent.output: + for field_name, field in agent.output.items(): + if not field.required: + raise ValueError( + f"Agent '{agent.name}' output field '{field_name}': " + "root-level output fields cannot be optional " + "(required: false is only allowed inside object properties)" + ) + return self diff --git a/src/conductor/executor/output.py b/src/conductor/executor/output.py index 14ef617c..e5a6f628 100644 --- a/src/conductor/executor/output.py +++ b/src/conductor/executor/output.py @@ -6,6 +6,7 @@ from __future__ import annotations +import re from typing import Any from conductor.config.schema import OutputField @@ -36,6 +37,8 @@ def validate_output( """ for field_name, field_def in schema.items(): if field_name not in content: + if not field_def.required: + continue raise ValidationError( f"Missing required output field: {field_name}", suggestion=f"Ensure agent returns '{field_name}' in output", @@ -44,6 +47,57 @@ def validate_output( _validate_field(field_name, content[field_name], field_def) +def _check_constraints(field_name: str, value: Any, field_def: OutputField) -> None: + """Validate scalar constraints (enum, pattern, length, range) for a field. + + Called after the type check has passed, so ``value`` is known to match + ``field_def.type``. Raises ``ValidationError`` with a suggestion on failure. + """ + if field_def.enum is not None: + if isinstance(value, bool): + if not (field_def.type == "boolean" and value in field_def.enum): + raise ValidationError( + f"Output field '{field_name}' must be one of {field_def.enum!r}, " + f"got {value!r}", + suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", + ) + elif value not in field_def.enum: + raise ValidationError( + f"Output field '{field_name}' must be one of {field_def.enum!r}, " + f"got {value!r}", + suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", + ) + + if field_def.pattern is not None and re.search(field_def.pattern, value) is None: + raise ValidationError( + f"Output field '{field_name}' does not match pattern '{field_def.pattern}'", + suggestion=f"Ensure '{field_name}' matches the pattern '{field_def.pattern}'", + ) + + if field_def.type == "string": + if field_def.minLength is not None and len(value) < field_def.minLength: + raise ValidationError( + f"Output field '{field_name}' is shorter than minLength {field_def.minLength}", + suggestion=f"Ensure '{field_name}' has at least {field_def.minLength} characters", + ) + if field_def.maxLength is not None and len(value) > field_def.maxLength: + raise ValidationError( + f"Output field '{field_name}' is longer than maxLength {field_def.maxLength}", + suggestion=f"Ensure '{field_name}' has at most {field_def.maxLength} characters", + ) + elif field_def.type == "number": + if field_def.minimum is not None and value < field_def.minimum: + raise ValidationError( + f"Output field '{field_name}' is below minimum {field_def.minimum}", + suggestion=f"Ensure '{field_name}' is at least {field_def.minimum}", + ) + if field_def.maximum is not None and value > field_def.maximum: + raise ValidationError( + f"Output field '{field_name}' is above maximum {field_def.maximum}", + suggestion=f"Ensure '{field_name}' is at most {field_def.maximum}", + ) + + def _validate_field(field_name: str, value: Any, field_def: OutputField) -> None: """Validate a single value against its output field definition. @@ -60,6 +114,14 @@ def _validate_field(field_name: str, value: Any, field_def: OutputField) -> None Raises: ValidationError: If the value or any nested value doesn't match. """ + if value is None: + if field_def.nullable: + return + raise ValidationError( + f"Output field '{field_name}' must not be null", + suggestion=f"Ensure '{field_name}' is not null or set nullable: true", + ) + if not check_type(value, field_def.type): raise ValidationError( f"Output field '{field_name}' has wrong type: " @@ -68,6 +130,8 @@ def _validate_field(field_name: str, value: Any, field_def: OutputField) -> None suggestion=f"Ensure agent returns correct type for '{field_name}'", ) + _check_constraints(field_name, value, field_def) + if field_def.type == "object" and field_def.properties and isinstance(value, dict): validate_output(value, field_def.properties) diff --git a/tests/test_config/test_root_optional_output.py b/tests/test_config/test_root_optional_output.py new file mode 100644 index 00000000..b6834678 --- /dev/null +++ b/tests/test_config/test_root_optional_output.py @@ -0,0 +1,154 @@ +"""Tests for root-level optional output fields gate. + +Requirement: root-level agent output fields must be required; optional fields +(required: false) are only allowed inside object properties. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from conductor.config.loader import load_config, load_config_string +from conductor.config.validator import validate_workflow_config + + +class TestRootLevelOptionalOutputGate: + """Root-level required: false must be rejected at load and validate time.""" + + def _workflow_with_root_optional(self) -> str: + return """ +workflow: + name: root-optional-test + entry_point: agent1 + +agents: + - name: agent1 + model: gpt-4 + prompt: "Hello" + output: + answer: + type: string + required: false + routes: + - to: $end +""" + + def _workflow_with_nested_optional(self) -> str: + return """ +workflow: + name: nested-optional-test + entry_point: agent1 + +agents: + - name: agent1 + model: gpt-4 + prompt: "Hello" + output: + data: + type: object + properties: + optional_child: + type: string + required: false + routes: + - to: $end +""" + + def _workflow_with_for_each_root_optional(self) -> str: + return """ +workflow: + name: foreach-optional-test + entry_point: finder + +agents: + - name: finder + model: gpt-4 + prompt: "Find items" + output: + items: + type: array + items: + type: string + routes: + - to: processors + +for_each: + - name: processors + type: for_each + source: finder.output.items + as: item + agent: + name: processor + model: gpt-4 + prompt: "Process {{ item }}" + output: + result: + type: string + required: false + routes: + - to: $end +""" + + def test_root_optional_rejected_by_load_config_string(self) -> None: + """load_config_string must reject a root-level optional output field.""" + with pytest.raises(Exception) as exc_info: + load_config_string(self._workflow_with_root_optional()) + + message = str(exc_info.value) + assert "Agent 'agent1' output field 'answer'" in message + assert "root-level output fields cannot be optional" in message + + def test_root_optional_rejected_by_validate_command_load_path(self, tmp_path: Path) -> None: + """load_config must reject a root-level optional output field, + + matching the validate command path. + """ + workflow_file = tmp_path / "workflow.yaml" + workflow_file.write_text(self._workflow_with_root_optional()) + with pytest.raises(Exception) as exc_info: + load_config(str(workflow_file)) + + message = str(exc_info.value) + assert "Agent 'agent1' output field 'answer'" in message + assert "root-level output fields cannot be optional" in message + + def test_nested_optional_allowed_by_load_config_string(self) -> None: + """Optional fields inside object properties must load without error.""" + config = load_config_string(self._workflow_with_nested_optional()) + assert config.agents[0].output is not None + properties = config.agents[0].output["data"].properties + assert properties is not None + assert properties["optional_child"].required is False + + def test_nested_optional_allowed_by_validate_workflow_config(self) -> None: + """Optional fields inside object properties must validate without error.""" + config = load_config_string(self._workflow_with_nested_optional()) + # Should not raise + validate_workflow_config(config) + + def test_for_each_inline_root_optional_rejected_by_load_config_string(self) -> None: + """A root-level optional field on an inline for-each agent must be rejected.""" + with pytest.raises(Exception) as exc_info: + load_config_string(self._workflow_with_for_each_root_optional()) + + message = str(exc_info.value) + assert "Agent 'processor' output field 'result'" in message + assert "root-level output fields cannot be optional" in message + + def test_for_each_inline_root_optional_rejected_by_validate_command_load_path( + self, tmp_path: Path + ) -> None: + """load_config must reject a root-level optional field on an inline for-each agent, + + matching the validate command path. + """ + workflow_file = tmp_path / "workflow.yaml" + workflow_file.write_text(self._workflow_with_for_each_root_optional()) + with pytest.raises(Exception) as exc_info: + load_config(str(workflow_file)) + + message = str(exc_info.value) + assert "Agent 'processor' output field 'result'" in message + assert "root-level output fields cannot be optional" in message diff --git a/tests/test_executor/test_output.py b/tests/test_executor/test_output.py index c9fbe372..4192fd27 100644 --- a/tests/test_executor/test_output.py +++ b/tests/test_executor/test_output.py @@ -441,6 +441,188 @@ def test_parse_json_with_multiple_fenced_blocks_first_wins(self) -> None: assert result == {"a": 1} +class TestValidateOutputConstraints: + """Tests for output field constraint validation.""" + + def test_nullable_null_passes(self) -> None: + """A nullable field must accept an explicit null value.""" + schema = {"value": OutputField(type="string", nullable=True)} + validate_output({"value": None}, schema) + + def test_nullable_null_rejected_when_not_nullable(self) -> None: + """A non-nullable field must reject an explicit null value.""" + schema = {"value": OutputField(type="string", nullable=False)} + with pytest.raises(ValidationError, match="Output field 'value' must not be null"): + validate_output({"value": None}, schema) + + def test_enum_passes_when_value_is_member(self) -> None: + """A value listed in enum must pass.""" + schema = {"color": OutputField(type="string", enum=["red", "blue"])} + validate_output({"color": "blue"}, schema) + + def test_enum_rejects_non_member(self) -> None: + """A value not listed in enum must raise a precise error.""" + schema = {"color": OutputField(type="string", enum=["red", "blue"])} + with pytest.raises( + ValidationError, + match="Output field 'color' must be one of \\['red', 'blue'\\], got 'green'", + ): + validate_output({"color": "green"}, schema) + + def test_enum_number_accepts_float_int_equality(self) -> None: + """A number enum [1] must accept 1.0 using plain Python equality.""" + schema = {"value": OutputField(type="number", enum=[1])} + validate_output({"value": 1.0}, schema) + + def test_enum_number_rejects_boolean(self) -> None: + """A number enum [1] rejects True at the type layer (bool is not a number), + + mirroring the _reject_bool BeforeValidator on the pydantic path. + """ + schema = {"value": OutputField(type="number", enum=[1])} + with pytest.raises(ValidationError, match="has wrong type"): + validate_output({"value": True}, schema) + + def test_enum_boolean_only_matches_boolean_enum(self) -> None: + """A boolean value must only match a boolean enum, not a number enum.""" + schema = {"value": OutputField(type="boolean", enum=[True])} + validate_output({"value": True}, schema) + + def test_pattern_passes(self) -> None: + """A string matching the regex pattern must pass.""" + schema = {"value": OutputField(type="string", pattern=r"^a")} + validate_output({"value": "abc"}, schema) + + def test_pattern_rejects_non_matching(self) -> None: + """A string not matching the regex pattern must raise a precise error.""" + schema = {"value": OutputField(type="string", pattern=r"^a")} + with pytest.raises( + ValidationError, + match="Output field 'value' does not match pattern '\\^a'", + ): + validate_output({"value": "xyz"}, schema) + + def test_min_length_passes(self) -> None: + """A string meeting minLength must pass.""" + schema = {"value": OutputField(type="string", minLength=2)} + validate_output({"value": "ab"}, schema) + + def test_min_length_rejects_too_short(self) -> None: + """A string shorter than minLength must raise a precise error.""" + schema = {"value": OutputField(type="string", minLength=2)} + with pytest.raises( + ValidationError, + match="Output field 'value' is shorter than minLength 2", + ): + validate_output({"value": "a"}, schema) + + def test_max_length_passes(self) -> None: + """A string meeting maxLength must pass.""" + schema = {"value": OutputField(type="string", maxLength=3)} + validate_output({"value": "abc"}, schema) + + def test_max_length_rejects_too_long(self) -> None: + """A string longer than maxLength must raise a precise error.""" + schema = {"value": OutputField(type="string", maxLength=3)} + with pytest.raises( + ValidationError, + match="Output field 'value' is longer than maxLength 3", + ): + validate_output({"value": "abcd"}, schema) + + def test_minimum_passes(self) -> None: + """A number meeting minimum must pass.""" + schema = {"value": OutputField(type="number", minimum=0)} + validate_output({"value": 0}, schema) + + def test_minimum_rejects_below(self) -> None: + """A number below minimum must raise a precise error.""" + schema = {"value": OutputField(type="number", minimum=0)} + with pytest.raises( + ValidationError, + match="Output field 'value' is below minimum 0", + ): + validate_output({"value": -1}, schema) + + def test_maximum_passes(self) -> None: + """A number meeting maximum must pass.""" + schema = {"value": OutputField(type="number", maximum=10)} + validate_output({"value": 10}, schema) + + def test_maximum_rejects_above(self) -> None: + """A number above maximum must raise a precise error.""" + schema = {"value": OutputField(type="number", maximum=10)} + with pytest.raises( + ValidationError, + match="Output field 'value' is above maximum 10", + ): + validate_output({"value": 11}, schema) + + def test_optional_field_absent_is_allowed(self) -> None: + """An optional object property omitted from content must not raise.""" + schema = { + "value": OutputField(type="string"), + "extra": OutputField(type="string", required=False), + } + validate_output({"value": "present"}, schema) + + def test_optional_field_present_is_validated(self) -> None: + """An optional object property present must still pass validation.""" + schema = { + "value": OutputField(type="string"), + "extra": OutputField(type="string", required=False, maxLength=3), + } + validate_output({"value": "present", "extra": "ok"}, schema) + + def test_optional_field_present_violates_constraints(self) -> None: + """An optional object property present with a bad value must raise.""" + schema = { + "value": OutputField(type="string"), + "extra": OutputField(type="string", required=False, maxLength=3), + } + with pytest.raises(ValidationError, match="longer than maxLength 3"): + validate_output({"value": "present", "extra": "too long"}, schema) + + def test_constraints_inside_array_object_items(self) -> None: + """Constraints inside array items must be enforced recursively.""" + schema = { + "items": OutputField( + type="array", + items=OutputField( + type="object", + properties={ + "tag": OutputField(type="string", pattern=r"^[ab]$"), + "score": OutputField(type="number", minimum=0, maximum=1), + }, + ), + ) + } + validate_output( + {"items": [{"tag": "a", "score": 0.5}, {"tag": "b", "score": 1.0}]}, + schema, + ) + + def test_constraints_inside_array_object_items_rejected(self) -> None: + """A nested item constraint violation inside array must raise.""" + schema = { + "items": OutputField( + type="array", + items=OutputField( + type="object", + properties={ + "tag": OutputField(type="string", pattern=r"^[ab]$"), + "score": OutputField(type="number", minimum=0, maximum=1), + }, + ), + ) + } + with pytest.raises(ValidationError, match="does not match pattern"): + validate_output( + {"items": [{"tag": "c", "score": 0.5}]}, + schema, + ) + + class TestValidationErrorValueDescription: """The error names the offending value, without echoing secrets. From 98fc9c56eb1340ae25ff587103b4cbe99ffdbe63 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 19:58:15 +0300 Subject: [PATCH 04/10] feat(providers): map output constraints onto pydantic-ai dynamic models --- .../providers/_pydantic_ai/converters.py | 163 +++++++++++++- .../_pydantic_ai/structured_output.py | 2 +- .../test_pydantic_ai_agent_builder.py | 66 ++++++ .../test_pydantic_ai_structured_output.py | 203 ++++++++++++++++++ 4 files changed, 427 insertions(+), 7 deletions(-) diff --git a/src/conductor/providers/_pydantic_ai/converters.py b/src/conductor/providers/_pydantic_ai/converters.py index 67fe236e..9c1e9a33 100644 --- a/src/conductor/providers/_pydantic_ai/converters.py +++ b/src/conductor/providers/_pydantic_ai/converters.py @@ -7,9 +7,10 @@ from __future__ import annotations +import re from typing import Annotated, Any -from pydantic import BaseModel, BeforeValidator, ConfigDict, Field, create_model +from pydantic import AfterValidator, BaseModel, BeforeValidator, ConfigDict, Field, create_model from conductor.config.schema import OutputField @@ -34,6 +35,80 @@ def _reject_bool(value: Any) -> Any: """Conductor ``integer`` type: accepts integers, rejects booleans.""" +class _NoDefaultBaseModel(BaseModel): + """Dynamic model base that strips ``default`` keys from JSON schemas. + + Pydantic v2 emits ``"default": null`` for ``Field(default=None)``. + Pydantic AI's tool schema must not include the JSON Schema ``default`` + keyword (it is not allowed on tool parameters), so this base removes it + recursively from the generated schema, including nested ``$defs``. + """ + + model_config = ConfigDict(extra="allow") + + @classmethod + def __get_pydantic_json_schema__(cls, core_schema: Any, handler: Any) -> dict[str, Any]: + schema = handler(core_schema) + _strip_default_keys(schema) + return schema + + +def _strip_default_keys(schema: Any) -> None: + """Recursively remove ``default`` keys from a JSON schema dict in place.""" + if isinstance(schema, dict): + schema.pop("default", None) + for value in schema.values(): + _strip_default_keys(value) + elif isinstance(schema, list): + for item in schema: + _strip_default_keys(item) + + +def _make_enum_validator(enum_values: list[Any], field_type: str) -> Any: + """Return an AfterValidator that enforces enum membership. + + Matches the shared semantics used by ``validate_output``: + + - A boolean value only passes when the field type is ``boolean`` and the + value is present in the enum. + - For all other values, plain Python membership is used (so ``1.0`` + satisfies a number enum ``[1]``). + """ + + def _validate(value: Any) -> Any: + if value is None: + return value + if isinstance(value, bool): + if field_type == "boolean" and value in enum_values: + return value + raise ValueError(f"{value!r} is not a valid boolean enum value") + if value in enum_values: + return value + raise ValueError(f"{value!r} is not one of {enum_values!r}") + + return _validate + + +def _make_pattern_validator(pattern: str) -> Any: + """Return an AfterValidator that runs Python ``re.search``. + + Mirrors ``validate_output`` so Python-only regex constructs (lookarounds, + backreferences) are evaluated by Python's regex engine, not pydantic's + Rust-based default. + """ + + def _validate(value: Any) -> Any: + if value is None: + return value + if not isinstance(value, str): + raise ValueError("pattern can only be applied to strings") + if re.search(pattern, value) is None: + raise ValueError(f"value does not match pattern {pattern!r}") + return value + + return _validate + + def _to_pascal(snake: str) -> str: """Convert a snake_case identifier to PascalCase. @@ -49,6 +124,84 @@ def _to_pascal(snake: str) -> str: return "".join(part.capitalize() for part in snake.split("_")) +def _field_json_schema_extra(field: OutputField) -> dict[str, Any] | None: + """Build ``json_schema_extra`` advertising Conductor constraints. + + Pydantic AI attaches this to the generated tool schema so the model sees + the same ``enum``/``pattern``/length/range keywords as the shared JSON + Schema builders. + """ + extra: dict[str, Any] = {} + if field.enum is not None: + extra["enum"] = field.enum + if field.pattern is not None: + extra["pattern"] = field.pattern + if field.minLength is not None: + extra["minLength"] = field.minLength + if field.maxLength is not None: + extra["maxLength"] = field.maxLength + if field.minimum is not None: + extra["minimum"] = field.minimum + if field.maximum is not None: + extra["maximum"] = field.maximum + return extra if extra else None + + +def _wrap_scalar_field_type(field: OutputField, base_type: Any) -> Any: + """Wrap a scalar base type with validators and nullability. + + Adds enum/pattern validators via ``AfterValidator`` and unions with + ``None`` for nullable fields. Optional fields are handled at the + ``Field`` level (``default=None``); the type union only changes when the + field is explicitly nullable. + """ + annotated = base_type + if field.enum is not None: + annotated = Annotated[ + annotated, + AfterValidator(_make_enum_validator(field.enum, field.type)), + ] + if field.pattern is not None: + annotated = Annotated[annotated, AfterValidator(_make_pattern_validator(field.pattern))] + + if field.nullable: + annotated = annotated | None + + return annotated + + +def _build_field_info(field: OutputField) -> Any: + """Build a Pydantic ``FieldInfo`` for a Conductor output field. + + Length/range constraints are enforced by ``Field`` itself (no regex + involved). Optional fields get ``default=None``; required fields stay + required with ``default=...``. The description is included when present, + and constraint metadata is mirrored in ``json_schema_extra`` so the + generated tool schema advertises them. + """ + extra = _field_json_schema_extra(field) + field_kwargs: dict[str, Any] = {} + if field.description: + field_kwargs["description"] = field.description + if extra is not None: + field_kwargs["json_schema_extra"] = extra + + if field.type == "string": + if field.minLength is not None: + field_kwargs["min_length"] = field.minLength + if field.maxLength is not None: + field_kwargs["max_length"] = field.maxLength + elif field.type in ("number", "integer"): + if field.minimum is not None: + field_kwargs["ge"] = field.minimum + if field.maximum is not None: + field_kwargs["le"] = field.maximum + + field_kwargs["default"] = ... if field.required else None + + return Field(**field_kwargs) + + def _map_output_field_type( field: OutputField, *, @@ -152,17 +305,15 @@ def _build_pydantic_model( depth=depth, max_depth=max_depth, ) - field_info = ( - Field(description=field.description) if field.description else Field(default=...) - ) + field_type = _wrap_scalar_field_type(field, field_type) + field_info = _build_field_info(field) model_fields[field_name] = (field_type, field_info) return create_model( name, **model_fields, - __base__=BaseModel, + __base__=_NoDefaultBaseModel, __doc__=description, - __config__=ConfigDict(extra="allow"), ) diff --git a/src/conductor/providers/_pydantic_ai/structured_output.py b/src/conductor/providers/_pydantic_ai/structured_output.py index bf626cd3..c5271969 100644 --- a/src/conductor/providers/_pydantic_ai/structured_output.py +++ b/src/conductor/providers/_pydantic_ai/structured_output.py @@ -55,7 +55,7 @@ def extract_content( return _wrap_text_output(output) if isinstance(output, BaseModel): - content = output.model_dump() + content = output.model_dump(exclude_unset=True) content = normalize_agent_output(content, output_schema) validate_output(content, output_schema) return content diff --git a/tests/test_providers/test_pydantic_ai_agent_builder.py b/tests/test_providers/test_pydantic_ai_agent_builder.py index 555a051f..3664c05b 100644 --- a/tests/test_providers/test_pydantic_ai_agent_builder.py +++ b/tests/test_providers/test_pydantic_ai_agent_builder.py @@ -352,6 +352,72 @@ async def _fake_model( await agent.run("go") +class TestConstraintOutputRetry: + """Tests that structured-output constraint violations trigger pydantic-ai output retries.""" + + @pytest.mark.asyncio + async def test_constraint_violation_triggers_output_retry( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """When the model first returns a tool call that violates an output + constraint, pydantic-ai's output retry must recover and return valid + structured output after exactly one retry.""" + calls: list[int] = [] + + async def _fake_model( + messages: list[Any], + info: Any, + ) -> ModelResponse: + calls.append(len(calls)) + output_tool_name = info.output_tools[0].name + if len(calls) == 1: + return ModelResponse( + parts=[ + ToolCallPart( + tool_name=output_tool_name, + args={"value": "bad"}, + ) + ] + ) + return ModelResponse( + parts=[ + ToolCallPart( + tool_name=output_tool_name, + args={"value": "abc"}, + ) + ] + ) + + monkeypatch.setattr( + "conductor.providers._pydantic_ai.agent_builder._resolve_anthropic_model", + lambda *_args, **_kwargs: FunctionModel(_fake_model), + ) + + agent_def = AgentDef( + name="formatter", + output={ + "value": OutputField( + type="string", + enum=["abc"], + pattern=r"^a", + minLength=3, + maxLength=3, + ) + }, + ) + pydantic_agent = build_agent(agent_def, system_prompt="sys", rendered_prompt="go") + + result = await pydantic_agent.run("go") + + assert len(calls) == 2 + assert result.output.value == "abc" + # Inspect the message history to confirm one output retry took place. + messages = result.all_messages() + model_responses = [m for m in messages if isinstance(m, ModelResponse)] + assert len(model_responses) == 2 + + class TestApiKey: """Tests for API key and auth token resolution.""" diff --git a/tests/test_providers/test_pydantic_ai_structured_output.py b/tests/test_providers/test_pydantic_ai_structured_output.py index 3a04042c..b71731ac 100644 --- a/tests/test_providers/test_pydantic_ai_structured_output.py +++ b/tests/test_providers/test_pydantic_ai_structured_output.py @@ -13,6 +13,7 @@ import pytest from pydantic import BaseModel +from pydantic import ValidationError as PydanticValidationError from pydantic_ai import Agent from pydantic_ai.models.test import TestModel from pydantic_ai.output import ToolOutput @@ -183,3 +184,205 @@ def test_text_fallback_unwraps_scalar_wrapper(self) -> None: content = parse_text_fallback(text, output_schema, "formatter") assert content == {"answer": "from wrapper"} + + +class TestEnumConstraint: + """Requirement: enum membership matches validate_output semantics.""" + + def test_number_enum_rejects_bool(self) -> None: + """A number enum [1] must reject a boolean value; bool is not an int.""" + output_schema = {"value": OutputField(type="number", enum=[1])} + dynamic_model = output_schema_to_pydantic_model("EnumOutput", output_schema) + assert dynamic_model is not None + + with pytest.raises(PydanticValidationError): + dynamic_model(value=True) + + def test_number_enum_accepts_float_int_equality(self) -> None: + """A number enum [1] must accept 1.0 using Python equality.""" + output_schema = {"value": OutputField(type="number", enum=[1])} + dynamic_model = output_schema_to_pydantic_model("EnumOutput", output_schema) + assert dynamic_model is not None + + instance = dynamic_model(value=1.0) + assert instance.value == 1.0 + + def test_string_enum_rejects_non_member(self) -> None: + """A string enum must reject a value not listed in enum.""" + output_schema = {"value": OutputField(type="string", enum=["a", "b"])} + dynamic_model = output_schema_to_pydantic_model("EnumOutput", output_schema) + assert dynamic_model is not None + + with pytest.raises(PydanticValidationError): + dynamic_model(value="c") + + +class TestPatternConstraint: + """Requirement: pattern is validated with Python regex, not Rust regex.""" + + def test_lookahead_pattern_builds_and_validates(self) -> None: + """A Python-only lookahead pattern must build cleanly and reject values + that do not match.""" + output_schema = {"value": OutputField(type="string", pattern=r"^(?=.*A).*$")} + dynamic_model = output_schema_to_pydantic_model("PatternOutput", output_schema) + assert dynamic_model is not None + + assert dynamic_model(value="Abc").value == "Abc" + with pytest.raises(PydanticValidationError): + dynamic_model(value="bc") + + +class TestLengthAndRangeConstraints: + """Requirement: minLength/maxLength/minimum/maximum are enforced.""" + + def test_min_length_and_max_length_reject_violations(self) -> None: + """String length constraints must reject too-short or too-long values.""" + output_schema = {"value": OutputField(type="string", minLength=2, maxLength=3)} + dynamic_model = output_schema_to_pydantic_model("LengthOutput", output_schema) + assert dynamic_model is not None + + assert dynamic_model(value="ab").value == "ab" + assert dynamic_model(value="abc").value == "abc" + with pytest.raises(PydanticValidationError): + dynamic_model(value="a") + with pytest.raises(PydanticValidationError): + dynamic_model(value="abcd") + + def test_minimum_and_maximum_reject_violations(self) -> None: + """Number range constraints must reject out-of-bounds values.""" + output_schema = {"value": OutputField(type="number", minimum=0, maximum=10)} + dynamic_model = output_schema_to_pydantic_model("RangeOutput", output_schema) + assert dynamic_model is not None + + assert dynamic_model(value=0).value == 0 + assert dynamic_model(value=10).value == 10 + with pytest.raises(PydanticValidationError): + dynamic_model(value=-1) + with pytest.raises(PydanticValidationError): + dynamic_model(value=11) + + +class TestNullableAndOptional: + """Requirement: nullable and optional fields behave as specified.""" + + def test_explicit_none_nullable_passes(self) -> None: + """A nullable field must accept an explicit None value.""" + output_schema = {"value": OutputField(type="string", nullable=True)} + dynamic_model = output_schema_to_pydantic_model("NullableOutput", output_schema) + assert dynamic_model is not None + + instance = dynamic_model(value=None) + assert instance.value is None + + def test_explicit_none_non_nullable_optional_fails(self) -> None: + """A non-nullable optional field must reject an explicit None.""" + output_schema = {"value": OutputField(type="string", required=False, nullable=False)} + dynamic_model = output_schema_to_pydantic_model("OptionalOutput", output_schema) + assert dynamic_model is not None + + with pytest.raises(PydanticValidationError): + dynamic_model(value=None) + + def test_omitted_optional_excluded_from_extract_content(self) -> None: + """An omitted optional field must not appear in the extracted content dict.""" + output_schema = { + "score": OutputField(type="number"), + "extra": OutputField(type="string", required=False, nullable=False), + } + dynamic_model = output_schema_to_pydantic_model("OptionalOmitOutput", output_schema) + assert dynamic_model is not None + + instance = dynamic_model.model_construct(score=1) + content = extract_content(instance, output_schema, "formatter") + assert "extra" not in content + assert content == {"score": 1} + + +class TestCombinedConstraints: + """Requirement: enum, pattern, and length constraints compose.""" + + def test_combined_constraints_reject_partial_matches(self) -> None: + """A value must satisfy every constraint; enum-pass/pattern-fail and + pattern-pass/enum-fail must both be rejected.""" + output_schema = { + "value": OutputField( + type="string", + enum=["abc"], + pattern=r"^a", + minLength=3, + maxLength=3, + ) + } + dynamic_model = output_schema_to_pydantic_model("CombinedOutput", output_schema) + assert dynamic_model is not None + + assert dynamic_model(value="abc").value == "abc" + with pytest.raises(PydanticValidationError): + dynamic_model(value="abd") # enum fail + with pytest.raises(PydanticValidationError): + dynamic_model(value="abcd") # pattern pass-ish, length fail + + +class TestToolSchemaKeywords: + """Requirement: the generated tool JSON schema exposes constraints and omits defaults.""" + + def _schema(self, output_schema: dict[str, OutputField]) -> dict[str, Any]: + """Build an agent and return the final_result tool JSON schema.""" + agent_def = AgentDef(name="formatter", output=output_schema) + pydantic_agent = build_agent(agent_def, system_prompt="", rendered_prompt="") + assert isinstance(pydantic_agent.output_type, ToolOutput) + toolset = pydantic_agent._output_schema.toolset + assert toolset is not None + assert len(toolset._tool_defs) == 1 + return toolset._tool_defs[0].parameters_json_schema + + def test_enum_pattern_length_range_in_schema(self) -> None: + """The tool schema must contain enum/pattern/minLength/maxLength/minimum/maximum.""" + output_schema = { + "tag": OutputField( + type="string", + enum=["a", "b"], + pattern=r"^[ab]$", + minLength=1, + maxLength=1, + ), + "count": OutputField(type="number", minimum=0, maximum=5), + } + schema = self._schema(output_schema) + props = schema["properties"] + assert props["tag"]["enum"] == ["a", "b"] + assert props["tag"]["pattern"] == r"^[ab]$" + assert props["tag"]["minLength"] == 1 + assert props["tag"]["maxLength"] == 1 + assert props["count"]["minimum"] == 0 + assert props["count"]["maximum"] == 5 + + def test_optional_field_has_no_default_and_not_required(self) -> None: + """Optional fields must not have a default key and must not be in the required array.""" + output_schema = { + "required_value": OutputField(type="string"), + "optional_value": OutputField(type="string", required=False), + } + schema = self._schema(output_schema) + props = schema["properties"] + assert "default" not in props["optional_value"] + assert "optional_value" not in schema.get("required", []) + assert "required_value" in schema["required"] + + +class TestOmittedOptionalMaterialization: + """Requirement: model_dump(exclude_unset=True) drops omitted optional keys.""" + + def test_unset_optional_field_is_dropped(self) -> None: + """An optional field the model never set must be absent from + ``model_dump(exclude_unset=True)`` so ``extract_content`` does not + materialize it as ``None``.""" + output_schema = { + "score": OutputField(type="number"), + "extra": OutputField(type="string", required=False, nullable=False), + } + dynamic_model = output_schema_to_pydantic_model("DumpOutput", output_schema) + assert dynamic_model is not None + + instance = dynamic_model.model_construct(score=1) + assert instance.model_dump(exclude_unset=True) == {"score": 1} From bf38c8a045d57f2927de916ecd64f2bb6ea587b4 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 20:12:12 +0300 Subject: [PATCH 05/10] docs: document output field constraints with example workflow --- docs/workflow-syntax.md | 71 +++++++++++++++++++++++++++- examples/output-constraints.yaml | 81 ++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 examples/output-constraints.yaml diff --git a/docs/workflow-syntax.md b/docs/workflow-syntax.md index f2431eef..1fbc9a4b 100644 --- a/docs/workflow-syntax.md +++ b/docs/workflow-syntax.md @@ -114,8 +114,16 @@ agents: output: # Optional: Output schema for validation field_name: - type: string + type: string # string | number | boolean | array | object description: "Field purpose" + enum: ["a", "b"] # Optional: Allowed scalar values (string/number/boolean) + pattern: "^[a-z]+$" # Optional: Regex pattern (string type only) + minimum: 0 # Optional: Inclusive minimum (number type only) + maximum: 100 # Optional: Inclusive maximum (number type only) + minLength: 1 # Optional: Minimum string length (string type only) + maxLength: 50 # Optional: Maximum string length (string type only) + nullable: true # Optional: Allow null value (default: false) + required: false # Optional: Only inside object properties (default: true) output_mode: raw # Optional: raw | envelope (default: inferred) # raw: skip JSON extraction, wrap response @@ -206,6 +214,67 @@ A response that parses as JSON but isn't an object at all (a bare `42` or an arr This is useful when you know an agent's output is simple and a single attempt should suffice, or when you want to fail fast instead of burning tokens on recovery loops. When the budget runs out, a schema-shape failure raises the specific validation error naming the offending field and its expected type, while a syntax failure raises a provider error. Each recovery attempt emits an `agent_parse_recovery` event, visible in the dashboard activity stream and the structured event log. + +### Field Constraints + +Output field definitions support optional validation constraints to enforce value boundaries and formatting rules. + +| Field | Applicable Type | Description | Semantics | +|-------|-----------------|-------------|-----------| +| `enum` | `string`, `number`, `boolean` | List of allowed scalar values | Uses exact value comparison. Cannot contain `null` (use `nullable: true` instead). | +| `pattern` | `string` | Regular expression pattern | Python `re.search` matching (unanchored by default; use `^` and `$` to anchor). Evaluated consistently on all providers. | +| `minimum` | `number` | Inclusive minimum numeric bound | Value must be greater than or equal to `minimum`. | +| `maximum` | `number` | Inclusive maximum numeric bound | Value must be less than or equal to `maximum`. | +| `minLength` | `string` | Inclusive minimum string length | String length must be greater than or equal to `minLength`. | +| `maxLength` | `string` | Inclusive maximum string length | String length must be less than or equal to `maxLength`. | +| `required` | Any (object property only) | Whether the object property must be present | Default: `true`. **Must be `true` for root-level fields**; setting `required: false` at the root level is rejected by `conductor validate`. | +| `nullable` | Any | Whether `null` is an acceptable value | Default: `false`. When `true`, renders as `type: [T, "null"]` in JSON Schema. | + +#### JSON Schema and Validation Semantics + +- **Inclusive Bounds**: `minimum`, `maximum`, `minLength`, and `maxLength` represent inclusive bounds. +- **Regex Pattern Matching**: `pattern` uses Python `re.search` semantics across all providers (including Claude). It matches anywhere in the target string unless explicitly anchored with `^` and `$`. +- **Nullable Fields**: Setting `nullable: true` renders the JSON Schema type as `type: [T, "null"]`, allowing the field to hold `null` or a value matching `type`. +- **Optional Object Properties**: The `required: false` constraint is permitted **only inside nested object properties** (e.g. `properties.details.required: false`). All root-level output fields must be required, so setting `required: false` on a root-level agent output field will be rejected during workflow validation (`conductor validate`). + +#### Field Constraints Example + +```yaml +agents: + - name: evaluator + prompt: "Evaluate the artifact and return structured metrics." + output: + status: + type: string + enum: ["passed", "failed", "pending"] + description: "Execution status" + score: + type: number + minimum: 0 + maximum: 100 + description: "Evaluation score between 0 and 100" + code: + type: string + pattern: "^ERR-[0-9]{3}$" + minLength: 7 + maxLength: 7 + description: "Error code in format ERR-123" + notes: + type: string + nullable: true + description: "Optional notes or null when absent" + metadata: + type: object + description: "Additional execution metadata" + properties: + reviewer: + type: string + description: "Reviewer identifier" + comments: + type: string + required: false + description: "Optional comments property inside object" +``` ### Choosing whether to declare `output:` Declaring `output:` does two things at once: it asks the model to return JSON matching the schema, and it parses the response as structured JSON. For some agents that's what you want. For others it produces parse-recovery loops and burns tokens. diff --git a/examples/output-constraints.yaml b/examples/output-constraints.yaml new file mode 100644 index 00000000..84050e4a --- /dev/null +++ b/examples/output-constraints.yaml @@ -0,0 +1,81 @@ +# Output Field Constraints Workflow +# +# Demonstrates structured output schema constraints: +# - `enum`: allowed scalar values +# - `pattern`: regular expression matching (Python re.search semantics) +# - `minimum` and `maximum`: numeric range boundaries +# - `minLength` and `maxLength`: string length boundaries +# - `required`: optional object properties (`required: false` inside object) +# - `nullable`: fields that permit null values (`nullable: true`) +# +# Usage: +# conductor run examples/output-constraints.yaml +# conductor validate examples/output-constraints.yaml + +workflow: + name: output-constraints + description: Workflow demonstrating all eight output field schema constraints + version: "1.0.0" + entry_point: audit_evaluator + + runtime: + provider: copilot + + input: + ticket_id: + type: string + required: false + default: "TICK-1234" + description: Ticket identifier to evaluate + +agents: + - name: audit_evaluator + description: Evaluates a system audit ticket and returns structured metrics with schema constraints + model: gpt-5.5 + prompt: | + Evaluate the audit ticket {{ workflow.input.ticket_id }}. + Return the verdict, score, reference ticket, summary, optional notes, and metadata object. + output: + verdict: + type: string + enum: ["passed", "failed", "inconclusive"] + description: Audit verdict (enum constraint) + ticket_ref: + type: string + pattern: "^TICK-[0-9]{4}$" + description: Formatted ticket reference matching pattern TICK-1234 + score: + type: number + minimum: 0.0 + maximum: 100.0 + description: Numeric evaluation score between 0 and 100 + summary: + type: string + minLength: 5 + maxLength: 200 + description: Brief summary string between 5 and 200 characters + notes: + type: string + nullable: true + description: Optional notes string or null when omitted + details: + type: object + description: Execution details object with an optional property + properties: + reviewer: + type: string + description: Identifier of the reviewer + comments: + type: string + required: false + description: Optional comments property inside object + + routes: + - to: $end + +output: + verdict: "{{ audit_evaluator.output.verdict }}" + ticket_ref: "{{ audit_evaluator.output.ticket_ref }}" + score: "{{ audit_evaluator.output.score }}" + summary: "{{ audit_evaluator.output.summary }}" + notes: "{{ audit_evaluator.output.notes }}" From f8922562fec927ce6a4d92be9edffcfb34923077 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 20:12:20 +0300 Subject: [PATCH 06/10] test(providers): add output-constraint provider parity and integration tests --- src/conductor/providers/aca.py | 5 +- .../test_output_constraints.py | 75 +++++ .../test_output_constraints_parity.py | 306 ++++++++++++++++++ 3 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 tests/test_integration/test_output_constraints.py create mode 100644 tests/test_providers/test_output_constraints_parity.py diff --git a/src/conductor/providers/aca.py b/src/conductor/providers/aca.py index ccd8ff41..5f875bfa 100644 --- a/src/conductor/providers/aca.py +++ b/src/conductor/providers/aca.py @@ -694,7 +694,10 @@ def _build_request( reasoning_effort = resolve_reasoning_effort(agent, self._default_reasoning_effort) working_dir = agent.sandbox.working_dir if agent.sandbox is not None else None output_schema = ( - {name: field.model_dump(exclude_none=True) for name, field in agent.output.items()} + { + name: field.model_dump(mode="json", exclude_none=True, exclude_defaults=True) + for name, field in agent.output.items() + } if agent.output else None ) diff --git a/tests/test_integration/test_output_constraints.py b/tests/test_integration/test_output_constraints.py new file mode 100644 index 00000000..740dc6f4 --- /dev/null +++ b/tests/test_integration/test_output_constraints.py @@ -0,0 +1,75 @@ +"""Integration test for output field constraints through the engine. + +Verifies that a workflow loaded from a YAML file with constrained output +fields surfaces a ValidationError when a mocked provider returns a payload +that violates those constraints. +""" + +from __future__ import annotations + +import textwrap +from pathlib import Path +from typing import Any + +import pytest + +from conductor.config.loader import load_config +from conductor.engine.workflow import WorkflowEngine +from conductor.exceptions import ValidationError +from conductor.providers.copilot import CopilotProvider + + +class TestOutputConstraintsIntegration: + """End-to-end constraint validation via workflow YAML and engine execution.""" + + @pytest.mark.asyncio + async def test_violating_payload_raises_validation_error(self, tmp_path: Path) -> None: + """A workflow with enum/range/length constraints on an agent output + must raise ValidationError when the provider returns a value outside + the allowed enum set.""" + workflow_file = tmp_path / "constrained.yaml" + workflow_file.write_text( + textwrap.dedent( + """\ + workflow: + name: constrained-workflow + entry_point: checker + + agents: + - name: checker + model: gpt-4 + prompt: Return a category and score. + output: + category: + type: string + enum: + - A + - B + - C + score: + type: number + minimum: 0 + maximum: 100 + routes: + - to: $end + + output: + result: "{{ checker.output.category }}" + """ + ) + ) + + config = load_config(workflow_file) + + def mock_handler(agent: Any, prompt: str, context: dict[str, Any]) -> dict[str, Any]: + # 'Z' is not in the allowed enum and must be rejected. + return {"category": "Z", "score": 50} + + provider = CopilotProvider(mock_handler=mock_handler) + engine = WorkflowEngine(config, provider) + + with pytest.raises(ValidationError) as exc_info: + await engine.run({}) + + assert "category" in str(exc_info.value) + assert "must be one of" in str(exc_info.value) diff --git a/tests/test_providers/test_output_constraints_parity.py b/tests/test_providers/test_output_constraints_parity.py new file mode 100644 index 00000000..4c351f94 --- /dev/null +++ b/tests/test_providers/test_output_constraints_parity.py @@ -0,0 +1,306 @@ +"""Provider parity tests for OutputField constraint extensions. + +These tests assert that a single shared output schema containing every +constraint keyword (enum, pattern, range, length, optional, nullable) is +translated consistently across all provider surfaces. They do not repeat the +per-constraint unit tests from Todos 2-4; they verify parity. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest +from pydantic import ValidationError as PydanticValidationError + +from conductor.config.schema import AgentDef, OutputField +from conductor.exceptions import ValidationError as ConductorValidationError +from conductor.executor.output import validate_output +from conductor.providers._pydantic_ai.converters import output_schema_to_pydantic_model +from conductor.providers.copilot import CopilotProvider + + +def _stub_handler(agent: AgentDef, prompt: str, context: dict[str, Any]) -> dict[str, Any]: + """Return a minimal dict for CopilotProvider mock-handler construction.""" + return {} + + +# Shared schema exercising every constraint keyword supported by OutputField. +# It intentionally mixes nullable, optional, and required fields so the tests +# can verify that each keyword survives its provider-specific transformation. +SHARED_OUTPUT_SCHEMA = { + "category": OutputField(type="string", enum=["A", "B", "C"]), + "code": OutputField( + type="string", + pattern=r"^[A-Z]{3}$", + minLength=3, + maxLength=3, + nullable=True, + ), + "score": OutputField(type="number", minimum=0, maximum=100), + "label": OutputField( + type="string", + minLength=1, + maxLength=10, + required=False, + ), +} + + +class TestCopilotPromptSchema: + """CopilotProvider._build_prompt_schema must carry every constraint keyword.""" + + def test_copilot_prompt_schema_contains_all_constraint_keywords(self) -> None: + """The prompt-facing schema produced for Copilot must include enum, + pattern, minLength, maxLength, minimum, maximum, nullable (as a type + list containing 'null'), and must omit optional fields from any implicit + required list.""" + provider = CopilotProvider(mock_handler=_stub_handler) + schema = provider._build_prompt_schema(SHARED_OUTPUT_SCHEMA) + + category = schema["category"] + assert category["type"] == "string" + assert category["enum"] == ["A", "B", "C"] + + code = schema["code"] + assert code["type"] == ["string", "null"] + assert code["pattern"] == r"^[A-Z]{3}$" + assert code["minLength"] == 3 + assert code["maxLength"] == 3 + + score = schema["score"] + assert score["type"] == "number" + assert score["minimum"] == 0 + assert score["maximum"] == 100 + + label = schema["label"] + assert label["type"] == "string" + assert label["minLength"] == 1 + assert label["maxLength"] == 10 + + +class TestHermesPromptSchema: + """HermesProvider._build_prompt_schema must carry every constraint keyword.""" + + @patch("conductor.providers.hermes.HERMES_SDK_AVAILABLE", True) + @patch("conductor.providers.hermes.AIAgent", MagicMock()) + def test_hermes_prompt_schema_contains_all_constraint_keywords(self) -> None: + """The prompt-facing schema produced for Hermes must include the same + constraint keywords as the Copilot surface, since both delegate to the + shared builder.""" + from conductor.providers.hermes import _build_prompt_schema + + schema = _build_prompt_schema(SHARED_OUTPUT_SCHEMA) + + category = schema["category"] + assert category["type"] == "string" + assert category["enum"] == ["A", "B", "C"] + + code = schema["code"] + assert code["type"] == ["string", "null"] + assert code["pattern"] == r"^[A-Z]{3}$" + assert code["minLength"] == 3 + assert code["maxLength"] == 3 + + score = schema["score"] + assert score["type"] == "number" + assert score["minimum"] == 0 + assert score["maximum"] == 100 + + label = schema["label"] + assert label["type"] == "string" + assert label["minLength"] == 1 + assert label["maxLength"] == 10 + + +class TestClaudeAgentSdkOutputFormat: + """claude_agent_sdk._build_output_format must carry every constraint keyword.""" + + def test_claude_agent_sdk_output_format_contains_all_constraint_keywords(self) -> None: + """The SDK output_format payload must contain enum, pattern, length, + range, and nullable keywords in the inner JSON schema, and must mark + required fields only (not optional ones).""" + pytest.importorskip( + "claude_agent_sdk", + reason="claude-agent-sdk extra not installed", + ) + from conductor.providers.claude_agent_sdk import _build_output_format + + payload = _build_output_format(SHARED_OUTPUT_SCHEMA) + + assert payload["type"] == "json_schema" + schema = payload["schema"] + props = schema["properties"] + + assert schema["required"] == ["category", "code", "score"] + + category = props["category"] + assert category["type"] == "string" + assert category["enum"] == ["A", "B", "C"] + + code = props["code"] + assert code["type"] == ["string", "null"] + assert code["pattern"] == r"^[A-Z]{3}$" + assert code["minLength"] == 3 + assert code["maxLength"] == 3 + + score = props["score"] + assert score["type"] == "number" + assert score["minimum"] == 0 + assert score["maximum"] == 100 + + label = props["label"] + assert label["type"] == "string" + assert label["minLength"] == 1 + assert label["maxLength"] == 10 + + +class TestClaudePydanticModel: + """output_schema_to_pydantic_model must enforce the constraints.""" + + def test_claude_dynamic_model_accepts_conforming_payload(self) -> None: + """A payload satisfying every constraint must validate cleanly.""" + model = output_schema_to_pydantic_model("Constrained", SHARED_OUTPUT_SCHEMA) + assert model is not None + + instance = model.model_validate({"category": "A", "code": "XYZ", "score": 42}) + assert instance.model_dump()["category"] == "A" + assert instance.model_dump()["code"] == "XYZ" + assert instance.model_dump()["score"] == 42 + + def test_claude_dynamic_model_rejects_violating_payload(self) -> None: + """A payload violating a constraint must raise Pydantic ValidationError.""" + model = output_schema_to_pydantic_model("Constrained", SHARED_OUTPUT_SCHEMA) + assert model is not None + + with pytest.raises(PydanticValidationError): + model(category="Z", code="XYZ", score=42) + + +class TestValidateOutputParity: + """validate_output must raise identical messages regardless of provider path.""" + + def test_validate_output_raises_identical_message_for_enum_violation(self) -> None: + """The provider-agnostic validation path must surface the same + constraint error message for a payload that violates an enum.""" + with pytest.raises(ConductorValidationError) as exc_info: + validate_output( + {"category": "Z", "code": "ABC", "score": 50}, + SHARED_OUTPUT_SCHEMA, + ) + + assert "must be one of" in str(exc_info.value) + assert "category" in str(exc_info.value) + + +class TestAcaWireBoundary: + """ACA host->runner serialization must preserve every constraint field.""" + + def _make_provider(self) -> Any: + from conductor.config.schema import ProviderSettings + from conductor.providers.aca import AcaRuntimeProvider + + settings = ProviderSettings( + name="aca", + pool_endpoint="https://pool.example.com", + api_version="2025-07-01", + ) + with patch("conductor.providers.aca.AZURE_IDENTITY_AVAILABLE", True): + return AcaRuntimeProvider(provider_settings=settings) + + def test_aca_request_carries_constraint_fields(self) -> None: + """AcaRuntimeProvider._build_request must serialize enum, pattern, + range, length, required, and nullable into request.agent.output.""" + provider = self._make_provider() + agent = AgentDef( + name="constrained", + prompt="test", + output=SHARED_OUTPUT_SCHEMA, + ) + + request = provider._build_request(agent, {}, "rendered", None) + wire_output = request.agent.output + assert wire_output is not None + + category = wire_output["category"] + assert category["type"] == "string" + assert category["enum"] == ["A", "B", "C"] + # required=True and nullable=False are defaults and should be omitted + # to keep the wire payload small. + assert "required" not in category + assert "nullable" not in category + + code = wire_output["code"] + assert code["type"] == "string" + assert code["pattern"] == r"^[A-Z]{3}$" + assert code["minLength"] == 3 + assert code["maxLength"] == 3 + assert code["nullable"] is True + assert "required" not in code + + score = wire_output["score"] + assert score["type"] == "number" + assert score["minimum"] == 0 + assert score["maximum"] == 100 + + label = wire_output["label"] + assert label["type"] == "string" + assert label["minLength"] == 1 + assert label["maxLength"] == 10 + assert label["required"] is False + assert "nullable" not in label + + def test_aca_runner_reconstructs_identical_output_schema(self) -> None: + """The runner's OutputField.model_validate must reconstruct the same + effective field values from the wire payload.""" + provider = self._make_provider() + agent = AgentDef( + name="constrained", + prompt="test", + output=SHARED_OUTPUT_SCHEMA, + ) + + request = provider._build_request(agent, {}, "rendered", None) + wire_output = request.agent.output + assert wire_output is not None + + # Simulate the runner reconstruction path from aca_runner/server.py. + reconstructed = { + name: OutputField.model_validate(field) for name, field in wire_output.items() + } + + for name in SHARED_OUTPUT_SCHEMA: + original = SHARED_OUTPUT_SCHEMA[name] + rebuilt = reconstructed[name] + assert original.type == rebuilt.type + assert original.enum == rebuilt.enum + assert original.pattern == rebuilt.pattern + assert original.minimum == rebuilt.minimum + assert original.maximum == rebuilt.maximum + assert original.minLength == rebuilt.minLength + assert original.maxLength == rebuilt.maxLength + assert original.required == rebuilt.required + assert original.nullable == rebuilt.nullable + + def test_aca_wire_body_preserves_constraint_fields(self) -> None: + """The actual JSON body sent to the runner must contain the constraint + fields after SecretStr unwrapping.""" + provider = self._make_provider() + agent = AgentDef( + name="constrained", + prompt="test", + output=SHARED_OUTPUT_SCHEMA, + ) + + request = provider._build_request(agent, {}, "rendered", None) + body = provider._wire_body(request) + + wire_output = body["agent"]["output"] + assert wire_output is not None + assert wire_output["category"]["enum"] == ["A", "B", "C"] + assert wire_output["code"]["pattern"] == r"^[A-Z]{3}$" + assert wire_output["code"]["nullable"] is True + assert wire_output["score"]["minimum"] == 0 + assert wire_output["score"]["maximum"] == 100 + assert wire_output["label"]["required"] is False From cc3b39e82f833e0c4e905c2f77adf1493d339a53 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 20:19:08 +0300 Subject: [PATCH 07/10] style(config,executor,tests): apply ruff formatting to pass lint --- src/conductor/config/schema.py | 10 ++-------- src/conductor/executor/output.py | 6 ++---- tests/test_config/test_schema.py | 12 +++--------- tests/test_providers/test_output_schema.py | 4 +--- 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 842c77f0..27a4b8e6 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -168,9 +168,7 @@ def validate_type_specific_fields(self) -> OutputField: } check = type_checks.get(self.type) if check is not None and not all(check(value) for value in self.enum): - raise ValueError( - f"enum values must match the declared type '{self.type}'" - ) + raise ValueError(f"enum values must match the declared type '{self.type}'") # String length validation. if self.minLength is not None and self.minLength < 0: @@ -185,11 +183,7 @@ def validate_type_specific_fields(self) -> OutputField: raise ValueError("minLength cannot be greater than maxLength") # Number range validation. - if ( - self.minimum is not None - and self.maximum is not None - and self.minimum > self.maximum - ): + if self.minimum is not None and self.maximum is not None and self.minimum > self.maximum: raise ValueError("minimum cannot be greater than maximum") # Pattern compilation. diff --git a/src/conductor/executor/output.py b/src/conductor/executor/output.py index e5a6f628..f052ec1a 100644 --- a/src/conductor/executor/output.py +++ b/src/conductor/executor/output.py @@ -57,14 +57,12 @@ def _check_constraints(field_name: str, value: Any, field_def: OutputField) -> N if isinstance(value, bool): if not (field_def.type == "boolean" and value in field_def.enum): raise ValidationError( - f"Output field '{field_name}' must be one of {field_def.enum!r}, " - f"got {value!r}", + f"Output field '{field_name}' must be one of {field_def.enum!r}, got {value!r}", suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", ) elif value not in field_def.enum: raise ValidationError( - f"Output field '{field_name}' must be one of {field_def.enum!r}, " - f"got {value!r}", + f"Output field '{field_name}' must be one of {field_def.enum!r}, got {value!r}", suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", ) diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index 38e3800b..3bd8db51 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -207,18 +207,14 @@ def test_output_field_negative_length_rejected(self) -> None: def test_output_field_min_length_exceeds_max_length_rejected(self) -> None: """Test that minLength greater than maxLength is rejected.""" with pytest.raises(ValidationError) as exc_info: - OutputField.model_validate( - {"type": "string", "minLength": 5, "maxLength": 1} - ) + OutputField.model_validate({"type": "string", "minLength": 5, "maxLength": 1}) assert "minLength" in str(exc_info.value) assert "maxLength" in str(exc_info.value) def test_output_field_minimum_exceeds_maximum_rejected(self) -> None: """Test that minimum greater than maximum is rejected.""" with pytest.raises(ValidationError) as exc_info: - OutputField.model_validate( - {"type": "number", "minimum": 10, "maximum": 0} - ) + OutputField.model_validate({"type": "number", "minimum": 10, "maximum": 0}) assert "minimum" in str(exc_info.value) assert "maximum" in str(exc_info.value) @@ -248,9 +244,7 @@ def test_output_field_enum_type_mismatch_rejected(self) -> None: def test_output_field_enum_null_rejected(self) -> None: """Test that None values inside enum are rejected with a precise message.""" with pytest.raises(ValidationError) as exc_info: - OutputField.model_validate( - {"type": "string", "enum": ["a", None], "nullable": True} - ) + OutputField.model_validate({"type": "string", "enum": ["a", None], "nullable": True}) assert "enum cannot contain null; use nullable: true to allow null values" in str( exc_info.value ) diff --git a/tests/test_providers/test_output_schema.py b/tests/test_providers/test_output_schema.py index f8b95c77..5cec7f5f 100644 --- a/tests/test_providers/test_output_schema.py +++ b/tests/test_providers/test_output_schema.py @@ -891,9 +891,7 @@ def test_json_schema_never_emits_default_keys(self) -> None: def _no_default_keys(value: Any) -> bool: if isinstance(value, dict): - return "default" not in value and all( - _no_default_keys(v) for v in value.values() - ) + return "default" not in value and all(_no_default_keys(v) for v in value.values()) if isinstance(value, list): return all(_no_default_keys(item) for item in value) return True From de4adee576aea103f534850d777b09ce6c4327da Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 22:13:45 +0300 Subject: [PATCH 08/10] fix(providers,executor): enforce output constraints on array items and sanitize pydantic-ai tool schema --- src/conductor/executor/output.py | 2 + .../providers/_pydantic_ai/agent_builder.py | 12 +- .../providers/_pydantic_ai/converters.py | 145 +++++++++++++-- tests/test_executor/test_output.py | 106 +++++++++++ .../test_pydantic_ai_agent_builder.py | 65 +++++++ .../test_pydantic_ai_converters.py | 9 +- .../test_pydantic_ai_structured_output.py | 172 ++++++++++++++++++ 7 files changed, 489 insertions(+), 22 deletions(-) diff --git a/src/conductor/executor/output.py b/src/conductor/executor/output.py index f052ec1a..4ea3944e 100644 --- a/src/conductor/executor/output.py +++ b/src/conductor/executor/output.py @@ -135,6 +135,8 @@ def _validate_field(field_name: str, value: Any, field_def: OutputField) -> None if field_def.type == "array" and field_def.items and isinstance(value, list): for i, item in enumerate(value): + if item is None and field_def.items.nullable: + continue if not check_type(item, field_def.items.type): raise ValidationError( f"Array item {i} in '{field_name}' has wrong type: " diff --git a/src/conductor/providers/_pydantic_ai/agent_builder.py b/src/conductor/providers/_pydantic_ai/agent_builder.py index 6d0d133d..acd517af 100644 --- a/src/conductor/providers/_pydantic_ai/agent_builder.py +++ b/src/conductor/providers/_pydantic_ai/agent_builder.py @@ -22,7 +22,10 @@ from pydantic_ai.providers.anthropic import AnthropicProvider from conductor.exceptions import ValidationError -from conductor.providers._pydantic_ai.converters import output_schema_to_pydantic_model +from conductor.providers._pydantic_ai.converters import ( + _sanitize_json_schema, + output_schema_to_pydantic_model, +) from conductor.providers.reasoning import ( ReasoningEffort, effort_to_budget_tokens, @@ -375,4 +378,11 @@ def build_agent( toolsets=toolsets or [], tools=tools or [], ) + + if isinstance(pydantic_agent.output_type, ToolOutput): + toolset = pydantic_agent._output_schema.toolset + if toolset is not None: + for tool_def in toolset._tool_defs: + _sanitize_json_schema(tool_def.parameters_json_schema) + return pydantic_agent diff --git a/src/conductor/providers/_pydantic_ai/converters.py b/src/conductor/providers/_pydantic_ai/converters.py index 9c1e9a33..57616f96 100644 --- a/src/conductor/providers/_pydantic_ai/converters.py +++ b/src/conductor/providers/_pydantic_ai/converters.py @@ -7,6 +7,7 @@ from __future__ import annotations +import copy import re from typing import Annotated, Any @@ -36,32 +37,108 @@ def _reject_bool(value: Any) -> Any: class _NoDefaultBaseModel(BaseModel): - """Dynamic model base that strips ``default`` keys from JSON schemas. + """Dynamic model base that sanitizes generated JSON schemas. - Pydantic v2 emits ``"default": null`` for ``Field(default=None)``. - Pydantic AI's tool schema must not include the JSON Schema ``default`` - keyword (it is not allowed on tool parameters), so this base removes it - recursively from the generated schema, including nested ``$defs``. + Pydantic v2 emits ``"default": null`` for ``Field(default=None)`` and uses + ``anyOf`` for nullable unions and ``$ref``/``$defs`` for nested models. + Pydantic AI's tool schema must not include the JSON Schema ``default``, + ``anyOf``, ``$ref``, or ``$defs`` keywords, so this base removes them and + flattens nullable unions into ``{type: [T, "null"]}``. """ model_config = ConfigDict(extra="allow") @classmethod - def __get_pydantic_json_schema__(cls, core_schema: Any, handler: Any) -> dict[str, Any]: - schema = handler(core_schema) - _strip_default_keys(schema) + def model_json_schema(cls, *args: Any, **kwargs: Any) -> dict[str, Any]: + schema = super().model_json_schema(*args, **kwargs) + _sanitize_json_schema(schema) return schema -def _strip_default_keys(schema: Any) -> None: - """Recursively remove ``default`` keys from a JSON schema dict in place.""" - if isinstance(schema, dict): - schema.pop("default", None) - for value in schema.values(): - _strip_default_keys(value) - elif isinstance(schema, list): - for item in schema: - _strip_default_keys(item) +def _convert_anyof_nullable(node: dict[str, Any]) -> dict[str, Any]: + """Convert a nullable ``anyOf`` union to a flat schema with ``type: [T, "null"]``. + + Pydantic emits ``{"anyOf": [{"type": "T", ...}, {"type": "null"}]}`` for + ``T | None``. The output tool schema must not contain ``anyOf``, so this + helper merges the non-null branch's constraints into a single schema while + preserving siblings such as ``title`` or ``description``. + """ + any_of = node.get("anyOf", []) + if len(any_of) != 2: + return node + + null_branch: dict[str, Any] | None = None + value_branch: dict[str, Any] | None = None + for branch in any_of: + if isinstance(branch, dict) and branch.get("type") == "null": + null_branch = branch + elif isinstance(branch, dict): + value_branch = branch + + if null_branch is None or value_branch is None: + return node + + merged = dict(value_branch) + value_type = merged.pop("type", None) + if value_type is not None: + merged["type"] = [value_type, "null"] + + for key, val in node.items(): + if key != "anyOf": + merged[key] = val + + return merged + + +def _sanitize_json_schema(schema: dict[str, Any]) -> None: + """Recursively sanitize a JSON schema in place. + + Removes ``default``, ``$ref``, and ``$defs`` keywords and flattens nullable + ``anyOf`` unions so the generated Pydantic-AI tool schema contains only the + agreed structural keywords. + """ + defs = schema.pop("$defs", {}) + _sanitize_schema_node(schema, defs) + schema.pop("$defs", None) + + +def _sanitize_schema_node(node: Any, defs: dict[str, Any]) -> None: + """Recursively sanitize a JSON schema node in place. + + ``$ref`` targets that cannot be resolved are left untouched so a caller with + a complete view of ``$defs`` (e.g. after Pydantic-AI builds the tool schema) + can perform a final inlining pass. + """ + if isinstance(node, dict): + nested_defs = node.pop("$defs", None) + if nested_defs: + defs = {**defs, **nested_defs} + + if "$ref" in node: + ref_name = node["$ref"].split("/")[-1] + if ref_name in defs: + inlined = copy.deepcopy(defs[ref_name]) + node.clear() + node.update(inlined) + _sanitize_schema_node(node, defs) + return + + if "anyOf" in node: + for branch in node["anyOf"]: + if isinstance(branch, dict) and "$ref" in branch: + _sanitize_schema_node(branch, defs) + converted = _convert_anyof_nullable(node) + if converted is not node: + node.clear() + node.update(converted) + + node.pop("default", None) + + for value in node.values(): + _sanitize_schema_node(value, defs) + elif isinstance(node, list): + for item in node: + _sanitize_schema_node(item, defs) def _make_enum_validator(enum_values: list[Any], field_type: str) -> Any: @@ -202,6 +279,38 @@ def _build_field_info(field: OutputField) -> Any: return Field(**field_kwargs) +def _build_array_item_type( + field: OutputField, + *, + prefix: str = "Output", + depth: int = 0, + max_depth: int = 10, +) -> Any: + """Build a Pydantic type for an array item, applying scalar constraints. + + Array elements are validated individually, so the item's ``nullable`` flag + and scalar constraints (enum, pattern, length, range) are applied exactly + like top-level scalar fields. Object and array items delegate constraint + enforcement to the recursive type builder. + """ + if depth > max_depth: + raise ValueError(f"Maximum output schema nesting depth of {max_depth} exceeded") + + base_type = _map_output_field_type( + field, + prefix=prefix, + depth=depth, + max_depth=max_depth, + ) + + if field.type in ("string", "number", "integer", "boolean"): + base_type = _wrap_scalar_field_type(field, base_type) + field_info = _build_field_info(field) + return Annotated[base_type, field_info] + + return base_type + + def _map_output_field_type( field: OutputField, *, @@ -245,7 +354,7 @@ def _map_output_field_type( return bool if field.type == "array": if field.items: - item_type = _map_output_field_type( + item_type = _build_array_item_type( field.items, prefix=f"{prefix}Item", depth=depth + 1, diff --git a/tests/test_executor/test_output.py b/tests/test_executor/test_output.py index 4192fd27..119c016a 100644 --- a/tests/test_executor/test_output.py +++ b/tests/test_executor/test_output.py @@ -623,6 +623,112 @@ def test_constraints_inside_array_object_items_rejected(self) -> None: ) +class TestValidateOutputNullableArrayItems: + """Regression tests for nullable array items (F1 round 2).""" + + def test_nullable_array_item_null_passes(self) -> None: + """A nullable string array item must accept None.""" + schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", nullable=True), + ) + } + validate_output({"values": [None]}, schema) + + def test_nullable_array_item_mixed_passes(self) -> None: + """A nullable array must accept both null and non-null values.""" + schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", nullable=True), + ) + } + validate_output({"values": [None, "a", None, "b"]}, schema) + + def test_nullable_array_item_false_rejects_null(self) -> None: + """A non-nullable array item must still reject None with the index-aware message.""" + schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", nullable=False), + ) + } + with pytest.raises( + ValidationError, + match="Array item 0 in 'values' has wrong type: expected string, got NoneType", + ): + validate_output({"values": [None]}, schema) + + def test_nullable_array_item_object_passes(self) -> None: + """A nullable object array item must accept None.""" + schema = { + "items": OutputField( + type="array", + items=OutputField( + type="object", + nullable=True, + properties={"name": OutputField(type="string")}, + ), + ) + } + validate_output({"items": [None, {"name": "x"}, None]}, schema) + + def test_nullable_array_item_nested_object(self) -> None: + """Nullable array items inside a nested object must accept None.""" + schema = { + "data": OutputField( + type="object", + properties={ + "tags": OutputField( + type="array", + items=OutputField(type="string", nullable=True), + ) + }, + ) + } + validate_output({"data": {"tags": [None, "ok", None]}}, schema) + + def test_nullable_array_item_constraints_still_enforced(self) -> None: + """Non-null nullable array items must still validate constraints.""" + schema = { + "codes": OutputField( + type="array", + items=OutputField(type="string", nullable=True, pattern=r"^[ab]$"), + ) + } + with pytest.raises(ValidationError, match="does not match pattern"): + validate_output({"codes": [None, "c", None]}, schema) + + def test_nullable_array_item_required_property_inside_object(self) -> None: + """A non-null object array item must still enforce required properties.""" + schema = { + "items": OutputField( + type="array", + items=OutputField( + type="object", + nullable=True, + properties={"name": OutputField(type="string")}, + ), + ) + } + with pytest.raises(ValidationError, match="Missing required output field: name"): + validate_output({"items": [{"name": "ok"}, {}]}, schema) + + def test_nullable_array_item_deeply_nested_array(self) -> None: + """Nullable array items at depth must accept None.""" + schema = { + "matrix": OutputField( + type="array", + items=OutputField( + type="array", + items=OutputField(type="number", nullable=True), + ), + ) + } + validate_output({"matrix": [[1, None, 3], [None, 2]]}, schema) + + class TestValidationErrorValueDescription: """The error names the offending value, without echoing secrets. diff --git a/tests/test_providers/test_pydantic_ai_agent_builder.py b/tests/test_providers/test_pydantic_ai_agent_builder.py index 3664c05b..2318f03f 100644 --- a/tests/test_providers/test_pydantic_ai_agent_builder.py +++ b/tests/test_providers/test_pydantic_ai_agent_builder.py @@ -272,6 +272,71 @@ def test_custom_output_retries_preserve_zero_tool_retries(self) -> None: assert pydantic_agent._max_output_retries == 4 +class TestArrayItemConstraintOutputRetry: + """Requirement: array-item constraint violations trigger pydantic-ai output retries.""" + + @pytest.mark.asyncio + async def test_array_item_constraint_violation_triggers_output_retry( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """When the model first returns a tool call with an array item that + violates an enum/pattern constraint, pydantic-ai's output retry must + recover and return valid structured output after exactly one retry.""" + calls: list[int] = [] + + async def _fake_model( + messages: list[Any], + info: Any, + ) -> ModelResponse: + calls.append(len(calls)) + output_tool_name = info.output_tools[0].name + if len(calls) == 1: + return ModelResponse( + parts=[ + ToolCallPart( + tool_name=output_tool_name, + args={"values": ["bad"]}, + ) + ] + ) + return ModelResponse( + parts=[ + ToolCallPart( + tool_name=output_tool_name, + args={"values": ["ok"]}, + ) + ] + ) + + monkeypatch.setattr( + "conductor.providers._pydantic_ai.agent_builder._resolve_anthropic_model", + lambda *_args, **_kwargs: FunctionModel(_fake_model), + ) + + agent_def = AgentDef( + name="formatter", + output={ + "values": OutputField( + type="array", + items=OutputField( + type="string", + enum=["ok"], + pattern="^o.*$", + minLength=2, + maxLength=2, + ), + ) + }, + ) + pydantic_agent = build_agent(agent_def, system_prompt="sys", rendered_prompt="go") + + result = await pydantic_agent.run("go") + + assert len(calls) == 2 + assert result.output.values == ["ok"] + + class TestOutputRecovery: """Regression tests for structured-output recovery from plain-text responses.""" diff --git a/tests/test_providers/test_pydantic_ai_converters.py b/tests/test_providers/test_pydantic_ai_converters.py index 5d8a6cbe..ab9fc846 100644 --- a/tests/test_providers/test_pydantic_ai_converters.py +++ b/tests/test_providers/test_pydantic_ai_converters.py @@ -17,6 +17,7 @@ from conductor.exceptions import ValidationError as ConductorValidationError from conductor.providers._pydantic_ai.converters import ( _map_output_field_type, + _sanitize_json_schema, output_schema_to_pydantic_model, ) @@ -415,11 +416,13 @@ def test_nested_descriptions_in_json_schema(self) -> None: schema = model.model_json_schema() findings = schema["properties"]["findings"] assert findings["description"] == "Top-level findings list." - item_ref = findings["items"]["$ref"] - item_key = item_ref.split("/")[-1] - item_schema = schema["$defs"][item_key] + _sanitize_json_schema(schema) + item_schema = findings["items"] + assert "description" in item_schema assert item_schema["description"] == "A single finding." assert item_schema["properties"]["title"]["description"] == "Finding title." + assert "$ref" not in schema + assert "$defs" not in schema class TestValidationParity: diff --git a/tests/test_providers/test_pydantic_ai_structured_output.py b/tests/test_providers/test_pydantic_ai_structured_output.py index b71731ac..bc2a34df 100644 --- a/tests/test_providers/test_pydantic_ai_structured_output.py +++ b/tests/test_providers/test_pydantic_ai_structured_output.py @@ -386,3 +386,175 @@ def test_unset_optional_field_is_dropped(self) -> None: instance = dynamic_model.model_construct(score=1) assert instance.model_dump(exclude_unset=True) == {"score": 1} + + +class TestArrayItemConstraints: + """Requirement: array-item scalar constraints are enforced like top-level fields.""" + + def _model(self, output_schema: dict[str, OutputField]) -> type[BaseModel]: + dynamic_model = output_schema_to_pydantic_model("ArrayItemOutput", output_schema) + assert dynamic_model is not None + return dynamic_model + + def test_array_item_enum_rejects_non_member(self) -> None: + """An array of strings with an enum must reject a non-member item.""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", enum=["ok"]), + ) + } + dynamic_model = self._model(output_schema) + + assert dynamic_model(values=["ok"]).values == ["ok"] + with pytest.raises(PydanticValidationError): + dynamic_model(values=["bad"]) + + def test_array_item_pattern_and_length_reject_violations(self) -> None: + """Array-item pattern and length constraints must reject violating items.""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", pattern="^o+$", minLength=2, maxLength=2), + ) + } + dynamic_model = self._model(output_schema) + + assert dynamic_model(values=["oo"]).values == ["oo"] + with pytest.raises(PydanticValidationError): + dynamic_model(values=["bad"]) # pattern fail + with pytest.raises(PydanticValidationError): + dynamic_model(values=["o"]) # length fail + with pytest.raises(PydanticValidationError): + dynamic_model(values=["ooo"]) # length fail + + def test_array_item_number_range_reject_violations(self) -> None: + """Array-item number range constraints must reject out-of-bounds values.""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField(type="number", minimum=0, maximum=10), + ) + } + dynamic_model = self._model(output_schema) + + assert dynamic_model(values=[0, 10]).values == [0, 10] + with pytest.raises(PydanticValidationError): + dynamic_model(values=[-1]) + with pytest.raises(PydanticValidationError): + dynamic_model(values=[11]) + + def test_array_item_nullable_accepts_none(self) -> None: + """An array of nullable strings must accept ``None`` items.""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", nullable=True), + ) + } + dynamic_model = self._model(output_schema) + + instance = dynamic_model(values=["ok", None]) + assert instance.values == ["ok", None] + + def test_array_item_non_nullable_rejects_none(self) -> None: + """An array of non-nullable strings must reject ``None`` items.""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", nullable=False), + ) + } + dynamic_model = self._model(output_schema) + + with pytest.raises(PydanticValidationError): + dynamic_model(values=[None]) + + +class TestToolSchemaForbiddenKeywords: + """Requirement: generated tool JSON schemas contain no forbidden keywords.""" + + def _schema(self, output_schema: dict[str, OutputField]) -> dict[str, Any]: + """Build an agent and return the final_result tool JSON schema.""" + agent_def = AgentDef(name="formatter", output=output_schema) + pydantic_agent = build_agent(agent_def, system_prompt="", rendered_prompt="") + assert isinstance(pydantic_agent.output_type, ToolOutput) + toolset = pydantic_agent._output_schema.toolset + assert toolset is not None + assert len(toolset._tool_defs) == 1 + return toolset._tool_defs[0].parameters_json_schema + + def _collect_keywords(self, node: Any, found: set[str]) -> None: + if isinstance(node, dict): + found.update(node.keys()) + for value in node.values(): + self._collect_keywords(value, found) + elif isinstance(node, list): + for item in node: + self._collect_keywords(item, found) + + def test_schema_has_no_anyof_ref_defs_for_nullable_object(self) -> None: + """A nullable nested object must not produce anyOf, $ref, or $defs and must + preserve the nested object structure and nullability.""" + output_schema = { + "wrapper": OutputField( + type="object", + properties={ + "item": OutputField( + type="object", + nullable=True, + properties={ + "tag": OutputField(type="string", enum=["a", "b"]), + }, + ) + }, + ) + } + schema = self._schema(output_schema) + keywords: set[str] = set() + self._collect_keywords(schema, keywords) + + assert "anyOf" not in keywords + assert "$ref" not in keywords + assert "$defs" not in keywords + + item_schema = schema["properties"]["wrapper"]["properties"]["item"] + assert set(item_schema["type"]) == {"object", "null"} + assert item_schema["properties"]["tag"]["enum"] == ["a", "b"] + + def test_array_item_constraints_appear_in_schema(self) -> None: + """Array-item constraints must be advertised in the generated tool schema.""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField( + type="string", + enum=["ok"], + pattern="^o$", + minLength=2, + maxLength=2, + ), + ) + } + schema = self._schema(output_schema) + item_schema = schema["properties"]["values"]["items"] + + assert item_schema["type"] == "string" + assert item_schema["enum"] == ["ok"] + assert item_schema["pattern"] == "^o$" + assert item_schema["minLength"] == 2 + assert item_schema["maxLength"] == 2 + + def test_array_item_nullable_schema_uses_null_type(self) -> None: + """Nullable array items must be represented as type ["string", "null"].""" + output_schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", nullable=True), + ) + } + schema = self._schema(output_schema) + item_schema = schema["properties"]["values"]["items"] + + assert "anyOf" not in item_schema + assert set(item_schema["type"]) == {"string", "null"} From d407dc02375a7730a6c825e3843b44a17ef37864 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Tue, 4 Aug 2026 22:13:47 +0300 Subject: [PATCH 09/10] style(tests): remove internal planning label from parity test docstring --- tests/test_providers/test_output_constraints_parity.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_providers/test_output_constraints_parity.py b/tests/test_providers/test_output_constraints_parity.py index 4c351f94..628be037 100644 --- a/tests/test_providers/test_output_constraints_parity.py +++ b/tests/test_providers/test_output_constraints_parity.py @@ -3,7 +3,8 @@ These tests assert that a single shared output schema containing every constraint keyword (enum, pattern, range, length, optional, nullable) is translated consistently across all provider surfaces. They do not repeat the -per-constraint unit tests from Todos 2-4; they verify parity. +individual constraint checks of the validator and schema builders; they verify +parity. """ from __future__ import annotations From c169c6e6f8af327e457efe77930aa472dc6d1885 Mon Sep 17 00:00:00 2001 From: Genadij Blinov Date: Thu, 6 Aug 2026 23:19:20 +0300 Subject: [PATCH 10/10] fix(providers,executor,config): address review feedback on output field constraints Claude-path parity blockers: - Keep the null branch when a nullable number carries minimum/maximum: pydantic nests the int|float union, and _convert_anyof_nullable dropped the null branch for such shapes. Nested unions now flatten to type: [integer, number, "null"], and unrecognized shapes are returned unchanged so null is never silently lost. - Accept nullable object and array items in the dynamic model, matching validate_output; nullable was previously applied only to scalar items, so the same YAML passed on four providers and failed on claude. Pattern matching safety: - Evaluate patterns with the regex engine (re-compatible) under a 1s wall-clock deadline (PATTERN_MATCH_TIMEOUT_SECONDS): a pathological pattern on model output now raises instead of stalling the event loop, and the pydantic path surfaces it as a ValueError driving the in-session output retry. Length checks now run before pattern matching. - Cache the compiled pattern on OutputField.compiled_pattern. Schema honesty: - Emit enum with null appended when a field is both enum-constrained and nullable, in both shared builders and the pydantic json_schema_extra; type and enum are conjunctive in JSON Schema, so the previous shape forbade the null that every enforcement layer accepted. - Strip pydantic-internal ge/le keys from the generated tool schema (the standard minimum/maximum remain). - Reject unknown OutputField keys (extra="forbid") so a constraint typo fails at load instead of silently unconstraining the field. - Keep integral minimum/maximum as int (no 0.0 in schemas and messages). Cleanups: - Remove the dead model_json_schema override (pydantic-ai never calls it; agent_builder performs the real tool-schema sanitization) and rename the base model to _OutputBaseModel. - Collapse unreachable bool/None branches in the enum/pattern validators. - Name the element index in array-item constraint errors and include the offending value in pattern/length/range messages. - Warn on undeclared output keys so a misspelled optional key is visible instead of silently dropped. Tests: the parity fixture now co-locates the previously-breaking combinations (nullable+range, enum+nullable, nullable object items, nested optional properties), a payload matrix asserts the Claude dynamic model and validate_output agree on accept/reject, and tool-schema hygiene tests pin the absence of ge/le/default keys through the real build_agent seam. --- docs/workflow-syntax.md | 4 +- pyproject.toml | 3 + src/conductor/config/schema.py | 35 +- src/conductor/executor/output.py | 71 ++-- .../providers/_pydantic_ai/converters.py | 131 ++++--- src/conductor/providers/_schema.py | 8 +- tests/test_config/test_schema.py | 29 ++ tests/test_executor/test_output.py | 78 +++- .../test_output_constraints_parity.py | 351 +++++++++++++++++- tests/test_providers/test_output_schema.py | 38 ++ .../test_pydantic_ai_agent_builder.py | 93 +++++ .../test_pydantic_ai_converters.py | 104 ++++++ uv.lock | 6 +- 13 files changed, 840 insertions(+), 111 deletions(-) diff --git a/docs/workflow-syntax.md b/docs/workflow-syntax.md index 1fbc9a4b..6f0ff683 100644 --- a/docs/workflow-syntax.md +++ b/docs/workflow-syntax.md @@ -222,7 +222,7 @@ Output field definitions support optional validation constraints to enforce valu | Field | Applicable Type | Description | Semantics | |-------|-----------------|-------------|-----------| | `enum` | `string`, `number`, `boolean` | List of allowed scalar values | Uses exact value comparison. Cannot contain `null` (use `nullable: true` instead). | -| `pattern` | `string` | Regular expression pattern | Python `re.search` matching (unanchored by default; use `^` and `$` to anchor). Evaluated consistently on all providers. | +| `pattern` | `string` | Regular expression pattern | Python `re.search` matching (unanchored by default; use `^` and `$` to anchor). Evaluated consistently on all providers. Matching is time-bounded (1 second); a pathological pattern fails validation instead of hanging the run. | | `minimum` | `number` | Inclusive minimum numeric bound | Value must be greater than or equal to `minimum`. | | `maximum` | `number` | Inclusive maximum numeric bound | Value must be less than or equal to `maximum`. | | `minLength` | `string` | Inclusive minimum string length | String length must be greater than or equal to `minLength`. | @@ -233,7 +233,7 @@ Output field definitions support optional validation constraints to enforce valu #### JSON Schema and Validation Semantics - **Inclusive Bounds**: `minimum`, `maximum`, `minLength`, and `maxLength` represent inclusive bounds. -- **Regex Pattern Matching**: `pattern` uses Python `re.search` semantics across all providers (including Claude). It matches anywhere in the target string unless explicitly anchored with `^` and `$`. +- **Regex Pattern Matching**: `pattern` uses Python `re.search` semantics across all providers (including Claude). It matches anywhere in the target string unless explicitly anchored with `^` and `$`. Matching runs on a `re`-compatible engine with a 1-second wall-clock deadline per check: model output is untrusted input, so a pattern with catastrophic backtracking raises a validation error (which drives the provider's output-recovery loop) instead of stalling the workflow. - **Nullable Fields**: Setting `nullable: true` renders the JSON Schema type as `type: [T, "null"]`, allowing the field to hold `null` or a value matching `type`. - **Optional Object Properties**: The `required: false` constraint is permitted **only inside nested object properties** (e.g. `properties.details.required: false`). All root-level output fields must be required, so setting `required: false` on a root-level agent output field will be rejected during workflow validation (`conductor validate`). diff --git a/pyproject.toml b/pyproject.toml index dee56c67..627df4e8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,6 +46,9 @@ dependencies = [ "websockets>=12.0", "httpx>=0.27.0", "packaging>=21.0", + # Timeout-bounded regex matching for output field constraints and provider + # response normalization. Ships typed stubs and supports per-match deadlines. + "regex>=2024.11.6", ] [project.optional-dependencies] diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 27a4b8e6..a80857e0 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -6,10 +6,11 @@ from __future__ import annotations -import re +import functools from typing import Any, Literal, get_args from urllib.parse import urlparse +import regex from pydantic import ( BaseModel, ConfigDict, @@ -39,6 +40,12 @@ # certainly wants ``limits.timeout_seconds`` reconsidered first. MAX_WAIT_DURATION_SECONDS = 24 * 60 * 60 +# Wall-clock bound for a single pattern match. Model output is untrusted input +# and Python ``re`` has no timeout, so matching uses the third-party ``regex`` +# engine which supports deadlines (and releases the GIL, so a pathological +# pattern cannot stall the event loop and neighboring parallel agents). +PATTERN_MATCH_TIMEOUT_SECONDS = 1.0 + class InputDef(BaseModel): """Definition for a workflow input parameter.""" @@ -88,6 +95,8 @@ def validate_default_type(cls, v: Any, info) -> Any: class OutputField(BaseModel): """Schema for a single output field from an agent.""" + model_config = ConfigDict(extra="forbid") + type: Literal["string", "number", "boolean", "array", "object"] """The type of the output field.""" @@ -106,10 +115,10 @@ class OutputField(BaseModel): pattern: str | None = None """Regular expression pattern for string types.""" - minimum: float | None = None + minimum: int | float | None = None """Minimum value for number types.""" - maximum: float | None = None + maximum: int | float | None = None """Maximum value for number types.""" minLength: int | None = None @@ -124,6 +133,19 @@ class OutputField(BaseModel): nullable: bool = False """Whether the field value may be null.""" + @functools.cached_property + def compiled_pattern(self) -> Any: + """Return a compiled regex pattern, or ``None`` when no pattern is set. + + Annotated as ``Any`` because the repo type checker (ty) does not yet + read the ``regex`` package stubs; the runtime object is always a + ``regex.Pattern`` or ``None``. + """ + + if self.pattern is None: + return None + return regex.compile(self.pattern) + @model_validator(mode="after") def validate_type_specific_fields(self) -> OutputField: """Ensure type-specific fields are properly set and consistent.""" @@ -186,11 +208,12 @@ def validate_type_specific_fields(self) -> OutputField: if self.minimum is not None and self.maximum is not None and self.minimum > self.maximum: raise ValueError("minimum cannot be greater than maximum") - # Pattern compilation. + # Pattern compilation. ``regex`` is a strict superset of the stdlib + # ``re`` module, so every previously valid pattern still compiles. if self.pattern is not None: try: - re.compile(self.pattern) - except re.error as exc: + regex.compile(self.pattern) + except regex.error as exc: raise ValueError(f"pattern is not a valid regular expression: {exc}") from exc return self diff --git a/src/conductor/executor/output.py b/src/conductor/executor/output.py index 4ea3944e..4880b4c6 100644 --- a/src/conductor/executor/output.py +++ b/src/conductor/executor/output.py @@ -6,12 +6,15 @@ from __future__ import annotations +import logging import re from typing import Any -from conductor.config.schema import OutputField +from conductor.config.schema import PATTERN_MATCH_TIMEOUT_SECONDS, OutputField from conductor.exceptions import ValidationError +logger = logging.getLogger(__name__) + def validate_output( content: dict[str, Any], @@ -46,52 +49,71 @@ def validate_output( _validate_field(field_name, content[field_name], field_def) + undeclared_keys = [k for k in content if k not in schema] + if undeclared_keys: + logger.warning( + "Output contains undeclared fields not present in the output schema: %s — " + "check for typos against the declared output fields", + undeclared_keys, + ) + def _check_constraints(field_name: str, value: Any, field_def: OutputField) -> None: - """Validate scalar constraints (enum, pattern, length, range) for a field. + """Validate scalar constraints (enum, length, pattern, range) for a field. Called after the type check has passed, so ``value`` is known to match ``field_def.type``. Raises ``ValidationError`` with a suggestion on failure. """ - if field_def.enum is not None: - if isinstance(value, bool): - if not (field_def.type == "boolean" and value in field_def.enum): - raise ValidationError( - f"Output field '{field_name}' must be one of {field_def.enum!r}, got {value!r}", - suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", - ) - elif value not in field_def.enum: - raise ValidationError( - f"Output field '{field_name}' must be one of {field_def.enum!r}, got {value!r}", - suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", - ) - - if field_def.pattern is not None and re.search(field_def.pattern, value) is None: + if field_def.enum is not None and value not in field_def.enum: raise ValidationError( - f"Output field '{field_name}' does not match pattern '{field_def.pattern}'", - suggestion=f"Ensure '{field_name}' matches the pattern '{field_def.pattern}'", + f"Output field '{field_name}' must be one of {field_def.enum!r}, got {value!r}", + suggestion=f"Ensure '{field_name}' is one of {field_def.enum!r}", ) if field_def.type == "string": if field_def.minLength is not None and len(value) < field_def.minLength: raise ValidationError( - f"Output field '{field_name}' is shorter than minLength {field_def.minLength}", + f"Output field '{field_name}' is shorter than minLength {field_def.minLength} " + f"(received: {_describe_value(value)})", suggestion=f"Ensure '{field_name}' has at least {field_def.minLength} characters", ) if field_def.maxLength is not None and len(value) > field_def.maxLength: raise ValidationError( - f"Output field '{field_name}' is longer than maxLength {field_def.maxLength}", + f"Output field '{field_name}' is longer than maxLength {field_def.maxLength} " + f"(received: {_describe_value(value)})", suggestion=f"Ensure '{field_name}' has at most {field_def.maxLength} characters", ) - elif field_def.type == "number": + + if field_def.pattern is not None: + try: + match = field_def.compiled_pattern.search(value, timeout=PATTERN_MATCH_TIMEOUT_SECONDS) + except TimeoutError as e: + raise ValidationError( + f"Output field '{field_name}' pattern match exceeded the " + f"{PATTERN_MATCH_TIMEOUT_SECONDS}s time limit", + suggestion=( + f"Simplify the pattern for '{field_name}' " + "or check for catastrophic backtracking" + ), + ) from e + if match is None: + raise ValidationError( + f"Output field '{field_name}' does not match pattern '{field_def.pattern}' " + f"(received: {_describe_value(value)})", + suggestion=f"Ensure '{field_name}' matches the pattern '{field_def.pattern}'", + ) + + if field_def.type == "number": if field_def.minimum is not None and value < field_def.minimum: raise ValidationError( - f"Output field '{field_name}' is below minimum {field_def.minimum}", + f"Output field '{field_name}' is below minimum {field_def.minimum} " + f"(received: {_describe_value(value)})", suggestion=f"Ensure '{field_name}' is at least {field_def.minimum}", ) if field_def.maximum is not None and value > field_def.maximum: raise ValidationError( - f"Output field '{field_name}' is above maximum {field_def.maximum}", + f"Output field '{field_name}' is above maximum {field_def.maximum} " + f"(received: {_describe_value(value)})", suggestion=f"Ensure '{field_name}' is at most {field_def.maximum}", ) @@ -144,7 +166,7 @@ def _validate_field(field_name: str, value: Any, field_def: OutputField) -> None f"(received: {_describe_value(item)})", suggestion=f"Ensure all items in '{field_name}' have correct type", ) - _validate_field(field_name, item, field_def.items) + _validate_field(f"array item {i} in '{field_name}'", item, field_def.items) def _describe_value(value: Any, max_chars: int = 200) -> str: @@ -223,7 +245,6 @@ def parse_json_output(raw_response: str) -> dict[str, Any]: ValidationError: If JSON parsing fails. """ import json - import re text = raw_response.strip() diff --git a/src/conductor/providers/_pydantic_ai/converters.py b/src/conductor/providers/_pydantic_ai/converters.py index 57616f96..e6ca404e 100644 --- a/src/conductor/providers/_pydantic_ai/converters.py +++ b/src/conductor/providers/_pydantic_ai/converters.py @@ -8,12 +8,11 @@ from __future__ import annotations import copy -import re from typing import Annotated, Any from pydantic import AfterValidator, BaseModel, BeforeValidator, ConfigDict, Field, create_model -from conductor.config.schema import OutputField +from conductor.config.schema import PATTERN_MATCH_TIMEOUT_SECONDS, OutputField def _reject_bool(value: Any) -> Any: @@ -36,24 +35,18 @@ def _reject_bool(value: Any) -> Any: """Conductor ``integer`` type: accepts integers, rejects booleans.""" -class _NoDefaultBaseModel(BaseModel): - """Dynamic model base that sanitizes generated JSON schemas. +class _OutputBaseModel(BaseModel): + """Dynamic model base that tolerates extra output keys. - Pydantic v2 emits ``"default": null`` for ``Field(default=None)`` and uses - ``anyOf`` for nullable unions and ``$ref``/``$defs`` for nested models. - Pydantic AI's tool schema must not include the JSON Schema ``default``, - ``anyOf``, ``$ref``, or ``$defs`` keywords, so this base removes them and - flattens nullable unions into ``{type: [T, "null"]}``. + ``extra="allow"`` matches Conductor's ``validate_output``, which ignores + undeclared keys rather than rejecting them. Tool-schema sanitization is + performed separately in :func:`agent_builder.build_agent` so that the + schema attached to the Pydantic AI tool definition contains only the + agreed structural keywords. """ model_config = ConfigDict(extra="allow") - @classmethod - def model_json_schema(cls, *args: Any, **kwargs: Any) -> dict[str, Any]: - schema = super().model_json_schema(*args, **kwargs) - _sanitize_json_schema(schema) - return schema - def _convert_anyof_nullable(node: dict[str, Any]) -> dict[str, Any]: """Convert a nullable ``anyOf`` union to a flat schema with ``type: [T, "null"]``. @@ -62,6 +55,13 @@ def _convert_anyof_nullable(node: dict[str, Any]) -> dict[str, Any]: ``T | None``. The output tool schema must not contain ``anyOf``, so this helper merges the non-null branch's constraints into a single schema while preserving siblings such as ``title`` or ``description``. + + The value branch may itself be a union (e.g. Conductor ``number`` is + ``int | float``), in which case pydantic emits a nested ``anyOf`` with no + top-level ``type`` key. In that case the inner union types are collected + into ``type: [T1, T2, "null"]`` when every inner branch declares a type; + otherwise the original ``anyOf`` node is returned unchanged so the null + branch is never silently dropped. """ any_of = node.get("anyOf", []) if len(any_of) != 2: @@ -82,6 +82,15 @@ def _convert_anyof_nullable(node: dict[str, Any]) -> dict[str, Any]: value_type = merged.pop("type", None) if value_type is not None: merged["type"] = [value_type, "null"] + elif "anyOf" in merged: + inner = merged.pop("anyOf") + types = [b["type"] for b in inner if isinstance(b, dict) and "type" in b] + if len(types) == len(inner): + merged["type"] = [*types, "null"] + else: + merged["anyOf"] = [*inner, null_branch] + else: + return node for key, val in node.items(): if key != "anyOf": @@ -93,9 +102,10 @@ def _convert_anyof_nullable(node: dict[str, Any]) -> dict[str, Any]: def _sanitize_json_schema(schema: dict[str, Any]) -> None: """Recursively sanitize a JSON schema in place. - Removes ``default``, ``$ref``, and ``$defs`` keywords and flattens nullable - ``anyOf`` unions so the generated Pydantic-AI tool schema contains only the - agreed structural keywords. + Removes ``default``, ``$ref``, and ``$defs`` keywords, strips the raw + non-standard ``ge``/``le`` keys pydantic emits for union numeric ranges, + and flattens nullable ``anyOf`` unions so the generated Pydantic-AI tool + schema contains only the agreed structural keywords. """ defs = schema.pop("$defs", {}) _sanitize_schema_node(schema, defs) @@ -105,9 +115,9 @@ def _sanitize_json_schema(schema: dict[str, Any]) -> None: def _sanitize_schema_node(node: Any, defs: dict[str, Any]) -> None: """Recursively sanitize a JSON schema node in place. - ``$ref`` targets that cannot be resolved are left untouched so a caller with - a complete view of ``$defs`` (e.g. after Pydantic-AI builds the tool schema) - can perform a final inlining pass. + ``$ref`` targets that cannot be resolved are left untouched defensively; + the caller in ``agent_builder.py`` already has the complete ``$defs`` view + at the time of sanitization, so unresolved references should not occur. """ if isinstance(node, dict): nested_defs = node.pop("$defs", None) @@ -133,6 +143,8 @@ def _sanitize_schema_node(node: Any, defs: dict[str, Any]) -> None: node.update(converted) node.pop("default", None) + node.pop("ge", None) + node.pop("le", None) for value in node.values(): _sanitize_schema_node(value, defs) @@ -141,24 +153,16 @@ def _sanitize_schema_node(node: Any, defs: dict[str, Any]) -> None: _sanitize_schema_node(item, defs) -def _make_enum_validator(enum_values: list[Any], field_type: str) -> Any: +def _make_enum_validator(enum_values: list[Any]) -> Any: """Return an AfterValidator that enforces enum membership. - Matches the shared semantics used by ``validate_output``: - - - A boolean value only passes when the field type is ``boolean`` and the - value is present in the enum. - - For all other values, plain Python membership is used (so ``1.0`` - satisfies a number enum ``[1]``). + Matches the shared semantics used by ``validate_output``: plain Python + membership is used (so ``1.0`` satisfies a number enum ``[1]``), and + booleans are rejected for non-boolean fields by the base-type validators + before reaching this validator. """ def _validate(value: Any) -> Any: - if value is None: - return value - if isinstance(value, bool): - if field_type == "boolean" and value in enum_values: - return value - raise ValueError(f"{value!r} is not a valid boolean enum value") if value in enum_values: return value raise ValueError(f"{value!r} is not one of {enum_values!r}") @@ -166,21 +170,27 @@ def _validate(value: Any) -> Any: return _validate -def _make_pattern_validator(pattern: str) -> Any: - """Return an AfterValidator that runs Python ``re.search``. +def _make_pattern_validator(compiled: Any) -> Any: + """Return an AfterValidator that runs ``regex.search`` with a timeout. Mirrors ``validate_output`` so Python-only regex constructs (lookarounds, backreferences) are evaluated by Python's regex engine, not pydantic's - Rust-based default. + Rust-based default. A match that exceeds the wall-clock bound is raised as + a ``ValueError`` so pydantic-ai retries the output instead of hanging on a + pathological pattern. """ def _validate(value: Any) -> Any: - if value is None: - return value if not isinstance(value, str): raise ValueError("pattern can only be applied to strings") - if re.search(pattern, value) is None: - raise ValueError(f"value does not match pattern {pattern!r}") + try: + if compiled.search(value, timeout=PATTERN_MATCH_TIMEOUT_SECONDS) is None: + raise ValueError(f"value does not match pattern {compiled.pattern!r}") + except TimeoutError as exc: + raise ValueError( + f"pattern match exceeded the {PATTERN_MATCH_TIMEOUT_SECONDS}s time limit " + "(catastrophic backtracking risk)" + ) from exc return value return _validate @@ -205,18 +215,17 @@ def _field_json_schema_extra(field: OutputField) -> dict[str, Any] | None: """Build ``json_schema_extra`` advertising Conductor constraints. Pydantic AI attaches this to the generated tool schema so the model sees - the same ``enum``/``pattern``/length/range keywords as the shared JSON - Schema builders. + the same ``enum``/``pattern``/range keywords as the shared JSON Schema + builders. Length constraints are already emitted by pydantic from the + ``min_length``/``max_length`` ``Field`` kwargs, so they are not duplicated + here. Nullable fields advertise ``None`` in the enum so the schema matches + the values accepted at validation time. """ extra: dict[str, Any] = {} if field.enum is not None: - extra["enum"] = field.enum + extra["enum"] = [*field.enum, None] if field.nullable else field.enum if field.pattern is not None: extra["pattern"] = field.pattern - if field.minLength is not None: - extra["minLength"] = field.minLength - if field.maxLength is not None: - extra["maxLength"] = field.maxLength if field.minimum is not None: extra["minimum"] = field.minimum if field.maximum is not None: @@ -232,14 +241,17 @@ def _wrap_scalar_field_type(field: OutputField, base_type: Any) -> Any: ``Field`` level (``default=None``); the type union only changes when the field is explicitly nullable. """ - annotated = base_type + annotated: Any = base_type if field.enum is not None: annotated = Annotated[ annotated, - AfterValidator(_make_enum_validator(field.enum, field.type)), + AfterValidator(_make_enum_validator(field.enum)), + ] + if field.compiled_pattern is not None: + annotated = Annotated[ + annotated, + AfterValidator(_make_pattern_validator(field.compiled_pattern)), ] - if field.pattern is not None: - annotated = Annotated[annotated, AfterValidator(_make_pattern_validator(field.pattern))] if field.nullable: annotated = annotated | None @@ -268,7 +280,7 @@ def _build_field_info(field: OutputField) -> Any: field_kwargs["min_length"] = field.minLength if field.maxLength is not None: field_kwargs["max_length"] = field.maxLength - elif field.type in ("number", "integer"): + elif field.type == "number": if field.minimum is not None: field_kwargs["ge"] = field.minimum if field.maximum is not None: @@ -286,12 +298,12 @@ def _build_array_item_type( depth: int = 0, max_depth: int = 10, ) -> Any: - """Build a Pydantic type for an array item, applying scalar constraints. + """Build a Pydantic type for an array item, applying constraints. Array elements are validated individually, so the item's ``nullable`` flag and scalar constraints (enum, pattern, length, range) are applied exactly - like top-level scalar fields. Object and array items delegate constraint - enforcement to the recursive type builder. + like top-level scalar fields. Object and array items are built recursively; + their nullability is applied as a union with ``None`` when requested. """ if depth > max_depth: raise ValueError(f"Maximum output schema nesting depth of {max_depth} exceeded") @@ -303,11 +315,14 @@ def _build_array_item_type( max_depth=max_depth, ) - if field.type in ("string", "number", "integer", "boolean"): + if field.type in ("string", "number", "boolean"): base_type = _wrap_scalar_field_type(field, base_type) field_info = _build_field_info(field) return Annotated[base_type, field_info] + if field.nullable: + base_type = base_type | None + return base_type @@ -421,7 +436,7 @@ def _build_pydantic_model( return create_model( name, **model_fields, - __base__=_NoDefaultBaseModel, + __base__=_OutputBaseModel, __doc__=description, ) diff --git a/src/conductor/providers/_schema.py b/src/conductor/providers/_schema.py index 04b59280..c22a96e8 100644 --- a/src/conductor/providers/_schema.py +++ b/src/conductor/providers/_schema.py @@ -72,7 +72,9 @@ def build_json_schema_field( schema["description"] = field.description if field.enum is not None: - schema["enum"] = field.enum + # YAML `enum` cannot contain null (schema.py rejects it); append None + # here so the generated schema is honest when type is ["string", "null"]. + schema["enum"] = [*field.enum, None] if field.nullable else field.enum if field.type == "string": if field.pattern is not None: @@ -160,7 +162,9 @@ def build_prompt_schema_field( schema["description"] = description if field.enum is not None: - schema["enum"] = field.enum + # YAML `enum` cannot contain null (schema.py rejects it); append None + # here so the generated schema is honest when type is ["string", "null"]. + schema["enum"] = [*field.enum, None] if field.nullable else field.enum if field.type == "string": if field.pattern is not None: diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index 3bd8db51..1498cfb6 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -142,6 +142,35 @@ def test_output_field_constraint_happy_path(self) -> None: assert output.required is True assert output.nullable is False + def test_output_field_extra_keys_rejected(self) -> None: + """Unknown keys like a typo in minlength are rejected instead of ignored.""" + with pytest.raises(ValidationError) as exc_info: + OutputField.model_validate({"type": "string", "minlength": 5}) + assert "minlength" in str(exc_info.value) + + def test_output_field_minimum_integer_round_trips(self) -> None: + """An integral minimum round-trips through model_dump as an int, not 0.0.""" + output = OutputField.model_validate({"type": "number", "minimum": 0, "maximum": 10}) + dumped = output.model_dump() + assert dumped["minimum"] == 0 + assert type(dumped["minimum"]) is int + assert dumped["maximum"] == 10 + assert type(dumped["maximum"]) is int + + def test_output_field_compiled_pattern_and_dump_exclusion(self) -> None: + """compiled_pattern returns a usable regex and is excluded from model_dump.""" + output = OutputField.model_validate({"type": "string", "pattern": "^a+$"}) + assert output.compiled_pattern is not None + assert output.compiled_pattern.match("aaa") is not None + assert output.compiled_pattern.match("bbb") is None + assert "compiled_pattern" not in output.model_dump() + + def test_output_field_lookahead_pattern_compiles(self) -> None: + """A Python-only lookahead pattern passes load-time validation.""" + output = OutputField.model_validate({"type": "string", "pattern": r"^(?=.*A).*$"}) + assert output.pattern == r"^(?=.*A).*$" + assert output.compiled_pattern is not None + def test_output_field_model_dump_round_trip(self) -> None: """Test model_dump preserves constraint fields for reconstruction.""" original = OutputField.model_validate( diff --git a/tests/test_executor/test_output.py b/tests/test_executor/test_output.py index 119c016a..6951be5e 100644 --- a/tests/test_executor/test_output.py +++ b/tests/test_executor/test_output.py @@ -8,6 +8,7 @@ - JSON parsing from raw responses """ +import time from typing import Any import pytest @@ -622,9 +623,84 @@ def test_constraints_inside_array_object_items_rejected(self) -> None: schema, ) + def test_pattern_timeout_raises_instead_of_hanging(self) -> None: + """A pathological regex must raise a ValidationError naming the field and + the configured time limit, and must complete well under the test deadline.""" + schema = {"value": OutputField(type="string", pattern=r"^(a|aa)+$")} + content = {"value": "a" * 60 + "b"} + + start = time.monotonic() + with pytest.raises( + ValidationError, + match="Output field 'value' pattern match exceeded the 1.0s time limit", + ): + validate_output(content, schema) + elapsed = time.monotonic() - start + + assert elapsed < 5.0 + + def test_length_checked_before_pattern(self) -> None: + """When a string violates both maxLength and pattern, the cheaper length + check must run first and name the length violation.""" + schema = {"value": OutputField(type="string", pattern=r"^[a-z]+$", maxLength=3)} + content = {"value": "xyz!"} + + with pytest.raises(ValidationError, match="longer than maxLength 3"): + validate_output(content, schema) + + def test_array_item_constraint_failure_names_index(self) -> None: + """A constraint failure on the third array element must name the index in + the error message, matching the existing array type-error convention.""" + schema = { + "values": OutputField( + type="array", + items=OutputField(type="string", pattern=r"^[ab]$"), + ) + } + content = {"values": ["a", "b", "c"]} + + with pytest.raises( + ValidationError, + match=r"array item 2 in 'values'.*does not match pattern", + ): + validate_output(content, schema) + + def test_undeclared_keys_warn(self, caplog: pytest.LogCaptureFixture) -> None: + """Extra keys not present in the schema must log a warning naming the + suspect keys so typos in optional output fields are visible.""" + schema = {"declared": OutputField(type="string")} + content = {"declared": "x", "declred": "typo"} + + with caplog.at_level("WARNING"): + validate_output(content, schema) + + assert "declred" in caplog.text + assert "undeclared fields not present in the output schema" in caplog.text + + def test_declared_keys_do_not_warn(self, caplog: pytest.LogCaptureFixture) -> None: + """Content whose keys exactly match the schema must not emit a warning.""" + schema = {"declared": OutputField(type="string")} + content = {"declared": "x"} + + with caplog.at_level("WARNING"): + validate_output(content, schema) + + assert "undeclared fields" not in caplog.text + + def test_enum_number_still_rejects_boolean_and_accepts_float(self) -> None: + """A number enum [1] must reject True via the type check and accept 1.0 via + plain equality, preserving the pre-existing pinned semantics.""" + schema = {"value": OutputField(type="number", enum=[1])} + + with pytest.raises(ValidationError, match="has wrong type"): + validate_output({"value": True}, schema) + + # Should not raise + validate_output({"value": 1.0}, schema) + class TestValidateOutputNullableArrayItems: - """Regression tests for nullable array items (F1 round 2).""" + """Regression tests for nullable array items in validate_output.""" def test_nullable_array_item_null_passes(self) -> None: """A nullable string array item must accept None.""" diff --git a/tests/test_providers/test_output_constraints_parity.py b/tests/test_providers/test_output_constraints_parity.py index 628be037..9a4deb0b 100644 --- a/tests/test_providers/test_output_constraints_parity.py +++ b/tests/test_providers/test_output_constraints_parity.py @@ -14,10 +14,12 @@ import pytest from pydantic import ValidationError as PydanticValidationError +from pydantic_ai.output import ToolOutput from conductor.config.schema import AgentDef, OutputField from conductor.exceptions import ValidationError as ConductorValidationError from conductor.executor.output import validate_output +from conductor.providers._pydantic_ai.agent_builder import build_agent from conductor.providers._pydantic_ai.converters import output_schema_to_pydantic_model from conductor.providers.copilot import CopilotProvider @@ -27,6 +29,28 @@ def _stub_handler(agent: AgentDef, prompt: str, context: dict[str, Any]) -> dict return {} +def _extract_output_tool_schema(agent: Any) -> dict[str, Any] | None: + """Return the sanitized parameters_json_schema for the output tool.""" + if not isinstance(agent.output_type, ToolOutput): + return None + toolset = agent._output_schema.toolset + if toolset is None or not toolset._tool_defs: + return None + return toolset._tool_defs[0].parameters_json_schema + + +def _assert_no_keys(node: Any, *keys: str) -> None: + """Recursively assert that none of the given JSON Schema keys appear.""" + if isinstance(node, dict): + for key in keys: + assert key not in node, f"forbidden key {key!r} found in schema: {node}" + for value in node.values(): + _assert_no_keys(value, *keys) + elif isinstance(node, list): + for item in node: + _assert_no_keys(item, *keys) + + # Shared schema exercising every constraint keyword supported by OutputField. # It intentionally mixes nullable, optional, and required fields so the tests # can verify that each keyword survives its provider-specific transformation. @@ -46,6 +70,23 @@ def _stub_handler(agent: AgentDef, prompt: str, context: dict[str, Any]) -> dict maxLength=10, required=False, ), + "ratio": OutputField(type="number", minimum=0, maximum=10, nullable=True), + "nullable_enum": OutputField(type="string", enum=["A", "B"], nullable=True), + "rows": OutputField( + type="array", + items=OutputField( + type="object", + nullable=True, + properties={"name": OutputField(type="string")}, + ), + ), + "nested": OutputField( + type="object", + properties={ + "req": OutputField(type="string"), + "opt": OutputField(type="string", required=False), + }, + ), } @@ -80,6 +121,27 @@ def test_copilot_prompt_schema_contains_all_constraint_keywords(self) -> None: assert label["minLength"] == 1 assert label["maxLength"] == 10 + ratio = schema["ratio"] + assert ratio["type"] == ["number", "null"] + assert ratio["minimum"] == 0 + assert ratio["maximum"] == 10 + + nullable_enum = schema["nullable_enum"] + assert nullable_enum["type"] == ["string", "null"] + assert nullable_enum["enum"] == ["A", "B", None] + + rows = schema["rows"] + assert rows["type"] == "array" + assert rows["items"]["type"] == ["object", "null"] + assert rows["items"]["properties"]["name"]["type"] == "string" + assert rows["items"]["required"] == ["name"] + + nested = schema["nested"] + assert nested["type"] == "object" + assert nested["required"] == ["req"] + assert nested["properties"]["req"]["type"] == "string" + assert nested["properties"]["opt"]["type"] == "string" + class TestHermesPromptSchema: """HermesProvider._build_prompt_schema must carry every constraint keyword.""" @@ -114,6 +176,27 @@ def test_hermes_prompt_schema_contains_all_constraint_keywords(self) -> None: assert label["minLength"] == 1 assert label["maxLength"] == 10 + ratio = schema["ratio"] + assert ratio["type"] == ["number", "null"] + assert ratio["minimum"] == 0 + assert ratio["maximum"] == 10 + + nullable_enum = schema["nullable_enum"] + assert nullable_enum["type"] == ["string", "null"] + assert nullable_enum["enum"] == ["A", "B", None] + + rows = schema["rows"] + assert rows["type"] == "array" + assert rows["items"]["type"] == ["object", "null"] + assert rows["items"]["properties"]["name"]["type"] == "string" + assert rows["items"]["required"] == ["name"] + + nested = schema["nested"] + assert nested["type"] == "object" + assert nested["required"] == ["req"] + assert nested["properties"]["req"]["type"] == "string" + assert nested["properties"]["opt"]["type"] == "string" + class TestClaudeAgentSdkOutputFormat: """claude_agent_sdk._build_output_format must carry every constraint keyword.""" @@ -121,7 +204,7 @@ class TestClaudeAgentSdkOutputFormat: def test_claude_agent_sdk_output_format_contains_all_constraint_keywords(self) -> None: """The SDK output_format payload must contain enum, pattern, length, range, and nullable keywords in the inner JSON schema, and must mark - required fields only (not optional ones).""" + all declared fields required in the schema sent to the SDK.""" pytest.importorskip( "claude_agent_sdk", reason="claude-agent-sdk extra not installed", @@ -134,7 +217,7 @@ def test_claude_agent_sdk_output_format_contains_all_constraint_keywords(self) - schema = payload["schema"] props = schema["properties"] - assert schema["required"] == ["category", "code", "score"] + assert schema["required"] == list(SHARED_OUTPUT_SCHEMA.keys()) category = props["category"] assert category["type"] == "string" @@ -156,27 +239,208 @@ def test_claude_agent_sdk_output_format_contains_all_constraint_keywords(self) - assert label["minLength"] == 1 assert label["maxLength"] == 10 + ratio = props["ratio"] + assert ratio["type"] == ["number", "null"] + assert ratio["minimum"] == 0 + assert ratio["maximum"] == 10 + + nullable_enum = props["nullable_enum"] + assert nullable_enum["type"] == ["string", "null"] + assert nullable_enum["enum"] == ["A", "B", None] + + rows = props["rows"] + assert rows["type"] == "array" + assert rows["items"]["type"] == ["object", "null"] + assert rows["items"]["properties"]["name"]["type"] == "string" + assert rows["items"]["required"] == ["name"] + + nested = props["nested"] + assert nested["type"] == "object" + assert nested["required"] == ["req"] + assert nested["properties"]["req"]["type"] == "string" + assert nested["properties"]["opt"]["type"] == "string" + + +@pytest.fixture(scope="module") +def parity_model() -> type[Any]: + """Build the Claude dynamic model once for the payload matrix.""" + model = output_schema_to_pydantic_model("Parity", SHARED_OUTPUT_SCHEMA) + assert model is not None + return model + class TestClaudePydanticModel: """output_schema_to_pydantic_model must enforce the constraints.""" - def test_claude_dynamic_model_accepts_conforming_payload(self) -> None: + def test_claude_dynamic_model_accepts_conforming_payload(self, parity_model: type[Any]) -> None: """A payload satisfying every constraint must validate cleanly.""" - model = output_schema_to_pydantic_model("Constrained", SHARED_OUTPUT_SCHEMA) - assert model is not None + instance = parity_model.model_validate( + { + "category": "A", + "code": "XYZ", + "score": 42, + "ratio": None, + "nullable_enum": None, + "rows": [None, {"name": "x"}], + "nested": {"req": "r"}, + } + ) + dumped = instance.model_dump() + assert dumped["category"] == "A" + assert dumped["code"] == "XYZ" + assert dumped["score"] == 42 + assert dumped["ratio"] is None + assert dumped["nullable_enum"] is None + assert dumped["rows"][0] is None + assert dumped["rows"][1]["name"] == "x" + assert dumped["nested"]["req"] == "r" + + def test_claude_dynamic_model_rejects_violating_payload(self, parity_model: type[Any]) -> None: + """A payload violating a constraint must raise Pydantic ValidationError.""" + with pytest.raises(PydanticValidationError): + parity_model(category="Z", code="XYZ", score=42) + + +class TestClaudeValidateOutputParity: + """The Claude dynamic model and validate_output must agree on every payload. + + This matrix exists because the reviewer mutation-tested the original parity + fixture: deleting all range/length enforcement or all nullable handling from + the Claude path left the test file green. Each row pins a concrete accept or + reject decision and asserts that both enforcement paths reach the same + verdict, so a missing constraint in either path breaks the test. + """ + + _BASE = { + "category": "A", + "code": "ABC", + "score": 5, + "ratio": None, + "nullable_enum": None, + "rows": [], + "nested": {"req": "r"}, + } + + @pytest.mark.parametrize( + ("payload", "expect_accept"), + [ + ( + { + "category": "A", + "code": "ABC", + "score": 5, + "label": "ok", + "ratio": None, + "nullable_enum": None, + "rows": [None, {"name": "x"}], + "nested": {"req": "r"}, + }, + True, + ), + ( + # Minimal valid payload: optional label omitted, nullable + # fields supplied as None, nested optional property omitted. + { + "category": "A", + "code": None, + "score": 5, + "ratio": None, + "nullable_enum": None, + "rows": [], + "nested": {"req": "r"}, + }, + True, + ), + ({**_BASE, "ratio": 0}, True), + ({**_BASE, "ratio": 10}, True), + ({**_BASE, "ratio": 2.5}, True), + ({**_BASE, "nullable_enum": "B"}, True), + ({**_BASE, "ratio": 42}, False), + ({**_BASE, "ratio": -1}, False), + ({**_BASE, "nullable_enum": "Z"}, False), + ({**_BASE, "score": None}, False), + ({**_BASE, "rows": [{"name": 7}]}, False), + ({**_BASE, "nested": {"req": "r", "opt": 5}}, False), + ({**_BASE, "nested": {}}, False), + ({**_BASE, "code": "AB"}, False), + ({**_BASE, "category": "D"}, False), + ({**_BASE, "score": 101}, False), + ], + ) + def test_claude_and_validate_output_agree( + self, + parity_model: type[Any], + payload: dict[str, Any], + expect_accept: bool, + ) -> None: + """Both enforcement paths must accept or reject the payload together.""" + model_accepted = self._call_accepts(parity_model.model_validate, payload) + validate_accepted = self._call_accepts(validate_output, payload, SHARED_OUTPUT_SCHEMA) + + assert model_accepted == validate_accepted, ( + f"payload {payload!r}: model accepted={model_accepted}, " + f"validate_output accepted={validate_accepted}" + ) + assert model_accepted == expect_accept, ( + f"payload {payload!r}: expected accept={expect_accept}, got {model_accepted}" + ) - instance = model.model_validate({"category": "A", "code": "XYZ", "score": 42}) - assert instance.model_dump()["category"] == "A" - assert instance.model_dump()["code"] == "XYZ" - assert instance.model_dump()["score"] == 42 + @staticmethod + def _call_accepts(func: Any, *args: Any, **kwargs: Any) -> bool: + """Return True if the call succeeds, False if it raises a validation error.""" + try: + func(*args, **kwargs) + except (PydanticValidationError, ConductorValidationError): + return False + return True + + +class TestClaudeToolSchemaHygiene: + """The Pydantic AI output tool schema must sanitize internal pydantic keys.""" + + def test_tool_schema_for_shared_schema_carries_constraints_and_no_defaults(self) -> None: + """The schema attached to the output tool must advertise nullable range + and enum constraints, must keep nullable object/array items honest, and + must never contain the pydantic-internal ``ge``/``le``/``default`` keys + anywhere in the schema tree.""" + agent_def = AgentDef(name="parity", output=SHARED_OUTPUT_SCHEMA) + pydantic_agent = build_agent( + agent_def, + system_prompt="sys", + rendered_prompt="p", + default_model="claude-sonnet-5", + api_key="sk-test-dummy", + ) - def test_claude_dynamic_model_rejects_violating_payload(self) -> None: - """A payload violating a constraint must raise Pydantic ValidationError.""" - model = output_schema_to_pydantic_model("Constrained", SHARED_OUTPUT_SCHEMA) - assert model is not None + schema = _extract_output_tool_schema(pydantic_agent) + assert schema is not None + _assert_no_keys(schema, "ge", "le", "default") - with pytest.raises(PydanticValidationError): - model(category="Z", code="XYZ", score=42) + props = schema["properties"] + + ratio_schema = props["ratio"] + assert isinstance(ratio_schema["type"], list) + assert "null" in ratio_schema["type"] + assert ratio_schema["minimum"] == 0 + assert ratio_schema["maximum"] == 10 + + nullable_enum_schema = props["nullable_enum"] + assert isinstance(nullable_enum_schema["type"], list) + assert "null" in nullable_enum_schema["type"] + assert None in nullable_enum_schema["enum"] + + rows_schema = props["rows"] + items_schema = rows_schema["items"] + assert isinstance(items_schema["type"], list) + assert "null" in items_schema["type"] + assert items_schema["properties"]["name"]["type"] == "string" + assert items_schema["required"] == ["name"] + + nested_schema = props["nested"] + assert nested_schema["type"] == "object" + assert nested_schema["required"] == ["req"] + assert nested_schema["properties"]["req"]["type"] == "string" + assert nested_schema["properties"]["opt"]["type"] == "string" class TestValidateOutputParity: @@ -252,6 +516,26 @@ def test_aca_request_carries_constraint_fields(self) -> None: assert label["required"] is False assert "nullable" not in label + ratio = wire_output["ratio"] + assert ratio["type"] == "number" + assert ratio["nullable"] is True + assert ratio["minimum"] == 0 + assert ratio["maximum"] == 10 + assert isinstance(ratio["minimum"], int) + assert isinstance(ratio["maximum"], int) + + rows = wire_output["rows"] + assert rows["type"] == "array" + assert rows["items"]["type"] == "object" + assert rows["items"]["nullable"] is True + assert rows["items"]["properties"]["name"]["type"] == "string" + + nested = wire_output["nested"] + assert nested["type"] == "object" + assert nested["properties"]["req"]["type"] == "string" + assert nested["properties"]["opt"]["type"] == "string" + assert nested["properties"]["opt"]["required"] is False + def test_aca_runner_reconstructs_identical_output_schema(self) -> None: """The runner's OutputField.model_validate must reconstruct the same effective field values from the wire payload.""" @@ -284,6 +568,26 @@ def test_aca_runner_reconstructs_identical_output_schema(self) -> None: assert original.required == rebuilt.required assert original.nullable == rebuilt.nullable + # Reconstruct nested object and array-item shape from the wire payload. + rows_original = SHARED_OUTPUT_SCHEMA["rows"].items + rows_rebuilt = reconstructed["rows"].items + assert rows_original is not None + assert rows_rebuilt is not None + assert rows_original.type == rows_rebuilt.type + assert rows_original.nullable == rows_rebuilt.nullable + assert rows_original.properties is not None + assert rows_rebuilt.properties is not None + assert rows_original.properties["name"].type == rows_rebuilt.properties["name"].type + + nested_original = SHARED_OUTPUT_SCHEMA["nested"].properties + nested_rebuilt = reconstructed["nested"].properties + assert nested_original is not None + assert nested_rebuilt is not None + assert nested_original["req"].type == nested_rebuilt["req"].type + assert nested_original["req"].required == nested_rebuilt["req"].required + assert nested_original["opt"].type == nested_rebuilt["opt"].type + assert nested_original["opt"].required == nested_rebuilt["opt"].required + def test_aca_wire_body_preserves_constraint_fields(self) -> None: """The actual JSON body sent to the runner must contain the constraint fields after SecretStr unwrapping.""" @@ -305,3 +609,20 @@ def test_aca_wire_body_preserves_constraint_fields(self) -> None: assert wire_output["score"]["minimum"] == 0 assert wire_output["score"]["maximum"] == 100 assert wire_output["label"]["required"] is False + + ratio = wire_output["ratio"] + assert ratio["nullable"] is True + assert ratio["minimum"] == 0 + assert ratio["maximum"] == 10 + assert isinstance(ratio["minimum"], int) + assert isinstance(ratio["maximum"], int) + + rows = wire_output["rows"] + assert rows["items"]["type"] == "object" + assert rows["items"]["nullable"] is True + assert rows["items"]["properties"]["name"]["type"] == "string" + + nested = wire_output["nested"] + assert nested["properties"]["req"]["type"] == "string" + assert nested["properties"]["opt"]["type"] == "string" + assert nested["properties"]["opt"]["required"] is False diff --git a/tests/test_providers/test_output_schema.py b/tests/test_providers/test_output_schema.py index 5cec7f5f..51bf6a65 100644 --- a/tests/test_providers/test_output_schema.py +++ b/tests/test_providers/test_output_schema.py @@ -975,3 +975,41 @@ def test_prompt_schema_omits_description_for_optional_field_without_description( "opt": {"type": "string"}, }, } + + def test_json_schema_nullable_enum_appends_none(self) -> None: + """A nullable string field with enum must emit enum including None so the + JSON schema honestly allows null alongside the listed values.""" + field = OutputField(type="string", enum=["a", "b"], nullable=True) + + actual = build_json_schema_field(field) + + assert actual["type"] == ["string", "null"] + assert actual["enum"] == ["a", "b", None] + + def test_json_schema_non_nullable_enum_unchanged(self) -> None: + """A non-nullable enum must be emitted exactly as declared.""" + field = OutputField(type="string", enum=["a", "b"], nullable=False) + + actual = build_json_schema_field(field) + + assert actual["type"] == "string" + assert actual["enum"] == ["a", "b"] + + def test_prompt_schema_nullable_enum_appends_none(self) -> None: + """A nullable string field with enum must include None in the prompt schema + enum just as in the JSON schema.""" + field = OutputField(type="string", enum=["a", "b"], nullable=True) + + actual = build_prompt_schema_field(field) + + assert actual["type"] == ["string", "null"] + assert actual["enum"] == ["a", "b", None] + + def test_prompt_schema_non_nullable_enum_unchanged(self) -> None: + """A non-nullable enum in the prompt schema must remain exactly as declared.""" + field = OutputField(type="string", enum=["a", "b"], nullable=False) + + actual = build_prompt_schema_field(field) + + assert actual["type"] == "string" + assert actual["enum"] == ["a", "b"] diff --git a/tests/test_providers/test_pydantic_ai_agent_builder.py b/tests/test_providers/test_pydantic_ai_agent_builder.py index 2318f03f..bf7820a5 100644 --- a/tests/test_providers/test_pydantic_ai_agent_builder.py +++ b/tests/test_providers/test_pydantic_ai_agent_builder.py @@ -46,6 +46,28 @@ def _extract_output_model(agent: Agent[Any, Any]) -> type[BaseModel] | None: return None +def _extract_output_tool_schema(agent: Agent[Any, Any]) -> dict[str, Any] | None: + """Return the sanitized parameters_json_schema for the output tool.""" + if not isinstance(agent.output_type, ToolOutput): + return None + toolset = agent._output_schema.toolset + if toolset is None or not toolset._tool_defs: + return None + return toolset._tool_defs[0].parameters_json_schema + + +def _assert_no_keys(node: Any, *keys: str) -> None: + """Recursively assert that none of the given JSON Schema keys appear.""" + if isinstance(node, dict): + for key in keys: + assert key not in node, f"forbidden key {key!r} found in schema: {node}" + for value in node.values(): + _assert_no_keys(value, *keys) + elif isinstance(node, list): + for item in node: + _assert_no_keys(item, *keys) + + class TestModelMapping: """Tests for resolving the Anthropic model identifier.""" @@ -574,3 +596,74 @@ def test_auth_token_reaches_client(self) -> None: assert isinstance(pydantic_agent.model, AnthropicModel) assert pydantic_agent.model.client.auth_token == "bearer-token" + + +class TestToolSchemaSanitization: + """Regression tests for the JSON schema attached to the Pydantic AI output tool.""" + + def test_nullable_ranged_number_advertises_null_and_strips_internal_keys(self) -> None: + """A nullable number with minimum/maximum must be advertised to the model + as a type list that includes ``null``, must keep the ``minimum``/``maximum`` + keywords, and must never expose pydantic-internal ``ge``/``le``/``default`` + keys.""" + agent_def = AgentDef( + name="ratio", + output={ + "ratio": OutputField( + type="number", + minimum=0, + maximum=10, + nullable=True, + ) + }, + ) + + pydantic_agent = build_agent( + agent_def, + system_prompt="sys", + rendered_prompt="p", + default_model="claude-sonnet-5", + api_key="sk-test-dummy", + ) + + schema = _extract_output_tool_schema(pydantic_agent) + assert schema is not None + ratio_schema = schema["properties"]["ratio"] + assert "type" in ratio_schema + assert isinstance(ratio_schema["type"], list) + assert "null" in ratio_schema["type"] + assert ratio_schema["minimum"] == 0 + assert ratio_schema["maximum"] == 10 + _assert_no_keys(schema, "ge", "le", "default") + + def test_ranged_number_never_exposes_ge_le(self) -> None: + """A non-nullable ranged number must advertise ``minimum``/``maximum`` + while keeping the raw ``ge``/``le`` keys out of the tool schema.""" + agent_def = AgentDef( + name="score", + output={ + "score": OutputField( + type="number", + minimum=0, + maximum=10, + ) + }, + ) + + pydantic_agent = build_agent( + agent_def, + system_prompt="sys", + rendered_prompt="p", + default_model="claude-sonnet-5", + api_key="sk-test-dummy", + ) + + schema = _extract_output_tool_schema(pydantic_agent) + assert schema is not None + score_schema = schema["properties"]["score"] + # NumberType is ``int | float``, so the non-nullable schema keeps pydantic's + # ``anyOf: [integer, number]`` shape rather than a single ``type``. + assert "anyOf" in score_schema + assert score_schema["minimum"] == 0 + assert score_schema["maximum"] == 10 + _assert_no_keys(schema, "ge", "le", "default") diff --git a/tests/test_providers/test_pydantic_ai_converters.py b/tests/test_providers/test_pydantic_ai_converters.py index ab9fc846..84b96099 100644 --- a/tests/test_providers/test_pydantic_ai_converters.py +++ b/tests/test_providers/test_pydantic_ai_converters.py @@ -9,6 +9,8 @@ from __future__ import annotations +import time + import pytest from pydantic import BaseModel from pydantic import ValidationError as PydanticValidationError @@ -160,6 +162,47 @@ def test_all_scalar_types(self) -> None: with pytest.raises(PydanticValidationError): model(name="Alice", count=True, active=True) + def test_nullable_object_array_item_accepted(self) -> None: + """Nullable object and array items must be accepted by the dynamic + model, matching ``validate_output`` behaviour.""" + model = output_schema_to_pydantic_model( + "NullableItems", + { + "rows": OutputField( + type="array", + items=OutputField( + type="object", + nullable=True, + properties={"name": OutputField(type="string")}, + ), + ) + }, + ) + assert model is not None + instance = model(rows=[None, {"name": "x"}]) + assert instance.rows[0] is None + assert instance.rows[1].name == "x" + + def test_nullable_nested_array_item_accepted(self) -> None: + """A nullable array item that is itself an array must accept ``None``.""" + model = output_schema_to_pydantic_model( + "NullableNestedArrays", + { + "grid": OutputField( + type="array", + items=OutputField( + type="array", + nullable=True, + items=OutputField(type="string"), + ), + ) + }, + ) + assert model is not None + instance = model(grid=[None, ["a"]]) + assert instance.grid[0] is None + assert instance.grid[1] == ["a"] + def test_missing_field_raises(self) -> None: """All output fields are required by default; missing fields must raise.""" model = output_schema_to_pydantic_model( @@ -476,3 +519,64 @@ def test_valid_content_passes_both(self) -> None: model(**content) validate_output(content, schema) + + +class TestPatternTimeout: + """Regression tests for pattern matching with catastrophic-backtracking protection.""" + + def test_pattern_match_respects_timeout(self) -> None: + """A pathological regex input must raise ``ValidationError`` quickly + instead of hanging the event loop.""" + model = output_schema_to_pydantic_model( + "Patterned", + { + "value": OutputField( + type="string", + pattern=r"^(a|aa)+$", + ) + }, + ) + assert model is not None + + start = time.monotonic() + with pytest.raises(PydanticValidationError): + model(value="a" * 60 + "b") + elapsed = time.monotonic() - start + assert elapsed < 5.0 + + +class TestNullableEnum: + """Tests for nullable enum advertisement and acceptance.""" + + def test_nullable_enum_includes_none_in_json_schema_extra(self) -> None: + """A nullable string enum must advertise ``None`` in the JSON Schema + ``enum`` so the model sees the same permitted values as the validator.""" + model = output_schema_to_pydantic_model( + "NullableEnum", + { + "status": OutputField( + type="string", + enum=["A", "B"], + nullable=True, + ) + }, + ) + assert model is not None + field_info = model.model_fields["status"] + assert field_info.json_schema_extra == {"enum": ["A", "B", None]} + + def test_nullable_enum_accepts_none(self) -> None: + """A nullable enum field must validate ``None`` at runtime.""" + model = output_schema_to_pydantic_model( + "NullableEnum", + { + "status": OutputField( + type="string", + enum=["A", "B"], + nullable=True, + ) + }, + ) + assert model is not None + instance = model(status=None) + assert instance.status is None diff --git a/uv.lock b/uv.lock index 885b754a..ea5795c7 100644 --- a/uv.lock +++ b/uv.lock @@ -476,6 +476,7 @@ dependencies = [ { name = "packaging" }, { name = "pydantic" }, { name = "pydantic-ai" }, + { name = "regex" }, { name = "rich" }, { name = "ruamel-yaml" }, { name = "simpleeval" }, @@ -516,6 +517,7 @@ requires-dist = [ { name = "packaging", specifier = ">=21.0" }, { name = "pydantic", specifier = ">=2.0.0" }, { name = "pydantic-ai", specifier = ">=1.44.0" }, + { name = "regex", specifier = ">=2024.11.6" }, { name = "rich", specifier = ">=13.0.0" }, { name = "ruamel-yaml", specifier = ">=0.18.0" }, { name = "simpleeval", specifier = ">=1.0.0" }, @@ -2460,8 +2462,8 @@ name = "secretstorage" version = "3.5.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "cryptography" }, - { name = "jeepney" }, + { name = "cryptography", marker = "sys_platform != 'win32'" }, + { name = "jeepney", marker = "sys_platform != 'win32'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/1c/03/e834bcd866f2f8a49a85eaff47340affa3bfa391ee9912a952a1faa68c7b/secretstorage-3.5.0.tar.gz", hash = "sha256:f04b8e4689cbce351744d5537bf6b1329c6fc68f91fa666f60a380edddcd11be", size = 19884, upload-time = "2025-11-23T19:02:53.191Z" } wheels = [