diff --git a/src/flask/brew_advice.py b/src/flask/brew_advice.py index da914dc..eeec114 100644 --- a/src/flask/brew_advice.py +++ b/src/flask/brew_advice.py @@ -33,14 +33,25 @@ log = logging.getLogger(__name__) #: The Ollama instance on the homelab. Reachable from the pod. DEFAULT_OLLAMA_URL = os.getenv("OLLAMA_URL", "http://192.168.178.45:11434") -#: A model that is known to work. `deepseek-v4-flash:cloud`, -#: `deepseek-v3.2:cloud` and `qwen3.5:cloud` are all retired and fail at -#: call time, so they are not safe defaults. -DEFAULT_MODEL = os.getenv("OLLAMA_MODEL", "glm-5.3:cloud") +#: The model to use. Override with the ``OLLAMA_MODEL`` environment +#: variable. +#: +#: A model that does NOT reason is much better here. The reasoning +#: models emit their thinking out of the same token allowance as the +#: answer, and several of them burned the whole budget on thinking and +#: returned an EMPTY reply - which looks like a broken model rather than +#: a budget problem. With `think: false` the model still shows its +#: working, but inline in the content, so the answer can be taken from +#: the end of it. +#: +#: `deepseek-v4-flash:cloud`, `deepseek-v3.2:cloud` and `qwen3.5:cloud` +#: are all retired and fail at call time while still appearing in the +#: model list, so the default is one that was probed. +DEFAULT_MODEL = os.getenv("OLLAMA_MODEL", "mistral-large-4:cloud") #: How long to wait for the model before giving up. The advice is #: generated in the background, so this only bounds the background thread. -DEFAULT_TIMEOUT = float(os.getenv("LLM_TIMEOUT", "45")) +DEFAULT_TIMEOUT = float(os.getenv("LLM_TIMEOUT", "120")) #: The advice is advice, not instruction. Keep it short - it has to be #: readable on the brewing page next to the brewer's own comment. @@ -163,11 +174,14 @@ def call_ollama(prompt: str, *, url: str | None = None, model: str | None = None ], "temperature": 0.4, # The reasoning models emit their thinking first, and it comes out - # of the same budget. At 500 tokens the reasoning used the whole - # allowance and the reply came back empty - which looked like a - # broken model rather than a budget problem. 2000 leaves room for - # the reasoning AND the advice. - "max_tokens": 3000, + # of the SAME budget as the answer. At 500 tokens the reasoning + # used the whole allowance and the reply came back empty - which + # looks like a broken model rather than a budget problem. + # + # `think: false` does NOT help: the models still think, they just + # put the working in the content instead of a separate field. + # So the budget has to cover the reasoning AND the advice. + "max_tokens": 8000, "stream": False, }).encode("utf-8") @@ -183,11 +197,117 @@ def call_ollama(prompt: str, *, url: str | None = None, model: str | None = None choices = body.get("choices") or [] if not choices: raise RuntimeError("the model returned no choices") - content = (choices[0].get("message") or {}).get("content") or "" + message = choices[0].get("message") or {} + content = message.get("content") or "" content = content.strip() if not content: raise RuntimeError("the model returned an empty response") - return content[:MAX_ADVICE_CHARS] + return _strip_reasoning(content)[:MAX_ADVICE_CHARS] + + +#: The reasoning models sometimes put their working IN the content, even +#: with thinking turned off - "Let me look at this..." for several +#: thousand characters before the actual advice. The advice is the last +#: few paragraphs, so the preamble is cut off. +_PREAMBLE_MARKERS = ( + "let me look", + "let me think", + "let me work", + "okay, let me", + "first, let me", + "hmm,", + "wait,", + "let me analyze", + "let me consider", +) + + +def _strip_reasoning(text: str) -> str: + """Cut a reasoning preamble off the front of a reply. + + The advice is the prose at the END - the working comes first, in + several paragraphs. So the preamble is removed by finding the last + paragraph break before the advice starts, not the first. + + Only a preamble that looks like working is removed; a reply that + starts straight into the advice is left alone. + """ + lowered = text.lower() + for marker in _PREAMBLE_MARKERS: + index = lowered.find(marker) + # index 0 is fine - a reply can START with "Let me look at this". + if index < 0: + continue + + tail = text[index:] + # The advice is the LAST paragraph of the tail. Split on the + # blank lines and take the paragraphs from the end, until the + # running text stops looking like working. + paragraphs = [p.strip() for p in tail.split("\n\n") if p.strip()] + if len(paragraphs) < 2: + continue + + # Walk back from the end while the paragraph still reads like the + # model's working rather than the brewer's advice. + kept = [paragraphs[-1]] + for paragraph in reversed(paragraphs[:-1]): + if _looks_like_advice(paragraph): + kept.append(paragraph) + else: + break + kept.reverse() + + candidate = "\n\n".join(kept).strip() + if len(candidate) >= 80: + return candidate + return text + + +#: Words that mark a paragraph as the ADVICE rather than the model's +#: working. The advice talks about what the brewer should DO. +_ADVICE_MARKERS = ( + "you should", + "you could", + "you may want", + "you might", + "give it", + "let it", + "take another", + "take the", + "raise", + "lower", + "rack", + "keep", + "suggest", + "recommend", + "in your case", + "here is", + "here's", + "don't", + "do not", +) + + +def _looks_like_advice(paragraph: str) -> bool: + """True when a paragraph reads like advice to the brewer. + + The model's working is full of hedging - "perhaps", "or maybe", "it + could be" - and is about interpreting the numbers. The advice is + about what to do. + """ + lowered = paragraph.lower() + if any(marker in lowered for marker in _ADVICE_MARKERS): + return True + # A paragraph that reads like working is not advice. The signs are + # hedging ("perhaps", "could be"), and the model talking to itself + # ("wait", "hmm", "so", "actually") rather than to the brewer. + working_words = ("perhaps", "maybe", "possibly", "could be", "might be", + "i think", "i am not sure", "actually", "hmm", "wait", + "let me", "so on brew day", "there is a discrepancy", + "that is a discrepancy", "but the", "which means") + if any(word in lowered for word in working_words): + return False + return True def generate_advice(run: dict, reading: dict, history: list[dict], *, diff --git a/tests/test_brew_advice.py b/tests/test_brew_advice.py index 3a12d0f..516890f 100644 --- a/tests/test_brew_advice.py +++ b/tests/test_brew_advice.py @@ -220,7 +220,42 @@ def test_the_token_budget_leaves_room_for_the_reasoning(monkeypatch): brew_advice.generate_advice({"canonical_name": "Ale"}, {"date": "2026-01-01"}, []) - assert captured["body"]["max_tokens"] >= 2000 + # The reasoning comes out of the same budget as the answer, and + # `think: false` does not stop it - the models just move the working + # into the content. The budget has to cover both. + assert captured["body"]["max_tokens"] >= 8000 + + +def test_a_reasoning_preamble_is_cut_off(): + """Some models put their working in the content even with thinking off. + + The advice is the prose at the end; the working comes first, in + several paragraphs. The preamble is removed by finding the advice, + not by taking the text after the first blank line. + """ + reply = ( + "Let me look at this. The brewer has a Christmas beer, OG 1.042, " + "fermenting at 19C.\n\n" + "So on brew day, the reading is 1.038, but the OG is stated as 1.042. " + "That is a discrepancy of 4 points.\n\n" + "Wait, the reading on brew day is 1.038 but the recorded OG is 1.042.\n\n" + "On brew day a gravity of 1.038 against a recorded original of 1.042 " + "is a small discrepancy but not a problem. Take the OG from a " + "well-mixed sample, and start the fermentation at 18-20C." + ) + + stripped = brew_advice._strip_reasoning(reply) + + assert stripped.startswith("On brew day a gravity of 1.038") + assert "Let me look at this" not in stripped + assert "That is a discrepancy of 4 points" not in stripped + + +def test_a_reply_that_starts_with_the_advice_is_untouched(): + """The stripper must not eat a good reply.""" + reply = ("The gravity looks fine for day nine. Give it three more days " + "before you do anything, and take another reading on Friday.") + assert brew_advice._strip_reasoning(reply) == reply def test_the_advice_is_truncated():