Skip to content

Frontend type constraints#4334

Open
0HyperCube wants to merge 1 commit into
GraphiteEditor:masterfrom
0HyperCube:frontend-type-constraints
Open

Frontend type constraints#4334
0HyperCube wants to merge 1 commit into
GraphiteEditor:masterfrom
0HyperCube:frontend-type-constraints

Conversation

@0HyperCube

Copy link
Copy Markdown
Contributor

Closes #4330

Note that this adds some warnings:

The default value for input index 1 node Color Overlay is Color[], but does not satisfy Limited({"Color"})
The default value for input index 1 node Color Overlay GPU is Color[], but does not satisfy Limited({"Color"})

I guess maybe these nodes should be updated to use the tables so that they don't produce errors when instantiated. Alternatively the list thing could be removed.

@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 introduces a structured FrontendTypeConstraintForInput enum to handle complex type constraints for node inputs, replacing the previous simple string representation. It includes logic to compute, intersect, and validate these constraints, along with comprehensive unit tests and corresponding frontend updates in NodeCatalog.svelte. The review feedback highlights several instances where the unqualified warn! macro should be replaced with log::warn! to prevent compilation errors, identifies a few typos in comments, and suggests using optional chaining in the frontend to safely access nested properties.

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.

}

let Some(implementations) = interpreted_executor::node_registry::NODE_REGISTRY.get(proto_node_identifier) else {
warn!("No implementations found for protonode {proto_node_identifier} in {name}");

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.

high

The warn! macro is used directly without being imported, which will likely cause a compilation error since the rest of the file consistently uses log::warn! or log::error!. Use log::warn! instead.

Suggested change
warn!("No implementations found for protonode {proto_node_identifier} in {name}");
log::warn!("No implementations found for protonode {proto_node_identifier} in {name}");

result_accepted
}
DocumentNodeImplementation::Extract => {
warn!("Input types for extract node {name} not supported");

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.

high

The warn! macro is used directly without being imported. Use log::warn! instead to maintain consistency and avoid compilation errors.

Suggested change
warn!("Input types for extract node {name} not supported");
log::warn!("Input types for extract node {name} not supported");


// Empty types are used when the input must come from the graph so they can be skipped.
if input_ty.nested_type() != &concrete!(()) && !constraint.satisfies(&input_ty) {
warn!("The default value for input index {index} node {name} is {input:?}, but does not satisfy {constraint:?}");

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.

high

The warn! macro is used directly without being imported. Use log::warn! instead to maintain consistency and avoid compilation errors.

Suggested change
warn!("The default value for input index {index} node {name} is {input:?}, but does not satisfy {constraint:?}");
log::warn!("The default value for input index {index} node {name} is {input:?}, but does not satisfy {constraint:?}");

Comment thread editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs Outdated
Comment thread editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs Outdated
Comment thread editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs Outdated

if (isTypeSearch && typeSearchTerm) {
matchesTypeSearch = node.inputTypes?.some((inputType) => inputType.toLowerCase().includes(typeSearchTerm)) || false;
matchesTypeSearch = node.inputTypes?.some((constraint) => constraint === "All" || constraint.Limited.some((inputType) => inputType.toLowerCase().includes(typeSearchTerm))) || false;

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

To prevent potential runtime TypeError exceptions if constraint has an unexpected shape or is null/undefined, use optional chaining constraint.Limited?.some(...) instead of constraint.Limited.some(...).

				matchesTypeSearch = node.inputTypes?.some((constraint) => constraint === "All" || constraint.Limited?.some((inputType) => inputType.toLowerCase().includes(typeSearchTerm))) || false;

@0HyperCube 0HyperCube force-pushed the frontend-type-constraints branch from 558bba1 to b5d20e4 Compare July 13, 2026 10:18
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.

Filtering based on types false negatives

1 participant