feat(f34): Excel workbook interoperability - #47
Conversation
Add Excel .xlsx interoperability (read + produce; not a formula engine) via calamine 0.36 (read) and rust_xlsxwriter 0.96 (write), in a new src-tauri/src/excel.rs. Read side: - inspect(): the open chooser — sheets with visibility (incl. hidden/very-hidden), named tables and named ranges, per-sheet used ranges + dimensions, formula and merged-cell counts, header-row candidates, bounded previews, and a no-cached-result warning. - preview()/import(): select a sheet, named table or named range (optionally an A1 cell sub-range); header (first row / chosen row / none); merged cells (top-left only / repeat); formulas (cached result / formula text / blank); blank-row and blank-column trimming. Honours the workbook 1900/1904 date system without shifting and reproduces the Excel 1900 leap-year bug via to_ymd_hms_milli; leading-zero text stays text (cell types respected, never numeric-coerced). Produces a fresh CEESVEE document through the DerivedDocumentBuilder pipeline (dirty; inferred schema attached); the original workbook is never modified. Write side: - export(): one sheet from one document, or several sheets (one per open tab) into one workbook. Values only; optional bold+fill header styling, frozen header row, autofilter, and column widths (grid or autofit). Typed numbers/dates/booleans from the F31 schema (text stays text; invalid-under-schema cells fall back to text). Excel's 1,048,576 x 16,384 limits validated before writing; committed through the atomic-save pipeline so a failed/cancelled export never touches an existing destination. Commands (revision-guarded, job-wrapped with progress/cancel): excel_inspect / get_excel_inspect, excel_import_preview / get_excel_import_preview, excel_import_apply, excel_export. Two preview caches registered in lib.rs; export options + sheet-spec DTOs in dto.rs. Tests (22): 1900 leap-year bug + 1904 epoch, merged top-left/repeat, formula cached/text/blank policies incl. no-cached warning flag, leading-zero text, A1 range + named-table selection, chosen header row, blank row/col trimming, inspection visibility/counts, over-limit refusal, multi-sheet + typed export round-trips, duplicate-sheet-name rejection, and cancel cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Frontend for Excel .xlsx interop, wiring the F34 engine's six commands into the app. - ExcelOpenDialog: workbook chooser (sheets with visibility/kind badges, dimensions, formula + merge counts; named tables; named ranges) with a source picker, A1 range input, header-row picker (first/chosen/none with detected candidates), merged/formula policy selects, blank-row/column trim toggles, indexed-backing toggle, a live preview grid, and a no-cached-result warning banner. Imports into a NEW document; states the original is never modified. - ExcelExportDialog: single active-document sheet (scope choices + grid widths) or one sheet per selected open tab; header styling, frozen header, autofilter, typed emission and column-width options; client-side sheet-name and Excel row/column limit pre-checks that refuse an over-limit export up front (the invoke re-checks too). - .xlsx routes through the open chooser in openPath; "Open Excel…" and "Export to Excel…" command-palette entries. - types.ts mirrors the Rust DTOs; tauri.ts adds the six invoke wrappers; store gains the excelImport slice (inspect + preview scans, derive-slot apply) and exportExcel action. - src/lib/excel.ts pure helpers (option assembly, source selection, A1 validation, sheet-name validation/sanitize/dedupe, limit checks, scope sizing, grid widths) with 26 vitest cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Full npm + rust gate suite green on the combined F34 state: 399 frontend tests, 711 rust lib tests, clippy -D warnings, fmt --check. No code fixes were needed. Split the README Excel entry to mirror the JSON docs pattern (import under Viewing "Open Excel", a dedicated "Export to Excel" bullet under Editing); CHANGELOG keeps its single consolidated F34 entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Typed export: guard date/datetime years to Excel's 1900..=9999 window before the `as u16` cast in write_typed, so a schema-typed out-of-range year (reachable via a custom input format parsing a signed year like +67536-01-01) falls back to text instead of being silently truncated (year mod 65536) into a bogus-but-valid in-range date. - Open chooser: hide the detected header-row chips whenever a custom A1 sub-range is entered (new headerCandidateChips helper). The candidate offsets are sheet-used-range-relative; a sub-range shifts the import origin, so applying a chip would mis-select the header row. The manual "Row N" control (region-relative) still applies to any chosen range. - Tests: drive real .xlsx date cells through the full read pipeline — serial 60 reproduces the 1900-02-29 leap-bug via import(), and a patched date1904="1" workbook proves the 1904 epoch is plumbed through inspect()/import() and honoured (serial reads 1462 days later, not shifted back). Add the out-of-range-year text-fallback export test and headerCandidateChips unit tests. Rejected finding #4 (preview() fully materialises the region): the documented invariant is "bounded windows to React", which holds — the wire sample is capped at SAMPLE_ROWS and preview runs as a cancellable, progress-reporting job (the sanctioned pattern for expensive scans). calamine's worksheet_range loads the whole sheet regardless, and import() materialises the identical full grid, so preview is not uniquely violating anything; a streaming rewrite of the shared materialize path (header extraction, blank-row/column trimming, and type/count inference all depend on the full grid) would add real correctness risk to shave a secondary, non-dominant copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71d166916
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let budget = if opts.force_indexed { | ||
| 0 | ||
| } else { | ||
| crate::derived::SPILL_BUDGET |
There was a problem hiding this comment.
When forceIndexed is selected (and also when a default import spills past SPILL_BUDGET), this budget forces DerivedDocumentBuilder down its indexed spill path; that path returns a Document::from_index(..., None, ...) without mark_derived_unsaved, so the new Excel import has no source path but is reported clean. In that context the tab gets no dirty dot/save prompt and closing it drops the temp-backed import, even though the user just created a new document.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b34ef46. Confirmed: the spill branch of DerivedDocumentBuilder::finish returned Document::from_index(..., None, ...) without mark_derived_unsaved, so a forceIndexed (or budget-spilled) import came back with saved_marker == 0 and reported is_dirty() == false — no dirty dot, no close warning, and the temp-backed data silently dropped on close.
The fix marks the spilled document unsaved in finish itself, mirroring the in-memory branch (both derived paths now start dirty). This also matches the documented import contract ("created fresh and marked unsaved"). A mark_saved(Some(path)) still clears the derived guard and resets saved_marker, so Save→Save As works as before. Regression tests added: derived::tests::crossing_the_budget_spills_to_an_indexed_document now asserts is_dirty() + pathless, and excel::tests::forced_indexed_import_is_read_only_but_starts_unsaved covers the F34 path end to end.
| name: meta.name.clone(), | ||
| visibility: visibility_name(meta.visible).to_string(), | ||
| kind: sheet_kind_name(meta.typ).to_string(), | ||
| has_data: range.is_some(), |
There was a problem hiding this comment.
For an empty worksheet, worksheet_range can still return an empty Range whose start()/end() are None (the code above already turns that into usedRows = 0/usedCols = 0), but this line marks it as having data just because the range exists. Workbooks with a blank visible first sheet will therefore auto-select an importable-looking sheet that immediately fails preview/import as empty, and empty sheets remain clickable instead of falling back to a real data sheet or table.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b34ef46. Confirmed: worksheet_range returns Ok(Range) even for a blank sheet, so range.is_some() was true while the extents were empty (the None arm's used_rows == 0 / used_cols == 0 only applies when the range itself is absent, e.g. chart sheets). A workbook with a blank first sheet therefore advertised an importable-looking sheet that fails preview/import as empty.
has_data now gates on the used dimensions (used_rows > 0 && used_cols > 0), which are already computed from the range's height/width, so an empty worksheet reports has_data == false and the chooser can fall through to a real data sheet or table. Regression test added: excel::tests::inspect_does_not_mark_an_empty_worksheet_as_having_data.
| case "selectedColumns": | ||
| return { | ||
| name, | ||
| dataRows: dims.totalRows, |
There was a problem hiding this comment.
For selectedColumns, the backend's export_scope::resolve_scope exports all visible rows when a filter/view is active, not every physical row, but this pre-check uses dims.totalRows. In a filtered million-plus-row document where the visible subset is within Excel's row limit, the dialog will incorrectly block exporting selected columns even though the backend would accept the same scope.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b34ef46. Confirmed against the backend: export_scope::resolve_scope's SelectedColumns arm resolves rows to doc.filter_view() (all VISIBLE rows), but sizingForScope's selectedColumns case used dims.totalRows. In a filtered million-plus-row document whose visible subset is within Excel's limit, checkExportLimits would block a scope the backend would accept.
sizingForScope now sizes selectedColumns on dims.visibleRows (matching the visibleRows scope and the backend). The existing sizing assertion was updated (100 → 30) and a regression test added: a EXCEL_MAX_ROWS + 500 document filtered to 10 visible rows now sizes to dataRows: 10 and passes checkExportLimits.
Rebase feat/f34-excel onto feat/f32-parquet (c9f53f8). f34 forked from feat/f42-highlight before the f33/f37/f38/f48/f40/f41/f42/.../f32-parquet chain grew beneath it, so the replay pulled every parent's changes in. Conflicts were purely-additive unions, resolved so F32 (Parquet/Arrow) and F34 (Excel) coexist: - src-tauri/Cargo.toml + Cargo.lock: union the arrow/parquet and calamine/rust_xlsxwriter dependency blocks; Cargo.lock regenerated offline against the unioned manifest. - src-tauri/src/commands.rs, lib.rs, dto.rs: both command sets plus their cache registrations side by side (the collapsed shared job-tail boilerplate was reconstructed so each command closes itself). - src/types.ts, lib/tauri.ts, lib/commandDefs.ts, store/useStore.ts, App.tsx: both DTO mirrors, invoke wrappers, palette entries, store slices, and dialog mounts. - CHANGELOG.md / README.md: all bullets kept in chain order (parquet then excel). The f34 gates-stage README refinement (import-only "Open Excel" bullet under Viewing plus a dedicated "Export to Excel" bullet under Editing) was preserved alongside the F32 columnar bullets. One silent-revert repair was needed: the per-commit 3-way merges dropped F32's project-restore-for-columnar wiring (`restoreOpenRoute` in src/lib/project.ts + its test, and `openColumnarRestore` plus the applyProjectPlan branch in src/store/useStore.ts) because f34 forked before that code existed and none of f34's commits touched those hunks. Restored verbatim from feat/f32-parquet — project.ts/project.test.ts are byte-identical to the base again, and useStore.ts carries the import, interface method, implementation, and restore-loop branch back. Without this a restored project would silently reopen Parquet/Arrow sources through the interactive inspect dialog (no tab created) instead of the non-interactive indexed open. Full gates green against the chain: npm lint/format/typecheck/test (43 files, 424 tests incl. 30 excel cases + the 2 restored restoreOpenRoute cases) and build; cargo fmt --check, clippy --all-targets --all-features -D warnings, and cargo test --lib (753 passed, 0 failed — up from 711 on the old f42 base as the rebase pulled in the chain's newer unit tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
e71d166 to
7d10054
Compare
…visible-row column export sizing - derived: mark spilled (indexed) derived documents unsaved like the in-memory branch. A forceIndexed / large Excel import is temp-backed with no source path, so it must warn on close and route Save to Save As instead of silently dropping the just-built document. - excel inspect: an empty worksheet still yields Some(range) with empty extents; gate has_data on the used dimensions so a blank first sheet is not offered as importable and the chooser falls through to a real data sheet. - excel export sizing: selected-column exports write the VISIBLE rows (mirroring export_scope::resolve_scope), so the up-front limit check must size on dims.visibleRows, not the unfiltered total — otherwise a filtered large document whose visible subset fits Excel's limit is wrongly blocked. Tests: derived spill dirty/pathless assertions; excel empty-sheet has_data and forced-indexed-unsaved tests; frontend visible-row selected-columns sizing + limit-check regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # src/lib/project.test.ts # src/store/useStore.ts
1 participant
F34 — Excel workbook interoperability
Read and produce Excel
.xlsxworkbooks without a detour through CSV and without a formula engine. Import a chosen sheet, named table, or named range (optionally anA1sub-range) with control over the header row, merged cells, formulas, and blank-row/column trimming; export one document — or several open tabs as one sheet each — into a single workbook with header styling, a frozen header row, an autofilter, column widths, and typed numbers/dates/booleans.What's here
Engine (
src-tauri)excel.rs: workbook inspection (sheets with visibility, named tables/ranges, used ranges + dimensions, formula/merged-cell counts, header candidates, bounded previews), import preview, import-apply, and typed multi-sheet export. calamine reads (with thedatesfeature so date cells honour the workbook epoch); rust_xlsxwriter produces.excel_inspect/get_excel_inspect,excel_import_preview/get_excel_import_preview,excel_import_apply,excel_export— all cancellable jobs through the shared registry, DTOs indto.rs, registered inlib.rs..xlsxsave.UI (
src)ExcelOpenDialog.tsx(chooser + import options) andExcelExportDialog.tsx, wired throughtypes.ts,lib/tauri.ts,lib/excel.ts,lib/commandDefs.ts(palette: "Open Excel…", "Export to Excel…"), theuseStoreslices, and mounts inApp.tsx.Tests
src/lib/excel.test.ts).cargo test --lib753 passed, 0 failed. Full suite runs on CI (windows MSVC + ubuntu).--all-targets --all-features -D warnings.Acceptance mapping
.xlsxwithout CSV detour →excel::inspect/preview/import+ExcelOpenDialog.A1sub-range →ExcelImportOptions(sheet/table/namedRange/range).ExcelHeaderMode/ExcelMergedPolicy/ExcelFormulaPolicy+ trim flags; formula-without-cached-result warning surfaced in the preview..xlsxcells driven end-to-end, incl. the 1904-epoch plumb-through).excel::plan_export/export+ExcelExportDialog.excel_import_applyregisters a fresh doc id.STACKED PR
Base is
feat/f32-parquet, notmain. Review only the delta vsfeat/f32-parquet— the diff againstmainalso contains every parent feature in the chain (f31 → infra → f33 → f37 → f38 → f48 → f40 → f41 → f42 → … → f32). This branch was rebased ontofeat/f32-parquet; all rebase conflicts were resolved as purely-additive unions so F32 (Parquet/Arrow) and F34 (Excel) coexist. Cargo.lock was regenerated against the unionedCargo.toml.The
chore(f34): integrate onto chaincommit also repairs one silent revert the per-commit 3-way merges introduced: F32's project-restore-for-columnar wiring (restoreOpenRoute+openColumnarRestoreand theapplyProjectPlanbranch) was dropped because f34 forked before that code existed and none of its commits touched those hunks. It's restored verbatim fromfeat/f32-parquet, sosrc/lib/project.ts/project.test.tsare byte-identical to the base and the delta contains no deletion-only files.🤖 Generated with Claude Code