Skip to content

feat(feats): author the 2014 Feat catalog rows behind the edition tag - #1731

Merged
Sandersland merged 2 commits into
stagingfrom
feat/1310-2014-feats
Aug 5, 2026
Merged

feat(feats): author the 2014 Feat catalog rows behind the edition tag#1731
Sandersland merged 2 commits into
stagingfrom
feat/1310-2014-feats

Conversation

@Sandersland

Copy link
Copy Markdown
Owner

Summary

Restores the 2014 half of the Feat catalog 6491c528 (#1154) replaced. Every Feat row is now edition-tagged (no edition: NULL remains) — before this change a 2014 character was served 36 rows of PHB'24 rules text, including feats PHB'14 doesn't have.

  • Adds 25 new EDITION_2014 rows: 24 recovered verbatim from git show 6491c528^:backend/prisma/seed/feats.ts, plus Grappler (transcribed from SRD 5.1 via the open dnd5eapi.co /2014/feats/grappler endpoint) and Savage Attacker (PHB'14-only — not SRD content — transcribed from and cross-checked against two independent published transcriptions, dnd5e.wikidot.com and aidedd.org, since it isn't available from an SRD API).
  • Stamps the 32 previously-shared rows EDITION_2024 (Alert's 2014 row already existed; its category moves origingeneral since PHB'14 has no Origin taxonomy).
  • 2014 catalog: 26 general/origin names + the 6 EDITION_2014 Fighting Style rows 2014 fighting styles: author the six PHB'14 styles as EDITION_2014 Feat rows #1311 already shipped = 32 total EDITION_2014 rows. 2024: 37 rows, unchanged in count. 0 NULL rows.
  • Removes featOfferedForAsiSlot's now-unused edition parameter (and its eslint-disable): PHB'14's "no per-feat level gate, earliest ASI at level 4" rule is already what the general branch's ?? 4 default encodes, so no fork is needed — an edition-invariant rule takes no edition parameter.

Content accuracy

  • Grappler: SRD 5.1 (open content), verified against the dnd5eapi.co API directly.
  • Savage Attacker: PHB'14, not SRD content — verified against two independent published transcriptions (dnd5e.wikidot.com, aidedd.org), both agreeing verbatim: "Once per turn when you roll damage for a melee weapon attack, you can reroll the weapon's damage dice and use either total."
  • The other 24 rows are recovered verbatim from the pre-2024 rules migration — waves 0-2 (epic #1126) #1154 catalog file per the issue's explicit instruction — that file's own header called its text "concise summaries", not page transcriptions, and the issue asks to preserve rather than rewrite them. One deliberate content change: 2014 Weapon Master drops its four hardcoded weapon-proficiency improvements (the description already reads "of your choice", matching the 2024 sibling's description-only treatment).

Live drift caught

species-creation-choices-2024.test.ts's featByName helper resolved by bare { name } with no edition filter — now that Savage Attacker and Magic Initiate fork, that lookup became non-deterministic and 3 tests went red. Pinned to EDITION_2024 (the file is 2024-only throughout).

Test plan (RED → GREEN)

  • prisma/seed/__tests__/seed-data.test.ts: 2 pre-existing tests went red on the seed change alone (General-feat invariant needed EDITION_2024 scoping; the Alert/Grappler illustration needed inverting since Grappler now forks) — fixed, plus a new FEATS — 2014 general/origin catalog (#1310) describe block (9 new assertions: row count, Mobile/Speedy split, Grappler/Savage Attacker/Weapon Master specifics, zero origin/epic_boon rows, zero NULL rows, all 4 BACKGROUNDS.originFeatName values present in both editions).
  • src/lib/rules/__tests__/catalog-edition-constraints.test.ts and src/routes/catalog/__tests__/feats.test.ts: both had a "Grappler stays one shared row" assertion that inverted — fixed, plus a new GET /api/feats?asiLevel= — 2014 general/origin feats (#1310) suite proving the 32/37 row counts and the ?? 4 default live against the real seeded catalog for 2014 (not just fixtures).
  • src/lib/srd/__tests__/srd-feat-offer.test.ts: updated for the 2-arg signature; the "same verdict in both editions" parity test (now meaningless — there's only one signature) is removed.
  • New src/routes/character/__tests__/feats-2014-general-take.test.ts: end-to-end creation + advancement-transaction coverage — a 2014 character takes Sentinel via an ASI slot with no abilityChoice (no PHB'14 bump), a 2014 half-feat (Athlete) requires one, a sub-level-4 2014 character is rejected, and a 2024 character is rejected on a 2014 feat id with edition wording (not a category message).
  • Stale comments/docstrings updated per the issue's explicit callouts: feats.test.ts and feats-asi-level-defaults.test.ts (the ?? 4 "unreachable from the real catalog" claim, now false), reference.test.ts (Savage Attacker no longer a shared-NULL row).
  • feat-fork-reseed.test.ts (already existed from 2014 fighting styles: author the six PHB'14 styles as EDITION_2014 Feat rows #1311, category-agnostic mechanism) — header comment updated to note it also backs this issue; verified live via prisma db seed run twice (32 EDITION_2014 / 37 EDITION_2024 / 0 NULL, stable both times, confirmed via direct psql query).

Gate: npm run lint, npm run test (backend 4379 passed, frontend 3469 passed), npm run typecheck (all workspaces) — all green. fallow audit --base origin/staging verdict pass (0 dead code, 0 duplication, one pre-existing/inherited complexity finding not introduced by this change). Pre-commit and pre-push hooks (lefthook: fallow, typecheck, test) passed.

Frontend

No changes. The two category === "fighting_style" filters the issue flagged as needing a no-crash check were already safe — #1311 gave 2014 six real fighting_style rows, so the "catalog now contains zero fighting_style rows for 2014" premise in that scope item no longer holds.

Closes #1310

🤖 Generated with Claude Code

https://claude.ai/code/session_018rre9Ho8Vx8zNtzzKkpvFn

Restores the 2014 half of the Feat catalog `6491c528` (#1154) replaced.
Adds 25 new EDITION_2014 rows (24 recovered verbatim from the pre-#1154
catalog, plus Grappler and Savage Attacker freshly transcribed from
SRD 5.1 / PHB'14) and stamps the 32 previously-shared rows EDITION_2024,
so every Feat row is now edition-tagged and no NULL row remains — a 2014
character no longer sees PHB'24 rules text for feats it never had.

PHB'14 has no Origin/Fighting Style/Epic Boon taxonomy, so every 2014 row
is category "general" with no levelPrerequisite; featOfferedForAsiSlot's
`?? 4` default is what encodes PHB'14's "earliest ASI is level 4" rule,
so the function drops its now-unused `edition` parameter (an
edition-invariant rule takes none).

Fixes the fixture-lookup drift the plan flagged in advance:
species-creation-choices-2024.test.ts's featByName helper resolved by
bare name and would have picked a nondeterministic edition now that
Savage Attacker/Magic Initiate fork — pinned to EDITION_2024.

Closes #1310

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018rre9Ho8Vx8zNtzzKkpvFn
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @Sandersland's task in 6m 16s —— View job


Code Review: PR #1731 — 2014 Feat catalog rows

  • Gather context and read changed files
  • Review seed data (feats.ts)
  • Review rule logic (srd/feats.ts, advancement.ts)
  • Review tests
  • Compile findings

Overview

Solid PR. The data model transformation is complete and correct: zero edition: null rows remain, the origin/fighting_style/epic_boon categories are all exclusively EDITION_2024-tagged, and the crossEditionRejection guard fires before featOfferedForAsiSlot so the function's edition-invariance claim is safe even if a client submits a cross-edition feat ID.

The removal of the edition parameter from featOfferedForAsiSlot — and its eslint-disable suppression — is correct and clean. The logic really is edition-invariant now that the data enforces edition separation.


Comment drift (CLAUDE.md: "update or delete its comment in the same edit")

cross-edition-catalog-id.test.ts:40–41 — this comment was not touched by the PR, but the change to Alert 2014's category makes it false:

// The seeded Alert fork (#1306's worked example) is unusable here: both rows
// are category "origin", rejected by featOfferedForAsiSlot BEFORE the guard
// runs — a naive test against Alert would pass for the wrong reason.

After this PR, Alert 2014 is category: "general" — only Alert 2024 remains "origin". The conclusion (use fixture rows instead of Alert) is still defensible because one of the two Alert rows is still origin-gated, but "both rows are category 'origin'" is now factually wrong. The comment should narrow to "Alert 2024 is category 'origin'" or simply drop the row-level claim in favour of "the Alert 2024 row is 'origin'-gated before the guard fires".

Fix this →


Test label imprecision (nit, not a bug)

feats-2014-general-take.test.ts:80 — the it name says "the ?? 4 general default, live" but the character is created at experiencePoints: 0 (level 1). A Fighter has zero ASI slots at level 1, so the 400 rejection fires from the "no available ASI slot" check before featOfferedForAsiSlot is ever called — not from the ?? 4 gate itself.

The actual ?? 4 live proof is the atThree = 0, atFour = 26 pair in feats.test.ts. The test result is correct; only the label overstates which specific gate fired. Worth fixing the label to "a 2014 character at level 1 has no ASI slot to take a feat" so a future reader doesn't look for ?? 4 proof here.


Rest of the changes — no issues found

Area Assessment
feats.ts seed data 26 new EDITION_2014 rows correctly structured; file-header comment is exemplary. The originFeatName property-based test is a clever future-proofing guard that would catch a gap on any future background change.
srd/feats.ts Edition-invariant simplification is correct and well-commented. The removed RulesEdition import and the eslint suppression are gone cleanly.
advancement.ts crossEditionRejection fires at line 371 before featOfferedForAsiSlot at line 382 — so passing a 2014 feat ID to a 2024 character reports the edition mismatch first, not a category error. The test at feats-2014-general-take.test.ts:113 verifies this with the right /2014 rules/ + /2024 rules/ regex.
catalog/feats.ts One-line removal of the stale edition arg; straightforward.
seed-data.test.ts The inverted Grappler assertion, the Alert category assertions, and the new 2014 describe block all correctly guard the new data invariants.
species-creation-choices-2024.test.ts Correct fix for the non-deterministic featByName — the bare-name lookup needed pinning once every General/Origin name got a 2014 sibling.
Content accuracy Grappler (SRD 5.1 via dnd5eapi.co), Savage Attacker (two independent published transcriptions), remaining 24 verbatim from git history — provenance clearly documented in comments.

One confirmed comment-drift finding, one test-label nit. Both are straightforward fixes.

- cross-edition-catalog-id.test.ts: the "both rows are category origin"
  comment went stale when this PR moved Alert 2014's category to
  "general" — only the Alert 2024 row is origin-gated now. Narrowed the
  comment to that fact; the conclusion (use a general-category fixture
  trio instead) is unchanged.
- feats-2014-general-take.test.ts: the level-1/no-XP case's label
  claimed to prove featOfferedForAsiSlot's `?? 4` default, but a
  level-1 character has zero ASI slots, so the 400 comes from the
  slot-availability check, not the category default. Renamed to match
  what the test actually exercises; the real `?? 4` proof is the
  atThree=0/atFour=26 pair in feats.test.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018rre9Ho8Vx8zNtzzKkpvFn
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @Sandersland's task in 1m 14s —— View job


Code Review: PR #1731 — 2014 Feat catalog rows (re-review)

  • Gather context
  • Read diff and verify fixes from prior review
  • Check cross-edition-catalog-id.test.ts comment fix
  • Check feats-2014-general-take.test.ts label fix
  • Full diff review for new issues

Fixes from prior review — both verified ✓

cross-edition-catalog-id.test.ts:40–41 — Comment correctly narrowed. The previous "both rows are category 'origin'" claim is gone; it now reads "the Alert 2024 row is 'origin'-gated, rejected by featOfferedForAsiSlot BEFORE the guard runs". The conclusion (use fixture rows instead) is preserved unchanged.

feats-2014-general-take.test.ts:88–93 — Label correctly updated. The it description now reads "a 2014 character at level 1 has no ASI slot to take a feat", and a new inline comment correctly routes the reader to the feats.test.ts atThree=0/atFour=26 pair for the real ?? 4 proof.

Both changes are accurate and exactly match what the prior review requested.


No new issues found

The fix commit (2232a5b) only touched the two files flagged, and both changes are tight surgical edits with no scope creep. No comment drift, no logic changes, no new test imprecision introduced.


Verdict: All prior findings resolved. PR is clean.

@Sandersland
Sandersland merged commit b9b7b2f into staging Aug 5, 2026
7 checks passed
@Sandersland
Sandersland deleted the feat/1310-2014-feats branch August 5, 2026 01:35
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.

2014 feats: author the 2014 Feat catalog rows behind the edition tag

1 participant