External/card-drop
Watch
5
0
Fork
You've already forked card-drop
 0
Code Issues 36 Pull requests 4 Projects 1 Releases 35 Packages Wiki Activity Actions

Validate card metadata types at parse time #516

New issue
Open
opened 2026-09-03 12:05:54 +01:00 by Smithy-bot · 1 comment
Smithy-bot commented 2026-09-03 12:05:54 +01:00
Member
Copy link

Parent: #487 (2026 Code Clean Up). Spun out of the review note on PR #515.

Why

Card metadata is JSON.parsed with no schema/type validation. Fields such as type are declared numeric (CardRarity) in TypeScript, but a metadata file can still contain a string, e.g. "type": "1".

Under loose equality (==) that value matched numeric 1. After the strict-equality sweep in #515 (===), the same card would silently disappear from:

  • the drop pool (GetUnclaimedCardsHelper)
  • /inventory type filters (InventoryHelper)
  • /stats rarity counts

Strict equality is the right long-term behaviour; the real fix is to reject or coerce bad metadata when it is loaded, not to keep ==.

docs/cards.md (PR #514) already asks for parse-level validation — this ticket is that work.

Suggested approach

  1. Find where card JSON is loaded/parsed (card folder → in-memory card objects).
  2. Validate (or safely coerce) known numeric/enum fields (type / rarity, and any other number fields used in comparisons).
  3. On invalid metadata: log a clear error identifying the file/card id, and either skip that card or fail fast in non-production — pick one and document it.
  4. Add unit tests for: valid numeric type, string "1" (invalid or coerced per chosen policy), missing type, and non-numeric garbage.
  5. Smoke /drop, /inventory, and /stats against the real card folder after the change.

Acceptance criteria

  • Malformed type (string/null/missing) cannot silently pass into drop/inventory/stats comparison paths.
  • Invalid card files are visible in logs (file path or card id).
  • Tests cover valid + invalid metadata shapes.
  • Manual smoke of /drop, /inventory, /stats against real cards still looks right.

Related

  • PR #515 — partial strict-equality sweep that surfaces this risk
  • PR #514 — docs note in docs/cards.md
  • Sibling #517 — finish the repo-wide eqeqeq sweep (prefer doing this validation first)
  • Parent #487
Parent: #487 (2026 Code Clean Up). Spun out of the review note on PR #515. ## Why Card metadata is `JSON.parse`d with no schema/type validation. Fields such as `type` are declared numeric (`CardRarity`) in TypeScript, but a metadata file can still contain a string, e.g. `"type": "1"`. Under loose equality (`==`) that value matched numeric `1`. After the strict-equality sweep in #515 (`===`), the same card would **silently disappear** from: - the drop pool (`GetUnclaimedCardsHelper`) - `/inventory` type filters (`InventoryHelper`) - `/stats` rarity counts Strict equality is the right long-term behaviour; the real fix is to reject or coerce bad metadata when it is loaded, not to keep `==`. `docs/cards.md` (PR #514) already asks for parse-level validation — this ticket is that work. ## Suggested approach 1. Find where card JSON is loaded/parsed (card folder → in-memory card objects). 2. Validate (or safely coerce) known numeric/enum fields (`type` / rarity, and any other number fields used in comparisons). 3. On invalid metadata: log a clear error identifying the file/card id, and either skip that card or fail fast in non-production — pick one and document it. 4. Add unit tests for: valid numeric `type`, string `"1"` (invalid or coerced per chosen policy), missing `type`, and non-numeric garbage. 5. Smoke `/drop`, `/inventory`, and `/stats` against the real card folder after the change. ## Acceptance criteria - [ ] Malformed `type` (string/null/missing) cannot silently pass into drop/inventory/stats comparison paths. - [ ] Invalid card files are visible in logs (file path or card id). - [ ] Tests cover valid + invalid metadata shapes. - [ ] Manual smoke of `/drop`, `/inventory`, `/stats` against real cards still looks right. ## Related - PR #515 — partial strict-equality sweep that surfaces this risk - PR #514 — docs note in `docs/cards.md` - Sibling #517 — finish the repo-wide `eqeqeq` sweep (prefer doing this validation first) - Parent #487
Smithy-bot added this to the 0.12.0 milestone 2026-09-03 12:05:54 +01:00
Vylpes stopped working 2026-09-06 09:30:55 +01:00
20 minutes 1 second
Smithy-bot commented 2026-09-06 16:13:25 +01:00
Author
Member
Copy link

QA results (step/testing → step/uat)

Tested against merged PR #518 on release/0.12.0 (bd1cd3d).

Pipeline (yarn build / yarn test / yarn lint)

  • build (tsc): pass
  • test (jest): 17/17 suites, 118/118 tests, 8 snapshots — pass
  • lint (eslint .): pass

Feature suite (tests/Functions/CardMetadataValidator.test.ts)

28/28 cases pass, covering the acceptance criteria:

  • valid numeric type accepted
  • string "1", missing, null, non-numeric garbage, and out-of-range type rejected
  • error messages include the file path (and card/series id)
  • series id numeric-string coercion still works

Acceptance criteria check

  • Malformed type cannot silently reach drop/inventory/stats comparison paths (reject-not-coerce; load aborts → safe mode via existing CardMetadataFunction catch)
  • Invalid files are identifiable in logs (LogError with jsonPath + validation message)
  • Tests cover valid + invalid metadata shapes
  • Manual Discord smoke of /drop, /inventory, /stats against the real card folder — not run here (no DATA_DIR card pack in the repo / no live bot in this environment). Worth a quick human check in UAT.

Moving to step/uat.

## QA results (step/testing → step/uat) Tested against merged PR #518 on `release/0.12.0` (`bd1cd3d`). ### Pipeline (`yarn build` / `yarn test` / `yarn lint`) - **build** (`tsc`): pass - **test** (`jest`): **17/17** suites, **118/118** tests, 8 snapshots — pass - **lint** (`eslint .`): pass ### Feature suite (`tests/Functions/CardMetadataValidator.test.ts`) **28/28** cases pass, covering the acceptance criteria: - valid numeric `type` accepted - string `"1"`, missing, null, non-numeric garbage, and out-of-range `type` rejected - error messages include the file path (and card/series id) - series `id` numeric-string coercion still works ### Acceptance criteria check - [x] Malformed `type` cannot silently reach drop/inventory/stats comparison paths (reject-not-coerce; load aborts → safe mode via existing `CardMetadataFunction` catch) - [x] Invalid files are identifiable in logs (`LogError` with `jsonPath` + validation message) - [x] Tests cover valid + invalid metadata shapes - [ ] Manual Discord smoke of `/drop`, `/inventory`, `/stats` against the real card folder — **not run here** (no `DATA_DIR` card pack in the repo / no live bot in this environment). Worth a quick human check in UAT. Moving to `step/uat`.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
release/0.12.0
cursor/487-strict-equality
cursor/487-developer-docs
cursor/487-inventory-dropdown-tests
cursor/487-2026-code-clean-up
release/0.10.0
release/0.11.0
develop
v0.9.4
v0.9.3
v0.9.2
v0.9.1
v0.9.0
v0.8.4
v0.8.3
v0.8.2
v0.8.1
v0.8.0
v0.7.0
v0.6.4
v0.6.3
v0.6.2
v0.6.1
v0.6.0
v0.5.2
v0.5.1
v0.5.0
v0.4.2
v0.4.1
v0.4.0
v0.3.1
v0.3.0
v0.2.1
v0.2.0
v0.1.8
v0.1.7
v0.1.6
v0.1.5
v0.1.4
v0.1.3
v0.1.2
v0.1.1
v0.1.0
Labels
Clear labels
  
blocked

   
duplicate
This issue or pull request already exists

  
needs
approval

  
needs
criteria

  
needs
estimate

  
needs
tests

  
question
More information is needed

  
step
doing

  
step
review

  
step
testing

  
step
todo

  
step
uat

  
type
admin

  
type
alert

  
type
bug

  
type
change

  
type
defect

  
type
epic

  
type
idea

  
type
incident

  
type
investigation

  
type
spike

  
type
story

  
wontfix
This won't be fixed

No labels
blocked
duplicate
needs
approval
needs
criteria
needs
estimate
needs
tests
question
step
doing
step
review
step
testing
step
todo
step
uat
type
admin
type
alert
type
bug
type
change
type
defect
type
epic
type
idea
type
incident
type
investigation
type
spike
type
story
wontfix
Milestone
Clear milestone
No items
No milestone
0.12.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
Vylpes
1 participant
Notifications
Total time spent: 20 minutes 1 second
Vylpes
20 minutes 1 second
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#517 Finish repo-wide strict equality sweep and enable eqeqeq
External/card-drop
Reference
External/card-drop#516
No description provided.