From 9398c13da5e3a94fba8c587f3fe558a6edff995c Mon Sep 17 00:00:00 2001 From: parththakkar106 Date: Fri, 28 Aug 2026 19:19:23 +0530 Subject: [PATCH] Delete seeded scenarios no seed file claims any more `previous_titles` stops a rename stranding the row it left behind, but the rows already stranded still had to be deleted by hand on every deployment. The seeder now removes them on the next boot, which retires the stale "Road to the Champion" demo without a database console. Only rows with a NULL owner and `is_public` are considered, and a player's own scenario is neither, so nothing anybody created is reachable. An adventure started from a deleted demo 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. Two cases skip the sweep, because neither is an instruction to remove live content: a seed file that fails to parse claims no title, and an empty seed directory reads as a packaging failure. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PacdRuPXSkQQy4ZYdH32hF --- README.md | 2 +- backend/app/seed.py | 60 +++++++++++- backend/tests/test_seed_sweep.py | 162 +++++++++++++++++++++++++++++++ docs/GUIDE.md | 9 ++ plan/16-world-state-refusals.md | 22 ++++- plan/STATUS.md | 13 ++- 6 files changed, 258 insertions(+), 10 deletions(-) create mode 100644 backend/tests/test_seed_sweep.py 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