[P3] Replay staleness key omits unit-derived stop spacing, so a completed ascent keeps the old unit's stop grid after a units toggle with no stale notice #110
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: P3
Type: bug
Source: full codebase review requested 2026-10-31 (independently reproduced against
2cac7cf)Location:
src/replay/DiveReplayComparison.tsx:107,src/replay/DiveReplayComparison.tsx:123,src/replay/DiveReplayComparison.tsx:136-143,src/replay/DiveReplayComparison.tsx:335-340,src/replay/DiveReplayComparison.tsx:449-452Problem and evidence
key = JSON.stringify(settings)(line 107) is the staleness token saved intoresults.settings. ThestopSpacingsent to the worker is added only at post time from theunitsprop (units === "imperial" ? 3.048 : 3, line 142), so it is not part of the key. DiveReplayComparison resets only whendivechanges. Closing the summary (active=false) stops the worker but keepsresults/expanded. If the user toggles Units and reopens the summary, the old results still show. A completed decompression ascent stays on the previous unit's grid (metric 3 m stops shown as 39.37/29.53/19.69/9.84 ft), and the "Settings changed. Calculate again" status never appears. The USER-GUIDE says the rebuilt ascent uses "the selected units' stop spacing", so the displayed result no longer matches the current settings and nothing says so. The deco-button gating (line 452) is a smaller part of the same issue. When an uncompleted result is shown, the button stays enabled after a units change, but clicking it re-runs with the new spacing, so its output is correct. Only the non-completed verdict shown before that may have been computed at the other spacing (for example, VPM-B decompressionRequired).I served the HEAD production build (/tmp/divesim-review-dist-SqbiNx) and ran the reviewer's Playwright script (copied to /tmp/divesim-verify-GXndBa/pw-units.mjs). Steps: Metric, 40 m, +1:00 x22, Surface, View dive stats, Compare & replay, Calculate comparison, Complete decompression ascent. That gives stops "12.00 m 1:00, 9.00 m 3:00, 6.00 m 4:00, 3.00 m 11:00". I then closed the summary, set Units to Imperial and clicked View last dive summary. Output: staleNotice [] (no status), and imperialStops "39.37 ft · 1:00, 29.53 ft · 3:00, 19.69 ft · 4:00, 9.84 ft · 11:00". No page errors. The Complete decompression ascent button was absent because the completed result clears decompressionRequired. In code, the only reset effect depends on
[dive](lines 46-56). Theactiveeffect only stops playback and the worker. Nothing reacts tounits.Suggested correction
Derive the effective settings once:
const effective = { ...settings, stopSpacing: units === "imperial" ? 3.048 : 3 }; const key = JSON.stringify(effective);and post{ ...effective, completeDeco }. The existing "Settings changed" status and thekey !== results.settingsgating on the deco button then cover unit toggles. An alternative is to clear results whenunitschanges, but that discards non-spacing results unnecessarily.Acceptance criteria
After a completed decompression ascent in metric, close the summary, switch to Imperial and reopen. Either the "Settings changed. Calculate again..." status is shown (and Complete decompression ascent is disabled until recalculated), or results are recomputed on a 10 ft grid. Recalculating in Imperial gives stops on 10 ft multiples. Regression test: a component/unit test that renders DiveReplayComparison with results, rerenders with a different
units, and asserts the stale status appears. Alternatively, a Playwright test following the reproduction above. pnpm verify passes.Scope: maintenance of the educational simulator. Preserve historical R1 acceptance and operational-validation limits.