Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hasinaxp
Copy link
Contributor

@hasinaxp hasinaxp commented Mar 11, 2025

Agentic flow added to - whatsapp broadcast, mail channel, schedule action

Summary by CodeRabbit

  • New Features

    • Introduced advanced flow execution accessible from the command-line, enabling dynamic bot workflows.
    • Enhanced scheduled actions to support both scripted and flow-driven operations.
    • Expanded messaging capabilities with flow-based broadcasting for WhatsApp and chat channels.
    • Streamlined mail processing by integrating dynamic flow execution with improved error handling and logging.
    • Added support for agentic flow events, allowing for more flexible event handling within the bot framework.
  • Bug Fixes

    • Improved error handling and logging during flow execution to enhance reliability.
  • Tests

    • Added comprehensive test coverage for the new agentic flow features, including CLI execution and event handling.
    • Updated tests to reflect changes in scheduled action structures and introduced new tests for agentic flow functionalities.

Copy link

coderabbitai bot commented Mar 11, 2025

Walkthrough

This 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

File(s) Change Summary
kairon/__init__.py, kairon/cli/agentic_flow.py Added the agentic_flow module; updated the CLI parser to include a new subcommand and corresponding execution functions.
kairon/events/definitions/agentic_flow.py, kairon/events/definitions/factory.py, kairon/shared/constants.py Introduced AgenticFlowEvent with validation, enqueue, and execution methods; updated event mapping and added a new event type constant (agentic_flow).
kairon/actions/definitions/schedule.py, kairon/shared/actions/data_objects.py Enhanced scheduling to support both PYSCRIPT and FLOW types using a new enum (ScheduleActionType), updated variable names (execution_info), and modified method signatures (added is_flow parameter).
kairon/shared/callback/data_objects.py Added explicit type annotations to methods in callback configuration and logging classes for improved clarity and type safety.
kairon/shared/channels/broadcast/whatsapp.py, kairon/shared/channels/mail/processor.py Integrated flow-based message sending: added a new private method for flow broadcasting, updated template message signatures to include a flowname, and refined mail processing (intent fallback and slot format changes).
kairon/shared/chat/agent/agent_flow.py Updated the AgenticFlow class with a modular tracker creation method (create_fake_tracker), caching in load_rule_events, enhanced rule execution (optional parameters for sender and slots), and a new flow_exists method.
kairon/shared/chat/broadcast/constants.py, kairon/shared/chat/broadcast/data_objects.py, kairon/shared/chat/models.py Expanded broadcast settings by adding a new enum member (flow) and a flowname field to allow flow-based message broadcasting.
Tests (tests/integration_test/..., tests/unit_test/...) Updated and added tests to validate scheduling changes, agentic flow event behavior, CLI integration for agentic flow, mail processing adjustments, and WhatsApp broadcasting. Note: a duplicate test method was observed in WhatsApp broadcast tests.

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
Loading

Possibly related PRs

  • Agentic flow imp #1780: The changes in the main PR, which involve the integration of the agentic_flow module into the Kairon CLI, are related to the retrieved PR as both involve modifications to the AgenticFlow class and its associated functionalities, specifically in how agentic flows are executed and managed.
  • content importer fix #1527: The changes in the main PR are related to those in the retrieved PR as both involve modifications to the create_argument_parser function in the kairon/__init__.py file, specifically adding new modules (agentic_flow and content_importer) to the command-line argument parser.

Suggested reviewers

  • hiteshghuge

Poem

In fields of code where bugs once played,
I hop along with joy unafraid.
Agentic flows guide my leaps so bright,
Crafting paths from day to night.
My rabbit heart sings in every line,
Celebrating fresh changes divine!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0e35e47 and 7c43bac.

📒 Files selected for processing (2)
  • kairon/shared/channels/broadcast/whatsapp.py (5 hunks)
  • kairon/shared/channels/broadcast/whatsapp.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • kairon/shared/channels/broadcast/whatsapp.py
  • kairon/shared/channels/broadcast/whatsapp.py
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Analyze (python)
  • GitHub Check: Python CI

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 using raise ... from err for better exception chaining

When 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 with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


282-282: Simplify dictionary key checking

Use key in dict instead of key 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 of key in dict.keys()

Remove .keys()

(SIM118)


331-343: New method flow_exists for checking flow existence

This 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 command

The 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 argument

The 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 of system_file

Replace system_file with SYSTEM_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:

  1. Sets up the necessary mock environment
  2. Creates a schedule action with appropriate type and parameters
  3. Executes the action via the webhook endpoint
  4. Verifies both the response and log entries
  5. 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 without from 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 e

Also applies to: 69-70

🧰 Tools
🪛 Ruff (0.8.2)

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)

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 where mock_load_rule_events returns an empty list or an unexpected data structure, ensuring execute_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, and mock_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 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)

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 invalid pyscript_code in the callback. You could add conditional checks to provide a clearer error message if callback['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 serializing schedule_data via json.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 updated if is_flow: block sets TASK_TYPE.EVENT, while the else sets TASK_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 as AppException if errors 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
Catching AppException and logging it is standard. Consider adding more context (e.g. flowname or sender_id) to the log to facilitate debugging of flow errors.

kairon/shared/channels/broadcast/whatsapp.py (1)

57-58: Extended signature for send_template_message
Adding flowname: Text = None provides more flexibility. Document how flowname 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

📥 Commits

Reviewing files that changed from the base of the PR and between be3ab3c and 4530704.

📒 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 of agentic_flow to EventClass enum

The addition of the agentic_flow member to the EventClass enum follows the existing pattern and properly integrates with the event handling framework.

kairon/__init__.py (2)

48-48: Import statement updated to include agentic_flow module

The import statement has been properly updated to include the new agentic_flow module for CLI integration.


67-67: Registered agentic_flow with command-line parser

The 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 creation

The constructor now uses the new create_fake_tracker method, enhancing code maintainability.


70-71: Cache initialization

Adding a cache object for storing rule events is a good performance optimization.


72-77: Well-structured create_fake_tracker method

This method nicely encapsulates the logic for tracker creation and improves code modularity.


108-128: Improved load_rule_events with caching

The 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 to sanitize_multiflow_events

Good addition of explicit return type which clarifies the method's output structure.


176-182: Enhanced type annotation and documentation for evaluate_criteria

The method's return type is now explicitly annotated as Any|None and a helpful docstring has been added.


192-208: Improved flexibility in execute_rule method

The 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 to get_entry method

The explicit type annotations for parameters and return value improve code clarity and type safety.


150-150: Type annotations added to get_auth_token method

The method signature now clearly indicates it returns a tuple containing a string and a boolean.


284-284: Enhanced get_value_from_json with type annotations

Type hints have been added to improve code readability and maintainability.


298-298: Complete type annotations for validate_entry method

The method now has clear type annotations for both parameters and return value.


366-366: Return type annotation added to create_success_entry

The explicit return type annotation clarifies the method's output.


399-399: Return type annotation added to create_failure_entry

The explicit return type annotation enhances code readability and type safety.


422-422: Complete type annotations for get_logs method

The 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 flows

The addition of the flow type to the MessageBroadcastType 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 factory

The 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 of ChatUtils.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 and EventClass 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 to None 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 and execution_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 of ScheduleActionType is clearly aligned with the new branching logic below. No issues found.


83-84: Initialized placeholders
execution_info and event_data are introduced here to store additional details on scheduled actions. This is straightforward and helps keep the code organized.


164-164: New is_flow parameter
This optional parameter extends scheduling flexibility, defaulting to False. It’s a clear approach to distinguish flow-based jobs from other tasks.

kairon/shared/channels/mail/processor.py (6)

14-14: Leveraging AgenticFlow
Importing AgenticFlow underscores the new flow-driven approach for mail processing.


29-30: Fallback to flowname
Using flowname when intent is not set maintains consistency with flow-based logic. Ensure self.config always contains a valid flowname 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 of flow = AgenticFlow(bot) and mp = MailProcessor(bot) sets the foundation for subsequent flow execution. Confirm that the MailProcessor does not re-initialize config or states unexpectedly for large mail batches.


230-231: Initializing user messages with flow intent
Assigning user_msg = mp.intent ensures consistent usage of the fallback or configured flow name. Confirm that mp.intent is never None or an empty string, which might cause silent failures.


247-248: Processing mail with Rasa chat response
Using mp.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 for AgenticFlow aligns with the new flow-based broadcast.


52-53: Flow-based broadcast condition
Checking self.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 to components 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:

  1. Calls the validate method
  2. Invokes the request_event_server with the correct parameters
  3. 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:

  1. Authentication is properly handled
  2. The AgenticFlow is initialized
  3. The execute_rule method is called with the correct parameters
  4. 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:

  1. Success paths for enqueueing and executing flows
  2. Error handling for both predictable errors and unexpected exceptions
  3. Input validation and edge cases
  4. 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.

Comment on lines +8 to +9
def exec_agentic_flow(args):
AgenticFlowEvent(args.bot, args.user).execute(flow_name=args.flow_name, slot_data=args.slot_data)
Copy link

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.

Suggested change
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

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4530704 and 0e35e47.

📒 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 new ScheduleActionType enumeration.


5702-5703: Approve both the action name change and type addition.

The changes here serve two purposes:

  1. Update the action name to 'test_pyscript_new'
  2. 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 parameter

The 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 type

Adding 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',
Copy link

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' with ScheduleActionType.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]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant