Skip to content

Fix engineering functions, VBA parsing, and sheet deletion bugs#40

Merged
Nebu1eto merged 20 commits into
mainfrom
claude/review-code-quality-O20qy
Feb 10, 2026
Merged

Fix engineering functions, VBA parsing, and sheet deletion bugs#40
Nebu1eto merged 20 commits into
mainfrom
claude/review-code-quality-O20qy

Conversation

@Nebu1eto

Copy link
Copy Markdown
Owner

Summary

This PR fixes multiple bugs across engineering functions, VBA parsing, threaded comments, and sheet operations. The changes improve correctness of Excel function implementations, robustness of file parsing, and safety of sheet deletion operations.

Key Changes

Engineering Functions (formula/functions/engineering.rs)

  • Fixed octal conversion functions (OCT2BIN, OCT2DEC, OCT2HEX): Corrected two's complement handling from 31-bit to proper 30-bit representation. Changed threshold from 0x3FFFFFFF to 0x1FFFFFFF and subtraction from 0x40000000 * 2 to 0x40000000.
  • Fixed DELTA function: Changed from epsilon-based comparison (f64::EPSILON) to exact bitwise equality (==), matching Excel's behavior.
  • Fixed Bessel functions: Updated BESSELI and BESSELJ to cast integer order n to f64 for proper function signature. Refactored Bessel function implementations to support real (non-integer) orders using power series with gamma function.
  • Replaced factorial with gamma function: Implemented Lanczos approximation for gamma function to properly handle non-integer arguments in Bessel calculations.
  • Fixed octal formatting: Corrected bit mask in format_oct from 0x7F_FFFF_FFFF to 0x3FFFFFFF for proper 30-bit representation.
  • Added comprehensive tests for negative octal values and Bessel function edge cases.

VBA Parsing (vba.rs)

  • Fixed UTF-16LE parsing: Added handling for odd-length record data by masking to even length before chunking, preventing panics on truncated records from corrupted files.

Threaded Comments (threaded_comment.rs)

  • Extended done field parsing: Changed from accepting only "1" to accepting both "1" and "true" strings, improving compatibility with different OOXML producers.
  • Added test for "true" string handling.

Form Controls (control.rs)

  • Added debug warning: When skipping form controls with invalid cell references, now emits a debug-only warning message to aid troubleshooting of corrupted files.

Cell Operations (workbook/cell_ops.rs)

  • Improved error handling: Changed SST index lookup from silently returning empty string to properly reporting out-of-bounds errors.

Drawing Resolution (workbook/drawing.rs)

  • Fixed bounds checking: Added proper bounds checks when accessing self.drawings array to prevent panics on invalid drawing indices.

VBA Project Field (workbook/features.rs, workbook/mod.rs)

  • Removed duplicate field: Eliminated redundant vba_project field that duplicated vba_blob. Now uses single vba_blob field for both round-trip preservation and VBA module extraction.

Sheet Deletion (workbook/sheet_ops.rs)

  • Fixed parallel vector desynchronization: Changed from conditional removal (checking bounds) to unconditional removal of all per-sheet vectors, preventing index misalignment when deleting sheets.
  • Added sync assertion: Implemented assert_parallel_vecs_in_sync() debug assertion to catch vector length mismatches early.
  • Added test for sheet deletion with comments to verify parallel vector consistency.

Stream Writer (workbook/io.rs)

  • Improved safety: Added proper bounds checking and and_then chaining when accessing sheet data during serialization to prevent panics on edge cases.

Documentation (docs/en/api-reference/)

  • Added comprehensive API reference for threaded comments (section 34) with examples in Rust and TypeScript.
  • Added complete error type reference (section 35) documenting all error variants, messages, and descriptions.
  • Added CSV export option documenting escapeFormulas parameter for formula injection prevention.

Notable Implementation Details

  • Bessel function refactoring maintains backward compatibility while fixing integer order handling through numerical differentiation for the limiting form.
  • Two's complement fix ensures Excel compatibility for negative values in base conversion functions.
  • Sheet deletion now uses uncon

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V

claude and others added 20 commits February 10, 2026 10:48
The 30-bit two's complement threshold was 0x3FFFFFFF (the maximum value
for a 10-digit octal number), making the negative-detection branch
unreachable. The subtraction also used 2^31 instead of 2^30.

Fix: threshold to 0x1FFFFFFF (2^29-1) and subtract 0x40000000 (2^30).

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
The mask for negative DEC2OCT output was 0x7F_FFFF_FFFF (39 bits) but
10 octal digits represent 30 bits, so the correct mask is 0x3FFFFFFF.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
bessel_y and bessel_k divided by sin(n*pi) which is 0 for integer n,
producing NaN/Inf. Replace with limiting-form evaluation using small
perturbation and Lanczos gamma function for real-order series.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Excel DELTA performs bitwise-exact comparison (n1 == n2), not an
epsilon-based approximation. The old code used f64::EPSILON which
gave wrong results for very large or very small numbers.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
The VML serialization path used .unwrap() on sheet_comments and
sheet_vml which could panic if parsing had silently failed. Replace
with bounds-checked .get() + pattern matching, gracefully skipping
sheets with missing data.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
delete_sheet used conditional bounds checks (if idx < vec.len()) for
each parallel vector, which could silently skip removal and desync
indices. Replace with unconditional removal and add debug_assert
validation of parallel vector lengths.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
The OOXML spec allows boolean attributes as "1"/"0" or "true"/"false".
The done field conversion only checked for "1", rejecting "true" from
some OOXML producers.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Cell values starting with =, +, -, or @ can trigger formula execution
when CSV files are opened in spreadsheet applications. Add an opt-in
escapeFormulas option that prefixes such values with a tab character.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
build_form_control_vml silently skipped controls with unparseable cell
references. Add a debug-mode warning so developers can detect corrupted
data during testing. The API-level add_form_control already validates
cell references upfront.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
get_cell_value silently returned an empty string when encountering an
invalid SST index, hiding data corruption. Now returns an Internal
error so callers can detect and handle corrupted workbooks.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
get_pictures, get_picture_cells, and resolve_drawing_rel_target used
direct indexing on self.drawings which could panic if a stale index
remained after drawing deletion. Replace with .get() and graceful
fallback.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
parse_pattern_type, parse_border_line_style, parse_horizontal_align,
and parse_vertical_align silently returned defaults for unrecognized
input strings. Add debug-mode warnings so developers can detect typos
in style specifications during development.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Explicitly truncate to even length before chunks_exact(2) to make the
intent clear and prevent silent data loss from a trailing byte in
corrupted VBA project records.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
The Workbook struct had both `vba_blob` and `vba_project` fields holding
identical VBA binary data. Remove `vba_project` and update
`get_vba_project()` / `get_vba_modules()` to read from `vba_blob`.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Document the new escapeFormulas option in the ToCSVOptions table for
both English and Korean API reference pages.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Add section 34 covering threaded comments to the API reference for
both English and Korean documentation. Includes all CRUD methods,
resolve/done state, person list management, and complete type tables
for ThreadedCommentInput/Data, PersonInput/Data.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Add section 35 with a comprehensive error type reference to the API
documentation. Covers all 34 Error enum variants grouped by category:
cells/references, sheets, styles, merge cells, formulas, named ranges,
features, stream writer, file I/O, encryption, and other. Includes
TypeScript error handling example.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Completes the vba_blob/vba_project deduplication by removing the
duplicate field and its doc comment from the struct definition.

https://claude.ai/code/session_01HkVx2dJyZrupXMUy5L2H1V
Allow excessive precision on Lanczos gamma coefficients (mathematical
constants) and apply cargo fmt formatting fixes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename `other` to `_other` in pattern/border/alignment parse functions
where the variable is only used inside #[cfg(debug_assertions)] blocks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Nebu1eto
Nebu1eto marked this pull request as ready for review February 10, 2026 11:49
@Nebu1eto
Nebu1eto merged commit 762be94 into main Feb 10, 2026
4 checks passed
@Nebu1eto
Nebu1eto deleted the claude/review-code-quality-O20qy branch February 10, 2026 11:50
@claude

claude Bot commented Feb 10, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

1 similar comment
@claude

claude Bot commented Feb 10, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

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.

2 participants