hermes
d18d037bb9
Two findings from the fifth review.
The pre-token fallback re-opened the hole
-----------------------------------------
A reading that has never been edited has no token. The guard fell back
to the gravity check for those, and a comment-only correction leaves the
gravity unchanged - so a late thread's write matched and overwrote the
fresh advice, for exactly the 47 existing readings.
Reproduced live before the fix:
pre-token reading token: None -> stale thread carries ""
fresh write modified: 1
stale write modified: 1 <<< stale advice overwrote the fresh one
final advice: 'STALE ADVICE'
The guard now matches on the ABSENCE of a token, which still tells the
generations apart: an edit writes a token, so a stale thread's "" stops
matching the moment the reading is corrected.
fresh write modified: 1 (must be 1)
stale write modified: 0 (must be 0)
The stripper could empty real advice
------------------------------------
"Actually the drop is fine.\\n\\nWait a couple of days and check again."
is advice, and judging it paragraph by paragraph emptied it - so the
brewer saw nothing at all, with no failure note.
The test is no longer a marker list, which real advice often does not
contain. A reply is all-working when it is LONG and has no instruction
in it: the model's working runs to thousands of characters, while the
advice is two to five sentences.
short reply with no marker -> kept
long reply, no instruction -> dropped
long reply with advice -> advice kept
single-paragraph working -> dropped
Also: the log message named the gravity, but the guard is the token.
182 tests (up from 178).
403 lines
15 KiB
Python
403 lines
15 KiB
Python
"""The brew-process pages: record readings and review the runs."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
import os
|
|
import threading
|
|
|
|
from flask import (Blueprint, current_app, flash, jsonify, redirect,
|
|
render_template, request, session, url_for)
|
|
from flask_wtf import FlaskForm
|
|
from wtforms import (BooleanField, DateField, IntegerField, StringField,
|
|
SubmitField, TextAreaField)
|
|
from wtforms.validators import DataRequired, Length, NumberRange, Optional
|
|
|
|
import brew_advice
|
|
from brewing import implausible_runs, is_plausible_sg, run_summary
|
|
|
|
log = logging.getLogger(__name__)
|
|
|
|
bp = Blueprint("brews", __name__)
|
|
|
|
|
|
def repo():
|
|
"""The request-scoped repository (one Mongo client per request)."""
|
|
from flask import current_app
|
|
|
|
return current_app.extensions["beer_repo"]()
|
|
|
|
|
|
def login_required(fn):
|
|
"""Redirect to the login page when there is no session."""
|
|
from functools import wraps
|
|
|
|
@wraps(fn)
|
|
def wrapper(*args, **kwargs):
|
|
if not session.get("logged_in"):
|
|
return redirect(url_for("auth.login", next=request.path))
|
|
return fn(*args, **kwargs)
|
|
|
|
return wrapper
|
|
|
|
|
|
def advice_enabled() -> bool:
|
|
"""Whether the LLM advice should be generated.
|
|
|
|
It is off in testing, so the suite never reaches out to a model, and
|
|
it can be turned off in production with ``LLM_ADVICE=0`` if the model
|
|
is misbehaving.
|
|
"""
|
|
if current_app.config.get("TESTING"):
|
|
return False
|
|
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:
|
|
"""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.
|
|
"""
|
|
if not reading_id:
|
|
return
|
|
if not advice_enabled():
|
|
return
|
|
|
|
try:
|
|
repository = repo()
|
|
run = repository.run(run_id)
|
|
if run is None:
|
|
return
|
|
meta = repository.run_metadata(run_id)
|
|
summary = run_summary(run, og_override=meta.get("og_points"))
|
|
run_info = {
|
|
"canonical_name": meta.get("name") or run.display_name(),
|
|
"name": run.display_name(),
|
|
"og_points": summary.get("og_points"),
|
|
"og_decimal": summary.get("og_decimal"),
|
|
"tap_number": meta.get("tap_number"),
|
|
"notes": meta.get("notes", ""),
|
|
}
|
|
|
|
# The model's view of the run: every reading, with the gravity in
|
|
# both forms a brewer would recognise.
|
|
readings = []
|
|
for reading in run.readings:
|
|
points = reading.sg_points
|
|
readings.append({
|
|
"_id": str(reading._id),
|
|
"advice_token": reading.advice_token,
|
|
"date": reading.date.isoformat() if reading.date else "",
|
|
"sg": points,
|
|
"sg_decimal": reading.sg_decimal,
|
|
"point_scale": f"{points} points" if points is not None else "",
|
|
"comment": reading.comment or "",
|
|
"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:
|
|
saved = dict(readings[-1]) if readings else {"date": "", "sg": None}
|
|
latest = dict(saved)
|
|
latest["_id"] = reading_id
|
|
|
|
except Exception: # noqa: BLE001 - advice must never break the save
|
|
log.exception("could not gather the reading for the advice")
|
|
return
|
|
|
|
# The gravity and the editing token of the reading being advised
|
|
# about. The write is conditional on both, so a slow advice thread
|
|
# cannot overwrite a correction with advice about the old value.
|
|
advice_sg = latest.get("sg")
|
|
advice_token = latest.get("advice_token")
|
|
|
|
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,
|
|
)
|
|
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 the "
|
|
"advice no longer matches it",
|
|
read_id,
|
|
)
|
|
except Exception: # noqa: BLE001 - never let the thread die noisily
|
|
log.exception("could not store the generated advice")
|
|
|
|
thread = threading.Thread(
|
|
target=brew_advice.generate_advice_in_background,
|
|
args=(run_info, latest, readings, write, corrected),
|
|
daemon=True,
|
|
)
|
|
thread.start()
|
|
|
|
|
|
def _last_reading_date(run) -> str | None:
|
|
"""The ISO date of the most recent reading in a run."""
|
|
last = run.last_reading()
|
|
if last is None or last.date is None:
|
|
return None
|
|
return last.date.isoformat()
|
|
|
|
|
|
class ReadingForm(FlaskForm):
|
|
"""Add or update a gravity reading."""
|
|
|
|
beer_run_id = IntegerField("Run number", validators=[DataRequired()])
|
|
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")
|
|
comment = TextAreaField("Comment", validators=[Optional(), Length(max=2000)])
|
|
submit = SubmitField("Save reading")
|
|
|
|
|
|
class RunMetaForm(FlaskForm):
|
|
"""Edit the canonical metadata of a run.
|
|
|
|
These are the details that cannot be derived from the readings: the name
|
|
the brew is actually known by, which tap it is on, and its original
|
|
gravity.
|
|
"""
|
|
|
|
name = StringField("Beer / cider name", validators=[Optional(), Length(max=120)])
|
|
tap_number = IntegerField("Tap number",
|
|
validators=[Optional(), NumberRange(min=1, max=99)])
|
|
og_points = IntegerField("Original gravity (points)",
|
|
validators=[Optional(), NumberRange(min=900, max=1500)])
|
|
notes = TextAreaField("Brew notes", validators=[Optional(), Length(max=4000)])
|
|
submit = SubmitField("Save run")
|
|
|
|
|
|
@bp.route("/brews", methods=["GET", "POST"])
|
|
@login_required
|
|
def index():
|
|
"""The brew-process page: record a reading, then review the runs."""
|
|
form = ReadingForm()
|
|
|
|
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(),
|
|
sg=sg_points,
|
|
reading_date=form.Date.data.isoformat(),
|
|
final_reading=bool(form.final_reading.data),
|
|
comment=(form.comment.data or "").strip(),
|
|
)
|
|
flash("Reading saved.", "success")
|
|
|
|
# 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)
|
|
return redirect(url_for("brews.index"))
|
|
|
|
runs = repo().runs()
|
|
metadata = repo().all_run_metadata()
|
|
|
|
summaries = []
|
|
for run in runs:
|
|
meta = metadata.get(run.beer_run_id, {})
|
|
summary = run_summary(run, og_override=meta.get("og_points"))
|
|
summary["canonical_name"] = meta.get("name") or run.display_name()
|
|
summary["tap_number"] = meta.get("tap_number")
|
|
summary["notes"] = meta.get("notes", "")
|
|
summary["last_date"] = _last_reading_date(run)
|
|
summaries.append(summary)
|
|
|
|
# Active runs: most recent activity first — that is what is fermenting now.
|
|
active = sorted([s for s in summaries if s["active"]],
|
|
key=lambda s: s["last_date"] or "", reverse=True)
|
|
# Finished runs: most recently finished first.
|
|
completed = sorted([s for s in summaries if not s["active"]],
|
|
key=lambda s: s["final_date"] or "", reverse=True)
|
|
|
|
return render_template(
|
|
"brews.html",
|
|
form=form,
|
|
summaries=summaries,
|
|
active_runs=active,
|
|
completed_runs=completed,
|
|
next_tap=repo().next_free_tap_number(),
|
|
)
|
|
|
|
|
|
@bp.route("/runs/<run_id>", methods=["GET", "POST"])
|
|
@login_required
|
|
def run_detail(run_id: str):
|
|
"""One run in detail, with its readings and its editable metadata."""
|
|
run = repo().run(run_id)
|
|
if run is None:
|
|
flash(f"No run numbered {run_id}.", "error")
|
|
return redirect(url_for("brews.index"))
|
|
|
|
meta = repo().run_metadata(run_id)
|
|
meta_form = RunMetaForm()
|
|
|
|
if meta_form.validate_on_submit():
|
|
repo().upsert_run_metadata(
|
|
run_id,
|
|
name=(meta_form.name.data or "").strip() or None,
|
|
tap_number=meta_form.tap_number.data,
|
|
og_points=meta_form.og_points.data,
|
|
notes=(meta_form.notes.data or "").strip() or None,
|
|
)
|
|
flash("Run details saved.", "success")
|
|
return redirect(url_for("brews.run_detail", run_id=run_id))
|
|
|
|
if not meta_form.is_submitted():
|
|
meta_form.name.data = meta.get("name") or run.display_name()
|
|
meta_form.tap_number.data = meta.get("tap_number")
|
|
meta_form.og_points.data = meta.get("og_points")
|
|
meta_form.notes.data = meta.get("notes", "")
|
|
|
|
summary = run_summary(run, og_override=meta.get("og_points"))
|
|
summary["canonical_name"] = meta.get("name") or run.display_name()
|
|
summary["tap_number"] = meta.get("tap_number")
|
|
summary["notes"] = meta.get("notes", "")
|
|
|
|
return render_template("run_detail.html", run=run, summary=summary,
|
|
meta_form=meta_form, meta=meta)
|
|
|
|
|
|
@bp.route("/api/runs")
|
|
@login_required
|
|
def api_runs():
|
|
"""JSON feed of the run summaries, used by the charts on the analysis
|
|
pages.
|
|
|
|
The charts fetch this rather than having the data inlined into the page,
|
|
so one fetch serves every chart on a page.
|
|
"""
|
|
runs = repo().runs()
|
|
metadata = repo().all_run_metadata()
|
|
|
|
# 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.
|
|
suspect_ids = {s["beer_run_id"] for s in implausible_runs(runs)}
|
|
|
|
payload = []
|
|
for run in runs:
|
|
meta = metadata.get(run.beer_run_id, {})
|
|
summary = run_summary(run, og_override=meta.get("og_points"))
|
|
summary["canonical_name"] = meta.get("name") or run.display_name()
|
|
summary["tap_number"] = meta.get("tap_number")
|
|
summary["last_date"] = _last_reading_date(run)
|
|
summary["suspect"] = run.beer_run_id in suspect_ids
|
|
payload.append(summary)
|
|
|
|
return jsonify(payload)
|
|
|
|
|
|
@bp.route("/api/runs/<run_id>")
|
|
@login_required
|
|
def api_run(run_id: str):
|
|
"""JSON detail of a single run."""
|
|
run = repo().run(run_id)
|
|
if run is None:
|
|
return jsonify({"error": "not found"}), 404
|
|
|
|
meta = repo().run_metadata(run_id)
|
|
summary = run_summary(run, og_override=meta.get("og_points"))
|
|
summary["canonical_name"] = meta.get("name") or run.display_name()
|
|
summary["tap_number"] = meta.get("tap_number")
|
|
# 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]))
|
|
return jsonify(summary)
|
|
|
|
|
|
@bp.route("/runs/<run_id>/final", methods=["POST"])
|
|
@login_required
|
|
def mark_final(run_id: str):
|
|
"""Flag a reading as the final one for its run."""
|
|
reading_date = request.form.get("date", "")
|
|
if not reading_date:
|
|
flash("No reading date supplied.", "error")
|
|
return redirect(url_for("brews.run_detail", run_id=run_id))
|
|
|
|
changed = repo().set_final_reading(run_id, reading_date, True)
|
|
if changed:
|
|
flash("Final reading recorded.", "success")
|
|
else:
|
|
flash("That reading could not be updated.", "error")
|
|
return redirect(url_for("brews.run_detail", run_id=run_id))
|
|
|
|
|
|
@bp.route("/runs/<run_id>/unfinal", methods=["POST"])
|
|
@login_required
|
|
def unfinal(run_id: str):
|
|
"""Un-flag a final reading, putting the run back into the active list."""
|
|
reading_date = request.form.get("date", "")
|
|
if not reading_date:
|
|
flash("No reading date supplied.", "error")
|
|
return redirect(url_for("brews.run_detail", run_id=run_id))
|
|
|
|
repo().set_final_reading(run_id, reading_date, False)
|
|
flash("Reading is no longer marked final.", "info")
|
|
return redirect(url_for("brews.run_detail", run_id=run_id))
|