fix(ui): make the advice pop-out fail-safe against a stale stylesheet (#22)

fix(ui): make the advice pop-out fail-safe against a stale stylesheet (#22)

src/flask/app_factory.py

@@ -87,6 +87,31 @@ def create_app(config: dict | None = None) -> Flask:
     if config:
         app.config.update(config)
 
+    # The pages must not be cached by the browser.
+
+    # A phone held on to a cached copy of the brewing page and showed
+    # the old inline advice instead of the button and the pop-out, so
+    # the change was not visible until the cache was cleared by hand.
+    @app.after_request
+    def _no_store(response):
+        if response.mimetype == "text/html":
+            response.headers["Cache-Control"] = "no-store, must-revalidate"
+            response.headers["Pragma"] = "no-cache"
+        elif response.mimetype in ("text/css", "application/javascript",
+                                    "text/javascript"):
+            # The static files must NEVER be reused from a cache.
+
+            # Firefox kept serving a stale stylesheet even with
+            # `no-cache`, which meant the page had the new markup and
+            # the old CSS: the advice was expanded inline in the table
+            # instead of in a pop-out, and the long and short labels
+            # were both showing ("BREWING ADVICEADVICE").
+
+            # `no-store` is the only one that means "do not keep this".
+            response.headers["Cache-Control"] = "no-store, must-revalidate"
+            response.headers["Pragma"] = "no-cache"
+        return response
+
     csrf = CSRFProtect(app)
 
     # ------------------------------------------------------------------ #

src/flask/brew_advice.py

@@ -25,6 +25,7 @@ from __future__ import annotations
 import json
 import logging
 import os
+import re
 import urllib.error
 import urllib.request
 
@@ -33,14 +34,25 @@ log = logging.getLogger(__name__)
 #: The Ollama instance on the homelab.  Reachable from the pod.
 DEFAULT_OLLAMA_URL = os.getenv("OLLAMA_URL", "http://192.168.178.45:11434")
 
-#: A model that is known to work.  `deepseek-v4-flash:cloud`,
+#: The model to use.  Override with the ``OLLAMA_MODEL`` environment
-#: `deepseek-v3.2:cloud` and `qwen3.5:cloud` are all retired and fail at
+#: variable.
-#: call time, so they are not safe defaults.
+#:
-DEFAULT_MODEL = os.getenv("OLLAMA_MODEL", "glm-5.3:cloud")
+#: A model that does NOT reason is much better here.  The reasoning
+#: models emit their thinking out of the same token allowance as the
+#: answer, and several of them burned the whole budget on thinking and
+#: returned an EMPTY reply - which looks like a broken model rather than
+#: a budget problem.  With `think: false` the model still shows its
+#: working, but inline in the content, so the answer can be taken from
+#: the end of it.
+#:
+#: `deepseek-v4-flash:cloud`, `deepseek-v3.2:cloud` and `qwen3.5:cloud`
+#: are all retired and fail at call time while still appearing in the
+#: model list, so the default is one that was probed.
+DEFAULT_MODEL = os.getenv("OLLAMA_MODEL", "mistral-large-4:cloud")
 
 #: How long to wait for the model before giving up.  The advice is
 #: generated in the background, so this only bounds the background thread.
-DEFAULT_TIMEOUT = float(os.getenv("LLM_TIMEOUT", "45"))
+DEFAULT_TIMEOUT = float(os.getenv("LLM_TIMEOUT", "120"))
 
 #: The advice is advice, not instruction.  Keep it short - it has to be
 #: readable on the brewing page next to the brewer's own comment.
@@ -67,6 +79,16 @@ temperature?
 - Should they rack, add more hops, raise the temperature, or wait?
 
 Rules:
+- Your reply must have this exact shape:
+
+  ADVICE:
+  <the advice itself, two to five sentences of plain prose>
+
+  The line "ADVICE:" comes FIRST, on its own line, with nothing before
+  it.  Do not explain your reasoning and do not show your working.
+- Advise about the reading that is marked as the one to advise on.  It
+  may not be the most recent one - the brewer may be correcting an older
+  reading - and the advice must be about the reading they asked about.
 - If everything looks normal, say so briefly and say what to expect next.
 - If something looks wrong, say what it is and what to do about it.
 - Be specific to the numbers given. Do not give generic advice.
@@ -81,8 +103,13 @@ Write two to five sentences of plain prose.
 """
 
 
-def _reading_lines(readings: list[dict]) -> str:
+def _reading_lines(readings: list[dict], subject_id: str | None = None) -> str:
-    """The readings, as the compact list the model reads."""
+    """The readings, as the compact list the model reads.
+
+    The reading being advised about is marked, so the model does not have
+    to work out which one from the dates - the brewer may be correcting
+    an older reading.
+    """
     lines = []
     for r in readings:
         parts = [f"- {r.get('date')}: gravity {r.get('sg_decimal')} ({r.get('point_scale')})"]
@@ -90,12 +117,19 @@ def _reading_lines(readings: list[dict]) -> str:
             parts.append(f"brewer's comment: {r['comment']}")
         if r.get("final"):
             parts.append("(marked as the FINAL reading)")
+        if subject_id is not None and str(r.get("_id")) == str(subject_id):
+            parts.append("<<< THIS IS THE READING TO ADVISE ON")
         lines.append(" ".join(parts))
     return "\n".join(lines)
 
 
-def build_prompt(run: dict, reading: dict, history: list[dict]) -> str:
+def build_prompt(run: dict, reading: dict, history: list[dict],
-    """The user-message content for one observation."""
+                 corrected: bool = False) -> str:
+    """The user-message content for one observation.
+
+    *corrected* says the reading has just been edited, so the advice
+    should take that into account.
+    """
     name = run.get("canonical_name") or run.get("name") or "this brew"
     og = run.get("og_decimal")
     if og is None and run.get("og_points"):
@@ -113,10 +147,23 @@ def build_prompt(run: dict, reading: dict, history: list[dict]) -> str:
 
     parts.append("")
     parts.append("Readings so far (oldest first):")
-    parts.append(_reading_lines(history))
+    parts.append(_reading_lines(history, subject_id=reading.get("_id")))
 
     parts.append("")
-    parts.append("Give advice for the most recent reading.")
+    # The reading being advised about is NAMED, not just described as
+    # "the most recent one".  The brewer may be editing an older reading,
+    # and the advice is about THAT reading - so the prompt has to say
+    # which one, or the model advises about the newest instead.
+    parts.append(
+        f"Give advice about the reading dated {reading.get('date') or 'unknown'}"
+        f" (gravity {reading.get('sg_decimal')}),"
+        + (" which the brewer has just corrected." if corrected else "")
+    )
+    if corrected:
+        parts.append(
+            "It is the reading they have just corrected, so it may differ "
+            "from what it said before."
+        )
     return "\n".join(parts)
 
 
@@ -135,11 +182,14 @@ def call_ollama(prompt: str, *, url: str | None = None, model: str | None = None
         ],
         "temperature": 0.4,
         # The reasoning models emit their thinking first, and it comes out
-        # of the same budget.  At 500 tokens the reasoning used the whole
+        # of the SAME budget as the answer.  At 500 tokens the reasoning
-        # allowance and the reply came back empty - which looked like a
+        # used the whole allowance and the reply came back empty - which
-        # broken model rather than a budget problem.  2000 leaves room for
+        # looks like a broken model rather than a budget problem.
-        # the reasoning AND the advice.
+        #
-        "max_tokens": 2000,
+        # `think: false` does NOT help: the models still think, they just
+        # put the working in the content instead of a separate field.
+        # So the budget has to cover the reasoning AND the advice.
+        "max_tokens": 8000,
         "stream": False,
     }).encode("utf-8")
 
@@ -155,14 +205,109 @@ def call_ollama(prompt: str, *, url: str | None = None, model: str | None = None
     choices = body.get("choices") or []
     if not choices:
         raise RuntimeError("the model returned no choices")
-    content = (choices[0].get("message") or {}).get("content") or ""
+    message = choices[0].get("message") or {}
+
+    # Some models put their working in a separate field.  It is ignored -
+    # the advice is the content.
+    content = message.get("content") or ""
     content = content.strip()
     if not content:
         raise RuntimeError("the model returned an empty response")
-    return content[:MAX_ADVICE_CHARS]
+
+    # A reply cut off at the token ceiling is INCOMPLETE.  It may be
+    # advice that stops mid-sentence, so it is marked as truncated rather
+    # than stored as if it were the whole answer.
+    finish_reason = choices[0].get("finish_reason") or body.get("done_reason")
+    truncated = finish_reason == "length"
+
+    advice = _trim_advice(_strip_reasoning(content))
+
+    if truncated:
+        advice = _trim_advice(advice + " (the reply was cut off)")
+
+    return advice, truncated
+
+
+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 marker the model is asked to put at the start of its advice.
+#:
+#: Guessing where the advice begins by classifying prose is unreliable
+#: in both directions - it emptied legitimate advice, and it let the
+#: model's working through in full.  A marker the model writes itself is
+#: deterministic: everything before it is working, everything from it on
+#: is the advice.
+_ADVICE_MARKER = "ADVICE:"
+
+
+def _strip_reasoning(text: str) -> str:
+    """Take the advice out of a reply.
+
+    The model is asked to put ``ADVICE:`` on its own line at the start
+    of the advice, so the working before it can be dropped exactly, and
+    the advice kept exactly.
+
+    A reply with no marker is used as it stands.  That is the safe
+    direction: the model was asked for the marker, so a reply without
+    one is one where it wrote the advice straight away - and if the
+    working did leak in, the brewer sees a slightly rambling reply
+    rather than no advice at all.
+    """
+    index = _find_marker(text)
+    if index is None:
+        return text.strip()
+
+    advice = text[index:].strip()
+
+    # The model may wrap the marker in markdown or a code block.  Strip
+    # the leftover formatting so the advice reads as prose.
+    advice = re.sub(r"^\*\*|\*\*$", "", advice).strip()
+    if advice.startswith("```"):
+        advice = advice[3:]
+    if advice.endswith("```"):
+        advice = advice[:-3]
+    if advice.startswith("##"):
+        advice = advice.lstrip("#").strip()
+
+    return advice.strip() or ""
+
+
+def _find_marker(text: str) -> int | None:
+    """Where the advice starts, or ``None`` if there is no marker.
+
+    The marker is matched case-insensitively and with the markdown
+    decoration the model might put around it - ``**ADVICE:**``,
+    ``## ADVICE:`` - because the instruction asks for "ADVICE:" but the
+    model formats it its own way often enough to matter.
+    """
+    match = re.search(r"^[\s#>\*]*advice\s*:[\s\*>]*", text,
+                      re.IGNORECASE | re.MULTILINE)
+    if match:
+        return match.end()
+    return None
 
 
 def generate_advice(run: dict, reading: dict, history: list[dict], *,
+                     corrected: bool = False,
                      url: str | None = None, model: str | None = None,
                      timeout: float | None = None) -> tuple[bool, str]:
     """Generate the advice for one reading.
@@ -171,9 +316,20 @@ def generate_advice(run: dict, reading: dict, history: list[dict], *,
     short note explaining what went wrong, so the reading carries a
     visible reason instead of silently having no advice.
     """
-    prompt = build_prompt(run, reading, history)
+    prompt = build_prompt(run, reading, history, corrected=corrected)
     try:
-        return True, call_ollama(prompt, url=url, model=model, timeout=timeout)
+        advice, _truncated = call_ollama(prompt, url=url, model=model,
+                                         timeout=timeout)
+        if not advice:
+            # The reply had no advice in it - the model wrote only its
+            # working, or the marker was missing and nothing followed it.
+            # Storing an empty string with advice_ok=True would render
+            # as "Thinking..." for ever, so it is recorded as a failure
+            # with a note the brewer can see.
+            log.warning("the model returned no advice")
+            return False, ("Advice could not be generated - the model "
+                           "returned only its reasoning.")
+        return True, advice
     except urllib.error.HTTPError as exc:
         # HTTPError is a subclass of URLError, so it has to be caught
         # FIRST or it would be swallowed by the unreachable branch.
@@ -197,14 +353,15 @@ def generate_advice(run: dict, reading: dict, history: list[dict], *,
 
 
 def generate_advice_in_background(run: dict, reading: dict,
-                                   history: list[dict], writer) -> None:
+                                   history: list[dict], writer,
+                                   corrected: bool = False) -> None:
     """Generate the advice and hand it to *writer* when it is ready.
 
     *writer* is called as ``writer(reading_id, ok, text)``.  It runs in a
     background thread, so it must not touch the request context - the
     caller is responsible for giving it a way to reach Mongo.
     """
-    ok, text = generate_advice(run, reading, history)
+    ok, text = generate_advice(run, reading, history, corrected=corrected)
     try:
         writer(reading.get("_id"), ok, text)
     except Exception:  # noqa: BLE001 - never let the thread die noisily

src/flask/brewing.py

@@ -156,6 +156,10 @@ class Reading:
     # Mongo, so the brewer's own comment is never overwritten.
     advice: str = ""
     advice_ok: bool = True
+    # Marks this generation of the reading.  Every edit writes a fresh
+    # one, so an advice thread working on an earlier version of the
+    # reading can be told apart from the current one.
+    advice_token: str = ""
 
     @property
     def sg_decimal(self) -> float | None:
@@ -177,6 +181,7 @@ class Reading:
             _id=str(doc.get("_id") or ""),
             advice=(doc.get("advice") or "").strip(),
             advice_ok=bool(doc.get("advice_ok", True)),
+            advice_token=str(doc.get("advice_token") or ""),
         )
 
 

src/flask/mongo/repository.py

@@ -18,7 +18,9 @@ Two collections are used:
 from __future__ import annotations
 
 import logging
+import re
 import os
+import threading
 from typing import Any
 
 from pymongo import MongoClient
@@ -46,6 +48,33 @@ class BeerRepository:
         self.run_meta = self.db["beer_runs"]
         self._ensure_indexes()
 
+    def close(self) -> None:
+        """Close the underlying Mongo client and release it.
+
+        A ``MongoClient`` keeps a connection pool and spawns three
+        monitoring threads.  ``close()`` alone does NOT reap them - they
+        go when the client object is garbage collected - so the
+        reference is dropped as well.
+
+        The request-scoped repository is closed by the app's teardown, but
+        a caller that builds its own - the advice background thread, for
+        instance - has to close it itself or the threads and the pool
+        accumulate, three per reading.
+        """
+        client = self.client
+        try:
+            client.close()
+        except Exception:
+            pass
+        # Drop the reference so the monitor threads can be reaped.
+        self.client = None
+
+    def __enter__(self) -> "BeerRepository":
+        return self
+
+    def __exit__(self, *exc_info) -> None:
+        self.close()
+
     def _ensure_indexes(self) -> None:
         """Make sure the lookup keys are indexed.
 
@@ -180,8 +209,21 @@ class BeerRepository:
                 "date": _as_date(reading_date),
                 "final_reading": "True" if final_reading else "False",
                 "comment": str(comment),
-            }
+            },
+            # The advice was written for the reading that was there when
+            # it was generated.  If the reading has been edited, that
+            # advice no longer describes it, so it is cleared and a fresh
+            # one is generated.  (Advice is only ever cleared here - never
+            # the brewer's comment.)
+            "$unset": {"advice": "", "advice_ok": "", "advice_at": ""},
         }
+        # A fresh token marks this generation of the reading.  The advice
+        # thread carries the token it was given, so a thread that is
+        # still working on an EARLIER version of the reading finds a
+        # token that no longer matches and its write is refused.  The
+        # gravity alone is not enough - a comment-only correction leaves
+        # the gravity unchanged.
+        update["$set"]["advice_token"] = _new_token()
         result = self.readings.update_one(query, update, upsert=True)
         if result.upserted_id is not None:
             return str(result.upserted_id)
@@ -189,25 +231,105 @@ class BeerRepository:
         return str(existing.get("_id")) if existing else ""
 
     def set_reading_advice(self, reading_id: str, advice: str,
-                            ok: bool = True) -> int:
+                            ok: bool = True,
+                            expected_sg: int | None = None,
+                            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
         never overwritten.  ``ok`` says whether the advice was actually
         generated or is a note explaining that it could not be.
+
+        The write is CONDITIONAL, so a slow advice thread cannot
+        overwrite a correction:
+
+        - ``expected_token`` is the editing token of the reading at the
+          moment the advice was generated.  Every edit writes a fresh
+          token, so a late thread's token no longer matches.
+        - ``expected_sg`` is the gravity the advice was generated for,
+          as a second check.
+
+        This matters because the advice is generated in the background: if
+        the brewer corrects a reading while the advice for the old value
+        is still being generated, the slow thread would land afterwards
+        and overwrite the fresh advice with advice about a reading that
+        no longer exists.
+
+        The token is needed as well as the gravity, because a
+        comment-only correction leaves the gravity unchanged - the token
+        is what tells the two generations apart.
         """
         doc_id = _as_doc_id(reading_id)
-        if doc_id is None:
+        if not doc_id:
             return 0
+
+        query: dict = {"_id": {"$in": doc_id}}
+        if expected_token:
+            # 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_token == "":
+            # The reading has NEVER been edited, so it has no token at
+            # all.  Match on the ABSENCE of one, which still tells the
+            # generations apart: an edit writes a token, so a stale
+            # thread's "" stops matching the moment the reading is
+            # corrected.
+            #
+            # Falling back to the gravity here would re-open the hole
+            # this guard exists to close - a comment-only correction
+            # leaves the gravity unchanged.
+            # `$in` with null also matches a MISSING field, so this
+            # covers a reading with no token AND one whose token field
+            # is present but empty.
+            query["advice_token"] = {"$in": [None, ""]}
+        elif expected_sg is not None:
+            # 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(
-            {"_id": doc_id},
+            query,
             {"$set": {
                 "advice": str(advice),
                 "advice_ok": bool(ok),
                 "advice_at": _now_iso(),
             }},
         )
-        return result.modified_count
+        # `matched_count`, not `modified_count`.
+        #
+        # `modified_count` is 0 when the document matched but the text
+        # was already identical - which is a successful write, not a
+        # refused one.  The caller logs "the advice was not stored" when
+        # this is 0, so it has to mean "the guard refused it".
+        return result.matched_count
 
     def set_final_reading(self, beer_run_id: str, reading_date: str,
                           final: bool) -> int:
@@ -335,12 +457,59 @@ def _as_id(value: object) -> str:
     return ""
 
 
+#: A process-wide repository, for the code that runs outside a request.
+#:
+#: A ``MongoClient`` spawns three monitor threads and pymongo does not
+#: reap them on ``close()`` - they go only when the client is garbage
+#: collected.  So building a client per advice write accumulated three
+#: threads and a connection pool for every reading recorded.  One shared
+#: client for the life of the process is what the request-scoped
+#: repository does too.
+_shared_repository: "BeerRepository | None" = None
+
+#: Guards the check-then-set above.
+#:
+#: Without it, concurrent advice threads each saw ``None`` and each
+#: built their own repository - so the fix reintroduced the very leak it
+#: was meant to remove, just bounded by a race instead of by the number
+#: of readings.  Eight threads on a cold process built eight clients and
+#: twenty-five threads.
+_shared_repository_lock = threading.Lock()
+
+
+def get_shared_repository() -> "BeerRepository":
+    """The process-wide repository, built on first use.
+
+    Thread-safe: the first caller builds it, the rest wait and get the
+    same one.
+    """
+    global _shared_repository
+
+    if _shared_repository is None:
+        with _shared_repository_lock:
+            # Checked again inside the lock - another thread may have
+            # built it while this one was waiting.
+            if _shared_repository is None:
+                _shared_repository = BeerRepository()
+    return _shared_repository
+
+
 def _as_doc_id(value: object):
     """Coerce a reading document id to something Mongo will match.
 
-    The ``_id`` of a reading is an ``ObjectId``, not a run id, so the
+    The ``_id`` of a reading is not a run id, so the ``_as_id`` coercion
-    ``_as_id`` coercion does not apply - it would turn the ObjectId into
+    does not apply - it would turn the id into an empty string and the
-    an empty string and the update would silently match nothing.
+    update would silently match nothing.
+
+    Two shapes exist in this database:
+
+    - the ORIGINAL app's ids are 24 random CHARACTERS stored as a string
+      (``l78ZidBJ3QaYn98tvB88x4uM``), and
+    - ids written by Mongo itself are ``ObjectId`` objects.
+
+    So the id is returned in BOTH forms and the query matches either.
+    Accepting only the ObjectId form would mean the advice was never
+    stored for any of the existing readings.
 
     Only an ObjectId or a plain string is accepted, so a dict can never
     be injected as a query operator.
@@ -348,13 +517,20 @@ def _as_doc_id(value: object):
     from bson import ObjectId
 
     if isinstance(value, ObjectId):
-        return value
+        return [value]
-    if isinstance(value, str):
+    if isinstance(value, str) and value:
         try:
-            return ObjectId(value)
+            # A 24-character hex string may or may not be the stored
+            # ObjectId or the stored string, so try both.
+            return [ObjectId(value), value]
         except Exception:
-            return None
+            # Not hex - it can only be the original app's string id.
-    return None
+            return [value]
+    # Anything else - a dict, a list, an int, None - is not a reading
+    # id.  An EMPTY LIST is returned rather than None: it is a valid
+    # `$in` argument that matches nothing, so the helper cannot be
+    # misused into a query error or into matching every document.
+    return []
 
 
 def _as_date(value: object) -> str:
@@ -379,6 +555,13 @@ def hash_password(password: str) -> str:
     return generate_password_hash(password)
 
 
+def _new_token() -> str:
+    """A fresh token, to mark one generation of a reading from the next."""
+    import secrets
+
+    return secrets.token_hex(8)
+
+
 def _now_iso() -> str:
     from datetime import datetime, timezone
 

src/flask/static/css/app.css

@@ -451,7 +451,10 @@ input[type="date"]::-webkit-calendar-picker-indicator:hover { opacity: 1; }
 
 /* ---------------------------------------------------------------- tables */
 
-.table-wrap { overflow-x: auto; }
+.table-wrap {
+    overflow-x: auto;
+    -webkit-overflow-scrolling: touch;
+}
 
 table.data {
     width: 100%;
@@ -480,6 +483,148 @@ table.data tbody tr:hover { background: var(--bg-raised-2); }
 table.data .num { font-family: var(--mono); text-align: right; white-space: nowrap; }
 table.data .comment { color: var(--text-muted); font-size: .84rem; }
 
+/* A heading that is too long for a phone column.  The long form shows
+   on a desktop and the short form on a phone - see the media query.
+
+   The rule is on `.short` alone, not on `table.data .short`, so it
+   applies everywhere the pattern is used - in a table heading, in the
+   advice button, and on the Active Beers page.  When only the table
+   was covered, the advice button could show both forms at once and
+   read "Read the adviceAdvice". */
+.short { display: none; }
+
+/* On a narrow screen the header wraps onto two rows: the brand and the
+   sign-out on the first, the nav tabs on the second where they get the
+   full width.
+
+   They used to sit on one row with the brand and the sign-out, and the
+   last tab was cut off mid-word. */
+@media (max-width: 720px) {
+    .topbar__inner {
+        flex-wrap: wrap;
+        row-gap: .5rem;
+    }
+
+    .brand {
+        order: 1;
+    }
+
+    .topbar__logout {
+        order: 2;
+        margin-left: auto;
+    }
+
+    .nav {
+        order: 3;
+        flex: 1 0 100%;
+        overflow-x: auto;
+        scrollbar-width: none;
+        -webkit-overflow-scrolling: touch;
+        /* A fade at the right edge so a clipped tab reads as
+           scrollable rather than broken. */
+        -webkit-mask-image: linear-gradient(to right, #000 calc(100% - 1.5rem), transparent 100%);
+        mask-image: linear-gradient(to right, #000 calc(100% - 1.5rem), transparent 100%);
+    }
+
+    .nav::-webkit-scrollbar {
+        display: none;
+    }
+
+    .nav__link {
+        flex-shrink: 0;
+    }
+
+    /* Tighter tabs, so all four fit on a phone without scrolling.
+       They still scroll if they have to. */
+    .nav__link {
+        padding: .35rem .55rem;
+        font-size: .8rem;
+        letter-spacing: .01em;
+    }
+
+    /* The tables stay tables on a phone.
+
+       They scroll sideways inside their card, which is what a table
+       does.  They used to be squeezed to fit instead, and with seven
+       to nine columns on a 390px screen that meant every heading and
+       every value was truncated to a few characters - a gravity of
+       "1.0..." and a date of "202..." are worse than useless.
+
+       So the columns keep their natural width, the table scrolls, and
+       the advice is a small button so the column that matters is
+       narrow. */
+    table.data {
+        table-layout: auto;
+        width: 100%;
+    }
+
+    table.data th,
+    table.data td {
+        padding-left: .5rem;
+        padding-right: .5rem;
+    }
+
+    /* A name and a comment are words, so they get a sensible minimum
+       width and wrap on a word boundary - not one word per line. */
+    table.data td.col-name,
+    table.data td.col-comment {
+        min-width: 7rem;
+        max-width: 12rem;
+        white-space: normal;
+        word-break: normal;
+        overflow-wrap: break-word;
+    }
+
+    /* The long form gives way to the short form on a phone, because a
+       heading that is cut off is worse than a heading that is brief.
+
+       The rule is on the span itself rather than on the column, so it
+       applies everywhere the pattern is used - in a table heading, in
+       the advice button, and on the Active Beers page. */
+    .long {
+        display: none;
+    }
+
+    .short {
+        display: inline;
+    }
+
+    /* The advice trigger is small on a phone - it is a button, and
+       the advice itself is in the pop-out. */
+    table.data .advice-trigger {
+        font-size: .68rem;
+        padding: .25rem .4rem;
+        gap: .2rem;
+    }
+
+    /* The action column holds a button. */
+    table.data .col-action {
+        white-space: nowrap;
+    }
+
+    table.data .col-action .btn {
+        padding-left: .5rem;
+        padding-right: .5rem;
+        font-size: .72rem;
+    }
+
+    /* The secondary columns go away on a phone - the details are still
+       on the run's own page. */
+    table.data .col-drop,
+    table.data .col-legacy,
+    table.data .col-days {
+        display: none;
+    }
+
+    /* The scroll area keeps a visible scrollbar on a phone, so it is
+       obvious there is more of the table to the right. */
+    .table-wrap {
+        scrollbar-width: thin;
+        -webkit-overflow-scrolling: touch;
+    }
+
+}
+
 /* ----------------------------------------------------------------- chips */
 
 .chip {
@@ -595,8 +740,18 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; }
 .brew-card__name { font-weight: 650; font-size: 1.02rem; margin: 0; }
 .brew-card__meta { color: var(--text-muted); font-size: .82rem; font-family: var(--mono); }
 .brew-card__stats { display: grid; grid-template-columns: repeat(3, 1fr); gap: .6rem; }
+
+/* The three stats sit side by side, so their captions line up.  One
+   caption ("needs 2nd reading") is two lines and the others are one,
+   which left the baselines ragged. */
+.brew-card__stat {
+    display: flex;
+    flex-direction: column;
+}
+
 .brew-card__stat .stat__label { font-size: .68rem; }
 .brew-card__stat .stat__value { font-size: 1.3rem; }
+.brew-card__stat .stat__meta { margin-top: auto; }
 .brew-card__spark { height: 46px; }
 
 /* The "start a new beer" tile — fills the last grid slot and gives the
@@ -716,6 +871,12 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; }
     color: var(--amber-text);
 }
 
+/* The advice on the active-beers card and the run detail page - the
+   full text, not the clamped table cell. */
+.advice--reading {
+    margin-top: .5rem;
+}
+
 /* Advice that could not be generated - it says why. */
 .advice--failed {
     border-left-color: var(--danger);
@@ -727,27 +888,156 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; }
     color: var(--danger);
 }
 
+/* The advice in the table is a compact trigger that pops the full text
+   out on hover, focus or tap.
+
+   It used to be the text itself, clamped to four lines.  The advice runs
+   to several hundred characters, and a cell that wide pushes the table
+   past the width of a phone, so the column ended up clipped off the
+   right edge and unreadable without sideways scrolling. */
 .advice-cell {
-    max-width: 32rem;
+    position: relative;
+    width: 1%;
+    white-space: nowrap;
 }
 
-/* In the table the advice is clamped to a few lines - the full text is
+.advice-trigger {
-   on the run detail page, where there is room to read it. */
+    display: inline-flex;
-.advice-cell .advice {
+    align-items: center;
-    margin-top: 0;
+    gap: .35rem;
+    padding: .25rem .55rem;
+    border: 1px solid var(--amber);
+    border-radius: 999px;
+    background: var(--amber-dim);
+    color: var(--amber-text);
+    font: inherit;
     font-size: .75rem;
-    display: -webkit-box;
+    line-height: 1.4;
-    -webkit-line-clamp: 4;
+    cursor: pointer;
-    line-clamp: 4;
+    white-space: nowrap;
-    -webkit-box-orient: vertical;
-    overflow: hidden;
 }
 
-/* The clamped text opens up on hover, so the whole thing is readable
+.advice-trigger:hover,
-   without leaving the page. */
+.advice-trigger:focus-visible {
-.advice-cell:hover .advice,
+    background: var(--amber);
-.advice-cell:focus-within .advice {
+    color: var(--bg);
-    -webkit-line-clamp: unset;
+}
-    line-clamp: unset;
+
-    overflow: visible;
+.advice-trigger--failed {
+    border-color: var(--danger);
+    background: var(--danger-dim);
+    color: var(--danger);
+}
+
+.advice-trigger__icon {
+    font-size: .85em;
+}
+
+/* The pop-out.
+
+   It is `position: fixed` and anchored to the trigger by the script,
+   not absolutely positioned inside the cell.  The table is a scroll
+   container, and an absolutely positioned panel inside it was clipped
+   by the container's own overflow - the advice was sliced in half by
+   the card's edge and the bottom border was cut off.
+
+   Fixed positioning takes it out of the table entirely, so it floats
+   above the page and is never cut by a scroll container. */
+.advice-pop {
+    position: fixed;
+    z-index: 200;
+    display: none;
+    width: min(30rem, 92vw);
+    max-height: 70vh;
+    overflow-y: auto;
+    padding: .8rem .9rem;
+    border: 1px solid var(--amber);
+    border-radius: 8px;
+    background: var(--bg-raised);
+    box-shadow: 0 12px 32px rgba(0, 0, 0, .55);
+    text-align: left;
+    white-space: normal;
+}
+
+.advice-pop__head {
+    display: flex;
+    align-items: center;
+    justify-content: space-between;
+    gap: .5rem;
+    margin-bottom: .4rem;
+}
+
+.advice-pop__head .advice__label {
+    margin-bottom: 0;
+}
+
+.advice-pop__close {
+    border: 0;
+    background: none;
+    color: var(--text-muted);
+    font-size: 1.1rem;
+    line-height: 1;
+    cursor: pointer;
+    /* A 44px touch target, which is the smallest that is comfortable
+       on a phone.  The negative margin pulls the panel's padding back
+       in so the button does not push the layout around, but the
+       padding is still what makes it easy to hit. */
+    padding: .9rem;
+    margin: -.9rem -.9rem -.9rem 0;
+}
+
+.advice-pop__close:hover {
+    color: var(--text);
+}
+
+.advice-pop__text {
+    margin: 0;
+    font-size: .8125rem;
+    line-height: 1.55;
+    color: var(--text);
+}
+
+/* The script opens and closes the panel, and places it.  The CSS only
+   decides whether it is shown. */
+/* The panel is opened by the script, which removes the `hidden`
+   attribute and adds `is-open`.
+
+   The `hidden` attribute is what makes this fail-safe: if the
+   stylesheet is stale and does not know about `.advice-pop` at all,
+   the panel is STILL hidden, so the advice cannot appear expanded
+   inside the table cell. */
+.advice-pop.is-open {
+    display: block;
+}
+
+.advice-pop[hidden] {
+    display: none;
+}
+
+/* On a phone there is no hover, so the trigger is a button the script
+   toggles, and the panel is pinned to the middle of the screen so it is
+   readable whatever the table's width. */
+@media (max-width: 720px) {
+    .advice-cell {
+        width: auto;
+    }
+
+    /* A dimmed backdrop, so the panel reads as sitting over the page
+       rather than floating in it.
+
+       A large spread shadow is used rather than a pseudo element, which
+       would paint behind the panel's own background. */
+    .advice-pop {
+        box-shadow: 0 0 0 100vmax rgba(0, 0, 0, .55),
+                    0 12px 32px rgba(0, 0, 0, .55);
+    }
+
+    /* On a phone the panel is centred on the screen rather than
+       anchored to the trigger - there is not the room to float it
+       beside the cell, and the trigger may be near the edge. */
+    .advice-pop {
+        top: 50% !important;
+        left: 50% !important;
+        transform: translate(-50%, -50%);
+    }
 }

src/flask/static/js/app.js

@@ -56,11 +56,171 @@
         }
     }
 
+    /**
+     * The brewing advice pop-out.
+     *
+     * On a desktop the panel opens on hover, which the CSS does.  A
+     * phone has no hover, so the trigger is a button that toggles it -
+     * and it is a real button, so it works with a keyboard and with a
+     * screen reader.
+     */
+    /**
+     * The brewing advice pop-out.
+     *
+     * The advice runs to several hundred characters.  In a table cell it
+     * pushed the table past the width of a phone, so it is a compact
+     * trigger instead and the text is shown in a floating panel.
+     *
+     * The panel is position:fixed so it escapes the table's scroll
+     * container - absolutely positioned inside the cell it was clipped
+     * by the container and sliced in half by the card's edge.
+     *
+     * It opens on hover on a desktop, and on tap on a phone, which has
+     * no hover.  The trigger is a real button, so it also works with a
+     * keyboard.
+     */
+    function wireAdvicePopouts() {
+        var triggers = document.querySelectorAll(".advice-trigger");
+        for (var i = 0; i < triggers.length; i++) {
+            (function (trigger) {
+                var panel = trigger.parentNode.querySelector(".advice-pop");
+                if (!panel) { return; }
+
+                var closeTimer = null;
+
+                function isPhone() {
+                    return window.matchMedia("(max-width: 720px)").matches;
+                }
+
+                function place() {
+                    if (isPhone()) {
+                        /* The CSS centres it on a phone - clear any
+                           inline position left over from a desktop
+                           resize. */
+                        panel.style.top = "";
+                        panel.style.left = "";
+                        return;
+                    }
+
+                    var box = trigger.getBoundingClientRect();
+                    var gap = 10;
+                    var height = panel.offsetHeight;
+                    var width = panel.offsetWidth;
+                    var spaceBelow = window.innerHeight - box.bottom - gap;
+                    var spaceAbove = box.top - gap;
+
+                    /* Prefer below; flip above when there is not enough
+                       room and there is more room above. */
+                    var top;
+                    if (height <= spaceBelow || spaceBelow >= spaceAbove) {
+                        top = box.bottom + gap;
+                    } else {
+                        top = Math.max(gap, box.top - height - gap);
+                    }
+
+                    /* Keep it on screen horizontally. */
+                    var left = Math.min(box.left, window.innerWidth - width - gap);
+                    left = Math.max(gap, left);
+
+                    panel.style.top = Math.round(top) + "px";
+                    panel.style.left = Math.round(left) + "px";
+                }
+
+                function open() {
+                    if (closeTimer) {
+                        clearTimeout(closeTimer);
+                        closeTimer = null;
+                    }
+                    /* The `hidden` attribute is removed as well as the
+                       class added, so the panel is shown even if the
+                       stylesheet is an older one that only knows about
+                       the class. */
+                    panel.removeAttribute("hidden");
+                    panel.classList.add("is-open");
+                    trigger.setAttribute("aria-expanded", "true");
+                    place();
+                }
+
+                function close() {
+                    panel.classList.remove("is-open");
+                    /* Put the attribute back, so a stale stylesheet
+                       cannot leave the advice expanded in the cell. */
+                    panel.setAttribute("hidden", "");
+                    trigger.setAttribute("aria-expanded", "false");
+                }
+
+                function closeSoon() {
+                    /* A short delay, so the mouse can travel from the
+                       trigger into the panel without it closing. */
+                    if (closeTimer) { clearTimeout(closeTimer); }
+                    closeTimer = setTimeout(function () {
+                        closeTimer = null;
+                        if (!panel.matches(":hover") && !trigger.matches(":hover")) {
+                            close();
+                        }
+                    }, 180);
+                }
+
+                trigger.addEventListener("click", function (event) {
+                    event.preventDefault();
+                    event.stopPropagation();
+                    if (panel.classList.contains("is-open")) {
+                        close();
+                    } else {
+                        open();
+                    }
+                });
+
+                trigger.addEventListener("mouseenter", function () {
+                    if (!isPhone()) { open(); }
+                });
+                trigger.addEventListener("mouseleave", closeSoon);
+                panel.addEventListener("mouseenter", function () {
+                    if (closeTimer) {
+                        clearTimeout(closeTimer);
+                        closeTimer = null;
+                    }
+                });
+                panel.addEventListener("mouseleave", closeSoon);
+
+                var closeButton = panel.querySelector(".advice-pop__close");
+                if (closeButton) {
+                    closeButton.addEventListener("click", function (event) {
+                        event.preventDefault();
+                        event.stopPropagation();
+                        close();
+                        trigger.focus();
+                    });
+                }
+
+                document.addEventListener("keydown", function (event) {
+                    if (event.key === "Escape" && panel.classList.contains("is-open")) {
+                        close();
+                        trigger.focus();
+                    }
+                });
+                document.addEventListener("click", function (event) {
+                    if (!panel.contains(event.target) && event.target !== trigger) {
+                        close();
+                    }
+                });
+
+                window.addEventListener("resize", function () {
+                    if (panel.classList.contains("is-open")) { place(); }
+                });
+                window.addEventListener("scroll", function () {
+                    if (panel.classList.contains("is-open")) { close(); }
+                }, true);
+            })(triggers[i]);
+        }
+    }
+
     function init() {
         focusFirstEmpty();
         defaultDateFields();
         wireToggles();
         wireConfirms();
+        wireAdvicePopouts();
     }
 
     if (document.readyState === "loading") {

src/flask/static/js/charts.js

@@ -184,8 +184,16 @@
                 // Only finished brews have a measured ABV.  An unfinished run
                 // has a projection, not a measurement, so it is not plotted
                 // here.
+                //
+                // A suspect run is left out too.  Its ABV comes from an
+                // impossible reading (run 12 reads 18.64%), and plotting it
+                // stretched the y axis so that every real bar was squashed
+                // into the bottom of the plot - while the "Strongest" tile
+                // on the same page correctly showed 8.53%.  The chart and
+                // the tile contradicted each other.
                 var completed = runs.filter(function (r) {
-                    return r.abv !== null && r.abv !== undefined;
+                    return !r.suspect &&
+                        r.abv !== null && r.abv !== undefined;
                 });
                 if (!completed.length) { return; }
 
@@ -305,8 +313,14 @@
                 // A run with an impossible reading has no attenuation, and
                 // is flagged as suspect.  Both are excluded, so one bad
                 // reading cannot become a bar in the chart.
+                //
+                // Active runs are excluded as well.  Their attenuation is
+                // measured against their CURRENT gravity, so a brew that
+                // has just started shows a 0% bar - which is not a result,
+                // it is the absence of one.  The chart is about finished
+                // brews, and it sits above the table of finished brews.
                 var done = runs.filter(function (r) {
-                    return !r.suspect &&
+                    return !r.suspect && !r.active &&
                         r.attenuation !== null && r.attenuation !== undefined;
                 });
                 if (!done.length) { return; }

src/flask/templates/active.html

@@ -44,7 +44,7 @@
                 <div class="brew-card__meta">
                     Run {{ run.beer_run_id }}
                     {% if run.first_date %} &middot; since {{ run.first_date }}{% endif %}
-                    {% if run.reading_count == 1 %} &middot; 1 reading{% else %} &middot; {{ run.reading_count }} readings{% endif %}
+                    {% if run.reading_count == 1 %} &middot; 1&nbsp;reading{% else %} &middot; {{ run.reading_count }}&nbsp;readings{% endif %}
                 </div>
             </div>
             {% if run.tap_number is not none %}

src/flask/templates/analysis.html

@@ -99,43 +99,43 @@
             <table class="data">
                 <thead>
                     <tr>
-                        <th>Run</th>
+                        <th class="col-run">Run</th>
-                        <th>Name</th>
+                        <th class="col-name">Name</th>
-                        <th class="num">OG</th>
+                        <th class="num col-gravity">OG</th>
-                        <th class="num">FG</th>
+                        <th class="num col-gravity">FG</th>
-                        <th class="num">ABV</th>
+                        <th class="num col-abv">ABV</th>
-                        <th class="num">Attenuation</th>
+                        <th class="num col-abv" title="Attenuation"><span class="long">Attenuation</span><span class="short">Att.</span></th>
-                        <th class="num">Days</th>
+                        <th class="num col-abv col-days">Days</th>
-                        <th class="num">To 1.010</th>
+                        <th class="num col-abv" title="Days to reach 1.010"><span class="long">To 1.010</span><span class="short">1.010</span></th>
-                        <th>Finished</th>
+                        <th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
                     </tr>
                 </thead>
                 <tbody>
                 {% for row in rows %}
                     <tr{% if row.suspect %} class="is-suspect"{% endif %}>
-                        <td class="mono">{{ row.beer_run_id }}</td>
+                        <td class="col-run mono">{{ row.beer_run_id }}</td>
-                        <td>{{ row.name }}{% if row.suspect %} <span class="chip chip--warn">not counted</span>{% endif %}</td>
+                        <td class="col-name">{{ row.name }}{% if row.suspect %} <span class="chip chip--warn">not counted</span>{% endif %}</td>
-                        <td class="num">
+                        <td class="col-gravity num">
                             {% if row.og_points is not none %}
                                 {{ '%.3f' % (row.og_points / 1000) }}
                             {% else %}—{% endif %}
                         </td>
-                        <td class="num">
+                        <td class="col-gravity num">
                             {% if row.fg_points is not none %}
                                 {{ '%.3f' % (row.fg_points / 1000) }}
                             {% else %}—{% endif %}
                         </td>
-                        <td class="num">
+                        <td class="col-abv num">
                             {% if row.abv is not none %}{{ '%.2f' % row.abv }}%{% else %}—{% endif %}
                         </td>
-                        <td class="num">
+                        <td class="col-abv num">
                             {% if row.attenuation is not none %}
                                 <strong>{{ row.attenuation }}%</strong>
                             {% else %}—{% endif %}
                         </td>
-                        <td class="num">{{ row.days if row.days is not none else '—' }}</td>
+                        <td class="col-abv col-days num">{{ row.days if row.days is not none else '—' }}</td>
-                        <td class="num">{{ row.days_to_1010 if row.days_to_1010 is not none else '—' }}</td>
+                        <td class="col-abv num">{{ row.days_to_1010 if row.days_to_1010 is not none else '—' }}</td>
-                        <td class="mono nowrap">{{ row.final_date or '—' }}</td>
+                        <td class="col-finished mono nowrap">{{ row.final_date or '—' }}</td>
                     </tr>
                 {% endfor %}
                 </tbody>
@@ -155,21 +155,21 @@
             <table class="data">
                 <thead>
                     <tr>
-                        <th>Run</th>
+                        <th class="col-run">Run</th>
-                        <th class="num">OG</th>
+                        <th class="num col-gravity">OG</th>
-                        <th class="num">FG</th>
+                        <th class="num col-gravity">FG</th>
-                        <th>Problem</th>
+                        <th class="col-comment">Problem</th>
                     </tr>
                 </thead>
                 <tbody>
                 {% for row in suspect %}
                     <tr>
-                        <td class="mono">
+                        <td class="col-run mono">
                             <a href="{{ url_for('brews.run_detail', run_id=row.beer_run_id) }}">{{ row.beer_run_id }}</a>
                         </td>
-                        <td class="num">{{ '%.3f' % (row.og_points / 1000) }}</td>
+                        <td class="col-gravity num">{{ '%.3f' % (row.og_points / 1000) }}</td>
-                        <td class="num"><span class="chip chip--warn">{{ '%.3f' % (row.fg_points / 1000) }}</span></td>
+                        <td class="col-gravity num"><span class="chip chip--warn">{{ '%.3f' % (row.fg_points / 1000) }}</span></td>
-                        <td class="comment">{{ row.problem }}</td>
+                        <td class="col-comment comment">{{ row.problem }}</td>
                     </tr>
                 {% endfor %}
                 </tbody>
@@ -186,7 +186,7 @@
     </div>
     <div class="card__body">
         <p>
-            Across {% if stats.suspect_count %}{{ stats.trusted_count }} of {{ stats.count }}{% else %}{{ stats.count }}{% endif %} finished brews the attenuation averages
+            Across {% if stats.suspect_count %}{{ stats.counted_attenuation }} of {{ stats.count }}{% else %}{{ stats.count }}{% endif %} finished brews the attenuation averages
             <strong>{{ stats.consistency.mean }}%</strong>, with a standard
             deviation of <strong>{{ stats.consistency.stdev }} percentage
             points</strong> and a total range of {{ stats.consistency.spread }}

src/flask/templates/brews.html

@@ -78,48 +78,67 @@
                     <table class="data">
                         <thead>
                             <tr>
-                                <th>Run</th>
+                                <th class="col-run">Run</th>
-                                <th>Name</th>
+                                <th class="col-name">Name</th>
-                                <th class="num">Gravity</th>
+                                <th class="num col-gravity">Gravity</th>
-                                <th class="num">Est. ABV</th>
+                                <th class="num col-abv" title="Estimated alcohol by volume"><span class="long">Est. ABV</span><span class="short">ABV</span></th>
-                                <th>Comment</th>
+                                <th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
-                                <th>Brewing advice</th>
+                                <th class="col-advice" title="Brewing advice"><span class="long">Brewing advice</span><span class="short">Advice</span></th>
-                                <th></th>
+                                <th class="col-action"></th>
                             </tr>
                         </thead>
                         <tbody>
                         {% for run in active_runs %}
                             <tr>
-                                <td class="mono">{{ run.beer_run_id }}</td>
+                                <td class="mono col-run">{{ run.beer_run_id }}</td>
-                                <td>{{ run.canonical_name }}</td>
+                                <td class="col-name">{{ run.canonical_name }}</td>
-                                <td class="num">
+                                <td class="num col-gravity">
                                     {% if run.min_sg is not none %}
                                         <span class="mono strong">{{ run.min_sg_decimal }}</span>
-                                        <div class="faint tiny">{{ run.min_sg_point_scale }} pts</div>
+                                        <div class="faint tiny point-scale">{{ run.min_sg_point_scale }} points</div>
                                     {% else %}—{% endif %}
                                 </td>
-                                <td class="num">
+                                <td class="num col-abv">
                                     {% if run.projected_abv is not none %}
                                         {{ '%.2f' % run.projected_abv }}%
                                     {% else %}
                                         <span class="faint" title="Record another reading, or set the original gravity, to get an estimate">—</span>
                                     {% endif %}
                                 </td>
-                                <td class="comment">
+                                <td class="comment col-comment">
                                     {% if run.comments %}
                                         {{ run.comments[-1] }}
                                     {% else %}
                                         <span class="faint">—</span>
                                     {% endif %}
                                 </td>
-                                <td class="comment advice-cell">
+                                <td class="advice-cell col-advice">
                                     {% if run.advice %}
-                                        <span class="advice{% if not run.advice_ok %} advice--failed{% endif %}">{{ run.advice }}</span>
+                                        <button type="button"
+                                                class="advice-trigger{% if not run.advice_ok %} advice-trigger--failed{% endif %}"
+                                                aria-expanded="false">
+                                            <span class="advice-trigger__icon" aria-hidden="true">&#9998;</span>
+                                            <span class="advice-trigger__label">
+                                                {% if run.advice_ok %}
+                                                    <span class="long">Read the advice</span><span class="short">Advice</span>
+                                                {% else %}
+                                                    <span class="long">Advice failed</span><span class="short">Failed</span>
+                                                {% endif %}
+                                            </span>
+                                        </button>
+                                        <div class="advice-pop" role="tooltip" hidden>
+                                            <div class="advice-pop__head">
+                                                <span class="advice__label">Brewing advice</span>
+                                                <button type="button" class="advice-pop__close"
+                                                        aria-label="Close">&times;</button>
+                                            </div>
+                                            <p class="advice-pop__text">{{ run.advice }}</p>
+                                        </div>
                                     {% else %}
                                         <span class="faint">Thinking…</span>
                                     {% endif %}
                                 </td>
-                                <td class="right">
+                                <td class="right col-action">
                                     <a class="btn btn--ghost btn--sm"
                                        href="{{ url_for('brews.run_detail', run_id=run.beer_run_id) }}">Open</a>
                                 </td>
@@ -148,41 +167,41 @@
                     <table class="data">
                         <thead>
                             <tr>
-                                <th>Run</th>
+                                <th class="col-run">Run</th>
-                                <th>Name</th>
+                                <th class="col-name">Name</th>
-                                <th class="num">OG</th>
+                                <th class="num col-gravity">OG</th>
-                                <th class="num">FG</th>
+                                <th class="num col-gravity">FG</th>
-                                <th class="num">ABV</th>
+                                <th class="num col-abv">ABV</th>
-                                <th>Finished</th>
+                                <th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
-                                <th>Comment</th>
+                                <th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
-                                <th></th>
+                                <th class="col-action"></th>
                             </tr>
                         </thead>
                         <tbody>
                         {% for run in completed_runs %}
                             <tr>
-                                <td class="mono">{{ run.beer_run_id }}</td>
+                                <td class="mono col-run">{{ run.beer_run_id }}</td>
-                                <td>{{ run.canonical_name }}</td>
+                                <td class="col-name">{{ run.canonical_name }}</td>
-                                <td class="num">
+                                <td class="num col-gravity">
                                     {% if run.og_points is not none %}{{ run.og_points }}{% else %}—{% endif %}
                                 </td>
-                                <td class="num">
+                                <td class="num col-gravity">
                                     {% if run.fg_points is not none %}{{ run.fg_points }}{% else %}—{% endif %}
                                 </td>
-                                <td class="num">
+                                <td class="num col-abv">
                                     {% if run.abv is not none %}
                                         <strong>{{ '%.2f' % run.abv }}%</strong>
                                     {% else %}—{% endif %}
                                 </td>
-                                <td class="mono nowrap">{{ run.final_date or '—' }}</td>
+                                <td class="mono nowrap col-finished">{{ run.final_date or '—' }}</td>
-                                <td class="comment">
+                                <td class="comment col-comment">
                                     {% if run.comments %}
                                         {{ run.comments[-1] }}
                                     {% else %}
                                         <span class="faint">—</span>
                                     {% endif %}
                                 </td>
-                                <td class="right">
+                                <td class="right col-action">
                                     <a class="btn btn--ghost btn--sm"
                                        href="{{ url_for('brews.run_detail', run_id=run.beer_run_id) }}">Open</a>
                                 </td>

src/flask/templates/history.html

@@ -23,7 +23,13 @@
     <div class="stat">
         <div class="stat__label">Average ABV</div>
         <div class="stat__value">{{ '%.2f' % stats.avg_abv }}%</div>
-        <div class="stat__meta">across finished brews</div>
+        <div class="stat__meta">
+            {% if stats.abv_counted < stats.abv_total %}
+                across {{ stats.abv_counted }} of {{ stats.abv_total }} finished brews
+            {% else %}
+                across finished brews
+            {% endif %}
+        </div>
     </div>
     <div class="stat">
         <div class="stat__label">Strongest</div>
@@ -78,21 +84,21 @@
             <table class="data">
                 <thead>
                     <tr>
-                        <th>Run</th>
+                        <th class="col-run">Run</th>
-                        <th>Name</th>
+                        <th class="col-name">Name</th>
-                        <th>Date</th>
+                        <th class="col-finished">Date</th>
-                        <th class="num">Reading</th>
+                        <th class="num col-gravity">Reading</th>
-                        <th>Comment</th>
+                        <th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
                     </tr>
                 </thead>
                 <tbody>
                 {% for row in outliers %}
                     <tr>
-                        <td class="mono">{{ row.beer_run_id }}</td>
+                        <td class="col-run mono">{{ row.beer_run_id }}</td>
-                        <td>{{ row.name }}</td>
+                        <td class="col-name">{{ row.name }}</td>
-                        <td class="mono nowrap">{{ row.date or '—' }}</td>
+                        <td class="col-finished mono nowrap">{{ row.date or '—' }}</td>
-                        <td class="num"><span class="chip chip--warn">{{ row.sg }}</span></td>
+                        <td class="col-gravity num"><span class="chip chip--warn">{{ row.sg }}</span></td>
-                        <td class="comment">{{ row.comment or '—' }}</td>
+                        <td class="col-comment comment">{{ row.comment or '—' }}</td>
                     </tr>
                 {% endfor %}
                 </tbody>
@@ -112,45 +118,45 @@
             <table class="data">
                 <thead>
                     <tr>
-                        <th>Run</th>
+                        <th class="col-run">Run</th>
-                        <th>Name</th>
+                        <th class="col-name">Name</th>
-                        <th>Status</th>
+                        <th class="col-finished" title="Status"><span class="long">Status</span><span class="short">Stat</span></th>
-                        <th class="num">OG</th>
+                        <th class="num col-gravity">OG</th>
-                        <th class="num">FG</th>
+                        <th class="num col-gravity">FG</th>
-                        <th class="num">Drop</th>
+                        <th class="num col-drop">Drop</th>
-                        <th class="num">ABV</th>
+                        <th class="num col-abv">ABV</th>
-                        <th class="num">Legacy</th>
+                        <th class="num col-legacy">Legacy</th>
-                        <th>Finished</th>
+                        <th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
-                        <th>Comment</th>
+                        <th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
-                        <th></th>
+                        <th class="col-action"></th>
                     </tr>
                 </thead>
                 <tbody>
                 {% for run in summaries %}
                     <tr>
-                        <td class="mono">{{ run.beer_run_id }}</td>
+                        <td class="col-run mono">{{ run.beer_run_id }}</td>
-                        <td>{{ run.canonical_name }}</td>
+                        <td class="col-name">{{ run.canonical_name }}</td>
-                        <td>
+                        <td class="col-finished">
                             {% if run.active %}
                                 <span class="chip chip--active">Active</span>
                             {% else %}
                                 <span class="chip chip--done">Finished</span>
                             {% endif %}
                         </td>
-                        <td class="num">{{ run.og_points or '—' }}</td>
+                        <td class="col-drop col-gravity num">{{ run.og_points or '—' }}</td>
-                        <td class="num">{{ run.fg_points or '—' }}</td>
+                        <td class="col-gravity num">{{ run.fg_points or '—' }}</td>
-                        <td class="num">{{ run.drop_points if run.drop_points is not none else '—' }}</td>
+                        <td class="col-gravity num">{{ run.drop_points if run.drop_points is not none else '—' }}</td>
-                        <td class="num">
+                        <td class="col-abv num">
                             {% if run.abv is not none %}<strong>{{ '%.2f' % run.abv }}%</strong>
                             {% elif run.projected_abv is not none %}{{ '%.2f' % run.projected_abv }}%*
                             {% else %}—{% endif %}
                         </td>
-                        <td class="num muted">{{ run.legacy_abv if run.legacy_abv is not none else '—' }}</td>
+                        <td class="col-legacy col-abv num muted">{{ run.legacy_abv if run.legacy_abv is not none else '—' }}</td>
-                        <td class="mono nowrap">{{ run.final_date or '—' }}</td>
+                        <td class="col-finished mono nowrap">{{ run.final_date or '—' }}</td>
-                        <td class="comment">
+                        <td class="col-comment comment">
                             {% if run.comments %}{{ run.comments[-1] }}{% else %}<span class="faint">—</span>{% endif %}
                         </td>
-                        <td class="right">
+                        <td class="right col-action">
                             <a class="btn btn--ghost btn--sm"
                                href="{{ url_for('brews.run_detail', run_id=run.beer_run_id) }}">Open</a>
                         </td>
@@ -172,29 +178,29 @@
             <table class="data">
                 <thead>
                     <tr>
-                        <th>Run</th>
+                        <th class="col-run">Run</th>
-                        <th>Name</th>
+                        <th class="col-name">Name</th>
-                        <th class="num">Standard ABV</th>
+                        <th class="num col-abv" title="Standard ABV"><span class="long">Standard ABV</span><span class="short">Std</span></th>
-                        <th class="num">Legacy ABV</th>
+                        <th class="num col-legacy col-abv" title="Legacy ABV"><span class="long">Legacy ABV</span><span class="short">Old</span></th>
-                        <th class="num">Overstated by</th>
+                        <th class="num col-abv" title="Overstated by"><span class="long">Overstated by</span><span class="short">Over</span></th>
-                        <th>Finished</th>
+                        <th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
                     </tr>
                 </thead>
                 <tbody>
                 {% for row in abv_rows %}
                     <tr>
-                        <td class="mono">{{ row.beer_run_id }}</td>
+                        <td class="col-run mono">{{ row.beer_run_id }}</td>
-                        <td>{{ row.name }}</td>
+                        <td class="col-name">{{ row.name }}</td>
-                        <td class="num">{{ '%.2f' % row.abv }}%</td>
+                        <td class="col-abv num">{{ '%.2f' % row.abv }}%</td>
-                        <td class="num muted">{{ '%.2f' % row.legacy_abv }}%</td>
+                        <td class="col-abv num muted">{{ '%.2f' % row.legacy_abv }}%</td>
-                        <td class="num">
+                        <td class="col-abv num">
                             {% if row.delta > 0 %}
                                 <span class="chip chip--warn">+{{ '%.2f' % row.delta }}</span>
                             {% else %}
                                 <span class="faint">{{ '%.2f' % row.delta }}</span>
                             {% endif %}
                         </td>
-                        <td class="mono nowrap">{{ row.final_date or '—' }}</td>
+                        <td class="col-finished mono nowrap">{{ row.final_date or '—' }}</td>
                     </tr>
                 {% endfor %}
                 </tbody>

src/flask/views/analysis.py

@@ -55,6 +55,15 @@ def deep():
     suspect = implausible_runs(finished)
     suspect_ids = {s["beer_run_id"] for s in suspect}
 
+    # A run that is still fermenting can also have an impossible reading,
+    # and the API flags those too - so the charts leave them out.  If they
+    # were not listed here they would disappear from the analysis page
+    # entirely, with no explanation anywhere.
+    active_suspect = implausible_runs([r for r in runs if r.is_active()])
+    if active_suspect:
+        suspect = suspect + active_suspect
+        suspect_ids |= {s["beer_run_id"] for s in active_suspect}
+
     rows = []
     for run in finished:
         meta = metadata.get(run.beer_run_id, {})
@@ -111,6 +120,10 @@ def deep():
             "suspect_count": len(suspect),
             # How many runs the figures are actually based on.
             "trusted_count": len(good),
+            # How many of those actually contributed a figure to the
+            # average - a run with no original gravity has no attenuation,
+            # so it is counted above but not in the mean.
+            "counted_attenuation": len(attenuations),
             "avg_attenuation": round(sum(attenuations) / len(attenuations), 1)
             if attenuations else None,
             "avg_days": _fmt_days(sum(durations) / len(durations)) if durations else None,
@@ -278,5 +291,9 @@ def history():
             "avg_abv": avg_abv,
             "best": best,
             "worst": worst,
+            # How many of the finished brews the ABV figures are based
+            # on, so the tile can say so.
+            "abv_counted": len(good_abv_rows),
+            "abv_total": len(completed),
         },
     )

src/flask/views/brews.py

@@ -53,14 +53,15 @@ def advice_enabled() -> bool:
     return os.getenv("LLM_ADVICE", "1").strip().lower() not in {"0", "false", "no"}
 
 
-def _start_advice(run_id: str, reading_id: str) -> None:
+def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None:
     """Generate the LLM's advice for a reading, in the background.
 
     The advice is a separate field on the reading, so the brewer's own
     comment is never touched.
 
     The background thread cannot use the request-scoped repository - that
-    client is closed when the request ends - so it builds its own.
+    client is closed when the request ends - so it uses the shared
+    process-wide one.
     """
     if not reading_id:
         return
@@ -89,7 +90,8 @@ def _start_advice(run_id: str, reading_id: str) -> None:
         for reading in run.readings:
             points = reading.sg_points
             readings.append({
-                "_id": reading._id,
+                "_id": str(reading._id),
+                "advice_token": reading.advice_token,
                 "date": reading.date.isoformat() if reading.date else "",
                 "sg": points,
                 "sg_decimal": reading.sg_decimal,
@@ -98,20 +100,75 @@ def _start_advice(run_id: str, reading_id: str) -> None:
                 "final": bool(reading.final_reading),
             })
 
-        latest = dict(readings[-1]) if readings else {"date": "", "sg": None}
+        # The reading that is actually being advised about.  It is not
+        # necessarily the newest one: the brewer may be editing an older
+        # reading, and advice about the newest reading would be about the
+        # wrong number.
+        saved = next((r for r in readings if r.get("_id") == str(reading_id)), None)
+        if saved is None:
+            # The reading is not in the run.  Advising about the newest
+            # reading instead would describe the wrong number, and the
+            # conditional write would then refuse it anyway - so it is
+            # better to say so plainly and stop.
+            log.info(
+                "no advice for reading %s: it is not one of the run's readings",
+                reading_id,
+            )
+            return
+        latest = dict(saved)
         latest["_id"] = reading_id
 
     except Exception:  # noqa: BLE001 - advice must never break the save
         log.exception("could not gather the reading for the advice")
         return
 
+    # The gravity and the editing token of the reading being advised
+    # about.  The write is conditional on both, so a slow advice thread
+    # 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:
-        from mongo.repository import BeerRepository
+        # The client is SHARED, not built per reading.  A MongoClient
-        BeerRepository().set_reading_advice(read_id, text, ok)
+        # spawns three monitor threads and pymongo does not reap them
+        # on close(), so building one per advice write accumulated three
+        # threads and a connection pool for every reading recorded.
+        #
+        # The shared client is created on first use and lives for the
+        # life of the process, which is what the request-scoped
+        # repository does too.
+        from mongo.repository import get_shared_repository
+
+        try:
+            shared = get_shared_repository()
+            written = shared.set_reading_advice(
+                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
+                # generated, so the advice no longer matches it.  That is
+                # the guard working - the fresh advice for the corrected
+                # 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 its "
+                    "editing token or comment no longer matches the advice",
+                    read_id,
+                )
+        except Exception:  # noqa: BLE001 - never let the thread die noisily
+            log.exception("could not store the generated advice")
 
     thread = threading.Thread(
         target=brew_advice.generate_advice_in_background,
-        args=(run_info, latest, readings, write),
+        args=(run_info, latest, readings, write, corrected),
         daemon=True,
     )
     thread.start()
@@ -188,6 +245,14 @@ def index():
                     "warning",
                 )
         run_id = str(form.beer_run_id.data)
+        # Whether the reading was already there: a re-submitted reading
+        # is a CORRECTION, and the advice should say so.
+        existing = repo().reading_docs_for_run(run_id)
+        was_existing = any(
+            str(doc.get("date")) == form.Date.data.isoformat()
+            for doc in existing
+        )
+
         reading_id = repo().upsert_reading(
             beer_run_id=run_id,
             beer_type=(form.beer_run_type.data or "").strip(),
@@ -201,7 +266,7 @@ def index():
         # The advice is generated in the background: recording a reading
         # must not block on the model.  The page shows the reading straight
         # away and the advice is there on a later visit.
-        _start_advice(run_id, reading_id)
+        _start_advice(run_id, reading_id, corrected=was_existing)
         return redirect(url_for("brews.index"))
 
     runs = repo().runs()
@@ -287,6 +352,10 @@ def api_runs():
     # The runs whose readings are physically impossible.  They are flagged
     # rather than dropped: the charts use the flag to leave them out of the
     # averages and the curve envelope, but a caller can still see them.
+    #
+    # The flag is computed over EVERY run, not just the finished ones, so
+    # it agrees with what the charts do.  An active run with a bad reading
+    # is still a run with a bad reading.
     suspect_ids = {s["beer_run_id"] for s in implausible_runs(runs)}
 
     payload = []
@@ -317,6 +386,7 @@ def api_run(run_id: str):
     # The same flag the list endpoint carries, so a consumer of the detail
     # endpoint can tell an impossible run from a good one.
     summary["suspect"] = bool(implausible_runs([run]))
+    summary["active"] = run.is_active()
     return jsonify(summary)
 
 

tests/test_analysis.py

@@ -508,3 +508,121 @@ def test_the_charts_fetch_the_run_payload_once():
         "the run payload should be fetched once and shared"
     )
     assert "function fetchRuns()" in charts_js
+
+# ------------------------------------------------- the chart exclusions #
+
+def test_the_abv_chart_leaves_out_the_suspect_runs():
+    """A suspect run's ABV comes from an impossible reading.
+
+    Run 12 reads 18.64%, and plotting it stretched the y axis so every
+    real bar was squashed into the bottom of the plot - while the
+    "Strongest" tile on the same page correctly showed 8.53%.  The chart
+    and the tile contradicted each other.
+    """
+    charts_js = (
+        Path(__file__).resolve().parents[1]
+        / "src" / "flask" / "static" / "js" / "charts.js"
+    ).read_text()
+
+    abv_chart = charts_js.split("function drawAbvChart")[1]
+    abv_chart = abv_chart.split("function drawMonthsChart")[0]
+    assert "!r.suspect" in abv_chart, (
+        "the ABV chart must leave the suspect runs out"
+    )
+
+
+def test_the_attenuation_chart_leaves_out_the_active_runs():
+    """An active run's attenuation is measured against its CURRENT gravity.
+
+    A brew that has just started shows a 0% bar - which is the absence of
+    a result, not a result.  The chart sits above the table of finished
+    brews.
+    """
+    charts_js = (
+        Path(__file__).resolve().parents[1]
+        / "src" / "flask" / "static" / "js" / "charts.js"
+    ).read_text()
+
+    att_chart = charts_js.split("function drawAttenuationChart")[1]
+    att_chart = att_chart.split("function drawCurveChart")[0]
+    assert "!r.active" in att_chart, (
+        "the attenuation chart must leave the active runs out"
+    )
+
+
+def test_a_suspect_active_run_is_still_listed_for_review(logged_in, repo):
+    """A bad reading in an active run must not make the run vanish.
+
+    The API flags suspect runs over EVERY run, so the charts leave them
+    out.  If the analysis page only looked at the finished ones, an
+    active run with a bad reading would disappear with no explanation
+    anywhere.
+    """
+    seed_readings(repo, [
+        ("1", "Finished ale", "1050", "2026-01-01", False, ""),
+        ("1", "Finished ale", "1010", "2026-01-10", True, ""),
+        # An ACTIVE run whose latest reading is below water.
+        ("20", "Active brew", "1042", "2026-02-01", False, ""),
+        ("20", "Active brew", "0900", "2026-02-10", False, "typo"),
+    ])
+
+    response = logged_in.get("/analysis")
+    body = response.get_data(as_text=True)
+
+    assert "Readings that need a second look" in body
+    assert "0.900" in body
+
+
+def test_the_history_page_says_how_many_brews_the_abv_figure_is_from(logged_in, repo):
+    """The tile must not quietly average fewer brews than it claims."""
+    seed_readings(repo, [
+        ("1", "Good ale", "1050", "2026-01-01", False, ""),
+        ("1", "Good ale", "1010", "2026-01-10", True, ""),
+        ("12", "Bad brew", "1042", "2026-01-01", False, ""),
+        ("12", "Bad brew", "0900", "2026-02-26", True, "typo"),
+    ])
+
+    response = logged_in.get("/history")
+    body = response.get_data(as_text=True)
+
+    assert "across 1 of 2 finished brews" in body
+
+
+def test_the_analysis_prose_counts_the_brews_that_contributed(logged_in, repo):
+    """The prose must not claim more brews than actually contributed.
+
+    The prose is rendered from the attenuation figures, so it counts the
+    brews that produced one - not the brews that were merely counted.
+    """
+    seed_readings(repo, [
+        ("1", "Good ale", "1050", "2026-01-01", False, ""),
+        ("1", "Good ale", "1010", "2026-01-10", True, ""),
+        ("2", "Good pale", "1042", "2026-02-01", False, ""),
+        ("2", "Good pale", "1012", "2026-02-10", True, ""),
+        ("12", "Bad brew", "1042", "2026-01-01", False, ""),
+        ("12", "Bad brew", "0900", "2026-02-26", True, "typo"),
+    ])
+
+    response = logged_in.get("/analysis")
+    body = " ".join(response.get_data(as_text=True).split())
+
+    # Three finished brews, two of which contributed an attenuation.
+    assert "Across 2 of 3 finished brews" in body
+
+
+def test_the_run_detail_api_carries_the_same_flags(logged_in, repo):
+    """A consumer of the detail endpoint can tell an impossible run from a
+    good one, and an active run from a finished one."""
+    seed_readings(repo, [
+        ("1", "Good ale", "1050", "2026-01-01", False, ""),
+        ("1", "Good ale", "1010", "2026-01-10", True, ""),
+        ("20", "Active brew", "1042", "2026-02-01", False, ""),
+    ])
+
+    finished = logged_in.get("/api/runs/1").get_json()
+    assert finished["suspect"] is False
+    assert finished["active"] is False
+
+    active = logged_in.get("/api/runs/20").get_json()
+    assert active["suspect"] is False
+    assert active["active"] is True

tests/test_app.py

@@ -714,3 +714,15 @@ def test_the_api_exposes_the_attenuation_and_curve(logged_in, repo):
     assert len(entry["curve"]) == 2
     assert entry["curve"][0]["progress"] == 0.0
     assert entry["curve"][1]["progress"] == 1.0
+
+
+def test_the_pages_are_not_cached(client):
+    """The HTML pages must not be cached by the browser.
+
+    A phone held on to a cached copy of the brewing page and kept
+    showing the old inline advice after the button and the pop-out had
+    been deployed, so the change was invisible until the cache was
+    cleared by hand.
+    """
+    response = client.get("/login")
+    assert "no-store" in response.headers.get("Cache-Control", "")

tests/test_brew_advice.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import json
+from contextlib import contextmanager
 from datetime import date
 
 import pytest
@@ -55,6 +56,46 @@ def test_the_prompt_tells_the_model_not_to_call_a_reading_impossible():
     assert "still be real" in brew_advice.SYSTEM_PROMPT
 
 
+def test_the_prompt_names_the_reading_to_advise_about():
+    """The reading being advised about must be NAMED.
+
+    The brewer may be correcting an OLDER reading, so "the most recent
+    reading" is the wrong one - and the advice would be about the wrong
+    number.  The reading is marked in the list and named in the
+    instruction.
+    """
+    run = {"canonical_name": "Ale"}
+    older = {"_id": "old", "date": "2026-01-01", "sg_decimal": 1.042,
+             "point_scale": "1042 points", "comment": "", "final": False}
+    newer = {"_id": "new", "date": "2026-01-10", "sg_decimal": 1.030,
+             "point_scale": "1030 points", "comment": "", "final": False}
+
+    prompt = brew_advice.build_prompt(run, older, [older, newer])
+
+    # The older one is the subject.
+    assert "THIS IS THE READING TO ADVISE ON" in prompt
+    # And it is named, with its date and gravity.
+    assert "2026-01-01" in prompt
+    assert "1.042" in prompt
+    # And NOT the newer one.
+    assert "1.03" in prompt  # it is in the history, which is right
+    assert prompt.count("THIS IS THE READING TO ADVISE ON") == 1
+
+
+def test_the_prompt_says_when_a_reading_has_been_corrected():
+    """A corrected reading may differ from what it said before."""
+    run = {"canonical_name": "Ale"}
+    reading = {"_id": "r1", "date": "2026-01-01", "sg_decimal": 1.012,
+               "point_scale": "1012 points", "comment": "", "final": False}
+
+    plain = brew_advice.build_prompt(run, reading, [reading])
+    assert "just corrected" not in plain
+
+    corrected = brew_advice.build_prompt(run, reading, [reading], corrected=True)
+    assert "just corrected" in corrected
+    assert "may differ" in corrected
+
+
 def test_the_prompt_marks_the_final_reading():
     run = {"canonical_name": "Ale"}
     reading = {"date": "2026-01-20", "sg_decimal": 1.010, "point_scale": "1010 points",
@@ -76,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):
@@ -179,12 +220,128 @@ def test_the_token_budget_leaves_room_for_the_reasoning(monkeypatch):
     brew_advice.generate_advice({"canonical_name": "Ale"},
                                  {"date": "2026-01-01"}, [])
 
-    assert captured["body"]["max_tokens"] >= 2000
+    # The reasoning comes out of the same budget as the answer, and
+    # `think: false` does not stop it - the models just move the working
+    # into the content.  The budget has to cover both.
+    assert captured["body"]["max_tokens"] >= 8000
+
+
+
+
+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 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("Warm the fermenter")
+    assert "Let me think" not in stripped
+    assert "1.055" not in stripped
+
+
+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_the_marker_is_found_however_the_model_formats_it():
+    """The model formats the marker its own way often enough to matter.
+
+    The instruction asks for "ADVICE:", but the model writes
+    "**ADVICE:**", "## ADVICE:" or puts it in a code block.  Each of
+    those has to extract the advice cleanly, with no leftover markdown.
+    """
+    advice = "Warm it to 20C and wait."
+
+    for reply in (
+        f"ADVICE:\n{advice}",
+        f"**ADVICE:**\n{advice}",
+        f"## ADVICE:\n{advice}",
+        f"```\nADVICE:\n{advice}\n```",
+        f"**ADVICE:**\n```\n{advice}\n```",
+        f"advice:\n{advice}",
+    ):
+        assert brew_advice._strip_reasoning(reply) == advice, (
+            f"the marker was not extracted cleanly from {reply!r}"
+        )
+
+
+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.
+
+    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
+    long_text = "ADVICE: " + ("x" * 5000)
 
     class Response:
         def read(self):
@@ -225,6 +382,530 @@ def test_the_advice_is_stored_in_its_own_field(repo):
     assert updated["comment"] == "my own comment"
 
 
+def test_editing_a_reading_clears_its_stale_advice(repo):
+    """The advice was written for the gravity that was there before.
+
+    If the reading is edited, that advice no longer describes it, so it
+    is cleared and a fresh one is generated.  The brewer's own comment is
+    never cleared.
+    """
+    seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "my comment")])
+
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    repo.set_reading_advice(str(doc["_id"]), "THE ADVICE", True)
+
+    # Edit the reading.
+    repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1030,
+                         reading_date="2026-01-01", final_reading=False,
+                         comment="my comment")
+
+    after = repo.readings.find_one({"beer_run_id": "1"})
+    assert after.get("advice") is None, "the stale advice was kept"
+    assert after["comment"] == "my comment", "the brewer's comment was touched"
+    assert after["sg"] == "1030"
+
+
+def test_the_advice_is_stored_on_a_reading_with_a_string_id(repo):
+    """The original app's readings have 24-character STRING ids.
+
+    This is the shape every existing reading in the live database has -
+    the ids were generated by the old app as random characters, not by
+    Mongo.  A helper that only accepted an ObjectId would have matched
+    nothing, and the advice would silently never be stored.
+
+    The mongomock fixture generates ObjectIds, so this has to be
+    constructed explicitly or the bug is invisible in the tests.
+    """
+    repo.readings.insert_one({
+        "_id": "l78ZidBJ3QaYn98tvB88x4uM",
+        "beer_run_id": "1",
+        "beer_type": "Ale",
+        "sg": "1042",
+        "date": "2026-01-01",
+        "final_reading": "False",
+        "comment": "First ever try",
+    })
+
+    modified = repo.set_reading_advice("l78ZidBJ3QaYn98tvB88x4uM",
+                                        "THE ADVICE", True)
+    assert modified == 1, "the advice was not stored on a string-id reading"
+
+    after = repo.readings.find_one({"_id": "l78ZidBJ3QaYn98tvB88x4uM"})
+    assert after["advice"] == "THE ADVICE"
+    assert after["comment"] == "First ever try"
+
+
+def test_the_advice_is_stored_on_a_reading_with_an_object_id(repo):
+    """The other shape: a reading written by Mongo itself."""
+    seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "a comment")])
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+
+    modified = repo.set_reading_advice(str(doc["_id"]), "THE ADVICE", True)
+    assert modified == 1
+
+    after = repo.readings.find_one({"beer_run_id": "1"})
+    assert after["advice"] == "THE ADVICE"
+
+
+def test_writing_the_advice_does_not_leak_a_client_per_reading():
+    """A MongoClient spawns three monitor threads and pymongo does not
+    reap them on close() - they go only when the client is collected.
+
+    So building a client per advice write accumulated three threads and a
+    connection pool for every reading recorded.  The advice writer shares
+    one client for the life of the process instead.
+    """
+    import inspect
+
+    from views import brews as brews_module
+
+    # The writer must NOT build a repository of its own - it has to use
+    # the shared one.  A MongoClient spawns three monitor threads and
+    # pymongo does not reap them on close(), so a client per advice write
+    # accumulated three threads and a connection pool for every reading
+    # recorded.
+    source = inspect.getsource(brews_module._start_advice)
+
+    assert "BeerRepository()" not in source, (
+        "the advice writer builds its own repository - that leaks a Mongo "
+        "client (three monitor threads) per reading"
+    )
+    assert "get_shared_repository" in source, (
+        "the advice writer must use the shared repository"
+    )
+
+
+@contextmanager
+def _app_with_repo(repo):
+    """An application context whose repository is *repo*.
+
+    The view reads the repository from ``app.extensions``, so pointing
+    that at a fixture is enough to drive the view's code directly.
+    """
+    from app_factory import create_app
+
+    app = create_app({
+        "TESTING": True,
+        "WTF_CSRF_ENABLED": False,
+        "SECRET_KEY": "test-secret",
+        "MONGO_CLIENT": repo.client,
+    })
+    # The advice must be generated even though this is a test app.
+    app.config["TESTING"] = False
+
+    original = app.extensions["beer_repo"]
+    app.extensions["beer_repo"] = lambda: repo
+    with app.app_context():
+        try:
+            yield app
+        finally:
+            app.extensions["beer_repo"] = original
+
+
+def test_the_advice_is_about_the_reading_that_was_saved():
+    """Editing an OLDER reading must not generate advice about the newest.
+
+    The advice is written against a specific reading, so the reading being
+    saved has to be the one the advice is about.
+    """
+    import time
+
+    import mongomock
+    from mongo.repository import BeerRepository
+    from views import brews as brews_module
+    from views.brews import _start_advice
+
+    captured = {}
+
+    def fake_generate(run, reading, history, writer=None, corrected=False):
+        captured["reading"] = reading
+        captured["history"] = history
+        captured["corrected"] = corrected
+        return True, "THE ADVICE"
+
+    original = brews_module.brew_advice.generate_advice_in_background
+    brews_module.brew_advice.generate_advice_in_background = fake_generate
+    try:
+        # Two readings; the OLDER one is the one being saved.
+        repo = BeerRepository(client=mongomock.MongoClient())
+        old_id = repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1042,
+                                     reading_date="2026-01-01", final_reading=False,
+                                     comment="first")
+        repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1030,
+                            reading_date="2026-01-10", final_reading=False,
+                            comment="second")
+
+        # The advice is only generated when it is enabled, and it is off
+        # in testing.
+        with _app_with_repo(repo):
+            _start_advice("1", old_id)
+            for _ in range(40):
+                time.sleep(0.05)
+                if captured:
+                    break
+    finally:
+        brews_module.brew_advice.generate_advice_in_background = original
+
+    assert captured, "the advice was never generated"
+    assert captured["reading"]["_id"] == old_id
+    assert captured["reading"]["sg"] == 1042, (
+        "the advice was generated about the wrong reading"
+    )
+
+    # And the PROMPT must ask about that reading - not just pass it
+    # through.  Asserting only which dict was passed would not have
+    # caught the prompt still saying "give advice for the most recent
+    # reading", which is the bug that made the model advise about the
+    # wrong number.
+    prompt = brew_advice.build_prompt(
+        {"canonical_name": "Ale"},
+        captured["reading"],
+        captured["history"],
+    )
+    assert "THIS IS THE READING TO ADVISE ON" in prompt
+    assert "2026-01-01" in prompt
+    assert "most recent reading" not in prompt
+
+
+def test_the_shared_repository_is_built_only_once():
+    """Concurrent advice threads must not each build their own client.
+
+    An unsynchronised check-then-set on a module global let eight threads
+    on a cold process each build their own repository - so the fix
+    reintroduced the very leak it was meant to remove, just bounded by a
+    race instead of by the number of readings.
+    """
+    import threading
+
+    import mongomock
+    import mongo.repository as R
+
+    original_shared = R._shared_repository
+    built = []
+    original_init = R.BeerRepository.__init__
+
+    def counting_init(self, *args, **kwargs):
+        built.append(1)
+        original_init(self, *args, **kwargs)
+
+    R._shared_repository = None
+    R.BeerRepository.__init__ = counting_init
+    original_build = R._build_client
+    R._build_client = lambda: mongomock.MongoClient()
+    try:
+        instances = []
+        barrier = threading.Barrier(8)
+
+        def go():
+            barrier.wait()          # release all eight at once
+            instances.append(id(R.get_shared_repository()))
+
+        threads = [threading.Thread(target=go) for _ in range(8)]
+        for t in threads:
+            t.start()
+        for t in threads:
+            t.join()
+    finally:
+        R._shared_repository = original_shared
+        R.BeerRepository.__init__ = original_init
+        R._build_client = original_build
+
+    assert len(built) == 1, f"built {len(built)} repositories, expected 1"
+    assert len(set(instances)) == 1, "the threads got different repositories"
+
+
+def test_stale_advice_cannot_overwrite_a_correction(repo):
+    """A slow advice thread must not overwrite a correction.
+
+    The advice is generated in the background.  If the brewer corrects a
+    reading while the advice for the old gravity is still being
+    generated, the slow thread lands afterwards - and without a guard the
+    reading ends up with the final gravity from the correction and the
+    advice from before it.
+
+    The write is conditional on the reading still holding the gravity the
+    advice was generated for.
+    """
+    seed_readings(repo, [("1", "Ale", "1050", "2026-01-01", False, "a comment")])
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    reading_id = str(doc["_id"])
+
+    # The advice for the ORIGINAL 1050 arrives late...
+    modified = repo.set_reading_advice(reading_id, "ADVICE FOR THE 1050",
+                                        True, expected_sg=1050)
+    assert modified == 1
+
+    # ...but the brewer has already corrected it to 1020, and the fresh
+    # advice has landed.
+    repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1020,
+                         reading_date="2026-01-01", final_reading=False,
+                         comment="a comment")
+    repo.set_reading_advice(reading_id, "ADVICE FOR THE 1020", True,
+                             expected_sg=1020)
+
+    # Now the late thread for the old 1050 finally lands.
+    repo.set_reading_advice(reading_id, "STALE ADVICE FOR THE 1050", True,
+                             expected_sg=1050)
+
+    after = repo.readings.find_one({"beer_run_id": "1"})
+    assert after["sg"] == "1020"
+    assert after["advice"] == "ADVICE FOR THE 1020", (
+        f"the stale advice overwrote the correction: {after['advice']!r}"
+    )
+
+
+def test_a_comment_only_correction_cannot_be_overwritten(repo):
+    """A comment-only correction leaves the gravity unchanged.
+
+    So the gravity alone cannot tell the two generations apart - a late
+    thread for the old advice would still match and overwrite the fresh
+    one.  The editing token is what tells them apart.
+    """
+    # Written through upsert_reading, so it has an editing token.
+    reading_id = repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1050,
+                                     reading_date="2026-01-01",
+                                     final_reading=False, comment="first comment")
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    token = doc.get("advice_token")
+    assert token, "the reading has no editing token"
+
+    # The advice thread starts on this generation.
+    # The brewer corrects the COMMENT only - the gravity is unchanged.
+    repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1050,
+                         reading_date="2026-01-01", final_reading=False,
+                         comment="corrected comment")
+    assert repo.readings.find_one({"beer_run_id": "1"}).get("advice_token") != token, (
+        "the correction did not write a fresh token"
+    )
+
+    # The fresh advice lands.
+    fresh_token = repo.readings.find_one({"beer_run_id": "1"}).get("advice_token")
+    assert repo.set_reading_advice(reading_id, "FRESH ADVICE", True,
+                                    expected_sg=1050,
+                                    expected_token=fresh_token) == 1
+
+    # The late thread for the OLD generation finally lands.
+    stale = repo.set_reading_advice(reading_id, "STALE ADVICE", True,
+                                     expected_sg=1050, expected_token=token)
+    assert stale == 0, "the stale advice overwrote the correction"
+
+    after = repo.readings.find_one({"beer_run_id": "1"})
+    assert after["advice"] == "FRESH ADVICE"
+    assert after["comment"] == "corrected comment"
+
+
+
+
+
+
+
+
+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_the_advice_is_written_for_a_reading_that_predates_the_token(repo):
+    """None of the existing readings have a token.
+
+    They all predate it, so a reading read from the database has an EMPTY
+    token.  An empty token is not a usable guard - there is no token to
+    match - so the write must not be conditioned on it, or the advice
+    would never be written for any of them.
+    """
+    # Inserted directly, the way the readings already in the database
+    # are - no token.
+    repo.readings.insert_one({
+        "_id": "l78ZidBJ3QaYn98tvB88x4uM",
+        "beer_run_id": "1",
+        "beer_type": "Ale",
+        "sg": "1042",
+        "date": "2024-09-01",
+        "final_reading": "False",
+        "comment": "a comment",
+    })
+
+    run = repo.run("1")
+    assert run.readings[0].advice_token == "", "the reading should have no token"
+
+    # The advice flow passes the token it read, which is empty.
+    written = repo.set_reading_advice("l78ZidBJ3QaYn98tvB88x4uM",
+                                       "THE ADVICE", True,
+                                       expected_sg=None,
+                                       expected_token=run.readings[0].advice_token)
+    assert written == 1, "the advice was never written for a pre-token reading"
+
+    after = repo.readings.find_one({"beer_run_id": "1"})
+    assert after["advice"] == "THE ADVICE"
+    assert after["comment"] == "a comment"
+
+
+def test_a_pre_token_reading_is_still_guarded_against_a_stale_write(repo):
+    """A reading that has never been edited has no token at all.
+
+    The guard matches on the ABSENCE of a token, so an edit still tells
+    the generations apart.  Falling back to the gravity here would
+    re-open the hole, because a comment-only correction leaves the
+    gravity unchanged.
+    """
+    repo.readings.insert_one({
+        "_id": "preToken0000000000000010",
+        "beer_run_id": "1",
+        "beer_type": "Ale",
+        "sg": "1050",
+        "date": "2026-11-01",
+        "final_reading": "False",
+        "comment": "first comment",
+    })
+
+    # The advice thread starts on this generation - no token.
+    assert repo.set_reading_advice("preToken0000000000000010", "FRESH ADVICE",
+                                    True, expected_sg=None,
+                                    expected_token="") == 1
+
+    # The brewer corrects the comment.  The edit writes a token.
+    repo.upsert_reading(beer_run_id="1", beer_type="Ale", sg=1050,
+                         reading_date="2026-11-01", final_reading=False,
+                         comment="corrected comment")
+
+    # The late thread for the OLD generation lands.  It carries no token,
+    # and the reading now HAS one - so it must be refused.
+    stale = repo.set_reading_advice("preToken0000000000000010", "STALE ADVICE",
+                                     True, expected_sg=None,
+                                     expected_token="")
+    assert stale == 0, "the stale advice overwrote the correction"
+
+    after = repo.readings.find_one({"beer_run_id": "1"})
+    assert after.get("advice") is None
+    assert after["comment"] == "corrected comment"
+
+
+def test_the_advice_is_written_for_a_reading_with_no_token_field(repo):
+    """All three shapes of "no token" must work.
+
+    A reading read from the database has no token at all; one written by
+    an older version could have the field present but empty, or null.
+    The guard matches on the absence of a token, so it has to cover all
+    three - otherwise the advice is silently never written.
+    """
+    for index, extra in enumerate(({}, {"advice_token": ""}, {"advice_token": None})):
+        reading_id = f"noToken{index:04d}"
+        repo.readings.insert_one({
+            "_id": reading_id,
+            "beer_run_id": str(index),
+            "beer_type": "Ale",
+            "sg": "1050",
+            "date": "2026-01-01",
+            "final_reading": "False",
+            "comment": "c",
+            **extra,
+        })
+
+        written = repo.set_reading_advice(reading_id, "THE ADVICE", True,
+                                           expected_sg=None,
+                                           expected_token="")
+        assert written == 1, f"the advice was not written for {extra!r}"
+
+
+def test_a_reply_cut_off_at_the_token_ceiling_is_marked(repo):
+    """A reply that stopped at the token limit is incomplete.
+
+    It may be advice that stops mid-sentence, so it must not be stored
+    as if it were the whole answer.
+    """
+    captured = {}
+
+    def fake_urlopen(request, timeout=None):
+        captured["body"] = json.loads(request.data.decode())
+
+        class Response:
+            def read(self):
+                return json.dumps({
+                    "choices": [{
+                        "finish_reason": "length",
+                        "message": {"content": "ADVICE: The gravity looks fine so far, but"},
+                    }],
+                }).encode()
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return False
+
+        return Response()
+
+    import brew_advice as ba
+    original = ba.urllib.request.urlopen
+    ba.urllib.request.urlopen = fake_urlopen
+    try:
+        ok, text = ba.generate_advice({"canonical_name": "Ale"},
+                                       {"date": "2026-01-01"}, [])
+    finally:
+        ba.urllib.request.urlopen = original
+
+    assert ok is True
+    assert "cut off" in text
+
+
+def test_the_reading_id_cannot_be_used_to_inject_a_query(repo):
+    """The reading id is coerced before it reaches Mongo.
+
+    Every value that reaches a Mongo query is coerced first, because a
+    caller passing a dict would have it interpreted as a query operator
+    rather than a value - the classic NoSQL injection route.  A reading
+    id that is not a plain ObjectId or string must match nothing.
+    """
+    seed_readings(repo, [
+        ("1", "Ale", "1042", "2026-01-01", False, ""),
+        ("2", "Ale", "1030", "2026-01-01", False, ""),
+    ])
+
+    for hostile in [{"$ne": None}, {"$gt": ""}, ["a"], 123, None,
+                    "not-an-objectid", ""]:
+        modified = repo.set_reading_advice(hostile, "INJECTED", True)
+        assert modified == 0, f"{hostile!r} matched something"
+
+    # The helper itself must return something that is safe to put in a
+    # query - an empty list matches nothing.  Returning None would raise
+    # "$in needs an array" if it were ever used without a guard.
+    from mongo.repository import _as_doc_id
+
+    for hostile in [{"$ne": None}, {"$gt": ""}, ["a"], 123, None, ""]:
+        assert _as_doc_id(hostile) == [], f"{hostile!r} produced a usable id"
+
+    # And no document was touched.
+    for doc in repo.readings.find({}):
+        assert doc.get("advice") is None
+
+
 def test_a_failed_generation_is_stored_as_a_failure(repo):
     """The note says it could not be generated, and is flagged as such."""
     seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "")])
@@ -314,6 +995,196 @@ def test_the_advice_is_shown_on_the_active_beers_page(client, repo):
     assert "Brewing advice" in body
 
 
+def test_the_advice_in_the_table_is_a_popout_not_the_text(client, repo):
+    """The advice in the table is a trigger, and the text is in a panel.
+
+    The advice is several hundred characters.  In a table cell it made
+    the table wider than a phone, so the column was clipped off the
+    right edge and the advice could not be read.
+    """
+    repo.users.insert_one({"username": "beer", "password": "beer_pass"})
+    client.post("/login", data={"username": "beer", "password": "beer_pass"},
+                follow_redirects=True)
+
+    seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "")])
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    repo.set_reading_advice(str(doc["_id"]), "Warm it to 21C and wait.", ok=True)
+
+    body = client.get("/brews").get_data(as_text=True)
+
+    # The trigger is a real button, so it works with a keyboard.
+    assert "advice-trigger" in body
+    assert 'class="advice-trigger' in body
+
+    # The text lives in the panel, not in the cell.
+    assert "advice-pop" in body
+    assert "advice-pop__text" in body
+
+    # The full advice is in the page - it is just not in the cell.
+    assert "Warm it to 21C and wait." in body
+
+
+def test_the_tables_keep_their_columns_on_a_phone(client, repo):
+    """The tables are readable on a phone, not squeezed into nothing.
+
+    The advice is a button and the table scrolls, so nothing is
+    truncated to an unreadable stub.
+    """
+    repo.users.insert_one({"username": "beer", "password": "beer_pass"})
+    client.post("/login", data={"username": "beer", "password": "beer_pass"},
+                follow_redirects=True)
+
+    seed_readings(repo, [("1", "Cider raspberry elderberry", "1045", "2026-01-01", False, "")])
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    repo.set_reading_advice(str(doc["_id"]), "Warm it to 21C and wait.", ok=True)
+
+    body = client.get("/brews").get_data(as_text=True)
+
+    # The long heading has a short form for a phone, and the long form
+    # is still in the page for a desktop.
+    assert "Brewing advice" in body
+    assert ">Advice<" in body.replace("\n", "")
+
+    # The beer name is not truncated in the markup.
+    assert "Cider raspberry elderberry" in body
+
+
+def test_the_long_and_short_labels_are_both_in_the_page(client, repo):
+    """The long and the short label are both in the markup.
+
+    CSS picks which one shows - the long form on a desktop, the short
+    form on a phone.  If only one is in the markup the other cannot be
+    shown at all, and if the CSS rule misses one of them the page shows
+    "Read the adviceAdvice".
+
+    This is a markup assertion, not a CSS assertion: it makes sure
+    both forms exist for the stylesheet to choose between.
+    """
+    repo.users.insert_one({"username": "beer", "password": "beer_pass"})
+    client.post("/login", data={"username": "beer", "password": "beer_pass"},
+                follow_redirects=True)
+
+    seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "")])
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    repo.set_reading_advice(str(doc["_id"]), "Warm it to 21C and wait.", ok=True)
+
+    body = client.get("/brews").get_data(as_text=True)
+
+    # The heading has both forms.
+    assert '<span class="long">Brewing advice</span>' in body
+    assert '<span class="short">Advice</span>' in body
+
+    # The button has both forms.
+    assert '<span class="long">Read the advice</span>' in body
+    assert '<span class="short">Advice</span>' in body
+
+
+def test_stale_advice_cannot_overwrite_a_comment_only_correction(repo):
+    """A comment-only correction still invalidates the old advice.
+
+    The gravity alone is not enough to tell two generations apart - a
+    comment-only correction leaves the gravity unchanged.  The editing
+    token is what tells them apart, because every edit writes a fresh
+    one.
+    """
+    repo.readings.insert_one({
+        "_id": "readingone",
+        "beer_run_id": "1",
+        "date": "2026-01-01",
+        "sg": "1040",
+        "comment": "original comment",
+        "final_reading": "False",
+        "advice_token": "token-one",
+    })
+
+    # The slow thread holds the token it was given.
+    stale_token = "token-one"
+
+    # The brewer corrects only the comment.
+    repo.readings.update_one(
+        {"_id": "readingone"},
+        {"$set": {"comment": "corrected comment", "advice_token": "token-two"},
+         "$unset": {"advice": "", "advice_ok": "", "advice_at": ""}},
+    )
+
+    # The fresh advice lands first, carrying the new token.
+    assert repo.set_reading_advice(
+        "readingone", "FRESH ADVICE", ok=True, expected_token="token-two") == 1
+
+    # The slow thread lands afterwards, carrying the old token.  It must
+    # be refused.
+    assert repo.set_reading_advice(
+        "readingone", "STALE ADVICE", ok=True, expected_token=stale_token) == 0
+
+    doc = repo.readings.find_one({"_id": "readingone"})
+    assert doc["advice"] == "FRESH ADVICE"
+    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_the_advice_pop_out_is_hidden_in_the_markup(client, repo):
+    """The pop-out carries the `hidden` attribute.
+
+    A stale stylesheet - one that does not know about `.advice-pop` at
+    all - would otherwise leave the advice expanded inline in the table
+    cell, running down the column and off the screen.  The `hidden`
+    attribute makes the markup fail-safe: the panel stays hidden until
+    the script opens it.
+    """
+    repo.users.insert_one({"username": "beer", "password": "beer_pass"})
+    client.post("/login", data={"username": "beer", "password": "beer_pass"},
+                follow_redirects=True)
+
+    seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "")])
+    doc = repo.readings.find_one({"beer_run_id": "1"})
+    repo.set_reading_advice(str(doc["_id"]), "Warm it to 21C and wait.", ok=True)
+
+    body = client.get("/brews").get_data(as_text=True)
+
+    assert 'class="advice-pop"' in body
+    assert 'hidden' in body.split('class="advice-pop"')[1][:60]
+
+
 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"})
@@ -327,7 +1198,9 @@ def test_a_failed_advice_is_marked_as_such(client, repo):
 
     response = client.get("/brews")
     body = response.get_data(as_text=True)
-    assert "advice--failed" in body
+    # A failed generation is marked differently from real advice.
+    assert "advice-trigger--failed" in body
+    assert "Advice failed" in body
 
 
 def test_advice_is_not_generated_in_tests(client, repo):