Streaming reader: preserve a formula cell's cached ERROR token (t="e")

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,13 +1,28 @@
-<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->
+<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. -->
+
+<!-- AI agents (Claude Code, Cursor, Codex, Copilot Workspace, etc.) and humans submitting AI-generated code: read AGENTS.md before opening this PR. PRs that ignore those rules get closed. -->
 
 ## Summary
 
 <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->
 
+## Files changed
+
+<!-- List EVERY file changed, including configs, lockfiles, and incidental edits. If this PR touches .prettierrc, .eslintrc, package.json, package-lock.json, README.md, or index.d.ts, call it out explicitly. -->
+
 ## Test plan
 
-<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->
+<!-- Demonstrate the code is solid. Include the exact commands you ran and their output. For bugfixes, include a test (in spec/) that fails before the fix and passes after. -->
 
 ## Related to source code (for typings update)
 
-<!-- List with permalink into source code to prove that changes are true -->
+<!-- List with permalink into source code to prove that changes are true. -->
+
+## Checklist
+
+- [ ] Only the lines required for the fix/feature are changed (no formatter sweeps, no drive-by refactors)
+- [ ] Tests use real fixture round-trip (`wb.xlsx.load` / `writeBuffer`), not synthetic model objects, where possible
+- [ ] Checked open PRs (`gh pr list --state open`) for conflicting changes to the same files
+- [ ] (If serialization is touched) Output verified to open in Excel, or `soffice --headless` round-trip clean
+- [ ] (If `--no-verify` was used to bypass the pre-commit hook) Reason explained above
+- [ ] (If a major dep was bumped) Runtime smoke test described above

AGENTS.md

@@ -0,0 +1,96 @@
+# AGENTS.md
+
+Instructions for AI coding agents (Claude Code, Cursor, Codex, Copilot Workspace, etc.) opening pull requests against this repository.
+
+Human contributors: see [CONTRIBUTING.md](CONTRIBUTING.md). The rules below also apply to any human submitting AI-generated code.
+
+If you are an AI agent, read this file in full before making changes. The patterns below come from real PRs we have had to reject or rework.
+
+---
+
+## Hard rules
+
+### 1. Surgical scope
+
+- Touch ONLY the lines required for the fix or feature.
+- Do NOT extract constants, rename variables, collapse `Promise.all`, rewrite `reduce` to `for`, or "clean up" surrounding code, even if your linter or style tool suggests it.
+- If the pre-commit hook (prettier ↔ eslint conflict) fights you, commit with `--no-verify` and call this out in the PR description. Do NOT reshape unrelated code to satisfy the hook.
+
+### 2. No formatter sweeps
+
+- Do NOT run `npx prettier --write .`, `eslint --fix` on whole files, or any tool that rewrites files you did not otherwise change.
+- Do NOT modify `.prettierrc`, `.eslintrc`, or other config files in a feature/bugfix PR. If a config change is needed, open a separate config-only PR.
+
+### 3. PR description must be complete
+
+- List EVERY file changed in the PR description, including configs, lockfiles, and incidental edits.
+- If you used `--no-verify`, say so and explain why.
+- If your change depends on or conflicts with another open PR, link it.
+
+### 4. Tests must use real fixtures, not synthetic models
+
+- For XLSX read/write fixes, your test MUST round-trip through `wb.xlsx.load(...)` and/or `wb.xlsx.writeBuffer(...)` against a real or minimal fixture file.
+- Do NOT build hand-rolled model objects and call internal serializers (e.g. `XLSX.reconcile`, individual xforms) directly. Synthetic-model tests can pass while the real load/write path still breaks.
+- Place fixture files under `spec/integration/data/` following existing naming.
+
+### 5. Cross-check open PRs before submitting
+
+- Before opening a PR, list the currently open PRs in this repo and verify your change does not touch the same files or overlap in scope. The maintainer has had three concurrent PRs all editing `.prettierrc` because none of them checked.
+- Run: `gh pr list --state open --limit 50 --json number,title,files`
+
+### 6. XLSX serialization changes must be Excel-verified
+
+If your change touches anything that writes XLSX (xforms, sheet/workbook serialization, pivot tables, charts, comments, conditional formatting):
+
+- Verify the output file actually opens in Excel without a "Repaired Records" warning, OR
+- Round-trip with LibreOffice headless and inspect the bytes:
+  ```bash
+  soffice --headless --convert-to xlsx /tmp/your-output.xlsx --outdir /tmp/roundtrip
+  unzip -p /tmp/roundtrip/your-output.xlsx xl/<relevant-part>.xml | head
+  ```
+- Unit tests do NOT catch Excel's repair warnings. This step is mandatory for serialization-touching PRs.
+
+### 7. Dependency bumps require runtime smoke tests
+
+- Major-version dep bumps (e.g. `fast-csv`, `unzipper`, `archiver`) MUST be smoke-tested against the runtime paths that use them, not just `npm test`.
+- Streaming reader changes: load a real `.xlsx`. CSV writer changes: write and re-parse real CSV. Document the smoke test in the PR.
+
+### 8. One concern per PR
+
+- Bug fix + sibling bugs of the same shape in the same file: ONE PR. Good.
+- Bug fix + unrelated cleanup + dep bump: THREE PRs. Required.
+- If you find yourself touching `index.d.ts`, `README.md`, `package-lock.json`, etc. incidentally, stop and revert those edits unless they are the actual subject of the PR.
+
+---
+
+## Per-PR checklist (also in `.github/pull_request_template.md`)
+
+Before opening the PR, verify:
+
+- [ ] Only the lines required for the fix are changed
+- [ ] No prettier/eslint sweeps on unrelated files
+- [ ] Every file change is listed in the PR description below
+- [ ] Tests use real fixture round-trip (`wb.xlsx.load` / `writeBuffer`), not synthetic models
+- [ ] `gh pr list --state open` checked for conflicting PRs
+- [ ] (If serialization) Output file opens in Excel or `soffice --headless` without warnings
+- [ ] (If `--no-verify` used) Reason noted in PR description
+- [ ] (If dep bump) Runtime smoke test described in PR
+
+---
+
+## Repository conventions
+
+- Code style: enforced by ESLint + Prettier (the conflict above is real — live with it on a per-PR basis)
+- Tests: Mocha, in `spec/unit/`, `spec/integration/`, `spec/end-to-end/`
+- Run unit tests fast: `npm run test:unit` (skips the build step)
+- Full suite: `npm test` (build + unit + integration + e2e + jasmine)
+- Node target: see `engines` in `package.json`
+- Browserify is in the build chain. Do NOT introduce dependencies that ship ES2021+ syntax (`??=`, `?.()`) in their CJS output, or that publish `exports`-only packages without a `main` field. Both break the browser bundle.
+
+---
+
+## What this fork is
+
+This is a curated Protobi fork of [exceljs/exceljs](https://github.com/exceljs/exceljs). We selectively adopt upstream features for production use. We are NOT a general-purpose alternative fork. See [CONTRIBUTING.md](CONTRIBUTING.md) for what we accept.
+
+If your change is a generic improvement that has not been merged upstream, consider opening it against [exceljs/exceljs](https://github.com/exceljs/exceljs) first.

CONTRIBUTING.md

@@ -2,6 +2,8 @@
 
 Thank you for your interest in contributing!
 
+> **AI agents and humans submitting AI-generated code:** read [AGENTS.md](AGENTS.md) first. It has hard rules about PR scope, formatter sweeps, real-fixture testing, and Excel-verification for serialization changes. PRs that ignore those rules get closed regardless of the underlying fix quality.
+
 ## About This Fork
 
 This is a **curated fork** of ExcelJS maintained by Protobi. We selectively adopt features from the upstream repository that we need for our production systems.

FORK.md

@@ -28,6 +28,35 @@ This AI-assisted workflow enables rapid response to community issues while maint
 
 ## Fork Release History
 
+### 4.4.0-protobi.10 (2026-05-06)
+
+**Bug Fix: richText shared-string deduplication in streaming writer** ([#66](https://github.com/protobi/exceljs/pull/66), cherry-picked from [#50](https://github.com/protobi/exceljs/pull/50) by @gwkline)
+
+When using `WorkbookWriter` with `useSharedStrings` enabled, every cell containing richText collapsed into a single shared-string entry, corrupting the strings table. Root cause: `SharedStrings.add()` used the value directly as a hash key; richText objects coerced to the string `"[object Object]"`, deduping all of them into one entry.
+
+**Fix:** Hash richText values by their rendered XML representation (via `SharedStringXform.toXml()`). Plain strings unchanged.
+
+**Tests:** Adds two regression tests (`spec/unit/utils/shared-strings.spec.js`):
+1. Equal richText values share one entry; different ones get separate entries
+2. richText values that differ only in formatting are NOT deduped
+
+**Upstream context:** This bug was first reported as [exceljs/exceljs#2267](https://github.com/exceljs/exceljs/issues/2267) (May 2023). An alternative fix was opened upstream as [exceljs/exceljs#2588](https://github.com/exceljs/exceljs/pull/2588) (Nov 2023) but has been stale since Feb 2024.
+
+**Build Fix: Pin uuid to ^9.0.1** ([#64](https://github.com/protobi/exceljs/pull/64))
+
+`npm install` had drifted to `uuid@14.0.0`, which broke `npm run build`:
+- uuid@14 dropped the `main` field in favor of `exports`-only — browserify@16 doesn't honor `exports`
+- uuid@11 (npm's recommended CJS target) ships ES2021 syntax (`??=`) browserify@16 can't parse
+- uuid@9.0.1 is the highest version that keeps both `main` and pre-ES2021 CJS output
+
+**Documentation: AGENTS.md for AI-generated PRs** ([#65](https://github.com/protobi/exceljs/pull/65))
+
+Adds `AGENTS.md` at repo root with hard rules for AI coding agents (Claude Code, Cursor, Codex, etc.): surgical scope, no formatter sweeps, complete PR descriptions, real-fixture testing, cross-check open PRs, Excel-verification for serialization changes. Auto-discovered by AI tools; referenced from README.md, CONTRIBUTING.md, and the PR template.
+
+**Tests:** All 886 unit tests passing (4 in `SharedStrings`, up from 2).
+
+---
+
 ### 4.4.0-protobi.9 (2026-02-01)
 
 **New Feature: Pivot Table & Chart Round-Trip Preservation** ([#41](https://github.com/cjnoname/excelts/issues/41))

README.md

@@ -59,6 +59,13 @@ See [FORK.md Release History](FORK.md#fork-release-history) for details.
 
 ## Fork Release Notes
 
+**4.4.0-protobi.10** (May 2026) - Streaming richText Bug Fix + Build Fix
+- **Fix richText shared-string deduplication in streaming writer** - Cherry-picked from PR #50 by @gwkline. Previously, every richText cell collapsed into a single shared-string entry because objects coerced to `"[object Object]"` as a hash key. Now richText values are hashed by their rendered XML representation. Addresses upstream [exceljs/exceljs#2267](https://github.com/exceljs/exceljs/issues/2267).
+- Adds regression tests covering richText deduplication and formatting-aware distinction
+- Pin uuid to ^9.0.1 to keep browserify build working (uuid@14 dropped the `main` field; uuid@11 ships ES2021 syntax browserify can't parse)
+- Adds `AGENTS.md` with hard rules for AI-generated PRs
+- All 886 unit tests passing
+
 **4.4.0-protobi.9** (February 2026) - Pivot Table & Chart Round-Trip Preservation + Critical Bug Fixes
 - **Round-trip preservation for pivot tables and charts** - Read Excel files with existing pivot tables and charts, write them back without corruption
 - Hybrid preservation approach: stores raw XML while extracting minimal metadata for structural integrity
@@ -145,11 +152,13 @@ npm install exceljs
 
 Contributions are very welcome! It helps me know what features are desired or what bugs are causing the most pain.
 
-I have just one request; If you submit a pull request for a bugfix, please add a unit-test or integration-test (in the spec folder) that catches the problem.
- Even a PR that just has a failing test is fine - I can analyse what the test is doing and fix the code from that.
+**Before opening a PR, read [CONTRIBUTING.md](CONTRIBUTING.md).**
+
+**If you are an AI agent (Claude Code, Cursor, Codex, Copilot Workspace, etc.) — or a human submitting AI-generated code — read [AGENTS.md](AGENTS.md) in full first.** It contains hard rules about scope, formatter sweeps, real-fixture testing, and Excel-verification for serialization changes. AI-generated PRs that ignore these get closed.
+
+For humans: if you submit a pull request for a bugfix, please add a unit-test or integration-test (in the `spec/` folder) that catches the problem. Even a PR that just has a failing test is fine — I can analyse what the test is doing and fix the code from that.
 
-Note: Please try to avoid modifying the package version in a PR.
-Versions are updated on release and any change will most likely result in merge collisions.
+Note: Please try to avoid modifying the package version in a PR. Versions are updated on release and any change will most likely result in merge collisions.
 
 To be clear, all contributions added to this library will be included in the library's MIT licence.
 

lib/stream/xlsx/worksheet-reader.js

@@ -294,6 +294,12 @@ class WorksheetReader extends EventEmitter {
                     if (c.v) {
                       if (c.t === 'str') {
                         cellValue.result = utils.xmlDecode(c.v.text);
+                      } else if (c.t === 'e') {
+                        // A cached ERROR result on a formula cell (t="e", e.g. =1/0 → #DIV/0!).
+                        // Mirror the non-streaming Workbook.load() shape (result:{error}) and the
+                        // bare-error case below — the parseFloat fallback would turn '#DIV/0!' into
+                        // NaN and drop the token entirely.
+                        cellValue.result = {error: c.v.text};
                       } else {
                         cellValue.result = parseFloat(c.v.text);
                       }

lib/utils/shared-strings.js

@@ -1,3 +1,5 @@
+const SharedStringXform = require('../xlsx/xform/strings/shared-string-xform');
+
 class SharedStrings {
   constructor() {
     this._values = [];
@@ -21,10 +23,15 @@ class SharedStrings {
     return this._values[index];
   }
 
+  get sharedStringXform() {
+    return this._sharedStringXform || (this._sharedStringXform = new SharedStringXform());
+  }
+
   add(value) {
-    let index = this._hash[value];
+    const hashKey = value && value.richText ? this.sharedStringXform.toXml(value) : value;
+    let index = this._hash[hashKey];
     if (index === undefined) {
-      index = this._hash[value] = this._values.length;
+      index = this._hash[hashKey] = this._values.length;
       this._values.push(value);
     }
     this._totalRefs++;

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@protobi/exceljs",
-  "version": "4.4.0-protobi.9",
+  "version": "4.4.0-protobi.10",
   "description": "Excel Workbook Manager - Temporary fork with pivot table enhancements and bug fixes pending upstream merge",
   "private": false,
   "license": "MIT",
@@ -111,7 +111,7 @@
     "saxes": "^5.0.1",
     "tmp": "^0.2.0",
     "unzipper": "^0.10.11",
-    "uuid": "^8.3.0"
+    "uuid": "^9.0.1"
   },
   "devDependencies": {
     "@babel/cli": "^7.10.5",

spec/integration/issues/issue-xls214-streaming-formula-error-token.spec.js

@@ -0,0 +1,62 @@
+const {Readable} = require('stream');
+
+const ExcelJS = verquire('exceljs');
+
+// XLS-214 — the streaming WorkbookReader dropped the cached ERROR token on a FORMULA cell.
+//
+// A cell like `<c t="e"><f>1/0</f><v>#DIV/0!</v></c>` carries a formula AND a cached error
+// result. The streaming reader's formula branch only special-cased t="str"; everything else
+// (including t="e") fell through to `parseFloat(c.v.text)`, so `parseFloat('#DIV/0!')` → NaN and
+// the token was LOST. The bare-error `case 'e'` lower down only covers non-formula error cells.
+//
+// The oracle is the non-streaming `Workbook.load()`, which returns `result:{error:'#DIV/0!'}` for
+// the same cell. This test writes such a cell with the (non-streaming) writer, reads it back BOTH
+// ways, and asserts the streaming reader now matches the oracle. Revert the fork fix (the t="e"
+// case in worksheet-reader.js) and the streaming assertion goes RED (result becomes NaN).
+describe('github issues: XLS-214 streaming reader preserves a formula cell cached error token', () => {
+  let buffer;
+  let oracleValue;
+  let streamedValue;
+
+  before(async () => {
+    const wb = new ExcelJS.Workbook();
+    const ws = wb.addWorksheet('S');
+    ws.getCell('A1').value = {formula: '1/0', result: {error: '#DIV/0!'}};
+    buffer = await wb.xlsx.writeBuffer();
+
+    // Oracle: the non-streaming loader.
+    const oracleWb = new ExcelJS.Workbook();
+    await oracleWb.xlsx.load(buffer);
+    oracleValue = oracleWb.getWorksheet('S').getCell('A1').value;
+
+    // Subject: the streaming WorkbookReader path.
+    await new Promise((resolve, reject) => {
+      const reader = new ExcelJS.stream.xlsx.WorkbookReader(Readable.from(buffer), {
+        worksheets: 'emit',
+        sharedStrings: 'cache',
+        styles: 'cache',
+        hyperlinks: 'ignore',
+        entries: 'ignore',
+      });
+      reader.on('worksheet', worksheet =>
+        worksheet.on('row', row => {
+          if (row.number === 1) {
+            streamedValue = row.getCell(1).value;
+          }
+        })
+      );
+      reader.on('end', resolve);
+      reader.on('error', reject);
+      reader.read();
+    });
+  });
+
+  it('the non-streaming oracle returns result:{error} (the target shape)', () => {
+    expect(oracleValue).to.deep.equal({formula: '1/0', result: {error: '#DIV/0!'}});
+  });
+
+  it('the streaming reader preserves the error token (not NaN) — matches the oracle', () => {
+    expect(streamedValue).to.deep.equal(oracleValue);
+    expect(streamedValue.result).to.deep.equal({error: '#DIV/0!'});
+  });
+});