diff --git a/src/lembas/services/chat.py b/src/lembas/services/chat.py index fd79dcb..f24e9fa 100644 --- a/src/lembas/services/chat.py +++ b/src/lembas/services/chat.py @@ -33,6 +33,13 @@ FORWARDED_PARAMS = frozenset( MAX_TITLE_LENGTH = 60 +# What one title call may spend. A title is a handful of words; the rest of this +# is headroom for a model that thinks before it answers, which is most of the +# interesting local ones. Too small is not a shorter title -- it is no title at +# all, because the thinking consumes the budget and the content field comes back +# empty or holding an unclosed ``. +TITLE_MAX_TOKENS = 512 + # How long a temporary chat survives after the last thing said in it. TEMPORARY_LIFETIME = timedelta(hours=24) @@ -468,24 +475,45 @@ async def generate_title( if not template.strip(): return fallback_title(question) + from lembas.services.reasoning import strip_reasoning + prompt = prompts_service.substitute( template, {"question": question[:500], "answer": answer[:500]} ) + body = { + "model": model_id, + "messages": [{"role": ROLE_USER, "content": prompt}], + # Enough that a model which thinks before answering can do both. It was + # 24, which is ample for six words and nowhere near enough for a + # reasoning model: the whole budget went on thinking and the reply came + # back either empty or as an unclosed ``, so every chat on such a + # model silently fell back to its first prompt and looked as though + # titling had never run. + "max_tokens": TITLE_MAX_TOKENS, + "temperature": 0.2, + } + # Deliberately *not* `apply_effort(body, "low")`, tempting as it is: naming + # a chat does not reward deliberation and a low effort would make this call + # much cheaper. But `reasoning_effort` and `chat_template_kwargs` appear + # only when somebody has opted in, precisely so a provider strict about + # unknown parameters sees exactly the request it always did — and sending + # them here would put them on every instance's title call, where a 400 is + # caught and turned into a fallback title. That is titling silently + # switching itself off, which is the failure this whole change is fixing. + # The token budget above is what makes room for the thinking instead. try: - raw = await complete( - endpoint, - { - "model": model_id, - "messages": [{"role": ROLE_USER, "content": prompt}], - "max_tokens": 24, - "temperature": 0.2, - }, - ) + raw = await complete(endpoint, body) except LLMError as exc: log.debug("auto-title failed, using fallback: %s", exc) return fallback_title(question) - title = " ".join(raw.split()).strip().strip('"“”\'') + # `complete` hands back `message.content` as it arrived. A model that emits + # `` tags inline puts them in exactly that field, so without this the + # title was "Okay, the user wants a short title for". Reasoning sent + # in a separate `reasoning_content` field is ignored by `complete` already. + answered, _thinking = strip_reasoning(raw) + + title = " ".join(answered.split()).strip().strip('"“”\'') # Small models sometimes ignore the instruction and answer the question # instead; an over-long reply is a better signal of that than anything else. if not title or len(title) > MAX_TITLE_LENGTH * 1.5: diff --git a/src/lembas/services/prompts.py b/src/lembas/services/prompts.py index 6c84eec..f26e00f 100644 --- a/src/lembas/services/prompts.py +++ b/src/lembas/services/prompts.py @@ -1211,13 +1211,18 @@ BUILTIN: tuple[Fragment, ...] = ( group=GROUP_TASKS, order=400, variables=("question", "answer"), - hint="A separate one-message request, not part of any chat. Clear it to " - "stop asking a model for titles: chats are then named from their first " - "message, and no request is made at all.", + hint="A separate one-message request, not part of any chat, made once " + "the first reply has finished so the title can describe the exchange " + "rather than only the question. Clear it to stop asking a model for " + "titles: chats are then named from their first message, and no request " + "is made at all. The emoji is asked for rather than assumed — it makes " + "a sidebar of twenty chats scannable — and a model that ignores the " + "instruction simply gives a title without one.", default=( - "Summarise this exchange as a title of at most six words. Reply with the " - "title alone: no quotes, no punctuation at the end, no preamble. Use the " - "language of the exchange.\n" + "Summarise this exchange as a title of at most six words, beginning with " + "a single emoji that fits it. Reply with the title alone: no quotes, no " + "punctuation at the end, no preamble, no explanation. Use the language of " + "the exchange.\n" "\n" "User: {{question}}\n" "\n" diff --git a/tests/test_chat.py b/tests/test_chat.py index d2320ce..bc6923e 100644 --- a/tests/test_chat.py +++ b/tests/test_chat.py @@ -156,6 +156,106 @@ async def test_the_title_prompt_carries_the_exchange(mock_http): assert seen == ["Name this: What is lembas? / Waybread. / {{nonsense}}"] +async def test_a_title_from_a_model_that_thinks_first(mock_http): + """The bug: a reasoning model puts `` in the very field the title is + read from, so every chat on one was named "Okay, the user wants a + short title for" -- or, once the guard caught that as too long, fell back to + the first prompt and looked as though titling had never run at all.""" + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "choices": [ + { + "message": { + "content": "Six words, an emoji\n" + "🌳 Mallorn trees explained" + } + } + ] + }, + ) + + mock_http(handler) + title = await chat_service.generate_title( + Endpoint("http://x.test", "", {}), + "m", + "What is a mallorn?", + "A golden tree.", + template="Name this: {{question}}", + ) + assert title == "🌳 Mallorn trees explained" + + +async def test_the_title_call_leaves_room_to_think(mock_http): + """24 tokens is ample for six words and nowhere near enough for a model that + reasons first: the budget went on thinking and the content came back empty. + Too small is not a shorter title, it is no title.""" + seen: list[dict] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(json.loads(request.content)) + return httpx.Response(200, json={"choices": [{"message": {"content": "A name"}}]}) + + mock_http(handler) + await chat_service.generate_title( + Endpoint("http://x.test", "", {}), "m", "q", "a", template="Name this: {{question}}" + ) + assert seen[0]["max_tokens"] == chat_service.TITLE_MAX_TOKENS + assert seen[0]["max_tokens"] >= 256 + + +async def test_the_title_call_sends_no_reasoning_effort(mock_http): + """Tempting, and wrong. Those two fields appear only when somebody has opted + in, so a provider strict about unknown parameters sees the request it always + did -- and a 400 here is caught and turned into a fallback title, which is + titling silently switching itself off.""" + seen: list[dict] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(json.loads(request.content)) + return httpx.Response(200, json={"choices": [{"message": {"content": "A name"}}]}) + + mock_http(handler) + await chat_service.generate_title( + Endpoint("http://x.test", "", {}), "m", "q", "a", template="Name this: {{question}}" + ) + assert "reasoning_effort" not in seen[0] + assert "chat_template_kwargs" not in seen[0] + + +async def test_a_title_that_is_only_thinking_falls_back(mock_http): + """Nothing but reasoning means nothing to name it with. The first prompt is + a better title than an empty one.""" + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, json={"choices": [{"message": {"content": "still deciding"}}]} + ) + + mock_http(handler) + title = await chat_service.generate_title( + Endpoint("http://x.test", "", {}), + "m", + "What is a mallorn?", + "A golden tree.", + template="Name this: {{question}}", + ) + assert title == "What is a mallorn?" + + +def test_the_shipped_title_prompt_asks_for_an_emoji(): + """It makes a sidebar of twenty chats scannable, and it is asked for rather + than assumed -- a model that ignores it gives a title without one.""" + from lembas.services import prompts + + fragment = next(f for f in prompts.BUILTIN if f.key == "task.title") + assert "emoji" in fragment.default + assert "{{question}}" in fragment.default + assert "{{answer}}" in fragment.default + + async def test_an_empty_title_prompt_asks_no_model_at_all(mock_http): """Clearing the fragment is how auto-titling is turned off. It must not cost a request that is then thrown away."""