From a0a17b154cb0c59855cdbeb2863ce05dc75920fd Mon Sep 17 00:00:00 2001 From: Eduardo Soares Date: Mon, 12 Oct 2026 19:29:23 -0300 Subject: [PATCH 1/5] #833: whole-row/column formatting re-fits row heights --- crates/engine/src/cmd/format.rs | 8 +- crates/engine/tests/row_autofit.rs | 47 ++++ .../2026-10-12-row-autofit-oversized-fonts.md | 238 ++++++++++++++++++ 3 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 crates/engine/tests/row_autofit.rs create mode 100644 docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index dde5fa1..2c3f397 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -237,10 +237,12 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> // Existing cells in those lines. let cells: Vec<(CellRef, StyleId)> = cx.wb.sheet(sheet).map(|sh| sh.cells.iter_range(*r).map(|(c, x)| (c, x.style)).collect()).unwrap_or_default(); - for (c, st) in cells { - let new = *cache.entry(st).or_insert_with(|| cx.wb.styles.derive(st, &f)); - cx.sheet_mut(sheet)?.set_style(c, new); + for (c, st) in &cells { + let new = *cache.entry(*st).or_insert_with(|| cx.wb.styles.derive(*st, &f)); + cx.sheet_mut(sheet)?.set_style(*c, new); } + // Formatting changed these cells' fonts: re-fit their rows (Excel auto-fits). + cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row))); continue; } if r.count() > 4_000_000 { diff --git a/crates/engine/tests/row_autofit.rs b/crates/engine/tests/row_autofit.rs new file mode 100644 index 0000000..8b9af4d --- /dev/null +++ b/crates/engine/tests/row_autofit.rs @@ -0,0 +1,47 @@ +//! Whole-row and whole-column formatting must re-fit row heights (issue #833). + +use serde_json::json; + +fn sheet_mut(s: &mut gridcraft_engine::Session) -> &mut gridcraft_engine::model::Sheet { + std::sync::Arc::make_mut(&mut s.active_mut().unwrap().wb).sheet_mut(0).unwrap() +} + +#[test] +fn whole_row_font_format_grows_row_height_and_undoes() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + let before = s.doc().unwrap().wb.active().unwrap().row_height(4); + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + let after = s.doc().unwrap().wb.active().unwrap().row_height(4); + assert!(after > before + 10.0, "20pt whole-row format should grow the row: before={before}, after={after}"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), before, "undo must restore the height"); +} + +#[test] +fn whole_column_font_format_grows_only_rows_with_cells() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a"]]})).unwrap(); + s.execute("range.setValues", json!({"range": "A50", "values": [["b"]]})).unwrap(); + s.execute("home.fontSize", json!({"range": "A:A", "size": 20})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + assert!(sh.row_height(0) > 30.0 && sh.row_height(49) > 30.0, "rows with cells grow"); + assert!(sh.rows.len() <= 3, "full-column format must fit only rows that have cells, rows map has {} entries", sh.rows.len()); +} + +#[test] +fn whole_row_format_does_not_resize_custom_height_rows() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + { + let sh = sheet_mut(&mut s); + let e = sh.rows.entry(4).or_default(); + e.custom = true; + e.size = Some(25.0); + } + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows keep their height"); +} diff --git a/docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md b/docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md new file mode 100644 index 0000000..af25cfc --- /dev/null +++ b/docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md @@ -0,0 +1,238 @@ +# Row Auto-Fit for Oversized Fonts (#833) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Rows without a user-set height grow to fit their tallest whole-cell font when a file opens and when whole-row/column formatting changes fonts, matching Excel's auto-fit; `customHeight` rows keep their height and clip as Excel does. + +**Architecture:** Two one-site fixes in existing flows. (1) `apply_style`'s whole-row/column branch already collects the touched cells — extend `Ctx::fit_rows` from them so the existing `commit()` → `auto_row_height()` pass re-fits the rows. (2) All file opens funnel through `open_bytes_with_password` in `io.rs` — after parsing, grow non-custom rows via the existing `auto_row_height`, clamped grow-only (a file's taller non-custom row is never shrunk; our estimate can differ from Excel's). No render change: once rows fit, the degenerate top-pin clamp in `cell_text_y` only fires for `customHeight` rows, where clipping matches Excel. + +**Tech Stack:** Rust; crates `engine` (L5) using `model`/`xlsx` (L2) APIs already in scope. No new dependencies. + +**Spec:** Upstream issue https://github.com/storytold/gridcraft/issues/833 and the bounded design approved in chat 2026-10-12 (sequencing decision: plain-cell paths on main now; per-run font measurement is a follow-up once rich-text-runs PR #837 merges — explicitly out of scope here). Triage facts: `auto_row_height` (`crates/engine/src/cmd/mod.rs:458`) has one call site (`commit`), and `format.rs`'s line-target branch `continue`s before the `fit_rows.extend` that the cell-by-cell branch has. + +## Global Constraints + +- No `unwrap`/`expect`/`panic!`/`unsafe` in non-test code (crate lints deny them); errors via `Result` and `?`. Tests may `unwrap`. +- Input-derived numbers are hostile: never iterate `1..MAX_ROWS`; fit only rows that have cells (sparse iteration). Existing caps stay (100,000-row fit guard, 546.0 height cap in `auto_row_height`). +- Excel-parity rules fixed by the spec: non-`custom` rows grow to fit their tallest whole-cell font on open and after whole-row/column formatting; `custom` rows never change; hidden rows stay hidden; open is grow-only (never shrinks a file's height). +- Layering: changes stay in `engine` (L5); `model`/`xlsx`/`core` are consumed, not modified. +- Never break wasm: new code is pure computation, no `std::env`, no threads. +- `cargo xtask ci` green before every commit (fmt, clippy -D warnings, tests, assets, layers, wasm). +- One task id per commit: `#833: `. +- Tests build workbooks in code; no binary fixtures. +- Per-run (rich text) font sizes are out of scope; do not touch `grid.rs` rendering. + +## Review Focus + +1. **`customHeight` rows must never be resized** by open-time fitting or whole-row formatting (user-set heights are sacred). → Task 1 test 3, Task 2 test B. +2. **Open must never shrink** a file's taller non-custom row (our line-height estimate differs from Excel's; shrinking would visibly change files that already look right). → Task 2 test C (grow-only clamp). +3. **Whole-column formatting spans 1,048,576 logical rows** — fitting must mark only rows that have cells, not the million-row span (hostile-input rule). → Task 1 test 2 asserts the sheet's `rows` map stays sparse. +4. **Default-font workbooks open as a no-op**: the plain-row fast path in `auto_row_height` must keep default heights and not pollute the `rows` map (big-file open cost stays O(content), sample files unchanged). → Task 2 test E. +5. **Hidden rows stay hidden** through open-time fitting and formatting. → Task 2 test D. + +(Undo integrity — formatting's height change must vanish on undo — is pinned inside Task 1 test 1.) + +--- + +### Task 1: Whole-row/column formatting re-fits row heights + +**Files:** +- Modify: `crates/engine/src/cmd/format.rs:237-244` (the line-target branch of `apply_style`) +- Test: `crates/engine/tests/row_autofit.rs` (new file) + +**Interfaces:** +- Consumes: `Ctx::fit_rows: Vec<(usize, u32)>` (sheet index, row) — already deduped by `commit()`; `cells: Vec<(CellRef, StyleId)>` already collected at format.rs:238-239. +- Produces: nothing new — behavior only. + +- [ ] **Step 1: Write the failing tests** in `crates/engine/tests/row_autofit.rs`, following the harness in `crates/engine/tests/autofit_filter_buttons.rs` (`Session::new()`, `s.new_workbook()`, `s.execute(...)`, read state via `s.doc().unwrap().wb.active().unwrap()`; copy its `sheet_mut` helper for setting row info): + +```rust +use serde_json::json; + +#[test] +fn whole_row_font_format_grows_row_height_and_undoes() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + let before = s.doc().unwrap().wb.active().unwrap().row_height(4); + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + let after = s.doc().unwrap().wb.active().unwrap().row_height(4); + assert!(after > before + 10.0, "20pt whole-row format should grow the row: before={before}, after={after}"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), before, "undo must restore the height"); +} + +#[test] +fn whole_column_font_format_grows_only_rows_with_cells() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a"]]})).unwrap(); + s.execute("range.setValues", json!({"range": "A50", "values": [["b"]]})).unwrap(); + s.execute("home.fontSize", json!({"range": "A:A", "size": 20})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + assert!(sh.row_height(0) > 30.0 && sh.row_height(49) > 30.0, "rows with cells grow"); + assert!(sh.rows.len() <= 3, "full-column format must fit only rows that have cells, rows map has {} entries", sh.rows.len()); +} + +#[test] +fn whole_row_format_does_not_resize_custom_height_rows() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + { let sh = sheet_mut(&mut s); let e = sh.rows.entry(4).or_default(); e.custom = true; e.size = Some(25.0); } + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows keep their height"); +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test -p gridcraft-engine --test row_autofit` +Expected: tests 1 and 2 FAIL (height unchanged — the line branch never marks `fit_rows`); test 3 may PASS already (pins existing behavior). + +- [ ] **Step 3: Implement the marking** in `crates/engine/src/cmd/format.rs`, in the `if full_rows || full_cols` branch, after the `for (c, st) in cells` loop and before its `continue` (line 244): + +```rust +// Formatting changed these cells' fonts: re-fit their rows (Excel auto-fits). +cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row))); +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test -p gridcraft-engine --test row_autofit` +Expected: 3 passed. Also run `cargo test -p gridcraft-engine` (no regressions in style/undo behavior). + +- [ ] **Step 5: Commit** + +```bash +git add crates/engine/src/cmd/format.rs crates/engine/tests/row_autofit.rs +git commit -m "#833: whole-row/column formatting re-fits row heights" +``` + +### Task 2: Opened files grow non-custom rows to their tallest font + +**Files:** +- Modify: `crates/engine/src/io.rs:57-74` (`open_bytes_with_password`) and add `fn fit_imported_rows` next to it +- Test: `crates/engine/src/tests.rs` (extend, near the existing `file.open` tests around line 1004) + +**Interfaces:** +- Consumes: `crate::cmd::auto_row_height(wb: &mut Workbook, si: usize, row: u32)` (pub, cmd/mod.rs:458); `Workbook::sheets: Vec>` (pub field); `Sheet::cells.iter() -> impl Iterator` (model store.rs:89); `Sheet::rows: map row -> LineInfo { size: Option, custom: bool, hidden: bool, .. }`; model builders `Workbook::new()`, `wb.styles.derive(StyleId::default(), |s: &mut Style| s.font.size = 20.0) -> StyleId`, `sh.set_value(CellRef, Value)`, `sh.set_style(CellRef, StyleId)`; `gridcraft_xlsx::write_xlsx(&wb) -> Result>`; `crate::io::base64_encode(&[u8])`. +- Produces: `fn fit_imported_rows(wb: &mut Workbook)` — private to `io.rs`, called at both `open_plain` call sites in `open_bytes_with_password`. + +- [ ] **Step 1: Write the failing tests and fixture builder** in `crates/engine/src/tests.rs`. Fixture workbooks are built with the model API directly (never through commands, which would auto-fit them) and exported with `write_xlsx`, which omits `ht` for rows without a size — exactly the bug's input: + +```rust +fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { + // row_info = (size, custom, hidden) for row 5 (index 4), None = no row entry at all. + // Cell A5 = "plain big bold", whole-cell font 20pt. 11pt cell in A1 for contrast rows. + // Returns write_xlsx bytes. Uses Workbook::new, styles.derive, set_value, set_style. +} + +#[test] // A — the issue's repro +fn open_grows_default_height_rows_to_their_font() { + let mut s = Session::new(); + s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + let h = s.doc().unwrap().wb.active().unwrap().row_height(4); + assert!(h >= 30.0, "20pt cell must grow its default-height row on open, got {h}"); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(0), /* default height captured before open */, "11pt row stays default"); +} + +#[test] // B — Review Focus 1 +fn open_keeps_custom_height_rows() { + // fixture with row_info (25.0, true, false): open → row_height(4) == 25.0 +} + +#[test] // C — Review Focus 2 +fn open_never_shrinks_taller_auto_rows() { + // fixture with row_info (60.0, false, false) and an 11pt cell: open → row_height(4) == 60.0 +} + +#[test] // D — Review Focus 5 +fn open_fitting_keeps_hidden_rows_hidden() { + // fixture with row_info (20.0, false, true) and the 20pt cell: open → height grows or keeps 20.0, + // and rows.get(&4).hidden stays true +} + +#[test] // E — Review Focus 4 +fn open_leaves_plain_workbooks_untouched() { + // fixture with only 11pt cells, no row entries: open → sh.rows.get(&0) is None or has size == None +} +``` + +Comments carry each test's assertions where code is elided; fill them in fully when writing. + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test -p gridcraft-engine open_` +Expected: A FAILS (row stays default — nothing fits on open); B and C may PASS (pin current behavior); D/E PASS or FAIL depending on current import behavior — record actual results in the commit-free state; only A's failure is required before implementing. + +- [ ] **Step 3: Implement `fit_imported_rows` in `crates/engine/src/io.rs` and call it from both `open_plain` sites in `open_bytes_with_password`** (the plain early-return at ~line 58-59 and the decrypted path at ~line 71; keep warnings untouched): + +```rust +/// Excel auto-fits rows to their tallest font when opening a file: rows without a user-set +/// height grow to fit their content. Rows the file made taller keep their height — our +/// line-height estimate can differ from Excel's, and shrinking files that already look +/// right would be a visible regression. Custom-height rows are left alone entirely. +fn fit_imported_rows(wb: &mut Workbook) { + for si in 0..wb.sheets.len() { + let Some(sh) = wb.sheet(si) else { continue }; + let mut rows: Vec = sh.cells.iter().map(|(c, _)| c.row).collect(); + rows.sort_unstable(); + rows.dedup(); + for row in rows { + let before = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); + crate::cmd::auto_row_height(wb, si, row); + let Some(before) = before else { continue }; + let after = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); + if after.is_none_or(|a| before > a + 0.5) { + if let Some(sh) = wb.sheet_mut(si) { + sh.rows.entry(row).or_default().size = Some(before); + } + } + } + } +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cargo test -p gridcraft-engine open_` then `cargo test -p gridcraft-engine` (all), plus a sample sanity check: `cargo test -p gridcraft-engine --test command_sequence_fuzz` (undo/round-trip/recalc invariants over opens). + +- [ ] **Step 5: Commit** + +```bash +git add crates/engine/src/io.rs crates/engine/src/tests.rs +git commit -m "#833: opened files grow non-custom rows to their tallest font" +``` + +### Task 3: Docs and tracking + +**Files:** +- Modify: `docs/target-app-parity.md:165` (the "Rows and columns … AutoFit" row) +- Modify: `docs/gaps.md` (revision history; check §24 for a stale row-height mention) +- Modify: `ROADMAP.md` (progress log + revision history + "Last updated" line) + +**Interfaces:** +- Consumes: merged state of Tasks 1-2. +- Produces: nothing (docs). + +- [ ] **Step 1: Update the three docs** + - `docs/target-app-parity.md:165`: append to the evidence column: `; rows auto-fit oversized fonts on open and row formatting (#833)`. + - `docs/gaps.md`: add revision-history row `| 2026-10-12 | minor | rows auto-fit oversized fonts on open and whole-row/column formatting (#833) |`. + - `ROADMAP.md`: progress-log row `| 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) |`; matching revision-history row; bump the `**Last updated:**` date. + +- [ ] **Step 2: Verify docs build/lint state unchanged** + +Run: `cargo xtask ci` +Expected: green (fmt, clippy, tests, assets, layers, wasm). + +- [ ] **Step 3: Commit** + +```bash +git add docs/target-app-parity.md docs/gaps.md ROADMAP.md +git commit -m "#833: docs — track row auto-fit for oversized fonts" +``` + +--- + +**Post-PR (not a plan task):** comment on issue #833 linking the PR and noting the per-run font-size follow-up that lands after rich-text-runs PR #837 merges (per-run sizes are not measured by `auto_row_height` yet). From 1086e4aab8e278d0804885f70271e1b292035a19 Mon Sep 17 00:00:00 2001 From: Eduardo Soares Date: Mon, 12 Oct 2026 19:44:27 -0300 Subject: [PATCH 2/5] #833: opened files grow non-custom rows to their tallest font --- crates/engine/src/io.rs | 31 ++++++++++++++++-- crates/engine/src/tests.rs | 65 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index bc7011d..96746d9 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -56,7 +56,9 @@ pub fn open_bytes(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { /// quote the password. pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str>) -> Result<(Workbook, Vec)> { if gridcraft_xlsx::sniff(bytes) != gridcraft_xlsx::Format::Encrypted { - return open_plain(name, bytes); + let (mut wb, warnings) = open_plain(name, bytes)?; + fit_imported_rows(&mut wb); + return Ok((wb, warnings)); } // A scheme no password opens is said straight away, rather than after asking for one. let decrypt_error = |e: gridcraft_xlsx::DecryptError| EngineError::Other(decrypt_message(name, e)); @@ -68,11 +70,36 @@ pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str> if gridcraft_xlsx::sniff(&plain) == gridcraft_xlsx::Format::Encrypted { return Err(EngineError::Other(format!("We can't open '{name}': the encrypted file is damaged (it holds another encrypted file)."))); } - let (wb, mut warnings) = open_plain(name, &plain)?; + let (mut wb, mut warnings) = open_plain(name, &plain)?; + fit_imported_rows(&mut wb); warnings.insert(0, DECRYPTED_NOTICE.into()); Ok((wb, warnings)) } +/// Excel auto-fits rows to their tallest font when opening a file: rows without a user-set +/// height grow to fit their content. Rows the file made taller keep their height — our +/// line-height estimate can differ from Excel's, and shrinking files that already look +/// right would be a visible regression. Custom-height rows are left alone entirely. +fn fit_imported_rows(wb: &mut Workbook) { + for si in 0..wb.sheets.len() { + let Some(sh) = wb.sheet(si) else { continue }; + let mut rows: Vec = sh.cells.iter().map(|(c, _)| c.row).collect(); + rows.sort_unstable(); + rows.dedup(); + for row in rows { + let before = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); + crate::cmd::auto_row_height(wb, si, row); + let Some(before) = before else { continue }; + let after = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); + if after.is_none_or(|a| before > a + 0.5) + && let Some(sh) = wb.sheet_mut(si) + { + sh.rows.entry(row).or_default().size = Some(before); + } + } + } +} + /// What to tell the user when an encrypted workbook can't be decrypted. Never quotes the password. fn decrypt_message(name: &str, e: gridcraft_xlsx::DecryptError) -> String { match e { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 33fdc11..49f67e8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1007,6 +1007,71 @@ fn spill_references_roundtrip_through_xlsx() { assert_eq!((v(&s, "C1"), v(&s, "C2")), (Value::Number(10.0), Value::Number(4.0))); } +/// Fixture for the open-time row fit (issue #833): `row_info` = (size, custom, hidden) for +/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold", whole-cell +/// font 20pt. 11pt cell in A1 for contrast rows. Built with the model API, never through +/// commands — commands would auto-fit the rows — and exported with `write_xlsx`, which +/// omits `ht` for rows without a size: exactly the bug's input. +fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { + let mut wb = gridcraft_model::Workbook::new(); + let big = wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |s| s.font.size = 20.0); + let sh = wb.sheet_mut(0).unwrap(); + sh.set_value(CellRef::new(0, 0), Value::text("small")); + sh.set_value(CellRef::new(4, 0), Value::text("plain big bold")); + sh.set_style(CellRef::new(4, 0), big); + if let Some((size, custom, hidden)) = row_info { + let e = sh.rows.entry(4).or_default(); + e.size = Some(size); + e.custom = custom; + e.hidden = hidden; + } + gridcraft_xlsx::write_xlsx(&wb).unwrap() +} + +#[test] // A — the issue's repro +fn open_grows_default_height_rows_to_their_font() { + let default_h = gridcraft_model::Workbook::new().active().unwrap().row_height(0); + let mut s = Session::new(); + s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + let h = s.doc().unwrap().wb.active().unwrap().row_height(4); + assert!(h >= 30.0, "20pt cell must grow its default-height row on open, got {h}"); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(0), default_h, "11pt row stays default"); +} + +#[test] // B — Review Focus 1 +fn open_keeps_custom_height_rows() { + let mut s = Session::new(); + s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false))))})) + .unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows are left alone on open"); +} + +#[test] // C — Review Focus 2 +fn open_never_shrinks_taller_auto_rows() { + let mut s = Session::new(); + s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false))))})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "open is grow-only: a 60px auto-height row keeps its height"); +} + +#[test] // D — Review Focus 5 +fn open_fitting_keeps_hidden_rows_hidden() { + let mut s = Session::new(); + s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true))))})) + .unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + let info = sh.rows.get(&4).unwrap(); + assert!(info.size.is_none_or(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size); + assert!(info.hidden, "fitting a hidden row must not unhide it"); +} + +#[test] // E — Review Focus 4 +fn open_leaves_plain_workbooks_untouched() { + let mut s = Session::new(); + s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + assert!(sh.rows.get(&0).is_none_or(|i| i.size.is_none()), "11pt default rows get no height entry"); +} + #[test] fn tables_and_structured_refs() { let mut s = s(); From dc3ccc4ea000aa93a6ceff4353d8ba6fe27cf0ba Mon Sep 17 00:00:00 2001 From: Eduardo Soares Date: Mon, 12 Oct 2026 19:51:18 -0300 Subject: [PATCH 3/5] =?UTF-8?q?#833:=20docs=20=E2=80=94=20track=20row=20au?= =?UTF-8?q?to-fit=20for=20oversized=20fonts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Conflicts: # ROADMAP.md # docs/gaps.md --- ROADMAP.md | 4 +++- docs/gaps.md | 3 ++- docs/target-app-parity.md | 5 +++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index e8e1c06..912df09 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (direct desktop printing through craft-print) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | File › Print selects a system printer, copies, duplex and grayscale, submits PDF jobs through craft-print on a worker and shows spooler errors; browser printing and PDF export remain available | | 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found | | 2026-10-11 | Password-protected workbooks open: GridCraft asks for the password and decrypts Agile and Standard encryption (#11) | @@ -178,6 +179,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row | | 2026-10-12 | minor | Desktop File > Print now discovers printers and submits PDF jobs through pinned craft-print with printer choice, copies, duplex and grayscale; browser printing and PDF export remain available. Remaining print parity includes page ranges and driver-specific tray/options UI; physical printer fidelity needs hardware validation. | | 2026-10-11 | minor | Password-encrypted workbooks open (#11); progress log row | | 2026-10-11 | minor | Forecast Sheet (#322); progress log row | diff --git a/docs/gaps.md b/docs/gaps.md index 17a56b8..1d8fa95 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (direct desktop printing through craft-print) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) Every known shortfall against Excel, one entry each, ranked by user impact (what blocks someone using GridCraft for real work first). This is the work list: pick from the top, check the open @@ -160,6 +160,7 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha- | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | rows auto-fit oversized fonts on open and whole-row/column formatting (#833) | | 2026-10-12 | minor | Desktop File > Print now discovers printers and submits PDF jobs through pinned craft-print with printer choice, copies, duplex and grayscale; browser printing and PDF export remain available. Remaining print parity includes page ranges and driver-specific tray/options UI; physical printer fidelity needs hardware validation. | | 2026-10-11 | minor | §6: iterative calculation for circular references landed (Maximum Iterations, Maximum Change) | | 2026-10-11 | minor | §15: trim-reference operators (`A1:.E10`, `A1.:E10`, `A1.:.E10`) parse and evaluate | diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 8b82092..d2f65d5 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,6 +1,6 @@ # Target-app parity: GridCraft vs Microsoft Excel -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (stability: command-sequence fuzzing against undo/redo, XLSX round-trip and full-recalc invariants) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and row formatting) · **Target:** Microsoft Excel (Microsoft 365) This is the authoritative parity assessment. [`ROADMAP.md`](../ROADMAP.md) summarizes it, [`gaps.md`](gaps.md) lists every shortfall, and the area checklists hold the detail: @@ -162,7 +162,7 @@ edge cases are left out, along with everything excluded from the mainstream numb | Typing data, simple formulas, AutoSum (SUM, AVERAGE, arithmetic) | 20 | 88 | Solid; 15-digit equality (#57) is rare at this level | | Basic formatting: fonts, bold, fills, borders, currency/percent/date formats | 15 | 85 | | | Copy, paste, fill handle, undo/redo | 15 | 78 | Selection bug (#28), monthly fill (#132) | -| Rows and columns: insert, delete, widths, AutoFit | 10 | 82 | AutoFit fixed (#42, #88) | +| Rows and columns: insert, delete, widths, AutoFit | 10 | 82 | AutoFit fixed (#42, #88); rows auto-fit oversized fonts on open and row formatting (#833) | | Sort and filter | 10 | 82 | | | Save, Save As, reopen XLSX | 10 | 85 | | | A basic chart | 8 | 55 | Created easily; can't be resized (#190) | @@ -332,6 +332,7 @@ The inventory carried over from the 2026-10-07 ROADMAP.md, updated for what land | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Essentials AutoFit evidence: rows auto-fit oversized fonts on open and row formatting (#833) | | 2026-10-11 | minor | Stability: command-sequence fuzz harness (`crates/engine/tests/command_sequence_fuzz.rs`) checks random command sequences against undo/redo, XLSX round-trip, full-recalc and structure invariants; the sequence bugs it found are fixed (no score change) | | 2026-10-11 | minor | Password-encrypted workbooks open (#11): Essentials file-opening work 24–36 h | | 2026-10-11 | minor | Worksheet functions 505 / 523 (96.6%; 496 working): ODDFPRICE, ODDFYIELD, ODDLPRICE, ODDLYIELD | From 26c6e3df441b6716a38aa5ccbbfebedc598de097 Mon Sep 17 00:00:00 2001 From: Eduardo Soares Date: Mon, 12 Oct 2026 20:05:58 -0300 Subject: [PATCH 4/5] #833: pin hidden-row formatting, grow-only restore branch, tighten tests --- crates/engine/src/cmd/format.rs | 2 +- crates/engine/src/tests.rs | 42 +++++++++++++++++++++--------- crates/engine/tests/row_autofit.rs | 14 ++++++++++ 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 2c3f397..da7bf62 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -241,7 +241,7 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> let new = *cache.entry(*st).or_insert_with(|| cx.wb.styles.derive(*st, &f)); cx.sheet_mut(sheet)?.set_style(*c, new); } - // Formatting changed these cells' fonts: re-fit their rows (Excel auto-fits). + // Formatting touched these cells: re-fit their rows (Excel auto-fits). cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row))); continue; } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 49f67e8..7fcc8fe 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1008,17 +1008,20 @@ fn spill_references_roundtrip_through_xlsx() { } /// Fixture for the open-time row fit (issue #833): `row_info` = (size, custom, hidden) for -/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold", whole-cell -/// font 20pt. 11pt cell in A1 for contrast rows. Built with the model API, never through -/// commands — commands would auto-fit the rows — and exported with `write_xlsx`, which -/// omits `ht` for rows without a size: exactly the bug's input. -fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { +/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold" — whole-cell +/// font 20pt with `big_font`, plain 11pt without (a tall plain row's fit removes its entry). +/// 11pt cell in A1 for contrast rows. Built with the model API, never through commands — +/// commands would auto-fit the rows — and exported with `write_xlsx`, which omits `ht` for +/// rows without a size: exactly the bug's input. +fn big_font_workbook(row_info: Option<(f32, bool, bool)>, big_font: bool) -> Vec { let mut wb = gridcraft_model::Workbook::new(); let big = wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |s| s.font.size = 20.0); let sh = wb.sheet_mut(0).unwrap(); sh.set_value(CellRef::new(0, 0), Value::text("small")); sh.set_value(CellRef::new(4, 0), Value::text("plain big bold")); - sh.set_style(CellRef::new(4, 0), big); + if big_font { + sh.set_style(CellRef::new(4, 0), big); + } if let Some((size, custom, hidden)) = row_info { let e = sh.rows.entry(4).or_default(); e.size = Some(size); @@ -1032,7 +1035,7 @@ fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { fn open_grows_default_height_rows_to_their_font() { let default_h = gridcraft_model::Workbook::new().active().unwrap().row_height(0); let mut s = Session::new(); - s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None, true))})).unwrap(); let h = s.doc().unwrap().wb.active().unwrap().row_height(4); assert!(h >= 30.0, "20pt cell must grow its default-height row on open, got {h}"); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(0), default_h, "11pt row stays default"); @@ -1041,35 +1044,48 @@ fn open_grows_default_height_rows_to_their_font() { #[test] // B — Review Focus 1 fn open_keeps_custom_height_rows() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false))))})) + s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false)), true))})) .unwrap(); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows are left alone on open"); } -#[test] // C — Review Focus 2 +#[test] // C — Review Focus 2 (Some→Some: the fit shrinks the row, the clamp keeps the taller height) fn open_never_shrinks_taller_auto_rows() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false))))})).unwrap(); + s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false)), true))})) + .unwrap(); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "open is grow-only: a 60px auto-height row keeps its height"); } +#[test] // C′ — Review Focus 2 (Some→None: a plain tall row's entry is removed by the fit, the clamp restores it) +fn open_restores_height_of_plain_rows_the_fit_removed() { + let mut s = Session::new(); + s.execute( + "file.open", + json!({"name": "tall-plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false)), false))}), + ) + .unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "a tall plain row keeps its height after the fit removes its entry"); +} + #[test] // D — Review Focus 5 fn open_fitting_keeps_hidden_rows_hidden() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true))))})) + s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true)), true))})) .unwrap(); let sh = s.doc().unwrap().wb.active().unwrap(); let info = sh.rows.get(&4).unwrap(); - assert!(info.size.is_none_or(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size); + assert!(info.size.is_some_and(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size); assert!(info.hidden, "fitting a hidden row must not unhide it"); } #[test] // E — Review Focus 4 fn open_leaves_plain_workbooks_untouched() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None, false))})).unwrap(); let sh = s.doc().unwrap().wb.active().unwrap(); assert!(sh.rows.get(&0).is_none_or(|i| i.size.is_none()), "11pt default rows get no height entry"); + assert!(sh.rows.values().all(|i| i.size.is_none()), "no row gains a height entry"); } #[test] diff --git a/crates/engine/tests/row_autofit.rs b/crates/engine/tests/row_autofit.rs index 8b9af4d..5bea5ab 100644 --- a/crates/engine/tests/row_autofit.rs +++ b/crates/engine/tests/row_autofit.rs @@ -45,3 +45,17 @@ fn whole_row_format_does_not_resize_custom_height_rows() { s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows keep their height"); } + +#[test] +fn whole_row_format_keeps_hidden_rows_hidden() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + let before = s.doc().unwrap().wb.active().unwrap().row_height(4); + sheet_mut(&mut s).rows.entry(4).or_default().hidden = true; + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + let info = sh.rows.get(&4).unwrap(); + assert!(info.size.unwrap_or(before) > before + 10.0, "hidden row still grows to its font: before={before}, size={:?}", info.size); + assert!(info.hidden, "formatting a hidden row must not unhide it"); +} From a8cd6f4b63b26bef9cea03444441688493541222 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 13 Oct 2026 01:52:12 -0400 Subject: [PATCH 5/5] Drop the planning notes from the PR Co-Authored-By: Claude Opus 5.5 --- .../2026-10-12-row-autofit-oversized-fonts.md | 238 ------------------ 1 file changed, 238 deletions(-) delete mode 100644 docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md diff --git a/docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md b/docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md deleted file mode 100644 index af25cfc..0000000 --- a/docs/superpowers/plans/2026-10-12-row-autofit-oversized-fonts.md +++ /dev/null @@ -1,238 +0,0 @@ -# Row Auto-Fit for Oversized Fonts (#833) Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Rows without a user-set height grow to fit their tallest whole-cell font when a file opens and when whole-row/column formatting changes fonts, matching Excel's auto-fit; `customHeight` rows keep their height and clip as Excel does. - -**Architecture:** Two one-site fixes in existing flows. (1) `apply_style`'s whole-row/column branch already collects the touched cells — extend `Ctx::fit_rows` from them so the existing `commit()` → `auto_row_height()` pass re-fits the rows. (2) All file opens funnel through `open_bytes_with_password` in `io.rs` — after parsing, grow non-custom rows via the existing `auto_row_height`, clamped grow-only (a file's taller non-custom row is never shrunk; our estimate can differ from Excel's). No render change: once rows fit, the degenerate top-pin clamp in `cell_text_y` only fires for `customHeight` rows, where clipping matches Excel. - -**Tech Stack:** Rust; crates `engine` (L5) using `model`/`xlsx` (L2) APIs already in scope. No new dependencies. - -**Spec:** Upstream issue https://github.com/storytold/gridcraft/issues/833 and the bounded design approved in chat 2026-10-12 (sequencing decision: plain-cell paths on main now; per-run font measurement is a follow-up once rich-text-runs PR #837 merges — explicitly out of scope here). Triage facts: `auto_row_height` (`crates/engine/src/cmd/mod.rs:458`) has one call site (`commit`), and `format.rs`'s line-target branch `continue`s before the `fit_rows.extend` that the cell-by-cell branch has. - -## Global Constraints - -- No `unwrap`/`expect`/`panic!`/`unsafe` in non-test code (crate lints deny them); errors via `Result` and `?`. Tests may `unwrap`. -- Input-derived numbers are hostile: never iterate `1..MAX_ROWS`; fit only rows that have cells (sparse iteration). Existing caps stay (100,000-row fit guard, 546.0 height cap in `auto_row_height`). -- Excel-parity rules fixed by the spec: non-`custom` rows grow to fit their tallest whole-cell font on open and after whole-row/column formatting; `custom` rows never change; hidden rows stay hidden; open is grow-only (never shrinks a file's height). -- Layering: changes stay in `engine` (L5); `model`/`xlsx`/`core` are consumed, not modified. -- Never break wasm: new code is pure computation, no `std::env`, no threads. -- `cargo xtask ci` green before every commit (fmt, clippy -D warnings, tests, assets, layers, wasm). -- One task id per commit: `#833: `. -- Tests build workbooks in code; no binary fixtures. -- Per-run (rich text) font sizes are out of scope; do not touch `grid.rs` rendering. - -## Review Focus - -1. **`customHeight` rows must never be resized** by open-time fitting or whole-row formatting (user-set heights are sacred). → Task 1 test 3, Task 2 test B. -2. **Open must never shrink** a file's taller non-custom row (our line-height estimate differs from Excel's; shrinking would visibly change files that already look right). → Task 2 test C (grow-only clamp). -3. **Whole-column formatting spans 1,048,576 logical rows** — fitting must mark only rows that have cells, not the million-row span (hostile-input rule). → Task 1 test 2 asserts the sheet's `rows` map stays sparse. -4. **Default-font workbooks open as a no-op**: the plain-row fast path in `auto_row_height` must keep default heights and not pollute the `rows` map (big-file open cost stays O(content), sample files unchanged). → Task 2 test E. -5. **Hidden rows stay hidden** through open-time fitting and formatting. → Task 2 test D. - -(Undo integrity — formatting's height change must vanish on undo — is pinned inside Task 1 test 1.) - ---- - -### Task 1: Whole-row/column formatting re-fits row heights - -**Files:** -- Modify: `crates/engine/src/cmd/format.rs:237-244` (the line-target branch of `apply_style`) -- Test: `crates/engine/tests/row_autofit.rs` (new file) - -**Interfaces:** -- Consumes: `Ctx::fit_rows: Vec<(usize, u32)>` (sheet index, row) — already deduped by `commit()`; `cells: Vec<(CellRef, StyleId)>` already collected at format.rs:238-239. -- Produces: nothing new — behavior only. - -- [ ] **Step 1: Write the failing tests** in `crates/engine/tests/row_autofit.rs`, following the harness in `crates/engine/tests/autofit_filter_buttons.rs` (`Session::new()`, `s.new_workbook()`, `s.execute(...)`, read state via `s.doc().unwrap().wb.active().unwrap()`; copy its `sheet_mut` helper for setting row info): - -```rust -use serde_json::json; - -#[test] -fn whole_row_font_format_grows_row_height_and_undoes() { - let mut s = gridcraft_engine::Session::new(); - s.new_workbook(); - s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); - let before = s.doc().unwrap().wb.active().unwrap().row_height(4); - s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); - let after = s.doc().unwrap().wb.active().unwrap().row_height(4); - assert!(after > before + 10.0, "20pt whole-row format should grow the row: before={before}, after={after}"); - s.execute("edit.undo", json!({})).unwrap(); - assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), before, "undo must restore the height"); -} - -#[test] -fn whole_column_font_format_grows_only_rows_with_cells() { - let mut s = gridcraft_engine::Session::new(); - s.new_workbook(); - s.execute("range.setValues", json!({"range": "A1", "values": [["a"]]})).unwrap(); - s.execute("range.setValues", json!({"range": "A50", "values": [["b"]]})).unwrap(); - s.execute("home.fontSize", json!({"range": "A:A", "size": 20})).unwrap(); - let sh = s.doc().unwrap().wb.active().unwrap(); - assert!(sh.row_height(0) > 30.0 && sh.row_height(49) > 30.0, "rows with cells grow"); - assert!(sh.rows.len() <= 3, "full-column format must fit only rows that have cells, rows map has {} entries", sh.rows.len()); -} - -#[test] -fn whole_row_format_does_not_resize_custom_height_rows() { - let mut s = gridcraft_engine::Session::new(); - s.new_workbook(); - s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); - { let sh = sheet_mut(&mut s); let e = sh.rows.entry(4).or_default(); e.custom = true; e.size = Some(25.0); } - s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); - assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows keep their height"); -} -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `cargo test -p gridcraft-engine --test row_autofit` -Expected: tests 1 and 2 FAIL (height unchanged — the line branch never marks `fit_rows`); test 3 may PASS already (pins existing behavior). - -- [ ] **Step 3: Implement the marking** in `crates/engine/src/cmd/format.rs`, in the `if full_rows || full_cols` branch, after the `for (c, st) in cells` loop and before its `continue` (line 244): - -```rust -// Formatting changed these cells' fonts: re-fit their rows (Excel auto-fits). -cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row))); -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cargo test -p gridcraft-engine --test row_autofit` -Expected: 3 passed. Also run `cargo test -p gridcraft-engine` (no regressions in style/undo behavior). - -- [ ] **Step 5: Commit** - -```bash -git add crates/engine/src/cmd/format.rs crates/engine/tests/row_autofit.rs -git commit -m "#833: whole-row/column formatting re-fits row heights" -``` - -### Task 2: Opened files grow non-custom rows to their tallest font - -**Files:** -- Modify: `crates/engine/src/io.rs:57-74` (`open_bytes_with_password`) and add `fn fit_imported_rows` next to it -- Test: `crates/engine/src/tests.rs` (extend, near the existing `file.open` tests around line 1004) - -**Interfaces:** -- Consumes: `crate::cmd::auto_row_height(wb: &mut Workbook, si: usize, row: u32)` (pub, cmd/mod.rs:458); `Workbook::sheets: Vec>` (pub field); `Sheet::cells.iter() -> impl Iterator` (model store.rs:89); `Sheet::rows: map row -> LineInfo { size: Option, custom: bool, hidden: bool, .. }`; model builders `Workbook::new()`, `wb.styles.derive(StyleId::default(), |s: &mut Style| s.font.size = 20.0) -> StyleId`, `sh.set_value(CellRef, Value)`, `sh.set_style(CellRef, StyleId)`; `gridcraft_xlsx::write_xlsx(&wb) -> Result>`; `crate::io::base64_encode(&[u8])`. -- Produces: `fn fit_imported_rows(wb: &mut Workbook)` — private to `io.rs`, called at both `open_plain` call sites in `open_bytes_with_password`. - -- [ ] **Step 1: Write the failing tests and fixture builder** in `crates/engine/src/tests.rs`. Fixture workbooks are built with the model API directly (never through commands, which would auto-fit them) and exported with `write_xlsx`, which omits `ht` for rows without a size — exactly the bug's input: - -```rust -fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { - // row_info = (size, custom, hidden) for row 5 (index 4), None = no row entry at all. - // Cell A5 = "plain big bold", whole-cell font 20pt. 11pt cell in A1 for contrast rows. - // Returns write_xlsx bytes. Uses Workbook::new, styles.derive, set_value, set_style. -} - -#[test] // A — the issue's repro -fn open_grows_default_height_rows_to_their_font() { - let mut s = Session::new(); - s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); - let h = s.doc().unwrap().wb.active().unwrap().row_height(4); - assert!(h >= 30.0, "20pt cell must grow its default-height row on open, got {h}"); - assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(0), /* default height captured before open */, "11pt row stays default"); -} - -#[test] // B — Review Focus 1 -fn open_keeps_custom_height_rows() { - // fixture with row_info (25.0, true, false): open → row_height(4) == 25.0 -} - -#[test] // C — Review Focus 2 -fn open_never_shrinks_taller_auto_rows() { - // fixture with row_info (60.0, false, false) and an 11pt cell: open → row_height(4) == 60.0 -} - -#[test] // D — Review Focus 5 -fn open_fitting_keeps_hidden_rows_hidden() { - // fixture with row_info (20.0, false, true) and the 20pt cell: open → height grows or keeps 20.0, - // and rows.get(&4).hidden stays true -} - -#[test] // E — Review Focus 4 -fn open_leaves_plain_workbooks_untouched() { - // fixture with only 11pt cells, no row entries: open → sh.rows.get(&0) is None or has size == None -} -``` - -Comments carry each test's assertions where code is elided; fill them in fully when writing. - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `cargo test -p gridcraft-engine open_` -Expected: A FAILS (row stays default — nothing fits on open); B and C may PASS (pin current behavior); D/E PASS or FAIL depending on current import behavior — record actual results in the commit-free state; only A's failure is required before implementing. - -- [ ] **Step 3: Implement `fit_imported_rows` in `crates/engine/src/io.rs` and call it from both `open_plain` sites in `open_bytes_with_password`** (the plain early-return at ~line 58-59 and the decrypted path at ~line 71; keep warnings untouched): - -```rust -/// Excel auto-fits rows to their tallest font when opening a file: rows without a user-set -/// height grow to fit their content. Rows the file made taller keep their height — our -/// line-height estimate can differ from Excel's, and shrinking files that already look -/// right would be a visible regression. Custom-height rows are left alone entirely. -fn fit_imported_rows(wb: &mut Workbook) { - for si in 0..wb.sheets.len() { - let Some(sh) = wb.sheet(si) else { continue }; - let mut rows: Vec = sh.cells.iter().map(|(c, _)| c.row).collect(); - rows.sort_unstable(); - rows.dedup(); - for row in rows { - let before = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); - crate::cmd::auto_row_height(wb, si, row); - let Some(before) = before else { continue }; - let after = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); - if after.is_none_or(|a| before > a + 0.5) { - if let Some(sh) = wb.sheet_mut(si) { - sh.rows.entry(row).or_default().size = Some(before); - } - } - } - } -} -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cargo test -p gridcraft-engine open_` then `cargo test -p gridcraft-engine` (all), plus a sample sanity check: `cargo test -p gridcraft-engine --test command_sequence_fuzz` (undo/round-trip/recalc invariants over opens). - -- [ ] **Step 5: Commit** - -```bash -git add crates/engine/src/io.rs crates/engine/src/tests.rs -git commit -m "#833: opened files grow non-custom rows to their tallest font" -``` - -### Task 3: Docs and tracking - -**Files:** -- Modify: `docs/target-app-parity.md:165` (the "Rows and columns … AutoFit" row) -- Modify: `docs/gaps.md` (revision history; check §24 for a stale row-height mention) -- Modify: `ROADMAP.md` (progress log + revision history + "Last updated" line) - -**Interfaces:** -- Consumes: merged state of Tasks 1-2. -- Produces: nothing (docs). - -- [ ] **Step 1: Update the three docs** - - `docs/target-app-parity.md:165`: append to the evidence column: `; rows auto-fit oversized fonts on open and row formatting (#833)`. - - `docs/gaps.md`: add revision-history row `| 2026-10-12 | minor | rows auto-fit oversized fonts on open and whole-row/column formatting (#833) |`. - - `ROADMAP.md`: progress-log row `| 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) |`; matching revision-history row; bump the `**Last updated:**` date. - -- [ ] **Step 2: Verify docs build/lint state unchanged** - -Run: `cargo xtask ci` -Expected: green (fmt, clippy, tests, assets, layers, wasm). - -- [ ] **Step 3: Commit** - -```bash -git add docs/target-app-parity.md docs/gaps.md ROADMAP.md -git commit -m "#833: docs — track row auto-fit for oversized fonts" -``` - ---- - -**Post-PR (not a plan task):** comment on issue #833 linking the PR and noting the per-run font-size follow-up that lands after rich-text-runs PR #837 merges (per-run sizes are not measured by `auto_row_height` yet).