[clemens]

On a sheet where most formulas read a few neighbouring cells, recalculation spends much of its time on bookkeeping around each cell read rather than on the formulas. This PR removes some of that bookkeeping. Results stay the same, bit for bit, with one exception for RAND described below.

Changes

• Evaluator::value reads a single-cell reference straight through area_value. Before, it built a Vec of areas through eval, reference_areas and deref. The nesting checks are the same: enter, now shared with eval.
• resolve_sheet returns the sheet range instead of a Vec.
• PassHost::cell_value looks in the pass's results first (known), so reading a cell calculated earlier in the pass takes one hash lookup instead of three.
• binary allocates only for a chain of operators, not for a plain a+b.
• A second commit adds me to contributors/people.toml, as the file asks contributors to do in their own PR.

Results stay the same

New tests cover:

• single-cell reads: another sheet, a one-sheet 3-D reference, a reference over several sheets (#VALUE!), a missing sheet (#REF!), and a formula calculated on demand;
• left-to-right operator chains;
• the MAX_DEPTH limit, reached by a recursive LAMBDA whose deepest call reads a cell.

The tests pass on main too, so they pin the existing behaviour.

One exception, which the tests don't reach: a single-cell read now takes fewer stack frames, so on a chain deep enough to run past STACK_BUDGET, evaluation can suspend at a different formula than on main. 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.

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. main and this PR were measured alternately in the same session.

Workbook	main	this PR
small_formulas	179 ms	158 ms (−12%)
windows	1,024 ms	910 ms (−11%)

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.85 s with this PR. Each is the median of 3 runs of 20 cases, with the builds taking turns.

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

This is one of three independent performance PRs that came out of profiling recalculation of one large workbook. The other two are #2953 (running SUM/COUNT/AVERAGE over formula cells, building on #2691) and #2954 (column blocks of formula results, building on #2814). 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

• Bug Fixes
  • Improved evaluation of single-cell references across sheets, including clearer handling of multi-sheet references and invalid sheet selections.
  • Fixed results for chained operators and formulas that read cells during recursive calculations, including correct errors when recursion exceeds the limit.
  • Improved recalculation so previously computed values and iterative-calculation results are handled consistently.

[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: 90ec3a1e-c867-44c9-bf5e-fb08914569b6

Commits

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

Files selected for processing: 4

• contributors/people.toml
• crates/calc/src/eval.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; 9 remain after this review.

Walkthrough

The calculation evaluator changes how it handles evaluation depth, direct cell-reference values, and binary operator chains. Recalculation now retrieves known results before checking pending cells. Contributor metadata adds a GitHub name synchronization entry for Clemens.

Changes: Calculation evaluation

Layer / File(s)	Summary
Evaluation depth checks crates/calc/src/eval.rs, crates/calc/src/tests.rs	Evaluation entry checks stack budget and maximum depth before incrementing depth. Recursive-lambda tests now include a formula that reads a cell.
Direct cell-reference values crates/calc/src/eval.rs, crates/calc/src/tests.rs	Direct cell references resolve the sheet selection and read the cell for a single-sheet result. Tests cover on-demand evaluation, cross-sheet references, 3-D references, and error results.
Binary operator chains crates/calc/src/eval.rs, crates/calc/src/tests.rs	The evaluator applies the outer operator after folding the left-associated chain and evaluating the original right operand. Tests cover arithmetic, concatenation, and error propagation.
Recalculation result lookup crates/calc/src/recalc/pass_host.rs	PassHost retrieves known results and propagates iterative-calculation taint. cell_value returns known results before checking whether a cell is pending.

Contributor metadata

Layer / File(s)	Summary
GitHub name synchronization contributors/people.toml	The contributor metadata adds Clemens with GitHub name synchronization enabled.

Priority: Low

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

Change: Refactor

Possibly related issues

• storytold/gridcraft issue 2161: The changes update evaluation depth and stack-budget checks, but do not add the issue’s proposed combined budget across evaluators.

Suggested reviewers: echelon, aiongg, dotcoocoo

Merge Risk: Minimal · up to 66a52

No merge-blocking issue was identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 40.00% 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.

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 describes the main recalculation performance changes, including fewer lookups and allocations.
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: Docstring Coverage

Explanation

Docstring coverage is 40.00% 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 10 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

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

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