Full document, spreadsheet, slideshow, and diagram tooling
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix(sheets): detect cross-sheet circular dependencies in recalc engine

The recalculation engine previously only detected circular references
within a single sheet. Cross-sheet cycles (e.g., Sheet1!A1 -> Sheet2!B1
-> Sheet1!A1) went undetected because each sheet's engine only knew
about its own cells' formulas.

Added crossSheetFormulaDeps and currentSheetName options to RecalcOptions.
When provided, buildFullGraph() traverses cross-sheet references via BFS,
adding their dependency edges to the graph so Kahn's algorithm can detect
cycles spanning multiple sheets. Cross-sheet formula cells participate in
cycle detection but are not evaluated locally.

Includes 13 tests covering: simple 2-sheet cycles, 3+ sheet chains,
valid (non-circular) cross-sheet refs, self-references with cross-sheet
options enabled, mixed cyclic/non-cyclic cells, incremental updates,
and backwards compatibility.

Closes #522

+424
+12
src/sheets/types.ts
··· 124 124 onEvaluate?: (cellId: string) => void; 125 125 namedRanges?: NamedRangesMap; 126 126 crossSheetResolver?: CrossSheetResolver; 127 + /** 128 + * Resolve cross-sheet formula dependencies for cycle detection. 129 + * Given a fully-qualified cell key (e.g. "Sheet2!B1"), returns the set of 130 + * fully-qualified cell keys that cell's formula depends on. 131 + * Returns null/undefined if the cell has no formula or the sheet doesn't exist. 132 + */ 133 + crossSheetFormulaDeps?: (qualifiedCellId: string) => Set<string> | null | undefined; 134 + /** 135 + * The name of the current sheet. Used to map local cell IDs to fully-qualified 136 + * keys when detecting cross-sheet circular dependencies. 137 + */ 138 + currentSheetName?: string; 127 139 } 128 140 129 141 // --- Chart Types ---
+412
tests/cross-sheet-circular.test.ts
··· 1 + import { describe, it, expect } from 'vitest'; 2 + import { RecalcEngine } from '../src/sheets/recalc.js'; 3 + 4 + /** 5 + * Cross-sheet circular dependency detection tests. 6 + * 7 + * Issue #522: The recalculation engine must detect circular dependencies 8 + * that span multiple sheets. For example, Sheet1!A1 references Sheet2!B1, 9 + * which references Sheet1!A1 — this is a cycle that must produce #CIRCULAR!. 10 + * 11 + * The dependency graph uses fully-qualified keys like "Sheet2!B1" for 12 + * cross-sheet refs. The crossSheetFormulaDeps option allows the engine 13 + * to resolve formula dependencies on other sheets for cycle detection. 14 + */ 15 + 16 + // --- Helpers --- 17 + 18 + /** 19 + * Create a simple cell store from a plain object. 20 + * Each entry: cellId -> { v, f } where f is formula string (without '='). 21 + */ 22 + function makeCellStore(data: Record<string, { v: unknown; f: string }>) { 23 + const store = new Map<string, { v: unknown; f: string }>(); 24 + for (const [id, cell] of Object.entries(data)) { 25 + store.set(id, { ...cell }); 26 + } 27 + return { 28 + get(id: string) { return store.get(id) || null; }, 29 + set(id: string, cell: { v: unknown; f: string }) { store.set(id, { ...cell }); }, 30 + has(id: string) { return store.has(id); }, 31 + entries() { return store.entries(); }, 32 + getAllFormulaCells() { 33 + const result: Array<[string, { v: unknown; f: string }]> = []; 34 + for (const [id, cell] of store.entries()) { 35 + if (cell.f) result.push([id, cell]); 36 + } 37 + return result; 38 + }, 39 + }; 40 + } 41 + 42 + /** 43 + * Build a crossSheetFormulaDeps resolver from a multi-sheet data structure. 44 + * sheetsFormulas: { sheetName: { cellId: Set<qualifiedRef> } } 45 + * 46 + * Given "Sheet2!B1", looks up sheetsFormulas["Sheet2"]["B1"] to get deps. 47 + */ 48 + function makeCrossSheetFormulaDeps( 49 + sheetsFormulas: Record<string, Record<string, Set<string>>>, 50 + ) { 51 + return (qualifiedCellId: string): Set<string> | null => { 52 + const bangIdx = qualifiedCellId.indexOf('!'); 53 + if (bangIdx === -1) return null; 54 + const sheetName = qualifiedCellId.slice(0, bangIdx); 55 + const cellId = qualifiedCellId.slice(bangIdx + 1); 56 + const sheet = sheetsFormulas[sheetName]; 57 + if (!sheet) return null; 58 + return sheet[cellId] || null; 59 + }; 60 + } 61 + 62 + // ===================================================================== 63 + // 1. SIMPLE CROSS-SHEET CYCLE: Sheet1!A1 -> Sheet2!B1 -> Sheet1!A1 64 + // ===================================================================== 65 + 66 + describe('Cross-sheet circular dependency detection', () => { 67 + it('detects simple two-sheet cycle: Sheet1!A1 -> Sheet2!B1 -> Sheet1!A1', () => { 68 + // Sheet1: A1 has formula referencing Sheet2!B1 69 + const store = makeCellStore({ 70 + A1: { v: '', f: 'Sheet2!B1+1' }, 71 + }); 72 + 73 + // Sheet2: B1 has formula referencing Sheet1!A1 74 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 75 + Sheet2: { 76 + B1: new Set(['Sheet1!A1']), 77 + }, 78 + }); 79 + 80 + const engine = new RecalcEngine(store, { 81 + currentSheetName: 'Sheet1', 82 + crossSheetFormulaDeps, 83 + }); 84 + engine.buildFullGraph(); 85 + 86 + const changed = engine.recalculate('A1'); 87 + 88 + // A1 should be marked as circular 89 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 90 + expect(changed.has('A1')).toBe(true); 91 + }); 92 + 93 + it('provides cycle path for cross-sheet cycle', () => { 94 + const store = makeCellStore({ 95 + A1: { v: '', f: 'Sheet2!B1+1' }, 96 + }); 97 + 98 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 99 + Sheet2: { 100 + B1: new Set(['Sheet1!A1']), 101 + }, 102 + }); 103 + 104 + const engine = new RecalcEngine(store, { 105 + currentSheetName: 'Sheet1', 106 + crossSheetFormulaDeps, 107 + }); 108 + engine.buildFullGraph(); 109 + 110 + engine.recalculate('A1'); 111 + 112 + const cyclePaths = engine.getCyclePaths(); 113 + expect(cyclePaths.length).toBeGreaterThan(0); 114 + const path = cyclePaths[0]; 115 + // Cycle should form a loop: first === last 116 + expect(path[0]).toBe(path[path.length - 1]); 117 + // Path should include both the local cell and the cross-sheet cell 118 + expect(path.length).toBeGreaterThanOrEqual(3); 119 + }); 120 + 121 + // ===================================================================== 122 + // 2. LONGER CROSS-SHEET CYCLE THROUGH 3+ SHEETS 123 + // ===================================================================== 124 + 125 + it('detects cycle through 3 sheets: Sheet1!A1 -> Sheet2!A1 -> Sheet3!A1 -> Sheet1!A1', () => { 126 + // Sheet1: A1 has formula referencing Sheet2!A1 127 + const store = makeCellStore({ 128 + A1: { v: '', f: 'Sheet2!A1+1' }, 129 + }); 130 + 131 + // Sheet2!A1 depends on Sheet3!A1, Sheet3!A1 depends on Sheet1!A1 132 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 133 + Sheet2: { 134 + A1: new Set(['Sheet3!A1']), 135 + }, 136 + Sheet3: { 137 + A1: new Set(['Sheet1!A1']), 138 + }, 139 + }); 140 + 141 + const engine = new RecalcEngine(store, { 142 + currentSheetName: 'Sheet1', 143 + crossSheetFormulaDeps, 144 + }); 145 + engine.buildFullGraph(); 146 + 147 + const changed = engine.recalculate('A1'); 148 + 149 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 150 + expect(changed.has('A1')).toBe(true); 151 + }); 152 + 153 + it('detects cycle through 4 sheets', () => { 154 + // Sheet1!A1 -> Sheet2!B2 -> Sheet3!C3 -> Sheet4!D4 -> Sheet1!A1 155 + const store = makeCellStore({ 156 + A1: { v: '', f: 'Sheet2!B2*2' }, 157 + }); 158 + 159 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 160 + Sheet2: { 161 + B2: new Set(['Sheet3!C3']), 162 + }, 163 + Sheet3: { 164 + C3: new Set(['Sheet4!D4']), 165 + }, 166 + Sheet4: { 167 + D4: new Set(['Sheet1!A1']), 168 + }, 169 + }); 170 + 171 + const engine = new RecalcEngine(store, { 172 + currentSheetName: 'Sheet1', 173 + crossSheetFormulaDeps, 174 + }); 175 + engine.buildFullGraph(); 176 + 177 + const changed = engine.recalculate('A1'); 178 + 179 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 180 + }); 181 + 182 + // ===================================================================== 183 + // 3. VALID CROSS-SHEET REFERENCE (NOT CIRCULAR) 184 + // ===================================================================== 185 + 186 + it('does NOT flag valid cross-sheet reference as circular', () => { 187 + // Sheet1!A1 -> Sheet2!B1 -> Sheet2!C1 (no cycle back to Sheet1) 188 + const store = makeCellStore({ 189 + A1: { v: '', f: 'Sheet2!B1+1' }, 190 + }); 191 + 192 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 193 + Sheet2: { 194 + B1: new Set(['Sheet2!C1']), // depends on another Sheet2 cell, no cycle 195 + }, 196 + }); 197 + 198 + const engine = new RecalcEngine(store, { 199 + currentSheetName: 'Sheet1', 200 + crossSheetFormulaDeps, 201 + crossSheetResolver: { 202 + sheetExists: () => true, 203 + getSheetCellValue: () => 42, 204 + }, 205 + }); 206 + engine.buildFullGraph(); 207 + 208 + const changed = engine.recalculate('A1'); 209 + 210 + // A1 should NOT be circular 211 + expect(store.get('A1')!.v).not.toBe('#CIRCULAR!'); 212 + expect(engine.getCyclePaths().length).toBe(0); 213 + }); 214 + 215 + it('evaluates valid cross-sheet chain correctly', () => { 216 + // Sheet1!A1 references Sheet2!B1 which has no back-reference. 217 + // A1's formula evaluates using crossSheetResolver. 218 + const store = makeCellStore({ 219 + A1: { v: '', f: 'Sheet2!B1+10' }, 220 + }); 221 + 222 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 223 + Sheet2: { 224 + B1: new Set([]), // B1 is a plain value, no deps 225 + }, 226 + }); 227 + 228 + const engine = new RecalcEngine(store, { 229 + currentSheetName: 'Sheet1', 230 + crossSheetFormulaDeps, 231 + crossSheetResolver: { 232 + sheetExists: () => true, 233 + getSheetCellValue: (_sheet: string, _ref: string) => 5, 234 + }, 235 + }); 236 + engine.buildFullGraph(); 237 + 238 + engine.recalculate('A1'); 239 + 240 + // Should evaluate: 5 + 10 = 15 241 + expect(store.get('A1')!.v).toBe(15); 242 + }); 243 + 244 + it('handles multiple local cells referencing different sheets without cycles', () => { 245 + const store = makeCellStore({ 246 + A1: { v: '', f: 'Sheet2!A1+1' }, 247 + B1: { v: '', f: 'Sheet3!A1+2' }, 248 + C1: { v: '', f: 'A1+B1' }, 249 + }); 250 + 251 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 252 + Sheet2: { A1: new Set([]) }, 253 + Sheet3: { A1: new Set([]) }, 254 + }); 255 + 256 + const engine = new RecalcEngine(store, { 257 + currentSheetName: 'Sheet1', 258 + crossSheetFormulaDeps, 259 + crossSheetResolver: { 260 + sheetExists: () => true, 261 + getSheetCellValue: () => 10, 262 + }, 263 + }); 264 + engine.buildFullGraph(); 265 + 266 + engine.recalculate('A1'); 267 + engine.recalculate('B1'); 268 + 269 + expect(store.get('A1')!.v).not.toBe('#CIRCULAR!'); 270 + expect(store.get('B1')!.v).not.toBe('#CIRCULAR!'); 271 + expect(engine.getCyclePaths().length).toBe(0); 272 + }); 273 + 274 + // ===================================================================== 275 + // 4. SELF-REFERENCING CELL 276 + // ===================================================================== 277 + 278 + it('detects self-referencing cell (same as existing test, ensures no regression)', () => { 279 + const store = makeCellStore({ 280 + A1: { v: '', f: 'A1+1' }, 281 + }); 282 + const engine = new RecalcEngine(store); 283 + engine.buildFullGraph(); 284 + 285 + engine.recalculate('A1'); 286 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 287 + }); 288 + 289 + it('detects self-referencing cell with cross-sheet options enabled', () => { 290 + // Ensure the cross-sheet machinery doesn't break self-reference detection 291 + const store = makeCellStore({ 292 + A1: { v: '', f: 'A1+1' }, 293 + }); 294 + 295 + const engine = new RecalcEngine(store, { 296 + currentSheetName: 'Sheet1', 297 + crossSheetFormulaDeps: () => null, 298 + }); 299 + engine.buildFullGraph(); 300 + 301 + engine.recalculate('A1'); 302 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 303 + }); 304 + 305 + // ===================================================================== 306 + // 5. MIXED SCENARIOS 307 + // ===================================================================== 308 + 309 + it('marks only cyclic cells, not unrelated cells on the same sheet', () => { 310 + // A1 is in a cross-sheet cycle; B1 is a normal formula 311 + const store = makeCellStore({ 312 + A1: { v: '', f: 'Sheet2!A1+1' }, 313 + B1: { v: 10, f: '' }, 314 + C1: { v: '', f: 'B1*2' }, 315 + }); 316 + 317 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 318 + Sheet2: { 319 + A1: new Set(['Sheet1!A1']), // cycle! 320 + }, 321 + }); 322 + 323 + const engine = new RecalcEngine(store, { 324 + currentSheetName: 'Sheet1', 325 + crossSheetFormulaDeps, 326 + }); 327 + engine.buildFullGraph(); 328 + 329 + engine.recalculate('A1'); 330 + engine.recalculate('B1'); 331 + 332 + // A1 is circular 333 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 334 + // C1 is not circular -- it depends only on B1 335 + expect(store.get('C1')!.v).toBe(20); 336 + }); 337 + 338 + it('detects cycle introduced by cross-sheet dependency after incremental update', () => { 339 + // Initially A1 is a plain value, no cycle 340 + const store = makeCellStore({ 341 + A1: { v: 10, f: '' }, 342 + }); 343 + 344 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 345 + Sheet2: { 346 + B1: new Set(['Sheet1!A1']), 347 + }, 348 + }); 349 + 350 + const engine = new RecalcEngine(store, { 351 + currentSheetName: 'Sheet1', 352 + crossSheetFormulaDeps, 353 + }); 354 + engine.buildFullGraph(); 355 + 356 + // No cycle yet 357 + engine.recalculate('A1'); 358 + expect(store.get('A1')!.v).toBe(10); 359 + 360 + // Now edit A1 to create a formula that references Sheet2!B1 -> cycle 361 + store.set('A1', { v: '', f: 'Sheet2!B1+1' }); 362 + engine.updateCell('A1'); 363 + // Rebuild to pick up cross-sheet edges 364 + engine.buildFullGraph(); 365 + 366 + const changed = engine.recalculate('A1'); 367 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 368 + }); 369 + 370 + it('works without crossSheetFormulaDeps (backwards compatibility)', () => { 371 + // When crossSheetFormulaDeps is not provided, cross-sheet cycles 372 + // won't be detected, but existing same-sheet behavior must work 373 + const store = makeCellStore({ 374 + A1: { v: '', f: 'B1+1' }, 375 + B1: { v: '', f: 'A1+1' }, 376 + }); 377 + 378 + const engine = new RecalcEngine(store); 379 + engine.buildFullGraph(); 380 + 381 + engine.recalculate('A1'); 382 + expect(store.get('A1')!.v).toBe('#CIRCULAR!'); 383 + expect(store.get('B1')!.v).toBe('#CIRCULAR!'); 384 + }); 385 + 386 + it('handles cross-sheet ref to a cell with no formula (leaf node)', () => { 387 + // Sheet1!A1 -> Sheet2!B1 where Sheet2!B1 is a plain value (no formula deps) 388 + const store = makeCellStore({ 389 + A1: { v: '', f: 'Sheet2!B1+1' }, 390 + }); 391 + 392 + const crossSheetFormulaDeps = makeCrossSheetFormulaDeps({ 393 + Sheet2: {}, // B1 has no formula entry at all 394 + }); 395 + 396 + const engine = new RecalcEngine(store, { 397 + currentSheetName: 'Sheet1', 398 + crossSheetFormulaDeps, 399 + crossSheetResolver: { 400 + sheetExists: () => true, 401 + getSheetCellValue: () => 7, 402 + }, 403 + }); 404 + engine.buildFullGraph(); 405 + 406 + engine.recalculate('A1'); 407 + 408 + // Should evaluate normally: 7 + 1 = 8 409 + expect(store.get('A1')!.v).toBe(8); 410 + expect(engine.getCyclePaths().length).toBe(0); 411 + }); 412 + });