fix(chat): show the answer as it streams, and keep a model override across /clear#49
Merged
Merged
Conversation
…cross /clear Five defects introduced by the four PRs already merged onto the 5.1.2 branch. Each one's own tests passed; all but one lived where two of them met. The chat stopped rendering. MessageList looked messages up in a map memoised on the messages array, but the transcript appends to that array in place, so its identity never changes: the map was built once at mount and every message after it resolved to nothing. The strips above kept ticking while the bodies stayed empty, and reloading "fixed" it because reloading replaces the array. Keyed on the transcript version instead, which is the cadence the neighbouring memo already used — the list still does not re-render per token. A model override no longer survives /clear. The restart replays the options the session was launched with, and the model is the one thing in them that can change while the session is alive, so the conversation quietly went back to the model it opened with after the browser had been told the switch applied. The choice now reaches those options too, by either door: the picker, and a /model typed straight into the composer, which was forwarded untouched and hit the same reversion. A collapsed turn's title escaped its bar. It set neither a size nor nowrap while every sibling in that fixed-height row sets both, so it inherited the page default and wrapped — and without nowrap the ellipsis was inert. An override could not be undone. The server had a "cleared" path, message and all, that nothing in the UI could reach: the text field refuses to submit empty and every listed model carries a name. A typo therefore stayed in force for every later launch of that conversation. Model names are now capped and stripped of control characters, which would otherwise ride into the best-effort /model turn as extra lines. The browser checks gated nothing: npm test runs only the unit suite, and the runner exited 0 when Chrome was absent. With the rendering bug restored, npm test passes 1357/1357 while the browser check fails. They now run in CI and refuse to skip there. Their fixture also grew a second turn, without which no turn is ever collapsed and the broken title was never rendered at all. Every fix has a test confirmed to fail against the unfixed code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes several regressions in the chat UI/session behavior on chore/version-bump-5.1.2, including restoring streaming rendering, persisting model overrides across in-place /clear restarts, tightening model-name sanitization, and ensuring browser-based layout checks actually run in CI.
Changes:
- Fix message rendering during streaming by rebuilding the message-id lookup map based on transcript
versionchanges. - Persist model override changes into the restart options used by
/clear, including a UI path to clear an override, with server-side model-name normalization. - Run browser layout checks in CI (and fail CI if Chrome is missing), plus expand fixtures/checks to cover the newly-guarded cases.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/chat-wiring.test.js | Adds wiring-level tests for remembering model overrides (including clearing + /model typed path) and model-name sanitization. |
| test/chat-clear-reset.test.js | Verifies /clear restarts with the remembered model (override and cleared-to-default cases). |
| test/browser/run.js | Makes missing Chrome a hard failure under CI instead of a silent skip. |
| test/browser/checks.ts | Adds a live streaming rendering check and expands fixed-bar wrap coverage with a second turn. |
| src/server/websocket/messages.ts | Normalizes typed model names and persists the effective model into /clear restart options. |
| src/server/chat/session.ts | Introduces rememberModel() to update stored restart options for in-place restarts. |
| src/server/chat/manager.ts | Plumbs rememberModel() through the chat session manager. |
| src/client/shell/chat/TurnStrip.tsx | Ensures collapsed turn titles don’t wrap/overflow and match bar typography. |
| src/client/shell/chat/MessageList.tsx | Fixes message lookup memoization so messages render as they stream. |
| src/client/shell/chat/Composer.tsx | Adds an explicit UI option to clear a model override back to default. |
| CHANGELOG.md | Updates 5.1.2 notes to describe shipped features/fixes for that release line. |
| .github/workflows/ci.yml | Adds npm run test:browser to CI to gate layout/streaming regressions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
260
to
+262
| return map; | ||
| }, [messages]); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [messages, version]); |
| session.agent as AgentKind, | ||
| session.workingDir, | ||
| ); | ||
| this.deps.chatManager?.rememberModel(session.id, model || profile?.model); |
This was referenced Jul 26, 2026
dnviti
added a commit
that referenced
this pull request
Jul 26, 2026
…l /clear (v5.1.2) (#37) * chore: bump version to 5.1.2 * feat(chat): let users override a conversation's model, independent of profile default (#39) Adds a per-conversation model override so any chat session can switch or type a custom model at any time, regardless of whether the underlying runtime already supports it. Persisted on the session (mirrors the chatBypassPermissions pattern), applied via a three-tier fallback (live adapter.setModel -> best-effort /model slash command -> saved for next launch), and always beats the runtime profile's default for that conversation only. Composer's model chip becomes an always-enabled combobox with free-text entry and honest per-outcome feedback, replacing the old dead-end disabled state. Closes #38 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * feat(chat): collapsible turn sections, auto-folded on a new turn (#34) (#40) A long conversation used to be one unbroken wall of fully-expanded turns. Each turn's strip now discloses/hides its own body; only the newest turn opens by default, a turn the user has explicitly opened or closed stays that way across new turns, and jumping to a turn via the rail or search force-opens it. Expand-all/collapse-all lives in the turn index header. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(chat): show Claude's slash commands from the moment a session opens (#41) Claude's CLI does not report `slash_commands` until it has processed a first turn's `system/init`, which only arrives after the first message is written to its stdin. The chat session spawns the process as soon as a chat is opened, well before any message is sent, so the command menu and its composer button stayed empty — looking broken — until a throwaway message unlocked them. The adapter now advertises a static baseline built from this app's own table of Claude's built-in commands as soon as it starts, so the menu and button are populated immediately. The real `init` list — including any project or plugin commands — still arrives with the first turn and replaces this baseline outright, exactly as before. ACP runtimes (kimi, omp) already report commands during their handshake, before any message is sent, so they were never affected. codex, grok and pi have no command support at all; the composer already hides the button entirely rather than showing an empty menu, which is the honest behavior for a runtime that offers none. Closes #30 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(chat): make /clear and /new actually reset the conversation (#47) `/clear` and `/new` forwarded their text to the still-running agent process like any other message, then only added a cosmetic marker that hid prior messages in the UI. The next message sent it right back into the same process, so the agent kept whatever context it already had — the reset never really happened, only its display did. Route clearing commands through the same restart path a manual "start fresh" relaunch already uses: stop the live adapter and start a new one with no resume id, so the next turn talks to a process that was never handed the prior conversation. This also fixes the id persisted for later reconnects, so a rejoin can't resurrect it either. Fixes #43 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(chat): show the answer as it streams, and keep a model override across /clear (#49) Five defects introduced by the four PRs already merged onto the 5.1.2 branch. Each one's own tests passed; all but one lived where two of them met. The chat stopped rendering. MessageList looked messages up in a map memoised on the messages array, but the transcript appends to that array in place, so its identity never changes: the map was built once at mount and every message after it resolved to nothing. The strips above kept ticking while the bodies stayed empty, and reloading "fixed" it because reloading replaces the array. Keyed on the transcript version instead, which is the cadence the neighbouring memo already used — the list still does not re-render per token. A model override no longer survives /clear. The restart replays the options the session was launched with, and the model is the one thing in them that can change while the session is alive, so the conversation quietly went back to the model it opened with after the browser had been told the switch applied. The choice now reaches those options too, by either door: the picker, and a /model typed straight into the composer, which was forwarded untouched and hit the same reversion. A collapsed turn's title escaped its bar. It set neither a size nor nowrap while every sibling in that fixed-height row sets both, so it inherited the page default and wrapped — and without nowrap the ellipsis was inert. An override could not be undone. The server had a "cleared" path, message and all, that nothing in the UI could reach: the text field refuses to submit empty and every listed model carries a name. A typo therefore stayed in force for every later launch of that conversation. Model names are now capped and stripped of control characters, which would otherwise ride into the best-effort /model turn as extra lines. The browser checks gated nothing: npm test runs only the unit suite, and the runner exited 0 when Chrome was absent. With the rendering bug restored, npm test passes 1357/1357 while the browser check fails. They now run in CI and refuse to skip there. Their fixture also grew a second turn, without which no turn is ever collapsed and the broken title was never rendered at all. Every fix has a test confirmed to fail against the unfixed code. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five defects introduced by the four PRs already merged onto
chore/version-bump-5.1.2(#39, #40, #41, #47). Each PR's own tests passed; all but one lived where two of them met — which is why every gate was green while the chat was visibly broken.Closes #48.
The chat stopped rendering
User-reported, and the worst of the five.
MessageListlooked messages up in amessageByIdmap memoised on[messages]— but the transcript appends to its array in place, so that array's identity never changes. The map was built once at mount and every message added afterwards resolved toundefinedand rendered nothing.The turn strips kept moving (they come from a memo that does depend on
version), so the header, tool counts and cost ticked away above a body that stayed empty, and a reload made everything appear at once — because reloading replaces the array.Keyed on
versioninstead. This does not make the list re-render per token:apply()bumpsversionon every event but only notifies the list listeners on structural ones, so the map is rebuilt at exactly the cadence the neighbouringmessagesmemo already used.A model override no longer survives
/clear/clearrestarts the agent in place from the options the session was launched with, and the model is the one thing in them that can change while the session is alive. So a conversation quietly went back to the model it opened with — after the browser had already been told the switch was applied, and while the record still held the override.The choice now reaches those options by either door: the picker, and a
/modeltyped straight into the composer, which was forwarded untouched and hit the same reversion.A collapsed turn's title escaped its bar
It set neither a font size nor
nowrap, while every sibling in that fixed 28px row sets both — so it inherited the page default and wrapped, and withoutnowrapthe ellipsis was inert. Measured at 924px: 68px of content in a 28px bar.An override could not be undone
The server had a
clearedpath, user-facing message and all, that nothing in the UI could reach — the text field refuses to submit empty and every listed model carries a name. A typo therefore stayed in force for every later launch of that conversation. There is now a way back to the runtime's default.Model names are also capped and stripped of control characters, which would otherwise ride into the best-effort
/modelturn as extra lines.The browser checks gated nothing
npm testruns only the unit suite; the browser checks were a separate script no workflow invoked, and the runner exited 0 when Chrome was absent. With the rendering bug restored,npm testpasses 1357/1357 while the browser check fails. They now run in CI and refuse to skip there.Their fixture also grew a second turn — without one, no turn is ever collapsed, so the broken title was never rendered during the check at all.
Verification
Typecheck clean · 1359 unit tests · 54/54 browser checks · zero failures.
Every fix has a test that was confirmed to fail against the unfixed code:
The live-rendering check also asserts the transcript holds the streamed message, and that passes in both states — so a failure can only mean the view did not render, never that the test fed it bad events.
Notes
🤖 Generated with Claude Code