fix(advice): guard the advice write on the comment as well as the token (#21)

fix(advice): guard the advice write on the comment as well as the token (#21)

src/flask/mongo/repository.py

@@ -18,6 +18,7 @@ Two collections are used:
 from __future__ import annotations
 
 import logging
+import re
 import os
 import threading
 from typing import Any
@@ -232,7 +233,8 @@ class BeerRepository:
     def set_reading_advice(self, reading_id: str, advice: str,
                             ok: bool = True,
                             expected_sg: int | None = None,
-                            expected_token: str | None = None) -> int:
+                            expected_token: str | None = None,
+                            expected_comment: str | None = None) -> int:
         """Attach the LLM's advice to a reading.
 
         The advice lives in its own field, so the brewer's own comment is
@@ -286,6 +288,31 @@ class BeerRepository:
             # A direct call with a gravity to check against.
             query["sg"] = str(expected_sg)
 
+        if expected_comment is not None:
+            # The comment the advice was written for.
+            #
+            # This is a SECOND, independent guard.  The token covers
+            # every edit the application makes, but a document edited
+            # outside the application - by a script, or by hand in a
+            # database tool - would keep its empty token and a stale
+            # thread's write would still match on the gravity alone.
+            #
+            # The advice is generated from the comment as well as the
+            # gravity, so the comment is part of what the advice
+            # describes.  Matching on it means a stale write can only
+            # land if BOTH the gravity and the comment are unchanged.
+            #
+            # The stored comment and the one the view sees can differ by
+            # trailing whitespace, so a regex is used to match either.
+            #
+            # `^comment\s*$` matches the stored value with any trailing
+            # whitespace, and the exact value too.  It is anchored, so it
+            # cannot match a longer comment that merely starts the same
+            # way.
+            query["comment"] = {
+                "$regex": "^" + re.escape(expected_comment.strip()) + r"\s*$"
+            }
+
         # `$in` matches either form.  A plain list would be an exact
         # ARRAY match in Mongo, which is not what is wanted here.
         result = self.readings.update_one(

src/flask/views/brews.py

@@ -127,6 +127,11 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
     # cannot overwrite a correction with advice about the old value.
     advice_sg = latest.get("sg")
     advice_token = latest.get("advice_token")
+    # The comment is part of what the advice describes, so it is a
+    # second, independent guard on the write.  The token covers every
+    # edit the application makes; this also covers a document edited
+    # outside it.
+    advice_comment = latest.get("comment") or ""
 
     def write(read_id: str, ok: bool, text: str) -> None:
         # The client is SHARED, not built per reading.  A MongoClient
@@ -145,6 +150,7 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
                 read_id, text, ok,
                 expected_sg=advice_sg,
                 expected_token=advice_token,
+                expected_comment=advice_comment,
             )
             if not written:
                 # The reading was corrected while the advice was being
@@ -153,8 +159,8 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
                 # reading will land in its own turn.
                 log.info(
                     "the advice for reading %s was not stored: the reading "
-                    "has been edited since the advice was generated, so the "
-                    "advice no longer matches it",
+                    "has been edited since the advice was generated, so its "
+                    "editing token or comment no longer matches the advice",
                     read_id,
                 )
         except Exception:  # noqa: BLE001 - never let the thread die noisily

tests/test_brew_advice.py

@@ -1121,6 +1121,47 @@ def test_stale_advice_cannot_overwrite_a_comment_only_correction(repo):
     assert doc["comment"] == "corrected comment"
 
 
+def test_stale_advice_cannot_overwrite_a_comment_edited_outside_the_app(repo):
+    """The comment is a guard in its own right, not just the token.
+
+    The token covers every edit the application makes.  A document
+    edited outside it - by a script, or by hand - keeps its empty
+    token, so a stale thread's write would still match on the gravity
+    alone.  Matching the comment as well closes that.
+    """
+    repo.readings.insert_one({
+        "_id": "outsideedit01",
+        "beer_run_id": "1",
+        "date": "2026-01-01",
+        "sg": "1040",
+        "comment": "original comment",
+        "final_reading": "False",
+        # No advice_token - a legacy reading.
+    })
+
+    # The advice is written for the original comment.
+    assert repo.set_reading_advice(
+        "outsideedit01", "FRESH ADVICE", ok=True,
+        expected_token="", expected_comment="original comment") == 1
+
+    # Somebody edits the comment directly in the database, leaving the
+    # token empty - so the token cannot tell the generations apart.
+    repo.readings.update_one(
+        {"_id": "outsideedit01"},
+        {"$set": {"comment": "edited outside the app"}},
+    )
+
+    # The slow thread lands.  It must be refused, even though the token
+    # still matches.
+    assert repo.set_reading_advice(
+        "outsideedit01", "STALE ADVICE", ok=True,
+        expected_token="", expected_comment="original comment") == 0
+
+    doc = repo.readings.find_one({"_id": "outsideedit01"})
+    assert doc["advice"] == "FRESH ADVICE"
+    assert doc["comment"] == "edited outside the app"
+
+
 def test_a_failed_advice_is_marked_as_such(client, repo):
     """A generation failure reads differently from real advice."""
     repo.users.insert_one({"username": "beer", "password": "beer_pass"})