From c3589b308969ff013b8c11a080c88a19dcee990b Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 15:50:55 -0500 Subject: [PATCH 01/24] docs(decks): spec and plan for deck checks (#258) Spec 079 (Approved v0.2.1) and its plan, each reviewed read-only on Opus: the spec twice, the plan once. Adds the specs index row. Co-Authored-By: Claude Sonnet 5.5 --- docs/specs/079-deck-checks/plan.md | 1181 ++++++++++++++++++++++++++++ docs/specs/079-deck-checks/spec.md | 305 +++++++ docs/specs/README.md | 1 + 3 files changed, 1487 insertions(+) create mode 100644 docs/specs/079-deck-checks/plan.md create mode 100644 docs/specs/079-deck-checks/spec.md diff --git a/docs/specs/079-deck-checks/plan.md b/docs/specs/079-deck-checks/plan.md new file mode 100644 index 00000000..7417ecde --- /dev/null +++ b/docs/specs/079-deck-checks/plan.md @@ -0,0 +1,1181 @@ +# Implementation Plan: Deck Checks (spec 079, issue #258) + +**Spec:** `docs/specs/079-deck-checks/spec.md` (Approved v0.2.1) · **Issue:** #258 · **Branch:** `079-deck-checks` · **Created:** 2026-11-12 +**Approved 2026-11-12** (the maintainer); plan review by Opus (read-only) 2026-11-12: APPROVE WITH CHANGES, applied. Retired by `distill-feature` after the merge (spec 027). + +## Context + +A collector can't tell whether a deck meets its format's rules. Spec 079 adds a **Check deck** link to the deck page; following it shows, on that page, the deck's size and copy-limit results and every card that has owned copies and proxies in one zone. The result is computed on request (a GET), never stored, and never blocks an edit. A game's rules live behind the collecting vocabulary (a third optional seam answer, `deck_rules`), so the core names no collectible, format or zone key (`spec/deck_core_spec.rb`). Owned-plus-proxy is a core check and runs for every deck. + +## Global Constraints + +- **Repo rules:** `CLAUDE.md`, `.claude/rules/*.md`. Specs tag their type (`type: :model|:request|:system`); `RSpec/ExampleLength` max 15 lines; `RSpec/MultipleMemoizedHelpers` max 5 (use `def`, not `let`); multi-expectation examples use `:aggregate_failures`; a multi-case AC is one example per case in an `:aggregate_failures` group (spec 079, Interactions). +- **Core files name nothing.** `spec/deck_core_spec.rb` reads whole files, comments included, with `/mtg|scryfall|magic/i` anywhere and `/(? [ true, true ]` becomes three; `:249` and `:258` need no edit) and `:264-273` (heights ≥ 36 now also cover the new link). `spec/requests/deck_pages_spec.rb:64-74` is scoped to the Edit link and stays. +- **Fixtures that can fail:** each counting AC has a fixture whose wrong answer differs from the right one. The statement-count example and each narrow-width assertion are broken once on purpose (mutation proofs, recorded in `verification.md`). + +## Goal + +Press **Check deck** on any deck page and see, between the head and the zones, a "Deck check" section: a summary ("3 findings", "1 finding", "No findings") and, in order, one result per size rule, the copy check and owned-plus-proxy. A deck whose format has no rules gets the no-rules line, then owned-plus-proxy. + +## Pre-implementation gates + +- **Simplicity (≤3 components): pass.** (1) `Catalog::DeckChecks` (core seam, counting, words), (2) `MTG::DeckRules` + `MTG::Collecting.deck_rules` (Magic's table and limits), (3) `Decks::ChecksController` + `decks/_check` + one CSS block. No gem, model or table. +- **Anti-abstraction: pass.** The `Data.define` types are the contract itself, not wrappers; the controller renders the existing `decks/show` with one more ivar. The three lines that build `Deck::Page` and the shape answer are repeated in the controller, not extracted (two callers). +- **Integration-first: pass.** The seam's types and contract comment, with their specs, are Phase 1, before Magic, the controller or the view. +- **Complexity note (T4.1):** a new design-system pattern (`DeckCheck`) is needed. No class gives a wrapping status-word + sentence row: `c-badge` is nowrap and limited to two words ("Over the limit" and "Owned and proxy" are three), `c-list li` doesn't wrap, and the deck page has no block between head and `c-deck`. Documented as an addition per `CLAUDE.md`. + +## Files + +| File | Change | Responsibility | +|---|---|---| +| `app/models/catalog/deck_checks.rb` | create | core seam: types, `report`, the three checks, words, the contract comment | +| `app/models/mtg/deck_rules.rb` | create | Magic's rules table and each printing's limit | +| `app/models/mtg/collecting.rb` | edit | `deck_rules(format_key)` delegating to `MTG::DeckRules` | +| `config/routes.rb` | edit | `resource :check, only: :show, module: :decks` | +| `app/controllers/decks/checks_controller.rb` | create | GET: build page, shape and report; render `decks/show` | +| `app/views/decks/show.html.erb`, `_check.html.erb` | edit, create | the link, the no-cache tag, the results section | +| `app/assets/stylesheets/collector/additions.css` | edit | the `c-check` block | +| `docs/design-system/components/DeckCheck.md`, `DeckPage.md`, `docs/design-system/README.md` | create, edit | the pattern's document; the page's head link and section; the additions line | +| `spec/deck_core_spec.rb`, `spec/system/deck_page_spec.rb` | edit | guard list and count; the head's three buttons | +| `spec/models/catalog/deck_checks_spec.rb`, `spec/models/mtg/deck_rules_spec.rb`, `spec/models/mtg/deck_checks_for_magic_spec.rb`, `spec/requests/deck_checks_spec.rb`, `spec/system/deck_checks_spec.rb` | create | the proofs | +| `docs/architecture/collection-and-lots.md`, `CLAUDE.md`, `docs/roadmap.md`, `docs/specs/README.md`, `docs/specs/079-deck-checks/verification.md` | edit, create | Story 7 | + +## Phase 0: Preflight (no code) + +**Implements:** execution preamble · **Satisfies:** supports AC-3.4, 3.5, 3.12, 3.13 + +- [ ] **T0.1** `git switch -c 079-deck-checks` from `main` (uncommitted spec, plan and index row travel with it), then one commit: `docs(decks): spec and plan for deck checks (#258)`. +- [ ] **T0.2 Confirm the unverified Scryfall facts**, once, with `curl` (`User-Agent: Collector-dev` and `Accept: application/json` as `.claude/rules/external-data-and-portability.md` asks, 100 ms between calls, no private data): `https://api.scryfall.com/cards/named?exact=Relentless+Rats`, `…exact=Seven+Dwarves`, `…exact=Nazg%C3%BBl` and `https://api.scryfall.com/cards/search?q=%21%22Lightning+Bolt%22+lang%3Aja&unique=prints`. Check the `oracle_text` wording ("A deck can have any number of cards named …", "… up to seven cards named …", "… up to nine …") and that `name` is English while `printed_name` is Japanese. Quote the lines in `verification.md` ("Scryfall facts"). **If a wording differs, update `MTG::DeckRules::ANY_NUMBER` / `STATED_NUMBER` and the T2.1 fixtures before T2.1; if `name` isn't English for a non-English printing, stop and raise it (spec decision 6).** + +## Phase 1: The core seam (contracts first) + +**Implements:** FR-1, FR-2; decisions 1, 2, 4, 5, 6, 10 · **Satisfies:** AC-1.7, 2.1–2.6, 2.9–2.13 (words and counts), 3.1–3.3, 3.5, 3.6, 3.8–3.11, 3.13, 3.15 (core), 4.1–4.10, 5.4, 5.5 +**Files:** `app/models/catalog/deck_checks.rb`, `spec/models/catalog/deck_checks_spec.rb`, `spec/deck_core_spec.rb` +**Interfaces:** Produces `Catalog::DeckChecks.report(deck_type, format_key, zones) → Report`; `SizeRule(zone_keys, min, max)`, `Limit(count, reason)`, `CopyRule(zone_keys, limit_for)`, `Rules(sizes, copies)`, `Line(status, sentence, fault)`, `Report(lines)` with `#faults` and `#summary`. Consumes `Deck::Page::Zone/Group/Row/Line`, `Catalog::Formats.name_for/zones_for`, `Catalog.collecting`. + +A fake vocabulary stands in for the game, so these examples prove the core with no Magic in them. Top of `spec/models/catalog/deck_checks_spec.rb` (shared helpers, defined once and used by every task below): + +```ruby +require "rails_helper" + +RSpec.describe Catalog::DeckChecks, type: :model do + def zone(key, name, *rows) + location = Struct.new(:zone, :name).new(key, name) + Deck::Page::Zone.new(location:, groups: rows.empty? ? [] : [ Deck::Page::Group.new(name: nil, rows:) ]) + end + + def card(name, id, type: "game") + build_stubbed(:catalog_entry, identity: build_stubbed(:catalog_identity, id:, name:), name:, collectible_type: type) + end + + def row(entry, owned: 0, proxy: 0) + lines = proxy.zero? ? [] : [ Deck::Page::Line.new(id: 1, finish: "nonfoil", quantity: proxy) ] + Deck::Page::Row.new(entry:, owned:, lines:, answer: Catalog::DeckCards::NONE) + end + + def size(keys, min: nil, max: nil) = Catalog::DeckChecks::SizeRule.new(zone_keys: keys, min:, max:) + def limit(count, reason = nil) = ->(_entry) { Catalog::DeckChecks::Limit.new(count:, reason:) } + def copies(keys, limit_for) = Catalog::DeckChecks::CopyRule.new(zone_keys: keys, limit_for:) + + def register(sizes: [], copies: nil) + rules = Catalog::DeckChecks::Rules.new(sizes:, copies:) + stub_const("GameVocabulary", Module.new { define_singleton_method(:deck_rules) { |_key| rules } }) + Catalog.collecting["game"] = "GameVocabulary" + end + + def report(*zones) = described_class.report("game", "tight", zones) + def lines(report) = report.lines.map { [ it.status, it.sentence ] } + + after { Catalog.collecting.delete("game") } + + # … the describe blocks of T1.1, T1.2 and T1.3 go here … +end +``` + +### T1.1 Types, contract, the no-rules path and owned plus proxy + +- [ ] **Test (red).** Inside the `RSpec.describe`: + +```ruby + describe ".report with no rules" do + it "says the format has no rules and still runs owned-plus-proxy", :aggregate_failures do + result = described_class.report("game", "tight", [ zone("main", "Main", row(card("Bolt", 1), owned: 2, proxy: 1)) ]) + expect(lines(result)).to eq([ [ nil, "tight has no rules to check." ], [ "Owned and proxy", "Bolt in Main: 2 owned, 1 proxy." ] ]) + expect([ result.lines.first.fault, result.summary ]).to eq([ false, "1 finding" ]) + end + + it "treats a missing method, a nil answer and an unregistered type alike (AC-5.5)", :aggregate_failures do + stub_const("SilentVocabulary", Module.new) + stub_const("NilVocabulary", Module.new { def self.deck_rules(_key) = nil }) + expected = [ [ nil, "tight has no rules to check." ], [ "Pass", "No card has both owned copies and proxies in one zone." ] ] + %w[SilentVocabulary NilVocabulary].each do |vocabulary| + Catalog.collecting["game"] = vocabulary + expect(lines(described_class.report("game", "tight", []))).to eq(expected) + end + expect(described_class.report("gone", "tight", []).summary).to eq("No findings") + end + end + + describe "owned-plus-proxy results" do + let(:bolt) { card("Lightning Bolt", 1) } + + it "flags a zone that holds both, across printings, naming the split", :aggregate_failures do + result = report(zone("main", "Main", row(bolt, owned: 2), row(card("Lightning Bolt", 1), proxy: 1))) + expect(lines(result)).to include([ "Owned and proxy", "Lightning Bolt in Main: 2 owned, 1 proxy." ]) + expect(result.faults).to eq(1) + end + + it "judges each zone alone, includes zones no rule names, and skips Considering's lone proxy", :aggregate_failures do + zones = [ zone("main", "Main", row(bolt, owned: 1), row(bolt, proxy: 1)), zone("stash", "Stash", row(bolt, owned: 2, proxy: 3)), + zone("considering", "Considering", row(bolt, proxy: 1)) ] + sentences = lines(report(*zones)).map(&:last) + expect(sentences).to include("Lightning Bolt in Main: 1 owned, 1 proxy.", "Lightning Bolt in Stash: 2 owned, 3 proxy.") + expect(sentences).not_to include(a_string_matching(/Considering/)) + end + + it "does not flag owned only, proxy only, owned in one zone with a proxy in another, or another collectible", :aggregate_failures do + stray = card("Other", 9, type: "stranger") + zones = [ zone("main", "Main", row(bolt, owned: 2)), zone("considering", "Considering", row(bolt, proxy: 1), row(stray, owned: 1, proxy: 1)) ] + expect(lines(report(*zones)).last).to eq([ "Pass", "No card has both owned copies and proxies in one zone." ]) + end + + it "delimits the split's numbers as the deck page's heads do", :aggregate_failures do + result = report(zone("main", "Main", row(bolt, owned: 1_234, proxy: 5_678))) + expect(lines(result)).to include([ "Owned and proxy", "Lightning Bolt in Main: 1,234 owned, 5,678 proxy." ]) + end + + it "orders findings by zone, then name, then identity", :aggregate_failures do + zones = [ zone("a", "Alpha", row(card("Zed", 1), owned: 1, proxy: 1), row(card("Abe", 2), owned: 1, proxy: 1)), zone("b", "Beta", row(card("Abe", 2), owned: 1, proxy: 1)) ] + expect(lines(report(*zones)).map(&:last)).to eq([ "Abe in Alpha: 1 owned, 1 proxy.", "Zed in Alpha: 1 owned, 1 proxy.", "Abe in Beta: 1 owned, 1 proxy." ]) + end + end +``` + + Run `bin/rspec spec/models/catalog/deck_checks_spec.rb` → **FAIL** (`uninitialized constant Catalog::DeckChecks`). + +- [ ] **Implement** `app/models/catalog/deck_checks.rb`. The header comment is the extension contract and names no format, zone key or game: + +```ruby +# What a deck page's Check deck answers (spec 079): the deck's size and copy limits judged by the collectible's own rules, and +# every card with owned copies and proxies in one zone, which needs no rule and runs for every deck. Like +# `Catalog::DeckShape`, an optional answer on the collecting vocabulary (`Catalog.collecting_for`), so the core counts and +# words the results and never names a collectible, a format or a zone key but `Catalog::Formats::CONSIDERING`. +# +# An extension contract: a vocabulary that has rules for a format provides +# * `deck_rules(format_key)` returning {Rules}, or nil for a format it has no rules for (the deck's stored key, registered or not). +# * `Rules#sizes`: {SizeRule}s in the order they are shown. A rule holds the `zone_keys` it counts (the extension names them; +# the core matches each to a zone row's key) and one of three shapes: `min` only, `max` only, or `min == max` (an exact +# size); a range with two different bounds is not offered. The extension supplies no words. +# * `Rules#copies`: a {CopyRule} or nil. Its `zone_keys` are counted together; `limit_for` is called with one counted printing +# (a `Catalog::Entry` whose `extension` may be nil) and returns a {Limit} (`count`, and a `reason` sentence part such as +# "restricted to 1", or nil) or nil for a card that is never limited. For a printing with no extension record it returns +# the format's ordinary limit. It reads loaded records only, never a query. +# The core counts copies by catalog identity over every counted printing and holds a card to the lowest number its printings +# answer; nil is above every number, so a card is unlimited only when every counted printing answers nil, and the reason is +# the one given with the lowest number (the first printing in page order on a tie). A card of another collectible than the +# deck's is left out of every check. Considering and any zone no rule names are counted by no size or copy rule. +# A vocabulary without `deck_rules`, a nil answer, a type no longer registered and a deck whose format is no longer registered +# have no size or copy rules. +# @see file:collection-and-lots.md +module Catalog::DeckChecks + # One size rule: the zone keys it counts together and a minimum, a maximum, or both equal (an exact size). + SizeRule = Data.define(:zone_keys, :min, :max) + # A card's limit: the most copies a deck may hold, and an optional sentence part that says why ("restricted to 1"). + Limit = Data.define(:count, :reason) + # The copy rule: the zone keys counted together and a callable from a loaded printing to a {Limit} or nil. + CopyRule = Data.define(:zone_keys, :limit_for) + # What a vocabulary answers for a format: its size rules and its copy rule (nil for none). + Rules = Data.define(:sizes, :copies) + # One line of a report: a status word (nil for a line that is no check), its sentence, and whether it is a finding. + Line = Data.define(:status, :sentence, :fault) + # The results of one check request, in the order they are shown. + Report = Data.define(:lines) do + # The number of findings. + def faults = lines.count(&:fault) + + # The summary line: "3 findings", "1 finding" or "No findings". + def summary = faults.zero? ? "No findings" : "#{faults} #{faults == 1 ? 'finding' : 'findings'}" + end + # A row of the deck's collectible with its zone and the zone's position in page order. + # @api private + Counted = Data.define(:zone, :position, :row) + + # The report for a deck. + # + # @param deck_type [String] the deck's collectible type + # @param format_key [String] the deck's stored format key, registered or not + # @param zones [Array] `Deck::Page#zones`; read from memory, no statement runs + # @return [Report] never nil, and never raises for an unregistered type or format + def self.report(deck_type, format_key, zones) + counted = counted_rows(deck_type, zones) + rules = rules_for(deck_type, format_key) + lines = rules ? rule_lines(rules, deck_type, format_key, zones, counted) : [ no_rules_line(deck_type, format_key) ] + Report.new(lines: lines + split_lines(counted)) + end + + # @api private + def self.rules_for(deck_type, format_key) + return unless Catalog.collecting.key?(deck_type) + + vocabulary = Catalog.collecting_for(deck_type) + vocabulary.deck_rules(format_key) if vocabulary.respond_to?(:deck_rules) + end + private_class_method :rules_for + + # Every row of the deck's collectible with its zone's position; another collectible's rows are left out (decision 4). + # @api private + def self.counted_rows(deck_type, zones) + zones.each_with_index.flat_map do |zone, position| + zone.groups.flat_map(&:rows).select { it.entry.collectible_type == deck_type }.map { Counted.new(zone:, position:, row: it) } + end + end + private_class_method :counted_rows + + # @api private + def self.rule_lines(_rules, _deck_type, _format_key, _zones, _counted) = [] + private_class_method :rule_lines + + # @api private + def self.no_rules_line(deck_type, format_key) + Line.new(status: nil, sentence: "#{Catalog::Formats.name_for(deck_type, format_key)} has no rules to check.", fault: false) + end + private_class_method :no_rules_line + + # A count with delimiters ("1,234"), as the deck page's heads and spec 069's panel write one. + # @api private + def self.delimited(number) = ActiveSupport::NumberHelper.number_to_delimited(number) + private_class_method :delimited + + # The owned-plus-proxy check (decision 5): each zone alone, any printing of the card; findings ordered by the zone's + # position, then the card's name, then its identity; or one pass line. + # @api private + def self.split_lines(counted) + findings = counted.group_by { [ it.position, it.row.entry.catalog_identity_id ] }.filter_map do |(position, identity_id), cards| + owned = cards.sum { it.row.owned } + proxy = cards.sum { it.row.proxy } + next unless owned.positive? && proxy.positive? + + [ position, cards.first.row.entry.name, identity_id, cards.first.zone.name, owned, proxy ] + end + return [ Line.new(status: "Pass", sentence: "No card has both owned copies and proxies in one zone.", fault: false) ] if findings.empty? + + findings.sort_by { it.first(3) }.map do |_position, name, _identity_id, zone_name, owned, proxy| + Line.new(status: "Owned and proxy", sentence: "#{name} in #{zone_name}: #{delimited(owned)} owned, #{delimited(proxy)} proxy.", fault: true) + end + end + private_class_method :split_lines +end +``` + +- [ ] **Guard.** In `spec/deck_core_spec.rb` add `app/models/catalog/deck_checks.rb` to the `patterns` list beside the other `catalog/*.rb` files and append `+ 1` to the total, with the comment clause `spec 079 adds the seam app/models/catalog/deck_checks.rb (a new pattern)`. +- [ ] Run `bin/rspec spec/models/catalog/deck_checks_spec.rb spec/deck_core_spec.rb` → PASS. `bin/rubocop -a` on both; `bin/yard doc --no-cache` has no warning. Commit `feat(decks): add Catalog::DeckChecks and its owned-plus-proxy check (#258)`. + +### T1.2 Size results + +- [ ] **Test (red).** Add inside the `RSpec.describe`: + +```ruby + describe "size results" do + it "judges a minimum with the true count in every sentence", :aggregate_failures do + register(sizes: [ size(%w[main], min: 60) ]) + bolt = card("Bolt", 1) + expect(lines(report(zone("main", "Main", row(bolt, owned: 58))))).to include([ "Short", "Main: 58 cards; tight needs at least 60." ]) + expect(lines(report(zone("main", "Main", row(bolt, owned: 59, proxy: 1))))).to include([ "Pass", "Main: 60 cards; tight needs at least 60." ]) + expect(lines(report(zone("main", "Main", row(bolt, owned: 61))))).to include([ "Pass", "Main: 61 cards; tight needs at least 60." ]) + end + + it "names a rule's zones in its key order and a zone the deck lacks by its key", :aggregate_failures do + register(sizes: [ size(%w[bench], max: 15), size(%w[lead main], min: 100, max: 100) ]) + zones = [ zone("main", "Main", row(card("A", 1), owned: 98)), zone("lead", "Lead", row(card("B", 2), owned: 1)) ] + expect(lines(report(*zones))).to start_with([ "Pass", "bench: 0 cards; tight allows at most 15." ], [ "Short", "Lead + Main: 99 cards; tight needs exactly 100." ]) + end + + it "says Over past a maximum and past an exact size, and the singular for one card", :aggregate_failures do + register(sizes: [ size(%w[bench], max: 15), size(%w[main], min: 1, max: 1) ]) + zones = [ zone("bench", "Bench", row(card("A", 1), owned: 16)), zone("main", "Main", row(card("B", 2), owned: 2)) ] + expect(lines(report(*zones))).to start_with([ "Over", "Bench: 16 cards; tight allows at most 15." ], [ "Over", "Main: 2 cards; tight needs exactly 1." ]) + expect(lines(report(zone("main", "Main", row(card("B", 2), owned: 1))))).to include([ "Pass", "Main: 1 card; tight needs exactly 1." ]) + end + + it "counts nothing in Considering or an unnamed zone, and passes at exactly the exact size", :aggregate_failures do + register(sizes: [ size(%w[main], min: 100, max: 100) ]) + zones = [ zone("main", "Main", row(card("A", 1), owned: 100)), zone("considering", "Considering", row(card("B", 2), owned: 30)) ] + expect(lines(report(*zones)).first).to eq([ "Pass", "Main: 100 cards; tight needs exactly 100." ]) + end + + it "separates thousands in a count" do + register(sizes: [ size(%w[main], min: 60) ]) + expect(lines(report(zone("main", "Main", row(card("A", 1), owned: 1_234)))).first.last).to eq("Main: 1,234 cards; tight needs at least 60.") + end + + it "leaves out another collectible's cards and zones no rule names", :aggregate_failures do + register(sizes: [ size(%w[main], min: 1) ]) + zones = [ zone("main", "Main", row(card("Other", 9, type: "stranger"), owned: 5)), zone("stash", "Stash", row(card("A", 1), owned: 7)) ] + expect(lines(report(*zones)).first).to eq([ "Short", "Main: 0 cards; tight needs at least 1." ]) + end + end +``` + + Run → **FAIL** (the rule lines are empty). + +- [ ] **Implement.** Replace the `rule_lines` stub and add the size helpers: + +```ruby + # @api private + def self.rule_lines(rules, deck_type, format_key, zones, counted) + rules.sizes.map { size_line(it, deck_type, format_key, zones, counted) } + end + private_class_method :rule_lines + + # One size result (decision 6). Zones are named in the rule's key order: by the zone row's shown name, else by the format's + # registration, else by the key. + # @api private + def self.size_line(rule, deck_type, format_key, zones, counted) + total = counted.select { rule.zone_keys.include?(it.zone.location.zone) }.sum { it.row.total } + subject = rule.zone_keys.map { zone_name(it, deck_type, format_key, zones) }.join(" + ") + status = if rule.min && total < rule.min then "Short" elsif rule.max && total > rule.max then "Over" else "Pass" end + format_name = Catalog::Formats.name_for(deck_type, format_key) + Line.new(status:, sentence: "#{subject}: #{counted_words(total, 'card')}; #{format_name} #{size_need(rule)}.", fault: status != "Pass") + end + private_class_method :size_line + + # The rule's requirement in words: "needs at least 60", "allows at most 15" or "needs exactly 100". + # @api private + def self.size_need(rule) + return "needs exactly #{delimited(rule.min)}" if rule.min && rule.min == rule.max + return "needs at least #{delimited(rule.min)}" if rule.min + + "allows at most #{delimited(rule.max)}" + end + private_class_method :size_need + + # @api private + def self.zone_name(key, deck_type, format_key, zones) + zones.find { it.location.zone == key }&.name || Catalog::Formats.zones_for(deck_type, format_key)&.find { it.key == key }&.name || key + end + private_class_method :zone_name + + # "1 card", "5 copies": the count with delimiters and the singular for one. + # @api private + def self.counted_words(number, word) = "#{delimited(number)} #{word.pluralize(number)}" + private_class_method :counted_words +``` + +- [ ] Run → PASS; rubocop; commit `feat(decks): size results for the deck checks (#258)`. + +### T1.3 Copy limits + +- [ ] **Test (red).** Add inside the `RSpec.describe`: + +```ruby + describe "copy results" do + let(:bolt) { card("Lightning Bolt", 1) } + + it "names a card over its limit across zones, printings and owned plus proxy", :aggregate_failures do + register(copies: copies(%w[main bench], limit(4))) + zones = [ zone("main", "Main", row(bolt, owned: 3), row(card("Lightning Bolt", 1), proxy: 1)), zone("bench", "Bench", row(bolt, owned: 1)) ] + expect(lines(report(*zones))).to include([ "Over the limit", "Lightning Bolt: 5 copies; the limit is 4." ]) + end + + it "passes at the limit and counts no copy in an unnamed zone", :aggregate_failures do + register(copies: copies(%w[main], limit(4))) + zones = [ zone("main", "Main", row(bolt, owned: 4)), zone("considering", "Considering", row(bolt, owned: 9)) ] + expect(lines(report(*zones))).to include([ "Pass", "No card is over its limit." ]) + end + + it "passes at the limit across two named zones (4 of 3 + 1)" do + register(copies: copies(%w[main bench], limit(4))) + zones = [ zone("main", "Main", row(bolt, owned: 3)), zone("bench", "Bench", row(bolt, owned: 1)) ] + expect(lines(report(*zones))).to include([ "Pass", "No card is over its limit." ]) + end + + it "breaks a tie on the lowest limit by the first printing in page order", :aggregate_failures do + first = card("Black Lotus", 2) + second = card("Black Lotus", 2) + answer = ->(entry) { Catalog::DeckChecks::Limit.new(count: 1, reason: entry.equal?(first) ? "restricted to 1" : "banned past 1") } + register(copies: copies(%w[main], answer)) + expect(lines(report(zone("main", "Main", row(second, owned: 1), row(first, owned: 1))))).to include([ "Over the limit", "Black Lotus: 2 copies; banned past 1." ]) + end + + it "orders two findings of one zone by name, then by identity when the names match", :aggregate_failures do + register(copies: copies(%w[main], limit(0))) + zones = [ zone("main", "Main", row(card("Zed", 1), owned: 1), row(card("Abe", 5), owned: 2), row(card("Abe", 2), owned: 1)) ] + expect(lines(report(*zones)).first(3).map(&:last)).to eq([ "Abe: 1 copy; the limit is 0.", "Abe: 2 copies; the limit is 0.", "Zed: 1 copy; the limit is 0." ]) + end + + it "holds a card to the lowest limit its printings answer, with that reason", :aggregate_failures do + one = card("Black Lotus", 2) + two = card("Black Lotus", 2) + answer = ->(entry) { entry.equal?(one) ? Catalog::DeckChecks::Limit.new(count: 4, reason: nil) : Catalog::DeckChecks::Limit.new(count: 1, reason: "restricted to 1") } + register(copies: copies(%w[main], answer)) + expect(lines(report(zone("main", "Main", row(one, owned: 1), row(two, owned: 1))))).to include([ "Over the limit", "Black Lotus: 2 copies; restricted to 1." ]) + end + + it "treats nil as above every number: unlimited only when every printing says nil", :aggregate_failures do + free = card("Plains", 3) + mixed = card("Plains", 3) + register(copies: copies(%w[main], ->(entry) { Catalog::DeckChecks::Limit.new(count: 4, reason: nil) unless entry.equal?(free) })) + expect(lines(report(zone("main", "Main", row(free, owned: 20))))).to include([ "Pass", "No card is over its limit." ]) + expect(lines(report(zone("main", "Main", row(free, owned: 3), row(mixed, owned: 2))))).to include([ "Over the limit", "Plains: 5 copies; the limit is 4." ]) + end + + it "orders findings by first zone, then name, then identity; leaves out another collectible", :aggregate_failures do + register(copies: copies(%w[main bench], limit(1))) + zones = [ zone("main", "Main", row(card("Zed", 1), owned: 2)), zone("bench", "Bench", row(card("Abe", 2), owned: 2), row(card("Other", 3, type: "stranger"), owned: 9)) ] + expect(lines(report(*zones)).map(&:last)).to start_with("Zed: 2 copies; the limit is 1.", "Abe: 2 copies; the limit is 1.") + expect(lines(report(*zones)).map(&:last)).not_to include(a_string_matching(/Other/)) + end + + it "says the singular for one copy" do + register(copies: copies(%w[main], limit(0))) + expect(lines(report(zone("main", "Main", row(bolt, owned: 1))))).to include([ "Over the limit", "Lightning Bolt: 1 copy; the limit is 0." ]) + end + end +``` + + Run → **FAIL**. + +- [ ] **Implement.** Make `rule_lines` return the sizes plus the copy lines, and add `copy_lines`: + +```ruby + # @api private + def self.rule_lines(rules, deck_type, format_key, zones, counted) + sizes = rules.sizes.map { size_line(it, deck_type, format_key, zones, counted) } + sizes + (rules.copies ? copy_lines(rules.copies, counted) : []) + end + private_class_method :rule_lines + + # The copy check (decisions 1, 3, 4): copies by catalog identity over the rule's zones, owned plus proxy; one finding per + # card over its limit, ordered by the first zone that holds it, then its name, then its identity; or one pass line. A card + # is held to the lowest number its printings answer; `min_by` keeps the first printing in page order on a tie. + # @api private + def self.copy_lines(rule, counted) + in_rule = counted.select { rule.zone_keys.include?(it.zone.location.zone) } + findings = in_rule.group_by { it.row.entry.catalog_identity_id }.filter_map do |identity_id, cards| + copies = cards.sum { it.row.total } + limit = cards.map { it.row.entry }.uniq.filter_map { rule.limit_for.call(it) }.min_by(&:count) + next if limit.nil? || copies <= limit.count + + [ cards.map(&:position).min, cards.first.row.entry.name, identity_id, copies, limit ] + end + return [ Line.new(status: "Pass", sentence: "No card is over its limit.", fault: false) ] if findings.empty? + + findings.sort_by { it.first(3) }.map do |_position, name, _identity_id, copies, limit| + Line.new(status: "Over the limit", sentence: "#{name}: #{counted_words(copies, 'copy')}; #{limit.reason || "the limit is #{limit.count}"}.", fault: true) + end + end + private_class_method :copy_lines +``` + +- [ ] Run the whole file and `spec/deck_core_spec.rb` → PASS; rubocop; `bin/yard doc --no-cache`; commit `feat(decks): copy-limit results for the deck checks (#258)`. + +## Phase 2: Magic's rules + +**Implements:** decisions 1 and 3 (Magic side), FR-2 · **Satisfies:** AC-2.1–2.4, 2.7, 2.8, 3.1–3.7, 3.12, 3.14, 3.16, 4.5 +**Files:** `app/models/mtg/deck_rules.rb`, `app/models/mtg/collecting.rb`, `spec/models/mtg/deck_rules_spec.rb`, `spec/models/mtg/deck_checks_for_magic_spec.rb` +**Interfaces:** Produces `MTG::DeckRules.for(format_key) → Rules | nil`, `MTG::DeckRules.limit_for(entry, format_key, ordinary)`, `MTG::Collecting.deck_rules(format_key)`. Consumes the Phase 1 types and the loaded `MTG::Printing` (`faces[].type_line`, `faces[].oracle_text`, `legalities`). + +### T2.1 `MTG::DeckRules` and the vocabulary answer + +- [ ] **Test (red)** `spec/models/mtg/deck_rules_spec.rb`: + +```ruby +require "rails_helper" + +RSpec.describe MTG::DeckRules, type: :model do + def entry(name = "Card", type_line: "Instant", text: nil, legalities: {}, record: true) + entry = create(:catalog_entry, name:) + create(:mtg_printing, entry:, legalities:, faces: [ { "name" => name, "type_line" => type_line, "oracle_text" => text, "image_uris" => {} } ]) if record + Catalog::Entry.preload_extensions([ entry ]).first + end + + def limit(format, entry) = described_class.for(format).copies.limit_for.call(entry)&.then { [ it.count, it.reason ] } + def shape(rule) = [ rule.zone_keys, rule.min, rule.max ] + + describe ".for" do + it "has size and copy rules for each constructed format", :aggregate_failures do + %w[standard pioneer modern legacy vintage pauper].each do |key| + rules = described_class.for(key) + expect(rules.sizes.map { shape(it) }).to eq([ [ %w[main], 60, nil ], [ %w[sideboard], nil, 15 ] ]) + expect(rules.copies.zone_keys).to eq(%w[main sideboard]) + end + end + + it "has Commander's exact size, a singleton copy rule, and none for casual or an unknown key", :aggregate_failures do + commander = described_class.for("commander") + expect(commander.sizes.map { shape(it) }).to eq([ [ %w[commander main], 100, 100 ] ]) + expect(commander.copies.zone_keys).to eq(%w[commander main]) + expect([ described_class.for("casual"), described_class.for("retired"), MTG::Collecting.deck_rules("casual") ]).to all(be_nil) + end + end + + describe "a card's limit" do + it "is 4 in constructed formats and 1 in Commander", :aggregate_failures do + bolt = entry("Lightning Bolt") + expect([ limit("standard", bolt), limit("pauper", bolt), limit("vintage", bolt), limit("commander", bolt) ]).to eq([ [ 4, nil ], [ 4, nil ], [ 4, nil ], [ 1, nil ] ]) + end + + it "is none for a basic land, Snow-Covered ones included", :aggregate_failures do + plains = entry("Plains", type_line: "Basic Land — Plains") + snow = entry("Snow-Covered Island", type_line: "Basic Snow Land — Island") + expect([ limit("standard", plains), limit("commander", snow) ]).to all(be_nil) + end + + it "is none for any-number text, in every format", :aggregate_failures do + rats = entry("Relentless Rats", type_line: "Creature — Rat", text: "Relentless Rats gets +1/+1 for each other creature you control named Relentless Rats.\nA deck can have any number of cards named Relentless Rats.") + expect([ limit("modern", rats), limit("commander", rats) ]).to all(be_nil) + end + + it "is the stated number for up-to text, in every format", :aggregate_failures do + dwarves = entry("Seven Dwarves", type_line: "Creature — Dwarf", text: "Seven Dwarves gets +1/+1 for each other creature named Seven Dwarves you control.\nA deck can have up to seven cards named Seven Dwarves.") + expect([ limit("standard", dwarves), limit("commander", dwarves) ]).to eq([ [ 7, nil ], [ 7, nil ] ]) + end + + it "is 1, restricted, in Vintage only", :aggregate_failures do + lotus = entry("Black Lotus", type_line: "Artifact", legalities: { "vintage" => "restricted", "legacy" => "banned" }) + expect([ limit("vintage", lotus), limit("legacy", lotus) ]).to eq([ [ 1, "restricted to 1" ], [ 4, nil ] ]) + end + + it "is the ordinary limit for a printing with no card data, a basic land included", :aggregate_failures do + bare = entry("Plains", record: false) + expect([ limit("standard", bare), limit("commander", bare) ]).to eq([ [ 4, nil ], [ 1, nil ] ]) + end + end +end +``` + + Run `bin/rspec spec/models/mtg/deck_rules_spec.rb` → **FAIL** (`uninitialized constant MTG::DeckRules`). + +- [ ] **Implement** `app/models/mtg/deck_rules.rb` (Magic's side may name formats and zones; it is not a core file): + +```ruby +# Magic's deck rules for the deck checks (spec 079 decision 3, the extension side of `Catalog::DeckChecks`): size and copy +# limits per format, and each printing's limit read from its first face's type line, its rules text and its legalities. +# Reads the loaded printing record only, so no query runs per card. +class MTG::DeckRules + # The formats that share one rule set: the main zone's minimum, the sideboard's maximum and a limit of four. + CONSTRUCTED = %w[standard pioneer modern legacy vintage pauper].freeze + # A constructed deck holds at least this many cards in its main zone (spec 079 decision 3). + MAIN_MINIMUM = 60 + # A constructed deck's sideboard holds at most this many cards. + SIDEBOARD_MAXIMUM = 15 + # The copies of one card a constructed deck may hold (a basic land and the cards that say otherwise excepted). + CONSTRUCTED_COPIES = 4 + # The copies of one card a Commander deck may hold: the singleton rule. + COMMANDER_COPIES = 1 + # A Commander deck's commander and main zones hold exactly this many cards together. + COMMANDER_SIZE = 100 + # Rules text of a card a deck may hold any number of: Scryfall's own words (confirmed in verification.md, T0.2). + ANY_NUMBER = /a deck can have any number of cards named/i + # Rules text of a card a deck may hold up to a stated number of; the number is the word after "up to". + STATED_NUMBER = /a deck can have up to (\w+) cards named/i + # The number words the stated limits use (Seven Dwarves: seven; Nazgûl: nine). + NUMBER_WORDS = %w[two three four five six seven eight nine ten].each_with_index.to_h { |word, index| [ word, index + 2 ] }.freeze + + # The rules of one format. + # + # @param format_key [String] the deck's stored format key + # @return [Catalog::DeckChecks::Rules, nil] nil for casual and for a key the collectible has no rules for + def self.for(format_key) + return commander if format_key == "commander" + + constructed(format_key) if CONSTRUCTED.include?(format_key) + end + + # A printing's limit in a format. Order matters: a basic land and any-number text are exempt first, then a stated number, + # then Vintage's restriction, then the ordinary limit. A printing with no card data is never exempt and never restricted. + # + # @param entry [Catalog::Entry] a printing, its `extension` loaded or nil + # @param format_key [String] + # @param ordinary [Integer] the format's ordinary limit + # @return [Catalog::DeckChecks::Limit, nil] nil for a card that is never limited + def self.limit_for(entry, format_key, ordinary) + printing = entry.extension + return Catalog::DeckChecks::Limit.new(count: ordinary, reason: nil) if printing.nil? + + faces = Array(printing.faces) + return if basic_land?(faces.first) + + text = faces.filter_map { it["oracle_text"] }.join("\n") + return if text.match?(ANY_NUMBER) + + stated = NUMBER_WORDS[text[STATED_NUMBER, 1].to_s.downcase] + return Catalog::DeckChecks::Limit.new(count: stated, reason: nil) if stated + return Catalog::DeckChecks::Limit.new(count: 1, reason: "restricted to 1") if format_key == "vintage" && printing.legalities.to_h["vintage"] == "restricted" + + Catalog::DeckChecks::Limit.new(count: ordinary, reason: nil) + end + + # @api private + def self.basic_land?(face) = face&.dig("type_line").to_s.split("—").first.to_s.split.include?("Basic") + private_class_method :basic_land? + + # @api private + def self.constructed(format_key) + Catalog::DeckChecks::Rules.new( + sizes: [ Catalog::DeckChecks::SizeRule.new(zone_keys: %w[main], min: MAIN_MINIMUM, max: nil), + Catalog::DeckChecks::SizeRule.new(zone_keys: %w[sideboard], min: nil, max: SIDEBOARD_MAXIMUM) ], + copies: Catalog::DeckChecks::CopyRule.new(zone_keys: %w[main sideboard], limit_for: ->(entry) { limit_for(entry, format_key, CONSTRUCTED_COPIES) }) + ) + end + private_class_method :constructed + + # @api private + def self.commander + Catalog::DeckChecks::Rules.new( + sizes: [ Catalog::DeckChecks::SizeRule.new(zone_keys: %w[commander main], min: COMMANDER_SIZE, max: COMMANDER_SIZE) ], + copies: Catalog::DeckChecks::CopyRule.new(zone_keys: %w[commander main], limit_for: ->(entry) { limit_for(entry, "commander", COMMANDER_COPIES) }) + ) + end + private_class_method :commander +end +``` + + In `app/models/mtg/collecting.rb`, after `deck_shape`: + +```ruby + # The deck checks' rules for one format (spec 079, the extension side of `Catalog::DeckChecks`): size and copy limits. + # See `MTG::DeckRules`. + # + # @param format_key [String] the deck's stored format key + # @return [Catalog::DeckChecks::Rules, nil] nil for casual and for a key with no rules + def self.deck_rules(format_key) = MTG::DeckRules.for(format_key.to_s) +``` + +- [ ] Run → PASS; `bin/rubocop -a`; `bin/yard doc --no-cache`; commit `feat(decks): Magic's deck rules for the deck checks (#258)`. + +### T2.2 Magic decks end to end through the seam + +- [ ] **Test** `spec/models/mtg/deck_checks_for_magic_spec.rb` (real decks, real `Deck::Page`; AC-2.1–2.4, 2.7, 2.8, 3.1–3.7, 3.12, 3.14, 3.16, 4.5): + +```ruby +require "rails_helper" + +RSpec.describe "Catalog::DeckChecks for a Magic deck", type: :model do + let(:account) { create(:user).account } + + def deck_of(format) = create(:deck, account:, format:, name: "Deck #{format}") + + def card(name, type_line: "Instant", text: nil, legalities: {}, identity: create(:catalog_identity, name:), **attributes) + entry = create(:catalog_entry, identity:, name:, **attributes) + create(:mtg_printing, entry:, legalities:, faces: [ { "name" => name, "type_line" => type_line, "oracle_text" => text, "image_uris" => {} } ]) + entry + end + + def land(name = "Plains") = card(name, type_line: "Basic Land — #{name}") + def own(deck, zone, entry, quantity) = create_lot_in_zone(deck, zone, entry:, quantity:) + def proxy(deck, zone, entry) = create(:deck_proxy, zone: deck.zone(zone), account:, entry:, finish: "nonfoil", quantity: 1) + + def lines(deck) + page = Deck::Page.new(Deck.includes(:location).find_by!(location_id: deck.location_id)) + Catalog::DeckChecks.report("mtg", deck.format, page.zones).lines.map { [ it.status, it.sentence ] } + end + + it "judges Main and Sideboard in each constructed format's own name", :aggregate_failures do + %w[standard pioneer modern legacy vintage pauper].each do |key| + deck = deck_of(key) + own(deck, "main", land, 58) + own(deck, "sideboard", land("Island"), 16) + expect(lines(deck)).to include([ "Short", "Main: 58 cards; #{key.titleize} needs at least 60." ], [ "Over", "Sideboard: 16 cards; #{key.titleize} allows at most 15." ]) + end + end + + it "counts a proxy toward Main and passes at 60 and at 15", :aggregate_failures do + deck = deck_of("standard") + own(deck, "main", land, 59) + proxy(deck, "main", land("Island")) + own(deck, "sideboard", land("Swamp"), 15) + expect(lines(deck)).to include([ "Pass", "Main: 60 cards; Standard needs at least 60." ], [ "Pass", "Sideboard: 15 cards; Standard allows at most 15." ]) + end + + it "judges Commander's two zones together", :aggregate_failures do + deck = deck_of("commander") + own(deck, "commander", card("Krenko"), 1) + own(deck, "main", land, 98) + expect(lines(deck)).to include([ "Short", "Commander + Main: 99 cards; Commander needs exactly 100." ]) + own(deck, "main", land("Island"), 2) + expect(lines(deck)).to include([ "Over", "Commander + Main: 101 cards; Commander needs exactly 100." ]) + end + + it "says Casual and an unregistered format have no rules but still flag a forgotten proxy", :aggregate_failures do + deck = deck_of("casual") + bolt = card("Lightning Bolt") + own(deck, "main", bolt, 2) + proxy(deck, "main", bolt) + expect(lines(deck)).to eq([ [ nil, "Casual has no rules to check." ], [ "Owned and proxy", "Lightning Bolt in Main: 2 owned, 1 proxy." ] ]) + deck.update!(format: "retired") + expect(lines(deck).first).to eq([ nil, "retired has no rules to check." ]) + end + + it "adds a card's copies across printings, zones, owned and proxy", :aggregate_failures do + deck = deck_of("standard") + bolt = create(:catalog_identity, name: "Lightning Bolt") + one = card("Lightning Bolt", identity: bolt) + two = card("Lightning Bolt", identity: bolt, set: create(:catalog_set)) + own(deck, "main", one, 3) + own(deck, "sideboard", two, 1) + proxy(deck, "main", two) + expect(lines(deck)).to include([ "Over the limit", "Lightning Bolt: 5 copies; the limit is 4." ]) + end + + it "names a card by its English name when its first row in page order is a Japanese printing", :aggregate_failures do + deck = deck_of("standard") # Main is read before Sideboard, and the Japanese printing is the only row of Main + bolt = create(:catalog_identity, name: "Lightning Bolt") + own(deck, "main", card("Lightning Bolt", identity: bolt, language: "ja", localized_name: "稲妻"), 3) + own(deck, "sideboard", card("Lightning Bolt", identity: bolt, set: create(:catalog_set)), 2) + expect(lines(deck)).to include([ "Over the limit", "Lightning Bolt: 5 copies; the limit is 4." ]) + expect(lines(deck).map(&:last)).not_to include(a_string_matching(/稲妻/)) + end + + it "names a Sideboard the deck has no zone row for from the registration (AC-2.12)" do + deck = deck_of("standard") + deck.zone("sideboard").destroy! + expect(lines(deck)).to include([ "Pass", "Sideboard: 0 cards; Standard allows at most 15." ]) + end + + it "passes Commander at exactly 100 while cards in Considering count nowhere (AC-2.4, 2.5)" do + commander = deck_of("commander") + own(commander, "commander", card("Krenko"), 1) + own(commander, "main", land, 99) + own(commander, "considering", land("Island"), 30) + expect(lines(commander)).to include([ "Pass", "Commander + Main: 100 cards; Commander needs exactly 100." ]) + end + + it "exempts basic lands and any-number cards, and holds Commander to one of the rest", :aggregate_failures do + standard = deck_of("standard") + own(standard, "main", land, 20) + rats = card("Relentless Rats", type_line: "Creature — Rat", text: "A deck can have any number of cards named Relentless Rats.") + own(standard, "main", rats, 9) + expect(lines(standard)).to include([ "Pass", "No card is over its limit." ]) + commander = deck_of("commander") + krenko = create(:catalog_identity, name: "Krenko") + own(commander, "commander", card("Krenko", identity: krenko), 1) + own(commander, "main", card("Krenko", identity: krenko, set: create(:catalog_set)), 1) + own(commander, "main", card("Snow-Covered Island", type_line: "Basic Snow Land — Island"), 2) + expect(lines(commander)).to include([ "Over the limit", "Krenko: 2 copies; the limit is 1." ]) + expect(lines(commander).map(&:last)).not_to include(a_string_matching(/Snow-Covered/)) + end + + it "holds a numbered card to its number in every format, and passes at that number", :aggregate_failures do + text = "A deck can have up to seven cards named Seven Dwarves." + %w[standard commander].each do |key| + over = deck_of(key) + own(over, "main", card("Seven Dwarves", type_line: "Creature — Dwarf", text:), 8) + expect(lines(over)).to include([ "Over the limit", "Seven Dwarves: 8 copies; the limit is 7." ]) + exact = create(:deck, account:, format: key, name: "Exact #{key}") + own(exact, "main", card("Seven Dwarves", type_line: "Creature — Dwarf", text:), 7) + expect(lines(exact)).to include([ "Pass", "No card is over its limit." ]) + end + end + + it "restricts a card to one in Vintage only", :aggregate_failures do + legalities = { "vintage" => "restricted" } + vintage = deck_of("vintage") + own(vintage, "main", card("Black Lotus", type_line: "Artifact", legalities:), 2) + expect(lines(vintage)).to include([ "Over the limit", "Black Lotus: 2 copies; restricted to 1." ]) + legacy = deck_of("legacy") + own(legacy, "main", card("Black Lotus", type_line: "Artifact", legalities:), 2) + expect(lines(legacy)).to include([ "Pass", "No card is over its limit." ]) + end + + it "counts a basic land's two printings, one with no card data, at the ordinary limit (AC-3.14, 3.16)", :aggregate_failures do + deck = deck_of("standard") + identity = create(:catalog_identity, name: "Plains") + own(deck, "main", card("Plains", type_line: "Basic Land — Plains", identity:), 1) + own(deck, "main", create(:catalog_entry, identity:, name: "Plains", set: create(:catalog_set)), 4) + expect(lines(deck)).to include([ "Over the limit", "Plains: 5 copies; the limit is 4." ]) + end + + it "leaves out another collectible's cards", :aggregate_failures do + deck = deck_of("standard") + own(deck, "main", create(:catalog_entry, collectible_type: "stranger", name: "Odd"), 70) + expect(lines(deck)).to include([ "Short", "Main: 0 cards; Standard needs at least 60." ], [ "Pass", "No card is over its limit." ]) + end +end +``` + +- [ ] Run → these should pass on first run, since T2.1 is in; this task is the end-to-end proof, not new behaviour. **Mutation proof:** set `MAIN_MINIMUM` to 61 once, see the Standard examples fail, restore. Record it in `verification.md`. Commit `test(decks): Magic deck checks end to end through the seam (#258)`. + +## Phase 3: The request and the page + +**Implements:** FR-3; decisions 7, 8, 9 · **Satisfies:** AC-1.1–1.9, 5.1, 5.2, 5.3 +**Files:** `config/routes.rb`, `app/controllers/decks/checks_controller.rb`, `app/views/decks/show.html.erb`, `app/views/decks/_check.html.erb`, `spec/requests/deck_checks_spec.rb`, `spec/deck_core_spec.rb`, `spec/system/deck_page_spec.rb` +**Interfaces:** Produces `GET /decks/:deck_id/check` (`deck_check_path(deck)`, `Decks::ChecksController#show`) and `@check` (a `Catalog::DeckChecks::Report`) in `decks/show`. Consumes Phases 1 and 2, `FindsDeck#set_deck`, `Deck::Page`, `Catalog::DeckShape.answer`. + +### T3.1 Route, controller, link, results section (plain markup) + +- [ ] **Test (red)** `spec/requests/deck_checks_spec.rb`: + +```ruby +require "rails_helper" + +RSpec.describe "Deck checks", type: :request do + let(:user) { create(:user) } + let(:account) { user.account } + let(:deck) { create(:deck, account:, name: "Izzet Tempo", format: "standard") } + + def html = Nokogiri::HTML5(response.body) + + def printing(name: "Lightning Bolt", type_line: "Instant") + entry = create(:catalog_entry, identity: create(:catalog_identity, name:), name:) + create(:mtg_printing, entry:, faces: [ { "name" => name, "type_line" => type_line, "mana_cost" => "{R}", "image_uris" => {} } ]) + entry + end + + def plains = printing(name: "Plains", type_line: "Basic Land — Plains") + def own(zone, entry, quantity: 1) = create_lot_in_zone(deck, zone, entry:, quantity:) + def proxy_in(zone, entry) = create(:deck_proxy, zone: deck.zone(zone), account:, entry:, finish: "nonfoil", quantity: 1) + def rows = html.css(".c-check__row").map { it.text.squish } + + context "when signed in" do + before { sign_in_as(user) } + + it "shows the button and no results on the plain deck page (AC-1.1, 1.9)", :aggregate_failures do + get deck_path(deck) + expect(html.at_css(".c-item__actions > a.c-btn--secondary[href='#{deck_check_path(deck)}']").text).to eq("Check deck") + expect(html.at_css("#deck-check-title")).to be_nil + expect(html.at_css("main form")).to be_nil + expect(response.body).not_to include('name="turbo-cache-control"') + end + + it "shows the summary and the results between the head and the zones (AC-1.2, 1.7, 1.8, 1.9)", :aggregate_failures do + own("main", plains, quantity: 58) + get deck_check_path(deck) + expect(response).to have_http_status(:ok) + expect([ html.at_css("#deck-check-title").text, html.at_css(".c-check__summary").text ]).to eq([ "Deck check", "1 finding" ]) + expect(rows).to eq([ "Short Main: 58 cards; Standard needs at least 60.", "Pass Sideboard: 0 cards; Standard allows at most 15.", + "Pass No card is over its limit.", "Pass No card has both owned copies and proxies in one zone." ]) + expect(html.css("main > *").first(3).map { it["class"].to_s.split.first }).to eq(%w[c-item__head c-check c-deck]) + expect(response.body).to include('') + expect(html.at_css("main form")).to be_nil + end + + it "shows the same results again and every link of the plain page (AC-1.4, 1.5)", :aggregate_failures do + own("main", plains, quantity: 58) + get deck_path(deck) + plain = html.css("main a[href]").map { it["href"] } + get deck_check_path(deck) + first = rows + get deck_check_path(deck) + expect(rows).to eq(first) + expect(html.css("main a[href]").map { it["href"] }).to include(*plain) + expect(html.css("main a, main button").map { it["disabled"] || it["aria-disabled"] }.compact).to be_empty + get edit_deck_path(deck) # following one of them works as from the plain page + expect(response).to have_http_status(:ok) + end + + it "changes nothing: no write reaches lots, proxies or locations (AC-4.8)", :aggregate_failures do + entry = printing + own("main", entry) + proxy_in("main", entry) + before = [ Lot.sum(:quantity), Deck::Proxy.sum(:quantity), Location.count ] + writes = [ Lot, Deck::Proxy, Location ].flat_map { |model| write_statements_to(model.table_name) { get deck_check_path(deck) } } + expect(writes).to be_empty + expect([ Lot.sum(:quantity), Deck::Proxy.sum(:quantity), Location.count ]).to eq(before) + end + + it "shows no results on the next deck page request after the deck changes (AC-1.3)", :aggregate_failures do + entry = printing + own("main", entry) + proxy = proxy_in("main", entry) + get deck_check_path(deck) + expect(rows).to include("Owned and proxy Lightning Bolt in Main: 1 owned, 1 proxy.") + delete deck_proxy_path(deck, proxy) + follow_redirect! + expect(html.at_css("#deck-check-title")).to be_nil + expect(html.at_css("a[href='#{deck_check_path(deck)}']")).to be_present + get deck_check_path(deck) # pressing the button again judges the changed deck + expect(rows).not_to include(a_string_matching(/Owned and proxy/)) + end + + it "words no findings, one finding and several (AC-1.7)", :aggregate_failures do + own("main", plains, quantity: 60) + get deck_check_path(deck) + expect(html.at_css(".c-check__summary").text).to eq("No findings") + own("sideboard", plains, quantity: 16) + get deck_check_path(deck) + expect(html.at_css(".c-check__summary").text).to eq("1 finding") + own("main", printing(name: "Shock"), quantity: 5) + get deck_check_path(deck) + expect(html.at_css(".c-check__summary").text).to eq("2 findings") + end + + it "gives a deck with no rules the no-rules line and the owned-plus-proxy result (AC-2.7)" do + get deck_check_path(create(:deck, account:, name: "Pile", format: "casual")) + expect(rows).to eq([ "Casual has no rules to check.", "Pass No card has both owned copies and proxies in one zone." ]) + end + + it "is a 404 for another account's deck, a zone and a bad id (AC-1.6)", :aggregate_failures do + theirs = "Rival #{SecureRandom.hex(4)}" # built at run time: the test 404 page prints the spec's own source lines (docs/lessons 2026-11-11) + other = create(:deck, account: create(:user).account, name: theirs) + [ other.to_param, deck.zone("main").id, 0, "abc" ].each do |id| + get deck_check_path(id) + expect(response).to have_http_status(:not_found), "#{id.inspect} was not a 404" + expect(response.body).not_to include(theirs) + end + end + end + + it "redirects a visitor who is not signed in (AC-5.2)" do + get deck_check_path(deck) + expect(response).to redirect_to(new_session_path) + end +end +``` + + Run `bin/rspec spec/requests/deck_checks_spec.rb` → **FAIL** (`undefined local variable deck_check_path`). + +- [ ] **Implement.** + - `config/routes.rb`, after `resource :selection, only: :create, module: :decks` (line 51): `resource :check, only: :show, module: :decks # spec 079: Check deck, the deck page with its checks`. + - `app/controllers/decks/checks_controller.rb`: + +```ruby +# Check deck (spec 079): the deck page with its checks' results. A read-only request, so the link to it needs no form and the +# result is never stored: any later request of the deck page has none (decision 7). The deck comes from the signed-in +# account's decks, so another account's is a 404 (`FindsDeck`). It builds the page and the shape panel as +# `DecksController#show` does and adds one report; the report reads the loaded rows and runs no statement of its own (AC-5.1). +class Decks::ChecksController < ApplicationController + include FindsDeck + + before_action :set_deck + + # Renders the deck page with `@check` set. + def show + @page = Deck::Page.new(@deck) + @shape = Catalog::DeckShape.answer(@deck.collectible_type, @page.zones) + @check = Catalog::DeckChecks.report(@deck.collectible_type, @deck.format, @page.zones) + render "decks/show" + end +end +``` + + - `app/views/decks/show.html.erb`: after line 1 add `<%# The results page isn't kept for Back: Back asks for the check again, so it never shows results from before an edit (spec 079 decision 7). %>` and `<% content_for :head, tag.meta(name: "turbo-cache-control", content: "no-cache") if @check %>`; after the Edit deck link add `<%= link_to "Check deck", deck_check_path(@deck), class: "c-btn c-btn--secondary", data: { turbo_prefetch: false } %>` (a hover prefetch would compute a check nobody asked for); between the head's closing `` and `
` add `<%= render "decks/check", check: @check if @check %>`. + - `app/views/decks/_check.html.erb`: + +```erb +
+

Deck check

+

<%= check.summary %>

+
    + <% check.lines.each do |line| %> +
  • "> + <% if line.status %> + <%= render(line.fault ? "icons/alert" : "icons/check") %><%= line.status %> + <% end %> + <%= line.sentence %> +
    • + <% end %> +
+
+``` + + - `spec/deck_core_spec.rb`: raise the total by `+ 2` with the clause `spec 079 adds the check's controller and the results partial (the globs already cover them)`. + - `spec/system/deck_page_spec.rb:259`: the head now has three `.c-btn` (Edit deck, Check deck, More): change `[ true, true ]` to `[ true, true, true ]`; read `:264-273` (the link is a `c-btn--secondary`, so heights ≥ 36 hold). +- [ ] Run `bin/rspec spec/requests/deck_checks_spec.rb spec/requests/deck_pages_spec.rb spec/deck_core_spec.rb spec/system/deck_page_spec.rb` → PASS (the last is the file this task edits). Rubocop; commit `feat(decks): Check deck on the deck page (#258)`. + +### T3.2 The statement count + +- [ ] **Test** append to `spec/requests/deck_checks_spec.rb` (AC-5.1; the form of `spec/requests/query_counts_spec.rb:227-253`, the bulk fixtures of `spec/requests/collection_import_proxy_timing_spec.rb:43-60`): + +```ruby + describe "statements" do + before { sign_in_as(user) } + + # `count_queries` is `spec/support/query_counting.rb`'s (cached, SCHEMA and TRANSACTION statements are not counted). + def queries_for(path) + count = count_queries { get path } + expect(response).to have_http_status(:ok) + count + end + + # +rows+ printings of one identity, bulk-inserted and owned in the deck's Main zone, ten of them also as proxies. + def fill(deck, rows, label) + set = create(:catalog_set, code: label) + identity = create(:catalog_identity, name: "Bulk #{label}") + now = Time.current + Catalog::Entry.insert_all(Array.new(rows) { |i| { collectible_type: "mtg", external_key: "#{label}-#{i}", catalog_set_id: set.id, catalog_identity_id: identity.id, number: i.to_s, language: "en", name: "Card #{i}", kind: "card", content_digest: "d", created_at: now, updated_at: now } }) + ids = Catalog::Entry.where(catalog_set_id: set.id).pluck(:id) + MTG::Printing.insert_all(ids.map { |id| { catalog_entry_id: id, rarity: "common", finishes: %w[nonfoil], layout: "normal", scryfall_uri: "https://example.test/#{id}", faces: [], created_at: now, updated_at: now } }) + location = deck.zone("main") + Lot.insert_all(ids.map { |id| { account_id: account.id, catalog_entry_id: id, quantity: 2, finish: "nonfoil", price_paid_cents: nil, location_id: location.id, lot_key: Lot.key_for("nonfoil", nil, nil, location.id), created_at: now, updated_at: now } }) + ids.first(10).each { |id| create(:deck_proxy, zone: location, account:, entry: Catalog::Entry.find(id), finish: "nonfoil", quantity: 1) } + end + + it "runs the plain deck page's statements however many rows the deck has", :aggregate_failures do + small = create(:deck, account:, name: "Small", format: "standard") + big = create(:deck, account:, name: "Big", format: "standard") + fill(small, 5, "small") + fill(big, 500, "big") + [ small, big ].each { |deck| get deck_path(deck) } # warm the page once + expect(queries_for(deck_check_path(small))).to eq(queries_for(deck_path(small))) + expect(queries_for(deck_check_path(big))).to eq(queries_for(deck_path(big))) + expect(queries_for(deck_check_path(big))).to eq(queries_for(deck_check_path(small))) + end + end +``` + + (The bulk printings list `nonfoil`, so each proxy uses `finish: "nonfoil"`. A 500-row page renders every row; record the example's runtime in `verification.md`.) +- [ ] Run → PASS (the check adds no statement), and run `bin/rspec spec/models/deck/page_spec.rb`: its examples at `:173-200` pin `Deck::Page#zones` at 3 and 6 statements, which is AC-5.1's "`Deck::Page#zones` is as before". **Mutation proof:** temporarily make `split_lines` call `Catalog::Entry.find(it.row.entry.id)` per card, see the example FAIL, revert. Record it in `verification.md`. Commit `test(decks): the check runs no statement of its own (#258)`. + +## Phase 4: Design system and browser proof + +**Implements:** NFR Accessibility and design; decisions 6–8 · **Satisfies:** AC-1.8 (browser), 6.1–6.4, 7.4 +**Files:** `app/assets/stylesheets/collector/additions.css`, `docs/design-system/components/DeckCheck.md`, `DeckPage.md`, `docs/design-system/README.md`, `spec/system/deck_checks_spec.rb` +**Interfaces:** Consumes the `decks/_check` markup: `c-check`, `c-check__title|summary|list|row|status|text`, `--pass|--fault|--plain`. + +### T4.1 The `DeckCheck` pattern + +- [ ] **Test first (red).** Write T4.2's file (below) and run `bin/rspec spec/system/deck_checks_spec.rb` before any CSS exists: the phone example must **FAIL** (the 60-character text and zone name are unwrapped and clipped with no `c-check` rule). That failure is the red step for this task's CSS. +- [ ] Invoke the `collector-design-system` skill; read `docs/design-system/README.md`, `DeckPage.md`, `Badge.md`, `StageList.md`. Reuse first: `c-section__title` for the heading; `icons/check` and `icons/alert` at the 12 px badge-icon size. The gap is the Complexity note above. +- [ ] Add to `additions.css` after the `c-deck` block (tokens only; text sizes are the type scale's: body 15/22, data 13/18; the 12 px icon takes the 2.5 stroke the README gives an icon no component sizes, as `.c-badge svg` does at `additions.css:634`): + +```css +/* DeckCheck (spec 079): the Check deck results, a full-width section between the deck page's head and its zones. A row is a + status word with its icon and a sentence; the word and the icon say pass or fault, so colour is never the only signal. */ +.c-check { margin-top:var(--space-6); display:flex; flex-direction:column; gap:var(--space-3); min-width:0; } +.c-check__title:focus-visible { outline:2px solid var(--focus); outline-offset:2px; border-radius:var(--radius-sm); } +.c-check__summary { margin:0; font:400 13px/18px var(--font-mono); color:var(--ink-muted); } +.c-check__list { list-style:none; margin:0; padding:0; background:var(--surface-raised); border:1px solid var(--line); border-radius:var(--radius-md); } +.c-check__row { display:grid; grid-template-columns:minmax(0, 1fr); gap:var(--space-1); padding:var(--space-3) var(--space-4); border-top:1px solid var(--line); overflow-wrap:anywhere; } +.c-check__row:first-child { border-top:0; } +.c-check__status { display:inline-flex; align-items:center; gap:var(--space-2); font:600 13px/18px var(--font-sans); color:var(--ink); } +.c-check__status svg { width:12px; height:12px; flex:none; stroke-width:2.5; } +.c-check__row--pass .c-check__status svg { color:var(--success); } +.c-check__row--fault .c-check__status svg { color:var(--warning); } +.c-check__text { font:400 15px/22px var(--font-sans); color:var(--ink); } +@container page (min-width: 640px) { + .c-check__row { grid-template-columns:11rem minmax(0, 1fr); align-items:baseline; gap:var(--space-4); } + .c-check__row--plain .c-check__text { grid-column:1 / -1; } +} +``` + + Adjust a value only to match an existing token (confirm `--radius-sm` exists in `tokens.css`); re-run T4.2 after any change. +- [ ] Write `docs/design-system/components/DeckCheck.md` (shape of `DeckShape.md`: what it is, a markup sample, the classes, the pass/fault/plain states, colour never alone, focus on the heading, phone behaviour, the 12 px icon at stroke 2.5). Edit `DeckPage.md` (the "As built" paragraph gets the head's Check deck link and the results section between head and `c-deck`; keep "no `form`") and `README.md` (an App additions line, and `DeckCheck`'s 12 px icon added to the Iconography list beside the badge and legality icons). Commit `feat(design): the DeckCheck pattern for the check results (#258)`. + +### T4.2 Browser proof + +- [ ] **Test** `spec/system/deck_checks_spec.rb` (type :system; helpers `system_sign_in_as`, `wait_for_turbo_idle`, `open_in_viewport_frame`, `within_narrow_frame`, `page_fits_sideways?`, `tab_to_text`; `set_theme` defined locally as in `spec/system/deck_page_spec.rb:32`): + +```ruby +require "rails_helper" + +RSpec.describe "Deck checks", type: :system do + let!(:user) { system_sign_in_as(create(:user)) } + let(:deck) { create(:deck, account: user.account, name: "Izzet Tempo", format: "standard") } + + def set_theme(name) = page.execute_script("document.documentElement.dataset.theme = #{name.to_json}") + + def printing(name, type_line: "Basic Land — Plains") + entry = create(:catalog_entry, identity: create(:catalog_identity, name:), name:) + create(:mtg_printing, entry:, faces: [ { "name" => name, "type_line" => type_line, "image_uris" => {} } ]) + entry + end + + def clipped(selector) = page.evaluate_script("[...document.querySelectorAll(#{selector.to_json})].filter(el => el.scrollWidth > el.clientWidth + 1).length") + + it "moves keyboard focus to the results heading (AC-6.3)", :aggregate_failures do + visit deck_path(deck) + expect(tab_to_text("Check deck")).to be(true) # the link is reachable by Tab, and Enter is pressed on it + page.send_keys(:enter) + expect(page).to have_css("#deck-check-title:focus", text: "Deck check") + expect(page.evaluate_script("document.getElementById('deck-check-title').tabIndex")).to eq(-1) + end + + it "asks for the check again on Back instead of restoring a Turbo snapshot (AC-1.8)", :aggregate_failures do + visit deck_path(deck) + click_link "Check deck" + expect(page).to have_text("Main: 0 cards; Standard needs at least 60.") + page.execute_script("window.__sameDocument = true") # Turbo visits keep the document; a full page load would drop this + create_lot_in_zone(deck, "main", entry: printing("Plains"), quantity: 60) + click_link "Edit deck" + wait_for_turbo_idle + page.go_back + expect(page).to have_text("Main: 60 cards; Standard needs at least 60.") + expect(page.evaluate_script("window.__sameDocument")).to be(true) # still the same document, so Turbo did the Back + end + + it "puts the results before the zones in reading order (AC-6.3)" do + visit deck_check_path(deck) + follows = "document.getElementById('deck-check-title').compareDocumentPosition(document.querySelector('.c-zone')) & Node.DOCUMENT_POSITION_FOLLOWING" + expect(page.evaluate_script("!!(#{follows})")).to be(true) + end + + it "fits a phone, in both themes, with long names (AC-6.1, 6.4)", :aggregate_failures do + long = "N" * 60 + create_lot_in_zone(deck, "main", entry: printing(long, type_line: "Instant"), quantity: 5) + deck.zone("main").update_columns(name: "Z" * 60) # a zone's shown name is a copy, so a long one is a fixture, not a rename flow + visit deck_path(deck) + expect(open_in_viewport_frame(deck_check_path(deck), viewport: 360, ready: "#deck-check-title")).to eq(360) + within_narrow_frame do + %w[light dark].each do |theme| + set_theme(theme) + expect([ page_fits_sideways?, clipped(".c-check__text, .c-check__status, .c-item__actions .c-btn") ]).to eq([ true, 0 ]) + end + end + end + + it "says pass or fault in words with an icon on every row, never by colour alone (AC-6.2)", :aggregate_failures do + create_lot_in_zone(deck, "main", entry: printing("Plains"), quantity: 58) + visit deck_check_path(deck) + rows = all(".c-check__row--fault .c-check__status") + expect(rows.map(&:text)).to all(be_in([ "Short", "Over", "Over the limit", "Owned and proxy" ])) + expect(page).to have_css(".c-check__status svg", count: page.all(".c-check__status").size) + end +end +``` + + (The narrow-frame example's card uses an Instant type line so the copy limit applies and the finding "NNN…: 5 copies; the limit is 4." is the long text under test; the other examples use basic lands so a count finding is the only finding. The Back example follows the repo's own pattern, `spec/system/collection_spec.rb:22-33` and `back_navigation_spec.rb`: Turbo navigations, then Back, with a marker on `window` showing the document is the same one, so the Back was Turbo's and the snapshot cache was in play.) +- [ ] **Mutation proofs (record each in `verification.md`):** (a) remove `overflow-wrap:anywhere` from `.c-check__row`, confirm the phone example fails, restore (memory `headless-firefox-narrow-frame.md`); (b) remove the `turbo-cache-control` line from `decks/show.html.erb`, confirm the Back example fails (Turbo restores the 0-card snapshot), restore; (c) remove `autofocus` from the heading, confirm the focus example fails, restore. Run `bin/rspec spec/system/deck_checks_spec.rb spec/system/deck_page_spec.rb` → PASS. Commit `test(decks): the check's focus, Back and phone width in the browser (#258)`. + +### T4.3 Visual evidence + +- [ ] Per memories `pr-screenshots-workflow.md` and `pr-visual-evidence-required.md` (headless Firefox, magick/ffmpeg; set `data-theme` for light shots; pause 0.6 s after a theme switch): seed a Standard deck with findings (a short Main, a 5-of, a split), a Vintage deck with a restricted card, and a Casual deck. Capture the results at desktop and 360 px in light and dark, the plain page's head with the new link at 360 px, and a GIF of deck page → Check deck → results → remove a proxy → plain page. Save under `docs/specs/079-deck-checks/screenshots/`, embedded in the PR by SHA-pinned link. + +## Phase 5: Documents, verification and review + +**Implements:** Story 7 · **Satisfies:** AC-5.3, 5.4, 7.1–7.6 +**Files:** `docs/architecture/collection-and-lots.md`, `CLAUDE.md`, `docs/roadmap.md`, `docs/specs/README.md`, `docs/specs/079-deck-checks/verification.md` + +- [ ] **T5.1** `collection-and-lots.md`: the deck checks' rules (decisions 1–10) with the spec as source and a pointer to `Catalog::DeckChecks`' comment (not a copy of the contract); spec 069's non-goal done in part; the new files in the area's list. `CLAUDE.md` Project Status gets one clause ("A collector checks a deck on demand from its page: deck size, copy limits and owned plus proxy of one card (`Catalog::DeckChecks`, `MTG::Collecting.deck_rules`, spec 079)"). `docs/roadmap.md` theme C lists #258 as shipped. Verified by reading, not RSpec (`.claude/rules/testing.md`). +- [ ] **T5.2** `verification.md` in the shape of spec 069's: an AC → proving-example table per story; Story 7 "verified by reading"; the supersession grep (`git grep -n -i "deck rules\|copy limit\|deck size" -- spec docs app ':!.kamal/secrets'`); the Scryfall facts (T0.2); the mutation proofs (T2.2's `MAIN_MINIMUM`, T3.2's per-row statement, T4.2's CSS, cache tag and `autofocus`); the screenshots; findings; deviations. +- [ ] **T5.3** `bin/ci` (RuboCop, YARD, comment ratchet, Brakeman, bundler-audit, importmap audit, the whole suite). A CI failure is reproduced with its seed first (memory `reproduce-ci-with-seed.md`). Cite the pushed SHA. AC-5.3's evidence is the unchanged schema, export spec (`FORMAT_VERSION == 4`) and backup guard. +- [ ] **T5.4** `sdd-review` Mode B on **Opus**, recorded under `## Implementation review` in `verification.md` with the verdict first; then `distill-feature` before the PR is marked ready; open the PR with the maintainer assigned and `gh pr view N --web` once CI is green (memory `open-green-prs-for-review.md`); merge only on their approval. + +## Spec coverage + +| AC | Task | AC | Task | +|---|---|---|---| +| 1.1, 1.9 | T3.1 | 4.1–4.4, 4.6, 4.7, 4.9, 4.10 | T1.1 | +| 4.8 | T3.1 (no write reaches lots, proxies or locations) | +| 1.2, 1.3, 1.4, 1.5, 1.6, 1.7 | T3.1 | 4.5 | T2.2, T3.1 | +| 1.8 | T3.1 (the tag), T4.2 (Back) | 5.1 | T3.2 | +| 2.1–2.6, 2.8, 2.9 | T1.2, T2.2 | 5.2 | T3.1 | +| 2.7 | T1.1, T2.2, T3.1 | 5.3 | T5.3 | +| 2.10–2.13 | T1.2, T2.2 | 5.4 | T1.1, T3.1 | +| 3.1–3.3, 3.5, 3.6, 3.8–3.11, 3.13, 3.15 | T1.3, T2.2 | 5.5 | T1.1 | +| 3.4, 3.7, 3.12, 3.14, 3.16 | T2.1, T2.2 | 6.1–6.4 | T4.2 | +| | | 7.1, 7.3–7.6 | T4.1, T5.1, T5.3 | +| | | 7.2 | T0.1 (the index row, committed with the spec) | + +FR-1 → T1.1, T2.1; FR-2 → T1.1–T1.3, T2.1; FR-3 → T3.1. NFRs: performance T3.2; security T3.1; reliability T1.1, T2.1; accessibility T4.1–T4.2. + +## Verification (end to end) + +1. `bin/rspec spec/models/catalog/deck_checks_spec.rb spec/models/mtg spec/requests/deck_checks_spec.rb spec/system/deck_checks_spec.rb spec/deck_core_spec.rb` green; then `bin/ci` green. +2. `bin/dev`, sign in, open a Standard deck, press **Check deck**: results appear between head and zones, focus is on "Deck check", the summary reads as expected; remove a proxy: the plain page returns with no results; press Back: a fresh check. Repeat at phone width in both themes. +3. Casual deck: "Casual has no rules to check." then the owned-plus-proxy result. Another account's deck URL: 404. + +## Risks and notes + +- **Unverified facts (T0.2):** Scryfall's rules-text wording and that `name` is English for a non-English printing. A wording difference changes two regexes and the fixtures, nothing else. +- **Autofocus on a heading:** Turbo honours `autofocus` on a link visit (an `advance` visit uses `PageRenderer`; morph renders don't focus, and this isn't one) and the browser does on a full load; the repo has no heading precedent, so T4.2's first example is the proof. If it fails, fall back to the repo's own pattern (`sitting_focus_controller.js`: `tabindex="-1"` and a one-shot Stimulus controller). +- **Three head buttons at 360 px:** Edit deck, Check deck and More share `c-item__actions`; T4.2 and the screenshots are the proof. If they overflow, wrap rather than shrink. +- **A numbered card with an unknown number word** ("up to twelve") falls through to the ordinary limit. Only two real cards have the text and both are in the map. +- **Left out on purpose:** boot-time verification of `deck_rules` (zone keys against the format's zones). It isn't in the spec; a wrong key counts as no cards and is named from the registration, which T1.2 tests. Offered as a follow-up. diff --git a/docs/specs/079-deck-checks/spec.md b/docs/specs/079-deck-checks/spec.md new file mode 100644 index 00000000..a64bacdc --- /dev/null +++ b/docs/specs/079-deck-checks/spec.md @@ -0,0 +1,305 @@ +# Feature 079: Deck Checks (Deck Size, Copy Limits, and Owned Plus Proxy of One Card) + +**Status:** Approved (2026-11-12, the maintainer, v0.2.1) +**Version:** 0.2.1 +**Created:** 2026-11-12 +**Last Updated:** 2026-11-12 +**Depends on:** [spec 060](../060-deck-components/spec.md) (the deck page's components; decision 12, a deck's cards are owned plus proxy), [spec 062](../062-create-rename-remove-a-deck/spec.md) (decks, zones and the format registry `Catalog::Formats`; merged), [spec 066](../066-deck-page-owned-cards-by-zone/spec.md) (the deck page, its seam `Catalog::DeckCards` and the legality marker; merged), [spec 068](../068-deck-proxies/spec.md) (proxies in a zone; merged), [spec 069](../069-deck-shape-panel/spec.md) (the second seam, `Catalog::DeckShape`, and the counted-zones and statement-count precedents; merged), [the decks and locations issue design](../../superpowers/specs/2026-11-09-decks-and-locations-issues-design.md) (decisions 5, 6 and 8 and items 13, 15 and 16) +**Branch:** `079-deck-checks` +**Issue:** [#258](https://github.com/plainprogrammer/Collector/issues/258) (the 2.0 milestone, roadmap theme C) +**ADR:** none: ADR 0028 already puts a deck's collectible part behind the collecting vocabulary, and specs 066 and 069 added optional answers to that seam; this spec adds a third the same way (decision 1) + +--- + +## Changelog + +| Version | Date | Change | +|---------|------|--------| +| 0.2.1 | 2026-11-12 | Spec review round 2 (Opus, NEEDS REVISION, 0 blocking, 7 important, 14 minor; no ruling reopened) fixed: the size rule holds zone keys, a minimum and a maximum and the core owns every word (decision 1; one shape: minimum, maximum or exact); a size rule's zone is counted over the deck's zone rows and named from the registration when the deck has none; the limit question is asked for every counted printing, a printing with no record gets the format's ordinary limit, "none" is above every number, and the reason is the lowest limit's; owned-plus-proxy findings are ordered by their own zone; AC-1.2 is for a deck with rules; the mechanism of "not kept for Back" is Turbo's no-cache tag, and the outcome is stated as a fresh check; `main` has no form; the results heading is "Deck check"; the split's wording; the guard's files are named; AC-7.1 says "in part"; a mis-cited decision, the header's citation of "Left to each issue's spec", AC-2.11's fixture and AC-1.5 are corrected; multi-case criteria are one example per case; `CLAUDE.md`'s Project Status is a Story 7 obligation | +| 0.2.0 | 2026-11-12 | Spec review round 1 (Opus, NEEDS REVISION, 2 blocking, 11 important, 8 minor) fixed: the seam now receives the cards and names its contract (decision 1); a printing with no card data has one rule (decision 4, Error Scenarios, NFR); a request-and-staleness design that fits Turbo (the check is a GET, decision 7); the placement of the control and results (decision 8); which name a card has in a finding and the order of findings (decision 6); every result string (decision 6); the results' order and a no-rules deck (decision 10); a card of another collectible is left out of every check, owned plus proxy included (decision 4); the focus rule for a screen reader (AC-6.3); the statement-count criterion in spec 069's form (AC-5.1); zones the rules don't name (decision 3); the guard, comment and YARD obligations (Story 7); wrong citations of the issue design. The maintainer ruled (2026-11-12) that a card whose text states a number is held to that number in every format, Commander included, and that the button sits in the page head with the results above the zones | +| 0.1.0 | 2026-11-12 | First draft from issue #258, the issue design, specs 062, 066, 068 and 069, and the maintainer's answers of 2026-11-12 (the Check deck button; size and copy limits only; copies counted by card across printings and zones; owned plus proxy of one card in one zone, any printing) | + +## Problem Statement + +A collector building a deck can't tell whether it meets its format's basic rules. Spec 066 marks each card that isn't legal, but nothing judges the deck as a whole: is it big enough, are there too many of one card, did a proxy stay behind next to the owned copy it was meant to be replaced by. Proxies change only by hand (the issue design's decision 8), so a forgotten proxy never announces itself, and a collector at the table or the sleeve box finds out too late. + +This spec adds a **Check deck** button to the deck page. Pressing it judges the deck against its format's size and copy rules and flags every card that has both owned copies and proxies in one zone, and shows the findings on the page. The checks are advisory (they never block an edit) and on demand (nothing is judged, stored or shown until the collector asks). The rules for a game and its formats are that collectible's, kept in code behind the collectible extension; the core deck page names no game, format or zone. + +> **Terms.** A *card* in a zone is one owned copy or one proxy, so a row of Owned 3 and Proxy 1 is four cards (spec 060 decision 12). *The same card* is one catalog identity across all its printings, so Lightning Bolt from two sets is one card. A *check* is one question put to the deck (its size, its copy limits, its owned-plus-proxy cards); a *finding* is a check's answer that something is wrong; a check that finds nothing *passes*. A *rule's zones* are the zones the extension names for that rule (decision 1); a zone no rule names is *unnamed*. + +## Goals + +- A deck page offers **Check deck**, and pressing it shows, on that page, the summary line and a result for each check that applies: a pass line or one finding per fault, each saying what is wrong in numbers. +- A Magic deck is judged against its format's rules (the table in decision 3): the size of its Main zone (and its Commander zone for Commander) and of its Sideboard, and the copies of each card. +- Any deck, whatever its game or format, is checked for a card with both owned copies and proxies in one zone, so a forgotten proxy shows up. +- The rules for a format are code in the collectible extension; the core names no collectible, format or zone key but `considering` (`spec/deck_core_spec.rb`). +- The checks add nothing to the page until asked for, store nothing, and run no statement beyond the ones the deck page already runs. + +## Non-Goals + +- Colour identity, banned cards and restricted cards of any format but Vintage's limit of one, and a whole-deck "is this deck legal" verdict. Spec 066's per-card marker is the legality view; this spec doesn't repeat it and doesn't judge the number or kind of commanders. +- Collector-defined formats and zones (the issue design's item 16) and rules a collector can change. +- Checks that run unasked: on every page view, on save, in the background or in an export. Nothing is stored and nothing is exported (decision 9). +- Blocking, warning on or refusing any edit because of a finding. The checks are read-only advice. +- A change to the proxy, the zone or the lot behaviour: a finding names a proxy, it never changes one (the issue design's decision 8 stands). +- Rules for Considering, and Sideboard rules beyond decision 3's size. Considering is never counted. +- A "fix it" action beside a finding, and a deck-builder workflow (the issue design's item 15). +- A new look: the results reuse the design system's patterns. If the plan finds a pattern it lacks, the pattern is added the way `CLAUDE.md` says (`collector/additions.css` and a component document), as an addition and not a redesign. + +## Users and Context + +**Primary users:** a collector with a deck, owned or planned, who is about to build, sleeve or play it. +**Usage context:** at a desk or on a phone, on the deck page, after adding or moving cards, asking "is this deck ready?". +**User mental model:** "a 60-card deck, no more than four of anything but basic lands; a Commander deck is 100 cards and one of each". A proxy is a card they will have on the table; "owned and proxy of one card" means "I meant to swap this proxy for my copy". + +## Decisions + +Decisions 1 to 10 are this spec's calls inside the issue and the issue design's outline ("Advisory and on demand, shown on the deck page"; "Formats stay rules in code, in the collectible extension"). Those marked *(my call)* are not stated by the issue or the maintainer's rulings; Gate A confirms them. + +1. **A third optional answer on the collecting vocabulary, beside `formats`, `deck_card` and `deck_shape` (the seam).** A vocabulary may answer `deck_rules(format_key)`, the rules of one of its formats, or nil for a format it has no rules for. A core module `Catalog::DeckChecks` (added to `spec/deck_core_spec.rb`'s list; it names no collectible, format or zone key but `considering`) asks it with the deck's collectible type and format, runs the rules over the zones as `Deck::Page#zones` built them, and returns the results the core partial draws. **The contract**, written in `Catalog::DeckChecks`' comment (the one place it is written down; the area doc points to it): + - **Size rules.** The answer lists size rules in the order they are shown. Each holds the *zone keys* it counts (the extension names them; the core matches them to each zone row's key and never names one) and one of three shapes: a minimum only, a maximum only, or an exact size (minimum and maximum equal); a range with two different bounds is not offered. The extension supplies no words: decision 6's sentences, the format's shown name (`Catalog::Formats.name_for`) and the zones' shown names are the core's. The cards of a rule are counted over the deck's zone rows with those keys; a key the deck has no zone row for counts as no cards and is named from the format's registration (`Catalog::Formats.zones_for`), or by its key when the registration lacks it. A deck zone no rule names is unnamed. + - **The copy rule.** The answer names the zone keys the copy rule counts together and gives a way to ask, for one counted printing (a `Catalog::Entry` whose extension record may be nil, as `Catalog::DeckCards` is asked for an entry loaded by `Catalog::Entry.preload_extensions`), what its card's limit is: a number, with an optional reason word ("restricted to 1"), or none (never limited). For a printing with no extension record the extension answers the format's ordinary limit (decision 4). The core counts copies by catalog identity and asks for **every** counted printing of a card; *none* is above every number, so a card is unlimited only when every counted printing answers none, and otherwise it is held to the lowest number, with the reason given alongside that number (the first printing in page order on a tie). *The mixed case* (one printing of a basic land has a record, another has none) therefore gives the ordinary limit, the same accepted consequence as decision 4's. + - **Reads.** Asking reads the loaded extension records only: no statement per card, and none beyond the deck page's own (decision 7, AC-5.1). The core reads each row's `catalog_identity_id`, which the page's entries already carry. + - **The core owns** the counting, the results, their words (decision 6), the order of findings and the pass lines, and it shows the zones' own names (`Zone#name`), joined with " + ". + + A vocabulary without `deck_rules`, a format it returns nil for, a collectible no longer registered and a deck whose format is no longer registered have **no size or copy rules**: the page says so in one line (decision 6). *Alternative:* a hard-coded table in the core. Rejected: the core would name formats, which `spec/deck_core_spec.rb` forbids and issue #258 rules out. +2. **Owned plus proxy is a core check, run for every deck.** It needs no game rule: it reads each zone's rows through their catalog identity and the owned and proxy counts the page already has. It runs for Casual and for a deck whose format is no longer registered, because a proxy forgotten there is as forgotten. *(my call)* +3. **The rules, by format (Magic).** *(The maintainer chose size and copy limits only, on 2026-11-12.)* + + | Format | Size | Copies of one card | + |---|---|---| + | Standard, Pioneer, Modern, Legacy, Pauper | Main at least 60; Sideboard at most 15 | at most 4, Main and Sideboard together | + | Vintage | as above | at most 4, and at most 1 of a card the catalog lists as restricted in Vintage, Main and Sideboard together | + | Commander | Commander and Main together exactly 100 | at most 1, Commander and Main together | + | Casual | none | none | + + Basic lands (a type line with the supertype Basic, Snow-Covered ones included) are never limited. A card whose own rules text says a deck can have any number of cards with its name is never limited; a card whose text states a number ("up to seven", "up to nine") is held to that number **in every format, Commander included** (the maintainer's ruling, 2026-11-12). Considering is never counted, for size or for copies, and neither is any unnamed zone. Owned copies and proxies both count (spec 060 decision 12); a proxy is a card on the table. A card is *restricted in Vintage* when any counted printing of it lists it so. +4. **Cards are counted by catalog identity; the cards counted are the deck's collectible's.** The copies of "a card" are summed over every printing of it in the rule's zones, owned plus proxy, so four Lightning Bolts from two sets are four. A card of another collectible than the deck's (a lot isn't type-checked when it is placed in a zone, so a page can hold one) is **left out of every check** (size, copies and owned plus proxy) and is named in no finding, as spec 069 decision 3 does for the panel, so a size count can be smaller than the zone head's "N cards", which counts every row. A token or any other kind of card placed in a zone counts like any card. A printing with no card data (no extension record: the catalog hasn't been refreshed since it was added) is a card like any other: it counts for size and for copies by its identity, and it is never exempt and never restricted, because nothing says it is. The consequence is stated and accepted: before a refresh, a basic land with no record is judged like any card. *(The maintainer chose "by card, across printings and zones", 2026-11-12; the no-card-data rule is my call.)* +5. **The owned-plus-proxy rule.** A card is flagged in a zone when that zone holds at least one owned copy and at least one proxy of the card, in the same or in different printings, in any zone including Considering and an unnamed zone. Each zone is judged alone: owned in Main and a proxy in Considering is not a finding, because a proxy in Considering is a plan and not a leftover *(my call)*. A finding names the card, the zone's shown name and the split ("2 owned, 1 proxy"). *(The maintainer chose "same card in one zone, any printing", 2026-11-12.)* +6. **Findings are numbers, in words; these are the strings.** Every result has a **status word** and a sentence, and the word and an icon the design system already has (if the plan finds one) say pass or fault, so nothing depends on colour alone. "N cards" and "N copies" use `number_with_delimiter` and the singular for one. *(my call)* + + | Result | Status word | Sentence | + |---|---|---| + | Size below the minimum | Short | "Main: 58 cards; Standard needs at least 60." | + | Size above the maximum | Over | "Sideboard: 17 cards; Standard allows at most 15." | + | Size not exact | Short or Over | "Commander + Main: 99 cards; Commander needs exactly 100." | + | Size right | Pass | the same sentence with the true count: "Main: 60 cards; Standard needs at least 60." (one line per size rule) | + | A card over its limit | Over the limit | "Lightning Bolt: 5 copies; the limit is 4." or, with a reason, "Black Lotus: 2 copies; restricted to 1." | + | No card over its limit | Pass | "No card is over its limit." | + | Owned and proxy in one zone | Owned and proxy | "Lightning Bolt in Main: 2 owned, 1 proxy." | + | No such card | Pass | "No card has both owned copies and proxies in one zone." | + | No size or copy rules | none | "Casual has no rules to check." (the format's shown name, or its stored key when it is no longer registered) | + + A card is named by the English name Scryfall gives the card (`Catalog::Entry#name`, the same for every printing, a double-faced card's "A // B" included), never by the localised name a non-English printing shows on the deck page. Copy findings are ordered by the position, in the page's zone order, of the first counted zone that holds the card, then by name, then by identity id. Owned-plus-proxy findings are ordered by their own zone's position in the page's zone order, then by name, then by identity id. A size finding names its rule's zones by their shown names joined with " + ". The split says "N owned, N proxy" (the proxy count's word does not change with the number, as the deck page's head and zone heads write it). The results section's heading is "Deck check". +7. **The check is a read-only request for the deck page; nothing about it is stored.** Check deck is a **link**, styled as a button like "Edit deck", to the deck's check: a read-only request (a GET, so no form and no redirect) that shows the deck page with the results. Pressing it again checks again against the deck as it then is. The results are never stored, so **any later request of the deck page has none**: a redirect after an edit, a change of format or a visit to the plain deck page shows no results and shows the button again. The results page carries Turbo's `turbo-cache-control: no-cache` tag (as the collection and search pages do), so Turbo keeps no snapshot of it: Back to the results page is a fresh request of the check, which judges the deck as it then is and never shows results computed before an edit. The deck page without a press runs the statements it runs today, and the check runs none beyond them. *(The maintainer chose the button, 2026-11-12; the mechanics are my call.)* +8. **Where it sits.** The button is in the page head's actions beside Edit deck. The results are a full-width section between the head and the zones (so a phone reads them first), with a heading and the summary line; the zones and the shape panel are drawn as on the plain page. On the results page the button remains and checks again. *(The maintainer chose this placement, 2026-11-12.)* +9. **Nothing is stored and nothing is exported.** A result is computed when the check is requested and shown; it needs no migration, no job, no cache, no column and no change to the export, the import or the backup. The export format stays at version 4. +10. **The summary line and the order of results.** Above the results, one line says how many findings there are ("3 findings", "1 finding") or "No findings" when every check passed or has no rules. The results follow in this order: the size rules in the extension's order, then the copy check, then owned plus proxy. A deck with no size or copy rules shows the no-rules line and then owned plus proxy. *(my call)* + +## User Stories + +### Story 1: Check a deck on demand + +**As a** collector reading a deck +**I want** to press Check deck and see what is wrong with it +**So that** I can fix it before I build or play the deck. + +**Acceptance criteria:** + +- [ ] **AC-1.1** Given a deck page When the collector has not asked for the check Then the page shows the Check deck button in the head's actions and no result, summary line or finding, and runs the statements it ran before this feature. +- [ ] **AC-1.2** Given a deck whose format has size and copy rules When the collector presses Check deck Then the deck page is shown with the summary line and, in the order of decision 10, one result for each size rule, the copy check and the owned-plus-proxy check, between the head and the zones, with the zones still shown. +- [ ] **AC-1.3** Given results are shown When the collector removes a proxy from a row's menu and the deck page is shown again Then it has no results and shows the button, and pressing the button shows the results for the changed deck. +- [ ] **AC-1.4** Given results are shown When the collector presses Check deck again without changing the deck Then the same results are shown. +- [ ] **AC-1.5** Given a deck that breaks a rule When the collector presses Check deck Then the results page has every link and button the plain deck page has, enabled, and following one of them (for example Edit deck) works as it does from the plain page. +- [ ] **AC-1.6** Given another account's deck When the collector requests its check Then the response is a 404 and nothing of that deck is revealed, as for the deck page itself. +- [ ] **AC-1.7** Given the summary line When there are N findings across the checks Then it reads "N findings" ("1 finding" for one), and "No findings" for none. +- [ ] **AC-1.8** Given the results page When it is served Then its head carries the `turbo-cache-control` tag with the value `no-cache`, and the plain deck page does not (decision 7). +- [ ] **AC-1.9** Given the deck page, plain and with results When it is inspected Then the Check deck control is a link and `main` has no `form`. + +### Story 2: Deck size + +**As a** collector with a Magic deck +**I want** its size judged against its format +**So that** I know it has enough cards, and not too many in the sideboard or in Commander. + +**Acceptance criteria:** + +- [ ] **AC-2.1** Given a Standard deck with 58 cards in Main (owned and proxy together) When the collector checks it Then there is a Short finding with the sentence "Main: 58 cards; Standard needs at least 60." +- [ ] **AC-2.2** Given a Standard deck with exactly 60 cards in Main When it is checked Then the Main size result passes with the sentence "Main: 60 cards; Standard needs at least 60.", and 61 or more also passes. +- [ ] **AC-2.3** Given a Modern deck with 16 cards in Sideboard When it is checked Then there is an Over finding "Sideboard: 16 cards; Modern allows at most 15."; given 15, or none, the Sideboard size result passes. +- [ ] **AC-2.4** Given a Commander deck with 1 card in Commander and 98 in Main When it is checked Then there is a Short finding "Commander + Main: 99 cards; Commander needs exactly 100."; with 101 cards it is Over and says 101; with 100 it passes. +- [ ] **AC-2.5** Given a deck with cards in Considering When it is checked Then those cards are in no size count. +- [ ] **AC-2.6** Given a Main of 59 owned copies and 1 proxy When it is checked Then the proxy counts: Main has 60 cards and the Main size result passes. +- [ ] **AC-2.7** Given a Casual deck, or a deck whose format the instance no longer registers When it is checked Then there is no size or copy result, one line says the format has no rules to check (its shown name, or its stored key when it is not registered), and the owned-plus-proxy result is still shown after it. +- [ ] **AC-2.8** Given a Pauper, Pioneer, Legacy or Vintage deck with a 59-card Main When it is checked Then each shows the same finding with its own format's name. +- [ ] **AC-2.9** Given a Standard deck with no card in Main When it is checked Then the finding says Main has 0 cards. +- [ ] **AC-2.10** Given a card of another collectible in a zone the size rule counts When the deck is checked Then it is not in the count. +- [ ] **AC-2.11** Given a deck with a zone row whose key no rule names, holding cards When it is checked Then that zone is in no size or copy count. +- [ ] **AC-2.12** Given a Standard deck that has no Sideboard zone row (so the rule's key has no zone) When it is checked Then the Sideboard size result passes as 0 cards and names the zone "Sideboard" from the registration. +- [ ] **AC-2.13** Given a Commander deck When it is checked Then the one size result names its zones "Commander + Main" and the sentence is decision 6's. + +### Story 3: Copy limits + +**As a** collector with a Magic deck +**I want** the cards I have too many of named +**So that** I can take them out before I play. + +**Acceptance criteria:** + +- [ ] **AC-3.1** Given a Standard deck with 5 copies of one card in Main When it is checked Then there is an Over the limit finding "Lightning Bolt: 5 copies; the limit is 4." naming the card. +- [ ] **AC-3.2** Given 3 copies of a card in Main and 2 in Sideboard of a Modern deck When it is checked Then the finding says 5 copies, because the zones are counted together; with 4 across both the copy check passes. +- [ ] **AC-3.3** Given 4 copies of a card from one printing and 1 from another printing in one Legacy deck, or 3 owned and 2 proxy of one printing, When it is checked Then each finding says 5 copies: printings and owned plus proxy are counted together. +- [ ] **AC-3.4** Given a deck with any number of Plains, or Snow-Covered Island, in Main When it is checked Then neither is a finding. +- [ ] **AC-3.5** Given a card whose rules text says a deck can have any number of cards with its name, in 9 copies When the deck is checked Then it is not a finding. +- [ ] **AC-3.6** Given a Commander deck with 2 copies of one card, one in Commander and one in Main When it is checked Then the finding says 2 copies and the limit is 1; two basic lands there are no finding. +- [ ] **AC-3.7** Given a Vintage deck with 2 copies of a card the catalog lists as restricted in Vintage When it is checked Then the finding reads "Black Lotus: 2 copies; restricted to 1."; given 2 copies of a card that is not restricted it passes, and the same restricted card in a Legacy deck is limited to 4. +- [ ] **AC-3.8** Given copies in Considering When the deck is checked Then they are counted by no copy rule. +- [ ] **AC-3.9** Given several cards over their limits When the deck is checked Then each has its own finding, ordered as decision 6 says. +- [ ] **AC-3.10** Given a deck with no card over any limit When it is checked Then the copy check passes with "No card is over its limit." +- [ ] **AC-3.11** Given a card of another collectible in a zone of a Magic deck When it is checked Then no rule counts it and no finding names it. +- [ ] **AC-3.12** Given a card whose rules text says a deck can have up to seven cards with its name, in 8 copies in a Standard deck When the deck is checked Then there is a finding saying 8 copies and the limit is 7; and given 8 copies in a Commander deck the finding says 8 copies and the limit is 7; and given 7 copies in either it passes. +- [ ] **AC-3.13** Given a Japanese printing and an English printing of one card in a Standard deck, 3 and 2 copies When the deck is checked Then the finding names the card by its English name and says 5 copies. +- [ ] **AC-3.14** Given a printing with no card data (no extension record) in 5 copies in a Standard deck When the deck is checked Then it is a finding as any card is, and in a Commander deck a basic land with no card data in 2 copies is a finding too (decision 4). +- [ ] **AC-3.16** Given a basic land with two printings in a Standard deck, one with card data and one without, 5 copies in all When the deck is checked Then there is a finding at the limit of 4 (decision 1's mixed case). +- [ ] **AC-3.15** Given a card in 5 copies in Main of a Standard deck and in 5 more in Sideboard When the deck is checked Then there is one finding for the card, saying 10 copies. + +### Story 4: A card with owned copies and proxies in one zone + +**As a** collector who plans a deck with proxies +**I want** to see the cards that have both +**So that** I notice a proxy I forgot to swap for my own copy. + +**Acceptance criteria:** + +- [ ] **AC-4.1** Given a zone with 2 owned copies and 1 proxy of one printing When the deck is checked Then there is an Owned and proxy finding "Lightning Bolt in Main: 2 owned, 1 proxy." +- [ ] **AC-4.2** Given a zone with an owned copy of one printing and a proxy of another printing of the same card When the deck is checked Then it is flagged. +- [ ] **AC-4.3** Given an owned copy of a card in Main and its proxy in Considering When the deck is checked Then it is not flagged. +- [ ] **AC-4.4** Given a zone with only owned copies of a card, or only proxies of it When the deck is checked Then the card is not flagged. +- [ ] **AC-4.5** Given a Casual deck, or a deck whose format is not registered When it is checked Then the owned-plus-proxy result is shown. +- [ ] **AC-4.6** Given a card with owned copies and proxies in both Main and Sideboard When the deck is checked Then there is one finding per zone, each with that zone's split. +- [ ] **AC-4.7** Given a deck with no card in this state When it is checked Then the result passes with "No card has both owned copies and proxies in one zone." +- [ ] **AC-4.8** Given a finding When the collector has read it Then nothing has changed: the proxy, the lots and the zones are as before. +- [ ] **AC-4.9** Given a card of another collectible with owned copies and a proxy in one zone of a Magic deck When the deck is checked Then it is not flagged (decision 4). +- [ ] **AC-4.10** Given a card with owned copies and a proxy in a zone no rule names When the deck is checked Then it is flagged, naming that zone. + +### Story 5: What the checks read, and what they change + +**As a** self-hoster and a maintainer +**I want** the checks to cost nothing extra, keep to one account and add no data +**So that** a big deck doesn't slow the page and an upgrade needs nothing. + +**Acceptance criteria:** + +- [ ] **AC-5.1** Given decks of 5 rows and of 500 rows When each is checked Then the check request runs the same number of statements as the plain deck page of that deck (spec 069 AC-6.1's form), and `Deck::Page#zones` is as before. +- [ ] **AC-5.2** Given a visitor who is not signed in When the check is requested Then the response is the sign-in redirect the other deck pages give. +- [ ] **AC-5.3** Given the app after the feature When its schema, the export, the import and the backup are compared with before Then nothing has changed and the export format is still version 4. +- [ ] **AC-5.4** Given the core files of the checks (`app/models/catalog/deck_checks.rb` as a new pattern, and the check's controller and views, which the existing globs cover, so the guard's hand-counted total rises by them) When `spec/deck_core_spec.rb` reads them, comments included Then none names a collectible, a format or a zone key but `considering`, so no core comment uses a format's name as an example; and the guard's list and count include them. +- [ ] **AC-5.5** Given a collectible whose vocabulary has no `deck_rules` When a deck of it is checked Then the page says it has no rules to check, shows the owned-plus-proxy result, and nothing raises. + +### Story 6: The results are readable + +**As a** collector on a phone, in either theme +**I want** the results to be legible +**So that** I can use them where I use the deck. + +**Acceptance criteria:** + +- [ ] **AC-6.1** Given the results at 360px in the light and the dark theme When they are shown Then nothing overflows the page horizontally and no text is clipped. +- [ ] **AC-6.2** Given a result When it is a pass or a finding Then its status word says which (decision 6), so no result depends on colour alone. +- [ ] **AC-6.3** Given the results page When it loads Then the results section has the heading "Deck check", keyboard focus is on that heading, and the button and every result are reachable in reading order. +- [ ] **AC-6.4** Given a card name that is long, or a zone name that is long When it appears in a finding Then it wraps and nothing overlaps. + +### Story 7: The documents and the guards + +**As a** maintainer and an agent +**I want** the rules written where the next feature will look +**So that** nothing here is rediscovered from the code. + +**Acceptance criteria:** + +- [ ] **AC-7.1** Given the area doc `collection-and-lots.md` When the feature is distilled Then it states the checks' rules (decisions 1 to 10) with this spec as the source, and says that spec 069's non-goal "deck rules" is done in part by this spec (size and copy limits; "is this deck legal as a whole" stays out, Non-Goals). +- [ ] **AC-7.2** Given the specs index When the spec is written Then it has a row for 079 naming the area docs it changes and `docs/design-system/components/DeckPage.md`, and notes that it supersedes nothing. +- [ ] **AC-7.3** Given `Catalog::DeckChecks` and `MTG::Collecting.deck_rules` When they are written Then each public class, module and method has a summary comment, `@param` and `@return` where `.claude/rules/code-documentation.md` asks for them, the contract of decision 1 is `Catalog::DeckChecks`' comment, the comment ratchet and the YARD build pass, and the area doc points to the comment. +- [ ] **AC-7.4** Given `docs/design-system/components/DeckPage.md` When the feature is distilled Then it describes the head's Check deck link, the results section and its place, and that the page remains form-free. +- [ ] **AC-7.5** Given the roadmap When the feature merges Then its theme C entry lists #258 as shipped. +- [ ] **AC-7.6** Given `CLAUDE.md`'s Project Status paragraph When the feature is distilled Then it carries one clause for the deck checks (`Catalog::DeckChecks`, `MTG::Collecting.deck_rules`, spec 079), as earlier deck specs added theirs, and restates no rule of the area doc. + +## Functional Requirements + +### FR-1: The seam + +**Must:** +- Ask the collecting vocabulary for a format's rules through an optional `deck_rules` answer (decision 1); treat a missing method, a nil answer and an unregistered type as "no rules" and never raise for them. +- Run the rules, and the owned-plus-proxy check for every deck, over the zones `Deck::Page` already built. +- Keep the core free of any collectible, format and zone key but `considering`. + +**Must not:** +- Name a format or a zone in the core, read a record by an id from the request, or run a statement per row. + +### FR-2: The checks + +**Must:** +- Judge size, copy limits and owned plus proxy by decisions 3 to 5, counting owned copies and proxies together, by catalog identity, and leaving out Considering, unnamed zones for the size and copy rules, and cards of another collectible. +- Return, for each check, either a pass with the line that says what passed, or one finding per fault with the words decision 6 gives, in decision 10's order. + +**Must not:** +- Change any lot, proxy, zone or deck, or store a result. + +### FR-3: The request and the page + +**Must:** +- Show the Check deck link in the page head and, for the check request, the summary line and the results between the head and the zones (decisions 7, 8 and 10). +- Show no result on a request of the deck page that is not the check. + +**Must not:** +- Disable or hide any of the page's actions because of a finding, or add a form to the page. + +## Non-Functional Requirements + +### Performance + +- The deck page without a press runs the statements it runs today; the check request runs none beyond them, however many zones, rows and lots the deck has (AC-5.1). The identity of a card and the rule data of a printing come from the entries and extension records the page already loaded. + +### Security + +- The check is a request for one deck of the signed-in account: another account's deck is a 404 and the identities read are catalog data (global, no `account_id`). No `account_id` or deck id is taken from a form field. Names of cards and zones shown in a finding are escaped as everywhere. + +### Reliability + +- A missing rule, an unregistered format, a collectible no longer registered or a printing without card data never raises and never shows a partial result: it gives the "no rules to check" line, or counts the printing by its identity as decision 4 says. + +### Accessibility and design + +- The results use the design system's tokens and `c-*` classes only; any new pattern gets a component document (`CLAUDE.md`). Nothing depends on colour alone; the page reads in both themes at 360px; focus goes to the results heading on load. + +## Error Scenarios + +| Scenario | Expected Behavior | +|----------|-------------------| +| The deck's format isn't registered (a format removed from the instance) | No size or copy result; one line says the stored key has no rules to check; the owned-plus-proxy result is shown | +| The collectible's vocabulary has no `deck_rules`, or returns nil for the format | The same line, naming the format's shown name | +| A printing has no card data (not refreshed, no extension record) | It counts for size and for copies by its identity, is never exempt and never restricted (decision 4); the card's name in a finding is its English name (decision 6) | +| A card of another collectible in a Magic deck's zone | Left out of every check and named in no finding (decision 4) | +| The deck has no card anywhere | A format with a minimum finds 0 cards in the named zone; the other results pass | +| A zone no rule names holds cards (an unregistered format's zones, or a zone the format no longer lists) | Not counted for size or copies; judged for owned plus proxy | +| A collector requests the check of a deck that was deleted meanwhile | The usual not-found response, as for the deck page | +| The deck changes after the check was shown | Any later request of the deck page has no results (decision 7), and Back does not show the old ones | + +## Interactions and Supersession + +- **Testing form:** an acceptance criterion that names several cases ("or", "with 101 it is Over") is one example per case in an `:aggregate_failures` group, so a failure names its case. +- **Supersedes:** nothing. Spec 069 left "deck rules" as a non-goal ("the 2.0 milestone, issue #258"); this spec is part of that work (size and copy limits) and changes no sentence of spec 069 or of spec 066. +- **Area docs it will change** (at distillation): [`collection-and-lots.md`](../../architecture/collection-and-lots.md) (decks and the deck page) and [`DeckPage.md`](../../design-system/components/DeckPage.md) (the head's link and the results section); the design system also if a new pattern is added. The specs index gets its row now. +- **Leaves alone:** spec 066's per-card marker, spec 068's proxy rules, spec 069's panel and the export format. + +## Open Questions + +None. Decisions marked *(my call)* are for Gate A: decision 2 (the owned-plus-proxy check runs for every deck), the no-card-data rule of decision 4, decision 5's per-zone judging, decision 6's strings and ordering, and decision 10's summary line. + +## Out of Scope (Future Considerations) + +- Colour identity, banned and restricted cards beyond Vintage's one, the number of commanders and partner rules. +- A check that runs on every view, on save or in the background, or one that warns while editing. +- Collector-defined formats and rules (the issue design's item 16), and rules for other collectibles (they register `deck_rules` when they exist). +- A "fix it" action beside a finding, such as "Use my copy" (spec 070 already offers that from the row's menu). +- Storing results, a history of checks or including them in the export. diff --git a/docs/specs/README.md b/docs/specs/README.md index ad907e59..78865c85 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -94,3 +94,4 @@ A plan is deleted from the tree in a commit of its own after its feature's pull | [073 "Free to use" and "In decks" on the Card Page](073-card-page-deck-stats/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) and [catalog core](../architecture/catalog-core.md) (the card page's stats and "In decks" section; the visitor rule and the card page's order); the card page's markup is `ItemPage` in the [design system](../design-system/README.md) | [Collection and lots](../architecture/collection-and-lots.md), [catalog core](../architecture/catalog-core.md), [design system](../design-system/README.md) (`ItemPage`, `ItemPagePhone`, `SingleStat`) | retired in `1bdb1c62a908b76131f1eb2a94f769b419a78689`; `git show 1bdb1c62a908b76131f1eb2a94f769b419a78689^:docs/specs/073-card-page-deck-stats/plan.md` | In part 004 FR-9 (the single stat; no "In decks" or "Free to use"), its Non-Goal and Out of Scope entries and AC-8.1's order; 071 AC-1.4 and its "where #50 sits" note (fulfilled); fulfils the Non-Goals that name #50 (029, 032, 033, 057, 070) and 057 decisions 7 and 8 (the strip and the section) | | | [077 The Deck Shape Panel Stays Tidy at Extreme Counts](077-deck-shape-extreme-counts/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph and Named constants); the panel's markup is `DeckShape` in the [design system](../design-system/README.md) | [Design system](../design-system/README.md) (`DeckShape`), [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph, Named constants) | retired in `a54b85eeb63a0ac540bd38b88747c79a0c2d47b4`; `git show a54b85eeb63a0ac540bd38b88747c79a0c2d47b4^:docs/specs/077-deck-shape-extreme-counts/plan.md` | In part 060 decision 2, AC-2.2, AC-2.4, AC-2.5 and AC-2.6 (the visible numbers, the types' alignment) and 069 decision 14, its four-digit-count Error Scenarios row and Findings 1 to 4 ([#320](https://github.com/plainprogrammer/Collector/issues/320)) | In part by [078](078-mana-bar-label-fit/spec.md) (decision 2's documented limit: the mana-label band, [#330](https://github.com/plainprogrammer/Collector/issues/330)) | | [078 A Mana Bar Label Fits Its Segment or Isn't Drawn](078-mana-bar-label-fit/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph); the panel's markup and CSS rule are `DeckShape` in the [design system](../design-system/README.md) | [Design system](../design-system/README.md) (`DeckShape`), [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph) | retired in `b275dc7e20ef03355d825eec48569c2eb7ce8df0`; `git show b275dc7e20ef03355d825eec48569c2eb7ce8df0^:docs/specs/078-mana-bar-label-fit/plan.md` | In part 077 decision 2 (the mana-label band as a documented limit, ruled A on 2026-11-11), its Goal 1's qualification, Open Questions, Error Scenarios row for the band, AC-5.1 and AC-5.3, and its Supersedes row's exception for 060 AC-2.4 ([#330](https://github.com/plainprogrammer/Collector/issues/330)); 060 decision 4 and 069 decision 9 are kept | | +| [079 Deck Checks (Deck Size, Copy Limits, and Owned Plus Proxy of One Card)](079-deck-checks/spec.md) | Approved ([#258](https://github.com/plainprogrammer/Collector/issues/258)) | [Collection and lots](../architecture/collection-and-lots.md) (decks and the deck page), [DeckPage](../design-system/components/DeckPage.md) | present | Nothing: it does the "deck rules" that 069 left as a non-goal | | From b2ee0f105f75669f5560bc26d2605dd1c5220447 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 15:53:46 -0500 Subject: [PATCH 02/24] feat(decks): add Catalog::DeckChecks and its owned-plus-proxy check (#258) Adds the core seam of spec 079: the contract types (SizeRule, Limit, CopyRule, Rules, Line, Report), the extension contract in the module's comment, the no-rules line for a vocabulary, type or format with no rules, and the owned-plus-proxy check that runs for every deck. The deck core guard now covers the new file (+1). The ordering example drops the leading no-rules line before comparing: the plan's expected list left it out, though no rules are registered. Co-Authored-By: Claude Sonnet 5.5 --- app/models/catalog/deck_checks.rb | 109 ++++++++++++++++++++++++ spec/deck_core_spec.rb | 3 +- spec/models/catalog/deck_checks_spec.rb | 85 ++++++++++++++++++ 3 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 app/models/catalog/deck_checks.rb create mode 100644 spec/models/catalog/deck_checks_spec.rb diff --git a/app/models/catalog/deck_checks.rb b/app/models/catalog/deck_checks.rb new file mode 100644 index 00000000..9867a900 --- /dev/null +++ b/app/models/catalog/deck_checks.rb @@ -0,0 +1,109 @@ +# What a deck page's Check deck answers (spec 079): the deck's size and copy limits judged by the collectible's own rules, and +# every card with owned copies and proxies in one zone, which needs no rule and runs for every deck. Like +# `Catalog::DeckShape`, an optional answer on the collecting vocabulary (`Catalog.collecting_for`), so the core counts and +# words the results and never names a collectible, a format or a zone key but `Catalog::Formats::CONSIDERING`. +# +# An extension contract: a vocabulary that has rules for a format provides +# * `deck_rules(format_key)` returning {Rules}, or nil for a format it has no rules for (the deck's stored key, registered or not). +# * `Rules#sizes`: {SizeRule}s in the order they are shown. A rule holds the `zone_keys` it counts (the extension names them; +# the core matches each to a zone row's key) and one of three shapes: `min` only, `max` only, or `min == max` (an exact +# size); a range with two different bounds is not offered. The extension supplies no words. +# * `Rules#copies`: a {CopyRule} or nil. Its `zone_keys` are counted together; `limit_for` is called with one counted printing +# (a `Catalog::Entry` whose `extension` may be nil) and returns a {Limit} (`count`, and a `reason` sentence part such as +# "restricted to 1", or nil) or nil for a card that is never limited. For a printing with no extension record it returns +# the format's ordinary limit. It reads loaded records only, never a query. +# The core counts copies by catalog identity over every counted printing and holds a card to the lowest number its printings +# answer; nil is above every number, so a card is unlimited only when every counted printing answers nil, and the reason is +# the one given with the lowest number (the first printing in page order on a tie). A card of another collectible than the +# deck's is left out of every check. Considering and any zone no rule names are counted by no size or copy rule. +# A vocabulary without `deck_rules`, a nil answer, a type no longer registered and a deck whose format is no longer registered +# have no size or copy rules. +# @see file:collection-and-lots.md +module Catalog::DeckChecks + # One size rule: the zone keys it counts together and a minimum, a maximum, or both equal (an exact size). + SizeRule = Data.define(:zone_keys, :min, :max) + # A card's limit: the most copies a deck may hold, and an optional sentence part that says why ("restricted to 1"). + Limit = Data.define(:count, :reason) + # The copy rule: the zone keys counted together and a callable from a loaded printing to a {Limit} or nil. + CopyRule = Data.define(:zone_keys, :limit_for) + # What a vocabulary answers for a format: its size rules and its copy rule (nil for none). + Rules = Data.define(:sizes, :copies) + # One line of a report: a status word (nil for a line that is no check), its sentence, and whether it is a finding. + Line = Data.define(:status, :sentence, :fault) + # The results of one check request, in the order they are shown. + Report = Data.define(:lines) do + # The number of findings. + def faults = lines.count(&:fault) + + # The summary line: "3 findings", "1 finding" or "No findings". + def summary = faults.zero? ? "No findings" : "#{faults} #{faults == 1 ? 'finding' : 'findings'}" + end + # A row of the deck's collectible with its zone and the zone's position in page order. + # @api private + Counted = Data.define(:zone, :position, :row) + + # The report for a deck. + # + # @param deck_type [String] the deck's collectible type + # @param format_key [String] the deck's stored format key, registered or not + # @param zones [Array] `Deck::Page#zones`; read from memory, no statement runs + # @return [Report] never nil, and never raises for an unregistered type or format + def self.report(deck_type, format_key, zones) + counted = counted_rows(deck_type, zones) + rules = rules_for(deck_type, format_key) + lines = rules ? rule_lines(rules, deck_type, format_key, zones, counted) : [ no_rules_line(deck_type, format_key) ] + Report.new(lines: lines + split_lines(counted)) + end + + # @api private + def self.rules_for(deck_type, format_key) + return unless Catalog.collecting.key?(deck_type) + + vocabulary = Catalog.collecting_for(deck_type) + vocabulary.deck_rules(format_key) if vocabulary.respond_to?(:deck_rules) + end + private_class_method :rules_for + + # Every row of the deck's collectible with its zone's position; another collectible's rows are left out (decision 4). + # @api private + def self.counted_rows(deck_type, zones) + zones.each_with_index.flat_map do |zone, position| + zone.groups.flat_map(&:rows).select { it.entry.collectible_type == deck_type }.map { Counted.new(zone:, position:, row: it) } + end + end + private_class_method :counted_rows + + # @api private + def self.rule_lines(_rules, _deck_type, _format_key, _zones, _counted) = [] + private_class_method :rule_lines + + # @api private + def self.no_rules_line(deck_type, format_key) + Line.new(status: nil, sentence: "#{Catalog::Formats.name_for(deck_type, format_key)} has no rules to check.", fault: false) + end + private_class_method :no_rules_line + + # A count with delimiters ("1,234"), as the deck page's heads and spec 069's panel write one. + # @api private + def self.delimited(number) = ActiveSupport::NumberHelper.number_to_delimited(number) + private_class_method :delimited + + # The owned-plus-proxy check (decision 5): each zone alone, any printing of the card; findings ordered by the zone's + # position, then the card's name, then its identity; or one pass line. + # @api private + def self.split_lines(counted) + findings = counted.group_by { [ it.position, it.row.entry.catalog_identity_id ] }.filter_map do |(position, identity_id), cards| + owned = cards.sum { it.row.owned } + proxy = cards.sum { it.row.proxy } + next unless owned.positive? && proxy.positive? + + [ position, cards.first.row.entry.name, identity_id, cards.first.zone.name, owned, proxy ] + end + return [ Line.new(status: "Pass", sentence: "No card has both owned copies and proxies in one zone.", fault: false) ] if findings.empty? + + findings.sort_by { it.first(3) }.map do |_position, name, _identity_id, zone_name, owned, proxy| + Line.new(status: "Owned and proxy", sentence: "#{name} in #{zone_name}: #{delimited(owned)} owned, #{delimited(proxy)} proxy.", fault: true) + end + end + private_class_method :split_lines +end diff --git a/spec/deck_core_spec.rb b/spec/deck_core_spec.rb index bf05bba7..c00233a9 100644 --- a/spec/deck_core_spec.rb +++ b/spec/deck_core_spec.rb @@ -17,6 +17,7 @@ %w[ app/controllers/decks_controller.rb app/controllers/decks/*.rb app/views/decks/**/*.erb app/models/deck.rb app/models/deck/*.rb app/models/location/placement.rb app/models/catalog/formats.rb app/models/catalog/deck_cards.rb app/models/catalog/deck_shape.rb + app/models/catalog/deck_checks.rb app/helpers/decks_helper.rb app/models/collection_import/decks.rb app/models/collection_import/deck_changes.rb app/views/collections/import_previews/_decks.html.erb app/views/icons/_decks.html.erb app/models/location/choices.rb app/helpers/location_choices_helper.rb @@ -36,7 +37,7 @@ def naming?(text) = text.match?(game_names) || text.match?(format_and_zone_keys) def relative(files) = files.map { |file| file.relative_path_from(Rails.root).to_s } it "covers every file of the feature" do - expect(files.size).to eq(20 + 6 + 1 + 1 + 4 + 1 + 15 + 2 + 6 + 5 + 1 + 3) # spec 069 adds the seam `app/models/catalog/deck_shape.rb` and the page's frame `app/views/decks/_shape.html.erb` (the views glob already covers it); spec 064 brought the list to 20; spec 065 adds the choices model, its helper and the four picker views; spec 067 adds the selections controller (the glob already covers it); spec 066 adds Deck::Page and the deck page's four views (the `app/models/deck/*.rb` and `app/views/decks/**/*.erb` globs already cover them) and the seam `app/models/catalog/deck_cards.rb`; spec 068 adds 15: `deck/proxy.rb` and the proxies controller (the globs already cover them), the six proxy views under `app/views/decks`, `decks/proxies/removals_controller.rb`, `deck/proxy/addition.rb` and `cap_exceeded.rb`, `finds_deck.rb`, the import's `proxies.rb` and `proxy_rows.rb`, and the preview's `_proxies.html.erb`; spec 070 adds 6: the replacements and copies controllers, UseCopies and AddCopy, and the two new views (the globs already cover them); spec 071 adds 5: the controller, its view, `Lot::FreeCopies` and the helper (new patterns) and `deck/addition.rb` (the `app/models/deck/*.rb` glob already covers it); spec 073 adds 1: `deck/card_uses.rb`, covered by the `app/models/deck/*.rb` glob; spec 073 adds 3 here: `catalog/card_overview.rb` and the card page's `_stats` and `_decks` partials (new patterns; the page's `show.html.erb` is not listed, because its `scryfall` link variable names a source) + expect(files.size).to eq(20 + 6 + 1 + 1 + 4 + 1 + 15 + 2 + 6 + 5 + 1 + 3 + 1) # spec 069 adds the seam `app/models/catalog/deck_shape.rb` and the page's frame `app/views/decks/_shape.html.erb` (the views glob already covers it); spec 064 brought the list to 20; spec 065 adds the choices model, its helper and the four picker views; spec 067 adds the selections controller (the glob already covers it); spec 066 adds Deck::Page and the deck page's four views (the `app/models/deck/*.rb` and `app/views/decks/**/*.erb` globs already cover them) and the seam `app/models/catalog/deck_cards.rb`; spec 068 adds 15: `deck/proxy.rb` and the proxies controller (the globs already cover them), the six proxy views under `app/views/decks`, `decks/proxies/removals_controller.rb`, `deck/proxy/addition.rb` and `cap_exceeded.rb`, `finds_deck.rb`, the import's `proxies.rb` and `proxy_rows.rb`, and the preview's `_proxies.html.erb`; spec 070 adds 6: the replacements and copies controllers, UseCopies and AddCopy, and the two new views (the globs already cover them); spec 071 adds 5: the controller, its view, `Lot::FreeCopies` and the helper (new patterns) and `deck/addition.rb` (the `app/models/deck/*.rb` glob already covers it); spec 073 adds 1: `deck/card_uses.rb`, covered by the `app/models/deck/*.rb` glob; spec 073 adds 3 here: `catalog/card_overview.rb` and the card page's `_stats` and `_decks` partials (new patterns; the page's `show.html.erb` is not listed, because its `scryfall` link variable names a source); spec 079 adds the seam app/models/catalog/deck_checks.rb (a new pattern) end it "names every file it lists, each of which exists" do diff --git a/spec/models/catalog/deck_checks_spec.rb b/spec/models/catalog/deck_checks_spec.rb new file mode 100644 index 00000000..c8c8416d --- /dev/null +++ b/spec/models/catalog/deck_checks_spec.rb @@ -0,0 +1,85 @@ +require "rails_helper" + +RSpec.describe Catalog::DeckChecks, type: :model do + def zone(key, name, *rows) + location = Struct.new(:zone, :name).new(key, name) + Deck::Page::Zone.new(location:, groups: rows.empty? ? [] : [ Deck::Page::Group.new(name: nil, rows:) ]) + end + + def card(name, id, type: "game") + build_stubbed(:catalog_entry, identity: build_stubbed(:catalog_identity, id:, name:), name:, collectible_type: type) + end + + def row(entry, owned: 0, proxy: 0) + lines = proxy.zero? ? [] : [ Deck::Page::Line.new(id: 1, finish: "nonfoil", quantity: proxy) ] + Deck::Page::Row.new(entry:, owned:, lines:, answer: Catalog::DeckCards::NONE) + end + + def size(keys, min: nil, max: nil) = Catalog::DeckChecks::SizeRule.new(zone_keys: keys, min:, max:) + def limit(count, reason = nil) = ->(_entry) { Catalog::DeckChecks::Limit.new(count:, reason:) } + def copies(keys, limit_for) = Catalog::DeckChecks::CopyRule.new(zone_keys: keys, limit_for:) + + def register(sizes: [], copies: nil) + rules = Catalog::DeckChecks::Rules.new(sizes:, copies:) + stub_const("GameVocabulary", Module.new { define_singleton_method(:deck_rules) { |_key| rules } }) + Catalog.collecting["game"] = "GameVocabulary" + end + + def report(*zones) = described_class.report("game", "tight", zones) + def lines(report) = report.lines.map { [ it.status, it.sentence ] } + + after { Catalog.collecting.delete("game") } + + describe ".report with no rules" do + it "says the format has no rules and still runs owned-plus-proxy", :aggregate_failures do + result = described_class.report("game", "tight", [ zone("main", "Main", row(card("Bolt", 1), owned: 2, proxy: 1)) ]) + expect(lines(result)).to eq([ [ nil, "tight has no rules to check." ], [ "Owned and proxy", "Bolt in Main: 2 owned, 1 proxy." ] ]) + expect([ result.lines.first.fault, result.summary ]).to eq([ false, "1 finding" ]) + end + + it "treats a missing method, a nil answer and an unregistered type alike (AC-5.5)", :aggregate_failures do + stub_const("SilentVocabulary", Module.new) + stub_const("NilVocabulary", Module.new { def self.deck_rules(_key) = nil }) + expected = [ [ nil, "tight has no rules to check." ], [ "Pass", "No card has both owned copies and proxies in one zone." ] ] + %w[SilentVocabulary NilVocabulary].each do |vocabulary| + Catalog.collecting["game"] = vocabulary + expect(lines(described_class.report("game", "tight", []))).to eq(expected) + end + expect(described_class.report("gone", "tight", []).summary).to eq("No findings") + end + end + + describe "owned-plus-proxy results" do + let(:bolt) { card("Lightning Bolt", 1) } + + it "flags a zone that holds both, across printings, naming the split", :aggregate_failures do + result = report(zone("main", "Main", row(bolt, owned: 2), row(card("Lightning Bolt", 1), proxy: 1))) + expect(lines(result)).to include([ "Owned and proxy", "Lightning Bolt in Main: 2 owned, 1 proxy." ]) + expect(result.faults).to eq(1) + end + + it "judges each zone alone, includes zones no rule names, and skips Considering's lone proxy", :aggregate_failures do + zones = [ zone("main", "Main", row(bolt, owned: 1), row(bolt, proxy: 1)), zone("stash", "Stash", row(bolt, owned: 2, proxy: 3)), + zone("considering", "Considering", row(bolt, proxy: 1)) ] + sentences = lines(report(*zones)).map(&:last) + expect(sentences).to include("Lightning Bolt in Main: 1 owned, 1 proxy.", "Lightning Bolt in Stash: 2 owned, 3 proxy.") + expect(sentences).not_to include(a_string_matching(/Considering/)) + end + + it "does not flag owned only, proxy only, owned in one zone with a proxy in another, or another collectible", :aggregate_failures do + stray = card("Other", 9, type: "stranger") + zones = [ zone("main", "Main", row(bolt, owned: 2)), zone("considering", "Considering", row(bolt, proxy: 1), row(stray, owned: 1, proxy: 1)) ] + expect(lines(report(*zones)).last).to eq([ "Pass", "No card has both owned copies and proxies in one zone." ]) + end + + it "delimits the split's numbers as the deck page's heads do", :aggregate_failures do + result = report(zone("main", "Main", row(bolt, owned: 1_234, proxy: 5_678))) + expect(lines(result)).to include([ "Owned and proxy", "Lightning Bolt in Main: 1,234 owned, 5,678 proxy." ]) + end + + it "orders findings by zone, then name, then identity", :aggregate_failures do + zones = [ zone("a", "Alpha", row(card("Zed", 1), owned: 1, proxy: 1), row(card("Abe", 2), owned: 1, proxy: 1)), zone("b", "Beta", row(card("Abe", 2), owned: 1, proxy: 1)) ] + expect(lines(report(*zones)).drop(1).map(&:last)).to eq([ "Abe in Alpha: 1 owned, 1 proxy.", "Zed in Alpha: 1 owned, 1 proxy.", "Abe in Beta: 1 owned, 1 proxy." ]) + end + end +end From 5362d86671fbaaed69f8d4a8100d26ff7f3a43fe Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 15:54:31 -0500 Subject: [PATCH 03/24] feat(decks): size results for the deck checks (#258) Judges each size rule a vocabulary answers: the deck's collectible's cards in the rule's zones, owned plus proxy, against a minimum, a maximum or an exact size, worded as Short, Over or Pass with the true count. Zones are named in the rule's key order, by the zone's shown name, the format's registration or the key. Co-Authored-By: Claude Sonnet 5.5 --- app/models/catalog/deck_checks.rb | 37 ++++++++++++++++++++++- spec/models/catalog/deck_checks_spec.rb | 40 +++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/app/models/catalog/deck_checks.rb b/app/models/catalog/deck_checks.rb index 9867a900..f5a9e17c 100644 --- a/app/models/catalog/deck_checks.rb +++ b/app/models/catalog/deck_checks.rb @@ -74,9 +74,44 @@ def self.counted_rows(deck_type, zones) private_class_method :counted_rows # @api private - def self.rule_lines(_rules, _deck_type, _format_key, _zones, _counted) = [] + def self.rule_lines(rules, deck_type, format_key, zones, counted) + rules.sizes.map { size_line(it, deck_type, format_key, zones, counted) } + end private_class_method :rule_lines + # One size result (decision 6). Zones are named in the rule's key order: by the zone row's shown name, else by the format's + # registration, else by the key. + # @api private + def self.size_line(rule, deck_type, format_key, zones, counted) + total = counted.select { rule.zone_keys.include?(it.zone.location.zone) }.sum { it.row.total } + subject = rule.zone_keys.map { zone_name(it, deck_type, format_key, zones) }.join(" + ") + status = if rule.min && total < rule.min then "Short" elsif rule.max && total > rule.max then "Over" else "Pass" end + format_name = Catalog::Formats.name_for(deck_type, format_key) + Line.new(status:, sentence: "#{subject}: #{counted_words(total, 'card')}; #{format_name} #{size_need(rule)}.", fault: status != "Pass") + end + private_class_method :size_line + + # The rule's requirement in words: "needs at least 60", "allows at most 15" or "needs exactly 100". + # @api private + def self.size_need(rule) + return "needs exactly #{delimited(rule.min)}" if rule.min && rule.min == rule.max + return "needs at least #{delimited(rule.min)}" if rule.min + + "allows at most #{delimited(rule.max)}" + end + private_class_method :size_need + + # @api private + def self.zone_name(key, deck_type, format_key, zones) + zones.find { it.location.zone == key }&.name || Catalog::Formats.zones_for(deck_type, format_key)&.find { it.key == key }&.name || key + end + private_class_method :zone_name + + # "1 card", "5 copies": the count with delimiters and the singular for one. + # @api private + def self.counted_words(number, word) = "#{delimited(number)} #{word.pluralize(number)}" + private_class_method :counted_words + # @api private def self.no_rules_line(deck_type, format_key) Line.new(status: nil, sentence: "#{Catalog::Formats.name_for(deck_type, format_key)} has no rules to check.", fault: false) diff --git a/spec/models/catalog/deck_checks_spec.rb b/spec/models/catalog/deck_checks_spec.rb index c8c8416d..ed9b9ee4 100644 --- a/spec/models/catalog/deck_checks_spec.rb +++ b/spec/models/catalog/deck_checks_spec.rb @@ -82,4 +82,44 @@ def lines(report) = report.lines.map { [ it.status, it.sentence ] } expect(lines(report(*zones)).drop(1).map(&:last)).to eq([ "Abe in Alpha: 1 owned, 1 proxy.", "Zed in Alpha: 1 owned, 1 proxy.", "Abe in Beta: 1 owned, 1 proxy." ]) end end + + describe "size results" do + it "judges a minimum with the true count in every sentence", :aggregate_failures do + register(sizes: [ size(%w[main], min: 60) ]) + bolt = card("Bolt", 1) + expect(lines(report(zone("main", "Main", row(bolt, owned: 58))))).to include([ "Short", "Main: 58 cards; tight needs at least 60." ]) + expect(lines(report(zone("main", "Main", row(bolt, owned: 59, proxy: 1))))).to include([ "Pass", "Main: 60 cards; tight needs at least 60." ]) + expect(lines(report(zone("main", "Main", row(bolt, owned: 61))))).to include([ "Pass", "Main: 61 cards; tight needs at least 60." ]) + end + + it "names a rule's zones in its key order and a zone the deck lacks by its key", :aggregate_failures do + register(sizes: [ size(%w[bench], max: 15), size(%w[lead main], min: 100, max: 100) ]) + zones = [ zone("main", "Main", row(card("A", 1), owned: 98)), zone("lead", "Lead", row(card("B", 2), owned: 1)) ] + expect(lines(report(*zones))).to start_with([ "Pass", "bench: 0 cards; tight allows at most 15." ], [ "Short", "Lead + Main: 99 cards; tight needs exactly 100." ]) + end + + it "says Over past a maximum and past an exact size, and the singular for one card", :aggregate_failures do + register(sizes: [ size(%w[bench], max: 15), size(%w[main], min: 1, max: 1) ]) + zones = [ zone("bench", "Bench", row(card("A", 1), owned: 16)), zone("main", "Main", row(card("B", 2), owned: 2)) ] + expect(lines(report(*zones))).to start_with([ "Over", "Bench: 16 cards; tight allows at most 15." ], [ "Over", "Main: 2 cards; tight needs exactly 1." ]) + expect(lines(report(zone("main", "Main", row(card("B", 2), owned: 1))))).to include([ "Pass", "Main: 1 card; tight needs exactly 1." ]) + end + + it "counts nothing in Considering or an unnamed zone, and passes at exactly the exact size", :aggregate_failures do + register(sizes: [ size(%w[main], min: 100, max: 100) ]) + zones = [ zone("main", "Main", row(card("A", 1), owned: 100)), zone("considering", "Considering", row(card("B", 2), owned: 30)) ] + expect(lines(report(*zones)).first).to eq([ "Pass", "Main: 100 cards; tight needs exactly 100." ]) + end + + it "separates thousands in a count" do + register(sizes: [ size(%w[main], min: 60) ]) + expect(lines(report(zone("main", "Main", row(card("A", 1), owned: 1_234)))).first.last).to eq("Main: 1,234 cards; tight needs at least 60.") + end + + it "leaves out another collectible's cards and zones no rule names", :aggregate_failures do + register(sizes: [ size(%w[main], min: 1) ]) + zones = [ zone("main", "Main", row(card("Other", 9, type: "stranger"), owned: 5)), zone("stash", "Stash", row(card("A", 1), owned: 7)) ] + expect(lines(report(*zones)).first).to eq([ "Short", "Main: 0 cards; tight needs at least 1." ]) + end + end end From afd799f341ffae4fed3369c4f361324397308e78 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 15:55:19 -0500 Subject: [PATCH 04/24] feat(decks): copy-limit results for the deck checks (#258) Adds the copy check after the size results: copies of a card counted by catalog identity over the rule's zones, owned plus proxy, held to the lowest limit its printings answer (nil above every number, the first printing in page order on a tie), one Over the limit finding per card in zone, name and identity order, or one pass line. Co-Authored-By: Claude Sonnet 5.5 --- app/models/catalog/deck_checks.rb | 24 +++++++++- spec/models/catalog/deck_checks_spec.rb | 64 +++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/app/models/catalog/deck_checks.rb b/app/models/catalog/deck_checks.rb index f5a9e17c..7cad33c5 100644 --- a/app/models/catalog/deck_checks.rb +++ b/app/models/catalog/deck_checks.rb @@ -75,10 +75,32 @@ def self.counted_rows(deck_type, zones) # @api private def self.rule_lines(rules, deck_type, format_key, zones, counted) - rules.sizes.map { size_line(it, deck_type, format_key, zones, counted) } + sizes = rules.sizes.map { size_line(it, deck_type, format_key, zones, counted) } + sizes + (rules.copies ? copy_lines(rules.copies, counted) : []) end private_class_method :rule_lines + # The copy check (decisions 1, 3, 4): copies by catalog identity over the rule's zones, owned plus proxy; one finding per + # card over its limit, ordered by the first zone that holds it, then its name, then its identity; or one pass line. A card + # is held to the lowest number its printings answer; `min_by` keeps the first printing in page order on a tie. + # @api private + def self.copy_lines(rule, counted) + in_rule = counted.select { rule.zone_keys.include?(it.zone.location.zone) } + findings = in_rule.group_by { it.row.entry.catalog_identity_id }.filter_map do |identity_id, cards| + copies = cards.sum { it.row.total } + limit = cards.map { it.row.entry }.uniq.filter_map { rule.limit_for.call(it) }.min_by(&:count) + next if limit.nil? || copies <= limit.count + + [ cards.map(&:position).min, cards.first.row.entry.name, identity_id, copies, limit ] + end + return [ Line.new(status: "Pass", sentence: "No card is over its limit.", fault: false) ] if findings.empty? + + findings.sort_by { it.first(3) }.map do |_position, name, _identity_id, copies, limit| + Line.new(status: "Over the limit", sentence: "#{name}: #{counted_words(copies, 'copy')}; #{limit.reason || "the limit is #{limit.count}"}.", fault: true) + end + end + private_class_method :copy_lines + # One size result (decision 6). Zones are named in the rule's key order: by the zone row's shown name, else by the format's # registration, else by the key. # @api private diff --git a/spec/models/catalog/deck_checks_spec.rb b/spec/models/catalog/deck_checks_spec.rb index ed9b9ee4..9cf2992f 100644 --- a/spec/models/catalog/deck_checks_spec.rb +++ b/spec/models/catalog/deck_checks_spec.rb @@ -122,4 +122,68 @@ def lines(report) = report.lines.map { [ it.status, it.sentence ] } expect(lines(report(*zones)).first).to eq([ "Short", "Main: 0 cards; tight needs at least 1." ]) end end + + describe "copy results" do + let(:bolt) { card("Lightning Bolt", 1) } + + it "names a card over its limit across zones, printings and owned plus proxy", :aggregate_failures do + register(copies: copies(%w[main bench], limit(4))) + zones = [ zone("main", "Main", row(bolt, owned: 3), row(card("Lightning Bolt", 1), proxy: 1)), zone("bench", "Bench", row(bolt, owned: 1)) ] + expect(lines(report(*zones))).to include([ "Over the limit", "Lightning Bolt: 5 copies; the limit is 4." ]) + end + + it "passes at the limit and counts no copy in an unnamed zone", :aggregate_failures do + register(copies: copies(%w[main], limit(4))) + zones = [ zone("main", "Main", row(bolt, owned: 4)), zone("considering", "Considering", row(bolt, owned: 9)) ] + expect(lines(report(*zones))).to include([ "Pass", "No card is over its limit." ]) + end + + it "passes at the limit across two named zones (4 of 3 + 1)" do + register(copies: copies(%w[main bench], limit(4))) + zones = [ zone("main", "Main", row(bolt, owned: 3)), zone("bench", "Bench", row(bolt, owned: 1)) ] + expect(lines(report(*zones))).to include([ "Pass", "No card is over its limit." ]) + end + + it "breaks a tie on the lowest limit by the first printing in page order", :aggregate_failures do + first = card("Black Lotus", 2) + second = card("Black Lotus", 2) + answer = ->(entry) { Catalog::DeckChecks::Limit.new(count: 1, reason: entry.equal?(first) ? "restricted to 1" : "banned past 1") } + register(copies: copies(%w[main], answer)) + expect(lines(report(zone("main", "Main", row(second, owned: 1), row(first, owned: 1))))).to include([ "Over the limit", "Black Lotus: 2 copies; banned past 1." ]) + end + + it "orders two findings of one zone by name, then by identity when the names match", :aggregate_failures do + register(copies: copies(%w[main], limit(0))) + zones = [ zone("main", "Main", row(card("Zed", 1), owned: 1), row(card("Abe", 5), owned: 2), row(card("Abe", 2), owned: 1)) ] + expect(lines(report(*zones)).first(3).map(&:last)).to eq([ "Abe: 1 copy; the limit is 0.", "Abe: 2 copies; the limit is 0.", "Zed: 1 copy; the limit is 0." ]) + end + + it "holds a card to the lowest limit its printings answer, with that reason", :aggregate_failures do + one = card("Black Lotus", 2) + two = card("Black Lotus", 2) + answer = ->(entry) { entry.equal?(one) ? Catalog::DeckChecks::Limit.new(count: 4, reason: nil) : Catalog::DeckChecks::Limit.new(count: 1, reason: "restricted to 1") } + register(copies: copies(%w[main], answer)) + expect(lines(report(zone("main", "Main", row(one, owned: 1), row(two, owned: 1))))).to include([ "Over the limit", "Black Lotus: 2 copies; restricted to 1." ]) + end + + it "treats nil as above every number: unlimited only when every printing says nil", :aggregate_failures do + free = card("Plains", 3) + mixed = card("Plains", 3) + register(copies: copies(%w[main], ->(entry) { Catalog::DeckChecks::Limit.new(count: 4, reason: nil) unless entry.equal?(free) })) + expect(lines(report(zone("main", "Main", row(free, owned: 20))))).to include([ "Pass", "No card is over its limit." ]) + expect(lines(report(zone("main", "Main", row(free, owned: 3), row(mixed, owned: 2))))).to include([ "Over the limit", "Plains: 5 copies; the limit is 4." ]) + end + + it "orders findings by first zone, then name, then identity; leaves out another collectible", :aggregate_failures do + register(copies: copies(%w[main bench], limit(1))) + zones = [ zone("main", "Main", row(card("Zed", 1), owned: 2)), zone("bench", "Bench", row(card("Abe", 2), owned: 2), row(card("Other", 3, type: "stranger"), owned: 9)) ] + expect(lines(report(*zones)).map(&:last)).to start_with("Zed: 2 copies; the limit is 1.", "Abe: 2 copies; the limit is 1.") + expect(lines(report(*zones)).map(&:last)).not_to include(a_string_matching(/Other/)) + end + + it "says the singular for one copy" do + register(copies: copies(%w[main], limit(0))) + expect(lines(report(zone("main", "Main", row(bolt, owned: 1))))).to include([ "Over the limit", "Lightning Bolt: 1 copy; the limit is 0." ]) + end + end end From 7988bb6d81897edb31fe3b07b9e3642226954089 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 15:57:14 -0500 Subject: [PATCH 05/24] feat(decks): Magic's deck rules for the deck checks (#258) MTG::DeckRules answers each format's size and copy rules and each printing's limit (basic lands and any-number text exempt, a stated number, Vintage's restriction, else the ordinary limit), read from the loaded printing only. MTG::Collecting.deck_rules is the vocabulary answer Catalog::DeckChecks asks for. Co-Authored-By: Claude Sonnet 5.5 --- app/models/mtg/collecting.rb | 7 +++ app/models/mtg/deck_rules.rb | 80 ++++++++++++++++++++++++++++++ spec/models/mtg/deck_rules_spec.rb | 62 +++++++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 app/models/mtg/deck_rules.rb create mode 100644 spec/models/mtg/deck_rules_spec.rb diff --git a/app/models/mtg/collecting.rb b/app/models/mtg/collecting.rb index 0c294d65..237dcbce 100644 --- a/app/models/mtg/collecting.rb +++ b/app/models/mtg/collecting.rb @@ -69,6 +69,13 @@ def self.deck_shape(zones) Catalog::DeckShape::Answer.new(partial: "mtg/deck_shapes/parts", figures:, scope: figures.scope, facts: figures.facts) end + # The deck checks' rules for one format (spec 079, the extension side of `Catalog::DeckChecks`): size and copy limits. + # See `MTG::DeckRules`. + # + # @param format_key [String] the deck's stored format key + # @return [Catalog::DeckChecks::Rules, nil] nil for casual and for a key with no rules + def self.deck_rules(format_key) = MTG::DeckRules.for(format_key.to_s) + # A printing with no extension record or no face data is not judged: it has no card data to be a legal or a banned card, # so it gets no marker (spec 066 AC-6.6). # @api private diff --git a/app/models/mtg/deck_rules.rb b/app/models/mtg/deck_rules.rb new file mode 100644 index 00000000..209c252c --- /dev/null +++ b/app/models/mtg/deck_rules.rb @@ -0,0 +1,80 @@ +# Magic's deck rules for the deck checks (spec 079 decision 3, the extension side of `Catalog::DeckChecks`): size and copy +# limits per format, and each printing's limit read from its first face's type line, its rules text and its legalities. +# Reads the loaded printing record only, so no query runs per card. +class MTG::DeckRules + # The formats that share one rule set: the main zone's minimum, the sideboard's maximum and a limit of four. + CONSTRUCTED = %w[standard pioneer modern legacy vintage pauper].freeze + # A constructed deck holds at least this many cards in its main zone (spec 079 decision 3). + MAIN_MINIMUM = 60 + # A constructed deck's sideboard holds at most this many cards. + SIDEBOARD_MAXIMUM = 15 + # The copies of one card a constructed deck may hold (a basic land and the cards that say otherwise excepted). + CONSTRUCTED_COPIES = 4 + # The copies of one card a Commander deck may hold: the singleton rule. + COMMANDER_COPIES = 1 + # A Commander deck's commander and main zones hold exactly this many cards together. + COMMANDER_SIZE = 100 + # Rules text of a card a deck may hold any number of: Scryfall's own words (confirmed in verification.md, T0.2). + ANY_NUMBER = /a deck can have any number of cards named/i + # Rules text of a card a deck may hold up to a stated number of; the number is the word after "up to". + STATED_NUMBER = /a deck can have up to (\w+) cards named/i + # The number words the stated limits use (Seven Dwarves: seven; Nazgûl: nine). + NUMBER_WORDS = %w[two three four five six seven eight nine ten].each_with_index.to_h { |word, index| [ word, index + 2 ] }.freeze + + # The rules of one format. + # + # @param format_key [String] the deck's stored format key + # @return [Catalog::DeckChecks::Rules, nil] nil for casual and for a key the collectible has no rules for + def self.for(format_key) + return commander if format_key == "commander" + + constructed(format_key) if CONSTRUCTED.include?(format_key) + end + + # A printing's limit in a format. Order matters: a basic land and any-number text are exempt first, then a stated number, + # then Vintage's restriction, then the ordinary limit. A printing with no card data is never exempt and never restricted. + # + # @param entry [Catalog::Entry] a printing, its `extension` loaded or nil + # @param format_key [String] + # @param ordinary [Integer] the format's ordinary limit + # @return [Catalog::DeckChecks::Limit, nil] nil for a card that is never limited + def self.limit_for(entry, format_key, ordinary) + printing = entry.extension + return Catalog::DeckChecks::Limit.new(count: ordinary, reason: nil) if printing.nil? + + faces = Array(printing.faces) + return if basic_land?(faces.first) + + text = faces.filter_map { it["oracle_text"] }.join("\n") + return if text.match?(ANY_NUMBER) + + stated = NUMBER_WORDS[text[STATED_NUMBER, 1].to_s.downcase] + return Catalog::DeckChecks::Limit.new(count: stated, reason: nil) if stated + return Catalog::DeckChecks::Limit.new(count: 1, reason: "restricted to 1") if format_key == "vintage" && printing.legalities.to_h["vintage"] == "restricted" + + Catalog::DeckChecks::Limit.new(count: ordinary, reason: nil) + end + + # @api private + def self.basic_land?(face) = face&.dig("type_line").to_s.split("—").first.to_s.split.include?("Basic") + private_class_method :basic_land? + + # @api private + def self.constructed(format_key) + Catalog::DeckChecks::Rules.new( + sizes: [ Catalog::DeckChecks::SizeRule.new(zone_keys: %w[main], min: MAIN_MINIMUM, max: nil), + Catalog::DeckChecks::SizeRule.new(zone_keys: %w[sideboard], min: nil, max: SIDEBOARD_MAXIMUM) ], + copies: Catalog::DeckChecks::CopyRule.new(zone_keys: %w[main sideboard], limit_for: ->(entry) { limit_for(entry, format_key, CONSTRUCTED_COPIES) }) + ) + end + private_class_method :constructed + + # @api private + def self.commander + Catalog::DeckChecks::Rules.new( + sizes: [ Catalog::DeckChecks::SizeRule.new(zone_keys: %w[commander main], min: COMMANDER_SIZE, max: COMMANDER_SIZE) ], + copies: Catalog::DeckChecks::CopyRule.new(zone_keys: %w[commander main], limit_for: ->(entry) { limit_for(entry, "commander", COMMANDER_COPIES) }) + ) + end + private_class_method :commander +end diff --git a/spec/models/mtg/deck_rules_spec.rb b/spec/models/mtg/deck_rules_spec.rb new file mode 100644 index 00000000..bc691e7f --- /dev/null +++ b/spec/models/mtg/deck_rules_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +RSpec.describe MTG::DeckRules, type: :model do + def entry(name = "Card", type_line: "Instant", text: nil, legalities: {}, record: true) + entry = create(:catalog_entry, name:) + create(:mtg_printing, entry:, legalities:, faces: [ { "name" => name, "type_line" => type_line, "oracle_text" => text, "image_uris" => {} } ]) if record + Catalog::Entry.preload_extensions([ entry ]).first + end + + def limit(format, entry) = described_class.for(format).copies.limit_for.call(entry)&.then { [ it.count, it.reason ] } + def shape(rule) = [ rule.zone_keys, rule.min, rule.max ] + + describe ".for" do + it "has size and copy rules for each constructed format", :aggregate_failures do + %w[standard pioneer modern legacy vintage pauper].each do |key| + rules = described_class.for(key) + expect(rules.sizes.map { shape(it) }).to eq([ [ %w[main], 60, nil ], [ %w[sideboard], nil, 15 ] ]) + expect(rules.copies.zone_keys).to eq(%w[main sideboard]) + end + end + + it "has Commander's exact size, a singleton copy rule, and none for casual or an unknown key", :aggregate_failures do + commander = described_class.for("commander") + expect(commander.sizes.map { shape(it) }).to eq([ [ %w[commander main], 100, 100 ] ]) + expect(commander.copies.zone_keys).to eq(%w[commander main]) + expect([ described_class.for("casual"), described_class.for("retired"), MTG::Collecting.deck_rules("casual") ]).to all(be_nil) + end + end + + describe "a card's limit" do + it "is 4 in constructed formats and 1 in Commander", :aggregate_failures do + bolt = entry("Lightning Bolt") + expect([ limit("standard", bolt), limit("pauper", bolt), limit("vintage", bolt), limit("commander", bolt) ]).to eq([ [ 4, nil ], [ 4, nil ], [ 4, nil ], [ 1, nil ] ]) + end + + it "is none for a basic land, Snow-Covered ones included", :aggregate_failures do + plains = entry("Plains", type_line: "Basic Land — Plains") + snow = entry("Snow-Covered Island", type_line: "Basic Snow Land — Island") + expect([ limit("standard", plains), limit("commander", snow) ]).to all(be_nil) + end + + it "is none for any-number text, in every format", :aggregate_failures do + rats = entry("Relentless Rats", type_line: "Creature — Rat", text: "Relentless Rats gets +1/+1 for each other creature you control named Relentless Rats.\nA deck can have any number of cards named Relentless Rats.") + expect([ limit("modern", rats), limit("commander", rats) ]).to all(be_nil) + end + + it "is the stated number for up-to text, in every format", :aggregate_failures do + dwarves = entry("Seven Dwarves", type_line: "Creature — Dwarf", text: "Seven Dwarves gets +1/+1 for each other creature named Seven Dwarves you control.\nA deck can have up to seven cards named Seven Dwarves.") + expect([ limit("standard", dwarves), limit("commander", dwarves) ]).to eq([ [ 7, nil ], [ 7, nil ] ]) + end + + it "is 1, restricted, in Vintage only", :aggregate_failures do + lotus = entry("Black Lotus", type_line: "Artifact", legalities: { "vintage" => "restricted", "legacy" => "banned" }) + expect([ limit("vintage", lotus), limit("legacy", lotus) ]).to eq([ [ 1, "restricted to 1" ], [ 4, nil ] ]) + end + + it "is the ordinary limit for a printing with no card data, a basic land included", :aggregate_failures do + bare = entry("Plains", record: false) + expect([ limit("standard", bare), limit("commander", bare) ]).to eq([ [ 4, nil ], [ 1, nil ] ]) + end + end +end From eb7156f4af208cf73447a0b3fb8ebb6be30a49a7 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 15:58:27 -0500 Subject: [PATCH 06/24] test(decks): Magic deck checks end to end through the seam (#258) Real Magic decks through Deck::Page and Catalog::DeckChecks.report: size per constructed format and Commander, the no-rules line, copies across printings, zones and proxies, the English name, basic-land, any-number, stated-number and Vintage limits, and another collectible left out. These are characterisations of behaviour T2.1 delivers; they passed on first run. Mutation proof: MAIN_MINIMUM set to 61 failed the three examples that judge Main's minimum; restored before this commit. Co-Authored-By: Claude Sonnet 5.5 --- spec/models/mtg/deck_checks_for_magic_spec.rb | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 spec/models/mtg/deck_checks_for_magic_spec.rb diff --git a/spec/models/mtg/deck_checks_for_magic_spec.rb b/spec/models/mtg/deck_checks_for_magic_spec.rb new file mode 100644 index 00000000..3820595f --- /dev/null +++ b/spec/models/mtg/deck_checks_for_magic_spec.rb @@ -0,0 +1,143 @@ +require "rails_helper" + +RSpec.describe "Catalog::DeckChecks for a Magic deck", type: :model do + let(:account) { create(:user).account } + + def deck_of(format) = create(:deck, account:, format:, name: "Deck #{format}") + + def card(name, type_line: "Instant", text: nil, legalities: {}, identity: create(:catalog_identity, name:), **attributes) + entry = create(:catalog_entry, identity:, name:, **attributes) + create(:mtg_printing, entry:, legalities:, faces: [ { "name" => name, "type_line" => type_line, "oracle_text" => text, "image_uris" => {} } ]) + entry + end + + def land(name = "Plains") = card(name, type_line: "Basic Land — #{name}") + def own(deck, zone, entry, quantity) = create_lot_in_zone(deck, zone, entry:, quantity:) + def proxy(deck, zone, entry) = create(:deck_proxy, zone: deck.zone(zone), account:, entry:, finish: "nonfoil", quantity: 1) + + def lines(deck) + page = Deck::Page.new(Deck.includes(:location).find_by!(location_id: deck.location_id)) + Catalog::DeckChecks.report("mtg", deck.format, page.zones).lines.map { [ it.status, it.sentence ] } + end + + it "judges Main and Sideboard in each constructed format's own name", :aggregate_failures do + %w[standard pioneer modern legacy vintage pauper].each do |key| + deck = deck_of(key) + own(deck, "main", land, 58) + own(deck, "sideboard", land("Island"), 16) + expect(lines(deck)).to include([ "Short", "Main: 58 cards; #{key.titleize} needs at least 60." ], [ "Over", "Sideboard: 16 cards; #{key.titleize} allows at most 15." ]) + end + end + + it "counts a proxy toward Main and passes at 60 and at 15", :aggregate_failures do + deck = deck_of("standard") + own(deck, "main", land, 59) + proxy(deck, "main", land("Island")) + own(deck, "sideboard", land("Swamp"), 15) + expect(lines(deck)).to include([ "Pass", "Main: 60 cards; Standard needs at least 60." ], [ "Pass", "Sideboard: 15 cards; Standard allows at most 15." ]) + end + + it "judges Commander's two zones together", :aggregate_failures do + deck = deck_of("commander") + own(deck, "commander", card("Krenko"), 1) + own(deck, "main", land, 98) + expect(lines(deck)).to include([ "Short", "Commander + Main: 99 cards; Commander needs exactly 100." ]) + own(deck, "main", land("Island"), 2) + expect(lines(deck)).to include([ "Over", "Commander + Main: 101 cards; Commander needs exactly 100." ]) + end + + it "says Casual and an unregistered format have no rules but still flag a forgotten proxy", :aggregate_failures do + deck = deck_of("casual") + bolt = card("Lightning Bolt") + own(deck, "main", bolt, 2) + proxy(deck, "main", bolt) + expect(lines(deck)).to eq([ [ nil, "Casual has no rules to check." ], [ "Owned and proxy", "Lightning Bolt in Main: 2 owned, 1 proxy." ] ]) + deck.update!(format: "retired") + expect(lines(deck).first).to eq([ nil, "retired has no rules to check." ]) + end + + it "adds a card's copies across printings, zones, owned and proxy", :aggregate_failures do + deck = deck_of("standard") + bolt = create(:catalog_identity, name: "Lightning Bolt") + one = card("Lightning Bolt", identity: bolt) + two = card("Lightning Bolt", identity: bolt, set: create(:catalog_set)) + own(deck, "main", one, 3) + own(deck, "sideboard", two, 1) + proxy(deck, "main", two) + expect(lines(deck)).to include([ "Over the limit", "Lightning Bolt: 5 copies; the limit is 4." ]) + end + + it "names a card by its English name when its first row in page order is a Japanese printing", :aggregate_failures do + deck = deck_of("standard") # Main is read before Sideboard, and the Japanese printing is the only row of Main + bolt = create(:catalog_identity, name: "Lightning Bolt") + own(deck, "main", card("Lightning Bolt", identity: bolt, language: "ja", localized_name: "稲妻"), 3) + own(deck, "sideboard", card("Lightning Bolt", identity: bolt, set: create(:catalog_set)), 2) + expect(lines(deck)).to include([ "Over the limit", "Lightning Bolt: 5 copies; the limit is 4." ]) + expect(lines(deck).map(&:last)).not_to include(a_string_matching(/稲妻/)) + end + + it "names a Sideboard the deck has no zone row for from the registration (AC-2.12)" do + deck = deck_of("standard") + deck.zone("sideboard").destroy! + expect(lines(deck)).to include([ "Pass", "Sideboard: 0 cards; Standard allows at most 15." ]) + end + + it "passes Commander at exactly 100 while cards in Considering count nowhere (AC-2.4, 2.5)" do + commander = deck_of("commander") + own(commander, "commander", card("Krenko"), 1) + own(commander, "main", land, 99) + own(commander, "considering", land("Island"), 30) + expect(lines(commander)).to include([ "Pass", "Commander + Main: 100 cards; Commander needs exactly 100." ]) + end + + it "exempts basic lands and any-number cards, and holds Commander to one of the rest", :aggregate_failures do + standard = deck_of("standard") + own(standard, "main", land, 20) + rats = card("Relentless Rats", type_line: "Creature — Rat", text: "A deck can have any number of cards named Relentless Rats.") + own(standard, "main", rats, 9) + expect(lines(standard)).to include([ "Pass", "No card is over its limit." ]) + commander = deck_of("commander") + krenko = create(:catalog_identity, name: "Krenko") + own(commander, "commander", card("Krenko", identity: krenko), 1) + own(commander, "main", card("Krenko", identity: krenko, set: create(:catalog_set)), 1) + own(commander, "main", card("Snow-Covered Island", type_line: "Basic Snow Land — Island"), 2) + expect(lines(commander)).to include([ "Over the limit", "Krenko: 2 copies; the limit is 1." ]) + expect(lines(commander).map(&:last)).not_to include(a_string_matching(/Snow-Covered/)) + end + + it "holds a numbered card to its number in every format, and passes at that number", :aggregate_failures do + text = "A deck can have up to seven cards named Seven Dwarves." + %w[standard commander].each do |key| + over = deck_of(key) + own(over, "main", card("Seven Dwarves", type_line: "Creature — Dwarf", text:), 8) + expect(lines(over)).to include([ "Over the limit", "Seven Dwarves: 8 copies; the limit is 7." ]) + exact = create(:deck, account:, format: key, name: "Exact #{key}") + own(exact, "main", card("Seven Dwarves", type_line: "Creature — Dwarf", text:), 7) + expect(lines(exact)).to include([ "Pass", "No card is over its limit." ]) + end + end + + it "restricts a card to one in Vintage only", :aggregate_failures do + legalities = { "vintage" => "restricted" } + vintage = deck_of("vintage") + own(vintage, "main", card("Black Lotus", type_line: "Artifact", legalities:), 2) + expect(lines(vintage)).to include([ "Over the limit", "Black Lotus: 2 copies; restricted to 1." ]) + legacy = deck_of("legacy") + own(legacy, "main", card("Black Lotus", type_line: "Artifact", legalities:), 2) + expect(lines(legacy)).to include([ "Pass", "No card is over its limit." ]) + end + + it "counts a basic land's two printings, one with no card data, at the ordinary limit (AC-3.14, 3.16)", :aggregate_failures do + deck = deck_of("standard") + identity = create(:catalog_identity, name: "Plains") + own(deck, "main", card("Plains", type_line: "Basic Land — Plains", identity:), 1) + own(deck, "main", create(:catalog_entry, identity:, name: "Plains", set: create(:catalog_set)), 4) + expect(lines(deck)).to include([ "Over the limit", "Plains: 5 copies; the limit is 4." ]) + end + + it "leaves out another collectible's cards", :aggregate_failures do + deck = deck_of("standard") + own(deck, "main", create(:catalog_entry, collectible_type: "stranger", name: "Odd"), 70) + expect(lines(deck)).to include([ "Short", "Main: 0 cards; Standard needs at least 60." ], [ "Pass", "No card is over its limit." ]) + end +end From 4c5097e6ba4a3d2312e4a06cbf5d045083042453 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:00:43 -0500 Subject: [PATCH 07/24] feat(decks): Check deck on the deck page (#258) GET /decks/:deck_id/check renders the deck page with a "Deck check" section between the head and the zones: the summary and one line per result. The head gains a Check deck link (no prefetch); the results page is marked turbo-cache-control no-cache. The deck guard counts the new controller and partial, and the head now has three buttons. Co-Authored-By: Claude Sonnet 5.5 --- app/controllers/decks/checks_controller.rb | 17 +++ app/views/decks/_check.html.erb | 14 +++ app/views/decks/show.html.erb | 4 + config/routes.rb | 1 + spec/deck_core_spec.rb | 2 +- spec/requests/deck_checks_spec.rb | 114 +++++++++++++++++++++ spec/system/deck_page_spec.rb | 2 +- 7 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 app/controllers/decks/checks_controller.rb create mode 100644 app/views/decks/_check.html.erb create mode 100644 spec/requests/deck_checks_spec.rb diff --git a/app/controllers/decks/checks_controller.rb b/app/controllers/decks/checks_controller.rb new file mode 100644 index 00000000..da8cb205 --- /dev/null +++ b/app/controllers/decks/checks_controller.rb @@ -0,0 +1,17 @@ +# Check deck (spec 079): the deck page with its checks' results. A read-only request, so the link to it needs no form and the +# result is never stored: any later request of the deck page has none (decision 7). The deck comes from the signed-in +# account's decks, so another account's is a 404 (`FindsDeck`). It builds the page and the shape panel as +# `DecksController#show` does and adds one report; the report reads the loaded rows and runs no statement of its own (AC-5.1). +class Decks::ChecksController < ApplicationController + include FindsDeck + + before_action :set_deck + + # Renders the deck page with `@check` set. + def show + @page = Deck::Page.new(@deck) + @shape = Catalog::DeckShape.answer(@deck.collectible_type, @page.zones) + @check = Catalog::DeckChecks.report(@deck.collectible_type, @deck.format, @page.zones) + render "decks/show" + end +end diff --git a/app/views/decks/_check.html.erb b/app/views/decks/_check.html.erb new file mode 100644 index 00000000..09b52366 --- /dev/null +++ b/app/views/decks/_check.html.erb @@ -0,0 +1,14 @@ +
+

Deck check

+

<%= check.summary %>

+
    + <% check.lines.each do |line| %> +
  • "> + <% if line.status %> + <%= render(line.fault ? "icons/alert" : "icons/check") %><%= line.status %> + <% end %> + <%= line.sentence %> +
    • + <% end %> +
+
diff --git a/app/views/decks/show.html.erb b/app/views/decks/show.html.erb index 1912b2de..223e4b68 100644 --- a/app/views/decks/show.html.erb +++ b/app/views/decks/show.html.erb @@ -1,4 +1,6 @@ <% content_for :title, "#{@deck.name} · Collector" %> +<%# The results page isn't kept for Back: Back asks for the check again, so it never shows results from before an edit (spec 079 decision 7). %> +<% content_for :head, tag.meta(name: "turbo-cache-control", content: "no-cache") if @check %> <%= render "layouts/appbar", section: :decks, detail: true, back_path: decks_path %>
@@ -8,6 +10,7 @@
<%= link_to "Edit deck", edit_deck_path(@deck), class: "c-btn c-btn--secondary" %> + <%= link_to "Check deck", deck_check_path(@deck), class: "c-btn c-btn--secondary", data: { turbo_prefetch: false } %>
<%= render "icons/more" %>
@@ -18,6 +21,7 @@
+ <%= render "decks/check", check: @check if @check %>
<% if @shape %> <%= render "decks/shape", answer: @shape, copy: :fold %> diff --git a/config/routes.rb b/config/routes.rb index 6c68a2a0..0d467d99 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,6 +49,7 @@ resource :removal, only: :new, module: :decks resource :format, only: %i[edit update], module: :decks # spec 064: change a deck's format resource :selection, only: :create, module: :decks # spec 067: the refusal page's shortcut into bulk mode + resource :check, only: :show, module: :decks # spec 079: Check deck, the deck page with its checks # Spec 068: a proxy is named by its own id under its deck (the deck's location id is :deck_id). scope module: :decks do resources :proxies, only: %i[edit update destroy] do diff --git a/spec/deck_core_spec.rb b/spec/deck_core_spec.rb index c00233a9..edfba1c7 100644 --- a/spec/deck_core_spec.rb +++ b/spec/deck_core_spec.rb @@ -37,7 +37,7 @@ def naming?(text) = text.match?(game_names) || text.match?(format_and_zone_keys) def relative(files) = files.map { |file| file.relative_path_from(Rails.root).to_s } it "covers every file of the feature" do - expect(files.size).to eq(20 + 6 + 1 + 1 + 4 + 1 + 15 + 2 + 6 + 5 + 1 + 3 + 1) # spec 069 adds the seam `app/models/catalog/deck_shape.rb` and the page's frame `app/views/decks/_shape.html.erb` (the views glob already covers it); spec 064 brought the list to 20; spec 065 adds the choices model, its helper and the four picker views; spec 067 adds the selections controller (the glob already covers it); spec 066 adds Deck::Page and the deck page's four views (the `app/models/deck/*.rb` and `app/views/decks/**/*.erb` globs already cover them) and the seam `app/models/catalog/deck_cards.rb`; spec 068 adds 15: `deck/proxy.rb` and the proxies controller (the globs already cover them), the six proxy views under `app/views/decks`, `decks/proxies/removals_controller.rb`, `deck/proxy/addition.rb` and `cap_exceeded.rb`, `finds_deck.rb`, the import's `proxies.rb` and `proxy_rows.rb`, and the preview's `_proxies.html.erb`; spec 070 adds 6: the replacements and copies controllers, UseCopies and AddCopy, and the two new views (the globs already cover them); spec 071 adds 5: the controller, its view, `Lot::FreeCopies` and the helper (new patterns) and `deck/addition.rb` (the `app/models/deck/*.rb` glob already covers it); spec 073 adds 1: `deck/card_uses.rb`, covered by the `app/models/deck/*.rb` glob; spec 073 adds 3 here: `catalog/card_overview.rb` and the card page's `_stats` and `_decks` partials (new patterns; the page's `show.html.erb` is not listed, because its `scryfall` link variable names a source); spec 079 adds the seam app/models/catalog/deck_checks.rb (a new pattern) + expect(files.size).to eq(20 + 6 + 1 + 1 + 4 + 1 + 15 + 2 + 6 + 5 + 1 + 3 + 1 + 2) # spec 069 adds the seam `app/models/catalog/deck_shape.rb` and the page's frame `app/views/decks/_shape.html.erb` (the views glob already covers it); spec 064 brought the list to 20; spec 065 adds the choices model, its helper and the four picker views; spec 067 adds the selections controller (the glob already covers it); spec 066 adds Deck::Page and the deck page's four views (the `app/models/deck/*.rb` and `app/views/decks/**/*.erb` globs already cover them) and the seam `app/models/catalog/deck_cards.rb`; spec 068 adds 15: `deck/proxy.rb` and the proxies controller (the globs already cover them), the six proxy views under `app/views/decks`, `decks/proxies/removals_controller.rb`, `deck/proxy/addition.rb` and `cap_exceeded.rb`, `finds_deck.rb`, the import's `proxies.rb` and `proxy_rows.rb`, and the preview's `_proxies.html.erb`; spec 070 adds 6: the replacements and copies controllers, UseCopies and AddCopy, and the two new views (the globs already cover them); spec 071 adds 5: the controller, its view, `Lot::FreeCopies` and the helper (new patterns) and `deck/addition.rb` (the `app/models/deck/*.rb` glob already covers it); spec 073 adds 1: `deck/card_uses.rb`, covered by the `app/models/deck/*.rb` glob; spec 073 adds 3 here: `catalog/card_overview.rb` and the card page's `_stats` and `_decks` partials (new patterns; the page's `show.html.erb` is not listed, because its `scryfall` link variable names a source); spec 079 adds the seam app/models/catalog/deck_checks.rb (a new pattern); spec 079 adds the check's controller and the results partial (the globs already cover them) end it "names every file it lists, each of which exists" do diff --git a/spec/requests/deck_checks_spec.rb b/spec/requests/deck_checks_spec.rb new file mode 100644 index 00000000..3d989dac --- /dev/null +++ b/spec/requests/deck_checks_spec.rb @@ -0,0 +1,114 @@ +require "rails_helper" + +RSpec.describe "Deck checks", type: :request do + let(:user) { create(:user) } + let(:account) { user.account } + let(:deck) { create(:deck, account:, name: "Izzet Tempo", format: "standard") } + + def html = Nokogiri::HTML5(response.body) + + def printing(name: "Lightning Bolt", type_line: "Instant") + entry = create(:catalog_entry, identity: create(:catalog_identity, name:), name:) + create(:mtg_printing, entry:, faces: [ { "name" => name, "type_line" => type_line, "mana_cost" => "{R}", "image_uris" => {} } ]) + entry + end + + def plains = printing(name: "Plains", type_line: "Basic Land — Plains") + def own(zone, entry, quantity: 1) = create_lot_in_zone(deck, zone, entry:, quantity:) + def proxy_in(zone, entry) = create(:deck_proxy, zone: deck.zone(zone), account:, entry:, finish: "nonfoil", quantity: 1) + def rows = html.css(".c-check__row").map { it.text.squish } + + context "when signed in" do + before { sign_in_as(user) } + + it "shows the button and no results on the plain deck page (AC-1.1, 1.9)", :aggregate_failures do + get deck_path(deck) + expect(html.at_css(".c-item__actions > a.c-btn--secondary[href='#{deck_check_path(deck)}']").text).to eq("Check deck") + expect(html.at_css("#deck-check-title")).to be_nil + expect(html.at_css("main form")).to be_nil + expect(response.body).not_to include('name="turbo-cache-control"') + end + + it "shows the summary and the results between the head and the zones (AC-1.2, 1.7, 1.8, 1.9)", :aggregate_failures do + own("main", plains, quantity: 58) + get deck_check_path(deck) + expect(response).to have_http_status(:ok) + expect([ html.at_css("#deck-check-title").text, html.at_css(".c-check__summary").text ]).to eq([ "Deck check", "1 finding" ]) + expect(rows).to eq([ "Short Main: 58 cards; Standard needs at least 60.", "Pass Sideboard: 0 cards; Standard allows at most 15.", + "Pass No card is over its limit.", "Pass No card has both owned copies and proxies in one zone." ]) + expect(html.css("main > *").first(3).map { it["class"].to_s.split.first }).to eq(%w[c-item__head c-check c-deck]) + expect(response.body).to include('') + expect(html.at_css("main form")).to be_nil + end + + it "shows the same results again and every link of the plain page (AC-1.4, 1.5)", :aggregate_failures do + own("main", plains, quantity: 58) + get deck_path(deck) + plain = html.css("main a[href]").map { it["href"] } + get deck_check_path(deck) + first = rows + get deck_check_path(deck) + expect(rows).to eq(first) + expect(html.css("main a[href]").map { it["href"] }).to include(*plain) + expect(html.css("main a, main button").map { it["disabled"] || it["aria-disabled"] }.compact).to be_empty + get edit_deck_path(deck) # following one of them works as from the plain page + expect(response).to have_http_status(:ok) + end + + it "changes nothing: no write reaches lots, proxies or locations (AC-4.8)", :aggregate_failures do + entry = printing + own("main", entry) + proxy_in("main", entry) + before = [ Lot.sum(:quantity), Deck::Proxy.sum(:quantity), Location.count ] + writes = [ Lot, Deck::Proxy, Location ].flat_map { |model| write_statements_to(model.table_name) { get deck_check_path(deck) } } + expect(writes).to be_empty + expect([ Lot.sum(:quantity), Deck::Proxy.sum(:quantity), Location.count ]).to eq(before) + end + + it "shows no results on the next deck page request after the deck changes (AC-1.3)", :aggregate_failures do + entry = printing + own("main", entry) + proxy = proxy_in("main", entry) + get deck_check_path(deck) + expect(rows).to include("Owned and proxy Lightning Bolt in Main: 1 owned, 1 proxy.") + delete deck_proxy_path(deck, proxy) + follow_redirect! + expect(html.at_css("#deck-check-title")).to be_nil + expect(html.at_css("a[href='#{deck_check_path(deck)}']")).to be_present + get deck_check_path(deck) # pressing the button again judges the changed deck + expect(rows).not_to include(a_string_matching(/Owned and proxy/)) + end + + it "words no findings, one finding and several (AC-1.7)", :aggregate_failures do + own("main", plains, quantity: 60) + get deck_check_path(deck) + expect(html.at_css(".c-check__summary").text).to eq("No findings") + own("sideboard", plains, quantity: 16) + get deck_check_path(deck) + expect(html.at_css(".c-check__summary").text).to eq("1 finding") + own("main", printing(name: "Shock"), quantity: 5) + get deck_check_path(deck) + expect(html.at_css(".c-check__summary").text).to eq("2 findings") + end + + it "gives a deck with no rules the no-rules line and the owned-plus-proxy result (AC-2.7)" do + get deck_check_path(create(:deck, account:, name: "Pile", format: "casual")) + expect(rows).to eq([ "Casual has no rules to check.", "Pass No card has both owned copies and proxies in one zone." ]) + end + + it "is a 404 for another account's deck, a zone and a bad id (AC-1.6)", :aggregate_failures do + theirs = "Rival #{SecureRandom.hex(4)}" # built at run time: the test 404 page prints the spec's own source lines (docs/lessons 2026-11-11) + other = create(:deck, account: create(:user).account, name: theirs) + [ other.to_param, deck.zone("main").id, 0, "abc" ].each do |id| + get deck_check_path(id) + expect(response).to have_http_status(:not_found), "#{id.inspect} was not a 404" + expect(response.body).not_to include(theirs) + end + end + end + + it "redirects a visitor who is not signed in (AC-5.2)" do + get deck_check_path(deck) + expect(response).to redirect_to(new_session_path) + end +end diff --git a/spec/system/deck_page_spec.rb b/spec/system/deck_page_spec.rb index 73f7aee1..06f88aef 100644 --- a/spec/system/deck_page_spec.rb +++ b/spec/system/deck_page_spec.rb @@ -256,7 +256,7 @@ def head_boxes within_narrow_frame do set_theme(theme) expect(page_fits_sideways?).to be(true) - expect(head_boxes).to eq({ "cut" => false, "title" => true, "buttons" => [ true, true ] }) + expect(head_boxes).to eq({ "cut" => false, "title" => true, "buttons" => [ true, true, true ] }) end end end From 81c5cf960fcb10f8c5eb371dddff78e05c671728 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:01:28 -0500 Subject: [PATCH 08/24] test(decks): the check runs no statement of its own (#258) AC-5.1: the check page runs the plain deck page's statements, for a 5-row and a 500-row deck alike (13 each). A characterisation: it passed on first run. Mutation proof: one Catalog::Entry.find per card in split_lines made it fail (13 vs 18, 13 vs 513, 18 vs 513); reverted. Co-Authored-By: Claude Sonnet 5.5 --- spec/requests/deck_checks_spec.rb | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/spec/requests/deck_checks_spec.rb b/spec/requests/deck_checks_spec.rb index 3d989dac..3845f113 100644 --- a/spec/requests/deck_checks_spec.rb +++ b/spec/requests/deck_checks_spec.rb @@ -111,4 +111,39 @@ def rows = html.css(".c-check__row").map { it.text.squish } get deck_check_path(deck) expect(response).to redirect_to(new_session_path) end + + describe "statements" do + before { sign_in_as(user) } + + # `count_queries` is `spec/support/query_counting.rb`'s (cached, SCHEMA and TRANSACTION statements are not counted). + def queries_for(path) + count = count_queries { get path } + expect(response).to have_http_status(:ok) + count + end + + # +rows+ printings of one identity, bulk-inserted and owned in the deck's Main zone, ten of them also as proxies. + def fill(deck, rows, label) + set = create(:catalog_set, code: label) + identity = create(:catalog_identity, name: "Bulk #{label}") + now = Time.current + Catalog::Entry.insert_all(Array.new(rows) { |i| { collectible_type: "mtg", external_key: "#{label}-#{i}", catalog_set_id: set.id, catalog_identity_id: identity.id, number: i.to_s, language: "en", name: "Card #{i}", kind: "card", content_digest: "d", created_at: now, updated_at: now } }) + ids = Catalog::Entry.where(catalog_set_id: set.id).pluck(:id) + MTG::Printing.insert_all(ids.map { |id| { catalog_entry_id: id, rarity: "common", finishes: %w[nonfoil], layout: "normal", scryfall_uri: "https://example.test/#{id}", faces: [], created_at: now, updated_at: now } }) + location = deck.zone("main") + Lot.insert_all(ids.map { |id| { account_id: account.id, catalog_entry_id: id, quantity: 2, finish: "nonfoil", price_paid_cents: nil, location_id: location.id, lot_key: Lot.key_for("nonfoil", nil, nil, location.id), created_at: now, updated_at: now } }) + ids.first(10).each { |id| create(:deck_proxy, zone: location, account:, entry: Catalog::Entry.find(id), finish: "nonfoil", quantity: 1) } + end + + it "runs the plain deck page's statements however many rows the deck has", :aggregate_failures do + small = create(:deck, account:, name: "Small", format: "standard") + big = create(:deck, account:, name: "Big", format: "standard") + fill(small, 5, "small") + fill(big, 500, "big") + [ small, big ].each { |deck| get deck_path(deck) } # warm the page once + expect(queries_for(deck_check_path(small))).to eq(queries_for(deck_path(small))) + expect(queries_for(deck_check_path(big))).to eq(queries_for(deck_path(big))) + expect(queries_for(deck_check_path(big))).to eq(queries_for(deck_check_path(small))) + end + end end From 8d2fba5af5194431a99acbf60eb2e8a9e772d71d Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:07:12 -0500 Subject: [PATCH 09/24] feat(design): the DeckCheck pattern for the check results (#258) The c-check block styles the Check deck results between the deck page's head and its zones: a status word with its 12px icon (stroke 2.5) and a sentence per row, wrapping inside a word, on one column below 640px and two from there. The root rule is scoped to .c-deckpage because .c-check is also Form's checkbox row. A second block wraps a long zone name in a zone's head and the shape fold's facts line, which pushed the results page to 743px at 360px (AC-6.1, AC-6.4). DeckCheck.md documents the pattern; DeckPage.md and the README list it. Co-Authored-By: Claude Sonnet 5.5 --- .../stylesheets/collector/additions.css | 30 +++++++++++ docs/design-system/README.md | 6 ++- docs/design-system/components/DeckCheck.md | 53 +++++++++++++++++++ docs/design-system/components/DeckPage.md | 2 +- 4 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 docs/design-system/components/DeckCheck.md diff --git a/app/assets/stylesheets/collector/additions.css b/app/assets/stylesheets/collector/additions.css index ef9146af..8d995daf 100644 --- a/app/assets/stylesheets/collector/additions.css +++ b/app/assets/stylesheets/collector/additions.css @@ -523,6 +523,36 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt .c-zone .c-section__actions { margin-left:auto; } } +/* ---------- DeckCheck (spec 079): the Check deck results between the deck page's head and its zones ---------- */ +/* A row is a status word with its icon and a sentence; the word and the icon say pass or fault, so colour is never the only + signal (decision 8). A row breaks inside a word: a card or zone name may be 60 characters with no space (AC-6.4). The + section's root class is also Form's checkbox row (`.c-check` near the top of this file), so the root rule is scoped to the + deck page, outranks that one and puts back what it sets (align-items, font). The 12px icon takes the 2.5 stroke, as a + badge's does (Iconography). */ +.c-deckpage > .c-check { margin-top:var(--space-6); display:flex; flex-direction:column; align-items:stretch; gap:var(--space-3); + min-width:0; font:400 15px/22px var(--font-sans); } +.c-check__title:focus-visible { outline:2px solid var(--focus); outline-offset:2px; border-radius:var(--radius-sm); } +.c-check__summary { margin:0; font:400 13px/18px var(--font-mono); color:var(--ink-muted); } +.c-check__list { list-style:none; margin:0; padding:0; background:var(--surface-raised); border:1px solid var(--line); border-radius:var(--radius-md); } +.c-check__row { display:grid; grid-template-columns:minmax(0, 1fr); gap:var(--space-1); padding:var(--space-3) var(--space-4); border-top:1px solid var(--line); overflow-wrap:anywhere; } +.c-check__row:first-child { border-top:0; } +.c-check__status { display:inline-flex; align-items:center; gap:var(--space-2); font:600 13px/18px var(--font-sans); color:var(--ink); } +.c-check__status svg { width:12px; height:12px; flex:none; stroke-width:2.5; } +.c-check__row--pass .c-check__status svg { color:var(--success); } +.c-check__row--fault .c-check__status svg { color:var(--warning); } +.c-check__text { font:400 15px/22px var(--font-sans); color:var(--ink); } +@container page (min-width: 640px) { + .c-check__row { grid-template-columns:11rem minmax(0, 1fr); align-items:baseline; gap:var(--space-4); } + .c-check__row--plain .c-check__text { grid-column:1 / -1; } +} + +/* ---------- Deck page long zone names (spec 079 AC-6.1, AC-6.4): a zone's name wraps in its head and the fold's facts ---------- */ +/* The results page is the deck page, so it fits 360px only if the zones do. The export's `.c-section__title` is nowrap and the + fold's facts line names the counted zone, so a 60-character zone name with no space pushed the page to 743px. Both break + inside a word; the title shrinks (min-width:0) beside "Add a proxy" rather than push it off the line. */ +.c-zone .c-section__title { min-width:0; white-space:normal; overflow-wrap:anywhere; } +.c-shapefold__facts { min-width:0; overflow-wrap:anywhere; } + /* ---------- Deck page zones (spec 060): a table per zone, grouped rows, one shared column layout ---------- */ /* From 640px of the table's container every zone table has the same fixed widths, so Cost, Owned, Proxy, Total and the menu sit at one x in every zone (decision 25). th is border-box: a width is the whole column, padding included. A pip is diff --git a/docs/design-system/README.md b/docs/design-system/README.md index 11a51415..72fd35d4 100644 --- a/docs/design-system/README.md +++ b/docs/design-system/README.md @@ -102,7 +102,7 @@ The words every deck screen uses (spec 057 FR-3). They name no format and no col ## Components - Components are plain CSS classes (prefix `c-`) in `collector/components.css`, with the app's additions in `collector/additions.css`, built only on these tokens. There is no JavaScript bundle: each component's README gives the HTML, which you wrap in a Rails partial or ViewComponent. -- Current set: `AppHeader`, `TabBar`, `Button`, `Menu`, `Input`, `Chip` (category chip, filter chip, tag), `Badge`, `ManaCost`, `ViewSwitch`, `FilterBar`, `ItemTile` (with `c-grid`), `CollectionTable` (with its `c-bulkbar`), `DeckShape` and `ProxyTag` (parts). Full screens: `CollectionPage` and `ItemPage` (desktop), `CollectionPagePhone` and `ItemPagePhone`, `DeckPage`, `DecksIndex` (with its New deck, Rename and removal pages), `AddToDeck`, `UseMyCopy` (built by spec 070, #257) and `AddProxy` (with its Add a new copy page, a use of `lots/_form`, spec 070). Card-page parts: `c-crumbs`, `c-item`, `c-rules`, `c-stats`, `c-section`, `c-list`, `c-legality`. +- Current set: `AppHeader`, `TabBar`, `Button`, `Menu`, `Input`, `Chip` (category chip, filter chip, tag), `Badge`, `ManaCost`, `ViewSwitch`, `FilterBar`, `ItemTile` (with `c-grid`), `CollectionTable` (with its `c-bulkbar`), `DeckShape`, `DeckCheck` and `ProxyTag` (parts). Full screens: `CollectionPage` and `ItemPage` (desktop), `CollectionPagePhone` and `ItemPagePhone`, `DeckPage`, `DecksIndex` (with its New deck, Rename and removal pages), `AddToDeck`, `UseMyCopy` (built by spec 070, #257) and `AddProxy` (with its Add a new copy page, a use of `lots/_form`, spec 070). Card-page parts: `c-crumbs`, `c-item`, `c-rules`, `c-stats`, `c-section`, `c-list`, `c-legality`. - Build new components from these before inventing new styles; a new pattern gets its own entry here first. - The design system is the project's own, in code (spec 057 decision 12). A new pattern goes in `collector/additions.css`, with a doc under `components/` and a line in the App additions list below; a rule changes in this README, in the same change as the code. @@ -146,6 +146,8 @@ App additions (spec 063, in `collector/additions.css`): the stroke of small icon App additions (spec 074, #326, in `collector/additions.css`): two blocks, no new class. `Menu`'s clearance of the phone tab bar (`scroll-margin-top` and `scroll-margin-bottom` on `.c-menu__list` and a room box, an `::after` on the open list, below 640px on a tabbed shell; the controller's scroll is in `menu_controller.js`), and `DeckPage`'s zone tables' menu column floor (`th.c-zone__c-menu`, 4rem below 640px of the table's container). See `Menu` and `DeckPage`. +App additions (spec 079, #258, in `collector/additions.css`): `DeckCheck` (`c-check` on the deck page, `c-check__title`, `c-check__summary`, `c-check__list`, `c-check__row` with `--pass`, `--fault` and `--plain`, `c-check__status`, `c-check__text`), the Check deck results; and the wrapping of a long zone name in a zone's head and the shape fold's facts line, documented in `DeckPage`. The section's root class shares its name with `Form`'s checkbox row, so its rule is scoped to the deck page. + ## Logo - The mark is three fanned cards, the front one vault teal with a foil diamond: a collection, one piece of which is special. It is deliberately game-neutral so it grows past trading cards. @@ -155,5 +157,5 @@ App additions (spec 074, #326, in `collector/additions.css`): two blocks, no new ## Iconography - No icon set chosen yet. Until one is, use Lucide (ISC-licensed) in `currentColor`, which matches Plex's weight. Flag any other set here before adopting it. -- Stroke follows the size an icon is drawn at (spec 063). A stroke scales with its icon, so a Lucide stroke of 1.5 draws 1.25px on screen at 20px but 0.75px at 12px, a hairline beside a 600-weight word. Below 20px an icon keeps about 1.25px on screen: the stroke is `30 ÷ size`, with the size in px, rounded to a half, never below 1.5 or above 2.5. The app draws 1.5 at 20px and above (the tab bar's 22px), 2 at 14px to 16px (buttons, inputs, menu items, the view switch, filter chips) and 2.5 at 12px (badges, legality marks). +- Stroke follows the size an icon is drawn at (spec 063). A stroke scales with its icon, so a Lucide stroke of 1.5 draws 1.25px on screen at 20px but 0.75px at 12px, a hairline beside a 600-weight word. Below 20px an icon keeps about 1.25px on screen: the stroke is `30 ÷ size`, with the size in px, rounded to a half, never below 1.5 or above 2.5. The app draws 1.5 at 20px and above (the tab bar's 22px), 2 at 14px to 16px (buttons, inputs, menu items, the view switch, filter chips) and 2.5 at 12px (badges, legality marks, `DeckCheck`'s status icons). - Every icon keeps `stroke-width="1.5"` in its markup (the weight at 20px, so a copied icon looks right on its own). `additions.css` sets the heavier stroke by the component that draws the icon, so an icon placed in one of those components needs nothing more. An icon drawn at a size no component sets gets its stroke by the rule above, and its size and stroke are added to the list of sizes above. diff --git a/docs/design-system/components/DeckCheck.md b/docs/design-system/components/DeckCheck.md new file mode 100644 index 00000000..156d4dc3 --- /dev/null +++ b/docs/design-system/components/DeckCheck.md @@ -0,0 +1,53 @@ +# DeckCheck + +The results of Check deck on the deck page: a full-width section between the page head and the zones, with a heading, a summary line and one row per result (spec 079, #258). The section names no game, format or zone; the core draws it (`decks/_check`) from `Catalog::DeckChecks`' report. + +**Markup** — a `section.c-check` labelled by its heading, then the summary and a `ul.c-check__list` of rows. A row is a status word with its icon (`c-check__status`) and a sentence (`c-check__text`), or a sentence alone for a line that is no check. + +```html +
+

Deck check

+

2 findings

+
    +
  • ShortMain: 58 cards; Standard needs at least 60.
    • +
  • PassSideboard: 0 cards; Standard allows at most 15.
    • +
  • Owned and proxyLightning Bolt in Main: 2 owned, 1 proxy.
    • +
+
+``` + +A deck whose format has no rules starts its list with a plain row, then owned plus proxy: + +```html +
  • Casual has no rules to check.
    • +``` + +**Classes** + +- `c-check`: the section, a column with a `space-3` gap and `space-6` above it, as sections separate. The root class is also `Form`'s checkbox row (`label.c-check`); the section's rule is scoped to the deck page (`.c-deckpage > .c-check`), outranks the label's and puts back what it sets (`align-items`, font). Never use the section outside the deck page's `main`. +- `c-check__title`: the heading, "Deck check", in `c-section__title`'s `heading` style. +- `c-check__summary`: "3 findings", "1 finding" or "No findings", in `data` (mono 13/18), `ink-muted`. +- `c-check__list`: the rows on `surface-raised` with a 1px `line` border and `radius-md` corners; rows are divided by `line`. +- `c-check__row`: one result, `space-3` by `space-4` padding. +- `c-check__status`: the status word at 13/18, weight 600, `ink`, after its 12px icon. +- `c-check__text`: the sentence in `body` (15/22), `ink`. + +**States** (spec 079 decision 6, AC-6.2) + +- `c-check__row--pass`: the word "Pass" with `icons/check` in `success`. +- `c-check__row--fault`: a finding, its word "Short", "Over", "Over the limit" or "Owned and proxy", with `icons/alert` in `warning`. A finding is advice, never an error: the check blocks nothing, so it is never `danger`. +- `c-check__row--plain`: no status, only the sentence ("Casual has no rules to check."), across the row's full width. + +**Colour is never the only signal.** The status word says pass or fault and the icon's shape repeats it (a tick or a triangle); the icon's colour only repeats both. The word stays `ink`, so it reads at 4.5:1 on `surface-raised` in both themes. + +**Focus** (AC-6.3). The heading has `tabindex="-1"` and `autofocus`: following Check deck lands keyboard and screen reader focus on "Deck check", and the next Tab goes on through the results and the zones in reading order, the section being before the zones in the DOM. The heading shows the 2px `focus` ring at `radius-sm` when focus is visible. + +**Widths** (AC-6.1, AC-6.4). The section answers to the page container (`@container page`). + +- **Below 640px:** a row is one column: the status word on its line, the sentence under it. +- **From 640px:** two columns, the status in 11rem (wide enough for "Owned and proxy" on one line) and the sentence in the rest, on one baseline with a `space-4` gap; a plain row's sentence spans both. +- A row breaks inside a word (`overflow-wrap: anywhere`, `minmax(0, 1fr)`), so a 60-character card or zone name with no space wraps and the page does not scroll sideways at 360px in either theme. The zone heads and the shape fold's facts line wrap a long zone name too (`DeckPage`). + +**Icon.** `icons/check` and `icons/alert` drawn at 12px with a stroke of 2.5, as a badge's icon is (`.c-check__status svg`; README, Iconography). The markup keeps `stroke-width="1.5"`. + +**Why not a badge or a list.** `c-badge` is one line and one or two words, and "Over the limit" and "Owned and proxy" are three; `c-list` rows do not wrap; and the deck page had no block between its head and `c-deck`. diff --git a/docs/design-system/components/DeckPage.md b/docs/design-system/components/DeckPage.md index eca0ffe9..b3dd0d3f 100644 --- a/docs/design-system/components/DeckPage.md +++ b/docs/design-system/components/DeckPage.md @@ -2,7 +2,7 @@ One deck at desktop and phone width: its zones as tables and, beside or above them, the shape panel. Shown for a Magic: The Gathering deck; the shell is collectible-agnostic (spec 060 decision 21). -**As built (specs 068, 069 and 070, #254, #256 and #257, on spec 066's page):** the page head's line reads `Format · N cards · N owned · N proxy`; a zone's head is its name, `N cards · N owned · N proxy` and "Add a proxy" (a link to the add-a-proxy pages, `AddProxy`); the table has Card, Cost, Owned, Proxy, Total and the row menu; a row with proxies shows the `ProxyTag` in its Proxy cell and in the line under its name on a phone, and its menu lists "Use my copy", "Add a new copy", "Change proxy count" and, after a separator, "Remove proxy" (spec 070, #257; the finish named in each item when the row has proxies of two finishes); a row with no proxy has an empty menu cell; a printing with a proxy and no owned copy is a row of Owned 0; Total is owned plus proxy and a group row's count is its Total; the card name links to the card page at its copies (`/catalog/entries/:key?from=collection#copies`). The page has no `form` (every action is a link). Spec 069 (#256) renders the shape panel when the deck's collectible draws one: the `details` copy, then the `aside` copy, then the zones, inside `c-deck` (the core partial `decks/_shape` draws each copy's frame, the extension's partial the three parts), so the second column holds the panel from 992px; a deck whose collectible draws none renders neither copy and leaves the column empty. The page's `main` carries `c-deckpage`, whose only rules let the head's name wrap anywhere (`overflow-wrap:anywhere`, `min-width:0`) so a 60-character deck name with no space stays inside a 360px screen. A group is its own `tbody` whose first row is the `th scope="rowgroup"` group row (a collectible that groups nothing gets one `tbody` and no group row) +**As built (specs 068, 069 and 070, #254, #256 and #257, on spec 066's page):** the page head's line reads `Format · N cards · N owned · N proxy`; a zone's head is its name, `N cards · N owned · N proxy` and "Add a proxy" (a link to the add-a-proxy pages, `AddProxy`); the table has Card, Cost, Owned, Proxy, Total and the row menu; a row with proxies shows the `ProxyTag` in its Proxy cell and in the line under its name on a phone, and its menu lists "Use my copy", "Add a new copy", "Change proxy count" and, after a separator, "Remove proxy" (spec 070, #257; the finish named in each item when the row has proxies of two finishes); a row with no proxy has an empty menu cell; a printing with a proxy and no owned copy is a row of Owned 0; Total is owned plus proxy and a group row's count is its Total; the card name links to the card page at its copies (`/catalog/entries/:key?from=collection#copies`). The page has no `form` (every action is a link). Spec 079 (#258) adds "Check deck" to the head's actions, after "Edit deck" and before the "…" `Menu`: a secondary button that is a link (a GET to `/decks/:id/check`, so the page still has no `form`). Its page is the deck page with the results, a `DeckCheck` section between the head and `c-deck`, so a phone reads them first; the head, the panel and the zones are drawn as on the plain page, the button stays and checks again, and the page carries `turbo-cache-control: no-cache`, so Back to it asks for the check again. The three head actions fit one line at 360px. A zone's name wraps in its head and in the shape fold's facts line, inside a word if it must (spec 079 AC-6.4). Spec 069 (#256) renders the shape panel when the deck's collectible draws one: the `details` copy, then the `aside` copy, then the zones, inside `c-deck` (the core partial `decks/_shape` draws each copy's frame, the extension's partial the three parts), so the second column holds the panel from 992px; a deck whose collectible draws none renders neither copy and leaves the column empty. The page's `main` carries `c-deckpage`, whose only rules let the head's name wrap anywhere (`overflow-wrap:anywhere`, `min-width:0`) so a 60-character deck name with no space stays inside a 360px screen. A group is its own `tbody` whose first row is the `th scope="rowgroup"` group row (a collectible that groups nothing gets one `tbody` and no group row) **What the page provides**: the deck's name, its format and counts, its zones with their rows, and the extension's contribution. The shell's rules below name no format and no game. The extension supplies the shape panel (`DeckShape`), the Cost column (`ManaCost` or a muted dash), the legality marker and the groups of a zone's rows; the sample's groups are a Magic deck's top-level card types. From d0acb71d81daf968023aac48e7212315bb6580e6 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:08:11 -0500 Subject: [PATCH 10/24] test(decks): the check's focus, Back and phone width in the browser (#258) Five examples in headless Firefox: focus lands on the Deck check heading after Enter on Check deck (AC-6.3), Back to the results asks for the check again (AC-1.8), the results come before the zones (AC-6.3), the results page fits a 360px viewport with 60-character names in both themes (AC-6.1, AC-6.4), and every status is a word with an icon (AC-6.2). Each of the first, second and fourth fails when its rule is removed (autofocus, the no-cache tag, the row's overflow-wrap). Co-Authored-By: Claude Sonnet 5.5 --- spec/system/deck_checks_spec.rb | 65 +++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 spec/system/deck_checks_spec.rb diff --git a/spec/system/deck_checks_spec.rb b/spec/system/deck_checks_spec.rb new file mode 100644 index 00000000..ad746c8f --- /dev/null +++ b/spec/system/deck_checks_spec.rb @@ -0,0 +1,65 @@ +require "rails_helper" + +RSpec.describe "Deck checks", type: :system do + let!(:user) { system_sign_in_as(create(:user)) } + let(:deck) { create(:deck, account: user.account, name: "Izzet Tempo", format: "standard") } + + def set_theme(name) = page.execute_script("document.documentElement.dataset.theme = #{name.to_json}") + + def printing(name, type_line: "Basic Land — Plains") + entry = create(:catalog_entry, identity: create(:catalog_identity, name:), name:) + create(:mtg_printing, entry:, faces: [ { "name" => name, "type_line" => type_line, "image_uris" => {} } ]) + entry + end + + def clipped(selector) = page.evaluate_script("[...document.querySelectorAll(#{selector.to_json})].filter(el => el.scrollWidth > el.clientWidth + 1).length") + + it "moves keyboard focus to the results heading (AC-6.3)", :aggregate_failures do + visit deck_path(deck) + expect(tab_to_text("Check deck")).to be(true) # the link is reachable by Tab, and Enter is pressed on it + page.send_keys(:enter) + expect(page).to have_css("#deck-check-title:focus", text: "Deck check") + expect(page.evaluate_script("document.getElementById('deck-check-title').tabIndex")).to eq(-1) + end + + it "asks for the check again on Back instead of restoring a Turbo snapshot (AC-1.8)", :aggregate_failures do + visit deck_path(deck) + click_link "Check deck" + expect(page).to have_text("Main: 0 cards; Standard needs at least 60.") + page.execute_script("window.__sameDocument = true") # Turbo visits keep the document; a full page load would drop this + create_lot_in_zone(deck, "main", entry: printing("Plains"), quantity: 60) + click_link "Edit deck" + wait_for_turbo_idle + page.go_back + expect(page).to have_text("Main: 60 cards; Standard needs at least 60.") + expect(page.evaluate_script("window.__sameDocument")).to be(true) # still the same document, so Turbo did the Back + end + + it "puts the results before the zones in reading order (AC-6.3)" do + visit deck_check_path(deck) + follows = "document.getElementById('deck-check-title').compareDocumentPosition(document.querySelector('.c-zone')) & Node.DOCUMENT_POSITION_FOLLOWING" + expect(page.evaluate_script("!!(#{follows})")).to be(true) + end + + it "fits a phone, in both themes, with long names (AC-6.1, 6.4)", :aggregate_failures do + long = "N" * 60 + create_lot_in_zone(deck, "main", entry: printing(long, type_line: "Instant"), quantity: 5) + deck.zone("main").update_columns(name: "Z" * 60) # a zone's shown name is a copy, so a long one is a fixture, not a rename flow + visit deck_path(deck) + expect(open_in_viewport_frame(deck_check_path(deck), viewport: 360, ready: "#deck-check-title")).to eq(360) + within_narrow_frame do + %w[light dark].each do |theme| + set_theme(theme) + expect([ page_fits_sideways?, clipped(".c-check__text, .c-check__status, .c-item__actions .c-btn") ]).to eq([ true, 0 ]) + end + end + end + + it "says pass or fault in words with an icon on every row, never by colour alone (AC-6.2)", :aggregate_failures do + create_lot_in_zone(deck, "main", entry: printing("Plains"), quantity: 58) + visit deck_check_path(deck) + rows = all(".c-check__row--fault .c-check__status") + expect(rows.map(&:text)).to all(be_in([ "Short", "Over", "Over the limit", "Owned and proxy" ])) + expect(page).to have_css(".c-check__status svg", count: page.all(".c-check__status").size) + end +end From 4ca7014d306bd9370217691a0982748d79f5fd19 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:11:07 -0500 Subject: [PATCH 11/24] fix(design): rename the DeckCheck classes to c-deckcheck, off Form's c-check (#258) The results section's classes no longer share a name with Form's checkbox row, so the deck-page scoping and its resets are gone. Co-Authored-By: Claude Sonnet 5.5 --- .../stylesheets/collector/additions.css | 32 +++++++-------- app/views/decks/_check.html.erb | 14 +++---- docs/design-system/README.md | 2 +- docs/design-system/components/DeckCheck.md | 40 +++++++++---------- spec/requests/deck_checks_spec.rb | 12 +++--- spec/system/deck_checks_spec.rb | 6 +-- 6 files changed, 52 insertions(+), 54 deletions(-) diff --git a/app/assets/stylesheets/collector/additions.css b/app/assets/stylesheets/collector/additions.css index 8d995daf..e7f36745 100644 --- a/app/assets/stylesheets/collector/additions.css +++ b/app/assets/stylesheets/collector/additions.css @@ -526,24 +526,22 @@ body:has(.c-locations) .c-appbar__back { min-height:var(--touch-floor); min-widt /* ---------- DeckCheck (spec 079): the Check deck results between the deck page's head and its zones ---------- */ /* A row is a status word with its icon and a sentence; the word and the icon say pass or fault, so colour is never the only signal (decision 8). A row breaks inside a word: a card or zone name may be 60 characters with no space (AC-6.4). The - section's root class is also Form's checkbox row (`.c-check` near the top of this file), so the root rule is scoped to the - deck page, outranks that one and puts back what it sets (align-items, font). The 12px icon takes the 2.5 stroke, as a - badge's does (Iconography). */ -.c-deckpage > .c-check { margin-top:var(--space-6); display:flex; flex-direction:column; align-items:stretch; gap:var(--space-3); - min-width:0; font:400 15px/22px var(--font-sans); } -.c-check__title:focus-visible { outline:2px solid var(--focus); outline-offset:2px; border-radius:var(--radius-sm); } -.c-check__summary { margin:0; font:400 13px/18px var(--font-mono); color:var(--ink-muted); } -.c-check__list { list-style:none; margin:0; padding:0; background:var(--surface-raised); border:1px solid var(--line); border-radius:var(--radius-md); } -.c-check__row { display:grid; grid-template-columns:minmax(0, 1fr); gap:var(--space-1); padding:var(--space-3) var(--space-4); border-top:1px solid var(--line); overflow-wrap:anywhere; } -.c-check__row:first-child { border-top:0; } -.c-check__status { display:inline-flex; align-items:center; gap:var(--space-2); font:600 13px/18px var(--font-sans); color:var(--ink); } -.c-check__status svg { width:12px; height:12px; flex:none; stroke-width:2.5; } -.c-check__row--pass .c-check__status svg { color:var(--success); } -.c-check__row--fault .c-check__status svg { color:var(--warning); } -.c-check__text { font:400 15px/22px var(--font-sans); color:var(--ink); } + class is `c-deckcheck`, distinct from Form's `c-check` checkbox row. The 12px icon takes the 2.5 stroke, as a badge's + does (Iconography). */ +.c-deckcheck { margin-top:var(--space-6); display:flex; flex-direction:column; gap:var(--space-3); min-width:0; } +.c-deckcheck__title:focus-visible { outline:2px solid var(--focus); outline-offset:2px; border-radius:var(--radius-sm); } +.c-deckcheck__summary { margin:0; font:400 13px/18px var(--font-mono); color:var(--ink-muted); } +.c-deckcheck__list { list-style:none; margin:0; padding:0; background:var(--surface-raised); border:1px solid var(--line); border-radius:var(--radius-md); } +.c-deckcheck__row { display:grid; grid-template-columns:minmax(0, 1fr); gap:var(--space-1); padding:var(--space-3) var(--space-4); border-top:1px solid var(--line); overflow-wrap:anywhere; } +.c-deckcheck__row:first-child { border-top:0; } +.c-deckcheck__status { display:inline-flex; align-items:center; gap:var(--space-2); font:600 13px/18px var(--font-sans); color:var(--ink); } +.c-deckcheck__status svg { width:12px; height:12px; flex:none; stroke-width:2.5; } +.c-deckcheck__row--pass .c-deckcheck__status svg { color:var(--success); } +.c-deckcheck__row--fault .c-deckcheck__status svg { color:var(--warning); } +.c-deckcheck__text { font:400 15px/22px var(--font-sans); color:var(--ink); } @container page (min-width: 640px) { - .c-check__row { grid-template-columns:11rem minmax(0, 1fr); align-items:baseline; gap:var(--space-4); } - .c-check__row--plain .c-check__text { grid-column:1 / -1; } + .c-deckcheck__row { grid-template-columns:11rem minmax(0, 1fr); align-items:baseline; gap:var(--space-4); } + .c-deckcheck__row--plain .c-deckcheck__text { grid-column:1 / -1; } } /* ---------- Deck page long zone names (spec 079 AC-6.1, AC-6.4): a zone's name wraps in its head and the fold's facts ---------- */ diff --git a/app/views/decks/_check.html.erb b/app/views/decks/_check.html.erb index 09b52366..de3a958f 100644 --- a/app/views/decks/_check.html.erb +++ b/app/views/decks/_check.html.erb @@ -1,13 +1,13 @@ -
    -

    Deck check

    -

    <%= check.summary %>

    -
    <%= link_to "Edit deck", edit_deck_path(@deck), class: "c-btn c-btn--secondary" %> + <%# No hover prefetch: it would run a check nobody asked for, wasted work that could show results from before an edit (spec 079 decision 7). %> <%= link_to "Check deck", deck_check_path(@deck), class: "c-btn c-btn--secondary", data: { turbo_prefetch: false } %>
    <%= render "icons/more" %> diff --git a/docs/architecture/collection-and-lots.md b/docs/architecture/collection-and-lots.md index 2a29c891..0e04a5e6 100644 --- a/docs/architecture/collection-and-lots.md +++ b/docs/architecture/collection-and-lots.md @@ -136,7 +136,7 @@ The page's collectible part is two optional answers of the collecting vocabulary The shape panel is the collecting vocabulary's third answer, after `formats` and `deck_card`: `deck_shape(zones)` takes `Deck::Page#zones` as the page built them and returns a `Catalog::DeckShape::Answer` (`partial`, `figures`, `scope`, `facts`) or nil, and `DecksController#show` asks once through `Catalog::DeckShape.answer`, one line after building the page; a vocabulary without it, a collectible type no longer registered and a nil answer draw no panel and leave the second column empty from 992px; the contract is written in `Catalog::DeckShape`'s comment, which names no collectible, format or zone (spec 069 decision 1, FR-1, AC-1.4; spec 060 AC-3.12). The core partial `decks/_shape` draws each copy's frame (the `aside`'s `section`, `h2` and scope line; the `details`' summary with the facts line and its `c-shape--bare` box) and renders the extension's partial inside, in the `details` copy, then the `aside` copy, before the zones; the figures are built once and rendered twice (decisions 1, 12, AC-1.1, AC-6.3). Magic's answer is `MTG::Collecting.deck_shape` over the figures object `MTG::DeckShape` and its partial `mtg/deck_shapes/_parts`. It counts the Commander and Main zones only (`MTG::DeckShape::COUNTED_ZONES`, in the deck's zone order; Sideboard and Considering never count, and the extension, not the core, names them), so the scope line reads "Main · 60 cards" or "Commander + Main · 100 cards" ("1 card"), a deck with neither zone has no panel, a deck of a format no longer registered keeps its Main zone's panel, and a deck with no card shows the panel's empty states (decisions 2, 10, 14, AC-1.5, AC-1.8, AC-2.8). A card is one copy or one proxy, so owned and proxy count together; a row of another collectible's card (page answer `Catalog::DeckCards::NONE`, no position) is skipped by every part, the scope line's number included (decision 3, AC-3.5). Card types: one type per card, the zone tables' own group (`row.answer.position`, `MTG::DeckGroups`) in the order of `MTG::DeckGroups::NAMES`, only the groups that have a card, each with its count and "N proxy" when it has proxies, so the counts add up to the scope line's number (decision 3, AC-3.1–AC-3.4). The curve counts nonland cards (not in Lands, so Dryad Arbor counts and an Artifact Land does not) whose card has a mana value, from `mtg_cards.mana_value` through the printing's identity (Scryfall's number, spec 058); column `n` from 0 to 5 holds the values from `n` up to but not including `n + 1` (floor, so 0.5 is in 0 and 2.5 in 2) and 6+ holds every value from 6, owned and proxy kept apart (decision 4, AC-2.1–AC-2.3, AC-2.6). The average is the exact mean of the curve's cards (rational arithmetic, so a mean of 0.15 shows 0.2), shown to one decimal rounded half up with the trailing zero kept ("3.0") and thousands delimited, and left out when no card is in the curve (decision 5, AC-2.4). A nonland card with no mana value is left out of the curve, the average and the legend, never counted as 0, and a second muted line under the average says "N nonland cards have no mana value yet." ("1 nonland card has no mana value yet."); the card is still in its type and the scope line (decision 6, AC-2.5). The mana bar counts coloured symbols of the first face's `mana_cost` on the loaded printing, the cost the zone tables show, never the joined `mtg_cards.mana_cost`, for every card in the counted zones, Lands included, each symbol times the card's count: `{W}`, `{U}`, `{B}`, `{R}` and `{G}` count one toward their colour; a hybrid (`{W/U}`) counts one toward each colour; a Phyrexian symbol (`{G/P}`) counts one toward its colour and a Phyrexian hybrid (`{G/W/P}`) one toward each; `{2/W}` and `{C/W}` count one toward the colour only; generic, `{X}`, `{C}`, snow and half symbols (`{HW}`) are never counted (decisions 7, 8, AC-4.1–AC-4.5). Each colour present shows its count and its share of all coloured symbols, each percentage rounded half up on its own so a caption may add to 99 or 101, in W, U, B, R, G order; a segment under 10% of the symbols (`count × 100 < total × 10`) is narrow and shows no label inside, its count staying in the caption; with no coloured symbol the part says "No coloured mana symbols." and an empty Card types part says "No cards yet." (decisions 9, 14, AC-4.6–AC-4.8). The visible number in a curve column's count and in a mana segment's label is abbreviated from 1,000 (`MTG::DeckShape#short_count`: 1,357 is "1.4k", 12,345 "12k", 1,250,000 "1.3m", half up on integers, at most four characters through 999m), because both sit in a slot of fixed width; every other number and every `aria-label` stays exact and delimited; the caption is a run of unbreakable items (`caption_items`, one per colour, so a line never starts with "·"); and the Card types list is a grid whose rows are subgrids, so the counts line up whatever the suffix (spec 077 decisions 1 to 5, #320; it supersedes, in part, spec 060 decision 2, AC-2.2, AC-2.4, AC-2.5 and AC-2.6 (the written-in-full numbers, the fixed 4.5rem minimum, the 279px) and spec 069 decision 14 for those two numbers). A mana label is drawn only if it fits its segment on one line (a segment under 10% never has one, spec 060 decision 4): the browser's own line breaking moves a label that cannot fit onto a second line that `overflow:clip` hides, and the caption carries that colour's count (spec 078, #330, which closes the limit spec 077 decision 2 had recorded; see `DeckShape.md`). **A split card's second half adds no symbol to the bar, and that is deliberate:** a card is read through its first face for its type, cost and symbols, as the zone tables read it, while its mana value is Scryfall's number for the card, both halves added, so a split card is in the curve column of that sum while its symbols are the first half's only (spec 069 Non-Goals, decision 7, AC-4.5). The panel costs one statement of its own, `MTG::Card` for the identity ids of the counted zones' printings, none when those zones hold no card, and reads everything else from the rows `Deck::Page` loaded, so a page's count is the same however large the deck is (decision 11, AC-6.1, AC-6.2); it stores nothing and the export is unchanged (decision 13). -**Check deck** (spec 079, [#258](https://github.com/plainprogrammer/Collector/issues/258); the markup is `DeckCheck`). Check deck is a link styled as a secondary button, after Edit deck in the head, to `GET /decks/:id/check` (`Decks::ChecksController#show`), so neither page has a `form` (decision 7, AC-1.9). The request finds the deck as the deck page does (another account's deck, a zone's id and an id that isn't digits only are 404, AC-1.6; a visitor gets the sign-in redirect, AC-5.2), builds the page and the shape panel as `DecksController#show` does, adds one report and renders the deck page with it: a full-width section between the head and the zones, so a phone reads it first, with the heading "Deck check" (which takes keyboard focus on load: `autofocus` on a heading with `tabindex="-1"`, which Turbo honours on a link visit), the summary line and one row per result; the head, the panel, the zones and every link of the plain page are as on the plain page, all enabled, and Check deck stays and checks again (decision 8, AC-1.2, AC-1.4, AC-1.5, AC-6.3). Nothing is stored: any later request of the deck page (the redirect after an edit, a change of format, a visit) has no results and shows the button again, and the results page alone carries `turbo-cache-control: no-cache`, so Turbo keeps no snapshot of it and Back to it is a fresh check of the deck as it then is (decisions 7, 9, AC-1.3, AC-1.8). The checks are advice: no finding disables, hides or refuses anything, and the request writes no lot, proxy or location (FR-3, AC-4.8). The plain page runs the statements it ran before the feature, and the check request runs the same number as the plain page, for a deck of 5 rows and of 500 (NFR Performance, AC-5.1; `spec/requests/deck_checks_spec.rb`). Nothing is exported, imported or backed up, and the export format stays at version 4 (decision 9, AC-5.3). +**Check deck** (spec 079, [#258](https://github.com/plainprogrammer/Collector/issues/258); the markup is `DeckCheck`). Check deck is a link styled as a secondary button, after Edit deck in the head, to `GET /decks/:id/check` (`Decks::ChecksController#show`), so neither page has a `form` (decision 7, AC-1.9). The request finds the deck as the deck page does (another account's deck, a zone's id and an id that isn't digits only are 404, AC-1.6; a visitor gets the sign-in redirect, AC-5.2), builds the page and the shape panel as `DecksController#show` does, adds one report and renders the deck page with it: a full-width section between the head and the zones, so a phone reads it first, with the heading "Deck check" (which takes keyboard focus on load: `autofocus` on a heading with `tabindex="-1"`, which Turbo honours on a link visit), the summary line and one row per result; the head, the panel, the zones and every link of the plain page are as on the plain page, all enabled, and Check deck stays and checks again (decision 8, AC-1.2, AC-1.4, AC-1.5, AC-6.3). The link turns Turbo's hover prefetch off (`data-turbo-prefetch="false"`): a prefetch would compute a check nobody asked for, and since nothing is stored it would be wasted work that could show results from before an edit (spec 079 decision 7). Nothing is stored: any later request of the deck page (the redirect after an edit, a change of format, a visit) has no results and shows the button again, and the results page alone carries `turbo-cache-control: no-cache`, so Turbo keeps no snapshot of it and Back to it is a fresh check of the deck as it then is (decisions 7, 9, AC-1.3, AC-1.8). The checks are advice: no finding disables, hides or refuses anything, and the request writes no lot, proxy or location (FR-3, AC-4.8). The plain page runs the statements it ran before the feature, and the check request runs the same number as the plain page, for a deck of 5 rows and of 500 (NFR Performance, AC-5.1; `spec/requests/deck_checks_spec.rb`). Nothing is exported, imported or backed up, and the export format stays at version 4 (decision 9, AC-5.3). The report is the core's `Catalog::DeckChecks.report` over `Deck::Page#zones` as the page built them, with the deck's collectible type and stored format key. Its rules are the collecting vocabulary's fourth answer, `deck_rules(format_key)`: the format's size rules (each the zone keys it counts and a minimum, a maximum or an exact size) and its copy rule (the zone keys counted together and the limit of a counted printing), or nil. **The contract is written in `Catalog::DeckChecks`' comment, the one place it is written down**; this doc does not restate it. A vocabulary without `deck_rules`, a nil answer, a collectible type the instance no longer registers and a deck whose format it no longer registers have no size or copy rules (decision 1, FR-1, AC-5.5). The core owns the counting, the words and the order, reads only the rows and extension records the page loaded (never a record by an id from the request, never a statement per row) and names no collectible, format or zone key but `considering` (decision 1, FR-1, AC-5.4; the deck guard, below). Magic's answer is `MTG::Collecting.deck_rules`, which returns `MTG::DeckRules.for(format_key)`. diff --git a/docs/design-system/components/DeckPage.md b/docs/design-system/components/DeckPage.md index 9951a8f1..e55c8a3b 100644 --- a/docs/design-system/components/DeckPage.md +++ b/docs/design-system/components/DeckPage.md @@ -2,7 +2,7 @@ One deck at desktop and phone width: its zones as tables and, beside or above them, the shape panel. Shown for a Magic: The Gathering deck; the shell is collectible-agnostic (spec 060 decision 21). -**As built (specs 068, 069 and 070, #254, #256 and #257, on spec 066's page):** the page head's line reads `Format · N cards · N owned · N proxy`; a zone's head is its name, `N cards · N owned · N proxy` and "Add a proxy" (a link to the add-a-proxy pages, `AddProxy`); the table has Card, Cost, Owned, Proxy, Total and the row menu; a row with proxies shows the `ProxyTag` in its Proxy cell and in the line under its name on a phone, and its menu lists "Use my copy", "Add a new copy", "Change proxy count" and, after a separator, "Remove proxy" (spec 070, #257; the finish named in each item when the row has proxies of two finishes); a row with no proxy has an empty menu cell; a printing with a proxy and no owned copy is a row of Owned 0; Total is owned plus proxy and a group row's count is its Total; the card name links to the card page at its copies (`/catalog/entries/:key?from=collection#copies`). The page has no `form` (every action is a link). Spec 079 (#258) adds "Check deck" to the head's actions, after "Edit deck" and before the "…" `Menu`: a secondary button that is a link (a GET to `/decks/:id/check`, so the page still has no `form`). Its page is the deck page with the results, a `DeckCheck` section between the head and `c-deck`, so a phone reads them first; the head, the panel and the zones are drawn as on the plain page, the button stays and checks again, and the page carries `turbo-cache-control: no-cache`, so Back to it asks for the check again. The three head actions fit one line at 360px. Spec 069 (#256) renders the shape panel when the deck's collectible draws one: the `details` copy, then the `aside` copy, then the zones, inside `c-deck` (the core partial `decks/_shape` draws each copy's frame, the extension's partial the three parts), so the second column holds the panel from 992px; a deck whose collectible draws none renders neither copy and leaves the column empty. The page's `main` carries `c-deckpage`, whose only rules let the head's name wrap anywhere (`overflow-wrap:anywhere`, `min-width:0`) so a 60-character deck name with no space stays inside a 360px screen. A group is its own `tbody` whose first row is the `th scope="rowgroup"` group row (a collectible that groups nothing gets one `tbody` and no group row) +**As built (specs 068, 069 and 070, #254, #256 and #257, on spec 066's page):** the page head's line reads `Format · N cards · N owned · N proxy`; a zone's head is its name, `N cards · N owned · N proxy` and "Add a proxy" (a link to the add-a-proxy pages, `AddProxy`); the table has Card, Cost, Owned, Proxy, Total and the row menu; a row with proxies shows the `ProxyTag` in its Proxy cell and in the line under its name on a phone, and its menu lists "Use my copy", "Add a new copy", "Change proxy count" and, after a separator, "Remove proxy" (spec 070, #257; the finish named in each item when the row has proxies of two finishes); a row with no proxy has an empty menu cell; a printing with a proxy and no owned copy is a row of Owned 0; Total is owned plus proxy and a group row's count is its Total; the card name links to the card page at its copies (`/catalog/entries/:key?from=collection#copies`). The page has no `form` (every action is a link). Spec 079 (#258) adds "Check deck" to the head's actions, after "Edit deck" and before the "…" `Menu`: a secondary button that is a link (a GET to `/decks/:id/check`, so the page still has no `form`), with `data-turbo-prefetch="false"` so hovering it never runs a check nobody asked for (spec 079 decision 7). Its page is the deck page with the results, a `DeckCheck` section between the head and `c-deck`, so a phone reads them first; the head, the panel and the zones are drawn as on the plain page, the button stays and checks again, and the page carries `turbo-cache-control: no-cache`, so Back to it asks for the check again. The three head actions fit one line at 360px. Spec 069 (#256) renders the shape panel when the deck's collectible draws one: the `details` copy, then the `aside` copy, then the zones, inside `c-deck` (the core partial `decks/_shape` draws each copy's frame, the extension's partial the three parts), so the second column holds the panel from 992px; a deck whose collectible draws none renders neither copy and leaves the column empty. The page's `main` carries `c-deckpage`, whose only rules let the head's name wrap anywhere (`overflow-wrap:anywhere`, `min-width:0`) so a 60-character deck name with no space stays inside a 360px screen. A group is its own `tbody` whose first row is the `th scope="rowgroup"` group row (a collectible that groups nothing gets one `tbody` and no group row) **What the page provides**: the deck's name, its format and counts, its zones with their rows, and the extension's contribution. The shell's rules below name no format and no game. The extension supplies the shape panel (`DeckShape`), the Cost column (`ManaCost` or a muted dash), the legality marker and the groups of a zone's rows; the sample's groups are a Magic deck's top-level card types. From 8c64f8ff83e3ecd215bb95018de241c9fa0e9a85 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:47:29 -0500 Subject: [PATCH 21/24] docs(decks): record the implementation review and close its follow-ups (#258) Records the Mode B review (SPEC-ALIGNED, round 1, HEAD 4144d1fe) and how each of its four follow-ups was resolved; names the new AC-3.1 and AC-4.5 proofs and the core's Considering examples; rewrites mutation 7's Considering entry, re-run after the core change, and adds mutation 8; the index's 079 row says it does 069's "deck rules" non-goal in part. Co-Authored-By: Claude Opus 5.5 --- docs/specs/079-deck-checks/verification.md | 46 ++++++++++++++++------ docs/specs/README.md | 2 +- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/docs/specs/079-deck-checks/verification.md b/docs/specs/079-deck-checks/verification.md index 0fc1e680..3db7f455 100644 --- a/docs/specs/079-deck-checks/verification.md +++ b/docs/specs/079-deck-checks/verification.md @@ -2,7 +2,7 @@ **Spec:** [spec.md](spec.md) (v0.2.1) · **Plan:** [plan.md](plan.md) (present, retirement owed after the merge) · **Issue:** [#258](https://github.com/plainprogrammer/Collector/issues/258) -Each acceptance criterion with the spec example (or the document check) that proves it. A path is a spec file; a quoted name is the example's title. Documents are verified by reading, never by RSpec (`.claude/rules/testing.md`). The feature's spec files were run together on 2026-11-12 (`bin/rspec spec/models/catalog/deck_checks_spec.rb spec/models/mtg/deck_rules_spec.rb spec/models/mtg/deck_checks_for_magic_spec.rb spec/requests/deck_checks_spec.rb spec/system/deck_checks_spec.rb spec/deck_core_spec.rb spec/models/deck/page_spec.rb`): 81 examples, 0 failures. The eleven magic examples added afterwards for the ACs below that had lacked a proof with the spec's own numbers (`848c6546`) were run with `bin/rspec spec/models/mtg spec/models/catalog spec/deck_core_spec.rb`: 806 examples, 0 failures. `bin/ci` is T5.3's and is not recorded here. +Each acceptance criterion with the spec example (or the document check) that proves it. A path is a spec file; a quoted name is the example's title. Documents are verified by reading, never by RSpec (`.claude/rules/testing.md`). The feature's spec files were run together on 2026-11-12 (`bin/rspec spec/models/catalog/deck_checks_spec.rb spec/models/mtg/deck_rules_spec.rb spec/models/mtg/deck_checks_for_magic_spec.rb spec/requests/deck_checks_spec.rb spec/system/deck_checks_spec.rb spec/deck_core_spec.rb spec/models/deck/page_spec.rb`): 81 examples, 0 failures. The eleven magic examples added afterwards for the ACs below that had lacked a proof with the spec's own numbers (`848c6546`) were run with `bin/rspec spec/models/mtg spec/models/catalog spec/deck_core_spec.rb`: 806 examples, 0 failures. After the implementation review's follow-ups (`56b26fba`: two core examples and two magic examples), `bin/rspec spec/models/catalog spec/models/mtg spec/deck_core_spec.rb`: 810 examples, 0 failures; after `178a00ec`, `bin/rspec spec/deck_core_spec.rb spec/requests/deck_checks_spec.rb spec/requests/deck_pages_spec.rb`: 61 examples, 0 failures. `bin/ci` is T5.3's and is not recorded here. Short names used below: **core** `spec/models/catalog/deck_checks_spec.rb`; **rules** `spec/models/mtg/deck_rules_spec.rb`; **magic** `spec/models/mtg/deck_checks_for_magic_spec.rb`; **request** `spec/requests/deck_checks_spec.rb`; **system** `spec/system/deck_checks_spec.rb`. @@ -30,13 +30,13 @@ Short names used below: **core** `spec/models/catalog/deck_checks_spec.rb`; **ru | AC-2.2 | magic "counts a proxy toward Main and passes at 60 and at 15" (60 passes, Standard); the 61 case: core "judges a minimum with the true count in every sentence" (a registered test format, not Magic's); request "words no findings, one finding and several (AC-1.7)" (60 Plains in Standard: "No findings") | | AC-2.3 | magic "judges a Modern Sideboard Over at 16 and passes it at 15 and at none (AC-2.3)" (three Modern decks: "Over" "Sideboard: 16 cards; Modern allows at most 15.", "Pass" at 15 and at 0); also magic "judges Main and Sideboard in each constructed format's own name" (Modern among the six, Sideboard 16) | | AC-2.4 | magic "judges Commander's two zones together" (99 Short, 101 Over) and "passes Commander at exactly 100 while cards in Considering count nowhere (AC-2.4, 2.5)" | -| AC-2.5 | magic "passes Commander at exactly 100 while cards in Considering count nowhere (AC-2.4, 2.5)"; core "counts nothing in Considering or an unnamed zone, and passes at exactly the exact size" | +| AC-2.5 | magic "passes Commander at exactly 100 while cards in Considering count nowhere (AC-2.4, 2.5)"; core "counts nothing in Considering or an unnamed zone, and passes at exactly the exact size" and "counts nothing in Considering even when a rule names it, and still names it in the subject (decision 3)" (a rule naming Considering: "Main + Considering: 3 cards; tight needs at least 5.", its 10 cards not counted) | | AC-2.6 | magic "counts a proxy toward Main and passes at 60 and at 15" (59 owned and 1 proxy); core "judges a minimum with the true count in every sentence" | | AC-2.7 | magic "says Casual and an unregistered format have no rules but still flag a forgotten proxy"; request "gives a deck with no rules the no-rules line and the owned-plus-proxy result (AC-2.7)"; core "says the format has no rules and still runs owned-plus-proxy" | | AC-2.8 | magic "is Short at a 59-card Main in Pauper, Pioneer, Legacy and Vintage, each in its own name (AC-2.8)" ("Short" "Main: 59 cards; Pauper needs at least 60.", and so for Pioneer, Legacy and Vintage) | | AC-2.9 | magic "leaves out another collectible's cards" ("Main: 0 cards; Standard needs at least 60."); system "asks for the check again on Back instead of restoring a Turbo snapshot (AC-1.8)" (an empty Standard deck reads "Main: 0 cards") | | AC-2.10 | magic "leaves out another collectible's cards"; core "leaves out another collectible's cards and zones no rule names" | -| AC-2.11 | magic "flags owned and proxy in Considering while no size or copy count includes it (AC-2.11)" (a Standard deck's Considering, a zone row Magic's rules don't name, holds 5 owned Lightning Bolt and 1 proxy: Main 0 Short, Sideboard 0 Pass, "No card is over its limit.", and "Lightning Bolt in Considering: 5 owned, 1 proxy."); core "leaves out another collectible's cards and zones no rule names" (a `stash` zone's 7 cards in no size count) and "passes at the limit and counts no copy in an unnamed zone" (copies) | +| AC-2.11 | magic "flags owned and proxy in Considering while no size or copy count includes it (AC-2.11)" (a Standard deck's Considering, a zone row Magic's rules don't name, holds 5 owned Lightning Bolt and 1 proxy: Main 0 Short, Sideboard 0 Pass, "No card is over its limit.", and "Lightning Bolt in Considering: 5 owned, 1 proxy."); core "leaves out another collectible's cards and zones no rule names" (a `stash` zone's 7 cards in no size count) and "passes at the limit and counts no copy in an unnamed zone" (copies); core "counts nothing in Considering even when a rule names it, and still names it in the subject (decision 3)" and "counts no copy in Considering even when the rule names it (decision 3)" (Considering is counted by no rule even when the vocabulary names it, `56b26fba`) | | AC-2.12 | magic "names a Sideboard the deck has no zone row for from the registration (AC-2.12)"; core "names a rule's zones in its key order and a zone the deck lacks by its key" (the fallback to the key) | | AC-2.13 | magic "judges Commander's two zones together" ("Commander + Main: …"); core "names a rule's zones in its key order and a zone the deck lacks by its key" | @@ -44,14 +44,14 @@ Short names used below: **core** `spec/models/catalog/deck_checks_spec.rb`; **ru | AC | Proved by | |---|---| -| AC-3.1 | magic "adds a card's copies across printings, zones, owned and proxy" ("Lightning Bolt: 5 copies; the limit is 4."); core "names a card over its limit across zones, printings and owned plus proxy"; request "words no findings, one finding and several (AC-1.7)" (5 Shock in Main is a finding). No example asserts the sentence for 5 copies of one printing in Main alone | +| AC-3.1 | magic "names a Standard deck's 5 copies of one card in Main over the limit of 4 (AC-3.1)" (one printing, 5 owned in Main: "Over the limit" "Lightning Bolt: 5 copies; the limit is 4."; `56b26fba`, mutation 8); also magic "adds a card's copies across printings, zones, owned and proxy"; core "names a card over its limit across zones, printings and owned plus proxy"; request "words no findings, one finding and several (AC-1.7)" (5 Shock in Main is a finding) | | AC-3.2 | magic "counts a Modern deck's Main and Sideboard together: 3 + 2 is over, 3 + 1 passes (AC-3.2)" ("Lightning Bolt: 5 copies; the limit is 4."; at 3 + 1 "No card is over its limit."); core "passes at the limit across two named zones (4 of 3 + 1)" | | AC-3.3 | magic "counts a Legacy deck's 4 of one printing and 1 of another as 5 copies (AC-3.3)" and "counts a Legacy deck's 3 owned and 2 proxy of one printing as 5 copies (AC-3.3)" (each "Lightning Bolt: 5 copies; the limit is 4."); also magic "adds a card's copies across printings, zones, owned and proxy" and core "names a card over its limit across zones, printings and owned plus proxy" | | AC-3.4 | magic "exempts basic lands and any-number cards, and holds Commander to one of the rest" (20 Plains in Standard; 2 Snow-Covered Island in Commander); rules "is none for a basic land, Snow-Covered ones included" | | AC-3.5 | magic "exempts basic lands and any-number cards, and holds Commander to one of the rest" (9 Relentless Rats); rules "is none for any-number text, in every format" | | AC-3.6 | magic "exempts basic lands and any-number cards, and holds Commander to one of the rest" (Krenko in Commander and Main: "2 copies; the limit is 1."; two Snow-Covered Islands no finding) | | AC-3.7 | magic "restricts a card to one in Vintage only" (Vintage "Black Lotus: 2 copies; restricted to 1."; Legacy passes at 2); rules "is 1, restricted, in Vintage only" (Legacy's limit is 4) and "is 4 in constructed formats and 1 in Commander" (an unrestricted card in Vintage is 4) | -| AC-3.8 | core "passes at the limit and counts no copy in an unnamed zone" (9 copies in Considering not counted) | +| AC-3.8 | core "passes at the limit and counts no copy in an unnamed zone" (9 copies in Considering not counted) and "counts no copy in Considering even when the rule names it (decision 3)" | | AC-3.9 | core "orders findings by first zone, then name, then identity; leaves out another collectible" and "orders two findings of one zone by name, then by identity when the names match" | | AC-3.10 | magic "exempts basic lands and any-number cards, and holds Commander to one of the rest" and "restricts a card to one in Vintage only" ("No card is over its limit."); request "shows the summary and the results between the head and the zones (AC-1.2, 1.7, 1.8, 1.9)" | | AC-3.11 | magic "leaves out another collectible's cards" (70 copies of another collectible's card, "No card is over its limit."); core "orders findings by first zone, then name, then identity; leaves out another collectible" | @@ -69,7 +69,7 @@ Short names used below: **core** `spec/models/catalog/deck_checks_spec.rb`; **ru | AC-4.2 | core "flags a zone that holds both, across printings, naming the split" (owned of one entry, proxy of another of the same identity) | | AC-4.3 | core "does not flag owned only, proxy only, owned in one zone with a proxy in another, or another collectible" and "judges each zone alone, includes zones no rule names, and skips Considering's lone proxy" | | AC-4.4 | core "does not flag owned only, proxy only, owned in one zone with a proxy in another, or another collectible" | -| AC-4.5 | magic "says Casual and an unregistered format have no rules but still flag a forgotten proxy" (Casual's finding; for the unregistered format the example asserts the no-rules line only); request "gives a deck with no rules the no-rules line and the owned-plus-proxy result (AC-2.7)"; core "says the format has no rules and still runs owned-plus-proxy" (a type and format the instance doesn't register: the finding is shown) | +| AC-4.5 | magic "says Casual and an unregistered format have no rules but still flag a forgotten proxy" (Casual's finding) and "says a deck whose format is no longer registered has no rules and still flags its owned and proxy card (AC-4.5)" (a Standard deck changed to `retired`: the lines equal exactly `[ nil, "retired has no rules to check." ]` and `[ "Owned and proxy", "Lightning Bolt in Main: 2 owned, 1 proxy." ]`; `56b26fba`, mutation 8); request "gives a deck with no rules the no-rules line and the owned-plus-proxy result (AC-2.7)"; core "says the format has no rules and still runs owned-plus-proxy" (a type and format the instance doesn't register: the finding is shown) | | AC-4.6 | magic "flags owned and proxy in Main and in Sideboard once each, with that zone's split (AC-4.6)" (the Owned and proxy lines equal exactly "Lightning Bolt in Main: 2 owned, 1 proxy." and "Lightning Bolt in Sideboard: 1 owned, 3 proxy."); core "judges each zone alone, includes zones no rule names, and skips Considering's lone proxy" | | AC-4.7 | core "does not flag owned only, proxy only, owned in one zone with a proxy in another, or another collectible"; request "shows the summary and the results between the head and the zones (AC-1.2, 1.7, 1.8, 1.9)" | | AC-4.8 | request "changes nothing: no write reaches lots, proxies or locations (AC-4.8)" | @@ -100,13 +100,13 @@ Short names used below: **core** `spec/models/catalog/deck_checks_spec.rb`; **ru | AC | Checked | |---|---| | AC-7.1 | `docs/architecture/collection-and-lots.md` (commit `32b86d92`): the Check deck paragraph, the seam paragraph pointing to `Catalog::DeckChecks`' comment for the contract, the bullets (what is counted, owned plus proxy, Magic's rules, the order), the words table, the paragraph saying spec 069's non-goal "deck rules" is done in part; the head's actions, the fourth vocabulary answer, the deck guard's count (54 corrected to 68, 071's and 073's files were missing), the statement-count invariant, four Named constants rows, a decision, the accepted-limits lines, Sources and the issues list. Each statement carries its spec 079 decision or AC. Grepped before for "deck rules", "no rules", "Check deck", "Non-Goal": no statement had been made stale but the guard's count | -| AC-7.2 | `docs/specs/README.md` row 079 (written with the spec): names collection and lots and `DeckPage.md`, "Nothing: it does the 'deck rules' that 069 left as a non-goal" | -| AC-7.3 | Comments read: `Catalog::DeckChecks` (the contract in its header, `@param`/`@return` on `.report`), `MTG::DeckRules` (`.for`, `.limit_for`), `MTG::Collecting.deck_rules`, `Decks::ChecksController`. `bin/yard doc --no-cache` builds with 0 warnings (run 2026-11-12 after T5.1). The comment ratchet runs in T5.3's `bin/ci`; it was not run here | -| AC-7.4 | `docs/design-system/components/DeckPage.md` ("Spec 079 (#258) adds 'Check deck'…": the link, the `DeckCheck` section between head and `c-deck`, still no `form`, the no-cache tag) and the new `DeckCheck.md` | +| AC-7.2 | `docs/specs/README.md` row 079 (written with the spec, its Supersedes cell reworded after the implementation review): names collection and lots and `DeckPage.md`, "Nothing. It does in part (size and copy limits) the 'deck rules' that 069 left as a non-goal" | +| AC-7.3 | Comments read: `Catalog::DeckChecks` (the contract in its header, which since `56b26fba` says the core never counts Considering for a size or copy rule even when a rule names it; `@param`/`@return` on `.report`), `MTG::DeckRules` (`.for`, `.limit_for`), `MTG::Collecting.deck_rules`, `Decks::ChecksController`. `bin/yard doc --no-cache` builds with 0 warnings (run 2026-11-12 after T5.1). The comment ratchet runs in T5.3's `bin/ci`; it was not run here | +| AC-7.4 | `docs/design-system/components/DeckPage.md` ("Spec 079 (#258) adds 'Check deck'…": the link, the `DeckCheck` section between head and `c-deck`, still no `form`, the no-cache tag; since `178a00ec` also why the link turns prefetch off) and the new `DeckCheck.md` | | AC-7.5 | `docs/roadmap.md` theme C, item 14n: #258 "Built 2026-11-12, done when its pull request merges", as the theme's other built items are marked before their merge | | AC-7.6 | `CLAUDE.md` Project Status: "A collector checks a deck on demand from its page, … (`Catalog::DeckChecks`, `MTG::Collecting.deck_rules`, `Decks::ChecksController`, spec 079)"; and the collection-and-lots guard sentence names the `Catalog::DeckChecks` seam. No rule of the area doc restated | -**ACs without a full proof:** none. AC-2.3, 2.8, 2.11, 3.2, 3.3, 3.14, 3.15, 4.6 and 4.9 are proved on Magic decks with the formats and numbers their wording names (`848c6546`); AC-4.9's proxy of another collectible is a fixture that skips `Deck::Proxy`'s validation, since the app refuses to make one. +**ACs without a full proof:** none. AC-2.3, 2.8, 2.11, 3.2, 3.3, 3.14, 3.15, 4.6 and 4.9 are proved on Magic decks with the formats and numbers their wording names (`848c6546`), and AC-3.1 and AC-4.5 since `56b26fba`; AC-4.9's proxy of another collectible is a fixture that skips `Deck::Proxy`'s validation, since the app refuses to make one. ## Supersession grep @@ -153,15 +153,19 @@ Each was made by hand, run, and reverted before its commit. - `SIDEBOARD_MAXIMUM` 15 → 16 failed 5: "judges a Modern Sideboard Over at 16 and passes it at 15 and at none (AC-2.3)", "judges Main and Sideboard in each constructed format's own name", "counts a proxy toward Main and passes at 60 and at 15", "names a Sideboard the deck has no zone row for from the registration (AC-2.12)", and the AC-2.11 example. - `CONSTRUCTED_COPIES` 4 → 5 failed 8: "counts a Modern deck's Main and Sideboard together: 3 + 2 is over, 3 + 1 passes (AC-3.2)", "counts a Legacy deck's 4 of one printing and 1 of another as 5 copies (AC-3.3)", "counts a Legacy deck's 3 owned and 2 proxy of one printing as 5 copies (AC-3.3)", "holds 5 copies of a printing with no card data in Standard to the ordinary limit (AC-3.14)", "gives a card in 5 copies in Main and 5 in Sideboard exactly one finding of 10 (AC-3.15)", "adds a card's copies across printings, zones, owned and proxy", "names a card by its English name when its first row in page order is a Japanese printing", "counts a basic land's two printings, one with no card data, at the ordinary limit (AC-3.14, 3.16)". - `COMMANDER_COPIES` 1 → 2 failed 2: "holds a Commander deck's basic land with no card data to one copy (AC-3.14)" and "exempts basic lands and any-number cards, and holds Commander to one of the rest". - - The constructed copy rule's zone keys with `considering` added failed 1: the AC-2.11 example. + - The constructed copy rule's zone keys with `considering` added failed 1 at `848c6546`: the AC-2.11 example. That failure showed the core counted whatever zones a rule named, so the implementation review asked the core to enforce decision 3 itself. Since `56b26fba` the core never counts Considering for a size or copy rule, even when a rule names it, so this mutation no longer fails an example: re-run after that commit (the key added to `MTG::DeckRules`' constructed copy rule, `bin/rspec spec/models/mtg/deck_checks_for_magic_spec.rb`), it stayed green, 26 examples, 0 failures, and the file was restored with `git checkout --`. What proves the rule now is the two core examples "counts nothing in Considering even when a rule names it, and still names it in the subject (decision 3)" and "counts no copy in Considering even when the rule names it (decision 3)", written red before the change: `expected: ["Short", "Main + Considering: 3 cards; tight needs at least 5."]` `got: ["Pass", "Main + Considering: 13 cards; tight needs at least 5."]`, and `expected: ["Pass", "No card is over its limit."]` `got: ["Over the limit", "Lightning Bolt: 6 copies; the limit is 4."]`. - `copy_lines` grouping by `it.row.entry.id` instead of `catalog_identity_id` (one finding per printing) failed 6: the AC-3.15 example, the first AC-3.3 example, "adds a card's copies across printings, zones, owned and proxy", "names a card by its English name when its first row in page order is a Japanese printing", "exempts basic lands and any-number cards, and holds Commander to one of the rest", "counts a basic land's two printings, one with no card data, at the ordinary limit (AC-3.14, 3.16)". - `split_lines` grouping by `[ 0, catalog_identity_id ]` (the zone's position ignored) failed 1: "flags owned and proxy in Main and in Sideboard once each, with that zone's split (AC-4.6)". - `counted_rows` without its `collectible_type == deck_type` filter failed 2: "does not flag another collectible's owned copy and proxy in a Magic deck's Main (AC-4.9)" and "leaves out another collectible's cards". +8. For the two magic examples of `56b26fba`, each mutation made in the working tree, the one example run, and the file restored exactly: + - `MTG::DeckRules::CONSTRUCTED_COPIES` 4 → 5 failed "names a Standard deck's 5 copies of one card in Main over the limit of 4 (AC-3.1)" (the lines did not include the finding); restored with `git checkout --`. + - `Catalog::DeckChecks.report` building `Report.new(lines: rules ? lines + split_lines(counted) : lines)` (no owned-plus-proxy check for a deck with no rules) failed "says a deck whose format is no longer registered has no rules and still flags its owned and proxy card (AC-4.5)" (got only `[nil, "retired has no rules to check."]`); the file held uncommitted work, so it was restored from a copy and compared byte for byte with `cmp`. **Characterisations** (examples that passed on arrival, because the behaviour was already built): - T2.2's 13 magic examples (`eb7156f4`). Proof of failing: mutations 1 and 7 fail 8 of them (lines 23, 32, 59, 70, 79, 93, 130, 138). The other 5 ("judges Commander's two zones together", "says Casual and an unregistered format have no rules but still flag a forgotten proxy", "passes Commander at exactly 100 while cards in Considering count nowhere (AC-2.4, 2.5)", "holds a numbered card to its number in every format, and passes at that number", "restricts a card to one in Vintage only") have no recorded mutation of their own. - The 11 magic examples of `848c6546`. Proof: mutation 7, which fails each of them. +- The 2 magic examples of `56b26fba` (AC-3.1, AC-4.5). Proof: mutation 8. - T3.2's statement-count example (`81c5cf96`). Proof: mutation 2. - T4.2's system examples (`d0acb71d`): focus (mutation 5), Back (mutation 4), status words (mutation 6, after `62e1bc82`), the phone-width examples (mutations 3 and 6). The reading-order example ("puts the results before the zones in reading order (AC-6.3)") has no recorded mutation proof. @@ -198,6 +202,11 @@ e. T4.3's screenshots came from a throwaway system spec rather than a dev server `git log --oneline c3589b30..HEAD` at this record's commit (newest first; this record's own commit follows): ``` +178a00ec docs(decks): say why Check deck turns prefetch off (#258) +56b26fba fix(decks): the core never counts Considering for a size or copy rule (#258) +4144d1fe docs(decks): verification record names the new proofs (#258) +848c6546 test(decks): prove the Magic copy, size and split ACs with the spec's own numbers (#258) +03e52d79 docs(decks): verification record for the deck checks (#258) 32b86d92 docs(decks): document the deck checks in the area doc, CLAUDE.md and the roadmap (#258) ec3260d4 docs(decks): screenshots and walkthrough for the deck checks (#258) 62e1bc82 test(decks): make the browser examples fail when their selectors match nothing (#258) @@ -213,3 +222,18 @@ afd799f3 feat(decks): copy-limit results for the deck checks (#258) 5362d866 feat(decks): size results for the deck checks (#258) b2ee0f10 feat(decks): add Catalog::DeckChecks and its owned-plus-proxy check (#258) ``` + +## Implementation review + +SPEC-ALIGNED (Opus, Mode B, round 1, HEAD 4144d1fe) + +The reviewer found no behaviour drift and no missing AC: all of AC-1.1 to AC-7.6 Covered. It re-ran the feature's 69 model, request and guard examples and its 35 browser examples, all green. `bin/ci` ran green on `4144d1fe`: 4610 examples, 0 failures, 1 pending that predates the feature (the art-agreement spec, which needs `COLLECTOR_ART_AGREEMENT`). + +Its four follow-ups, and how each was resolved: + +1. The header contract of `Catalog::DeckChecks` said Considering is counted by no size or copy rule, but the core filtered by each rule's zone keys only, so it held only because Magic's rules never name Considering. Fixed in `56b26fba`: the core leaves Considering out of every size and copy count even when a rule names it (decision 3), the owned-plus-proxy check still judges it (decision 5), the comment says exactly that, and two core examples prove it (red before the change; mutation 7). +2. Why Check deck turns Turbo's prefetch off was written only in the plan, which retires after the merge. Fixed in `178a00ec`: a comment above the link in `app/views/decks/show.html.erb`, a sentence in the Check deck paragraph of `collection-and-lots.md` and one in `DeckPage.md`, each citing spec 079 decision 7. +3. This record: AC-3.1 had no example asserting its exact sentence, and AC-4.5's unregistered-format case asserted only the no-rules line, so "ACs without a full proof: none" was not true. Closed by the two magic examples of `56b26fba` (mutation 8) and the rows above; mutation 7's Considering entry is rewritten to say what it now shows. +4. The index's Supersedes cell for 079 now says it does the "deck rules" of 069's non-goal in part (size and copy limits) and supersedes nothing. + +The reviewer did not re-run the supersession grep, YARD or the comment ratchet (`bin/ci` did), and did not open the walkthrough GIF. diff --git a/docs/specs/README.md b/docs/specs/README.md index 78865c85..f4bd397c 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -94,4 +94,4 @@ A plan is deleted from the tree in a commit of its own after its feature's pull | [073 "Free to use" and "In decks" on the Card Page](073-card-page-deck-stats/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) and [catalog core](../architecture/catalog-core.md) (the card page's stats and "In decks" section; the visitor rule and the card page's order); the card page's markup is `ItemPage` in the [design system](../design-system/README.md) | [Collection and lots](../architecture/collection-and-lots.md), [catalog core](../architecture/catalog-core.md), [design system](../design-system/README.md) (`ItemPage`, `ItemPagePhone`, `SingleStat`) | retired in `1bdb1c62a908b76131f1eb2a94f769b419a78689`; `git show 1bdb1c62a908b76131f1eb2a94f769b419a78689^:docs/specs/073-card-page-deck-stats/plan.md` | In part 004 FR-9 (the single stat; no "In decks" or "Free to use"), its Non-Goal and Out of Scope entries and AC-8.1's order; 071 AC-1.4 and its "where #50 sits" note (fulfilled); fulfils the Non-Goals that name #50 (029, 032, 033, 057, 070) and 057 decisions 7 and 8 (the strip and the section) | | | [077 The Deck Shape Panel Stays Tidy at Extreme Counts](077-deck-shape-extreme-counts/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph and Named constants); the panel's markup is `DeckShape` in the [design system](../design-system/README.md) | [Design system](../design-system/README.md) (`DeckShape`), [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph, Named constants) | retired in `a54b85eeb63a0ac540bd38b88747c79a0c2d47b4`; `git show a54b85eeb63a0ac540bd38b88747c79a0c2d47b4^:docs/specs/077-deck-shape-extreme-counts/plan.md` | In part 060 decision 2, AC-2.2, AC-2.4, AC-2.5 and AC-2.6 (the visible numbers, the types' alignment) and 069 decision 14, its four-digit-count Error Scenarios row and Findings 1 to 4 ([#320](https://github.com/plainprogrammer/Collector/issues/320)) | In part by [078](078-mana-bar-label-fit/spec.md) (decision 2's documented limit: the mana-label band, [#330](https://github.com/plainprogrammer/Collector/issues/330)) | | [078 A Mana Bar Label Fits Its Segment or Isn't Drawn](078-mana-bar-label-fit/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph); the panel's markup and CSS rule are `DeckShape` in the [design system](../design-system/README.md) | [Design system](../design-system/README.md) (`DeckShape`), [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph) | retired in `b275dc7e20ef03355d825eec48569c2eb7ce8df0`; `git show b275dc7e20ef03355d825eec48569c2eb7ce8df0^:docs/specs/078-mana-bar-label-fit/plan.md` | In part 077 decision 2 (the mana-label band as a documented limit, ruled A on 2026-11-11), its Goal 1's qualification, Open Questions, Error Scenarios row for the band, AC-5.1 and AC-5.3, and its Supersedes row's exception for 060 AC-2.4 ([#330](https://github.com/plainprogrammer/Collector/issues/330)); 060 decision 4 and 069 decision 9 are kept | | -| [079 Deck Checks (Deck Size, Copy Limits, and Owned Plus Proxy of One Card)](079-deck-checks/spec.md) | Approved ([#258](https://github.com/plainprogrammer/Collector/issues/258)) | [Collection and lots](../architecture/collection-and-lots.md) (decks and the deck page), [DeckPage](../design-system/components/DeckPage.md) | present | Nothing: it does the "deck rules" that 069 left as a non-goal | | +| [079 Deck Checks (Deck Size, Copy Limits, and Owned Plus Proxy of One Card)](079-deck-checks/spec.md) | Approved ([#258](https://github.com/plainprogrammer/Collector/issues/258)) | [Collection and lots](../architecture/collection-and-lots.md) (decks and the deck page), [DeckPage](../design-system/components/DeckPage.md) | present | Nothing. It does in part (size and copy limits) the "deck rules" that 069 left as a non-goal | | From 1257c4c4ea263073ae6b671d0e62d6459a01f152 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:58:29 -0500 Subject: [PATCH 22/24] docs(architecture): distil spec 079 into collection-and-lots The area doc already carried the deck checks from T5.1. Distillation adds the glossary's check, finding and pass (spec 079 Terms); says the core leaves Considering out of a size or copy count even when a rule names its key (decision 3, the review's first follow-up, 56b26fba); corrects the deck page's count of optional vocabulary answers to three; and adds the Distillation log row. The index marks 079 frozen by the area doc, its results section's rule DeckCheck in the design system, plan retirement owed. Co-Authored-By: Claude Sonnet 5.5 --- docs/architecture/collection-and-lots.md | 6 ++++-- docs/specs/README.md | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/architecture/collection-and-lots.md b/docs/architecture/collection-and-lots.md index 0e04a5e6..3c51633e 100644 --- a/docs/architecture/collection-and-lots.md +++ b/docs/architecture/collection-and-lots.md @@ -55,6 +55,7 @@ What a collector owns and where it is kept: lots and their identity, the ways co - **Number entry / list / line / line key:** the "Add by number" page; the account's added lines until Done; one submitted number, quantity, finish and place; the random token that makes a line add at most once (spec 026 Terms, decision 10). - **Holdings:** what an account's lots hold in one location: lots and copies; a deck holds what its zones hold together (spec 032 P1, P2; spec 062 AC-3.3). - **Collection mark:** `from=collection` on a card page URL, which makes its breadcrumb and current section Collection (spec 004 AC-8.9). +- **Check / finding / pass:** a check is one question Check deck puts to a deck (its size, its copy limits, its owned-plus-proxy cards); a finding is a check's answer that something is wrong; a check that finds nothing passes. A rule's zones are the zone keys the extension names for it; a zone no rule names is unnamed (spec 079 Terms). ## Invariants @@ -132,7 +133,7 @@ The Decks section is the second link of the header and of the tab bar, after Col `GET /decks/:id` is the deck page (spec 066, #253; the markup is `DeckPage`): the id is the deck's location id and another account's deck, a place's id, a zone's id and an id that is not digits only are 404 ([tenancy and accounts](tenancy-and-accounts.md); AC-1.2). It reads only, through `Deck::Page`, and adds no way to add or move copies (AC-4.2). The head reads " · 69 cards · 58 owned · 11 proxy" (every zone, Considering included; cards are owned plus proxy) and a zone's head "12 cards · 12 owned · 0 proxy" with "Add a proxy", a link, at its end (spec 068 decisions 5, 7, AC-2.3). The zones are in the format's registered order, a zone the format does not list and every zone of an unregistered format after them by id, so a zone a change of format added later still reads Main, Sideboard, Considering (decision 3). A zone is one table, one row per printing: its lots of every finish, condition and price are summed into Owned (decision 2); the columns are Card, Cost, Owned, Proxy, Total and the row menu: a row with proxies shows the Proxy tag (the sum of its finishes' proxies) and a menu whose items are links, in this order: "Use my copy", "Add a new copy", "Change proxy count", a separator and "Remove proxy" (danger, last); a row with proxies of two finishes has each item for each finish, the finish named as the vocabulary labels it and the items grouped by verb ("Use my copy for Foil proxy", "Use my copy for Nonfoil proxy", then both "Add a new copy for …", both Change items, the separator and both Remove items), and a proxy whose printing's collectible the instance no longer registers has Change and Remove only (spec 070 decision 1, AC-1.1–AC-1.4, which supersede spec 068 decision 5 and AC-2.6), Total is owned plus proxy, a printing with a proxy and no copy is a row of Owned 0, and a row with no proxy has an empty menu cell (spec 068 decisions 5, 8, AC-2.1, AC-2.6, AC-2.7's "no `form`" and "no Move copies"; spec 066 decision 7 and 9 stand for the rest), and below 640px of the table's container Owned and Proxy are hidden (spec 060 decision 9, decision 7). Rows are grouped by the collectible's groups and ordered by the shown name, set code, collector number as a person reads it (`Catalog::Entry::NUMBER_HAS_DIGITS_FIRST`, `NUMBER_VALUE`), number text and language (decision 3, `Catalog::Entry::DISPLAY_NAME_SQL`). The card's name links to the card page in the collection's context at its copies, where Move copies per lot lives (decisions 9, 10). `Deck::Page#zones` runs at most 6 statements (3 for a deck with no lot and no proxy: the zones, the lot sums, the proxies, then the entries, their sets and one extension read per type; spec 068 AC-2.10) and the controller 2 more for the deck, however many zones, rows, lots and proxies there are (AC-2.10). A deck whose collectible draws a shape panel shows it in the right column, in two copies before the zones; its rules are in the two paragraphs after this one (spec 069, #256). It supersedes spec 066 AC-1.6's "no shape panel" and AC-5.1's "the second column is empty" for such a deck; a deck whose collectible draws none still leaves the column empty (spec 060 AC-3.12). The head's actions are Edit deck, Check deck and a "…" menu of Change format and Remove deck; Check deck's page is the deck page with the check's results, below (spec 079 decision 8, #258). -The page's collectible part is two optional answers of the collecting vocabulary, after its first, `formats`: `deck_card(entry, format_key)` returns a `Catalog::DeckCards::Answer` (group, position, legality word, whether a `deck_cost` partial draws the cost); the contract is written in `Catalog::DeckCards`' comment, and a vocabulary that answers nothing gets one flat table, a muted dash and no marker (spec 066 decision 5, AC-6.1). Magic groups by the first face's type line: a Creature is a Creature, otherwise a Land is a Land, otherwise the first of Planeswalkers, Battles, Instants, Sorceries, Artifacts, Enchantments, then Other (decision 6, `MTG::DeckGroups`); the cost is the first face's (decision 11, `mtg/printings/_deck_cost`); the legality marker is the printing's stored legality in the deck's own format, nothing for Casual or an unregistered format, "Not legal" for a missing entry, written twice and shown once by width (decision 8, `MTG::Collecting::LEGALITY_FORMATS`). The third answer is `deck_shape(zones)` (spec 069 decision 1, #256) and the fourth `deck_rules(format_key)` (spec 079 decision 1, #258), both below. +The page's collectible part is three optional answers of the collecting vocabulary, after its first, `formats` (the third since spec 079): `deck_card(entry, format_key)` returns a `Catalog::DeckCards::Answer` (group, position, legality word, whether a `deck_cost` partial draws the cost); the contract is written in `Catalog::DeckCards`' comment, and a vocabulary that answers nothing gets one flat table, a muted dash and no marker (spec 066 decision 5, AC-6.1). Magic groups by the first face's type line: a Creature is a Creature, otherwise a Land is a Land, otherwise the first of Planeswalkers, Battles, Instants, Sorceries, Artifacts, Enchantments, then Other (decision 6, `MTG::DeckGroups`); the cost is the first face's (decision 11, `mtg/printings/_deck_cost`); the legality marker is the printing's stored legality in the deck's own format, nothing for Casual or an unregistered format, "Not legal" for a missing entry, written twice and shown once by width (decision 8, `MTG::Collecting::LEGALITY_FORMATS`). The third answer is `deck_shape(zones)` (spec 069 decision 1, #256) and the fourth `deck_rules(format_key)` (spec 079 decision 1, #258), both below. The shape panel is the collecting vocabulary's third answer, after `formats` and `deck_card`: `deck_shape(zones)` takes `Deck::Page#zones` as the page built them and returns a `Catalog::DeckShape::Answer` (`partial`, `figures`, `scope`, `facts`) or nil, and `DecksController#show` asks once through `Catalog::DeckShape.answer`, one line after building the page; a vocabulary without it, a collectible type no longer registered and a nil answer draw no panel and leave the second column empty from 992px; the contract is written in `Catalog::DeckShape`'s comment, which names no collectible, format or zone (spec 069 decision 1, FR-1, AC-1.4; spec 060 AC-3.12). The core partial `decks/_shape` draws each copy's frame (the `aside`'s `section`, `h2` and scope line; the `details`' summary with the facts line and its `c-shape--bare` box) and renders the extension's partial inside, in the `details` copy, then the `aside` copy, before the zones; the figures are built once and rendered twice (decisions 1, 12, AC-1.1, AC-6.3). Magic's answer is `MTG::Collecting.deck_shape` over the figures object `MTG::DeckShape` and its partial `mtg/deck_shapes/_parts`. It counts the Commander and Main zones only (`MTG::DeckShape::COUNTED_ZONES`, in the deck's zone order; Sideboard and Considering never count, and the extension, not the core, names them), so the scope line reads "Main · 60 cards" or "Commander + Main · 100 cards" ("1 card"), a deck with neither zone has no panel, a deck of a format no longer registered keeps its Main zone's panel, and a deck with no card shows the panel's empty states (decisions 2, 10, 14, AC-1.5, AC-1.8, AC-2.8). A card is one copy or one proxy, so owned and proxy count together; a row of another collectible's card (page answer `Catalog::DeckCards::NONE`, no position) is skipped by every part, the scope line's number included (decision 3, AC-3.5). Card types: one type per card, the zone tables' own group (`row.answer.position`, `MTG::DeckGroups`) in the order of `MTG::DeckGroups::NAMES`, only the groups that have a card, each with its count and "N proxy" when it has proxies, so the counts add up to the scope line's number (decision 3, AC-3.1–AC-3.4). The curve counts nonland cards (not in Lands, so Dryad Arbor counts and an Artifact Land does not) whose card has a mana value, from `mtg_cards.mana_value` through the printing's identity (Scryfall's number, spec 058); column `n` from 0 to 5 holds the values from `n` up to but not including `n + 1` (floor, so 0.5 is in 0 and 2.5 in 2) and 6+ holds every value from 6, owned and proxy kept apart (decision 4, AC-2.1–AC-2.3, AC-2.6). The average is the exact mean of the curve's cards (rational arithmetic, so a mean of 0.15 shows 0.2), shown to one decimal rounded half up with the trailing zero kept ("3.0") and thousands delimited, and left out when no card is in the curve (decision 5, AC-2.4). A nonland card with no mana value is left out of the curve, the average and the legend, never counted as 0, and a second muted line under the average says "N nonland cards have no mana value yet." ("1 nonland card has no mana value yet."); the card is still in its type and the scope line (decision 6, AC-2.5). The mana bar counts coloured symbols of the first face's `mana_cost` on the loaded printing, the cost the zone tables show, never the joined `mtg_cards.mana_cost`, for every card in the counted zones, Lands included, each symbol times the card's count: `{W}`, `{U}`, `{B}`, `{R}` and `{G}` count one toward their colour; a hybrid (`{W/U}`) counts one toward each colour; a Phyrexian symbol (`{G/P}`) counts one toward its colour and a Phyrexian hybrid (`{G/W/P}`) one toward each; `{2/W}` and `{C/W}` count one toward the colour only; generic, `{X}`, `{C}`, snow and half symbols (`{HW}`) are never counted (decisions 7, 8, AC-4.1–AC-4.5). Each colour present shows its count and its share of all coloured symbols, each percentage rounded half up on its own so a caption may add to 99 or 101, in W, U, B, R, G order; a segment under 10% of the symbols (`count × 100 < total × 10`) is narrow and shows no label inside, its count staying in the caption; with no coloured symbol the part says "No coloured mana symbols." and an empty Card types part says "No cards yet." (decisions 9, 14, AC-4.6–AC-4.8). The visible number in a curve column's count and in a mana segment's label is abbreviated from 1,000 (`MTG::DeckShape#short_count`: 1,357 is "1.4k", 12,345 "12k", 1,250,000 "1.3m", half up on integers, at most four characters through 999m), because both sit in a slot of fixed width; every other number and every `aria-label` stays exact and delimited; the caption is a run of unbreakable items (`caption_items`, one per colour, so a line never starts with "·"); and the Card types list is a grid whose rows are subgrids, so the counts line up whatever the suffix (spec 077 decisions 1 to 5, #320; it supersedes, in part, spec 060 decision 2, AC-2.2, AC-2.4, AC-2.5 and AC-2.6 (the written-in-full numbers, the fixed 4.5rem minimum, the 279px) and spec 069 decision 14 for those two numbers). A mana label is drawn only if it fits its segment on one line (a segment under 10% never has one, spec 060 decision 4): the browser's own line breaking moves a label that cannot fit onto a second line that `overflow:clip` hides, and the caption carries that colour's count (spec 078, #330, which closes the limit spec 077 decision 2 had recorded; see `DeckShape.md`). **A split card's second half adds no symbol to the bar, and that is deliberate:** a card is read through its first face for its type, cost and symbols, as the zone tables read it, while its mana value is Scryfall's number for the card, both halves added, so a split card is in the curve column of that sum while its symbols are the first half's only (spec 069 Non-Goals, decision 7, AC-4.5). The panel costs one statement of its own, `MTG::Card` for the identity ids of the counted zones' printings, none when those zones hold no card, and reads everything else from the rows `Deck::Page` loaded, so a page's count is the same however large the deck is (decision 11, AC-6.1, AC-6.2); it stores nothing and the export is unchanged (decision 13). @@ -140,7 +141,7 @@ The shape panel is the collecting vocabulary's third answer, after `formats` and The report is the core's `Catalog::DeckChecks.report` over `Deck::Page#zones` as the page built them, with the deck's collectible type and stored format key. Its rules are the collecting vocabulary's fourth answer, `deck_rules(format_key)`: the format's size rules (each the zone keys it counts and a minimum, a maximum or an exact size) and its copy rule (the zone keys counted together and the limit of a counted printing), or nil. **The contract is written in `Catalog::DeckChecks`' comment, the one place it is written down**; this doc does not restate it. A vocabulary without `deck_rules`, a nil answer, a collectible type the instance no longer registers and a deck whose format it no longer registers have no size or copy rules (decision 1, FR-1, AC-5.5). The core owns the counting, the words and the order, reads only the rows and extension records the page loaded (never a record by an id from the request, never a statement per row) and names no collectible, format or zone key but `considering` (decision 1, FR-1, AC-5.4; the deck guard, below). Magic's answer is `MTG::Collecting.deck_rules`, which returns `MTG::DeckRules.for(format_key)`. -- **What is counted** (decisions 3, 4). A card is one owned copy or one proxy, so owned and proxy count together in every check (spec 060 decision 12; AC-2.6, AC-3.3). The copies of a card are summed by catalog identity over every printing of it in the rule's zones (AC-3.3, AC-3.15). A size rule counts the deck's zone rows with its keys; a key the deck has no zone row for counts 0 cards and is named from the format's registration, or by its key when the registration lacks it (decision 1, AC-2.12). Considering and any zone no rule names are counted by no size or copy rule (decision 3, AC-2.5, AC-2.11, AC-3.8). A card of another collectible than the deck's is left out of every check, owned plus proxy included, and named in no finding, as spec 069 decision 3 does for the panel, so a size count can be smaller than the zone head's "N cards" (decision 4, AC-2.10, AC-3.11, AC-4.9). A token or any other kind of card counts like any card. A printing with no card data (no extension record: the catalog hasn't been refreshed since it was added) counts by its identity like any card and is never exempt and never restricted, because nothing says it is, so before a refresh a basic land with no record is judged like any card (decision 4, AC-3.14). A card's limit is the lowest number any counted printing of it answers; it is unlimited only when every counted printing answers none, so a basic land with one printing that has card data and one that has none is held to the ordinary limit (decision 1's mixed case, AC-3.16). +- **What is counted** (decisions 3, 4). A card is one owned copy or one proxy, so owned and proxy count together in every check (spec 060 decision 12; AC-2.6, AC-3.3). The copies of a card are summed by catalog identity over every printing of it in the rule's zones (AC-3.3, AC-3.15). A size rule counts the deck's zone rows with its keys; a key the deck has no zone row for counts 0 cards and is named from the format's registration, or by its key when the registration lacks it (decision 1, AC-2.12). Considering and any zone no rule names are counted by no size or copy rule (decision 3, AC-2.5, AC-2.11, AC-3.8); the core leaves Considering out even when a rule names its key, and such a rule still names it in its size line, so the rule holds for every collectible and not only because Magic's rules never name it (decision 3; the implementation review's first follow-up, spec 079 `verification.md`). A card of another collectible than the deck's is left out of every check, owned plus proxy included, and named in no finding, as spec 069 decision 3 does for the panel, so a size count can be smaller than the zone head's "N cards" (decision 4, AC-2.10, AC-3.11, AC-4.9). A token or any other kind of card counts like any card. A printing with no card data (no extension record: the catalog hasn't been refreshed since it was added) counts by its identity like any card and is never exempt and never restricted, because nothing says it is, so before a refresh a basic land with no record is judged like any card (decision 4, AC-3.14). A card's limit is the lowest number any counted printing of it answers; it is unlimited only when every counted printing answers none, so a basic land with one printing that has card data and one that has none is held to the ordinary limit (decision 1's mixed case, AC-3.16). - **Owned plus proxy is the core's own check and runs for every deck** (decisions 2, 5): Casual, a format the instance no longer registers and a collectible with no rules included (AC-2.7, AC-4.5). A card is flagged in a zone that holds at least one owned copy and at least one proxy of it, of one printing or of two, in any zone, Considering and a zone no rule names included (AC-4.1, AC-4.2, AC-4.10). Each zone is judged alone: owned in Main and a proxy in Considering is no finding, because a proxy in Considering is a plan and not a leftover (AC-4.3), and a card with both in Main and in Sideboard has a finding per zone, each with its zone's split (AC-4.6). A finding names a proxy and never changes one (spec 079 Non-Goals; the issue design's decision 8). - **Magic's rules** (decision 3; `MTG::DeckRules`, constants below). Standard, Pioneer, Modern, Legacy, Vintage and Pauper: Main at least 60 cards, Sideboard at most 15, and at most 4 copies of a card across Main and Sideboard. Vintage also holds a card to 1, "restricted to 1", when a counted printing's stored legalities list it as restricted in Vintage; the same card in Legacy is held to 4 (AC-3.7). Commander: Commander and Main together exactly 100 cards, and at most 1 copy of a card across them. Casual: no rules. Never limited, in any format: a basic land (its first face's type line has the supertype Basic, Snow-Covered ones included; AC-3.4, AC-3.6) and a card whose rules text says a deck can have any number of cards with its name (AC-3.5). A card whose text states a number ("A deck can have up to seven cards named Seven Dwarves.", "up to nine" for Nazgûl) is held to that number in every format, Commander included (the maintainer's ruling, 2026-11-12; AC-3.12); a number word outside `MTG::DeckRules::NUMBER_WORDS` falls through to the ordinary limit (plan, Risks). The wording the two texts are matched on is Scryfall's, checked against its API (spec 079 `verification.md`, Scryfall facts); the order of the tests is `MTG::DeckRules.limit_for`'s comment. - **The order** (decisions 6, 10). The summary line reads "3 findings", "1 finding" or "No findings" (AC-1.7). The results are the size rules in the extension's order, then the copy check, then owned plus proxy; a deck with no size or copy rules shows the no-rules line, then owned plus proxy (AC-1.2, AC-2.7). Copy findings are ordered by the position, in the page's zone order, of the first counted zone that holds the card, then by name, then by identity id (AC-3.9); owned-plus-proxy findings by their own zone's position, then by name, then by identity id. @@ -420,3 +421,4 @@ Measured bars, not constants (recorded in each spec's `verification.md` where on | 2026-11-11 | Spec 073 (the card page's "Free to use" and "In decks", #50: the three-stat strip, the one definition of free read through `Lot::FreeCopies` with "In decks" as its complement, the "In decks" section from `Deck::CardUses` and its head count, the card as the scope with the Proxy column counting any printing, a visitor's overview reading nothing, the constant number of statements, the three scoped CSS rules and "Your copies"' place class; the Considering and card-page paragraphs, scope, sources and the open-issues line; supersedes spec 004 FR-9's single stat and fulfils spec 071 AC-1.4) | [#333](https://github.com/plainprogrammer/Collector/pull/333) | | 2026-11-11 | Spec 077 (the deck shape panel at extreme counts, #320: `MTG::DeckShape#short_count` and `#caption_items`, the curve column's count and the mana segment's label abbreviated from 1,000 and every other number exact, the Card types list a grid of subgrid rows, the caption's unbreakable items, the mana-label band recorded as a limit and offered as #330; the two `SHORT_COUNT_*` Named constants; the Issues bullet; specs 060 and 069 superseded in part) | [#331](https://github.com/plainprogrammer/Collector/pull/331) | | 2026-11-12 | Spec 078 (a mana bar label fits its segment or isn't drawn, #330: the fit rule in `additions.css`, a wrapping flex segment with `overflow:clip` and a first-item `::before`, so the browser's line breaking clips a label that cannot fit; the 10% narrow class kept; the panel's paragraph, the Issues bullet and Sources; spec 077 decision 2's documented limit superseded in part) | [#336](https://github.com/plainprogrammer/Collector/pull/336) | +| 2026-11-12 | Spec 079 (the deck checks, #258: Check deck, a link to `GET /decks/:id/check` and `Decks::ChecksController`, the results section between the head and the zones with its focus, no stored result and the `no-cache` tag, prefetch off; the fourth vocabulary answer `deck_rules` and the core `Catalog::DeckChecks`, whose comment holds the contract; what is counted (owned plus proxy, by catalog identity, Considering and unnamed zones never, another collectible's cards never, a printing with no card data like any card, the lowest limit), owned plus proxy for every deck, Magic's rules in `MTG::DeckRules`, the words, the order and the summary line; the statement count; the glossary, the head's actions, the deck guard's count, the Named constants, a decision, an accepted limit, Sources; spec 069's non-goal "deck rules" done in part; supersedes nothing) | spec 079's pull request | diff --git a/docs/specs/README.md b/docs/specs/README.md index f4bd397c..d8671d3f 100644 --- a/docs/specs/README.md +++ b/docs/specs/README.md @@ -94,4 +94,4 @@ A plan is deleted from the tree in a commit of its own after its feature's pull | [073 "Free to use" and "In decks" on the Card Page](073-card-page-deck-stats/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) and [catalog core](../architecture/catalog-core.md) (the card page's stats and "In decks" section; the visitor rule and the card page's order); the card page's markup is `ItemPage` in the [design system](../design-system/README.md) | [Collection and lots](../architecture/collection-and-lots.md), [catalog core](../architecture/catalog-core.md), [design system](../design-system/README.md) (`ItemPage`, `ItemPagePhone`, `SingleStat`) | retired in `1bdb1c62a908b76131f1eb2a94f769b419a78689`; `git show 1bdb1c62a908b76131f1eb2a94f769b419a78689^:docs/specs/073-card-page-deck-stats/plan.md` | In part 004 FR-9 (the single stat; no "In decks" or "Free to use"), its Non-Goal and Out of Scope entries and AC-8.1's order; 071 AC-1.4 and its "where #50 sits" note (fulfilled); fulfils the Non-Goals that name #50 (029, 032, 033, 057, 070) and 057 decisions 7 and 8 (the strip and the section) | | | [077 The Deck Shape Panel Stays Tidy at Extreme Counts](077-deck-shape-extreme-counts/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph and Named constants); the panel's markup is `DeckShape` in the [design system](../design-system/README.md) | [Design system](../design-system/README.md) (`DeckShape`), [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph, Named constants) | retired in `a54b85eeb63a0ac540bd38b88747c79a0c2d47b4`; `git show a54b85eeb63a0ac540bd38b88747c79a0c2d47b4^:docs/specs/077-deck-shape-extreme-counts/plan.md` | In part 060 decision 2, AC-2.2, AC-2.4, AC-2.5 and AC-2.6 (the visible numbers, the types' alignment) and 069 decision 14, its four-digit-count Error Scenarios row and Findings 1 to 4 ([#320](https://github.com/plainprogrammer/Collector/issues/320)) | In part by [078](078-mana-bar-label-fit/spec.md) (decision 2's documented limit: the mana-label band, [#330](https://github.com/plainprogrammer/Collector/issues/330)) | | [078 A Mana Bar Label Fits Its Segment or Isn't Drawn](078-mana-bar-label-fit/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph); the panel's markup and CSS rule are `DeckShape` in the [design system](../design-system/README.md) | [Design system](../design-system/README.md) (`DeckShape`), [collection and lots](../architecture/collection-and-lots.md) (the panel's paragraph) | retired in `b275dc7e20ef03355d825eec48569c2eb7ce8df0`; `git show b275dc7e20ef03355d825eec48569c2eb7ce8df0^:docs/specs/078-mana-bar-label-fit/plan.md` | In part 077 decision 2 (the mana-label band as a documented limit, ruled A on 2026-11-11), its Goal 1's qualification, Open Questions, Error Scenarios row for the band, AC-5.1 and AC-5.3, and its Supersedes row's exception for 060 AC-2.4 ([#330](https://github.com/plainprogrammer/Collector/issues/330)); 060 decision 4 and 069 decision 9 are kept | | -| [079 Deck Checks (Deck Size, Copy Limits, and Owned Plus Proxy of One Card)](079-deck-checks/spec.md) | Approved ([#258](https://github.com/plainprogrammer/Collector/issues/258)) | [Collection and lots](../architecture/collection-and-lots.md) (decks and the deck page), [DeckPage](../design-system/components/DeckPage.md) | present | Nothing. It does in part (size and copy limits) the "deck rules" that 069 left as a non-goal | | +| [079 Deck Checks (Deck Size, Copy Limits, and Owned Plus Proxy of One Card)](079-deck-checks/spec.md) | Frozen, by [collection and lots](../architecture/collection-and-lots.md) (the checks' rules); the results section's markup and CSS rule are `DeckCheck` in the [design system](../design-system/README.md) | [Collection and lots](../architecture/collection-and-lots.md) (decks and the deck page), [DeckPage](../design-system/components/DeckPage.md) | retirement owed | Nothing. It does in part (size and copy limits) the "deck rules" that 069 left as a non-goal | | From 01b3efb12ccfec36d6a07455562fcbba6ccaf9a4 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:58:39 -0500 Subject: [PATCH 23/24] docs(specs): mark spec 079 frozen by the collection-and-lots area doc (spec 027 D4) Co-Authored-By: Claude Sonnet 5.5 --- docs/specs/079-deck-checks/spec.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/specs/079-deck-checks/spec.md b/docs/specs/079-deck-checks/spec.md index a64bacdc..fd31ba5c 100644 --- a/docs/specs/079-deck-checks/spec.md +++ b/docs/specs/079-deck-checks/spec.md @@ -8,6 +8,7 @@ **Branch:** `079-deck-checks` **Issue:** [#258](https://github.com/plainprogrammer/Collector/issues/258) (the 2.0 milestone, roadmap theme C) **ADR:** none: ADR 0028 already puts a deck's collectible part behind the collecting vocabulary, and specs 066 and 069 added optional answers to that seam; this spec adds a third the same way (decision 1) +**Frozen:** the current rules are in [`collection-and-lots.md`](../../architecture/collection-and-lots.md) (whole spec); the results section's markup and CSS rule are `DeckCheck` in the [design system](../../design-system/README.md); see the [specs index](../README.md) --- From 5c0b27bc802fb1f1560348bd9e85e948f4eca53d Mon Sep 17 00:00:00 2001 From: James Thompson Date: Thu, 12 Nov 2026 16:58:56 -0500 Subject: [PATCH 24/24] docs(decks): record the distillation of spec 079 in verification.md (#258) Co-Authored-By: Claude Sonnet 5.5 --- docs/specs/079-deck-checks/verification.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/specs/079-deck-checks/verification.md b/docs/specs/079-deck-checks/verification.md index 3db7f455..db19a0f2 100644 --- a/docs/specs/079-deck-checks/verification.md +++ b/docs/specs/079-deck-checks/verification.md @@ -237,3 +237,23 @@ Its four follow-ups, and how each was resolved: 4. The index's Supersedes cell for 079 now says it does the "deck rules" of 069's non-goal in part (size and copy limits) and supersedes nothing. The reviewer did not re-run the supersession grep, YARD or the comment ratchet (`bin/ci` did), and did not open the walkthrough GIF. + +## Distillation + +Distilled on 2026-11-12 by the `distill-feature` skill, state 2, before the pull request is opened. + +- **Area doc:** [`collection-and-lots.md`](../../architecture/collection-and-lots.md), written in T5.1 (`32b86d92`, `178a00ec`) and checked against the finished code (`Catalog::DeckChecks`, `MTG::DeckRules`, `MTG::Collecting.deck_rules`, `Decks::ChecksController`, `decks/_check`, `decks/show`). It now states: + - Check deck: a link to `GET /decks/:id/check` after Edit deck, no `form`, the 404 and sign-in rules, the results section between the head and the zones with focus on "Deck check", prefetch off and why, nothing stored, `turbo-cache-control: no-cache` on the results page only, the statement count (decisions 7, 8, 9; AC-1.x, AC-5.1–AC-5.3, AC-6.3). + - The seam: `deck_rules` as the vocabulary's fourth answer, the core `Catalog::DeckChecks` with the contract pointed to in its comment and not restated, the no-rules cases (decision 1, FR-1, AC-5.5). + - What is counted: owned plus proxy, by catalog identity, Considering and unnamed zones never (even when a rule names Considering, added in this distillation after `56b26fba`), another collectible's cards never, a printing with no card data like any card, the lowest limit and the mixed case (decisions 1, 3, 4). + - Owned plus proxy for every deck, each zone alone (decisions 2, 5). + - Magic's rules and their Named constants (`MTG::DeckRules`; decision 3). + - The words, the order and the summary line (decisions 6, 10). + - The glossary's check, finding and pass (spec 079 Terms; added in this distillation). + - The deck guard's 68 files, a decision (on demand, advice, rules in the extension), an accepted limit (a 60-character zone name with no space), Sources (spec 079, #258), and that spec 069's "deck rules" non-goal is done in part (AC-7.1). + - Corrected in this distillation: the deck page's collectible part is three optional answers after `formats`, not two. +- **Design system:** the results section's markup and CSS rule are `DeckCheck` (`docs/design-system/components/DeckCheck.md`, `.c-deckcheck` in `additions.css`), and the head's link is in `DeckPage.md`; both were written in T4.1 and T5.1 and agree with the final CSS (the `c-deckcheck` rename of `4ca7014d`). They are not area docs. +- **ADR:** none proposed. ADR 0028 already puts a deck's collectible part behind the collecting vocabulary; specs 066 and 069 added optional answers to it and this spec adds a third the same way (the spec's header). +- **CLAUDE.md:** its Project Status clause for the checks (AC-7.6) restates no rule of the area doc; unchanged. +- **Index:** 079 is Frozen by the area doc, with `DeckCheck` in the design system; Plan is "retirement owed": the plan stays on the branch and is retired after the pull request merges (state 1 of the skill). Spec 079 supersedes nothing, so no other row changed. +- **Distillation log:** the row names "spec 079's pull request" until the pull request has a number; state 1 replaces it with the link after the merge.