Skip to content

Undo/redo for annotations, frictionless delete/merge, exclusive tools, dataset-splitter fixes (#80, #81)#82

Closed
cofade wants to merge 41 commits into
bnsreenu:masterfrom
cofade:feature/undo-redo-tool-mode-splitter
Closed

Undo/redo for annotations, frictionless delete/merge, exclusive tools, dataset-splitter fixes (#80, #81)#82
cofade wants to merge 41 commits into
bnsreenu:masterfrom
cofade:feature/undo-redo-tool-mode-splitter

Conversation

@cofade

@cofade cofade commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Bundles seven related changes across three themes. Full test suite green: 380 passed, 3 pre-existing skips (+116 new tests). Reviewed by the repo's senior-reviewer gate; two P1s found and fixed (mid-gesture undo guard; dead renumber in restore), re-reviewed clean.

1–2. Dataset splitter (dialogs/dataset_splitter.py)

  • Setting test percentage to 0% still produces exactly 1 test frame #81 — Setting Test % to 0 no longer produces a stray 1-image test set. New compute_split_counts() distributes the flooring remainder only among subsets with pct > 0, and the test slice is now explicitly bounded. Counts always sum to N; a 0% subset gets exactly 0.
  • Dataset splitter allows for shares not matching 100% #80 — The three percentage spin boxes auto-balance to 100%: changing train adjusts val (spilling into test); changing val adjusts test then train; changing test adjusts val then train. The old "must add up to 100%" error is now an unreachable safety net.

3–4. Frictionless delete & merge (controllers/annotation_controller.py)

  • Delete: removed both the "Are you sure?" confirmation and the "N deleted" success dialog — deletion is instant.
  • Merge: removed the keep/delete prompt (always replaces the originals with their union) and the success dialog.
  • Both are safe because they're now reversible (below).

5. Undo / redo for annotation edits — ADR-026

A new controllers/annotation_history.py (AnnotationHistory) keeps snapshot-based, per-image-key undo/redo stacks. Restoring a whole-image snapshot sidesteps the value-equality / renumber / segmentation_raw subtleties a command-replay design would have to reproduce.

  • Covers every annotation edit: create (polygon/rectangle/paint/SAM/DINO accept), delete, merge, move/scale (editing rectangle. #40 handles), change-class, eraser, Detail % (Masking tool sugestion #24), and polygon vertex edits (double-click).
  • One record_history() choke-point before each synchronous mutation; deferred gestures (bbox drag, paint stroke, vertex edit) capture a baseline at gesture start (editBaselineRequested) and push it at commit, with a deep-equal dedup so aborted gestures leave no entry.
  • Vertex edit also got a save-discipline fix: its commit now syncs all_annotations (it previously relied on a later save), and Esc now reverts the in-place vertex drags instead of silently keeping them.
  • Detail-% drags coalesce into a single undo entry.
  • Ctrl+Z / Ctrl+Y / Ctrl+Shift+Z as ApplicationShortcuts (the annotation-list table would otherwise eat Ctrl+Z); no-op during project load, an open modal, text-field focus, or an in-flight gesture. Undo persists via auto-save.

6. Mutually-exclusive tools (F)

A single ImageAnnotator.activate_tool() choke-point keeps current_tool, the SAM flags, and the toolbar button checks in sync, so a SAM tool can no longer be active at the same time as a manual tool. toggle_tool and the SAM toggles all delegate here.

7. Esc → selection mode (G)

Pressing Esc now cancels any in-progress shape and deactivates the active tool, returning the canvas to selection mode (its default). Previously the tool stayed selected.

Tests

tests/unit/test_dataset_splitter_math.py, tests/ui/test_splitter_balance.py, tests/unit/test_annotation_history.py, tests/integration/test_annotation_history.py (incl. mid-paint-undo block, number-parity, and vertex-edit commit/Esc guards), tests/integration/test_tool_mode.py; smoke module list extended.

Docs: ADR-026, crosscutting concepts (tool activation + Esc), runtime view, CLAUDE.md shortcuts + patterns table.

🤖 Generated with Claude Code

cofade and others added 30 commits June 12, 2026 22:44
Records the triaged open upstream issues (validated 2026-06-12) with size
classification. Each row is deleted in the PR that resolves it; the whole
section is removed once empty.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Captures the method of writing triaged work items into CLAUDE.md as a
temporary table with a deletion hook: each resolving PR deletes its row,
and the section removes itself once empty. Un-ignores .claude/skills/ so
project skills are tracked like the senior-reviewer agent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a combo above the image list (All / Without annotations / With
annotations). Rows are hidden via setRowHidden — never removed — so
row-index iteration (DINO navigation, COCO import) stays valid and no
spurious currentRowChanged/switch_image fires. The current row is never
hidden. Re-apply hooks into ClassController.update_slice_list_colors,
which every annotation-mutation site already calls. Multi-dim images
count as annotated when any of their slices has annotations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bnsreenu#57)

torch.cuda.is_available() is True for Pascal GPUs (e.g. GTX 1050,
sm_61) even though torch>=2.8 wheels ship sm_70+ kernels only, so every
launch died with 'CUDA error: no kernel image is available'. New
core/torch_utils.resolve_torch_device() compares the device capability
against torch.cuda.get_arch_list() and returns cpu + an actionable
warning on mismatch (cached process-wide). SAM passes device= on every
predict, DINO's _resolve_device delegates to the helper (DINO_DEVICE
override kept), YOLO train/predict pass device=. One-time QMessageBox
via maybe_warn_cpu_fallback at SAM model pick and DINO detect. README
gains a Pascal/Maxwell note with the torch-downgrade command.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the device-fallback concept and the hide-don't-remove image-list
rule to crosscutting concepts, registers core/torch_utils and the
filter responsibility in the building block view, and removes the #27
and bnsreenu#57 rows from the CLAUDE.md backlog per its deletion hook.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Comment/doc now state the real re-apply contract (direct
  update_slice_list_colors call OR annotationsBatchSaved route) instead
  of overclaiming that every mutation site calls it directly.
- apply_image_filter early-outs on the default 'All images' mode so the
  per-refresh annotation scan only runs with an active filter.
- Integration test exercising the real commit path
  (annotationsBatchSaved -> save -> color refresh -> filter re-apply).
- torch_utils: document the deliberate device-0-only probe; note the
  _8bit-key caveat in the prefix fallback; register core.torch_utils in
  the import smoke list.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Manual testing feedback: under an active filter, a selected image that
doesn't match (e.g. unannotated under 'With annotations') stayed in the
list until the user navigated away. Drop the never-hide-current-row
exemption — the current row is now hidden like any other non-match.
Hiding doesn't touch current_image or fire currentRowChanged, so the
canvas keeps showing the worked-on image while its row leaves the list,
which is the requested behavior. New integration test asserts the
canvas-unchanged / no-switch_image guarantee; unit test updated to the
new expectation; arc42 updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nd-pascal-gpu-fallback

feat: Image-list annotation filter (bnsreenu#27) + CPU fallback for unsupported CUDA GPUs (bnsreenu#57)
model.export() converts to a deployment format and crashes when given a
.pt save path; model.save() writes the checkpoint as intended. Reporter-
verified fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nsreenu#44)

On Windows, open() defaults to the cp1252 code page, so any YAML/JSON/TXT
containing non-ASCII (e.g. a unicode class name, or a non-ASCII path in
an .iap project) crashed with 'charmap codec can't decode/encode'. Add
encoding='utf-8' to every text-mode open across YAML (yolo_trainer,
import/export_formats, dataset_splitter), JSON (project load/save,
COCO import/export, augmenter, combiner, dino merge, project search),
and YOLO label .txt writes. PIL Image.open (binary) left untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…u#33)

start_polygon_edit returned the first polygon containing the double-click,
so an annotation fully nested inside another could never be edited. Scan
all containing polygons and pick the smallest by area (reusing the
existing shoelace calculate_area from utils.py). bbox-only annotations
remain out of scope (that's bnsreenu#40).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New ImageController.sort_image_list orders all_images and the list
widget together (case-insensitive), keeping the all_images[i] <->
image_list.item(i) invariant that COCO import relies on. Rebuilds with
signals blocked rather than setSortingEnabled, since currentRowChanged
is wired to switch_image and a live re-sort would fire spurious image
switches. Wired into add_images_to_list (selects/switches to the first
added image), update_image_list, and the COCO import path. Skipped per
image during project load to avoid O(n^2) re-sorts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…enu#56)

Reading an LZW-compressed TIFF without the optional imagecodecs package
raised ValueError and crashed the app. Add imagecodecs to
install_requires (fixes new installs) and catch the codec ValueError in
add_images_to_list: show an actionable 'pip install imagecodecs' dialog
and skip the file instead of crashing or leaving a half-added entry.
Non-codec ValueErrors still propagate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ard test, execute backlog hook

Adds the image-list sort and TIFF-codec concepts to crosscutting docs,
updates the ImageController building-block row, adds a UTF-8 round-trip
regression test for bnsreenu#44, and removes the five resolved quick-win rows
(bnsreenu#30/bnsreenu#44/bnsreenu#33/bnsreenu#60/bnsreenu#56) from the CLAUDE.md backlog per its deletion hook.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes unused/duplicate imports, placeholder-less f-strings, and one
unused local in files modified by this PR, so ruff check --select F,E9
passes cleanly. No behavior change; full suite still green (156 passed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- _is_missing_codec_error: drop the over-broad 'compression' substring
  arm that could swallow unrelated ValueErrors behind a misleading
  'install imagecodecs' dialog; match only the reliable 'imagecodecs'
  token (bnsreenu#56).
- sort_image_list: use info.get('file_name','') so the sort key is total
  even if an entry ever lacks file_name (bnsreenu#60).
- image_label: hoist 'from ..utils import calculate_area' to module scope
  per ADR-016 (no inline imports that rot after module moves).
- Add test_project_load_path_ends_sorted pinning the load -> update_ui
  rebuild-sorts contract.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nsreenu#56)

Locks in the P1 narrowing from the prior commit so a future broadening
of _is_missing_codec_error is caught.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ultralytics ships no SAM trainer (SAM.train() -> NotImplementedError), so
this adds a custom decoder (optionally encoder) fine-tuning loop that reuses
Ultralytics' own SAM2Predictor forward methods (get_im_features /
prompt_inference) under autograd, with focal+dice loss + AdamW. Fine-tuned
checkpoints save as {"model": state_dict} and reload through the unchanged
SAM(path) inference path, appearing in the SAM model selector.

- training/sam_trainer.py: SAMFineTuner engine + geometry/loss/naming helpers
- training/sam_dataset.py + io/export_formats.export_sam_dataset: project and
  prepared-folder dataset producers
- dialogs/sam_trainer_dialog.py: config dialog (reuses TrainingInfoDialog)
- controllers/sam_train_controller.py: menu, GPU gate, SAMTrainingThread,
  selector registration; wired in annotator_window
- inference/sam_utils.py: custom-model registry so fine-tuned models load by path
- tests/integration/test_sam_finetuning.py: geometry/loss/dataset/API-drift
  guards + opt-in end-to-end (SAM_TRAIN_E2E=1)
- docs: ADR-021, building-block, runtime, glossary; CLAUDE.md structure

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Encoder path: backprop once per image (sum instance losses) so the shared
  encoder feature graph isn't freed mid-loop; batch_size now accumulates over
  images. Fixes "backward through the graph a second time" crash on any image
  with >1 instance when fine-tuning the image encoder.
- Concurrency: lock SAM inference UI (tools, model selector, fine-tune menu)
  for the duration of a run; restore in training_finished on success AND error.
  The trainer uses its OWN SAM instance — corrected the false "drives the
  resident model" narrative in code docstrings, ADR-021, and runtime view; the
  real hazard is two models on one CUDA context.
- Threading: convert in-memory slice QImages to numpy on the GUI thread before
  handing to the worker (matches _qimage_to_numpy cross-thread contract).
- Harden export_sam_dataset path components with os.path.basename.
- Add opt-in encoder-path multi-instance regression test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- _launch wraps setup-through-start() in try/except so a failure before the
  worker thread starts re-enables the SAM inference UI (otherwise tools stay
  disabled until restart — training_finished was the only other unlock site).
- export_sam_dataset slice lookup uses `is not None` (consistency with
  sam_dataset.py).
- Add controller tests: UI lock toggle + lock-recovery on setup failure.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Unicode → in the progress print raises UnicodeEncodeError on a Windows
cp1252 console. Use -> so a console-launched run can't crash on the export log.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Found during real GUI testing on non-square images.

- Mask shift (correctness): training loss mapped decoder logits to the image
  with a naive F.interpolate, but SAM2 letterboxes (pad bottom/right) and
  inference crops that padding via ops.scale_masks(padding=False). The decoder
  learned padding-shifted targets -> masks shifted down on non-square images.
  Fix: run logits through the SAME ops.scale_masks(padding=False) inference
  uses. Validated on a landscape image: fine-tuned mask centroid dY=-0.1px,
  IoU=0.994 (was visibly shifted). Square test images had hidden the bug.
- Progress dialog: clear the reused log on launch; disable Stop on completion
  so a post-finish click can't strand it on "Stopping...". SAM instance only;
  shared TrainingInfoDialog class untouched (YOLO unaffected).
- Device label: show GPU name (cuda:0 read as "not detected").
- Light mode: drop inline color:palette(text) on the config-dialog note and the
  pre-existing DINO sidebar caption (No Hardcoded Colors Rule).
- Tests: ops.scale_masks padding=False crop-geometry guard + opt-in landscape
  shift regression. ADR-021 coordinate-frame consequence documented.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The loaded canvas + updated window title already make a successful open
obvious; the modal was just an extra click. Error dialogs unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat: Fine-tune SAM 2 / 2.1 on user annotations (bnsreenu#73)
The ClassThresholdTable header styled itself inline with
background-color: palette(mid); color: palette(text). Those palette()
references resolve against the OS palette (dark on Windows), so in the
app's light mode the header painted dark-on-light, out of step with the
rest of the UI. Light mode (default_stylesheet) also had no
QHeaderView::section rule at all, while dark mode did.

Strip the inline palette() colours (keep only structural font-weight /
padding) and add a light QHeaderView::section rule to default_stylesheet,
symmetric with the dark sheet. Header colour now follows the active theme.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bnsreenu#75)

Give the canvas the same selection power the annotation list already had,
and make selection legible.

Selection (idle mode — no drawing/SAM tool active):
- single click selects the smallest mask under the cursor (segmentation
  or bbox); click on empty space clears
- Shift+click toggles a mask; drag draws a rubber band that box-selects;
  Shift+drag adds. Esc cancels an in-progress band. Ctrl stays pan.
- Delete removes the selected mask(s). Canvas selection is unified with
  the annotation list via AnnotationController.apply_canvas_selection,
  which mirrors onto the QListWidget (signals blocked) and matches by
  dict value-equality; Delete/Merge/Change Class are reused unchanged.
  Canvas Delete gates on the list selection (CanvasContext) so a stale
  highlight can't pop a spurious warning.

Selection rendering — no longer recolours the mask red (invisible on a
red-class mask). The mask keeps its class colour; a class-colour-
independent overlay is drawn last: a dashed selection-blue bounding box
plus bright handle squares at corners/edge-midpoints (open-garden-planner
CAD style; handles are visual-only, resize is upstream bnsreenu#40).

Class colours: default palette no longer starts on red — core/constants
default_class_color (curated tab10-style, red last, muted) replaces the
Qt.GlobalColor(n % 16 + 7) formula at the 4 assignment sites. Default
fill opacity 0.3 -> 0.2 so masks don't bury the image. Saved projects
keep their persisted colours.

Tests: unit (hit-testing, gesture resolution, Esc-cancel, render-no-
recolor, palette) + integration (controller selection mirror, canvas-
delete gating). arc42: ADR-022 (+amendment), runtime, crosscutting,
glossary, CLAUDE.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Canvas mask selection, multi-delete & visible selection styling (bnsreenu#75)
…nu#32, bnsreenu#36)

Bundles three related annotation-geometry fixes:

bnsreenu#40 — Direct-manipulation bbox editing. Selecting a single "bbox" annotation
now makes its 8 selection handles (from bnsreenu#75) draggable: drag a handle to
resize (opposite side anchored, stays rectangular, >=1px), drag the interior
to move. No new mode, modeled on open-garden-planner's ResizeHandle (anchor-
from-handle + per-handle cursors). Move is drag-gated past the click threshold
so a plain click still selects (nested-mask click-through preserved). On
release the box is clamped and persisted via bboxEditCommitted ->
commit_bbox_edit; Esc reverts.

bnsreenu#32 — Clamp manual edits. Polygon vertex edits and bbox drags now clamp
coordinates into the image rectangle on commit (clamp_segmentation/clamp_bbox,
per-coordinate so vertex count is preserved). Drawn shapes were already clipped.

bnsreenu#36 — Clip augmented polygons. The Image Augmenter clips each transformed
polygon to the image via a shapely intersection (clip_polygon_to_bounds,
buffer(0) repair, largest part); polygons left fully outside are dropped.

Geometry helpers live in utils.py. Docs: ADR-023 (bbox editing) + ADR-024
(clamp vs clip), runtime view, crosscutting, glossary, CLAUDE.md patterns +
shortcuts; backlog rows removed. Tests: 30 new (geometry, bbox-edit lifecycle,
controller persistence, augmenter clip). 234 passed, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p collapse

P1 (data loss): a bbox selected via the annotation LIST stored a value-equal
copy (item.data(UserRole) round-trips as a copy) in highlighted_annotations, so
the in-place handle-drag mutated a throwaway object and the edit was silently
lost. _single_selected_bbox now resolves to the live object inside
image_label.annotations by value-equality; _commit_bbox_drag re-points
highlighted_annotations at that live object so re-selection survives the rebuild.

P1 (geometry): clamp_bbox collapsed a moved box that crossed the top/left edge to
a 1px sliver (independent-corner clamping is right for resize, wrong for move).
Added fit_bbox_inside (size-preserving translate-inside) and use it for the move
path; clamp_bbox stays the resize/trim clamp.

P2: clip_polygon_to_bounds now drops shapely's duplicate closing vertex to match
the app's unclosed flat-ring convention.

Tests: +7 (fit_bbox_inside, top/left-off move preserves size, live-object
resolution, list-selection-after-switch persistence). 241 passed, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cofade and others added 11 commits June 21, 2026 21:26
- Split _single_selected_bbox (cheap, geometry-only, safe per-hover) from
  _live_annotation (the value-equality scan that resolves to the saved object);
  the press handler now resolves only on drag-arm, so the O(n) scan no longer
  runs on every hover frame.
- Comment that the post-commit highlighted_annotations reassignment is a no-op
  for canvas-click selection and only matters for a stale list-selection copy.
- Note the exact-tuple == in clip_polygon_to_bounds' closing-vertex drop.

Re-review verdict: merge-ready, no P0/P1. 241 passed, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ped annotations

Manual GUI testing found the handles inert on every shape: the feature only
armed for annotations with a "bbox" key, but in-app shapes (drawn rectangles,
polygons, SAM/DINO masks) are all stored as "segmentation" — only imported
COCO/YOLO boxes get a bbox key. So _single_selected_bbox always returned None
and the drawn handles did nothing.

Generalize to any single selected shape (user-confirmed: resize scales, drag-
inside moves):
- _single_selected_shape: any single annotation with a bounding box (seg or bbox).
- _begin_shape_edit records kind: "seg" scales/translates vertices
  (_scale_segmentation / _translate_segmentation); "bbox" edits [x,y,w,h].
- _sync_bbox_key keeps an imported annotation's bbox consistent with its edited
  segmentation (feeds export/training).
- Commit clamp by kind+mode: polygon move slides inside (shape-preserving) then
  clamp_segmentation as a safety net; polygon resize clamp_segmentation; box
  move fit_bbox_inside; box resize clamp_bbox.

Docs (ADR-023, ADR-022 amendment, runtime, crosscutting, glossary, CLAUDE.md)
updated to "shape editing". Tests: +6 (polygon scale/move/clamp/bbox-key-sync,
polygon resize persistence). 247 passed, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Senior-review follow-ups:
- _annotation_bbox / _annotation_contains crashed (None[0::2]) on imported
  bbox-only annotations, which carry "segmentation": None — exactly bnsreenu#40's
  original target, and the hover/select path now calls _annotation_bbox on
  every selected annotation. Both now fall through to the bbox key when
  segmentation is None/empty. Test locks selectable + resizable.
- Refresh the stale self.bbox_edit init comment (now lists kind/orig_seg, "shape")
  and the hover-cursor comment (bbox -> shape).

248 passed, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nsreenu#24

Closes the remaining piece of #24 (the "mask complexity — less ↔ more points"
control; point add/remove was already covered by the SAM-points tool).

- utils.simplify_polygon: Douglas-Peucker (cv2.approxPolyDP) thinning to a
  Detail % vertex budget (100 = raw), binary-searched epsilon.
- Annotations panel: QListWidget → QTableWidget (ID | Class | Area | Detail %),
  mirroring the DINO ClassThresholdTable idiom (per-row spinbox, SelectRows,
  NoEditTriggers, stylesheet-only header). Col 0's UserRole holds the annotation
  (the bnsreenu#75 value-equality marker). Selection mirror uses setRangeSelected
  (additive) — selectRow() replaces in ExtendedSelection. count/item/selectedItems
  → rowCount/item(r,0)/row-deduped selectedIndexes across the bridge + CRUD.
- Per-row Detail % spinbox → on_detail_pct_changed: resolves the live drawn
  object (_live_annotation), lazy-captures segmentation_raw, simplifies (or
  restores raw at 100), recomputes bbox, refreshes Area+UserRole in place, saves.
  valueChanged connected after the initial setValue so table builds don't fire it.
- New keys segmentation_raw + detail_pct round-trip via .iap (ann.copy → JSON);
  exports emit the effective (simplified) segmentation.

Docs: ADR-025, runtime, crosscutting, glossary; CLAUDE.md pattern row + backlog
reconciled (drop closed bnsreenu#63 + done #24, add bnsreenu#74). Tests: +11 (simplify unit +
table/persistence/export integration); existing bnsreenu#75/bnsreenu#40 selection tests adapted
to the table. 259 passed, ruff clean on changed files.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
P1 (data loss): reshaping a thinned polygon with the bnsreenu#40 handles orphaned its
segmentation_raw, so a later Detail %=100 reverted the reshape. _clamp_edited_shape
now invalidates the baseline on a seg edit (drop segmentation_raw, reset
detail_pct=100) — the edited geometry becomes the new raw.

P1 (stale selection): on_detail_pct_changed mutated the live object but left
highlighted_annotations holding the pre-simplify value, so the selection overlay
drew stale geometry and a subsequent handle drag (_live_annotation) couldn't
re-match. Now re-points highlighted_annotations at the mutated object.

P2: corrected the binary-search comment (keeps the richest polygon within budget)
and ADR-025's reversibility note (reshape resets the raw baseline).

Tests: +2 seam tests (thin→reshape→baseline-invalidated; detail-change keeps
selection resolvable for a handle drag). 261 passed, ruff clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Senior re-review round 2 (production fixes confirmed correct):
- test_detail_change_keeps_selection_resolvable_for_handle_drag selected by
  identity (apply_canvas_selection), so it passed pre-fix too. Now selects via
  the table (update_highlighted_annotations stores a UserRole *copy*), so it
  actually exercises the stale-highlight bug and fails on pre-fix code.
- Delete the orphaned delete_annotation (controller method + window delegate,
  no callers): its removeRow-without-rebuild was incompatible with the index-
  bound per-row spinbox lambdas — a tripwire for the next single-row-delete.
- Glossary: Annotation List is a QTableWidget now, not a QListWidget.

261 passed, ruff clean on changed files.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat: Reversible per-annotation mask complexity (Detail %) — closes #24
… fixes

Bundles seven related changes:

Dataset splitter (dialogs/dataset_splitter.py)
- bnsreenu#81: compute_split_counts() + bounded slices so a 0% subset yields
  exactly 0 images (was leaking the flooring remainder into test).
- bnsreenu#80: auto-balance the train/val/test spin boxes so they always sum
  to 100 (val absorbs train; test→train absorb val; etc.).

Frictionless destructive ops (annotation_controller.py)
- Delete: removed the confirmation AND success dialogs — instant.
- Merge: removed the keep/delete prompt (always replaces originals
  with the union) and the success dialog.

Undo/redo for annotation edits (ADR-026)
- New controllers/annotation_history.py: snapshot-based, per-image-key
  undo/redo stacks with deep-equal dedup and a depth cap.
- record_history() choke-point before every synchronous mutation
  (finish poly/rect, delete, merge, change-class, eraser, SAM/DINO
  accept); deferred bbox/paint/temp gestures capture a baseline at
  start (editBaselineRequested) and push it at commit.
- Detail-% drags coalesce to one undo entry.
- Ctrl+Z / Ctrl+Y / Ctrl+Shift+Z as ApplicationShortcuts; undo/redo
  no-op during load, modal, text focus, or an in-flight gesture.

Tool mode (annotator_window.py, sam_controller.py, image_label.py)
- F: single activate_tool() choke-point makes all six tools (incl.
  SAM) mutually exclusive — a SAM tool can no longer be active
  alongside a manual tool.
- G: Esc cancels the in-progress shape AND returns to selection mode
  (selectModeRequested -> activate_tool(None)).

Docs: ADR-026, crosscutting concepts (tool activation, Esc), runtime
view, CLAUDE.md shortcuts + patterns table.

Tests: +114 (splitter math/balance, AnnotationHistory unit, history
integration incl. mid-paint-undo + number-parity guards, tool mode).
378 passed, 3 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Vertex edits (double-click polygon edit) were the one annotation edit
Ctrl+Z couldn't reverse. Their Enter-commit only refreshed the list and
relied on a later save to persist, and Esc silently kept the in-place
vertex drags instead of reverting them.

- start_polygon_edit captures the segmentation + emits editBaselineRequested
  (undo baseline) on entry.
- New polygonEditCommitted signal -> commit_polygon_edit syncs
  all_annotations (save_current_annotations) + pushes the baseline +
  refreshes the list. Gated on an actual geometry change.
- Esc now reverts the drags from the captured original; _editing_polygon_orig
  is cleared on commit/cancel/delete/exit/clear.

Tests: vertex-edit commit->undo and Esc-reverts-without-history.
380 passed, 3 skipped. ADR-026 + CLAUDE.md updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
switch_slice didn't call exit_editing_mode the way switch_image does, so
editing a polygon then switching slice left editing_polygon and the new
_editing_polygon_orig pointing at the previous slice's object (stale
overlay). One-line parity fix; closes the leak for multi-dim stacks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants