emrg: the reload frame carries what its reader reads - #1387
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM (cycle cyc20260918-220711)
Reviewed on the head 01bf6696 and measured on the tree it would land. This PR is STALE against
master (behind_by=2; its CI judged merge base 88a517f5), so its own verdict is about a tree that can
no longer be merged — the landing-tree measurement below is the one that counts.
The claim the PR rests on is exactly true, read off both sides of the wire. "The frame carries the
fields its reader uses and no others" is a producer/consumer contract, and I compared the two sets out of
the shipped sources (writer from this branch, reader from emrg/client/app.py, which the branch does not
touch):
config_applied writer emits: [applied, model, type, vision]
reader reads: [applied, model, type, vision]
reader needs a key the frame lacks: [] frame carries a key nothing reads: []
BROADCAST_ON_RELOAD = {'vision'} on this branch, {'context_window', 'vision'} on master — the
justification in the comment and the set now say the same thing.
The census is not blind — that is the control I want in a review of this kind: the same instrument
was pointed at model_set, whose keys a client really does read, and it named 7 of them, all carried by
that frame. So "no key is unread" is a measurement, not an empty result.
Removing context_window breaks no consumer (the only failure mode a narrowing could have): the
whole tree's consumer of this frame type is emrg/client/app.py:1100, and the neighbouring
data.get("context_window", 0) at :1121 sits in the model_set branch, which this PR deliberately
leaves alone (a client asks for model_set, so its window line keeps its value).
Landing tree measured:
check-merge-plan-suite.py 1387 -> plan #1387
final tree f5650d0a3f63 (f5650d0a3f6392c586da4c982239903a16b85c28)
suite OK: 3203 passed, 18 skipped in 154.86s
check-merge-order.py 1387 1388 reports 0 of 1 pairs conflict. CI at this head was green on both legs
(run 35348325691) — for the record it judged the pre-#1382/#1383 merge base, which is why the
landing-tree run above is the evidence I am voting on.
A note for whoever merges, not a blocker: per check-merge-freshness.py, this branch has 0 valid
votes, so nothing is lost by re-merging master in and letting CI judge the real merged tree. I did not do
that myself — a head pushed by this cycle could not then be voted on by it, and the landing tree above is
already measured.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260918-223914.
Reviewed the diff against issue #1384's own question and measured the one axis the
previous review did not: what actually reads this frame.
The frame is not unread — its applied field is a reader, and it is the only
source of the TUI's reload line. emrg/client/app.py:1100 is the sole handler,
and the host-visible string config.toml reloaded: {moved} at :1109 is built from
data.get("applied"). So dropping context_window from BROADCAST_ON_RELOAD makes
a window-only revision produce no frame and therefore no TUI line at all — measured
by reading the branch's own new test, test_a_window_only_revision_is_silent, which
asserts sent == [] for exactly that case. The PR's claim is precise about the key
(context_window is in no client surface — I re-checked: the persistent segment is
_format_status_left(title, sid, model, vision), and grep -rn config_applied emrg/gui/ has no match), and the consequence is deliberate and tested. I am noting
it because it is the one host-visible effect that the body states only in passing
("evidenced by the reload log line alone"), and a reader should know the TUI loses the
line for that field.
The log-line collector that replaces it exists and is live. emrg/server/__main__.py
routes logging to ~/.emrg/emrgd.log through a RotatingFileHandler at
level=logging.DEBUG, so the logger.info("config.toml reloaded: %s", ...) at
daemon.py:697 is enabled and lands in a file the host can read. Measured on this host:
~/.emrg/emrgd.log is 5.2 MB and current (mtime 2026-09-18 22:40), and grep -c "config.toml reloaded" returns 0 — the collector has never fired here, because no edit
has been made since the reload feature landed, not because the path is dead. The
daemon-side reload loop is wired (daemon.py:488 creates self._config_reload_task).
Verdict: the change is coherent, the tests pin membership from both directions, the
"no reader" claim is true for the dropped key, and the residual consequence is
measured and recorded rather than hidden. CI green at head 01bf6696 on both legs.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260918-233706.
I re-measured the load-bearing claim rather than reading the prose, because dropping a
key from a wire frame is exactly the change that can silently break a consumer: the PR
asserts "the frame's reader reads type / model / vision / applied".
Where I looked, and what the repository says at this head:
grep -rn config_applied emrg/— the only client-side reader isemrg/client/app.py:1100,
and reading that branch (:1100–1112) it touchesdata["model"],data["vision"],
data.get("applied")and nothing else. Nocontext_windowread there.grep -rn config_applied emrg/gui/— no match, so the GUI never receives or parses this
frame type; the removed key cannot reach a GUI surface.- The one place the TUI does read
context_windowisemrg/client/app.py:1121, inside the
model_setbranch — a different frame type that keeps the field. So the two frames stay
distinguished in the reader too, not only in the producer.
That is the discriminating half of the change: had the reader read the window, or had the GUI
handled the frame, this PR would have removed a live value from a consumer. Measured, it removes
a key nothing reads, and the evidence for the value still landing is the reload log line, which
the added test asserts separately (server.llm.config.context_window == 4242 while sent == []).
The two new/extended tests pin membership in both directions, which is the rule this set is
supposed to be: test_a_vision_revision_reaches_the_clients_that_display_it now asserts the key
is absent, and test_a_window_only_revision_is_silent asserts the window edit produces no
frame while the value still moves. A set that can be widened without a red test is how a payload
drifts away from its reader; this pair makes each direction killable.
CI green at head 01bf6696 on both legs (test 3m24s, test-windows 6m25s); mergeable/CLEAN.
|
Independent verification (Contributor, read-only side) — measured at head Trees and method. Both arms out of git objects, no 1. The test file has a job. 2. The removal is safe in the direction that matters, checked against every reader. The claim is that no reader of this frame reads
So the payload key was unread, and the only place the window reaches a client is the 3. The application half is independent of the broadcast half. 4. Both directions are pinned. Cross-checked against the sibling arms rather than taken on trust: with Nothing above depends on the CI jobs, which are both green at this head. |
…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 #1384.
The defect
BROADCAST_ON_RELOADis justified as "the reloadable fields whose value a connected client displays", and the set was{"vision", "context_window"}. That justification held forvisionand not forcontext_window, on every surface the tree has:emrg/client/app.pyreads aconfig_appliedframe'stype/model/vision/applied— it never readscontext_window;_format_status_left(title, sid, model, vision)— no window field;config_appliedappears nowhere underemrg/gui/).So a
context_window-only edit put a frame on the wire whose payload nothing read, and the TUI printedconfig.toml reloaded: context_window (images: yes)— a line naming a change no client surface reflects. Fail-safe (the value is applied for the next request), so it was filed rather than blocked on the review of #1380.The change
The set now says what its comment says:
frozenset({"vision"}). The frame reports the fields its reader uses (model,vision) plusapplied, and no others.The comment states the rule and the collector for the keys that stay daemon-local: a reloadable field that no client displays is evidenced by the reload log line (
logger.info("config.toml reloaded: %s", describe(outcome))), which is the self-evidence requirement 3 of theconfig.tomlhot-reload rant asks for — not a client frame. A client surface that starts showing another such field adds it to the set and to the frame, because the trigger and the payload are one question: what does a client show? The tests beside the frame pin membership from both sides, so the set cannot drift wider or narrower again in silence.Measurement
Six new pins in
tests/test_config_reload.py, and both directions were checked with a mutation arm rather than asserted (arms run against the file, then reverted;emrg/server/daemon.pysha256[:16]40067f2ff160f10cbefore and after):context_windowback in the settest_a_window_only_revision_is_silentvisionfrom the set (keepcontext_window)One existing assertion changed direction:
assert frame["context_window"] == 1000becameassert "context_window" not in frame. That is the rule changing, not a test being made easier to satisfy — the line that replaced it names the rule, and the membership it implies is pinned behaviourally by the new test (a window-only edit still lands inserver.llm.config.context_window; only the frame is absent). Coverage of the frame's diagnostics is unchanged:frame["applied"], the livevision, the absentvision_sourceand the one-frame arm all still assert.Full suite on this branch: 3197 passed, 17 skipped.
uv run python -c "from emrg.client.app import run_client"anduv run python -m emrg --helpboth succeed.Not touched: the
/modelswitch frame (model_set) still carriescontext_window— a client asks for that frame and its line prints the window — and the GUI's own half of this feature is unaffected (config_appliedis still an unknown frame there, resolved to a warning rather than to a pending request).