onev.cat / Prowl
native macOS codings agent orchestrator
5
fork

Configure Feed

Select the types of activity you want to include in your feed.

Revise sidebar refactor plan from review

onevcat 4ac50a7b 4c9b72e3

+175 -66
+175 -66
doc-onevcat/plans/2026-05-03-sidebar-container-refactor-plan.md
··· 12 12 13 13 - incorrect repository drag insertion indicators when dragging downward across expanded repositories 14 14 - unstable bulk expand/collapse animations 15 - - potential sidebar flicker during drag when live terminal / notification / ordering updates arrive 15 + - potential sidebar flicker during drag when live terminal, notification, or ordering updates arrive 16 16 17 17 ## Current Findings 18 18 ··· 56 56 57 57 The current `List` carries several behaviors at once: 58 58 59 - - special rows: Canvas, Shelf, archived worktrees, repository list header 59 + - archived worktree selection and repository list header 60 60 - repository row selection for plain folders 61 61 - worktree multi-selection 62 62 - repository expand/collapse ··· 65 65 - reveal-in-sidebar via `ScrollViewReader.scrollTo` 66 66 - native sidebar styling and accessibility 67 67 68 - This makes local fixes brittle because changing row structure affects selection, drag, animation, and scroll identity at the same time. 68 + Canvas, Shelf, and the footer are not `List` rows today; they are safe-area inset chrome around the list. The refactor should preserve that boundary unless a later design intentionally moves them into the scroll content. 69 69 70 70 ### 3. Live state still reaches rows during drag 71 71 ··· 80 80 81 81 These are not necessarily the root cause of the drop-indicator bug, but they are credible contributors to #222-style flicker. 82 82 83 - ## Recommended Direction 83 + ## Revised Direction 84 84 85 85 Use a custom sidebar scroll container rather than trying to keep the current flat `List` structure. 86 86 87 + Execution order matters: first stabilize the old `List` path with a reducer-level drag gate, then replace the visual structure. The drag gate is a hard prerequisite because it reduces #222 risk before the broader #249 rewrite starts. 88 + 87 89 Recommended shape: 88 90 89 91 ```text 90 - ScrollViewReader 91 - └── ScrollView 92 - └── LazyVStack or VStack 93 - ├── Special rows 94 - ├── RepositoryContainerRow 95 - │ ├── RepositoryHeaderRow 96 - │ └── WorktreeRows 97 - └── FailedRepositoryRow 92 + SidebarView chrome 93 + ├── top safeAreaInset buttons 94 + │ ├── Canvas 95 + │ └── Shelf 96 + ├── ScrollViewReader 97 + │ └── ScrollView 98 + │ └── LazyVStack or VStack 99 + │ ├── repository list header 100 + │ ├── RepositoryContainerRow 101 + │ │ ├── RepositoryHeaderRow 102 + │ │ └── WorktreeRows 103 + │ ├── FailedRepositoryRow 104 + │ └── ArchivedWorktreesRow 105 + └── bottom safeAreaInset footer 98 106 ``` 99 107 100 108 Key property: repository containers are the only repository-level siblings in the outer stack. Expanded worktrees are children inside the container, not siblings beside it. ··· 140 148 - must rebuild keyboard navigation, multi-selection, reorder, and accessibility affordances 141 149 - more implementation work 142 150 143 - This is the recommended route for #249 if the goal is "fix the sidebar design once" rather than patch one symptom. 151 + This remains the recommended route for #249 if the goal is "fix the sidebar design once" rather than patch one symptom. 144 152 145 153 ### Option C: Short-term drag-time freeze only 146 154 ··· 154 162 - does not fix repository insertion indicator because row boundaries remain wrong 155 163 - leaves the main structural mismatch in place 156 164 157 - This can be kept as a subset of Option B, but it is not enough alone. 165 + This is now the mandatory M1 prerequisite for Option B, not a replacement for Option B. 166 + 167 + ## Hard Requirements 168 + 169 + The refactor must preserve these behavior and state contracts. 170 + 171 + ### Reducer actions and persistence 172 + 173 + - Keep the existing reducer actions and persistence paths for repository and worktree ordering unless a later implementation note explicitly proves a rename is worth it. 174 + - Preserve calls behind repository reorder, pinned worktree reorder, unpinned worktree reorder, and notification-driven reorder. 175 + - Treat failed repository reorder semantics as an explicit product decision: 176 + - either failed repository rows are reorderable and their order persists through the same root ordering path 177 + - or they are not reorderable and the UI gives consistent feedback with no insertion target around them 178 + 179 + ### Expanded and collapsed state 180 + 181 + - Preserve `@Shared` write-back semantics for collapsed repository IDs. 182 + - Preserve cleanup of invalid collapsed IDs when repository IDs change. 183 + - Ensure bulk expand/collapse and single expand/collapse share the same model path. 184 + 185 + ### Focused actions and selection synchronization 186 + 187 + - Preserve `SidebarView` focused values for `confirmWorktreeAction`, `archiveWorktreeAction`, `deleteWorktreeAction`, and `visibleHotkeyWorktreeRows`. 188 + - Preserve the `sidebarSelections -> setSidebarSelectedWorktreeIDs` synchronization currently owned by `SidebarView`. 189 + - Do not regress menu commands or numbered worktree hotkeys when replacing `List(selection:)`. 190 + 191 + ### Existing row affordances 192 + 193 + - Preserve repository and worktree context menus. 194 + - Preserve drag previews. 195 + - Preserve current worktree row type-select behavior. Worktree rows currently use `.typeSelectEquivalent("")`; V1 should keep type-select effectively disabled for those rows. 196 + - Preserve root-level `dropDestination(for: URL.self)` on the sidebar container, including drops into blank sidebar space. 197 + 198 + ### Ordered roots 199 + 200 + - Converge the current `orderedRoots.isEmpty` fallback and non-empty custom-order path into one presentation path. 201 + - The empty ordered-roots case is a valid user state and must have tests. 158 202 159 203 ## Proposed Architecture 160 204 161 - ### SidebarPresentationModel 205 + ### SidebarPresentation 162 206 163 207 Introduce a pure presentation model that flattens current repository state into explicit sidebar units. 164 208 ··· 171 215 172 216 enum SidebarItem: Equatable, Identifiable { 173 217 case listHeader(SidebarListHeaderModel) 174 - case special(SidebarSpecialRowModel) 175 218 case repository(SidebarRepositoryContainerModel) 176 219 case failedRepository(FailedRepositoryModel) 220 + case archivedWorktrees(ArchivedWorktreesRowModel) 177 221 } 178 222 179 223 struct SidebarRepositoryContainerModel: Equatable, Identifiable { ··· 189 233 190 234 Rules: 191 235 236 + - build `SidebarPresentation` from reducer/state-side pure functions or equivalent helpers 192 237 - outer `items` contains one item per repository, not one item per row 193 238 - worktree sections remain inside the repository container 194 - - presentation construction should be pure and unit-tested 195 - - live terminal state should not be part of the broad presentation model unless required for layout identity 239 + - presentation construction is pure and unit-tested 240 + - high-frequency terminal notification/task/run-script state stays in leaf views, not in broad presentation state 241 + - Canvas, Shelf, and footer chrome remain outside `SidebarPresentation` in V1 196 242 197 243 ### Selection 198 244 199 245 Replace `List(selection:)` with explicit selection handling. 200 246 201 - Keep `RepositoriesFeature.State.selection` and `sidebarSelectedWorktreeIDs` as the source of truth, but route clicks through helper functions: 247 + Keep `RepositoriesFeature.State.selection` and `sidebarSelectedWorktreeIDs` as the source of truth, but route clicks through helper functions. 202 248 203 - - repository header click: 204 - - plain folder: select repository 205 - - git repository: toggle expand by default, or select repository if a future repository-detail mode needs it 206 - - worktree row click: 207 - - normal click: select worktree and focus terminal 208 - - Cmd-click: toggle multi-selection 209 - - Shift-click: optional follow-up, only if current behavior supports it through `List` 210 - - Canvas / Shelf / Archived rows: dispatch existing actions 249 + Compatibility matrix: 250 + 251 + | Interaction | State behavior | Focus behavior | 252 + | --- | --- | --- | 253 + | Canvas button | Selects Canvas and clears incompatible sidebar worktree selection. | Does not focus a terminal. | 254 + | Shelf button | Selects Shelf and clears incompatible sidebar worktree selection. | Does not focus a terminal. | 255 + | Archived worktrees row | Selects archived worktrees and clears incompatible worktree selection. | Does not focus a terminal. | 256 + | Git repository header click | Toggles expanded state by default. | Does not focus a terminal. | 257 + | Plain folder repository click | Selects the repository. | Does not focus a terminal unless current behavior already does. | 258 + | Worktree row normal click | Selects one worktree and updates sidebar selected worktree IDs to that one ID. | Focuses the terminal for the selected worktree. | 259 + | Worktree row Cmd-click | Toggles membership in sidebar selected worktree IDs, preserving multi-select priority. | Does not steal focus unless the resulting primary selection changes by existing rules. | 260 + | Empty sidebar selection | Clears sidebar selected worktree IDs. | Does not focus a terminal. | 211 261 212 262 Selection visuals should be explicit in `RepositoryHeaderRow` and `WorktreeRow`, not inherited from `List`. 213 263 ··· 227 277 - command shortcuts still select worktrees 228 278 - selected row is scrolled into view on reveal 229 279 - focus returns to terminal after single worktree selection 230 - - sidebar focus does not accidentally forward text while Canvas / Shelf rules say it should not 280 + - sidebar focus does not accidentally forward text while Canvas, Shelf, or Archived rules say it should not 231 281 232 282 ### Repository Reorder 233 283 ··· 238 288 - make `RepositoryContainerRow` draggable with repository ID payload 239 289 - render a custom repository insertion indicator between repository containers 240 290 - compute drop destination as a repository index 241 - - dispatch existing `.worktreeOrdering(.repositoriesMoved(offsets, destination))` or a new clearer action such as `.repositoriesReordered([Repository.ID])` 291 + - dispatch existing repository-ordering actions or a new reducer action that delegates to the same persistence path 242 292 243 293 The custom indicator should always render at repository container boundaries: 244 294 ··· 267 317 268 318 ### Drag-Time Freeze 269 319 270 - Add a small sidebar drag state to suppress non-essential row churn. 320 + Add sidebar drag state at reducer level and use it in both the old and new sidebar paths. 271 321 272 322 During any sidebar drag: 273 323 ··· 276 326 - suppress row-ID animations caused by notification-driven reorder 277 327 - defer "move notified worktree to top" until drag ends, or apply it without animation after drop 278 328 329 + Reducer behavior: 330 + 331 + - drag begin records that sidebar drag is active 332 + - `worktreeNotificationReceived` while drag is active records pending reorder IDs instead of mutating row order immediately 333 + - drag end flushes pending notification reorders in deterministic order, dropping stale worktree IDs 334 + - `moveNotifiedWorktreeToTop == false` remains a no-op 335 + 279 336 This addresses #222 without requiring every live data read to stop. 280 337 281 338 ### Expand / Collapse ··· 295 352 296 353 - repository container: `SidebarScrollID.repository(repositoryID)` 297 354 - worktree row: `SidebarScrollID.worktree(worktreeID)` 298 - - special rows: `SidebarScrollID.canvas`, etc. 355 + - archived worktrees row: `SidebarScrollID.archivedWorktrees` 299 356 300 357 When revealing a collapsed worktree: 301 358 302 359 1. expand its repository 303 - 2. yield for layout materialization 360 + 2. wait for an event-driven row availability signal 304 361 3. scroll to `SidebarScrollID.worktree(worktreeID)` 305 362 4. consume pending reveal 306 363 307 - This matches the current two-yield approach but removes dependency on `List` row materialization. 364 + Do not rely on a fixed number of `Task.yield()` calls in the new architecture. The implementation can use a scroll target registry, preference key, or equivalent view materialization signal. 308 365 309 366 ### Accessibility 310 367 ··· 328 385 - sidebar multi-selection 329 386 - reveal-in-sidebar 330 387 - Add signposts around sidebar presentation build and drag state transitions if trace work is needed. 331 - - Keep current `List` code untouched until presentation tests exist. 388 + - Establish `LazyVStack` vs `VStack` decision metrics before replacing the list: 389 + - expand/collapse latency for 10+ repositories 390 + - frame stability during repository drag 391 + - CPU peak during drag and bulk expand/collapse 392 + - body recomputation count for repository container and worktree row views 393 + - Keep current `List` code untouched until M1 and presentation tests exist. 394 + 395 + ### M1: Stabilize Old `List` Drag Behavior 396 + 397 + Files likely involved: 398 + 399 + - `supacode/Features/Repositories/Reducer/RepositoriesFeature.swift` 400 + - `supacode/Features/Repositories/Reducer/RepositoriesFeature+WorktreeOrdering.swift` 401 + - `supacode/Features/Repositories/Views/SidebarListView.swift` 402 + - `supacodeTests/RepositoriesFeatureTests.swift` 403 + 404 + Deliver: 405 + 406 + - reducer-level sidebar drag state 407 + - view action for drag begin/end from the old `List` path 408 + - delayed or no-animation handling for notification-driven reorder during drag 409 + - deterministic pending reorder flush on drag end 410 + 411 + Tests: 412 + 413 + - notification during sidebar drag does not mutate visible worktree order immediately 414 + - drag end applies the pending notification reorder in deterministic order 415 + - multiple notifications during one drag produce stable ordering 416 + - stale pending worktree IDs are ignored 417 + - `moveNotifiedWorktreeToTop == false` remains a no-op 418 + - persistence is called only when the reorder is actually applied 332 419 333 420 ### Phase 1: Pure Presentation and Reorder Mapping 334 421 ··· 343 430 - pure sidebar presentation builder 344 431 - stable scroll IDs 345 432 - pure drop-destination mapping for repository and worktree reorder 346 - - tests for: 347 - - expanded repository keeps one outer item with child rows 348 - - failed repositories preserve order 349 - - plain folders produce repository containers with no worktree children 350 - - pinned/main/pending/unpinned sections are preserved 351 - - repository drop destinations map to expected order 352 - - worktree drop destinations map within pinned/unpinned sections 433 + - one unified presentation path for empty and non-empty ordered roots 434 + - explicit failed repository row reorder semantics 435 + 436 + Tests: 437 + 438 + - expanded repository keeps one outer item with child rows 439 + - failed repositories preserve the chosen reorder semantics 440 + - plain folders produce repository containers with no worktree children 441 + - pinned/main/pending/unpinned sections are preserved 442 + - empty ordered roots and custom ordered roots produce equivalent presentation rules 443 + - repository drop destinations map to expected order 444 + - worktree drop destinations map within pinned/unpinned sections 353 445 354 446 ### Phase 2: New Container Views Behind a Switch 355 447 ··· 366 458 - render the new container sidebar behind a local compile-time or private runtime switch 367 459 - no reducer changes except new presentation helpers if needed 368 460 - preserve row styling visually before enabling custom drag/drop 461 + - preserve root-level URL drop for files dragged into blank sidebar space 462 + - preserve context menus and drag previews 369 463 370 464 This phase should be screenshot/manual verified before deleting the old `List` path. 371 465 372 - ### Phase 3: Explicit Selection and Reveal 466 + ### Phase 3: Explicit Selection, Focus, and Reveal 373 467 374 468 Deliver: 375 469 376 470 - click handling for repository and worktree rows 377 471 - explicit selection visuals 378 - - multi-selection behavior matching current sidebar expectations 379 - - reveal-in-sidebar via new scroll IDs 472 + - multi-selection behavior matching the compatibility matrix 473 + - `sidebarSelections -> setSidebarSelectedWorktreeIDs` synchronization 474 + - focused actions and hotkey row values 475 + - reveal-in-sidebar via new scroll IDs and row availability events 380 476 - focused terminal handoff after single worktree selection 381 477 382 478 Tests: 383 479 384 - - pure selection helper tests if logic is factored out 385 - - existing reducer selection tests should keep passing 480 + - pure selection helper tests 481 + - reducer tests for sidebar selected worktree synchronization 482 + - focused action manual checklist for confirm/archive/delete and numbered hotkeys 386 483 387 484 ### Phase 4: Custom Repository Reorder 388 485 ··· 390 487 391 488 - repository drag payload 392 489 - custom repo-level insertion indicator 393 - - drop handling that dispatches repository reorder 490 + - drop handling that dispatches repository reorder through the existing persistence path 394 491 - drag-time UI freeze for non-essential row affordances 395 492 396 493 Manual verification: 397 494 398 495 - dragging a repository upward shows indicator below the target repository container when appropriate 399 496 - dragging a repository downward never shows the indicator between target header and target worktree rows 400 - - failed repository rows either reorder correctly or are explicitly non-reorderable 497 + - failed repository rows follow the documented reorder semantics 401 498 402 499 ### Phase 5: Custom Worktree Reorder 403 500 ··· 429 526 Automated: 430 527 431 528 - `SidebarPresentationTests` 529 + - reducer tests for sidebar drag gate and notification reorder concurrency 530 + - reducer tests for expanded/collapsed state write-back and invalid collapsed ID cleanup 531 + - reducer tests for sidebar selected worktree synchronization 432 532 - existing `RepositoriesFeatureTests` ordering tests 433 533 - existing `RepositorySectionViewTests` migrated or renamed 434 534 - `make check` ··· 437 537 Manual: 438 538 439 539 1. Select a plain folder repository row. 440 - 2. Select a git repository worktree row and confirm terminal focus. 441 - 3. Cmd-click multiple worktree rows and confirm bulk archive/delete commands still target selected rows. 442 - 4. Expand/collapse one repository. 443 - 5. Bulk expand/collapse at least 10 repositories. 444 - 6. Drag repository upward and downward across expanded repositories. 445 - 7. Drag pinned worktrees within a repository. 446 - 8. Drag unpinned worktrees within a repository. 447 - 9. Trigger reveal-in-sidebar from Canvas or command. 448 - 10. Verify Canvas / Shelf / Archived rows remain selectable. 449 - 11. Verify notification/task/run-script indicators update without moving rows during a drag. 540 + 2. Click a git repository header and confirm it expands/collapses without selecting a worktree. 541 + 3. Select a git repository worktree row and confirm terminal focus. 542 + 4. Cmd-click multiple worktree rows and confirm bulk archive/delete commands still target selected rows. 543 + 5. Verify confirm/archive/delete menu commands target the same worktrees as before. 544 + 6. Verify numbered worktree hotkeys use visible sidebar rows. 545 + 7. Expand/collapse one repository. 546 + 8. Bulk expand/collapse at least 10 repositories. 547 + 9. Drag repository upward and downward across expanded repositories. 548 + 10. Drag pinned worktrees within a repository. 549 + 11. Drag unpinned worktrees within a repository. 550 + 12. Trigger reveal-in-sidebar from Canvas or command. 551 + 13. Verify Canvas / Shelf / Archived interactions remain correct. 552 + 14. Verify notification/task/run-script indicators update without moving rows during a drag. 553 + 15. Drop a repository URL onto a visible row and onto blank sidebar space. 554 + 16. Verify repository and worktree context menus. 555 + 17. Verify drag previews. 556 + 18. Verify worktree rows do not gain type-select behavior in V1. 450 557 451 558 ## Risks 452 559 ··· 477 584 Mitigation: 478 585 479 586 - stage behind a private switch until visual behavior is verified 480 - - land presentation model tests first 587 + - land M1 and presentation model tests first 481 588 - keep reducer actions and persistence shape stable 482 589 483 590 ### Performance regressions ··· 488 595 489 596 - start with `LazyVStack` 490 597 - switch only repository containers to non-lazy child stacks if expand/collapse animation needs it 491 - - measure with the existing signpost / Instruments workflow if the sidebar feels worse 598 + - decide using the Phase 0 metrics rather than visual impression alone 492 599 493 600 ## Recommendation 494 601 ··· 498 605 499 606 The safest execution path is: 500 607 501 - 1. pure presentation model and tests 502 - 2. render-only new sidebar path 503 - 3. explicit selection/reveal 504 - 4. custom repository reorder 505 - 5. custom worktree reorder 506 - 6. remove old `List` path 608 + 1. baseline metrics and manual guardrails 609 + 2. M1 old `List` drag gate and reducer concurrency tests 610 + 3. pure presentation model and tests 611 + 4. render-only new sidebar path 612 + 5. explicit selection, focus, and reveal 613 + 6. custom repository reorder 614 + 7. custom worktree reorder 615 + 8. remove old `List` path 507 616 508 617 This is larger than a tactical #222 fix, but it addresses the underlying sidebar design mismatch and gives future sidebar features a cleaner foundation.