From 75efc14fc035adafbb2fdea07a9422df6cedffa1 Mon Sep 17 00:00:00 2001 From: Zamil Majdy Date: Thu, 8 Jan 2026 18:07:35 -0600 Subject: [PATCH] refactor: improve review result handling with early returns and type safety - Use ternary operator for cleaner message assignment in HITLReviewHelper - Remove redundant null checks in HumanInTheLoopBlock - Add type assertion to help type checker understand review_result cannot be None - Improve code readability and maintainability --- .../backend/backend/blocks/helpers/review.py | 10 ++++---- .../backend/blocks/human_in_the_loop.py | 23 ++++++++----------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/autogpt_platform/backend/backend/blocks/helpers/review.py b/autogpt_platform/backend/backend/blocks/helpers/review.py index bd4534a8e4..24bfa6eaee 100644 --- a/autogpt_platform/backend/backend/blocks/helpers/review.py +++ b/autogpt_platform/backend/backend/blocks/helpers/review.py @@ -176,11 +176,13 @@ class HITLReviewHelper: # Still awaiting review - return None to pause execution return None + # Review is complete, determine outcome should_proceed = review_result.status == ReviewStatus.APPROVED - if should_proceed: - message = review_result.message or "Execution approved by reviewer" - else: - message = review_result.message or "Execution rejected by reviewer" + message = review_result.message or ( + "Execution approved by reviewer" + if should_proceed + else "Execution rejected by reviewer" + ) return ReviewDecision( should_proceed=should_proceed, message=message, review_result=review_result diff --git a/autogpt_platform/backend/backend/blocks/human_in_the_loop.py b/autogpt_platform/backend/backend/blocks/human_in_the_loop.py index f7ee7b44a3..cbcf3c924e 100644 --- a/autogpt_platform/backend/backend/blocks/human_in_the_loop.py +++ b/autogpt_platform/backend/backend/blocks/human_in_the_loop.py @@ -132,18 +132,15 @@ class HumanInTheLoopBlock(Block): # Execution is paused, the helper already handled status updates return - # Process the review result and yield appropriate outputs - if ( - decision.review_result - and decision.review_result.status == ReviewStatus.APPROVED - ): + # Decision is complete - review_result is guaranteed to be present + assert ( + decision.review_result is not None + ), "review_result should not be None when decision exists" + if decision.review_result.status == ReviewStatus.APPROVED: yield "approved_data", decision.review_result.data - if decision.message: - yield "review_message", decision.message - elif ( - decision.review_result - and decision.review_result.status == ReviewStatus.REJECTED - ): + elif decision.review_result.status == ReviewStatus.REJECTED: yield "rejected_data", decision.review_result.data - if decision.message: - yield "review_message", decision.message + + # Always yield the review message if present + if decision.message: + yield "review_message", decision.message