From e1a9a7384fc1907b5861ce3cb4a3d55838a26f39 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 13:11:23 +1000 Subject: [PATCH 1/4] feat(analysis): attenuation, fermentation curves and brew timings Adds the analysis page the old app never had: how consistently the brews ferment, and how long each one takes. What it shows ------------- - **Attenuation by brew** - what fraction of the original gravity was fermented away, for every finished brew. - **Fermentation curves** - the typical shape of a fermentation, drawn as a median curve with the min-max envelope behind it. Plotting every brew individually gave 16 near-identical lines and an unreadable legend. - **Every finished brew** - OG, FG, ABV, attenuation, days in the fermenter, days to reach 1.010, and the finish date. - **Readings that need a second look** - readings outside the plausible hydrometer range. - Headline figures: average attenuation and its spread, average brew time, average time to 1.010. Data quality ------------ One live run (run 12) has a final gravity of 900, which worked out as 338% attenuation and dragged the average to 103.2%. That is not a real result, so: - Attenuation is rejected when the final gravity is not physically possible for the original gravity. - Runs that fail that check are EXCLUDED from the averages rather than silently distorting them, and are listed on the page as needing review. - The average now reads 87.5% +/- 7.4 percentage points across 16 brews. Chart bugs found while building this ------------------------------------ - **The gravity charts had their y axis reversed.** With `reverse: true` the high reading sits at the BOTTOM, so the fermentation curve climbed left to right - it read as gravity *rising* as the beer fermented. A brewer reads a fermentation as a descending line. The point-scale run chart on the run detail page had the same bug. Both are fixed. - **The curve chart's x axis was treated as categorical.** The x values are numeric (0-100), and Chart.js defaults a line chart's x axis to category, which collapsed every point onto index 0 and rendered the curve as a sliver at the origin. The axis is now explicitly linear. - **The curve was interpolated by exact key lookup**, so it only worked when every brew happened to be read at the same progress points. It now interpolates between a brew's actual readings, and the curve carries 21 points across the full range. - **Long beer names were clipped** at the left edge of the attenuation chart ("...x crying out loud"). Names are trimmed to 18 characters with an ellipsis; the tooltip and the table carry the full name. Tests ----- 127 (up from 105). The two chart regressions are covered by reading `charts.js` and asserting the gravity axis is not reversed and that long names are trimmed - these are exactly the kind of change that silently reverts, and they are invisible to a screenshot test. --- src/flask/brewing.py | 117 ++++++++++++ src/flask/static/js/charts.js | 294 +++++++++++++++++++++++++++++- src/flask/templates/analysis.html | 221 ++++++++++++++++++++++ src/flask/templates/base.html | 2 + src/flask/views/analysis.py | 87 ++++++++- tests/test_analysis.py | 243 ++++++++++++++++++++++++ tests/test_app.py | 62 +++++++ 7 files changed, 1023 insertions(+), 3 deletions(-) create mode 100644 src/flask/templates/analysis.html create mode 100644 tests/test_analysis.py diff --git a/src/flask/brewing.py b/src/flask/brewing.py index 800bec2..32126bd 100644 --- a/src/flask/brewing.py +++ b/src/flask/brewing.py @@ -431,6 +431,121 @@ def _fmt_point_scale(points: int | None) -> str: return f"{points % 1000}" +def fermentation_curve(run: Run) -> list[dict]: + """The run's gravity readings shaped for plotting. + + Each point carries the reading date, the gravity, and how far through + the fermentation it was - the fraction of the total gravity drop that + had happened by that reading. That makes two runs with different + gravities directly comparable, which is the point of the chart. + """ + values = run.sg_values() + if len(values) < 2: + return [] + + og = values[0] + total_drop = og - values[-1] + if total_drop <= 0: + # No drop to measure progress against. + return [] + + points = [] + for reading in run.readings: + if reading.sg_points is None: + continue + drop = og - reading.sg_points + points.append({ + "date": reading.date.isoformat() if reading.date else None, + "sg": reading.sg_points, + "sg_decimal": _fmt_decimal(reading.sg_points), + "point_scale": _fmt_point_scale(reading.sg_points), + # 0.0 at the start, 1.0 at the final reading. + "progress": round(max(0.0, min(1.0, drop / total_drop)), 4), + "final": reading.final_reading, + "comment": reading.comment, + }) + return points + + +def attenuation_percent(run: Run) -> float | None: + """The apparent attenuation: how much of the original gravity was + fermented away. + + This is the number a brewer actually compares between recipes, and it is + more informative than ABV alone because it normalises for how strong the + starting wort was. + + Returns ``None`` when the figure would be meaningless: + + - The original gravity was not above 1.000, so there is nothing to + attenuate. + - The final gravity is below 1.000. Beer cannot be less dense than + water, so a reading below 1.000 means the reading is wrong (a + mis-entry, or a hydrometer read at the wrong temperature). Run 12 in + the live data has a final gravity of 900, which would otherwise show + as a nonsense 338% attenuation. The run is reported separately by + :func:`implausible_runs` rather than being quietly folded into the + averages. + """ + og = run.original_gravity() + fg = run.final_gravity() + if og is None or fg is None or og <= 1000 or fg < 1000: + return None + attenuation = (og - fg) / (og - 1000) * 100 + if attenuation > 100: + # Cannot ferment away more than the sugar that was there. + return None + return round(attenuation, 1) + + +def implausible_runs(runs: Iterable[Run]) -> list[dict]: + """The runs whose gravity readings are physically impossible. + + Beer cannot be less dense than water, so a final gravity below 1.000 + means a reading is wrong. These runs are excluded from the averages and + listed on the analysis page instead of being silently included. + """ + problems = [] + for run in runs: + og = run.original_gravity() + fg = run.final_gravity() + if fg is not None and fg < 1000: + problems.append({ + "beer_run_id": run.beer_run_id, + "og_points": og, + "fg_points": fg, + "problem": ( + f"final gravity {fg / 1000:.3f} is below 1.000 — " + "beer cannot be less dense than water, so this reading " + "is probably wrong" + ), + }) + 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({ + "beer_run_id": run.beer_run_id, + "og_points": og, + "fg_points": fg, + "problem": "the gravity drop implies more than 100% attenuation", + }) + return problems + + +def days_to_target(run: Run, target: int) -> int | None: + """How many days the run took to reach *target* gravity. + + Used to answer "how long does this usually take?", which is the question + a brewer actually has when planning the next batch. + """ + for reading in run.readings: + 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 + + def run_summary(run: Run, today: date | None = None, og_override: int | None = None) -> dict: """A flat dict describing a run, ready for a template or a JSON response.""" @@ -466,6 +581,8 @@ def run_summary(run: Run, today: date | None = None, "days_in_fermenter": run.days_in_fermenter(today), "comments": run.comments(), "last_comment": run.comments()[-1] if run.comments() else "", + "attenuation": attenuation_percent(run), + "curve": fermentation_curve(run), "outliers": [r.sg_points for r in run.outlier_readings()], "readings": [ { diff --git a/src/flask/static/js/charts.js b/src/flask/static/js/charts.js index 0d874d9..f90f2d5 100644 --- a/src/flask/static/js/charts.js +++ b/src/flask/static/js/charts.js @@ -152,8 +152,18 @@ } }, scales: { + x: { + // The dates are strings, so this axis stays + // categorical - but it must be declared, + // because Chart.js would otherwise infer + // something else from the {x, y} points. + type: "category", + ticks: { maxRotation: 0, autoSkipPadding: 12 } + }, y: { - reverse: true, + // NOT reversed - see the fermentation curve. + // Gravity falls as the beer ferments, so the + // line must descend left to right. title: { display: true, text: "Specific gravity (points)" } } } @@ -206,7 +216,19 @@ }, scales: { y: { beginAtZero: true, title: { display: true, text: "% ABV" } }, - x: { ticks: { maxRotation: 45, minRotation: 0 } } + x: { + // The names are long and the chart is narrow, + // so the labels are trimmed above and + // rotated. autoSkip keeps them from + // overlapping; the tooltip carries the + // full name for every bar. + ticks: { + maxRotation: 45, + minRotation: 45, + autoSkip: true, + autoSkipPadding: 8 + } + } } }) }); @@ -274,11 +296,279 @@ .catch(function (err) { console.warn(err); }); } + /* ------------------------------------------------ attenuation by brew */ + + function drawAttenuationChart() { + var canvas = document.getElementById("attenuation-chart"); + if (!canvas) { return; } + + fetch("/api/runs", { headers: { "Accept": "application/json" } }) + .then(function (r) { return r.json(); }) + .then(function (runs) { + var done = runs.filter(function (r) { + return r.attenuation !== null && r.attenuation !== undefined; + }); + if (!done.length) { return; } + + new Chart(canvas.getContext("2d"), { + type: "bar", + data: { + labels: done.map(function (r) { + var name = r.canonical_name || ("Run " + r.beer_run_id); + // Long names were clipped at the left edge of + // the plot when rotated. Trim them to a + // readable length; the tooltip and the table + // below carry the full name. + if (name.length > 18) { + name = name.slice(0, 17) + "\u2026"; + } + return name; + }), + datasets: [{ + label: "Attenuation %", + data: done.map(function (r) { return r.attenuation; }), + backgroundColor: withAlpha(ACCENT, 0.65), + borderColor: ACCENT, + borderWidth: 1, + borderRadius: 4 + }] + }, + options: baseOptions({ + plugins: { + legend: { display: false }, + tooltip: { + callbacks: { + label: function (ctx) { + return ctx.parsed.y.toFixed(1) + "% attenuated"; + } + } + } + }, + scales: { + y: { + beginAtZero: true, + // Attenuation cannot exceed 100%, so the axis + // is capped there. Without the cap a single + // bad reading would squash every other bar + // into an unreadable stub. + suggestedMax: 100, + title: { display: true, text: "% attenuated" } + }, + x: { + // The names are long and the chart is narrow, + // so the labels are trimmed above and + // rotated. autoSkip keeps them from + // overlapping; the tooltip carries the + // full name for every bar. + ticks: { + maxRotation: 45, + minRotation: 45, + autoSkip: true, + autoSkipPadding: 8 + } + } + } + }) + }); + }) + .catch(function (err) { console.warn(err); }); + } + + /* -------------------------------------------- fermentation curve overlay */ + + function drawCurveChart() { + var canvas = document.getElementById("curve-chart"); + if (!canvas) { return; } + + fetch("/api/runs", { headers: { "Accept": "application/json" } }) + .then(function (r) { return r.json(); }) + .then(function (runs) { + var withCurve = runs.filter(function (r) { + return r.curve && r.curve.length > 1; + }); + if (!withCurve.length) { return; } + + // Plotting every brew individually gives 16 near-identical + // lines and an unreadable legend. Instead: a MEDIAN curve + // with the min/max envelope behind it, which shows the + // typical shape and the spread without the clutter. + // + // Each curve is the fraction of its own gravity drop + // completed, so brews with different starting gravities can + // be compared directly. + var samples = 21; // 0%, 5%, ... 100% + + // Interpolate each brew's curve onto the common progress axis. + // + // The brew's readings are at its own progress values (say + // 0%, 40%, 100%), so each sample point is interpolated + // BETWEEN the two readings that bracket it. Looking the + // value up by an exact key only works when every brew + // happened to be read at the same progress points, which is + // why the first attempt produced three points. + function interpolate(curve, pct) { + if (!curve.length) { return null; } + if (pct <= curve[0].x) { return curve[0].y; } + var last = curve[curve.length - 1]; + if (pct >= last.x) { return last.y; } + for (var k = 0; k < curve.length - 1; k++) { + var a = curve[k]; + var b = curve[k + 1]; + if (pct >= a.x && pct <= b.x) { + var span = b.x - a.x; + if (span === 0) { return a.y; } + return a.y + (b.y - a.y) * ((pct - a.x) / span); + } + } + return last.y; + } + + // The raw readings of each brew, sorted by progress. + var rawCurves = withCurve.map(function (r) { + var points = r.curve.map(function (p) { + return { + x: p.progress * 100, + y: parseFloat(p.sg_decimal) + }; + }).filter(function (p) { return !isNaN(p.y); }); + points.sort(function (a, b) { return a.x - b.x; }); + return points; + }).filter(function (c) { return c.length > 1; }); + + if (!rawCurves.length) { return; } + + // The interpolated value of every brew at every sample point. + var sampled = rawCurves.map(function (curve) { + var out = []; + for (var i = 0; i < samples; i++) { + var pct = (i / (samples - 1)) * 100; + var y = interpolate(curve, pct); + if (y === null) { continue; } + out.push({ x: pct, y: y }); + } + return out; + }).filter(function (c) { return c.length > 1; }); + + if (!sampled.length) { return; } + + // The envelope: the highest and lowest gravity at each + // progress point. + var envelope = []; + var median = []; + for (var i = 0; i < samples; i++) { + var pct = (i / (samples - 1)) * 100; + var values = []; + sampled.forEach(function (curve) { + var match = curve.filter(function (p) { + return Math.abs(p.x - pct) < 0.01; + }); + if (match.length) { values.push(match[0].y); } + }); + if (!values.length) { continue; } + values.sort(function (a, b) { return a - b; }); + envelope.push({ + x: pct, + y: values[0], + yMax: values[values.length - 1] + }); + var mid = Math.floor(values.length / 2); + median.push({ + x: pct, + y: values.length % 2 + ? values[mid] + : (values[mid - 1] + values[mid]) / 2 + }); + } + + new Chart(canvas.getContext("2d"), { + type: "line", + data: { + datasets: [ + { + label: "Range (min–max)", + data: envelope, + borderColor: withAlpha(ACCENT, 0.35), + backgroundColor: withAlpha(ACCENT, 0.12), + borderWidth: 1, + pointRadius: 0, + fill: "+1", + tension: 0.3 + }, + { + label: "Median brew", + data: median, + borderColor: ACCENT_BRIGHT, + backgroundColor: withAlpha(ACCENT_BRIGHT, 0.15), + borderWidth: 2.5, + pointRadius: 3, + pointBackgroundColor: ACCENT_BRIGHT, + fill: false, + tension: 0.3 + } + ] + }, + options: baseOptions({ + plugins: { + legend: { + display: true, + labels: { + boxWidth: 10, + boxHeight: 10, + usePointStyle: true, + font: { size: 11 } + } + }, + tooltip: { + callbacks: { + label: function (ctx) { + var point = ctx.parsed; + if (ctx.datasetIndex === 0) { + return "range " + point.y + + " to " + (point.yMax || point.y); + } + return "median " + point.y; + } + } + } + }, + scales: { + x: { + // The x values are NUMERIC (0-100), so the + // axis must be linear. Chart.js defaults a + // line chart's x-axis to category, which + // collapses every point onto index 0 and + // renders the curve as a sliver at the + // origin. + type: "linear", + title: { display: true, text: "% of gravity drop" }, + min: 0, + max: 100, + ticks: { stepSize: 10 } + }, + y: { + // NOT reversed. Specific gravity falls as + // the beer ferments, and the scale puts the + // HIGH reading at the top by default, so the + // curve descends left to right - the way a + // brewer reads a fermentation. With + // reverse:true the curve climbed instead, + // which read as gravity rising. + title: { display: true, text: "Specific gravity" } + } + } + }) + }); + }) + .catch(function (err) { console.warn(err); }); + } + function init() { drawSparklines(); drawRunChart(); drawAbvChart(); drawMonthsChart(); + drawAttenuationChart(); + drawCurveChart(); } if (document.readyState === "loading") { diff --git a/src/flask/templates/analysis.html b/src/flask/templates/analysis.html new file mode 100644 index 0000000..d0a78d3 --- /dev/null +++ b/src/flask/templates/analysis.html @@ -0,0 +1,221 @@ +{% extends 'base.html' %} + +{% block title %}Analysis — Brew Log{% endblock %} + +{% block content %} +
+
+

Analysis

+

How consistently the brews ferment, and how long each one takes.

+
+
+ History + Active beers +
+
+ +{% if not rows %} +
+
+ +

No finished brews to analyse yet.

+

Once a run has a final reading it appears here.

+
+
+{% else %} + +
+
+
Average attenuation
+
+ {% if stats.avg_attenuation is not none %}{{ stats.avg_attenuation }}%{% else %}—{% endif %} +
+
of the original gravity fermented
+
+
+
Spread
+
+ {% if stats.consistency %}±{{ stats.consistency.stdev }}pp{% else %}—{% endif %} +
+
+ {% if stats.consistency %} + standard deviation of attenuation, lower is better + {% else %} + needs at least two brews + {% endif %} +
+
+
+
Average brew time
+
+ {% if stats.avg_days is not none %}{{ stats.avg_days }}d{% else %}—{% endif %} +
+
first reading to final reading
+
+
+
Time to 1.010
+
+ {% if stats.avg_days_to_1010 is not none %}{{ stats.avg_days_to_1010 }}d{% else %}—{% endif %} +
+
+ {% if stats.reached_1010 %} + average of {{ stats.reached_1010 }} brew{{ '' if stats.reached_1010 == 1 else 's' }} that got there + {% else %} + no brew reached 1.010 + {% endif %} +
+
+
+ +
+
+
+

Attenuation by brew

+ How much of the original gravity was fermented away +
+
+ +
+
+ +
+
+

Fermentation curves

+ Progress through fermentation, 0% to 100% +
+
+ +
+
+
+ +
+
+

Every finished brew

+ {{ stats.count }} brews +
+
+
+ + + + + + + + + + + + + + + + {% for row in rows %} + + + + + + + + + + + + {% endfor %} + +
RunNameOGFGABVAttenuationDaysTo 1.010Finished
{{ row.beer_run_id }}{{ row.name }} + {% if row.og_points is not none %} + {{ '%.3f' % (row.og_points / 1000) }} + {% else %}—{% endif %} + + {% if row.fg_points is not none %} + {{ '%.3f' % (row.fg_points / 1000) }} + {% else %}—{% endif %} + + {% if row.abv is not none %}{{ '%.2f' % row.abv }}%{% else %}—{% endif %} + + {% if row.attenuation is not none %} + {{ row.attenuation }}% + {% else %}—{% endif %} + {{ row.days if row.days is not none else '—' }}{{ row.days_to_1010 if row.days_to_1010 is not none else '—' }}{{ row.final_date or '—' }}
+
+
+
+ +{% if suspect %} +
+
+

Readings that need a second look

+ {{ suspect | length }} run{{ '' if suspect | length == 1 else 's' }} excluded from the averages +
+
+
+ + + + + + + + + + + {% for row in suspect %} + + + + + + + {% endfor %} + +
RunOGFGProblem
{{ row.beer_run_id }}{{ '%.3f' % (row.og_points / 1000) }}{{ '%.3f' % (row.fg_points / 1000) }}{{ row.problem }}
+
+
+
+{% endif %} + +{% if stats.consistency %} +
+
+

What the numbers say

+
+
+

+ Across {{ stats.count }} finished brews the attenuation averages + {{ stats.consistency.mean }}%, with a standard + deviation of {{ stats.consistency.stdev }} percentage + points and a total range of {{ stats.consistency.spread }} + percentage points. + {% if stats.consistency.stdev <= 3 %} + That is a tight spread — the process is repeatable. + {% elif stats.consistency.stdev <= 6 %} + That is a moderate spread — some variation between batches. + {% else %} + That is a wide spread — something in the process is not + consistent between batches. + {% endif %} +

+ {% if stats.fastest and stats.slowest and stats.fastest != stats.slowest %} +

+ The quickest brew was {{ stats.fastest.name }} at + {{ stats.fastest.days }} days; the longest was + {{ stats.slowest.name }} at + {{ stats.slowest.days }} days. +

+ {% endif %} + {% if stats.avg_days_to_1010 %} +

+ On average a brew reaches gravity 1.010 after + {{ stats.avg_days_to_1010 }} days — the number to plan around when + deciding when to check on a fermenter. +

+ {% endif %} +
+
+{% endif %} + +{% endif %} +{% endblock %} diff --git a/src/flask/templates/base.html b/src/flask/templates/base.html index 3e78494..f998f91 100644 --- a/src/flask/templates/base.html +++ b/src/flask/templates/base.html @@ -23,6 +23,8 @@ href="{{ url_for('analysis.active') }}">Active Beers History + Analysis
diff --git a/src/flask/views/analysis.py b/src/flask/views/analysis.py index 0737c73..99d0cee 100644 --- a/src/flask/views/analysis.py +++ b/src/flask/views/analysis.py @@ -7,7 +7,8 @@ from datetime import date from flask import Blueprint, redirect, render_template, request, session, url_for -from brewing import Run, is_plausible_sg, run_summary +from brewing import (Run, attenuation_percent, days_to_target, implausible_runs, + is_plausible_sg, run_summary) bp = Blueprint("analysis", __name__) @@ -32,6 +33,90 @@ def login_required(fn): return wrapper +@bp.route("/analysis") +@login_required +def deep(): + """The deeper analysis: how the brews compare, and how long they take. + + This is the page that answers the questions the history page cannot: + how consistent is my attenuation, which brew fermented fastest, and what + do the gravity curves look like side by side. + """ + runs = repo().runs() + metadata = repo().all_run_metadata() + + finished = [r for r in runs if not r.is_active()] + + # Runs whose gravity readings are physically impossible are excluded from + # the averages and listed separately, rather than being silently folded + # in (a final gravity below 1.000 would otherwise show as a 338% + # attenuation). + suspect = implausible_runs(finished) + + rows = [] + for run in finished: + meta = metadata.get(run.beer_run_id, {}) + summary = _decorate(run, metadata) + rows.append({ + "beer_run_id": run.beer_run_id, + "name": meta.get("name") or run.display_name(), + "final_date": summary["final_date"], + "first_date": summary["first_date"], + "og_points": summary["og_points"], + "fg_points": summary["fg_points"], + "abv": summary["abv"], + "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"], + }) + + # The rows with a usable curve, for the overlay chart. + curves = [r for r in rows if r["curve"]] + + attenuations = [r["attenuation"] for r in rows if r["attenuation"] is not None] + durations = [r["days"] for r in rows if r["days"] is not None] + to_1010 = [r["days_to_1010"] for r in rows if r["days_to_1010"] is not None] + + # --- consistency ------------------------------------------------------- # + # How much the attenuation varies. A tight spread means the process is + # repeatable; a wide one means something is inconsistent. + consistency = None + if len(attenuations) >= 2: + mean = sum(attenuations) / len(attenuations) + variance = sum((a - mean) ** 2 for a in attenuations) / len(attenuations) + consistency = { + "mean": round(mean, 1), + "stdev": round(variance ** 0.5, 1), + "spread": round(max(attenuations) - min(attenuations), 1), + } + + fastest = min((r for r in rows if r["days"] is not None), + key=lambda r: r["days"], default=None) + slowest = max((r for r in rows if r["days"] is not None), + key=lambda r: r["days"], default=None) + + return render_template( + "analysis.html", + rows=rows, + curves=curves, + suspect=suspect, + stats={ + "count": len(rows), + "avg_attenuation": round(sum(attenuations) / len(attenuations), 1) + if attenuations else None, + "avg_days": _fmt_days(sum(durations) / len(durations)) if durations else None, + "avg_days_to_1010": _fmt_days(sum(to_1010) / len(to_1010)) if to_1010 else None, + "reached_1010": len(to_1010), + "consistency": consistency, + "fastest": fastest, + "slowest": slowest, + }, + ) + + def _month_range(first: str, last: str) -> list[str]: """Every ``YYYY-MM`` from *first* to *last* inclusive. diff --git a/tests/test_analysis.py b/tests/test_analysis.py new file mode 100644 index 0000000..f95fad9 --- /dev/null +++ b/tests/test_analysis.py @@ -0,0 +1,243 @@ +"""Tests for the deeper analysis: attenuation, fermentation curves, timings.""" + +from __future__ import annotations + +from datetime import date +from pathlib import Path + +import pytest + +from brewing import (Run, attenuation_percent, days_to_target, + fermentation_curve) + + +def _doc(run_id="1", beer_type="Ale", sg="1045", date_="2026-01-01", + final="False", comment=""): + return { + "_id": f"id-{run_id}-{date_}-{sg}", + "beer_run_id": run_id, + "beer_type": beer_type, + "sg": sg, + "date": date_, + "final_reading": final, + "comment": comment, + } + + +# ------------------------------------------------------------ attenuation # + +def test_attenuation_percent(): + """Attenuation is the fraction of the original gravity fermented away.""" + # OG 1050, FG 1010: 40 of the 50 points above 1.000 went away. + run = Run.from_docs("1", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1010", date_="2026-01-20", final="True"), + ]) + assert attenuation_percent(run) == 80.0 + + +def test_attenuation_percent_of_a_full_fermentation(): + """A wort that finishes at 1.000 has 100% attenuation.""" + run = Run.from_docs("2", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1000", date_="2026-01-20", final="True"), + ]) + assert attenuation_percent(run) == 100.0 + + +def test_attenuation_percent_needs_a_gravity_above_1000(): + """A run that started at or below 1.000 has no meaningful attenuation.""" + run = Run.from_docs("3", [ + _doc(sg="1000", date_="2026-01-01"), + _doc(sg="0995", date_="2026-01-20", final="True"), + ]) + assert attenuation_percent(run) is None + + +def test_attenuation_percent_of_an_unfinished_run_uses_its_current_gravity(): + """The current gravity stands in for the final one until the run ends.""" + run = Run.from_docs("4", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1020", date_="2026-01-10"), + ]) + assert attenuation_percent(run) == 60.0 + + +# ------------------------------------------------------- fermentation curve # + +def test_fermentation_curve_is_empty_for_a_single_reading(): + """One reading is no curve.""" + run = Run.from_docs("5", [_doc(sg="1045", date_="2026-01-01")]) + assert fermentation_curve(run) == [] + + +def test_fermentation_curve_progresses_from_zero_to_one(): + """The curve is the fraction of the gravity drop completed.""" + run = Run.from_docs("6", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1030", date_="2026-01-10"), + _doc(sg="1010", date_="2026-01-20", final="True"), + ]) + curve = fermentation_curve(run) + + assert len(curve) == 3 + assert curve[0]["progress"] == 0.0 + assert curve[1]["progress"] == 0.5 + assert curve[2]["progress"] == 1.0 + + +def test_fermentation_curve_carries_the_reading_details(): + run = Run.from_docs("7", [ + _doc(sg="1050", date_="2026-01-01", comment="brew day"), + _doc(sg="1010", date_="2026-01-20", final="True", comment="kegging"), + ]) + curve = fermentation_curve(run) + + assert curve[0]["date"] == "2026-01-01" + assert curve[0]["sg_decimal"] == "1.050" + assert curve[0]["point_scale"] == "50" + assert curve[0]["comment"] == "brew day" + assert curve[1]["final"] is True + + +def test_fermentation_curve_is_empty_when_the_gravity_never_falls(): + """A run that only goes up has no drop to measure progress against.""" + run = Run.from_docs("8", [ + _doc(sg="1010", date_="2026-01-01"), + _doc(sg="1020", date_="2026-01-10", final="True"), + ]) + assert fermentation_curve(run) == [] + + +# ------------------------------------------------------------- days to target # + +def test_days_to_target(): + """How long the brew took to reach a given gravity.""" + run = Run.from_docs("9", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1020", date_="2026-01-08"), + _doc(sg="1005", date_="2026-01-20", final="True"), + ]) + # 1020 is still above 1015, so the run does not reach 1015 until the + # third reading, 19 days in. + assert days_to_target(run, 1015) == 19 + assert days_to_target(run, 1010) == 19 + # 1020 is below 1025, so that target is reached on the second reading. + assert days_to_target(run, 1025) == 7 + assert days_to_target(run, 1000) is None + + +def test_days_to_target_returns_none_when_never_reached(): + run = Run.from_docs("10", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1040", date_="2026-01-20", final="True"), + ]) + assert days_to_target(run, 1010) is None + + +# ------------------------------------------------------------------- summary # + +def test_run_summary_includes_the_new_analysis_fields(): + from brewing import run_summary + + run = Run.from_docs("11", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1010", date_="2026-01-20", final="True"), + ]) + summary = run_summary(run) + + assert summary["attenuation"] == 80.0 + assert len(summary["curve"]) == 2 + assert summary["curve"][0]["progress"] == 0.0 + + +def test_attenuation_is_none_for_a_final_gravity_below_water(): + """Beer cannot be less dense than water. Run 12 in the live data has a + final gravity of 900, which would otherwise show as 338% attenuation.""" + run = Run.from_docs("12", [ + _doc(sg="1042", date_="2025-10-05"), + _doc(sg="900", date_="2025-11-30", final="True"), + ]) + assert attenuation_percent(run) is None + + +def test_attenuation_is_none_when_it_would_exceed_100_percent(): + run = Run.from_docs("13", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1000", date_="2026-01-20", final="True"), + ]) + # 1050 -> 1000 is exactly 100%, which is the limit but still valid. + assert attenuation_percent(run) == 100.0 + + run2 = Run.from_docs("14", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1010", date_="2026-01-20", final="True"), + ]) + assert attenuation_percent(run2) == 80.0 + + +def test_implausible_runs_are_identified(): + from brewing import implausible_runs + + good = Run.from_docs("1", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1010", date_="2026-01-20", final="True"), + ]) + bad = Run.from_docs("12", [ + _doc(sg="1042", date_="2025-10-05"), + _doc(sg="900", date_="2025-11-30", final="True"), + ]) + + problems = implausible_runs([good, bad]) + assert len(problems) == 1 + assert problems[0]["beer_run_id"] == "12" + assert problems[0]["fg_points"] == 900 + assert "below 1.000" in problems[0]["problem"] + + +def test_implausible_runs_ignores_good_runs(): + from brewing import implausible_runs + + good = Run.from_docs("1", [ + _doc(sg="1050", date_="2026-01-01"), + _doc(sg="1010", date_="2026-01-20", final="True"), + ]) + assert implausible_runs([good]) == [] + +# ------------------------------------------------------- chart rendering # + +def test_charts_do_not_reverse_the_gravity_axis(): + """Gravity must FALL on screen as the beer ferments. + + The gravity charts originally set `reverse: true` on the y axis, which + puts the HIGH reading at the bottom of the plot - so the fermentation + curve climbed left to right and read as gravity *rising*. A brewer + reads a fermentation as a descending line, so the axis must not be + reversed. The point-scale run chart had the same bug. + """ + charts_js = ( + Path(__file__).resolve().parents[1] + / "src" / "flask" / "static" / "js" / "charts.js" + ).read_text() + + assert "reverse: true" not in charts_js, ( + "a gravity chart is reversing the y axis; the fermentation curve " + "would climb instead of descend" + ) + + +def test_attenuation_chart_truncates_long_names(): + """Long beer names were clipped at the left edge of the plot. + + The rotated x-axis labels were cut off ("...x crying out loud"), so the + names are trimmed to a readable length before they are drawn. The + tooltip and the table below carry the full name. + """ + charts_js = ( + Path(__file__).resolve().parents[1] + / "src" / "flask" / "static" / "js" / "charts.js" + ).read_text() + + assert "name.length > 18" in charts_js, ( + "the attenuation chart is not trimming long beer names" + ) diff --git a/tests/test_app.py b/tests/test_app.py index b33f4ba..7f27e91 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -639,3 +639,65 @@ def test_month_range_covers_a_year_boundary(): "2025-11", "2025-12", "2026-01", "2026-02", ] assert _month_range("2026-05", "2026-05") == ["2026-05"] + + +# ------------------------------------------------------------- deep analysis # + +def test_the_analysis_page_renders(logged_in, repo): + seed_readings(repo, [ + ("1", "Pale Ale", "1050", "2026-01-01", False, ""), + ("1", "Pale Ale", "1010", "2026-01-20", True, "done"), + ("2", "Ginger Beer", "1045", "2026-02-01", False, ""), + ("2", "Ginger Beer", "1015", "2026-02-15", True, "done"), + ]) + + response = logged_in.get("/analysis") + assert response.status_code == 200 + body = response.data.decode() + assert "Pale Ale" in body + assert "Ginger Beer" in body + # The attenuation figures and the summary sentence. + assert "80.0%" in body + assert "attenuation" in body.lower() + + +def test_the_analysis_page_excludes_active_runs(logged_in, repo): + """An unfinished brew has no final reading, so it is not comparable.""" + seed_readings(repo, [ + ("1", "Finished", "1050", "2026-01-01", False, ""), + ("1", "Finished", "1010", "2026-01-20", True, ""), + ("2", "Still Going", "1045", "2026-02-01", False, ""), + ]) + + body = logged_in.get("/analysis").data.decode() + assert "Finished" in body + assert "Still Going" not in body + + +def test_the_analysis_page_needs_at_least_one_finished_brew(logged_in, repo): + seed_readings(repo, [("1", "Just Started", "1045", "2026-02-01", False, "")]) + + response = logged_in.get("/analysis") + assert response.status_code == 200 + assert b"No finished brews" in response.data + + +def test_the_analysis_page_is_protected(client): + response = client.get("/analysis") + assert response.status_code == 302 + assert "/login" in response.headers["Location"] + + +def test_the_api_exposes_the_attenuation_and_curve(logged_in, repo): + """The charts fetch the curve data from the API.""" + seed_readings(repo, [ + ("1", "Pale Ale", "1050", "2026-01-01", False, ""), + ("1", "Pale Ale", "1010", "2026-01-20", True, ""), + ]) + + payload = logged_in.get("/api/runs").get_json() + entry = payload[0] + assert entry["attenuation"] == 80.0 + assert len(entry["curve"]) == 2 + assert entry["curve"][0]["progress"] == 0.0 + assert entry["curve"][1]["progress"] == 1.0 From 4d1e070d746f3f76cb304825b194b24166046ce4 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 13:12:51 +1000 Subject: [PATCH 2/4] docs: document the analysis page and the CI deploy flow The RUNBOOK still said the workflow did not deploy to the cluster. It does now - it builds, pushes, applies the manifests, rolls the pods and verifies the app from inside the pod. The manual deploy path is documented alongside it, along with the dry-run step that catches the schema errors kubectl apply silently swallows. --- README.md | 23 ++++++++++++++++++++++- RUNBOOK.md | 44 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 545f88c..75b504e 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,7 @@ The tests use `mongomock`, so they need no database and no network. | `/brews` | The brew process: record a reading, review every run | | `/active` | What is in the fermenter right now — tap, gravity, projected strength | | `/history` | How every brew has gone, with charts and the standard-vs-legacy ABV comparison | +| `/analysis` | How consistently the brews ferment — attenuation, fermentation curves, brew timings | ### `/active` — the active beers page @@ -59,6 +60,27 @@ A run with only one reading and no recorded original gravity has no reliable estimate, so the alcohol figure shows a dash rather than a misleading number. Record the OG on the run (or take a second reading) and it appears. +### `/analysis` — how the brews ferment + +The page that turns the history into something you can brew better from: + +- **Attenuation by brew** — what fraction of the original gravity was + fermented away, for every finished brew. +- **Fermentation curves** — the typical shape of a fermentation, drawn as a + median curve with the min–max envelope behind it. Plotting every brew + individually gives sixteen near-identical lines and an unreadable legend. +- **Every finished brew** — OG, FG, ABV, attenuation, days in the fermenter, + days to reach 1.010, and the finish date. +- **Readings that need a second look** — readings outside the plausible + hydrometer range. +- Headline figures: average attenuation and its spread, average brew time, + average time to 1.010. + +A reading that is physically impossible — a final gravity below the original +gravity by more than the wort itself could have — is excluded from the +averages rather than silently distorting them, and the run is listed on the +page so it can be corrected. + ### Run metadata Some details cannot be derived from the readings, so they are stored per run in @@ -94,7 +116,6 @@ src/flask/ views/ blueprints: auth, brews, analysis mongo/repository.py Mongo access for the beer_data and beer_runs collections templates/ Jinja templates - static/ CSS, JS, fonts, logo (all served locally — no CDN) static/ CSS, JS, fonts, logo (all served locally - no CDN) tests/ pytest suite scripts/ operational scripts (the run-metadata backfill) diff --git a/RUNBOOK.md b/RUNBOOK.md index bca333f..e2e46cd 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -69,15 +69,53 @@ env: ## Deploying -Pushing to `master` triggers `.gitea/workflows/build_push.yml`, which runs the -test suite and then builds and pushes the image. **The workflow does not -currently deploy to the cluster** — apply the manifests yourself: +Pushing to `master` triggers `.gitea/workflows/build_push.yml`, which builds +the image, pushes it, **deploys it to the cluster and verifies it**. There is +no manual step. + +The workflow is a single job, in this order: + +1. **Checkout** +2. **Create Kubeconfig** — writes `secrets.KUBEC_CONFIG_BUILDX_NEW_2` to + `$HOME/.kube/config`. This is FIRST because the buildx `kubernetes` driver + needs it to create its builder pod in the cluster. +3. **Set up Docker Buildx** — `driver: kubernetes`, `namespace=gitea-runner`. +4. **Login to the registry** — `secrets.REG_PASSWORD`. +5. **Build and Push** — `linux/amd64` only. A multi-arch build runs qemu, + which crashes the buildkit pod on this NUC (exit 255, `SandboxChanged`) + and the build then hangs for many minutes. +6. **Deploy** — installs kubectl, applies `kube/*.yaml`, creates the + image-pull secret idempotently, then `rollout restart` + `rollout status`. + The rollout restart is required: the image tag in the manifest is + `:latest`, so `kubectl apply` alone does not roll the pods. +7. **Verify the deployed app** — exercises `/healthz` and `/readyz` from + inside the pod, using the pod's own python (there is no `curl` in the + slim image). + +> The deploy step does **not** delete the namespace. That would destroy the +> hand-provisioned `beer-db` and `beer-app` Secrets. + +### Deploying by hand + +If you need to deploy without going through CI — for example to test a +manifest change — do the same thing the workflow does: ```bash +kubectl apply -f kube/beer_namespace.yaml +kubectl apply -f kube/beer-deployment.yaml +kubectl apply -f kube/beer-service.yaml kubectl -n beer rollout restart deployment/beer-deployment kubectl -n beer rollout status deployment/beer-deployment --timeout=180s ``` +Validate the manifests before applying them. `kubectl apply` **silently +rejects** a manifest with a schema error, so a broken manifest leaves the +old Deployment in place while the job still reports success: + +```bash +kubectl apply --dry-run=client -f kube/ +``` + ### Verify the deploy actually took CI green and pods `Ready` do **not** prove the new code is running. Check the From bbd9fe17e585943e5d361a20f59349091f52e9b7 Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 14:03:07 +1000 Subject: [PATCH 3/4] fix(analysis): exclude suspect runs from every average, not just attenuation Review finding, and it was right. The PR claimed that runs with impossible readings were "excluded from the averages". That was only true of the attenuation average. Run 12's bad final gravity was still poisoning three other statistics: - **Brew time** - its 56 days were folded into the average, making a typical brew look like 13.7 days when it is 10.9. - **Time to 1.010** - its 56 days did the same to that figure (8.4d). - **The history page's ABV** - an 18.64% ABV, which made run 12 the "strongest" beer in the history and pulled the average up. - **The fermentation curve** - its curve plunges from 1.042 to 0.900, which dragged the median line down and stretched the min-max envelope to cover a gravity below water. One bad reading was reaching four different statistics. What changed ------------ - The suspect runs are computed once and excluded from every aggregate: the attenuation, brew-time and time-to-1.010 averages, the fastest and slowest brews, the history page's ABV average, and its strongest and lightest. - `/api/runs` now carries a `suspect` flag, and the charts skip the flagged runs - the attenuation chart and the fermentation curve. - The suspect run stays in the table, with a "not counted" chip and a tinted row, so it can be corrected. It is excluded from the figures, not hidden. The card header says "16 brews (15 counted)" and the prose says how many brews the figures are based on. Tests ----- 133 (up from 127). New coverage: the suspect run is excluded from the brew time and time-to-1.010 averages, it is still listed for review, it is flagged in the API, it is not the strongest brew in the history, and the charts filter it out. The `logged_in` fixture moved to conftest.py, where both test modules can use it. --- src/flask/static/css/app.css | 7 ++ src/flask/static/js/charts.js | 11 ++- src/flask/templates/analysis.html | 8 +- src/flask/views/analysis.py | 43 +++++++--- src/flask/views/brews.py | 8 +- tests/conftest.py | 11 +++ tests/test_analysis.py | 134 ++++++++++++++++++++++++++++++ tests/test_app.py | 11 --- 8 files changed, 204 insertions(+), 29 deletions(-) diff --git a/src/flask/static/css/app.css b/src/flask/static/css/app.css index e8a486e..6bc8a98 100644 --- a/src/flask/static/css/app.css +++ b/src/flask/static/css/app.css @@ -498,6 +498,13 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; } .chip--warn { background: var(--warn-dim); color: var(--warn); } .chip--danger { background: var(--danger-dim); color: var(--danger); } .chip--good { background: var(--success-dim); color: var(--success); } +/* A run whose readings are impossible: shown in the table so it can be + corrected, but marked as not counted in the averages. */ +.data tbody tr.is-suspect td { + color: var(--text-muted); + background: rgba(232, 163, 61, .06); +} + .chip--tap { background: var(--bg-raised-2); border: 1px solid var(--amber-deep); diff --git a/src/flask/static/js/charts.js b/src/flask/static/js/charts.js index f90f2d5..3b2555e 100644 --- a/src/flask/static/js/charts.js +++ b/src/flask/static/js/charts.js @@ -305,8 +305,12 @@ 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. var done = runs.filter(function (r) { - return r.attenuation !== null && r.attenuation !== undefined; + return !r.suspect && + r.attenuation !== null && r.attenuation !== undefined; }); if (!done.length) { return; } @@ -383,8 +387,11 @@ 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. var withCurve = runs.filter(function (r) { - return r.curve && r.curve.length > 1; + return !r.suspect && r.curve && r.curve.length > 1; }); if (!withCurve.length) { return; } diff --git a/src/flask/templates/analysis.html b/src/flask/templates/analysis.html index d0a78d3..a42ad75 100644 --- a/src/flask/templates/analysis.html +++ b/src/flask/templates/analysis.html @@ -92,7 +92,7 @@

Every finished brew

- {{ stats.count }} brews + {{ stats.count }} brews{% if stats.suspect_count %} ({{ stats.trusted_count }} counted){% endif %}
@@ -112,9 +112,9 @@ {% for row in rows %} - + {{ row.beer_run_id }} - {{ row.name }} + {{ row.name }}{% if row.suspect %} not counted{% endif %} {% if row.og_points is not none %} {{ '%.3f' % (row.og_points / 1000) }} @@ -184,7 +184,7 @@

- Across {{ stats.count }} finished brews the attenuation averages + Across {{ stats.trusted_count }} finished brews the attenuation averages {{ stats.consistency.mean }}%, with a standard deviation of {{ stats.consistency.stdev }} percentage points and a total range of {{ stats.consistency.spread }} diff --git a/src/flask/views/analysis.py b/src/flask/views/analysis.py index 99d0cee..44b0ff7 100644 --- a/src/flask/views/analysis.py +++ b/src/flask/views/analysis.py @@ -48,10 +48,12 @@ def deep(): finished = [r for r in runs if not r.is_active()] # Runs whose gravity readings are physically impossible are excluded from - # the averages and listed separately, rather than being silently folded - # in (a final gravity below 1.000 would otherwise show as a 338% - # attenuation). + # EVERY aggregate and listed separately, rather than being silently + # folded in. A final gravity below 1.000 would otherwise show as a + # 338% attenuation AND a 56-day brew AND the strongest beer in the + # history - one bad reading poisoning three different statistics. suspect = implausible_runs(finished) + suspect_ids = {s["beer_run_id"] for s in suspect} rows = [] for run in finished: @@ -71,14 +73,20 @@ def deep(): "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, + # but it contributes to none of the averages. + "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"]] - attenuations = [r["attenuation"] for r in rows if r["attenuation"] is not None] - durations = [r["days"] for r in rows if r["days"] is not None] - to_1010 = [r["days_to_1010"] for r in rows if r["days_to_1010"] is not None] + # Only the trustworthy runs feed the statistics. + good = [r for r in rows if not r["suspect"]] + + attenuations = [r["attenuation"] for r in good if r["attenuation"] is not None] + durations = [r["days"] for r in good if r["days"] is not None] + to_1010 = [r["days_to_1010"] for r in good if r["days_to_1010"] is not None] # --- consistency ------------------------------------------------------- # # How much the attenuation varies. A tight spread means the process is @@ -93,9 +101,9 @@ def deep(): "spread": round(max(attenuations) - min(attenuations), 1), } - fastest = min((r for r in rows if r["days"] is not None), + fastest = min((r for r in good if r["days"] is not None), key=lambda r: r["days"], default=None) - slowest = max((r for r in rows if r["days"] is not None), + slowest = max((r for r in good if r["days"] is not None), key=lambda r: r["days"], default=None) return render_template( @@ -105,6 +113,9 @@ def deep(): suspect=suspect, stats={ "count": len(rows), + "suspect_count": len(suspect), + # How many runs the figures are actually based on. + "trusted_count": len(good), "avg_attenuation": round(sum(attenuations) / len(attenuations), 1) if attenuations else None, "avg_days": _fmt_days(sum(durations) / len(durations)) if durations else None, @@ -198,6 +209,8 @@ def history(): summaries = [_decorate(run, metadata) for run in runs] completed = [s for s in summaries if not s["active"]] + # The Run objects for the finished brews, for the plausibility check. + completed_runs = [run for run in runs if not run.is_active()] # --- ABV comparison: standard formula vs the legacy one ---------------- # abv_rows = [] @@ -244,9 +257,17 @@ def history(): "comment": reading["comment"], }) - avg_abv = round(sum(r["abv"] for r in abv_rows) / len(abv_rows), 2) if abv_rows else 0 - best = max(abv_rows, key=lambda r: r["abv"]) if abv_rows else None - worst = min(abv_rows, key=lambda r: r["abv"]) if abv_rows else None + # A run with an impossible final gravity produces an impossible ABV + # (run 12 reads 18.64%), which would make it the "strongest" beer in + # the history and drag the average up. Those runs are excluded from + # the ABV figures here and listed on the analysis page. + history_suspect = {s["beer_run_id"] for s in implausible_runs(completed_runs)} + good_abv_rows = [r for r in abv_rows if r["beer_run_id"] not in history_suspect] + + avg_abv = round(sum(r["abv"] for r in good_abv_rows) / len(good_abv_rows), 2) \ + if good_abv_rows else 0 + best = max(good_abv_rows, key=lambda r: r["abv"]) if good_abv_rows else None + worst = min(good_abv_rows, key=lambda r: r["abv"]) if good_abv_rows else None return render_template( "history.html", diff --git a/src/flask/views/brews.py b/src/flask/views/brews.py index 3998241..28ac62d 100644 --- a/src/flask/views/brews.py +++ b/src/flask/views/brews.py @@ -9,7 +9,7 @@ from wtforms import (BooleanField, DateField, IntegerField, StringField, SubmitField, TextAreaField) from wtforms.validators import DataRequired, Length, NumberRange, Optional -from brewing import is_plausible_sg, run_summary +from brewing import implausible_runs, is_plausible_sg, run_summary bp = Blueprint("brews", __name__) @@ -178,6 +178,11 @@ def api_runs(): 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, {}) @@ -185,6 +190,7 @@ def api_runs(): 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) diff --git a/tests/conftest.py b/tests/conftest.py index f196fd5..10896b5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -53,6 +53,17 @@ def repo(mongo_client): return BeerRepository(client=mongo_client) +@pytest.fixture() +def logged_in(client, repo): + """A client with an authenticated session and a user in the database.""" + repo.users.insert_one({"username": "beer", "password": "beer_pass"}) + + response = client.post("/login", data={"username": "beer", "password": "beer_pass"}, + follow_redirects=True) + assert response.status_code == 200 + return client + + def seed_readings(repository, rows): """Insert reading documents directly, bypassing the app. diff --git a/tests/test_analysis.py b/tests/test_analysis.py index f95fad9..79cd3a0 100644 --- a/tests/test_analysis.py +++ b/tests/test_analysis.py @@ -9,6 +9,7 @@ import pytest from brewing import (Run, attenuation_percent, days_to_target, fermentation_curve) +from conftest import seed_readings def _doc(run_id="1", beer_type="Ale", sg="1045", date_="2026-01-01", @@ -241,3 +242,136 @@ def test_attenuation_chart_truncates_long_names(): assert "name.length > 18" in charts_js, ( "the attenuation chart is not trimming long beer names" ) + +# ------------------------------------------- suspect runs are not counted # + +def _two_runs(repo): + """One good brew and one with an impossible final gravity. + + The bad run mimics the live data: run 12 has a final gravity of 900, + which is below water and makes the run look like a 56-day brew that + reached 18.64% alcohol. + """ + seed_readings(repo, [ + ("1", "Good ale", "1050", "2026-01-01", False, ""), + ("1", "Good ale", "1010", "2026-01-10", True, ""), + # 56 days, and a final gravity below 1.000. + ("12", "Bad brew", "1042", "2026-01-01", False, ""), + ("12", "Bad brew", "0900", "2026-02-26", True, "typo"), + ]) + + +def _stat_values(body): + """The figures in the analysis stat tiles, in order. + + The tiles are rendered as label/value pairs, so the numbers can be + pulled out without depending on the exact markup. + """ + import re + values = re.findall(r'

([^<]+)
', body) + return [v.strip() for v in values] + + +def test_a_suspect_run_is_excluded_from_the_brew_time_average(logged_in, repo): + """The 56-day bad run must not drag the average brew time up. + + This was the review finding: the run was excluded from the attenuation + average but not from the brew-time average, so one bad reading was + poisoning two different statistics. + """ + _two_runs(repo) + + response = logged_in.get("/analysis") + assert response.status_code == 200 + + body = response.get_data(as_text=True) + values = _stat_values(body) + assert values, "the analysis page rendered no stat tiles" + + # The stat tiles are: average attenuation, spread, average brew time, + # time to 1.010. The good run took 9 days; with the bad run folded + # in the average would be (9 + 56) / 2 = 32.5 days. + assert "9d" in values, f"expected a 9 day brew time, got {values}" + + +def test_a_suspect_run_is_excluded_from_the_time_to_1010_average(logged_in, repo): + """Same for the "time to 1.010" figure.""" + _two_runs(repo) + + response = logged_in.get("/analysis") + body = response.get_data(as_text=True) + values = _stat_values(body) + assert values + # The good run reached 1.010 in 9 days; the bad one "reached" it in 56. + assert "9d" in values, f"expected 9 days to 1.010, got {values}" + + +def test_the_suspect_run_is_still_listed_for_review(logged_in, repo): + """Excluding it from the averages must not hide it. + + The run has to stay visible so it can be corrected. + """ + _two_runs(repo) + + 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_a_suspect_run_is_flagged_in_the_api(logged_in, repo): + """The charts fetch /api/runs, so the flag has to be in the payload.""" + _two_runs(repo) + + response = logged_in.get("/api/runs") + assert response.status_code == 200 + + payload = response.get_json() + by_run = {str(r["beer_run_id"]): r for r in payload} + assert by_run["12"]["suspect"] is True + assert by_run["1"]["suspect"] is False + + +def test_a_suspect_run_is_not_the_strongest_brew_in_the_history(logged_in, repo): + """The history page showed the bad run as the strongest beer. + + An impossible final gravity produced an 18.64% ABV, which made run 12 + the "strongest" brew in the history and pulled the average up. + + The run's own row still shows its ABV - it is real data, and hiding it + would be worse - but it must not be counted as the strongest brew, and + it must not appear in the "Strongest" figure. + """ + _two_runs(repo) + + response = logged_in.get("/history") + assert response.status_code == 200 + + body = response.get_data(as_text=True) + + # The "Strongest" tile shows the good run's 4.76%, not the bad + # run's 18.64%. Find the Strongest tile and read its value. + import re + strongest = re.search( + r"Strongest.*?stat__value[^>]*>([^<]+)<", body, re.DOTALL + ) + assert strongest, "the history page has no Strongest figure" + assert "18.64" not in strongest.group(1), ( + f"the suspect run was counted as the strongest brew: {strongest.group(1)}" + ) + + +def test_the_charts_skip_suspect_runs(): + """The chart script must leave the suspect runs out. + + A suspect run's fermentation curve plunges to a gravity below water, + which would drag the median line down and stretch the envelope. + """ + charts_js = ( + Path(__file__).resolve().parents[1] + / "src" / "flask" / "static" / "js" / "charts.js" + ).read_text() + + assert charts_js.count("!r.suspect") >= 2, ( + "the charts must filter out the runs flagged as suspect" + ) diff --git a/tests/test_app.py b/tests/test_app.py index 7f27e91..609eb45 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -7,17 +7,6 @@ import pytest from conftest import seed_readings -@pytest.fixture() -def logged_in(client, repo): - """A client with an authenticated session and a user in the database.""" - repo.users.insert_one({"username": "beer", "password": "beer_pass"}) - - response = client.post("/login", data={"username": "beer", "password": "beer_pass"}, - follow_redirects=True) - assert response.status_code == 200 - return client - - # ------------------------------------------------------------------ public # def test_healthz_is_public(client): From 6fec78e9e4f2ddf0355d6dd7588de07bf3e137eb Mon Sep 17 00:00:00 2001 From: hermes Date: Mon, 9 Nov 2026 14:18:29 +1000 Subject: [PATCH 4/4] fix(analysis): address the two review nits The re-review returned APPROVED with two low-severity notes. - The prose said "Across 15 finished brews", which reads as though there are only 15 brews. It now says "Across 15 of 16 finished brews", so the exclusion is visible in the sentence itself and not only in the card header. - `/api/runs/` did not carry the `suspect` flag that `/api/runs` does, so a consumer of the detail endpoint could not tell an impossible run from a good one. It does now, with a test. 134 tests. --- src/flask/templates/analysis.html | 2 +- src/flask/views/brews.py | 3 +++ tests/test_analysis.py | 17 +++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/flask/templates/analysis.html b/src/flask/templates/analysis.html index a42ad75..a626181 100644 --- a/src/flask/templates/analysis.html +++ b/src/flask/templates/analysis.html @@ -184,7 +184,7 @@

- Across {{ stats.trusted_count }} 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 {{ stats.consistency.mean }}%, with a standard deviation of {{ stats.consistency.stdev }} percentage points and a total range of {{ stats.consistency.spread }} diff --git a/src/flask/views/brews.py b/src/flask/views/brews.py index 28ac62d..02a24c1 100644 --- a/src/flask/views/brews.py +++ b/src/flask/views/brews.py @@ -208,6 +208,9 @@ def api_run(run_id: str): 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) diff --git a/tests/test_analysis.py b/tests/test_analysis.py index 79cd3a0..cdb3dce 100644 --- a/tests/test_analysis.py +++ b/tests/test_analysis.py @@ -361,6 +361,23 @@ def test_a_suspect_run_is_not_the_strongest_brew_in_the_history(logged_in, repo) ) +def test_the_run_detail_api_also_flags_a_suspect_run(logged_in, repo): + """The detail endpoint carries the same flag as the list endpoint. + + A consumer of /api/runs/ must be able to tell an impossible + run from a good one. + """ + _two_runs(repo) + + response = logged_in.get("/api/runs/12") + assert response.status_code == 200 + assert response.get_json()["suspect"] is True + + response = logged_in.get("/api/runs/1") + assert response.status_code == 200 + assert response.get_json()["suspect"] is False + + def test_the_charts_skip_suspect_runs(): """The chart script must leave the suspect runs out.