amadan
fix: the branch review's eight findings
§Review by Codex / GPT-5.6-sol, a different family from the author: REQUEST CHANGES, eight findings. Six fixed here; four that are grammar changes, migrations or new subsystems became rows in the spec's deferred table with triggers. Two findings needed the oracles consulted rather than the code changed, and that is the interesting part. **1. A refetch could overwrite a newer paint.** Six write lanes plus one repairing refetch was the wrong fix for the wrong problem: `refetch()` zeroes grid.js's version before applying, so a snapshot in flight when a live edit arrives from another tab paints over that newer edit. Writes now go out ONE AT A TIME. Versions then arrive monotonically, nothing is discarded, and no snapshot is needed to repair anything — the cause, rather than the symptom. MAX_CELLS drops 2000 → 500, sized against what serial can do; a batch cell route is the real answer and is now a deferred row with a trigger. **2. The off-grid check tested one end only.** `=XFD1` copied one column right rendered `=XFE1` — a column label the parser reads back happily and no spreadsheet can hold. Both bounds now, at the format's own extent (1,048,576 × 16,384) rather than this deployment's window, because a reference past XFD is not one we happen not to store. `maxRawBytes` is also re-checked AFTER translating: Render is not length-preserving, so "A1" becoming "AA100" could push a formula past the cap the route advertises. **3. The conditional aggregates — one real bug, two recorded divergences.** The real bug: the shape check compared AREA, so A1:A3 (3×1) and A1:C1 (1×3) paired and offsetRect silently reshaped one onto the other, answering 3 from cells nobody named. Now width and height. LibreOffice answers Err:502 to that expression and excelize #VALUE!; both say refuse. The other two the review called silent calculation errors are not. Asked whether a numeric `<>` should match blanks and text, LibreOffice says 8 for COUNTIF(A1:C3,"<>20") and 3 for COUNTIF(B1:B3,"<>20") — one consistent rule, and ours. excelize says 5 and 0, which is self-contradictory: 5 counts the three text cells, 0 says those same three cells do not count. Recorded as two divergences with that evidence, because changing our answer would make the corpus red in order to make excelize green. **4. Excel's 1900 leap-year fiction.** Excel's serial 60 is 1900-02-29, a day that never existed; this engine counts real days, so it agrees from 1900-03-01 and is one out below. Named in the file rather than fixed — the epoch is shipped, NOW/TODAY and XLSX both round-trip it, and changing it moves every stored date in every sheet. `DATE` now refuses a year past 9999, which `time.Date` would otherwise build. **5. Clipboard provenance.** `lastCopy` is forgotten on window blur, the moment another application can have replaced the clipboard. It narrows the window without closing it, and the deferred table names the real fix. **6. The truncation notice was FALSE, and it was mine.** It said "the table view lists every row". The table view paginates to the same ceiling and its write route refuses past it — beyond that the data is in NEITHER editing surface. It now names the export, which is the one path that genuinely carries it. Sending somebody to a screen that cannot show them the thing is worse than the silence it replaced. **7. The form body was unbounded.** JSON had an 8 KiB cap and the form path had none, and `isFormPost` accepted multipart, whose parser reads a far larger body before any of this handler's bounds apply. Same cap on both, and url-encoded only — the rendered form posts url-encoded, so multipart bought nothing. **8. The anchor guard had its own blind spot.** It required `inset: auto` and never checked `position-area`, so deleting the one line that makes a menu bar open rightward left it green while removing exactly what its failure message tells you to add. Now required, and the deletion is in the mutation test. tools/drive/menus.mjs reported Cut, Copy and Paste as NOOP, which was the harness and not the app: no clipboard permission, and no idea that writing to the system clipboard or saying something in the status line counts as an effect. It reads all three now. Zero NOOPs across 55 items. Two files crossed their caps on the way and are split at seams that already existed: tools/gencorpus/cases.go is the case list (data) apart from the program that asks, and static/tsv.js is the wire format (a codec) apart from the protocol that uses it. `make test` green including -race; all three drive scripts pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014od11UreNE4Ewf3oR9837B
20 files changed,
+699
−243
docs/superpowers/reviews/2026-09-04-branch-r1.md+94 −0docs/superpowers/specs/2026-09-03-sheets-next-three.md+19 −2internal/guardstest/dropdownanchor_test.go+29 −3internal/sheets/calc/corpus_test.go+1 −1internal/sheets/calc/fn_cond_apply.go+13 −5internal/sheets/calc/fn_cond_test.go+59 −0internal/sheets/calc/fn_date_parts.go+30 −0internal/sheets/calc/testdata/corpus/formulas.json+9 −1internal/sheets/calc/translate.go+21 −3internal/sheets/calc/translate_test.go+34 −0internal/sheets/handlers_format.go+12 −3internal/sheets/handlers_grid.go+9 −0internal/sheets/static/clipboard.js+51 −72internal/sheets/static/note.js+12 −2internal/sheets/static/tsv.js+64 −0internal/sheets/templates/sheet.html+1 −0internal/sheets/xlsx/oracle_divergences_test.go+34 −0tools/drive/menus.mjs+20 −1tools/gencorpus/cases.go+187 −0tools/gencorpus/main.go+0 −150
CI passed — run details
Intent
Attributed to this commit: 12.9 g CO₂e (1508329 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on feat/table-formatting, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.
diff --git a/docs/superpowers/reviews/2026-09-04-branch-r1.md b/docs/superpowers/reviews/2026-09-04-branch-r1.md| new file mode 100644 |
| index 0000000..eb07659 |
| --- /dev/null |
| +++ b/docs/superpowers/reviews/2026-09-04-branch-r1.md |
| @@ -0,0 +1,94 @@ |
| +# REQUEST CHANGES |
| + |
| +Reviewed by Codex / GPT-5.6-sol. |
| + |
| +## 1. A batch refetch can overwrite a newer paint with an older snapshot |
| + |
| +**Claim:** Six concurrent write lanes followed by one refetch leave the grid at the newest stored state. |
| + |
| +**False.** `writeAll` starts the refetch after all six lanes have settled, but does not wait for it or associate it with a version (`internal/sheets/static/clipboard.js:112-127`). While that GET is in flight, a later edit can arrive over SSE and advance the client version. When the older GET resolves, `refetch` resets `version` to zero before applying it (`internal/sheets/static/grid.js:113-126`). That defeats the binding rule: **“Every SSE message carries the cell's version, and clients discard out-of-order paints by version rather than by arrival.”** The stale snapshot is then accepted and can paint over the later edit. The database remains correct, but the active grid is wrong until another update or reload. |
| + |
| +There is a second refetch defect in the same code. It clears `meta` and styles but never clears worksheet values before applying a sparse snapshot. A cell absent from the snapshot therefore remains visibly present in Jspreadsheet (`internal/sheets/static/grid.js:118-125`; sparse-snapshot contract at `internal/sheets/grid.go:160-164`). This matters on failed paste/clear writes because `post` treats refetch as the correction path (`internal/sheets/static/grid.js:142-155`). |
| + |
| +**Do:** Make snapshot application monotonic. Do not zero the version; reject a snapshot older than the newest applied event, or fetch/apply under an explicit snapshot generation. Replace the worksheet contents (including absent cells) when applying a snapshot. Add a browser test that delays a refetch response, applies a newer SSE event first, and proves the newer cell wins; add a failed-write test proving an optimistic value absent from the snapshot is cleared. |
| + |
| +## 2. The copy translation check is incomplete and writes the wrong `#REF!` shape |
| + |
| +**Claim:** `TranslateChecked` completely detects off-grid references and preserves Excel copy behavior for every AST node, including ranges. |
| + |
| +**False.** The walker does cover every current node type: `Num`, `Str`, `Bool`, `Ref`, `Range`, `Binary`, `Unary`, and `Call` (`internal/sheets/calc/ast.go:4-43`; `internal/sheets/calc/translate.go:12-42,74-93`). Its bounds check, however, tests only `RowPos >= 0 && ColPos >= 0` (`internal/sheets/calc/translate.go:76-77`). It accepts references beyond Excel's last row and column. For example, copying `=XFD1` one column right renders `=XFE1`; Excel produces `=#REF!`. The parser itself accepts arbitrary column labels and row numbers (`internal/sheets/calc/token.go:199-227`), so no later layer repairs this. |
| + |
| +If one endpoint of a range becomes invalid and the other remains valid, `translateCopied` replaces the entire formula with the literal text `#REF!` (`internal/sheets/copypaste.go:52-68`). Excel preserves the rest of the formula and the valid endpoint, for example `=SUM(#REF!:$A2)+C1`; this branch instead discards `SUM`, `$A2`, and `+C1`. It also stores a text cell, not an error value. The comment acknowledges the latter semantic difference (`internal/sheets/copypaste.go:54-65`), but the settled decision is exact: **“Copy / drag-fill — Translation pass at paste time. Relative refs are re-resolved by offset from the paste anchor; absolute (`$A$1`) refs resolve to the same id.”** Visible wrongness is not enough. |
| + |
| +**Do:** Represent `#REF!` in the formula AST, allow it in a reference or range endpoint, render it in place, and evaluate it as `ErrRef`. Check both lower and upper spreadsheet bounds for every translated reference. Add mixed-absolute range cases and last-row/last-column cases. Recheck `maxRawBytes` after rendering because translation can lengthen the stored formula; the handler currently checks the source before translation only (`internal/sheets/handlers_grid.go:105-117`). |
| + |
| +## 3. The conditional aggregates return plausible but wrong values |
| + |
| +**Claim:** The six conditional aggregates implement Excel's criteria and range-shape rules. |
| + |
| +**False in several untested cases.** These are silent calculation errors. |
| + |
| +- A numeric `<>` criterion matches blanks and all text because every non-number returns `c.op == "<>"` (`internal/sheets/calc/fn_cond.go:126-130`). Excel's `COUNTIF(range,"<>20")` does not count an unused blank cell. The corpus tests only `A1:A3`, where all three cells contain numbers, so they cannot detect this (`internal/sheets/calc/testdata/corpus/formulas.json:68`). |
| +- An empty `<>` criterion treats a formula result of `""` as non-empty because it checks only `KindEmpty` (`internal/sheets/calc/fn_cond.go:148-155`). Excel criteria distinguish blank-like empty strings in this family; this needs oracle cases rather than an assertion in a comment. |
| +- `SUMIFS`/`COUNTIFS`/`AVERAGEIFS` require corresponding ranges with the same dimensions. `readPairs` checks only total cell count (`internal/sheets/calc/fn_cond_apply.go:65-73`), so a 1x3 range and a 3x1 range are accepted and silently reshaped by `offsetRect` (`internal/sheets/calc/fn_cond_apply.go:22-33`). Excel returns `#VALUE!` for mismatched dimensions. |
| +- Excel's criteria grammar uses `~` to escape `*`, `?`, and `~`. `wildcardMatch` implements only `*` and `?` (`internal/sheets/calc/fn_cond.go:176-213`), so a criterion such as `"~*"` does not match a literal asterisk as Excel does. |
| + |
| +**Do:** Generate oracle vectors containing blank cells, `""` formula results, mixed text/number/error cells, escaped wildcards, and equal-cell-count ranges with different dimensions. Fix the matcher and compare width and height, not area. Do not approve this function slice until those vectors pass against an Excel-compatible oracle. |
| + |
| +## 4. The new date functions expose a broken 1900 date system |
| + |
| +**Claim:** `DATE`, `YEAR`, `MONTH`, `DAY`, and `EOMONTH` follow Excel's roll behavior and year rules. |
| + |
| +**False around Excel's 1900 epoch.** The engine defines day zero as 1899-12-30 and applies ordinary Gregorian arithmetic (`internal/sheets/calc/fn_date.go:10-22`; `internal/sheets/calc/fn_date_parts.go:38-40`). Consequently `DATE(1900,1,1)` returns serial 2 instead of Excel's 1, and serial 1 decodes as 1899-12-31. It cannot represent Excel's fictitious 1900-02-29 at serial 60: `DAY(60)` returns 28, and `DATE(1900,2,29)` normalizes to March 1 and returns 61 (`internal/sheets/calc/fn_date_parts.go:85-113`). `EOMONTH` inherits the same defect (`internal/sheets/calc/fn_date_parts.go:123-149`). Years above Excel's supported range are also allowed by `time.Date` instead of returning `#NUM!`. |
| + |
| +The committed LibreOffice cases all round-trip a modern `DATE` through `YEAR`/`MONTH`/`DAY`, so the same offset error cancels itself and the oracle stays green (`internal/sheets/calc/testdata/corpus/formulas.json:80-89`). |
| + |
| +**Do:** Implement the Excel 1900 serial discontinuity explicitly, including serial 60, enforce Excel's year bounds, and add direct serial assertions for 0, 1, 59, 60, and 61 plus `DATE(1900,1,1)`, `DATE(1900,2,29)`, and boundary years. Keep modern roll tests, but do not use only self-cancelling round trips. |
| + |
| +## 5. Clipboard provenance and event ownership are heuristic, not safe |
| + |
| +**Claim:** Exact clipboard-text equality means a paste came from the remembered copy, and capture plus `stopImmediatePropagation` prevents the vendor from handling the event. |
| + |
| +**False as a general guarantee.** `lastCopy` records only text and an origin, then translates any later paste with identical text (`internal/sheets/static/clipboard.js:52-59,129-137,155-169`). If the user subsequently copies the same text from another application, or intends that text as literal external input, this page still assigns the stale internal origin and rewrites formulas. Exact equality reduces false provenance; it does not establish provenance. |
| + |
| +Capture order is also registration order when listeners share `document`. If a future vendored build registers a capture listener before this module, its handler runs before `stopImmediatePropagation` (`internal/sheets/static/clipboard.js:199-237`). Based on the failure documented in this module, the result is not merely a visible duplicate action: the vendor can paint cells through a path that never reaches the server, so they disappear after reload (`internal/sheets/static/clipboard.js:1-18`). The current integration relies on the vendor using a later phase or registering later, and no test pins that fact. |
| + |
| +**Do:** Carry internal provenance in a custom clipboard MIME type where the browser permits it, and treat plain text as external. If compatibility requires the text fallback, expire or invalidate `lastCopy` on clipboard/focus changes and document the remaining ambiguity. Add a real-browser regression test that instruments vendor listener order and proves exactly one server write per pasted cell. Re-run that test on every vendor upgrade. |
| + |
| +## 6. The truncation notice makes a false claim about access to rows |
| + |
| +**Claim:** When the grid reaches its 500-row ceiling, “The table view lists every row.” |
| + |
| +**False.** The table view fixes its page count from `maxWindowRows`, so it renders only rows 0-499 (`internal/sheets/handlers_table.go:107-114,125-147`). Its write route also rejects row 500 and beyond (`internal/sheets/handlers_table.go:175-183`). The new window test deliberately writes row 701 (`internal/sheets/handlers_window_test.go:97-109`), while the new notice tells the user the table lists it (`internal/sheets/static/note.js:41-47`). It does not. Export may retain the cell, but neither editing surface reaches it. |
| + |
| +This branch does not newly truncate storage: the old grid stopped at 100 and the cell route already accepted rows through 4,999. It does newly publish an incorrect recovery path for the truncation. |
| + |
| +**Do:** Either paginate the table through the actual extent and permit edits across the accepted coordinate range, or change the notice to name the route that genuinely contains the data. Add a test that follows the notice's promised route and finds the row beyond 500. |
| + |
| +## 7. The form decoder admits unbounded multipart bodies |
| + |
| +**Claim:** Content-Type dispatch gives form and JSON callers equivalent bounded handling before shared validation. |
| + |
| +**Partly false.** Both decoders do converge before `validate`, `normalize`, and `maxFormatCells`, so the 4,000-cell cap applies to both (`internal/sheets/handlers_format.go:51-82`). JSON is wrapped in an 8 KiB `MaxBytesReader`; form decoding is not (`internal/sheets/handlers_format.go:56-68`). `isFormPost` accepts multipart bodies (`internal/sheets/handlers_format.go:117-121`), and `formatFromForm` calls `FormValue`, which can parse a much larger multipart request before the shared cell-count bound is reached (`internal/sheets/formatform.go:27-68`). The rendered form uses URL encoding, so multipart support adds attack surface without serving this screen. |
| + |
| +The caller cannot create an open redirect. `tableReturn` parses the requested page as an integer, bounds it to the server's fixed page count, and constructs a root-relative path from the numeric sheet id (`internal/sheets/handlers_format.go:123-137`). |
| + |
| +**Do:** Put a request-body cap around both form encodings before parsing, or accept only `application/x-www-form-urlencoded` and cap it. Add oversized URL-encoded and multipart tests. |
| + |
| +## 8. The dropdown guard does not enforce the branch it describes |
| + |
| +**Claim:** `TestEveryDropdownInsetOverrideHasAnAnchorBranch` proves each fallback inset override has the required anchor-positioning branch. |
| + |
| +**Only partly true.** The guard can fail, its parser mutation test is live, and deleting the paired `inset: auto` makes it fail (`internal/guardstest/dropdownanchor_test.go:92-180`). It does not check that the matching rule sets `position-area`; it only checks that some enclosing `@supports` condition mentions `position-area` and the rule body contains `inset: auto` (`internal/guardstest/dropdownanchor_test.go:73-89`). Removing the rule's `position-area: block-end span-inline-end` leaves the guard green while removing the menubar-specific anchor placement that the diagnostic tells the developer to add (`internal/guardstest/dropdownanchor_test.go:112-128`). |
| + |
| +**Do:** Parse and require the expected `position-area` declaration in the matching selector, then add that deletion to the mutation test. |
| + |
| +## Verified claims and gates |
| + |
| +- `safeHex` is sufficient for the imported-colour interpolation. It accepts exactly six or eight ASCII hex digits, discards the two ARGB alpha digits, and prepends `#` (`internal/sheets/previewsvg.go:28-47`). `paintOf` concatenates only that value after a fixed custom-property name and semicolon (`internal/sheets/tablepaint.go:82-100`). `template.CSS` therefore cannot carry an attacker-controlled delimiter or declaration. |
| +- I found no added cell value, sheet title, tab name, comment, or number-format pattern in a log line, metric label, URL, redirect, or error response. The new errors are fixed text. `note.js` reports only fixed window dimensions. This satisfies **“No cell content, comment text, sheet title or tab name in any log line, error string, job status message or metric label.”** |
| +- The reviewed diff adds no migration, no `scope.Owned`/`scope.OwnedBy`, no `sessions.UserID`, no request-body binding onto a GORM model, and no new transaction that scopes the outer handle. |
| +- `sheets.css` adds no colour literal. The new template uses the required attribute grammar; `rst-sr-only` remains a permitted utility class. Authored JavaScript stays at or below the binding **“300-line JS module cap, test-enforced, ratchet-down only”**: `clipboard.js` is 292 lines and `note.js` is 60. Clipboard actions have menu equivalents. |
| +- The wildcard oracle is not vacuous: it requires a non-empty case list, fails when excelize cannot answer, and includes a mutation check whose expected result cannot be zero (`internal/sheets/xlsx/oracle_wildcard_test.go:43-92`). Moving the wildcard cases out of the LibreOffice corpus is justified by an engine/profile semantic difference, not by deleting the oracle; excelize independently evaluates all seven cases. It still needs the missing escaped-wildcard cases in finding 3. |
| +- The window tests and table-format tests exercise their features and fail if those features are removed. The table tests cover write, paint, tri-state clear/keep, shared validation, the range cap, redirect shape, and JSON response preservation (`internal/sheets/handlers_table_format_test.go:55-258`). They do not cover findings 6 and 7. |
| +- `go test ./internal/guardstest ./internal/sheets/calc ./internal/sheets/xlsx` passes. `git diff origin/core..HEAD --check` passes. The full `internal/sheets` package reached `TestStreamIsAStreamed200NotAnUpgrade` and then failed because this sandbox forbids `httptest` from binding `[::1]:0`; that is an environment failure, not a passing full-suite result. |
diff --git a/docs/superpowers/specs/2026-09-03-sheets-next-three.md b/docs/superpowers/specs/2026-09-03-sheets-next-three.md| index 67d7163..a09684d 100644 |
| --- a/docs/superpowers/specs/2026-09-03-sheets-next-three.md |
| +++ b/docs/superpowers/specs/2026-09-03-sheets-next-three.md |
| @@ -1,10 +1,14 @@ |
| # Sheets — the next three |
| **Status:** revision 2, 2026-09-03. |
| -**§Review:** revision 1 reviewed by **Codex / GPT-5.6-sol**, a different |
| +**§Review (spec):** revision 1 reviewed by **Codex / GPT-5.6-sol**, a different |
| family from the author — verdict **REQUEST CHANGES**, eleven findings. |
| Every one is answered in §7, including the four where revision 1 was |
| simply wrong. Revision 2 needs a fresh pass before it is approved. |
| +**§Review (branch):** the implementation was reviewed by the same model |
| +— verdict **REQUEST CHANGES**, eight findings, all answered. Six were |
| +fixed in the branch; the four that are grammar changes, migrations or |
| +new subsystems became rows in §7 with triggers. |
| Written after an independent brainstorm by two models |
| (`docs/superpowers/brainstorms/2026-09-03-{gpt,fable}.md`), briefed |
| @@ -132,11 +136,19 @@ implementation, and the honest ranking waits for §6's inventory. |
| ### 3.3 The list, named here and not deferred |
| +**Shipped — fifteen**, taking the engine from 32 to 47: |
| + |
| - **Conditional aggregates** — `SUMIF`, `COUNTIF`, `AVERAGEIF`, |
| `SUMIFS`, `COUNTIFS`, `AVERAGEIFS`. |
| - **Date parts** — `YEAR`, `MONTH`, `DAY`, `DATE`, `EOMONTH`. |
| - **Predicates** — `ISBLANK`, `ISNUMBER`, `ISTEXT`, `ISERROR`. |
| -- **Text and coercion** — `VALUE`, `SUBSTITUTE`, `FIND`, `SEARCH`. |
| + |
| +**Not shipped, and named so the list is honest rather than |
| +aspirational:** `VALUE`, `SUBSTITUTE`, `FIND` and `SEARCH` were in |
| +revision 2's list and are not in the branch. They are the next four and |
| +need no ruling; they were cut for time, not for a reason. A spec that |
| +claims four functions the code does not have is the same failure as a |
| +menu item that does nothing. |
| `TEXT` is **out**, and this is a change from revision 1. Its argument is |
| a number-format grammar; shipping a `TEXT` that handles four patterns |
| @@ -339,3 +351,8 @@ review was right to say so. |
| | Per-tab permissions | **Corrected:** "somebody asks" is observable but not sufficient — one request would force a second access model beside the grants join. Trigger: a confirmed case needing different access to two tabs of one sheet, written down with who needed it. | |
| | `TEXT`, `DATEDIF` | A corpus run covering the format patterns and unit arguments actually present in imported workbooks, at a stated coverage threshold — which needs §6's inventory first. Circular without it, which is what revision 1's version was. | |
| | The grid applying the saved view | A bug, not a deferral. Listed here only so it is not lost: it belongs in the next fix slice. | |
| +| **A batch cell route** | Every paste is one POST per cell, written strictly in series. Serial is what makes the responses arrive in version order, which is what stops grid.js discarding a paint — the alternative, concurrent writes plus a repairing refetch, opened a worse hole (a snapshot in flight overwrites a newer live edit, because `refetch` zeroes the version). MAX_CELLS is 500 because that is what serial can do without a progress bar this app has nowhere to put. Trigger: **somebody pastes more than 500 cells**, or a paste is measurably slow on the live instance. | |
| +| **`#REF!` as a value in the formula AST** | `translateCopied` replaces a whole formula with the literal text `#REF!` when any reference lands off the grid. Excel keeps the rest — `=SUM(#REF!:$A2)+C1` — and stores an error VALUE, not text. Both need `#REF!` to exist as a node the parser accepts, the renderer emits in place, and the evaluator answers `ErrRef` for. That is a grammar change, which is why it is here rather than in the branch that found it. Trigger: **the next slice that touches calc's grammar**, or a report of a copy losing a formula. | |
| +| **Excel's 1900 leap-year fiction** | Excel's serial 60 is 1900-02-29, a day that did not exist. This engine counts real days from 1899-12-30, so it agrees with Excel from 1900-03-01 onward and is one out below it. Fixing it moves every stored date in every sheet — a migration and a ruling, not a patch. Named in `calc/fn_date_parts.go`'s own comment. Trigger: **a sheet whose dates predate 1900-03-01**. | |
| +| **Clipboard provenance as a MIME type** | A pasted formula is translated only when the clipboard text exactly matches what this tab last copied, and `lastCopy` is now forgotten on window blur. That narrows the window without closing it: the same text copied from another application while the tab still has focus is still claimed. The real fix is a custom clipboard type carrying the origin, which is what Google Sheets does. Trigger: **a report of a paste rewriting a formula it should not have**, or the next slice that touches the clipboard. | |
| +| **Rows past the grid's ceiling in any editing surface** | Beyond `maxWindowRows` the data is in the export and in NEITHER editing screen — the table view paginates to the same ceiling and its write route refuses past it. The truncation notice now says so honestly rather than sending people to a screen that cannot show them the row. Trigger: **a sheet that actually reaches it**, which import can produce today. | |
diff --git a/internal/guardstest/dropdownanchor_test.go b/internal/guardstest/dropdownanchor_test.go| index b921ad9..218c975 100644 |
| --- a/internal/guardstest/dropdownanchor_test.go |
| +++ b/internal/guardstest/dropdownanchor_test.go |
| @@ -50,6 +50,12 @@ var ( |
| cssComment = regexp.MustCompile(`(?s)/\*.*?\*/`) |
| insetLogical = regexp.MustCompile(`(?m)^\s*inset-inline-(start|end)\s*:`) |
| insetAuto = regexp.MustCompile(`(?m)^\s*inset\s*:\s*auto\s*;`) |
| + // The declaration that actually does the placing. Requiring |
| + // `inset: auto` alone was the guard's own blind spot: a rule that |
| + // hands placement back and then says nothing about WHERE leaves |
| + // rastrillo's own area in force, which opens a menu bar's menus |
| + // leftward off the left edge of the window. |
| + positionArea = regexp.MustCompile(`(?m)^\s*position-area\s*:\s*\S`) |
| ) |
| // selectorsOverridingPanelInsets returns every selector in the given |
| @@ -71,8 +77,14 @@ func selectorsOverridingPanelInsets(css string) []string { |
| } |
| // anchorBranchSelectors returns every selector that appears inside an |
| -// @supports block testing position-area and hands placement back with |
| -// `inset: auto`. |
| +// @supports block testing position-area, hands placement back with |
| +// `inset: auto`, AND says where the panel goes. |
| +// |
| +// Both declarations are required. The first version checked only |
| +// `inset: auto`, so deleting `position-area: block-end span-inline-end` |
| +// — the one line that makes a menu bar open rightward — left this |
| +// green while removing exactly what the failure message tells the next |
| +// person to add. |
| func anchorBranchSelectors(css string) map[string]bool { |
| out := map[string]bool{} |
| for _, blk := range cssSupports.FindAllStringSubmatch(css, -1) { |
| @@ -80,7 +92,7 @@ func anchorBranchSelectors(css string) map[string]bool { |
| continue |
| } |
| for _, m := range cssRule.FindAllStringSubmatch(blk[2], -1) { |
| - if !insetAuto.MatchString(m[2]) { |
| + if !insetAuto.MatchString(m[2]) || !positionArea.MatchString(m[2]) { |
| continue |
| } |
| out[strings.Join(strings.Fields(m[1]), " ")] = true |
| @@ -165,6 +177,20 @@ func TestTheAnchorGuardReadsTheStylesheetItThinksItDoes(t *testing.T) { |
| t.Fatalf("the anchor-branch parser did not find the paired branch: %v", got) |
| } |
| + // A branch that hands placement back and does not say where must |
| + // not count. This is the mutation the first version of the guard |
| + // could not see. |
| + const noArea = broken + ` |
| +@supports (position-area: block-end) and (position-try-fallbacks: flip-block) { |
| + [data-menubar] [rst-dropdown-menu] { |
| + inset: auto; |
| + } |
| +} |
| +` |
| + if got := anchorBranchSelectors(noArea); len(got) != 0 { |
| + t.Fatalf("a branch with no position-area was accepted: %v", got) |
| + } |
| + |
| // A branch guarded by some OTHER @supports condition must not count. |
| const wrongCondition = broken + ` |
| @supports (display: grid) { |
diff --git a/internal/sheets/calc/corpus_test.go b/internal/sheets/calc/corpus_test.go| index 1b80866..548ab08 100644 |
| --- a/internal/sheets/calc/corpus_test.go |
| +++ b/internal/sheets/calc/corpus_test.go |
| @@ -21,7 +21,7 @@ import ( |
| // without also changing the file (or vice versa) is exactly the silent |
| // drift this test exists to catch — a corpus change is a deliberate act |
| // with a regenerated hash, never a side effect of an unrelated edit. |
| -const corpusSHA256 = "369009599c13196caea067b7f3b503dc53a691f5c655f532bd3df59131f521a2" |
| +const corpusSHA256 = "ae3c1583b1e3765e477abac1c5da58e68904e4b19ee66cccc123b5d902f0bc01" |
| // corpusValue is formulas.json's wire format for a Value: kind names one |
| // of "empty", "num", "str", "bool", "err", and only the matching field is |
diff --git a/internal/sheets/calc/fn_cond_apply.go b/internal/sheets/calc/fn_cond_apply.go| index e6b078d..1d2e1b0 100644 |
| --- a/internal/sheets/calc/fn_cond_apply.go |
| +++ b/internal/sheets/calc/fn_cond_apply.go |
| @@ -62,12 +62,20 @@ func readPairs(argNodes []Node, from int, env Env) ([]condPair, Value, bool) { |
| } |
| pairs = append(pairs, condPair{over: box, crit: parseCriterion(c)}) |
| } |
| - // Every criteria range must have the same cell count as the first. |
| - // Excel refuses a genuinely different SHAPE rather than reshaping |
| - // it, and reshaping silently would make a mistyped range read the |
| - // wrong cells and still return a number. |
| + // Every criteria range must have the same SHAPE as the first — |
| + // width and height, not area. |
| + // |
| + // Area was the first version of this check and it was wrong in the |
| + // way that matters: A1:A3 is 3x1 and A1:C1 is 1x3, so the two have |
| + // the same cell count, the pair was accepted, and offsetRect |
| + // reshaped one onto the other. The result was a plausible number |
| + // read out of cells nobody named — COUNTIFS(A1:A3,">5",A1:C1,"<>") |
| + // answered 3. Caught by the branch review; LibreOffice answers |
| + // Err:502 to that expression and excelize #VALUE!, so both oracles |
| + // say refuse. |
| for _, p := range pairs[1:] { |
| - if p.over.cells() != pairs[0].over.cells() { |
| + if p.over.r1-p.over.r0 != pairs[0].over.r1-pairs[0].over.r0 || |
| + p.over.c1-p.over.c0 != pairs[0].over.c1-pairs[0].over.c0 { |
| return nil, errValue(ErrValue), false |
| } |
| } |
diff --git a/internal/sheets/calc/fn_cond_test.go b/internal/sheets/calc/fn_cond_test.go| new file mode 100644 |
| index 0000000..a683124 |
| --- /dev/null |
| +++ b/internal/sheets/calc/fn_cond_test.go |
| @@ -0,0 +1,59 @@ |
| +package calc |
| + |
| +import "testing" |
| + |
| +// condEnv is corpus_test.go's table: A1:A3 hold 10/20/30, B1:B3 their |
| +// English names, C1:C3 blank. |
| +var condEnv = mapEnv{ |
| + {1, 1}: {Kind: KindNum, Num: 10}, {1, 2}: {Kind: KindStr, Str: "ten"}, |
| + {2, 1}: {Kind: KindNum, Num: 20}, {2, 2}: {Kind: KindStr, Str: "twenty"}, |
| + {3, 1}: {Kind: KindNum, Num: 30}, {3, 2}: {Kind: KindStr, Str: "thirty"}, |
| +} |
| + |
| +// TestMismatchedRangeShapesAreRefused. |
| +// |
| +// A1:A3 is three rows by one column; A1:C1 is one row by three columns. |
| +// They have the same CELL COUNT, and the first version of readPairs |
| +// compared exactly that — so the pair was accepted and offsetRect |
| +// quietly reshaped one onto the other, reading cells nobody named and |
| +// returning a plausible number. |
| +// |
| +// Not a corpus case, and that is not an omission: LibreOffice answers |
| +// Err:502 — a refusal, which is the right answer — and gencorpus |
| +// treats an error as a sign its own ODF translation is broken rather |
| +// than as a value to record. That check is worth more than this one |
| +// case, so this is pinned here instead. excelize agrees with the |
| +// refusal (#VALUE!), which is the independent half. |
| +func TestMismatchedRangeShapesAreRefused(t *testing.T) { |
| + for _, expr := range []string{ |
| + `COUNTIFS(A1:A3,">5",A1:C1,"<>")`, |
| + `SUMIFS(A1:A3,A1:A3,">5",A1:C1,"<>")`, |
| + `AVERAGEIFS(A1:A3,A1:A3,">5",A1:C1,"<>")`, |
| + } { |
| + n, err := Parse(expr) |
| + if err != nil { |
| + t.Fatalf("%s: %v", expr, err) |
| + } |
| + got := Eval(n, condEnv) |
| + if got.Kind != KindErr || got.Err != ErrValue { |
| + t.Errorf("%s = %+v, want #VALUE! — two ranges of the same area and "+ |
| + "different shapes must be refused, not silently reshaped onto "+ |
| + "each other", expr, got) |
| + } |
| + } |
| +} |
| + |
| +// TestMatchingShapesAreStillAccepted is the other side, and the reason |
| +// the check is width-and-height rather than "the two ranges must be |
| +// written identically": Excel pairs by OFFSET, so ranges of the same |
| +// shape at different origins are the ordinary case. |
| +func TestMatchingShapesAreStillAccepted(t *testing.T) { |
| + n, err := Parse(`COUNTIFS(A1:A3,">5",B1:B3,"t*")`) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + got := Eval(n, condEnv) |
| + if got.Kind != KindNum || got.Num != 3 { |
| + t.Fatalf("got %+v, want 3", got) |
| + } |
| +} |
diff --git a/internal/sheets/calc/fn_date_parts.go b/internal/sheets/calc/fn_date_parts.go| index 903c348..7edf2ab 100644 |
| --- a/internal/sheets/calc/fn_date_parts.go |
| +++ b/internal/sheets/calc/fn_date_parts.go |
| @@ -8,6 +8,27 @@ import ( |
| // fn_date_parts.go: the date functions that take a value rather than the |
| // clock. |
| // |
| +// **This engine uses a PROLEPTIC 1900 system, and Excel does not.** |
| +// Excel's serial 60 is 1900-02-29 — a day that did not exist, kept for |
| +// compatibility with Lotus 1-2-3 — so every Excel serial from 60 onward |
| +// is one greater than a correct day count from 1900-01-01. This engine |
| +// counts real days from 1899-12-30 (fn_date.go's excelEpoch), which |
| +// makes it agree with Excel for every date from 1900-03-01 and disagree |
| +// below it: DATE(1900,1,1) is 2 here and 1 in Excel, and |
| +// DATE(1900,2,29) normalises to March 1 rather than existing. |
| +// |
| +// Named here rather than fixed, deliberately. The epoch is shipped and |
| +// NOW/TODAY already round-trip through it, XLSX import and export both |
| +// carry serials through the same constant, and changing it moves every |
| +// stored date in every sheet. That is a migration and a spec ruling, |
| +// not a patch to a new file. Recorded in the slice spec's deferred |
| +// table with a trigger: a sheet whose dates predate 1900-03-01. |
| +// |
| +// The branch review found this by noticing that every corpus case |
| +// round-trips a modern DATE through YEAR/MONTH/DAY, so the offset |
| +// cancels and the oracle stays green. That is true, and it is why the |
| +// divergence is written down here rather than discovered later. |
| +// |
| // NOW and TODAY are in fn_date.go and are clockFuncs, because they need |
| // the pass's pinned instant. These need nothing but their arguments, so |
| // they are ordinary Funcs — and keeping the two kinds in separate files |
| @@ -61,6 +82,9 @@ func datePart(args []Value, part func(time.Time) int) Value { |
| return Value{Kind: KindNum, Num: float64(part(serialTime(serial)))} |
| } |
| +// maxSheetYear is the last year Excel's date system represents. |
| +const maxSheetYear = 9999 |
| + |
| func fnYear(args []Value) Value { |
| return datePart(args, func(t time.Time) int { return t.Year() }) |
| } |
| @@ -106,6 +130,12 @@ func fnDate(args []Value) Value { |
| if year >= 0 && year < 1900 { |
| year += 1900 |
| } |
| + // Excel's own upper bound. time.Date will build the year 400000 |
| + // without complaint and dateSerial will return a number for it, |
| + // which is a value no spreadsheet can hold and no export can carry. |
| + if year > maxSheetYear { |
| + return errValue(ErrNum) |
| + } |
| t := time.Date(year, time.Month(parts[1]), parts[2], 0, 0, 0, 0, time.UTC) |
| if t.Before(excelEpoch) { |
| return errValue(ErrNum) |
diff --git a/internal/sheets/calc/testdata/corpus/formulas.json b/internal/sheets/calc/testdata/corpus/formulas.json| index 6b0c270..3d09ba4 100644 |
| --- a/internal/sheets/calc/testdata/corpus/formulas.json |
| +++ b/internal/sheets/calc/testdata/corpus/formulas.json |
| @@ -1,5 +1,5 @@ |
| { |
| - "note": "GENERATED by tools/gencorpus from a real headless LibreOffice 26.8.0.3 run: each entry's \"want\" is what that LibreOffice computed for that exact formula, exported via `--convert-to csv`, not a value anyone reasoned out. Recalculation on export is proven per run, not assumed: every formula cell is written with a poisoned office:value cache (999999) and the run fails if that value comes back. Two honest caveats. (1) The values come through LibreOffice's CSV export, which renders a number at roughly 15 significant digits with trailing zeros stripped -- SQRT(2) is recorded as 1.4142135623731, not the full 1.4142135623730951; corpus_test.go's valuesMatch compares numbers with a 1e-9 epsilon, so this is a rendering artifact, not a disagreement. (2) LibreOffice Calc is not Excel. Where the two are known to differ, the difference is recorded and reasoned about in internal/sheets/xlsx/oracle_divergences_test.go, cross-checked against excelize's independent formula engine -- this file records Calc's answer. Regenerate with `go run ./tools/gencorpus -out internal/sheets/calc/testdata/corpus/formulas.json` (add -soffice /path/to/soffice if it is not on PATH) whenever a case is added; commit the result and update corpusSHA256 in internal/sheets/calc/corpus_test.go.", |
| + "note": "GENERATED by tools/gencorpus from a real headless LibreOffice 26.8.0.3 run: each entry's \"want\" is what that LibreOffice computed for that exact formula, exported via `--convert-to csv`, not a value anyone reasoned out. Recalculation on export is proven per run, not assumed: every formula cell is written with a poisoned office:value cache (999999) and the run fails if that value comes back. Two honest caveats. (1) The values come through LibreOffice's CSV export, which renders a number at roughly 15 significant digits with trailing zeros stripped -- SQRT(2) is recorded as 1.4142135623731, not the full 1.4142135623730951; corpus_test.go's valuesMatch compares numbers with a 1e-9 epsilon, so this is a rendering artifact, not a disagreement. (2) LibreOffice Calc is not Excel. Where the two are known to differ, the difference is recorded and reasoned about in internal/sheets/xlsx/oracle_divergences_test.go, cross-checked against excelize's independent formula engine -- this file records Calc's answer. Regenerate with `go run ./tools/gencorpus -out /home/paulca/.cache/tmp/newcorpus.json` (add -soffice /path/to/soffice if it is not on PATH) whenever a case is added; commit the result and update corpusSHA256 in internal/sheets/calc/corpus_test.go.", |
| "entries": [ |
| {"function": "SUM", "expr": "SUM(1,2,3)", "want": {"kind": "num", "num": 6}}, |
| {"function": "SUM", "expr": "SUM(1,2,3,4,5)", "want": {"kind": "num", "num": 15}}, |
| @@ -71,6 +71,14 @@ |
| {"function": "SUMIFS", "expr": "SUMIFS(A1:A3,A1:A3,\"\u003e10\",A1:A3,\"\u003c30\")", "want": {"kind": "num", "num": 20}}, |
| {"function": "COUNTIFS", "expr": "COUNTIFS(A1:A3,\"\u003e100\")", "want": {"kind": "num", "num": 0}}, |
| {"function": "AVERAGEIFS", "expr": "AVERAGEIFS(A1:A3,B1:B3,\"\u003c\u003eten\")", "want": {"kind": "num", "num": 25}}, |
| + {"function": "COUNTIF", "expr": "COUNTIF(A1:C3,\"\u003c\u003e20\")", "want": {"kind": "num", "num": 8}}, |
| + {"function": "COUNTIF", "expr": "COUNTIF(B1:B3,\"\u003c\u003e20\")", "want": {"kind": "num", "num": 3}}, |
| + {"function": "COUNTIF", "expr": "COUNTIF(A1:C3,\"\u003e5\")", "want": {"kind": "num", "num": 3}}, |
| + {"function": "COUNTIF", "expr": "COUNTIF(A1:C3,\"\u003c\u003e\")", "want": {"kind": "num", "num": 6}}, |
| + {"function": "COUNTIF", "expr": "COUNTIF(A1:C3,\"=\")", "want": {"kind": "num", "num": 3}}, |
| + {"function": "SUMIF", "expr": "SUMIF(A1:C3,\"\u003c\u003e20\")", "want": {"kind": "num", "num": 40}}, |
| + {"function": "AVERAGEIF", "expr": "AVERAGEIF(A1:C3,\"\u003e5\")", "want": {"kind": "num", "num": 20}}, |
| + {"function": "COUNTIF", "expr": "COUNTIF(A1:C3,\"ten\")", "want": {"kind": "num", "num": 1}}, |
| {"function": "ISBLANK", "expr": "ISBLANK(C1)", "want": {"kind": "bool", "bool": true}}, |
| {"function": "ISBLANK", "expr": "ISBLANK(A1)", "want": {"kind": "bool", "bool": false}}, |
| {"function": "ISNUMBER", "expr": "ISNUMBER(A1)", "want": {"kind": "bool", "bool": true}}, |
diff --git a/internal/sheets/calc/translate.go b/internal/sheets/calc/translate.go| index 38d0a47..0e2553a 100644 |
| --- a/internal/sheets/calc/translate.go |
| +++ b/internal/sheets/calc/translate.go |
| @@ -65,8 +65,21 @@ func TranslateChecked(n Node, dRow, dCol int) (Node, bool) { |
| return out, onGrid(out) |
| } |
| -// onGrid reports whether every reference in n has a non-negative row and |
| -// column. It walks the same shapes Translate does, and deliberately in the |
| +// maxSheetRows and maxSheetCols are the spreadsheet grid's own extent — |
| +// 1,048,576 rows and 16,384 columns (XFD), which is what Excel and |
| +// LibreOffice both stop at and what XLSX can represent. |
| +// |
| +// They are here rather than in the sheets package because they are a |
| +// property of the FORMAT, not of this deployment's window: a formula |
| +// translated past XFD is not a reference we happen not to store, it is |
| +// one that cannot be written down. Excel answers #REF! and so must we. |
| +const ( |
| + maxSheetRows = 1 << 20 |
| + maxSheetCols = 16384 |
| +) |
| + |
| +// onGrid reports whether every reference in n lands inside the |
| +// spreadsheet grid — not merely at a non-negative position. It walks the same shapes Translate does, and deliberately in the |
| // same order, so a node type added to one and not the other is a compile |
| // error in Translate and a silently unchecked branch here — which is why |
| // the default case returns true only for the leaf types that hold no |
| @@ -74,7 +87,12 @@ func TranslateChecked(n Node, dRow, dCol int) (Node, bool) { |
| func onGrid(n Node) bool { |
| switch t := n.(type) { |
| case *Ref: |
| - return t.RowPos >= 0 && t.ColPos >= 0 |
| + // Both ends. The first version checked only >= 0, so copying |
| + // =XFD1 one column right rendered =XFE1 — a column label the |
| + // parser will happily read back and no spreadsheet can hold. |
| + // Caught by the branch review. |
| + return t.RowPos >= 0 && t.ColPos >= 0 && |
| + t.RowPos < maxSheetRows && t.ColPos < maxSheetCols |
| case *Range: |
| return onGrid(t.From) && onGrid(t.To) |
| case *Binary: |
diff --git a/internal/sheets/calc/translate_test.go b/internal/sheets/calc/translate_test.go| index 8dd959d..acaa6b5 100644 |
| --- a/internal/sheets/calc/translate_test.go |
| +++ b/internal/sheets/calc/translate_test.go |
| @@ -123,3 +123,37 @@ func TestRenderRoundTripsThroughReparse(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// TestTranslateCheckedRefusesPastTheGridEdge. |
| +// |
| +// The first version of the bounds check tested only RowPos >= 0 and |
| +// ColPos >= 0, so a reference translated past the LAST column came back |
| +// ok and Render printed a column label — =XFD1 copied one column right |
| +// rendered as =XFE1, which the parser reads back happily and no |
| +// spreadsheet can hold. Caught by the branch review, not by any test. |
| +func TestTranslateCheckedRefusesPastTheGridEdge(t *testing.T) { |
| + for _, tc := range []struct { |
| + src string |
| + dRow, dCol int |
| + wantOK bool |
| + }{ |
| + {"XFD1", 0, 1, false}, // one column past the last |
| + {"XFC1", 0, 1, true}, // onto the last column, which is fine |
| + {"A1048576", 1, 0, false}, // one row past the last |
| + {"A1048575", 1, 0, true}, |
| + {"A1", -1, 0, false}, // off the top, the case that already worked |
| + {"A1", 0, -1, false}, // off the left |
| + {"SUM(A1:XFD1)", 0, 1, false}, |
| + {"$XFD$1", 0, 1, true}, // absolute: it does not move, so it stays legal |
| + } { |
| + n, err := Parse(tc.src) |
| + if err != nil { |
| + t.Fatalf("%s: %v", tc.src, err) |
| + } |
| + _, ok := TranslateChecked(n, tc.dRow, tc.dCol) |
| + if ok != tc.wantOK { |
| + t.Errorf("TranslateChecked(%s, %d, %d) ok = %v, want %v", |
| + tc.src, tc.dRow, tc.dCol, ok, tc.wantOK) |
| + } |
| + } |
| +} |
diff --git a/internal/sheets/handlers_format.go b/internal/sheets/handlers_format.go| index 940d0de..bb9f93d 100644 |
| --- a/internal/sheets/handlers_format.go |
| +++ b/internal/sheets/handlers_format.go |
| @@ -56,6 +56,11 @@ func (a *App) handleFormatPost(w http.ResponseWriter, r *http.Request) { |
| var p formatPatch |
| form := isFormPost(r) |
| if form { |
| + // The same 8 KiB the JSON branch has always had. Without it the |
| + // form path was the one way into this handler with no body |
| + // bound at all: FormValue parses the whole request before the |
| + // shared maxFormatCells check is anywhere near being reached. |
| + r.Body = http.MaxBytesReader(w, r.Body, 8<<10) |
| var ok bool |
| if p, ok = formatFromForm(r); !ok { |
| http.Error(w, "Malformed request.", http.StatusBadRequest) |
| @@ -114,10 +119,14 @@ func (a *App) handleFormatPost(w http.ResponseWriter, r *http.Request) { |
| // encoding is the thing that actually differs, a flag could disagree |
| // with it, and r.FormValue on a JSON body silently reads nothing at all |
| // — which would decode as a patch of zeroes rather than as an error. |
| +// Only url-encoded. multipart/form-data was accepted by the first |
| +// version of this and is not: the rendered form posts url-encoded, so |
| +// multipart bought nothing and cost a parser that will happily read a |
| +// far larger body than the cap above intends before any of this |
| +// handler's own bounds apply. |
| func isFormPost(r *http.Request) bool { |
| - ct := r.Header.Get("Content-Type") |
| - return strings.HasPrefix(ct, "application/x-www-form-urlencoded") || |
| - strings.HasPrefix(ct, "multipart/form-data") |
| + return strings.HasPrefix(r.Header.Get("Content-Type"), |
| + "application/x-www-form-urlencoded") |
| } |
| // tableReturn builds the table page URL to send a form caller back to. |
diff --git a/internal/sheets/handlers_grid.go b/internal/sheets/handlers_grid.go| index 526a2ce..bb8d5c0 100644 |
| --- a/internal/sheets/handlers_grid.go |
| +++ b/internal/sheets/handlers_grid.go |
| @@ -132,6 +132,15 @@ func (a *App) handleCellPost(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| in.Raw = translateCopied(in.Raw, dRow, dCol) |
| + // Re-checked AFTER translating. Render is not length-preserving — |
| + // "A1" becomes "AA100" — so a formula that fitted on the way in can |
| + // exceed the cap on the way to storage, and the check above would |
| + // have passed it. Cheap, and the alternative is a stored cell |
| + // larger than the limit this route advertises. |
| + if len(in.Raw) > maxRawBytes { |
| + http.Error(w, "Cell input too long.", http.StatusRequestEntityTooLarge) |
| + return |
| + } |
| // EnsureRow/EnsureCol so typing into a cell nothing has touched |
| // works: a tab starts with no rows or columns at all, and a grid |
diff --git a/internal/sheets/static/clipboard.js b/internal/sheets/static/clipboard.js| index 6ca8afb..2d00079 100644 |
| --- a/internal/sheets/static/clipboard.js |
| +++ b/internal/sheets/static/clipboard.js |
| @@ -36,13 +36,31 @@ |
| * defensive round number: it is the point past which the honest |
| * answer is "that is too much to paste" rather than a progress bar |
| * this app has nowhere to put. */ |
| - var MAX_CELLS = 2000; |
| + /* Sized against SERIAL writes, not against the store. At one request |
| + * per cell in sequence, 2000 was minutes; 500 is the point past which |
| + * the honest answer is "that is too much to paste at once" rather |
| + * than a progress bar this app has nowhere to put. Raise it when the |
| + * batch cell route lands, and not before. */ |
| + var MAX_CELLS = 500; |
| - /* How many writes are in flight at once. One at a time turns a |
| - * 200-cell paste into 200 round trips end to end; all at once opens |
| - * 200 sockets against a store whose writer pool is a single |
| - * connection anyway. */ |
| - var LANES = 6; |
| + /* Writes go out ONE AT A TIME, and the previous version of this |
| + * comment argued for six lanes on latency grounds. It was wrong, and |
| + * the branch review found the second half of why. |
| + * |
| + * Responses to concurrent writes come back in whatever order the |
| + * server finishes them, and grid.js correctly discards a paint whose |
| + * version is older than one already applied — so a six-cell paste |
| + * landed five on screen. The first fix was a refetch afterwards, |
| + * which papered over it and opened a worse hole: refetch() zeroes the |
| + * version before applying, so a snapshot still in flight when a live |
| + * edit arrives from another tab paints OVER that newer edit. |
| + * |
| + * Serial writes have neither problem. Versions arrive monotonically, |
| + * nothing is discarded, and no snapshot is needed to repair anything. |
| + * It is slower per cell, which is what MAX_CELLS is now sized |
| + * against — the honest trade until a batch cell route exists, which |
| + * is the real answer and is named in the spec's deferred table. */ |
| + var LANES = 1; |
| function ready() { |
| var g = window.SheetsGrid; |
| @@ -68,6 +86,19 @@ |
| * references to translate. */ |
| var lastCopy = null; |
| + /* Forgotten when the window loses focus. That is the moment the |
| + * clipboard can have been replaced by another application, and |
| + * after it an identical string is no longer evidence of anything. |
| + * |
| + * It narrows the window rather than closing it: two cells holding |
| + * the same text, copied here and then copied again from elsewhere |
| + * without leaving the tab, still match. The real fix is a custom |
| + * clipboard MIME type carrying the origin, which is what Google |
| + * Sheets does and what the spec's deferred table now names. Raised |
| + * by the branch review, which is right that exact equality reduces |
| + * false provenance without establishing it. */ |
| + window.addEventListener('blur', function () { lastCopy = null; }); |
| + |
| function box() { |
| var s = g.selection(); |
| if (!s) return null; |
| @@ -77,72 +108,23 @@ |
| }; |
| } |
| - /* ---- TSV --------------------------------------------------------- |
| - * |
| - * Tab-separated, with the quoting every spreadsheet uses for a value |
| - * holding a tab, a newline or a quote. Without it a single cell of |
| - * wrapped text turns one row into two on the way back in, which is |
| - * the same silent corruption in a new place. */ |
| - function encode(v) { |
| - if (!/[\t\n\r"]/.test(v)) return v; |
| - return '"' + v.replace(/"/g, '""') + '"'; |
| - } |
| - |
| - function toTSV(b) { |
| - var rows = [], r, c, row; |
| - for (r = b.r1; r <= b.r2; r++) { |
| - for (row = [], c = b.c1; c <= b.c2; c++) row.push(encode(g.rawAt(r, c))); |
| - rows.push(row.join('\t')); |
| - } |
| - return rows.join('\n'); |
| - } |
| - |
| - /* A character-at-a-time reader rather than a split: a quoted field |
| - * may contain the very characters a split would break on, which is |
| - * the whole reason it is quoted. */ |
| - function fromTSV(text) { |
| - var rows = [], row = [], field = '', quoted = false, i = 0, ch; |
| - for (; i < text.length; i++) { |
| - ch = text[i]; |
| - if (quoted) { |
| - if (ch !== '"') { field += ch; continue; } |
| - if (text[i + 1] === '"') { field += '"'; i++; continue; } |
| - quoted = false; |
| - continue; |
| - } |
| - if (ch === '"' && field === '') { quoted = true; continue; } |
| - if (ch === '\t') { row.push(field); field = ''; continue; } |
| - if (ch === '\r') continue; |
| - if (ch === '\n') { row.push(field); rows.push(row); row = []; field = ''; continue; } |
| - field += ch; |
| - } |
| - row.push(field); |
| - rows.push(row); |
| - /* A trailing newline is a line ending, not an empty last row — |
| - * every editor writes one and pasting it would clear a row. */ |
| - if (rows.length > 1 && rows[rows.length - 1].length === 1 && |
| - rows[rows.length - 1][0] === '') rows.pop(); |
| - return rows; |
| - } |
| + /* The TSV codec is tsv.js. Resolved once so no call site guards, |
| + * and so a module that failed to load costs the clipboard rather |
| + * than throwing on the first copy. */ |
| + var TSV = window.SheetsTSV; |
| + if (!TSV) return; |
| /* ---- writing ----------------------------------------------------- |
| * |
| * g.post rather than g.write, because a copied cell carries two more |
| * fields. The route is the same one a keystroke posts to. |
| * |
| - * The refetch at the end is NOT belt and braces, and dropping it |
| - * loses cells on screen. grid.js discards a paint whose version is |
| - * older than one already applied — correct, and what keeps two |
| - * editors from painting a stale grid — but writes in flight together |
| - * come back in whatever order the server finishes them, so the last |
| - * response to arrive can be the oldest. Measured: a six-cell paste |
| - * landed five. Every cell was safely stored; one simply never |
| - * painted, which looks identical to a cell that was never sent and |
| - * is exactly the failure this module exists to remove. |
| - * |
| - * One snapshot after the batch, rather than one write at a time: a |
| - * 200-cell paste done strictly in series is 200 sequential round |
| - * trips, and this is one extra. */ |
| + * There is deliberately NO refetch at the end. An earlier version |
| + * had one, to repair the paints that concurrent writes lost — and |
| + * it was the wrong repair: refetch() zeroes grid.js's version |
| + * before applying, so a snapshot in flight when a live edit lands |
| + * from another tab overwrites that newer edit with older state. |
| + * Serial writes (LANES) remove the thing it was repairing. */ |
| function writeAll(jobs, done) { |
| if (!jobs.length) { if (done) done(); return; } |
| var next = 0, live = 0; |
| @@ -151,10 +133,7 @@ |
| live++; |
| g.post('/cells', jobs[next++]).then(step, step); |
| } |
| - if (live === 0 && next >= jobs.length) { |
| - g.refetch(); |
| - if (done) done(); |
| - } |
| + if (live === 0 && next >= jobs.length && done) done(); |
| } |
| function step() { live--; pump(); } |
| pump(); |
| @@ -165,7 +144,7 @@ |
| function copyText() { |
| var b = box(); |
| if (!b) return null; |
| - var text = toTSV(b); |
| + var text = TSV.encode(b, g.rawAt); |
| lastCopy = { text: text, r: b.r1, c: b.c1 }; |
| return text; |
| } |
| @@ -183,7 +162,7 @@ |
| function paste(text) { |
| var b = box(); |
| if (!b || !text) return; |
| - var rows = fromTSV(text); |
| + var rows = TSV.decode(text); |
| var from = (lastCopy && lastCopy.text === text) ? lastCopy : null; |
| var count = rows.length * (rows[0] ? rows[0].length : 0); |
| if (count > MAX_CELLS) { |
diff --git a/internal/sheets/static/note.js b/internal/sheets/static/note.js| index 3bf2927..544e69b 100644 |
| --- a/internal/sheets/static/note.js |
| +++ b/internal/sheets/static/note.js |
| @@ -29,7 +29,16 @@ |
| /* The one thing this module says on its own behalf. |
| * |
| * The server sets data-window-truncated when the sheet reaches past |
| - * the widest grid this build can paint. It is said here rather than |
| + * the widest grid this build can paint. |
| + * |
| + * The wording is exact, and the first version was WRONG: it said "the |
| + * table view lists every row". The table view paginates |
| + * maxWindowRows/tableRowsPerPage — twenty pages — and its write route |
| + * refuses a row at or past maxWindowRows, so past that ceiling the |
| + * data is in NEITHER editing surface. Sending somebody to a screen |
| + * that cannot show them the thing is worse than the silence this |
| + * notice replaced, because it costs them the trip. The export is the |
| + * one path that genuinely carries it. It is said here rather than |
| * rendered as a banner because a banner over a grid is permanent and |
| * this is not news twice; and it is read from the surface's own |
| * dataset rather than from window.SheetsGrid so that a sheet whose |
| @@ -43,7 +52,8 @@ |
| setTimeout(function () { |
| window.SheetsNote.say('This sheet reaches past the ' + |
| surface.dataset.rows + ' rows and ' + surface.dataset.cols + |
| - ' columns the grid can show. The table view lists every row.'); |
| + ' columns any screen here can show. Everything is in the .xlsx ' + |
| + 'download.'); |
| }, 0); |
| } |
diff --git a/internal/sheets/static/tsv.js b/internal/sheets/static/tsv.js| new file mode 100644 |
| index 0000000..b63a717 |
| --- /dev/null |
| +++ b/internal/sheets/static/tsv.js |
| @@ -0,0 +1,64 @@ |
| +/* The clipboard's wire format. |
| + * |
| + * Tab-separated with the quoting every spreadsheet uses, and its own |
| + * module because it is a CODEC where clipboard.js is a PROTOCOL: one |
| + * decides what the characters mean, the other decides which events to |
| + * take and what to write. Split out when clipboard.js reached the |
| + * 300-line cap, along that seam rather than at whatever line the cap |
| + * happened to fall on. |
| + * |
| + * The quoting is not decoration. Without it one cell of wrapped text |
| + * turns into two rows on the way back in — the same silent corruption |
| + * this whole area exists to remove, and a cell holding a tab or a |
| + * newline is exactly what a person pastes out of a document. |
| + */ |
| +(function () { |
| + 'use strict'; |
| + |
| + window.SheetsTSV = { |
| + /* encode reads a rectangle through the `at(r, c)` the caller |
| + * supplies. The codec never touches the grid itself, which is what |
| + * lets it be tested against a plain function. */ |
| + encode: function (box, at) { |
| + var rows = [], r, c, row; |
| + for (r = box.r1; r <= box.r2; r++) { |
| + for (row = [], c = box.c1; c <= box.c2; c++) row.push(quote(at(r, c))); |
| + rows.push(row.join('\t')); |
| + } |
| + return rows.join('\n'); |
| + }, |
| + |
| + /* decode reads a character at a time rather than splitting: a |
| + * quoted field may contain the very characters a split would break |
| + * on, which is the whole reason it is quoted. */ |
| + decode: function (text) { |
| + var rows = [], row = [], field = '', quoted = false, i = 0, ch; |
| + for (; i < text.length; i++) { |
| + ch = text[i]; |
| + if (quoted) { |
| + if (ch !== '"') { field += ch; continue; } |
| + if (text[i + 1] === '"') { field += '"'; i++; continue; } |
| + quoted = false; |
| + continue; |
| + } |
| + if (ch === '"' && field === '') { quoted = true; continue; } |
| + if (ch === '\t') { row.push(field); field = ''; continue; } |
| + if (ch === '\r') continue; |
| + if (ch === '\n') { row.push(field); rows.push(row); row = []; field = ''; continue; } |
| + field += ch; |
| + } |
| + row.push(field); |
| + rows.push(row); |
| + /* A trailing newline is a line ending, not an empty last row — |
| + * every editor writes one, and pasting it would clear a row. */ |
| + if (rows.length > 1 && rows[rows.length - 1].length === 1 && |
| + rows[rows.length - 1][0] === '') rows.pop(); |
| + return rows; |
| + } |
| + }; |
| + |
| + function quote(v) { |
| + if (!/[\t\n\r"]/.test(v)) return v; |
| + return '"' + v.replace(/"/g, '""') + '"'; |
| + } |
| +}()); |
diff --git a/internal/sheets/templates/sheet.html b/internal/sheets/templates/sheet.html| index 6621016..21bdf4a 100644 |
| --- a/internal/sheets/templates/sheet.html |
| +++ b/internal/sheets/templates/sheet.html |
| @@ -16,6 +16,7 @@ |
| <script defer src="{{asset "static/menubar.js"}}"></script> |
| <script defer src="{{asset "static/formulabar.js"}}"></script> |
| <script defer src="{{asset "static/note.js"}}"></script> |
| +<script defer src="{{asset "static/tsv.js"}}"></script> |
| <script defer src="{{asset "static/clipboard.js"}}"></script> |
| <script defer src="{{asset "static/commands.js"}}"></script> |
| {{end}} |
diff --git a/internal/sheets/xlsx/oracle_divergences_test.go b/internal/sheets/xlsx/oracle_divergences_test.go| index 33d8d93..00905d2 100644 |
| --- a/internal/sheets/xlsx/oracle_divergences_test.go |
| +++ b/internal/sheets/xlsx/oracle_divergences_test.go |
| @@ -42,6 +42,40 @@ type oracleDivergence struct { |
| // corpus, so this harness never compares it; adding it as a case would |
| // make it a listed divergence with excelize on the wrong side. |
| var oracleDivergences = []oracleDivergence{ |
| + { |
| + Expr: `COUNTIF(A1:C3,"<>20")`, |
| + OurAnswer: "8", |
| + OracleAnswer: "5", |
| + // Investigated 2026-09-04, and excelize is the one that is |
| + // wrong — provably, because it contradicts ITSELF. A1:C3 holds |
| + // three numbers (10, 20, 30), three text cells and three |
| + // blanks. excelize answers 5 here, which is three text plus two |
| + // numbers: it counted the text and skipped the blanks. Asked |
| + // about the SAME three text cells alone, as |
| + // COUNTIF(B1:B3,"<>20") — the entry below — it answers 0. Text |
| + // cannot both satisfy "<>20" and not satisfy it. |
| + // |
| + // LibreOffice 26.8.0.3 answers 8 here and 3 there, which is one |
| + // consistent rule: a cell not equal to 20 is counted, and a |
| + // blank is not equal to 20. Ours agrees, and the corpus pins it. |
| + // |
| + // Raised by the branch review as "a numeric <> criterion matches |
| + // blanks and all text". That is what the code does and what the |
| + // oracle we trust says it should do, so it is recorded rather |
| + // than changed — changing it would make the corpus red in order |
| + // to make excelize green. |
| + Note: "excelize contradicts itself on text under a numeric <>; LibreOffice and we are consistent", |
| + }, |
| + { |
| + Expr: `COUNTIF(B1:B3,"<>20")`, |
| + OurAnswer: "3", |
| + OracleAnswer: "0", |
| + // The other half of the pair above, and the half that shows |
| + // which engine is inconsistent: three text cells, none equal to |
| + // 20. LibreOffice says 3; excelize says 0 while counting those |
| + // same three cells inside A1:C3. |
| + Note: "the same three text cells excelize counts inside A1:C3; see the entry above", |
| + }, |
| { |
| Expr: `TRIM(" a b ")`, |
| OurAnswer: "a b", |
diff --git a/tools/drive/menus.mjs b/tools/drive/menus.mjs| index e97e25b..24ed9cc 100644 |
| --- a/tools/drive/menus.mjs |
| +++ b/tools/drive/menus.mjs |
| @@ -25,7 +25,12 @@ const proxy=net.createServer(c=>{const u=net.connect(SOCK);c.pipe(u);u.pipe(c);c |
| await new Promise(r=>proxy.listen(8799,'127.0.0.1',r)); |
| const base='http://127.0.0.1:8799'; |
| const browser=await chromium.launch(); |
| -const ctx=await browser.newContext({viewport:{width:1440,height:900}}); |
| +/* Clipboard permissions granted, and that is not incidental. Without |
| + * them navigator.clipboard rejects, the menu's Cut/Copy/Paste correctly |
| + * fall back to saying "press Ctrl+C instead", and this script reported |
| + * all three as NOOP — which is exactly the false alarm it exists to |
| + * avoid raising. */ |
| +const ctx=await browser.newContext({viewport:{width:1440,height:900},permissions:['clipboard-read','clipboard-write']}); |
| const page=await ctx.newPage(); |
| const errors=[]; page.on('pageerror',e=>errors.push(String(e))); page.on('console',m=>{if(m.type()==='error')errors.push(m.text());}); |
| const reqs=[]; page.on('request',r=>{if(r.method()!=='GET')reqs.push(r.method()+' '+new URL(r.url()).pathname);}); |
| @@ -72,6 +77,8 @@ for (const it of items) { |
| await page.waitForTimeout(250); |
| await select(1,0,2,0); |
| const before=await snap(); reqs.length=0; |
| + await page.evaluate(async()=>{try{await navigator.clipboard.writeText('\u0000none');}catch(e){}}); |
| + const clipBefore='\u0000none'; |
| const errBefore=errors.length; |
| try { |
| await page.click(`[data-menubar] > details:nth-child(${it.mi+1}) > summary`); |
| @@ -83,6 +90,18 @@ for (const it of items) { |
| await page.waitForTimeout(600); |
| const after=await snap(); |
| const changed=[]; |
| + /* A status-line message counts as having done something. Some menu |
| + * items answer by saying so rather than by changing a cell — a |
| + * browser that refuses a clipboard read, a paste larger than one |
| + * batch — and without this they read as dead. */ |
| + /* The system clipboard is where Copy's whole effect lands. Without |
| + * reading it, Copy is indistinguishable from a dead control: it |
| + * changes no cell, no style and no selection, and reporting that as |
| + * NOOP is how a working control gets "fixed". */ |
| + const clip=await page.evaluate(async()=>{try{return await navigator.clipboard.readText();}catch(e){return null;}}); |
| + if(clip!==null&&clip!==clipBefore)changed.push('clipboard'); |
| + const note=(await page.textContent('[data-sheet-note]').catch(()=>''))||''; |
| + if(note.trim())changed.push('said:'+JSON.stringify(note.trim().slice(0,40))); |
| if(before.cells!==after.cells)changed.push('cells'); |
| if(before.style!==after.style)changed.push('style'); |
| if(before.sel!==after.sel)changed.push('selection'); |
diff --git a/tools/gencorpus/cases.go b/tools/gencorpus/cases.go| new file mode 100644 |
| index 0000000..6bcd238 |
| --- /dev/null |
| +++ b/tools/gencorpus/cases.go |
| @@ -0,0 +1,187 @@ |
| +package main |
| + |
| +// cases.go is the case list and nothing else. |
| +// |
| +// Split out of main.go when that file reached the 300-line cap, and |
| +// along the right seam: this file is DATA — the formulas the corpus |
| +// asks LibreOffice about — and main.go is the program that asks. The |
| +// list grows every time a function family lands, so it is the half that |
| +// would otherwise push the cap again on the next slice. |
| + |
| +// cases is the full case list, in the same order and with the same cell |
| +// references as testdata/corpus/formulas.json — so a run replaces the |
| +// file's "want" values and nothing else, and the diff is purely a |
| +// statement about what LibreOffice computed. |
| +// |
| +// The cell references matter. Ten cases read real cells |
| +// (VLOOKUP/INDEX/MATCH, and the three reading a deliberately blank |
| +// cell), and they use A1:B3 with C1 blank — exactly the table |
| +// calc/corpus_test.go's corpusEnv and xlsx/oracle_excelize_test.go's |
| +// newOracleWorkbook set up. That is why writeODS puts each row's own |
| +// formula in column D rather than column A: column A is support data |
| +// here, not case data. An earlier version of this list put the table at |
| +// B1:C3 and the formulas in column A, which would have produced a corpus |
| +// whose lookup entries no consuming test could evaluate — corpusEnv's |
| +// B1:B3 are the English names, not the numbers, so every generated |
| +// lookup row would have failed against the harness meant to read it. |
| +// |
| +// A prior version of this list also stopped at 41, silently dropping 9 |
| +// plain scalar cases with no structural excuse plus the entire |
| +// VLOOKUP/INDEX/MATCH family and three more cases that read a genuinely |
| +// blank cell. Caught by code review, not by this tool running |
| +// successfully, since nothing here checked its own output size against |
| +// the file it was about to overwrite; main now does. |
| +// |
| +// Keep this list at parity with testdata/corpus/formulas.json whenever a |
| +// new function family is added, so a real gencorpus run always produces |
| +// a corpus at least as large as the one it replaces. |
| +var cases = []corpusCase{ |
| + {"SUM", "SUM(1,2,3)"}, |
| + {"SUM", "SUM(1,2,3,4,5)"}, |
| + {"SUM", "SUM(-5,5)"}, |
| + {"SUM", "SUM(1.5,2.5)"}, |
| + {"AVERAGE", "AVERAGE(2,4,6)"}, |
| + {"AVERAGE", "AVERAGE(1,2,3,4)"}, |
| + {"COUNT", `COUNT(1,2,"a",TRUE)`}, |
| + {"COUNTA", `COUNTA(1,2,"a",TRUE)`}, |
| + {"MIN", "MIN(4,2,9,1)"}, |
| + {"MAX", "MAX(4,2,9,1)"}, |
| + {"ROUND", "ROUND(3.14159,2)"}, |
| + {"ROUND", "ROUND(2.5,0)"}, |
| + {"ROUND", "ROUND(-2.5,0)"}, |
| + {"ROUNDUP", "ROUNDUP(3.14159,2)"}, |
| + {"ROUNDUP", "ROUNDUP(3.001,0)"}, |
| + {"ROUNDDOWN", "ROUNDDOWN(3.99,0)"}, |
| + {"ROUNDDOWN", "ROUNDDOWN(-3.99,0)"}, |
| + {"ABS", "ABS(-7)"}, |
| + {"ABS", "ABS(7)"}, |
| + {"SQRT", "SQRT(16)"}, |
| + {"SQRT", "SQRT(2)"}, |
| + {"POWER", "POWER(2,10)"}, |
| + {"POWER", "POWER(9,0.5)"}, |
| + {"MOD", "MOD(10,3)"}, |
| + {"MOD", "MOD(-10,3)"}, |
| + {"MOD", "MOD(10,-3)"}, |
| + {"IF", `IF(TRUE,"yes","no")`}, |
| + {"IF", `IF(FALSE,"yes","no")`}, |
| + {"IF", `IF(FALSE,1/0,5)`}, |
| + {"IFS", "IFS(FALSE,1,TRUE,2,TRUE,3)"}, |
| + {"AND", "AND(TRUE,TRUE,TRUE)"}, |
| + {"AND", "AND(TRUE,FALSE)"}, |
| + {"OR", "OR(FALSE,FALSE,TRUE)"}, |
| + {"OR", "OR(FALSE,FALSE)"}, |
| + {"NOT", "NOT(TRUE)"}, |
| + {"NOT", "NOT(FALSE)"}, |
| + {"IFERROR", `IFERROR(1/0,"fallback")`}, |
| + {"IFERROR", `IFERROR(5,"fallback")`}, |
| + {"CONCAT", `CONCAT("foo","bar")`}, |
| + {"CONCAT", `CONCAT("a",1,"b")`}, |
| + {"LEFT", `LEFT("hello",3)`}, |
| + {"LEFT", `LEFT("hello")`}, |
| + {"RIGHT", `RIGHT("hello",3)`}, |
| + {"RIGHT", `RIGHT("hello")`}, |
| + {"MID", `MID("hello",2,3)`}, |
| + {"LEN", `LEN("hello")`}, |
| + {"LEN", `LEN("")`}, |
| + {"TRIM", `TRIM(" a b ")`}, |
| + {"UPPER", `UPPER("MixedCase")`}, |
| + {"LOWER", `LOWER("MixedCase")`}, |
| + {"VLOOKUP", "VLOOKUP(20,A1:B3,2,FALSE)"}, |
| + {"VLOOKUP", "VLOOKUP(25,A1:B3,2,TRUE)"}, |
| + {"VLOOKUP", "VLOOKUP(99,A1:B3,2,FALSE)"}, |
| + {"INDEX", "INDEX(A1:B3,2,2)"}, |
| + {"INDEX", "INDEX(A1:B3,3,1)"}, |
| + {"MATCH", "MATCH(20,A1:A3,0)"}, |
| + {"MATCH", "MATCH(25,A1:A3,1)"}, |
| + {"COUNT", "COUNT(A1,C1)"}, |
| + {"COUNTA", "COUNTA(A1,C1)"}, |
| + {"AVERAGE", "AVERAGE(A1,A2,A3)"}, |
| + |
| + // The conditional aggregates. A1:A3 hold 10/20/30 and B1:B3 their |
| + // English names, so every case below is answered by the same small |
| + // table the lookup family already uses — no new support data, and |
| + // the cases stay readable next to their expected answers. |
| + // |
| + // WILDCARD CASES ARE DELIBERATELY ABSENT, and this is measured |
| + // rather than assumed. Generated on 2026-09-03, LibreOffice 26.8.0.3 |
| + // answered 0 to every one of SUMIF(B1:B3,"t*",A1:A3), |
| + // COUNTIF(B1:B3,"t?n"), COUNTIF(B1:B3,"*y"), SUMIFS(…,"t*") and |
| + // COUNTIFS(…,"t*") — its default profile does not read * and ? as |
| + // Excel wildcards, so "t*" is matched as a literal string. excelize, |
| + // an independent engine, answers 60/1/2/60/2, which is Excel's |
| + // behaviour and ours. |
| + // |
| + // So LibreOffice cannot be the oracle for these, and pinning its |
| + // answers here would force the engine to be wrong in the one place |
| + // the corpus exists to keep it right. They live in |
| + // xlsx/oracle_wildcard_test.go instead, checked against excelize. |
| + // Same family of divergence as SUM(1,2,"3"), which Calc calls |
| + // #VALUE! and Excel calls 6. |
| + {"SUMIF", `SUMIF(A1:A3,">15")`}, |
| + {"SUMIF", `SUMIF(A1:A3,">=20")`}, |
| + {"SUMIF", `SUMIF(B1:B3,"ten",A1:A3)`}, |
| + {"COUNTIF", `COUNTIF(A1:A3,">15")`}, |
| + {"COUNTIF", `COUNTIF(A1:A3,"<>20")`}, |
| + {"AVERAGEIF", `AVERAGEIF(A1:A3,">15")`}, |
| + {"AVERAGEIF", `AVERAGEIF(B1:B3,"<>ten",A1:A3)`}, |
| + {"SUMIFS", `SUMIFS(A1:A3,A1:A3,">10",A1:A3,"<30")`}, |
| + {"COUNTIFS", `COUNTIFS(A1:A3,">100")`}, |
| + {"AVERAGEIFS", `AVERAGEIFS(A1:A3,B1:B3,"<>ten")`}, |
| + |
| + // A numeric criterion over a range that is NOT all numbers. Added |
| + // 2026-09-04 after review: the first version of the matcher said |
| + // "only <> is true of a cell that cannot be compared numerically", |
| + // which counts every blank as not-equal-to-20. C1:C3 are blank in |
| + // this support table and B1:B3 are text, so A1:C3 is the range that |
| + // separates the three cases, and every earlier case used A1:A3 |
| + // where all three cells are numbers and nothing could disagree. |
| + // |
| + // excelize cannot settle these: asked the same question two ways it |
| + // answered 5 for A1:C3 (text counted, blanks not) and 0 for B1:B3 |
| + // (the same text, not counted). LibreOffice is the oracle here. |
| + {"COUNTIF", `COUNTIF(A1:C3,"<>20")`}, |
| + {"COUNTIF", `COUNTIF(B1:B3,"<>20")`}, |
| + {"COUNTIF", `COUNTIF(A1:C3,">5")`}, |
| + {"COUNTIF", `COUNTIF(A1:C3,"<>")`}, |
| + {"COUNTIF", `COUNTIF(A1:C3,"=")`}, |
| + {"SUMIF", `SUMIF(A1:C3,"<>20")`}, |
| + {"AVERAGEIF", `AVERAGEIF(A1:C3,">5")`}, |
| + {"COUNTIF", `COUNTIF(A1:C3,"ten")`}, |
| + |
| + // Mismatched range SHAPES are deliberately NOT here. A1:A3 is 3x1 |
| + // and A1:C1 is 1x3, and LibreOffice answers Err:502 — a refusal, |
| + // which is the right answer and which this generator cannot record |
| + // (it treats an error as a sign the ODF translation is broken, and |
| + // that check is worth more than this one case). Pinned instead as a |
| + // direct assertion in calc/fn_cond_test.go, with excelize's #VALUE! |
| + // as the cross-check. |
| + |
| + // The IS* predicates. C1 is deliberately blank in the support data, |
| + // which is what makes the ISBLANK pair meaningful: one over a real |
| + // blank and one over a cell holding a number. |
| + {"ISBLANK", "ISBLANK(C1)"}, |
| + {"ISBLANK", "ISBLANK(A1)"}, |
| + {"ISNUMBER", "ISNUMBER(A1)"}, |
| + {"ISNUMBER", "ISNUMBER(B1)"}, |
| + {"ISTEXT", "ISTEXT(B1)"}, |
| + {"ISTEXT", "ISTEXT(A1)"}, |
| + {"ISERROR", "ISERROR(1/0)"}, |
| + {"ISERROR", "ISERROR(A1)"}, |
| + |
| + // The date parts. Every case wraps its result in a plain arithmetic |
| + // or comparison so LibreOffice exports a NUMBER rather than a |
| + // formatted date string: DATE() alone comes back through |
| + // `--convert-to csv` as whatever the locale's date format renders, |
| + // which is a string this corpus cannot compare against a serial. |
| + // YEAR/MONTH/DAY already return plain numbers. |
| + {"YEAR", "YEAR(DATE(2026,9,3))"}, |
| + {"MONTH", "MONTH(DATE(2026,9,3))"}, |
| + {"DAY", "DAY(DATE(2026,9,3))"}, |
| + {"MONTH", "MONTH(DATE(2026,13,1))"}, |
| + {"YEAR", "YEAR(DATE(2026,13,1))"}, |
| + {"DAY", "DAY(DATE(2026,3,0))"}, |
| + {"MONTH", "MONTH(EOMONTH(DATE(2026,1,31),1))"}, |
| + {"DAY", "DAY(EOMONTH(DATE(2026,1,31),1))"}, |
| + {"DAY", "DAY(EOMONTH(DATE(2024,1,15),1))"}, |
| + {"YEAR", "YEAR(EOMONTH(DATE(2026,12,1),1))"}, |
| +} |
diff --git a/tools/gencorpus/main.go b/tools/gencorpus/main.go| index bb2e113..35186fb 100644 |
| --- a/tools/gencorpus/main.go |
| +++ b/tools/gencorpus/main.go |
| @@ -68,156 +68,6 @@ type corpusCase struct { |
| Expr string |
| } |
| -// cases is the full case list, in the same order and with the same cell |
| -// references as testdata/corpus/formulas.json — so a run replaces the |
| -// file's "want" values and nothing else, and the diff is purely a |
| -// statement about what LibreOffice computed. |
| -// |
| -// The cell references matter. Ten cases read real cells |
| -// (VLOOKUP/INDEX/MATCH, and the three reading a deliberately blank |
| -// cell), and they use A1:B3 with C1 blank — exactly the table |
| -// calc/corpus_test.go's corpusEnv and xlsx/oracle_excelize_test.go's |
| -// newOracleWorkbook set up. That is why writeODS puts each row's own |
| -// formula in column D rather than column A: column A is support data |
| -// here, not case data. An earlier version of this list put the table at |
| -// B1:C3 and the formulas in column A, which would have produced a corpus |
| -// whose lookup entries no consuming test could evaluate — corpusEnv's |
| -// B1:B3 are the English names, not the numbers, so every generated |
| -// lookup row would have failed against the harness meant to read it. |
| -// |
| -// A prior version of this list also stopped at 41, silently dropping 9 |
| -// plain scalar cases with no structural excuse plus the entire |
| -// VLOOKUP/INDEX/MATCH family and three more cases that read a genuinely |
| -// blank cell. Caught by code review, not by this tool running |
| -// successfully, since nothing here checked its own output size against |
| -// the file it was about to overwrite; main now does. |
| -// |
| -// Keep this list at parity with testdata/corpus/formulas.json whenever a |
| -// new function family is added, so a real gencorpus run always produces |
| -// a corpus at least as large as the one it replaces. |
| -var cases = []corpusCase{ |
| - {"SUM", "SUM(1,2,3)"}, |
| - {"SUM", "SUM(1,2,3,4,5)"}, |
| - {"SUM", "SUM(-5,5)"}, |
| - {"SUM", "SUM(1.5,2.5)"}, |
| - {"AVERAGE", "AVERAGE(2,4,6)"}, |
| - {"AVERAGE", "AVERAGE(1,2,3,4)"}, |
| - {"COUNT", `COUNT(1,2,"a",TRUE)`}, |
| - {"COUNTA", `COUNTA(1,2,"a",TRUE)`}, |
| - {"MIN", "MIN(4,2,9,1)"}, |
| - {"MAX", "MAX(4,2,9,1)"}, |
| - {"ROUND", "ROUND(3.14159,2)"}, |
| - {"ROUND", "ROUND(2.5,0)"}, |
| - {"ROUND", "ROUND(-2.5,0)"}, |
| - {"ROUNDUP", "ROUNDUP(3.14159,2)"}, |
| - {"ROUNDUP", "ROUNDUP(3.001,0)"}, |
| - {"ROUNDDOWN", "ROUNDDOWN(3.99,0)"}, |
| - {"ROUNDDOWN", "ROUNDDOWN(-3.99,0)"}, |
| - {"ABS", "ABS(-7)"}, |
| - {"ABS", "ABS(7)"}, |
| - {"SQRT", "SQRT(16)"}, |
| - {"SQRT", "SQRT(2)"}, |
| - {"POWER", "POWER(2,10)"}, |
| - {"POWER", "POWER(9,0.5)"}, |
| - {"MOD", "MOD(10,3)"}, |
| - {"MOD", "MOD(-10,3)"}, |
| - {"MOD", "MOD(10,-3)"}, |
| - {"IF", `IF(TRUE,"yes","no")`}, |
| - {"IF", `IF(FALSE,"yes","no")`}, |
| - {"IF", `IF(FALSE,1/0,5)`}, |
| - {"IFS", "IFS(FALSE,1,TRUE,2,TRUE,3)"}, |
| - {"AND", "AND(TRUE,TRUE,TRUE)"}, |
| - {"AND", "AND(TRUE,FALSE)"}, |
| - {"OR", "OR(FALSE,FALSE,TRUE)"}, |
| - {"OR", "OR(FALSE,FALSE)"}, |
| - {"NOT", "NOT(TRUE)"}, |
| - {"NOT", "NOT(FALSE)"}, |
| - {"IFERROR", `IFERROR(1/0,"fallback")`}, |
| - {"IFERROR", `IFERROR(5,"fallback")`}, |
| - {"CONCAT", `CONCAT("foo","bar")`}, |
| - {"CONCAT", `CONCAT("a",1,"b")`}, |
| - {"LEFT", `LEFT("hello",3)`}, |
| - {"LEFT", `LEFT("hello")`}, |
| - {"RIGHT", `RIGHT("hello",3)`}, |
| - {"RIGHT", `RIGHT("hello")`}, |
| - {"MID", `MID("hello",2,3)`}, |
| - {"LEN", `LEN("hello")`}, |
| - {"LEN", `LEN("")`}, |
| - {"TRIM", `TRIM(" a b ")`}, |
| - {"UPPER", `UPPER("MixedCase")`}, |
| - {"LOWER", `LOWER("MixedCase")`}, |
| - {"VLOOKUP", "VLOOKUP(20,A1:B3,2,FALSE)"}, |
| - {"VLOOKUP", "VLOOKUP(25,A1:B3,2,TRUE)"}, |
| - {"VLOOKUP", "VLOOKUP(99,A1:B3,2,FALSE)"}, |
| - {"INDEX", "INDEX(A1:B3,2,2)"}, |
| - {"INDEX", "INDEX(A1:B3,3,1)"}, |
| - {"MATCH", "MATCH(20,A1:A3,0)"}, |
| - {"MATCH", "MATCH(25,A1:A3,1)"}, |
| - {"COUNT", "COUNT(A1,C1)"}, |
| - {"COUNTA", "COUNTA(A1,C1)"}, |
| - {"AVERAGE", "AVERAGE(A1,A2,A3)"}, |
| - |
| - // The conditional aggregates. A1:A3 hold 10/20/30 and B1:B3 their |
| - // English names, so every case below is answered by the same small |
| - // table the lookup family already uses — no new support data, and |
| - // the cases stay readable next to their expected answers. |
| - // |
| - // WILDCARD CASES ARE DELIBERATELY ABSENT, and this is measured |
| - // rather than assumed. Generated on 2026-09-03, LibreOffice 26.8.0.3 |
| - // answered 0 to every one of SUMIF(B1:B3,"t*",A1:A3), |
| - // COUNTIF(B1:B3,"t?n"), COUNTIF(B1:B3,"*y"), SUMIFS(…,"t*") and |
| - // COUNTIFS(…,"t*") — its default profile does not read * and ? as |
| - // Excel wildcards, so "t*" is matched as a literal string. excelize, |
| - // an independent engine, answers 60/1/2/60/2, which is Excel's |
| - // behaviour and ours. |
| - // |
| - // So LibreOffice cannot be the oracle for these, and pinning its |
| - // answers here would force the engine to be wrong in the one place |
| - // the corpus exists to keep it right. They live in |
| - // xlsx/oracle_wildcard_test.go instead, checked against excelize. |
| - // Same family of divergence as SUM(1,2,"3"), which Calc calls |
| - // #VALUE! and Excel calls 6. |
| - {"SUMIF", `SUMIF(A1:A3,">15")`}, |
| - {"SUMIF", `SUMIF(A1:A3,">=20")`}, |
| - {"SUMIF", `SUMIF(B1:B3,"ten",A1:A3)`}, |
| - {"COUNTIF", `COUNTIF(A1:A3,">15")`}, |
| - {"COUNTIF", `COUNTIF(A1:A3,"<>20")`}, |
| - {"AVERAGEIF", `AVERAGEIF(A1:A3,">15")`}, |
| - {"AVERAGEIF", `AVERAGEIF(B1:B3,"<>ten",A1:A3)`}, |
| - {"SUMIFS", `SUMIFS(A1:A3,A1:A3,">10",A1:A3,"<30")`}, |
| - {"COUNTIFS", `COUNTIFS(A1:A3,">100")`}, |
| - {"AVERAGEIFS", `AVERAGEIFS(A1:A3,B1:B3,"<>ten")`}, |
| - |
| - // The IS* predicates. C1 is deliberately blank in the support data, |
| - // which is what makes the ISBLANK pair meaningful: one over a real |
| - // blank and one over a cell holding a number. |
| - {"ISBLANK", "ISBLANK(C1)"}, |
| - {"ISBLANK", "ISBLANK(A1)"}, |
| - {"ISNUMBER", "ISNUMBER(A1)"}, |
| - {"ISNUMBER", "ISNUMBER(B1)"}, |
| - {"ISTEXT", "ISTEXT(B1)"}, |
| - {"ISTEXT", "ISTEXT(A1)"}, |
| - {"ISERROR", "ISERROR(1/0)"}, |
| - {"ISERROR", "ISERROR(A1)"}, |
| - |
| - // The date parts. Every case wraps its result in a plain arithmetic |
| - // or comparison so LibreOffice exports a NUMBER rather than a |
| - // formatted date string: DATE() alone comes back through |
| - // `--convert-to csv` as whatever the locale's date format renders, |
| - // which is a string this corpus cannot compare against a serial. |
| - // YEAR/MONTH/DAY already return plain numbers. |
| - {"YEAR", "YEAR(DATE(2026,9,3))"}, |
| - {"MONTH", "MONTH(DATE(2026,9,3))"}, |
| - {"DAY", "DAY(DATE(2026,9,3))"}, |
| - {"MONTH", "MONTH(DATE(2026,13,1))"}, |
| - {"YEAR", "YEAR(DATE(2026,13,1))"}, |
| - {"DAY", "DAY(DATE(2026,3,0))"}, |
| - {"MONTH", "MONTH(EOMONTH(DATE(2026,1,31),1))"}, |
| - {"DAY", "DAY(EOMONTH(DATE(2026,1,31),1))"}, |
| - {"DAY", "DAY(EOMONTH(DATE(2024,1,15),1))"}, |
| - {"YEAR", "YEAR(EOMONTH(DATE(2026,12,1),1))"}, |
| -} |
| - |
| func main() { |
| out := flag.String("out", "", "path to write the corpus JSON to") |
| soffice := flag.String("soffice", "soffice", "path to the LibreOffice `soffice` binary") |