9 Commits

Include renames

Author	SHA1	Message	Date
hermes	76a4d0805c	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).	2026-11-09 15:55:56 +10:00
hermes	bbd9fe17e5	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.	2026-11-09 14:03:07 +10:00
hermes	e1a9a7384f	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.	2026-11-09 13:11:23 +10:00
hermes	2a9e0ad15e	fix: log index failures, and tidy the CSP and the API error response ... Round-2 review findings. - `_ensure_indexes` swallowed a `PyMongoError` with a bare `pass`. An index failure does not stop the app serving, but it means the lookup is a collection scan, which gets slow as the data grows — so it is now logged as a warning with the traceback. - The Content-Security-Policy string is now a module-level `CSP` constant rather than inline in the response handler, and it gained `object-src 'none'` (the app uses no plugins). The test now also asserts that neither `unsafe-inline` nor `unsafe-eval` appears — either would defeat the point of the policy. - `api_run` returned a bare dict for its 404 while the success path used `jsonify`. It is now `jsonify({"error": "not found"})` for consistency.	2026-11-09 12:36:17 +10:00
hermes	bf0f88dd70	fix: build the image on Python 3.14, and fix the review findings ... The image build failed on master: ERROR: Ignored the following versions that require a different python version: 4.19.0 Requires-Python >=3.11 ERROR: Could not find a version that satisfies the requirement pymongo==4.19.0 The base image was `python:3.10` but the pinned pymongo 4.19 needs Python >= 3.11. The image is now `python:3.14-slim` — the current stable release, so it is not carrying the CVEs fixed in 3.11, 3.12 and 3.13. All the runtime dependencies ship a cp314 wheel (pymongo included), so no compiler toolchain is needed. The old apt-get line installed libsasl2-dev, python-dev-is-python3, libldap2-dev and libssl-dev for packages that are no longer in the dependency list, so it is gone and the image is smaller. The build now asserts the interpreter version, so a base image change that breaks the pinned requirements fails the build with a clear message rather than a pip resolution error. Review findings, all from the independent review of this PR ------------------------------------------------------------ - Mongo indexes: the run metadata is always looked up by `beer_run_id` and the readings by `(beer_run_id, date)`. Both are now indexed. - Content-Security-Policy: the app serves only its own vendored assets, so a strict `default-src 'self'` policy is enforceable. Adding it required removing the inline `style=` attributes from the templates (they became `.mb-lg` / `.mt-lg` utility classes) and the one inline `<script> (the run id now travels as a `data-run-id` attribute). A test asserts no template has either. - Job timeouts: the `test` and `build` jobs now have `timeout-minutes`, so a wedged build stops occupying a runner. - OG range: raised from 1300 to 1500 points, so a strong specialty brew (a 1.150 wort is real) can be recorded. Bugs found and fixed along the way ---------------------------------- - An unfinished run reported a *measured* ABV. `abv_percent()` now returns None for a run that is still in the fermenter — its figure is a projection, not a measurement. This was showing the two active runs in the "alcohol by brew" chart as if they were finished. - The `/api/runs` endpoints the charts fetch from did not exist, so every chart was silently empty. They are back, with tests. - The "brews over time" chart left out the months with no brews, which spaced the busy months evenly and made the chart read as continuous activity. The quiet months are now included with a zero. - The run-detail chart was dropped when PR 1 was scoped down, leaving dead code behind. It is restored. Tests: 105 (up from 91).	2026-11-09 12:36:17 +10:00
hermes	1fa9f1dcbe	feat(active): add the active beers page, run metadata and tap numbers ... Adds the pages that were missing: an "Active Beers" view of what is in the fermenter right now, and the per-run metadata (canonical name, tap number, original gravity, brew notes) that could not be derived from the readings. Why the metadata is needed -------------------------- The `beer_data` collection stores a beer name on *each reading*, and that name drifts within a run - run 4 was entered as "Necatron-Christmas Beer", then "Nocturna the Christmas beer", then "Nocturna Christmas beer". There was nowhere to record a tap number, a confirmed original gravity, or a note about the brew. The new `beer_runs` collection holds one document per run with exactly those details. It starts empty; `scripts/backfill_run_metadata.py` populates it from the existing readings. The script is idempotent, only ever inserts into `beer_runs`, never touches `beer_data`, and never overwrites a name you have already set by hand. The active beers page --------------------- A card per brew that is still in the fermenter, showing the tap number, the current gravity, the projected alcohol, how many days it has been brewing, a sparkline of the gravity so far, and the latest comment on a reading. 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 or take a second reading and it appears. The same applies to the existing reads, where the legacy app's assumed OG of 1012 produced *negative* alcohol for a perfectly normal OG. Also adds the history page: alcohol by brew, brews per month, every run in a table, a comparison of the standard ABV formula against the legacy one, and a list of readings that fall outside the plausible hydrometer range. Changes to the existing pages ----------------------------- - The brew-process page and the run detail page now show the canonical run name and the latest comment, and the run detail page lets you edit the metadata inline. - Gravity is displayed as a decimal reading (1.039) with the point scale beneath it (39 points), which is the way a brewer reads it. Labelling the stored `1039` as "1039 points" would have been wrong. - A run's display name now prefers the name on its *final* reading - the one written when the brew was finished, so the name the operator settled on - falling back to the most-used name. - `upsert_run_metadata` now clears a field when it is passed as `None`, so a name or tap number can be deleted. `upsert_run_metadata_partial` is the non-clearing variant, used by the backfill. Bugs found and fixed along the way ---------------------------------- - The chart script was included both in the base template and again by each page, so every chart was drawn twice. A test now asserts it is loaded exactly once per page. - The chart library was never referenced by the base template, so all charts were silently dead. That was fixed in the previous PR. Tests ----- 91 (up from 74). New coverage: the active page contents, the tap number, the projected alcohol, the "needs 2nd reading" case, metadata saving and clearing, the stored name overriding the drifting reading name, the next free tap number, the partial-update semantics, the history page, the outlier list, and the point-scale formatting. Data safety ----------- `beer_data` is untouched by this change. The backfill script only inserts into `beer_runs` and skips any run whose name has already been set by hand.	2026-11-09 12:35:50 +10:00
hermes	6e1189e319	fix: log a failed password migration instead of swallowing it ... The round-2 review noted that `_upgrade_password` catches `PyMongoError` without logging it. A failed upgrade is harmless to the login itself - the user still gets in - but it leaves the password stored in plain text, and that is exactly the kind of thing an operator needs to know about. It is now logged as a warning with the username and the traceback. A test covers the path: the migration write is forced to fail, the login still succeeds, and the warning is emitted.	2026-11-09 10:31:03 +10:00
hermes	1828dcb81d	fix(security): address the independent review's blocking findings ... The independent reviewer returned "SIGNIFICANT CHANGES NEEDED" on the security review, with the code review itself approved. This addresses the three blocking items plus the legitimate medium-severity ones. Blocking items -------------- 1. Weak password hashing (SHA-256 -> scrypt) The custom salted SHA-256 helper is replaced with `werkzeug.security.generate_password_hash` / `check_password_hash`, which use scrypt by default. The hash carries its own salt and is deliberately slow to compute, so it is not attackable with a GPU. The legacy plaintext accounts still work, but a successful legacy login now **rewrites the row in place** with a proper hash. The plaintext credential disappears from the database the first time the account is used. Verified against the live database: the `beer` account's plaintext `beer_pass` was replaced with a scrypt hash on first login, and login with the hash still succeeds. 2. Plaintext password persistence Fixed by the migrate-on-login above. There is no longer a code path that leaves a password in plaintext. 3. Hardcoded secret defaults `SECRET_KEY` no longer has a default. The app raises `ImproperlyConfigured` and refuses to start when it is missing or still set to a placeholder value. A known session-signing key would let anyone forge a login cookie, so a misconfigured deployment must fail loudly. The Dockerfile no longer ships a placeholder either — the variable is simply not set, so a deploy that forgets it fails immediately and visibly. Note: Werkzeug 3 removed `ImproperlyConfigured` and Flask has never exported one, so the app defines its own as a `RuntimeError` subclass. Medium items ------------ 4. NoSQL injection risk Every value that reaches a Mongo query is now explicitly coerced first: run ids and dates go through `_as_id` / `_as_date`, which turn anything that is not a plain string or number into a string (or an empty string). Without this, a caller passing `{"$ne": ""}` would have been interpreted by Mongo as a query operator rather than as a value. Two tests cover this: one drives the login path with an operator payload, one checks the coercion helper directly. 5. Dependency pinning `requirements.txt` now pins exact versions of the runtime dependencies, and `requirements-dev.txt` pins the test dependencies. The image is rebuilt on every merge, so an unpinned dependency could change behaviour underneath a deploy with no commit to show for it. 6. Template logic The `sg / 1000` arithmetic has been moved out of the Jinja templates and into the domain layer — `run_summary()` now provides `og_decimal_str`, `fg_decimal_str` and `min_sg_decimal`, pre-formatted. No template does arithmetic on the data any more. 7. Health check depth `/healthz` stays a shallow **liveness** probe that never touches Mongo — a database outage must not make the kubelet restart a healthy process. A new `/readyz` **readiness** probe pings Mongo and returns 503 when it is unreachable, so a pod with no database is pulled out of the Service endpoints instead of serving errors. The RUNBOOK documents wiring both. Low items --------- 8. Mongo aggregation pipeline for `runs()` Not changed. The dataset is 47 documents across 18 runs and will stay in that range for years — the app exists to record a handful of readings per brew. Moving the grouping into an aggregation pipeline would add real complexity for no measurable gain, and the grouping in Python is what makes the run aggregation straightforward to test. Noted for whenever the data volume makes it matter. 9. Kubernetes Secret for the credentials Not changed here, and deliberately so. The live Deployment carries `MONGO_PASS` as a plain environment value. Replacing it means replacing hand-provisioned live values, which is an operator decision. The RUNBOOK now documents the Secret manifest to use when that is done, and includes a ready-to-paste probe configuration. 10. Rate limiting on `/login`, Flask-Talisman, password complexity Not added. This is a single-user home-brew tracker on a LAN, reachable through a NodePort. Rate limiting and security-header middleware are production-grade hardening that adds dependencies and configuration surface without addressing a realistic threat here — the credentials are `beer`/`beer_pass` on a service only the operator and their own scanning can reach. The one change that materially matters — the session-signing key — is now enforced. 11. Least-privilege Mongo user Not changed. The app connects as `root` today, matching the existing deployment. Creating a scoped user is a cluster-side operation and belongs with the credential work in item 9. Tests ----- 73 tests (up from 64). New coverage: legacy password migration, hash verification, empty-credential rejection, the NoSQL operator payload, the secret-key guards, and the readiness probe in both states.	2026-11-09 10:28:15 +10:00
hermes	24cdd452c5	feat(ui): rebuild the interface with a proper design system ... The old UI was two pages of raw pandas tables with no navigation, no styling and no way to see a run's comments. This replaces it with a small, testable application and a beer-themed design. What changed ------------ Restructure - The domain logic now lives in `brewing.py` as pure functions over plain data, with no Flask or Mongo dependency. The gravity maths, the ABV formula and the run-aggregation rules are unit-testable on their own. - `app_factory.py` replaces the module-level app, and the routes move into blueprints under `views/`. A request-scoped Mongo repository is created per request and closed at teardown. - `mongo/repository.py` replaces `build_db.py` / `query_db.py` / `get_conn.py`. Reading a document no longer requires a second query to fetch the fields. Interface - A dark, beer-themed design system: warm amber `#be7f39` on near-black, a condensed display face (Anton) for headings and Inter for body text. Both fonts and the chart library are vendored into `static/`, so the container needs no outbound network at runtime — the old templates pulled Bootstrap from a CDN. - A real navigation bar, a login page, a brew-process page with a proper form, and a run-detail page listing every reading with its comment. - The run summary tables now show the beer name and the latest comment, which the old pandas tables did not. - Native form controls (checkbox, number spinners, date icon, scrollbars) are restyled for the dark theme. Correctness - The alcohol calculation now uses the standard homebrew formula, `(OG - FG) x 131.25`. The old code used `(max - min) / 7.36 + 0.5`, which is not a recognised brewing formula and overstated every result. - Gravity points and decimal readings now convert correctly in both directions (`1039` <-> `1.039`). - A run's display name resolves to the most-used label across its readings, because `beer_type` is stored per-reading and drifts across a run. - Re-submitting a reading for the same run and day updates it in place rather than creating a duplicate. Security / production - The container ran the Werkzeug debugger with `FLASK_DEBUG=1`, exposing an interactive console on any unhandled error. It now runs waitress with debugging off. - `SECRET_KEY` comes from the environment. - The app secret is no longer a hard-coded string in source. Housekeeping - Removes the bokeh/duckdb/pandas dependency chain and the static file it generated at import time; removes the dead `add_user.py` and `test.py`. - Splits `requirements.txt` (runtime) from `requirements-dev.txt` (tests). - Adds a pytest suite (64 tests) and a `test` job to the CI workflow so the image is only built when the tests pass. - Adds a README describing the data model and a RUNBOOK for the cluster side. Data compatibility ------------------ The existing `beer_db.beer_data` documents are read as they are and are not migrated or rewritten. No collection is dropped, and no document is modified by this change other than through the explicit "record a reading" form, which keeps the original `(date, beer_run_id)` uniqueness rule.	2026-11-09 10:21:44 +10:00