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

Configure Feed

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

Merge pull request 'fix: formula bugs — INDEX 2D, MATCH types, NPV, safety guards' (#238) from fix/formula-bugs-qa into main

scott 77cc06e7 d86725e7

+188 -17
+1 -1
package.json
··· 1 1 { 2 2 "name": "tools", 3 - "version": "0.23.2", 3 + "version": "0.23.3", 4 4 "private": true, 5 5 "type": "module", 6 6 "main": "electron/main.js",
+37 -6
src/sheets/formulas.ts
··· 684 684 resolveRange(startRef: string, endRef: string): RangeArray { 685 685 const start = parseRef(startRef); 686 686 const end = parseRef(endRef); 687 + if (!start || !end) return ['#REF!'] as unknown as RangeArray; 687 688 const values: RangeArray = []; 688 689 const rowMin = Math.min(start.row, end.row); 689 690 const rowMax = Math.max(start.row, end.row); 690 691 const colMin = Math.min(start.col, end.col); 691 692 const colMax = Math.max(start.col, end.col); 693 + if ((rowMax - rowMin + 1) * (colMax - colMin + 1) > 10000) return ['#VALUE!'] as unknown as RangeArray; 692 694 for (let r = rowMin; r <= rowMax; r++) { 693 695 for (let c = colMin; c <= colMax; c++) { 694 696 const ref = colToLetter(c) + r; ··· 773 775 case 'ROUNDUP': { const f = Math.pow(10, toNum(args[1] ?? 0)); return Math.ceil(toNum(args[0]) * f) / f; } 774 776 case 'ROUNDDOWN': { const f = Math.pow(10, toNum(args[1] ?? 0)); return Math.floor(toNum(args[0]) * f) / f; } 775 777 case 'INT': return Math.floor(toNum(args[0])); 776 - case 'MOD': return toNum(args[0]) % toNum(args[1]); 778 + case 'MOD': { const mDiv = toNum(args[1]); return mDiv === 0 ? '#DIV/0!' : toNum(args[0]) % mDiv; } 777 779 case 'POWER': return Math.pow(toNum(args[0]), toNum(args[1])); 778 780 case 'SQRT': return Math.sqrt(toNum(args[0])); 779 781 case 'LOG': return args.length > 1 ? Math.log(toNum(args[0])) / Math.log(toNum(args[1])) : Math.log10(toNum(args[0])); ··· 862 864 const range = Array.isArray(args[0]) ? args[0] : [args[0]]; 863 865 const row = toNum(args[1]) - 1; 864 866 const col = args[2] != null ? toNum(args[2]) - 1 : 0; 865 - return range[row] ?? '#REF!'; 867 + const idxCols = (range as RangeArray)._rangeCols || range.length; 868 + const idxRows = (range as RangeArray)._rangeRows || 1; 869 + if (row < 0 || row >= idxRows || col < 0 || col >= idxCols) return '#REF!'; 870 + return range[row * idxCols + col] ?? '#REF!'; 866 871 } 867 872 868 873 case 'MATCH': { 869 874 const needle = args[0]; 870 875 const range = Array.isArray(args[1]) ? args[1] : [args[1]]; 871 - const idx = range.findIndex(v => v === needle || String(v) === String(needle)); 872 - return idx === -1 ? '#N/A' : idx + 1; 876 + const matchType = args[2] != null ? toNum(args[2]) : 1; 877 + if (matchType === 0) { 878 + // Exact match 879 + const idx = range.findIndex(v => v === needle || String(v) === String(needle)); 880 + return idx === -1 ? '#N/A' : idx + 1; 881 + } else if (matchType === 1) { 882 + // Sorted ascending: find largest value <= needle 883 + const nv = toNum(needle); 884 + let bestIdx = -1; 885 + for (let i = 0; i < range.length; i++) { 886 + const rv = toNum(range[i]); 887 + if (rv <= nv) bestIdx = i; else break; 888 + } 889 + return bestIdx === -1 ? '#N/A' : bestIdx + 1; 890 + } else { 891 + // match_type = -1: sorted descending: find smallest value >= needle 892 + const nv = toNum(needle); 893 + let bestIdx = -1; 894 + for (let i = 0; i < range.length; i++) { 895 + const rv = toNum(range[i]); 896 + if (rv >= nv) bestIdx = i; else break; 897 + } 898 + return bestIdx === -1 ? '#N/A' : bestIdx + 1; 899 + } 873 900 } 874 901 875 902 case 'ADDRESS': { ··· 1160 1187 case 'FACT': { 1161 1188 const fn2 = Math.floor(toNum(args[0])); 1162 1189 if (fn2 < 0) return '#NUM!'; 1190 + if (fn2 > 170) return '#NUM!'; // 171! exceeds Number.MAX_VALUE 1163 1191 if (fn2 <= 1) return 1; 1164 1192 let fResult = 1; 1165 1193 for (let i = 2; i <= fn2; i++) fResult *= i; ··· 1211 1239 case 'PROPER': { 1212 1240 return String(args[0]).toLowerCase().replace(/(?:^|\s|[^\w])\w/g, c => c.toUpperCase()); 1213 1241 } 1214 - case 'REPT': return String(args[0]).repeat(Math.max(0, Math.floor(toNum(args[1])))); 1242 + case 'REPT': { const rCount = Math.max(0, Math.floor(toNum(args[1]))); if (rCount > 32767) return '#VALUE!'; return String(args[0]).repeat(rCount); } 1215 1243 case 'EXACT': return String(args[0]) === String(args[1]); 1216 1244 case 'REPLACE': { 1217 1245 const rpText = String(args[0]); ··· 1355 1383 case 'NPV': { 1356 1384 const npvRate = toNum(args[0]); 1357 1385 let npvSum = 0; 1386 + let npvPeriod = 1; 1358 1387 for (let i = 1; i < args.length; i++) { 1359 1388 const npvVals = Array.isArray(args[i]) ? (args[i] as unknown[]).map(toNum) : [toNum(args[i])]; 1360 1389 for (const nvItem of npvVals) { 1361 - npvSum += nvItem / Math.pow(1 + npvRate, i); 1390 + npvSum += nvItem / Math.pow(1 + npvRate, npvPeriod); 1391 + npvPeriod++; 1362 1392 } 1363 1393 } 1364 1394 return npvSum; ··· 1525 1555 // SEQUENCE(rows, [cols], [start], [step]) 1526 1556 const seqRows = Math.max(1, Math.floor(toNum(args[0]))); 1527 1557 const seqCols = args.length > 1 ? Math.max(1, Math.floor(toNum(args[1]))) : 1; 1558 + if (seqRows * seqCols > 10000) return '#VALUE!'; 1528 1559 const seqStart = args.length > 2 ? toNum(args[2]) : 1; 1529 1560 const seqStep = args.length > 3 ? toNum(args[3]) : 1; 1530 1561 const seqValues: unknown[] = [];
+147 -7
tests/formulas-edge-cases.test.ts
··· 43 43 // ============================================================ 44 44 45 45 describe('Division by zero — various contexts', () => { 46 - it('MOD(5, 0) returns Infinity or NaN', () => { 47 - const result = evalWith('MOD(5,0)'); 48 - // JS % 0 returns NaN 49 - expect(result).toBeNaN(); 46 + it('MOD(5, 0) returns #DIV/0!', () => { 47 + expect(evalWith('MOD(5,0)')).toBe('#DIV/0!'); 50 48 }); 51 49 52 50 it('1/0 returns #DIV/0!', () => { ··· 388 386 389 387 it('MATCH with exact match finds first occurrence', () => { 390 388 const cells = { A1: 'x', A2: 'y', A3: 'x' }; 391 - expect(evalWith('MATCH("x",A1:A3)', cells)).toBe(1); 389 + expect(evalWith('MATCH("x",A1:A3,0)', cells)).toBe(1); 392 390 }); 393 391 394 - it('MATCH returns #N/A for value not in range', () => { 392 + it('MATCH returns #N/A for value not in range (exact match)', () => { 395 393 const cells = { A1: 1, A2: 2, A3: 3 }; 396 - expect(evalWith('MATCH(99,A1:A3)', cells)).toBe('#N/A'); 394 + expect(evalWith('MATCH(99,A1:A3,0)', cells)).toBe('#N/A'); 397 395 }); 398 396 399 397 it('nested INDEX(MATCH()) pattern', () => { ··· 854 852 expect(result).toContain('#NAME?'); 855 853 }); 856 854 }); 855 + 856 + // ============================================================ 857 + // QA issue #364 — Formula bug fixes 858 + // ============================================================ 859 + 860 + describe('INDEX — 2D range with col argument', () => { 861 + it('INDEX returns correct cell from 2D range using row and col', () => { 862 + // 3x2 range: A1=1, B1=2, A2=3, B2=4, A3=5, B3=6 863 + const cells: Record<string, unknown> = { 864 + A1: 1, B1: 2, A2: 3, B2: 4, A3: 5, B3: 6, 865 + }; 866 + const result = evaluate('INDEX(A1:B3,2,2)', (ref) => cells[ref] ?? ''); 867 + expect(result).toBe(4); // row 2, col 2 = B2 868 + }); 869 + 870 + it('INDEX with col=1 returns first column value', () => { 871 + const cells: Record<string, unknown> = { 872 + A1: 10, B1: 20, A2: 30, B2: 40, 873 + }; 874 + const result = evaluate('INDEX(A1:B2,2,1)', (ref) => cells[ref] ?? ''); 875 + expect(result).toBe(30); // row 2, col 1 = A2 876 + }); 877 + 878 + it('INDEX with out-of-bounds col returns #REF!', () => { 879 + const cells: Record<string, unknown> = { A1: 1, B1: 2 }; 880 + const result = evaluate('INDEX(A1:B1,1,5)', (ref) => cells[ref] ?? ''); 881 + expect(result).toBe('#REF!'); 882 + }); 883 + 884 + it('INDEX with out-of-bounds row returns #REF!', () => { 885 + const cells: Record<string, unknown> = { A1: 1, A2: 2 }; 886 + const result = evaluate('INDEX(A1:A2,5,1)', (ref) => cells[ref] ?? ''); 887 + expect(result).toBe('#REF!'); 888 + }); 889 + }); 890 + 891 + describe('MATCH — match_type argument', () => { 892 + it('MATCH with match_type=0 does exact match', () => { 893 + const cells: Record<string, unknown> = { A1: 10, A2: 20, A3: 30 }; 894 + const result = evaluate('MATCH(20,A1:A3,0)', (ref) => cells[ref] ?? ''); 895 + expect(result).toBe(2); 896 + }); 897 + 898 + it('MATCH with match_type=1 finds largest value <= needle (sorted asc)', () => { 899 + const cells: Record<string, unknown> = { A1: 10, A2: 20, A3: 30 }; 900 + const result = evaluate('MATCH(25,A1:A3,1)', (ref) => cells[ref] ?? ''); 901 + expect(result).toBe(2); // 20 is the largest <= 25 902 + }); 903 + 904 + it('MATCH with match_type=-1 finds smallest value >= needle (sorted desc)', () => { 905 + const cells: Record<string, unknown> = { A1: 30, A2: 20, A3: 10 }; 906 + const result = evaluate('MATCH(25,A1:A3,-1)', (ref) => cells[ref] ?? ''); 907 + expect(result).toBe(1); // 30 is the smallest >= 25 908 + }); 909 + 910 + it('MATCH with match_type=1 returns #N/A when needle is smaller than all', () => { 911 + const cells: Record<string, unknown> = { A1: 10, A2: 20, A3: 30 }; 912 + const result = evaluate('MATCH(5,A1:A3,1)', (ref) => cells[ref] ?? ''); 913 + expect(result).toBe('#N/A'); 914 + }); 915 + 916 + it('MATCH with match_type=-1 returns #N/A when needle is larger than all', () => { 917 + const cells: Record<string, unknown> = { A1: 30, A2: 20, A3: 10 }; 918 + const result = evaluate('MATCH(35,A1:A3,-1)', (ref) => cells[ref] ?? ''); 919 + expect(result).toBe('#N/A'); 920 + }); 921 + 922 + it('MATCH default match_type is 1 (sorted ascending)', () => { 923 + const cells: Record<string, unknown> = { A1: 10, A2: 20, A3: 30 }; 924 + const result = evaluate('MATCH(25,A1:A3)', (ref) => cells[ref] ?? ''); 925 + expect(result).toBe(2); // defaults to match_type=1 926 + }); 927 + }); 928 + 929 + describe('resolveRange — oversized range guard', () => { 930 + it('huge range does not freeze and returns quickly', () => { 931 + const start = Date.now(); 932 + const result = evalWith('SUM(ZZZ999:A1)'); 933 + const elapsed = Date.now() - start; 934 + // Must complete in under 100ms (not hang), and return a finite value 935 + expect(elapsed).toBeLessThan(100); 936 + expect(result).toBe(0); // SUM coerces #VALUE! guard to 0 937 + }); 938 + 939 + it('INDEX on oversized range returns #VALUE!', () => { 940 + const result = evalWith('INDEX(A1:ZZ500,1,1)'); 941 + expect(result).toBe('#VALUE!'); 942 + }); 943 + }); 944 + 945 + describe('NPV — correct discount exponents for range values', () => { 946 + it('NPV with individual values uses sequential exponents', () => { 947 + // NPV(10%, -1000, 300, 420, 680) 948 + // = 300/(1.1)^1 + 420/(1.1)^2 + 680/(1.1)^3 949 + const result = evalWith('NPV(0.1,-1000,300,420,680)'); 950 + expect(result).toBeCloseTo( 951 + -1000/1.1 + 300/Math.pow(1.1,2) + 420/Math.pow(1.1,3) + 680/Math.pow(1.1,4), 952 + 2, 953 + ); 954 + }); 955 + 956 + it('NPV with a range uses sequential exponents across range values', () => { 957 + // NPV(rate, range) where range = [100, 200, 300] 958 + // Should be: 100/1.1^1 + 200/1.1^2 + 300/1.1^3 959 + const cells: Record<string, unknown> = { A1: 100, A2: 200, A3: 300 }; 960 + const result = evaluate('NPV(0.1,A1:A3)', (ref) => cells[ref] ?? ''); 961 + const expected = 100/1.1 + 200/Math.pow(1.1,2) + 300/Math.pow(1.1,3); 962 + expect(result).toBeCloseTo(expected, 2); 963 + }); 964 + }); 965 + 966 + describe('Safety guards — FACT, REPT, SEQUENCE', () => { 967 + it('FACT(170) returns a number (max before Infinity)', () => { 968 + expect(evalWith('FACT(170)')).toBeLessThan(Infinity); 969 + }); 970 + 971 + it('FACT(171) returns #NUM! (would be Infinity)', () => { 972 + expect(evalWith('FACT(171)')).toBe('#NUM!'); 973 + }); 974 + 975 + it('FACT(1000) returns #NUM! not Infinity', () => { 976 + expect(evalWith('FACT(1000)')).toBe('#NUM!'); 977 + }); 978 + 979 + it('REPT with huge count returns #VALUE!', () => { 980 + expect(evalWith('REPT("a",100000)')).toBe('#VALUE!'); 981 + }); 982 + 983 + it('REPT with reasonable count works', () => { 984 + expect(evalWith('REPT("ab",3)')).toBe('ababab'); 985 + }); 986 + 987 + it('SEQUENCE with huge dimensions returns #VALUE!', () => { 988 + expect(evalWith('SEQUENCE(10000,10000)')).toBe('#VALUE!'); 989 + }); 990 + 991 + it('SEQUENCE with reasonable size works', () => { 992 + const result = evalWith('SEQUENCE(3,1,1,1)'); 993 + expect(Array.isArray(result)).toBe(true); 994 + expect((result as number[])[2]).toBe(3); 995 + }); 996 + });
+3 -3
tests/formulas.test.ts
··· 560 560 561 561 it('MATCH returns #N/A when value not found', () => { 562 562 const cells = { A1: 10, A2: 20, A3: 30 }; 563 - const result = evalWith('MATCH(99,A1:A3)', cells); 563 + const result = evalWith('MATCH(99,A1:A3,0)', cells); 564 564 expect(result).toBe('#N/A'); 565 565 }); 566 566 567 567 it('INDEX/MATCH combo works', () => { 568 568 // Use MATCH to find position, then INDEX to get corresponding value 569 569 const cells = { A1: 'apple', A2: 'banana', A3: 'cherry', B1: 100, B2: 200, B3: 300 }; 570 - // MATCH("banana",A1:A3) = 2, INDEX(B1:B3,2) = 200 571 - const matchResult = evalWith('MATCH("banana",A1:A3)', cells); 570 + // MATCH("banana",A1:A3,0) = 2, INDEX(B1:B3,2) = 200 571 + const matchResult = evalWith('MATCH("banana",A1:A3,0)', cells); 572 572 expect(matchResult).toBe(2); 573 573 const indexResult = evalWith('INDEX(B1:B3,2)', cells); 574 574 expect(indexResult).toBe(200);