diff options
| -rw-r--r-- | README.md | 47 | ||||
| -rw-r--r-- | llamachat/backend.py | 138 | ||||
| -rw-r--r-- | llamachat/config.py | 21 | ||||
| -rw-r--r-- | llamachat/search.py | 24 | ||||
| -rw-r--r-- | llamachat/ui.py | 85 | ||||
| -rwxr-xr-x | test_llamachat.py | 161 |
6 files changed, 437 insertions, 39 deletions
@@ -17,6 +17,9 @@ persistent process and toggles like a scratchpad from a Hyprland keybind. vision model, and dropping one on a text model offers to switch. - **History** in SQLite with FTS5 full-text search. Past chat sessions reopen and continue with their context intact; one-shot entries reopen read-only. + Sessions are named by the model itself: once the first reply lands, it is + asked to title the exchange in a few words, which replaces the opening + words of the question the entry was created with. - **Streaming replies** rendered token by token, formatted as markdown: headings, bold and italic, bullet and numbered lists, tables, inline code and tinted fenced code blocks. What you type is shown exactly as typed, so @@ -144,7 +147,7 @@ search_url = "" search_results = 5 search_snippet_chars = 300 search_timeout = 10 -max_searches = 1 +max_searches = 2 ``` `presets.ini` is read for two things the API does not report: which models @@ -382,21 +385,33 @@ Expanding it lists each result's title, link and snippet. The queries are visible on purpose: when an answer is wrong it is usually the query that was wrong, and without seeing it a bad search and a bad answer look identical. -`max_searches` caps searches per turn, default 1. On the last round the tool -is withdrawn from the request, which forces the model to answer instead of -searching again. A local 9B will otherwise keep searching when it is unsure. - -The default is 1 rather than 2 because of an upstream limitation. The first -tool call of a turn arrives as a proper `tool_calls` delta, but a second one, -issued after the model has seen the first set of results, often comes back as -literal `<tool_call><function=web_search>` text inside the thinking instead. -There is no structured call to act on, so the turn ends with an empty reply. - -Observed with Qwen3.5-9B through llama.cpp's router. Newer builds parse -follow-up calls some of the time (1 in 5 on b10208) rather than never, so -this may be worth retesting; raise the cap if your model and build handle -them reliably. Enabling more search engines helps more than raising the cap, -since a first round that returns plenty removes the reason to search twice. +`max_searches` caps searches per turn, default 2. On the last round the tool +is withdrawn from the request and the tool result says so in words, which is +what forces the model to answer instead of searching again. A local 9B will +otherwise keep searching when it is unsure. + +Withdrawing the schema on its own is not enough, because the model cannot +see a schema disappear. It asks for another search regardless, and that +request surfaces either as literal `<tool_call><function=web_search>` text +in the reply or buried in the thinking block, leaving the reply itself +empty. Saying "this was your last search" in the tool result is what fixes +it. The note has to ride on the tool result rather than a trailing system +message: Qwen3.5's chat template rejects those outright, raising `System +message must be at the beginning`. + +If replies still come back empty after a search, check the server's KV +cache. llama.cpp's own function-calling documentation warns that quantized +KV degrades tool calling, and it does: with `cache-type-k`/`cache-type-v` +set to `q8_0`, three of five turns on one question came back broken, and +dropping to the `f16` default made the same question answer six times out +of six. + +Turns that search once are reliable. Turns that genuinely need two are not +quite: on a question forcing two rounds, four and five of six succeeded +across two runs, the rest coming back empty. Lower `max_searches` to 1 if +you would rather never see an empty reply, or if you keep the quantized +cache. Enabling more search engines helps either way, since a first round +that returns plenty removes the reason to search twice. Failures do not abort the turn. A timeout, a refused connection, a non-JSON response or zero results all come back to the model as a tool result saying diff --git a/llamachat/backend.py b/llamachat/backend.py index d3eb3fa..3a32ff7 100644 --- a/llamachat/backend.py +++ b/llamachat/backend.py @@ -185,6 +185,40 @@ class Client: raise BackendError(f"Router sent invalid JSON: {exc}") return [m["id"] for m in payload.get("data", []) if "id" in m] + def complete(self, model: str, messages: list[dict], max_tokens: int = 48) -> str: + """One short non-streaming reply, for side errands like titling. + + No tools are offered and failures are the caller's to catch: this is + never the user's own turn, so a dead router must not break the chat. + + Thinking is turned off through the chat template: a reasoning model + otherwise spends the whole budget on its thoughts and returns an + empty content field. `reasoning_budget: 0` does not do this, it ends + the thinking immediately and still yields nothing. + """ + try: + resp = httpx.post( + f"{self.base_url}/v1/chat/completions", + json={ + "model": model, + "messages": messages, + "stream": False, + "max_tokens": max_tokens, + "chat_template_kwargs": {"enable_thinking": False}, + }, + timeout=httpx.Timeout(self.timeout, connect=10), + ) + resp.raise_for_status() + payload = resp.json() + except httpx.HTTPError as exc: + raise BackendError(f"Request failed: {exc}") + except ValueError as exc: + raise BackendError(f"Router sent invalid JSON: {exc}") + choices = payload.get("choices") or [] + if not choices: + return "" + return str((choices[0].get("message") or {}).get("content") or "") + def stream_chat( self, model: str, messages: list[dict], search_cfg: "SearchConfig | None" = None ): @@ -250,10 +284,16 @@ class Client: ], } ) + # The round after this one is the last, so its results are the + # last the model will get. Saying so is what makes it answer + # instead of asking for a search it can no longer make. + final = round_number + 1 == search_cfg.max_searches for call in wanted: - yield from self._run_search(convo, call, search_cfg) + yield from self._run_search(convo, call, search_cfg, final) - def _run_search(self, convo: list[dict], call: dict, cfg: "SearchConfig"): + def _run_search( + self, convo: list[dict], call: dict, cfg: "SearchConfig", last: bool = False + ): """Perform one search, append its result to `convo`, report both ends. Every failure still appends a tool message: the model is waiting on @@ -264,7 +304,9 @@ class Client: # Unusable arguments: nothing to search, but the call still needs # an answer or the next request is malformed. convo.append( - search.tool_message(call["id"], None, error="Malformed search request") + search.tool_message( + call["id"], None, error="Malformed search request", last=last + ) ) return @@ -274,13 +316,15 @@ class Client: cfg.url, query, cfg.results, cfg.snippet_chars, cfg.timeout ) except search.SearchError as exc: - convo.append(search.tool_message(call["id"], None, error=str(exc))) + convo.append( + search.tool_message(call["id"], None, error=str(exc), last=last) + ) yield ( "search_done", json.dumps({"query": query, "results": [], "error": str(exc)}), ) return - convo.append(search.tool_message(call["id"], results)) + convo.append(search.tool_message(call["id"], results, last=last)) yield ( "search_done", json.dumps({"query": query, "results": results, "error": ""}), @@ -322,6 +366,90 @@ class Client: raise BackendError(f"Request failed: {exc}") +# How much of the first exchange the titling request is shown. Enough to +# see what the conversation is about, far short of a whole context. +TITLE_EXCERPT = 1200 +TITLE_MAX_CHARS = 60 + +_TITLE_SYSTEM = ( + "You write short titles for chat conversations. Reply with the title " + "alone: at most six words, no quotes, no punctuation at the end, no " + "explanation, no preamble." +) + + +def placeholder_title(text: str) -> str: + """The stand-in title a new session gets: the question's opening words.""" + return text[:TITLE_MAX_CHARS] + + +def needs_title(current_title: str, first_question: str, reply_text: str) -> bool: + """Whether a finished turn should be handed to the titler. + + Keyed on the session still wearing the placeholder derived from its + opening question, rather than on being the first turn. A first reply can + come back empty, and a one-shot window appends every question to the + same session until New is pressed, so "first turn" gives up too early in + both cases. + """ + if not reply_text.strip(): + return False + return current_title == placeholder_title(first_question) + + +def title_messages(user_text: str, reply_text: str) -> list[dict]: + """The request that asks the model to title its own first exchange.""" + exchange = ( + f"User: {user_text[:TITLE_EXCERPT]}\n\n" + f"Assistant: {reply_text[:TITLE_EXCERPT]}" + ) + return [ + {"role": "system", "content": _TITLE_SYSTEM}, + { + "role": "user", + "content": ( + f"{exchange}\n\nTitle this conversation in at most six words." + ), + }, + ] + + +def clean_title(raw: str) -> str: + """Reduce a model's reply to a usable title, or '' if there is none. + + Models decorate: they think out loud, add a "Title:" label, wrap the + answer in quotes or bold, or chat before answering. Everything here is + salvage, and returning '' is a valid outcome that leaves the caller's + fallback title in place. + """ + text = raw or "" + if "<think>" in text: + # An unclosed block means the reply was cut off mid-thought and + # nothing after it can be trusted as a title. + _, _, after = text.partition("</think>") + text = after if "</think>" in raw else "" + + # Chatter comes before the answer, so the last non-empty line is it. + lines = [line.strip() for line in text.splitlines() if line.strip()] + if not lines: + return "" + title = lines[-1] + + title = title.strip("*_` ") + for label in ("title:", "Title:", "TITLE:"): + if title.startswith(label): + title = title[len(label):].strip() + title = title.strip().strip('"“”\'') + title = title.rstrip(".!,;:").strip() + + if len(title) > TITLE_MAX_CHARS: + cut = title[:TITLE_MAX_CHARS] + # Prefer a word boundary, but never return a single truncated word. + head, sep, _ = cut.rpartition(" ") + title = (head if sep and head else cut).rstrip(".,;:-") + return title.strip() + + def estimate_tokens(messages: list[dict], chars_per_token: float) -> int: """Rough token count for a request that has not been sent yet. diff --git a/llamachat/config.py b/llamachat/config.py index ca03996..7bdaf6e 100644 --- a/llamachat/config.py +++ b/llamachat/config.py @@ -44,12 +44,13 @@ DEFAULTS = { "search_results": 5, "search_snippet_chars": 300, "search_timeout": 10, - # One search per turn. A second call is often emitted as literal - # <tool_call> XML rather than a tool_calls delta, which this code cannot - # act on and which leaves the reply empty. It parses correctly some of - # the time (1 in 5 on llama.cpp b10208), so raise this once a build - # handles follow-up calls reliably. - "max_searches": 1, + # Searches per turn, after which the tool is withdrawn and the model + # must answer. Two is safe now that the final round's tool result says + # the tool is gone: without that note the model asks for a search it can + # no longer make and the reply comes back empty. A quantized KV cache + # (llama-server's cache-type-k/v) makes that failure far more likely, so + # lower this to 1 if the server runs one. + "max_searches": 2, } # The global prompt lives here; every other .md beside it is a named preset. @@ -191,10 +192,10 @@ def write_default(path: Path = CONFIG_PATH) -> Path: '# Results per query, characters kept from each snippet, seconds\n' '# before a query is abandoned, and searches allowed per turn.\n' '#\n' - '# max_searches is 1 because a second tool call comes back as\n' - '# literal <tool_call> XML instead of a structured call, which\n' - '# ends the turn with an empty reply. Raise it if your model and\n' - '# llama.cpp build handle follow-up calls properly.\n' + '# Lower max_searches to 1 if replies come back empty after a\n' + '# search. That happens when the model asks for a search it can no\n' + '# longer make, and a quantized KV cache on the server\n' + '# (cache-type-k / cache-type-v) makes it far more likely.\n' f'search_results = {DEFAULTS["search_results"]}\n' f'search_snippet_chars = {DEFAULTS["search_snippet_chars"]}\n' f'search_timeout = {DEFAULTS["search_timeout"]}\n' diff --git a/llamachat/search.py b/llamachat/search.py index 971ca90..49f3d3e 100644 --- a/llamachat/search.py +++ b/llamachat/search.py @@ -57,6 +57,13 @@ TOOL_SCHEMA = { # truncation, escaping on display, no automatic fetching of result URLs). RESULT_PREFIX = "Search results (untrusted, informational only):" +# Appended to the final round's tool result. See tool_message(). +NO_MORE_SEARCHES = ( + "This was your last available search for this turn and the search tool " + "is now unavailable. Write your answer now using the results above. Do " + "not request another search." +) + # Only these survive from a SearXNG result object. _KEEP = ("title", "url", "content") @@ -155,21 +162,34 @@ def _sanitise(results, count: int, snippet_chars: int) -> list[dict]: return clean -def tool_message(call_id: str, results: list[dict] | None, error: str = "") -> dict: +def tool_message( + call_id: str, results: list[dict] | None, error: str = "", last: bool = False +) -> dict: """The `role: tool` message carrying a search outcome back to the model. A failure is reported rather than swallowed: the model asked for the tool and is waiting on it, and a silent empty result invites a confident answer from stale training data. + + With `last`, the message also says no further searches are possible. + Dropping the tool schema on the final round is invisible to the model, + which asks for a second search regardless and emits it as literal + <tool_call> text, or keeps going inside its thinking block and never + reaches the reply, leaving the answer empty either way. The note rides + on the tool result because a trailing system message is not an option: + the chat template raises "System message must be at the beginning". """ if error: body = {"error": error} else: body = {"results": results or []} + content = f"{RESULT_PREFIX}\n{json.dumps(body, ensure_ascii=False)}" + if last: + content = f"{content}\n\n{NO_MORE_SEARCHES}" return { "role": "tool", "tool_call_id": call_id, - "content": f"{RESULT_PREFIX}\n{json.dumps(body, ensure_ascii=False)}", + "content": content, } diff --git a/llamachat/ui.py b/llamachat/ui.py index b746d70..7d5c4a2 100644 --- a/llamachat/ui.py +++ b/llamachat/ui.py @@ -193,6 +193,35 @@ class StreamWorker(QObject): self.finished.emit() +class TitleWorker(QObject): + """Asks the model to name a conversation, off the GUI thread. + + Silent by design: a title is a nicety, so any failure leaves the + fallback title alone rather than showing the user an error. + """ + + ready = Signal(int, str) + finished = Signal() + + def __init__(self, client, model: str, session_id: int, messages: list[dict]): + super().__init__() + self.client = client + self.model = model + self.session_id = session_id + self.messages = messages + + @Slot() + def run(self) -> None: + try: + raw = self.client.complete(self.model, self.messages) + title = backend.clean_title(raw) + except Exception: # noqa: BLE001 - a missing title is not an error + title = "" + if title: + self.ready.emit(self.session_id, title) + self.finished.emit() + + class PromptDialog(QDialog): """Browse, edit and add system prompts. @@ -349,6 +378,10 @@ class ChatWindow(QMainWindow): self.attachments: list[Attachment] = [] self.thread: QThread | None = None self.worker: StreamWorker | None = None + # Titling runs on its own short-lived threads. Each (thread, worker) + # pair is held here for as long as the request is in flight, since + # neither survives on the Qt side alone. + self.title_threads: list[tuple] = [] self.assistant_message_id: int | None = None self.assistant_buffer = "" self.reasoning_buffer = "" @@ -902,7 +935,9 @@ class ChatWindow(QMainWindow): return if self.session_id is None: - title = (text or self.attachments[0].path.name)[:60] + title = backend.placeholder_title( + text or self.attachments[0].path.name + ) self.session_id = self.history.create_session( self.mode, model, @@ -1080,7 +1115,55 @@ class ChatWindow(QMainWindow): self.reasoning_buffer, self._searches_json(), ) + titled = self.assistant_message_id is not None and self._should_title() self._teardown_stream() + if titled: + self._start_titling() + self.refresh_history() + + def _should_title(self) -> bool: + session = self.history.get_session(self.session_id) + rows = self.history.messages(self.session_id) + if session is None or not rows: + return False + return backend.needs_title( + session["title"] or "", rows[0]["content"], self.assistant_buffer + ) + + def _start_titling(self) -> None: + """Replace the first-words placeholder with a model-written title.""" + rows = self.history.messages(self.session_id) + model = self.current_model() + if not model: + return + # The question this reply answers, which in a one-shot window is not + # necessarily the one the session was created from. + question = next( + (r["content"] for r in reversed(rows) if r["role"] == "user"), "" + ) + messages = backend.title_messages(question, self.assistant_buffer) + + thread = QThread(self) + worker = TitleWorker(self.client, model, self.session_id, messages) + worker.moveToThread(thread) + thread.started.connect(worker.run) + worker.ready.connect(self._on_title_ready) + worker.finished.connect(thread.quit) + # Both halves are held until the thread ends. moveToThread does not + # take ownership, so a worker kept alive only by a local is collected + # as soon as this returns, taking its queued started() call with it + # and leaving a thread that runs forever without ever calling run(). + pair = (thread, worker) + self.title_threads.append(pair) + thread.finished.connect( + lambda p=pair: p in self.title_threads + and self.title_threads.remove(p) + ) + thread.start() + + @Slot(int, str) + def _on_title_ready(self, session_id: int, title: str) -> None: + self.history.set_title(session_id, title) self.refresh_history() @Slot(str) diff --git a/test_llamachat.py b/test_llamachat.py index a766382..a06d737 100755 --- a/test_llamachat.py +++ b/test_llamachat.py @@ -967,11 +967,11 @@ def test_search_tool_schema(): assert out == [("content", "hi")] assert sent == [None], sent - # One search per turn by default: a follow-up call arrives as literal - # <tool_call> XML rather than a structured call, and a turn that ends on - # one shows the user an empty reply. - assert config.DEFAULTS["max_searches"] == 1 - assert config.load(Path("/nonexistent/config.toml")).max_searches == 1 + # Two searches per turn, which a follow-up call now survives: the final + # round's tool result says the tool is gone, and quantized KV cache, the + # other half of the empty-reply bug, is a server-side setting. + assert config.DEFAULTS["max_searches"] == 2 + assert config.load(Path("/nonexistent/config.toml")).max_searches == 2 # A configured-but-urlless setup resolves to disabled at config load. with tempfile.TemporaryDirectory() as tmp: @@ -1472,6 +1472,152 @@ def test_search_html(): print("ok search html") +def test_final_round_note(): + """The last search result tells the model it has no searches left. + + Withdrawing the tool schema is invisible to the model: it tries a second + search anyway and emits it as literal <tool_call> text, or trails off + inside the thinking block, either way leaving the reply empty. Saying so + in the tool result is what actually ends the turn with an answer. The + note cannot be a trailing system message: this model's chat template + raises "System message must be at the beginning". + """ + from llamachat import search + + plain = search.tool_message("call_1", [{"title": "T", "url": "u", "content": "c"}]) + final = search.tool_message( + "call_1", [{"title": "T", "url": "u", "content": "c"}], last=True + ) + assert plain["role"] == "tool" and final["role"] == "tool" + assert search.NO_MORE_SEARCHES not in plain["content"] + assert search.NO_MORE_SEARCHES in final["content"] + # The results themselves survive the note being appended. + for message in (plain, final): + assert '"title": "T"' in message["content"] or '"title":"T"' in message["content"] + # A failed final search still gets the note, or the model retries. + failed = search.tool_message("call_1", None, error="boom", last=True) + assert "boom" in failed["content"] + assert search.NO_MORE_SEARCHES in failed["content"] + print("ok final round note") + + +def test_title_cleaning(): + clean = backend.clean_title + + assert clean("Kernel build failure") == "Kernel build failure" + # Quotes, trailing punctuation and a leading label all come off. + assert clean('"Kernel build failure"') == "Kernel build failure" + assert clean("Title: Kernel build failure.") == "Kernel build failure" + assert clean("**Kernel build failure**") == "Kernel build failure" + # Thinking that leaks into the body is dropped, answer kept. + assert clean("<think>hmm, short</think>Kernel build") == "Kernel build" + # An unclosed think block leaves nothing usable rather than a stray tag. + assert clean("<think>still reasoning") == "" + # Chatter before the title: the last non-empty line wins. + assert clean("Sure, here you go:\nKernel build failure") == "Kernel build failure" + # Too long is cut on a word boundary, not mid-word. + long = clean("word " * 40) + assert len(long) <= 60, long + assert not long.endswith("wor"), long + # Nothing usable yields nothing, so the caller keeps its fallback. + assert clean("") == "" + assert clean(" \n\n ") == "" + assert clean('""') == "" + print("ok title cleaning") + + +def test_title_request(): + """Client.complete posts a non-streaming request and returns the text.""" + seen = {} + + class FakeResponse: + status_code = 200 + text = "" + + def raise_for_status(self): + pass + + def json(self): + return {"choices": [{"message": {"content": " A Title "}}]} + + def fake_post(url, json=None, timeout=None): + seen["url"] = url + seen["body"] = json + return FakeResponse() + + import httpx + + original = httpx.post + httpx.post = fake_post + try: + client = backend.Client("http://router:8181") + out = client.complete("m", [{"role": "user", "content": "hi"}], max_tokens=32) + finally: + httpx.post = original + + assert out == " A Title ", out + assert seen["url"] == "http://router:8181/v1/chat/completions" + assert seen["body"]["model"] == "m" + assert seen["body"]["stream"] is False + assert seen["body"]["max_tokens"] == 32 + # No tools are offered: a title turn must not trigger a search. + assert "tools" not in seen["body"] + # Thinking off, or a reasoning model spends the budget and returns "". + assert seen["body"]["chat_template_kwargs"] == {"enable_thinking": False} + + # A reply with no choices is empty rather than an exception. + class Empty(FakeResponse): + def json(self): + return {} + + httpx.post = lambda url, json=None, timeout=None: Empty() + try: + assert backend.Client("http://x").complete("m", []) == "" + finally: + httpx.post = original + print("ok title request") + + +def test_needs_title(): + """Which finished turns are worth asking the model to name. + + The trigger is a session still carrying its placeholder title, not the + turn number: a first reply that comes back empty (the follow-up + <tool_call> bug) must not forfeit titling for the whole session, and a + one-shot window keeps appending to the same session until New is + pressed. + """ + needs = backend.needs_title + q = "how do I build a custom kernel on slackware?" + + assert needs(q, q, "Fetch the sources.") is True + # Nothing to title from: the empty-reply turn is skipped, not consumed. + assert needs(q, q, "") is False + assert needs(q, q, " \n ") is False + # Already named by the model, so leave it alone. + assert needs("Building a custom kernel", q, "Fetch the sources.") is False + # A long question is stored truncated; that still counts as untouched. + long_q = "x" * 200 + assert needs(long_q[:60], long_q, "an answer") is True + # A title the user or model set that happens to be short is not a + # placeholder, even though it is under the cut-off. + assert needs("Kernels", long_q, "an answer") is False + print("ok needs title") + + +def test_title_prompt(): + """The title request carries the exchange and asks for a short title.""" + messages = backend.title_messages("how do I build a kernel?", "Run make.") + assert messages[-1]["role"] == "user" + blob = json.dumps(messages) + assert "how do I build a kernel?" in blob + assert "Run make." in blob + # A huge exchange is trimmed so titling never costs a full context. + big = backend.title_messages("x" * 10000, "y" * 10000) + assert len(json.dumps(big)) < 6000, len(json.dumps(big)) + print("ok title prompt") + + if __name__ == "__main__": test_presets() test_real_presets() @@ -1504,4 +1650,9 @@ if __name__ == "__main__": test_search_storage() test_date_note() test_search_html() + test_final_round_note() + test_title_cleaning() + test_title_request() + test_needs_title() + test_title_prompt() print("\nall checks passed") |