-
Notifications
You must be signed in to change notification settings - Fork 107
feat: agent automatic context managment #447
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
Conversation
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.
Caution
Changes requested ❌
Reviewed everything up to c20256a in 2 minutes and 28 seconds. Click for details.
- Reviewed
314lines of code in4files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. dynamiq/nodes/agents/agent.py:778
- Draft comment:
Consider re-raising the exception (or handling it more explicitly) in _ensure_context_manager_tool instead of only logging the error, to avoid silent failures. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a model validator that runs during initialization. The current code swallows all exceptions and only logs them, which could hide serious issues. However, this appears to be an optional feature (adding a context manager tool) that shouldn't prevent the entire object from being created. The code is defensive by catching exceptions and continuing. Re-raising could break object initialization unnecessarily. I may be underestimating the importance of the ContextManagerTool. If it's critical for summarization to work properly, then silently failing could lead to subtle bugs later. Looking at the code more carefully, this is a best-effort attempt to add a convenience tool. The summarization_config.enabled check suggests this is an optional feature. Failing silently is appropriate here since it's not critical functionality. The current error handling is appropriate for this optional initialization code. Re-raising would be too aggressive and could break initialization unnecessarily.
2. dynamiq/nodes/agents/agent.py:1099
- Draft comment:
The try and except blocks in _apply_context_manager_tool_effect contain duplicate code. Consider refactoring to avoid repetition. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. examples/use_cases/agents_use_cases/agent_deep_scraping.py:32
- Draft comment:
Ensure that the new tool configuration (ScaleSerpTool and E2BInterpreterTool) is fully compatible with the agent's XML inference mode and LLM setup. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_PBNiSPTN7o78ilSm
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
…iq-ai/dynamiq into feat/agent-context-manager
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.
Caution
Changes requested ❌
Reviewed everything up to 491fe6c in 3 minutes and 1 seconds. Click for details.
- Reviewed
314lines of code in4files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. dynamiq/nodes/agents/agent.py:770
- Draft comment:
Consider verifying that the automatic addition of ContextManagerTool in _ensure_context_manager_tool uses a robust check (e.g., by tool name or unique identifier) to avoid duplicates if the tool already exists. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. examples/use_cases/agents_use_cases/agent_deep_scraping.py:32
- Draft comment:
Ensure that the new automatic context management workflow integrates well with the agent’s configuration, especially with summarization_config and file_store settings. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
3. examples/use_cases/agents_use_cases/agent_deep_scraping.py:12
- Draft comment:
Typo: The multi-line string for AGENT_ROLE appears to be missing its closing triple quotes. Please add the closing triple quotes to complete the string. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_STTJLmG8A9HCeDqi
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed 33504dc in 58 seconds. Click for details.
- Reviewed
30lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. dynamiq/nodes/tools/context_manager.py:22
- Draft comment:
Added 'History to compress:' and '{history}' in prompt template. Ensure downstream formatting and LLM output integration uses the new placeholder. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. dynamiq/nodes/tools/context_manager.py:94
- Draft comment:
Improved string concatenation with added spaces in tool description. Verify the formatting meets design expectations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_5xFqlG7BGVnkADVW
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed a5ea310 in 1 minute and 19 seconds. Click for details.
- Reviewed
103lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. dynamiq/nodes/agents/agent.py:1018
- Draft comment:
The 'history_offset' parameter has been removed from summarize_history in favor of using the internal _history_offset attribute. Please update the docstring and any API docs accordingly to reflect this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to update documentation, which is not allowed according to the rules. The comment is not making a specific code suggestion or pointing out a potential issue with the code itself.
2. dynamiq/nodes/agents/agent.py:1122
- Draft comment:
Assigning '_history_offset' alongside summary_offset (i.e. 'summary_offset = self._history_offset = len(self._prompt.messages)') nicely encapsulates context management. Consider verifying that this state resets appropriately between separate runs. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify the behavior of the code, which violates the rule against asking the author to confirm or ensure behavior. It does not provide a specific suggestion or point out a clear issue.
3. dynamiq/nodes/agents/base.py:389
- Draft comment:
The addition of the PrivateAttr _history_offset with a default of 2 to represent the offset for conversation history is a clear improvement. Ensure that its description and default value meet all use case needs, and consider whether this offset should be configurable from outside as needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is asking the PR author to ensure that the description and default value meet all use case needs, which is against the rules. It also suggests considering configurability, which is a valid suggestion. However, the primary focus of the comment is on ensuring and considering, which violates the rules.
Workflow ID: wflow_QcFPctio7yTDH57U
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed e5e5a19 in 1 minute and 12 seconds. Click for details.
- Reviewed
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. dynamiq/nodes/agents/base.py:390
- Draft comment:
Changing the verbose description to an inline comment is fine if the description metadata isn’t used by PrivateAttr. If documentation for introspection is needed, consider a short docstring instead. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_gmkKZJ8oyUUN1ZvS
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Important
Adds
ContextManagerToolfor automatic context management in agents, enabling summarization and context compression.ContextManagerTooltoagent.pyfor automatic context management when summarization is enabled.update_system_message()fromagent.py.summarize_history()inagent.pyto removehistory_offsetparameter.ContextManagerToolincontext_manager.pyfor compressing message history into a summary._apply_context_manager_tool_effect()incontext_manager.pyto apply context changes.agent_file_storage.yamlto enable summarization and increasemax_loopsto 15.agent_deep_scraping.pyto useScaleSerpToolandE2BInterpreterToolwith summarization enabled.ContextManagerToolimport to__init__.pyintoolspackage.This description was created by
for e5e5a19. You can customize this summary. It will automatically update as commits are pushed.