[P3] Planner hold field shows another segment's old time after removing or reordering segments when the moved segment's hold is invalid #107
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/planner/ProfilePlanner.tsx:578,src/planner/ProfilePlanner.tsx:588,src/planner/ProfilePlanner.tsx:208,src/planner/ProfilePlanner.tsx:655,src/ui/DurationInput.tsx:20,src/ui/DurationInput.tsx:21Problem and evidence
Segment fieldsets are keyed by array index (
<fieldset key={i}>, ProfilePlanner.tsx:578). After a segment is removed or moved, React reuses the DurationInput instance at each index for whatever segment now sits there. DurationInput keeps its text in localdraftstate. Its sync effect (DurationInput.tsx:20-25) updatesdraftonly when the newminutesvalue is finite. If a segment with an invalid (NaN) hold moves into an index that previously showed a valid hold, the effect skips the update. The field keeps showing the old segment's valid-looking time (e.g. "20:00" or "3:00"), while aria-invalid=true and the profile alert say that segment's hold is invalid. What the field shows no longer matches the value the plan actually uses.I ran Playwright against the served HEAD production build (/tmp/divesim-review-dist-SqbiNx). Steps: Continue to simulator > Planner Mode > Add segment. Seg1 showed "20:00" and seg2 showed "3:00". I cleared seg2's hold and blurred it: the value became "" with aria-invalid=true. Then I clicked "Remove segment 1". Segment 1's hold field showed "20:00" (the removed segment's value) with aria-invalid=true, and the alerts read "Complete a valid profile to project its ascent." and "Segment 1: depth 0–984 ft, hold 0:00–1440:00, and rates above 0 up to 328 ft/min required." Reorder case: I then clicked Add segment (seg1 showed the stale "20:00" but held NaN; seg2 was 3:00) and clicked "Move segment 1 down". Segment 1 showed "3:00" with aria-invalid=false (correct), but segment 2 showed "3:00" with aria-invalid=true. That 3:00 is stale text; the real hold is NaN. Both remove and reorder reproduce the bug.
Suggested correction
Give each segment row a stable generated id when it is created (default rows, Add segment, auto-ascent output, loaded or imported plans) and key the fieldset by that id instead of the index. Alternatively, or as well, key the DurationInput by row identity, or have DurationInput reset its draft when
minuteschanges to NaN while the field is not being edited (e.g. set the draft to "" or keep a per-row draft), so the text never shows a valid time for an invalid hold.Acceptance criteria
Browser regression test: open Planner and add a segment. Clear segment 2's hold and blur. Remove segment 1. Segment 1's hold field must not show "20:00"; it should show empty or the invalid text, consistent with aria-invalid=true. Similarly, with segment 1 invalid and segment 2 at 3:00, "Move segment 1 down" must leave segment 2's field without a valid-looking stale time, while segment 1 shows 3:00. Existing planner browser and unit tests still pass. pnpm verify passes.
Scope: maintenance of the educational simulator. Preserve historical R1 acceptance and operational-validation limits.