feat: name the region that would not settle, and what the waiting cost (#271)

Stabilisation failures printed a bare list of attempt paths. Diagnosing one
meant opening N PNGs and eyeballing them -- so `sleep 2` won, and suites got
SLOWER as a consequence of diagnosis being hard. A maintainer reported a
10-minute suite dominated by stabilisation waiting, with sleeps adopted
deliberately "to avoid debugging as much as possible".

The information was already here and thrown away: AttemptsReporter compares
every consecutive pair of attempts, and that comparison knows the region that
changed. Print it, with the escape hatch:

  Could not get stable screenshot for 'index-with-ticker' within 1.2s (5 attempts).
    The page kept changing in 1 area, over 4 attempt pairs:
      [67,50,213,68] (left,top,right,bottom edges) -- 0.55% of the 800x600 image, changed in 4 of 4 pairs
    Always the same area, in every pair: that is an animation, clock, carousel or live counter.
    Exclude it and the page is stable without waiting:
      assert_matches_screenshot "index-with-ticker", skip_area: [67,50,213,68]
    <attempt paths>

Animation vs churn is decided by count, not by shape: regions are clustered by
overlap, and a cluster present in EVERY attempt pair is animating -- skip_area
fixes it. Anything less is the page still rendering, where masking would hide
real content, so the message says so and suggests nothing to mask.

The suggested coordinates are the ones just measured. Guarded by following the
advice on a real browser and a really unstable page (test/fixtures/app/
index-with-ticker.html): the failing run's own suggestion, pasted back in,
makes the page stable. Fabricating the coordinate reds that test -- which is
the check this gem lacked when it shipped RECORD_SCREENSHOTS=1 in its own
error message for years while nothing read it.

Success path: the run-level summary now reports the worst stabilisation it
saw. A user who set `stability_time_limit: 2` had no way to learn their pages
settle on the first retry, and without evidence tuning it down is guesswork.
Run-level rather than per-assertion (per-test noise is the last thing a slow
suite needs) and silent when nothing waited -- the same rule as the
never-matched-selector line. It rides the fork-parallel fragment, since a
run-level line that vanishes under Rails' default parallelize is #269 again;
counts add, worst cases max.

Pairs with #272 (masking is instant) and #279 (dead selectors are surfaced):
"here is the region, mask it" is finally a complete workflow.

Author: pftg

docs/configuration.md

@@ -356,6 +356,53 @@ test 'stability_time_limit' do
 end
 ```
 
+### When the page will not settle
+
+The failure names the area that kept changing, and hands you the command that
+fixes it:
+
+```
+Could not get stable screenshot for 'index-with-ticker' within 1.2s (5 attempts).
+  The page kept changing in 1 area, over 4 attempt pairs:
+    [67,50,213,68] (left,top,right,bottom edges) -- 0.55% of the 800x600 image, changed in 4 of 4 pairs
+  Always the same area, in every pair: that is an animation, clock, carousel or live counter.
+  Exclude it and the page is stable without waiting:
+    assert_matches_screenshot "index-with-ticker", skip_area: [67,50,213,68]
+  <one annotated attempt image per line>
+```
+
+The coordinates are measured, not guessed: they come from the comparisons the
+gem just ran between consecutive attempts, so pasting the suggested `skip_area`
+in works.
+
+Read the "changed in N of N pairs" line before acting on it:
+
+- **N of N — one area, every single pair.** Something is animating in one place:
+  a clock, a carousel, a spinner, a live counter. `skip_area` is the fix, and
+  since masking no longer waits it costs nothing.
+- **Fewer than N, or several areas each changing once.** The page is still
+  *rendering*, not animating. Masking those areas would hide real content. The
+  message says so and suggests nothing to mask — settle the page in a
+  [readiness block](#the-readiness-block) instead, or raise `wait:`.
+
+### Knowing what the waiting cost
+
+Every run that waited for stability ends with what it actually paid:
+
+```
+[snap_diff] 34 screenshots waited for the page to settle: 0.19s and 2 attempts at worst. Every screenshot settled on its first retry, so a lower stability_time_limit would cost less per screenshot.
+```
+
+Two attempts is the floor — one capture, plus the retry that matched it. Hitting
+the floor across the whole run means no page was ever still moving when the
+retry was taken, so every `stability_time_limit` sleep was spent on a page that
+had already stopped. That is the evidence for tuning it *down*; without it,
+lowering the setting is guesswork, and guesswork loses to `sleep`.
+
+Run-level and silent when nothing waited, for the same reason as the
+never-matched-selector line: a line printed on every screenshot is a line
+people learn to skip.
+
 ### Maximum wait limit
 
 When the `stability_time_limit` is set, but no stable screenshot can be taken, a timeout occurs.

lib/snap_diff/attempts_reporter.rb

@@ -1,23 +1,48 @@
 # frozen_string_literal: true
 
 require "fileutils"
+require "json"
 
 require "snap_diff/comparison"
+require "snap_diff/region"
 
 module SnapDiff
+  # The message a user reads when a page would not hold still.
+  #
+  # It used to be a bare list of attempt paths (#271). That made diagnosis a
+  # matter of opening N PNGs and eyeballing them, and faced with that versus
+  # `sleep 2`, sleep wins -- so the suite got SLOWER as a consequence of the
+  # diagnosis being hard. The information needed was already here and thrown
+  # away: every consecutive pair of attempts is compared, and that comparison
+  # knows the region that changed.
+  #
+  # So name it, and hand over the escape hatch that removes the need to sleep.
   class AttemptsReporter
+    # One place on the page that changed between attempts, and how many of
+    # the attempt pairs it showed up in. `pairs == total` means it changed
+    # EVERY time -- an animation, and `skip_area` is the fix. Fewer means the
+    # page was still rendering there, and masking would hide a real change.
+    Area = Struct.new(:region, :pairs)
+
     def initialize(snapshot, comparison_options, stability_options = {})
       @snapshot = snapshot
       @comparison_options = comparison_options
       @wait = stability_options[:wait]
     end
 
     def generate
-      attempts_screenshot_paths = @snapshot.find_attempts_paths
+      # Sorted: `attempt_%02i` sorts lexically in capture order, and the
+      # reader wants oldest-first regardless of what the glob hands back.
+      attempts_screenshot_paths = @snapshot.find_attempts_paths.sort
 
-      annotate_attempts(attempts_screenshot_paths)
+      areas, dimensions = annotate_attempts(attempts_screenshot_paths)
 
-      "Could not get stable screenshot within #{@wait}s:\n#{attempts_screenshot_paths.join("\n")}"
+      [
+        "Could not get stable screenshot for '#{@snapshot.full_name}' within #{@wait}s " \
+          "(#{attempts_screenshot_paths.size} attempts).",
+        *diagnosis_lines(areas, dimensions),
+        *attempts_screenshot_paths
+      ].join("\n")
     end
 
     def build_comparison_for(attempt_path, previous_attempt_path)
@@ -26,14 +51,25 @@ def build_comparison_for(attempt_path, previous_attempt_path)
 
     private
 
+    # Annotates each attempt with its diff against the next one -- and keeps
+    # the regions, which is the whole point of #271.
+    #
+    # @return [Array(Array<Area>, Array(Integer, Integer))] the clustered
+    #   changed areas and the [width, height] of the attempts.
     def annotate_attempts(attempts_screenshot_paths)
+      regions = []
+      dimensions = nil
       previous_file = nil
+
       attempts_screenshot_paths.reverse_each do |file_name|
         if previous_file && File.exist?(previous_file)
           attempts_comparison = build_comparison_for(file_name, previous_file)
 
           if attempts_comparison.different?
             FileUtils.mv(attempts_comparison.reporter.annotated_base_image_path, previous_file, force: true)
+            region = attempts_comparison.difference.region
+            regions << region if region
+            dimensions ||= dimensions_of(attempts_comparison)
           else
             warn "[capybara-screenshot-diff] Some attempts was stable, but mistakenly marked as not: " \
                    "#{previous_file} and #{file_name} are equal"
@@ -45,7 +81,101 @@ def annotate_attempts(attempts_screenshot_paths)
         previous_file = file_name
       end
 
-      previous_file
+      # Worst offender first: the area that changed in the most pairs is the
+      # one to act on, and a stable order keeps the message diffable.
+      areas = cluster(regions).sort_by { |area| [-area.pairs, -area.region.size] }
+
+      [areas, dimensions]
+    end
+
+    def dimensions_of(comparison)
+      images = comparison.difference.comparison
+      comparison.driver.dimension(images.base_image) if images&.base_image
+    end
+
+    # Groups the per-pair regions into the places on the page they occupy: a
+    # region that overlaps one we have already seen is the same place, moved
+    # or resized, so the place grows to cover both.
+    #
+    # ponytail: first-overlap wins, so a chain of regions that each overlap
+    # the next merges into one area. That is the right answer for the case
+    # this message exists for (something animating in one spot) and only ever
+    # UNDER-counts areas, which cannot turn churn into a masking suggestion.
+    def cluster(regions)
+      regions.each_with_object([]) do |region, areas|
+        existing = areas.find { |area| area.region.intersect?(region) }
+        if existing
+          existing.region = union(existing.region, region)
+          existing.pairs += 1
+        else
+          areas << Area.new(region, 1)
+        end
+      end
+    end
+
+    def union(one, other)
+      Region.from_edge_coordinates(
+        [one.left, other.left].min,
+        [one.top, other.top].min,
+        [one.right, other.right].max,
+        [one.bottom, other.bottom].max
+      )
+    end
+
+    def diagnosis_lines(areas, dimensions)
+      return [] if areas.empty? || dimensions.nil?
+
+      total_pairs = areas.sum(&:pairs)
+      # An area that changed in EVERY pair is animating. One that did not is
+      # the page still rendering -- masking it would hide a real change.
+      animating, settling = areas.partition { |area| area.pairs == total_pairs }
+
+      [
+        "  The page kept changing in #{count(areas.size, "area")}, over #{count(total_pairs, "attempt pair")}:",
+        *areas.map { |area| "    #{area_line(area, total_pairs, dimensions)}" },
+        *animating_lines(animating),
+        *settling_lines(settling, animating)
+      ]
+    end
+
+    def area_line(area, total_pairs, dimensions)
+      width, height = dimensions
+      share = area.region.size.to_f / (width * height)
+
+      "#{area.region.to_edge_coordinates.to_json} (left,top,right,bottom edges) " \
+        "-- #{Reporters::Default.percent(share)} of the #{width}x#{height} image, " \
+        "changed in #{area.pairs} of #{total_pairs} pairs"
+    end
+
+    # The escape hatch. Every coordinate here came off the comparison that
+    # just ran -- never a placeholder, and never a knob nothing reads.
+    def animating_lines(animating)
+      return [] if animating.empty?
+
+      skip_area = animating.map { |area| area.region.to_edge_coordinates }
+      skip_area = skip_area.first if skip_area.size == 1
+
+      [
+        "  Always the same area, in every pair: that is an animation, clock, carousel or live counter.",
+        "  Exclude it and the page is stable without waiting:",
+        "    assert_matches_screenshot #{@snapshot.full_name.to_s.inspect}, skip_area: #{skip_area.to_json}"
+      ]
+    end
+
+    def settling_lines(settling, animating)
+      return [] if settling.empty?
+
+      subject = animating.empty? ? "D" : "The other #{count(settling.size, "area")}: d"
+
+      [
+        "  #{subject}ifferent areas at different times -- the page is still rendering, not animating in one place.",
+        "  skip_area masks a fixed area and will not help here: settle the page first (a readiness",
+        "  block on the assertion -- see docs/configuration.md) or raise wait:."
+      ]
+    end
+
+    def count(number, noun)
+      "#{number} #{noun}#{"s" unless number == 1}"
     end
   end
 end

lib/snap_diff/reporters/default.rb

@@ -61,6 +61,14 @@ def build_error_for_different_dimensions
 
       NEW_LINE = "\n"
 
+      # The one place the gem turns a fraction of the image into prose.
+      # Public because AttemptsReporter reports the same kind of number and
+      # must say it the same way (#264 vocabulary).
+      def self.percent(fraction)
+        value = fraction * 100
+        (value.positive? && value < 0.01) ? "<0.01%" : format("%.2f%%", value)
+      end
+
       # The thresholds a comparison is judged against, in the order they read
       # best. Only the ones actually set are printed -- see #thresholds.
       THRESHOLDS = [
@@ -146,8 +154,7 @@ def display_path(path)
       end
 
       def percent(fraction)
-        value = fraction * 100
-        (value.positive? && value < 0.01) ? "<0.01%" : format("%.2f%%", value)
+        self.class.percent(fraction)
       end
 
       def base_image_path

lib/snap_diff/reporting.rb

@@ -23,6 +23,9 @@ module Reporting
     @unmatched_selectors = Set.new
     @verified = 0
     @changed = 0
+    @stable_captures = 0
+    @worst_settle_seconds = 0.0
+    @worst_settle_attempts = 0
 
     class << self
       attr_reader :reporters, :mutex
@@ -74,6 +77,19 @@ def record_selector_use(selector, matched:)
         end
       end
 
+      # Remembers what a capture that DID settle cost (#271).
+      #
+      # Only the worst case is kept, because that is the number the setting
+      # has to cover: an average would suggest a `stability_time_limit` that
+      # is too low for the slowest page in the suite.
+      def record_stable_capture(seconds, attempts)
+        @mutex.synchronize do
+          @stable_captures += 1
+          @worst_settle_seconds = seconds if seconds > @worst_settle_seconds
+          @worst_settle_attempts = attempts if attempts > @worst_settle_attempts
+        end
+      end
+
       # @api private
       # Per-test isolation for this gem's own suite: everything {finalize!}
       # reports, cleared in one call. One surface rather than one reset per
@@ -86,6 +102,9 @@ def reset_run_totals!
           @unmatched_selectors.clear
           @verified = 0
           @changed = 0
+          @stable_captures = 0
+          @worst_settle_seconds = 0.0
+          @worst_settle_attempts = 0
         end
       end
 
@@ -222,6 +241,10 @@ def finalize!
         if (msg = never_matched_selectors_summary)
           $stdout.puts msg
         end
+
+        if (msg = stable_captures_summary)
+          $stdout.puts msg
+        end
       end
 
       # --- fork-parallel reports (issue #258) ---------------------------
@@ -279,6 +302,9 @@ def dump_parallel_fragment
           "unmatched_selectors" => @mutex.synchronize { @unmatched_selectors.to_a },
           "verified" => @verified,
           "changed" => @changed,
+          "stable_captures" => @stable_captures,
+          "worst_settle_seconds" => @worst_settle_seconds,
+          "worst_settle_attempts" => @worst_settle_attempts,
           "reporters" => @mutex.synchronize { @reporters.dup }
             .map { |reporter| reporter.dump_state if reporter.respond_to?(:dump_state) }
         }
@@ -315,6 +341,11 @@ def merge_parallel_fragments!
             payload.fetch("unmatched_selectors", []).each { |selector| @unmatched_selectors << selector }
             @verified += payload.fetch("verified", 0)
             @changed += payload.fetch("changed", 0)
+            # Counts add up; worst cases do not -- the slowest page in the
+            # run is the slowest page in whichever worker happened to run it.
+            @stable_captures += payload.fetch("stable_captures", 0)
+            @worst_settle_seconds = [@worst_settle_seconds, payload.fetch("worst_settle_seconds", 0.0)].max
+            @worst_settle_attempts = [@worst_settle_attempts, payload.fetch("worst_settle_attempts", 0)].max
           end
 
           reporters_snapshot = @mutex.synchronize { @reporters.dup }
@@ -383,6 +414,45 @@ def never_matched_selectors_summary
           "#{names.map(&:inspect).join(", ")}. " \
           "A selector that matches nothing masks nothing -- check for a typo or a stale selector."
       end
+
+      # What waiting for the page to settle actually cost, on the runs where
+      # it WORKED (#271).
+      #
+      # The failure path names the region that would not settle; this is the
+      # other half. A maintainer who set `stability_time_limit: 2` on the
+      # docs' recommendation has no way to learn their pages settle on the
+      # first retry -- and without evidence, tuning it down is guesswork,
+      # which loses to `sleep`. The measurement is free: the stable
+      # screenshoter already knows both numbers at the moment it succeeds.
+      #
+      # Run-level rather than per-assertion, and silent when nothing waited:
+      # the same reasoning as {never_matched_selectors_summary}. A line
+      # printed on every screenshot of every run is a line users learn to
+      # skip, and per-test noise is exactly what a debugging aid must not
+      # add to a suite already too slow.
+      #
+      # @return [String, nil] nil when no capture waited for stability
+      def stable_captures_summary
+        captures, seconds, attempts = @mutex.synchronize {
+          [@stable_captures, @worst_settle_seconds, @worst_settle_attempts]
+        }
+        return if captures.zero?
+
+        label = (captures == 1) ? "1 screenshot" : "#{captures} screenshots"
+        line = "[snap_diff] #{label} waited for the page to settle: " \
+          "#{format("%.2f", seconds)}s and #{attempts} attempts at worst."
+
+        # Two attempts is the floor -- one capture, then the retry that
+        # matched it. Hitting the floor everywhere means no page in the run
+        # was ever still moving, so every sleep between attempts was spent
+        # on a page that had already stopped.
+        if attempts <= 2
+          line += " Every screenshot settled on its first retry, so a lower " \
+            "stability_time_limit would cost less per screenshot."
+        end
+
+        line
+      end
     end
   end
 end