Python: Bound summarization input before provider call - #7375
Python: Bound summarization input before provider call#7375scarab-systems wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Python core compaction pipeline’s SummarizationStrategy to bound the size of the summarizer request before calling the provider, preventing summarization from failing due to context-window overruns in long-running conversations (fixing #7214).
Changes:
- Added
max_summary_input_tokens(defaulted) and a configurableTokenizerProtocolimplementation to estimate summarizer request size. - Implemented group-wise selection to include only whole message groups up to the configured input budget and only mark those groups as summarized/excluded.
- Added a regression test to verify bounded summarizer input and that unsent/oversized groups remain eligible for later compaction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_compaction.py | Adds token-budgeted selection of summary input groups and new SummarizationStrategy configuration for bounded summarizer requests. |
| python/packages/core/tests/core/test_compaction.py | Adds a test double and regression test validating bounded summarization and preservation of unsent groups. |
| if candidate_token_count > max_summary_input_tokens: | ||
| break |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
python/packages/core/tests/core/test_compaction.py:495
- _ScriptedSummarizer uses
BaseExceptionin its outcomes type andisinstancecheck. In tests this makes it easy to accidentally raiseSystemExit/KeyboardInterruptand abort the test run; usingExceptionis the safer, conventional choice for expected failures.
def __init__(self, outcomes: list[str | BaseException]) -> None:
python/packages/core/agent_framework/_compaction.py:1003
- _select_summary_input_groups rebuilds the full formatted transcript and re-tokenizes it on every group iteration. For long histories this is O(n^2) in both formatting and tokenization work, which can become a bottleneck exactly in the large-history scenarios this change targets.
for group_id, group_messages in groups:
candidate_messages = [*selected_messages, *group_messages]
candidate_text = _format_messages_for_summary(candidate_messages)
candidate_token_count = prompt_token_count + tokenizer.count_tokens(candidate_text)
if candidate_token_count > max_summary_input_tokens:
python/packages/core/agent_framework/_compaction.py:1109
- Failure escalation state is stored on the
SummarizationStrategyinstance (_consecutive_summary_failures,_summary_failure_error_emitted). Since compaction strategies are commonly configured per chat client and reused across independent conversations, this can mix failure counts across conversations and produce misleading ERROR escalation (or suppress it) for a given thread.
self.tokenizer = tokenizer or CharacterEstimatorTokenizer()
self._consecutive_summary_failures = 0
self._summary_failure_error_emitted = False
SummarizationStrategy now selects complete message groups that fit a configurable summary input token budget before calling the summary client. Only messages actually sent to the summarizer are annotated and excluded, leaving oversized later groups for a later compaction pass instead of shipping the whole transcript unbounded. Validation: uv run pytest packages/core/tests/core/test_compaction.py -k bounds_summary_input -m "not integration" failed before the implementation and passed after it; uv run pytest packages/core/tests/core/test_compaction.py -m "not integration" passed; uv run poe test -P core passed; uv run poe install completed; uv run poe check -P core passed.
Skip individually over-budget leading groups when selecting summarization input so a large early transcript item does not prevent later compactable groups from being summarized. Validation: uv run pytest packages/core/tests/core/test_compaction.py -k skips_oversized_first_group -q; uv run pytest packages/core/tests/core/test_compaction.py -q; uv run poe check -P core.
Track consecutive SummarizationStrategy failures and emit a single error once the strategy has failed three times without a successful summary. Reset the escalation state after a successful summary so only persistent failures become loud. Validation: uv run pytest packages/core/tests/core/test_compaction.py -k 'repeated_summary_failures or resets_failure_escalation' -q; uv run pytest packages/core/tests/core/test_compaction.py -q; uv run poe check -P core.
Avoid rebuilding and re-tokenizing the full selected summary transcript on every candidate group while preserving complete-group selection and oversized leading group skipping. Tighten the scripted summarizer test helper to expected Exception failures instead of BaseException. Verification: uv run pytest packages/core/tests/core/test_compaction.py -q; uv run poe syntax -P core.
3da6007 to
086442c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
python/packages/core/agent_framework/_compaction.py:1192
- The warning logged when no group fits the input budget doesn’t include the actual
max_summary_input_tokensvalue, which makes it harder to diagnose misconfiguration or unexpectedly large prompts/groups in production logs.
if self.max_summary_input_tokens is not None:
logger.warning(
"Skipping summarization compaction: no complete message group fits within max_summary_input_tokens."
)
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
Motivation & Context
SummarizationStrategycurrently formats every older message selected for summarization into a single summarizer request. For long histories, that can exceed a provider context limit; the strategy then logs a warning, returnsFalse, and stops contributing summary compaction for that round.This fixes #7214 by bounding the summarizer input before the provider call and making persistent summarize failures louder than the existing per-round warning.
Description & Review Guide
What are the major changes?
max_summary_input_tokensbudget forSummarizationStrategy, with the existing character-estimator tokenizer as the default estimator.What is the impact of these changes?
Nonetomax_summary_input_tokensto opt out of the budget.What do you want reviewers to focus on?
Scope note
Related Issue
Fixes #7214
No other open PR for this issue was found before submitting.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.