From a9df826fef12d769f1a98857720b95ae0ab8b560 Mon Sep 17 00:00:00 2001 From: Lukas Date: Wed, 4 Mar 2026 10:05:13 +0100 Subject: [PATCH] Implement the 004-edit-combatant feature that adds the possibility to change a combatants name --- apps/web/src/App.tsx | 74 +++++++- apps/web/src/hooks/use-encounter.ts | 15 ++ .../src/edit-combatant-use-case.ts | 24 +++ packages/application/src/index.ts | 1 + .../src/__tests__/edit-combatant.test.ts | 163 ++++++++++++++++++ packages/domain/src/edit-combatant.ts | 62 +++++++ packages/domain/src/events.ts | 10 +- packages/domain/src/index.ts | 5 + .../checklists/requirements.md | 34 ++++ .../contracts/domain-contract.md | 37 ++++ specs/004-edit-combatant/data-model.md | 59 +++++++ specs/004-edit-combatant/plan.md | 70 ++++++++ specs/004-edit-combatant/quickstart.md | 41 +++++ specs/004-edit-combatant/research.md | 40 +++++ specs/004-edit-combatant/spec.md | 77 +++++++++ specs/004-edit-combatant/tasks.md | 147 ++++++++++++++++ 16 files changed, 854 insertions(+), 5 deletions(-) create mode 100644 packages/application/src/edit-combatant-use-case.ts create mode 100644 packages/domain/src/__tests__/edit-combatant.test.ts create mode 100644 packages/domain/src/edit-combatant.ts create mode 100644 specs/004-edit-combatant/checklists/requirements.md create mode 100644 specs/004-edit-combatant/contracts/domain-contract.md create mode 100644 specs/004-edit-combatant/data-model.md create mode 100644 specs/004-edit-combatant/plan.md create mode 100644 specs/004-edit-combatant/quickstart.md create mode 100644 specs/004-edit-combatant/research.md create mode 100644 specs/004-edit-combatant/spec.md create mode 100644 specs/004-edit-combatant/tasks.md diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 54528e2..070c2c1 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -1,4 +1,5 @@ -import { type FormEvent, useState } from "react"; +import type { CombatantId } from "@initiative/domain"; +import { type FormEvent, useCallback, useRef, useState } from "react"; import { useEncounter } from "./hooks/use-encounter"; function formatEvent(e: ReturnType["events"][number]) { @@ -11,12 +12,72 @@ function formatEvent(e: ReturnType["events"][number]) { return `Added combatant: ${e.name}`; case "CombatantRemoved": return `Removed combatant: ${e.name}`; + case "CombatantUpdated": + return `Renamed combatant: ${e.oldName} → ${e.newName}`; } } +function EditableName({ + name, + combatantId, + isActive, + onRename, +}: { + name: string; + combatantId: CombatantId; + isActive: boolean; + onRename: (id: CombatantId, newName: string) => void; +}) { + const [editing, setEditing] = useState(false); + const [draft, setDraft] = useState(name); + const inputRef = useRef(null); + + const commit = useCallback(() => { + const trimmed = draft.trim(); + if (trimmed !== "" && trimmed !== name) { + onRename(combatantId, trimmed); + } + setEditing(false); + }, [draft, name, combatantId, onRename]); + + const startEditing = useCallback(() => { + setDraft(name); + setEditing(true); + requestAnimationFrame(() => inputRef.current?.select()); + }, [name]); + + if (editing) { + return ( + setDraft(e.target.value)} + onBlur={commit} + onKeyDown={(e) => { + if (e.key === "Enter") commit(); + if (e.key === "Escape") setEditing(false); + }} + /> + ); + } + + return ( + + ); +} + export function App() { - const { encounter, events, advanceTurn, addCombatant, removeCombatant } = - useEncounter(); + const { + encounter, + events, + advanceTurn, + addCombatant, + removeCombatant, + editCombatant, + } = useEncounter(); const activeCombatant = encounter.combatants[encounter.activeIndex]; const [nameInput, setNameInput] = useState(""); @@ -40,7 +101,12 @@ export function App() {
    {encounter.combatants.map((c, i) => (
  • - {i === encounter.activeIndex ? `▶ ${c.name}` : c.name}{" "} + {" "} diff --git a/apps/web/src/hooks/use-encounter.ts b/apps/web/src/hooks/use-encounter.ts index a59d832..368e132 100644 --- a/apps/web/src/hooks/use-encounter.ts +++ b/apps/web/src/hooks/use-encounter.ts @@ -2,6 +2,7 @@ import type { EncounterStore } from "@initiative/application"; import { addCombatantUseCase, advanceTurnUseCase, + editCombatantUseCase, removeCombatantUseCase, } from "@initiative/application"; import type { CombatantId, DomainEvent, Encounter } from "@initiative/domain"; @@ -78,11 +79,25 @@ export function useEncounter() { [makeStore], ); + const editCombatant = useCallback( + (id: CombatantId, newName: string) => { + const result = editCombatantUseCase(makeStore(), id, newName); + + if (isDomainError(result)) { + return; + } + + setEvents((prev) => [...prev, ...result]); + }, + [makeStore], + ); + return { encounter, events, advanceTurn, addCombatant, removeCombatant, + editCombatant, } as const; } diff --git a/packages/application/src/edit-combatant-use-case.ts b/packages/application/src/edit-combatant-use-case.ts new file mode 100644 index 0000000..4895d6c --- /dev/null +++ b/packages/application/src/edit-combatant-use-case.ts @@ -0,0 +1,24 @@ +import { + type CombatantId, + type DomainError, + type DomainEvent, + editCombatant, + isDomainError, +} from "@initiative/domain"; +import type { EncounterStore } from "./ports.js"; + +export function editCombatantUseCase( + store: EncounterStore, + id: CombatantId, + newName: string, +): DomainEvent[] | DomainError { + const encounter = store.get(); + const result = editCombatant(encounter, id, newName); + + if (isDomainError(result)) { + return result; + } + + store.save(result.encounter); + return result.events; +} diff --git a/packages/application/src/index.ts b/packages/application/src/index.ts index b243a15..80eb064 100644 --- a/packages/application/src/index.ts +++ b/packages/application/src/index.ts @@ -1,4 +1,5 @@ export { addCombatantUseCase } from "./add-combatant-use-case.js"; export { advanceTurnUseCase } from "./advance-turn-use-case.js"; +export { editCombatantUseCase } from "./edit-combatant-use-case.js"; export type { EncounterStore } from "./ports.js"; export { removeCombatantUseCase } from "./remove-combatant-use-case.js"; diff --git a/packages/domain/src/__tests__/edit-combatant.test.ts b/packages/domain/src/__tests__/edit-combatant.test.ts new file mode 100644 index 0000000..21f8050 --- /dev/null +++ b/packages/domain/src/__tests__/edit-combatant.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from "vitest"; +import { editCombatant } from "../edit-combatant.js"; +import type { Combatant, Encounter } from "../types.js"; +import { combatantId, isDomainError } from "../types.js"; + +// --- Helpers --- + +function makeCombatant(name: string): Combatant { + return { id: combatantId(name), name }; +} + +const Alice = makeCombatant("Alice"); +const Bob = makeCombatant("Bob"); + +function enc( + combatants: Combatant[], + activeIndex = 0, + roundNumber = 1, +): Encounter { + return { combatants, activeIndex, roundNumber }; +} + +function successResult(encounter: Encounter, id: string, newName: string) { + const result = editCombatant(encounter, combatantId(id), newName); + if (isDomainError(result)) { + throw new Error(`Expected success, got error: ${result.message}`); + } + return result; +} + +// --- Acceptance Scenarios (T004) --- + +describe("editCombatant", () => { + describe("acceptance scenarios", () => { + it("scenario 1: rename succeeds with correct event containing combatantId, oldName, newName", () => { + const e = enc([Alice, Bob]); + const { encounter, events } = successResult(e, "Bob", "Robert"); + + expect(encounter.combatants[1]).toEqual({ + id: combatantId("Bob"), + name: "Robert", + }); + expect(events).toEqual([ + { + type: "CombatantUpdated", + combatantId: combatantId("Bob"), + oldName: "Bob", + newName: "Robert", + }, + ]); + }); + + it("scenario 2: activeIndex and roundNumber preserved when renaming the active combatant", () => { + const e = enc([Alice, Bob], 1, 3); + const { encounter } = successResult(e, "Bob", "Robert"); + + expect(encounter.activeIndex).toBe(1); + expect(encounter.roundNumber).toBe(3); + expect(encounter.combatants[1].name).toBe("Robert"); + }); + + it("scenario 3: combatant list order preserved", () => { + const Cael = makeCombatant("Cael"); + const e = enc([Alice, Bob, Cael]); + const { encounter } = successResult(e, "Bob", "Robert"); + + expect(encounter.combatants.map((c) => c.name)).toEqual([ + "Alice", + "Robert", + "Cael", + ]); + }); + + it("scenario 4: renaming to same name still emits event", () => { + const e = enc([Alice, Bob]); + const { encounter, events } = successResult(e, "Bob", "Bob"); + + expect(encounter.combatants[1].name).toBe("Bob"); + expect(events).toHaveLength(1); + expect(events[0]).toEqual({ + type: "CombatantUpdated", + combatantId: combatantId("Bob"), + oldName: "Bob", + newName: "Bob", + }); + }); + }); + + // --- Invariant Tests (T005) --- + + describe("invariants", () => { + it("INV-1: determinism — same inputs produce same outputs", () => { + const e = enc([Alice, Bob], 1, 3); + const result1 = editCombatant(e, combatantId("Alice"), "Aria"); + const result2 = editCombatant(e, combatantId("Alice"), "Aria"); + expect(result1).toEqual(result2); + }); + + it("INV-2: exactly one event emitted on success", () => { + const e = enc([Alice, Bob]); + const { events } = successResult(e, "Alice", "Aria"); + expect(events).toHaveLength(1); + expect(events[0].type).toBe("CombatantUpdated"); + }); + + it("INV-3: original encounter is not mutated", () => { + const e = enc([Alice, Bob], 0, 1); + const originalCombatants = [...e.combatants]; + const originalActiveIndex = e.activeIndex; + const originalRoundNumber = e.roundNumber; + + successResult(e, "Alice", "Aria"); + + expect(e.combatants).toEqual(originalCombatants); + expect(e.activeIndex).toBe(originalActiveIndex); + expect(e.roundNumber).toBe(originalRoundNumber); + }); + }); + + // --- Error Scenarios (T011) --- + + describe("error scenarios", () => { + it("non-existent id returns combatant-not-found error", () => { + const e = enc([Alice, Bob]); + const result = editCombatant(e, combatantId("nonexistent"), "NewName"); + + expect(isDomainError(result)).toBe(true); + if (isDomainError(result)) { + expect(result.code).toBe("combatant-not-found"); + } + }); + + it("empty name returns invalid-name error", () => { + const e = enc([Alice, Bob]); + const result = editCombatant(e, combatantId("Alice"), ""); + + expect(isDomainError(result)).toBe(true); + if (isDomainError(result)) { + expect(result.code).toBe("invalid-name"); + } + }); + + it("whitespace-only name returns invalid-name error", () => { + const e = enc([Alice, Bob]); + const result = editCombatant(e, combatantId("Alice"), " "); + + expect(isDomainError(result)).toBe(true); + if (isDomainError(result)) { + expect(result.code).toBe("invalid-name"); + } + }); + + it("empty encounter returns combatant-not-found for any id", () => { + const e = enc([]); + const result = editCombatant(e, combatantId("any"), "Name"); + + expect(isDomainError(result)).toBe(true); + if (isDomainError(result)) { + expect(result.code).toBe("combatant-not-found"); + } + }); + }); +}); diff --git a/packages/domain/src/edit-combatant.ts b/packages/domain/src/edit-combatant.ts new file mode 100644 index 0000000..70fcf14 --- /dev/null +++ b/packages/domain/src/edit-combatant.ts @@ -0,0 +1,62 @@ +import type { DomainEvent } from "./events.js"; +import type { CombatantId, DomainError, Encounter } from "./types.js"; + +export interface EditCombatantSuccess { + readonly encounter: Encounter; + readonly events: DomainEvent[]; +} + +/** + * Pure function that renames a combatant in an encounter by ID. + * + * FR-001: Accepts Encounter, CombatantId, and newName; returns next state + events. + * FR-002: Emits a CombatantUpdated event with combatantId, oldName, newName. + * FR-004: Rejects empty/whitespace-only names with DomainError. + * FR-005: Preserves activeIndex and roundNumber. + * FR-006: Preserves combatant list order. + */ +export function editCombatant( + encounter: Encounter, + id: CombatantId, + newName: string, +): EditCombatantSuccess | DomainError { + const trimmed = newName.trim(); + + if (trimmed === "") { + return { + kind: "domain-error", + code: "invalid-name", + message: "Combatant name must not be empty", + }; + } + + const index = encounter.combatants.findIndex((c) => c.id === id); + + if (index === -1) { + return { + kind: "domain-error", + code: "combatant-not-found", + message: `No combatant found with ID "${id}"`, + }; + } + + const oldName = encounter.combatants[index].name; + + return { + encounter: { + combatants: encounter.combatants.map((c) => + c.id === id ? { ...c, name: trimmed } : c, + ), + activeIndex: encounter.activeIndex, + roundNumber: encounter.roundNumber, + }, + events: [ + { + type: "CombatantUpdated", + combatantId: id, + oldName, + newName: trimmed, + }, + ], + }; +} diff --git a/packages/domain/src/events.ts b/packages/domain/src/events.ts index df2e817..8763751 100644 --- a/packages/domain/src/events.ts +++ b/packages/domain/src/events.ts @@ -25,8 +25,16 @@ export interface CombatantRemoved { readonly name: string; } +export interface CombatantUpdated { + readonly type: "CombatantUpdated"; + readonly combatantId: CombatantId; + readonly oldName: string; + readonly newName: string; +} + export type DomainEvent = | TurnAdvanced | RoundAdvanced | CombatantAdded - | CombatantRemoved; + | CombatantRemoved + | CombatantUpdated; diff --git a/packages/domain/src/index.ts b/packages/domain/src/index.ts index 05e60f1..ea6fef4 100644 --- a/packages/domain/src/index.ts +++ b/packages/domain/src/index.ts @@ -1,8 +1,13 @@ export { type AddCombatantSuccess, addCombatant } from "./add-combatant.js"; export { advanceTurn } from "./advance-turn.js"; +export { + type EditCombatantSuccess, + editCombatant, +} from "./edit-combatant.js"; export type { CombatantAdded, CombatantRemoved, + CombatantUpdated, DomainEvent, RoundAdvanced, TurnAdvanced, diff --git a/specs/004-edit-combatant/checklists/requirements.md b/specs/004-edit-combatant/checklists/requirements.md new file mode 100644 index 0000000..2406d17 --- /dev/null +++ b/specs/004-edit-combatant/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Edit Combatant + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-03-03 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass. Spec is ready for `/speckit.clarify` or `/speckit.plan`. diff --git a/specs/004-edit-combatant/contracts/domain-contract.md b/specs/004-edit-combatant/contracts/domain-contract.md new file mode 100644 index 0000000..7c2b75c --- /dev/null +++ b/specs/004-edit-combatant/contracts/domain-contract.md @@ -0,0 +1,37 @@ +# Domain Contract: editCombatant + +## Function Signature + +``` +editCombatant(encounter, id, newName) → EditCombatantSuccess | DomainError +``` + +### Inputs + +| Parameter | Type | Description | +|-----------|------|-------------| +| encounter | Encounter | Current encounter state | +| id | CombatantId | Identity of combatant to rename | +| newName | string | New name to assign | + +### Success Output + +| Field | Type | Description | +|-------|------|-------------| +| encounter | Encounter | Updated encounter with renamed combatant | +| events | DomainEvent[] | Exactly one `CombatantUpdated` event | + +### Error Output + +| Code | Condition | +|------|-----------| +| `"combatant-not-found"` | No combatant with given id exists | +| `"invalid-name"` | newName is empty or whitespace-only | + +## Hook Contract + +`useEncounter()` returns an additional action: + +| Method | Signature | Description | +|--------|-----------|-------------| +| editCombatant | `(id: CombatantId, newName: string) => void` | Rename combatant, append events on success | diff --git a/specs/004-edit-combatant/data-model.md b/specs/004-edit-combatant/data-model.md new file mode 100644 index 0000000..e7b821f --- /dev/null +++ b/specs/004-edit-combatant/data-model.md @@ -0,0 +1,59 @@ +# Data Model: Edit Combatant + +**Feature**: 004-edit-combatant +**Date**: 2026-03-03 + +## Entities + +### Combatant (unchanged) + +| Field | Type | Notes | +|-------|------|-------| +| id | CombatantId (branded string) | Immutable identity | +| name | string | Mutable — this feature updates it | + +### Encounter (unchanged structure) + +| Field | Type | Notes | +|-------|------|-------| +| combatants | readonly Combatant[] | Edit replaces name in-place by mapping | +| activeIndex | number | Preserved during edit | +| roundNumber | number | Preserved during edit | + +## Events + +### CombatantUpdated (new) + +| Field | Type | Notes | +|-------|------|-------| +| type | "CombatantUpdated" | Discriminant | +| combatantId | CombatantId | Which combatant was renamed | +| oldName | string | Name before edit | +| newName | string | Name after edit | + +Added to the `DomainEvent` union type. + +## State Transitions + +### editCombatant(encounter, id, newName) + +**Preconditions**: +- `newName` is non-empty and not whitespace-only +- `id` matches a combatant in `encounter.combatants` + +**Postconditions**: +- The combatant with matching `id` has `name` set to `newName` +- `activeIndex` and `roundNumber` unchanged +- Combatant list order unchanged +- Exactly one `CombatantUpdated` event emitted + +**Error cases**: +- `id` not found → `DomainError { code: "combatant-not-found" }` +- `newName` empty/whitespace → `DomainError { code: "invalid-name" }` + +## Validation Rules + +| Rule | Condition | Error Code | +|------|-----------|------------| +| Name must be non-empty | `newName.trim().length === 0` | `"invalid-name"` | +| Combatant must exist | No combatant with matching `id` | `"combatant-not-found"` | diff --git a/specs/004-edit-combatant/plan.md b/specs/004-edit-combatant/plan.md new file mode 100644 index 0000000..148c7f7 --- /dev/null +++ b/specs/004-edit-combatant/plan.md @@ -0,0 +1,70 @@ +# Implementation Plan: Edit Combatant + +**Branch**: `004-edit-combatant` | **Date**: 2026-03-03 | **Spec**: [spec.md](./spec.md) +**Input**: Feature specification from `/specs/004-edit-combatant/spec.md` + +## Summary + +Add the ability to rename a combatant by id within an encounter. A pure domain function `editCombatant` validates the id and new name, returns the updated encounter with a `CombatantUpdated` event, or a `DomainError`. Wired through an application use case and exposed via the existing `useEncounter` hook to a minimal UI control. + +## Technical Context + +**Language/Version**: TypeScript 5.x (strict mode, verbatimModuleSyntax) +**Primary Dependencies**: React 19, Vite +**Storage**: In-memory React state (local-first, single-user MVP) +**Testing**: Vitest +**Target Platform**: Browser (localhost:5173) +**Project Type**: Web application (monorepo: domain → application → web) +**Performance Goals**: N/A — single-user local state, instant updates +**Constraints**: Pure domain logic, no I/O in domain layer +**Scale/Scope**: Single-user encounter tracker + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. Deterministic Domain Core | PASS | `editCombatant` is a pure function: same encounter + id + name → same result | +| II. Layered Architecture | PASS | Domain function → use case → hook/UI. No layer violations. | +| III. Agent Boundary | N/A | No agent layer involvement | +| IV. Clarification-First | PASS | Spec is complete, no ambiguities remain | +| V. Escalation Gates | PASS | All work is within spec scope | +| VI. MVP Baseline Language | PASS | Spec uses "MVP baseline does not include" for out-of-scope items | +| VII. No Gameplay Rules | PASS | Constitution contains no gameplay logic | + +## Project Structure + +### Documentation (this feature) + +```text +specs/004-edit-combatant/ +├── plan.md +├── research.md +├── data-model.md +├── quickstart.md +├── contracts/ +└── tasks.md +``` + +### Source Code (repository root) + +```text +packages/domain/src/ +├── edit-combatant.ts # New: pure editCombatant function +├── events.ts # Modified: add CombatantUpdated event +├── types.ts # Unchanged (Combatant, Encounter, DomainError) +├── index.ts # Modified: re-export editCombatant +└── __tests__/ + └── edit-combatant.test.ts # New: acceptance + invariant tests + +packages/application/src/ +├── edit-combatant-use-case.ts # New: use case wiring +└── index.ts # Modified: re-export use case + +apps/web/src/ +├── hooks/use-encounter.ts # Modified: add editCombatant action +└── App.tsx # Modified: add rename UI control +``` + +**Structure Decision**: Follows the existing monorepo layout (`packages/domain` → `packages/application` → `apps/web`). Each new file mirrors the pattern established by `add-combatant` and `remove-combatant`. diff --git a/specs/004-edit-combatant/quickstart.md b/specs/004-edit-combatant/quickstart.md new file mode 100644 index 0000000..c97197e --- /dev/null +++ b/specs/004-edit-combatant/quickstart.md @@ -0,0 +1,41 @@ +# Quickstart: Edit Combatant + +**Feature**: 004-edit-combatant + +## Setup + +```bash +pnpm install # Install dependencies (if needed) +pnpm check # Verify everything passes before starting +``` + +## Development + +```bash +pnpm --filter web dev # Start dev server at localhost:5173 +pnpm test:watch # Run tests in watch mode +``` + +## Implementation Order + +1. **Domain event** — Add `CombatantUpdated` to `events.ts` +2. **Domain function** — Create `edit-combatant.ts` with pure `editCombatant` function +3. **Domain tests** — Create `edit-combatant.test.ts` with acceptance scenarios + invariants +4. **Domain exports** — Re-export from `index.ts` +5. **Application use case** — Create `edit-combatant-use-case.ts` +6. **Application exports** — Re-export from `index.ts` +7. **Hook** — Add `editCombatant` action to `useEncounter` hook +8. **UI** — Add inline name editing to `App.tsx` + +## Verification + +```bash +pnpm check # Must pass — format + lint + typecheck + test +``` + +## Key Files to Reference + +- `packages/domain/src/add-combatant.ts` — Pattern to follow for domain function +- `packages/domain/src/remove-combatant.ts` — Pattern for "not found" error handling +- `packages/application/src/add-combatant-use-case.ts` — Pattern for use case +- `apps/web/src/hooks/use-encounter.ts` — Pattern for hook wiring diff --git a/specs/004-edit-combatant/research.md b/specs/004-edit-combatant/research.md new file mode 100644 index 0000000..d6518b4 --- /dev/null +++ b/specs/004-edit-combatant/research.md @@ -0,0 +1,40 @@ +# Research: Edit Combatant + +**Feature**: 004-edit-combatant +**Date**: 2026-03-03 + +## Research Summary + +No unknowns or NEEDS CLARIFICATION items exist in the spec or technical context. The feature follows well-established patterns already present in the codebase. + +## Decision: Domain Function Pattern + +**Decision**: Follow the identical pattern used by `addCombatant` and `removeCombatant` — pure function returning `EditCombatantSuccess | DomainError`. + +**Rationale**: Consistency with existing code. All three existing domain operations use the same signature shape `(encounter, ...args) => { encounter, events } | DomainError`. No reason to deviate. + +**Alternatives considered**: None — the pattern is well-established and fits perfectly. + +## Decision: Event Shape + +**Decision**: `CombatantUpdated` event includes `combatantId`, `oldName`, and `newName` fields. + +**Rationale**: Including both old and new name enables downstream consumers (logging, undo, UI feedback) without needing to diff state. Follows the pattern of `CombatantRemoved` which includes `name` for context. + +**Alternatives considered**: Including only `newName` — rejected because losing the old name makes undo/logging harder with no storage savings. + +## Decision: Name Validation + +**Decision**: Reuse the same validation logic as `addCombatant` (reject empty and whitespace-only strings, same error code `"invalid-name"`). + +**Rationale**: Consistent user experience. The spec explicitly states this assumption. + +**Alternatives considered**: None — spec is explicit. + +## Decision: UI Mechanism + +**Decision**: Minimal inline edit — clicking a combatant name makes it editable via an input field, confirmed on blur or Enter. + +**Rationale**: Simplest interaction that meets FR-007 without adding modals or prompts. Follows MVP baseline. + +**Alternatives considered**: Modal dialog, browser `prompt()` — both rejected as heavier than needed for MVP. diff --git a/specs/004-edit-combatant/spec.md b/specs/004-edit-combatant/spec.md new file mode 100644 index 0000000..de27685 --- /dev/null +++ b/specs/004-edit-combatant/spec.md @@ -0,0 +1,77 @@ +# Feature Specification: Edit Combatant + +**Feature Branch**: `004-edit-combatant` +**Created**: 2026-03-03 +**Status**: Draft +**Input**: User description: "EditCombatant: allow updating a combatant's name by id in Encounter (emit CombatantUpdated, error if id not found) and wire through application + minimal UI." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Rename a Combatant (Priority: P1) + +A user running an encounter realizes a combatant's name is misspelled or wants to change it. They select the combatant by its identity, provide a new name, and the system updates the combatant in-place while preserving turn order and round state. + +**Why this priority**: Core feature — without the ability to rename, the entire edit-combatant feature has no value. + +**Independent Test**: Can be fully tested by creating an encounter with combatants, editing one combatant's name, and verifying the name is updated while all other encounter state remains unchanged. + +**Acceptance Scenarios**: + +1. **Given** an encounter with combatants [Alice, Bob], **When** the user updates Bob's name to "Robert", **Then** the encounter contains [Alice, Robert] and a `CombatantUpdated` event is emitted with the combatant's id, old name, and new name. +2. **Given** an encounter with combatants [Alice, Bob] where Bob is the active combatant, **When** the user updates Bob's name to "Robert", **Then** Bob remains the active combatant (active index unchanged) and the round number is preserved. + +--- + +### User Story 2 - Error Feedback on Invalid Edit (Priority: P2) + +A user attempts to edit a combatant that no longer exists (e.g., removed in another action) or provides an invalid name. The system returns a clear error without modifying the encounter. + +**Why this priority**: Error handling ensures data integrity and provides a usable experience when things go wrong. + +**Independent Test**: Can be tested by attempting to edit a non-existent combatant id and verifying an error is returned with no state change. + +**Acceptance Scenarios**: + +1. **Given** an encounter with combatants [Alice, Bob], **When** the user attempts to update a combatant with a non-existent id, **Then** the system returns a "combatant not found" error and the encounter is unchanged. +2. **Given** an encounter with combatants [Alice, Bob], **When** the user attempts to update Alice's name to an empty string, **Then** the system returns an "invalid name" error and the encounter is unchanged. +3. **Given** an encounter with combatants [Alice, Bob], **When** the user attempts to update Alice's name to a whitespace-only string, **Then** the system returns an "invalid name" error and the encounter is unchanged. + +--- + +### Edge Cases + +- What happens when the user sets a combatant's name to the same value it already has? The system treats it as a valid update — the encounter state is unchanged but a `CombatantUpdated` event is still emitted. +- What happens when the encounter has no combatants? Editing any id returns a "combatant not found" error. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: System MUST allow updating a combatant's name by providing the combatant's id and a new name. +- **FR-002**: System MUST emit a `CombatantUpdated` event containing the combatant id, old name, and new name upon successful update. +- **FR-003**: System MUST return a "combatant not found" error when the provided id does not match any combatant in the encounter. +- **FR-004**: System MUST return an "invalid name" error when the new name is empty or whitespace-only. +- **FR-005**: System MUST preserve turn order (active index) and round number when a combatant is renamed. +- **FR-006**: System MUST preserve the combatant's position in the combatant list (no reordering). +- **FR-007**: The user interface MUST provide a way to trigger a name edit for each combatant in the encounter. + +### Key Entities + +- **Combatant**: Identified by a unique id; has a mutable name. Editing updates only the name, preserving identity and list position. +- **CombatantUpdated (event)**: Records that a combatant's name changed. Contains combatant id, old name, and new name. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Users can rename any combatant in the encounter in a single action. +- **SC-002**: Renaming a combatant never disrupts turn order, active combatant, or round number. +- **SC-003**: Invalid edit attempts (missing combatant, empty name) produce a clear, actionable error message with no side effects. +- **SC-004**: The combatant's updated name is immediately visible in the encounter UI after editing. + +## Assumptions + +- Name validation follows the same rules as adding a combatant (reject empty and whitespace-only names). +- No uniqueness constraint on combatant names — multiple combatants may share the same name. +- MVP baseline does not include editing other combatant attributes (e.g., initiative score, HP). Only name editing is in scope. +- MVP baseline uses inline editing (click-to-edit input field) as the name editing mechanism. More complex UX (e.g., modal dialogs, undo/redo) is not in the MVP baseline. diff --git a/specs/004-edit-combatant/tasks.md b/specs/004-edit-combatant/tasks.md new file mode 100644 index 0000000..999ee70 --- /dev/null +++ b/specs/004-edit-combatant/tasks.md @@ -0,0 +1,147 @@ +# Tasks: Edit Combatant + +**Input**: Design documents from `/specs/004-edit-combatant/` +**Prerequisites**: plan.md (required), spec.md (required for user stories), research.md, data-model.md, contracts/ + +**Tests**: Tests are included as this project follows test-driven patterns established by prior features. + +**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (e.g., US1, US2) +- Include exact file paths in descriptions + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Add the `CombatantUpdated` event type shared by all user stories + +- [x] T001 Add `CombatantUpdated` event interface and add it to the `DomainEvent` union in `packages/domain/src/events.ts` +- [x] T002 Add `EditCombatantSuccess` interface and `editCombatant` function signature (stub returning `DomainError`) in `packages/domain/src/edit-combatant.ts` +- [x] T003 Re-export `editCombatant` and `EditCombatantSuccess` from `packages/domain/src/index.ts` + +**Checkpoint**: Domain types compile, `editCombatant` exists as a stub + +--- + +## Phase 2: User Story 1 - Rename a Combatant (Priority: P1) 🎯 MVP + +**Goal**: A user can rename an existing combatant by id. The encounter state is updated in-place with a `CombatantUpdated` event emitted. Turn order and round number are preserved. + +**Independent Test**: Create an encounter with combatants, edit one name, verify updated name + unchanged activeIndex/roundNumber + correct event. + +### Tests for User Story 1 + +> **NOTE: Write these tests FIRST, ensure they FAIL before implementation** + +- [x] T004 [US1] Write acceptance scenario tests in `packages/domain/src/__tests__/edit-combatant.test.ts`: (1) rename succeeds with correct event containing combatantId, oldName, newName; (2) activeIndex and roundNumber preserved when renaming the active combatant; (3) combatant list order preserved; (4) renaming to same name still emits event +- [x] T005 [US1] Write invariant tests in `packages/domain/src/__tests__/edit-combatant.test.ts`: (INV-1) determinism — same inputs produce same outputs; (INV-2) exactly one event emitted on success; (INV-3) original encounter is not mutated + +### Implementation for User Story 1 + +- [x] T006 [US1] Implement `editCombatant` pure function in `packages/domain/src/edit-combatant.ts` — find combatant by id, validate name, return updated encounter with mapped combatants list and `CombatantUpdated` event +- [x] T007 [US1] Create `editCombatantUseCase` in `packages/application/src/edit-combatant-use-case.ts` following the pattern in `add-combatant-use-case.ts` (get → call domain → check error → save → return events) +- [x] T008 [US1] Re-export `editCombatantUseCase` from `packages/application/src/index.ts` +- [x] T009 [US1] Add `editCombatant(id: CombatantId, newName: string)` action to `useEncounter` hook in `apps/web/src/hooks/use-encounter.ts` +- [x] T010 [US1] Add inline name editing UI for each combatant in `apps/web/src/App.tsx` — click name to edit via input field, confirm on Enter or blur + +**Checkpoint**: User Story 1 fully functional — renaming works end-to-end, all tests pass + +--- + +## Phase 3: User Story 2 - Error Feedback on Invalid Edit (Priority: P2) + +**Goal**: Invalid edit attempts (non-existent id, empty/whitespace name) return clear errors with no side effects on encounter state. + +**Independent Test**: Attempt to edit a non-existent combatant id and an empty name, verify error returned and encounter unchanged. + +### Tests for User Story 2 + +- [x] T011 [US2] Write error scenario tests in `packages/domain/src/__tests__/edit-combatant.test.ts`: (1) non-existent id returns `"combatant-not-found"` error; (2) empty name returns `"invalid-name"` error; (3) whitespace-only name returns `"invalid-name"` error; (4) empty encounter returns `"combatant-not-found"` for any id + +### Implementation for User Story 2 + +- [x] T012 [US2] Add name validation (empty/whitespace check) to `editCombatant` in `packages/domain/src/edit-combatant.ts` — return `DomainError` with code `"invalid-name"` (should already be partially covered by T006; this task ensures the guard is correct and tested) + +**Checkpoint**: Error paths fully tested, `pnpm check` passes + +--- + +## Phase 4: Polish & Cross-Cutting Concerns + +**Purpose**: Final validation across all stories + +- [x] T013 Run `pnpm check` (format + lint + typecheck + test) and fix any issues +- [x] T014 Verify layer boundaries pass (`packages/domain` has no application/web imports) + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No dependencies — can start immediately +- **User Story 1 (Phase 2)**: Depends on Setup (T001–T003) +- **User Story 2 (Phase 3)**: Depends on Setup (T001–T003); can run in parallel with US1 for tests, but implementation builds on T006 +- **Polish (Phase 4)**: Depends on all user stories being complete + +### User Story Dependencies + +- **User Story 1 (P1)**: Can start after Setup — no dependencies on other stories +- **User Story 2 (P2)**: Error handling is part of the same domain function as US1; tests can be written in parallel, but implementation in T012 refines the function created in T006 + +### Within Each User Story + +- Tests MUST be written and FAIL before implementation +- Domain function before use case +- Use case before hook +- Hook before UI + +### Parallel Opportunities + +- T004 and T005 (US1 tests) target the same file — execute sequentially +- T007 and T008 (use case + export) are sequential but fast +- T011 (US2 tests) can be written in parallel with US1 implementation (T006–T010) +- T013 and T014 (polish) can run in parallel + +--- + +## Parallel Example: User Story 1 + +```bash +# Write both test groups in parallel: +Task T004: "Acceptance scenario tests in packages/domain/src/__tests__/edit-combatant.test.ts" +Task T005: "Invariant tests in packages/domain/src/__tests__/edit-combatant.test.ts" + +# Then implement sequentially (each depends on prior): +Task T006: Domain function → T007: Use case → T008: Export → T009: Hook → T010: UI +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1: Setup (T001–T003) +2. Complete Phase 2: User Story 1 (T004–T010) +3. **STOP and VALIDATE**: `pnpm check` passes, rename works in browser +4. Deploy/demo if ready + +### Full Feature + +1. Setup (T001–T003) → Foundation ready +2. User Story 1 (T004–T010) → Rename works end-to-end (MVP!) +3. User Story 2 (T011–T012) → Error handling complete +4. Polish (T013–T014) → Final validation + +--- + +## Notes + +- [P] tasks = different files, no dependencies +- [Story] label maps task to specific user story for traceability +- T004 and T005 both write to the same test file — execute sequentially +- Commit after each phase or logical group +- Stop at any checkpoint to validate story independently