Compare commits
4
Commits
a38febaca8
...
master
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8e4770f870 | ||
|
|
fb1088ceec | ||
|
|
cd45cc2f17 | ||
|
|
e603f23928 |
@@ -97,6 +97,19 @@ def create_app(config: dict | None = None) -> Flask:
|
|||||||
if response.mimetype == "text/html":
|
if response.mimetype == "text/html":
|
||||||
response.headers["Cache-Control"] = "no-store, must-revalidate"
|
response.headers["Cache-Control"] = "no-store, must-revalidate"
|
||||||
response.headers["Pragma"] = "no-cache"
|
response.headers["Pragma"] = "no-cache"
|
||||||
|
elif response.mimetype in ("text/css", "application/javascript",
|
||||||
|
"text/javascript"):
|
||||||
|
# The static files must NEVER be reused from a cache.
|
||||||
|
|
||||||
|
# Firefox kept serving a stale stylesheet even with
|
||||||
|
# `no-cache`, which meant the page had the new markup and
|
||||||
|
# the old CSS: the advice was expanded inline in the table
|
||||||
|
# instead of in a pop-out, and the long and short labels
|
||||||
|
# were both showing ("BREWING ADVICEADVICE").
|
||||||
|
|
||||||
|
# `no-store` is the only one that means "do not keep this".
|
||||||
|
response.headers["Cache-Control"] = "no-store, must-revalidate"
|
||||||
|
response.headers["Pragma"] = "no-cache"
|
||||||
return response
|
return response
|
||||||
|
|
||||||
csrf = CSRFProtect(app)
|
csrf = CSRFProtect(app)
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ Two collections are used:
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import re
|
||||||
import os
|
import os
|
||||||
import threading
|
import threading
|
||||||
from typing import Any
|
from typing import Any
|
||||||
@@ -232,7 +233,8 @@ class BeerRepository:
|
|||||||
def set_reading_advice(self, reading_id: str, advice: str,
|
def set_reading_advice(self, reading_id: str, advice: str,
|
||||||
ok: bool = True,
|
ok: bool = True,
|
||||||
expected_sg: int | None = None,
|
expected_sg: int | None = None,
|
||||||
expected_token: str | None = None) -> int:
|
expected_token: str | None = None,
|
||||||
|
expected_comment: str | None = None) -> int:
|
||||||
"""Attach the LLM's advice to a reading.
|
"""Attach the LLM's advice to a reading.
|
||||||
|
|
||||||
The advice lives in its own field, so the brewer's own comment is
|
The advice lives in its own field, so the brewer's own comment is
|
||||||
@@ -286,6 +288,31 @@ class BeerRepository:
|
|||||||
# A direct call with a gravity to check against.
|
# A direct call with a gravity to check against.
|
||||||
query["sg"] = str(expected_sg)
|
query["sg"] = str(expected_sg)
|
||||||
|
|
||||||
|
if expected_comment is not None:
|
||||||
|
# The comment the advice was written for.
|
||||||
|
#
|
||||||
|
# This is a SECOND, independent guard. The token covers
|
||||||
|
# every edit the application makes, but a document edited
|
||||||
|
# outside the application - by a script, or by hand in a
|
||||||
|
# database tool - would keep its empty token and a stale
|
||||||
|
# thread's write would still match on the gravity alone.
|
||||||
|
#
|
||||||
|
# The advice is generated from the comment as well as the
|
||||||
|
# gravity, so the comment is part of what the advice
|
||||||
|
# describes. Matching on it means a stale write can only
|
||||||
|
# land if BOTH the gravity and the comment are unchanged.
|
||||||
|
#
|
||||||
|
# The stored comment and the one the view sees can differ by
|
||||||
|
# trailing whitespace, so a regex is used to match either.
|
||||||
|
#
|
||||||
|
# `^comment\s*$` matches the stored value with any trailing
|
||||||
|
# whitespace, and the exact value too. It is anchored, so it
|
||||||
|
# cannot match a longer comment that merely starts the same
|
||||||
|
# way.
|
||||||
|
query["comment"] = {
|
||||||
|
"$regex": "^" + re.escape(expected_comment.strip()) + r"\s*$"
|
||||||
|
}
|
||||||
|
|
||||||
# `$in` matches either form. A plain list would be an exact
|
# `$in` matches either form. A plain list would be an exact
|
||||||
# ARRAY match in Mongo, which is not what is wanted here.
|
# ARRAY match in Mongo, which is not what is wanted here.
|
||||||
result = self.readings.update_one(
|
result = self.readings.update_one(
|
||||||
|
|||||||
@@ -999,10 +999,21 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; }
|
|||||||
|
|
||||||
/* The script opens and closes the panel, and places it. The CSS only
|
/* The script opens and closes the panel, and places it. The CSS only
|
||||||
decides whether it is shown. */
|
decides whether it is shown. */
|
||||||
|
/* The panel is opened by the script, which removes the `hidden`
|
||||||
|
attribute and adds `is-open`.
|
||||||
|
|
||||||
|
The `hidden` attribute is what makes this fail-safe: if the
|
||||||
|
stylesheet is stale and does not know about `.advice-pop` at all,
|
||||||
|
the panel is STILL hidden, so the advice cannot appear expanded
|
||||||
|
inside the table cell. */
|
||||||
.advice-pop.is-open {
|
.advice-pop.is-open {
|
||||||
display: block;
|
display: block;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.advice-pop[hidden] {
|
||||||
|
display: none;
|
||||||
|
}
|
||||||
|
|
||||||
/* On a phone there is no hover, so the trigger is a button the script
|
/* On a phone there is no hover, so the trigger is a button the script
|
||||||
toggles, and the panel is pinned to the middle of the screen so it is
|
toggles, and the panel is pinned to the middle of the screen so it is
|
||||||
readable whatever the table's width. */
|
readable whatever the table's width. */
|
||||||
|
|||||||
@@ -131,6 +131,11 @@
|
|||||||
clearTimeout(closeTimer);
|
clearTimeout(closeTimer);
|
||||||
closeTimer = null;
|
closeTimer = null;
|
||||||
}
|
}
|
||||||
|
/* The `hidden` attribute is removed as well as the
|
||||||
|
class added, so the panel is shown even if the
|
||||||
|
stylesheet is an older one that only knows about
|
||||||
|
the class. */
|
||||||
|
panel.removeAttribute("hidden");
|
||||||
panel.classList.add("is-open");
|
panel.classList.add("is-open");
|
||||||
trigger.setAttribute("aria-expanded", "true");
|
trigger.setAttribute("aria-expanded", "true");
|
||||||
place();
|
place();
|
||||||
@@ -138,6 +143,9 @@
|
|||||||
|
|
||||||
function close() {
|
function close() {
|
||||||
panel.classList.remove("is-open");
|
panel.classList.remove("is-open");
|
||||||
|
/* Put the attribute back, so a stale stylesheet
|
||||||
|
cannot leave the advice expanded in the cell. */
|
||||||
|
panel.setAttribute("hidden", "");
|
||||||
trigger.setAttribute("aria-expanded", "false");
|
trigger.setAttribute("aria-expanded", "false");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -126,7 +126,7 @@
|
|||||||
{% endif %}
|
{% endif %}
|
||||||
</span>
|
</span>
|
||||||
</button>
|
</button>
|
||||||
<div class="advice-pop" role="tooltip">
|
<div class="advice-pop" role="tooltip" hidden>
|
||||||
<div class="advice-pop__head">
|
<div class="advice-pop__head">
|
||||||
<span class="advice__label">Brewing advice</span>
|
<span class="advice__label">Brewing advice</span>
|
||||||
<button type="button" class="advice-pop__close"
|
<button type="button" class="advice-pop__close"
|
||||||
|
|||||||
@@ -127,6 +127,11 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
|
|||||||
# cannot overwrite a correction with advice about the old value.
|
# cannot overwrite a correction with advice about the old value.
|
||||||
advice_sg = latest.get("sg")
|
advice_sg = latest.get("sg")
|
||||||
advice_token = latest.get("advice_token")
|
advice_token = latest.get("advice_token")
|
||||||
|
# The comment is part of what the advice describes, so it is a
|
||||||
|
# second, independent guard on the write. The token covers every
|
||||||
|
# edit the application makes; this also covers a document edited
|
||||||
|
# outside it.
|
||||||
|
advice_comment = latest.get("comment") or ""
|
||||||
|
|
||||||
def write(read_id: str, ok: bool, text: str) -> None:
|
def write(read_id: str, ok: bool, text: str) -> None:
|
||||||
# The client is SHARED, not built per reading. A MongoClient
|
# The client is SHARED, not built per reading. A MongoClient
|
||||||
@@ -145,6 +150,7 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
|
|||||||
read_id, text, ok,
|
read_id, text, ok,
|
||||||
expected_sg=advice_sg,
|
expected_sg=advice_sg,
|
||||||
expected_token=advice_token,
|
expected_token=advice_token,
|
||||||
|
expected_comment=advice_comment,
|
||||||
)
|
)
|
||||||
if not written:
|
if not written:
|
||||||
# The reading was corrected while the advice was being
|
# The reading was corrected while the advice was being
|
||||||
@@ -153,8 +159,8 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
|
|||||||
# reading will land in its own turn.
|
# reading will land in its own turn.
|
||||||
log.info(
|
log.info(
|
||||||
"the advice for reading %s was not stored: the reading "
|
"the advice for reading %s was not stored: the reading "
|
||||||
"has been edited since the advice was generated, so the "
|
"has been edited since the advice was generated, so its "
|
||||||
"advice no longer matches it",
|
"editing token or comment no longer matches the advice",
|
||||||
read_id,
|
read_id,
|
||||||
)
|
)
|
||||||
except Exception: # noqa: BLE001 - never let the thread die noisily
|
except Exception: # noqa: BLE001 - never let the thread die noisily
|
||||||
|
|||||||
@@ -1121,6 +1121,70 @@ def test_stale_advice_cannot_overwrite_a_comment_only_correction(repo):
|
|||||||
assert doc["comment"] == "corrected comment"
|
assert doc["comment"] == "corrected comment"
|
||||||
|
|
||||||
|
|
||||||
|
def test_stale_advice_cannot_overwrite_a_comment_edited_outside_the_app(repo):
|
||||||
|
"""The comment is a guard in its own right, not just the token.
|
||||||
|
|
||||||
|
The token covers every edit the application makes. A document
|
||||||
|
edited outside it - by a script, or by hand - keeps its empty
|
||||||
|
token, so a stale thread's write would still match on the gravity
|
||||||
|
alone. Matching the comment as well closes that.
|
||||||
|
"""
|
||||||
|
repo.readings.insert_one({
|
||||||
|
"_id": "outsideedit01",
|
||||||
|
"beer_run_id": "1",
|
||||||
|
"date": "2026-01-01",
|
||||||
|
"sg": "1040",
|
||||||
|
"comment": "original comment",
|
||||||
|
"final_reading": "False",
|
||||||
|
# No advice_token - a legacy reading.
|
||||||
|
})
|
||||||
|
|
||||||
|
# The advice is written for the original comment.
|
||||||
|
assert repo.set_reading_advice(
|
||||||
|
"outsideedit01", "FRESH ADVICE", ok=True,
|
||||||
|
expected_token="", expected_comment="original comment") == 1
|
||||||
|
|
||||||
|
# Somebody edits the comment directly in the database, leaving the
|
||||||
|
# token empty - so the token cannot tell the generations apart.
|
||||||
|
repo.readings.update_one(
|
||||||
|
{"_id": "outsideedit01"},
|
||||||
|
{"$set": {"comment": "edited outside the app"}},
|
||||||
|
)
|
||||||
|
|
||||||
|
# The slow thread lands. It must be refused, even though the token
|
||||||
|
# still matches.
|
||||||
|
assert repo.set_reading_advice(
|
||||||
|
"outsideedit01", "STALE ADVICE", ok=True,
|
||||||
|
expected_token="", expected_comment="original comment") == 0
|
||||||
|
|
||||||
|
doc = repo.readings.find_one({"_id": "outsideedit01"})
|
||||||
|
assert doc["advice"] == "FRESH ADVICE"
|
||||||
|
assert doc["comment"] == "edited outside the app"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_advice_pop_out_is_hidden_in_the_markup(client, repo):
|
||||||
|
"""The pop-out carries the `hidden` attribute.
|
||||||
|
|
||||||
|
A stale stylesheet - one that does not know about `.advice-pop` at
|
||||||
|
all - would otherwise leave the advice expanded inline in the table
|
||||||
|
cell, running down the column and off the screen. The `hidden`
|
||||||
|
attribute makes the markup fail-safe: the panel stays hidden until
|
||||||
|
the script opens it.
|
||||||
|
"""
|
||||||
|
repo.users.insert_one({"username": "beer", "password": "beer_pass"})
|
||||||
|
client.post("/login", data={"username": "beer", "password": "beer_pass"},
|
||||||
|
follow_redirects=True)
|
||||||
|
|
||||||
|
seed_readings(repo, [("1", "Ale", "1042", "2026-01-01", False, "")])
|
||||||
|
doc = repo.readings.find_one({"beer_run_id": "1"})
|
||||||
|
repo.set_reading_advice(str(doc["_id"]), "Warm it to 21C and wait.", ok=True)
|
||||||
|
|
||||||
|
body = client.get("/brews").get_data(as_text=True)
|
||||||
|
|
||||||
|
assert 'class="advice-pop"' in body
|
||||||
|
assert 'hidden' in body.split('class="advice-pop"')[1][:60]
|
||||||
|
|
||||||
|
|
||||||
def test_a_failed_advice_is_marked_as_such(client, repo):
|
def test_a_failed_advice_is_marked_as_such(client, repo):
|
||||||
"""A generation failure reads differently from real advice."""
|
"""A generation failure reads differently from real advice."""
|
||||||
repo.users.insert_one({"username": "beer", "password": "beer_pass"})
|
repo.users.insert_one({"username": "beer", "password": "beer_pass"})
|
||||||
|
|||||||
Reference in New Issue
Block a user