From 19dc48b69abc452549fe5b99d71430d63ed3a848 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 23 Feb 2026 10:47:11 -0800 Subject: [PATCH] docs: document PrimitiveNode copy/paste semantics and widgets_values loss (#9119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Documents the PrimitiveNode copy/paste bug mechanism and connection lifecycle semantics in `WIDGET_SERIALIZATION.md`. This is tribal knowledge from debugging [#1757](https://github.com/Comfy-Org/ComfyUI_frontend/issues/1757) and the related [Slack discussion](https://comfy-organization.slack.com/archives/C09AQRB49QX/p1771806268469089). ## What's documented - **The clone→serialize gap**: `_serializeItems()` calls `item.clone()?.serialize()`. The clone has no `this.widgets` (PrimitiveNode creates them on connection), so `serialize()` silently drops `widgets_values`. - **Why seed survives but control_after_generate doesn't**: Primary widget value is copied from the target on reconnect; secondary widgets read from `this.widgets_values` which was lost. - **Current vs. proposed lifecycle**: Empty-on-copy → morph-on-connect (current) vs. clone-configured-instance → empty-on-disconnect (proposed). - **Design considerations**: `input.widget` override flexibility, deserialization ordering, and the minimal `serialize()` override fix. ## Related - Issue: #1757 - Fix PR: #8938 - Companion: #9102 (initial WIDGET_SERIALIZATION.md), #9105 (type/JSDoc improvements) - Notion: COM-15282 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9119-docs-document-PrimitiveNode-copy-paste-semantics-and-widgets_values-loss-3106d73d3650816ba7f7d9e6f3bb3868) by [Unito](https://www.unito.io) --- docs/WIDGET_SERIALIZATION.md | 25 +++++++++++ ...006-primitive-node-copy-paste-lifecycle.md | 45 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 docs/adr/0006-primitive-node-copy-paste-lifecycle.md diff --git a/docs/WIDGET_SERIALIZATION.md b/docs/WIDGET_SERIALIZATION.md index a1adf4290..2fd89d1db 100644 --- a/docs/WIDGET_SERIALIZATION.md +++ b/docs/WIDGET_SERIALIZATION.md @@ -24,6 +24,28 @@ These correspond to the two data formats in `ComfyMetadata` embedded in output f - `LGraphNode.serialize()` only writes `widgets_values` if `this.widgets` is truthy. Nodes that create widgets dynamically (like `PrimitiveNode`) will have no `widgets_values` in serialized output if serialized before widget creation — even if `this.widgets_values` exists on the instance from a prior `configure()` call. - `widget.options.serialize` is typed as `IWidgetOptions.serialize` — both properties share the name `serialize` but live at different levels of the widget object. +## PrimitiveNode and copy/paste + +`PrimitiveNode` creates widgets dynamically on connection — it starts as an empty polymorphic node and morphs to match its target widget in `_onFirstConnection()`. This interacts badly with the copy/paste pipeline. + +### The clone→serialize gap + +`LGraphCanvas._serializeItems()` copies nodes via `item.clone()?.serialize()` (line 3911). For PrimitiveNode this fails: + +1. `clone()` calls `this.serialize()` on the **original** node (which has widgets, so `widgets_values` is captured correctly). +2. `clone()` creates a **fresh** PrimitiveNode via `LiteGraph.createNode()` and calls `configure(data)` on it — this stores `widgets_values` on the instance. +3. But the fresh PrimitiveNode has no `this.widgets` (widgets are created only on connection), so when `serialize()` is called on the clone, `LGraphNode.serialize()` skips the `widgets_values` block entirely (line 964: `if (widgets && this.serialize_widgets)`). + +Result: `widgets_values` is silently dropped from the clipboard data. + +### Why seed survives but control_after_generate doesn't + +When the pasted PrimitiveNode reconnects to the pasted target node, `_createWidget()` copies `theirWidget.value` from the target (line 254). This restores the **primary** widget value (e.g., `seed`). + +But `control_after_generate` is a **secondary** widget created by `addValueControlWidgets()`, which reads its initial value from `this.widgets_values?.[1]` (line 263). That value was lost during clone→serialize, so it falls back to `'fixed'` (line 265). + +See [ADR-0006](adr/0006-primitive-node-copy-paste-lifecycle.md) for proposed fixes and design tradeoffs. + ## Code references - `widget.serialize` checked: `src/lib/litegraph/src/LGraphNode.ts` serialize() and configure() @@ -31,3 +53,6 @@ These correspond to the two data formats in `ComfyMetadata` embedded in output f - `widget.options.serialize` set: `src/scripts/widgets.ts` addValueControlWidgets() - `widget.serialize` set: `src/composables/node/useNodeImage.ts`, `src/extensions/core/previewAny.ts`, etc. - Metadata types: `src/types/metadataTypes.ts` +- PrimitiveNode: `src/extensions/core/widgetInputs.ts` +- Copy/paste serialization: `src/lib/litegraph/src/LGraphCanvas.ts` `_serializeItems()` +- Clone: `src/lib/litegraph/src/LGraphNode.ts` `clone()` diff --git a/docs/adr/0006-primitive-node-copy-paste-lifecycle.md b/docs/adr/0006-primitive-node-copy-paste-lifecycle.md new file mode 100644 index 000000000..e8025d910 --- /dev/null +++ b/docs/adr/0006-primitive-node-copy-paste-lifecycle.md @@ -0,0 +1,45 @@ +# 6. PrimitiveNode Copy/Paste Lifecycle + +Date: 2026-02-22 + +## Status + +Proposed + +## Context + +PrimitiveNode creates widgets dynamically on connection. When copied, the clone has no `this.widgets`, so `LGraphNode.serialize()` drops `widgets_values` from the clipboard data. This causes secondary widget values (e.g., `control_after_generate`) to be lost on paste. See [WIDGET_SERIALIZATION.md](../WIDGET_SERIALIZATION.md#primitiveno-and-copypaste) for the full mechanism. + +Related: [#1757](https://github.com/Comfy-Org/ComfyUI_frontend/issues/1757), [#8938](https://github.com/Comfy-Org/ComfyUI_frontend/pull/8938) + +## Options + +### A. Minimal fix: override `serialize()` on PrimitiveNode + +Override `serialize()` to fall back to `this.widgets_values` (set during `configure()`) when the base implementation omits it due to missing `this.widgets`. + +- **Pro**: No change to connection lifecycle semantics. Lowest risk. +- **Pro**: Doesn't affect workflow save/load (which already works via `onAfterGraphConfigured`). +- **Con**: Doesn't address the deeper design issue — primitives are still empty on copy. + +### B. Clone-configured-instance lifecycle + +On copy, the primitive is a clone of the configured instance (with widgets intact). On disconnect or paste without connections, it returns to empty state. + +- **Pro**: Copy→serialize captures `widgets_values` correctly. Matches OOP expectations. +- **Pro**: Secondary widget state survives round-trips without special-casing. +- **Con**: `input.widget[CONFIG]` allows extensions to make PrimitiveNode create a _different_ widget than the target. Widget config is derived at connection time, not stored, so cloning the configured state may not be faithful. +- **Con**: Deserialization ordering — `configure()` runs before links are restored. PrimitiveNode needs links to know what widgets to create. `onAfterGraphConfigured()` handles this for workflow load, but copy/paste uses a different code path. +- **Con**: Higher risk of regressions in extension compatibility. + +### C. Projection model (like Subgraph widgets) + +Primitives act as a synchronization mechanism — no own state, just a projection of the target widget's resolved value. + +- **Pro**: Cleanest conceptual model. Eliminates state duplication. +- **Con**: Primitives can connect to multiple targets. Projection with multiple targets is ambiguous. +- **Con**: Major architectural change with broad impact. + +## Decision + +Pending. Option A is the most pragmatic first step. Option B can be revisited after Option A ships and stabilizes.