From 9ee052c51e99151b80c33c27ce71f5d1ab1d28d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 07:37:04 +0000 Subject: [PATCH] Give the adventure a persona, so the protagonist has a name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The player had stats but no identity. `stat_schema.player` carried hp and mana beside `npc.gwen.trust`, but where an NPC has a name and a description the player had neither, so the block rendered as `You: hp 100/100` and nothing in the prompt said who "you" was. Three columns on `adventures`: name, pronouns, description. All user-only, all optional, and an empty name means the app behaves exactly as it did before — no backfill, no special case for an adventure that predates the migration. They are adventure columns rather than part of `stat_schema` for two reasons. An adventure with no RPG layer still has a protagonist, and that is the case this was added for. And `worldstate.schema._initials` treats every dict inside a stat section as a stat definition, so a persona placed there would be instantiated, rendered in the guide, and handed an `initial` value as though it were one. The paths do not change. `player.hp` stays `player.hp`; only the label moves, to `Kaelen (player): hp 100/100`, the same way NPC lines already print a display name beside the id. A path carrying the persona's name would break the moment a player renamed their character, because `_history_text` replays every past turn's stored delta into the prompt and those blobs hold literal `player.hp` strings. The section sits in the system block. Only the user can edit it, so it never changes mid-story and stays inside the cached prefix. That is what makes it free, and it is why the AI must not be able to move it — a delta aimed at `persona.*` is already refused by `_resolve`, and there is now a test holding that in place. The modal that used to appear only for scenarios with `${Placeholder}` tokens now always opens, and is where the character is named. Persona and placeholders stay independent: a scenario asking for `${Name}` is asking its own question. No scenario in the repo uses placeholders at all, so the overlap is hypothetical. Phase 2, which feeds the persona and the cast to the summarizer, is written up in plan/18 and not started. That is where the memory-quality problem actually gets fixed; this change is what gives it a name to use. Not yet driven in a browser — plan/18 lists what to check by hand. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NPyQN926gkZTAYfgugcaok --- backend/app/bundle.py | 27 ++ backend/app/context/__init__.py | 3 +- backend/app/context/builder.py | 37 +- backend/app/migrations.py | 6 + backend/app/models.py | 14 + backend/app/routers/adventures/crud.py | 6 + backend/app/schemas.py | 16 + backend/app/worldstate/render.py | 26 +- backend/tests/test_bundle_v2.py | 47 +++ backend/tests/test_persona.py | 359 +++++++++++++++++ backend/tests/test_prompt_caching.py | 9 +- docs/GUIDE.md | 1 + frontend/src/components.jsx | 84 +++- frontend/src/pages/Home.jsx | 19 +- .../pages/Play/drawers/WorldStateDrawer.jsx | 7 +- frontend/src/pages/Play/index.jsx | 3 +- frontend/src/pages/Play/panels/PlotPanel.jsx | 19 + frontend/src/pages/Scenarios.jsx | 35 +- frontend/src/styles/modals.css | 12 + frontend/src/styles/panels.css | 12 + plan/18-persona-and-memory-quality.md | 373 ++++++++++++++++++ 21 files changed, 1076 insertions(+), 39 deletions(-) create mode 100644 backend/tests/test_persona.py create mode 100644 plan/18-persona-and-memory-quality.md diff --git a/backend/app/bundle.py b/backend/app/bundle.py index 5f368af..9381244 100644 --- a/backend/app/bundle.py +++ b/backend/app/bundle.py @@ -101,6 +101,14 @@ def export(db: Session, adventure: models.Adventure) -> dict: "authorsNote": adventure.authors_note, "aiInstructions": adventure.ai_instructions, "storySummary": adventure.story_summary, + # Phase 18. A bundle written before personas existed has no key here, + # and the import below reads it with `.get`, so it lands with an empty + # persona — which is the same as having none. No FORMAT bump needed. + "persona": { + "name": adventure.persona_name, + "pronouns": adventure.persona_pronouns, + "desc": adventure.persona_desc, + }, "scriptState": adventure.script_state, "worldState": adventure.world_state, "autoSummarize": adventure.auto_summarize, @@ -194,6 +202,24 @@ def _exported_memory(memory: models.Memory, local: dict[int, int]) -> dict: } +def _imported_persona(persona) -> dict: + """Reads a bundle's `persona` block into `Adventure` keyword arguments. + + A raw-dict import bypasses the schemas, so the strings are truncated to the + widths the columns declare, in the same way the rest of `_import` does. A + bundle written before Phase 18 has no block at all, and an empty persona is + the same as having none. + """ + if not isinstance(persona, dict): + return {} + return { + "persona_name": str(persona.get("name") or "")[:schemas.PERSONA_NAME_MAX], + "persona_pronouns": + str(persona.get("pronouns") or "")[:schemas.PERSONA_PRONOUNS_MAX], + "persona_desc": str(persona.get("desc") or ""), + } + + def _exported_anchor( adventure: models.Adventure, cursor: cursors.Cursor, local: dict[int, int] ) -> dict: @@ -632,6 +658,7 @@ def materialize( world_state=payload.get("worldState") or {}, auto_summarize=bool(payload.get("autoSummarize", False)), memory_bank_enabled=bool(payload.get("memoryBankEnabled", False)), + **_imported_persona(payload.get("persona")), ) db.add(adventure) db.flush() diff --git a/backend/app/context/__init__.py b/backend/app/context/__init__.py index 674f15f..4145c6e 100644 --- a/backend/app/context/__init__.py +++ b/backend/app/context/__init__.py @@ -1,11 +1,12 @@ from . import history -from .builder import build_context, count_tokens, truncate_to_last_tokens +from .builder import build_context, count_tokens, render_persona, truncate_to_last_tokens from .history import story_actions __all__ = [ "build_context", "count_tokens", "history", + "render_persona", "story_actions", "truncate_to_last_tokens", ] diff --git a/backend/app/context/builder.py b/backend/app/context/builder.py index 2b97716..ca96a63 100644 --- a/backend/app/context/builder.py +++ b/backend/app/context/builder.py @@ -2,6 +2,7 @@ (help.aidungeon.com/faq/the-memory-system): [AI Instructions] always included + [Player Character] always included when the adventure has a persona [Plot Essentials] always included (classic "Memory") [Story Summary] always included (manual in Phase 3, auto in Phase 6) [Used Memories] top-K memory-bank retrievals (Phase 6, when enabled) @@ -130,6 +131,30 @@ def length_hint(max_output_tokens: int, *, has_ws: bool) -> str: ) +def render_persona(adventure: models.Adventure) -> str: + """Returns the Player Character section, or "" when there is no persona. + + The three fields are independent. A name alone is enough, a description + alone is enough, and the wording holds together for either. Pronouns are + stated because the summarizer in `memorybank` writes about the protagonist + in the third person, and a model that has to infer a pronoun from a name + will sometimes infer wrongly and then repeat that error in every memory it + writes. + """ + name = adventure.persona_name.strip() + pronouns = adventure.persona_pronouns.strip() + desc = adventure.persona_desc.strip() + if not (name or desc): + return "" + head = f"You are {name}" if name else "" + if head and pronouns: + head += f" ({pronouns})" + # Joined with a space, not `SEPARATOR`: this is one short paragraph about + # one character, and a blank line inside it reads as two unrelated notes. + body = " ".join(part for part in (f"{head}." if head else "", desc) if part) + return f"Player character:\n{body}" + + def _script_memory(adventure: models.Adventure) -> dict: """Script-provided memory overrides (populated by Phase 4 scripting).""" state = adventure.script_state if isinstance(adventure.script_state, dict) else {} @@ -228,8 +253,9 @@ def build_context( # schema and the emit rule do not change while the scenario is unchanged. stat_schema = adventure.scenario.stat_schema if adventure.scenario else None has_ws = worldstate.has_schema(stat_schema) + persona_name = adventure.persona_name.strip() if has_ws: - guide = worldstate.render_reference(stat_schema) + guide = worldstate.render_reference(stat_schema, persona_name) if guide: system_sections.append(Section("world_state_guide", guide)) system_sections.append(Section("world_state_rule", worldstate.EMIT_RULE)) @@ -238,6 +264,14 @@ def build_context( system_sections.append(Section("script_context", script_mem["context"].strip())) if adventure.ai_instructions.strip(): system_sections.append(Section("ai_instructions", adventure.ai_instructions.strip())) + # Phase 18. This sits in the static block because only the user can edit it, + # so it never changes mid-story and stays inside the cached prefix. It is + # emitted whether or not the adventure has an RPG layer: an adventure with + # no stats still has a protagonist, and that is the case the persona was + # added for. + persona_text = render_persona(adventure) + if persona_text: + system_sections.append(Section("persona", persona_text)) if adventure.memory.strip(): system_sections.append( Section("plot_essentials", f"Plot essentials:\n{adventure.memory.strip()}") @@ -267,6 +301,7 @@ def build_context( recent = history.tail(adventure, NPC_WINDOW, exclude_action_id) block = worldstate.render_state_section( adventure.world_state, stat_schema, _visible_npcs(recent, stat_schema), + persona_name, ) if block: world_state_section = Section("world_state", block) diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 65ebab8..5270361 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -332,6 +332,12 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ (71, 'ALTER TABLE actions DROP COLUMN "index"'), (72, "ALTER TABLE adventures DROP COLUMN memory_cursor"), (73, "ALTER TABLE adventures DROP COLUMN summary_cursor"), + # Phase 18: the persona. Defaulting to '' rather than NULL means every read + # is a plain string test, and an adventure that predates the feature is + # indistinguishable from one whose player left the fields blank. + (74, "ALTER TABLE adventures ADD COLUMN persona_name VARCHAR(80) NOT NULL DEFAULT ''"), + (75, "ALTER TABLE adventures ADD COLUMN persona_pronouns VARCHAR(40) NOT NULL DEFAULT ''"), + (76, "ALTER TABLE adventures ADD COLUMN persona_desc TEXT NOT NULL DEFAULT ''"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) diff --git a/backend/app/models.py b/backend/app/models.py index 94d146f..dba938a 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -107,6 +107,20 @@ class Adventure(Base): authors_note: Mapped[str] = mapped_column(Text, default="") ai_instructions: Mapped[str] = mapped_column(Text, default="") story_summary: Mapped[str] = mapped_column(Text, default="") + # Phase 18: who the player is playing as. The AI never writes these — they + # are user-only, which is what lets them sit in the cached system block + # rather than below the history with the values that change. An empty + # `persona_name` means the adventure has no persona, and every read below + # falls back to the wording used before this existed. + # + # These are adventure columns rather than part of the scenario's + # `stat_schema`, for two reasons. An adventure with no RPG layer still has a + # protagonist, and `worldstate.schema._initials` treats every dict inside a + # stat section as a stat definition, so a persona placed there would be + # instantiated, rendered and given an `initial` value as though it were one. + persona_name: Mapped[str] = mapped_column(String(80), default="") + persona_pronouns: Mapped[str] = mapped_column(String(40), default="") + persona_desc: Mapped[str] = mapped_column(Text, default="") script_state: Mapped[dict] = mapped_column(JSON, default=dict) # Phase 12: live RPG world state (world/player/npc stats + milestones), # instantiated from the scenario's stat_schema. Empty when there's no RPG layer. diff --git a/backend/app/routers/adventures/crud.py b/backend/app/routers/adventures/crud.py index f42052e..e2d2876 100644 --- a/backend/app/routers/adventures/crud.py +++ b/backend/app/routers/adventures/crud.py @@ -178,6 +178,12 @@ def create_adventure( # Stored so that a later "Update from scenario" fills the copied text # with the same answers instead of inserting literal `${...}` tokens. placeholders=dict(values), + # Phase 18: the persona the player named before starting. It belongs to + # the adventure rather than the scenario, so nothing is copied here and + # "Update from scenario" never touches it. + persona_name=payload.persona_name.strip(), + persona_pronouns=payload.persona_pronouns.strip(), + persona_desc=payload.persona_desc.strip(), ) db.add(adventure) db.flush() diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 905659b..9ff2c28 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -24,6 +24,8 @@ MEMORY_TEXT_MAX = 5_000 IMAGE_MAX = 400_000 ICON_MAX = 16 # One emoji or glyph. VARCHAR(16). BRANCH_NAME_MAX = 80 # What a player called one line of the story. VARCHAR(80). +PERSONA_NAME_MAX = 80 # The protagonist's name. VARCHAR(80). +PERSONA_PRONOUNS_MAX = 40 # "they/them" and the like. VARCHAR(40). Name = Annotated[str, Field(max_length=NAME_MAX)] Tags = Annotated[str, Field(max_length=TAGS_MAX)] @@ -33,6 +35,8 @@ ScriptSource = Annotated[str, Field(max_length=SCRIPT_MAX)] ActionText = Annotated[str, Field(max_length=ACTION_MAX)] Image = Annotated[str, Field(max_length=IMAGE_MAX)] Icon = Annotated[str, Field(max_length=ICON_MAX)] +PersonaName = Annotated[str, Field(max_length=PERSONA_NAME_MAX)] +PersonaPronouns = Annotated[str, Field(max_length=PERSONA_PRONOUNS_MAX)] class ORMModel(BaseModel): @@ -141,6 +145,12 @@ class AdventureCreate(BaseModel): # The `${Placeholder}` values collected from the player at the start, which # is the AI Dungeon behavior. placeholders: dict[str, str] = {} + # Phase 18: who the player is playing as, collected by the same modal. These + # are independent of `placeholders`: a scenario that asks for `${Name}` is + # asking its own question, and nothing here fills it in. + persona_name: PersonaName = "" + persona_pronouns: PersonaPronouns = "" + persona_desc: Prose = "" class AdventureUpdate(BaseModel): @@ -151,6 +161,9 @@ class AdventureUpdate(BaseModel): story_summary: Prose | None = None auto_summarize: bool | None = None memory_bank_enabled: bool | None = None + persona_name: PersonaName | None = None + persona_pronouns: PersonaPronouns | None = None + persona_desc: Prose | None = None class AdventureRefresh(BaseModel): @@ -297,6 +310,9 @@ class AdventureOut(ORMModel): story_summary: str auto_summarize: bool memory_bank_enabled: bool + persona_name: str + persona_pronouns: str + persona_desc: str created_at: datetime updated_at: datetime story_cards: list[StoryCardOut] = [] diff --git a/backend/app/worldstate/render.py b/backend/app/worldstate/render.py index 421a5bc..708970c 100644 --- a/backend/app/worldstate/render.py +++ b/backend/app/worldstate/render.py @@ -25,11 +25,16 @@ def _stat_line(defs: dict, values: dict) -> str: def render_state_section(world_state: dict, stat_schema: dict, - visible_npcs: dict[str, str]) -> str: + visible_npcs: dict[str, str], + player_name: str = "") -> str: """Returns the compact context block that every turn includes. `visible_npcs` maps a card id to a display name, for the NPCs currently in the scene. + + `player_name` is the persona's name (Phase 18), used to label the player's + stat block. It is empty for an adventure with no persona, and the block then + reads `You:` as it did before personas existed. """ ws = world_state if isinstance(world_state, dict) else {} lines: list[str] = [] @@ -42,7 +47,12 @@ def render_state_section(world_state: dict, stat_schema: dict, player_defs = stat_schema.get("player") or {} player_line = _stat_line(player_defs, ws.get("player") or {}) if player_line: - lines.append(f"You: {player_line}.") + # Name the block after the persona, and show the path beside it exactly + # as the NPC lines below do. The model reads the name in the narration, + # so without the path in view it writes `kaelen.hp` and the delta is + # refused as an unknown path. + label = f"{player_name} (player)" if player_name else "You" + lines.append(f"{label}: {player_line}.") npcs = stat_schema.get("npcs") or {} npc_state = ws.get("npc") or {} @@ -122,14 +132,24 @@ def _describe_stat(name: str, d: dict) -> str | None: return f"{label} — {'; '.join(bits)}." if bits else None -def render_reference(stat_schema: dict) -> str: +def render_reference(stat_schema: dict, player_name: str = "") -> str: """Returns a fixed, per-scenario legend for the stats. Each line gives what a stat means, from its `desc`, and its band ladder. The legend does not change from turn to turn, and it is separate from the live values. + + `player_name` names the persona, so that the guide ties the name the model + reads in the story to the `player.` paths it has to write. The persona's + description is deliberately not repeated here: it has its own section, and + this guide is about paths. """ lines: list[str] = [] + if player_name: + lines.append( + f"Protagonist {player_name} — the player character; their stats are " + f"addressed as player.." + ) for section in STAT_SECTIONS: for name, d in (stat_schema.get(section) or {}).items(): if isinstance(d, dict): diff --git a/backend/tests/test_bundle_v2.py b/backend/tests/test_bundle_v2.py index 20f8eec..9d97c0c 100644 --- a/backend/tests/test_bundle_v2.py +++ b/backend/tests/test_bundle_v2.py @@ -586,3 +586,50 @@ def test_an_unknown_format_is_refused(client): r = _import(client, {"format": "ai-dnd-adventure-v3", "title": "From the future"}) assert r.status_code == 400, r.text assert bundle.FORMAT in r.json()["detail"] + + +# ------------------------------------------------------- the persona (Phase 18) + +def test_the_persona_survives_the_round_trip(client): + adv_id = client.adv_id + assert client.patch(f"/api/adventures/{adv_id}", json={ + "persona_name": "Kaelen", + "persona_pronouns": "he/him", + "persona_desc": "A half-elf ranger, exiled from the northern holds.", + }).status_code == 200 + + payload = _export(client, adv_id) + assert payload["persona"] == { + "name": "Kaelen", + "pronouns": "he/him", + "desc": "A half-elf ranger, exiled from the northern holds.", + } + + copy_id = _imported(client, payload) + r = client.get(f"/api/adventures/{copy_id}") + assert r.status_code == 200, r.text + copy = r.json() + assert copy["persona_name"] == "Kaelen" + assert copy["persona_pronouns"] == "he/him" + assert copy["persona_desc"].startswith("A half-elf ranger") + + +def test_a_bundle_written_before_personas_still_imports(client): + """The key is read with `.get`, so a v2 file from before Phase 18 lands + with an empty persona rather than failing. No FORMAT bump was needed.""" + payload = _export(client, client.adv_id) + del payload["persona"] + copy = client.get(f"/api/adventures/{_imported(client, payload)}").json() + assert copy["persona_name"] == "" + assert copy["persona_pronouns"] == "" + assert copy["persona_desc"] == "" + + +def test_an_over_long_persona_in_a_raw_bundle_is_truncated(client): + """A raw-dict import bypasses the schemas, so the widths the columns + declare are enforced here instead. Postgres would reject the INSERT.""" + payload = _export(client, client.adv_id) + payload["persona"] = {"name": "K" * 500, "pronouns": "p" * 500, "desc": "d"} + copy = client.get(f"/api/adventures/{_imported(client, payload)}").json() + assert len(copy["persona_name"]) == 80 + assert len(copy["persona_pronouns"]) == 40 diff --git a/backend/tests/test_persona.py b/backend/tests/test_persona.py new file mode 100644 index 0000000..9eec972 --- /dev/null +++ b/backend/tests/test_persona.py @@ -0,0 +1,359 @@ +"""Phase 18: the adventure's persona — who the player is playing as. + +Before this existed the protagonist had no name anywhere in the prompt. The +player's stat block rendered as `You: hp 100/100` and the summarizer was handed +second-person prose with nothing to say who "you" was. + +The rules this file holds in place: + +* An empty `persona_name` behaves exactly as the app did before personas + existed, so an adventure that predates the migration is not a special case. +* The persona is in the **system** block. It is user-only, so it never changes + during a story, and putting it in the cached prefix is what makes it free. +* A persona works without an RPG layer. That is the case it was added for. +* Naming the block does not rename the path. The model still writes + `player.hp`, and a delta aimed at `persona.*` is refused. + + python -m pytest tests/test_persona.py -v +""" +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient + +from app import auth, limits, models, worldstate +from app.context import builder +from app.database import Base, SessionLocal, engine, get_db +from app.main import app + +SCHEMA = { + "player": {"hp": {"min": 0, "max": 100, "initial": 100, "desc": "Health"}}, + "world": {"day": {"min": 1, "initial": 1, "desc": "Day"}}, +} + + +def _persona(**kwargs) -> models.Adventure: + """An unsaved adventure carrying only the fields `render_persona` reads.""" + return models.Adventure( + persona_name=kwargs.get("name", ""), + persona_pronouns=kwargs.get("pronouns", ""), + persona_desc=kwargs.get("desc", ""), + ) + + +# ------------------------------------------------------------- the rendering + +def test_no_persona_renders_nothing(): + assert builder.render_persona(_persona()) == "" + + +def test_whitespace_only_is_the_same_as_empty(): + """The columns default to '', but a player can clear a field to spaces.""" + assert builder.render_persona(_persona(name=" ", pronouns=" ", desc="\n")) == "" + + +def test_name_and_pronouns_and_description(): + text = builder.render_persona( + _persona(name="Kaelen", pronouns="he/him", desc="A half-elf ranger.") + ) + assert text.startswith("Player character:\n") + assert "You are Kaelen (he/him)." in text + assert "A half-elf ranger." in text + + +def test_name_alone_is_enough(): + text = builder.render_persona(_persona(name="Kaelen")) + assert "You are Kaelen." in text + assert "(" not in text, "no empty pronoun bracket" + + +def test_description_alone_is_enough(): + """A player who names no character still gets their description through.""" + text = builder.render_persona(_persona(desc="A nameless wanderer.")) + assert "A nameless wanderer." in text + assert "You are" not in text + + +# ---------------------------------------------------- the world-state labels + +def test_state_section_labels_the_block_with_the_name(): + state = worldstate.instantiate(SCHEMA) + block = worldstate.render_state_section(state, SCHEMA, {}, "Kaelen") + # The path travels with the name. Without it the model reads "Kaelen" in + # the prose and writes `kaelen.hp`, which `apply_delta` then refuses. + assert "Kaelen (player): hp 100/100" in block + + +def test_state_section_falls_back_to_you(): + state = worldstate.instantiate(SCHEMA) + block = worldstate.render_state_section(state, SCHEMA, {}, "") + assert "You: hp 100/100" in block + assert "(player)" not in block + + +def test_reference_ties_the_name_to_the_path(): + guide = worldstate.render_reference(SCHEMA, "Kaelen") + assert "Protagonist Kaelen" in guide + assert "player." in guide + + +def test_reference_without_a_persona_is_unchanged(): + assert "Protagonist" not in worldstate.render_reference(SCHEMA, "") + + +def test_reference_does_not_repeat_the_description(): + """The description has its own section. The guide is about paths.""" + guide = worldstate.render_reference(SCHEMA, "Kaelen") + assert "half-elf" not in guide + + +# ------------------------------------------------------ the assembled prompt + +@pytest.fixture() +def story(): + """An adventure with an RPG layer, a persona, and a little history.""" + Base.metadata.create_all(bind=engine) + db = SessionLocal() + user = models.User(is_guest=False, email="persona@example.com") + db.add(user) + db.flush() + settings = models.Settings(user_id=user.id, api_key="enc:dummy", model="m") + db.add(settings) + scenario = models.Scenario( + user_id=user.id, title="S", prompt="A road.", stat_schema=SCHEMA + ) + db.add(scenario) + db.flush() + adventure = models.Adventure( + user_id=user.id, title="A", scenario_id=scenario.id, script_state={}, + memory="The hero hunts bandits.", + ai_instructions="Write in second person.", + story_summary="The hero left the village.", + world_state=worldstate.instantiate(SCHEMA), + persona_name="Kaelen", + persona_pronouns="he/him", + persona_desc="A half-elf ranger, exiled from the northern holds.", + ) + db.add(adventure) + db.flush() + for i in range(6): + db.add(models.Action(adventure_id=adventure.id, + type="ai" if i % 2 else "do", + text=f"[{i}] The road bends onward past the treeline.")) + db.commit() + db.expire_all() + adventure = db.get(models.Adventure, adventure.id) + settings = db.get(models.Settings, settings.id) + try: + yield db, adventure, settings + finally: + db.close() + Base.metadata.drop_all(bind=engine) + + +def test_persona_is_in_the_system_block(story): + """It is user-only, so it belongs in the cached prefix rather than below + the history with the values that move.""" + db, adventure, settings = story + system_text, story_text, _ = builder.build_context(adventure, settings) + assert "You are Kaelen (he/him)." in system_text + assert "A half-elf ranger" in system_text + assert "Kaelen (he/him)" not in story_text + + +def test_persona_does_not_move_between_turns(story): + """The static block has to be byte-identical across turns. A persona that + changed with the world state would re-price the whole history.""" + db, adventure, settings = story + before, _, _ = builder.build_context(adventure, settings) + adventure.world_state = {**adventure.world_state, "player": {"hp": 40}} + db.commit() + after, story_text, _ = builder.build_context(adventure, settings) + assert before == after + assert "Kaelen (player): hp 40/100" in story_text + + +def test_persona_sits_above_the_plot_essentials(story): + db, adventure, settings = story + _, _, report = builder.build_context(adventure, settings) + labels = [s["label"] for s in report["sections"]] + assert labels.index("persona") < labels.index("plot_essentials") + + +def test_no_persona_section_when_the_fields_are_blank(story): + db, adventure, settings = story + adventure.persona_name = "" + adventure.persona_pronouns = "" + adventure.persona_desc = "" + db.commit() + system_text, story_text, report = builder.build_context(adventure, settings) + assert "persona" not in [s["label"] for s in report["sections"]] + assert "Player character" not in system_text + assert "You: hp 100/100" in story_text + + +def test_persona_is_charged_to_the_token_budget(story): + """It moved into `system_sections`, and everything there is counted in + `reserved`. If it were not, the history would overrun the budget.""" + db, adventure, settings = story + _, _, report = builder.build_context(adventure, settings) + persona = next(s for s in report["sections"] if s["label"] == "persona") + assert persona["tokens"] > 0 + assert report["tokens"]["total"] >= persona["tokens"] + + +# --------------------------------------------- a persona with no RPG layer + +@pytest.fixture() +def plain(): + """A blank adventure: no scenario, so no `stat_schema` at all.""" + Base.metadata.create_all(bind=engine) + db = SessionLocal() + user = models.User(is_guest=False, email="plain@example.com") + db.add(user) + db.flush() + settings = models.Settings(user_id=user.id, api_key="enc:dummy", model="m") + db.add(settings) + adventure = models.Adventure( + user_id=user.id, title="A", script_state={}, world_state={}, + persona_name="Wren", persona_pronouns="they/them", + persona_desc="A courier who reads other people's letters.", + ) + db.add(adventure) + db.flush() + db.add(models.Action(adventure_id=adventure.id, type="do", text="Walk east.")) + db.commit() + db.expire_all() + adventure = db.get(models.Adventure, adventure.id) + settings = db.get(models.Settings, settings.id) + try: + yield db, adventure, settings + finally: + db.close() + Base.metadata.drop_all(bind=engine) + + +def test_persona_works_without_an_rpg_layer(plain): + """The case the persona exists for. It must not be gated behind `has_ws`.""" + db, adventure, settings = plain + system_text, _, _ = builder.build_context(adventure, settings) + assert "You are Wren (they/them)." in system_text + assert "A courier who reads other people's letters." in system_text + + +# ------------------------------------------------- the AI cannot rewrite it + +@pytest.mark.parametrize("path", ["persona.name", "persona.desc", "persona"]) +def test_a_delta_aimed_at_the_persona_is_refused(path): + """Nothing was built for this: `_resolve` rejects unknown paths already. + The test holds the behavior in place, because the persona sitting in the + cached prefix depends on the model being unable to move it.""" + state = worldstate.instantiate(SCHEMA) + _, report = worldstate.apply_delta(state, SCHEMA, {path: "Bob"}, 1) + assert [r["path"] for r in report["rejected"]] == [path] + assert not report["applied"] + + +def test_the_player_path_still_works_with_a_persona_set(): + """Naming the block does not rename the path.""" + state = worldstate.instantiate(SCHEMA) + new_state, report = worldstate.apply_delta(state, SCHEMA, {"player.hp": -15}, 1) + assert new_state["player"]["hp"] == 85 + assert [r["path"] for r in report["applied"]] == ["player.hp"] + + +# ------------------------------------- setting it at the start, editing it later + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + setup = SessionLocal() + user = models.User(is_guest=False, email="persona-api@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings(user_id=user.id, api_key="enc:dummy", model="test-model")) + setup.commit() + user_id = user.id + setup.close() + + monkeypatch.setattr(limits, "rate_limit", lambda *a, **k: None) + monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None) + + def _current_user(db=Depends(get_db)): + return db.get(models.User, user_id) + + app.dependency_overrides[auth.get_current_user] = _current_user + c = TestClient(app) + try: + yield c + finally: + app.dependency_overrides.clear() + Base.metadata.drop_all(bind=engine) + + +def test_a_persona_can_be_named_when_the_adventure_is_created(client): + """The modal that used to appear only for `${Placeholder}` scenarios now + always opens, and this is what it posts.""" + r = client.post("/api/adventures", json={ + "title": "A", "persona_name": "Kaelen", "persona_pronouns": "he/him", + "persona_desc": "A half-elf ranger.", + }) + assert r.status_code == 201, r.text + body = r.json() + assert body["persona_name"] == "Kaelen" + assert body["persona_pronouns"] == "he/him" + assert body["persona_desc"] == "A half-elf ranger." + + +def test_creating_without_a_persona_still_works(client): + """Every field is optional. A blank adventure is the old behavior.""" + r = client.post("/api/adventures", json={"title": "A"}) + assert r.status_code == 201, r.text + assert r.json()["persona_name"] == "" + + +def test_the_persona_is_stripped_on_the_way_in(client): + r = client.post("/api/adventures", json={ + "title": "A", "persona_name": " Kaelen ", "persona_desc": " ranger \n", + }) + assert r.json()["persona_name"] == "Kaelen" + assert r.json()["persona_desc"] == "ranger" + + +def test_the_persona_can_be_edited_later(client): + """A player renames their character mid-story; nothing else moves.""" + adv_id = client.post("/api/adventures", json={ + "title": "A", "persona_name": "Kaelen", + }).json()["id"] + r = client.patch(f"/api/adventures/{adv_id}", json={"persona_name": "Aria"}) + assert r.status_code == 200, r.text + assert r.json()["persona_name"] == "Aria" + assert client.get(f"/api/adventures/{adv_id}").json()["persona_name"] == "Aria" + + +def test_an_over_long_name_is_refused_rather_than_truncated(client): + """The column is VARCHAR(80). A 422 here is a 500 at INSERT otherwise.""" + r = client.post("/api/adventures", json={"title": "A", "persona_name": "K" * 81}) + assert r.status_code == 422 + + +def test_placeholders_and_the_persona_are_independent(client): + """A scenario asking for `${Name}` is asking its own question. Nothing + fills it in from the persona, and nothing fills the persona from it.""" + db = SessionLocal() + scenario = models.Scenario( + user_id=db.query(models.User).first().id, + title="S", prompt="A guard sneers at ${Name}.", + ) + db.add(scenario) + db.commit() + scenario_id = scenario.id + db.close() + + adv = client.post("/api/adventures", json={ + "scenario_id": scenario_id, + "persona_name": "Kaelen", + "placeholders": {"Name": "Wren"}, + }).json() + assert adv["persona_name"] == "Kaelen" + opening = next(a for a in adv["actions"] if a["type"] == "start") + assert "A guard sneers at Wren." == opening["text"] diff --git a/backend/tests/test_prompt_caching.py b/backend/tests/test_prompt_caching.py index e9d65c3..0aa6c29 100644 --- a/backend/tests/test_prompt_caching.py +++ b/backend/tests/test_prompt_caching.py @@ -130,6 +130,12 @@ def story(): ai_instructions="Write in second person.", story_summary="The hero left the village.", world_state=worldstate.instantiate(SCHEMA), + # Phase 18. Set here so that every test in this file runs with a + # persona present: it is user-only, so it belongs in the static block, + # and this is the file that guards what may live there. + persona_name="Kaelen", + persona_pronouns="he/him", + persona_desc="A half-elf ranger.", ) db.add(adventure) db.flush() @@ -164,7 +170,8 @@ def test_changing_a_stat_leaves_the_static_block_untouched(story): def test_the_static_block_holds_the_things_that_do_not_move(story): db, adventure, settings = story system_text, story_text, _ = builder.build_context(adventure, settings) - for fixed in ("Write in second person.", "The hero hunts bandits."): + for fixed in ("Write in second person.", "The hero hunts bandits.", + "You are Kaelen (he/him).", "A half-elf ranger."): assert fixed in system_text # The stat guide is derived from the schema, so it is fixed. The live # values it describes are not fixed, and belong to the story text. diff --git a/docs/GUIDE.md b/docs/GUIDE.md index 8c489e2..5eae3bc 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -154,6 +154,7 @@ Fixed (always included, whatever they cost): | `world_state` | Current values of every stat, plus NPCs in scene. | | `world_state_rule` | How to report changes. | | `ai_instructions` | Per-adventure steering. | +| `persona` | Who the player is playing as: name, pronouns, description. Only the player edits it, so it never moves. | | `plot_essentials` | AI Dungeon's "Memory": the premise. | | `story_summary` | The auto-maintained running summary. | | `used_memories` | Top-K retrievals from the memory bank. | diff --git a/frontend/src/components.jsx b/frontend/src/components.jsx index df64be7..2d2170b 100644 --- a/frontend/src/components.jsx +++ b/frontend/src/components.jsx @@ -199,30 +199,83 @@ export function extractPlaceholders(...texts) { return names } -export function PlaceholderModal({ title, names, onSubmit, onCancel }) { +// The modal shown before an adventure begins. It always asks who the player is +// playing as, and it also collects any `${Placeholder}` answers the scenario's +// text asks for. +// +// The two sets of fields are independent on purpose. A scenario that writes +// `${Name}` is asking its own question, and the persona does not answer it. No +// scenario in the repo uses placeholders at all, so the overlap is hypothetical; +// pre-filling one from the other is a small change here if it ever bites. +// +// `onSubmit` receives `{ persona, placeholders }`. Every persona field is +// optional — submitting them all blank gives an adventure with no persona, +// which behaves exactly as adventures did before personas existed. +export function BeginAdventureModal({ title, names = [], onSubmit, onCancel }) { + const [persona, setPersona] = useState({ name: '', pronouns: '', desc: '' }) const [values, setValues] = useState(Object.fromEntries(names.map((n) => [n, '']))) + const setField = (field, value) => setPersona((p) => ({ ...p, [field]: value })) + const submit = (e) => { e.preventDefault() - onSubmit(values) + onSubmit({ persona, placeholders: values }) } return (
e.stopPropagation()} onSubmit={submit}>

{title}

-

This scenario asks a few questions before you begin.

- {names.map((name, i) => ( -