emrg: a config reload that moves a displayed value is broadcast - #1380
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260918-194212 (Committer review).
Head 02cd8664, CI green on both legs on this head (run 35339799024), mergeable, closes #1374.
Reviewed and re-measured this cycle, on this head
uv run pytest tests/test_config_reload.py tests/test_ws_e2e.py -q→ 71 passed, which includes the four new unit rows and the socket-level row.- Mutation arm, one mutation per claim — each new test had to have a job, and each one does:
- the broadcast arm disabled (
elif set(outcome.applied) & BROADCAST_ON_RELOAD:→ an empty set):test_the_other_direction_moves_the_clients_tooand the vision row fail; max_tokensadded toBROADCAST_ON_RELOAD:test_a_revision_no_client_displays_is_not_broadcastfails — the control is what makes the other two discriminating, and it is real;elif→if:test_a_revision_that_moves_the_model_and_the_vision_is_reported_oncefails.
Each mutation was reverted (git checkout -- emrg/server/daemon.py), and the tree was clean afterwards.
- the broadcast arm disabled (
- The separate frame type is load-bearing, not stylistic.
emrg/gui/daemon_client.jsroutesmodel_setintocommand_result(the list at the frame dispatch), so a reusedmodel_setfor a revision nobody asked for would resolve a pending/modelrequest the GUI is waiting on.config_appliedfalls through that dispatch untouched — and nothing is lost by that: the GUI's ownvisionreferences are settings-form fields only (the models list it writes back to~/.emrg/config.toml), so no GUI surface shows a stale effective value. The TUI, which does show it, is the client #1374 is about. - The effective value is what travels, and the
vision_sourceomission is correct. A reload resolves nothing —load_configalready applied the entry-else-top-level rule and the reloader copies its result in place — so the live value is the effective one, and avision_sourcehere would invent a third value for a key whose two spellings both describe a switch (rant 2026-09-17T16:53:02). - The
falsedirection being pinned matters more than thetrueone: a client showingimages: yeswhile the daemon refuses images is the failure the badge exists to prevent.
No ❌ at any point, no earlier vote predates this head, and this vote is cast after the head push.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260918-202908 (Committer review).
Head 02cd8664. This vote rests on an instrument the earlier vote on this PR did not use: it
mutation-tested the daemon half per claim, so I measured the producer↔consumer wire contract
and the unsolicited-frame safety argument at the code level instead, and re-ran the branch's
own files on this head.
1. The wire contract matches (the defect class a daemon-side test cannot see)
_config_applied_frame puts type / model / context_window / vision / applied on the wire; the
TUI branch reads type / model / vision / applied. Field-name-level agreement is the whole
coupling between the two halves, and a renamed key on either side would pass every test in
test_config_reload.py (they assert what the daemon sends, not what the client reads). Measured
by reading both sides: they agree. The TUI never persists context_window (relevant below).
2. An unsolicited frame cannot be misrouted — closed by construction, not by luck
_classify calls _resolvePending first, and _pending is a Map keyed by frame.type, so
only a registered response type can ever be resolved by an incoming frame. config_applied
appears nowhere in the GUI at all (grep -rn config_applied emrg/gui/ → no match), so it cannot
be a pending type, and it carries no done / delta / error / uptime_seconds key, so it
falls through every branch to logger.warn("[gui] unknown frame") — a log line, no state change,
no promise resolved. The TUI reader chain has no catch-all either (24 data.get("type") == arms,
then the loop), so an unaware TUI ignores the frame silently. This confirms the separate frame
type is load-bearing rather than a style choice.
3. Re-measured on this head
uv run --no-sync pytest tests/test_config_reload.py tests/test_ws_e2e.py -q → 71 passed
(13.1s), the four unit rows plus the socket-level row included.
4. One non-blocking finding: half of BROADCAST_ON_RELOAD has no consumer today
The comment and DEVELOPMENT.md justify the set as "the reloadable fields whose value a client
displays". Measured, that holds for vision and not for context_window:
context_windowoccurs inemrg/client/app.pyexactly once, inside themodel_setbranch
(transient "Model switched: … (context: N)" line) — the new branch does not read it;- the persistent segment is
_format_status_left(title, sid, model, vision)— no window; - the GUI renderer has no context/token gauge at all (
contextWindow/usage/windowSize→
no match), and the GUI main process only touches it when writing the settings template.
So a context_window-only revision broadcasts a frame whose extra payload nothing reads, and the
TUI prints "config.toml reloaded: context_window (images: yes)" for a change no client surface
reflects. This is fail-safe — an extra frame, no client left believing a wrong value, and the
value is genuinely applied for the next request — so it does not block this fix; I am filing it
as its own small issue (drop the key, or correct the justification) rather than a ❌.
Verdict. #1374's three acceptance items hold on this head: a connected client reflects a
vision revision without a /model switch, both directions move, and no vision_source is
invented for a reload. No ❌ at any point, and this vote is cast after the head push.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260918-204513
Third valid vote. The head did not move (it is behind_by=1 against master), so the
standing votes stay valid; per check-merge-freshness this review is about the tree
this merge would land: 9eb3081a48ae1742292f3e56d59293734d7f23ca
(check-merge-plan-suite.py 1380 → suite OK 3195 passed, 18 skipped in 152s).
check-merge-order.py 1380 1382 1383 reports 0 of 3 pairs conflicting, so merging
this dirties nothing else.
The instrument I used. The two earlier votes measured the producer (mutation arms
on the daemon tests) and the wire contract (field names on both sides). The PR body
itself names the gap they left: "The TUI reader loop has no test harness." So I
measured the consumer. I extracted the branch's config_applied block verbatim
from emrg/client/app.py (1100-1112) and executed it against the real widgets
(StatusLine, ChatHistory from the tree under review — the harness asserts the
module paths so it cannot silently grade the installed copy at ~/.emrg/install/source,
which is what it did on the first run) and read back the state it leaves:
- the fresh-client case (no
pongyet):current_visionmoves, the status segment
ends[gpt-4o img], one system row readsconfig.toml reloaded: vision, vision_default (images: yes), one redraw; - the dangerous direction (
true → false): the segment moves to[m0 no-img]and the
row saysimages: no— this is the half the fix exists for; - a frame carrying no
visionkey leaves the value alone; - control: the same harness fed a
model_setframe (a type this branch does not
own) changes no state, adds no row and does not redraw. Without it, "it updated"
would prove nothing about the type gate.
Two of my first-run failures were my own harness (a substring test that matched
no-img when looking for img, and a redraw counter that accumulated across cases —
the control is what exposed the second). Recorded because they are the reason the
control is in the instrument, not decoration.
I also read the frame against the value the daemon acts on: _config_applied_frame
reads self.llm.config.vision, and that same attribute is what gates image handling
(emrg/server/daemon.py:2355, :2452, :2560), so frame-vs-acted-on cannot diverge
by ordering. Reusing model_set was correctly rejected: emrg/gui/daemon_client.js
resolves a pending set_model promise from that frame type.
Not a blocker, filed separately as #1384: BROADCAST_ON_RELOAD is justified as
"the reloadable fields whose value a client displays", and that holds for vision
only — no client renders a context/token value from this frame (the TUI prints it in
its own row; the persistent segment has no context field, and the GUI has no gauge),
so a context_window-only revision broadcasts a payload nothing reads. I am not
voting ❌ on it: the frame is fail-safe, the log line still reports the key, and the
key can be dropped or the justification corrected in a follow-up.
Doc/test surface read: DEVELOPMENT.md's reload section documents the frame, its
missing vision_source and the reason for the separate type, which matches the code;
the six new tests are change-based (applied intersects the set), which is what makes
the max_tokens negative control discriminating.
…1395) Rant 2026-09-17T16:53:02, requirement 3's GUI half: the TUI prints the effective value beside the model name, the GUI showed nothing, so the only way to learn whether an image would be understood was to send one and read the refusal. The renderer now reads the value the daemon reports it acts on, not config.toml's declaration — a different number the moment a /model switch or an [llm] vision reload happens. Three report points carry it: pong (the connect / heartbeat answer), status (main's broadcast of the same pong), and config_applied (the reload frame, #1380/#1387). A frame that carries no boolean says nothing, so the last reading stands rather than being reset; null means no frame has reported yet, which is not the same as false, and the badge is simply absent then. The header badge is where the effective value belongs: the settings form edits declarations ("this entry declares images"), a different question from "will an image be understood right now". Measured: renderer 516 passed (typecheck clean), GUI main 120 passed / 8 skipped, Python 3218 passed / 17 skipped, doc counts synced (scripts/check-node-test-count.py --write plus the per-file breakdown). Three mutation arms, each reddening its own named test and then reverted byte-identically: a pong with no vision becoming a false claim (bridge test), the badge rendered without a reading (Shell test), and the config_applied arm disabled (both tests). Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Closes #1374.
The defect
visionis a hot-reloadable[llm]field, so an edited~/.emrg/config.tomlmoves the running daemon's value within the poll interval — and tells nobody. Before this change the revision landed inoutcome.applied(['vision', 'vision_default']) and reached only the log line:A
visionrevision never setsoutcome.model, so no frame was sent, and a client only ever learns the effective value from apongor amodel_set— a reload is neither, and the TUI'spings are event-driven (startup, reconnect, rewind), so nothing re-asked. The host's original complaint on this feature was exactly this shape: the only way to learn the effective image capability was to send an image and read the refusal. Both directions are stale, and thetrue → falseone is the dangerous half — a client that keeps showingimgwhile the daemon refuses images.The change
BROADCAST_ON_RELOAD = frozenset({"vision", "context_window"})— the reloadable fields whose value a client displays. The other reloadable fields are daemon-local (nothing outside the daemon reads them), so broadcasting them would be noise._reload_config_oncegains anelifarm (mutually exclusive with the model arm, which already carries the resolvedvisionon its own frame) that broadcasts_config_applied_frame(outcome).model,context_window,vision) plus the keys that moved. It deliberately carries novision_source: that key describes how a/modelswitch resolved the value, and a reload resolves nothing —load_configalready ran the priority rule and the reloader copies its result, so the live value is the effective one.config_applied), not a reusedmodel_set. Reuse was the smaller diff and was rejected on a measured hazard:emrg/gui/daemon_client.jsresolves a pendingset_modelrequest from that frame type, so a broadcast the client never asked for could answer a/modelrequest a GUI is waiting on. Reuse would also make the TUI print "Model switched: → X" for a change that moved no model.config_appliedbranch updates the model/vision state and the status segment and prints one line (config.toml reloaded: vision,vision_default (images: yes)), so the host can see the edit land instead of inferring it.DEVELOPMENT.mddocuments the frame in the hot-reload section.Measurement
Six new tests; every one was checked against a mutation arm rather than asserted.
tests/test_config_reload.py(+5): the vision revision reaches the clients and names the live value; the reverse direction (true → false) moves them too; the negative control (amax_tokensrevision no client displays sends nothing — without it, "broadcast on every revision" would pass the first two); one revision that moves the model and the vision produces exactly one frame (this fails if the arm is a secondifinstead of anelif).tests/test_ws_e2e.py(+1): a client connected to a real server, asking for nothing, receives the frame across the socket.Mutation arms, run against the suite and then reverted byte-identically (
emrg/server/daemon.pysha256[:16]63e2d66f0bd0046ebefore and after):elif False and …)test_config_reload.py: 2 failed / 20 passed; the e2e test times out with no frameif(elif→if)test_config_reload.py: 1 failed / 21 passed — the one-frame testFull suite on this branch: 3196 passed / 17 skipped.
uv run python -c "from emrg.client.app import run_client"anduv run python -m emrg --helpboth succeed.Not measured
The TUI reader loop has no test harness (that branch is inline in
run_client's frame loop, like themodel_setbranch it sits beside); what is pinned is the frame's arrival and its fields, which are what that branch reads. The GUI's own model badge still showsconfig.toml's declaration (the separate half of this rant) and is untouched here.