[nihilist-narwhal]

Formula entry and XLSX save got ~20% and ~13% slower over the ~880 commits since 23829d8. This finds where the time went and wins most of it back. Output doesn't change.

Measured with the calculation bench from #2651 (crates/engine/examples/bench.rs) at 100k rows on an M2 Pro, with main and this branch run in turn three times:

scenario	main	this PR
enter.formulas (paste 100k formulas)	~420 ms	~379 ms (−10%)
xlsx.save (300k-formula model)	~525 ms	~478 ms (−9%)

The other scenarios don't change beyond noise.

Changes

• number_to_text (f29ac7d): the check for rounding up to 1E+15 formatted every number with an exact {:.14e} and parsed it back. Formula printing calls this once per number literal, on entry and on save. The check now runs only for a ≥ 9.9999999999999e14. A value can only round up from 999999999999999.5 or higher, so the output is the same. New boundary tests: …999.5 → 1E+15, …999.4 → 999999999999999, …989.9 → 999999999999990.
• Expr::clone, Expr::map, xlsx prefix_params (2aeb21d): a binary node whose left operand isn't itself a chain (A1*2, the common case) no longer allocates a spine Vec, and map rewrites the operands in their existing boxes. The visit order and results are the same as the chain code with a spine of length 1. Expr::walk reserves 8 slots up front.
• grow_autofilter: checks that the sheet has an AutoFilter before looking up the cell. Most sheets have none.
• retained_bytes_against: looks up the newer band's row once per row instead of once per cell.
• Journal size cap: an entry's JSON size is counted with a writer that only counts bytes, instead of building the text. A big paste's params can be megabytes.

Not changed

What's left of the entry gap, and all of edit.fanout's (~+12%), is Undo's byte budget (#1227): sizing each snapshot and freeing evicted ones. That's the intended cost of bounding Undo's memory, so it stays. A follow-up could cache a size estimate per band, or free evicted snapshots off the hot path (native only).

Overlap with #449

#449 rewrites number_to_text to format every number with {:.14e} up front. Whichever PR merges second will conflict on this function, and #449 as written would bring the per-literal formatting cost back on formula entry and save. If #449 lands first, the same guard idea applies: only pay for exact formatting where the result can change.

Checks

fmt and clippy pass, and 951 tests pass in core, formula, model, xlsx and engine.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved display of numbers near the scientific-notation boundary, including more consistent rounding and formatting.
• Performance
  • Reduced overhead when processing formulas, workbook changes, and stored data.
  • Improved handling of edits near AutoFilter ranges while preserving existing range-growth behavior.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/gridcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: d8c5f58
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6fafe2a5-8f6f-4acb-ba92-7a8f09330968

Commits

Reviewing files that changed from the base of the PR and between d8c5f58 and 568d07e.

Files selected for processing: 6

• crates/core/src/value.rs
• crates/engine/src/cmd/data.rs
• crates/engine/src/lib.rs
• crates/formula/src/ast.rs
• crates/model/src/store.rs
• crates/xlsx/src/fmla.rs

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

Walkthrough

The pull request updates number-format boundary handling, AutoFilter growth checks, journal-size measurement, expression traversal, and snapshot byte accounting.

Changes: Number formatting

Layer / File(s)	Summary
Number formatting boundary crates/core/src/value.rs	The formatter applies its rounding check at or above the specified threshold. Tests add expectations for three values near 1e15.

AutoFilter growth

Layer / File(s)	Summary
AutoFilter growth eligibility crates/engine/src/cmd/data.rs	grow_autofilter checks that the edited cell is eligible before inspecting its value or formula. The range-growth conditions and outcome remain unchanged.

Journal size measurement

Layer / File(s)	Summary
Streamed journal size measurement crates/engine/src/lib.rs	ByteCounter measures serialized byte counts. push_journal streams entries through it instead of allocating a serialized byte vector.

Expression traversal

Layer / File(s)	Summary
Binary expression traversal paths crates/formula/src/ast.rs, crates/xlsx/src/fmla.rs	Expr::walk reserves capacity for eight pending nodes. Expr::map, Expr::clone, and prefix_params add direct paths for binary expressions whose left operand is not binary.

Snapshot byte accounting

Layer / File(s)	Summary
Snapshot row lookup crates/model/src/store.rs	retained_bytes_against resolves the matching row once per old row and reuses it for cell lookups. Byte accounting remains unchanged.

Priority: Low

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

Change: Refactor

Suggested reviewers: echelon, silvertree2010, harrison-booth, marcoarnulfo

Merge Risk: Minimal · up to 568d0

No concrete regression or merge-blocking risk is established by the reviewed changes.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 6 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 summarizes the primary objective: recovering formula-entry and XLSX-save performance lost since the referenced commit.
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

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

[FredApps]

Reviewed at SHA 568d07e.

What it does: a set of behaviour-preserving speed-ups for formula entry and XLSX save, measured by the author with the #2651 bench: number_to_text only does the exact 15-digit formatting for values near 1e15, grow_autofilter does its cheap AutoFilter checks before the cell lookup, push_journal sizes entries with a counting writer instead of building the JSON text, Expr::walk/map/clone and the XLSX prefix_params get a fast path for a single binary operator with a non-chain left operand, and CellStore hoists the per-row lookup in its size estimate.

Checked (all hunks read):

• number_to_text: the new guard a >= 9.9999999999999e14 is below the point where 15-digit rounding reaches 1e15 (about 999999999999999.5), so it only skips values that cannot round up; the added tests pin 999999999999999.5/.4 and 999999999999989.9.
• grow_autofilter: the two conditions are pure checks that must both hold, so reordering cannot change the result.
• ByteCounter: counts exactly the bytes to_vec would produce (saturating add), so journal size accounting is unchanged.
• Expr::map fast path: visits operands left to right then the parent, the same order as the chain path; right-hand recursion already existed in the chain path, so no new stack-depth exposure; clone and prefix_params fast paths follow the same pattern.
• The store.rs hoist is a pure refactor.

Not verified: I did not run the tests or reproduce the timings (the percentages are the author's, from an M2 Pro); the hosted gate is green at this head.