fix(stats): drop stale excluded-word sync notifications

- Skip notifyServerSync for superseded excluded-word writes so an in-flight
  ack can't trigger a redundant aggregate recompute
- Await hook teardown so React's scheduler drains before unmounting DOM
  globals in tests
- Add regression test proving a slow, superseded aggregates refresh cannot
  overwrite the newest totals
This commit is contained in:
2026-08-17 02:12:14 -07:00
parent 3c111d2e3b
commit e64763b487
3 changed files with 72 additions and 10 deletions
+3 -2
View File
@@ -278,8 +278,9 @@ test('overlapping writes serialize so an older list cannot overwrite a newer edi
JSON.stringify({ words: third }), JSON.stringify({ words: third }),
]); ]);
assert.deepEqual(getExcludedWordsSnapshot(), third); assert.deepEqual(getExcludedWordsSnapshot(), third);
assert.equal(syncs.length, 2); // Only the final revision notifies: the first write's acknowledgement was
assert.equal(syncs.at(-1), JSON.stringify(third)); // already obsolete, so it must not trigger an aggregate recomputation.
assert.deepEqual(syncs, [JSON.stringify(third)]);
} finally { } finally {
unsubscribe(); unsubscribe();
globalThis.fetch = originalFetch; globalThis.fetch = originalFetch;
+4 -1
View File
@@ -142,7 +142,10 @@ export async function setExcludedWords(words: ExcludedWord[]): Promise<void> {
console.error('Failed to persist excluded words to stats database', error); console.error('Failed to persist excluded words to stats database', error);
throw error; throw error;
} }
notifyServerSync(); // A newer edit arrived while this write was in flight, so the server state
// this acknowledges is already obsolete. Its own acknowledgement notifies
// with the newest list; skipping here avoids a wasted aggregate scan.
if (revision === writeRevision) notifyServerSync();
}); });
writeChain = write.catch(() => {}); writeChain = write.catch(() => {});
return write; return write;
+65 -7
View File
@@ -123,7 +123,7 @@ interface Harness {
flush: () => Promise<void>; flush: () => Promise<void>;
tick: (ms: number) => Promise<void>; tick: (ms: number) => Promise<void>;
unmount: () => Promise<void>; unmount: () => Promise<void>;
teardown: () => void; teardown: () => Promise<void>;
} }
async function mountHook(): Promise<Harness> { async function mountHook(): Promise<Harness> {
@@ -170,9 +170,15 @@ async function mountHook(): Promise<Harness> {
root = null; root = null;
}); });
}, },
teardown: () => { teardown: async () => {
if (root) root.unmount(); await act(async () => {
root?.unmount();
root = null;
});
clock.restore(); clock.restore();
// React's scheduler can still have deferred work queued; let it drain on
// a real timer while the DOM globals it reads are still installed.
await new Promise((resolve) => setTimeout(resolve, 0));
uninstallLocalStorage(); uninstallLocalStorage();
uninstallDom(); uninstallDom();
resetExcludedWordsStoreForTests(); resetExcludedWordsStoreForTests();
@@ -242,7 +248,7 @@ test('aggregate failures retry with backoff, then surface an error that Retry cl
assert.equal(harness.state().aggregatesError, null); assert.equal(harness.state().aggregatesError, null);
assert.deepEqual(harness.state().summary, summaryFixture()); assert.deepEqual(harness.state().summary, summaryFixture());
} finally { } finally {
harness.teardown(); await harness.teardown();
restoreClient(); restoreClient();
console.error = originalConsoleError; console.error = originalConsoleError;
} }
@@ -274,7 +280,7 @@ test('charts poll while the backfill is pending and stop once it is ready', asyn
await harness.tick(60_000); await harness.tick(60_000);
assert.equal(chartCalls, 3); assert.equal(chartCalls, 3);
} finally { } finally {
harness.teardown(); await harness.teardown();
restoreClient(); restoreClient();
} }
}); });
@@ -307,7 +313,7 @@ test('aggregates refetch after an exclusion edit is acknowledged by the server',
assert.equal(summaryCalls, 2, 'totals must not keep counting the excluded word'); assert.equal(summaryCalls, 2, 'totals must not keep counting the excluded word');
assert.equal(chartCalls, 2); assert.equal(chartCalls, 2);
} finally { } finally {
harness.teardown(); await harness.teardown();
restoreClient(); restoreClient();
} }
}); });
@@ -334,8 +340,60 @@ test('pending retries are cancelled when the tab unmounts', async () => {
assert.equal(summaryCalls, 1, 'no retry may run after unmount'); assert.equal(summaryCalls, 1, 'no retry may run after unmount');
} finally { } finally {
harness.teardown(); await harness.teardown();
restoreClient(); restoreClient();
console.error = originalConsoleError; console.error = originalConsoleError;
} }
}); });
test('a slow response from a superseded refresh cannot replace the newest aggregates', async () => {
let summaryCalls = 0;
let releaseSuperseded: (() => void) | null = null;
const restoreClient = stubVocabularyClient({
getVocabularySummary: async () => {
summaryCalls += 1;
const call = summaryCalls;
// The second call is the one that gets superseded while still in flight.
if (call === 2) {
await new Promise<void>((resolve) => {
releaseSuperseded = resolve;
});
}
return { ...summaryFixture(), uniqueWords: call };
},
getVocabularyCharts: async () => chartsFixture(),
});
const harness = await mountHook();
try {
await harness.flush();
assert.equal(harness.state().summary?.uniqueWords, 1);
// First refresh stalls, then a second refresh supersedes it and resolves.
await act(async () => {
harness.state().refreshAggregates();
});
await harness.flush();
await act(async () => {
harness.state().refreshAggregates();
});
await harness.flush();
assert.equal(summaryCalls, 3);
assert.equal(harness.state().summary?.uniqueWords, 3);
const release = releaseSuperseded as (() => void) | null;
assert.ok(release, 'expected the superseded request to still be in flight');
release();
await harness.flush();
assert.equal(
harness.state().summary?.uniqueWords,
3,
'the superseded response must not overwrite the newest totals',
);
} finally {
await harness.teardown();
restoreClient();
}
});