fix: address multi-agent journey review findings

Critical:
- at_exit output_path access moved into finalize (no crash on custom reporters)
- at_exit registered inside unless guard (no double registration)
- README Quick Start adds include modules (was missing, NoMethodError)
- relative_path uses expand_path to handle absolute vs relative paths

High:
- matrix-screenshot-driver: always() → failure() (no artifact noise on pass)
- README "What You Get" clarifies .base.png is runtime copy, not baseline

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: pftg

.github/workflows/test.yml

@@ -218,7 +218,7 @@ jobs:
           SCREENSHOT_DRIVER: ${{ matrix.screenshot-driver }}
 
       - uses: ./.github/actions/upload-screenshots
-        if: always()
+        if: failure()
         with:
           name: screenshots-${{ matrix.capybara-driver }}-${{ matrix.screenshot-driver }}
 

README.md

@@ -19,8 +19,13 @@ require 'capybara_screenshot_diff/minitest'
 ```
 
 ```ruby
-# test/system/homepage_test.rb
-class HomepageTest < ActionDispatch::SystemTestCase
+# test/application_system_test_case.rb (or test/system/homepage_test.rb)
+class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
+  include CapybaraScreenshotDiff::DSL
+  include CapybaraScreenshotDiff::Minitest::Assertions
+end
+
+class HomepageTest < ApplicationSystemTestCase
   test "homepage" do
     visit "/"
     screenshot "homepage"  # First run: saves baseline. Next runs: compares.
@@ -42,8 +47,8 @@ When a screenshot differs, you get:
 
 | File | Description |
 |------|-------------|
-| `homepage.png` | Current screenshot |
-| `homepage.base.png` | Committed baseline |
+| `homepage.png` | Committed baseline (in version control) |
+| `homepage.base.png` | Runtime copy of baseline (for comparison) |
 | `homepage.diff.png` | Visual diff with changes highlighted in red |
 | `homepage.heatmap.diff.png` | Heatmap of pixel differences |
 

docs/configuration.md

@@ -215,7 +215,7 @@ test 'color threshold' do
 end
 ```
 
-The difference is calculated as the eucledian distance.  You can also set this globally:
+The difference is calculated as the euclidean distance.  You can also set this globally:
 
 ```ruby
 Capybara::Screenshot::Diff.color_distance_limit = 42

lib/capybara_screenshot_diff/reporters/html.rb

@@ -43,7 +43,7 @@ def finalize
         end
 
         write_report
-        true
+        output_path
       end
 
       def passed = total - failures.size
@@ -82,10 +82,10 @@ def failure_entry_for(name, compare)
       def relative_path(path)
         return unless path
 
-        pathname = Pathname.new(path)
+        pathname = Pathname.new(path).expand_path
         return unless pathname.exist?
 
-        pathname.relative_path_from(@report_dir).to_s
+        pathname.relative_path_from(@report_dir.expand_path).to_s
       end
 
       def write_report
@@ -98,13 +98,13 @@ def write_report
 
 unless CapybaraScreenshotDiff.reporters.any?(CapybaraScreenshotDiff::Reporters::HTML)
   CapybaraScreenshotDiff.reporters << CapybaraScreenshotDiff::Reporters::HTML.new
-end
 
-at_exit do
-  CapybaraScreenshotDiff.reporters.each do |reporter|
-    wrote = reporter.finalize
-    $stdout.puts "[snap_diff] HTML report: #{reporter.output_path}" if wrote
-  rescue => e
-    warn "[snap_diff] Reporter #{reporter.class} failed (#{e.class}: #{e.message})" if ENV["DEBUG"]
+  at_exit do
+    CapybaraScreenshotDiff.reporters.each do |reporter|
+      result = reporter.finalize
+      $stdout.puts "[snap_diff] HTML report: #{result}" if result.is_a?(Pathname)
+    rescue => e
+      warn "[snap_diff] Reporter #{reporter.class} failed (#{e.class}: #{e.message})" if ENV["DEBUG"]
+    end
   end
 end