fix: accept the set-widget addresses the agent derives - #919
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review paused — included plan limit reachedKeep your review moving with free on-demand reviews.
On-demand reviews are free for the next 18 days.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Promotion and pricing detailsOn-demand reviews are free for the next 18 days. After that, they cost $0.25 per reviewed file. Review limit detailsOr wait 8 minutes for your next included review. Limit details: You’ve used all 6 included reviews currently available. Your 40 included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (7)
Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour. 📝 WalkthroughWalkthroughThe pull request adds model-specific workflow emission, implied flag handling, and Gemini image-node support. It also adds print-binding fallback and legacy primitive-node address resolution for widget updates, with regression coverage for both change sets. ChangesWorkflow generation
Widget address resolution
Sequence Diagram(s)sequenceDiagram
participant CLI
participant generate_workflow
participant NodeSpec
participant GeminiImage2Node
CLI->>generate_workflow: request workflow with model
generate_workflow->>NodeSpec: select matching model variant
NodeSpec->>GeminiImage2Node: apply variant inputs and defaults
GeminiImage2Node-->>CLI: emit workflow node
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This change adds model-specific workflow emission and broader widget-address support while preserving rejection of unsupported values; it is ready to merge. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
369c5f5 to
a1707af
Compare
|
Release split required: isolate address edits; preserve adapter work separately. Full context for agent readersThe release-critical minimum is only:
Please keep those changes in an existing-owner carrier and move/preserve the unrelated Nano Banana and Flux adapter work ( After the split, return both exact heads and focused/full check receipts. The edit-only carrier also needs substantive review before inclusion. |
- a print_workflow binding key used as the node of a set-widget address - a direct write to a legacy PrimitiveNode's value widget Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
b840140 to
46f15ba
Compare
skishore23
changed the title
- set-widget resolves a print_workflow binding key to the address it maps to, after the literal address fails (a real node id always wins). - A top-level legacy PrimitiveNode takes a write on its single widget, addressed as `value` or by its output marker's name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@christian-byrne split done.
|
| @@ -636,9 +636,35 @@ def set_widget( | |||
| try: | |||
| return _set_widget_impl(workflow, graph, node_id, widget, value, actor=actor, base_version=base_version) | |||
| except ValueError as e: | |||
There was a problem hiding this comment.
P1: Restrict fallback to address-resolution failures; literal write errors must not redirect.
Full context for agent readers
The wrapper catches every ValueError, including a literal node that exists but rejects the widget or value, then retries another node if render_py().bindings has the same key. That contradicts the documented “real node id always wins” rule and can silently mutate the alias target. Add a collision regression where the literal node exists, its write fails, and a same-spelled binding maps elsewhere; preserve the literal failure instead of redirecting.
Swarmhost agentic reviewThe detailed evaluation is available to employees in the internal Slack review thread. Updated by Swarmhost's agentic review process. |
4 participants
Summary
Two set-widget errors from stg-v2 and nightly agent traces (Langfuse, 2026-09-20 to 21) where comfy-cli refuses input it produced itself:
<instance>/primitive_string_multiline_2.valueis a key fromprint_workflow'sbindingsmap, and set-widget refused it as an unknown interior node. set-widget now resolves a binding key to the address it maps to, but only after the literal address fails, so a real node id always wins (workflow_ops.py).PrimitiveNode.value(8 calls). A write to the node the primitive feeds already redirects onto the primitive, but a direct51.valuewas refused with "available widgets: (none)". A top-level PrimitiveNode now takes the write on its single widget, addressed asvalueor by its output marker's name (cql/promoted.py).Scope (review): the Nano Banana and Flux emit work that was here moved unchanged to #921, so this carrier holds only the address edits. The pre-split head is kept on
backup/919-before-split.Red-Green Verification
test: ...fix: ...Test Plan
🤖 Generated with Claude Code