refactor(sdk/llm/mixim): separate data from logic by VascoSch92 · Pull Request #2354 · OpenHands/software-agent-sdk

VascoSch92 · 2026-03-07T00:18:39Z

Summary

Separating data from logic.

Before, fn_call_converter.py was 1288 lines where ~380 lines of prompt example strings were interleaved with the actual
conversion algorithms. When you opened the file to understand or modify the conversion logic, you had to scroll past
hundreds of lines of static text. And when you wanted to update an example prompt, you had to navigate through parsing
and regex code to find it.

Now the responsibilities are clear:

fn_call_examples.py — "what do the examples look like?" (data you edit when tool interfaces change)
fn_call_converter.py — "how do we convert between message formats?" (logic you edit when the conversion algorithm
changes)

These two concerns change for different reasons and at different times. A tool rename touches the examples file. A bug in
parameter parsing touches the converter file. Before, both changes happened in the same 1288-line file, increasing the
chance of merge conflicts and making code review harder because reviewers had to mentally separate data edits from logic
edits.

Checklist

If the PR is changing/adding functionality, are there tests to reflect this?
If there is an example, have you run the example to make sure that it works?
If there are instructions on how to run the code, have you followed the instructions and made sure that it works?
If the feature is significant enough to require documentation, is there a PR open on the OpenHands/docs repository with the same branch name?
Is the github CI passing?

Agent Server images for this PR

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	`eclipse-temurin:17-jdk`	Link
python	amd64, arm64	`nikolaik/python-nodejs:python3.13-nodejs22`	Link
golang	amd64, arm64	`golang:1.21-bookworm`	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:d367529-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-d367529-python \
  ghcr.io/openhands/agent-server:d367529-python

All tags pushed for this build

ghcr.io/openhands/agent-server:d367529-golang-amd64
ghcr.io/openhands/agent-server:d367529-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:d367529-golang-arm64
ghcr.io/openhands/agent-server:d367529-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:d367529-java-amd64
ghcr.io/openhands/agent-server:d367529-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:d367529-java-arm64
ghcr.io/openhands/agent-server:d367529-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:d367529-python-amd64
ghcr.io/openhands/agent-server:d367529-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-amd64
ghcr.io/openhands/agent-server:d367529-python-arm64
ghcr.io/openhands/agent-server:d367529-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-arm64
ghcr.io/openhands/agent-server:d367529-golang
ghcr.io/openhands/agent-server:d367529-java
ghcr.io/openhands/agent-server:d367529-python

About Multi-Architecture Support

Each variant tag (e.g., d367529-python) is a multi-arch manifest supporting both amd64 and arm64
Docker automatically pulls the correct architecture for your platform
Individual architecture tags (e.g., d367529-python-amd64) are also available if needed

github-actions · 2026-03-07T00:19:05Z

API breakage checks (Griffe)

Result: Passed

Action log

github-actions · 2026-03-07T00:19:22Z

Agent server REST API breakage checks (OpenAPI)

Result: Failed

Log excerpt (first 1000 characters)

{"asctime": "2026-03-10 16:17:14,563", "levelname": "WARNING", "name": "openhands.agent_server.config", "filename": "config.py", "lineno": 173, "message": "\u26a0\ufe0f OH_SECRET_KEY was not defined. Secrets will not be persisted between restarts."}
::error title=openhands-agent-server REST API::Breaking REST API change detected without MINOR version bump (1.12.0 -> 1.12.0).

Breaking REST API changes detected compared to baseline release:
- added '#/components/schemas/HookExecutionEvent' to the '/items/anyOf[#/components/schemas/Event]/' response property 'oneOf' list for the response status '200'
- added '#/components/schemas/HookExecutionEvent' to the 'items/items/' response property 'oneOf' list for the response status '200'
- added '#/components/schemas/HookExecutionEvent' to the response body 'oneOf' list for the response status '200'
- the 'file' request property type/format changed from 'string'/'' to 'string'/'binary'
- added the new 'hook' enum value to the '/items/anyOf[#/co

Action log

all-hands-bot

Clean refactoring that separates data from logic. Verified no breaking changes - the moved constants and functions are not imported elsewhere. This improves maintainability by reducing merge conflict risk and making both concerns easier to navigate.

all-hands-bot

🟢 Good taste - Clean separation of data from logic that makes both concerns easier to navigate and maintain. No behavior change, no breaking changes, reduces cognitive load. LGTM.

enyst

Thank you, this is overdue! I agree, I'm just thinking though, merge may complicate a little bit the merge for this older PR

#2251

Could we maybe do like, merge 2251 then 2354?

VascoSch92 · 2026-03-08T19:05:18Z

Thank you, this is overdue! I agree, I'm just thinking though, merge may complicate a little bit the merge for this older PR

fix: add security_risk and summary to tool examples for non-native function calling #2251

Could we maybe do like, merge 2251 then 2354?

Ah don't worry. It is not high priority.

I was justing testing the refactoring capability of OH :-)

xingyaoww

Thanks!

xingyaoww · 2026-03-10T16:07:31Z

@OpenHands pls merge from main and resolve conflicts

openhands-ai · 2026-03-10T16:07:53Z

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

Incorporate security_risk and summary parameters into tool examples in fn_call_examples.py to match the changes made on main in fn_call_converter.py. Also uses single-line command format consistent with main. Co-authored-by: openhands <openhands@all-hands.dev>

openhands-ai · 2026-03-10T16:16:58Z

Here's a summary of the work done:

Summary

The PR branch vasco/refactor-call-converter has been merged with main and all conflicts resolved. The changes have been pushed to the remote.

What was done:

Merged main into the branch — There was a conflict in fn_call_converter.py because main (via PR fix: add security_risk and summary to tool examples for non-native function calling #2251) added security_risk and summary parameters to the inline tool examples, while this PR moved those examples to a separate fn_call_examples.py file.
Resolved the conflict in fn_call_converter.py — Kept the PR's approach: no inline data, just an import from fn_call_examples.py. (The remote had already pushed a merge commit resolving this the same way, so after rebasing this was clean.)
Updated fn_call_examples.py with changes from main — The merge from main didn't automatically propagate the security_risk and summary parameter additions into the separated examples file. I updated all tool examples to include these parameters (matching main's format: single-line command parameters + security_risk/summary fields). This was committed as 185aab72.

Verification:

✅ All 612 LLM tests pass
✅ Pre-commit checks (ruff format, ruff lint, pycodestyle, pyright, import deps, tool registration) all pass
✅ No conflict markers remain
✅ Changes pushed to origin/vasco/refactor-call-converter

The PR is at: #2354

xingyaoww · 2026-03-10T16:20:54Z

@OpenHands review diff from main and review this PR.

Check carefully and tell me, did we make ANY copy-pasting error in the refactor PR?

openhands-ai · 2026-03-10T16:21:14Z

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

xingyaoww · 2026-03-10T16:24:00Z

Review: Checking for copy-paste errors in the refactor

I did a thorough diff-level and runtime-equivalence check. Here are the results:

✅ `TOOL_EXAMPLES` dictionary — IDENTICAL (294 lines, character-for-character match)

✅ Tool name constants — IDENTICAL (all 6 constants: same keys, same values)

The only change is the addition of Final[str] type annotations, which is a nice improvement.

✅ `get_example_for_tools()` function — IDENTICAL (73 lines)

The only difference is refine_prompt → _refine_prompt (underscore prefix to mark it module-private), which is an intentional rename, not a copy-paste error.

✅ `fn_call_converter.py` remainder — IDENTICAL

Everything from IN_CONTEXT_LEARNING_EXAMPLE_PREFIX onward is unchanged. The header only differs by: -import sys and +from openhands.sdk.llm.mixins.fn_call_examples import get_example_for_tools. Both are correct.

✅ End-to-end runtime equivalence — VERIFIED

I extracted both versions, ran get_example_for_tools() with multiple tool combinations (all tools, terminal-only, file_editor-only, terminal+file_editor, edit_file-only, empty), and confirmed the outputs are byte-identical in every case.

Verdict

No copy-paste errors found. The refactor is a clean extraction — data moved out, logic stayed, nothing lost or duplicated.

openhands-ai · 2026-03-10T16:24:42Z

Summary

The PR was reviewed for copy-paste errors as requested. No code changes were made — this was purely an analysis task.

Answer to the Review Question

No copy-paste errors were found in the refactor. Here's what I verified:

✅ TOOL_EXAMPLES dictionary — 294 lines, character-for-character identical between main and the new fn_call_examples.py
✅ Tool name constants (all 6) — same keys, same values. Only improvement: added Final[str] type annotations
✅ get_example_for_tools() function — 73 lines, identical. The only difference is refine_prompt → _refine_prompt (underscore prefix to mark it module-private), which is an intentional rename
✅ fn_call_converter.py remainder — all conversion logic from IN_CONTEXT_LEARNING_EXAMPLE_PREFIX onward is completely unchanged
✅ End-to-end runtime equivalence — I ran get_example_for_tools() with 6 different tool combinations (all tools, terminal-only, file_editor-only, terminal+file_editor, edit_file-only, empty list) and confirmed outputs are byte-identical in every case

The refactor is a clean extraction: data moved to fn_call_examples.py, logic stayed in fn_call_converter.py, nothing lost or duplicated. These findings were posted as a comment on PR #2354.

refactor

6a2095e

VascoSch92 requested a review from all-hands-bot March 7, 2026 00:18

all-hands-bot approved these changes Mar 7, 2026

View reviewed changes

VascoSch92 marked this pull request as ready for review March 7, 2026 00:22

all-hands-bot approved these changes Mar 7, 2026

View reviewed changes

enyst reviewed Mar 8, 2026

View reviewed changes

xingyaoww approved these changes Mar 10, 2026

View reviewed changes

VascoSch92 and others added 2 commits March 10, 2026 17:08

Merge branch 'main' into vasco/refactor-call-converter

dc0ee30

xingyaoww merged commit 13092ce into main Mar 10, 2026
21 checks passed

xingyaoww deleted the vasco/refactor-call-converter branch March 10, 2026 16:26

Conversation

VascoSch92 commented Mar 7, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

github-actions bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

API breakage checks (Griffe)

Uh oh!

github-actions bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Agent server REST API breakage checks (OpenAPI)

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

enyst left a comment

Choose a reason for hiding this comment

Uh oh!

VascoSch92 commented Mar 8, 2026

Uh oh!

xingyaoww left a comment

Choose a reason for hiding this comment

Uh oh!

xingyaoww commented Mar 10, 2026

Uh oh!

openhands-ai bot commented Mar 10, 2026

Uh oh!

openhands-ai bot commented Mar 10, 2026

Summary

What was done:

Verification:

Uh oh!

xingyaoww commented Mar 10, 2026

Uh oh!

openhands-ai bot commented Mar 10, 2026

Uh oh!

xingyaoww commented Mar 10, 2026

Review: Checking for copy-paste errors in the refactor

✅ TOOL_EXAMPLES dictionary — IDENTICAL (294 lines, character-for-character match)

✅ Tool name constants — IDENTICAL (all 6 constants: same keys, same values)

✅ get_example_for_tools() function — IDENTICAL (73 lines)

✅ fn_call_converter.py remainder — IDENTICAL

✅ End-to-end runtime equivalence — VERIFIED

Verdict

Uh oh!

openhands-ai bot commented Mar 10, 2026

Summary

Answer to the Review Question

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VascoSch92 commented Mar 7, 2026 •

edited by github-actions bot

Loading

github-actions bot commented Mar 7, 2026 •

edited

Loading

github-actions bot commented Mar 7, 2026 •

edited

Loading

✅ `TOOL_EXAMPLES` dictionary — IDENTICAL (294 lines, character-for-character match)

✅ `get_example_for_tools()` function — IDENTICAL (73 lines)

✅ `fn_call_converter.py` remainder — IDENTICAL