Improve performance of LLM summarizing condenser #6597
Conversation
| # Create a new summary event with the condensed content | ||
| summary_event = AgentCondensationObservation(summary_response) | ||
| for forgotten_event in forgotten_events: | ||
| prompt += str(forgotten_event) + '\n\n' |
There was a problem hiding this comment.
This might be an argument for standardizing the Event -> Message conversion and breaking it out of the agent control-flow.
Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
| TESTS: test_format() passed | ||
| CHANGES: str(val) replaces f"{val:.16G}" | ||
| DEPS: None modified | ||
| INTENT: Fix float precision overflow""" |
There was a problem hiding this comment.
Interesting that Sonnet is doing reasonable with this prompt. I'm going to test it on Deepseek 😅
(not necessary for this PR, of course, just a thought)
There was a problem hiding this comment.
I'm surprised this is what it ended up being, it's definitely not how I would default to prompting!
I started off with a very simple prompt, then I passed it and a trajectory or two to Claude and asked what it would change. Took two or three iterations before it met the baseline but it got me from 0% to 40%. I have to imagine you'd arrive at a very different result if you repeated the process with DeepSeek.
| mock_llm.completion.assert_called_once() | ||
| call_args = mock_llm.completion.call_args[1] | ||
| assert 'messages' in call_args | ||
| assert len(call_args['messages']) == 1 | ||
| assert 'Event 1' in call_args['messages'][0]['content'] | ||
| assert 'Event 2' in call_args['messages'][0]['content'] |
There was a problem hiding this comment.
Maybe we can still test for some message inclusion? Not a big deal, just thinking that tests with condensation are not easy, so maybe we can include in unit tests whatever we can
…ith49/OpenHands into feat/llm-summarizing-condenser-fixes
Co-authored-by: Calvin Smith <calvin@all-hands.dev> Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
End-user friendly description of the problem this fixes or functionality that this introduces
Give a summary of what the PR does, explaining any non-trivial design decisions
Updates the LLM-based summarizing condenser to improve performance.
LLMSummarizingCondensernow relies on theRollingCondenserto control when summarization happens.LLMSummarizingCondenseris now the default condenser (still off by default!), compared to the more aggressiveAmortizedForgettingCondenserthat has the same forgetting behavior as theLLMSummarizingCondenserbut without the summarization.To test performance, I ran this condenser against the no-condensation baseline (both on OH v0.21.0) on the whole of SWE-bench Verified with a maximum of 100 iterations. The condenser resolved 200 instances, compared to the baseline's 203, and cost $40 more to run due to the lower prompt cache utilization. However, the average response latency is a consistent 8 seconds compared to the 12 seconds (at iteration 30) and 16 seconds (at iteration 100) of the baseline.
Link of any specific issues this addresses