set-widget writes the wrong slot on a save whose dynamic-combo option has since gained a sub-input, clobbering control_after_generate

オープン
#1,079 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
57/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
python
領域
backend, cli

調査の方向性

Start with the regression test described in the issue: set-widget seed should change index 7 and preserve "randomize" at index 8. Then read comfy_cli/cql/engine.py, especially _expand_widget_entries and Graph.widget_order_for_node, alongside the saved-order helper and tests from #1069. Done means saved widget markers determine the order used by set-widget, with catalog order as the fallback, and the affected order consumers are covered.

索引モデルが issue の本文から書いたものです。

説明

agent-coded bug effort:medium

Summary

On a workflow saved before a dynamic combo's option gained a sub-input, comfy workflow set-widget writes to the wrong positional slot. The targeted widget keeps its old value and a neighbouring slot is overwritten instead. On a node with a control_after_generate companion the overwritten neighbour is that marker, so the file ends up with a number where a control keyword belongs.

This is on main today and is independent of the conversion-side fix in #1069.

Reproduction

ByteDance2ReferenceNode (Seedance 2.0), whose model option gained an optional auto_upscale after auto_downscale. A save made before that, with its own inputs[].widget markers naming the nine widgets the node actually had:

SAVED = ["Seedance 2.0", "a pirate dances", "720p", "16:9", 5, True, True, 118383194, "randomize", False]
#          model          model.prompt       res     ratio  dur  gen_audio  auto_downscale  seed  control  watermark
#          0              1                  2       3      4    5          6               7     8        9

out, _ = workflow_ops.set_widget(wf, graph, 14, "seed", 999)

Result:

before: ["Seedance 2.0", "a pirate dances", "720p", "16:9", 5, true, true, 118383194, "randomize", false]
after:  ["Seedance 2.0", "a pirate dances", "720p", "16:9", 5, true, true, 118383194,          999, false]
                                                                          ^^^^^^^^^  ^^^
                                                                          seed       control marker
                                                                          unchanged  clobbered

The user asked to set the seed. The seed is untouched at index 7, and the control_after_generate marker at index 8 became 999.

Cause

Graph.widget_order_for_node expands a dynamic combo's selected option using the current catalog, so it produces eleven slots for this ten-value save:

slot 0: model
slot 1: model.prompt
slot 2: model.resolution
slot 3: model.ratio
slot 4: model.duration
slot 5: model.generate_audio
slot 6: model.auto_downscale
slot 7: model.auto_upscale   <-- the save never had this widget
slot 8: seed                 <-- but the file's seed is at index 7
slot 9: control_after_generate
slot 10: watermark

_expand_widget_entries (comfy_cli/cql/engine.py:4550) walks declared ports and, at a dynamic combo, inlines the selected option's sub-inputs from _dynamic_combo_sub_ports. That list is today's option layout. The save's layout is recorded on the node, in its inputs[].widget markers, and nothing reads them: every slot from the inserted sub-input onward is off by one.

The docstring at :4563-4565 says one walk serves both the static catalog and "the value-aware order set-widget indexes by", which is true and is exactly why both are wrong together here.

Why it is quiet

Nothing detects it. widgets_values stays the same length, the written value is type-valid for some slot, and the node still converts and runs. The user sees their seed unchanged and may simply retry. The corrupted control_after_generate surfaces later as a validation error naming a field the user never touched, or as a seed that will not randomize.

Relationship to the conversion fix

#1069 makes the convert path expand these sub-inputs in the order the save records, which is what the frontend does. That fixes reading. It does not change this, because set-widget resolves its index through widget_order_for_node instead.

Worth being precise about the direction, because it is easy to read this as a regression introduced by that PR: the edit path was already writing the wrong slot for such a save, and both paths were wrong together before. #1069 fixes one of them, which makes the other one visible. The two now disagree, and the edit side is the one that is wrong.

Suggested fix

Give _expand_widget_entries the same saved-order input the converter now uses: when the node records inputs[].widget markers, order the selected option's sub-inputs by those markers and omit any the save does not name, falling back to the catalog order when the node records none. That is the same rule and the same evidence as _saved_sub_order in #1069, so the right shape is probably one shared helper both call rather than two implementations of it. Tracked as part of the shared-resolution work in #1077.

Task Decomposition
ID Sub-task Tier Est Depends Acceptance
1 Regression test first: set-widget seed on the save above must change index 7 and leave index 8 as "randomize" routine 20m — Test is red on main
2 Extract the saved-sub-order rule from #1069 into one helper both the converter and _expand_widget_entries call analytical 45m #1069 landing No duplicated rule; both call sites covered by its own tests
3 Thread the node (not just widgets_values) into widget_order_for_node / _expand_widget_entries analytical 45m 2 Signature change absorbed by callers: set-widget, slots, validate, add_node
4 Audit the other consumers of that order for the same off-by-one: editable_widget_names, workflow slots, validate analytical 30m 3 Each either correct or covered by a test
5 Check whether a control_after_generate slot already corrupted by this is repairable on read routine 30m 1 A documented answer; a repair only if it is unambiguous

Task 1 stands alone and is worth landing even if the rest waits, because it pins the bug.

Found while closing out #1069, where I had told a reviewer this would be filed separately.

主要言語
Python
スター
1k
フォーク
176
平均マージ
1日 11時間
マージ済み PR(30日)
87

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

Comfy-Org/comfy-cli のほかの issue

Comfy-Org/comfy-cli の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。