From ddca3a2f79eeb31ecf9720cbd8d8a124f06a3a9e Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 9 Jun 2026 10:45:45 +0000 Subject: [PATCH 1/4] HarnessAgent: Disable compaction when max tokens not provided --- .../core/agent_framework/_harness/_agent.py | 54 ++--- .../packages/core/agent_framework/_skills.py | 10 +- .../core/tests/core/test_harness_agent.py | 17 ++ .../core/tests/core/test_mcp_observability.py | 4 +- .../core/tests/core/test_mcp_skills.py | 190 ++++++++---------- python/samples/02-agents/harness/README.md | 14 +- 6 files changed, 141 insertions(+), 148 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 5896f72141..38a639bf77 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -65,18 +65,14 @@ def _assemble_instructions( def _assemble_compaction_provider( *, - disable_compaction: bool, max_context_window_tokens: int, max_output_tokens: int, history_source_id: str, before_compaction_strategy: CompactionStrategy | None, after_compaction_strategy: CompactionStrategy | None, tokenizer: TokenizerProtocol | None, -) -> CompactionProvider | None: +) -> CompactionProvider: """Build the compaction provider from parameters or defaults.""" - if disable_compaction: - return None - before_strategy = before_compaction_strategy or ContextWindowCompactionStrategy( max_context_window_tokens=max_context_window_tokens, max_output_tokens=max_output_tokens, @@ -157,8 +153,8 @@ def create_harness_agent( harness_instructions: str | None = None, agent_instructions: str | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, - max_context_window_tokens: int, - max_output_tokens: int, + max_context_window_tokens: int | None = None, + max_output_tokens: int | None = None, history_provider: HistoryProvider | None = None, disable_compaction: bool = False, before_compaction_strategy: CompactionStrategy | None = None, @@ -206,8 +202,6 @@ def create_harness_agent( agent = create_harness_agent( OpenAIChatClient(model="gpt-4o"), - max_context_window_tokens=128_000, - max_output_tokens=16_384, ) session = agent.create_session() response = await agent.run("Plan a weekend trip to Seattle", session=session) @@ -243,7 +237,10 @@ def create_harness_agent( (e.g., "You are a research assistant focused on academic sources."). tools: Additional tools to include in the agent's toolset. max_context_window_tokens: Maximum tokens the model's context window supports. + When None (default), compaction is automatically disabled. max_output_tokens: Maximum output tokens per response. + When None (default), compaction is automatically disabled and no + default max_tokens option is set. history_provider: Custom history provider. When None, an InMemoryHistoryProvider is used. disable_compaction: When True, skip compaction provider setup. before_compaction_strategy: Custom before-run compaction strategy. @@ -283,29 +280,35 @@ def create_harness_agent( A fully configured :class:`~agent_framework.Agent` instance. Raises: - ValueError: If max_context_window_tokens <= 0 or max_output_tokens < 0 - or max_output_tokens >= max_context_window_tokens. + ValueError: If max_context_window_tokens is provided and <= 0, or + max_output_tokens is provided and < 0, or max_output_tokens >= + max_context_window_tokens when both are provided. """ - if max_context_window_tokens <= 0: + if max_context_window_tokens is not None and max_context_window_tokens <= 0: raise ValueError("max_context_window_tokens must be positive.") - if max_output_tokens < 0: + if max_output_tokens is not None and max_output_tokens < 0: raise ValueError("max_output_tokens must be non-negative.") - if max_output_tokens >= max_context_window_tokens: + if ( + max_context_window_tokens is not None + and max_output_tokens is not None + and max_output_tokens >= max_context_window_tokens + ): raise ValueError("max_output_tokens must be less than max_context_window_tokens.") # Build history provider. resolved_history = history_provider or InMemoryHistoryProvider() - # Build compaction provider. - compaction_provider = _assemble_compaction_provider( - disable_compaction=disable_compaction, - max_context_window_tokens=max_context_window_tokens, - max_output_tokens=max_output_tokens, - history_source_id=resolved_history.source_id, - before_compaction_strategy=before_compaction_strategy, - after_compaction_strategy=after_compaction_strategy, - tokenizer=tokenizer, - ) + # Build compaction provider (disabled when token params are not provided). + compaction_provider: CompactionProvider | None = None + if not disable_compaction and max_context_window_tokens is not None and max_output_tokens is not None: + compaction_provider = _assemble_compaction_provider( + max_context_window_tokens=max_context_window_tokens, + max_output_tokens=max_output_tokens, + history_source_id=resolved_history.source_id, + before_compaction_strategy=before_compaction_strategy, + after_compaction_strategy=after_compaction_strategy, + tokenizer=tokenizer, + ) # Build context providers. assembled_providers = _assemble_context_providers( @@ -347,7 +350,8 @@ def create_harness_agent( # Build default options dict. default_opts: dict[str, Any] = dict(default_options) if default_options else {} - default_opts.setdefault("max_tokens", max_output_tokens) + if max_output_tokens is not None: + default_opts.setdefault("max_tokens", max_output_tokens) agent = Agent( client, diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 97afe66cea..91bdb61914 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -3516,9 +3516,7 @@ async def get_content(self) -> str: result = await self._client.read_resource(_mcp_any_url(self._skill_md_uri)) text = _mcp_join_text(result) if not text: - raise ValueError( - f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'." - ) + raise ValueError(f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'.") self._content = text return text @@ -3572,11 +3570,7 @@ def _validate_resource_name(name: str) -> str | None: or ``None`` if the name is unsafe. """ normalized = name.replace("\\", "/") - if ( - normalized.startswith("/") - or "://" in normalized - or any(seg == ".." for seg in normalized.split("/")) - ): + if normalized.startswith("/") or "://" in normalized or any(seg == ".." for seg in normalized.split("/")): logger.debug("Rejecting resource name with unsafe path components: %r", name) return None return normalized diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 58ef3f5f2d..446d9f2fd3 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -194,6 +194,23 @@ def test_create_harness_agent_returns_full_agent() -> None: assert isinstance(agent, FullAgent) +def test_create_harness_agent_no_token_params_disables_compaction() -> None: + """When token params are omitted, compaction is automatically disabled.""" + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + ) + provider_types = [type(p) for p in agent.context_providers] + assert CompactionProvider not in provider_types + + +def test_create_harness_agent_no_token_params_skips_max_tokens_option() -> None: + """When max_output_tokens is omitted, max_tokens should not be set in default options.""" + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + ) + assert agent.default_options.get("max_tokens") is None + + # --- Validation Tests --- diff --git a/python/packages/core/tests/core/test_mcp_observability.py b/python/packages/core/tests/core/test_mcp_observability.py index 226e976120..8b17f3dc88 100644 --- a/python/packages/core/tests/core/test_mcp_observability.py +++ b/python/packages/core/tests/core/test_mcp_observability.py @@ -281,9 +281,7 @@ async def test_mcp_prompts_get_creates_client_span(span_exporter: InMemorySpanEx async def test_mcp_prompts_get_mcp_error_sets_error_type(span_exporter: InMemorySpanExporter): """When session.get_prompt() raises McpError, the span should have error.type and ERROR status.""" tool = _make_connected_mcp_tool() - tool.session.get_prompt = AsyncMock( - side_effect=McpError(ErrorData(code=-32602, message="prompt not found")) - ) + tool.session.get_prompt = AsyncMock(side_effect=McpError(ErrorData(code=-32602, message="prompt not found"))) span_exporter.clear() with pytest.raises(ToolExecutionException): diff --git a/python/packages/core/tests/core/test_mcp_skills.py b/python/packages/core/tests/core/test_mcp_skills.py index 3e7c67662a..74993997d0 100644 --- a/python/packages/core/tests/core/test_mcp_skills.py +++ b/python/packages/core/tests/core/test_mcp_skills.py @@ -35,26 +35,22 @@ Body content here. """ -SAMPLE_SKILL_INDEX = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://unit-converter/SKILL.md", - } - ], - } -) +SAMPLE_SKILL_INDEX = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://unit-converter/SKILL.md", + } + ], +}) def _make_text_result(text: str, uri: str = "skill://test") -> ReadResourceResult: """Create a ReadResourceResult with a single TextResourceContents.""" - return ReadResourceResult( - contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")] - ) + return ReadResourceResult(contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")]) def _make_blob_result( @@ -230,12 +226,10 @@ async def test_get_content_raises_on_empty(self) -> None: @pytest.mark.asyncio async def test_get_resource_text(self) -> None: - client = _make_client( - **{ - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), - } - ) + client = _make_client(**{ + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), + }) from agent_framework import SkillFrontmatter fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.") @@ -249,12 +243,10 @@ async def test_get_resource_text(self) -> None: @pytest.mark.asyncio async def test_get_resource_binary(self) -> None: data = bytes([0x01, 0x02, 0x03, 0x04]) - client = _make_client( - **{ - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/assets/icon.bin": _make_blob_result(data), - } - ) + client = _make_client(**{ + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/assets/icon.bin": _make_blob_result(data), + }) from agent_framework import SkillFrontmatter fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.") @@ -345,12 +337,10 @@ class TestMCPSkillsSource: @pytest.mark.asyncio async def test_index_based_discovery_returns_skill(self) -> None: - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + }) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -373,9 +363,7 @@ async def test_no_index_returns_empty(self) -> None: async def test_does_not_read_skill_md_during_discovery(self) -> None: # Index points to a skill, but SKILL.md is not registered on the server. # Discovery should succeed because it only reads the index. - client = _make_client( - **{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -384,19 +372,17 @@ async def test_does_not_read_skill_md_during_discovery(self) -> None: @pytest.mark.asyncio async def test_invalid_name_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "UnitConverter", # Invalid: uppercase - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://UnitConverter/SKILL.md", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "UnitConverter", # Invalid: uppercase + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://UnitConverter/SKILL.md", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -404,18 +390,16 @@ async def test_invalid_name_is_skipped(self) -> None: @pytest.mark.asyncio async def test_missing_required_fields_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - # Missing description and url - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + # Missing description and url + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -423,19 +407,17 @@ async def test_missing_required_fields_is_skipped(self) -> None: @pytest.mark.asyncio async def test_unsupported_type_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "some-skill", - "type": "archive", - "description": "Packaged skill.", - "url": "skill://some-skill.tar.gz", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "some-skill", + "type": "archive", + "description": "Packaged skill.", + "url": "skill://some-skill.tar.gz", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -443,18 +425,16 @@ async def test_unsupported_type_is_skipped(self) -> None: @pytest.mark.asyncio async def test_template_type_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "type": "mcp-resource-template", - "description": "Per-product documentation skill", - "url": "skill://docs/{product}/SKILL.md", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "type": "mcp-resource-template", + "description": "Per-product documentation skill", + "url": "skill://docs/{product}/SKILL.md", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -462,31 +442,25 @@ async def test_template_type_is_skipped(self) -> None: @pytest.mark.asyncio async def test_empty_index_returns_empty(self) -> None: - client = _make_client( - **{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() assert skills == [] @pytest.mark.asyncio async def test_malformed_index_json_returns_empty(self) -> None: - client = _make_client( - **{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() assert skills == [] @pytest.mark.asyncio async def test_sibling_text_resource(self) -> None: - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), + }) source = MCPSkillsSource(client=client) skill = (await source.get_skills())[0] resource = await skill.get_resource("references/checklist.md") @@ -497,13 +471,11 @@ async def test_sibling_text_resource(self) -> None: @pytest.mark.asyncio async def test_sibling_binary_resource(self) -> None: data = bytes([0x01, 0x02, 0x03, 0x04]) - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/assets/icon.bin": _make_blob_result(data), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/assets/icon.bin": _make_blob_result(data), + }) source = MCPSkillsSource(client=client) skill = (await source.get_skills())[0] resource = await skill.get_resource("assets/icon.bin") @@ -649,9 +621,7 @@ async def test_get_resource_generic_mcp_error_propagates(self) -> None: from agent_framework import SkillFrontmatter client = AsyncMock() - client.read_resource = AsyncMock( - side_effect=McpError(error=ErrorData(code=0, message="Handler error")) - ) + client.read_resource = AsyncMock(side_effect=McpError(error=ErrorData(code=0, message="Handler error"))) fm = SkillFrontmatter(name="test-skill", description="Test.") skill = MCPSkill(frontmatter=fm, skill_md_uri="skill://test/SKILL.md", client=client) with pytest.raises(McpError): diff --git a/python/samples/02-agents/harness/README.md b/python/samples/02-agents/harness/README.md index 3bf0f09110..15424e1422 100644 --- a/python/samples/02-agents/harness/README.md +++ b/python/samples/02-agents/harness/README.md @@ -45,13 +45,23 @@ python samples/02-agents/harness/harness_research.py ### Minimal Setup -`create_harness_agent` requires only a chat client and token budget parameters: +`create_harness_agent` requires only a chat client: ```python from agent_framework import create_harness_agent from agent_framework.foundry import FoundryChatClient from azure.identity import AzureCliCredential +agent = create_harness_agent( + client=FoundryChatClient(credential=AzureCliCredential()), +) +``` + +### With Compaction + +Provide token budget parameters to enable automatic context-window compaction: + +```python agent = create_harness_agent( client=FoundryChatClient(credential=AzureCliCredential()), max_context_window_tokens=128_000, @@ -59,7 +69,7 @@ agent = create_harness_agent( ) ``` -### Customization +### Further Customization Disable or customize any feature: From bc43fb39ef15611d4d2595f2c1401d3f32257f79 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 9 Jun 2026 10:58:38 +0000 Subject: [PATCH 2/4] Fix regression. --- .../core/agent_framework/_harness/_agent.py | 69 ++++++++++++------- .../core/tests/core/test_harness_agent.py | 25 +++++++ 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 38a639bf77..dcbf4459e4 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -65,19 +65,35 @@ def _assemble_instructions( def _assemble_compaction_provider( *, - max_context_window_tokens: int, - max_output_tokens: int, + disable_compaction: bool, + max_context_window_tokens: int | None, + max_output_tokens: int | None, history_source_id: str, before_compaction_strategy: CompactionStrategy | None, after_compaction_strategy: CompactionStrategy | None, tokenizer: TokenizerProtocol | None, -) -> CompactionProvider: - """Build the compaction provider from parameters or defaults.""" - before_strategy = before_compaction_strategy or ContextWindowCompactionStrategy( - max_context_window_tokens=max_context_window_tokens, - max_output_tokens=max_output_tokens, - tokenizer=tokenizer, - ) +) -> CompactionProvider | None: + """Build the compaction provider from parameters or defaults. + + Returns None when compaction is explicitly disabled, or when no before-strategy can be + resolved (no custom ``before_compaction_strategy`` and no token budget to build the default). + """ + if disable_compaction: + return None + + # A user-supplied strategy is used as-is; otherwise fall back to the token-budget-aware + # default, which requires the token params. + before_strategy = before_compaction_strategy + if before_strategy is None and max_context_window_tokens is not None and max_output_tokens is not None: + before_strategy = ContextWindowCompactionStrategy( + max_context_window_tokens=max_context_window_tokens, + max_output_tokens=max_output_tokens, + tokenizer=tokenizer, + ) + + if before_strategy is None: + return None + after_strategy = after_compaction_strategy or ToolResultCompactionStrategy(keep_last_tool_call_groups=2) return CompactionProvider( @@ -237,14 +253,18 @@ def create_harness_agent( (e.g., "You are a research assistant focused on academic sources."). tools: Additional tools to include in the agent's toolset. max_context_window_tokens: Maximum tokens the model's context window supports. - When None (default), compaction is automatically disabled. + Used to construct the default token-budget-aware compaction strategy. When None + (default) and no ``before_compaction_strategy`` is provided, compaction is + automatically disabled. max_output_tokens: Maximum output tokens per response. - When None (default), compaction is automatically disabled and no - default max_tokens option is set. + Used to construct the default compaction strategy and sets a default max_tokens + chat option. When None (default), no default max_tokens option is set, and if no + ``before_compaction_strategy`` is provided, compaction is automatically disabled. history_provider: Custom history provider. When None, an InMemoryHistoryProvider is used. disable_compaction: When True, skip compaction provider setup. - before_compaction_strategy: Custom before-run compaction strategy. - Defaults to ContextWindowCompactionStrategy (token-budget aware). + before_compaction_strategy: Custom before-run compaction strategy. When provided, + compaction runs even if token params are omitted. Defaults to + ContextWindowCompactionStrategy (token-budget aware), which requires the token params. after_compaction_strategy: Custom after-run compaction strategy. Defaults to ToolResultCompactionStrategy. tokenizer: Custom tokenizer for compaction strategies. @@ -298,17 +318,16 @@ def create_harness_agent( # Build history provider. resolved_history = history_provider or InMemoryHistoryProvider() - # Build compaction provider (disabled when token params are not provided). - compaction_provider: CompactionProvider | None = None - if not disable_compaction and max_context_window_tokens is not None and max_output_tokens is not None: - compaction_provider = _assemble_compaction_provider( - max_context_window_tokens=max_context_window_tokens, - max_output_tokens=max_output_tokens, - history_source_id=resolved_history.source_id, - before_compaction_strategy=before_compaction_strategy, - after_compaction_strategy=after_compaction_strategy, - tokenizer=tokenizer, - ) + # Build compaction provider. + compaction_provider = _assemble_compaction_provider( + disable_compaction=disable_compaction, + max_context_window_tokens=max_context_window_tokens, + max_output_tokens=max_output_tokens, + history_source_id=resolved_history.source_id, + before_compaction_strategy=before_compaction_strategy, + after_compaction_strategy=after_compaction_strategy, + tokenizer=tokenizer, + ) # Build context providers. assembled_providers = _assemble_context_providers( diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 446d9f2fd3..f2cb73d491 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -211,6 +211,31 @@ def test_create_harness_agent_no_token_params_skips_max_tokens_option() -> None: assert agent.default_options.get("max_tokens") is None +def test_create_harness_agent_custom_before_strategy_enables_compaction_without_tokens() -> None: + """A custom before_compaction_strategy enables compaction even when token params are omitted.""" + from agent_framework import ToolResultCompactionStrategy + + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + before_compaction_strategy=ToolResultCompactionStrategy(), + ) + provider_types = [type(p) for p in agent.context_providers] + assert CompactionProvider in provider_types + + +def test_create_harness_agent_disable_compaction_overrides_custom_before_strategy() -> None: + """disable_compaction=True wins even when a custom before strategy is provided.""" + from agent_framework import ToolResultCompactionStrategy + + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + before_compaction_strategy=ToolResultCompactionStrategy(), + disable_compaction=True, + ) + provider_types = [type(p) for p in agent.context_providers] + assert CompactionProvider not in provider_types + + # --- Validation Tests --- From 6b011b52aedd16a9865c069466bd1d41fcbefe4e Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 9 Jun 2026 11:19:29 +0000 Subject: [PATCH 3/4] Address PR comments --- .../core/agent_framework/_harness/_agent.py | 43 ++++++++++++------- .../core/tests/core/test_harness_agent.py | 15 +++++++ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index dcbf4459e4..5d813f99d6 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -75,14 +75,19 @@ def _assemble_compaction_provider( ) -> CompactionProvider | None: """Build the compaction provider from parameters or defaults. - Returns None when compaction is explicitly disabled, or when no before-strategy can be - resolved (no custom ``before_compaction_strategy`` and no token budget to build the default). + The token-budget defaults (``ContextWindowCompactionStrategy`` for the before phase and + ``ToolResultCompactionStrategy`` for the after phase) are only applied when the token + params are provided. Caller-supplied strategies are always honored. Either phase may end + up ``None``, which ``CompactionProvider`` interprets as "skip that phase". + + Returns None when compaction is explicitly disabled, or when neither phase has a strategy + (no custom strategies and no token budget to build the defaults). """ if disable_compaction: return None - # A user-supplied strategy is used as-is; otherwise fall back to the token-budget-aware - # default, which requires the token params. + # Resolve the before-strategy: custom strategy wins; otherwise fall back to the + # token-budget-aware default when token params are available. before_strategy = before_compaction_strategy if before_strategy is None and max_context_window_tokens is not None and max_output_tokens is not None: before_strategy = ContextWindowCompactionStrategy( @@ -91,10 +96,15 @@ def _assemble_compaction_provider( tokenizer=tokenizer, ) - if before_strategy is None: - return None + # Resolve the after-strategy: custom strategy wins; otherwise fall back to the default + # when token params are available. + after_strategy = after_compaction_strategy + if after_strategy is None and max_context_window_tokens is not None and max_output_tokens is not None: + after_strategy = ToolResultCompactionStrategy(keep_last_tool_call_groups=2) - after_strategy = after_compaction_strategy or ToolResultCompactionStrategy(keep_last_tool_call_groups=2) + # Nothing to compact in either phase: skip the provider entirely. + if before_strategy is None and after_strategy is None: + return None return CompactionProvider( before_strategy=before_strategy, @@ -253,20 +263,21 @@ def create_harness_agent( (e.g., "You are a research assistant focused on academic sources."). tools: Additional tools to include in the agent's toolset. max_context_window_tokens: Maximum tokens the model's context window supports. - Used to construct the default token-budget-aware compaction strategy. When None - (default) and no ``before_compaction_strategy`` is provided, compaction is - automatically disabled. + Used to construct the default token-budget-aware compaction strategies. When None + (default) and no custom ``before_compaction_strategy`` / ``after_compaction_strategy`` + is provided, compaction is automatically disabled. max_output_tokens: Maximum output tokens per response. - Used to construct the default compaction strategy and sets a default max_tokens - chat option. When None (default), no default max_tokens option is set, and if no - ``before_compaction_strategy`` is provided, compaction is automatically disabled. + Used to construct the default compaction strategies and sets a default max_tokens + chat option. When None (default), no default max_tokens option is set, and unless a + custom compaction strategy is provided, compaction is automatically disabled. history_provider: Custom history provider. When None, an InMemoryHistoryProvider is used. disable_compaction: When True, skip compaction provider setup. before_compaction_strategy: Custom before-run compaction strategy. When provided, compaction runs even if token params are omitted. Defaults to - ContextWindowCompactionStrategy (token-budget aware), which requires the token params. - after_compaction_strategy: Custom after-run compaction strategy. - Defaults to ToolResultCompactionStrategy. + ContextWindowCompactionStrategy (token-budget aware) when token params are provided. + after_compaction_strategy: Custom after-run compaction strategy. When provided, + compaction runs even if token params are omitted. Defaults to + ToolResultCompactionStrategy when token params are provided. tokenizer: Custom tokenizer for compaction strategies. disable_todo: When True, skip the TodoProvider. todo_provider: Custom TodoProvider instance. Ignored when disable_todo is True. diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index f2cb73d491..512546157e 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -236,6 +236,21 @@ def test_create_harness_agent_disable_compaction_overrides_custom_before_strateg assert CompactionProvider not in provider_types +def test_create_harness_agent_custom_after_strategy_enables_compaction_without_tokens() -> None: + """A custom after_compaction_strategy enables compaction even when token params are omitted.""" + from agent_framework import ToolResultCompactionStrategy + + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + after_compaction_strategy=ToolResultCompactionStrategy(), + ) + compaction_providers = [p for p in agent.context_providers if isinstance(p, CompactionProvider)] + assert len(compaction_providers) == 1 + # Before phase is skipped (no token budget, no custom before strategy), after phase is set. + assert compaction_providers[0].before_strategy is None + assert compaction_providers[0].after_strategy is not None + + # --- Validation Tests --- From cefef16c7a0c36fe9604c482ef5408e865f94dd5 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 9 Jun 2026 17:11:42 +0000 Subject: [PATCH 4/4] Require max_output_tokens to be positive Reject max_output_tokens=0 (must be positive), mirroring max_context_window_tokens. Addresses PR review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core/agent_framework/_harness/_agent.py | 6 +++--- .../core/tests/core/test_harness_agent.py | 17 +++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 5d813f99d6..0ae0c73032 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -312,13 +312,13 @@ def create_harness_agent( Raises: ValueError: If max_context_window_tokens is provided and <= 0, or - max_output_tokens is provided and < 0, or max_output_tokens >= + max_output_tokens is provided and <= 0, or max_output_tokens >= max_context_window_tokens when both are provided. """ if max_context_window_tokens is not None and max_context_window_tokens <= 0: raise ValueError("max_context_window_tokens must be positive.") - if max_output_tokens is not None and max_output_tokens < 0: - raise ValueError("max_output_tokens must be non-negative.") + if max_output_tokens is not None and max_output_tokens <= 0: + raise ValueError("max_output_tokens must be positive.") if ( max_context_window_tokens is not None and max_output_tokens is not None diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 512546157e..7da1bdbf36 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -264,14 +264,15 @@ def test_create_harness_agent_rejects_invalid_context_tokens() -> None: ) -def test_create_harness_agent_rejects_negative_output_tokens() -> None: - """max_output_tokens must be non-negative.""" - with pytest.raises(ValueError, match="max_output_tokens must be non-negative"): - create_harness_agent( - client=_FakeChatClient(), # type: ignore[arg-type] - max_context_window_tokens=1000, - max_output_tokens=-1, - ) +def test_create_harness_agent_rejects_non_positive_output_tokens() -> None: + """max_output_tokens must be positive when provided.""" + for invalid_value in (0, -1): + with pytest.raises(ValueError, match="max_output_tokens must be positive"): + create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=1000, + max_output_tokens=invalid_value, + ) def test_create_harness_agent_rejects_output_gte_context() -> None: