Refactor state management into smaller slices#321
Conversation
Move window state, cursor, visibility, and hidden-buffer helpers into a dedicated state.ui table. Update callers across modules and tests to reference state.ui.*, improving separation between core store logic and UI utilities.
865ed29 to
726772b
Compare
726772b to
5c2fa45
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors opencode.state into a protected, domain-based state store (UI/session/jobs/model/renderer/context) and updates the core UI/runtime code plus tests to use the new mutation APIs instead of direct state writes.
Changes:
- Introduces a new protected state store (
opencode.state.store) and domain-specific state mutation modules (state.ui,state.session,state.jobs,state.model,state.renderer,state.context). - Updates core runtime modules (UI, renderer, core, server job, context) to mutate state exclusively via domain setters.
- Updates unit/replay/manual tests to align with the new state mutation APIs and adds a direct-write error assertion.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/zoom_spec.lua | Updates zoom/window state mutations to use state.ui setters. |
| tests/unit/state_spec.lua | Reworks observable-state tests to use domain setters and adds direct-write error test. |
| tests/unit/snapshot_spec.lua | Switches active-session state writes to state.session setters. |
| tests/unit/session_spec.lua | Uses state.jobs.set_api_client instead of direct state.api_client writes. |
| tests/unit/server_job_spec.lua | Migrates server state manipulation to state.jobs APIs. |
| tests/unit/render_state_spec.lua | Uses state.renderer.set_messages rather than direct state.messages writes. |
| tests/unit/persist_state_spec.lua | Refactors multiple state writes to use state.* domains and store.set for generic keys. |
| tests/unit/permission_integration_spec.lua | Moves message/permission/session setup to state.renderer + state.session. |
| tests/unit/loading_animation_spec.lua | Uses state.session setters for active session state. |
| tests/unit/input_window_spec.lua | Updates window/input-content/display-route state mutations to state.ui APIs. |
| tests/unit/hooks_spec.lua | Migrates active session, user_message_count, and pending permissions mutations to new domains. |
| tests/unit/event_manager_spec.lua | Uses state.jobs for server state changes. |
| tests/unit/dialog_spec.lua | Updates window state setup/teardown to state.ui. |
| tests/unit/cursor_tracking_spec.lua | Moves cursor + window state operations to state.ui and uses store.set for cursor persistence keys. |
| tests/unit/core_spec.lua | Updates tests to use domain setters and store.set when restoring state keys. |
| tests/unit/context_spec.lua | Migrates context/session/opening mutations to state.context/state.session/state.ui. |
| tests/unit/context_bar_spec.lua | Uses state.ui.set_windows for context bar rendering tests. |
| tests/unit/config_file_spec.lua | Migrates api client setup to state.jobs.set_api_client. |
| tests/unit/api_spec.lua | Uses state.session for active session and state.jobs for api client; uses state.model for current_model. |
| tests/unit/api_client_spec.lua | Uses state.context.set_current_cwd instead of direct writes. |
| tests/replay/renderer_spec.lua | Migrates replay tests to state.session and state.renderer. |
| tests/minimal/init.lua | Stubs Tree-sitter start in minimal test init to avoid side effects. |
| tests/manual/renderer_replay.lua | Updates manual replay script to use state.session and state.jobs APIs. |
| tests/helpers.lua | Uses state.model.set_mode and state.ui.set_windows in test setup. |
| lua/opencode/variant_picker.lua | Writes selected variant via state.model.set_variant. |
| lua/opencode/ui/ui.lua | Migrates UI state reads/writes (cursors, windows, hidden snapshots, zoom) to state.ui APIs. |
| lua/opencode/ui/session_picker.lua | Uses state.session.set_active when deleting current session. |
| lua/opencode/ui/renderer.lua | Migrates renderer-related state to state.renderer and model/mode to state.model. |
| lua/opencode/ui/output_window.lua | Uses state.ui cursor/window focus and saved-options setters. |
| lua/opencode/ui/input_window.lua | Uses state.ui cursor/window focus and input-content setters. |
| lua/opencode/ui/autocmds.lua | Migrates last-code-window/current-buf and cwd/panel-focused updates to state.ui/state.context. |
| lua/opencode/state/ui.lua | New UI state domain module (windows, focus, cursors, hidden snapshots, toggle decision logic). |
| lua/opencode/state/store.lua | New protected store implementation with subscriptions/emits and guarded writes. |
| lua/opencode/state/session.lua | New session state domain module (active session, restore points, last sent context, counts). |
| lua/opencode/state/renderer.lua | New renderer state domain module (messages, permissions, stats). |
| lua/opencode/state/model.lua | New model state domain module (mode/model/variant + overrides). |
| lua/opencode/state/jobs.lua | New jobs state domain module (job count, server, api client, event manager, CLI version). |
| lua/opencode/state/init.lua | New opencode.state entry point exposing store + domains and forbidding direct writes. |
| lua/opencode/state/context.lua | New context state domain module (context config, timestamps, cwd). |
| lua/opencode/state.lua | Thin wrapper re-exporting opencode.state.init. |
| lua/opencode/snapshot.lua | Updates restore-point state writes to state.session APIs. |
| lua/opencode/server_job.lua | Migrates job count + server state mutations to state.jobs. |
| lua/opencode/quick_chat.lua | Uses state.session.set_active for debug quick-chat session wiring. |
| lua/opencode/image_handler.lua | Updates context timestamp mutation to state.context. |
| lua/opencode/event_manager.lua | Uses state.jobs.set_event_manager during setup. |
| lua/opencode/core.lua | Migrates many state writes (visibility checks, open/close, model/mode, session, context config, job/server) to domains. |
| lua/opencode/context/chat_context.lua | Updates context timestamp writes to state.context. |
| lua/opencode/context.lua | Switches current_context_config updates to state.context setters (and refactors ensure logic). |
| lua/opencode/api.lua | Uses state.ui for window state/visibility/toggle decisions and state.session for active session writes. |
| lua/opencode/api_client.lua | Uses state.jobs.set_server when ensuring server as fallback. |
Comments suppressed due to low confidence (1)
lua/opencode/ui/ui.lua:169
hide_visible_windowsmutates fields on thestate.windowstable directly (state.windows.input_win = nil, etc.). With the new protected store approach, mutating nested tables bypassesstore.set/change notifications and can leave subscribers observing stale window state. Consider updating windows via a dedicated state.ui helper (or re-setting the whole windows table throughstate.ui.set_windows(...)) instead of mutating nested fields in place.
state.ui.stash_hidden_buffers(snapshot)
if state.windows == windows then
state.windows.input_win = nil
state.windows.output_win = nil
state.windows.footer_win = nil
state.windows.output_was_at_bottom = snapshot.output_was_at_bottom
end
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
638b05f to
dfbbe6b
Compare
2414691 to
6f6866f
Compare
6f6866f to
dac4648
Compare
ed89809 to
13c630e
Compare
Replace top-level observable helpers (state.subscribe, state.unsubscribe, state.append, state.emit)
|
So, after refactoring the PR, it will be considered for direct merging into the main branch, right? -- Many of my tasks depend on each other. I can only make each commit as readable as possible, but the overall change may be large. Should I just produce a draft? Even so, that could still make the review process painful. It might sound a bit odd, but could I create a personal branch here with you? I don't know if it would help our local workflow (I would only create one or two at most), and it would make management easier I don't have much experience collaborating this way, so please handle it however you're most comfortable. (I also never got the hang of GitHub branch permissions.) |
|
Yes once completed it will be merged to main. I'm almost done. I am using this branch daily and it's pretty stable. |
|
As per branch collaboration usually there is a checkbox when creating a PR where you call allow the owner to make changes. I'm mobile at the moment so I can't really check. I will try to merge this branch today or tomorrow. So you can start from it |
* fix(ui): handle split resize safely Use window-type-aware resize logic in output_window.update_dimensions. - guard missing or invalid output_win before resizing - use nvim_win_set_width for split windows (relative == '') - keep nvim_win_set_config for floating windows - add regression tests for float-focus resize and invalid window This prevents "Cannot split a floating window" on VimResized while preserving existing zoom width behavior. Verified with: - ./run_tests.sh -t tests/unit/zoom_spec.lua * fix(reference-picker): use store subscribe Follow the state observable API migration by switching reference picker setup from state.subscribe(...) to state.store.subscribe(...). This matches the refactor that centralized observable helpers under state.store and fixes startup error: attempt to call field 'subscribe' (a nil value). Verified with: ./run_tests.sh -t tests/unit/reference_picker_spec.lua --------- Co-authored-by: oujinsai <oujinsai@baidu.com>
This pull request refactors the codebase to centralize and encapsulate state management by replacing direct access to state variables with method calls on new or existing state submodules (such as
state.session,state.model,state.ui, andstate.jobs). This improves code maintainability, readability, and reduces the risk of inconsistent state updates.