[clemens]

This builds on #2691 by @vangel76 (for #2681). It extends #2691's per-pass block sums to blocks with formula cells, so each running total down a column of formulas costs one addition instead of a walk over every row above it.

Why

#2691 remembers the sum and count of blocks that hold only constants. #2681 leaves blocks with formulas out of that memo. A common shape is a running total over a calculated column: B2 =A2*1.01, C2 =SUM(B$2:B2), C3 =SUM(B$2:B3), and so on. With #2691 alone, every row of C still walks all the rows of B above it, which is quadratic in the rows.

What

• The first commit is perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691 unchanged, applied onto current main as one commit, with vangel76 kept as its author. If perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691 merges first, I'll rebase and drop it.
• range_sum now also adds formula cells. It reads them through cell_value, in the row-major order that visit_range uses. Outside iterative calculation, a formula's result doesn't change once it has been calculated in a pass. So a block's sum stays valid for the rest of the pass, and a longer block of the same rows continues it: the same additions in the same order, starting from +0.0.
• A walk is not remembered when a formula it reads suspends the evaluation (MAX_NESTED) or spills. A block with an error value is remembered as None, as in perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691. Iterative calculation and spills found earlier in the pass still go the long way.
• COUNT reuses a block's count whatever the sum so far, because it doesn't use the sum. This was CodeRabbit's suggestion on perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691.
• Clippy's type_complexity flags two types from perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691. They now go through a BlockSums alias and a type alias in its test.

Results stay the same

• remembered_block_sums_change_no_result builds random sheets, recalculates them, then applies ten rounds of edits. It compares every value bit for bit, and the circular references found, with block sums on and off, using a test-only switch. The sheets have:
  • running totals of formula columns, including a 150-deep chain calculated on demand, so evaluations suspend inside a walk;
  • error values, and a spill that a total reads before the spill is calculated;
  • a circular reference, iterated in one of the sheets;
  • RAND, whose values follow the order formulas are calculated in.
• One exception, which the tests don't reach: a walk adds stack frames, so on a chain deep enough to run past STACK_BUDGET, evaluation can suspend at a different formula. That changes the order formulas are calculated in, and with it what RAND draws. Results that don't depend on that order stay the same.
• running_totals_over_formulas_walk_each_cell_once: 2,000 running totals walk 2,000 cells in all, and each total equals the top-down sum bit for bit.
• block_sums_count_values_spilled_in_the_same_pass, plus an assertion added to wide_read_at_the_nesting_limit_suspends_once, check the spill and suspension rules within a single pass. Across passes, the next pass would correct a wrong value from the spill it stores, so a whole-workbook test can't see these.
• Removing any one of the guards makes at least one test fail. The guards are: the suspension check, the new-spill check, the sheet-spill check, the iteration check, the error case and the +0.0 start.
• A private production workbook (6 sheets, about 214,000 formulas) gives bit-identical values for 20 input cases on main and with this PR.

Benchmarks

Median time of formulas.calculateNow through gridcraft-cli: release build, manual calculation mode, 30 runs each, Apple M3 Pro. All builds were measured alternately in the same session.

Workbook	main	#2691 alone	this PR
running_totals	21.3 s	23.2 s	55 ms (389× faster)
windows	1,024 ms	1,016 ms	1,022 ms
small_formulas	179 ms	174 ms	175 ms

The running_totals times on main and with #2691 alone come from 3 runs per build, which vary by about ±10%.

On the private workbook, one input case (set the inputs, calculateNow, read the results) takes 0.94 s on main and 0.83 s with this PR (0.93 s with #2691 alone). Each is the median of 3 runs of 20 cases, with the builds taking turns.

running_totals has 20,000 rows: A a constant, B =A*1.01, C =SUM(B$1:Bn), D =AVERAGE(B$1:Bn).

Synthetic workbooks (openpyxl)

"""Synthetic workbooks for the calc benchmarks (no private data).
usage: make_books.py <out dir>"""
import os, sys
import openpyxl

OUT = sys.argv[1]
os.makedirs(OUT, exist_ok=True)

def book(name, rows, cols):
    wb = openpyxl.Workbook()
    ws = wb.active
    for r in range(1, rows + 1):
        for col, f in cols:
            ws[f"{col}{r}"] = f(r)
    wb.save(os.path.join(OUT, name))

# Running totals over formula columns.
book("running_totals.xlsx", 20000, [
    ("A", lambda r: r * 0.1),
    ("B", lambda r: f"=A{r}*1.01"),
    ("C", lambda r: f"=SUM(B$1:B{r})"),
    ("D", lambda r: f"=AVERAGE(B$1:B{r})"),
])
# Windowed SUMIFS and lookups over formula columns.
book("windows.xlsx", 4000, [
    ("A", lambda r: f"=ROW()"),
    ("B", lambda r: f"=A{r}*1.5"),
    ("C", lambda r: f'=SUMIFS(B$1:B{r},A$1:A{r},"<="&A{r},A$1:A{r},">"&(A{r}-12))'),
    ("D", lambda r: f"=VLOOKUP(MOD({r},500)+1,A$1:B$500,2,FALSE)"),
    ("E", lambda r: f'=COUNTIFS(A$1:A{r},">"&(A{r}-24))'),
])
# Many small formulas.
book("small_formulas.xlsx", 30000, [
    ("A", lambda r: r),
    ("B", lambda r: f"=A{r}*2+1"),
    ("C", lambda r: f"=IF(B{r}>A{r},B{r}-A{r},0)"),
    ("D", lambda r: f"=ROUND(C{r}/3,2)+MAX(A{r},B{r})"),
    ("E", lambda r: f"=D{r}+C{r}*B{r}-A{r}"),
])
print(sorted(os.listdir(OUT)))

Related

• perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691 and Perf: SUM, COUNT and AVERAGE over big ranges cost about 47 ns per cell, and every formula walks the same range again #2681 (@vangel76): the base of this PR.
• calc: rebuild the dependency graph from shared syntax trees #2693 (@vangel76, for Perf: rebuilding the dependency graph deep-copies every formula's syntax tree #2685) makes the graph rebuild cheaper, and Recalc: defined names as static precedents, per-pass name cache, TEXT fast path #1499 (@RedBearAK) caches defined names. Both are independent of this PR.
• This is one of three performance PRs from profiling one large workbook. The others are Speed up running COUNTIFS/SUMIFS over growing column ranges #2954 (column blocks of formula results, building on Speed up running COUNTIFS/SUMIFS over growing column ranges #2814) and perf(calc): fewer lookups and allocations in recalculation #2952 (fewer lookups and allocations per cell read). All three change crates/calc/src/recalc/pass_host.rs near each other. Whichever lands second may need a small rebase.

🤖 Generated with Claude Code

Summary by CodeRabbit

Summary

• Performance
  • Range-based SUM, COUNT, and AVERAGE calculations can be faster for eligible ranges, while maintaining the same results as standard calculations.
  • Ranges containing errors or spills, and ranges without reusable results, continue to use the standard calculation path.
• Bug Fixes
  • Improved consistency of range calculations after edits and recalculation, including workbooks with formulas, spills, circular references, or iterative calculations.

[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: 68a4b1a
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 096848c4-8ecb-4ded-9deb-b4d1f3817736

Commits

Reviewing files that changed from the base of the PR and between 68a4b1a and f064383.

Files selected for processing: 5

• crates/calc/src/eval.rs
• crates/calc/src/eval/call.rs
• crates/calc/src/recalc.rs
• crates/calc/src/recalc/pass_host.rs
• crates/calc/src/tests.rs

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

Walkthrough

The calculator adds an optional host-provided range sum and count. PassHost caches eligible row-wise block sums and extends shorter cached blocks. Range aggregation uses these results in eligible cases. Tests compare cached results with reference calculations and check fallback behavior.

Changes: Range sum caching

Layer / File(s)	Summary
Range sum contract and aggregation crates/calc/src/eval.rs, crates/calc/src/eval/call.rs	Host adds an optional method for returning a range’s numeric sum and count. aggregate_area uses available results for eligible COUNT, SUM, and AVG ranges; otherwise, it continues through the existing traversal.
Per-pass cache and validation crates/calc/src/recalc/pass_host.rs, crates/calc/src/recalc.rs, crates/calc/src/tests.rs	PassHost caches eligible block sums and extends shorter cached blocks. Tests check running totals, spill cases, cell counts, and results with caching enabled and disabled.

Priority: Normal

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

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant aggregate_area
  participant PassHost
  participant sum_block
  participant cell_value
  aggregate_area->>PassHost: request range_sum
  PassHost->>sum_block: extend cached block
  sum_block->>cell_value: evaluate formula cells
  cell_value-->>sum_block: formula value or suspension
  sum_block-->>PassHost: block sum and count or no reusable result
  PassHost-->>aggregate_area: cached sum and count or None

Loading

Possibly related issues

• Perf: SUM, COUNT and AVERAGE over big ranges cost about 47 ns per cell, and every formula walks the same range again #2681 — The changes add per-pass reuse for range sums and counts, and test running totals over formula ranges.
• Recommendations: fuzz targets, CI checks, resource-limit suite #2166 — The changes add a nesting-limit assertion and tests for formula evaluation under recalculation.

Suggested reviewers: dylanmtaylor, aiongg

Merge Risk: Minimal · up to f0643

No concrete merge-blocking defect is established by the supplied evidence. Merge after normal validation.

Security architecture risk: Minimal · up to f0643

The optimization remains within workbook calculation and preserves traversal fallbacks when cached results cannot safely be used. No introduced security weakness was identified in the reviewed change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The new mutable state is scoped to one workbook's calculation host, with sheet-qualified keys and no cross-pass retention. The directly evidenced exposure is workbook calculation results and calculation-process resources; tenant or service-level sharing was not established.

Trust Boundaries and Controls

• inferred — The added public Rust method is an evaluator capability rather than a demonstrated external security entrypoint. Formula-controlled ranges continue to use calculator-owned workbook access and existing formula evaluation; the inspected fast path introduces no credential, network or privileged-operation sink.

Resilience and Maintainability Implications

• observed — Iteration and newly discovered spills on the requested sheet bypass cache lookup. A walk that suspends or produces a spill is not committed as a cached numeric result. Existing suspension recovery calculates dependencies before restarting abandoned evaluations, while errors and stored spills force traversal fallback.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: per-pass memoization of SUM, COUNT, and AVERAGE over blocks in the calc engine. The raw summary and objectives confirm this is the core change, which now covers ranges…
Docstring Coverage		Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
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.

Full details: Title check

Explanation

The title names the main change: per-pass memoization of SUM, COUNT, and AVERAGE over blocks in the calc engine. The raw summary and objectives confirm this is the core change, which now covers ranges with formula cells as well as constant-only ranges. The title uses the conventional perf(calc) form and states the performance intent. It is 79 characters, which is slightly above a short-title ideal but still concise and specific.

Full details: Docstring Coverage

Explanation

Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 8 functions across 5 files.

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.}

[clemens]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[FredApps]

ReviewerA — source-read COMMENT (not an approval)

I read the first hunk: the calculation host trait gains range_sum(sheet, range), returning (sum, count) of the numbers in a block that has no error or spilled value, "added row-major from 0.0 exactly as visit_range would hand them over (formula cells calculated in the same order)", or None if the block is not like that or the host can't tell; per the title (#2681) SUM, COUNT and AVERAGE of constant-only blocks are remembered per pass, so a running total over a growing range is not quadratic. The doc comment sets the right requirement: floating-point addition is not associative, so the cached sum must be added in the same order as the plain loop, or results differ in the last bits and a recalculation after a reload differs from the first. Cache hits must be dropped when a cell in the block changes during the pass (iterative calculation), and only constant cells may be in the block. This and gridcraft#2814 (COUNTIFS and SUMIFS column cache) touch the same area; say how they relate. I did not read the cache. Add a property test comparing the cached and uncached results bit for bit on random blocks (with text, blanks, booleans, huge and tiny numbers) and the timing for 50,000 rows. Nothing was run.

I ran nothing and have no exact-head CI result for this PR, so this is not an approval. Where a diff was long I read its code hunks rather than every generated or documentation line.

Status: waiting_author.