set-widget writes the wrong slot on a save whose dynamic-combo option has since gained a sub-input, clobbering control_after_generate
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 57/100
調査の方向性
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 の本文から書いたものです。
説明
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
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
Comfy-Org/comfy-cli のほかの issue
-
test_file_utils umask tests assert against a umask the code never reads (import-time _UMASK), so they only pass on a 0o022 host対応中かも @neverbiasu が 10 日前に担当しました。 オープンbug
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 86/100
Comfy-Org/comfy-cli#907 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
bug
難易度 1/5 1時間未満 初心者へのやさしさ 92/100
Comfy-Org/comfy-cli#781 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
`comfy launch --background` fails with FileNotFoundError when comfy-cli is invoked by path対応中かも @RaphaelFakhri が 42 日前に担当しました。 オープンbug
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
Comfy-Org/comfy-cli#691 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
Comfy-Org/comfy-cli#670 · コメント 2 件 ·
メンテナーはふだん 1 日以内に返信
Comfy-Org/comfy-cli の issue をすべて見る
似ている issue
-
add entry to blockオープン
難易度 1/5 1時間未満 初心者へのやさしさ 65/100
StevenBlack/hosts#3299 ·
-
`garak --report` overwrites the input report when its name has no ".report"対応中かも @0z5a が今日担当しました。 オープンneeds-triage
難易度 2/5 半日 初心者へのやさしさ 72/100
メンテナーはふだん 2 日以内に返信
-
plot_graph crashes when a mesh node has no m2m edge対応中かも @tygvi が今日担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
mllam/neural-lam#883 · コメント 1 件 ·
メンテナーはふだん 2 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 62/100
future-agi/future-agi#4092 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信
-
transcript analytics: a text whose only protected spans the prose pass redacts is withheld wholeオープン
難易度 1/5 1時間未満 初心者へのやさしさ 82/100
WingedGuardian/GENesis-AGI#4641 ·
メンテナーはふだん 1 日以内に返信