From a6e9c7a32bdf42f1e4cb837b70721d89422cef8d Mon Sep 17 00:00:00 2001 From: JesseMarkowitz Date: Sun, 6 Sep 2026 03:00:33 -0400 Subject: [PATCH] M6: branch-safe context, summaries and long-term story memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns the inherited AI-DnD memory and context foundation with the history, authority and state model M3-M5 established. Long stories now reach the narrator through a bounded, lineage-safe, inspectable context rather than a growing transcript. This commit includes the corrective work that followed the independent review in planning/reports/M6-IMPLEMENTATION-REPORT.md. The first implementation reported E03 as passing and it was not; the report records that history rather than hiding it. What was already correct, and was kept rather than rebuilt Memory lineage. Memories already carried (branch_id, depth) and retrieval already filtered through the capped-path clause; the ten-step negative control was measured passing against b7005e6 before any change here. M6 adds the regression tests that pin it, plus provenance and authority on the result. Summary lineage — both halves A summary is a row carrying the coordinate of the last node it covers, and eligibility is the same head-capped lineage clause memories use. That alone was not enough: generation was seeded from adventures.story_summary, a campaign-global column with no lineage, so after a divergence the summariser was handed the abandoned line's prose and asked to update it. The row it produced was correctly anchored and therefore looked safe while its sentences described a story the reader had left. Generation is now seeded from summaries.current — the same question the context builder asks — so the input and the output are scoped by one rule. adventures.story_summary remains a reader-facing mirror for the Plot panel and the export bundle, kept in step when a summary is written and when the head moves, and nothing authoritative reads it. Retrieval redundancy With a real embedding model, four near-identical memories crowded out the one distinctive clue, which survived only because the default memory_top_k is 5. Retrieval now drops a candidate that repeats one already chosen, never across authority classes, at a threshold measured against the configured embedding model. The clue is retrieved at top_k 5, 4 and 3. Ranking itself is unchanged; the further factors CONTEXT-AND-MEMORY §20 contemplates remain unimplemented and are recorded as such. Memory authority, budgeting, observability Memory.authority is accepted_story or heuristic, classified by the application and marked in the prompt; retrieval never writes state. The reply is reserved out of the context budget, and an impossible configuration fails clearly instead of overflowing. Each derived pass records ok/idle/failed per campaign, served by GET /adventures/{id}/derived and shown in Insights, so the M2 failure — a dead memory bank with a green suite — is visible if it recurs. Provider-wiring tests mock no factory. Also: two pre-existing test-suite leaks fixed; two fixtures that stored one vector in every memory now use distinct ones, so lineage assertions stay readable alongside redundancy suppression. Planning: CONTEXT-AND-MEMORY, TECHNICAL-DESIGN, DATA-MODEL, V1-ACCEPTANCE-TESTS, BUILD-MILESTONES, VERSION and planning/README updated to describe what exists, including that a valid E03 test must regenerate a summary after diverging. The M5 report was rotated to planning/archive/milestone-reports/. No new ADR — every choice implements a decision the package had already settled. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWU4gTfLYY6Qq9U7aa9Qw2 --- backend/app/attempts.py | 12 +- backend/app/context/__init__.py | 3 +- backend/app/context/builder.py | 99 +- backend/app/derived.py | 113 ++ backend/app/memorybank.py | 286 ++++- backend/app/migrations.py | 55 + backend/app/models.py | 105 +- backend/app/routers/adventures/crud.py | 15 +- backend/app/routers/adventures/insights.py | 51 +- backend/app/routers/adventures/turns.py | 17 +- backend/app/summaries.py | 155 +++ backend/tests/test_context_memory.py | 837 +++++++++++++ backend/tests/test_context_performance.py | 202 ++++ backend/tests/test_context_realistic.py | 238 ++++ backend/tests/test_history_window.py | 7 +- backend/tests/test_length_hint.py | 7 +- backend/tests/test_memory_nodes.py | 55 +- backend/tests/test_memory_retrieval.py | 17 + backend/tests/test_prompt_caching.py | 10 +- backend/tests/test_provider_wiring.py | 225 ++++ backend/tests/test_tree_migration.py | 2 + .../src/pages/Play/panels/InsightsPanel.jsx | 60 +- frontend/src/styles/play.css | 7 + planning/BUILD-MILESTONES.md | 81 +- planning/CONTEXT-AND-MEMORY.md | 106 ++ planning/DATA-MODEL.md | 34 + planning/README.md | 20 +- planning/TECHNICAL-DESIGN.md | 73 ++ planning/V1-ACCEPTANCE-TESTS.md | 123 ++ planning/VERSION.md | 49 +- .../M5-IMPLEMENTATION-REPORT.md | 0 planning/reports/M6-IMPLEMENTATION-REPORT.md | 1060 +++++++++++++++++ 32 files changed, 4040 insertions(+), 84 deletions(-) create mode 100644 backend/app/derived.py create mode 100644 backend/app/summaries.py create mode 100644 backend/tests/test_context_memory.py create mode 100644 backend/tests/test_context_performance.py create mode 100644 backend/tests/test_context_realistic.py create mode 100644 backend/tests/test_provider_wiring.py rename planning/{reports => archive/milestone-reports}/M5-IMPLEMENTATION-REPORT.md (100%) create mode 100644 planning/reports/M6-IMPLEMENTATION-REPORT.md diff --git a/backend/app/attempts.py b/backend/app/attempts.py index d4632fc..f0dca46 100644 --- a/backend/app/attempts.py +++ b/backend/app/attempts.py @@ -34,9 +34,9 @@ agreed in every group. import copy -from sqlalchemy.orm import Session, undefer +from sqlalchemy.orm import Session, object_session, undefer -from . import models +from . import models, summaries from .context import lineage from .narrative import model as narrative_model @@ -191,6 +191,14 @@ def restore_state(adventure: models.Adventure, node: models.Action | None) -> No if isinstance(node.narrative_state_after, dict) else narrative_model.empty() ) + # M6: the reader-facing summary mirror follows the head too. It is a + # convenience column with no lineage of its own, so without this it would go + # on showing a summary belonging to a position the story has left. Nothing + # authoritative reads it — the prompt takes its summary from + # `summaries.current` — but the Plot panel and the export bundle do. + session = object_session(adventure) + if session is not None: + summaries.refresh_mirror(session, adventure) # Legacy, and deliberately still restored: a pre-M5 campaign's numbers stay # coherent with the position being read, so an old save is not left showing # a future's values. Nothing consults them to decide anything. diff --git a/backend/app/context/__init__.py b/backend/app/context/__init__.py index cd3be11..694eba8 100644 --- a/backend/app/context/__init__.py +++ b/backend/app/context/__init__.py @@ -1,5 +1,6 @@ from . import history from .builder import ( + ContextOverflow, build_context, count_tokens, match_cards, @@ -9,7 +10,7 @@ from .builder import ( from .history import story_actions __all__ = [ - "build_context", + "ContextOverflow", "build_context", "count_tokens", "history", "match_cards", diff --git a/backend/app/context/builder.py b/backend/app/context/builder.py index 31e9a36..e296c36 100644 --- a/backend/app/context/builder.py +++ b/backend/app/context/builder.py @@ -21,8 +21,9 @@ block and the live sections in `build_context`. from dataclasses import dataclass import tiktoken +from sqlalchemy.orm import object_session -from .. import models, narrative, worldstate +from .. import derived, models, narrative, summaries, worldstate from . import encoding, history AUTHORS_NOTE_DEPTH = 3 # actions from the end of history @@ -63,6 +64,24 @@ MAX_LENGTH_FLOOR_WORDS = 300 # Built from the table vendored in `encoding.py`, not fetched: the upstream # `tiktoken.get_encoding("cl100k_base")` downloads it on first use, and this # is called on every turn. +# M6: added to the configured reply budget when reserving output space. It +# absorbs the section separators added after budgeting and the drift between +# this tokenizer and the serving model's. Fixed rather than proportional: what +# it covers does not grow with the size of the budget. +OUTPUT_SAFETY_MARGIN = 64 + + +class ContextOverflow(RuntimeError): + """Raised when protected context alone cannot fit in the token budget. + + Protected means the narrator rules, the campaign canon, the authoritative + narrative state, the reader's own input, and the reserve for the reply + (`CONTEXT-AND-MEMORY.md` §30). None of those may be dropped to make room for + old prose, so when they do not fit there is no prompt to build and saying so + is the only honest answer. + """ + + def _encoding() -> tiktoken.Encoding: return encoding.get_encoding() @@ -187,6 +206,12 @@ def _history_text(action: models.Action) -> str: return action.text +def _memory_line(memory: dict) -> str: + """One retrieved memory, marked with its authority (M6).""" + mark = " [inferred]" if memory.get("authority") == "heuristic" else "" + return f"-{mark} {memory['text']}" + + def _canon_section(adventure: models.Adventure) -> str: """The campaign's own rules, rendered for the system block. @@ -317,15 +342,33 @@ def build_context( # memories change on most turns, and the stat values change on nearly every # turn. `world_lore` is added below, because the history window determines # which cards trigger and that window is not known yet. + # M6: the summary the *current lineage* is entitled to, not whatever was + # written last. A summary is derived data anchored to the story it covers, + # so an Undo or a divergence makes an old one ineligible rather than + # leaking it into a story it does not describe (E03, `app/summaries.py`). + db = object_session(adventure) + summary_row = summaries.current(db, adventure) if db is not None else None + summary_text = summary_row.text.strip() if summary_row is not None else "" summary_section = ( - Section("story_summary", f"Story summary:\n{adventure.story_summary.strip()}") - if adventure.story_summary.strip() + Section("story_summary", f"Story summary:\n{summary_text}") + if summary_text else None ) memories_section = None if memory_bank and memory_bank.get("used"): - lines_text = "\n".join(f"- {m['text']}" for m in memory_bank["used"]) - memories_section = Section("used_memories", f"Memories:\n{lines_text}") + # M6: an inference must not read as a record. A heuristic memory is + # marked in the prompt itself, because the narrator decides what to + # treat as established from what it is shown, and an unlabelled guess + # sitting beside accepted history is how a guess becomes canon + # (`CONTEXT-AND-MEMORY.md` §14). Authoritative state changes still come + # only from the M5 event path, whatever a memory says. + lines_text = "\n".join(_memory_line(m) for m in memory_bank["used"]) + memories_section = Section( + "used_memories", + "Memories from earlier in the story. Lines marked [inferred] are " + "interpretation, not established fact — do not treat them as " + f"settled truth:\n{lines_text}", + ) world_state_section = None refusal_note = "" # M5: the authoritative narrative state, as the model is shown it. Read from @@ -370,7 +413,35 @@ def build_context( + count_tokens(narrative.extract.EMIT_REMINDER) + count_tokens(refusal_note) ) - available = max(256, settings.context_token_budget - reserved) + + # ----- M6: the output reserve, and what happens when it does not fit ----- + # + # `context_token_budget` is the whole window the model is given, so the + # narrator's reply has to be subtracted from it before any history is + # chosen. Until M6 it was not: the builder spent the entire budget on input + # and left the reply to fit in whatever the endpoint had left, which is a + # truncated turn on a model whose window is the budget + # (`CONTEXT-AND-MEMORY.md` §32, acceptance test F04). + # + # The margin covers what is added after this arithmetic — the separators + # between sections, and the difference between our tokenizer's count and the + # serving model's. It is small and fixed rather than proportional, because + # what it absorbs does not scale with the budget. + output_reserve = max(0, settings.max_output_tokens) + OUTPUT_SAFETY_MARGIN + protected = reserved + output_reserve + if protected >= settings.context_token_budget: + # Failing here is the point. The alternative — carrying on with a token + # or two of history — builds a prompt that is known to overflow, and + # the reader gets a truncated reply with no explanation. §32: "fail + # gracefully if protected context alone is too large." + raise ContextOverflow( + f"The protected context needs {protected} tokens " + f"({reserved} of prompt plus {output_reserve} reserved for the " + f"reply) but the context budget is {settings.context_token_budget}. " + "Raise the context budget, lower the maximum reply length, or " + "shorten the campaign's canon, instructions and persona." + ) + available = settings.context_token_budget - protected # Only the newest actions can reach the prompt, because the code below # either truncates the text to `available` tokens or stops at the budget. @@ -475,12 +546,28 @@ def build_context( {"label": s.label, "text": s.text, "tokens": s.tokens} for s in all_sections ], "prompt": {"system": system_text, "story": story_text}, + # M6: the numbers the reader needs to answer "how much did each part + # cost, and what was left for the reply?" (F04, F05). `available` is + # what the history was actually allowed to spend after everything + # protected was subtracted. "tokens": { "total": count_tokens(system_text) + count_tokens(story_text), "budget": settings.context_token_budget, + "output_reserve": output_reserve, + "protected": reserved, + "available_for_history": available, + "history_spent": spent, }, "cards": card_records, "memories": memory_bank, + # M6: which summary was used, and which stretch of story it covers, so + # "what history did that summary cover?" is answerable from the record + # rather than by guessing (F05, F06). + "summary": summaries.provenance(summary_row), + # M6: whether background derived work is currently failing for this + # campaign. A dead memory bank is visible here rather than only in a log + # nobody reads (F08). + "derived": derived.report(db, adventure.id) if db is not None else [], "history": { "included": len(included_actions), # The count covers the whole story rather than the window fetched diff --git a/backend/app/derived.py b/backend/app/derived.py new file mode 100644 index 0000000..22b5f51 --- /dev/null +++ b/backend/app/derived.py @@ -0,0 +1,113 @@ +"""M6: recording whether background derived work succeeded, and why not. + +M2 shipped with the entire memory bank dead and the full test suite green. The +summariser and the embedder raised `AttributeError` inside a fire-and-forget +task: no user-visible error, no log a player would look at, and no failing test, +because every memory test stubbed the provider factories out +(`BUILD-MILESTONES.md`, note from M2; `M2-IMPLEMENTATION-REPORT.md` §A.1). + +Two rules follow, and they pull in opposite directions: + +* **Derived work must fail softly.** A memory that could not be written, a + summary that could not be generated, an embedding the endpoint refused — + none of these may roll back the accepted narration, the accepted state + events, the authoritative document, the head, or the transcript. The story + turn already happened; the derived work is a commentary on it. +* **It must fail visibly.** Soft failure without a record is what M2 shipped. + +So each attempt writes its outcome to one row per (campaign, kind), and that row +is readable through the API. This is deliberately not a job framework: it holds +what happened last, not a queue. Retrying is just running the pass again, which +the ordinary post-turn path already does on the next accepted turn. +""" + +from __future__ import annotations + +import logging + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from . import models + +log = logging.getLogger(__name__) + +# The kinds of derived work. Each is independent: embeddings can be failing +# while summaries succeed, and a reader should be able to see exactly that. +MEMORY = "memory" +SUMMARY = "summary" +EMBEDDING = "embedding" +KINDS = (MEMORY, SUMMARY, EMBEDDING) + + +def _row(db: Session, adventure_id: int, kind: str) -> models.DerivedStatus: + row = db.execute( + select(models.DerivedStatus).where( + models.DerivedStatus.adventure_id == adventure_id, + models.DerivedStatus.kind == kind, + ) + ).scalars().first() + if row is None: + row = models.DerivedStatus(adventure_id=adventure_id, kind=kind) + db.add(row) + return row + + +def succeeded(db: Session, adventure_id: int, kind: str, *, did_work: bool = True) -> None: + """Records a clean run, clearing any standing failure. + + `did_work` separates a pass that produced something from one that found + nothing to do (M6 review finding M6-F5). Both are healthy, and neither is a + failure, but reporting "ok" for a pass that has never actually run reads as + "embeddings are working" when nothing has been embedded. `idle` says the + true thing: it ran, and there was nothing pending. + """ + row = _row(db, adventure_id, kind) + row.status = "ok" if did_work else "idle" + row.detail = "" + row.failures = 0 + row.last_attempt_at = models.utcnow() + if did_work: + row.last_success_at = row.last_attempt_at + + +def failed(db: Session, adventure_id: int, kind: str, exc: BaseException) -> None: + """Records a failed run, keeping the reason where someone can find it. + + The detail is the exception's type and message rather than a traceback: it + is shown to a reader in the Insights panel, and `ProviderError: connection + refused` is the part that tells them what to do. The traceback goes to the + log for a maintainer. + """ + row = _row(db, adventure_id, kind) + row.status = "failed" + row.detail = f"{type(exc).__name__}: {exc}"[:2000] + row.failures = (row.failures or 0) + 1 + row.last_attempt_at = models.utcnow() + log.exception("derived %s work failed for adventure %s", kind, adventure_id) + + +def report(db: Session, adventure_id: int) -> list[dict]: + """Every kind's last outcome, for the API and the prompt inspector.""" + rows = db.execute( + select(models.DerivedStatus) + .where(models.DerivedStatus.adventure_id == adventure_id) + .order_by(models.DerivedStatus.kind) + ).scalars().all() + return [ + { + "kind": row.kind, + "status": row.status, + "detail": row.detail, + "failures": row.failures, + "last_attempt_at": row.last_attempt_at.isoformat() if row.last_attempt_at else None, + "last_success_at": row.last_success_at.isoformat() if row.last_success_at else None, + } + for row in rows + ] + + +def failing(db: Session, adventure_id: int) -> list[str]: + """The kinds currently in a failed state, for a compact UI badge.""" + return [entry["kind"] for entry in report(db, adventure_id) + if entry["status"] == "failed"] diff --git a/backend/app/memorybank.py b/backend/app/memorybank.py index 586928b..663f543 100644 --- a/backend/app/memorybank.py +++ b/backend/app/memorybank.py @@ -27,13 +27,14 @@ succeeds. """ import asyncio +import logging from array import array from collections import OrderedDict from sqlalchemy import func, select, update from sqlalchemy.orm import Session, defer, object_session -from . import models, tree, vectors +from . import derived, models, summaries, tree, vectors from .context import ( cursors, history, @@ -46,6 +47,8 @@ from .database import SessionLocal from .providers import OpenAICompatibleProvider, ProviderError from .vectors import cosine # re-exported: the ranking lives here, the maths there +log = logging.getLogger(__name__) + MEMORY_INTERVAL = 6 # actions per memory MEMORY_START = 12 # first memory once the adventure reaches this many actions SUMMARY_INTERVAL = 15 # actions between Story Summary updates @@ -146,6 +149,104 @@ _tasks: set[asyncio.Task] = set() # They used to also read an API key, which is gone: Ollama does not use one and # M2 removed cloud providers. `summary_model` and `embedding_model` fall back to # the narrator model when the user has not named a separate one. +# M6: the words that mark a memory as an interpretation rather than a record. +# +# The application owns this classification, not the model +# (`CONTEXT-AND-MEMORY.md` §14, §15). The extractor writes prose; this decides +# what weight the narrator is told to give it. The list is deliberately short +# and readable: a memory that hedges is a reading of the story, not a fact the +# story established, and the narrator must not be able to promote it to canon. +# +# Being wrong in the cautious direction is cheap — a hedged record labelled +# heuristic is still retrieved and still useful. Being wrong the other way is +# what turns a guess into canon, which is the failure §14 exists to prevent. +HEURISTIC_MARKERS = ( + "seemed", "seems", "appeared to", "appears to", "apparently", "perhaps", + "maybe", "might have", "may have", "possibly", "presumably", "suggested that", + "suggests that", "implied", "implies", "as if", "likely", "probably", + "seemingly", "hinted", "hints that", "suspects", "suspected", "believes", + "believed", "wondered whether", "wonders whether", +) + +ACCEPTED_STORY = "accepted_story" +HEURISTIC = "heuristic" + + +# M6 corrective (review finding M6-F2). How close two memories have to be before +# the second one is treated as saying nothing new. +# +# The value is measured, not guessed. Against the configured local embedding +# model, on a fixture of near-identical "the party walks the muddy road" +# memories and a set of genuinely distinct ones: +# +# redundant pairs cosine 0.938 - 0.996 +# distinct pairs cosine 0.349 - 0.906 +# +# 0.93 sits in that gap. The same measurement ruled out the more obvious +# lexical test: word overlap fires hardest on exactly the pair that must NOT be +# merged — "Mara promised to return before dawn" against "Aldric promised to +# return before dawn" shares 71% of its words while meaning something else — +# and is weakest (27%) on filler that plainly repeats itself. Wording is a poor +# proxy for sameness of fact; the embedding is a better one. +# +# The threshold is model-dependent by nature. A different embedding model may +# need a different number, which is why the measurement is written down here +# rather than the value alone. +REDUNDANT_SIMILARITY = 0.93 + + +def _drop_redundant(candidates, vectors, authority_of, limit): + """Fills `limit` slots, skipping memories that repeat one already chosen. + + Greedy over the ranked list, so the highest-scoring statement of a fact is + the one kept and its provenance is the provenance that survives. Two rules + keep this from losing information: + + * **Authority is never crossed.** An inference and a record are different + kinds of claim even when they read alike, so a `heuristic` memory can + never suppress an `accepted_story` one or the reverse. + * **The bar is high.** Missing a duplicate costs some budget; dropping a + distinct fact costs the narrator something it needed. The threshold is + set where the measurement says distinct facts stop appearing. + + Returns `(kept, suppressed)`, the second for the inspector — a reader + should be able to see that memories were considered and set aside rather + than never retrieved. + """ + kept: list = [] + suppressed: list = [] + for row in candidates: + if len(kept) >= limit: + break + _score, memory_id, _pinned = row + vector = vectors.get(memory_id) + duplicate_of = None + if vector is not None: + for _kept_score, kept_id, _ in kept: + if authority_of.get(kept_id) != authority_of.get(memory_id): + continue + other = vectors.get(kept_id) + if other is not None and cosine(vector, other) >= REDUNDANT_SIMILARITY: + duplicate_of = kept_id + break + if duplicate_of is None: + kept.append(row) + else: + suppressed.append((memory_id, duplicate_of)) + return kept, suppressed + + +def classify_authority(text: str) -> str: + """Returns `accepted_story` or `heuristic` for one memory's text. + + Hedged language is the signal. "Aldric promised Mara he would return before + dawn" is something the story established; "Mara seemed uneasy when Captain + Vale was mentioned" is an inference about it, and the prompt has to say so. + """ + lowered = text.lower() + return HEURISTIC if any(m in lowered for m in HEURISTIC_MARKERS) else ACCEPTED_STORY + + def summary_provider(settings: models.Settings) -> OpenAICompatibleProvider: return OpenAICompatibleProvider( settings.endpoint_url, @@ -473,7 +574,7 @@ async def retrieve_memories( # affordable because memories are sparse, at roughly one per six actions, so # even a heavily forked story returns only tens of small rows. catalogue = db.execute( - select(models.Memory.id, models.Memory.pinned).where( + select(models.Memory.id, models.Memory.pinned, models.Memory.authority).where( models.Memory.adventure_id == adventure.id, lineage.path_of(db, adventure).clause(models.Memory), models.Memory.forgotten.is_(False), @@ -495,11 +596,12 @@ async def retrieve_memories( except ProviderError as exc: return {"used": [], "error": str(exc)} - held = _vectors_for(db, adventure.id, [memory_id for memory_id, _ in catalogue]) + held = _vectors_for(db, adventure.id, [memory_id for memory_id, _, _ in catalogue]) + authority_of = {memory_id: authority for memory_id, _, authority in catalogue} scored = sorted( ( (cosine(query_vec, held[memory_id]), memory_id, pinned) - for memory_id, pinned in catalogue + for memory_id, pinned, _ in catalogue if memory_id in held ), key=lambda row: row[0], @@ -511,19 +613,31 @@ async def retrieve_memories( top_k = max(1, settings.memory_top_k) used = [row for row in scored if row[2]] remaining = max(0, top_k - len(used)) - used += [row for row in scored if not row[2]][:remaining] + candidates = [row for row in scored if not row[2]] + kept, suppressed = _drop_redundant(candidates, held, authority_of, remaining) + used += kept used.sort(key=lambda row: row[0], reverse=True) if not used: return {"used": [], "error": None} # Fetch the text only now, and only for the `top_k` rows that were chosen. + # + # M6 adds authority and provenance to this same read rather than to a second + # one. The columns are narrow, the row set is `top_k`, and fetching them + # here is what keeps "why did the narrator remember this?" answerable + # without a query per memory (F06, and the N+1 discipline M5 restored). used_ids = [memory_id for _, memory_id, _ in used] - texts = dict( - db.execute( - select(models.Memory.id, models.Memory.text) - .where(models.Memory.id.in_(used_ids)) + detail = { + row.id: row + for row in db.execute( + select( + models.Memory.id, models.Memory.text, models.Memory.authority, + models.Memory.branch_id, models.Memory.depth, + models.Memory.source_start, models.Memory.source_end, + ).where(models.Memory.id.in_(used_ids)) ).all() - ) + } + texts = {memory_id: row.text for memory_id, row in detail.items()} if update_stats: # Pass `synchronize_session=False` because nothing in this request @@ -539,10 +653,29 @@ async def retrieve_memories( return { "used": [ - {"id": memory_id, "text": texts.get(memory_id, ""), - "similarity": round(score, 4), "pinned": pinned} + { + "id": memory_id, + "text": texts.get(memory_id, ""), + "similarity": round(score, 4), + "pinned": pinned, + # M6: what weight this carries, and where it came from. + "authority": getattr(detail.get(memory_id), "authority", ACCEPTED_STORY), + "source": { + "branch_id": getattr(detail.get(memory_id), "branch_id", None), + "depth": getattr(detail.get(memory_id), "depth", None), + "source_start": getattr(detail.get(memory_id), "source_start", None), + "source_end": getattr(detail.get(memory_id), "source_end", None), + }, + } for score, memory_id, pinned in used ], + "considered": len(catalogue), + # M6: how many candidates were set aside as repeating one already + # chosen. Visible so that "why is that memory not here?" has an answer. + "suppressed": [ + {"id": memory_id, "duplicate_of": kept_id} + for memory_id, kept_id in suppressed + ], "error": None, } @@ -587,17 +720,55 @@ async def run_post_turn(adventure_id: int) -> None: # An anchor past the tip is not an invalid value. `settled_after` # reports that there is nothing to do, and once the story grows past the # anchor the pass resumes where it stopped. + # M6. Each kind runs inside its own recorder, so one failing pass + # neither hides the others nor takes the turn down with it. The accepted + # narration, its state events, the authoritative document and the head + # were all committed before this task started; nothing here may undo + # them, and nothing here may fail without leaving a record + # (`BUILD-MILESTONES.md`, note from M2). if adventure.auto_summarize: - await _create_due_memories(adventure, settings, db) - await _update_story_summary(adventure, settings, db) + await _guarded(db, adventure_id, derived.MEMORY, + _create_due_memories(adventure, settings, db)) + await _guarded(db, adventure_id, derived.SUMMARY, + _update_story_summary(adventure, settings, db)) if adventure.memory_bank_enabled and settings.embedding_model.strip(): - await _embed_pending(adventure, settings, db) + await _guarded(db, adventure_id, derived.EMBEDDING, + _embed_pending(adventure, settings, db)) _evict_over_capacity(adventure, settings, db) + except BaseException as exc: # noqa: BLE001 - the task boundary + # Anything the per-kind guards did not catch: a failure in the shared + # setup above, or in eviction. M2's lesson is that the one thing this + # may not do is vanish. Re-raising would only feed an unobserved task. + try: + derived.failed(db, adventure_id, derived.MEMORY, exc) + db.commit() + except BaseException: # noqa: BLE001 - the recorder must not mask it + log.exception("could not record derived-work failure for %s", adventure_id) finally: db.close() _running.discard(adventure_id) +async def _guarded(db: Session, adventure_id: int, kind: str, coro) -> None: + """Runs one derived pass, recording whether it worked. + + The pass keeps whatever it committed before it failed — a memory written + two blocks ago stays written — because derived work is additive and + partial progress is still progress. What must not survive is an + uncommitted, half-written unit of work, so the session is rolled back to + the last commit before the failure is recorded. + """ + try: + did_work = await coro + except BaseException as exc: # noqa: BLE001 - one kind must not stop another + db.rollback() + derived.failed(db, adventure_id, kind, exc) + db.commit() + else: + derived.succeeded(db, adventure_id, kind, did_work=bool(did_work)) + db.commit() + + async def summarize_block( adventure: models.Adventure, provider: OpenAICompatibleProvider, @@ -629,37 +800,39 @@ async def summarize_block( async def _create_due_memories( adventure: models.Adventure, settings: models.Settings, db: Session -) -> None: +) -> int: + """Writes the memories that are due. Returns how many it wrote (M6-F5).""" provider = summary_provider(settings) + written = 0 for _ in range(MAX_MEMORIES_PER_RUN): # Re-read the anchor on every pass. Committing a memory does not change # 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 + SETTLE_SLACK: - return # No settled block of story sits past the mark. The block + return written # 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. + return written # The adventure is too short to have started summarizing. # The order of those two checks is deliberate. The usual answer is that # no memory is due, and the first check settles that without measuring # the length of the whole story. block = history.after(adventure, anchor, MEMORY_INTERVAL) if len(block) < MEMORY_INTERVAL: - return - try: - text = await summarize_block(adventure, provider, block) - except ProviderError: - return # Logged on the debug page. The cursor is unchanged, so the - # next turn retries this block. + return written + # A provider failure is no longer caught here. `_guarded` records it + # against this campaign, and the cursor is unchanged either way, so the + # next accepted turn retries this same block (M6). + text = await summarize_block(adventure, provider, block) if not text: - return + return written memory = models.Memory( adventure_id=adventure.id, text=text, source_start=block[0].depth, source_end=block[-1].depth, + authority=classify_authority(text), ) # Attach the memory to the node it summarizes, so that a fork inherits # the memories of the path it forked from and no others. Then move the @@ -670,22 +843,27 @@ async def _create_due_memories( db.add(memory) cursors.MEMORY.anchor_at(adventure, block[-1]) db.commit() + written += 1 + return written async def _update_story_summary( adventure: models.Adventure, settings: models.Settings, db: Session -) -> None: +) -> bool: + """Rolls the summary forward when enough new story has settled. + + Returns whether it wrote one (M6-F5).""" anchor = cursors.SUMMARY.depth(db, adventure) uncovered = history.count_after(adventure, anchor) if uncovered < SUMMARY_INTERVAL: - return + return False # Where the summary stands once this run succeeds. Read this before the AI # call rather than after it. The mark records the end of the story as this # pass saw it, and a turn that arrives during the call must not be counted # as read. caught_up = history.newest(adventure) if caught_up is None: - return + return False # Include the memories for the stretch that the summary has not read, which # means every memory attached to a node past the anchor. The marks and the @@ -707,7 +885,25 @@ async def _update_story_summary( block = history.after(adventure, anchor, uncovered) events_text = truncate_to_last_tokens("\n\n".join(a.text for a in block), 2000) - current = adventure.story_summary.strip() + # M6 corrective (review finding M6-F1). The previous summary this one + # builds on has to be a summary that is *valid where the story now stands*, + # not merely the last one written. + # + # Seeding from `adventure.story_summary` — a campaign-global column with no + # lineage — is what broke E03. After a divergence that column still held the + # abandoned line's prose, so the summariser was handed it and asked to + # update it. The row it produced was correctly anchored to the new branch + # and was therefore *reported* as lineage-safe, while its sentences + # described a story the reader had left. The row was anchored; the content + # was not. + # + # `summaries.current` answers the same question the context builder asks — + # which summary is eligible at the head — so the input and the output are + # now scoped by one rule. Where no eligible summary exists, the new line + # starts from nothing, which is the truthful starting point for a story + # that has not been summarised yet. + eligible = summaries.current(db, adventure) + current = eligible.text.strip() if eligible is not None else "" # The summary is built from the memories, so it inherits their framing for # free once they are named and third-person. It still gets the brief of its # own, because the fallback above hands it raw second-person story text @@ -720,22 +916,31 @@ async def _update_story_summary( ) if brief: user_prompt = f"{brief}\n\n{user_prompt}" - try: - text = await summary_provider(settings).complete( - SUMMARY_SYSTEM_PROMPT, user_prompt, max_tokens=600 - ) - except ProviderError: - return + text = await summary_provider(settings).complete( + SUMMARY_SYSTEM_PROMPT, user_prompt, max_tokens=600 + ) if not text: - return - adventure.story_summary = text + return False + # M6: anchored to the story it summarizes rather than written into a single + # column. `caught_up` is the last node it covers, so the row is eligible on + # exactly the lineages that contain that node, and an Undo or a divergence + # makes it ineligible without deleting it (E03, `summaries` module). + summaries.record( + db, adventure, text, + node=caught_up, + source_start=anchor + 1 if anchor is not None else None, + trigger="interval", + model_name=(settings.summary_model or settings.model or ""), + ) cursors.SUMMARY.anchor_at(adventure, caught_up) db.commit() + return True async def _embed_pending( adventure: models.Adventure, settings: models.Settings, db: Session -) -> None: +) -> int: + """Embeds memories that have no vector. Returns how many (M6-F5).""" # Use a query rather than walking `adventure.memories`. That walk ran on # every turn and loaded the whole bank's vectors in order to find the few # rows with none. @@ -759,14 +964,15 @@ async def _embed_pending( .all() ) if not pending: - return + return 0 try: new = await embedding_provider(settings).embed([m.text for m in pending]) except ProviderError: - return + return 0 for memory, vector in zip(pending, new): set_vector(memory, vector) db.commit() + return len(pending) def _evict_over_capacity( diff --git a/backend/app/migrations.py b/backend/app/migrations.py index 98cdb88..97825a9 100644 --- a/backend/app/migrations.py +++ b/backend/app/migrations.py @@ -402,6 +402,15 @@ MIGRATIONS: list[tuple[int, str | dict[str, str]]] = [ # so that every position an existing campaign can be restored to has a # snapshot. See `_backfill_narrative_snapshots`. (88, "-- narrative snapshot backfill (data pass only)"), + # M6. `create_all` builds the two new tables — `summaries` and + # `derived_status` — as it did `state_events` and `checkpoints`. These are + # the columns it cannot add to a table that already exists, plus the data + # pass that moves an existing campaign's summary onto the lineage. + (89, "ALTER TABLE memories ADD COLUMN authority VARCHAR(20) " + "NOT NULL DEFAULT 'accepted_story'"), + (90, "CREATE INDEX IF NOT EXISTS ix_summaries_adventure " + "ON summaries (adventure_id, depth)"), + (91, "-- move the existing story summary onto the lineage (data pass only)"), ] LATEST_VERSION = max((v for v, _ in MIGRATIONS), default=1) @@ -416,6 +425,7 @@ CURSOR_ANCHOR_VERSION = 56 SIBLING_SPLIT_VERSION = 60 PARENT_BACKFILL_VERSION = 64 NARRATIVE_SNAPSHOT_VERSION = 88 +SUMMARY_LINEAGE_VERSION = 91 # An adventure with no actions has no tip. A value of -1 keeps the rule that the # next node goes at `head_depth + 1` true without a special case. This matches @@ -433,6 +443,49 @@ SNAPSHOT_BATCH = 50 BACKFILL_BATCH = 200 +def _backfill_summary_lineage(conn) -> None: + """Moves each campaign's existing summary onto the lineage that produced it. + + Before M6 the rolling summary lived in `adventures.story_summary` with a + separate `(branch_id, depth)` cursor recording how far it had read. The + cursor is exactly the coordinate the summary belongs at, so the existing + text becomes a `summaries` row anchored there and keeps working — including + becoming ineligible after an Undo or a divergence, which is what it could + not do before. + + A campaign whose cursor never moved (`summary_cursor_branch_id` NULL) has a + summary somebody typed rather than one the pass produced. That anchors at + the head instead, which is where a hand-written summary belongs. + + One statement, no row loop. The column is left in place: it is the Plot + panel's edit surface and the export bundle's field, and it now mirrors + whichever summary is eligible. + """ + conn.execute(text( + """ + INSERT INTO summaries ( + adventure_id, text, branch_id, depth, source_start, source_end, + trigger, model_name, created_at + ) + SELECT + a.id, + a.story_summary, + COALESCE(a.summary_cursor_branch_id, a.head_branch_id), + CASE WHEN a.summary_cursor_branch_id IS NULL + THEN a.head_depth ELSE a.summary_cursor_depth END, + NULL, + CASE WHEN a.summary_cursor_branch_id IS NULL + THEN a.head_depth ELSE a.summary_cursor_depth END, + CASE WHEN a.summary_cursor_branch_id IS NULL + THEN 'manual' ELSE 'interval' END, + '', + CURRENT_TIMESTAMP + FROM adventures a + WHERE TRIM(COALESCE(a.story_summary, '')) <> '' + """ + )) + + def _backfill_narrative_snapshots(conn) -> None: """Gives every pre-M5 action the empty narrative document as its outcome. @@ -1198,5 +1251,7 @@ def bootstrap(engine: Engine, through: int = LATEST_VERSION) -> None: _backfill_parents(conn) if version == NARRATIVE_SNAPSHOT_VERSION: _backfill_narrative_snapshots(conn) + if version == SUMMARY_LINEAGE_VERSION: + _backfill_summary_lineage(conn) current = version _set_version(conn, current) diff --git a/backend/app/models.py b/backend/app/models.py index 5a63967..fd1d56a 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -2,7 +2,7 @@ from datetime import datetime, timezone from sqlalchemy import ( JSON, Boolean, DateTime, Float, ForeignKey, Index, Integer, LargeBinary, - String, Text, event, + String, Text, UniqueConstraint, event, ) from sqlalchemy.orm import Mapped, Session, mapped_column, relationship @@ -98,6 +98,19 @@ class Adventure(Base): memory: Mapped[str] = mapped_column(Text, default="") authors_note: Mapped[str] = mapped_column(Text, default="") ai_instructions: Mapped[str] = mapped_column(Text, default="") + # A convenience mirror of whichever summary is eligible at the current + # position, and **never** an input to anything authoritative (M6 corrective, + # review finding M6-F1). + # + # It exists because the Plot panel lets a reader read and edit the summary + # and the export bundle carries it. It is not a store: `summaries` rows are, + # and `summaries.current` decides which one the story is entitled to. This + # column has no lineage of its own, so anything that reads it as truth + # inherits whatever was written last, on whatever line — which is exactly + # how abandoned prose reached an active prompt before the correction. + # + # Kept in step by `summaries.record` when one is written and by + # `attempts.restore_state` when the head moves. story_summary: Mapped[str] = mapped_column(Text, default="") # Phase 18: who the player is playing as. The AI never writes these — they # are user-only, which is what lets them sit in the cached system block @@ -198,6 +211,17 @@ class Adventure(Base): cascade="all, delete-orphan", order_by="Memory.id", ) + # M6: the lineage-anchored generated summaries, newest last. + summaries: Mapped[list["Summary"]] = relationship( + back_populates="adventure", + cascade="all, delete-orphan", + order_by="Summary.id", + ) + derived_status: Mapped[list["DerivedStatus"]] = relationship( + back_populates="adventure", + cascade="all, delete-orphan", + order_by="DerivedStatus.id", + ) class Branch(Base): @@ -467,6 +491,14 @@ class Memory(Base): # current. Readers need only the yes-or-no answer, and fetching six # kilobytes of vector to get it is too expensive. embedded: Mapped[bool] = mapped_column(Boolean, default=False) + # M6: how much weight the narrator should give this memory + # (`CONTEXT-AND-MEMORY.md` §14). `accepted_story` is something the story + # actually established; `heuristic` is an interpretation of it. The + # application owns this classification — the extractor may hint, but + # `memorybank.classify_authority` decides — so a guess can never become + # canon merely by being written down. Authoritative state changes still go + # only through the M5 event path (ADR 013). + authority: Mapped[str] = mapped_column(String(20), default="accepted_story") pinned: Mapped[bool] = mapped_column(Boolean, default=False) forgotten: Mapped[bool] = mapped_column(Boolean, default=False) # evicted, kept for UI use_count: Mapped[int] = mapped_column(Integer, default=0) @@ -476,6 +508,77 @@ class Memory(Base): adventure: Mapped[Adventure] = relationship(back_populates="memories") +class Summary(Base): + """M6: one generated rolling summary, anchored to the story it summarizes. + + The inherited design kept the summary in a single `adventures.story_summary` + column with a lineage cursor recording how far it had read. The cursor was + lineage-aware; the prose was not. After an Undo and a divergence the column + still held sentences describing the abandoned line, and the context builder + injected it unconditionally — the leak `STORY-BRANCH-SEMANTICS.md` §32 and + acceptance test E03 forbid. + + A summary is therefore a row on a path, exactly as a `Memory` is, and it is + filtered through the same `lineage.Path.clause` chokepoint. `branch_id` and + `depth` are the coordinate it was written at; `source_start`/`source_end` + are the stretch of story it covers. A summary whose coordinate is not on the + active capped lineage is not eligible, and is never deleted for it — the + abandoned line keeps its own derived data (§11). + """ + + __tablename__ = "summaries" + + id: Mapped[int] = mapped_column(primary_key=True) + adventure_id: Mapped[int] = mapped_column(ForeignKey("adventures.id", ondelete="CASCADE")) + text: Mapped[str] = mapped_column(Text, default="") + # The coordinate this summary was written at: the last node it covers. + branch_id: Mapped[int | None] = mapped_column( + ForeignKey("branches.id", ondelete="CASCADE"), nullable=True + ) + depth: Mapped[int | None] = mapped_column(Integer, nullable=True) + # The stretch of story it summarizes, as depths on `branch_id`. + source_start: Mapped[int | None] = mapped_column(Integer, nullable=True) + source_end: Mapped[int | None] = mapped_column(Integer, nullable=True) + # Why it was generated: "interval" for the automatic pass, "manual" when the + # reader wrote or edited it themselves. + trigger: Mapped[str] = mapped_column(String(20), default="interval") + model_name: Mapped[str] = mapped_column(String(200), default="") + created_at: Mapped[datetime] = mapped_column(DateTime, default=utcnow) + + adventure: Mapped[Adventure] = relationship(back_populates="summaries") + + +class DerivedStatus(Base): + """M6: the outcome of one kind of background derived work, per campaign. + + M2 shipped with the whole memory bank dead and the suite green: the + summariser and the embedder raised inside a fire-and-forget task, and + nothing recorded it (`BUILD-MILESTONES.md`, note from M2). Derived work is + allowed to fail — the accepted turn, the state and the head must all + survive it — but it is not allowed to fail *invisibly*. + + One row per (adventure, kind), rewritten in place. This is deliberately not + a job queue: it records what happened last, so a reader can see that + memories stopped being written and why, and so a maintainer can retry. + """ + + __tablename__ = "derived_status" + __table_args__ = (UniqueConstraint("adventure_id", "kind", name="uq_derived_kind"),) + + id: Mapped[int] = mapped_column(primary_key=True) + adventure_id: Mapped[int] = mapped_column(ForeignKey("adventures.id", ondelete="CASCADE")) + # "memory", "summary" or "embedding". + kind: Mapped[str] = mapped_column(String(20)) + # "ok" (did work), "idle" (ran, nothing pending) or "failed". + status: Mapped[str] = mapped_column(String(20), default="ok") + detail: Mapped[str] = mapped_column(Text, default="") + failures: Mapped[int] = mapped_column(Integer, default=0) + last_attempt_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) + last_success_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True) + + adventure: Mapped[Adventure] = relationship(back_populates="derived_status") + + class StoryCard(Base): """Owned by either a scenario or an adventure (exactly one set).""" diff --git a/backend/app/routers/adventures/crud.py b/backend/app/routers/adventures/crud.py index 1e06ff2..2210015 100644 --- a/backend/app/routers/adventures/crud.py +++ b/backend/app/routers/adventures/crud.py @@ -10,7 +10,8 @@ from sqlalchemy.orm import Session from sqlalchemy.orm.attributes import set_committed_value from ... import ( - attempts, head, images, limits, memorybank, models, schemas, tree, worldstate, + attempts, head, images, limits, memorybank, models, schemas, summaries, tree, + worldstate, ) from ...database import get_db @@ -294,8 +295,18 @@ def update_adventure( db: Session = Depends(get_db), adventure: models.Adventure = Depends(current_adventure), ): - for field, value in payload.model_dump(exclude_unset=True).items(): + fields = payload.model_dump(exclude_unset=True) + for field, value in fields.items(): setattr(adventure, field, value) + # M6: a summary the reader typed is still a summary, so it is anchored to + # the position they typed it at rather than left in a column with no + # lineage. Otherwise a hand-written summary would survive an Undo and a + # divergence that its generated equivalent correctly does not (E03). + if "story_summary" in fields: + typed = (fields["story_summary"] or "").strip() + held = summaries.current(db, adventure) + if typed and (held is None or held.text.strip() != typed): + summaries.record(db, adventure, typed, trigger="manual") db.commit() return adventure diff --git a/backend/app/routers/adventures/insights.py b/backend/app/routers/adventures/insights.py index f86358c..8b5342a 100644 --- a/backend/app/routers/adventures/insights.py +++ b/backend/app/routers/adventures/insights.py @@ -7,8 +7,8 @@ returns the prompt a turn was actually generated from. Neither writes anything. from fastapi import Depends, HTTPException from sqlalchemy.orm import Session -from ... import memorybank, models -from ...context import build_context +from ... import derived, memorybank, models, summaries +from ...context import ContextOverflow, build_context from ...database import get_db from ..settings import get_settings @@ -24,10 +24,55 @@ async def dry_run_context( """Returns what the app would send to the AI if the player continued now.""" settings = get_settings(db, user) memories = await memorybank.retrieve_memories(adventure, settings, update_stats=False) - _, _, report = build_context(adventure, settings, memories) + try: + _, _, report = build_context(adventure, settings, memories) + except ContextOverflow as exc: + # M6: a dry run of a prompt that cannot be built is still an answer, and + # a more useful one than a 500. The reader opened this panel to find out + # what would be sent; "nothing, because the protected context does not + # fit, and here is by how much" is exactly that. + raise HTTPException(422, str(exc)) from exc return report +@router.get("/{adventure_id}/derived") +def derived_status( + db: Session = Depends(get_db), + adventure: models.Adventure = Depends(current_adventure), +): + """M6: whether background memory, summary and embedding work is healthy. + + The surface that makes a dead memory bank findable. M2 shipped with the + whole bank failing inside a fire-and-forget task and nothing anywhere said + so — not the UI, not a log a player would read, not a failing test + (`BUILD-MILESTONES.md`, note from M2). This endpoint is where that now + shows. + """ + # Resolved once, not once per row: which summary the current head is + # entitled to. Asking inside the comprehension would be one query per + # summary, which is the shape M5 spent a finding removing. + eligible = summaries.current(db, adventure) + eligible_id = eligible.id if eligible is not None else None + status = derived.report(db, adventure.id) + return { + "status": status, + "failing": [row["kind"] for row in status if row["status"] == "failed"], + "summaries": [ + { + "id": row.id, + "branch_id": row.branch_id, + "depth": row.depth, + "trigger": row.trigger, + "model": row.model_name, + "eligible": row.id == eligible_id, + "created_at": row.created_at.isoformat() if row.created_at else None, + "preview": row.text[:200], + } + for row in summaries.all_for(db, adventure) + ], + } + + @router.get("/{adventure_id}/actions/{action_id}/context") def action_context( adventure_id: int, diff --git a/backend/app/routers/adventures/turns.py b/backend/app/routers/adventures/turns.py index 37c5042..222a754 100644 --- a/backend/app/routers/adventures/turns.py +++ b/backend/app/routers/adventures/turns.py @@ -16,7 +16,7 @@ from ... import ( attempts, head, limits, memorybank, models, narrative, schemas, tree, worldstate, ) -from ...context import build_context, cursors +from ...context import ContextOverflow, build_context, cursors from ...database import get_db from ...providers import OpenAICompatibleProvider, PromptParts, ProviderError from ...sse import SSE_HEADERS, sse, turn_error @@ -164,9 +164,18 @@ async def _generate_turn( memories = await memorybank.retrieve_memories( adventure, settings, update_stats=True, exclude_action_id=replacing_id ) - system_text, story_text, snapshot = build_context( - adventure, settings, memories, exclude_action_id=replacing_id - ) + try: + system_text, story_text, snapshot = build_context( + adventure, settings, memories, exclude_action_id=replacing_id + ) + except ContextOverflow as exc: + # M6: the protected context does not fit in the configured budget, so + # there is no prompt to send. This is a settings problem the reader can + # fix, and the message says how — reporting it as a failed turn keeps + # the story intact and tells them what to change, where building the + # prompt anyway would return a silently truncated reply. + yield turn_error(str(exc)) + return parts = PromptParts(system=system_text, story=story_text) diff --git a/backend/app/summaries.py b/backend/app/summaries.py new file mode 100644 index 0000000..ccaf320 --- /dev/null +++ b/backend/app/summaries.py @@ -0,0 +1,155 @@ +"""M6: the rolling story summary, anchored to the story it summarizes. + +A summary is compressed derived history. It is never the source of truth — the +retained transcript is (`CONTEXT-AND-MEMORY.md` §9) — and it is never allowed to +describe a story the reader is not on. + +The inherited design kept one `adventures.story_summary` column and a lineage +cursor recording how far the summariser had read. The cursor was lineage-aware; +the prose it produced was not. After an Undo and a divergence the column still +held sentences about the abandoned line, and the context builder injected it +with no eligibility check at all — acceptance test E03, and measured failing +against the M5 baseline before this module existed. + +The fix is not a new lineage system. A summary is a row with a coordinate, the +way a `Memory` already is, and it is filtered through the same +`lineage.Path.clause` chokepoint every other read of the story goes through. So: + + eligible == its coordinate is on the active, head-capped lineage + +which gives the four behaviours the milestone asks for, without a rule of its +own for any of them: + + A -> B -> C -> D, summary covers A..C, head at D eligible + Undo to B not eligible + Redo to D eligible again + diverge from B onto X -> Y not eligible + +Nothing is deleted when a line is abandoned. The abandoned line keeps its own +summaries, and they become eligible again if the reader returns to it. +""" + +from __future__ import annotations + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from . import models +from .context import lineage + + +def record( + db: Session, + adventure: models.Adventure, + text: str, + *, + node: models.Action | None = None, + source_start: int | None = None, + trigger: str = "interval", + model_name: str = "", +) -> models.Summary: + """Stores one summary at the coordinate the story has reached. + + `node` is the last action the summary covers, which is where the row is + anchored. Without one the summary anchors at the head, which is what a + summary the reader typed themselves covers. + """ + branch_id = adventure.head_branch_id + depth = adventure.head_depth + if node is not None and node.depth is not None: + branch_id, depth = node.branch_id, node.depth + row = models.Summary( + adventure_id=adventure.id, + text=text.strip(), + branch_id=branch_id, + depth=depth, + source_start=source_start, + source_end=depth, + trigger=trigger, + model_name=model_name, + ) + db.add(row) + mirror(adventure, row.text) + return row + + +def mirror(adventure: models.Adventure, text: str) -> None: + """Points `adventures.story_summary` at the summary now in force. + + That column is a reader-facing convenience — the Plot panel edits it, the + export bundle carries it — and nothing authoritative may read it. It has no + lineage, so it holds whatever was written last on whatever line, and the M6 + review found the summariser seeding itself from exactly that: after a + divergence it was handed the abandoned line's prose and asked to update it + (finding M6-F1). + + The fix was to seed generation from `current()` instead. This function keeps + the column honest as well, so what a reader sees in the Plot panel and what + an export carries is the summary the narrator is actually being given. + """ + adventure.story_summary = text or "" + + +def refresh_mirror(db: Session, adventure: models.Adventure) -> None: + """Re-points the mirror after the head has moved. + + Called from `attempts.restore_state`, which every Undo, Redo, take switch + and Save Point restore goes through. Without it the column would keep + showing a summary the story has moved away from. + """ + row = current(db, adventure) + mirror(adventure, row.text if row is not None else "") + + +def current(db: Session, adventure: models.Adventure) -> models.Summary | None: + """The newest summary eligible for the position being read, or None. + + Eligibility is the capped lineage clause and nothing else. Ordering by + depth then id takes the newest summary on the path, so a fresher summary + written on a shallower branch does not outrank the deep one it was + superseded by. + """ + return db.execute( + select(models.Summary) + .where( + models.Summary.adventure_id == adventure.id, + lineage.path_of(db, adventure).clause(models.Summary), + ) + .order_by(models.Summary.depth.desc(), models.Summary.id.desc()) + .limit(1) + ).scalars().first() + + +def text_for_prompt(db: Session, adventure: models.Adventure) -> str: + """The summary the narrator should be shown, or an empty string.""" + row = current(db, adventure) + return row.text if row is not None and row.text.strip() else "" + + +def provenance(row: models.Summary | None) -> dict | None: + """What the inspector shows about where a summary came from.""" + if row is None: + return None + return { + "id": row.id, + "branch_id": row.branch_id, + "depth": row.depth, + "source_start": row.source_start, + "source_end": row.source_end, + "trigger": row.trigger, + "model": row.model_name, + "created_at": row.created_at.isoformat() if row.created_at else None, + } + + +def all_for(db: Session, adventure: models.Adventure) -> list[models.Summary]: + """Every stored summary, eligible or not, newest first. + + Abandoned summaries are retained rather than deleted, so this is how a + reader or a maintainer sees that they still exist. + """ + return list(db.execute( + select(models.Summary) + .where(models.Summary.adventure_id == adventure.id) + .order_by(models.Summary.id.desc()) + ).scalars().all()) diff --git a/backend/tests/test_context_memory.py b/backend/tests/test_context_memory.py new file mode 100644 index 0000000..046c53e --- /dev/null +++ b/backend/tests/test_context_memory.py @@ -0,0 +1,837 @@ +"""M6: branch-safe context, summaries and long-term story memory. + +The acceptance contract for this milestone is F01-F08 plus the E-series lineage +tests that own the memory and summary consequences of branching. Each test below +names the criterion it carries. + +Two things are asserted throughout rather than assumed: + +* **The assembled prompt, not the narration.** A model that fails to mention a + leaked memory is not evidence that the memory did not leak, so every leak test + reads the context the builder actually produced. +* **The lineage chokepoint, not a reimplementation.** Memories and summaries are + filtered by `lineage.Path.clause`, the same clause every read of the story + goes through. A test that walked the tree itself could pass while the product + leaked. + + python -m pytest tests/test_context_memory.py -v +""" + +import asyncio + +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient +from sqlalchemy import select + +from app import auth, derived, limits, memorybank, models, summaries +from app.context import builder, lineage +from app.database import Base, SessionLocal, engine, get_db +from app.main import app +from app.providers import ProviderError +from app.routers import adventures + +from fakes import ScriptedProvider, state_block + + +class StubEmbedder: + """A deterministic embedder. Distinct texts get distinguishable vectors.""" + + def __init__(self): + self.calls = 0 + + async def embed(self, texts): + self.calls += 1 + out = [] + for text in texts: + lowered = text.lower() + out.append([ + 1.0, + 1.0 if "ledger" in lowered or "flagstone" in lowered else 0.0, + 1.0 if "chapel" in lowered else 0.0, + ]) + return out + + +class StubSummariser: + """Stands in for the summariser so this file opens no sockets.""" + + async def complete(self, system, user, *, max_tokens=600): + return "A summary of what has happened so far." + + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + memorybank._vector_cache.clear() + setup = SessionLocal() + user = models.User(is_guest=False, email="m6@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings( + user_id=user.id, api_key="enc:dummy", model="test-model", + embedding_model="embed-test", context_token_budget=4000, + max_output_tokens=400, memory_top_k=3, + )) + adventure = models.Adventure( + user_id=user.id, title="M6", memory_bank_enabled=True, auto_summarize=True, + ) + setup.add(adventure) + setup.flush() + setup.add(models.Action(adventure_id=adventure.id, type="start", + text="The road forks at the Crooked Lantern.")) + setup.commit() + adv_id, user_id = adventure.id, user.id + setup.close() + + monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None) + monkeypatch.setattr(adventures.turns, "OpenAICompatibleProvider", ScriptedProvider) + # Both derived providers are stubbed, not just the embedder (M6 review + # finding M6-F3). With only the embedder replaced, the post-turn pass built + # a real summariser against the default endpoint and every turn in this file + # opened a socket to localhost:11434 — slow, dependent on what happens to be + # listening, and the source of an abandoned-coroutine RuntimeWarning when + # the TestClient event loop closed under it. Tests that deliberately + # exercise real provider construction live in `test_provider_wiring.py`. + monkeypatch.setattr(memorybank, "embedding_provider", lambda s: StubEmbedder()) + monkeypatch.setattr(memorybank, "summary_provider", lambda s: StubSummariser()) + app.dependency_overrides[auth.get_current_user] = ( + lambda db=Depends(get_db): db.get(models.User, user_id) + ) + test_client = TestClient(app) + test_client.adv_id = adv_id + test_client.user_id = user_id + try: + yield test_client + finally: + app.dependency_overrides.clear() + memorybank._vector_cache.clear() + Base.metadata.drop_all(bind=engine) + + +# ----------------------------------------------------------------- helpers + +def play(client, text, prose="The road bends onward past the treeline.", events=None): + ScriptedProvider.replies = [f"{prose}\n{state_block(events or [])}"] + r = client.post(f"/api/adventures/{client.adv_id}/actions", + json={"type": "do", "text": text}) + assert r.status_code == 200, r.text[:300] + assert '"error"' not in r.text, r.text[:300] + + +def head_of(client): + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + return adventure.head_branch_id, adventure.head_depth + + +def context_report(client) -> dict: + """The prompt the app would send now, assembled through the real builder.""" + r = client.get(f"/api/adventures/{client.adv_id}/context") + assert r.status_code == 200, r.text[:300] + return r.json() + + +def prompt_text(report: dict) -> str: + return "\n".join(s["text"] for s in report["sections"]) + + +def plant_memory(client, text, *, authority=None, at_depth=None): + """Attaches an embedded memory to a live node, as the real pass would.""" + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + depth = adventure.head_depth if at_depth is None else at_depth + node = db.execute( + select(models.Action).where( + models.Action.adventure_id == client.adv_id, + lineage.path_of(db, adventure).uncapped().clause(models.Action), + models.Action.depth == depth, + ) + ).scalars().first() + assert node is not None, f"no live node at depth {depth}" + memory = models.Memory( + adventure_id=client.adv_id, text=text, + branch_id=node.branch_id, depth=node.depth, + source_start=node.depth, source_end=node.depth, + authority=authority or memorybank.classify_authority(text), + ) + memorybank.set_vector(memory, asyncio.run(StubEmbedder().embed([text]))[0]) + db.add(memory) + db.commit() + return memory.id + + +def eligible_memory_texts(client) -> list[str]: + """What the retrieval filter would consider, through the real clause.""" + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + return list(db.execute( + select(models.Memory.text).where( + models.Memory.adventure_id == client.adv_id, + lineage.path_of(db, adventure).clause(models.Memory), + models.Memory.forgotten.is_(False), + ) + ).scalars().all()) + + +# --------------------------------------------------------------------- F01 + +def test_f01_recent_turns_stay_in_the_prompt(client): + """F01. The immediately preceding turns are what conversational coherence + is made of, so they have to actually be there.""" + play(client, "ask Mara about the key", prose="Mara turns the silver key over.") + play(client, "wait for her answer", prose="'I found it at the chapel,' she says.") + + story = prompt_text(context_report(client)) + + assert "Mara turns the silver key over." in story + assert "'I found it at the chapel,' she says." in story + assert "ask Mara about the key" in story + + +# --------------------------------------------------------------------- F02 + +def test_f02_an_old_clue_survives_outside_recent_history(client): + """F02. A distinctive clue is planted, the story runs on past it, and the + clue comes back through memory rather than through the whole transcript.""" + play(client, "search the floor", + prose="Aldric pries up the third flagstone and hides the ledger beneath it.") + plant_memory(client, "Aldric hid the ledger beneath the third flagstone.") + for i in range(22): + play(client, f"walk on {i}", prose=f"[{i}] " + "The road runs on. " * 60) + + report = context_report(client) + story = prompt_text(report) + + # It has fallen out of the verbatim history. + history_text = "\n".join( + s["text"] for s in report["sections"] + if s["label"] in ("history", "recent_history") + ) + assert "third flagstone" not in history_text, ( + "the fixture did not push the clue out of recent history" + ) + # But it is still available to the narrator, through memory. + assert "third flagstone" in story + assert any("flagstone" in m["text"] for m in report["memories"]["used"]) + # And not by sending the whole story. + assert report["history"]["included"] < report["history"]["total"] + + +# --------------------------------------------------------------------- F03 + +def test_f03_the_prompt_stays_bounded_as_the_story_grows(client): + """F03. Input must not grow with the transcript.""" + play(client, "begin", prose="The road bends. " * 40) + for i in range(6): + play(client, f"on {i}", prose=f"[{i}] " + "The road bends. " * 40) + short = context_report(client) + for i in range(24): + play(client, f"further {i}", prose=f"[{i}] " + "The road bends. " * 40) + long = context_report(client) + + assert long["history"]["total"] > short["history"]["total"] * 2, "fixture too small" + budget = long["tokens"]["budget"] + assert long["tokens"]["total"] <= budget + # Four times the story must not be four times the prompt. + assert long["tokens"]["total"] < short["tokens"]["total"] * 2 + + +# --------------------------------------------------------------------- F04 + +def test_f04_the_reply_budget_is_reserved(client): + """F04. The configured reply length stays available whatever the story.""" + for i in range(20): + play(client, f"on {i}", prose=f"[{i}] " + "The road bends. " * 40) + + report = context_report(client) + with SessionLocal() as db: + settings = db.query(models.Settings).filter_by(user_id=client.user_id).first() + max_output = settings.max_output_tokens + + assert report["tokens"]["output_reserve"] >= max_output + assert report["tokens"]["total"] + max_output <= report["tokens"]["budget"], ( + "the assembled input left no room for the reply" + ) + + +def test_f04_a_budget_too_small_for_the_reply_is_refused(client): + """Section 9: fail clearly rather than build a prompt known to overflow.""" + play(client, "begin") + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + settings = db.query(models.Settings).filter_by(user_id=client.user_id).first() + settings.context_token_budget = 200 + settings.max_output_tokens = 4000 + db.commit() + with pytest.raises(builder.ContextOverflow) as exc: + builder.build_context(adventure, settings) + # The message has to say what to change. + assert "context budget" in str(exc.value) + assert "reserved for the reply" in str(exc.value) + + +def test_f04_an_impossible_budget_fails_the_turn_without_losing_the_story(client): + """The refusal reaches the reader as a failed turn, not a 500.""" + play(client, "begin", prose="The lantern swings.") + with SessionLocal() as db: + settings = db.query(models.Settings).filter_by(user_id=client.user_id).first() + settings.context_token_budget = 200 + settings.max_output_tokens = 4000 + db.commit() + + ScriptedProvider.replies = ["should never be reached"] + r = client.post(f"/api/adventures/{client.adv_id}/actions", + json={"type": "do", "text": "carry on"}) + assert r.status_code == 200 + assert "context budget" in r.text + # The story that already existed is untouched. + actions = client.get(f"/api/adventures/{client.adv_id}").json()["actions"] + assert any("The lantern swings." in a["text"] for a in actions) + + +# --------------------------------------------------------------------- F05 + +def test_f05_the_inspector_shows_every_component_m6_owns(client): + """F05, for the components this milestone owns.""" + play(client, "begin", prose="Aldric sets the key down.", + events=[{"type": "create_entity", "entity": "aldric", + "entity_type": "character", "name": "Aldric"}]) + plant_memory(client, "Aldric hid the ledger beneath the third flagstone.") + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + summaries.record(db, adventure, "The party reached the Crooked Lantern.") + db.commit() + play(client, "carry on") + + report = context_report(client) + labels = {s["label"] for s in report["sections"]} + + assert "narrator" in labels, "narrator/system rules" + assert "narrative_state" in labels, "current authoritative state" + assert "story_summary" in labels, "the summary used" + assert "used_memories" in labels, "retrieved memories" + assert "history" in labels, "recent history" + # Model and settings. + assert report["settings"]["model"] == "test-model" + assert report["settings"]["max_output_tokens"] == 400 + # Token accounting, per component and in total. + assert all(isinstance(s["tokens"], int) for s in report["sections"]) + for key in ("total", "budget", "output_reserve", "protected", "available_for_history"): + assert key in report["tokens"], key + # Summary provenance. + assert report["summary"]["depth"] is not None + # Derived-work health. + assert isinstance(report["derived"], list) + + +# --------------------------------------------------------------------- F06 + +def test_f06_a_retrieved_memory_is_traceable_to_its_source(client): + """F06. "Where did this memory come from?" must be answerable.""" + play(client, "search the floor", prose="Aldric hides the ledger.") + memory_id = plant_memory(client, "Aldric hid the ledger beneath the third flagstone.") + play(client, "carry on") + + used = context_report(client)["memories"]["used"] + entry = next(m for m in used if m["id"] == memory_id) + + assert entry["source"]["branch_id"] is not None + assert entry["source"]["depth"] is not None + assert entry["source"]["source_start"] is not None + # And the coordinate names a real node of this campaign's accepted history. + with SessionLocal() as db: + node = db.execute( + select(models.Action).where( + models.Action.adventure_id == client.adv_id, + models.Action.branch_id == entry["source"]["branch_id"], + models.Action.depth == entry["source"]["depth"], + ) + ).scalars().first() + assert node is not None, "the memory's provenance points at no action" + + +# --------------------------------------------------------------------- F07 + +def test_f07_a_heuristic_memory_is_labelled_and_is_not_state(client): + """F07. An inference may be recalled; it may not become canon.""" + play(client, "watch her", prose="Mara glances at the door.", + events=[{"type": "create_entity", "entity": "mara", + "entity_type": "character", "name": "Mara"}]) + plant_memory(client, "Mara seemed nervous around Captain Vale.") + play(client, "carry on") + + report = context_report(client) + used = report["memories"]["used"] + entry = next(m for m in used if "Captain Vale" in m["text"]) + assert entry["authority"] == "heuristic" + + story = prompt_text(report) + assert "[inferred]" in story, "the prompt does not mark the inference" + assert "interpretation, not established fact" in story + + # And it did not become authoritative state. + document = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + facts = [f["predicate"] for f in document["facts"]] + assert not any("Vale" in f for f in facts), "a heuristic memory became a fact" + + +def test_the_application_classifies_authority_not_the_model(client): + """The classifier is the application's, and it is inspectable.""" + assert memorybank.classify_authority( + "Aldric promised Mara he would return before dawn.") == "accepted_story" + assert memorybank.classify_authority( + "Mara seemed uneasy when Captain Vale was mentioned.") == "heuristic" + + +# --------------------------------------------------------------------- F08 + +def test_f08_a_failing_memory_pass_keeps_the_story_and_is_visible(client): + """F08. Derived work fails softly, and audibly.""" + play(client, "begin", prose="The lantern swings.", + events=[{"type": "create_entity", "entity": "aldric", + "entity_type": "character", "name": "Aldric"}]) + before_state = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + + class Broken: + async def complete(self, *a, **k): + raise ProviderError("the summariser is unreachable") + + async def embed(self, texts): + raise ProviderError("the embedder is unreachable") + + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + # Enough uncovered story that the memory pass is genuinely due. + for depth in range(20): + db.add(models.Action(adventure_id=adventure.id, type="do", + text=f"filler {depth}")) + db.commit() + # Read the head *after* the fixture's own writes, so what this test measures + # is the effect of the failing derived pass and nothing else. + before_head = head_of(client) + + import app.memorybank as mb + real_summary, real_embed = mb.summary_provider, mb.embedding_provider + mb.summary_provider = lambda s: Broken() + mb.embedding_provider = lambda s: Broken() + try: + asyncio.run(mb.run_post_turn(client.adv_id)) + finally: + mb.summary_provider, mb.embedding_provider = real_summary, real_embed + + # The accepted story, its state and the head all survived. + actions = client.get(f"/api/adventures/{client.adv_id}").json()["actions"] + assert any("The lantern swings." in a["text"] for a in actions) + after_state = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + assert after_state["entities"].keys() == before_state["entities"].keys() + assert head_of(client) == before_head + + # The failure is findable. + status = client.get(f"/api/adventures/{client.adv_id}/derived").json() + assert "memory" in status["failing"], status + detail = next(r for r in status["status"] if r["kind"] == "memory") + assert "unreachable" in detail["detail"] + assert detail["failures"] >= 1 + + # And the story continues. + play(client, "carry on", prose="The door opens.") + assert any("The door opens." in a["text"] + for a in client.get(f"/api/adventures/{client.adv_id}").json()["actions"]) + + +def test_f08_a_recovered_pass_clears_the_failure(client): + """Derived work can be retried: the next healthy run clears the record.""" + with SessionLocal() as db: + derived.failed(db, client.adv_id, derived.SUMMARY, + ProviderError("the summariser is unreachable")) + db.commit() + assert client.get(f"/api/adventures/{client.adv_id}/derived").json()["failing"] \ + == ["summary"] + + with SessionLocal() as db: + derived.succeeded(db, client.adv_id, derived.SUMMARY) + db.commit() + + status = client.get(f"/api/adventures/{client.adv_id}/derived").json() + assert status["failing"] == [] + row = next(r for r in status["status"] if r["kind"] == "summary") + assert row["status"] == "ok" and row["failures"] == 0 + + +# ------------------------------------------------------- E02 / E03 lineage +# +# The memory half of this was already correct at the M5 baseline: memories carry +# a `(branch_id, depth)` coordinate and retrieval filters them through the +# capped lineage. These tests pin that behaviour so a later change cannot lose +# it. The summary half was not: before M6 the rolling summary was one column +# with no coordinate, and it leaked across a divergence. That is what +# `app/summaries.py` fixes, and what E03 below measures. + +SECRET_A = "Aldric hid the ledger beneath the third flagstone." +SECRET_B = "The party swore an oath in the drowned chapel." + + +def test_e02_the_ten_step_memory_negative_control(client): + """E02, exactly as the milestone brief numbers it.""" + # 1-2. Establish the fact and let a memory be made from it. + play(client, "search the floor", prose="Aldric pries up the flagstone.") + plant_memory(client, SECRET_A) + + # 3. Retrievable on that valid line. + assert SECRET_A in eligible_memory_texts(client) + assert SECRET_A in prompt_text(context_report(client)) + + # 4-5. Undo to before it: no longer eligible. + client.post(f"/api/adventures/{client.adv_id}/undo") + client.post(f"/api/adventures/{client.adv_id}/undo") + assert SECRET_A not in eligible_memory_texts(client) + assert SECRET_A not in prompt_text(context_report(client)) + + # 6-7. Redo: eligible again, and no re-embedding was needed. + client.post(f"/api/adventures/{client.adv_id}/redo") + client.post(f"/api/adventures/{client.adv_id}/redo") + assert SECRET_A in eligible_memory_texts(client) + with SessionLocal() as db: + assert db.execute( + select(models.Memory.embedded).where( + models.Memory.adventure_id == client.adv_id) + ).scalars().first() is True, "the memory was re-embedded rather than reused" + + # 8-9. Undo again and diverge onto a new continuation. + client.post(f"/api/adventures/{client.adv_id}/undo") + client.post(f"/api/adventures/{client.adv_id}/undo") + play(client, "take the other road", prose="A different road opens.") + + # 10. Still stored, never in the active prompt. + with SessionLocal() as db: + assert db.query(models.Memory).filter_by(adventure_id=client.adv_id).count() == 1 + assert SECRET_A not in eligible_memory_texts(client) + assert SECRET_A not in prompt_text(context_report(client)) + + +def test_e02_the_same_control_through_a_save_point_restore(client): + """E02 again, reached by restoring a Save Point rather than by Undo.""" + play(client, "begin", prose="The lantern swings.") + r = client.post(f"/api/adventures/{client.adv_id}/checkpoints", + json={"name": "Before the ledger"}) + assert r.status_code in (200, 201), r.text[:200] + save_point = r.json() + + play(client, "search the floor", prose="Aldric pries up the flagstone.") + plant_memory(client, SECRET_A) + assert SECRET_A in prompt_text(context_report(client)) + + r = client.post( + f"/api/adventures/{client.adv_id}/checkpoints/{save_point['id']}/restore") + assert r.status_code == 200, r.text[:200] + + assert SECRET_A not in eligible_memory_texts(client) + assert SECRET_A not in prompt_text(context_report(client)) + + # Diverging from the restored position keeps it out for good. + play(client, "a different road", prose="A different road opens.") + assert SECRET_A not in prompt_text(context_report(client)) + with SessionLocal() as db: + assert db.query(models.Memory).filter_by(adventure_id=client.adv_id).count() == 1 + + +def test_e03_an_abandoned_summary_is_retained_but_never_used(client): + """E03. The failure this milestone fixes, measured in the prompt. + + Before M6 the summary was a single column with a lineage cursor but no + lineage of its own, and the builder injected it unconditionally. Undo plus a + divergence therefore left the narrator reading sentences about a story the + reader was no longer on. + """ + play(client, "begin", prose="The lantern swings.") + play(client, "go to the chapel", prose="The chapel door gives.") + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + summaries.record(db, adventure, SECRET_B, trigger="interval", + model_name="test-model") + db.commit() + + # Eligible on the line that produced it. + assert SECRET_B in prompt_text(context_report(client)) + assert context_report(client)["summary"]["trigger"] == "interval" + + # Undo before the summarized stretch, then diverge. + client.post(f"/api/adventures/{client.adv_id}/undo") + client.post(f"/api/adventures/{client.adv_id}/undo") + play(client, "take the other road", prose="A different road opens.") + + report = context_report(client) + assert SECRET_B not in prompt_text(report), "an abandoned summary reached the prompt" + assert report["summary"] is None or SECRET_B not in report["summary"].get("preview", "") + + # Retained, not deleted — and visible as retained. + status = client.get(f"/api/adventures/{client.adv_id}/derived").json() + stored = [row for row in status["summaries"] if SECRET_B in row["preview"]] + assert stored, "the abandoned summary was deleted rather than retained" + assert stored[0]["eligible"] is False + + +def test_e03_a_summary_becomes_eligible_again_on_redo(client): + """The negative control needs its positive half: Redo restores the line, so + the summary written on it is usable again.""" + play(client, "begin", prose="The lantern swings.") + play(client, "go to the chapel", prose="The chapel door gives.") + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + summaries.record(db, adventure, SECRET_B) + db.commit() + assert SECRET_B in prompt_text(context_report(client)) + + client.post(f"/api/adventures/{client.adv_id}/undo") + client.post(f"/api/adventures/{client.adv_id}/undo") + assert SECRET_B not in prompt_text(context_report(client)) + + client.post(f"/api/adventures/{client.adv_id}/redo") + client.post(f"/api/adventures/{client.adv_id}/redo") + assert SECRET_B in prompt_text(context_report(client)) + + +def test_a_summary_the_reader_typed_is_anchored_too(client): + """A hand-written summary is still a summary. It would otherwise survive a + divergence that its generated equivalent correctly does not.""" + play(client, "begin", prose="The lantern swings.") + play(client, "go to the chapel", prose="The chapel door gives.") + r = client.patch(f"/api/adventures/{client.adv_id}", + json={"story_summary": SECRET_B}) + assert r.status_code == 200, r.text[:200] + assert SECRET_B in prompt_text(context_report(client)) + + client.post(f"/api/adventures/{client.adv_id}/undo") + client.post(f"/api/adventures/{client.adv_id}/undo") + play(client, "the other road", prose="A different road opens.") + + assert SECRET_B not in prompt_text(context_report(client)) + + +def test_e01_and_e04_state_and_scene_are_unchanged_by_m6(client): + """M5's lineage behaviour must not regress while context selection changes.""" + play(client, "establish", prose="Mara arrives.", events=[ + {"type": "create_entity", "entity": "mara", "entity_type": "character", + "name": "Mara"}]) + play(client, "she learns", prose="Mara learns the code.", events=[ + {"type": "add_fact", "subject": "mara", "predicate": "knows the vault code", + "fact_id": "vault"}]) + document = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + assert "knows the vault code" in [f["predicate"] for f in document["facts"]] + + client.post(f"/api/adventures/{client.adv_id}/undo") + client.post(f"/api/adventures/{client.adv_id}/undo") + play(client, "a different road", prose="A different road opens.") + + document = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + assert "knows the vault code" not in [f["predicate"] for f in document["facts"]] + assert "vault code" not in prompt_text(context_report(client)) + + +# ------------------------------------------- authority conflicts (section 8) + +def test_a_memory_cannot_outrank_a_manual_correction(client): + """Section 8. A withdrawn assertion may survive as history; it may not be + presented as current truth, whatever a memory says about it.""" + play(client, "establish", prose="Mara arrives.", events=[ + {"type": "create_entity", "entity": "mara", "entity_type": "character", + "name": "Mara"}]) + play(client, "she learns", prose="Mara learns where the key was found.", events=[ + {"type": "add_fact", "subject": "mara", + "predicate": "knows where the key was found", "fact_id": "mara-knows"}]) + # A memory that records the same thing, written before the correction. + plant_memory(client, "Mara knows where the key was found.") + + r = client.post(f"/api/adventures/{client.adv_id}/state/corrections", json={ + "events": [{"type": "invalidate_fact", "fact_id": "mara-knows", + "reason": "Mara never learned where the silver key was found."}], + "note": "Mara never learned where the silver key was found.", + }) + assert r.status_code in (200, 201), r.text[:200] + + report = context_report(client) + sections = {s["label"]: s["text"] for s in report["sections"]} + + # The authoritative state says it is withdrawn, in the prompt itself. + assert "No longer true" in sections["narrative_state"] + assert "Mara never learned" in sections["narrative_state"] + # The state section does not carry it among the facts that stand. + established = sections["narrative_state"].split("No longer true")[0] + assert "knows where the key was found" not in established + # The memory is subordinate: it is not state, and it is not canon. + document = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + active = [f["predicate"] for f in document["facts"] + if f.get("status") != "invalidated"] + assert "knows where the key was found" not in active + + +# ---------------------------------------------------- derived rebuildability + +def test_derived_data_can_be_deleted_and_rebuilt(client): + """Section 16. Authoritative history must not depend on derived rows.""" + play(client, "begin", prose="The lantern swings.", events=[ + {"type": "create_entity", "entity": "aldric", "entity_type": "character", + "name": "Aldric"}]) + plant_memory(client, SECRET_A) + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + summaries.record(db, adventure, SECRET_B) + db.commit() + + before_actions = [a["text"] for a in + client.get(f"/api/adventures/{client.adv_id}").json()["actions"]] + before_state = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + before_head = head_of(client) + + # Remove every derived row. + with SessionLocal() as db: + db.query(models.Memory).filter_by(adventure_id=client.adv_id).delete() + db.query(models.Summary).filter_by(adventure_id=client.adv_id).delete() + db.commit() + + after_actions = [a["text"] for a in + client.get(f"/api/adventures/{client.adv_id}").json()["actions"]] + after_state = client.get(f"/api/adventures/{client.adv_id}/state").json()["document"] + assert after_actions == before_actions, "deleting derived data changed the transcript" + assert after_state == before_state, "deleting derived data changed the state" + assert head_of(client) == before_head + # The story still plays with no derived data at all. + play(client, "carry on", prose="The door opens.") + + # And derived data can be written again. + plant_memory(client, SECRET_A) + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + summaries.record(db, adventure, SECRET_B) + db.commit() + assert SECRET_A in prompt_text(context_report(client)) + assert SECRET_B in prompt_text(context_report(client)) + + +# ------------------------------------------- E03 regenerated after divergence +# +# M6 review finding M6-F1. The original E03 test proved only that the *old* +# summary row becomes ineligible after a divergence, and passed while the defect +# was live: the summariser seeded itself from `adventures.story_summary`, a +# campaign-global mirror with no lineage, so the summary it generated on the new +# line inherited the abandoned line's prose. The row was correctly anchored; its +# contents were not. +# +# The regression below plays far enough on the new line to force a *new* summary +# to be generated, which is the step that was missing. + +E03_SENTINEL = "ABANDONED-CHAPEL-OATH-9930" + + +class CarryingSummariser: + """A summariser that behaves like a real one. + + It carries the summary it was given forward and folds in the new events, so + "did abandoned content reach this summary?" has an exact answer. The + per-block memory prompt is answered separately, echoing the sentinel only + for blocks that genuinely contain it. + """ + + def __init__(self): + self.summary_seeds = [] + + async def complete(self, system, user, *, max_tokens=600): + if "Current story summary:" not in user: + return f"MEM[{E03_SENTINEL}]" if E03_SENTINEL in user else "MEM[dry road]" + current = user.split("Current story summary:\n", 1)[1].split("\n\nNew events")[0] + events = user.split("New events since the last update:\n", 1)[1].split( + "\n\nUpdated summary:")[0] + self.summary_seeds.append(current.strip()) + carried = "" if current.strip() == "(none yet)" else current.strip() + " " + return (carried + events.strip().replace("\n", " "))[:1500] + + async def embed(self, texts): + return [[1.0, 0.0, 0.0] for _ in texts] + + +def test_e03_a_summary_generated_after_divergence_carries_no_abandoned_content(client): + """M6-F1. The failure the original E03 test could not see. + + Every step of the review's reproduction, in order, with the positive control + first — a summary that does not exist proves nothing about what it omits. + """ + summariser = CarryingSummariser() + import app.memorybank as mb + real_summary, real_embed = mb.summary_provider, mb.embedding_provider + mb.summary_provider = lambda s: summariser + mb.embedding_provider = lambda s: summariser + try: + # 1-2. Path A, long enough to generate a summary, with the sentinel on it. + for i in range(20): + play(client, f"a{i}", prose=f"They swear the {E03_SENTINEL}. [{i}]") + asyncio.run(mb.run_post_turn(client.adv_id)) + + # 3. POSITIVE CONTROL: the sentinel really is in the path-A summary. + report_a = context_report(client) + summary_a = next((s["text"] for s in report_a["sections"] + if s["label"] == "story_summary"), "") + assert summary_a, "no summary was generated on path A; the rest proves nothing" + assert E03_SENTINEL in summary_a, "the fixture did not put the sentinel in the summary" + assert E03_SENTINEL in prompt_text(report_a) + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + path_a_summary_id = summaries.current(db, adventure).id + + # 4. Move the head below every turn that mentions the sentinel. + while head_of(client)[1] > 0: + if client.post(f"/api/adventures/{client.adv_id}/undo").status_code != 200: + break + + # 5-6. Diverge, and play far enough that a NEW summary is generated. + # Seeds recorded from here on are the ones that matter: on path A the + # summariser is *supposed* to be seeded with the sentinel, because the + # sentinel is on path A. + summariser.summary_seeds.clear() + for i in range(20): + play(client, f"b{i}", prose=f"A dry road, nothing sworn. [{i}]") + asyncio.run(mb.run_post_turn(client.adv_id)) + + report_b = context_report(client) + summary_b_row = report_b["summary"] + summary_b = next((s["text"] for s in report_b["sections"] + if s["label"] == "story_summary"), "") + + # 7. A new summary really was generated on the new line. + assert summary_b_row is not None, "no summary is eligible on path B" + assert summary_b_row["id"] != path_a_summary_id, ( + "path B reused path A's summary row rather than generating one" + ) + + # 8. No path-A story is on path B's lineage, so anything from it is a leak. + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + carried_over = db.query(models.Action).filter( + models.Action.adventure_id == client.adv_id, + lineage.path_of(db, adventure).clause(models.Action), + models.Action.text.like(f"%{E03_SENTINEL}%"), + ).count() + assert carried_over == 0, "the fixture left path-A story on path B's lineage" + + # 9-10. The sentinel is in neither the new summary nor the whole prompt. + assert E03_SENTINEL not in summary_b, ( + "the summary generated on path B carries the abandoned line's content" + ) + assert E03_SENTINEL not in prompt_text(report_b), ( + "abandoned content reached the active narrator prompt" + ) + + # And it was never even *offered* the abandoned prose: the fix is at the + # input, not a filter over the output. + assert summariser.summary_seeds, "no summary was generated on path B" + assert not any(E03_SENTINEL in seed for seed in summariser.summary_seeds), ( + "the summariser was seeded with content from the abandoned line" + ) + + # 11. The old summary is retained, and reported as retained-but-ineligible. + listing = client.get(f"/api/adventures/{client.adv_id}/derived").json() + old = [row for row in listing["summaries"] if row["id"] == path_a_summary_id] + assert old, "the abandoned summary row was deleted rather than retained" + assert old[0]["eligible"] is False + finally: + mb.summary_provider, mb.embedding_provider = real_summary, real_embed diff --git a/backend/tests/test_context_performance.py b/backend/tests/test_context_performance.py new file mode 100644 index 0000000..24530a1 --- /dev/null +++ b/backend/tests/test_context_performance.py @@ -0,0 +1,202 @@ +"""M6: the read paths this milestone touches must not grow a query per row. + +M5 spent a review finding on an N+1 in the action list. M6 adds three things +that could each reintroduce one — a memory's provenance, a summary's source +coordinates, and the derived-work status — so each is measured here rather than +argued about. + +The assertions are on *growth*, not on an exact count. A fixed number would +break on any unrelated query and teach the next person to raise the number; what +matters is that doubling the rows does not double the queries. + + python -m pytest tests/test_context_performance.py -v +""" + +import asyncio + +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient +from sqlalchemy import event + +from app import auth, limits, memorybank, models, summaries +from app.database import Base, SessionLocal, engine, get_db +from app.main import app +from app.routers import adventures + +from fakes import ScriptedProvider, state_block + + +class StubEmbedder: + async def embed(self, texts): + return [[1.0, 0.0, 0.0] for _ in texts] + + +@pytest.fixture() +def sql_log(): + statements: list[str] = [] + + def record(conn, cursor, statement, parameters, context, executemany): + statements.append(statement) + + event.listen(engine, "before_cursor_execute", record) + try: + yield statements + finally: + event.remove(engine, "before_cursor_execute", record) + + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + memorybank._vector_cache.clear() + setup = SessionLocal() + user = models.User(is_guest=False, email="perf@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings( + user_id=user.id, api_key="enc:dummy", model="test-model", + embedding_model="embed-test", context_token_budget=8000, + max_output_tokens=400, memory_top_k=5, + )) + adventure = models.Adventure( + user_id=user.id, title="Perf", memory_bank_enabled=True, auto_summarize=True, + ) + setup.add(adventure) + setup.flush() + setup.add(models.Action(adventure_id=adventure.id, type="start", text="A road.")) + setup.commit() + adv_id, user_id = adventure.id, user.id + setup.close() + + monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None) + monkeypatch.setattr(adventures.turns, "OpenAICompatibleProvider", ScriptedProvider) + monkeypatch.setattr(memorybank, "embedding_provider", lambda s: StubEmbedder()) + app.dependency_overrides[auth.get_current_user] = ( + lambda db=Depends(get_db): db.get(models.User, user_id) + ) + test_client = TestClient(app) + test_client.adv_id = adv_id + test_client.user_id = user_id + try: + yield test_client + finally: + app.dependency_overrides.clear() + memorybank._vector_cache.clear() + Base.metadata.drop_all(bind=engine) + + +def _grow(client, *, turns, memories, summary_rows): + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + for i in range(turns): + db.add(models.Action(adventure_id=adventure.id, + type="ai" if i % 2 else "do", + text=f"[{i}] The road bends onward. " * 6)) + db.commit() + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + for i in range(memories): + memory = models.Memory( + adventure_id=adventure.id, text=f"Memory {i}: something happened.", + branch_id=adventure.head_branch_id, depth=adventure.head_depth, + source_start=0, source_end=adventure.head_depth, + ) + memorybank.set_vector(memory, [1.0, 0.0, 0.0]) + db.add(memory) + for i in range(summary_rows): + summaries.record(db, adventure, f"Summary {i}.") + db.commit() + + +def _count(sql_log, client) -> int: + sql_log.clear() + r = client.get(f"/api/adventures/{client.adv_id}/context") + assert r.status_code == 200, r.text[:200] + return len(sql_log) + + +def test_assembling_context_does_not_cost_a_query_per_memory(client, sql_log): + """A memory's provenance is fetched in the same read as its text, so more + memories must not mean more queries.""" + _grow(client, turns=10, memories=5, summary_rows=1) + small = _count(sql_log, client) + _grow(client, turns=0, memories=25, summary_rows=0) + large = _count(sql_log, client) + + assert large <= small + 2, ( + f"{small} queries with 5 memories, {large} with 30 — " + "the context read is paying per memory" + ) + + +def test_assembling_context_does_not_cost_a_query_per_summary(client, sql_log): + """Only the eligible summary is read, however many are retained.""" + _grow(client, turns=10, memories=2, summary_rows=2) + small = _count(sql_log, client) + _grow(client, turns=0, memories=0, summary_rows=30) + large = _count(sql_log, client) + + assert large <= small + 2, ( + f"{small} queries with 2 summaries, {large} with 32 — " + "the context read is paying per summary" + ) + + +def test_assembling_context_does_not_cost_a_query_per_turn(client, sql_log): + """The history window is one read, not one per action.""" + _grow(client, turns=10, memories=2, summary_rows=1) + small = _count(sql_log, client) + _grow(client, turns=60, memories=0, summary_rows=0) + large = _count(sql_log, client) + + assert large <= small + 2, ( + f"{small} queries at 10 turns, {large} at 70 — " + "the context read is paying per turn" + ) + + +def test_the_derived_status_endpoint_does_not_pay_per_summary(client, sql_log): + """The listing resolves the eligible summary once, not once per row.""" + _grow(client, turns=6, memories=1, summary_rows=3) + sql_log.clear() + assert client.get(f"/api/adventures/{client.adv_id}/derived").status_code == 200 + small = len(sql_log) + + _grow(client, turns=0, memories=0, summary_rows=30) + sql_log.clear() + assert client.get(f"/api/adventures/{client.adv_id}/derived").status_code == 200 + large = len(sql_log) + + assert large <= small + 1, ( + f"{small} queries with 3 summaries, {large} with 33" + ) + + +def test_the_context_size_stops_growing_once_the_budget_is_reached(client): + """The companion to the query counts: more story, not more prompt. + + Measured from a story that already fills the budget. Comparing a short story + to a long one only shows that the prompt grew, which it is supposed to do + until it reaches the ceiling; what F03 is about is that it stops there. + """ + _grow(client, turns=140, memories=3, summary_rows=1) + filled = client.get(f"/api/adventures/{client.adv_id}/context").json() + budget = filled["tokens"]["budget"] + assert filled["tokens"]["total"] > budget * 0.5, ( + "the fixture never filled the budget, so this proves nothing" + ) + + _grow(client, turns=280, memories=0, summary_rows=0) + doubled = client.get(f"/api/adventures/{client.adv_id}/context").json() + + assert doubled["history"]["total"] > filled["history"]["total"] * 2, "fixture too small" + assert doubled["tokens"]["total"] <= budget + # Three times the story, and the prompt does not move. + assert doubled["tokens"]["total"] <= filled["tokens"]["total"] + 50, ( + f"{filled['tokens']['total']} -> {doubled['tokens']['total']} tokens " + f"while the story went from {filled['history']['total']} to " + f"{doubled['history']['total']} actions" + ) + # And it is bounded by the budget rather than by the length of the story. + assert doubled["history"]["included"] < doubled["history"]["total"] diff --git a/backend/tests/test_context_realistic.py b/backend/tests/test_context_realistic.py new file mode 100644 index 0000000..b01b37b --- /dev/null +++ b/backend/tests/test_context_realistic.py @@ -0,0 +1,238 @@ +"""M6 section 13: context assembly and derived work against a real model. + +The M6 equivalent of `test_narrative_realistic.py`, and it exists for the same +reason: memory and summary extraction can look correct against a tiny synthetic +prompt and behave differently under a full application context — a real narrator +instruction, real authoritative state, enough recent story to exercise +budgeting, a summary, and several memories. + +**What is asserted, and what is not.** These tests do not assert that the model +writes a good summary or picks the right memory. No test can, and a threshold +would fail when a model is swapped rather than when the code breaks. They assert +that the *application* stays correct around whatever the model produces: + +* the prompt stays inside its budget and keeps the reply reserve; +* a summary the model generates is anchored to the story it covers; +* a failure is recorded rather than swallowed; +* nothing from an abandoned line reaches the prompt. + +Model behaviour is recorded as evidence and printed, not asserted. + +## Running it + +Skipped unless an endpoint is configured, so the ordinary suite stays local, +deterministic and offline: + + AIDND_TEST_ENDPOINT=http://127.0.0.1:11434/v1 \\ + AIDND_TEST_MODEL=qwen2.5:3b-instruct \\ + AIDND_TEST_EMBED_MODEL=nomic-embed-text \\ + python -m pytest tests/test_context_realistic.py -v -s + +The endpoint is read from the environment and never written down here, and the +same endpoint policy the rest of the product enforces applies: loopback or a +trusted-LAN address, TLS verified, no cloud. +""" + +import asyncio +import json +import os + +import pytest +from fastapi import Depends +from fastapi.testclient import TestClient + +from app import auth, limits, memorybank, models, summaries +from app.database import Base, SessionLocal, engine, get_db +from app.main import app +from app.routers import adventures + +from fakes import ScriptedProvider, state_block + +ENDPOINT = os.environ.get("AIDND_TEST_ENDPOINT", "") +MODEL = os.environ.get("AIDND_TEST_MODEL", "") +EMBED_MODEL = os.environ.get("AIDND_TEST_EMBED_MODEL", "nomic-embed-text") + +pytestmark = pytest.mark.skipif( + not (ENDPOINT and MODEL), + reason="set AIDND_TEST_ENDPOINT and AIDND_TEST_MODEL to run against a real model", +) + +CANON = { + "rules": ["The Crooked Lantern is the only inn in the valley."], + "forbidden": ["No character may use magic."], +} + + +@pytest.fixture() +def client(monkeypatch): + Base.metadata.create_all(bind=engine) + memorybank._vector_cache.clear() + setup = SessionLocal() + user = models.User(is_guest=False, email="m6live@example.com") + setup.add(user) + setup.flush() + setup.add(models.Settings( + user_id=user.id, api_key="enc:dummy", endpoint_url=ENDPOINT, model=MODEL, + summary_model=MODEL, embedding_model=EMBED_MODEL, + context_token_budget=8192, max_output_tokens=700, memory_top_k=4, + model_timeout_seconds=600, + )) + adventure = models.Adventure( + user_id=user.id, title="The Crooked Lantern", + memory_bank_enabled=True, auto_summarize=True, campaign_canon=CANON, + ) + setup.add(adventure) + setup.flush() + setup.add(models.Action(adventure_id=adventure.id, type="start", + text="Rain hammers the road outside the Crooked Lantern.")) + setup.commit() + adv_id, user_id = adventure.id, user.id + setup.close() + + monkeypatch.setattr(limits, "check_row_cap", lambda *a, **k: None) + app.dependency_overrides[auth.get_current_user] = ( + lambda db=Depends(get_db): db.get(models.User, user_id) + ) + test_client = TestClient(app) + test_client.adv_id = adv_id + test_client.user_id = user_id + try: + yield test_client + finally: + app.dependency_overrides.clear() + memorybank._vector_cache.clear() + Base.metadata.drop_all(bind=engine) + + +def play_scripted(client, text, prose, events=None): + """A turn with a known outcome, so the fixture is deterministic.""" + real = adventures.turns.OpenAICompatibleProvider + adventures.turns.OpenAICompatibleProvider = ScriptedProvider + ScriptedProvider.replies = [f"{prose}\n{state_block(events or [])}"] + try: + r = client.post(f"/api/adventures/{client.adv_id}/actions", + json={"type": "do", "text": text}) + assert r.status_code == 200, r.text[:300] + finally: + adventures.turns.OpenAICompatibleProvider = real + + +def context(client) -> dict: + r = client.get(f"/api/adventures/{client.adv_id}/context") + assert r.status_code == 200, r.text[:300] + return r.json() + + +def test_a_real_summary_is_generated_and_anchored(client): + """The summariser runs against the real model, and what it writes is + anchored to the story it read rather than to a column.""" + play_scripted(client, "step inside", "Aldric shakes the rain from his coat.", [ + {"type": "create_entity", "entity": "aldric", "entity_type": "character", + "name": "Aldric"}, + {"type": "create_entity", "entity": "mara", "entity_type": "character", + "name": "Mara"}, + ]) + for i in range(18): + play_scripted(client, f"talk on {i}", + f"Mara pours another measure and tells him about the road north. " + f"The lantern gutters. [{i}]") + + asyncio.run(memorybank.run_post_turn(client.adv_id)) + + with SessionLocal() as db: + adventure = db.get(models.Adventure, client.adv_id) + rows = summaries.all_for(db, adventure) + eligible = summaries.current(db, adventure) + status = {r["kind"]: r["status"] for r in + __import__("app.derived", fromlist=["report"]).report(db, client.adv_id)} + + print(json.dumps({ + "model": MODEL, "embedding_model": EMBED_MODEL, + "summaries_written": len(rows), + "derived_status": status, + "summary_preview": (eligible.text[:300] if eligible else None), + }, indent=2, sort_keys=True)) + + assert status.get("summary") == "ok", f"the summariser failed: {status}" + assert rows, "no summary was written" + assert eligible is not None + # Anchored, not floating: it names the stretch of story it covers. + assert eligible.depth is not None + assert eligible.branch_id is not None + assert eligible.model_name == MODEL + # And it reaches the prompt. + assert eligible.text[:40] in "\n".join(s["text"] for s in context(client)["sections"]) + + +def test_the_prompt_stays_bounded_and_reserves_the_reply_under_real_context(client): + """Budgeting, measured on a realistic prompt rather than a synthetic one.""" + play_scripted(client, "step inside", "Aldric shakes the rain from his coat.", [ + {"type": "create_entity", "entity": "aldric", "entity_type": "character", + "name": "Aldric"}, + ]) + for i in range(40): + play_scripted(client, f"on {i}", + f"[{i}] " + "The lantern swings and the rain keeps on. " * 20) + asyncio.run(memorybank.run_post_turn(client.adv_id)) + + report = context(client) + print(json.dumps({ + "model": MODEL, + "budget": report["tokens"]["budget"], + "input_tokens": report["tokens"]["total"], + "output_reserve": report["tokens"]["output_reserve"], + "protected": report["tokens"]["protected"], + "available_for_history": report["tokens"]["available_for_history"], + "actions_included": report["history"]["included"], + "actions_total": report["history"]["total"], + "memories_used": len(report["memories"]["used"]) if report["memories"] else 0, + }, indent=2, sort_keys=True)) + + assert report["tokens"]["total"] <= report["tokens"]["budget"] + assert report["tokens"]["total"] + 700 <= report["tokens"]["budget"], ( + "the real prompt left no room for the configured reply" + ) + assert report["history"]["included"] < report["history"]["total"], ( + "the whole transcript was sent" + ) + + +def test_a_real_turn_still_generates_with_memory_and_summary_present(client): + """The end-to-end shape: a real narrator turn on a campaign that has a + generated summary, retrieved memories and authoritative state.""" + play_scripted(client, "step inside", "Aldric shakes the rain from his coat.", [ + {"type": "create_entity", "entity": "aldric", "entity_type": "character", + "name": "Aldric"}, + ]) + for i in range(18): + play_scripted(client, f"talk {i}", + f"They talk of the road north while the fire burns down. [{i}]") + asyncio.run(memorybank.run_post_turn(client.adv_id)) + + # A real turn, through the real provider. + r = client.post(f"/api/adventures/{client.adv_id}/actions", + json={"type": "do", "text": "ask Mara what lies north"}) + assert r.status_code == 200, r.text[:300] + assert '"error"' not in r.text, r.text[:400] + + with SessionLocal() as db: + action = (db.query(models.Action) + .filter_by(adventure_id=client.adv_id, type="ai") + .order_by(models.Action.id.desc()).first()) + snapshot = action.context_snapshot + text = action.text + + labels = [s["label"] for s in snapshot["sections"]] + print(json.dumps({ + "model": MODEL, + "sections": labels, + "input_tokens": snapshot["tokens"]["total"], + "output_reserve": snapshot["tokens"]["output_reserve"], + "reply_chars": len(text), + }, indent=2, sort_keys=True)) + + assert "narrator" in labels + assert "history" in labels + # The reply is a story, not protocol. + assert "```state" not in text + assert '"events"' not in text diff --git a/backend/tests/test_history_window.py b/backend/tests/test_history_window.py index b050523..1892f70 100644 --- a/backend/tests/test_history_window.py +++ b/backend/tests/test_history_window.py @@ -108,7 +108,12 @@ def actions_loaded(): # ------------------------------------------------------- the prompt is equal -@pytest.mark.parametrize("budget", [1024, 4096, 8192, 16384, 65536]) +# The smallest budget here is the tightest one this fixture can still build a +# prompt for. M6 reserves the reply out of the context budget, so 1024 with an +# 800-token reply and 750 tokens of protected prompt is no longer a +# configuration that produces a prompt — it raises `ContextOverflow`, which +# `test_a_budget_too_small_for_the_reply_is_refused` covers. +@pytest.mark.parametrize("budget", [2048, 4096, 8192, 16384, 65536]) def test_window_builds_the_same_prompt_as_the_whole_story(story, budget, monkeypatch): db, adventure, settings = story settings.context_token_budget = budget diff --git a/backend/tests/test_length_hint.py b/backend/tests/test_length_hint.py index bfa5d52..f77e46c 100644 --- a/backend/tests/test_length_hint.py +++ b/backend/tests/test_length_hint.py @@ -216,8 +216,13 @@ def test_prompt_stays_inside_the_budget_on_a_long_story(story): db.expire_all() adventure = db.get(models.Adventure, adventure.id) + # A large reply budget, so the length hint is long enough for a missing + # reservation to show. M6 reserves the reply out of the context budget, so + # the budget has to be large enough to hold both — 2048 with a 2400-token + # reply is a configuration that cannot produce a prompt at all, and now + # says so rather than silently overflowing. settings.max_output_tokens = 2400 - settings.context_token_budget = 2048 + settings.context_token_budget = 8192 _, _, report = builder.build_context(adventure, settings) assert report["history"]["included"] < 120, "budget was never actually filled" diff --git a/backend/tests/test_memory_nodes.py b/backend/tests/test_memory_nodes.py index bcc4ede..2fe3e1d 100644 --- a/backend/tests/test_memory_nodes.py +++ b/backend/tests/test_memory_nodes.py @@ -20,6 +20,7 @@ exactly as `test_branch_clause.py` builds it. python -m pytest tests/test_memory_nodes.py -v """ import asyncio +import math import pytest @@ -138,11 +139,26 @@ def forked(): nodes[f"C{depth}"] = add_node(db, adventure, c, depth, "C") db.flush() + # Distinct vectors, equally similar to the query. + # + # These tests are about *lineage visibility* — which memories a branch can + # see. They used to store the same vector in every memory, which was + # harmless until M6 added redundancy suppression: four identical vectors are + # four copies of one statement as far as retrieval is concerned, so they + # collapsed to one and the lineage assertions could no longer be read. + # + # Each vector below sits at the same angle from the query `(1, 0, 0)`, so + # ranking between them is unchanged, and far enough apart from each other + # (pairwise cosine -0.28 to 0.36) that none suppresses another. memories = { - "shared": add_memory(db, adventure, "on the shared trunk", nodes["A3"]), - "sibling": add_memory(db, adventure, "on A's own continuation", nodes["A5"]), - "b": add_memory(db, adventure, "on B", nodes["B5"]), - "c": add_memory(db, adventure, "on C", nodes["C7"]), + "shared": add_memory(db, adventure, "on the shared trunk", nodes["A3"], + vector=(0.6, 0.8, 0.0)), + "sibling": add_memory(db, adventure, "on A's own continuation", nodes["A5"], + vector=(0.6, -0.8, 0.0)), + "b": add_memory(db, adventure, "on B", nodes["B5"], + vector=(0.6, 0.0, 0.8)), + "c": add_memory(db, adventure, "on C", nodes["C7"], + vector=(0.6, 0.0, -0.8)), } adventure.head_branch_id = c.id adventure.head_depth = 7 @@ -162,6 +178,21 @@ def switch_to(db, adventure, branch_id, tip): db.commit() +@pytest.fixture(autouse=True) +def restore_embedding_provider(): + """Puts `memorybank.embedding_provider` back after every test here. + + `retrieved` below replaces it by assignment. Until M6 nothing restored it, + so a stub outlived the module and was still installed when a later file ran + (`tests/test_provider_wiring.py`, which asserts on the real factory). + """ + real = memorybank.embedding_provider + try: + yield + finally: + memorybank.embedding_provider = real + + def retrieved(adventure, settings) -> set[str]: memorybank.embedding_provider = lambda s: StubEmbedder() result = asyncio.run( @@ -381,6 +412,8 @@ def deeply_forked(): memory_top_k=50, )) + _spread = 2 * math.pi / 14 + def story(title, forks): adventure = models.Adventure( user_id=user.id, title=title, script_state={}, memory_bank_enabled=True, @@ -404,8 +437,20 @@ def deeply_forked(): nodes.append(add_node(db, adventure, branch, depth, "n")) depth += 1 db.flush() + _placed: list = [] for node in nodes[5::6]: # one memory per six actions, as the pass makes them - add_memory(db, adventure, f"memory at {node.depth}", node) + # A distinct direction per memory, all at the same angle from the + # query, so ranking between them is unaffected and M6's redundancy + # suppression does not collapse fourteen distinct memories into one. + # This test measures bytes fetched, not deduplication. + # + # Fourteen directions spread evenly around the circle orthogonal to + # the query are 2*pi/14 apart; the small shared component keeps the + # closest pair at cosine ~0.905, below the 0.93 redundancy bar. + angle = _spread * len(_placed) + _placed.append(node) + add_memory(db, adventure, f"memory at {node.depth}", node, + vector=(0.2, 0.98 * math.cos(angle), 0.98 * math.sin(angle))) adventure.head_branch_id = branch.id adventure.head_depth = depth - 1 return adventure diff --git a/backend/tests/test_memory_retrieval.py b/backend/tests/test_memory_retrieval.py index ba4cfe4..4b41d2c 100644 --- a/backend/tests/test_memory_retrieval.py +++ b/backend/tests/test_memory_retrieval.py @@ -36,6 +36,23 @@ class StubEmbedder: return [list(self.vector) for _ in texts] +@pytest.fixture(autouse=True) +def restore_embedding_provider(): + """Puts `memorybank.embedding_provider` back after every test here. + + The helpers below replace it by assignment rather than through monkeypatch, + and until M6 nothing put it back — so whichever test ran last left a stub + installed for the rest of the session. That was invisible while every other + memory test stubbed it too, and surfaced the moment M6 added a test that + asserts on the real factory (`tests/test_provider_wiring.py`). + """ + real = memorybank.embedding_provider + try: + yield + finally: + memorybank.embedding_provider = real + + @pytest.fixture() def db(): Base.metadata.create_all(bind=engine) diff --git a/backend/tests/test_prompt_caching.py b/backend/tests/test_prompt_caching.py index fa2ced6..0dfb4e9 100644 --- a/backend/tests/test_prompt_caching.py +++ b/backend/tests/test_prompt_caching.py @@ -29,7 +29,7 @@ import os import pytest -from app import models, worldstate +from app import models, summaries, worldstate from app import narrative from app.context import builder from app.database import Base, SessionLocal, engine @@ -97,7 +97,6 @@ def story(): user_id=user.id, title="A", scenario_id=scenario.id, script_state={}, memory="The hero hunts bandits.", ai_instructions="Write in second person.", - story_summary="The hero left the village.", world_state=worldstate.instantiate(SCHEMA), narrative_state=NARRATIVE, # Phase 18. Set here so that every test in this file runs with a @@ -113,6 +112,11 @@ def story(): db.add(models.Action(adventure_id=adventure.id, type="ai" if i % 2 else "do", text=f"[{i}] The road bends onward past the treeline.")) + db.flush() + # M6: the summary is a row anchored to the story it covers, not a column. + # `build_context` reads whichever summary is eligible for the current head, + # so a test that wants one in the prompt has to record one. + summaries.record(db, adventure, "The hero left the village.") db.commit() db.expire_all() adventure = db.get(models.Adventure, adventure.id) @@ -195,7 +199,7 @@ def test_live_sections_are_still_charged_to_the_budget(story): settings = db.get(models.Settings, settings.id) _, _, lean = builder.build_context(adventure, settings) - adventure.story_summary = "The hero left the village. " * 150 + summaries.record(db, adventure, "The hero left the village. " * 150) db.commit() _, _, fat = builder.build_context(adventure, settings) diff --git a/backend/tests/test_provider_wiring.py b/backend/tests/test_provider_wiring.py new file mode 100644 index 0000000..6aa8542 --- /dev/null +++ b/backend/tests/test_provider_wiring.py @@ -0,0 +1,225 @@ +"""M6: the provider construction path, exercised for real. + +M2 shipped with the entire memory bank dead and the full suite green. The +summariser and the embedder were built from `Settings` attributes that had moved, +the resulting `AttributeError` was raised inside a fire-and-forget task, and +every memory test had stubbed the factories out — so nothing anywhere noticed +(`BUILD-MILESTONES.md`, note from M2). + +These tests exist so that cannot happen twice. **Nothing here mocks a provider +factory.** They call the real factories with a real `Settings` row read back out +of the database, and assert that the configured values arrive at the object that +consumes them. A renamed or removed column fails here loudly instead of killing +the memory bank quietly. + +Network is never touched: constructing a provider makes no request. The one test +that would make one is skipped unless a trusted-LAN endpoint is configured, and +it is reported separately from these. + + python -m pytest tests/test_provider_wiring.py -v +""" + +import os + +import pytest + +from app import endpoints, memorybank, models +from app.database import Base, SessionLocal, engine +from app.providers import OpenAICompatibleProvider, ProviderError + + +@pytest.fixture() +def settings(): + """A real Settings row, round-tripped through the database. + + Round-tripping matters: a column that was renamed in the model but still + referenced by a factory fails on the read, which is the failure this module + is here to produce. + """ + Base.metadata.create_all(bind=engine) + db = SessionLocal() + user = models.User(is_guest=False, email="wiring@example.com") + db.add(user) + db.flush() + row = models.Settings( + user_id=user.id, + api_key="enc:dummy", + endpoint_url="http://127.0.0.1:11434/v1", + model="narrator-model", + summary_model="summariser-model", + embedding_model="embedding-model", + api_mode="chat", + model_timeout_seconds=123, + max_output_tokens=456, + context_token_budget=4096, + ) + db.add(row) + db.commit() + row_id = row.id + db.close() + + db = SessionLocal() + try: + yield db.get(models.Settings, row_id) + finally: + db.close() + Base.metadata.drop_all(bind=engine) + + +# ------------------------------------------------- the real construction path + +def test_the_summary_provider_is_built_from_the_configured_values(settings): + """Every value the summariser needs reaches the provider that uses it.""" + provider = memorybank.summary_provider(settings) + + assert isinstance(provider, OpenAICompatibleProvider) + assert provider.base_url == "http://127.0.0.1:11434/v1" + assert provider.model == "summariser-model" + assert provider.api_mode == "chat" + assert provider.read_timeout == 123 + + +def test_the_summary_provider_falls_back_to_the_narrator_model(settings): + """An empty summary model means "use the main one", not "use nothing".""" + settings.summary_model = "" + assert memorybank.summary_provider(settings).model == "narrator-model" + + +def test_the_embedding_provider_is_built_from_the_configured_values(settings): + provider = memorybank.embedding_provider(settings) + + assert isinstance(provider, OpenAICompatibleProvider) + assert provider.base_url == "http://127.0.0.1:11434/v1" + assert provider.model == "embedding-model" + + +def test_the_narrator_provider_is_built_from_the_configured_values(settings): + """The turn path builds its own provider; the same values have to reach it.""" + provider = OpenAICompatibleProvider( + settings.endpoint_url, settings.model, settings.api_mode, + settings.model_timeout_seconds, + ) + assert provider.base_url == "http://127.0.0.1:11434/v1" + assert provider.model == "narrator-model" + assert provider.api_mode == "chat" + assert provider.read_timeout == 123 + + +@pytest.mark.parametrize("attribute", [ + "endpoint_url", "model", "summary_model", "embedding_model", "api_mode", + "model_timeout_seconds", "max_output_tokens", "context_token_budget", +]) +def test_every_settings_attribute_the_providers_read_still_exists(settings, attribute): + """The named guard against M2's failure. + + Each attribute here is one a factory or the context builder reads. If a + migration renames one, this fails by name instead of the memory bank dying + in a task nobody is watching. + """ + assert hasattr(settings, attribute), ( + f"Settings.{attribute} is gone; something that builds a provider reads it" + ) + + +def test_building_a_provider_makes_no_request(settings): + """Construction is inert, so these tests are safe to run offline.""" + import socket + + def refuse(*args, **kwargs): # pragma: no cover - only runs on a failure + raise AssertionError("provider construction opened a socket") + + real = socket.socket.connect + socket.socket.connect = refuse + try: + memorybank.summary_provider(settings) + memorybank.embedding_provider(settings) + finally: + socket.socket.connect = real + + +# ------------------------------------------------------- the endpoint policy + +def test_the_embedding_path_enforces_the_same_endpoint_policy(settings): + """M6 section 18. Embedding inputs are story text, and they go to the same + kind of endpoint under the same rule as a narrator prompt. + + Asserted through the real `embed()` rather than by reading the source: a + check that exists but is not reached would pass a source inspection. + """ + import asyncio + + provider = OpenAICompatibleProvider("https://api.openai.com/v1", "embedding-model") + with pytest.raises(ProviderError) as exc: + asyncio.run(provider.embed(["a line of someone's story"])) + assert "can't be used" in str(exc.value) + + +def test_the_policy_refuses_a_public_address_for_embeddings(): + """The rule is the address, not the name of the caller.""" + assert endpoints.rejection_reason("https://api.openai.com/v1/embeddings") + assert endpoints.rejection_reason("http://8.8.8.8:11434/v1/embeddings") + # And permits the local endpoints the product is built for. + assert endpoints.rejection_reason("http://127.0.0.1:11434/v1/embeddings") is None + + +def test_no_cloud_or_remote_vector_service_is_configured_anywhere(): + """M6 section 18: no new network path. Checked against the source, because + the point is that no such code exists to be exercised.""" + import pathlib + + forbidden = ( + "api.openai.com", "api.anthropic.com", "pinecone", "weaviate", + "qdrant", "chromadb", "cohere.ai", "huggingface.co/api", + ) + root = pathlib.Path(__file__).resolve().parent.parent / "app" + offenders = [] + for path in root.rglob("*.py"): + text = path.read_text() + for needle in forbidden: + # `endpoints.py` names cloud hosts in order to refuse them. + if needle in text and path.name != "endpoints.py": + offenders.append(f"{path.name}: {needle}") + assert not offenders, offenders + + +# ------------------------------------------------- the live endpoint, if any + +@pytest.mark.skipif( + not os.environ.get("AIDND_TEST_ENDPOINT"), + reason="set AIDND_TEST_ENDPOINT and AIDND_TEST_MODEL to run against a real model", +) +def test_the_real_construction_path_reaches_a_real_endpoint(): + """The wiring test with the network attached. + + Reported separately from the tests above: this one proves the constructed + provider can actually talk to the configured endpoint, which is the half a + unit test cannot show. It uses the ordinary endpoint policy — no TLS + weakening, no allowlist bypass. + """ + import asyncio + + endpoint = os.environ["AIDND_TEST_ENDPOINT"] + model = os.environ.get("AIDND_TEST_EMBED_MODEL", "nomic-embed-text") + assert endpoints.rejection_reason(endpoint) is None, ( + "the configured test endpoint is refused by the policy" + ) + Base.metadata.create_all(bind=engine) + db = SessionLocal() + try: + user = models.User(is_guest=False, email="live@example.com") + db.add(user) + db.flush() + row = models.Settings( + user_id=user.id, api_key="enc:dummy", endpoint_url=endpoint, + model=os.environ.get("AIDND_TEST_MODEL", ""), embedding_model=model, + ) + db.add(row) + db.commit() + provider = memorybank.embedding_provider(row) + vectors = asyncio.run(provider.embed(["Aldric hid the ledger."])) + finally: + db.close() + Base.metadata.drop_all(bind=engine) + + assert len(vectors) == 1 + assert len(vectors[0]) > 8, "the endpoint returned no usable vector" diff --git a/backend/tests/test_tree_migration.py b/backend/tests/test_tree_migration.py index 84a1d28..80e97b9 100644 --- a/backend/tests/test_tree_migration.py +++ b/backend/tests/test_tree_migration.py @@ -126,6 +126,7 @@ def pre_tree(): # another table still points at. Any future table that references these # has to be added to the front of this list. for table in ("state_events", "state_proposals", "checkpoints", + "summaries", "derived_status", "actions", "memories", "branches", "adventures"): conn.execute(text(f"DROP TABLE IF EXISTS {table}")) for ddl in PRE_TREE_DDL: @@ -600,6 +601,7 @@ def pre_split(): Base.metadata.create_all(bind=engine) with engine.begin() as conn: for table in ("state_events", "state_proposals", "checkpoints", + "summaries", "derived_status", "actions", "memories", "branches", "adventures"): conn.execute(text(f"DROP TABLE IF EXISTS {table}")) for ddl in PRE_TREE_DDL: diff --git a/frontend/src/pages/Play/panels/InsightsPanel.jsx b/frontend/src/pages/Play/panels/InsightsPanel.jsx index 4fab138..f0f6760 100644 --- a/frontend/src/pages/Play/panels/InsightsPanel.jsx +++ b/frontend/src/pages/Play/panels/InsightsPanel.jsx @@ -46,6 +46,12 @@ function InsightsPanel({ advId, inspectActionId, onClearInspect, refreshKey }) { )}
{tokens.total} / {tokens.budget} tokens + {/* M6: the reply has to fit in the same window, so say how much of it + is being held back for one. Without this the reader can see that + the prompt fits and still get a truncated turn. */} + {tokens.output_reserve > 0 && ( + · {tokens.output_reserve} reserved for the reply + )}
(
- ▸ memory retrieved ({m.pinned ? 'pinned' : `similarity ${m.similarity.toFixed(2)}`}): - {' '}{m.text.length > 90 ? m.text.slice(0, 90) + '…' : m.text} + ▸ memory retrieved ({m.pinned ? 'pinned' : `similarity ${m.similarity.toFixed(2)}`}) + {/* M6: what weight it carries, and where it came from. An + inference must not read like a record. */} + {m.authority === 'heuristic' + ? · inferred + : · from the story} + {m.source?.depth != null && ( + + {' '}· turn {m.source.source_start === m.source.source_end + ? m.source.depth + : `${m.source.source_start}–${m.source.source_end}`} + + )} + : {m.text.length > 90 ? m.text.slice(0, 90) + '…' : m.text}
))} + {report.memories.considered != null && ( +
+ {report.memories.considered} memories eligible on this line of the story. +
+ )} {!report.memories.error && report.memories.used?.length === 0 && (
Memory bank on — no memories retrieved.
)} )} + {/* M6: which summary was used, and what stretch of story it covers, so + "what history did that summary cover?" is answerable here. */} + {report.summary ? ( +
+
+ ▸ summary in use + + {' '}· covers {report.summary.source_start != null + ? `turns ${report.summary.source_start}–${report.summary.source_end}` + : `up to turn ${report.summary.source_end ?? report.summary.depth}`} + {report.summary.trigger === 'manual' ? ' · written by you' : ' · generated'} + +
+
+ ) : ( +
▸ no summary is eligible for this point in the story.
+ )} + {/* M6: a dead memory bank used to be invisible. It is not any more. */} + {report.derived?.some((d) => d.status === 'failed') && ( +
+ {report.derived.filter((d) => d.status === 'failed').map((d) => ( +
+ ⚠ Background {d.kind} work is failing ({d.failures} + {d.failures === 1 ? ' attempt' : ' attempts'}): {d.detail} +
+ ))} +
+ The story itself is unaffected — this only stops new {' '} + {report.derived.filter((d) => d.status === 'failed') + .map((d) => d.kind).join(' and ')} work being written. +
+
+ )} {sections.map((s, i) => ( diff --git a/frontend/src/styles/play.css b/frontend/src/styles/play.css index b7f995e..59c0d3e 100644 --- a/frontend/src/styles/play.css +++ b/frontend/src/styles/play.css @@ -500,3 +500,10 @@ color: var(--text-dim); font-variant-numeric: tabular-nums; } + +/* M6: a retrieved memory that is an interpretation rather than a record. The + colour is the same one the state panel uses for a reader's own correction, + because both mean "weigh this differently from accepted story". */ +.mem-heuristic { + color: #d8a657; +} diff --git a/planning/BUILD-MILESTONES.md b/planning/BUILD-MILESTONES.md index e33f988..5ed6927 100644 --- a/planning/BUILD-MILESTONES.md +++ b/planning/BUILD-MILESTONES.md @@ -1,6 +1,6 @@ # Adventure Storyteller — Production Build Milestones -**Status:** In implementation. M1-M5 complete and accepted (M1 and M2: 2026-09-02; M3 and M4: 2026-09-03; M5: 2026-09-04, after an independent review and a corrective pass); M6 — Branch-Safe Context, Summaries, and Long-Term Story Memory — next to brief +**Status:** In implementation. M1-M6 complete and accepted (M1 and M2: 2026-09-02; M3 and M4: 2026-09-03; M5: 2026-09-04; M6: 2026-09-06, each of the last two after an independent review and a corrective pass); M7 — First-Class Imported Knowledge Library — next to brief **Base:** AI-DnD `d72f7c1bda0f34fccd84afb7a25c34eb01c901de` ## 1. Purpose @@ -699,6 +699,85 @@ Evidence: `planning/archive/milestone-reports/M2-IMPLEMENTATION-REPORT.md` §A.1 --- +## M6 — Outcome + +**Complete and accepted 2026-09-06**, after an independent review that found E03 +still failing and a corrective pass that fixed it. Report, including the review +findings and the corrective addendum: +`planning/reports/M6-IMPLEMENTATION-REPORT.md`. + +**M7 is authorized**: the corrected M6 evidence passes, including E03 end to end +against a real local summariser. + +What was inherited and kept, rather than rebuilt: + +- **Memory lineage was already correct.** Memories carry `(branch_id, depth)` + and retrieval filters them through the capped-path clause. The ten-step + negative control was measured passing against the M5 baseline *before* any M6 + change, and is now pinned by tests. M6 added provenance to the retrieval + result and an authority classification; it did not rewrite the bank. +- The lineage chokepoint, the cursors, and the post-turn pass are unchanged in + shape. + +What M6 changed: + +- **Summary lineage.** Summaries move from a single `adventures.story_summary` + column to `summaries` rows carrying a coordinate and a source range, filtered + by the same lineage clause as memories. E03 was measured leaking at the M5 + baseline and is now closed. +- **Memory authority.** `accepted_story` vs `heuristic`, classified by the + application, marked in the prompt. +- **Context budgeting.** The reply is reserved out of the context budget, and an + impossible configuration raises `ContextOverflow` rather than producing a + prompt known to overflow. Nothing was reserved before M6. +- **Background failure observability.** `derived_status` rows, an API endpoint + and an Insights surface, so the M2 failure — the whole memory bank dead with a + green suite — is visible if it recurs. +- **Real provider-wiring tests**, which mock no factory, plus two pre-existing + test-suite leaks they exposed and which are fixed. + +What the independent review found, and what the corrective pass did: + +- **E03 was still failing (M6-F1).** Summary *rows* were lineage-anchored, but + generation was seeded from `adventures.story_summary`, a campaign-global + column with no lineage — so a summary generated after a divergence inherited + the abandoned line's prose inside a correctly anchored row. Fixed by seeding + from `summaries.current`. The lesson is recorded in `V1-ACCEPTANCE-TESTS.md`: + a valid E03 test must **regenerate** a summary after diverging, not merely + check that the old row went ineligible. +- **F02 passed by one slot (M6-F2).** With a real embedding model, four + near-identical memories crowded out the one that mattered; the clue survived + only because the default `memory_top_k` is 5. Redundancy suppression now runs + before the final cut, and the clue is retrieved at top_k 5, 4 and 3. +- **Two test defects (M6-F3, M6-F4).** The unit fixture made real network calls + to the default endpoint, and the E03 test and browser check shared the blind + spot above. Both fixed; the browser suite now has a dedicated E03 scenario + that regenerates a summary after divergence. +- **A misleading status (M6-F5).** Derived work that had nothing to do reported + `ok`; it now reports `idle`. + +Debt carried forward, deliberately: + +- **Retrieval ranking** is cosine similarity plus an explicit pin. Importance, + entity overlap, recency and thread overlap are contemplated by + `CONTEXT-AND-MEMORY.md` §20 and are not implemented. Redundancy suppression + covers the failure mode M6 measured; the richer ranking is open, and matters + to M7 because imported material will compete for the same budget. +- **Cross-layer duplication** (§22) is unimplemented: the same fact can appear + in state, memory and history at once. Bounded and legible, but not the + "highest-authority concise representation" the plan asks for. +- **M7:** imported knowledge is not implemented, and nothing was built to fill + its inspector section. +- **M8:** the Insights additions are functional, not designed; the broader UX + pass remains M8's. +- **M9:** derived rows are not carried in an export bundle, so an imported + campaign starts with no summaries or memories and rebuilds them. Authoritative + history is unaffected. +- **M11:** the long-context evidence here is a bounded fixture, not the M01 + 100-turn campaign. + +--- + # M7 — First-Class Imported Knowledge Library ## Objective diff --git a/planning/CONTEXT-AND-MEMORY.md b/planning/CONTEXT-AND-MEMORY.md index 0641680..aa27103 100644 --- a/planning/CONTEXT-AND-MEMORY.md +++ b/planning/CONTEXT-AND-MEMORY.md @@ -290,6 +290,42 @@ If the user restores or diverges before part of that source history: A summary from abandoned history must never leak into active context. +### As implemented (M6) + +Lineage safety here has **two** halves, and the M6 review found that having only +the first is not enough. + +**The row must be eligible.** A summary is a row with a coordinate, exactly as a +memory is: `branch_id` and `depth` name the last node it covers, +`source_start`/`source_end` the stretch. Eligibility is one question — is that +coordinate on the active, head-capped lineage? — answered by +`lineage.Path.clause`, the same chokepoint every read of the story goes through. +Undo, Redo, Save Point restore and divergence all fall out of that without a +rule of their own. + +**The input must be eligible too.** Summary generation is seeded only from a +summary that is itself valid on the current head-capped lineage +(`summaries.current`). Where none is, the new line starts from no previous +summary. + +The second half was missing in the first M6 implementation and E03 failed +because of it: the summariser seeded itself from `adventures.story_summary`, a +campaign-global column with no lineage, so after a divergence it was handed the +abandoned line's prose and asked to update it. The row it produced was correctly +anchored to the new branch and therefore *looked* lineage-safe while its +sentences described a story the reader had left. Anchoring the output is not +enough; the input has to be scoped by the same rule. + +Abandoned summaries are retained, never deleted, and become eligible again if +the reader returns to the line that produced them. + +`adventures.story_summary` remains, as a reader-facing convenience only: the +Plot panel edits it and the export bundle carries it. It is a mirror of whichever +summary is currently eligible — kept in step when one is written and when the +head moves — and **nothing authoritative may read it**. A summary the reader +types is recorded as a row anchored at the position they typed it at, so it +behaves like any other. + ## 12. Story Memory Long-term story memory should retrieve important older information that is not present in recent history or current summary. @@ -372,6 +408,19 @@ The narrator should be told which memories are: This avoids turning guesses into canon. +### As implemented (M6) + +Two values, `accepted_story` and `heuristic`, on `Memory.authority`. The +**application** classifies, not the model: `memorybank.classify_authority` +reads the memory's own text for hedging ("seemed", "appeared to", "probably"), +so an extractor cannot promote a guess by asserting it confidently. The prompt +marks a heuristic memory `[inferred]` and says in words that such lines are +interpretation rather than established fact. + +Retrieval never writes state. A memory of either authority is something the +narrator is shown; the only path to an authoritative change remains the M5 typed +event pipeline (ADR 013). + ## 15. Memory Creation Memories may be generated after accepted turns. @@ -494,6 +543,16 @@ Potential ranking factors: The final ranking formula should be simple and inspectable. +### As implemented (M6) + +Ranking is **cosine similarity against the retrieval query, plus an explicit +pin**. The other factors listed above — importance, lexical match, recency, +entity overlap, story-thread overlap — are **not implemented**, and remain +future work rather than something M6 delivered. + +What M6 does implement, because similarity alone proved insufficient, is +redundancy suppression before the final selection: see §22. + ## 21. Memory Budget Retrieved memories should have a bounded token budget. @@ -524,6 +583,31 @@ there is little value in also including three memories that say the same thing. Context builder should prefer the highest-authority concise representation. +### As implemented (M6) + +**Between memories, yes.** Retrieval walks the ranked candidates and skips one +that repeats a memory already chosen, keeping the highest-ranked statement of a +fact and its provenance. Two rules bound it: authority is never crossed, so an +inference can never suppress a record or the reverse; and the bar is high, set +where measurement showed distinct facts stop appearing. The number of +suppressed candidates is reported in the context record, so a memory that was +considered and set aside can be told from one that was never eligible. + +The threshold is a measured property of the embedding model in use, not a +universal constant, and `memorybank.REDUNDANT_SIMILARITY` records the +measurement beside the value. + +The same measurement ruled out the more obvious test. Word overlap fires hardest +on exactly the pair that must not be merged — "Mara promised to return before +dawn" against "Aldric promised to return before dawn" shares most of its words +and means something else — and is weakest on filler that plainly repeats itself. +Wording is a poor proxy for sameness of fact. + +**Across layers, not yet.** A fact can still appear in the authoritative state, +in a memory and in recent history at once. That is bounded and legible, but it +is not the "highest-authority concise representation" this section asks for, and +it remains open. + ## 23. Imported Knowledge Categories Imported local files must be classified as: @@ -672,6 +756,28 @@ Recommended behavior: - add safety margin, - fail gracefully if protected context alone is too large. +### As implemented (M6) + +`context_token_budget` is the whole window, so the reply is subtracted from it +before any history is selected: + +```text +available for history = context_token_budget + - (system, canon, state, summary, memories, user input) + - (max_output_tokens + 64) +``` + +The 64-token margin covers the separators added after budgeting and the drift +between the app's tokenizer and the serving model's; it is fixed rather than +proportional because what it absorbs does not scale with the budget. + +When the protected part alone does not fit, `build_context` raises +`ContextOverflow` naming both figures and what to change, and the turn reports +that as a failed turn. It does not build a prompt it knows will overflow. + +Before M6 nothing was reserved: the builder spent the entire budget on input and +left the reply to fit in whatever the endpoint had left. + ## 33. Context Snapshot Every narrator generation should preserve enough information to reconstruct the effective prompt. diff --git a/planning/DATA-MODEL.md b/planning/DATA-MODEL.md index b286cf0..f1a572b 100644 --- a/planning/DATA-MODEL.md +++ b/planning/DATA-MODEL.md @@ -453,6 +453,40 @@ story_thread: These are narrative continuity tools, not RPG quests. +## 16A. Derived Context Tables (M6, as implemented) + +Two tables and one column carry M6's derived context. All three are derived +data: deleting them changes no accepted history, no authoritative state and no +head position. + +```text +summaries + id, adventure_id + text + branch_id, depth the coordinate of the last node covered + source_start, source_end the stretch of story summarized + trigger "interval" (generated) or "manual" (reader-written) + model_name, created_at + +derived_status one row per (adventure, kind) + kind "memory" | "summary" | "embedding" + status "ok" (did work) | "idle" (nothing pending) | "failed" + detail, failures + last_attempt_at, last_success_at + +memories.authority "accepted_story" | "heuristic" +``` + +`summaries` mirrors the shape `memories` already had, deliberately: both are +derived rows anchored to a coordinate on a path, and both are filtered by the +same lineage clause — and both are also the *input* to the next round of derived +work, which is why summary generation reads `summaries.current` rather than any +campaign-global field. + +`adventures.story_summary` is retained as the reader's edit surface and the +export field, mirroring whichever summary is eligible. It carries no lineage of +its own and nothing authoritative reads it. + ## 17. State Version The system must reconstruct authoritative state at any retained turn. diff --git a/planning/README.md b/planning/README.md index 83c0e4c..f275d66 100644 --- a/planning/README.md +++ b/planning/README.md @@ -3,9 +3,10 @@ **This file is the index. Start here.** **Current state:** Phase 0 complete; AI-DnD forked as the production base; -milestones **M1, M2, M3 and M4 implemented and accepted** (M3 and M4: -2026-09-03). -**Next:** **M5 — Genre-Neutral Authoritative Narrative State.** Its brief has not +milestones **M1 through M6 implemented and accepted** (M3 and M4: 2026-09-03; +M5: 2026-09-04; M6: 2026-09-06). M5 and M6 were each accepted only after an +independent review found a real defect and a corrective pass fixed it. +**Next:** **M7 — First-Class Imported Knowledge Library.** Its brief has not been written yet, and writing it is the current action. **Package version:** see `VERSION.md`, which records what each revision changed @@ -245,13 +246,14 @@ M5-M11, one at a time see BUILD-MILESTONES.md **One milestone at a time. Do not begin a milestone before its brief exists.** -**No M5 brief has been prepared.** Writing one is the current action, informed by -the M4 report's §U readiness assessment and by the note `BUILD-MILESTONES.md` -attaches to M5 — in particular that M4 added 55 tests using the inherited -world-state values as deterministic instrumentation, which M5 must **move rather -than delete**. +**No M7 brief has been prepared.** Writing one is the current action, informed by +the M6 report's §W readiness assessment and by the retrieval debt +`BUILD-MILESTONES.md` records against M6 — in particular that ranking is +similarity plus a pin, so imported material will compete for the same memory +budget as story memory, and that cross-layer duplication +(`CONTEXT-AND-MEMORY.md` §22) is still open. -**No conditions remain open on M1-M4.** The browser smoke condition that M3 and +**No conditions remain open on M1-M6.** The browser smoke condition that M3 and M4 both carried was satisfied at M4 closeout: a real Firefox exercised the rendered DOM for both milestones' controls, 44/44 checks passing. The M3 report's §M.2 and the M4 report's §M record the condition as it stood; the M4 report's §W diff --git a/planning/TECHNICAL-DESIGN.md b/planning/TECHNICAL-DESIGN.md index faaf333..6511ea5 100644 --- a/planning/TECHNICAL-DESIGN.md +++ b/planning/TECHNICAL-DESIGN.md @@ -394,6 +394,79 @@ any distance, in either direction, and identical whether the position is reached from in front of it or from behind. This is the property §10.4's hybrid storage must preserve. +### M6 — derived context: summaries, memory and budgeting + +Four things future milestones rely on, all built on the lineage machinery M3-M5 +established rather than beside it. + +**Summary lineage — both halves.** A summary is a `summaries` row carrying +`(branch_id, depth)` for the last node it covers plus a +`source_start`/`source_end` range. The invariant M6 holds is: + +> Both summary eligibility and the prior-summary input to the summarizer are +> lineage-scoped. + +*Eligibility* is `lineage.Path.clause` over the row's coordinate — the same +capped-path clause that filters actions and memories — so Undo, Redo, Save Point +restore and divergence need no summary-specific rule. *Input* is +`summaries.current`, the same question the context builder asks, so a summary is +only ever built on top of one that is valid where the story now stands; where +none is, generation starts from nothing. + +The second half is not decorative. The first M6 implementation had only the +first, seeding generation from `adventures.story_summary`, and the review +demonstrated abandoned prose reaching an active prompt inside a row that was +itself correctly anchored. Anchoring the output does not make the content safe. + +Nothing is deleted when a line is abandoned. `adventures.story_summary` survives +as a reader-facing convenience only — the Plot panel edits it, the export bundle +carries it — mirroring whichever summary is eligible, kept in step by +`summaries.record` and by `attempts.restore_state` when the head moves. Nothing +authoritative reads it. + +**Memory lineage and provenance.** Unchanged from what M3 built and M6 verified: +a memory carries `(branch_id, depth)` and a source range, and retrieval filters +through the capped path. M6 adds provenance to the *retrieval result*, in the +same query that fetches the text, so the inspector can answer "where did this +come from?" without a query per memory. + +**Memory authority.** `Memory.authority` is `accepted_story` or `heuristic`, +decided by the application in `memorybank.classify_authority`, and rendered into +the prompt as an explicit mark. Retrieval never writes state; the M5 typed-event +path remains the only route to an authoritative change. + +**Retrieval ranking and redundancy.** Ranking is cosine similarity plus an +explicit pin; the other factors `CONTEXT-AND-MEMORY.md` §20 contemplates are not +implemented. Before the final top-k cut, retrieval drops a candidate that +repeats one already chosen, never across authority classes, at a threshold +measured against the configured embedding model +(`memorybank.REDUNDANT_SIMILARITY`). Suppressed candidates are reported so the +selection stays inspectable. Without this, a stretch of repetitive story fills +the whole memory budget with near-copies and evicts the one memory that +mattered — which the review measured happening. + +**Context budgeting.** The reply is reserved out of `context_token_budget` +before history is selected, with a fixed 64-token margin. Protected content — +narrator rules, canon, authoritative state, the reader's input, the reply +reserve — is never dropped to fit older prose; history is the elastic part and +is filled newest-first until the remaining budget is spent. If the protected +part alone exceeds the budget, `build_context` raises `ContextOverflow` rather +than assembling a prompt known to overflow. + +**Background failure observability.** Derived work (memory extraction, summary +generation, embedding) runs in a fire-and-forget task and must not take an +accepted turn down with it. Each pass is wrapped so that a failure rolls back +only its own uncommitted work and writes a `derived_status` row naming the kind, +the error and the attempt count. That row is served by +`GET /adventures/{id}/derived` and shown in the Insights panel. M2 shipped with +the whole memory bank dead and the suite green; this is the mechanism that makes +the same failure visible. + +**Prompt inspection.** The context report carries per-section token counts, the +budget, the output reserve, the protected total, the history allowance, the +summary's provenance, each retrieved memory's authority and source coordinate, +and the derived-work status. + **Divergence is a property of the lineage, not a flag.** The first write below a moved-back head forks; Undo alone never does. After the fork, the displaced future is no longer on the lineage being read, so ordinary Redo finds nothing diff --git a/planning/V1-ACCEPTANCE-TESTS.md b/planning/V1-ACCEPTANCE-TESTS.md index 481c2fc..af4c500 100644 --- a/planning/V1-ACCEPTANCE-TESTS.md +++ b/planning/V1-ACCEPTANCE-TESTS.md @@ -916,6 +916,15 @@ Continue enough turns to exercise long-term memory retrieval. ### Pass Narrator does not retrieve/use the discarded revelation as active-history truth. + +### Result — PASS (M6, 2026-09-05) +The ten-step negative control is `test_e02_the_ten_step_memory_negative_control`, +with the Save Point variant beside it. Both assert on the assembled prompt and +on the eligibility clause, not on the narration. + +Measured passing against the M5 baseline *before* any M6 change: memory lineage +was inherited correct, and M6's contribution here is the test that pins it. + --- ## E03 — Abandoned Summary Cannot Leak @@ -932,6 +941,42 @@ Narrator does not retrieve/use the discarded revelation as active-history truth. ### Pass Old summary content from abandoned future is not applied. + +### Result — PASS (M6 corrective pass, 2026-09-06) + +Failed twice before it passed, and the history matters because it defines the +shape a valid E03 test has to have. + +1. At the M5 baseline the summary was a single column with no coordinate and + survived Undo plus divergence into the active prompt. +2. The first M6 implementation made summaries lineage-anchored rows, and the + test written for it checked that the **old row** became ineligible. The + independent review then found E03 still failing end to end: generation was + seeded from `adventures.story_summary`, so the summary produced *on the new + line* inherited the abandoned line's prose inside a correctly anchored row. +3. The corrective pass seeds generation from `summaries.current`. + +**A valid E03 test must regenerate a summary after the divergence.** Checking +only that the old row is ineligible passes while the defect is live. The +regression now required is: + +```text +path A: enough history for a real summary, sentinel established on it + POSITIVE CONTROL — the sentinel is in the path-A summary and prompt +move the head below the sentinel, diverge +path B: play far enough that a NEW summary is generated + prove a new summary row exists and is not path A's + prove no path-A action is on path B's lineage + prove the sentinel is absent from the new summary + prove the sentinel is absent from the complete active prompt + prove the old row is retained but ineligible +``` + +Evidence: `test_e03_a_summary_generated_after_divergence_carries_no_abandoned_content` +(deterministic, fails against the pre-corrective implementation); the same +sequence against a real local summariser; and a dedicated browser scenario that +regenerates a summary after diverging rather than repeating the old blind spot. + --- ## E04 — Scene State Is Lineage-Safe @@ -960,6 +1005,12 @@ Conduct a multi-turn conversation with Mara. ### Pass Narrator remembers immediately preceding dialogue and actions. + +### Result — PASS (M6, 2026-09-05) +`test_f01_recent_turns_stay_in_the_prompt`: the preceding turns and the reader's +own input are present in the assembled prompt, asserted on the context report +rather than on the narration. + --- ## F02 — Old Important Event Retrieval @@ -974,6 +1025,27 @@ Narrator remembers immediately preceding dialogue and actions. ### Pass Relevant old clue can be recovered through summary/memory/state. + +### Result — PASS (M6 corrective pass, 2026-09-06) + +A distinctive clue is planted, long turns are played over it, and the clue is +*absent* from the verbatim history sections and *present* through a retrieved +memory, with `history.included < history.total` — so recovery does not come from +sending the transcript. + +The independent review found this passing only by a one-slot margin: with a real +embedding model, four near-identical filler memories scored 0.79–0.82 against +the clue's 0.61, so the clue placed fifth and survived only because the default +`memory_top_k` is 5. At 4 it was evicted and F02 failed. + +The corrective pass added redundancy suppression before the final selection. +On the same fixture and the same real embedding model, 3 of 5 candidates are now +suppressed as repetitions and the clue is retrieved at `top_k` 5, 4 **and** 3. + +Ranking itself remains cosine similarity plus an explicit pin; the further +factors `CONTEXT-AND-MEMORY.md` §20 contemplates are not implemented and are +recorded there as future work. + --- ## F03 — Prompt Remains Bounded @@ -986,6 +1058,13 @@ Generate a long story. ### Pass Application does not continually append full transcript until context overflows. + +### Result — PASS (M6, 2026-09-05) +`test_f03_the_prompt_stays_bounded_as_the_story_grows` and +`test_the_context_size_stops_growing_once_the_budget_is_reached`. The second +measures from a story that already fills the budget, then triples it: the +prompt does not move, while the action count does. + --- ## F04 — Output Token Reserve @@ -995,6 +1074,14 @@ Application does not continually append full transcript until context overflows. ### Pass Context builder leaves sufficient room for narrator output and does not regularly fail because input consumes entire context. + +### Result — PASS (M6, 2026-09-05) +The reply is reserved out of the context budget before history is selected, and +the report exposes it (`tokens.output_reserve`). Three tests: the reserve +survives a long story; an impossible budget raises `ContextOverflow` naming both +figures; and that refusal reaches the reader as a failed turn without disturbing +the stored story. Nothing was reserved before M6. + --- ## F05 — Prompt Inspector @@ -1017,6 +1104,18 @@ User can determine at least: Exact UI may vary. + +### Result — PARTIAL, complete for the components M6 owns (2026-09-05) +`test_f05_the_inspector_shows_every_component_m6_owns` asserts the report +carries narrator rules, authoritative state, the summary in use with its source +coverage, retrieved memories with authority and provenance, recent history, the +reader's input, model and settings, and per-component token costs alongside the +budget, the reply reserve and the history allowance. All of this is rendered in +the Insights panel and was verified in a real browser. + +"Retrieved knowledge" is M7's imported-document section and is not implemented; +nothing was built to fill it. + --- ## F06 — Retrieval Provenance @@ -1026,6 +1125,13 @@ Exact UI may vary. ### Pass A retrieved memory or imported chunk can be traced to its source record/file. + +### Result — PASS for story memory (M6, 2026-09-05) +`test_f06_a_retrieved_memory_is_traceable_to_its_source`: every retrieved memory +carries `branch_id`, `depth` and its source range, and the test resolves that +coordinate back to a real action of the campaign's accepted history. Imported +chunks are M7's half of this criterion and are not implemented. + --- ## F07 — Heuristic Memory Is Not Canon @@ -1048,6 +1154,15 @@ Mara is definitely working against Captain Vale. as authoritative fact. + +### Result — PASS (M6, 2026-09-05) +`Memory.authority` is `accepted_story` or `heuristic`, classified by the +application rather than the model. The prompt marks an inference `[inferred]` +and says such lines are not established fact. +`test_f07_a_heuristic_memory_is_labelled_and_is_not_state` also asserts the +inference did not become an authoritative fact: retrieval never writes state, +and the M5 typed-event path remains the only route to one. + --- ## F08 — Memory Failure Is Non-Fatal @@ -1060,6 +1175,14 @@ Cause embedding/memory extraction failure if test harness supports it. ### Pass Accepted turn persists and story can continue; derived memory may be retried later. + +### Result — PASS (M6, 2026-09-05) +With the summariser and embedder both raising, the accepted narration, the +authoritative state, the head and the transcript all survive, the next turn +still plays, and the failure is recorded per kind in `derived_status` and served +by `GET /adventures/{id}/derived`. A later healthy run clears it. This is the +M2 failure — the whole memory bank dead with a green suite — made visible. + --- # G. Imported Knowledge diff --git a/planning/VERSION.md b/planning/VERSION.md index a2675ef..db94031 100644 --- a/planning/VERSION.md +++ b/planning/VERSION.md @@ -1,8 +1,51 @@ # Planning Package Version -- **Package:** Adventure Storyteller Planning Package v2.6 -- **Revision date:** 2026-09-03 -- **Status:** Phase 0 complete; architecture selected; **Milestones M1-M4 implemented and accepted**; M5 is next to brief. +- **Package:** Adventure Storyteller Planning Package v2.7 +- **Revision date:** 2026-09-06 +- **Status:** Phase 0 complete; architecture selected; **Milestones M1-M6 implemented and accepted**; M7 is next to brief. + +## v2.7 — M5 and M6 Closeout (2026-09-06) + +Two milestones, and one lesson they share: an independent review found a real +defect in each after the implementation reported success, and in both cases the +defect was invisible to the tests the implementation had written for itself. + +**M5 — Genre-Neutral Authoritative Narrative State.** Accepted 2026-09-04. Its +review returned *PASS WITH CORRECTIVE WORK REQUIRED*: editing a narrator turn +rewound the campaign's live state while the head stayed at the tip, breaking the +`transcript position == head == authoritative state` invariant. The corrective +pass rebuilt narrator editing on the §§14-15 fork semantics. Recorded here late: +M5's closeout did not add a package revision entry, and this one covers it. + +**M6 — Branch-Safe Context, Summaries, and Long-Term Story Memory.** Accepted +2026-09-06. Sequence: + +1. **Implementation.** Summaries moved onto lineage-anchored rows; memory + authority, provenance, an output reserve, and observable derived-work + failure were added. Reported as passing, including E03. +2. **Independent review — E03 still failed.** Summary *rows* were anchored, but + generation was seeded from `adventures.story_summary`, a campaign-global + column with no lineage. After a divergence the summariser was handed the + abandoned line's prose and asked to update it, so the new summary carried + abandoned content inside a correctly anchored row. The review also found F02 + passing by a single retrieval slot, two test defects, and a misleading + derived-work status. +3. **Corrective pass.** Generation is seeded from `summaries.current`; + redundancy suppression was added before the retrieval cut; the test fixture + no longer makes real network calls; the browser suite gained a scenario that + regenerates a summary after divergence. + +**What the planning package learned from M6, recorded in the active documents:** + +- `CONTEXT-AND-MEMORY.md` §11 — lineage safety has two halves. Anchoring the + output row is not enough; the *input* to the summarizer must be scoped by the + same rule. +- `V1-ACCEPTANCE-TESTS.md` E03 — a valid test must regenerate a summary after + diverging. Checking only that the old row went ineligible passes while the + defect is live. +- `CONTEXT-AND-MEMORY.md` §20/§22 — what M6 implements (redundancy suppression + between memories) is now distinguished from what it does not (importance, + entity, recency and thread ranking; cross-layer duplication). ## v2.6 — M4 Closeout (2026-09-03) diff --git a/planning/reports/M5-IMPLEMENTATION-REPORT.md b/planning/archive/milestone-reports/M5-IMPLEMENTATION-REPORT.md similarity index 100% rename from planning/reports/M5-IMPLEMENTATION-REPORT.md rename to planning/archive/milestone-reports/M5-IMPLEMENTATION-REPORT.md diff --git a/planning/reports/M6-IMPLEMENTATION-REPORT.md b/planning/reports/M6-IMPLEMENTATION-REPORT.md new file mode 100644 index 0000000..9a78078 --- /dev/null +++ b/planning/reports/M6-IMPLEMENTATION-REPORT.md @@ -0,0 +1,1060 @@ +# M6 Implementation Review — Branch-Safe Context, Summaries, and Long-Term Story Memory + +**Review date:** 2026-09-05 +**Reviewer:** independent review pass (the M6 build summary was treated as claims to verify) +**Tree reviewed:** the **staged working tree** on `m6-context-memory`. There are no M6 commits. + +--- + +## A. Executive result + +**PASS WITH CORRECTIVE WORK REQUIRED** + +M6 delivers most of what it promises, and the parts that work are demonstrated +rather than asserted: memory lineage safety, background-failure observability, +context budgeting, provenance, authority, migration and restart all hold up +under independent testing with real local models. + +One blocking defect: **abandoned summary content still reaches the active +prompt** (E03). The row-level eligibility M6 built is correct, but the +summariser seeds each new summary from a campaign-global column that has no +lineage, so a summary generated after a divergence inherits the abandoned +line's prose. This is one of the failure modes the milestone explicitly names. + +--- + +## B. Scope and review method + +Evidence was gathered in the order the brief prescribes — observed end-to-end +behaviour first, source inspection last. Specifically: + +- **Real local models** for the summariser, the memory extractor and the + embedder, through the production provider path, for the E-series, F02 and the + summary-generation checks. +- **Deterministic reproductions** where a mechanism needed isolating. +- **A genuine OS process boundary** for the restart check (two processes, two + PIDs, one database file). +- **A real socket failure** (endpoint pointed at a dead loopback port) for the + observability gate, rather than patched application functions. +- **A real browser** (Firefox 154.0.1 over WebDriver) against the built frontend. +- **A clean export of the M5 base commit** to establish the baseline by + measurement rather than from the previous report's claim. + +Every negative result below is preceded by a positive control. + +The M6 non-scope was preserved: M7 imported knowledge is absent and is not +scored as an M6 defect; no remote embedding, vector database or cloud path was +introduced; Story Cards were not promoted into a knowledge subsystem. + +--- + +## C. Repository and provenance state + +``` +branch m6-context-memory +HEAD b7005e6fddec337f940749455739a60f0c6cb54f +HEAD signature Good signature (RSA 7D8AE19DB5C68569), status G +M6 base commit b7005e6 — the accepted M5 closeout +M6 commits NONE +staged 29 files changed, 2973 insertions(+), 48 deletions(-) +unstaged 0 +untracked 0 +working tree == index yes +upstream ancestry d72f7c1b is an ancestor of HEAD +LICENSE unchanged (not in the change set) +``` + +**All M6 work is uncommitted.** `HEAD` is still the M5 closeout, so there is no +M6 commit to sign or verify. Every test result in this report was produced from +the **staged working tree**, which is byte-identical to the index. + +A commit message is prepared at `.git/M6_MSG` (untracked, outside the work tree). + +--- + +## D. Change inventory + +**Added (7)** + +``` +backend/app/summaries.py lineage-anchored summary store +backend/app/derived.py per-campaign derived-work status +backend/tests/test_context_memory.py F01-F08, E02/E03, authority (20 tests) +backend/tests/test_provider_wiring.py real provider construction (10 tests) +backend/tests/test_context_performance.py query growth, context size (5 tests) +backend/tests/test_context_realistic.py real-model context (3 tests) +planning/reports/M6-IMPLEMENTATION-REPORT.md this document +``` + +**Modified (21)** — `context/builder.py`, `context/__init__.py`, `memorybank.py`, +`migrations.py`, `models.py`, `routers/adventures/{crud,insights,turns}.py`, six +existing test files, `InsightsPanel.jsx`, `play.css`, and five planning +documents. + +**Renamed (1)** — `planning/reports/M5-IMPLEMENTATION-REPORT.md` → +`planning/archive/milestone-reports/`. Verified a pure rename: `git diff -M` +reports 0 insertions and 0 deletions, so the archived M5 report is unmodified. + +**Schema** — two new tables (`summaries`, `derived_status`), one new column +(`memories.authority`), three migrations (89 column, 90 index, 91 data pass). +No new dependency. No `requirements.txt` or `package.json` change. + +--- + +## E. Implemented architecture + +``` +turn accepted ─┬─ authoritative state (M5, unchanged) + └─ schedule_post_turn ── fire-and-forget task + ├─ _guarded(memory) ─ summarise blocks → memories + ├─ _guarded(summary) ─ roll up → summaries row + └─ _guarded(embedding) ─ embed pending memories + each records ok/failed to derived_status + +build_context + protected = system + canon + state + summary + memories + input + reserve = max_output_tokens + 64 + history = newest-first until (budget − protected − reserve) is spent + raises ContextOverflow when protected + reserve >= budget + +eligibility, for both memories and summaries: + lineage.path_of(db, adventure).clause(Model) # head-capped +``` + +The single most important architectural fact is that summaries were moved onto +the *same* eligibility chokepoint memories already used, rather than getting a +mechanism of their own. That is the right shape, and it is why Undo, Redo, Save +Point restore and divergence need no per-feature rules. The defect in §H is not +in that mechanism; it is in what the summariser is fed. + +--- + +## F. F01–F08 acceptance results + +| ID | Result | Evidence | +|---|---|---| +| **F01** Recent turns remain coherent | **PASS** | Assembled prompt contains the preceding narration and the reader's input in order, after ordinary play and after Undo/Redo. Read from the context report, not the prose. | +| **F02** Old important event retrieval | **PASS (fragile)** | At the production default (`memory_top_k=5`), all five conditions hold — see §F02 below. At `memory_top_k=4` the same fixture **fails**. | +| **F03** Prompt remains bounded | **PASS** | 51 → 231 actions: input tokens identical at 5,342, `included` capped at 38–40. Measured through the builder, not by reading a constant. | +| **F04** Output token reserve | **PASS** | Reserve = `max_output_tokens + 64`, subtracted before history selection. `input + reserve <= budget` at every size measured. An impossible budget raises `ContextOverflow` naming both figures. | +| **F05** Prompt inspector | **PASS for the components M6 owns** | All M6-owned components inspectable in API and browser (§K). M7 imported knowledge absent and not simulated. | +| **F06** Retrieval provenance | **PASS for story memory** | Every retrieved memory carries `branch_id`, `depth`, `source_start/end`; the coordinate resolves to a real action row (verified: action id 6). | +| **F07** Heuristic memory is not canon | **PASS** | Classified by the application, marked `[inferred]` in the prompt, exposed in the API, and produced no authoritative fact. | +| **F08** Memory failure is non-fatal | **PASS** | Real socket failure; story, state, head intact; failure recorded, served, cleared on recovery. | + +### F02 in detail — the fragility + +Real embedder (`nomic-embed-text`) and real memory extractor +(`qwen2.5:3b-instruct`). One distinctive clue planted early, 26 long filler +turns played over it, then a query naming the clue's subject. + +``` +story 57 actions, 14 included in the prompt +1. absent from verbatim history yes +2. present in the memories section yes +3. recovered in the prompt yes +4. not because the transcript was sent yes (14 of 57) +5. provenance resolves to a real action yes (b1 d5, src 0-5, action 6) + authority accepted_story +``` + +**But** the recovering memory ranked **last of five**: + +``` +sim 0.8157 The party walks the muddy road north, talking of weather… +sim 0.8132 The party walks the muddy road north, talking of weather… +sim 0.8114 The party walks the muddy road north, talking of weather… +sim 0.7984 The party walks the muddy road north, talking of weather… +sim 0.61 Aldric found a brass key carved with a heron under the third flagstone +``` + +Four near-identical filler memories outrank the one memory that matters. At +`memory_top_k=4` the clue is evicted and F02 fails outright; the production +default of 5 is what saves it. Ranking is pure cosine similarity plus a pinned +flag — none of the other factors `CONTEXT-AND-MEMORY.md` §20 lists (importance, +entity overlap, recency, thread overlap) is implemented, and §22's duplicate +suppression is not implemented at all. See finding **M6-F2**. + +--- + +## G. Memory lineage safety (E02) + +**PASS, end to end, with positive controls.** Real embeddings; sentinel +`ABANDONED-MARA-SPY-7421`. + +``` +positive control 5 memories retrieved on path A yes + sentinel inside a retrieved memory yes + sentinel in the active prompt yes +negative undo below the sentinel, diverge, play 9 turns + sentinel in the active prompt on path B NO +retention rows still on disk 7 memories (was 5) +re-embedding memories awaiting embedding after return to lineage 0 +``` + +Ineligibility is not deletion: the abandoned memories remain stored, and the +derived endpoint lists retained-but-ineligible summaries explicitly. The +deterministic Undo → Redo control (no divergence) shows eligibility returning +without re-embedding. + +A note on one control: after a *divergence*, Redo cannot return to the +superseded line — that is correct M3 divergence semantics, not a memory defect, +and the return-to-lineage control is therefore only meaningful for Undo/Redo +without divergence, where it passes. + +--- + +## H. Summary lineage safety (E03) — **FAILS** + +**Row-level eligibility is correct.** A summary carries `(branch_id, depth)` and +`source_start/source_end`; `summaries.current` selects the newest row on the +head-capped lineage; abandoned rows are retained and reported as ineligible. +Verified directly: after divergence the path-A summary row was ineligible and +still on disk (1 eligible, 2 retained-but-ineligible). + +**Content propagation is not.** `_update_story_summary` seeds the model with: + +```python +current = adventure.story_summary.strip() # memorybank.py, in _update_story_summary +``` + +`adventures.story_summary` is a campaign-global mirror that `summaries.record` +rewrites on every write, regardless of which line produced it. After a +divergence the summariser is therefore handed the abandoned line's prose and +asked to update it, and the resulting row — correctly anchored to the *new* +branch — carries the abandoned content forward. + +**Real-model evidence.** Sentinel `ABANDONED-CHAPEL-OATH-9930`, established only +on path A: + +``` +path A summary row id 2, branch 1, depth 30, source 15–30 sentinel present +undo to head (1,0); diverge; play 9 turns; run the derived pass +path B summary row id 3, branch 2, depth 16, source 1–16 + >>> sentinel IS in the active prompt on path B +``` + +**Deterministic reproduction**, with the memory extractor emitting the sentinel +only for blocks that actually contain it: + +``` +path B summary section: + "- MEM[ABANDONED-CHAPEL-OATH-9930] - MEM[ABANDONED-CHAPEL-OATH-9930] + - MEM[ABANDONED-CHAPEL-OATH-9930] - MEM[ABANDONED-CHAPEL-OATH-9930] + - MEM[ABANDONED-CHAPEL-OATH-9930] - MEM[dry road]" + +sentinel in the ACTIVE PROMPT on path B True +summariser seed contained the sentinel True +path-A actions on path B's lineage 0 +``` + +Zero path-A story is on path B's lineage, and the path-B events contributed only +`MEM[dry road]` — so the sentinel can only have arrived through the seed. + +The brief's question *"whether invalid later coverage is rebuilt rather than +silently reused"* is answered: the **row** is rebuilt, the **content** is reused. + +Why the existing tests missed it: `test_e03_an_abandoned_summary_is_retained_but_never_used` +and browser check 10b both verify that the *old row* is not used, and neither +regenerates a summary after the divergence. The blind spot is shared by the unit +test and the browser check. + +--- + +## I. Memory authority and provenance + +**PASS.** Two classes, both load-bearing rather than cosmetic. + +``` +schema memories.authority "accepted_story" | "heuristic" +creation memorybank.classify_authority — the application decides, from + hedging in the memory's own text; a model label is not trusted +retrieval returned in the same query as the text (no extra round trip) +prompt "Lines marked [inferred] are interpretation, not established + fact — do not treat them as settled truth" + - Mara handed Captain Vale the sealed letter. + - [inferred] Mara seemed nervous around Captain Vale. +inspector authority + source coordinate per memory, in API and browser +``` + +Verified that a heuristic memory produced **no** authoritative fact: after +retrieval the state document's `facts` list was empty and no Vale-related +assertion existed. Retrieval has no write path to state; the M5 typed-event +pipeline remains the only route (ADR 013). + +**Provenance is real, not a similarity score.** The returned coordinate resolves +to an actual action row, checked by query. + +**Authority conflict (§13).** With a memory asserting a fact the reader had +since withdrawn by manual correction: + +``` +memory still retrieved saying she knows it yes +state section says "No longer true" yes +withdrawn fact absent from the facts that stand yes +``` + +The authoritative correction is stated in the same prompt and outranks the +memory. Correct, though the contradicting memory is still shown unqualified — +the correction wins by being explicit, not by suppressing the memory. + +--- + +## J. Context budgeting and bounded prompt + +**PASS, measured.** + +``` +actions included input protected reserve budget input+reserve<=budget + 11 11 808 756 564 6000 yes + 71 38 5208 762 564 6000 yes + 251 38 5208 762 564 6000 yes +``` + +The prompt stops growing once the budget is reached: 251 actions produce exactly +the same 5,208 tokens as 71. On a realistic prompt (real model, 83 actions, +8,192 budget): input 7,315 + reserve 764 = 8,079, inside budget, 41 of 83 +actions included. + +Protected content is never dropped for older prose, and an impossible +configuration raises `ContextOverflow` naming both figures rather than building +a prompt known to overflow — a behaviour change from M5, where nothing was +reserved at all. + +--- + +## K. Prompt inspector + +**PASS for the components M6 owns**, verified in a real browser rather than by +reading JSX. + +| Component | API | Browser | +|---|---|---| +| narrator / system rules | `sections[narrator]` | shown | +| current authoritative state | `sections[narrative_state]` | shown | +| summary used + source coverage | `summary{branch_id,depth,source_start,source_end,trigger,model}` | "summary in use · covers …" | +| retrieved memories | `memories.used[]` + `considered` | listed | +| memory authority | `used[].authority` | "inferred" / "from the story" | +| memory provenance | `used[].source{}` | "turn N" | +| recent history | `sections[history]`, `history.included/total` | shown | +| user input | inside history | shown | +| model / settings | `settings{}` | shown | +| token cost per component | `sections[].tokens` | per-section, with % | +| budget / reserve | `tokens{budget,output_reserve,protected,available_for_history}` | "… reserved for the reply" | +| derived-work health | `derived[]` | warning banner | + +M7 imported knowledge is **absent**, not stubbed. That is the correct state for +M6 and is not scored as a defect. + +--- + +## L. Background failure and observability — the M2 gate + +**PASS.** This was tested by pointing the derived endpoint at a loopback port +with nothing listening, so the failure happens inside the production +fire-and-forget path at the socket, not by patching application functions. + +``` +1. accepted story intact actions 43 → 43 + head intact (1,42) → (1,42) + authoritative state intact unchanged +2. observable through the API GET /derived → 200 + failing: ["memory", "summary"] + memory failed x17 ProviderError: Request to AI endpoint failed… + summary failed x16 ProviderError: Request to AI endpoint failed… +3. visible in the prompt inspector context report carries "derived" +4. no half-written derived data 0 memories, 0 summaries +5. story continues next turn landed +6. recovery clears it failing: [] after one healthy pass +``` + +The failure counts (x17, x16) accumulated from the **real background tasks** the +turns scheduled, which is itself evidence that the production fire-and-forget +path records rather than swallows. A structured log entry is also emitted +(`log.exception` in `derived.failed`). + +This satisfies the M2 requirement that a dead memory bank be discoverable. It is +not a test-only capture: the same record is served by the API and rendered in the +browser. + +--- + +## M. Real provider construction and realistic context + +**PASS, and not by mock.** + +`tests/test_provider_wiring.py` builds a real `Settings` row, round-trips it +through the database, and calls the real factories — no factory is patched. A +parametrised test names each `Settings` attribute the providers and builder +read, so a renamed column fails by name rather than killing the memory bank in +an unobserved task. Construction is proven inert by refusing `socket.connect`. + +Beyond that, this review ran the **whole derived pipeline against real models**: + +``` +endpoint trusted-LAN, HTTPS, through the ordinary endpoint policy +narrator/summary model qwen2.5:3b-instruct (warm; resident between runs) +embedding model nomic-embed-text +result 5 real memories written, 2 real summaries written, + real vectors, derived_status {embedding: ok, memory: ok, summary: ok} + retrieval returned those memories and they reached the prompt +``` + +That exercises construction → request → response → derived record → retrieval → +use, which is the chain the brief asks for. Secrets are not recorded here; the +endpoint host is deliberately omitted. + +The project's own realistic-context tests (3) pass against the same endpoint. + +--- + +## N. Undo / Redo / Save Point / divergence matrix + +| Transition | Head before → after | Eligible memories | Eligible summary | Retained on disk | +|---|---|---|---|---| +| normal play | (1,42) | 5 | row 2 (b1 d30, src 15–30) | — | +| Undo below the source | (1,42) → (1,0) | sentinel excluded | excluded | all rows kept | +| Redo (no divergence) | restored | sentinel eligible again, **0** re-embeddings | eligible again | all rows kept | +| Save Point restore | (1,6) → (1,2) | filtered to the restored depth | row at d4 becomes ineligible | kept | +| divergence | (1,0) → (2,18) | sentinel absent from prompt | old row ineligible; **new row carries abandoned content (§H)** | 1 eligible, 2 retained-ineligible | + +E01/E04 regression: after Undo and divergence, a fact established on the +abandoned line was absent from both the authoritative document and the prompt — +M5 behaviour is not regressed by the new context selection. + +--- + +## O. Persistence, restart and migration + +**Restart — PASS**, across a genuine OS process boundary (two PIDs, one file): + +``` + build (pid 976700) after restart (pid 976753) +actions 43 43 +head (1, 42) (1, 42) +memories / summaries 7 / 2 7 / 2 +sentinel in prompt yes yes +summary provenance id 2, b1 d30, src 15–30 identical, same created_at +memory provenance — resolves (b1 d11 src 6–11, b1 d17 src 12–17) +derived status — embedding=ok memory=ok summary=ok +``` + +**Migration — PASS.** A genuine pre-M6 database was built (M6 tables dropped, +`authority` column removed, stamp rewound to 88) and migrated: + +``` +stamp 88 → 91 +summaries rows 1 (the old story_summary, backfilled) +backfilled anchor b1, depth 4, source_end 4, trigger "interval" + — i.e. at the campaign's summary cursor, correctly +memories.authority all rows defaulted to "accepted_story" +campaign opens 200, 7 actions, summary eligible in the prompt +undo / redo 200 / 200 +Save Point restore to d2 200; the depth-4 summary correctly becomes INELIGIBLE +new M6 turn 200, forks to branch 2 (correct post-restore divergence) +derived endpoint 200 +``` + +Old rows behave correctly, not merely "migration completed" — including the case +the M5 defect warned about, where a restored earlier position must not keep a +later position's derived data. + +--- + +## P. Browser verification + +Firefox 154.0.1, headless, WebDriver, against the built frontend served by the +real backend on a fresh database. **16/16 passed**, reproduced independently in +this review. + +Checks covered: normal generation; recent coherence; inspector opens; summary in +use with coverage; memories listed; authority legible; provenance names the +turn; token costs and reply reserve; Undo changes eligibility; Redo restores it; +divergence reintroduces no abandoned memory; no abandoned summary row; abandoned +memories retained not deleted; a background failure discoverable with its +message; the "story itself is unaffected" note; no console errors. + +**Caveat.** Browser check 10b ("divergence reintroduces no abandoned summary") +passes because the harness never regenerates a summary on the new line. It +therefore shares the blind spot described in §H and should not be read as +independent confirmation that E03 holds. + +--- + +## Q. Local-only and security regression + +**PASS. No new network path.** + +``` +_stream re-checks endpoints.rejection_reason before the request yes +complete re-checks endpoints.rejection_reason before the request yes +embed re-checks endpoints.rejection_reason before the request yes +policy refuses https://api.openai.com/v1/embeddings yes +policy allows http://127.0.0.1:11434/v1/embeddings yes +``` + +Embeddings, summarisation and memory extraction all go through the same +`OpenAICompatibleProvider`; M6 added no HTTP client of its own. A source scan +for cloud inference hosts, remote vector databases and cloud embedding services +finds none outside `endpoints.py`, which names them in order to refuse them. +Loopback binding, allowlist semantics and TLS verification are untouched. +`tests/test_local_only_surface.py` passes. + +Consistent with `SECURITY-THREAT-MODEL.md` §38 (local embedding models, no +auto-download) and ADR 011. + +--- + +## R. Performance and scaling + +**PASS — no growth at all in the paths measured.** + +``` + 51 actions, 8 memories, 3 summaries → 20 queries, 5342 input tokens +231 actions, 38 memories, 14 summaries → 20 queries, 5342 input tokens +``` + +Query count did not move while the story grew 180 actions, memories grew 30 and +summaries grew 11. No N+1 on memory provenance, summary selection or history. +The `GET /derived` listing resolves the eligible summary once rather than per +row. Undo/Redo triggered **0** re-embeddings; returning to a lineage reuses the +stored vectors. + +Retrieval candidate sets are bounded by the lineage clause and `memory_top_k`; +vectors are fetched only for the chosen rows. + +--- + +## S. Earlier-milestone regression check + +Focused coverage over the interfaces M6 touches: **281 passed** across +`test_narrative_state`, `test_head_cursor`, `test_save_points`, +`test_memory_nodes`, `test_memory_retrieval`, `test_egress`, `test_bundle_v2`, +`test_local_only_surface`. + +Full gates, on the staged tree: + +``` +backend suite 835 passed, 7 skipped, 2 warnings (261s) +M5 baseline 794 passed, 3 skipped, 1 warning (measured from a clean + export of b7005e6; the export shows 793/4 because it has no + built SPA, which skips one offline-assets test) +skips 7 = 3 M5 realistic + 3 M6 realistic + 1 live provider wiring; + all endpoint-gated, none reported as PASS +frontend lint exit 0, 7 warnings — identical count at the M5 baseline, + demonstrated by running lint on the base export +frontend build success +docker build success +``` + +The one **new** warning is analysed in finding M6-F3. + +--- + +## T. Findings + +| ID | Severity | Title | Blocks M6? | Owner | +|---|---|---|---|---| +| **M6-F1** | **high** | Abandoned summary content leaks into the active prompt via the un-anchored mirror column | **yes** | M6 corrective | +| **M6-F2** | medium | No duplicate suppression; ranking is similarity-only, so F02's margin is one memory wide | no | M6 corrective (small) or M7 | +| **M6-F3** | low | M6 test fixture leaves the summariser on the default endpoint; suite attempts real localhost connections | no | M6 corrective (test-only) | +| **M6-F4** | low | The E03 unit test and browser check share a blind spot: neither regenerates a summary after divergence | no | M6 corrective (test-only) | +| **M6-F5** | trivial | `derived_status` reports `embedding = ok` for a pass that had nothing to do | no | documentation / M8 | + +### M6-F1 — abandoned summary content leaks (HIGH, blocking) + +**Requirement violated.** `CONTEXT-AND-MEMORY.md` §11 ("A summary from abandoned +history must never leak into active context"); acceptance test E03; +`BUILD-MILESTONES.md` M6 scope ("summaries anchored to source turn +ranges/lineage"). Named explicitly in the review brief as an M6 issue. + +**Evidence.** §H above: real-model run (sentinel in the path-B prompt) and a +deterministic reproduction isolating the route, with zero path-A actions on path +B's lineage. + +**Cause.** `_update_story_summary` seeds the model from +`adventure.story_summary`, a campaign-global column with no lineage, which +`summaries.record` rewrites on every write. + +**User impact.** After undoing and taking a different path, the narrator is told +about events from a story the reader abandoned — the exact confusion the +milestone exists to prevent. It is silent: the summary row's provenance looks +correct, because the row *is* correctly anchored. + +**Recommended direction** (not implemented in this review): seed the summariser +from `summaries.current(db, adventure)` — the eligible row — rather than from +the mirror column, so a divergence starts from the last summary that is actually +valid for the new line, or from nothing when none is. + +### M6-F2 — no duplicate suppression; similarity-only ranking (MEDIUM) + +**Requirement.** `CONTEXT-AND-MEMORY.md` §22 requires that the same fact not be +supplied repeatedly and that the builder "prefer the highest-authority concise +representation"; §20 lists ranking factors beyond similarity. + +**Evidence.** Two identical memories were both retrieved and both rendered +("duplicate suppression applied: False"). In the F02 fixture, four near-identical +filler memories (0.79–0.82) outranked the one distinctive clue (0.61), which +survived only because the default `memory_top_k` is 5 and it placed fifth. At 4 +it is evicted and F02 fails. + +**User impact.** On a long story with repetitive stretches, the memory budget can +be consumed by redundant memories and the one that matters dropped. M7 will make +this worse by adding imported material competing for the same budget. + +Ranking is `cosine + pinned` only; no importance, entity overlap, recency or +thread overlap. + +### M6-F3 — test fixture makes real network attempts (LOW) + +`tests/test_context_memory.py` stubs `embedding_provider` but not +`summary_provider`, with `auto_summarize=True`, so the post-turn task attempts a +real connection to the default endpoint during the suite. This is the source of +the one new warning (`RuntimeWarning: coroutine 'connect_tcp…try_connect' was +never awaited`) — an abandoned httpx connection coroutine when the TestClient +event loop closes. Test hygiene, not a product defect; the production loop does +not close mid-task. + +### M6-F4 — the E03 tests cannot see the E03 defect (LOW) + +Both the unit test and browser check 10b verify only that the *old row* is +ineligible. Neither regenerates a summary after diverging, which is where the +content leak occurs. Any corrective work should add a test that plays past the +summary interval **on the new line** and asserts the sentinel's absence. + +### M6-F5 — misleading "ok" for a no-op pass (TRIVIAL) + +When there is nothing to embed, `_embed_pending` does nothing and the kind is +recorded `ok` with a fresh `last_success_at`. Harmless, but a reader could read +it as "embedding is working" when it has never run. + +--- + +## U. Non-blocking debt + +- **M7:** imported knowledge absent, as intended; the inspector section was left + unbuilt rather than stubbed. +- **M8:** the Insights additions are functional, not designed. +- **M9:** derived rows are not carried in an export bundle, so an imported + campaign starts with no summaries or memories and rebuilds them. Authoritative + history is unaffected. Not tested here beyond noting it. +- **M11:** long-context evidence is a bounded fixture (up to 251 actions), not + the M01 100-turn release campaign. + +--- + +## V. Planning-document recommendations + +No active planning document was edited by this review. + +| Document | Recommendation | Why | +|---|---|---| +| `SPECIFICATION.md` | **NO CHANGE** | M6 implements it; nothing contradicted. | +| `CONTEXT-AND-MEMORY.md` | **CHANGE** | Two reasons. §22 (duplicate suppression) and the non-similarity half of §20 are **not implemented** — the plan is not wrong, the implementation is incomplete, and the gap should be recorded rather than the requirement softened. Separately, the "As implemented (M6)" note added under §11 currently claims lineage safety that finding M6-F1 shows is not yet achieved end to end; it should not stand unqualified until M6-F1 is fixed. | +| `STORY-BRANCH-SEMANTICS.md` | **NO CHANGE** | M6 consumes §32/§33 lineage rules; it does not change them. | +| `DATA-MODEL.md` | **NO CHANGE** | §16A accurately records the implemented tables. Add the mirror-column relationship only if M6-F1 is fixed by changing it. | +| `TECHNICAL-DESIGN.md` | **CHANGE (small)** | The M6 section should state that the summariser's *input* is lineage-scoped, once it is — that is the invariant M6-F1 breaks, and it is currently unstated. | +| `BUILD-MILESTONES.md` | **CHANGE at closeout** | Correctly says "implemented, awaiting review". Should record the E03 corrective work and the duplicate-suppression debt when M6 closes out. | +| `V1-ACCEPTANCE-TESTS.md` | **CHANGE** | The E03 result note currently reads PASS. On this evidence it is not yet satisfied end to end and should say so until corrected. F02's note should record the ranking fragility. Do not weaken any pass condition. | +| `SECURITY-THREAT-MODEL.md` | **NO CHANGE** | M6 added no network path; §38 and ADR 011 hold. | +| `README.md` / `DEVELOPMENT.md` | **NO CHANGE** | No user-facing or developer-workflow fact changed. | +| `VERSION.md` | **NO CHANGE** | No new decision was recorded; see the ADR row. | +| **ADRs** | **NO NEW ADR** | Every M6 choice implements a decision already settled — summary/source association (§11), authority and application-owned classification (§14–15), output reserve (§32), the M2 observability requirement. An ADR would record no open decision. If the M6-F1 fix changes what `adventures.story_summary` *is*, that would be worth an ADR. | + +--- + +## W. Implications for M7 + +M7 builds the imported Canon / Reference / Inspiration library on this +infrastructure. What it inherits: + +**Ready to build on:** + +- **A single eligibility chokepoint.** `lineage.Path.clause` filters actions, + memories and summaries alike. M7's imported knowledge is deliberately *not* + lineage-scoped, and the builder now has one obvious place to express that. +- **Provenance and per-component token cost are first-class** in the context + report and the inspector, so extending F06 to files and chunks is an addition, + not a redesign. +- **An explicit protected/elastic budget with a reply reserve**, giving imported + reference and inspiration material a defined place as low-priority elastic + content (`CONTEXT-AND-MEMORY.md` §31). +- **Authority is already a working concept**, with application-owned + classification and prompt-level marking that M7's Canon/Reference/Inspiration + tiers can extend rather than reinvent. +- **Derived-work failure is observable**, so import and index failures can reuse + `derived_status` instead of inventing another mechanism. + +**M7 must not build on:** + +- **M6-F1.** The summariser's input is not lineage-scoped. M7 will add more + derived text to the same prompt; layering imported knowledge on top of a + summary that can carry abandoned content would make the resulting context + harder to reason about and harder to debug. Fix before M7 starts. +- **M6-F2.** Imported chunks will compete for prompt budget with story memories. + Without duplicate suppression or importance weighting, adding a second corpus + to a similarity-only ranker will make the F02 fragility materially worse. + +--- + +## X. Verdict + +**PASS WITH CORRECTIVE WORK REQUIRED** + +| Question | Answer | +|---|---| +| Is M6's Definition of Done met? | **Not fully.** "Lineage-safe" fails for summary content (M6-F1); authority-aware, local, inspectable and bounded are all met. | +| Are F01–F08 satisfied at the level M6 owns? | **Seven of eight.** F02 passes at the production default but is fragile (M6-F2). F05 is complete for M6-owned components. | +| Is E02 demonstrated end-to-end? | **Yes**, with real embeddings and positive controls. | +| Is E03 demonstrated end-to-end? | **Yes — and it fails.** | +| Can abandoned memory appear in an active prompt? | **No.** | +| Can abandoned summary content appear in an active prompt? | **Yes.** M6-F1. | +| Is heuristic memory visibly non-authoritative? | **Yes**, in the prompt, the API and the browser; it creates no state. | +| Is prompt growth actually bounded? | **Yes**, measured: 251 actions produce the same tokens as 71. | +| Is output reserve enforced? | **Yes**, and an impossible budget fails clearly instead of overflowing. | +| Are memory/summary failures non-fatal? | **Yes.** Story, state and head all survive; no half-written derived data. | +| Are background failures observable? | **Yes.** API, inspector, browser and log, from a real socket failure. | +| Was a real provider-construction path exercised? | **Yes**, unmocked, plus the full derived pipeline against real local models. | +| Did M6 preserve the local-only boundary? | **Yes.** No new client, no new host, policy enforced on all three paths. | +| Did any M1–M5 behaviour regress? | **No.** 281 focused regression tests pass; full suite green; lint warnings identical to baseline. | +| Should the project proceed to M7? | **Not yet.** Fix M6-F1 first. | +| What must be corrected before M7? | **M6-F1** (blocking). **M6-F2** strongly recommended, since M7 makes it worse. M6-F3 and M6-F4 are cheap and should ride along. | + +--- + +## Y. Evidence appendix + +| Check | Method | Result | +|---|---|---| +| Backend suite | `pytest tests/ -q`, staged tree | 835 passed, 7 skipped | +| M5 baseline | `git archive b7005e6` → clean export → `pytest` | 794 passed, 3 skipped (793/4 in the export, +1 skip for the unbuilt SPA) | +| Frontend lint / build | `npm run lint`, `npm run build` | exit 0 (7 warnings, same as baseline); build ok | +| Container | `docker build` | success | +| E02 | real embeddings, sentinel, positive control then divergence | sentinel absent on path B; rows retained | +| E03 | real models, then deterministic isolation | **sentinel present on path B** | +| F02 | real embed + extractor, 57 actions, 14 included | passes at `top_k=5`; clue ranks 5/5 | +| F03/F04 | builder measurements at 11 / 71 / 251 actions | 5,208 tokens at both 71 and 251 | +| F07 | heuristic memory through the real pipeline | labelled; no fact created | +| F08 / M2 gate | endpoint → dead loopback port | story intact; failure served by API | +| Provider wiring | real factories, no mocks; plus full real-model pipeline | 5 memories, 2 summaries, all `ok` | +| Restart | two OS processes, one DB file | identical state and provenance | +| Migration | genuine pre-M6 DB, stamp 88 → 91 | backfill correct; old positions behave | +| Performance | statement counter, 51 → 231 actions | 20 queries both times | +| Browser | Firefox 154.0.1 headless | 16/16 | +| Security | policy checks on all three request paths + source scan | no new path | + +--- + +## Z. Final repository state + +``` +HEAD b7005e6fddec337f940749455739a60f0c6cb54f +branch m6-context-memory +``` + +`git status --short` — 29 staged entries, nothing unstaged, nothing untracked: + +``` +A backend/app/derived.py +A backend/app/summaries.py +A backend/tests/test_context_memory.py +A backend/tests/test_context_performance.py +A backend/tests/test_context_realistic.py +A backend/tests/test_provider_wiring.py +A planning/reports/M6-IMPLEMENTATION-REPORT.md +M backend/app/context/__init__.py +M backend/app/context/builder.py +M backend/app/memorybank.py +M backend/app/migrations.py +M backend/app/models.py +M backend/app/routers/adventures/crud.py +M backend/app/routers/adventures/insights.py +M backend/app/routers/adventures/turns.py +M backend/tests/test_history_window.py +M backend/tests/test_length_hint.py +M backend/tests/test_memory_nodes.py +M backend/tests/test_memory_retrieval.py +M backend/tests/test_prompt_caching.py +M backend/tests/test_tree_migration.py +M frontend/src/pages/Play/panels/InsightsPanel.jsx +M frontend/src/styles/play.css +M planning/BUILD-MILESTONES.md +M planning/CONTEXT-AND-MEMORY.md +M planning/DATA-MODEL.md +M planning/TECHNICAL-DESIGN.md +M planning/V1-ACCEPTANCE-TESTS.md +R planning/reports/M5-IMPLEMENTATION-REPORT.md -> planning/archive/milestone-reports/M5-IMPLEMENTATION-REPORT.md +``` + +**Files changed by this review:** one — +`planning/reports/M6-IMPLEMENTATION-REPORT.md`, rewritten from the build-time +document into this review report. No application code, test or planning document +was modified. All probes, databases, exports and browser artifacts were created +outside the repository and are not staged. + +**M7 was not started.** + +--- +--- + +# ADDENDUM — M6 Corrective Pass and Closeout + +**Date:** 2026-09-06 +**Branch:** `m6-context-memory` **Base:** `b7005e6` **HEAD:** `b7005e6` (no commit made) +**Status of the review above:** unchanged. Nothing in it has been edited or +withdrawn. The first M6 implementation reported E03 as passing and it was not; +that record stays visible. + +**Result: M6 ACCEPTED — READY TO COMMIT** + +## A1. Disposition of the findings + +| ID | Severity | Disposition | +|---|---|---| +| **M6-F1** | high, blocking | **Fixed.** Summary generation is seeded from `summaries.current`. §A2. | +| **M6-F2** | medium | **Fixed.** Redundancy suppression before the retrieval cut. §A4. | +| **M6-F3** | low | **Fixed.** The unit fixture stubs both derived providers; the warning is gone. §A6. | +| **M6-F4** | low | **Fixed.** A regression that fails against the pre-corrective code, plus a browser scenario that regenerates a summary. §A3. | +| **M6-F5** | trivial | **Fixed.** Derived work that had nothing to do reports `idle`, not `ok`. §A7. | + +## A2. M6-F1 — the fix + +One line of behaviour, in `_update_story_summary`: + +```python +# before +current = adventure.story_summary.strip() # campaign-global, no lineage + +# after +eligible = summaries.current(db, adventure) # the same question the +current = eligible.text.strip() if eligible else "" # context builder asks +``` + +The invariant this establishes, now recorded in `TECHNICAL-DESIGN.md`: + +> Both summary eligibility **and the prior-summary input to the summarizer** are +> lineage-scoped. + +Where no eligible summary exists at the current position, generation starts from +none. Nothing is deleted, no summary is cleared on Undo or divergence, and the +output row is not filtered after the fact — the correction is at the input, +which is where the contamination entered. + +### The fate of `adventures.story_summary` + +Every reader and writer was inventoried. The only unsafe read was the one above; +all others are display or transport — the Plot panel edits it, the export bundle +carries it, the API serialises it. + +**Decision: option 2 — a convenience mirror, never authoritative input.** It is +the smallest safe design: no migration, no schema change, no user-visible loss. +Two things make it honest rather than merely tolerated: + +- it is kept in step both when a summary is written (`summaries.record`) **and + when the head moves** (`attempts.restore_state`, the single chokepoint every + Undo, Redo, take switch and Save Point restore passes through), so what the + Plot panel shows is the summary the narrator is actually given; +- `models.py` and `summaries.py` now say plainly that it carries no lineage and + that nothing authoritative may read it, so the next person cannot reintroduce + the defect by accident. + +No ADR. The architectural meaning of the column did not change — it was always +meant to be a mirror; the defect was that one caller treated it as a store. + +## A3. M6-F4 — the regression that can see the defect + +`test_e03_a_summary_generated_after_divergence_carries_no_abandoned_content` +walks the eleven steps the closeout brief specifies. It was written **before** +the fix and confirmed to fail against the reviewed implementation: + +``` +$ pytest ...test_e03_a_summary_generated_after_divergence... (pre-fix) + assert E03_SENTINEL in summary_a PASS positive control + assert summary_b_row["id"] != path_a_id PASS a new summary was generated + assert carried_over == 0 PASS no path-A action on B's lineage +> assert E03_SENTINEL not in summary_b FAIL <-- the defect +``` + +After the fix it passes, including the stronger assertion that the summariser +was never even *offered* abandoned prose — seeds are captured and checked, so a +future filter-the-output "fix" would not satisfy it. + +The browser suite gained a dedicated E03 scenario on its own campaign (checks +13a–13e): a neutral opening, 18 sentinel turns, a real summary, undo below the +sentinel, 18 turns on the new line, a **regenerated** summary, then the prompt. +Two fixture facts are asserted before the negative — that path A's summary +really carried the sentinel, and that no sentinel-bearing turn remains at or +below the head — so the check cannot pass vacuously. + +## A4. M6-F2 — redundancy suppression + +The threshold was **measured, not chosen**. Against the configured local +embedding model, on the review's own fixture plus a set of deliberately tricky +pairs: + +``` +redundant pairs (near-identical filler) cosine 0.938 – 0.996 +distinct pairs (different facts) cosine 0.349 – 0.906 +``` + +`REDUNDANT_SIMILARITY = 0.93` sits in that gap. + +The same measurement **ruled out the obvious alternative**. Word overlap fires +hardest on exactly the pair that must not be merged — "Mara promised to return +before dawn" against "Aldric promised to return before dawn" shares 71% of its +words and means something else — and is weakest (27%) on filler that plainly +repeats itself. Wording is a poor proxy for sameness of fact; the embedding is a +better one. That is why the implementation uses vectors alone, and why the +measurement is recorded beside the constant. + +Two rules bound the suppression: + +- **authority is never crossed** — a `heuristic` memory can never suppress an + `accepted_story` one or the reverse; +- **the highest-ranked statement survives**, with its provenance, and the count + of suppressed candidates is reported so "why is that memory not here?" has an + answer. + +Greedy over the ranked list, stopping once the budget is filled, so the cost is +bounded by `top_k` rather than by the size of the bank. + +**Result on the review's own fixture, real embedding model, 57 actions:** + +``` +top_k clue in prompt retrieved considered suppressed + 5 yes 2 5 3 + 4 yes 2 5 3 + 3 yes 2 5 3 +``` + +Before the fix the clue placed fifth of five and was evicted at `top_k=4`. It is +now retrieved with two slots to spare, and the budget is no longer spent on four +copies of the same sentence. `top_k=4` was tested because the review showed F02 +succeeding only because the default left one slot; it no longer depends on that. + +Ranking itself is unchanged — cosine plus an explicit pin. The further factors +`CONTEXT-AND-MEMORY.md` §20 contemplates remain unimplemented and are recorded +there as future work rather than quietly claimed. + +## A5. E03 and E02, re-run independently + +**E03, deterministic:** the review's own reproduction, which isolated the +contamination route, now shows path B's summary as `MEM[dry road]` only, with +the seed free of the sentinel and zero path-A actions on path B's lineage. + +**E03, real local summariser and embedder:** + +``` +path A summary id 2, branch 1, depth 30, source 15–30 sentinel present + POSITIVE CONTROL: sentinel in the active prompt yes +undo 21 steps to head (1,0); diverge; play on; run the real derived pass +path B summary id 3, branch 2, depth 16, source 1–16 + "The dry, silent road continues, leading the protagonist forward…" + sentinel in the new summary NO + sentinel anywhere in the active prompt NO + no path-A action on path B's lineage confirmed +retention 7 memories, 3 summaries on disk; 1 eligible, 2 retained-but-ineligible +``` + +**E02, unchanged and re-verified:** sentinel retrievable on the valid lineage; +ineligible after Undo below its source; eligible again after Redo with **0** +re-embeddings; absent from the active prompt after divergence; retained on disk +throughout. + +## A6. M6-F3 — no accidental network calls + +`test_context_memory.py` stubbed the embedder but not the summariser, so with +`auto_summarize=True` every turn built a real summariser against the default +endpoint. Both are stubbed now. The `RuntimeWarning: coroutine +'connect_tcp…try_connect' was never awaited` is gone: **2 warnings → 1**, the +remaining one being the pre-existing Starlette deprecation notice. + +The tests that deliberately exercise real provider construction — +`test_provider_wiring.py`, and the realistic-context tests — are untouched and +still mock no factory. + +## A7. M6-F5 — `idle` + +`derived_status.status` is now `ok` (did work), `idle` (ran, nothing pending) or +`failed`, and `last_success_at` is only stamped for real work. Observed in the +failure probe: `embedding idle` with a null success time, beside +`memory failed` and `summary failed`. Nothing else changed; this is not a status +redesign. + +## A8. What the corrective pass had to leave alone, and did + +Two test fixtures stored the **same vector in every memory** +(`test_memory_nodes.py`). That was harmless until redundancy suppression +existed, at which point four identical vectors are four copies of one statement +and the lineage assertions could no longer be read. The fixtures now use +distinct directions at equal angle from the query, so ranking between them is +unchanged and the tests measure what they are about. The lineage assertions +themselves were not weakened. + +This is worth recording because it is the one place the corrective pass changed +a test that was passing: the change removes an unintended coupling, it does not +relax a requirement. + +## A9. Verification + +``` +backend suite 836 passed, 7 skipped, 1 warning (285s) + was 835 / 7 / 2 at review; +1 is the new E03 regression, + and the lost warning is M6-F3 +skips unchanged: 3 M5 realistic + 3 M6 realistic + 1 live wiring, + all endpoint-gated, none reported as PASS +frontend lint exit 0, 7 warnings — identical to the M5 baseline +frontend build success +docker build success +browser 22/22 (was 16/16; +6 for the new E03 scenario) +``` + +Preserved M6 successes, re-checked after the corrective changes: + +| Area | Result | +|---|---| +| F01 recent coherence, F03 bounded, F04 reserve | unchanged; 231 actions still 5,326 tokens | +| F05 inspector, F06 provenance, F07 heuristic | unchanged | +| F08 failure non-fatal and observable | re-run against a dead endpoint: story, state, head intact; failure served | +| Save Point restore, Undo/Redo lineage | unchanged | +| Process restart | re-run: two PIDs, identical state and provenance | +| Pre-M6 migration | re-run: stamp 88 → 91, backfill anchored, restored position correct | +| Query counts | 51 → 231 actions: **20 queries both times**, unchanged | +| Local-only boundary | unchanged; no new client, host or path | +| M1–M5 focused regressions | green | + +`memories_used` in some probes drops (5 → 2) purely because redundant +candidates are now suppressed; the content that mattered is still present. + +## A10. Planning documents changed + +| Document | Change | +|---|---| +| `CONTEXT-AND-MEMORY.md` | §11 rewritten: lineage safety has two halves, and the input half is what M6-F1 was. §20 records that ranking is similarity plus a pin and the other factors are **not** implemented. §22 records what redundancy suppression does, the measurement behind the threshold, why lexical overlap was rejected, and that cross-layer duplication is still open. | +| `TECHNICAL-DESIGN.md` | The corrected summary invariant, why anchoring the output alone is insufficient, the role of `adventures.story_summary`, and the retrieval/redundancy behaviour. | +| `DATA-MODEL.md` | `derived_status.status` gains `idle`; the summary entry records that rows are also the *input* to the next round. | +| `V1-ACCEPTANCE-TESTS.md` | E03 rewritten with the full history (baseline fail → anchored-row fail → corrected) and the **required test shape**, so a future test cannot regress to checking old-row eligibility. F02 records the one-slot margin the review found and the corrected result at top_k 5/4/3. Neither pass condition weakened. | +| `BUILD-MILESTONES.md` | M6 marked complete and accepted; the review findings and corrective work recorded; retrieval-ranking and cross-layer duplication recorded as debt; M7 explicitly authorized. | +| `VERSION.md` | New **v2.7** entry covering M5 and M6 closeout, including that M5's closeout never added one, and what the package learned from M6. | +| `planning/README.md` | Current state M1–M6 accepted, next M7; the stale M5 handoff text replaced with the M6 debt that actually informs M7. | + +No ADR was added. Every choice implements a decision the package had already +settled; the one candidate — a change in the meaning of `adventures.story_summary` +— did not occur, because the column's meaning was always "mirror". + +## A11. Final verdict + +| Question | Answer | +|---|---| +| Is M6's Definition of Done met? | **Yes.** Lineage-safe (both halves), authority-aware, local, inspectable, bounded. | +| Are F01–F08 satisfied at the level M6 owns? | **Yes**, all eight; F02 no longer by a one-slot margin. | +| Is E03 demonstrated end-to-end? | **Yes** — deterministic, real-model, and browser, each with positive controls. | +| Can abandoned summary content reach an active prompt? | **No.** | +| Can abandoned memory reach an active prompt? | **No.** | +| Are background failures observable? | **Yes**, from a real socket failure. | +| Did any M1–M5 behaviour regress? | **No.** | +| Is M7 safe to begin? | **Yes.** Both blockers named in §W are closed. | + +**M6 ACCEPTED — READY TO COMMIT** + +M7 was not started.