[dmythro]

readXls peaked at 511 MB RSS on a 17 MB workbook (one sheet, ~38k rows × 25 columns, 960,600 cells). Two things held most of it, both for the whole read:

• Every record at once. parseRecords built an object and a view for each of the stream's 961k records before the first cell was decoded, then offsetToIndex mapped every offset to its index: ~150 MB.
• Strings as ropes. The SST and inline strings were built with += a character at a time. V8 keeps each concatenation as its own node until something flattens it, and nothing in a read did: 61k shared strings (2.3M characters) held ~70 MB, against ~7 MB flat.

What landed:

• RecordCursor walks the stream a record at a time. Framing is unchanged: a last record running past the end is cut short, and zero padding ends the stream. A BOUNDSHEET offset is checked against record starts in one header-only pass (recordStarts), so an offset that lands inside a record still reads as an empty sheet.
• decodeChars decodes a whole run per record: bytes or UTF-16 units through String.fromCharCode, 8,192 per call. TextDecoder is not used because it would be wrong twice: the Encoding Standard maps latin1 to windows-1252, and utf-16le replaces a lone surrogate with U+FFFD, which is what each half of a pair becomes when a CONTINUE boundary splits it. Malformed input keeps the old per-character read exactly: a UTF-16 character straddling a block end, or a string running past its record.

Same file, Node 24.22 on macOS arm64, one process per run, three runs each:

	time	peak RSS	heap the workbook keeps
main	~235 ms	511 MB	86 MB
branch	~138 ms	195 MB	21 MB

The output is identical: a SHA-256 over every cell value matches.

Tests: three new cases. Shared strings that continue across CONTINUE records: a run longer than one fromCharCode call, a switch to UTF-16 mid-string, and a surrogate pair split by the boundary. LABEL text over 8,192 characters in both widths. A BOUNDSHEET offset that misses a record boundary. No behaviour change is intended, so all three pass on main too. They pin what the faster path has to keep, and they catch the two breakages I tried: TextDecoder for UTF-16 runs fails one, a chunk one character short fails two. bun run test is green.

Not verified: I measured on macOS only, not Linux. The BIFF5 path is covered by the unit tests alone; its strings already went through TextDecoder, so only the record walk changed for it. The XLSB reader builds strings the same += way (xlsb/record.ts) and is not touched here.

Summary by CodeRabbit

• Bug Fixes
  • Improved XLS spreadsheet parsing for shared strings split across records, including mixed compressed and UTF-16 encoding.
  • Improved handling of long text values and surrogate pairs that cross record boundaries.
  • Invalid sheet offsets are now handled safely without disrupting valid sheets.
  • Improved support for formulas followed by string results and empty worksheets.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c446f20a-279f-4ac9-b504-c53b9d838bbd

📥 Commits

Reviewing files that changed from the base of the PR and between c991ad0 and 63736da.

📒 Files selected for processing: 3

• src/xls/biff.ts
• src/xls/reader.ts
• test/xls.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The XLS reader now traverses BIFF records incrementally instead of materializing a record array. It validates sheet offsets against record boundaries and decodes strings in character runs, with fallback handling for characters split across record blocks.

Changes: XLS BIFF parsing

Layer / File(s)	Summary
Record cursor and stream scanning src/xls/biff.ts, src/xls/reader.ts	RecordCursor reads records in place, and recordStarts checks record boundaries. Workbook globals and code-page scans use the cursor.
Workbook and sheet traversal src/xls/reader.ts, test/xls.test.ts	Sheet records and formula string results are read sequentially. Sheet offsets inside a record produce empty sheets.
String decoding across records src/xls/biff.ts, src/xls/reader.ts, test/xls.test.ts	String decoding uses character runs and retains fallback handling across block boundaries. SST continuation records are gathered from the cursor. Tests cover mixed-width continuation, split surrogate pairs, and long inline strings.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant XLSReader
  participant RecordCursor
  participant BlockStream
  XLSReader->>RecordCursor: Read SST and adjacent CONTINUE records
  RecordCursor-->>XLSReader: Provide SST record data
  XLSReader->>BlockStream: Decode character runs across blocks
  BlockStream-->>XLSReader: Return decoded strings

Loading

Suggested reviewers: productdevbook

Merge Risk: ⚪ Minimal · up to 63736

No actionable issue was found that would prevent merging after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely summarizes the main changes: in-place BIFF record traversal and run-based string decoding.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}