Open an adventure on a window of the story, and page upward
Opening a finished adventure fetched every action in one response: 589.5 kB on
production's longest, and nothing about that curve bends on its own, because a
story only ever gets longer. The page load now brings the newest 60 actions
and the reader pages up from there. On the harness's 600-action fixture that
is 606.0 kB down to 62.6 kB, and -- the part that matters -- it no longer
depends on how long the story is.
Paged by anchor, not by offset. `before_id` is the oldest action the caller
holds; the server returns what precedes it. An offset counted back from the
newest would shift every older position the moment a turn lands, which is
exactly when someone is likely to be scrolling, and the reader would get one
action twice and never see another. It also keeps working when the story stops
being a flat list: comparing indices to order a branch survives the story tree,
treating them as positions does not.
`has_more` comes from fetching one row past the window rather than from
counting. A deleted anchor -- undo, mid-scroll -- reports the end rather than
guessing and serving a page the reader already has.
GET /{id}/actions and POST /{id}/undo now return {actions, total, has_more}
instead of a bare list. Undo is the action most likely to be repeated several
times running, so having it re-fetch the whole story would have undone the
paging on the worst case.
The adventure payload gets its window through set_committed_value rather than
by assignment: the actions relationship cascades delete-orphan, so assigning a
60-item list to it would delete everything outside the window on the next
flush.
In Play.jsx the prepend is followed by a useLayoutEffect that restores the
scroll position, before paint, so the story does not jump. Loading starts 400px
from the top rather than at it, guarded by a ref because scroll fires far
faster than React re-renders. There is a button as well as the scroll trigger:
on a short viewport the transcript may not be tall enough to scroll at all, and
a reader who cannot scroll must still be able to reach the beginning.
Verified against a running backend and a 220-action adventure: the page load
returns 60 of 220 ending on the newest, and walking back from an anchor returns
exactly the actions before it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Dvvqn9ZDR4ixeFPHNbww7
This commit is contained in:
co-authored by
Claude Opus 5
parent
ae6e5af6c7
commit
cf8ec22e8b
@@ -7,6 +7,7 @@ from fastapi import APIRouter, Body, Depends, HTTPException, Request
|
||||
from fastapi.responses import StreamingResponse
|
||||
from sqlalchemy import func
|
||||
from sqlalchemy.orm import Session, load_only, undefer
|
||||
from sqlalchemy.orm.attributes import set_committed_value
|
||||
|
||||
from .. import auth, images, limits, memorybank, models, schemas, worldstate
|
||||
from ..context import build_context
|
||||
@@ -43,6 +44,76 @@ ACTION_LIST_COLUMNS = (
|
||||
models.Action.created_at,
|
||||
)
|
||||
|
||||
# How many actions an adventure opens with, and how many arrive per scroll.
|
||||
#
|
||||
# Opening a finished adventure used to fetch the whole story in one response —
|
||||
# 589.5 kB on production's longest, and growing, because a story only ever gets
|
||||
# longer. 60 is a few screens of reading: enough that the common case (open,
|
||||
# read the end, take a turn) never pages at all, small enough that the worst
|
||||
# case is bounded by the window rather than by the story.
|
||||
ACTION_PAGE = 60
|
||||
|
||||
|
||||
def action_window(
|
||||
db: Session,
|
||||
adventure_id: int,
|
||||
before_id: int | None = None,
|
||||
limit: int = ACTION_PAGE,
|
||||
) -> tuple[list[models.Action], int, bool]:
|
||||
"""The `limit` actions immediately older than `before_id`, oldest first.
|
||||
|
||||
Returns (actions, total, has_more). `before_id=None` is the newest window.
|
||||
|
||||
Anchored on an action, not on a count, and never on arithmetic over
|
||||
`Action.index`. Two separate reasons, and both bite:
|
||||
|
||||
* **Appends.** Counting back from the newest means every older position
|
||||
shifts when a turn lands. A reader who scrolls up while a turn is
|
||||
generating would be handed a window one row out — re-sending one action
|
||||
and silently skipping another. An anchor is fixed: "older than this one"
|
||||
means the same thing before and after the story grows.
|
||||
* **The story tree.** Index is a dense 0..n sequence today and branching
|
||||
ends that. Comparing indices to order a branch survives; treating them as
|
||||
positions does not.
|
||||
|
||||
`has_more` comes from asking for one row past the window rather than from
|
||||
counting, so it costs a row and not a scan.
|
||||
"""
|
||||
total = (
|
||||
db.query(func.count(models.Action.id))
|
||||
.filter(models.Action.adventure_id == adventure_id)
|
||||
.scalar()
|
||||
)
|
||||
if limit <= 0:
|
||||
return [], total, total > 0
|
||||
|
||||
query = (
|
||||
db.query(models.Action)
|
||||
.options(load_only(*ACTION_LIST_COLUMNS))
|
||||
.filter(models.Action.adventure_id == adventure_id)
|
||||
)
|
||||
if before_id is not None:
|
||||
anchor = (
|
||||
db.query(models.Action.index)
|
||||
.filter(models.Action.id == before_id,
|
||||
models.Action.adventure_id == adventure_id)
|
||||
.scalar()
|
||||
)
|
||||
if anchor is None:
|
||||
# The anchor was deleted (undo, or a turn edited away) while the
|
||||
# reader was scrolling. Nothing older can be identified relative to
|
||||
# a row that no longer exists, so report the end rather than
|
||||
# guessing and handing back a duplicate page.
|
||||
return [], total, False
|
||||
query = query.filter(models.Action.index < anchor)
|
||||
|
||||
rows = query.order_by(models.Action.index.desc()).limit(limit + 1).all()
|
||||
has_more = len(rows) > limit
|
||||
rows = rows[:limit]
|
||||
rows.reverse()
|
||||
return rows, total, has_more
|
||||
|
||||
|
||||
# Exactly what schemas.MemoryOut renders. `embedded` is a real column and is on
|
||||
# the list; the vector it describes is not, and must never be.
|
||||
MEMORY_LIST_COLUMNS = (
|
||||
@@ -303,7 +374,25 @@ def create_adventure(
|
||||
def get_adventure(
|
||||
adventure_id: int, db: Session = Depends(get_db), user: models.User = CurrentUser
|
||||
):
|
||||
return get_adventure_or_404(adventure_id, db, user)
|
||||
"""The adventure, and the newest window of its story.
|
||||
|
||||
`actions` is the last ACTION_PAGE, not all of them; `action_count` says how
|
||||
many there are so the reader knows there is more above. Older pages come
|
||||
from GET /{id}/actions as they scroll up.
|
||||
"""
|
||||
adventure = get_adventure_or_404(adventure_id, db, user)
|
||||
actions, total, _ = action_window(db, adventure_id)
|
||||
# Hand the response the window as if the relationship had loaded it.
|
||||
# `set_committed_value` is the only way to do this safely: assigning
|
||||
# `adventure.actions = [...]` marks the collection dirty, and the
|
||||
# relationship cascades delete-orphan, so the actions left out of the
|
||||
# window would be deleted on the next flush. This records them as the
|
||||
# loaded, unmodified value instead, so serialising touches no lazy load
|
||||
# and nothing is pending.
|
||||
set_committed_value(adventure, "actions", actions)
|
||||
out = schemas.AdventureOut.model_validate(adventure)
|
||||
out.action_count = total
|
||||
return out
|
||||
|
||||
|
||||
@router.get("/{adventure_id}/script-state")
|
||||
@@ -950,7 +1039,7 @@ def select_variant(
|
||||
_active_turns.discard(adventure_id)
|
||||
|
||||
|
||||
@router.post("/{adventure_id}/undo", response_model=list[schemas.ActionOut])
|
||||
@router.post("/{adventure_id}/undo", response_model=schemas.ActionPage)
|
||||
def undo_turn(
|
||||
adventure_id: int, db: Session = Depends(get_db), user: models.User = CurrentUser
|
||||
):
|
||||
@@ -992,7 +1081,16 @@ def undo_turn(
|
||||
memorybank.prune_dangling_memories(adventure, db)
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
return adventure.actions
|
||||
# The newest window, not the whole story: the client replaces its
|
||||
# transcript with this, and the transcript is a window now. Returning
|
||||
# everything here would undo the paging on the one action most likely
|
||||
# to be repeated several times in a row.
|
||||
actions, total, has_more = action_window(db, adventure_id)
|
||||
return schemas.ActionPage(
|
||||
actions=[schemas.ActionOut.model_validate(a) for a in actions],
|
||||
total=total,
|
||||
has_more=has_more,
|
||||
)
|
||||
finally:
|
||||
_active_turns.discard(adventure_id)
|
||||
|
||||
@@ -1575,17 +1673,29 @@ def delete_memory(
|
||||
|
||||
# ---------- Actions (CRUD) ----------
|
||||
|
||||
@router.get("/{adventure_id}/actions", response_model=list[schemas.ActionOut])
|
||||
@router.get("/{adventure_id}/actions", response_model=schemas.ActionPage)
|
||||
def list_actions(
|
||||
adventure_id: int, db: Session = Depends(get_db), user: models.User = CurrentUser
|
||||
adventure_id: int,
|
||||
before_id: int | None = None,
|
||||
limit: int = ACTION_PAGE,
|
||||
db: Session = Depends(get_db),
|
||||
user: models.User = CurrentUser,
|
||||
):
|
||||
"""A page of the story, walking backwards from the newest action.
|
||||
|
||||
`before_id` is the oldest action the caller already holds, so scrolling up
|
||||
is "give me what comes before this". Omit it for the newest window. See
|
||||
action_window for why this anchors on a row rather than an offset.
|
||||
"""
|
||||
get_adventure_or_404(adventure_id, db, user)
|
||||
return (
|
||||
db.query(models.Action)
|
||||
.options(load_only(*ACTION_LIST_COLUMNS))
|
||||
.filter(models.Action.adventure_id == adventure_id)
|
||||
.order_by(models.Action.index)
|
||||
.all()
|
||||
limit = max(1, min(limit, ACTION_PAGE * 4))
|
||||
actions, total, has_more = action_window(
|
||||
db, adventure_id, before_id=before_id, limit=limit
|
||||
)
|
||||
return schemas.ActionPage(
|
||||
actions=[schemas.ActionOut.model_validate(a) for a in actions],
|
||||
total=total,
|
||||
has_more=has_more,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -225,7 +225,21 @@ class AdventureOut(ORMModel):
|
||||
created_at: datetime
|
||||
updated_at: datetime
|
||||
story_cards: list[StoryCardOut] = []
|
||||
# The NEWEST window of the story, not all of it — older pages arrive from
|
||||
# GET /{id}/actions as the reader scrolls up. `action_count` is the whole
|
||||
# story's length, which is how the client knows there is more above.
|
||||
actions: list[ActionOut] = []
|
||||
action_count: int = 0
|
||||
|
||||
|
||||
class ActionPage(BaseModel):
|
||||
"""A slice of the story, counted back from the newest action."""
|
||||
|
||||
actions: list[ActionOut] = []
|
||||
total: int = 0
|
||||
# Whether anything older than this slice exists. Computed server-side so
|
||||
# the client never has to do arithmetic on positions to find the end.
|
||||
has_more: bool = False
|
||||
|
||||
|
||||
# ---------- Memory bank (Phase 6) ----------
|
||||
|
||||
@@ -0,0 +1,240 @@
|
||||
"""Opening an adventure fetches a window, not the whole story.
|
||||
|
||||
A story only ever gets longer. Production's longest is 607 actions and 589.5 kB
|
||||
in one response, and nothing about that curve bends on its own — so the page
|
||||
load returns the newest ACTION_PAGE and the reader pages upward.
|
||||
|
||||
The paging anchors on an action id rather than an offset, and these tests are
|
||||
mostly about why. An offset counted back from the newest shifts every older
|
||||
position the moment a turn lands, which is precisely when a reader is likely
|
||||
to be scrolling. An anchor means the same thing before and after.
|
||||
|
||||
python -m pytest tests/test_action_paging.py -v
|
||||
"""
|
||||
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 fastapi import Depends
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app import auth, limits, models
|
||||
from app.database import Base, SessionLocal, engine, get_db
|
||||
from app.main import app
|
||||
from app.routers.adventures import ACTION_PAGE
|
||||
from tools import dbmeter
|
||||
|
||||
TOTAL = ACTION_PAGE * 3 + 7 # deliberately not a whole number of pages
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def client(monkeypatch):
|
||||
Base.metadata.create_all(bind=engine)
|
||||
setup = SessionLocal()
|
||||
user = models.User(is_guest=False, email="paging@example.com")
|
||||
setup.add(user)
|
||||
setup.flush()
|
||||
setup.add(models.Settings(user_id=user.id, api_key="enc:dummy", model="m"))
|
||||
adventure = models.Adventure(user_id=user.id, title="Cave", script_state={})
|
||||
setup.add(adventure)
|
||||
setup.flush()
|
||||
for i in range(TOTAL):
|
||||
setup.add(models.Action(
|
||||
adventure_id=adventure.id, index=i,
|
||||
type="start" if i == 0 else ("ai" if i % 2 else "do"),
|
||||
text=f"Action {i}." + "word " * 200,
|
||||
))
|
||||
setup.commit()
|
||||
adv_id, user_id = adventure.id, user.id
|
||||
setup.close()
|
||||
|
||||
monkeypatch.setattr(limits, "rate_limit", lambda *a, **k: None)
|
||||
monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None)
|
||||
|
||||
def _current_user(db=Depends(get_db)):
|
||||
return db.get(models.User, user_id)
|
||||
|
||||
app.dependency_overrides[auth.get_current_user] = _current_user
|
||||
c = TestClient(app)
|
||||
c.adv_id = adv_id
|
||||
try:
|
||||
yield c
|
||||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
Base.metadata.drop_all(bind=engine)
|
||||
|
||||
|
||||
def page(client, before_id=None, limit=None):
|
||||
params = {}
|
||||
if before_id is not None:
|
||||
params["before_id"] = before_id
|
||||
if limit is not None:
|
||||
params["limit"] = limit
|
||||
r = client.get(f"/api/adventures/{client.adv_id}/actions", params=params)
|
||||
assert r.status_code == 200, r.text
|
||||
return r.json()
|
||||
|
||||
|
||||
def add_action(client, text="A new turn.") -> int:
|
||||
db = SessionLocal()
|
||||
try:
|
||||
highest = db.query(models.Action.index).order_by(
|
||||
models.Action.index.desc()).first()[0]
|
||||
action = models.Action(
|
||||
adventure_id=client.adv_id, index=highest + 1, type="ai", text=text
|
||||
)
|
||||
db.add(action)
|
||||
db.commit()
|
||||
return action.id
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
# ------------------------------------------------------------- the page load
|
||||
|
||||
def test_the_page_load_returns_only_the_newest_window(client):
|
||||
r = client.get(f"/api/adventures/{client.adv_id}")
|
||||
assert r.status_code == 200
|
||||
body = r.json()
|
||||
assert len(body["actions"]) == ACTION_PAGE
|
||||
assert body["action_count"] == TOTAL
|
||||
# ...and it is the *newest* window, ending on the last action.
|
||||
assert body["actions"][-1]["index"] == TOTAL - 1
|
||||
assert body["actions"][0]["index"] == TOTAL - ACTION_PAGE
|
||||
|
||||
|
||||
def test_a_short_story_is_returned_whole(client):
|
||||
db = SessionLocal()
|
||||
try:
|
||||
db.query(models.Action).filter(models.Action.index >= 5).delete()
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
body = client.get(f"/api/adventures/{client.adv_id}").json()
|
||||
assert len(body["actions"]) == 5
|
||||
assert body["action_count"] == 5
|
||||
|
||||
|
||||
def test_the_page_load_does_not_grow_with_the_story(client):
|
||||
"""The point of the change. Whatever the story's length, opening it costs
|
||||
a window."""
|
||||
meter = dbmeter.Meter()
|
||||
meter.attach(engine)
|
||||
try:
|
||||
with meter.scope("page load"):
|
||||
client.get(f"/api/adventures/{client.adv_id}")
|
||||
windowed = meter.scopes[-1].total.fetched
|
||||
finally:
|
||||
meter.detach()
|
||||
|
||||
# Each action carries ~1 kB of text and there are 187 of them; a window is
|
||||
# 60. Generous ceiling, but far below the whole story.
|
||||
assert windowed < ACTION_PAGE * 2_000, f"{windowed:,} B for one window"
|
||||
assert windowed < TOTAL * 500, (
|
||||
f"{windowed:,} B — that is the whole story, not a window"
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------ paging up
|
||||
|
||||
def test_the_first_page_is_the_newest(client):
|
||||
body = page(client)
|
||||
assert len(body["actions"]) == ACTION_PAGE
|
||||
assert body["total"] == TOTAL
|
||||
assert body["has_more"] is True
|
||||
assert body["actions"][-1]["index"] == TOTAL - 1
|
||||
|
||||
|
||||
def test_paging_up_covers_the_whole_story_exactly_once(client):
|
||||
seen = []
|
||||
body = page(client)
|
||||
seen = [a["index"] for a in body["actions"]]
|
||||
guard = 0
|
||||
while body["has_more"]:
|
||||
guard += 1
|
||||
assert guard < 20, "paging did not terminate"
|
||||
body = page(client, before_id=body["actions"][0]["id"])
|
||||
seen = [a["index"] for a in body["actions"]] + seen
|
||||
|
||||
assert seen == list(range(TOTAL)), "gap, duplicate or reordering while paging"
|
||||
|
||||
|
||||
def test_has_more_is_false_at_the_beginning_of_the_story(client):
|
||||
body = page(client)
|
||||
while body["has_more"]:
|
||||
body = page(client, before_id=body["actions"][0]["id"])
|
||||
assert body["actions"][0]["index"] == 0
|
||||
|
||||
|
||||
def test_each_page_is_ordered_oldest_first(client):
|
||||
body = page(client)
|
||||
indices = [a["index"] for a in body["actions"]]
|
||||
assert indices == sorted(indices)
|
||||
|
||||
|
||||
# ------------------------------------------------- the reason for the anchor
|
||||
|
||||
def test_a_turn_arriving_mid_scroll_does_not_shift_the_next_page(client):
|
||||
"""The failure an offset would have. Read the newest page, let a turn land,
|
||||
then page up: the reader must get exactly what precedes what they hold —
|
||||
no duplicate, no skipped action."""
|
||||
first = page(client)
|
||||
oldest_held = first["actions"][0]
|
||||
|
||||
add_action(client)
|
||||
|
||||
older = page(client, before_id=oldest_held["id"])
|
||||
assert older["actions"][-1]["index"] == oldest_held["index"] - 1, \
|
||||
"the page shifted when a turn landed"
|
||||
assert all(a["index"] < oldest_held["index"] for a in older["actions"])
|
||||
# The new turn moved the total, which is fine — it must not move the window.
|
||||
assert older["total"] == TOTAL + 1
|
||||
|
||||
|
||||
def test_a_deleted_anchor_reports_the_end_rather_than_a_duplicate_page(client):
|
||||
"""Undo can remove the action a slow scroll was anchored to. Better to stop
|
||||
than to hand back a page the reader already has."""
|
||||
body = page(client)
|
||||
anchor = body["actions"][0]
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
db.query(models.Action).filter(models.Action.id == anchor["id"]).delete()
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
after = page(client, before_id=anchor["id"])
|
||||
assert after["actions"] == []
|
||||
assert after["has_more"] is False
|
||||
|
||||
|
||||
# ------------------------------------------------------------------- limits
|
||||
|
||||
def test_limit_is_honoured_and_capped(client):
|
||||
assert len(page(client, limit=5)["actions"]) == 5
|
||||
# A client asking for the whole story does not get to undo the paging.
|
||||
assert len(page(client, limit=100_000)["actions"]) <= ACTION_PAGE * 4
|
||||
|
||||
|
||||
def test_a_nonsense_limit_still_returns_something(client):
|
||||
assert len(page(client, limit=0)["actions"]) >= 1
|
||||
assert len(page(client, limit=-5)["actions"]) >= 1
|
||||
|
||||
|
||||
# --------------------------------------------------------------------- undo
|
||||
|
||||
def test_undo_returns_a_window_not_the_story(client):
|
||||
r = client.post(f"/api/adventures/{client.adv_id}/undo")
|
||||
assert r.status_code == 200, r.text
|
||||
body = r.json()
|
||||
assert len(body["actions"]) == ACTION_PAGE
|
||||
assert body["total"] == TOTAL - 1
|
||||
assert body["has_more"] is True
|
||||
@@ -294,7 +294,8 @@ def test_undo_removes_the_action_and_its_history(client):
|
||||
_retry(client)
|
||||
r = client.post(f"/api/adventures/{client.adv_id}/undo")
|
||||
assert r.status_code == 200, r.text
|
||||
assert [a["type"] for a in r.json()] == ["start"]
|
||||
# Undo returns the newest window now, not the whole story.
|
||||
assert [a["type"] for a in r.json()["actions"]] == ["start"]
|
||||
assert _adv(client.adv_id)[0] == {}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user