docs: clarify setRowOrder/setColumnOrder permutation semantics - #1687
Conversation
* Fix package-lock file * Docs: remove CodeSandbox embedded demos and add links to working exa,ples in Stackblitz (#1621)
<!-- CURSOR_SUMMARY --> > [!NOTE] > **Low Risk** > Low risk documentation-only changes: adds new guide pages and adjusts VuePress sidebar navigation with no runtime or API impact. > > **Overview** > Adds three new AI-focused documentation pages: `ai-sdk`, `integration-with-langchain`, and `mcp-server`, describing how to use HyperFormula for deterministic spreadsheet computation in agent workflows. > > Updates the VuePress guide sidebar to surface these pages under **Integrations**, renames the section from *Framework integration* to *Integrations*, and moves the former *Overview* links into a new *About* section. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 54c541b. 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: GreenFlux <support@greenflux.us> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The existing examples for setRowOrder and setColumnOrder used self-inverse permutations like [2, 1, 0] or [0, 3, 2, 1]. These produce the same output under either interpretation of the permutation argument: A. newOrder[i] = new position for the row/column currently at index i B. newOrder[i] = previous position of the row/column ending up at index i The implementation uses interpretation A, but the old examples did not disambiguate, leading users to assume interpretation B (issue #1668). This change replaces the examples with non-self-inverse cyclic shifts ([1, 2, 0] on a 3-element sheet) and adds explicit prose explaining the semantics in both the JSDoc and the guide pages. Updates apply to: - API JSDoc for setRowOrder, isItPossibleToSetRowOrder, setColumnOrder and isItPossibleToSetColumnOrder in src/HyperFormula.ts - docs/guide/sorting-data.md (intro, row and column step-by-step sections, with new warning callouts) - docs/guide/basic-operations.md (Reordering rows and columns subsections) - CHANGELOG.md entry under [Unreleased] Co-authored-by: Kuba Sekowski <sequba@users.noreply.github.com>
✅ Deploy Preview for hyperformula-dev-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Documentation-only changes don't need a CHANGELOG.md entry. Remove the entry added for the setRowOrder docs clarification and add a note to DEV_DOCS.md spelling out the convention so future contributors know. Co-authored-by: Kuba Sekowski <sequba@users.noreply.github.com>
sequba
left a comment
There was a problem hiding this comment.
@cursoragent fix it
Address review feedback: move the note as a sub-bullet under '- changelog entry' instead of a separate section. Co-authored-by: Kuba Sekowski <sequba@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1687 +/- ##
========================================
Coverage 97.16% 97.16%
========================================
Files 176 176
Lines 15322 15322
Branches 3356 3356
========================================
Hits 14887 14887
Misses 427 427
Partials 8 8
🚀 New features to boost your workflow:
|
Performance comparison of head (a189ead) vs base (3064a8b) |
marcin-kordas-hoc
left a comment
There was a problem hiding this comment.
LGTM. I verified every example against a live HyperFormula build:
- The documented semantics (
newOrder[i]= the new position for the row/column currently at indexi) match the implementation (mappingFromOrderyields[i, newOrder[i]], andOperations.setRowOrdermoves the row atsource=itotarget=newOrder[i]). - All guide + JSDoc examples reproduce exactly, including the
changesarray contents and ordering:[A,B,C]+[1,2,0]->[[C],[A],[B]](rows) /[[C,A,B]](columns), and theisItPossible...true/false cases.
Using a non-self-inverse permutation ([1,2,0]) is the right call for disambiguating #1668, and nice cleanups on the side (fixing the bare setRowOrder(...)/setColumnOrder(...) calls, dropping the ragged [4,5] data that produced confusing null changes).
One tiny typo inline.
| - for breaking changes: a section in the migration guide | ||
| - Technical documentation in the form of JSDoc comments (high-level description of the concepts used in more complex code fragments) | ||
| - Changelog entry | ||
| - Changelog entry (not required for documentation-only changes (guides, JSDoc, README, etc.) |
There was a problem hiding this comment.
Unbalanced parentheses — the opening ( before "not required" is never closed; the ) at the end closes the inner (guides, JSDoc, README, etc.).
| - Changelog entry (not required for documentation-only changes (guides, JSDoc, README, etc.) | |
| - Changelog entry (not required for documentation-only changes such as guides, JSDoc, README, etc.) |


Context
Fixes #1668. The reporter ran
setRowOrderexpecting the array argument to mean "previous position for each new row", got the opposite behavior, and was unable to disambiguate from the docs.Root cause: every example in the JSDoc and guide pages used a self-inverse permutation (
[2, 1, 0]on 3 rows,[0, 3, 2, 1]on 4 rows). Self-inverse permutations produce the same output under both of the two natural interpretations of the argument:newOrder[i]= new position for the row/column currently at indexi(this is the actual behavior, verified inmappingFromOrderinsrc/CrudOperations.tsandOperations.setRowOrderinsrc/Operations.ts)newOrder[i]= previous position of the row/column that should end up at indexiSo the examples didn't actually demonstrate which interpretation was correct.
Changes
Doc-only. No runtime behavior changed.
[1, 2, 0]on a 3-element sheet → result[['C'], ['A'], ['B']]). Under interpretation B the result would have been[['B'], ['C'], ['A']], so the example now visibly reinforces the correct semantics.iis the new position for the row/column currently at indexi".::: warningcallout in the sorting guide explicitly contrasting the correct interpretation with the inverse-permutation interpretation that the reporter assumed.setColumnOrder/isItPossibleToSetColumnOrderdocumentation for consistency.DEV_DOCS.mdclarifying that documentation-only PRs do not require aCHANGELOG.mdentry.Files touched:
src/HyperFormula.ts- JSDoc forsetRowOrder,isItPossibleToSetRowOrder,setColumnOrder,isItPossibleToSetColumnOrder.docs/guide/sorting-data.md- intro paragraph, "Sorting rows" and "Sorting columns" step-by-step sections.docs/guide/basic-operations.md- "Reordering rows" and "Reordering columns" subsections.DEV_DOCS.md- new "Documentation-only changes" note under "Definition of Done".Out of scope:
swapRowIndexes/swapColumnIndexesuse[[source, target], ...]pairs and are already described as "array mapping original positions to final positions"; the ambiguity does not apply, so those are untouched.How did you test your changes?
npm run lint- 0 errors (pre-existing warnings in tests are unrelated).npm run compile- clean TypeScript compile.Types of changes
Related issues:
setRowOrderis unclear #1668Checklist: