-
Notifications
You must be signed in to change notification settings - Fork 0
fix: fix datahub loop result collation #978
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: dev
Are you sure you want to change the base?
Changes from 6 commits
520a1f7
06240ef
8207bbc
2e4c497
65c52d8
b82b51e
ebe90cd
dac074a
8f11428
0e5c6c2
2037437
f694c93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -699,37 +699,49 @@ def local_loop_condition(): | |
| result = "Tool error: no results received." | ||
|
|
||
| elif has_loop and len(ai_msg.tool_calls) > 0: # if loop, we need to transform results | ||
| result = result[-1].content # this is a tuple | ||
| # format of result: (result, success, is_intermediate_step) | ||
| log.warning("my-overall-result") | ||
| log.warning(result) | ||
| result_content = result[0] | ||
| success = result[1] | ||
| is_intermediate_step = eval(result[2]) | ||
| collated_result = "" | ||
| feedback_reasons = [] | ||
| for i, r in enumerate(result): | ||
| current_result = r.content # this is a tuple | ||
| # format of result: (result, success, is_intermediate_step) | ||
| log.warning("my-overall-result") | ||
| log.warning(current_result) | ||
| result_content = current_result[0] | ||
| success = current_result[1] | ||
| is_intermediate_step = eval(current_result[2]) | ||
|
|
||
| if len(current_result) > 3: | ||
| reason = current_result[3] | ||
| feedback_reasons.append(f"Failure reason: {reason}.\n\n{result_content}") | ||
| else: | ||
| collated_result += f"<tool_result_{i}>{result_content}</tool_result_{i}>" | ||
|
|
||
| if success == "fail": | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this not bool? Should it be? Unsure if there is a non pass/fail hence why String
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| # pass error back if any | ||
| additional_variables.update({"previous_tool_error": result_content}) | ||
| else: | ||
| # if success tool invocation, and intermediate steps then pass info back | ||
| if is_intermediate_step: | ||
| additional_variables.update( | ||
| {"previous_tool_error": "", "previous_tool_results": all_results} | ||
| ) | ||
|
|
||
| if len(result) > 3: | ||
| reason = result[3] | ||
| if feedback_reasons: | ||
| combined_feedback = "\n\n".join(feedback_reasons) | ||
| return { | ||
| "agents_results": { | ||
| task.id: AIMessage( | ||
| content=f"<{agent_name}_Result>Ask user for feedback based on failure reason. Failure reason: {reason}.\n\n{result_content}</{agent_name}_Result>", | ||
| content=f"<{agent_name}_Result>Ask user for feedback based on failure reason. {combined_feedback}\n\n{collated_result}</{agent_name}_Result>", | ||
| kwargs={ | ||
| "reason": reason, | ||
| "reason": combined_feedback, | ||
| }, | ||
| ) | ||
| }, | ||
| "tasks_evaluator": task.task + "\n" + task.expected_output, | ||
| "agent_plans": state.agent_plans.update_task_status(task.id, TaskStatus.REQUIRES_USER_FEEDBACK), | ||
| } | ||
|
|
||
| if success == "fail": | ||
| # pass error back if any | ||
| additional_variables.update({"previous_tool_error": result_content}) | ||
| else: | ||
| # if success tool invocation, and intermediate steps then pass info back | ||
| if is_intermediate_step: | ||
| additional_variables.update({"previous_tool_error": "", "previous_tool_results": all_results}) | ||
| result = result_content | ||
| result = collated_result | ||
|
|
||
| if isinstance(result, str): | ||
| log.warning(f"{log_stub} Using raw string result.") | ||
|
|
||
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.
Maybe unneeded as warning but unsure and super minor