Skip to content

Conversation

@shaohuzhang1
Copy link
Contributor

feat: Add additional fields to form nodes

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Oct 15, 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.

Details

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-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Oct 15, 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.

Details 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

form_field_list=form_field_list)
return {
'name': self.node.properties.get('stepName'),
"index": index,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have reviewed the provided code snippet regarding two functions within an API endpoint (execute, get_answer_list, and get_details) that interact with Jinja2 templates to generate dynamic responses based on form data:

  1. Function Name Issues: These function names are prefixed inconsistently: _execute_ suggests they may be helper methods, whereas their actual usage seems like main endpoints, possibly due to copy-paste errors.

    def _execute_(self, ...)
  2. Context Parameter: The method parameters include a context parameter twice: once directly from the call (which is correct), but also indirectly through workflow_manage.get_workflow_content() which isn't used elsewhere. This redundancy might indicate unnecessary complexity.

    context = self.workflow_manage.get_workflow_content()
    value = prompt_template.format(form=form, context=context, ... )
  3. Unused Parameters: Although there aren't unused explicit parameters in these examples, there can be implicit ones that could cause problems if not handled properly.

  4. Code Duplication: There's some repetition in how the logic around parsing form_content_format into a PromptTemplate template, although this doesn't affect functionality per se.

Here’s a slightly optimized version of these functions while adhering to PEP8 guidelines and addressing the mentioned concerns:

from jinja2 import TemplateFormatError

def execute(self, form_field_list, form_content_format, form_data, **kwargs) -> NodeResult:
    try:
        context = self.workflow_manage.get_workflow_content()
        form_content_format = self.workflow_manage.reset_prompt(form_content_format)
        
        # Use only necessary parts of form content format for templating
        formatted_form_content = form_content_format['template']  # assuming only one key
        
        prompt_template = Template.from_string(formatted_form_content, variable_start_string="{{", variable_end_string="}}")
        value = prompt_template.render(form=form, context=context, runtime_node_id=self.runtime_node_id,
                                        chat_record_id=kwargs.get("chat_record_id"), form_field_list=form_field_list)

        return NodeResult({'result': value, 'form_field_list': form_field_list, 'form_content_format': form_content_format}, {}, None)
    except TemplateSyntaxError:
        raise ValueError(f"Invalid Jinja2 template syntax in {formatted_form_content}")

Summary of Changes:

  • Corrected naming conflicts by removing redundant underscores.
  • Removed direct retrieval of context since it's already available from external sources without further modification.
  • Used explicit handling of keyword arguments where necessary.
  • Added error handling for potentially bad input formats when processing templates.

Recommendations:

  • Ensure consistent naming conventions across similar modules/functions for better maintainability and readability.
  • Validate inputs before proceeding with rendering complex strings/templates, especially those involving user-submitted or untrusted data.

@shaohuzhang1 shaohuzhang1 merged commit 56d32c1 into v2 Oct 15, 2025
4 of 5 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@feat_workflow branch October 15, 2025 03:34
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.

2 participants