[Bug] reasoning_effort is annotated with a function call, which is not valid Literal typing - #2102
Open
ayaangazali wants to merge 1 commit into
Open
Conversation
…n call `reasoning_effort: Literal[get_reasoning_efforts()]` is not valid typing. Literal takes literal members, so every type checker rejects a call there, and the annotation is evaluated when the class body runs -- which calls get_reasoning_efforts(), and that function does `import litellm` to read the values off litellm.completion's signature. Spell the members out as a ReasoningEffort Literal and derive the tuple from it with get_args, so the static type and the runtime tuple cannot disagree. get_reasoning_efforts() is unchanged and still unions whatever the installed litellm advertises, so no accepted value is narrowed.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The follow-up you suggested on #1773 — but smaller than proposed, and for a different reason than I expected. Details below.
The bug
swarms/structs/agent.py:394on master:Literaltakes literal members. A function call there is rejected by every type checker, soreasoning_efforthas effectively been unannotated for tooling purposes. It happens to work at runtime only because annotations are evaluated as ordinary expressions.And it is evaluated — at class-body execution, i.e. when
agent.pyis imported.get_reasoning_efforts()does this:So an annotation reaches into litellm's signature at import time.
The fix
Spell the members out once as a
Literal, and derive the tuple from it:The static type and the runtime tuple now come from one source, so they cannot disagree.
get_reasoning_efforts()is untouched and still unions whatever the installed litellm advertises, so no accepted value is narrowed — that behaviour was the point of the helper and it is preserved.Verified
Why this is smaller than you proposed
You suggested salvaging two files from #1773. I checked both and only one survives contact:
get_reasoning_efforts.py— real, and it turned out to be a correctness bug rather than only a perf one. That is this PR.swarms/tools/mcp_manager.py— I built the leaf deferral (from __future__ import annotations,TYPE_CHECKINGfor the two annotations, local imports at the two genuine runtime sitesisinstance(tool, MCPTool)andasync with ClientSession(...), pluslru_cacheon_mcp_is_v2) and then measured it, and it changes nothing today:Because importing
swarms.tools.mcp_managerexecutesswarms/__init__.pyfirst, which pulls the package in regardless. The leaf deferral only pays off once the package boundary is handled — which is #2090's PEP 562 work. So it is not "helps anyone importing mcp_manager directly" yet, and I would rather not ship a change whose stated benefit I cannot demonstrate. Happy to send it as a one-file follow-up the moment #2090 lands, when it will actually compose.One correction to the framing on #1773: #2090 has not landed — it is still open as of now, and master still has
from litellm import model_listatagent.py:21. Nothing here depends on it either way; this PR stands alone.