Keep the outward wire cache exact through node input and output replacement, insertion, and deletion, and validate it after every test message

[Keavon]

Opening a document saved before roughly May 2026 could silently scramble its node graph layout. Nodes that belonged to a layer's chain came loose and sat as free-floating nodes, and layers could drop out of their stack. On the May save of isometric-fountain, about 134 nodes came out of their chains. The same corruption also made later edits unsafe: deleting a node could disconnect and rewire an unrelated input, and the "delete with children" logic could misjudge what was shared. It persisted for the whole session until an undo or paste happened to rebuild things.

The graph stores connections in one direction: each node input records which output feeds it. To answer the reverse question, "who consumes this output?", the editor keeps a cached reverse index. That cache is normally patched one input at a time, and the patch relies on being told the input's old value so it knows which entry to remove. Document migrations swap a node's entire input list in one step and then re-wire the surviving inputs individually. The bulk swap never told the cache anything, so when each re-wire ran, the old value it saw was a placeholder rather than the wire the cache still held. The remove step found nothing, the add step appended a second copy, and the output was listed with two consumers where it had one. Layout code that asks "does this node feed exactly one thing?" then said no and un-chained it. Two smaller drifts came from the same audit: inserting a node never registered its outputs in the cache, and deleting one never unregistered them. Four migrations carried "pre-load the cache" lines meant to prevent a suspected problem; since the patch only runs when the cache is loaded, those lines guaranteed the corruption instead.

Every operation that rewrites inputs or outputs without going through the single-input path now updates the cache in the same incremental way: swapping an input list unhooks each old wire and hooks each new one, changing a node's implementation or inserting or deleting a node adds or removes its output entries, and the pre-loads are gone. The interface's invariant checker, which the test harness already runs after every message in every editor test, now compares a loaded cache against a fresh recomputation, so any future drift fails the test that caused it. That check flagged 34 existing tests before the fix and none after. Focused tests cover each failure mode, and the migration tests now also run the shipped demo files.

[cubic-dev-ai]

All reported issues were addressed across 7 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

3 issues found across 4 files (changes from recent commits).

Confidence score: 3/5

• editor/src/messages/portfolio/document/utility_types/network_interface/mutations.rs — remove_node_entry can delete a graph node while returning None, so its ID is omitted from removed_nodes and stale output keys can remain in the loaded outward-wire state; ensure deletion is reported even when metadata is missing.
• editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs — caches for vanished outputs can drain to empty without being removed because cleanup only runs during output shrink, allowing stale entries to persist; remove empty entries during update_outward_wires or unhook_outward_wire.
• editor/src/messages/portfolio/document_migration.rs — the migration test does not prime nested-network wire caches, so regressions on recursive paths may go undetected; populate the outward-wire cache for every recursive network path before migration.

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/mutations.rs">

<violation number="1" location="editor/src/messages/portfolio/document/utility_types/network_interface/mutations.rs:832">
P2: `remove_node_entry` can remove the graph node and still return `None` when its metadata entry is missing. This branch then omits the ID from `removed_nodes`, leaving that deleted node’s output keys in the loaded outward cache; record graph removal independently or make `remove_node_entry` report it separately from the joined template result.</violation>
</file>

<file name="editor/src/messages/portfolio/document_migration.rs">

<violation number="1" location="editor/src/messages/portfolio/document_migration.rs:3248">
P2: This test setup does not validate nested-network wire caches, so a migration regression in a nested path can pass unnoticed. Prime the outward-wire cache for every recursive network path before running the migration, then let `validate_invariants` compare each loaded cache.</violation>
</file>

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

<violation number="1" location="editor/src/messages/portfolio/document/utility_types/network_interface/caches.rs:540">
P2: An entry kept for a vanished output can later drain to empty without being deleted. The keep-if-non-empty check runs only at output-shrink time; afterward `update_outward_wires`/`unhook_outward_wire` (which never removes an entry, only `retain`s its consumers) can empty the entry when a consumer is rewired away. `compute_outward_wires` initializes entries only for `0..number_of_outputs` and re-creates one elsewhere only while a graph wire references it, so a leftover empty entry for a vanished output no longer matches a fresh computation (`validate_outward_wires` compares `cached Some([])` to `computed None`). The migration tests now run with the cache loaded, but this sequence (shrink outputs while a consumer is still wired, then rewire that consumer) is not covered, so the drift can silently persist in production where the invariant check does not run.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: remove_node_entry can remove the graph node and still return None when its metadata entry is missing. This branch then omits the ID from removed_nodes, leaving that deleted node’s output keys in the loaded outward cache; record graph removal independently or make remove_node_entry report it separately from the joined template result.

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/mutations.rs, line 832:

<comment>`remove_node_entry` can remove the graph node and still return `None` when its metadata entry is missing. This branch then omits the ID from `removed_nodes`, leaving that deleted node’s output keys in the loaded outward cache; record graph removal independently or make `remove_node_entry` report it separately from the joined template result.</comment>

<file context>
@@ -828,7 +829,9 @@ impl NodeNetworkInterface {
 			}
 
-			self.remove_node_entry(NodeLocator::new(*delete_node_id, network_path));
+			if self.remove_node_entry(NodeLocator::new(*delete_node_id, network_path)).is_some() {
+				removed_nodes.insert(*delete_node_id);
+			}
</file context>

[cubic-dev-ai]

P2: This test setup does not validate nested-network wire caches, so a migration regression in a nested path can pass unnoticed. Prime the outward-wire cache for every recursive network path before running the migration, then let validate_invariants compare each loaded cache.

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_migration.rs, line 3248:

<comment>This test setup does not validate nested-network wire caches, so a migration regression in a nested path can pass unnoticed. Prime the outward-wire cache for every recursive network path before running the migration, then let `validate_invariants` compare each loaded cache.</comment>

<file context>
@@ -3239,12 +3239,17 @@ fn migrate_removed_catalog_definitions(node_id: &NodeId, node: &DocumentNode, ne
+	}
+
+	fn migrate_with_context(document: &mut DocumentMessageHandler, context: &str) {
+		assert!(document.network_interface.outward_wires(&[]).is_some());
 		document_migration_upgrades(document, false);
 
</file context>

[cubic-dev-ai]

P2: An entry kept for a vanished output can later drain to empty without being deleted. The keep-if-non-empty check runs only at output-shrink time; afterward update_outward_wires/unhook_outward_wire (which never removes an entry, only retains its consumers) can empty the entry when a consumer is rewired away. compute_outward_wires initializes entries only for 0..number_of_outputs and re-creates one elsewhere only while a graph wire references it, so a leftover empty entry for a vanished output no longer matches a fresh computation (validate_outward_wires compares cached Some([]) to computed None). The migration tests now run with the cache loaded, but this sequence (shrink outputs while a consumer is still wired, then rewire that consumer) is not covered, so the drift can silently persist in production where the invariant check does not run.

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/caches.rs, line 540:

<comment>An entry kept for a vanished output can later drain to empty without being deleted. The keep-if-non-empty check runs only at output-shrink time; afterward `update_outward_wires`/`unhook_outward_wire` (which never removes an entry, only `retain`s its consumers) can empty the entry when a consumer is rewired away. `compute_outward_wires` initializes entries only for `0..number_of_outputs` and re-creates one elsewhere only while a graph wire references it, so a leftover empty entry for a vanished output no longer matches a fresh computation (`validate_outward_wires` compares `cached Some([])` to `computed None`). The migration tests now run with the cache loaded, but this sequence (shrink outputs while a consumer is still wired, then rewire that consumer) is not covered, so the drift can silently persist in production where the invariant check does not run.</comment>

<file context>
@@ -534,8 +534,12 @@ impl NodeNetworkInterface {
 		for output_index in new_output_count..old_output_count {
-			outward_wires.remove(&OutputConnector::node(*node_id, output_index));
+			let output = OutputConnector::node(*node_id, output_index);
+			if outward_wires.get(&output).is_some_and(Vec::is_empty) {
+				outward_wires.remove(&output);
+			}
</file context>