The model is now `mistral-large-4:cloud`, on the same Ollama instance.
It was probed before being made the default - `deepseek-v4-flash:cloud`,
`deepseek-v3.2:cloud` and `qwen3.5:cloud` are all retired and fail at
call time while still appearing in the model list.
What the probes showed
----------------------
`think: false` does NOT stop the reasoning. It only changes where the
working goes: with thinking off, the model writes it into the CONTENT
instead of a separate field. Either way the reasoning comes out of the
same token allowance as the answer, and at 500-3000 tokens the reply
came back EMPTY - which looks like a broken model rather than a budget
problem. The budget is now 8000, and the timeout 120s.
The reply is also stripped of a reasoning preamble. The advice is the
prose at the END; the working comes first, in several paragraphs. The
preamble is found by walking back from the end of the reply and keeping
the paragraphs that read like advice to the brewer, rather than like
the model thinking out loud.
Verified against the live model, on a stalled brew (OG 1.042, 1.022 after
nine days, "no change since day 7"):
That 1.022 reading after nine days at 19C - with no movement since
day seven - looks like a stalled fermentation; a 1.042 wort should
be well on its way to 1.010-1.012 by now, not sitting at only about
48% attenuation. Gently swirl the fermenter to rouse the yeast cake
and keep the temperature steady at 19-20C, and if the gravity hasn't
budged in another 48 hours, pitch a fresh, healthy yeast starter...
And on a CORRECTED older reading - the case the review found broken -
the advice is about the corrected reading, with the later one as context:
A brew-day reading of 1.038 against an OG of 1.042 is only four points
low, which is almost certainly just the normal difference between
your kettle gravity and the post-transfer fermenter reading...
169 tests (up from 167).
Two findings from the re-review of PR #14.
A race that recreated the leak
-------------------------------
`get_shared_repository()` did an unsynchronised check-then-set on a
module global. Eight concurrent advice threads on a cold process each
saw None and each built their own repository - so eight MongoClients and
twenty-five threads. The fix that was meant to remove the leak
reintroduced it, just bounded by a race instead of by the number of
readings.
Measured before: 8 repositories built, 25 threads.
Measured after: 1 repository built, 4 threads.
There is a lock, and the check is repeated inside it.
The prompt was about the wrong reading
--------------------------------------
The advice targeted the right reading, but the PROMPT said "give advice
for the most recent reading" - and when the brewer corrects an OLDER
reading, the most recent one in the history is a different reading. So
the model was asked about the wrong number while the advice was filed
against the right one.
The reading is now named in the instruction (with its date and gravity),
marked in the reading list as the one to advise on, and the system prompt
says to advise about the marked reading because it may not be the most
recent one.
A corrected reading is also flagged as such, so the model knows it may
differ from what it said before.
167 tests (up from 164).
_as_doc_id returned None for a hostile id. That was guarded at the one
call site, but the helper could still be misused into "$in needs an
array" - and a caller who forgot the guard would have got an error
rather than a safe no-op.
It now returns an EMPTY LIST for anything that is not a reading id. An
empty list is a valid $in argument that matches nothing, so the helper
cannot produce a query error, cannot match every document, and cannot be
interpreted as a query operator.
Verified against the live database: a hostile id matches 0 documents and
updates 0 documents.
164 tests.
The review of PR #14 found a critical defect and two real bugs. All
verified against the live database, not just read.
CRITICAL - the advice would never have been stored
--------------------------------------------------
`_as_doc_id` accepted only an ObjectId or a string that parses as one.
But **all 47 live readings have string `_id`s** - 24 random characters
from the original app's importer, like `l78ZidBJ3QaYn98tvB88x4uM`. The
helper turned every one of them into None, so the update matched nothing
and the advice was silently never stored in production.
The mongomock fixture generates ObjectIds, so the tests could not see it.
`_as_doc_id` now returns both forms and the query uses `$in` (a plain
list would be an exact ARRAY match in Mongo, which is not what is
wanted). Verified against the live database: the advice now lands on a
real reading, with the brewer's comment untouched.
There are now tests for BOTH id shapes, with the string shape built
explicitly, because the fixture would not produce it.
The advice was written about the wrong reading
----------------------------------------------
`latest` was taken from the date-sorted tail of the run, so editing an
OLDER reading generated advice about the NEWEST one. The advice is now
written against the reading that was actually saved.
A leaked MongoClient per reading
--------------------------------
The background thread built a `BeerRepository()` - and therefore a
`MongoClient` - for every advice write, and never closed it. A
MongoClient spawns three monitor threads and pymongo does NOT reap them
on `close()`; they go only when the client is garbage collected.
Measured: three threads and a connection pool accumulated per reading
recorded. There is now one shared repository for the life of the
process, which is what the request-scoped repository does too. Measured
again after the fix: 4 threads total, not 16 over five writes.
Also
----
- `BeerRepository` gained `close()` and the context-manager protocol.
- The advice writer now logs instead of dying silently if the write
fails.
164 tests (up from 160).
Two things I wanted to be sure of before this merges.
Stale advice
------------
The advice is written for the gravity that was there when it was
generated. If the brewer then edits the reading - to fix a typo, or to
mark it final - the old advice no longer describes it, and the page was
showing advice that contradicted the number next to it.
`upsert_reading` now clears the advice fields, so the stale advice goes
and a fresh one is generated (the form already regenerates on every save).
Only the advice fields are cleared - the brewer's own comment is never
touched.
The injection guard
-------------------
The reading's `_id` is an ObjectId, so the original `_as_id` coercion
(which turns run ids into plain strings, to stop a caller passing a dict
that Mongo would interpret as a query operator) did not apply - it turned
the ObjectId into an empty string and the update silently matched
nothing. `_as_doc_id` accepts only an ObjectId or a string that parses as
one.
Verified against a live injection attempt: `{"$ne": None}`, `{"$gt": ""}`,
a list, an int, None and a non-ObjectId string all return None and match
nothing. There is now a test that pins it, because it is a security
property that would otherwise only be covered by reading the code.
160 tests (up from 158).
Stores a model's advice for each recorded reading, in its own field, so the
brewer's own comment is never overwritten.
Fire and forget: the advice is generated in a background thread, so
recording a reading never blocks on the model. A failure is recorded as a
visible note saying the advice could not be generated, because a silent
failure is indistinguishable from "still thinking".
The model is `glm-5.3:cloud` on the Ollama instance at 192.168.178.45:11434.
Several models on that instance are retired and fail at call time while
still appearing in the model list, so the default is one that was probed.
The system prompt tells the model never to call a reading impossible: a
gravity below 1.000 is always a mis-entry, but the rest of the run may be a
genuine long fermentation and still worth understanding.
150 tests.
hermes