Put a number on what an endpoint may fetch, not just a column list
test_egress.py asserted which columns a statement names, which is the shape both of this project's egress blowouts took. It would all still pass if a response grew tenfold within the columns it is allowed to read -- and a story that keeps getting longer does exactly that. Production's longest adventure is 607 actions where the plan assumed 200. So dbmeter, which was built to be importable from tests and was not yet used by any, now backs four byte ceilings: the page load, the action list, and one action's snapshot fetched on demand. Budgets are per action rather than absolute, so they mean the same thing whatever size the fixture is set to, and generous -- 3 kB against a real 994 B. They are there to catch an order of magnitude, not to freeze a byte count. The fourth test is the one that keeps the other three honest. A ceiling proves nothing unless the thing it excludes would breach it, so it undefers the snapshot on purpose and asserts the same twelve rows cost more than ten times the budget. If the fixture ever shrinks below the point where that holds, that test fails rather than the ceilings quietly passing on nothing. Meter grows detach() and a context manager. A script exits and takes the wrapping with it; a test does not, and one test leaving the shared engine metered would charge bytes to a scope nobody opened. 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
be66780a26
commit
12d57afdac
@@ -1,9 +1,19 @@
|
|||||||
"""Guards on how much the database is asked for.
|
"""Guards on how much the database is asked for.
|
||||||
|
|
||||||
context_snapshot holds the entire assembled prompt for a turn (~74 KB/row in
|
context_snapshot holds the entire assembled prompt for a turn — 163 KB a row
|
||||||
production, 94% of the database). It used to be pulled for every action on
|
averaged over production, 232 KB on the longest adventure, and 89% of the
|
||||||
every adventure load and every turn, to read two tiny things out of it. These
|
database. It used to be pulled for every action on every adventure load and
|
||||||
tests fail if that regresses.
|
every turn, to read two tiny things out of it. These tests fail if that
|
||||||
|
regresses.
|
||||||
|
|
||||||
|
Two kinds of guard live here, and both are needed:
|
||||||
|
|
||||||
|
* **column guards** assert which columns a statement names. That is the shape
|
||||||
|
both of this project's egress blowouts took — one query quietly carrying a
|
||||||
|
column nobody read.
|
||||||
|
* **byte ceilings** assert what a request actually costs. Every column guard
|
||||||
|
would still pass if a response grew tenfold within the columns it is allowed
|
||||||
|
to read, which is what a story that keeps getting longer does.
|
||||||
|
|
||||||
python -m pytest tests/test_egress.py -v
|
python -m pytest tests/test_egress.py -v
|
||||||
"""
|
"""
|
||||||
@@ -16,15 +26,19 @@ os.environ["AIDND_DB_PATH"] = _tmp.name
|
|||||||
os.environ.pop("AIDND_DATABASE_URL", None)
|
os.environ.pop("AIDND_DATABASE_URL", None)
|
||||||
os.environ.pop("DATABASE_URL", None)
|
os.environ.pop("DATABASE_URL", None)
|
||||||
|
|
||||||
|
import json
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from fastapi import Depends
|
from fastapi import Depends
|
||||||
from fastapi.testclient import TestClient
|
from fastapi.testclient import TestClient
|
||||||
from sqlalchemy import event, text
|
from sqlalchemy import event, text
|
||||||
|
from sqlalchemy.orm import undefer
|
||||||
|
|
||||||
from app import auth, limits, migrations, models
|
from app import auth, limits, migrations, models
|
||||||
from app.context import history
|
from app.context import history
|
||||||
from app.database import Base, SessionLocal, engine, get_db
|
from app.database import Base, SessionLocal, engine, get_db
|
||||||
from app.main import app
|
from app.main import app
|
||||||
|
from tools import dbmeter
|
||||||
|
|
||||||
# A stand-in for the real thing: the assembled prompt, which is what makes the
|
# A stand-in for the real thing: the assembled prompt, which is what makes the
|
||||||
# column enormous, plus the small world_state slice the UI actually needs.
|
# column enormous, plus the small world_state slice the UI actually needs.
|
||||||
@@ -250,3 +264,112 @@ def test_backfill_leaves_actions_without_world_state_alone(client):
|
|||||||
assert all(a.world_delta is None for a in db.query(models.Action).all())
|
assert all(a.world_delta is None for a in db.query(models.Action).all())
|
||||||
finally:
|
finally:
|
||||||
db.close()
|
db.close()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------- byte ceilings
|
||||||
|
#
|
||||||
|
# The tests above assert which *columns* a statement names, which is the shape
|
||||||
|
# both of this project's egress blowouts took. They would all still pass if a
|
||||||
|
# response quietly grew tenfold within the columns it is allowed to read — and
|
||||||
|
# a story that keeps getting longer does exactly that. These put a number on it.
|
||||||
|
#
|
||||||
|
# Ceilings are per action rather than absolute, so they mean the same thing
|
||||||
|
# whatever size the fixture is set to, and they are generous: the point is to
|
||||||
|
# catch a tenfold regression, not to freeze today's byte count.
|
||||||
|
|
||||||
|
ACTIONS_IN_FIXTURE = 12
|
||||||
|
|
||||||
|
# 3 kB an action against a real 994 B, measured on production 2026-08-17.
|
||||||
|
# Anything that pulls a deferred column blows past this by two orders of
|
||||||
|
# magnitude — see test_the_ceiling_discriminates below.
|
||||||
|
PAGE_LOAD_BYTES_PER_ACTION = 3_000
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture()
|
||||||
|
def meter():
|
||||||
|
"""A byte meter on the shared engine, removed again afterwards.
|
||||||
|
|
||||||
|
Requested *after* `client` in a test's arguments so that building the
|
||||||
|
fixture — a write path nobody plays — is not charged to any scope.
|
||||||
|
"""
|
||||||
|
m = dbmeter.Meter()
|
||||||
|
m.attach(engine)
|
||||||
|
try:
|
||||||
|
yield m
|
||||||
|
finally:
|
||||||
|
m.detach()
|
||||||
|
|
||||||
|
|
||||||
|
def fetched(meter) -> int:
|
||||||
|
return meter.scopes[-1].total.fetched
|
||||||
|
|
||||||
|
|
||||||
|
def test_page_load_stays_under_its_byte_ceiling(client, meter):
|
||||||
|
with meter.scope("page load"):
|
||||||
|
r = client.get(f"/api/adventures/{client.adv_id}")
|
||||||
|
assert r.status_code == 200
|
||||||
|
|
||||||
|
budget = ACTIONS_IN_FIXTURE * PAGE_LOAD_BYTES_PER_ACTION
|
||||||
|
assert fetched(meter) < budget, (
|
||||||
|
f"page load fetched {fetched(meter):,} B for {ACTIONS_IN_FIXTURE} "
|
||||||
|
f"actions, over the {budget:,} B budget"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_action_list_stays_under_its_byte_ceiling(client, meter):
|
||||||
|
with meter.scope("action list"):
|
||||||
|
r = client.get(f"/api/adventures/{client.adv_id}/actions")
|
||||||
|
assert r.status_code == 200
|
||||||
|
|
||||||
|
budget = ACTIONS_IN_FIXTURE * PAGE_LOAD_BYTES_PER_ACTION
|
||||||
|
assert fetched(meter) < budget, (
|
||||||
|
f"the action list fetched {fetched(meter):,} B, over {budget:,} B"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_reading_one_action_does_not_cost_the_whole_story(client, meter):
|
||||||
|
"""The snapshot is reachable on demand, and that request should pay for
|
||||||
|
one row's worth — not the adventure's."""
|
||||||
|
db = SessionLocal()
|
||||||
|
try:
|
||||||
|
action_id = db.query(models.Action.id).order_by(models.Action.id).first()[0]
|
||||||
|
finally:
|
||||||
|
db.close()
|
||||||
|
|
||||||
|
with meter.scope("one snapshot"):
|
||||||
|
r = client.get(f"/api/adventures/{client.adv_id}/actions/{action_id}/context")
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
|
||||||
|
one_snapshot = len(json.dumps(BIG_SNAPSHOT))
|
||||||
|
assert fetched(meter) < one_snapshot * 2, (
|
||||||
|
f"fetching one action's snapshot cost {fetched(meter):,} B; one "
|
||||||
|
f"snapshot is {one_snapshot:,} B"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_ceiling_discriminates(client, meter):
|
||||||
|
"""A ceiling is only worth having if the thing it excludes would breach it.
|
||||||
|
|
||||||
|
This is the regression the byte tests exist to catch, performed on purpose:
|
||||||
|
undefer the snapshot and the same twelve rows cost two orders of magnitude
|
||||||
|
more. If this ever stops exceeding the budget, the fixture has gone too
|
||||||
|
small for the tests above to mean anything.
|
||||||
|
"""
|
||||||
|
budget = ACTIONS_IN_FIXTURE * PAGE_LOAD_BYTES_PER_ACTION
|
||||||
|
db = SessionLocal()
|
||||||
|
try:
|
||||||
|
with meter.scope("undeferred"):
|
||||||
|
rows = (
|
||||||
|
db.query(models.Action)
|
||||||
|
.options(undefer(models.Action.context_snapshot))
|
||||||
|
.all()
|
||||||
|
)
|
||||||
|
assert len(rows) == ACTIONS_IN_FIXTURE
|
||||||
|
finally:
|
||||||
|
db.close()
|
||||||
|
|
||||||
|
assert fetched(meter) > budget * 10, (
|
||||||
|
"undeferring the snapshot cost only "
|
||||||
|
f"{fetched(meter):,} B — the fixture is too small for the byte "
|
||||||
|
"ceilings above to catch anything"
|
||||||
|
)
|
||||||
|
|||||||
@@ -174,7 +174,26 @@ class Meter:
|
|||||||
|
|
||||||
metered_creator._dbmeter = self
|
metered_creator._dbmeter = self
|
||||||
pool._creator = metered_creator
|
pool._creator = metered_creator
|
||||||
self._attached_pools.append(pool)
|
self._attached_pools.append((engine, pool, creator))
|
||||||
|
|
||||||
|
def detach(self) -> None:
|
||||||
|
"""Put every metered engine back as it was.
|
||||||
|
|
||||||
|
A script exits and takes the wrapping with it; a test does not, and one
|
||||||
|
test leaving the shared engine metered would go on charging bytes to a
|
||||||
|
scope nobody opened. Pooled connections are dropped again on the way
|
||||||
|
out for the same reason attach drops them on the way in.
|
||||||
|
"""
|
||||||
|
while self._attached_pools:
|
||||||
|
engine, pool, creator = self._attached_pools.pop()
|
||||||
|
pool._creator = creator
|
||||||
|
engine.dispose()
|
||||||
|
|
||||||
|
def __enter__(self) -> "Meter":
|
||||||
|
return self
|
||||||
|
|
||||||
|
def __exit__(self, *exc) -> None:
|
||||||
|
self.detach()
|
||||||
|
|
||||||
# ------------------------------------------------------------- reporting
|
# ------------------------------------------------------------- reporting
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user