test(web): assert the overlay focus and record the measured probe counts
Addresses ds-review-bot: the keyboard step now asserts that focus landed on the sheet overlay before pressing arrows, the comment keeps only the local why, and the Agent Note carries the measured counts (10 of 11 master runs, 7 of 38 probe runs, 26 of 26 keyboard runs, 10 of 10 local repetitions).
This commit is contained in:
Tianyi Cui committed
1 parent
979b77a6bc
commit
d7b5b42231
4 files changed
+23
-23
No files matched your search
+8
-8
@@ -6,17 +6,17 @@
|
||||
en: 015053d7e4cc8b74
|
||||
zh: 8afd22975bcb9814
|
||||
/agent-note-web-lane-moves-spreadsheet-selections-by-keyboard-before-reflow-assertions/problem:
|
||||
en: 73050d2c7d3bceeb
|
||||
zh: 68f45fbaa8df5786
|
||||
en: 27ce4e01ca063f29
|
||||
zh: cd8c872750c725bc
|
||||
/agent-note-web-lane-moves-spreadsheet-selections-by-keyboard-before-reflow-assertions/decision:
|
||||
en: 05033995f6accde4
|
||||
zh: 1de1372d99a4dc4b
|
||||
en: 49b7f008e97f05cb
|
||||
zh: 00571e867ba82057
|
||||
/agent-note-web-lane-moves-spreadsheet-selections-by-keyboard-before-reflow-assertions/alternatives-considered:
|
||||
en: 6085e5faea6ec83f
|
||||
zh: f5a790200d7787a9
|
||||
en: 51c1315b1d424915
|
||||
zh: 2f2f537cfdde2469
|
||||
/agent-note-web-lane-moves-spreadsheet-selections-by-keyboard-before-reflow-assertions/verification:
|
||||
en: 4b57417a68d13275
|
||||
zh: e0bbbf54e449eb0c
|
||||
en: 316dbc106810d68f
|
||||
zh: 8771fd22b885e4ab
|
||||
/agent-note-web-lane-moves-spreadsheet-selections-by-keyboard-before-reflow-assertions/consequences:
|
||||
en: 03eedcc3cfdad180
|
||||
zh: 8d4843e01448fe83
|
||||
+5
-5
@@ -6,13 +6,13 @@ English | [中文](2026-09-27-spreadsheet-selection-by-keyboard-before-reflow.zh
|
||||
|
||||
## Problem
|
||||
|
||||
[document-preview.e2e.ts](../../../../apps/web/tests/document-preview.e2e.ts) opens `meeting.xlsx`, clicks the sheet overlay at `{ x: 60, y: 40 }` to select `A1`, then widens and narrows the sidebar four times and asserts after each reflow that the name box still reads `A1`. Every `CI master` run of the serial self-hosted Linux lane between 2026-09-24 and 2026-09-27 failed one or both locale cases of that scenario with `expected 'B1' to be 'A1'` at the first reflow, and hosted `node 24 / snapshots and artifacts` runs failed the same way (runs 36110572677 and 36213089675). The failure is not locale-bound: en-US failed alone in four master runs, both locales in three, zh-CN alone in three. Polling the read (#5243) changed nothing because `B1` is the committed selection, not a stale sample.
|
||||
[document-preview.e2e.ts](../../../../apps/web/tests/document-preview.e2e.ts) opens `meeting.xlsx`, clicks the sheet overlay at `{ x: 60, y: 40 }` to select `A1`, then widens and narrows the sidebar four times and asserts after each reflow that the name box still reads `A1`. Ten of the eleven `CI master` runs of the serial self-hosted Linux lane between 2026-09-24 and 2026-09-27 failed one or both locale cases of that scenario with `expected 'B1' to be 'A1'` at the first reflow, and hosted `node 24 / snapshots and artifacts` runs failed the same way (runs 36110572677 and 36213089675). The failure is not locale-bound: en-US failed alone in four master runs, both locales in three, zh-CN alone in three. Polling the read (#5243) changed nothing because `B1` is the committed selection, not a stale sample.
|
||||
|
||||
`A1` is the workbook's initial selection, so the polls that follow the click are satisfied before the click commits and never verify what the click selected. FortuneSheet's `SheetOverlay` handles the press as `setContext(draft => handleCellAreaMouseDown(draft, globalCache, nativeEvent, cellInput, cellArea, …))`, and `handleCellAreaMouseDown` resolves the cell from `cellArea.getBoundingClientRect()` and the event's page coordinates at the moment the updater runs. React runs `useState` updaters during render and runs them again when the hook queue holds a lower-priority update ahead of them; the freshly mounted workbook still has such updates pending, and Playwright precedes the press with a `mousemove` whose FortuneSheet handler is a continuous-priority update on the same state. An instrumented verbatim copy of the scenario on the Blacksmith runner recorded four executions of `handleCellAreaMouseDown` for the one click: two before the click returned reading `left=969.5`, and two after the scenario had already set the panel to `1000px`, reading `left=725.5`. The stored pointer x of 985 then resolves to 259.5 inside the grid, and because the meeting sheet has two columns `colLocation` clamps it to the last one: `B1`. On a machine that commits the workbook's initial selection normalization before the click, the same point lies on A1's own selection box, whose `onMouseDown` stops propagation; the click then selects nothing, no geometry-bound updater is queued, and the scenario passes without having tested the click.
|
||||
|
||||
## Decision
|
||||
|
||||
The meeting-sheet step selects by keyboard. The scenario focuses the sheet overlay, presses `ArrowRight` and asserts `B1` with an empty formula bar, then presses `ArrowLeft` and asserts `A1` with `会议纪要`, and only then starts the reflow loop. `handleArrowKey` → `moveHighlightCell` computes the target from committed state — the current selection and `visibledatacolumn` — so a re-executed updater yields the same cell whatever the grid's geometry is by then, and the two moves make the interaction observable because `B1` differs from the initial selection. The reflow loop, its `A1` assertion, the sheet-tab scroll-control counts, and the canvas identity check are unchanged.
|
||||
The meeting-sheet step selects by keyboard. The scenario focuses the sheet overlay (`tabindex="-1"`), asserts the focus landed, presses `ArrowRight` and asserts `B1` with an empty formula bar, then presses `ArrowLeft` and asserts `A1` with `会议纪要`, and only then starts the reflow loop. `handleArrowKey` → `moveHighlightCell` computes the target from committed state — the current selection and `visibledatacolumn` — so a re-executed updater yields the same cell whatever the grid's geometry is by then, and the two moves make the interaction observable because `B1` differs from the initial selection. The reflow loop, its `A1` assertion, the sheet-tab scroll-control counts, and the canvas identity check are unchanged.
|
||||
|
||||
The budget-sheet click at the start of the scenario stays a pointer click: it targets a cell other than the initial selection, its result is verified by the `46281` formula read, and keyboard, clipboard, and layout reads separate it from the first reflow by far more than one deferred render. The `values.csv` click followed by a narrowing stays too: a re-execution against the narrowed grid resolves a negative x, which `colLocation` clamps to column A, the same cell.
|
||||
|
||||
@@ -24,13 +24,13 @@ The budget-sheet click at the start of the scenario stays a pointer click: it ta
|
||||
|
||||
**Wait for the renderer to go idle before the first reflow.** No product state names "React has no re-execution pending"; an idle or fixed wait sized to the runner is the flake-masking wait [dsh-ci-test-reliability](../../../skills/dsh-ci-test-reliability/SKILL.md) rejects.
|
||||
|
||||
**Patch FortuneSheet to resolve the cell outside the updater.** `handleCellAreaMouseDown` lives in `@fortune-sheet/core`; snapshotting the rectangle before `setContext` is a dependency patch for a window a user cannot reach — a pointer press and a sidebar reflow inside one deferred render. It stays an upstream concern; the product change is out of this test fix.
|
||||
**Extend the `@fortune-sheet/react` patch to resolve the cell outside the updater.** `handleCellAreaMouseDown` lives in `@fortune-sheet/core`, and the window a user would need to hit is a pointer press and a sidebar reflow inside one deferred render. It stays an upstream concern; the product change is out of this test fix.
|
||||
|
||||
## Verification
|
||||
|
||||
The instrumented verbatim copy of the shipped scenario, run on the hosted Linux lane of draft PR #5291 with `DEBUG=pw:api` and a `getBoundingClientRect` hook on the cell area, reproduced `expected 'B1' to be 'A1'` at the first reflow in 3 of 12 runs across two lane runs, each with the four-execution signature above and with `mousedown` targeting `#luckysheet-sheettable_0` rather than the drag handle. The keyboard-driven copy passed 6 of 6 in the lane run that reproduced the click failure. Locally, `Emulation.setCPUThrottlingRate` at 20× makes the click reach the cell area and records three executions of `handleCellAreaMouseDown` per click; the keyboard-driven copy passes 6 of 6 under that throttle and 6 of 6 with React's scheduler wake-ups deferred by 200 ms.
|
||||
The instrumented verbatim copy of the shipped scenario, run on the hosted Linux lane of PR #5291 while it was a draft, reproduced `expected 'B1' to be 'A1'` at the first reflow in 7 of 38 runs across three lane runs (2 of 6, 1 of 6, 4 of 20). Every failure recorded `mousedown` targeting `#luckysheet-sheettable_0` rather than the drag handle and the `B1` commit preceding the deferred `cellInput.focus()` that only the press updater schedules; the five failures in the two lane runs that also hooked the cell area's `getBoundingClientRect` all carried the four-execution signature above. The keyboard-driven copy ran in those same two lane runs and passed 26 of 26. Locally, `Emulation.setCPUThrottlingRate` at 20× makes the click reach the cell area and records three executions of `handleCellAreaMouseDown` per click; the keyboard-driven copy passes 6 of 6 under that throttle and 6 of 6 with React's scheduler wake-ups deferred by 200 ms.
|
||||
|
||||
`env -u TSX_TSCONFIG_PATH DSH_SNAPSHOT=replay pnpm exec vitest run --config vitest.web.config.ts apps/web/tests/document-preview.e2e.ts -t "fills the spreadsheet pane"` passes both locale cases in every local repetition recorded in the PR.
|
||||
`env -u TSX_TSCONFIG_PATH DSH_SNAPSHOT=replay pnpm exec vitest run --config vitest.web.config.ts apps/web/tests/document-preview.e2e.ts -t "fills the spreadsheet pane"` passed both locale cases in 10 of 10 local repetitions, and the whole file passed its 6 cases in 2 of 3 local runs; the third failed the unrelated `Coding Tools` switch poll at `:849` while documentation gates ran concurrently, with the failure screenshot showing the switch on.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
+5
-5
@@ -6,13 +6,13 @@ Status: implemented
|
||||
|
||||
## Problem
|
||||
|
||||
[document-preview.e2e.ts](../../../../apps/web/tests/document-preview.e2e.ts) 打开 `meeting.xlsx`，在表格 overlay 的 `{ x: 60, y: 40 }` 处点击以选中 `A1`，随后把侧栏加宽、收窄各两次，并在每次重排后断言名称框仍显示 `A1`。2026-09-24 到 2026-09-27 之间 `CI master` 的 serial 自托管 Linux lane 每一次运行都在第一次重排处以 `expected 'B1' to be 'A1'` 失败该场景的一个或两个 locale 用例，托管的 `node 24 / snapshots and artifacts` 也以同样方式失败（run 36110572677 与 36213089675）。失败与 locale 无关：四次 master 运行只有 en-US 失败，三次两者都失败，三次只有 zh-CN 失败。把读取改为轮询（#5243）没有作用，因为 `B1` 是已提交的选区，不是过期采样。
|
||||
[document-preview.e2e.ts](../../../../apps/web/tests/document-preview.e2e.ts) 打开 `meeting.xlsx`，在表格 overlay 的 `{ x: 60, y: 40 }` 处点击以选中 `A1`，随后把侧栏加宽、收窄各两次，并在每次重排后断言名称框仍显示 `A1`。2026-09-24 到 2026-09-27 之间 `CI master` 的 serial 自托管 Linux lane 的 11 次运行中有 10 次在第一次重排处以 `expected 'B1' to be 'A1'` 失败该场景的一个或两个 locale 用例，托管的 `node 24 / snapshots and artifacts` 也以同样方式失败（run 36110572677 与 36213089675）。失败与 locale 无关：四次 master 运行只有 en-US 失败，三次两者都失败，三次只有 zh-CN 失败。把读取改为轮询（#5243）没有作用，因为 `B1` 是已提交的选区，不是过期采样。
|
||||
|
||||
`A1` 是工作簿的初始选区，所以点击之后的两个轮询在点击提交前就已满足，从未验证点击选中了什么。FortuneSheet 的 `SheetOverlay` 以 `setContext(draft => handleCellAreaMouseDown(draft, globalCache, nativeEvent, cellInput, cellArea, …))` 处理按下，而 `handleCellAreaMouseDown` 在 updater 执行的那一刻用 `cellArea.getBoundingClientRect()` 与事件的页面坐标解析目标单元格。React 在渲染期间运行 `useState` 的 updater，并且当 hook 队列里有更低优先级的更新排在它前面时会再次运行它；刚挂载的工作簿仍有这类更新待处理，而 Playwright 在按下之前先派发一次 `mousemove`，其 FortuneSheet 处理器是同一状态上的 continuous 优先级更新。在 Blacksmith runner 上运行的带探针的逐字副本记录到这一次点击触发了四次 `handleCellAreaMouseDown`：两次在点击返回前读到 `left=969.5`，两次在场景已把面板设为 `1000px` 之后读到 `left=725.5`。保存下来的指针 x 985 于是在网格内解析为 259.5，而会议表只有两列，`colLocation` 把它钳到最后一列：`B1`。在点击前就已提交工作簿初始选区归一化的机器上，同一位置落在 A1 自身的选区框上，其 `onMouseDown` 阻止了传播；点击于是什么也没选中，没有几何绑定的 updater 入队，场景在没有测试到点击的情况下通过。
|
||||
|
||||
## Decision
|
||||
|
||||
会议表这一步改为键盘选择。场景聚焦表格 overlay，按 `ArrowRight` 并断言 `B1` 且公式栏为空，再按 `ArrowLeft` 并断言 `A1` 且公式栏为 `会议纪要`，然后才开始重排循环。`handleArrowKey` → `moveHighlightCell` 只依据已提交状态（当前选区与 `visibledatacolumn`）计算目标，所以被重新执行的 updater 无论此时网格几何如何都得到同一单元格，而两次移动使交互可观察，因为 `B1` 与初始选区不同。重排循环、其中的 `A1` 断言、表页滚动控件计数与 canvas 身份检查保持不变。
|
||||
会议表这一步改为键盘选择。场景聚焦表格 overlay（`tabindex="-1"`），断言焦点已落在其上，按 `ArrowRight` 并断言 `B1` 且公式栏为空，再按 `ArrowLeft` 并断言 `A1` 且公式栏为 `会议纪要`，然后才开始重排循环。`handleArrowKey` → `moveHighlightCell` 只依据已提交状态（当前选区与 `visibledatacolumn`）计算目标，所以被重新执行的 updater 无论此时网格几何如何都得到同一单元格，而两次移动使交互可观察，因为 `B1` 与初始选区不同。重排循环、其中的 `A1` 断言、表页滚动控件计数与 canvas 身份检查保持不变。
|
||||
|
||||
场景开头预算表的点击仍是指针点击：它的目标不是初始选区，其结果由 `46281` 公式读取验证，而键盘、剪贴板和布局读取把它与第一次重排隔开远超一次延迟渲染。`values.csv` 的点击及其后的收窄也保持不变：针对收窄后的网格重新执行会解析出负的 x，`colLocation` 把它钳到 A 列，即同一单元格。
|
||||
|
||||
@@ -24,13 +24,13 @@ Status: implemented
|
||||
|
||||
**第一次重排前等待渲染器空闲。** 没有产品状态能表达“React 没有待重新执行的更新”；按 runner 调校的空闲等待或固定等待正是 [dsh-ci-test-reliability](../../../skills/dsh-ci-test-reliability/SKILL.md) 拒绝的掩盖式等待。
|
||||
|
||||
**给 FortuneSheet 打补丁，在 updater 之外解析单元格。** `handleCellAreaMouseDown` 位于 `@fortune-sheet/core`；在 `setContext` 之前快照矩形是针对用户无法触达的窗口（一次指针按下与一次侧栏重排落在同一次延迟渲染内）的依赖补丁。它仍是上游问题；产品改动不在本次测试修复范围内。
|
||||
**扩展既有的 `@fortune-sheet/react` 补丁，在 updater 之外解析单元格。** `handleCellAreaMouseDown` 位于 `@fortune-sheet/core`，而用户需要命中的窗口是一次指针按下与一次侧栏重排落在同一次延迟渲染内。它仍是上游问题；产品改动不在本次测试修复范围内。
|
||||
|
||||
## Verification
|
||||
|
||||
在草稿 PR #5291 的托管 Linux lane 上以 `DEBUG=pw:api` 与 cell area 上的 `getBoundingClientRect` 钩子运行的带探针逐字副本，在两次 lane 运行共 12 次中有 3 次在第一次重排处复现 `expected 'B1' to be 'A1'`，每次都带有上述四次执行的特征，且 `mousedown` 的目标是 `#luckysheet-sheettable_0` 而不是拖拽手柄。键盘驱动的副本在复现了点击失败的那次 lane 运行中 6 次全部通过。本地以 20× 的 `Emulation.setCPUThrottlingRate` 运行可让点击到达 cell area，并记录到每次点击三次 `handleCellAreaMouseDown` 执行；键盘驱动的副本在该限速下 6 次全部通过，在 React 调度器唤醒被推迟 200 ms 时也 6 次全部通过。
|
||||
PR #5291 处于草稿阶段时在托管 Linux lane 上运行的带探针逐字副本，在三次 lane 运行共 38 次中有 7 次（2/6、1/6、4/20）在第一次重排处复现 `expected 'B1' to be 'A1'`。每次失败都记录到 `mousedown` 的目标是 `#luckysheet-sheettable_0` 而不是拖拽手柄，且 `B1` 的提交先于只有按下 updater 才会安排的延迟 `cellInput.focus()`；在同时给 cell area 的 `getBoundingClientRect` 加了钩子的两次 lane 运行里，5 次失败全部带有上述四次执行的特征。键盘驱动的副本在这两次 lane 运行中 26 次全部通过。本地以 20× 的 `Emulation.setCPUThrottlingRate` 运行可让点击到达 cell area，并记录到每次点击三次 `handleCellAreaMouseDown` 执行；键盘驱动的副本在该限速下 6 次全部通过，在 React 调度器唤醒被推迟 200 ms 时也 6 次全部通过。
|
||||
|
||||
`env -u TSX_TSCONFIG_PATH DSH_SNAPSHOT=replay pnpm exec vitest run --config vitest.web.config.ts apps/web/tests/document-preview.e2e.ts -t "fills the spreadsheet pane"` 在 PR 中记录的每一次本地重复运行里都通过两个 locale 用例。
|
||||
`env -u TSX_TSCONFIG_PATH DSH_SNAPSHOT=replay pnpm exec vitest run --config vitest.web.config.ts apps/web/tests/document-preview.e2e.ts -t "fills the spreadsheet pane"` 在本地 10 次重复中 10 次都通过两个 locale 用例；整个文件的 6 个用例在 3 次本地运行中 2 次通过，第 3 次在文档 gate 并行运行时失败于无关的 `:849` `Coding Tools` 开关轮询，失败截图显示开关已打开。
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
@@ -267,11 +267,11 @@ it.skipIf(MODE === 'record').each(['en-US', 'zh-CN'])('fills the spreadsheet pan
|
||||
const meetingCanvas = await canvas.elementHandle()
|
||||
if (meetingCanvas === null) throw new Error('meeting spreadsheet canvas is unavailable')
|
||||
await expectExcelLayout(excel)
|
||||
// Arrow keys move the selection from committed state. A pointer click resolves its cell
|
||||
// from the grid's live geometry inside a React state updater that a later render re-runs,
|
||||
// so a click followed by the reflows below can land on whatever column the moved grid puts
|
||||
// under the stale pointer coordinates.
|
||||
await excel.locator('.fortune-sheet-overlay').focus()
|
||||
// Arrow keys pick the cell from committed state, so a later render cannot re-resolve it
|
||||
// against the reflowed grid the way it re-resolves a pointer press.
|
||||
const overlay = excel.locator('.fortune-sheet-overlay')
|
||||
await overlay.focus()
|
||||
await expect.poll(() => overlay.evaluate(node => document.activeElement === node)).toBe(true)
|
||||
await page.keyboard.press('ArrowRight')
|
||||
await expect.poll(() => selection.innerText()).toBe('B1')
|
||||
await expect.poll(() => formula.innerText()).toBe('')
|
||||
|
||||
Reference in new issue
Block a user