From ca4b9558124d0c824a1442a865c8e3b23a6235d1 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 4 Aug 2026 13:17:36 +0200 Subject: [PATCH 1/4] refactor(memory): let any caller turn a question into a vault search The rewrite that turns "What do we still need to buy for the camping trip?" into keywords lived in the archivist, so a family question answered in chat and returned nothing from `stack memory search`. It now lives with the vault it searches, in memory.lib, and takes the caller's LLM rather than opening one. The archivist keeps what is its own: the `?` trigger, the Whoosh rendering for Paperless, and the "Searched for: ..." line. Refs FAM-21 --- stacklets/docs/bot/pipeline.py | 123 ++------ stacklets/docs/bot/recall.py | 14 +- stacklets/memory/lib.py | 160 +++++++++- tests/stacklets/test_memory_host_stdlib.py | 41 ++- tests/stacklets/test_memory_query_rewrite.py | 306 +++++++++++++++++++ tests/stacklets/test_pipeline.py | 94 +----- 6 files changed, 542 insertions(+), 196 deletions(-) create mode 100644 tests/stacklets/test_memory_query_rewrite.py diff --git a/stacklets/docs/bot/pipeline.py b/stacklets/docs/bot/pipeline.py index 0ba4c19..f4850a5 100644 --- a/stacklets/docs/bot/pipeline.py +++ b/stacklets/docs/bot/pipeline.py @@ -28,8 +28,10 @@ import asyncio import json import re +import sys from dataclasses import dataclass, field from datetime import date +from pathlib import Path from typing import TYPE_CHECKING, Any import aiohttp @@ -57,6 +59,18 @@ ModelCapabilities, ) +# Sibling stacklets resolve through `stacklets/` on sys.path: in the +# bot-runner container that directory is the read-only mount, locally it +# is the source tree. archivist.py wires the same path for its own +# imports, but it imports this module first, so the path goes on here +# rather than relying on who loads whom. +_STACKLETS_DIR = Path(__file__).resolve().parents[2] +if str(_STACKLETS_DIR) not in sys.path: + sys.path.insert(0, str(_STACKLETS_DIR)) +# The recall prompt lives with the vault it searches. See +# `memory.lib`'s query-rewrite section for why it moved there. +from memory.lib import rewrite_query as memory_rewrite_query # noqa: E402 + # Back-compat alias — the docs pipeline shipped with `ImageAttachment` # before the framework introduced `LLMImage`. The fields are identical # so callers stay duck-typed. @@ -791,35 +805,19 @@ async def rewrite_query( ) -> list[str]: """Extract search keywords from a natural-language question. - Used by the archivist when a message ends with `?`. The LLM - reads the family's topic + doctype ontology and produces 2-4 - keywords that would literally appear in a matching document -- - translation and synonym expansion happen here, so the regex - walker downstream stays dumb. - - Best-effort: any LLM transport failure or parse error returns - an empty list, which the caller treats as "no rewrite, search - the question verbatim." Synonym selection is the LLM's job; - we don't second-guess it here, but we do strip empties and - cap the list length so a chatty model can't blow the regex up. + Used by the archivist when a message ends with `?`. The prompt, + the parsing, and the best-effort contract live in + `memory.lib.rewrite_query`, because memory owns the vault those + keywords are aimed at and every other caller of that vault needs + the same hop. What the archivist contributes is the LLM it + already built, which is all memory was missing. """ - prompt = _build_rewrite_prompt(question, ontology_section, lang) - try: - raw = await self._llm.complete("recall", prompt, json_mode=True) - except (LLMUnavailableError, LLMModelNotFoundError, LLMTimeoutError) as e: - logger.warning("[recall] LLM unavailable for rewrite: {}", e) - return [] - keywords = _parse_rewrite_response(raw) - if not keywords: - # Surface the raw payload so an empty keyword list is - # debuggable: an off-shape JSON response is a prompt or - # model issue, not a transport one, and we can't fix it - # blind. - logger.warning( - "[recall] rewrite parse produced no keywords; raw={!r}", - (raw or "")[:200], - ) - return keywords + return await memory_rewrite_query( + question, + llm=self._llm, + ontology_section=ontology_section, + language=lang, + ) async def synthesize_answer( self, @@ -1218,75 +1216,6 @@ def _build_reformat_prompt(ocr_text: str) -> str: ---""" -# ── Query rewrite ──────────────────────────────────────────────────────── -# -# The recall-mode entry point. When a family member asks a question -# (anything ending in `?`), the archivist asks the LLM to extract 2-4 -# keywords that would literally appear in a matching document. The -# regex walker then OR-alternates them into a single search pattern. -# Two wins: (1) "When did Bart get vaccinated?" becomes a search for -# Impfung/MMR/Auffrischung, which actually hits the German vaccination -# record; (2) the ontology block primes synonym + translation knowledge -# without us having to ship a thesaurus. - -def _build_rewrite_prompt( - question: str, ontology_section: str, lang: str, -) -> str: - """Recall-mode prompt: question → JSON list of search keywords.""" - return f"""You extract search keywords from a question, so a regex walker can look up family documents. - -The family classifies their documents under these topics and forms: - -{ontology_section} - -Language hint: {lang}. -- If the question is in German, prefer German keywords. -- If it is in English, prefer English keywords. -- For an ambiguous or generic question, include the most likely topic name plus one or two synonyms in the document language. - -Question: {question} - -Reply with a JSON object: {{"keywords": ["word1", "word2", "word3"]}}. -2 to 4 keywords. Each keyword is a literal word that would appear in -the document (a noun, a name, a topic). No phrases, no quotes, no -prose around the JSON. Output ONLY the JSON object.""" - - -def _parse_rewrite_response(raw: str) -> list[str]: - """Pull a keyword list out of the rewrite LLM's response. - - Accepts the requested object form `{"keywords": [...]}` and the - bare-array fallback that some smaller models produce when they - forget the wrapper. Empty list on any parse failure -- the caller - treats empty as "no rewrite, search the question verbatim." Caps - at 6 entries so a chatty model can't blow up the alternation - regex; trims whitespace and drops empties so a stray `""` doesn't - poison the join. - """ - try: - data = json.loads(raw) - except (ValueError, TypeError): - return [] - - if isinstance(data, dict): - v = data.get("keywords") - candidates = v if isinstance(v, list) else [] - elif isinstance(data, list): - candidates = data - else: - candidates = [] - - cleaned: list[str] = [] - for x in candidates: - if isinstance(x, (str, int, float)): - s = str(x).strip() - if s: - cleaned.append(s) - if len(cleaned) >= 6: - break - return cleaned - - # ── Answer synthesis ───────────────────────────────────────────────────── def _format_evidence_block(evidence: list[dict]) -> str: diff --git a/stacklets/docs/bot/recall.py b/stacklets/docs/bot/recall.py index d07e7de..598eb8d 100644 --- a/stacklets/docs/bot/recall.py +++ b/stacklets/docs/bot/recall.py @@ -27,12 +27,17 @@ from __future__ import annotations -import re from typing import Optional, Tuple from loguru import logger from pipeline import Classifier +# Memory owns the vault, so it owns the rewrite and the regex the +# walker reads. This module keeps the chat-side half: when to spend an +# LLM call, and how Paperless wants the same keywords rendered. The +# import above puts `stacklets/` on sys.path, which is what makes the +# sibling stacklet reachable here. +from memory.lib import keywords_to_regex async def resolve_search_query( @@ -102,10 +107,9 @@ async def resolve_search_query( return query, query, [] logger.info("[recall] keywords: {}", keywords) - # Memory side: regex alternation, re.escape each keyword so a - # chatty LLM that returns "C++" or "Lisa's" can't blow up the - # compile step. - memory_regex = "|".join(re.escape(k) for k in keywords) + # Memory side: regex alternation, escaped per keyword. Memory + # renders this because memory is what has to read it back. + memory_regex = keywords_to_regex(keywords) # Paperless side: Whoosh OR alternation, bare tokens. The # PaperlessAPI wraps this in `_to_search_query` which adds the # prefix wildcard on each bare term -- so "fish OR price" becomes diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index 44a0b03..3c93da6 100644 --- a/stacklets/memory/lib.py +++ b/stacklets/memory/lib.py @@ -29,12 +29,13 @@ from __future__ import annotations +import json import re import subprocess import time from dataclasses import dataclass, field from pathlib import Path -from typing import Callable, List, Optional +from typing import TYPE_CHECKING, Callable, List, Optional # Vault-layout conventions live in the framework so both stacklets share # one source. Re-exported here for memory's own callers (and back-compat @@ -55,6 +56,11 @@ from stack.forgejo import ForgejoClient, ForgejoError from stack.ontology import Ontology +if TYPE_CHECKING: + # Type-only: `stack.ai.client` pulls in the OpenAI SDK, which the + # host interpreter does not have. See the query-rewrite section. + from stack.ai.client import LLM + STACKLET_DIR = Path(__file__).resolve().parent SEEDS_DIR = STACKLET_DIR / "seeds" @@ -1598,6 +1604,158 @@ def search_memory( return results[:limit] +# ─── Natural-language query rewrite ────────────────────────────────────── +# +# `search_memory` takes a Python regex. A family question is not one: +# "What do we still need to buy for the camping trip?" asks for those +# exact words, adjacent, which no file contains. Anyone holding a +# sentence has to turn it into keywords first, and that step lives here +# rather than in whichever bot happens to ask. Memory owns the vault, +# so memory owns what a query means against it. The archivist had this +# hop and the agent did not, which is why the same question answered in +# chat and returned nothing from the CLI. +# +# The rewrite needs a model, and memory neither owns one nor opens one: +# `rewrite_query` takes the caller's `stack.ai.client.LLM`. The +# archivist already built one, and the memory CLI commands that need a +# model already docker-exec into the same bot-runner container that +# holds it (see `cli/_common.py`). Nothing new gets wired up. +# +# That is also why the third-party imports sit inside the function. +# `stack memory search` runs on the host's stdlib-only python and +# imports this module, exactly like the frontmatter loader above. + + +def build_rewrite_prompt( + question: str, ontology_section: str, language: str = "en", +) -> str: + """Recall-mode prompt: question in, JSON list of search keywords out. + + The ontology block is what makes the keywords hit: it names the + topics and forms this family actually files under, so the model + expands and translates into the vault's vocabulary instead of + guessing at a generic one. + """ + return f"""You extract search keywords from a question, so a regex walker can look up family documents. + +The family classifies their documents under these topics and forms: + +{ontology_section} + +Language hint: {language}. +- If the question is in German, prefer German keywords. +- If it is in English, prefer English keywords. +- For an ambiguous or generic question, include the most likely topic name plus one or two synonyms in the document language. + +Question: {question} + +Reply with a JSON object: {{"keywords": ["word1", "word2", "word3"]}}. +2 to 4 keywords. Each keyword is a literal word that would appear in +the document (a noun, a name, a topic). No phrases, no quotes, no +prose around the JSON. Output ONLY the JSON object.""" + + +def parse_rewrite_response(raw: str) -> List[str]: + """Pull a keyword list out of the rewrite model's response. + + Accepts the requested object form `{"keywords": [...]}` and the + bare-array fallback that some smaller models produce when they + forget the wrapper. Empty list on any parse failure -- the caller + treats empty as "no rewrite, search the question verbatim." Caps + at 6 entries so a chatty model can't blow up the alternation + regex; trims whitespace and drops empties so a stray `""` doesn't + poison the join. + """ + try: + data = json.loads(raw) + except (ValueError, TypeError): + return [] + + if isinstance(data, dict): + v = data.get("keywords") + candidates = v if isinstance(v, list) else [] + elif isinstance(data, list): + candidates = data + else: + candidates = [] + + cleaned: List[str] = [] + for x in candidates: + if isinstance(x, (str, int, float)): + s = str(x).strip() + if s: + cleaned.append(s) + if len(cleaned) >= 6: + break + return cleaned + + +async def rewrite_query( + question: str, + *, + llm: "LLM", + ontology_section: str = "", + language: str = "en", +) -> List[str]: + """Extract search keywords from a natural-language question. + + The model reads the family's topic and doctype ontology and + produces 2-4 keywords that would literally appear in a matching + document. Translation and synonym expansion happen here, so the + regex walker downstream stays dumb. + + Best-effort: any transport failure or parse error returns an empty + list, which callers treat as "no rewrite, search the question + verbatim." Recall is a quality-of-life feature, never a gate. + Synonym selection is the model's job and we don't second-guess it, + but we do strip empties and cap the list length. + + Callers get the keywords rather than a finished query because they + render differently: `keywords_to_regex` for the vault walker, an + ` OR ` join for Paperless, and a "Searched for: ..." line in chat + so a family can see when a bad rewrite hid results. + """ + from loguru import logger + from stack.ai.client import ( + LLMModelNotFoundError, + LLMTimeoutError, + LLMUnavailableError, + ) + + prompt = build_rewrite_prompt(question, ontology_section, language) + try: + raw = await llm.complete("recall", prompt, json_mode=True) + except (LLMUnavailableError, LLMModelNotFoundError, LLMTimeoutError) as e: + logger.warning("[recall] LLM unavailable for rewrite: {}", e) + return [] + keywords = parse_rewrite_response(raw) + if not keywords: + # Surface the raw payload so an empty keyword list is + # debuggable: an off-shape JSON response is a prompt or + # model issue, not a transport one, and we can't fix it + # blind. + logger.warning( + "[recall] rewrite parse produced no keywords; raw={!r}", + (raw or "")[:200], + ) + return keywords + + +def keywords_to_regex(keywords: List[str]) -> str: + """Render keywords as the alternation regex `search_memory` reads. + + `re.escape` per keyword is mandatory, not decoration: a model that + answers "C++" or "Lisa's" or "(foo)" would otherwise ship a pattern + that fails to compile, and `search_memory` returns `[]` on a bad + regex, so the search would go quiet rather than loud. + + An empty keyword list renders as the empty pattern, which matches + every line. Callers that got no keywords back fall back to the + literal query instead of calling this. + """ + return "|".join(re.escape(k) for k in keywords) + + def install_memory_to_forgejo_admin( *, code_url: str, diff --git a/tests/stacklets/test_memory_host_stdlib.py b/tests/stacklets/test_memory_host_stdlib.py index c633330..864db34 100644 --- a/tests/stacklets/test_memory_host_stdlib.py +++ b/tests/stacklets/test_memory_host_stdlib.py @@ -21,13 +21,16 @@ from __future__ import annotations +import os +import subprocess import sys +import textwrap from pathlib import Path import pytest -sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent - / "stacklets" / "memory")) +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "memory")) from lib import ( # noqa: E402 load_correspondents_from_vault, @@ -104,6 +107,40 @@ def test_correspondents_load_without_the_pip_package(bare_host, tmp_path): assert correspondent.aliases == ["Duff Beer"] +# ── query rewrite: the LLM stack must stay off the host ────────────── + +def test_the_query_rewrite_does_not_drag_the_llm_stack_onto_the_host(): + """`stack memory search` imports the module that holds the rewrite. + + The rewrite needs a model, so its imports (loguru, the OpenAI SDK + through `stack.ai.client`) are inside the function that uses them. + Hoisting one of those to the top of the module is a natural-looking + tidy-up that kills the search command on every clean host, and no + test in the bot suites would notice, because the bot has both. + + A subprocess is the point: it imports the module fresh, with those + packages blocked the way a machine that never ran `pip install` + blocks them. + """ + probe = textwrap.dedent(""" + import sys + for absent in ("loguru", "openai", "aiohttp", "frontmatter"): + sys.modules[absent] = None + import lib + print(lib.keywords_to_regex(["C++", "Camping"])) + print("keywords" in lib.build_rewrite_prompt("q?", "(...)", "en")) + print(lib.parse_rewrite_response('{"keywords": ["Zelt"]}')) + """) + env = {**os.environ, "PYTHONPATH": os.pathsep.join([ + str(_REPO_ROOT / "lib"), str(_REPO_ROOT / "stacklets" / "memory"), + ])} + result = subprocess.run([sys.executable, "-c", probe], + capture_output=True, text=True, timeout=60, env=env) + + assert result.returncode == 0, result.stderr + assert result.stdout.splitlines() == [r"C\+\+|Camping", "True", "['Zelt']"] + + def test_a_vault_with_nothing_in_it_is_not_an_error(bare_host, tmp_path): """The empty case ran before the import did, which is what hid this. diff --git a/tests/stacklets/test_memory_query_rewrite.py b/tests/stacklets/test_memory_query_rewrite.py new file mode 100644 index 0000000..b555236 --- /dev/null +++ b/tests/stacklets/test_memory_query_rewrite.py @@ -0,0 +1,306 @@ +"""Turning a family question into something the vault can answer. + +`search_memory` reads a Python regex. A person asks "What do we still +need to buy for the camping trip?", which as a regex asks for those +exact words, adjacent, and matches nothing. The gap between the two is +this module's subject: a model reads the question against the family's +ontology, answers with keywords that would literally appear in a +matching file, and `keywords_to_regex` renders them for the walker. + +It lives in `memory.lib` because memory owns the vault, and therefore +owns what a query means against it. It used to live in the archivist, +where it worked, and every other caller of the same vault got nothing. +The last class here is about that: both callers now share one rewrite, +and a test says so out loud. + +The model is a hand-written stand-in that records what it was asked +and returns what the test wants back. Memory never opens an LLM +client; callers hand it theirs, so a stub is the honest shape of the +collaborator, not a mock of one. +""" + +from __future__ import annotations + +import json +import re +import sys +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "lib")) +sys.path.insert(0, str(_REPO_ROOT / "stacklets")) +sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "docs" / "bot")) + +from memory.lib import ( # noqa: E402 + build_rewrite_prompt, + keywords_to_regex, + parse_rewrite_response, + rewrite_query, +) +from stack.ai.client import LLMTimeoutError, LLMUnavailableError # noqa: E402 + + +# ── Stand-in model ────────────────────────────────────────────────────── + +class _StubLLM: + """The `stack.ai.client.LLM` surface `rewrite_query` actually uses. + + One method, because one method is all memory calls. `response` is + what the model "says"; `raises`, when set, is raised instead so a + test can play the unreachable endpoint. `calls` records the role + and prompt, which is how we assert the rewrite asks for JSON + rather than trusting that it does. + """ + + def __init__(self, *, response: str = '{"keywords": []}', + raises: Exception | None = None): + self._response = response + self._raises = raises + self.calls: list[dict] = [] + + async def complete(self, role, prompt, *, images=None, + json_mode=False, model_override=None, temperature=None): + self.calls.append({"role": role, "prompt": prompt, "json_mode": json_mode}) + if self._raises is not None: + raise self._raises + return self._response + + +def _llm_saying(*keywords: str) -> _StubLLM: + return _StubLLM(response=json.dumps({"keywords": list(keywords)})) + + +# ── The prompt ────────────────────────────────────────────────────────── + +class TestBuildRewritePrompt: + """The recall prompt: question + ontology section in, prompt string out.""" + + @staticmethod + def _build(question="Wann hatte Bart MMR?", section="(...topics...)", lang="de"): + return build_rewrite_prompt(question, section, lang) + + def test_question_appears_verbatim(self): + # The LLM needs the literal phrasing to pick the right keywords; + # any rewriting on our side would defeat the point. + prompt = self._build(question="Was kostet die Auto-Versicherung?") + assert "Was kostet die Auto-Versicherung?" in prompt + + def test_ontology_section_embedded(self): + # Synonym and translation expansion is the ontology's job, so + # the rendered section has to land inside the prompt. + prompt = self._build(section="- Insurance (Versicherung)") + assert "- Insurance (Versicherung)" in prompt + + def test_language_hint_present(self): + # The hint primes the model to answer in the document language; + # without it we'd get English keywords for a German vault. + prompt = self._build(lang="de") + assert "de" in prompt + + def test_json_output_contract_present(self): + # The parser only knows two shapes. The prompt has to ask for + # one of them explicitly. + prompt = self._build() + assert "JSON" in prompt + assert "keywords" in prompt + + +# ── The parser ────────────────────────────────────────────────────────── + +class TestParseRewriteResponse: + """The keyword extractor parser — what the recall layer trusts.""" + + @staticmethod + def _parse(raw): + return parse_rewrite_response(raw) + + def test_object_form(self): + # The contract shape: {"keywords": [...]} -- the format the + # prompt explicitly asks for. + assert self._parse('{"keywords": ["Auto", "KFZ"]}') == ["Auto", "KFZ"] + + def test_bare_array_fallback(self): + # Some smaller models forget the wrapper. Honoring a bare + # array means a question doesn't lose recall just because + # the model skipped the keys. + assert self._parse('["Auto", "KFZ"]') == ["Auto", "KFZ"] + + def test_strips_empty_strings(self): + # An empty string in the alternation regex matches every line. + # Drop them before the join — better to lose a slot than to + # turn the search into a "match everything" query. + assert self._parse('{"keywords": ["Auto", "", " ", "KFZ"]}') == ["Auto", "KFZ"] + + def test_strips_whitespace_around_keywords(self): + # Models occasionally pad with spaces; not worth a re-prompt. + assert self._parse('{"keywords": [" Auto ", "KFZ\\n"]}') == ["Auto", "KFZ"] + + def test_coerces_numbers_to_strings(self): + # A year keyword like 2026 comes back as a JSON number. Recall + # against a date is legitimate ("documents from 2026"), so + # accept and stringify rather than drop. + assert self._parse('{"keywords": [2026, "Steuer"]}') == ["2026", "Steuer"] + + def test_caps_at_six(self): + # A runaway model that returns twenty keywords would build an + # absurd alternation; cap defensively so the regex compile + # stays cheap. + many = '{"keywords": ["a","b","c","d","e","f","g","h","i","j"]}' + assert self._parse(many) == ["a", "b", "c", "d", "e", "f"] + + def test_invalid_json_returns_empty(self): + # No keywords means: literal-query fallback. The recall layer + # is best-effort, never a gate. + assert self._parse("not json") == [] + assert self._parse("") == [] + assert self._parse("{") == [] + + def test_wrong_shape_returns_empty(self): + # Object without "keywords" key, scalar response — same outcome + # as a parse failure. Don't try to guess the model's intent. + assert self._parse('{"foo": "bar"}') == [] + assert self._parse('"just a string"') == [] + assert self._parse('42') == [] + + +# ── The hop itself ────────────────────────────────────────────────────── + +class TestRewriteQuery: + """Question in, keywords out, with the caller's model doing the work.""" + + @pytest.mark.asyncio + async def test_returns_the_keywords_the_model_chose(self): + # The whole point of the hop: "when was Bart vaccinated" leaves + # as words that appear in the German vaccination record. + llm = _llm_saying("Impfung", "MMR", "Auffrischung") + keywords = await rewrite_query( + "When did Bart get vaccinated?", llm=llm, + ontology_section="- Medical (Gesundheit)", language="de", + ) + assert keywords == ["Impfung", "MMR", "Auffrischung"] + + @pytest.mark.asyncio + async def test_asks_the_recall_role_for_json(self): + # Role names route the request to the model the family + # configured for recall, and JSON mode is what the parser is + # written against. Both are part of the request, not details + # of it: a rewrite asked in prose mode parses to nothing. + llm = _llm_saying("Auto") + await rewrite_query("Autoversicherung?", llm=llm) + assert llm.calls[0]["role"] == "recall" + assert llm.calls[0]["json_mode"] is True + + @pytest.mark.asyncio + async def test_the_family_ontology_reaches_the_model(self): + # A caller passes the vault's own ontology so the keywords come + # back in the vocabulary this family files under. If it stopped + # reaching the prompt, recall would still "work" and quietly + # get worse, which is the failure we most want pinned. + llm = _llm_saying("Versicherung") + await rewrite_query( + "Was kostet die Versicherung?", llm=llm, + ontology_section="- Insurance (Versicherung)", language="de", + ) + assert "- Insurance (Versicherung)" in llm.calls[0]["prompt"] + + @pytest.mark.asyncio + async def test_unreachable_model_returns_no_keywords(self): + # The family's AI stacklet is down, or was never set up. The + # caller falls back to the literal query on an empty list, so + # a question still searches for something. + llm = _StubLLM(raises=LLMUnavailableError("no endpoint")) + assert await rewrite_query("Anything?", llm=llm) == [] + + @pytest.mark.asyncio + async def test_slow_model_returns_no_keywords(self): + # Same contract for a timeout: recall degrades, it never + # propagates an exception into a search. + llm = _StubLLM(raises=LLMTimeoutError("took too long")) + assert await rewrite_query("Anything?", llm=llm) == [] + + @pytest.mark.asyncio + async def test_off_shape_answer_returns_no_keywords(self): + # The model was reachable and answered with prose. Nothing to + # search for, so the caller gets the same empty list it gets + # from a dead endpoint and treats both the same way. + llm = _StubLLM(response="Sure! Here are some keywords: Auto, KFZ.") + assert await rewrite_query("Autoversicherung?", llm=llm) == [] + + +# ── Rendering for the walker ──────────────────────────────────────────── + +class TestKeywordsToRegex: + """The string `search_memory` gets handed.""" + + def test_alternation_joined_with_pipe(self): + # Python regex alternation. Order is the model's ranking and + # survives the join. + assert keywords_to_regex(["Auto", "KFZ", "Versicherung"]) == \ + "Auto|KFZ|Versicherung" + + def test_metacharacters_are_escaped(self): + # "C++", "Lisa's" and "(foo)" are all plausible keywords and + # all break a raw join: an unbalanced paren fails to compile, + # and `search_memory` answers a bad regex with an empty result + # set. A silent no-hits is the worst failure mode we have. + pattern = keywords_to_regex(["C++", "Lisa's", "(foo)"]) + compiled = re.compile(pattern) + assert compiled.search("got C++ done") + assert compiled.search("Lisa's room") + assert compiled.search("inside (foo) block") + + def test_a_single_keyword_is_itself(self): + assert keywords_to_regex(["Grillfest"]) == "Grillfest" + + +# ── One rewrite, every caller ─────────────────────────────────────────── + +class TestBothCallersShareTheRewrite: + """The gap that hid the original bug, pinned. + + `resolve_search_query` had eight passing tests while the agent got + nothing from the same vault, because nothing checked that the other + caller shared the contract. These do: the archivist's classifier + and the chat-side resolver both have to end up in this module, so a + future caller inherits the fix instead of rediscovering the bug. + """ + + @pytest.mark.asyncio + async def test_the_archivist_classifier_asks_memory(self): + # `Classifier.rewrite_query` is the archivist's entry point. It + # must be memory's prompt and memory's parser with the bot's + # own LLM handed over, not a second copy that can drift. + from pipeline import Classifier + + llm = _llm_saying("Impfung", "MMR") + keywords = await Classifier(llm).rewrite_query( # type: ignore[arg-type] + "Wann hatte Bart MMR?", "- Medical (Gesundheit)", "de", + ) + + assert keywords == ["Impfung", "MMR"] + assert llm.calls[0]["prompt"] == build_rewrite_prompt( + "Wann hatte Bart MMR?", "- Medical (Gesundheit)", "de", + ) + + @pytest.mark.asyncio + async def test_the_chat_resolver_renders_memorys_regex(self): + # recall.py keeps the `?` trigger and the Paperless rendering, + # but the string the vault walker reads comes from here. Same + # keywords in, byte-identical pattern out. + from recall import resolve_search_query + + class _Classifier: + async def rewrite_query(self, question, ontology_section, lang): + return ["C++", "Lisa's"] + + memory_regex, paperless_query, keywords = await resolve_search_query( + "Anything?", classifier=_Classifier(), # type: ignore[arg-type] + ontology_section="", language="en", + ) + + assert memory_regex == keywords_to_regex(keywords) + # Paperless stays the archivist's business: Whoosh reads ` OR `, + # not `|`, and that rendering deliberately did not move. + assert paperless_query == "C++ OR Lisa's" diff --git a/tests/stacklets/test_pipeline.py b/tests/stacklets/test_pipeline.py index 1d803eb..71e381a 100644 --- a/tests/stacklets/test_pipeline.py +++ b/tests/stacklets/test_pipeline.py @@ -1441,97 +1441,9 @@ async def test_non_image_mime_filtered(self): assert c._stub.calls[0]["images"] is None -# ── Query rewrite (recall mode) ──────────────────────────────────────── - -class TestBuildRewritePrompt: - """The recall prompt: question + ontology section in, prompt string out.""" - - @staticmethod - def _build(question="Wann hatte Bart MMR?", section="(...topics...)", lang="de"): - from pipeline import _build_rewrite_prompt - return _build_rewrite_prompt(question, section, lang) - - def test_question_appears_verbatim(self): - # The LLM needs the literal phrasing to pick the right keywords; - # any rewriting on our side would defeat the point. - prompt = self._build(question="Was kostet die Auto-Versicherung?") - assert "Was kostet die Auto-Versicherung?" in prompt - - def test_ontology_section_embedded(self): - # Synonym and translation expansion is the ontology's job, so - # the rendered section has to land inside the prompt. - prompt = self._build(section="- Insurance (Versicherung)") - assert "- Insurance (Versicherung)" in prompt - - def test_language_hint_present(self): - # The hint primes the model to answer in the document language; - # without it we'd get English keywords for a German vault. - prompt = self._build(lang="de") - assert "de" in prompt - - def test_json_output_contract_present(self): - # The parser only knows two shapes. The prompt has to ask for - # one of them explicitly. - prompt = self._build() - assert "JSON" in prompt - assert "keywords" in prompt - - -class TestParseRewriteResponse: - """The keyword extractor parser — what the recall layer trusts.""" - - @staticmethod - def _parse(raw): - from pipeline import _parse_rewrite_response - return _parse_rewrite_response(raw) - - def test_object_form(self): - # The contract shape: {"keywords": [...]} -- the format the - # prompt explicitly asks for. - assert self._parse('{"keywords": ["Auto", "KFZ"]}') == ["Auto", "KFZ"] - - def test_bare_array_fallback(self): - # Some smaller models forget the wrapper. Honoring a bare - # array means a question doesn't lose recall just because - # the model skipped the keys. - assert self._parse('["Auto", "KFZ"]') == ["Auto", "KFZ"] - - def test_strips_empty_strings(self): - # An empty string in the alternation regex matches every line. - # Drop them before the join — better to lose a slot than to - # turn the search into a "match everything" query. - assert self._parse('{"keywords": ["Auto", "", " ", "KFZ"]}') == ["Auto", "KFZ"] - - def test_strips_whitespace_around_keywords(self): - # Models occasionally pad with spaces; not worth a re-prompt. - assert self._parse('{"keywords": [" Auto ", "KFZ\\n"]}') == ["Auto", "KFZ"] - - def test_coerces_numbers_to_strings(self): - # A year keyword like 2026 comes back as a JSON number. Recall - # against a date is legitimate ("documents from 2026"), so - # accept and stringify rather than drop. - assert self._parse('{"keywords": [2026, "Steuer"]}') == ["2026", "Steuer"] - - def test_caps_at_six(self): - # A runaway model that returns twenty keywords would build an - # absurd alternation; cap defensively so the regex compile - # stays cheap. - many = '{"keywords": ["a","b","c","d","e","f","g","h","i","j"]}' - assert self._parse(many) == ["a", "b", "c", "d", "e", "f"] - - def test_invalid_json_returns_empty(self): - # No keywords means: literal-query fallback. The recall layer - # is best-effort, never a gate. - assert self._parse("not json") == [] - assert self._parse("") == [] - assert self._parse("{") == [] - - def test_wrong_shape_returns_empty(self): - # Object without "keywords" key, scalar response — same outcome - # as a parse failure. Don't try to guess the model's intent. - assert self._parse('{"foo": "bar"}') == [] - assert self._parse('"just a string"') == [] - assert self._parse('42') == [] +# The recall-mode query rewrite used to be tested here. It now lives in +# memory, which owns the vault it searches, and so do its tests: +# tests/stacklets/test_memory_query_rewrite.py. # ── PaperlessAPI.wait_task — task-status polling ────────────────────── From 24456b58bee4afbfa5425c6592b9b93a2d765f24 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 4 Aug 2026 15:28:18 +0200 Subject: [PATCH 2/4] feat(memory): ask the family vault a question in plain words `stack memory search --nl "what do we still need for the camping trip"` turns the question into the words that are actually on disk, searches for those, and prints what it searched for so a bad guess is visible: Searched for: Zelt, Schlafsack The default is unchanged and still a regex: no model, no container, same speed. A single word never calls a model even with --nl, and when no model is reachable the query is searched literally instead of failing. Stacky now passes the flag, so its own searches stop coming back empty on every question. Refs FAM-21 --- docs/design-notes.md | 21 +- stacklets/agent/runtime/memory_tool.py | 6 + stacklets/docs/bot/recall.py | 8 + stacklets/memory/bot/cli/rewrite.py | 60 ++++ stacklets/memory/bot/cli_entrypoint.py | 10 +- stacklets/memory/cli/_common.py | 47 +++ stacklets/memory/cli/search.py | 117 ++++++- tests/stacklets/test_memory_query_rewrite.py | 51 ++++ tests/stacklets/test_memory_search_nl.py | 301 +++++++++++++++++++ 9 files changed, 609 insertions(+), 12 deletions(-) create mode 100644 stacklets/memory/bot/cli/rewrite.py create mode 100644 tests/stacklets/test_memory_search_nl.py diff --git a/docs/design-notes.md b/docs/design-notes.md index 58793c3..72047e3 100644 --- a/docs/design-notes.md +++ b/docs/design-notes.md @@ -132,14 +132,19 @@ Moving it makes every caller correct at once: the agent tool, the archivist, `stack memory search` from a terminal, and whatever asks next. Leaving it means the next consumer re-learns this the way the agent did. -**What has to be decided when it moves.** The rewrite needs an LLM, so a -search command that has never called a model would start to, and that changes -its latency and its failure modes. Options are a flag (`--natural`), inferring -it from the trailing `?` the way the archivist does, or keeping the rewrite as -a lib function that callers opt into. The archivist also needs the keywords -back, not just the regex, because it shows `Searched for: ...` so a family can -see when a bad rewrite hid results; that visibility is worth keeping and the -return shape has to carry it. +**What was decided when it moved.** The rewrite lives in `memory.lib` and +takes the caller's LLM, so memory owns the hop without owning a client. On the +CLI it sits behind `stack memory search --nl`, which reaches the model in the +bot-runner the same way `stack memory wiki` does. Opt-in rather than inferred: +this surface's default query language is a regex, in which `?` is a quantifier, +so inferring from a trailing `?` would turn `Zelt?` into a surprise model call, +and it is on an agent's hot path where the default has to stay cheap. Chat keeps +inferring, because there a rewrite per message is affordable and one character +is a rule a family can learn. Two guards keep the cost honest: a query with no +whitespace never leaves the host, and every failure (no AI, container down, +model silent) falls back to searching the query literally. Both surfaces print +`Searched for: ...`, because a rewrite that picked the wrong words has to look +different from an empty vault. Both current callers should keep working unchanged through the move. That is the test. diff --git a/stacklets/agent/runtime/memory_tool.py b/stacklets/agent/runtime/memory_tool.py index 77c6c37..7006529 100644 --- a/stacklets/agent/runtime/memory_tool.py +++ b/stacklets/agent/runtime/memory_tool.py @@ -64,11 +64,17 @@ async def execute( person: str | None = None, tag: str | None = None, ) -> str: + # `--nl` is what makes the parameter description above true. The + # CLI's default query language is a regex, so a question sent + # without it asks for those exact words, adjacent, and matches + # nothing. The CLI skips the model itself on a single word, so + # passing this always costs nothing on keyword lookups. args = [ "stack", "memory", "search", query, + "--nl", "--limit", str(limit or 5), ] diff --git a/stacklets/docs/bot/recall.py b/stacklets/docs/bot/recall.py index 598eb8d..1d0e8e9 100644 --- a/stacklets/docs/bot/recall.py +++ b/stacklets/docs/bot/recall.py @@ -23,6 +23,14 @@ Everything is best-effort: an unreachable LLM, a malformed response, or an empty keyword list falls back to the literal query. Recall is a quality-of-life feature, never a gate. + +Chat infers the trigger; the CLI does not. `stack memory search` +reaches the same rewrite behind an explicit `--nl`, and that +difference is deliberate on both sides. Here, every message is +already being read for a family, and one character is a rule a +family can learn. There, the default query language is a regex, in +which `?` is a quantifier, so inferring from it would turn `Zelt?` +into a surprise model call. Do not "fix" either one into the other. """ from __future__ import annotations diff --git a/stacklets/memory/bot/cli/rewrite.py b/stacklets/memory/bot/cli/rewrite.py new file mode 100644 index 0000000..c873656 --- /dev/null +++ b/stacklets/memory/bot/cli/rewrite.py @@ -0,0 +1,60 @@ +"""memory rewrite — the keyword hop, run where the model lives. + +`stack memory search --nl` needs a model, and the host `./stack` is +stdlib-only by design. So the host asks this, inside the bot-runner, +for the one thing it cannot work out itself: which words would +literally appear in a document that answers the question. The host +still does the searching. + +Output is one keyword per line on stdout, which is the smallest thing +that survives a `docker exec` round trip without a parser. Exit 0 with +keywords, exit 1 with none. Exit 1 is not an error: the host reads it +as "no rewrite available" and searches the question literally, the +same fallback the archivist has always had. + +The ontology comes from the live vault (`MEMORY_VAULT_DIR`), because +the keywords have to be in the vocabulary this family files under. A +`--vault` override on the host does not change that: the taxonomy is +the family's, not the directory's. +""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[3])) +from memory.lib import get_ontology, rewrite_query # noqa: E402 + +from stack.ai.client import LLM # noqa: E402 + +HELP = "Turn a natural-language question into vault search keywords" + + +async def run(llm: LLM, argv: list[str]) -> int: + """Entry point the dispatcher calls with the shared LLM client.""" + if not argv or not argv[0].strip(): + print("usage: rewrite ", file=sys.stderr) + return 2 + + question = argv[0] + vault = Path(os.environ.get("MEMORY_VAULT_DIR", "/data/memory/vault")) + language = os.environ.get("LANGUAGE", "en") + + # `get_ontology` falls back to the shipped seed when the vault has + # no ontology.toml of its own, so a fresh install still gets sane + # topic names rather than an empty prompt section. + ontology = get_ontology(vault if vault.exists() else None) + + keywords = await rewrite_query( + question, + llm=llm, + ontology_section=ontology.classifier_prompt_section(language), + language=language, + ) + if not keywords: + return 1 + + print("\n".join(keywords)) + return 0 diff --git a/stacklets/memory/bot/cli_entrypoint.py b/stacklets/memory/bot/cli_entrypoint.py index 0767e9d..d73abe9 100644 --- a/stacklets/memory/bot/cli_entrypoint.py +++ b/stacklets/memory/bot/cli_entrypoint.py @@ -10,6 +10,13 @@ re-use the bot-runner as their tools runtime. Commands: + rewrite + Print the search keywords a question should be looked up by, + one per line. `stack memory search --nl` is the caller: it + does the searching on the host and only comes here for the + words. Exit 1 means no keywords, which the host treats as + "search it literally" rather than as a failure. + wiki [--home] [--member ]... [--topic ]... [--dry-run] Regenerate the family wiki's entry pages. Apply by default; `--dry-run` previews to stdout. Bare invocation regenerates @@ -30,10 +37,11 @@ from stack.ai.client import LLM, LLMUnavailableError -from cli import wiki +from cli import rewrite, wiki _HANDLERS = { + "rewrite": rewrite.run, "wiki": wiki.run, } diff --git a/stacklets/memory/cli/_common.py b/stacklets/memory/cli/_common.py index 474aa1d..8eee49b 100644 --- a/stacklets/memory/cli/_common.py +++ b/stacklets/memory/cli/_common.py @@ -60,3 +60,50 @@ def dispatch(command: str, *argv: str) -> dict: if rc != 0: sys.exit(rc) return {"ok": True} + + +def dispatch_capture(command: str, *argv: str, + timeout: int = 60) -> tuple[int, str, str]: + """The same hop, for a caller that wants the output as a value. + + `dispatch` is right when the container's output *is* the result: + it streams to the terminal and exits with the container's status. + `stack memory search --nl` is the other shape. It asks the + container for keywords and then does its own work with them, so it + needs them back, and it must stay alive when the hop is + unavailable. + + Returns `(returncode, stdout, reason)`. A stopped container, a host + with no docker, and a model that timed out all come back as a + nonzero code, because to the caller they mean the same thing: no + answer from here, carry on without one. + + `reason` is the first line of the container's stderr, and it is + first line rather than all of it on purpose. An entry point that + does not know the command prints its whole usage text, and a + version-skewed host (updated code, bot-runner not restarted yet) + would dump that over a family's search results. One line stays + diagnostic without ever becoming a wall. + """ + if not _bot_runner_running(): + return 1, "", f"{BOT_RUNNER_CONTAINER} is not running" + + cmd = [ + "docker", "exec", "-i", + BOT_RUNNER_CONTAINER, + "python", ENTRYPOINT_PATH, command, *argv, + ] + try: + result = subprocess.run( + cmd, capture_output=True, text=True, timeout=timeout, + ) + except FileNotFoundError: + return 1, "", "docker CLI not found on this host" + except subprocess.TimeoutExpired: + return 1, "", f"{command} timed out after {timeout}s" + + first_line = next( + (ln.strip() for ln in (result.stderr or "").splitlines() if ln.strip()), + "", + ) + return result.returncode, result.stdout, first_line diff --git a/stacklets/memory/cli/search.py b/stacklets/memory/cli/search.py index b77fd97..7104748 100644 --- a/stacklets/memory/cli/search.py +++ b/stacklets/memory/cli/search.py @@ -15,7 +15,7 @@ Contract — kept stable so wrappers (CLI, MCP, Matrix bot) can build on it without rework: - stack memory search [--person ] [--tag ] + stack memory search [--nl] [--person ] [--tag ] [--scope ] [--limit N] [--paths | --count] [--vault ] [--no-refresh] @@ -33,6 +33,42 @@ file that tags `'Person: Homer'` — writers shouldn't have to know which spelling lives on disk. +`--nl` is the sentence path. "What do we still need to buy for the +camping trip" is not a regex — as one it asks for those exact words, +adjacent, which no file contains — so with `--nl` the question goes +to a model first, which answers with 2-4 words that would literally +appear in a matching document, and *those* become the regex. The +command prints what it searched for so a bad rewrite is visible +rather than silent: + + Searched for: Travel, Shopping, Trip + +The line is suppressed under `--paths` and `--count`, which are +machine-readable output. + +Three properties of `--nl` are deliberate: + + * **It degrades, never fails.** No AI configured, bot-runner down, + model unreachable, empty keyword list: the query is searched + literally instead. Recall is a quality-of-life feature, never a + gate, and the exit codes below mean the same thing either way. + * **A single bare word skips the model.** `search camping` is a + keyword, not a question. It short-circuits before the round trip, + so the cheap case stays cheap even when a caller passes `--nl` + on everything. + * **It is opt-in, and chat's trigger is not.** The archivist infers + the same rewrite from a trailing `?` (see `docs/bot/recall.py`), + which is right there and wrong here: on this surface the default + query language is a regex, where `?` is a quantifier, so + `Zelt?` would silently become a model call. Chat also pays per + message, while this is on an agent's hot path. The asymmetry is + the point, not an oversight. + +The model runs in `stack-core-bot-runner`, reached the same way +`stack memory wiki` reaches it (`cli/_common.py`). The host stays +stdlib-only; only the keywords cross the boundary, and the vault walk, +the filters and the output all stay here. + Results are sorted by frontmatter `date` (newest first). Files without a parseable date fall to the end. @@ -65,7 +101,15 @@ # stdlib-only python3, so we manipulate sys.path before importing # from `lib` rather than relying on a package install. sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) -from lib import refresh_vault_if_stale, search_memory, vault_path_for # noqa: E402 +from lib import ( # noqa: E402 + keywords_to_regex, + refresh_vault_if_stale, + search_memory, + vault_path_for, +) + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from _common import dispatch_capture # noqa: E402 HELP = "Full-text search over the curated memory vault" @@ -79,6 +123,14 @@ def _parser() -> argparse.ArgumentParser: description=HELP, ) p.add_argument("query", help="search term (regex, case-insensitive)") + p.add_argument( + "--nl", action="store_true", + help=( + "treat the query as a question: ask the model for search " + "keywords first, then search for those (falls back to a " + "literal search when no model is reachable)" + ), + ) p.add_argument( "--person", action="append", default=[], metavar="NAME", help="filter by person; repeatable, OR within axis", @@ -121,6 +173,57 @@ def _parser() -> argparse.ArgumentParser: return p +# ── the sentence path ─────────────────────────────────────────────────── + +def _resolve_query(query: str) -> tuple[str, list[str]]: + """Turn a `--nl` question into `(regex, keywords)`. + + Every failure lands on the same answer: `(query, [])`, meaning + "search what the caller typed". An empty keyword list is how the + caller tells the difference, and it is also what suppresses the + `Searched for:` line, because there is nothing to disclose when no + rewrite happened. + + The single-word short-circuit is a cost decision, not a + correctness one. `search camping --nl` would round-trip to a + container and wait on a model to be told that the keyword for + "camping" is "camping". A query with no whitespace is already the + shape the walker wants. + """ + if not _looks_like_a_sentence(query): + return query, [] + + rc, out, reason = dispatch_capture("rewrite", query) + if rc != 0: + # Container down, no docker, no AI endpoint, model timed out, + # or an entry point too old to know the command. Deliberately + # one branch and one line: the caller asked for results, not + # for a report on our infrastructure, and a literal search + # still finds whatever literally matches. + detail = f" ({reason})" if reason else "" + print( + f"[memory] no rewrite available{detail}, " + "searching the query literally", + file=sys.stderr, + ) + return query, [] + + keywords = [line.strip() for line in out.splitlines() if line.strip()] + if not keywords: + return query, [] + return keywords_to_regex(keywords), keywords + + +def _looks_like_a_sentence(query: str) -> bool: + """Is this worth spending a model call on? + + Whitespace is the whole test. One word is a keyword and already + works; two or more are a phrase, which as a regex means "these + words, adjacent" and is the case that returns nothing today. + """ + return len(query.split()) > 1 + + # ── output ────────────────────────────────────────────────────────────── def _format_block(r: dict) -> str: @@ -187,13 +290,21 @@ def run(args, stacklet, config) -> dict | None: # there's nothing to say, the second because nagging on every # offline call would be noise. + query, keywords = _resolve_query(ns.query) if ns.nl else (ns.query, []) + results = search_memory( - ns.query, vault, + query, vault, persons=ns.person, tags=ns.tag, scopes=ns.scope or None, limit=ns.limit, ) + # Before the results, and before the no-results exit: a rewrite + # that found nothing is exactly when a family needs to see which + # words were used. Machine-readable modes stay machine-readable. + if keywords and not (ns.paths or ns.count): + print(f"Searched for: {', '.join(keywords)}\n") + if not results: sys.exit(1) diff --git a/tests/stacklets/test_memory_query_rewrite.py b/tests/stacklets/test_memory_query_rewrite.py index b555236..b7e4a2b 100644 --- a/tests/stacklets/test_memory_query_rewrite.py +++ b/tests/stacklets/test_memory_query_rewrite.py @@ -72,6 +72,24 @@ def _llm_saying(*keywords: str) -> _StubLLM: return _StubLLM(response=json.dumps({"keywords": list(keywords)})) +def _memory_cli_rewrite(): + """Load memory's container-side `rewrite` command by file path. + + Three stacklets ship a package called `cli`, so `from cli import + rewrite` resolves to whichever one another test imported first. + Inside the container only memory's is on the path and the plain + import is correct; here the file is named explicitly so the test + cannot quietly exercise the docs stacklet instead. + """ + import importlib.util + + path = _REPO_ROOT / "stacklets" / "memory" / "bot" / "cli" / "rewrite.py" + spec = importlib.util.spec_from_file_location("memory_cli_rewrite", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + # ── The prompt ────────────────────────────────────────────────────────── class TestBuildRewritePrompt: @@ -284,6 +302,39 @@ async def test_the_archivist_classifier_asks_memory(self): "Wann hatte Bart MMR?", "- Medical (Gesundheit)", "de", ) + @pytest.mark.asyncio + async def test_the_cli_hop_prints_one_keyword_per_line(self, tmp_path, monkeypatch, capsys): + # `stack memory search --nl` reaches the same rewrite through a + # container, so the third caller is a command whose whole + # contract is its stdout. One keyword per line survives a + # `docker exec` round trip without anyone writing a parser. + rewrite = _memory_cli_rewrite() + + monkeypatch.setenv("MEMORY_VAULT_DIR", str(tmp_path / "absent")) + monkeypatch.setenv("LANGUAGE", "de") + llm = _llm_saying("Zelt", "Schlafsack") + + code = await rewrite.run(llm, ["Was fehlt uns noch fürs Camping?"]) + + assert code == 0 + assert capsys.readouterr().out == "Zelt\nSchlafsack\n" + # The container's own env carries the household language, so a + # German family gets German keywords without the host saying so. + assert "Language hint: de" in llm.calls[0]["prompt"] + + @pytest.mark.asyncio + async def test_the_cli_hop_reports_no_keywords_as_exit_one(self, tmp_path, monkeypatch): + # Exit 1 is not a failure here: the host reads it as "no + # rewrite available" and searches the question literally. A + # nonzero-means-broken reading would turn a quiet degradation + # into a dead command. + rewrite = _memory_cli_rewrite() + + monkeypatch.setenv("MEMORY_VAULT_DIR", str(tmp_path / "absent")) + llm = _StubLLM(response="I could not think of any keywords.") + + assert await rewrite.run(llm, ["what did we buy"]) == 1 + @pytest.mark.asyncio async def test_the_chat_resolver_renders_memorys_regex(self): # recall.py keeps the `?` trigger and the Paperless rendering, diff --git a/tests/stacklets/test_memory_search_nl.py b/tests/stacklets/test_memory_search_nl.py new file mode 100644 index 0000000..b72ecb2 --- /dev/null +++ b/tests/stacklets/test_memory_search_nl.py @@ -0,0 +1,301 @@ +"""`stack memory search --nl` — asking the vault a question in words. + +Without `--nl` the query is a Python regex, which is the right default +for a surface agents call in a loop and the wrong shape for "what do +we still need to buy for the camping trip". That sentence as a regex +asks for those exact words, adjacent, and matches nothing. `--nl` +sends it to a model first and searches for the words that come back. + +The model lives in the bot-runner container, so these tests stand in +for that hop rather than starting a container: `_resolve_query` is +driven with a replacement `dispatch_capture` that returns what the +container would have printed. Everything on this side of the hop is +the real thing, including the regex assembly, because that is where +the behaviour under test lives. + +The CLI cases below run the real `stack memory search` as a +subprocess against a fixture vault. No bot-runner is up in a test +environment, so they exercise the degradation path for free, which is +the one that has to hold: a family asking a question on a host with +no AI configured still gets whatever literally matches. +""" + +from __future__ import annotations + +import sys +import textwrap +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "memory")) +sys.path.insert(0, str(_REPO_ROOT / "stacklets" / "memory" / "cli")) + +import search # noqa: E402 + + +# ── Fixture vault ─────────────────────────────────────────────────────── + +def _write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(textwrap.dedent(text).lstrip("\n")) + + +@pytest.fixture +def vault(tmp_path): + """Two camping notes, phrased the way a family writes them. + + Neither body contains the sentence anyone would ask, which is the + entire problem: recall has to go through the words that are on + disk ("Zelt", "Schlafsack"), not the words in the question. + """ + v = tmp_path / "vault" + _write(v / "family" / "camping" / "packliste.md", """ + --- + title: Packliste Campingausflug + date: 2026-08-02 + persons: + - Bart + --- + + # Packliste Campingausflug + + Noch zu besorgen: Schlafsack für Bart, Ersatzstab für das Zelt. + """) + _write(v / "family" / "camping" / "reservation.md", """ + --- + title: Stellplatz reserviert + date: 2026-08-01 + persons: + - Marge + --- + + # Stellplatz reserviert + + Zeltplatz am See gebucht, zwei Nächte. + """) + return v + + +@pytest.fixture +def container_says(monkeypatch): + """Stand in for the bot-runner hop with a canned container reply. + + Returns a setter taking `(returncode, stdout, reason)` exactly as + `dispatch_capture` returns it, plus a `calls` list so a test can + assert the hop did *not* happen, which is half of what these + tests are about. + """ + calls: list[tuple] = [] + reply = {"rc": 0, "out": "", "reason": ""} + + def fake_dispatch(command, *argv, timeout=60): + calls.append((command, argv)) + return reply["rc"], reply["out"], reply["reason"] + + monkeypatch.setattr(search, "dispatch_capture", fake_dispatch) + + def _set(rc, out="", reason=""): + reply["rc"], reply["out"], reply["reason"] = rc, out, reason + return calls + + _set.calls = calls # type: ignore[attr-defined] + return _set + + +# ── When the model is asked, and when it is not ───────────────────────── + +class TestWhenTheModelIsAsked: + """`--nl` is opt-in, and even then not every query is worth a call.""" + + def test_a_question_becomes_its_keywords(self, container_says): + # The hop returns the words that are actually on disk; the + # search runs on those, OR-alternated, not on the sentence. + container_says(0, "Zelt\nSchlafsack\n") + + query, keywords = search._resolve_query( + "what do we still need to buy for the camping trip", + ) + + assert keywords == ["Zelt", "Schlafsack"] + assert query == "Zelt|Schlafsack" + + def test_a_single_word_never_leaves_the_host(self, container_says): + # `search camping --nl` would otherwise spend a container round + # trip and a model call to learn that the keyword for "camping" + # is "camping". Agents pass --nl on everything, so this is the + # common case, not an edge one. + calls = container_says(0, "should-not-be-asked\n") + + query, keywords = search._resolve_query("camping") + + assert query == "camping" + assert keywords == [] + assert calls == [] + + def test_a_regex_query_is_never_rewritten(self, vault, capsys): + # Without --nl nothing is asked at all: `run` does not even + # reach the resolver. This is the default path, and it must + # stay free of the container and the model. + search.run(["Zelt|Schlafsack", "--vault", str(vault), "--no-refresh"], + None, None) + + out = capsys.readouterr().out + assert "Packliste Campingausflug" in out + assert "Searched for" not in out + + +# ── Degrade, never fail ───────────────────────────────────────────────── + +class TestDegradation: + """Every way the hop can fail ends in a literal search.""" + + def test_no_bot_runner_falls_back_to_the_literal_query(self, container_says, capsys): + # Container down, no docker, model timed out: one branch, + # because the caller wants results, not a report on our + # infrastructure. + container_says(1, "") + + query, keywords = search._resolve_query("camping trip packing list") + + assert query == "camping trip packing list" + assert keywords == [] + assert "searching the query literally" in capsys.readouterr().err + + def test_an_outdated_container_degrades_in_one_line(self, container_says, capsys): + # Version skew is the realistic version of this: the host has + # new code, the bot-runner has not been restarted, and its + # entry point answers an unknown command with its whole usage + # text. One line of it reaches the person, as context on our + # own note, never as a wall over their search results. + container_says(2, "", "Unknown command: rewrite") + + search._resolve_query("what do we still need") + + err = capsys.readouterr().err + assert err.splitlines() == [ + "[memory] no rewrite available (Unknown command: rewrite), " + "searching the query literally", + ] + + def test_a_model_with_nothing_to_say_falls_back(self, container_says): + # Exit 0 and no keywords is the container telling us the model + # answered off-shape. Same outcome as a dead container: search + # what the caller typed. + container_says(0, "\n \n") + + assert search._resolve_query("what did we buy") == ("what did we buy", []) + + def test_falling_back_still_finds_what_literally_matches(self, vault, container_says, capsys): + # The point of degrading rather than failing: with no AI + # anywhere in sight, the words the caller typed are still + # searched, and a phrase that is on disk still comes back. + container_says(1, "") + + search.run(["Zeltplatz am See", "--nl", + "--vault", str(vault), "--no-refresh"], None, None) + + out = capsys.readouterr().out + assert "Stellplatz reserviert" in out + assert "Searched for" not in out # nothing was rewritten + + +# ── What the family is told ───────────────────────────────────────────── + +class TestSearchedForLine: + """A bad rewrite has to be visible, not silent.""" + + def test_printed_when_a_rewrite_happened(self, vault, container_says, capsys): + container_says(0, "Zelt\nSchlafsack\n") + + search.run(["what do we need for camping", "--nl", + "--vault", str(vault), "--no-refresh"], None, None) + + out = capsys.readouterr().out + assert out.startswith("Searched for: Zelt, Schlafsack\n") + assert "Packliste Campingausflug" in out + + def test_printed_even_when_nothing_matched(self, vault, container_says, capsys): + # This is the case it exists for. Without the line, a rewrite + # that picked the wrong words is indistinguishable from an + # empty vault, which is what hid the original bug for months. + container_says(0, "Bootsführerschein\n") + + with pytest.raises(SystemExit) as exit_code: + search.run(["do we have a boat licence", "--nl", + "--vault", str(vault), "--no-refresh"], None, None) + + assert exit_code.value.code == 1 + assert "Searched for: Bootsführerschein" in capsys.readouterr().out + + def test_suppressed_under_paths(self, vault, container_says, capsys): + # `--paths` feeds xargs. One extra line would send a + # non-existent file into whatever runs next. + container_says(0, "Schlafsack\n") + + search.run(["what about the sleeping bag", "--nl", "--paths", + "--vault", str(vault), "--no-refresh"], None, None) + + out = capsys.readouterr().out + assert "Searched for" not in out + assert out.strip() == "family/camping/packliste.md" + + def test_suppressed_under_count(self, vault, container_says, capsys): + # `--count` promises an integer and nothing else. Both notes + # match here (`Zelt` is inside `Zeltplatz`), which is the + # alternation doing its job. + container_says(0, "Zelt\nSchlafsack\n") + + search.run(["what do we need", "--nl", "--count", + "--vault", str(vault), "--no-refresh"], None, None) + + assert capsys.readouterr().out.strip() == "2" + + +# ── Exit codes, across every path ─────────────────────────────────────── + +class TestExitCodes: + """The contract wrappers read. `memory_tool` treats 1 as an answer. + + Exercised through the real CLI, because the exit code is what the + process returns, not what a function returns. No bot-runner is + running here, so `--nl` takes its fallback path, which is the one + a host without AI configured takes in production. + """ + + def test_hit_exits_zero(self, stack_cli, vault): + rc, _, _ = stack_cli("memory", "search", "Zelt", + "--vault", str(vault), "--no-refresh") + assert rc == 0 + + def test_miss_exits_one(self, stack_cli, vault): + rc, _, _ = stack_cli("memory", "search", "Regenschirm", + "--vault", str(vault), "--no-refresh") + assert rc == 1 + + def test_nl_miss_still_exits_one(self, stack_cli, vault): + # Not 3, and not 0. A question nobody can answer is "no + # results", the same as a keyword nobody can answer. The agent + # reads anything above 1 as "the search is broken" and retries. + rc, _, _ = stack_cli("memory", "search", "where is the boat licence", + "--nl", "--vault", str(vault), "--no-refresh") + assert rc == 1 + + def test_nl_hit_exits_zero_through_the_fallback(self, stack_cli, vault): + # Two words, so the rewrite is attempted and unavailable, and + # the literal search underneath still matches. + rc, out, _ = stack_cli("memory", "search", "Schlafsack für Bart", + "--nl", "--vault", str(vault), "--no-refresh") + assert rc == 0 + assert "Packliste" in out + + def test_bad_arguments_exit_two(self, stack_cli, vault): + rc, _, _ = stack_cli("memory", "search", "Zelt", "--paths", "--count", + "--vault", str(vault), "--no-refresh") + assert rc == 2 + + def test_missing_vault_exits_three(self, stack_cli, tmp_path): + rc, _, _ = stack_cli("memory", "search", "Zelt", + "--vault", str(tmp_path / "nope"), "--no-refresh") + assert rc == 3 From 1c7c8b469607bd85616efbc222fb85f9ff16f4d1 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 4 Aug 2026 15:32:10 +0200 Subject: [PATCH 3/4] fix(memory): make the same question search for the same words The rewrite left sampling to the model, so asking twice picked different keywords and returned different results. One run of "what do we still need to buy for the camping trip" searched Travel, Holiday, Shopping and surfaced a vaccination record; the next run found the camping notes. Picking words a document already contains is a lookup, not a creative act, so it now decodes greedily, like the wiki generator does for the same reason. Refs FAM-21 --- stacklets/memory/lib.py | 9 ++++++++- tests/stacklets/test_memory_query_rewrite.py | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index 3c93da6..58b94b0 100644 --- a/stacklets/memory/lib.py +++ b/stacklets/memory/lib.py @@ -1724,7 +1724,14 @@ async def rewrite_query( prompt = build_rewrite_prompt(question, ontology_section, language) try: - raw = await llm.complete("recall", prompt, json_mode=True) + # Greedy, like the wiki generator and for the same reason: + # sampling makes the same question search for different words + # run to run. Picking the words a document already contains is + # a lookup, not a creative act, and a family comparing two + # searches should not be comparing two dice rolls. + raw = await llm.complete( + "recall", prompt, json_mode=True, temperature=0.0, + ) except (LLMUnavailableError, LLMModelNotFoundError, LLMTimeoutError) as e: logger.warning("[recall] LLM unavailable for rewrite: {}", e) return [] diff --git a/tests/stacklets/test_memory_query_rewrite.py b/tests/stacklets/test_memory_query_rewrite.py index b7e4a2b..56ecb23 100644 --- a/tests/stacklets/test_memory_query_rewrite.py +++ b/tests/stacklets/test_memory_query_rewrite.py @@ -62,7 +62,8 @@ def __init__(self, *, response: str = '{"keywords": []}', async def complete(self, role, prompt, *, images=None, json_mode=False, model_override=None, temperature=None): - self.calls.append({"role": role, "prompt": prompt, "json_mode": json_mode}) + self.calls.append({"role": role, "prompt": prompt, + "json_mode": json_mode, "temperature": temperature}) if self._raises is not None: raise self._raises return self._response @@ -209,6 +210,10 @@ async def test_asks_the_recall_role_for_json(self): await rewrite_query("Autoversicherung?", llm=llm) assert llm.calls[0]["role"] == "recall" assert llm.calls[0]["json_mode"] is True + # Greedy: the same question has to search for the same words. + # Left to the server default, this samples, and two runs of one + # question return different keywords and different results. + assert llm.calls[0]["temperature"] == 0.0 @pytest.mark.asyncio async def test_the_family_ontology_reaches_the_model(self): From d176660ddbfd3cd2151c223e399486800d67b06f Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 4 Aug 2026 15:36:29 +0200 Subject: [PATCH 4/4] fix(memory): search for what a question is about, not its category Asked "what do we still need to buy for the camping trip", the rewrite answered Travel, Shopping, Receipt. All three are real topic names from the family's ontology, none of them appear in any camping note, and the search came back with a Kwik-E-Mart receipt. Handed a list of categories, the model was answering from the list. The question's own subject now comes first, and the topic list is context for spelling and language rather than a menu. Same question now searches camping, trip, shopping. Refs FAM-21 --- stacklets/memory/lib.py | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/stacklets/memory/lib.py b/stacklets/memory/lib.py index 58b94b0..773bff7 100644 --- a/stacklets/memory/lib.py +++ b/stacklets/memory/lib.py @@ -1635,6 +1635,17 @@ def build_rewrite_prompt( topics and forms this family actually files under, so the model expands and translates into the vault's vocabulary instead of guessing at a generic one. + + It also has to be held at arm's length, which is what the closing + instruction does. Given the list and nothing else, a model answers + with the list: "what do we still need to buy for the camping + trip" came back as Travel, Shopping, Receipt, all real topic + names, none of them words any camping note contains, and the + search returned a Kwik-E-Mart receipt. `search_memory` matches + document bodies with frontmatter stripped, so a category that + only ever appears as a tag can never match. The subject of the + question comes first; the topics are there for spelling and + language. """ return f"""You extract search keywords from a question, so a regex walker can look up family documents. @@ -1650,9 +1661,13 @@ def build_rewrite_prompt( Question: {question} Reply with a JSON object: {{"keywords": ["word1", "word2", "word3"]}}. -2 to 4 keywords. Each keyword is a literal word that would appear in -the document (a noun, a name, a topic). No phrases, no quotes, no -prose around the JSON. Output ONLY the JSON object.""" +2 to 4 keywords. Start with the concrete subject of the question: +the thing, place, activity or person it is about, written the way a +document about it would write it. The topic list above is context for +vocabulary and language, not a menu to answer from; a category name is +only a keyword if a document would actually contain that word. No +phrases, no quotes, no prose around the JSON. Output ONLY the JSON +object.""" def parse_rewrite_response(raw: str) -> List[str]: