Repository navigation
fix(condenser): show the summarizer whole events, not their 500-char preview #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import os | ||
| from collections.abc import Sequence | ||
| from enum import Enum | ||
| from typing import Final | ||
| from typing import ClassVar, Final | ||
|
|
||
| from pydantic import Field, model_validator | ||
|
|
||
|
|
@@ -13,15 +13,16 @@ | |
| from openhands.sdk.context.condenser.utils import ( | ||
| get_suffix_length_for_token_reduction, | ||
| get_total_token_count, | ||
| render_event_for_summary, | ||
| ) | ||
| from openhands.sdk.context.prompts import render_template | ||
| from openhands.sdk.context.view import View | ||
| from openhands.sdk.event.base import LLMConvertibleEvent | ||
| from openhands.sdk.event.condenser import Condensation | ||
| from openhands.sdk.event.condenser import Condensation, CondensationSummaryEvent | ||
| from openhands.sdk.llm import LLM, Message, TextContent | ||
| from openhands.sdk.llm.exceptions.classifier import is_context_window_exceeded | ||
| from openhands.sdk.logger import get_logger | ||
| from openhands.sdk.observability.laminar import observe | ||
| from openhands.sdk.utils import maybe_truncate | ||
|
|
||
|
|
||
| logger = get_logger(__name__) | ||
|
|
@@ -62,6 +63,13 @@ class LLMSummarizingCondenser(RollingCondenser): | |
| are treated as errors. | ||
| """ | ||
|
|
||
| max_event_length: ClassVar[int] = 10_000 | ||
| """Longest a single forgotten event may be when shown to the summarizer, in | ||
| characters; longer ones keep their head and tail. A previous summary is exempt: | ||
| it is the agent's only record of everything condensed before it. A class | ||
| constant, not a field, so the condenser's settings schema is unchanged. | ||
| """ | ||
|
|
||
| hard_context_reset_max_retries: int = Field(default=5, gt=0) | ||
| """Number of attempts to perform hard context reset before raising an error.""" | ||
|
|
||
|
|
@@ -186,6 +194,129 @@ def condensation_requirement( | |
| if Reason.REQUEST in reasons: | ||
| return CondensationRequirement.HARD | ||
|
|
||
| def _render_forgotten_events( | ||
| self, | ||
| forgotten_events: Sequence[LLMConvertibleEvent], | ||
| max_event_str_length: int | None = None, | ||
| ) -> list[str]: | ||
| """Render forgotten events for the summarizer, whole up to the length caps. | ||
|
|
||
| Every event is capped at ``max_event_length``, except a previous summary. | ||
| ``max_event_str_length`` -- set only when the summarizer overflowed and the | ||
| input is being shrunk -- caps every event, the previous summary included. | ||
| """ | ||
| rendered = [] | ||
| for event in forgotten_events: | ||
| limits = [max_event_str_length] | ||
| if not isinstance(event, CondensationSummaryEvent): | ||
| limits.append(self.max_event_length) | ||
| caps = [limit for limit in limits if limit is not None] | ||
| rendered.append( | ||
| render_event_for_summary(event, min(caps) if caps else None) | ||
| ) | ||
| return rendered | ||
|
|
||
| def _shrunk_event_length( | ||
| self, | ||
| events: Sequence[LLMConvertibleEvent], | ||
| max_event_str_length: int | None, | ||
| ) -> int: | ||
| """The next per-event cap after the summarizer call failed. | ||
|
|
||
| Chosen so the rendered total -- not just the longest event -- falls to | ||
| ``hard_context_reset_context_scaling`` of what was just sent: scaling the | ||
| longest event alone leaves every shorter one untouched, so a prompt made | ||
| of many mid-sized events is re-sent almost unchanged. Returns 0 when no | ||
| positive cap gets there, i.e. there is nothing left to cut. | ||
| """ | ||
| lengths = [ | ||
| len(text) | ||
| for text in self._render_forgotten_events(events, max_event_str_length) | ||
| ] | ||
| target = sum(lengths) * self.hard_context_reset_context_scaling | ||
| # The largest cap whose capped total fits the target (capping never grows | ||
| # an event, so the total is monotonic in the cap). | ||
| low, high = 0, max(lengths) | ||
| while low < high: | ||
| cap = (low + high + 1) // 2 | ||
| if sum(min(length, cap) for length in lengths) <= target: | ||
| low = cap | ||
| else: | ||
| high = cap - 1 | ||
| return low | ||
|
|
||
| def _summarize( | ||
| self, forgotten_events: Sequence[LLMConvertibleEvent], summary_offset: int | ||
| ) -> Condensation: | ||
| """Summarize the forgotten events whole; shrink them only on overflow. | ||
|
|
||
| Whole events can exceed a summarizer whose window is smaller than the | ||
| agent's. Rather than fail -- and fall through to a hard context reset that | ||
| forgets everything -- retry with every event cut further each time. Any | ||
| other failure is raised at once. | ||
| """ | ||
| max_event_str_length: int | None = None | ||
| for _ in range(self.hard_context_reset_max_retries): | ||
| try: | ||
| return self._generate_condensation( | ||
| forgotten_events=forgotten_events, | ||
| summary_offset=summary_offset, | ||
| max_event_str_length=max_event_str_length, | ||
| ) | ||
| except NoCondensationAvailableException as e: | ||
| cause = e.__cause__ | ||
| if not isinstance(cause, Exception) or not is_context_window_exceeded( | ||
| cause | ||
| ): | ||
| raise | ||
| max_event_str_length = self._shrunk_event_length( | ||
| forgotten_events, max_event_str_length | ||
| ) | ||
| if max_event_str_length <= 0: | ||
| raise NoCondensationAvailableException( | ||
| "Summarizer context exceeded with nothing left to cut." | ||
| ) from e | ||
| logger.warning( | ||
| "Summarizer context exceeded; retrying with each event cut to " | ||
| f"{max_event_str_length} characters." | ||
| ) | ||
| raise NoCondensationAvailableException( | ||
| "Summarizer context exceeded at every event length tried." | ||
| ) | ||
|
|
||
| async def _asummarize( | ||
| self, forgotten_events: Sequence[LLMConvertibleEvent], summary_offset: int | ||
| ) -> Condensation: | ||
| """Async variant of :meth:`_summarize`.""" | ||
| max_event_str_length: int | None = None | ||
| for _ in range(self.hard_context_reset_max_retries): | ||
| try: | ||
| return await self._agenerate_condensation( | ||
| forgotten_events=forgotten_events, | ||
| summary_offset=summary_offset, | ||
| max_event_str_length=max_event_str_length, | ||
| ) | ||
| except NoCondensationAvailableException as e: | ||
| cause = e.__cause__ | ||
| if not isinstance(cause, Exception) or not is_context_window_exceeded( | ||
| cause | ||
| ): | ||
| raise | ||
| max_event_str_length = self._shrunk_event_length( | ||
| forgotten_events, max_event_str_length | ||
| ) | ||
| if max_event_str_length <= 0: | ||
| raise NoCondensationAvailableException( | ||
| "Summarizer context exceeded with nothing left to cut." | ||
| ) from e | ||
| logger.warning( | ||
| "Summarizer context exceeded; retrying with each event cut to " | ||
| f"{max_event_str_length} characters." | ||
| ) | ||
| raise NoCondensationAvailableException( | ||
| "Summarizer context exceeded at every event length tried." | ||
| ) | ||
|
|
||
| def _generate_condensation( | ||
| self, | ||
| forgotten_events: Sequence[LLMConvertibleEvent], | ||
|
|
@@ -209,11 +340,9 @@ def _generate_condensation( | |
| """ | ||
| assert len(forgotten_events) > 0, "No events to condense." | ||
|
|
||
| # Convert events to strings for the template | ||
| event_strings = [ | ||
| maybe_truncate(str(forgotten_event), truncate_after=max_event_str_length) | ||
| for forgotten_event in forgotten_events | ||
| ] | ||
| event_strings = self._render_forgotten_events( | ||
| forgotten_events, max_event_str_length | ||
| ) | ||
|
|
||
| prompt = render_template( | ||
| os.path.join(os.path.dirname(__file__), "prompts"), | ||
|
|
@@ -342,15 +471,10 @@ def hard_context_reset( | |
| max_event_str_length=max_event_str_length, | ||
| ) | ||
| except Exception as e: | ||
| # If we haven't set a max_event_str_length yet, set it as the largest | ||
| # event string length. | ||
| if max_event_str_length is None: | ||
| max_event_str_length = max(len(str(event)) for event in view.events) | ||
|
|
||
| # Since the summarization failed, reduce the max_event_str_length by 20% | ||
| assert max_event_str_length is not None | ||
| max_event_str_length = int( | ||
| max_event_str_length * self.hard_context_reset_context_scaling | ||
| # Since the summarization failed, cut every event to 80% of the | ||
| # longest one as rendered -- not of its 500-character preview. | ||
| max_event_str_length = max( | ||
| 1, self._shrunk_event_length(view.events, max_event_str_length) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Low] |
||
| ) | ||
|
|
||
| # Log the exception so we can track these failures | ||
|
|
@@ -393,10 +517,7 @@ def get_condensation( | |
| "threshold." | ||
| ) | ||
|
|
||
| return self._generate_condensation( | ||
| forgotten_events=forgotten_events, | ||
| summary_offset=summary_offset, | ||
| ) | ||
| return self._summarize(forgotten_events, summary_offset) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Async variants | ||
|
|
@@ -411,10 +532,9 @@ async def _agenerate_condensation( | |
| """Async variant of :meth:`_generate_condensation`.""" | ||
| assert len(forgotten_events) > 0, "No events to condense." | ||
|
|
||
| event_strings = [ | ||
| maybe_truncate(str(fe), truncate_after=max_event_str_length) | ||
| for fe in forgotten_events | ||
| ] | ||
| event_strings = self._render_forgotten_events( | ||
| forgotten_events, max_event_str_length | ||
| ) | ||
|
|
||
| prompt = render_template( | ||
| os.path.join(os.path.dirname(__file__), "prompts"), | ||
|
|
@@ -475,10 +595,7 @@ async def aget_condensation( | |
| "progress threshold." | ||
| ) | ||
|
|
||
| return await self._agenerate_condensation( | ||
| forgotten_events=forgotten_events, | ||
| summary_offset=summary_offset, | ||
| ) | ||
| return await self._asummarize(forgotten_events, summary_offset) | ||
|
|
||
| async def ahard_context_reset( | ||
| self, | ||
|
|
@@ -497,11 +614,8 @@ async def ahard_context_reset( | |
| max_event_str_length=max_event_str_length, | ||
| ) | ||
| except Exception as e: | ||
| if max_event_str_length is None: | ||
| max_event_str_length = max(len(str(ev)) for ev in view.events) | ||
| assert max_event_str_length is not None | ||
| max_event_str_length = int( | ||
| max_event_str_length * self.hard_context_reset_context_scaling | ||
| max_event_str_length = max( | ||
| 1, self._shrunk_event_length(view.events, max_event_str_length) | ||
| ) | ||
| logger.warning( | ||
| f"Hard context reset summarization failed: {e}. " | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Medium] This retry only lowers the per-event cap (
_shrunk_event_lengthderives it from the longest rendered event). When the overflow comes from the summed size of many events that are already shorter than the cap, each iteration re-sends an essentially unchanged oversized prompt, burns allhard_context_reset_max_retriessummarizer calls, and then still raises — so the condensation falls through to the full hard reset this retry was added to avoid. Bail out when the new cap does not reduce the rendered total (or start dropping events) instead of retrying blindly. Same defect in_asummarizeat line 271.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 84b438a.
_shrunk_event_lengthnow picks the largest cap whose capped total is at mosthard_context_reset_context_scalingof the prompt just sent. Each retry therefore cuts the whole prompt by that factor, not just the longest event. When no positive cap gets there, it returns 0 and the retry raises ("nothing left to cut"). This matters becausemaybe_truncate(text, 0)means no truncation and would re-send the full prompt. The hard-reset loops floor the cap at 1 for the same reason.New tests:
test_summarizer_overflow_shrinks_the_whole_prompt_not_one_event: one 9k result plus forty 1.2k ones. It was red on a8babaa.test_summarizer_overflow_never_resends_untrimmed_events: the 0-cap guard. Mutation-checked: removing the guard turns it red.