diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index 0012b62..69d87b4 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -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 diff --git a/backend/app/routers/adventures/turns.py b/backend/app/routers/adventures/turns.py index 1e0369a..3f24d15 100644 --- a/backend/app/routers/adventures/turns.py +++ b/backend/app/routers/adventures/turns.py @@ -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, diff --git a/backend/tests/test_memory_nodes.py b/backend/tests/test_memory_nodes.py index da4b774..bcc4ede 100644 --- a/backend/tests/test_memory_nodes.py +++ b/backend/tests/test_memory_nodes.py @@ -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"] diff --git a/backend/tests/test_memory_rewrite.py b/backend/tests/test_memory_rewrite.py index 9790093..efc0e5b 100644 --- a/backend/tests/test_memory_rewrite.py +++ b/backend/tests/test_memory_rewrite.py @@ -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() diff --git a/backend/tests/test_memory_settling.py b/backend/tests/test_memory_settling.py index 4f98335..88ff77a 100644 --- a/backend/tests/test_memory_settling.py +++ b/backend/tests/test_memory_settling.py @@ -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 diff --git a/docs/GUIDE.md b/docs/GUIDE.md index 5eae3bc..40d4d10 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -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 diff --git a/docs/architecture.html b/docs/architecture.html index b7eddbe..49b14f2 100644 --- a/docs/architecture.html +++ b/docs/architecture.html @@ -988,7 +988,7 @@ ORDER BY depth DESC LIMIT 32 -- → A0 A1 A2 A3 B4 B5 C6 C7Cursors become anchors

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 Action.index existed to patch up the fact that the list moves.

A cursor is now an anchor: (branch_id, depth), 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.

-

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.

+

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. (SETTLE_SLACK later put one action of slack back, for what redoing a block costs rather than for what it could get wrong.)

Three findings only hand-driving produced diff --git a/docs/guide.html b/docs/guide.html index 30365e0..1db98b8 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -733,7 +733,7 @@ into the inn, it should come back.

- + @@ -755,8 +755,18 @@ across a fork: the ones above the fork point already sit on ancestors both lines action — forget_node withdraws the memory hanging off that coordinate and 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.

+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 here is best-effort. If summarization fails, the function returns and the cursor is unchanged, so the same block is diff --git a/plan/STATUS.md b/plan/STATUS.md index 792ed03..5eb94ca 100644 --- a/plan/STATUS.md +++ b/plan/STATUS.md @@ -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`**

LayerCadencePurpose
Memoryevery 6 actions, from 12One or two past-tense sentences of concrete fact.
Memoryevery 6 actions, from 12, once one action sits past the blockOne or two past-tense sentences of concrete fact.
Story summaryevery 15 actionsA single ≤250-word overview, rewritten by folding in the new memories.
Retrievalevery turnEmbed the last 4 actions (≤600 tokens), cosine-rank the bank, inject the top 5.