Conversation
|
Your PR has commits that are missing the Signed-off-by trailer. This is likely due to the pre-commit hook not being configured on your local machine. The usual fix for this issue is to run |
Dependency Review✅ No vulnerabilities or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #978 +/- ##
==========================================
+ Coverage 82.20% 82.40% +0.20%
==========================================
Files 197 197
Lines 8133 8152 +19
==========================================
+ Hits 6686 6718 +32
+ Misses 1447 1434 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dougmills-DIT
left a comment
There was a problem hiding this comment.
Could you add unit tests to prove the bug is fixed?
| else: | ||
| collated_result += f"<tool_result_{i}>{result_content}</tool_result_{i}>" | ||
|
|
||
| if success == "fail": |
There was a problem hiding this comment.
Is this not bool? Should it be? Unsure if there is a non pass/fail hence why String
There was a problem hiding this comment.
yeah this is supposed to be string - this follows the same logic as build_agent_with_loop for datahub agent
think the initial reason it was set to string so could be put in AIMessage (which is what run_tools_parallel returns), however, in future want to refactor run_tools_parallel so can leverage more structured return types so dont need these poorly typed strings
| log.warning("my-overall-result") | ||
| log.warning(current_result) |
There was a problem hiding this comment.
Maybe unneeded as warning but unsure and super minor
| log.warning("my-overall-result") | |
| log.warning(current_result) | |
| log.debug("my-overall-result") | |
| log.debug(current_result) |
…p-result-collation
Context
Fix datahub loop result collation - previously multiple results were not collated properly causing tool results to be lost
What
Have you written unit tests?
Are there any specific instructions on how to test this change?
Relevant links