[clemens]

Fixes #2813

This builds on #2814 by @Silvertree2010 (for #2813). It extends #2814's per-pass column cache from one column of constants to blocks of any number of columns, including formula results.

Why

#2814 keeps the constants of a single column, so that running COUNTIFS/SUMIFS ranges ($B$2:B2, $B$2:B3, …) stop walking the cell store again for every formula. Ranges over formula columns, and blocks of more than one column such as a VLOOKUP table over A$1:B$500, still take the normal path. On every read, they gather each cell from the cell store and the pass's results.

What

• The first commit is Speed up running COUNTIFS/SUMIFS over growing column ranges #2814 unchanged, cherry-picked onto current main, with Silvertree2010 kept as its author. If Speed up running COUNTIFS/SUMIFS over growing column ranges #2814 merges first, I'll rebase and drop it.
• A kept block now covers the columns of the range read: it is keyed by sheet and first and last column, and holds values row-major. As in Speed up running COUNTIFS/SUMIFS over growing column ranges #2814, it grows up or down as later reads need.
• Formula cells are read through cell_value, in the order that range_values reads them. Outside iterative calculation, a formula's result doesn't change once it has been calculated in a pass, so the kept values stay valid.
• To keep that order, a block reads only rows of the range asked for. A read that doesn't touch the kept block replaces it, instead of filling the rows in between: filling them would calculate formulas the reader never reads, ahead of their turn. This can create a circular reference that main doesn't have.
• A read is not kept when:
  • a formula in it is still pending (in progress, or abandoned for a suspension);
  • a formula in it spills;
  • a formula read for it changes the block kept of the same columns in the meantime (grows, replaces or gives it up);
  • it no longer fits the budget (see below).
    While iterating, a formula ends the block, as in Speed up running COUNTIFS/SUMIFS over growing column ranges #2814.
• A read within the kept block copies the rows it asks for straight from the block, as in Speed up running COUNTIFS/SUMIFS over growing column ranges #2814. Growing a block downwards moves the kept values instead of copying them.
• Formulas read for a block may keep blocks of other columns, so the budget is checked again after the read, before the block is kept. Harrison-Booth asked on Speed up running COUNTIFS/SUMIFS over growing column ranges #2814 for a test of the budget: column_blocks_stay_within_the_budget covers it.
• From CodeRabbit's review of Speed up running COUNTIFS/SUMIFS over growing column ranges #2814: a block given up for the rest of the pass (for a stored spill, or a formula while iterating) now gives its values back to the budget.
• Clippy's type_complexity flags one type in Speed up running COUNTIFS/SUMIFS over growing column ranges #2814's test, which now goes through a type alias.

Results stay the same

• kept_column_blocks_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 kept blocks on and off, using a test-only switch. Formulas read growing, moving and fixed blocks of one to three columns, through SUMIFS, COUNTIFS, AVERAGEIFS, SUMIF, COUNTIF, VLOOKUP, INDEX, MATCH and SUMPRODUCT. The sheets have:
  • a 150-deep chain calculated on demand, so evaluations suspend inside a read;
  • error values, and a spill that is read before it 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: reading a block 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.
• Tests within a single pass cover six cases:
  • a read of a formula in progress (column_blocks_keep_only_final_values);
  • a spill found during a read;
  • a nested read that grows the same block;
  • a read away from the kept rows (column_blocks_read_no_rows_between);
  • a nested read that keeps a block of other columns and leaves no room in the budget;
  • a block given up for the pass, which gives its values back to the budget.
• Removing any one of the rules makes at least one test fail. The rules are: the pending check, the new-spill check, the sheet-spill check, the stored-spill check, the nested-read check, reading no rows between, the iteration rule, the budget check after the read, and giving the budget back.
• 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	#2814 alone	this PR
windows	1,024 ms	919 ms (−10%)	175 ms (5.8× faster)
small_formulas	179 ms	172 ms	176 ms

running_totals stays at about 21 s with this PR (3 runs per build, which vary by about ±10%, too much to show a difference).

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

windows has 4,000 rows:

• A =ROW() and B =A*1.5;
• C: a SUMIFS over B$1:Bn with two criteria on A$1:An;
• D: a VLOOKUP into A$1:B$500;
• E: a COUNTIFS over A$1:An.

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)))

Not covered

• Lookups still copy the key column out of the kept block on each call. Perf: lookups copy the whole key column or row on every call (VLOOKUP, HLOOKUP, LOOKUP, MATCH, XLOOKUP, XMATCH) #2678 (@vangel76) proposes searching a borrowed view instead, which this PR doesn't do.
• The work per formula is still proportional to its range length, as Speed up running COUNTIFS/SUMIFS over growing column ranges #2814 notes. This PR makes the constant smaller for formula columns but doesn't change the shape.

Related

• Speed up running COUNTIFS/SUMIFS over growing column ranges #2814 and Recalc: a running COUNTIFS over a growing range ($B$2:B2) is 13-19x slower than Excel and grows faster #2813 (@Silvertree2010, @KNUFFIXX): the base of this PR.
• This is one of three performance PRs from profiling one large workbook. The others are perf(calc): remember SUM/COUNT/AVERAGE of constant-only blocks per pass (#2681) #2953 (running SUM/COUNT/AVERAGE over formula cells, building on perf(calc): SUM/COUNT/AVERAGE over big ranges reuse constant-only results #2691) 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

• Performance
  • Multi-row spreadsheet calculations can now reuse cached column values, improving efficiency across repeated and overlapping range reads.
  • Text criteria matching is more efficient, including wildcard searches and comparisons involving array values.
  • Calculation results and circular-reference behavior remain consistent when column-block reads are used.

[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: f2c5bc23-5650-4f49-92ac-8cc00a48080b

Commits

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

Files selected for processing: 4

• crates/calc/src/recalc.rs
• crates/calc/src/recalc/pass_host.rs
• crates/calc/src/tests.rs
• crates/functions/src/criteria.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

PassHost now caches eligible multi-row column reads within a per-pass value budget. Text criteria store parsed wildcard tokens and use updated matching paths. Tests compare calculation results with column-block reads enabled and disabled.

Changes: Running range calculations

Layer / File(s)	Summary
Text criteria matching crates/functions/src/criteria.rs	Text criteria store parsed wildcard tokens. Matching borrows scalar values and uses a byte-based path for ASCII text.
Column-block cache and range reads crates/calc/src/recalc/pass_host.rs	PassHost caches eligible column blocks within a per-pass budget. range_values uses the cache for eligible multi-row ranges and otherwise retains the existing range-reading path.
Calculation and cache validation crates/calc/src/recalc.rs, crates/calc/src/tests.rs	Tests check cache growth and budget behavior, spills, edits, and formula results against uncached calculations. An ignored timing test compares running COUNTIFS and COUNT recalculation.

Priority: Normal

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

Change: Refactor · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant COUNTIFS
  participant range_values
  participant PassHost
  participant read_rows
  COUNTIFS->>range_values: request multi-row range values
  range_values->>PassHost: request column block
  PassHost->>read_rows: read missing rows
  read_rows-->>PassHost: return row values
  PassHost-->>range_values: return cached block values
  range_values-->>COUNTIFS: return range values

Loading

Suggested reviewers: aiongg, dylanmtaylor

Merge Risk: Minimal · up to dc180

No demonstrated calculation or matching regression remains; the PR is mergeable after normal checks.

Security architecture risk: Low · up to dc180

The optimization keeps cached values within one calculation pass and preserves fallback paths for unfinished formulas, spills and iterative calculation. No introduced security defect was established. Remaining uncertainty concerns peak resource use and volatile-formula behavior in exceptionally deep dependency chains.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Workbook-controlled formulas, ranges and criteria influence computation and retained values in the calculating process. Cache identity includes the sheet and exact column span, and its lifetime is confined to the host. The inspected path establishes no cross-workbook cache sharing; shared-service or tenant exposure remains unknown.

Trust Boundaries and Controls

• observed — Wildcard matching retains the preexisting per-operation step budget. Both ASCII and non-ASCII whole-text paths use the budgeted matcher, and wildcard search shares one budget across attempts.

Resilience and Maintainability Implications

• observed — Focused tests cover final-value retention, nested growth, disjoint reads, same-pass spills, nested budget consumption and budget release. A separate cache-on/cache-off test compares values and circular references across recalculation and edits, including spills and iteration. These are inspected assertions, not verified execution results.

Hardening Proposals

• proposed — For deployments that calculate untrusted workbooks in shared processes, consider byte-aware cache accounting and peak-memory checks covering large text values and nested reads. This would strengthen resource containment beyond the retained-slot cap; it is not an observed vulnerability.

Pre-merge checks: 3 · 1 · 1

Failed checks: 1 warning · 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check		The implementation directly targets [#2813]. PassHost::column_block caches growing multi-column ranges, including formula results, and criteria.rs avoids repeated wildcard parsing and value clonin…	Provide reproducible timings for the [#2813] generator at the issue's row counts, including recalculation after edits, and report the resulting scaling and correctness.

Passed checks: 3 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: improving performance for running COUNTIFS and SUMIFS formulas over growing column ranges.
Out of Scope Changes check		The changes stay within [#2813]. The cache implementation, criteria matching optimization, and tests support faster growing-range COUNTIFS and related criteria reads. The randomized tests validate c…

Full details: Out of Scope Changes check

Explanation

The changes stay within [#2813]. The cache implementation, criteria matching optimization, and tests support faster growing-range COUNTIFS and related criteria reads. The randomized tests validate cache behavior and result preservation. No unrelated production feature or unrelated test change is demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 69.23% which is insufficient. 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 13 functions across 4 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Linked Issues check

Explanation

The implementation directly targets [#2813]. PassHost::column_block caches growing multi-column ranges, including formula results, and criteria.rs avoids repeated wildcard parsing and value cloning. Tests cover the ledger workload, edits above the table, and formula results. The available evidence does not establish the requested performance against the issue's 1,000–8,000-row workload or show its scaling toward Excel. The timing test is ignored and compares COUNTIFS with COUNT, not with Excel.

Resolution

Provide reproducible timings for the [#2813] generator at the issue's row counts, including recalculation after edits, and report the resulting scaling and correctness.

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]

ReviewerA — source-read COMMENT (not an approval)

I read the first hunk: the recalculation tests gain a helper pass_results(cells, order) that calculates a pass with the given formulas in a given order and returns the results, and per the title COUNTIFS and SUMIFS over growing column ranges are sped up. This is the same optimisation as gridcraft#2814 (a COLUMN_CACHE_BUDGET cache for COUNTIFS and SUMIFS passes) and is related to gridcraft#2953 (SUM, COUNT and AVERAGE of constant-only blocks), from the same author as this one: three PRs on one problem that will conflict. The maintainers should choose one design, or say how they combine. The test helper that runs the same cells in different orders is a good basis for the main risk: results must not depend on the order in which cells are calculated. Add tests comparing the cached and uncached results bit for bit for several orders, text and error cells in the range, an edit between passes, and a 50,000-row timing. I did not read the optimisation. 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.