-
Notifications
You must be signed in to change notification settings - Fork 138
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
Support regenerate for execute API #1709
Support regenerate for execute API #1709
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## feature/agent_framework_dev #1709 +/- ##
=================================================================
- Coverage 69.78% 69.56% -0.23%
- Complexity 2660 2667 +7
=================================================================
Files 244 244
Lines 12977 13036 +59
Branches 1305 1309 +4
=================================================================
+ Hits 9056 9068 +12
- Misses 3303 3347 +44
- Partials 618 621 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
// for regenerate, original interaction id must provide | ||
String regenerateInteractionId = params.get(REGENERATE_INTERACTION_ID); |
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.
Can interaction regeneration logic be handled in MLAgenExecutor instead of Chat agent? In future there could be new agent types and we should not re-implement regeneration logic in each agent type
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.
@arjunkumargiri Instead of put logic in MLAgenExecutor
, we can have a base class for all kinds of agent runner, put common logic in that class would make more sense.
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.
Agreed, makes sense.
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.
@arjunkumargiri Since the root interaction been moved to MLAgentRunner
,regenerate is coupled with root interaction, move the logic to that class also.
ml-algorithms/src/main/java/org/opensearch/ml/engine/algorithms/agent/MLChatAgentRunner.java
Outdated
Show resolved
Hide resolved
ml-algorithms/src/main/java/org/opensearch/ml/engine/memory/MLMemoryManager.java
Outdated
Show resolved
Hide resolved
6e051cc
to
c67bbdb
Compare
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]> Add includeToolputInAgentResponse flag Signed-off-by: Hailong Cui <[email protected]> Apply spotless Signed-off-by: Hailong Cui <[email protected]> address review comments Signed-off-by: Hailong Cui <[email protected]> address review comments Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
c67bbdb
to
fc3227e
Compare
Signed-off-by: Hailong Cui <[email protected]>
String appType = mlAgent.getAppType(); | ||
String question = inputDataSet.getParameters().get(QUESTION); | ||
|
||
if (memoryId == null && regenerateInteractionId != null) { |
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.
What if both are null? Do we still want to continue?
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.
if regenerateInteractionId
is null, it's a normal flow, memory will automatically create if needed
…nto regenerate Signed-off-by: Hailong Cui <[email protected]>
done in #1816 |
Description
Support regenerate on existing interaction,
regenerate_interaction_id
andmemory_id
must be provide for regenerate case.Response
Issues Resolved
[List any issues this PR will resolve]
Check List
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.