fix: keep a new shape's group id and description across a later undo - #2677
Conversation
|
Agent review (standards + spec axes), verdict: recommend-merge. Verified against the code at 79f9652, not the description: The regression test fails on No findings. One pre-existing judgement call left as is: |
Committing a drawn shape replaced the draft undo snapshot inside the label-and-flags step, before the group id and description from the dialog were assigned. The "current" snapshot therefore lacked them, so undoing the next change (for example another new shape) also wiped the earlier shape's group id and description. Take the snapshot once, after every dialog field is applied, so the last history entry equals the current state as on the other edit paths. Fixes #2676
79f9652 to
e2e8ba2
Compare
|
Second agent review (standards + spec axes) at e2e8ba2, verdict: recommend-merge. Rebased onto main to resolve the conflict from the private-submodule rename in #2678; the diff is otherwise the same as before except one comment reworded to state why the snapshot is retaken instead of restating the two calls beneath it. Verified against the code, not the description: the regression test fails with the fix reverted and passes with it. Unit tests plus the undo, label-edit, shape-editing, last-label, dialog-flags, action-availability, auto-save, annotation, and AI text-to-annotation e2e suites pass locally (1273 passed). Lint and ty check pass. Spec axis: no missing or wrong requirements, no scope creep. The issue's repro says "Save" between steps and the test skips that; saving never touches the undo stack, so the omission does not reduce coverage. Standards axis: no remaining findings. The direct pop on the canvas backup deque mirrors the pre-existing dialog-cancel branch a few lines up, so no new abstraction was added. |
Fixes #2676
What was wrong
Same bug class as #2638, on the shape-creation path. When a drawn shape was committed,
set_last_labelreplaced the draft undo snapshot right after assigning the label and flags, and only afterwards did_on_new_shapeassign the group id and description from the dialog. The "current" undo snapshot therefore had the new shape with no group id and no description, so undoing the next change (for example another new shape) also wiped them from the earlier shape.Fix
set_last_labelno longer touches the undo history._on_new_shapereplaces the draft snapshot once, after every dialog field is applied, so the last history entry equals the current state as on the other edit paths.Tests
tests/e2e/undo_after_new_shape_test.py: draw a polygon and give it group id7and descriptionfirstin the dialog → draw another polygon → undo. Asserts only the second polygon is gone and the first keeps its group id and description. Fails onmain, passes with the fix.Also ran
tests/unitand theundo_after_label_edit,shape_editing,last_label_and_restore,label_dialog_flags,action_availability,auto_save,annotation,ai_text_to_annotatione2e suites (QT_QPA_PLATFORM=offscreen), plusmake lintandty check.