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/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. diff --git a/src/formats/sheet/mod.rs b/src/formats/sheet/mod.rs index f61ebcc..971762e 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; @@ -24,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)?; @@ -85,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 { @@ -155,13 +167,25 @@ 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| { + // 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(), + }, 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()), @@ -207,42 +231,100 @@ 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 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 { + 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("") + ), ), ]; + 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".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.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 { @@ -272,7 +354,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] @@ -307,4 +389,117 @@ mod tests { assert_eq!(format_duration_days(days), "26:30:15"); assert_eq!(format_duration_days(-0.5), "-12:00:00"); } + + 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(&super::test_util::xlsx_with(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(&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(&super::test_util::xlsx_with(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(&super::test_util::xlsx_with(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(&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:?}"); + } + + #[test] + fn string_cells_ignore_format_codes() { + let styles = r#""#; + let sheet = r#"5.75%"#; + let doc = parse(&super::test_util::xlsx_with(Some(styles), sheet)).unwrap(); + 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 new file mode 100644 index 0000000..1aa90fc --- /dev/null +++ b/src/formats/sheet/numfmt.rs @@ -0,0 +1,260 @@ +//! 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. 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() +} + +/// Resolve the format code for a style's `numFmtId`. +/// +/// 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, 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()); + } + 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. +/// +/// 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; + } + 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, + // 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), + ('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 +} + +/// 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::*; + + 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 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())); + } + + #[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())); + // 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] + 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 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); + 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..08997f6 --- /dev/null +++ b/src/formats/sheet/styles.rs @@ -0,0 +1,383 @@ +//! 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::R; +use crate::package::xml::parse_xml; +use std::collections::HashMap; +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 { + 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 = 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!( + "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(None); + }; + let mut by_sheet = HashMap::new(); + for (name, path) in parts { + let Some(part) = pkg.optional_part(&path)? else { + log::warn!("spreadsheet: sheet part {path} missing; skipping its formats"); + continue; + }; + 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 })) + } + + /// 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]>) -> 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)) = + (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(); + // 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). + if xfs.is_empty() { + return Ok(None); + } + 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.optional_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], +) -> 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") { + 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()); + } + } + } + Ok(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 + .checked_mul(26) + .and_then(|c| c.checked_add(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()?; + // 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)) +} + +#[cfg(test)] +mod tests { + use super::*; + + 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 = 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%")); + } + + #[test] + fn builtin_id_resolves_without_custom_entry() { + let styles = r#""#; + 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 = super::super::test_util::xlsx_with( + Some(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 = super::super::test_util::xlsx_with( + Some(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 = super::super::test_util::xlsx_with( + Some(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 = 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 = 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 = super::super::test_util::xlsx_with( + Some(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 = 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); + } + + #[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))); + 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); + } +} diff --git a/tests/fixtures/xlsx/handmade-numberformats.xlsx b/tests/fixtures/xlsx/handmade-numberformats.xlsx new file mode 100644 index 0000000..8987e94 Binary files /dev/null and b/tests/fixtures/xlsx/handmade-numberformats.xlsx differ 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\| | 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 |