feat(xls-codec): recover formula text from BIFF8 Ptg token streams - #890
Merged
Conversation
Mearman
marked this pull request as ready for review
September 3, 2026 17:29
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Walks a Formula record's compiled rgce token stream (the postfix Ptg vocabulary [MS-XLS] 2.5.198 defines) and rebuilds it into the infix text a spreadsheet application would show, using an operand stack where each entry carries the precedence of whatever built it so a child gets literal parentheses only when its own precedence genuinely requires them under the operator combining it -- correct regardless of an operator's true associativity, since a same-precedence right child only ever needs wrapping when the postfix stream itself demanded that grouping. Covers literal operands (PtgInt/PtgNum/PtgStr/PtgBool/PtgErr/ PtgMissArg), cell and range references including their 3D forms ($-qualified per their own relative/absolute flags, a 3D reference's sheet resolved through a caller-supplied FormulaSheetContext), every arithmetic/comparison/unary/percent operator plus explicit parentheses, PtgAttrSum's single-range SUM optimisation, and function calls through both PtgFunc (fixed arity) and PtgFuncVar (variable arity, its own on-disk cparams) resolved by name against the built-in Ftab table. A token this reader does not resolve -- a shared formula's PtgExp, an array constant's PtgArray, a defined name, a natural- language reference, or an unresolved 3D reference -- aborts the whole parse rather than guessing. The Ftab table (ptg-functions.ts) transcribes every entry [MS-XLS] 2.5.198.17 publishes, plus each function's fixed argument count where the grammar states one with no optional or repeated argument -- the split confirmed empirically against real LibreOffice-written BIFF8 (PI, SIN, ROUND, ATAN2, SYD, and REPLACE all compile as PtgFunc with no on-disk count; COUNT, SUMIF, and CONCATENATE as PtgFuncVar with an explicit cparams byte), since PtgFunc's own token carries no count of its own for this module to read back.
A PtgRef3d/PtgArea3d's own ixti indexes the EXTERNSHEET record's rgXTI array; each XTI names a supporting link (a SupBook record) plus a sheet-scope pair. The workbook globals reader now collects every SupBook in arrival order, recognising its cch field's 0x0401 marker as "this workbook itself" rather than another workbook, a DDE/OLE data source, or an add-in, and resolves EXTERNSHEET's own XTI array against that collection into WorkbookGlobals.sheetRanges -- direct BoundSheet8 indices for a self-referencing XTI whose itabFirst/itabLast name a real sheet, undefined for everything else this reader does not resolve (a genuinely external workbook, or the "sheet could not be found" -1 sentinel). BlockCursor gains i16(), sign-extended the same way i32() already is, for itabFirst/itabLast's own signed 16-bit encoding.
readFormula now reads past the cached FormulaValue into the record's flags, calculation cache, and CellParsedFormula (cce + rgce), handing the token bytes to parseFormulaText alongside the sheet context readSheetRecords is given. RawCell gains an optional formula field, populated whenever the Ptg reader resolves the expression and left absent otherwise -- the cached value keeps working exactly as before regardless.
… expressions readSheet now threads the workbook's own sheets and resolved 3D sheetRanges into readSheetRecords, and mapCell carries a resolved RawCell.formula onto ContentSheetCell.formula -- the last hop from a BIFF8 Formula record's compiled expression to the schema field a caller actually reads.
…ndary Updates the Status/Architecture sections, the Formula-records row of the writer-scope table, and the Read-side gaps entry to describe what the Ptg reader now resolves and name the three constructs that still leave ContentSheetCell.formula absent -- shared formulas, array formulas, and genuinely external workbook references.
Mearman
force-pushed
the
feat/xls-codec-formula-text-recovery
branch
from
September 3, 2026 17:57
ec0dbd6 to
4d3c705
Compare
Contributor
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Progresses #815
Recovers
ContentSheetCell.formulafor BIFF8Formularecords by implementing thePtgcompiled-expression vocabulary ([MS-XLS] 2.5.198) and rebuilding it into infix formula text.What's covered
PtgInt,PtgNum,PtgStr(with quote-doubling),PtgBool,PtgErr,PtgMissArg.$-qualified per their own relative/absolute flags:PtgRef,PtgArea, and their 3D (cross-sheet) formsPtgRef3d/PtgArea3d, resolved throughEXTERNSHEETand a self-referencingSupBook(WorkbookGlobals.sheetRanges), with sheet-name quoting ('Sheet Name'!...) matched to when Excel itself would quote.PtgAdd/Sub/Mul/Div/Power/Concat/Lt/Le/Eq/Ge/Gt/Ne/Uplus/Uminus/Percent) and explicit parentheses (PtgParen), via a precedence-tagged operand stack that only wraps a child in()when the operator combining it genuinely requires it.PtgAttrSum's single-rangeSUM(...)optimisation.PtgFunc(fixed arity) andPtgFuncVar(variable arity, its own on-diskcparams), resolved by name against the full published Ftab table (src/biff/ptg-functions.ts), withPtgFunc's own fixed argument count taken from a curated subset of that same table (only where the grammar states a fixed, non-optional arity —PtgFunc's own token carries no count).PtgAttrIf/PtgAttrGoto/PtgAttrSemi/PtgAttrSpace/PtgAttrSpaceSemi/PtgAttrBaxcelconsumed as pure control/display no-ops.Every byte layout above was cross-checked against a real BIFF8 workbook LibreOffice wrote for the identical formulas during development (arithmetic, refs,
SUM/AVERAGE/IF/ROUND/ATAN2/SYD/REPLACE/SUMIF/CONCATENATE, a cross-sheetSUM, and a parenthesised(A1+B1)*C1) — it produced byte-for-byte the same token streams the tests assert against.What still leaves
formulaabsentPer the task's explicit scope boundary, three constructs abort the whole parse for that cell rather than fabricating text, exactly as the reader already does for anything it can't map:
PtgExp) — the real expression lives in a separateShrFmlarecord this reader does not yet join.PtgArray) — the literal values live in a separatePtgExtraArraytrailer this reader does not yet parse.SupBooknaming another file, a DDE/OLE data source, or an add-in, rather than this same workbook.Also unresolved: defined names (
PtgName/PtgNameX) and natural-language "Elf" references, for the same reason defined names (Lbl) aren't read at all yet.No write side —
writeXlsContentcontinues to silently ignoreContentSheetCell.formula, unchanged; writing Ptg tokens back out is a materially separate task.Verification
src/biff/ptg.test.ts— 23 hand-built-byte-sequence unit tests (per this package's own fixture convention) covering every operator, precedence/parenthesisation edge cases, literal types, both function forms, 3D resolution (success, multi-sheet, and unresolved), and the unsupported-token/malformed-stack abort paths.src/workbook/globals.test.ts—EXTERNSHEET/SupBookresolution, including the "not self-referencing" and "sheet could not be found" cases.src/workbook/sheet.test.ts/src/content.test.ts— end-to-end wiring, including a real cross-sheetSUMthrough a hand-built compound file._lint,_typecheck,_test,_test:workersfor xls-codec, plus a full-workspace_lint _typechecksanity pass and_test:smoke --filter=xls-codec.