refactor: SnapDiff::Config becomes the single config storage (ADR-008 step 1)

[pftg]

ADR-008 step 1b: the config storage inversion — the riskiest PR of the accepted end-state plan, kept deliberately small and revertable.

What changed

Before: the legacy mattr_accessors on Capybara::Screenshot / Capybara::Screenshot::Diff were the storage; SnapDiff::Config was a view forwarding every read/write to them.

After: SnapDiff::Config (the eager SnapDiff.config singleton) IS the single storage; the legacy accessors are thin delegators generated from Config::MAPPING. One storage, two views — bidirectional visibility is structural, not synchronized. The adopter-visible write surface (Capybara::Screenshot.window_size = ..., Capybara::Screenshot::Diff.tolerance = ..., both configure shapes, SnapDiff.start / SnapDiff.configure) is unchanged.

Require topology (stays acyclic)

• snap_diff/config.rb is the new leaf of the config graph. It predefines the empty Capybara::Screenshot::Diff module skeleton (same technique legacy_shims.rb already uses) so MAPPING's module references resolve at class-body eval, and requires only snap_diff/screenshoter + snap_diff/snap_manager (needed as default values at require time; neither requires back).
• config_legacy.rb now requires snap_diff/config (plus snap_diff/utils for AVAILABLE_DRIVERS) and installs the delegators. The old config.rb -> config_legacy edge is deleted, so the dependency arrow flipped without ever being bidirectional.
• The snap_diff entry still never loads the umbrella (snap_diff_test.rb acyclicity guard green); docs/thread_safety.md load-order section updated to name the new leaf.

Default timing contract (#223 guard: 12/12 green, untouched)

Setting	Moment	Where it now evaluates
fail_if_new (ENV["CI"])	require time, frozen	Config#initialize, run by the eager Config.new at the bottom of snap_diff/config.rb — the same load moment the mattr default block ran at
root (Rails.root / pwd)	require time, frozen	same (raw Rails.root, no coercion — coercion only in the Config#root= writer, as before)
default_options[:wait]	call time, live	unchanged method-body read of Capybara.default_max_wait_time in Diff.default_options — never stored
all other mapped settings	literal defaults	Config#initialize ivars; both surfaces read/write the same ivar

Delegator surface parity: mattr_accessor defined both singleton and instance accessors (include Capybara::Screenshot::Diff relies on the instance ones), so both are installed; root keeps its historical asymmetry (instance reader only, module-level coercing writer).

Mutation evidence (each applied, run, reverted)

1. Legacy write dropped (Diff.fail_if_new = x no longer reaches storage): snap_diff_config_test red — 2 failures, both legacy-write round-trip tests (writing fail_if_new via the old mattr_accessor is visible via config, same for window_size). Timing guard stays green for this direction (it pins defaults, not writes — the round-trip tests are the write guard).
2. Legacy read frozen at load (config writes invisible through legacy readers): snap_diff_config_test red — 5 failures (every mapped setting is readable..., the root and window_size round-trips, screenshot_enabled/enabled independence, SnapDiff.configure set-through). The fail_if_new round-trip passes only coincidentally under this mutation: with CI unset, the frozen initial false happens to match the value the test writes back.
3. wait frozen into load-time storage: timing guard red — 4 failures, the default_options[:wait] follows Capybara.default_max_wait_time set after require probe, once per entry point (expected 42.5, got 2).

Adversarial-review fixes (post-open)

• Per-test isolation regression fixed (4118a53): nil-defaulted settings had no ivar until first write, so test_helper's ivar snapshot missed them and a mid-test legacy write (e.g. tolerance) leaked into the next test. Config#initialize now pre-sets every MAPPING key to nil before the explicit defaults, so all 27 ivars always exist. TDD evidence: the new "stores exactly one ivar per MAPPING key" assertion landed first and was red (17 ivars present — including leaked @window_size/@disable_animations from earlier tests, demonstrating the leak — vs 27 expected), green after the fix.
• Reflection test reworked (50bb057): the old class_variables-based completeness check passed vacuously post-inversion. Replaced with two directions: (a) every singleton writer on the legacy modules must appear in MAPPING — enumeration found 13 writers on Capybara::Screenshot and 14 on Capybara::Screenshot::Diff, all mapped, so the explicit NON_CONFIG_WRITERS exclusion list is currently empty — and (b) SnapDiff.config stores exactly one ivar per MAPPING key (also guards test_helper snapshot completeness).
• Dropped the now-unused active_support/core_ext/module/attribute_accessors require in snap_manager.rb — nothing in lib uses mattr_* since the inversion; full suite green confirms nothing else needed it loaded.

Test results

• Baseline at 40204de: rake test:unit 504 runs / 0 failures. After (incl. review fixes): rake test:unit 505 runs / 1442 assertions / 0 failures; rake test 538 runs / 0 failures / 0 errors (6 pre-existing skips); standardrb clean.
• test/test_helper.rb per-test isolation snapshotted class_variables; it now snapshots SnapDiff.config's instance variables (e954ba7) — otherwise isolation would have silently become a no-op.

Rollback plan

Revert all four commits in one step (lib inversion + test-helper isolation + review fixes — the helper and the reworked guard read Config ivars, so they travel with the inversion in either direction):

git revert --no-edit 50bb057 4118a53 e954ba7 f3bfd0d

What signals rollback:

• Any red in test/unit/config_default_timing_test.rb (the 12-test timing guard) on this branch or after merge — a default's evaluation moment shifted.
• Any entry-point probe divergence: snap_diff_test.rb load-order/standalone probes, or the timing guard's per-entry-point probes disagreeing between capybara_screenshot_diff, capybara_screenshot_diff/minitest, snap_diff, capybara/screenshot/diff.
• Any adopter report of a legacy write (Capybara::Screenshot.x = ...) not being visible through SnapDiff.config or vice versa.

🤖 Generated with Claude Code

[sourcery-ai]

Sorry @pftg, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Review limit reached

@pftg, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 48 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 48fca38b-0824-4771-aa93-4a3bb0ae23e5

📥 Commits

Reviewing files that changed from the base of the PR and between 40204de and 50bb057.

📒 Files selected for processing (7)

• docs/thread_safety.md
• lib/capybara/screenshot/diff/config_legacy.rb
• lib/snap_diff.rb
• lib/snap_diff/config.rb
• lib/snap_diff/snap_manager.rb
• test/test_helper.rb
• test/unit/snap_diff_config_test.rb

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sourcery-ai]

Reviewer's Guide

SnapDiff::Config is refactored into the single source of truth for configuration, with legacy Capybara::Screenshot / Capybara::Screenshot::Diff accessors turned into delegators, the require graph kept acyclic, default-evaluation timing preserved, and test isolation updated to snapshot the new storage.

File-Level Changes

Change	Details	Files
Move configuration storage from legacy mattr_accessors to SnapDiff::Config as the single authoritative store, with legacy surfaces delegating to it.	Introduce SnapDiff::Config as the sole holder of config state using instance variables and explicit defaults in Config#initialize. Define the legacy Capybara::Screenshot::Diff module skeleton in the config file so MAPPING can reference it without additional requires. Install delegator methods on Capybara::Screenshot and Capybara::Screenshot::Diff singleton and instance surfaces that read/write SnapDiff.config attributes per MAPPING. Preserve the special handling of root via an instance reader and module-level writer with Pathname coercion now implemented in Config#root=.	lib/snap_diff/config.rb lib/capybara/screenshot/diff/config_legacy.rb
Reshape the require topology so snap_diff/config becomes the config leaf and the graph stays acyclic while maintaining existing entry points.	Make snap_diff/config.rb require only snap_diff/screenshoter and snap_diff/snap_manager and no longer require config_legacy. Have config_legacy.rb require snap_diff/config and snap_diff/utils, reversing the previous dependency direction. Adjust comments in snap_diff.rb to describe the new position of config_legacy above the snap_diff/config leaf and leave SnapDiff.config definition to the leaf file. Update documentation to describe snap_diff/config as the config-storage leaf with legacy view config_legacy depending on it.	lib/snap_diff/config.rb lib/capybara/screenshot/diff/config_legacy.rb lib/snap_diff.rb docs/thread_safety.md
Preserve and pin default-evaluation timing and live-read behavior for configuration values, especially fail_if_new, root, and default_options[:wait].	Evaluate all config defaults once in Config#initialize at require time via an eager Config.new, matching the legacy mattr_accessor evaluation moment. Implement fail_if_new default from ENV["CI"] and root default from Rails.root or current directory using Pathname in Config#initialize. Leave default_options[:wait] as a call-time read of Capybara.default_max_wait_time within Diff.default_options, not stored in config. Document the timing contract in comments tied to config_default_timing_test.rb so changes that break timing are detectable.	lib/snap_diff/config.rb lib/capybara/screenshot/diff/config_legacy.rb
Update test infrastructure to isolate configuration state via SnapDiff.config instance variables instead of legacy class variables.	Replace GLOBAL_STATE_MODULES/class_variable snapshotting with capturing SnapDiff.config instance_variables before each test. Restore SnapDiff.config instance variables from the snapshot in teardown to avoid cross-test leakage. Clarify comments that the single storage since ADR-008 step 1 is SnapDiff.config, and legacy surfaces delegate to it.	test/test_helper.rb

Possibly linked issues

• #ADR-004: The PR directly implements ADR-004 Phase 1 PR 6 by consolidating mattr_accessor storage into SnapDiff::Config.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.