diff --git a/backend/tests/test_memory_rewrite.py b/backend/tests/test_memory_rewrite.py index b8446b9..45f3e98 100644 --- a/backend/tests/test_memory_rewrite.py +++ b/backend/tests/test_memory_rewrite.py @@ -121,8 +121,9 @@ def fill_bank(db, adventure, *, count=2): def options(**overrides): - args = dict(write=False, adventure=None, limit=None, include_forgotten=False, - embed=False, endpoint=None, model=None, api_key=None) + args = dict(write=False, adventure=None, email=None, limit=None, + include_forgotten=False, embed=False, endpoint=None, + model=None, api_key=None) args.update(overrides) return argparse.Namespace(**args) @@ -358,6 +359,25 @@ def test_only_the_named_adventure_is_touched(db, monkeypatch): assert changed.text == "Kaelen entered the crypt 1." +def test_only_the_named_account_is_touched(db, monkeypatch): + """The hosted database holds other people's stories, and each adventure is + summarized with its owner's key.""" + mine = make_adventure(db, email="mine@example.com") + theirs = make_adventure(db, email="theirs@example.com") + kept, _ = fill_bank(db, theirs) + changed, _ = fill_bank(db, mine) + monkeypatch.setattr(memorybank, "summary_provider", lambda s: StubSummarizer()) + assert run_tool(options(write=True, email=["MINE@example.com"])) == 0 + db.expire_all() + assert kept.text == "You entered the crypt 1." + assert changed.text == "Kaelen entered the crypt 1." + + +def test_an_unknown_email_is_an_error(db): + make_adventure(db) + assert run_tool(options(email=["nobody@example.com"])) == 2 + + def test_an_unknown_adventure_id_is_an_error(db): make_adventure(db) assert run_tool(options(adventure=[9999])) == 2 diff --git a/backend/tools/rewrite_memories.py b/backend/tools/rewrite_memories.py index 490237f..98fad05 100644 --- a/backend/tools/rewrite_memories.py +++ b/backend/tools/rewrite_memories.py @@ -60,13 +60,32 @@ Without `--write` it makes no model calls, spends nothing, and only reports the scope. Every run reads the database the app reads: `AIDND_DB_PATH`, or `DATABASE_URL` for a hosted Postgres. Take a copy of it first — the old text is overwritten and is not kept anywhere. + +**On the hosted deploy, name whose adventures you mean.** That database holds +other people's stories, and each adventure is summarized with *its owner's* key, +so an unfiltered `--write` spends other people's money on memories they did not +ask to have rewritten. `--email` restricts the run to the accounts you name and +`--adventure` to single adventures; a dry run costs nothing and lists both, with +the owner of each. Guests have no email and can only be reached by id. + +Two environment variables reach that database from a checkout: + + AIDND_DATABASE_URL= \ + AIDND_SECRET_KEY= \ + python -m tools.rewrite_memories --email you@example.com + +`AIDND_SECRET_KEY` is not optional there. Stored API keys are encrypted with it, +and with the wrong one `decrypt_secret` returns "" and every adventure is skipped +as having no key (see `security.py`). The deployed image does not carry this +directory — the Dockerfile copies `backend/app` alone — so run it from a +checkout against the hosted database rather than from a shell on the box. """ import argparse import asyncio import sys from pathlib import Path -from sqlalchemy import inspect as sa_inspect +from sqlalchemy import func, inspect as sa_inspect, select def words(text: str) -> int: @@ -95,6 +114,20 @@ async def main(args) -> int: adventures = db.query(models.Adventure).order_by(models.Adventure.id) if args.adventure: adventures = adventures.filter(models.Adventure.id.in_(args.adventure)) + if args.email: + # Compared case-insensitively: an address is typed on the command line + # here and was typed into a registration form there. + wanted = [e.strip().lower() for e in args.email] + owners = db.execute( + select(models.User.id, func.lower(models.User.email)) + .where(func.lower(models.User.email).in_(wanted)) + ).all() + unknown = sorted(set(wanted) - {email for _, email in owners}) + if unknown: + print(f"no account with that email: {', '.join(unknown)}") + return 2 + adventures = adventures.filter( + models.Adventure.user_id.in_([user_id for user_id, _ in owners])) adventures = adventures.all() if args.adventure and len(adventures) != len(set(args.adventure)): found = {a.id for a in adventures} @@ -150,7 +183,10 @@ async def main(args) -> int: usable = settings is not None and bool( args.api_key or args.endpoint or settings.api_key_plain ) - print(f"\nadventure {adventure.id}: {adventure.title!r} " + owner = db.get(models.User, adventure.user_id) + who = (owner.email if owner and owner.email + else f"guest #{adventure.user_id}") + print(f"\nadventure {adventure.id}: {adventure.title!r} ({who}) " f"— {len(memories)} memories" + ("" if usable else " [owner has no API key in Settings]")) @@ -258,6 +294,9 @@ if __name__ == "__main__": help="rewrite. Without it, only report what would change.") parser.add_argument("--adventure", type=int, action="append", help="restrict to this adventure id; repeatable.") + parser.add_argument("--email", action="append", + help="restrict to this account's adventures; repeatable. " + "Use it on a shared database.") parser.add_argument("--limit", type=int, help="stop after rewriting this many memories.") parser.add_argument("--include-forgotten", action="store_true", diff --git a/plan/18-persona-and-memory-quality.md b/plan/18-persona-and-memory-quality.md index 5ad4644..0e54b28 100644 --- a/plan/18-persona-and-memory-quality.md +++ b/plan/18-persona-and-memory-quality.md @@ -4,7 +4,7 @@ Two changes, in order. Phase 1 gives the adventure a persona. Phase 2 uses it, along with the cast, to fix the memories. Phase 1 is worth shipping on its own; Phase 2 depends on it and is much smaller once it lands. -**Both phases are built and green (627 backend tests). Phase 1 was driven in a +**Both phases are built and green (629 backend tests). Phase 1 was driven in a browser (21/21 checks). Phase 2 was run end to end against a real model, as a controlled A/B on one story — see "Run with a real model". A bank written under the old prompt can be rewritten in place — see the last section.** @@ -615,6 +615,39 @@ a vector written from outside it can sit behind a stale cached copy until it restarts. Clearing alone is safe at any time: an unembedded memory leaves the catalogue, which is what the cache invalidates on. +## Running it against the hosted deploy + +Two things make production different from a local database, and both are easy +to get wrong quietly. + +**It holds other people's stories, and each adventure is summarized with its +owner's key.** An unfiltered `--write` would spend other people's money on +memories they did not ask to have rewritten. `--email` restricts a run to named +accounts and `--adventure` to single adventures; the dry run costs nothing and +prints the owner of each. Guests have no email and are reachable only by id, +which is the right amount of friction for rewriting a stranger's bank. + +**The stored API keys are encrypted with `AIDND_SECRET_KEY`.** Render generates +that value and holds it for the web service, so a run from a checkout has to +carry the same one. With a different secret, `decrypt_secret` returns "" rather +than failing, and every adventure is reported as having no key — a run that +looks like it worked and did nothing. + +``` +AIDND_DATABASE_URL= \ +AIDND_SECRET_KEY= \ + python -m tools.rewrite_memories --email you@example.com +``` + +Run it from a checkout rather than from a shell on Render. The image copies +`backend/app` alone, so `tools/` is not on the box, and the free plan has no +shell anyway. The database is the same one either way. + +Two smaller notes for that environment. The Neon URL to use is the direct +endpoint, not `-pooler`, for the same reason the sizing queries in STATUS use +it. And an adventure owned by a visitor playing on the shared demo key is +skipped, because summarization has never spent that key. + **The story summary is not rewritten.** It is one text per adventure rather than a bank, and `_update_story_summary` hands the model the whole of it and asks for an updated version under the new framing rule, so the next scheduled update diff --git a/plan/STATUS.md b/plan/STATUS.md index 3d87162..1374dd1 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -81,7 +81,7 @@ needed; nothing requires reading a row of anyone's story. **`plan/18-persona-and-memory-quality.md` is the writeup. Both changes are on `main`** — the note here that said they were sitting unmerged on `claude/ai-dnd-memories-summarization-3muo98` is out of date; `main` is at `71b24b6`, -the tip of that work. Both green at 627 tests, plus the backfill below. +the tip of that work. Both green at 629 tests, plus the backfill below. **The protagonist now has a name.** An adventure carries `persona_name`, `persona_pronouns` and `persona_desc` (migrations 74-76), and the player's stat block @@ -137,7 +137,21 @@ python -m tools.rewrite_memories --write --embed # the whole backfill It reads whichever database the app reads (`AIDND_DB_PATH`, or `DATABASE_URL` on the hosted deploy), so **take a copy first** — the old text is overwritten and kept nowhere. `--endpoint`/`--model`/`--api-key` point the summarizer somewhere else, `claude_shim.py` -included. Hand-written memories, and memories whose actions have been deleted, are left +included. + +**Against production, name whose adventures you mean.** That database holds other +people's stories and each adventure is summarized with its owner's key, so `--email` +(or `--adventure`) is what keeps a run to your own. Reach it from a checkout, not from +a shell on Render — the image copies `backend/app` alone, so `tools/` is not on the box: + +``` +AIDND_DATABASE_URL= AIDND_SECRET_KEY= \ + python -m tools.rewrite_memories --email you@example.com +``` + +`AIDND_SECRET_KEY` is not optional there: stored API keys are encrypted with it, and +with the wrong one `decrypt_secret` returns "" and every adventure is reported as having +no key — a run that looks fine and does nothing. Hand-written memories, and memories whose actions have been deleted, are left alone; so is an adventure whose owner has no API key, because summarization spends the user's own key and never the demo key.