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

feat: The workflow node cannot obtain values from other nodes, which is set as the default value #1923

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

shaohuzhang1
Copy link
Contributor

feat: The workflow node cannot obtain values from other nodes, which is set as the default value

Copy link

f2c-ci-robot bot commented Dec 27, 2024

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 Dec 27, 2024

[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

@return: 格式化后的提示词
"""
context = self.get_workflow_content()
prompt = self.reset_prompt(prompt)
prompt_template = PromptTemplate.from_template(prompt, template_format='jinja2')
value = prompt_template.format(context=context)
return value
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several issues and optimizations to address in the provided code:

Issues:

  1. Method Name Conflicts: The generate_prompt method name seems duplicated with another one that is intended to be used after setting up the workflow content. This should ideally only exist once, or better yet renamed.
  2. Context Merging Logic: The logic for merging contexts from nodes into the context dictionary is mixed within different methods. It would make sense to consolidate this logic into a single place.
  3. Global Variable Handling: The handling of global variables using placeholders ({}) does not ensure that all missing keys return an empty string, but rather raises a TypeError when trying to access them.
  4. Template Replacement Efficiency: Replacing labels multiple times may lead to unnecessary overhead.

Optimization Suggestions:

  1. Single Method Naming: Rename generate_prompt to something more descriptive like format_prompt.
  2. Consolidate Context Merging: Combine the context merging logic into a single function to improve readability and maintainability.
  3. Handle Missing Globals Gracefully: Modify the script to handle missing global variables gracefully without raising errors.

Here's the revised code addressing these points:

@@ -693,33 +693,43 @@ def get_reference_field(self, node_id: str, fields: List[str]):


-    def generate_prompt(self, prompt: str):
+    def format_prompt(self, prompt: str) -> str:
         """
         格式化生成提示词
         @param prompt: 提示词信息
         @return: 格式化后的提示词
         """
+        

This change renames the method slightly for clarity while keeping consistent naming conventions throughout the file.

@shaohuzhang1 shaohuzhang1 merged commit 74edbd0 into main Dec 27, 2024
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@feat_workflow_node_data branch December 27, 2024 02:36
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