amadan
Fix round 2: reword two comments to name real mechanisms
graph.go's collectRefs comment claimed an over-cap range's precedents become stale until 'a full recalc walks it again' — no such full-recalc primitive exists in this API (only Recalc(origin), scoped to transitive dependents, and Value(key), a single fresh walk). Reworded to state what actually makes the missing edge safe: evalCallArg applies the identical maxRangeCells cap to the same Range at evaluation time, so an untracked range's formula is always #REF! and never reads live data from it — there is no stale-but-plausible value for the missing edge to fail to invalidate. eval.go's maxRangeCells comment called the trigger a 'malformed or pathological reference'; the realistic trigger is an ordinary whole-column formula (e.g. SUM over A1:A200000) on a tab at its own 200,000-cell cap, not a malformed one. Reworded to describe that trigger accurately. Comment-only changes; no behaviour differs.
2 files changed,
+19
−15
internal/sheets/calc/eval.go+11 −10internal/sheets/calc/graph.go+8 −5
Intent
diff --git a/internal/sheets/calc/eval.go b/internal/sheets/calc/eval.go| index c7ad758..7910ec8 100644 |
| --- a/internal/sheets/calc/eval.go |
| +++ b/internal/sheets/calc/eval.go |
| @@ -70,16 +70,17 @@ func refKey(r *Ref) CellKey { |
| // maxRangeCells caps how many cells a single Range expands to in |
| // rangeCells. The spec's own hard ceiling is 200,000 cells per tab (see |
| -// spec §4.4's budgets), so no legitimately addressable range can cover |
| -// more than that — this cap sits at exactly that ceiling, which means it |
| -// can only ever fire on a malformed or pathological reference, never on a |
| -// real one. Without it, a single formula's Range — e.g. one spanning a |
| -// tab at its own cap — would synchronously materialize up to 200,000 |
| -// CellKeys, twice over: once as call arguments when a range-taking |
| -// function evaluates it (evalCallArg), and again as dependency-graph |
| -// precedent edges every time SetFormula parses that same formula |
| -// (collectRefs). This is rangeCells' fan-in counterpart to Recalc's |
| -// maxDirtyPerPass, which caps the symmetric fan-out case. |
| +// spec §4.4's budgets), so no addressable range can cover more than |
| +// that — this cap sits at exactly that ceiling. The realistic way to hit |
| +// it is not a malformed reference but an ordinary whole-column formula — |
| +// e.g. a SUM over A1:A200000 on a tab sized at its own cap — from a user |
| +// who has no reason to know they've asked for every cell the tab has. |
| +// Without this cap, that single formula's Range would synchronously |
| +// materialize up to 200,000 CellKeys, twice over: once as call arguments |
| +// when a range-taking function evaluates it (evalCallArg), and again as |
| +// dependency-graph precedent edges every time SetFormula parses that same |
| +// formula (collectRefs). This is rangeCells' fan-in counterpart to |
| +// Recalc's maxDirtyPerPass, which caps the symmetric fan-out case. |
| const maxRangeCells = 200_000 |
| // rangeCells expands a Range into every CellKey it covers, normalizing a |
diff --git a/internal/sheets/calc/graph.go b/internal/sheets/calc/graph.go| index 9dff996..8b881c8 100644 |
| --- a/internal/sheets/calc/graph.go |
| +++ b/internal/sheets/calc/graph.go |
| @@ -77,11 +77,14 @@ func (g *Graph) SetFormula(key CellKey, n Node) { |
| // precedent/dependent edges. |
| // |
| // A *Range over maxRangeCells is left untracked here rather than adding |
| -// hundreds of thousands of edges for one formula: the formula still |
| -// evaluates correctly (evalCallArg turns the same oversized range into a |
| -// #REF! argument), but a change to a cell inside that range will not by |
| -// itself dirty this formula until a full recalc walks it again. Disclosed |
| -// trade-off, not a silent one — see maxRangeCells. |
| +// hundreds of thousands of edges for one formula. This is safe, not just |
| +// cheap: evalCallArg applies the exact same cap to the same Range when |
| +// the formula actually evaluates, turning it into a single #REF! |
| +// argument (see maxRangeCells) — so a formula with an untracked range |
| +// never reads live data from that range in the first place. There is no |
| +// stale-but-plausible value for a missing edge to fail to invalidate; |
| +// the formula is #REF! until its Range shrinks under the cap, at which |
| +// point re-parsing it (SetFormula) tracks it again. |
| func collectRefs(n Node) []CellKey { |
| var out []CellKey |
| var walk func(Node) |