Skip to content

Model Picker and VRAM Display Correctness #3

Open

Description

@androidand

Model Picker and VRAM Display Correctness

Why

Choosing a local model is the most consequential decision in a session — an undersized model wastes the run, an oversized one wedges the provider — and the TUI actively misinforms that choice in three ways.

1. Model size disappears exactly when you are choosing. dialog-model.tsx:55 (recents/favorites) and dialog-model.tsx:95 (provider browsing) both set footer: formatModelSize(info.sizeBytes), so the data is present in both branches. But dialog-select.tsx:616 renders

footer={flatten() ? (option.category ?? option.footer) : option.footer}

and flatten() (dialog-select.tsx:179) is props.flat && store.filter.length > 0, with DialogModel passing flat={true} (dialog-model.tsx:203). The moment you type a filter, the GB footer is replaced by option.category — which for provider-browse rows is the provider name (dialog-model.tsx:92). Recents keep their size only because that view renders only when the query is empty (dialog-model.tsx:32,37). That is precisely the reported asymmetry: sizes "only show for recently used". They show for everything, until you type.

2. There is no recommendation. Nothing tells you which model actually fits the target provider. The data to answer it already exists — llama-skein's /api/fit returns fit_level, model_mb, vram_total_mb, max_safe_ctx, and an estimated tok/s — but the picker does not consult it.

3. The VRAM bar lies after a model switch. sidebar/context.tsx:186 memoizes on state().baseURL alone, and the poll effect (:188-227) keys on that memo. Switching model within one provider leaves baseURL unchanged, so the effect never re-runs, the setMem(null) reset at :191 never fires, and the bar keeps rendering the previous model's model_mb / kv_estimate_mb (:47-48) until the 30-second setInterval (:220) happens to refresh it.

The string-equality memo is deliberate — its comment explains that keying on state() directly would restart the poll on every stream tick. So the fix is to widen the key, not remove it.

Neither bug is filed. Nine near-duplicate "context management" proposals touched this sidebar and all were rejected without naming either defect.

What Changes

  • Size survives filtering. The footer slot stops being overloaded. flatten() exists so that a flattened, filtered list still shows which group a row came from; that provenance moves to a distinct field rather than displacing the footer, so size and provenance coexist.
  • A fit-aware recommendation. The picker annotates each model with its fit against the target provider, sourced from /api/fit, and marks a recommended choice. Models that cannot fit are visibly marked rather than silently offered. Absent or stale fit data degrades to today's unannotated list rather than blocking the picker.
  • The VRAM bar tracks the model. The hardware poll key widens from baseURL to baseURL + model identity, preserving the string-equality property that keeps stream ticks from restarting the poll. On a model switch the stale sample is dropped immediately and a fresh sample is fetched, rather than waiting up to 30 s.
  • A latent size-clobbering footgun is closed. mergeDiscoveredModel (provider.ts:1452-1456) spreads ...existing over ...discovered, so a config-declared model entry carrying an explicit sizeBytes: undefined would overwrite a freshly discovered size. Nothing writes such an entry today (local/sync.ts writes no models key), so this is prevention, not a live bug.

Capabilities

New Capabilities

  • models-picker-ux: size visibility independent of filter state, fit-based recommendation in the picker, and model-keyed hardware polling.

Modified Capabilities

Non-Goals

  • No change to model discovery or to the /v1/models client. sizeBytes already arrives correctly (provider.ts:1532, schema :1114).
  • No new fit engine. This consumes llama-skein's /api/fit; it does not compute fit locally.
  • No redesign of the dialog component. flatten() keeps its purpose; only the overloaded footer slot is separated.

Impact

  • packages/tui/src/ui/dialog-select.tsx:179,616 — separate provenance from footer so both render.
  • packages/tui/src/component/dialog-model.tsx:32,37,55,92,95,203 — supply provenance in its own field; add the fit annotation and recommended marker.
  • packages/tui/src/feature-plugins/sidebar/context.tsx:186,188-227 — widen the poll key to include model identity; keep the string-equality memo.
  • packages/opencode/src/provider/provider.ts:1452-1456 — do not let an undefined sizeBytes in an existing entry clobber a discovered one.
  • Consumes llama-skein /api/fit. Related: llama-skein add-model-size (size_bytes on /v1/models) and bound-max-safe-ctx (stops /api/fit advertising an unachievable max_safe_ctx) — the recommendation is only as good as the latter, so it should land first or the recommendation must treat max_safe_ctx as advisory.

Tasks

Tasks: Model Picker and VRAM Display Correctness

1. Size survives filtering

  • 1.1 Add a distinct provenance field to the dialog option type in packages/tui/src/ui/dialog-select.tsx so a flattened, filtered row can show its group without consuming the footer slot. Verify: type-checks (bun run typecheck or the repo's configured check).
  • 1.2 Change the render at dialog-select.tsx:616 from footer={flatten() ? (option.category ?? option.footer) : option.footer} so that footer always renders option.footer and provenance renders in the new field when flatten() is true. Keep flatten() (:179) and its purpose intact. Verify: with flat={true} and a non-empty filter, both size and group render.
    • Implemented as flattenedFooter() combining option.footer and option.provenance ?? option.category into one string ("<size> · <provenance>") rather than a second UI slot on Option — smaller diff, same outcome: neither is dropped.
  • 1.3 packages/tui/src/component/dialog-model.tsx: populate the new provenance field with the provider name (currently passed as category at :92) and leave footer: formatModelSize(info.sizeBytes) at :55 and :95 unchanged. Verify: browse a provider, type a filter, confirm sizes remain visible — this is the reported bug.
    • Provenance also carries a per-provider VRAM/unified-memory label when known (see task 6 below), e.g. "z4 · 32.6 GB VRAM".
  • 1.4 Regression test covering the exact defect: a flattened list with a non-empty filter renders both size and provenance. Verify: the test fails against the current :616 expression and passes after 1.2.
    • packages/tui/test/ui/dialog-select-footer.test.ts, unit-testing the extracted flattenedFooter() helper directly.
  • 1.5 Confirm no regression to recents/favorites, which render only when the query is empty (dialog-model.tsx:32,37). Verify: open the picker with no query, sizes render as before.
    • toOptions() (favorites/recents) is untouched aside from adding provenance; footer still comes from formatModelSize.

2. Fit-based recommendation

  • 2.1 Add a client call for the target provider's fit report (/api/fit), returning fit level, model size, VRAM total, max safe context, and estimated tok/s. Cache per provider for the dialog's lifetime; never block dialog open on it. Verify: the dialog opens at current speed with the endpoint unreachable.
  • 2.2 Annotate rows with fit level and visibly mark models that cannot fit. Verify: against a provider with known VRAM, a too-large model is marked and a fitting one is not.
  • 2.3 Mark one recommended model per provider. Define and document the rule (largest model whose fit level is acceptable at the configured context). Verify: the rule is stated in the change notes and the marked model matches it.
  • 2.4 Degrade cleanly: with no or stale fit data, render the unannotated list and keep selection working. Verify: with the endpoint stubbed to fail, the picker is fully usable.
  • 2.5 Treat max safe context as advisory — llama-skein bound-max-safe-ctx exists because it has advertised unachievable values. Verify: the recommendation does not present it as a guarantee.

3. VRAM bar tracks the model

  • 3.1 packages/tui/src/feature-plugins/sidebar/context.tsx:186: widen hardwareBaseURL to a composite string key of base URL and model identity (for example `${state().baseURL}|${state().modelID}`). It must stay a string-equality memo — the comment at :184 records that keying on state() restarts the poll every stream tick. Verify: type-checks; the memo returns a string.
  • 3.2 Confirm the existing effect (:188-227) now re-runs on model switch, so setMem(null) / setTuning(null) (:191) fire and a fresh sample is fetched immediately instead of waiting for the 30 s setInterval (:220). Verify: switch model on one provider and confirm the VRAM bar and MemBreakdown (:125-140) update promptly rather than showing the prior model's model_mb / kv_estimate_mb (:47-48).
  • 3.3 Confirm no poll restart on stream ticks with unchanged provider and model. Verify: instrument or test that the effect body runs once across many state updates.
  • 3.4 Confirm provider switching still behaves as before. Verify: switch provider, confirm reset-and-refetch is unchanged.

4. Close the size-clobbering footgun

  • 4.1 packages/opencode/src/provider/provider.ts:1452-1456: mergeDiscoveredModel spreads ...existing over ...discovered, so an existing entry with sizeBytes: undefined would overwrite a discovered size. Make the merge preserve the discovered value when the existing one is absent or undefined. Note this is prevention — local/sync.ts writes no models key today, so nothing triggers it. Verify: unit test merging an existing entry with undefined size against a discovered entry with a real size retains the real size.
    • mergeDiscoveredModel exported for testability; packages/opencode/test/provider/merge-discovered-model.test.ts covers the regression plus the surrounding cases (no existing entry, existing has a real size, neither side knows it).

5. Verification

  • 5.1 Repo typecheck, lint, and test suites clean, with only pre-existing baseline failures. Record the baseline diff in task notes.
    • packages/opencode, packages/tui, packages/core typecheck clean. New/changed test files pass (packages/tui/test/ui/dialog-select-footer.test.ts, packages/opencode/test/provider/merge-discovered-model.test.ts). Full suite run still pending — see session-summary-write-amplification's verification, which covers packages/opencode/test/session; a full bun test across every package has not been run in this pass.
  • 5.2 Manual pass against a live provider: (a) browse models, type a filter, sizes remain visible; (b) an oversized model is marked as not fitting and one model is recommended; (c) switching model within one provider updates the VRAM bar promptly; (d) switching provider still resets correctly. Record observations in task notes.
    • (b) and (c) are tasks 2.x/3.x, not implemented this pass (see below) — not verifiable yet. (a) implemented; not manually smoke-tested against the live TUI in this pass (no interactive terminal in this environment) — code-reviewed and unit-tested only.

6. Delivered outside the original scope (this session)

Two things the user asked for that weren't in this spec's original three requirements —
implemented as a lighter version of what task 2/3 would eventually give, per explicit
direction to skip the full /api/fit recommendation and VRAM-bar polling fix for now:

  • 6.1 Per-provider VRAM/unified-memory label. packages/tui/src/component/dialog-model.tsx:
    on dialog mount, fetch getHardware() once per local provider (same
    LlamaSkeinClient/normalizeBaseURL pattern as dialog-model-ctx.tsx and the
    sidebar), cache per dialog lifetime, and fold into the category/provenance
    string, e.g. "z4 · 32.6 GB VRAM". Never blocks dialog open; a provider that
    doesn't answer /api/hardware simply has no label. This is not the fit-aware
    recommendation in task 2 — no per-model fit computation, just the provider's total.
  • 6.2 "Set as default" action. New entry in DialogModel's actions, alongside
    "Favorite" and "Set context size": writes the selection to the workspace's
    config.json via sdk.client.config.update({ workspace, config: { model } }),
    then refreshes sync.data.config so local.tsx's fallbackModel picks it up
    without a restart.

Plan changes

8 done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions