Stop retried and deleted actions from corrupting story context and memories
Three fallout bugs from keeping the retried action row alive (906ba42),
plus two long-standing cursor bugs the same investigation turned up.
Retry context leak: the row being regenerated is still attached to the
adventure, so it was replayed as established story and the model wrote a
continuation of the attempt it was meant to replace — the story visibly
blended both takes. It leaked into four places, not one: history replay,
story-card trigger matching, in-scene NPC detection, and the memory-bank
similarity query. Adds a shared context.story_actions(exclude_action_id),
threaded through build_context and retrieve_memories.
Memory holdback: a memory could summarize the just-generated turn; retry
rewrites Action.text but memory_cursor has already advanced, so the memory
was never regenerated and went on describing narration no longer in the
story. settled_story_actions() holds the newest action back one turn —
only the last action is retryable, so that makes it unreachable. The
settled list is always a prefix, so cursors stay valid and nothing is
skipped. The run_post_turn clamp deliberately still uses the full count:
clamping to settled rewinds legacy adventures a step and double-covers an
action.
Cursor bookkeeping: memory_cursor is a position into story_actions() while
Memory.source_* are Action.index values, and the two diverge as soon as
anything is deleted. Deleting a middle action slid a never-summarized
action into the covered range, skipping it forever; and pruning a memory
left the actions it covered stranded behind the cursor. Adds
note_action_removed() (called before the delete in delete_action and
undo_turn) and a rewind in prune_dangling_memories. delete_action also
now prunes at all, which it never did.
Not addressed: editing an already-summarized action still leaves its
memory stale, and the cumulative story summary can't have one fact
un-mixed from it.
117 backend tests pass, including new test_memory_settling.py (12) and
two retry-context regression tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UeQVy5bEjLhfgWNc27Efet
This commit is contained in:
co-authored by
Claude Opus 5
parent
906ba423d8
commit
7c538c8235
@@ -1,3 +1,3 @@
|
|||||||
from .builder import build_context, count_tokens, truncate_to_last_tokens
|
from .builder import build_context, count_tokens, story_actions, truncate_to_last_tokens
|
||||||
|
|
||||||
__all__ = ["build_context", "count_tokens", "truncate_to_last_tokens"]
|
__all__ = ["build_context", "count_tokens", "story_actions", "truncate_to_last_tokens"]
|
||||||
|
|||||||
@@ -74,10 +74,24 @@ def _history_text(action: models.Action) -> str:
|
|||||||
return text
|
return text
|
||||||
|
|
||||||
|
|
||||||
def _visible_npcs(adventure: models.Adventure, stat_schema: dict) -> dict[str, str]:
|
def story_actions(
|
||||||
|
adventure: models.Adventure, exclude_action_id: int | None = None
|
||||||
|
) -> list[models.Action]:
|
||||||
|
"""The adventure's non-empty actions, as the model should see them.
|
||||||
|
|
||||||
|
`exclude_action_id` drops one action from the story — used by retry, where
|
||||||
|
the row being regenerated is still attached to the adventure (it holds the
|
||||||
|
variant history) but must not appear in the context assembled to replace it.
|
||||||
|
"""
|
||||||
|
return [
|
||||||
|
a for a in adventure.actions
|
||||||
|
if a.text.strip() and (exclude_action_id is None or a.id != exclude_action_id)
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def _visible_npcs(actions: list[models.Action], stat_schema: dict) -> dict[str, str]:
|
||||||
"""Defined NPCs whose trigger words appear in the recent story — the ones
|
"""Defined NPCs whose trigger words appear in the recent story — the ones
|
||||||
"in scene", so only their stats get injected. Maps npc id -> display name."""
|
"in scene", so only their stats get injected. Maps npc id -> display name."""
|
||||||
actions = [a for a in adventure.actions if a.text.strip()]
|
|
||||||
recent = SEPARATOR.join(a.text for a in actions[-6:]).lower()
|
recent = SEPARATOR.join(a.text for a in actions[-6:]).lower()
|
||||||
visible: dict[str, str] = {}
|
visible: dict[str, str] = {}
|
||||||
for npc_key, ndef in (stat_schema.get("npcs") or {}).items():
|
for npc_key, ndef in (stat_schema.get("npcs") or {}).items():
|
||||||
@@ -107,10 +121,13 @@ def build_context(
|
|||||||
adventure: models.Adventure,
|
adventure: models.Adventure,
|
||||||
settings: models.Settings,
|
settings: models.Settings,
|
||||||
memory_bank: dict | None = None,
|
memory_bank: dict | None = None,
|
||||||
|
exclude_action_id: int | None = None,
|
||||||
) -> tuple[str, str, dict]:
|
) -> tuple[str, str, dict]:
|
||||||
"""Returns (system_text, story_text, context_report). `memory_bank` is the
|
"""Returns (system_text, story_text, context_report). `memory_bank` is the
|
||||||
result of memorybank.retrieve_memories (None when the bank is off)."""
|
result of memorybank.retrieve_memories (None when the bank is off);
|
||||||
|
`exclude_action_id` omits one action from the story (see story_actions)."""
|
||||||
script_mem = _script_memory(adventure)
|
script_mem = _script_memory(adventure)
|
||||||
|
actions = story_actions(adventure, exclude_action_id)
|
||||||
|
|
||||||
# ----- Always-included components -----
|
# ----- Always-included components -----
|
||||||
system_sections: list[Section] = [Section("narrator", settings.narrator_prompt.strip())]
|
system_sections: list[Section] = [Section("narrator", settings.narrator_prompt.strip())]
|
||||||
@@ -123,7 +140,7 @@ def build_context(
|
|||||||
if guide:
|
if guide:
|
||||||
system_sections.append(Section("world_state_guide", guide))
|
system_sections.append(Section("world_state_guide", guide))
|
||||||
block = worldstate.render_state_section(
|
block = worldstate.render_state_section(
|
||||||
adventure.world_state, stat_schema, _visible_npcs(adventure, stat_schema)
|
adventure.world_state, stat_schema, _visible_npcs(actions, stat_schema)
|
||||||
)
|
)
|
||||||
if block:
|
if block:
|
||||||
system_sections.append(Section("world_state", block))
|
system_sections.append(Section("world_state", block))
|
||||||
@@ -163,7 +180,6 @@ def build_context(
|
|||||||
available = max(256, settings.context_token_budget - reserved)
|
available = max(256, settings.context_token_budget - reserved)
|
||||||
|
|
||||||
# ----- Story cards: triggered by recent story text (the window history could fill) -----
|
# ----- Story cards: triggered by recent story text (the window history could fill) -----
|
||||||
actions = [a for a in adventure.actions if a.text.strip()]
|
|
||||||
trigger_window = truncate_to_last_tokens(SEPARATOR.join(a.text for a in actions), available)
|
trigger_window = truncate_to_last_tokens(SEPARATOR.join(a.text for a in actions), available)
|
||||||
triggered = _match_cards(adventure.story_cards, trigger_window)
|
triggered = _match_cards(adventure.story_cards, trigger_window)
|
||||||
|
|
||||||
|
|||||||
+76
-10
@@ -4,7 +4,9 @@
|
|||||||
After each turn, a fire-and-forget task (`run_post_turn`) runs with its own DB
|
After each turn, a fire-and-forget task (`run_post_turn`) runs with its own DB
|
||||||
session:
|
session:
|
||||||
- every MEMORY_INTERVAL actions (starting at MEMORY_START), each uncovered
|
- every MEMORY_INTERVAL actions (starting at MEMORY_START), each uncovered
|
||||||
block of actions is summarized into a short "memory";
|
block of actions is summarized into a short "memory". Summarization only
|
||||||
|
ever reads *settled* actions (see settled_story_actions) — the newest action
|
||||||
|
is held back one turn because it is still retryable;
|
||||||
- every SUMMARY_INTERVAL actions, the Story Summary is rewritten folding in
|
- every SUMMARY_INTERVAL actions, the Story Summary is rewritten folding in
|
||||||
the new memories (the user-edited text is always the base, never clobbered);
|
the new memories (the user-edited text is always the base, never clobbered);
|
||||||
- new memories are embedded (OpenAI-compatible /v1/embeddings) and the bank
|
- new memories are embedded (OpenAI-compatible /v1/embeddings) and the bank
|
||||||
@@ -23,7 +25,7 @@ import math
|
|||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from . import models
|
from . import models
|
||||||
from .context import truncate_to_last_tokens
|
from .context import story_actions, truncate_to_last_tokens
|
||||||
from .database import SessionLocal
|
from .database import SessionLocal
|
||||||
from .providers import OpenAICompatibleProvider, ProviderError
|
from .providers import OpenAICompatibleProvider, ProviderError
|
||||||
|
|
||||||
@@ -86,22 +88,78 @@ def cosine(a: list[float], b: list[float]) -> float:
|
|||||||
return dot / norm if norm else 0.0
|
return dot / norm if norm else 0.0
|
||||||
|
|
||||||
|
|
||||||
def story_actions(adventure: models.Adventure) -> list[models.Action]:
|
def settled_story_actions(adventure: models.Adventure) -> list[models.Action]:
|
||||||
return [a for a in adventure.actions if a.text.strip()]
|
"""Story actions old enough to summarize: everything but the newest one.
|
||||||
|
|
||||||
|
Only the *last* action can be retried, so once an action has another action
|
||||||
|
after it, its text is final. Summarizing right up to the newest action meant
|
||||||
|
a memory could describe an attempt the player then retried away — the
|
||||||
|
memory's cursor has already advanced, so it is never regenerated, leaving a
|
||||||
|
memory (and, downstream, a story summary) describing narration that is no
|
||||||
|
longer in the story. Holding one action back costs a turn of latency and
|
||||||
|
makes that unreachable.
|
||||||
|
|
||||||
|
The result is always a prefix of story_actions(), so memory_cursor and
|
||||||
|
summary_cursor stay valid positions and no action is ever skipped.
|
||||||
|
"""
|
||||||
|
return story_actions(adventure)[:-1]
|
||||||
|
|
||||||
|
|
||||||
|
def _rewind_cursors_to_index(adventure: models.Adventure, index: int) -> None:
|
||||||
|
"""Move both cursors back to the position of Action.index `index`.
|
||||||
|
|
||||||
|
The cursors are *positions* into story_actions() while Memory.source_* are
|
||||||
|
Action.index values, so the two spaces have to be translated between (they
|
||||||
|
diverge as soon as any action is deleted).
|
||||||
|
"""
|
||||||
|
actions = story_actions(adventure)
|
||||||
|
position = next((i for i, a in enumerate(actions) if a.index >= index), len(actions))
|
||||||
|
adventure.memory_cursor = min(adventure.memory_cursor, position)
|
||||||
|
adventure.summary_cursor = min(adventure.summary_cursor, position)
|
||||||
|
|
||||||
|
|
||||||
|
def note_action_removed(adventure: models.Adventure, action: models.Action) -> None:
|
||||||
|
"""Keep the cursors pointing at the same actions when one is deleted from
|
||||||
|
*before* them. Call BEFORE the delete, while the action is still in the list.
|
||||||
|
|
||||||
|
memory_cursor counts actions from the start of the story, so removing an
|
||||||
|
earlier action slides every later one down a slot — without this, an action
|
||||||
|
that was never summarized shifts into the "already covered" range and is
|
||||||
|
skipped forever.
|
||||||
|
"""
|
||||||
|
actions = story_actions(adventure)
|
||||||
|
position = next((i for i, a in enumerate(actions) if a.id == action.id), None)
|
||||||
|
if position is None:
|
||||||
|
return
|
||||||
|
if position < adventure.memory_cursor:
|
||||||
|
adventure.memory_cursor -= 1
|
||||||
|
if position < adventure.summary_cursor:
|
||||||
|
adventure.summary_cursor -= 1
|
||||||
|
|
||||||
|
|
||||||
def prune_dangling_memories(adventure: models.Adventure, db: Session) -> int:
|
def prune_dangling_memories(adventure: models.Adventure, db: Session) -> int:
|
||||||
"""Delete memories that summarized actions which no longer exist (e.g. after
|
"""Delete memories that summarized actions which no longer exist (e.g. after
|
||||||
undo). source_start/source_end are Action.index values; a memory is dangling
|
undo). source_start/source_end are Action.index values; a memory is dangling
|
||||||
if any covered action is past the current end of the story. Returns the count
|
if any covered action is past the current end of the story. Returns the count
|
||||||
removed. Cursors are self-healing in run_post_turn, so this is cleanup only."""
|
removed.
|
||||||
|
|
||||||
|
Throwing a memory away is not enough on its own: the actions it covered are
|
||||||
|
still behind memory_cursor, so they would read as summarized with nothing
|
||||||
|
describing them. Rewind to where the earliest discarded memory began, so
|
||||||
|
those actions are summarized again.
|
||||||
|
"""
|
||||||
max_index = max((a.index for a in adventure.actions), default=-1)
|
max_index = max((a.index for a in adventure.actions), default=-1)
|
||||||
dangling = [
|
dangling = [
|
||||||
m for m in adventure.memories
|
m for m in adventure.memories
|
||||||
if m.source_end is not None and m.source_end > max_index
|
if m.source_end is not None and m.source_end > max_index
|
||||||
]
|
]
|
||||||
|
if not dangling:
|
||||||
|
return 0
|
||||||
|
starts = [m.source_start for m in dangling if m.source_start is not None]
|
||||||
for m in dangling:
|
for m in dangling:
|
||||||
db.delete(m)
|
db.delete(m)
|
||||||
|
if starts:
|
||||||
|
_rewind_cursors_to_index(adventure, min(starts))
|
||||||
return len(dangling)
|
return len(dangling)
|
||||||
|
|
||||||
|
|
||||||
@@ -112,11 +170,13 @@ async def retrieve_memories(
|
|||||||
settings: models.Settings,
|
settings: models.Settings,
|
||||||
*,
|
*,
|
||||||
update_stats: bool,
|
update_stats: bool,
|
||||||
|
exclude_action_id: int | None = None,
|
||||||
) -> dict | None:
|
) -> dict | None:
|
||||||
"""Returns {"used": [{id, text, similarity, pinned}], "error": str|None},
|
"""Returns {"used": [{id, text, similarity, pinned}], "error": str|None},
|
||||||
or None when the memory bank is off for this adventure. `update_stats`
|
or None when the memory bank is off for this adventure. `update_stats`
|
||||||
bumps use counters (real turns only, not Insights dry runs); the caller's
|
bumps use counters (real turns only, not Insights dry runs).
|
||||||
commit persists them."""
|
`exclude_action_id` drops the action being retried from the similarity
|
||||||
|
query, so the discarded attempt can't steer which memories come back."""
|
||||||
if not adventure.memory_bank_enabled:
|
if not adventure.memory_bank_enabled:
|
||||||
return None
|
return None
|
||||||
if not settings.embedding_model.strip():
|
if not settings.embedding_model.strip():
|
||||||
@@ -126,7 +186,7 @@ async def retrieve_memories(
|
|||||||
if not candidates:
|
if not candidates:
|
||||||
return {"used": [], "error": None}
|
return {"used": [], "error": None}
|
||||||
|
|
||||||
actions = story_actions(adventure)
|
actions = story_actions(adventure, exclude_action_id)
|
||||||
query = truncate_to_last_tokens(
|
query = truncate_to_last_tokens(
|
||||||
"\n\n".join(a.text for a in actions[-4:]), RETRIEVAL_WINDOW_TOKENS
|
"\n\n".join(a.text for a in actions[-4:]), RETRIEVAL_WINDOW_TOKENS
|
||||||
)
|
)
|
||||||
@@ -198,6 +258,12 @@ async def run_post_turn(adventure_id: int) -> None:
|
|||||||
return
|
return
|
||||||
# Undo/retry can shrink the action list below a stored cursor, which
|
# Undo/retry can shrink the action list below a stored cursor, which
|
||||||
# would stall summarization until the story grew past it again.
|
# would stall summarization until the story grew past it again.
|
||||||
|
# Deliberately the FULL count, not the settled one: an adventure that
|
||||||
|
# was caught up under the old rule can have a cursor equal to the action
|
||||||
|
# count, and clamping to settled would rewind it one step, re-covering
|
||||||
|
# an already-summarized action in the next block. Both consumers below
|
||||||
|
# read settled actions and bail on a negative remainder, so a cursor
|
||||||
|
# briefly sitting one past the settled end is harmless.
|
||||||
count = len(story_actions(adventure))
|
count = len(story_actions(adventure))
|
||||||
adventure.memory_cursor = min(adventure.memory_cursor, count)
|
adventure.memory_cursor = min(adventure.memory_cursor, count)
|
||||||
adventure.summary_cursor = min(adventure.summary_cursor, count)
|
adventure.summary_cursor = min(adventure.summary_cursor, count)
|
||||||
@@ -215,7 +281,7 @@ async def run_post_turn(adventure_id: int) -> None:
|
|||||||
async def _create_due_memories(
|
async def _create_due_memories(
|
||||||
adventure: models.Adventure, settings: models.Settings, db: Session
|
adventure: models.Adventure, settings: models.Settings, db: Session
|
||||||
) -> None:
|
) -> None:
|
||||||
actions = story_actions(adventure)
|
actions = settled_story_actions(adventure)
|
||||||
provider = summary_provider(settings)
|
provider = summary_provider(settings)
|
||||||
for _ in range(MAX_MEMORIES_PER_RUN):
|
for _ in range(MAX_MEMORIES_PER_RUN):
|
||||||
cursor = adventure.memory_cursor
|
cursor = adventure.memory_cursor
|
||||||
@@ -246,7 +312,7 @@ async def _create_due_memories(
|
|||||||
async def _update_story_summary(
|
async def _update_story_summary(
|
||||||
adventure: models.Adventure, settings: models.Settings, db: Session
|
adventure: models.Adventure, settings: models.Settings, db: Session
|
||||||
) -> None:
|
) -> None:
|
||||||
actions = story_actions(adventure)
|
actions = settled_story_actions(adventure)
|
||||||
if len(actions) - adventure.summary_cursor < SUMMARY_INTERVAL:
|
if len(actions) - adventure.summary_cursor < SUMMARY_INTERVAL:
|
||||||
return
|
return
|
||||||
|
|
||||||
|
|||||||
@@ -506,6 +506,11 @@ async def _generate_turn(
|
|||||||
):
|
):
|
||||||
settings = get_settings(db, user)
|
settings = get_settings(db, user)
|
||||||
cfg = auth.resolve_provider_config(settings)
|
cfg = auth.resolve_provider_config(settings)
|
||||||
|
# On a retry the row being regenerated is still attached to the adventure
|
||||||
|
# (it carries the variant history), so it has to be filtered out of the
|
||||||
|
# context — otherwise the model is shown the attempt it is replacing as if
|
||||||
|
# it were established story, and writes a continuation of it.
|
||||||
|
replacing_id = retry_of.id if retry_of is not None else None
|
||||||
if cfg.using_demo:
|
if cfg.using_demo:
|
||||||
# No embedding/summarization calls on the server-funded key: memory
|
# No embedding/summarization calls on the server-funded key: memory
|
||||||
# retrieval is skipped (with a visible note when the bank is on).
|
# retrieval is skipped (with a visible note when the bank is on).
|
||||||
@@ -515,12 +520,16 @@ async def _generate_turn(
|
|||||||
else None
|
else None
|
||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
memories = await memorybank.retrieve_memories(adventure, settings, update_stats=True)
|
memories = await memorybank.retrieve_memories(
|
||||||
|
adventure, settings, update_stats=True, exclude_action_id=replacing_id
|
||||||
|
)
|
||||||
# Scoreboard as it stands before this AI turn's context/output hooks mutate
|
# Scoreboard as it stands before this AI turn's context/output hooks mutate
|
||||||
# it — stapled onto the AI action so retry can start over from here.
|
# it — stapled onto the AI action so retry can start over from here.
|
||||||
state_before = snapshot_state(adventure)
|
state_before = snapshot_state(adventure)
|
||||||
world_state_before = snapshot_world_state(adventure)
|
world_state_before = snapshot_world_state(adventure)
|
||||||
system_text, story_text, snapshot = build_context(adventure, settings, memories)
|
system_text, story_text, snapshot = build_context(
|
||||||
|
adventure, settings, memories, exclude_action_id=replacing_id
|
||||||
|
)
|
||||||
|
|
||||||
# onModelContext: scripts see (and may rewrite) the whole assembled context.
|
# onModelContext: scripts see (and may rewrite) the whole assembled context.
|
||||||
combined = f"{system_text}\n\n{story_text}" if system_text else story_text
|
combined = f"{system_text}\n\n{story_text}" if system_text else story_text
|
||||||
@@ -863,9 +872,11 @@ def undo_turn(
|
|||||||
last = actions.pop()
|
last = actions.pop()
|
||||||
# The earliest action removed in this turn holds the pre-turn scoreboard.
|
# The earliest action removed in this turn holds the pre-turn scoreboard.
|
||||||
first_removed = last
|
first_removed = last
|
||||||
|
memorybank.note_action_removed(adventure, last)
|
||||||
db.delete(last)
|
db.delete(last)
|
||||||
if last.type == "ai" and actions and actions[-1].type in ("do", "say", "story"):
|
if last.type == "ai" and actions and actions[-1].type in ("do", "say", "story"):
|
||||||
first_removed = actions.pop()
|
first_removed = actions.pop()
|
||||||
|
memorybank.note_action_removed(adventure, first_removed)
|
||||||
db.delete(first_removed)
|
db.delete(first_removed)
|
||||||
if first_removed.state_before is not None:
|
if first_removed.state_before is not None:
|
||||||
adventure.script_state = copy.deepcopy(first_removed.state_before)
|
adventure.script_state = copy.deepcopy(first_removed.state_before)
|
||||||
@@ -1480,9 +1491,15 @@ def delete_action(
|
|||||||
db: Session = Depends(get_db),
|
db: Session = Depends(get_db),
|
||||||
user: models.User = CurrentUser,
|
user: models.User = CurrentUser,
|
||||||
):
|
):
|
||||||
get_adventure_or_404(adventure_id, db, user)
|
adventure = get_adventure_or_404(adventure_id, db, user)
|
||||||
action = db.get(models.Action, action_id)
|
action = db.get(models.Action, action_id)
|
||||||
if action is None or action.adventure_id != adventure_id:
|
if action is None or action.adventure_id != adventure_id:
|
||||||
raise HTTPException(404, "Action not found")
|
raise HTTPException(404, "Action not found")
|
||||||
|
# Cursor bookkeeping, same as undo: slide the cursors down if this action
|
||||||
|
# sits before them, then drop any memory left describing a deleted action.
|
||||||
|
memorybank.note_action_removed(adventure, action)
|
||||||
db.delete(action)
|
db.delete(action)
|
||||||
|
db.flush() # apply the delete so pruning sees the shrunken action list
|
||||||
|
db.expire(adventure, ["actions"])
|
||||||
|
memorybank.prune_dangling_memories(adventure, db)
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|||||||
@@ -0,0 +1,283 @@
|
|||||||
|
"""Memories must never describe an attempt the player can still retry away.
|
||||||
|
|
||||||
|
Only the last action is retryable, so summarization holds the newest action
|
||||||
|
back one turn (memorybank.settled_story_actions). Without that, a memory could
|
||||||
|
cover the just-generated AI turn; retrying it rewrites Action.text but the
|
||||||
|
memory cursor has already advanced, so the memory is never regenerated and goes
|
||||||
|
on describing narration that is no longer in the story.
|
||||||
|
|
||||||
|
python -m pytest tests/test_memory_settling.py -v
|
||||||
|
"""
|
||||||
|
import asyncio
|
||||||
|
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 memorybank, models
|
||||||
|
from app.database import Base, SessionLocal, engine
|
||||||
|
|
||||||
|
|
||||||
|
class StubSummarizer:
|
||||||
|
"""Records every excerpt handed to the summarizer."""
|
||||||
|
|
||||||
|
def __init__(self):
|
||||||
|
self.excerpts: list[str] = []
|
||||||
|
|
||||||
|
async def complete(self, system, user, **kwargs):
|
||||||
|
self.excerpts.append(user)
|
||||||
|
return f"Memory {len(self.excerpts)}."
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture()
|
||||||
|
def db():
|
||||||
|
Base.metadata.create_all(bind=engine)
|
||||||
|
session = SessionLocal()
|
||||||
|
try:
|
||||||
|
yield session
|
||||||
|
finally:
|
||||||
|
session.close()
|
||||||
|
Base.metadata.drop_all(bind=engine)
|
||||||
|
|
||||||
|
|
||||||
|
def make_adventure(db, action_count: int) -> models.Adventure:
|
||||||
|
"""An adventure whose actions alternate player/AI, newest last."""
|
||||||
|
user = models.User(is_guest=False, email="memory@example.com")
|
||||||
|
db.add(user)
|
||||||
|
db.flush()
|
||||||
|
settings = models.Settings(user_id=user.id, api_key="enc:dummy", model="test-model")
|
||||||
|
db.add(settings)
|
||||||
|
adventure = models.Adventure(
|
||||||
|
user_id=user.id, title="Cave", script_state={}, auto_summarize=True
|
||||||
|
)
|
||||||
|
db.add(adventure)
|
||||||
|
db.flush()
|
||||||
|
for i in range(action_count):
|
||||||
|
db.add(models.Action(
|
||||||
|
adventure_id=adventure.id, index=i,
|
||||||
|
type="ai" if i % 2 else "do", text=f"Action {i}.",
|
||||||
|
))
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
return adventure
|
||||||
|
|
||||||
|
|
||||||
|
def run_memories(db, adventure, stub, monkeypatch):
|
||||||
|
monkeypatch.setattr(memorybank, "summary_provider", lambda s: stub)
|
||||||
|
settings = db.query(models.Settings).first()
|
||||||
|
asyncio.run(memorybank._create_due_memories(adventure, settings, db))
|
||||||
|
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------- settling
|
||||||
|
|
||||||
|
def test_settled_actions_holds_back_the_newest(db):
|
||||||
|
adventure = make_adventure(db, 5)
|
||||||
|
settled = memorybank.settled_story_actions(adventure)
|
||||||
|
assert [a.index for a in settled] == [0, 1, 2, 3]
|
||||||
|
|
||||||
|
|
||||||
|
def test_settled_actions_is_a_prefix_so_cursors_stay_valid(db):
|
||||||
|
"""The safety property behind the whole approach: dropping the newest
|
||||||
|
action can never renumber or skip an earlier one."""
|
||||||
|
adventure = make_adventure(db, 9)
|
||||||
|
full = memorybank.story_actions(adventure)
|
||||||
|
settled = memorybank.settled_story_actions(adventure)
|
||||||
|
assert full[: len(settled)] == settled
|
||||||
|
|
||||||
|
|
||||||
|
def test_settled_actions_on_a_one_action_story(db):
|
||||||
|
adventure = make_adventure(db, 1)
|
||||||
|
assert memorybank.settled_story_actions(adventure) == []
|
||||||
|
|
||||||
|
|
||||||
|
# ------------------------------------------------------- the bug this prevents
|
||||||
|
|
||||||
|
def test_memory_never_covers_the_newest_retryable_action(db, monkeypatch):
|
||||||
|
"""cursor=6 with 12 actions is exactly the case that used to bite: the
|
||||||
|
6-action block ends on the newest action, which is still retryable."""
|
||||||
|
adventure = make_adventure(db, 12)
|
||||||
|
adventure.memory_cursor = 6
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
stub = StubSummarizer()
|
||||||
|
run_memories(db, adventure, stub, monkeypatch)
|
||||||
|
|
||||||
|
assert stub.excerpts == [] # only 11 settled — one short of a block
|
||||||
|
assert db.query(models.Memory).count() == 0
|
||||||
|
assert adventure.memory_cursor == 6
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_block_lands_a_turn_later_without_the_newest_action(db, monkeypatch):
|
||||||
|
"""One more action and the same block is summarized — minus the new one."""
|
||||||
|
adventure = make_adventure(db, 13)
|
||||||
|
adventure.memory_cursor = 6
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
stub = StubSummarizer()
|
||||||
|
run_memories(db, adventure, stub, monkeypatch)
|
||||||
|
|
||||||
|
assert len(stub.excerpts) == 1
|
||||||
|
excerpt = stub.excerpts[0]
|
||||||
|
assert "Action 11." in excerpt # the block's real last action
|
||||||
|
assert "Action 12." not in excerpt # the newest, still retryable
|
||||||
|
memory = db.query(models.Memory).one()
|
||||||
|
assert (memory.source_start, memory.source_end) == (6, 11)
|
||||||
|
assert adventure.memory_cursor == 12
|
||||||
|
|
||||||
|
|
||||||
|
def test_first_memory_waits_one_action_past_memory_start(db, monkeypatch):
|
||||||
|
adventure = make_adventure(db, memorybank.MEMORY_START)
|
||||||
|
stub = StubSummarizer()
|
||||||
|
run_memories(db, adventure, stub, monkeypatch)
|
||||||
|
assert stub.excerpts == []
|
||||||
|
|
||||||
|
db.add(models.Action(
|
||||||
|
adventure_id=adventure.id, index=memorybank.MEMORY_START, type="do", text="Later.",
|
||||||
|
))
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
run_memories(db, adventure, stub, monkeypatch)
|
||||||
|
# 12 settled actions = two full blocks, caught up in one run (MAX_MEMORIES_
|
||||||
|
# PER_RUN allows 5); neither may reach the newly added newest action.
|
||||||
|
assert len(stub.excerpts) == 2
|
||||||
|
assert not any("Later." in e for e in stub.excerpts)
|
||||||
|
|
||||||
|
|
||||||
|
def test_legacy_caught_up_adventure_is_not_rewound(db, monkeypatch):
|
||||||
|
"""An adventure summarized under the OLD rule can have memory_cursor equal
|
||||||
|
to its action count. The run_post_turn clamp must use the FULL count, not
|
||||||
|
the settled one — clamping to settled would rewind the cursor a step and
|
||||||
|
re-cover an already-summarized action in the next block."""
|
||||||
|
adventure = make_adventure(db, 12)
|
||||||
|
db.add(models.Memory(adventure_id=adventure.id, text="A", source_start=0, source_end=5))
|
||||||
|
db.add(models.Memory(adventure_id=adventure.id, text="B", source_start=6, source_end=11))
|
||||||
|
adventure.memory_cursor = 12
|
||||||
|
adventure.summary_cursor = 12
|
||||||
|
db.commit()
|
||||||
|
|
||||||
|
# The clamp as run_post_turn applies it.
|
||||||
|
count = len(memorybank.story_actions(adventure))
|
||||||
|
adventure.memory_cursor = min(adventure.memory_cursor, count)
|
||||||
|
assert adventure.memory_cursor == 12 # not rewound to 11
|
||||||
|
|
||||||
|
# Grow the story and let the next block form.
|
||||||
|
for i in range(12, 25):
|
||||||
|
db.add(models.Action(adventure_id=adventure.id, index=i, type="do", text=f"Action {i}."))
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
run_memories(db, adventure, StubSummarizer(), monkeypatch)
|
||||||
|
|
||||||
|
covered: dict[int, int] = {}
|
||||||
|
for m in db.query(models.Memory).all():
|
||||||
|
for i in range(m.source_start, m.source_end + 1):
|
||||||
|
covered[i] = covered.get(i, 0) + 1
|
||||||
|
assert [i for i, c in covered.items() if c > 1] == [] # no action summarized twice
|
||||||
|
assert [i for i in range(max(covered) + 1) if i not in covered] == [] # no gaps
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_memories_before_memory_start(db, monkeypatch):
|
||||||
|
adventure = make_adventure(db, 8)
|
||||||
|
stub = StubSummarizer()
|
||||||
|
run_memories(db, adventure, stub, monkeypatch)
|
||||||
|
assert stub.excerpts == []
|
||||||
|
|
||||||
|
|
||||||
|
# ------------------------------------------- deleting already-summarized ground
|
||||||
|
|
||||||
|
def orphans(db, adventure) -> list[int]:
|
||||||
|
"""Action indices the cursor calls summarized that no memory describes."""
|
||||||
|
covered: set[int] = set()
|
||||||
|
for m in db.query(models.Memory).filter_by(adventure_id=adventure.id):
|
||||||
|
covered |= set(range(m.source_start, m.source_end + 1))
|
||||||
|
actions = memorybank.story_actions(adventure)
|
||||||
|
return [a.index for a in actions[: adventure.memory_cursor] if a.index not in covered]
|
||||||
|
|
||||||
|
|
||||||
|
def summarized_adventure(db):
|
||||||
|
"""13 actions with two memories covering indices 0-11, cursor at 12."""
|
||||||
|
adventure = make_adventure(db, 13)
|
||||||
|
db.add(models.Memory(adventure_id=adventure.id, text="A", source_start=0, source_end=5))
|
||||||
|
db.add(models.Memory(adventure_id=adventure.id, text="B", source_start=6, source_end=11))
|
||||||
|
adventure.memory_cursor = 12
|
||||||
|
adventure.summary_cursor = 12
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
return adventure
|
||||||
|
|
||||||
|
|
||||||
|
def test_deleting_a_middle_action_does_not_skip_a_later_one(db):
|
||||||
|
"""memory_cursor counts positions, so removing an earlier action slides a
|
||||||
|
never-summarized one into the covered range unless the cursor slides too."""
|
||||||
|
adventure = summarized_adventure(db)
|
||||||
|
victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=5).one()
|
||||||
|
|
||||||
|
memorybank.note_action_removed(adventure, victim)
|
||||||
|
db.delete(victim)
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
|
||||||
|
assert adventure.memory_cursor == 11 # slid down by one
|
||||||
|
assert orphans(db, adventure) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_deleting_a_later_action_leaves_cursors_alone(db):
|
||||||
|
"""Only actions *before* the cursor shift it."""
|
||||||
|
adventure = summarized_adventure(db)
|
||||||
|
victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=12).one()
|
||||||
|
|
||||||
|
memorybank.note_action_removed(adventure, victim)
|
||||||
|
db.delete(victim)
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
|
||||||
|
assert adventure.memory_cursor == 12
|
||||||
|
assert orphans(db, adventure) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_pruning_a_memory_rewinds_to_where_it_started(db):
|
||||||
|
"""Discarding a memory isn't enough — the actions it covered are still
|
||||||
|
behind the cursor, so they must be handed back to the summarizer."""
|
||||||
|
adventure = summarized_adventure(db)
|
||||||
|
# Delete back past index 11, so memory B (6..11) covers a missing action.
|
||||||
|
for index in (12, 11):
|
||||||
|
victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=index).one()
|
||||||
|
memorybank.note_action_removed(adventure, victim)
|
||||||
|
db.delete(victim)
|
||||||
|
db.flush()
|
||||||
|
db.expire(adventure, ["actions"])
|
||||||
|
|
||||||
|
assert memorybank.prune_dangling_memories(adventure, db) == 1
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
|
||||||
|
assert [m.text for m in db.query(models.Memory).all()] == ["A"]
|
||||||
|
assert adventure.memory_cursor == 6 # back to where the discarded memory began
|
||||||
|
assert orphans(db, adventure) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_repeated_deletes_never_orphan_an_action(db):
|
||||||
|
"""The scenario that motivated this: undo/delete-last, over and over."""
|
||||||
|
adventure = summarized_adventure(db)
|
||||||
|
for _ in range(6):
|
||||||
|
actions = memorybank.story_actions(adventure)
|
||||||
|
if not actions:
|
||||||
|
break
|
||||||
|
victim = max(actions, key=lambda a: a.index)
|
||||||
|
memorybank.note_action_removed(adventure, victim)
|
||||||
|
db.delete(victim)
|
||||||
|
db.flush()
|
||||||
|
db.expire(adventure, ["actions"])
|
||||||
|
memorybank.prune_dangling_memories(adventure, db)
|
||||||
|
count = len(memorybank.story_actions(adventure))
|
||||||
|
adventure.memory_cursor = min(adventure.memory_cursor, count)
|
||||||
|
adventure.summary_cursor = min(adventure.summary_cursor, count)
|
||||||
|
db.commit()
|
||||||
|
db.refresh(adventure)
|
||||||
|
assert orphans(db, adventure) == []
|
||||||
|
assert adventure.memory_cursor <= len(memorybank.story_actions(adventure))
|
||||||
@@ -39,6 +39,7 @@ class ScriptedProvider:
|
|||||||
"""Streams the next canned reply each call, so successive retries differ."""
|
"""Streams the next canned reply each call, so successive retries differ."""
|
||||||
replies: list = []
|
replies: list = []
|
||||||
calls = 0
|
calls = 0
|
||||||
|
prompts: list = [] # every assembled (system, story) pair, for context assertions
|
||||||
|
|
||||||
def __init__(self, *a, **k):
|
def __init__(self, *a, **k):
|
||||||
pass
|
pass
|
||||||
@@ -46,6 +47,7 @@ class ScriptedProvider:
|
|||||||
async def generate(self, parts: PromptParts, *, temperature, max_tokens):
|
async def generate(self, parts: PromptParts, *, temperature, max_tokens):
|
||||||
index = min(ScriptedProvider.calls, len(ScriptedProvider.replies) - 1)
|
index = min(ScriptedProvider.calls, len(ScriptedProvider.replies) - 1)
|
||||||
ScriptedProvider.calls += 1
|
ScriptedProvider.calls += 1
|
||||||
|
ScriptedProvider.prompts.append((parts.system, parts.story))
|
||||||
reply = ScriptedProvider.replies[index]
|
reply = ScriptedProvider.replies[index]
|
||||||
if isinstance(reply, Exception):
|
if isinstance(reply, Exception):
|
||||||
raise reply
|
raise reply
|
||||||
@@ -79,6 +81,7 @@ def client(monkeypatch):
|
|||||||
|
|
||||||
ScriptedProvider.replies = ["Attempt one."]
|
ScriptedProvider.replies = ["Attempt one."]
|
||||||
ScriptedProvider.calls = 0
|
ScriptedProvider.calls = 0
|
||||||
|
ScriptedProvider.prompts = []
|
||||||
monkeypatch.setattr(adventures, "OpenAICompatibleProvider", ScriptedProvider)
|
monkeypatch.setattr(adventures, "OpenAICompatibleProvider", ScriptedProvider)
|
||||||
monkeypatch.setattr(auth, "resolve_provider_config", lambda s: auth.ProviderConfig(
|
monkeypatch.setattr(auth, "resolve_provider_config", lambda s: auth.ProviderConfig(
|
||||||
"http://fake", "k", "test-model", False))
|
"http://fake", "k", "test-model", False))
|
||||||
@@ -146,6 +149,35 @@ def test_retry_keeps_the_discarded_attempt(client):
|
|||||||
assert [v["active"] for v in r.json()] == [False, True]
|
assert [v["active"] for v in r.json()] == [False, True]
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_context_excludes_the_attempt_being_replaced(client):
|
||||||
|
"""The whole point of a retry is a fresh take on the *same* turn. The row
|
||||||
|
now survives the retry (it holds the variant history), so it is still in
|
||||||
|
`adventure.actions` while the replacement context is assembled — it must be
|
||||||
|
filtered out, or the model is asked to continue *past* the attempt it is
|
||||||
|
supposed to be replacing and writes a sequel that blends both."""
|
||||||
|
ScriptedProvider.replies = ["Attempt one.", "Attempt two."]
|
||||||
|
_play(client)
|
||||||
|
_retry(client)
|
||||||
|
|
||||||
|
retry_story = ScriptedProvider.prompts[-1][1]
|
||||||
|
assert "Attempt one." not in retry_story
|
||||||
|
# The turn's own player action must still be there — it's what to respond to.
|
||||||
|
assert "look around" in retry_story
|
||||||
|
assert "You enter a cave." in retry_story
|
||||||
|
|
||||||
|
|
||||||
|
def test_retry_context_keeps_earlier_ai_turns(client):
|
||||||
|
"""Only the action being retried is dropped, not AI history in general."""
|
||||||
|
ScriptedProvider.replies = ["First turn.", "Second turn.", "Second, again."]
|
||||||
|
_play(client, "go north")
|
||||||
|
_play(client, "go south")
|
||||||
|
_retry(client)
|
||||||
|
|
||||||
|
retry_story = ScriptedProvider.prompts[-1][1]
|
||||||
|
assert "First turn." in retry_story
|
||||||
|
assert "Second turn." not in retry_story
|
||||||
|
|
||||||
|
|
||||||
def test_never_retried_action_has_no_variants(client):
|
def test_never_retried_action_has_no_variants(client):
|
||||||
_play(client)
|
_play(client)
|
||||||
last = _actions(client)[-1]
|
last = _actions(client)[-1]
|
||||||
|
|||||||
Reference in New Issue
Block a user