Fix Encodable for Dataclass and Scalar in #548 by datvo06 · Pull Request #571 · BasisResearch/effectful

datvo06 · 2026-02-16T02:57:48Z

This PR:

Add encodable for scalar and dataclass so that we won't break the test for these classes.
Update fixtures.
Minor fix on tool call encoding to use tool signatures.

datvo06 · 2026-02-16T03:09:04Z

I found some failure tests. Checking more before re-opening

eb8680 · 2026-02-16T03:19:25Z

The problem is the traces being replayed, which use the old response_model wrapper and are now obsolete. They need to be regenerated under the changes in #548, after which this PR is unnecessary.

datvo06 · 2026-02-16T03:35:34Z

The problem is the traces being replayed, which use the old response_model wrapper and are now obsolete. They need to be regenerated under the changes in #548, after which this PR is unnecessary.

Yes. Fixtures are part of it, but also these following cases now fail, which I think because of this comment #548 (comment) (we dropped the default response_model, but did not add encodables to handle dataclass or primitives):

@Template.define
def primes(first_digit: int) -> int:
    """Give a prime number with {first_digit} as the first digit. Do not use any tools."""
    raise NotHandled


with handler(provider):
    assert type(primes(6)) is int

Result:

APIConnectionError: litellm.APIConnectionError: APIConnectionError: OpenAIException - Unsupported response_format type - <class 'int'>

And this:

@dataclasses.dataclass
class KnockKnockJoke:
    whos_there: str
    punchline: str


@Template.define
def write_joke(theme: str) -> KnockKnockJoke:
    """Write a knock-knock joke on the theme of {theme}. Do not use any tools."""
    raise NotHandled


@Template.define
def rate_joke(joke: KnockKnockJoke) -> bool:
    """Decide if {joke} is funny or not. Do not use any tools."""
    raise NotHandled


def do_comedy():
    joke = write_joke("lizards")
    print("> You are onstage at a comedy club. You tell the following joke:")
    print(
        f"Knock knock.\nWho's there?\n{joke.whos_there}.\n{joke.whos_there} who?\n{joke.punchline}"
    )
    if rate_joke(joke):
        print("> The crowd laughs politely.")
    else:
        print("> The crowd stares in stony silence.")


with handler(provider):
    do_comedy()

Result:

APIConnectionError: litellm.APIConnectionError: APIConnectionError: OpenAIException - Unsupported response_format type - <class '__main__.KnockKnockJoke'>

eb8680 · 2026-02-16T03:44:13Z

The solution is not to handle scalars or dataclasses or other types that can theoretically be coerced into Pydantic piecemeal, but to do the encoding generically in the base case of Encodable.

datvo06 · 2026-02-16T04:13:09Z

The solution is not to handle scalars or dataclasses or other types that can theoretically be coerced into Pydantic piecemeal, but to do the encoding generically in the base case of Encodable.

I see, I think we can make encoding more generic is adding WrappedEncodable. I added the first pass but will think of what can be better.

kiranandcode · 2026-02-17T13:44:23Z

effectful/handlers/llm/encoding.py



+@functools.cache
+def _wrapped_response_model(ty: Hashable) -> type[pydantic.BaseModel]:


name of this function could be made a little clearer - it creates a pydantic model where value is T, ..., from the call sites I guess it's only used inside _encodable_object? maybe it could be inlined? it feels a bit bespoke.

Yeah, I tried inlining it, but functools.cache doesnt work very well with nested function. Maybe after all suggested changes below, this would be no longer needed.

kiranandcode · 2026-02-17T13:45:34Z

effectful/handlers/llm/encoding.py

+        Encodable[T, U],
+        WrappedEncodable(
+            ty,
+            typing.cast(type[pydantic.BaseModel], model_cls),


is this cast here needed? _wrapped_response_model aloready returns a type[pydantic.BaseModel]

Oh right! Removing

kiranandcode · 2026-02-17T13:46:55Z

effectful/handlers/llm/encoding.py

+        encoded_args = _param_model(sig).model_validate(
            {
-                k: Encodable.define(type(v), self.ctx).encode(v)
+                k: Encodable.define(sig.parameters[k].annotation, self.ctx).encode(v)


wouldn't nested_type(v) handle this? The runtime types should be more precise than any annotation on the signature.

kiranandcode · 2026-02-17T13:48:58Z

effectful/handlers/llm/encoding.py

        __config__={"extra": "forbid"},
        **{
-            name: Encodable.define(param.annotation).enc
+            name: _embedded_type_from_enc(Encodable.define(param.annotation))


this feels a little ad-hoc. Can't this _embedded_type_from_enc be folded into the respective smart constructors of Wrapped, Tuple and MutableSequence-Encodable?

Thanks, I wanted to make the change centralized to here, but adding it to constructor is better architecturally.

kiranandcode · 2026-02-17T13:50:00Z

effectful/handlers/llm/encoding.py

+        return typing.cast(typing.Any, self.adapter.validate_python(value))
+
+    def decode(self, encoded_value: typing.Any) -> T:
+        if isinstance(encoded_value, pydantic.BaseModel):


shouldn't the invariant be that the input to encode should always be the enc type such that these kinds of isinstance checks can be avoided?

Thanks! The invariant is broken after I started using embedded/wrapped type to fix scalar error.
I'm reverting that now and thinking if I can better fix it somehow.

Yeah I remember the need for embedded_type now, we want to have formatting for template being different from pydandic base model (int 3 should be just 3 instead of {"value": 3}).

hmmm, maybe the encodable for ints should be simpler? or that logic of unwrapping the value should be encapsulated inside the formatting section (i.e if the return value is a dict with a single field "value", then just inject the value directly?)

Yeah, but I feel it's kind of adhoc/unsure how sound or complete we need this to be.

datvo06 · 2026-02-17T16:00:47Z

Temporary rebase to master to see if tests work.

eb8680 · 2026-02-19T20:05:56Z

effectful/handlers/llm/encoding.py



+@dataclass
+class MappingEncodable[K, V](Encodable[Mapping[K, V], typing.Any]):


MappingEncodable doesn't seem to be doing anything different from BaseEncodable, why do we need it?

eb8680 · 2026-02-19T20:07:39Z

effectful/handlers/llm/encoding.py

+
+    @staticmethod
+    @functools.cache
+    def wrapped_model(ty: Hashable) -> type[_BoxEncoding[Any]]:


Can this just be inlined into _encodable_object? Where else do you need to call it?

Still unaddressed

effectful/handlers/llm/encoding.py

eb8680 · 2026-02-19T20:10:51Z

effectful/handlers/llm/encoding.py

+
+    return typing.cast(
+        Encodable[Mapping[K, V], U],
+        MappingEncodable(ty, ty, ctx),


Why is ty being passed as both base and enc here? I would expect enc to be a BaseModel. Is this code path being exercised at all in the tests?

eb8680 · 2026-02-19T20:11:09Z

tests/test_handlers_llm_encoding.py

 _TOOL_IDS = frozenset(
    {"tool-add", "tool-greet", "tool-process", "tool-no-params", "tool-pydantic-param"}
 )
+_DTC_IDS = frozenset(


This appears to be unused now

* Adding tests * Lint * More lint * Minor * Removing helper functions * Check deserialize invariant * Minor fix for typeddict * Better pydantic-based check * Lint * Test ToolSchema * Inline completion with tools * More inlining * reimport requires_openai * Removing casts * mark xfail some tests agian * Merging integration test into encoding

eb8680 · 2026-02-20T21:46:57Z

effectful/handlers/llm/encoding.py

+) -> Encodable[Mapping[K, V], U]:
+    ctx = {} if ctx is None else ctx
+
+    enc: type[Any] = ty


Not sure what happened here in response to my previous comment on this function. Originally it was wrong because ty is not a BaseModel:

return MappingEncodable(ty, enc=ty, ctx=ctx) # ty is not a BaseModel

Now it's still wrong for the same reason, just with an extra layer of indirection:

totally_not_wrong = ty # still not a BaseModel return MappingEncodable(ty, enc=totally_not_wrong, ctx=ctx)

eb8680 · 2026-02-20T21:55:08Z

effectful/handlers/llm/encoding.py

+
+    @staticmethod
+    @functools.cache
+    def _typeddict_model(td: type[Any]) -> type[pydantic.BaseModel]:


It seems like this is the only part of MappingEncodable that is different from BaseEncodable. Maybe just have a separate TypedDictEncodable and get rid of MappingEncodable?

TypedDictEncodable still doesn't seem any different from BaseEncodable, i.e. BaseEncodable(ty, enc, ctx) is indistinguishable from TypedDictEncodable(ty, enc, ctx) for the same enc. If the only difference is the encoding, you can just move this logic into _encodable_mapping below and get rid of TypedDictEncodable.

Still unaddressed

datvo06 · 2026-02-20T23:21:56Z

Previously list-image and other tool call was failing in indempotency due to:
(1) our encodable types being OpenAIMessage, which are TypedDict, but not pydantic model
(2) calling nested type on these value return MutableMapping or Mapping or dict, but not typed dict.
(3) Our previous Image and ToolCall encoding was also returning just dictionary, not typed dict
(4) So encoding twices give Response_MutableMapping type instead of a typed dict pydantic base model.
The new fix uses TypedDict pydantic model for constructing enc for both ImageEncodable and ToolCallEncodable.

Demonstration of (2):

from typing import TypedDict
from effectful.internals.unification import nested_type

class UserTD(TypedDict):
    name: str
    age: int

print("nested_type(UserTD(name='a', age=1)).value ->", nested_type(UserTD(name='a', age=1)).value)

Result:

nested_type(UserTD(name='a', age=1)).value -> <class 'dict'>

eb8680 · 2026-02-20T23:44:36Z

our encodable types being OpenAIMessage, which are TypedDict, but not pydantic model

The arbitrary decisions about which types are BaseModels and which are only TypedDicts (which don't work with isinstance/issubclass, among other things) is one of the many annoying inconsistencies inside the litellm API. I've been thinking it might make sense for us to coerce important litellm-internal TypedDicts like ChatCompletionToolParam into compatible Pydantic models, kind of like what litellm already does with Message.

calling nested type on these value return MutableMapping or Mapping, but not typed dict.

I think this can be regarded as a bug (or at least missing feature) in nested_type and fixed in another PR.

eb8680 · 2026-02-20T23:52:21Z

effectful/handlers/llm/encoding.py

+
+
+@dataclass
+class _ComplexEncodable(Encodable[complex, _ComplexParts]):


Is complex not compatible with BaseEncodable and _BoxEncoding? When I tried making a BaseModel with a complex-valued field in the REPL and validating/serializing data, it seemed to work fine.

eb8680 · 2026-02-20T23:57:21Z

effectful/handlers/llm/encoding.py

+        identity_encoder = typing.cast(
+            Encodable[T, typing.Any],
+            BaseEncodable(
+                typing.cast(type[T], object),
+                typing.cast(
+                    type[_BoxEncoding[T]],
+                    BaseEncodable.wrapped_model(typing.cast(Hashable, object)),


All the casts here are quite confusing/misleading (why Hashable?), just use # type: ignore.

eb8680 · 2026-02-21T00:03:27Z

effectful/handlers/llm/encoding.py

+        self, encoded_value: pydantic.BaseModel | Mapping[str, Any]
+    ) -> Image.Image:
+        normalized = self.enc.model_validate(encoded_value)
+        image_url = typing.cast(str, getattr(normalized, "url"))


If this cast is saying image_url is not None, prefer an assertion about that (assert normalized.url is not None) - cast is just a lie to the type checker. In general you should only be using cast to coerce to types that are unavailable at runtime, like generics, TypeVars or TypedDict.

eb8680 · 2026-02-21T00:05:36Z

effectful/handlers/llm/encoding.py

+
+    @staticmethod
+    @functools.cache
+    def wrapped_model(ty: Hashable) -> type[_BoxEncoding[Any]]:


Still unaddressed

eb8680 · 2026-02-21T00:05:55Z

effectful/handlers/llm/encoding.py

+
+    @staticmethod
+    @functools.cache
+    def _typeddict_model(td: type[Any]) -> type[pydantic.BaseModel]:


Still unaddressed

* Move logic from completion to encoding * errors * attempt to appease mypy * nit * import * nit * fix * nit * simplify and add tests * update tests * tests * more consolidation * stash * refactor tests * Fix Encodable for Dataclass and Scalar in #548 (#571) * Adding scalar encoding, make tools use signature and update fixtures * Lint * Lint * More general wrappedencodable * Lint * Architectural fixes * Removing embedded_type * Lint * Minor * Minor fix * Lint * Add param_schema_type * Minor * New Encodable's architecture * Recursive dataclass encodable * Linting * Simplify encodable * Minor * Fix test * Lint * Trim * More trimming * Handling images * Lint * Further trimming * Updating test template formatting * Minor * Add MappingEncodable and cover more tests * Lint * More precise BaseEncodable * Lint * Removing adapter from base * Move wrapped model to a staticmethod * More concise handling of empty lists * Minor * Use nested_type as _wrapped_model is resolved * Remove DTC_IDS * Minor fix * Special typeddict handling * Complex Encodable * Removing casts * Add Encoding/Integration Parameterized Tests (#582) * Adding tests * Lint * More lint * Minor * Removing helper functions * Check deserialize invariant * Minor fix for typeddict * Better pydantic-based check * Lint * Test ToolSchema * Inline completion with tools * More inlining * reimport requires_openai * Removing casts * mark xfail some tests agian * Merging integration test into encoding * Enforce Tool and Image to be PydanticBase model * Update fixtures * Update minor fixture --------- Co-authored-by: Dat Nguyen (Marc) <15943389+datvo06@users.noreply.github.com>

datvo06 added 3 commits February 15, 2026 21:52

Adding scalar encoding, make tools use signature and update fixtures

26ff31b

Lint

67d9e2c

Lint

415aa6e

datvo06 changed the title ~~Fix Encodable for Dataclass and Scalar in #568~~ Fix Encodable for Dataclass and Scalar in #548 Feb 16, 2026

datvo06 requested a review from eb8680 February 16, 2026 03:01

datvo06 marked this pull request as draft February 16, 2026 03:08

datvo06 added 2 commits February 15, 2026 23:11

More general wrappedencodable

e5cd229

Lint

59f8c06

kiranandcode reviewed Feb 17, 2026

View reviewed changes

datvo06 added 2 commits February 17, 2026 10:50

Architectural fixes

c2a8312

Removing embedded_type

e5d90ac

datvo06 changed the base branch from eb-remove-response to master February 17, 2026 15:59

datvo06 marked this pull request as ready for review February 17, 2026 16:00

datvo06 added 7 commits February 17, 2026 11:01

Lint

a8d9a8a

Minor

0ac1875

Minor fix

8755647

Lint

1ace406

Add param_schema_type

e78acce

Minor

bd0bbd7

New Encodable's architecture

09654d8

datvo06 added 8 commits February 19, 2026 12:09

Lint

952805e

More precise BaseEncodable

6d1c43d

Lint

c6e1eba

Removing adapter from base

73c6dbc

Move wrapped model to a staticmethod

c4a1600

More concise handling of empty lists

dc367ae

Minor

4729074

Use nested_type as _wrapped_model is resolved

de2fe7c

datvo06 requested a review from eb8680 February 19, 2026 19:29

eb8680 reviewed Feb 19, 2026

View reviewed changes

datvo06 added 2 commits February 19, 2026 15:44

Remove DTC_IDS

80818e1

Minor fix

197d412

eb8680 added the module:llm label Feb 19, 2026

datvo06 and others added 4 commits February 19, 2026 16:58

Special typeddict handling

3f5182f

Complex Encodable

6466ea8

Removing casts

545ec44

eb8680 reviewed Feb 20, 2026

View reviewed changes

datvo06 added 2 commits February 20, 2026 18:04

Enforce Tool and Image to be PydanticBase model

3633cd4

Update fixtures

f34635d

datvo06 requested a review from eb8680 February 20, 2026 23:19

Update minor fixture

61a0726

eb8680 reviewed Feb 21, 2026

View reviewed changes

eb8680 merged commit e036596 into eb-remove-response Feb 21, 2026

eb8680 deleted the dn-fix-encodable-eb-remove-response branch February 21, 2026 18:21

datvo06 mentioned this pull request Feb 22, 2026

Move tool/tool call encoding logic from completion to encoding #548

Merged

eb8680 mentioned this pull request Feb 24, 2026

CI build times are getting too long again #587

Open



		@functools.cache
		def _wrapped_response_model(ty: Hashable) -> type[pydantic.BaseModel]:



		@dataclass
		class MappingEncodable[K, V](Encodable[Mapping[K, V], typing.Any]):



		@dataclass
		class _ComplexEncodable(Encodable[complex, _ComplexParts]):

Conversation

datvo06 commented Feb 16, 2026

Uh oh!

datvo06 commented Feb 16, 2026

Uh oh!

eb8680 commented Feb 16, 2026

Uh oh!

datvo06 commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eb8680 commented Feb 16, 2026

Uh oh!

datvo06 commented Feb 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datvo06 Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datvo06 commented Feb 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datvo06 commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eb8680 commented Feb 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

datvo06 commented Feb 16, 2026 •

edited

Loading

datvo06 Feb 17, 2026 •

edited

Loading

datvo06 commented Feb 20, 2026 •

edited

Loading