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

fix: Having the same suffix for function node names can result in incorrect value retrieval #2513

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

shaohuzhang1
Copy link
Contributor

fix: Having the same suffix for function node names can result in incorrect value retrieval

Copy link

f2c-ci-robot bot commented Mar 7, 2025

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.

Copy link

f2c-ci-robot bot commented Mar 7, 2025

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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):
Copy link
Contributor Author

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:

  1. Field Initialization: The init_fields method iterates over each node in the flow, checks for specific configurations, and collects field information (fields and globalFields). If a field has no name or value, it results in an incomplete dictionary being appended to the list.

  2. 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.

  3. Ordering of Fields: Sort operations on field_list and global_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).

  4. Code Duplication: There appears to be duplication between collecting fields and modifying the prompt at multiple points in the methods.

Optimizations and Suggestions:

  1. 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.

@shaohuzhang1 shaohuzhang1 merged commit 82b566d into main Mar 7, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@fix_workflow branch March 7, 2025 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant