Address review nitpicks: model validation, scope placement, mock style, lock-safe DDL

All four checked against the repo's own conventions before applying.

Mirror the check constraint as a model validation. Assigning
reconciled_by_statement without reconciled_at raised StatementInvalid rather
than a validation error. CLAUDE.md Convention 5 asks for exactly this pairing --
the constraint in the database, an ActiveRecord validation for form-friendly
errors.

Stop hijacking the pending-scope comment. The reconciliation scopes were
inserted directly under "Pending transaction scopes", so that header read as
documentation for them and the provider note below read as a continuation of
reconciled_by. Given the reconciliation scopes their own header.

Use OpenStruct for the provider response double, per "Always prefer OpenStruct
when creating mock instances". Verified OpenStruct.new(success?: true) responds
to success?, and ostruct is already a dependency used elsewhere in test/.

Make the migration lock-safe. entries is the largest table in the app: both
indexes now build concurrently, and the check constraint is added unvalidated
then validated separately so VALIDATE takes only SHARE UPDATE EXCLUSIVE instead
of holding ACCESS EXCLUSIVE for a full scan. This follows existing practice --
13 migrations already use disable_ddl_transaction! and 11 use algorithm:
:concurrently, with add_offline_reason_to_securities combining add_column and a
concurrent index in one migration exactly like this. The suggested follow-up
migration for validation was not needed: validating in the same non-
transactional migration gets the same lock behavior without a second file, and
the repo has no validate: false precedent in 400 migrations.

schema.rb is unchanged: a validated constraint and a concurrently-built index
dump identically.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KvWSJotDiqP6qSR7qEiC8Q

app/models/entry.rb

@@ -14,6 +14,10 @@ class Entry < ApplicationRecord
   belongs_to :parent_entry, class_name: "Entry", optional: true
   belongs_to :reconciled_by_statement, class_name: "AccountStatement", optional: true
 
+  # Mirrors chk_entries_reconciled_at_present_when_statement_set so a direct
+  # assignment surfaces a validation error rather than a StatementInvalid.
+  validates :reconciled_at, presence: true, if: -> { reconciled_by_statement_id.present? }
+
   has_many :child_entries, class_name: "Entry", foreign_key: :parent_entry_id, dependent: :destroy
 
   delegated_type :entryable, types: Entryable::TYPES, dependent: :destroy
@@ -51,11 +55,12 @@ class Entry < ApplicationRecord
     )
   }
 
-  # Pending transaction scopes - check Transaction.extra for provider pending flags
+  # Reconciliation scopes - see AddReconciliationToEntries
   scope :reconciled, -> { where.not(reconciled_at: nil) }
   scope :unreconciled, -> { where(reconciled_at: nil) }
   scope :reconciled_by, ->(statement) { where(reconciled_by_statement_id: statement) }
 
+  # Pending transaction scopes - check Transaction.extra for provider pending flags
   # Works with any provider that stores pending status in extra["provider_name"]["pending"]
   scope :pending, -> {
     conditions = Transaction::PENDING_PROVIDERS.map { |p| "(transactions.extra -> '#{p}' ->> 'pending')::boolean = true" }

db/migrate/20260820120000_add_reconciliation_to_entries.rb

@@ -1,4 +1,10 @@
 class AddReconciliationToEntries < ActiveRecord::Migration[7.2]
+  # entries is the largest table in the app, so the indexes are built
+  # concurrently and the check constraint is added unvalidated then validated
+  # separately -- VALIDATE takes only SHARE UPDATE EXCLUSIVE, so neither step
+  # blocks writes for the length of a full scan.
+  disable_ddl_transaction!
+
   # Reconciliation follows the Quicken model of uncleared / cleared / reconciled,
   # but only the last state is stored.
   #
@@ -25,27 +31,31 @@ class AddReconciliationToEntries < ActiveRecord::Migration[7.2]
     add_index :entries,
               :reconciled_by_statement_id,
               where: "reconciled_by_statement_id IS NOT NULL",
-              name: "index_entries_on_reconciled_by_statement"
+              name: "index_entries_on_reconciled_by_statement",
+              algorithm: :concurrently
 
     # Supports the per-account reconciled/unreconciled split the import review
     # screen and the account ledger both need.
     add_index :entries,
               [ :account_id, :reconciled_at ],
               where: "reconciled_at IS NOT NULL",
-              name: "index_entries_on_account_and_reconciled_at"
+              name: "index_entries_on_account_and_reconciled_at",
+              algorithm: :concurrently
 
     # An entry can be reconciled without a statement on file (marked by hand, or
     # the statement was later deleted and the FK nulled), but it can never point
     # at a statement without being reconciled.
     add_check_constraint :entries,
                          "reconciled_by_statement_id IS NULL OR reconciled_at IS NOT NULL",
-                         name: "chk_entries_reconciled_at_present_when_statement_set"
+                         name: "chk_entries_reconciled_at_present_when_statement_set",
+                         validate: false
+    validate_check_constraint :entries, name: "chk_entries_reconciled_at_present_when_statement_set"
   end
 
   def down
     remove_check_constraint :entries, name: "chk_entries_reconciled_at_present_when_statement_set"
-    remove_index :entries, name: "index_entries_on_account_and_reconciled_at"
-    remove_index :entries, name: "index_entries_on_reconciled_by_statement"
+    remove_index :entries, name: "index_entries_on_account_and_reconciled_at", algorithm: :concurrently
+    remove_index :entries, name: "index_entries_on_reconciled_by_statement", algorithm: :concurrently
     remove_reference :entries, :reconciled_by_statement, type: :uuid, foreign_key: { to_table: :account_statements }
     remove_column :entries, :reconciled_at
   end

test/models/pdf_import_reconciliation_test.rb

@@ -120,7 +120,7 @@ class PdfImportReconciliationTest < ActiveSupport::TestCase
     provider = mock("llm_provider")
     Provider::Registry.stubs(:preferred_llm_provider).returns(provider)
     provider.stubs(:extract_bank_statement).returns(
-      stub(success?: true, data: { transactions: [ { date: @date.to_s, amount: "-5.0", name: "Coffee" } ] })
+      OpenStruct.new(success?: true, data: { transactions: [ { date: @date.to_s, amount: "-5.0", name: "Coffee" } ] })
     )
     @import.stubs(:pdf_file_content).returns("fake-pdf")