Skip to content

Fix a previewed node's dashed output wire appearing at the wrong position#4282

Merged
timon-schelling merged 2 commits into
masterfrom
fix-graph-export-ui-position
Jun 26, 2026
Merged

Fix a previewed node's dashed output wire appearing at the wrong position#4282
timon-schelling merged 2 commits into
masterfrom
fix-graph-export-ui-position

Conversation

@timon-schelling

Copy link
Copy Markdown
Member

Closes #4279

Patch written by @0HyperCube in #4279 (comment)

Co-authored-by: James Lindsay <78500760+0HyperCube@users.noreply.github.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds a port refresh mechanism when altering an export in the network interface. The review feedback suggests adding a semicolon to the function call for idiomatic Rust and highlights that disconnecting a node from an export also alters the export, suggesting the check be moved or duplicated to handle this case.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +4242 to +4245
// Altering an export may move the connectors meaning the ports must be refreshed.
if matches!(input_connector, InputConnector::Export(_)) {
self.unload_import_export_ports(network_path)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Add a semicolon to the unload_import_export_ports call for idiomatic Rust and consistency with the rest of the codebase.

Additionally, note that disconnecting a node from an export (which is handled in the (NodeInput::Node { .. }, NodeInput::Value { .. } | ...) arm of set_input) also alters the export and can change whether the first export is a layer, which affects the connector positions. You should consider either adding a similar check to that arm or moving this check to the very end of the set_input function so it runs for any successful export modification.

				// Altering an export may move the connectors meaning the ports must be refreshed.
				if matches!(input_connector, InputConnector::Export(_)) {
					self.unload_import_export_ports(network_path);
				}

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file

Confidence score: 4/5

  • In editor/src/messages/portfolio/document/utility_types/network_interface.rs, unload_import_export_ports appears to run when connecting a node to an export but not when disconnecting in the (NodeInput::Node { .. }, NodeInput::Value { .. } | ...) path, which can leave import/export ports out of sync after a disconnect and produce incorrect graph/interface state; add the corresponding disconnect-path handling (and a regression test for disconnecting from exports) before merging.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/portfolio/document/utility_types/network_interface.rs">

<violation number="1" location="editor/src/messages/portfolio/document/utility_types/network_interface.rs:4243">
P2: This `unload_import_export_ports` call only covers the case where a node is being connected to an export. However, disconnecting a node from an export (the `(NodeInput::Node { .. }, NodeInput::Value { .. } | ...)` arm) can also change whether the first export is a layer, affecting connector positions. Consider moving this check to the end of `set_input` so it runs for any successful export modification, or add an equivalent check in that arm.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

NodeTypePersistentMetadata::Node(_) => {}
}
// Altering an export may move the connectors meaning the ports must be refreshed.
if matches!(input_connector, InputConnector::Export(_)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This unload_import_export_ports call only covers the case where a node is being connected to an export. However, disconnecting a node from an export (the (NodeInput::Node { .. }, NodeInput::Value { .. } | ...) arm) can also change whether the first export is a layer, affecting connector positions. Consider moving this check to the end of set_input so it runs for any successful export modification, or add an equivalent check in that arm.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/utility_types/network_interface.rs, line 4243:

<comment>This `unload_import_export_ports` call only covers the case where a node is being connected to an export. However, disconnecting a node from an export (the `(NodeInput::Node { .. }, NodeInput::Value { .. } | ...)` arm) can also change whether the first export is a layer, affecting connector positions. Consider moving this check to the end of `set_input` so it runs for any successful export modification, or add an equivalent check in that arm.</comment>

<file context>
@@ -4239,6 +4239,10 @@ impl NodeNetworkInterface {
 					NodeTypePersistentMetadata::Node(_) => {}
 				}
+				// Altering an export may move the connectors meaning the ports must be refreshed.
+				if matches!(input_connector, InputConnector::Export(_)) {
+					self.unload_import_export_ports(network_path)
+				}
</file context>

@Keavon Keavon changed the title Fix node graph connection to export in wrong place Fix a previewed node's dashed output wire appearing at the wrong position Jun 26, 2026
@timon-schelling timon-schelling added this pull request to the merge queue Jun 26, 2026
Merged via the queue into master with commit 6844961 Jun 26, 2026
10 checks passed
@timon-schelling timon-schelling deleted the fix-graph-export-ui-position branch June 26, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node graph connection to export in wrong place

2 participants