-
Notifications
You must be signed in to change notification settings - Fork 184
Add verbose_filter feature and expose verbose parameter to PER #4358
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
Add verbose_filter feature and expose verbose parameter to PER #4358
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4358 +/- ##
============================================
+ Coverage 80.09% 80.16% +0.07%
- Complexity 10199 10225 +26
============================================
Files 855 855
Lines 44374 44407 +33
Branches 5135 5145 +10
============================================
+ Hits 35540 35598 +58
+ Misses 6670 6647 -23
+ Partials 2164 2162 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…agent Signed-off-by: Pavan Yekbote <[email protected]>
Signed-off-by: Pavan Yekbote <[email protected]>
eac5443 to
ce553e4
Compare
|
Can we use |
| public static final String EXECUTOR_VERBOSE = "executor_verbose"; | ||
| public static final String EXECUTOR_VERBOSE_FILTER = "executor_verbose_filter"; |
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.
In the example, this field name is "verbose" and "verbose_filter", which seems matching to the name in chatAgent. I think "verbose" and "verbose filter" is simpler.
| String parentInteractionId = tmpParameters.get(MLAgentExecutor.PARENT_INTERACTION_ID); | ||
| boolean verbose = Boolean.parseBoolean(tmpParameters.getOrDefault(VERBOSE, "false")); | ||
| boolean traceDisabled = tmpParameters.containsKey(DISABLE_TRACE) && Boolean.parseBoolean(tmpParameters.get(DISABLE_TRACE)); | ||
| List<String> traceFilter = parseTraceFilter(tmpParameters.get(VERBOSE_FILTER)); |
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.
Has any validation applied to this verbose filter value? like verboseFilter.matches("^[a-zA-Z]+(,[a-zA-Z]+)*$"). I think it's worth a check before using the filters.
| ); | ||
|
|
||
| // Pass through verbose and verbose_filter if provided | ||
| if (allParams.containsKey(EXECUTOR_VERBOSE)) { |
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.
Consider validation too.
| if (!allResponses.isEmpty()) { | ||
| StringBuilder stepResult = new StringBuilder(); | ||
| stepResult.append(allResponses.getLast()); | ||
| if (allResponses.size() > 1) { | ||
| stepResult.append("\n\n<step-traces>"); | ||
| } | ||
|
|
||
| for (int i = 0; i < allResponses.size() - 1; i++) { | ||
| stepResult.append("\n\n").append(allResponses.get(i)); | ||
| if (i == allResponses.size() - 2) { | ||
| stepResult.append("\n</step-traces>"); | ||
| } | ||
| } | ||
|
|
||
| results.put(STEP_RESULT_FIELD, stepResult.toString()); | ||
| } | ||
|
|
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 (!allResponses.isEmpty()) {
results.put(STEP_RESULT_FIELD, formatStepResultWithTraces(allResponses));
}
private String formatStepResultWithTraces(List<String> responses) {
StringBuilder result = new StringBuilder(responses.getLast());
if (responses.size() > 1) {
List<String> traces = responses.subList(0, responses.size() - 1);
result.append("\n\n<step-traces>\n\n")
.append(String.join("\n\n", traces))
.append("\n</step-traces>");
}
return result.toString();
}
This is a better version to avoid for loop.
Thanks, I can use this! I will raise a PR with PER processing this information if returned. |
|
Raised a new PR to process tool output if it is returned when using the I still think this is a good feature to have if folks want to filter on the verbose output. I will remove the changes for PER and keep the chat agent changes for the feature itself. |
|
let's moved the discussion to #4369 |
Description
verbose_filterparameter to filter out the verbose response.executor_verboseandexecutor_verbose_filterin PERLLMin filter to get only LLM responses, otherwise use the tool name.verbose_filteraccepts a csv list of tools andLLMas the filterExample:
Related Issues
Resolves #4357
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.