Comfy-Org / Comfy-Org/ComfyUI_frontend

Neither widget serialize flag has any test coverage of its effect (mutation-verified: 2 mutations, 16,590 tests, 0 failures)

Open
#15,727 3 comments 0 reactions 0 assignees View on GitHub
area:testing area:widgets
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

Deleting either widget serialization flag check leaves the full unit suite green. Two mutations, 16,590 tests, 0 failures each.

There are two flags, documented in `src/lib/litegraph/src/types/widgets.ts` since #9105:

| Flag | Controls | Consumer |
| --- | --- | --- |
| `widget.serialize === false` | exclusion from the workflow JSON (`widgets_values`) | `LGraphNode.serialize` / `LGraphNode.configure` |
| `widget.options.serialize === false` | exclusion from the API prompt sent to the backend | `graphToPrompt` (`src/utils/executionUtil.ts:101`) |

Neither one's *effect* is asserted anywhere. `src/extensions/core/uploadAudio.test.ts:288-303` asserts that `AUDIO_UI` sets both flags; nothing asserts what setting them does.

## Reproduction

At `origin/main` `296fc5cd07`, run the full suite once for a baseline, then apply each mutation and run it again.

Baseline: `1 failed | 1204 passed (1205)` test files, `16570 passed | 7 expected fail | 13 skipped (16590)` tests. The one failing file is `scripts/skills/update-ai-attribution.test.ts`, a pre-existing transform error contributing 0 tests. It fails identically in all arms.

**Mutation 1** in `src/utils/executionUtil.ts:101`, stop honouring the prompt flag:

```diff
- if (!widget.name || widget.options?.serialize === false) continue
+ if (!widget.name) continue
```

Result: `16570 passed`, 0 failed. Every button, divider, label and `control_after_generate` value now goes into the API prompt and no test notices.

**Mutation 2** in `src/lib/litegraph/src/LGraphNode.ts:1081`, make the workflow serialiser honour the prompt flag (a change that would drop `control_after_generate` from every `KSampler`'s `widgets_values`):

```diff
- if (widget.serialize === false) continue
+ if (widget.serialize === false || widget.options?.serialize === false)
+ continue
```

Result: `16570 passed`, 0 failed. An on-disk format change for 41 of 834 node classes ships green.

Counts are identical across all three arms (1205 files, 16590 tests), so these are detection failures, not suite drift.

## Why the round-trip tests miss it

Both existing shapes compare a value against itself after a save/load cycle. `browser_tests/tests/customNodes/allNodes.spec.ts:1157-1220` compares `widgets_values` before a reload against `widgets_values` after it. When the writer and the reader change together, that comparison stays consistent and the test passes. This is the same failure mode #15688 documented for its own Mutant B.

## What to add

Four unit tests, all in existing files, no new fixtures needed. Each must be verified to go red under the matching mutation above.

1. `graphToPrompt` **omits** a widget with `options.serialize === false` from `output[nodeId].inputs`, and **includes** its sibling widgets. Goes red under mutation 1.
2. `graphToPrompt` **includes** a widget that has `widget.serialize === false` but no options flag. Pins the two flags as independent, so nobody "unifies" them.
3. `LGraphNode.serialize()` **writes** the value of a widget whose exclusion is options-only. Goes red under mutation 2.
4. `LGraphNode.serialize()` **skips** a widget with a direct `widget.serialize === false`, leaving a hole at that index rather than compacting.

A minimal fixture for 1 and 3 is a node built as `[seed, control_after_generate, steps]` where `control_after_generate` is added via `node.addWidget('combo', ..., { values: [...], serialize: false })`, matching `src/scripts/widgets.ts:144`. Note `node.serialize_widgets = true` is required for `serialize()` to emit `widgets_values` on a bare `LGraphNode`. Expected values at `296fc5cd07`:

```
node.serialize().widgets_values -> [42, 'fixed', 20]
(await graphToPrompt(graph)).output['1'].inputs -> { seed: 42, steps: 20 }
```

## Why this matters now

The distinction is easy to misread as a bug. An audit in progress read the workflow serialiser ignoring `options.serialize` as "the option is silently ignored repo-wide" and drafted a fix to honour it. That fix is mutation 2. It merges green, and before the read-side fix in #15688 it shifts every widget after index 0 on every saved `KSampler` on load.

Two custom-node packs in a 29-pack corpus (9 sites: rgthree-comfy seed buttons, dividers and labels; ComfyUI-KJNodes canvas editors) pass `serialize: false` in options and depend on the prompt exclusion working. Zero packs in that corpus use the direct form.

## Scope

Test-only. No production behaviour should change. If a test cannot be made to pass without changing `executionUtil.ts` or `LGraphNode.ts`, that is a finding to report on this issue, not something to fix here.

Contributor guide

Open the contributing guide

Research direction

Read src/utils/executionUtil.ts and src/lib/litegraph/src/LGraphNode.ts, then locate their existing unit tests and run the relevant tests for graphToPrompt and LGraphNode.serialize. Add the four specified assertions using the minimal widget fixture, and verify each test turns red under its matching mutation while the expected prompt and widgets_values remain correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.