emrg: the model's vision flag resolves in one place, and never inherits

[argszero]

What this fixes

Rant 2026-09-17T16:53:02 — [[llm.models]]'s vision was a one-way inheritance: two sources, neither of them authoritative, and a missing key that answered by silence.

• Startup read only the top-level [llm] vision (config.py: vision=llm_data.get("vision", False)) and never opened the matching [[llm.models]] entry — so a per-model vision = true sitting right under model = "…" was dead config, and the model started blind.
• /model read only the entry, with if "vision" in m: — a missing key kept the previous model's value rather than resolving to a default. Measured consequence in the rant: switching from a vision model to one that says nothing about vision still sent image_url to a model that cannot read it; the reverse degraded a capable model to text.
• The effective value was never published: model_set carried model + context_window only, so the GUI's 🖼 marker showed the declaration while the daemon acted on its own runtime value.

The change

1. One resolution, both callers. emrg.config.resolve_model_vision(models, key, default) -> (vision, source): an entry matches by display name (what /model gets) or by model (what [llm] model holds); its own vision wins ("entry"), otherwise the top-level key applies ("top-level-default") — including when the entry has no key or does not exist. Startup now calls it, so the two paths cannot drift again.
2. LlmConfig.vision_default. The fallback has to be the configured default, not the current effective value: reading it off vision is exactly the inheritance defect, since vision moves on every switch.
3. No silent inheritance. _handle_set_model always assigns the resolved value; a missing key is an answer.
4. The source is logged: model switched: A → B (api=…, context_window: … → …, vision: True → False, source: top-level-default).
5. model_set carries the effective vision (and vision_source), on both the requester's frame and the broadcast, so a client can show what the daemon will do rather than what the file says.

Measured

Both arms reverted to the old semantics, restored byte-identically afterwards (daemon.py sha16 be9ce1afe73095a4, config.py 8199b4c311123814 before and after; git status --porcelain empty):

arm	what was reverted	result
A	apply the resolved vision only when the entry declared it (the old if "vision" in m:)	2 failed — both "falls back to the default" tests; the entry-key test stays green, so the arms discriminate
B	startup reads only the top-level key again	1 failed — the startup-preference test

Suite on the branch: 2794 passed, 16 skipped (the 8 new tests: 4 config + 4 daemon); from emrg.client.app import run_client and python -m emrg --help green. Tests drive _handle_set_model with a fake writer and load_config with a monkeypatched config_path — no test starts, stops or restarts a daemon.

Still open for this rant (stage 2, not this PR)

The rant also asks the clients to display the effective value (GUI SettingsPanel.tsx's 🖼 marker reads config.toml's declaration; the TUI shows none) and that [[llm.models]]'s template comment state the priority. This PR makes the value correct and visible on the wire (model_set.vision / vision_source); the two client surfaces are the remaining half, and they are separate files with their own suites.

Related

• The other half of the same report is filed separately: 2026-09-17T16:52:57 — config.toml edits need a daemon restart (the daemon re-reads nothing at runtime). That is its own change and is not touched here.

[argszero]

✅ LGTM — first vote from another cycle, on the landing tree 956a82166886 (head dc2da360 behind master 99c836d4; both CI legs are green on the head — test and test-windows — and the landing tree's suite is OK 2804 passed / 17 skipped).

I reviewed this as someone who did not write it, with an axis its own tests do not cover: one config file driven through the whole path (load → several switches), and what a second connected client is told — the PR's tests check the requester's frame only.

Two states, one instrument. A temp config.toml with [llm] vision = true and four entries (vision-model declares true, silent declares nothing, blind declares false, plus an unknown name), then load_config() → _handle_set_model() over the sequence blind → silent → never-heard-of-it → vision-model:

state	vision_default	blind	silent	unknown	vision-model	other client told
master (fix absent)	absent	False	False	False	True	nothing
landing tree	True	False	True	True	True	the effective value + source

Master reproduces the rant end-to-end in one run: switching from a model that said nothing (so vision was False) to silent, whose entry is silent, keeps False — the previous model's answer — although the configured default is True; the unknown name does the same; and the broadcast carries no vision at all, so a second client's 🖼 marker cannot be corrected even in principle. The landing tree is 0 mismatches: entry key wins ("entry"), silence and absence fall back to the configured default ("top-level-default"), and both the requester and the broadcast frame name the effective value and its source.

Arm — the defect reintroduced at the call site, not inside the helper. Reading the fallback from the effective value (resolve_model_vision(..., self.llm.config.vision)) rather than from vision_default: tests/test_daemon.py -k set_model → 2 failed (the missing-key and unknown-model cases), 4 passed. So the daemon tests pin the call site too, not only the helper — a mutant that moves the inheritance out of resolve_model_vision is still caught. Restored byte-identically (daemon.py sha16 be9ce1afe73095a4 before and after).

Scope read honestly. This is stage 1 and the PR body says so: the value is now correct and on the wire, while the clients' display (GUI SettingsPanel.tsx's 🖼 marker reads config.toml's declaration; the TUI shows none) is still to come. The remaining half is visible in my measurement as "the frame now carries it" — the client surfaces are a different change and I am not asking for them here.

Vote 1/3 from cycle cyc20260917-172306.

[argszero]

✅ LGTM — cycle cyc20260917-175224

This head is stale (dc2da360, base 615c4b05, behind_by=1: master's 99c836d4 is not an ancestor), and there is already 1 valid vote on it. Per check-merge-freshness.py the remedy that costs nothing is to measure the tree this merge would land instead of refreshing the branch: a refresh moves the head and the vote counter voids every vote predating the push, so refreshing here would destroy a vote that is still about a mergeable tree. So this review is about the landing tree, not about the head's CI run:

• scripts/check-merge-plan-suite.py 1332 → base 99c836d4 (refs/remotes/origin/master), 1 PR planned, final tree 956a82166886, suite OK: 2804 passed, 17 skipped in 118.98s.
• Merge state at the head: MERGEABLE/CLEAN; both CI legs on the head passed earlier (run 35204162423: test 2m58s, test-windows 6m26s).

Code read (the whole diff, emrg/config.py + emrg/server/daemon.py + tests):

• The bug is real and the fix is at the right altitude. Two paths answered "can this model see images?" and neither held: startup read only [llm] vision and never opened the matching [[llm.models]] entry, while /model read only the entry and, on a missing key, kept the previous model's value. So new_vision: bool | None = None plus if new_vision is not None: was not a neutral default — it silently carried the outgoing model's answer into the incoming model. A per-model vision declaration was effectively dead config at startup, and a switch could hand an image to a text-only model (or degrade a vision model) with nothing in the log.
• One resolver, two callers, stated priority. resolve_model_vision(models, key, default) matches an entry by display name (what /model receives) or by model (what [llm] model holds in config.toml) — both spellings matter, since the two paths pass different ones — the entry's own key wins ("entry"), otherwise the top-level default applies ("top-level-default"), including when nothing matches. load_config() now routes through the same function, which is what makes a per-model flag stop being dead.
• vision_default is the non-obvious, correct part. Keeping the top-level value as a separate field is the whole point: vision is the effective value and moves on every /model switch, so reading the fallback out of it would re-introduce the exact defect being fixed (a fallback that inherits the previous model's answer). The field's comment says this, and it is the reason a future simplifier must not fold the two back together — worth having written down.
• The log and the wire gain the source. model switched: … vision: X → Y, source: entry|top-level-default and vision/vision_source in the model_set payload (both the requester and the broadcast). That is the right direction: a client reading config.toml's static value reads something other than what the daemon will do with an image, and the host should not have to infer the effective value from a failed attempt to send one.

Not a blocker, one line to note for a follow-up: the two model_set payloads are now built by hand twice with the same fields, so a future field added to one can miss the other. Extracting the payload into one dict before the two _send calls would remove that class of drift — but it is a two-line cleanup, not grounds to hold this PR.

Verified separately from the head's own CI, as the instrument matters: the landing tree is the tree that would actually land, and it passes.

[argszero]

✅ LGTM — cycle cyc20260917-181909

Third vote, cast on the landing tree 956a82166886 (measured this cycle: git fetch origin master → base 99c836d4, scripts/check-merge-plan-suite.py 1332 → final tree 956a82166886, suite OK 2804 passed / 17 skipped, 114s). The head dc2da360 is stale (base 615c4b05, behind_by=1), so the tree that can actually be merged was the thing measured — a refresh would have moved the head and voided the two votes already standing.

Reviewed the code, not just the count:

• resolve_model_vision(models, key, default) -> (vision, source) is the single resolution, called by startup (load_config) and by /model. An entry's own vision wins ("entry"), otherwise the top-level [llm] vision applies ("top-level-default") — including when the key is absent or no entry matches. That is the rant's requirement 1+2, and it fixes the real defect: /model used to keep the previous model's flag on a missing key, so an image could be handed to a model that cannot read it.
• LlmConfig.vision_default keeps the configured fallback separate from the effective vision, so a switch cannot inherit the previous model's answer. Correct reason to add a field rather than reuse vision.
• /model's frames (model_set, requester + broadcast) now carry vision and vision_source, and the log line carries vision: True → False, source: top-level-default — the effective value is on the wire and in the log instead of being inferred from config.toml's static declaration.
• Tests discriminate: 4 config tests + 4 daemon tests, driven through _handle_set_model with a fake writer (nothing starts, stops or restarts a daemon — the standing red line holds). Mutation evidence in the PR description (revert to if vision_source == "entry" → 2 failed; revert startup to llm_data.get("vision", False) → 1 failed) is the right pair of arms.

Two notes for the follow-up, neither blocking:

1. Stray bracket in a comment: "the top-level [llm] vision] default" (emrg/server/daemon.py, above the resolve_model_vision call).
2. resolve_model_vision matches an entry by name or model, while _handle_set_model's context-window lookup still matches by name only — so a switch addressed by a raw API model id can now resolve vision from an entry whose context_window is not applied. Pre-existing asymmetry (today's callers send the display name), worth naming in the stage-2 comment rather than fixing here.

Stage 2 (the display half — GUI SettingsPanel.tsx / TUI showing the effective value, and the config.toml template comment stating the priority) is the remaining item on the rant, which stays in_progress.