bring back router node title in prefix#97
Merged
srijanpatel merged 1 commit intomainfrom Jan 18, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 8cfd7c5 in 39 seconds
More details
- Looked at
62lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. backend/app/nodes/llm/single_llm_call.py:115
- Draft comment:
The logic for splittingself.config.url_variables["file"]assumes that if there are more than two parts, the first and last parts are the node_id and var_name. This might not be correct if the variable name itself contains dots. Consider revising this logic to handle such cases more robustly. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. backend/app/nodes/logic/router.py:127
- Draft comment:
The change from__fields__tomodel_fieldsis correct for compatibility with Pydantic v2. Ensure that the rest of the codebase is also updated to usemodel_fieldswhere applicable. - Reason this comment was not posted:
Confidence changes required:0%
The change inrouter.pyfrom__fields__tomodel_fieldsis correct asmodel_fieldsis the updated attribute in Pydantic v2. This change is necessary for compatibility with the newer version of Pydantic.
3. frontend/src/components/nodes/nodeSidebar/NodeSidebar.tsx:186
- Draft comment:
The removal of the special handling forRouterNodetypes means their output schema keys will now be prefixed with the node title. Ensure this change aligns with the intended behavior for displaying router nodes in the sidebar. - Reason this comment was not posted:
Confidence changes required:50%
TheNodeSidebar.tsxfile had a section removed that specifically handledRouterNodetypes differently. This change might affect how router nodes are displayed in the sidebar, as their output schema keys are no longer prefixed with the node title. This could be intentional, but it's worth noting the change in behavior.
Workflow ID: wflow_N94VWbhSn5p6s1w4
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Enhance URL variable handling in LLM nodes, update router node output model creation, and include node titles in NodeSidebar output schema keys.
single_llm_call.py: Adjust URL variable handling to support cases with more than two parts by using the first and last parts asnode_idandvar_name.router.py: Changeinput.__fields__toinput.model_fieldsfor output model creation.router.pytest case.NodeSidebar.tsx: Remove special handling forRouterNodeincollectIncomingSchema, ensuring node titles are included in output schema keys.This description was created by
for 8cfd7c5. It will automatically update as commits are pushed.