-
Notifications
You must be signed in to change notification settings - Fork 81
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 history use seperate collection {bot}_agent and routes c… #1802
agentic flow history use seperate collection {bot}_agent and routes c… #1802
Conversation
…orresponding to fetching
WalkthroughThis pull request introduces new FastAPI endpoints for retrieving agentic flow conversation data. Both general and user-specific retrieval functions are added in two router modules. Updates ensure that date parameters are validated and the collection names in MongoDB now include an "_agent" suffix. Additionally, integration and unit tests have been extended to cover the new endpoints and modified collection access. These changes enhance the API's ability to serve agentic flow conversation histories without altering existing functionalities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Router
participant History_Server
Client->>API_Router: GET /agentic_flow (from_date, to_date, collection)
API_Router->>History_Server: Request flattened conversation data
History_Server-->>API_Router: Return conversation data
API_Router-->>Client: Respond with agentic flow conversations
sequenceDiagram
participant Client
participant API_Router
participant History_Server
Client->>API_Router: GET /agentic_flow/user/{sender} (from_date, to_date, collection)
API_Router->>History_Server: Request user-specific conversation data
History_Server-->>API_Router: Return user conversation data
API_Router-->>Client: Respond with user agentic flow conversation history
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🧹 Nitpick comments (1)
tests/integration_test/history_client_test.py (1)
135-138
: Use context manager for file handling.Use a context manager (
with
statement) to ensure proper file handling and resource cleanup.-def history_conversations_agent(*args, **kwargs): - json_data = json.load(open("tests/testing_data/history/conversation_history_agent.json")) - return json_data, None +def history_conversations_agent(*args, **kwargs): + with open("tests/testing_data/history/conversation_history_agent.json") as file: + json_data = json.load(file) + return json_data, None🧰 Tools
🪛 Ruff (0.8.2)
136-136: Use a context manager for opening files
(SIM115)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
kairon/api/app/routers/history.py
(1 hunks)kairon/history/router/conversations.py
(1 hunks)kairon/shared/chat/agent/agent_flow.py
(1 hunks)tests/integration_test/history_client_test.py
(2 hunks)tests/integration_test/history_services_test.py
(2 hunks)tests/unit_test/agentic_flow_test.py
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- kairon/shared/chat/agent/agent_flow.py
🧰 Additional context used
🪛 Ruff (0.8.2)
kairon/api/app/routers/history.py
295-295: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
296-296: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
297-297: Do not perform function call Security
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
311-311: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
312-312: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
313-313: Do not perform function call Security
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
kairon/history/router/conversations.py
31-31: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
32-32: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
44-44: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
45-45: Do not perform function call Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
tests/integration_test/history_client_test.py
136-136: Use a context manager for opening files
(SIM115)
tests/integration_test/history_services_test.py
1139-1139: Use a context manager for opening files
(SIM115)
⏰ 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 (10)
kairon/history/router/conversations.py (2)
29-39
: LGTM! Clean implementation of the agentic flow conversations endpoint.The implementation follows the established pattern and correctly appends "_agent" to the collection name.
🧰 Tools
🪛 Ruff (0.8.2)
31-31: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
32-32: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
41-50
: LGTM! Clean implementation of the user-specific agentic flow history endpoint.The implementation follows the established pattern and correctly appends "_agent" to the collection name.
🧰 Tools
🪛 Ruff (0.8.2)
44-44: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
45-45: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
tests/unit_test/agentic_flow_test.py (1)
455-456
: Verify if removing the tag filter is intentional.The test now retrieves all documents from the collection without filtering by "tag". Please confirm if this change is intentional and doesn't affect the test's validity.
kairon/api/app/routers/history.py (2)
293-306
: LGTM! Clean implementation of the agentic flow conversations endpoint.The implementation follows the established pattern and correctly validates date ranges before triggering the history server request.
🧰 Tools
🪛 Ruff (0.8.2)
295-295: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
296-296: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
297-297: Do not perform function call
Security
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
308-322
: LGTM! Clean implementation of the user-specific agentic flow history endpoint.The implementation follows the established pattern and correctly validates date ranges before triggering the history server request.
🧰 Tools
🪛 Ruff (0.8.2)
311-311: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
312-312: Do not perform function call
Depends
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
313-313: Do not perform function call
Security
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
tests/integration_test/history_services_test.py (3)
1144-1155
: LGTM! Good test coverage for empty agentic flow conversations.The test verifies the response structure and empty data case correctly.
1159-1171
: LGTM! Good test coverage for agentic flow conversations.The test verifies the response structure and data count correctly.
1175-1188
: LGTM! Good test coverage for user-specific agentic flow conversations.The test verifies the response structure, data count, and sender ID correctly.
tests/integration_test/history_client_test.py (2)
1556-1578
: LGTM!The test is well-structured and follows the established patterns. It properly mocks the HTTP response and includes comprehensive assertions to verify the response data.
1581-1602
: LGTM!The test is well-structured and follows the established patterns. It properly mocks the HTTP response and includes comprehensive assertions to verify both the response data and the sender ID.
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.
approved
…orresponding to fetching
Summary by CodeRabbit
New Features
Tests