Skip to content

feat(catalog): store each card's mana value from the catalog (#250) - #279

Merged
plainprogrammer merged 15 commits into
mainfrom
058-mtg-mana-value
Nov 10, 2026
Merged

feat(catalog): store each card's mana value from the catalog (#250)#279
plainprogrammer merged 15 commits into
mainfrom
058-mtg-mana-value

Conversation

@plainprogrammer

@plainprogrammer plainprogrammer commented Nov 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Closes #250. Spec 058 (roadmap 14d): the deck shape panel (#256) can draw a mana curve from local data.

What changes

  • mtg_cards.mana_value: a nullable float, MTG only, no default; one reversible migration (AddManaValueToMTGCards). No core table changes.
  • MTG::Scryfall::Mapper.mana_value reads Scryfall's cmc, else the front face's (real reversible cards have no top-level cmc), else nil. Numeric only, never fetch or Float(), so an odd value can never make a record malformed and drop the card.
  • MTG::Scryfall::Source#reapply? gains a second reason (cards exist and none has a mana value), so an upgraded instance fills the column on its next refresh even when Scryfall's file is unchanged.
  • Docs: catalog core and catalog operations area docs, an upgrade note in docs/releasing.md, spec, plan and verification.md under docs/specs/058-mtg-mana-value/.

Verification

  • Pre-push hook run: full bin/ci passed at pushed SHA d415dd15 (3054 examples by bin/rspec --dry-run). GitHub CI run 42828485850 on the same SHA: all jobs green (checks, RSpec 1-4 of 4, both image jobs with the upgrade smoke).
  • Failing first: only the new reapply? reason and AC-3.2 failed before the change; the other new examples are guards (stated in the Phase 3 commit body).
  • Reversibility: db:migrate:redo:primary (plain db:migrate:redo aborts in this four-database app; spec v1.0.3 names it). Schema diff: the version line and the one column.
  • Reviews (read-only Opus): spec round 1 APPROVE WITH CHANGES; plan round 1 REVISE (2 blocking, all folded in), plan round 2 APPROVE WITH CHANGES (no blocking); implementation review SPEC-ALIGNED WITH NOTES (no blocking or major, 6 minor, fixed or recorded).

For the maintainer

  • Spec 058 has Gate A and Gate B approvals; the spec is v1.0.3 (the redo command wording only).
  • The session-wrap output is a lesson (docs/lessons/2026-11-10-…) and a line in .claude/memory/migrate-schema-noise.md; written without a separate approval, drop them if unwanted.
  • The migration class is AddManaValueToMTGCards (acronym inflection); spec AC-4.3 spells it AddManaValueToMtgCards, recorded in verification.md.
  • Other specs' owed plan retirements are not touched here.

🤖 Generated with Claude Code

plainprogrammer and others added 13 commits November 10, 2026 07:17
@plainprogrammer @claude
docs(spec): 058 store each card's mana value from the catalog (#250)
2d81771 
Spec 058, approved at Gate A: a nullable mtg_cards.mana_value filled from
Scryfall cmc, reapplied once on upgrade so existing instances fill it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs(plan): 058 mana value plan, five phases (#250)
21acf2a 
Revised after round 1 of the read-only Opus plan review (two blocking
findings and nine smaller ones, all folded in). Round 2 follows at the
maintainer's request.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs(spec): 058 plan review round 2 changes, spec 1.0.3 (#250)
5c6e4d0 
Round 2 (Opus, APPROVE WITH CHANGES, nothing blocking): the redo command is
db:migrate:redo:primary in the spec and plan; AC-3.1 checks the cards' other
attributes; AC-4.1 says why the value is not derived from the mana cost;
verification.md is written in two steps; every core spec runs.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
feat(catalog): add mtg_cards.mana_value (#250)
e1854f9 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
feat(mtg): map Scryfall's cmc to a card's mana value (#250)
17f8dfa 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
style(catalog): space before the migration superclass (#250)
9bf19c8 
The class is AddManaValueToMTGCards: the MTG acronym inflection makes the
file name camelize to that, not AddManaValueToMtgCards.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
feat(mtg): apply an already-applied file once for cards without a man…
d2739aa 
…a value (#250)

MTG::Scryfall::Source#reapply? is now true for either of two reasons: spec 011
AC-2.3 (printings have no artwork ids) or spec 058 decision 3 (cards exist and
none has a mana value), so an upgraded instance fills the column on its next
scheduled refresh even when Scryfall has published no new file. Catalog::Refresh,
Catalog::Sources, views, helpers and controllers are untouched.

Failed first (the only two): "asks once for a catalog whose cards have no mana
value yet, when artwork ids are all there" (source_spec, AC-3.4) and "applies a
scheduled refresh of an already-applied version while no card has a mana value"
(refresh_scryfall_spec, AC-3.2: the run was skipped). The other new examples
(the spec 011 hook example, "does not ask while some card has a mana value",
"asks for either reason alone", and the refresh examples for AC-3.1, AC-3.3,
AC-3.5, AC-3.6 and AC-3.7) are guards that passed before the change.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs: spec 058 area docs, upgrade note and verification (#250)
cdade2d 
Catalog core and catalog operations record the mana value and the second
reapply reason, releasing.md gets the upgrade note, verification.md holds the
evidence of Phases 1-4, and the plan names the migration class as built
(AddManaValueToMTGCards, from the MTG acronym inflection).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs: spec 058 implementation review, comment and limit fixes (#250)
f9e346d 
SPEC-ALIGNED WITH NOTES: no blocking or major findings. The reapply? cost
comment says at most four queries, the migration comment is reflowed, the
degenerate no-cmc file is an accepted limit, and the verdict is recorded in
verification.md.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
chore(memory): capture spec 058 learnings (#250)
61e0995 
A lesson on filling a new catalog column (refresh digest, reapply? reason, the
pre-upgrade state a spec must reset) and a note on db:migrate:redo:primary and
the MTG acronym in a migration class name.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs(architecture): distil spec 058 into catalog core and catalog ope…
f8a9648 
…rations (#250)

Distillation log rows in both area docs; the index row reads Frozen with the
plan marked retirement owed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs(specs): mark spec 058 frozen by the catalog core and catalog ope…
4d3be2c 
…rations area docs (spec 027 D4)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer @claude
docs: record the spec 058 distillation in verification.md (#250)
d415dd1 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@plainprogrammer
plainprogrammer marked this pull request as ready for review November 10, 2026 13:58
@plainprogrammer plainprogrammer self-assigned this Nov 10, 2026
@plainprogrammer
plainprogrammer merged commit 105560f into main Nov 10, 2026
9 checks passed
@plainprogrammer
plainprogrammer deleted the 058-mtg-mana-value branch November 10, 2026 14:39
@plainprogrammer plainprogrammer mentioned this pull request Nov 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@plainprogrammer plainprogrammer

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Store each card's mana value from the catalog

1 participant

@plainprogrammer