refactor: deprecate vision_enabled field on Message, pass as parameter instead #1797
+166
−56
Conversation
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
…r instead This refactoring moves vision_enabled from being a stored field on Message to being a parameter passed during serialization. This is a cleaner design because vision capability is an LLM property, not a message property. Changes: - to_chat_dict() now accepts optional vision_enabled parameter - _list_serializer() now requires vision_enabled parameter - format_messages_for_llm() now passes vision_enabled during serialization - Added deprecation notice to vision_enabled field (deprecated in 1.10.0, removed in 1.11.0) - Backward compatibility maintained: if vision_enabled parameter is None, falls back to the field value (with deprecation warning at version 1.10.0+) This aligns to_chat_dict() with to_responses_value() which already takes vision_enabled as a parameter. Benefits: - Cleaner separation of concerns (message content vs serialization context) - No need to set fields on messages before serializing - Eliminates potential for stored state to get out of sync with actual LLM capabilities Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
Coverage Report •
|
|||||||||||||||||||||||||
Contributor
Author
|
Closing in favor of #1798 |
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.
Summary
This PR refactors the
vision_enabledfield on Message objects by deprecating it as a stored field and passing it as a parameter to serialization methods instead.Problem
The
vision_enabledfield was being stored on each Message object, but this created several issues:_supports_vision()stateSolution
to_chat_dict()now accepts an optionalvision_enabled: bool | None = Noneparameter_list_serializer()now requiresvision_enabled: boolas a keyword-only parameterformat_messages_for_llm()now passesvision_enabled=self._supports_vision()to the serializerChanges
openhands-sdk/openhands/sdk/llm/message.pyto_chat_dict()to accept optionalvision_enabledparameter_list_serializer()to requirevision_enabledas keyword-only parametervision_enabledfield is used directlyopenhands-sdk/openhands/sdk/llm/llm.pyformat_messages_for_llm()to passvision_enabled=self._supports_vision()Test files updated
test_message_serialization.py- updated calls to_list_serializer()test_thinking_blocks.py- updated calls to_list_serializer()test_message.py- updated calls to_list_serializer()Testing
All 495 tests pass.
Related
This is a follow-up to PR #1795 which fixed vision detection for proxy model names. During investigation of that issue, it was discovered that the
vision_enabledfield design could be improved.Migration Guide
For users directly calling
_list_serializer():For users relying on stored
vision_enabledfield:The field is deprecated and will be removed in v1.11.0. The field value is still respected for backward compatibility, but new code should pass vision_enabled as a parameter to serialization methods.
@neubig can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:b47690c-pythonRun
All tags pushed for this build
About Multi-Architecture Support
b47690c-python) is a multi-arch manifest supporting both amd64 and arm64b47690c-python-amd64) are also available if needed