Skip to content

Main#13058

Closed
kristentr wants to merge 6 commits intoComfy-Org:masterfrom
kristentr:main
Closed

Main#13058
kristentr wants to merge 6 commits intoComfy-Org:masterfrom
kristentr:main

Conversation

@kristentr
Copy link
Copy Markdown

@kristentr kristentr commented Mar 18, 2026 •
edited by github-actions bot
Loading

API Node PR Checklist

Scope

  • Is API Node Change

Pricing & Billing

  • Need pricing update
  • No pricing update

If Need pricing update:

  • Metronome rate cards updated
  • Auto‑billing tests updated and passing

QA

  • QA done
  • QA not required

Comms

  • Informed Kosinkadink

christian-byrne and others added 6 commits February 27, 2026 17:12
@christian-byrne
Add prompt_id to progress_text binary WS messages
1c7e656 
Add supports_progress_text_metadata feature flag and extend
send_progress_text() to accept optional prompt_id param. When
prompt_id is provided and the client supports the new format,
the binary wire format includes a length-prefixed prompt_id field:

  [4B event_type][4B prompt_id_len][prompt_id][4B node_id_len][node_id][text]

Legacy format preserved for clients without the flag.

Both callers (nodes_images.py, client.py) updated to pass prompt_id
from get_executing_context().

Part of COM-12671: parallel workflow execution support.

Amp-Thread-ID: https://ampcode.com/threads/T-019c79f7-f19b-70d9-b662-0687cc206282
@christian-byrne
refactor: add prompt_id as hidden type, fix imports, add docstrings
83df2a8 
- Add PROMPT_ID as a new hidden type in the Hidden enum, HiddenHolder,
  HiddenInputTypeDict, and execution engine resolution (both V3 and legacy)
- Refactor GetImageSize to use cls.hidden.prompt_id instead of manually
  calling get_executing_context() — addresses reviewer feedback
- Remove lazy import of get_executing_context from nodes_images.py
- Add docstrings to send_progress_text, _display_text, HiddenHolder,
  and HiddenHolder.from_dict

Amp-Thread-ID: https://ampcode.com/threads/T-019ca1cb-0150-7549-8b1b-6713060d3408
@christian-byrne
fix: send_progress_text unicasts to client_id instead of broadcasting
d74dfd2 
- Default sid to self.client_id when not explicitly provided, matching
  every other WS message dispatch (executing, executed, progress_state, etc.)
- Previously sid=None caused broadcast to all connected clients
- Format signature per ruff, remove redundant comments
- Add unit tests for routing, legacy format, and new prompt_id format

Amp-Thread-ID: https://ampcode.com/threads/T-019ca3ce-c530-75dd-8d68-349e745a022e
@christian-byrne
remove send_progress_text stub tests
09e9bdb 
Copy-paste stub tests don't verify the real implementation and add
maintenance burden without meaningful coverage.

Amp-Thread-ID: https://ampcode.com/threads/T-019ca3ce-c530-75dd-8d68-349e745a022e
@christian-byrne
fix: always send new binary format when client supports feature flag
69d3bfa 
When prompt_id is None, encode as zero-length string instead of falling
back to old format. Prevents binary parse corruption on the frontend.

Addresses review feedback:
Comfy-Org#12540 (comment)
@kristentr
Merge pull request #1 from Comfy-Org/progress-text-prompt-id
95de83f 
Progress text prompt
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cb355254-02de-459b-b104-f5152f095900

📥 Commits

Reviewing files that changed from the base of the PR and between 56ff88f and 95de83f.

📒 Files selected for processing (7)
  • comfy/comfy_types/node_typing.py
  • comfy_api/feature_flags.py
  • comfy_api/latest/_io.py
  • comfy_api_nodes/util/client.py
  • comfy_extras/nodes_images.py
  • execution.py
  • server.py
📝 Walkthrough

Walkthrough

This pull request introduces prompt ID tracking across the Comfy system. Changes include adding a prompt_id field to the HiddenInputTypeDict type definition, creating a corresponding Hidden.prompt_id enum member in the I/O layer, and extending the HiddenHolder class to accept and store the prompt ID. The get_input_data function is updated to accept and thread the prompt ID through the execution pipeline. A new server feature flag supports_progress_text_metadata is added to control progress text message framing. The send_progress_text method is enhanced to optionally include prompt ID in its message format when the feature flag is enabled. Node implementations like GetImageSize are updated to pass prompt ID to progress text dispatches via the updated execution context retrieval mechanism.

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description contains only a template checklist unrelated to the changeset itself; it does not explain what the pull request actually implements. Replace the checklist template with a meaningful description of the changes, including the purpose of adding prompt_id support and how it impacts progress tracking.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Main' is generic and does not convey the actual changeset content; it lacks meaningful information about the pull request's purpose. Revise the title to reflect the actual changes, such as 'Add prompt_id support for progress tracking across nodes' or similar.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

Tip

CodeRabbit can enforce grammar and style rules using `languagetool`.

Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.

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

Reviewers

@comfyanonymous comfyanonymous Awaiting requested review from comfyanonymous comfyanonymous is a code owner

@Kosinkadink Kosinkadink Awaiting requested review from Kosinkadink Kosinkadink is a code owner

@guill guill Awaiting requested review from guill guill is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kristentr @Kosinkadink @christian-byrne