test(react): pin the copy/paste column-space coupling while grouped - #485
Merged
Conversation
The derived group column sits in paste's target space: `mapPasteToTargets` filters only `ROW_SELECT_COLUMN_ID`, so `__pretable_group__` occupies a slot, takes the block's first value when the anchor lands at column 0, and reports it back as a `not-editable` rejection. That reads like an oversight and isn't. Copy and CSV both emit a field for the same column — the group label on a group row, an empty field on a data row — so the slot is what keeps the two sides counting across the same column space. Dropping it from paste alone shifts every value one column right on the way back in: a whole-row copy of `\tr0a\tr0b\tr0c\tr0d` pasted onto another row writes `a=""` (blanking a real column), `b=r0a`, `c=r0b`, `d=r0c`, and clips `r0d`. Silent and destructive, where today's cost is a cosmetic rejection entry. Nothing pinned that coupling: all 116 existing clipboard tests pass with the group column excluded from `mapPasteToTargets`. This test round-trips a whole grouped row through `serializeRanges` -> `parseTsv` -> `mapPasteToTargets` and asserts every value lands back in the column it came from. It holds under either arrangement — group column in both spaces or in neither — and fails only when the two disagree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
blove
enabled auto-merge (squash)
August 24, 2026 17:36
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Vercel preview readyPreview: https://pretable-nrr810fiy-cacheplane.vercel.app Updated automatically by the |
This was referenced Aug 25, 2026
blove
added a commit
that referenced
this pull request
Aug 27, 2026
* docs: clipboard is a spreadsheet interchange format * fix(react): the clipboard carries only real data columns Pasting from Excel into a grouped grid lost the user's first column. The synthetic `__pretable_group__` column held a paste slot, so N values for N visible columns tiled across N+1 targets: the first landed in a column nothing can be written to, and every other value shifted one right. The clipboard is a spreadsheet interchange format, not an internal round-trip format. Both sides now filter the same predicate (`isSyntheticColumnId`), and a group row puts its label in the leftmost column of the copied range with aggregates in their own — the shape Excel's Subtotal and Sheets' pivot output produce. Both sides together, deliberately: #485 measured that removing paste's slot alone silently blanks column `a` on a copy-then-paste and shifts the rest right. `paste-map.test.ts` guards the coupling and its range is now bounded on the row-select column, so it catches a one-sided change to either emitter. Also re-anchors a range bound or paste anchor that lands on the group column to the first data column, in copy, paste, and the surface's anchor resolver — without it a whole-row grouped selection measured one column wider than anything could be written to and announced a phantom clip. Ungrouped output is byte-identical: proven against the pre-change modules over 800 copy bound/row combinations, the CSV option matrix, and the paste anchor/shape matrix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(website): grouped clipboard e2e now expects the spreadsheet shape The synthetic group column no longer occupies a clipboard field, so a grouped copy is N fields wide, not N+1. Data rows lose the empty lead; group rows keep their label, now in the leftmost real column. These pinned the pre-change shape and are the CI half of the same both-sides change — the website Playwright smoke suite is not part of `pnpm test`, so it passed locally and failed against the preview. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I was asked to fix, and why I didn't
The scoped defect was: the derived group column is not excluded from paste's column space —
mapPasteToTargetsfilters onlyROW_SELECT_COLUMN_ID(packages/react/src/paste.ts:316), so__pretable_group__occupies a slot no consumer column occupies.Both predicted manifestations are real and reachable:
ROW_SELECT_COLUMN_IDis guarded in itsonClick). A plain click focuses it (data-pretable-focused="true"), and a paste there sends block column 0 into__pretable_group__:rejected: ["r1:__pretable_group__=x (not-editable)"],cells: ["r1:name=y"].But the inclusion is load-bearing, not an oversight.
copy.ts:327andcsv.ts:523both deliberately emit a field for the same derived column — the group label on a group row, an empty field on a data row (its definition isvalue: () => ""). Verified directly: a whole grouped row serializes to"\tr0a\tr0b\tr0c\tr0d". That leading empty field is what the paste slot lines up with.Excluding the group column from paste alone therefore breaks the copy-rows → paste-rows round trip, destructively. Measured by mutating
paste.tsexactly as scoped:ais silently blanked with copy's empty group field and every value shifts one column right. Today's cost is a cosmeticrejectedentry naming a column the consumer can recognize (GROUP_COLUMN_IDis public API). Trading a benign rejection for a silent wrong write is the wrong direction, so I did not ship the fix.What this PR does
Nothing pinned that coupling — all 116 existing clipboard tests pass with the group column excluded from
mapPasteToTargets, which is why this was scoped as a one-line fix in the first place. This adds the missing guard.The test round-trips a whole grouped row through
serializeRanges→parseTsv→mapPasteToTargetson the same drawn-column array the surface passes to both, and asserts every value lands back in the column it came from. It is written against the invariant, not the current choice: it passes whether the group column is in both column spaces or neither, and fails only when the two disagree.The real trade, left open
The two sides genuinely conflict and the resolution is a product decision, not a bug fix:
__pretable_group__and the rest shift right.The only arrangement that satisfies both is coherent change on both sides: stop copy/CSV emitting a standalone field for the derived group column (e.g. move the group label into the first data column), then drop the slot from paste. That changes documented copy behavior (
clipboard.mdx:44-47) and would lose group labels from a select-all copy unless the label is rehomed — worth a brainstorm, not a drive-by.Gate
pnpm build,pnpm typecheck,pnpm lint,pnpm format,pnpm test(every package ran; react 1247 passed),pnpm api:check— all exit 0.One snag worth knowing:
pnpm buildfailed twice withTS2307: Cannot find module '@pretable/core'frompackages/renderer-domuntilpnpm installrelinked it — the worktree'spackages/renderer-dom/node_moduleshad no@pretabledirectory at all. Not caused by this branch.🤖 Generated with Claude Code