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

Add parameterKeys to textPrompt #1191

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Nov 14, 2024

Important

Add parameterKeys support to textPrompt nodes and refactor TaskNodeParametersPanel to ParametersMultiSelect.

  • Behavior:
    • Add parameterKeys to textPrompt nodes in FlowRenderer.tsx, TextPromptNode.tsx, and types.ts.
    • Update WorkflowParametersPanel.tsx to handle parameterKeys for textPrompt nodes when parameters are deleted.
    • Modify getWorkflowBlock() and convertToNode() in workflowEditorUtils.ts to include parameterKeys for textPrompt nodes.
  • Refactor:
    • Rename TaskNodeParametersPanel to ParametersMultiSelect and update imports in TaskNode.tsx and TextPromptNode.tsx.

This description was created by Ellipsis for 820f7ad. It will automatically update as commits are pushed.

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Add `parameterKeys` support to `textPrompt` nodes and refactor `TaskNodeParametersPanel` to `ParametersMultiSelect`.
>
>   - **Behavior**:
>     - Add `parameterKeys` to `textPrompt` nodes in `FlowRenderer.tsx`, `TextPromptNode.tsx`, and `types.ts`.
>     - Update `WorkflowParametersPanel.tsx` to handle `parameterKeys` for `textPrompt` nodes when parameters are deleted.
>     - Modify `getWorkflowBlock()` and `convertToNode()` in `workflowEditorUtils.ts` to include `parameterKeys` for `textPrompt` nodes.
>   - **Refactor**:
>     - Rename `TaskNodeParametersPanel` to `ParametersMultiSelect` and update imports in `TaskNode.tsx` and `TextPromptNode.tsx`.
>
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 78533597da77e61a956415a701ed9709dcfa8507. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 820f7ad in 36 seconds

More details
  • Looked at 205 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/FlowRenderer.tsx:421
  • Draft comment:
    The TODO comment here is redundant and can be removed as the code already handles textPrompt nodes separately.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code in FlowRenderer.tsx has a redundant comment that can be removed. The comment is a TODO note that is no longer necessary because the code is already handling the textPrompt node type separately.
2. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:204
  • Draft comment:
    The mapping for parameterKeys is redundant here as it is already handled in commonData. Consider removing it from this case.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The convertToNode function in workflowEditorUtils.ts has a redundant mapping for parameterKeys in the text_prompt case. This mapping is already handled in the commonData object, so it can be removed from the specific case.
3. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:631
  • Draft comment:
    The mapping for parameter_keys is redundant here as it is already handled in base. Consider removing it from this case.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The getWorkflowBlock function in workflowEditorUtils.ts has a redundant mapping for parameter_keys in the text_prompt case. This mapping is already handled in the base object, so it can be removed from the specific case.

Workflow ID: wflow_mmLzMb3lRPFrQA1n


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 820f7ad in 51 seconds

More details
  • Looked at 206 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/FlowRenderer.tsx:421
  • Draft comment:
    Consider resolving the TypeScript issue mentioned in the comment by refining the type definitions or using type guards to ensure TypeScript correctly infers the types for both task and textPrompt nodes.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new feature to handle parameterKeys for textPrompt nodes. However, the code in FlowRenderer.tsx has a comment indicating a TypeScript issue when combining conditions for task and textPrompt nodes. This suggests a potential type inference problem that should be addressed to ensure code maintainability and correctness.
2. skyvern-frontend/src/routes/workflows/editor/nodes/TaskNode/TaskNode.tsx:30
  • Draft comment:
    Refactored TaskNodeParametersPanel to ParametersMultiSelect and updated imports. Ensure all references are updated to avoid runtime errors.
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The PR refactors TaskNodeParametersPanel to ParametersMultiSelect and updates the imports accordingly. This is a straightforward refactor, and the changes are consistent across the files.
3. skyvern-frontend/src/routes/workflows/editor/nodes/TextPromptNode/TextPromptNode.tsx:100
  • Draft comment:
    Implemented ParametersMultiSelect for textPrompt nodes to handle parameterKeys. Ensure this aligns with the intended functionality and test thoroughly.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The PR adds parameterKeys to textPrompt nodes, which is a new feature. The implementation in TextPromptNode.tsx correctly uses the ParametersMultiSelect component to handle parameter selection. This is consistent with the intended functionality.
4. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:204
  • Draft comment:
    Updated convertToNode to include parameterKeys for textPrompt nodes. Ensure this is consistent with the data model and test for correct YAML conversion.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The PR updates the convertToNode and getWorkflowBlock functions to handle parameterKeys for textPrompt nodes. This ensures that the conversion between node data and YAML includes the new parameterKeys field.
5. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:631
  • Draft comment:
    Updated getWorkflowBlock to include parameterKeys for textPrompt nodes. Verify that this change is correctly reflected in the YAML output.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The PR updates the getWorkflowBlock function to include parameterKeys for textPrompt nodes. This ensures that the YAML representation of the workflow includes the new parameter keys.

Workflow ID: wflow_8E7zo5xpP57bMfyb


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@msalihaltun msalihaltun merged commit 9dd4263 into main Nov 14, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/add-parameters-to-text-prompt branch November 14, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants