sequel: sole and sole!, and fix the change-log double
`sole_or_nil` was a name for what Ruby already has a convention for.
`sole` answers nil where no row is the ordinary answer, `sole!` raises
Sequel::NoMatchingRow where it is a corruption, and both still raise
TooManyRows: the eleven reads in Store say which they meant, and the
nil half gets the unit tests it never had.

The change-log double in StoreTest still took #record's four
positionals, which is the Test job's three failures on b1d0704.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WWFQaLD58uLFedh3Dvfb2C
change
commit fbcb40e05aa7836c25d25b85fb0ef4b7927c1cb6
author Claude <noreply@anthropic.com>
date
parent b1d0704f
diff --git a/AGENTS.md b/AGENTS.md
index 09eff06..d974def 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -210,13 +210,13 @@ not a fixture; edit a `.vcf` to change what the replay serves.
   so no repair is ever needed for it. A write that touches a card must
   land its change-log entry in the same transaction, because a client's
   sync token silently skips whatever the log missed.
-- A read whose filter is meant to identify one row uses `sole`, not
-  `first` — `first` answers with one of several rather than saying the
-  filter was too loose. `sole` raises either way it is wrong:
-  `Sequel::NoMatchingRow` for none, `Sequel::Sole::TooManyRows` for
-  more. A caller for whom no row is ordinary rescues the first, which is
-  what `Store#contact` does for the 404 path. The store loads the
-  extension on every open.
+- A read whose filter is meant to identify one row uses `sole` or
+  `sole!`, not `first` — `first` answers with one of several rather
+  than saying the filter was too loose. Both raise
+  `Sequel::Sole::TooManyRows` for more than one row. They differ on
+  none: `sole!` raises `Sequel::NoMatchingRow`, and `sole` answers nil
+  for a read where no row is ordinary, which is what `Store#contact`
+  does for the 404 path. The store loads the extension on every open.
 - An etag is derived from the card, never stored beside it: `Contact`
   hashes what it serves, and its constructor is the only way to make
   one — no second path takes an etag on trust. The etag in `changes` is
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 0c911d8..f03401b 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -342,7 +342,7 @@ module ProTacts
     # the ordinary answer for an href nobody has — the 404 path.
     #: (String id) -> Contact?
     def contact(id)
-      row = cards.where(id:).sole_or_nil
+      row = cards.where(id:).sole
       row && contact_from(row, birthday_of(id), inherited_of(id))
     end
 
@@ -388,7 +388,7 @@ module ProTacts
     # is set either way.
     #: (String login) -> String
     def book_name(login)
-      row = books.where(login:).sole_or_nil
+      row = books.where(login:).sole
       row ? row.fetch(:name).to_s : login
     end
 
@@ -422,7 +422,7 @@ module ProTacts
     # cards sharing a UID is a corruption to raise on, not a choice.
     #: (String uid) -> String?
     def card_id_with_uid(uid)
-      row = card_properties.where(name: "UID", value: uid).sole_or_nil
+      row = card_properties.where(name: "UID", value: uid).sole
       row && row.fetch(:card_id).to_s
     end
 
@@ -570,10 +570,10 @@ module ProTacts
     # keep serving the dead href out of its cache forever.
     #: (String id) -> String
     def reid(id)
-      # `sole` rather than #contact's nil-for-404: a read whose filter
+      # `sole!` rather than #contact's nil: a read whose filter
       # means one row, and no row here is the caller's mistake to hear
       # about as Sequel::NoMatchingRow, not a request to answer.
-      before = contact_from(cards.where(id:).sole, birthday_of(id), inherited_of(id))
+      before = contact_from(cards.where(id:).sole!, birthday_of(id), inherited_of(id))
 
       CONTACT_ID_ATTEMPTS.times do
         new_id = ChangeId.mint(ChangeId::CONTACT_LENGTH)
@@ -601,7 +601,7 @@ module ProTacts
             # rather than moved — the deal #reindex always gives it. The
             # parameters follow their properties away on the cascade.
             card_properties.where(card_id: id).delete
-            after = contact_from(cards.where(id: new_id).sole, birthday_of(new_id), inherited_of(new_id))
+            after = contact_from(cards.where(id: new_id).sole!, birthday_of(new_id), inherited_of(new_id))
             record(id, action: Action::DELETE, etag: nil, diff: CardDiff.between(before.vcard, nil))
             record(
               new_id,
@@ -662,7 +662,7 @@ module ProTacts
     # #contact's own shape.
     #: (String id) -> Group?
     def group(id)
-      row = groups.select(*GROUP_COLUMNS, group_label.as(:label)).where(id:).sole_or_nil
+      row = groups.select(*GROUP_COLUMNS, group_label.as(:label)).where(id:).sole
       row && load_groups([row]).fetch(0)
     end
 
@@ -676,7 +676,7 @@ module ProTacts
     def rename_group(id, name:)
       name = nil if name.to_s.strip.empty?
       @database.transaction do
-        was = groups.where(id:).sole.fetch(:name)&.to_s
+        was = groups.where(id:).sole!.fetch(:name)&.to_s
         if was != name && (sync_name?(was) || sync_name?(name))
           members = member_ids([id])
           fan_out(members, moved: members) { groups.where(id:).update(name:) }
@@ -757,7 +757,7 @@ module ProTacts
     # the ordinary miss, #delete's shape.
     #: (String id) -> bool
     def delete_group(id)
-      row = groups.where(id:).sole_or_nil
+      row = groups.where(id:).sole
       return false if row.nil?
 
       name = row.fetch(:name).to_s
@@ -961,7 +961,7 @@ module ProTacts
     # (db/migrations/008_group_names.rb).
     #: () -> String
     def everyone_group_id
-      row = groups.where(name: EVERYONE).sole_or_nil
+      row = groups.where(name: EVERYONE).sole
       row ? row.fetch(:id).to_s : create_group(name: EVERYONE)
     end
 
@@ -978,7 +978,7 @@ module ProTacts
 
     #: (String group_id) -> bool
     def sync_group?(group_id)
-      sync_name?(groups.where(id: group_id).sole.fetch(:name)&.to_s)
+      sync_name?(groups.where(id: group_id).sole!.fetch(:name)&.to_s)
     end
 
     #: (String group_id, String card_id) -> bool
@@ -999,11 +999,11 @@ module ProTacts
     end
 
     # A group_members row names a card by foreign key, so a missing
-    # row here is the corruption `sole` exists to raise on rather than
+    # row here is the corruption `sole!` exists to raise on rather than
     # the ordinary miss an href off the wire is.
     #: (String id) -> Contact
     def contact!(id)
-      contact_from(cards.where(id:).sole, birthday_of(id), inherited_of(id))
+      contact_from(cards.where(id:).sole!, birthday_of(id), inherited_of(id))
     end
 
     # Replaces a card's rows in the index. Rebuilt wholesale rather than
diff --git a/lib/pro_tacts/web/contacts.rb b/lib/pro_tacts/web/contacts.rb
index 36ede4b..399ff6c 100644
--- a/lib/pro_tacts/web/contacts.rb
+++ b/lib/pro_tacts/web/contacts.rb
@@ -190,7 +190,7 @@ module ProTacts
     # `was` is the boxes checked when the page loaded, so a group joined
     # or left elsewhere since stays as it is, and a stale page has
     # nothing to revert and no snapshot to refuse. Only ids naming a
-    # group: Store#add_member reads the group with `sole`, so a doctored
+    # group: Store#add_member reads the group with `sole!`, so a doctored
     # one would be a 500 rather than the bad input it is. A `new` name
     # is a group the filter box offered to create, joined in the same
     # save; a blank one is none.
diff --git a/lib/sequel/extensions/sole.rb b/lib/sequel/extensions/sole.rb
index 7390e05..a489d13 100644
--- a/lib/sequel/extensions/sole.rb
+++ b/lib/sequel/extensions/sole.rb
@@ -2,19 +2,20 @@
 require "sequel"
 
 module Sequel
-  # The row a filter identifies, or a raise.
+  # The row a filter identifies, and what to do when there is not one.
   #
   # A filter meant to identify one row — a lookup by primary key, later
   # the group that contributed a property to a card — reads naturally as
   # `first`, and `first` answers with one of several without a word when
   # the filter turns out not to identify anything. That is the failure
-  # worth catching: the query claimed one row, so anything else is a
-  # broken assumption rather than a result to pick from.
+  # worth catching: the query claimed one row, so more than one is a
+  # broken assumption rather than a result to pick from, and both of
+  # these raise on it.
   #
-  # Both ways of being wrong raise, so the assertion is in one method
-  # rather than split across a pair. A caller for whom no row is
-  # ordinary says so by rescuing Sequel::NoMatchingRow, which is the
-  # error Sequel's own `first!` raises for it.
+  # Where the two differ is no row at all, which is the ordinary answer
+  # for some reads — an href nobody has, a login whose book was never
+  # named — and a corruption for others. `sole` answers nil there and
+  # `sole!` raises, which is what the bang says everywhere else in Ruby.
   #
   # Load it with `DB.extension(:sole)`, which reaches every dataset the
   # database makes.
@@ -25,7 +26,7 @@ module Sequel
     class TooManyRows < Sequel::Error; end
 
     #: () -> Hash[Symbol, untyped]
-    def sole
+    def sole!
       # Two is enough to know, and a LIMIT keeps a filter that turned out
       # to match the whole table from dragging it back to prove the
       # point. It replaces any limit already set, which is why this is
@@ -41,14 +42,12 @@ module Sequel
       rows.fetch(0)
     end
 
-    # The same read where no row is the ordinary answer — an href
-    # nobody has, a login whose book was never named — so the caller
-    # says so once here rather than spelling the rescue out again.
-    # Only the empty case: more rows than the filter claimed is still
-    # the broken assumption #sole exists to raise on.
+    # Sequel::NoMatchingRow is what Sequel's own `first!` raises for an
+    # empty dataset, so the rescue names one error rather than this
+    # extension's own.
     #: () -> Hash[Symbol, untyped]?
-    def sole_or_nil
-      sole
+    def sole
+      sole!
     rescue Sequel::NoMatchingRow
       nil
     end
diff --git a/sig/gems/sequel.rbs b/sig/gems/sequel.rbs
index 32b10bc..d5955f8 100644
--- a/sig/gems/sequel.rbs
+++ b/sig/gems/sequel.rbs
@@ -154,7 +154,8 @@ module Sequel
   class Error < StandardError
   end
 
-  # What `first!` raises on an empty dataset, and `sole` with it.
+  # What `first!` raises on an empty dataset, `sole!` with it, and
+  # `sole` rescues to answer nil.
   class NoMatchingRow < Error
     def initialize: (?untyped message_or_dataset) -> void
   end
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index ab3d55c..a544beb 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1302,7 +1302,7 @@ class StoreTest < Minitest::Test
     with_store do |store|
       id = store.create_group
 
-      assert_nil database(store)[:groups].where(id:).sole.fetch(:name)
+      assert_nil database(store)[:groups].where(id:).sole!.fetch(:name)
       assert_raises(Sequel::ConstraintViolation) { store.create_group(name: "") }
     end
   end
@@ -2149,7 +2149,7 @@ class StoreTest < Minitest::Test
   class FailingLog < ProTacts::Store
     private
 
-    def record(card_id, action, etag, diff)
+    def record(card_id, action:, etag:, diff:)
       raise "the change log is unavailable"
     end
   end
@@ -2167,7 +2167,7 @@ class StoreTest < Minitest::Test
   end
 
   def card_row(store, id)
-    database(store)[:cards].where(id:).sole
+    database(store)[:cards].where(id:).sole!
   end
 
   def indexed_names(store, card_id)
diff --git a/test/sequel/extensions/test_sole.rb b/test/sequel/extensions/test_sole.rb
index 16c852a..17b55b1 100644
--- a/test/sequel/extensions/test_sole.rb
+++ b/test/sequel/extensions/test_sole.rb
@@ -23,7 +23,7 @@ class SoleTest < Minitest::Test
 
   def test_the_one_row_comes_back
     with_rows(%w[a b]) do |things|
-      assert_equal "body of a", things.where(id: "a").sole.fetch(:body)
+      assert_equal "body of a", things.where(id: "a").sole!.fetch(:body)
     end
   end
 
@@ -31,12 +31,33 @@ class SoleTest < Minitest::Test
   # for whom no row is ordinary has one error to name.
   def test_no_rows_raises
     with_rows(%w[a]) do |things|
-      assert_raises(Sequel::NoMatchingRow) { things.where(id: "nobody").sole }
+      assert_raises(Sequel::NoMatchingRow) { things.where(id: "nobody").sole! }
     end
   end
 
   # The half `first` gets wrong: it would answer with one of the two.
   def test_more_than_one_row_raises
+    with_rows(%w[a a]) do |things|
+      assert_raises(Sequel::Sole::TooManyRows) { things.where(id: "a").sole! }
+    end
+  end
+
+  def test_sole_answers_the_one_row
+    with_rows(%w[a b]) do |things|
+      assert_equal "body of a", things.where(id: "a").sole.fetch(:body)
+    end
+  end
+
+  # The half the pair exists for: a read where no row is the answer
+  # rather than a corruption.
+  def test_sole_answers_nil_for_no_rows
+    with_rows(%w[a]) do |things|
+      assert_nil things.where(id: "nobody").sole
+    end
+  end
+
+  # More than one is the broken assumption either way it is asked.
+  def test_sole_still_raises_for_more_than_one_row
     with_rows(%w[a a]) do |things|
       assert_raises(Sequel::Sole::TooManyRows) { things.where(id: "a").sole }
     end
@@ -46,7 +67,7 @@ class SoleTest < Minitest::Test
   # filter can carry card content, so the message names the table only.
   def test_the_message_names_the_table_and_not_the_query
     with_rows(%w[a a]) do |things|
-      error = assert_raises(Sequel::Sole::TooManyRows) { things.where(body: "body of a").sole }
+      error = assert_raises(Sequel::Sole::TooManyRows) { things.where(body: "body of a").sole! }
 
       assert_equal "more than one row in things", error.message
       refute_includes error.message, "body of a"
@@ -78,7 +99,7 @@ class SoleTest < Minitest::Test
       log = QueryLog.new
       things.db.loggers << log
 
-      assert_raises(Sequel::Sole::TooManyRows) { things.sole }
+      assert_raises(Sequel::Sole::TooManyRows) { things.sole! }
       assert(log.queries.any? { it.include?("LIMIT 2") }, log.queries.inspect)
     end
   end