Fix a previewed node's dashed output wire appearing at the wrong position#4282
Conversation
Co-authored-by: James Lindsay <78500760+0HyperCube@users.noreply.github.com>
There was a problem hiding this comment.
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.
| // 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) | ||
| } |
There was a problem hiding this comment.
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);
}There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 4/5
- In
editor/src/messages/portfolio/document/utility_types/network_interface.rs,unload_import_export_portsappears 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(_)) { |
There was a problem hiding this comment.
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>
Closes #4279
Patch written by @0HyperCube in #4279 (comment)