-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: Having the same suffix for function node names can result in incorrect value retrieval #2513
Conversation
…orrect value retrieval
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. Instructions 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/test-infra 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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
globeLabel = f"全局变量.{field.get('value')}" | ||
globeLabelNew = f"global.{field.get('value')}" | ||
globeValue = f"context.get('global').get('{field.get('value', '')}','')" | ||
prompt = prompt.replace(globeLabel, globeValue).replace(globeLabelNew, globeValue) | ||
return prompt | ||
|
||
def generate_prompt(self, prompt: str): |
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 does not contain apparent syntax errors. However, here are some concerns and optimizations that could be made:
Concerns:
-
Field Initialization: The
init_fields
method iterates over each node in the flow, checks for specific configurations, and collects field information (fields
andglobalFields
). If a field has no name or value, it results in an incomplete dictionary being appended to the list. -
Placeholder Logic: In the
reset_prompt
method, there is a loop that attempts to replace variable labels with their values from different contexts (context
,global
, etc.). Since these values are obtained using context retrieval logic, there might be additional complexity added unnecessarily if the context itself is complex or changes frequently. -
Ordering of Fields: Sort operations on
field_list
andglobal_field_list
place nodes alphabetically by stepName, which may lead to inconsistent ordering across runs unless further filtering is applied based on specific needs (e.g., importance levels). -
Code Duplication: There appears to be duplication between collecting fields and modifying the prompt at multiple points in the methods.
Optimizations and Suggestions:
-
Sanitize Field Values: Before adding fields to the lists, ensure they have valid names and values. This can prevent partial dictionaries causing issues later on.
def sanitize_field(field): return {**field, 'node_id': None} if ('node_name' not in field) or not field['node_name'] else field def init_fields(self): self.field_list = [] self.global_field_list = [] for node in self.flow.nodes: properties = node.properties node_name = properties.get('stepName') node_id = node.id node_config = properties.get('config') if node_config is not None: fields = node_config.get('fields', []) global_fields = node_config.get('globalFields', []) # Sanitized before appending sanitized_fields = [sanitize_field(field) for field in fields] sanitized_global_fields = [sanitize_field(global_field) for global_field in global_fields] self.field_list.extend(sanitized_fields) self.global_field_list.extend(sanitized_global_fields) self.field_list.sort(key=lambda f: len(f.get('node_name'), reverse=True)) self.global_field_list.sort(key=lambda f: len(f.get('node_name'), reverse=True))
2. **Combine Placeholder Replacement Logic**: Instead of replacing label and new label separately, consider combining them into one line in the replacement logic. This reduces duplicate logic.
```python
def reset_prompt(self, prompt: str):
placeholder = "{}"
for field in self.field_list + self.global_field_list:
globeLabel = f"{field.get('node_name')}.{field.get('value')}"
globeValue = f"context.get('{field.get('node_id')}',{placeholder}).ge'{field.get('value', '')}','')"
prompt = prompt.replace(globeLabel, globeValue)
return prompt
These optimizations should help improve the robustness and efficiency of the code while maintaining consistency.
fix: Having the same suffix for function node names can result in incorrect value retrieval