-
Notifications
You must be signed in to change notification settings - Fork 107
feat(condenser): Hard context reset on unrecoverable error #1596
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi! I started running the condenser tests on your PR. You will receive a comment with the results shortly. Note: These are non-blocking tests that validate condenser functionality across different LLMs. |
Condenser Test Results (Non-Blocking)
🧪 Condenser Tests ResultsOverall Success Rate: 100.0% 📊 Summary
📋 Detailed Resultslitellm_proxy_gpt_5.1_codex_max
Skipped Tests:
litellm_proxy_anthropic_claude_opus_4_5_20251101
|
all-hands-bot
left a comment
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.
Overall this is a solid implementation of a fallback strategy for handling unrecoverable context situations. The approach is reasonable, but there are a few important issues around error handling and observability that should be addressed. See inline comments for details.
openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py
Show resolved
Hide resolved
openhands-sdk/openhands/sdk/context/condenser/llm_summarizing_condenser.py
Show resolved
Hide resolved
| if hard_reset_condensation is not None: | ||
| return hard_reset_condensation | ||
|
|
||
| # In all other situations re-raise the exception. |
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.
🟢 Nit: Comment could be more specific about which situations trigger re-raise.
For clarity, consider:
| # In all other situations re-raise the exception. | |
| # Re-raise if: (1) request is HARD but hard_context_reset returned None, | |
| # or (2) request is neither SOFT nor HARD | |
| raise e |
|
This PR looks like it works -- until you try to continue a conversation after a hard reset. The I'm working on an extension to address this issue in a separate PR, and will proceed with this one when that feature is merged. |
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Summary
Extends the rolling condenser API to support hard context resets, and adds one such strategy for the LLM summarizing condenser. This strategy triggers when a hard condensation request comes through but one can't be found.
The quality of the summary and the agent's behavior afterwards is not likely to be good. This is intended solely as a last-ditch fallback option.
Changes
RollingCondenserbase class to exposehard_context_reset-- an optional strategy for last-ditch context managementLLMSummarizingCondenserc02condensation integration test to check for the hard context reset instead of the expected errorChecklist
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:de3797c-pythonRun
All tags pushed for this build
About Multi-Architecture Support
de3797c-python) is a multi-arch manifest supporting both amd64 and arm64de3797c-python-amd64) are also available if needed