Skip to content
Open
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
11 changes: 11 additions & 0 deletions openhands-sdk/openhands/sdk/subagent/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,21 @@ def _factory(llm: "LLM") -> "Agent":
)
tools.append(Tool(name=tool_name))

# Build condenser if configured
condenser = None
if agent_def.condenser:
from openhands.sdk.context.condenser import LLMSummarizingCondenser

condenser = LLMSummarizingCondenser(
llm=llm.model_copy(update={"usage_id": "condenser"}),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do worry a little bit that the more of these we do the more we'll have to sort out... what if we sent a profile name, or think about how this could be like to replace these LLMs with profiles references if that makes sense?

Though the condenser is a pretty obvious place, maybe not exactly to worry much about

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah solution would be to save a condenser LLM profile that we use just for that and we go on the default one if there is no one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! That sounds great to me, or even, if possible, read the condenser profile at the start of the conversation, if none initialize one from the other

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I think this should be addressed in another PR, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, the more model_copy(update...) we add to the codebase, the more we add tech debt and need to fix it

But I also see the opposite argument here. I'm just starting to feel we're adding too many, though I could be wrong

**agent_def.condenser,
)

return Agent(
llm=llm,
tools=tools,
agent_context=agent_context,
condenser=condenser,
)

return _factory
Expand Down
40 changes: 40 additions & 0 deletions openhands-sdk/openhands/sdk/subagent/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"skills",
"max_iteration_per_run",
"profile_store_dir",
"condenser",
}


Expand Down Expand Up @@ -57,6 +58,35 @@ def _extract_skills(fm: dict[str, object]) -> list[str]:
return skills


_CONDENSER_VALID_KEYS: Final[set[str]] = {
"max_size",
"max_tokens",
"keep_first",
"minimum_progress",
"hard_context_reset_max_retries",
"hard_context_reset_context_scaling",
}


def _extract_condenser(fm: dict[str, object]) -> dict[str, Any] | None:
"""Extract condenser configuration from frontmatter."""
condenser_raw = fm.get("condenser")
if condenser_raw is None:
return None
if not isinstance(condenser_raw, dict):
raise ValueError(
f"condenser must be a mapping of configuration parameters, "
f"got {type(condenser_raw)}"
)
unknown_keys = set(condenser_raw.keys()) - _CONDENSER_VALID_KEYS
if unknown_keys:
raise ValueError(
f"Unknown condenser parameter(s): {sorted(unknown_keys)}. "
f"Valid parameters are: {sorted(_CONDENSER_VALID_KEYS)}"
)
return condenser_raw


def _extract_profile_store_dir(fm: dict[str, object]) -> str | None:
"""Extract profile store directory from frontmatter."""
profile_store_dir_raw = fm.get("profile_store_dir")
Expand Down Expand Up @@ -121,6 +151,13 @@ class AgentDefinition(BaseModel):
"It must be strictly positive, or None for default.",
gt=0,
)
condenser: dict[str, Any] | None = Field(
default=None,
description="Condenser configuration for this agent. "
"Parameters are passed to LLMSummarizingCondenser (e.g., max_size, ...). "
"The condenser LLM is inherited from the agent's LLM.",
examples=[{"max_size": 100, "keep_first": 2}],
)
profile_store_dir: str | None = Field(
default=None,
description="Path to the directory where LLM profiles are stored. "
Expand All @@ -139,6 +176,7 @@ def load(cls, agent_path: Path) -> AgentDefinition:
- description: Description with optional <example> tags for triggering
- tools (optional): List of allowed tools
- skills (optional): Comma-separated skill names or list of skill names
- condenser (optional): Condenser configuration (e.g., max_size, keep_first)
- model (optional): Model profile to use (default: 'inherit')
- color (optional): Display color
- max_iterations_per_run: Max iteration per run
Expand All @@ -165,6 +203,7 @@ def load(cls, agent_path: Path) -> AgentDefinition:
tools: list[str] = _extract_tools(fm)
skills: list[str] = _extract_skills(fm)
max_iteration_per_run: int | None = _extract_max_iteration_per_run(fm)
condenser: dict[str, Any] | None = _extract_condenser(fm)
profile_store_dir: str | None = _extract_profile_store_dir(fm)

# Extract whenToUse examples from description
Expand All @@ -181,6 +220,7 @@ def load(cls, agent_path: Path) -> AgentDefinition:
tools=tools,
skills=skills,
max_iteration_per_run=max_iteration_per_run,
condenser=condenser,
profile_store_dir=profile_store_dir,
system_prompt=content,
source=str(agent_path),
Expand Down
113 changes: 112 additions & 1 deletion tests/sdk/subagent/test_subagent_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
import pytest
from pydantic import ValidationError

from openhands.sdk.subagent.schema import AgentDefinition, _extract_examples
from openhands.sdk.subagent.schema import (
_CONDENSER_VALID_KEYS,
AgentDefinition,
_extract_examples,
)


class TestAgentDefinition:
Expand Down Expand Up @@ -319,6 +323,113 @@ def test_profile_store_dir_default_none(self):
agent = AgentDefinition(name="test")
assert agent.profile_store_dir is None

def test_condenser_default_none(self):
"""Test that condenser defaults to None on direct construction."""
agent = AgentDefinition(name="test")
assert agent.condenser is None

def test_condenser_as_dict(self):
"""Test creating AgentDefinition with condenser as dict."""
config = {"max_size": 100, "keep_first": 2}
agent = AgentDefinition(name="condensed-agent", condenser=config)
assert agent.condenser == config

def test_load_condenser_from_frontmatter(self, tmp_path: Path):
"""Test loading condenser from YAML frontmatter."""
agent_md = tmp_path / "condensed.md"
agent_md.write_text(
"""---
name: condensed-agent
condenser:
max_size: 100
keep_first: 3
---

You are an agent with condensation.
"""
)

agent = AgentDefinition.load(agent_md)
assert agent.condenser is not None
assert agent.condenser["max_size"] == 100
assert agent.condenser["keep_first"] == 3

def test_load_condenser_not_in_metadata(self, tmp_path: Path):
"""Test that condenser doesn't leak into metadata."""
agent_md = tmp_path / "agent.md"
agent_md.write_text(
"""---
name: agent
condenser:
max_size: 50
custom_field: value
---

Prompt.
"""
)
agent = AgentDefinition.load(agent_md)
assert "condenser" not in agent.metadata
assert agent.metadata.get("custom_field") == "value"

def test_load_condenser_non_dict_raises(self, tmp_path: Path):
"""Test that non-dict condenser value raises ValueError."""
agent_md = tmp_path / "bad-condenser.md"
agent_md.write_text(
"""---
name: bad-condenser
condenser: true
---

Prompt.
"""
)
with pytest.raises(ValueError, match="must be a mapping"):
AgentDefinition.load(agent_md)

def test_load_condenser_unknown_key_raises(self, tmp_path: Path):
"""Test that unknown condenser parameters raise ValueError."""
agent_md = tmp_path / "bad-condenser.md"
agent_md.write_text(
"""---
name: bad-condenser
condenser:
max_size: 100
bogus_param: 42
---

Prompt.
"""
)
with pytest.raises(ValueError, match="Unknown condenser parameter"):
AgentDefinition.load(agent_md)

def test_load_without_condenser(self, tmp_path: Path):
"""Test that loading from file without condenser gives None."""
agent_md = tmp_path / "agent.md"
agent_md.write_text(
"""---
name: no-condenser
---

Prompt.
"""
)
agent = AgentDefinition.load(agent_md)
assert agent.condenser is None

def test_condenser_valid_keys_match_llm_summarizing_condenser(self):
"""Ensure _CONDENSER_VALID_KEYS stays in sync with LLMSummarizingCondenser."""
from openhands.sdk.context.condenser import LLMSummarizingCondenser

# Get all user-configurable fields (exclude 'llm' which is inherited)
condenser_fields = set(LLMSummarizingCondenser.model_fields.keys()) - {"llm"}
assert _CONDENSER_VALID_KEYS == condenser_fields, (
f"_CONDENSER_VALID_KEYS is out of sync with LLMSummarizingCondenser. "
f"Missing: {condenser_fields - _CONDENSER_VALID_KEYS}, "
f"Extra: {_CONDENSER_VALID_KEYS - condenser_fields}"
)


class TestExtractExamples:
"""Tests for _extract_examples function."""
Expand Down
Loading