fix(truncator): keep an all-system context in the system half#8855
fix(truncator): keep an all-system context in the system half#8855he-yufeng wants to merge 1 commit into
Conversation
_split_system_rest initialized first_non_system to 0, so when every message is a system message the loop never breaks and the split is reversed: the system messages land in the non-system half. truncate_by_halving then treats them as droppable turns and discards half of them (e.g. 4 system messages -> 2). System messages must never be dropped. Default first_non_system to len(messages) so an all-system list stays entirely in the system half; mixed and empty inputs are unchanged.
There was a problem hiding this comment.
Code Review
This pull request fixes a bug in the context truncator where an all-system message list was incorrectly split, potentially leading to system messages being dropped during truncation. The fix initializes the split index to the length of the messages, ensuring all-system lists are correctly handled. Corresponding unit tests have been added to verify this behavior. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new tests exercise behavior via the private helper
_split_system_rest; consider adding/adjusting a test that validates the same invariant purely through the public truncation APIs so future refactors of the helper don’t require test changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new tests exercise behavior via the private helper `_split_system_rest`; consider adding/adjusting a test that validates the same invariant purely through the public truncation APIs so future refactors of the helper don’t require test changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
What
ContextTruncator._split_system_restsplits a message list into(system_messages, rest). It initializedfirst_non_system = 0and only updated it when it found a non-system message. When every message is a system message the loop never breaks, sofirst_non_systemstays0and the split is reversed — the system messages end up in theresthalf and the system half is empty.The truncation strategies then treat those system messages as droppable turns. For example
truncate_by_halvingon four system messages returns only two:System messages must never be dropped by truncation. The fix defaults
first_non_systemtolen(messages), so an all-system list stays entirely in the system half. Mixed inputs (the loop still breaks at the first non-system message) and empty inputs are unchanged.Test plan
Added two tests in
tests/agent/test_truncator.py:test_split_system_rest_with_only_system_messages— all-system input puts everything in the system half.test_halving_keeps_all_system_messages—truncate_by_halvingkeeps all four system messages.Both fail on
masterand pass with this change.ruff checkandruff format --checkpass on the changed files.Summary by Sourcery
Ensure context truncation never drops system messages when splitting or halving message histories.
Bug Fixes:
Tests: