diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index c798f80..d5e7608 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -209,7 +209,7 @@ visible from within. ## Tests ```bash -cd backend && .venv/bin/python -m pytest tests/ -q # 638 tests +cd backend && .venv/bin/python -m pytest tests/ -q # 680 tests cd frontend && npm run lint && npm run build ``` @@ -225,6 +225,10 @@ suite as complete evidence. is lost from the union, or if a new HTTP client is added without the shared verification context. +M4 added `test_save_points.py`, which fails if restoring a Save Point starts +deleting history, stops going through the active head, forks on its own, or lets +a Save Point on one campaign be restored through another. + M2 added two more. `test_endpoint_policy.py` fails if the set of reachable addresses widens, or if either place the rule is applied stops applying it — it resolves hostnames through a stub, so it tests the policy rather than diff --git a/README.md b/README.md index ad9f342..d2f2043 100644 --- a/README.md +++ b/README.md @@ -67,10 +67,20 @@ that isn't the live one starts a new branch. displaced future stays on the line it was written for, and ordinary Redo stops offering it. Nothing a retry replaces is discarded either — the old attempt stays as another take of that turn, one keystroke and one click from becoming a branch of its own. +- **Save Points.** Name a moment — "Before entering the abbey" — keep playing, + restart the app, and come back to it. Restoring one moves the story back to + that moment and deletes nothing: the turns you wrote after it stay, Redo still + walks forward into them, and writing something different from the Save Point + is what starts a new line while the old one is kept. A Save Point is a name for + a position and holds no copy of the story, so restoring it is the same + movement Undo makes (`backend/app/routers/adventures/checkpoints.py`, + `backend/app/head.py`). They last until you delete them, and deleting one + deletes no story. - **Import and export.** AI Dungeon-compatible scenario format; JSON for everything else. An adventure exports as `ai-dnd-adventure-v2`, which carries the whole tree: - every branch, every take, the fork points, which branches the story has left behind, and the - position it is being read at — all of them chosen rather than computed, which is the rule for - what a bundle carries. A campaign exported after two Undos imports still undone, with its + every branch, every take, the fork points, which branches the story has left behind, the Save + Points and the position it is being read at — all of them chosen rather than computed, which is + the rule for what a bundle carries. A campaign opens where its head says, never at a Save Point + merely because it has one. A campaign exported after two Undos imports still undone, with its retained future intact, instead of silently reopening at its newest turn. Files that predate the head position, and files saved in the old single-line format, still import. - **Single user, no accounts.** There is no sign-up, no login, no session and no API key @@ -196,6 +206,7 @@ frontend/ React + Vite SPA ──HTTP/SSE──► backend/ FastAPI ├─ tlstrust.py one TLS context: the OS trust store unioned with certifi's ├─ tree.py forking, promotion, and where a node is placed ├─ head.py the active head: where the story is read, and what moving it costs + ├─ checkpoints Save Points: durable names for positions, in routers/adventures/ ├─ attempts.py the takes of one turn, grouped by parent ├─ context/ prompt assembly under a token budget + lineage/history windowing ├─ worldstate/ the stat engine: clamps, cooldowns, bands, milestones @@ -210,7 +221,7 @@ development, Vite proxies `/api` to FastAPI. ## Tests -638 backend tests: unit tests plus full HTTP integration through the real turn engine, with +680 backend tests: unit tests plus full HTTP integration through the real turn engine, with the model provider mocked. They run with no route to the Internet, which is a requirement rather than a convenience — an offline claim proved on a machine that has been online once proves nothing. diff --git a/backend/app/bundle.py b/backend/app/bundle.py index bf97c70..3c1ea87 100644 --- a/backend/app/bundle.py +++ b/backend/app/bundle.py @@ -134,6 +134,16 @@ def export(db: Session, adventure: models.Adventure) -> dict: "memoryCursor": _exported_anchor(adventure, cursors.MEMORY, local), "summaryCursor": _exported_anchor(adventure, cursors.SUMMARY, local), "memories": [_exported_memory(m, local) for m in adventure.memories], + # M4. A Save Point is a decision — someone chose this position and gave + # it a name — so it goes in the file by the rule at the top of this + # module. Nothing here is derived: the coordinate is the one stored, not + # one recomputed from the rows, because the whole point of the pointer + # is that no amount of reading the turns can tell you which one somebody + # named. A bundle written before M4 has no key here and imports with no + # Save Points, which is what such a campaign had. + "checkpoints": [ + _exported_checkpoint(c, local) for c in _checkpoints_of(db, adventure) + ], "storyCards": [ {"type": c.type, "name": c.name, "keys": c.keys, "entry": c.entry, "notes": c.notes} @@ -215,6 +225,35 @@ def _exported_memory(memory: models.Memory, local: dict[int, int]) -> dict: } +def _checkpoints_of(db: Session, adventure: models.Adventure) -> list[models.Checkpoint]: + """Returns the campaign's Save Points in creation order. + + Read with a query rather than through a relationship, for the reason + `models.Branch` declares none: a relationship on `Adventure` would be loaded + by anything that touches an adventure, and the export is the only thing in + the application that wants every Save Point at once. + """ + return ( + db.query(models.Checkpoint) + .filter(models.Checkpoint.adventure_id == adventure.id) + .order_by(models.Checkpoint.id) + .all() + ) + + +def _exported_checkpoint(checkpoint: models.Checkpoint, local: dict[int, int]) -> dict: + return { + "name": checkpoint.name, + "note": checkpoint.note, + # The branch as a position in this file's list, like every other branch + # reference in the bundle. The depth is a coordinate along it and needs + # no translation. + "branch": _local(checkpoint.branch_id, local), + "depth": checkpoint.depth, + "createdAt": checkpoint.created_at.isoformat() if checkpoint.created_at else None, + } + + def _imported_persona(persona) -> dict: """Reads a bundle's `persona` block into `Adventure` keyword arguments. @@ -280,6 +319,13 @@ def plan(bundle: dict, version: str) -> dict: "branches": branches, "nodes": nodes, "memories": _planned_memories(bundle, len(branches)), + # M4. Empty for a version 1 bundle and for any version 2 bundle written + # before Save Points existed, which is the same answer: no one had named + # a position in those campaigns. + "checkpoints": ( + _planned_checkpoints(bundle, len(branches), nodes) + if version == FORMAT else [] + ), "head": head, # None means the file does not say, which is every version 1 bundle and # every version 2 bundle written before M3. `_point_the_head` derives it @@ -525,6 +571,57 @@ def _planned_memories(bundle: dict, branches: int) -> list[dict]: return out +def _planned_checkpoints( + bundle: dict, branches: int, nodes: list[dict] +) -> list[dict]: + """Returns the file's Save Points, checked against the tree it also carries. + + A Save Point whose coordinate names no turn in the file is dropped rather + than imported, and dropped rather than raising. The two halves of that are + each deliberate: + + * Dropped, because an imported pointer to a position the imported story does + not contain is a Save Point that can only ever refuse to restore. It would + be a row that exists to disappoint. + * Not a 400, unlike the head depth. The head is a position the campaign is + read at, so a file that misplaces it opens the story in the wrong place + and every read is affected. A Save Point is a bookmark, and a bad one + spoils nothing else in the file — refusing to import a whole campaign + because one bookmark is wrong would lose the story to save the bookmark. + + A name that is blank once trimmed is dropped for the same reason the create + endpoint refuses one: an unnamed Save Point is not identifiable in a list. + """ + # Every coordinate the file writes, not only the ones it marks live. + # `_write_nodes` makes exactly one attempt at each coordinate live whatever + # the file says, so a coordinate that exists is a coordinate that will + # resolve — and reading the flags here would drop a Save Point over a + # question the writer has already settled. + written = {(n["branch"], n["depth"]) for n in nodes} + raw = bundle.get("checkpoints") + out: list[dict] = [] + for entry in raw if isinstance(raw, list) else []: + if not isinstance(entry, dict): + continue + name = str(entry.get("name") or "").strip()[:schemas.CHECKPOINT_NAME_MAX] + if not name: + continue + depth = entry.get("depth") + if not _is_int(depth): + continue + branch = _as_index(entry.get("branch"), branches, default=None) + if branch is None or (branch, depth) not in written: + continue + out.append({ + "name": name, + "note": str(entry.get("note") or ""), + "branch": branch, + "depth": depth, + "createdAt": _as_time(entry.get("createdAt")), + }) + return out + + def _planned_anchors(bundle: dict, branches: int) -> dict: anchors = {} for name in ("memory", "summary"): @@ -550,6 +647,7 @@ def write(db: Session, adventure: models.Adventure, story: dict) -> None: _write_nodes(db, adventure, story["nodes"], ids) _write_memories(db, adventure, story["memories"], ids) _point_the_head(adventure, story, ids) + _write_checkpoints(db, adventure, story["checkpoints"], ids) _write_anchors(adventure, story, ids) @@ -665,6 +763,32 @@ def _write_memories( db.add(memory) +def _write_checkpoints( + db: Session, adventure: models.Adventure, specs: list[dict], ids: list[int] +) -> None: + """Writes the Save Points, and moves nothing. + + Note what this function does not touch. The head is pointed by + `_point_the_head` from the file's own `headBranch`/`headDepth`, and importing + a Save Point must not disturb it — a campaign exported at turn 30 with a + Save Point at turn 12 opens at turn 30. The bundle records where the story + was being read and, separately, which positions someone named; restoring one + of them is a thing the user does afterwards, not a thing an import does for + them. + """ + for spec in specs: + checkpoint = models.Checkpoint( + adventure_id=adventure.id, + name=spec["name"], + note=spec["note"], + branch_id=ids[spec["branch"]], + depth=spec["depth"], + ) + if spec["createdAt"] is not None: + checkpoint.created_at = spec["createdAt"] + db.add(checkpoint) + + def _point_the_head( adventure: models.Adventure, story: dict, ids: list[int] ) -> None: diff --git a/backend/app/head.py b/backend/app/head.py index 796826e..889623e 100644 --- a/backend/app/head.py +++ b/backend/app/head.py @@ -314,6 +314,40 @@ def move_to(db: Session, adventure: models.Adventure, depth: int) -> None: attempts.restore_state(adventure, node_at(db, adventure, depth)) +def move_to_node(db: Session, adventure: models.Adventure, node: models.Action) -> bool: + """Moves the head onto `node`, changing line only if it is not on this one. + + M4 restores a Save Point through this, and it adds no restoring of its own: + the depth half is `move_to` unchanged, so the state, the transcript, the + assembled context and memory eligibility all arrive exactly as they do for + Undo and Redo. Returns whether the line had to change as well as the depth, + which is the one thing about a restore a caller cannot work out afterwards. + + The head is two values, and the two halves move for different reasons. A + Save Point almost always names a position on the story being read — its own + line, or the shared prefix that line inherits — and then only the depth + moves. Leaving the branch alone is what makes the restored position keep the + continuation it has: after a divergence, restoring to the shared prefix must + put the reader back on the *new* line, where Redo walks into the turns they + are still writing, not into the future they left. Reaching for the Save + Point's own branch there would quietly hand back the abandoned story. + + The other case is real and has to work. A Save Point survives divergence + (`STORY-BRANCH-SEMANTICS.md` §19), so one can name a position on a line the + story has since left, and no amount of depth movement reaches a branch this + path does not contain. The line then moves as well — one assignment, the + same one `switch_branch` makes — and the depth still moves through + `move_to`. Nothing is created: a restore never forks, whichever case it + takes. The first write below the restored head does, through + `fork_if_behind_head`, like every other write. + """ + switched = not lineage.path_of(db, adventure).uncapped().contains(node) + if switched: + adventure.head_branch_id = node.branch_id + move_to(db, adventure, node.depth) + return switched + + def fork_if_behind_head(db: Session, adventure: models.Adventure) -> bool: """Gives the story a new branch when a write would displace a retained future. diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 7e098db..9cc897c 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -350,6 +350,17 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ # No backfill. (78, "ALTER TABLE branches ADD COLUMN superseded_at TIMESTAMP"), (79, "ALTER TABLE branches ADD COLUMN superseded_depth INTEGER"), + # M4: Save Points. `create_all` creates the `checkpoints` table itself, on + # existing databases as well as fresh ones, exactly as it did for + # `memories` at version 2 and `branches` at version 46. What it does not + # create is the index every list and every cascade reads, so that is what + # this version is. + # + # No backfill. A Save Point records a decision someone made, and nobody has + # made one yet: an M3 database has no position a user chose to name, and + # inventing one would be inventing the decision. + (80, "CREATE INDEX IF NOT EXISTS ix_checkpoints_adventure " + "ON checkpoints (adventure_id)"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) diff --git a/backend/app/models.py b/backend/app/models.py index e9a7978..e607b27 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -241,6 +241,60 @@ class Branch(Base): superseded_depth: Mapped[int | None] = mapped_column(Integer, nullable=True) +class Checkpoint(Base): + """M4: a Save Point — a durable named pointer to a story position. + + "Save Point" is what the user reads; `checkpoint` is what the code calls it + (`BROWSER-UX-SPEC.md` §23). + + The row holds a name and a coordinate, and no story. `DATA-MODEL.md` §8 + describes the pointer as naming a turn; the coordinate here is + `(branch_id, depth)`, which is what M3 made the head and what + `head.node_at` resolves. Restoring one is therefore head movement with a + bounds check rather than a restore system of its own — see ADR 012 and + `head.move_to_node`. + + A coordinate rather than an action id, deliberately. One coordinate can + hold several attempts at a turn and exactly one of them is live, so a + retry replaces the row a Save Point would have pinned. "Turn 42 of this + line" survives a retry; "action 918" would point at a take the story no + longer tells. + + `branch_id` is the branch the node itself sits on, not the branch that was + being read when the Save Point was made. Those differ whenever the head is + resting in a shared prefix, and the node's own branch is the one that still + names the position after the reader has moved elsewhere. + + Deleting a branch deletes its Save Points, by the same cascade that takes + its memories: the story the pointer names is gone with it. Nothing else + removes one. They are not cleaned up for going stale, for being behind the + head, or for pointing into a future the story has left + (`STORY-BRANCH-SEMANTICS.md` §19). + """ + + __tablename__ = "checkpoints" + + id: Mapped[int] = mapped_column(primary_key=True) + adventure_id: Mapped[int] = mapped_column( + ForeignKey("adventures.id", ondelete="CASCADE") + ) + name: Mapped[str] = mapped_column(String(120), default="") + # `DATA-MODEL.md` §8's optional notes, and `BROWSER-UX-SPEC.md` §24's + # optional second field. Empty is the ordinary case. + note: Mapped[str] = mapped_column(Text, default="") + branch_id: Mapped[int] = mapped_column( + ForeignKey("branches.id", ondelete="CASCADE") + ) + depth: Mapped[int] = mapped_column(Integer) + created_at: Mapped[datetime] = mapped_column(DateTime, default=utcnow) + # Bumped by a rename, which is the only edit a Save Point allows. The + # coordinate is never rewritten: `STORY-BRANCH-SEMANTICS.md` §24 keeps a + # Save Point's meaning auditable by making "move it" delete-and-recreate. + updated_at: Mapped[datetime] = mapped_column( + DateTime, default=utcnow, onupdate=utcnow + ) + + class Memory(Base): """Phase 6: an auto-summarized (or hand-written) fact about the adventure. diff --git a/backend/app/routers/adventures/__init__.py b/backend/app/routers/adventures/__init__.py index 8941e18..546fb17 100644 --- a/backend/app/routers/adventures/__init__.py +++ b/backend/app/routers/adventures/__init__.py @@ -13,6 +13,7 @@ Read the modules in this order to follow a turn from end to end: turns playing a turn, and the lock that allows only one at a time takes retries and the attempts that collect at one coordinate branches where a story splits + checkpoints Save Points: durable names for positions the head can return to What this package re-exports, and what it deliberately does not: @@ -31,6 +32,7 @@ from . import ( # noqa: F401 turns, takes, branches, + checkpoints, bundle_io, refresh, insights, diff --git a/backend/app/routers/adventures/checkpoints.py b/backend/app/routers/adventures/checkpoints.py new file mode 100644 index 0000000..c9b99f9 --- /dev/null +++ b/backend/app/routers/adventures/checkpoints.py @@ -0,0 +1,258 @@ +"""M4: Save Points — create, list, rename, delete, and restore. + +A Save Point is a durable named pointer to a story position and nothing else. +It stores a coordinate, never a copy of any story, and restoring one moves the +active head to that coordinate. That is the whole design, and it is what +`BUILD-MILESTONES.md`'s note on M4 and ADR 012 ask for: M3 made the head a +stored `(branch, depth)` and made arriving at one a row lookup plus a state +restore, so a Save Point needs no restore machinery of its own. + +What is deliberately absent from this module, because a second copy of any of it +would be the failure M4 is warned about: + +* no head fields are assigned here — `head.move_to_node` moves the head, and + `head.move_to` under it restores the state, exactly as Undo and Redo do; +* nothing reconstructs state, prunes a memory, copies a turn, or deletes one; +* nothing forks. Restore is not a decision to abandon anything, so it creates no + branch. The first write below the restored head forks, through the same + `fork_if_behind_head` every other write goes through, and the displaced future + stays retained (`STORY-BRANCH-SEMANTICS.md` §20). + +The user-facing word is "Save Point" and the internal one is `checkpoint` +(`BROWSER-UX-SPEC.md` §23). Error strings here are read by a player, so they say +Save Point. +""" + +from fastapi import Depends, HTTPException +from sqlalchemy.orm import Session + +from ... import head, models, schemas +from ...context import lineage +from ...database import get_db + +from . import turns +from .deps import current_adventure, router +from .paging import current_window + + +def _node_at( + db: Session, adventure: models.Adventure, branch_id: int, depth: int +) -> models.Action | None: + """Returns the live turn a Save Point's coordinate names, or None. + + The lookup is by coordinate and is not scoped to any path. That is the + point of it: a Save Point outlives the reader moving away, so the question + it has to answer is "is this position still in this campaign's retained + history", not "is it on the story being read now". Whether it is on the + current path is a separate question, and `head.move_to_node` is what acts on + the answer. + + `live` is what makes the coordinate follow a retry. One coordinate can hold + several attempts at a turn, and a Save Point names the turn rather than the + attempt, so it lands on whichever take the story currently tells. + """ + return ( + db.query(models.Action) + .filter( + models.Action.adventure_id == adventure.id, + models.Action.branch_id == branch_id, + models.Action.depth == depth, + models.Action.live.is_(True), + ) + .order_by(models.Action.id) + .first() + ) + + +def _rendered( + db: Session, adventure: models.Adventure, checkpoint: models.Checkpoint +) -> schemas.CheckpointOut: + """Reads one Save Point out with the three facts the panel needs about it.""" + node = _node_at(db, adventure, checkpoint.branch_id, checkpoint.depth) + out = schemas.CheckpointOut.model_validate(checkpoint) + # The same `depth + 1` the branch list counts with, so "turn 42" means the + # same thing in both places. + out.turn = checkpoint.depth + 1 + out.resolved = node is not None + out.on_path = node is not None and lineage.path_of(db, adventure).uncapped().contains(node) + return out + + +def _get_or_404( + db: Session, adventure: models.Adventure, checkpoint_id: int +) -> models.Checkpoint: + """Resolves a Save Point id, refusing one that belongs to another campaign. + + The ownership check is the reason this is a function rather than a `db.get` + at each call site. A Save Point names a position in one campaign's history, + and a coordinate from another campaign would name a different story's turn — + or, worse, resolve against this one by arithmetic coincidence. So the id is + matched against this adventure, and a Save Point belonging to another is a + 404 rather than a restore of the wrong story. + """ + checkpoint = db.get(models.Checkpoint, checkpoint_id) + if checkpoint is None or checkpoint.adventure_id != adventure.id: + raise HTTPException(404, "Save Point not found") + return checkpoint + + +def _clean_name(raw: str) -> str: + """Returns the trimmed name, refusing one that is blank once trimmed.""" + name = (raw or "").strip() + if not name: + raise HTTPException(400, "A Save Point needs a name.") + return name + + +@router.get("/{adventure_id}/checkpoints", response_model=list[schemas.CheckpointOut]) +def list_checkpoints( + db: Session = Depends(get_db), + adventure: models.Adventure = Depends(current_adventure), +): + """Returns the campaign's Save Points, newest first. + + Newest first rather than in story order, because story order is not + something this list can honestly claim. Depths are positions along a path, + and two Save Points on lines that parted company are not comparable by depth + at all — ordering by it would draw a sequence that no reading of the story + passes through. When they were made is a fact about all of them. + """ + rows = ( + db.query(models.Checkpoint) + .filter(models.Checkpoint.adventure_id == adventure.id) + .order_by(models.Checkpoint.created_at.desc(), models.Checkpoint.id.desc()) + .all() + ) + return [_rendered(db, adventure, row) for row in rows] + + +@router.post( + "/{adventure_id}/checkpoints", + response_model=schemas.CheckpointOut, + status_code=201, +) +def create_checkpoint( + payload: schemas.CheckpointCreate, + db: Session = Depends(get_db), + adventure: models.Adventure = Depends(current_adventure), +): + """Names the position the story is currently being read at. + + The active head, not the retained tip. Creating a Save Point after two Undos + saves the undone position, because that is where the reader is and the + position they are looking at is the one they mean. The distinction only + exists at all because M3 stopped Undo from deleting. + + The node at the head is resolved before the row is written, and its own + branch is what gets stored — which is not always the branch being read. A + head resting in a shared prefix sits on an ancestor's node, and the + ancestor is the branch that still names that position after the reader has + forked away from it. + """ + name = _clean_name(payload.name) + node = head.node_at(db, adventure, adventure.head_depth) + if node is None: + raise HTTPException(400, "There is no turn here to save yet.") + checkpoint = models.Checkpoint( + adventure_id=adventure.id, + name=name, + note=payload.note or "", + branch_id=node.branch_id, + depth=node.depth, + ) + db.add(checkpoint) + db.commit() + db.refresh(checkpoint) + return _rendered(db, adventure, checkpoint) + + +@router.patch( + "/{adventure_id}/checkpoints/{checkpoint_id}", + response_model=schemas.CheckpointOut, +) +def rename_checkpoint( + checkpoint_id: int, + payload: schemas.CheckpointRename, + db: Session = Depends(get_db), + adventure: models.Adventure = Depends(current_adventure), +): + """Changes a Save Point's label. Nothing else about it moves. + + Not the coordinate, not the head, not a row of story. A Save Point that has + been renamed restores to exactly the position it did before, which is + `STORY-BRANCH-SEMANTICS.md` §23. + """ + checkpoint = _get_or_404(db, adventure, checkpoint_id) + if payload.name is not None: + checkpoint.name = _clean_name(payload.name) + if payload.note is not None: + checkpoint.note = payload.note + db.commit() + db.refresh(checkpoint) + return _rendered(db, adventure, checkpoint) + + +@router.delete("/{adventure_id}/checkpoints/{checkpoint_id}", status_code=204) +def delete_checkpoint( + checkpoint_id: int, + db: Session = Depends(get_db), + adventure: models.Adventure = Depends(current_adventure), +): + """Removes the named pointer, and only the pointer. + + The turn it named stays, its branch stays, the future past it stays, and the + head does not move. This endpoint deletes one row of the `checkpoints` + table. `STORY-BRANCH-SEMANTICS.md` §25. + """ + checkpoint = _get_or_404(db, adventure, checkpoint_id) + db.delete(checkpoint) + db.commit() + return None + + +@router.post( + "/{adventure_id}/checkpoints/{checkpoint_id}/restore", + response_model=schemas.ActionPage, +) +def restore_checkpoint( + adventure_id: int, + checkpoint_id: int, + db: Session = Depends(get_db), + adventure: models.Adventure = Depends(current_adventure), +): + """Returns the story to a Save Point, deleting nothing. + + Four steps, and the last one is not this module's code: resolve the + coordinate, refuse it if it no longer names a live turn, hand it to + `head.move_to_node`, and answer with the window the head now caps. The + transcript, the world state, the assembled context and which memories can be + retrieved all move together, because all four already read through the one + path object the head caps — the same reason Undo needed no memory pruning. + + The turns past the restored position are retained, exactly as they are after + an Undo, and ordinary Redo can still walk forward into them until the user + writes something different. Restore does not fork; the first write below the + head does. + + A coordinate that no longer resolves is refused rather than approximated. + Moving the head to the nearest surviving turn would be the one outcome worse + than doing nothing: a Save Point that silently means somewhere else. + """ + checkpoint = _get_or_404(db, adventure, checkpoint_id) + turns.acquire_turn_lock(adventure_id) + try: + node = _node_at(db, adventure, checkpoint.branch_id, checkpoint.depth) + if node is None: + raise HTTPException( + 409, + "That Save Point's position is no longer part of this story.", + ) + head.move_to_node(db, adventure, node) + adventure.updated_at = models.utcnow() + db.commit() + db.refresh(adventure) + # A window, not the whole story, for the reason Undo gives: the client + # replaces its transcript with this, and the transcript is a window. + return current_window(db, adventure) + finally: + turns._active_turns.discard(adventure_id) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index fa07084..c5df8ee 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -25,6 +25,10 @@ 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). +# M4: what a player called a Save Point. VARCHAR(120). Wider than a branch name +# because these are sentences rather than labels — "Before entering the abbey" +# is the example the specification uses throughout. +CHECKPOINT_NAME_MAX = 120 Name = Annotated[str, Field(max_length=NAME_MAX)] Tags = Annotated[str, Field(max_length=TAGS_MAX)] @@ -35,6 +39,7 @@ 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)] +CheckpointName = Annotated[str, Field(max_length=CHECKPOINT_NAME_MAX)] class ORMModel(BaseModel): @@ -267,6 +272,67 @@ class BranchRename(BaseModel): name: Annotated[str, Field(max_length=BRANCH_NAME_MAX)] | None = None +# ---------- Save Points (M4) ---------- +# +# "Save Point" is the user-facing term and `checkpoint` is the internal one +# (`BROWSER-UX-SPEC.md` §23). The wire format uses the internal name, as the +# rest of this module does. + + +class CheckpointOut(ORMModel): + """One Save Point: a name and the position it names. + + The position is reported three ways because the panel needs three different + things from it. `turn` is what a reader counts — the same `depth + 1` the + branch list shows. `depth` and `branch_id` are the coordinate itself. + `on_path` says whether the position lies on the story being read, which is + how the panel can tell a Save Point on this line from one naming a line the + story has left; restoring either works, but they are not the same offer. + + `resolved` is false when the coordinate no longer names a live turn, which + an action deleted out of the middle of a story can do. Restore refuses such + a Save Point rather than moving the head somewhere approximate, so the list + says so before the button is pressed. + """ + + id: int + adventure_id: int + name: str + note: str = "" + branch_id: int + depth: int + turn: int = 0 + on_path: bool = True + resolved: bool = True + created_at: datetime + updated_at: datetime + + +class CheckpointCreate(BaseModel): + """A Save Point at wherever the story is being read. + + The position is not a field. A Save Point is made at the campaign's active + head, which the server already knows, and accepting a coordinate from the + client would be the second way to name a position — the thing this milestone + exists not to build. + """ + + name: CheckpointName + note: Prose = "" + + +class CheckpointRename(BaseModel): + """A new label, and nothing else. + + There is deliberately no coordinate here. `STORY-BRANCH-SEMANTICS.md` §24 + keeps a Save Point's meaning auditable by refusing to move one: rename it, + or delete it and make another where you are. + """ + + name: CheckpointName | None = None + note: Prose | None = None + + class ActionUpdate(BaseModel): text: ActionText diff --git a/backend/tests/test_save_points.py b/backend/tests/test_save_points.py new file mode 100644 index 0000000..0ba190a --- /dev/null +++ b/backend/tests/test_save_points.py @@ -0,0 +1,1131 @@ +"""M4: Save Points, and the promise that restoring one deletes nothing. + +This file is the acceptance contract for the milestone that gave a story +position a durable name. Its subject is one claim: + + A Save Point is a name for a coordinate, and restoring it is head movement. + +Everything else follows. Restore keeps later history because head movement +deletes nothing (M3). Redo still walks forward afterwards because the retained +lineage is unchanged. Divergence after a restore forks through the same check +every write goes through, and the Save Point still names the same position when +it is over. Memory and context narrow and widen with the head, because the head +caps the one path all four reads share. + +The tests are named for the acceptance items they discharge — D11-D14, E01-E04, +I04 and L03 in `planning/V1-ACCEPTANCE-TESTS.md`. + +The world state is instrumentation here, not the subject, exactly as it is in +`test_head_cursor.py`: each scripted reply banks ten gold so that "the state at +this position" is a number a test can assert rather than a paragraph it has to +interpret. M5 replaces that machinery with genre-neutral narrative state, and +these tests then need the instrumentation moved, not the assertions removed. + + python -m pytest tests/test_save_points.py -v +""" +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient + +from app import limits, models +from app.context import lineage +from app.database import Base, SessionLocal, engine, get_db +from app.main import app +from app import auth +from app.routers import adventures + +from fakes import GOLD_PER_TURN, GOLD_SCHEMA, ScriptedProvider, gold_replies + + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + setup = SessionLocal() + user = models.User(is_guest=False, email="save@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings(user_id=user.id, api_key="enc:dummy", model="test-model")) + scenario = models.Scenario(user_id=user.id, title="S", stat_schema=GOLD_SCHEMA) + setup.add(scenario) + setup.flush() + adv = models.Adventure( + user_id=user.id, title="Abbey", scenario_id=scenario.id, + world_state={"player": {"hp": 100, "gold": 0}}, + ) + setup.add(adv) + setup.flush() + setup.add(models.Action(adventure_id=adv.id, type="start", text="The road forks.")) + setup.commit() + adv_id, user_id = adv.id, user.id + setup.close() + + ScriptedProvider.replies = gold_replies() + monkeypatch.setattr(adventures.turns, "OpenAICompatibleProvider", ScriptedProvider) + 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) + c.adv_id = adv_id + try: + yield c + finally: + app.dependency_overrides.clear() + adventures.turns._active_turns.clear() + Base.metadata.drop_all(bind=engine) + + +# ------------------------------------------------------------------ helpers + +def _play(client, text="look around", type="do", adv_id=None): + r = client.post( + f"/api/adventures/{adv_id or client.adv_id}/actions", + json={"type": type, "text": text}, + ) + assert r.status_code == 200, r.text + return r + + +def _turns(client, count): + for n in range(count): + _play(client, f"turn {n}") + + +def _undo(client): + return client.post(f"/api/adventures/{client.adv_id}/undo") + + +def _redo(client): + return client.post(f"/api/adventures/{client.adv_id}/redo") + + +def _save(client, name="Before entering the abbey", note=None, adv_id=None): + """Creates a Save Point at wherever the story is being read.""" + payload = {"name": name} + if note is not None: + payload["note"] = note + r = client.post( + f"/api/adventures/{adv_id or client.adv_id}/checkpoints", json=payload + ) + assert r.status_code == 201, r.text + return r.json() + + +def _list(client, adv_id=None): + r = client.get(f"/api/adventures/{adv_id or client.adv_id}/checkpoints") + assert r.status_code == 200, r.text + return r.json() + + +def _restore(client, checkpoint_id, adv_id=None): + return client.post( + f"/api/adventures/{adv_id or client.adv_id}/checkpoints/{checkpoint_id}/restore" + ) + + +def _adventure(client, adv_id=None) -> dict: + r = client.get(f"/api/adventures/{adv_id or client.adv_id}") + assert r.status_code == 200, r.text + return r.json() + + +def _texts(client, adv_id=None) -> list[str]: + return [a["text"] for a in _adventure(client, adv_id)["actions"]] + + +def _rows(adv_id) -> list[models.Action]: + """Every action row, story or not, live or not, head or no head.""" + db = SessionLocal() + try: + return ( + db.query(models.Action) + .filter(models.Action.adventure_id == adv_id) + .order_by(models.Action.branch_id, models.Action.depth, models.Action.id) + .all() + ) + finally: + db.close() + + +def _gold(adv_id) -> int: + db = SessionLocal() + try: + adv = db.get(models.Adventure, adv_id) + return (adv.world_state or {}).get("player", {}).get("gold", 0) + finally: + db.close() + + +def _head(adv_id) -> tuple[int, int]: + db = SessionLocal() + try: + adv = db.get(models.Adventure, adv_id) + return adv.head_branch_id, adv.head_depth + finally: + db.close() + + +def _restart(client): + """Simulates stopping and restarting the application. + + A Save Point is a row, so what a restart has to prove is that nothing about + it lived in the process. Every session this file opens is closed again, so + dropping the client's own session and reading through a new one is what a + restart changes: no cached adventure, no cached head, no in-memory list. + """ + client.close() + adventures.turns._active_turns.clear() + fresh = TestClient(app) + fresh.adv_id = client.adv_id + return fresh + + +def _export(client, adv_id=None) -> dict: + r = client.get(f"/api/adventures/{adv_id or client.adv_id}/export") + assert r.status_code == 200, r.text + return r.json() + + +def _import(client, bundle) -> dict: + r = client.post("/api/adventures/import", json=bundle) + assert r.status_code == 201, r.text + return r.json() + + +def _story_of(adv_id) -> list[str]: + """The story an adventure tells, read through its own head.""" + db = SessionLocal() + try: + adventure = db.get(models.Adventure, adv_id) + rows = ( + db.query(models.Action) + .filter( + models.Action.adventure_id == adv_id, + lineage.path_of(db, adventure).clause(models.Action), + ) + .order_by(models.Action.depth, models.Action.id) + .all() + ) + return [a.text for a in rows] + finally: + db.close() + + +# --------------------------------------------------------- D11: named, durable + +def test_d11_a_named_save_point_survives_a_restart(client): + """The acceptance test names the Save Point, so this one does too.""" + _turns(client, 3) + made = _save(client, "Before entering the abbey") + assert made["name"] == "Before entering the abbey" + + after = _restart(client) + try: + kept = _list(after) + assert [c["name"] for c in kept] == ["Before entering the abbey"] + assert kept[0]["id"] == made["id"] + assert (kept[0]["branch_id"], kept[0]["depth"]) == ( + made["branch_id"], made["depth"] + ) + finally: + after.close() + + +def test_a_save_point_records_the_position_the_story_is_read_at(client): + """The active head, not the retained tip. A Save Point made after an Undo + names the undone position, because that is the turn the reader is looking + at — a distinction that only exists because M3 stopped Undo deleting.""" + _turns(client, 5) + _undo(client) + _undo(client) + branch, depth = _head(client.adv_id) + + made = _save(client, "Two turns back") + + assert (made["branch_id"], made["depth"]) == (branch, depth) + assert made["turn"] == depth + 1 + + +def test_a_save_point_needs_a_name(client): + _turns(client, 1) + for blank in ("", " ", "\n\t "): + r = client.post( + f"/api/adventures/{client.adv_id}/checkpoints", json={"name": blank} + ) + assert r.status_code == 400, r.text + assert "needs a name" in r.json()["detail"] + assert _list(client) == [] + + +def test_a_name_is_stored_trimmed(client): + _turns(client, 1) + made = _save(client, " Before entering the abbey ") + assert made["name"] == "Before entering the abbey" + + +def test_an_empty_story_has_no_position_to_save(client): + """The opening node is the campaign's first position, so this refuses only + the case where the head rests in front of every turn.""" + _undo(client) # nothing to undo; the head is already on the opening + db = SessionLocal() + try: + adv = db.get(models.Adventure, client.adv_id) + adv.head_depth = lineage.NO_DEPTH + db.commit() + finally: + db.close() + + r = client.post( + f"/api/adventures/{client.adv_id}/checkpoints", json={"name": "Nowhere"} + ) + assert r.status_code == 400 + assert "no turn here to save" in r.json()["detail"] + + +# ------------------------------------------------------------ D12: restoring + +def test_d12_restore_returns_the_transcript_and_the_state(client): + _turns(client, 2) + told = _texts(client) + banked = _gold(client.adv_id) + made = _save(client, "Before entering the abbey") + + _turns(client, 3) + assert _gold(client.adv_id) == banked + 3 * GOLD_PER_TURN + + page = _restore(client, made["id"]) + assert page.status_code == 200, page.text + + assert [a["text"] for a in page.json()["actions"]] == told + assert _texts(client) == told + assert _gold(client.adv_id) == banked + + +def test_restore_is_the_same_head_movement_undo_makes(client): + """The mechanism claim, measured rather than asserted in prose. Restoring to + a position and undoing to it must leave the campaign in the same state, both + halves of the head included.""" + _turns(client, 4) + made = _save(client, "Here") + _turns(client, 2) + + _restore(client, made["id"]) + by_restore = (_head(client.adv_id), _gold(client.adv_id), _texts(client)) + + _redo(client) + _redo(client) + _undo(client) + _undo(client) + by_undo = (_head(client.adv_id), _gold(client.adv_id), _texts(client)) + + assert by_restore == by_undo + + +def test_restoring_the_same_save_point_repeatedly_is_stable(client): + _turns(client, 3) + made = _save(client, "Here") + _turns(client, 2) + + seen = [] + for _ in range(3): + assert _restore(client, made["id"]).status_code == 200 + seen.append((_head(client.adv_id), _gold(client.adv_id), _texts(client))) + assert seen[0] == seen[1] == seen[2] + + +def test_a_save_point_at_the_current_tip_restores_to_a_story_that_never_moved(client): + _turns(client, 3) + made = _save(client, "Right here") + before = (_head(client.adv_id), _gold(client.adv_id), _texts(client)) + + assert _restore(client, made["id"]).status_code == 200 + + assert (_head(client.adv_id), _gold(client.adv_id), _texts(client)) == before + + +def test_a_save_point_at_the_campaign_opening_restores_to_the_opening(client): + """The floor Undo stops at is a position like any other, and naming it must + not need a special case.""" + _turns(client, 3) + for _ in range(3): + _undo(client) + assert _texts(client) == ["The road forks."] + made = _save(client, "The very beginning") + + _redo(client) + _redo(client) + assert len(_texts(client)) == 5 + + assert _restore(client, made["id"]).status_code == 200 + assert _texts(client) == ["The road forks."] + assert _gold(client.adv_id) == 0 + + +def test_restore_after_undo_and_redo_activity_lands_where_the_name_says(client): + _turns(client, 5) + _undo(client) + _undo(client) + made = _save(client, "Amid the undoing") + at_save = (_head(client.adv_id), _gold(client.adv_id), _texts(client)) + + _redo(client) + _undo(client) + _undo(client) + _redo(client) + _redo(client) + + assert _restore(client, made["id"]).status_code == 200 + assert (_head(client.adv_id), _gold(client.adv_id), _texts(client)) == at_save + + +def test_several_save_points_at_different_positions_each_restore_to_their_own(client): + marks = [] + for n in range(4): + _play(client, f"turn {n}") + marks.append((_save(client, f"After turn {n}"), _texts(client), _gold(client.adv_id))) + + for mark, told, banked in reversed(marks): + assert _restore(client, mark["id"]).status_code == 200 + assert _texts(client) == told + assert _gold(client.adv_id) == banked + + +def test_two_save_points_may_name_the_same_position(client): + """No uniqueness is imposed on names or on positions. Nothing in the product + requirements asks for it, and two names for one turn is a reasonable thing + for a player to want.""" + _turns(client, 2) + first = _save(client, "Before the abbey") + second = _save(client, "Where I keep dying") + + assert first["id"] != second["id"] + assert (first["branch_id"], first["depth"]) == (second["branch_id"], second["depth"]) + + _turns(client, 2) + for made in (first, second): + assert _restore(client, made["id"]).status_code == 200 + assert len(_texts(client)) == 5 + + +# ---------------------------------------- D13: restore does not delete history + +def test_d13_restore_deletes_no_accepted_history(client): + """The measurement, on row identity rather than on a count: every row that + existed before the restore is still there afterwards, and it is the same + row.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + _turns(client, 3) + before = {a.id for a in _rows(client.adv_id)} + + assert _restore(client, made["id"]).status_code == 200 + + after = {a.id for a in _rows(client.adv_id)} + assert after == before + assert len(after) == 11 # the opening, plus two rows for each of five turns + + +def test_d13_the_retained_continuation_can_still_be_redone(client): + """Restore is not a decision to abandon anything, so the future it steps + behind is still the continuation this story tells. `STORY-BRANCH-SEMANTICS` + §20.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + _turns(client, 3) + whole = _texts(client) + tip_gold = _gold(client.adv_id) + + _restore(client, made["id"]) + assert _adventure(client)["can_redo"] is True + + for _ in range(3): + assert _redo(client).status_code == 200 + assert _texts(client) == whole + assert _gold(client.adv_id) == tip_gold + + +def test_d13_a_different_continuation_forks_and_keeps_the_old_future(client): + """The other half of D13. The fork happens on the first write below the + restored head, not because Restore was clicked.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + _turns(client, 3) + old_future = {a.id for a in _rows(client.adv_id)} + branches_before = _branch_count(client.adv_id) + + _restore(client, made["id"]) + # Restore itself created nothing. + assert _branch_count(client.adv_id) == branches_before + + ScriptedProvider.replies = ["Through the side door.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go around the back") + + # The write forked... + assert _branch_count(client.adv_id) == branches_before + 1 + # ...ordinary Redo no longer offers the displaced future... + assert _adventure(client)["can_redo"] is False + assert _redo(client).status_code == 400 + # ...and not one row of it was deleted to achieve that. + assert old_future <= {a.id for a in _rows(client.adv_id)} + assert _texts(client)[-1].startswith("Through the side door.") + + +def test_d13_the_save_point_still_names_the_same_position_after_divergence(client): + """`STORY-BRANCH-SEMANTICS.md` §19: a divergence does not disturb a Save + Point. It still points at the turn it was made on, and restoring it now + returns to the *new* line's reading of that position.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + told = _texts(client) + _turns(client, 3) + + _restore(client, made["id"]) + ScriptedProvider.replies = ["Through the side door.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go around the back") + + kept = _list(client) + assert len(kept) == 1 + assert (kept[0]["branch_id"], kept[0]["depth"]) == (made["branch_id"], made["depth"]) + + assert _restore(client, made["id"]).status_code == 200 + assert _texts(client) == told + # The new continuation is what Redo walks into now, not the displaced one. + assert _adventure(client)["can_redo"] is True + _redo(client) + assert _texts(client)[-1].startswith("Through the side door.") + + +def test_a_save_point_on_a_line_the_story_left_still_restores(client): + """The case that needs the head's other half to move. + + A Save Point survives divergence, so one can name a position on a future the + story has since displaced — and no amount of depth movement reaches a branch + the current path does not contain. Restoring it moves the line as well, the + same single assignment a branch switch makes, and still forks nothing. + """ + _turns(client, 2) + fork_point = _save(client, "The fork") + _turns(client, 3) + deep = _save(client, "Down the old road") + old_story = _texts(client) + old_gold = _gold(client.adv_id) + old_branch, old_depth = _head(client.adv_id) + + _restore(client, fork_point["id"]) + ScriptedProvider.replies = ["Through the side door.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go around the back") + new_branch, _ = _head(client.adv_id) + assert new_branch != old_branch + + # The displaced Save Point is not on the line being read... + listed = {c["id"]: c for c in _list(client)} + assert listed[deep["id"]]["on_path"] is False + assert listed[fork_point["id"]]["on_path"] is True + assert listed[deep["id"]]["resolved"] is True + + # ...and restoring it goes back to the line it names, with its own state. + assert _restore(client, deep["id"]).status_code == 200 + assert _head(client.adv_id) == (old_branch, old_depth) + assert _texts(client) == old_story + assert _gold(client.adv_id) == old_gold + + +def test_restore_onto_an_inherited_position_keeps_the_line_being_read(client): + """A Save Point in the shared prefix must not drag the reader back onto the + ancestor. The turn is the same row either way; which continuation follows it + is not, and the active line is the one the reader chose.""" + _turns(client, 4) + _undo(client) + _undo(client) + ScriptedProvider.replies = ["A new road.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "the other way") + new_branch, _ = _head(client.adv_id) + + # Made now, on the new line, but naming a turn that physically lives on the + # branch the story left. + _undo(client) + _undo(client) + _undo(client) + made = _save(client, "In the shared past") + assert made["branch_id"] != new_branch + + _redo(client) + _redo(client) + assert _restore(client, made["id"]).status_code == 200 + + # Still reading the new line, so Redo walks up the shared past and on into + # the new continuation — depth 6 holds "A new road." on this line and the + # displaced "Take 3." on the one the story left. + assert _head(client.adv_id)[0] == new_branch + for _ in range(3): + assert _adventure(client)["can_redo"] is True + assert _redo(client).status_code == 200 + assert _texts(client)[-1].startswith("A new road.") + + +def _branch_count(adv_id) -> int: + db = SessionLocal() + try: + return db.query(models.Branch).filter_by(adventure_id=adv_id).count() + finally: + db.close() + + +# ------------------------------------------------------------- D14: deleting + +def test_d14_delete_removes_the_pointer_and_no_story(client): + _turns(client, 3) + made = _save(client, "Before entering the abbey") + _turns(client, 2) + rows = {a.id for a in _rows(client.adv_id)} + told = _texts(client) + head_before = _head(client.adv_id) + + r = client.delete(f"/api/adventures/{client.adv_id}/checkpoints/{made['id']}") + assert r.status_code == 204, r.text + + assert _list(client) == [] + assert {a.id for a in _rows(client.adv_id)} == rows + assert _texts(client) == told + assert _head(client.adv_id) == head_before + assert _branch_count(client.adv_id) >= 1 + + +def test_deleting_one_save_point_leaves_the_others(client): + _turns(client, 1) + first = _save(client, "One") + _turns(client, 1) + second = _save(client, "Two") + + client.delete(f"/api/adventures/{client.adv_id}/checkpoints/{first['id']}") + + assert [c["id"] for c in _list(client)] == [second["id"]] + + +def test_deleting_a_save_point_twice_is_a_404(client): + _turns(client, 1) + made = _save(client, "One") + path = f"/api/adventures/{client.adv_id}/checkpoints/{made['id']}" + assert client.delete(path).status_code == 204 + assert client.delete(path).status_code == 404 + + +# --------------------------------------------------------------- renaming + +def test_rename_changes_the_label_and_not_the_coordinate(client): + """`STORY-BRANCH-SEMANTICS.md` §23.""" + _turns(client, 3) + made = _save(client, "Before entering the abbey") + _turns(client, 2) + + r = client.patch( + f"/api/adventures/{client.adv_id}/checkpoints/{made['id']}", + json={"name": " Before the abbey, second try "}, + ) + assert r.status_code == 200, r.text + renamed = r.json() + assert renamed["name"] == "Before the abbey, second try" + assert (renamed["branch_id"], renamed["depth"]) == (made["branch_id"], made["depth"]) + + # And it still restores to exactly the position it always did. + _restore(client, made["id"]) + assert len(_texts(client)) == 7 + + +def test_rename_moves_no_story_and_no_head(client): + _turns(client, 3) + made = _save(client, "One") + rows = {a.id for a in _rows(client.adv_id)} + head_before = _head(client.adv_id) + + client.patch( + f"/api/adventures/{client.adv_id}/checkpoints/{made['id']}", + json={"name": "Another name"}, + ) + + assert {a.id for a in _rows(client.adv_id)} == rows + assert _head(client.adv_id) == head_before + + +def test_rename_refuses_a_blank_name(client): + _turns(client, 1) + made = _save(client, "One") + r = client.patch( + f"/api/adventures/{client.adv_id}/checkpoints/{made['id']}", json={"name": " "} + ) + assert r.status_code == 400 + assert _list(client)[0]["name"] == "One" + + +def test_a_note_can_be_kept_and_edited(client): + """`DATA-MODEL.md` §8's optional notes, and `BROWSER-UX-SPEC.md` §24's + optional second field.""" + _turns(client, 1) + made = _save(client, "One", note="the door was locked") + assert made["note"] == "the door was locked" + + r = client.patch( + f"/api/adventures/{client.adv_id}/checkpoints/{made['id']}", + json={"note": "the door was barred"}, + ) + assert r.status_code == 200 + assert r.json()["note"] == "the door was barred" + assert r.json()["name"] == "One" + + +# ------------------------------------------------------ ownership and errors + +def test_a_save_point_cannot_be_reached_through_another_campaign(client): + """A coordinate from another campaign names a different story's turn. The id + is matched against the adventure in the path, so this is a 404 rather than a + restore of the wrong story.""" + _turns(client, 3) + mine = _save(client, "Mine") + + other = client.post( + "/api/adventures", json={"title": "Another campaign"} + ) + assert other.status_code in (200, 201), other.text + other_id = other.json()["id"] + + for method, path in ( + ("post", f"/api/adventures/{other_id}/checkpoints/{mine['id']}/restore"), + ("patch", f"/api/adventures/{other_id}/checkpoints/{mine['id']}"), + ("delete", f"/api/adventures/{other_id}/checkpoints/{mine['id']}"), + ): + call = getattr(client, method) + r = call(path, json={"name": "x"}) if method == "patch" else call(path) + assert r.status_code == 404, (path, r.text) + + # It is untouched, and still restores in its own campaign. + assert [c["id"] for c in _list(client)] == [mine["id"]] + assert _restore(client, mine["id"]).status_code == 200 + + +def test_an_unknown_save_point_is_a_404(client): + _turns(client, 1) + assert _restore(client, 999_999).status_code == 404 + assert client.delete( + f"/api/adventures/{client.adv_id}/checkpoints/999999" + ).status_code == 404 + + +def test_a_save_point_whose_turn_is_gone_refuses_rather_than_approximating(client): + """Moving the head to the nearest surviving turn would be worse than doing + nothing: a Save Point that silently means somewhere else.""" + _turns(client, 3) + made = _save(client, "Before entering the abbey") + _turns(client, 2) + head_before = _head(client.adv_id) + + db = SessionLocal() + try: + doomed = ( + db.query(models.Action) + .filter_by( + adventure_id=client.adv_id, + branch_id=made["branch_id"], + depth=made["depth"], + ) + .all() + ) + assert doomed + for row in doomed: + db.delete(row) + db.commit() + finally: + db.close() + + listed = _list(client) + assert listed[0]["resolved"] is False + + r = _restore(client, made["id"]) + assert r.status_code == 409 + assert "no longer part of this story" in r.json()["detail"] + # And nothing moved. + assert _head(client.adv_id) == head_before + + +def test_deleting_a_branch_takes_its_save_points_with_it(client): + """Referential integrity, not cleanup. Nothing removes a Save Point for + going stale; this one goes because the story it named went.""" + _turns(client, 2) + _undo(client) + _undo(client) + ScriptedProvider.replies = ["A new road.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "the other way") + forked = _save(client, "On the new line") + + # Read somewhere the doomed branch is not load-bearing, then delete it. + db = SessionLocal() + try: + adv = db.get(models.Adventure, client.adv_id) + root = ( + db.query(models.Branch) + .filter_by(adventure_id=client.adv_id, parent_branch_id=None) + .one() + ) + doomed_id = forked["branch_id"] + assert doomed_id != root.id + finally: + db.close() + + client.post(f"/api/adventures/{client.adv_id}/branches/{root.id}/switch") + r = client.delete(f"/api/adventures/{client.adv_id}/branches/{doomed_id}") + assert r.status_code in (200, 204), r.text + + assert [c["id"] for c in _list(client)] == [] + + +# --------------------------------------------- E-series: lineage and memory + +def test_e01_a_memory_past_a_restored_head_stops_being_retrievable(client): + """The M3 chokepoint, exercised through the M4 door. Nothing prunes a memory + here — it stops matching the capped path, and starts again on Redo, without + being deleted or re-embedded.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + _turns(client, 3) + + branch, depth = _head(client.adv_id) + db = SessionLocal() + try: + db.add(models.Memory( + adventure_id=client.adv_id, + text="Mara learns the location of the key.", + branch_id=branch, depth=depth, + )) + db.commit() + finally: + db.close() + + assert _visible_memories(client) == ["Mara learns the location of the key."] + + _restore(client, made["id"]) + assert _visible_memories(client) == [] + # Not deleted — still a row, still embedded as it was. + assert _memory_rows(client.adv_id) == 1 + + for _ in range(3): + _redo(client) + assert _visible_memories(client) == ["Mara learns the location of the key."] + assert _memory_rows(client.adv_id) == 1 + + +def test_e01_an_old_futures_memory_stays_out_of_a_new_continuation(client): + """After restore plus a divergent write, the displaced line's memory must not + become eligible on the line now being read.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + _turns(client, 3) + + branch, depth = _head(client.adv_id) + db = SessionLocal() + try: + db.add(models.Memory( + adventure_id=client.adv_id, + text="Mara learns the location of the key.", + branch_id=branch, depth=depth, + )) + db.commit() + finally: + db.close() + + _restore(client, made["id"]) + ScriptedProvider.replies = ["Through the side door.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go around the back") + + assert _visible_memories(client) == [] + # Play on: it must not reappear as the new line grows past the old depth. + ScriptedProvider.replies = gold_replies("New") + _turns(client, 3) + assert _visible_memories(client) == [] + assert _memory_rows(client.adv_id) == 1 + + +def test_e04_the_transcript_after_a_restore_holds_only_the_active_lineage(client): + """Everything a read can see comes through the one capped path, so the + displaced continuation is absent from the transcript rather than filtered out + of it.""" + _turns(client, 2) + made = _save(client, "Before entering the abbey") + ScriptedProvider.replies = gold_replies("Old") + _turns(client, 3) + displaced = [t for t in _texts(client) if t.startswith("Old")] + assert displaced + + _restore(client, made["id"]) + ScriptedProvider.replies = ["Through the side door.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go around the back") + ScriptedProvider.replies = gold_replies("New") + _turns(client, 2) + + told = _texts(client) + assert not any(t.startswith("Old") for t in told) + assert any(t.startswith("New") for t in told) + # The rows are still there; they are simply not on this path. + assert any(a.text.startswith("Old") for a in _rows(client.adv_id)) + + +def _visible_memories(client) -> list[str]: + r = client.get(f"/api/adventures/{client.adv_id}/memories") + assert r.status_code == 200, r.text + db = SessionLocal() + try: + adventure = db.get(models.Adventure, client.adv_id) + path = lineage.path_of(db, adventure) + rows = ( + db.query(models.Memory) + .filter( + models.Memory.adventure_id == client.adv_id, + path.clause(models.Memory), + ) + .order_by(models.Memory.id) + .all() + ) + return [m.text for m in rows] + finally: + db.close() + + +def _memory_rows(adv_id) -> int: + db = SessionLocal() + try: + return db.query(models.Memory).filter_by(adventure_id=adv_id).count() + finally: + db.close() + + +# ------------------------------------------------------ I04: export / import + +def test_i04_named_save_points_survive_export_and_import(client): + _turns(client, 4) + _undo(client) + _undo(client) + early = _save(client, "Before entering the abbey", note="the door was locked") + _redo(client) + late = _save(client, "In the cloister") + told = _texts(client) + + bundle = _export(client) + assert [c["name"] for c in bundle["checkpoints"]] == [ + "Before entering the abbey", "In the cloister", + ] + + imported = _import(client, bundle) + copied = _list(client, adv_id=imported["id"]) + + assert sorted(c["name"] for c in copied) == [ + "Before entering the abbey", "In the cloister", + ] + by_name = {c["name"]: c for c in copied} + assert by_name["Before entering the abbey"]["note"] == "the door was locked" + # The coordinates point into the imported story, not the original's rows. + assert by_name["Before entering the abbey"]["depth"] == early["depth"] + assert by_name["In the cloister"]["depth"] == late["depth"] + assert all(c["resolved"] for c in copied) + assert {c["branch_id"] for c in copied} & { + b.id for b in _branch_rows(imported["id"]) + } == {c["branch_id"] for c in copied} + + +def test_i04_an_import_opens_where_the_bundle_was_read_not_at_a_save_point(client): + """A Save Point in the file is a position someone named, not the position the + campaign is read at. The head comes from `headDepth`, as it did before M4.""" + _turns(client, 5) + _undo(client) + _undo(client) + _save(client, "Way back at the start") # made here, then the head moves on + _redo(client) + undone_story = _texts(client) + + imported = _import(client, _export(client)) + + assert _story_of(imported["id"]) == undone_story + assert _adventure(client, adv_id=imported["id"])["can_redo"] is True + # And the Save Point arrived pointing somewhere else entirely. + saved = _list(client, adv_id=imported["id"])[0] + assert saved["depth"] < _head(imported["id"])[1] + + +def test_i04_an_imported_save_point_restores_in_the_new_campaign(client): + _turns(client, 2) + made = _save(client, "Before entering the abbey") + at_save = _texts(client) + _turns(client, 3) + + imported = _import(client, _export(client)) + copied = _list(client, adv_id=imported["id"])[0] + assert copied["id"] != made["id"] + + r = _restore(client, copied["id"], adv_id=imported["id"]) + assert r.status_code == 200, r.text + assert _story_of(imported["id"]) == at_save + # The original campaign did not move. + assert len(_texts(client)) == 11 + + +def test_a_bundle_written_before_m4_imports_with_no_save_points(client): + """Backward compatibility. A file with no `checkpoints` key is one written + when Save Points did not exist, and a campaign that had none is what it + records — so it opens, and it opens empty.""" + _turns(client, 3) + told = _texts(client) + bundle = _export(client) + del bundle["checkpoints"] + + imported = _import(client, bundle) + + assert _story_of(imported["id"]) == told + assert _list(client, adv_id=imported["id"]) == [] + + +def test_a_save_point_naming_a_turn_the_file_does_not_carry_is_dropped(client): + """A bookmark pointing outside the story is dropped rather than refusing the + whole import. The head is checked the other way, because misplacing *it* + affects every read in the file.""" + _turns(client, 2) + _save(client, "Real") + bundle = _export(client) + bundle["checkpoints"].append( + {"name": "Imaginary", "note": "", "branch": 0, "depth": 999} + ) + bundle["checkpoints"].append({"name": " ", "branch": 0, "depth": 1}) + bundle["checkpoints"].append({"name": "No branch", "branch": 77, "depth": 1}) + + imported = _import(client, bundle) + + assert [c["name"] for c in _list(client, adv_id=imported["id"])] == ["Real"] + + +def test_save_points_on_two_branches_survive_the_round_trip(client): + _turns(client, 2) + shared = _save(client, "The fork") + _turns(client, 2) + old_line = _save(client, "Down the old road") + + _restore(client, shared["id"]) + ScriptedProvider.replies = ["Through the side door.\n```state\n{\"player.gold\": 1}\n```"] + _play(client, "go around the back") + new_line = _save(client, "Down the new road") + + imported = _import(client, _export(client)) + copied = _list(client, adv_id=imported["id"]) + + assert sorted(c["name"] for c in copied) == [ + "Down the new road", "Down the old road", "The fork", + ] + # Three names, and they did not all collapse onto one branch. + assert len({c["branch_id"] for c in copied}) == 2 + assert all(c["resolved"] for c in copied) + del old_line, new_line + + +def _branch_rows(adv_id): + db = SessionLocal() + try: + return db.query(models.Branch).filter_by(adventure_id=adv_id).all() + finally: + db.close() + + +# ------------------------------------------- L03: reconstruction after restart + +def test_l03_a_save_point_restores_the_right_state_after_a_restart(client): + _turns(client, 3) + banked = _gold(client.adv_id) + told = _texts(client) + made = _save(client, "Before entering the abbey") + + _turns(client, 4) + assert _gold(client.adv_id) == banked + 4 * GOLD_PER_TURN + + after = _restart(client) + try: + r = _restore(after, made["id"]) + assert r.status_code == 200, r.text + assert _texts(after) == told + assert _gold(after.adv_id) == banked + # And the later history is still all there. + assert len(_rows(after.adv_id)) == 1 + 2 * 7 + finally: + after.close() + + +def test_l03_the_save_point_list_is_rebuilt_from_rows_alone(client): + _turns(client, 2) + first = _save(client, "One", note="a note") + _turns(client, 2) + second = _save(client, "Two") + + after = _restart(client) + try: + kept = _list(after) + assert [c["name"] for c in kept] == ["Two", "One"] # newest first + assert kept[1]["note"] == "a note" + assert [c["id"] for c in kept] == [second["id"], first["id"]] + assert all(c["resolved"] for c in kept) + finally: + after.close() + + +# ---------------------------------------------------- the schema M4 adds + +def test_an_m3_database_gains_the_save_point_table_and_keeps_its_story(): + """An M3 campaign database opens under M4 with no Save Points and no loss. + + The table is created by `create_all`, on existing databases as well as fresh + ones, exactly as `memories` and `branches` were before it; migration 80 adds + the index. What this proves is that a database stamped at M3's version — one + that has never seen a `checkpoints` table — reaches M4's version with the + table, the index, and every row it already had. + """ + import os + import tempfile + from sqlalchemy import create_engine, inspect, text + from sqlalchemy.orm import sessionmaker + from app import migrations + + path = os.path.join(tempfile.mkdtemp(), "m3.db") + m3 = create_engine(f"sqlite:///{path}") + Base.metadata.create_all(bind=m3) + # A campaign written by M3. Built through the models so that the columns it + # carries are whatever the application writes, rather than a list this test + # would have to keep current. + session = sessionmaker(bind=m3)() + try: + owner = models.User(is_guest=False, email="m3@example.com") + session.add(owner) + session.flush() + session.add(models.Adventure(user_id=owner.id, title="Old", head_depth=2)) + session.commit() + finally: + session.close() + with m3.begin() as conn: + # Now make it an M3 *schema*: no Save Points, stamped at M3's version. + conn.execute(text("DROP TABLE checkpoints")) + conn.execute(text("PRAGMA user_version = 79")) + assert "checkpoints" not in inspect(m3).get_table_names() + + migrations.bootstrap(m3) + + insp = inspect(m3) + assert "checkpoints" in insp.get_table_names() + assert {c["name"] for c in insp.get_columns("checkpoints")} == { + "id", "adventure_id", "name", "note", "branch_id", "depth", + "created_at", "updated_at", + } + assert "ix_checkpoints_adventure" in {i["name"] for i in insp.get_indexes("checkpoints")} + with m3.connect() as conn: + assert conn.execute(text("PRAGMA user_version")).scalar() == 80 + # No Save Points were invented for a campaign whose owner named none... + assert conn.execute(text("SELECT COUNT(*) FROM checkpoints")).scalar() == 0 + # ...and the campaign it already had is untouched. + assert conn.execute(text("SELECT title, head_depth FROM adventures")).one() == ("Old", 2) + + # Running it again changes nothing. + migrations.bootstrap(m3) + assert "checkpoints" in inspect(m3).get_table_names() + m3.dispose() diff --git a/backend/tests/test_tree_migration.py b/backend/tests/test_tree_migration.py index 7c25640..0467836 100644 --- a/backend/tests/test_tree_migration.py +++ b/backend/tests/test_tree_migration.py @@ -119,7 +119,12 @@ def pre_tree(): # Dropping the tables is the only way to remove the columns. # SQLite refuses to drop a column that a foreign key references, # and that is exactly the case for `branch_id`. - for table in ("actions", "memories", "branches", "adventures"): + # + # `checkpoints` (M4) is dropped first and for a different reason: it + # references both `branches` and `adventures`, and SQLite refuses to + # drop a table another table still points at. Any future table that + # references these four has to be added to the front of this list. + for table in ("checkpoints", "actions", "memories", "branches", "adventures"): conn.execute(text(f"DROP TABLE IF EXISTS {table}")) for ddl in PRE_TREE_DDL: conn.execute(text(ddl)) @@ -592,7 +597,7 @@ def pre_split(): Base.metadata.drop_all(bind=engine) Base.metadata.create_all(bind=engine) with engine.begin() as conn: - for table in ("actions", "memories", "branches", "adventures"): + for table in ("checkpoints", "actions", "memories", "branches", "adventures"): conn.execute(text(f"DROP TABLE IF EXISTS {table}")) for ddl in PRE_TREE_DDL: conn.execute(text(ddl)) diff --git a/frontend/src/api.js b/frontend/src/api.js index 5895213..5c9a845 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -106,6 +106,25 @@ export const api = { }), deleteBranch: (advId, branchId) => request(`/adventures/${advId}/branches/${branchId}`, { method: 'DELETE' }), + // Save Points (M4). A Save Point is a durable name for a story position; the + // server stores the coordinate and nothing else. Create takes no position — + // it is always made at the campaign's active head, which is where the reader + // is. Restore answers with the story as it now stands, like a branch switch, + // so the caller replaces its window rather than reloading everything. + listCheckpoints: (advId) => request(`/adventures/${advId}/checkpoints`), + createCheckpoint: (advId, name, note = '') => + request(`/adventures/${advId}/checkpoints`, { + method: 'POST', body: JSON.stringify({ name, note }), + }), + renameCheckpoint: (advId, checkpointId, name) => + request(`/adventures/${advId}/checkpoints/${checkpointId}`, { + method: 'PATCH', body: JSON.stringify({ name }), + }), + deleteCheckpoint: (advId, checkpointId) => + request(`/adventures/${advId}/checkpoints/${checkpointId}`, { method: 'DELETE' }), + restoreCheckpoint: (advId, checkpointId) => + request(`/adventures/${advId}/checkpoints/${checkpointId}/restore`, { method: 'POST' }), + // Play a turn again, differently (SP9). An AI turn regenerates; a player's // own takes the text given. Streams, because it is a turn like any other. // diff --git a/frontend/src/pages/Play/index.jsx b/frontend/src/pages/Play/index.jsx index 4c29811..9e42cd9 100644 --- a/frontend/src/pages/Play/index.jsx +++ b/frontend/src/pages/Play/index.jsx @@ -22,6 +22,7 @@ import { BranchPanel } from './panels/BranchPanel' import { InsightsPanel } from './panels/InsightsPanel' import { MemoryPanel } from './panels/MemoryPanel' import { PlotPanel } from './panels/PlotPanel' +import { SavePointPanel } from './panels/SavePointPanel' const MODES = ['do', 'say', 'story'] const PLAYER_TYPES = ['do', 'say', 'story'] @@ -68,7 +69,7 @@ export default function Play() { const [busy, setBusy] = useState(false) const [toast, setToast] = useState(null) const [editing, setEditing] = useState(null) - const [panel, setPanel] = useState(null) // null | 'plot' | 'insights' + const [panel, setPanel] = useState(null) // null | 'plot' | 'memory' | 'branches' | 'savepoints' | 'insights' // Bumped when something outside the turn loop changes the drawers' state // (currently "Update from scenario"), which no action count would reflect. const [stateKey, setStateKey] = useState(0) @@ -545,6 +546,8 @@ export default function Play() { onClick={() => setPanel(panel === 'memory' ? null : 'memory')}>Memory + + {/* Save Point sits with the history controls because that is what + it is: a name for a position Undo and Redo move between. The + button opens the panel, where the name is typed — the moment it + saves is wherever the story is being read, so there is nothing + to choose first. */} +
@@ -752,7 +762,10 @@ export default function Play() { {panel && (
-

{{ plot: 'Plot Components', memory: 'Memory Bank', branches: 'Branches', insights: 'Insights' }[panel]}

+

{{ + plot: 'Plot Components', memory: 'Memory Bank', branches: 'Branches', + savepoints: 'Save Points', insights: 'Insights', + }[panel]}

{panel === 'plot' ? ( @@ -764,6 +777,17 @@ export default function Play() { // but deleting a branch deletes the memories that hung off it, // and that happens without a turn being played. refreshKey={`${actions.length}:${stateKey}`} /> + ) : panel === 'savepoints' ? ( + // Restoring one moves the story exactly as Undo and Redo do, so it + // adopts the returned window the same way a branch switch does — + // the state panels are showing another position's numbers until + // they re-read. + setToast({ text: message, isError: true })} + /> ) : panel === 'branches' ? ( { + let cancelled = false + setFailed(null) + api.listCheckpoints(advId) + .then((list) => { if (!cancelled) setPoints(list) }) + .catch((err) => { if (!cancelled) setFailed(err.message) }) + return () => { cancelled = true } + }, [advId, refreshKey, tick]) + + // Answers whether it worked, so a caller can keep its editor open on a + // refusal — a rename the server turned down must not take the typed name + // with it. + async function run(id, work) { + setBusyId(id) + try { + await work() + setTick((t) => t + 1) + return true + } catch (err) { + onError(err.message) + return false + } finally { + setBusyId(null) + } + } + + async function create(e) { + e.preventDefault() + if (!name.trim() || saving) return + setSaving(true) + try { + await api.createCheckpoint(advId, name.trim(), note.trim()) + setName('') + setNote('') + setTick((t) => t + 1) + } catch (err) { + onError(err.message) + } finally { + setSaving(false) + } + } + + const restore = (p) => run(p.id, async () => { + onRestored(await api.restoreCheckpoint(advId, p.id)) + setConfirming(null) + }) + const rename = async (p) => { + if (await run(p.id, () => api.renameCheckpoint(advId, p.id, renaming.text))) { + setRenaming(null) + } + } + const remove = async (p) => { + if (await run(p.id, () => api.deleteCheckpoint(advId, p.id))) setConfirming(null) + } + + return ( +
+
+ + setName(e.target.value)} + /> + setNote(e.target.value)} + /> + +

+ Saves the moment you are reading now. If you have stepped back, that + is the moment it saves. +

+
+ + {failed &&
Couldn’t read the Save Points — {failed}
} + {!failed && !points &&
Reading your Save Points…
} + {!failed && points?.length === 0 && ( +

+ No Save Points yet. Name a moment you might want to come back to, then + keep playing — going back to it later leaves everything you wrote + after it in place. +

+ )} + +
+ {(points || []).map((p) => { + const isRenaming = renaming?.id === p.id + const busy = busyId === p.id + const confirm = confirming?.id === p.id ? confirming.kind : null + return ( +
+
+ {isRenaming ? ( + setRenaming({ ...renaming, text: e.target.value })} + onKeyDown={(e) => { + if (e.key === 'Enter') rename(p) + if (e.key === 'Escape') setRenaming(null) + }} + /> + ) : ( + {p.name} + )} +
+ +
+ Moment {p.turn} + {/* Said plainly, without naming a branch: the story took a + different turning after this point, and going back to it + returns to the telling it was saved in. */} + {!p.on_path && p.resolved && ' · on a path you left'} + {!p.resolved && ' · this moment is no longer in the story'} +
+ {p.note &&
{p.note}
} + + {confirm === 'restore' ? ( +
+ + The story will return to this Save Point. Everything you + wrote after it is kept — it just stops being where you are. + + + +
+ ) : confirm === 'delete' ? ( +
+ + Delete this Save Point? Deleting it does not delete any of + the story — only the name you gave this moment. + + + +
+ ) : ( +
+ + {isRenaming ? ( + <> + + + + ) : ( + + )} + +
+ )} +
+ ) + })} +
+
+ ) +} + +export { SavePointPanel } diff --git a/frontend/src/styles/play.css b/frontend/src/styles/play.css index 117c668..618545d 100644 --- a/frontend/src/styles/play.css +++ b/frontend/src/styles/play.css @@ -255,6 +255,102 @@ flex-basis: 100%; } +/* Save Points (M4). Deliberately the same furniture as the branch panel: a Save + Point is a name for a position, and it should not look like a different + species of thing from the lines it names positions on. The confirmations + borrow the branch panel's shape but not its danger colouring — restoring + destroys nothing, and only deletion is red. */ +.save-point-panel { display: flex; flex-direction: column; gap: 14px; } +.save-point-new { display: flex; flex-direction: column; gap: 7px; } +.save-point-new label { + font-size: 0.64rem; + letter-spacing: 0.14em; + text-transform: uppercase; + color: var(--text-dim); +} +.save-point-new input { + padding: 5px 9px; + font-family: var(--font-story); + font-size: 0.95rem; +} +.save-point-new input.save-point-note { + font-size: 0.82rem; +} +.save-point-hint, .save-point-intro { + margin: 0; + color: var(--text-dim); + font-size: 0.78rem; + line-height: 1.55; +} +.save-point-list { display: flex; flex-direction: column; gap: 8px; } +.save-point-row { + display: flex; + flex-direction: column; + gap: 5px; + padding: 9px 11px; + border: 1px solid var(--border); + border-radius: 5px; + background: var(--bg-input); +} +/* A Save Point naming a moment on a telling the story has left. Dimmed rather + than hidden: it still restores, and hiding it would be the automatic cleanup + this milestone deliberately does not do. */ +.save-point-row.elsewhere { opacity: 0.72; } +.save-point-head { display: flex; align-items: center; gap: 7px; } +.save-point-name { + font-family: var(--font-story); + font-size: 0.98rem; + color: var(--text); +} +.save-point-rename { + flex: 1; + min-width: 0; + padding: 3px 7px; + font-family: var(--font-story); + font-size: 0.95rem; +} +.save-point-meta { + font-size: 0.72rem; + color: var(--text-dim); + font-variant-numeric: tabular-nums; +} +.save-point-note-text { + font-size: 0.78rem; + color: var(--text-dim); + line-height: 1.5; +} +.save-point-tools, .save-point-confirm { + display: flex; + align-items: center; + gap: 5px; + flex-wrap: wrap; +} +.save-point-tools button, .save-point-confirm button { + padding: 3px 9px; + font-size: 0.72rem; + color: var(--text-dim); + background: transparent; + border: 1px solid var(--border); +} +.save-point-tools button:hover:not(:disabled), +.save-point-confirm button:hover:not(:disabled) { + color: var(--accent-bright); + border-color: var(--border-bright); +} +.save-point-tools button.danger:hover:not(:disabled), +.save-point-confirm button.danger:hover:not(:disabled) { + color: var(--danger); + border-color: var(--danger); +} +.save-point-tools button:disabled, +.save-point-confirm button:disabled { opacity: 0.4; cursor: default; } +.save-point-confirm span { + font-size: 0.74rem; + color: var(--text-dim); + line-height: 1.5; + flex-basis: 100%; +} + .action-edit { margin-bottom: 14px; } /* Sized by AutoTextarea to fit the text being edited — an AI beat is usually several paragraphs, and the old fixed 110px turned that into a keyhole. diff --git a/planning/BUILD-MILESTONES.md b/planning/BUILD-MILESTONES.md index dcd74f6..542d267 100644 --- a/planning/BUILD-MILESTONES.md +++ b/planning/BUILD-MILESTONES.md @@ -1,6 +1,6 @@ # Adventure Storyteller — Production Build Milestones -**Status:** In implementation. M1, M2 and M3 complete and accepted (M1 and M2: 2026-09-02; M3: 2026-09-03); M4 — Named Save Points / Checkpoints — next +**Status:** In implementation. M1, M2 and M3 complete and accepted (M1 and M2: 2026-09-02; M3: 2026-09-03); M4 — Named Save Points / Checkpoints — implemented 2026-09-03, awaiting review **Base:** AI-DnD `d72f7c1bda0f34fccd84afb7a25c34eb01c901de` ## 1. Purpose @@ -351,6 +351,62 @@ M3's cost was reconciling them; a parallel checkpoint mover would recreate that divergence in a place where the two paths would silently disagree about what "restore" means. See ADR 012. +## Status: IMPLEMENTED — awaiting review + +Implementation landed 2026-09-03. **Not accepted**: the milestone report has not +been written and no reviewer has read the change. The Definition of Done above is +met by the code and the tests below; whether it is met by the *product* is what +the review is for. + +**What M4 delivered:** + +- **`checkpoints`**, a table holding a name, an optional note, and a + `(branch, depth)` coordinate — and no copy of any story. `create_all` builds + it, as it did `memories` and `branches`; migration 80 adds the index. No + backfill: nobody had named a position before M4, and inventing one would be + inventing the decision. +- **Create / list / rename / delete / restore** under + `/api/adventures/{id}/checkpoints`, campaign-scoped, with a Save Point from + another campaign a 404 rather than a restore of the wrong story. +- **Create at the active head, not the retained tip**, so a Save Point made after + two Undos names the undone position. +- **Restore that delegates**, and is the whole of the milestone's architecture: + resolve the coordinate, refuse it if it names no live turn, then + `head.move_to_node` — one function whose depth half is M3's `head.move_to` + unchanged, and whose branch half is the single assignment `switch_branch` + makes. Restore forks nothing. +- **A browser Save Point panel** — create form, list, Restore, Rename, Delete, + with both confirmations saying what is *not* destroyed — plus a Save Point + button beside Undo and Redo, where it belongs. No branch explorer, no + discarded-history browser, no merge UI. +- **Export/import of Save Points** with no format version bump, and a pre-M4 + bundle importing with none. + +**The one architectural decision M4 had to make**, which ADR 012 does not settle: +a Save Point can name a position on a line the story has since left, so restore +moves the branch half of the head as well — but *only* when the coordinate is not +on the path being read. Doing it unconditionally would quietly hand back an +abandoned continuation whenever a Save Point in a shared prefix was restored. No +new ADR: this is ADR 012's mechanism applied to both halves of a coordinate ADR +012 already defines, not a new architecture. `TECHNICAL-DESIGN.md` §8.8 records +it. + +**Tests:** 42 in `backend/tests/test_save_points.py`, covering D11-D14, I04, L03, +E-series lineage and memory isolation after restore and divergence, the edge +cases in the brief, and the M3-database migration. + +**Outstanding condition, carried from M3 and not resolved here:** the **browser +smoke test has still not been performed**, for M3 or for M4. No session has had a +usable browser. The M4 sequence was driven end-to-end over HTTP against a live +server with a real process restart, and every server-side behaviour it covers +passes; the DOM-level behaviour of the Save Point panel, its buttons and its +confirmations remains unverified by observation. + +**Debt M4 carries forward:** none newly discovered in the head model. The Save +Point panel has no frontend test, because the project still has no frontend test +runner at all (M8). `POST /adventures/import` still returns every branch's rows +rather than a head-capped window (inherited, M3). + --- # M5 — Genre-Neutral Authoritative Narrative State diff --git a/planning/DATA-MODEL.md b/planning/DATA-MODEL.md index 9fce8b6..436f0fb 100644 --- a/planning/DATA-MODEL.md +++ b/planning/DATA-MODEL.md @@ -210,6 +210,40 @@ checkpoint: A checkpoint is a named pointer to a recoverable story position. It should normally remain tied to the turn where it was created. Restoring it moves the campaign active head; it does not delete later retained history. A new branch is created on the first divergent write after restore, not merely because the checkpoint was opened. +As implemented in M4, the pointer is a **coordinate rather than a turn id**: +`(branch_id, depth)`, which is the same pair §4 records as the campaign's active +head and which `head.node_at` resolves. This is the equivalence §4 already draws +between a turn reference and a branch-plus-depth, applied to the same position +from the other end, and it is not a shortcut — it is the more correct pointer of +the two for this data model: + +- **A coordinate follows a retry; a row id does not.** One coordinate holds + every attempt at a turn and exactly one of them is live (§7). A Save Point + names the turn, so it must land on whichever take the story currently tells. + Pinning the row would leave the pointer on a superseded attempt the reader + cannot see. +- **The user-facing term is Save Point**; `checkpoint` remains the internal name + (`BROWSER-UX-SPEC.md` §23). + +The row carries the name, an optional note, the coordinate, and its timestamps. +It carries **no** copy of the transcript, the state, the prompt, a memory, a +summary, or a branch's contents. Everything a restore produces comes from the +retained history the coordinate points into. + +Two consequences worth recording here: + +- **Restore reuses the campaign's one head-movement mechanism.** It resolves the + coordinate and moves the head; nothing is reconstructed and nothing is + deleted. The branch half of the head moves only when the coordinate is not on + the path being read, which is what makes a Save Point on a departed line + restorable at all — and what keeps a Save Point in a shared prefix from + dragging the reader off the line they chose. See `TECHNICAL-DESIGN.md` §8.8. +- **A checkpoint is durable against everything but its own deletion and its + branch's.** No pass removes one for going stale, sitting behind the head, or + naming a line the story left (`STORY-BRANCH-SEMANTICS.md` §19). Deleting a + branch removes its checkpoints by cascade, as it removes its memories, because + the story they named is gone. + ## 9. Narrative Entity An entity is a persistent thing or concept in the fictional world. @@ -684,7 +718,11 @@ The physical container format remains an implementation choice, but the export m As implemented in M3, the export carries the active branch, the active head position on it, and each branch's disposition, alongside the whole retained turn -graph. The governing rule for this package is that an export carries what was +graph. **M4 added the checkpoints**, by the same rule: a position someone chose +to name cannot be recomputed from the turns, because nothing about a turn records +that it was bookmarked. The head and the checkpoints stay independent on import — +a campaign opens where its head says, never at a checkpoint merely because one is +in the file. The governing rule for this package is that an export carries what was *chosen* and recomputes what is *derived* — and the active head moved from the second category to the first, because once Undo stops deleting, two campaigns with identical turns can be being read at different positions and no import can diff --git a/planning/README.md b/planning/README.md index 8e5f2ac..d78c652 100644 --- a/planning/README.md +++ b/planning/README.md @@ -4,8 +4,9 @@ **Current state:** Phase 0 complete; AI-DnD forked as the production base; milestones **M1, M2 and M3 implemented and accepted** (M3: 2026-09-03). -**Next:** **M4 — named Save Points.** Its brief has not been written yet, and -writing it is the current action. +**M4 — named Save Points — is implemented (2026-09-03) and awaiting review.** +Its implementation report has not been written, and writing it is the current +action. Do not begin M5. **Package version:** see `VERSION.md`, which records what each revision changed and why. @@ -132,7 +133,9 @@ that is the one the next milestone's planning has to consult: Completed earlier milestones are in `archive/milestone-reports/`. When M4's report lands, M3's moves there too: a milestone report is useful during the -immediate next milestone and historical afterwards. +immediate next milestone and historical afterwards. **M3's report has not moved +yet**, because M4's does not exist — the rotation belongs to M4's closeout, not +to its implementation. ## The decision this package rests on @@ -234,8 +237,8 @@ Milestone M3 COMPLETE (2026-09-03) active-head export and ADR 012 | v -Milestone M4 NEXT — brief not yet prepared - named Save Points +Milestone M4 IMPLEMENTED 2026-09-03 — + named Save Points awaiting review; no report yet | v M5-M11, one at a time see BUILD-MILESTONES.md @@ -245,15 +248,15 @@ M5-M11, one at a time see BUILD-MILESTONES.md **One milestone at a time. Do not begin a milestone before its brief exists.** -**No M4 brief has been prepared.** Writing one is the current action, informed -by the post-M3 corrections below, by the note `BUILD-MILESTONES.md` attaches to -M4, and by **ADR 012**, which records the head-movement mechanism M4 must reuse -rather than reimplement. +**M4 is implemented and unreviewed.** Its implementation report is the current +action; M5 does not begin before that report is written and accepted. -One M3 condition remains open and does not block M4: the required **browser -smoke test has not been performed**, because no session in which M3 was -implemented or reviewed had a browser available. See -`reports/M3-IMPLEMENTATION-REPORT.md` §M and §W.4. +Two conditions remain open. The **browser smoke test has still not been +performed** — now for M3 and for M4 — because no session so far has had a usable +browser. See `reports/M3-IMPLEMENTATION-REPORT.md` §M and §W.4, and the M4 status +block in `BUILD-MILESTONES.md`. And **no M4 review exists**: the status block was +written by the implementation and records what it built, which is not the same as +a reviewer having read it. ## What each milestone closeout corrected diff --git a/planning/TECHNICAL-DESIGN.md b/planning/TECHNICAL-DESIGN.md index 98fbc71..24983ee 100644 --- a/planning/TECHNICAL-DESIGN.md +++ b/planning/TECHNICAL-DESIGN.md @@ -413,6 +413,49 @@ than act silently, because retained history must not be made to disagree with itself in a way the user cannot see. See `STORY-BRANCH-SEMANTICS.md` §10 and §14A. +### 8.8 Save Points, as implemented in M4 + +M4 added durable named Save Points and built nothing in §8 that was not already +there. This records what the milestone establishes as fact. + +**A Save Point is a name and a coordinate.** The stored row holds the name, an +optional note, and `(branch, depth)` — the same pair §8.7 calls the head. It +holds no transcript, no state, no summary, no memory, and no branch contents. +`DATA-MODEL.md` §8 describes the pointer as naming a turn; the coordinate is +that turn's address, and `DATA-MODEL.md` §8's implementation note records why +this project uses the address rather than a row id: one coordinate can hold +several attempts at a turn, and a retry replaces the live one. "Turn 42 of this +line" survives a retry; a row id would pin a take the story no longer tells. + +**Restore is head movement, and nothing else.** It resolves the coordinate, +refuses it if it no longer names a live turn, and then moves the head — the +depth through §8.7's single move operation, unchanged. The transcript, the +assembled context, the state and memory eligibility all arrive together because +they already read through the one capped lineage. There is no second restore +path, no state reconstruction, no memory pruning and no separate redo stack: +D13 is satisfied by the mechanism rather than by code written to satisfy it. + +**A Save Point may name a position on a line the story has left.** Save Points +survive divergence, so this is reachable in ordinary use, and the depth half of +the head cannot reach a branch the current path does not contain. Restore +therefore moves the branch half as well when, and only when, the coordinate is +not on the path being read — the same single assignment a branch switch makes. +The distinction matters in the other direction too: a Save Point in a shared +prefix must *not* drag the reader onto the ancestor, because which continuation +follows that turn is exactly what the reader has already chosen. + +**Restore never forks.** Moving the head is not a decision to abandon anything. +The first write below the restored head forks, through §8.7's existing check, +and the displaced future stays retained — so Redo still walks the original +continuation until the user writes something different, and stops offering it +once they have. + +**Nothing removes a Save Point but the user.** There is no cleanup pass, and none +is wanted: a Save Point pointing behind the head, or into a line the story left, +is doing its job. The one exception is referential and not a policy — deleting a +branch takes its Save Points with it, by the same cascade that takes its +memories, because the story they named went with it. + ## 9. Export / Import and Head Position AI-DnD's current export carries branch information but reconstructs the imported head at the branch tip. @@ -460,6 +503,30 @@ Every row of an abandoned line is exported either way, so without that metadata restored campaign could not distinguish abandoned history from active history — which is precisely what a later cleanup or recovery feature has to select on. +### 9.2 Save Points in the bundle, as implemented in M4 + +Save Points are exported and imported with the campaign, which is `I04`. They +fall on the "chosen" side of §9.1's rule without argument: a position someone +named is not recoverable from the rows, since nothing about a turn records that +a player once bookmarked it. + +No format version bump. A bundle written before M4 has no `checkpoints` key and +imports with none, which is what such a campaign had — the same unambiguous +absence §9.1 relies on for the head depth, and the same treatment the persona +block and the branch disposition received. + +The head and the Save Points are independent, deliberately. An import opens the +campaign where `headDepth` says, never at a Save Point merely because the file +carries one: the bundle records where the story was being read and, separately, +which positions were named, and choosing between them is the user's to make +after the file is open. + +A Save Point whose coordinate names no turn in the file is dropped rather than +refusing the import — the opposite of the head depth's treatment, and for a +stated reason. A misplaced head affects every read in the file; a bookmark +pointing outside the story affects only itself, and rejecting a whole campaign +to protect one bookmark would lose the story to save the pointer. + ## 10. Authoritative Narrative State ### 10.1 Do not retain the RPG state protocol as the product model diff --git a/planning/VERSION.md b/planning/VERSION.md index 46fece8..96e8dc1 100644 --- a/planning/VERSION.md +++ b/planning/VERSION.md @@ -1,8 +1,42 @@ # Planning Package Version -- **Package:** Adventure Storyteller Planning Package v2.4 +- **Package:** Adventure Storyteller Planning Package v2.5 - **Revision date:** 2026-09-03 -- **Status:** Phase 0 complete; architecture selected; **Milestones M1, M2 and M3 implemented and accepted**; M4 is next to brief. +- **Status:** Phase 0 complete; architecture selected; **Milestones M1, M2 and M3 implemented and accepted**; **M4 implemented 2026-09-03 and awaiting review.** + +## v2.5 — M4 Implementation (2026-09-03) + +M4 added durable named Save Points. This revision records only what the +implementation established as fact; **no product requirement changed**, and the +milestone is **not** marked accepted — its review has not been written. + +- `TECHNICAL-DESIGN.md` gains **§8.8** and **§9.2**: the Save Point as a name + plus a coordinate holding no story, restore as head movement with a bounds + check, the rule that the branch half of the head moves only when the + coordinate is off the path being read, restore never forking, and the bundle + carrying Save Points independently of the head. +- `DATA-MODEL.md` **§8** records the pointer as implemented — `(branch, depth)` + rather than a turn id, with the reason: one coordinate holds every attempt at + a turn and exactly one is live, so a coordinate follows a retry where a row id + would pin a superseded take. **§29** records the checkpoints in the export. +- `BUILD-MILESTONES.md` **M4** gains a status block: what shipped, the one + architectural decision the milestone had to make and why it needed no new ADR, + the test count, and the outstanding browser condition. +- `README.md` describes Save Points as a user-facing capability. + +**No ADR was created.** ADR 012 already decides the architecture M4 needed — +restore reuses active-head movement — and a table is not a decision. The one +question ADR 012 does not answer, whether restore moves the branch half of the +head, is that same mechanism applied to a coordinate ADR 012 already defines; +`TECHNICAL-DESIGN.md` §8.8 records the answer rather than a new ADR asserting it. + +`SPECIFICATION.md`, `SECURITY-THREAT-MODEL.md`, `STORY-BRANCH-SEMANTICS.md` and +`V1-ACCEPTANCE-TESTS.md` are unchanged. M4 altered no product requirement, added +no outbound path, and implemented the checkpoint semantics +`STORY-BRANCH-SEMANTICS.md` §18-25 already specified rather than amending them. + +**The browser smoke test remains unperformed, now for both M3 and M4.** No +session has had a usable browser. See `BUILD-MILESTONES.md` M3 and M4. ## v2.4 — Documentation Consolidation (2026-09-03)