Add Labeled Grid Exporter module and test suite #90
Conversation
Reviewer's GuideThis PR adds a new LabeledGridExporter module that discovers recent DreamLayer images, extracts or mocks generation metadata, assembles them into parameter-labeled grids, and exports optimized PNGs. It also provides a CLI interface, integrates the feature into the frontend Extras tab with font assets for cross-platform Unicode support, and delivers a comprehensive 11-test pytest suite including snapshot testing. Sequence diagram for CLI grid export processsequenceDiagram
actor User
participant CLI
participant LabeledGridExporter
participant FileSystem
User->>CLI: Run grid_exporter.py with arguments
CLI->>LabeledGridExporter: __init__()
CLI->>LabeledGridExporter: create_grid_from_recent(count, grid_size, filename)
LabeledGridExporter->>FileSystem: get_recent_images(count)
LabeledGridExporter->>FileSystem: extract_metadata_from_logs()
LabeledGridExporter->>LabeledGridExporter: create_labeled_grid(images, grid_size)
LabeledGridExporter->>FileSystem: export_grid(images, filename, grid_size)
CLI->>User: Output success message with grid path
Class diagram for the new LabeledGridExporter moduleclassDiagram
class LabeledGridExporter {
+__init__(output_dir: Optional[str])
+get_recent_images(count: int) List[Dict]
+extract_metadata_from_logs(image_filename: str, image_timestamp: float) Dict[str, str]
+create_labeled_grid(images: List[Dict], grid_size: Optional[Tuple[int, int]]) Image
+export_grid(images: List[Dict], filename: Optional[str], grid_size: Optional[Tuple[int, int]]) str
+create_grid_from_recent(count: int, grid_size: Optional[Tuple[int, int]], filename: Optional[str]) str
-_setup_font() ImageFont
-output_dir: str
-served_images_dir: str
-logs_dir: str
-font: ImageFont
}
LabeledGridExporter --> ImageFont : uses
LabeledGridExporter --> Image : creates
LabeledGridExporter --> Dict : returns
LabeledGridExporter --> List : returns
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. WalkthroughAdds a labeled image grid export feature: backend exporter module, JSON settings, API endpoints, frontend UI for creating/previewing grids, a pytest suite, a CLI entry, and supporting files including a UI mockup and local permissions config. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant BackendAPI
participant LabeledGridExporter
participant FileSystem
User->>Frontend: Open Grid Export UI, set options, submit
Frontend->>BackendAPI: POST /api/extras/grid-export (params)
BackendAPI->>LabeledGridExporter: create_grid_from_recent(params)
LabeledGridExporter->>FileSystem: scan recent images (served/output dirs)
LabeledGridExporter->>LabeledGridExporter: extract metadata (logs or mock)
LabeledGridExporter->>FileSystem: load images, assemble canvas, render labels
LabeledGridExporter->>FileSystem: save PNG to served dir
LabeledGridExporter-->>BackendAPI: return file path, size, dimensions
BackendAPI-->>Frontend: JSON with image URL and metadata
Frontend-->>User: display preview and info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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 @bryzle - I've reviewed your changes - here's some feedback:
- Replace the numerous print statements with Python’s logging module to allow configurable verbosity and better integration with your existing logs.
- The extract_metadata_from_logs method currently returns mock metadata—consider implementing actual log-file parsing or clearly separating the stub logic to avoid confusion in production.
- The LabeledGridExporter class is handling many responsibilities; think about refactoring into smaller components (e.g., metadata extraction, grid layout, rendering) to improve maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Replace the numerous print statements with Python’s logging module to allow configurable verbosity and better integration with your existing logs.
- The extract_metadata_from_logs method currently returns mock metadata—consider implementing actual log-file parsing or clearly separating the stub logic to avoid confusion in production.
- The LabeledGridExporter class is handling many responsibilities; think about refactoring into smaller components (e.g., metadata extraction, grid layout, rendering) to improve maintainability.
## Individual Comments
### Comment 1
<location> `dream_layer_backend/grid_exporter.py:353` </location>
<code_context>
+ output_path = os.path.join(self.output_dir, filename)
+
+ # Save with metadata
+ grid_image.save(output_path, format='PNG', optimize=True)
+
+ file_size = os.path.getsize(output_path)
+ print(f"✅ Grid exported successfully:")
+ print(f" 📄 File: {output_path}")
</code_context>
<issue_to_address>
PNG metadata is mentioned but not actually set.
To embed metadata as described, use the 'pnginfo' parameter in the save method.
</issue_to_address>
### Comment 2
<location> `dream_layer_backend/test_grid_exporter.py:101` </location>
<code_context>
+ metadata2 = self.exporter.extract_metadata_from_logs("test_image_002.png", 1234567891.0)
+ assert metadata != metadata2 # Should be different due to index variation
+
+ def test_create_labeled_grid_basic(self):
+ """Test basic grid creation functionality"""
+ images = self.exporter.get_recent_images(count=4)
+ grid_image = self.exporter.create_labeled_grid(images, grid_size=(2, 2))
+
+ assert isinstance(grid_image, Image.Image)
+ assert grid_image.mode == 'RGB'
+ assert grid_image.width > 0
+ assert grid_image.height > 0
+
+ # Grid should be larger than individual images due to labels and padding
+ assert grid_image.width > 64 * 2 # Larger than 2 images side by side
+ assert grid_image.height > 64 * 2 # Larger than 2 images stacked
+
+ def test_grid_auto_sizing(self):
</code_context>
<issue_to_address>
Test does not verify that labels are actually rendered in the grid.
Add an assertion to check that the expected label text or color regions appear in the grid image to ensure labels are rendered correctly.
Suggested implementation:
```python
def test_create_labeled_grid_basic(self):
"""Test basic grid creation functionality"""
import pytesseract
images = self.exporter.get_recent_images(count=4)
grid_image = self.exporter.create_labeled_grid(images, grid_size=(2, 2))
assert isinstance(grid_image, Image.Image)
assert grid_image.mode == 'RGB'
assert grid_image.width > 0
assert grid_image.height > 0
# Grid should be larger than individual images due to labels and padding
assert grid_image.width > 64 * 2 # Larger than 2 images side by side
assert grid_image.height > 64 * 2 # Larger than 2 images stacked
# Check that expected label text appears in the grid image
# Assuming labels are based on image filenames
expected_labels = [img.filename if hasattr(img, "filename") else f"Image {i+1}" for i, img in enumerate(images)]
ocr_text = pytesseract.image_to_string(grid_image)
for label in expected_labels:
assert label in ocr_text, f"Label '{label}' not found in grid image OCR output"
```
- If your images do not have a `filename` attribute, adjust the `expected_labels` list to match how labels are generated in `create_labeled_grid`.
- Ensure `pytesseract` is installed and available in your test environment.
- If you use a different method for rendering labels (e.g., colored rectangles), you may want to check pixel colors instead of using OCR.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| grid_image.save(output_path, format='PNG', optimize=True) | ||
|
|
||
| file_size = os.path.getsize(output_path) |
There was a problem hiding this comment.
issue: PNG metadata is mentioned but not actually set.
To embed metadata as described, use the 'pnginfo' parameter in the save method.
| def test_create_labeled_grid_basic(self): | ||
| """Test basic grid creation functionality""" | ||
| images = self.exporter.get_recent_images(count=4) | ||
| grid_image = self.exporter.create_labeled_grid(images, grid_size=(2, 2)) | ||
|
|
||
| assert isinstance(grid_image, Image.Image) | ||
| assert grid_image.mode == 'RGB' | ||
| assert grid_image.width > 0 | ||
| assert grid_image.height > 0 | ||
|
|
There was a problem hiding this comment.
suggestion (testing): Test does not verify that labels are actually rendered in the grid.
Add an assertion to check that the expected label text or color regions appear in the grid image to ensure labels are rendered correctly.
Suggested implementation:
def test_create_labeled_grid_basic(self):
"""Test basic grid creation functionality"""
import pytesseract
images = self.exporter.get_recent_images(count=4)
grid_image = self.exporter.create_labeled_grid(images, grid_size=(2, 2))
assert isinstance(grid_image, Image.Image)
assert grid_image.mode == 'RGB'
assert grid_image.width > 0
assert grid_image.height > 0
# Grid should be larger than individual images due to labels and padding
assert grid_image.width > 64 * 2 # Larger than 2 images side by side
assert grid_image.height > 64 * 2 # Larger than 2 images stacked
# Check that expected label text appears in the grid image
# Assuming labels are based on image filenames
expected_labels = [img.filename if hasattr(img, "filename") else f"Image {i+1}" for i, img in enumerate(images)]
ocr_text = pytesseract.image_to_string(grid_image)
for label in expected_labels:
assert label in ocr_text, f"Label '{label}' not found in grid image OCR output"- If your images do not have a
filenameattribute, adjust theexpected_labelslist to match how labels are generated increate_labeled_grid. - Ensure
pytesseractis installed and available in your test environment. - If you use a different method for rendering labels (e.g., colored rectangles), you may want to check pixel colors instead of using OCR.
| for i in range(len(images) - 1): | ||
| assert images[i]['mtime'] >= images[i+1]['mtime'] |
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
dream_layer_backend/grid_exporter.py
Outdated
| try: | ||
| return ImageFont.load_default() | ||
| except: | ||
| return ImageFont.load_default() |
There was a problem hiding this comment.
issue (code-quality): Use except Exception: rather than bare except: (do-not-use-bare-except)
dream_layer_backend/grid_exporter.py
Outdated
|
|
||
| # Extract index from filename (e.g., DreamLayer_00001_.png -> 1) | ||
| match = re.search(r'(\d+)', image_filename) | ||
| index = int(match.group(1)) if match else 0 |
There was a problem hiding this comment.
issue (code-quality): Replace m.group(x) with m[x] for re.Match objects (use-getitem-for-re-match-groups)
| 'seed': seeds[index % len(seeds)] | ||
| } | ||
|
|
||
| def create_labeled_grid(self, |
There was a problem hiding this comment.
issue (code-quality): Low code quality found in LabeledGridExporter.create_labeled_grid - 25% (low-code-quality)
Explanation
The quality score for this function is below the quality threshold of 25%.This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
their own functions. This is the most important thing you can do - ideally a
function should be less than 10 lines. - Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
sits together within the function rather than being scattered.
dream_layer_backend/grid_exporter.py
Outdated
| if not filename.lower().endswith('.png'): | ||
| filename += '.png' | ||
|
|
||
| print(f"🎨 Creating labeled grid...") |
There was a problem hiding this comment.
issue (code-quality): Replace f-string with no interpolated values with string [×2] (remove-redundant-fstring)
dream_layer_backend/grid_exporter.py
Outdated
| if not recent_images: | ||
| raise ValueError("No recent images found to create grid") | ||
|
|
||
| print(f"📋 Selected images:") |
There was a problem hiding this comment.
issue (code-quality): Replace f-string with no interpolated values with string (remove-redundant-fstring)
|
|
||
|
|
||
| def main(): | ||
| """ |
There was a problem hiding this comment.
issue (code-quality): We've found these issues:
- Move setting of default value for variable into
elsebranch (introduce-default-else) - Extract code out into function (
extract-method) - Replace if statement with if expression (
assign-if-exp)
| file_size = os.path.getsize(output_path) | ||
| assert 5000 < file_size < 100000 # Reasonable PNG size for small grid | ||
|
|
||
| print(f"✅ Snapshot test passed!") |
There was a problem hiding this comment.
issue (code-quality): Replace f-string with no interpolated values with string (remove-redundant-fstring)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
dream_layer_backend/settings.json (1)
9-9: Confirm metadata writing honors UTF-8 and Windows-safe behavior.With
"saveMetadata": true, ensure the writer uses UTF-8, disables ASCII-escaping, and normalizes newlines so Unicode remains intact on Windows.Checklist:
- Use encoding="utf-8" and ensure_ascii=False when serializing JSON metadata.
- Normalize path separators when embedding paths in metadata.
- If EXIF/IPTC writing is involved, validate code paths on Windows with non-ASCII content. I can help add a small utility to centralize safe metadata writes.
comfy-easy-grids (1)
1-1: Optional: prefer a tagged release or document the bumpIf a stable tag exists for this commit, consider pinning to the tag for clarity. Otherwise, add a brief note in CHANGELOG/Release notes indicating the submodule bump and any expected impact.
dream_layer_backend/test_grid_exporter.py (3)
12-12: Remove unused import Path
pathlib.Pathis imported but not used.-from pathlib import Path
186-186: Remove unnecessary f-string without placeholdersAvoid f-strings with no interpolations.
- print(f"✅ Snapshot test passed!") + print("✅ Snapshot test passed!")
161-191: Optional: Avoid using the closed PIL image outside the context managerWhile width/height are accessible, it’s cleaner to compute and print inside the context or cache values before closing.
Example:
with Image.open(output_path) as grid_img: w, h = grid_img.width, grid_img.height grid_array = np.array(grid_img) # ... print(f" 📐 Dimensions: {w}x{h}")dream_layer_backend/grid_exporter.py (3)
15-15: Remove unused imports
jsonandPathare not used.-import json @@ -from pathlib import PathAlso applies to: 17-17
343-343: Remove unnecessary f-strings without placeholdersThese prints don’t interpolate values; drop the f prefix to appease linters.
- print(f"🎨 Creating labeled grid...") + print("🎨 Creating labeled grid...") @@ - print(f"✅ Grid exported successfully:") + print("✅ Grid exported successfully:") @@ - print(f"📋 Selected images:") + print("📋 Selected images:")Also applies to: 356-356, 393-393
410-446: Optional: Replace prints with logging for better control and Windows encoding safetyEmojis can cause UnicodeEncodeError on some Windows consoles. Using logging with a safe formatter or stripping non-ASCII for stream handlers can improve UX.
Example:
import logging logger = logging.getLogger(__name__) # then: logger.info("Grid exported successfully")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
comfy-easy-grids(1 hunks)dream_layer_backend/grid_exporter.py(1 hunks)dream_layer_backend/settings.json(1 hunks)dream_layer_backend/test_grid_exporter.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code Graph Analysis (1)
dream_layer_backend/test_grid_exporter.py (1)
dream_layer_backend/grid_exporter.py (7)
LabeledGridExporter(23-398)get_recent_images(107-158)extract_metadata_from_logs(160-194)create_labeled_grid(196-315)export_grid(317-361)create_grid_from_recent(363-398)main(401-446)
🪛 Ruff (0.12.2)
dream_layer_backend/test_grid_exporter.py
12-12: pathlib.Path imported but unused
Remove unused import: pathlib.Path
(F401)
186-186: f-string without any placeholders
Remove extraneous f prefix
(F541)
215-215: Local variable grid is assigned to but never used
Remove assignment to unused variable grid
(F841)
dream_layer_backend/grid_exporter.py
15-15: json imported but unused
Remove unused import: json
(F401)
17-17: pathlib.Path imported but unused
Remove unused import: pathlib.Path
(F401)
104-104: Do not use bare except
(E722)
343-343: f-string without any placeholders
Remove extraneous f prefix
(F541)
356-356: f-string without any placeholders
Remove extraneous f prefix
(F541)
393-393: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (2)
dream_layer_backend/settings.json (1)
1-10: Centralizing backend paths in a single config is a good move.This improves testability and simplifies CLI/UI integration. No blockers from a config-structure standpoint.
comfy-easy-grids (1)
1-1: Ignore submodule verification – no submodule definedThere is no
.gitmodulesfile in the repository andcomfy-easy-grids/is a normal source directory, not a git submodule. You can disregard the SHA‐validation and CI submodule initialization steps.Likely an incorrect or invalid review comment.
dream_layer_backend/settings.json
Outdated
| "outputDirectory": "/path/to/outputs", | ||
| "modelsDirectory": "/path/to/parent/of/models", | ||
| "controlNetModelsPath": "/path/to/models/ControlNet", | ||
| "upscalerModelsPath": "/path/to/models/ESRGAN", | ||
| "vaeModelsPath": "/path/to/models/VAE", | ||
| "loraEmbeddingsPath": "/path/to/models/Lora", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make paths portable and safe-by-default (avoid absolute placeholders).
Use relative, project-root-based defaults and allow env/home expansion to work cross-platform and in CI. Absolute placeholders like “/path/to/…” often break on Windows and in sandboxed environments.
Suggested diff:
- "outputDirectory": "/path/to/outputs",
- "modelsDirectory": "/path/to/parent/of/models",
- "controlNetModelsPath": "/path/to/models/ControlNet",
- "upscalerModelsPath": "/path/to/models/ESRGAN",
- "vaeModelsPath": "/path/to/models/VAE",
- "loraEmbeddingsPath": "/path/to/models/Lora",
+ "outputDirectory": "./outputs",
+ "modelsDirectory": "./models",
+ "controlNetModelsPath": "./models/ControlNet",
+ "upscalerModelsPath": "./models/ESRGAN",
+ "vaeModelsPath": "./models/VAE",
+ "loraEmbeddingsPath": "./models/Lora",Additionally:
- Support expansion at load time: expanduser (~), expandvars ($VAR/%VAR%), and Path.resolve().
- Consider allowing overrides via environment variables or CLI flags (e.g., DREAMLAYER_OUTPUT_DIR). I can draft the loader helper if helpful.
📝 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.
| "outputDirectory": "/path/to/outputs", | |
| "modelsDirectory": "/path/to/parent/of/models", | |
| "controlNetModelsPath": "/path/to/models/ControlNet", | |
| "upscalerModelsPath": "/path/to/models/ESRGAN", | |
| "vaeModelsPath": "/path/to/models/VAE", | |
| "loraEmbeddingsPath": "/path/to/models/Lora", | |
| "outputDirectory": "./outputs", | |
| "modelsDirectory": "./models", | |
| "controlNetModelsPath": "./models/ControlNet", | |
| "upscalerModelsPath": "./models/ESRGAN", | |
| "vaeModelsPath": "./models/VAE", | |
| "loraEmbeddingsPath": "./models/Lora", |
🤖 Prompt for AI Agents
In dream_layer_backend/settings.json lines 2 to 7, replace the absolute
placeholder paths with relative paths based on the project root to ensure
portability across platforms. Implement path expansion at load time to handle
user home (~) and environment variables ($VAR or %VAR%) and resolve paths
properly. Additionally, modify the configuration loader to allow overriding
these paths via environment variables or CLI flags for flexibility in different
environments like CI or sandboxed systems.
dream_layer_backend/settings.json
Outdated
| "upscalerModelsPath": "/path/to/models/ESRGAN", | ||
| "vaeModelsPath": "/path/to/models/VAE", | ||
| "loraEmbeddingsPath": "/path/to/models/Lora", | ||
| "filenameFormat": "paste the path if it is outside project root", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
filenameFormat contains an instruction, not a usable format; clarify intent and set a valid default.
Current value looks like guidance text rather than a real format string, which can cause runtime issues if the code expects a pattern.
Action options (pick the one that matches actual usage in grid_exporter.py):
- If this is a filename pattern: set a sane default like a timestamp-based name.
- "filenameFormat": "paste the path if it is outside project root",
+ "filenameFormat": "grid_{timestamp}.png",- If the intent was a directory path (e.g., a “served images” dir), then rename the key and provide a valid path instead:
- "filenameFormat": "paste the path if it is outside project root",
+ "servedImagesDirectory": "./outputs/served",To verify actual usage in the repo, run:
🏁 Script executed:
#!/bin/bash
# Inspect where/ how filenameFormat (or a served images dir) is referenced.
rg -n --hidden -S "filenameFormat|servedImagesDirectory|served.*images|file(name)?_?format" -A 3Length of output: 13448
Replace placeholder with a concrete filename pattern
The filenameFormat setting isn’t a directory path but a filename template for saved images. Right now it’s just guidance text, which will break any code that tries to format filenames at runtime. Please update it to a valid pattern and keep front- and back-end defaults in sync:
• In dream_layer_backend/settings.json (around line 8):
- "filenameFormat": "paste the path if it is outside project root",
+ "filenameFormat": "[model_name]_[seed]_[prompt_words]",• In dream_layer_frontend/src/features/Configurations/useSettingsStore.ts (around line 70):
- filenameFormat: 'paste the path if it is outside project root',
+ filenameFormat: '[model_name]_[seed]_[prompt_words]',If you’d rather use a timestamp-based default, you could substitute "[model_name]_[seed]_[prompt_words]" with something like "grid_{timestamp}.png". Just ensure it’s a real template string, not instructional text.
📝 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.
| "filenameFormat": "paste the path if it is outside project root", | |
| "filenameFormat": "[model_name]_[seed]_[prompt_words]", |
| "filenameFormat": "paste the path if it is outside project root", | |
| filenameFormat: '[model_name]_[seed]_[prompt_words]', |
🤖 Prompt for AI Agents
In dream_layer_backend/settings.json at line 8, replace the placeholder text in
the "filenameFormat" setting with a valid filename template string instead of
instructional text. Use a concrete pattern like "grid_{timestamp}.png" or
"[model_name]_[seed]_[prompt_words].png" to ensure the filename formatting code
works correctly at runtime. Also verify that this pattern matches the default
used in dream_layer_frontend/src/features/Configurations/useSettingsStore.ts
around line 70 to keep front- and back-end defaults consistent.
| # Test with invalid grid size | ||
| images = self.exporter.get_recent_images(count=4) | ||
| grid = self.exporter.create_labeled_grid(images, grid_size=(0, 1)) | ||
| # Should handle gracefully and auto-calculate | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Invalid grid_size test relies on unimplemented fallback; add assertion and align implementation
create_labeled_grid currently does not auto-calc when grid_size has zeros; this can lead to creating a 0-width image. Either assert that a ValueError is raised or, preferably, make the exporter treat invalid grid_size as None and auto-calc. Also fixes the F841 lint by using the returned value.
Apply this diff to the test:
- # Test with invalid grid size
- images = self.exporter.get_recent_images(count=4)
- grid = self.exporter.create_labeled_grid(images, grid_size=(0, 1))
- # Should handle gracefully and auto-calculate
+ # Test with invalid grid size -> should be handled gracefully by auto-calculation
+ images = self.exporter.get_recent_images(count=4)
+ grid = self.exporter.create_labeled_grid(images, grid_size=(0, 1))
+ assert isinstance(grid, Image.Image)And update the implementation as suggested in grid_exporter.py (see separate comment) to auto-calc on invalid (cols < 1 or rows < 1).
📝 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.
| # Test with invalid grid size | |
| images = self.exporter.get_recent_images(count=4) | |
| grid = self.exporter.create_labeled_grid(images, grid_size=(0, 1)) | |
| # Should handle gracefully and auto-calculate | |
| # Test with invalid grid size -> should be handled gracefully by auto-calculation | |
| images = self.exporter.get_recent_images(count=4) | |
| grid = self.exporter.create_labeled_grid(images, grid_size=(0, 1)) | |
| assert isinstance(grid, Image.Image) |
🧰 Tools
🪛 Ruff (0.12.2)
215-215: Local variable grid is assigned to but never used
Remove assignment to unused variable grid
(F841)
🤖 Prompt for AI Agents
In dream_layer_backend/test_grid_exporter.py around lines 213 to 217, the test
for invalid grid_size currently calls create_labeled_grid with a zero dimension
but does not assert the expected behavior or use the returned value, causing a
lint warning. Update the test to either assert that a ValueError is raised when
grid_size contains zeros or modify the test to check that the exporter treats
invalid grid_size as None and auto-calculates the grid size. Also, ensure the
returned grid is used to avoid the F841 lint error. Additionally, update the
create_labeled_grid implementation in grid_exporter.py to auto-calc grid size
when cols or rows are less than 1.
- Added state management for grid export settings including count, grid size, and filename. - Integrated API call to create grid and handle responses, including success and error states. - Enhanced UI components in ExtrasTab and ExtrasPage for grid export settings and preview. - Introduced logging for test cases in test_grid_exporter.py to replace print statements with logger. - Created a mockup HTML for grid export UI to visualize the feature. - Updated permissions in settings.local.json for backend operations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
dream_layer_backend/grid_exporter.py (3)
585-593: Invalid grid_size (0 or negative) still not handledIf a caller supplies
(0, 1)or similar,colsorrowsbecome zero and later maths will produce a zero-width canvas. Refactor to fall back to auto-calc when either dimension ≤ 0 (see previous review).
595-611: Image files opened without context managerImages remain open, causing Windows file-lock issues and resource leaks. Use
with Image.open(...) as im:and.copy()to detach.
404-415: Bareexcepthides real errors in font loadingReplace with
except Exception:(or log & continue) and drop the second try/except –ImageFont.load_default()never raises.- try: - return ImageFont.load_default() - except: - return ImageFont.load_default() + return ImageFont.load_default()dream_layer_backend/test_grid_exporter.py (1)
220-222: Unused variablegridThe variable is assigned but not used, tripping Ruff F841. Either assert on it or drop the assignment.
🧹 Nitpick comments (6)
dream_layer_backend/extras.py (1)
344-350: Remove redundantf-stringsLines 344–349 contain
f"strings without placeholders – drop thefprefix to silence Ruff F541 and avoid the runtime cost of interpolating nothing.
Same applies to the similar log block around Line 374.dream_layer_backend/grid_exporter.py (1)
13-19: Remove unused imports
jsonandpathlib.Pathare imported but never used – triggers Ruff F401.-import json -from pathlib import Path +# json and Path not used – delete to keep the import list cleandream_layer_backend/test_grid_exporter.py (1)
13-14: Drop unused import
Pathis never referenced – remove to satisfy Ruff F401.dream_layer_frontend/src/components/tabs/ExtrasTab.tsx (2)
29-32: Debugconsole.logleft in production codeRemove or wrap with a debug flag to avoid noisy consoles and potential PII leakage.
50-56: Store numeric fields as numbers, not strings
countandgridSizedefault to string values and are repeatedly coerced withparseInt. Keep them numeric to simplify state handling and avoid accidental NaNs.- const [gridSettings, setGridSettings] = useState({ - count: '4', + const [gridSettings, setGridSettings] = useState({ + count: 4,dream_layer_frontend/src/features/Extras/ExtrasPage.tsx (1)
538-550: Button handler branches are getting complexConsider extracting the “Grid” vs “Upscale” action into two small callbacks and wiring the correct one via a variable (
const onPrimaryAction = activeSubTab==='grid' ? handleCreateGrid : handleGenerate). Keeps JSX tidy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.claude/settings.local.json(1 hunks)dream_layer_backend/extras.py(2 hunks)dream_layer_backend/grid_exporter.py(1 hunks)dream_layer_backend/test_grid_exporter.py(1 hunks)dream_layer_frontend/src/components/tabs/ExtrasTab.tsx(4 hunks)dream_layer_frontend/src/features/Extras/ExtrasPage.tsx(5 hunks)ui_mockup_grid_export.html(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .claude/settings.local.json
- ui_mockup_grid_export.html
🧰 Additional context used
🪛 Ruff (0.12.2)
dream_layer_backend/extras.py
341-341: Do not use bare except
(E722)
344-344: f-string without any placeholders
Remove extraneous f prefix
(F541)
374-374: f-string without any placeholders
Remove extraneous f prefix
(F541)
dream_layer_backend/grid_exporter.py
15-15: json imported but unused
Remove unused import: json
(F401)
18-18: pathlib.Path imported but unused
Remove unused import: pathlib.Path
(F401)
414-414: Do not use bare except
(E722)
dream_layer_backend/test_grid_exporter.py
13-13: pathlib.Path imported but unused
Remove unused import: pathlib.Path
(F401)
191-191: f-string without any placeholders
Remove extraneous f prefix
(F541)
220-220: Local variable grid is assigned to but never used
Remove assignment to unused variable grid
(F841)
🔇 Additional comments (1)
dream_layer_backend/extras.py (1)
351-360: Unused request flags
show_labelsandshow_filenamesare parsed but never forwarded toLabeledGridExporter, so the endpoint ignores the caller’s choices. Pass them through (or drop them from the request contract).Would you like a patch that plumbs these flags into
create_grid_from_recentand toggles label rendering?
| try: | ||
| cols, rows = grid_size_str.split('x') | ||
| grid_size = (int(cols), int(rows)) | ||
| except: | ||
| grid_size = None | ||
|
|
There was a problem hiding this comment.
Replace bare except with a specific exception
Catching all exceptions swallows programming errors and makes debugging harder. Catch ValueError (and optionally TypeError) that int() / split() may raise instead.
- try:
- cols, rows = grid_size_str.split('x')
- grid_size = (int(cols), int(rows))
- except:
- grid_size = None
+ try:
+ cols, rows = grid_size_str.split('x')
+ grid_size = (int(cols), int(rows))
+ except (ValueError, TypeError):
+ grid_size = None📝 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.
| try: | |
| cols, rows = grid_size_str.split('x') | |
| grid_size = (int(cols), int(rows)) | |
| except: | |
| grid_size = None | |
| try: | |
| cols, rows = grid_size_str.split('x') | |
| grid_size = (int(cols), int(rows)) | |
| except (ValueError, TypeError): | |
| grid_size = None |
🧰 Tools
🪛 Ruff (0.12.2)
341-341: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In dream_layer_backend/extras.py around lines 338 to 343, replace the bare
except clause with a specific exception handler. Catch ValueError and optionally
TypeError exceptions that can be raised by the split or int conversion
operations. This will prevent swallowing unrelated exceptions and improve
debugging by only handling expected conversion errors.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
dream_layer_backend/grid_exporter.py (1)
708-708: PNG metadata still not embedded
The save call lacks apnginfoobject, so no metadata is written despite earlier feedback.
🧹 Nitpick comments (3)
dream_layer_backend/grid_exporter.py (3)
81-83: Modernisere.MatchaccessSince Python 3.11
re.Matchimplements__getitem__;match[1]is shorter and marginally faster thanmatch.group(1).
289-295: Avoid magiccell_paddingconstant inside renderer
GridRenderer.render_cell()hard-codescell_padding = 15, drifting fromGridLayoutManager.cell_padding. Pass the padding via arguments or read from the layout manager to keep one source of truth.
402-410: Default font size may clash with 22 px line spacing
ImageFont.load_default()returns a ~10 px bitmap font, yet later you assume 22 px per label line. Consider scaling the fallback font or reducingline_heightto avoid truncated text on systems without TTFs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.claude/settings.local.json(1 hunks)dream_layer_backend/grid_exporter.py(1 hunks)dream_layer_backend/settings.json(1 hunks)dream_layer_backend/test_grid_exporter.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- dream_layer_backend/settings.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/settings.local.json
- dream_layer_backend/test_grid_exporter.py
| if grid_size is not None: | ||
| return grid_size | ||
|
|
||
| # Auto-calculate grid size to be roughly square | ||
| cols = int(np.ceil(np.sqrt(num_images))) | ||
| rows = int(np.ceil(num_images / cols)) | ||
|
|
||
| logger.debug(f"Calculated grid size: {cols}x{rows} for {num_images} images") | ||
| return (cols, rows) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate negative/zero grid_size values
calculate_grid_size() returns the provided tuple verbatim, so (0, n) or (-1, 2) flows through to later canvas maths and raises at runtime. Mirror the guard you already added in create_labeled_grid.
🤖 Prompt for AI Agents
In dream_layer_backend/grid_exporter.py around lines 158 to 166, the function
calculate_grid_size returns the provided grid_size tuple without validating its
values, allowing zero or negative dimensions to cause runtime errors later. Add
validation to check if grid_size is not None and both dimensions are positive
integers; if not, raise a ValueError or handle the invalid input appropriately
before returning grid_size. This mirrors the existing guard in
create_labeled_grid and prevents invalid grid sizes from propagating.
1 participant
Description
Brief description of what this PR does.
Changes Made
Evidence Required ✅
UI Screenshot
<!
Generated Image
Tests (Optional)
Checklist
Summary by Sourcery
Introduce a new LabeledGridExporter module and test suite to generate annotated image grids from recent DreamLayer outputs, complete with metadata labels, CLI support, and frontend integration.
New Features:
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Tests
Chores