-
Notifications
You must be signed in to change notification settings - Fork 82
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
Agentic flow integration #1849
base: master
Are you sure you want to change the base?
Agentic flow integration #1849
Conversation
WalkthroughThis pull request introduces a new agentic flow feature integrated throughout the system. The CLI now supports an “agentic-flow” subcommand, while the event and scheduling frameworks are updated to differentiate between standard and flow-based actions. Several components—including mail processors, WhatsApp broadcasters, and chat agents—receive enhancements for flow execution, improved type annotations, caching, and error handling. In addition, tests across integration and unit suites have been expanded to cover the new functionalities and modifications. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (agentic_flow)
participant EVT as AgenticFlowEvent
participant FLOW as AgenticFlow
Note over CLI: User executes "agentic-flow" command
CLI->>EVT: exec_agentic_flow(args)
EVT->>EVT: Validate flow configuration
EVT->>FLOW: Call execute_rule(sender_id, slot_vals)
FLOW-->>EVT: Return response & errors
EVT-->>CLI: Output execution result
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (19)
kairon/shared/chat/agent/agent_flow.py (3)
132-132
: Consider usingraise ... from err
for better exception chainingWhen raising exceptions inside an except block, it's better to use
raise ... from err
to maintain the exception context.- raise AppException(e) + raise AppException(e) from e🧰 Tools
🪛 Ruff (0.8.2)
132-132: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
282-282
: Simplify dictionary key checkingUse
key in dict
instead ofkey in dict.keys()
for better readability and slight performance improvement.- for name in self.fake_tracker.slots.keys(): + for name in self.fake_tracker.slots:🧰 Tools
🪛 Ruff (0.8.2)
282-282: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
331-343
: New methodflow_exists
for checking flow existenceThis static method efficiently checks if a flow exists in the database. However, the return statement can be simplified.
- if Rules.objects(bot=bot, block_name=flow_name): - return True - if MultiflowStories.objects(bot=bot, block_name=flow_name): - return True - return False + return bool(Rules.objects(bot=bot, block_name=flow_name) or + MultiflowStories.objects(bot=bot, block_name=flow_name))🧰 Tools
🪛 Ruff (0.8.2)
341-343: Return the condition directly
Inline condition
(SIM103)
kairon/cli/agentic_flow.py (2)
18-19
: Incorrect help text for the agentic-flow commandThe help text says "Mail channel initiate reading" which is unrelated to agentic flows. It appears to be copied from another command.
agentic_fow_parser = subparsers.add_parser( "agentic-flow", conflict_handler="resolve", formatter_class=ArgumentDefaultsHelpFormatter, parents=parents, - help="Mail channel initiate reading" + help="Execute an agentic flow with specified parameters" )
32-34
: Add validation and improve documentation for slot_data argumentThe
slot_data
parameter expects a JSON string, but there's no validation to ensure it's valid JSON, nor clear documentation about its expected format.Consider adding explicit JSON parsing and better documentation:
agentic_fow_parser.add_argument('slot_data', type=str, - help="json containing slot values dictionary", action='store') + help="JSON string containing slot values (e.g., '{\"key\": \"value\"}')", action='store')And in the
exec_agentic_flow
function:import json from json.decoder import JSONDecodeError def exec_agentic_flow(args): + try: + slot_data = json.loads(args.slot_data) if args.slot_data else {} + AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=slot_data) + except JSONDecodeError: + print(f"Error: Invalid JSON format in slot_data: {args.slot_data}") + return 1 - AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=args.slot_data)tests/unit_test/cli_test.py (1)
337-344
: Well-structured test class for AgenticFlowCli.The test class follows the established patterns in the codebase and properly sets up the test environment.
According to static analysis, it would be better to use a capitalized environment variable name:
- os.environ["system_file"] = "./tests/testing_data/system.yaml" + os.environ["SYSTEM_FILE"] = "./tests/testing_data/system.yaml"However, since all other tests use the lowercase version, it's more important to stay consistent with the existing codebase.
🧰 Tools
🪛 Ruff (0.8.2)
341-341: Use capitalized environment variable
SYSTEM_FILE
instead ofsystem_file
Replace
system_file
withSYSTEM_FILE
(SIM112)
tests/integration_test/action_service_test.py (1)
14307-14403
: Well-structured test for flow-based schedule actions.The new test case
test_schedule_action_execution_flow
provides comprehensive coverage for the flow-based schedule action functionality. It correctly:
- Sets up the necessary mock environment
- Creates a schedule action with appropriate type and parameters
- Executes the action via the webhook endpoint
- Verifies both the response and log entries
- Validates the scheduled job's configuration
Particularly good is the verification of
EventClass.agentic_flow
in the job state, which confirms the correct event type is being used for the new flow execution functionality.One suggestion to consider: add a negative test case that verifies error handling when an invalid flow name is provided.
kairon/events/definitions/agentic_flow.py (2)
25-28
: Correct the docstring to reflect actual validation logic.The docstring mentions validating a "mail channel," but the code checks whether the specified flow name exists. Consider updating the description to align with the actual validation of
flow_name
.- validate mail channel exists and works properly + Validate that the specified flow name exists for the bot
45-47
: Use exception chaining to clarify error origin.Raising exceptions within the
except
block withoutfrom
can obscure the original cause. Consider chaining them:- raise AppException(e) + raise AppException(e) from e- raise AppException(f"Failed to execute flow {self.flow_name} for bot {self.bot}. Error: {str(e)}") + raise AppException(f"Failed to execute flow {self.flow_name} for bot {self.bot}. Error: {str(e)}") from eAlso applies to: 69-70
🧰 Tools
🪛 Ruff (0.8.2)
47-47: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
tests/unit_test/agentic_flow_test.py (2)
471-488
: Add more negative test scenarios.While this test verifies the creation of a tracker with valid slot data, consider adding a scenario where no slot values are provided or an invalid slot name is used. This can help ensure robustness in
create_fake_tracker
.
490-513
: Expand coverage for empty or erroneous rule events.
test_execute_rule_2
focuses on a normal flow. Consider testing edge cases wheremock_load_rule_events
returns an empty list or an unexpected data structure, ensuringexecute_rule
handles malformed rules gracefully.tests/unit_test/channels/whatsapp_broadcast_test.py (1)
639-645
: Remove unused local variables to adhere to best practices.Variables like
mock_get_client
,mock_channel_client
, andmock_init
are assigned but never used. Please remove them to maintain clean, warning-free code.- with patch.object(WhatsappBroadcast, '_WhatsappBroadcast__get_client', return_value=MagicMock()) as mock_get_client, \ - patch.object(whatsapp_broadcast, 'channel_client', new_callable=MagicMock) as mock_channel_client, \ - patch.object(AgenticFlow, '__init__', return_value=None) as mock_init, \ + with patch.object(WhatsappBroadcast, '_WhatsappBroadcast__get_client', return_value=MagicMock()), \ + patch.object(whatsapp_broadcast, 'channel_client', new_callable=MagicMock()), \ + patch.object(AgenticFlow, '__init__', return_value=None), \Also applies to: 681-686
🧰 Tools
🪛 Ruff (0.8.2)
639-639: Local variable
mock_get_client
is assigned to but never usedRemove assignment to unused variable
mock_get_client
(F841)
640-640: Local variable
mock_channel_client
is assigned to but never usedRemove assignment to unused variable
mock_channel_client
(F841)
643-643: Local variable
mock_init
is assigned to but never usedRemove assignment to unused variable
mock_init
(F841)
kairon/actions/definitions/schedule.py (3)
103-115
: Ensure robust error handling for PYSCRIPT
When triggering a PYSCRIPT-based schedule, consider gracefully handling missing or invalidpyscript_code
in thecallback
. You could add conditional checks to provide a clearer error message ifcallback['pyscript_code']
is not found or is empty.if not callback.get('pyscript_code'): + raise ActionFailure("No pyscript code configured for schedule action.") event_data = ...
116-130
: Validate JSON for flow-based scheduling
When serializingschedule_data
viajson.dumps
, ensure the content is always serializable (e.g., no custom objects). Consider adding a try/except or pre-validation to avoid potential crashes on invalid data types.+import json try: slot_str = json.dumps(schedule_data) except (TypeError, ValueError) as e: + raise ActionFailure(f"Could not serialize slot data: {str(e)}") event_data = { 'data': { 'slot_data': slot_str, ...
170-175
: Separate tasks for flows and actions
The updatedif is_flow:
block setsTASK_TYPE.EVENT
, while the else setsTASK_TYPE.ACTION
. As a minor improvement, consider adding docstrings or inline comments that clarify the difference between flow-based events and standard action tasks.kairon/shared/channels/mail/processor.py (2)
237-243
: Flow execution and error handling
Flow errors are raised asAppException
iferrors
is non-empty. This clearly signals partial execution failures. If you want to capture partial success, consider logging the partial output before raising the exception.
251-252
: Detailed exception logging
CatchingAppException
and logging it is standard. Consider adding more context (e.g.flowname
orsender_id
) to the log to facilitate debugging of flow errors.kairon/shared/channels/broadcast/whatsapp.py (1)
57-58
: Extended signature forsend_template_message
Addingflowname: Text = None
provides more flexibility. Document howflowname
overrides or supplements standard components to avoid confusion.tests/unit_test/events/definitions_test.py (1)
1294-1308
: Test covers JSON string slot data format.This test effectively verifies that the execute method correctly handles slot data when provided as a JSON string instead of a dictionary. This is important for ensuring flexibility in how data can be passed to the method.
However, the indentation on lines 1298-1302 is inconsistent with the rest of the code.
mock_get_current_user_and_bot.return_value = type('User', (object,), { 'get_bot': lambda: 'test_bot', - 'get_user': lambda: 'test_user'} - ) + 'get_user': lambda: 'test_user' + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
kairon/__init__.py
(2 hunks)kairon/actions/definitions/schedule.py
(6 hunks)kairon/cli/agentic_flow.py
(1 hunks)kairon/events/definitions/agentic_flow.py
(1 hunks)kairon/events/definitions/factory.py
(2 hunks)kairon/shared/actions/data_objects.py
(3 hunks)kairon/shared/callback/data_objects.py
(7 hunks)kairon/shared/channels/broadcast/whatsapp.py
(5 hunks)kairon/shared/channels/mail/processor.py
(5 hunks)kairon/shared/chat/agent/agent_flow.py
(9 hunks)kairon/shared/chat/broadcast/constants.py
(1 hunks)kairon/shared/chat/broadcast/data_objects.py
(1 hunks)kairon/shared/chat/models.py
(1 hunks)kairon/shared/constants.py
(1 hunks)tests/integration_test/action_service_test.py
(7 hunks)tests/unit_test/agentic_flow_test.py
(3 hunks)tests/unit_test/channels/mail_channel_test.py
(3 hunks)tests/unit_test/channels/whatsapp_broadcast_test.py
(2 hunks)tests/unit_test/cli_test.py
(1 hunks)tests/unit_test/events/definitions_test.py
(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/unit_test/cli_test.py
341-341: Use capitalized environment variable SYSTEM_FILE
instead of system_file
Replace system_file
with SYSTEM_FILE
(SIM112)
kairon/events/definitions/agentic_flow.py
47-47: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
70-70: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
tests/unit_test/channels/whatsapp_broadcast_test.py
639-639: Local variable mock_get_client
is assigned to but never used
Remove assignment to unused variable mock_get_client
(F841)
640-640: Local variable mock_channel_client
is assigned to but never used
Remove assignment to unused variable mock_channel_client
(F841)
643-643: Local variable mock_init
is assigned to but never used
Remove assignment to unused variable mock_init
(F841)
656-656: Redefinition of unused test_send_template_message_success_with_flow
from line 614
(F811)
681-681: Local variable mock_get_client
is assigned to but never used
Remove assignment to unused variable mock_get_client
(F841)
682-682: Local variable mock_channel_client
is assigned to but never used
Remove assignment to unused variable mock_channel_client
(F841)
685-685: Local variable mock_init
is assigned to but never used
Remove assignment to unused variable mock_init
(F841)
kairon/shared/chat/agent/agent_flow.py
132-132: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
282-282: Use key in dict
instead of key in dict.keys()
Remove .keys()
(SIM118)
341-343: Return the condition directly
Inline condition
(SIM103)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python CI
- GitHub Check: Analyze (python)
🔇 Additional comments (55)
kairon/shared/constants.py (1)
85-85
: Clean addition ofagentic_flow
toEventClass
enumThe addition of the
agentic_flow
member to theEventClass
enum follows the existing pattern and properly integrates with the event handling framework.kairon/__init__.py (2)
48-48
: Import statement updated to includeagentic_flow
moduleThe import statement has been properly updated to include the new
agentic_flow
module for CLI integration.
67-67
: Registeredagentic_flow
with command-line parserThe
agentic_flow.add_subparser()
call correctly registers the new agentic flow functionality as a subcommand in the CLI.kairon/shared/chat/agent/agent_flow.py (7)
62-62
: Good refactoring of tracker creationThe constructor now uses the new
create_fake_tracker
method, enhancing code maintainability.
70-71
: Cache initializationAdding a cache object for storing rule events is a good performance optimization.
72-77
: Well-structuredcreate_fake_tracker
methodThis method nicely encapsulates the logic for tracker creation and improves code modularity.
108-128
: Improvedload_rule_events
with cachingThe method now checks if the requested rule is already cached before querying the database, reducing redundant calls and improving performance.
135-135
: Return type annotation added tosanitize_multiflow_events
Good addition of explicit return type which clarifies the method's output structure.
176-182
: Enhanced type annotation and documentation forevaluate_criteria
The method's return type is now explicitly annotated as
Any|None
and a helpful docstring has been added.
192-208
: Improved flexibility inexecute_rule
methodThe method now accepts optional parameters for dynamic updates to the tracker and slot values.
kairon/shared/callback/data_objects.py (7)
102-102
: Added type annotations toget_entry
methodThe explicit type annotations for parameters and return value improve code clarity and type safety.
150-150
: Type annotations added toget_auth_token
methodThe method signature now clearly indicates it returns a tuple containing a string and a boolean.
284-284
: Enhancedget_value_from_json
with type annotationsType hints have been added to improve code readability and maintainability.
298-298
: Complete type annotations forvalidate_entry
methodThe method now has clear type annotations for both parameters and return value.
366-366
: Return type annotation added tocreate_success_entry
The explicit return type annotation clarifies the method's output.
399-399
: Return type annotation added tocreate_failure_entry
The explicit return type annotation enhances code readability and type safety.
422-422
: Complete type annotations forget_logs
methodThe method signature now clearly indicates it returns a tuple containing a list of dictionaries and an integer.
kairon/shared/chat/broadcast/constants.py (1)
16-16
: LGTM: Added new broadcast type for agentic flowsThe addition of the
flow
type to theMessageBroadcastType
enum is appropriate and aligns with the PR's objective of integrating agentic flows with message broadcasting capabilities.kairon/events/definitions/factory.py (1)
1-1
: LGTM: Properly registered AgenticFlowEvent in the factoryThe new event type is imported correctly and registered in the
EventFactory
following the existing pattern, which enables the system to handle agentic flow events.Also applies to: 27-27
kairon/shared/actions/data_objects.py (2)
967-970
: Good addition of ScheduleActionType enum.The addition of the
ScheduleActionType
enum is well-structured and provides a clear way to distinguish between different types of scheduled actions. This sets a good foundation for the agentic flow integration.
980-982
: Clean implementation of the schedule_action_type field.The field is properly configured with a default value and appropriate choices from the enum. Good practice using the enum values for the choices parameter.
kairon/shared/chat/broadcast/data_objects.py (1)
86-86
: Good expansion of broadcast_type options.Adding the flow option to the broadcast_type field enables the new agentic flow integration feature for message broadcasting.
tests/unit_test/channels/mail_channel_test.py (2)
260-261
: Good update to slot format in tests.The change from list format to dictionary format for slots matches the implementation changes in the agentic flow integration. This ensures the tests properly verify the current behavior.
Also applies to: 268-269
405-405
: Correct adaptation to the new AgenticFlow implementation.The test now correctly mocks
AgenticFlow.execute_rule
instead ofChatUtils.process_messages_via_bot
and updates the expected return format, aligning with the new implementation for handling mail messages through the agentic flow system.Also applies to: 428-428
tests/unit_test/cli_test.py (2)
345-356
: Good test coverage for CLI command execution.The test correctly verifies that the CLI command correctly passes parameters to the
AgenticFlowEvent.execute
method.
357-368
: Comprehensive direct function test.This test ensures that the function works correctly when called directly, not just through the CLI parser. Good practice to test both paths.
tests/integration_test/action_service_test.py (5)
38-38
: Imports updated for agentic flow integration.The addition of
ScheduleActionType
andEventClass
imports supports the new agentic flow functionality being integrated across the system. These imports are correctly placed and will allow the tests to verify the new flow-based action scheduling mechanism.Also applies to: 45-45
13919-13919
: Assertion updated to include execution_info field.The test assertion has been appropriately updated to include the new
execution_info
field which is set toNone
for failure scenarios. This reflects the architectural changes made to support different execution types beyond just script code.
13985-13986
: Log structure updated for schedule actions.The log verification has been properly updated to handle the new structure with modified field set. The additions of
timezone
andexecution_info
fields align with the changes in the implementation to support agentic flows.Also applies to: 13993-13994
14081-14081
: Structured execution_info replaces direct code reference.Good restructuring of the execution information with proper type identification. The change from directly using
pyscript_code
to a structured format with type information facilitates the polymorphic behavior needed for supporting different execution types.
14183-14183
: Consistent updates to execution_info structure across test cases.The execution info structure has been consistently updated across all test cases, providing clear type information alongside the code to be executed. This is a good practice that ensures consistent testing patterns.
Also applies to: 14289-14289
tests/unit_test/agentic_flow_test.py (1)
516-539
: Comprehensive coverage of flow existence checks.These tests effectively cover all scenarios (
rules
present,multiflow
present, none present). The logic looks sound.tests/unit_test/channels/whatsapp_broadcast_test.py (2)
569-570
: Verification of error structure appears correct.Checking for the standard error payload is consistent with the broadcast failure scenario. Looks good.
575-609
: Solid test coverage for flow-based broadcasting.Your approach to patching and verifying calls ensures clarity on each step. No issues here.
kairon/actions/definitions/schedule.py (3)
24-24
: New import usage
The addition ofScheduleActionType
is clearly aligned with the new branching logic below. No issues found.
83-84
: Initialized placeholders
execution_info
andevent_data
are introduced here to store additional details on scheduled actions. This is straightforward and helps keep the code organized.
164-164
: Newis_flow
parameter
This optional parameter extends scheduling flexibility, defaulting toFalse
. It’s a clear approach to distinguish flow-based jobs from other tasks.kairon/shared/channels/mail/processor.py (6)
14-14
: LeveragingAgenticFlow
ImportingAgenticFlow
underscores the new flow-driven approach for mail processing.
29-30
: Fallback toflowname
Usingflowname
whenintent
is not set maintains consistency with flow-based logic. Ensureself.config
always contains a validflowname
in scenarios requiring flow execution.
170-171
: Simplified slot retrieval
Directly retrieving slots as a dictionary (slots = rasa_chat_response.get('slots', {})
) is cleaner and ensures no type errors if slots are missing.
217-218
: Creating flow and mail processor objects
The introduction offlow = AgenticFlow(bot)
andmp = MailProcessor(bot)
sets the foundation for subsequent flow execution. Confirm that theMailProcessor
does not re-initialize config or states unexpectedly for large mail batches.
230-231
: Initializing user messages with flow intent
Assigninguser_msg = mp.intent
ensures consistent usage of the fallback or configured flow name. Confirm thatmp.intent
is neverNone
or an empty string, which might cause silent failures.
247-248
: Processing mail with Rasa chat response
Usingmp.process_mail(chat_response, log_id=...)
ensures that the new flow responses are included, but confirm that the updated slot format matches the mail template expansions.kairon/shared/channels/broadcast/whatsapp.py (3)
15-15
: Import for agentic flows
The import statement forAgenticFlow
aligns with the new flow-based broadcast.
52-53
: Flow-based broadcast condition
Checkingself.config["broadcast_type"] == MessageBroadcastType.flow.value
triggers a distinct flow path. This is an effective way to isolate new behavior.
62-71
: Transforming flow responses to template components
The snippet where text or custom data from the flow is converted tocomponents
is clever. Ensure the user’s response format always matches the broadcast template structure (list or dictionary).tests/unit_test/events/definitions_test.py (9)
15-15
: New import looks good for including the AgenticFlowEvent class.This import is part of the necessary changes to support testing the new agentic flow integration.
1254-1270
: Well-structured test for enqueue functionality.The test correctly validates that the AgenticFlowEvent.enqueue method:
- Calls the validate method
- Invokes the request_event_server with the correct parameters
- Passes along the flow name and slot data appropriately
Good use of mocking to isolate the test from external dependencies.
1271-1278
: Good error handling test.This test correctly verifies that the AgenticFlowEvent.enqueue method properly transforms exceptions from the request_event_server into AppExceptions, maintaining a consistent error handling pattern throughout the codebase.
1279-1293
: Well-implemented execute method test.The test properly verifies that:
- Authentication is properly handled
- The AgenticFlow is initialized
- The execute_rule method is called with the correct parameters
- Success cases are handled appropriately
Good use of mocking for all external dependencies.
1309-1324
: Good test for error handling during execution.This test properly verifies that when execute_rule returns errors, the method correctly raises an AppException with a meaningful error message that includes the specific errors. This ensures users receive actionable feedback when flows fail to execute.
1325-1339
: Well-implemented test for exception handling.This test verifies that unexpected exceptions during flow execution are properly caught and wrapped in AppExceptions with context-specific error messages. The error message includes both the flow name and bot identifier, which is useful for debugging.
1340-1347
: Good validation test.This test effectively verifies that the validate method checks if the flow exists for the specified bot and flow name. The correct patching of the flow_exists method ensures the test is isolated from external dependencies.
1348-1353
: Test covers edge case with no flow name.This test appropriately verifies that the validate method behaves correctly when no flow_name is provided (returning None instead of checking flow existence). This is important for ensuring robust behavior in edge cases.
1254-1353
: Comprehensive test coverage for the AgenticFlowEvent class.The test suite for AgenticFlowEvent is well-structured and thorough, covering:
- Success paths for enqueueing and executing flows
- Error handling for both predictable errors and unexpected exceptions
- Input validation and edge cases
- Different input formats (dictionary vs JSON string for slot data)
This comprehensive test coverage is essential for ensuring the reliability of the new agentic flow integration across the system.
def exec_agentic_flow(args): | ||
AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=args.slot_data) |
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.
🛠️ Refactor suggestion
Add error handling for AgenticFlowEvent execution
The function doesn't handle any exceptions that might be raised during the execution of the agentic flow, which could lead to ungraceful termination of the CLI command.
def exec_agentic_flow(args):
+ try:
AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=args.slot_data)
+ return 0
+ except Exception as e:
+ print(f"Error executing agentic flow '{args.flow_name}': {str(e)}")
+ return 1
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
def exec_agentic_flow(args): | |
AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=args.slot_data) | |
def exec_agentic_flow(args): | |
try: | |
AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=args.slot_data) | |
return 0 | |
except Exception as e: | |
print(f"Error executing agentic flow '{args.flow_name}': {str(e)}") | |
return 1 |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/integration_test/services_test.py
(2 hunks)tests/unit_test/data_processor/data_processor_test.py
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python CI
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
tests/integration_test/services_test.py (2)
5636-5636
: Good addition of the new schedule_action_type field.The new
schedule_action_type
field properly identifies this as a 'pyscript' action, consistent with the agentic flow integration being implemented in this PR. This addition helps clarify the type of scheduled action and aligns with the newScheduleActionType
enumeration.
5702-5703
: Approve both the action name change and type addition.The changes here serve two purposes:
- Update the action name to 'test_pyscript_new'
- Add the action type classification as 'pyscript'
These changes properly align the test expectations with the updated scheduling system that now differentiates between different types of scheduled actions.
tests/unit_test/data_processor/data_processor_test.py (2)
18653-18659
: Good addition of schedule action type parameterThe addition of
'schedule_action_type': 'pyscript'
properly specifies the type of schedule action being tested. This aligns with the new agentic flow integration where actions can be differentiated between script-based and flow-based types.
18791-18797
: Consistent implementation of schedule action typeAdding the
'schedule_action_type': 'pyscript'
parameter here maintains consistency with the previous test case. This ensures proper testing of the schedule action functionality with the new action type differentiation introduced in the agentic flow integration.
@@ -5633,6 +5633,7 @@ def test_list_schedule_action(): | |||
'schedule_time': {'value': '2024-08-06T09:00:00.000+0530', 'parameter_type': 'value'}, | |||
'timezone': 'UTC', | |||
'schedule_action': 'test_pyscript', | |||
'schedule_action_type': 'pyscript', |
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.
💡 Verification agent
🧩 Analysis chain
Consider using the enum value instead of string literals.
Since the PR introduces a ScheduleActionType
enumeration (with PYSCRIPT
and FLOW
members), consider using the enum value in tests rather than string literals to ensure consistency and make tests more resistant to enum value changes.
- 'schedule_action_type': 'pyscript',
+ 'schedule_action_type': ScheduleActionType.PYSCRIPT.value,
Also applies to: 5703-5703
🏁 Script executed:
#!/bin/bash
# Check if the ScheduleActionType enum is already imported or used in this file
rg -A 5 "ScheduleActionType" "tests/integration_test/services_test.py"
# Find enum definition to understand its implementation
rg -A 5 "class ScheduleActionType" --type py
Length of output: 462
Action Required: Use Enum Values in Tests Instead of String Literals
Please update the test assertions at the following locations in tests/integration_test/services_test.py
so that they reference the enum values directly. This will ensure that the tests remain consistent with any changes to the enum definitions:
- Line 5636: Replace the string literal
'pyscript'
withScheduleActionType.PYSCRIPT.value
. - Line 5703: Similarly, update the string literal to use
ScheduleActionType.PYSCRIPT.value
(or the appropriate enum member for any similar value).
Additionally, ensure that the file imports ScheduleActionType
from kairon/shared/actions/data_objects.py
at the top of the file. For example:
from kairon.shared.actions.data_objects import ScheduleActionType
- 'schedule_action_type': 'pyscript',
+ 'schedule_action_type': ScheduleActionType.PYSCRIPT.value,
[fact_check_passed]
Agentic flow added to - whatsapp broadcast, mail channel, schedule action
Summary by CodeRabbit
New Features
Bug Fixes
Tests