fix: honor ValidModifierClass subset; inherit usage type; harden ordering

- Fix 1: capture <ValidModifierClass> codes from <ModifiedBy all="false">
  and filter each modifier level to only valid codes before cartesian
  product; eliminates ~194 phantom codes (e.g. M07.01–03) while keeping
  valid ones (M07.00/04/07/09). Split Start/Empty XML event arms to
  correctly track children of <ModifiedBy>.
- Fix 2: synthesized para295/para301 inherit parent values; "V" is
  promoted to "P" (parent "V" = needs subdivision, terminal is codeable).
  Other types ("P", "O", "Z", "") pass through unchanged.
- Fix 3: debug_assert in ModifierClass handler guards against ClaML
  document-order assumption changing silently in debug/test builds.
- Fix 4: add expands_single_modifier_chain regression test verifying
  C88.00/C88.01 synthesis from a single all="true" ModifiedBy (corpus-
  verified: Alpha-ID I30531, I31044).

src/claml.rs

@@ -26,12 +26,23 @@ struct ModifierClassDef {
     label: String,
 }
 
+/// One `<ModifiedBy>` reference on a category `<Class>`, including the
+/// optional `<ValidModifierClass>` subset restriction.
+struct ModifiedByRef {
+    /// The modifier code (value of the `code` attribute).
+    code: String,
+    /// When `all="false"` and `valid` is non-empty, only the listed
+    /// `<ModifierClass code=…>` values are allowed for this parent.
+    /// When `all="true"` or `valid` is empty, all ModifierClass codes apply.
+    valid: Vec<String>,
+}
+
 /// Records, for one `<Class kind="category">`, the ordered list of
-/// `<ModifiedBy code=…>` references so the modifier chain can be expanded
+/// `<ModifiedBy>` references so the modifier chain can be expanded
 /// after the whole document has been parsed.
 struct ModifiedByRefs {
     parent_code: String,
-    refs: Vec<String>,
+    refs: Vec<ModifiedByRef>,
 }
 
 /// Stream-parse the ClaML XML into a map keyed by normalised ICD code.
@@ -74,8 +85,10 @@ pub fn load(path: &str) -> Result<HashMap<String, IcdMeta>, AppError> {
     // ModifiedBy refs collected per category Class (in document order).
     let mut modified_by: Vec<ModifiedByRefs> = Vec::new();
     // Refs accumulating for the currently-open category Class.
-    let mut cur_refs: Vec<String> = Vec::new();
+    let mut cur_refs: Vec<ModifiedByRef> = Vec::new();
     let mut cur_code = String::new();
+    // Currently-open <ModifiedBy> element (Start variant only, not self-closing).
+    let mut cur_mb: Option<ModifiedByRef> = None;
     // Currently-open <ModifierClass>, if any (these are NOT category Classes,
     // so `cur` stays None and the normal description path is bypassed).
     let mut cur_mc: Option<ModifierClassDef> = None;
@@ -86,7 +99,111 @@ pub fn load(path: &str) -> Result<HashMap<String, IcdMeta>, AppError> {
             Err(e) => return Err(AppError::Parse(format!("claml: {e}"))),
             Ok(Event::Eof) => break,
 
-            Ok(Event::Start(ref t)) | Ok(Event::Empty(ref t)) => {
+            // Self-closing / empty elements.
+            Ok(Event::Empty(ref t)) => {
+                match t.name().as_ref() {
+                    b"Class" => {
+                        // Self-closing <Class/> rarely appears; handle for completeness.
+                        let mut code = String::new();
+                        cur_kind.clear();
+                        for a in t.attributes().flatten() {
+                            match a.key.as_ref() {
+                                b"code" => code = String::from_utf8_lossy(&a.value).into_owned(),
+                                b"kind" => cur_kind = String::from_utf8_lossy(&a.value).into_owned(),
+                                _ => {}
+                            }
+                        }
+                        if cur_kind == "chapter" {
+                            if let Some(padded) = roman_to_padded(&code) {
+                                current_chapter = padded;
+                            }
+                            cur = None;
+                        } else if cur_kind == "category" {
+                            cur_code = code.clone();
+                            cur_refs.clear();
+                            let m = IcdMeta {
+                                code,
+                                chapter: current_chapter.clone(),
+                                ..IcdMeta::default()
+                            };
+                            cur = Some(m);
+                        } else {
+                            cur = None;
+                        }
+                    }
+                    b"ModifiedBy" => {
+                        // Self-closing <ModifiedBy/> always means all="true" (no restriction).
+                        if cur.is_some() {
+                            let mut mb_code = String::new();
+                            for a in t.attributes().flatten() {
+                                if a.key.as_ref() == b"code" {
+                                    mb_code = String::from_utf8_lossy(&a.value).into_owned();
+                                }
+                            }
+                            if !mb_code.is_empty() {
+                                cur_refs.push(ModifiedByRef { code: mb_code, valid: Vec::new() });
+                            }
+                        }
+                    }
+                    b"ValidModifierClass" => {
+                        // Child of <ModifiedBy all="false">: collect the valid code.
+                        if let Some(mb) = cur_mb.as_mut() {
+                            for a in t.attributes().flatten() {
+                                if a.key.as_ref() == b"code" {
+                                    mb.valid.push(String::from_utf8_lossy(&a.value).into_owned());
+                                }
+                            }
+                        }
+                    }
+                    b"SuperClass" => {
+                        if let Some(m) = cur.as_mut() {
+                            for a in t.attributes().flatten() {
+                                if a.key.as_ref() == b"code" {
+                                    let sup = String::from_utf8_lossy(&a.value);
+                                    if m.group.is_empty() {
+                                        m.group = sup.into_owned();
+                                    }
+                                }
+                            }
+                        }
+                    }
+                    b"Meta" => {
+                        if let Some(m) = cur.as_mut() {
+                            let mut k = String::new();
+                            let mut v = String::new();
+                            for a in t.attributes().flatten() {
+                                match a.key.as_ref() {
+                                    b"name" => k = String::from_utf8_lossy(&a.value).into_owned(),
+                                    b"value" => v = String::from_utf8_lossy(&a.value).into_owned(),
+                                    _ => {}
+                                }
+                            }
+                            match k.as_str() {
+                                "Para295" => m.para295 = v,
+                                "Para301" => m.para301 = v,
+                                "Exotic" => m.exotic = v == "J",
+                                "Infectious" => m.ifsg = v == "J",
+                                "Content" => m.content = v == "J",
+                                "chapter" => m.chapter = v,
+                                _ => {}
+                            }
+                        }
+                    }
+                    b"Rubric" => {
+                        let mut rubric_kind = String::new();
+                        for a in t.attributes().flatten() {
+                            if a.key.as_ref() == b"kind" {
+                                rubric_kind = String::from_utf8_lossy(&a.value).into_owned();
+                            }
+                        }
+                        in_preferred = rubric_kind == "preferred";
+                    }
+                    _ => {}
+                }
+            }
+
+            // Opening tags (with a matching closing tag).
+            Ok(Event::Start(ref t)) => {
                 let name = t.name();
                 match name.as_ref() {
                     b"Class" => {
@@ -119,6 +236,11 @@ pub fn load(path: &str) -> Result<HashMap<String, IcdMeta>, AppError> {
                         }
                     }
                     b"ModifierClass" => {
+                        // Fix 3: assert ordering assumption in debug/test builds.
+                        debug_assert!(
+                            cur.is_none(),
+                            "ClaML ordering changed: ModifierClass appeared inside/after a category Class"
+                        );
                         let mut mc = ModifierClassDef::default();
                         cur_mc_modifier.clear();
                         for a in t.attributes().flatten() {
@@ -135,13 +257,47 @@ pub fn load(path: &str) -> Result<HashMap<String, IcdMeta>, AppError> {
                         cur = None;
                     }
                     b"ModifiedBy" => {
-                        // Only meaningful while a category Class is open.
+                        // Opening <ModifiedBy> may have <ValidModifierClass> children.
                         if cur.is_some() {
+                            let mut mb_code = String::new();
+                            let mut all_true = true;
+                            for a in t.attributes().flatten() {
+                                match a.key.as_ref() {
+                                    b"code" => mb_code = String::from_utf8_lossy(&a.value).into_owned(),
+                                    b"all" => all_true = &*a.value != b"false",
+                                    _ => {}
+                                }
+                            }
+                            if !mb_code.is_empty() {
+                                // Store in cur_mb; ValidModifierClass children will populate valid.
+                                // On End(ModifiedBy) we'll push it to cur_refs.
+                                cur_mb = Some(ModifiedByRef {
+                                    code: mb_code,
+                                    // Pre-populate as empty; if all="false" the End handler
+                                    // will use whatever ValidModifierClass children collected.
+                                    // If all="true", valid stays empty = unrestricted.
+                                    valid: Vec::new(),
+                                });
+                                // If all="true", mark it as unrestricted even if no children.
+                                // We track this implicitly: valid is empty ↔ unrestricted.
+                                // But for all="true" we still want to keep it empty.
+                                // For all="false", children will push codes into mb.valid.
+                                // So no extra flag needed; empty valid = unrestricted.
+                                // However, if all="false" but NO ValidModifierClass children
+                                // appeared (unusual but possible), empty valid = unrestricted,
+                                // which is the safest fallback (full set).
+                                if all_true {
+                                    // Explicitly mark unrestricted; empty valid is the signal.
+                                } // else: children will fill valid; if none appear → unrestricted.
+                            }
+                        }
+                    }
+                    b"ValidModifierClass" => {
+                        // Child of <ModifiedBy all="false">: collect the valid code.
+                        if let Some(mb) = cur_mb.as_mut() {
                             for a in t.attributes().flatten() {
                                 if a.key.as_ref() == b"code" {
-                                    cur_refs.push(
+                                    mb.valid.push(String::from_utf8_lossy(&a.value).into_owned());
-                                        String::from_utf8_lossy(&a.value).into_owned(),
-                                    );
                                 }
                             }
                         }
@@ -207,6 +363,14 @@ pub fn load(path: &str) -> Result<HashMap<String, IcdMeta>, AppError> {
 
             Ok(Event::End(ref t)) => match t.name().as_ref() {
                 b"Rubric" => in_preferred = false,
+                b"ModifiedBy" => {
+                    // Closing tag: push the accumulated ref (if any) to cur_refs.
+                    if let Some(mb) = cur_mb.take() {
+                        if cur.is_some() {
+                            cur_refs.push(mb);
+                        }
+                    }
+                }
                 b"Class" => {
                     if let Some(m) = cur.take() {
                         if !m.code.is_empty() {
@@ -258,10 +422,18 @@ pub fn load(path: &str) -> Result<HashMap<String, IcdMeta>, AppError> {
 /// ICD code (e.g. `E11` + `.9` + `0` = `E11.90`, `I10.0` + `1` = `I10.01`),
 /// which exactly matches the Alpha-ID corpus code format.
 ///
+/// When a `<ModifiedBy>` ref carries a non-empty `valid` set (sourced from
+/// `<ValidModifierClass>` children with `all="false"`), only those specific
+/// `<ModifierClass>` codes are included for that level of the cartesian product;
+/// the full modifier list is used when the set is empty (i.e. `all="true"`).
+///
 /// Only the terminal leaf (every modifier in the chain applied) is emitted as
-/// a billable code (`Para295`/`Para301` = `"P"`); intermediate prefixes such as
+/// a synthesized code.  Its `Para295`/`Para301` values are inherited from the
-/// the bare 3-steller keep whatever the explicit `<Class>` said (typically
+/// parent, with `"V"` promoted to `"P"` (the parent carries `"V"` precisely
-/// `"V"`).  The description is the parent's preferred label followed by each
+/// because only the subdivided terminal is codeable).  Other types (`"P"`,
+/// `"O"`, `"Z"`, `""`) are passed through unchanged.
+///
+/// The description is the parent's preferred label followed by each
 /// `<ModifierClass>` label, joined by " - " (empty labels skipped).
 ///
 /// Purely additive: a key already present (produced by an explicit `<Class>`)
@@ -273,12 +445,31 @@ fn expand_modifiers(
 ) {
     for mb in modified_by {
         // Resolve the chain: each ModifiedBy ref => that modifier's ordered
-        // ModifierClass list.  Skip the whole Class if any ref is unknown.
+        // ModifierClass list (filtered to the valid subset when applicable).
-        let mut chain: Vec<&Vec<ModifierClassDef>> = Vec::with_capacity(mb.refs.len());
+        // Skip the whole Class if any ref is unknown.
+        //
+        // We store owned filtered lists rather than references, because the
+        // valid-subset filter may need to exclude entries from the full list.
+        let mut chain: Vec<Vec<&ModifierClassDef>> = Vec::with_capacity(mb.refs.len());
         let mut resolvable = true;
         for r in &mb.refs {
-            match modifier_classes.get(r) {
+            match modifier_classes.get(&r.code) {
-                Some(list) if !list.is_empty() => chain.push(list),
+                Some(list) if !list.is_empty() => {
+                    // Fix 1: if a non-empty valid set is given (all="false"),
+                    // restrict this level to only those ModifierClass codes.
+                    let filtered: Vec<&ModifierClassDef> = if r.valid.is_empty() {
+                        // all="true" or no restriction: use everything.
+                        list.iter().collect()
+                    } else {
+                        // all="false": keep only the listed valid codes.
+                        list.iter().filter(|mc| r.valid.contains(&mc.code)).collect()
+                    };
+                    if filtered.is_empty() {
+                        resolvable = false;
+                        break;
+                    }
+                    chain.push(filtered);
+                }
                 _ => {
                     resolvable = false;
                     break;
@@ -301,6 +492,19 @@ fn expand_modifiers(
         let parent_ifsg = parent.ifsg;
         let parent_content = parent.content;
 
+        // Fix 2: inherit para295/para301 from parent; promote "V" → "P"
+        // ("V" means "needs subdivision" — only the terminal leaf is codeable).
+        let synth_para295 = if parent.para295 == "V" {
+            "P".to_string()
+        } else {
+            parent.para295.clone()
+        };
+        let synth_para301 = if parent.para301 == "V" {
+            "P".to_string()
+        } else {
+            parent.para301.clone()
+        };
+
         // Cartesian product over the chain, in ModifiedBy (digit) order.
         // `acc` holds (concatenated-code-suffix, composed-label-tail) pairs.
         let mut acc: Vec<(String, String)> = vec![(String::new(), String::new())];
@@ -344,8 +548,8 @@ fn expand_modifiers(
                     description,
                     chapter: parent_chapter.clone(),
                     group: parent_group.clone(),
-                    para295: "P".to_string(),
+                    para295: synth_para295.clone(),
-                    para301: "P".to_string(),
+                    para301: synth_para301.clone(),
                     exotic: parent_exotic,
                     ifsg: parent_ifsg,
                     content: parent_content,

tests/claml_modifier_tests.rs

@@ -14,3 +14,39 @@ fn expands_diabetes_5th_digit_codes_as_billable() {
     assert!(a010.description.contains("Typhus abdominalis"));
     assert_eq!(map.get("E11").expect("E11 root").para295, "V");
 }
+
+/// Regression test: a category with a single `<ModifiedBy all="true">` expands
+/// correctly into its terminal 5th-digit leaves.
+///
+/// `C88.0` (Makroglobulinämie Waldenström) has exactly one `<ModifiedBy
+/// all="true" code="S02C88_5"/>` with two `<ModifierClass>` codes ("0" and
+/// "1"), yielding `C88.00` and `C88.01`.  Both codes appear in the
+/// Alpha-ID corpus (`data/icd10gm2026_alphaidse_edvtxt_20250926.txt`).
+///
+/// The parent `C88.0` carries `Para295 = "P"` (already billable at the 4th
+/// digit) and `Para301 = "V"`.  The synthesized terminals therefore inherit
+/// `para295 = "P"` (parent value, not "V") and `para301 = "P"` ("V" promoted).
+#[test]
+fn expands_single_modifier_chain() {
+    let map = claml::load("icd-claml/Klassifikationsdateien/icd10gm2026syst_claml_20250912.xml").unwrap();
+
+    // C88.00 – verified in corpus (e.g. Alpha-ID I30531 "Makroglobulinämie Waldenström").
+    let c8800 = map.get("C88.00").expect("C88.00 synthesized from single ModifiedBy");
+    assert_eq!(c8800.para295, "P",
+        "para295 inherited from parent C88.0 (which is already P, not V)");
+    // Parent label + modifier label joined by " - ".
+    assert!(c8800.description.contains("Waldenström"),
+        "description must contain parent label: {:?}", c8800.description);
+    assert!(c8800.description.contains("Remission"),
+        "description must contain modifier label: {:?}", c8800.description);
+
+    // C88.01 – verified in corpus (Alpha-ID I31044 "Makroglobulinämie in kompletter Remission").
+    let c8801 = map.get("C88.01").expect("C88.01 synthesized");
+    assert_eq!(c8801.para295, "P");
+    assert!(c8801.description.contains("kompletter Remission"),
+        "description must include modifier label: {:?}", c8801.description);
+
+    // The 4th-digit parent itself must still be present and unchanged.
+    let c880 = map.get("C88.0").expect("C88.0 explicit parent still present");
+    assert_eq!(c880.para295, "P", "explicit parent entry must not be overwritten");
+}