bindgen: works

crates/brk_bindgen/src/analysis/positions.rs

@@ -24,6 +24,8 @@ struct InstanceAnalysis {
     field_parts: BTreeMap<String, String>,
     /// Whether this instance appears to be suffix mode
     is_suffix_mode: bool,
+    /// Whether children have no common prefix/suffix (outlier naming like sopr/asopr)
+    has_outlier: bool,
 }
 
 /// Analyze all pattern instances and determine their modes.
@@ -54,9 +56,127 @@ pub fn analyze_pattern_modes(
         }
     }
 
+    // Second pass: 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<String, Vec<InstanceAnalysis>> = BTreeMap::new();
+    collect_updated_analyses(tree, "", pattern_lookup, &node_bases, &mut all_analyses2);
+    for pattern in patterns.iter_mut() {
+        if let Some(analyses) = all_analyses2.get(&pattern.name) {
+            pattern.mode = determine_pattern_mode(analyses, &pattern.fields);
+        }
+    }
+
     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<Vec<PatternField>, String>,
+    node_bases: &BTreeMap<String, PatternBaseResult>,
+    all_analyses: &mut BTreeMap<String, Vec<InstanceAnalysis>>,
+) {
+    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(
+    node: &TreeNode,
+    path: &str,
+    pattern_lookup: &BTreeMap<Vec<PatternField>, String>,
+    patterns: &[StructuralPattern],
+    node_bases: &mut BTreeMap<String, PatternBaseResult>,
+) {
+    let TreeNode::Branch(children) = node else {
+        return;
+    };
+
+    // Recurse first (bottom-up)
+    for (field_name, child_node) in children {
+        let child_path = build_child_path(path, field_name);
+        fill_mixed_empty_field_parts(child_node, &child_path, pattern_lookup, patterns, node_bases);
+    }
+
+    // Check if this node has mixed empty/non-empty field_parts
+    let Some(base_result) = node_bases.get(path) else {
+        return;
+    };
+    let has_empty = base_result.field_parts.values().any(|v| v.is_empty());
+    let has_nonempty = base_result.field_parts.values().any(|v| !v.is_empty());
+    if !has_empty || !has_nonempty {
+        return;
+    }
+
+    let prefix = format!("{}_", base_result.base);
+    let mut updates: Vec<(String, String)> = Vec::new();
+
+    for (field_name, child_node) in children {
+        let part = base_result.field_parts.get(field_name.as_str());
+        if !part.is_some_and(|p| p.is_empty()) {
+            continue;
+        }
+
+        // Check if the child's pattern is templated (needs disc from parent)
+        let child_pattern_is_templated = if let TreeNode::Branch(ch) = child_node {
+            let child_fields = get_node_fields(ch, pattern_lookup);
+            pattern_lookup
+                .get(&child_fields)
+                .and_then(|name| patterns.iter().find(|p| &p.name == name))
+                .is_some_and(|p| p.is_templated())
+        } else {
+            false
+        };
+
+        // Only fill if the child needs disc (templated) or is a leaf
+        let is_leaf = matches!(child_node, TreeNode::Leaf(_));
+        if !child_pattern_is_templated && !is_leaf {
+            continue;
+        }
+
+        if let Some(leaf) = get_shortest_leaf_name(child_node)
+            && let Some(suffix) = leaf.strip_prefix(&prefix)
+            && !suffix.is_empty()
+            && suffix.contains(field_name.trim_start_matches('_'))
+            && suffix.len() > field_name.trim_start_matches('_').len()
+        {
+            updates.push((field_name.clone(), suffix.to_string()));
+        }
+    }
+
+    if !updates.is_empty() {
+        let base_result = node_bases.get_mut(path).unwrap();
+        for (field_name, suffix) in updates {
+            base_result.field_parts.insert(field_name, suffix);
+        }
+    }
+}
+
 /// Recursively collect instance analyses bottom-up.
 /// Returns the "base" for this node (used by parent for its analysis).
 ///
@@ -98,52 +218,52 @@ fn collect_instance_analyses(
 
             // When some field_parts are empty (children returned the same base),
             // replace empty parts with discriminators derived from shortest leaf names.
-            let has_empty = analysis.field_parts.values().any(|v| v.is_empty());
-            let has_nonempty = analysis.field_parts.values().any(|v| !v.is_empty());
-            if has_empty && has_nonempty {
-                // Mixed case: some fields have parts, some don't.
-                // Use shortest leaf to derive discriminators for empty fields.
+            let all_empty = analysis.field_parts.len() > 1
+                && analysis.field_parts.values().all(|v| v.is_empty());
+            if all_empty {
+                // All-empty case: all children returned the same base.
+                // Use shortest leaf to derive field_parts for fields whose key
+                // matches the metric suffix (e.g., pct1 → suffix "pct1").
                 let prefix = format!("{}_", analysis.base);
+                let mut any_filled = false;
                 for (field_name, child_node) in children {
                     if let Some(part) = analysis.field_parts.get(field_name)
                         && part.is_empty()
                         && let Some(leaf) = get_shortest_leaf_name(child_node)
                         && let Some(suffix) = leaf.strip_prefix(&prefix)
                         && !suffix.is_empty()
-                        // Only use if the suffix starts with the field key,
-                        // avoiding internal sub-field names like "0sd" from a Price child
                         && suffix.starts_with(field_name.trim_start_matches('_'))
                     {
                         analysis
                             .field_parts
                             .insert(field_name.clone(), suffix.to_string());
+                        any_filled = true;
                     }
                 }
-            } else if has_empty && analysis.field_parts.len() > 1 {
-                // All-empty case: all children returned the same base.
-                // Re-analyze using shortest leaf names which may differentiate.
-                let mut leaf_bases: BTreeMap<String, String> = BTreeMap::new();
-                for (field_name, child_node) in children {
-                    if let Some(leaf) = get_shortest_leaf_name(child_node) {
-                        leaf_bases.insert(field_name.clone(), leaf);
-                    }
-                }
-                if leaf_bases.len() == child_bases.len() {
-                    let leaf_analysis = analyze_instance(&leaf_bases);
-                    if !leaf_analysis.field_parts.values().all(|v| v.is_empty()) {
-                        analysis.field_parts = leaf_analysis.field_parts;
+
+                // If no fields could be filled and all children are the same type,
+                // mark as outlier so the tree inlines instead of using identity
+                // (handles patterns like period windows where field keys differ
+                // from metric suffixes: all/_4y don't match 0sd/0sd_4y).
+                // When children are different types (like absolute/rate), identity
+                // is correct — each child handles its own suffixes internally.
+                if !any_filled {
+                    let child_fields = get_node_fields(children, pattern_lookup);
+                    let all_same_type = child_fields
+                        .windows(2)
+                        .all(|w| w[0].rust_type == w[1].rust_type);
+                    if all_same_type {
+                        analysis.has_outlier = true;
                     }
                 }
             }
 
             // Store the base result for this node
-            // Note: has_outlier is false because we use recursive base computation
-            // which gives correct bases without needing outlier detection
             node_bases.insert(
                 path.to_string(),
                 PatternBaseResult {
                     base: analysis.base.clone(),
-                    has_outlier: false,
+                    has_outlier: analysis.has_outlier,
                     is_suffix_mode: analysis.is_suffix_mode,
                     field_parts: analysis.field_parts.clone(),
                 },
@@ -158,8 +278,14 @@ fn collect_instance_analyses(
                     .push(analysis.clone());
             }
 
-            // Return the base for parent
-            Some(analysis.base)
+            // Return the base for parent.
+            // For outlier nodes (no common prefix among children), return the
+            // shortest leaf name so the parent can still detect naming patterns.
+            if analysis.has_outlier {
+                Some(get_shortest_leaf_name(node).unwrap_or(analysis.base))
+            } else {
+                Some(analysis.base)
+            }
         }
     }
 }
@@ -179,14 +305,13 @@ fn try_detect_template(
         return None;
     }
 
-    // Strategy 1: Find an embedded discriminator (shortest non-empty field_part
-    // that differs between instances and appears as substring in other parts)
-    if let Some(mode) = try_embedded_disc(majority, fields) {
+    // Strategy 1: suffix discriminator (e.g., ratio_sd vs ratio_sd_4y)
+    if let Some(mode) = try_suffix_disc(majority, fields) {
         return Some(mode);
     }
 
-    // Strategy 2: Find a common suffix difference across ALL field_parts
-    try_suffix_disc(majority, fields)
+    // Strategy 2: embedded discriminator (e.g., ratio_pct99_bps vs ratio_pct1_bps)
+    try_embedded_disc(majority, fields)
 }
 
 /// Strategy 1: embedded discriminator (e.g., pct99 inside ratio_pct99_bps)
@@ -241,9 +366,11 @@ fn try_suffix_disc(
 ) -> Option<PatternMode> {
     let first = &majority[0];
 
-    // For each other instance, check if ALL field_parts differ from the first
-    // by the same suffix. Use the first field to detect the suffix.
-    let ref_field = &fields[0].name;
+    // Use a non-empty field to detect the suffix
+    let ref_field = fields
+        .iter()
+        .find(|f| first.field_parts.get(&f.name).is_some_and(|v| !v.is_empty()))
+        .map(|f| &f.name)?;
     let ref_first = first.field_parts.get(ref_field)?;
 
     // Build templates from the first instance
@@ -268,8 +395,13 @@ fn try_suffix_disc(
             let other_part = analysis.field_parts.get(&field.name)?;
 
             if first_part.is_empty() {
-                // Identity field — must stay empty in all instances
-                if !other_part.is_empty() {
+                // Identity field — must be empty OR equal to the suffix
+                if other_part.is_empty() {
+                    // stays empty — ok
+                } else if other_part == suffix {
+                    // empty in first, equals suffix in other — disc IS the part
+                    templates.insert(field.name.clone(), "{disc}".to_string());
+                } else {
                     return None;
                 }
             } else {
@@ -312,11 +444,11 @@ fn analyze_instance(child_bases: &BTreeMap<String, String>) -> InstanceAnalysis
         // period windows (all/_4y/_2y/_1y) where children differ by a suffix
         // that corresponds to the tree key.
         if field_parts.len() > 1 && field_parts.values().all(|v| v.is_empty()) {
-            // Can't differentiate — this pattern is non-parameterizable
             return InstanceAnalysis {
                 base,
                 field_parts,
                 is_suffix_mode: true,
+                has_outlier: false,
             };
         }
 
@@ -324,6 +456,7 @@ fn analyze_instance(child_bases: &BTreeMap<String, String>) -> InstanceAnalysis
             base,
             field_parts,
             is_suffix_mode: true,
+            has_outlier: false,
         };
     }
 
@@ -345,12 +478,13 @@ fn analyze_instance(child_bases: &BTreeMap<String, String>) -> InstanceAnalysis
             base,
             field_parts,
             is_suffix_mode: false,
+            has_outlier: false,
         };
     }
 
     // No common prefix or suffix - use empty base so _m(base, relative) returns just the relative.
-    // This handles cases like utxo_cohorts.all.activity where children have completely
-    // different bases (coinblocks_destroyed, coindays_destroyed, etc.)
+    // No common prefix or suffix — outlier naming (e.g., sopr/asopr/adj_).
+    // Children have unrelated metric names that can't be parameterized.
     let field_parts = child_bases
         .iter()
         .map(|(k, v)| (k.clone(), v.clone()))
@@ -360,6 +494,7 @@ fn analyze_instance(child_bases: &BTreeMap<String, String>) -> InstanceAnalysis
         base: String::new(),
         field_parts,
         is_suffix_mode: true,
+        has_outlier: true,
     }
 }
 
@@ -372,6 +507,12 @@ fn determine_pattern_mode(
 ) -> Option<PatternMode> {
     analyses.first()?;
 
+    // If any instance has outlier naming (no common prefix/suffix among children),
+    // the pattern can't be parameterized.
+    if analyses.iter().any(|a| a.has_outlier) {
+        return None;
+    }
+
     // Pick the majority mode
     let suffix_count = analyses.iter().filter(|a| a.is_suffix_mode).count();
     let is_suffix = suffix_count * 2 >= analyses.len();
@@ -521,6 +662,7 @@ mod tests {
             .into_iter()
             .collect(),
             is_suffix_mode: true,
+            has_outlier: false,
         };
         let suffix2 = InstanceAnalysis {
             base: "sth_cost_basis".to_string(),
@@ -532,6 +674,7 @@ mod tests {
             .into_iter()
             .collect(),
             is_suffix_mode: true,
+            has_outlier: false,
         };
         let suffix3 = InstanceAnalysis {
             base: "utxo_cost_basis".to_string(),
@@ -543,6 +686,7 @@ mod tests {
             .into_iter()
             .collect(),
             is_suffix_mode: true,
+            has_outlier: false,
         };
 
         // 1 prefix mode instance (minority - root level)
@@ -556,6 +700,7 @@ mod tests {
             .into_iter()
             .collect(),
             is_suffix_mode: false,
+            has_outlier: false,
         };
 
         let analyses = vec![suffix1, suffix2, suffix3, prefix1];
@@ -606,6 +751,7 @@ mod tests {
             .into_iter()
             .collect(),
             is_suffix_mode: true,
+            has_outlier: false,
         };
         let instance2 = InstanceAnalysis {
             base: "metric_b".to_string(),
@@ -616,6 +762,7 @@ mod tests {
             .into_iter()
             .collect(),
             is_suffix_mode: true,
+            has_outlier: false,
         };
 
         let analyses = vec![instance1, instance2];
@@ -631,4 +778,366 @@ mod tests {
             PatternMode::Templated { .. } => panic!("Expected suffix mode, got templated"),
         }
     }
+
+    #[test]
+    fn test_embedded_disc_percentile_bands() {
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "bps".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "price".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "ratio".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let pct99 = InstanceAnalysis {
+            base: "realized_price".into(),
+            field_parts: [("bps".into(), "ratio_pct99_bps".into()), ("price".into(), "pct99".into()), ("ratio".into(), "ratio_pct99".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let pct1 = InstanceAnalysis {
+            base: "realized_price".into(),
+            field_parts: [("bps".into(), "ratio_pct1_bps".into()), ("price".into(), "pct1".into()), ("ratio".into(), "ratio_pct1".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let mode = determine_pattern_mode(&[pct99, pct1], &fields);
+        assert!(mode.is_some());
+        match mode.unwrap() {
+            PatternMode::Templated { templates } => {
+                assert_eq!(templates.get("bps").unwrap(), "ratio_{disc}_bps");
+                assert_eq!(templates.get("price").unwrap(), "{disc}");
+                assert_eq!(templates.get("ratio").unwrap(), "ratio_{disc}");
+            }
+            other => panic!("Expected Templated, got {:?}", other),
+        }
+    }
+
+    #[test]
+    fn test_suffix_disc_period_windows() {
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "p1sd".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "sd".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "zscore".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let all_time = InstanceAnalysis {
+            base: "realized_price".into(),
+            field_parts: [("p1sd".into(), "p1sd".into()), ("sd".into(), "ratio_sd".into()), ("zscore".into(), "ratio_zscore".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let four_year = InstanceAnalysis {
+            base: "realized_price".into(),
+            field_parts: [("p1sd".into(), "p1sd_4y".into()), ("sd".into(), "ratio_sd_4y".into()), ("zscore".into(), "ratio_zscore_4y".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let mode = determine_pattern_mode(&[all_time, four_year], &fields);
+        assert!(mode.is_some());
+        match mode.unwrap() {
+            PatternMode::Templated { templates } => {
+                assert_eq!(templates.get("p1sd").unwrap(), "p1sd{disc}");
+                assert_eq!(templates.get("sd").unwrap(), "ratio_sd{disc}");
+                assert_eq!(templates.get("zscore").unwrap(), "ratio_zscore{disc}");
+            }
+            other => panic!("Expected Templated, got {:?}", other),
+        }
+    }
+
+    #[test]
+    fn test_suffix_disc_with_empty_fields() {
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "band".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "sd".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let all_time = InstanceAnalysis {
+            base: "price".into(),
+            field_parts: [("band".into(), "".into()), ("sd".into(), "ratio_sd".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let four_year = InstanceAnalysis {
+            base: "price".into(),
+            field_parts: [("band".into(), "".into()), ("sd".into(), "ratio_sd_4y".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let mode = determine_pattern_mode(&[all_time, four_year], &fields);
+        assert!(mode.is_some());
+        match mode.unwrap() {
+            PatternMode::Templated { templates } => {
+                assert_eq!(templates.get("band").unwrap(), "");
+                assert_eq!(templates.get("sd").unwrap(), "ratio_sd{disc}");
+            }
+            other => panic!("Expected Templated, got {:?}", other),
+        }
+    }
+
+    #[test]
+    fn test_suffix_disc_empty_to_nonempty() {
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "all".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "sth".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let regular = InstanceAnalysis {
+            base: "supply".into(),
+            field_parts: [("all".into(), "".into()), ("sth".into(), "sth_".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let profitability = InstanceAnalysis {
+            base: "utxos_in_profit".into(),
+            field_parts: [("all".into(), "supply".into()), ("sth".into(), "sth_supply".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let mode = determine_pattern_mode(&[regular, profitability], &fields);
+        assert!(mode.is_some());
+        match mode.unwrap() {
+            PatternMode::Templated { templates } => {
+                assert_eq!(templates.get("all").unwrap(), "{disc}");
+                assert_eq!(templates.get("sth").unwrap(), "sth_{disc}");
+            }
+            other => panic!("Expected Templated, got {:?}", other),
+        }
+    }
+
+    #[test]
+    fn test_outlier_rejects_pattern() {
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "ratio".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "value".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        // SOPR case: one instance has outlier naming (no common prefix)
+        let normal = InstanceAnalysis {
+            base: "metric".into(),
+            field_parts: [("ratio".into(), "ratio".into()), ("value".into(), "value".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let outlier = InstanceAnalysis {
+            base: "".into(),
+            field_parts: [("ratio".into(), "asopr".into()), ("value".into(), "adj_value".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: true,
+        };
+        let mode = determine_pattern_mode(&[normal, outlier], &fields);
+        assert!(mode.is_none(), "Pattern with outlier instance should be non-parameterizable");
+    }
+
+    #[test]
+    fn test_unanimity_rejects_disagreeing_instances() {
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "a".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "b".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let inst1 = InstanceAnalysis {
+            base: "x".into(),
+            field_parts: [("a".into(), "foo".into()), ("b".into(), "bar".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let inst2 = InstanceAnalysis {
+            base: "y".into(),
+            field_parts: [("a".into(), "baz".into()), ("b".into(), "qux".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let mode = determine_pattern_mode(&[inst1, inst2], &fields);
+        assert!(mode.is_none(), "Should be non-parameterizable when no pattern detected");
+    }
+
+    #[test]
+    fn test_all_empty_different_types_uses_identity() {
+        // AbsoluteRatePattern: absolute (_1m1w1y24hPattern) and rate (_1m1w1y24hPattern2)
+        // have different types. Both return the same base → all-empty field_parts.
+        // Should keep identity (empty parts) so both children receive acc unchanged.
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "absolute".into(), rust_type: "TypeA".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            PatternField { name: "rate".into(), rust_type: "TypeB".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let inst = InstanceAnalysis {
+            base: "supply_delta".into(),
+            field_parts: [("absolute".into(), "".into()), ("rate".into(), "".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: false,
+        };
+        let mode = determine_pattern_mode(&[inst], &fields);
+        assert!(mode.is_some());
+        match mode.unwrap() {
+            PatternMode::Suffix { relatives } => {
+                assert_eq!(relatives.get("absolute"), Some(&"".to_string()), "absolute should be identity");
+                assert_eq!(relatives.get("rate"), Some(&"".to_string()), "rate should be identity");
+            }
+            other => panic!("Expected Suffix with identity, got {:?}", other),
+        }
+    }
+
+    #[test]
+    fn test_all_empty_same_type_marks_outlier() {
+        // RatioPerBlockStdDevBands: all children are the same type (StdDevPerBlockExtended)
+        // and all return the same base → all-empty field_parts.
+        // Should be marked as outlier so the tree inlines instead of using a
+        // factory that can't differentiate the children.
+        let mut child_bases = BTreeMap::new();
+        child_bases.insert("all".to_string(), "realized_price".to_string());
+        child_bases.insert("_4y".to_string(), "realized_price".to_string());
+        child_bases.insert("_2y".to_string(), "realized_price".to_string());
+        child_bases.insert("_1y".to_string(), "realized_price".to_string());
+
+        let analysis = analyze_instance(&child_bases);
+
+        assert_eq!(analysis.base, "realized_price");
+        assert!(
+            analysis.field_parts.values().all(|v| v.is_empty()),
+            "All field_parts should be empty when children return same base"
+        );
+        // Note: has_outlier is set by collect_instance_analyses based on
+        // all_same_type check, not by analyze_instance directly.
+        // The test for outlier detection is via determine_pattern_mode
+        // with has_outlier flag set.
+    }
+
+    #[test]
+    fn test_non_parameterizable_cascade() {
+        // When a pattern has outlier instances, determine_pattern_mode returns None.
+        // Parent patterns containing non-parameterizable children should also
+        // be detected via metadata.is_parameterizable (recursive check).
+        use std::collections::BTreeSet;
+        let fields = vec![
+            PatternField { name: "a".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+        ];
+        let inst = InstanceAnalysis {
+            base: "".into(),
+            field_parts: [("a".into(), "standalone_name".into())].into_iter().collect(),
+            is_suffix_mode: true, has_outlier: true,
+        };
+        let mode = determine_pattern_mode(&[inst], &fields);
+        assert!(mode.is_none(), "Pattern with outlier should be non-parameterizable");
+    }
+
+    #[test]
+    fn test_extract_disc_from_instance() {
+        // StdDevPerBlockExtended 4y instance: field_parts include "0sd_4y", "p1sd_4y", "ratio_sd_4y".
+        // Templates are "0sd{disc}", "p1sd{disc}", "ratio_sd{disc}".
+        // The extracted disc should be "_4y", not "0sd_4y" (the shortest field_part).
+        use crate::StructuralPattern;
+        use std::collections::BTreeSet;
+
+        let pattern = StructuralPattern {
+            name: "TestPattern".into(),
+            fields: vec![
+                PatternField { name: "_0sd".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+                PatternField { name: "p1sd".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+                PatternField { name: "sd".into(), rust_type: "T".into(), json_type: "n".into(), indexes: BTreeSet::new(), type_param: None },
+            ],
+            mode: Some(PatternMode::Templated {
+                templates: [
+                    ("_0sd".into(), "0sd{disc}".into()),
+                    ("p1sd".into(), "p1sd{disc}".into()),
+                    ("sd".into(), "ratio_sd{disc}".into()),
+                ]
+                .into_iter()
+                .collect(),
+            }),
+            is_generic: false,
+        };
+
+        // 4y instance
+        let field_parts_4y: BTreeMap<String, String> = [
+            ("_0sd".into(), "0sd_4y".into()),
+            ("p1sd".into(), "p1sd_4y".into()),
+            ("sd".into(), "ratio_sd_4y".into()),
+        ]
+        .into_iter()
+        .collect();
+
+        let disc = pattern.extract_disc_from_instance(&field_parts_4y);
+        assert_eq!(disc, Some("4y".to_string()));
+
+        // All-time instance (no period suffix)
+        let field_parts_all: BTreeMap<String, String> = [
+            ("_0sd".into(), "0sd".into()),
+            ("p1sd".into(), "p1sd".into()),
+            ("sd".into(), "ratio_sd".into()),
+        ]
+        .into_iter()
+        .collect();
+
+        let disc = pattern.extract_disc_from_instance(&field_parts_all);
+        assert_eq!(disc, Some(String::new()));
+    }
+
+    #[test]
+    fn test_mixed_empty_fills_with_longer_suffix() {
+        // CapLossMvrvNetPriceProfitSoprPattern: "loss" field is empty but its
+        // shortest leaf is "realized_loss" which contains "loss" and is longer.
+        // Should fill with "realized_loss". But "supply" field whose suffix equals
+        // the field name exactly should NOT be filled (identity).
+        let mut child_bases = BTreeMap::new();
+        child_bases.insert("cap".to_string(), "utxos_realized_cap".to_string());
+        child_bases.insert("loss".to_string(), "utxos".to_string()); // returns parent base
+        child_bases.insert("mvrv".to_string(), "utxos_mvrv".to_string());
+        child_bases.insert("price".to_string(), "utxos_realized_price".to_string());
+        child_bases.insert("supply".to_string(), "utxos".to_string()); // returns parent base
+
+        let analysis = analyze_instance(&child_bases);
+        assert_eq!(analysis.base, "utxos");
+
+        // loss and supply should be empty from common prefix analysis
+        assert_eq!(analysis.field_parts.get("loss"), Some(&"".to_string()));
+        assert_eq!(analysis.field_parts.get("supply"), Some(&"".to_string()));
+        // others should be non-empty
+        assert_eq!(analysis.field_parts.get("cap"), Some(&"realized_cap".to_string()));
+        assert_eq!(analysis.field_parts.get("mvrv"), Some(&"mvrv".to_string()));
+        assert_eq!(analysis.field_parts.get("price"), Some(&"realized_price".to_string()));
+    }
+
+    #[test]
+    fn test_mixed_empty_fills_loss_from_shortest_leaf() {
+        // Integration test: "loss" child returns same base as parent (because
+        // its children like neg_realized_loss break the prefix). The mixed-empty
+        // fix should fill it from shortest leaf "utxos_realized_loss".
+        use brk_types::{MetricLeaf, MetricLeafWithSchema, TreeNode};
+
+        fn leaf(name: &str) -> TreeNode {
+            TreeNode::Leaf(MetricLeafWithSchema::new(
+                MetricLeaf::new(name.into(), "f32".into(), std::collections::BTreeSet::new()),
+                serde_json::Value::Null,
+            ))
+        }
+
+        let parent = TreeNode::Branch(
+            [
+                ("cap".into(), leaf("utxos_realized_cap")),
+                (
+                    "loss".into(),
+                    TreeNode::Branch(
+                        [
+                            ("base".into(), leaf("utxos_realized_loss")),
+                            ("negative".into(), leaf("utxos_neg_realized_loss")),
+                        ]
+                        .into_iter()
+                        .collect(),
+                    ),
+                ),
+                ("mvrv".into(), leaf("utxos_mvrv")),
+            ]
+            .into_iter()
+            .collect(),
+        );
+
+        let mut all_analyses = BTreeMap::new();
+        let mut node_bases = BTreeMap::new();
+        let pattern_lookup = BTreeMap::new();
+
+        collect_instance_analyses(
+            &parent,
+            "test",
+            &pattern_lookup,
+            &mut all_analyses,
+            &mut node_bases,
+        );
+
+        let result = node_bases.get("test").expect("should have node_bases entry");
+        assert_eq!(result.base, "utxos");
+        assert!(!result.has_outlier);
+        assert_eq!(result.field_parts.get("cap"), Some(&"realized_cap".to_string()));
+        assert_eq!(result.field_parts.get("mvrv"), Some(&"mvrv".to_string()));
+        // loss stays empty after first pass (child returned same base as parent).
+        // The second pass (fill_mixed_empty_field_parts) fills it for templated
+        // children, but that requires pattern_lookup which this test doesn't set up.
+        assert_eq!(result.field_parts.get("loss"), Some(&"".to_string()));
+    }
 }