From 14c5d4881dc954120167da0b2c0acb144ad6d962 Mon Sep 17 00:00:00 2001 From: Kamil Krzywda Date: Wed, 14 Oct 2026 13:42:46 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20nation-op=20split/merge=20review=20?= =?UTF-8?q?=E2=80=94=20dialog=20(part=20list=20+=20successor=20radio,=20me?= =?UTF-8?q?rge=20name),=20docs=20(P4b,=20#16)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 68 ++++++---- src/App.tsx | 11 +- src/components/ActionPanel.tsx | 8 +- src/components/NationOpDialog.tsx | 201 ++++++++++++++++++++++++++---- src/lib/api.ts | 11 +- 5 files changed, 239 insertions(+), 60 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2e21123..372b60a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,13 +68,13 @@ history/ ├── shared/ # NO runtime deps — the single source of truth │ ├── types.ts # domain types (Nation, GameState, RoundItem, Preset, …) │ └── protocol.ts # LLM TurnResult schema (incl. `unowned`, v0.22 #16 P3, and -│ # `nationOps`, v0.23 #16 P4a) + AddAction/AddDiplomacy/RemoveQueueItem/ +│ # `nationOps`, v0.23 #16 P4a; v0.24 #16 P4b) + AddAction/AddDiplomacy/RemoveQueueItem/ │ # Jump + NationOp confirm/reject request types + pure helpers ├── server/ │ ├── index.ts # Express: /api/* (health|action|diplomacy|queue/remove|jump|advisor| │ │ # nation-op/confirm|nation-op/reject|games|map|presets), /sse/:id; reads │ │ # PORT/LLM_*/DATA_DIR -│ ├── engine.ts # pure: prompts / parse+validate / applyTurnResult (sole-ownership + no-man's-land releases, v0.22 #16 P3) / runTurn (tool path + plain fallback) / getNeighbours tool + executor / playerFrontier / queue ops / advisor / nation-op ops (resolveNationRef / applyNationOp / confirmNationOp / rejectNationOp, v0.23 #16 P4a) +│ ├── engine.ts # pure: prompts / parse+validate / applyTurnResult (sole-ownership + no-man's-land releases, v0.22 #16 P3) / runTurn (tool path + plain fallback) / getNeighbours tool + executor / playerFrontier / queue ops / advisor / nation-op ops (resolveNationRef / freshNationId / NEW_NATION_STATS / applyNationOp rename+split+merge / confirmNationOp / rejectNationOp, v0.23 #16 P4a; v0.24 #16 P4b) │ ├── llm.ts # LlmClient: chat() + chatWithTools() (bounded function-calling loop), injectable transport, retry+backoff, logging + env config │ ├── regions.ts # turf-free region-key data access: load manifest/geo/adjacency (public/ then dist/), country→keys expansion, reference table, neighboursOf/hasAdjacency │ ├── stub.ts # deterministic LLM transport (tests/e2e): one frontier conquest per turn + ADVISOR_MARKER @@ -96,7 +96,7 @@ history/ ├── WorldMap.tsx # MapLibre basemap + choropleth + click→dossier + hover tooltip + player highlight ├── NationPanel.tsx # a nation's stats/territory/diplomacy dossier ├── ActionPanel.tsx # tabs: Action / Diplomacy / Advisor + pending-queue list (per-item trash button + confirm dialog) + "Jump forward" + "Nation proposals" trigger (v0.23 #16 P4a) - ├── NationOpDialog.tsx # review LLM-proposed nation ops (rename/split/merge) — confirm (editable rename) or reject (v0.23 #16 P4a) + ├── NationOpDialog.tsx # review LLM-proposed nation ops (rename/split/merge) — confirm (editable rename/merge name, split successor radio) or reject (v0.24 #16 P4b) ├── EventFeed.tsx # scrolling event log (newest first) ├── FloatingWindow.tsx # the HUD window primitive — free-floating (draggable, position persisted), or `pinned` (fixed top-center bar), or `dock`ed (full-height left/right column); renders dock controls when `onDockChange` is set ├── StatBar.tsx @@ -189,24 +189,44 @@ history/ — the distinction is decided by the client from `GameState.unowned` (via `regionFillStyle` / `unownedTooltipLabel`), never by the `RegionMap`, which only knows "owned" vs "absent". A fresh game always starts with `unowned: []` (legacy SQLite rows load with it defaulted). -- **Nation ops — “LLM proposes, player confirms” (v0.23, #16 P4a).** `TurnResult.nationOps` (required - in the schema) carries the LLM’s proposed **`NationOp`s** — a discriminated union of `rename` / - `split` / `merge`. `rename` is fully applied; `split` and `merge` are **declared shapes only** - (accepted and stored as pending, but a no-op until P4b implements them). Each op names its target - via `nationRef` — the LLM may use a nation’s **id or name**; `resolveNationRef` resolves id-first, - then a case-insensitive unique-name match, and returns `null` for an unknown or ambiguous ref (such - ops are dropped defensively in `validateNationOps`, never thrown). A turn’s ops are appended to - `GameState.pendingNationOps`, which is **never cleared on a turn commit** — pending proposals persist - across turns until resolved (like the action queue). The player then decides per op via - `POST /api/games/:id/nation-op/confirm` (`{index, edits?{name?}}`) or `.../nation-op/reject` - (`{index}`): `confirmNationOp` applies the op to a clone (a rename sets `Nation.name` to the edited - value or, failing that, the proposed `to`; id / territory / relations / `playerNationId` stay valid, - so a renamed player is re-named in place) and `rejectNationOp` just drops it — both splice by index - and bump `version` once (an invalid index → `null` → HTTP 400). The response is `{ game }` and the - server broadcasts a version-bumped SSE `state`, so every tab’s version-guarded `applyState` stays in - sync. The client lists the pending ops in a “Nation proposals” dialog (in the “Your move” panel) with - an editable rename name plus Confirm / Reject. The stub never proposes a nation op (`nationOps: []`), - so this path is covered by unit tests only (`server/engine.test.ts`). +- **Nation ops — “LLM proposes, player confirms” (v0.23, #16 P4a; v0.24, #16 P4b).** + `TurnResult.nationOps` (required in the schema) carries the LLM’s proposed **`NationOp`s** — a + discriminated union of `rename` / `split` / `merge`, all three of which are now applied on confirm + (P4b implemented split/merge). Each op names its target via `nationRef` — the LLM may use a + nation’s **id or name**; `resolveNationRef` resolves id-first, then a case-insensitive + unique-name match, and returns `null` for an unknown or ambiguous ref (such ops are dropped + defensively in `validateNationOps`, never thrown). A turn’s ops are appended to + `GameState.pendingNationOps`, which is **never cleared on a turn commit** — pending proposals + persist across turns until resolved (like the action queue). The player then decides per op via + `POST /api/games/:id/nation-op/confirm` (`{index, edits?{name?, playerPart?}}`) or + `.../nation-op/reject` (`{index}`): `confirmNationOp` applies the op to a clone — **rename** sets + `Nation.name` to the edited value or, failing that, the proposed `to` (id / territory / relations / + `playerNationId` stay valid, so a renamed player is re-named in place); **split** (the parent + must resolve and have ≥2 named parts, else a no-op) partitions the parent’s region-key territory + into new nations in parts order — each new nation gets a fresh id (`freshNationId`: first free of + `n1`, `n2`, …) and the baseline `NEW_NATION_STATS` (not a copy of the parent’s), plus a copy of + the parent’s relations; a key named by several parts goes to the first, and the parent’s + unclaimed keys go to part 0 (in territory order), so sole ownership holds and nothing is + orphaned; if the parent was the player’s nation, `playerNationId` moves to the part chosen by + `edits.playerPart` (clamped into range, default 0), which also receives `edits.name`; **merge** + (every ref must resolve, ≥2 DISTINCT inputs, and the survivor must be one of them, else a + no-op) folds the inputs into the survivor, which keeps its id and its own stats, gains the + union of the inputs’ territories (the survivor’s keys first) and a union of their relations + (the survivor’s own view of a third party wins, else the first input’s), is renamed + `edits.name` → `into` → its own name, while the other inputs are removed and their ids dropped + from every remaining nation’s relations maps; if the player led a removed input, + `playerNationId` moves to the survivor. An op that is invalid at confirm time (an ambiguous ref, + …) is a no-op on the world but is still removed — and `rejectNationOp` just drops it; both + splice by index and bump `version` once (an invalid index → `null` → HTTP 400). The response is + `{ game }` and the server broadcasts a version-bumped SSE `state`, so every tab’s + version-guarded `applyState` stays in sync. The client lists the pending ops in a “Nation + proposals” dialog (in the “Your move” panel): a rename and a merge show an editable name field + (the merge also highlights the survivor among the input chips); a split lists each part (name + + proposed region count) with a successor radio that appears only when the parent is the player’s + own nation (preselected part 0 — the choice is always valid). The system prompt documents all + three op shapes as a proposal queue (at most 1–2 per turn, each narrated in events). The stub + never proposes a nation op (`nationOps: []`), so this path is covered by unit tests only + (`server/engine.test.ts`). - **Map click → dossier target (owner resolved from the RegionMap, by region key).** Clicking a sub-region does NOT open a popup — it switches the dossier to that sub-region's owning nation (`focusNation(ownerId, {name})` in `App.tsx`, which also records which region was @@ -233,9 +253,9 @@ history/ no man's land (it always returns `unowned: []` and ignores the payload's `unowned` field) — its one-conquest behavior is what the e2e assertions rely on, so the release/re-claim path is covered by unit tests only (`server/engine.test.ts`, `server/map.test.ts`). Since v0.23 (#16 P4a) - the stub also returns `nationOps: []` — it **never proposes** a nation op (rename/split/merge), so - the confirm/reject path is covered by unit tests only, mirroring the P3 approach; all e2e - assertions are unaffected. The stub answers plain `content` (never tool calls), so it naturally + the stub also returns `nationOps: []` — it **never proposes** a nation op (rename/split/merge; + still true since P4b implemented split/merge), so the confirm/reject path is covered by unit + tests only, mirroring the P3 approach; all e2e assertions are unaffected. The stub answers plain `content` (never tool calls), so it naturally bypasses the tool loop in a single round. Unit + e2e assertions rely on these exact values, so keep the stub deterministic. - **HUD layout.** The game screen is a full-viewport map (z-0) with `FloatingWindow` overlays. The diff --git a/src/App.tsx b/src/App.tsx index cb9bd0a..9f9b223 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -405,11 +405,12 @@ export default function App() { [game, applyState], ); - // Apply a pending nation op the model proposed (confirm, with an optional - // rename-name edit). Fast, non-LLM — gated by `opBusy`, not `busy`. The - // version-bumped state comes back in the response and the SSE `state` event. + // Apply a pending nation op the model proposed (confirm; `edits.name` + // overrides a rename/merge name, `edits.playerPart` picks the player's + // successor part in a split). Fast, non-LLM — gated by `opBusy`, not `busy`. + // The version-bumped state comes back in the response and the SSE `state` event. const runConfirmNationOp = useCallback( - async (index: number, edits?: { name?: string }) => { + async (index: number, edits?: { name?: string; playerPart?: number }) => { if (!game) return; setOpBusy(true); try { @@ -603,7 +604,7 @@ interface GameScreenProps { onRemoveQueueItem: (index: number) => void; onAdvisor: (p?: string) => void; onJump: () => void; - onConfirmNationOp: (index: number, edits?: { name?: string }) => void; + onConfirmNationOp: (index: number, edits?: { name?: string; playerPart?: number }) => void; onRejectNationOp: (index: number) => void; onHome: () => void; } diff --git a/src/components/ActionPanel.tsx b/src/components/ActionPanel.tsx index b614631..a53eaba 100644 --- a/src/components/ActionPanel.tsx +++ b/src/components/ActionPanel.tsx @@ -44,8 +44,12 @@ interface Props { onAdvisor: (prompt?: string) => void; /** Commit the whole pending round in a single LLM turn. */ onJump: () => void; - /** Apply a pending nation op; `edits.name` optionally overrides a rename. */ - onConfirmNationOp: (index: number, edits?: { name?: string }) => void; + /** + * Apply a pending nation op: `edits.name` overrides a rename's target name + * (or a merge's name); `edits.playerPart` picks the player's successor part + * in a split (default 0). + */ + onConfirmNationOp: (index: number, edits?: { name?: string; playerPart?: number }) => void; /** Drop (reject) a pending nation op by its index. */ onRejectNationOp: (index: number) => void; /** diff --git a/src/components/NationOpDialog.tsx b/src/components/NationOpDialog.tsx index 5ad1fb5..9bf88e2 100644 --- a/src/components/NationOpDialog.tsx +++ b/src/components/NationOpDialog.tsx @@ -19,8 +19,12 @@ interface Props { onOpenChange: (open: boolean) => void; /** A confirm/reject request is in flight — disable the per-op controls. */ opBusy: boolean; - /** Apply a pending op; `edits.name` optionally overrides a rename's target name. */ - onConfirm: (index: number, edits?: { name?: string }) => void; + /** + * Apply a pending op: `edits.name` overrides a rename's target name (or the + * merged name for a merge); `edits.playerPart` picks which new part the + * player's nation becomes in a split (default 0). + */ + onConfirm: (index: number, edits?: { name?: string; playerPart?: number }) => void; /** Drop a pending op (reject / dismiss). */ onReject: (index: number) => void; } @@ -37,10 +41,12 @@ function describeOp(op: NationOp, name: (ref: string) => string): string { /** * Reviews the nation ops the LLM has proposed (stored in `pendingNationOps`) - * and lets the player apply ("LLM proposes, player confirms") or drop each one. - * A rename shows the model's proposed name in an editable field (so the player - * can tweak it before confirming); split and merge are declared but not yet - * applied (P4b) — they can be dismissed but not confirmed. + * and lets the player apply ("LLM proposes, player confirms") or drop each one: + * a rename shows the proposed name in an editable field; a split lists the + * proposed parts (with the number of regions each is assigned) and, when the + * split parent is the player's own nation, radio buttons to pick which new + * part the player leads; a merge shows the input nations (the survivor + * highlighted) and an editable name for the merged state (default `into`). */ export default function NationOpDialog({ game, @@ -52,9 +58,24 @@ export default function NationOpDialog({ }: Props) { const ops = game.pendingNationOps; const name = (ref: string) => game.nations[ref]?.name ?? ref; - // The player's edit to a rename's target name, keyed by pending-op index. - // An unset entry means "use the model's proposed name" (`op.to`). + // Resolve a nation ref the way the server does (id first, then a unique + // case-insensitive name; null when unknown/ambiguous) — UI hints only, the + // server stays authoritative. + const resolveRef = (ref: string): string | null => { + const r = (ref ?? '').trim(); + if (!r) return null; + if (game.nations[r]) return r; + const matches = Object.values(game.nations).filter( + (n) => n.name.trim().toLowerCase() === r.toLowerCase(), + ); + return matches.length === 1 ? matches[0].id : null; + }; + // The player's edits, keyed by pending-op index: the rename/merge target + // name (an unset entry means "use the model's proposed name"), and which + // split part the player succeeds to (preselected 0 — the radio is + // "required" and always has a valid answer). const [edits, setEdits] = useState>({}); + const [playerParts, setPlayerParts] = useState>({}); return ( @@ -68,7 +89,8 @@ export default function NationOpDialog({ The model proposed changes to nations. Confirm to apply them (you can edit a - rename) or reject to leave the world as it is. + name and choose your successor state in a split) or reject to leave the world + as it is. @@ -80,7 +102,34 @@ export default function NationOpDialog({
    {ops.map((op, i) => { const isRename = op.kind === 'rename'; + const isSplit = op.kind === 'split'; + const isMerge = op.kind === 'merge'; const finalName = isRename ? (edits[i] ?? (op as RenameOp).to).trim() : ''; + // Split: is the parent the player's own nation? (the successor radio + // is offered only in that case — the engine ignores playerPart otherwise) + const splitParentIsPlayer = + isSplit && resolveRef((op as { nationRef: string }).nationRef) === game.playerNationId; + const playerPart = playerParts[i] ?? 0; + // Merge: the survivor (highlighted) and whether the proposal is + // even valid (2+ distinct inputs; the survivor among them). + const survivorId = isMerge ? resolveRef((op as { survivingRef: string }).survivingRef) : null; + const inputIds = isMerge + ? new Set( + (op as { nationRefs: string[] }).nationRefs + .map(resolveRef) + .filter((x): x is string => x !== null), + ) + : new Set(); + const mergeValid = + !isMerge || (inputIds.size >= 2 && (survivorId !== null && inputIds.has(survivorId))); + const mergeName = isMerge ? (edits[i] ?? (op as { into: string }).into).trim() : ''; + + const confirm = () => { + if (isRename) onConfirm(i, finalName ? { name: finalName } : undefined); + else if (isMerge) onConfirm(i, mergeName ? { name: mergeName } : undefined); + else onConfirm(i, splitParentIsPlayer ? { playerPart } : undefined); + }; + return (
  • + {isRename ? (
    +
    + + ) : isSplit ? ( +
    +
    + {(op as { parts: { name: string; territory?: string[] }[] }).parts.map((p, pi) => ( +
    + {splitParentIsPlayer ? ( + setPlayerParts((prev) => ({ ...prev, [i]: pi }))} + disabled={opBusy} + className="size-3.5 shrink-0 accent-amber-400" + aria-label={`Make ${p.name} the player's successor state`} + data-testid="nation-op-player-part" + /> + ) : ( + + )} + {p.name} + + {p.territory?.length ?? 0} region{(p.territory?.length ?? 0) === 1 ? '' : 's'} + +
    + ))} +
    + {splitParentIsPlayer && ( +

    + Choose which new state you will lead after the split. +

    + )} + {(op as { parts: unknown[] }).parts.length < 2 && ( +

    + This split names fewer than two parts — confirming will dismiss it + without changing the world. +

    + )} +
    + +
    ) : ( -
    - - {op.kind === 'split' ? 'Split' : 'Merge'} isn’t available yet — dismiss to ignore it. - - +
    +
    + {(op as { nationRefs: string[] }).nationRefs.map((ref, ri) => { + const isSurvivor = resolveRef(ref) === survivorId; + return ( + + {name(ref)} + {isSurvivor ? ' (survivor)' : ''} + + ); + })} +
    + setEdits((p) => ({ ...p, [i]: e.target.value }))} + placeholder="New name…" + data-testid="nation-op-name-input" + /> + {!mergeValid && ( +

    + This merge is not valid as proposed — confirming will dismiss it + without changing the world. +

    + )} +
    + + +
    )}
    • diff --git a/src/lib/api.ts b/src/lib/api.ts index f60af69..9d6bcb0 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -62,10 +62,13 @@ export const api = { method: 'POST', body: JSON.stringify({ index }), }), - // (v0.23, #16 P4a) Confirm a pending nation-op proposal by its index in - // `game.pendingNationOps` (a rename takes effect; `edits.name` overrides the - // proposed name). Fast, non-LLM — no turn advance. Returns the updated game. - confirmNationOp: (id: string, index: number, edits?: { name?: string }) => + // (v0.23, #16 P4a; v0.24, #16 P4b) Confirm a pending nation-op proposal by + // its index in `game.pendingNationOps`: a rename takes effect (`edits.name` + // overrides the proposed name), a split partitions the nation's territory + // (`edits.playerPart` picks which part the player leads, default 0), and a + // merge folds its inputs into the survivor (`edits.name` overrides the + // merged name). Fast, non-LLM — no turn advance. Returns the updated game. + confirmNationOp: (id: string, index: number, edits?: { name?: string; playerPart?: number }) => req(`/api/games/${id}/nation-op/confirm`, { method: 'POST', body: JSON.stringify({ index, ...(edits ? { edits } : {}) }),