Skip to content

Comments

Remove deprecated to_llm_dict() and from_litellm_message() methods#642

Merged
xingyaoww merged 3 commits intomainfrom
openhands/remove-deprecated-llm-message-methods
Oct 5, 2025
Merged

Remove deprecated to_llm_dict() and from_litellm_message() methods#642
xingyaoww merged 3 commits intomainfrom
openhands/remove-deprecated-llm-message-methods

Conversation

@xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Oct 4, 2025

Summary

This PR removes the deprecated to_llm_dict() and from_litellm_message() methods that were kept for backward compatibility after the native responses API implementation (PR #622).

Fixes #639

Changes

Method Removals

  • Removed Message.to_llm_dict() - deprecated alias for to_chat_dict()
  • Removed Message.from_litellm_message() - deprecated alias for from_llm_chat_message()

Internal Refactoring

  • Renamed BaseContent.to_llm_dict()_to_content_dicts() - clarified this is an internal method
  • Updated implementations in TextContent and ImageContent
  • Updated internal usage in Message._list_serializer()

Production Code Updates

  • openhands/sdk/llm/llm.py - changed message.to_llm_dict() to message.to_chat_dict()

Test Updates

  • All test files - migrated from deprecated methods to new API:
    • to_llm_dict()to_chat_dict() for Message serialization
    • from_litellm_message()from_llm_chat_message() for Message deserialization
    • to_llm_dict()_to_content_dicts() for content unit tests
  • Test function names - updated to reflect new method names
  • Docstrings and comments - updated references to deprecated methods

Files Modified

  • openhands/sdk/llm/message.py - removed deprecated methods, renamed internal methods
  • openhands/sdk/llm/llm.py - updated to use to_chat_dict()
  • tests/sdk/llm/test_message.py - migrated to new API
  • tests/sdk/llm/test_message_serialization.py - migrated to new API
  • tests/sdk/llm/test_thinking_blocks.py - migrated to new API
  • tests/sdk/llm/test_reasoning_content.py - migrated to new API
  • tests/fixtures/llm_data/data_generator.py - migrated to new API

Testing

All 334 tests in tests/sdk/llm/ pass successfully:

============================== 334 passed, 1 warning in 11.36s ========================

Pre-commit hooks pass (ruff format, ruff lint, pycodestyle, pyright).

Motivation

The new API provides:

  1. Clear separation between Chat Completions and Responses API formats
  2. Better naming that explicitly indicates which API format is being used
  3. Consistency with the dual completion() and responses() approach

This cleanup eliminates technical debt early in the responses API rollout.

@xingyaoww can click here to continue refining the PR


Agent Server images for this PR

GHCR package: https://github.com/All-Hands-AI/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Base Image Docs / Tags
golang golang:1.21-bookworm Link
java eclipse-temurin:17-jdk Link
python nikolaik/python-nodejs:python3.12-nodejs22 Link

Pull (multi-arch manifest)

docker pull ghcr.io/all-hands-ai/agent-server:705eb2a-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-705eb2a-python \
  ghcr.io/all-hands-ai/agent-server:705eb2a-python

All tags pushed for this build

ghcr.io/all-hands-ai/agent-server:705eb2a-golang
ghcr.io/all-hands-ai/agent-server:v1.0.0_golang_tag_1.21-bookworm_golang
ghcr.io/all-hands-ai/agent-server:705eb2a-java
ghcr.io/all-hands-ai/agent-server:v1.0.0_eclipse-temurin_tag_17-jdk_java
ghcr.io/all-hands-ai/agent-server:705eb2a-python
ghcr.io/all-hands-ai/agent-server:v1.0.0_nikolaik_s_python-nodejs_tag_python3.12-nodejs22_python

The 705eb2a tag is a multi-arch manifest (amd64/arm64); your client pulls the right arch automatically.

- Removed Message.to_llm_dict() alias (deprecated in favor of to_chat_dict())
- Removed Message.from_litellm_message() alias (deprecated in favor of from_llm_chat_message())
- Renamed internal BaseContent.to_llm_dict() to _to_content_dicts() to clarify it's internal
- Updated all production code to use to_chat_dict() instead of to_llm_dict()
- Updated all tests to use new API methods
- Updated test function names to reflect new method names
- Updated docstrings and comments to reference new method names
- All 334 tests in tests/sdk/llm/ pass

Co-authored-by: openhands <openhands@all-hands.dev>
@xingyaoww
Copy link
Collaborator Author

@OpenHands review diff from main and revert changes we did to to_llm_dict of openhands/sdk/llm/message.py

@openhands-ai
Copy link

openhands-ai bot commented Oct 5, 2025

I'm on it! xingyaoww can track my progress at all-hands.dev

The previous commit renamed BaseContent.to_llm_dict() to _to_content_dicts()
as part of removing deprecated methods. However, this method is an internal
implementation detail of the content classes, not a deprecated public API.

Changes:
- Reverted BaseContent.to_llm_dict() name (from _to_content_dicts)
- Reverted TextContent.to_llm_dict() name (from _to_content_dicts)
- Reverted ImageContent.to_llm_dict() name (from _to_content_dicts)
- Updated call site in Message._list_serializer()
- Updated test references in tests/sdk/llm/test_message.py

The deprecated Message.to_llm_dict() and Message.from_litellm_message()
methods remain removed as intended by the PR.

Co-authored-by: openhands <openhands@all-hands.dev>
@openhands-ai
Copy link

openhands-ai bot commented Oct 5, 2025

Summary

I've successfully reviewed the diff from main and reverted the changes to to_llm_dict in openhands/sdk/llm/message.py as requested.

What Was Reverted

The PR originally made two types of changes to to_llm_dict:

  1. Removed Message.to_llm_dict() - the deprecated public API (backward-compatible alias for to_chat_dict())
  2. Renamed BaseContent.to_llm_dict()_to_content_dicts() - an internal method used by content classes

I reverted only the rename (#2), keeping the removal of the deprecated method (#1) as intended by the PR.

Changes Made

Reverted in openhands/sdk/llm/message.py:

  • BaseContent._to_content_dicts()BaseContent.to_llm_dict()
  • TextContent._to_content_dicts()TextContent.to_llm_dict()
  • ImageContent._to_content_dicts()ImageContent.to_llm_dict()
  • Updated call site in Message._list_serializer() to use to_llm_dict()

Updated in tests/sdk/llm/test_message.py:

  • Changed all test references from _to_content_dicts() to to_llm_dict()

Rationale

  • BaseContent.to_llm_dict() is an internal implementation method used by content classes (TextContent, ImageContent), not a deprecated public API
  • The deprecated methods that should remain removed are Message.to_llm_dict() and Message.from_litellm_message()

Verification

✅ All 334 tests in tests/sdk/llm/ pass
✅ Pre-commit hooks pass (ruff format, ruff lint, pycodestyle, pyright)
✅ Changes committed and pushed to branch openhands/remove-deprecated-llm-message-methods

The PR now correctly removes only the deprecated public API methods while keeping the internal content class methods unchanged.

View full conversation

@enyst
Copy link
Collaborator

enyst commented Oct 5, 2025

OpenHands review diff from main and revert changes we did to to_llm_dict of openhands/sdk/llm/message.py

Yes, IMHO to_llm_dict on Content classes could stay like that... though it feels like there must be some room for improvement here... Maybe there's a better name, but it seems like it's a bit difficult to have a "parallel" responses variant on these small bits of code. 🤔
My cloud conversation from yesterday, on this, refuses to load, I should maybe make a new to look a bit.

On a side note, I find that a great way to use the agent is like:

  • load a branch on the cloud, in a tab
  • explore, talk, analyze code design variants
  • when I like one, I tell to make it on the branch
  • or when I kinda like one, but I'm not sure, I tell it to make it on a new branch, so I can stare at it or whatever needs done
  • and go at a higher level of abstraction
  • and back to the code.

That may sound totally normal, but idk, what I mean... I'm not sure how to say this. Let's say this way: we're in October 2025.

  • around January 2025, I had a lot of fun with rounds of OpenHands+Gemini-2.0-FT on code exploration and brainstorming. But: 1) it wasn't too easy to switch levels of abstraction, or only up to a point; and 2) the brainstorming was close to the code, so to speak, and yet little 2.0-FT would stumble if you tried to use it to make that code real, so well... not that useful.
  • (this is where "switch llm" helps immensely, that was exactly the missing piece, sometimes I did switch to Sonnet-3.7 on that context, to do stuff; but it required reloading; and then there was a timeframe when, for a reason I never understood, the web GUI forced a new conversation, so you couldn't use it at all)
  • in October 2025, with OpenHands-GPT-5, I can go almost to any level of abstraction and back; I can also tell it to go put stuff on a new branch and come back to our branch and topic; it does it without blinking!
  • for example, we talked APIs, and jumped to "how do we refactor LLM to x, y, z", and then jumped to "okay, so, stats and serialization, how is it affected if we switch llm like this? or like that?", write a branch on my playground, then spawn another branch for the question "okay, now let's see the boundaries between SDK and clients on this issue" --- in the same conversation
  • there can be a lot of context needed when switching levels of abstraction or perspectives. Sometimes it gets confused. But it's a lot, a lot, more usable than ever before.
  • OH is a "coding tool", we say. But it's not just a coding tool. It's a tool that helps me make up my mind, about the code design / architecture / boundaries of core functionality. That's no small thing!

Old OpenHands-Gemini-2.0-FT could do exploratory conversations better than 3.7. On the other hand, OpenHands-GPT-5 feels like... the real deal. I bump into some usability issues. But it's so good.

I don't understand how people still use the LLM providers apps, when they're hamstrung by the inability to (insert here: "explore the other repo", "make a md file on this, on a new branch, push it out and come back to me", "with this in mind, review that PR and fix it", "map this execution path and give me concise pseudocode, now change it to include x").

Anyway, sorry. I'm so excited about this.

What relation does it have with content.to_llm_dict? Not a lot maybe 😅, but I see every detail of the LLM interaction as part of the whole. If to_llm_dict is just a small detail that can sit there and bother no one, ok. If it's confusing, maybe we could inline it and be done. Or remove it b/c we add litellm types for the Message parts. 🫠

@xingyaoww xingyaoww marked this pull request as ready for review October 5, 2025 19:00
@xingyaoww
Copy link
Collaborator Author

Yes, IMHO to_llm_dict on Content classes could stay like that... though it feels like there must be some room for improvement here... Maybe there's a better name, but it seems like it's a bit difficult to have a "parallel" responses variant on these small bits of cod

Yeah my primary concern is that if we keep this, we will have three different ways to convert Messages to dict with two of those be the same 😓

So i think it is better for us to keep things more explicit

@xingyaoww xingyaoww merged commit f1bf8d2 into main Oct 5, 2025
14 checks passed
@xingyaoww xingyaoww deleted the openhands/remove-deprecated-llm-message-methods branch October 5, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove deprecated to_llm_dict() and from_litellm_message() in favor of new responses API

3 participants