Compare commits

..
1 Commits
Author SHA1 Message Date
hermes c7ca9c3801 feat(advice): LLM brewing advice on every observation (work in progress)
Stores a model's advice for each recorded reading, in its own field, so the
brewer's own comment is never overwritten.

Fire and forget: the advice is generated in a background thread, so
recording a reading never blocks on the model.  A failure is recorded as a
visible note saying the advice could not be generated, because a silent
failure is indistinguishable from "still thinking".

The model is `glm-5.3:cloud` on the Ollama instance at 192.168.178.45:11434.
Several models on that instance are retired and fail at call time while
still appearing in the model list, so the default is one that was probed.

The system prompt tells the model never to call a reading impossible: a
gravity below 1.000 is always a mis-entry, but the rest of the run may be a
genuine long fermentation and still worth understanding.

150 tests.
2026-11-09 15:52:26 +10:00
16 changed files with 203 additions and 2396 deletions
-25
View File
@@ -87,31 +87,6 @@ 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)
# ------------------------------------------------------------------ #
+22 -179
View File
@@ -25,7 +25,6 @@ from __future__ import annotations
import json
import logging
import os
import re
import urllib.error
import urllib.request
@@ -34,25 +33,14 @@ 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")
#: The model to use. Override with the ``OLLAMA_MODEL`` environment
#: variable.
#:
#: 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")
#: A model that is known to work. `deepseek-v4-flash:cloud`,
#: `deepseek-v3.2:cloud` and `qwen3.5:cloud` are all retired and fail at
#: call time, so they are not safe defaults.
DEFAULT_MODEL = os.getenv("OLLAMA_MODEL", "glm-5.3: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", "120"))
DEFAULT_TIMEOUT = float(os.getenv("LLM_TIMEOUT", "45"))
#: 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.
@@ -79,16 +67,6 @@ 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.
@@ -103,13 +81,8 @@ Write two to five sentences of plain prose.
"""
def _reading_lines(readings: list[dict], subject_id: str | None = None) -> str:
"""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.
"""
def _reading_lines(readings: list[dict]) -> str:
"""The readings, as the compact list the model reads."""
lines = []
for r in readings:
parts = [f"- {r.get('date')}: gravity {r.get('sg_decimal')} ({r.get('point_scale')})"]
@@ -117,19 +90,12 @@ def _reading_lines(readings: list[dict], subject_id: str | None = None) -> 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],
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.
"""
def build_prompt(run: dict, reading: dict, history: list[dict]) -> str:
"""The user-message content for one observation."""
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"):
@@ -147,23 +113,10 @@ def build_prompt(run: dict, reading: dict, history: list[dict],
parts.append("")
parts.append("Readings so far (oldest first):")
parts.append(_reading_lines(history, subject_id=reading.get("_id")))
parts.append(_reading_lines(history))
parts.append("")
# 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."
)
parts.append("Give advice for the most recent reading.")
return "\n".join(parts)
@@ -182,14 +135,11 @@ 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 as the answer. At 500 tokens the reasoning
# used the whole allowance and the reply came back empty - which
# looks like a broken model rather than a budget problem.
#
# `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,
# of the same budget. At 500 tokens the reasoning used the whole
# allowance and the reply came back empty - which looked like a
# broken model rather than a budget problem. 2000 leaves room for
# the reasoning AND the advice.
"max_tokens": 2000,
"stream": False,
}).encode("utf-8")
@@ -205,109 +155,14 @@ 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")
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 = (choices[0].get("message") or {}).get("content") or ""
content = content.strip()
if not content:
raise RuntimeError("the model returned an empty response")
# 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
return content[:MAX_ADVICE_CHARS]
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.
@@ -316,20 +171,9 @@ 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, corrected=corrected)
prompt = build_prompt(run, reading, history)
try:
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
return True, call_ollama(prompt, url=url, model=model, timeout=timeout)
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.
@@ -353,15 +197,14 @@ def generate_advice(run: dict, reading: dict, history: list[dict], *,
def generate_advice_in_background(run: dict, reading: dict,
history: list[dict], writer,
corrected: bool = False) -> None:
history: list[dict], writer) -> 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, corrected=corrected)
ok, text = generate_advice(run, reading, history)
try:
writer(reading.get("_id"), ok, text)
except Exception: # noqa: BLE001 - never let the thread die noisily
+8 -54
View File
@@ -156,10 +156,6 @@ 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:
@@ -181,7 +177,6 @@ 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 ""),
)
@@ -223,32 +218,14 @@ class Run:
"""Gravity points of every *usable* reading, in chronological order."""
return [r.sg_points for r in self.readings if r.sg_points is not None]
def dated_sg_values(self) -> list[int]:
"""Gravity points of the readings that have a date, chronologically.
An undated reading cannot be placed in the run's timeline, so it
must not be used as the original gravity: it sorts to the front
and would put the OG after the FG.
"""
dated = [r for r in self.readings
if r.sg_points is not None and r.date is not None]
return [r.sg_points for r in dated]
def original_gravity(self) -> int | None:
"""OG = the first *dated* usable reading of the run."""
values = self.dated_sg_values()
"""OG = the first usable reading of the run."""
values = self.sg_values()
return values[0] if values else None
def final_gravity(self) -> int | None:
"""FG = the last *dated* usable reading of the run (if it is finished).
For a finished run the final reading is the one flagged as final,
which is the reading the brewer took when they racked it.
"""
final = self.final_reading()
if final is not None and final.sg_points is not None:
return final.sg_points
values = self.dated_sg_values()
"""FG = the last usable reading of the run (if the run is finished)."""
values = self.sg_values()
return values[-1] if values else None
def max_sg(self) -> int | None:
@@ -539,11 +516,6 @@ def attenuation_percent(run: Run) -> float | None:
fg = run.final_gravity()
if og is None or fg is None or og <= 1000 or fg < 1000:
return None
if fg > og:
# The gravity went UP. That is not attenuation - it is either a
# mis-ordered pair of readings or a contaminated sample, and a
# negative attenuation is not a number a brewer can act on.
return None
attenuation = (og - fg) / (og - 1000) * 100
if attenuation > 100:
# Cannot ferment away more than the sugar that was there.
@@ -573,17 +545,6 @@ def implausible_runs(runs: Iterable[Run]) -> list[dict]:
"is probably wrong"
),
})
elif og is not None and fg is not None and fg > og:
problems.append({
"beer_run_id": run.beer_run_id,
"og_points": og,
"fg_points": fg,
"problem": (
f"the final gravity {fg / 1000:.3f} is higher than the "
f"original {og / 1000:.3f} — the gravity went up, which "
"is usually a mis-ordered pair of readings"
),
})
elif og is not None and fg is not None and og > 1000 and fg > 1000 \
and (og - fg) / (og - 1000) * 100 > 100:
problems.append({
@@ -601,18 +562,11 @@ def days_to_target(run: Run, target: int) -> int | None:
Used to answer "how long does this usually take?", which is the question
a brewer actually has when planning the next batch.
"""
first = run.first_reading()
if first is None or first.date is None:
# Without a start date there is no elapsed time to measure.
return None
# Skip the readings that cannot be placed on the timeline (no date)
# rather than giving up on the whole run because of one of them.
for reading in run.readings:
if reading.sg_points is None or reading.sg_points > target:
continue
if reading.date is None:
continue
if reading.sg_points is not None and reading.sg_points <= target:
first = run.first_reading()
if first is None or first.date is None or reading.date is None:
return None
return days_between(first.date, reading.date)
return None
+13 -196
View File
@@ -18,9 +18,7 @@ Two collections are used:
from __future__ import annotations
import logging
import re
import os
import threading
from typing import Any
from pymongo import MongoClient
@@ -48,33 +46,6 @@ 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.
@@ -209,21 +180,8 @@ 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)
@@ -231,105 +189,25 @@ class BeerRepository:
return str(existing.get("_id")) if existing else ""
def set_reading_advice(self, reading_id: str, advice: str,
ok: bool = True,
expected_sg: int | None = None,
expected_token: str | None = None,
expected_comment: str | None = None) -> int:
ok: bool = True) -> 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 not doc_id:
if doc_id is None:
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(
query,
{"_id": doc_id},
{"$set": {
"advice": str(advice),
"advice_ok": bool(ok),
"advice_at": _now_iso(),
}},
)
# `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
return result.modified_count
def set_final_reading(self, beer_run_id: str, reading_date: str,
final: bool) -> int:
@@ -457,59 +335,12 @@ 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 not a run id, so the ``_as_id`` coercion
does not apply - it would turn the id into an empty string and the
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.
The ``_id`` of a reading is an ``ObjectId``, not a run id, so the
``_as_id`` coercion does not apply - it would turn the ObjectId into
an empty string and the update would silently match nothing.
Only an ObjectId or a plain string is accepted, so a dict can never
be injected as a query operator.
@@ -517,20 +348,13 @@ def _as_doc_id(value: object):
from bson import ObjectId
if isinstance(value, ObjectId):
return [value]
if isinstance(value, str) and value:
return value
if isinstance(value, str):
try:
# A 24-character hex string may or may not be the stored
# ObjectId or the stored string, so try both.
return [ObjectId(value), value]
return ObjectId(value)
except Exception:
# Not hex - it can only be the original app's string id.
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 []
return None
return None
def _as_date(value: object) -> str:
@@ -555,13 +379,6 @@ 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
+18 -308
View File
@@ -451,10 +451,7 @@ input[type="date"]::-webkit-calendar-picker-indicator:hover { opacity: 1; }
/* ---------------------------------------------------------------- tables */
.table-wrap {
overflow-x: auto;
-webkit-overflow-scrolling: touch;
}
.table-wrap { overflow-x: auto; }
table.data {
width: 100%;
@@ -483,148 +480,6 @@ 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 {
@@ -740,18 +595,8 @@ 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
@@ -871,12 +716,6 @@ 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);
@@ -888,156 +727,27 @@ 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 {
position: relative;
width: 1%;
white-space: nowrap;
max-width: 32rem;
}
.advice-trigger {
display: inline-flex;
align-items: center;
gap: .35rem;
padding: .25rem .55rem;
border: 1px solid var(--amber);
border-radius: 999px;
background: var(--amber-dim);
color: var(--amber-text);
font: inherit;
/* In the table the advice is clamped to a few lines - the full text is
on the run detail page, where there is room to read it. */
.advice-cell .advice {
margin-top: 0;
font-size: .75rem;
line-height: 1.4;
cursor: pointer;
white-space: nowrap;
display: -webkit-box;
-webkit-line-clamp: 4;
line-clamp: 4;
-webkit-box-orient: vertical;
overflow: hidden;
}
.advice-trigger:hover,
.advice-trigger:focus-visible {
background: var(--amber);
color: var(--bg);
}
.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%);
}
/* The clamped text opens up on hover, so the whole thing is readable
without leaving the page. */
.advice-cell:hover .advice,
.advice-cell:focus-within .advice {
-webkit-line-clamp: unset;
line-clamp: unset;
overflow: visible;
}
-160
View File
@@ -56,171 +56,11 @@
}
}
/**
* 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") {
+12 -75
View File
@@ -179,21 +179,14 @@
var canvas = document.getElementById("abv-chart");
if (!canvas) { return; }
fetchRuns()
fetch("/api/runs", { headers: { "Accept": "application/json" } })
.then(function (r) { return r.json(); })
.then(function (runs) {
// 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.suspect &&
r.abv !== null && r.abv !== undefined;
return r.abv !== null && r.abv !== undefined;
});
if (!completed.length) { return; }
@@ -247,7 +240,8 @@
var canvas = document.getElementById("months-chart");
if (!canvas) { return; }
fetchRuns()
fetch("/api/runs", { headers: { "Accept": "application/json" } })
.then(function (r) { return r.json(); })
.then(function (runs) {
var counts = {};
runs.forEach(function (r) {
@@ -308,19 +302,14 @@
var canvas = document.getElementById("attenuation-chart");
if (!canvas) { return; }
fetchRuns()
fetch("/api/runs", { headers: { "Accept": "application/json" } })
.then(function (r) { return r.json(); })
.then(function (runs) {
// 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 && !r.active &&
return !r.suspect &&
r.attenuation !== null && r.attenuation !== undefined;
});
if (!done.length) { return; }
@@ -395,18 +384,14 @@
var canvas = document.getElementById("curve-chart");
if (!canvas) { return; }
fetchRuns()
fetch("/api/runs", { headers: { "Accept": "application/json" } })
.then(function (r) { return r.json(); })
.then(function (runs) {
// A run with an impossible reading has a broken curve - it
// plunges to a gravity below water - which would drag the
// median line down and stretch the envelope to cover it.
// Only FINISHED brews belong on the curve. An active run's
// curve is a partial fermentation, and the curve is meant to
// describe how a completed fermentation behaves - including
// a partial one pulls the median line off the real shape.
var withCurve = runs.filter(function (r) {
return !r.suspect && !r.active &&
r.curve && r.curve.length > 1;
return !r.suspect && r.curve && r.curve.length > 1;
});
if (!withCurve.length) { return; }
@@ -477,7 +462,6 @@
// progress point.
var envelope = [];
var median = [];
var envelopeMax = [];
for (var i = 0; i < samples; i++) {
var pct = (i / (samples - 1)) * 100;
var values = [];
@@ -494,7 +478,6 @@
y: values[0],
yMax: values[values.length - 1]
});
envelopeMax.push({ x: pct, y: values[values.length - 1] });
var mid = Math.floor(values.length / 2);
median.push({
x: pct,
@@ -509,12 +492,6 @@
data: {
datasets: [
{
// The band is drawn as TWO datasets: the min
// curve filled down to the max curve. With
// a single dataset and `fill: "+1"` the band
// was filled to the NEXT dataset - which was
// the median line, not the max - so the
// tooltip and the shading disagreed.
label: "Range (min–max)",
data: envelope,
borderColor: withAlpha(ACCENT, 0.35),
@@ -524,18 +501,6 @@
fill: "+1",
tension: 0.3
},
{
// The upper edge of the band. It is the
// dataset the one above fills to.
label: "Range max",
data: envelopeMax,
borderColor: withAlpha(ACCENT, 0.35),
backgroundColor: "transparent",
borderWidth: 1,
pointRadius: 0,
fill: false,
tension: 0.3
},
{
label: "Median brew",
data: median,
@@ -557,12 +522,7 @@
boxWidth: 10,
boxHeight: 10,
usePointStyle: true,
font: { size: 11 },
// The max curve is the helper the band
// fills to, not a series of its own.
filter: function (item, data) {
return item.text !== "Range max";
}
font: { size: 11 }
}
},
tooltip: {
@@ -573,9 +533,6 @@
return "range " + point.y +
" to " + (point.yMax || point.y);
}
if (ctx.datasetIndex === 1) {
return null;
}
return "median " + point.y;
}
}
@@ -612,26 +569,6 @@
.catch(function (err) { console.warn(err); });
}
/* ------------------------------------------------- shared run payload */
// The analysis page has two charts that both need /api/runs, and the
// payload contains every reading of every run. Fetch it once and
// share it rather than making two identical round-trips.
var runsPromise = null;
function fetchRuns() {
if (!runsPromise) {
runsPromise = fetch("/api/runs", { headers: { "Accept": "application/json" } })
.then(function (r) { return r.json(); })
.catch(function (err) {
console.warn(err);
runsPromise = null;
return [];
});
}
return runsPromise;
}
function init() {
drawSparklines();
drawRunChart();
+1 -1
View File
@@ -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&nbsp;reading{% else %} &middot; {{ run.reading_count }}&nbsp;readings{% endif %}
{% if run.reading_count == 1 %} &middot; 1 reading{% else %} &middot; {{ run.reading_count }} readings{% endif %}
</div>
</div>
{% if run.tap_number is not none %}
+27 -29
View File
@@ -99,43 +99,43 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="col-name">Name</th>
<th class="num col-gravity">OG</th>
<th class="num col-gravity">FG</th>
<th class="num col-abv">ABV</th>
<th class="num col-abv" title="Attenuation"><span class="long">Attenuation</span><span class="short">Att.</span></th>
<th class="num col-abv col-days">Days</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 class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
<th>Run</th>
<th>Name</th>
<th class="num">OG</th>
<th class="num">FG</th>
<th class="num">ABV</th>
<th class="num">Attenuation</th>
<th class="num">Days</th>
<th class="num">To 1.010</th>
<th>Finished</th>
</tr>
</thead>
<tbody>
{% for row in rows %}
<tr{% if row.suspect %} class="is-suspect"{% endif %}>
<td class="col-run mono">{{ row.beer_run_id }}</td>
<td class="col-name">{{ row.name }}{% if row.suspect %} <span class="chip chip--warn">not counted</span>{% endif %}</td>
<td class="col-gravity num">
<td class="mono">{{ row.beer_run_id }}</td>
<td>{{ row.name }}{% if row.suspect %} <span class="chip chip--warn">not counted</span>{% endif %}</td>
<td class="num">
{% if row.og_points is not none %}
{{ '%.3f' % (row.og_points / 1000) }}
{% else %}—{% endif %}
</td>
<td class="col-gravity num">
<td class="num">
{% if row.fg_points is not none %}
{{ '%.3f' % (row.fg_points / 1000) }}
{% else %}—{% endif %}
</td>
<td class="col-abv num">
<td class="num">
{% if row.abv is not none %}{{ '%.2f' % row.abv }}%{% else %}—{% endif %}
</td>
<td class="col-abv num">
<td class="num">
{% if row.attenuation is not none %}
<strong>{{ row.attenuation }}%</strong>
{% else %}—{% endif %}
</td>
<td class="col-abv col-days num">{{ row.days if row.days 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="col-finished mono nowrap">{{ row.final_date or '—' }}</td>
<td class="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="mono nowrap">{{ row.final_date or '—' }}</td>
</tr>
{% endfor %}
</tbody>
@@ -155,21 +155,19 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="num col-gravity">OG</th>
<th class="num col-gravity">FG</th>
<th class="col-comment">Problem</th>
<th>Run</th>
<th class="num">OG</th>
<th class="num">FG</th>
<th>Problem</th>
</tr>
</thead>
<tbody>
{% for row in suspect %}
<tr>
<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="col-gravity num">{{ '%.3f' % (row.og_points / 1000) }}</td>
<td class="col-gravity num"><span class="chip chip--warn">{{ '%.3f' % (row.fg_points / 1000) }}</span></td>
<td class="col-comment comment">{{ row.problem }}</td>
<td class="mono">{{ row.beer_run_id }}</td>
<td class="num">{{ '%.3f' % (row.og_points / 1000) }}</td>
<td class="num"><span class="chip chip--warn">{{ '%.3f' % (row.fg_points / 1000) }}</span></td>
<td class="comment">{{ row.problem }}</td>
</tr>
{% endfor %}
</tbody>
@@ -186,7 +184,7 @@
</div>
<div class="card__body">
<p>
Across {% if stats.suspect_count %}{{ stats.counted_attenuation }} of {{ stats.count }}{% else %}{{ stats.count }}{% endif %} finished brews the attenuation averages
Across {% if stats.suspect_count %}{{ stats.trusted_count }} 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 }}
+32 -51
View File
@@ -78,67 +78,48 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="col-name">Name</th>
<th class="num col-gravity">Gravity</th>
<th class="num col-abv" title="Estimated alcohol by volume"><span class="long">Est. ABV</span><span class="short">ABV</span></th>
<th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
<th class="col-advice" title="Brewing advice"><span class="long">Brewing advice</span><span class="short">Advice</span></th>
<th class="col-action"></th>
<th>Run</th>
<th>Name</th>
<th class="num">Gravity</th>
<th class="num">Est. ABV</th>
<th>Comment</th>
<th>Brewing advice</th>
<th></th>
</tr>
</thead>
<tbody>
{% for run in active_runs %}
<tr>
<td class="mono col-run">{{ run.beer_run_id }}</td>
<td class="col-name">{{ run.canonical_name }}</td>
<td class="num col-gravity">
<td class="mono">{{ run.beer_run_id }}</td>
<td>{{ run.canonical_name }}</td>
<td class="num">
{% if run.min_sg is not none %}
<span class="mono strong">{{ run.min_sg_decimal }}</span>
<div class="faint tiny point-scale">{{ run.min_sg_point_scale }} points</div>
<div class="faint tiny">{{ run.min_sg_point_scale }} pts</div>
{% else %}—{% endif %}
</td>
<td class="num col-abv">
<td class="num">
{% 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 col-comment">
<td class="comment">
{% if run.comments %}
{{ run.comments[-1] }}
{% else %}
<span class="faint">—</span>
{% endif %}
</td>
<td class="advice-cell col-advice">
<td class="comment advice-cell">
{% if run.advice %}
<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>
<span class="advice{% if not run.advice_ok %} advice--failed{% endif %}">{{ run.advice }}</span>
{% else %}
<span class="faint">Thinking…</span>
{% endif %}
</td>
<td class="right col-action">
<td class="right">
<a class="btn btn--ghost btn--sm"
href="{{ url_for('brews.run_detail', run_id=run.beer_run_id) }}">Open</a>
</td>
@@ -167,41 +148,41 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="col-name">Name</th>
<th class="num col-gravity">OG</th>
<th class="num col-gravity">FG</th>
<th class="num col-abv">ABV</th>
<th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
<th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
<th class="col-action"></th>
<th>Run</th>
<th>Name</th>
<th class="num">OG</th>
<th class="num">FG</th>
<th class="num">ABV</th>
<th>Finished</th>
<th>Comment</th>
<th></th>
</tr>
</thead>
<tbody>
{% for run in completed_runs %}
<tr>
<td class="mono col-run">{{ run.beer_run_id }}</td>
<td class="col-name">{{ run.canonical_name }}</td>
<td class="num col-gravity">
<td class="mono">{{ run.beer_run_id }}</td>
<td>{{ run.canonical_name }}</td>
<td class="num">
{% if run.og_points is not none %}{{ run.og_points }}{% else %}—{% endif %}
</td>
<td class="num col-gravity">
<td class="num">
{% if run.fg_points is not none %}{{ run.fg_points }}{% else %}—{% endif %}
</td>
<td class="num col-abv">
<td class="num">
{% if run.abv is not none %}
<strong>{{ '%.2f' % run.abv }}%</strong>
{% else %}—{% endif %}
</td>
<td class="mono nowrap col-finished">{{ run.final_date or '—' }}</td>
<td class="comment col-comment">
<td class="mono nowrap">{{ run.final_date or '—' }}</td>
<td class="comment">
{% if run.comments %}
{{ run.comments[-1] }}
{% else %}
<span class="faint">—</span>
{% endif %}
</td>
<td class="right col-action">
<td class="right">
<a class="btn btn--ghost btn--sm"
href="{{ url_for('brews.run_detail', run_id=run.beer_run_id) }}">Open</a>
</td>
+45 -51
View File
@@ -23,13 +23,7 @@
<div class="stat">
<div class="stat__label">Average ABV</div>
<div class="stat__value">{{ '%.2f' % stats.avg_abv }}%</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 class="stat__meta">across finished brews</div>
</div>
<div class="stat">
<div class="stat__label">Strongest</div>
@@ -84,21 +78,21 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="col-name">Name</th>
<th class="col-finished">Date</th>
<th class="num col-gravity">Reading</th>
<th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
<th>Run</th>
<th>Name</th>
<th>Date</th>
<th class="num">Reading</th>
<th>Comment</th>
</tr>
</thead>
<tbody>
{% for row in outliers %}
<tr>
<td class="col-run mono">{{ row.beer_run_id }}</td>
<td class="col-name">{{ row.name }}</td>
<td class="col-finished mono nowrap">{{ row.date or '—' }}</td>
<td class="col-gravity num"><span class="chip chip--warn">{{ row.sg }}</span></td>
<td class="col-comment comment">{{ row.comment or '—' }}</td>
<td class="mono">{{ row.beer_run_id }}</td>
<td>{{ row.name }}</td>
<td class="mono nowrap">{{ row.date or '—' }}</td>
<td class="num"><span class="chip chip--warn">{{ row.sg }}</span></td>
<td class="comment">{{ row.comment or '—' }}</td>
</tr>
{% endfor %}
</tbody>
@@ -118,45 +112,45 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="col-name">Name</th>
<th class="col-finished" title="Status"><span class="long">Status</span><span class="short">Stat</span></th>
<th class="num col-gravity">OG</th>
<th class="num col-gravity">FG</th>
<th class="num col-drop">Drop</th>
<th class="num col-abv">ABV</th>
<th class="num col-legacy">Legacy</th>
<th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
<th class="col-comment" title="Comment"><span class="long">Comment</span><span class="short">Note</span></th>
<th class="col-action"></th>
<th>Run</th>
<th>Name</th>
<th>Status</th>
<th class="num">OG</th>
<th class="num">FG</th>
<th class="num">Drop</th>
<th class="num">ABV</th>
<th class="num">Legacy</th>
<th>Finished</th>
<th>Comment</th>
<th></th>
</tr>
</thead>
<tbody>
{% for run in summaries %}
<tr>
<td class="col-run mono">{{ run.beer_run_id }}</td>
<td class="col-name">{{ run.canonical_name }}</td>
<td class="col-finished">
<td class="mono">{{ run.beer_run_id }}</td>
<td>{{ run.canonical_name }}</td>
<td>
{% if run.active %}
<span class="chip chip--active">Active</span>
{% else %}
<span class="chip chip--done">Finished</span>
{% endif %}
</td>
<td class="col-drop col-gravity num">{{ run.og_points or '—' }}</td>
<td class="col-gravity num">{{ run.fg_points or '—' }}</td>
<td class="col-gravity num">{{ run.drop_points if run.drop_points is not none else '—' }}</td>
<td class="col-abv num">
<td class="num">{{ run.og_points or '—' }}</td>
<td class="num">{{ run.fg_points or '—' }}</td>
<td class="num">{{ run.drop_points if run.drop_points is not none else '—' }}</td>
<td class="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="col-legacy col-abv num muted">{{ run.legacy_abv if run.legacy_abv is not none else '—' }}</td>
<td class="col-finished mono nowrap">{{ run.final_date or '—' }}</td>
<td class="col-comment comment">
<td class="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="comment">
{% if run.comments %}{{ run.comments[-1] }}{% else %}<span class="faint">—</span>{% endif %}
</td>
<td class="right col-action">
<td class="right">
<a class="btn btn--ghost btn--sm"
href="{{ url_for('brews.run_detail', run_id=run.beer_run_id) }}">Open</a>
</td>
@@ -178,29 +172,29 @@
<table class="data">
<thead>
<tr>
<th class="col-run">Run</th>
<th class="col-name">Name</th>
<th class="num col-abv" title="Standard ABV"><span class="long">Standard ABV</span><span class="short">Std</span></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 col-abv" title="Overstated by"><span class="long">Overstated by</span><span class="short">Over</span></th>
<th class="col-finished" title="Finished"><span class="long">Finished</span><span class="short">Done</span></th>
<th>Run</th>
<th>Name</th>
<th class="num">Standard ABV</th>
<th class="num">Legacy ABV</th>
<th class="num">Overstated by</th>
<th>Finished</th>
</tr>
</thead>
<tbody>
{% for row in abv_rows %}
<tr>
<td class="col-run mono">{{ row.beer_run_id }}</td>
<td class="col-name">{{ row.name }}</td>
<td class="col-abv num">{{ '%.2f' % row.abv }}%</td>
<td class="col-abv num muted">{{ '%.2f' % row.legacy_abv }}%</td>
<td class="col-abv num">
<td class="mono">{{ row.beer_run_id }}</td>
<td>{{ row.name }}</td>
<td class="num">{{ '%.2f' % row.abv }}%</td>
<td class="num muted">{{ '%.2f' % row.legacy_abv }}%</td>
<td class="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="col-finished mono nowrap">{{ row.final_date or '—' }}</td>
<td class="mono nowrap">{{ row.final_date or '—' }}</td>
</tr>
{% endfor %}
</tbody>
+5 -17
View File
@@ -55,15 +55,6 @@ 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, {})
@@ -79,6 +70,7 @@ def deep():
"attenuation": attenuation_percent(run),
"days": summary["days_in_fermenter"],
"days_to_1010": days_to_target(run, 1010),
"days_to_1015": days_to_target(run, 1015),
"readings": summary["reading_count"],
"curve": summary["curve"],
# A suspect run is shown in the table so it can be corrected,
@@ -86,6 +78,9 @@ def deep():
"suspect": run.beer_run_id in suspect_ids,
})
# The rows with a usable curve, for the overlay chart.
curves = [r for r in rows if r["curve"]]
# Only the trustworthy runs feed the statistics.
good = [r for r in rows if not r["suspect"]]
@@ -114,16 +109,13 @@ def deep():
return render_template(
"analysis.html",
rows=rows,
curves=curves,
suspect=suspect,
stats={
"count": len(rows),
"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,
@@ -291,9 +283,5 @@ 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),
},
)
+8 -95
View File
@@ -53,15 +53,14 @@ 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, corrected: bool = False) -> None:
def _start_advice(run_id: str, reading_id: str) -> 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 uses the shared
process-wide one.
client is closed when the request ends - so it builds its own.
"""
if not reading_id:
return
@@ -90,8 +89,7 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
for reading in run.readings:
points = reading.sg_points
readings.append({
"_id": str(reading._id),
"advice_token": reading.advice_token,
"_id": reading._id,
"date": reading.date.isoformat() if reading.date else "",
"sg": points,
"sg_decimal": reading.sg_decimal,
@@ -100,75 +98,20 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
"final": bool(reading.final_reading),
})
# 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 = dict(readings[-1]) if readings else {"date": "", "sg": None}
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:
# The client is SHARED, not built per reading. A MongoClient
# 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")
from mongo.repository import BeerRepository
BeerRepository().set_reading_advice(read_id, text, ok)
thread = threading.Thread(
target=brew_advice.generate_advice_in_background,
args=(run_info, latest, readings, write, corrected),
args=(run_info, latest, readings, write),
daemon=True,
)
thread.start()
@@ -189,10 +132,6 @@ class ReadingForm(FlaskForm):
beer_run_type = StringField("Beer / cider name",
validators=[Optional(), Length(max=120)])
Date = DateField("Reading date", validators=[DataRequired()])
# The floor is deliberately lower than is_plausible_sg's. An
# out-of-range reading is SAVED with a warning rather than rejected:
# the brewer may have mis-typed it, and seeing it recorded is how they
# spot and correct the mistake.
sg = IntegerField("Gravity reading (points)",
validators=[DataRequired(), NumberRange(min=900, max=1500)])
final_reading = BooleanField("This is the final reading")
@@ -226,33 +165,12 @@ def index():
if form.validate_on_submit():
sg_points = int(form.sg.data)
if not is_plausible_sg(sg_points):
if sg_points < 1000:
# Beer cannot be less dense than water, so this is always
# a mis-entry - and a mis-typed "1.012" as "012" is the
# likely cause. It is still saved so the mistake is
# visible and can be corrected.
flash(
f"{sg_points} is below 1.000 — beer cannot be less dense "
"than water, so this is almost certainly a typo (a 1.012 "
"reading entered as 012?). It was saved anyway; correct "
"it on the run's page.",
"warning",
)
else:
flash(
f"{sg_points} is outside the usual hydrometer range "
"(990-1200). It was saved anyway — double-check the reading.",
"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(),
@@ -266,7 +184,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, corrected=was_existing)
_start_advice(run_id, reading_id)
return redirect(url_for("brews.index"))
runs = repo().runs()
@@ -352,10 +270,6 @@ 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 = []
@@ -386,7 +300,6 @@ 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)
+1 -235
View File
@@ -8,7 +8,7 @@ from pathlib import Path
import pytest
from brewing import (Run, attenuation_percent, days_to_target,
fermentation_curve, implausible_runs)
fermentation_curve)
from conftest import seed_readings
@@ -392,237 +392,3 @@ def test_the_charts_skip_suspect_runs():
assert charts_js.count("!r.suspect") >= 2, (
"the charts must filter out the runs flagged as suspect"
)
# ------------------------------------------------- rising gravity #
def test_attenuation_is_none_when_the_gravity_went_up():
"""A negative attenuation is not a number a brewer can act on.
The guard rejected a final gravity below 1.000 and one implying more
than 100% attenuation, but a run whose gravity ROSE produced a
negative figure that was averaged into the headline number.
"""
run = Run.from_docs("30", [
_doc(sg="1010", date_="2026-01-01"),
_doc(sg="1020", date_="2026-01-20", final="True"),
])
assert attenuation_percent(run) is None
def test_a_rising_gravity_run_is_reported_as_implausible():
"""It must be listed for review, not just quietly dropped."""
run = Run.from_docs("31", [
_doc(sg="1010", date_="2026-01-01"),
_doc(sg="1020", date_="2026-01-20", final="True"),
])
problems = implausible_runs([run])
assert len(problems) == 1
assert problems[0]["beer_run_id"] == "31"
assert "went up" in problems[0]["problem"]
# ------------------------------------------------- undated readings #
def test_the_original_gravity_comes_from_the_first_dated_reading():
"""An undated reading cannot be placed on the timeline.
It used to sort to the front, so it became the OG - which put the OG
after the FG and produced a negative attenuation.
"""
run = Run.from_docs("32", [
_doc(sg="1050", date_="2026-01-01"),
{"_id": "x", "beer_run_id": "32", "beer_type": "Ale", "sg": "1005",
"date": None, "final_reading": "False", "comment": ""},
_doc(sg="1010", date_="2026-01-25", final="True"),
])
assert run.original_gravity() == 1050
assert run.final_gravity() == 1010
assert attenuation_percent(run) is not None
assert attenuation_percent(run) > 0
def test_days_to_target_skips_an_undated_reading():
"""One undated reading must not lose the whole figure.
The function gave up and returned None when the first reading at or
below the target had no date, even when a later reading did.
"""
run = Run.from_docs("33", [
_doc(sg="1050", date_="2026-01-01"),
{"_id": "y", "beer_run_id": "33", "beer_type": "Ale", "sg": "1005",
"date": None, "final_reading": "False", "comment": ""},
_doc(sg="1010", date_="2026-01-25", final="True"),
])
assert days_to_target(run, 1010) == 24
# ------------------------------------------------------- the charts #
def test_the_curve_chart_excludes_active_and_suspect_runs():
"""Only finished, plausible brews belong on the curve.
An active run's curve is a partial fermentation, and the curve is
meant to describe how a completed fermentation behaves.
"""
charts_js = (
Path(__file__).resolve().parents[1]
/ "src" / "flask" / "static" / "js" / "charts.js"
).read_text()
assert "!r.active" in charts_js, (
"the curve chart must leave the active runs out"
)
def test_the_curve_band_is_filled_to_the_max_not_the_median():
"""The shaded band has to span min to max.
With one dataset and `fill: "+1"` the band was filled to the NEXT
dataset - which was the median line, so the upper half of the range
was drawn as a hairline but never shaded, and the tooltip disagreed
with the shading.
"""
charts_js = (
Path(__file__).resolve().parents[1]
/ "src" / "flask" / "static" / "js" / "charts.js"
).read_text()
assert "envelopeMax" in charts_js, (
"the band needs a separate max dataset to fill down to"
)
assert 'label: "Range max"' in charts_js
def test_the_charts_fetch_the_run_payload_once():
"""/api/runs carries every reading of every run.
Four charts each fetched it independently, so the analysis page made
two identical round-trips of the whole collection.
"""
charts_js = (
Path(__file__).resolve().parents[1]
/ "src" / "flask" / "static" / "js" / "charts.js"
).read_text()
assert charts_js.count('fetch("/api/runs"') == 1, (
"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
+1 -37
View File
@@ -186,16 +186,11 @@ def test_logout_clears_the_session(logged_in):
def test_out_of_range_reading_is_saved_but_warned(logged_in, repo):
"""A reading outside the range is SAVED with a warning.
Rejecting it would lose the brewer's actual observation, and seeing it
recorded is how they spot and correct a typo.
"""
response = logged_in.post("/brews", data={
"beer_run_id": "12",
"beer_run_type": "Odd",
"Date": "2026-05-01",
"sg": "1250",
"sg": "950",
}, follow_redirects=True)
assert response.status_code == 200
assert b"outside the usual hydrometer range" in response.data
@@ -203,25 +198,6 @@ def test_out_of_range_reading_is_saved_but_warned(logged_in, repo):
assert repo.readings.find_one({"beer_run_id": "12"}) is not None
def test_a_reading_below_water_says_it_is_always_a_typo(logged_in, repo):
"""A gravity below 1.000 cannot be real, so the warning says so.
This is the run 12 case: a 1.012 reading entered as "012". The
reading is still saved, but the message names the likely cause.
"""
response = logged_in.post("/brews", data={
"beer_run_id": "13",
"beer_run_type": "Typo",
"Date": "2026-05-01",
"sg": "900",
}, follow_redirects=True)
assert response.status_code == 200
assert b"below 1.000" in response.data
assert b"typo" in response.data
# Still saved, so the mistake is visible and can be corrected.
assert repo.readings.find_one({"beer_run_id": "13"}) is not None
# ------------------------------------------------------- security: passwords #
def test_legacy_plaintext_password_is_accepted_and_migrated(repo):
@@ -714,15 +690,3 @@ 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", "")
+4 -877
View File
@@ -3,7 +3,6 @@
from __future__ import annotations
import json
from contextlib import contextmanager
from datetime import date
import pytest
@@ -56,46 +55,6 @@ 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",
@@ -117,7 +76,7 @@ def test_a_successful_call_returns_the_text(monkeypatch):
class Response:
def read(self):
return json.dumps({
"choices": [{"message": {"content": "ADVICE: Leave it alone, it is still fermenting."}}]
"choices": [{"message": {"content": " Leave it alone, it is still fermenting. "}}]
}).encode()
def __enter__(self):
@@ -220,128 +179,12 @@ def test_the_token_budget_leaves_room_for_the_reasoning(monkeypatch):
brew_advice.generate_advice({"canonical_name": "Ale"},
{"date": "2026-01-01"}, [])
# 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
assert captured["body"]["max_tokens"] >= 2000
def test_the_advice_is_truncated():
"""A runaway reply must not fill the reading document."""
long_text = "ADVICE: " + ("x" * 5000)
long_text = "x" * 5000
class Response:
def read(self):
@@ -382,530 +225,6 @@ 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, "")])
@@ -995,196 +314,6 @@ 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"})
@@ -1198,9 +327,7 @@ def test_a_failed_advice_is_marked_as_such(client, repo):
response = client.get("/brews")
body = response.get_data(as_text=True)
# A failed generation is marked differently from real advice.
assert "advice-trigger--failed" in body
assert "Advice failed" in body
assert "advice--failed" in body
def test_advice_is_not_generated_in_tests(client, repo):