From aed234de7bf4498b7c381c5af4889d42c179d438 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 3 Mar 2026 23:46:47 +0100 Subject: [PATCH] Implement the 003-remove-combatant feature that adds the possibility to remove a combatant from an encounter --- CLAUDE.md | 7 + apps/web/src/App.tsx | 10 +- apps/web/src/hooks/use-encounter.ts | 24 ++- packages/application/src/index.ts | 1 + .../src/remove-combatant-use-case.ts | 23 +++ .../src/__tests__/remove-combatant.test.ts | 142 ++++++++++++++++++ packages/domain/src/events.ts | 12 +- packages/domain/src/index.ts | 6 +- packages/domain/src/remove-combatant.ts | 65 ++++++++ .../checklists/requirements.md | 34 +++++ specs/003-remove-combatant/data-model.md | 69 +++++++++ specs/003-remove-combatant/plan.md | 71 +++++++++ specs/003-remove-combatant/quickstart.md | 39 +++++ specs/003-remove-combatant/research.md | 48 ++++++ specs/003-remove-combatant/spec.md | 101 +++++++++++++ specs/003-remove-combatant/tasks.md | 117 +++++++++++++++ 16 files changed, 763 insertions(+), 6 deletions(-) create mode 100644 packages/application/src/remove-combatant-use-case.ts create mode 100644 packages/domain/src/__tests__/remove-combatant.test.ts create mode 100644 packages/domain/src/remove-combatant.ts create mode 100644 specs/003-remove-combatant/checklists/requirements.md create mode 100644 specs/003-remove-combatant/data-model.md create mode 100644 specs/003-remove-combatant/plan.md create mode 100644 specs/003-remove-combatant/quickstart.md create mode 100644 specs/003-remove-combatant/research.md create mode 100644 specs/003-remove-combatant/spec.md create mode 100644 specs/003-remove-combatant/tasks.md diff --git a/CLAUDE.md b/CLAUDE.md index 68ddb2d..8a2b4f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -49,3 +49,10 @@ The constitution (`.specify/memory/constitution.md`) governs all feature work: 3. **Clarification-First** — Ask before making non-trivial assumptions. 4. **MVP Baseline** — Say "MVP baseline does not include X", never permanent bans. 5. **Every feature begins with a spec** — Spec → Plan → Tasks → Implementation. + +## Active Technologies +- TypeScript 5.x (strict mode, verbatimModuleSyntax) + React 19, Vite (003-remove-combatant) +- In-memory React state (local-first, single-user MVP) (003-remove-combatant) + +## Recent Changes +- 003-remove-combatant: Added TypeScript 5.x (strict mode, verbatimModuleSyntax) + React 19, Vite diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 6f6f8be..54528e2 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -9,11 +9,14 @@ function formatEvent(e: ReturnType["events"][number]) { return `Round advanced to ${e.newRoundNumber}`; case "CombatantAdded": return `Added combatant: ${e.name}`; + case "CombatantRemoved": + return `Removed combatant: ${e.name}`; } } export function App() { - const { encounter, events, advanceTurn, addCombatant } = useEncounter(); + const { encounter, events, advanceTurn, addCombatant, removeCombatant } = + useEncounter(); const activeCombatant = encounter.combatants[encounter.activeIndex]; const [nameInput, setNameInput] = useState(""); @@ -37,7 +40,10 @@ export function App() { diff --git a/apps/web/src/hooks/use-encounter.ts b/apps/web/src/hooks/use-encounter.ts index 5f9033f..a59d832 100644 --- a/apps/web/src/hooks/use-encounter.ts +++ b/apps/web/src/hooks/use-encounter.ts @@ -2,8 +2,9 @@ import type { EncounterStore } from "@initiative/application"; import { addCombatantUseCase, advanceTurnUseCase, + removeCombatantUseCase, } from "@initiative/application"; -import type { DomainEvent, Encounter } from "@initiative/domain"; +import type { CombatantId, DomainEvent, Encounter } from "@initiative/domain"; import { combatantId, createEncounter, @@ -64,5 +65,24 @@ export function useEncounter() { [makeStore], ); - return { encounter, events, advanceTurn, addCombatant } as const; + const removeCombatant = useCallback( + (id: CombatantId) => { + const result = removeCombatantUseCase(makeStore(), id); + + if (isDomainError(result)) { + return; + } + + setEvents((prev) => [...prev, ...result]); + }, + [makeStore], + ); + + return { + encounter, + events, + advanceTurn, + addCombatant, + removeCombatant, + } as const; } diff --git a/packages/application/src/index.ts b/packages/application/src/index.ts index d58062a..b243a15 100644 --- a/packages/application/src/index.ts +++ b/packages/application/src/index.ts @@ -1,3 +1,4 @@ export { addCombatantUseCase } from "./add-combatant-use-case.js"; export { advanceTurnUseCase } from "./advance-turn-use-case.js"; export type { EncounterStore } from "./ports.js"; +export { removeCombatantUseCase } from "./remove-combatant-use-case.js"; diff --git a/packages/application/src/remove-combatant-use-case.ts b/packages/application/src/remove-combatant-use-case.ts new file mode 100644 index 0000000..718c96f --- /dev/null +++ b/packages/application/src/remove-combatant-use-case.ts @@ -0,0 +1,23 @@ +import { + type CombatantId, + type DomainError, + type DomainEvent, + isDomainError, + removeCombatant, +} from "@initiative/domain"; +import type { EncounterStore } from "./ports.js"; + +export function removeCombatantUseCase( + store: EncounterStore, + id: CombatantId, +): DomainEvent[] | DomainError { + const encounter = store.get(); + const result = removeCombatant(encounter, id); + + if (isDomainError(result)) { + return result; + } + + store.save(result.encounter); + return result.events; +} diff --git a/packages/domain/src/__tests__/remove-combatant.test.ts b/packages/domain/src/__tests__/remove-combatant.test.ts new file mode 100644 index 0000000..9b503f1 --- /dev/null +++ b/packages/domain/src/__tests__/remove-combatant.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it } from "vitest"; +import { removeCombatant } from "../remove-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 A = makeCombatant("A"); +const B = makeCombatant("B"); +const C = makeCombatant("C"); +const D = makeCombatant("D"); + +function enc( + combatants: Combatant[], + activeIndex = 0, + roundNumber = 1, +): Encounter { + return { combatants, activeIndex, roundNumber }; +} + +function successResult(encounter: Encounter, id: string) { + const result = removeCombatant(encounter, combatantId(id)); + if (isDomainError(result)) { + throw new Error(`Expected success, got error: ${result.message}`); + } + return result; +} + +// --- Acceptance Scenarios --- + +describe("removeCombatant", () => { + describe("acceptance scenarios", () => { + it("AS-1: remove combatant after active — activeIndex unchanged", () => { + // [A*, B, C] remove C → [A*, B], activeIndex stays 0 + const e = enc([A, B, C], 0, 2); + const { encounter, events } = successResult(e, "C"); + + expect(encounter.combatants).toEqual([A, B]); + expect(encounter.activeIndex).toBe(0); + expect(encounter.roundNumber).toBe(2); + expect(events).toEqual([ + { + type: "CombatantRemoved", + combatantId: combatantId("C"), + name: "C", + }, + ]); + }); + + it("AS-2: remove combatant before active — activeIndex decrements", () => { + // [A, B, C*] remove A → [B, C*], activeIndex 2→1 + const e = enc([A, B, C], 2, 3); + const { encounter } = successResult(e, "A"); + + expect(encounter.combatants).toEqual([B, C]); + expect(encounter.activeIndex).toBe(1); + expect(encounter.roundNumber).toBe(3); + }); + + it("AS-3: remove active combatant mid-list — next slides in", () => { + // [A, B*, C, D] remove B → [A, C*, D], activeIndex stays 1 + const e = enc([A, B, C, D], 1, 1); + const { encounter } = successResult(e, "B"); + + expect(encounter.combatants).toEqual([A, C, D]); + expect(encounter.activeIndex).toBe(1); + }); + + it("AS-4: remove active combatant at end — wraps to 0", () => { + // [A, B, C*] remove C → [A, B], activeIndex wraps to 0 + const e = enc([A, B, C], 2, 1); + const { encounter } = successResult(e, "C"); + + expect(encounter.combatants).toEqual([A, B]); + expect(encounter.activeIndex).toBe(0); + }); + + it("AS-5: remove only combatant — empty list, activeIndex 0", () => { + const e = enc([A], 0, 5); + const { encounter } = successResult(e, "A"); + + expect(encounter.combatants).toEqual([]); + expect(encounter.activeIndex).toBe(0); + expect(encounter.roundNumber).toBe(5); + }); + + it("AS-6: ID not found — returns DomainError", () => { + const e = enc([A, B], 0, 1); + const result = removeCombatant(e, combatantId("nonexistent")); + + expect(isDomainError(result)).toBe(true); + if (isDomainError(result)) { + expect(result.code).toBe("combatant-not-found"); + } + }); + }); + + describe("invariants", () => { + it("event shape includes combatantId and name", () => { + const e = enc([A, B], 0, 1); + const { events } = successResult(e, "B"); + + expect(events).toHaveLength(1); + expect(events[0]).toEqual({ + type: "CombatantRemoved", + combatantId: combatantId("B"), + name: "B", + }); + }); + + it("roundNumber never changes on removal", () => { + const e = enc([A, B, C], 1, 7); + const { encounter } = successResult(e, "A"); + expect(encounter.roundNumber).toBe(7); + }); + + it("determinism — same input produces same output", () => { + const e = enc([A, B, C], 1, 3); + const result1 = removeCombatant(e, combatantId("B")); + const result2 = removeCombatant(e, combatantId("B")); + expect(result1).toEqual(result2); + }); + + it("every success emits exactly one CombatantRemoved event", () => { + const scenarios: [Encounter, string][] = [ + [enc([A]), "A"], + [enc([A, B], 1), "A"], + [enc([A, B, C], 2, 5), "C"], + ]; + + for (const [e, id] of scenarios) { + const { events } = successResult(e, id); + expect(events).toHaveLength(1); + expect(events[0].type).toBe("CombatantRemoved"); + } + }); + }); +}); diff --git a/packages/domain/src/events.ts b/packages/domain/src/events.ts index 4075a87..df2e817 100644 --- a/packages/domain/src/events.ts +++ b/packages/domain/src/events.ts @@ -19,4 +19,14 @@ export interface CombatantAdded { readonly position: number; } -export type DomainEvent = TurnAdvanced | RoundAdvanced | CombatantAdded; +export interface CombatantRemoved { + readonly type: "CombatantRemoved"; + readonly combatantId: CombatantId; + readonly name: string; +} + +export type DomainEvent = + | TurnAdvanced + | RoundAdvanced + | CombatantAdded + | CombatantRemoved; diff --git a/packages/domain/src/index.ts b/packages/domain/src/index.ts index 94d9f78..05e60f1 100644 --- a/packages/domain/src/index.ts +++ b/packages/domain/src/index.ts @@ -1,12 +1,16 @@ export { type AddCombatantSuccess, addCombatant } from "./add-combatant.js"; export { advanceTurn } from "./advance-turn.js"; - export type { CombatantAdded, + CombatantRemoved, DomainEvent, RoundAdvanced, TurnAdvanced, } from "./events.js"; +export { + type RemoveCombatantSuccess, + removeCombatant, +} from "./remove-combatant.js"; export { type Combatant, type CombatantId, diff --git a/packages/domain/src/remove-combatant.ts b/packages/domain/src/remove-combatant.ts new file mode 100644 index 0000000..18534a3 --- /dev/null +++ b/packages/domain/src/remove-combatant.ts @@ -0,0 +1,65 @@ +import type { DomainEvent } from "./events.js"; +import type { CombatantId, DomainError, Encounter } from "./types.js"; + +export interface RemoveCombatantSuccess { + readonly encounter: Encounter; + readonly events: DomainEvent[]; +} + +/** + * Pure function that removes a combatant from an encounter by ID. + * + * Adjusts activeIndex to preserve turn integrity: + * - Removed after active → unchanged + * - Removed before active → decrement + * - Removed is active, mid-list → same index (next slides in) + * - Removed is active, at end → wrap to 0 + * - Only combatant removed → 0 + * + * roundNumber is never changed. + */ +export function removeCombatant( + encounter: Encounter, + id: CombatantId, +): RemoveCombatantSuccess | DomainError { + const removedIdx = encounter.combatants.findIndex((c) => c.id === id); + + if (removedIdx === -1) { + return { + kind: "domain-error", + code: "combatant-not-found", + message: `No combatant found with ID "${id}"`, + }; + } + + const removed = encounter.combatants[removedIdx]; + const newCombatants = encounter.combatants.filter((_, i) => i !== removedIdx); + + let newActiveIndex: number; + if (newCombatants.length === 0) { + newActiveIndex = 0; + } else if (removedIdx < encounter.activeIndex) { + newActiveIndex = encounter.activeIndex - 1; + } else if (removedIdx > encounter.activeIndex) { + newActiveIndex = encounter.activeIndex; + } else { + // removedIdx === activeIndex + newActiveIndex = + removedIdx >= newCombatants.length ? 0 : encounter.activeIndex; + } + + return { + encounter: { + combatants: newCombatants, + activeIndex: newActiveIndex, + roundNumber: encounter.roundNumber, + }, + events: [ + { + type: "CombatantRemoved", + combatantId: removed.id, + name: removed.name, + }, + ], + }; +} diff --git a/specs/003-remove-combatant/checklists/requirements.md b/specs/003-remove-combatant/checklists/requirements.md new file mode 100644 index 0000000..48d0414 --- /dev/null +++ b/specs/003-remove-combatant/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Remove 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/003-remove-combatant/data-model.md b/specs/003-remove-combatant/data-model.md new file mode 100644 index 0000000..92d4442 --- /dev/null +++ b/specs/003-remove-combatant/data-model.md @@ -0,0 +1,69 @@ +# Data Model: Remove Combatant + +**Feature**: 003-remove-combatant +**Date**: 2026-03-03 + +## Existing Entities (no changes) + +### Encounter + +| Field | Type | Description | +|-------|------|-------------| +| combatants | readonly Combatant[] | Ordered list of participants | +| activeIndex | number | Index of the combatant whose turn it is | +| roundNumber | number | Current round (≥ 1, never changes on removal) | + +### Combatant + +| Field | Type | Description | +|-------|------|-------------| +| id | CombatantId (branded string) | Unique identifier | +| name | string | Display name | + +## New Event Type + +### CombatantRemoved + +| Field | Type | Description | +|-------|------|-------------| +| type | "CombatantRemoved" (literal) | Event discriminant | +| combatantId | CombatantId | ID of the removed combatant | +| name | string | Name of the removed combatant | + +Added to the `DomainEvent` discriminated union alongside `TurnAdvanced`, `RoundAdvanced`, and `CombatantAdded`. + +## New Domain Function + +### removeCombatant + +| Parameter | Type | Description | +|-----------|------|-------------| +| encounter | Encounter | Current encounter state | +| id | CombatantId | ID of combatant to remove | + +**Returns**: `RemoveCombatantSuccess | DomainError` + +### RemoveCombatantSuccess + +| Field | Type | Description | +|-------|------|-------------| +| encounter | Encounter | Updated encounter after removal | +| events | DomainEvent[] | Exactly one CombatantRemoved event | + +### DomainError (existing, reused) + +Returned with code `"combatant-not-found"` when ID does not match any combatant. + +## State Transition Rules + +### activeIndex Adjustment + +Given removal of combatant at index `removedIdx` with current `activeIndex`: + +| Condition | New activeIndex | +|-----------|----------------| +| removedIdx > activeIndex | activeIndex (unchanged) | +| removedIdx < activeIndex | activeIndex - 1 | +| removedIdx === activeIndex, not last in list | activeIndex (next slides in) | +| removedIdx === activeIndex, last in list | 0 (wrap) | +| Only combatant removed (list becomes empty) | 0 | diff --git a/specs/003-remove-combatant/plan.md b/specs/003-remove-combatant/plan.md new file mode 100644 index 0000000..c25fd15 --- /dev/null +++ b/specs/003-remove-combatant/plan.md @@ -0,0 +1,71 @@ +# Implementation Plan: Remove Combatant + +**Branch**: `003-remove-combatant` | **Date**: 2026-03-03 | **Spec**: [spec.md](./spec.md) +**Input**: Feature specification from `/specs/003-remove-combatant/spec.md` + +## Summary + +Add a `removeCombatant` pure domain function that removes a combatant by ID from an Encounter, correctly adjusts `activeIndex` to preserve turn integrity, keeps `roundNumber` unchanged, and emits a `CombatantRemoved` event. Wire through an application-layer use case and expose via a minimal UI remove action per combatant. + +## 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**: Web (localhost:5173 dev, production build via Vite) +**Project Type**: Web application (monorepo: packages/domain, packages/application, apps/web) +**Performance Goals**: N/A (local-first, small data sets) +**Constraints**: Domain must be pure (no I/O); layer boundaries enforced by automated script +**Scale/Scope**: Single-user, single encounter at a time + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Evidence | +|-----------|--------|----------| +| I. Deterministic Domain Core | PASS | `removeCombatant` is a pure function: same input → same output, no I/O | +| II. Layered Architecture | PASS | Domain function → use case → React hook/UI. No layer violations. | +| III. Agent Boundary | N/A | No agent layer involved in this feature | +| IV. Clarification-First | PASS | Spec fully specifies all activeIndex adjustment rules; no ambiguity | +| V. Escalation Gates | PASS | All functionality is within spec scope | +| VI. MVP Baseline Language | PASS | No permanent bans introduced | +| VII. No Gameplay Rules | PASS | Removal is encounter management, not gameplay mechanics | + +**Gate result**: PASS — no violations. + +## Project Structure + +### Documentation (this feature) + +```text +specs/003-remove-combatant/ +├── plan.md +├── research.md +├── data-model.md +├── quickstart.md +└── tasks.md +``` + +### Source Code (repository root) + +```text +packages/domain/src/ +├── remove-combatant.ts # Pure domain function +├── events.ts # Add CombatantRemoved to DomainEvent union +├── types.ts # Existing types (no changes expected) +├── index.ts # Re-export removeCombatant +└── __tests__/ + └── remove-combatant.test.ts # Acceptance scenarios from spec + +packages/application/src/ +├── remove-combatant-use-case.ts # Orchestrates store.get → domain → store.save +└── index.ts # Re-export use case + +apps/web/src/ +├── hooks/use-encounter.ts # Add removeCombatant callback +└── App.tsx # Add remove button per combatant + event display +``` + +**Structure Decision**: Follows the existing monorepo layered architecture (packages/domain → packages/application → apps/web) exactly mirroring the addCombatant feature's file layout. diff --git a/specs/003-remove-combatant/quickstart.md b/specs/003-remove-combatant/quickstart.md new file mode 100644 index 0000000..cc6f362 --- /dev/null +++ b/specs/003-remove-combatant/quickstart.md @@ -0,0 +1,39 @@ +# Quickstart: Remove Combatant + +**Feature**: 003-remove-combatant + +## Prerequisites + +- Node.js 18+, pnpm +- Repository cloned, `pnpm install` run + +## Development + +```bash +git checkout 003-remove-combatant +pnpm test:watch # Run tests in watch mode during development +pnpm --filter web dev # Dev server at localhost:5173 +``` + +## Verification + +```bash +pnpm check # Must pass before commit (format + lint + typecheck + test) +``` + +## Implementation Order + +1. **Domain**: Add `CombatantRemoved` event type → implement `removeCombatant` pure function → tests +2. **Application**: Add `removeCombatantUseCase` → re-export +3. **Web**: Add `removeCombatant` to `useEncounter` hook → add remove button in `App.tsx` + +## Key Files + +| Layer | File | Purpose | +|-------|------|---------| +| Domain | `packages/domain/src/remove-combatant.ts` | Pure removal function | +| Domain | `packages/domain/src/events.ts` | CombatantRemoved event type | +| Domain | `packages/domain/src/__tests__/remove-combatant.test.ts` | Acceptance tests | +| Application | `packages/application/src/remove-combatant-use-case.ts` | Use case orchestration | +| Web | `apps/web/src/hooks/use-encounter.ts` | Hook integration | +| Web | `apps/web/src/App.tsx` | UI remove button | diff --git a/specs/003-remove-combatant/research.md b/specs/003-remove-combatant/research.md new file mode 100644 index 0000000..e9acb05 --- /dev/null +++ b/specs/003-remove-combatant/research.md @@ -0,0 +1,48 @@ +# Research: Remove Combatant + +**Feature**: 003-remove-combatant +**Date**: 2026-03-03 + +## R1: activeIndex Adjustment Strategy on Removal + +**Decision**: Use positional comparison between removed index and activeIndex to determine adjustment. + +**Rationale**: The spec defines five distinct cases based on the relationship between the removed combatant's index and the current activeIndex. These map cleanly to a single conditional: + +1. **Removed index > activeIndex** → no change (combatant was after active) +2. **Removed index < activeIndex** → decrement activeIndex by 1 (shift left) +3. **Removed index === activeIndex and not last** → keep same index (next combatant slides into position) +4. **Removed index === activeIndex and last** → wrap to 0 +5. **Last remaining combatant removed** → activeIndex = 0 + +This mirrors the inverse of addCombatant's "always append, never adjust" approach — removal requires adjustment because positions shift. + +**Alternatives considered**: +- Storing active combatant by ID instead of index: Would simplify removal but requires changing the Encounter type (out of scope, breaks existing advanceTurn). +- Emitting a TurnAdvanced event on active removal: Rejected — spec explicitly says roundNumber is unchanged, and the next-in-line simply inherits. + +## R2: CombatantRemoved Event Shape + +**Decision**: Follow the existing event pattern with `type` discriminant. Include `combatantId` and `name` fields. + +**Rationale**: Consistent with `CombatantAdded` which carries `combatantId`, `name`, and `position`. For removal, `position` is less meaningful (the combatant is gone), so we include only ID and name. + +**Alternatives considered**: +- Including the removed index: Rejected — the index is ephemeral and not useful after the fact. +- Including the full Combatant object: Over-engineered for current needs; ID + name suffices. + +## R3: Use Case Pattern + +**Decision**: Mirror `addCombatantUseCase` exactly — `store.get()` → domain function → `store.save()` → return events. + +**Rationale**: No new patterns needed. The existing use case pattern handles the get-transform-save cycle cleanly. + +## R4: UI Pattern for Remove Action + +**Decision**: Add a remove button next to each combatant in the list. The button calls `removeCombatant(id)` from the hook. + +**Rationale**: Minimal UI per spec. No confirmation dialog needed for MVP (spec doesn't require it). Mirrors the simplicity of the existing add form. + +**Alternatives considered**: +- Confirmation modal before removal: MVP baseline does not include this; can be added later. +- Swipe-to-remove gesture: Not applicable for web MVP. diff --git a/specs/003-remove-combatant/spec.md b/specs/003-remove-combatant/spec.md new file mode 100644 index 0000000..d5284bd --- /dev/null +++ b/specs/003-remove-combatant/spec.md @@ -0,0 +1,101 @@ +# Feature Specification: Remove Combatant + +**Feature Branch**: `003-remove-combatant` +**Created**: 2026-03-03 +**Status**: Draft +**Input**: User description: "RemoveCombatant: allow removing a combatant by id from Encounter (adjust activeIndex correctly, keep roundNumber, emit CombatantRemoved, error if id not found) and wire through application + minimal UI." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Remove a Combatant from an Active Encounter (Priority: P1) + +A game master is running a combat encounter and a combatant is defeated or leaves. The GM removes that combatant by clicking a remove action. The combatant disappears from the initiative order and the turn continues correctly without disruption. + +**Why this priority**: Core functionality — removing combatants is the primary purpose of this feature and must work correctly to maintain encounter integrity. + +**Independent Test**: Can be fully tested by adding combatants to an encounter, removing one, and verifying the combatant list, activeIndex, and roundNumber are correct. + +**Acceptance Scenarios**: + +1. **Given** an encounter with combatants [A, B, C] and activeIndex 1 (B's turn), **When** the GM removes combatant C (index 2, after active), **Then** the encounter has [A, B], activeIndex remains 1, roundNumber unchanged, and a CombatantRemoved event is emitted. +2. **Given** an encounter with combatants [A, B, C] and activeIndex 2 (C's turn), **When** the GM removes combatant A (index 0, before active), **Then** the encounter has [B, C], activeIndex becomes 1 (still C's turn), roundNumber unchanged. +3. **Given** an encounter with combatants [A, B, C] and activeIndex 1 (B's turn), **When** the GM removes combatant B (the active combatant), **Then** the encounter has [A, C], activeIndex becomes 1 (C is now active — the next combatant takes over), roundNumber unchanged. +4. **Given** an encounter with combatants [A, B, C] and activeIndex 2 (C's turn, last position), **When** the GM removes combatant C (active and last), **Then** the encounter has [A, B], activeIndex wraps to 0 (A is now active), roundNumber unchanged. +5. **Given** an encounter with combatants [A] and activeIndex 0, **When** the GM removes combatant A, **Then** the encounter has [], activeIndex is 0, roundNumber unchanged. +6. **Given** an encounter with combatants [A, B, C], **When** the GM attempts to remove a combatant with an ID that does not exist, **Then** a domain error is returned with a descriptive error code, and the encounter is unchanged. + +--- + +### User Story 2 - Remove Combatant via UI (Priority: P2) + +A game master sees a list of combatants in the encounter UI. Each combatant has a remove action. Clicking it removes the combatant and the UI updates to reflect the new initiative order. + +**Why this priority**: Provides the user-facing interaction for the core domain functionality. Without UI, the feature is not accessible. + +**Independent Test**: Can be tested by rendering the encounter UI, clicking the remove action on a combatant, and verifying the combatant disappears from the list. + +**Acceptance Scenarios**: + +1. **Given** an encounter with combatants displayed in the UI, **When** the GM clicks the remove action on a combatant, **Then** that combatant is removed from the displayed list. +2. **Given** an encounter displayed in the UI, **When** a removal results in a domain error (ID not found), **Then** the removal is silently ignored and the encounter state remains unchanged. + +--- + +### Edge Cases + +- What happens when removing the only combatant? The encounter becomes empty with activeIndex 0. +- What happens when removing the active combatant who is last in the list? activeIndex wraps to 0. +- What happens when removing from an empty encounter? This is covered by the "ID not found" error since no combatant IDs exist. +- What happens if the same ID is passed twice in sequence? The first call succeeds; the second returns an error (ID not found). + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: System MUST remove a combatant identified by CombatantId from the encounter's combatant list. +- **FR-002**: System MUST return a domain error with code `"combatant-not-found"` when the given CombatantId does not match any combatant in the encounter. +- **FR-003**: System MUST preserve the roundNumber unchanged after removal. +- **FR-004**: System MUST adjust activeIndex so that the same combatant remains active after removal when the removed combatant is before the active one (activeIndex decrements by 1). +- **FR-005**: System MUST keep activeIndex unchanged when the removed combatant is after the active one. +- **FR-006**: System MUST advance activeIndex to the next combatant (same index position) when the active combatant is removed, allowing the next-in-line to take over. +- **FR-007**: System MUST wrap activeIndex to 0 when the active combatant is removed and it was the last in the list. +- **FR-008**: System MUST set activeIndex to 0 when the last remaining combatant is removed (empty encounter). +- **FR-009**: System MUST emit exactly one CombatantRemoved event on successful removal, containing the removed combatant's ID and name. +- **FR-010**: System MUST expose the remove-combatant operation through the application layer via a use case / port interface. +- **FR-011**: System MUST provide a UI control for each combatant that triggers removal. + +### Key Entities + +- **Encounter**: The combat encounter containing an ordered list of combatants, an activeIndex, and a roundNumber. +- **Combatant**: A participant in the encounter identified by a unique CombatantId and a name. +- **CombatantRemoved** (event): A domain event recording the removal, carrying the removed combatant's ID and name. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Removing a combatant from any position in the initiative order preserves correct turn tracking (the intended combatant remains or becomes active). +- **SC-002**: All six acceptance scenarios pass as automated tests. +- **SC-003**: The round number never changes as a result of removal. +- **SC-004**: The UI reflects combatant removal immediately after the action, with no stale state displayed. + +## Assumptions + +- ID generation and lookup is the caller's responsibility, consistent with the addCombatant pattern. +- Removal does not trigger a round advance — roundNumber is always preserved. +- The domain function is pure: deterministic given identical inputs, no I/O. +- The CombatantRemoved event follows the same plain-data-object pattern as existing domain events. +- When the active combatant is removed, the next combatant in order inherits the turn (no automatic turn advance or round increment occurs). +- Error feedback for invalid removal is a silent no-op for MVP. MVP baseline does not include user-visible error messages for removal failures. + +## Constitution Check + +| Principle | Status | Evidence | +|-----------|--------|----------| +| I. Deterministic Domain Core | PASS | removeCombatant is a pure state transition with no I/O | +| II. Layered Architecture | PASS | Domain function → use case → UI adapter | +| III. Agent Boundary | N/A | No agent layer involved | +| IV. Clarification-First | PASS | All activeIndex rules fully specified; no ambiguity | +| V. Escalation Gates | PASS | All requirements within original spec scope | +| VI. MVP Baseline Language | PASS | No permanent bans; confirmation dialog excluded via MVP baseline language | +| VII. No Gameplay Rules | PASS | Encounter management only, no game mechanics | diff --git a/specs/003-remove-combatant/tasks.md b/specs/003-remove-combatant/tasks.md new file mode 100644 index 0000000..4e1b97b --- /dev/null +++ b/specs/003-remove-combatant/tasks.md @@ -0,0 +1,117 @@ +# Tasks: Remove Combatant + +**Input**: Design documents from `/specs/003-remove-combatant/` +**Prerequisites**: plan.md, spec.md, research.md, data-model.md + +**Tests**: Included — spec requires all six acceptance scenarios as automated tests (SC-002). + +**Organization**: Tasks grouped by user story for independent implementation and testing. + +## 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) +- Exact file paths included in descriptions + +## Phase 1: Foundational (Event Type) + +**Purpose**: Add the CombatantRemoved event type that all subsequent tasks depend on. + +- [x] T001 Add `CombatantRemoved` interface and extend `DomainEvent` union in `packages/domain/src/events.ts` +- [x] T002 Export `CombatantRemoved` type from `packages/domain/src/index.ts` + +**Checkpoint**: CombatantRemoved event type available for domain function and UI event display. + +--- + +## Phase 2: User Story 1 - Remove Combatant Domain Logic (Priority: P1) MVP + +**Goal**: Pure `removeCombatant` domain function that removes a combatant by ID, adjusts activeIndex correctly, preserves roundNumber, and emits CombatantRemoved. + +**Independent Test**: Call `removeCombatant` with various encounter states and verify combatant list, activeIndex, roundNumber, events, and error cases. + +### Tests for User Story 1 + +> **NOTE: Write these tests FIRST, ensure they FAIL before implementation** + +- [x] T003 [US1] Write acceptance tests for `removeCombatant` in `packages/domain/src/__tests__/remove-combatant.test.ts` covering all 6 spec scenarios: remove after active (AS-1), remove before active (AS-2), remove active combatant mid-list (AS-3), remove active combatant at end/wrap (AS-4), remove only combatant (AS-5), ID not found error (AS-6). Also test: event shape (CombatantRemoved with id+name), roundNumber invariance, and determinism. + +### Implementation for User Story 1 + +- [x] T004 [US1] Implement `removeCombatant` pure function and `RemoveCombatantSuccess` type in `packages/domain/src/remove-combatant.ts` — find combatant by ID, compute new activeIndex per data-model rules, filter combatant list, emit CombatantRemoved event, return DomainError for not-found +- [x] T005 [US1] Export `removeCombatant` and `RemoveCombatantSuccess` from `packages/domain/src/index.ts` + +**Checkpoint**: All 6 acceptance tests pass. Domain function is complete and independently testable. + +--- + +## Phase 3: User Story 2 - Application + UI Wiring (Priority: P2) + +**Goal**: Wire removeCombatant through application use case and expose via minimal UI with a remove button per combatant. + +**Independent Test**: Render encounter UI, click remove on a combatant, verify it disappears from the list and event log updates. + +### Implementation for User Story 2 + +- [x] T006 [P] [US2] Create `removeCombatantUseCase` in `packages/application/src/remove-combatant-use-case.ts` — follows existing pattern: `store.get()` → `removeCombatant()` → `store.save()` → return events or DomainError +- [x] T007 [US2] Export `removeCombatantUseCase` from `packages/application/src/index.ts` +- [x] T008 [US2] Add `removeCombatant(id: CombatantId)` callback to `useEncounter` hook in `apps/web/src/hooks/use-encounter.ts` — call use case, append events to log on success +- [x] T009 [US2] Add remove button per combatant and `CombatantRemoved` event display case in `apps/web/src/App.tsx` + +**Checkpoint**: Full vertical slice works — GM can remove combatants from UI, initiative order updates correctly, event log shows removal. + +--- + +## Phase 4: Polish & Cross-Cutting Concerns + +- [x] T010 Run `pnpm check` (format + lint + typecheck + test) and fix any issues + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Phase 1 (Foundational)**: No dependencies — start immediately +- **Phase 2 (US1 Domain)**: Depends on Phase 1 (needs CombatantRemoved type) +- **Phase 3 (US2 App+UI)**: Depends on Phase 2 (needs domain function) +- **Phase 4 (Polish)**: Depends on Phase 3 + +### Within Each Phase + +- T001 → T002 (export after defining) +- T003 (tests first) → T004 (implement) → T005 (export) +- T006 → T007 (export after creating use case file) +- T008 depends on T006+T007 (needs use case) +- T009 depends on T008 (needs hook callback) + +### Parallel Opportunities + +- Within T003, individual test cases are independent + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1: Event type (T001–T002) +2. Complete Phase 2: Domain tests + function (T003–T005) +3. **STOP and VALIDATE**: All 6 acceptance tests pass +4. Domain is complete and usable without UI + +### Full Feature + +1. Phase 1 → Phase 2 → Phase 3 → Phase 4 +2. Each phase adds a testable increment +3. Commit after each phase checkpoint + +--- + +## Notes + +- [P] tasks = different files, no dependencies +- [Story] label maps task to specific user story +- Tests written first (TDD) per spec requirement SC-002 +- Commit after each phase checkpoint +- Total: 10 tasks across 4 phases