Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
from openhands.sdk.event import (
ActionEvent,
AgentErrorEvent,
Condensation,
CondensationRequest,
Event,
EventID,
Expand Down Expand Up @@ -454,7 +455,9 @@ def _default_callback(e):
# This runs on first run()/send_message() call and handles both
# explicit hooks and plugin hooks in one place
self._hook_processor = None
self._on_event = self._tree_stamping(self._rules_injecting(base_callback))
self._on_event = self._tree_stamping(
self._activation_state_syncing(self._rules_injecting(base_callback))
)
self._on_token = (
BaseConversation.compose_callbacks(token_callbacks)
if token_callbacks
Expand Down Expand Up @@ -555,6 +558,51 @@ def wrapped(event: Event) -> None:

return cast(ConversationCallbackType, wrapped)

def _activation_state_syncing(
self, inner: ConversationCallbackType
) -> ConversationCallbackType:
"""Wrap a callback to rebuild trigger state after condensation."""

def wrapped(event: Event) -> None:
inner(event)
if isinstance(event, Condensation):
self._rebuild_skill_activation_state()

return cast(ConversationCallbackType, wrapped)

def _rebuild_skill_activation_state(self) -> None:
"""Rebuild trigger deduplication state from the current conversation view."""
agent_context = self.agent.agent_context
activated_knowledge_skills: list[str] = []
activated_path_rules: list[str] = []

for event in self._state.view.events:
if isinstance(event, MessageEvent):
activated_names = event.activated_skills
elif isinstance(event, ObservationEvent) and event.extended_content:
if agent_context is None:
continue
file_path = self._touched_rule_path(event)
if file_path is None:
continue
result = agent_context.get_tool_use_suffix(
file_path=file_path,
skip_skill_names=[],
)
activated_names = result[1] if result is not None else []
else:
continue

for name in activated_names:
if isinstance(event, MessageEvent):
if name not in activated_knowledge_skills:
activated_knowledge_skills.append(name)
elif name not in activated_path_rules:
activated_path_rules.append(name)

self._state.activated_knowledge_skills = activated_knowledge_skills
self._state.activated_path_rules = activated_path_rules

def _maybe_inject_path_rules(self, event: Event) -> Event:
"""Return ``event`` with matching path-rule content, or unchanged.

Expand Down Expand Up @@ -1250,7 +1298,9 @@ def _ensure_plugins_loaded(self) -> None:
visualizer=self._visualizer,
conversation_stats=self._state.stats,
)
self._on_event = self._tree_stamping(self._rules_injecting(raw_on_event))
self._on_event = self._tree_stamping(
self._activation_state_syncing(self._rules_injecting(raw_on_event))
)
self._hook_processor.set_conversation_state(self._state)
self._hook_processor.run_session_start()

Expand Down Expand Up @@ -1323,7 +1373,9 @@ def _merge_runtime_plugin_hooks(self, plugin_hooks: HookConfig) -> None:
visualizer=self._visualizer,
conversation_stats=self._state.stats,
)
self._on_event = self._tree_stamping(self._rules_injecting(raw_on_event))
self._on_event = self._tree_stamping(
self._activation_state_syncing(self._rules_injecting(raw_on_event))
)
self._hook_processor.set_conversation_state(self._state)
self._hook_processor.run_session_start()

Expand Down Expand Up @@ -1847,9 +1899,6 @@ def send_message(self, message: str | Message, sender: str | None = None) -> Non
# We skip skills that were already activated
skip_skill_names=self._state.activated_knowledge_skills,
)
# TODO(calvin): we need to update
# self._state.activated_knowledge_skills
# so condenser can work
if ctx:
content, activated_skill_names = ctx
logger.debug(
Expand Down
170 changes: 170 additions & 0 deletions tests/sdk/conversation/test_triggered_skills_after_condensation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
"""Regression tests for trigger-based skills across conversation condensation."""

from pathlib import Path

from openhands.sdk.agent import Agent
from openhands.sdk.context.agent_context import AgentContext
from openhands.sdk.context.condenser import LLMSummarizingCondenser
from openhands.sdk.conversation import LocalConversation
from openhands.sdk.event import ActionEvent, MessageEvent, ObservationEvent
from openhands.sdk.llm import Message, MessageToolCall, TextContent
from openhands.sdk.skills import KeywordTrigger, PathTrigger, Skill
from openhands.sdk.testing import TestLLM
from openhands.sdk.tool.builtins.finish import FinishObservation
from openhands.sdk.tool.schema import Action


class _CondensationFileAction(Action):
path: str
command: str = "view"


def _message(text: str) -> Message:
return Message(role="assistant", content=[TextContent(text=text)])


def _conversation(tmp_path: Path, skills: list[Skill]) -> LocalConversation:
agent = Agent(
llm=TestLLM.from_messages([_message("agent response")]),
condenser=LLMSummarizingCondenser(
llm=TestLLM.from_messages([_message("condensed history")]),
max_size=20,
keep_first=2,
),
tools=[],
include_default_tools=[],
agent_context=AgentContext(skills=skills),
)
return LocalConversation(
agent=agent,
workspace=tmp_path,
persistence_dir=tmp_path / "conversation",
delete_on_close=True,
)


def _append_path_observation(
conversation: LocalConversation, path: Path, tool_call_id: str
) -> ObservationEvent:
action = ActionEvent(
thought=[TextContent(text="inspect file")],
action=_CondensationFileAction(path=str(path)),
tool_name="file_editor",
tool_call_id=tool_call_id,
tool_call=MessageToolCall(
id=tool_call_id,
name="file_editor",
arguments="{}",
origin="completion",
),
llm_response_id=f"response-{tool_call_id}",
source="agent",
)
observation = ObservationEvent(
observation=FinishObservation(content=[TextContent(text="file contents")]),
action_id=action.id,
tool_name=action.tool_name,
tool_call_id=tool_call_id,
)
with conversation._state:
conversation._on_event(action)
conversation._on_event(observation)

persisted = [
event
for event in conversation.state.events
if isinstance(event, ObservationEvent) and event.action_id == action.id
]
assert persisted
return persisted[-1]


def test_keyword_skill_can_reactivate_after_its_event_is_condensed(
tmp_path: Path,
) -> None:
skill = Skill(
name="python_tips",
content="Prefer small, focused Python functions.",
trigger=KeywordTrigger(keywords=["python"]),
)
conversation = _conversation(tmp_path, [skill])
try:
for index in range(5):
conversation.send_message(f"prefix message {index}")
conversation.send_message("Show me a python example")
triggered_event = conversation.state.events[-1]
assert isinstance(triggered_event, MessageEvent)
assert triggered_event.activated_skills == ["python_tips"]

conversation.send_message("Show me another python example")
deduped_event = conversation.state.events[-1]
assert isinstance(deduped_event, MessageEvent)
assert deduped_event.activated_skills == []

for index in range(10):
conversation.send_message(f"tail message {index}")

conversation.condense()

assert triggered_event.id not in {
event.id for event in conversation.state.view.events
}
assert conversation.state.activated_knowledge_skills == []

conversation.send_message("Show me another python example")
retriggered_event = conversation.state.events[-1]
assert isinstance(retriggered_event, MessageEvent)
assert retriggered_event.activated_skills == ["python_tips"]
assert any(
"Prefer small, focused Python functions." in content.text
for content in retriggered_event.extended_content
)
finally:
conversation.close()


def test_path_rule_can_reactivate_after_its_event_is_condensed(
tmp_path: Path,
) -> None:
rule = Skill(
name="typescript_rules",
content="Keep TypeScript modules narrowly scoped.",
trigger=PathTrigger(paths=["src/**/*.ts"]),
)
conversation = _conversation(tmp_path, [rule])
try:
for index in range(6):
conversation.send_message(f"prefix message {index}")
first_observation = _append_path_observation(
conversation, tmp_path / "src" / "app.ts", "first-call"
)
assert any(
"Keep TypeScript modules narrowly scoped." in content.text
for content in first_observation.extended_content
)

deduped_observation = _append_path_observation(
conversation, tmp_path / "src" / "second.ts", "deduped-call"
)
assert deduped_observation.extended_content == []

for index in range(10):
conversation.send_message(f"tail message {index}")

conversation.condense()

assert first_observation.id not in {
event.id for event in conversation.state.view.events
}
assert conversation.state.activated_path_rules == []

second_observation = _append_path_observation(
conversation, tmp_path / "src" / "another.ts", "second-call"
)
assert any(
"Keep TypeScript modules narrowly scoped." in content.text
for content in second_observation.extended_content
)
assert conversation.state.activated_path_rules == ["typescript_rules"]
finally:
conversation.close()
Loading