test(patterns): bounded-latency E2E for client targeted DOM mutation (livetemplate/client#108)

[adnaan]

Summary

Adds two chromedp subtests that verify the client-side per-op targeted DOM mutation path shipped in livetemplate/client#108 (released as @livetemplate/client@0.8.39):

1. TestLargeTable/Delete_Targeted_Apply_Path_Taken — runs at the existing N=200 CI scale. Asserts that after a single-row delete, window.__lvtTargetedHits === 1 — confirming the RangeDomApplier actually fired (vs silently hitting the fallback path).

2. TestLargeTable_DeleteLatency_10k — stress test at the demo's default 10,000-row scale with a 2500ms wall-clock ceiling. Skipped under -short. Pre-fix wall-clock at this scale was 6-8s; post-fix is ~1.5s. The ceiling is intentionally generous to absorb headless-Chrome variance and the residual browser layout cost (the floor that drove follow-up issue livetemplate/client#109 for table virtualization).

Depends on @livetemplate/client@0.8.39+ for the window.__lvtTargetedHits observability hook. Cross-repo CI uses the CDN build — should pick up the new version automatically.

Test plan

• Local: LVT_LOCAL_CLIENT=.../client/dist/livetemplate-client.browser.js go test -v -run "^TestLargeTable$|^TestLargeTable_DeleteLatency_10k$" -timeout 5m — green at 1.5-1.8s wall-clock with targeted-apply hits: 1
• CI: cross-repo workflow runs against published @livetemplate/client@0.8.39 from CDN

🤖 Generated with Claude Code

[claude]

Code Review

Overview

This PR adds two chromedp subtests to verify the client-side targeted DOM mutation path introduced in livetemplate/client#108: a correctness probe at N=200 and a wall-clock regression guard at N=10,000. The motivation is clear and well-documented. A few issues need attention before merging.

---

Bugs / Correctness Issues

1. Docstring ceiling (3500 ms) doesn't match the constant (2500 ms)

In TestLargeTable_DeleteLatency_10k:

// The 3500 ms ceiling is intentionally generous ...
const ceilingMs = 2500

The comment says 3500 ms but ceilingMs = 2500. One of them is wrong. Given that the PR description also says "2500ms wall-clock ceiling", the constant looks correct and the docstring needs updating.

2. Comment references client#107 instead of client#108

// Verifies the client#107 targeted-apply path actually fires for

The PR summary and all other references say client#108. Minor but misleading.

---

Test Robustness Issues

3. No graceful handling when the observability hook is absent

Both tests read window.__lvtTargetedHits || 0 and call t.Errorf if the value is 0. If the CDN serves a cached version of the client older than 0.8.39 (where the hook doesn't exist), __lvtTargetedHits will be undefined, || 0 will resolve to 0, and the test will appear to fail with a misleading message about the targeted-apply predicate.

A safer guard:

var hookExists bool
chromedp.Evaluate(`typeof window.__lvtTargetedHits !== "undefined"`, &hookExists),
// ...
if !hookExists {
    t.Skip("window.__lvtTargetedHits not present — client < 0.8.39, skipping targeted-apply assertion")
}

This converts a confusing "targeted-apply did not fire" failure into a clear skip when the feature is simply absent from the loaded client.

4. Delete_Targeted_Apply_Path_Taken doesn't assert the deleted row is gone

Per CLAUDE.md: "Assert full page state after each mutation, not just the changed element."

The subtest waits for the count text ("Showing 199 of 199 rows.") but never verifies that tbody tr[data-key="row-00100"] is actually absent. The count could theoretically update via the scalar morphdom path while the row lingers. Adding a WaitFor for row absence (as done in the 10k test) would complete the assertion:

e2etest.WaitFor(`document.querySelector('tbody tr[data-key="row-00100"]') === null`, 5*time.Second),

---

Minor Observations

5. Ceiling headroom is tight for CI

2500 ms for a 10k-row delete leaves roughly 1 s headroom above the reported ~1.5 s local time. Headless Chrome in CI can exhibit 2–3× slowdowns during resource contention. The PR already acknowledges the ceiling is "intentionally generous to absorb headless-Chrome variance" — just flagging this is worth re-checking if CI starts flaking, given the PR description also mentions the docstring said 3500 ms (which would give more room).

6. 30 s WaitFor timeout on row removal could mask slow failures

e2etest.WaitFor(`document.querySelector('tbody tr[data-key="row-05000"]') === null`, 30*time.Second),

If the delete operation fails silently (e.g., WebSocket error), this will hang for 30 s before giving a generic chromedp error. Not a blocker, but a tighter initial wait (e.g., 10 s) with a clearer error message would make CI failures faster to diagnose.

---

What's Done Well

• All waits use condition-based helpers (WaitFor, WaitForText, WaitForCount) — no chromedp.Sleep. ✓
• testing.Short() guard on the stress test is correct and necessary. ✓
• t.Setenv gives proper test isolation for LARGE_TABLE_SIZE. ✓
• Counter reset (window.__lvtTargetedHits = 0) before the critical section is correct. ✓
• Comments explain why each timing decision was made, not just what the code does. ✓
• performance.now() timing is measured inside the browser, avoiding Go-side network latency noise. ✓

---

Summary

The approach is solid. Fix the docstring/constant mismatch (#1), the client#107 → client#108 reference (#2), add a hook-existence guard (#3), and add a row-absence assertion to the N=200 subtest (#4). The remaining points are suggestions, not blockers.

[Copilot]

Pull request overview

This PR extends the patterns E2E suite to validate the Large Table demo’s client-side targeted DOM mutation path, adding both a functional signal check and a bounded-latency stress test around single-row deletion at 10k-row scale.

Changes:

• Adds a TestLargeTable subtest that inspects window.__lvtTargetedHits after a delete to confirm the targeted-apply path ran.
• Adds a new top-level 10k-row latency test with a wall-clock ceiling and -short skip behavior.
• Uses LARGE_TABLE_SIZE overrides to run the existing suite at CI-friendly scale and the new stress test at full demo scale.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[adnaan]

Bot review fix landed in livetemplate/client v0.8.40 (livetemplate/client#114) — the test-examples failure (TestHighlightOnChange) was caused by livetemplate/client#108 directive-scan over-eagerness, fixed in #114. Re-running CI against the published v0.8.40 client.