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).
This commit is contained in:
@@ -202,7 +202,28 @@ def call_ollama(prompt: str, *, url: str | None = None, model: str | None = None
|
|||||||
content = content.strip()
|
content = content.strip()
|
||||||
if not content:
|
if not content:
|
||||||
raise RuntimeError("the model returned an empty response")
|
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
|
#: 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.
|
starts straight into the advice is left alone.
|
||||||
"""
|
"""
|
||||||
lowered = text.lower()
|
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:
|
for marker in _PREAMBLE_MARKERS:
|
||||||
index = lowered.find(marker)
|
index = lowered.find(marker)
|
||||||
# index 0 is fine - a reply can START with "Let me look at this".
|
# index 0 is fine - a reply can START with "Let me look at this".
|
||||||
|
|||||||
@@ -258,6 +258,33 @@ def test_a_reply_that_starts_with_the_advice_is_untouched():
|
|||||||
assert brew_advice._strip_reasoning(reply) == reply
|
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():
|
def test_the_advice_is_truncated():
|
||||||
"""A runaway reply must not fill the reading document."""
|
"""A runaway reply must not fill the reading document."""
|
||||||
long_text = "x" * 5000
|
long_text = "x" * 5000
|
||||||
|
|||||||
Reference in New Issue
Block a user