Skip to content

test(ui): wait for computed Git diffs before composer selection - #1248

Merged
pascalandr merged 2 commits into
devfrom
fix/git-history-context-click
Nov 1, 2026
Merged

pascalandr merged 2 commits into
devfrom
fix/git-history-context-click

Conversation

@pascalandr

@pascalandr pascalandr commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix the recurring git-history.test.ts composer-context CI failure at its fixture readiness boundary. The real Monaco editor, native pointer hit testing and all existing composer assertions remain in use. No application code, timeout, force click, retry or sleep is added.

Current head: 6584d355129a40a41b5042470a56f6fd276dc20b. The delayed-paint regression synchronizes to readiness/native pointer boundaries, addressing the P2 review finding without an elapsed frame budget.

Root cause

The first visible .view-line is plain model text, not evidence that Monaco's asynchronous diff has finished. The worker result can insert inline deleted-code view zones after Playwright measures that line but before native mouse-down. Its DOM can still look stable while Monaco's internal hit-test geometry has already changed.

That click opens Monaco's Copy changed lines menu (InlineDiffDeletedCodeMargin's CONTENT_VIEW_ZONE mouse-down handler). Home/Shift+End go to the menu's shadow-root focus rather than making an editor selection. The apparent insert action is therefore hover-only. Moving from the modified editor to its sibling context overlay calls onMouseLeave, clears the hovered line and removes the widget before its own mouse-enter handler runs. Playwright reports <html> interception followed by detachment. This is not an HTML overlay, a slow click, a composer-focus timer, or identical-target reader disposal.

The identical-target reload's editor and diff-model identity assertions passed before the failing click. The fixture drives that reload directly through the preview store; no native event-dispatcher activity is required for this failure.

Fix

Use one shared test interaction for all four modified-line selections: wait for the fixture's expected .line-insert decoration, then locate and normally click the line. Other Git browser cases already use this computed-diff marker. Keep the original multi-line, local/commit-qualified, WSL service-directory, focus and draft-restoration assertions. Document the readiness rule in dev-docs/BROWSER_TESTING.md.

Production behavior is unchanged, so public help pages need no update. The modified browser test is approximately 1,085 lines; no unrelated file split is included.

Evidence and regression

  • Investigated run 41261807276, browser shard 1, head 0e5820489aeda482369157eaf530e63e26a6fcb1. Its Git reader/widget/test/fixture/style sources are identical to the new branch's origin/dev base, abb14f83c.
  • The untouched baseline reproduced the exact Ajouter au prompt / visible-enabled-stable / <html> interception / detached-widget signature on Windows. Passing reruns with added diagnostics were not used as proof of a fix.
  • Controlled reproduction buffered the real Monaco $computeDiff worker reply, delivered it between coordinate measurement and native mouse-down, and withheld repaint. The original pointer/keyboard sequence deterministically focused the Copy changed lines shadow-root menu, kept the selection collapsed at line 3, and reproduced the exact terminal click signature.
  • The committed regression buffers only the first real diff reply and holds render callbacks until an explicit interaction boundary. The helper's real main-world DOM readiness probe delivers that reply and releases repaint. If that one wait is removed, the reply is delivered only at native pointer movement, after Playwright has measured/scrolled the old line; repaint stays held through native mouse-down and releases only on native pointer-up, after Monaco has opened its deleted-code menu. There is no frame/time expiry. Native input, DOM hit testing and Playwright's captured actionability frames stay live; no diff or renderer is reimplemented.
  • Negative controls: remove only the .line-insert DOM wait in clickModifiedDiffLine: 8/8 independent controls fail on the actual Copy changed lines menu (1 !== 0), comprising five unthrottled runs and three with renderer CPU throttled 6×. Restore only that wait: 8/8 corresponding runs pass both the delayed regression and original composer test (16/16 cases), explicitly asserting that no menu opened, repaint was released by the readiness probe, Monaco has a non-empty selection, and the real session composer receives File: src/components/git-panel.tsx : 3-3. Controls use native pointer/keyboard input throughout.

Validation

  • npm ci
  • npx playwright install chromium
  • npm run typecheck --workspace @codenomad/ui — passed.
  • From packages/ui: node --import tsx --test --test-name-pattern='composer selection waits|central diff inserts' tests/browser/git-history.test.ts — 2/2 passed.
  • Revised interaction-synchronized regression: three serial repetitions of the full git-history, git-changes-scale, git-init and project-git-lifecycle browser suites — 37/37 passed per repetition, 111/111 total, zero failures/cancellations/skips (142.0 s, 132.2 s, 119.6 s).
  • Original head 0531a9b46: its three full affected-suite repetitions also finished 37/37 each, 111/111 total, zero failures/cancellations/skips (166.8 s, 187.6 s, 233.5 s).
  • node --import tsx --test tests/browser/fixture-wiring.test.ts — 1/1 passed.
  • git diff --check — passed.

WSL Ubuntu was inspected read-only: it has no Node, Chromium or Xvfb installed, so no Linux toolchain was provisioned. The PR's standard Linux browser CI provides the next platform qualification. The downloaded failed-run fixture artifact contains only a clean daemon shutdown, without a browser trace.

Reproduce the targeted checks from packages/ui:

node --import tsx --test --test-name-pattern='composer selection waits|central diff inserts' tests/browser/git-history.test.ts
node --import tsx --test --test-concurrency=1 tests/browser/git-history.test.ts tests/browser/git-changes-scale.test.ts tests/browser/git-init.test.ts tests/browser/project-git-lifecycle.test.ts

The Git history composer test can measure an initially rendered model line before Monaco's worker installs inline deleted-code view zones. A native click at that stale point opens Copy changed lines; Home and Shift+End then target that menu instead of selecting editor text. The remaining hover-only context widget disappears during pointer movement, producing the misleading html-interception and DOM-detachment failure seen across unrelated PRs.

Wait for the fixture's expected computed insertion decoration before locating and clicking each modified line. Keep the real pointer checks and every existing composer assertion, including identical-target editor/model identity, multi-line selections, commit-qualified context, WSL service-directory authority and draft restoration. Document this browser-fixture readiness rule; application behavior is unchanged.

Add a real-worker regression that buffers the first computeDiff reply and withholds repaint callbacks for 30 native frames while normal input and Playwright actionability remain live. Removing the readiness wait makes the regression fail because Copy changed lines is present; restoring it passes with a non-empty native editor selection and the expected composer context. The untouched baseline also reproduced the exact CI html-interception/detachment signature locally. UI typecheck and both focused browser cases pass; repeated full affected suites provide additional validation in the PR.

@pascalandr pascalandr left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: changes requested — 1 finding (P0: 0, P1: 0, P2: 1, P3: 0)

Reviewed exact head 0531a9b46d4691b34993f0fe93274f4df8f7c47f against dev (abb14f83c6168b9eb04001359574e5d31ecc72d0). Submitted as a COMMENT review as requested.

[P2] Keep the injected stale geometry until the interaction boundary

Location: packages/ui/tests/browser/git-history.test.ts:821-830.

The repaint hold expires after 30 native animation frames, independently of whether Playwright has reached the line click. If actionability checks or browser/driver scheduling outlast that window, repaint finishes first and the regression passes even with the readiness fix removed. I independently removed only the helper's .line-insert wait in an isolated exact-head snapshot: runs 1 and 3 failed on Copy changed lines (1 !== 0), but run 2 exited successfully, including the real-composer assertion. Thus the committed regression can silently accept removal of the very wait it is meant to protect.

Fix: synchronize the forced model/DOM skew with the selection/readiness interaction instead of releasing it after an elapsed frame budget. Keep native hit testing, and verify that the one-line removed-wait negative control reliably fails under load. Increasing the frame count would only widen the timing window.

Evidence: C:/Users/Admin/AppData/Local/Temp/opencode/review-1248-negative-r1/red-1.log, red-2.log, red-3.log; control-no-wait.ts preserves the exact one-line negative control. Six further instrumented controls failed with native pointerdown 40–58 ms after releasing the worker reply, no insertion decorations, a collapsed line-3 selection and the shadow-root menu present (timed-*.log and timed-followup-*.log).

Root cause and scope verification

  • Independently reproduced the full native-click failure using the real worker result delivered at pointer movement while repaint was held: Copy changed lines appeared, focus moved to shadow-root-host, selection stayed empty, and clicking Ajouter au prompt produced the exact <html> interception followed by DOM detachment. Evidence: C:/Users/Admin/AppData/Local/Temp/opencode/review-1248-signature-r1/signature.log.
  • The .line-insert wait itself is a sound readiness boundary for these fixture inputs: Monaco publishes diff decorations and view-zone changes from the computed result; text geometry is rendered before the decoration overlay. All four helper callers open fresh reader identities with insertion-bearing fixture content. The default action timeout remains bounded at 30 seconds.
  • Native pointer/keyboard input, local and revision-qualified context, multiline ranges, identical-target editor/model identity, WSL service-directory authority, focus and draft restoration assertions remain intact. The change introduces no swallowed errors, force clicks, selection API shortcut or retry/sleep workaround.
  • Checked the original failed CI run and its matching source objects, the supplied controlled/hover traces and negative-control evidence, area instructions and fixture conventions. No production behavior changes or public-help update are needed.

Independent checks

  • UI typecheck: passed.
  • Fixture wiring: 1/1 passed.
  • Full git-history, git-changes-scale, git-init and project-git-lifecycle browser suites, serial within each run: 37/37 passed in each of three independent repetitions (111 successful tests; approximately 231.5 s, 172.0 s and 154.6 s). Logs: C:/Users/Admin/AppData/Local/Temp/opencode/review-1248-r1/suite-1.log through suite-3.log.
  • Targeted fixed regression and original composer test: 2/2 passed separately; controlled reproduction of the exact original failure signature: 1/1 passed by asserting that failure.
  • Local browser qualification was on Windows; all four Linux browser CI shards were queued at publication.
  • git diff --check: passed. Worktree remains clean at the reviewed head; validation used separate temporary snapshots and fixture-owned caches/assets. The author's processes and shared OpenCode daemon were untouched.

File-size note: packages/ui/tests/browser/git-history.test.ts is 1,059 lines (approximately 1,060); size alone is not a finding.

The regression's elapsed 30-frame paint hold could expire before a slow browser driver reached the line click, allowing removal of the readiness wait to pass. Bind fault injection to the real readiness and pointer boundaries instead of an elapsed frame budget.

The main-world DOM readiness probe delivers the buffered real diff reply and releases painting before it can observe Monaco's insertion decoration. Without that single wait, deliver the reply only on native pointer movement after the old line has been measured and scrolled, then retain stale DOM through mouse-down until native pointer-up. Assert the readiness release boundary alongside the existing absent-menu, native selection and real composer-context checks. Native hit testing, keyboard input and all original composer assertions remain unchanged.

Eight one-line removed-wait negative controls all fail on the actual Copy changed lines menu: five ordinary runs and three at 6x renderer CPU throttling. Restoring only the wait passes both targeted cases in all eight matching runs (16/16). Three new full affected browser-suite repetitions pass 37/37 each (111/111), as did the original head's three repetitions. UI typecheck, fixture wiring and diff whitespace checks pass. No CI runs were manipulated.

@pascalandr pascalandr left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: pass — P0: 0, P1: 0, P2: 0, P3: 0

Reviewed exact head 6584d355129a40a41b5042470a56f6fd276dc20b against dev (abb14f83c6168b9eb04001359574e5d31ecc72d0). No remaining findings. This is a COMMENT review as requested.

Previous P2: resolved

The stale-render window is now tied to the readiness/native-input boundaries rather than an elapsed frame budget. Removing only the readiness wait delivers the real worker result at native pointer movement, after the old line's coordinates have been measured, and preserves the stale DOM through mouse-down until pointer-up. The resulting failure is Monaco's actual Copy changed lines menu, not merely an assertion about which fixture branch ran.

The positive path's query hook only releases the buffered real worker reply and queued render callbacks. It returns the original DOM query's result, so the helper must still observe a real .line-insert decoration before normally clicking the line. No line positions, diff results, selections or menu elements are fabricated. This controls a possible worker-result/render/input ordering without disposing the editor or bypassing native pointer hit testing.

All four shared-helper callers retain their real pointer/keyboard selections and their local/commit-qualified, multiline, editor/model-identity, WSL service-directory, composer-focus and draft-restoration assertions. The readiness wait remains bounded by Playwright's unchanged default timeout; the instrumentation and cleanup belong to the fixture page. Production code is unchanged.

Independent validation

  • Removed only the helper's readiness wait in an isolated snapshot: 3/3 controls failed on the actual menu assertion (1 !== 0), comprising two normal runs and one with Chromium renderer CPU throttled 6×. The preserved control-no-wait.ts differs from the reviewed source by exactly that one deleted line.
  • Restored only that wait and verified that the source matches the exact-head snapshot after line-ending normalization: all three corresponding positive runs passed both the delayed regression and original composer test (6/6 cases), including the 6× throttled pair. Every run had zero failures/cancellations/skips.
  • Full git-history, git-changes-scale, git-init and project-git-lifecycle browser suites: 37/37 passed in each of two serial repetitions (74/74 total), with zero failures/cancellations/skips (131.5 s and 115.6 s).
  • Additional native-event traces independently confirm the geometry transition: without the wait, line 3's real Monaco top changes from 36 to 90 between trusted pointer movement and mouse-down while the decoration is absent; repaint releases at pointer-up, focus is in the shadow-root menu, and selection is collapsed. With the wait restored, the decoration is already present and line top remains 90 throughout trusted pointer input; keyboard input reaches Monaco's editor, selection is nonempty, and a trusted pointer click inserts context into the real composer. This traced positive run passed.
  • UI typecheck: passed. Fixture wiring: 1/1 passed. git diff --check: passed.
  • Rechecked the supplied eight negative controls, eight corresponding two-case positive runs and three full-suite logs; their recorded results match the PR's claims.

Evidence: C:/Users/Admin/AppData/Local/Temp/opencode/review-1248-controls-r2/ contains control-no-wait.ts, red-1.log through red-3.log, green-1.log through green-3.log, and the 6× throttle preloader. Full-suite logs are under C:/Users/Admin/AppData/Local/Temp/opencode/review-1248-r2/.

Native-event trace evidence: C:/Users/Admin/AppData/Local/Temp/opencode/review-1248-trace-r2/native-negative.log and native-positive.log.

Validation used separate temporary snapshots with fixture-owned assets/caches. The reviewed worktree remains clean; no commits, pushes, merges, CI changes or shared-daemon operations were performed. Local browser validation is Windows-only; CI qualification remains with the coordinator.

File-size note: packages/ui/tests/browser/git-history.test.ts is 1,084 lines; size alone is not a finding.

@pascalandr
pascalandr merged commit 186d8fc into dev Nov 1, 2026
44 of 45 checks passed
@pascalandr
pascalandr deleted the fix/git-history-context-click branch November 1, 2026 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant