Skip to content

Demo: card-script review CI (advisory) — do not merge#23

Open
MostCromulent wants to merge 2 commits into
masterfrom
card-review-demo
Open

Demo: card-script review CI (advisory) — do not merge#23
MostCromulent wants to merge 2 commits into
masterfrom
card-review-demo

Conversation

@MostCromulent

Copy link
Copy Markdown
Owner

Summary

Proof of concept for an advisory CI check that reviews changed card scripts on a PR and posts terse inline comments — a deterministic linter plus a Scryfall card-frame fact-check. It never fails the check, it only comments; a reviewer decides what to act on. Do not merge — the fixtures below are deliberately broken so the bot has something to flag.

What it checks

  • .github/scripts/cardlint.py — deterministic linter: undefined Execute$/SubAbility$/Choices$ refs (a bad Execute$ means the card fails to load), duplicate or unknown params, illegal mana tokens, a missing ManaCost, loyalty abilities missing Planeswalker$ True, and more. Its "known params" are derived from the card corpus itself, so it adapts when the engine adds or renames a param.
  • .github/scripts/card_script_review.py — compares the card frame (name, type line, power/toughness, mana cost, loyalty) against Scryfall. Catches transcription slips like a missing Legendary, an instant/sorcery swap, or a wrong mana symbol. Stays silent when a card isn't on Scryfall yet.

Fixtures in this PR

File Injected defect Expected comment
crystal_inhuman_princess.txt Legendary supertype dropped Types → add Legendary
elektra_femme_fatale.txt wrong power/toughness PT 4/23/2
vulture_feathered_fiend.txt wrong mana cost ManaCost 3 U U2 U U
zztest_broken_etb.txt Execute$ points at an undefined SVar undefined SVar (card won't load) + orphan SVar
zztest_param_typo.txt ValidTgt$ typo + made-up Foozle$ ValidTgt$ValidTgts$; unknown param
zztest_no_cost.txt non-Land with no ManaCost line missing ManaCost line

The three real cards (Crystal/Elektra/Vulture) are recent cards not yet in this tree, chosen so they shadow no existing card or test; the zztest_* cards use placeholder names.


🤖 Generated with Claude Code

Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
Repository owner deleted a comment from github-actions Bot Jun 14, 2026
ManaCost:2 U
Types:Artifact Creature Robot
PT:2/2
T:Mode$ ChangesZone | Origin$ Any | Destination$ Battlefield | ValidCard$ Card.Self | Execute$ TrigNope | TriggerDescription$ When this enters, draw a card.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Execute$ TrigNope → undefined SVar (card won't load)

Types:Artifact Creature Robot
PT:2/2
T:Mode$ ChangesZone | Origin$ Any | Destination$ Battlefield | ValidCard$ Card.Self | Execute$ TrigNope | TriggerDescription$ When this enters, draw a card.
SVar:TrigDraw:DB$ Draw

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SVar TrigDraw is never referenced (clause never fires)

Name:Zztest Dup Param
ManaCost:1 R
Types:Instant
A:SP$ DealDamage | ValidTgts$ Any | ValidTgts$ Creature | NumDmg$ 2 | SpellDescription$ Deal 2 damage.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate ValidTgts$ → engine keeps only the last

@@ -0,0 +1,4 @@
Name:Zztest No Cost

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing a ManaCost line

Name:Zztest Param Typo
ManaCost:R
Types:Instant
A:SP$ DealDamage | ValidTgt$ Any | NumDmg$ 2 | Foozle$ True | SpellDescription$ Deal 2 damage to any target.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValidTgt$ValidTgts$

Name:Zztest Param Typo
ManaCost:R
Types:Instant
A:SP$ DealDamage | ValidTgt$ Any | NumDmg$ 2 | Foozle$ True | SpellDescription$ Deal 2 damage to any target.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Foozle$ → appears in no other card (unknown or outdated param?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant