pedro/DiveSim
1
0
Fork 0
Code Issues 74 Pull requests Projects Releases 1 Packages Wiki Activity Actions

[P3] WebKit compatibility smoke is flaky on a loaded host because of fixed timing assumptions in the harness (no product defect) #123

New issue
Open
opened 2026-10-31 19:47:41 +00:00 by pedro · 0 comments
pedro commented 2026-10-31 19:47:41 +00:00
Owner
Copy link

Priority: P3
Type: bug
Source: full codebase review requested 2026-10-31 (independently reproduced against 2cac7cf)

Location: tests/browser/check-compatibility-browser.mjs:128, tests/browser/check-compatibility-browser.mjs:129, tests/browser/check-compatibility-browser.mjs:169, tests/browser/check-compatibility-browser.mjs:174, src/game/ReefRunner.tsx:64, src/game/reefGame.ts:24, src/planner/PlanLibrary.tsx:59

Problem and evidence

Under host load, tests/browser/check-compatibility-browser.mjs fails intermittently with BROWSER_ENGINE=webkit even though the app behaves correctly. Two failure points are shown here.
(a) Line 128 waits for the planner dialog to become visible. Line 129 then reads the 'Planned cylinder 1 label' value once. The shared plan is restored afterwards, in PlanLibrary's mount effect (PlanLibrary.tsx:59-67), so the single read can still return the default ''.
(c) Line 174 waits a fixed 20 s for the 'coins banked' heading. ReefRunner clamps each frame to dt <= 0.05 s (ReefRunner.tsx:64). The return phase needs about 8.3 s of game time: the 3-game-minute stop at REEF_TIME_SCALE 24, plus the ascent. Below about 8 fps, the real time needed is more than 20 s.
(b) Line 169 reads the master-gain value once. The earlier reviewer saw it return undefined once. I did not reproduce this, and the mechanism is unclear: React flushes passive effects from a discrete click synchronously, so setSilent(true) should already have run.

I served /tmp/divesim-review-dist-SqbiNx with python http.server and ran the unmodified harness 3 times with BROWSER_ENGINE=webkit, using the Playwright 1.64.1 copy at /tmp/divesim-browser/node_modules/playwright/index.mjs (it matches installed webkit-2370; the toolbox copy expects webkit-2368, which is not installed). Load average was 11-17 on 6 cores.

  • Run 1 failed at line 129 after 27 s: AssertionError '' !== 'Reef café 🐠'.
  • Runs 2 and 3 failed at line 174 after 63 s and 65 s: "locator.waitFor: Timeout 20000ms exceeded ... heading /coins banked/".
    A WebKit probe measured requestAnimationFrame at 2.6 fps while the Reef Runner game was swimming.
    A scratch script ran the real stepReef from returning to banked at fixed fps, with dt clamped as in ReefRunner. Real seconds to bank:
  • 60 fps: 8.3 s
  • 20 fps: 8.4 s
  • 10 fps: 16.3 s
  • 5 fps: 33.0 s
  • 3 fps: 55.3 s
    Game time was about 8.3 s in every case and the phase always reached 'banked', so the app is correct and only the real-time budget is short.
    A separate probe opened the share link in 6 fresh WebKit contexts. All six showed 'Reef café 🐠' both immediately and 2 s later. So (a) is a race that does not show up every time, and the label is always applied eventually.

Suggested correction

Change only the harness:

  • Line 129: replace the single inputValue read with a poll, e.g. await shared.waitForFunction(() => document.querySelector('[aria-label="Planned cylinder 1 label"]')?.value === 'Reef café 🐠'), or expect.poll/toHaveValue with a timeout.
  • Line 169: poll with page.waitForFunction(() => window.__qaAudio.at(-1)?.qaGains[0]?.qaGainCalls.at(-1)?.value === 0) instead of reading once.
  • Line 174: size the bank wait from game progress (for example, wait for the stop status to reach 3/3, then the banked heading), or use a much larger timeout such as 180 s.
    Leave the game's dt clamp unchanged.

Acceptance criteria

  • Run BROWSER_ENGINE=webkit node tests/browser/check-compatibility-browser.mjs against the production build on a host with load average 10 or more. It should pass in at least 3 consecutive runs, including runs where the game renders at 5 fps or less.
  • Chromium and Firefox runs should still pass.
  • The harness should still fail if the shared label is never restored, if pause never sets the master gain to 0, or if the game never banks. Check this by temporarily breaking each behaviour in a scratch build. pnpm verify passes.

Scope: maintenance of the educational simulator. Preserve historical R1 acceptance and operational-validation limits.

Priority: P3 Type: bug Source: full codebase review requested 2026-10-31 (independently reproduced against `2cac7cf`) Location: `tests/browser/check-compatibility-browser.mjs:128`, `tests/browser/check-compatibility-browser.mjs:129`, `tests/browser/check-compatibility-browser.mjs:169`, `tests/browser/check-compatibility-browser.mjs:174`, `src/game/ReefRunner.tsx:64`, `src/game/reefGame.ts:24`, `src/planner/PlanLibrary.tsx:59` Problem and evidence Under host load, tests/browser/check-compatibility-browser.mjs fails intermittently with BROWSER_ENGINE=webkit even though the app behaves correctly. Two failure points are shown here. (a) Line 128 waits for the planner dialog to become visible. Line 129 then reads the 'Planned cylinder 1 label' value once. The shared plan is restored afterwards, in PlanLibrary's mount effect (PlanLibrary.tsx:59-67), so the single read can still return the default ''. (c) Line 174 waits a fixed 20 s for the 'coins banked' heading. ReefRunner clamps each frame to dt <= 0.05 s (ReefRunner.tsx:64). The return phase needs about 8.3 s of game time: the 3-game-minute stop at REEF_TIME_SCALE 24, plus the ascent. Below about 8 fps, the real time needed is more than 20 s. (b) Line 169 reads the master-gain value once. The earlier reviewer saw it return undefined once. I did not reproduce this, and the mechanism is unclear: React flushes passive effects from a discrete click synchronously, so setSilent(true) should already have run. I served /tmp/divesim-review-dist-SqbiNx with python http.server and ran the unmodified harness 3 times with BROWSER_ENGINE=webkit, using the Playwright 1.64.1 copy at /tmp/divesim-browser/node_modules/playwright/index.mjs (it matches installed webkit-2370; the toolbox copy expects webkit-2368, which is not installed). Load average was 11-17 on 6 cores. - Run 1 failed at line 129 after 27 s: AssertionError '' !== 'Reef café 🐠'. - Runs 2 and 3 failed at line 174 after 63 s and 65 s: "locator.waitFor: Timeout 20000ms exceeded ... heading /coins banked/". A WebKit probe measured requestAnimationFrame at 2.6 fps while the Reef Runner game was swimming. A scratch script ran the real stepReef from returning to banked at fixed fps, with dt clamped as in ReefRunner. Real seconds to bank: - 60 fps: 8.3 s - 20 fps: 8.4 s - 10 fps: 16.3 s - 5 fps: 33.0 s - 3 fps: 55.3 s Game time was about 8.3 s in every case and the phase always reached 'banked', so the app is correct and only the real-time budget is short. A separate probe opened the share link in 6 fresh WebKit contexts. All six showed 'Reef café 🐠' both immediately and 2 s later. So (a) is a race that does not show up every time, and the label is always applied eventually. Suggested correction Change only the harness: - Line 129: replace the single inputValue read with a poll, e.g. `await shared.waitForFunction(() => document.querySelector('[aria-label="Planned cylinder 1 label"]')?.value === 'Reef café 🐠')`, or `expect.poll`/`toHaveValue` with a timeout. - Line 169: poll with `page.waitForFunction(() => window.__qaAudio.at(-1)?.qaGains[0]?.qaGainCalls.at(-1)?.value === 0)` instead of reading once. - Line 174: size the bank wait from game progress (for example, wait for the stop status to reach 3/3, then the banked heading), or use a much larger timeout such as 180 s. Leave the game's dt clamp unchanged. Acceptance criteria - Run `BROWSER_ENGINE=webkit node tests/browser/check-compatibility-browser.mjs` against the production build on a host with load average 10 or more. It should pass in at least 3 consecutive runs, including runs where the game renders at 5 fps or less. - Chromium and Firefox runs should still pass. - The harness should still fail if the shared label is never restored, if pause never sets the master gain to 0, or if the game never banks. Check this by temporarily breaking each behaviour in a scratch build. pnpm verify passes. Scope: maintenance of the educational simulator. Preserve historical R1 acceptance and operational-validation limits.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
t3/project-branding-style-guide
maintenance/review-2026-10-27
codex/release-r1
R1
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: pedro/DiveSim#123
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?