Skip to content

feat: Format profiling, VBA, tables, data utils, sheet views#16

Merged
Nebu1eto merged 15 commits into
mainfrom
dev/phase-a-integration
Feb 10, 2026
Merged

feat: Format profiling, VBA, tables, data utils, sheet views#16
Nebu1eto merged 15 commits into
mainfrom
dev/phase-a-integration

Conversation

@Nebu1eto

Copy link
Copy Markdown
Owner

Summary

Phase A delivers 6 new feature groups that close critical gaps vs SheetJS/excelize:

  • Workbook Format Profiling (A1): WorkbookFormat enum (xlsx/xlsm/xltx/xltm/xlam), auto-detection on open, correct content type on save
  • VBA Preservation (A2): Transparent round-trip of vbaProject.bin for macro-enabled files
  • Extension-Based Save (A3): save() infers target format from file extension, rejects unsupported extensions
  • Data Conversion Utilities (A4): toJSON, toCSV, toHTML, fromJSON helpers (Node.js)
  • Full Table CRUD (A5): addTable, getTables, deleteTable with OOXML relationship/content-type wiring
  • Sheet View + Visibility (A6): setSheetViewOptions, getSheetViewOptions, setSheetVisibility, getSheetVisibility

Test Coverage

  • 1521 Rust tests (1325 core + 183 xml + 7 integration + 6 doc-tests) - all passing
  • 321 Node.js tests (6 test files) - all passing
  • New tests per feature: A1 (8), A2 (7), A3 (17), A4 (47), A5 (38), A6 (~49), Integration (17)

Commits

Commit Description
6574a3b WorkbookFormat enum for OOXML format profiling
255e5fa VBA project blob round-trip preservation
fe55d48 Extension-based save format inference
0395635 toJSON, toCSV, toHTML, fromJSON conversion helpers
c2fa48d Full OOXML table CRUD with rel/content-type wiring
bb8ee2b Sheet view options and sheet visibility APIs
f857577 Node.js bindings for format and table CRUD + integration tests
3a73b68 EN/KO documentation for all Phase A features

Documentation

  • 16 doc files updated (8 EN + 8 KO)
  • API reference and user guide sections for all features
  • ~1460 lines of documentation added

Verification

  • cargo build --workspace
  • cargo test --workspace (1521 tests)
  • cargo clippy --workspace (no warnings)
  • cargo fmt --check
  • pnpm build (napi + typecheck + tsdown)
  • pnpm test (321 tests)

Test plan

  • Verify format detection with real .xlsm/.xltx/.xltm/.xlam files
  • Verify VBA macro preservation with actual macro-enabled workbooks
  • Test table creation and round-trip with Excel/LibreOffice
  • Verify data conversion helpers with edge cases
  • Test sheet visibility states in Excel

🤖 Generated with Claude Code

Nebu1eto and others added 11 commits February 10, 2026 11:26
…ltx/xltm/xlam)

Introduce explicit workbook format detection and preservation across the
open/save cycle. The format is inferred from the workbook content type in
[Content_Types].xml during open, stored on the Workbook struct, and used
to emit the correct content type on save. This enables round-trip fidelity
for macro-enabled workbooks, templates, and add-ins.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add high-level data conversion methods to the Workbook class for
common developer workflows. These methods use getRowsBuffer/SheetData
internally for efficient buffer-based data access without unnecessary
re-parsing.

- toJSON(sheet, options?) - convert sheet data to array of objects
- toCSV(sheet, options?) - convert sheet data to CSV string
- toHTML(sheet, options?) - convert sheet data to HTML table string
- fromJSON(sheet, data, options?) - write array of objects to sheet

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ring

Upgrade table support from auto-filter helper level to full table part
management. Tables are now stored as proper xl/tables/table{N}.xml parts
with correct worksheet relationships, content type overrides, and
tableParts references in the worksheet XML.

New capabilities:
- add_table(): creates table XML part with columns, style, auto-filter
- get_tables(): lists table metadata for a sheet
- delete_table(): removes table by name with full cleanup
- Round-trip persistence: tables survive save/open/re-save cycles
- Sheet deletion properly cleans up associated tables

XML schema: new sheetkit-xml::table module with TableXml, TableColumnXml,
TableStyleInfoXml, and TableAutoFilter serde types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…workbooks

Load xl/vbaProject.bin as opaque bytes during open and write it back
unchanged on save. Synchronize the VBA content type override and
workbook relationship entry so the blob is correctly referenced in
the output package. Non-VBA workbooks are unaffected.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add SheetViewOptions (show_gridlines, show_formulas, show_row_col_headers,
zoom_scale, view_mode, top_left_cell) and SheetVisibility (Visible, Hidden,
VeryHidden) with get/set methods on Workbook. Includes XML schema updates,
core business logic, Node.js napi bindings, TypeScript wrapper, and tests
across all layers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When saving to a file, the target format is now inferred from the output
file extension (.xlsx/.xlsm/.xltx/.xltm/.xlam). This ensures the content
type in [Content_Types].xml always matches the file extension.

- Add WorkbookFormat::from_extension() with case-insensitive matching
- Add UnsupportedFileExtension error variant for unrecognized extensions
- Modify save() to validate and apply the extension-derived format
- save_to_buffer() continues to use the stored format unchanged
- Add comprehensive tests for each extension and error cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts:
#	crates/sheetkit-core/src/workbook/io.rs
#	crates/sheetkit-core/src/workbook/mod.rs
# Conflicts:
#	crates/sheetkit-core/src/workbook/features.rs
#	packages/sheetkit/__test__/index.spec.ts
#	packages/sheetkit/index.ts
…tegration tests

Expose getFormat() for workbook format detection (xlsx/xlsm/xltx/xltm/xlam),
and addTable/getTables/deleteTable for table management via the napi layer.
Add cross-feature integration tests covering format + tables + views + styles
+ defined names + validation + sheet protection combinations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover workbook format profiling, VBA preservation, extension-based save,
table CRUD, data conversion utilities, sheet view options, and sheet visibility
across API reference and user guide sections.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Feb 10, 2026

Copy link
Copy Markdown

Code Review

Found 2 issues that need attention:

1. Korean Comments in Documentation Code Blocks (CLAUDE.md Violation)

Korean comments appear in code blocks within Korean documentation. According to CLAUDE.md line 64:

Documentation prose may be in other languages (e.g., Korean guide), but all code blocks within documentation must use English.

Examples:

  • // 눈금선 숨기고 확대/축소를 150%로 설정 in docs/ko/guide/advanced.md:941
  • // 테이블 목록 조회 in docs/ko/guide/data-features.md

Files affected:

  • docs/ko/guide/advanced.md
  • docs/ko/guide/data-features.md

Fix: Replace all Korean comments in code blocks with English equivalents.

Reference:

- Documentation prose may be in other languages (e.g., Korean guide), but all code blocks within documentation must use English.


2. Table Path Collision Bug (Critical)

The table ZIP path computation uses self.tables.len() + 1, which can cause path collisions after table deletion.

Location: crates/sheetkit-core/src/workbook/features.rs:123

Reproduction:

  1. Add table1 (path = xl/tables/table1.xml)
  2. Add table2 (path = xl/tables/table2.xml)
  3. Delete table1 (Vec shrinks to 1 entry)
  4. Add table3 - self.tables.len() is 1, so table_num = 2, giving path xl/tables/table2.xml (collision!)

This causes ZIP entry collision and data corruption when saving.

Suggested fix: Use monotonically increasing path numbers similar to table_id computation.

Reference:

// Assign a unique table ID (max existing + 1).
let table_id = self.tables.iter().map(|(_, t, _)| t.id).max().unwrap_or(0) + 1;
let table_num = self.tables.len() + 1;
let table_path = format!("xl/tables/table{}.xml", table_num);
let table_xml = crate::table::build_table_xml(config, table_id);

@Nebu1eto

Nebu1eto commented Feb 10, 2026

Copy link
Copy Markdown
Owner Author

I reviewed the Phase A workstream branches (dev/phase-a-format-vba, dev/phase-a-data-utils, dev/phase-a-tables, dev/phase-a-sheet-view, and the merged dev/phase-a-integration) and ran focused local repro/tests.

Findings (ordered by severity):

  1. [P1] Table part path reuse can break save with duplicate ZIP entries.
  • Code path: crates/sheetkit-core/src/workbook/features.rs (table_num = self.tables.len() + 1).
  • If you add 2 tables, delete one, then add another table, the new table can reuse an existing xl/tables/tableN.xml path.
  • Repro (JS): add T1, add T2, delete T1, add T3, then save -> ZIP error: invalid Zip archive: Duplicate filename.
  1. [P1] Deleting a table after reopen can leave dangling tableParts in worksheet XML.
  • Save path writes <tableParts> when self.tables had entries, but if tables are removed later the existing worksheet table_parts field is not explicitly cleared when serializing the no-extras path.
  • Repro: save workbook with a table -> reopen -> delete table -> save again. Resulting sheet1.xml still contains <tableParts count="1"><tablePart r:id="rId1"/></tableParts> while sheet1.xml.rels has no table relationship and there is no table part.
  1. [P1] Node CI is currently failing due formatting issues.
  • PR check status shows Node job failing.
  • Failure is from pnpm check / Biome formatting in:
    • packages/sheetkit/index.ts
    • packages/sheetkit/__test__/index.spec.ts
  1. [P2] fromJSON drops keys that only appear in later records.
  • Code path: packages/sheetkit/index.ts (keys = Object.keys(data[0])).
  • Repro: fromJSON('Sheet1', [{ A: 1 }, { B: 2 }]) then toJSON('Sheet1') -> second-row B is lost.
  1. [P2] setSheetVisibility silently accepts invalid strings and can unexpectedly reset visibility to visible.
  • Code path: packages/sheetkit/src/lib.rs maps unknown strings to SheetVisibility::Visible.
  • Repro: set hidden, then call setSheetVisibility('Sheet1', 'hiddden-typo') -> sheet becomes visible.
  1. [P3] ToHtmlOptions.includeStyles is publicly exposed but not implemented.
  • API surface includes includeStyles?: boolean, but toHTML currently ignores it.

Branch-level summary:

  • dev/phase-a-format-vba: no blocking findings in my review.
  • dev/phase-a-data-utils: good feature coverage, but has CI format failure + fromJSON key-union gap + unused includeStyles option.
  • dev/phase-a-tables: contains two blocking issues (1 and 2 above).
  • dev/phase-a-sheet-view: mostly solid, but invalid visibility input handling should be strict.
  • dev/phase-a-integration: integrates all features, but currently carries the above unresolved issues.

If helpful, I can follow up with concrete patch suggestions for each item.

Nebu1eto and others added 4 commits February 10, 2026 12:33
…rts, fromJSON key union, visibility validation, Biome formatting)

- Fix table path collision after deletion by using max existing path number
- Clear stale tableParts from worksheet XML when all tables are deleted
- Collect keys from all records in fromJSON, not just the first object
- Return error for invalid visibility strings instead of silent default
- Remove unimplemented includeStyles option from ToHtmlOptions
- Fix Biome formatting in index.ts and index.spec.ts
- Add regression tests for all fixed issues

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CLAUDE.md requires all code blocks in documentation to use English.
Replace ~128 Korean inline comments across 6 Korean doc files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update CLAUDE.md language policy to permit Korean inline comments
in code blocks within docs/ko/ for reader comprehension.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Nebu1eto Nebu1eto changed the title feat: Phase A - Format profiling, VBA, tables, data utils, sheet views feat: Format profiling, VBA, tables, data utils, sheet views Feb 10, 2026
@Nebu1eto
Nebu1eto merged commit ddf7a5e into main Feb 10, 2026
3 checks passed
@Nebu1eto
Nebu1eto deleted the dev/phase-a-integration branch February 10, 2026 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant