Mark the story with a node, not with a count
The memory bank and the story summary each kept a cursor: how many story actions they had already covered. A count is a position in a list, and this list moves — delete an action in front of the mark and every later one slides down a slot, so the mark now covers one it has never read. All the cursor bookkeeping existed to patch that up. Both marks are now (branch_id, depth): the node up to and including which the work is done. A depth is a coordinate along a path, not an offset into a list, so nothing in front of it can move it. That deletes rather than rewrites `position_of_index`, `note_action_removed`, `_rewind_cursors_to_index`, `prune_dangling_memories` and the every-pass clamp in `run_post_turn`. A memory hangs off the node its block ends on, so a fork inherits its ancestors' memories without copying any, and retrieval selects through the branch clause over the *whole* lineage — recall is long-range by definition and cannot be windowed. Measured: 1,807 B on a story forked twenty times against 1,823 B on a flat one of the same length. Migrations 53-56 translate the old counts into nodes. They rewrite `adventures` and not `actions`, so this one needs no VACUUM FULL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Dvvqn9ZDR4ixeFPHNbww7
This commit is contained in:
committed by
Parth
co-authored by
Claude Opus 5
parent
c7b6a46a8a
commit
c51531709d
@@ -1,10 +1,19 @@
|
||||
"""Memories must never describe an attempt the player can still retry away.
|
||||
"""Memories must never describe an attempt the player can still retry away,
|
||||
and must never skip a stretch of story.
|
||||
|
||||
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.
|
||||
cover the just-generated AI turn; retrying it rewrites Action.text but the mark
|
||||
has already moved past it, so the memory is never regenerated and goes on
|
||||
describing narration that is no longer in the story.
|
||||
|
||||
Phase 14 SP3 changed what that mark *is*. It used to be a count of covered
|
||||
story actions, and the second half of this file is the price of that: deleting
|
||||
an action from in front of a position slid a never-summarized action into the
|
||||
covered range, so every delete had to slide the cursors too. The mark is a node
|
||||
now — `(branch_id, depth)` — and a node does not move when something in front
|
||||
of it is deleted, so those tests assert that nothing happens where they used to
|
||||
assert that the right correction happened.
|
||||
|
||||
python -m pytest tests/test_memory_settling.py -v
|
||||
"""
|
||||
@@ -20,7 +29,8 @@ os.environ.pop("DATABASE_URL", None)
|
||||
|
||||
import pytest
|
||||
|
||||
from app import memorybank, models
|
||||
from app import memorybank, models, tree
|
||||
from app.context import cursors
|
||||
from app.database import Base, SessionLocal, engine
|
||||
|
||||
|
||||
@@ -68,6 +78,26 @@ def make_adventure(db, action_count: int) -> models.Adventure:
|
||||
return adventure
|
||||
|
||||
|
||||
def cover(db, adventure, position: int) -> None:
|
||||
"""Mark the first `position` story actions as already summarized.
|
||||
|
||||
Written as a position and translated to the node it names, because that is
|
||||
what every adventure in the database looked like before SP3 and what a v1
|
||||
bundle still carries. `memory_cursor` keeps the old number so the two
|
||||
coordinate systems can be compared where a test cares.
|
||||
"""
|
||||
adventure.memory_cursor = position
|
||||
adventure.summary_cursor = position
|
||||
cursors.anchor_at_position(adventure, cursors.MEMORY, position)
|
||||
cursors.anchor_at_position(adventure, cursors.SUMMARY, position)
|
||||
db.commit()
|
||||
|
||||
|
||||
def covered_depth(db, adventure) -> int:
|
||||
"""The memory mark, as a depth on the story being played."""
|
||||
return cursors.MEMORY.depth(db, adventure)
|
||||
|
||||
|
||||
def run_memories(db, adventure, stub, monkeypatch):
|
||||
monkeypatch.setattr(memorybank, "summary_provider", lambda s: stub)
|
||||
settings = db.query(models.Settings).first()
|
||||
@@ -99,25 +129,23 @@ def test_settled_actions_on_a_one_action_story(db):
|
||||
# ------------------------------------------------------- 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."""
|
||||
"""Covered up to action 5 with 12 actions is exactly the case that used to
|
||||
bite: the 6-action block ends on the newest action, still retryable."""
|
||||
adventure = make_adventure(db, 12)
|
||||
adventure.memory_cursor = 6
|
||||
db.commit()
|
||||
cover(db, adventure, 6)
|
||||
|
||||
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
|
||||
assert covered_depth(db, adventure) == 5
|
||||
|
||||
|
||||
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()
|
||||
cover(db, adventure, 6)
|
||||
|
||||
stub = StubSummarizer()
|
||||
run_memories(db, adventure, stub, monkeypatch)
|
||||
@@ -128,7 +156,10 @@ def test_the_block_lands_a_turn_later_without_the_newest_action(db, monkeypatch)
|
||||
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
|
||||
# The mark and the memory name the same node — that is what keeps them from
|
||||
# drifting apart however gappy the depths underneath are.
|
||||
assert (memory.branch_id, memory.depth) == cursors.MEMORY.stored(adventure)
|
||||
assert covered_depth(db, adventure) == 11
|
||||
|
||||
|
||||
def test_first_memory_waits_one_action_past_memory_start(db, monkeypatch):
|
||||
@@ -150,21 +181,20 @@ def test_first_memory_waits_one_action_past_memory_start(db, monkeypatch):
|
||||
|
||||
|
||||
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."""
|
||||
"""An adventure summarized under the OLD rule carries a cursor equal to its
|
||||
action count — one past the settled end. That used to need a clamp on every
|
||||
post-turn pass, and clamping it to the *settled* count re-covered an action.
|
||||
|
||||
A mark that names a node has no such edge: the newest action is the node,
|
||||
and "everything after it" is empty until the story grows.
|
||||
"""
|
||||
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()
|
||||
cover(db, adventure, 12)
|
||||
|
||||
# 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
|
||||
assert covered_depth(db, adventure) == 11 # the newest action, not one past it
|
||||
assert memorybank.settled_after(adventure, covered_depth(db, adventure)) == -1
|
||||
|
||||
# Grow the story and let the next block form.
|
||||
for i in range(12, 25):
|
||||
@@ -191,93 +221,114 @@ def test_no_memories_before_memory_start(db, monkeypatch):
|
||||
# ------------------------------------------- deleting already-summarized ground
|
||||
|
||||
def orphans(db, adventure) -> list[int]:
|
||||
"""Action indices the cursor calls summarized that no memory describes."""
|
||||
"""Depths the mark calls summarized that no memory describes.
|
||||
|
||||
The failure this whole section is about, stated once: an action behind the
|
||||
mark with nothing covering it is never summarized again, and nothing ever
|
||||
reports it.
|
||||
"""
|
||||
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]
|
||||
mark = cursors.MEMORY.depth(db, adventure)
|
||||
return [
|
||||
a.depth for a in memorybank.story_actions(adventure)
|
||||
if a.depth <= mark and a.depth not in covered
|
||||
]
|
||||
|
||||
|
||||
def summarized_adventure(db):
|
||||
"""13 actions with two memories covering indices 0-11, cursor at 12."""
|
||||
"""13 actions with two memories covering depths 0-11, the mark on node 11."""
|
||||
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()
|
||||
for text, start, end in (("A", 0, 5), ("B", 6, 11)):
|
||||
node = db.query(models.Action).filter_by(
|
||||
adventure_id=adventure.id, index=end
|
||||
).one()
|
||||
memory = models.Memory(
|
||||
adventure_id=adventure.id, text=text, source_start=start, source_end=end
|
||||
)
|
||||
tree.attach_memory(memory, node)
|
||||
db.add(memory)
|
||||
cover(db, adventure, 12)
|
||||
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()
|
||||
def test_deleting_a_middle_action_leaves_the_mark_where_it_was(db):
|
||||
"""The bug that motivated the old machinery, and the reason it is gone.
|
||||
|
||||
memorybank.note_action_removed(adventure, victim)
|
||||
A position cursor counted actions from the start, so deleting an earlier
|
||||
one slid a never-summarized action into the covered range and every delete
|
||||
had to correct for it. A depth is not a count: node 11 is still node 11
|
||||
with node 5 gone.
|
||||
"""
|
||||
adventure = summarized_adventure(db)
|
||||
# Node 4 is inside memory A's block but is not the node it hangs off, so
|
||||
# nothing is withdrawn — the same reading the old code had, where only a
|
||||
# memory whose *end* had fallen off the story was pruned.
|
||||
victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=4).one()
|
||||
|
||||
assert memorybank.forget_node(db, adventure, victim) == 0
|
||||
db.delete(victim)
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
|
||||
assert adventure.memory_cursor == 11 # slid down by one
|
||||
assert covered_depth(db, adventure) == 11
|
||||
assert [m.text for m in db.query(models.Memory).all()] == ["A", "B"]
|
||||
assert orphans(db, adventure) == []
|
||||
|
||||
|
||||
def test_deleting_a_later_action_leaves_cursors_alone(db):
|
||||
"""Only actions *before* the cursor shift it."""
|
||||
def test_deleting_a_later_action_leaves_the_mark_alone(db):
|
||||
adventure = summarized_adventure(db)
|
||||
victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=12).one()
|
||||
|
||||
memorybank.note_action_removed(adventure, victim)
|
||||
memorybank.forget_node(db, adventure, victim)
|
||||
db.delete(victim)
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
|
||||
assert adventure.memory_cursor == 12
|
||||
assert covered_depth(db, adventure) == 11
|
||||
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"])
|
||||
def test_deleting_a_summarized_node_withdraws_its_memory(db):
|
||||
"""Discarding the memory isn't enough — the story it covered is still
|
||||
behind the mark, so the mark has to come back to where that block began.
|
||||
|
||||
assert memorybank.prune_dangling_memories(adventure, db) == 1
|
||||
Memory B ends on node 11, so deleting node 11 is what withdraws it. The old
|
||||
code found this by scanning for a memory whose covered range had fallen off
|
||||
the end of the story; the memory hangs off the node now, so it is a lookup.
|
||||
"""
|
||||
adventure = summarized_adventure(db)
|
||||
victim = db.query(models.Action).filter_by(adventure_id=adventure.id, index=11).one()
|
||||
|
||||
assert memorybank.forget_node(db, adventure, victim) == 1
|
||||
db.delete(victim)
|
||||
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 covered_depth(db, adventure) == 5 # back to where the discarded memory began
|
||||
assert cursors.SUMMARY.depth(db, adventure) == 5 # and the summary with it
|
||||
assert orphans(db, adventure) == []
|
||||
|
||||
|
||||
def test_repeated_deletes_never_orphan_an_action(db):
|
||||
"""The scenario that motivated this: undo/delete-last, over and over."""
|
||||
"""The scenario that motivated this: undo/delete-last, over and over.
|
||||
|
||||
No clamp in the loop any more, and no bookkeeping call per delete beyond
|
||||
withdrawing what the node produced.
|
||||
"""
|
||||
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)
|
||||
victim = max(actions, key=lambda a: a.depth)
|
||||
memorybank.forget_node(db, 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))
|
||||
|
||||
Reference in New Issue
Block a user