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. 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/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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| prompt = prompt.replace(globeLabel, globeValue).replace(globeLabelNew, globeValue) | ||
| return prompt | ||
|
|
||
| def generate_prompt(self, prompt: str): |
There was a problem hiding this comment.
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_fieldsmethod iterates over each node in the flow, checks for specific configurations, and collects field information (fieldsandglobalFields). If a field has no name or value, it results in an incomplete dictionary being appended to the list. -
Placeholder Logic: In the
reset_promptmethod, 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_listandglobal_field_listplace 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