Skip to content

feat: improve schema usability type instantiation #47

@phemmer

Description

@phemmer

Problem statement

I'm just diving into using this SDK to add a compatibility layer to an agent service for ACP clients, and encountered a few initial difficulties with regards to the schema that felt could be improved.

First is that when instantiating classes that have a required field and only one possible value, that field shouldn't need to be provided, it should default. For example, when I construct AgentMessageChunk, the session_update field only has one possible value, "agent_message_chunk". It's a required field, and it cannot be anything else. So why do I have to provide it? I understand that it's used as the discriminator when deserializing, but when AgentMessageChunk is constructed explicitly, it shouldn't be needed. This is a common pattern, not exclusive to AgentMessageChunk.

However if these fields are given a default such as:

class AgentMessageChunk(ContentChunk):
    session_update: Annotated[Literal["agent_message_chunk"], Field(alias="sessionUpdate")] = "agent_message_chunk"

This then leads to another issue, in that when the SDK serializes the message, it's omitted because of exclude_defaults=True in

return params.model_dump(by_alias=True, exclude_none=True, exclude_defaults=True)

Now after going through the examples, I noticed that you provide helpers such as update_agent_message for addressing this. But IMHO these should not be necessary. When I look at a function or class, and I see that it accepts some type, I shouldn't have to go hunting through the source code to see if there's a helper that is supposed to be used for construction of that type. And while I would argue that the type shouldn't need a helper at all, and that types should be simple to instantiate directly, if there is going to be a helper, I think it would be better for discoverability to put the helper as a static/class method on the class itself. Then if you want to expose all the helpers in some top level package for ease of import, you would declare local variables which reference the static method.

Proposed solution

  • Change all types that have a required parameter with only a single possible value to default to that value.
  • Change params.model_dump to use exclude_defaults=False.
    • A little more work, but if you still want to use exclude_defaults, then implement a custom serializer that does exclude defaults, but only when the field is optional.
  • Remove the helpers.

Alternatives considered

Make the helpers easier to discover by putting them as static methods on the class they construct.

Additional context

No response

Can you help build it?

  • I can contribute code or docs for this request.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions