fix: restore model from session messages on load#346
fix: restore model from session messages on load#346jensenojs wants to merge 1 commit intosudo-tee:refactor/renderer-componentsfrom
Conversation
|
Sorry, I haven't really tried this branch yet, but the reason I encountered this issue is because I temporarily wanted to try Mimo-v2-pro today (my impression of it is pretty good, it makes me feel more emotionally stable than kimi2.5). @sudo-tee Proving the correctness of this PR is more complex than I imagined, as it involves data races. But I believe it's correct. I'll take another look later. |
There was a problem hiding this comment.
Pull request overview
This PR consolidates “current model” initialization into core.initialize_current_model() and updates the renderer to always invoke it after loading a session, aiming to restore the model/mode from session message history instead of defaulting to the config model on reload.
Changes:
- Removed renderer-local logic that derived model/mode from message history.
- Updated
renderer._render_full_session_data()to always callcore.initialize_current_model()after session render. - Expanded
core.initialize_current_model()to restore model/mode from the most recent message (falling back to config only if needed).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lua/opencode/ui/renderer.lua | Removes local model/mode restoration and calls core.initialize_current_model() after session render. |
| lua/opencode/core.lua | Makes initialize_current_model() restore model/mode from state.messages before falling back to existing/config values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| M.initialize_current_model = Promise.async(function() | ||
| if state.messages then | ||
| for i = #state.messages, 1, -1 do | ||
| local msg = state.messages[i] | ||
| if msg and msg.info and msg.info.modelID and msg.info.providerID then | ||
| local model_str = msg.info.providerID .. '/' .. msg.info.modelID | ||
| if state.current_model ~= model_str then | ||
| state.model.set_model(model_str) | ||
| end | ||
| if msg.info.mode and state.current_mode ~= msg.info.mode then | ||
| state.model.set_mode(msg.info.mode) | ||
| end | ||
| return state.current_model | ||
| end |
There was a problem hiding this comment.
initialize_current_model() now prioritizes state.messages even when state.current_model is already set. Since this function is called in non-session-load paths (e.g. core.send_message() sets opts.model = opts.model or M.initialize_current_model():await()), this will override a user-selected model (via configure_provider() / state.model.set_model) back to whatever model was used in the most recent message, effectively making model switching not stick until after a new assistant message is produced.
Suggested fix: don’t apply the “restore from messages” logic when state.current_model is already set, or gate it behind an explicit option (e.g. initialize_current_model({ prefer_messages = true })) so only the session-load renderer path forces restoration from message history.
There was a problem hiding this comment.
The edge case you described only occurs when a user switches models in TUI without sending a message. If they switch and send, the message history is updated with the new model, and nvim will correctly use it on the next send.
Given that users typically switch models to use them immediately, this edge case is rare. The current fix prioritizes simplicity over handling this specific scenario with an additional API option.
| if set_mode_from_messages then | ||
| set_model_and_mode_from_messages() | ||
| end | ||
| require('opencode.core').initialize_current_model() |
There was a problem hiding this comment.
_render_full_session_data() now unconditionally calls core.initialize_current_model() after rendering. If initialize_current_model() is updated to take an option (or otherwise gate message-based restoration), consider calling the “restore from rendered session messages” behavior explicitly here so session reload uses the session’s last model/mode, without changing model selection during other code paths that call initialize_current_model() (e.g. sending a message).
| require('opencode.core').initialize_current_model() | |
| local core = require('opencode.core') | |
| if core and core.initialize_current_model then | |
| core.initialize_current_model({ restore_from_rendered_session_messages = true }) | |
| end |
Consolidate model initialization logic into core.initialize_current_model() to respect user-switched models when reloading sessions, rather than always overriding with the config default.