Skip to content

fix: suppress control_after_generate when widget input is linked#10219

Open
artokun wants to merge 8 commits intoComfy-Org:mainfrom
artokun:fix/subgraph-promoted-control-after-generate
Open

fix: suppress control_after_generate when widget input is linked#10219
artokun wants to merge 8 commits intoComfy-Org:mainfrom
artokun:fix/subgraph-promoted-control-after-generate

Conversation

@artokun
Copy link
Contributor

@artokun artokun commented Mar 17, 2026

Summary

  • When an external node is connected to a SubgraphNode input, suppress the interior's control_after_generate combo widget on the promoted view
  • The external node owns the value, so the interior control should not be active or visible

Problem

Promoted subgraph widgets (Int, seed, float) expose the interior node's control_after_generate widget regardless of connection state. When an external node drives the input, the user sees two independent controls — one on the external node and one on the promoted widget — with potentially conflicting modes (e.g. external is "fixed" but promoted shows "randomize").

Solution

Override linkedWidgets on PromotedWidgetView to return undefined when the corresponding SubgraphNode input has an active link. This hides the control combo from the promoted widget when the value is externally driven.

Video of Fix

Screen.Recording.2026-03-17.at.4.15.30.PM.mov

Test plan

┆Issue is synchronized with this Notion page by Unito

When an external node is connected to a SubgraphNode input, the
promoted widget's control_after_generate combo should not be active
since the external node owns the value. The interior node's control
widget was being exposed via linkedWidgets delegation regardless of
connection state.

Fixes Comfy-Org#10218
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

Adds detection for widgets whose values are externally driven (including subgraph-promoted inputs), prevents applying control widgets in those cases, updates Vue widget rendering to omit controlWidget for linked slots, and adds unit and browser tests validating control_after_generate behavior across node and subgraph scenarios.

Changes

Cohort / File(s) Summary
Core logic
src/scripts/widgets.ts
New isControlExternallyDriven(node, targetWidget) + SUBGRAPH_INPUT_ID constant; integrate check into widget-control application flow to skip applying control widgets when value is externally driven.
Renderer
src/renderer/extensions/vueNodes/components/NodeWidgets.vue
When a widget's slotMetadata indicates a linked slot, set the SimplifiedWidget's controlWidget to undefined to avoid rendering control for externally-driven inputs.
Unit tests
src/scripts/widgets.test.ts
New Vitest tests for isControlExternallyDriven() covering no-link, direct link, subgraph interior with external link, and subgraph interior without external link scenarios; includes Pinia setup and fixture resets.
Browser tests
browser_tests/tests/subgraphControlAfterGenerate.spec.ts
New Playwright tests validating control_after_generate behavior when an Int node is linked to a target and after the link is disconnected (value stability while linked, increment after disconnect).

Sequence Diagram(s)

sequenceDiagram
  participant UI as UI (Widget)
  participant Node as Node
  participant Subgraph as SubgraphNode
  participant Graph as Graph

  UI->>Node: render widget / request control applicability
  Node->>Graph: check input links for targetWidget
  Graph-->>Node: link info (direct or via SubgraphInput)
  alt link originates from SubgraphInput
    Node->>Subgraph: traverse to parent node / check external linkage
    Subgraph-->>Node: external link found / not found
  end
  Node-->>UI: isControlExternallyDriven = true|false
  alt externally driven
    UI-->>UI: omit controlWidget (undefined)
  else not externally driven
    UI-->>UI: attach controlWidget (apply control_after_generate)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniffed the links both near and far,
Found widgets led by another star,
I hid the knobs where wires prevail,
Then wrote some tests to tell the tale,
Now promoted inputs behave on par. ✨

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: suppressing control_after_generate when widget input is linked, which is the core objective of the PR.
Linked Issues check ✅ Passed The PR fully addresses issue #10218 objectives: suppresses control_after_generate UI and execution when widget input is linked, handles subgraph interior node link tracing, and includes comprehensive unit and E2E test coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the control_after_generate suppression issue: NodeWidgets.vue UI changes, widgets.ts execution logic, browser tests, and unit tests with no unrelated modifications.
End-To-End Regression Coverage For Fixes ✅ Passed PR includes new browser test file under browser_tests/ directory satisfying check condition 2.
Description check ✅ Passed The PR description covers all key aspects: clear problem statement, solution approach, test plan (both unit and manual), link to related issue, and supporting video documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

🎨 Storybook: loading Building...

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

🎭 Playwright: ⏳ Running...

@artokun artokun marked this pull request as draft March 17, 2026 22:26
Hide the control widget UI (NodeWidgets.vue) and skip its execution
(isControlExternallyDriven in widgets.ts) when the widget's input has
an active link. For subgraph interior nodes, traces the link chain
through SubgraphInput to the SubgraphNode to detect external links.

Fixes Comfy-Org#10218
@artokun artokun changed the title fix: suppress control_after_generate on linked promoted widgets fix: suppress control_after_generate when widget input is linked Mar 17, 2026
@artokun artokun marked this pull request as ready for review March 17, 2026 23:23
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@browser_tests/tests/subgraphControlAfterGenerate.spec.ts`:
- Around line 50-53: The current guard uses test.skip when setup is falsy which
masks failures; replace the test.skip(...) branches with an immediate test
failure by throwing an Error (e.g., throw new Error('Could not create Int
nodes') ) or calling the test runner's fail mechanism so the test fails fast;
update both occurrences that check the setup variable (the blocks using "if
(!setup) { test.skip(...)" ) to throw an Error with a clear message instead of
skipping.
- Around line 39-43: The test currently finds controlWidget via
targetWidget?.linkedWidgets?.find(...) and silently skips configuration if it's
missing; update the test to assert that controlWidget exists before mutating it
(e.g., throw or use your test framework's expect) so the test fails if the
'control_after_generate' linked widget isn't present; apply the same presence
assertion and configuration for the other occurrence around the code referenced
at lines 111-115. Ensure you reference the variables controlWidget,
targetWidget, linkedWidgets and the widget name 'control_after_generate' when
adding the assertion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8758c4a1-5bca-4a2b-847f-5e936520eef0

📥 Commits

Reviewing files that changed from the base of the PR and between 3bd8963 and 778e6bc.

📒 Files selected for processing (4)
  • browser_tests/tests/subgraphControlAfterGenerate.spec.ts
  • src/renderer/extensions/vueNodes/components/NodeWidgets.vue
  • src/scripts/widgets.test.ts
  • src/scripts/widgets.ts

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 18, 2026
@artokun artokun requested a review from a team March 18, 2026 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promoted subgraph widget doesn't mirror control_after_generate from source

1 participant