From cf08b751e48a55ea23158a35a2f85d4e8ef37706 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 17:10:24 +1000 Subject: [PATCH] feat(advice): use mistral-large-4, and cope with models that show their working The model is now `mistral-large-4:cloud`, on the same Ollama instance. It was probed before being made the default - `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. What the probes showed ---------------------- `think: false` does NOT stop the reasoning. It only changes where the working goes: with thinking off, the model writes it into the CONTENT instead of a separate field. Either way the reasoning comes out of the same token allowance as the answer, and at 500-3000 tokens the reply came back EMPTY - which looks like a broken model rather than a budget problem. The budget is now 8000, and the timeout 120s. The reply is also stripped of a reasoning preamble. The advice is the prose at the END; the working comes first, in several paragraphs. The preamble is found by walking back from the end of the reply and keeping the paragraphs that read like advice to the brewer, rather than like the model thinking out loud. Verified against the live model, on a stalled brew (OG 1.042, 1.022 after nine days, "no change since day 7"): That 1.022 reading after nine days at 19C - with no movement since day seven - looks like a stalled fermentation; a 1.042 wort should be well on its way to 1.010-1.012 by now, not sitting at only about 48% attenuation. Gently swirl the fermenter to rouse the yeast cake and keep the temperature steady at 19-20C, and if the gravity hasn't budged in another 48 hours, pitch a fresh, healthy yeast starter... And on a CORRECTED older reading - the case the review found broken - the advice is about the corrected reading, with the later one as context: A brew-day reading of 1.038 against an OG of 1.042 is only four points low, which is almost certainly just the normal difference between your kettle gravity and the post-transfer fermenter reading... 169 tests (up from 167). --- src/flask/brew_advice.py | 144 ++++++++++++++++++++++++++++++++++---- tests/test_brew_advice.py | 37 +++++++++- 2 files changed, 168 insertions(+), 13 deletions(-) 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():