.NET: Remove required token params from HarnessAgent, make compaction opt-in#6409
Conversation
Remove the required maxContextWindowTokens and maxOutputTokens constructor parameters from HarnessAgent and AsHarnessAgent, replacing them with optional MaxContextWindowTokens and MaxOutputTokens properties on HarnessAgentOptions. When both values are provided, compaction is enabled as before (in-loop CompactionProvider and chat reducer on the default InMemoryChatHistory Provider). When either is null, compaction is disabled entirely, making it opt-in. New constructor: HarnessAgent(IChatClient, HarnessAgentOptions?, ILoggerFactory?, IServiceProvider?) Closes microsoft#6333 Co-authored-by: Copilot <[email protected]>
github-actions
Bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 87%
✓ Correctness
The PR correctly makes compaction opt-in by moving token parameters to HarnessAgentOptions as nullable properties. The conditional logic properly enables compaction only when both values are provided, validation still fires via ContextWindowCompactionStrategy's constructor, the chat client pipeline is correctly assembled in both paths, and the InMemoryChatHistoryProvider is properly configured with or without a chat reducer. No correctness issues found.
✓ Security Reliability
The PR is clean from a security and reliability perspective. Input validation for token values is properly delegated to the ContextWindowCompactionStrategy constructor (which throws ArgumentOutOfRangeException for invalid values), null handling is correct throughout, and the conditional compaction logic is well-structured. There is one minor documentation/behavior inconsistency: when only MaxOutputTokens is set (without MaxContextWindowTokens), the code still applies it to ChatOptions.MaxOutputTokens despite the documentation claiming it will be 'left unchanged' in that scenario.
✓ Test Coverage
The PR correctly implements opt-in compaction and updates existing tests. However, the three new 'Compaction Opt-in' tests (lines 1743-1808) only assert
NotNull— they verify construction succeds but don't confirm whether the CompactionProvider is actually present or absent in the pipeline. Given that the pipeline-inspection pattern (innerAgent.ChatClient.GetService<T>()) is already used elsewhere in this file, the new tests could meaningfully assert that theAIContextProviderChatClientis absent when compaction is disabled and present when enabled.
✓ Failure Modes
The PR cleanly makes compaction opt-in. Validation is delegated to ContextWindowCompactionStrategy (which throws on invalid values when both tokens are provided), and graceful degradation when compaction is disabled is sound. No silent failures, lost errors, or operational failure paths introduced. One minor doc/code inconsistency exists around MaxOutputTokens being applied to ChatOptions even when compaction is disabled, but this is safer behavior than documented.
✓ Design Approach
The overall opt-in compaction change is coherent, but the new implementation drops validation for partially supplied token settings. That leaves malformed
HarnessAgentOptionsvalues silently accepted instead of rejected, which is a design regression against the constructor contract and the existing compaction strategy validation behavior.
Suggestions
- Doc/code mismatch: HarnessAgentOptions.MaxOutputTokens docs (line 60-61) state 'When either value is null, compaction is disabled and ChatOptions.MaxOutputTokens is left unchanged.' However, BuildChatOptions (HarnessAgent.cs:225-228) applies MaxOutputTokens to ChatOptions regardless of whether MaxContextWindowTokens is set. Consider either guarding the assignment with
compactionEnabledor updating the documentation to reflect that MaxOutputTokens is always applied as a default cap on model output.
Automated review by westey-m's agents
There was a problem hiding this comment.
Pull request overview
This PR updates the .NET Harness agent API to make context-window compaction opt-in by moving required token parameters into nullable HarnessAgentOptions properties, and simplifying both HarnessAgent and AsHarnessAgent construction accordingly.
Changes:
- Removed required
maxContextWindowTokens/maxOutputTokensconstructor parameters and updatedAsHarnessAgentto match. - Added nullable
MaxContextWindowTokens/MaxOutputTokenstoHarnessAgentOptions; compaction is enabled only when both are provided. - Updated unit tests and samples to use the new options-based API.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.Harness.UnitTests/HarnessAgentTests.cs | Updates tests for new constructor/extension signatures and adds coverage for compaction opt-in scenarios. |
| dotnet/src/Microsoft.Agents.AI.Harness/HarnessAgentOptions.cs | Introduces nullable token settings and documents compaction being opt-in. |
| dotnet/src/Microsoft.Agents.AI.Harness/HarnessAgent.cs | Makes compaction conditional, updates default chat history behavior, and simplifies construction. |
| dotnet/src/Microsoft.Agents.AI.Harness/ChatClientHarnessExtensions.cs | Simplifies AsHarnessAgent signature to accept only options/logger/services. |
| dotnet/samples/02-agents/Harness/Harness_Step04_CodeExecution/Program.cs | Updates sample to pass compaction token settings via HarnessAgentOptions. |
| dotnet/samples/02-agents/Harness/Harness_Step03_DataProcessing/Program.cs | Updates sample to pass compaction token settings via HarnessAgentOptions. |
| dotnet/samples/02-agents/Harness/Harness_Step02_Research_WithBackgroundAgents/Program.cs | Updates sample agents to pass compaction token settings via HarnessAgentOptions. |
| dotnet/samples/02-agents/Harness/Harness_Step01_Research/Program.cs | Updates sample to pass compaction token settings via HarnessAgentOptions. |
…gentOptions Allow users to provide their own CompactionStrategy via options, with a clear priority system: 1. DisableCompaction=true: no compaction regardless of other settings 2. Custom CompactionStrategy provided: use it (token params ignored) 3. Both MaxContextWindowTokens and MaxOutputTokens set: default strategy 4. Otherwise: no compaction Co-authored-by: Copilot <[email protected]>
- Update chatClient param XML doc to reflect compaction is opt-in - Strengthen compaction tests to assert ChatReducer is null/not-null rather than just asserting construction succeeds Co-authored-by: Copilot <[email protected]>
Summary
Removes the required
maxContextWindowTokensandmaxOutputTokensconstructor parameters fromHarnessAgentandAsHarnessAgent, replacing them with optional properties onHarnessAgentOptions.When both
MaxContextWindowTokensandMaxOutputTokensare provided in options, compaction is enabled as before. When either is null (the default), compaction is disabled entirely — making it opt-in.New API
Changes
HarnessAgentOptions.cs— AddedMaxContextWindowTokensandMaxOutputTokensnullable int propertiesHarnessAgent.cs— Simplified constructor, made compaction conditionalChatClientHarnessExtensions.cs— SimplifiedAsHarnessAgentsignatureCloses #6333