Stop paying twice for a block a retry can still throw away
A memory whose block ends on the newest action is the one memory a player can reach: retry and take-switching both refuse anything else. Each retry of that turn withdrew the memory and wrote it again, and a block closes every six actions while a normal turn writes two, so that was one turn in three. SETTLE_SLACK asks for one action past a block before the block is summarized. The block is still MEMORY_INTERVAL actions; only the moment moves. This is not the pre-SP4 holdback returning: that one was about a retry rewriting text in place, which sibling attempts and forget_node settled, and correctness still rests on the withdrawal rather than on the slack. Undo and delete can carry a summarized node back to the tip, so the withdrawal path stays reachable, just rarely. The slack buys nothing back. The block that just closed is still in the history window in full, so a memory of it says what the model can already read. Tests: the settling suite asserts the new rule and that a retry at the tip finds nothing to withdraw; the rewrite suite builds thirteen actions so both of its blocks settle; the one path test that ended on a block boundary sets the slack to zero, because it is about which actions a block is read from rather than about when a block forms. 632 green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NcrxCJjqgDvAkamKeLWdn
This commit is contained in:
@@ -8,7 +8,8 @@ database session. It does three things:
|
||||
|
||||
- Every `MEMORY_INTERVAL` actions, starting once the adventure reaches
|
||||
`MEMORY_START` actions, it summarizes each uncovered block of actions into a
|
||||
short memory.
|
||||
short memory. A block waits until `SETTLE_SLACK` actions sit past it, so a
|
||||
memory never ends on the action a retry could replace.
|
||||
- Every `SUMMARY_INTERVAL` actions, it rewrites the story summary to include the
|
||||
new memories. The rewrite always starts from the text the user edited and
|
||||
never discards it.
|
||||
@@ -55,6 +56,32 @@ RETRIEVAL_WINDOW_ACTIONS = 4 # ...taken from this many of the newest actions
|
||||
SUMMARY_MAX_WORDS = 250
|
||||
MEMORY_EXCERPT_TOKENS = 2000 # of the block, when a block is longer than this
|
||||
|
||||
# How much story has to sit past a block before that block is summarized.
|
||||
#
|
||||
# This is not the pre-SP4 holdback returning. That rule made the newest action
|
||||
# invisible to the summarizer everywhere, and it existed because a retry
|
||||
# rewrote an action's text in place, so a memory covering the tip could end up
|
||||
# describing narration the player had already replaced. Sibling attempts and
|
||||
# `forget_node` settled that, and correctness does not rest on the number
|
||||
# below: a delete or an undo anywhere in the story is still repaired by
|
||||
# withdrawing what the coordinate produced.
|
||||
#
|
||||
# This is a cost rule, and it is only about the tip. Retry and take-switching
|
||||
# both refuse anything but the newest action (`takes.retry_action`,
|
||||
# `takes.switch_take`), and both withdraw the derived work at the coordinate
|
||||
# they change. So a memory whose block ends on the tip is the one memory a
|
||||
# player can still throw away, and every retry of that turn pays for it twice:
|
||||
# once to write it, once to write it again. A block closes every
|
||||
# MEMORY_INTERVAL actions and a normal turn writes two of them, so that is one
|
||||
# turn in three.
|
||||
#
|
||||
# One action of slack moves the block end out of reach of both endpoints, and
|
||||
# it buys nothing back in context quality: the block that just closed is still
|
||||
# in the history window in full, so a memory of it tells the model what it can
|
||||
# already read. Memories earn their place once the raw text has scrolled out,
|
||||
# which is never the turn the block closed.
|
||||
SETTLE_SLACK = 1
|
||||
|
||||
# ---- The cast brief (Phase 18b) ----
|
||||
# How many characters the brief names, and how much of each description it
|
||||
# carries. The cast is authored content rather than generated, so it is small in
|
||||
@@ -613,8 +640,10 @@ async def _create_due_memories(
|
||||
# the story, but this loop is the only code that moves the anchor, so
|
||||
# both numbers must be current.
|
||||
anchor = cursors.MEMORY.depth(db, adventure)
|
||||
if history.count_after(adventure, anchor) < MEMORY_INTERVAL:
|
||||
return # No full block of story sits past the mark.
|
||||
if history.count_after(adventure, anchor) < MEMORY_INTERVAL + SETTLE_SLACK:
|
||||
return # No settled block of story sits past the mark. The block
|
||||
# itself is still MEMORY_INTERVAL actions; the slack asks
|
||||
# for story past its end. See `SETTLE_SLACK`.
|
||||
if history.count(adventure) < MEMORY_START:
|
||||
return # The adventure is too short to have started summarizing.
|
||||
# The order of those two checks is deliberate. The usual answer is that
|
||||
|
||||
@@ -290,7 +290,10 @@ async def _generate_turn(
|
||||
# longer describes the story. Withdraw the memory attached to the node
|
||||
# and return that stretch to both passes. Before SP4 this code was
|
||||
# unreachable, because the summarizer held the newest action back until
|
||||
# a turn landed on top of it. See `memorybank`.
|
||||
# a turn landed on top of it. `memorybank.SETTLE_SLACK` keeps a memory
|
||||
# off the tip again, for cost rather than for correctness, so this is
|
||||
# now the rare case: undo or delete can carry a summarized node back to
|
||||
# the tip, and then a retry of it lands here. See `memorybank`.
|
||||
memorybank.forget_node(db, adventure, retry_of)
|
||||
cursors.rewind_all(adventure, retry_of.branch_id, ai_depth - 1)
|
||||
# Flush so the new attempt has an id. The session does not autoflush,
|
||||
|
||||
@@ -334,6 +334,10 @@ def test_a_block_is_summarized_from_the_path_and_hung_off_its_last_node(
|
||||
db, adventure, settings, ids = forked
|
||||
monkeypatch.setattr(memorybank, "MEMORY_START", 0)
|
||||
monkeypatch.setattr(memorybank, "MEMORY_INTERVAL", 4)
|
||||
# This test is about which actions a block is read from, not about when a
|
||||
# block forms, so switch the settling slack off and let the path end on a
|
||||
# block boundary. `test_memory_settling` owns the timing rule.
|
||||
monkeypatch.setattr(memorybank, "SETTLE_SLACK", 0)
|
||||
|
||||
class Stub:
|
||||
def __init__(self):
|
||||
@@ -347,8 +351,7 @@ def test_a_block_is_summarized_from_the_path_and_hung_off_its_last_node(
|
||||
monkeypatch.setattr(memorybank, "summary_provider", lambda s: stub)
|
||||
asyncio.run(memorybank._create_due_memories(adventure, settings, db))
|
||||
|
||||
# Two blocks of four from a path of eight, and since SP4 nothing is held
|
||||
# back, so both form in one pass.
|
||||
# Two blocks of four from a path of eight, both formed in one pass.
|
||||
first, second = stub.excerpts
|
||||
assert "A5" not in first + second, "a sibling's narration reached the summarizer"
|
||||
assert ["A0", "A1", "A2", "A3"] == [line for line in first.split() if line[0] in "ABC"]
|
||||
|
||||
@@ -69,8 +69,13 @@ def db():
|
||||
Base.metadata.drop_all(bind=engine)
|
||||
|
||||
|
||||
def make_adventure(db, *, actions=12, key="sk-test-key", email="rewrite@example.com"):
|
||||
"""An adventure whose bank was written by the old prompt."""
|
||||
def make_adventure(db, *, actions=13, key="sk-test-key", email="rewrite@example.com"):
|
||||
"""An adventure whose bank was written by the old prompt.
|
||||
|
||||
Thirteen actions, not twelve: two blocks of six, plus the one action that
|
||||
settles the second of them. A block is not summarized while it ends on the
|
||||
newest action. See `memorybank.SETTLE_SLACK`.
|
||||
"""
|
||||
user = models.User(is_guest=False, email=email)
|
||||
db.add(user)
|
||||
db.flush()
|
||||
|
||||
@@ -6,10 +6,15 @@ short of the newest, because only the last action was retryable, and a
|
||||
retry rewrote `Action.text` under a mark that had already moved past it.
|
||||
SP4 ended that: a retry writes a sibling node, and the coordinate's derived
|
||||
work is withdrawn as it happens, using the same repair that undo and delete
|
||||
already made. The holdback is gone, so the first half of this file now
|
||||
asserts the property that replaced it. A block forms as soon as there is a
|
||||
block, and changing what a coordinate says takes back what was derived from
|
||||
it.
|
||||
already made. Correctness has rested on that withdrawal ever since, and the
|
||||
second half of this file is where it is asserted.
|
||||
|
||||
The first half is about what the withdrawal costs. Redoing a block is
|
||||
correct and it is also paid for twice, so `memorybank.SETTLE_SLACK` keeps a
|
||||
block from ending on the newest action, which is the only action retry and
|
||||
take-switching can reach. A block forms as soon as one action has settled
|
||||
past it, and changing what a coordinate says still takes back what was
|
||||
derived from it.
|
||||
|
||||
Phase 14 SP3 changed what the mark is. It used to be a count of covered
|
||||
story actions, and the second half of this file is the cost of that.
|
||||
@@ -99,17 +104,17 @@ def run_memories(db, adventure, stub, monkeypatch):
|
||||
asyncio.run(memorybank._create_due_memories(adventure, settings, db))
|
||||
|
||||
|
||||
# --------------------------------------------------- no holdback, since SP4
|
||||
# ------------------------------------------- when a block forms (SETTLE_SLACK)
|
||||
|
||||
def test_a_block_forms_as_soon_as_the_story_holds_one(db, monkeypatch):
|
||||
"""Covered to action 5 with 12 actions: block 6-11 ends on the newest
|
||||
action, and is summarized now rather than a turn later.
|
||||
def test_a_block_that_ends_on_the_newest_action_waits(db, monkeypatch):
|
||||
"""Covered to action 5 with 12 actions: block 6-11 is full, but it ends on
|
||||
the newest action, so it is not written yet.
|
||||
|
||||
This is exactly the case the holdback existed to refuse. What makes it
|
||||
safe is no longer that the block stops short. It is that a retry of
|
||||
node 11 would withdraw this memory on its way past (see
|
||||
`test_deleting_a_summarized_node_withdraws_its_memory`, the same
|
||||
repair).
|
||||
Writing it would be correct. A retry of node 11 withdraws it on its way
|
||||
past, which is the same repair as
|
||||
`test_deleting_a_summarized_node_withdraws_its_memory`. It would also mean
|
||||
the block was summarized once, thrown away, and summarized again, for a
|
||||
memory that says nothing the history window does not still carry.
|
||||
"""
|
||||
adventure = make_adventure(db, 12)
|
||||
cover(db, adventure, 6)
|
||||
@@ -117,8 +122,19 @@ def test_a_block_forms_as_soon_as_the_story_holds_one(db, monkeypatch):
|
||||
stub = StubSummarizer()
|
||||
run_memories(db, adventure, stub, monkeypatch)
|
||||
|
||||
assert stub.excerpts == []
|
||||
assert covered_depth(db, adventure) == 5 # the mark stays where it was
|
||||
|
||||
# One more action settles the block. It is written from the same six: the
|
||||
# slack asks for story past the block, it does not grow the block.
|
||||
db.add(models.Action(adventure_id=adventure.id, type="ai", text="Action 12."))
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
run_memories(db, adventure, stub, monkeypatch)
|
||||
|
||||
assert len(stub.excerpts) == 1
|
||||
assert "Action 11." in stub.excerpts[0]
|
||||
assert "Action 12." not in stub.excerpts[0]
|
||||
memory = db.query(models.Memory).one()
|
||||
assert (memory.source_start, memory.source_end) == (6, 11)
|
||||
# The mark and the memory name the same node. That is what keeps them
|
||||
@@ -127,6 +143,26 @@ def test_a_block_forms_as_soon_as_the_story_holds_one(db, monkeypatch):
|
||||
assert covered_depth(db, adventure) == 11
|
||||
|
||||
|
||||
def test_a_retry_at_the_tip_has_no_memory_to_withdraw(db, monkeypatch):
|
||||
"""The slack, stated as the case it removes.
|
||||
|
||||
Retry and take-switching both refuse anything but the newest action
|
||||
(`takes.retry_action`, `takes.switch_take`), so holding the block end one
|
||||
action back puts every memory out of their reach. Nothing here is a claim
|
||||
about the withdrawal path, which undo and delete still reach at any node.
|
||||
"""
|
||||
adventure = make_adventure(db, 13)
|
||||
run_memories(db, adventure, StubSummarizer(), monkeypatch)
|
||||
|
||||
assert db.query(models.Memory).count() == 2 # blocks 0-5 and 6-11
|
||||
assert covered_depth(db, adventure) == 11
|
||||
|
||||
tip = history.newest(adventure)
|
||||
assert tip.depth == 12 # the deepest memory is a node behind it
|
||||
assert memorybank.forget_node(db, adventure, tip) == 0
|
||||
assert covered_depth(db, adventure) == 11 # so a retry rewinds nothing
|
||||
|
||||
|
||||
def test_the_first_memory_lands_at_memory_start(db, monkeypatch):
|
||||
adventure = make_adventure(db, memorybank.MEMORY_START - 1)
|
||||
stub = StubSummarizer()
|
||||
@@ -140,8 +176,19 @@ def test_the_first_memory_lands_at_memory_start(db, monkeypatch):
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
run_memories(db, adventure, stub, monkeypatch)
|
||||
# MEMORY_START is 12 actions = two full blocks, caught up in one run
|
||||
# (MAX_MEMORIES_PER_RUN allows 5), and the newest is in the second of them.
|
||||
# MEMORY_START is 12 actions = two full blocks. The first is settled,
|
||||
# because the second sits past it. The second ends on the newest action
|
||||
# and waits.
|
||||
assert len(stub.excerpts) == 1
|
||||
assert "Later." not in stub.excerpts[-1]
|
||||
assert covered_depth(db, adventure) == memorybank.MEMORY_INTERVAL - 1
|
||||
|
||||
# One action past the second block settles it too, and both are caught up
|
||||
# in one run (MAX_MEMORIES_PER_RUN allows 5).
|
||||
db.add(models.Action(adventure_id=adventure.id, type="ai", text="Even later."))
|
||||
db.commit()
|
||||
db.refresh(adventure)
|
||||
run_memories(db, adventure, stub, monkeypatch)
|
||||
assert len(stub.excerpts) == 2
|
||||
assert "Later." in stub.excerpts[-1]
|
||||
assert covered_depth(db, adventure) == memorybank.MEMORY_START - 1
|
||||
|
||||
+16
-4
@@ -392,7 +392,7 @@ raw turns → memories → story summary
|
||||
|
||||
| Layer | Cadence | Purpose |
|
||||
|---|---|---|
|
||||
| **Memory** | Every 6 actions, starting at 12 | One or two past-tense sentences of concrete fact. |
|
||||
| **Memory** | Every 6 actions, starting at 12, once one action sits past the block | One or two past-tense sentences of concrete fact. |
|
||||
| **Story summary** | Every 15 actions | A single ≤250-word overview of the whole plot, rewritten by folding in the new memories. |
|
||||
| **Retrieval** | Every turn | Embed the last 4 actions (≤600 tokens), cosine-rank the bank, inject the top K (default 5). |
|
||||
|
||||
@@ -413,8 +413,18 @@ This is also how repair works. When a turn's text is replaced or removed (a retr
|
||||
a deleted action), `forget_node` withdraws the memory attached to that coordinate and
|
||||
rewinds both marks to just before the stretch it covered, so the ground is summarized again
|
||||
from what the story now says. An earlier version instead held the newest action back a turn
|
||||
so it could never be summarized before it stopped being retryable. That is no longer
|
||||
needed, because the repair exists whether or not the invalidation happens at the tip.
|
||||
so it could never be summarized before it stopped being retryable. Correctness no longer
|
||||
rests on that, because the repair exists whether or not the invalidation happens at the tip.
|
||||
|
||||
**A block still waits for one action to settle past it** (`SETTLE_SLACK`), and that is a
|
||||
cost rule rather than a correctness one. Retry and take-switching both refuse anything but
|
||||
the newest action, so a memory whose block ends on the tip is the one memory a player can
|
||||
still throw away: every retry of that turn writes it, withdraws it, and writes it again. A
|
||||
block closes every 6 actions and a normal turn writes 2, so without the slack that is one
|
||||
turn in three. The slack costs nothing in return, because the block that just closed is
|
||||
still in the history window in full — a memory of it says what the model can already read.
|
||||
Memories earn their place once the raw text has scrolled out, which is never the turn the
|
||||
block closed.
|
||||
|
||||
**Cursors only advance on success.** Every AI call in this module is best-effort. If
|
||||
summarization fails, the function returns and the cursor is unchanged, so the same block is
|
||||
@@ -724,7 +734,9 @@ is ambiguous once two branches both have a node 41.
|
||||
|
||||
`position_of_index`, `note_action_removed`, `settled_story_actions` and the cursor-rewind
|
||||
machinery were **deleted**, not left unused. So was the one-turn memory holdback that
|
||||
existed because a retry could rewrite an action the mark had already passed.
|
||||
existed because a retry could rewrite an action the mark had already passed. (`SETTLE_SLACK`
|
||||
later put one action of slack back, for what redoing a block costs rather than for what it
|
||||
could get wrong. See "The memory system".)
|
||||
|
||||
### Derived work attaches to the node that produced it
|
||||
|
||||
|
||||
@@ -988,7 +988,7 @@ ORDER BY depth DESC LIMIT 32 -- → A0 A1 A2 A3 B4 B5 C6 C7</code></pre
|
||||
<h4>Cursors become anchors</h4>
|
||||
<p>The two marks, how far the memory bank has got and how far the summary has got, used to be counts. A count is a position in a list. Every rule about sliding, rewinding, and translating between positions and <code>Action.index</code> existed to patch up the fact that the list moves.</p>
|
||||
<p>A cursor is now an <strong>anchor</strong>: <code>(branch_id, depth)</code>, the node up to and including which the work is done. Deleting an action does not move it. “What is not covered yet?” becomes a question about the story instead of a list index, and it answers correctly no matter what has been deleted in front of it. The branch half is what makes it survive forking. A depth alone is ambiguous once two branches both have a node 41.</p>
|
||||
<p><code>position_of_index</code>, <code>note_action_removed</code>, <code>settled_story_actions</code>, and the cursor-rewind machinery were <strong>deleted</strong>, not left unused. So was the one-turn memory holdback that existed because a retry could rewrite an action the mark had already passed.</p>
|
||||
<p><code>position_of_index</code>, <code>note_action_removed</code>, <code>settled_story_actions</code>, and the cursor-rewind machinery were <strong>deleted</strong>, not left unused. So was the one-turn memory holdback that existed because a retry could rewrite an action the mark had already passed. (<code>SETTLE_SLACK</code> later put one action of slack back, for what redoing a block costs rather than for what it could get wrong.)</p>
|
||||
|
||||
<div class="trap">
|
||||
<span class="lab">Three findings only hand-driving produced</span>
|
||||
|
||||
+13
-3
@@ -733,7 +733,7 @@ into the inn, it should come back.</p>
|
||||
<table>
|
||||
<thead><tr><th>Layer</th><th>Cadence</th><th>Purpose</th></tr></thead>
|
||||
<tbody>
|
||||
<tr><td><strong>Memory</strong></td><td>every 6 actions, from 12</td><td>One or two past-tense sentences of concrete fact.</td></tr>
|
||||
<tr><td><strong>Memory</strong></td><td>every 6 actions, from 12, once one action sits past the block</td><td>One or two past-tense sentences of concrete fact.</td></tr>
|
||||
<tr><td><strong>Story summary</strong></td><td>every 15 actions</td><td>A single ≤250-word overview, rewritten by folding in the new memories.</td></tr>
|
||||
<tr><td><strong>Retrieval</strong></td><td>every turn</td><td>Embed the last 4 actions (≤600 tokens), cosine-rank the bank, inject the top 5.</td></tr>
|
||||
</tbody>
|
||||
@@ -755,8 +755,18 @@ across a fork: the ones above the fork point already sit on ancestors both lines
|
||||
action — <code>forget_node</code> withdraws the memory hanging off that coordinate <em>and</em>
|
||||
rewinds both marks to just before the stretch it covered, so that ground is summarized again from
|
||||
what the story now says. An earlier version instead held the newest action back a turn so it could
|
||||
never be summarized before it stopped being retryable; that is no longer needed, because the repair
|
||||
exists whether or not the invalidation happens at the tip.</p>
|
||||
never be summarized before it stopped being retryable; correctness no longer rests on that, because
|
||||
the repair exists whether or not the invalidation happens at the tip.</p>
|
||||
|
||||
<p><strong>A block still waits for one action to settle past it</strong>
|
||||
(<code>SETTLE_SLACK</code>), and that is a cost rule rather than a correctness one. Retry and
|
||||
take-switching both refuse anything but the newest action, so a memory whose block ends on the tip
|
||||
is the one memory a player can still throw away: every retry of that turn writes it, withdraws it,
|
||||
and writes it again. A block closes every 6 actions and a normal turn writes 2, so without the
|
||||
slack that is one turn in three. The slack costs nothing in return, because the block that just
|
||||
closed is still in the history window in full — a memory of it says what the model can already
|
||||
read. Memories earn their place once the raw text has scrolled out, which is never the turn the
|
||||
block closed.</p>
|
||||
|
||||
<p><strong>Cursors only advance on success.</strong> Every AI call here is best-effort. If
|
||||
summarization fails, the function returns and the cursor is unchanged, so the same block is
|
||||
|
||||
@@ -76,6 +76,44 @@ needed; nothing requires reading a row of anyone's story.
|
||||
|
||||
---
|
||||
|
||||
## What happened on 2026-08-31, part two — one action of settling slack
|
||||
|
||||
**A block is no longer summarized while it ends on the newest action.**
|
||||
`memorybank.SETTLE_SLACK` (1) asks for one action past a block before the block is
|
||||
written. The block is still `MEMORY_INTERVAL` actions; only the moment moves.
|
||||
|
||||
**This is not the SP4 holdback coming back, and the reasoning is worth keeping
|
||||
straight.** The holdback existed because a retry rewrote `Action.text` in place, so a
|
||||
memory could describe narration that no longer existed and nothing reported it. Sibling
|
||||
attempts plus `forget_node` settled that, and SP4 deleted the holdback on the grounds
|
||||
that "nothing is left to hold back". True — about staleness. Nobody priced the other
|
||||
half. Withdrawing a memory and writing it again is correct and it is also two calls for
|
||||
one block.
|
||||
|
||||
**Retry and take-switching both refuse anything but the newest action**
|
||||
(`takes.retry_action`, `takes.switch_take`), so the exposure was exactly one memory: the
|
||||
one whose block ends on the tip. A block closes every 6 actions and a normal turn writes
|
||||
2, so that was one turn in three, and each further retry of that turn paid again. The
|
||||
summary pass was never the expensive half — `rewind_all` takes the mark to the same
|
||||
depth however many times a player retries.
|
||||
|
||||
**The slack buys nothing back, which is why it is worth having.** The block that just
|
||||
closed is still inside the 16384-token history window in full, so a memory of it tells
|
||||
the model what it can already read. Memories earn their place once the raw text has
|
||||
scrolled out, and that is never the turn the block closed.
|
||||
|
||||
**`forget_node` stays, and its path is still reachable.** Undo and delete reach any node,
|
||||
and either can carry a summarized node back to the tip, where a retry of it lands on the
|
||||
withdrawal again. Rare now instead of routine.
|
||||
|
||||
632 tests green. Three test files moved with it: `test_memory_settling.py` asserts the
|
||||
new rule (and a retry at the tip finding nothing to withdraw), `test_memory_rewrite.py`
|
||||
builds 13 actions instead of 12 so both of its blocks settle, and the one path test in
|
||||
`test_memory_nodes.py` sets the slack to 0, because it is about which actions a block is
|
||||
read from rather than about when a block forms.
|
||||
|
||||
---
|
||||
|
||||
## What happened on 2026-08-31 — the persona, and what the summarizer is told
|
||||
|
||||
**`plan/18-persona-and-memory-quality.md` is the writeup. Both changes are on `main`**
|
||||
|
||||
Reference in New Issue
Block a user