From 3cc25573f016a080e00a5c2acc2699732fa52167 Mon Sep 17 00:00:00 2001 From: goyalpramod Date: Tue, 12 Nov 2024 19:01:09 +0530 Subject: [PATCH 1/4] task: added warning when none is called in intervention handler --- .../src/autogen_core/base/intervention.py | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/python/packages/autogen-core/src/autogen_core/base/intervention.py b/python/packages/autogen-core/src/autogen_core/base/intervention.py index c9600ac9e13..2628df8430f 100644 --- a/python/packages/autogen-core/src/autogen_core/base/intervention.py +++ b/python/packages/autogen-core/src/autogen_core/base/intervention.py @@ -1,5 +1,5 @@ +import warnings from typing import Any, Awaitable, Callable, Protocol, final - from autogen_core.base import AgentId __all__ = [ @@ -14,6 +14,27 @@ class DropMessage: ... +def warn_if_none(result: Any, handler_name: str) -> Any: + """ + Utility function to check if the intervention handler returned None and issue a warning. + + Args: + result: The return value from the intervention handler + handler_name: Name of the intervention handler method for the warning message + + Returns: + The original result value + """ + if result is None: + warnings.warn( + f"Intervention handler {handler_name} returned None. This might be unintentional. " + "Consider returning the original message or DropMessage explicitly.", + RuntimeWarning, + stacklevel=2 + ) + return result + + InterventionFunction = Callable[[Any], Any | Awaitable[type[DropMessage]]] @@ -27,10 +48,13 @@ async def on_response( class DefaultInterventionHandler(InterventionHandler): async def on_send(self, message: Any, *, sender: AgentId | None, recipient: AgentId) -> Any | type[DropMessage]: - return message + result = message + return warn_if_none(result, "on_send") async def on_publish(self, message: Any, *, sender: AgentId | None) -> Any | type[DropMessage]: - return message + result = message + return warn_if_none(result, "on_publish") async def on_response(self, message: Any, *, sender: AgentId, recipient: AgentId | None) -> Any | type[DropMessage]: - return message + result = message + return warn_if_none(result, "on_response") \ No newline at end of file From 72bde3bedf5a7b8b1ade8892348b2435ea29f931 Mon Sep 17 00:00:00 2001 From: Pramod Goyal <81946962+goyalpramod@users.noreply.github.com> Date: Mon, 18 Nov 2024 07:21:36 +0530 Subject: [PATCH 2/4] add leading underscore to indicate private to _warn_if_none method in intervention.py --- .../autogen-core/src/autogen_core/base/intervention.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/packages/autogen-core/src/autogen_core/base/intervention.py b/python/packages/autogen-core/src/autogen_core/base/intervention.py index 2628df8430f..d36f11b66fe 100644 --- a/python/packages/autogen-core/src/autogen_core/base/intervention.py +++ b/python/packages/autogen-core/src/autogen_core/base/intervention.py @@ -14,7 +14,7 @@ class DropMessage: ... -def warn_if_none(result: Any, handler_name: str) -> Any: +def _warn_if_none(result: Any, handler_name: str) -> Any: """ Utility function to check if the intervention handler returned None and issue a warning. @@ -57,4 +57,4 @@ async def on_publish(self, message: Any, *, sender: AgentId | None) -> Any | typ async def on_response(self, message: Any, *, sender: AgentId, recipient: AgentId | None) -> Any | type[DropMessage]: result = message - return warn_if_none(result, "on_response") \ No newline at end of file + return warn_if_none(result, "on_response") From 7ba2fb0f33af3d3951270fd7c9c93f68c41e8c74 Mon Sep 17 00:00:00 2001 From: Pramod Goyal <81946962+goyalpramod@users.noreply.github.com> Date: Mon, 18 Nov 2024 07:27:54 +0530 Subject: [PATCH 3/4] address comment of returning Any for result in intervention.py --- .../src/autogen_core/base/intervention.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/python/packages/autogen-core/src/autogen_core/base/intervention.py b/python/packages/autogen-core/src/autogen_core/base/intervention.py index d36f11b66fe..3c6a083cbe2 100644 --- a/python/packages/autogen-core/src/autogen_core/base/intervention.py +++ b/python/packages/autogen-core/src/autogen_core/base/intervention.py @@ -14,25 +14,21 @@ class DropMessage: ... -def _warn_if_none(result: Any, handler_name: str) -> Any: +def _warn_if_none(value: Any, handler_name: str) -> None: """ Utility function to check if the intervention handler returned None and issue a warning. Args: - result: The return value from the intervention handler + value: The return value to check handler_name: Name of the intervention handler method for the warning message - - Returns: - The original result value """ - if result is None: + if value is None: warnings.warn( f"Intervention handler {handler_name} returned None. This might be unintentional. " "Consider returning the original message or DropMessage explicitly.", RuntimeWarning, stacklevel=2 ) - return result InterventionFunction = Callable[[Any], Any | Awaitable[type[DropMessage]]] @@ -49,12 +45,15 @@ async def on_response( class DefaultInterventionHandler(InterventionHandler): async def on_send(self, message: Any, *, sender: AgentId | None, recipient: AgentId) -> Any | type[DropMessage]: result = message - return warn_if_none(result, "on_send") + _warn_if_none(result, "on_send") + return result async def on_publish(self, message: Any, *, sender: AgentId | None) -> Any | type[DropMessage]: result = message - return warn_if_none(result, "on_publish") + _warn_if_none(result, "on_publish") + return result async def on_response(self, message: Any, *, sender: AgentId, recipient: AgentId | None) -> Any | type[DropMessage]: result = message - return warn_if_none(result, "on_response") + _warn_if_none(result, "on_response") + return result From 120633bf123debef817232bb2877379193c026bb Mon Sep 17 00:00:00 2001 From: Pramod Goyal <81946962+goyalpramod@users.noreply.github.com> Date: Mon, 18 Nov 2024 07:29:59 +0530 Subject: [PATCH 4/4] Update intervention.py to remove redundant name change --- .../src/autogen_core/base/intervention.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/python/packages/autogen-core/src/autogen_core/base/intervention.py b/python/packages/autogen-core/src/autogen_core/base/intervention.py index 3c6a083cbe2..330049534e7 100644 --- a/python/packages/autogen-core/src/autogen_core/base/intervention.py +++ b/python/packages/autogen-core/src/autogen_core/base/intervention.py @@ -44,16 +44,13 @@ async def on_response( class DefaultInterventionHandler(InterventionHandler): async def on_send(self, message: Any, *, sender: AgentId | None, recipient: AgentId) -> Any | type[DropMessage]: - result = message - _warn_if_none(result, "on_send") - return result + _warn_if_none(message, "on_send") + return message async def on_publish(self, message: Any, *, sender: AgentId | None) -> Any | type[DropMessage]: - result = message - _warn_if_none(result, "on_publish") - return result + _warn_if_none(message, "on_publish") + return message async def on_response(self, message: Any, *, sender: AgentId, recipient: AgentId | None) -> Any | type[DropMessage]: - result = message - _warn_if_none(result, "on_response") - return result + _warn_if_none(message, "on_response") + return message