Task #5: One-Click Report Bundle Generator with Frontend Integration #84
Conversation
Reviewer's GuideIntroduces a one-click report bundle feature by adding a Python script that packages inference artifacts into report.zip, exposing API endpoints in both the Flask backend and ComfyUI’s internal server to trigger generation, and integrating a React button in the UI; includes unit tests verifying the ZIP contents. Sequence diagram for frontend-triggered report bundle generationsequenceDiagram
actor User
participant Frontend as React UI
participant Backend as Flask API
participant Script as generate_report.py
User->>Frontend: Clicks Generate Report button
Frontend->>Backend: POST /api/generate-report
Backend->>Script: Run generate_report.py (subprocess)
Script-->>Backend: Report.zip created (stdout)
Backend-->>Frontend: JSON response (success/failure)
Frontend-->>User: Show alert (success/failure)
Class diagram for new GenerateReportButton React componentclassDiagram
class GenerateReportButton {
+handleGenerateReport()
+render()
}
Class diagram for generate_report.py script structureclassDiagram
class generate_report {
+create_report()
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThis update introduces a complete report generation feature spanning backend, frontend, and utility layers. New endpoints allow report creation via API calls, a React button triggers the process, and a utility script assembles a zipped bundle with results and configuration. Supporting tests, workflow, and configuration files were also added or updated. Changes
Sequence Diagram(s)Report Generation Flow (Frontend to Backend to Utility Script)sequenceDiagram
participant User
participant Frontend (React)
participant Backend (Flask API)
participant Utility Script (generate_report.py)
User->>Frontend (React): Click "Generate Report" button
Frontend->>Backend (Flask API): POST /api/generate-report
Backend->>Utility Script (generate_report.py): Run script via subprocess
Utility Script-->>Backend: Return stdout or error
Backend-->>Frontend: JSON response (success/error)
Frontend-->>User: Alert with result message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
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 (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @mdeshp11 - I've reviewed your changes - here's some feedback:
Blocking issues:
- function
generate_reportis defined inside a function but never used (link) - Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
General comments:
- The change from
if os.environ.get(...)toif True:in dream_layer.py forces CPU mode unconditionally—restore the environment-based check or make CPU mode configurable. - The
generate_reportendpoint implementation is duplicated in both the Flask backend and ComfyUI internal routes—consider centralizing this logic into a shared function or service. - In GenerateReportButton.tsx the API URL is hard-coded to http://localhost:5002—externalize the base URL into configuration or use a relative path for better environment flexibility.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change from `if os.environ.get(...)` to `if True:` in dream_layer.py forces CPU mode unconditionally—restore the environment-based check or make CPU mode configurable.
- The `generate_report` endpoint implementation is duplicated in both the Flask backend and ComfyUI internal routes—consider centralizing this logic into a shared function or service.
- In GenerateReportButton.tsx the API URL is hard-coded to http://localhost:5002—externalize the base URL into configuration or use a relative path for better environment flexibility.
## Individual Comments
### Comment 1
<location> `dream_layer_backend/dream_layer.py:91` </location>
<code_context>
# Check for environment variable to force ComfyUI CPU mode
-if os.environ.get('DREAMLAYER_COMFYUI_CPU_MODE', 'false').lower() == 'true':
+if True:
print("Forcing ComfyUI to run in CPU mode as requested.")
sys.argv.append('--cpu')
</code_context>
<issue_to_address>
Hardcoded CPU mode activation may not be intended for production.
Replacing the environment variable check with 'if True:' forces CPU mode in all cases, overriding user or deployment settings. If this is intentional for development, please clarify with a comment or revert to the original check for production use.
</issue_to_address>
### Comment 2
<location> `dream_layer_backend/dream_layer.py:329` </location>
<code_context>
+@app.route('/api/generate-report', methods=['POST'])
+def handle_generate_report():
+ try:
+ script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py')
+ result = subprocess.run(["python", script_path], capture_output=True, text=True, check=True)
+ return jsonify({
</code_context>
<issue_to_address>
Backslash in path may cause issues on non-Windows systems.
Use os.path.join or pathlib.Path to ensure the path works across all operating systems.
</issue_to_address>
### Comment 3
<location> `dream_layer_backend/dream_layer.py:330` </location>
<code_context>
+def handle_generate_report():
+ try:
+ script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py')
+ result = subprocess.run(["python", script_path], capture_output=True, text=True, check=True)
+ return jsonify({
+ "status": "success",
</code_context>
<issue_to_address>
Invoking 'python' may not use the correct interpreter.
Use sys.executable instead of 'python' to ensure the subprocess runs with the same interpreter as the parent process.
Suggested implementation:
```python
result = subprocess.run([sys.executable, script_path], capture_output=True, text=True, check=True)
```
```python
print("Forcing ComfyUI to run in CPU mode as requested.")
sys.argv.append('--cpu')
import sys
```
</issue_to_address>
### Comment 4
<location> `ComfyUI/api_server/routes/internal/internal_routes.py:73` </location>
<code_context>
+ async def generate_report(request):
+ try:
+ script_path = Path(__file__).parent.parent.parent / "utils" / "generate_report.py"
+ result = subprocess.run(["python", str(script_path)], capture_output=True, text=True, check=True)
+ return web.json_response({
+ "status": "success",
</code_context>
<issue_to_address>
Subprocess call may not use the correct Python interpreter.
Consider replacing 'python' with sys.executable to ensure the subprocess uses the current Python interpreter.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
result = subprocess.run(["python", str(script_path)], capture_output=True, text=True, check=True)
=======
import sys
result = subprocess.run([sys.executable, str(script_path)], capture_output=True, text=True, check=True)
>>>>>>> REPLACE
</suggested_fix>
### Comment 5
<location> `ComfyUI/utils/generate_report.py:41` </location>
<code_context>
+ zipf.write(image_file, arcname=f"grids/{image_file.name}")
+
+ # Add readme
+ readme_path = REPORT_DIR / "README.txt"
+ with open(readme_path, 'w') as f:
+ f.write(README_CONTENT)
+ zipf.write(readme_path, arcname="README.txt")
+
+ print(f"Report generated at: {REPORT_ZIP.resolve()}")
</code_context>
<issue_to_address>
Temporary README.txt file is left in the report directory.
Consider writing the README directly to the zip with ZipFile.writestr, or deleting the temporary file after archiving to avoid clutter in REPORT_DIR.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
# Add readme
readme_path = REPORT_DIR / "README.txt"
with open(readme_path, 'w') as f:
f.write(README_CONTENT)
zipf.write(readme_path, arcname="README.txt")
=======
# Add readme directly to the zip
zipf.writestr("README.txt", README_CONTENT)
>>>>>>> REPLACE
</suggested_fix>
### Comment 6
<location> `ComfyUI/tests-unit/report_bundle_test/test_generate_report.py:20` </location>
<code_context>
+ assert "README.txt" in names, "Missing README.txt in ZIP"
+
+ # Check image references in CSV exist in ZIP
+ with zipf.open("results.csv") as csvfile:
+ reader = csv.DictReader(line.decode('utf-8') for line in csvfile)
+ for row in reader:
+ image_name = f"grids/{row['image_file']}"
</code_context>
<issue_to_address>
Potential issue with reading CSV from ZIP file.
Using `io.TextIOWrapper` to wrap the file object ensures proper handling of newlines and encodings, making CSV reading more reliable.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
with zipf.open("results.csv") as csvfile:
reader = csv.DictReader(line.decode('utf-8') for line in csvfile)
for row in reader:
image_name = f"grids/{row['image_file']}"
assert image_name in names, f"Missing image file {image_name} in ZIP"
=======
with zipf.open("results.csv") as csvfile:
with io.TextIOWrapper(csvfile, encoding='utf-8', newline='') as textfile:
reader = csv.DictReader(textfile)
for row in reader:
image_name = f"grids/{row['image_file']}"
assert image_name in names, f"Missing image file {image_name} in ZIP"
>>>>>>> REPLACE
</suggested_fix>
## Security Issues
### Issue 1
<location> `ComfyUI/api_server/routes/internal/internal_routes.py:69` </location>
<issue_to_address>
**security (python.lang.maintainability.useless-inner-function):** function `generate_report` is defined inside a function but never used
*Source: opengrep*
</issue_to_address>
### Issue 2
<location> `ComfyUI/api_server/routes/internal/internal_routes.py:73` </location>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Issue 3
<location> `dream_layer_backend/dream_layer.py:330` </location>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*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.
|
|
||
| # Check for environment variable to force ComfyUI CPU mode | ||
| if os.environ.get('DREAMLAYER_COMFYUI_CPU_MODE', 'false').lower() == 'true': | ||
| if True: |
There was a problem hiding this comment.
issue: Hardcoded CPU mode activation may not be intended for production.
Replacing the environment variable check with 'if True:' forces CPU mode in all cases, overriding user or deployment settings. If this is intentional for development, please clarify with a comment or revert to the original check for production use.
| @app.route('/api/generate-report', methods=['POST']) | ||
| def handle_generate_report(): | ||
| try: | ||
| script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py') |
There was a problem hiding this comment.
issue: Backslash in path may cause issues on non-Windows systems.
Use os.path.join or pathlib.Path to ensure the path works across all operating systems.
| def handle_generate_report(): | ||
| try: | ||
| script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py') | ||
| result = subprocess.run(["python", script_path], capture_output=True, text=True, check=True) |
There was a problem hiding this comment.
suggestion (bug_risk): Invoking 'python' may not use the correct interpreter.
Use sys.executable instead of 'python' to ensure the subprocess runs with the same interpreter as the parent process.
Suggested implementation:
result = subprocess.run([sys.executable, script_path], capture_output=True, text=True, check=True) print("Forcing ComfyUI to run in CPU mode as requested.")
sys.argv.append('--cpu')
import sys| async def generate_report(request): | ||
| try: | ||
| script_path = Path(__file__).parent.parent.parent / "utils" / "generate_report.py" | ||
| result = subprocess.run(["python", str(script_path)], capture_output=True, text=True, check=True) |
There was a problem hiding this comment.
suggestion (bug_risk): Subprocess call may not use the correct Python interpreter.
Consider replacing 'python' with sys.executable to ensure the subprocess uses the current Python interpreter.
| result = subprocess.run(["python", str(script_path)], capture_output=True, text=True, check=True) | |
| import sys | |
| result = subprocess.run([sys.executable, str(script_path)], capture_output=True, text=True, check=True) |
| # Add readme | ||
| readme_path = REPORT_DIR / "README.txt" | ||
| with open(readme_path, 'w') as f: | ||
| f.write(README_CONTENT) | ||
| zipf.write(readme_path, arcname="README.txt") |
There was a problem hiding this comment.
suggestion: Temporary README.txt file is left in the report directory.
Consider writing the README directly to the zip with ZipFile.writestr, or deleting the temporary file after archiving to avoid clutter in REPORT_DIR.
| # Add readme | |
| readme_path = REPORT_DIR / "README.txt" | |
| with open(readme_path, 'w') as f: | |
| f.write(README_CONTENT) | |
| zipf.write(readme_path, arcname="README.txt") | |
| # Add readme directly to the zip | |
| zipf.writestr("README.txt", README_CONTENT) |
| @self.routes.post('/generate_report') | ||
| async def generate_report(request): | ||
| try: | ||
| script_path = Path(__file__).parent.parent.parent / "utils" / "generate_report.py" | ||
| result = subprocess.run(["python", str(script_path)], capture_output=True, text=True, check=True) | ||
| return web.json_response({ | ||
| "status": "success", | ||
| "message": "Report generated successfully.", | ||
| "output": result.stdout | ||
| }) | ||
| except subprocess.CalledProcessError as e: | ||
| return web.json_response({ | ||
| "status": "error", | ||
| "message": "Report generation failed.", | ||
| "error": e.stderr | ||
| }, status=500) |
There was a problem hiding this comment.
security (python.lang.maintainability.useless-inner-function): function generate_report is defined inside a function but never used
Source: opengrep
| async def generate_report(request): | ||
| try: | ||
| script_path = Path(__file__).parent.parent.parent / "utils" / "generate_report.py" | ||
| result = subprocess.run(["python", str(script_path)], capture_output=True, text=True, check=True) |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| def handle_generate_report(): | ||
| try: | ||
| script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py') | ||
| result = subprocess.run(["python", script_path], capture_output=True, text=True, check=True) |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| for row in reader: | ||
| image_name = f"grids/{row['image_file']}" | ||
| assert image_name in names, f"Missing image file {image_name} in ZIP" |
There was a problem hiding this comment.
issue (code-quality): Avoid loops in tests. (no-loop-in-tests)
Explanation
Avoid complex code, like loops, in test functions.Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
- loops
- conditionals
Some ways to fix this:
- Use parametrized tests to get rid of the loop.
- Move the complex logic into helpers.
- Move the complex part into pytest fixtures.
Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / Don't Put Logic in Tests
| # Check for environment variable to force ComfyUI CPU mode | ||
| if os.environ.get('DREAMLAYER_COMFYUI_CPU_MODE', 'false').lower() == 'true': | ||
| if True: | ||
| print("Forcing ComfyUI to run in CPU mode as requested.") | ||
| sys.argv.append('--cpu') |
There was a problem hiding this comment.
suggestion (code-quality): Remove redundant conditional (remove-redundant-if)
| # Check for environment variable to force ComfyUI CPU mode | |
| if os.environ.get('DREAMLAYER_COMFYUI_CPU_MODE', 'false').lower() == 'true': | |
| if True: | |
| print("Forcing ComfyUI to run in CPU mode as requested.") | |
| sys.argv.append('--cpu') | |
| print("Forcing ComfyUI to run in CPU mode as requested.") | |
| sys.argv.append('--cpu') |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
ComfyUI/utils/report/README.txt (1)
1-7: Remove duplicate README files inutils/reportBoth
ComfyUI/utils/report/README.txtandComfyUI/utils/report/report/README.txtcontain identical content and are not referenced anywhere in the codebase. Maintaining two copies can lead to drift and confusion.• Locations to address:
- ComfyUI/utils/report/README.txt
- ComfyUI/utils/report/report/README.txt
Action: Remove one of these files (or consolidate them) unless there’s a specific reason to keep both, and update any documentation or packaging scripts accordingly.
♻️ Duplicate comments (1)
ComfyUI/utils/report/report/config.json (1)
1-385: Same observations as the sample_data copyAll remarks left on
utils/sample_data/config.json(duplicate UUID, hard-coded checkpoint, duplicated file) apply here verbatim. Please address them once at the source of truth to avoid split-brain edits.
🧹 Nitpick comments (9)
ComfyUI/utils/report/report/README.txt (1)
1-7: LGTM! Consider adding more detailed descriptions.The README clearly describes the report bundle contents and aligns well with the PR objectives for generating comprehensive report bundles.
Consider adding more detailed descriptions to help users understand the bundle contents better:
# DreamLayer Inference Report This bundle contains: -- Inference results (results.csv) -- Configuration used (config.json) -- Grid images with seeds and parameters +- Inference results (results.csv) - Generated image metadata including paths, parameters, and settings +- Configuration used (config.json) - ComfyUI workflow configuration that produced these results +- Grid images with seeds and parameters - Visual outputs from the generation processdream_layer_frontend/src/pages/Index.tsx (1)
59-62: LGTM! Well-integrated button placement.The GenerateReportButton is appropriately placed in the UI hierarchy between navigation and content, with proper styling using the
my-4margin class. The placement makes sense for a global report generation feature.Consider adding semantic meaning to the wrapper div:
- {/* Generate Report Button */} - <div className="my-4"> + {/* Generate Report Button */} + <div className="my-4" role="toolbar" aria-label="Report generation actions"> <GenerateReportButton /> </div>ComfyUI/tests-unit/report_bundle_test/test_generate_report.py (1)
26-29: Add missing newline at end of file.The success message is appropriate for standalone test execution, but the file should end with a newline as flagged by static analysis.
Apply this diff:
if __name__ == "__main__": - test_report_zip() + test_report_zip() +dream_layer_frontend/src/components/GenerateReportButton.tsx (1)
4-21: Consider improving URL configuration and user feedback.The async handler logic is solid, but consider these improvements:
- Hardcoded URL: The localhost URL should be configurable via environment variables for different environments.
- User feedback: Using
alert()is not very user-friendly. Consider using toast notifications or inline status messages.For the URL, consider using an environment variable:
- const res = await fetch("http://localhost:5002/api/generate-report", { + const res = await fetch(`${process.env.REACT_APP_API_URL || 'http://localhost:5002'}/api/generate-report`, {ComfyUI/api_server/routes/internal/internal_routes.py (1)
91-91: Add missing newline at end of file.The file should end with a newline as flagged by static analysis.
Apply this diff:
- return self._app + return self._app +ComfyUI/utils/generate_report.py (2)
1-5: Remove unused imports.Static analysis correctly identified that
osandjsonimports are unused in this file.Apply this diff:
-import os import csv -import json import zipfile from pathlib import Path
48-49: Add missing newline at end of file.The main execution block is correct, but the file should end with a newline as flagged by static analysis.
Apply this diff:
if __name__ == "__main__": - create_report() + create_report() +ComfyUI/utils/sample_data/config.json (2)
168-168: Absolute/implicit model filename reduces portability
"v1-5-pruned-emaonly.safetensors"assumes the file exists in the current ComfyUI checkpoint search path.
For users cloning the repo or running in CI this model may be missing, causing graph load failures.Recommend:
- "v1-5-pruned-emaonly.safetensors" + "./checkpoints/v1-5-pruned-emaonly.safetensors" # or make path configurable… or resolve the checkpoint via an environment variable / CLI argument instead of a hard-coded relative name.
1-385: Duplicate graph JSON – consider de-duplicationThis file is byte-for-byte identical to
utils/report/report/config.json. Keeping two copies:
- Bloats the repo (future edits must touch both).
- Risks them silently diverging.
If they serve different purposes (sample vs. report template) store a single canonical JSON and reference it from both locations (symlink, import at runtime, or copy in the report generator).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
ComfyUI/utils/report/report.zipis excluded by!**/*.zipComfyUI/utils/report/report/grids/grid_001.pngis excluded by!**/*.pngComfyUI/utils/report/report/grids/grid_002.pngis excluded by!**/*.pngComfyUI/utils/report/report/results.csvis excluded by!**/*.csvComfyUI/utils/sample_data/grid_001.pngis excluded by!**/*.pngComfyUI/utils/sample_data/grid_002.pngis excluded by!**/*.pngComfyUI/utils/sample_data/results.csvis excluded by!**/*.csv
📒 Files selected for processing (12)
ComfyUI/api_server/routes/internal/internal_routes.py(2 hunks)ComfyUI/tests-unit/report_bundle_test/test_generate_report.py(1 hunks)ComfyUI/user/default/comfy.settings.json(1 hunks)ComfyUI/user/default/workflows/test.json(1 hunks)ComfyUI/utils/generate_report.py(1 hunks)ComfyUI/utils/report/README.txt(1 hunks)ComfyUI/utils/report/report/README.txt(1 hunks)ComfyUI/utils/report/report/config.json(1 hunks)ComfyUI/utils/sample_data/config.json(1 hunks)dream_layer_backend/dream_layer.py(2 hunks)dream_layer_frontend/src/components/GenerateReportButton.tsx(1 hunks)dream_layer_frontend/src/pages/Index.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in report_bundler/bundler.py, the required_columns set intentionally contains only essential fields ...
Learnt from: rachanavarsha
PR: DreamLayer-AI/DreamLayer#80
File: report_bundler/bundler.py:7-7
Timestamp: 2025-08-06T18:59:31.412Z
Learning: In report_bundler/bundler.py, the REQUIRED_COLUMNS set intentionally contains only essential fields (image_path, sampler, steps, cfg, preset, seed) needed for bundling logic. Additional columns like width, height, grid_label, and notes are treated as optional metadata.
Applied to files:
ComfyUI/utils/report/README.txtComfyUI/utils/report/report/README.txt
🪛 Ruff (0.12.2)
ComfyUI/tests-unit/report_bundle_test/test_generate_report.py
26-26: print found
Remove print
(T201)
29-29: No newline at end of file
Add trailing newline
(W292)
ComfyUI/api_server/routes/internal/internal_routes.py
91-91: No newline at end of file
Add trailing newline
(W292)
ComfyUI/utils/generate_report.py
1-1: os imported but unused
Remove unused import: os
(F401)
3-3: json imported but unused
Remove unused import: json
(F401)
46-46: print found
Remove print
(T201)
49-49: No newline at end of file
Add trailing newline
(W292)
dream_layer_backend/dream_layer.py
329-329: SyntaxError: Got unexpected unicode
🔇 Additional comments (18)
dream_layer_frontend/src/pages/Index.tsx (1)
13-13: LGTM! Clean component import.The import statement correctly references the new GenerateReportButton component using appropriate relative path.
ComfyUI/user/default/comfy.settings.json (1)
2-8: Timestamp Verified – Approving Configuration UpdatesThe provided timestamp (1754554707284 ms) converts to 2025-08-07 08:18:27 UTC, which is within the expected range. All added settings—including color palette, version, timestamp, and the “KSamplerAdvanced” bookmark—are valid and consistent with ComfyUI conventions.
ComfyUI/tests-unit/report_bundle_test/test_generate_report.py (5)
1-4: LGTM!The imports are appropriate and all are used within the test function.
5-8: LGTM!The path construction using pathlib is clean and the relative navigation to find the report.zip file is correct.
9-9: LGTM!The assertion with descriptive error message is appropriate for testing file existence.
11-17: LGTM!The ZIP file validation logic is comprehensive and uses good practices like context managers and descriptive assertions.
19-24: LGTM!The CSV validation logic effectively ensures data integrity by verifying that all referenced images are included in the ZIP. The text decoding handling is correctly implemented for reading CSV from within the ZIP file.
dream_layer_frontend/src/components/GenerateReportButton.tsx (3)
1-3: LGTM!Standard React component setup with appropriate imports and functional component syntax.
23-30: LGTM!The button implementation is well-styled with appropriate hover states and visual feedback. The emoji icon adds a nice touch to the UI.
32-33: LGTM!Standard React component export.
ComfyUI/api_server/routes/internal/internal_routes.py (2)
7-8: LGTM!The new imports are appropriate for the route functionality that executes external scripts with path manipulation.
69-84: LGTM!The route implementation is well-structured with:
- Correct path navigation to the generate_report.py script
- Appropriate subprocess execution parameters for capturing output
- Proper error handling with CalledProcessError
- Consistent JSON response format with appropriate HTTP status codes
dream_layer_backend/dream_layer.py (1)
91-93: Verify the unconditional CPU mode change.The condition was changed from an environment variable check to
if True:, which means CPU mode will always be forced. Please confirm if this is intentional or if it should revert to the original conditional logic.If this was meant to be conditional, apply this diff:
-if True: +if os.environ.get('FORCE_CPU_MODE', '').lower() == 'true':ComfyUI/utils/generate_report.py (4)
7-18: LGTM!The path definitions using pathlib are clean and the README content appropriately describes the report bundle contents.
20-23: LGTM!Proper directory creation and ZIP file handling with compression and context manager usage.
24-38: LGTM!The file inclusion logic properly handles optional files and maintains the correct directory structure in the ZIP. The CSV processing ensures all referenced images are included in the bundle.
40-46: LGTM!The README creation and completion logic is sound. The print statement provides useful feedback for a utility script, which is appropriate despite static analysis flagging it.
ComfyUI/utils/sample_data/config.json (1)
2-2: Hard-coded UUID collides with another config fileBoth
utils/sample_data/config.jsonandutils/report/report/config.jsonembed the same"id": "cc1b2b54-eec0-4305-ae97-16299b26e39a".
If these configs are ever loaded simultaneously (e.g., listed in a UI, imported into the DB, etc.), the duplicate primary key may clash or overwrite metadata.
- Generate a fresh UUID for one of the files or drop the
idfield if it is not required at runtime.- Consider adding a brief comment in the README explaining that the UUID is arbitrary.
| @@ -0,0 +1 @@ | |||
| {"id":"cc1b2b54-eec0-4305-ae97-16299b26e39a","revision":0,"last_node_id":12,"last_link_id":12,"nodes":[{"id":8,"type":"VAEDecode","pos":[1385.1756591796875,225.75196838378906],"size":[210,46],"flags":{},"order":5,"mode":0,"inputs":[{"localized_name":"samples","name":"samples","type":"LATENT","link":7},{"localized_name":"vae","name":"vae","type":"VAE","link":11}],"outputs":[{"localized_name":"IMAGE","name":"IMAGE","type":"IMAGE","slot_index":0,"links":[9]}],"properties":{"Node name for S&R":"VAEDecode"},"widgets_values":[]},{"id":5,"type":"EmptyLatentImage","pos":[488.48797607421875,706.76806640625],"size":[315,106],"flags":{},"order":0,"mode":0,"inputs":[{"localized_name":"width","name":"width","type":"INT","widget":{"name":"width"},"link":null},{"localized_name":"height","name":"height","type":"INT","widget":{"name":"height"},"link":null},{"localized_name":"batch_size","name":"batch_size","type":"INT","widget":{"name":"batch_size"},"link":null}],"outputs":[{"localized_name":"LATENT","name":"LATENT","type":"LATENT","slot_index":0,"links":[2]}],"properties":{"Node name for S&R":"EmptyLatentImage"},"widgets_values":[512,512,1]},{"id":7,"type":"CLIPTextEncode","pos":[413.4464416503906,393.6659240722656],"size":[425.27801513671875,180.6060791015625],"flags":{},"order":3,"mode":0,"inputs":[{"localized_name":"clip","name":"clip","type":"CLIP","link":10},{"localized_name":"text","name":"text","type":"STRING","widget":{"name":"text"},"link":null}],"outputs":[{"localized_name":"CONDITIONING","name":"CONDITIONING","type":"CONDITIONING","slot_index":0,"links":[6]}],"properties":{"Node name for S&R":"CLIPTextEncode"},"widgets_values":["text, watermark"],"color":"#323","bgcolor":"#535"},{"id":4,"type":"CheckpointLoaderSimple","pos":[12.355875015258789,538.53125],"size":[315,98],"flags":{},"order":1,"mode":0,"inputs":[{"localized_name":"ckpt_name","name":"ckpt_name","type":"COMBO","widget":{"name":"ckpt_name"},"link":null}],"outputs":[{"localized_name":"MODEL","name":"MODEL","type":"MODEL","slot_index":0,"links":[12]},{"localized_name":"CLIP","name":"CLIP","type":"CLIP","slot_index":1,"links":[3,10]},{"localized_name":"VAE","name":"VAE","type":"VAE","slot_index":2,"links":[11]}],"properties":{"Node name for S&R":"CheckpointLoaderSimple"},"widgets_values":["v1-5-pruned-emaonly.safetensors"]},{"id":9,"type":"SaveImage","pos":[1456.6739501953125,425.198486328125],"size":[210,270],"flags":{},"order":6,"mode":0,"inputs":[{"localized_name":"images","name":"images","type":"IMAGE","link":9},{"localized_name":"filename_prefix","name":"filename_prefix","type":"STRING","widget":{"name":"filename_prefix"},"link":null}],"outputs":[],"properties":{},"widgets_values":["ComfyUI"]},{"id":3,"type":"KSampler","pos":[1028.920166015625,474.3313293457031],"size":[315,262],"flags":{},"order":4,"mode":0,"inputs":[{"localized_name":"model","name":"model","type":"MODEL","link":12},{"localized_name":"positive","name":"positive","type":"CONDITIONING","link":4},{"localized_name":"negative","name":"negative","type":"CONDITIONING","link":6},{"localized_name":"latent_image","name":"latent_image","type":"LATENT","link":2},{"localized_name":"seed","name":"seed","type":"INT","widget":{"name":"seed"},"link":null},{"localized_name":"steps","name":"steps","type":"INT","widget":{"name":"steps"},"link":null},{"localized_name":"cfg","name":"cfg","type":"FLOAT","widget":{"name":"cfg"},"link":null},{"localized_name":"sampler_name","name":"sampler_name","type":"COMBO","widget":{"name":"sampler_name"},"link":null},{"localized_name":"scheduler","name":"scheduler","type":"COMBO","widget":{"name":"scheduler"},"link":null},{"localized_name":"denoise","name":"denoise","type":"FLOAT","widget":{"name":"denoise"},"link":null}],"outputs":[{"localized_name":"LATENT","name":"LATENT","type":"LATENT","slot_index":0,"links":[7]}],"properties":{"Node name for S&R":"KSampler"},"widgets_values":[157166686672613,"randomize",20,8,"euler","normal",1]},{"id":6,"type":"CLIPTextEncode","pos":[436.2958984375,132.760009765625],"size":[422.84503173828125,164.31304931640625],"flags":{},"order":2,"mode":0,"inputs":[{"localized_name":"clip","name":"clip","type":"CLIP","link":3},{"localized_name":"text","name":"text","type":"STRING","widget":{"name":"text"},"link":null}],"outputs":[{"localized_name":"CONDITIONING","name":"CONDITIONING","type":"CONDITIONING","slot_index":0,"links":[4]}],"properties":{"Node name for S&R":"CLIPTextEncode"},"widgets_values":["cute little golden retriever puppies"],"color":"#223","bgcolor":"#335"}],"links":[[2,5,0,3,3,"LATENT"],[3,4,1,6,0,"CLIP"],[4,6,0,3,1,"CONDITIONING"],[6,7,0,3,2,"CONDITIONING"],[7,3,0,8,0,"LATENT"],[9,8,0,9,0,"IMAGE"],[10,4,1,7,0,"CLIP"],[11,4,2,8,1,"VAE"],[12,4,0,3,0,"MODEL"]],"groups":[],"config":{},"extra":{"ds":{"scale":0.6115909044841489,"offset":[522.8425740808559,15.336086631059548]}},"version":0.4} No newline at end of file | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Improve JSON formatting and validate hardcoded references.
This minified JSON is difficult to read and maintain. Consider formatting it for better readability, and verify that hardcoded references like "v1-5-pruned-emaonly.safetensors" correspond to actual available models.
Apply this formatting for better readability:
-{"id":"cc1b2b54-eec0-4305-ae97-16299b26e39a","revision":0,"last_node_id":12,"last_link_id":12,"nodes":[...
+{
+ "id": "cc1b2b54-eec0-4305-ae97-16299b26e39a",
+ "revision": 0,
+ "last_node_id": 12,
+ "last_link_id": 12,
+ "nodes": [
+ ...
+ ],
+ ...
+}Verify that the referenced model and workflow components exist:
🏁 Script executed:
#!/bin/bash
# Verify model file and workflow validity
echo "Checking for referenced model file..."
fd "v1-5-pruned-emaonly.safetensors" --type f
echo "Validating JSON structure..."
python3 -c "
import json
import sys
try:
with open('ComfyUI/user/default/workflows/test.json', 'r') as f:
workflow = json.load(f)
print(f'Valid JSON with {len(workflow.get(\"nodes\", []))} nodes')
# Check for required node types
node_types = [node.get('type') for node in workflow.get('nodes', [])]
required_types = ['CheckpointLoaderSimple', 'CLIPTextEncode', 'KSampler', 'VAEDecode', 'SaveImage']
missing = [t for t in required_types if t not in node_types]
if missing:
print(f'Missing node types: {missing}')
else:
print('All required node types present')
except Exception as e:
print(f'JSON validation failed: {e}')
sys.exit(1)
"Length of output: 1034
Ensure JSON readability and model availability
The workflow JSON is valid and includes all required nodes, but it’s still minified and references a model file that isn’t present in the repo. Please:
-
Format
ComfyUI/user/default/workflows/test.jsonfor readability:-{"id":"cc1b2b54-eec0-4305-ae97-16299b26e39a","revision":0,"last_node_id":12,"last_link_id":12,"nodes":[...],"version":0.4} +{ + "id": "cc1b2b54-eec0-4305-ae97-16299b26e39a", + "revision": 0, + "last_node_id": 12, + "last_link_id": 12, + "nodes": [ + { + "id": 4, + "type": "CheckpointLoaderSimple", + "widgets_values": ["v1-5-pruned-emaonly.safetensors"], + ... + }, + ... + ], + "version": 0.4 +}
-
Verify or include the referenced model:
- The script
fd "v1-5-pruned-emaonly.safetensors" --type freturned no matches. - Either add
v1-5-pruned-emaonly.safetensorsto your model directory, update the filename to one that exists, or mock the checkpoint for testing.
- The script
ComfyUI workflows should be both maintainable and immediately executable without missing dependencies.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ComfyUI/user/default/workflows/test.json at line 1, the JSON content is
minified and references a model file "v1-5-pruned-emaonly.safetensors" that is
missing from the repository. To fix this, pretty-print the JSON to improve
readability by formatting it with proper indentation and line breaks.
Additionally, verify the presence of the referenced model file in the model
directory; if it is missing, either add the actual
"v1-5-pruned-emaonly.safetensors" file, update the JSON to reference an existing
model file, or create a mock checkpoint file for testing purposes to ensure the
workflow can run without missing dependencies.
| @app.route('/api/generate-report', methods=['POST']) | ||
| def handle_generate_report(): | ||
| try: | ||
| script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py') | ||
| result = subprocess.run(["python", script_path], capture_output=True, text=True, check=True) | ||
| return jsonify({ | ||
| "status": "success", | ||
| "message": "Report generated successfully.", | ||
| "output": result.stdout | ||
| }) | ||
| except subprocess.CalledProcessError as e: | ||
| return jsonify({ | ||
| "status": "error", | ||
| "message": f"Failed to generate report: {e.stderr}" | ||
| }), 500 | ||
| except Exception as e: | ||
| return jsonify({ | ||
| "status": "error", | ||
| "message": f"Unexpected error: {str(e)}" | ||
| }), 500 | ||
|
|
There was a problem hiding this comment.
Fix the path construction syntax error.
The path construction has a critical syntax error with literal backslashes and will not work cross-platform.
Apply this diff to fix the path construction:
- script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py')
+ script_path = os.path.join(os.path.dirname(__file__), '..', 'ComfyUI', 'utils', 'generate_report.py')This uses proper path separators that work cross-platform and fixes the unicode syntax error flagged by static analysis.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @app.route('/api/generate-report', methods=['POST']) | |
| def handle_generate_report(): | |
| try: | |
| script_path = os.path.join(os.path.dirname(__file__), '..\ComfyUI\utils\generate_report.py') | |
| result = subprocess.run(["python", script_path], capture_output=True, text=True, check=True) | |
| return jsonify({ | |
| "status": "success", | |
| "message": "Report generated successfully.", | |
| "output": result.stdout | |
| }) | |
| except subprocess.CalledProcessError as e: | |
| return jsonify({ | |
| "status": "error", | |
| "message": f"Failed to generate report: {e.stderr}" | |
| }), 500 | |
| except Exception as e: | |
| return jsonify({ | |
| "status": "error", | |
| "message": f"Unexpected error: {str(e)}" | |
| }), 500 | |
| @app.route('/api/generate-report', methods=['POST']) | |
| def handle_generate_report(): | |
| try: | |
| script_path = os.path.join(os.path.dirname(__file__), '..', 'ComfyUI', 'utils', 'generate_report.py') | |
| result = subprocess.run( | |
| ["python", script_path], | |
| capture_output=True, | |
| text=True, | |
| check=True | |
| ) | |
| return jsonify({ | |
| "status": "success", | |
| "message": "Report generated successfully.", | |
| "output": result.stdout | |
| }) | |
| except subprocess.CalledProcessError as e: | |
| return jsonify({ | |
| "status": "error", | |
| "message": f"Failed to generate report: {e.stderr}" | |
| }), 500 | |
| except Exception as e: | |
| return jsonify({ | |
| "status": "error", | |
| "message": f"Unexpected error: {str(e)}" | |
| }), 500 |
🧰 Tools
🪛 Ruff (0.12.2)
329-329: SyntaxError: Got unexpected unicode
🤖 Prompt for AI Agents
In dream_layer_backend/dream_layer.py around lines 326 to 346, the script_path
is constructed using a string with literal backslashes, causing syntax errors
and cross-platform issues. Replace the string concatenation with os.path.join
using separate arguments for each path component to ensure proper path
separators and avoid unicode errors. This will make the path construction
cross-platform compatible and fix the syntax error.
1 participant
Description
This PR implements Task #5: Report Bundle (report.zip) as part of the DreamLayer open source challenge. It adds a one-click report generation feature that packages all necessary artifacts from a generation run into a single, portable
report.zip.Changes Made
generate_report.pyto generatereport.zipcontaining:config.jsonresults.csvgrids/<images>README.txttests-unit/report_bundle_test/to verify:POST /api/generate-reportGenerateReportButton.tsxIndex.tsx)Evidence Required ✅
UI Screenshot
Generated Image
Logs