Skip to content

HF-161: document OFFSET function limitations#1662

Closed
marcin-kordas-hoc wants to merge 2 commits into
handsontable:developfrom
marcin-kordas-hoc:feature/hf-161-offset-limitations
Closed

HF-161: document OFFSET function limitations#1662
marcin-kordas-hoc wants to merge 2 commits into
handsontable:developfrom
marcin-kordas-hoc:feature/hf-161-offset-limitations

Conversation

@marcin-kordas-hoc
Copy link
Copy Markdown
Collaborator

@marcin-kordas-hoc marcin-kordas-hoc commented Apr 28, 2026

Summary

Adds a single canonical ### OFFSET function sub-section under ## Nuances of the implemented functions in docs/guide/known-limitations.md. Documents all six behavioral limits of the OFFSET function in HyperFormula, each backed either by an existing test in unit/parser/offset-translation.spec.ts or by a runtime check captured before this PR was opened.

Removes the now-superseded one-row OFFSET entry from docs/guide/list-of-differences.md (per Kuba's decision in the 2026-04-21 meeting: "można wtedy to stąd też usunąć. Żeby wszystko było tam jednak").

Linked

  • Closes #1572Docs: describe limitations of the OFFSET function
  • Tracks the dynamic-args follow-up: #910
  • Out of scope (separate task): #943 — restructuring known-limitations / list-of-differences / specifications-and-limits pages
  • Spec: agents/hyperformula/docs/specs/2026-04-21-hf-161-offset-limitations.md
  • Tech rationale: agents/hyperformula/docs/specs/2026-04-24-hf-161-tech-rationale.md
  • Implementation plan: agents/hyperformula/docs/specs/2026-04-28-hf-161-offset-docs-plan.md

Limits documented

  1. First argument must be a single-cell reference (passing a range = parser error)
  2. Row/column/height/width arguments must be static integer literals (parser error otherwise)
  3. Height and width must be positive integers
  4. Result outside sheet evaluates to #REF!
  5. getCellFormula returns the resolved reference, not the original =OFFSET(...)
  6. Architectural rationale: OFFSET is resolved at parse time, not at evaluation; OFFSET is registered in FunctionRegistry._protectedPlugins, not as a regular plugin

Runtime verification

All six limits verified runtime against this branch's HEAD before publishing. Excerpt:

A. OFFSET in registered names: true
B. getCellFormula recovers: "=B1" (expected "=B1", NOT "=OFFSET(A1, 0, 1)")
C. Out-of-sheet value: {"value":"#REF!","address":"Sheet1!A1","type":"REF","message":"Resulting reference is out of the sheet."}

=== HF-161 runtime verification — 2026-04-28T02:31:29Z ===
Branch: feature/hf-161-offset-limitations
HEAD SHA: 1852159ec8b4c82107366b18fbd86d27735a452b
Tests in offset-translation.spec.ts: PASSED (24 tests)
Architectural claims A, B, C: PASSED
FunctionRegistry _protectedPlugins entry: present (line 44)

Test references (private repo)

All six limits are covered by test/hyperformula-tests/unit/parser/offset-translation.spec.ts lines 13–206 (24 tests). Run via npm run test:jest -- --testPathPattern="offset-translation".

Test plan

  • CI green on handsontable/hyperformula
  • Manual: npm run docs:dev and visit /guide/known-limitations to verify the new sub-section renders, including the five embedded js code blocks
  • Manual: visit /guide/list-of-differences to confirm the table is intact and the OFFSET row is gone

Notes

  • This is docs-only — no CHANGELOG entry per project convention.
  • The internal ErrorMessage.OutOfSheet string is intentionally NOT quoted verbatim in the docs; the bullet describes the behavior instead, so future internal-string refactors don't break the docs.

Note

Low Risk
Docs-only updates that clarify OFFSET behavior and relocate existing information; no runtime or API changes.

Overview
Adds a canonical ### OFFSET function section to docs/guide/known-limitations.md documenting HyperFormula’s parse-time OFFSET rewriting and the resulting constraints (single-cell first arg, static integer shifts/sizes, positive height/width, out-of-bounds #REF!, and getCellFormula returning the resolved reference).

Removes the now-redundant OFFSET entry from the docs/guide/list-of-differences.md table.

Reviewed by Cursor Bugbot for commit dac40ae. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@qunabu
Copy link
Copy Markdown
Contributor

qunabu commented Apr 28, 2026

@marcin-kordas-hoc
Copy link
Copy Markdown
Collaborator Author

Superseded by #1666.

Same content (same HEAD SHA), but routed from the upstream branch instead of fork. The fork-side PR was hitting a structural CI failure: 3 matrix checks (Test performance, unit-tests, browser-tests) require the DEPLOY_TOKEN secret to clone the private hyperformula-tests repo, and GitHub does not expose org secrets to fork-originated workflow runs. Re-routing to the upstream branch resolves that.

Worktree push routing for future PRs is being fixed locally so this won't repeat.

sequba added a commit that referenced this pull request May 19, 2026
## Summary

Adds a single canonical `### OFFSET function` sub-section under `##
Nuances of the implemented functions` in
`docs/guide/known-limitations.md`. Documents all six behavioral limits
of the OFFSET function in HyperFormula, each backed either by an
existing test in `unit/parser/offset-translation.spec.ts` or by a
runtime check captured before this PR was opened.

Removes the now-superseded one-row OFFSET entry from
`docs/guide/list-of-differences.md` (per Kuba's decision in the
2026-04-21 meeting: *"można wtedy to stąd też usunąć. Żeby wszystko było
tam jednak"*). The HF-vs-Excel/Sheets behavioral differences for OFFSET
are not lost — they remain documented in full in `known-limitations.md`
under the new sub-section, which is the canonical place for parse-time
restrictions per the 04-21 decision to consolidate.

> **Note on PR routing**: this PR replaces #1662 which was opened from a
fork branch. Same content, now from upstream branch — CI will have full
access (no fork-PR DEPLOY_TOKEN issue). Closing #1662 in favor of this
one.

## Linked

- Closes
[#1572](#1572) —
*Docs: describe limitations of the OFFSET function*
- Tracks the dynamic-args follow-up:
[#910](#910)
- Out of scope (separate task):
[#943](#943) —
restructuring `known-limitations` / `list-of-differences` /
`specifications-and-limits` pages
- Unblocked by:
[handsontable/hyperformula-tests#12](handsontable/hyperformula-tests#12)
(merged 2026-05-14, cleared lint regression introduced by #1672)
- Internal spec / tech rationale / implementation plan: tracked in the
team workspace (not committed); summary in this PR description
- Supersedes: #1662

## Limits documented

1. First argument must be a single-cell reference (passing a range =
parser error stored as cell value)
2. Row/column/height/width arguments must be static integer literals
(parser error otherwise)
3. Height and width must be **bare** positive integer literals —
`NUMBER` AST nodes only (unary `+`, parens, non-integers, values <1 all
rejected at parse time)
4. Out-of-sheet target → `#REF!` error stored at parse time (not
evaluation time), with the message *Resulting reference is out of the
sheet*
5. `getCellFormula` returns the resolved reference, not the original
`=OFFSET(...)`
6. Architectural rationale: OFFSET is rewritten at parse time into a
plain cell reference, so introspection via `getCellFormula` shows the
resolved reference rather than the call

## Runtime verification

All six limits were verified against this branch's HEAD before
publishing:

```
A. OFFSET in registered names: false  (correct — OFFSET is parse-time, not registered)
B. getCellFormula recovers: "=B1"     (rewritten reference, NOT "=OFFSET(A1, 0, 1)")
C. Out-of-sheet value: { value: "#REF!", message: "Resulting reference is out of the sheet." }
```

Tests covering all six limits live in
`test/hyperformula-tests/unit/parser/offset-translation.spec.ts` (24
tests, lines 13–206 in the private repo). Run via `npm run test:jest --
--testPathPattern="offset-translation"`.

## Test plan

- [ ] CI green on `handsontable/hyperformula`
- [ ] Netlify deploy preview:
[`/guide/known-limitations`](https://deploy-preview-1666--hyperformula-dev-docs.netlify.app/docs/guide/known-limitations.html)
— verify the new `### OFFSET function` sub-section renders, including
the four embedded `js` code blocks
- [ ] Netlify deploy preview:
[`/guide/list-of-differences`](https://deploy-preview-1666--hyperformula-dev-docs.netlify.app/docs/guide/list-of-differences.html)
— verify the table is intact and the OFFSET row is gone

## Notes

- This is docs-only — no CHANGELOG entry per project convention.
- The internal `ErrorMessage.OutOfSheet` string is intentionally NOT
quoted verbatim in the docs; the bullet describes the behavior instead,
so future internal-string refactors don't break the docs.
- Post-Codex review (2026-05-14): wording clarified to distinguish
parser-error-as-cell-value vs API exception, and to specify that
height/width accept only bare `NUMBER` literals (unary `+` etc.
rejected).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk docs-only change; the main risk is confusing users if the
newly documented OFFSET constraints are inaccurate or drift from
implementation.
> 
> **Overview**
> Adds a canonical **`### OFFSET function`** section to
`docs/guide/known-limitations.md` describing HyperFormula’s parse-time
rewriting behavior and the resulting constraints (single-cell first arg,
static integer shifts/sizes, strict positive literal height/width,
out-of-sheet `#REF!` at parse time, and `getCellFormula` returning the
resolved reference), with small JS snippets.
> 
> Removes the now-redundant `OFFSET` row from
`docs/guide/list-of-differences.md` to consolidate documentation in one
place.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
67ad2cd. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Kuba Sekowski <jakub.sekowski@handsontable.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants