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):