Name the columns a list response carries
deferred=True keeps the four heavy Action columns out of a bulk read, but it makes narrowness the thing a future column has to remember to ask for -- and both egress blowouts this project has had were a column nobody remembered. Listing what each list response renders inverts the default: a new column costs nothing on these paths until someone adds it to the tuple. The adventures index was not merely a future risk. It loaded whole Adventure entities to render a title, a stamp and a snippet, and an Adventure carries script_state, world_state, placeholders, story_summary, memory, authors_note and ai_instructions -- ~15 kB a row in production, none of it on that screen, all of it fetched once per adventure on every index load. Measured on six adventures with 78 kB of body each: 469.7 kB entity-loaded against 318 B projected. The memories drawer stops walking adventure.memories. The walk is what retrieval used to do and the reason a turn cost megabytes; a relationship load takes whole entities, so it picks up whatever the model happens to grow. Nothing changes today -- embedding_blob is already deferred -- which is the point. world_delta stays on the action list because ActionOut.world_changes is computed from it. Leaving it off would not save the bytes, it would spend them one row at a time as a lazy load. Two tests cover the index: one asserts the listing query names none of the body columns, one puts a byte ceiling on six adventures carrying 80 kB apiece. 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
12d57afdac
commit
a6cb49293c
@@ -6,7 +6,7 @@ import threading
|
||||
from fastapi import APIRouter, Body, Depends, HTTPException, Request
|
||||
from fastapi.responses import StreamingResponse
|
||||
from sqlalchemy import func
|
||||
from sqlalchemy.orm import Session, undefer
|
||||
from sqlalchemy.orm import Session, load_only, undefer
|
||||
|
||||
from .. import auth, images, limits, memorybank, models, schemas, worldstate
|
||||
from ..context import build_context
|
||||
@@ -20,6 +20,44 @@ router = APIRouter(prefix="/api/adventures", tags=["adventures"])
|
||||
|
||||
CurrentUser = Depends(auth.get_current_user)
|
||||
|
||||
# Exactly what schemas.ActionOut renders, named rather than implied.
|
||||
#
|
||||
# `deferred=True` in models.py already keeps the four heavy columns out of a
|
||||
# bulk read, but it makes narrowness the default that a *future* column has to
|
||||
# remember to ask for — and both egress blowouts this project has had were a
|
||||
# column nobody remembered. Listing what a list response carries inverts that:
|
||||
# a new column costs nothing here until someone adds it to this tuple.
|
||||
#
|
||||
# `world_delta` is on the list because ActionOut.world_changes is computed from
|
||||
# it. Leaving it off would not save the bytes, it would spend them one row at a
|
||||
# time as a lazy load, which is worse.
|
||||
ACTION_LIST_COLUMNS = (
|
||||
models.Action.adventure_id,
|
||||
models.Action.index,
|
||||
models.Action.type,
|
||||
models.Action.text,
|
||||
models.Action.reasoning,
|
||||
models.Action.world_delta,
|
||||
models.Action.variant_count,
|
||||
models.Action.variant_index,
|
||||
models.Action.created_at,
|
||||
)
|
||||
|
||||
# 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 = (
|
||||
models.Memory.adventure_id,
|
||||
models.Memory.text,
|
||||
models.Memory.pinned,
|
||||
models.Memory.forgotten,
|
||||
models.Memory.embedded,
|
||||
models.Memory.use_count,
|
||||
models.Memory.last_used_at,
|
||||
models.Memory.source_start,
|
||||
models.Memory.source_end,
|
||||
models.Memory.created_at,
|
||||
)
|
||||
|
||||
|
||||
def get_adventure_or_404(
|
||||
adventure_id: int, db: Session, user: models.User
|
||||
@@ -85,9 +123,18 @@ def _latest_narration(db: Session, adventure_ids: list[int]) -> dict[int, str]:
|
||||
|
||||
@router.get("", response_model=list[schemas.AdventureListItem])
|
||||
def list_adventures(db: Session = Depends(get_db), user: models.User = CurrentUser):
|
||||
# Four columns of Adventure, named, rather than the entity. The entity is
|
||||
# sixteen columns wide and carries script_state, world_state, placeholders,
|
||||
# story_summary, memory, authors_note and ai_instructions — ~15 kB a row in
|
||||
# production, none of it on this screen, all of it fetched once per
|
||||
# adventure every time the index loads. Naming the columns also means the
|
||||
# next wide column added to Adventure has to opt *in* to being listed here.
|
||||
rows = (
|
||||
db.query(
|
||||
models.Adventure,
|
||||
models.Adventure.id,
|
||||
models.Adventure.scenario_id,
|
||||
models.Adventure.title,
|
||||
models.Adventure.updated_at,
|
||||
func.count(models.Action.id),
|
||||
models.Scenario.title,
|
||||
models.Scenario.image,
|
||||
@@ -112,22 +159,23 @@ def list_adventures(db: Session = Depends(get_db), user: models.User = CurrentUs
|
||||
.order_by(models.Adventure.updated_at.desc())
|
||||
.all()
|
||||
)
|
||||
narration = _latest_narration(db, [adv.id for adv, *_ in rows])
|
||||
narration = _latest_narration(db, [row[0] for row in rows])
|
||||
return [
|
||||
schemas.AdventureListItem(
|
||||
id=adv.id,
|
||||
scenario_id=adv.scenario_id,
|
||||
id=adv_id,
|
||||
scenario_id=scenario_id,
|
||||
scenario_title=scenario_title,
|
||||
title=adv.title,
|
||||
updated_at=adv.updated_at,
|
||||
title=title,
|
||||
updated_at=updated_at,
|
||||
action_count=count,
|
||||
snippet=_snippet(narration.get(adv.id, "")),
|
||||
snippet=_snippet(narration.get(adv_id, "")),
|
||||
# The art belongs to the scenario, so the cache-busting stamp is the
|
||||
# scenario's updated_at, not the adventure's.
|
||||
image_url=images.public_url(adv.scenario_id, image or "", scenario_updated),
|
||||
image_url=images.public_url(scenario_id, image or "", scenario_updated),
|
||||
icon=icon or "",
|
||||
)
|
||||
for adv, count, scenario_title, image, icon, scenario_updated in rows
|
||||
for (adv_id, scenario_id, title, updated_at, count,
|
||||
scenario_title, image, icon, scenario_updated) in rows
|
||||
]
|
||||
|
||||
|
||||
@@ -1455,7 +1503,19 @@ def action_context(
|
||||
def list_memories(
|
||||
adventure_id: int, db: Session = Depends(get_db), user: models.User = CurrentUser
|
||||
):
|
||||
return get_adventure_or_404(adventure_id, db, user).memories
|
||||
get_adventure_or_404(adventure_id, db, user)
|
||||
# A query naming its columns, not a walk of `adventure.memories`. The walk
|
||||
# is what retrieval used to do, and it is the reason a turn cost megabytes:
|
||||
# a relationship load takes whole entities, so it picks up whatever the
|
||||
# model happens to carry. `embedding_blob` is deferred and so would stay
|
||||
# out today — this is about the next wide column, not that one.
|
||||
return (
|
||||
db.query(models.Memory)
|
||||
.options(load_only(*MEMORY_LIST_COLUMNS))
|
||||
.filter(models.Memory.adventure_id == adventure_id)
|
||||
.order_by(models.Memory.id)
|
||||
.all()
|
||||
)
|
||||
|
||||
|
||||
@router.post("/{adventure_id}/memories", response_model=schemas.MemoryOut, status_code=201)
|
||||
@@ -1522,6 +1582,7 @@ def list_actions(
|
||||
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()
|
||||
|
||||
@@ -347,6 +347,74 @@ def test_reading_one_action_does_not_cost_the_whole_story(client, meter):
|
||||
)
|
||||
|
||||
|
||||
def _fat_adventures(user_id: int, count: int = 5, body: int = 20_000) -> None:
|
||||
"""Adventures whose bodies are heavy and whose index cards are not.
|
||||
|
||||
script_state, world_state and story_summary belong to the play screen. The
|
||||
index shows a title, a stamp and a snippet, and used to load all of it.
|
||||
"""
|
||||
db = SessionLocal()
|
||||
try:
|
||||
for i in range(count):
|
||||
db.add(models.Adventure(
|
||||
user_id=user_id,
|
||||
title=f"Adventure {i}",
|
||||
script_state={"log": "s" * body},
|
||||
world_state={"player": {"notes": "w" * body}},
|
||||
story_summary="y" * body,
|
||||
memory="m" * body,
|
||||
))
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
def test_the_index_does_not_read_the_adventure_body(client, sql_log):
|
||||
db = SessionLocal()
|
||||
try:
|
||||
user_id = db.query(models.User.id).first()[0]
|
||||
finally:
|
||||
db.close()
|
||||
_fat_adventures(user_id)
|
||||
|
||||
r = client.get("/api/adventures")
|
||||
assert r.status_code == 200
|
||||
assert len(r.json()) == 6 # the fixture's one, plus five
|
||||
|
||||
listing = [
|
||||
s for s in sql_log
|
||||
if "FROM adventures" in s and s.lstrip().upper().startswith("SELECT")
|
||||
]
|
||||
assert listing, "expected a listing query"
|
||||
for column in ("script_state", "world_state", "story_summary", "memory",
|
||||
"authors_note", "ai_instructions", "placeholders"):
|
||||
assert not any(column in s for s in listing), (
|
||||
f"the index read adventures.{column}, which nothing on that "
|
||||
f"screen displays"
|
||||
)
|
||||
|
||||
|
||||
def test_the_index_stays_under_its_byte_ceiling(client, meter):
|
||||
db = SessionLocal()
|
||||
try:
|
||||
user_id = db.query(models.User.id).first()[0]
|
||||
finally:
|
||||
db.close()
|
||||
_fat_adventures(user_id)
|
||||
|
||||
with meter.scope("index"):
|
||||
r = client.get("/api/adventures")
|
||||
assert r.status_code == 200
|
||||
|
||||
# Six adventures carrying 80 kB of body each. A card is a title, a stamp
|
||||
# and a 220-character snippet; 4 kB apiece is already generous.
|
||||
budget = 6 * 4_000
|
||||
assert fetched(meter) < budget, (
|
||||
f"the index fetched {fetched(meter):,} B for six adventures, over "
|
||||
f"{budget:,} B — it is reading the bodies again"
|
||||
)
|
||||
|
||||
|
||||
def test_the_ceiling_discriminates(client, meter):
|
||||
"""A ceiling is only worth having if the thing it excludes would breach it.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user