Low-vision accessibility mode, modular architecture refactor, and QA fixes#70
Merged
bnsreenu merged 49 commits intoJun 11, 2026
Merged
Conversation
* chore: Add PyQt6 + Torch coexistence smoke test (Phase 0 gate) Validates the central hypothesis of the upcoming subprocess-removal work: that PyQt6 sidesteps the WinError 1114 DLL load-order conflict on Windows + Python 3.14 that motivated sam_worker.py / dino_worker.py (see ADR-011). Run manually before deleting any worker code. Exit code 0 unblocks Phase 2 of the PyQt5 -> PyQt6 + in-process inference migration. https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * feat: Migrate from PyQt5 to PyQt6 (Phase 1 of in-process inference PR) End-to-end migration of the GUI binding. 34 files, ~360 lines changed. All 65 tests still pass on PyQt6 6.11; the full app constructs and renders headlessly via QT_QPA_PLATFORM=offscreen. What changed ------------ - Dependency pins: PyQt5>=5.15 -> PyQt6>=6.7 (requirements.txt, setup.py) - Bulk import rewrite: `from PyQt5...` -> `from PyQt6...` (28 files) - Symbol relocations: * QAction moved from QtWidgets to QtGui (annotator_window.py) * QDesktopWidget removed -> QGuiApplication.primaryScreen() (snake_game.py) - Enum namespacing (Qt6 requires fully-qualified names everywhere): * Qt.AlignmentFlag / MouseButton / KeyboardModifier / Key * Qt.PenStyle / BrushStyle / CursorShape / GlobalColor * Qt.WindowType / WindowModality / FocusPolicy / TransformationMode * Qt.ItemDataRole / ItemFlag / ContextMenuPolicy / ScrollBarPolicy * Qt.TextFormat / TextInteractionFlag / MatchFlag / CheckState * QMessageBox.StandardButton / .Icon / .ButtonRole * QDialog.DialogCode, QFileDialog.AcceptMode / FileMode / Option * QAbstractItemView.SelectionMode / SelectionBehavior / EditTrigger * QHeaderView.ResizeMode, QSlider.TickPosition * QPainter.RenderHint, QImage.Format.Format_* * QDialogButtonBox.StandardButton / .ButtonRole * QKeySequence.StandardKey - Modern event API in image_label.py: event.pos()/.x()/.y() -> event.position() returning QPointF end-to-end. Scrollbar setValue() takes int() of the QPointF delta (the boundary). - Removed dead workaround in annotator_window.py: clearing WindowContextHelpButtonHint from dialog flags. Qt6 already suppresses this; the flag itself was removed. - exec_() -> exec() in main.py entry point. - CI: add libegl1/libgl1 to the Linux apt-install list (Qt6 needs them). - Docs: CLAUDE.md, README.md, docs/02_architecture_constraints.md updated to reflect PyQt6 and the relaxed Linux support story. Not touched ----------- - sam_worker.py, dino_worker.py, sam_utils.py, dino_utils.py subprocess pattern (Phase 2, gated on Win+Py3.14 validation). - exec_() call sites outside main.py (still work as deprecated alias). https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * feat: Remove subprocess workers, run SAM/DINO in-process with QThread wrapping Phase 2 of the PyQt6 migration. The DLL conflict that motivated the subprocess workers (ADR-011) no longer applies under PyQt6, so the ~1-2 s subprocess spawn per inference is gone, and models stay cached across calls. What changed ------------ - Deleted sam_worker.py (288 lines), dino_worker.py (231 lines), tools/check_worker_isolation.py (134 lines). - Rewrote sam_utils.py end-to-end: * SAMUtils inherits QObject, caches the Ultralytics model in self._model * change_sam_model() loads eagerly (on worker thread, UI stays alive) * apply_sam_points / apply_sam_prediction / apply_sam_predictions_batch all run inference on a QThread via the new _run_sync helper * Lazy import of torch/ultralytics keeps app startup snappy * Added unload() for future Tools-menu memory release - Rewrote dino_utils.py with the same pattern (DINOUtils as QObject, model cached across calls, transformers lazy-imported). - Added _run_sync: spawns a QThread, pumps the calling thread's QEventLoop until done. Public API stays synchronous so the existing call sites in annotator_window.py work unchanged. UI events (timers, redraws, progress dialog cancels) keep flowing during the wait. - Removed the stale "If you are on Python 3.14, PyTorch may not yet be fully supported" message in change_sam_model error path. - utils.py docstring: drop sam_worker reference. Docs ---- - ADR-011 marked Superseded, with pointer to ADR-013. - New ADR-013 documents the in-process + QThread decision, the latency win, and the trade-offs (re-entrancy via QEventLoop pump, no more crash isolation). - arc42 docs/05_building_block_view.md, docs/06_runtime_view.md, docs/12_glossary.md updated to drop subprocess wording. - CLAUDE.md SAM Integration section rewritten. Verification ------------ - 65 tests pass (pytest, QT_QPA_PLATFORM=offscreen). - Full app constructs and renders headlessly. - _run_sync round-trip verified end-to-end against a 0.3 s sleep. - Phase 0 PyQt6+torch+transformers+ultralytics coexistence smoke test passes on Linux+Py3.11. Windows+Py3.14 verification is the user's responsibility before this PR ships. https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * docs: Require senior reviewer to run in foreground, not background The quality gate is blocking by design — the next steps (address P0s, push, open PR) depend on its findings. Backgrounding it just defers the work and risks shipping unreviewed code. https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * fix: Address senior reviewer P0/P1 findings on PyQt6+in-process PR P0 — correctness ---------------- - _InferenceThread.run no longer swallows exceptions. Stores them on the thread instance; _run_sync re-raises on the calling thread. Silent model-load failures previously showed up as "No mask matches" / "No detections" dialogs with no way to diagnose. (sam_utils.py) - Added _inference_in_flight module flag + InferenceBusyError. The earlier QMutex draft would have deadlocked: same-thread re-acquisition of a non-recursive mutex hangs, and a recursive mutex would defeat the whole serialization point. A flag with an explicit exception surfaces re-entry instead of corrupting the model with concurrent .forward() calls. (sam_utils.py) - Added _sam_inference_in_flight guard in annotator_window.apply_sam_prediction — the SAM debounce timer can fire while a previous inference is pumping inside _run_sync; the guard skips the re-entrant call so the next click + debounce restart issues a fresh inference with the up-to-date point set. P1 — should-fix --------------- - dino_utils._detect_blocking no longer shuffles the model CPU<->GPU on every call. Moving a 1.9 GB DINO base over PCIe was wiping out the in-process caching gain the whole PR was meant to deliver. - change_sam_model now flips current_sam_model AFTER successful load, not before. On load failure the state stays consistent. - _qimage_to_numpy always returns a fresh copy. The fallback path was particularly broken: the converted QImage was local, would go out of scope at return, and the worker thread's numpy buffer would have aliased freed memory. - Updated misleading dino_utils.detect comment about marshaling — the safety actually comes from the .copy() inside _qimage_to_numpy, not from where it runs. - ADR-001 marked Superseded with pointer to new ADR-014. - New ADR-014 documents the PyQt5->PyQt6 migration decision. - Updated ADR-013 consequences to honestly describe the re-entrancy guards (replaced the "acceptable for now; revisit if users hit it" hand-wave the reviewer specifically called out). - docs/01_introduction_and_goals.md, docs/05_building_block_view.md (ASCII diagram), docs/06_runtime_view.md (app.exec_() typo) all updated for PyQt6. - CLAUDE.md: Testing Checklist no longer references deleted sam_worker /dino_worker; senior-reviewer agent prompt no longer references deleted check_worker_isolation.py. - .claude/agents/senior-reviewer.md retargeted from PyQt5 -> PyQt6 and rewritten to check ADR-013 re-entrancy guards instead of ADR-011's retired subprocess isolation. P2 — opportunistic ------------------ - tools/check_pyqt6_torch_coexistence.py now constructs a QApplication after importing torch. Pure import alone does not load Qt's native platform plugin (qwindows.dll on Windows) — which is the actual site of the historical WinError 1114. The previous green result was a false positive on the strictest test. - CI matrix gains Python 3.13. ADR-013 claims PyQt6+torch coexist on modern Pythons; this adds CI evidence (Py3.14 still manual via the coexistence script — pip wheels not yet broadly available). - CI apt-install list deduped (libxcb-cursor0 was listed twice). Verification ------------ - 65 tests pass. - Smoke-tested both fixes: exception propagation works (boom() raises ValueError out of _run_sync); re-entry detection works (timer-driven inner call raises InferenceBusyError while outer is pumping). - coexistence script with QApplication construction passes end-to-end on Linux+Py3.11. https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * fix: Address second-pass reviewer findings on PyQt6 PR P0 — correctness (regression unblocked by the first round's fix) ---------------------------------------------------------------- - annotator_window.py: import traceback at module level. The except block at the DINO call site (line 3022 pre-edit) referenced traceback.print_exc() without the module being importable in scope. Before the previous fix dino_utils.detect() returned None on error so the except was rarely entered; now it raises for real, so the NameError was about to start firing and leave the detect buttons permanently disabled with no user-visible dialog. P1 — should-fix --------------- - annotator_window.apply_sam_prediction now catches inference exceptions. The slot is driven by a QTimer; before this patch a CUDA OOM or InferenceBusyError would fall out into PyQt6's default unhandled-slot handler (stderr only). InferenceBusyError is suppressed silently (defense-in-depth alongside the call-site flag); other exceptions show a critical QMessageBox. - Same wrapping added to the unprotected SAM-batch calls inside both DINO flows (single image at line 3063, per-image loop at 3170). - Wired SAMUtils.unload() and DINOUtils.unload() to a new Tools menu entry "Unload AI Models (Free GPU Memory)". The DINO CPU<->GPU shuffle was removed in the previous round, which removes the automatic between-call free; this gives users on constrained GPUs a manual recovery path. - Bumped version 0.8.12 -> 0.9.0 in setup.py and __init__.py to signal the binding change (PyQt5 -> PyQt6) and the in-process inference rework. Anyone reading the wheel changelog now sees the binding switch in the version. - docs/05_building_block_view.md SAMUtils block rewritten to match the actual class shape (sam_model -> _model, qimage_to_numpy is a module-level helper not a method, _run_sync added). - Deleted PYTHON314_SETUP.md — it described the migration as future work, in the present tense, with the now-retired DLL workaround as a known issue. Easier to delete than keep coherent. P2 — cleanup ------------ - Dropped the unused `import traceback` in sam_utils.py (_InferenceThread captures exceptions on the instance now; no printing inside the worker). - The "No mask generated." batch fallback now builds a fresh dict per bbox via list comprehension instead of `[d] * N` (avoided shared-reference footgun). - Removed the dead `qimage_to_numpy` method on ImageAnnotator — module-level `_qimage_to_numpy` in sam_utils superseded it. - Folded the local `import traceback` inside `add_class`'s except block into the module-level import. Architectural belt-and-braces ----------------------------- - Added an assert at the top of `_run_sync`: the function MUST be called from the GUI thread. The `_inference_in_flight` flag is a plain global, not protected against cross-thread access — if a future contributor drives inference from a worker thread it becomes a true race. The assert is the tripwire. Reviewer flagged this as the kind of constraint that gets violated six months later when nobody remembers the design. Verification ------------ - 65 tests still pass. - Exception propagation and re-entry detection both re-tested in the full-app context — outer call returns 'done', timer-driven inner call raises InferenceBusyError, both as designed. - App constructs and renders headlessly. https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * fix: Third-pass reviewer findings (TESTING.md + assert tripwire) P1 -- - TESTING.md: removed the "Known Issues — Python 3.14 + PyTorch Compatibility" section (the WinError 1114 it described is gone with the PyQt6 migration), removed the "Milestone 1.2: PyQt6 Migration" future-work entry (the migration is done), and added a brief "Headless Testing" section pointing at the CI deps list. Also bumped the CI Python row to mention 3.13. The file was not touched by earlier commits in this branch; the reviewer correctly pointed out that the branch is what made it wrong, so it's owed. P2 -- - Replaced the GUI-thread tripwire in sam_utils._run_sync with an explicit `if ...: raise RuntimeError(...)` instead of `assert`. `python -O` strips asserts; the tripwire was the kind of thing that would only matter once it had silently disappeared. Verification: 65 tests still pass. App still constructs. https://claude.ai/code/session_01ADoBX5VmUYpCrwbkecKMHL * fix: Manual-testing fix pack on PyQt6 + in-process inference branch Closes user-reported regressions and rough edges discovered during Windows manual testing of PR #4. Covers crashes, UX bugs, and silent failures that the 65-test pytest-qt suite doesn't exercise. PyQt5 → PyQt6 mechanical migration gaps: - 15× .exec_() → .exec() across annotator_window, dino_merge_dialog, image_patcher, project_search, snake_game, stack_to_slices. The QMenu crash on right-click in the class list (annotator_window:4607) was the first user-visible casualty. - Missing `import traceback` in dino_merge_dialog.py. - F2 (Snake game) moved from keyPressEvent to QShortcut(ApplicationShortcut) so QTableWidget's in-cell-edit doesn't swallow it. Canvas — pan + zoom-to-cursor: - Pan now uses event.globalPosition() so the widget shifting under the cursor mid-drag doesn't absorb half the delta (former half-speed pan). - New cursor-anchored Ctrl+wheel zoom; post-zoom offset derived analytically from viewport().width() instead of the stale self.width() that's wrong on zoom-out before layout settles. DINO panel + detection: - Threshold column widths (88 px fixed) + setFrame(True) so values "0,25" / "0,50" are readable. - PhraseEditorPanel auto-reveals on class-add; row-0 phrase is now renamable. Removed the silent class-name re-prepend in get_phrases_for + _run_for_class so a renamed row-0 actually reaches DINO. - Auto-accept dropdown now honored by both single + batch paths. - "Detect All Images" extended to multi-dim image slices via _collect_dino_batch_work_items (was silently skipping stacks). - New _navigate_to_image_or_slice handles slice names in batch review; orphan results are popped instead of leaving a half-state. - temp_annotations is a single field — _refresh_dino_temp_for_current syncs it on every switch_slice / switch_image so masks don't bleed between slices. - Application-wide _DINOReviewEventFilter makes Enter / Escape work during review regardless of which widget has focus. ADR-015 documents the choice over QShortcut and force-focus alternatives. - dino_batch_results initialised in __init__; dropped 4 lazy-hasattr checks. - Verbose [DINO] / [SAM] diagnostic prints at decision points (un-gated per user request — print is the project convention). Multi-dim TIFF loading: - load_tiff reads tifffile.series[0].axes and maps Y→H, X→W into the app's dimension vocab. DimensionDialog defaults to these hints when ndim matches. - Explicit ndim 3-6 fallback table, plus generic ["T"]*(ndim-2) + ["H","W"] for ndim ≥ 7. The earlier default_dimensions[-ndim:] of a 4-element list silently degraded for 5D TZCYX inputs and produced 2560 one-row "slices". Tools → Unload AI Models: - Three-step recipe: model.cpu() → gc.collect() → empty_cache + ipc_collect + synchronize. Disclosure dialog now mentions the per-process CUDA context that survives unload. - Resets both SAM + DINO dropdowns and disables Detect buttons on unload. YOLO export: - image_paths lookup uses exact-key match first, substring fallback only (prevents "bee.jpg" matching "honeybee.jpg" by substring). - Diagnostic [YOLO v5+] / [YOLO v4] prints, warning when a class isn't in class_mapping. Dark mode: - Dark mode now on by default at startup. - Removed hardcoded #e0e0e0 / #f5f5f5 from ClassThresholdTable header and lbl_dino_status (they punched bright boxes into the dark sidebar). - Added QRadioButton / QCheckBox / QHeaderView / QTableWidget / QSpinBox / QDoubleSpinBox / QComboBox / QGroupBox rules to soft_dark_stylesheet so dataset splitter radio buttons + DINO panel widgets render with adequate contrast. - Annotated-slice highlight changed from light blue (173,216,230) to muted steel-blue (58,95,140) on dark mode. Docs: - ADR-015 added — application-wide event filter for DINO review. - Cross-cutting concepts gained sections for Pan + Zoom Reference Frames, Dark Mode No Hardcoded Colors Rule, Releasing Model GPU Memory, DINO Temp Annotations (lifecycle / event filter / batch / navigation / auto-accept), Multi-dim TIFF Axis Defaults, Export Format Filename Matching. - CLAUDE.md gained a "Patterns introduced in v0.9.0" index table pointing at the arc42 deep-dives so new contributors don't re-derive them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
…(Phase 1)
First phase of the modular refactoring plan: mechanical re-homing of
25 self-contained modules into six subpackages, plus a smoke test that
locks in the new layout.
No logic changes. Every move is a `git mv` followed by an import-path
rewrite in the affected files.
Subpackages introduced:
- core/ constants, annotation_utils
- widgets/ image_label
- inference/ sam_utils, dino_utils
- io/ export_formats, import_formats
- ui/ default_stylesheet, soft_dark_stylesheet
- dialogs/ 15 standalone dialog/tool windows (statistics, splitter,
augmenter, registration, interpolator, patcher, dicom,
snake, help, project_search, project_details, phrase
editor, merge dialog, stack_to_slices, yolo_trainer,
coco_json_combiner)
Files staying at the package root: main.py, utils.py, annotator_window.py,
__init__.py. The public API surface (ImageAnnotator, ImageLabel,
SAMUtils, calculate_area, calculate_bbox) and the CLI entry points
(digitalsreeni-image-annotator, sreeni) are unchanged.
Tests updated for the new paths:
- tests/integration/test_export_formats.py: import from io.export_formats
- tests/unit/test_conversions.py: file-path loader points at widgets/image_label.py
New tests/integration/test_smoke.py asserts the public API and that
every internal module imports cleanly — the safety net for subsequent
refactor phases.
Result: 94 tests pass (47 unit + 18 integration + 29 new smoke).
Phases 2–9 (controller extractions, ImageLabel signal decoupling,
per-tool handlers, UI assembly extraction, docs) ship in follow-up PRs.
…tator (Phase 2) Second phase of the modular refactoring. Moves three self-contained method clusters out of the 5,900-line annotator_window.py into focused modules. ImageAnnotator keeps thin delegating methods so external callers (menu actions, signal connections, library consumers) keep working unchanged. New modules: - core/image_utils.py — pure NumPy/QImage helpers (no Qt main-window dependency): convert_to_serializable, normalize_array, adjust_contrast, convert_to_8bit_rgb, array_to_qimage. - ui/theme.py — theme + font-size application taking the main window as first arg: apply_theme_and_font, toggle_dark_mode, apply_stylesheet, update_ui_colors, setup_font_size_selector, on_font_size_changed, change_font_size. - controllers/io_controller.py — UI glue around the existing pure I/O format functions: import_annotations, export_annotations, save_slices. The format readers/writers themselves (io.export_formats, io.import_formats) are untouched. annotator_window.py shrinks from 5,900 to 5,476 lines (-424 lines, -7%). Dead imports of the stylesheet constants and the export/import format functions removed from the top of the file — they live with their new home modules now. No behaviour change. The .iap save/load schema is untouched (the delegating convert_to_serializable still produces the same JSON-ready dicts). ADR-013's in-process inference path is untouched. The is_loading_project guard remains in ImageAnnotator. Tests: all 94 pass (47 unit + 18 integration + 29 smoke). https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Third phase of the modular refactoring, split into two parts. This PR covers Phase 3a: extracting project-lifecycle methods. Phase 3b will extract image/slice loading into an ImageController in a follow-up PR. The original plan called for both controllers in one PR. Splitting makes the diff reviewable: 19 project methods is already ~550 lines of moved code, and ImageController would have added another ~25 methods including the multi-dimensional image handling that touches TIFF/CZI loaders. New controllers/project_controller.py (548 lines) — ProjectController(QObject) owns the project lifecycle: - new_project, open_project, open_specific_project, backup_project_before_open, restore_project_from_backup - load_project_data (the .iap reader, including DINO config restore) - save_project (the .iap writer), save_project_as, auto_save - close_project - handle_missing_images, remove_missing_images, prompt_load_missing_images, load_missing_images, check_missing_images - update_window_title ImageAnnotator now holds self.project_controller = ProjectController(self) and keeps thin delegating methods for each so menu actions and signal connections continue to work unchanged. State (is_loading_project, backup_project_path, current_project_file, current_project_dir, project_notes, etc.) deliberately STAYS on the main window in this phase. The controller reads/writes them via self.mw.X. A future phase can migrate ownership; for now this is pure method relocation and reduces risk. The is_loading_project guard (ADR-required, prevents autosave during load) is preserved: auto_save() still checks self.mw.is_loading_project, open_specific_project still flips it around load_project_data(). Methods NOT moved in this PR despite appearing in the plan: - clear_all — touches ~30 state attributes across every controller's domain; deferred until after all controllers exist so it can be decomposed properly - closeEvent — Qt method override on QMainWindow, conceptually belongs on the main window Dead imports of datetime and shutil dropped from annotator_window.py since their only consumers moved out. annotator_window.py: 5,476 → 4,944 lines (-532, -9.7%). Total reduction vs original master: 5,900 → 4,944 (-16.2%). Tests: all 94 pass (47 unit + 18 integration + 29 smoke). https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Second half of Phase 3 in the modular refactoring. Completes the work started by ProjectController (PR #7) by giving image and slice loading / navigation its own home. New controllers/image_controller.py (869 lines) — ImageController(QObject) plus the DimensionDialog class (moved out of annotator_window.py top level since it is only used by process_multidimensional_image). Methods relocated to ImageController: Loaders: - load_image (extension dispatch) - load_tiff (with axes-hint detection from TIFF series metadata — the bugfix from arc42 for ndim>=5 TZCYX inputs is preserved verbatim) - load_czi - load_regular_image - load_multi_slice_image (orchestrates TIFF/CZI multi-slice loading) - process_multidimensional_image (dimension assignment dialog flow) - create_slices (per-axis slicing + progress dialog) Navigation / activation: - switch_image, switch_slice, activate_slice, activate_current_slice - update_image_list, update_slice_list, clear_slice_list - add_slice_to_list, setup_slice_list Image lifecycle: - open_images, add_images_to_list, update_all_images - remove_image, delete_selected_image, redefine_dimensions - display_image - is_multi_dimensional State (current_image, current_slice, slices, image_paths, image_slices, image_dimensions, image_shapes, all_images, image_file_name, etc.) deliberately STAYS on the main window — accessed via self.mw.X. Same deferral as Phase 3a; future phase may migrate ownership. ImageAnnotator now holds self.image_controller = ImageController(self) and keeps thin delegating methods for each so menu actions and signal connections continue to work unchanged. DimensionDialog moved with the methods that use it. The pre-fill from TIFF series axes and the ndim>=7 fallback ("T"*N + ["H","W"]) are preserved verbatim — those are the fixes from arc42's "ndim≥5 [-ndim:] slice bug" note. Dead imports of czifile.CziFile and tifffile.TiffFile removed from annotator_window.py since their only consumers moved into the controller. annotator_window.py: 4,944 → 4,122 lines (-822, -16.6%). Total reduction vs original master: 5,900 → 4,122 (-30.1%). Tests: all 94 pass (47 unit + 18 integration + 29 smoke). https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Phase 4a in the modular refactoring: extract SAM tool / inference methods into a dedicated controller. Phase 4b (DINO) and Phase 4c (YOLO) follow as separate PRs. New controllers/sam_controller.py (267 lines) — SAMController(QObject) owns the SAM tool lifecycle (magic wand, box, points), the debounce timer state machine, and the model picker dropdown plumbing. Methods relocated: - activate_sam_magic_wand, deactivate_sam_magic_wand - toggle_sam_assisted, toggle_sam_magic_wand - toggle_sam_box, toggle_sam_points - schedule_sam_prediction, apply_sam_prediction, accept_sam_prediction - change_sam_model ADR-013 invariants preserved verbatim: - `_sam_inference_in_flight` flag set BEFORE calling sam_utils, cleared in `finally`. - `InferenceBusyError` swallowed silently — next user click restarts the debounce. - `change_sam_model` blocks via _run_sync event-loop pump. State stays on the main window (consistent with Phase 3): - sam_utils, sam_inference_timer, _sam_inference_in_flight, current_sam_model accessed via self.mw.X - External readers (image_label.py:1007 reads main_window.sam_inference_timer; clear_all resets current_sam_model; the sidebar button enabling logic reads current_sam_model) keep working unchanged. The sam_inference_timer.timeout signal connection in __init__ still points at self.apply_sam_prediction — that's now the thin delegate that calls into the controller. Single extra call frame, zero behaviour change. Dead import of InferenceBusyError removed from annotator_window.py since the only consumer moved into the controller. annotator_window.py: 4,122 → 3,913 lines (-209, -5.1%). Total reduction vs original master: 5,900 → 3,913 (-33.7%). Tests: all 94 pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Phase 4b in the modular refactoring: DINO (LLM-assisted detection) methods + the application-wide key event filter move into a dedicated controller. Phase 4c (YOLO) follows. New controllers/dino_controller.py (796 lines) — DINOController(QObject) plus the _DINOReviewEventFilter class. Methods relocated: - Model picker plumbing: _resolve_dino_model_path, _on_dino_model_changed, _ensure_dino_model_downloaded, browse_dino_model, on_dino_class_row_changed, _build_dino_class_configs - Detection workflows: run_dino_detection_single, run_dino_detection_batch, _collect_dino_batch_work_items, _commit_dino_results, _store_dino_batch_results, _show_dino_batch_review, _navigate_to_image_or_slice, _refresh_dino_temp_for_current, accept_dino_results, reject_dino_results - Temp-class review (shared with YOLO predictions): add_temp_classes, has_visible_temp_classes, verify_current_class, accept_visible_temp_classes, select_first_primary_class, reject_visible_temp_classes, check_temp_annotations, remove_all_temp_annotations The _DINOReviewEventFilter (ADR-015) moves alongside its only caller. It still installs into the QApplication during ImageAnnotator.__init__; the installation continues to call accept_dino_results / reject_dino_results on the main window, which now delegate into the controller. Behaviour preserved. State stays on the main window (consistent with prior phases): dino_utils, dino_model_loaded, dino_custom_model_path, dino_batch_results, all DINO widgets (phrase panel, threshold table, model selector, batch mode combo, status label, browse row, detect buttons) accessed via self.mw.X. External readers (clear_all touches dino_model_loaded / dino_custom_model_path; load_project_data restores dino_batch_results via add_temp_classes; ImageController calls self.mw._refresh_dino_temp_for_current() from switch_image / switch_slice) keep working unchanged. Also fixes 4 dormant import bugs left over from Phase 1's file moves — local imports inside method bodies that weren't updated: - .dino_utils → .inference.dino_utils (in _resolve_dino_model_path) - .project_search → .dialogs.project_search (in show_project_search) - .annotation_statistics → .dialogs.annotation_statistics - .project_details → .dialogs.project_details These didn't fail tests because they only run when their containing method is called, but the first user trying to open the project search or project details dialogs would have hit ModuleNotFoundError. Dead imports of QEvent, QObject, and QProgressDialog removed from annotator_window.py since their only remaining consumers (the filter class and process_multidim) have already moved out. annotator_window.py: 3,913 → 3,194 lines (-719, -18.4%). Total reduction vs original master: 5,900 → 3,194 (-45.9%). Tests: all 94 pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Phase 4c completes Phase 4 in the modular refactoring: YOLO training and prediction methods + the TrainingThread worker move into a dedicated controller. With this PR all three inference engines (SAM / DINO / YOLO) live in their own modules. New controllers/yolo_controller.py (539 lines) — YOLOController(QObject) plus the TrainingThread(QThread) class. Methods relocated: - Menu: setup_yolo_menu - Training data + model loading: load_yolo_model, prepare_yolo_dataset, load_yolo_yaml, save_yolo_model, load_prediction_model, initialize_yolo_trainer - Training execution: show_train_dialog, start_training, training_finished - Prediction: set_confidence_threshold, show_predict_dialog, run_predictions, predict_single_image, process_yolo_results State (yolo_trainer, training_thread, training_dialog) stays on the main window for consistency. ImageAnnotator.__init__ still sets yolo_trainer = None; setup_yolo_menu (delegated to the controller) is called from __init__ after the menu bar exists. Dead top-level imports removed from annotator_window.py: - cv2 (only used by process_yolo_results) - numpy (only used by process_yolo_results) - QThread, pyqtSignal (only used by TrainingThread, now in YOLOController) - QDoubleSpinBox (only used by show_predict_dialog) - LoadPredictionModelDialog, TrainingInfoDialog, YOLOTrainer (referenced by methods that moved) annotator_window.py: 3,194 → 2,757 lines (-437, -13.7%). Total reduction vs original master: 5,900 → 2,757 (-53.3%). More than half the original monolith now lives in focused controller modules: - controllers/io_controller.py 338 - controllers/project_controller.py 548 - controllers/image_controller.py 869 - controllers/sam_controller.py 267 - controllers/dino_controller.py 796 - controllers/yolo_controller.py 539 - core/image_utils.py 74 - ui/theme.py 68 Tests: all 94 pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Phase 5a in the modular refactoring: annotation CRUD and list management methods move into a dedicated controller. Phase 5b (ClassController) follows. This is the cluster ImageLabel mutates most directly via main_window.add_annotation_to_list(...), main_window.save_current_annotations(), etc. Phase 5 keeps the delegation pattern on ImageAnnotator so ImageLabel's call sites continue to work; Phase 6 will replace those with Qt signals. New controllers/annotation_controller.py (823 lines) — AnnotationController(QObject). Methods relocated (24): - COCO conversion: create_coco_annotation - List widget updates: update_all_annotation_lists, update_annotation_list, update_annotation_list_colors, update_annotation_list_with_sorted - Per-image cache sync: load_image_annotations, save_current_annotations - Sorting: sort_annotations_by_class, sort_annotations_by_area - COCO JSON load (independent of project save/load): load_annotations - Highlighting/selection: clear_highlighted_annotation, update_highlighted_annotations, highlight_annotation, highlight_annotation_in_list, select_annotation_in_list - Annotation numbering: renumber_annotations - Delete/merge/change-class: delete_annotation, delete_selected_annotations, merge_annotations, change_annotation_class - Tool commit paths: finish_polygon, finish_rectangle, add_annotation_to_list - Edit mode: enter_edit_mode, exit_edit_mode State stays on the main window (consistent with prior phases): - all_annotations, image_label.annotations - editing_mode, loaded_json, current_sort_method - annotation_list, merge_button, change_class_button widgets Dead imports of copy, json, shapely (MultiPolygon, Point, Polygon, unary_union, make_valid), and utils (calculate_area, calculate_bbox) removed from annotator_window.py — all consumers moved. annotator_window.py: 2,757 → 2,038 lines (-719, -26.1%). Total reduction vs original master: 5,900 → 2,038 (-65.5%). Tests: all 94 pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Phase 5b completes Phase 5 in the modular refactoring: class management methods move into a dedicated controller. The ImageLabel-mutates-ImageAnnotator cluster is now fully extracted — Phase 6 can wire the signal decoupling. New controllers/class_controller.py (422 lines) — ClassController(QObject). Methods relocated (14): - Selection: select_class, on_class_selected, update_class_selection, is_class_visible - Add/delete/rename/colour: add_class, delete_class, delete_selected_class, rename_class, change_class_color - List widget: update_class_item_color, update_class_list - Visibility: toggle_class_visibility - Context menu: show_class_context_menu - Slice-list colouring driven by per-slice annotations: update_slice_list_colors State stays on the main window: - class_mapping, image_label.class_colors, image_label.class_visibility - current_class - class_list, slice_list widgets - DINO widgets (dino_class_table, dino_phrase_panel) Minor robustness improvement: ClassController.delete_class accepts either a QListWidgetItem (the right-click context-menu path) or a class-name string (the orphaned delete_selected_class path). delete_selected_class is dead code in the codebase — nothing calls it — but the original delete_class(class_name) would have crashed with AttributeError on str.text() if the path were ever wired up. Dead imports removed from annotator_window.py: - traceback - QIcon, QColorDialog (only used by class methods that moved) - QGridLayout (was used only by the DimensionDialog → moved in Phase 3b) - QListWidgetItem (used only by class/annotation methods) - QCheckBox (no consumers left) - QInputDialog (only used by methods that moved) - QDialogButtonBox (only used by show_train_dialog → moved in Phase 4c) annotator_window.py: 2,038 → 1,669 lines (-369, -18.1%). Total reduction vs original master: 5,900 → 1,669 (-71.7%). Tests: all 94 pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
…se 6) Replace every self.main_window.* call site in ImageLabel (~50 across 17 methods) with either a pyqtSignal emission (writes / method calls) or a read through a new CanvasContext accessor. The orchestrator (ImageAnnotator) wires each signal to the matching controller slot in __init__ via _connect_image_label_signals(). What changed: - New widgets/canvas_context.py: narrow read-only view of main-window state for ImageLabel (paint_brush_size, eraser_size, current_class, class_id, class_mapping, is_class_visible, current_image_key, all_annotations, scroll_area, dialog_parent). - widgets/image_label.py: removed self.main_window field and set_main_window() method. Added 20 pyqtSignals covering annotation lifecycle, SAM, class, tool/UI state, navigation. All 50 main_window references replaced with self._ctx.<accessor>() or self.<signal>.emit(). - annotator_window.py: drops set_main_window call; constructs CanvasContext(self) after controllers are instantiated and connects every ImageLabel signal to a controller slot. Adds two thin slots: _on_tool_size_changed (brush/eraser size RMW) and _on_annotations_batch_saved (finalize paint/accept-temp batch). - controllers/annotation_controller.py: new replace_annotations() takes (image_key, per-class dict) for the eraser path; does the full replace + list update + save + slice-colour refresh atomically. - controllers/sam_controller.py: new cancel_sam_prediction() handles the Escape-during-sam_points timer-stop path. Invariants preserved: - v0.9.0 pan/zoom (event.globalPosition() for pan, viewport().width() for post-zoom offset). - ADR-013 SAM re-entrancy guard. - DINO temp_annotations single-field re-sync (untouched). - is_loading_project autosave guard (untouched). All 94 tests pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
P1 documentation updates (mandatory per CLAUDE.md): - docs/11_risks_and_technical_debt.md: mark "ImageLabel ↔ ImageAnnotator tight coupling" debt as Resolved and point at ADR-016. - docs/05_building_block_view.md: dependency diagram now shows ImageLabel emitting signals to ImageAnnotator and reading via CanvasContext (was: "references ──> ImageAnnotator (callbacks)"). - docs/09_architecture_decisions.md: new ADR-016 recording the signals+CanvasContext decision, rules for adding new traffic in either direction, and the synchronous-emit ordering / batch-save invariants the pattern depends on. - docs/08_crosscutting_concepts.md: new "Canvas Decoupling" section with the rules of the pattern (no back-reference to ImageAnnotator; use CanvasContext for reads; batch-save signal preserves O(1) save). - CLAUDE.md: "Adding a New Annotation Tool" step 5 referred to main_window.add_annotation() (which never existed and is misleading post-Phase-6). Updated to emit annotationCommitted with a pointer to ADR-016. P2 code cleanups: - Drop the `committed` / `had_annotations` guards in commit_paint_annotation and accept_temp_annotations — the original pre-refactor code emitted save_current_annotations() + update_slice_list_colors() unconditionally inside the same branch. Restores literal semantic equivalence with the pre-Phase-6 code as promised in the Phase 6 commit body. - Rename SAMController.cancel_sam_prediction → cancel_sam_debounce. The method stops the debounce timer; it does NOT abort an in-flight inference (the _sam_inference_in_flight guard handles that — ADR-013). The old name conflated the two. All 94 tests pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Carve the four mouse-driven tools (polygon, rectangle, paint_brush,
eraser) out of the if/elif chains in ImageLabel's event handlers into
focused ToolHandler subclasses under widgets/tools/. ImageLabel keeps
a thin dispatcher: mouse/key events route to the active handler;
paintEvent calls active_tool_handler.paint_overlay() for in-progress
state rendering.
What changed:
- New widgets/tools/{__init__, base, rectangle_tool, polygon_tool,
paint_tool, eraser_tool}.py: 525 LOC across 6 files. Each handler
subclasses ToolHandler (plain Python object, not a QObject) and
emits back through ImageLabel's Phase 6 signals — no controller
imports.
- widgets/image_label.py: dropped start_painting / continue_painting /
finish_painting / commit_paint_annotation / discard_paint_annotation
+ the mirror eraser methods + finish_polygon / cancel_current_annotation /
finish_current_annotation / get_rectangle_from_points /
draw_temp_paint_mask / draw_temp_eraser_mask / draw_current_rectangle.
paintEvent's tool-specific branches collapse to a single
active_tool_handler.paint_overlay() call. Mouse/key event chains
collapse to dispatch calls. check_unsaved_changes iterates
has_unsaved_state across the handlers. 1239 -> 963 LOC (-276).
- Added image_label.active_tool_handler property and set_active_tool()
with deactivate() lifecycle hook.
- annotator_window.py: tool-button toggle paths switch from
`image_label.current_tool = "X"` to `image_label.set_active_tool("X")`
(6 sites). closeEvent unsaved-changes path now routes through
the paint_brush / eraser handlers' commit() / discard().
- controllers/image_controller.py: switch_slice unsaved-changes path
same routing update.
- tests/unit/test_conversions.py: dropped the spec_from_file_location
trick (relative imports inside widgets/ subpackage break it); plain
import works now and the comment about torch dependency is stale —
widgets/__init__.py is empty.
Removed dead code (no readers anywhere):
- ImageLabel.paint_mask + ImageLabel.eraser_mask fields
- draw_paint_mask + draw_eraser_mask methods
- Stale Qt imports (cv2, numpy, QApplication, QPolygon)
State ownership (deliberate deviation from the original plan):
Tool handlers contain only behavior, not state. Fields like
current_rectangle, start_point, end_point, current_annotation,
temp_point, drawing_polygon, drawing_rectangle, temp_paint_mask,
is_painting, temp_eraser_mask, is_erasing stay on ImageLabel —
AnnotationController.finish_rectangle / finish_polygon and other
controllers still read mw.image_label.X directly. Moving state into
the handlers would require a parallel controller refactor; left for
a follow-up.
Invariants preserved:
- ADR-013 SAM re-entrancy: SAM logic stays on ImageLabel.
- v0.9.0 pan/zoom: Ctrl-modifier branches in mouse events run before
tool dispatch.
- ADR-015 DINO event filter: temp_annotations + accept/discard stays
on ImageLabel.
- ADR-016 (Phase 6) signal contract: tools emit via self.label.X.emit().
- Polygon edit mode (editing_polygon + handle_editing_click +
handle_editing_move + draw_editing_polygon) stays on ImageLabel as
a modal state orthogonal to tool selection.
- Eraser commit OpenCV polygon-cutting moved byte-for-byte.
All 94 tests pass.
https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
P1 regressions fixed: - widgets/tools/polygon_tool.py: PolygonTool.has_unsaved_state() previously returned True for any in-progress polygon (>= 1 point), but commit() requires len > 2 — so clicking "Yes save" on a 1- or 2-point polygon silently kept stale state across image switches. Narrowed has_unsaved_state to len > 2; commit() now matches. - widgets/image_label.py: paintEvent now iterates ALL handlers' paint_overlay(), not just the active tool's. Pre-Phase-7 the temp paint/eraser masks and polygon-in-progress drew whenever their state field was populated, regardless of current_tool; Phase 7 had silently hidden them on tool switch. Each handler's paint_overlay short-circuits when its state is empty, so the iteration is cheap. Dead duplicate dialog blocks removed: - annotator_window.py: closeEvent had a second "unsaved changes" dialog AFTER check_unsaved_changes had already prompted and committed/discarded. The block was dead in the happy path (masks were already None) and contained a pre-existing bug — event.accept() was called BEFORE the second prompt, so the post-prompt event.ignore() on Cancel had no effect. Removed the whole block. - controllers/image_controller.py: switch_slice had the same dead duplicate block. Removed. Other P2 cleanups: - widgets/tools/rectangle_tool.py: added has_unsaved_state() (returns True while drawing_rectangle is set) and commit() (treats Yes as discard since a mid-drag rectangle has no finishable state). Removed unused QPointF import. Documentation (CLAUDE.md mandates arc42 updates in the same PR): - docs/09_architecture_decisions.md: new ADR-017 covering the ToolHandler pattern, the deliberate state-on-widget non-decision with rationale, the iterate-all-handlers paint_overlay rule, the has_unsaved_state contract, and a pattern guide for adding a new tool. Acknowledges the state-leak architectural smell that will need a follow-up. - docs/05_building_block_view.md: updated package tree to reflect the full post-Phase-1/6/7 layout (core/, widgets/, controllers/, inference/, io/, ui/, dialogs/, widgets/tools/). Rewrote ImageLabel section to describe the dispatcher pattern and list per-tool state fields. Removed stale references to start_painting / start_erasing / image_label.py at root. All 94 tests pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
Carve the four live UI-building methods out of ImageAnnotator into
focused builder functions under ui/. Also delete three dead UI
builders that were never called from anywhere, and factor two inline
init blocks (snake-game F2 shortcut, DINO review event filter) into
small named functions.
After this change, ImageAnnotator.setup_ui() is a short sequence of
function calls:
build_menu_bar(self)
build_sidebar(self)
build_image_area(self)
build_image_list(self)
self.setup_slice_list()
self.update_ui_for_current_tool()
What changed:
- New ui/menu_bar.py (~115 LOC): build_menu_bar(window) — moves
create_menu_bar verbatim. Every action.triggered connects to a
window.<method> (the existing delegate methods on ImageAnnotator
that forward to controllers).
- New ui/sidebar.py (~310 LOC): build_sidebar(window) + build_image_area
+ build_image_list. Widget refs attach as window.X = QWidget(...)
so the rest of the code that reads self.class_list etc. keeps
working unchanged. Private _section_header helper for the section
labels.
- New ui/shortcuts.py (~25 LOC): install_shortcuts(window) for F2 →
snake game, install_event_filters(window) for the ADR-015 DINO
review filter.
- annotator_window.py: 1725 → 1169 LOC (-556). Methods removed:
create_menu_bar, setup_sidebar, setup_image_area, setup_image_list
(all moved), plus three dead methods that were never called from
setup_ui or anywhere else (setup_class_list, setup_tool_buttons —
the latter referenced a ghost self.load_sam2_button widget that's
never created, setup_annotation_list). Inline init cleanup: dropped
the duplicate early sam_magic_wand_button + tool_group creation
(build_sidebar creates them), dropped the duplicate central_widget
+ layout early init (setup_ui creates them), dropped the duplicate
sam_magic_wand_button.clicked.connect post-setup_ui (build_sidebar
wires it), and moved class_list.itemChanged.connect next to the
class_list construction inside build_sidebar. QtWidgets import
block trimmed from ~22 names down to 10.
Pattern: functions, not classes. ImageAnnotator builders run once at
construction; a class would add nothing. Follows the existing
ui/theme.py shape (functions taking the main window).
Invariants preserved:
- Construction order: menu bar -> central widget -> sidebar ->
image area -> image list -> slice list -> update_ui_for_current_tool.
- All widget refs that other modules read (class_list, annotation_list,
scroll_area, zoom_slider, image_info_label, tool buttons, DINO
widgets, etc.) end up on window.X after the builders return.
- ADR-015 event filter still installs on QApplication.instance() via
install_event_filters.
- F2 shortcut still has ApplicationShortcut context.
- Phase 6 signal connections (_connect_image_label_signals) unchanged.
All 94 tests pass. App boots; menu bar populated; widget refs verified
present.
https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
P1 documentation drift fixes:
- ADR-017 ("adding a new mouse-driven tool") pointed at
ImageAnnotator.setup_tool_buttons() which Phase 8 deleted as dead
code. Updated to point at ui/sidebar.py:build_sidebar.
- ADR-015 "Related" block claimed _DINOReviewEventFilter and its
installEventFilter call both live in annotator_window.py. The class
actually moved to controllers/dino_controller.py in Phase 4b and
the install call moved to ui/shortcuts.py in Phase 8. Updated.
- docs/05_building_block_view.md: ui/ directory line listed only
"menu_bar, sidebar, theme, stylesheets" — added shortcuts.
P2 cleanups:
- annotator_window.py: dropped the dead ClassThresholdTable /
PhraseEditorPanel import (those widgets now instantiated in
ui/sidebar.py; the orchestrator only mentions them in a comment).
- Updated stale comment "Also, add the options in create_menu_bar
method" — the method moved to ui/menu_bar.build_menu_bar in
Phase 8. Comment now points there.
- ui/sidebar.py: replaced hardcoded color:#555 on lbl_dino_custom
with palette(text) so the colour follows the active stylesheet.
Pre-existing CLAUDE.md "no hardcoded colors" violation; fixed
while touching the file.
- Deleted the duplicate self.showMaximized() call (pre-existing
cruft).
- Renamed controllers.dino_controller._DINOReviewEventFilter ->
DINOReviewEventFilter. The underscore convention says "private
to this module"; with Phase 8 the symbol is imported by both
the orchestrator (historic, since removed) and ui/shortcuts.py.
Two outside importers means the underscore was already a lie.
Updated all 4 reference sites + docs.
All 94 tests pass. App boots; event filter wired correctly under
the new name.
https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
…architecture Phase 9 — the final phase of the 9-phase modular refactor. No code changes; the inventory found __init__.py already exposes the documented public API (ImageAnnotator, ImageLabel, SAMUtils, calculate_area, calculate_bbox), no Phase-1-era top-level shims survive, and docs/06_runtime_view.md already names the post-refactor call paths. What was left was three documents lagging the code. What changed: - CLAUDE.md: project-structure tree now shows the subpackage layout (core/, controllers/, widgets/, widgets/tools/, inference/, io/, ui/, dialogs/). "Key Classes" table grew from 3 rows to 14 — CanvasContext, ToolHandler, all 4 tool subclasses, all 7 controllers, SAMUtils, DINOUtils. Test count corrected from "65 tests" to "94 tests". - docs/05_building_block_view.md: new "Level 3: Controllers" section with a one-line responsibility per controller, mirroring the shape of the existing "Level 3: Export/Import Subsystem" section. Cross-references ADR-013 (SAM re-entrancy), ADR-015 (DINO event filter), ADR-016 (signal decoupling). - docs/12_glossary.md: added entries for CanvasContext, Controller (as a pattern term), ToolHandler, the four tool subclasses, and the ui/ builders (build_menu_bar / build_sidebar / build_image_area / build_image_list). "Key Classes (Code)" table grew from 6 rows to 18 with the same coverage as CLAUDE.md, plus the DINOReviewEventFilter and the dialog-housed YOLOTrainer / DimensionDialog / TrainingThread. - docs/11_risks_and_technical_debt.md: rewrote the "No Automated Tests" entry. Old text claimed zero tests (true pre-refactor); current state is 94 tests with ~15% line coverage. The remaining gap — canvas event flow + inference paths — is named, with a concrete plan that leans on the Phase 7 ToolHandler isolation (each handler can be unit-tested with a stub label). All 94 tests pass. Public API import still resolves: `from src.digitalsreeni_image_annotator import ImageAnnotator, ImageLabel, SAMUtils, calculate_area, calculate_bbox`. End state of the 9-phase refactor: - annotator_window.py: 5,900 -> 1,164 LOC (-80%) - image_label.py: 1,193 -> ~960 LOC, no main_window reference - 7 controllers (project, image, sam, dino, yolo, annotation, class) - 4 tool handlers (rectangle, polygon, paint, eraser) - 3 UI builders + a CanvasContext accessor + a shortcuts/event-filter installer - 5 ADRs added or preserved (013, 015, 016, 017) - Public API surface unchanged https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
All four P2 cleanups from the reviewer: - CLAUDE.md project-tree comment said "dialogs/ ... 14 files"; the directory has 16 non-init Python files. Updated the count. - docs/05_building_block_view.md Controllers section opened with "The seven controllers/* modules" and "each controller is a QObject subclass" but the table beneath had 8 rows — the last being io_controller, which is a module of helper functions, not a QObject. Rewrote the intro to acknowledge "seven QObject controllers plus an io_controller helper module" so the prose matches the table. - docs/05_building_block_view.md top-level tree (line 43) listed the controllers/ contents as just "Project/Image/SAM/DINO/YOLO/ Annotation/Class" while the same document's new Level 3 section includes io_controller. Same document, two stories — added "+ io_controller helpers" to the tree comment. - docs/12_glossary.md "Tool subclasses" entry enumerated 6 hooks but missed on_double_click and deactivate from widgets/tools/base.py. Listed all 8 to match the base-class contract exactly. No code changes. 94/94 tests still pass. https://claude.ai/code/session_01QxGci8QYbXHtV6BpoBLfAU
1. Brush/eraser '+' key (Qt.Key.Key_Plus) now increases size alongside '='. 2. Four stale inline imports after Phase 1 subpackage move corrected (dino_utils, project_search, annotation_statistics, project_details). 3. Dark mode DINO phrase-editor table headers now inherit palette colors instead of overriding without providing alternatives. Also fixes WinError 1114 on SAM model load: - Root cause: __init__.py eagerly imported PyQt6-dependent modules, so Qt loaded before torch when launching via 'sreeni' console script. - Fix: __getattr__-based lazy loading in __init__.py keeps package import Qt-free; main.py already imports torch before QApplication. - Rewrote tools/check_pyqt6_torch_coexistence.py to test both orders. - Added AST-based smoke test for stale inline imports. - Documented in ADR-016 (inline imports) and ADR-017 (torch-first). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes color: #333 and color: #777 from lbl_title and hint QLabel style sheets. In dark mode these were near-invisible against the #2F2F2F background. Let the global soft_dark_stylesheet handle text color consistently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…bpackages refactor: Phase 1 — reorganize package into responsibility-focused subpackages
refactor: Phase 2 — extract theme, I/O wrappers, and image utils from ImageAnnotator
refactor: Phase 3a — extract ProjectController from ImageAnnotator
refactor: Phase 3b — extract ImageController from ImageAnnotator
refactor: Phase 4a — extract SAMController from ImageAnnotator
…ctoring-phase4b # Conflicts: # src/digitalsreeni_image_annotator/annotator_window.py
refactor: Phase 4b — extract DINOController from ImageAnnotator
refactor: Phase 4c — extract YOLOController from ImageAnnotator
refactor: Phase 5a — extract AnnotationController from ImageAnnotator
refactor: Phase 5b — extract ClassController from ImageAnnotator
…ctoring-phase6 # Conflicts: # docs/09_architecture_decisions.md # src/digitalsreeni_image_annotator/widgets/image_label.py
refactor(phase6): Decouple ImageLabel from ImageAnnotator via Qt signals + CanvasContext
…ctoring-phase7 # Conflicts: # docs/05_building_block_view.md # docs/09_architecture_decisions.md
refactor(phase7): Extract per-tool handlers from ImageLabel
…ctoring-phase8 # Conflicts: # docs/05_building_block_view.md # src/digitalsreeni_image_annotator/annotator_window.py
refactor(phase8): Extract UI assembly into ui/menu_bar + ui/sidebar
…ctoring-phase9 # Conflicts: # docs/05_building_block_view.md # docs/11_risks_and_technical_debt.md
- Fix ADR-016/017 → ADR-018/019 cross-document drift in CLAUDE.md, docs/, and widget tools/base.py (P0) - Remove dead `highlight_annotation` (singular) method; fix `clear_highlighted_annotation` to use `highlighted_annotations` plural list consistently. Remove redundant `.clear()` from `image_controller`. (P1) - Fix `_DINOReviewEventFilter` → `DINOReviewEventFilter` comment drift in dino_controller.py (P1) - Add defensive `.copy()` on mask before QImage construction in paint and eraser tools (P2) 94 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_annotation annotation_controller.py:delete_annotation was still setting the never-read `highlighted_annotation` (singular) instead of clearing the real `highlighted_annotations` list. No functional change other than consistency with all other highlight-clear paths. 94 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Images+Annotations split took filenames from the COCO JSON and called shutil.copy2 without checking the selected input directory actually contains them - a wrong directory crashed the whole app with an unhandled FileNotFoundError. Now every listed image is validated up front and the split aborts with a warning dialog listing the missing files (a partial split would silently produce a broken dataset). The split dispatch is additionally wrapped so any other error surfaces as a dialog instead of killing the app. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Magic Wand button was redundant with SAM-box (same draw-a-bbox interaction) and actually broken since the controller extraction: its mouse release emitted samPredictionApplyRequested, but apply_sam_prediction only handles sam_box / sam_points and silently returned for sam_magic_wand, so the wand did nothing. - Remove the button, its tool_group wiring, the activate/deactivate/toggle controller methods (two of which were connected to no signal at all), the sam_magic_wand_active canvas flag and all its mouse/key/paint branches. - New SAMController.deactivate_sam_tools() replaces deactivate_sam_magic_wand at the YOLO-prediction call sites and the model-unset path; it only clears current_tool when a SAM tool is active, so e.g. polygon mode survives a YOLO prediction. - Picking a SAM model no longer auto-activates any tool; SAM-box and SAM-points are activated explicitly via their buttons. - Escape now clears a pending SAM-box bbox/prediction (this branch was previously gated on the wand flag), and the cross cursor is applied for sam_box/sam_points in update_ui_for_current_tool (it was previously overridden back to arrow). - Update in-app Help, README, and arc42 docs (05, 06, 09) to describe the SAM-box / SAM-points workflow instead of the removed button. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs(phase9): Refresh CLAUDE.md + arc42 to reflect the post-refactor architecture
Adds an accessibility feature for users with decreased eyesight: the application font scales continuously from 8 to 24pt via Ctrl+Shift+= / Ctrl+Shift+- / Ctrl+Shift+0 (also Ctrl++ / Ctrl+-) and new Settings menu entries. The existing Font Size presets stay as checkable entry points into the same range. - Single source of truth ImageAnnotator.ui_font_pt (int, clamped 8-24); every change funnels through theme.set_font_pt (ADR-020) - Canvas overlays scale too: annotation label fonts, SAM point markers, pen widths, edit handles and hit-test tolerances derive from ui_scale = ui_font_pt / 10 via ImageLabel._pen_w / _overlay_font; at the default 10pt rendering is pixel-identical - First QSettings usage (app_settings.py): ui/font_pt and ui/dark_mode persist across restarts (per-user, not in .iap) - Appended QSS overrides scale the stylesheets' hardcoded px values (section header, checkbox/radio indicators) by ui_font_pt / 10 in px, reproducing the static sheets exactly at the default - Fix HelpWindow.apply_font_size wiping the theme stylesheet - Remove dead theme.setup_font_size_selector / on_font_size_changed - Fix DINO Browse button clipping at large fonts (fixed 60px width) - Tests: app_settings clamp + INI roundtrip, theme stepping / clamping / preset sync / stylesheet pinning (113 total, 1s) - Docs: UI Font Zoom concept in arc42 08, ADR-020 in 09, structure and shortcut tables in 05 / CLAUDE.md / in-app help Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QA on PR #18 showed the DINO threshold table, phrase list, hint and Add/Remove Phrase buttons stayed tiny while the rest of the UI zoomed: their inline font-size:10/11px styles beat the global rule. - Remove all inline font-size tokens from dino_phrase_editor.py (other style properties kept); hint label gets an objectName - theme.py appended-override block now owns the compact sizes via type/objectName selectors, scaled by ui_font_pt / 10 in px — 11px/10px at the default, exactly the legacy look - Threshold table rows switch from fixed 26px to ResizeToContents so cell text is not clipped at large fonts (26px -> 42px at 24pt) - New rendered-font test pins that the QSS px rules win over the findChildren setFont loop (the invariant this fix depends on) - ADR-020 debt note narrowed: dino_merge_dialog.py is the only remaining non-scaling dialog; compact-widget pattern documented Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
feat: Low-vision mode - continuous UI font zoom with persistence
Upstream PR bnsreenu#69 (PyQt6 migration + in-process inference) was developed on this fork; the fork carries the same content plus later refinements (ADR-017 torch-first findings in the coexistence check, lazy package __init__). Verified file-by-file that no upstream-only change exists: TESTING.md is identical, all other deltas are fork-side additions. This ours merge makes the branch a descendant of upstream/master so the PR diff shows exactly the new work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Sreeni! 👋
Following up on the PyQt6 + in-process inference migration you merged in #69, this PR brings over everything that has matured on my fork since then. It's three things in one: a low-vision accessibility mode, a modular architecture refactoring of the main window, and a handful of bug fixes found during manual QA.
A friendly aside: in your recent YouTube video you mentioned that an accessibility / visibility mode was still missing from the annotator — I was happy to support you with exactly that one. 😉 It's the headline feature of this PR.
1. Low-vision mode — continuous UI font zoom (new feature)
For users with decreased eyesight, the entire application font now scales continuously from 8 to 24pt, persisted across restarts:
Ctrl+Shift+=/Ctrl+Shift+-step the size ±1pt (Ctrl++/Ctrl+-work too),Ctrl+Shift+0resets. All three live in Settings → Font Size, so they're discoverable.QSettings(per-user, deliberately not in the.iapproject file).2. Modular architecture refactoring (Phases 1–9)
annotator_window.pyhad grown to ~5,800 lines doing everything at once. It is now a thin orchestrator (~1,100 lines) that wires up focused components:Key design points (each documented as an ADR in
docs/09_architecture_decisions.md):ImageLabelis decoupled from the main window: it reads state through a narrowCanvasContextaccessor and communicates back exclusively via Qt signals (ADR-018) — no more reaching into the orchestrator.docs/) were updated throughout: building block view, runtime view, crosscutting concepts (coordinate systems, dark-mode rules, DINO temp-annotation lifecycle, …) and ADR-014 through ADR-020.3. Fixes & cleanup
FileNotFoundErrorin a Qt slot). Now pre-validates and shows a clear warning dialog listing the missing files; the split also won't start half-way and leave a broken dataset behind.main.py; the package__init__is lazy so importing it stays Qt-free.Testing
The diff looks large (70 files, +7.7k/−5.7k) but the bulk is the mechanical move of code from
annotator_window.pyinto the packages above. The genuinely new code is the low-vision feature, the controllers' wiring, and the tests/docs.Happy to split this into smaller PRs if you'd prefer reviewing it piecewise — and thanks again for the great tool and videos!
🤖 Generated with Claude Code