openai-python: Fix typing errors from mypy when calling completion

tuples: list[tuple[Literal["user", "assistant", "system"], str]] = []
completion = await client.chat.completions.create(
    messages=[{"role": r, "content": c} for r, c in tuples],
    model="foobar",
)

This is a pretty compelling example and I am surprised that Pyright doesn’t infer the right type here.

It seems like it works when the first entry in the tuple is typed as just Literal['user'] but breaks down when it’s expanded to include other values, e.g. Literal['user', 'system'].

I’d appreciate it if someone here could open an issue / discussion on the Pyright repository with a minimal reproduction of this case to see if this behaviour could be improved on their end, I won’t have time to do so until later today.

So, first of all, I want to check if you are defending the current implementation and feel that it is a good design that shouldn’t be improved. If that’s not the case, then what I’m telling you is that there are issues in the design that make seemingly obvious things difficult. Blaming python doesn’t really inspire confidence in your python programmers who actually try to solve such design problems.

I do not believe the SDK is completely perfect, I am sure there are other edge cases where behaviour can be improved. We spend a lot of time trying to design the most idiomatic SDK possible so your examples here are appreciated, we were not aware of this problem.

I don’t want to be rude here, I would just like to put out there that I found your initial response to the first commenter a bit dismissive. If you are a person of authority, than you being dismissive will always stifle discussion more than someone without it being confrontational

I appreciate your candidness, can you share why you feel that my initial response was dismissive? That was not my intention. I was under the impression that the initial comment was coming from a misunderstanding of type inferring in python, not that there are real situations where typing is very cumbersome which you have pointed out.

Thanks very much for sharing your examples and input @antont, and for turning this into a productive technical conversation @nathan-chappell and @RobertCraigie, highlighting some of the shortcomings of our current approach and the reasons why it’s tricky to change. I also appreciate your kind words @nathan-chappell!

My personal takeaway here is that it’d probably be pretty handy for this library to provide constructors for message params, perhaps something loosely along these lines:

from openai.types.chat.completions import *

completion = await client.chat.completions.create(
    model="gpt-4",
    messages=[
        SystemMessage("You are a helpful robot"),
        UserMessage("Why is the sky blue? Some context will follow"),
        *(Message(role=r, content=c) for r,c in some_dict.items()),
        AssistantMessage(tool_calls=[…]),
        ToolMessage("<results>", tool_call_id="123")
    ],
)

My guess is that yes, these would be pydantic models, as suggested here. (Though Robert and I will take this offline to discuss details/internals)

What do you think of something like this?

Thank you, your change in tone is very much appreciated 😃

I don’t know if there will be a quick fix for this particular scenario.

I agree, there’s no guarantee that this is a quick fix but I have seen Eric make changes incredibly quickly so I’m holding out hope.

For what it’s worth, this snippet should work for you and while it is a little verbose, I think it’s much better than having to import lots of types or define your own helper functions while we come up with a better solution:

messages: list[ChatCompletionMessageParam] = []
for role, content in tuples:
    if role == 'user':
        messages.append({"role": 'user', 'content': content})
    elif role == 'system':
        messages.append({"role": 'system', 'content': content})

@RobertCraigie Python’s typing might be limiting to some extent, but the fact is that you guys automatically generated your client library with some tool that did a pretty good, but not perfect, job.

Let’s suppose I want to use the openai types to create a list of messages. Up until the new library I was using a definition like so:

class MessageDict(TypedDict):
    role: Literal["system", "user", "assistant"]
    content: str

It doesn’t handle every possible way to access the api, but it’s been good enough (usually if I’m doing function calling there is a lot more ceremony involved anyways).

I would like to use such a class as follows:

completion = await client.chat.completions.create(
    messages=[MessageDict(role=r, content=c) for r,c in some_dict.items()],
    ...
)

As of right now the type checkers don’t like this. Fine, how can I do it with the types you’ve provided us?

class ChatCompletionFunctionMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    name: Required[str]
    role: Required[Literal["function"]]

class ChatCompletionToolMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    role: Required[Literal["tool"]]
    tool_call_id: Required[str]

class ChatCompletionAssistantMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    role: Required[Literal["assistant"]]
    function_call: FunctionCall
    tool_calls: List[ChatCompletionMessageToolCallParam]

class ChatCompletionUserMessageParam(TypedDict, total=False):
    content: Required[Union[str, List[ChatCompletionContentPartParam], None]]
    role: Required[Literal["user"]]

class ChatCompletionSystemMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    role: Required[Literal["system"]]

# openai.types.chat

ChatCompletionMessageParam = Union[
    ChatCompletionSystemMessageParam,
    ChatCompletionUserMessageParam,
    ChatCompletionAssistantMessageParam,
    ChatCompletionToolMessageParam,
    ChatCompletionFunctionMessageParam,
]

# Since this is a union type an not a base, it can't be used to create dicts like all the other types here can.

Listen man, this is an ugly mess. It’s pretty good for automatically generated, and I’d certainly be willing to deal if it was somewhat convenient, but no man, it’s not:

• either import a bunch of long ass names everywhere (ugly and inconvenient)
• make a bunch of aliases (somewhat ugly, adds complexity)
• add #type: ignore (as ugly as it gets)

What’s funny is that the MessageDict could essentially serve as a base class to all those other TypedDict classes, but the Union type is generated, so it can’t be used for creating objects.

TLDR: whatever

The new client is a huge improvement on that old one, it almost feels like modern python. But it’s not perfect, so please don’t act like it is. Oh yea, and the documentation I would like to see is an OpenAI python-programmer using the types from the new library effectively.

If you have feedback on the above proposal, sharing it here would be great!

@RobertCraigie If you say you weren’t being dismissive then I believe you and I apologize. I’m probably a bit biased, I’ll try to remain more professional with you guys - I appreciate your response.

If I have time I’ll look into if Pyright has an intention of addressing this, but I wouldn’t be surprised if that [https://github.com/erictraut](Eric Traut) has big plans already, they are really pushing the type system (and python itself it seems) pretty hard. If I had to guess this would need to be part of some sort of principled expansion of dependent-type handling, I don’t know if there will be a quick fix for this particular scenario.

@antont I like all of the solutions you’ve proposed, and would be happy to see something like that packaged with the client. I really want the openai client to be good enough to prevent the proliferation of useless convenience tools like we’ve seen this “ai spring.”

@nathan-chappell

That’s actually what I would do, and I think completely sensible if for some reason you have the roles as strings.

tuples: list[tuple[Literal["user", "assistant", "system"], str]] = []
(...)
# I hope you aren't serious...
    messages=[
        (
            ChatCompletionUserMessageParam(role=r, content=c)
            if r == "user"
            else ChatCompletionAssistantMessageParam(role=r, content=c) if r == "assistant" else ChatCompletionSystemMessageParam(role="system", content=c)
        )
        for r, c in tuples
    ],

I’d probably use a dict to map the strings to classes. Well, actually I do have something like this in our system, just from a different role enum that we use internally.

role2message = {
  'user': ChatCompletionUserMessageParam,
  'assistant': ChatCompletionAssistantMessageParam
  'system': ChatCompletionSystemMessageParam
}

messages=[role2message[role](content=content, role=role)
          for role, content in tuples]

I don’t find this unreasonable when you have strings that define which type should be used.

@nathan-chappell

class ChatCompletionSystemMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    role: Required[Literal["system"]]

Okay, can’t put a default value in there because it’s a TypedDict

Ah, I didn’t even know that.

Maybe the message types should be Pydantic models, like the API responses already are. They can be instantiated quite nicely like ChatCompletionUserMessageParam(content=text) and have extra validation such as string lengths and number ranges etc. which I guess could be useful.

OpenAI folks want to support passing just dicts as the messages, but I guess the functions could accept both the correct Pydantic type, and a dict that’d be then used to create an instance of that type, and thus be validated at that stage. Maybe this change could be done in a backwards compatible way, I’m not sure.

@nathan-chappell Hm I don’t quite get this.

Am doing:

messages: list[ChatCompletionMessageParam] #I get this as param to a function and then add more messages to it

messages.extend([
    system_message("You are a helpful assistant."),
    user_message(question)
])

completion = await client.chat.completions.create(
    model="gpt-4",
    messages=messages
)

where am using a couple factory funcs to have the message instance creation more simple and robust

def system_message(content: str):
    return ChatCompletionSystemMessageParam(
        content=content,
        role="system"
    )

def user_message(content: str):
    return ChatCompletionUserMessageParam(
        content=content,
        role="user"
    )

And at least in VSCode (with I guess with pyright?) the type checker is happy.

Is this perhaps a limitation of mypy? I think the author mentioned that in some discussion here.

Or did I miss something? Only gave a quick read so far, and figured that might be useful to share what am doing.

This is what I refer to as make a bunch of aliases (somewhat ugly, adds complexity). See, the thing is that probably everyone would like to make these factory functions, but if that’s the case then why isn’t it just provided by the client library? Also, why would you have to provide the role parameter to the class ChatCompletionSystemMessageParam? There is one reasonable choice, and it should be filled in automatically if I’m putting System in the damn class name. But let’s look at the class definition:

class ChatCompletionSystemMessageParam(TypedDict, total=False):
    content: Required[Optional[str]]
    role: Required[Literal["system"]]

Okay, can’t put a default value in there because it’s a TypedDict. At this point as library writers we have two options: say screw it, user’s problem, or come up with a better design. I don’t know if this is some sort of impossible thing to do because the API is too overloaded, but right now the current design is not tuned towards convenience.

Your first method snippet will not work regardless of what our types are, it’s entirely dependant on how you’ve typed some_dict.

Listen man, this is an ugly mess. It’s pretty good for automatically generated, and I’d certainly be willing to deal if it was somewhat convenient, but no man, it’s not:

I’m not sure where you’re coming from here - why do you want to explicitly instantiate an object for the params?

All of this goes away if you in-line the dictionary (assuming some_dict is typed properly):

completion = await client.chat.completions.create(
    messages=[{'role': r, 'content': 'c'} for r,c in some_dict.items()],
    ...
)

What’s funny is that the MessageDict could essentially serve as a base class to all those other TypedDict classes, but the Union type is generated, so it can’t be used for creating objects.

Are you suggesting that we update the params type to just be your MessageDict type? That will just result in less type-safety, as it would have to include all of the properties from the actual message types and let you represent states that are not valid.

So even if we extracted some properties into a common base class the params type should still be a Union so that it represents the actual types that the API expects. Which then makes the MessageDict base class entirely moot because you wouldn’t be able to assign it to our union type.

Then additionally, using that as a base class is potentially unsound which is why Pyright will report an error in strict mode for this definition:

class BaseMessageParam(TypedDict):
    role: Literal["system", "user", "assistant"]
    content: str

class AssistantMessageParam(BaseMessageParam):
    role: Literal['assistant']
    # ...

# Type of TypedDict field "role" cannot be redefined
#    Type in parent class is "Literal['system', 'user', 'assistant']" and type in child class is "Literal['assistant']"

I found your comment a bit rude; some parts did not contribute to a productive discussion.

This is an unfortunate limitation of type inferring in the current state of python’s static types.

If you don’t inline the messages param you need to give it an explicit type

from openai.types.chat import ChatCompletionMessageParam

messages: list[ChatCompletionMessageParam] = [
    {
        "role": "user",
        "content": "Say this is a test",
    },
]

You don’t need to actually instantiate the type like in your example unless you really want to.