Skip to content

fix(layout): reserve image preview height - #925

Open
christian-byrne wants to merge 2 commits into
mainfrom
christian-byrne/pm-1607-image-preview-layout
Open

christian-byrne wants to merge 2 commits into
mainfrom
christian-byrne/pm-1607-image-preview-layout

Conversation

@christian-byrne

Copy link
Copy Markdown
Contributor

Summary

  • reserve the frontend's 190px first-render image-preview host when sizing image-upload nodes
  • persist the same preview-aware size used by batch placement
  • cover the reported five LoadImageSaveImage rows with a no-overlap regression

The PM-1607/PM-1608 artifact shows the SaveImage column remaining clean while each populated LoadImage preview grows into the row below it. The previous model counted the image combo and upload button but not the DOM preview host added by createImageHost, so it persisted a roughly 102px body for a loader that renders at roughly 292px once populated.

Validation

  • uv run pytest tests/comfy_cli/test_layout.py tests/comfy_cli/test_layout_quality.py tests/comfy_cli/test_persisted_node_size.py tests/comfy_cli/command/test_workflow_edit.py -q — 235 passed
  • uv run ruff check comfy_cli/layout.py comfy_cli/workflow_ops.py tests/comfy_cli/test_persisted_node_size.py
  • uv run ruff format --check comfy_cli/layout.py comfy_cli/workflow_ops.py tests/comfy_cli/test_persisted_node_size.py
  • full suite: 7,817 passed, 32 skipped, 47 unrelated baseline failures (the same filesystem/preflight cluster observed on clean PM-1443 validation)

Linear: PM-1607

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 9f9a26bb-a403-45d9-b865-3436c958401d

📥 Commits

Reviewing files that changed from the base of the PR and between 64026d2 and 0e0ee2a.

📒 Files selected for processing (3)
  • comfy_cli/layout.py
  • comfy_cli/workflow_ops.py
  • tests/comfy_cli/test_persisted_node_size.py

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Image-upload widgets now add preview height to node size estimates. New and persisted nodes pass detected preview counts into layout sizing. Regression coverage verifies populated image-loader nodes reserve space and do not overlap.

Changes

Image Preview Layout

Layer / File(s) Summary
Preview-aware size estimation
comfy_cli/layout.py
The layout module defines the image-preview minimum height, counts upload-backed widgets, and adds 190px per detected preview to node height estimates.
Node layout integration and regression coverage
comfy_cli/layout.py, comfy_cli/workflow_ops.py, tests/comfy_cli/test_persisted_node_size.py
Position assignment and add_node pass image-preview counts into size estimation. Tests add image-node metadata and verify five populated loader rows reserve preview height without overlap.

Priority: ⬇️ Low

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 0e0ee

Image-upload nodes now reserve space for rendered previews, preventing row overlap while preserving existing sizing behavior. The change is ready to merge.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR

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

@coderabbitai
coderabbitai Bot requested a review from skishore23 September 22, 2026 11:04

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant