fix(advice): the remaining review findings

Four things from the earlier review of this PR.

The prompt test did not test the prompt
---------------------------------------
`test_the_advice_is_about_the_reading_that_was_saved` asserted which
dict was PASSED to the generator - not what the model was actually ASKED.
So it passed even while the prompt still said "give advice for the most
recent reading", which is the bug that made the model advise about the
wrong number while the advice was filed against the right one.

It now asserts on the prompt content: the reading is marked, its date is
named, and "most recent reading" is gone.

Verified by reverting the prompt fix - the test fails, and so does
`test_the_prompt_names_the_reading_to_advise_about`.  Two tests now
guard it, and reverting the fix breaks both.

A write that matches nothing was silent
--------------------------------------
The advice writer discarded the return value of `set_reading_advice`.  A
write that matched 0 documents - which is now the EXPECTED outcome when
the brewer corrected the reading while the advice was being generated -
was dropped without a word.  It is logged now.

A stale docstring
-----------------
The background thread "builds its own" client - it has used the shared
one since the leak was fixed.

A missing CSS class
-------------------
`advice--reading` is used on the active-beers card and the run detail
page, and was never defined, so it had no styles.

173 tests (unchanged - the new assertions were added to existing tests).
This commit is contained in:
2026-11-09 17:47:14 +10:00
parent 3e1e4f42b7
commit 8da3601941
3 changed files with 34 additions and 2 deletions
+6
View File
@@ -716,6 +716,12 @@ table.data .comment { color: var(--text-muted); font-size: .84rem; }
color: var(--amber-text);
}
/* The advice on the active-beers card and the run detail page - the
full text, not the clamped table cell. */
.advice--reading {
margin-top: .5rem;
}
/* Advice that could not be generated - it says why. */
.advice--failed {
border-left-color: var(--danger);
+14 -2
View File
@@ -60,7 +60,8 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
comment is never touched.
The background thread cannot use the request-scoped repository - that
client is closed when the request ends - so it builds its own.
client is closed when the request ends - so it uses the shared
process-wide one.
"""
if not reading_id:
return
@@ -130,7 +131,18 @@ def _start_advice(run_id: str, reading_id: str, corrected: bool = False) -> None
try:
shared = get_shared_repository()
shared.set_reading_advice(read_id, text, ok, expected_sg=advice_sg)
written = shared.set_reading_advice(read_id, text, ok,
expected_sg=advice_sg)
if not written:
# The reading was corrected while the advice was being
# generated, so the advice no longer matches it. That is
# the guard working - the fresh advice for the corrected
# reading will land in its own turn.
log.info(
"the advice for reading %s was not stored: the reading "
"no longer holds the gravity it was written for",
read_id,
)
except Exception: # noqa: BLE001 - never let the thread die noisily
log.exception("could not store the generated advice")