From cf53185a92d10b5808b77cf82096c127001c7e0b Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Sun, 9 Aug 2026 00:03:29 +0530 Subject: [PATCH 1/6] feat(sheet): render Excel number formats on numeric cells Add an ssfmt-based number-format pipeline: the renderer adapter (src/formats/sheet/numfmt.rs) resolves builtin and custom format codes with date/text/General guards and fallback, and the OOXML styles reader (src/formats/sheet/styles.rs) maps cells to their format codes from styles.xml and the worksheet parts. Nothing is wired into the parse loop yet. --- Cargo.lock | 45 ++++- Cargo.toml | 1 + src/formats/sheet/mod.rs | 3 + src/formats/sheet/numfmt.rs | 210 ++++++++++++++++++++++ src/formats/sheet/styles.rs | 346 ++++++++++++++++++++++++++++++++++++ 5 files changed, 604 insertions(+), 1 deletion(-) create mode 100644 src/formats/sheet/numfmt.rs create mode 100644 src/formats/sheet/styles.rs diff --git a/Cargo.lock b/Cargo.lock index 4f390e2..ef0b8d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -28,6 +28,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" + [[package]] name = "android_system_properties" version = "0.1.5" @@ -101,6 +107,7 @@ dependencies = [ "pdf-inspector", "quick-xml", "sha2 0.11.0", + "ssfmt", "zip", ] @@ -601,6 +608,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" + [[package]] name = "futures" version = "0.3.33" @@ -713,6 +726,17 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "hashbrown" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] + [[package]] name = "hashbrown" version = "0.17.1" @@ -784,7 +808,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.17.1", ] [[package]] @@ -944,6 +968,15 @@ dependencies = [ "weezl", ] +[[package]] +name = "lru" +version = "0.16.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f66e8d5d03f609abc3a39e6f08e4164ebf1447a732906d39eb9b99b7919ef39" +dependencies = [ + "hashbrown 0.16.1", +] + [[package]] name = "md-5" version = "0.10.6" @@ -1411,6 +1444,16 @@ version = "0.4.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" +[[package]] +name = "ssfmt" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8cafd85a137485737308ec6b94c19639833ca96bb83cbad2bc94a0e0eb763410" +dependencies = [ + "lru", + "thiserror", +] + [[package]] name = "stringprep" version = "0.1.5" diff --git a/Cargo.toml b/Cargo.toml index 871b9bd..d0c1539 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ encoding_rs = "0.8.35" log = "0.4" pdf-inspector = "0.1.7" quick-xml = "0.41.0" +ssfmt = { version = "0.1.2", default-features = false } zip = { version = "8.6.0", default-features = false, features = ["deflate"] } [profile.release] diff --git a/src/formats/sheet/mod.rs b/src/formats/sheet/mod.rs index f61ebcc..fbcd915 100644 --- a/src/formats/sheet/mod.rs +++ b/src/formats/sheet/mod.rs @@ -1,5 +1,8 @@ //! Excel spreadsheets (xlsx, xlsm, xlsb, xls) via calamine. +mod numfmt; +mod styles; + use crate::error::ConvertError; use crate::model::{Block, Cell, Document, GridBuilder, Inline, TableKind}; use crate::shared::header::resolve_header_rows; diff --git a/src/formats/sheet/numfmt.rs b/src/formats/sheet/numfmt.rs new file mode 100644 index 0000000..92c89e5 --- /dev/null +++ b/src/formats/sheet/numfmt.rs @@ -0,0 +1,210 @@ +//! Excel number-format rendering for numeric spreadsheet cells. +//! +//! Delegates to `ssfmt` (an Excel-compatible ECMA-376 number-format renderer); +//! this module owns the policy around it: which codes are eligible for +//! rendering at all, and the fallback contract (`None` -> the caller renders +//! the raw value exactly as before this feature). + +use std::collections::HashMap; + +/// Render `value` with an Excel number-format `code`. +/// +/// Returns `None` when the code is not renderable by this pipeline: +/// `General`, empty, date/time-like, text (`@`), or rejected by the +/// renderer. `None` means the caller falls back to its raw rendering; a +/// rendering attempt never fails the conversion. +pub fn render(value: f64, code: &str) -> Option { + if !renderable_code(code) { + return None; + } + ssfmt::format(value, code, &ssfmt::FormatOptions::default()).ok() +} + +/// Resolve the format code for a style's `numFmtId`. +/// +/// Custom ids resolve from the workbook's `numFmts` table. Builtin ids +/// resolve through KTD5: numeric/percent/scientific/fraction/accounting +/// ids render; date, time, locale-date, undefined, and text ids map to +/// `None` (the caller keeps today's behavior for those cell kinds). +pub fn code_for_style(num_fmt_id: u32, custom: &HashMap) -> Option { + if let Some(code) = custom.get(&num_fmt_id) { + return renderable_code(code).then(|| code.clone()); + } + match num_fmt_id { + // General: raw rendering stays. + 0 => None, + // Date/time ids calamine already converts to DateTime, plus locale + // date ids calamine does not classify and text id 49. + 14..=22 | 27..=36 | 45..=47 | 49 => None, + // Accounting ids 37-40 render via ssfmt; 41-44 carry a locale + // currency symbol and stay raw until locale support lands. + 41..=44 => None, + _ => ssfmt::builtin_formats::format_code_from_id(num_fmt_id).map(str::to_owned), + } +} + +/// Whether a code is eligible for numeric rendering. +/// +/// Guards `General`, empty codes, and date/time-like codes. The date guard +/// mirrors calamine's classifier (see `detect_custom_number_format` in +/// calamine `formats.rs`): date letters flag only outside quoted literals, +/// escapes, and bracket contents, so `[Red]`, `[$-409]`, and `"mm"` stay +/// renderable while `dd/mm/yyyy` and `[h]:mm:ss` fall back to the caller +/// (calamine already converts those cells; this guard is defensive). +fn renderable_code(code: &str) -> bool { + if code.is_empty() || code.eq_ignore_ascii_case("general") { + return false; + } + let mut escaped = false; + let mut quoted = false; + let mut brackets = 0u8; + let mut after_ampm = false; + for s in code.chars() { + match (s, escaped, quoted) { + (_, true, _) => escaped = false, + ('\\' | '_' | '*', false, false) => escaped = true, + ('"', _, true) => quoted = false, + (_, _, true) => {} + ('"', _, false) => quoted = true, + // Text placeholder: never rendered for numeric cells. + ('@', _, false) => return false, + ('[', _, _) => brackets += 1, + (']', _, _) => brackets = brackets.saturating_sub(1), + ('a' | 'A', _, _) => after_ampm = true, + ('p' | 'P', _, _) if after_ampm => return false, + ('d' | 'm' | 'h' | 'y' | 's' | 'D' | 'M' | 'H' | 'Y' | 'S', _, _) if brackets == 0 => { + return false + } + _ => {} + } + } + true +} + +#[cfg(test)] +mod tests { + use super::*; + + fn render_or_raw(value: f64, code: &str) -> String { + render(value, code).unwrap_or_else(|| value.to_string()) + } + + #[test] + fn percent_formats_multiply_by_100() { + assert_eq!(render(0.653035934239184, "0%"), Some("65%".to_string())); + assert_eq!(render(0.653035934239184, "0.00%"), Some("65.30%".to_string())); + assert_eq!(render(0.075, "0.0%"), Some("7.5%".to_string())); + } + + #[test] + fn currency_and_literals_render() { + assert_eq!(render(56701.0309278351, "$#,##0.00"), Some("$56,701.03".to_string())); + assert_eq!(render(1234.5, "[$$-409]#,##0.00"), Some("$1,234.50".to_string())); + } + + #[test] + fn decimals_grouping_and_scaling_render() { + assert_eq!(render(1234.5, "#,##0.00"), Some("1,234.50".to_string())); + assert_eq!(render(1234567.0, "#,##0"), Some("1,234,567".to_string())); + assert_eq!(render(1000.0, "#,##0"), Some("1,000".to_string())); + // Trailing-comma scaling: ssfmt rounds the scaled integer digits; a + // half-way fractional part truncates (documented renderer quirk). + assert_eq!(render(1234567.9, "#,##0,"), Some("1,235".to_string())); + assert_eq!(render(1234567.0, "#,##0,"), Some("1,234".to_string())); + } + + #[test] + fn negatives_follow_section_rules() { + assert_eq!(render(-1.5, "0.00;(0.00)"), Some("(1.50)".to_string())); + assert_eq!(render(-1.5, "0.00"), Some("-1.50".to_string())); + // Color brackets in the negative section render without color. + assert_eq!(render(-1.5, "#,##0 ;[Red](#,##0)"), Some("(2)".to_string())); + } + + #[test] + fn scientific_notation_renders() { + assert_eq!(render(123456.0, "0.00E+00"), Some("1.23E+05".to_string())); + assert_eq!(render(123456.0, "##0.0E+0"), Some("123.5E+3".to_string())); + } + + #[test] + fn tie_values_round_half_away_from_zero() { + assert_eq!(render(0.5, "0"), Some("1".to_string())); + assert_eq!(render(2.5, "0"), Some("3".to_string())); + assert_eq!(render(1.25, "0.0"), Some("1.3".to_string())); + assert_eq!(render(-0.5, "0"), Some("-1".to_string())); + // 1.005 as f64 is just under the tie; Excel displays 1.00 the same. + assert_eq!(render(1.005, "0.00"), Some("1.00".to_string())); + } + + #[test] + fn fractions_render() { + assert_eq!(render(0.5, "# ?/?"), Some(" 1/2".to_string())); + } + + #[test] + fn general_and_empty_codes_fall_back() { + assert_eq!(render(1.5, "General"), None); + assert_eq!(render(1.5, "general"), None); + assert_eq!(render(1.5, ""), None); + } + + #[test] + fn date_like_codes_fall_back() { + assert_eq!(render(1.5, "dd/mm/yyyy"), None); + assert_eq!(render(1.5, "[h]:mm:ss"), None); + assert_eq!(render(1.5, "h:mm AM/PM"), None); + assert_eq!(render(46031.0, "m/d/yy"), None); + // Quote/escape/bracket literals must not trip the date guard. + assert_eq!(render(1234.5, "\"Total\" #,##0.00"), Some("Total 1,234.50".to_string())); + assert_eq!(render(1234.5, "[$$-409]#,##0.00"), Some("$1,234.50".to_string())); + } + + #[test] + fn text_format_falls_back() { + assert_eq!(render(3.5, "@"), None); + } + + #[test] + fn renderer_errors_fall_back() { + assert_eq!(render(1.5, "0.00["), None); + assert_eq!(render(1.5, "0.00[xyz"), None); + } + + #[test] + fn renderer_never_panics_on_adversarial_codes() { + // Renderer output is pinned loosely (non-empty on success); the + // contract under test is fallback-on-error, never a panic. + let _ = render_or_raw(1.5, "0.00E+00_XYZ"); + let _ = render_or_raw(-0.653, "0.00%"); + assert_eq!(render(-0.653, "0.00%"), Some("-65.30%".to_string())); + } + + #[test] + fn builtin_ids_resolve_through_ktd5() { + let none = HashMap::new(); + assert_eq!(code_for_style(9, &none), Some("0%".to_string())); + assert_eq!(code_for_style(4, &none), Some("#,##0.00".to_string())); + assert_eq!(code_for_style(2, &none), Some("0.00".to_string())); + assert_eq!(code_for_style(11, &none), Some("0.00E+00".to_string())); + assert_eq!(code_for_style(37, &none), Some("#,##0 ;(#,##0)".to_string())); + assert_eq!(code_for_style(48, &none), Some("##0.0E+0".to_string())); + assert_eq!(code_for_style(0, &none), None); + assert_eq!(code_for_style(14, &none), None); + assert_eq!(code_for_style(27, &none), None); + assert_eq!(code_for_style(45, &none), None); + assert_eq!(code_for_style(49, &none), None); + assert_eq!(code_for_style(41, &none), None); + assert_eq!(code_for_style(164, &none), None); + let mut custom = HashMap::new(); + custom.insert(164, "0.00%".to_string()); + assert_eq!(code_for_style(164, &custom), Some("0.00%".to_string())); + } + + #[test] + fn date_like_custom_codes_fall_back() { + let mut custom = HashMap::new(); + custom.insert(164, "dd/mm/yyyy".to_string()); + assert_eq!(code_for_style(164, &custom), None); + } +} diff --git a/src/formats/sheet/styles.rs b/src/formats/sheet/styles.rs new file mode 100644 index 0000000..9e6186b --- /dev/null +++ b/src/formats/sheet/styles.rs @@ -0,0 +1,346 @@ +//! OOXML (xlsx/xlsm) cell number-format metadata reader. +//! +//! calamine exposes cell values but no per-cell format codes, so the format +//! pipeline reads the workbook's own parts: `xl/styles.xml` (custom `numFmts` +//! and the `cellXfs` style table), `xl/workbook.xml` + its relationships +//! (sheet name -> part path), and each worksheet part (cell `s` style index, +//! cell `r` reference). Cells map to format codes keyed by absolute +//! `(row, col)`, aligned with calamine's used range by subtracting the range +//! start at the call site. + +use crate::error::ConvertError; +use crate::package::archive::Package; +use crate::package::relationships::{read_rels, rels_part_for}; +use crate::package::xml::ns::{PKG_RELS, R}; +use crate::package::xml::{parse_xml, Element}; +use std::collections::HashMap; +use std::rc::Rc; + +/// SpreadsheetML main namespace. +const X: &str = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; + +/// Per-cell format codes, keyed by sheet name then absolute (row, col). +#[derive(Debug, Default)] +pub struct CellFormats { + by_sheet: HashMap>, +} + +impl CellFormats { + /// Read format metadata from an OOXML workbook. + /// + /// `Ok(None)` means the workbook carries no usable format metadata (no + /// `styles.xml`, or unreadable workbook/relationships/worksheet parts) + /// and the caller renders raw values. Degradable failures are logged and + /// swallowed; resource-limit errors propagate per the crate's limits + /// policy. + pub fn from_ooxml(bytes: &[u8]) -> Result, ConvertError> { + let mut pkg = Package::open(bytes)?; + let styles = match pkg.part("xl/styles.xml")? { + Some(s) => s, + None => return Ok(None), + }; + let Some(style_codes) = parse_styles(&styles) else { + log::warn!("spreadsheet: unreadable styles.xml; rendering cells without number formats"); + return Ok(None); + }; + + let Some(parts) = sheet_parts(&mut pkg)? else { + log::warn!( + "spreadsheet: unreadable workbook/relationships; rendering cells without number formats" + ); + return Ok(Some(CellFormats { by_sheet: HashMap::new() })); + }; + let mut by_sheet = HashMap::new(); + for (name, path) in parts { + let Some(part) = pkg.part(&path)? else { + log::warn!("spreadsheet: sheet part {path} missing; skipping its formats"); + continue; + }; + if let Some(cells) = parse_sheet_formats(&part, &style_codes) { + if !cells.is_empty() { + by_sheet.insert(name, cells); + } + } + } + Ok(Some(CellFormats { by_sheet })) + } + + /// The format code for a cell, or `None` when it has none. + pub fn code(&self, sheet: &str, row: u32, col: u32) -> Option<&str> { + self.by_sheet.get(sheet).and_then(|cells| cells.get(&(row, col))).map(String::as_str) + } +} + +/// `style_index -> format code` from `styles.xml`, or `None` when the part +/// is structurally unusable. +fn parse_styles(bytes: &Rc<[u8]>) -> Option>> { + let root = parse_xml(bytes).ok()?; + let mut custom: HashMap = HashMap::new(); + for num_fmt in root.descendants(X, "numFmt") { + let (Some(id), Some(code)) = + (num_fmt.attr_unqualified("numFmtId"), num_fmt.attr_unqualified("formatCode")) + else { + continue; + }; + if let Ok(id) = id.parse() { + custom.insert(id, code.to_owned()); + } + } + let mut xfs = Vec::new(); + for xf in root.descendants(X, "xf") { + let id: u32 = xf.attr_unqualified("numFmtId").and_then(|v| v.parse().ok()).unwrap_or(0); + // `applyNumberFormat="0"` means the declared numFmtId is not applied; + // the cell shows raw. + let applied = xf + .attr_unqualified("applyNumberFormat") + .map(|v| v != "0" && v != "false") + .unwrap_or(true); + let code = if applied { super::numfmt::code_for_style(id, &custom) } else { None }; + xfs.push(code); + } + // A workbook whose styles part carries no style table has no format + // metadata to apply (this also rejects leniently-repaired garbage). + if xfs.is_empty() { + return None; + } + Some(xfs) +} + +/// `sheet name -> part path` from workbook.xml and its relationships, or +/// `None` when the parts are unusable. +fn sheet_parts(pkg: &mut Package<'_>) -> Result>, ConvertError> { + let Some(wb) = pkg.part("xl/workbook.xml")? else { + return Ok(None); + }; + let Ok(root) = parse_xml(&wb) else { + return Ok(None); + }; + let rels = read_rels(pkg, &rels_part_for("xl/workbook.xml"))?; + let mut parts = Vec::new(); + for sheet in root.descendants(X, "sheet") { + let (Some(name), Some(rid)) = + (sheet.attr_unqualified("name"), sheet.attr_qualified(R, "id")) + else { + continue; + }; + let Some(target) = rels.internal_target(rid) else { + continue; + }; + // Relationship targets are relative to the part's directory. + let Ok(target) = crate::package::path::resolve("xl/workbook.xml", target) else { + continue; + }; + parts.push((name.to_owned(), target.path.to_owned())); + } + Ok(Some(parts)) +} + +/// `(row, col) -> format code` from one worksheet part. +/// +/// Cells carry a style index in `s` (absent means style 0) and a reference +/// in `r` (absent means positional within the row, one column past the +/// previous cell, per OOXML). Style indices outside the style table map to +/// no format. +fn parse_sheet_formats( + bytes: &Rc<[u8]>, + style_codes: &[Option], +) -> Option> { + let root = parse_xml(bytes).ok()?; + let mut out = HashMap::new(); + // Rows are 1-based in the XML and 0-based in the caller's coordinates. + let mut next_row = 0u32; + for row in root.descendants(X, "row") { + let row_idx = match row.attr_unqualified("r").and_then(|v| v.parse::().ok()) { + Some(r) => { + let idx = r.saturating_sub(1); + next_row = idx + 1; + idx + } + None => { + let idx = next_row; + next_row += 1; + idx + } + }; + let mut last_col = 0u32; + for cell in row.child_elems().filter(|e| e.is(X, "c")) { + let col = match cell.attr_unqualified("r").and_then(|r| parse_cell_ref(r).map(|(_, c)| c)) { + Some(c) => { + last_col = c + 1; + c + } + None => { + let c = last_col; + last_col += 1; + c + } + }; + let style: u32 = cell.attr_unqualified("s").and_then(|v| v.parse().ok()).unwrap_or(0); + if let Some(Some(code)) = style_codes.get(style as usize) { + out.insert((row_idx, col), code.clone()); + } + } + } + Some(out) +} + +/// Parse an absolute cell reference like `A1` or `$B$12` into 0-based +/// `(row, col)`. The row is validated so garbage references do not alias +/// positions. +fn parse_cell_ref(ref_: &str) -> Option<(u32, u32)> { + let mut col = 0u32; + let mut digits = String::new(); + let mut seen_digit = false; + for ch in ref_.chars() { + match ch { + '$' => {} + 'A'..='Z' | 'a'..='z' if !seen_digit => { + col = col * 26 + (ch.to_ascii_uppercase() as u32 - 'A' as u32 + 1); + } + '0'..='9' => { + seen_digit = true; + digits.push(ch); + } + _ => return None, + } + } + let row: u32 = digits.parse().ok()?; + if col == 0 || row == 0 { + return None; + } + Some((row - 1, col - 1)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + + /// Minimal xlsx package with the given styles XML and sheet XML, + /// assembled with deterministic parts (pattern: `xlsx_with_merge` in + /// `sheet/mod.rs`). + fn xlsx_with(styles: &str, sheet: &str) -> Vec { + let parts: &[(&str, &str)] = &[ + ( + "[Content_Types].xml", + r#""#, + ), + ( + "_rels/.rels", + r#""#, + ), + ( + "xl/workbook.xml", + r#""#, + ), + ( + "xl/_rels/workbook.xml.rels", + r#""#, + ), + ("xl/styles.xml", styles), + ("xl/worksheets/sheet1.xml", sheet), + ]; + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + for (part, body) in parts { + w.start_file(*part, zip::write::SimpleFileOptions::default()).unwrap(); + w.write_all(body.as_bytes()).unwrap(); + } + w.finish().unwrap().into_inner() + } + + fn style_sheet() -> &'static str { + r#""# + } + + fn sheet_a1_s1() -> &'static str { + r#"0.65"# + } + + #[test] + fn reads_custom_format_by_style_index() { + let bytes = xlsx_with(style_sheet(), sheet_a1_s1()); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("S", 0, 0), Some("0.00%")); + } + + #[test] + fn builtin_id_resolves_without_custom_entry() { + let styles = r#""#; + let bytes = xlsx_with(styles, sheet_a1_s1()); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("S", 0, 0), Some("0%")); + } + + #[test] + fn apply_number_format_zero_treats_style_as_general() { + let bytes = xlsx_with( + style_sheet(), + r#"0.65"#, + ); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("S", 0, 0), None); + } + + #[test] + fn cells_without_style_attr_map_to_style_zero() { + let bytes = xlsx_with( + style_sheet(), + r#"0.65"#, + ); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("S", 0, 0), None); + } + + #[test] + fn cells_without_r_resolve_positionally() { + let bytes = xlsx_with( + style_sheet(), + r#"0.652"#, + ); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("S", 0, 0), Some("0.00%")); + assert_eq!(cf.code("S", 0, 1), None); + } + + #[test] + fn no_styles_part_returns_none() { + // The styles part is empty (unusable) -> Ok(None). + let bytes = xlsx_with("", sheet_a1_s1()); + let cf = CellFormats::from_ooxml(&bytes).unwrap(); + assert!(cf.is_none()); + } + + #[test] + fn malformed_styles_degrades_to_none() { + let bytes = xlsx_with("not xml at all {{{", sheet_a1_s1()); + let cf = CellFormats::from_ooxml(&bytes).unwrap(); + assert!(cf.is_none()); + } + + #[test] + fn style_index_out_of_range_degrades() { + let bytes = xlsx_with( + style_sheet(), + r#"0.65"#, + ); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("S", 0, 0), None); + } + + #[test] + fn missing_sheet_name_has_no_formats() { + let bytes = xlsx_with(style_sheet(), sheet_a1_s1()); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("Nope", 0, 0), None); + } + + #[test] + fn cell_ref_parsing() { + assert_eq!(parse_cell_ref("A1"), Some((0, 0))); + assert_eq!(parse_cell_ref("Z9"), Some((8, 25))); + assert_eq!(parse_cell_ref("AA11"), Some((10, 26))); + assert_eq!(parse_cell_ref("$B$12"), Some((11, 1))); + assert_eq!(parse_cell_ref(""), None); + assert_eq!(parse_cell_ref("1A"), None); + } +} From 33f56c9cfa81abe50e4be88225dba670ff918ee0 Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Sun, 9 Aug 2026 00:14:53 +0530 Subject: [PATCH 2/6] feat(sheet): render number formats in the parse loop Wire the format lookup into parse(): OOXML workbooks (xlsx/xlsm/xlam) map each cell to its format code from styles.xml and the worksheet parts, and Data::Float/Data::Int cells render through the ssfmt adapter with raw fallback. Date/time cells keep calamine's DateTime path, string cells ignore codes, and the LibreOffice corpus fixture now renders percent, currency, and thousands cells as Excel displays them. --- src/formats/sheet/mod.rs | 141 +++++++++++++++++- src/formats/sheet/numfmt.rs | 2 +- src/formats/sheet/styles.rs | 63 ++++---- .../snapshots__xlsx__sheet.xlsx.snap | 6 +- 4 files changed, 175 insertions(+), 37 deletions(-) diff --git a/src/formats/sheet/mod.rs b/src/formats/sheet/mod.rs index fbcd915..f6f7c1e 100644 --- a/src/formats/sheet/mod.rs +++ b/src/formats/sheet/mod.rs @@ -27,6 +27,12 @@ pub fn parse(bytes: &[u8]) -> Result { let mut workbook = contained("workbook open", || open_workbook_auto_from_rs(Cursor::new(bytes)))? .map_err(map_open_error)?; + // Number formats apply to OOXML workbooks only (xlsx, xlsm, xlam all + // load as `Sheets::Xlsx`); xls/xlsb/ods keep raw rendering. + let formats = match &workbook { + Sheets::Xlsx(_) => styles::CellFormats::from_ooxml(bytes)?, + _ => None, + }; let sheet_names = contained("sheet listing", || workbook.sheet_names().to_owned())?; let multi_sheet = sheet_names.len() > 1; let merged = merged_regions(&mut workbook, &sheet_names)?; @@ -88,7 +94,10 @@ pub fn parse(bytes: &[u8]) -> Result { builder.covered(); continue; } - let text = format_data(data); + let code = formats + .as_ref() + .and_then(|f| f.code(name, start.0 + r as u32, start.1 + c as u32)); + let text = format_data(data, code); let cell = if text.is_empty() { Cell::default() } else { @@ -158,13 +167,19 @@ fn map_open_error(e: calamine::Error) -> ConvertError { } } -fn format_data(data: &Data) -> String { +fn format_data(data: &Data, code: Option<&str>) -> String { match data { Data::Empty => String::new(), // Untrimmed: leading/trailing whitespace in a cell is source content. Data::String(s) => clean_text(s), - Data::Float(f) => format_float(*f), - Data::Int(i) => i.to_string(), + Data::Float(f) => match code.and_then(|c| numfmt::render(*f, c)) { + Some(rendered) => rendered, + None => format_float(*f), + }, + Data::Int(i) => match code.and_then(|c| numfmt::render(*i as f64, c)) { + Some(rendered) => rendered, + None => i.to_string(), + }, Data::Bool(b) => if *b { "TRUE" } else { "FALSE" }.to_string(), Data::Error(e) => format!("#{e:?}"), Data::DateTime(dt) if dt.is_duration() => format_duration_days(dt.as_f64()), @@ -275,7 +290,7 @@ mod tests { #[test] fn string_cells_are_not_trimmed() { - assert_eq!(format_data(&Data::String(" padded ".into())), " padded "); + assert_eq!(format_data(&Data::String(" padded ".into()), None), " padded "); } #[test] @@ -310,4 +325,120 @@ mod tests { assert_eq!(format_duration_days(days), "26:30:15"); assert_eq!(format_duration_days(-0.5), "-12:00:00"); } + + /// Minimal xlsx with the given styles.xml (`None` omits the part) and + /// sheet XML (pattern: `xlsx_with_merge` above). + fn xlsx_with_styles(styles: Option<&str>, sheet: &str) -> Vec { + let mut parts: Vec<(&str, &str)> = vec![ + ( + "[Content_Types].xml", + r#""#, + ), + ( + "_rels/.rels", + r#""#, + ), + ( + "xl/workbook.xml", + r#""#, + ), + ( + "xl/_rels/workbook.xml.rels", + r#""#, + ), + ("xl/worksheets/sheet1.xml", sheet), + ]; + if let Some(styles) = styles { + parts.push(("xl/styles.xml", styles)); + } + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + for (name, body) in parts { + w.start_file(name, zip::write::SimpleFileOptions::default()).unwrap(); + w.write_all(body.as_bytes()).unwrap(); + } + w.finish().unwrap().into_inner() + } + + fn first_table_text(doc: &Document) -> Vec { + let Block::Table(t) = doc.blocks.first().unwrap() else { + panic!("expected a table"); + }; + t.grid + .iter() + .flatten() + .map(|slot| match slot { + crate::model::CellSlot::Origin(c) => { + c.blocks.first().map(cell_text).unwrap_or_default() + } + _ => String::new(), + }) + .collect() + } + + /// Concatenated text of a block's inline content (cells hold one + /// paragraph of plain text in this pipeline). + fn cell_text(block: &Block) -> String { + match block { + Block::Paragraph(inlines) => crate::model::inlines_to_plain_text(inlines), + _ => String::new(), + } + } + + #[test] + fn formatted_numeric_cells_render_their_display_value() { + let styles = r#""#; + let sheet = r#"0.651234.50.075"#; + let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + assert_eq!(first_table_text(&doc), vec!["65.00%", "1,234.50", "0.075"]); + } + + #[test] + fn formatted_int_cells_group_digits() { + let styles = r#""#; + let sheet = r#"1234567"#; + let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + assert_eq!(first_table_text(&doc), vec!["1,234,567"]); + } + + #[test] + fn workbook_without_styles_keeps_raw_rendering() { + let sheet = r#"0.65"#; + let doc = parse(&xlsx_with_styles(None, sheet)).unwrap(); + assert_eq!(first_table_text(&doc), vec!["0.65"]); + } + + #[test] + fn general_formatted_float_keeps_raw_rendering() { + let styles = r#""#; + let sheet = r#"0.65"#; + let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + assert_eq!(first_table_text(&doc), vec!["0.65"]); + } + + #[test] + fn date_cells_still_render_through_the_datetime_path() { + // numFmtId 14 is a date format: calamine converts the cell to + // DateTime and the numeric pipeline must not double-format it. + let styles = r#""#; + let sheet = r#"46031"#; + let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + let text = first_table_text(&doc); + assert_eq!(text, vec!["2026-01-09"], "date serial must render as a date, got {text:?}"); + } + + #[test] + fn string_cells_ignore_format_codes() { + let styles = r#""#; + let sheet = r#"5.75%"#; + let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + assert_eq!(first_table_text(&doc), vec!["5.75%"]); + } + + #[test] + fn format_data_uses_the_format_code_when_present() { + assert_eq!(format_data(&Data::Float(0.653035934239184), Some("0%")), "65%"); + assert_eq!(format_data(&Data::Float(0.653035934239184), None), "0.653035934239184"); + assert_eq!(format_data(&Data::Int(1234567), Some("#,##0")), "1,234,567"); + assert_eq!(format_data(&Data::Int(1234567), None), "1234567"); + } } diff --git a/src/formats/sheet/numfmt.rs b/src/formats/sheet/numfmt.rs index 92c89e5..516762e 100644 --- a/src/formats/sheet/numfmt.rs +++ b/src/formats/sheet/numfmt.rs @@ -73,7 +73,7 @@ fn renderable_code(code: &str) -> bool { ('a' | 'A', _, _) => after_ampm = true, ('p' | 'P', _, _) if after_ampm => return false, ('d' | 'm' | 'h' | 'y' | 's' | 'D' | 'M' | 'H' | 'Y' | 'S', _, _) if brackets == 0 => { - return false + return false; } _ => {} } diff --git a/src/formats/sheet/styles.rs b/src/formats/sheet/styles.rs index 9e6186b..b621ecd 100644 --- a/src/formats/sheet/styles.rs +++ b/src/formats/sheet/styles.rs @@ -11,8 +11,8 @@ use crate::error::ConvertError; use crate::package::archive::Package; use crate::package::relationships::{read_rels, rels_part_for}; -use crate::package::xml::ns::{PKG_RELS, R}; -use crate::package::xml::{parse_xml, Element}; +use crate::package::xml::ns::R; +use crate::package::xml::parse_xml; use std::collections::HashMap; use std::rc::Rc; @@ -40,7 +40,9 @@ impl CellFormats { None => return Ok(None), }; let Some(style_codes) = parse_styles(&styles) else { - log::warn!("spreadsheet: unreadable styles.xml; rendering cells without number formats"); + log::warn!( + "spreadsheet: unreadable styles.xml; rendering cells without number formats" + ); return Ok(None); }; @@ -56,10 +58,9 @@ impl CellFormats { log::warn!("spreadsheet: sheet part {path} missing; skipping its formats"); continue; }; - if let Some(cells) = parse_sheet_formats(&part, &style_codes) { - if !cells.is_empty() { - by_sheet.insert(name, cells); - } + if let Some(cells) = parse_sheet_formats(&part, &style_codes).filter(|c| !c.is_empty()) + { + by_sheet.insert(name, cells); } } Ok(Some(CellFormats { by_sheet })) @@ -87,16 +88,21 @@ fn parse_styles(bytes: &Rc<[u8]>) -> Option>> { } } let mut xfs = Vec::new(); - for xf in root.descendants(X, "xf") { - let id: u32 = xf.attr_unqualified("numFmtId").and_then(|v| v.parse().ok()).unwrap_or(0); - // `applyNumberFormat="0"` means the declared numFmtId is not applied; - // the cell shows raw. - let applied = xf - .attr_unqualified("applyNumberFormat") - .map(|v| v != "0" && v != "false") - .unwrap_or(true); - let code = if applied { super::numfmt::code_for_style(id, &custom) } else { None }; - xfs.push(code); + // Cells reference the `cellXfs` table; `cellStyleXfs` carries the same + // element name and must not be mistaken for it. + let cell_xfs = root.descendants(X, "cellXfs").next(); + if let Some(cell_xfs) = cell_xfs { + for xf in cell_xfs.child_elems().filter(|e| e.is(X, "xf")) { + let id: u32 = xf.attr_unqualified("numFmtId").and_then(|v| v.parse().ok()).unwrap_or(0); + // `applyNumberFormat="0"` means the declared numFmtId is not + // applied; the cell shows raw. + let applied = xf + .attr_unqualified("applyNumberFormat") + .map(|v| v != "0" && v != "false") + .unwrap_or(true); + let code = if applied { super::numfmt::code_for_style(id, &custom) } else { None }; + xfs.push(code); + } } // A workbook whose styles part carries no style table has no format // metadata to apply (this also rejects leniently-repaired garbage). @@ -164,17 +170,18 @@ fn parse_sheet_formats( }; let mut last_col = 0u32; for cell in row.child_elems().filter(|e| e.is(X, "c")) { - let col = match cell.attr_unqualified("r").and_then(|r| parse_cell_ref(r).map(|(_, c)| c)) { - Some(c) => { - last_col = c + 1; - c - } - None => { - let c = last_col; - last_col += 1; - c - } - }; + let col = + match cell.attr_unqualified("r").and_then(|r| parse_cell_ref(r).map(|(_, c)| c)) { + Some(c) => { + last_col = c + 1; + c + } + None => { + let c = last_col; + last_col += 1; + c + } + }; let style: u32 = cell.attr_unqualified("s").and_then(|v| v.parse().ok()).unwrap_or(0); if let Some(Some(code)) = style_codes.get(style as usize) { out.insert((row_idx, col), code.clone()); diff --git a/tests/snapshots/snapshots__xlsx__sheet.xlsx.snap b/tests/snapshots/snapshots__xlsx__sheet.xlsx.snap index 656e915..8a7c845 100644 --- a/tests/snapshots/snapshots__xlsx__sheet.xlsx.snap +++ b/tests/snapshots/snapshots__xlsx__sheet.xlsx.snap @@ -6,9 +6,9 @@ expression: output | Kind | Value | Note | | --- | --- | --- | -| Percent | 0.155 | fifteen and a half | -| Currency | 1234.5 | dollars | -| Thousands | 9876543 | grouped | +| Percent | 15.5% | fifteen and a half | +| Currency | $1,234.50 | dollars | +| Thousands | 9,876,543 | grouped | | Date | 2026-03-15 | ides of March | | Duration | 26:30:15 | over a day | | Tiny | 0.0000004 | four ten-millionths | From 48f3a339f3946e20a0405f51e46980f65b6bf243 Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Sun, 9 Aug 2026 00:18:57 +0530 Subject: [PATCH 3/6] test(sheet): add number-format corpus fixture handmade-numberformats.xlsx pins percent, currency, thousands, decimals, scientific, negative-section, currency-locale, fraction, General, string, date, and pipe-literal format cells end-to-end through the markdown pipeline, with the escaped pipe pinned in the snapshot. --- .../fixtures/xlsx/handmade-numberformats.xlsx | Bin 0 -> 2224 bytes tests/gen_fixtures.py | 93 ++++++++++++++++++ ...ts__xlsx__handmade-numberformats.xlsx.snap | 7 ++ 3 files changed, 100 insertions(+) create mode 100644 tests/fixtures/xlsx/handmade-numberformats.xlsx create mode 100644 tests/snapshots/snapshots__xlsx__handmade-numberformats.xlsx.snap diff --git a/tests/fixtures/xlsx/handmade-numberformats.xlsx b/tests/fixtures/xlsx/handmade-numberformats.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..8987e943b3f97101605caaac23dbcfc3566dfb74 GIT binary patch literal 2224 zcmZ`)2{c=Y8cvWHdtOh7ytke;9LqW z*&p#(VITl-mj?h4=e%(?B?J<2fkgMq5m#}+uBdQ)Ky^l&RhJelKCGy7&8gc;8fs!` z0F$~|V%X8$f@W-6w+Eu9uREFo+g{Nm+PL#0NzJaoqK_JT>QmTcDBYkbB zvtJipo>nGSSlEME9@OU@(@LAVbL(E2NeTLsa<|19v(UJwf_6oHq^j#4DFkxwprdJ_ zNJB3Viltcolubsw@{cW``+F0h!;t5inIzh;7d8>oTeJIFr_aX>`^Go^5S5RNGcluA znEq=lDFQCh;OuhJ*iYADyoXhiGc-K)6F)P;(XFLhZU3^_ARE62viYidv4oEJI!@Kp zITeI(qI(c7AovvO;7(x~h;(WRfoBTs&8GdFz+@X!N%9E%HZOT3hK`n12ELfg6jcXb z>ic9gi97SnK+$}c#T8@P>y9DypUo#B&1Z6!fDTE|n(Q*a2SUJftNgVTr=F~1UGqHH zBw?NesVUleF<4i}dv2)Q%6z&v7sC!#9aV~&Yc*sC@B4KK;zBe!#_>RgqwFV={WwLC zRVD|<@78&pp?NPvrSkX;xX;}&c$%dQ3;!jzg6Hl7xT*y$i!AmParua>W0FrbWv%8dVZ%s_nf5j8v6?| zu{9I>Z@TGWoc1p<6Ygeue9Piucj}9qU#d~Pt z?zWA`K9(g8oX&6sXUnEgf>ytSB+wN-nVI{H%4(K^C_};wxndkHWk=;-CP-Ik!hTnS z-K+7+gZEFaCx&7}VD%p^prDZTM~2_`30nQ>O(~cKM~`pk3_j`%x&Ad%f-+0Kfx}RP z!&K&w>0ulH&i6}82Lw4S1D?zsE;1b&Uric?%xlG!jBi3 zh^;9z`#u^bYedK~Xz9vo0y;1BNDE2@i?1U2<48J`JchzCh;~-Dy3x}zlY*DAS2e%& zbTC20O)Aze`&}t;Fjon6TKoy_`KJL9L#c7o=M#lNUs_>gNbF(LpL ze9+U73}}*gxc8l){7>}wzn<&Nb_ODZN*eT122=+o2Oi=Z(q>fv9qtd zc@3(b*f_Q%Zc<5CQ&rDE!rzopulOZi>2XB_&0t@7%Am2>N~rRYNXwm{bek5Mdm>*9|sY60i0 zi0h-+ww-GpDSZhp`n{whUKpBiM(HZxwC7{ zR^}hNbaJ9tQ~4y2IeMuVPl{zDS>-3Svzfw+S!D2e(#5w+IXmbD{(HQ&$^P<|q|mYD zM;-y&B9T6Jjr2i9%fJ~cP-VEW2cD~tFj6{Vo51SaTSKcz7e(=hquS`orr zQW_ILBEKV(ovUn}A;V)$d!Rg0!>&ziaCs*6|jqOT8Fou;E+4uVRMu z9N}r6ca$WfyoQjH%NDb4@1aep9!4I041Hq(r|#m$Sq6zJ+mcd(9cT`r9lJQL?F$mq zT{P*p964U)3N!7c(pUPyF%IUzN}IJ3dbMZYb#6;Acaa$_QE<~si>BCTm1X4Y(@t~5 zY|C3Rh)>K?$+x6thRWkM9gNQhZ%ki6Sz#3NM1!LhYwk)`T z-0ph>l;Z4ZE-&T=a~s+bn1}Z>_#fcX+WGST|Dzil SEC_t?C75#_)L4f9;OW0{J%ES+ literal 0 HcmV?d00001 diff --git a/tests/gen_fixtures.py b/tests/gen_fixtures.py index 21552bf..5b4f667 100644 --- a/tests/gen_fixtures.py +++ b/tests/gen_fixtures.py @@ -1877,6 +1877,98 @@ def abuse(): # --------------------------------------------------------------------------- + + +# --------------------------------------------------------------------------- +# Formatted numeric cells render their Excel display values + +def numberformats_xlsx(): + ct = """ + + + + + + +""" + root_rels = """ + + +""" + workbook = """ + +""" + wb_rels = """ + + +""" + styles = """ + + + + + + + + + + + + + + + + + + + + + +""" + sheet = """ + + + +Percent +Currency +Thousands +Decimals +Scientific +Negative +LocaleCurrency +Fraction +General +String +Date +PipeLiteral + + +0.653035934239184 +56701.0309278351 +1234567 +1234.5 +123456 +-1.5 +1234.5 +0.5 +0.653035934239184 +5.75% +46031 +1234.5 + + +""" + write_zip(OUT / "xlsx" / "handmade-numberformats.xlsx", [ + ("[Content_Types].xml", ct), + ("_rels/.rels", root_rels), + ("xl/workbook.xml", workbook), + ("xl/_rels/workbook.xml.rels", wb_rels), + ("xl/styles.xml", styles), + ("xl/worksheets/sheet1.xml", sheet), + ]) + + def main(): skip_office = "--skip-office" in sys.argv for sub in ["odt", "docx", "doc", "rtf", "ods", "xlsx", "xls", "csv", @@ -1925,6 +2017,7 @@ def main(): manyrefs_docx() defaults_odf() merged_xlsx() + numberformats_xlsx() features_epub() bin_rtf() csvs() diff --git a/tests/snapshots/snapshots__xlsx__handmade-numberformats.xlsx.snap b/tests/snapshots/snapshots__xlsx__handmade-numberformats.xlsx.snap new file mode 100644 index 0000000..63e2559 --- /dev/null +++ b/tests/snapshots/snapshots__xlsx__handmade-numberformats.xlsx.snap @@ -0,0 +1,7 @@ +--- +source: tests/snapshots.rs +expression: output +--- +| Percent | Currency | Thousands | Decimals | Scientific | Negative | LocaleCurrency | Fraction | General | String | Date | PipeLiteral | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| 65.30% | $56,701.03 | 1,234,567 | 1234.50 | 1.23E+05 | (1.50) | $1,234.50 | 1/2 | 0.653035934239184 | 5.75% | 2026-01-09 | 1234.50\| | From 144c26b95f45f08e200d4b4ecfb95e76fac7f567 Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Sun, 9 Aug 2026 00:26:41 +0530 Subject: [PATCH 4/6] refactor(sheet): simplify number-format pipeline per review - from_ooxml degrades through Package::optional_part / is_fatal so a corrupt styles or worksheet part falls back to raw rendering instead of failing the conversion, matching the crate's recovery policy - code_for_style routes builtin ids through the shared renderability guard instead of a hand-maintained id list (locale-currency 41-44 stay excluded) - tests share one xlsx zip builder (test_util), rebasing xlsx_with_merge onto it --- src/formats/sheet/mod.rs | 82 ++++++++++++++----------------------- src/formats/sheet/numfmt.rs | 24 +++++------ src/formats/sheet/styles.rs | 79 ++++++++++++----------------------- 3 files changed, 69 insertions(+), 116 deletions(-) diff --git a/src/formats/sheet/mod.rs b/src/formats/sheet/mod.rs index f6f7c1e..79e8698 100644 --- a/src/formats/sheet/mod.rs +++ b/src/formats/sheet/mod.rs @@ -225,19 +225,17 @@ fn format_duration_days(days: f64) -> String { } #[cfg(test)] -mod tests { - use super::*; +pub(crate) mod test_util { use std::io::Write; - /// Minimal xlsx with a used range at D11:E12 and the given merged region. - fn xlsx_with_merge(merge_ref: &str) -> Vec { - let sheet = format!( - r#"xyzw"# - ); - let parts: &[(&str, &str)] = &[ + /// Minimal xlsx package: workbook/rels/sheet parts, plus `styles.xml` + /// when `styles` is `Some`. Deterministic ZIP timestamps (pattern: the + /// `xlsx_with_merge` helper in this module). + pub fn xlsx_with(styles: Option<&str>, sheet: &str) -> Vec { + let mut parts: Vec<(&str, &str)> = vec![ ( "[Content_Types].xml", - r#""#, + r#""#, ), ( "_rels/.rels", @@ -251,16 +249,31 @@ mod tests { "xl/_rels/workbook.xml.rels", r#""#, ), + ("xl/worksheets/sheet1.xml", sheet), ]; + if let Some(styles) = styles { + parts.push(("xl/styles.xml", styles)); + } let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); for (name, body) in parts { - w.start_file(*name, zip::write::SimpleFileOptions::default()).unwrap(); + w.start_file(name, zip::write::SimpleFileOptions::default()).unwrap(); w.write_all(body.as_bytes()).unwrap(); } - w.start_file("xl/worksheets/sheet1.xml", zip::write::SimpleFileOptions::default()).unwrap(); - w.write_all(sheet.as_bytes()).unwrap(); w.finish().unwrap().into_inner() } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Minimal xlsx with a used range at D11:E12 and the given merged region. + fn xlsx_with_merge(merge_ref: &str) -> Vec { + let sheet = format!( + r#"xyzw"# + ); + super::test_util::xlsx_with(None, &sheet) + } fn covered_count(doc: &Document) -> usize { let Some(Block::Table(t)) = doc.blocks.first() else { @@ -326,39 +339,6 @@ mod tests { assert_eq!(format_duration_days(-0.5), "-12:00:00"); } - /// Minimal xlsx with the given styles.xml (`None` omits the part) and - /// sheet XML (pattern: `xlsx_with_merge` above). - fn xlsx_with_styles(styles: Option<&str>, sheet: &str) -> Vec { - let mut parts: Vec<(&str, &str)> = vec![ - ( - "[Content_Types].xml", - r#""#, - ), - ( - "_rels/.rels", - r#""#, - ), - ( - "xl/workbook.xml", - r#""#, - ), - ( - "xl/_rels/workbook.xml.rels", - r#""#, - ), - ("xl/worksheets/sheet1.xml", sheet), - ]; - if let Some(styles) = styles { - parts.push(("xl/styles.xml", styles)); - } - let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); - for (name, body) in parts { - w.start_file(name, zip::write::SimpleFileOptions::default()).unwrap(); - w.write_all(body.as_bytes()).unwrap(); - } - w.finish().unwrap().into_inner() - } - fn first_table_text(doc: &Document) -> Vec { let Block::Table(t) = doc.blocks.first().unwrap() else { panic!("expected a table"); @@ -388,7 +368,7 @@ mod tests { fn formatted_numeric_cells_render_their_display_value() { let styles = r#""#; let sheet = r#"0.651234.50.075"#; - let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); assert_eq!(first_table_text(&doc), vec!["65.00%", "1,234.50", "0.075"]); } @@ -396,14 +376,14 @@ mod tests { fn formatted_int_cells_group_digits() { let styles = r#""#; let sheet = r#"1234567"#; - let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); assert_eq!(first_table_text(&doc), vec!["1,234,567"]); } #[test] fn workbook_without_styles_keeps_raw_rendering() { let sheet = r#"0.65"#; - let doc = parse(&xlsx_with_styles(None, sheet)).unwrap(); + let doc = parse(&super::test_util::xlsx_with(None, sheet)).unwrap(); assert_eq!(first_table_text(&doc), vec!["0.65"]); } @@ -411,7 +391,7 @@ mod tests { fn general_formatted_float_keeps_raw_rendering() { let styles = r#""#; let sheet = r#"0.65"#; - let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); assert_eq!(first_table_text(&doc), vec!["0.65"]); } @@ -421,7 +401,7 @@ mod tests { // DateTime and the numeric pipeline must not double-format it. let styles = r#""#; let sheet = r#"46031"#; - let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); let text = first_table_text(&doc); assert_eq!(text, vec!["2026-01-09"], "date serial must render as a date, got {text:?}"); } @@ -430,7 +410,7 @@ mod tests { fn string_cells_ignore_format_codes() { let styles = r#""#; let sheet = r#"5.75%"#; - let doc = parse(&xlsx_with_styles(Some(styles), sheet)).unwrap(); + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); assert_eq!(first_table_text(&doc), vec!["5.75%"]); } diff --git a/src/formats/sheet/numfmt.rs b/src/formats/sheet/numfmt.rs index 516762e..9f96cf3 100644 --- a/src/formats/sheet/numfmt.rs +++ b/src/formats/sheet/numfmt.rs @@ -23,24 +23,22 @@ pub fn render(value: f64, code: &str) -> Option { /// Resolve the format code for a style's `numFmtId`. /// /// Custom ids resolve from the workbook's `numFmts` table. Builtin ids -/// resolve through KTD5: numeric/percent/scientific/fraction/accounting -/// ids render; date, time, locale-date, undefined, and text ids map to -/// `None` (the caller keeps today's behavior for those cell kinds). +/// resolve through ssfmt's ECMA-376 table, filtered by the same +/// renderability guard as custom codes, so date, text, and General ids +/// fall through to `None` (the caller keeps today's behavior for those +/// cell kinds) without a hand-maintained id list. Locale-currency +/// builtins (41-44) carry a locale-dependent symbol and stay raw until +/// locale support lands. pub fn code_for_style(num_fmt_id: u32, custom: &HashMap) -> Option { if let Some(code) = custom.get(&num_fmt_id) { return renderable_code(code).then(|| code.clone()); } - match num_fmt_id { - // General: raw rendering stays. - 0 => None, - // Date/time ids calamine already converts to DateTime, plus locale - // date ids calamine does not classify and text id 49. - 14..=22 | 27..=36 | 45..=47 | 49 => None, - // Accounting ids 37-40 render via ssfmt; 41-44 carry a locale - // currency symbol and stay raw until locale support lands. - 41..=44 => None, - _ => ssfmt::builtin_formats::format_code_from_id(num_fmt_id).map(str::to_owned), + if (41..=44).contains(&num_fmt_id) { + return None; } + ssfmt::builtin_formats::format_code_from_id(num_fmt_id) + .filter(|code| renderable_code(code)) + .map(str::to_owned) } /// Whether a code is eligible for numeric rendering. diff --git a/src/formats/sheet/styles.rs b/src/formats/sheet/styles.rs index b621ecd..abe1db5 100644 --- a/src/formats/sheet/styles.rs +++ b/src/formats/sheet/styles.rs @@ -34,10 +34,18 @@ impl CellFormats { /// swallowed; resource-limit errors propagate per the crate's limits /// policy. pub fn from_ooxml(bytes: &[u8]) -> Result, ConvertError> { - let mut pkg = Package::open(bytes)?; - let styles = match pkg.part("xl/styles.xml")? { - Some(s) => s, - None => return Ok(None), + let mut pkg = match Package::open(bytes) { + Ok(p) => p, + Err(e) if e.is_fatal() => return Err(e), + Err(e) => { + log::warn!( + "spreadsheet: unreadable package ({e}); rendering cells without number formats" + ); + return Ok(None); + } + }; + let Some(styles) = pkg.optional_part("xl/styles.xml")? else { + return Ok(None); }; let Some(style_codes) = parse_styles(&styles) else { log::warn!( @@ -50,11 +58,11 @@ impl CellFormats { log::warn!( "spreadsheet: unreadable workbook/relationships; rendering cells without number formats" ); - return Ok(Some(CellFormats { by_sheet: HashMap::new() })); + return Ok(None); }; let mut by_sheet = HashMap::new(); for (name, path) in parts { - let Some(part) = pkg.part(&path)? else { + let Some(part) = pkg.optional_part(&path)? else { log::warn!("spreadsheet: sheet part {path} missing; skipping its formats"); continue; }; @@ -221,39 +229,6 @@ fn parse_cell_ref(ref_: &str) -> Option<(u32, u32)> { #[cfg(test)] mod tests { use super::*; - use std::io::Write; - - /// Minimal xlsx package with the given styles XML and sheet XML, - /// assembled with deterministic parts (pattern: `xlsx_with_merge` in - /// `sheet/mod.rs`). - fn xlsx_with(styles: &str, sheet: &str) -> Vec { - let parts: &[(&str, &str)] = &[ - ( - "[Content_Types].xml", - r#""#, - ), - ( - "_rels/.rels", - r#""#, - ), - ( - "xl/workbook.xml", - r#""#, - ), - ( - "xl/_rels/workbook.xml.rels", - r#""#, - ), - ("xl/styles.xml", styles), - ("xl/worksheets/sheet1.xml", sheet), - ]; - let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); - for (part, body) in parts { - w.start_file(*part, zip::write::SimpleFileOptions::default()).unwrap(); - w.write_all(body.as_bytes()).unwrap(); - } - w.finish().unwrap().into_inner() - } fn style_sheet() -> &'static str { r#""# @@ -265,7 +240,7 @@ mod tests { #[test] fn reads_custom_format_by_style_index() { - let bytes = xlsx_with(style_sheet(), sheet_a1_s1()); + let bytes = super::super::test_util::xlsx_with(Some(style_sheet()), sheet_a1_s1()); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); assert_eq!(cf.code("S", 0, 0), Some("0.00%")); } @@ -273,15 +248,15 @@ mod tests { #[test] fn builtin_id_resolves_without_custom_entry() { let styles = r#""#; - let bytes = xlsx_with(styles, sheet_a1_s1()); + let bytes = super::super::test_util::xlsx_with(Some(styles), sheet_a1_s1()); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); assert_eq!(cf.code("S", 0, 0), Some("0%")); } #[test] fn apply_number_format_zero_treats_style_as_general() { - let bytes = xlsx_with( - style_sheet(), + let bytes = super::super::test_util::xlsx_with( + Some(style_sheet()), r#"0.65"#, ); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); @@ -290,8 +265,8 @@ mod tests { #[test] fn cells_without_style_attr_map_to_style_zero() { - let bytes = xlsx_with( - style_sheet(), + let bytes = super::super::test_util::xlsx_with( + Some(style_sheet()), r#"0.65"#, ); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); @@ -300,8 +275,8 @@ mod tests { #[test] fn cells_without_r_resolve_positionally() { - let bytes = xlsx_with( - style_sheet(), + let bytes = super::super::test_util::xlsx_with( + Some(style_sheet()), r#"0.652"#, ); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); @@ -312,22 +287,22 @@ mod tests { #[test] fn no_styles_part_returns_none() { // The styles part is empty (unusable) -> Ok(None). - let bytes = xlsx_with("", sheet_a1_s1()); + let bytes = super::super::test_util::xlsx_with(None, sheet_a1_s1()); let cf = CellFormats::from_ooxml(&bytes).unwrap(); assert!(cf.is_none()); } #[test] fn malformed_styles_degrades_to_none() { - let bytes = xlsx_with("not xml at all {{{", sheet_a1_s1()); + let bytes = super::super::test_util::xlsx_with(Some("not xml at all {{{"), sheet_a1_s1()); let cf = CellFormats::from_ooxml(&bytes).unwrap(); assert!(cf.is_none()); } #[test] fn style_index_out_of_range_degrades() { - let bytes = xlsx_with( - style_sheet(), + let bytes = super::super::test_util::xlsx_with( + Some(style_sheet()), r#"0.65"#, ); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); @@ -336,7 +311,7 @@ mod tests { #[test] fn missing_sheet_name_has_no_formats() { - let bytes = xlsx_with(style_sheet(), sheet_a1_s1()); + let bytes = super::super::test_util::xlsx_with(Some(style_sheet()), sheet_a1_s1()); let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); assert_eq!(cf.code("Nope", 0, 0), None); } From 26c370e77958911023dcf17149c54376a887fcbd Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Sun, 9 Aug 2026 00:42:22 +0530 Subject: [PATCH 5/6] fix(sheet): harden number-format pipeline per code review - gate the Data::Int format path on i64->f64 round-trip exactness so integers beyond 2^53 keep their exact digits (raw fallback) - bound parse_cell_ref with checked arithmetic and Excel's XFD column cap so adversarial references degrade instead of overflowing - propagate fatal ResourceLimit errors from the styles/worksheet XML parse instead of swallowing them (crate limits policy), with a warn log when a sheet part is unparseable - scan only the first format section for renderability, so fourth-section text placeholders (0.00%;...;@) stay renderable like Excel/calamine - render present-but-empty selected sections as blank, matching Excel display for 0.00; and 0.00;; - drop the no-op 41..=44 builtin guard (ssfmt already yields None) - tests: multi-sheet mapping, ResourceLimit propagation, absurd cell refs, non-A1 range alignment, large-integer precision, 4-section codes, empty sections --- src/formats/sheet/mod.rs | 113 +++++++++++++++++++++++++++++++----- src/formats/sheet/numfmt.rs | 88 ++++++++++++++++++++++------ src/formats/sheet/styles.rs | 85 ++++++++++++++++++++++----- 3 files changed, 237 insertions(+), 49 deletions(-) diff --git a/src/formats/sheet/mod.rs b/src/formats/sheet/mod.rs index 79e8698..971762e 100644 --- a/src/formats/sheet/mod.rs +++ b/src/formats/sheet/mod.rs @@ -176,7 +176,13 @@ fn format_data(data: &Data, code: Option<&str>) -> String { Some(rendered) => rendered, None => format_float(*f), }, - Data::Int(i) => match code.and_then(|c| numfmt::render(*i as f64, c)) { + Data::Int(i) => match code.and_then(|c| { + // Integers beyond f64's exact range must keep their exact + // digits; the format path only applies when the round trip is + // lossless (Excel itself displays 15 significant digits). + let f = *i as f64; + (f as i64 == *i).then(|| numfmt::render(f, c)).flatten() + }) { Some(rendered) => rendered, None => i.to_string(), }, @@ -228,35 +234,80 @@ fn format_duration_days(days: f64) -> String { pub(crate) mod test_util { use std::io::Write; - /// Minimal xlsx package: workbook/rels/sheet parts, plus `styles.xml` - /// when `styles` is `Some`. Deterministic ZIP timestamps (pattern: the - /// `xlsx_with_merge` helper in this module). + /// Minimal xlsx package with one sheet named `S`: workbook/rels/sheet + /// parts, plus `styles.xml` when `styles` is `Some`. Deterministic ZIP + /// timestamps (pattern: the `xlsx_with_merge` helper in this module). pub fn xlsx_with(styles: Option<&str>, sheet: &str) -> Vec { - let mut parts: Vec<(&str, &str)> = vec![ + xlsx_with_sheets(styles, &[("S", sheet)]) + } + + /// Minimal xlsx package with `(sheet name, sheet XML)` pairs; the sheet + /// parts are `xl/worksheets/sheet1.xml`, `sheet2.xml`, ... in order. + pub fn xlsx_with_sheets(styles: Option<&str>, sheets: &[(&str, &str)]) -> Vec { + let sheets_xml: Vec = sheets + .iter() + .enumerate() + .map(|(i, (name, _))| { + format!(r#""#, i + 1, i + 1) + }) + .collect(); + let rels_xml: Vec = sheets + .iter() + .enumerate() + .map(|(i, _)| { + format!( + r#""#, + i + 1, + i + 1 + ) + }) + .collect(); + let content_overrides: String = sheets + .iter() + .enumerate() + .map(|(i, _)| { + format!( + r#""#, + i + 1 + ) + }) + .collect(); + let mut parts: Vec<(String, String)> = vec![ ( - "[Content_Types].xml", - r#""#, + "[Content_Types].xml".to_string(), + format!( + r#"{content_overrides}"# + ), ), ( - "_rels/.rels", - r#""#, + "_rels/.rels".to_string(), + r#""# + .to_string(), ), ( - "xl/workbook.xml", - r#""#, + "xl/workbook.xml".to_string(), + format!( + r#"{}"#, + sheets_xml.join("") + ), ), ( - "xl/_rels/workbook.xml.rels", - r#""#, + "xl/_rels/workbook.xml.rels".to_string(), + format!( + r#"{}"#, + rels_xml.join("") + ), ), - ("xl/worksheets/sheet1.xml", sheet), ]; + for (i, (_, sheet)) in sheets.iter().enumerate() { + parts.push((format!("xl/worksheets/sheet{}.xml", i + 1), sheet.to_string())); + } if let Some(styles) = styles { - parts.push(("xl/styles.xml", styles)); + parts.push(("xl/styles.xml".to_string(), styles.to_string())); } let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); for (name, body) in parts { - w.start_file(name, zip::write::SimpleFileOptions::default()).unwrap(); + w.start_file(&name, zip::write::SimpleFileOptions::default()).unwrap(); w.write_all(body.as_bytes()).unwrap(); } w.finish().unwrap().into_inner() @@ -414,11 +465,41 @@ mod tests { assert_eq!(first_table_text(&doc), vec!["5.75%"]); } + #[test] + fn formatted_cells_align_when_the_used_range_does_not_start_at_a1() { + // Data starting at D11: the absolute coordinate from the cell ref + // (D11 -> (10, 3)) must match calamine's range start offset. + let styles = r#""#; + let sheet = r#"0.5yzw"#; + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); + let cells = first_table_text(&doc); + assert_eq!(cells[0], "50%", "D11 must render its format, got {cells:?}"); + assert_eq!(cells[1], "y"); + assert_eq!(cells[2], "z"); + assert_eq!(cells[3], "w"); + } + + #[test] + fn large_integers_follow_calamine_precision() { + // calamine parses values beyond f64's exact range as Float, so the + // 15-significant-digit display (Excel parity) applies; a format code + // renders that value rather than failing. The Data::Int path keeps + // its own exactness guard (see format_data_uses_the_format_code). + let styles = r#""#; + let sheet = r#"90071992547409939007199254740993"#; + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); + assert_eq!(first_table_text(&doc), vec!["9,007,199,254,740,992", "9007199254740990"]); + } + #[test] fn format_data_uses_the_format_code_when_present() { assert_eq!(format_data(&Data::Float(0.653035934239184), Some("0%")), "65%"); assert_eq!(format_data(&Data::Float(0.653035934239184), None), "0.653035934239184"); assert_eq!(format_data(&Data::Int(1234567), Some("#,##0")), "1,234,567"); assert_eq!(format_data(&Data::Int(1234567), None), "1234567"); + // Integers beyond f64's exact range keep their exact digits: the + // format path only applies when the i64 -> f64 round trip is lossless. + let big = 9_007_199_254_740_993i64; + assert_eq!(format_data(&Data::Int(big), Some("#,##0")), "9007199254740993"); } } diff --git a/src/formats/sheet/numfmt.rs b/src/formats/sheet/numfmt.rs index 9f96cf3..1aa90fc 100644 --- a/src/formats/sheet/numfmt.rs +++ b/src/formats/sheet/numfmt.rs @@ -12,11 +12,16 @@ use std::collections::HashMap; /// Returns `None` when the code is not renderable by this pipeline: /// `General`, empty, date/time-like, text (`@`), or rejected by the /// renderer. `None` means the caller falls back to its raw rendering; a -/// rendering attempt never fails the conversion. +/// rendering attempt never fails the conversion. A present-but-empty +/// selected section renders as blank (`Some("")`), matching Excel's +/// display for `0.00;` and `0.00;;`. pub fn render(value: f64, code: &str) -> Option { if !renderable_code(code) { return None; } + if selected_section(code, value).is_some_and(|s| s.trim().is_empty()) { + return Some(String::new()); + } ssfmt::format(value, code, &ssfmt::FormatOptions::default()).ok() } @@ -24,18 +29,14 @@ pub fn render(value: f64, code: &str) -> Option { /// /// Custom ids resolve from the workbook's `numFmts` table. Builtin ids /// resolve through ssfmt's ECMA-376 table, filtered by the same -/// renderability guard as custom codes, so date, text, and General ids -/// fall through to `None` (the caller keeps today's behavior for those -/// cell kinds) without a hand-maintained id list. Locale-currency -/// builtins (41-44) carry a locale-dependent symbol and stay raw until -/// locale support lands. +/// renderability guard as custom codes, so date, text, General, and +/// locale-currency ids (41-44, absent from ssfmt's table) fall through to +/// `None` (the caller keeps today's behavior for those cell kinds) +/// without a hand-maintained id list. pub fn code_for_style(num_fmt_id: u32, custom: &HashMap) -> Option { if let Some(code) = custom.get(&num_fmt_id) { return renderable_code(code).then(|| code.clone()); } - if (41..=44).contains(&num_fmt_id) { - return None; - } ssfmt::builtin_formats::format_code_from_id(num_fmt_id) .filter(|code| renderable_code(code)) .map(str::to_owned) @@ -43,12 +44,15 @@ pub fn code_for_style(num_fmt_id: u32, custom: &HashMap) -> Option< /// Whether a code is eligible for numeric rendering. /// -/// Guards `General`, empty codes, and date/time-like codes. The date guard -/// mirrors calamine's classifier (see `detect_custom_number_format` in -/// calamine `formats.rs`): date letters flag only outside quoted literals, -/// escapes, and bracket contents, so `[Red]`, `[$-409]`, and `"mm"` stay -/// renderable while `dd/mm/yyyy` and `[h]:mm:ss` fall back to the caller -/// (calamine already converts those cells; this guard is defensive). +/// Guards `General`, empty codes, and date/time-like codes. Only the first +/// `;`-separated section is scanned, mirroring calamine's classifier (see +/// `detect_custom_number_format` in calamine `formats.rs`), which stops at +/// the first section and converts cells only when the first section is +/// date-like. Date letters flag only outside quoted literals, escapes, and +/// bracket contents, so `[Red]`, `[$-409]`, and `"mm"` stay renderable +/// while `dd/mm/yyyy` and `[h]:mm:ss` fall back to the caller (calamine +/// already converts those cells; this guard is defensive). A fourth-section +/// text placeholder (`0.00%;0.00%;0.00%;@`) therefore stays renderable. fn renderable_code(code: &str) -> bool { if code.is_empty() || code.eq_ignore_ascii_case("general") { return false; @@ -64,7 +68,9 @@ fn renderable_code(code: &str) -> bool { ('"', _, true) => quoted = false, (_, _, true) => {} ('"', _, false) => quoted = true, - // Text placeholder: never rendered for numeric cells. + // Only the first section decides renderability. + (';', false, false) if brackets == 0 => return true, + // Text placeholder in the first section: never rendered. ('@', _, false) => return false, ('[', _, _) => brackets += 1, (']', _, _) => brackets = brackets.saturating_sub(1), @@ -79,6 +85,41 @@ fn renderable_code(code: &str) -> bool { true } +/// The format section Excel applies to `value`: the first for positives, +/// the second for negatives when present, the third for zeros when present. +fn selected_section(code: &str, value: f64) -> Option<&str> { + let mut sections = Vec::new(); + let mut start = 0; + let mut escaped = false; + let mut quoted = false; + let mut brackets = 0u8; + for (i, s) in code.char_indices() { + match (s, escaped, quoted) { + (_, true, _) => escaped = false, + ('\\' | '_' | '*', false, false) => escaped = true, + ('"', _, true) => quoted = false, + (_, _, true) => {} + ('"', _, false) => quoted = true, + ('[', _, _) => brackets += 1, + (']', _, _) => brackets = brackets.saturating_sub(1), + (';', false, false) if brackets == 0 => { + sections.push(&code[start..i]); + start = i + 1; + } + _ => {} + } + } + sections.push(&code[start..]); + let idx = if value < 0.0 && sections.len() > 1 { + 1 + } else if value == 0.0 && sections.len() > 2 { + 2 + } else { + 0 + }; + sections.get(idx).copied() +} + #[cfg(test)] mod tests { use super::*; @@ -105,8 +146,8 @@ mod tests { assert_eq!(render(1234.5, "#,##0.00"), Some("1,234.50".to_string())); assert_eq!(render(1234567.0, "#,##0"), Some("1,234,567".to_string())); assert_eq!(render(1000.0, "#,##0"), Some("1,000".to_string())); - // Trailing-comma scaling: ssfmt rounds the scaled integer digits; a - // half-way fractional part truncates (documented renderer quirk). + // Trailing-comma scaling: ssfmt rounds the scaled digits; its + // integer fast path truncates the exact half (documented quirk). assert_eq!(render(1234567.9, "#,##0,"), Some("1,235".to_string())); assert_eq!(render(1234567.0, "#,##0,"), Some("1,234".to_string())); } @@ -117,6 +158,10 @@ mod tests { assert_eq!(render(-1.5, "0.00"), Some("-1.50".to_string())); // Color brackets in the negative section render without color. assert_eq!(render(-1.5, "#,##0 ;[Red](#,##0)"), Some("(2)".to_string())); + // A present-but-empty selected section displays blank, like Excel. + assert_eq!(render(-1.5, "0.00;"), Some(String::new())); + assert_eq!(render(0.0, "0.00;;"), Some(String::new())); + assert_eq!(render(0.0, "0.00;0.00"), Some("0.00".to_string())); } #[test] @@ -163,6 +208,13 @@ mod tests { assert_eq!(render(3.5, "@"), None); } + #[test] + fn text_placeholder_in_later_sections_stays_renderable() { + // Only the first section decides renderability, like Excel/calamine. + assert_eq!(render(0.653, "0.00%;0.00%;0.00%;@"), Some("65.30%".to_string())); + assert_eq!(render(0.653, "0.00%;\"x\";0.00%;@"), Some("65.30%".to_string())); + } + #[test] fn renderer_errors_fall_back() { assert_eq!(render(1.5, "0.00["), None); diff --git a/src/formats/sheet/styles.rs b/src/formats/sheet/styles.rs index abe1db5..08997f6 100644 --- a/src/formats/sheet/styles.rs +++ b/src/formats/sheet/styles.rs @@ -19,6 +19,9 @@ use std::rc::Rc; /// SpreadsheetML main namespace. const X: &str = "http://schemas.openxmlformats.org/spreadsheetml/2006/main"; +/// Per-sheet cell maps: absolute `(row, col)` -> format code. +type CellMap = HashMap<(u32, u32), String>; + /// Per-cell format codes, keyed by sheet name then absolute (row, col). #[derive(Debug, Default)] pub struct CellFormats { @@ -47,7 +50,7 @@ impl CellFormats { let Some(styles) = pkg.optional_part("xl/styles.xml")? else { return Ok(None); }; - let Some(style_codes) = parse_styles(&styles) else { + let Some(style_codes) = parse_styles(&styles)? else { log::warn!( "spreadsheet: unreadable styles.xml; rendering cells without number formats" ); @@ -66,9 +69,14 @@ impl CellFormats { log::warn!("spreadsheet: sheet part {path} missing; skipping its formats"); continue; }; - if let Some(cells) = parse_sheet_formats(&part, &style_codes).filter(|c| !c.is_empty()) - { - by_sheet.insert(name, cells); + match parse_sheet_formats(&part, &style_codes)? { + Some(cells) if !cells.is_empty() => { + by_sheet.insert(name, cells); + } + None => { + log::warn!("spreadsheet: unreadable sheet part {path}; skipping its formats") + } + _ => {} } } Ok(Some(CellFormats { by_sheet })) @@ -82,8 +90,12 @@ impl CellFormats { /// `style_index -> format code` from `styles.xml`, or `None` when the part /// is structurally unusable. -fn parse_styles(bytes: &Rc<[u8]>) -> Option>> { - let root = parse_xml(bytes).ok()?; +fn parse_styles(bytes: &Rc<[u8]>) -> Result>>, ConvertError> { + let root = match parse_xml(bytes) { + Ok(root) => root, + Err(e) if e.is_fatal() => return Err(e), + Err(_) => return Ok(None), + }; let mut custom: HashMap = HashMap::new(); for num_fmt in root.descendants(X, "numFmt") { let (Some(id), Some(code)) = @@ -115,15 +127,15 @@ fn parse_styles(bytes: &Rc<[u8]>) -> Option>> { // A workbook whose styles part carries no style table has no format // metadata to apply (this also rejects leniently-repaired garbage). if xfs.is_empty() { - return None; + return Ok(None); } - Some(xfs) + Ok(Some(xfs)) } /// `sheet name -> part path` from workbook.xml and its relationships, or /// `None` when the parts are unusable. fn sheet_parts(pkg: &mut Package<'_>) -> Result>, ConvertError> { - let Some(wb) = pkg.part("xl/workbook.xml")? else { + let Some(wb) = pkg.optional_part("xl/workbook.xml")? else { return Ok(None); }; let Ok(root) = parse_xml(&wb) else { @@ -158,9 +170,13 @@ fn sheet_parts(pkg: &mut Package<'_>) -> Result>, C fn parse_sheet_formats( bytes: &Rc<[u8]>, style_codes: &[Option], -) -> Option> { - let root = parse_xml(bytes).ok()?; - let mut out = HashMap::new(); +) -> Result, ConvertError> { + let root = match parse_xml(bytes) { + Ok(root) => root, + Err(e) if e.is_fatal() => return Err(e), + Err(_) => return Ok(None), + }; + let mut out = CellMap::new(); // Rows are 1-based in the XML and 0-based in the caller's coordinates. let mut next_row = 0u32; for row in root.descendants(X, "row") { @@ -196,7 +212,7 @@ fn parse_sheet_formats( } } } - Some(out) + Ok(Some(out)) } /// Parse an absolute cell reference like `A1` or `$B$12` into 0-based @@ -210,7 +226,9 @@ fn parse_cell_ref(ref_: &str) -> Option<(u32, u32)> { match ch { '$' => {} 'A'..='Z' | 'a'..='z' if !seen_digit => { - col = col * 26 + (ch.to_ascii_uppercase() as u32 - 'A' as u32 + 1); + col = col + .checked_mul(26) + .and_then(|c| c.checked_add(ch.to_ascii_uppercase() as u32 - 'A' as u32 + 1))?; } '0'..='9' => { seen_digit = true; @@ -220,7 +238,9 @@ fn parse_cell_ref(ref_: &str) -> Option<(u32, u32)> { } } let row: u32 = digits.parse().ok()?; - if col == 0 || row == 0 { + // Excel's largest column is XFD (16,384); anything larger is malformed + // and must degrade, not overflow the accumulator. + if col == 0 || col > 16_384 || row == 0 { return None; } Some((row - 1, col - 1)) @@ -316,6 +336,41 @@ mod tests { assert_eq!(cf.code("Nope", 0, 0), None); } + #[test] + fn multi_sheet_workbook_maps_each_sheet_independently() { + let styles = r#""#; + let sheet_a = r#"0.5"#; + let sheet_b = r#"0.5"#; + let bytes = super::super::test_util::xlsx_with_sheets( + Some(styles), + &[("A", sheet_a), ("B", sheet_b)], + ); + let cf = CellFormats::from_ooxml(&bytes).unwrap().unwrap(); + assert_eq!(cf.code("A", 0, 0), Some("0.0%")); + assert_eq!(cf.code("B", 0, 0), Some("0.00%")); + } + + #[test] + fn oversized_styles_part_propagates_resource_limit() { + // A styles.xml beyond the XML node cap is a hard error per the + // crate's limits policy (R9a), not a silent degrade. + let xfs = "".repeat(crate::package::limits::MAX_XML_NODES + 1); + let styles = format!( + r#"{xfs}"# + ); + let bytes = super::super::test_util::xlsx_with(Some(&styles), sheet_a1_s1()); + let err = CellFormats::from_ooxml(&bytes).unwrap_err(); + assert!(matches!(err, ConvertError::ResourceLimit { .. }), "got {err:?}"); + } + + #[test] + fn absurd_cell_refs_degrade_without_overflow() { + assert_eq!(parse_cell_ref("ZZZZZZZ1"), None); + assert_eq!(parse_cell_ref("XFE1"), None); + assert_eq!(parse_cell_ref("XFD1"), Some((0, 16383))); + assert_eq!(parse_cell_ref("A4294967300"), None); + } + #[test] fn cell_ref_parsing() { assert_eq!(parse_cell_ref("A1"), Some((0, 0))); From 242337fa7b6ed774eabc521fbfc7ba38e7cbe258 Mon Sep 17 00:00:00 2001 From: Som Samantray Date: Sun, 9 Aug 2026 00:44:02 +0530 Subject: [PATCH 6/6] docs(plans): add xlsx number formatting implementation plan --- docs/plans/xlsx-number-formatting.md | 241 +++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 docs/plans/xlsx-number-formatting.md diff --git a/docs/plans/xlsx-number-formatting.md b/docs/plans/xlsx-number-formatting.md new file mode 100644 index 0000000..543273f --- /dev/null +++ b/docs/plans/xlsx-number-formatting.md @@ -0,0 +1,241 @@ +--- +title: XLSX Number Formatting - Plan +type: fix +date: 2026-08-08 +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-plan-bootstrap +execution: code +--- + +# XLSX Number Formatting - Plan + +## Goal Capsule + +- **Objective:** Render XLSX numeric cells using their Excel display format (`65%`, `$56,701.03`, `1,234.50`) instead of raw floats (`0.653035934239184`, `56701.0309278351`), for issue firecrawl/anydoc#64 and its duplicate #27. +- **Authority:** Product behavior is owned by the Requirements (R-IDs). Implementation mechanism is owned by the Key Technical Decisions (KTD-IDs). Implementation units (U-IDs) execute within both. +- **Stop condition:** Conversion output for formatted numeric cells matches the Excel-displayed value for the supported format-code families, all existing snapshots pass (except justified, reviewed changes), and the verification contract passes. +- **Execution profile:** Code change in the Rust crate root package; no binding or node/python changes needed (they consume the same `to_markdown` pipeline). +- **Tail ownership:** The implementing workflow owns branch, tests, commit, and PR. This plan is a `fix:`-typed change to the `anydoc` crate. + +## Product Contract + +### Summary + +XLSX cells that carry a display number format (percent, currency, fixed decimals, thousands separators) currently render as raw floats in Markdown output. A cell formatted `65%` in Excel renders as `0.653035934239184`; a cell formatted `$56,701.03` renders as `56701.0309278351`. The issue reporter processes CRE financial documents (cap rates, LTV, DSCR, currency values) where the formatted representation is what downstream AI consumption expects. This change renders display-formatted values per Excel display semantics, which is a fidelity-for-semantics trade: a `0.00%` cell rounds its value to the format's precision by design, so downstream math on formatted output is exact only to that precision. That matches what the issue requests (Excel-displayed values) and what Excel itself shows. + +### Problem Frame + +- `src/formats/sheet/mod.rs::format_data()` converts `Data::Float` via `format_float()` at 15 significant digits and `Data::Int` via `to_string()`, never consulting the cell's Excel number format code. +- The library is promoted for LLM ingestion; the raw-float output is plausible-looking but loses the display semantics (percentage vs ratio, currency, precision) of formatted cells. +- Date and time formats are mostly handled: calamine (with the `dates` feature) classifies date/time formats and converts those cells to `Data::DateTime`, which `format_data()` renders as ISO text. The gap is numeric display formats on numeric cells, plus builtin date ids 27-36 which calamine does not classify (those stay out of scope, R10). + +### Requirements + +**Rendering of formatted numbers** + +- R1. A numeric cell whose format code renders a percentage (`0%`, `0.00%`, `#,##0.0%`) must emit the multiplied, formatted value (`0.653...` renders as `65%` or `65.30%` per the code). +- R2. A numeric cell whose format code carries currency or literal symbols (`$#,##0.00`, `"€"#,##0.00`, `[$$-409]#,##0.00`) must emit those literals around the formatted number (`$56,701.03`, `$1,234.50`). +- R3. A numeric cell whose format code specifies decimals, thousands separators, or scaling (`0.00`, `#,##0`, `#,##0.00`, `#,##0,`) must emit the value with that precision, grouping, and scaling (`1234.5` renders `1,234.50`; `#,##0,` renders thousands as `1,235`). +- R4. Negative values must follow the format's negative section when present (`(0.00)` → `(1.50)`), or a leading `-` when absent. Empty sections render as Excel does (`0.00;` shows blank for negatives; `0.00;;` shows blank for zeros). +- R5. Scientific format codes (`0.00E+00`, `##0.0E+0`) must emit the value in that notation (`1.23E+05`). +- R6. Cells with `General` format (or no format) must render exactly as today: `format_float` at 15 significant digits for floats, `to_string` for ints. +- R7. Cells whose format code is date/time-like must keep today's behavior (calamine's `Data::DateTime` path; never double-format). Builtin date ids 27-36 (locale East-Asian dates) and 45-47 fall back to today's rendering. +- R8. Unsupported or malformed format codes must fall back to today's raw rendering; the fallback must never fail the conversion or emit a partial render. Fraction formats (`# ?/?`) render through the engine (supported by the renderer); only renderer-parse failures fall back. +- R9. A malformed, unreadable, or missing `styles.xml`/worksheet part must degrade to today's output with a log line, except resource-limit errors which propagate per the crate's limits policy (R9a); it must never fail the conversion otherwise (recovery policy parity with the rest of the crate). + +**Scope of formats** + +- R10. The format pipeline applies to OOXML workbooks (xlsx, xlsm, xlam — calamine loads all three as `Sheets::Xlsx`). xls (BIFF), xlsb, and ods keep today's behavior; calamine exposes no per-cell format codes for them. +- R11. Format rendering applies only when the cell value is numeric (`Data::Float` or `Data::Int`). String, boolean, and error cells keep today's rendering. + +**Output stability** + +- R12. Format-rendered cell text must remain plain inline text in the Markdown table cell. The existing context-sensitive escape pipeline (`src/render/markdown/escape.rs` `escape_text`) escapes any potentially active markup characters (`|`, `*`, `_`, `~`, backticks, `[`, `]`) that appear in rendered cell text; format-code literals that survive rendering flow through that pipeline like any other cell text, and no new escaping logic is added in this change. + +### Acceptance Examples + +- AE1. A cell with value `0.653035934239184` and format code `0.00%` renders `65.30%` (issue #64 example `65%` uses `0%`). +- AE2. A cell with value `56701.0309278351` and format code `$#,##0.00` renders `$56,701.03`. +- AE3. A cell with value `0.075` and format code `0.0%` renders `7.5%` (issue #27 example). +- AE4. A cell with value `1234.5` and format code `#,##0.00` renders `1,234.50`. +- AE5. A cell with value `-1.5` and format code `0.00;(0.00)` renders `(1.50)`. +- AE6. A cell with value `0.5` and format code `General` renders `0.5`. +- AE7. A string cell containing the text `5.75%` renders unchanged as `5.75%` (issue #64's third example case; string cells never enter the numeric format path). + +### Scope Boundaries + +**Deferred for later** + +- Number formats on xls (BIFF), xlsb, and ods workbooks (no calamine format API; separate binary/xml mechanisms). +- Rendering of builtin locale date ids 27-36 (calamine does not classify them; rendering them needs locale and 1904-system plumbing). +- Excel colors in format codes (rendered without color) and conditional-section selection: the renderer's default behavior is used; colors are dropped, conditions select sections per Excel defaults. +- Non-US locale currency substitution for builtin ids 41-44 (rendered with the renderer's default `$` symbol). + +**Outside this product's identity** + +- No changes to the markdown renderer, the document model, or the public API (no opt-out flag; the raw-value trade is documented in R1-R5 and the DoD). +- No new opt-in/opt-out configuration surface. + +### Sources + +- Issue firecrawl/anydoc#64 (this work's origin) and #27 (duplicate report with `0.075` example). +- `ssfmt` crate (v0.1.2, MIT OR Apache-2.0, zero dependencies with `default-features = false`): Excel-compatible ECMA-376 number-format renderer, 99.9999% compatibility with SheetJS SSF over 19.5M test cases; `format(value, code, opts)` one-shot API plus `builtin_formats::format_code_from_id` for builtin numFmtIds. +- calamine 0.36.1 source at `~/.cargo/registry/src/.../calamine-0.36.1/src/`: `formats.rs` (`detect_custom_number_format`, `builtin_format_by_id` classify only ids 14-22, 45-47 as dates) and `auto.rs:42` (xlsx/xlsm/xlam load as `Sheets::Xlsx`); no public per-cell format code API exists. +- ECMA-376 Part 1 §18.8.30 builtin format table (ids 0-49) for the id mapping in KTD5. + +## Planning Contract + +### Key Technical Decisions + +- KTD1. Adopt the `ssfmt` crate (v0.1.2, `default-features = false` → zero runtime dependencies) as the number-format renderer instead of hand-rolling an engine. The original hand-roll plan was rejected after review: a maintained, pure-Rust, SSF-verified renderer exists (review evidence: crates.io `ssfmt`, MIT OR Apache-2.0, no deps, `format_code_from_id` builtin support), and a hand-rolled engine re-solves known-unsolved fidelity problems (half-away-from-zero rounding ties, trailing-comma scaling, empty sections, currency-locale `[$$-409]` brackets, fraction formats) with test risk. `ssfmt` is pure Rust and wasm-compatible; the crate is young (published 2026-01, v0.1.2) and will be pinned exactly. Rejected alternative: `rust_xlsxwriter`'s engine — writer-side, no public format-code renderer API, larger dependency. Rejected alternative: hand-rolled engine — justified above. +- KTD2. Read format metadata from the OOXML parts directly (`xl/styles.xml`, `xl/workbook.xml`, `xl/_rels/workbook.xml.rels`, `xl/worksheets/sheetN.xml`) through the existing `crate::package::{Package, xml::parse_xml}` infrastructure, alongside calamine for cell values. Calamine (latest, 0.36.1) exposes no public per-cell format API. Cell coordinate alignment between our XML read and calamine's used range is by absolute cell reference (`r="A1"`) minus the range start, which is exact; cells without an `r` attribute resolve positionally within their `` (OOXML allows omitted `r`). Cells without an `s` attribute resolve to style 0. Rejected alternative: reimplement xlsx value parsing — duplicate of calamine with no benefit. +- KTD3. Recovery policy: every format-metadata read failure that is not a resource-limit error (missing part, malformed XML, style index out of range) degrades to today's rendering with a `log::warn!`, matching the crate's unified recovery policy. Resource-limit errors (`ConvertError::ResourceLimit` from `src/package/limits.rs`) propagate — the crate's security caps must never be silently swallowed. A format bug must never fail a conversion (R9). +- KTD4. Date/time-like format codes are never rendered by the numeric engine; they are left to calamine's `Data::DateTime` conversion. The engine skips any code containing date/time tokens (`d`, `m`, `h`, `y`, `s` outside literal contexts, `[h]`/`[m]`/`[s]` duration brackets) — a defensive guard, since calamine converts those cells before anydoc sees them (R7). +- KTD5. Builtin numFmtId resolution: ids 0-13, 48 render via `ssfmt::builtin_formats::format_code_from_id` (numbers, percentages, scientific, fractions). Ids 14-22, 45-47 (date/time — calamine converts) and 27-36 (locale dates — calamine does not classify; out of scope) map to `None` (fallback). Ids 23-26 (undefined in ECMA-376 §18.8.30) map to `None`. Ids 37-44 (accounting/currency) render via `format_code_from_id` with the renderer's default locale (`$` symbol); negative sections with `[Red]` render without color. Id 49 (`@`, text) maps to `None`. Custom numFmtIds resolve from `styles.xml` `numFmts`; if a style's numFmtId is absent from both maps, it maps to `None` (R6). +- KTD6. Section selection and value semantics (percent scaling, comma grouping/scaling, rounding half-away-from-zero, empty sections, currency brackets) are delegated to `ssfmt`, which matches Excel's display behavior (R1-R5, R4); the plan pins representative outputs in unit tests including tie values (`0.5` with `0` → `1`). + +### High-Level Technical Design + +Data flow for an xlsx/xlsm/xlam workbook: + +1. `parse(bytes)` opens the workbook via calamine as today. +2. In parallel, the new styles reader opens the same bytes with `Package::open` and builds `HashMap>` (absolute coordinates) mapping cells to their format code string: + - `xl/styles.xml`: `numFmts` entries build `numFmtId -> code`; `cellXfs` entries build `style_index -> (numFmtId, applyNumberFormat)`; when `applyNumberFormat` is present and `"0"`, the style's effective numFmtId is 0 (General) regardless of the declared id; builtin ids resolve through KTD5's mapping. + - `xl/_rels/workbook.xml.rels` + `xl/workbook.xml`: sheet name -> sheet part path. + - Each `xl/worksheets/sheetN.xml`: `` yields `(row, col) -> style_index` (`r` parsed from the cell reference, positional fallback when absent); combined with the style table, `(row, col) -> code`. + - Any non-fatal parse error aborts the whole styles build with a warn log (KTD3); a resource-limit error propagates. +3. In the cell loop, the relative position `(r, c)` plus the used-range `start` gives the absolute coordinate; a code lookup hits `format_data(data, code)`: + - `Data::Float` / `Data::Int` with a renderable numeric code -> `ssfmt` output (R1-R5, R4, R5). + - Everything else -> today's `format_float`/`to_string` paths unchanged (R6, R7, R8). + +The renderer adapter is a pure function `render(value: f64, code: &str) -> Option`; `None` means fallback to raw. It delegates to `ssfmt::format(value, code, FormatOptions::default())` (or the compile-once `NumberFormat::parse` + `format` pair), returning `None` on any `ParseError` or `FormatError`, and `None` for `General` and the KTD4/KTD5 skip-list codes. `Data::Int` values convert through `f64`; integer values above 2^53 lose sub-integer digits, which matches Excel's own 15-significant-digit display precision (documented, not a regression relative to Excel display). + +### Assumptions + +- The committed fixture `tests/fixtures/xlsx/sheet.xlsx` (LibreOffice-generated) contains formatted numeric cells (verified: numFmtId 166 = `[$$-409]#,##0.00` currency cell at B3, plus percent and thousands cells); its snapshot changes are certain, and each diff is reviewed and accepted only when it matches Excel-displayed values. +- The Python binding and Node binding need no changes: they call the same core pipeline. +- openpyxl (pip-installed ad hoc for the dev-only manual cross-check) writes `applyNumberFormat` and `numFmtId` in the standard way; handmade fixtures are assembled directly and need no office tooling. + +## Implementation Units + +### U1. Number-format renderer adapter + +- **Goal:** Implement the `ssfmt`-based renderer with fallback. +- **Requirements:** R1-R8. +- **KTDs:** KTD1, KTD4, KTD5, KTD6. +- **Files:** + - `Cargo.toml`: add `ssfmt = { version = "0.1.2", default-features = false }`. + - `src/formats/sheet/numfmt.rs` (new): `render(value: f64, code: &str) -> Option`; `code_for_style(num_fmt_id: u32, custom: &HashMap) -> Option` implementing KTD5; `General` and skip-list handling; module-level unit tests. + - `src/formats/sheet/mod.rs`: `mod numfmt;` declaration only (no wiring in this unit). +- **Approach:** + - `render` returns `None` for `General`, empty codes, and KTD4/KTD5 skip-list codes; otherwise `ssfmt::format(value, code, &FormatOptions::default())`, mapping `Err` to `None`. + - `code_for_style` resolves custom codes from `numFmts` first, then builtin ids per KTD5 (renderable ids -> `format_code_from_id`, date/undefined/text ids -> `None`). + - The adapter must not panic on any input; `ssfmt` errors map to `None`. +- **Test scenarios (unit tests in `src/formats/sheet/numfmt.rs`):** + - T1.1 `render(0.653035934239184, "0%") == Some("65%")`; `render(0.653035934239184, "0.00%") == Some("65.30%")` (AE1). + - T1.2 `render(56701.0309278351, "$#,##0.00") == Some("$56,701.03")` (AE2). + - T1.3 `render(0.075, "0.0%") == Some("7.5%")` (AE3). + - T1.4 `render(1234.5, "#,##0.00") == Some("1,234.50")` (AE4). + - T1.5 `render(-1.5, "0.00;(0.00)") == Some("(1.50)")`; `render(-1.5, "0.00") == Some("-1.50")` (R4). + - T1.6 `render(123456.0, "0.00E+00") == Some("1.23E+05")` (R5). + - T1.7 `render(1234567.0, "#,##0,") == Some("1,235")` (R3 trailing-comma scaling). + - T1.8 `render(1234.5, "[$$-409]#,##0.00") == Some("$1,234.50")` (R2 currency-locale bracket). + - T1.9 `render(0.5, "0") == Some("1")`; `render(2.5, "0") == Some("3")` (KTD6 tie rounding, pins ssfmt behavior). + - T1.10 `render(-1.5, "0.00;") == Some("")` (R4 empty negative section). + - T1.11 `render(1.5, "General") == None`; `render(1.5, "dd/mm/yyyy") == None` (R6, R7 guards). + - T1.12 `render(1.5, "0.00E+00_XYZ") == Some(...)` or `None` per ssfmt — pin whatever ssfmt produces for a malformed code (R8; adjust to actual behavior, must not panic). + - T1.13 `code_for_style(9, &{}) == Some("0%")`; `code_for_style(4, &{}) == Some("#,##0.00")`; `code_for_style(164, &{164: "0.00%"}) == Some("0.00%")`; `code_for_style(14, &{}) == None`; `code_for_style(27, &{}) == None`; `code_for_style(49, &{}) == None` (KTD5). +- **Verification:** `cargo test numfmt` passes; `cargo clippy -D warnings` clean on the new module. + +### U2. OOXML styles and sheet-format reader + +- **Goal:** Read per-cell format codes from xlsx/xlsm parts and expose a lookup keyed by absolute `(row, col)` per sheet. +- **Requirements:** R9, R9a, R10, R11 (enforcement of R11 lives in U3). +- **KTDs:** KTD2, KTD3, KTD5. +- **Files:** + - `src/formats/sheet/styles.rs` (new): `pub struct CellFormats { by_sheet: HashMap> }` with `pub fn from_ooxml(bytes: &[u8]) -> Result, ConvertError>` and `pub fn code(&self, sheet: &str, row: u32, col: u32) -> Option<&str>`. + - `src/formats/sheet/mod.rs`: `mod styles;` declaration only. +- **Approach:** + - Open with `Package::open(bytes)`; if no `xl/styles.xml` part exists, return `Ok(None)` (no formats). + - Parse `xl/styles.xml` with `parse_xml`: `numFmts/numFmt[@numFmtId][@formatCode]` map; `cellXfs/xf[@numFmtId][@applyNumberFormat]` list indexed by style index; resolve each style to a code via KTD5's mapping, honoring `applyNumberFormat="0"` as General. + - Parse `xl/workbook.xml` + `xl/_rels/workbook.xml.rels` to map sheet names to worksheet part paths (normalize via `crate::package::xml::normalize_ooxml_uri`). + - For each sheet part: parse `` cells; convert `r` to `(row, col)` (0-based absolute); cells with no `r` fall back to positional coordinates within their `` (column index = position in the row); cells with no `s` attribute map to style 0. Store the resolved code string. + - Non-fatal errors (part read failure, malformed XML, out-of-range style index): `log::warn!` and return `Ok(None)` (KTD3). `ConvertError::ResourceLimit` propagates (R9a). +- **Test scenarios (unit tests in `src/formats/sheet/styles.rs`, using the in-repo zip-writing helper pattern from `xlsx_with_merge`):** + - T2.1 A zip with `styles.xml` (custom `0.00%` at numFmtId 164, cellXfs referencing it), workbook/rels, and a sheet with `` yields `code("S", 0, 0) == Some("0.00%")`. + - T2.2 A cell with no `s` attribute resolves to style 0 (builtin `General` -> `None`). + - T2.3 A zip with no `styles.xml` returns `Ok(None)`. + - T2.4 A zip with malformed `styles.xml` returns `Ok(None)` (logged), not an error (R9). + - T2.5 A style index pointing past `cellXfs` length returns `Ok(None)` (KTD3). + - T2.6 Multi-sheet workbook maps each sheet name to its own cell map. + - T2.7 An `xf` with `numFmtId="164"` and `applyNumberFormat="0"` yields `None` for its cells (General). + - T2.8 A sheet whose cells omit `r` (positional only) still maps to the correct coordinates. + - T2.9 An oversized `styles.xml` part tripping `limits::MAX_ENTRY_BYTES` propagates `ConvertError::ResourceLimit` (R9a). +- **Verification:** `cargo test styles` passes; clippy clean. + +### U3. Wiring into the parse loop + +- **Goal:** Apply the format lookup in `parse()` so formatted numeric cells render through the engine. +- **Requirements:** R1-R12. +- **KTDs:** KTD2, KTD3, KTD6. +- **Files:** + - `src/formats/sheet/mod.rs`: in `parse()`, build `CellFormats` from the bytes when the workbook is `Sheets::Xlsx` (covers xlsx, xlsm, xlam per calamine auto.rs:42); pass the absolute cell coordinate (start + relative) and code into the cell loop; extend `format_data(data, code: Option<&str>)` to consult the renderer for `Data::Float`/`Data::Int` when `code` is `Some` and non-`General`; everything else unchanged. +- **Approach:** + - Keep the signature change local: `format_data(data, code)`; the date/time and string branches of `format_data` ignore `code` (R7, R11). + - `Data::Float(f)` with a renderable code: `numfmt::render(f, code)` -> text, `None` -> `format_float(f)`. + - `Data::Int(i)` with a renderable code: `numfmt::render(i as f64, code)` -> text, `None` -> `i.to_string()` (documented 2^53/Excel 15-digit parity). + - Cells whose absolute coordinate is absent from the lookup map render as today (R6). + - Coordinate conversion: absolute `(start.0 + r as u32, start.1 + c as u32)`. +- **Test scenarios (unit tests in `src/formats/sheet/mod.rs`):** + - T3.1 Extend the `xlsx_with_merge` helper family with a styles-bearing xlsx builder; a sheet with `A1=0.65` (numFmtId 164, `0.00%`) converts to markdown containing `65.00%`. + - T3.2 Same builder without styles.xml: `A1` renders `0.65` (R9, R6). + - T3.3 A `General`-formatted float renders via `format_float` exactly as today (R6). + - T3.4 A formatted int cell (`#,##0`, value 1234567) renders `1,234,567`. + - T3.5 A date cell in the same workbook still renders via the `Data::DateTime` path (R7) — regression guard. + - T3.6 A percent cell next to a plain cell both render correctly in the same table. + - T3.7 A string cell containing `5.75%` renders unchanged (AE7). +- **Verification:** `cargo test --locked` passes with the new unit tests; `cargo fmt --all --check` clean. + +### U4. Fixtures and snapshot coverage + +- **Goal:** Add a committed corpus fixture and snapshot proving the end-to-end behavior, and review the existing-snapshot deltas. +- **Requirements:** R1-R5, R7, R8, R9, R12. +- **Files:** + - `tests/gen_fixtures.py`: new `numberformats_xlsx()` builder (mirrors `merged_xlsx()` at `tests/gen_fixtures.py:872`) writing `tests/fixtures/xlsx/handmade-numberformats.xlsx` with: a custom `0.00%` numFmt (id 164), builtin `$#,##0.00` (id 4), builtin `#,##0` (id 3), `0.00` (id 2), `0.00E+00` (id 11), a negative-section custom `0.00;(0.00)` (id 165), a currency-locale `[$$-409]#,##0.00` (id 166), a fraction format (id 12) cell, a `General` float, a string cell `5.75%`, a date cell, and a format-code literal cell with a pipe character; register the builder in `main()`. + - `tests/fixtures/xlsx/handmade-numberformats.xlsx`: generated artifact (regenerate via `python3 tests/gen_fixtures.py --skip-office` if the rest of the corpus allows, otherwise emit the fixture by hand from the same XML). + - `tests/snapshots/snapshots__xlsx__handmade-numberformats.xlsx.snap`: new snapshot via `INSTA_UPDATE` run, then reviewed. +- **Approach:** + - The handmade xlsx encodes: header row of labels, then rows of value/format pairs covering AE1-AE7 and the fallback cases. + - Run the snapshot suite; review every diff, including the certain `sheet.xlsx` diff (Assumptions); accept only diffs that match Excel-displayed values. + - Include one cell whose format code contains a pipe character inside a quoted literal to pin R12 escaping through the existing pipeline. +- **Test scenarios:** + - T4.1 Snapshot `handmade-numberformats.xlsx` contains `65.00%`, `$56,701.03`, `1,234.50`, `1,234,567`, `1.23E+05`, `(1.50)`, `$1,234.50` (currency-locale cell), the fraction-formatted value, raw `0.653035934239184`-style fallback only where the renderer rejects the code, `5.75%` for the string cell, and the ISO date. + - T4.2 The pre-existing `sheet.xlsx` snapshot diff is justified cell-by-cell against Excel display semantics. +- **Verification:** `cargo test --locked` (snapshot suite) green; `cargo fmt --all --check` and clippy clean; `python3 tests/gen_fixtures.py --skip-office` regenerates the fixture byte-identically (deterministic ZIP timestamps). + +## Verification Contract + +- `cargo fmt --all --check` +- `cargo clippy --workspace --all-targets --all-features -- -D warnings` +- `cargo test --locked` +- Manual cross-check (dev only, not committed): openpyxl writes `0.00%` / `$#,##0.00` / `#,##0` / `[$$-409]#,##0.00` cells, `anydoc::to_markdown` output matches Excel display; the issue #64 example values render per AE1, AE2, AE7; a tie value (1.005 with `0.00`) matches Excel's displayed output. +- The full snapshot corpus must remain green; every snapshot delta must be traced to the format change and reviewed against Excel display semantics. + +## Definition of Done + +**Global** + +- Issue #64's three example cells render their formatted values (AE1, AE2, AE7). +- All R1-R12 hold; renderer-rejected format families fall back to raw (R8) with a logged, non-fatal recovery path (R9), and resource-limit errors propagate (R9a). +- Verification Contract passes in a clean tree. +- No dead or experimental code remains: any approach variant abandoned during implementation is removed, not left behind commented or feature-gated. +- One pinned dependency added (`ssfmt 0.1.2`, `default-features = false`); no other runtime dependencies; no public API changes. +- Known limitation recorded in the PR description: formatted output is display-rounded to the format's precision (values differing below that precision become indistinguishable); General cells keep exact raw rendering. + +**Per unit** + +- U1: renderer adapter unit tests T1.1-T1.13 green; dependency pinned. +- U2: styles reader unit tests T2.1-T2.9 green. +- U3: parse-loop unit tests T3.1-T3.7 green; `format_data` signature change complete with all call sites updated. +- U4: fixture committed, snapshot committed and reviewed, generator regenerates byte-identically.