Use more specific node input indexing when displaying invalid input errors (#3415)

* Reduce displayed invalid inputs

* Correct error offset for convert node

* Apply suggestions from code review

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>
This commit is contained in:
Adam Gerhant 2025-12-18 02:48:25 -08:00 committed by GitHub
parent b97978e91d
commit 6733a24e47
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 38 additions and 19 deletions

View file

@ -2048,7 +2048,7 @@ impl<'a> ParameterWidgetsInfo<'a> {
let (name, description) = context.network_interface.displayed_input_name_and_description(&node_id, index, context.selection_network_path);
let input_type = context
.network_interface
.input_type(&InputConnector::node(node_id, index), context.selection_network_path)
.input_type_not_invalid(&InputConnector::node(node_id, index), context.selection_network_path)
.displayed_type();
let document_node = context.network_interface.document_node(&node_id, context.selection_network_path);

View file

@ -2,7 +2,7 @@ use std::collections::{HashMap, HashSet};
use graph_craft::document::value::TaggedValue;
use graph_craft::document::{DocumentNodeImplementation, InlineRust, NodeInput};
use graph_craft::proto::GraphErrors;
use graph_craft::proto::{GraphErrorType, GraphErrors};
use graph_craft::{Type, concrete};
use graphene_std::uuid::NodeId;
use interpreted_executor::dynamic_executor::{NodeTypes, ResolvedDocumentNodeTypesDelta};
@ -129,7 +129,14 @@ impl NodeNetworkInterface {
InputConnector::Export(_) => false,
})
}
DocumentNodeImplementation::ProtoNode(_) => self.resolved_types.node_graph_errors.iter().any(|error| error.node_path == node_path),
DocumentNodeImplementation::ProtoNode(_) => self.resolved_types.node_graph_errors.iter().any(|error| {
error.node_path == node_path
&& match &error.error {
GraphErrorType::InvalidImplementations { error_inputs, .. } => error_inputs.iter().any(|solution| solution.iter().any(|(index, _)| index == input_index)),
_ => true,
}
}),
DocumentNodeImplementation::Extract => false,
}
}
@ -137,7 +144,7 @@ impl NodeNetworkInterface {
}
}
fn input_type_not_invalid(&mut self, input_connector: &InputConnector, network_path: &[NodeId]) -> TypeSource {
pub fn input_type_not_invalid(&mut self, input_connector: &InputConnector, network_path: &[NodeId]) -> TypeSource {
let Some(input) = self.input_from_connector(input_connector, network_path) else {
return TypeSource::Error("Could not get input from connector");
};

View file

@ -84,6 +84,8 @@ pub struct OriginalLocation {
pub dependants: Vec<Vec<NodeId>>,
/// A list of flags indicating whether the input is exposed in the UI
pub inputs_exposed: Vec<bool>,
/// For automatically inserted Convert and Into nodes, if there is an error, display it on the node it is connect to.
pub auto_convert_index: Option<usize>,
}
impl Default for DocumentNode {
@ -664,12 +666,9 @@ impl NodeNetwork {
if node.original_location.path.is_some() {
log::warn!("Attempting to overwrite node path");
} else {
node.original_location = OriginalLocation {
path: Some(new_path),
inputs_exposed: node.inputs.iter().map(|input| input.is_exposed()).collect(),
dependants: (0..node.implementation.output_count()).map(|_| Vec::new()).collect(),
..Default::default()
};
node.original_location.path = Some(new_path);
node.original_location.inputs_exposed = node.inputs.iter().map(|input| input.is_exposed()).collect();
node.original_location.dependants = (0..node.implementation.output_count()).map(|_| Vec::new()).collect();
}
}
}

View file

@ -539,11 +539,23 @@ impl ProtoNetwork {
#[derive(Clone, PartialEq, serde::Serialize, serde::Deserialize)]
pub enum GraphErrorType {
NodeNotFound(NodeId),
UnexpectedGenerics { index: usize, inputs: Vec<Type> },
UnexpectedGenerics {
index: usize,
inputs: Vec<Type>,
},
NoImplementations,
NoConstructor,
InvalidImplementations { inputs: String, error_inputs: Vec<Vec<(usize, (Type, Type))>> },
MultipleImplementations { inputs: String, valid: Vec<NodeIOTypes> },
/// The `inputs` represents a formatted list of input indices corresponding to their types.
/// Each element in `error_inputs` represents a valid `NodeIOTypes` implementation.
/// The inner Vec stores the inputs which need to be changed and what type each needs to be changed to.
InvalidImplementations {
inputs: String,
error_inputs: Vec<Vec<(usize, (Type, Type))>>,
},
MultipleImplementations {
inputs: String,
valid: Vec<NodeIOTypes>,
},
}
impl Debug for GraphErrorType {
// TODO: format with the document graph context so the input index is the same as in the graph UI.
@ -756,9 +768,11 @@ impl TypingContext {
match valid_impls.as_slice() {
[] => {
let convert_node_index_offset = node.original_location.auto_convert_index.unwrap_or(0);
let mut best_errors = usize::MAX;
let mut error_inputs = Vec::new();
for node_io in impls.keys() {
// For errors on Convert nodes, offset the input index so it correctly corresponds to the node it is connected to.
let current_errors = [call_argument]
.into_iter()
.chain(&inputs)
@ -766,10 +780,7 @@ impl TypingContext {
.zip([&node_io.call_argument].into_iter().chain(&node_io.inputs).cloned())
.enumerate()
.filter(|(_, (p1, p2))| !valid_type(p1, p2))
.map(|(index, ty)| {
let i = node.original_location.inputs(index).min_by_key(|s| s.node.len()).map(|s| s.index).unwrap_or(index);
(i, ty)
})
.map(|(index, expected)| (index - 1 + convert_node_index_offset, expected))
.collect::<Vec<_>>();
if current_errors.len() < best_errors {
best_errors = current_errors.len();
@ -783,7 +794,7 @@ impl TypingContext {
.into_iter()
.chain(&inputs)
.enumerate()
.filter_map(|(i, t)| if i == 0 { None } else { Some(format!("• Input {i}: {t}")) })
.filter_map(|(i, t)| if i == 0 { None } else { Some(format!("• Input {}: {t}", i + convert_node_index_offset)) })
.collect::<Vec<_>>()
.join("\n");
Err(vec![GraphError::new(node, GraphErrorType::InvalidImplementations { inputs, error_inputs })])

View file

@ -86,11 +86,13 @@ pub fn generate_node_substitutions() -> HashMap<ProtoNodeIdentifier, DocumentNod
} else {
identity_node.clone()
};
let mut original_location = OriginalLocation::default();
original_location.auto_convert_index = Some(i);
DocumentNode {
inputs,
implementation: DocumentNodeImplementation::ProtoNode(proto_node),
visible: true,
original_location,
..Default::default()
}
}