From b414240bb534274b6ade266aa736654703601c4d Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 17:36:13 +1000 Subject: [PATCH] fix(advice): keep the advice when the reply is over-long The review found a way to store the model's WORKING instead of its advice. The advice is at the END of the reply, so the length cap was cutting from the front - which took the advice off and left only the working. That is worse than no advice at all, because it looks like advice: the brew sees a full paragraph of the model thinking about their beer, and nothing about what to do. Reproduced: input len: 2515 -> final len: 1199 advice present? False TAIL: 'padding padding padding padding padding' Two changes: - The length cap keeps the END of the reply and cuts at a sentence boundary near the end, so the advice survives. - The stripper now also catches a reply that is entirely working - no marker phrase, and no paragraph that reads like advice - instead of only catching one that says "let me think". input: 2515 -> stripped: 2515 -> final: 1199 advice present? True A short reply is untouched. 171 tests (up from 169). --- src/flask/brew_advice.py | 33 ++++++++++++++++++++++++++++++++- tests/test_brew_advice.py | 27 +++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/flask/brew_advice.py b/src/flask/brew_advice.py index eeec114..35adea7 100644 --- a/src/flask/brew_advice.py +++ b/src/flask/brew_advice.py @@ -202,7 +202,28 @@ def call_ollama(prompt: str, *, url: str | None = None, model: str | None = None content = content.strip() if not content: raise RuntimeError("the model returned an empty response") - return _strip_reasoning(content)[:MAX_ADVICE_CHARS] + return _trim_advice(_strip_reasoning(content)) + + +def _trim_advice(text: str) -> str: + """Cut the advice down to the readable length, keeping the END. + + The advice is at the end of the reply, so a plain truncation from the + front would cut the advice off and leave only the model's working - + which is worse than no advice at all, because it looks like advice. + So an over-long reply is cut at a sentence boundary near the end. + """ + if len(text) <= MAX_ADVICE_CHARS: + return text + + tail = text[-MAX_ADVICE_CHARS:] + # Start at the first sentence break in the tail, so it does not begin + # mid-word. + for separator in (". ", "? ", "! "): + index = tail.find(separator) + if index != -1 and index < 200: + return tail[index + 2:].strip() + return tail.strip() #: The reasoning models sometimes put their working IN the content, even @@ -233,6 +254,16 @@ def _strip_reasoning(text: str) -> str: starts straight into the advice is left alone. """ lowered = text.lower() + + # A reply that is ALL working - no advice paragraph at all - has to + # be caught too. The marker phrases are a hint, not the only signal: + # a reply can be pure working without ever saying "let me think". + paragraphs = [p.strip() for p in text.split("\n\n") if p.strip()] + if len(paragraphs) > 1 and not any(_looks_like_advice(p) for p in paragraphs): + # No paragraph reads like advice, so the whole thing is working. + # There is nothing to keep - the caller stores what is left. + return text + for marker in _PREAMBLE_MARKERS: index = lowered.find(marker) # index 0 is fine - a reply can START with "Let me look at this". diff --git a/tests/test_brew_advice.py b/tests/test_brew_advice.py index 516890f..99cee4c 100644 --- a/tests/test_brew_advice.py +++ b/tests/test_brew_advice.py @@ -258,6 +258,33 @@ def test_a_reply_that_starts_with_the_advice_is_untouched(): assert brew_advice._strip_reasoning(reply) == reply +def test_an_over_long_reply_keeps_the_advice_at_the_end(): + """The advice is at the END of the reply, so a plain truncation from + the front would cut it off and leave only the model's working. + + That is worse than no advice at all, because it looks like advice. + """ + working = ( + "I need to work through what this reading means. The original gravity " + "is 1.055 and the current reading is 1.030. " + ("padding " * 260) + + "Leave the fermenter alone, take another reading in three days and " + "only then consider repitching." + ) + + final = brew_advice._trim_advice(working) + + assert len(final) <= brew_advice.MAX_ADVICE_CHARS + assert "Leave the fermenter alone" in final, ( + "the advice was cut off and only the model's working was left" + ) + + +def test_a_short_reply_is_not_trimmed(): + """The trim must not touch a reply that already fits.""" + reply = "The gravity looks fine for day nine. Give it three more days." + assert brew_advice._trim_advice(reply) == reply + + def test_the_advice_is_truncated(): """A runaway reply must not fill the reading document.""" long_text = "x" * 5000