diff --git a/crates/brk_bindgen/src/analysis/positions.rs b/crates/brk_bindgen/src/analysis/positions.rs index 626eaec56..5368be6a1 100644 --- a/crates/brk_bindgen/src/analysis/positions.rs +++ b/crates/brk_bindgen/src/analysis/positions.rs @@ -43,27 +43,48 @@ pub fn analyze_pattern_modes( ) -> BTreeMap { // Collect analyses from all instances, keyed by pattern name let mut all_analyses: BTreeMap> = BTreeMap::new(); - // Also collect base results for each node, keyed by tree path + // Base results for each node, keyed by tree path let mut node_bases: BTreeMap = BTreeMap::new(); + // Track which tree path belongs to which pattern (avoids re-traversal) + let mut path_to_pattern: BTreeMap = BTreeMap::new(); - // Bottom-up traversal - collect_instance_analyses(tree, "", pattern_lookup, &mut all_analyses, &mut node_bases); + // Pass 1: bottom-up traversal + collect_instance_analyses( + tree, + "", + pattern_lookup, + &mut all_analyses, + &mut node_bases, + &mut path_to_pattern, + ); - // For each pattern, determine mode from collected instances + // Determine initial modes for pattern in patterns.iter_mut() { if let Some(analyses) = all_analyses.get(&pattern.name) { pattern.mode = determine_pattern_mode(analyses, &pattern.fields); } } - // Second pass: fill mixed-empty field_parts now that pattern modes are known. + // Pass 2: fill mixed-empty field_parts now that pattern modes are known fill_mixed_empty_field_parts(tree, "", pattern_lookup, patterns, &mut node_bases); - // Re-collect analyses from updated node_bases and re-determine pattern modes - let mut all_analyses2: BTreeMap> = BTreeMap::new(); - collect_updated_analyses(tree, "", pattern_lookup, &node_bases, &mut all_analyses2); + // Re-determine modes from updated node_bases (no tree re-traversal needed) + let mut updated_analyses: BTreeMap> = BTreeMap::new(); + for (path, pattern_name) in &path_to_pattern { + if let Some(br) = node_bases.get(path) { + updated_analyses + .entry(pattern_name.clone()) + .or_default() + .push(InstanceAnalysis { + base: br.base.clone(), + field_parts: br.field_parts.clone(), + is_suffix_mode: br.is_suffix_mode, + has_outlier: br.has_outlier, + }); + } + } for pattern in patterns.iter_mut() { - if let Some(analyses) = all_analyses2.get(&pattern.name) { + if let Some(analyses) = updated_analyses.get(&pattern.name) { pattern.mode = determine_pattern_mode(analyses, &pattern.fields); } } @@ -71,39 +92,6 @@ pub fn analyze_pattern_modes( node_bases } -/// Re-collect instance analyses from updated node_bases for pattern mode redetermination. -fn collect_updated_analyses( - node: &TreeNode, - path: &str, - pattern_lookup: &BTreeMap, String>, - node_bases: &BTreeMap, - all_analyses: &mut BTreeMap>, -) { - let TreeNode::Branch(children) = node else { - return; - }; - - for (field_name, child_node) in children { - let child_path = build_child_path(path, field_name); - collect_updated_analyses(child_node, &child_path, pattern_lookup, node_bases, all_analyses); - } - - let fields = get_node_fields(children, pattern_lookup); - if let Some(pattern_name) = pattern_lookup.get(&fields) { - if let Some(base_result) = node_bases.get(path) { - all_analyses - .entry(pattern_name.clone()) - .or_default() - .push(InstanceAnalysis { - base: base_result.base.clone(), - field_parts: base_result.field_parts.clone(), - is_suffix_mode: base_result.is_suffix_mode, - has_outlier: base_result.has_outlier, - }); - } - } -} - /// Second pass: fill empty field_parts for nodes that have a mix of empty and /// non-empty parts, using shortest leaf names for children that need disc. fn fill_mixed_empty_field_parts( @@ -187,6 +175,7 @@ fn collect_instance_analyses( pattern_lookup: &BTreeMap, String>, all_analyses: &mut BTreeMap>, node_bases: &mut BTreeMap, + path_to_pattern: &mut BTreeMap, ) -> Option { match node { TreeNode::Leaf(leaf) => { @@ -204,6 +193,7 @@ fn collect_instance_analyses( pattern_lookup, all_analyses, node_bases, + path_to_pattern, ) { child_bases.insert(field_name.clone(), base); } @@ -272,6 +262,7 @@ fn collect_instance_analyses( // Get the pattern name for this node (if any) let fields = get_node_fields(children, pattern_lookup); if let Some(pattern_name) = pattern_lookup.get(&fields) { + path_to_pattern.insert(path.to_string(), pattern_name.clone()); all_analyses .entry(pattern_name.clone()) .or_default() @@ -1107,6 +1098,7 @@ mod tests { let mut all_analyses = BTreeMap::new(); let mut node_bases = BTreeMap::new(); + let mut path_to_pattern = BTreeMap::new(); let pattern_lookup = BTreeMap::new(); collect_instance_analyses( @@ -1115,6 +1107,7 @@ mod tests { &pattern_lookup, &mut all_analyses, &mut node_bases, + &mut path_to_pattern, ); let result = node_bases.get("test").expect("should have node_bases entry"); diff --git a/crates/brk_bindgen/src/generate/fields.rs b/crates/brk_bindgen/src/generate/fields.rs index 9dd2ec32d..f05118ec4 100644 --- a/crates/brk_bindgen/src/generate/fields.rs +++ b/crates/brk_bindgen/src/generate/fields.rs @@ -8,7 +8,7 @@ use std::fmt::Write; use brk_types::MetricLeafWithSchema; -use crate::{ClientMetadata, LanguageSyntax, PatternField, StructuralPattern}; +use crate::{ClientMetadata, LanguageSyntax, PatternBaseResult, PatternField, StructuralPattern}; /// Create a path suffix from a name. /// Adds `_` prefix only if the name doesn't already start with `_`. @@ -102,10 +102,11 @@ pub fn generate_parameterized_field( .unwrap(); } -/// Generate a tree node field with a specific child node for pattern instance base detection. +/// Generate a tree node field using pre-computed base results. /// -/// This is used when generating tree nodes where we need to detect the pattern instance -/// base from descendant leaf names. +/// Handles pattern fields (both templated and non-templated), leaf fields, +/// and non-pattern branch fields. For templated patterns, extracts the +/// discriminator from the base result's field_parts. pub fn generate_tree_node_field( output: &mut String, syntax: &S, @@ -113,36 +114,69 @@ pub fn generate_tree_node_field( metadata: &ClientMetadata, indent: &str, child_name: &str, - pattern_base: Option<&str>, + client_expr: &str, + base_result: Option<&PatternBaseResult>, ) { let field_name = syntax.field_name(&field.name); let type_ann = metadata.field_type_annotation(field, false, None, syntax.generic_syntax()); let value = if metadata.is_pattern_type(&field.rust_type) { - // Use metric base only for parameterizable patterns - let use_base = metadata - .find_pattern(&field.rust_type) - .is_some_and(|p| p.is_parameterizable()) - && pattern_base.is_some(); + let pattern = metadata.find_pattern(&field.rust_type); + let use_base = pattern.is_some_and(|p| p.is_parameterizable()) && base_result.is_some(); - let path_arg = if use_base { - syntax.string_literal(pattern_base.unwrap()) + if use_base { + let br = base_result.unwrap(); + let base_arg = syntax.string_literal(&br.base); + if let Some(pat) = pattern + && pat.is_templated() + { + let disc = pat + .extract_disc_from_instance(&br.field_parts) + .unwrap_or_default(); + let disc_arg = syntax.string_literal(&disc); + format!( + "{}({}, {}, {})", + syntax.constructor_name(&field.rust_type), + client_expr, + base_arg, + disc_arg + ) + } else { + format!( + "{}({}, {})", + syntax.constructor_name(&field.rust_type), + client_expr, + base_arg + ) + } } else { - syntax.path_expr("base_path", &path_suffix(child_name)) - }; - syntax.constructor(&field.rust_type, &path_arg) + let path_arg = syntax.path_expr("base_path", &path_suffix(child_name)); + format!( + "{}({}, {})", + syntax.constructor_name(&field.rust_type), + client_expr, + path_arg + ) + } } else if let Some(accessor) = metadata.find_index_set_pattern(&field.indexes) { - // Leaf field - use metric name if provided, else tree path - let path_arg = pattern_base - .map(|name| syntax.string_literal(name)) + let path_arg = base_result + .map(|br| syntax.string_literal(&br.base)) .unwrap_or_else(|| syntax.path_expr("base_path", &path_suffix(child_name))); - syntax.constructor(&accessor.name, &path_arg) + format!( + "{}({}, {})", + syntax.constructor_name(&accessor.name), + client_expr, + path_arg + ) } else if field.is_branch() { - // Non-pattern branch - instantiate the nested struct let path_expr = syntax.path_expr("base_path", &path_suffix(child_name)); - syntax.constructor(&field.rust_type, &path_expr) + format!( + "{}({}, {})", + syntax.constructor_name(&field.rust_type), + client_expr, + path_expr + ) } else { - // All metrics must be indexed panic!( "Field '{}' is a leaf with no index accessor. All metrics must be indexed.", field.name diff --git a/crates/brk_bindgen/src/generators/javascript/tree.rs b/crates/brk_bindgen/src/generators/javascript/tree.rs index 33670b556..d657167c9 100644 --- a/crates/brk_bindgen/src/generators/javascript/tree.rs +++ b/crates/brk_bindgen/src/generators/javascript/tree.rs @@ -7,7 +7,7 @@ use brk_types::TreeNode; use crate::{ ClientMetadata, Endpoint, GenericSyntax, JavaScriptSyntax, PatternField, build_child_path, - generate_leaf_field, prepare_tree_node, to_camel_case, + generate_leaf_field, generate_tree_node_field, prepare_tree_node, to_camel_case, }; use super::api::generate_api_methods; @@ -220,29 +220,16 @@ fn generate_tree_initializer( ); writeln!(output, "{}}},", indent_str).unwrap(); } else { - // Use pattern factory - let pattern = metadata.find_pattern(&child.field.rust_type); - if let Some(pat) = pattern - && pat.is_templated() - { - // Templated: extract discriminator using the pattern's templates - let disc = pat - .extract_disc_from_instance(&child.base_result.field_parts) - .unwrap_or_default(); - writeln!( - output, - "{}{}: create{}(this, '{}', '{}'),", - indent_str, field_name, child.field.rust_type, child.base_result.base, disc - ) - .unwrap(); - } else { - writeln!( - output, - "{}{}: create{}(this, '{}'),", - indent_str, field_name, child.field.rust_type, child.base_result.base - ) - .unwrap(); - } + generate_tree_node_field( + output, + &syntax, + &child.field, + metadata, + &indent_str, + child.name, + "this", + Some(&child.base_result), + ); } } } diff --git a/crates/brk_bindgen/src/generators/python/tree.rs b/crates/brk_bindgen/src/generators/python/tree.rs index 019089073..cddadcc6c 100644 --- a/crates/brk_bindgen/src/generators/python/tree.rs +++ b/crates/brk_bindgen/src/generators/python/tree.rs @@ -6,8 +6,8 @@ use std::fmt::Write; use brk_types::TreeNode; use crate::{ - ClientMetadata, GenericSyntax, PatternField, PythonSyntax, build_child_path, - escape_python_keyword, generate_leaf_field, prepare_tree_node, to_snake_case, + ClientMetadata, LanguageSyntax, PatternField, PythonSyntax, build_child_path, + generate_leaf_field, generate_tree_node_field, prepare_tree_node, }; /// Generate tree classes @@ -70,8 +70,6 @@ fn generate_tree_class( let syntax = PythonSyntax; for child in &ctx.children { - let field_name_py = escape_python_keyword(&to_snake_case(child.name)); - if child.is_leaf { if let TreeNode::Leaf(leaf) = child.node { generate_leaf_field( @@ -79,43 +77,24 @@ fn generate_tree_class( ); } } else if child.should_inline { - // Inline class + let field_name = syntax.field_name(child.name); writeln!( output, " self.{}: {} = {}(client)", - field_name_py, child.inline_type_name, child.inline_type_name + field_name, child.inline_type_name, child.inline_type_name ) .unwrap(); } else { - // Use pattern class with metric base - let py_type = metadata.resolve_tree_field_type( + generate_tree_node_field( + output, + &syntax, &child.field, - child.child_fields.as_deref(), - name, + metadata, + " ", child.name, - GenericSyntax::PYTHON, + "client", + Some(&child.base_result), ); - let pattern = metadata.find_pattern(&child.field.rust_type); - if let Some(pat) = pattern - && pat.is_templated() - { - let disc = pat - .extract_disc_from_instance(&child.base_result.field_parts) - .unwrap_or_default(); - writeln!( - output, - " self.{}: {} = {}(client, '{}', '{}')", - field_name_py, py_type, child.field.rust_type, child.base_result.base, disc - ) - .unwrap(); - } else { - writeln!( - output, - " self.{}: {} = {}(client, '{}')", - field_name_py, py_type, child.field.rust_type, child.base_result.base - ) - .unwrap(); - } } } diff --git a/crates/brk_bindgen/src/generators/rust/tree.rs b/crates/brk_bindgen/src/generators/rust/tree.rs index 6d611d391..2edc276ac 100644 --- a/crates/brk_bindgen/src/generators/rust/tree.rs +++ b/crates/brk_bindgen/src/generators/rust/tree.rs @@ -97,34 +97,16 @@ fn generate_tree_node( ) .unwrap(); } else { - // Pattern type - use ::new() constructor - let pattern = metadata.find_pattern(&child.field.rust_type); - if let Some(pat) = pattern - && pat.is_templated() - { - let disc = pat - .extract_disc_from_instance(&child.base_result.field_parts) - .unwrap_or_default(); - writeln!( - output, - " {}: {}::new(client.clone(), {}, {}.to_string()),", - field_name, - child.field.rust_type, - syntax.string_literal(&child.base_result.base), - syntax.string_literal(&disc), - ) - .unwrap(); - } else { - generate_tree_node_field( - output, - &syntax, - &child.field, - metadata, - " ", - child.name, - Some(&child.base_result.base), - ); - } + generate_tree_node_field( + output, + &syntax, + &child.field, + metadata, + " ", + child.name, + "client.clone()", + Some(&child.base_result), + ); } }