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

Aperta
  • Qualcosa non va?
#1,079 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
57/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
python
Ambito
backend, cli

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.

Lingua principale
Python
Stelle
1k
Fork
176
Merge medio
1g 11h
PR unite (30g)
87

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di Comfy-Org/comfy-cli

Tutte le issue di Comfy-Org/comfy-cli

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.