fix(web): render multi-level subtasks in sidebar task tree#1204
Conversation
The sidebar renderer was hardcoded to two levels, silently dropping depth-2+ tasks. Replace with recursive tree rendering, depth-aware indent, mobile parentTaskId mapping, and e2e test seeding for deep trees. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds backend test-harness task seeding, depth/indent infrastructure, level-scoped sortable DnD plus recursive TaskSwitcher rendering, mobile integration for parent linkage, E2E and unit tests, and depth-aware TaskItem indentation to support arbitrary-depth task hierarchies. ChangesMulti-level subtask hierarchy support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Claude finished @carlosflorencio's task in 4m 28s —— View job Code Review
Review complete. Verdict: Ready to merge with suggestions — no blockers found. 2 suggestions (non-blocking):
The recursive |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
FindingsSuggestion (recommended, doesn't block)
Summary
Verdict: Ready to merge with suggestions The recursive |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 15 files
You’re at about 93% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
| Filename | Overview |
|---|---|
| apps/web/components/task/task-switcher.tsx | Core recursive renderer. Replaces flat GroupTaskList with TaskTreeNode/TaskTreeLevel mutual recursion; introduces TaskTreeContext. countGroupTasks now used for both group header and toggle badge — previous asymmetry resolved. |
| apps/web/components/task/task-switcher-subtask-dnd.tsx | Refactored to generic SortableTaskNode/SortableTaskLevel. isDraggable=false path renders without grab cursor; grab-cursor concern from previous review is resolved. |
| apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts | parentTaskId mapped in both mapSnapshotToKanban/toSheetItem and buildKanbanTaskUpsert, fixing the prior P1 where WS-upserted subtasks reset to root level in the mobile sidebar. |
| apps/web/lib/sidebar/apply-view.ts | Adds countGroupTasks — iterative DFS with visited-set cycle guard. applyView/separateSubtasks unchanged; the existing map already contained all ancestor-child pairs. |
| apps/web/lib/sidebar/row-indent.ts | New utility computing pixel indentation capped at MAX_VISUAL_DEPTH=6. Depth-0 connectorLeftPx is -6px but never applied (guarded by depth>0 check in task-item.tsx). |
| apps/backend/internal/office/testharness/routes.go | Adds POST /api/v1/_test/tasks harness route bypassing depth guard intentionally; gated behind KANDEV_E2E_MOCK consistent with existing harness routes. |
Reviews (4): Last reviewed commit: "docs(web): move SortableTaskNode JSDoc t..." | Re-trigger Greptile
Map parentTaskId in buildKanbanTaskUpsert so in-session subtask creation nests immediately on mobile. Add visited-set guard to countGroupTasks. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Claude finished @carlosflorencio's task in 3m 33s —— View job Code Review
FindingsSuggestion (recommended, doesn't block)
Previous-round fixes verified ✓
Summary
Verdict: Ready to merge with suggestions The recursive |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/backend/internal/office/testharness/routes.go (1)
37-48: 💤 Low valueOptional: Consider consistent error response pattern across handlers.
The new
errJSONhelper and constantserrInvalidJSONPrefix/respKeyErrorare only used by the newseedTaskHandler, while existing handlers in this file (e.g., lines 188, 192, 262) inlinegin.H{"error": ...}. This creates a minor inconsistency within the file.For a uniform codebase, either adopt
errJSONacross all handlers or stick with the inline pattern. Since this is test-harness code, the current mix is low-risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/internal/office/testharness/routes.go` around lines 37 - 48, The file mixes a new errJSON helper and constants (errInvalidJSONPrefix, respKeyError) with older handlers that inline gin.H{"error": ...}; pick one consistent approach and apply it across handlers: either replace inline responses in existing handlers (those that currently call c.JSON(..., gin.H{"error": ...})) to use errJSON(c, code, msg) and use errInvalidJSONPrefix where malformed-JSON messages are constructed, or remove the unused errJSON/consts and keep the inline pattern; update all occurrences of error responses and tests accordingly so seedTaskHandler and the other handlers all use the same respKeyError/errJSON API or the inline gin.H literal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/backend/internal/office/testharness/routes.go`:
- Around line 37-48: The file mixes a new errJSON helper and constants
(errInvalidJSONPrefix, respKeyError) with older handlers that inline
gin.H{"error": ...}; pick one consistent approach and apply it across handlers:
either replace inline responses in existing handlers (those that currently call
c.JSON(..., gin.H{"error": ...})) to use errJSON(c, code, msg) and use
errInvalidJSONPrefix where malformed-JSON messages are constructed, or remove
the unused errJSON/consts and keep the inline pattern; update all occurrences of
error responses and tests accordingly so seedTaskHandler and the other handlers
all use the same respKeyError/errJSON API or the inline gin.H literal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90f3b7c8-8b16-48f6-9436-761754bac150
📒 Files selected for processing (15)
apps/backend/internal/office/testharness/routes.goapps/backend/internal/office/testharness/routes_test.goapps/web/components/task/mobile/session-mobile-top-bar.tsxapps/web/components/task/mobile/session-task-switcher-sheet-hooks.tsapps/web/components/task/task-item.tsxapps/web/components/task/task-switcher-subtask-dnd.tsxapps/web/components/task/task-switcher.test.tsxapps/web/components/task/task-switcher.tsxapps/web/e2e/helpers/api-client.tsapps/web/e2e/tests/task/mobile-sidebar-subtasks.spec.tsapps/web/e2e/tests/task/sidebar-multilevel-subtasks.spec.tsapps/web/lib/sidebar/apply-view-count.test.tsapps/web/lib/sidebar/apply-view.tsapps/web/lib/sidebar/row-indent.test.tsapps/web/lib/sidebar/row-indent.ts
Skip sortable wiring on non-draggable tree levels so nested rows no longer show a grab cursor when reorder is disabled. Subtask collapse badges now count all descendants to match what collapse hides. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Claude finished @carlosflorencio's task in 2m 45s —— View job Code Review
FindingsPrevious-round fixes verified ✓
Nitpick (no-block)
Summary
Verdict: Ready to merge All four suggestions from the two prior review rounds are correctly addressed and backed by new unit tests. The |
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Claude finished @carlosflorencio's task in 1m 50s —— View job Code Review
All previous-round findings resolved ✓
SummaryVerdict: Ready to merge No open findings. The recursive |
1 participant
Grandchild tasks existed in the store but never appeared in the sidebar because the renderer only walked two levels. This replaces the flat root/subtask split with a recursive tree so any depth is visible, with collapse and sibling-scoped drag-and-drop at every level.
Important Changes
TaskTreeNode/TaskTreeLevelrenderer replaces the hardcoded 2-level sidebar walkrow-indent.ts) capped at visual depth 6 to protect narrow sidebarsparentTaskIdso subtasks nest on mobile for the first timePOST /api/v1/_test/tasksharness route seeds depth ≥ 2 chains for e2e (bypasses service depth guard)Validation
make -C apps/backend fmt test lintpnpm --filter @kandev/web typecheck lint test runsidebar-multilevel-subtasks.spec.ts(depth-3 desktop + mid-level collapse),mobile-sidebar-subtasks.spec.ts(depth-2 mobile), existing depth-1 sidebar specPossible Improvements
Low risk — production CreateTask still enforces depth ≤ 1; this only affects display of existing deep trees and e2e-seeded data.
Checklist
apps/web/), I have added or updated Playwright e2e tests inapps/web/e2e/and verified them withmake test-e2e.Made with Cursor