Enforce mypy and fix typing issues #145
Conversation
More typing
| @@ -11,7 +11,7 @@ | |||
| AgentNameMode = Literal["inline"] | |||
|
|
|||
|
|
|||
| def _is_content_blocks_content(content: list[dict] | str) -> bool: | |||
| def _is_content_blocks_content(content: list[dict | str] | str) -> TypeGuard[list[dict]]: | |||
There was a problem hiding this comment.
i don't think there should be a list of str here
There was a problem hiding this comment.
This was needed because of how BaseMessage.content is typed.
| @@ -35,12 +35,13 @@ def add_inline_agent_name(message: BaseMessage) -> BaseMessage: | |||
| return message | |||
|
|
|||
| formatted_message = message.model_copy() | |||
| if _is_content_blocks_content(formatted_message.content): | |||
| if _is_content_blocks_content(message.content): | |||
There was a problem hiding this comment.
This allows the new TypeGuard returned by _is_content_blocks_content to treat message.content as list[dict]. message.content is used/iterated within this conditional, so it requires the type guard to be treated as a list[dict].
| text_blocks = [block for block in message.content if block["type"] == "text"] | ||
| non_text_blocks = [block for block in message.content if block["type"] != "text"] | ||
| content = text_blocks[0]["text"] if text_blocks else "" | ||
| formatted_content = f"<name>{message.name}</name><content>{content}</content>" | ||
| formatted_message.content = [{"type": "text", "text": formatted_content}] + non_text_blocks | ||
| formatted_message_content = [{"type": "text", "text": formatted_content}] + non_text_blocks |
There was a problem hiding this comment.
This is a weird one. Storing it in an intermediate variable avoids this mypy error from assigning the expression directly to formatted_message.content:
Incompatible types in assignment (expression has type "list[dict[str, str]]", variable has type "str | list[str | dict[Any, Any]]")
I think the only alternative would be to explicitly cast to the expected type (which I prefer to avoid if possible).
langgraph_supervisor/agent_name.py
Outdated
| if is_content_blocks_content: | ||
| text_blocks = [block for block in message.content if block["type"] == "text"] | ||
| if is_content_blocks_content := _is_content_blocks_content(message.content): | ||
| content_blocks_content = message.content |
There was a problem hiding this comment.
I thought it was necessary to get around a mypy error, but it looks like it's not. Removed.
| @@ -18,6 +18,11 @@ def _normalize_agent_name(agent_name: str) -> str: | |||
| return WHITESPACE_RE.sub("_", agent_name.strip()).lower() | |||
|
|
|||
|
|
|||
| def _has_multiple_content_blocks(content: str | list[str | dict]) -> TypeGuard[list[dict]]: | |||
There was a problem hiding this comment.
don't think it should have list of str
There was a problem hiding this comment.
Same as above – it's due to the type of BaseMessage.content.
langgraph_supervisor/supervisor.py
Outdated
| handoff_destinations = _get_handoff_destinations(tools or []) | ||
| if handoff_destinations: | ||
| if missing_handoff_destinations := set(agent_names) - set(handoff_destinations): | ||
| extracted_handoff_destinations = _get_handoff_destinations( |
There was a problem hiding this comment.
_get_handoff_destinations returns a list[str], but then down in the else block, handoff_destinations becomes a list[BaseTool]. This change is to avoid having to cast the handoff_destinations variable from list[str] to list[BaseTool] later on.
| def test_add_inline_agent_name_content_blocks(): | ||
| content_blocks = [ | ||
| def test_add_inline_agent_name_content_blocks() -> None: | ||
| content_blocks: list[str | dict] = [ |
There was a problem hiding this comment.
this should be just list of dict
There was a problem hiding this comment.
content_blocks is passed as the content parameter to AIMessage below, and that expects list[str | dict]. Because list is invariant, mypy won't accept a list[str] passed to a list[str | dict].
Perhaps it would be better for AIMessage to expect list[str] | list[dict] (or even Sequence[str | dict], since Sequence is covariant) to avoid needing to do this, but that would require a change in that library. Happy to do that if you think that's preferable.
I noticed that there are mypy issues in my editor since it isn't being enforced, so I fixed the error and added enforcement for it.
I tried to avoid using
castas much as possible, which sometimes required creating multiple variables where there once was one (mypy doesn't like when variables are reassigned with a different type). I also addedTypeGuards in certain cases where appropriate.The
langgraphdependency inuv.lockwas also updated to latest to pull in this fix to its typing.