diff --git a/src/flask/brew_advice.py b/src/flask/brew_advice.py index 1304614..ecfc5bf 100644 --- a/src/flask/brew_advice.py +++ b/src/flask/brew_advice.py @@ -271,11 +271,13 @@ def _strip_reasoning(text: str) -> str: # 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". + # + # The working is DROPPED, not returned. Returning it unchanged would + # be a no-op, and the brewer would be shown the model thinking about + # their beer instead of any advice at all. 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 + return "" for marker in _PREAMBLE_MARKERS: index = lowered.find(marker) @@ -309,21 +311,29 @@ def _strip_reasoning(text: str) -> str: #: Words that mark a paragraph as the ADVICE rather than the model's #: working. The advice talks about what the brewer should DO. +#: Phrases that mark a paragraph as the ADVICE rather than the model's +#: working. They are matched as PHRASES, not bare words - "keep" alone +#: matches "I need to keep in mind that the hydrometer was read at a +#: different temperature", which is working, not advice. _ADVICE_MARKERS = ( "you should", "you could", "you may want", "you might", + "you will want", "give it", - "let it", - "take another", - "take the", - "raise", - "lower", - "rack", - "keep", - "suggest", - "recommend", + "let it sit", + "let it ferment", + "take another reading", + "take a reading", + "raise it to", + "raise the", + "lower the", + "rack it", + "you could repitch", + "i would suggest", + "i suggest", + "i recommend", "in your case", "here is", "here's", @@ -348,7 +358,9 @@ def _looks_like_advice(paragraph: str) -> bool: 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") + "that is a discrepancy", "which means", + "i need to keep in mind", "the question is", + "this could mean", "it could be", "or maybe") if any(word in lowered for word in working_words): return False return True diff --git a/src/flask/mongo/repository.py b/src/flask/mongo/repository.py index 931eb1c..a0f0505 100644 --- a/src/flask/mongo/repository.py +++ b/src/flask/mongo/repository.py @@ -263,10 +263,14 @@ class BeerRepository: return 0 query: dict = {"_id": {"$in": doc_id}} - if expected_sg is not None: - query["sg"] = str(expected_sg) if expected_token is not None: + # The editing token is the PRIMARY guard: it changes on every + # edit, so it catches a stale generation whatever else + # changed - the gravity, the comment, or nothing. query["advice_token"] = str(expected_token) + elif expected_sg is not None: + # No token - a direct call with a gravity to check against. + query["sg"] = str(expected_sg) # `$in` matches either form. A plain list would be an exact # ARRAY match in Mongo, which is not what is wanted here. diff --git a/tests/test_brew_advice.py b/tests/test_brew_advice.py index 960f47c..314c9a4 100644 --- a/tests/test_brew_advice.py +++ b/tests/test_brew_advice.py @@ -640,6 +640,76 @@ 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.\n\n" + "Perhaps the drop is smaller than expected, and it could be that the " + "yeast is stressed.\n\n" + "Wait, I need to keep in mind that the hydrometer was read at a " + "different temperature.\n\n" + "I need to keep in mind that the original gravity here was measured " + "before the hops went in.\n\n" + "So the question is what the expected attenuation should be, and I " + "am not sure without more data." + ) + + stripped = brew_advice._strip_reasoning(working_only) + + assert stripped == "", ( + f"the model's working was kept as advice: {stripped[:120]!r}" + ) + + +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): + """The token changes on EVERY edit, so it catches a stale generation + whatever else changed. + + The gravity alone is not enough - a comment-only correction leaves it + unchanged - and it can be unparseable for some readings, so + `expected_sg` can be None. The token is what the guard relies on. + """ + reading_id = repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1050, + reading_date="2026-01-01", + final_reading=False, comment="first") + token = repo.readings.find_one({"beer_run_id": "1"})["advice_token"] + + # A direct call with no generation context is not conditional - it is + # a deliberate write, so it goes through. + assert repo.set_reading_advice(reading_id, "DIRECT ADVICE", True) == 1 + + # The brewer edits the reading, so the generation changes. + repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1050, + reading_date="2026-01-01", final_reading=False, + comment="edited") + + # A write for the generation that no longer exists is refused, even + # with no gravity to check against. + assert repo.set_reading_advice(reading_id, "STALE ADVICE", True, + expected_sg=None, + expected_token=token) == 0 + after = repo.readings.find_one({"beer_run_id": "1"}) + # The edit cleared the advice, and the stale write did not put it back. + assert after.get("advice") is None + + def test_a_reply_cut_off_at_the_token_ceiling_is_marked(repo): """A reply that stopped at the token limit is incomplete.