Skip to content

fix(jobs): prefer media over text for job preview_output#14681

Open
DeniDoman wants to merge 8 commits into
Comfy-Org:masterfrom
DeniDoman:fix/jobs-preview-prefer-media-over-text
Open

fix(jobs): prefer media over text for job preview_output#14681
DeniDoman wants to merge 8 commits into
Comfy-Org:masterfrom
DeniDoman:fix/jobs-preview-prefer-media-over-text

Conversation

@DeniDoman

Copy link
Copy Markdown

Addresses #14680

get_outputs_summary shared a single fallback slot between text and
non-output media, so a text node iterated before an image (e.g. a temp
preview) claimed the preview and masked the image, surfacing a text
cover for mixed image+text jobs. Give text its own lowest-priority slot
so a media thumbnail always wins; text is used only when the job
produced no image/video/audio/3d output.

Adds coverage for the previously-untested text preview path.
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3dbdcb75-76e8-428b-911a-a43e2ee0ae8c

📥 Commits

Reviewing files that changed from the base of the PR and between 28890f0 and 0f6ab9c.

📒 Files selected for processing (2)
  • comfy_execution/jobs.py
  • tests/execution/test_jobs.py
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Keep changes small, direct, and limited to the narrowest code path and fewest files necessary; prefer practical fixes over broad architectural work.
Prefer fewer dependencies and do not add a ComfyUI dependency unless absolutely necessary.
Remove obsolete code, dead branches, unused options, debug prints, and unnecessary compatibility paths.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicit.
Keep architectural layers focused; do not leak UI, API, workflow, queue, persistence, telemetry, model-loading, node, or execution concerns into unrelated layers.
Shared core modules should depend only on lower-level primitives and their own domain concepts; higher-level concepts belong at callers, adapters, services, or boundaries.
Pass only the narrowest data needed across boundaries and keep identity mapping, persistence, history, telemetry, response shaping, and UI state in their owning layers.
Before touching many files, identify the smallest owner layer; use caller-side mappings, adapters, events, or narrow interfaces instead of exposing private concepts across layers.
Core ComfyUI code must not make unsolicited internet requests or add uploads, telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or similar outbound paths.
Model downloading is allowed only when explicitly authorized by the user, limited to the requested artifact, and free of telemetry, tracking, unrelated metadata, or background activity.
Warning and info messages should be short and actionable; documentation and README changes should be concise, factual, and tied to changed behavior.
Use short direct commit subjects such as Fix ..., Add ..., Support ..., Remove ..., or Update ...; keep PR descriptions short and state the problem, behavior change, and tests.
Prefer one coherent behavioral change per commit and prioritize crashes, incorrect dtype/device behavior, m...

Files:

  • tests/execution/test_jobs.py
  • comfy_execution/jobs.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Keep state and capability flags on the object that owns the behavior; use explicit parent-owned fields rather than probing children with getattr for parent control flow.
Keep public methods aligned with caller contracts; preserve arguments, parameter order, return types, side effects, and error behavior unless all affected interfaces are intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers.
Normalize third-party and upstream return conventions at integration boundaries so core code receives the expected type and shape.
Avoid caller-side unwrapping such as out = out[0] unless the called interface documents that return structure.
Do not add torch.no_grad, torch.inference_mode, or inference-mode wrappers; only disable a globally enabled inference mode when a training path requires gradients.
Do not add freeze, unfreeze, or trainability toggles to ComfyUI model classes.
Remove training-only behavior such as dropout from inference models while preserving checkpoint and state-dict compatibility, using nn.Identity when necessary to retain slots.
Keep imports at module scope; use inline imports only for established optional-backend probes or import-cycle avoidance.
Use try/except only for optional dependency, platform, or backend detection with a useful fallback, and prefer specific exception types.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, quantization metadata, and bad states should fail clearly.
Match local file style and keep comments sparse, useful, and non-obvious; remove comments that merely restate code.

Files:

  • tests/execution/test_jobs.py
  • comfy_execution/jobs.py
**

⚙️ CodeRabbit configuration file

**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing a with: block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.

Files:

  • tests/execution/test_jobs.py
  • comfy_execution/jobs.py
comfy_execution/**

⚙️ CodeRabbit configuration file

comfy_execution/**: Execution engine (graph execution, caching, jobs). Focus on:

  • Caching correctness
  • Concurrent execution safety
  • Graph validation edge cases

Files:

  • comfy_execution/jobs.py
🧠 Learnings (1)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.

Applied to files:

  • tests/execution/test_jobs.py
  • comfy_execution/jobs.py
🔇 Additional comments (2)
comfy_execution/jobs.py (1)

173-185: LGTM!

Also applies to: 275-291, 319-320, 337-345

tests/execution/test_jobs.py (1)

283-305: LGTM!

Also applies to: 306-332, 333-362


📝 Walkthrough

Walkthrough

In comfy_execution/jobs.py, get_outputs_summary classifies text previews and separates saved text-file and raw-text fallbacks. Preview selection now prioritizes visual media, other previewable media, saved text files, and raw text. Tests cover these priorities and verify output counting for media, saved files, and raw text metadata.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preferring media over text for job preview output.
Description check ✅ Passed The description is related to the change because it references the issue this preview-priority fix addresses.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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

❤️ Share

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

@DeniDoman

Copy link
Copy Markdown
Author

So, what should be done to merge it?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@comfy_execution/jobs.py`:
- Around line 269-271: Update the output-counting logic in the job processing
flow so entries with media_type == 'text' increment outputs_count before the
preview-only continue. Preserve the existing text fallback-preview behavior
while ensuring text-only jobs count as 1 and mixed text/media jobs include text
entries in the total.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6af74e1d-738c-4ef9-8269-fa5305475e22

📥 Commits

Reviewing files that changed from the base of the PR and between 9a5d582 and aa6dbb8.

📒 Files selected for processing (1)
  • comfy_execution/jobs.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Keep imports at module scope; avoid inline imports unless they are already part of an established optional-backend probe or are needed to avoid an import cycle.
Do not add unnecessary try/except blocks; use them for optional dependency, platform, or backend capability detection only when the program has a useful fallback, and prefer specific exception types when changing new code.
If a library version is pinned in requirements.txt, do not add code to ComfyUI to handle older versions of that library.
Remove any workarounds for PyTorch versions that ComfyUI no longer officially supports; deprecated workarounds include catching an exception and rerunning the same op with the input cast to float unless the code comments name the exact supported PyTorch versions.
Let unsupported model formats, invalid quantization metadata, and bad states fail with clear errors instead of silently producing lower quality output.
Match the existing local style in the file you edit; long lines, simple helper functions, module-level state, and direct tensor operations are acceptable when they make the code easier to follow.
Keep comments sparse and useful; strip comments that restate the code or describe obvious behavior, and keep short TODOs only when they name the concrete missing follow-up.
Treat dtype, device placement, VRAM usage, and offloading behavior as core correctness concerns; check CPU, CUDA, ROCm, MPS, DirectML, XPU, NPU, and low-VRAM implications when touching shared execution or loading code.
Prefer native ComfyUI formats and existing quantization/offload helpers over adding parallel code paths; use comfy.quant_ops, comfy.model_management, comfy.memory_management, comfy.pinned_memory, comfy_aimdo, and comfy-kitchen helpers where they already solve the problem.
Use optimized comfy-kitchen ops in places where they improve performance without changing the expected dtype, device, memory, or interface behavior.
All models should use the optimized atte...

Files:

  • comfy_execution/jobs.py
**

⚙️ CodeRabbit configuration file

**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing a with: block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.

Files:

  • comfy_execution/jobs.py
comfy_execution/**

⚙️ CodeRabbit configuration file

comfy_execution/**: Execution engine (graph execution, caching, jobs). Focus on:

  • Caching correctness
  • Concurrent execution safety
  • Graph validation edge cases

Files:

  • comfy_execution/jobs.py
🧠 Learnings (1)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.

Applied to files:

  • comfy_execution/jobs.py
🔇 Additional comments (2)
comfy_execution/jobs.py (2)

59-60: LGTM!

Also applies to: 163-168


262-267: LGTM!

Also applies to: 327-327

Comment thread comfy_execution/jobs.py
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 15, 2026
@alexisrolland

Copy link
Copy Markdown
Member

Hi @DeniDoman thanks for pushing this. I'll give it a test and will merge if everything works fine.

…ntics

The master merge stopped counting text entries as outputs (they are
preview-only metadata). Update the two preview-priority tests so their
count assertions match; the media-over-text preview assertions are
unchanged.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 15, 2026
@DeniDoman

Copy link
Copy Markdown
Author

@alexisrolland awesome!

I have another annoying bug fix in the Frontend part, it would be also nice to merge it!
Comfy-Org/ComfyUI_frontend#13294

@alexisrolland

alexisrolland commented Jul 16, 2026

Copy link
Copy Markdown
Member

@DeniDoman this does not seem to work as intended, in all situation, I can still see text output in the preview in the asset panel. You can reproduce with the following:

  • Blank canvas
  • Add Text String (Multiline) -> Save Text
  • Run workflow
  • Add Load Image -> Save Image (Advanced)
  • Run workflow

Second run is KO and still shows the text as preview, probably because of some caching mechanism.

{F0ACE938-0BA2-47A2-A3C5-FAC4169F51E7}

@DeniDoman

DeniDoman commented Jul 16, 2026

Copy link
Copy Markdown
Author

@alexisrolland oh, sorry, lack of testing, I was too focused with "preview" so forgot to verify the "save" flow.

The root cause turned out to be one level deeper than my original fix. "Text" exists in two shapes in job outputs: raw strings under text (preview-only metadata since #14102), and the real .txt/.md/.json files SaveText writes under files. My first commit only demoted the raw strings. The saved file has type='output' and is previewable via its extension, so it landed in the top priority slot and beat every image - even a SaveImage, depending on node order.

The priority is now: saved visual media → any visual media (incl. temp previews) → saved text file → raw text.

I also added a couple more tests. Thank you for the reviewing!

P.S. I already signed CLA, but seems it's failed, so:

I have read and agree to the Contributor License Agreement

@DeniDoman

Copy link
Copy Markdown
Author

@alexisrolland could you please look one more time?

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.

2 participants