refactor: default reporter, Comparison::Images, driver registry get SnapDiff homes (ADR-008 steps 4+5)

docs/architecture.md

@@ -158,7 +158,7 @@ Drivers abstract image processing operations. Shared default behavior lives in t
 
 Handles baseline retrieval from git. Uses `git show HEAD:<path>` to extract the committed version. Supports Git LFS via `git lfs smudge`. Returns `false` if the file doesn't exist in VCS (first-run scenario).
 
-### 9. Reporters (`lib/capybara/screenshot/diff/reporters/default.rb`, `lib/snap_diff/reporters/html.rb`)
+### 9. Reporters (`lib/snap_diff/reporters/default.rb`, `lib/snap_diff/reporters/html.rb`)
 
 **Default reporter:** Generates annotated diff images:
 - `image.diff.png` — new screenshot with diff region outlined in red
@@ -281,7 +281,6 @@ lib/
   capybara/screenshot/diff/
     config_legacy.rb                           # mattr_accessor settings storage (source of truth)
     region.rb                                  # Bounding box region value object (top-level Region)
-    reporters/default.rb                       # Default annotated-image reporter
     version.rb                                 # Capybara::Screenshot::Diff::VERSION (gemspec reads it)
     ...                                        # Everything else forwards to snap_diff/
 ```

lib/capybara/screenshot/diff/image_compare.rb

@@ -6,7 +6,10 @@
 # snap_diff/comparison itself pulls in the ComparisonResult and Drivers
 # units, and the shims keep the old ::Difference / ::Drivers names
 # resolvable, so this path still provides everything the pre-move
-# image_compare.rb did. The internal Comparison struct and LOADED_DRIVERS
-# keep their legacy names and are defined by snap_diff/comparison.rb itself.
+# image_compare.rb did. The internal images-holder struct lives at
+# SnapDiff::Comparison::Images (its old ::Comparison name resolves via the
+# shims) and the driver cache at SnapDiff::Drivers.loaded, with
+# LOADED_DRIVERS kept as an eager same-object alias by legacy_shims
+# (ADR-008 step 5).
 require "snap_diff/comparison"
 require "snap_diff/legacy_shims"

lib/capybara/screenshot/diff/reporters/default.rb

@@ -1,109 +1,7 @@
 # frozen_string_literal: true
 
-require "snap_diff/annotation_service"
-# Defines the Capybara::Screenshot::Diff namespace this file reopens.
+# Forwarder (ADR-008 step 4): the default reporter lives at
+# SnapDiff::Reporters::Default; the old name now resolves lazily via
+# snap_diff/legacy_shims' const_missing, with a deprecation warning.
+require "snap_diff/reporters/default"
 require "snap_diff/legacy_shims"
-
-module Capybara::Screenshot::Diff
-  module Reporters
-    class Default
-      attr_reader :difference
-
-      def initialize(difference)
-        @difference = difference
-        @annotation_service = SnapDiff::AnnotationService.new(difference)
-      end
-
-      def annotated_image_path
-        annotation_service.annotated_image_path
-      end
-
-      def annotated_base_image_path
-        annotation_service.annotated_base_image_path
-      end
-
-      def heatmap_diff_path
-        annotation_service.heatmap_diff_path
-      end
-
-      def generate
-        if difference.equal?
-          # NOTE: Delete previous run runtime files
-          clean_tmp_files
-          return nil
-        end
-
-        if difference.failed? && difference.failed_by[:different_dimensions]
-          return build_error_for_different_dimensions
-        end
-
-        annotate_and_save_images
-        build_error_message
-      end
-
-      def clean_tmp_files
-        annotation_service.clean_tmp_files
-      end
-
-      def annotate_and_save_images
-        annotation_service.annotate_and_save_images
-      end
-
-      def save_annotation_for(image, image_path)
-        annotation_service.save_annotation_for(image, image_path)
-      end
-
-      def annotate_difference(image, region)
-        annotation_service.annotate_difference(image, region)
-      end
-
-      def annotate_skip_areas(image, skip_areas)
-        annotation_service.annotate_skip_areas(image, skip_areas)
-      end
-
-      def save(image, image_path)
-        annotation_service.save(image, image_path)
-      end
-
-      def build_error_for_different_dimensions
-        change_msg = [comparison.base_image, comparison.new_image]
-          .map { |image| driver.dimension(image).join("x") }
-          .join(" => ")
-
-        "Dimensions have changed: #{change_msg}\n#{base_image_path.to_path}\n#{image_path.to_path}"
-      end
-
-      NEW_LINE = "\n"
-
-      def build_error_message
-        [
-          "(#{difference.to_h.to_json})",
-          image_path.to_path,
-          annotated_base_image_path.to_path,
-          annotated_image_path.to_path,
-          heatmap_diff_path.to_path
-        ].join(NEW_LINE)
-      end
-
-      private
-
-      attr_reader :annotation_service
-
-      def base_image_path
-        comparison.base_image_path
-      end
-
-      def image_path
-        comparison.new_image_path
-      end
-
-      def driver
-        @_driver ||= comparison.driver
-      end
-
-      def comparison
-        @_comparison ||= difference.comparison
-      end
-    end
-  end
-end

lib/snap_diff/comparison.rb

@@ -6,28 +6,7 @@
 require "snap_diff/comparison_result"
 require "snap_diff/drivers"
 require "snap_diff/image_preprocessor"
-require "capybara/screenshot/diff/reporters/default"
-
-# The internal images-holder struct and the driver cache keep their legacy
-# Capybara::Screenshot::Diff homes for now: SnapDiff::Comparison is the
-# comparison class below (ex-ImageCompare), so the struct cannot take the
-# same name. Its SnapDiff home arrives only when it is folded into
-# Comparison as a nested value (v2 design section 2) -- renaming it here
-# would exceed step 5's two approved renames.
-module Capybara
-  module Screenshot
-    module Diff
-      LOADED_DRIVERS = {}
-
-      # Holds the two images (and their paths/options/driver) being compared.
-      class Comparison < Struct.new(:new_image, :base_image, :options, :driver, :new_image_path, :base_image_path)
-        def skip_area
-          options[:skip_area]
-        end
-      end
-    end
-  end
-end
+require "snap_diff/reporters/default"
 
 module SnapDiff
   # Handles comparison of two images with a focus on performance and accuracy.
@@ -53,6 +32,14 @@ module SnapDiff
   # - Only performing expensive operations when absolutely necessary
   # - Maintaining high accuracy for complex comparisons
   class Comparison
+    # Holds the two images (and their paths/options/driver) being compared
+    # (ADR-008 step 5: ex-Capybara::Screenshot::Diff::Comparison struct).
+    Images = Struct.new(:new_image, :base_image, :options, :driver, :new_image_path, :base_image_path) do
+      def skip_area
+        options[:skip_area]
+      end
+    end
+
     TOLERABLE_OPTIONS = [:tolerance, :color_distance_limit, :shift_distance_limit, :area_size_limit].freeze
 
     attr_reader :driver, :driver_options
@@ -136,7 +123,7 @@ def without_tolerable_options?
 
     def load_images_and_build_comparison(base_path, new_path, options)
       base_img, new_img = driver.load_images(base_path, new_path)
-      Capybara::Screenshot::Diff::Comparison.new(new_img, base_img, options, driver, new_path, base_path)
+      Images.new(new_img, base_img, options, driver, new_path, base_path)
     end
 
     def image_preprocessor
@@ -155,7 +142,7 @@ def find_difference(quick_mode: false)
 
     # Analyzes the comparison and determines if images are different.
     #
-    # @param comparison [Capybara::Screenshot::Diff::Comparison] The comparison object containing images to analyze.
+    # @param comparison [Comparison::Images] The comparison object containing images to analyze.
     # @param quick_mode [Boolean] When true, performs minimal checks and returns early.
     #   In quick mode, returns [is_equal, difference] where:
     #   - is_equal is true if images are considered equal
@@ -195,31 +182,31 @@ def difference=(new_difference)
 
     def build_reporter
       current_difference = difference || build_null_difference
-      Capybara::Screenshot::Diff::Reporters::Default.new(current_difference)
+      Reporters::Default.new(current_difference)
     end
 
     # Loads and preprocesses images for detailed comparison.
     #
     # This method is responsible for:
     # 1. Loading both images using the configured driver
     # 2. Applying any necessary preprocessing (cropping, normalization)
-    # 3. Creating a Capybara::Screenshot::Diff::Comparison object that holds the image data
+    # 3. Creating a Comparison::Images object that holds the image data
     #
     # @param base_path [String,Pathname] Path to the baseline/reference image
     # @param new_path [String,Pathname] Path to the new/candidate image
     # @param options [Hash] Comparison options including:
     #   - :crop [Array<Integer>] Optional crop area [x, y, width, height]
     #   - :skip_area [Array<Array>] Areas to exclude from comparison
     #   - :tolerance [Numeric] Color tolerance threshold
-    # @return [Capybara::Screenshot::Diff::Comparison] Prepared comparison object ready for analysis
+    # @return [Comparison::Images] Prepared comparison object ready for analysis
     # @raise [ArgumentError] If image files are invalid or unreadable
     def load_comparison(base_path, new_path, options)
       comparison = load_images_and_build_comparison(base_path, new_path, options)
       image_preprocessor.process_comparison(comparison)
     end
 
     def build_null_difference(failed_by = nil)
-      comparison = Capybara::Screenshot::Diff::Comparison.new(nil, nil, driver_options, driver, image_path, base_image_path).freeze
+      comparison = Images.new(nil, nil, driver_options, driver, image_path, base_image_path).freeze
       ComparisonResult.build_null(comparison, base_image_path, image_path, failed_by)
     end
 

lib/snap_diff/drivers.rb

@@ -10,5 +10,25 @@ def self.for(driver_options = {})
 
       Utils.find_driver_class_for(driver_option).new
     end
+
+    # Canonical driver-class cache (ADR-008 step 5b, ex
+    # Capybara::Screenshot::Diff::LOADED_DRIVERS): driver name => driver
+    # class, filled lazily by Utils.find_driver_class_for. Mutated in
+    # place -- including by user registration through the legacy constant,
+    # which legacy_shims pins as an EAGER same-object alias of this hash
+    # (a lazy copy would silently drop such registrations).
+    def self.loaded
+      @loaded ||= {}
+    end
+
+    # Canonical read API for the detected-drivers list. The value itself
+    # stays on Capybara::Screenshot::Diff::AVAILABLE_DRIVERS (assigned in
+    # config_legacy.rb at load time, exactly when detection historically
+    # ran); this reads it live rather than caching, because that constant
+    # is the published stubbing point (image_compare_test stubs it to []
+    # to exercise the no-drivers error path).
+    def self.available
+      Capybara::Screenshot::Diff::AVAILABLE_DRIVERS
+    end
   end
 end

lib/snap_diff/legacy_shims.rb

@@ -24,6 +24,14 @@
 #   shared SnapDiff::Drivers module (the Drivers alias is same-object by
 #   contract), so const_missing can never fire for the leaf names;
 #   resolving them through the old path still warns for ...::Drivers.
+# - Diff::LOADED_DRIVERS: user code registers custom drivers by mutating
+#   this hash in place, so it must be the exact same object as the
+#   canonical SnapDiff::Drivers.loaded -- a lazy warn-once shim could not
+#   keep a mutable alias, and warning on a supported registration surface
+#   would be noise. Assigned eagerly below.
+# - Diff::AVAILABLE_DRIVERS: stays a real constant defined by
+#   config_legacy.rb (detection runs at that load moment);
+#   SnapDiff::Drivers.available is the canonical reader.
 module SnapDiff
   # @api private
   module LegacyShims
@@ -47,6 +55,11 @@ def self.install(namespace, old_prefix, mapping)
 module Capybara
   module Screenshot
     module Diff
+      # EAGER same-object alias of the canonical driver cache (see header).
+      LOADED_DRIVERS = SnapDiff::Drivers.loaded
+
+      module Reporters
+      end
     end
   end
 end
@@ -71,9 +84,14 @@ module Reporters
   ScreenshotMatcher: "SnapDiff::ScreenshotMatcher",
   Drivers: "SnapDiff::Drivers",
   ImageCompare: "SnapDiff::Comparison",
+  Comparison: "SnapDiff::Comparison::Images",
   Difference: "SnapDiff::ComparisonResult"
 }.freeze)
 
+SnapDiff::LegacyShims.install(Capybara::Screenshot::Diff::Reporters, "Capybara::Screenshot::Diff::Reporters", {
+  Default: "SnapDiff::Reporters::Default"
+}.freeze)
+
 SnapDiff::LegacyShims.install(CapybaraScreenshotDiff, "CapybaraScreenshotDiff", {
   RED_RGBA: "SnapDiff::RED_RGBA",
   ORANGE_RGBA: "SnapDiff::ORANGE_RGBA",