From 58f282ab0a7d4a04795b11e2b7c28b1516b174dd Mon Sep 17 00:00:00 2001 From: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com> Date: Sun, 23 Aug 2026 13:07:30 +0000 Subject: [PATCH 1/2] fix(docx): continue counters across numIds that share an abstract (#96) Word treats every w:num referencing one w:abstractNum as a single logical list; startOverride exists precisely to break away from it. Counters were keyed per numId instead, so documents whose outline is split across several instances of one abstract restarted at every switch: headings came out as Section 1, 2, 1(!), 1.1(!), 3 ... exactly the reported multiple-section-1 / 4.x-to-1.1 symptom in #96. Reapplying the paragraph style clears it because it drops the direct numPr pointing at the second instance, collapsing the document back to one numId. Counters are now keyed by the resolved abstract definition. Entering a list through an instance that carries lvlOverride restarts exactly its overridden levels (nested w:lvl or startOverride); everything else keeps counting. The real-world text.docx fixture shows why this direction is right: consecutive items use numId 5 then numId 4 and the source itself labels them four, five -- previously rendered as IV, I. Snapshot updates: handmade-numbering's "Two-one independent counter" now continues the shared sequence (5.) rather than restarting (1.), and the style-referenced paragraph follows the merged sequence (8.). The override-driven cases (10., 7)) still restart as before. If the intent was that bare shared instances stay independent in anydoc despite Word, this is easy to flip behind a flag -- but Word parity is what #96 asks for. --- src/formats/docx/mod.rs | 41 +++++++++++ src/formats/docx/numbering.rs | 68 +++++++++++++++++-- ...pshots__docx__handmade-numbering.docx.snap | 4 +- .../snapshots/snapshots__docx__text.docx.snap | 2 +- ...malformed__corrupt-styles--skips.docx.snap | 2 +- ...malformed__missing-styles--skips.docx.snap | 2 +- 6 files changed, 107 insertions(+), 12 deletions(-) diff --git a/src/formats/docx/mod.rs b/src/formats/docx/mod.rs index eee5317e..02c2e801 100644 --- a/src/formats/docx/mod.rs +++ b/src/formats/docx/mod.rs @@ -191,6 +191,47 @@ mod tests { format!(r#"{para}{para}"#) } + #[test] + fn instances_sharing_an_abstract_continue_one_sequence() { + // #96: Word treats every w:num that references one w:abstractNum as + // the same logical list; converters emit exactly this shape for a + // single outline split across several numIds. Counters must continue + // across the instance switches, and a startOverride must restart. + let document = r#" + + one + two + three + ten + "#; + let numbering = r#" + + + + + + + + + "#; + let bytes = + docx_parts(&[("word/document.xml", document), ("word/numbering.xml", numbering)]); + let doc = parse(&bytes).unwrap(); + let starts: Vec = doc + .blocks + .iter() + .filter_map(|b| match b { + Block::List(list) if list.ordered() => Some(list.start), + _ => None, + }) + .collect(); + assert_eq!( + starts, + [1, 3, 10], + "instance 2 continues at three; the override restarts at ten" + ); + } + #[test] fn huge_numbering_start_values_cannot_overflow() { // H2: w:start is ST_DecimalNumber (xsd:int); out-of-range values are diff --git a/src/formats/docx/numbering.rs b/src/formats/docx/numbering.rs index a6334008..bb892cd4 100644 --- a/src/formats/docx/numbering.rs +++ b/src/formats/docx/numbering.rs @@ -46,6 +46,15 @@ struct AbstractNum { pub struct Instance { pub levels: [LevelDef; LEVELS], pstyles: [Option; LEVELS], + /// Identity of the abstract definition this instance resolves to (after + /// any numStyleLink indirection). Instances sharing an abstract are one + /// logical list in Word, so their counters continue across instance + /// switches (#96). + pub(crate) abstract_key: u64, + /// Levels this instance overrides (`lvlOverride` with a nested `w:lvl` or + /// a `startOverride`). Entering the list through such an instance restarts + /// exactly those levels; every other level keeps counting (#96). + pub(crate) overrides: [bool; LEVELS], } impl Instance { @@ -104,24 +113,39 @@ pub fn parse( direct.insert(num_id, (abs_id, num)); } + // Stable identity per abstract definition (document order). Counters key + // on this so instances that share an abstract continue one logical list. + let mut abstract_keys: HashMap<&str, u64> = HashMap::new(); + for abs in root.find_all(ns::W, "abstractNum") { + let Some(id) = abs.attr(ns::W, "abstractNumId") else { continue }; + let key = abstract_keys.len() as u64; + abstract_keys.entry(id).or_insert(key); + } + let mut numbering = Numbering::default(); for (&num_id, &(abs_id, num_elem)) in &direct { - let Some(abs) = resolve_abstract(abs_id, &abstracts, &direct, style_num_id)? else { + let Some((resolved_id, abs)) = resolve_abstract(abs_id, &abstracts, &direct, style_num_id)? + else { log::warn!("numbering instance {num_id} references unknown abstract {abs_id:?}"); continue; }; let mut levels = abs.levels.clone(); let mut pstyles = abs.pstyles.clone(); + let mut overrides = [false; LEVELS]; for over in num_elem.find_all(ns::W, "lvlOverride") { let ilvl: usize = over.attr(ns::W, "ilvl").and_then(|v| v.parse().ok()).unwrap_or(0); if ilvl >= LEVELS { continue; } // A nested w:lvl replaces the level wholesale; startOverride is - // applied last so it survives the replacement. + // applied last so it survives the replacement. Both count as an + // override: entering the list through this instance restarts the + // level instead of continuing the shared sequence (#96). + let mut overridden = false; if let Some(lvl) = over.find(ns::W, "lvl") { levels[ilvl] = parse_level(lvl); pstyles[ilvl] = level_pstyle(lvl); + overridden = true; } if let Some(start) = over .find(ns::W, "startOverride") @@ -129,9 +153,19 @@ pub fn parse( .and_then(parse_start) { levels[ilvl].start = start; + overridden = true; } + overrides[ilvl] = overridden; } - numbering.instances.insert(num_id, Instance { levels, pstyles }); + numbering.instances.insert( + num_id, + Instance { + levels, + pstyles, + abstract_key: abstract_keys.get(resolved_id.as_str()).copied().unwrap_or(u64::MAX), + overrides, + }, + ); } Ok(numbering) } @@ -143,7 +177,7 @@ fn resolve_abstract<'n>( abstracts: &'n HashMap<&str, AbstractNum>, direct: &HashMap, style_num_id: &impl Fn(&str) -> Option, -) -> Result, ConvertError> { +) -> Result, ConvertError> { let mut seen: Vec = Vec::new(); let mut current = abs_id.to_string(); loop { @@ -157,14 +191,14 @@ fn resolve_abstract<'n>( return Ok(None); }; let Some(style_id) = &abs.num_style_link else { - return Ok(Some(abs)); + return Ok(Some((current.clone(), abs))); }; let linked = style_num_id(style_id) .and_then(|num_id| direct.get(&num_id)) .map(|(abs_id, _)| abs_id.to_string()); match linked { Some(next) => current = next, - None => return Ok(Some(abs)), + None => return Ok(Some((current.clone(), abs))), } } } @@ -226,6 +260,11 @@ struct InstanceState { value: [u64; LEVELS], initialized: [bool; LEVELS], restart_pending: [bool; LEVELS], + /// The instance currently driving this abstract (`u64::MAX` = none yet; + /// real `numId`s never reach the counters as `u64::MAX`). Switching + /// instances inside one logical list restarts only the levels the + /// entering instance overrides (#96). + last_num: u64, } impl Counters { @@ -234,7 +273,22 @@ impl Counters { /// reproducible from the marker kind alone, the composite label. pub fn next(&mut self, num_id: u64, ilvl: usize, instance: &Instance) -> (u64, Option) { let ilvl = ilvl.min(LEVELS - 1); - let state = self.state.entry(num_id).or_default(); + let state = self + .state + .entry(instance.abstract_key) + .or_insert_with(|| InstanceState { last_num: u64::MAX, ..Default::default() }); + if state.last_num != num_id { + // First paragraph of this list, or an instance switch within it: + // overridden levels restart, everything else keeps counting. + if state.last_num != u64::MAX { + for (l, overridden) in instance.overrides.iter().enumerate() { + if *overridden { + state.restart_pending[l] = true; + } + } + } + state.last_num = num_id; + } let def = &instance.levels[ilvl]; if !state.initialized[ilvl] || state.restart_pending[ilvl] { state.value[ilvl] = def.start; diff --git a/tests/snapshots/snapshots__docx__handmade-numbering.docx.snap b/tests/snapshots/snapshots__docx__handmade-numbering.docx.snap index 6a19b3e2..4a760316 100644 --- a/tests/snapshots/snapshots__docx__handmade-numbering.docx.snap +++ b/tests/snapshots/snapshots__docx__handmade-numbering.docx.snap @@ -18,7 +18,7 @@ Interruption paragraph. 4. One-four continues the count -1. Two-one independent counter +5. Two-one independent counter 10. Ten-start via override @@ -28,7 +28,7 @@ Interruption paragraph. Suppressed numbering paragraph -5. Style-numbered paragraph +8. Style-numbered paragraph 1. pStyle-bound level one diff --git a/tests/snapshots/snapshots__docx__text.docx.snap b/tests/snapshots/snapshots__docx__text.docx.snap index 655b643e..2eee1b87 100644 --- a/tests/snapshots/snapshots__docx__text.docx.snap +++ b/tests/snapshots/snapshots__docx__text.docx.snap @@ -28,7 +28,7 @@ Interrupting paragraph between lists. - IV. Roman starting at four -- I. Roman five +- V. Roman five - Bullet one diff --git a/tests/snapshots/snapshots__malformed__corrupt-styles--skips.docx.snap b/tests/snapshots/snapshots__malformed__corrupt-styles--skips.docx.snap index 750148a5..1ce7a0ab 100644 --- a/tests/snapshots/snapshots__malformed__corrupt-styles--skips.docx.snap +++ b/tests/snapshots/snapshots__malformed__corrupt-styles--skips.docx.snap @@ -28,7 +28,7 @@ Interrupting paragraph between lists. - IV. Roman starting at four -- I. Roman five +- V. Roman five - Bullet one diff --git a/tests/snapshots/snapshots__malformed__missing-styles--skips.docx.snap b/tests/snapshots/snapshots__malformed__missing-styles--skips.docx.snap index 750148a5..1ce7a0ab 100644 --- a/tests/snapshots/snapshots__malformed__missing-styles--skips.docx.snap +++ b/tests/snapshots/snapshots__malformed__missing-styles--skips.docx.snap @@ -28,7 +28,7 @@ Interrupting paragraph between lists. - IV. Roman starting at four -- I. Roman five +- V. Roman five - Bullet one From 52dfa5ebb8082a3343735793f409af24ae914abe Mon Sep 17 00:00:00 2001 From: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:59:58 +0000 Subject: [PATCH 2/2] fix(docx): per-instance override restarts without sentinel collision Cubic review follow-ups on #129, both real: - last_num is now Option: a document may legally carry numId=18446744073709551615 (xsd:int clamps were only applied to start values), and a u64::MAX sentinel would have swallowed the instance switch that triggers an override restart. - Overridden levels restart on their first USE under the entering instance (fresh_overrides) instead of eagerly marking pending bits at the switch. Eager marking let a following plain instance consume a restart it never earned: n1(1), n2[startOverride](unused at L0), n3 plain restarted to 1 instead of continuing at 2. Regression test walks nested lists and asserts the continuation. --- src/formats/docx/mod.rs | 45 +++++++++++++++++++++++++++++++++++ src/formats/docx/numbering.rs | 34 +++++++++++++------------- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/formats/docx/mod.rs b/src/formats/docx/mod.rs index 02c2e801..7911cabf 100644 --- a/src/formats/docx/mod.rs +++ b/src/formats/docx/mod.rs @@ -232,6 +232,51 @@ mod tests { ); } + #[test] + fn override_restart_does_not_leak_into_the_next_instance() { + // Entering an overriding instance schedules ITS levels' restarts; a + // following plain instance of the same abstract must keep counting + // instead of consuming a restart it never earned. Also exercises a + // numId that would collide with a u64::MAX sentinel if one existed. + let document = r#" + + one + sub + two + "#; + let numbering = r#" + + + + + + + + + + "#; + let bytes = + docx_parts(&[("word/document.xml", document), ("word/numbering.xml", numbering)]); + let doc = parse(&bytes).unwrap(); + let mut starts: Vec> = Vec::new(); + fn walk_lists(blocks: &[Block], out: &mut Vec>) { + for b in blocks { + if let Block::List(list) = b { + out.push(list.ordered().then_some(list.start)); + for item in &list.items { + walk_lists(&item.blocks, out); + } + } + } + } + walk_lists(&doc.blocks, &mut starts); + assert_eq!( + starts, + [Some(1), Some(1), Some(2)], + "sub-list starts at a; the outer sequence continues at two across the overriding instance instead of restarting" + ); + } + #[test] fn huge_numbering_start_values_cannot_overflow() { // H2: w:start is ST_DecimalNumber (xsd:int); out-of-range values are diff --git a/src/formats/docx/numbering.rs b/src/formats/docx/numbering.rs index bb892cd4..61637a7d 100644 --- a/src/formats/docx/numbering.rs +++ b/src/formats/docx/numbering.rs @@ -263,8 +263,15 @@ struct InstanceState { /// The instance currently driving this abstract (`u64::MAX` = none yet; /// real `numId`s never reach the counters as `u64::MAX`). Switching /// instances inside one logical list restarts only the levels the - /// entering instance overrides (#96). - last_num: u64, + /// entering instance overrides (#96). `None` until the list's first + /// paragraph; `Some` never collides with a real id because it is + /// compared as an `Option`, not against a sentinel value. + last_num: Option, + /// The overridden levels of the currently active instance. Each restarts + /// on its first USE under this instance — not eagerly at the switch, so a + /// following plain instance cannot consume a restart that was never + /// earned (cubic review of #129). + fresh_overrides: [bool; LEVELS], } impl Counters { @@ -273,24 +280,15 @@ impl Counters { /// reproducible from the marker kind alone, the composite label. pub fn next(&mut self, num_id: u64, ilvl: usize, instance: &Instance) -> (u64, Option) { let ilvl = ilvl.min(LEVELS - 1); - let state = self - .state - .entry(instance.abstract_key) - .or_insert_with(|| InstanceState { last_num: u64::MAX, ..Default::default() }); - if state.last_num != num_id { - // First paragraph of this list, or an instance switch within it: - // overridden levels restart, everything else keeps counting. - if state.last_num != u64::MAX { - for (l, overridden) in instance.overrides.iter().enumerate() { - if *overridden { - state.restart_pending[l] = true; - } - } - } - state.last_num = num_id; + let state = self.state.entry(instance.abstract_key).or_default(); + if state.last_num != Some(num_id) { + // First paragraph of this list, or an instance switch within it. + state.fresh_overrides = instance.overrides; + state.last_num = Some(num_id); } let def = &instance.levels[ilvl]; - if !state.initialized[ilvl] || state.restart_pending[ilvl] { + let override_restart = std::mem::take(&mut state.fresh_overrides[ilvl]); + if !state.initialized[ilvl] || state.restart_pending[ilvl] || override_restart { state.value[ilvl] = def.start; state.initialized[ilvl] = true; state.restart_pending[ilvl] = false;