feat(sheet): 37 missing Excel functions, array spill rendering, _xlfn round trip - #373
Merged
Merged
Conversation
HyperFormula ships 418 functions but misses most of the modern Excel set. Register them as one plugin so they behave like built-ins (coercion, errors, autocomplete): CONCAT, TEXTBEFORE/TEXTAFTER, NUMBERVALUE, FIXED, DOLLAR, XMATCH, LOOKUP, UNIQUE, SORT, SORTBY, TAKE, DROP, VSTACK, HSTACK, TOCOL, TOROW, CHOOSECOLS, CHOOSEROWS, EXPAND, AVERAGEIFS, RANK(.EQ/.AVG), MODE(.SNGL/.MULT), TRIMMEAN, PERMUT, PERMUTATIONA, INTERCEPT, FORECAST(.LINEAR), FREQUENCY, ERROR.TYPE, TYPE, XIRR. Array results now render: blank cells fall back to the engine value, so spilled ranges (UNIQUE, SORT, SEQUENCE, FILTER) are visible like in Excel. XLSX round trip: Excel namespaces post-2007 functions (_xlfn.XLOOKUP, _xlfn._xlws.SORT). Strip on import, add back on export - without it every modern formula we wrote showed #NAME? in Excel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
PR Summary by QodoAdd 37 Excel functions, spill rendering, and XLSX namespaces
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
PR Summary by QodoAdd 37 Excel functions, spill rendering, and XLSX namespaces
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1. CSV omits visible spills
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+642
to
+644
| const cash = nums(vs); | ||
| const when = nums(ds); | ||
| if (cash.length !== when.length || cash.length < 2) return numErr('XIRR needs matching values and dates.'); |
There was a problem hiding this comment.
2. Xirr pairs shift silently 🐞 Bug ≡ Correctness
xirr filters cash flows and dates independently before pairing them by index. Nonnumeric entries at different positions can therefore associate a cash flow with the wrong date while still passing the equal-length check, producing an incorrect financial result.
Agent Prompt
## Issue description
`XIRR` independently compacts its values and dates, which can shift positional associations and calculate a rate from incorrect pairs.
## Issue Context
Validate original range lengths and process entries positionally. Reject invalid dates or filter whole pairs according to the intended Excel semantics, rather than filtering each range independently.
## Fix Focus Areas
- ui/src/js/sheet/excelFunctions.ts[632-668]
- ui/src/js/sheet/excelFunctions.ts[132-150]
- ui/src/js/sheet/excelFunctions.test.ts[207-221]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+181
to
+182
| const spilled = engine.getValue(r, c); | ||
| return spilled.type === 'empty' ? '' : formatValue(spilled.value, '', propsOf(r, c).numFmt); |
There was a problem hiding this comment.
3. Csv omits visible spills 🐞 Bug ≡ Correctness
Spilled values are now rendered in cells without persisted raw content, but CSV bounds are still calculated only from nonempty raw cells. A sheet with =SEQUENCE(10) only in A1 visibly has ten rows but exports just the anchor row.
Agent Prompt
## Issue description
CSV export omits visible rows and columns produced by array spills because its used-range calculation only considers persisted raw cells.
## Issue Context
Extend CSV bounds using nonempty evaluated spill cells, or expose spill ranges from the formula engine. Avoid an unbounded worksheet scan and add a dynamic-array CSV regression test.
## Fix Focus Areas
- ui/src/js/sheet/sheetEditor.ts[175-185]
- ui/src/js/sheet/sheetEditor.ts[320-332]
- ui/src/js/sheet/formulaEngine.ts[43-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+48
to
+50
| for _, p := range []string{"_xlfn._xlws.", "_xlfn.", "_xlws."} { | ||
| formula = strings.ReplaceAll(formula, p, "") | ||
| } |
There was a problem hiding this comment.
4. Prefix stripping rewrites literals 🐞 Bug ≡ Correctness
stripFunctionPrefixes removes namespace substrings from the entire imported formula instead of
only function tokens. For example, the formula ="_xlfn.XLOOKUP(" is silently changed to
="XLOOKUP(", corrupting its string-literal content.
Agent Prompt
## Issue description
XLSX import strips namespace substrings inside quoted formula text and other non-function tokens.
## Issue Context
Use a quote-aware and token-aware scanner similar to the export implementation. Remove a prefix only when it directly qualifies a function call, and add tests for quoted literals and escaped quotes.
## Fix Focus Areas
- lib/xlsx/formulanames.go[45-51]
- lib/xlsx/formulanames.go[54-101]
- lib/xlsx/formulanames_test.go[27-37]
- lib/xlsx/import.go[58-61]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+40
to
+42
| // Dotted names are the 2010+ statistical/compatibility set (NORM.DIST, | ||
| // MODE.SNGL, CEILING.MATH, ...), all of which Excel namespaces. | ||
| return strings.Contains(name, ".") |
There was a problem hiding this comment.
5. Legacy function gets prefixed 🐞 Bug ≡ Correctness
needsPrefix classifies every dotted name as a post-2007 function, so the newly supported legacy ERROR.TYPE exports as _xlfn.ERROR.TYPE. That namespace form is not valid for ERROR.TYPE and can make the exported formula unrecognized.
Agent Prompt
## Issue description
The blanket dotted-name heuristic incorrectly namespaces legacy dotted functions such as `ERROR.TYPE` during XLSX export.
## Issue Context
Replace the heuristic with an explicit classification of functions requiring `_xlfn` or add a verified legacy exclusion set. Add an export regression test for `ERROR.TYPE`.
## Fix Focus Areas
- lib/xlsx/formulanames.go[17-43]
- lib/xlsx/formulanames_test.go[5-24]
- ui/src/js/sheet/excelFunctions.ts[597-616]
- ui/src/js/sheet/excelFunctions.ts[1013-1016]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+103
to
+106
| const fixedText = (n: number, decimals: number, commas: boolean): string => { | ||
| const d = Math.trunc(decimals); | ||
| const rounded = d < 0 ? Math.round(n / 10 ** -d) * 10 ** -d : n; | ||
| const s = rounded.toFixed(Math.max(0, d)); |
There was a problem hiding this comment.
7. Fixed rounds negative ties wrong 🐞 Bug ≡ Correctness
fixedText uses Math.round for negative decimal places, which rounds negative half-ties toward positive infinity rather than away from zero. Consequently, FIXED(-125,-1) returns -120 instead of Excel-compatible -130.
Agent Prompt
## Issue description
`FIXED` produces incorrect results for negative numbers at half-ties when rounding left of the decimal point.
## Issue Context
Implement explicit half-away-from-zero rounding, such as rounding the absolute magnitude and restoring the sign. Add regression coverage for positive and negative tie cases.
## Fix Focus Areas
- ui/src/js/sheet/excelFunctions.ts[101-108]
- ui/src/js/sheet/excelFunctions.ts[256-266]
- ui/src/js/sheet/excelFunctions.test.ts[54-60]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was fehlt(e)
HyperFormula bringt 418 Funktionen mit, aber praktisch keine der modernen Excel-Funktionen. Dieser PR schließt die größten Lücken.
Neue Funktionen (als ein HyperFormula-Plugin, damit sie sich wie Built-ins verhalten)
Autocomplete in der Formelleiste zieht die Namen automatisch aus der HyperFormula-Registry, also ohne weitere Verdrahtung.
Array-Spill wird gerendert
Bisher zeigte nur die Formelzelle selbst einen Wert — Zellen ohne eigenen
rawblieben leer, wodurch jedes Array-Ergebnis (auch das schon vorhandene FILTER/SEQUENCE/TRANSPOSE) unsichtbar war. Leere Zellen fragen jetzt die Engine, spilled Ranges erscheinen wie in Excel.XLSX-Round-Trip
Excel speichert alles ab 2010 mit Namespace (
_xlfn.XLOOKUP,_xlfn._xlws.SORT,_xlfn.NORM.DIST). Import strippt die Präfixe, Export setzt sie wieder — vorher zeigte Excel#NAME?für jede moderne Formel, die wir geschrieben haben (betraf auch schon TEXTJOIN/IFS/XLOOKUP).Tests
26 neue Vitest-Cases (
excelFunctions.test.ts) gegen ein Fixture-Grid, plus Go-Tests für die Präfix-Umschreibung inkl. String-Literalen.go test ./lib/xlsx/... ./lib/sheetdoc/...undvitest run(119 Tests) grün.Bewusst ausgelassen
🤖 Generated with Claude Code