Repository navigation
feat(desktop): clarify variant-only timeline notices - #54880
Conversation
There was a problem hiding this comment.
The change looks sound. The server already sends previous on session.model.selected and on model-switched messages, and switchModel skips switches where nothing changed. Carrying previous into the optimistic and live notices keeps the label right before the echo, and older messages without previous still show "Switched to X". The projection and client data unit tests and the app/client/session-ui/ui type checks pass. I couldn't run the Playwright component tests (no browser in the sandbox) or render the web timeline here, so I relied on your Storybook before/after screenshots.
Public API: unchanged. previous already exists on the event and message schemas; this only adds two English UI strings.
One small point on the new client test (inline).
| } | ||
| }) | ||
|
|
||
| test("keeps the previous model in a live variant notice until the projected message arrives", async () => { |
There was a problem hiding this comment.
This test is large for what it checks: a mocked fetch held back by a deferred promise plus a wait for the refetch. And because the event already carries previous, it only checks that the field passes through. It never reaches the new fallback (event.data.previous ?? store.session.info[...]?.model). Could it be cut down to the fallback case: an event without previous, checking that the notice gets the session's stored model? Or drop it if the projection test is enough.
Issue for this PR
Reported in Slack; no issue number.
Type of change
What does this PR do?
A variant-only switch back to Default looked like a model change in the web timeline. The notice now compares the previous and selected model and names the variant change instead. Live and optimistic notices carry the previous model too, so the text stays correct before the server echo arrives.
How did you verify your code works?
bun run checkScreenshots / recordings
Checklist