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

Đang mở
  • Có gì không ổn?
#1,079 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
57/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
python
Lĩnh vực
backend, cli

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
Python
Star
1k
Fork
176
Merge trung bình
1 ngày 11 giờ
Pull request đã merge (30 ngày)
87

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của Comfy-Org/comfy-cli

Tất cả issue của Comfy-Org/comfy-cli

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.