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.
This commit is contained in:
2026-11-09 14:03:07 +10:00
parent 4d1e070d74
commit bbd9fe17e5
8 changed files with 204 additions and 29 deletions
+11
View File
@@ -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.
+134
View File
@@ -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'<div class="stat__value">([^<]+)</div>', 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"
)
-11
View File
@@ -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):