fix(web): a starting copy preview cancels the debounce it already answers (BUG-3151) (#1440)

* fix(web): a starting copy preview cancels the debounce it already answers (BUG-3151)

copy-dialog.spec.ts' coalescing test failed in 2 of 3 full desktop runs
(4 preflights instead of 3) and passed in isolation. The CI traces show the
4th request's body is byte-identical to the 3rd's.

The runner reads the current state when a run starts, so that run answers
every edit made before it, including one whose debounce timer is still
pending. When edits are spaced wider than the 250ms debounce while a preview
is in flight, the earlier edit's timer queues the trailing run. The last
edit's own timer is then still pending when that trailing run starts, and it
fires afterwards to send the same body again. Suite load is what spaced the
test's picks that wide; a 300ms gap reproduces it with no load, every time.

runPreflight now cancels any pending debounce timer when it starts a run or
queues the trailing one. The spec runs the collapse test at two pick spacings
(0ms and 350ms). It waits out the debounce before counting, because the
duplicate arrives after the point where the count used to be read.

* test(e2e): derive the copy-dialog collapse spec's timings from the shared debounce constant (BUG-3151, lead condition)
This commit is contained in:
xarmian
2026-09-22 16:56:44 -04:00
committed by GitHub
parent 69c2f5d4dd
commit 275efc17be
3 changed files with 27 additions and 2 deletions
+13 -1
View File
@@ -2,6 +2,7 @@ import { test, expect } from './fixtures';
import { browserLogin, seedDoc } from './lib/collab-helpers';
import type { APIRequestContext, Locator, Page } from '@playwright/test';
import type { SuiteFixture } from './fixtures';
import { COPY_PREFLIGHT_DEBOUNCE_MS } from '../src/lib/items/copyPreflightTiming';
/**
* Cross-workspace copy / move dialog (PLAN-2373 / TASK-2355).
@@ -601,7 +602,13 @@ test.describe('cross-workspace copy dialog (PLAN-2373 / TASK-2355)', () => {
});
test('override edits collapse to one in-flight preflight plus one trailing run', async ({
// Two spacings (BUG-3151). Picks faster than the debounce
// (COPY_PREFLIGHT_DEBOUNCE_MS, CopyItemDialog's runner) collapse in
// the timer; picks SLOWER than it each fire a run, which queues the trailing
// run while #2 is held — and the last pick's own timer is then still pending
// when that trailing run starts. It used to fire afterwards with an identical
// body (4 requests), which only full-suite load made slow enough to happen.
for (const gap of [0, COPY_PREFLIGHT_DEBOUNCE_MS + 100]) test(`override edits collapse to one in-flight preflight plus one trailing run (picks ${gap}ms apart)`, async ({
page,
fixture,
request,
@@ -657,11 +664,16 @@ test.describe('cross-workspace copy dialog (PLAN-2373 / TASK-2355)', () => {
await expect.poll(() => started).toBe(2);
await pick(1);
if (gap) await page.waitForTimeout(gap);
await pick(2);
// Single-flight: neither pick issued a request of its own.
expect(started).toBe(2);
releaseSecond();
await expect(dialog.getByRole('button', { name: 'Copy', exact: true })).toBeEnabled();
// Outlast any debounce still pending, so a late duplicate is counted
// here rather than after the assertion (BUG-3151: this count used to be
// read before the stray timer fired, except under load).
await page.waitForTimeout(2 * COPY_PREFLIGHT_DEBOUNCE_MS + 100);
// …and both were served by ONE trailing run, not one request each.
expect(started).toBe(3);
});
@@ -43,6 +43,7 @@ their own recovery paths; swallowing those into outcome-unknown would send the
user hunting for an item that provably does not exist.
-->
<script lang="ts">
import { COPY_PREFLIGHT_DEBOUNCE_MS } from '$lib/items/copyPreflightTiming';
import { tick, untrack } from 'svelte';
import Modal from '$lib/components/common/Modal.svelte';
import FieldEditor from '$lib/components/fields/FieldEditor.svelte';
@@ -120,7 +121,7 @@ user hunting for an item that provably does not exist.
// So: a debounce, PLUS single-flight trailing coalescing (round-12 fold-in).
// At most one preflight is in flight at any moment; edits that settle while
// one is running collapse into exactly ONE trailing run.
const PREFLIGHT_DEBOUNCE_MS = 250;
const PREFLIGHT_DEBOUNCE_MS = COPY_PREFLIGHT_DEBOUNCE_MS;
/**
* Types the dialog can safely collect a value for — the set `FieldEditor`
@@ -618,6 +619,14 @@ user hunting for an item that provably does not exist.
// arrives while one is running sets the trailing flag, and exactly one
// trailing run fires when the current one settles — so a burst of edits
// across many inputs collapses to (in-flight + 1) rather than N.
// A run reads the CURRENT state when it starts (buildRequest), so it
// answers every edit made before that moment — including one whose
// debounce timer is still pending. Left pending, that timer fires after
// and sends the same body again: a duplicate preview whenever edits are
// spaced wider than the debounce while one is in flight (BUG-3151, the
// full-suite-only failure of copy-dialog.spec.ts' coalescing test).
clearTimeout(debounceTimer);
debounceTimer = undefined;
if (preflightInFlight) {
trailingQueued = true;
return;
+4
View File
@@ -0,0 +1,4 @@
// The copy dialog's preview debounce (CopyItemDialog.svelte), in a plain module
// so the e2e spec that measures the runner can derive its timings from it
// (BUG-3151): a change here retunes the spec instead of silently weakening it.
export const COPY_PREFLIGHT_DEBOUNCE_MS = 250;