fix(analysis): the review findings - rising gravity, undated readings, the band
A second, deeper review of PR #12 found five things the first review did not. All are fixed here. The band was not the band ------------------------- The fermentation curve's shaded range was drawn as ONE dataset with `fill: "+1"`, which fills to the NEXT dataset - and the next dataset was the median line, not the maximum. So the band shaded min-to-median, the upper half of the range was drawn as a hairline but never shaded, and the tooltip said "range min to max" while the picture showed something else. The band is now two datasets - the min curve filled down to a max curve - with the max hidden from the legend. Rising gravity -------------- `attenuation_percent` rejected a final gravity below 1.000 and one implying more than 100% attenuation, but a run whose gravity went UP produced a NEGATIVE attenuation - which was then averaged into the headline figure and plotted as a negative bar. Verified against the live code: OG 1010 -> FG 1020 gave -100.0%. Now rejected, and reported by `implausible_runs` with the reason "the gravity went up, which is usually a mis-ordered pair of readings". Undated readings ---------------- An undated reading sorted to the front of the run, so it became the original gravity - which put the OG after the FG and produced a negative attenuation. The OG now comes from the earliest *dated* reading, and `days_to_target` skips the readings it cannot place on the timeline rather than giving up on the whole run. Verified: readings 1050 (dated), 1005 (undated), 1010 (dated) gave None instead of 24 days, and an OG of 1005 instead of 1050. Active runs on the curve ------------------------ The curve chart filtered on `curve.length > 1` only, so it plotted the active runs as well - a partial fermentation on a chart meant to describe how a completed one behaves. It now excludes them, along with the suspect runs. Four fetches, one payload ------------------------- `/api/runs` carries every reading of every run, and four charts each fetched it independently - so the analysis page made two identical round-trips of the whole collection. There is now one shared fetch. Also ---- - The "Readings that need a second look" table links to the run, so a suspect reading can be corrected in one click. - `days_to_1015` and the `curves` variable were computed and never used. Removed. - A reading below 1.000 now says what it is - "beer cannot be less dense than water, so this is almost certainly a typo (a 1.012 reading entered as 012?)". The reading is still SAVED: rejecting it would lose the brewer's actual observation, and seeing it recorded is how they spot the mistake. 142 tests (up from 134).
This commit is contained in:
+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