Repository navigation
fix(tui): keep model selection owned by the tab - #54367
cleardevice wants to merge 2 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The cross-client case from #54363 is fixed. On the base, TUI A's footer switched to the model picked in TUI B; with this change it stays on its own model. I checked this with the issue's opencode-drive script on both checkouts. bun test test/context and bun run typecheck pass in packages/tui, and both new tests fail on the base.
Public API: unchanged. Only TUI-internal selection state is touched.
Concerns:
/newand/clearstill lose the session's variant. The handlers inpackages/tui/src/app.tsxstill calllocal.model.set(model)(andlocal.agent.set) right afterroute.navigate({ type: "home" }). That goes throughselectModel, which storespreferredSelection(model), so the variant carried over from the session is replaced by the shared per-model variant preference. In a quick test (session onthird/high, shared preferencethird=low), going home and callinglocal.model.set(...)as/newdoes left the selection onlow; without that call it stays onhigh. Since the description says the carry-over covers/newand/clearin one place, removing those re-sets inapp.tsxlooks like the simplest fix. A test of that flow with the shared variant preference set to something different would cover it.- Overlap with #52533. The new effect also writes a fallback into session tabs that have no stored model, which is what the open #52533 does. Whichever lands second will need to be reconciled so there is one mechanism.
| const current = agent.current() | ||
| if (!current) return false | ||
| setSelectionState("newSessionModelByLocationAgent", locationAgentKey(current.id), model) | ||
| setSelectionState("newSessionModelByLocationAgent", locationAgentKey(current.id), preferredSelection(model)) |
There was a problem hiding this comment.
On the new-session screen, selectModel always stores preferredSelection(model), so the variant comes from the shared preference. The /new and /clear handlers in app.tsx call local.model.set(model) right after navigating home, so they overwrite the variant that the carry-over just set from the session. I confirmed this with a small test: session third/high, shared preference third = low, navigate home, then local.model.set(...) leaves the selection on low. Either drop those re-sets in app.tsx (the carry-over already handles them), or keep the existing draft's variant here when the model is unchanged, as the session branch above does.
9798096 to
5bf057b
Compare
|
Thanks, both points addressed:
|
5bf057b to
73f903d
Compare
|
Rebased onto current dev, tests pass, still relevant. |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Closes #54363
Type of change
What does this PR do?
/new,/clear, the tab rail "+" and the open menu in one place. Agents with a configuredmodelkeep it.How did you verify your code works?
bun test test/contextandbun typecheckinpackages/tui. The two new tests inlocal-selection.test.tsxfail onv2without the change.Screenshots / recordings
Before / after recordings below.
Checklist