Compare commits

...
Author SHA1 Message Date
Elliot Slusky 420908401c fix(engine): count tool call payloads in token estimates (#614)
Closes #608. Make Message.content officially Optional (str | None) with a Message.text accessor that treats None as empty, and count tool-call IDs/names/arguments, tool-result IDs, and reasoning/thinking metadata in estimate_prompt_tokens — all are replayed into later prompt turns, so they belong in the estimate. Extends estimator and message-type regression tests.
2026-06-29 17:34:10 -07:00
Elliot Slusky b70be55681 fix(openhands): handle none content in token estimates (#612)
Closes #607. Assistant tool-call turns can carry content=None, which crashed token estimation (len(m.content)) and think-tag stripping. Normalize with 'content or ""', route native OpenHands truncation through the shared estimate_prompt_tokens, and add tests for the estimator, the truncation helper, and an end-to-end tool-call run with None content.
2026-06-29 16:51:52 -07:00
6 changed files with 143 additions and 9 deletions
+5 -5
View File
@@ -19,6 +19,7 @@ from openjarvis.agents.prompt_loader import (
from openjarvis.core.events import EventBus
from openjarvis.core.registry import AgentRegistry
from openjarvis.core.types import Message, Role, ToolCall, ToolResult
from openjarvis.engine._base import estimate_prompt_tokens
from openjarvis.engine._stubs import InferenceEngine
from openjarvis.tools._stubs import BaseTool, build_tool_descriptions
@@ -116,8 +117,7 @@ class NativeOpenHandsAgent(ToolUsingAgent):
max_prompt_tokens: int = 3000,
) -> list[Message]:
"""Truncate messages if estimated token count exceeds limit."""
total_chars = sum(len(m.content) for m in messages)
estimated_tokens = total_chars // 4
estimated_tokens = estimate_prompt_tokens(messages)
if estimated_tokens <= max_prompt_tokens:
return messages
# Find the last user message and truncate its content
@@ -125,7 +125,7 @@ class NativeOpenHandsAgent(ToolUsingAgent):
if messages[i].role == Role.USER:
excess_tokens = estimated_tokens - max_prompt_tokens
excess_chars = excess_tokens * 4
original = messages[i].content
original = messages[i].content or ""
if len(original) > excess_chars + 200:
truncated = original[: len(original) - excess_chars]
messages[i] = Message(
@@ -258,7 +258,7 @@ class NativeOpenHandsAgent(ToolUsingAgent):
# still emitted before re-raising.
self._emit_turn_end(turns=1, error=True)
raise
content = self._strip_think_tags(result.get("content", ""))
content = self._strip_think_tags(result.get("content") or "")
usage = result.get("usage", {})
self._emit_turn_end(turns=1)
return AgentResult(
@@ -315,7 +315,7 @@ class NativeOpenHandsAgent(ToolUsingAgent):
for k in total_usage:
total_usage[k] += usage.get(k, 0)
content = result.get("content", "")
content = result.get("content") or ""
# Strip think tags so they don't interfere with parsing
content = self._strip_think_tags(content)
last_content = content
+6 -1
View File
@@ -63,7 +63,7 @@ class Message:
"""A single chat message (OpenAI-compatible structure)."""
role: Role
content: str = ""
content: str | None = ""
name: Optional[str] = None
tool_calls: Optional[List[ToolCall]] = None
tool_call_id: Optional[str] = None
@@ -73,6 +73,11 @@ class Message:
# empty for text-only messages (the common case).
images: Optional[List[str]] = None
@property
def text(self) -> str:
"""Return message content as text, treating ``None`` as empty."""
return self.content or ""
@dataclass(slots=True)
class Conversation:
+20 -2
View File
@@ -13,6 +13,22 @@ class EngineConnectionError(Exception):
"""Raised when an engine is unreachable."""
_REASONING_METADATA_KEYS = ("reasoning_content", "thinking")
def _message_estimated_chars(message: Message) -> int:
parts = [message.text]
for key in _REASONING_METADATA_KEYS:
value = message.metadata.get(key)
if isinstance(value, str):
parts.append(value)
for tc in message.tool_calls or []:
parts.extend((tc.id, tc.name, tc.arguments))
if message.tool_call_id:
parts.append(message.tool_call_id)
return sum(len(part) for part in parts)
def messages_to_dicts(messages: Sequence[Message]) -> List[Dict[str, Any]]:
"""Convert ``Message`` objects to OpenAI-format dicts."""
out: List[Dict[str, Any]] = []
@@ -53,9 +69,11 @@ def estimate_prompt_tokens(messages: Sequence[Message]) -> int:
provider would charge.
Uses ~4 characters per token (standard BPE average for English) plus
a small per-message overhead for role markers and separators.
a small per-message overhead for role markers and separators. Counts
content, reasoning metadata, tool-call payloads, and tool result IDs
because all are replayed into later prompt turns when present.
"""
total_chars = sum(len(m.content) for m in messages)
total_chars = sum(_message_estimated_chars(m) for m in messages)
# ~4 tokens overhead per message for role markers / separators
overhead = len(messages) * 4
return max(1, total_chars // 4 + overhead)
+46 -1
View File
@@ -8,7 +8,7 @@ from openjarvis.agents._stubs import AgentContext
from openjarvis.agents.native_openhands import NativeOpenHandsAgent
from openjarvis.core.events import EventBus, EventType
from openjarvis.core.registry import AgentRegistry
from openjarvis.core.types import Conversation, Message, Role, ToolResult
from openjarvis.core.types import Conversation, Message, Role, ToolCall, ToolResult
from openjarvis.tools._stubs import BaseTool, ToolSpec
# ---------------------------------------------------------------------------
@@ -118,6 +118,51 @@ class TestNativeOpenHandsRegistration:
class TestNativeOpenHandsAgent:
def test_truncate_handles_none_content_tool_call_turn(self):
"""Tool-call assistant turns may carry content=None."""
engine = MagicMock()
engine.engine_id = "mock"
agent = NativeOpenHandsAgent(engine, "test-model")
messages = [
Message(role=Role.USER, content="hi"),
Message(
role=Role.ASSISTANT,
content=None, # type: ignore[arg-type]
tool_calls=[ToolCall(id="call_1", name="calculator", arguments="{}")],
),
]
assert agent._truncate_if_needed(messages) == messages
def test_native_tool_call_with_none_content_does_not_crash(self):
"""Native tool-call responses may omit assistant text content."""
engine = MagicMock()
engine.engine_id = "mock"
engine.generate.side_effect = [
_engine_response(
None,
tool_calls=[
{
"id": "call_1",
"name": "calculator",
"arguments": '{"expression": "2+2"}',
}
],
),
_engine_response("The result is 4."),
]
agent = NativeOpenHandsAgent(
engine,
"test-model",
tools=[_CalculatorStub()],
)
result = agent.run("What is 2+2?")
assert result.content == "The result is 4."
assert result.turns == 2
assert [tr.content for tr in result.tool_results] == ["4"]
def test_simple_response(self):
"""No code -> direct answer."""
engine = MagicMock()
+6
View File
@@ -35,9 +35,15 @@ class TestMessage:
msg = Message(role=Role.USER, content="hello")
assert msg.role == Role.USER
assert msg.content == "hello"
assert msg.text == "hello"
assert msg.tool_calls is None
assert msg.metadata == {}
def test_none_content_text_helper(self) -> None:
msg = Message(role=Role.ASSISTANT, content=None)
assert msg.content is None
assert msg.text == ""
def test_tool_calls(self) -> None:
tc = ToolCall(id="1", name="calc", arguments='{"x": 1}')
msg = Message(role=Role.ASSISTANT, content="", tool_calls=[tc])
+60
View File
@@ -0,0 +1,60 @@
from __future__ import annotations
from openjarvis.core.types import Message, Role, ToolCall
from openjarvis.engine._base import estimate_prompt_tokens
def test_estimate_prompt_tokens_handles_none_content_tool_call_turn() -> None:
messages = [
Message(role=Role.USER, content="hi"),
Message(
role=Role.ASSISTANT,
content=None,
tool_calls=[ToolCall(id="call_1", name="lookup", arguments="{}")],
),
]
assert estimate_prompt_tokens(messages) == 12
def test_estimate_prompt_tokens_counts_tool_call_arguments() -> None:
base = [
Message(role=Role.USER, content="hi"),
Message(role=Role.ASSISTANT, content=None),
]
with_tool_call = [
Message(role=Role.USER, content="hi"),
Message(
role=Role.ASSISTANT,
content=None,
tool_calls=[ToolCall(id="", name="", arguments="abcdefgh")],
),
]
assert estimate_prompt_tokens(with_tool_call) - estimate_prompt_tokens(base) == 2
def test_estimate_prompt_tokens_counts_reasoning_metadata() -> None:
base = [
Message(role=Role.USER, content="hi"),
Message(role=Role.ASSISTANT, content=None),
]
with_reasoning = [
Message(role=Role.USER, content="hi"),
Message(
role=Role.ASSISTANT,
content=None,
metadata={"reasoning_content": "abcdefgh"},
),
]
assert estimate_prompt_tokens(with_reasoning) - estimate_prompt_tokens(base) == 2
def test_estimate_prompt_tokens_counts_tool_result_ids() -> None:
messages = [
Message(role=Role.USER, content="hi"),
Message(role=Role.TOOL, content="ok", tool_call_id="abcdefgh"),
]
assert estimate_prompt_tokens(messages) == 11