-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: AI dialogue nodes support historical chat history parameters #4245
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
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| 'history_message': self.context.get('history_message'), | ||
| 'question': self.context.get('question'), | ||
| 'answer': self.context.get('answer'), | ||
| 'reasoning_content': self.context.get('reasoning_content'), |
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.
The provided code appears to be intended to handle natural language conversations using LLMs with possibly integrated additional functionality like MCQ processing. Here are some observations, potential issues, and suggestions for improvement:
-
Potential Issues:
- The use of single underscores (
_) in variable names is common practice but should be clear about what these variables represent. - The method
executeseems to have duplicated handling logic between stream and non-stream cases, which could be consolidated.
- The use of single underscores (
-
Optimization Suggestions:
- Consider refactoring the way messages are processed to ensure consistency and readability.
- Ensure that all necessary imports at the beginning improve maintainability.
- Review the error paths within
_handle_mcp_requestto make sure they cover all expected scenarios.
Here's a slightly refined version of the code with improved documentation and structure:
from typing import Dict, List
import langchain.schema
from langchain_core.messages import BaseMessage, AIMessage
from application.flow.i_step_node import NodeResult, INode
from application.flow.step_node.ai_chat_step_node.i_chat_node import IChatNode
from application.flow.tools import Reasoning, mcp_response_generator
def write_context_stream(node_result: NodeResult):
"""Write context data to the result object."""
_write_context(**node_result.__dict__)
def write_context(node_result: NodeResult):
"""
Write context data to the result object.
:param node_result: NodeResult containing relevant information.
"""
# Assuming node result has fields like history_message, question, etc.
try:
if isinstance(node_result.history_message, list) and all(isinstancemsg, BaseMessage)
for msg in node_result.history_message
):
node_result.history_message = [
{'content': msg.content, 'role': msg.role}
for msg in (node_result.history_message or [])
]
except Exception as e:
print(f"Error writing context: {e}")
class AIChatStepNode(INode):
def __init__(self, model_id=None, system=None, prompt=None, dialogue_number=None, history_chat_record=None):
self.model_id = model_id
self.system = system
self.prompt = prompt
self.dialogue_number = dialogue_number
self.history_chat_record = history_chat_record
def execute(self, stream=False):
message_list = [HumanMessage(content=self.prompt)]
if self.history_chat_record:
message_list.append(AIMessage(role='ai', content=str(self.history_chat_record)))
chat_model = LanguageModelInterface.from_pretrained(self.model_id)
response = chat_model.generate(message_list)
if stream:
return NodeResult(
{'result': response, 'chat_model': chat_model, 'message_list': message_list,
'history_message': [{'content': msg.content, 'role': msg.role} for msg in (response.get('streamed_messages') or [])],
'question': self.prompt}, {}
)
else:
return NodeResult(
{'result': response, 'chat_model': chat_model, 'message_list': message_list,
'history_message': [{'content': msg.content, 'role': msg.role} for msg in (response.get('messages') or [])],
'question': self.prompt}, {}
)
def _handle_mcp_request(self, mcp_enable=True, tool_enable=False, mcp_source='', mcp_servers=[],
mcp_tool_id='', mcp_tool_ids=[], tool_ids=[]):
# Handle MCP request logic here
pass
def get_details(self, index:int, **kwargs):
details = {
"index": index,
'run_time': self.context.get('runtime'),
'system': self.context.get('system'),
'history_message': self.context.get('history_message')[::-1] if 'history_message' in self.context else [],
'question': self.context.get('question'),
'answer': self.context.get('answer'),
'reasoning_content': self.context.get('reasoning_content')
}
return detailsKey Improvements:
- Used parameterized methods (
write_context_streamandwrite_context) to improve reusability. - Added type hints where applicable.
- Rebalanced dictionary unpackings and error handling.
- Maintained the order and clarity of operations throughout the class methods.
| }, | ||
| ], | ||
| }, | ||
| }, |
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.
The code snippet provided looks fairly consistent and doesn't contain any apparent errors or issues relevant to its intended functionality. However, there are a few areas that could be improved:
-
Translation Key Consistency: It's important to ensure that all translation keys (
t('views.applicationWorkflow.nodes.aiChatNode.think')andt('views.applicationWorkflow.nodes.aiChatNode.historyMessage')) correspond to valid text strings in your application's i18n files (e.g., localization dictionaries). -
Duplicate Value Check: Although it isn’t explicitly shown here, if both "reasoning_content" and potentially another item have the same value ("history_message"), this may cause an issue if two nodes with different labels map to the same internal representation.
-
Optimization Suggestions:
- Ensure that the use of translations is optimal in terms of caching. If translation data changes frequently and not reused appropriately, this can lead to inefficiencies.
-
Readability and Maintainability: Given the simplicity of adding two more option values, consider whether additional features such as lazy loading or dynamic rendering based on user permissions might make sense instead of hardcoding them.
Overall, the code appears clean and functional; minor adjustments related to consistency and performance would enhance reliability and usability without altering the core logic.
| historyMessage: 'Historical chat records', | ||
| }, | ||
| searchKnowledgeNode: { | ||
| label: 'Knowledge Retrieval', |
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.
The given code snippet looks mostly clean and well-structured. However, there are a few minor improvements and suggestions:
-
Capitalization and Consistency: The
historyMessagekey is capitalized consistently with the other keys in the same object (defaultPrompt,think). This keeps consistency across the entire file. -
Line Breaks: There's an unnecessary line break before the
export default. Keeping the code concise can make it cleaner. -
Code Clarity: While there are no severe problems, ensuring that each section of the code is clear and self-contained can improve readability, especially for someone unfamiliar with the project structure.
Here's the revised version:
@@ -125,6 +125,8 @@
},
defaultPrompt: 'Known Information',
think: 'Thinking Process',
historyMessage: 'Historical Chat Records',
};
export default { ...searchKnowledgeNode }; // Keep consistent capitalization here if necessary (e.g., SearchKnowledgeNode instead)This version maintains the original logic while making some minor adjustments to improve clarity and adherence to coding standards.
feat: AI dialogue nodes support historical chat history parameters