fix(analysis): rising gravity, undated readings, and the range band #13
+48
-7
@@ -193,14 +193,32 @@ class Run:
|
||||
"""Gravity points of every *usable* reading, in chronological order."""
|
||||
return [r.sg_points for r in self.readings if r.sg_points is not None]
|
||||
|
||||
def dated_sg_values(self) -> list[int]:
|
||||
"""Gravity points of the readings that have a date, chronologically.
|
||||
|
||||
An undated reading cannot be placed in the run's timeline, so it
|
||||
must not be used as the original gravity: it sorts to the front
|
||||
and would put the OG after the FG.
|
||||
"""
|
||||
dated = [r for r in self.readings
|
||||
if r.sg_points is not None and r.date is not None]
|
||||
return [r.sg_points for r in dated]
|
||||
|
||||
def original_gravity(self) -> int | None:
|
||||
"""OG = the first usable reading of the run."""
|
||||
values = self.sg_values()
|
||||
"""OG = the first *dated* usable reading of the run."""
|
||||
values = self.dated_sg_values()
|
||||
return values[0] if values else None
|
||||
|
||||
def final_gravity(self) -> int | None:
|
||||
"""FG = the last usable reading of the run (if the run is finished)."""
|
||||
values = self.sg_values()
|
||||
"""FG = the last *dated* usable reading of the run (if it is finished).
|
||||
|
||||
For a finished run the final reading is the one flagged as final,
|
||||
which is the reading the brewer took when they racked it.
|
||||
"""
|
||||
final = self.final_reading()
|
||||
if final is not None and final.sg_points is not None:
|
||||
return final.sg_points
|
||||
values = self.dated_sg_values()
|
||||
return values[-1] if values else None
|
||||
|
||||
def max_sg(self) -> int | None:
|
||||
@@ -491,6 +509,11 @@ def attenuation_percent(run: Run) -> float | None:
|
||||
fg = run.final_gravity()
|
||||
if og is None or fg is None or og <= 1000 or fg < 1000:
|
||||
return None
|
||||
if fg > og:
|
||||
# The gravity went UP. That is not attenuation - it is either a
|
||||
# mis-ordered pair of readings or a contaminated sample, and a
|
||||
# negative attenuation is not a number a brewer can act on.
|
||||
return None
|
||||
attenuation = (og - fg) / (og - 1000) * 100
|
||||
if attenuation > 100:
|
||||
# Cannot ferment away more than the sugar that was there.
|
||||
@@ -520,6 +543,17 @@ def implausible_runs(runs: Iterable[Run]) -> list[dict]:
|
||||
"is probably wrong"
|
||||
),
|
||||
})
|
||||
elif og is not None and fg is not None and fg > og:
|
||||
problems.append({
|
||||
"beer_run_id": run.beer_run_id,
|
||||
"og_points": og,
|
||||
"fg_points": fg,
|
||||
"problem": (
|
||||
f"the final gravity {fg / 1000:.3f} is higher than the "
|
||||
f"original {og / 1000:.3f} — the gravity went up, which "
|
||||
"is usually a mis-ordered pair of readings"
|
||||
),
|
||||
})
|
||||
elif og is not None and fg is not None and og > 1000 and fg > 1000 \
|
||||
and (og - fg) / (og - 1000) * 100 > 100:
|
||||
problems.append({
|
||||
@@ -537,11 +571,18 @@ def days_to_target(run: Run, target: int) -> int | None:
|
||||
Used to answer "how long does this usually take?", which is the question
|
||||
a brewer actually has when planning the next batch.
|
||||
"""
|
||||
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:
|
||||
if first is None or first.date is None:
|
||||
# Without a start date there is no elapsed time to measure.
|
||||
return None
|
||||
|
||||
# Skip the readings that cannot be placed on the timeline (no date)
|
||||
# rather than giving up on the whole run because of one of them.
|
||||
for reading in run.readings:
|
||||
if reading.sg_points is None or reading.sg_points > target:
|
||||
continue
|
||||
if reading.date is None:
|
||||
continue
|
||||
return days_between(first.date, reading.date)
|
||||
return None
|
||||
|
||||
|
||||
@@ -179,8 +179,7 @@
|
||||
var canvas = document.getElementById("abv-chart");
|
||||
if (!canvas) { return; }
|
||||
|
||||
fetch("/api/runs", { headers: { "Accept": "application/json" } })
|
||||
.then(function (r) { return r.json(); })
|
||||
fetchRuns()
|
||||
.then(function (runs) {
|
||||
// Only finished brews have a measured ABV. An unfinished run
|
||||
// has a projection, not a measurement, so it is not plotted
|
||||
@@ -240,8 +239,7 @@
|
||||
var canvas = document.getElementById("months-chart");
|
||||
if (!canvas) { return; }
|
||||
|
||||
fetch("/api/runs", { headers: { "Accept": "application/json" } })
|
||||
.then(function (r) { return r.json(); })
|
||||
fetchRuns()
|
||||
.then(function (runs) {
|
||||
var counts = {};
|
||||
runs.forEach(function (r) {
|
||||
@@ -302,8 +300,7 @@
|
||||
var canvas = document.getElementById("attenuation-chart");
|
||||
if (!canvas) { return; }
|
||||
|
||||
fetch("/api/runs", { headers: { "Accept": "application/json" } })
|
||||
.then(function (r) { return r.json(); })
|
||||
fetchRuns()
|
||||
.then(function (runs) {
|
||||
// A run with an impossible reading has no attenuation, and
|
||||
// is flagged as suspect. Both are excluded, so one bad
|
||||
@@ -384,14 +381,18 @@
|
||||
var canvas = document.getElementById("curve-chart");
|
||||
if (!canvas) { return; }
|
||||
|
||||
fetch("/api/runs", { headers: { "Accept": "application/json" } })
|
||||
.then(function (r) { return r.json(); })
|
||||
fetchRuns()
|
||||
.then(function (runs) {
|
||||
// A run with an impossible reading has a broken curve - it
|
||||
// plunges to a gravity below water - which would drag the
|
||||
// median line down and stretch the envelope to cover it.
|
||||
// Only FINISHED brews belong on the curve. An active run's
|
||||
// curve is a partial fermentation, and the curve is meant to
|
||||
// describe how a completed fermentation behaves - including
|
||||
// a partial one pulls the median line off the real shape.
|
||||
var withCurve = runs.filter(function (r) {
|
||||
return !r.suspect && r.curve && r.curve.length > 1;
|
||||
return !r.suspect && !r.active &&
|
||||
r.curve && r.curve.length > 1;
|
||||
});
|
||||
if (!withCurve.length) { return; }
|
||||
|
||||
@@ -462,6 +463,7 @@
|
||||
// progress point.
|
||||
var envelope = [];
|
||||
var median = [];
|
||||
var envelopeMax = [];
|
||||
for (var i = 0; i < samples; i++) {
|
||||
var pct = (i / (samples - 1)) * 100;
|
||||
var values = [];
|
||||
@@ -478,6 +480,7 @@
|
||||
y: values[0],
|
||||
yMax: values[values.length - 1]
|
||||
});
|
||||
envelopeMax.push({ x: pct, y: values[values.length - 1] });
|
||||
var mid = Math.floor(values.length / 2);
|
||||
median.push({
|
||||
x: pct,
|
||||
@@ -492,6 +495,12 @@
|
||||
data: {
|
||||
datasets: [
|
||||
{
|
||||
// The band is drawn as TWO datasets: the min
|
||||
// curve filled down to the max curve. With
|
||||
// a single dataset and `fill: "+1"` the band
|
||||
// was filled to the NEXT dataset - which was
|
||||
// the median line, not the max - so the
|
||||
// tooltip and the shading disagreed.
|
||||
label: "Range (min–max)",
|
||||
data: envelope,
|
||||
borderColor: withAlpha(ACCENT, 0.35),
|
||||
@@ -501,6 +510,18 @@
|
||||
fill: "+1",
|
||||
tension: 0.3
|
||||
},
|
||||
{
|
||||
// The upper edge of the band. It is the
|
||||
// dataset the one above fills to.
|
||||
label: "Range max",
|
||||
data: envelopeMax,
|
||||
borderColor: withAlpha(ACCENT, 0.35),
|
||||
backgroundColor: "transparent",
|
||||
borderWidth: 1,
|
||||
pointRadius: 0,
|
||||
fill: false,
|
||||
tension: 0.3
|
||||
},
|
||||
{
|
||||
label: "Median brew",
|
||||
data: median,
|
||||
@@ -522,7 +543,12 @@
|
||||
boxWidth: 10,
|
||||
boxHeight: 10,
|
||||
usePointStyle: true,
|
||||
font: { size: 11 }
|
||||
font: { size: 11 },
|
||||
// The max curve is the helper the band
|
||||
// fills to, not a series of its own.
|
||||
filter: function (item, data) {
|
||||
return item.text !== "Range max";
|
||||
}
|
||||
}
|
||||
},
|
||||
tooltip: {
|
||||
@@ -533,6 +559,9 @@
|
||||
return "range " + point.y +
|
||||
" to " + (point.yMax || point.y);
|
||||
}
|
||||
if (ctx.datasetIndex === 1) {
|
||||
return null;
|
||||
}
|
||||
return "median " + point.y;
|
||||
}
|
||||
}
|
||||
@@ -569,6 +598,26 @@
|
||||
.catch(function (err) { console.warn(err); });
|
||||
}
|
||||
|
||||
/* ------------------------------------------------- shared run payload */
|
||||
|
||||
// The analysis page has two charts that both need /api/runs, and the
|
||||
// payload contains every reading of every run. Fetch it once and
|
||||
// share it rather than making two identical round-trips.
|
||||
var runsPromise = null;
|
||||
|
||||
function fetchRuns() {
|
||||
if (!runsPromise) {
|
||||
runsPromise = fetch("/api/runs", { headers: { "Accept": "application/json" } })
|
||||
.then(function (r) { return r.json(); })
|
||||
.catch(function (err) {
|
||||
console.warn(err);
|
||||
runsPromise = null;
|
||||
return [];
|
||||
});
|
||||
}
|
||||
return runsPromise;
|
||||
}
|
||||
|
||||
function init() {
|
||||
drawSparklines();
|
||||
drawRunChart();
|
||||
|
||||
@@ -164,7 +164,9 @@
|
||||
<tbody>
|
||||
{% for row in suspect %}
|
||||
<tr>
|
||||
<td class="mono">{{ row.beer_run_id }}</td>
|
||||
<td class="mono">
|
||||
<a href="{{ url_for('brews.run_detail', run_id=row.beer_run_id) }}">{{ row.beer_run_id }}</a>
|
||||
</td>
|
||||
<td class="num">{{ '%.3f' % (row.og_points / 1000) }}</td>
|
||||
<td class="num"><span class="chip chip--warn">{{ '%.3f' % (row.fg_points / 1000) }}</span></td>
|
||||
<td class="comment">{{ row.problem }}</td>
|
||||
|
||||
@@ -70,7 +70,6 @@ def deep():
|
||||
"attenuation": attenuation_percent(run),
|
||||
"days": summary["days_in_fermenter"],
|
||||
"days_to_1010": days_to_target(run, 1010),
|
||||
"days_to_1015": days_to_target(run, 1015),
|
||||
"readings": summary["reading_count"],
|
||||
"curve": summary["curve"],
|
||||
# A suspect run is shown in the table so it can be corrected,
|
||||
@@ -78,9 +77,6 @@ def deep():
|
||||
"suspect": run.beer_run_id in suspect_ids,
|
||||
})
|
||||
|
||||
# The rows with a usable curve, for the overlay chart.
|
||||
curves = [r for r in rows if r["curve"]]
|
||||
|
||||
# Only the trustworthy runs feed the statistics.
|
||||
good = [r for r in rows if not r["suspect"]]
|
||||
|
||||
@@ -109,7 +105,6 @@ def deep():
|
||||
return render_template(
|
||||
"analysis.html",
|
||||
rows=rows,
|
||||
curves=curves,
|
||||
suspect=suspect,
|
||||
stats={
|
||||
"count": len(rows),
|
||||
|
||||
@@ -49,6 +49,10 @@ class ReadingForm(FlaskForm):
|
||||
beer_run_type = StringField("Beer / cider name",
|
||||
validators=[Optional(), Length(max=120)])
|
||||
Date = DateField("Reading date", validators=[DataRequired()])
|
||||
# The floor is deliberately lower than is_plausible_sg's. An
|
||||
# out-of-range reading is SAVED with a warning rather than rejected:
|
||||
# the brewer may have mis-typed it, and seeing it recorded is how they
|
||||
# spot and correct the mistake.
|
||||
sg = IntegerField("Gravity reading (points)",
|
||||
validators=[DataRequired(), NumberRange(min=900, max=1500)])
|
||||
final_reading = BooleanField("This is the final reading")
|
||||
@@ -82,6 +86,19 @@ def index():
|
||||
if form.validate_on_submit():
|
||||
sg_points = int(form.sg.data)
|
||||
if not is_plausible_sg(sg_points):
|
||||
if sg_points < 1000:
|
||||
# Beer cannot be less dense than water, so this is always
|
||||
# a mis-entry - and a mis-typed "1.012" as "012" is the
|
||||
# likely cause. It is still saved so the mistake is
|
||||
# visible and can be corrected.
|
||||
flash(
|
||||
f"{sg_points} is below 1.000 — beer cannot be less dense "
|
||||
"than water, so this is almost certainly a typo (a 1.012 "
|
||||
"reading entered as 012?). It was saved anyway; correct "
|
||||
"it on the run's page.",
|
||||
"warning",
|
||||
)
|
||||
else:
|
||||
flash(
|
||||
f"{sg_points} is outside the usual hydrometer range "
|
||||
"(990-1200). It was saved anyway — double-check the reading.",
|
||||
|
||||
+117
-1
@@ -8,7 +8,7 @@ from pathlib import Path
|
||||
import pytest
|
||||
|
||||
from brewing import (Run, attenuation_percent, days_to_target,
|
||||
fermentation_curve)
|
||||
fermentation_curve, implausible_runs)
|
||||
from conftest import seed_readings
|
||||
|
||||
|
||||
@@ -392,3 +392,119 @@ def test_the_charts_skip_suspect_runs():
|
||||
assert charts_js.count("!r.suspect") >= 2, (
|
||||
"the charts must filter out the runs flagged as suspect"
|
||||
)
|
||||
|
||||
# ------------------------------------------------- rising gravity #
|
||||
|
||||
def test_attenuation_is_none_when_the_gravity_went_up():
|
||||
"""A negative attenuation is not a number a brewer can act on.
|
||||
|
||||
The guard rejected a final gravity below 1.000 and one implying more
|
||||
than 100% attenuation, but a run whose gravity ROSE produced a
|
||||
negative figure that was averaged into the headline number.
|
||||
"""
|
||||
run = Run.from_docs("30", [
|
||||
_doc(sg="1010", date_="2026-01-01"),
|
||||
_doc(sg="1020", date_="2026-01-20", final="True"),
|
||||
])
|
||||
assert attenuation_percent(run) is None
|
||||
|
||||
|
||||
def test_a_rising_gravity_run_is_reported_as_implausible():
|
||||
"""It must be listed for review, not just quietly dropped."""
|
||||
run = Run.from_docs("31", [
|
||||
_doc(sg="1010", date_="2026-01-01"),
|
||||
_doc(sg="1020", date_="2026-01-20", final="True"),
|
||||
])
|
||||
problems = implausible_runs([run])
|
||||
assert len(problems) == 1
|
||||
assert problems[0]["beer_run_id"] == "31"
|
||||
assert "went up" in problems[0]["problem"]
|
||||
|
||||
|
||||
# ------------------------------------------------- undated readings #
|
||||
|
||||
def test_the_original_gravity_comes_from_the_first_dated_reading():
|
||||
"""An undated reading cannot be placed on the timeline.
|
||||
|
||||
It used to sort to the front, so it became the OG - which put the OG
|
||||
after the FG and produced a negative attenuation.
|
||||
"""
|
||||
run = Run.from_docs("32", [
|
||||
_doc(sg="1050", date_="2026-01-01"),
|
||||
{"_id": "x", "beer_run_id": "32", "beer_type": "Ale", "sg": "1005",
|
||||
"date": None, "final_reading": "False", "comment": ""},
|
||||
_doc(sg="1010", date_="2026-01-25", final="True"),
|
||||
])
|
||||
assert run.original_gravity() == 1050
|
||||
assert run.final_gravity() == 1010
|
||||
assert attenuation_percent(run) is not None
|
||||
assert attenuation_percent(run) > 0
|
||||
|
||||
|
||||
def test_days_to_target_skips_an_undated_reading():
|
||||
"""One undated reading must not lose the whole figure.
|
||||
|
||||
The function gave up and returned None when the first reading at or
|
||||
below the target had no date, even when a later reading did.
|
||||
"""
|
||||
run = Run.from_docs("33", [
|
||||
_doc(sg="1050", date_="2026-01-01"),
|
||||
{"_id": "y", "beer_run_id": "33", "beer_type": "Ale", "sg": "1005",
|
||||
"date": None, "final_reading": "False", "comment": ""},
|
||||
_doc(sg="1010", date_="2026-01-25", final="True"),
|
||||
])
|
||||
assert days_to_target(run, 1010) == 24
|
||||
|
||||
|
||||
# ------------------------------------------------------- the charts #
|
||||
|
||||
def test_the_curve_chart_excludes_active_and_suspect_runs():
|
||||
"""Only finished, plausible brews belong on the curve.
|
||||
|
||||
An active run's curve is a partial fermentation, and the curve is
|
||||
meant to describe how a completed fermentation behaves.
|
||||
"""
|
||||
charts_js = (
|
||||
Path(__file__).resolve().parents[1]
|
||||
/ "src" / "flask" / "static" / "js" / "charts.js"
|
||||
).read_text()
|
||||
|
||||
assert "!r.active" in charts_js, (
|
||||
"the curve chart must leave the active runs out"
|
||||
)
|
||||
|
||||
|
||||
def test_the_curve_band_is_filled_to_the_max_not_the_median():
|
||||
"""The shaded band has to span min to max.
|
||||
|
||||
With one dataset and `fill: "+1"` the band was filled to the NEXT
|
||||
dataset - which was the median line, so the upper half of the range
|
||||
was drawn as a hairline but never shaded, and the tooltip disagreed
|
||||
with the shading.
|
||||
"""
|
||||
charts_js = (
|
||||
Path(__file__).resolve().parents[1]
|
||||
/ "src" / "flask" / "static" / "js" / "charts.js"
|
||||
).read_text()
|
||||
|
||||
assert "envelopeMax" in charts_js, (
|
||||
"the band needs a separate max dataset to fill down to"
|
||||
)
|
||||
assert 'label: "Range max"' in charts_js
|
||||
|
||||
|
||||
def test_the_charts_fetch_the_run_payload_once():
|
||||
"""/api/runs carries every reading of every run.
|
||||
|
||||
Four charts each fetched it independently, so the analysis page made
|
||||
two identical round-trips of the whole collection.
|
||||
"""
|
||||
charts_js = (
|
||||
Path(__file__).resolve().parents[1]
|
||||
/ "src" / "flask" / "static" / "js" / "charts.js"
|
||||
).read_text()
|
||||
|
||||
assert charts_js.count('fetch("/api/runs"') == 1, (
|
||||
"the run payload should be fetched once and shared"
|
||||
)
|
||||
assert "function fetchRuns()" in charts_js
|
||||
|
||||
+25
-1
@@ -186,11 +186,16 @@ def test_logout_clears_the_session(logged_in):
|
||||
|
||||
|
||||
def test_out_of_range_reading_is_saved_but_warned(logged_in, repo):
|
||||
"""A reading outside the range is SAVED with a warning.
|
||||
|
||||
Rejecting it would lose the brewer's actual observation, and seeing it
|
||||
recorded is how they spot and correct a typo.
|
||||
"""
|
||||
response = logged_in.post("/brews", data={
|
||||
"beer_run_id": "12",
|
||||
"beer_run_type": "Odd",
|
||||
"Date": "2026-05-01",
|
||||
"sg": "950",
|
||||
"sg": "1250",
|
||||
}, follow_redirects=True)
|
||||
assert response.status_code == 200
|
||||
assert b"outside the usual hydrometer range" in response.data
|
||||
@@ -198,6 +203,25 @@ def test_out_of_range_reading_is_saved_but_warned(logged_in, repo):
|
||||
assert repo.readings.find_one({"beer_run_id": "12"}) is not None
|
||||
|
||||
|
||||
def test_a_reading_below_water_says_it_is_always_a_typo(logged_in, repo):
|
||||
"""A gravity below 1.000 cannot be real, so the warning says so.
|
||||
|
||||
This is the run 12 case: a 1.012 reading entered as "012". The
|
||||
reading is still saved, but the message names the likely cause.
|
||||
"""
|
||||
response = logged_in.post("/brews", data={
|
||||
"beer_run_id": "13",
|
||||
"beer_run_type": "Typo",
|
||||
"Date": "2026-05-01",
|
||||
"sg": "900",
|
||||
}, follow_redirects=True)
|
||||
assert response.status_code == 200
|
||||
assert b"below 1.000" in response.data
|
||||
assert b"typo" in response.data
|
||||
# Still saved, so the mistake is visible and can be corrected.
|
||||
assert repo.readings.find_one({"beer_run_id": "13"}) is not None
|
||||
|
||||
|
||||
# ------------------------------------------------------- security: passwords #
|
||||
|
||||
def test_legacy_plaintext_password_is_accepted_and_migrated(repo):
|
||||
|
||||
Reference in New Issue
Block a user