Skip to content

Plugin refactor [8]: Improve layer management/lifecycle - #186

Merged
C-Achard merged 75 commits into
cy/feature-refactor-2026from
cy/layer-lifecycle
Apr 29, 2026
Merged

C-Achard merged 75 commits into
cy/feature-refactor-2026from
cy/layer-lifecycle

Conversation

@C-Achard

@C-Achard C-Achard commented Apr 20, 2026 •

Copy link
Copy Markdown
Member

Note: this is a work-in-progress PR meant to assist in refactor-related debugging

This pull request introduces a debug window UI, as well as unit tests for the debug logging and reporting utilities.
It also aims to improve layer wiring via better centralization.

  • Refactors the main widget to integrate the debug window and log recorder
  • Improves layer lifecycle management
  • Enhances debug logging throughout key layer-handling methods.
  • Adds a separate save workflow
  • And various other enhancements

Automated summary

This pull request introduces several improvements and refactorings to the napari-deeplabcut codebase. The most significant changes include enhancing the handling of DeepLabCut (DLC) project context and metadata in I/O functions, expanding and improving the test suite for layer lifecycle management and keypoint storage, and refactoring test code to use a more robust API for accessing layer stores. These changes improve maintainability, correctness, and test coverage.

Enhancements to DLC project context and layer metadata:

  • Added _build_dlc_layer_meta utility and updated get_video_reader and get_folder_parser to attach explicit DLC lifecycle metadata to layers, improving session and project context handling for both video and image data. [1] [2] [3] [4]

Layer lifecycle and keypoint store improvements:

  • Added comprehensive tests for the RuntimeRegistry and ManagedPointsRuntime classes, covering registration, resolution, duplicate handling, iteration, dead entry cleanup, and auditing.
  • Expanded tests for KeypointStore to cover layer resolver attachment, authoritative resolver behavior, and correct updating of layer references and IDs.

Test code refactoring and robustness:

Keypoint store and test fixes:

  • Updated tests to assert correct layer_id values and to use the public add method instead of the private _add method for adding keypoints. [1] [2]

Test imports and setup:

  • Added missing pytest imports to test files to enable test execution.

C-Achard added 10 commits April 14, 2026 10:31
Introduce an in-memory debug recorder and helper functions for collecting diagnostics. Adds install_debug_recorder/get_debug_recorder and InMemoryDebugRecorder (bounded deque, fail-open, no I/O, safe message/exception handling) attached to the 'napari-deeplabcut' logger. Also adds collect_environment_summary, summarize_layer, summarize_viewer, and format_debug_report to produce a redact-safe diagnostic report with environment info, viewer/layer summaries, and recent plugin logs.
Introduce build_debug_report to assemble environment info, viewer summary, and recorder logs into the existing format_debug_report. It accepts a viewer, an optional InMemoryDebugRecorder and a log_limit (default 300), uses recorder.render_text when available, and falls back to a placeholder string. Also adds a top-of-file path comment.
Introduce a new debug text viewer dialog (src/napari_deeplabcut/ui/debug_window.py). Adds DebugTextWindow — a read-only, refreshable, copyable dialog using a text provider callable, with fixed-width font and optional Ctrl/Cmd-C shortcut. Also adds helper providers make_log_text_provider and make_issue_report_provider to adapt InMemoryDebugRecorder and build_debug_report for display. Handles provider exceptions gracefully and provides status feedback when copying to the clipboard.
Install a debug recorder and add a "Generate debug log" help-menu action that opens a DebugTextWindow with recent plugin logs (using make_issue_report_provider/get_debug_recorder). Move and consolidate label_mode/color_mode handlers and related multi-animal checks earlier in the file.
Rename several menu actions to include "napari-dlc" (debug log, tutorial, shortcuts) to make the plugin-specific commands explicit. Change DebugTextWindow instantiation to use self as the parent instead of viewer.window._qt_window to ensure correct parenting. Replace moveCursor call with QTextCursor.MoveOperation.Start and add the QTextCursor import for a reliable cursor move to the start. Minor comment/separator added.
Introduce logger.debug statements across KeypointControls to aid debugging of layer setup, merging, and adoption flows. Log context includes layer name/type, metadata (project, root, paths length), store counts, allowed merges, and keypoint diffs when merging. Also minor refactor to use a local md variable (layer.metadata or {}) and copy it for new_metadata.
Reduce the minimum in-memory log buffer from 10 to 1 (use max(1, int(capacity))) so callers can specify very small capacities. Also normalize _safe_tail output to POSIX-style strings by calling as_posix() on returned Path objects to ensure consistent path formatting across platforms and avoid backslashes on Windows.
Add comprehensive unit tests for the debug UI and recorder utilities. Tests cover DebugTextWindow behaviors (initial load, refresh, copy, shortcut, failure handling) and the text providers (log and issue report). Add tests for InMemoryDebugRecorder (installation idempotency, capture of messages and exceptions, bounded capacity, clear, render_text, and handling unrenderable messages). Also test helper utilities: _safe_tail, collect_environment_summary, summarize_layer, summarize_viewer, and format_debug_report. Uses qtbot for GUI tests and simple fakes/mocks for viewers/layers and recorder.
Extract plugin-related UI setup into _add_plugin_actions and consolidate repeated form builders (_form_dropdown_menus, _form_mode_radio_buttons, _form_color_mode_selector). Move help-menu actions (tutorial, shortcuts, debug log) and global keybinding installation into a single helper and remove duplicated method definitions to reduce code dispersion and improve readability. No functional changes intended.
Introduce a new core.layer_lifecycle package (manager, registry, merge types) to centralize viewer layer lifecycle handling and managed Points runtime attachments. Integrate the new LayerLifecycleManager into _widgets.py (instantiate manager, comment out direct viewer layer signal hookups) and add a debug log in on_insert. The RuntimeRegistry and ManagedPointsRuntime provide weak-ref backed registrations for managed Points layers, while MergeDecision types define merge policy primitives.
@C-Achard C-Achard self-assigned this Apr 20, 2026
@C-Achard C-Achard added the enhancement New feature or request label Apr 20, 2026
C-Achard added 17 commits April 22, 2026 11:58
Replace KeypointControls' internal _stores usage with LayerLifecycleManager and update lifecycle wiring. KeypointControls now creates LayerLifecycleManager(owner=self), calls attach(), and defers points registration/unregistration and layer queries to the manager (new managed_points_layers/iter_managed_points/managed_points_count/has_managed_points helpers). Deprecated direct _stores access and updated TrailsController and various control paths to use the manager API. LayerLifecycleManager constructor now accepts owner, derives viewer from owner, supports QObject parenting, and adds register_managed_layer wrapper that delegates to register_managed_points_layer (with stricter type checks). Also added iteration helpers and counting convenience methods. Minor registry file: added logger setup.
Replace direct accesses to the private _stores dict with the public layer manager API in tests. Tests now call get_layer_store(...) (controls/keypoint_controls) and use register_managed_layer(...) instead of assigning into _stores, improving encapsulation and avoiding reliance on internal state. Updated test files: e2e/test_layer_coloring.py, e2e/test_overwrite_and_merge.py, e2e/test_points_layers.py, e2e/test_routing_and_provenance.py, and test_widgets.py.
Add unit tests for LayerLifecycleManager and RuntimeRegistry. Tests cover idempotent attach/detach, registering/unregistering managed points, on_insert/on_remove behavior delegating to owner methods, adopting existing layers while skipping already-managed points, resolving inserted layers from event payloads, and manager remapping/refresh calls. Also add RuntimeRegistry tests for register/get/require/unregister roundtrips, duplicate registration errors, items/layers/runtimes consistency, and purging dead weakrefs. Includes small test helpers (dummy viewer/owner/layers).
Major refactor of the layer lifecycle registry and manager to centralize layer liveness resolution and provide observable cleanup/auditing.

Key changes:
- RuntimeRegistry now tracks entries by stable layer_id (id(layer)) and resolves layer liveness via weakrefs; ManagedPointsRuntime stores layer_id instead of a strong layer reference.
- Added diagnostic types: ClearedRegistryEntry, RegistryAuditIssue, RegistryAuditReport, and an audit() API to produce a structured report of live/dead entries and issues.
- Added clear_dead_entries() to remove and return dead entries (with optional logging) so reaping is observable instead of implicit/purge-on-access.
- Introduced live-focused iterators and accessors: iter_live_items(), iter_live_layers(), iter_live_runtimes(), resolve_live_layer(), get_live_runtime()/require_live_runtime(), get_store()/require_store(), and layer_ids()/contains_layer_id().
- Registry methods now validate runtime.layer_id consistency and fall back to strong storage with a logged error if an object cannot be weak-referenced.
- RuntimeRegistry.assert_consistent() now uses the audit report and raises if issues exist.

Manager and widget updates:
- LayerLifecycleManager exposes new facade methods (resolve_live_layer, get_live_runtime, get_store, require_store, clear_dead_entries, audit_registry) and adapts registration/unregistration to accept layer objects or ids.
- Iteration and counting use live-only iterators; register ensures runtime.layer_id matches the actual layer id.
- KeypointControls close handler now clears dead entries and logs an audit report if issues are found.

Motivation:
- Centralize liveness checks and make cleanup explicit and debuggable to avoid silent resource leaks and make lifecycle bugs easier to diagnose.
Refactor and extend tests for LayerLifecycleManager and RuntimeRegistry to match updated runtime API and lifecycle behavior. Tests now import gc and exercise resolve_live_layer/get_live_runtime/get_store/require_store paths, use ManagedPointsRuntime with layer_id, and assert registry iteration helpers (iter_live_items/iter_live_layers/iter_live_runtimes) and layer_ids. Added new tests to verify reaping/clearing of dead entries, audit/reporting of stale entries, and unregister-by-layer-id behavior. Adjusted expectations and renamed several test functions to reflect the new APIs.
Import deprecation helper and mark legacy layer-handling methods as deprecated. Added from napari_deeplabcut.utils.deprecations import deprecated and applied @deprecated to _adopt_existing_layers, _adopt_layer, and on_insert with messages pointing to LayerLifecycleManager / self.layer_manager replacements, clarifying that layer adoption and direct layer management are now handled elsewhere.
Add @deprecated annotations to KeypointControls methods that belong in LayerLifecycleManager: _setup_image_layer, _wire_points_layer, and _setup_points_layer. The decorators include messages indicating these methods are still used by LayerLifecycleManager but should be moved there. No functional logic changes, only deprecation metadata to guide refactoring.
Refactor lifecycle responsibilities by moving image/points setup, wiring, removal and remap logic out of the UI widget and into LayerLifecycleManager. Key changes:
- _widgets.py: switched to relative imports, removed many lifecycle implementations and replaced them with deprecated UI-facing wrappers that delegate to layer_manager; preserved small UI completion hooks used by the widget.
- core/layer_lifecycle/manager.py: added lifecycle-owned methods (_setup_image_layer, _wire_points_layer, _setup_points_layer, _handle_removed_layer, _remap_frame_indices) and attach_points_layer_runtime to centralize runtime bindings (paste/add wrappers, events, keybindings), plus adoption/insert/remove handling.
- core/keypoints.py: KeypointStore now supports weak refs and a resolver-based lifecycle (layer id/resolution), raises LayerUnavailableError when layer is gone, exposes add() (with a temporary _add compat wrapper), and renamed nearest-neighbor helper to find_nearest_neighbors; several safety/fallbacks added.
- core/layer_lifecycle/registry.py: added PointsRuntimeResources dataclass to track non-Qt runtime attachments for managed Points layers.
- Misc: logging/message tweaks, deprecated annotations for moved methods, and transitional notes to keep UI-specific behavior separated from lifecycle logic.
This reorganizes ownership of runtime behavior to make lifecycle management testable and encapsulated while retaining UI-only completion in the widget.
Replace uses of internal helpers with public API (keypoints._add -> keypoints.add; keypoints._find_nearest_neighbors -> keypoints.find_nearest_neighbors) and add missing pytest import. Add tests covering layer resolver behavior (maybe_layer, LayerUnavailableError) and the store.layer setter updating layer_id and keypoints. Also add an assertion to verify store.layer_id matches id(store.layer) in test_store_labels.
Align tests with recent refactors to the layer lifecycle manager and keypoint APIs. Tests now use lightweight fakes and fixtures (FakeStore, immediate QTimer, fake runtime attachment) and a more featureful DummyOwner that exposes MagicMock hooks so assertions check behavior via those hooks instead of internal call lists. Adapted tests to renamed/updated APIs: manager parameter existing -> existing_resources, reap_dead_entries -> clear_dead_entries, and the temporary keypoints wrapper renamed to add; tests also call store.add directly where appropriate. Minor test reorganizations improve determinism and isolate lifecycle orchestration from napari internals.
Centralize lifecycle-owned image and project metadata in LayerLifecycleManager: added ImageMetadata storage, project_path, and convenience accessors (image_root, image_paths, image_name). Moved image metadata update, points→image metadata sync, and project-path caching from KeypointControls into the manager, and updated _setup_image_layer and related flows to use the manager-owned context. KeypointControls retains compatibility shims and deprecated wrappers that delegate to LayerLifecycleManager, and call sites were updated to reference layer_manager.* instead of local fields. Import and metadata helper usage was adjusted accordingly; behavior should be preserved while ownership is moved to the manager.
Add explicit DLC lifecycle metadata propagation and lifecycle handling for image/video layers.

- Reader changes: build and attach a standardized dlc metadata dict for images and videos (via a new _build_dlc_layer_meta); infer project context for opened videos and labeled folders and pass dlc_meta into read_images/read_video. Non-DLC media remain supported but are marked as non-session.
- IO refactor: read_images now accepts dlc_meta and uses a helper to build image layer kwargs; read_video accepts optional dlc_meta and includes it in returned metadata.
- Project paths: add infer_dlc_project_from_labeled_folder, infer_dlc_project_from_video_path, and session_key_from_project_context to better infer session contexts for folders and directly opened videos.
- Lifecycle manager: track the active DLC image layer id, determine/accept/reject DLC session image layers, expose active_dlc_image_layer, and add safe helpers for removing layers and setting visibility; ensure conflicting DLC layers are rejected and non-session images are ignored.
- Widgets/UX: KeypointControls updated to use layer lifecycle manager to hide/show or remove placeholder layers safely, improved logic to pick visible existing layers, and a safer implementation to move image layers to bottom. Uses manager.active_dlc_image_layer instead of deprecated helper.
- API note: find_relevant_image_layer is marked deprecated in favor of LayerLifecycleManager.active_dlc_image_layer.

These changes let the plugin reliably identify DLC project/video sessions, attach session metadata to Napari layers, and prevent multiple conflicting DLC sessions from being opened simultaneously.
Detect and merge temporary "config placeholder" Points layers into existing managed points workflows, updating headers, presentation metadata, dropdowns, and UI state, then remove the placeholder layer. KeypointControls: added _is_config_placeholder_points_layer and rewrote _maybe_merge_config_points_layer to use a managed layer as the reference, merge header/bodypart changes, refresh menus defensively, apply colormap/face-color cycles, and remove the placeholder via QTimer. LayerLifecycleManager: made active_dlc_image_layer resolve by identity across viewer.layers (safer than resolve_live_layer) and short-circuit insertion when a placeholder layer is consumed by the widget merge. Tests updated with a helper (mark_as_dlc_session_image) and adjusted assertions to reflect the new merge/move behavior. Also adjusted a deprecation decorator parameter name in core/layers.py.
Add explicit session conflict handling and a UI notification hook.

- Introduce a new Signal session_conflict_rejected(str) on LayerLifecycleManager to notify the UI when a DLC labeled-data folder conflicts with the current session.
- Change the rejection flow to emit the signal and defer layer removal via QTimer.singleShot(0) instead of removing the layer synchronously (avoids Napari list-insertion/finalization issues).
- Expand the rejection message to provide clearer guidance to users about loading annotations vs switching projects.
- Wire the new signal into KeypointControls: attach LayerLifecycleManager, connect session_conflict_rejected to a new _on_session_conflict_detected handler, and show a QMessageBox warning when a conflict is detected.
- Improve logging when deferred removal fails.

This change brings better user feedback when loading several projects.
Wrap widget removal in a safe try/except. In KeypointStore, skip revalidating header metadata when the same live layer is rebound (check by id) to avoid repeated expensive reads and add a debug log. In LayerLifecycleManager add a post-remove coalescing QTimer and flush handler to batch UI refreshes after layer removals, make _update_image_meta_from_layer return a bool indicating whether the authoritative image context changed, and only sync points layers when the context actually changed. Also adjust layer setup/remap logic to only remap non-image layers when necessary and improve related logging. Overall these changes reduce unnecessary work, prevent transient exceptions, and coalesce UI updates during layer changes.
Introduce a lightweight log_timing contextmanager for scoped performance measurements and a NAPARI_DLC_LOG_TIMING flag to control sampling. Instrument several lifecycle and UI hotspots (Points/Image/Tracks removal, viewer.layers.remove, post-remove refresh, and active-layer changes) to emit timing logs when enabled. Enhance the InMemoryDebugRecorder snapshot formatting to include milli-second precision timestamps, relative elapsed ms per record, and guard the empty-record case. These changes aid low-overhead debugging and performance investigation.
@C-Achard C-Achard changed the title Improve layer management/lifecycle Plugin refactor: Improve layer management/lifecycle Apr 22, 2026
@C-Achard
C-Achard changed the base branch from cy/refactor-fix-traj-plot-y-axis-rebased to cy/feature-refactor-2026 April 27, 2026 14:20
@C-Achard C-Achard changed the title Plugin refactor: Improve layer management/lifecycle Plugin refactor [8]: Improve layer management/lifecycle Apr 27, 2026
@C-Achard
C-Achard marked this pull request as ready for review April 27, 2026 14:21
@C-Achard
C-Achard requested a review from Copilot April 27, 2026 14:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 48 out of 50 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/napari_deeplabcut/widget_factory.py Resolved
Comment thread src/napari_deeplabcut/core/io.py Resolved
Comment thread src/napari_deeplabcut/core/io.py Resolved
Return early from guarantee_multiindex_rows when the DataFrame has no rows to avoid indexing into df.index[0]. Added test_guarantee_multiindex_rows_empty_df_is_noop to assert an empty DataFrame remains unchanged (length 0 and not a MultiIndex).
Introduce widget-owned timer management to replace ad-hoc QTimer.singleShot usage. Add _timers and _temp_timers, plus _schedule_once and _single_shot_owned helpers to coalesce named single-shot callbacks and track transient timers. Connect _cleanup_timers to destroyed to defensively stop/delete timers during teardown and avoid RuntimeError from torn-down C++ objects. Use the new scheduling API for startup refresh/dock/status tasks and deferred recolor flushing to improve robustness and prevent stray timers from firing after widget destruction.
Adjust logging and minor docs/comments for clarity and less noise:

- core/io.py: Downgrade missing-HDF5-key errors to warnings when trying fallback keys and when no expected keys are found; make multianimal config debug message include the actual value resolved from config.
- core/dataframes.py: Rename parameter in the docstring from `is_ma_from_config` to `is_ma_project` to better reflect purpose.
- widget_factory.py: Update module header comment path to match actual file name.

These changes reduce spurious error logs when reading HDF5 files, improve debug output for multianimal project resolution, and clarify naming/comments.
Use the most recent save history entry if available (falling back to the user's home directory) as the default folder in the save dialog. Simplify the points-layer fast-path check so it also applies when a layer lacks a safe folder anchor (previously excluded machine layers). Adjust InMemoryDebugRecorder to only return an empty string when there are no records and no dropped entries, avoiding premature empty results.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 50 out of 52 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/napari_deeplabcut/core/io.py Resolved
Comment thread src/napari_deeplabcut/widget_factory.py Resolved
Comment thread src/napari_deeplabcut/utils/debug.py Resolved
Add a suite of unit tests for infer_dlc_project_from_labeled_folder to validate behavior when no config is present, when a project config exists (both preferring project root and keeping dataset anchor), respecting max_levels bounds when searching upward, and rejecting invalid inputs. Update test imports to include infer_dlc_project_from_labeled_folder. Also remove a stale FIXME comment in project_paths.py; no functional code changes were made. These tests increase coverage for project path inference logic and document expected anchoring behavior.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 50 out of 52 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/napari_deeplabcut/utils/debug.py Resolved
Annotate encode_categories with @deprecated and a note to move it out of misc.py (likely to io.py where it's used). Add TODO comments indicating CycleEnumMeta and CycleEnum should be moved to keypoints.py. No other functional changes.
Add extensive debug logging in read_hdf_single and write_hdf to help diagnose DataFrame column/header layouts (columns names, sample columns, header.bodyparts, stacked df unique bodyparts/individuals, header/properties at write, and final output columns). Update harmonize_keypoint_column_index comment to clarify that inserting an empty "individuals" level is only for internal single-animal comparisons and should not be written to disk.
Add helper _replace_layer_metadata_in_place to update a Points layer's metadata in-place (clear+update) and ensure metadata is initialized when None. Update _temporary_layer_metadata to capture and restore the original mapping contents instead of reassigning layer.metadata, and use the new helper in PointsLayerSaveWorkflow when persisting save-time metadata changes. This avoids replacing the metadata mapping object (which could break external references) while still updating its contents.
@C-Achard
C-Achard merged commit 2884999 into cy/feature-refactor-2026 Apr 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants