-
Notifications
You must be signed in to change notification settings - Fork 186
FEATURE: Summarize the steps when max steps limit reached #4416
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: feature/agentic_memory_integration
Are you sure you want to change the base?
FEATURE: Summarize the steps when max steps limit reached #4416
Conversation
) Signed-off-by: Dhrubo Saha <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
…roject#4331)" This reverts commit fb90b1b.
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Dhrubo Saha <[email protected]>
Signed-off-by: Pavan Yekbote <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
Signed-off-by: Pavan Yekbote <[email protected]>
Signed-off-by: Pavan Yekbote <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
…Agents (opensearch-project#4397) * add hooks in ml-commons (opensearch-project#4326) Signed-off-by: Xun Zhang <[email protected]> * initiate context management api with hook implementation (opensearch-project#4345) * initiate context management api with hook implementation Signed-off-by: Mingshi Liu <[email protected]> * apply spotless Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Mingshi Liu <[email protected]> * Add Context Manager to PER (opensearch-project#4379) * add pre_llm hook to per agent Signed-off-by: Mingshi Liu <[email protected]> change context management passing from query parameters to payload Signed-off-by: Mingshi Liu <[email protected]> pass hook registery into PER Signed-off-by: Mingshi Liu <[email protected]> apply spotless Signed-off-by: Mingshi Liu <[email protected]> initiate context management api with hook implementation Signed-off-by: Mingshi Liu <[email protected]> * add comment Signed-off-by: Mingshi Liu <[email protected]> * format Signed-off-by: Mingshi Liu <[email protected]> * add validation Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Mingshi Liu <[email protected]> * add inner create context management to agent register api Signed-off-by: Mingshi Liu <[email protected]> * add code coverage Signed-off-by: Mingshi Liu <[email protected]> * allow context management hook register in during agent execute Signed-off-by: Mingshi Liu <[email protected]> * add code coverage Signed-off-by: Mingshi Liu <[email protected]> * add more code coverage Signed-off-by: Mingshi Liu <[email protected]> * add validation check Signed-off-by: Mingshi Liu <[email protected]> * adapt to inplace update for context Signed-off-by: Mingshi Liu <[email protected]> * fix test Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Xun Zhang <[email protected]> Signed-off-by: Mingshi Liu <[email protected]> Co-authored-by: Xun Zhang <[email protected]>
…ut atm) Signed-off-by: Pavan Yekbote <[email protected]>
Signed-off-by: Dhrubo Saha <[email protected]>
…ing in index (opensearch-project#4403) * allow inline create context management without storing in agent register Signed-off-by: Mingshi Liu <[email protected]> * make ML_COMMONS_MULTI_TENANCY_ENABLED default is false Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Mingshi Liu <[email protected]>
Signed-off-by: Dhrubo Saha <[email protected]>
Signed-off-by: Yaliang Wu <[email protected]>
…#4407) Signed-off-by: Dhrubo Saha <[email protected]>
…roject#4408) * Fix POST_TOOL hook interaction updates and add tenant ID support Signed-off-by: Mingshi Liu <[email protected]> - Fix POST_TOOL hook to return full ContextManagerContext like PRE_LLM hook - Update MLChatAgentRunner to properly handle interaction updates from POST_TOOL hook - Ensure interactions list and tmpParameters.INTERACTIONS stay synchronized - Add tenant ID support to MLPredictionTaskRequest in ModelGuardrail and SummarizationManager Signed-off-by: Mingshi Liu <[email protected]> * fix error message escaping Signed-off-by: Mingshi Liu <[email protected]> * consolicate post_hook logic Signed-off-by: Mingshi Liu <[email protected]> --------- Signed-off-by: Mingshi Liu <[email protected]>
Hailong-am
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.
cherry picked from original PR
Signed-off-by: Jiaru Jiang <[email protected]>
Signed-off-by: Jiaru Jiang <[email protected]>
| } | ||
| } | ||
|
|
||
| public String extractSummaryFromResponse(MLTaskResponse response) { |
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.
try to avoid returning nulls inside methods, ideally this method would also handle the errors and return a fallback response.
If you return nulls in a method, it requires the caller to handle the null which can potentially lead to NPEs.
Not urgent, can address in future PRs
| }, finalListener::onFailure); | ||
|
|
||
| generateSummary(llm, completedSteps, allParams.get(TENANT_ID_FIELD), ActionListener.wrap(summary -> { | ||
| log.info("Summary generated successfully"); |
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.
let's remove this log, seems redundant
| String fallbackResult = completedSteps.isEmpty() || completedSteps.size() < 2 | ||
| ? String.format("Max Steps Limit (%d) Reached. Use memory_id with same task to restart.", maxSteps) | ||
| : String | ||
| .format( | ||
| "Max Steps Limit (%d) Reached. Use memory_id with same task to restart. \n " | ||
| + "Last executed step: %s, \n " | ||
| + "Last executed step result: %s", | ||
| maxSteps, | ||
| completedSteps.get(completedSteps.size() - 2), | ||
| completedSteps.getLast() | ||
| ); |
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.
you can pass the fallback from the previous method which is generated, this can make the current method more reusable
it's fine for now!
| } | ||
| } | ||
|
|
||
| private String extractSummaryFromResponse(MLTaskResponse response) { |
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.
you can move this to agent utils in a later PR
pyek-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.
LGTM, let's merge and address the comments in the main branch PR (or in a follow up PR)
Thanks for taking this up!
ce7f3f5 to
cd738fa
Compare
Description
This feature provides intelligent summarization when ML Commons Chat Agents and PER (planner) agent reach their maximum iteration limit without completing tasks.
Related Issues
#4150
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.