Skip to content

Conversation

@erichare
Copy link
Collaborator

@erichare erichare commented Dec 15, 2025

This pull request improves the handling of loop input and output types for generic nodes in the frontend. The main focus is on correctly distinguishing between standard output handles and loop input handles, ensuring proper type validation and connection logic, especially for nodes that support looping outputs.

Enhancements to output and loop input handling:

  • Introduced separate memoized values for the standard output handle (id) and the loop input handle (loopInputId) in NodeOutputParameter, ensuring that loop input handles include all relevant loop types for validation.
  • Updated the NodeOutputField component and its type definition to accept and use the new loopInputId prop, so that the left handle (loop input) receives the correct identifier and type information. [1] [2] [3] [4]
  • Modified the OutputParameter component to pass both id (for the right/output handle) and loopInputId (for the left/loop input handle) to NodeOutputField, supporting more accurate handle identification and validation.

Connection validation improvements:

  • Enhanced the cleanEdges utility to ensure that, when validating connections for generic nodes, loop input handles correctly include all applicable loop types if allows_loop is true, improving edge validation logic.

Summary by CodeRabbit

  • Improvements
    • Enhanced loop node functionality with improved input validation logic for loop configurations
    • Loop node output types now accurately reflect available loop types when loop support is enabled

✏️ Tip: You can customize this high-level summary in your review settings.

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 15, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This change adds loop input handling to node output components across multiple files. A new loopInputId prop is introduced to pass loop-related type information from NodeOutputParameter through NodeOutputField to LoopHandle. Output type computation logic is updated to include loop_types when allows_loop is true.

Changes

Cohort / File(s) Summary
Loop Handle Integration
src/frontend/src/CustomNodes/GenericNode/components/NodeOutputParameter/index.tsx, src/frontend/src/CustomNodes/GenericNode/components/NodeOutputfield/index.tsx
Introduces memoized loopInputId combining output_types and loop_types; threads it through NodeOutputField to LoopHandle for validation. Adjusts useMemo dependencies accordingly.
Type Definition
src/frontend/src/types/components/index.ts
Adds optional loopInputId?: sourceHandleType property to NodeOutputFieldComponentType.
Output Type Computation
src/frontend/src/utils/reactflowUtils.ts
Updates cleanEdges output type logic to conditionally include loop_types from output object when allows_loop is true; uses selected type or first baseType as primary.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Loop type inclusion logic in reactflowUtils.ts – Verify the conditional logic correctly selects output.selected vs. baseTypes[0] and properly spreads loop_types.
  • Memoization of loopInputId in NodeOutputParameter – Ensure dependency array is complete and memoization provides expected performance benefit.
  • Prop threading accuracy – Confirm loopInputId is correctly constructed and passed through component hierarchy without data loss.

Pre-merge checks and finishing touches

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 3 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error The PR modifies critical components and utilities for loop input handling but includes no new or updated test files to verify the changes. Add unit tests for NodeOutputParameter loopInputId generation, NodeOutputField prop passing, cleanEdges loop type handling, and integration tests for end-to-end loop input connections.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage ⚠️ Warning Pull request modifies component props and utility logic for loop input handling without any corresponding test files or test case coverage. Add unit tests for NodeOutputParameter/NodeOutputfield components, integration tests for loop input/output handling, and tests for cleanEdges function to cover loop_types validation logic.
Test File Naming And Structure ⚠️ Warning Pull request modifies four frontend files for loop input type handling but adds no corresponding test files despite repository's established testing pattern. Add test files in tests directories to cover NodeOutputParameter, NodeOutputField, reactflowUtils, and loop type validation logic with positive and negative cases.
Excessive Mock Usage Warning ❓ Inconclusive No test files found for the modified components in this PR, preventing assessment of mock usage patterns. Verify test file locations or provide test files for the modified components to assess mock usage appropriateness.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'fix: Make sure loop inputs are properly handled in research' accurately describes the main objective of the changeset—improving loop input handling for generic nodes in the frontend.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 15, 2025

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 17%
16.64% (4686/28149) 10% (2179/21784) 10.93% (676/6181)

Unit Test Results

Tests Skipped Failures Errors Time
1829 0 💤 0 ❌ 0 🔥 24.177s ⏱️

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.11%. Comparing base (c1c930b) to head (ea6ea81).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/frontend/src/utils/reactflowUtils.ts 0.00% 14 Missing ⚠️
...nericNode/components/NodeOutputParameter/index.tsx 0.00% 3 Missing ⚠️
...s/GenericNode/components/NodeOutputfield/index.tsx 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (39.27%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #11029      +/-   ##
==========================================
+ Coverage   33.09%   33.11%   +0.02%     
==========================================
  Files        1389     1389              
  Lines       65699    65713      +14     
  Branches     9720     9729       +9     
==========================================
+ Hits        21743    21764      +21     
+ Misses      42842    42831      -11     
- Partials     1114     1118       +4     
Flag Coverage Δ
backend 52.43% <ø> (+0.05%) ⬆️
frontend 15.34% <0.00%> (-0.01%) ⬇️
lfx 39.27% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/GenericNode/components/NodeOutputfield/index.tsx 0.00% <0.00%> (ø)
...nericNode/components/NodeOutputParameter/index.tsx 0.00% <0.00%> (ø)
src/frontend/src/utils/reactflowUtils.ts 9.83% <0.00%> (-0.10%) ⬇️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
erichare and others added 2 commits December 15, 2025 13:02
Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 15, 2025
@github-actions github-actions bot added the lgtm This PR has been approved by a maintainer label Dec 16, 2025
@olayinkaadelakun
Copy link
Collaborator

Hey @erichare, Are there any screenshot or testcases that can make this change easily validated

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 16, 2025
@erichare erichare added this pull request to the merge queue Dec 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 16, 2025
@erichare erichare added this pull request to the merge queue Dec 16, 2025
Merged via the queue into main with commit ae70c61 Dec 16, 2025
87 of 89 checks passed
@erichare erichare deleted the fix-research-translation-template branch December 16, 2025 17:14
erichare added a commit that referenced this pull request Dec 16, 2025
* fix: Make sure loop inputs are properly handled in research

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* [autofix.ci] apply automated fixes

* Update reactflowUtils.ts

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Update reactflowUtils.ts

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Revert "Update reactflowUtils.ts"

This reverts commit 9c8b1d1.

* Revert "Update reactflowUtils.ts"

This reverts commit 6be7ab9.

* Fix template

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* [autofix.ci] apply automated fixes

* Update Research Translation Loop.json

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Update reactflowUtils.ts

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

---------

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
erichare added a commit that referenced this pull request Dec 16, 2025
…rating (#11042)

* fix: Make sure loop inputs are properly handled in research (#11029)

* fix: Make sure loop inputs are properly handled in research

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* [autofix.ci] apply automated fixes

* Update reactflowUtils.ts

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Update reactflowUtils.ts

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Revert "Update reactflowUtils.ts"

This reverts commit 9c8b1d1.

* Revert "Update reactflowUtils.ts"

This reverts commit 6be7ab9.

* Fix template

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* [autofix.ci] apply automated fixes

* Update Research Translation Loop.json

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Update reactflowUtils.ts

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

---------

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* Update Research Translation Loop.json

Co-Authored-By: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants