mirrors/hkuds--openharness
1
0
Fork 0
Code Issues Pull Requests Actions 2 Deployments Models Agent Notes Packages Projects Releases Wiki Activity

fix(compact): keep tool pairs intact across compact boundaries

Browse Source
This commit is contained in:
tjb-tech
2026-04-24 16:26:57 +00:00
parent 0bba07fe19
commit 835588daa0
2 changed files with 168 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/openharness/services/compact/__init__.py
+56 -11
View File
@@ -24,6 +24,7 @@ from openharness.engine.messages import (
TextBlock,
ToolResultBlock,
ToolUseBlock,
sanitize_conversation_messages,
)
from openharness.engine.stream_events import CompactProgressEvent
from openharness.hooks import HookEvent, HookExecutor
@@ -256,8 +257,7 @@ def try_context_collapse(
if len(messages) <= preserve_recent + 2:
return None
older = messages[:-preserve_recent]
newer = messages[-preserve_recent:]
older, newer = _split_preserving_tool_pairs(messages, preserve_recent=preserve_recent)
changed = False
collapsed_older: list[ConversationMessage] = []
for message in older:
@@ -406,6 +406,53 @@ def build_post_compact_messages(result: CompactionResult) -> list[ConversationMe
]
def _boundary_crosses_tool_pair(previous: ConversationMessage, current: ConversationMessage) -> bool:
"""Return True when a preserve boundary would split a tool_use/result pair."""
if previous.role != "assistant" or current.role != "user":
return False
pending_tool_ids = {block.id for block in previous.content if isinstance(block, ToolUseBlock)}
if not pending_tool_ids:
return False
result_ids = {block.tool_use_id for block in current.content if isinstance(block, ToolResultBlock)}
return bool(pending_tool_ids & result_ids)
def _split_preserving_tool_pairs(
messages: list[ConversationMessage],
*,
preserve_recent: int,
) -> tuple[list[ConversationMessage], list[ConversationMessage]]:
"""Split older/newer segments without cutting through a tool_use/result pair.
The preserved segment is also sanitized so trailing orphan tool_use blocks
never survive the compaction boundary.
"""
if len(messages) <= preserve_recent:
return [], sanitize_conversation_messages(list(messages))
split_index = max(0, len(messages) - preserve_recent)
while split_index > 0 and _boundary_crosses_tool_pair(messages[split_index - 1], messages[split_index]):
split_index -= 1
older = list(messages[:split_index])
newer = sanitize_conversation_messages(list(messages[split_index:]))
return older, newer
def _sanitize_compaction_segments(result: CompactionResult) -> None:
"""Normalize summary+preserved messages into a provider-safe sequence."""
if not result.summary_messages and not result.messages_to_keep:
return
combined = [*result.summary_messages, *result.messages_to_keep]
sanitized = sanitize_conversation_messages(combined)
summary_count = len(result.summary_messages)
result.summary_messages = sanitized[:summary_count]
result.messages_to_keep = sanitized[summary_count:]
def _create_recent_attachments_attachment_if_needed(
attachment_paths: list[str],
) -> CompactAttachment | None:
@@ -625,6 +672,7 @@ def _build_compact_attachments(
def _finalize_compaction_result(result: CompactionResult) -> CompactionResult:
_sanitize_compaction_segments(result)
messages = build_post_compact_messages(result)
result.compact_metadata.setdefault("post_compact_message_count", len(messages))
result.compact_metadata.setdefault("post_compact_token_count", estimate_message_tokens(messages))
@@ -779,8 +827,7 @@ def try_session_memory_compaction(
"""Cheap deterministic compaction for long chats before full LLM compaction."""
if len(messages) <= preserve_recent + 4:
return None
older = messages[:-preserve_recent]
newer = messages[-preserve_recent:]
older, newer = _split_preserving_tool_pairs(messages, preserve_recent=preserve_recent)
summary_message = _build_session_memory_message(older)
if summary_message is None:
return None
@@ -1023,8 +1070,7 @@ async def compact_conversation(
log.info("Compacting conversation: %d messages, ~%d tokens", len(messages), pre_compact_tokens)
# Step 2: split into older (summarize) and newer (preserve)
older = messages[:-preserve_recent]
newer = messages[-preserve_recent:]
older, newer = _split_preserving_tool_pairs(messages, preserve_recent=preserve_recent)
# Step 3: build compact request — send older messages + compact prompt
compact_prompt = get_compact_prompt(custom_instructions)
@@ -1541,19 +1587,18 @@ def compact_messages(
) -> list[ConversationMessage]:
"""Replace older conversation history with a synthetic summary (legacy)."""
if len(messages) <= preserve_recent:
return list(messages)
older = messages[:-preserve_recent]
newer = messages[-preserve_recent:]
return sanitize_conversation_messages(list(messages))
older, newer = _split_preserving_tool_pairs(messages, preserve_recent=preserve_recent)
summary = summarize_messages(older)
if not summary:
return list(newer)
return [
return sanitize_conversation_messages([
ConversationMessage(
role="user",
content=[TextBlock(text=f"[conversation summary]\n{summary}")],
),
*newer,
]
])
__all__ = [
tests/test_services/test_compact.py
+112 -1
View File
@@ -8,7 +8,7 @@ import pytest
from openharness.api.client import ApiMessageCompleteEvent
from openharness.api.usage import UsageSnapshot
from openharness.engine.messages import ConversationMessage, ImageBlock, TextBlock, ToolUseBlock
from openharness.engine.messages import ConversationMessage, ImageBlock, TextBlock, ToolResultBlock, ToolUseBlock
from openharness.hooks import HookEvent
from openharness.services import (
build_post_compact_messages,
@@ -53,6 +53,53 @@ def test_compact_and_summarize_messages():
assert estimate_conversation_tokens(compacted) >= 1
def test_compact_messages_shifts_boundary_to_keep_tool_pair_intact():
messages = [
ConversationMessage.from_user_text("first"),
ConversationMessage(
role="assistant",
content=[ToolUseBlock(id="toolu_pair", name="read_file", input={"path": "x"})],
),
ConversationMessage(
role="user",
content=[ToolResultBlock(tool_use_id="toolu_pair", content="ok", is_error=False)],
),
ConversationMessage(role="assistant", content=[TextBlock(text="done")]),
]
compacted = compact_messages(messages, preserve_recent=2)
assert any(
isinstance(block, ToolUseBlock) and block.id == "toolu_pair"
for message in compacted
for block in message.content
)
assert any(
isinstance(block, ToolResultBlock) and block.tool_use_id == "toolu_pair"
for message in compacted
for block in message.content
)
def test_compact_messages_drops_dangling_preserved_tool_use():
messages = [
ConversationMessage.from_user_text("first"),
ConversationMessage(role="assistant", content=[TextBlock(text="second")]),
ConversationMessage(
role="assistant",
content=[ToolUseBlock(id="toolu_orphan", name="edit_file", input={"path": "x"})],
),
]
compacted = compact_messages(messages, preserve_recent=1)
assert not any(
isinstance(block, ToolUseBlock) and block.id == "toolu_orphan"
for message in compacted
for block in message.content
)
class _CompactApiClient:
def __init__(self, responses):
self._responses = list(responses)
@@ -223,6 +270,70 @@ async def test_compact_conversation_runs_hooks_and_preserves_carryover_state(tmp
assert "41 passed" in joined
@pytest.mark.asyncio
async def test_compact_conversation_keeps_tool_pair_when_boundary_would_split_it():
messages = [
ConversationMessage.from_user_text("alpha"),
ConversationMessage(role="assistant", content=[TextBlock(text="beta")]),
ConversationMessage(role="user", content=[TextBlock(text="gamma")]),
ConversationMessage(
role="assistant",
content=[ToolUseBlock(id="toolu_pair", name="read_file", input={"path": "demo.txt"})],
),
ConversationMessage(
role="user",
content=[ToolResultBlock(tool_use_id="toolu_pair", content="contents", is_error=False)],
),
ConversationMessage(role="assistant", content=[TextBlock(text="used the tool")]),
ConversationMessage(role="user", content=[TextBlock(text="continue")]),
]
compacted = await compact_conversation(
messages,
api_client=_CompactApiClient(["<summary>condensed</summary>"]),
model="claude-test",
preserve_recent=3,
)
rebuilt = build_post_compact_messages(compacted)
pair_positions: list[tuple[int, str]] = []
for index, message in enumerate(rebuilt):
for block in message.content:
if isinstance(block, ToolUseBlock) and block.id == "toolu_pair":
pair_positions.append((index, "use"))
if isinstance(block, ToolResultBlock) and block.tool_use_id == "toolu_pair":
pair_positions.append((index, "result"))
assert pair_positions == [(2, "use"), (3, "result")]
@pytest.mark.asyncio
async def test_compact_conversation_drops_orphan_preserved_tool_use():
messages = [
ConversationMessage.from_user_text("alpha"),
ConversationMessage(role="assistant", content=[TextBlock(text="beta")]),
ConversationMessage(role="user", content=[TextBlock(text="gamma")]),
ConversationMessage(
role="assistant",
content=[ToolUseBlock(id="toolu_orphan", name="edit_file", input={"path": "demo.txt"})],
),
]
compacted = await compact_conversation(
messages,
api_client=_CompactApiClient(["<summary>condensed</summary>"]),
model="claude-test",
preserve_recent=1,
)
rebuilt = build_post_compact_messages(compacted)
assert not any(
isinstance(block, ToolUseBlock) and block.id == "toolu_orphan"
for message in rebuilt
for block in message.content
)
@pytest.mark.asyncio
async def test_compact_post_messages_keep_boundary_summary_recent_then_attachments():
messages = [