fix(web): restore manual sort drag and keep per-group expand state#2221
Conversation
pingdotgg#2055 silently regressed two sidebar behaviors by changing how projects are keyed in `uiStateStore`: - Manual-sort drag reorder snapped projects back to their original position. Writers populated `projectOrder` with physical keys (env + cwd) but readers matched items by scoped keys (env + project id), so `preferredIds` never matched and manual order was discarded. - Expand/collapse state was wiped whenever projects were grouped (grouping != `separate`), because `syncProjects` seeded `projectExpandedById` by physical key while the UI read and wrote it by logical (group) key. Route readers and writers through a single source of truth: - `getProjectOrderKey` centralizes the physical-key derivation used by `projectOrder` so future callers cannot silently drift again. - `SyncProjectInput` now carries both a physical `key` (sort order) and a `logicalKey` (expand/collapse). `syncProjects` keys `projectExpandedById` by `logicalKey`, and persistence tracks the member cwds per group so localStorage hydration still works for grouped projects.
When late-arriving project metadata flips the grouping identity for a project (e.g. physical key to repository canonical key), the row itself did not change but its entry in `projectExpandedById` moved to a new key. Before this change, we fell through to the persisted-cwds fallback, which often meant silently defaulting the row back to expanded. Track the previous logical key per physical key across syncs, then if a project's new logical key is unseen, fall back to the previous logical key's expand state before touching the persisted fallback.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This bug fix introduces non-trivial changes to sidebar project state management, including new keying strategies (physical vs logical keys), persistence format migration, and complex fallback logic for preserving expand state across restarts. The extensive new tests demonstrate the scope of behavioral changes. Given the complexity and the author being new to this code, human review is warranted. You can customize Macroscope's approvability policy. Learn more. |
When the user collapsed every sidebar project row, the next launch re-expanded them all. `persistState` only wrote cwds of expanded projects, and an empty `expandedProjectCwds` array on rehydrate is indistinguishable from a fresh install, so the `syncProjects` fallback returned `true`. Persist `collapsedProjectCwds` alongside `expandedProjectCwds` so the fallback can tell "user collapsed everything" apart from "no info yet". For one session after upgrade, sessions whose persisted blob predates this field keep the old "not in expanded list = collapsed" semantic so nothing flips on the first restart; the next persistState writes the new shape and the legacy branch goes dormant.
Locks in the persistState/hydrate/syncProjects contract behind the collapsed/expanded fix: - All-collapsed state survives a restart (the original regression). - Mixed expand state is preserved and a brand-new project defaults to expanded under the new shape. - Legacy "not in expandedProjectCwds = collapsed" semantic is preserved for one upgrade session when collapsedProjectCwds is missing. Exports PERSISTED_STATE_KEY, PersistedUiState, hydratePersistedProjectState, and persistState so the tests can drive a deterministic localStorage round-trip without spinning up the zustand store.
Closes coverage gaps Codex flagged in the prior commit: - projectOrderCwds round-trip: reorder, persist, hydrate, sync, and assert the manual order survives. Catches regressions where ordering persistence breaks while the expand-state tests still pass. - Restart with logical-key change: covers the persisted-cwd fallback in syncProjects, which is the only path that can carry collapse state across a restart when a project also moves into a new logical group. The existing in-memory pure-function test does not exercise this because it only covers same-session migration.
Dismissing prior approval to re-evaluate fb544d9
|
Pushed three more commits that fix a related pre-existing bug. Also closes #1910. The bugPre-existing (not caused by this PR's grouping refactor): collapse all sidebar projects, restart the app, and they all re-expand. Root cause is that The fix (
|
Upstream additions: - fix(web): restore manual sort drag and keep per-group expand state (pingdotgg#2221) - fix: Change right panel sheet to be below title bar / action bar (pingdotgg#2224) - Refactor OpenCode lifecycle and structured output handling (pingdotgg#2218) - effect-codex-app-server (pingdotgg#1942) - Redesign model picker with favorites and search (pingdotgg#2153) - fix(server): prevent probeClaudeCapabilities from wasting API requests (pingdotgg#2192) - fix(server): handle OpenCode text response format in commit message gen (pingdotgg#2202) - Devcontainer / IDE updates (pingdotgg#2208) - Expand leading ~ in Codex home paths before exporting CODEX_HOME (pingdotgg#2210) - fix(release): use v<semver> tag format for nightly releases (pingdotgg#2186) Fork adaptations: - Took upstream's redesigned model picker with favorites and search - Removed deleted codexAppServerManager (replaced by effect-codex-app-server) - Stubbed fetchCodexUsage (manager-based readout no longer available) - Extended PROVIDER_ICON_BY_PROVIDER for all 8 fork providers - Extended modelOptionsByProvider test fixtures for all 8 providers - Inline ClaudeSlashCommand type (not yet re-exported from SDK) - Updated SettingsPanels imports for new picker module structure - Preserved fork's CI customizations (ubuntu-24.04 not Blacksmith)
Summary
projectOrderwith physical keys while readers matched items by scoped keys, sopreferredIdsnever matched. Same reader/writer mismatch as [Bug]: v0.0.16 regression: drag-and-drop project reordering silently fails when sort is set to Manual #1902, re-introduced in feat: configurable project grouping #2055.separate) lost their expand/collapse state on everysyncProjectsrun, becauseprojectExpandedByIdwas written under physical keys while the UI reads/writes by logical (group) keys.Fixes #2219.
Repro
Manual. Drag a project. It snaps back.Group by repository. Collapse a grouped row. Trigger anything that re-enterssyncProjects(e.g. a project refresh). It re-expands.Diagnosis
apps/web/src/uiStateStore.tstracksprojectOrderandprojectExpandedById. PR #2055 changed the writers to use physical keys (env + normalized cwd) but did not update the sibling readers.For
projectOrder:handleProjectDragEndinSidebar.tsx:2857passesmember.physicalProjectKeytoreorderProjects(see lines 2870-2873).orderedProjectsmemo inSidebar.tsx:2707anduseHandleNewThread.ts:168both usegetId: (project) => scopedProjectKey(scopeProjectRef(project.environmentId, project.id)).For
projectExpandedById:syncProjectsseeded entries keyed by the physical key.separategrouping modes is the repo canonical key, not the physical key.Why this approach
For
projectOrder, I extracted a named helper so any future caller writingprojectOrdercan be pinned to the same derivation as the readers:Both
orderedProjectsanduseHandleNewThreadnow usegetProjectOrderKeyas theirgetId, matching the drag-end writer. That way the next silent divergence becomes a grep target, rather than a deploy-day regression.For
projectExpandedById, I splitSyncProjectInputinto a physicalkey(used forprojectOrder, stable across grouping changes) and alogicalKey(used for expand/collapse, matches what the UI reads).syncProjectsnow keysprojectExpandedByIdbylogicalKey, and localStorage persistence tracks the member cwds per group so hydration still works for grouped projects (no migration needed).syncProjectsalso preserves expand state when a project's logical key changes mid-session -- for example, late-arriving repo metadata flipping the grouping identity from the physical key to the repo canonical key. Without that, the row would unexpectedly reopen the instant metadata arrived.Considered alternatives
Rolling back #2055's writer-side choice would also solve regression A, but #2055 keyed persistence by cwd to survive project id churn, which is the right direction. Aligning the readers (and disentangling the two pieces of state) is the smaller, forward-compatible fix.
Known limitation
If the user toggles
sidebarProjectGroupingModeat runtime without any project snapshot events following,syncProjectsdoes not re-run and existingprojectExpandedByIdentries stay keyed under the previous grouping until the next project event arrives. Today the most common flow triggers a sync before the user notices, but a subscriber that re-runssyncProjectUiFromStorewhen grouping settings change would make this tighter. I left that out to keep this PR small; happy to fold it in if you prefer.Test plan
bun fmtbun lintbun typecheckbun run test(938 tests pass, 1 file skipped, no new failures)apps/web/src/components/Sidebar.logic.test.ts: assertsgetProjectOrderKeyis whatorderItemsByPreferredIdsuses to honorprojectOrder(locks in the snap-back fix).apps/web/src/uiStateStore.test.ts:syncProjects keys projectExpandedById by the logical key, not the physical key(two physical projects sharing a logical group both respect a collapsed logical-key entry).apps/web/src/uiStateStore.test.ts:syncProjects preserves expand state when a project's logical key changes(covers the late-metadata flip).Manualand dragged several projects into new positions (rows that used to snap back). They stayed put.Group by repository, collapsed several grouped rows.Note
Medium Risk
Touches sidebar ordering and
uiStateStorepersistence/keying logic; regressions could affect project ordering/expand state across sessions, but scope is contained to client UI state.Overview
Restores manual project drag ordering by making sidebar readers (
Sidebar,useHandleNewThread) identify projects using the same physical key (environmentId+ normalizedcwd) that drag handlers/store write, via newgetProjectOrderKey.Refactors project UI state syncing/persistence to support grouped projects:
syncProjectsnow distinguishes physicalkey(ordering) fromlogicalKey(expand/collapse), preserves expand state when logical keys change, and persists bothcollapsedProjectCwdsand ordering cwds to avoid “everything re-expands” and order loss on restart. Adds a non-hookgetClientSettings()accessor so runtime services can computelogicalKeyduring project sync.Adds regression tests covering physical-key ordering, logical-key expand state, logical-key churn, and persistence round-trips for collapse/order.
Reviewed by Cursor Bugbot for commit fb544d9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix manual sort drag and per-group expand/collapse state persistence in sidebar
getProjectOrderKeyin logicalProject.ts as the canonical key for project sort order, then updates Sidebar.tsx and useHandleNewThread.ts to use it, preventing manual drag order from snapping back.syncProjectsin uiStateStore.ts to key expand/collapse state by logical group rather than physical project id, preserving collapsed rows across project id churn and restarts.collapsedProjectCwdsto the persisted state shape and updatespersistState/hydratePersistedProjectStateso an all-collapsed sidebar remains collapsed after a page reload.logicalKey(derived viaderiveLogicalProjectKeyFromSettings) intosyncProjectsfrom service.ts to support grouping-aware expand state.collapsedProjectCwds) defaults all projects to collapsed for one session to avoid ambiguity, then writes the new format on next persist.Macroscope summarized fb544d9.