-
Notifications
You must be signed in to change notification settings - Fork 215
Finished task1 Runway text-to-image node with properly integrated int… #71
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
base: main
Are you sure you want to change the base?
Conversation
…o the existing `nodes_runway.py` file and has tests coverage
Reviewer's GuideThis PR introduces a new ComfyUI node “RunwayText2ImgNode” that interfaces directly with Runway’s Gen-4 text_to_image API, handling API‐key validation, HTTP requests with polling, comprehensive error handling, and conversion of the downloaded image into a [1,3,H,W] PyTorch tensor, registers the node in the existing mappings, patches the internal logs endpoint to decode message bytes, and adds both unit and integration tests covering all key flows. Class diagram for the new RunwayText2ImgNode and related mappingsclassDiagram
class RunwayText2ImgNode {
+generate_image(prompt: str, ratio: str, unique_id: Optional[str] = None, **kwargs)
+RETURN_TYPES = ("IMAGE",)
+FUNCTION = "generate_image"
+CATEGORY = "api node/image/Runway"
+API_NODE = True
+DESCRIPTION = "Generate an image from a text prompt using Runway's API directly."
+INPUT_TYPES()
}
class ComfyNodeABC {
}
RunwayText2ImgNode --|> ComfyNodeABC
class NODE_CLASS_MAPPINGS {
+"RunwayText2ImgNode": RunwayText2ImgNode
}
class NODE_DISPLAY_NAME_MAPPINGS {
+"RunwayText2ImgNode": "Runway Text to Image (Simple)"
}
NODE_CLASS_MAPPINGS o-- RunwayText2ImgNode
NODE_DISPLAY_NAME_MAPPINGS o-- RunwayText2ImgNode
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ElenkaSan - I've reviewed your changes - here's some feedback:
Blocking issues:
- function
get_logsis defined inside a function but never used (link) - time.sleep() call; did you mean to leave this in? (link)
General comments:
- The test
test_api_request_structureasserts for a "prompt" key, but the node’s payload actually sends "promptText", so please update the test to match the real payload. - Instead of using print statements for error logging inside the node, switch to ComfyUI’s logging utility or the standard logging module to ensure logs are captured consistently.
- Consider extracting the HTTP polling and image‐download logic into a shared helper (or reusing the existing api_call flow) to reduce duplication and make the node implementation more maintainable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test `test_api_request_structure` asserts for a "prompt" key, but the node’s payload actually sends "promptText", so please update the test to match the real payload.
- Instead of using print statements for error logging inside the node, switch to ComfyUI’s logging utility or the standard logging module to ensure logs are captured consistently.
- Consider extracting the HTTP polling and image‐download logic into a shared helper (or reusing the existing api_call flow) to reduce duplication and make the node implementation more maintainable.
## Individual Comments
### Comment 1
<location> `ComfyUI/comfy_api_nodes/nodes_runway.py:717` </location>
<code_context>
+
+ try:
+ # Make the API request
+ response = requests.post(url, headers=headers, json=payload, timeout=60)
+ # print("Response status:", response.status_code)
+ # print("Response text:", response.text)
</code_context>
<issue_to_address>
A fixed 60-second timeout for the initial API request may not be sufficient in all environments.
Make the timeout value configurable or add better handling for timeout exceptions to enhance robustness.
Suggested implementation:
```python
try:
# Make the API request
response = requests.post(url, headers=headers, json=payload, timeout=timeout)
# print("Response status:", response.status_code)
# print("Response text:", response.text)
response.raise_for_status()
# Parse the response
result_id = response.json().get("id")
```
```python
try:
# Make the API request
response = requests.post(url, headers=headers, json=payload, timeout=timeout)
# print("Response status:", response.status_code)
# print("Response text:", response.text)
response.raise_for_status()
# Parse the response
result_id = response.json().get("id")
if not result_id:
raise Exception("No result ID received from Runway API")
# Poll for result
status_url = f"https://api.dev.runwayml.com/v1/tasks/{result_id}"
max_attempts = 30
except requests.exceptions.Timeout:
raise Exception(f"Request to Runway API timed out after {timeout} seconds")
```
1. You must add a `timeout` parameter to the function definition that contains this code, with a default value of 60 (or your preferred default).
2. Ensure that any calls to this function are updated if you want to specify a custom timeout.
3. Make sure `import requests` is present at the top of the file if not already imported.
</issue_to_address>
### Comment 2
<location> `ComfyUI/comfy_api_nodes/nodes_runway.py:732` </location>
<code_context>
+ max_attempts = 30
+ image_url = None
+
+ for attempt in range(max_attempts):
+ time.sleep(2)
+ status_response = requests.get(status_url, headers=headers)
+ status_response.raise_for_status()
+ status_data = status_response.json()
</code_context>
<issue_to_address>
Polling with a fixed sleep interval may unnecessarily delay result retrieval.
Consider implementing exponential backoff, a shorter initial interval, or making the polling interval configurable to reduce latency.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
max_attempts = 30
image_url = None
for attempt in range(max_attempts):
time.sleep(2)
status_response = requests.get(status_url, headers=headers)
status_response.raise_for_status()
status_data = status_response.json()
=======
max_attempts = 30
image_url = None
# Exponential backoff parameters
initial_interval = 0.5 # seconds
max_interval = 8.0 # seconds
backoff_factor = 2.0
interval = initial_interval
for attempt in range(max_attempts):
time.sleep(interval)
status_response = requests.get(status_url, headers=headers)
status_response.raise_for_status()
status_data = status_response.json()
interval = min(interval * backoff_factor, max_interval)
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `ComfyUI/comfy_api_nodes/nodes_runway.py:750` </location>
<code_context>
+ raise Exception("Timeout waiting for image generation.")
+
+ # Download and convert image
+ image_response = requests.get(image_url)
+ image_response.raise_for_status()
+ image_bytes = image_response.content
</code_context>
<issue_to_address>
No timeout is set for the image download request.
Please specify a timeout in the requests.get call to prevent indefinite hangs if the server does not respond.
</issue_to_address>
### Comment 4
<location> `ComfyUI/api_server/routes/internal/internal_routes.py:24` </location>
<code_context>
@self.routes.get('/logs')
async def get_logs(request):
- return web.json_response("".join([(l["t"] + " - " + l["m"]) for l in app.logger.get_logs()]))
+ return web.json_response("".join([(l["t"] + " - " + l["m"].decode("utf-8")) for l in app.logger.get_logs()]))
+ # return web.json_response("".join([(l["t"] + " - " + l["m"]) for l in app.logger.get_logs()]))
</code_context>
<issue_to_address>
Decoding log messages as UTF-8 may fail if the log contains non-UTF-8 bytes.
Handle potential UnicodeDecodeError or ensure all log messages are UTF-8 encoded to avoid runtime issues.
</issue_to_address>
### Comment 5
<location> `ComfyUI/tests/api_nodes/test_runway_text2img.py:65` </location>
<code_context>
+@mock.patch("requests.post")
</code_context>
<issue_to_address>
Test does not actually invoke the node or function under test.
Refactor the test to call the relevant function, using mocks if needed, to verify the request is constructed and sent as expected.
Suggested implementation:
```python
@mock.patch("requests.post")
def test_api_request_structure(mock_post):
"""Test the structure of API requests."""
# Mock successful response
mock_post.return_value.status_code = 200
mock_post.return_value.json.return_value = {
"output": "fake_base64_data"
}
# Example input for the function under test
prompt = "A cat in space"
# Call the function under test (replace with actual import if needed)
# from comfyui.api_nodes.runway import runway_text2img
result = runway_text2img(prompt=prompt)
# Assert that requests.post was called with the expected URL and payload
expected_url = "https://api.dev.runwayml.com/v1/text_to_image"
expected_payload = {"prompt": prompt}
mock_post.assert_called_once_with(expected_url, json=expected_payload)
# Optionally, check the result
assert result == "fake_base64_data"
```
- If `runway_text2img` is not already imported, you will need to import it at the top of the test file.
- Adjust the arguments to `runway_text2img` and the expected payload as appropriate for your actual function signature and API contract.
</issue_to_address>
## Security Issues
### Issue 1
<location> `ComfyUI/api_server/routes/internal/internal_routes.py:22` </location>
<issue_to_address>
**security (python.lang.maintainability.useless-inner-function):** function `get_logs` is defined inside a function but never used
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `ComfyUI/comfy_api_nodes/nodes_runway.py:733` </location>
<issue_to_address>
**security (python.lang.best-practice.arbitrary-sleep):** time.sleep() call; did you mean to leave this in?
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🧹 Nitpick comments (9)
ComfyUI/user/default/comfy.settings.json (2)
3-4: Release metadata probably doesn’t belong in a user-scope settings file
Comfy.Release.VersionandComfy.Release.Timestamplook like build-time / product metadata, whereas everything else in this JSON is user preference. Hard-coding these here means every release will rewrite this file and create unnecessary merge conflicts for downstream forks, and it risks clobbering local user overrides once the file is copied to~/.comfyui.Consider relocating the values to either
• a dedicatedversion.jsoncommitted underComfyUI/, or
• a Python constant (e.g.,comfyui.__version__,__build_ts__) generated by the build pipeline.At runtime the UI can read those constants to display the version, leaving this file purely for user settings.
4-4: Prefer ISO-8601 string over raw epoch for timestampStoring
1754104559219forces every consumer to remember the unit (ms vs s) and convert manually. A readable string such as"2025-08-31T10:35:59Z"(or at least seconds) improves clarity and debuggability with zero runtime cost.ComfyUI/user/default/workflows/Unsaved Workflow.json (1)
1-1: Consider formatting this JSON for better readability.This workflow appears to be a standard ComfyUI workflow unrelated to the Runway integration. The minified JSON format makes it difficult to review and maintain.
Consider formatting the JSON with proper indentation for better readability:
# Format the JSON file python -m json.tool "ComfyUI/user/default/workflows/Unsaved Workflow.json" > formatted_workflow.jsonComfyUI/tests/api_nodes/test_runway_integration.py (3)
9-9: Remove unused import.The
torchimport is not used in this test file.-import torch
40-51: Improve content validation robustness.String-based content checking is fragile and may break with code formatting changes. Consider using AST-based validation for better reliability.
- # Check for required methods and attributes - required_elements = [ - "RETURN_TYPES = (\"IMAGE\",)", - "FUNCTION = \"generate_image\"", - "CATEGORY = \"api node/image/Runway\"", - "def generate_image(self, prompt: str, ratio: str, unique_id: Optional[str] = None", - "RUNWAY_API_KEY" - ] - - for element in required_elements: - assert element in content, f"Required element '{element}' not found in file" - print(f"Found required element: {element}") + # Parse and validate using AST for more robust checking + tree = ast.parse(content) + class_node = None + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef) and node.name == "RunwayText2ImgNode": + class_node = node + break + + assert class_node is not None, "RunwayText2ImgNode class not found" + + # Check for class attributes and methods using AST + has_return_types = any( + isinstance(node, ast.Assign) and + any(isinstance(target, ast.Name) and target.id == "RETURN_TYPES" for target in node.targets) + for node in class_node.body + ) + assert has_return_types, "RETURN_TYPES not found in class"
120-141: Enhance mock testing coverage.The current mock test only checks basic import functionality. Consider testing the actual class instantiation and key methods.
with mock.patch.dict('sys.modules', { 'utils.json_util': mock.MagicMock(), 'server': mock.MagicMock(), 'comfy': mock.MagicMock(), 'comfy.comfy_types': mock.MagicMock(), 'comfy.comfy_types.node_typing': mock.MagicMock(), + 'requests': mock.MagicMock(), + 'os': mock.MagicMock(), + 'time': mock.MagicMock(), }): try: # Try to import the module import importlib.util spec = importlib.util.spec_from_file_location("nodes_runway", file_path) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) # Check if the class exists assert hasattr(module, 'RunwayText2ImgNode'), "RunwayText2ImgNode not found in module" - print("RunwayText2ImgNode imported successfully with mocked dependencies") + + # Test class attributes + node_class = getattr(module, 'RunwayText2ImgNode') + assert hasattr(node_class, 'INPUT_TYPES'), "INPUT_TYPES not found" + assert hasattr(node_class, 'generate_image'), "generate_image method not found" except Exception as e: - print(f"Import with mock failed: {e}") - # This is not a failure, just informational - pass + # Log the failure but don't fail the test as this is expected in some environments + import logging + logging.warning(f"Mock import test failed (expected): {e}")ComfyUI/comfy_api_nodes/nodes_runway.py (1)
648-805: Fix code style issuesMultiple code style issues detected:
- Remove whitespace from blank lines (lines 648, 654, 672, 676, 680, 683, 696, 700, 708, 714, 764, 774)
- Remove trailing whitespace (lines 712, 805)
- Add newline at end of file (line 805)
Clean up all whitespace issues and ensure the file ends with a newline character.
ComfyUI/tests/api_nodes/test_runway_text2img.py (2)
1-186: Well-structured test suite with minor style issuesThe test suite provides comprehensive coverage of the
RunwayText2ImgNodefunctionality. However, there are a few minor issues to address:
- Line 128: Remove unnecessary f-string prefix
- Multiple blank lines contain whitespace
- raise Exception(f"Runway API error (HTTP 500): Internal Server Error") + raise Exception("Runway API error (HTTP 500): Internal Server Error")Also clean up whitespace on blank lines throughout the file.
The test coverage is thorough and covers important scenarios including error handling, API interactions, and tensor conversions.
1-186: Consider adapting tests for the existingRunwayTextToImageNodeIf the recommendation to use the existing
RunwayTextToImageNodeis followed, these test cases should be adapted to ensure the existing node has equivalent test coverage, particularly for:
- API key validation from environment variables (if that pattern is adopted)
- HTTP error handling scenarios
- Polling timeout behavior
- Image tensor shape validation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dream_layer_frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
ComfyUI/api_server/routes/internal/internal_routes.py(1 hunks)ComfyUI/comfy_api_nodes/nodes_runway.py(2 hunks)ComfyUI/tests/api_nodes/test_runway_integration.py(1 hunks)ComfyUI/tests/api_nodes/test_runway_text2img.py(1 hunks)ComfyUI/user/default/comfy.settings.json(1 hunks)ComfyUI/user/default/workflows/Unsaved Workflow.json(1 hunks)ComfyUI/user/default/workflows/runway_advanced_workflow.json(1 hunks)ComfyUI/user/default/workflows/runway_text2img_workflow.json(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
ComfyUI/tests/api_nodes/test_runway_integration.py
9-9: torch imported but unused
Remove unused import: torch
(F401)
18-18: Blank line contains whitespace
Remove whitespace from blank line
(W293)
29-29: Blank line contains whitespace
Remove whitespace from blank line
(W293)
30-30: print found
Remove print
(T201)
31-31: Blank line contains whitespace
Remove whitespace from blank line
(W293)
34-34: Blank line contains whitespace
Remove whitespace from blank line
(W293)
37-37: print found
Remove print
(T201)
38-38: Blank line contains whitespace
Remove whitespace from blank line
(W293)
47-47: Blank line contains whitespace
Remove whitespace from blank line
(W293)
50-50: print found
Remove print
(T201)
51-51: Blank line contains whitespace
Remove whitespace from blank line
(W293)
52-52: print found
Remove print
(T201)
59-59: Blank line contains whitespace
Remove whitespace from blank line
(W293)
63-63: Blank line contains whitespace
Remove whitespace from blank line
(W293)
66-66: Blank line contains whitespace
Remove whitespace from blank line
(W293)
72-72: print found
Remove print
(T201)
72-72: f-string without any placeholders
Remove extraneous f prefix
(F541)
73-73: Blank line contains whitespace
Remove whitespace from blank line
(W293)
80-80: print found
Remove print
(T201)
85-85: Blank line contains whitespace
Remove whitespace from blank line
(W293)
87-87: Blank line contains whitespace
Remove whitespace from blank line
(W293)
98-98: Blank line contains whitespace
Remove whitespace from blank line
(W293)
101-101: Blank line contains whitespace
Remove whitespace from blank line
(W293)
105-105: Blank line contains whitespace
Remove whitespace from blank line
(W293)
108-108: Blank line contains whitespace
Remove whitespace from blank line
(W293)
109-109: print found
Remove print
(T201)
116-116: Blank line contains whitespace
Remove whitespace from blank line
(W293)
119-119: Blank line contains whitespace
Remove whitespace from blank line
(W293)
133-133: Blank line contains whitespace
Remove whitespace from blank line
(W293)
136-136: print found
Remove print
(T201)
137-137: Blank line contains whitespace
Remove whitespace from blank line
(W293)
139-139: print found
Remove print
(T201)
141-141: Trailing whitespace
Remove trailing whitespace
(W291)
141-141: No newline at end of file
Add trailing newline
(W292)
ComfyUI/tests/api_nodes/test_runway_text2img.py
15-15: Blank line contains whitespace
Remove whitespace from blank line
(W293)
21-21: Blank line contains whitespace
Remove whitespace from blank line
(W293)
25-25: Blank line contains whitespace
Remove whitespace from blank line
(W293)
28-28: Blank line contains whitespace
Remove whitespace from blank line
(W293)
32-32: Blank line contains whitespace
Remove whitespace from blank line
(W293)
42-42: Blank line contains whitespace
Remove whitespace from blank line
(W293)
45-45: Blank line contains whitespace
Remove whitespace from blank line
(W293)
56-56: Blank line contains whitespace
Remove whitespace from blank line
(W293)
60-60: Blank line contains whitespace
Remove whitespace from blank line
(W293)
73-73: Blank line contains whitespace
Remove whitespace from blank line
(W293)
80-80: Blank line contains whitespace
Remove whitespace from blank line
(W293)
86-86: Blank line contains whitespace
Remove whitespace from blank line
(W293)
89-89: Blank line contains whitespace
Remove whitespace from blank line
(W293)
128-128: f-string without any placeholders
Remove extraneous f prefix
(F541)
180-180: Blank line contains whitespace
Remove whitespace from blank line
(W293)
ComfyUI/comfy_api_nodes/nodes_runway.py
18-18: base64 imported but unused
Remove unused import: base64
(F401)
25-25: Redefinition of unused Optional from line 14
Remove definition: Optional
(F811)
27-27: nodes imported but unused
Remove unused import: nodes
(F401)
28-28: Redefinition of unused torch from line 22
Remove definition: torch
(F811)
29-29: Redefinition of unused time from line 20
Remove definition: time
(F811)
648-648: Blank line contains whitespace
Remove whitespace from blank line
(W293)
654-654: Blank line contains whitespace
Remove whitespace from blank line
(W293)
672-672: Blank line contains whitespace
Remove whitespace from blank line
(W293)
676-676: Blank line contains whitespace
Remove whitespace from blank line
(W293)
680-680: Blank line contains whitespace
Remove whitespace from blank line
(W293)
683-683: Blank line contains whitespace
Remove whitespace from blank line
(W293)
696-696: Blank line contains whitespace
Remove whitespace from blank line
(W293)
700-700: Blank line contains whitespace
Remove whitespace from blank line
(W293)
708-708: Blank line contains whitespace
Remove whitespace from blank line
(W293)
712-712: Trailing whitespace
Remove trailing whitespace
(W291)
714-714: Blank line contains whitespace
Remove whitespace from blank line
(W293)
721-721: Blank line contains whitespace
Remove whitespace from blank line
(W293)
764-764: Blank line contains whitespace
Remove whitespace from blank line
(W293)
774-774: Blank line contains whitespace
Remove whitespace from blank line
(W293)
776-776: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
777-777: print found
Remove print
(T201)
805-805: Trailing whitespace
Remove trailing whitespace
(W291)
805-805: No newline at end of file
Add trailing newline
(W292)
🔇 Additional comments (3)
ComfyUI/user/default/comfy.settings.json (1)
3-3: Double-check version consistency across the repoBefore merging, verify that
0.3.48matches the version exposed bysetup.py,__init__.__version__, Docker labels, etc., to avoid user confusion.ComfyUI/user/default/workflows/runway_text2img_workflow.json (1)
6-6: Aspect ratio format is supportedThe
RunwayTextToImageAspectRatioEnumdefinition includes"1024:1024"as a valid member, so using"1024:1024"in the workflow is fully supported by RunwayText2ImgNode. No changes required.ComfyUI/comfy_api_nodes/nodes_runway.py (1)
636-638: Good defensive programming practice!Adding an explicit check for
image_urlbefore using it prevents potential runtime errors in edge cases whereget_image_url_from_task_statusmight return None.
|
Hey! Can you please post the PR in discord? We can discuss further there. Also don't forget to include the UI screenshot and generated image in the PR description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (7)
ComfyUI/comfy_api_nodes/nodes_runway.py (7)
679-679: Make timeout configurable or add better timeout handling.The fixed 60-second timeout may not be sufficient in all environments. Consider making it configurable or adding better timeout exception handling.
- def generate(self, prompt: str, ratio: str, unique_id: Optional[str] = None, timeout: int = 60, **kwargs): + def generate(self, prompt: str, ratio: str, unique_id: Optional[str] = None, timeout: int = 60, **kwargs):Add timeout exception handling in the try-except block:
+ except requests.exceptions.Timeout: + raise RunwayApiError(f"Request to Runway API timed out after {timeout} seconds")
723-723: Consider exponential backoff for polling.Fixed 1-second polling intervals may unnecessarily delay result retrieval. Consider implementing exponential backoff or shorter initial intervals.
- interval = 1.0 + # Exponential backoff parameters + initial_interval = 0.5 # seconds + max_interval = 4.0 # seconds + backoff_factor = 1.5 + interval = initial_interval + for attempt in range(self.MAX_POLL_ATTEMPTS): time.sleep(interval) status_response = requests.get(status_url, headers=headers) status_response.raise_for_status() status_data = status_response.json() + interval = min(interval * backoff_factor, max_interval)
639-785: Significant code duplication - consider using existingRunwayTextToImageNode.This new node duplicates the functionality of the existing
RunwayTextToImageNode(lines 499-631). Both generate images from text prompts using Runway's API, support the same aspect ratios, handle polling, and convert images to tensors.The existing node uses established abstractions (
ApiEndpoint,SynchronousOperation,PollingOperation) which provide consistent error handling, authentication, and progress tracking.Consider either:
- Using the existing
RunwayTextToImageNodedirectly- Extending it if additional functionality is needed
- Creating a wrapper that delegates to the existing implementation
This would maintain consistency with the codebase architecture and avoid maintenance burden.
672-677: Authentication approach inconsistent with other Runway nodes.The node reads the API key directly from environment variables (line 684), while other Runway nodes use the
auth_kwargspattern with hidden inputs (auth_token,comfy_api_key).Although the hidden inputs are defined here, they're not used in the implementation. Update the
generatemethod to useauth_kwargsinstead of direct environment variable access to maintain consistency.
693-694: Use established proxy endpoints instead of direct API URLs.The node uses hardcoded API URLs instead of the proxy endpoints defined at the top of the file (
PATH_TEXT_TO_IMAGE,PATH_GET_TASK_STATUS).- api_base_url = "https://api.dev.runwayml.com/v1" - url = f"{api_base_url}/text_to_image" + url = f"{kwargs.get('api_base_url', '')}{PATH_TEXT_TO_IMAGE}"- status_url = f"{api_base_url}/tasks/{result_id}" + status_url = f"{kwargs.get('api_base_url', '')}{PATH_GET_TASK_STATUS}/{result_id}"Also applies to: 718-718
686-686: Replace generic exceptions with specific exception types.Multiple locations use generic
ValueError,RuntimeError, andTimeoutErrorinstead of the establishedRunwayApiErrorfor consistency.- raise ValueError("RUNWAY_API_KEY environment variable is missing.") + raise RunwayApiError("RUNWAY_API_KEY environment variable is missing.")- raise ValueError("Prompt cannot be empty.") + raise RunwayApiError("Prompt cannot be empty.")- raise RuntimeError("No result ID returned from Runway API.") + raise RunwayApiError("No result ID returned from Runway API.")- raise RuntimeError(f"Runway task failed with status: {status_data['status']}") + raise RunwayApiError(f"Runway task failed with status: {status_data['status']}")- raise TimeoutError("Image generation timed out.") + raise RunwayApiError("Image generation timed out.")Also applies to: 690-690, 715-715, 734-734, 737-737
740-740: Add timeout for image download request.The image download request lacks a timeout specification, which could cause indefinite hangs.
- img_resp = requests.get(image_url) + img_resp = requests.get(image_url, timeout=timeout)
🧹 Nitpick comments (1)
ComfyUI/nodes.py (1)
1584-1598: Fix whitespace issue and approve tensor validation improvementsThe tensor validation and processing improvements look excellent for supporting the new Runway node output format. The dimension handling, error messages, and pixel value scaling are all implemented correctly.
However, there's a whitespace issue on line 1595 flagged by static analysis:
else: img_np = np.clip(img_np, 0, 255).astype(np.uint8) - + if img_np.shape[2] != 3:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ComfyUI/comfy_api_nodes/nodes_runway.py(2 hunks)ComfyUI/nodes.py(2 hunks)ComfyUI/user/default/comfy.settings.json(1 hunks)ComfyUI/user/default/workflows/runway_advanced_workflow.json(1 hunks)ComfyUI/user/default/workflows/runway_text2img_workflow.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ComfyUI/user/default/comfy.settings.json
🚧 Files skipped from review as they are similar to previous changes (2)
- ComfyUI/user/default/workflows/runway_advanced_workflow.json
- ComfyUI/user/default/workflows/runway_text2img_workflow.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the lumaphotondepth2img node (comfyui/custom_nodes/luma_photon_node/luma_photon_node.py), the wei...
Learnt from: divyaprakash0426
PR: DreamLayer-AI/DreamLayer#34
File: ComfyUI/custom_nodes/luma_photon_node/luma_photon_node.py:224-224
Timestamp: 2025-07-16T05:27:07.946Z
Learning: In the LumaPhotonDepth2Img node (ComfyUI/custom_nodes/luma_photon_node/luma_photon_node.py), the weight transformation `1.0 - image_weight` is intentional and functions as designed, despite the tooltip indicating higher values should preserve the image more.
Applied to files:
ComfyUI/nodes.py
🪛 Ruff (0.12.2)
ComfyUI/nodes.py
1595-1595: Blank line contains whitespace
Remove whitespace from blank line
(W293)
ComfyUI/comfy_api_nodes/nodes_runway.py
691-691: Blank line contains whitespace
Remove whitespace from blank line
(W293)
716-716: Blank line contains whitespace
Remove whitespace from blank line
(W293)
720-720: Blank line contains whitespace
Remove whitespace from blank line
(W293)
758-758: Trailing whitespace
Remove trailing whitespace
(W291)
774-774: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
801-801: No newline at end of file
Add trailing newline
(W292)
🔇 Additional comments (4)
ComfyUI/nodes.py (1)
1802-1811: LGTM! Simplified tensor handling is more efficientThe removal of unnecessary tensor dimension permutations is a good optimization. The assumption that input tensors are already in
[B, C, H, W]format aligns well with the Runway node output and eliminates redundant operations.The aspect ratio calculation logic is preserved correctly, and the direct call to
comfy.utils.common_upscaleis appropriate.ComfyUI/comfy_api_nodes/nodes_runway.py (3)
16-26: LGTM - necessary imports for new functionality.The new imports are all used in the
RunwayText2ImgNodeimplementation and are appropriate for the direct API integration approach.
629-630: Good defensive programming improvement.Adding explicit validation for the image URL extraction prevents potential runtime errors and provides clearer error messaging.
792-792: Node mappings correctly implemented.The new node is properly registered in both mapping dictionaries with an appropriate display name that distinguishes it from the existing node.
Also applies to: 800-800
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
ComfyUI/tests/api_nodes/test_runway_integration.py (1)
1-138: Comprehensive integration test suite with existing feedback.The test structure is thorough, validating file existence, content, AST parsing, mappings, and import capabilities. However, previous review comments have already identified several code quality improvements needed.
ComfyUI/comfy_api_nodes/nodes_runway.py (1)
639-813: Acknowledge comprehensive architectural feedback.Previous review comments have identified significant architectural issues with this implementation, including code duplication with
RunwayTextToImageNode, inconsistent authentication patterns, and bypassing established proxy endpoints. These are valid concerns that should be addressed to maintain codebase consistency and reduce maintenance overhead.
🧹 Nitpick comments (1)
ComfyUI/tests/api_nodes/test_runway_text2img.py (1)
10-10: Remove unused import.The
torchimport is not used in this test file.-import torch
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ComfyUI/comfy_api_nodes/nodes_runway.py(2 hunks)ComfyUI/tests/api_nodes/test_runway_integration.py(1 hunks)ComfyUI/tests/api_nodes/test_runway_text2image.py(1 hunks)ComfyUI/tests/api_nodes/test_runway_text2img.py(1 hunks)ComfyUI/user/default/workflows/runway_advanced_workflow.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- ComfyUI/user/default/workflows/runway_advanced_workflow.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
ComfyUI/tests/api_nodes/test_runway_text2image.py (1)
ComfyUI/comfy_api_nodes/nodes_runway.py (2)
RunwayText2ImgNode(639-796)generate(679-796)
🪛 Ruff (0.12.2)
ComfyUI/tests/api_nodes/test_runway_text2image.py
4-4: base64 imported but unused
Remove unused import: base64
(F401)
6-6: numpy imported but unused
Remove unused import: numpy
(F401)
10-10: ComfyUI.utils.json_util.merge_json_recursive imported but unused
Remove unused import: ComfyUI.utils.json_util.merge_json_recursive
(F401)
17-17: Blank line contains whitespace
Remove whitespace from blank line
(W293)
34-34: Blank line contains whitespace
Remove whitespace from blank line
(W293)
54-54: Blank line contains whitespace
Remove whitespace from blank line
(W293)
60-60: Blank line contains whitespace
Remove whitespace from blank line
(W293)
63-63: Blank line contains whitespace
Remove whitespace from blank line
(W293)
72-72: Undefined name requests
(F821)
76-76: Blank line contains whitespace
Remove whitespace from blank line
(W293)
ComfyUI/comfy_api_nodes/nodes_runway.py
691-691: Blank line contains whitespace
Remove whitespace from blank line
(W293)
716-716: Blank line contains whitespace
Remove whitespace from blank line
(W293)
720-720: Blank line contains whitespace
Remove whitespace from blank line
(W293)
758-758: Trailing whitespace
Remove trailing whitespace
(W291)
774-774: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
785-785: Blank line contains whitespace
Remove whitespace from blank line
(W293)
813-813: No newline at end of file
Add trailing newline
(W292)
ComfyUI/tests/api_nodes/test_runway_integration.py
9-9: torch imported but unused
Remove unused import: torch
(F401)
18-18: Blank line contains whitespace
Remove whitespace from blank line
(W293)
29-29: Blank line contains whitespace
Remove whitespace from blank line
(W293)
32-32: Blank line contains whitespace
Remove whitespace from blank line
(W293)
35-35: print found
Remove print
(T201)
36-36: Blank line contains whitespace
Remove whitespace from blank line
(W293)
43-43: Blank line contains whitespace
Remove whitespace from blank line
(W293)
46-46: print found
Remove print
(T201)
47-47: Blank line contains whitespace
Remove whitespace from blank line
(W293)
48-48: print found
Remove print
(T201)
55-55: Blank line contains whitespace
Remove whitespace from blank line
(W293)
59-59: Blank line contains whitespace
Remove whitespace from blank line
(W293)
62-62: Blank line contains whitespace
Remove whitespace from blank line
(W293)
68-68: print found
Remove print
(T201)
68-68: f-string without any placeholders
Remove extraneous f prefix
(F541)
69-69: Blank line contains whitespace
Remove whitespace from blank line
(W293)
76-76: print found
Remove print
(T201)
81-81: Blank line contains whitespace
Remove whitespace from blank line
(W293)
83-83: Blank line contains whitespace
Remove whitespace from blank line
(W293)
94-94: Blank line contains whitespace
Remove whitespace from blank line
(W293)
97-97: Blank line contains whitespace
Remove whitespace from blank line
(W293)
101-101: Blank line contains whitespace
Remove whitespace from blank line
(W293)
104-104: Blank line contains whitespace
Remove whitespace from blank line
(W293)
105-105: print found
Remove print
(T201)
112-112: Blank line contains whitespace
Remove whitespace from blank line
(W293)
115-115: Blank line contains whitespace
Remove whitespace from blank line
(W293)
129-129: Blank line contains whitespace
Remove whitespace from blank line
(W293)
132-132: print found
Remove print
(T201)
133-133: Blank line contains whitespace
Remove whitespace from blank line
(W293)
135-135: print found
Remove print
(T201)
137-137: Trailing whitespace
Remove trailing whitespace
(W291)
ComfyUI/tests/api_nodes/test_runway_text2img.py
10-10: torch imported but unused
Remove unused import: torch
(F401)
45-45: Blank line contains whitespace
Remove whitespace from blank line
(W293)
54-54: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (5)
ComfyUI/tests/api_nodes/test_runway_text2image.py (4)
12-45: Well-structured integration test.The test properly mocks the complete API flow including task creation, polling, and image download. Good validation of the tensor output format with correct assertions on shape and type.
46-56: Good error handling test.Properly validates that the node raises a
ValueErrorwhen the requiredRUNWAY_API_KEYenvironment variable is missing.
57-65: Good input validation test.Properly tests that the node validates prompt input and raises appropriate errors for empty or whitespace-only prompts.
66-78: HTTP error handling test looks good.The test structure properly validates 401 Unauthorized responses, but this depends on the missing
requestsimport being fixed.ComfyUI/tests/api_nodes/test_runway_text2img.py (1)
14-102: Well-organized test structure.Good use of helper functions to abstract path management and file operations. The test functions are focused and clear, properly validating the node's presence, structure, and functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (4)
ComfyUI/comfy_api_nodes/nodes_runway.py (4)
639-784: Critical: This implementation duplicates existing functionality and has multiple architectural issuesThis new node significantly duplicates the existing
RunwayTextToImageNodefunctionality, which violates DRY principles and creates maintenance overhead.Key issues that need to be addressed:
- Code duplication: The existing
RunwayTextToImageNodealready provides this functionality with better architecture- Authentication inconsistency: Direct environment variable access instead of using the established
auth_kwargspattern- Proxy bypass: Hardcoded API URLs instead of using the configured proxy endpoints
- Error handling: Multiple exception handling blocks and inconsistent error types
Recommended approach: Either extend the existing
RunwayTextToImageNodeor create a wrapper that delegates to it, maintaining consistency with the established codebase patterns.
684-686: Use consistent authentication patternThe node reads the API key directly from environment variables, which is inconsistent with other Runway nodes that use the
auth_kwargspattern.Follow the established pattern by using the
auth_kwargsparameter that's already available in the method signature.
693-694: Use proxy endpoints instead of hardcoded URLsThe hardcoded API URLs bypass the proxy configuration defined at the top of the file.
Replace with the established proxy endpoints:
- url = f"{api_base_url}/text_to_image" + url = f"{kwargs.get('api_base_url', '')}{PATH_TEXT_TO_IMAGE}"- status_url = f"{api_base_url}/tasks/{result_id}" + status_url = f"{kwargs.get('api_base_url', '')}{PATH_GET_TASK_STATUS}/{result_id}"Also applies to: 718-718
774-784: Fix error handling inconsistenciesThe error handling has several issues that were flagged in previous reviews but remain unaddressed.
Apply this fix:
- except requests.exceptions.HTTPError: + except requests.exceptions.HTTPError: if response.status_code == 401: - raise RunwayApiError("Invalid Runway API key. Please check your RUNWAY_API_KEY.") + raise RunwayApiError("Invalid Runway API key. Please check your RUNWAY_API_KEY.") elif response.status_code == 400: - raise RunwayApiError(f"Bad request: {response.text}") + raise RunwayApiError(f"Bad request: {response.text}") else: - raise RunwayApiError(f"Runway API error (HTTP {response.status_code}): {response.text}") + raise RunwayApiError(f"Runway API error (HTTP {response.status_code}): {response.text}") except requests.exceptions.RequestException as e: - raise RunwayApiError(f"Failed to connect to Runway API: {str(e)}") + raise RunwayApiError(f"Failed to connect to Runway API: {str(e)}") except Exception as e: - raise RunwayApiError(f"Unhandled error: {str(e)}") + raise RunwayApiError(f"Unhandled error: {str(e)}")
🧹 Nitpick comments (1)
ComfyUI/comfy_api_nodes/nodes_runway.py (1)
792-792: Node mappings correctly addedThe new node is properly registered in both class and display name mappings.
Fix the missing newline at end of file:
"RunwayText2ImgNode": "Runway Text to Image (Simple)", -} +} +Also applies to: 800-801
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ComfyUI/comfy_api_nodes/nodes_runway.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
ComfyUI/comfy_api_nodes/nodes_runway.py
691-691: Blank line contains whitespace
Remove whitespace from blank line
(W293)
716-716: Blank line contains whitespace
Remove whitespace from blank line
(W293)
720-720: Blank line contains whitespace
Remove whitespace from blank line
(W293)
758-758: Trailing whitespace
Remove trailing whitespace
(W291)
773-773: Blank line contains whitespace
Remove whitespace from blank line
(W293)
801-801: No newline at end of file
Add trailing newline
(W292)
🔇 Additional comments (2)
ComfyUI/comfy_api_nodes/nodes_runway.py (2)
16-26: LGTM - Necessary imports for new functionalityThe added imports are required for the direct API implementation in the new
RunwayText2ImgNodeclass.
629-630: Good defensive programming practiceAdding validation for the image URL presence improves error handling for edge cases where the API response format might be unexpected.
ElenkaSan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes provided, please PR ready for review
For Task 1 of the DreamLayer open-source challenge, I implemented a new ComfyUI node called
RunwayText2ImgNode, which is now properly integrated into the existingnodes_runway.pyfile and has tests/test_runway_text2img.pyand/test_runway_integration.pycoverage.Description
This PR adds a new ComfyUI node
RunwayText2ImgNodethat connects directly to Runway’s Gen-4/v1/text_to_imageAPI. This custom node allows users to input a text prompt and generate an image directly within ComfyUI — no extra tools or setup required.The node includes full error handling, polling support, and outputs a valid PyTorch tensor compatible with the ComfyUI image pipeline.
The new node is fully integrated, clean, and extensively tested. It’s now production-ready and easy to extend in the future (e.g., adding referenceImage, style, seed, etc.).
Changes Made
RunwayText2ImgNodetoComfyUI/comfy_api_nodes/nodes_runway.py.env– Includes RUNWAY_API_KEY for local testingComfyUI/tests/api_nodes/test_runway_text2img.pyand integration testsComfyUI/tests/api_nodes/test_runway_integration.pyThese changes improve stability when processing multiple or large images.
Evidence Required ✅
UI Screenshot
Simple workflow
Generated Image
Link to generated image
Advance workflow
Link to generated image
Logs
Debugging, then removed debug print statement
Simple workflow
Advance workflow
Tests
I've fully resolved the original pytest import error and significantly improved test coverage across both core logic and integration.
Edge-Case Testing: Manually tested failure scenarios like:
Checklist
Summary by Sourcery
Introduce a new RunwayText2ImgNode for ComfyUI to generate images from text prompts via Runway’s Gen-4 API, integrate it into node mappings, implement full error handling, polling, and tensor conversion, and add comprehensive unit and integration tests.
New Features:
Enhancements:
Tests:
Chores:
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests
Chores