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/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/deploy/rag_resplit.py b/deploy/rag_resplit.py new file mode 100755 index 0000000..196d738 --- /dev/null +++ b/deploy/rag_resplit.py @@ -0,0 +1,201 @@ +#!/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() 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 +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 = inert_count = 0 + for entry in targets: + units = transcript.split(entry["content"]) + + 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 + 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 + + 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") + + 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 + 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/docs/memory.md b/docs/memory.md index 90d38f4..abf89c7 100644 --- a/docs/memory.md +++ b/docs/memory.md @@ -34,6 +34,29 @@ 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). 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 crosses `COMPACTION_TOKEN_THRESHOLD` tokens, aiming to bring it back to @@ -111,6 +134,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`): diff --git a/src/forge/compaction.py b/src/forge/compaction.py index e069323..1cdef13 100644 --- a/src/forge/compaction.py +++ b/src/forge/compaction.py @@ -25,7 +25,7 @@ free to drift from the one that matters. """ -from forge import rag +from forge import rag, transcript from forge.config import ( COMPACTION_ENABLED, COMPACTION_KEEP_RECENT, @@ -190,18 +190,31 @@ 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) + # 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() 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,13 +222,21 @@ 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), + 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, + ) + 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": transcript.pointer(len(messages), ids), "pinned": False, } @@ -230,7 +251,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/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/rag.py b/src/forge/rag.py index 87bb846..fc1e694 100644 --- a/src/forge/rag.py +++ b/src/forge/rag.py @@ -216,6 +216,96 @@ 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. + + 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: + """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 +317,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/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/src/forge/tools/memory.py b/src/forge/tools/memory.py index 351dfd4..8e0d1da 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,130 @@ 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, entry_id) 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, 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. + + 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 [] + + # 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"])) + + 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])) + 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/src/forge/transcript.py b/src/forge/transcript.py new file mode 100644 index 0000000..d9dc2ba --- /dev/null +++ b/src/forge/transcript.py @@ -0,0 +1,214 @@ +""" +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, 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 +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. `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) 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 +# previous turn. +ROLES = ("user", "assistant", "system") + +_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. + + 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], 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. + + 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. + + `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 + 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, source) + kept.append({**m, "content": unwrapped if unwrapped is not None else content}) + return kept + + +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. + + Grouping only, no filtering. `units` is what callers want. + """ + 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 units(messages: list[dict], source: str = "compaction") -> list[str]: + """What the vector store should hold for these messages.""" + return blocks(indexable(messages, source)) + + +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), source) 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..ecf4f1e --- /dev/null +++ b/tests/test_compaction_intake.py @@ -0,0 +1,185 @@ +""" +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, transcript + + +@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 transcript.POINTER_RE.match(transcript.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", transcript.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", transcript.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)]) + + +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 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): 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 diff --git a/tests/test_rag_batch.py b/tests/test_rag_batch.py new file mode 100644 index 0000000..869d56d --- /dev/null +++ b/tests/test_rag_batch.py @@ -0,0 +1,176 @@ +""" +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 + + +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 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 diff --git a/tests/test_rag_resplit.py b/tests/test_rag_resplit.py new file mode 100644 index 0000000..6becddb --- /dev/null +++ b/tests/test_rag_resplit.py @@ -0,0 +1,230 @@ +""" +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() + + +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_tool_memory.py b/tests/test_tool_memory.py index 5892356..a5f6d48 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,145 @@ 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 + + +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 diff --git a/tests/test_transcript.py b/tests/test_transcript.py new file mode 100644 index 0000000..48afd07 --- /dev/null +++ b/tests/test_transcript.py @@ -0,0 +1,134 @@ +""" +Unit tests for forge.transcript. + +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 + + +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_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.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(): + # 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]