Add ADRs for branded types, bestiary loading, and pre-commit gates
ADR-003: Branded types for compile-time identity safety at zero runtime cost. ADR-004: On-demand bestiary via compact index + IndexedDB cache, avoiding distribution of copyrighted content. ADR-005: All quality gates at pre-commit for tight agent feedback loops, with analysis of per-change hooks as a future option. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
53
docs/adr/003-branded-types-for-identity.md
Normal file
53
docs/adr/003-branded-types-for-identity.md
Normal file
@@ -0,0 +1,53 @@
|
||||
# ADR-003: Branded Types for Identity Safety
|
||||
|
||||
**Date**: 2026-03-25
|
||||
**Status**: accepted
|
||||
|
||||
## Context
|
||||
|
||||
The domain model has multiple entity types with string-based identifiers: combatants, creatures, and player characters. All IDs are strings at runtime (UUIDs or slug-based), making it easy to accidentally pass one ID type where another is expected. Such bugs are silent — the code compiles, runs, and only fails at runtime when a lookup returns `undefined` or mutates the wrong entity.
|
||||
|
||||
TypeScript's structural type system treats all `string` values as interchangeable, so a plain `string` type alias provides no protection.
|
||||
|
||||
## Decision
|
||||
|
||||
Identity types use TypeScript branded types — a `string` intersected with a phantom `readonly __brand` property that exists only at the type level:
|
||||
|
||||
```typescript
|
||||
type CombatantId = string & { readonly __brand: "CombatantId" };
|
||||
type CreatureId = string & { readonly __brand: "CreatureId" };
|
||||
type PlayerCharacterId = string & { readonly __brand: "PlayerCharacterId" };
|
||||
```
|
||||
|
||||
Each type has a factory function that casts a plain string into the branded type:
|
||||
|
||||
```typescript
|
||||
function combatantId(id: string): CombatantId {
|
||||
return id as CombatantId;
|
||||
}
|
||||
```
|
||||
|
||||
The `__brand` property is never assigned at runtime — it's a compile-time-only construct. The cast in the factory is the single point where the type system is "convinced" that the string carries the brand.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
**Plain `string` type aliases** (`type CombatantId = string`) — provides documentation value but zero type safety. TypeScript treats the alias as fully interchangeable with `string` and with all other string aliases. This is what most TypeScript codebases do, accepting the risk of ID confusion.
|
||||
|
||||
**Opaque types via unique symbols** (`declare const brand: unique symbol; type CombatantId = string & { [brand]: void }`) — stricter than the `__brand` approach because the symbol is truly unique and unexportable. Slightly more boilerplate and harder to read. The simpler `__brand` string approach provides sufficient safety for this codebase's scale.
|
||||
|
||||
**Wrapper classes** (`class CombatantId { constructor(public readonly value: string) {} }`) — provides nominal typing naturally, but introduces runtime overhead (object allocation, `.value` access everywhere), breaks JSON serialization, and doesn't play well with the project's preference for plain data over classes.
|
||||
|
||||
**Runtime validation** (check ID format at every function boundary) — catches errors at runtime but not at compile time. Adds overhead and doesn't prevent the bug from being written in the first place.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
- Passing a `CreatureId` where a `CombatantId` is expected produces a compile-time error — the bug is caught before the code runs.
|
||||
- Zero runtime cost. The brand is erased during compilation; at runtime, IDs are plain strings.
|
||||
- JSON serialization works naturally — no custom serializers needed for persistence or network transport.
|
||||
- Factory functions (`combatantId()`, `creatureId()`) serve as explicit construction points, making it clear where IDs originate.
|
||||
|
||||
**Negative:**
|
||||
- The `as CombatantId` cast in factory functions is an escape hatch from the type system. If misused (casting arbitrary strings elsewhere), the safety guarantee is undermined. In practice, casts are confined to factory functions and adapter-layer deserialization.
|
||||
- The `__brand` property appears in IDE autocomplete and hover tooltips, which can be confusing for developers unfamiliar with the pattern.
|
||||
- Branded types are a community convention, not a TypeScript language feature. There is no official syntax or standard library support.
|
||||
42
docs/adr/004-on-demand-bestiary-loading.md
Normal file
42
docs/adr/004-on-demand-bestiary-loading.md
Normal file
@@ -0,0 +1,42 @@
|
||||
# ADR-004: On-Demand Bestiary Loading via Compact Index and IndexedDB Cache
|
||||
|
||||
**Date**: 2026-03-25
|
||||
**Status**: accepted
|
||||
|
||||
## Context
|
||||
|
||||
The application integrates a D&D creature bestiary containing 3,300+ creatures from the 5etools dataset. The full bestiary data (stat blocks, traits, actions, spellcasting) is several megabytes of JSON. Bundling it directly into the application would create two problems: a large initial download for every user, and the distribution of copyrighted game content as part of the application bundle.
|
||||
|
||||
## Decision
|
||||
|
||||
The bestiary is split into two tiers:
|
||||
|
||||
1. **Compact search index** (`data/bestiary/index.json`, ~350KB) — shipped with the application bundle. Contains only the fields needed for search and display in the autocomplete dropdown: name, source, AC, HP, DEX, CR, initiative proficiency, size, and type. Field names are abbreviated (`n`, `s`, `ac`, `hp`, `dx`, `cr`, `ip`, `sz`, `tp`) to minimize file size. Generated offline by `scripts/generate-bestiary-index.mjs` from a local clone of the 5etools repository.
|
||||
|
||||
2. **On-demand source data** — full creature stat blocks are fetched per-source when a user first needs them (e.g., when viewing a stat block or adding a creature with HP/AC pre-fill). Fetched data is cached in IndexedDB (`initiative-bestiary` database) via the `idb` library, with an in-memory Map fallback when IndexedDB is unavailable. Users can also upload source files directly or bulk-import all sources.
|
||||
|
||||
The application never bundles or redistributes the full creature data. Users fetch it themselves from their own configured source URLs.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
**Bundle all bestiary data** — simplest approach, used during early development. Eliminated because it would distribute copyrighted content in the application bundle and inflate the initial download by several megabytes. Most users only need a fraction of the available sources.
|
||||
|
||||
**Server-side API** — a backend service could serve creature data on demand. This would keep the client lightweight and solve the bundle size concern, but the copyright issue remains — we would still be distributing copyrighted content, just from a server instead of a bundle. It also contradicts the project's local-first, single-user, no-backend architecture and would require hosting infrastructure and a network dependency for basic functionality.
|
||||
|
||||
**Service Worker with lazy caching** — fetch and cache bestiary data transparently via a Service Worker. More complex to implement and debug than explicit IndexedDB caching. The explicit approach gives users visibility and control over which sources are cached (via the source manager UI).
|
||||
|
||||
**localStorage for caching** — simpler API than IndexedDB, but localStorage has a ~5MB limit per origin, which is insufficient for multiple bestiary sources. IndexedDB has no practical storage limit.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
- The application does not distribute copyrighted game content. Users fetch data from their own sources.
|
||||
- Initial bundle stays small (~350KB for the search index). The full bestiary data is only downloaded when needed and then cached locally.
|
||||
- Offline capability: once sources are cached in IndexedDB, creature data is available without network access.
|
||||
- Users have explicit control over cached sources (import, clear, manage via UI).
|
||||
|
||||
**Negative:**
|
||||
- First-time use requires fetching source data before full stat blocks are available. The bulk import feature mitigates this but requires an initial download.
|
||||
- The search index must be regenerated manually when the upstream 5etools dataset changes. In practice this is infrequent (new D&D source books release a few times per year), so a manual process triggered by a new book release is sufficient at this scale.
|
||||
- Two separate data representations (compact index vs full source) must be kept conceptually in sync. A creature that appears in the index but whose source hasn't been fetched will show limited information until the source is cached.
|
||||
- IndexedDB adds adapter complexity (async API, database versioning, migration handling) compared to the synchronous localStorage used for encounter persistence.
|
||||
58
docs/adr/005-all-quality-gates-at-pre-commit.md
Normal file
58
docs/adr/005-all-quality-gates-at-pre-commit.md
Normal file
@@ -0,0 +1,58 @@
|
||||
# ADR-005: All Quality Gates at Pre-Commit
|
||||
|
||||
**Date**: 2026-03-25
|
||||
**Status**: accepted
|
||||
|
||||
## Context
|
||||
|
||||
This project is developed primarily through agentic coding — AI coding agents generate and modify code under human supervision. Agents are highly productive but can drift from established conventions, introduce subtle style inconsistencies, or produce code that compiles but doesn't meet the project's quality standards.
|
||||
|
||||
The conventional approach in most software projects is to keep pre-commit hooks lightweight (formatting, maybe linting) and defer heavier checks (tests, type checking, coverage, copy-paste detection) to CI pipelines. This optimizes for developer speed at commit time.
|
||||
|
||||
However, when working with AI agents, the dynamics are different. Agents iterate quickly and can fix issues immediately — but only if they receive feedback immediately. A failing CI pipeline minutes later breaks the feedback loop: the agent's context has moved on, and the human must re-engage to address the failure.
|
||||
|
||||
## Decision
|
||||
|
||||
All quality gates run at pre-commit via Lefthook, as a single sequential `pnpm check` command. No gate may exist only as a CI step or as a manual process. The full gate sequence is:
|
||||
|
||||
1. `pnpm audit --audit-level=high` — security vulnerability scan
|
||||
2. `knip` — unused code detection
|
||||
3. `biome check .` — linting and formatting (50+ rules)
|
||||
4. `oxlint --tsconfig ... --type-aware` — type-aware linting
|
||||
5. `check-lint-ignores.mjs` — caps biome-ignore directives
|
||||
6. `check-cn-classnames.mjs` — bans template-literal classNames
|
||||
7. `check-component-props.mjs` — max 8 props per component
|
||||
8. `tsc --build` — TypeScript type checking
|
||||
9. `vitest run` — tests with per-path coverage thresholds
|
||||
10. `jscpd` — copy-paste detection
|
||||
|
||||
Layer boundary enforcement runs as a Vitest test within step 9.
|
||||
|
||||
This takes ~8 seconds on the current codebase. Every commit is guaranteed to pass all checks.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
**Lightweight pre-commit, full checks in CI** — the industry default. Pre-commit runs only formatting and basic linting; tests, type checking, and coverage run in a CI pipeline. This is faster at commit time but creates a delayed feedback loop. For agentic coding workflows, this delay is costly: the agent produces a commit, moves on, and the CI failure arrives minutes later when context has shifted. The human must re-engage the agent with the failure context, losing the tight iteration loop.
|
||||
|
||||
**No pre-commit hooks, CI only** — maximum commit speed, all enforcement in CI. Risks accumulating multiple broken commits before issues surface. Particularly problematic with agents that commit frequently.
|
||||
|
||||
**Selective pre-commit (fast checks only)** — run formatting, linting, and type checking at pre-commit; defer tests and coverage to CI as a compromise. Still breaks the feedback loop for test failures and coverage regressions, which are the checks most likely to catch agent-introduced bugs.
|
||||
|
||||
**Per-change hooks (e.g., Claude Code hooks)** — run checks after every file edit or tool call, not just at commit time. This is an even tighter feedback loop than pre-commit: the agent learns about a violation seconds after introducing it, before more code is written on top of it. Claude Code supports hooks that trigger on events like `PostToolUse`, which could run linting or type checking after every file write.
|
||||
|
||||
However, running the full gate after every edit breaks test-driven workflows: writing a test before its implementation, or updating implementation before updating tests, produces intermediate states that legitimately fail type checking or tests. Scoping hooks to only fast, non-breaking checks (formatting, linting) would avoid this, but splits the gate into two tiers — adding complexity for unclear benefit when pre-commit already catches everything within ~8 seconds.
|
||||
|
||||
Pre-commit is the current sweet spot: tight enough that agents get feedback in the same context window, but not so tight that it interferes with red-green-refactor or incremental editing. Per-change hooks remain a future option if the codebase grows to a point where pre-commit becomes too slow.
|
||||
|
||||
## Consequences
|
||||
|
||||
**Positive:**
|
||||
- Early backpressure in short feedback loops. Agents receive immediate, comprehensive feedback on every commit attempt. If a check fails, the agent can fix it in the same context window, maintaining continuity.
|
||||
- Every commit on `main` is guaranteed to pass all quality gates. There is no state where "it compiled but the tests are broken" or "formatting drifted."
|
||||
- No CI/local divergence. The same checks run everywhere, eliminating "works on my machine" or "CI caught something pre-commit didn't."
|
||||
- Enforces discipline incrementally: each commit is small, clean, and complete rather than "I'll fix the tests later."
|
||||
|
||||
**Negative:**
|
||||
- ~8 seconds per commit attempt. This is acceptable for the current codebase size but will grow with the test suite. If it exceeds ~15 seconds, selective pre-commit with CI for the rest may become necessary.
|
||||
- Developers (or agents) cannot make quick "WIP" or "checkpoint" commits without passing all gates. This is intentional — every commit should be a valid state — but it prevents some workflows like committing broken code to switch branches.
|
||||
- The sequential chain means a failure in step 1 (audit) prevents discovering failures in step 9 (tests). In practice, this rarely matters because failures are fixed immediately and the chain is re-run.
|
||||
Reference in New Issue
Block a user