diff --git a/effectful/handlers/llm/harness/serialization.py b/effectful/handlers/llm/harness/serialization.py index c78c97bfc..3168f7bc4 100644 --- a/effectful/handlers/llm/harness/serialization.py +++ b/effectful/handlers/llm/harness/serialization.py @@ -434,23 +434,6 @@ def _pydantic_type_str[T](ty: type[T]) -> type[T]: return ty -@dataclasses.dataclass(frozen=True) -class _NoEncoding: - """Refuses a *validation* JSON schema for a type that has no encoding.""" - - ty: typing_extensions.TypeForm - - def __get_pydantic_json_schema__(self, schema, handler): - if handler.mode == "validation": - raise pydantic.errors.PydanticInvalidForJsonSchema( - f"`{inspect.formatannotation(self.ty)}` has no `Encodable` encoding, so a " - f"value of it can be sent to the model but not decoded from the " - f"model's output. Register one with `TypeToPydanticType.register` " - f"if the model needs to produce these." - ) - return handler(schema) - - def _serialize_unencodable(value: typing.Any) -> str: """Render a value whose type has no encoding, as text. @@ -475,16 +458,28 @@ def _pydantic_type_base(ty: typing.Any) -> typing.Any: pydantic.TypeAdapter(ty) return ty except pydantic.errors.PydanticSchemaGenerationError: + name = inspect.formatannotation(ty) return typing.Annotated[ ty, pydantic.InstanceOf, pydantic.PlainSerializer( _serialize_unencodable, - return_type=typing.Annotated[ - str, pydantic.Field(description=inspect.formatannotation(ty)) + return_type=typing.Annotated[str, pydantic.Field(description=name)], + ), + pydantic.BeforeValidator( + lambda value: value, + json_schema_input_type=typing.Annotated[ + str, + pydantic.Field( + title=_UndecodableReturn.__schema_title__, + description=( + f"No decoding exists for `{name}`, so a direct reply of " + f"it cannot be decoded. Do not answer directly: call a " + f"tool that produces a final answer instead." + ), + ), ], ), - _NoEncoding(ty), ] @@ -499,6 +494,22 @@ def _best_effort_schema( return {"description": inspect.formatannotation(annotation)} +def _is_decodable(annotation: typing_extensions.TypeForm) -> bool: + """Whether the model can be asked to produce a value of ``annotation``.""" + + def refuses(node: typing.Any) -> bool: + if isinstance(node, dict): + return node.get("title") == _UndecodableReturn.__schema_title__ or any( + refuses(v) for v in node.values() + ) + return isinstance(node, list) and any(refuses(v) for v in node) + + try: + return not refuses(pydantic.TypeAdapter(Encodable[annotation]).json_schema()) # type: ignore + except Exception: + return False + + @TypeToPydanticType.register(type) @TypeToPydanticType.register(abc.ABCMeta) @TypeToPydanticType.register(GenericAlias) @@ -529,6 +540,8 @@ class _UndecodableReturn: reply cannot be decoded soundly. Do not answer directly: call a tool that produces a final answer instead.""" + __schema_title__: typing.ClassVar[typing.Literal["$UNDECODABLE"]] = "$UNDECODABLE" + def _fail_validation(value: typing.Any) -> typing.Any: raise ValueError(inspect.getdoc(_UndecodableReturn)) @@ -539,7 +552,10 @@ def _pydantic_type_undecodable_return(ty: type[_UndecodableReturn]) -> typing.An return typing.Annotated[ str, pydantic.PlainValidator(_fail_validation, json_schema_input_type=str), - pydantic.Field(description=inspect.getdoc(_UndecodableReturn)), + pydantic.Field( + title=_UndecodableReturn.__schema_title__, + description=inspect.getdoc(_UndecodableReturn), + ), ] @@ -815,9 +831,16 @@ def _tool_description(tool: Tool, *, param_schemas: bool = False) -> str: def _serialize_name_and_tool(value: _NameAndTool) -> ChatCompletionToolParam: name, tool = value + params = inspect.signature(tool).parameters + for param_name, param in params.items(): + if not _is_decodable(param.annotation): + raise pydantic.errors.PydanticSchemaGenerationError( + f"`{name}` cannot be advertised as JSON: no value of parameter " + f"`{param_name}` could be decoded from the model's output" + ) fields: dict[str, typing.Any] = { param_name: TypeToPydanticType().evaluate(param.annotation) - for param_name, param in inspect.signature(tool).parameters.items() + for param_name, param in params.items() } sig_model = pydantic.create_model( "Params", diff --git a/tests/test_handlers_llm_harness_serialization.py b/tests/test_handlers_llm_harness_serialization.py index 4297bddfa..940bd5f82 100644 --- a/tests/test_handlers_llm_harness_serialization.py +++ b/tests/test_handlers_llm_harness_serialization.py @@ -38,7 +38,10 @@ _TYPE_CHECK_ANCHOR_KEY, CONTENT_BLOCK_TYPES, DecodedToolCall, + _BoxedResponse, + _is_decodable, _NameAndTool, + _UndecodableReturn, to_content_blocks, ) from effectful.handlers.llm.harness.validation.ty import TyTypeChecker @@ -701,8 +704,8 @@ def test_metadata_does_not_make_an_unencodable_type_encodable(): """Orthogonality in the other direction: metadata is not a way in. A type the registry cannot encode is equally undecodable annotated, and - fails the same way -- so attaching a contract never turns a clear schema - error into something subtler. + refuses the same way -- so attaching a contract never turns a refusal into + a schema that promises the type. Stated on the *validation* schema rather than on building the adapter, since an unencodable type is still serializable (see @@ -715,10 +718,9 @@ class Widget: marker = pydantic.AfterValidator(lambda v: v) for ty in (Widget, Annotated[Widget, marker]): - with pytest.raises( - pydantic.errors.PydanticInvalidForJsonSchema, match="no `Encodable`" - ): - pydantic.TypeAdapter(Encodable[ty]).json_schema() + schema = pydantic.TypeAdapter(Encodable[ty]).json_schema() + assert schema["type"] == "string" + assert "No decoding exists" in schema["description"] # ============================================================================ @@ -1128,18 +1130,100 @@ def test_unencodable_type_serializes_but_does_not_decode(): degradation -- worst case the model reads a `repr` -- and it happens on paths that never asked the model for anything: a tool result, a value spliced into a prompt, a trace. Decoding is not, because nothing rebuilds - an arbitrary object from that text, so the validation schema refuses - instead of promising a string. + an arbitrary object from that text, so the validation schema asks for a + string that nothing satisfies rather than one that would decode. """ adapter = pydantic.TypeAdapter(Encodable[_WidgetWithRepr]) assert ( adapter.dump_python(_WidgetWithRepr(1), mode="json") == "_WidgetWithRepr(n=1)" ) assert adapter.json_schema(mode="serialization")["type"] == "string" - with pytest.raises( - pydantic.errors.PydanticInvalidForJsonSchema, match="_WidgetWithRepr" - ): - adapter.json_schema() + + validation = adapter.json_schema() + assert validation["type"] == "string" + assert "_WidgetWithRepr" in validation["description"] + with pytest.raises(pydantic.ValidationError): + adapter.validate_python("_WidgetWithRepr(n=1)") + + +@pytest.mark.parametrize( + "ty", + [_WidgetWithRepr, list[_WidgetWithRepr], tuple[int, _WidgetWithRepr]], + ids=["bare", "nested", "in-tuple"], +) +def test_unencodable_response_format_reaches_the_provider(ty): + """Refusing with a schema rather than an exception is what keeps a response + format buildable, which is the obligation `Encodable` carries: a `Skill` + returning such a type answers by calling a final-answer tool, and never gets + to if assembling the request cannot be done at all. + + However deep the unencodable type sits, the refusal is emitted at that leaf + and names it, leaving every other part its real schema. + """ + box = pydantic.create_model( + "BoxedResponse", value=Encodable[ty], __base__=_BoxedResponse + ) + schema = litellm.utils.type_to_response_format_param(box) + assert "_WidgetWithRepr" in json.dumps(schema) + + +@pytest.mark.parametrize("ty", [_UndecodableReturn, _WidgetWithRepr], ids=str) +def test_refusals_announce_themselves_on_the_wire(ty): + """Both refusing schemas carry the title `_is_decodable` recognizes them by + -- the one for a return type never instantiated, and the one for a type with + no encoding -- and strict-mode post-processing leaves it alone.""" + box = pydantic.create_model( + "BoxedResponse", value=Encodable[ty], __base__=_BoxedResponse + ) + schema = litellm.utils.type_to_response_format_param(box) + assert _UndecodableReturn.__schema_title__ in json.dumps(schema) + + +@pytest.mark.parametrize( + "ty,expected", + [ + (int, True), + (Image.Image, True), + (dict[str, int], True), + (_WidgetWithRepr, False), + (list[_WidgetWithRepr], False), + (tuple[int, _WidgetWithRepr], False), + (Operation, False), + ], + ids=str, +) +def test_is_decodable(ty, expected): + """Whether the model can be *asked* for a value, as opposed to shown one. + + False covers both ways a type can fail to name something the model could + send: a schema that refuses however deeply it sits, and no schema at all. + """ + assert _is_decodable(ty) is expected + + +def test_is_decodable_looks_past_an_encoding_that_replaces_its_arguments(): + """A refusal only counts where the model would actually meet it. + + `SkillBody` is asked for as source and decoded by compiling it, so its own + schema stands in for its arguments' -- which is what lets a Skill returning + an undecodable type still be answered by synthesizing one, the whole point + of refusing a direct reply. Reading the type rather than the schema it + generates gets this backwards and withdraws the tool that was the way out. + """ + from effectful.handlers.llm.harness.synthesis.body import SkillBody + + assert not _is_decodable(_WidgetWithRepr) + assert _is_decodable(SkillBody[[int], _WidgetWithRepr]) + + +def test_unencodable_element_does_not_block_encoding_its_container(): + """A container is sendable when its parts are. One part having no decoding + does not change that: the refusal is that element's, in one direction.""" + adapter = pydantic.TypeAdapter(Encodable[tuple[int, _WidgetWithRepr]]) + assert adapter.dump_python((7, _WidgetWithRepr(1)), mode="json", context={}) == { + "item_0": 7, + "item_1": "_WidgetWithRepr(n=1)", + } def test_unencodable_value_rendering_is_stable_across_runs(): diff --git a/tests/test_handlers_llm_harness_toolcall.py b/tests/test_handlers_llm_harness_toolcall.py index dbbbdf191..ea9dc3957 100644 --- a/tests/test_handlers_llm_harness_toolcall.py +++ b/tests/test_handlers_llm_harness_toolcall.py @@ -96,7 +96,7 @@ def dbl(x: int) -> int: from dataclasses import dataclass from effectful.handlers.llm import Agent, Skill, Tool -from effectful.ops.types import NotHandled, Operation +from effectful.ops.types import Interpretation, NotHandled, Operation calls = [] @@ -126,6 +126,12 @@ def op_tool(op: Operation) -> str: return op.__name__ +@Tool.define +def interp_tool(i: Interpretation) -> str: + """A tool whose parameter type advertises but cannot be decoded.""" + return str(len(i)) + + @Tool.define def vsum(*xs: int) -> int: """Sum any number of integers.""" @@ -600,7 +606,8 @@ def test_505_generic_tool_in_scope_does_not_break_unrelated_skill(poly_mod): assert result == "just text" -def test_json_mode_skips_unadvertisable_tool(poly_mod, caplog): +@pytest.mark.parametrize("skipped", ["op_tool", "interp_tool"]) +def test_json_mode_skips_unadvertisable_tool(poly_mod, caplog, skipped): # Under the JSON pathway a tool whose advertisement cannot be encoded is # skipped (with a warning) instead of breaking every request it is merely # in scope for. The skip happens at encoding time, in `call_assistant`'s @@ -610,6 +617,10 @@ def test_json_mode_skips_unadvertisable_tool(poly_mod, caplog): # *generic* tool still advertises there, but degraded to untyped `{}` # parameter schemas -- the #489 decode ambiguity the expression pathway # exists to fix.) + # + # Two ways a parameter can fail to describe a value the model could send: + # `op_tool`'s has no schema at all, and `interp_tool`'s has one that no + # reply satisfies. Both make the tool uncallable, so both are skipped. advertised: list[list] = [] class _SpecCapture(ObjectInterpretation): @@ -630,9 +641,9 @@ def _c(self, *args, **kwargs): assert poly_mod.grow([1, 2, 3]) == "just text" names = {spec["function"]["name"] for specs in advertised for spec in specs} assert "other_tool" in names and "extend_sequence" in names - assert "op_tool" not in names + assert skipped not in names assert any( - "op_tool" in record.message and record.levelname == "WARNING" + skipped in record.message and record.levelname == "WARNING" for record in caplog.records ) @@ -1163,6 +1174,65 @@ def test_generic_skill_without_binding_redirects_to_code_mode(generic_mod): assert result == [generic_mod.Item(text="be kind")] +# ============================================================================ +# Return types with no `Encodable` decoding at all +# ============================================================================ + +_UNENCODABLE_SKILL_SRC = ''' +from effectful.handlers.llm import Skill + + +class Widget: + """A type the encoding registry knows nothing about.""" + + def __init__(self, n: int) -> None: + self.n = n + + def __repr__(self) -> str: + return f"Widget({self.n})" + + +@Skill.define +def make_widget(n: int) -> Widget: + """Build a widget holding {n}.""" +''' + + +@pytest.fixture +def widget_mod(tmp_path, request): + modname = f"_widget_fixture_{request.node.name}".replace("[", "_").replace("]", "") + mod = _import_fixture(tmp_path, _UNENCODABLE_SKILL_SRC, modname) + yield mod + sys.modules.pop(modname, None) + + +def test_unencodable_return_redirects_to_code_mode(widget_mod): + # A return type with no decoding is answerable by the route an uninstantiated + # one takes: the response format asks for a string nothing satisfies, so the + # plausible-looking reply below is refused and the feedback steers the model + # to `write_and_run_body`, whose result is the answer. + from effectful.handlers.llm.harness.durability.retrying import TenacityRetryer + from effectful.handlers.llm.harness.synthesis.body import FinalBodySynthesizer + + result = _run_generic( + widget_mod, + lambda: widget_mod.make_widget(3), + [ + make_text_response(json.dumps({"value": "Widget(3)"})), + make_tool_call_response( + "write_and_run_body", + json.dumps( + {"implementation": "def make_widget(n):\n return Widget(n)\n"} + ), + ), + ], + FinalBodySynthesizer(), + TenacityRetryer(), + ) + assert isinstance(result, widget_mod.Widget) + assert result.n == 3 + + # ============================================================================ # Live model # ============================================================================