diff --git a/docs/design-notes.md b/docs/design-notes.md index 58793c36..72047e3c 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 77c6c370..7006529d 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/pipeline.py b/stacklets/docs/bot/pipeline.py index 0ba4c19a..f4850a5f 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 d07e7dea..1d0e8e9b 100644 --- a/stacklets/docs/bot/recall.py +++ b/stacklets/docs/bot/recall.py @@ -23,16 +23,29 @@ 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 -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 +115,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/bot/cli/rewrite.py b/stacklets/memory/bot/cli/rewrite.py new file mode 100644 index 00000000..c873656b --- /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 0767e9d1..d73abe93 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 474aa1df..8eee49b0 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 b77fd97e..71047489 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/stacklets/memory/lib.py b/stacklets/memory/lib.py index 44a0b033..773bff78 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,180 @@ 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. + + 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. + +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. 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]: + """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: + # 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 [] + 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 c633330c..864db349 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 00000000..56ecb237 --- /dev/null +++ b/tests/stacklets/test_memory_query_rewrite.py @@ -0,0 +1,362 @@ +"""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, "temperature": temperature}) + 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)})) + + +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: + """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 + # 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): + # 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_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, + # 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_memory_search_nl.py b/tests/stacklets/test_memory_search_nl.py new file mode 100644 index 00000000..b72ecb2d --- /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 diff --git a/tests/stacklets/test_pipeline.py b/tests/stacklets/test_pipeline.py index 1d803eb2..71e381a0 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 ──────────────────────