[P2] Manual ascent can surface without completing the dive when float rounding leaves a sub-femtometre remainder (no summary, clock keeps running) #111
Loading…
Add table
Add a link
Reference in a new issue
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?
Priority: P2
Type: bug
Source: full codebase review requested 2026-10-31 (independently reproduced against
2cac7cf)Location:
src/models/zhl16c.ts:372-376,src/App.tsx:912-916,src/App.tsx:692-695,src/App.tsx:1446-1449Problem and evidence
advance() only snaps depth to the target when
travel === Math.abs(target - dive.depth) / rateis exactly true. Otherwise it computesdive.depth - rate*travel. Repeated 1-minute (+1:00) or capped 1-s background steps can therefore leave a tiny positive depth. For example, 90 ft at 30 ft/min (27.432 m at 9.144 m/min) leaves 3.55e-15 m at minute 7. On the next step the surfacing branch passesMath.min(1, d.depth / ascent), which is about 3.9e-16 min. Adding that to dive.minutes = 7 rounds away, so depth becomes 0 but minutes stays 7. The recorder's completion check (App.tsx:912-916) requirescurrent.time > previous.time, so it skips this transition. On every later sample previous.depth is already 0, so it never fires. Nothing creates a CompletedDive or opens the 'Dive complete' dialog, no splash plays, andrunningis never set false. Surface ticks keep charging gas to the cylinder. The next descent resets recordedSamples, so that dive's summary and replay are lost.Math.min(1, depth/ascent). Output: depth 18.288 @5, 9.144 @6, 3.55e-15 @7, then step m=3.885e-16 gives depth 0 with minutes still 7 ("time advanced? false" while prevDepth>0). Later steps start at depth 0, so the completion condition can never be met. 2) Playwright against the served HEAD production build (/tmp/divesim-review-dist-SqbiNx, port 34085, server killed afterwards), using the reviewer's pw-stall.mjs. Imperial units. Set Ascent, set depth, click +1:00 four times, click Surface, then click +1:00 up to 6 times. Results: 30 ft/min from 90 ft gives dialogOpen=false on all 6 clicks and no summary button. 30 ft/min from 120 ft gives the same. The control, 29.5 ft/min from 90 ft, opens the dialog on click 4 and shows the summary button.Suggested correction
In advance(), snap to the target with a tolerance. Compute
dist = Math.abs(target - dive.depth)andreach = dist / rate. Ifminutes >= reach * (1 - 1e-12)ordist < 1e-9, set depth = target and travel = Math.min(minutes, reach). This prevents the 1e-15 m residual entirely. As defence in depth, the completion check at App.tsx:915 could acceptcurrent.time >= previous.time. That still excludes reset and model-change cases, where time goes backwards.Acceptance criteria
Unit test: from newDive(), advance 4x1 min toward metresFromDisplay(90,'imperial') at ascent metresFromDisplay(30,'imperial'), then step toward 0 with the App's
Math.min(1, depth/ascent)duration. Depth must reach exactly 0 on the same step that advances minutes (minute 7). Also sweep imperial depth/ascent combinations, for example 40ft@10, 60@15, 90@15, 90@30, 100@20, 120@15, 120@30. Each must reach depth 0 with minutes strictly greater than on the previous step. Playwright/UI check: 30 ft/min from 90 ft and from 120 ft opens the 'Dive complete' dialog, and the running clock stops. pnpm verify passes.Scope: maintenance of the educational simulator. Preserve historical R1 acceptance and operational-validation limits.