Inject scope nodes

[timon-schelling]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a Preprocessor struct to manage node substitutions and scope injections, replacing the previous static expand_network function. It adds support for an inject_scope attribute in node macros, allowing nodes to expose their scope to other consumer nodes. A critical issue was identified in the preprocessor's network expansion logic, where removing scope injections from the parent network prevents other external consumer nodes from resolving that scope.

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.

[gemini-code-assist]

Removing the scope injection from the parent network's scope_injections map will prevent any other consumer nodes at the parent level (or in other nested networks) from resolving this scope.

Since the producer node (producer_id) still exists in the parent network (it has just been substituted with a Network implementation), it remains a valid node that other parent-level consumers can reference. By keeping the scope injection in the parent network, we allow both internal consumers (inside the producer's nested network) and external consumers (at the parent level) to correctly resolve the scope.

We should avoid removing the key from network.scope_injections.

						for (key, ty) in matching_scope_injections {
							node_network.scope_injections.insert(key, (export_id, ty));
						}

[cubic-dev-ai]

1 issue found across 11 files

Confidence score: 5/5

• This PR looks low risk to merge: the only reported issue is low severity (3/10) and appears to affect node presentation rather than core runtime behavior.
• In node-graph/nodes/gcore/src/debug.rs, test-only inject-scope nodes are being registered as production Debug nodes, which could confuse users and clutter the visible node surface with no-op entries.
• Pay close attention to node-graph/nodes/gcore/src/debug.rs - ensure test-only inject-scope nodes are gated so they don’t appear as regular production Debug nodes.

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="node-graph/nodes/gcore/src/debug.rs">

<violation number="1" location="node-graph/nodes/gcore/src/debug.rs:39">
P3: Test-only inject-scope nodes are being added as regular production Debug nodes, which can mislead users and clutter the node surface with no-op scaffolding.</violation>
</file>

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

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: Test-only inject-scope nodes are being added as regular production Debug nodes, which can mislead users and clutter the node surface with no-op scaffolding.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/gcore/src/debug.rs, line 39:

<comment>Test-only inject-scope nodes are being added as regular production Debug nodes, which can mislead users and clutter the node surface with no-op scaffolding.</comment>

<file context>
@@ -34,3 +34,13 @@ fn unwrap_option<T: Default>(_: impl Ctx, #[implementations(Option<f64>, Option<
 }
+
+#[node_macro::node(category("Debug"), inject_scope)]
+fn inject_scope_test_producer(_: impl Ctx) -> bool {
+	true
+}
</file context>

[cubic-dev-ai]

1 issue found across 4 files (changes from recent commits).

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="node-graph/libraries/core-types/src/types.rs">

<violation number="1" location="node-graph/libraries/core-types/src/types.rs:158">
P1: `as_static_str()` panics for `Cow::Owned` variant, but `ProtoNodeIdentifier::with_owned_string()` allows constructing that variant. Callers have no way to statically guarantee safety, making this a runtime panic trap.</violation>
</file>

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

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: as_static_str() panics for Cow::Owned variant, but ProtoNodeIdentifier::with_owned_string() allows constructing that variant. Callers have no way to statically guarantee safety, making this a runtime panic trap.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/core-types/src/types.rs, line 158:

<comment>`as_static_str()` panics for `Cow::Owned` variant, but `ProtoNodeIdentifier::with_owned_string()` allows constructing that variant. Callers have no way to statically guarantee safety, making this a runtime panic trap.</comment>

<file context>
@@ -154,6 +154,13 @@ impl ProtoNodeIdentifier {
 		self.name.as_ref()
 	}
+
+	pub const fn as_static_str(&self) -> &'static str {
+		match self.name {
+			Cow::Borrowed(name) => name,
</file context>

[TrueDoctor]

can you share a minimal POC containing a panic production?

[cubic-dev-ai]

@TrueDoctor Sure. Here's a minimal example that would produce the panic at runtime:

// The panic happens when as_static_str() is called on a
// ProtoNodeIdentifier backed by Cow::Owned (created via with_owned_string)

fn make_dynamic_identifier(name: String) -> ProtoNodeIdentifier {
    ProtoNodeIdentifier::with_owned_string(name)
}

// In a node definition:
#[node_macro::node(category("Foo"))]
fn some_node(
    _: impl Ctx,
    #[scope(make_dynamic_identifier("bar".into()))] injected: f64,
) -> f64 {
    injected
}

The codegen hits the else branch in codegen.rs (line 177) because make_dynamic_identifier(...) is not a string literal, so it generates:

RegistryValueScope::Scope(make_dynamic_identifier("bar".into()).as_static_str())

At runtime, make_dynamic_identifier returns a ProtoNodeIdentifier backed by Cow::Owned("bar"), and as_static_str() hits the panic!.

However — I should be honest: with the current codebase, this path is unlikely to fire naturally. All IDENTIFIER constants generated by the macro use ProtoNodeIdentifier::new(...) (borrowed), so as_static_str() works fine on them. The inject_scope_test_producer::IDENTIFIER in the test you added is safe. The with_owned_string path is only used internally in the preprocessor for dynamically-named conversion nodes like IntoNode<SomeType>, and those aren't exposed through #[scope(...)].

So the P1 severity might be overstated — it's more of a latent footgun than an active one. The real concern is that as_static_str() advertises itself as infallible (const fn returning &'static str) but can panic, which violates the principle of making invalid states unrepresentable. A cleaner design would be to either:

• Make as_static_str() return Option<&'static str> or Result
• Restrict ProtoNodeIdentifier so owned variants can't be created in contexts where as_static_str() could be called
• Or just document that it panics if called on an owned variant (acceptable if you're confident it'll never happen in practice)

_{Tip: get faster answers by chatting with cubic’s review copilot in the review UI}