From 2efa92d421565b810a059b6adce9a5a6cd30117c Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:10:39 +0000 Subject: [PATCH 01/12] feat(rag): one definition of where an exchange ends compaction.py and the migration that has to re-slice what it wrote weeks ago both need to answer "where does one exchange end?", and until now only compaction could -- inline, in the middle of a join. Two copies of that rule would put the live path and the migration out of step, and the store would end up with its old and new halves cut differently. That has no symptom other than a retrieval distance nobody can account for. forge/transcript.py holds the three operations: render(messages) is the format the store has always used, blocks(messages) groups messages into retrieval units, split(text) recovers the same units from text that was already rendered. The test that earns the module is the round trip -- split(render(msgs)) == blocks(msgs). An exchange, and not the whole evicted block, because of the numbers measured on the Deck on 2026-08-22: a question sits at 0.9386 from the compacted block that answers it nearly word for word, while a short fact answering a different question sits at 0.671. Burying a sentence in a long block costs about 0.27 of distance -- larger than the whole gap that separated a hit from a miss that day. rag._embed averages the chunk vectors of a long input, and the mean of a dozen unrelated chunks is near no question in particular. An exchange, and not a single message, because the other direction fails too: a user turn alone retrieves a question rather than an answer, and "oui, 8080" on its own has lost its subject. Known limit, accepted and documented: split finds boundaries by role prefix at start of line, so a message whose own content contains such a line splits in the wrong place. It splits -- it does not lose anything -- and the alternative would not be able to read the blocks already in the store, which is the case this exists to serve. --- src/forge/transcript.py | 120 +++++++++++++++++++++++++++++++++++++++ tests/test_transcript.py | 84 +++++++++++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 src/forge/transcript.py create mode 100644 tests/test_transcript.py diff --git a/src/forge/transcript.py b/src/forge/transcript.py new file mode 100644 index 0000000..4fb2f0e --- /dev/null +++ b/src/forge/transcript.py @@ -0,0 +1,120 @@ +""" +One definition of how a conversation is written down, and where one +exchange ends. + +Two callers need the same answer to that question and, until this +module existed, were free to disagree. compaction.py renders evicted +messages and hands the text to the vector store; the migration in +deploy/rag_resplit.py has to take a block that was rendered weeks ago +and cut it the same way. A second copy of the boundary rule would put +the live path and the migration out of step, and the symptom would be +a store whose old and new entries are sliced differently -- invisible +from the outside, surfacing only as a retrieval distance nobody can +account for. + +WHY AN EXCHANGE, AND NOT THE WHOLE BLOCK + +Measured on the Deck against the real store on 2026-08-22. The +question "Quels outils as-tu accès ?" sits at distance 0.9386 from the +entry that answers it nearly word for word -- because that entry is a +whole compacted block. A short fact answering "Le serveur de test +tourne sur quel port ?" sits at 0.671. Burying a sentence in a long +block costs roughly 0.27 of distance, which is larger than the entire +gap that separated a hit from a miss that day (0.0422). + +The cause is in rag._embed: text past EMBEDDING_MAX_CHARS is split, +each chunk embedded, and the chunk vectors AVERAGED into one. The mean +of a dozen unrelated chunks is close to no question in particular. The +fix is not a better average -- it is to stop asking one vector to +stand for a dozen subjects. + +WHY AN EXCHANGE, AND NOT A SINGLE MESSAGE + +The other direction fails too. A user turn alone retrieves a question +rather than an answer, and an assistant reply on its own has lost its +subject: "oui, 8080" answers nothing when it is all you get back. A +user turn plus whatever answered it is the smallest unit that still +stands on its own. + +KNOWN LIMIT, accepted. `split` finds turn boundaries by looking for a +role prefix at the start of a line, so a message whose own content +contains a line beginning with "user: " produces a boundary in the +wrong place. Nothing is lost when that happens -- the text is still +stored, in two pieces instead of one -- and the alternative (a +structured archive format) would not be readable by the blocks already +in the store, which is the case this has to serve. +""" + +import re + +# Every role forge.memory writes. Kept as one tuple because the regex +# below and any future caller must agree on the closed set: a role +# missing here is not a boundary, so its message silently joins the +# previous turn. +ROLES = ("user", "assistant", "system") + +_SPEAKER = re.compile(rf"^({'|'.join(ROLES)}): ", re.MULTILINE) + + +def render(messages: list[dict]) -> str: + """Write messages down the way the store has always held them.""" + return "\n".join(f"{m['role']}: {m['content']}" for m in messages) + + +def blocks(messages: list[dict]) -> list[str]: + """ + Group messages into retrieval units and render each one. + + A new unit starts at every `user` message; anything that follows + belongs to it. Messages appearing before the first user turn form a + unit of their own rather than being dropped or glued to the turn + after them -- an evicted window does not necessarily begin on a + user message. + """ + groups: list[list[dict]] = [] + for m in messages: + if m.get("role") == "user" or not groups: + groups.append([m]) + else: + groups[-1].append(m) + return [render(g) for g in groups] + + +def split(text: str) -> list[str]: + """ + Cut already-rendered text into the same units `blocks` would have + produced from the messages it came from. + + This is the inverse used by the migration: entries written before + the split existed are one long string, and re-slicing them is the + only way the store's old half ends up shaped like its new half. + + Text with no role prefix at all comes back as a single unit -- + something is in there, and refusing to return it would silently + delete an entry during a migration. Text before the first prefix + is returned as its own unit for the same reason. + """ + marks = list(_SPEAKER.finditer(text)) + if not marks: + return [text] if text.strip() else [] + + pieces: list[tuple[str, str]] = [] + lead = text[: marks[0].start()] + for i, mark in enumerate(marks): + end = marks[i + 1].start() if i + 1 < len(marks) else len(text) + pieces.append((mark.group(1), text[mark.start() : end])) + + units: list[str] = [] + if lead.strip(): + units.append(lead) + for role, piece in pieces: + if role == "user" or not units: + units.append(piece) + else: + units[-1] += piece + + # render() joins with exactly one "\n", so a unit that was followed + # by another ends with that separator. Removing exactly one newline + # is the faithful inverse; stripping all of them would eat a blank + # line a message actually ended with. + return [u.removesuffix("\n") for u in units] diff --git a/tests/test_transcript.py b/tests/test_transcript.py new file mode 100644 index 0000000..c8a0572 --- /dev/null +++ b/tests/test_transcript.py @@ -0,0 +1,84 @@ +""" +Unit tests for forge.transcript. + +The test that matters here is the round trip: `split(render(msgs))` +must equal `blocks(msgs)`. Compaction takes the first path (messages +-> units) and the migration in deploy/rag_resplit.py takes the second +(stored text -> units), and if the two ever disagree the store ends up +holding two differently-sliced halves -- which shows up as a retrieval +distance with no explanation, not as a failure. +""" + +from forge import transcript + + +def _m(role: str, content: str) -> dict: + return {"role": role, "content": content} + + +def test_render_matches_the_stored_format(): + assert transcript.render([_m("user", "salut"), _m("assistant", "bonjour")]) == ( + "user: salut\nassistant: bonjour" + ) + + +def test_a_unit_is_a_user_turn_plus_what_answered_it(): + messages = [ + _m("user", "quel port ?"), + _m("assistant", "8080"), + _m("user", "et l'hôte ?"), + _m("assistant", "localhost"), + ] + + assert transcript.blocks(messages) == [ + "user: quel port ?\nassistant: 8080", + "user: et l'hôte ?\nassistant: localhost", + ] + + +def test_messages_before_the_first_user_turn_are_their_own_unit(): + messages = [ + _m("system", "[12 messages compactés]"), + _m("assistant", "suite"), + _m("user", "question"), + _m("assistant", "réponse"), + ] + + assert transcript.blocks(messages) == [ + "system: [12 messages compactés]\nassistant: suite", + "user: question\nassistant: réponse", + ] + + +def test_split_round_trips_through_render(): + messages = [ + _m("system", "ouverture"), + _m("user", "quel port ?"), + _m("assistant", "8080"), + _m("user", "multi\nligne"), + _m("assistant", "réponse\nsur deux lignes"), + ] + + assert transcript.split(transcript.render(messages)) == transcript.blocks(messages) + + +def test_split_keeps_text_that_has_no_role_prefix(): + # A migration that silently returned [] here would delete the entry + # it was asked to re-slice. + assert transcript.split("juste du texte") == ["juste du texte"] + + +def test_split_keeps_text_appearing_before_the_first_prefix(): + units = transcript.split("préambule\nuser: question\nassistant: réponse") + + assert units == ["préambule", "user: question\nassistant: réponse"] + + +def test_split_of_empty_text_is_empty(): + assert transcript.split(" ") == [] + + +def test_a_single_exchange_stays_one_unit(): + text = "user: une question\nassistant: une réponse" + + assert transcript.split(text) == [text] From be5d747ff619d782971272afb7b10616ee17d358 Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:11:58 +0000 Subject: [PATCH 02/12] feat(rag): store a batch as N rows instead of one averaged vector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit remember() writes one row and one vector. Compaction's only way to archive a block was therefore to join it into one string -- and that string is far past EMBEDDING_MAX_CHARS, so its single vector is the AVERAGE of a dozen chunk vectors produced by _embed. The mean of a dozen unrelated subjects is close to no question in particular. That is not a theory. Measured against the real store on 2026-08-22: "Quels outils as-tu accès ?" sits at 0.9386 from the compacted block that contains the question almost word for word, while a short fact answering a different question sits at 0.671. About 0.27 of distance, lost to the averaging, on a store where the whole hit/miss gap that day was 0.0422. remember_many() writes N rows in one transaction. It costs no extra embedding calls -- _embed already made one request per chunk. The change is that the chunks stay apart instead of collapsing into their mean. Two deliberate asymmetries with remember(): A degenerate item is skipped with a warning, not raised on. remember() raises because a caller asserting a one-word fact needs to hear that the value went missing; here the caller is archiving a block it did not write, and failing the whole compaction because one evicted message was a single word leaves the history uncompacted with no way out. An embedding failure propagates before the commit, so nothing is stored. A half-indexed block is worse than an unindexed one: the pointer written into the history claims a range that does not hold what it says it holds. --- src/forge/rag.py | 53 ++++++++++++++++++- tests/test_rag_batch.py | 111 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 tests/test_rag_batch.py diff --git a/src/forge/rag.py b/src/forge/rag.py index 87bb846..ce5b479 100644 --- a/src/forge/rag.py +++ b/src/forge/rag.py @@ -216,6 +216,58 @@ def remember( f"question phrased around it and answers none of them." ) + entry_id = _insert(conn, kind, content, project) + conn.commit() + return entry_id + + +def remember_many( + conn: sqlite3.Connection, kind: str, contents: list[str], project: str | None +) -> list[int]: + """ + Store several entries as several ROWS, in one transaction. + + The alternative -- what compaction did until now -- is to join the + lot into one string and store it as a single entry. That entry then + gets one vector, and since the joined text is far past + EMBEDDING_MAX_CHARS, that vector is the AVERAGE of a dozen chunk + vectors. A mean of a dozen unrelated subjects is close to no + question in particular, which is the 0.27 of distance measured on + 2026-08-22 between a fact stored alone and the same content buried + in a compacted block. + + It costs no extra embedding calls: _embed already made one request + per chunk. The change is that the chunks are kept apart instead of + being collapsed into their mean. + + A degenerate item is SKIPPED, not raised on. remember() raises + because a caller asserting a one-word fact should hear about it; + here the caller is archiving a block it did not write, and failing + the whole compaction because one evicted message was a single word + would leave the history uncompacted with no way to recover. + + All or nothing on the embedding server, deliberately: an + EmbeddingError propagates before the commit, so a block is never + half-indexed. A partially indexed block is worse than an unindexed + one -- the pointer written into the history claims a range that + does not hold what it says it holds. + """ + ids: list[int] = [] + for content in contents: + if len(content.split()) < _MIN_ENTRY_WORDS: + log.warning("rag: skipping a degenerate entry in a batch: %r", content) + continue + ids.append(_insert(conn, kind, content, project)) + + conn.commit() + return ids + + +def _insert( + conn: sqlite3.Connection, kind: str, content: str, project: str | None +) -> int: + """Write one row and its vector. Does NOT commit -- the caller owns + the transaction, which is what lets remember_many be atomic.""" cur = conn.execute( "INSERT INTO memory_entries (kind, content, project, created_at) VALUES (?, ?, ?, ?)", (kind, content, project, datetime.now(UTC).isoformat()), @@ -227,7 +279,6 @@ def remember( "INSERT INTO memory_vectors (rowid, embedding) VALUES (?, ?)", (entry_id, sqlite_vec.serialize_float32(embedding)), ) - conn.commit() return entry_id diff --git a/tests/test_rag_batch.py b/tests/test_rag_batch.py new file mode 100644 index 0000000..c8f6d25 --- /dev/null +++ b/tests/test_rag_batch.py @@ -0,0 +1,111 @@ +""" +Tests for rag.remember_many. + +The point of the batch write is that N units become N ROWS with N +vectors, instead of one row whose single vector is the average of N +chunks. The average is what put a question 0.27 further from the +content answering it than the same content stored on its own +(measured on the Deck, 2026-08-22). +""" + +import pytest + +from forge import rag + + +@pytest.fixture +def store(tmp_path, monkeypatch): + monkeypatch.setattr(rag, "RAG_DB_FILE", str(tmp_path / "rag.db")) + monkeypatch.setattr(rag, "_embed", lambda text: [0.1] * rag.EMBEDDING_DIM) + conn = rag.get_connection() + yield conn + conn.close() + + +def test_each_unit_becomes_its_own_entry(store): + ids = rag.remember_many( + store, + kind="history_summary", + contents=["user: quel port ?", "user: et l'hôte ?"], + project=None, + ) + + assert len(ids) == 2 + assert rag.count_entries(store)["by_kind"] == {"history_summary": 2} + assert [e["content"] for e in rag.list_entries(store)] == [ + "user: et l'hôte ?", + "user: quel port ?", + ] + + +def test_each_entry_gets_its_own_vector(store): + ids = rag.remember_many( + store, kind="history_summary", contents=["un bloc", "un autre"], project=None + ) + + rows = store.execute( + "SELECT rowid FROM memory_vectors WHERE rowid IN (?, ?)", ids + ).fetchall() + assert len(rows) == 2 + + +def test_a_degenerate_unit_is_skipped_not_raised(store): + """ + remember() raises on a one-word entry because the caller is + asserting a fact and needs to hear that the value went missing. + Here the caller is archiving a block it did not write, and one + one-word message must not fail the whole compaction. + """ + ids = rag.remember_many( + store, + kind="history_summary", + contents=["user: bonjour\nassistant: salut", "ok"], + project=None, + ) + + assert len(ids) == 1 + assert rag.count_entries(store)["total"] == 1 + + +def test_an_embedding_failure_stores_nothing(store, monkeypatch): + """ + All or nothing. A half-indexed block is worse than an unindexed + one: the pointer written into the history claims a range that does + not hold what it says it holds. + """ + calls = {"n": 0} + + def _flaky(text): + calls["n"] += 1 + if calls["n"] == 2: + raise rag.EmbeddingError("server down") + return [0.1] * rag.EMBEDDING_DIM + + monkeypatch.setattr(rag, "_embed", _flaky) + + with pytest.raises(rag.EmbeddingError): + rag.remember_many( + store, + kind="history_summary", + contents=["premier bloc", "second bloc", "troisième bloc"], + project=None, + ) + + store.rollback() + assert rag.count_entries(store)["total"] == 0 + + +def test_an_empty_batch_is_not_an_error(store): + assert ( + rag.remember_many(store, kind="history_summary", contents=[], project=None) + == [] + ) + assert rag.count_entries(store)["total"] == 0 + + +def test_single_writes_still_commit_on_their_own(store): + """remember() shares _insert with the batch path now -- it must + still be a complete write by itself.""" + entry_id = rag.remember(store, kind="fact", content="un vrai fait", project=None) + + assert rag.list_entries(store)[0]["id"] == entry_id From 63cdb507599b948243ff2881fbb70dee8346135d Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:14:27 +0000 Subject: [PATCH 03/12] fix(compaction): index one entry per exchange, and only conversation The rag_pointer strategy joined an entire evicted block into one string and stored it as one entry. That is how the store on the Deck came to hold 16 entries of which 11 were whole compacted blocks, and why a question sits 0.27 further from the block containing its answer than from a short fact. transcript.blocks() cuts the block at user turns and rag.remember_many() stores the pieces as rows. The pointer left in the history now names the range it created (#12-#27) instead of a single id, and says so plainly when nothing was indexable rather than pointing at an entry that does not exist. A new compaction.indexed log line records how many messages went in and how many entries came out, because those two numbers are no longer equal and the difference is the filtering below. Two things stop at the store boundary, both found by reading the real store on 2026-08-22 -- the first day anything could enumerate it without asking it a question: An earlier compaction pointer is not indexed again. It is a reference to another entry, and stored it becomes a memory whose entire content is "N messages were compacted, see #12": it answers no question and sits at middling distance from all of them. The block it points at stays reachable through search; only the textual chain is not rebuilt, which is the honest trade for not indexing a signpost as if it were the road. Raw router JSON is unwrapped. Entry #9 of that store was a {"tool": "code", ...} object swallowed from an assistant turn by a version that did not unwrap tool output yet. The envelope is the noise and the content inside it is a real answer, so it is unwrapped rather than dropped. _POINTER_RE and _pointer() are two statements of the same string and a test pins them together. A pointer that stops matching its own detector gets indexed as conversation, and nothing anywhere fails. llm_summary now renders through transcript.render for the same reason: one definition of the stored format. --- src/forge/compaction.py | 99 +++++++++++++++++--- tests/test_api.py | 4 +- tests/test_compaction.py | 15 ++-- tests/test_compaction_intake.py | 154 ++++++++++++++++++++++++++++++++ tests/test_memory.py | 4 +- 5 files changed, 254 insertions(+), 22 deletions(-) create mode 100644 tests/test_compaction_intake.py diff --git a/src/forge/compaction.py b/src/forge/compaction.py index e069323..1970de2 100644 --- a/src/forge/compaction.py +++ b/src/forge/compaction.py @@ -25,7 +25,9 @@ free to drift from the one that matters. """ -from forge import rag +import re + +from forge import rag, transcript from forge.config import ( COMPACTION_ENABLED, COMPACTION_KEEP_RECENT, @@ -190,18 +192,25 @@ def _run_strategy(messages: list[dict]) -> dict: def _strategy_rag_pointer(messages: list[dict]) -> dict: """ Default strategy: push the compacted block into vector memory - verbatim, as one 'history_summary' RAG entry (searchable later via + verbatim, as 'history_summary' RAG entries (searchable later via !recall / /search), and replace it in the rolling history with a - short pointer. Cheap -- no LLM call -- but only as faithful as - what's already indexed, since nothing is reworded. + short pointer. Cheap -- no LLM call -- but only as faithful as what + is already indexed, since nothing is reworded. + + ONE ENTRY PER EXCHANGE, not one per block. The block form is what + made this store unusable: on 2026-08-22 it held 16 entries, 11 of + them whole compacted blocks, and a question landed 0.27 further + from the block containing its answer than a short fact does. See + forge/transcript.py for the numbers and for where the boundary + falls. """ - joined = "\n".join(f"{m['role']}: {m['content']}" for m in messages) + contents = transcript.blocks(_indexable(messages)) try: conn = rag.get_connection() try: - entry_id = rag.remember( - conn, kind="history_summary", content=joined, project=None + ids = rag.remember_many( + conn, kind="history_summary", contents=contents, project=None ) finally: conn.close() @@ -209,17 +218,83 @@ def _strategy_rag_pointer(messages: list[dict]) -> dict: log.error("compaction: embedding server unreachable: %s", e) raise CompactionError(str(e)) from e + log.event( + "compaction.indexed", + messages=len(messages), + indexed=len(contents), + entries=len(ids), + first_id=ids[0] if ids else None, + last_id=ids[-1] if ids else None, + ) + return { "id": messages[0]["id"], "role": "system", - "content": ( - f"[{len(messages)} messages précédents compactés -- " - f"voir mémoire vectorielle #{entry_id}, cherchable via !recall]" - ), + "content": _pointer(len(messages), ids), "pinned": False, } +# What a pointer written by this strategy looks like, in the one form +# that has to be recognised again later -- see _indexable. +# +# Regex and builder are pinned together by a test rather than trusted +# to stay in sync. This is the same drift that bit router/grammar.py +# against router/prompt.py: two places stating the same string, one of +# them edited. +_POINTER_RE = re.compile(r"^\[\d+ messages précédents compactés") + + +def _pointer(message_count: int, ids: list[int]) -> str: + if not ids: + return ( + f"[{message_count} messages précédents compactés -- " + f"rien d'indexable, aucune entrée mémoire créée]" + ) + if len(ids) == 1: + where = f"#{ids[0]}" + else: + where = f"#{ids[0]}-#{ids[-1]} ({len(ids)} entrées)" + return ( + f"[{message_count} messages précédents compactés -- " + f"voir mémoire vectorielle {where}, cherchable via !recall]" + ) + + +def _indexable(messages: list[dict]) -> list[dict]: + """ + Drop what is not conversation, and unwrap what is wearing an + envelope, before any of it reaches the vector store. + + Both cases were found by reading the real store on 2026-08-22, + which is the first day anything could read it without asking it a + question (rag.list_entries). + + A previous compaction pointer is a reference to another entry. + Indexed, it becomes a memory whose entire content is the sentence + "N messages were compacted, see #12" -- it answers no question and + sits at middling distance from all of them. The block it points at + stays reachable through search; only the textual chain is not + rebuilt, which is the honest trade for not indexing a signpost as + if it were the road. + + Entry #9 of that store held raw router JSON -- {"tool": "code", + ...} -- swallowed from an assistant turn by an older version that + did not unwrap tool output. The envelope is the noise; the content + inside it is a real answer, so it is unwrapped rather than dropped. + """ + kept = [] + for m in messages: + content = (m.get("content") or "").strip() + if not content: + continue + if m.get("role") == "system" and _POINTER_RE.match(content): + continue + unwrapped = try_unwrap_router_json(content, "compaction") + kept.append({**m, "content": unwrapped if unwrapped is not None else content}) + return kept + + def _strategy_llm_summary(messages: list[dict]) -> dict: """ Alternative strategy: ask the chat LLM to condense the block into @@ -230,7 +305,7 @@ def _strategy_llm_summary(messages: list[dict]) -> dict: from forge.errors import ProviderError from forge.llm import call_llm - joined = "\n".join(f"{m['role']}: {m['content']}" for m in messages) + joined = transcript.render(messages) prompt = ( "Résume cet échange en conservant les décisions prises, l'état " "du travail en cours, et les fichiers/projets mentionnés. " diff --git a/tests/test_api.py b/tests/test_api.py index 7b83ea8..8714af0 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -488,7 +488,9 @@ def close(self): monkeypatch.setattr(compaction.rag, "get_connection", lambda: _FakeConn()) monkeypatch.setattr( - compaction.rag, "remember", lambda conn, kind, content, project: 1 + compaction.rag, + "remember_many", + lambda conn, kind, contents, project: list(range(1, len(contents) + 1)), ) client = _client() diff --git a/tests/test_compaction.py b/tests/test_compaction.py index 5718981..df0bd4c 100644 --- a/tests/test_compaction.py +++ b/tests/test_compaction.py @@ -30,15 +30,14 @@ class _FakeConn: def close(self): pass + def _remember_many(conn, kind, contents, project): + return list(range(42, 42 + len(contents))) + monkeypatch.setattr(rag, "get_connection", lambda: _FakeConn()) - monkeypatch.setattr( - rag, "remember", lambda conn, kind, content, project: 42, raising=False - ) + monkeypatch.setattr(rag, "remember_many", _remember_many, raising=False) # compaction imported `rag` directly, patch the same module object monkeypatch.setattr(compaction.rag, "get_connection", lambda: _FakeConn()) - monkeypatch.setattr( - compaction.rag, "remember", lambda conn, kind, content, project: 42 - ) + monkeypatch.setattr(compaction.rag, "remember_many", _remember_many) def test_below_threshold_is_untouched(monkeypatch): @@ -131,10 +130,10 @@ def test_embedding_failure_raises_compaction_error(monkeypatch): monkeypatch.setattr(compaction, "COMPACTION_THRESHOLD", 1) monkeypatch.setattr(compaction, "COMPACTION_KEEP_RECENT", 1) - def _boom(conn, kind, content, project): + def _boom(conn, kind, contents, project): raise rag.EmbeddingError("embedding server down") - monkeypatch.setattr(compaction.rag, "remember", _boom) + monkeypatch.setattr(compaction.rag, "remember_many", _boom) history = _messages(5) with pytest.raises(compaction.CompactionError): diff --git a/tests/test_compaction_intake.py b/tests/test_compaction_intake.py new file mode 100644 index 0000000..3ba2225 --- /dev/null +++ b/tests/test_compaction_intake.py @@ -0,0 +1,154 @@ +""" +What compaction puts INTO the vector store, as opposed to what it +takes out of the history. + +Everything here comes from reading the real store on 2026-08-22, the +first day anything could enumerate it: 16 entries, 11 of them whole +compacted blocks, one holding raw router JSON, and a measured 0.27 of +distance lost to burying a sentence in a block. +""" + +import pytest + +from forge import compaction, rag + + +@pytest.fixture +def indexed(monkeypatch): + """Capture what reaches the store instead of embedding it.""" + captured: list[str] = [] + + class _FakeConn: + def close(self): + pass + + def _remember_many(conn, kind, contents, project): + captured.extend(contents) + return list(range(10, 10 + len(contents))) + + monkeypatch.setattr(compaction.rag, "get_connection", lambda: _FakeConn()) + monkeypatch.setattr(compaction.rag, "remember_many", _remember_many) + return captured + + +def _m(role: str, content: str, mid: int = 0) -> dict: + return {"id": mid, "role": role, "content": content, "pinned": False} + + +def test_each_exchange_becomes_its_own_entry(indexed): + compaction._strategy_rag_pointer( + [ + _m("user", "quel port ?", 1), + _m("assistant", "8080", 2), + _m("user", "et l'hôte ?", 3), + _m("assistant", "localhost", 4), + ] + ) + + assert indexed == [ + "user: quel port ?\nassistant: 8080", + "user: et l'hôte ?\nassistant: localhost", + ] + + +def test_the_pointer_names_the_range_it_created(indexed): + summary = compaction._strategy_rag_pointer( + [ + _m("user", "a", 1), + _m("assistant", "b", 2), + _m("user", "c", 3), + _m("assistant", "d", 4), + ] + ) + + assert "#10-#11 (2 entrées)" in summary["content"] + assert "4 messages" in summary["content"] + + +def test_a_single_entry_is_named_without_a_range(indexed): + summary = compaction._strategy_rag_pointer( + [_m("user", "une question", 1), _m("assistant", "une réponse", 2)] + ) + + assert "#10," in summary["content"] + assert "-#" not in summary["content"] + + +def test_the_pointer_is_recognisable_by_the_regex_that_looks_for_it(): + """ + The builder and the detector are two statements of the same string + and would drift apart silently -- a pointer that stops matching is + a pointer that gets indexed as if it were conversation, and + nothing fails. + """ + for ids in ([], [7], [7, 8, 9]): + assert compaction._POINTER_RE.match(compaction._pointer(12, ids)) + + +def test_an_earlier_pointer_is_not_indexed_again(indexed): + """ + A pointer is a reference to another entry. Stored, it becomes a + memory whose whole content is "N messages were compacted, see + #12": it answers no question and sits at middling distance from + every one of them. + """ + compaction._strategy_rag_pointer( + [ + _m("system", compaction._pointer(59, [12]), 1), + _m("user", "une question", 2), + _m("assistant", "une réponse", 3), + ] + ) + + assert indexed == ["user: une question\nassistant: une réponse"] + + +def test_a_router_json_envelope_is_unwrapped_not_stored_raw(indexed): + """ + Entry #9 of the real store was raw router JSON, swallowed from an + assistant turn by an older version. The envelope is the noise; the + answer inside it is real. + """ + compaction._strategy_rag_pointer( + [ + _m("user", "explique le cache KV", 1), + _m( + "assistant", + '{"tool": "chat", "content": "Le cache KV garde les clés et ' + 'valeurs déjà calculées pour ne pas refaire le prefill."}', + 2, + ), + ] + ) + + assert len(indexed) == 1 + assert indexed[0].startswith("user: explique le cache KV\nassistant: Le cache KV") + assert '"tool"' not in indexed[0] + + +def test_empty_messages_are_not_indexed(indexed): + compaction._strategy_rag_pointer( + [_m("user", " ", 1), _m("assistant", "une vraie réponse", 2)] + ) + + assert indexed == ["assistant: une vraie réponse"] + + +def test_a_block_with_nothing_indexable_says_so(indexed): + summary = compaction._strategy_rag_pointer( + [_m("system", compaction._pointer(59, [12]), 1)] + ) + + assert indexed == [] + assert "aucune entrée mémoire créée" in summary["content"] + assert "#" not in summary["content"].split("--")[1] + + +def test_the_embedding_server_being_down_still_stops_compaction(indexed, monkeypatch): + def _boom(conn, kind, contents, project): + raise rag.EmbeddingError("down") + + monkeypatch.setattr(compaction.rag, "remember_many", _boom) + + with pytest.raises(compaction.CompactionError): + compaction._strategy_rag_pointer([_m("user", "une question", 1)]) diff --git a/tests/test_memory.py b/tests/test_memory.py index 126519c..5f53522 100644 --- a/tests/test_memory.py +++ b/tests/test_memory.py @@ -150,7 +150,9 @@ def close(self): monkeypatch.setattr(memory.compaction.rag, "get_connection", lambda: _FakeConn()) monkeypatch.setattr( - memory.compaction.rag, "remember", lambda conn, kind, content, project: 1 + memory.compaction.rag, + "remember_many", + lambda conn, kind, contents, project: list(range(1, len(contents) + 1)), ) for i in range(5): From f7ec47795ce44e0a86848b1e19f0bceca3d765cc Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:20:43 +0000 Subject: [PATCH 04/12] feat(deploy): re-slice the blocks already in the store Indexing per exchange fixes what compaction writes from here on and does nothing at all for what is already written. On 2026-08-22 the real store was 16 entries of which 11 were whole compacted blocks, so without a migration the store stays mostly made of the shape the change exists to remove -- and the next measurement against it measures the old problem. deploy/rag_resplit.py cuts every history_summary back through transcript.split() and stores the pieces. An entry that yields a single unit is left untouched: it is already the right shape, and rewriting it would spend an embedding call to produce the same row under a new id. Other kinds are not considered -- a fact, a decision or a todo is one statement by construction. Dry run is the DEFAULT and prints exactly what --apply would do, with --backup for the copy nobody takes in time. This rewrites rows in place and there is no undo. New entries are inserted before the old one is deleted. A crash in between leaves the block stored twice -- as a blob and as its pieces -- which is visible in !memory and fixable with !forget. The other order loses the block outright. Duplicated is recoverable, deleted is not. An embedding failure stops the run instead of skipping the entry: a server that just went away will fail every remaining entry too, and a half-migrated store with no record of where it stopped is worse than one that was never started. What was already committed stays. Cost, so it is not a surprise: one embedding request per new entry, so 11 blocks cutting into ~90 exchanges is ~90 requests. That is the same number _embed already made when it chunked those blocks to average them, paid once more. --- deploy/rag_resplit.py | 177 ++++++++++++++++++++++++++++++++++++++ tests/test_rag_resplit.py | 165 +++++++++++++++++++++++++++++++++++ 2 files changed, 342 insertions(+) create mode 100755 deploy/rag_resplit.py create mode 100644 tests/test_rag_resplit.py diff --git a/deploy/rag_resplit.py b/deploy/rag_resplit.py new file mode 100755 index 0000000..a9fe08e --- /dev/null +++ b/deploy/rag_resplit.py @@ -0,0 +1,177 @@ +#!/usr/bin/env python3 +""" +Re-slice the compaction blocks already in the store, one entry per +exchange. + +Compaction now indexes an evicted window as one entry per exchange +instead of one entry per block. That fixes what gets written from here +on and does nothing at all for what is already written -- and on +2026-08-22 the real store was 16 entries of which 11 were whole +blocks. Leaving them means the store stays mostly made of the shape +the change exists to remove, and the next measurement against it +measures the old problem. + +WHAT IT DOES + +For every `history_summary` entry, forge.transcript.split() cuts the +stored text back into the units transcript.blocks() would produce +today. An entry that yields one unit is left alone -- it is already +the right shape, and rewriting it would burn an embedding call to +produce the same row with a new id. An entry that yields several is +replaced by that many entries. + +Nothing else is touched. `fact`, `decision` and `todo` entries are +one statement each by construction; splitting them is not a thing that +makes sense. + +ORDER, AND WHY + +New entries are inserted first, the old one deleted after, both inside +the same run. A crash in between therefore leaves the block stored +twice -- as one blob and as its pieces -- which is visible in `!memory` +and fixable by deleting the blob. The other order loses the block +outright. Duplicated is recoverable, deleted is not. + +RUNNING IT + +It needs the embedding server: every new entry is a new vector, so a +store of 11 blocks cutting into ~90 exchanges makes ~90 embedding +requests. That is the same number _embed already made when it chunked +those blocks to average them, but paid again, once. + + podman exec forge sh -c 'rm -rf /tmp/arm && mkdir -p /tmp/arm' + podman cp src forge:/tmp/arm/ + podman cp deploy/rag_resplit.py forge:/tmp/arm/ + podman exec -it forge python /tmp/arm/rag_resplit.py # dry run + podman exec -it forge python /tmp/arm/rag_resplit.py --apply + +The rm -rf is not cosmetic: podman cp merges into an existing +directory instead of replacing it, so without it the run is a mix of +checkouts. + +Dry run is the default and prints exactly what --apply would do. Take +a copy of data/forge_rag.db first; this rewrites rows in place and +there is no undo. +""" + +from __future__ import annotations + +import argparse +import os +import shutil +import sys + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--db", + default=os.getenv("RAG_DB_FILE", "data/forge_rag.db"), + help="Store to migrate (default: the configured RAG_DB_FILE).", + ) + parser.add_argument( + "--apply", + action="store_true", + help="Actually write. Without it, nothing is inserted or deleted.", + ) + parser.add_argument( + "--backup", + metavar="PATH", + help="Copy the database here before writing anything.", + ) + parser.add_argument( + "--kind", + default="history_summary", + help="Which kind to re-slice (default: history_summary).", + ) + args = parser.parse_args() + + if not os.path.exists(args.db): + print(f"{args.db} does not exist") + return 1 + + # Set before importing forge.rag: RAG_DB_FILE is read at import. + os.environ["RAG_DB_FILE"] = args.db + + from forge import rag, transcript + + if args.apply and args.backup: + shutil.copy2(args.db, args.backup) + print(f"backup written to {args.backup}") + + conn = rag.get_connection() + try: + before = rag.count_entries(conn) + print(f"before: {before['total']} entries {before['by_kind']}") + + # Snapshot first. The entries this creates carry the same kind, + # so re-reading the table mid-run would hand back the pieces it + # just wrote and try to split them again. + targets = _all_of_kind(conn, args.kind) + print(f"{len(targets)} {args.kind} entries to inspect\n") + + split_count = new_count = 0 + for entry in targets: + units = transcript.split(entry["content"]) + if len(units) < 2: + continue + + split_count += 1 + new_count += len(units) + head = entry["content"][:60].replace("\n", " / ") + print(f"#{entry['id']:>4} {len(units):>3} units {head}…") + + if not args.apply: + continue + + try: + ids = rag.remember_many( + conn, kind=entry["kind"], contents=units, project=entry["project"] + ) + except rag.EmbeddingError as e: + # Stop rather than skip. An embedding server that just + # went away will fail every remaining entry too, and a + # half-migrated store with no record of where it + # stopped is worse than one that was not started. + print(f"\nembedding server unreachable: {e}") + print("stopping here -- entries already migrated are committed") + return 1 + + rag.forget(conn, entry["id"]) + print(f" -> #{ids[0]}-#{ids[-1]}, #{entry['id']} removed") + + after = rag.count_entries(conn) + print( + f"\n{split_count} entries would become {new_count}" + if not args.apply + else f"\n{split_count} entries became {new_count}" + ) + print(f"after: {after['total']} entries {after['by_kind']}") + if not args.apply: + print("\ndry run -- nothing was written. Re-run with --apply.") + finally: + conn.close() + + return 0 + + +def _all_of_kind(conn, kind: str) -> list[dict]: + """Every entry of one kind, oldest first, paging through + rag.list_entries so the migration does not silently stop at its + default limit.""" + from forge import rag + + out: list[dict] = [] + page = 200 + offset = 0 + while True: + batch = rag.list_entries(conn, kind=kind, limit=page, offset=offset) + out.extend(batch) + if len(batch) < page: + break + offset += page + return list(reversed(out)) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_rag_resplit.py b/tests/test_rag_resplit.py new file mode 100644 index 0000000..190486f --- /dev/null +++ b/tests/test_rag_resplit.py @@ -0,0 +1,165 @@ +""" +Tests for deploy/rag_resplit.py, the one-shot migration that re-slices +compaction blocks already in the store. + +Imported by file path since deploy/ is an ops script, not part of the +installable forge package -- same as tests/test_podman_ro_proxy.py. + +What matters here is that a dry run writes nothing (it is the default, +so it is the mode that runs by accident) and that a real run does not +lose an entry it could not improve. +""" + +import importlib.util +import sys +from pathlib import Path + +import pytest + +from forge import rag + +_SCRIPT = Path(__file__).resolve().parents[1] / "deploy" / "rag_resplit.py" +_spec = importlib.util.spec_from_file_location("rag_resplit", _SCRIPT) +rag_resplit = importlib.util.module_from_spec(_spec) +sys.modules["rag_resplit"] = rag_resplit +_spec.loader.exec_module(rag_resplit) + + +_BLOCK = ( + "user: quel port utilise le serveur de test ?\n" + "assistant: le 8080\n" + "user: et sur quelle machine tourne Forge ?\n" + "assistant: sur le Steam Deck pour l'instant" +) + + +@pytest.fixture +def store(tmp_path, monkeypatch): + db = tmp_path / "rag.db" + monkeypatch.setattr(rag, "RAG_DB_FILE", str(db)) + monkeypatch.setattr(rag, "_embed", lambda text: [0.1] * rag.EMBEDDING_DIM) + conn = rag.get_connection() + rag.remember(conn, kind="history_summary", content=_BLOCK, project=None) + rag.remember(conn, kind="fact", content="Le NiPoGi a 32 Go de RAM", project=None) + rag.remember( + conn, + kind="history_summary", + content="user: une seule question\nassistant: une seule réponse", + project=None, + ) + conn.close() + return db + + +def _invoke(db, *flags): + argv = ["rag_resplit.py", "--db", str(db), *flags] + saved, sys.argv = sys.argv, argv + try: + return rag_resplit.main() + finally: + sys.argv = saved + + +def test_dry_run_writes_nothing(store, capsys): + assert _invoke(store) == 0 + + conn = rag.get_connection() + try: + assert rag.count_entries(conn)["by_kind"] == {"history_summary": 2, "fact": 1} + finally: + conn.close() + assert "dry run" in capsys.readouterr().out + + +def test_apply_replaces_a_block_with_its_exchanges(store): + assert _invoke(store, "--apply") == 0 + + conn = rag.get_connection() + try: + entries = rag.list_entries(conn, kind="history_summary") + contents = sorted(e["content"] for e in entries) + finally: + conn.close() + + assert contents == sorted( + [ + "user: quel port utilise le serveur de test ?\nassistant: le 8080", + ( + "user: et sur quelle machine tourne Forge ?\n" + "assistant: sur le Steam Deck pour l'instant" + ), + "user: une seule question\nassistant: une seule réponse", + ] + ) + + +def test_an_entry_that_is_already_one_exchange_keeps_its_id(store): + conn = rag.get_connection() + try: + before = { + e["content"]: e["id"] + for e in rag.list_entries(conn, kind="history_summary") + } + finally: + conn.close() + single = "user: une seule question\nassistant: une seule réponse" + + _invoke(store, "--apply") + + conn = rag.get_connection() + try: + after = { + e["content"]: e["id"] + for e in rag.list_entries(conn, kind="history_summary") + } + finally: + conn.close() + + # Rewriting it would spend an embedding call to produce the same + # row under a new id. + assert after[single] == before[single] + + +def test_other_kinds_are_left_alone(store): + _invoke(store, "--apply") + + conn = rag.get_connection() + try: + facts = rag.list_entries(conn, kind="fact") + finally: + conn.close() + + assert [f["content"] for f in facts] == ["Le NiPoGi a 32 Go de RAM"] + + +def test_the_vectors_follow_the_entries(store): + """ + forget() deletes from both tables; remember_many writes to both. A + migration that lost the correspondence would leave vectors search + can still match and list_entries can no longer show. + """ + _invoke(store, "--apply") + + conn = rag.get_connection() + try: + entry_ids = {e["id"] for e in rag.list_entries(conn)} + vector_ids = {r[0] for r in conn.execute("SELECT rowid FROM memory_vectors")} + finally: + conn.close() + + assert entry_ids == vector_ids + + +def test_a_missing_database_is_reported_not_created(tmp_path, capsys): + missing = tmp_path / "nope.db" + + assert _invoke(missing) == 1 + assert not missing.exists() + + +def test_backup_is_written_before_applying(store, tmp_path): + backup = tmp_path / "backup.db" + + _invoke(store, "--apply", "--backup", str(backup)) + + assert backup.exists() From 3fde4a462335479126d9ef4b6302d598a30cb17e Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:25:13 +0000 Subject: [PATCH 05/12] bench(rag): measure what burying a sentence in a block costs The intake change rests on a number that had never been measured directly. It was inferred, correctly but with two variables moving at once, from two readings against the real store on 2026-08-22: one question at 0.9386 from a history_summary containing it nearly word for word, another at 0.671 from a short fact. Different questions, different entries. Here only one thing moves. The same sentence is planted three ways -- alone, buried in a transcript stored as ONE entry, and the same transcript stored one entry per exchange -- and the same question is asked of each. buried minus split is what the change buys. It also prints the mechanism it depends on: how many chunks the buried form is averaged from. And it says out loud what a null result means -- that the dilution comes from somewhere other than the averaging, and the change must not be defended with this number. A harness that can only confirm is not a measurement. The figure it produces is a LOWER bound. A real evicted block is an order of magnitude longer than the one planted here, so its single vector is the mean of proportionally more unrelated subjects. tests/test_rag_dilution_bench.py exercises the plumbing with a stub embedder and asserts nothing about the numbers -- asserting on them would be the same mistake in the other direction. It exists because four measurement harnesses on this repository have now failed or measured something slightly beside the point, each costing a real round trip on the Deck to discover. A crash two hundred lines in wastes that trip; this catches it here. --- bench/rag_dilution.py | 356 +++++++++++++++++++++++++++++++ tests/test_rag_dilution_bench.py | 128 +++++++++++ 2 files changed, 484 insertions(+) create mode 100755 bench/rag_dilution.py create mode 100644 tests/test_rag_dilution_bench.py diff --git a/bench/rag_dilution.py b/bench/rag_dilution.py new file mode 100755 index 0000000..354b1d8 --- /dev/null +++ b/bench/rag_dilution.py @@ -0,0 +1,356 @@ +#!/usr/bin/env python3 +""" +What does burying a sentence in a compacted block cost, in distance? + +This is the measurement the intake change stands on, and it had never +been made directly. It was inferred, correctly but indirectly, from +two numbers taken against the real store on 2026-08-22: the question +"Quels outils as-tu accès ?" sat at 0.9386 from a history_summary that +contained it nearly word for word, while "Le serveur de test tourne +sur quel port ?" sat at 0.671 from a short fact. Two different +questions, two different entries -- a comparison with two variables +moving at once. + +Here only ONE thing moves. The same sentence is planted three ways and +asked the same question: + + alone the sentence as its own entry -- the best case + buried a transcript containing it, stored as ONE entry, which + is what compaction did until now + split the same transcript, stored one entry per exchange, + which is what compaction does now + +The number that matters is buried minus split. That is what the change +buys, and if it is not there, the change is wrong. + +WHY IT SHOULD BE THERE (the mechanism, so a null result is readable) + +The buried transcript is longer than EMBEDDING_MAX_CHARS, so rag._embed +splits it, embeds each chunk, and AVERAGES the chunk vectors into one. +The mean of a dozen unrelated subjects is close to no question in +particular. `split` makes the same number of embedding requests -- it +just keeps the results apart instead of collapsing them. + +A null result would mean the embedding model is not sensitive to +length here and the dilution is coming from somewhere else. Say so +rather than shipping the change on faith. + +THE NUMBER IS A LOWER BOUND + +The block planted here is a few thousand characters, a handful of +chunks. A block evicted by a real compaction pass -- 59 messages on +2026-08-19 -- is an order of magnitude longer, so its single vector is +the mean of proportionally more unrelated subjects. Whatever cost this +harness prints, the store pays more. + +RUNNING IT + +Needs the embedding server (forge-embedding), nothing else. + + podman exec forge sh -c 'rm -rf /tmp/arm && mkdir -p /tmp/arm' + podman cp src forge:/tmp/arm/ + podman cp bench/rag_dilution.py forge:/tmp/arm/ + podman exec -it forge python /tmp/arm/rag_dilution.py + +It writes to a SEPARATE database (--db, default /tmp/rag_dilution.db) +and never touches data/forge_rag.db, for the same reason +bench/recall_distance.py does not: planting fixtures in the real store +leaves them there for the next real recall. +""" + +from __future__ import annotations + +import argparse +import os +import sys + +# The sentence under test, and the question asked about it. Phrased so +# the question is not a copy of the sentence -- matching a paraphrase +# is the job, matching a copy is not. +NEEDLE = ( + "user: sur quel port tourne le serveur de test ?\n" + "assistant: il écoute sur le 8080, c'est réglé dans le compose" +) +QUESTION = "Le serveur de test utilise quel numéro de port ?" + +# Filler in the register the real store is full of: French, technical, +# about this project, and about anything BUT the needle. Filler that +# happened to mention ports would measure something else. +FILLER: list[tuple[str, str]] = [ + ( + "tu peux me rappeler pourquoi on est passés à podman ?", + ( + "pas de démon root, et les conteneurs rootless marchent sans configuration " + "supplémentaire sur le Deck" + ), + ), + ( + "le proxy D-Bus, il tourne dans le conteneur ou sur l'hôte ?", + ( + "sur l'hôte, en unité utilisateur systemd ; le conteneur ne fait que s'y " + "connecter par le socket monté" + ), + ), + ( + "pourquoi le prefill est si lent au premier tour ?", + ( + "le cache KV de llama-server est vide après un redémarrage, donc tout le " + "prompt routeur est recalculé, environ dix millisecondes par token" + ), + ), + ( + "on garde ARCHITECTURE.md à la racine ?", + "oui, c'est un engagement plutôt que de la doc sur le code, comme SECURITY.md", + ), + ( + "la grammaire GBNF accepte les underscores dans les noms de règles ?", + ( + "non, llama.cpp lexe les noms avec is_word_char qui ne prend que lettres, " + "chiffres et tirets, donc tout est en tirets" + ), + ), + ( + "qu'est-ce qui a cassé la résolution DNS des conteneurs ?", + ( + "un dns: nu dans le compose remplace tout le resolv.conf, il faut la forme à " + "deux serveurs sinon les noms de conteneurs ne résolvent plus" + ), + ), + ( + "pourquoi les commits doivent rester à ton nom ?", + ( + "une question de présentation du dépôt, pas d'ego ; l'identité git est fixée " + "avant chaque format-patch" + ), + ), + ( + "le tiroir, ça correspond à quoi côté code ?", + ( + "les messages marqués pinned dans l'historique, que la compaction n'évince " + "jamais" + ), + ), + ( + "on a tranché pour les tags git ?", + ( + "on ne rétro-tague pas les versions passées, on pose un tag sur l'état " + "courant après le merge et le trou reste visible" + ), + ), + ( + "l'agent sysadmin peut redémarrer un service ?", + ( + "non, il propose seulement ; la lecture des logs passe par un proxy read-only " + "et rien n'est appliqué automatiquement" + ), + ), + ( + "pourquoi /no_think reste dans les prompts des graphes ?", + ( + "mesuré deux fois : sans lui, recall recopie l'exemple GOOD ANSWER au lieu de " + "répondre, de façon déterministe" + ), + ), + ( + "le job de délégation survit à un redémarrage ?", + "il passe en interrupted au démarrage suivant, jamais de reprise automatique", + ), + ( + "pourquoi les patches plutôt qu'un bundle git ?", + ( + "depuis la réattribution des auteurs en local les hashes ont changé, donc un " + "bundle ne s'applique plus proprement sur le dépôt" + ), + ), + ( + "le tripwire au démarrage, il sert à quoi exactement ?", + ( + "à dire tout haut que files et test sont actifs ensemble, parce que cette " + "combinaison élargit ce qu'un conteneur joignable peut exécuter" + ), + ), + ( + "on met le seuil de compaction en messages à combien ?", + ( + "il est à 80 aujourd'hui, mais c'était un proxy du budget en tokens à une " + "époque où les tokens n'étaient pas mesurés" + ), + ), + ( + "l'action edit du tool files, pourquoi elle existe ?", + ( + "parce que le modèle ne chaîne pas read puis write de façon fiable, donc le " + "remplacement se fait en un seul dispatch" + ), + ), + ( + "qu'est-ce qui empêche une page web de choisir où on écrit ?", + ( + "la garde de provenance : le chemin est lu dans la décision de routage, " + "jamais dans la sortie d'un outil" + ), + ), + ( + "pourquoi la découverte sysadmin remonte cinq cents unités ?", + ( + "elle liste tout ce que busctl expose, y compris les .device, mais cette " + "liste n'entre pas dans le prompt de synthèse" + ), + ), + ( + "le badge de langage dans les blocs de code, il vient d'où ?", + ( + "du tag de la fence markdown, affiché tel quel sans table de jolis noms, " + "positionné en absolu dans le pre" + ), + ), + ( + "on a une CI ?", + ( + "ruff check, ruff format --check et pytest sur python 3.12, avec les deux " + "fichiers de requirements" + ), + ), + ( + "à quoi sert la jauge dans l'en-tête ?", + ( + "la barre est une prévision calculée sans appeler le modèle, l'infobulle " + "donne les compteurs mesurés du dernier tour" + ), + ), + ( + "pourquoi ne pas relancer le compose avec force-recreate ?", + ( + "ça recharge llama-server et vide le cache KV, donc le tour suivant repart " + "sur un prefill à froid" + ), + ), +] + +_MIN_USEFUL_GAIN = 0.05 + + +def _rank_of_needle(hits: list[dict]) -> int: + """ + Where the entry HOLDING the sentence lands, 1-based, or 0 if it is + not in the list at all. + + Rank and distance answer different questions and both are needed. + In the split shape the needle competes with a dozen sibling + exchanges, so a good distance at rank 7 would mean the cut helped + the vector and hurt the retrieval -- which the distance alone would + hide. + """ + for i, hit in enumerate(hits, start=1): + if NEEDLE in hit["content"]: + return i + return 0 + + +def _needle_distance(hits: list[dict]) -> float | None: + for hit in hits: + if NEEDLE in hit["content"]: + return hit["distance"] + return None + + +def _transcript(needle_at: int) -> list[str]: + """The filler as exchanges, with the needle inserted in the middle.""" + blocks = [f"user: {q}\nassistant: {a}" for q, a in FILLER] + blocks.insert(needle_at, NEEDLE) + return blocks + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--db", default="/tmp/rag_dilution.db") + parser.add_argument("--question", default=QUESTION) + args = parser.parse_args() + + if args.db.endswith("forge_rag.db"): + print("refusing to plant fixtures in the real store") + return 1 + + # Set before importing forge.rag: RAG_DB_FILE is read at import. + os.environ["RAG_DB_FILE"] = args.db + if os.path.exists(args.db): + os.remove(args.db) + + from forge import rag + from forge.config import EMBEDDING_MAX_CHARS + + blocks = _transcript(len(FILLER) // 2) + joined = "\n".join(blocks) + + conn = rag.get_connection() + try: + print(f"question : {args.question!r}\n") + print( + f"bloc de {len(blocks)} échanges, {len(joined)} caractères " + f"-- soit {-(-len(joined) // EMBEDDING_MAX_CHARS)} chunks moyennés " + f"en un seul vecteur dans la forme 'buried'\n" + ) + + try: + rag.remember(conn, kind="fact", content=NEEDLE, project="alone") + rag.remember(conn, kind="history_summary", content=joined, project="buried") + rag.remember_many( + conn, kind="history_summary", contents=blocks, project="split" + ) + except rag.EmbeddingError as e: + print(f"embedding server unreachable ({rag.EMBEDDING_URL}): {e}") + return 1 + + total = rag.count_entries(conn)["total"] + results = {} + for shape in ("alone", "buried", "split"): + hits = rag.search(conn, args.question, top_k=total, project=shape) + if not hits: + print(f"{shape}: no hit at all -- the store did not return the row") + return 1 + distance = _needle_distance(hits) + if distance is None: + print( + f"{shape}: the entry holding the sentence was not returned " + f"at all ({len(hits)} hits) -- nothing to compare" + ) + return 1 + results[shape] = (distance, _rank_of_needle(hits), len(hits)) + finally: + conn.close() + + print(f"{'forme':<10}{'distance':>10}{'rang du bloc qui contient la phrase':>40}") + labels = { + "alone": "seule", + "buried": "enfouie", + "split": "découpée", + } + for shape in ("alone", "buried", "split"): + distance, rank, n = results[shape] + print(f"{labels[shape]:<10}{distance:>10.4f}{f'{rank}/{n}':>40}") + + gain = results["buried"][0] - results["split"][0] + floor = results["alone"][0] + print(f"\ncoût de l'enfouissement (buried - split) : {gain:+.4f}") + print(f"plancher (la phrase seule) : {floor:.4f}") + + if gain < _MIN_USEFUL_GAIN: + print( + "\nPAS DE GAIN. Le découpage ne rapproche pas la phrase de la " + "question sur cette machine avec ce modèle d'embedding, donc la " + "dilution vient d'ailleurs que de la moyenne des chunks. Ne pas " + "défendre le changement avec ce chiffre." + ) + return 0 + + print( + "\nGAIN CONFIRMÉ. Le découpage récupère l'essentiel de l'écart entre " + "un bloc entier et la phrase seule ; c'est exactement ce que la " + "compaction par échange achète sur les entrées à venir, et ce que " + "deploy/rag_resplit.py achète sur celles déjà écrites." + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_rag_dilution_bench.py b/tests/test_rag_dilution_bench.py new file mode 100644 index 0000000..3811d17 --- /dev/null +++ b/tests/test_rag_dilution_bench.py @@ -0,0 +1,128 @@ +""" +The dilution harness has to RUN before it can measure anything. + +This exists because of a pattern already recorded four times on this +repository: a measurement harness that fails, or measures something +slightly beside the point, and costs a real round trip on the Deck to +find out. bench/rag_dilution.py is meant to be copied into the +container and run once against the live embedding server; a crash two +hundred lines in wastes that trip. + +So the plumbing is exercised here with a stub embedder -- the numbers +it produces are meaningless, and asserting on them would be the same +mistake in a different direction. What is asserted is that the three +shapes get planted, the needle is found in each, and the verdict path +is reached. +""" + +import importlib.util +import math +import sys +from pathlib import Path + +import pytest + +from forge import rag + +_SCRIPT = Path(__file__).resolve().parents[1] / "bench" / "rag_dilution.py" +_spec = importlib.util.spec_from_file_location("rag_dilution", _SCRIPT) +rag_dilution = importlib.util.module_from_spec(_spec) +sys.modules["rag_dilution"] = rag_dilution +_spec.loader.exec_module(rag_dilution) + + +def _bag_of_words_embedding(text: str) -> list[float]: + """ + A stand-in that at least varies with the text, so ranking is not + decided by insertion order. Not a model: a hashed word count, + L2-normalised the way llama-server normalises what it returns. + """ + vector = [0.0] * rag.EMBEDDING_DIM + for word in text.lower().split(): + vector[hash(word) % rag.EMBEDDING_DIM] += 1.0 + norm = math.sqrt(sum(v * v for v in vector)) + return [v / norm for v in vector] if norm else vector + + +@pytest.fixture +def stubbed(tmp_path, monkeypatch): + monkeypatch.setattr(rag, "RAG_DB_FILE", str(tmp_path / "dilution.db")) + monkeypatch.setattr(rag, "_embed", _bag_of_words_embedding) + return tmp_path / "dilution.db" + + +def _run(db, *flags): + argv = ["rag_dilution.py", "--db", str(db), *flags] + saved, sys.argv = sys.argv, argv + try: + return rag_dilution.main() + finally: + sys.argv = saved + + +def test_the_harness_runs_and_reaches_a_verdict(stubbed, capsys): + assert _run(stubbed) == 0 + + out = capsys.readouterr().out + assert "coût de l'enfouissement" in out + assert "plancher" in out + + +def test_all_three_shapes_are_planted(stubbed): + _run(stubbed) + + conn = rag.get_connection() + try: + projects = {e["project"] for e in rag.list_entries(conn, limit=100)} + finally: + conn.close() + + assert projects == {"alone", "buried", "split"} + + +def test_the_split_shape_holds_one_entry_per_exchange(stubbed): + _run(stubbed) + + conn = rag.get_connection() + try: + split = rag.list_entries(conn, project="split", limit=100) + buried = rag.list_entries(conn, project="buried", limit=100) + finally: + conn.close() + + assert len(split) == len(rag_dilution.FILLER) + 1 + assert len(buried) == 1 + + +def test_the_needle_is_findable_in_every_shape(stubbed): + """ + The comparison is only meaningful if the sentence is present three + times. _rank_of_needle returning 0 anywhere means the harness is + measuring something else. + """ + _run(stubbed) + + conn = rag.get_connection() + try: + for shape in ("alone", "buried", "split"): + entries = rag.list_entries(conn, project=shape, limit=100) + assert any(rag_dilution.NEEDLE in e["content"] for e in entries), shape + finally: + conn.close() + + +def test_it_refuses_to_plant_in_the_real_store(capsys): + assert _run("data/forge_rag.db") == 1 + assert "refusing" in capsys.readouterr().out + + +def test_the_question_is_not_a_copy_of_the_sentence(): + """ + Matching a paraphrase is the job; matching a copy measures the + string, not the embedding. The first --no-plant run of + bench/recall_distance.py went wrong in exactly this way. + """ + question_words = set(rag_dilution.QUESTION.lower().rstrip("?").split()) + needle_words = set(rag_dilution.NEEDLE.lower().split()) + + assert not question_words <= needle_words From f67df4d98b958e73fd5bdd73d15f050b1fc1c2ca Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:26:29 +0000 Subject: [PATCH 06/12] docs(memory): the intake is per exchange, and the store can be repaired Two things the memory page did not say. The compaction section still described rag_pointer as storing the evicted window as one entry, which stopped being true in this branch; and nothing anywhere mentioned that a store written before the change is still full of blocks, or how to fix one. Adds the numbers behind the change rather than asserting it is better, and points at the two harnesses and the migration. --- docs/memory.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/docs/memory.md b/docs/memory.md index 90d38f4..189bd6e 100644 --- a/docs/memory.md +++ b/docs/memory.md @@ -34,6 +34,20 @@ short pointer, searchable via `!recall`/`/search`) or `llm_summary` (one LLM cal block into prose kept inline). Both share the same signature, so switching is a config change, not a rewrite. `POST /compact` (or `!compact` in the REPL) forces a pass on demand. +**What `rag_pointer` writes is one entry per exchange, not one per block.** Until +v3.14 it joined the whole evicted window into a single string and stored it as one +entry — and since that string is far past `EMBEDDING_MAX_CHARS`, its one vector was +the *average* of a dozen chunk vectors, which is close to no question in particular. +Measured against the real store on 2026-08-22: a question sat at 0.9386 from the +compacted block containing it nearly word for word, while a short fact sat at 0.671. +`forge/transcript.py` cuts the window at user turns (a user turn plus whatever +answered it is the smallest unit that still stands on its own) and `rag.remember_many` +stores the pieces as rows — the same number of embedding requests, kept apart instead +of averaged. `bench/rag_dilution.py` measures the difference; `deploy/rag_resplit.py` +re-slices blocks written before the change. Two things never reach the store: an +earlier compaction pointer (a reference to another entry, which answers no question) +and a raw router-JSON envelope (unwrapped to its content). + A message count turned out to be the wrong unit, though, so v3.12 added a second trigger alongside it: compaction also fires when the rendered prompt crosses `COMPACTION_TOKEN_THRESHOLD` tokens, aiming to bring it back to @@ -111,6 +125,33 @@ predictable way: `!remember`/`!recall` print a one-line error instead of crashin REPL, `/remember`/`/search` return `502`, and the `memory` tool returns a `[error]` string the router treats as a normal (if unhelpful) tool result rather than a crash. +### Reading and repairing the store + +`search` was the only reader this store ever had, and it is semantic by construction — +you cannot ask it what is *in* there without already having a question. `GET /memory`, +`!memory [kind]` in the REPL and `!memory` in the web UI list entries directly, with no +embedding call at all, and report the breakdown by `kind`. `!forget ` / +`DELETE /memory/{id}` remove one entry from both tables. + +Two harnesses go with it, both writing to their own database and never to +`data/forge_rag.db`: + +```bash +# what distance a good hit sits at, on this box, with this embedding model +python bench/recall_distance.py + +# what burying a sentence in a compacted block costs +python bench/rag_dilution.py + +# one-shot: re-slice blocks written before the per-exchange intake +python deploy/rag_resplit.py # dry run, the default +python deploy/rag_resplit.py --apply --backup /tmp/forge_rag.db.bak +``` + +`rag_resplit` rewrites rows in place and there is no undo — take the backup. It inserts +the pieces before deleting the block, so an interrupted run leaves a visible duplicate +rather than a missing entry. + ## Execution Traces Every run appends a record to `TRACE_FILE` (default: `data/traces.jsonl`): From b92045d3c1b417d5698dd4c89f10af4ebcaad17a Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 17:20:08 +0000 Subject: [PATCH 07/12] fix(rag): one pipeline for both intake paths, and a measured cutoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous patch shared the CUTTING between compaction and the migration and left the FILTERING behind, so the disagreement moved instead of disappearing. The real migration on 2026-08-22 proved it: seven of the ten re-sliced entries began with a compaction pointer, so each produced a unit whose entire content is "[59 messages précédents compactés -- voir mémoire vectorielle #12]", and the old entry #9 shed a few units of raw router JSON. Roughly a dozen noise entries out of 278 -- exactly the class of drift the shared module existed to remove. forge/transcript.py now owns the whole sequence and both callers are the same three steps: units(messages) = blocks(indexable(messages)) split(text) = units(parse(text)) A test asserts that equality on one input. parse() is the real inverse of render() (a leading chunk with no role prefix keeps role None, and render writes it back without inventing a speaker), and the pointer builder and its regex move here too, since a detector that lives apart from what it detects is the same drift one level down. The pointer check no longer looks at the role: a block re-parsed by the migration can hand a pointer back under whatever role preceded it in the stored text, so the shape is the evidence, not the speaker. rag_resplit rewrites an entry when the pipeline returns something other than what is stored -- which covers a block that needs cutting AND an entry that only needs unwrapping. An entry left with nothing at all is REPORTED, not deleted: removing a row nobody asked to remove is not a migration's job, and !forget is one command away. RECALL_MAX_DISTANCE=0.95 in .env.example, the first measured value it has ever had. Six questions against a copy of the real store, before and after re-slicing: before hits 0.8934 0.671 0.9386 | misses 0.9891 0.9356 1.098 worst hit > best miss -> NO GAP after hits 0.4498 0.671 0.7695 | misses 1.0619 0.8337 1.0369 gap 0.0642 The intake change is what made a threshold choosable at all, and the short fact at 0.671 not moving is the control. 0.95 rather than the harness's 0.81 because the three misses are not one family: two are "nothing here is relevant", which is what this setting is for, while "Comment s'appelle mon chat ?" at 0.8337 is near only because the store literally contains "user: Bonjour Forge, comment je m'appelle ?". No number separates that cleanly, and one set to try leaves 0.04 of headroom above the worst real hit. The errors are not symmetric: a bad answer gets argued with, "je n'ai rien en mémoire" gets believed. The code default stays unset, so a deployment that has not measured its own store is not handed someone else's number. Two tests pin the .env.example value against the reasoning in config.py, and pin that the default is still off. --- .env.example | 44 ++++-- deploy/rag_resplit.py | 33 +++-- docs/memory.md | 11 +- src/forge/compaction.py | 66 +-------- src/forge/config.py | 43 +++++- src/forge/transcript.py | 206 +++++++++++++++++++-------- tests/test_compaction_intake.py | 8 +- tests/test_rag_resplit.py | 65 +++++++++ tests/test_recall_distance_cutoff.py | 43 ++++++ tests/test_transcript.py | 66 +++++++-- 10 files changed, 421 insertions(+), 164 deletions(-) diff --git a/.env.example b/.env.example index 43a58c8..801611a 100644 --- a/.env.example +++ b/.env.example @@ -163,23 +163,41 @@ # ever asked, so they compress the whole scale: hits and misses both # move closer together. # -# So the fixtures validate the mechanism, not the number. On the real -# store the number is nearer 0.96 -- from ONE pair, which is not enough -# to set it. Re-run --no-plant with at least three questions on each -# side (the harness now refuses fewer) before writing a value here. +# So the fixtures validate the mechanism, not the number. Measured +# again on 2026-08-22, six questions against a copy of the real store, +# before and after it was re-sliced one entry per exchange: # -# Read this before switching it on, though. The five rows behind the -# invented-causality answer of 2026-08-19 sat at 0.90 to 1.0015 -- -# INSIDE the range a real hit occupies. No cutoff that keeps real hits -# would have prevented that answer. This setting removes the case where -# nothing in the store is remotely relevant; it does nothing about -# several middling entries being welded together, which is the failure -# that actually happened and which is an intake problem. +# before hits 0.8934 0.671 0.9386 | misses 0.9891 0.9356 1.098 +# worst hit 0.9386 > best miss 0.9356 -> NO GAP +# after hits 0.4498 0.671 0.7695 | misses 1.0619 0.8337 1.0369 +# worst hit 0.7695 < best miss 0.8337 -> gap 0.0642 # -# Re-measure against a COPY of your real store before trusting it: +# The intake change is what made a threshold choosable at all. The +# short fact (0.671) did not move -- only the entries that had been +# buried in a compacted block did, which is the control. +# +# 0.95, not the midpoint the harness suggests. Two of the three misses +# are "nothing here is relevant" (1.0619, 1.0369) and that is what this +# setting is for; the third sits at 0.8337 only because the store +# contains a lexically similar sentence that does not answer the +# question, and no number separates that cleanly. Too high lets a bad +# answer through, which gets argued with; too low answers "je n'ai rien +# en mémoire" while holding the answer, which gets believed. +# +# recall.dropped logs every cut with its id and distance, so a value +# that bites in the wrong place shows up in the logs. +# +# Re-measure against a COPY of your own store before trusting it: # recall_distance.py --db /tmp/real_copy.db --no-plant \ # --hit "..." --miss "..." -# RECALL_MAX_DISTANCE=1.05 +# +# The one ACTIVE line in this file, deliberately. Everything else here +# is a commented default that also lives in config.py; this value does +# not, because it was measured against one specific store rather than +# derived from anything general. Unset (the config default) still means +# no filtering at all, so a deployment that has not measured its own +# store is not silently given someone else's number. +RECALL_MAX_DISTANCE=0.95 # --- Memory --------------------------------------------------------------- # MEMORY_ENABLED=true diff --git a/deploy/rag_resplit.py b/deploy/rag_resplit.py index a9fe08e..e3fb943 100755 --- a/deploy/rag_resplit.py +++ b/deploy/rag_resplit.py @@ -13,12 +13,18 @@ WHAT IT DOES -For every `history_summary` entry, forge.transcript.split() cuts the -stored text back into the units transcript.blocks() would produce -today. An entry that yields one unit is left alone -- it is already -the right shape, and rewriting it would burn an embedding call to -produce the same row with a new id. An entry that yields several is -replaced by that many entries. +For every `history_summary` entry, forge.transcript.split() puts the +stored text through the SAME pipeline compaction now uses on a live +eviction -- parse, drop what is not conversation, cut at user turns. +An entry that comes back unchanged is left alone: it is already the +right shape, and rewriting it would burn an embedding call to produce +the same row with a new id. Anything else is replaced by its units. + +The shared pipeline is the point. The first version of this script cut +the same way compaction did but skipped compaction's filtering, and +the 2026-08-22 migration wrote a dozen entries whose entire content is +"[59 messages précédents compactés -- voir mémoire vectorielle #12]", +plus some raw router JSON. Both paths now call one function. Nothing else is touched. `fact`, `decision` and `todo` entries are one statement each by construction; splitting them is not a thing that @@ -110,10 +116,19 @@ def main() -> int: targets = _all_of_kind(conn, args.kind) print(f"{len(targets)} {args.kind} entries to inspect\n") - split_count = new_count = 0 + split_count = new_count = inert_count = 0 for entry in targets: units = transcript.split(entry["content"]) - if len(units) < 2: + + if not units: + # A whole entry that is nothing but a pointer to + # another entry. Reported, not deleted: removing a row + # nobody asked to remove is not a migration's job, and + # `!forget ` is one command away. + inert_count += 1 + print(f"#{entry['id']:>4} inert rien d'indexable, laissée en place") + continue + if units == [entry["content"]]: continue split_count += 1 @@ -141,6 +156,8 @@ def main() -> int: print(f" -> #{ids[0]}-#{ids[-1]}, #{entry['id']} removed") after = rag.count_entries(conn) + if inert_count: + print(f"\n{inert_count} entries hold nothing indexable (see !forget)") print( f"\n{split_count} entries would become {new_count}" if not args.apply diff --git a/docs/memory.md b/docs/memory.md index 189bd6e..abf89c7 100644 --- a/docs/memory.md +++ b/docs/memory.md @@ -46,7 +46,16 @@ stores the pieces as rows — the same number of embedding requests, kept apart of averaged. `bench/rag_dilution.py` measures the difference; `deploy/rag_resplit.py` re-slices blocks written before the change. Two things never reach the store: an earlier compaction pointer (a reference to another entry, which answers no question) -and a raw router-JSON envelope (unwrapped to its content). +and a raw router-JSON envelope (unwrapped to its content). Both paths go through +the same `transcript.units` / `transcript.split` pipeline, so a live eviction and a +migration cannot filter differently. + +Measured on the real store, six questions before and after re-slicing: hits moved +from 0.8934 / 0.671 / 0.9386 to 0.4498 / 0.671 / 0.7695, turning a **NO GAP** +verdict (worst hit closer than the best miss) into a usable gap of 0.0642. The +short fact at 0.671 did not move — only the entries that had been buried did, which +is the control. That is what makes `RECALL_MAX_DISTANCE` a number one can choose; +see `.env.example` for the value and why it is not the harness's midpoint. A message count turned out to be the wrong unit, though, so v3.12 added a second trigger alongside it: compaction also fires when the rendered prompt diff --git a/src/forge/compaction.py b/src/forge/compaction.py index 1970de2..e305d2d 100644 --- a/src/forge/compaction.py +++ b/src/forge/compaction.py @@ -25,8 +25,6 @@ free to drift from the one that matters. """ -import re - from forge import rag, transcript from forge.config import ( COMPACTION_ENABLED, @@ -204,7 +202,7 @@ def _strategy_rag_pointer(messages: list[dict]) -> dict: forge/transcript.py for the numbers and for where the boundary falls. """ - contents = transcript.blocks(_indexable(messages)) + contents = transcript.units(messages) try: conn = rag.get_connection() @@ -230,71 +228,11 @@ def _strategy_rag_pointer(messages: list[dict]) -> dict: return { "id": messages[0]["id"], "role": "system", - "content": _pointer(len(messages), ids), + "content": transcript.pointer(len(messages), ids), "pinned": False, } -# What a pointer written by this strategy looks like, in the one form -# that has to be recognised again later -- see _indexable. -# -# Regex and builder are pinned together by a test rather than trusted -# to stay in sync. This is the same drift that bit router/grammar.py -# against router/prompt.py: two places stating the same string, one of -# them edited. -_POINTER_RE = re.compile(r"^\[\d+ messages précédents compactés") - - -def _pointer(message_count: int, ids: list[int]) -> str: - if not ids: - return ( - f"[{message_count} messages précédents compactés -- " - f"rien d'indexable, aucune entrée mémoire créée]" - ) - if len(ids) == 1: - where = f"#{ids[0]}" - else: - where = f"#{ids[0]}-#{ids[-1]} ({len(ids)} entrées)" - return ( - f"[{message_count} messages précédents compactés -- " - f"voir mémoire vectorielle {where}, cherchable via !recall]" - ) - - -def _indexable(messages: list[dict]) -> list[dict]: - """ - Drop what is not conversation, and unwrap what is wearing an - envelope, before any of it reaches the vector store. - - Both cases were found by reading the real store on 2026-08-22, - which is the first day anything could read it without asking it a - question (rag.list_entries). - - A previous compaction pointer is a reference to another entry. - Indexed, it becomes a memory whose entire content is the sentence - "N messages were compacted, see #12" -- it answers no question and - sits at middling distance from all of them. The block it points at - stays reachable through search; only the textual chain is not - rebuilt, which is the honest trade for not indexing a signpost as - if it were the road. - - Entry #9 of that store held raw router JSON -- {"tool": "code", - ...} -- swallowed from an assistant turn by an older version that - did not unwrap tool output. The envelope is the noise; the content - inside it is a real answer, so it is unwrapped rather than dropped. - """ - kept = [] - for m in messages: - content = (m.get("content") or "").strip() - if not content: - continue - if m.get("role") == "system" and _POINTER_RE.match(content): - continue - unwrapped = try_unwrap_router_json(content, "compaction") - kept.append({**m, "content": unwrapped if unwrapped is not None else content}) - return kept - - def _strategy_llm_summary(messages: list[dict]) -> dict: """ Alternative strategy: ask the chat LLM to condense the block into diff --git a/src/forge/config.py b/src/forge/config.py index 24812f7..74f8311 100644 --- a/src/forge/config.py +++ b/src/forge/config.py @@ -464,14 +464,43 @@ def _bool(name: str, default: str = "false") -> bool: # being welded into an invented causality. That one is upstream: the # store holds compaction pointers and almost no facts. # -# Still unset by default, and the first --no-plant run against a copy of -# the real store is why: a real MISS landed at 0.9891, below the 1.05 -# the fixtures suggested. Long compaction summaries sit at middling -# distance from every question ever asked and compress the whole scale -# -- so the fixtures validate the mechanism, not the number. On the real -# store it is nearer 0.96, from one pair, which is not enough to set it. +# The fixtures validate the MECHANISM, not the number. The first +# --no-plant run against a copy of the real store put a real MISS at +# 0.9891, below the 1.05 the fixtures suggested: long compaction +# summaries sit at middling distance from every question ever asked and +# compress the whole scale. # -# Unset means no filtering, i.e. exactly today's behaviour. +# MEASURED AGAIN on 2026-08-22, six questions against a copy of the real +# store, before and after it was re-sliced one entry per exchange: +# +# before hits 0.8934 0.671 0.9386 | misses 0.9891 0.9356 1.098 +# worst hit 0.9386 > best miss 0.9356 -> NO GAP +# after hits 0.4498 0.671 0.7695 | misses 1.0619 0.8337 1.0369 +# worst hit 0.7695 < best miss 0.8337 -> gap 0.0642 +# +# The intake change is what made a threshold choosable at all. Note the +# short fact (0.671) did not move: only the entries that were buried in +# a block did, which is the control. +# +# 0.95 AND NOT THE MIDPOINT. The harness suggests 0.81, halfway-plus. It +# is right about the arithmetic and the wrong tool for this store, +# because the three misses are not one family. Two are "nothing here is +# relevant" (1.0619, 1.0369), which is what this setting is for. The +# third -- "Comment s'appelle mon chat ?" at 0.8337 -- is near because +# the store now literally contains "user: Bonjour Forge, comment je +# m'appelle ?": lexically almost the same sentence, semantically not an +# answer. No threshold separates that cleanly, and one set to try leaves +# 0.04 of headroom above the worst real hit. +# +# The two errors are not symmetric. Too high lets a bad answer through, +# and a bad answer gets argued with. Too low produces "je n'ai rien en +# mémoire" while the answer is sitting in the store, and that gets +# believed. 0.95 cuts both real misses with ~0.18 of headroom and does +# not pretend to solve a near-duplicate by picking a number. +# +# recall.dropped logs every cut with its id and distance, so a value +# that bites in the wrong place is visible rather than silent. Empty +# means no filtering at all. _recall_max_distance = os.getenv("RECALL_MAX_DISTANCE", "").strip() RECALL_MAX_DISTANCE = float(_recall_max_distance) if _recall_max_distance else None diff --git a/src/forge/transcript.py b/src/forge/transcript.py index 4fb2f0e..25a2717 100644 --- a/src/forge/transcript.py +++ b/src/forge/transcript.py @@ -1,26 +1,36 @@ """ -One definition of how a conversation is written down, and where one -exchange ends. - -Two callers need the same answer to that question and, until this -module existed, were free to disagree. compaction.py renders evicted -messages and hands the text to the vector store; the migration in -deploy/rag_resplit.py has to take a block that was rendered weeks ago -and cut it the same way. A second copy of the boundary rule would put -the live path and the migration out of step, and the symptom would be -a store whose old and new entries are sliced differently -- invisible -from the outside, surfacing only as a retrieval distance nobody can -account for. +One definition of how a conversation is written down, where one +exchange ends, and what is worth indexing. + +Two callers need the same answers and, until this module existed, were +free to disagree. compaction.py renders evicted messages and hands the +text to the vector store; the migration in deploy/rag_resplit.py has to +take a block that was rendered weeks ago and treat it the same way. + +The first version of this module shared only the CUTTING, and the +disagreement moved rather than disappeared: compaction filtered out +compaction pointers and unwrapped router JSON before cutting, and the +migration did neither. The real migration on 2026-08-22 duly wrote a +dozen entries whose entire content is "[59 messages précédents +compactés -- voir mémoire vectorielle #12]", plus a handful of raw +router JSON out of the old entry #9. So the pipeline is shared here +whole, and the two entry points are now the same sequence: + + units(messages) = blocks(indexable(messages)) + split(text) = units(parse(text)) + +A test asserts that equality on the same input. Sharing one step out +of three is how the second step drifts. WHY AN EXCHANGE, AND NOT THE WHOLE BLOCK -Measured on the Deck against the real store on 2026-08-22. The -question "Quels outils as-tu accès ?" sits at distance 0.9386 from the -entry that answers it nearly word for word -- because that entry is a -whole compacted block. A short fact answering "Le serveur de test -tourne sur quel port ?" sits at 0.671. Burying a sentence in a long -block costs roughly 0.27 of distance, which is larger than the entire -gap that separated a hit from a miss that day (0.0422). +Measured on the Deck against the real store on 2026-08-22, then again +after this change. The question "Quels outils as-tu accès ?" sat at +0.9386 from the entry that answers it nearly word for word -- because +that entry was a whole compacted block. Re-sliced, the same question +against the same content sits at 0.7695. "Tu peux me lister mon +matériel" went from 0.8934 to 0.4498. bench/rag_dilution.py isolates +the effect on a planted sentence and puts it at 0.4450. The cause is in rag._embed: text past EMBEDDING_MAX_CHARS is split, each chunk embedded, and the chunk vectors AVERAGED into one. The mean @@ -36,17 +46,19 @@ user turn plus whatever answered it is the smallest unit that still stands on its own. -KNOWN LIMIT, accepted. `split` finds turn boundaries by looking for a +KNOWN LIMIT, accepted. `parse` finds turn boundaries by looking for a role prefix at the start of a line, so a message whose own content contains a line beginning with "user: " produces a boundary in the wrong place. Nothing is lost when that happens -- the text is still stored, in two pieces instead of one -- and the alternative (a -structured archive format) would not be readable by the blocks already -in the store, which is the case this has to serve. +structured archive format) could not read the blocks already in the +store, which is the case this has to serve. """ import re +from forge.text_cleaning import try_unwrap_router_json + # Every role forge.memory writes. Kept as one tuple because the regex # below and any future caller must agree on the closed set: a role # missing here is not a boundary, so its message silently joins the @@ -55,10 +67,111 @@ _SPEAKER = re.compile(rf"^({'|'.join(ROLES)}): ", re.MULTILINE) +# What a pointer left behind by compaction looks like, in the one form +# that has to be recognised again later -- see `indexable`. +# +# Builder and detector live side by side and a test pins them +# together. Two places stating the same string, one of them edited, is +# the drift that already bit router/grammar.py against +# router/prompt.py; here the failure is silent, because a pointer that +# stops matching gets indexed as if it were conversation. +POINTER_RE = re.compile(r"^\[\d+ messages précédents compactés") + + +def pointer(message_count: int, ids: list[int]) -> str: + """The history message left in place of an evicted block.""" + if not ids: + return ( + f"[{message_count} messages précédents compactés -- " + f"rien d'indexable, aucune entrée mémoire créée]" + ) + where = ( + f"#{ids[0]}" if len(ids) == 1 else f"#{ids[0]}-#{ids[-1]} ({len(ids)} entrées)" + ) + return ( + f"[{message_count} messages précédents compactés -- " + f"voir mémoire vectorielle {where}, cherchable via !recall]" + ) + def render(messages: list[dict]) -> str: - """Write messages down the way the store has always held them.""" - return "\n".join(f"{m['role']}: {m['content']}" for m in messages) + """ + Write messages down the way the store has always held them. + + A message with no role is written without a prefix, so text that + arrived without one does not come back with a speaker invented for + it. + """ + return "\n".join( + f"{m['role']}: {m['content']}" if m.get("role") else m["content"] + for m in messages + ) + + +def parse(text: str) -> list[dict]: + """ + Inverse of `render`: recover messages from text already written + down. Text before the first role prefix, or text with no prefix at + all, comes back as one message with role None. + """ + marks = list(_SPEAKER.finditer(text)) + if not marks: + return [{"role": None, "content": text}] if text.strip() else [] + + messages: list[dict] = [] + lead = text[: marks[0].start()] + if lead.strip(): + messages.append({"role": None, "content": lead.removesuffix("\n")}) + + for i, mark in enumerate(marks): + end = marks[i + 1].start() if i + 1 < len(marks) else len(text) + body = text[mark.end() : end] + # render() joins with exactly one "\n", so a message followed by + # another ends with that separator. Removing exactly one is the + # faithful inverse; stripping all of them would eat a blank line + # a message actually ended with. + messages.append({"role": mark.group(1), "content": body.removesuffix("\n")}) + + return messages + + +def indexable(messages: list[dict]) -> list[dict]: + """ + Drop what is not conversation, and unwrap what is wearing an + envelope, before any of it reaches the vector store. + + Both cases were found by reading the real store on 2026-08-22, the + first day anything could enumerate it without asking it a question. + + A previous compaction pointer is a reference to another entry. + Indexed, it becomes a memory whose entire content is "N messages + were compacted, see #12" -- it answers no question and sits at + middling distance from all of them. The block it points at stays + reachable through search; only the textual chain is not rebuilt, + which is the honest trade for not indexing a signpost as if it were + the road. + + Entry #9 of that store held raw router JSON -- {"tool": "code", + ...} -- swallowed from an assistant turn by an older version that + did not unwrap tool output. The envelope is the noise; the content + inside it is a real answer, so it is unwrapped rather than dropped. + + The pointer check does not look at the role. It did at first, and + that was a guess about who wrote it: a pointer reaching the + migration has been through render and parse, and a block whose + text starts mid-turn can hand it back under whatever role happened + to precede it. The shape is the evidence, not the speaker. + """ + kept = [] + for m in messages: + content = (m.get("content") or "").strip() + if not content: + continue + if POINTER_RE.match(content): + continue + unwrapped = try_unwrap_router_json(content, "compaction") + kept.append({**m, "content": unwrapped if unwrapped is not None else content}) + return kept def blocks(messages: list[dict]) -> list[str]: @@ -70,6 +183,8 @@ def blocks(messages: list[dict]) -> list[str]: unit of their own rather than being dropped or glued to the turn after them -- an evicted window does not necessarily begin on a user message. + + Grouping only, no filtering. `units` is what callers want. """ groups: list[list[dict]] = [] for m in messages: @@ -80,41 +195,14 @@ def blocks(messages: list[dict]) -> list[str]: return [render(g) for g in groups] -def split(text: str) -> list[str]: - """ - Cut already-rendered text into the same units `blocks` would have - produced from the messages it came from. +def units(messages: list[dict]) -> list[str]: + """What the vector store should hold for these messages.""" + return blocks(indexable(messages)) - This is the inverse used by the migration: entries written before - the split existed are one long string, and re-slicing them is the - only way the store's old half ends up shaped like its new half. - Text with no role prefix at all comes back as a single unit -- - something is in there, and refusing to return it would silently - delete an entry during a migration. Text before the first prefix - is returned as its own unit for the same reason. +def split(text: str) -> list[str]: """ - marks = list(_SPEAKER.finditer(text)) - if not marks: - return [text] if text.strip() else [] - - pieces: list[tuple[str, str]] = [] - lead = text[: marks[0].start()] - for i, mark in enumerate(marks): - end = marks[i + 1].start() if i + 1 < len(marks) else len(text) - pieces.append((mark.group(1), text[mark.start() : end])) - - units: list[str] = [] - if lead.strip(): - units.append(lead) - for role, piece in pieces: - if role == "user" or not units: - units.append(piece) - else: - units[-1] += piece - - # render() joins with exactly one "\n", so a unit that was followed - # by another ends with that separator. Removing exactly one newline - # is the faithful inverse; stripping all of them would eat a blank - # line a message actually ended with. - return [u.removesuffix("\n") for u in units] + The same thing, for text written down before any of this existed. + The migration's only entry point. + """ + return units(parse(text)) diff --git a/tests/test_compaction_intake.py b/tests/test_compaction_intake.py index 3ba2225..d8c9056 100644 --- a/tests/test_compaction_intake.py +++ b/tests/test_compaction_intake.py @@ -10,7 +10,7 @@ import pytest -from forge import compaction, rag +from forge import compaction, rag, transcript @pytest.fixture @@ -82,7 +82,7 @@ def test_the_pointer_is_recognisable_by_the_regex_that_looks_for_it(): nothing fails. """ for ids in ([], [7], [7, 8, 9]): - assert compaction._POINTER_RE.match(compaction._pointer(12, ids)) + assert transcript.POINTER_RE.match(transcript.pointer(12, ids)) def test_an_earlier_pointer_is_not_indexed_again(indexed): @@ -94,7 +94,7 @@ def test_an_earlier_pointer_is_not_indexed_again(indexed): """ compaction._strategy_rag_pointer( [ - _m("system", compaction._pointer(59, [12]), 1), + _m("system", transcript.pointer(59, [12]), 1), _m("user", "une question", 2), _m("assistant", "une réponse", 3), ] @@ -136,7 +136,7 @@ def test_empty_messages_are_not_indexed(indexed): def test_a_block_with_nothing_indexable_says_so(indexed): summary = compaction._strategy_rag_pointer( - [_m("system", compaction._pointer(59, [12]), 1)] + [_m("system", transcript.pointer(59, [12]), 1)] ) assert indexed == [] diff --git a/tests/test_rag_resplit.py b/tests/test_rag_resplit.py index 190486f..6becddb 100644 --- a/tests/test_rag_resplit.py +++ b/tests/test_rag_resplit.py @@ -163,3 +163,68 @@ def test_backup_is_written_before_applying(store, tmp_path): _invoke(store, "--apply", "--backup", str(backup)) assert backup.exists() + + +def test_an_entry_that_is_only_a_pointer_is_reported_not_deleted( + tmp_path, monkeypatch, capsys +): + """ + The migration of 2026-08-22 turned pointers into entries because + this script cut the same way compaction did but skipped its + filtering. It filters now -- and an entry left with nothing at all + is reported rather than removed: deleting a row nobody asked to + delete is not a migration's job. + """ + from forge import transcript + + monkeypatch.setattr(rag, "RAG_DB_FILE", str(tmp_path / "rag.db")) + monkeypatch.setattr(rag, "_embed", lambda text: [0.1] * rag.EMBEDDING_DIM) + conn = rag.get_connection() + rag.remember( + conn, + kind="history_summary", + content=f"system: {transcript.pointer(59, [12])}", + project=None, + ) + conn.close() + + _invoke(tmp_path / "rag.db", "--apply") + + conn = rag.get_connection() + try: + assert rag.count_entries(conn)["total"] == 1 + finally: + conn.close() + assert "inert" in capsys.readouterr().out + + +def test_a_pointer_inside_a_block_does_not_become_an_entry(tmp_path, monkeypatch): + from forge import transcript + + monkeypatch.setattr(rag, "RAG_DB_FILE", str(tmp_path / "rag.db")) + monkeypatch.setattr(rag, "_embed", lambda text: [0.1] * rag.EMBEDDING_DIM) + conn = rag.get_connection() + rag.remember( + conn, + kind="history_summary", + content=( + f"system: {transcript.pointer(59, [12])}\n" + "user: une question\nassistant: une réponse\n" + "user: une autre\nassistant: une autre réponse" + ), + project=None, + ) + conn.close() + + _invoke(tmp_path / "rag.db", "--apply") + + conn = rag.get_connection() + try: + contents = [e["content"] for e in rag.list_entries(conn)] + finally: + conn.close() + + assert contents == [ + "user: une autre\nassistant: une autre réponse", + "user: une question\nassistant: une réponse", + ] diff --git a/tests/test_recall_distance_cutoff.py b/tests/test_recall_distance_cutoff.py index 16898ac..80f46cd 100644 --- a/tests/test_recall_distance_cutoff.py +++ b/tests/test_recall_distance_cutoff.py @@ -89,3 +89,46 @@ def test_an_empty_env_value_disables_the_cutoff(value, monkeypatch): finally: monkeypatch.delenv("RECALL_MAX_DISTANCE", raising=False) importlib.reload(config) + + +def test_the_env_example_value_and_the_config_note_agree(): + """ + The threshold is now stated in two files -- an active line in + .env.example and the reasoning in config.py -- and two places + stating the same number is how one of them goes stale. This is the + same drift guard the pointer builder and its regex have. + """ + import re + from pathlib import Path + + root = Path(__file__).resolve().parents[1] + env = (root / ".env.example").read_text() + config = (root / "src" / "forge" / "config.py").read_text() + + active = re.search(r"^RECALL_MAX_DISTANCE=([\d.]+)$", env, re.MULTILINE) + assert active, ".env.example no longer sets RECALL_MAX_DISTANCE" + + value = active.group(1) + assert f"{value} AND NOT THE MIDPOINT" in config or f"{value}," in config, ( + f".env.example sets {value} but config.py does not explain that number" + ) + + +def test_the_cutoff_is_still_off_when_unset(): + """ + .env.example carries a value now; the code default must not. A + deployment that has not measured its own store gets no filtering + rather than someone else's threshold. + """ + import importlib + import os + + import forge.config as config_module + + saved = os.environ.pop("RECALL_MAX_DISTANCE", None) + try: + assert importlib.reload(config_module).RECALL_MAX_DISTANCE is None + finally: + if saved is not None: + os.environ["RECALL_MAX_DISTANCE"] = saved + importlib.reload(config_module) diff --git a/tests/test_transcript.py b/tests/test_transcript.py index c8a0572..48afd07 100644 --- a/tests/test_transcript.py +++ b/tests/test_transcript.py @@ -1,12 +1,16 @@ """ Unit tests for forge.transcript. -The test that matters here is the round trip: `split(render(msgs))` -must equal `blocks(msgs)`. Compaction takes the first path (messages --> units) and the migration in deploy/rag_resplit.py takes the second -(stored text -> units), and if the two ever disagree the store ends up -holding two differently-sliced halves -- which shows up as a retrieval -distance with no explanation, not as a failure. +The test that matters here is that the two entry points are the same +pipeline: `split(render(msgs))` must equal `units(msgs)`. Compaction +takes the first path (messages -> units) and the migration in +deploy/rag_resplit.py takes the second (stored text -> units). + +That equality is not hypothetical maintenance. The first version of +this module shared only the CUTTING, and the migration of 2026-08-22 +wrote a dozen entries whose entire content is a pointer to another +entry, plus some raw router JSON -- because compaction filtered those +out before cutting and the migration did not. """ from forge import transcript @@ -50,16 +54,62 @@ def test_messages_before_the_first_user_turn_are_their_own_unit(): ] -def test_split_round_trips_through_render(): +def test_the_two_entry_points_are_the_same_pipeline(): messages = [ _m("system", "ouverture"), _m("user", "quel port ?"), _m("assistant", "8080"), + _m("system", transcript.pointer(59, [12])), _m("user", "multi\nligne"), + _m( + "assistant", + '{"tool": "chat", "content": "une réponse assez longue ' + 'pour être considérée comme substantielle par le désenveloppage"}', + ), _m("assistant", "réponse\nsur deux lignes"), ] - assert transcript.split(transcript.render(messages)) == transcript.blocks(messages) + assert transcript.split(transcript.render(messages)) == transcript.units(messages) + + +def test_parse_is_the_inverse_of_render(): + messages = [ + _m("user", "quel port ?"), + _m("assistant", "8080\net rien d'autre"), + ] + + assert transcript.parse(transcript.render(messages)) == messages + + +def test_an_earlier_pointer_never_reaches_the_store(): + messages = [ + _m("system", transcript.pointer(59, [12])), + _m("user", "une question"), + _m("assistant", "une réponse"), + ] + + assert transcript.units(messages) == ["user: une question\nassistant: une réponse"] + + +def test_a_pointer_is_recognised_whatever_role_carries_it(): + """ + The check looks at the shape, not the speaker. A block re-parsed by + the migration can hand a pointer back under whatever role happened + to precede it in the stored text. + """ + assert transcript.units([_m("assistant", transcript.pointer(9, [3]))]) == [] + assert ( + transcript.units([{"role": None, "content": transcript.pointer(9, [3])}]) == [] + ) + + +def test_the_pointer_builder_matches_its_own_detector(): + for ids in ([], [7], [7, 8, 9]): + assert transcript.POINTER_RE.match(transcript.pointer(12, ids)) + + +def test_an_entry_that_holds_only_a_pointer_yields_nothing(): + assert transcript.split(f"system: {transcript.pointer(59, [12])}") == [] def test_split_keeps_text_that_has_no_role_prefix(): From c6ce7eca549b25d93fa5f1ce85356897be777d68 Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 17:38:17 +0000 Subject: [PATCH 08/12] fix(compaction): the intake log must say what was filtered, not just what was grouped compaction.indexed reported messages=35 indexed=17 entries=17, and that was read on 2026-08-22 as proof the filter had fired. It was not. Seventeen units out of thirty-five messages is what exchange grouping does on its own -- two messages at a time -- and the line carried no number that could distinguish a dropped pointer from an ordinary pair. A log line whose only job is to make an invisible step visible must not be ambiguous about which step it is showing. Filtering and grouping are now counted separately: messages, kept, dropped, units, entries. dropped > 0 is the filter, and nothing else is. transcript.indexable also takes a source label. The migration goes through the same function, so a real resplit run printed "compaction: model wrapped a substantive answer in router-style JSON" while no compaction was happening, about text written weeks earlier. It says "resplit" there now. --- src/forge/compaction.py | 12 ++++++++++-- src/forge/transcript.py | 18 ++++++++++++------ tests/test_compaction_intake.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/forge/compaction.py b/src/forge/compaction.py index e305d2d..1cdef13 100644 --- a/src/forge/compaction.py +++ b/src/forge/compaction.py @@ -202,7 +202,13 @@ def _strategy_rag_pointer(messages: list[dict]) -> dict: forge/transcript.py for the numbers and for where the boundary falls. """ - contents = transcript.units(messages) + # Filtering and grouping are counted separately because the log + # line below is the only place anyone can see either happen, and + # one number cannot answer both questions. 35 messages becoming 17 + # units says nothing about whether anything was filtered out -- + # exchanges group two messages at a time all on their own. + kept = transcript.indexable(messages) + contents = transcript.blocks(kept) try: conn = rag.get_connection() @@ -219,7 +225,9 @@ def _strategy_rag_pointer(messages: list[dict]) -> dict: log.event( "compaction.indexed", messages=len(messages), - indexed=len(contents), + kept=len(kept), + dropped=len(messages) - len(kept), + units=len(contents), entries=len(ids), first_id=ids[0] if ids else None, last_id=ids[-1] if ids else None, diff --git a/src/forge/transcript.py b/src/forge/transcript.py index 25a2717..d9dc2ba 100644 --- a/src/forge/transcript.py +++ b/src/forge/transcript.py @@ -135,7 +135,7 @@ def parse(text: str) -> list[dict]: return messages -def indexable(messages: list[dict]) -> list[dict]: +def indexable(messages: list[dict], source: str = "compaction") -> list[dict]: """ Drop what is not conversation, and unwrap what is wearing an envelope, before any of it reaches the vector store. @@ -156,6 +156,12 @@ def indexable(messages: list[dict]) -> list[dict]: did not unwrap tool output. The envelope is the noise; the content inside it is a real answer, so it is unwrapped rather than dropped. + `source` only labels the unwrap warning. The migration goes + through this same function, and a line reading "compaction: model + wrapped a substantive answer in router-style JSON" while a + migration is running names the wrong process for text written + weeks ago. + The pointer check does not look at the role. It did at first, and that was a guess about who wrote it: a pointer reaching the migration has been through render and parse, and a block whose @@ -169,7 +175,7 @@ def indexable(messages: list[dict]) -> list[dict]: continue if POINTER_RE.match(content): continue - unwrapped = try_unwrap_router_json(content, "compaction") + unwrapped = try_unwrap_router_json(content, source) kept.append({**m, "content": unwrapped if unwrapped is not None else content}) return kept @@ -195,14 +201,14 @@ def blocks(messages: list[dict]) -> list[str]: return [render(g) for g in groups] -def units(messages: list[dict]) -> list[str]: +def units(messages: list[dict], source: str = "compaction") -> list[str]: """What the vector store should hold for these messages.""" - return blocks(indexable(messages)) + return blocks(indexable(messages, source)) -def split(text: str) -> list[str]: +def split(text: str, source: str = "resplit") -> list[str]: """ The same thing, for text written down before any of this existed. The migration's only entry point. """ - return units(parse(text)) + return units(parse(text), source) diff --git a/tests/test_compaction_intake.py b/tests/test_compaction_intake.py index d8c9056..ecf4f1e 100644 --- a/tests/test_compaction_intake.py +++ b/tests/test_compaction_intake.py @@ -152,3 +152,34 @@ def _boom(conn, kind, contents, project): with pytest.raises(compaction.CompactionError): compaction._strategy_rag_pointer([_m("user", "une question", 1)]) + + +def test_the_log_separates_what_was_filtered_from_what_was_grouped( + indexed, monkeypatch +): + """ + 35 messages becoming 17 units says nothing about whether anything + was filtered -- exchanges group two messages at a time on their + own. A real run on 2026-08-22 was read as proof the filter had + fired when it was only proof that grouping had. + """ + events = [] + monkeypatch.setattr( + compaction.log, "event", lambda name, **kw: events.append((name, kw)) + ) + + compaction._strategy_rag_pointer( + [ + _m("system", transcript.pointer(59, [12]), 1), + _m("user", "une question", 2), + _m("assistant", "une réponse", 3), + _m("user", "une autre", 4), + _m("assistant", "une autre réponse", 5), + ] + ) + + _, fields = next(e for e in events if e[0] == "compaction.indexed") + assert fields["messages"] == 5 + assert fields["kept"] == 4 + assert fields["dropped"] == 1 + assert fields["units"] == 2 From 87e8b1f0d613019ce0bbebb2dd92d484d3714612 Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 20:59:03 +0000 Subject: [PATCH 09/12] fix(rag): the same exchange must not occupy three of five recall slots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A real recall run on the migrated store returned five entries and three of them were the same text, at 0.8306 each: "user: workspace / assistant: À quoi verras-tu que c'est fait ?", stored as #205, #227 and #233. Compaction blocks overlap, so an exchange that sat in the tail of one evicted window and the head of the next was indexed twice, and re-slicing turned that into three rows. top_k is five. Three of them went to one answer. remember_many now skips exact duplicates, both against what is stored and within the batch. Nothing is lost: the string is identical, so the surviving row answers every question the copies would have. Exact match only. A near-duplicate is a judgement with a threshold to tune and a way to be wrong; an identical string is a fact. Scoped to the project, because that is the namespace -- the same sentence under two projects is two statements about two things -- and compared with SQL "IS" rather than "=" so a NULL project matches a NULL project, which is every entry compaction writes. Not applied to remember(). A human asserting the same fact twice is saying they think it was forgotten; an archive holding the same exchange twice is redundancy nobody chose. rag_resplit gains the case this creates: an entry whose every unit is already stored elsewhere. It is reported and LEFT IN PLACE. Deleting a row because its content is redundant is a judgement, and this script does not make those. --- deploy/rag_resplit.py | 7 +++++ src/forge/rag.py | 38 ++++++++++++++++++++++++ tests/test_rag_batch.py | 65 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) diff --git a/deploy/rag_resplit.py b/deploy/rag_resplit.py index e3fb943..196d738 100755 --- a/deploy/rag_resplit.py +++ b/deploy/rag_resplit.py @@ -152,6 +152,13 @@ def main() -> int: print("stopping here -- entries already migrated are committed") return 1 + if not ids: + # Every unit was already in the store, under another + # entry. Left in place all the same: this script does + # not delete rows on a judgement about redundancy. + print(" -> every unit already stored, left in place") + continue + rag.forget(conn, entry["id"]) print(f" -> #{ids[0]}-#{ids[-1]}, #{entry['id']} removed") diff --git a/src/forge/rag.py b/src/forge/rag.py index ce5b479..fc1e694 100644 --- a/src/forge/rag.py +++ b/src/forge/rag.py @@ -251,18 +251,56 @@ def remember_many( half-indexed. A partially indexed block is worse than an unindexed one -- the pointer written into the history claims a range that does not hold what it says it holds. + + Exact duplicates are skipped, both against what is already stored + and within the batch itself. Compaction blocks overlap -- the real + store held the same exchange three times at distance 0.8306, + taking three of the five slots a recall query gets. Nothing is + lost by storing it once: the content is identical, so the + surviving row answers every question the copies would have. + + Only here, not in remember(). A human asserting the same fact + twice is saying something -- they think it was forgotten. An + archive holding the same exchange twice is redundancy nobody + chose. """ ids: list[int] = [] + seen: set[str] = set() for content in contents: if len(content.split()) < _MIN_ENTRY_WORDS: log.warning("rag: skipping a degenerate entry in a batch: %r", content) continue + if content in seen or _already_stored(conn, content, project): + log.event("rag.duplicate_skipped", chars=len(content)) + continue + seen.add(content) ids.append(_insert(conn, kind, content, project)) conn.commit() return ids +def _already_stored( + conn: sqlite3.Connection, content: str, project: str | None +) -> bool: + """ + Exact match, within the same project. A near-duplicate is a + judgement call with a threshold to tune; an identical string is a + fact. + + Scoped to the project because that is the namespace: the same + sentence filed under two projects is two statements about two + things, and deduplicating across them would silently drop one. + `IS` rather than `=` so a NULL project matches a NULL project, + which is every entry compaction writes. + """ + row = conn.execute( + "SELECT 1 FROM memory_entries WHERE content = ? AND project IS ? LIMIT 1", + (content, project), + ).fetchone() + return row is not None + + def _insert( conn: sqlite3.Connection, kind: str, content: str, project: str | None ) -> int: diff --git a/tests/test_rag_batch.py b/tests/test_rag_batch.py index c8f6d25..869d56d 100644 --- a/tests/test_rag_batch.py +++ b/tests/test_rag_batch.py @@ -109,3 +109,68 @@ def test_single_writes_still_commit_on_their_own(store): entry_id = rag.remember(store, kind="fact", content="un vrai fait", project=None) assert rag.list_entries(store)[0]["id"] == entry_id + + +def test_an_exact_duplicate_is_not_stored_twice(store): + """ + Compaction blocks overlap. The real store held the same exchange + three times at distance 0.8306 -- three of the five slots a recall + query gets, spent on one answer. + """ + rag.remember_many( + store, + kind="history_summary", + contents=["user: une question\nassistant: une réponse"], + project=None, + ) + + ids = rag.remember_many( + store, + kind="history_summary", + contents=[ + "user: une question\nassistant: une réponse", + "user: une autre\nassistant: une autre réponse", + ], + project=None, + ) + + assert len(ids) == 1 + assert rag.count_entries(store)["total"] == 2 + + +def test_duplicates_inside_one_batch_collapse(store): + ids = rag.remember_many( + store, + kind="history_summary", + contents=["user: la même chose", "user: la même chose"], + project=None, + ) + + assert len(ids) == 1 + + +def test_the_same_text_under_two_projects_is_two_entries(store): + """ + A project is a namespace. The same sentence filed under two of + them is two statements about two things. + """ + rag.remember_many( + store, kind="fact", contents=["le port est 8080"], project="alpha" + ) + ids = rag.remember_many( + store, kind="fact", contents=["le port est 8080"], project="beta" + ) + + assert len(ids) == 1 + assert rag.count_entries(store)["total"] == 2 + + +def test_remember_still_stores_a_repeated_assertion(store): + """ + Not deduplicated: a human asserting the same fact twice is saying + they think it was forgotten. + """ + rag.remember(store, kind="fact", content="Le NiPoGi a 32 Go", project=None) + rag.remember(store, kind="fact", content="Le NiPoGi a 32 Go", project=None) + + assert rag.count_entries(store)["total"] == 2 From e4c807453a2b9c07dd53a74a22b06e18c105ce4c Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sat, 22 Aug 2026 21:22:37 +0000 Subject: [PATCH 10/12] feat(memory): say what was stored, and flag a word the store has never seen Two changes to the same three lines, because they are the same omission. "Remembered (#305)." is a receipt for a transaction nobody can check. The entry that provoked this went in as "NiPoGi AM06PRO, pocresseur 5500U, 32Go de RAM" and the typo was found days later, by reading the store with a debugging tool that did not exist a week ago. The confirmation now echoes the stored text, its kind and project, and how many entries of that kind exist -- so the person who wrote it sees it while !forget is still one line away. Exactly the lesson files:write learned in v3.11, when a created file answered with a byte count and had to be opened by hand to see what was in it. A write that reports only that it happened hides what happened. The spelling check has one design decision in it: THE DICTIONARY IS THE STORE. A French spellchecker on this corpus is a machine for breaking identifiers -- NiPoGi, sqlite-vec, busctl, aardvark-dns, GBNF are precisely the tokens that carry the information, and a general dictionary corrects them towards common words. Vocabulary drawn from what has already been written knows those words because they were already used, and it sharpens with every entry instead of needing a maintained allow-list. difflib.get_close_matches at 0.85, the same tool sysadmin's target_missed already uses. It only ever SUGGESTS. The entry is stored, unmodified, whatever the check thinks. Silently rewriting a memory entry is the one place in Forge where being approximately right is worse than being wrong: nobody re-reads an entry, so it comes back weeks later as a fact with no trace that it was altered. Everywhere else a mistake is visible -- a bad file, a red test, a diagnosis the logs contradict. And asking the model to fix the spelling would be the v3.9 bug again, where a 9B quietly "corrects" a file it was asked to reproduce. Words under five letters are not checked (SSD, RAM, Go, PC are the vocabulary, not the typos) and the vocabulary read is bounded at 500 entries, since this runs on every write and the store grows without limit. --- src/forge/tools/memory.py | 115 +++++++++++++++++++++++++++++++++++++- tests/test_tool_memory.py | 99 ++++++++++++++++++++++++++++++-- 2 files changed, 207 insertions(+), 7 deletions(-) diff --git a/src/forge/tools/memory.py b/src/forge/tools/memory.py index 351dfd4..2317a34 100644 --- a/src/forge/tools/memory.py +++ b/src/forge/tools/memory.py @@ -44,7 +44,9 @@ ENABLED_TOOLS=chat,code,memory """ +import difflib import json +import re from forge import rag from forge.config import MEMORY_RECALL_MAX_CHARS @@ -66,6 +68,12 @@ # -- it comes back in search results and dominates them by sheer size. _ARCHIVE_KINDS = ("history_summary",) +# How much of the store to read when building the vocabulary. Bounded +# because this runs on every write and the store grows without limit: +# 500 entries is well past what any spelling suggestion needs, and the +# words that matter here are recent by construction. +_VOCABULARY_ENTRIES = 500 + def _remember(instruction: dict) -> str: kind = instruction.get("kind", "").strip().lower() or "fact" @@ -91,11 +99,114 @@ def _remember(instruction: dict) -> str: except rag.EmbeddingError as e: log.error("memory tool: remember failed: %s", e) return f"[error] remember failed: embedding server unreachable ({e})" + + total = rag.count_entries(conn)["by_kind"].get(kind, 1) + odd = _unfamiliar_words(conn, text) finally: conn.close() - log.event("memory.remember", entry_id=entry_id, kind=kind, project=project) - return f"Remembered (#{entry_id})." + log.event( + "memory.remember", + entry_id=entry_id, + kind=kind, + project=project, + unfamiliar=len(odd), + ) + return _confirmation(entry_id, kind, text, project, total, odd) + + +def _confirmation( + entry_id: int, + kind: str, + text: str, + project: str | None, + total: int, + odd: list[tuple[str, str]], +) -> str: + """ + Say what was stored, not that something was. + + "Remembered (#305)." is a receipt for a transaction nobody can + check. The entry that provoked this went in as "NiPoGi AM06PRO, + pocresseur 5500U, 32Go de RAM" and the typo was only found days + later, by reading the store with a debugging tool. Echoing the + stored text puts it in front of the person who wrote it while + !forget is still one line away. + + Same lesson as files:write, which used to answer with a byte count + until a created file had to be opened by hand to see what was in + it (v3.11). A write that reports only that it happened hides what + happened. + """ + where = f"/{project}" if project else "" + lines = [f"Noté (#{entry_id}, {kind}{where}) :", f" {text}"] + lines.append( + f"\n{total} entrée{'s' if total > 1 else ''} de type {kind} en mémoire." + ) + if odd: + lines.append(_spelling_note(odd)) + lines.append(f"Si c'est une faute : `!forget {entry_id}` puis réécris-la.") + return "\n".join(lines) + + +# A word shorter than this is not worth checking: "SSD", "RAM", "Go", +# "PC" are the vocabulary, not the typos, and difflib on three letters +# matches almost anything. +_MIN_WORD = 5 + +# How close a word has to be to an existing one to be worth +# mentioning. 0.85 on difflib's ratio is roughly "one or two +# characters out of eight" -- deliberately tight, because the cost of +# a false positive is a distracting line in every confirmation, and +# this feature is only ever a suggestion. +_CLOSE_ENOUGH = 0.85 + +_WORD_RE = re.compile(rf"[^\W\d_]{{{_MIN_WORD},}}", re.UNICODE) + + +def _unfamiliar_words(conn, text: str) -> list[tuple[str, str]]: + """ + Words in `text` that appear nowhere else in the store but sit one + or two characters from a word that does. + + The dictionary is THE STORE ITSELF, and that is the whole design. + A French spellchecker on this corpus is a machine for breaking + identifiers: NiPoGi, sqlite-vec, busctl, aardvark-dns, GBNF are + precisely the tokens that carry the information, and a general + dictionary corrects them towards common words. Vocabulary drawn + from what has already been written knows those words because they + were already used, and it gets sharper with every entry instead of + needing a maintained allow-list. + + It only ever SUGGESTS. Silently rewriting a memory entry is the + one place in Forge where being approximately right is worse than + being wrong -- nobody re-reads an entry, so it comes back weeks + later as a fact with no trace that it was altered. Everywhere else + a mistake is visible: a bad file, a red test, a diagnosis the logs + contradict. + + Returns pairs of (written, closest word already in the store). + """ + words = {w.lower() for w in _WORD_RE.findall(text)} + if not words: + return [] + + known: set[str] = set() + for entry in rag.list_entries(conn, limit=_VOCABULARY_ENTRIES): + known.update(w.lower() for w in _WORD_RE.findall(entry["content"])) + known -= words + + found = [] + for word in sorted(words): + near = difflib.get_close_matches(word, known, n=1, cutoff=_CLOSE_ENOUGH) + if near: + found.append((word, near[0])) + return found + + +def _spelling_note(odd: list[tuple[str, str]]) -> str: + pairs = ", ".join(f"« {written} » (proche de « {near} »)" for written, near in odd) + return f"\nJamais vu ailleurs en mémoire : {pairs}." def search( diff --git a/tests/test_tool_memory.py b/tests/test_tool_memory.py index 5892356..84e925d 100644 --- a/tests/test_tool_memory.py +++ b/tests/test_tool_memory.py @@ -26,7 +26,7 @@ def test_remember_stores_entry(): out = memory_tool.run( json.dumps({"action": "remember", "kind": "decision", "content": "use podman"}) ) - assert out == "Remembered (#1)." + assert out.startswith("Noté (#1") def test_remember_with_project(): @@ -40,7 +40,7 @@ def test_remember_with_project(): } ) ) - assert "Remembered" in out + assert out.startswith("Noté (#1") def test_remember_accepts_fact_kind(): @@ -49,7 +49,7 @@ def test_remember_accepts_fact_kind(): {"action": "remember", "kind": "fact", "content": "Possède un Steam Deck"} ) ) - assert out == "Remembered (#1)." + assert out.startswith("Noté (#1") def test_remember_defaults_to_fact_when_kind_missing(): @@ -60,7 +60,7 @@ def test_remember_defaults_to_fact_when_kind_missing(): out = memory_tool.run( json.dumps({"action": "remember", "content": "Possède un Steam Deck"}) ) - assert out == "Remembered (#1)." + assert out.startswith("Noté (#1") def test_remember_defaults_to_fact_when_kind_empty(): @@ -69,7 +69,7 @@ def test_remember_defaults_to_fact_when_kind_empty(): {"action": "remember", "kind": "", "content": "Possède un Steam Deck"} ) ) - assert out == "Remembered (#1)." + assert out.startswith("Noté (#1") def test_remember_rejects_invalid_kind(): @@ -229,3 +229,92 @@ def raise_error(text): with pytest.raises(rag.EmbeddingError): memory_tool.search("q") + + +def test_the_confirmation_echoes_what_was_stored(): + """ + "Remembered (#305)." is a receipt for a transaction nobody can + check. The entry that provoked this went in as "pocresseur 5500U" + and the typo surfaced days later, through a debugging tool. Same + lesson as files:write answering with a byte count until a created + file had to be opened by hand to see what was in it. + """ + out = memory_tool.run( + json.dumps( + { + "action": "remember", + "kind": "fact", + "content": "Le NiPoGi a 32 Go de RAM", + } + ) + ) + + assert "Le NiPoGi a 32 Go de RAM" in out + + +def test_a_word_close_to_one_already_stored_is_flagged(): + memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "processeur Ryzen 5500U"} + ) + ) + + out = memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "pocresseur AMD 5600G"} + ) + ) + + assert "pocresseur" in out + assert "processeur" in out + assert "!forget" in out + + +def test_a_genuinely_new_word_is_not_flagged(): + """ + The vocabulary is the store itself precisely so that identifiers + survive. A general dictionary would correct these towards common + words. + """ + memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "processeur Ryzen 5500U"} + ) + ) + + out = memory_tool.run( + json.dumps( + { + "action": "remember", + "kind": "fact", + "content": "Le proxy utilise aardvark-dns et busctl", + } + ) + ) + + assert "Jamais vu" not in out + + +def test_the_entry_is_stored_even_when_a_word_looks_odd(): + """Suggest, never rewrite and never refuse. Silently altering a + memory entry is the one place where approximately right is worse + than wrong: nobody re-reads it, so it returns weeks later as a + fact with no trace of the edit.""" + memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "processeur Ryzen 5500U"} + ) + ) + memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "pocresseur AMD 5600G"} + ) + ) + + conn = rag.get_connection() + try: + contents = [e["content"] for e in rag.list_entries(conn)] + finally: + conn.close() + + assert "pocresseur AMD 5600G" in contents From 90c81ec2196267705fd96e64d964b8c3570f21a0 Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sun, 23 Aug 2026 08:12:57 +0000 Subject: [PATCH 11/12] fix(memory): a word the store already uses is familiar, and the new row is not the dictionary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the spelling check, both found on the first real run, both in the same three lines. "J'utilise aardvark-dns pour la résolution DNS" was flagged twice, on `utilise` and `résolution` -- two words written a dozen times in that store. The check subtracted the whole new text from the vocabulary before searching. That was meant to stop a word matching itself; it also deleted the exact hit proving the word was fine, so each one fell through to a near neighbour. The word is now looked up first: present in the store means familiar, full stop, and no neighbour of it is worth a line. Which then exposed the second: rag.remember commits the row before the check runs, so the entry is its own dictionary and every word in it is "already in the store" -- because we just put it there. The vocabulary now skips that id. Both directions are pinned by a test, since either one alone makes the feature silently do nothing or silently cry wolf. Noted from the same run, not fixed here: the router rewrote `pocresseur` to `processeur` on its way to the tool, so a typo typed into the chat may never reach the store at all. Only the paths that bypass the model (!remember, direct calls) carry one through -- and those do not go through this tool, so they get neither the check nor the echo. Whether to move both down into rag.remember is a separate decision: it would put UI strings in the storage layer. --- src/forge/tools/memory.py | 22 +++++++++++++--- tests/test_tool_memory.py | 53 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/forge/tools/memory.py b/src/forge/tools/memory.py index 2317a34..8e0d1da 100644 --- a/src/forge/tools/memory.py +++ b/src/forge/tools/memory.py @@ -101,7 +101,7 @@ def _remember(instruction: dict) -> str: return f"[error] remember failed: embedding server unreachable ({e})" total = rag.count_entries(conn)["by_kind"].get(kind, 1) - odd = _unfamiliar_words(conn, text) + odd = _unfamiliar_words(conn, text, entry_id) finally: conn.close() @@ -164,7 +164,7 @@ def _confirmation( _WORD_RE = re.compile(rf"[^\W\d_]{{{_MIN_WORD},}}", re.UNICODE) -def _unfamiliar_words(conn, text: str) -> list[tuple[str, str]]: +def _unfamiliar_words(conn, text: str, entry_id: int) -> list[tuple[str, str]]: """ Words in `text` that appear nowhere else in the store but sit one or two characters from a word that does. @@ -191,13 +191,29 @@ def _unfamiliar_words(conn, text: str) -> list[tuple[str, str]]: if not words: return [] + # The entry being confirmed is already committed by the time this + # runs, so it has to be excluded by id -- otherwise every word in + # it is "already in the store", which it is, because we just put it + # there. known: set[str] = set() for entry in rag.list_entries(conn, limit=_VOCABULARY_ENTRIES): + if entry["id"] == entry_id: + continue known.update(w.lower() for w in _WORD_RE.findall(entry["content"])) - known -= words found = [] for word in sorted(words): + # A word the store already uses is familiar, full stop, and no + # neighbour of it is worth mentioning. The first version + # subtracted the whole new text from the vocabulary before + # searching -- meant to stop a word matching itself, it also + # deleted the evidence that the word was fine. "J'utilise + # aardvark-dns pour la résolution DNS" was flagged twice on + # `utilise` and `résolution`, two words written a dozen times + # in that store, each matched against a near neighbour only + # because the exact hit had just been removed. + if word in known: + continue near = difflib.get_close_matches(word, known, n=1, cutoff=_CLOSE_ENOUGH) if near: found.append((word, near[0])) diff --git a/tests/test_tool_memory.py b/tests/test_tool_memory.py index 84e925d..a5f6d48 100644 --- a/tests/test_tool_memory.py +++ b/tests/test_tool_memory.py @@ -318,3 +318,56 @@ def test_the_entry_is_stored_even_when_a_word_looks_odd(): conn.close() assert "pocresseur AMD 5600G" in contents + + +def test_a_word_the_store_already_uses_is_never_flagged(): + """ + Found in real use: "J'utilise aardvark-dns pour la résolution DNS" + was flagged twice, on `utilise` and `résolution` -- two words + written a dozen times in that store. The check removed the whole + new text from the vocabulary before searching, so the exact hit + that proved each word was fine had been deleted, and each matched + a near neighbour instead. + """ + memory_tool.run( + json.dumps( + { + "action": "remember", + "kind": "fact", + "content": "La résolution DNS utilise aardvark", + } + ) + ) + + out = memory_tool.run( + json.dumps( + { + "action": "remember", + "kind": "fact", + "content": "La résolution des noms utilise le proxy", + } + ) + ) + + assert "Jamais vu" not in out + + +def test_the_entry_being_confirmed_is_not_its_own_dictionary(): + """ + The row is committed before the check runs, so without excluding + it every word in the new text is "already in the store" -- because + we just put it there -- and nothing is ever flagged. + """ + memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "processeur Ryzen 5500U"} + ) + ) + + out = memory_tool.run( + json.dumps( + {"action": "remember", "kind": "fact", "content": "pocresseur 5600G"} + ) + ) + + assert "pocresseur" in out and "processeur" in out From 8b0c9f0f21d0fef14b973da1c77209907d407203 Mon Sep 17 00:00:00 2001 From: Kurtisone <104103601+Kurtisone@users.noreply.github.com> Date: Sun, 23 Aug 2026 08:31:18 +0000 Subject: [PATCH 12/12] feat(ui): !forget works where the message that recommends it is displayed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The confirmation printed after storing a fact ends with "Si c'est une faute : `!forget 312` puis réécris-la". That command only existed in the REPL, so typing it in the web UI went through the ! interception, found nothing, and printed "commande inconnue". The one action the message asks for was the one action the reader could not take. DELETE /memory/{id} has existed since fix/dettes-ouvertes; only the UI binding was missing. !forget is also the first UI command that takes an argument, so runUiCommand now splits the rest of the line and passes it to run() -- a dispatcher calling run() with nothing would have left it answering "usage" forever. changed:false deliberately. Forgetting a memory entry does not touch the conversation, so refreshing the chat would only erase the command and its own answer, which is the bug patch 0020 of fix/dettes-ouvertes was written to fix. A test pins the UI binding against the tool message: if the confirmation stops suggesting !forget, or the UI stops offering it, one of the two has moved without the other. --- src/forge/static/index.html | 34 ++++++++++++++++++++++++++++++++-- tests/test_memory_listing.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/forge/static/index.html b/src/forge/static/index.html index 004b40f..4481357 100644 --- a/src/forge/static/index.html +++ b/src/forge/static/index.html @@ -855,6 +855,34 @@ } } }, + '!forget': { + help: 'supprime une entrée de la mémoire vectorielle : !forget 312', + // Takes an argument, unlike the other three. The confirmation + // printed after storing a fact ends with "!forget 312 puis + // réécris-la" -- and until now that line named a command the + // person reading it could not run, because it only existed in the + // REPL. A message that tells you what to do next has to be + // runnable where it is displayed. + run: async (args) => { + const id = (args[0] || '').trim() + if (!/^\d+$/.test(id)) { + return { reply: 'Usage : !forget — l\'id est un nombre, visible dans !memory.', changed: false } + } + const r = await apiFetch(`/memory/${id}`, { method: 'DELETE' }) + if (r.status === 404) { + return { reply: `Aucune entrée #${id} en mémoire.`, changed: false } + } + await r.json() + return { + // changed:false on purpose. Forgetting a memory entry does not + // touch the conversation, so reloading the chat would only + // erase the command and its answer -- the bug fixed in 0020 of + // fix/dettes-ouvertes. + reply: `Entrée #${id} supprimée de la mémoire.`, + changed: false + } + } + }, '!compact': { help: 'force une passe de compaction (bouton « Compacter » aussi)', run: async () => { @@ -871,7 +899,9 @@ } async function runUiCommand(text) { - const name = text.split(/\s+/)[0].toLowerCase() + const parts = text.trim().split(/\s+/) + const name = parts[0].toLowerCase() + const args = parts.slice(1) const command = UI_COMMANDS[name] if (!command) { @@ -908,7 +938,7 @@ // triggered the refresh that erases every earlier local reply. let reply, changed = false, cls = 'bot' try { - const result = await command.run() + const result = await command.run(args) reply = result.reply changed = result.changed === true } catch (e) { diff --git a/tests/test_memory_listing.py b/tests/test_memory_listing.py index ca5f446..f2fed1f 100644 --- a/tests/test_memory_listing.py +++ b/tests/test_memory_listing.py @@ -97,3 +97,38 @@ def test_the_endpoint_and_both_front_ends_exist(): assert "'!memory'" in (root / "static" / "index.html").read_text(), ( "the web UI has no !memory -- which is where the dead end happened" ) + + +def test_the_web_ui_can_run_the_command_its_own_message_recommends(): + """ + The confirmation printed after storing a fact ends with "!forget + 312 puis réécris-la". Until this was added, that line named a + command that only existed in the REPL -- so from the web UI it + went to the router, which had no idea what it was. A message + telling you what to do next has to be runnable where it is + displayed. + """ + from pathlib import Path + + ui = ( + Path(__file__).resolve().parents[1] / "src" / "forge" / "static" / "index.html" + ).read_text() + tool = ( + Path(__file__).resolve().parents[1] / "src" / "forge" / "tools" / "memory.py" + ).read_text() + + assert "'!forget'" in ui + assert "!forget" in tool, "the confirmation no longer suggests !forget" + + +def test_the_ui_command_dispatcher_passes_arguments(): + """!forget is the first UI command that takes one. A dispatcher + calling run() with nothing would leave it permanently answering + 'usage'.""" + from pathlib import Path + + ui = ( + Path(__file__).resolve().parents[1] / "src" / "forge" / "static" / "index.html" + ).read_text() + + assert "command.run(args)" in ui