feat(sheet): round-trip styles, dimensions and freeze panes through xlsx

[SamTV12345]

What

lib/xlsx was still v1 from the original spreadsheet PR (#306): only values and formulas survived import/export. The sheet model has since gained style props (M2 #319), col/row dimensions + freeze panes (M4 #326) and font props (#328). This PR carries all of it through the xlsx round-trip:

Export — cell styles (bold/italic/underline, font color/family/size, bg fill, alignment, wrap, border, numFmt), column widths, row heights, frozen first row/col.

Import — the reverse mapping into the allowlisted prop vocabulary, interned into the workbook StylePool; sparse dimension overrides (probed against the file default so only real overrides are stored); freeze panes clamped to the model's 0/1.

Notes

• The symbolic numFmt vocabulary (number:2, currency:2, percent:0, date, text) maps to xlsx format codes both ways; foreign files with builtin numFmt ids are classified best-effort.
• sheet.validateProps is now exported (ValidateProps) and re-applied to imported props — uploaded files bypass Op.Validate but their props still end up as inline CSS on every viewer's DOM.
• Styled-but-empty cells don't appear in excelize's GetRows and the dimension attribute is not maintained, so import sweeps the fixed 200x52 grid for styles (cheap, import-only path).
• Merges remain out of scope (the sheet model does not store them).

Tests

• Full round-trip test: all 11 props + col width + row height + freeze + styled-empty cell survive Export→Import.
• Foreign-file test: file built directly with excelize (builtin 0.00% numFmt, ARGB-less color) imports correctly, so mapping gaps can't hide behind a symmetric round-trip.
• numFmt code mapping table test in both directions.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Round-trip sheet styles, dimensions, and freeze panes in XLSX import/export

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Export XLSX now preserves cell styles, column/row dimensions, and freeze panes.
• Import maps XLSX styles into allowlisted sheet props and interns them into StylePool.
• Add round-trip and foreign-file tests for styling and numFmt mappings.

Diagram

graph TD
  WB["Workbook model"] --> SP["StylePool"] --> EXP["XLSX Export"] --> XL["excelize"] --> FILE[(".xlsx")]
  FILE --> XL --> IMP["XLSX Import"] --> SNAP["Workbook snapshot"]
  IMP --> VP["ValidateProps"] --> SP

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Parse sheet XML for styled-empty cells/dimensions

• ➕ Avoids fixed 200x52 GetCellStyle sweep by reading only cells with style records
• ➕ More scalable if the client grid size grows substantially
• ➖ More complex and brittle (depends on XLSX internal XML structures)
• ➖ Higher maintenance cost and more edge cases across Excel producers

2. Store opaque XLSX style ids for round-trip

• ➕ Potentially more faithful styling round-trip for non-allowlisted properties
• ➕ Less mapping logic for exotic formats
• ➖ Not safe: imported styles ultimately become inline CSS; still needs strict allowlisting
• ➖ Couples model to excelize/XLSX internals and makes non-XLSX backends harder

Recommendation: Current approach is the right tradeoff: keep the model’s strict, allowlisted prop vocabulary as the trust boundary (via exported ValidateProps) while implementing a best-effort XLSX mapping layer. The fixed-grid style sweep is acceptable given the client’s fixed 200x52 surface and keeps import logic simple and robust compared to XML-level parsing.

Files changed (5) +500 / -14

Enhancement (4) +385 / -14

op.goExport ValidateProps and reuse in op validation +4/-4

Export ValidateProps and reuse in op validation

• Renames the internal style prop allowlist validator to the exported ValidateProps and updates Op.Validate to call it. This enables XLSX import to apply the same trust-boundary validation used for collaborator-driven ops.

lib/sheet/op.go

export.goExport styles, dimensions, and freeze panes to XLSX +64/-6

Export styles, dimensions, and freeze panes to XLSX

• Adds style-id translation from the workbook StylePool to excelize style ids and applies styles per cell. Exports column widths, row heights, and freeze panes; also avoids writing empty raw values as numbers/strings while still allowing styled-empty cells to carry style metadata.

lib/xlsx/export.go

import.goImport styles, sparse dimensions, and freeze panes from XLSX +79/-4

Import styles, sparse dimensions, and freeze panes from XLSX

• Imports excelize cell styles into StylePool entries via styleToProps and assigns StyleId on cells, including styled-but-empty cells via a fixed-grid sweep. Imports column/row dimensions as sparse overrides by probing defaults, and clamps freeze panes to the model’s supported 0/1 semantics.

lib/xlsx/import.go

style.goImplement props↔excelize style mapping and unit/numFmt conversions +238/-0

Implement props↔excelize style mapping and unit/numFmt conversions

• Introduces conversions for column/row dimensions (px↔xlsx units), symbolic numFmt↔format codes (including builtin ids), and color normalization/expansion. Implements propsToStyle and styleToProps with a final ValidateProps gate to drop anything outside the allowed style vocabulary.

lib/xlsx/style.go

Tests (1) +115 / -0

import_test.goAdd round-trip, foreign-file, and numFmt mapping tests +115/-0

Add round-trip, foreign-file, and numFmt mapping tests

• Adds a comprehensive round-trip test covering all allowlisted style props plus dimensions, freeze panes, and styled-empty cells. Adds a foreign-file import test created directly with excelize to validate builtin numFmt/id handling and color normalization, plus a numFmt code mapping table test.

lib/xlsx/import_test.go

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Negative col width values 🐞 Bug ≡ Correctness

Description

Export converts pixel column widths via pxToColWidth(px) = (px-5)/7, so valid sheet dimension
overrides in the 1–4px range become negative and are passed to excelize.SetColWidth, yielding
library-dependent behavior and potentially incorrect export sizing.

Code

lib/xlsx/style.go[R18-21]

+func colWidthToPx(w float64) int   { return int(math.Round(w*7 + 5)) }
+func pxToColWidth(px int) float64  { return float64(px-5) / 7 }
+func rowHeightToPx(h float64) int  { return int(math.Round(h * 96 / 72)) }
+func pxToRowHeight(px int) float64 { return float64(px) * 72 / 96 }

Evidence

The model explicitly allows dimension sizes down to 1px, while the newly introduced XLSX conversion
formula produces negative widths for px<5 and is used unconditionally during export.

lib/sheet/op.go[133-142]
lib/xlsx/style.go[18-21]
lib/xlsx/export.go[88-95]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`pxToColWidth(px)` returns a negative value when `px < 5`, but the sheet model allows any positive dimension size (including 1–4). Export then passes the negative value into `SetColWidth`, which is an invalid/nonsensical XLSX column width.

### Issue Context
- Sheet dimension validation allows `Size > 0` (no minimum aligned to the XLSX conversion).
- XLSX conversion uses `(px-5)/7` which crosses below zero for small px values.

### Fix Focus Areas
- lib/xlsx/style.go[18-21]
- lib/xlsx/export.go[88-95]
- lib/sheet/op.go[133-142]

### Suggested fix
- Clamp in `pxToColWidth`: e.g. `if px < 5 { px = 5 }` (or clamp the computed width to `>= 0`).
- Alternatively (or additionally), tighten `OpSetDimension` validation to enforce a minimum column width that is representable in XLSX.
- Add a small unit test for `pxToColWidth` (or an export smoke test) covering px=1..4 to ensure it never produces a negative width.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. numFmt misclassified as date 🐞 Bug ≡ Correctness

Description

codeToNumFmt checks for any 'y'/'m'/'d' character before currency/percent/number detection, so
format codes containing those letters in quoted literals (e.g. "USD") are incorrectly mapped to the
model's 'date' numFmt and then accepted into props.

Code

lib/xlsx/style.go[R79-89]

+	low := strings.ToLower(code)
+	switch {
+	case strings.ContainsAny(low, "ymd") && !strings.Contains(low, "red"): // date letters; "[Red]" is a color, not a date
+		return "date"
+	case strings.Contains(code, "%"):
+		return fmt.Sprintf("percent:%d", dec)
+	case strings.ContainsAny(code, "$€£") || strings.Contains(code, "[$"):
+		return fmt.Sprintf("currency:%d", dec)
+	case strings.ContainsAny(code, "0#"):
+		return fmt.Sprintf("number:%d", dec)
+	}

Evidence

The classifier’s first branch is a broad ContainsAny("ymd") check, and the resulting symbolic
value is propagated into props and accepted by the allowlist validator, so misclassification
directly affects imported workbook styles.

lib/xlsx/style.go[63-91]
lib/xlsx/style.go[221-229]
lib/sheet/op.go[153-180]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`codeToNumFmt` classifies any format code containing the letters `y`, `m`, or `d` as a date *before* checking for percent/currency/number. This misclassifies non-date formats that include those letters inside quoted literals (e.g. `"USD" #,##0.00`) or other non-date tokens.

### Issue Context
- Import uses `codeToNumFmt` to set `props["numFmt"]`.
- `sheet.ValidateProps` allowlists `date`, so the misclassification survives the safety gate and changes how the client formats values.

### Fix Focus Areas
- lib/xlsx/style.go[63-91]
- lib/xlsx/style.go[221-229]
- lib/sheet/op.go[153-180]

### Suggested fix
- Reorder classification to check percent/currency/number before date detection.
- Make date detection stricter by ignoring quoted literals (and optionally bracketed sections like `[Red]`, `[$USD]`) before searching for date tokens.
- Add a regression test for a format like `"USD" #,##0.00` (or similar) ensuring it maps to `currency:<decimals>` (or `number:<decimals>`) rather than `date`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

1. Negative col width values 🐞 Bug ≡ Correctness

Export converts pixel column widths via pxToColWidth(px) = (px-5)/7, so valid sheet dimension
overrides in the 1–4px range become negative and are passed to excelize.SetColWidth, yielding
library-dependent behavior and potentially incorrect export sizing.

Agent Prompt

### Issue description
`pxToColWidth(px)` returns a negative value when `px < 5`, but the sheet model allows any positive dimension size (including 1–4). Export then passes the negative value into `SetColWidth`, which is an invalid/nonsensical XLSX column width.

### Issue Context
- Sheet dimension validation allows `Size > 0` (no minimum aligned to the XLSX conversion).
- XLSX conversion uses `(px-5)/7` which crosses below zero for small px values.

### Fix Focus Areas
- lib/xlsx/style.go[18-21]
- lib/xlsx/export.go[88-95]
- lib/sheet/op.go[133-142]

### Suggested fix
- Clamp in `pxToColWidth`: e.g. `if px < 5 { px = 5 }` (or clamp the computed width to `>= 0`).
- Alternatively (or additionally), tighten `OpSetDimension` validation to enforce a minimum column width that is representable in XLSX.
- Add a small unit test for `pxToColWidth` (or an export smoke test) covering px=1..4 to ensure it never produces a negative width.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

2. Numfmt misclassified as date 🐞 Bug ≡ Correctness

codeToNumFmt checks for any 'y'/'m'/'d' character before currency/percent/number detection, so
format codes containing those letters in quoted literals (e.g. "USD") are incorrectly mapped to the
model's 'date' numFmt and then accepted into props.

Agent Prompt

### Issue description
`codeToNumFmt` classifies any format code containing the letters `y`, `m`, or `d` as a date *before* checking for percent/currency/number. This misclassifies non-date formats that include those letters inside quoted literals (e.g. `"USD" #,##0.00`) or other non-date tokens.

### Issue Context
- Import uses `codeToNumFmt` to set `props["numFmt"]`.
- `sheet.ValidateProps` allowlists `date`, so the misclassification survives the safety gate and changes how the client formats values.

### Fix Focus Areas
- lib/xlsx/style.go[63-91]
- lib/xlsx/style.go[221-229]
- lib/sheet/op.go[153-180]

### Suggested fix
- Reorder classification to check percent/currency/number before date detection.
- Make date detection stricter by ignoring quoted literals (and optionally bracketed sections like `[Red]`, `[$USD]`) before searching for date tokens.
- Add a regression test for a format like `"USD" #,##0.00` (or similar) ensuring it maps to `currency:<decimals>` (or `number:<decimals>`) rather than `date`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools