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

Abierto
  • ¿Algo no está bien?
#1,079 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
57/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python
Área
backend, cli

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Python
Estrellas
1k
Forks
176
Merge medio
1 d 11 h
PR fusionados (30 d)
87

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de Comfy-Org/comfy-cli

Todos los issues de Comfy-Org/comfy-cli

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.