feat(conversation-manager): improve tool result truncation strategy

[kevmyung]

Description

Improves the SlidingWindowConversationManager truncation strategy to reduce
context size more gracefully without misleading the agent.

• Partially truncate large tool results (>400 chars) preserving first/last 200 chars,
  instead of replacing entire content with an error message
• Replace image blocks (top-level and inside toolResult) with descriptive text placeholders
• Target oldest tool results first so the most relevant recent results are preserved longer
• Add ValueError for invalid per_turn values (0 or negative integer)
• Rename _find_last_message_with_tool_results → _find_oldest_message_with_tool_results
  to accurately reflect iteration order

Related Issues

Closes #1545

Documentation PR

N/A

Type of Change

Other: Enhancement - improve existing truncation behavior

Testing

• I ran hatch run prepare

Added unit tests covering partial truncation, image block replacement,
oldest-first ordering, idempotency guard, and boundary conditions.

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

[kevmyung]

@JackYPCOnline Tagging you for review!

[github-actions]

Issue: This method is quite long (~70 lines) with multiple nested loops handling three different scenarios (top-level images, nested images in tool results, and text truncation).

Suggestion: Consider extracting helper methods for improved readability:

• _replace_image_with_placeholder(image_block) → returns the text placeholder
• _truncate_text_block(text, preserve_chars) → returns truncated text or None if not needed

This would simplify the main loop logic and make the code easier to maintain.

[github-actions]

Issue: The class docstring doesn't mention the new partial truncation behavior. Users looking at the class docs wouldn't know about the improved truncation strategy.

Suggestion: Add a brief note in the class docstring about the truncation behavior, e.g.:

"""Implements a sliding window strategy for managing conversation history.

...

When truncation is enabled, tool results are partially truncated (preserving
first and last 200 characters) rather than completely replaced, and image
blocks are replaced with descriptive placeholders.
"""

[github-actions]

Assessment: Comment

This PR improves the truncation strategy with a solid implementation that preserves context rather than completely replacing tool results. The test coverage is comprehensive.

Review Summary

• Configurability: The _PRESERVE_CHARS constant should be exposed as a constructor parameter to align with the "configurable character threshold" mentioned in [FEATURE] Improve tool result truncation strategy for graceful context reduction #1545 and the SDK's extensibility tenet
• Code Organization: The _truncate_tool_results method handles multiple concerns and could benefit from helper method extraction for maintainability
• Documentation: Class-level docstring should mention the new truncation behavior for discoverability

Good work on maintaining backward compatibility and implementing oldest-first ordering.

[github-actions]

Assessment: Comment

Thanks for updating the class docstring! The truncation behavior is now well-documented (lines 26-29).

Review Status

Addressed:

• ✅ Class docstring now documents the truncation behavior

Still Open (from previous review threads):

• Configurability: Issue [FEATURE] Improve tool result truncation strategy for graceful context reduction #1545 explicitly mentions "configurable character threshold" as a requirement. Consider exposing preserve_chars as a constructor parameter to fully address the issue scope.
• Method complexity: The _truncate_tool_results method suggestion for helper extraction is still relevant but non-blocking.

The implementation is solid and test coverage is comprehensive. The configurability question is worth addressing to fully align with #1545's requirements, but could also be a follow-up PR if preferred.

[codecov]

Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ion_manager/sliding_window_conversation_manager.py	94.11%	1 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!