-
Notifications
You must be signed in to change notification settings - Fork 1
fix(langchain): stop dropping temperature on gpt-5.x models [PC-4988] #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
2 changes: 1 addition & 1 deletion
2
packages/uipath_langchain_client/src/uipath_langchain_client/__version__.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| __title__ = "UiPath LangChain Client" | ||
| __description__ = "A Python client for interacting with UiPath's LLM services via LangChain." | ||
| __version__ = "1.18.3" | ||
| __version__ = "1.18.4" |
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
83 changes: 83 additions & 0 deletions
83
...es/uipath_langchain_client/src/uipath_langchain_client/clients/openai/gpt5_temperature.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| """Keep ``temperature`` on gpt-5 variants that default to no reasoning. | ||
|
|
||
| langchain-openai strips ``temperature`` for every ``gpt-5*`` model unless reasoning | ||
| effort is explicitly the string ``"none"``, at two sites in | ||
| ``langchain_openai.chat_models.base``: ``validate_temperature`` and | ||
| ``_construct_responses_api_payload``. Unset effort is ``None``, not ``"none"``, so | ||
| the value is dropped by default. That is right for base ``gpt-5`` and the ``pro`` | ||
| variants (default effort medium) but wrong for the dotted ones (``gpt-5.2``, | ||
| ``gpt-5.4``), which default to effort ``none`` and do accept it. | ||
|
|
||
| Delete once langchain-ai/langchain#35424 ships. Upstream: langchain-ai/langchain#35423. | ||
| """ | ||
|
|
||
| import re | ||
| from collections.abc import Mapping | ||
| from typing import Any, cast | ||
|
|
||
| from langchain_core.language_models import LanguageModelInput | ||
| from pydantic import model_validator | ||
|
|
||
| _DOTTED_GPT5 = re.compile(r"gpt-5\.\d+") | ||
|
|
||
|
|
||
| def gpt5_keeps_temperature( | ||
| model: str | None, | ||
| *, | ||
| reasoning_effort: str | None = None, | ||
| reasoning: Mapping[str, Any] | None = None, | ||
| ) -> bool: | ||
| """Whether this model accepts ``temperature`` as currently configured. | ||
|
|
||
| True only for dotted gpt-5 variants with effort unset or ``"none"``. False | ||
| everywhere else, so the caller defers to langchain. | ||
| """ | ||
| name = (model or "").lower() | ||
| if "chat" in name or "pro" in name: | ||
| return False | ||
| if not _DOTTED_GPT5.match(name): | ||
| return False | ||
| effort = reasoning_effort or (reasoning or {}).get("effort") | ||
| return effort is None or effort == "none" | ||
|
|
||
|
|
||
| class Gpt5TemperatureMixin: | ||
| """Restore ``temperature`` at both sites langchain-openai strips it. | ||
|
|
||
| Mix in ahead of the vendor chat class so the overrides win on the MRO. | ||
| """ | ||
|
|
||
| @model_validator(mode="before") | ||
| @classmethod | ||
| def validate_temperature(cls, values: dict[str, Any]) -> Any: | ||
| """Skip langchain's strip when the model does support ``temperature``.""" | ||
| if gpt5_keeps_temperature( | ||
| values.get("model_name") or values.get("model"), | ||
| reasoning_effort=values.get("reasoning_effort"), | ||
| reasoning=values.get("reasoning"), | ||
| ): | ||
| return values | ||
| return cast(Any, super()).validate_temperature(values) | ||
|
|
||
| def _get_request_payload( | ||
| self, | ||
| input_: LanguageModelInput, | ||
| *, | ||
| stop: list[str] | None = None, | ||
| **kwargs: Any, | ||
| ) -> dict[str, Any]: | ||
| """Put ``temperature`` back after the Responses payload builder drops it.""" | ||
| payload = cast( | ||
| dict[str, Any], | ||
| cast(Any, super())._get_request_payload(input_, stop=stop, **kwargs), | ||
| ) | ||
| if "temperature" in payload: | ||
| return payload | ||
| requested = kwargs.get("temperature", getattr(self, "temperature", None)) | ||
| if requested is None: | ||
| return payload | ||
| if "temperature" in (getattr(self, "disabled_params", None) or {}): | ||
| return payload | ||
| if gpt5_keeps_temperature(payload.get("model"), reasoning=payload.get("reasoning")): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about reasoning effort? |
||
| payload["temperature"] = requested | ||
| return payload | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| """Temperature survives on gpt-5 variants that default to no reasoning. | ||
|
|
||
| Covers both sites langchain-openai strips it, and pins the cases where stripping | ||
| is correct. Upstream: langchain-ai/langchain#35423. | ||
| """ | ||
|
|
||
| from typing import Any | ||
|
|
||
| import pytest | ||
| from langchain_core.messages import HumanMessage | ||
| from langchain_openai.chat_models import ChatOpenAI | ||
| from pydantic import SecretStr | ||
| from uipath_langchain_client.clients.openai.chat_models import ( | ||
| UiPathAzureChatOpenAI, | ||
| UiPathChatOpenAI, | ||
| ) | ||
| from uipath_langchain_client.settings import ApiFlavor | ||
|
|
||
| from uipath.llm_client.settings import UiPathBaseSettings | ||
|
|
||
| MESSAGES = [HumanMessage(content="hi")] | ||
|
|
||
|
|
||
| def _build(client_settings: UiPathBaseSettings, **kwargs: Any) -> Any: | ||
| chat_class = kwargs.pop("chat_class", UiPathChatOpenAI) | ||
| return chat_class( | ||
| model=kwargs.pop("model", "gpt-5.4"), | ||
| client_settings=client_settings, | ||
| model_details=kwargs.pop("model_details", {}), | ||
| api_flavor=ApiFlavor.RESPONSES, | ||
| **kwargs, | ||
| ) | ||
|
|
||
|
|
||
| def _payload(chat: Any) -> dict[str, Any]: | ||
| return chat._get_request_payload(MESSAGES) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("chat_class", [UiPathChatOpenAI, UiPathAzureChatOpenAI]) | ||
| def test_dotted_gpt5_keeps_temperature( | ||
| chat_class: type, client_settings: UiPathBaseSettings | ||
| ) -> None: | ||
| chat = _build(client_settings, chat_class=chat_class, temperature=0.64) | ||
| assert chat.temperature == 0.64 | ||
| assert _payload(chat)["temperature"] == 0.64 | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "overrides", | ||
| [ | ||
| {"reasoning_effort": "low"}, | ||
| {"reasoning": {"effort": "low"}}, | ||
| {"model": "gpt-5"}, | ||
| {"model": "gpt-5.4-pro"}, | ||
| ], | ||
| ids=["effort", "reasoning-dict", "base-gpt-5", "pro"], | ||
| ) | ||
| def test_dropped_where_temperature_is_unsupported( | ||
| overrides: dict[str, Any], client_settings: UiPathBaseSettings | ||
| ) -> None: | ||
| chat = _build(client_settings, temperature=0.64, **overrides) | ||
| assert chat.temperature is None | ||
| assert "temperature" not in _payload(chat) | ||
|
|
||
|
|
||
| def test_discovery_skip_flag_blocks_the_restore( | ||
| client_settings: UiPathBaseSettings, | ||
| ) -> None: | ||
| chat = _build(client_settings, model_details={"shouldSkipTemperature": True}) | ||
| assert "temperature" not in chat._get_request_payload(MESSAGES, temperature=0.5) | ||
|
|
||
|
|
||
| def test_langchain_handling_still_delegated(client_settings: UiPathBaseSettings) -> None: | ||
| assert _build(client_settings, model="o1").temperature == 1 | ||
| assert _build(client_settings, model="gpt-5-chat", temperature=0.64).temperature == 0.64 | ||
|
|
||
|
|
||
| def test_upstream_bug_still_present() -> None: | ||
| """Fails when langchain-openai fixes #35423; delete the shim and this file then.""" | ||
| plain = ChatOpenAI( | ||
| model="gpt-5.4", | ||
| api_key=SecretStr("x"), | ||
| temperature=0.64, | ||
| use_responses_api=True, | ||
| ) | ||
| assert plain.temperature is None | ||
| assert "temperature" not in plain._get_request_payload(MESSAGES) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't rely on model name for BYO because users can set a custom name for their models