diff --git a/README.md b/README.md index fd88f13..7382a27 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ development, Vite proxies `/api` to FastAPI. ## Tests -539 backend tests: unit tests plus full HTTP integration through the real quickjs scripting +549 backend tests: unit tests plus full HTTP integration through the real quickjs scripting engine, with the LLM provider mocked. CI runs them on every push, alongside the frontend lint/build and a Docker image build. diff --git a/backend/app/seed.py b/backend/app/seed.py index 2318d1d..3b14cb1 100644 --- a/backend/app/seed.py +++ b/backend/app/seed.py @@ -8,8 +8,10 @@ adventure copies the scenario's story cards and scripts into the adventure, so the seeded scripts run for guests too. Seed files are the source of truth for demo content: a scenario is inserted if -missing, and reconciled in place when a seed file's content changes, so an edit -ships on the next deploy. When a seed already matches, nothing is written, so +missing, reconciled in place when a seed file's content changes, and deleted +when no file claims its title any more, so an edit ships on the next deploy. +Rename a seed by changing its `title` and listing the old one under +`previous_titles`, which moves the rename onto the existing row. When a seed already matches, nothing is written, so this stays cheap to run on every boot. An adventure already started from a demo keeps its own copied cards and scripts and is unchanged. Only a new adventure picks up the updated content. @@ -46,18 +48,26 @@ def seed_public_scenarios(engine: Engine) -> None: db = SessionLocal() try: changed = 0 + # Every title the files claim, including the ones they used to use. The + # sweep below deletes the seeded rows this set does not name. + claimed: set[str] = set() + complete = True for path in files: try: data = json.loads(path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as exc: logger.warning("Skipping seed file %s: %s", path.name, exc) + complete = False continue title = (data.get("title") or "").strip() if not title: continue - existing = _find_seeded(db, title) or _find_renamed(db, data) + claimed.add(title) + claimed.update(str(old) for old in data.get("previous_titles") or []) + + existing = find_seeded(db, title) or _find_renamed(db, data) if existing is None: _insert_scenario(db, data) changed += 1 @@ -65,6 +75,7 @@ def seed_public_scenarios(engine: Engine) -> None: _update_scenario(db, existing, data) changed += 1 + changed += _sweep_unclaimed(db, claimed) if complete else 0 if changed: db.commit() logger.info("Seeded/updated %d public demo scenario(s).", changed) @@ -76,6 +87,45 @@ def seed_public_scenarios(engine: Engine) -> None: db.close() +def _sweep_unclaimed(db, claimed: set[str]) -> int: + """Deletes seeded scenarios no seed file claims any more, and returns how + many went. + + A rename used to strand the row it left behind. `previous_titles` stops new + ones appearing, and this removes the ones already out there, which was + otherwise hand-work on every deployment. Only rows with a NULL owner and + `is_public` are considered, and a player's own scenario is neither, so + nothing anybody created can be reached from here. + + An adventure started from a deleted demo survives. `adventures.scenario_id` + is `ON DELETE SET NULL`, so the story, its cards, and its scripts are its + own copies and stay; the adventure loses the cover art it inherited. + + The caller skips this when a seed file failed to parse. A file that cannot + be read claims no title, and deleting on that basis would treat a syntax + error as an instruction to remove live content. An empty seed directory + never reaches here at all, for the same reason. + """ + stale = ( + db.query(models.Scenario) + .filter( + models.Scenario.user_id.is_(None), + models.Scenario.is_public.is_(True), + models.Scenario.title.notin_(claimed) if claimed else True, + ) + .all() + ) + for scenario in stale: + logger.info("Removing seeded scenario %r; no seed file claims it.", scenario.title) + # The scripts are joined through a secondary table, so nothing cascades + # to them. They have a NULL owner and no other reader. + for script in list(scenario.scripts): + db.delete(script) + scenario.scripts = [] + db.delete(scenario) + return len(stale) + + def _card_tuple(source, get) -> tuple: return tuple(get(source, f) for f in _CARD_FIELDS) @@ -84,7 +134,7 @@ def _script_tuple(source, get) -> tuple: return tuple(get(source, f) for f in _SCRIPT_FIELDS) -def _find_seeded(db, title: str) -> models.Scenario | None: +def find_seeded(db, title: str) -> models.Scenario | None: """Returns the seeded scenario with this exact title, if there is one.""" return ( db.query(models.Scenario) @@ -110,7 +160,7 @@ def _find_renamed(db, data: dict) -> models.Scenario | None: Drop a `previous_titles` entry once every deployment has booted past it. """ for old in data.get("previous_titles") or []: - found = _find_seeded(db, str(old)) + found = find_seeded(db, str(old)) if found is not None: return found return None diff --git a/backend/tests/test_seed_sweep.py b/backend/tests/test_seed_sweep.py new file mode 100644 index 0000000..c5affd0 --- /dev/null +++ b/backend/tests/test_seed_sweep.py @@ -0,0 +1,162 @@ +"""Seeded demo scenarios: renames land in place, and orphans are removed. + +A seed file is matched to its scenario by title, so renaming one used to insert +a second scenario and leave the first public forever. Both halves of the fix are +here: `previous_titles` moves the rename onto the existing row, and the sweep +deletes seeded rows no file claims any more. + + python -m pytest tests/test_seed_sweep.py -v +""" +import json +import os +import tempfile + +_tmp = tempfile.NamedTemporaryFile(suffix=".db", delete=False) +_tmp.close() +os.environ["AIDND_DB_PATH"] = _tmp.name +os.environ.pop("AIDND_DATABASE_URL", None) +os.environ.pop("DATABASE_URL", None) + +import pytest + +from app import models, seed +from app.database import Base, SessionLocal, engine + + +@pytest.fixture() +def db(): + Base.metadata.create_all(bind=engine) + session = SessionLocal() + try: + yield session + finally: + session.close() + Base.metadata.drop_all(bind=engine) + + +@pytest.fixture() +def seed_dir(tmp_path, monkeypatch): + monkeypatch.setattr(seed, "SEED_DIR", tmp_path) + return tmp_path + + +def write_seed(seed_dir, name: str, **fields) -> None: + data = {"title": fields.pop("title"), "description": "d", "prompt": "p"} + data.update(fields) + (seed_dir / name).write_text(json.dumps(data), encoding="utf-8") + + +def titles(db) -> set[str]: + return {s.title for s in db.query(models.Scenario).all()} + + +def test_a_seed_is_inserted_once(db, seed_dir): + write_seed(seed_dir, "a.json", title="Alpha") + seed.seed_public_scenarios(engine) + seed.seed_public_scenarios(engine) + assert titles(db) == {"Alpha"} + + +def test_a_rename_moves_the_existing_row(db, seed_dir): + """The whole point: one row, one id, and no orphan left behind.""" + write_seed(seed_dir, "a.json", title="Alpha") + seed.seed_public_scenarios(engine) + original = db.query(models.Scenario).one().id + + write_seed(seed_dir, "a.json", title="Alpha Prime", previous_titles=["Alpha"]) + seed.seed_public_scenarios(engine) + db.expire_all() + row = db.query(models.Scenario).one() + assert (row.id, row.title) == (original, "Alpha Prime") + + +def test_a_seeded_row_no_file_claims_is_deleted(db, seed_dir): + """The stale demo this sweep exists for, reproduced.""" + write_seed(seed_dir, "a.json", title="Alpha") + write_seed(seed_dir, "b.json", title="Beta") + seed.seed_public_scenarios(engine) + assert titles(db) == {"Alpha", "Beta"} + + (seed_dir / "b.json").unlink() + seed.seed_public_scenarios(engine) + db.expire_all() + assert titles(db) == {"Alpha"} + + +def test_a_previous_title_still_counts_as_claimed(db, seed_dir): + """A rename runs the sweep in the same pass, and must not eat its own row.""" + write_seed(seed_dir, "a.json", title="Alpha") + seed.seed_public_scenarios(engine) + write_seed(seed_dir, "a.json", title="Alpha Prime", previous_titles=["Alpha"]) + seed.seed_public_scenarios(engine) + db.expire_all() + assert titles(db) == {"Alpha Prime"} + + +def test_a_players_own_scenario_is_never_touched(db, seed_dir): + """Only a NULL owner and `is_public` make a row seeded. A player's is + neither, so nothing anybody created is reachable from the sweep.""" + user = models.User(is_guest=True) + db.add(user) + db.flush() + db.add(models.Scenario(user_id=user.id, is_public=True, title="Mine")) + db.add(models.Scenario(user_id=user.id, is_public=False, title="Also mine")) + db.commit() + + write_seed(seed_dir, "a.json", title="Alpha") + seed.seed_public_scenarios(engine) + db.expire_all() + assert titles(db) == {"Alpha", "Mine", "Also mine"} + + +def test_an_unreadable_seed_file_stops_the_sweep(db, seed_dir): + """A syntax error is not an instruction to delete live demo content. + + A file that will not parse claims no title, so sweeping on that pass would + remove the scenario it describes, and the next deploy would put it back. + """ + write_seed(seed_dir, "a.json", title="Alpha") + write_seed(seed_dir, "b.json", title="Beta") + seed.seed_public_scenarios(engine) + + (seed_dir / "b.json").write_text("{ not json", encoding="utf-8") + seed.seed_public_scenarios(engine) + db.expire_all() + assert titles(db) == {"Alpha", "Beta"} + + +def test_an_adventure_outlives_the_demo_it_started_from(db, seed_dir): + """`adventures.scenario_id` is ON DELETE SET NULL, so the story survives and + loses only the cover art it inherited.""" + write_seed(seed_dir, "a.json", title="Alpha") + write_seed(seed_dir, "keep.json", title="Kept") + seed.seed_public_scenarios(engine) + scenario = db.query(models.Scenario).filter_by(title="Alpha").one() + + user = models.User(is_guest=True) + db.add(user) + db.flush() + adventure = models.Adventure(user_id=user.id, scenario_id=scenario.id, title="Mine") + db.add(adventure) + db.commit() + adventure_id = adventure.id + + (seed_dir / "a.json").unlink() + seed.seed_public_scenarios(engine) + db.expire_all() + assert titles(db) == {"Kept"} + survivor = db.get(models.Adventure, adventure_id) + assert survivor is not None + assert survivor.scenario_id is None + + +def test_an_empty_seed_directory_deletes_nothing(db, seed_dir): + """No files at all reads as a packaging failure, not as a request to remove + every demo. The seeder returns before the sweep.""" + write_seed(seed_dir, "a.json", title="Alpha") + seed.seed_public_scenarios(engine) + + (seed_dir / "a.json").unlink() + seed.seed_public_scenarios(engine) + db.expire_all() + assert titles(db) == {"Alpha"} diff --git a/docs/GUIDE.md b/docs/GUIDE.md index 76fe36e..8c489e2 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -954,6 +954,15 @@ guest survives with no re-parenting and no migration step. Three kinds of row sh users table: local (email NULL, not guest), guest (email NULL, guest), registered (email set). +**A seed file owns its scenario's whole life.** `seed.py` inserts a demo that is +missing, reconciles one whose file changed, and deletes a seeded row no file claims any +more. Matching is by title, so a rename needs the old name under `previous_titles` or it +inserts a second scenario and strands the first. Only rows with a NULL owner and +`is_public` are seeded rows, which is what keeps the sweep away from anything a player +made. An adventure started from a demo that is later removed survives: +`adventures.scenario_id` is `ON DELETE SET NULL`, and the adventure holds its own copies +of the cards and scripts, so it loses only the inherited cover art. + **Guests start with a story already in progress.** `starter.py` copies a shipped export bundle into each new guest account at the same point the row is created. An empty account gives a visitor nothing to read, and the daily demo turns are limited, so learning what diff --git a/plan/16-world-state-refusals.md b/plan/16-world-state-refusals.md index 88e7edd..108b75f 100644 --- a/plan/16-world-state-refusals.md +++ b/plan/16-world-state-refusals.md @@ -158,8 +158,11 @@ resize a maximized window. the narration ends mid-sentence with `finish_reason: length`. - **Every `hp` stat still has the `initial == max` shape.** Now visible when it bites, rather than silent, but not designed out. -- **The stale "Road to the Champion" scenario and adventure 42** are still on - production. Deleting them is hand-work and was deliberately not automated. +- ~~**The stale "Road to the Champion" scenario and adventure 42** are still on + production. Deleting them is hand-work and was deliberately not automated.~~ + Automated on 2026-08-28: `seed.py` now deletes seeded scenarios no seed file + claims. Adventure 42 survives with a NULL `scenario_id`, and losing the cover + art is the whole cost. - **The Bandit Camp demo (`04-rpg-world-state.json`) was not checked** for the same milestone problem. Its milestones were equally unnamed to the model before this change, so it is worth asking whether one has ever fired there. @@ -311,6 +314,15 @@ because SVG can carry script and these bytes are served from the app's own origin. `backend/tools/make_pokeball.py` draws the ball with `zlib` alone, so regenerating it needs no image library. +**Seeded scenarios no seed file claims are deleted.** `previous_titles` stops a +rename stranding a row, but the rows already stranded still needed deleting by +hand on every deployment. `_sweep_unclaimed` removes them on the next boot. Only +a NULL owner with `is_public` is reachable, so nothing a player made can be +touched, and `adventures.scenario_id` is `ON DELETE SET NULL`, so an adventure +started from a deleted demo keeps its story and loses only the artwork. The +sweep is skipped when a seed file fails to parse, and an empty seed directory +never reaches it: neither reads as an instruction to delete live content. + **Every new guest gets the played adventure.** `app/starter.py` copies a shipped export bundle into each new guest account, from the guest mint in `routers/auth.py`. The bundle is this session's adventure trimmed to its first @@ -318,6 +330,12 @@ two exchanges, which ends on the knockout and shows an applied change, a refused one, and a milestone. It stops before the Onix bug above, and the state it leaves has Onix at its own 90 HP so a guest can play on from it. +An adventure has no cover art of its own and inherits its scenario's, and a +bundle carries no scenario id, because an id is local to one database. The +starter file names its source under `scenarioTitle` instead, and +`starter._link_scenario` looks it up, so the copy shows the Pokeball rather than +a monogram tile. + The row building that `POST /adventures/import` did inline moved into `bundle.materialize`, which both callers now use. The limit and rate checks stayed in the endpoint: the starter writes a file the server ships, so it has no diff --git a/plan/STATUS.md b/plan/STATUS.md index d983b04..edd1924 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -237,11 +237,20 @@ first orphaned and public forever. This is the same failure this file already re "Road to the Champion". Seed files now carry `previous_titles`, and the rename lands on the existing row. Verified: the Pokemon demo kept its id. +**A seeded scenario nobody claims is now deleted on boot.** `previous_titles` +stops a rename stranding a row; the sweep removes the ones already stranded, which +retires "[Demo] Road to the Champion" without a console. Only rows with a NULL owner +and `is_public` are reachable, and `adventures.scenario_id` is `ON DELETE SET NULL`, so +an adventure started from a deleted demo keeps its story and loses only the cover art. +A seed file that fails to parse, or an empty seed directory, skips the sweep. + **Every new guest is given a pre-played adventure.** `app/starter.py` copies a shipped export bundle into each new guest account. The point is the first screen: real turns with their world-state chips, including a refused change and a milestone, before spending any of the daily demo turns. The row building inside `POST /adventures/import` moved to -`bundle.materialize` so both callers share one writer. +`bundle.materialize` so both callers share one writer. The copy is linked back to its +demo scenario by title, because an adventure has no art of its own and a bundle cannot +carry an id that means anything in another database. **An SVG data URI is not usable as scenario art.** `app/images.py` accepts raster formats only, deliberately, because SVG can carry script and the bytes are served from the app's @@ -944,7 +953,7 @@ the SQLite dev parity this codebase protects on purpose). ``` cd backend -.venv/Scripts/python.exe -m pytest tests/ # 539 tests (~180s) +.venv/Scripts/python.exe -m pytest tests/ # 549 tests (~180s) .venv/Scripts/python.exe -m tools.stress_session # egress report (SQLite) # Same harness against a real Postgres. The target must be a THROWAWAY database