armistace/beer_data
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(advice): a deterministic split, not a guess at where the advice starts

Browse Source 
The stripper tried to work out where the model's working ends and the
advice begins by classifying prose - marker words, hedging words,
length.  It failed in BOTH directions, and the review caught both:

- it emptied legitimate advice (a 1,093-character reply containing one
  hedging word and no marker phrase was reduced to ""), so the brewer
  saw nothing at all; and
- it let the model's working through in full (a 1,053-character reply
  with no hedging word was returned verbatim), so the brewer was shown
  the model thinking about their beer.

Neither a marker list nor a length threshold is reliable.  Prose is not
a format.

So the model is asked for the format instead.  Its reply must have this
shape:

    ADVICE:
    <the advice itself>

Everything before the marker is working, everything from it on is the
advice.  The split is exact, with no judgement calls.

Verified against the live model, four different brews:

    Test Pale Ale        marker@pos=0  finish=stop  advice_len=656
    Nocturna Christmas   marker@pos=0  finish=stop  advice_len=668
    Ginger beer          marker@pos=0  finish=stop  advice_len=621
    Cider raspberry      marker@pos=0  finish=stop  advice_len=539

All four start clean, with no marker left in the advice.

A reply with no marker is used as it stands - the safe direction, where
the brewer sees a slightly rambling reply rather than no advice at all.

An empty advice is now recorded as a FAILURE rather than stored as a
successful generation.  Storing an empty string with advice_ok=True
made the page render "Thinking..." for ever, because the template's
`{% if reading.advice %}` was false - so the brewer never learned that
it had failed.

181 tests (up from 184 - the eight heuristic-era stripper tests are
replaced by six that test the marker behaviour).
This commit is contained in:
hermes
2026-11-09 19:44:52 +10:00
parent cbc71716e8
commit 75f94d47e2
2 changed files with 99 additions and 313 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_brew_advice.py
+55 -125
View File
@@ -117,7 +117,7 @@ def test_a_successful_call_returns_the_text(monkeypatch):
class Response:
def read(self):
return json.dumps({
"choices": [{"message": {"content": " Leave it alone, it is still fermenting. "}}]
"choices": [{"message": {"content": "ADVICE: Leave it alone, it is still fermenting."}}]
}).encode()
def __enter__(self):
@@ -226,38 +226,70 @@ def test_the_token_budget_leaves_room_for_the_reasoning(monkeypatch):
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.
def test_the_model_is_asked_for_an_advice_marker():
"""The marker is what makes the split deterministic.
Guessing where the advice begins by classifying prose failed in both
directions - it emptied legitimate advice, and it let the model's
working through in full. A marker the model writes itself is exact.
"""
assert "ADVICE:" in brew_advice.SYSTEM_PROMPT
def test_the_working_before_the_marker_is_dropped():
"""Everything before the marker is the model's working."""
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."
"Let me think about what this reading means. The original gravity "
"is 1.055 and the current reading is 1.030, which is a drop of 25 "
"points over nine days, and I need to work out what that means for "
"the expected attenuation for this style at this temperature.\n\n"
"ADVICE:\n"
"Warm the fermenter to 20C and give the yeast a gentle swirl to "
"rouse it, then take another reading in three days."
)
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
assert stripped.startswith("Warm the fermenter")
assert "Let me think" not in stripped
assert "1.055" 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.")
def test_a_reply_with_the_marker_is_kept_whole():
"""A legitimate advice reply is never emptied.
Whatever length it is, and whatever hedging words it happens to
contain, the advice after the marker is the advice.
"""
advice = (
"Actually the drop is fine. Perhaps it will finish this week. Wait "
"a couple of days and check again, and if it has not moved, give "
"the yeast a gentle swirl and take another reading before you "
"consider repitching anything."
)
reply = "ADVICE:\n" + advice
assert brew_advice._strip_reasoning(reply) == advice
def test_a_reply_without_the_marker_is_used_as_it_stands():
"""The safe direction when the model does not use the marker.
The brewer sees a slightly rambling reply rather than no advice at
all.
"""
reply = "The gravity looks fine for day nine. Give it three more days."
assert brew_advice._strip_reasoning(reply) == reply
def test_a_reply_with_only_working_after_the_marker_is_empty():
"""Nothing after the marker means there is no advice."""
assert brew_advice._strip_reasoning("ADVICE:") == ""
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.
@@ -287,7 +319,7 @@ def test_a_short_reply_is_not_trimmed():
def test_the_advice_is_truncated():
"""A runaway reply must not fill the reading document."""
long_text = "x" * 5000
long_text = "ADVICE: " + ("x" * 5000)
class Response:
def read(self):
@@ -640,112 +672,10 @@ def test_a_comment_only_correction_cannot_be_overwritten(repo):
assert after["comment"] == "corrected comment"
def test_a_reply_that_is_all_working_stores_no_working():
"""A reply with no advice paragraph in it must not be stored.
The old code returned the text unchanged, which was a no-op - and in
the case where a preamble marker WAS present it returned the whole
reply, working included. So the brewer was shown the model thinking
about their beer instead of any advice at all.
"""
working_only = (
"Let me think about what this reading means. The original gravity is "
"1.055 and the current reading is 1.030, which is a drop of 25 points "
"over nine days at 19 degrees, and I need to work out what that means "
"for the expected attenuation.\n\n"
"Perhaps the drop is smaller than expected, and it could be that the "
"yeast is stressed by the temperature rather than actually done, "
"because most ale strains slow down considerably below 18 degrees.\n\n"
"Wait, I need to keep in mind that the hydrometer was read at a "
"different temperature than the sample, which would make the reading "
"read lower than it really is, and I have no way of knowing from the "
"data alone what the correction factor was.\n\n"
"I need to keep in mind that the original gravity here was measured "
"before the hops went in, and the reading on brew day was 1.055, so "
"the drop is twenty-five points against a potential of fifty-five.\n\n"
"So the question is what the expected attenuation should be for this "
"style at this temperature, and I am not sure without more data or a "
"temperature-corrected reading to compare against."
)
stripped = brew_advice._strip_reasoning(working_only)
assert stripped == "", (
f"the model's working was kept as advice: {stripped[:120]!r}"
)
def test_a_short_reply_is_advice_even_without_a_marker_word():
"""Real advice is often phrased without any marker phrase.
"Actually the drop is fine. Wait a couple of days and check again."
is advice - and judging it paragraph by paragraph emptied it, so the
brewer saw nothing at all.
"""
reply = ("Actually the drop is fine.\n\n"
"Wait a couple of days and check again.")
assert brew_advice._strip_reasoning(reply) == reply
def test_a_long_legitimate_advice_reply_is_kept():
"""Length alone is a blunt instrument.
A legitimate advice reply can run to a thousand characters, and a
short working reply can be a few hundred - so a length threshold
gets both wrong. What separates them is the SHAPE: the working
hedges about the data, the advice tells the brewer what to do.
"""
advice = (
"The ferment has dropped steadily since brew day and the numbers "
"line up with a healthy fermentation, so the yeast is clearly "
"working through the wort rather than sitting on it. At 19 degrees "
"this is on the cool side for an ale yeast and it will finish "
"slowly, but that is not a problem in itself. Give the ferment "
"another three or four days before you draw any conclusions, and "
"take a reading on the eighteenth so you can see whether it is "
"still moving. Do not raise the temperature in a hurry either, "
"because a sudden change while the yeast is finishing can make it "
"give up what it has left to give. If it is still stuck at the "
"same gravity after that, then it is worth rousing the yeast with "
"a gentle swirl and considering a fresh sachet of dry yeast, "
"rehydrated in the usual way and pitched at the same temperature "
"the beer is sitting at."
)
assert len(advice) > 600, "the sample should be longer than the threshold"
assert brew_advice._strip_reasoning(advice) == advice
def test_a_long_reply_with_no_instruction_is_dropped():
"""Long, and nothing in it reads like an instruction - all working."""
working = ("Let me think about this.\n\n" +
"Perhaps the yeast is stressed, and it could be that the "
"temperature is too low. " * 20)
assert brew_advice._strip_reasoning(working) == ""
def test_a_long_reply_with_an_instruction_keeps_it():
"""The working is trimmed from it, but the advice stays."""
mixed = ("Let me think about this.\n\n" +
"Perhaps the yeast is stressed. " * 15 +
"\n\nWarm the fermenter to 20C and give the yeast a gentle "
"swirl to rouse it.")
out = brew_advice._strip_reasoning(mixed)
assert "Warm the fermenter" in out
def test_a_paragraph_about_keeping_something_in_mind_is_not_advice():
""""Keep" alone is not an advice marker.
"I need to keep in mind that the hydrometer was read at a different
temperature" is the model thinking, not advice to the brewer.
"""
working = ("I need to keep in mind that the hydrometer was read at a "
"different temperature, which explains the discrepancy.")
assert brew_advice._looks_like_advice(working) is False
def test_the_token_is_the_primary_guard(repo):
@@ -898,7 +828,7 @@ def test_a_reply_cut_off_at_the_token_ceiling_is_marked(repo):
return json.dumps({
"choices": [{
"finish_reason": "length",
"message": {"content": "The gravity looks fine so far, but"},
"message": {"content": "ADVICE: The gravity looks fine so far, but"},
}],
}).encode()