Add bounded sidebar task navigation stack - #721
Conversation
Deploying kent with
|
| Latest commit: |
ca8baeb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2807451f.kent-3kj.pages.dev |
| Branch Preview URL: | https://kent-356-sidebar-stack.kent-3kj.pages.dev |
|
Warning Review limit reached
Next review available in: 16 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (59)
📝 WalkthroughWalkthroughChangesSidebar stack and destination lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Route
participant SidebarRootOwner
participant SidebarProvider
participant SidebarDestinationView
participant TaskDetailSurface
Route->>SidebarRootOwner: create owned sidebar scope
SidebarRootOwner->>SidebarProvider: open destination
SidebarProvider->>SidebarDestinationView: render current stack page
SidebarDestinationView->>TaskDetailSurface: render Task Detail with navigator
TaskDetailSurface->>SidebarProvider: navigate back when task is missing
SidebarProvider-->>SidebarRootOwner: settle root lifecycle
sequenceDiagram
participant TaskDetailContent
participant SidebarPageNavigator
participant TaskDetailList
participant VirtualizedInfiniteList
TaskDetailContent->>SidebarPageNavigator: capture retained draft and scroll state
SidebarPageNavigator-->>TaskDetailContent: restore retained Task Detail state
TaskDetailContent->>TaskDetailList: pass restoration request
TaskDetailList->>VirtualizedInfiniteList: forward pixel offset
VirtualizedInfiniteList->>VirtualizedInfiniteList: apply offset once
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 402f04ee51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (13)
apps/desktop/src/app/sidebarProvider.test.tsx (3)
362-366: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse jest-dom matchers for attribute and disabled assertions.
@testing-library/jest-domis available in this suite.toHaveAttributeandtoBeDisabledreport the actual element state on failure, whilegetAttribute(...)/hasAttribute(...)only reportnullorfalse.♻️ Proposed change
- expect(screen.getByTestId("app-sidebar-page").getAttribute("data-direction")).toBe("push"); + expect(screen.getByTestId("app-sidebar-page")).toHaveAttribute("data-direction", "push"); fireEvent.click(screen.getByTestId("toggle-availability")); - expect(headerButtons()[0]?.hasAttribute("disabled")).toBe(true); - expect(headerButtons()[1]?.hasAttribute("disabled")).toBe(true); + expect(headerButtons()[0]).toBeDisabled(); + expect(headerButtons()[1]).toBeDisabled();🤖 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/desktop/src/app/sidebarProvider.test.tsx` around lines 362 - 366, Update the assertions in the sidebar test around the direction and availability checks to use jest-dom matchers: assert the page element with toHaveAttribute and assert each header button with toBeDisabled, preserving the existing expected values.
193-220: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueBound the Back traversal loop.
The
while (result.current.shell.canGoBack)loop depends onback()reducing the stack on every iteration. If a regression makesback()return"accepted"without changing the entries, this test hangs until the Vitest timeout rather than failing with a clear message. Add an explicit iteration guard.♻️ Proposed guard
const visited: string[] = []; - while (result.current.shell.canGoBack) { + for (let step = 0; result.current.shell.canGoBack; step += 1) { + if (step > 50) { + throw new Error("Sidebar Back did not reduce the stack."); + } const current = result.current.shell.activeDestination;🤖 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/desktop/src/app/sidebarProvider.test.tsx` around lines 193 - 220, Add an explicit maximum-iteration guard to the Back traversal loop in the test, limiting iterations to the expected bounded history size while retaining the canGoBack condition. Ensure the test fails clearly if back() repeatedly returns "accepted" without reducing the stack, and preserve the existing visited-count and destination assertions.
310-345: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore real timers even when an assertion fails.
Line 311 enables fake timers and Line 344 restores them. If any assertion between those lines fails,
vi.useRealTimers()never runs. Every later test in this file then runs with fake timers, which produces misleading cascading failures. Move the restoration into anafterEachhook.♻️ Proposed change
describe("SidebarProvider stack", () => { + afterEach(() => { + vi.useRealTimers(); + }); +expect(result.current.shell.activeDestination).toBeNull(); - vi.useRealTimers(); });🤖 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/desktop/src/app/sidebarProvider.test.tsx` around lines 310 - 345, Move fake-timer cleanup for the test containing “retains widths per profile and keeps the closing page rendered through the exit phase” into an afterEach hook, ensuring vi.useRealTimers() runs even when assertions fail. Remove the inline restoration and keep the test’s timer setup behavior unchanged.apps/desktop/src/features/task-detail/TaskDetailRetainedState.test.tsx (1)
85-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the focus behavior that the test name claims.
The test name states that first-open focus is preserved. The body passes
initialFocus: { kind: "dependencies" }but never asserts that the Dependencies area receives focus. The current assertions only verify the decoded fallback draft and the selected tab. Add an assertion for the focused dependencies element, or rename the test to describe only the retained-state fallback.🤖 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/desktop/src/features/task-detail/TaskDetailRetainedState.test.tsx` around lines 85 - 95, Update the test case “ignores malformed retained state and preserves first-open focus” to assert that the Dependencies area receives focus after mounting with initialFocus kind “dependencies”; retain the existing fallback draft and selected-tab assertions.apps/desktop/src/features/home/useHomeData.test.tsx (1)
322-322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReset the shared navigator between tests.
sidebarNavigatoris created once at module scope and is reused by every test in this file.createTestSidebarNavigatorreturns mock functions, so call history accumulates across tests. A future assertion onsidebarNavigator.replaceorsidebarNavigator.pushwould then observe calls from an earlier test. Create the navigator inside each test, or clear it in abeforeEachhook.♻️ Proposed reset
+beforeEach(() => { + vi.clearAllMocks(); +});🤖 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/desktop/src/features/home/useHomeData.test.tsx` at line 322, Reset the shared sidebarNavigator mock state between tests by moving its creation into each test or clearing its mock functions in a beforeEach hook. Update the existing sidebarNavigator setup while preserving the current test behavior and assertions.apps/desktop/src/test-support/task-detail/index.ts (1)
423-424: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLet the caller supply the root controller.
mountTaskDetailSurfacealways installs a throwawaycreateTestSidebarController()as theSidebarRootContextvalue. Callers can only observe root opens through the separateopenSidebaroption. A test that relies on the context instead of the prop silently observes nothing. Add an option for the controller so the fixture has one place that defines root-open observation.♻️ Proposed change
retainedState?: unknown; + rootController?: SidebarRootController | undefined; sidebarMode?: SidebarMode | undefined;children: createElement(SidebarRootContext.Provider, { - value: createTestSidebarController(), + value: options.rootController ?? createTestSidebarController(),🤖 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/desktop/src/test-support/task-detail/index.ts` around lines 423 - 424, Update mountTaskDetailSurface to accept an optional root sidebar controller in its options, and pass that controller as the SidebarRootContext.Provider value instead of always creating one with createTestSidebarController(). Preserve the existing default for callers that do not provide the option, and use the supplied controller so context-based root-open observations share the fixture’s configured controller.Source: Coding guidelines
apps/desktop/src/ui/VirtualizedInfiniteList.test.tsx (1)
11-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReset every virtualizer mock between tests.
beforeEachonly clearsscrollToOffsetand resetsgetVirtualItems.getOffsetForIndex,scrollToIndex, andmeasureElementkeep call history across tests. That state can leak if a later test asserts on them.♻️ Proposed reset
beforeEach(() => { + vi.clearAllMocks(); + virtualizer.getTotalSize.mockReturnValue(120); virtualizer.getVirtualItems.mockReturnValue([]); - virtualizer.scrollToOffset.mockClear(); });Also applies to: 60-80, 89-117
🤖 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/desktop/src/ui/VirtualizedInfiniteList.test.tsx` around lines 11 - 34, Reset all virtualizer mock call histories in the test setup before each test, including getOffsetForIndex, scrollToIndex, and measureElement alongside the existing resets for scrollToOffset and getVirtualItems. Update the beforeEach associated with the virtualizer mock so later tests cannot observe calls from earlier tests.docs/dev/specs/desktop-gui.md (1)
274-303: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the sidebar-stack rules into their own section.
These 30 bullets describe the sidebar navigation stack, lifecycle, ownership, and restoration. They are inserted into the middle of the
Task Dependenciesbullet list. The list resumes dependency-row rules at Line 304 ("Each relationship row has an accessible trailing Remove action"), so the dependency rules are now split across two blocks.Add a dedicated section, for example
## Sidebar Navigation, and keep only the dependency-specific navigation bullets (Lines 276-279, 300-301) inTask Dependencies. This keeps one primary responsibility per section and keeps the dependency rules contiguous.🤖 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 `@docs/dev/specs/desktop-gui.md` around lines 274 - 303, Create a dedicated “Sidebar Navigation” section for the sidebar stack, lifecycle, ownership, and restoration bullets currently mixed into “Task Dependencies.” Keep only the dependency-specific navigation rules—related Task selection, Dependency Add, related creation behavior, and navigation availability—under “Task Dependencies,” ensuring the remaining dependency-row rules stay contiguous.Source: Coding guidelines
apps/desktop/src/app-facade/projectDeletionEvents.test.ts (2)
55-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the teardown into
afterEach.Lines 57 to 59 unmount the hook, unsubscribe the observer, and clear the query client only when every assertion passes. A failed assertion leaves the observer subscribed and the query client populated, which can affect the following table case. Register the teardown in
afterEachinstead.🤖 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/desktop/src/app-facade/projectDeletionEvents.test.ts` around lines 55 - 59, Move the cleanup calls from the test body into an afterEach teardown for the relevant test suite, ensuring view.unmount(), unsubscribe(), and queryClient.clear() run even when assertions fail. Keep the existing setup and assertions unchanged.
15-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo table rows exercise the same query key.
"Link Workflow"and"Project Workflow Editor"both passqueryKeys.projectWorkflowLinks("project-1"). The second row runs the identical assertions against the identical key, so it adds no coverage and only doubles the run time. Either remove one row, or point the Project Workflow Editor row at the query key that owner actually reads.🤖 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/desktop/src/app-facade/projectDeletionEvents.test.ts` around lines 15 - 20, Remove the duplicate table row in the parameterized test, or update the “Project Workflow Editor” case to use the distinct query key that its owner reads instead of queryKeys.projectWorkflowLinks("project-1"). Keep each scenario mapped to a unique query key.apps/desktop/src/features/project-edit/ProjectDeleteButton.tsx (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
navigatorshadows the globalnavigatorobject.The prop name
navigatorhideswindow.navigatorin every scope inside this component. The code here does not use the DOM object, so behavior is correct today. A later addition such asnavigator.clipboardwould resolve to the sidebar navigator and fail. The same shadowing now exists insidebarDestinations.tsx,ProjectEditRoute.tsx, andWorkflowEditorRoute.tsx. ConsidersidebarNavigatororpageNavigatorfor the public prop.🤖 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/desktop/src/features/project-edit/ProjectDeleteButton.tsx` at line 25, Rename the navigator prop and its usages in ProjectDeleteButton to sidebarNavigator or pageNavigator, and apply the same public-parameter rename to the corresponding symbols in sidebarDestinations.tsx, ProjectEditRoute.tsx, and WorkflowEditorRoute.tsx so the global navigator remains accessible.apps/desktop/src/app/sidebar.tsx (1)
292-306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why
PageBoundarymust stay a per-entry component identity.
BoundaryinsidebarStack.createEntryis a new function component for every stack entry, and it renders only aFragment. The remount of the destination subtree depends entirely on that changing component identity. A later change that hoists or memoizesBoundarywould silently keep inactive destinations mounted and break retained-state isolation. The name also suggests error isolation, which this component does not provide.Add a short comment here, or rename the field to state the lifecycle intent.
🤖 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/desktop/src/app/sidebar.tsx` around lines 292 - 306, Document the lifecycle purpose of the PageBoundary component around SidebarDestinationView: its per-entry identity must remain distinct so destination subtrees remount and retained state stays isolated. Add a brief comment or rename the related boundary field to avoid implying error isolation, without changing the existing behavior.apps/desktop/src/api/errors.test.ts (1)
16-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName the generic RPC code and add a negative Task case.
Line 19 uses the literal
-32000while the neighboring assertions userpcErrorCodes. The literal exercises the structured-data path, so give it a name that states that intent.isTaskMissingErroralso has no negative assertion, so a regression to message-based detection would stay undetected on the Task side.♻️ Proposed change
+const genericRpcErrorCode = -32000; + describe("sidebar missing-entity errors", () => { it("recognizes typed Task and Project missing errors without parsing messages", () => { const error = (code: number, data?: Readonly<Record<string, string>>) => new RpcError({ code, data, message: "changed", method: "owner.operation" }); expect(isTaskMissingError(error(rpcErrorCodes.workflowTaskNotFound))).toBe(true); - expect(isProjectMissingError(error(-32000, { reason: "project_not_found" }))).toBe(true); + expect(isTaskMissingError(new Error("workflow_task_not_found"))).toBe(false); + expect(isProjectMissingError(error(genericRpcErrorCode, { reason: "project_not_found" }))).toBe(true); expect(isProjectMissingError(error(rpcErrorCodes.projectNotFound))).toBe(true); expect(isProjectMissingError(new Error("project_not_found"))).toBe(false); }); });🤖 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/desktop/src/api/errors.test.ts` around lines 16 - 21, Update the test helper or constants in the error test to name the generic RPC code used for the structured-data project case instead of the raw -32000 literal, and add a negative isTaskMissingError assertion using an Error message that should not be recognized as a missing task. Keep the existing positive task and project assertions unchanged.
🤖 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.
Inline comments:
In `@apps/desktop/src/app-facade/sidebarContext.ts`:
- Around line 142-165: Update SidebarRootOwner to track whether the owner is
still active, and mark it inactive during the cleanup effect before releasing
and clearing handles. In the open callback, reject calls made after cleanup by
returning a handle whose lifecycle is already resolved with "released", without
invoking roots.open; preserve the existing handle tracking and release behavior
while active.
In `@apps/desktop/src/app/sidebarDestinations.test.tsx`:
- Around line 78-83: Extend the “deduplicates only Task Detail destinations”
test to assert that two taskDetail destinations with different taskID values are
not equal, while preserving the existing equal-ID and custom-destination
assertions in sidebarDestinationPolicy.equals.
In `@apps/desktop/src/app/sidebarDestinations.tsx`:
- Around line 127-129: Handle rejected post-close navigation promises through
the shared follow helper in apps/desktop/src/app/sidebarDestinations.tsx:127-129
by attaching a catch that reports a danger status via useStatusController,
covering onCreated and onLinked. At
apps/desktop/src/app/sidebarDestinations.tsx:155-157, route
navigation.openWorkflowEditor through follow instead of using bare void; no
separate error handling is needed there.
In `@apps/desktop/src/app/sidebarStack.ts`:
- Around line 42-46: Update notify to publish a fresh view object rather than
the existing view reference, ensuring availability mutations trigger React state
updates. Preserve the current capability check and use a shallow copy when
calling publish.
In `@apps/desktop/src/features/project-edit/ProjectEditRoute.tsx`:
- Around line 77-82: Update MissingProjectDismissal to capture the result of
navigator.back() and render the existing ErrorState whenever the outcome is not
"accepted"; retain the current null rendering after a successful dismissal.
In `@apps/desktop/src/ui/VirtualizedInfiniteList.tsx`:
- Around line 257-268: The useEffect applying validatedPixelOffsetRequest must
not consume the request until offsetPx is reachable in the virtualizer’s current
scroll range. Update the guard or readiness condition around
virtualizer.scrollToOffset so early fixed-row renders leave
lastPixelOffsetKeyRef pending, allowing later page loads to retry; alternatively
require an explicit data-ready signal before marking the request key applied.
---
Nitpick comments:
In `@apps/desktop/src/api/errors.test.ts`:
- Around line 16-21: Update the test helper or constants in the error test to
name the generic RPC code used for the structured-data project case instead of
the raw -32000 literal, and add a negative isTaskMissingError assertion using an
Error message that should not be recognized as a missing task. Keep the existing
positive task and project assertions unchanged.
In `@apps/desktop/src/app-facade/projectDeletionEvents.test.ts`:
- Around line 55-59: Move the cleanup calls from the test body into an afterEach
teardown for the relevant test suite, ensuring view.unmount(), unsubscribe(),
and queryClient.clear() run even when assertions fail. Keep the existing setup
and assertions unchanged.
- Around line 15-20: Remove the duplicate table row in the parameterized test,
or update the “Project Workflow Editor” case to use the distinct query key that
its owner reads instead of queryKeys.projectWorkflowLinks("project-1"). Keep
each scenario mapped to a unique query key.
In `@apps/desktop/src/app/sidebar.tsx`:
- Around line 292-306: Document the lifecycle purpose of the PageBoundary
component around SidebarDestinationView: its per-entry identity must remain
distinct so destination subtrees remount and retained state stays isolated. Add
a brief comment or rename the related boundary field to avoid implying error
isolation, without changing the existing behavior.
In `@apps/desktop/src/app/sidebarProvider.test.tsx`:
- Around line 362-366: Update the assertions in the sidebar test around the
direction and availability checks to use jest-dom matchers: assert the page
element with toHaveAttribute and assert each header button with toBeDisabled,
preserving the existing expected values.
- Around line 193-220: Add an explicit maximum-iteration guard to the Back
traversal loop in the test, limiting iterations to the expected bounded history
size while retaining the canGoBack condition. Ensure the test fails clearly if
back() repeatedly returns "accepted" without reducing the stack, and preserve
the existing visited-count and destination assertions.
- Around line 310-345: Move fake-timer cleanup for the test containing “retains
widths per profile and keeps the closing page rendered through the exit phase”
into an afterEach hook, ensuring vi.useRealTimers() runs even when assertions
fail. Remove the inline restoration and keep the test’s timer setup behavior
unchanged.
In `@apps/desktop/src/features/home/useHomeData.test.tsx`:
- Line 322: Reset the shared sidebarNavigator mock state between tests by moving
its creation into each test or clearing its mock functions in a beforeEach hook.
Update the existing sidebarNavigator setup while preserving the current test
behavior and assertions.
In `@apps/desktop/src/features/project-edit/ProjectDeleteButton.tsx`:
- Line 25: Rename the navigator prop and its usages in ProjectDeleteButton to
sidebarNavigator or pageNavigator, and apply the same public-parameter rename to
the corresponding symbols in sidebarDestinations.tsx, ProjectEditRoute.tsx, and
WorkflowEditorRoute.tsx so the global navigator remains accessible.
In `@apps/desktop/src/features/task-detail/TaskDetailRetainedState.test.tsx`:
- Around line 85-95: Update the test case “ignores malformed retained state and
preserves first-open focus” to assert that the Dependencies area receives focus
after mounting with initialFocus kind “dependencies”; retain the existing
fallback draft and selected-tab assertions.
In `@apps/desktop/src/test-support/task-detail/index.ts`:
- Around line 423-424: Update mountTaskDetailSurface to accept an optional root
sidebar controller in its options, and pass that controller as the
SidebarRootContext.Provider value instead of always creating one with
createTestSidebarController(). Preserve the existing default for callers that do
not provide the option, and use the supplied controller so context-based
root-open observations share the fixture’s configured controller.
In `@apps/desktop/src/ui/VirtualizedInfiniteList.test.tsx`:
- Around line 11-34: Reset all virtualizer mock call histories in the test setup
before each test, including getOffsetForIndex, scrollToIndex, and measureElement
alongside the existing resets for scrollToOffset and getVirtualItems. Update the
beforeEach associated with the virtualizer mock so later tests cannot observe
calls from earlier tests.
In `@docs/dev/specs/desktop-gui.md`:
- Around line 274-303: Create a dedicated “Sidebar Navigation” section for the
sidebar stack, lifecycle, ownership, and restoration bullets currently mixed
into “Task Dependencies.” Keep only the dependency-specific navigation
rules—related Task selection, Dependency Add, related creation behavior, and
navigation availability—under “Task Dependencies,” ensuring the remaining
dependency-row rules stay contiguous.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2062b621-6600-45ca-95bb-e2e0d76d67bd
📒 Files selected for processing (53)
.kent/plans/KENT-356.mdapps/desktop/src/api/errors.test.tsapps/desktop/src/api/errors.tsapps/desktop/src/api/index.tsapps/desktop/src/api/rpcErrorCodes.tsapps/desktop/src/app-facade/projectDeletionEvents.test.tsapps/desktop/src/app-facade/projectDeletionEvents.tsapps/desktop/src/app-facade/sidebarContext.tsapps/desktop/src/app/AppChrome.tsxapps/desktop/src/app/AttentionNotificationController.tsxapps/desktop/src/app/ProjectMissingMutationSeams.test.tsxapps/desktop/src/app/sidebar.tsxapps/desktop/src/app/sidebarDestinationPolicy.tsapps/desktop/src/app/sidebarDestinations.test.tsxapps/desktop/src/app/sidebarDestinations.tsxapps/desktop/src/app/sidebarPageContext.tsapps/desktop/src/app/sidebarProvider.test.tsxapps/desktop/src/app/sidebarProvider.tsxapps/desktop/src/app/sidebarStack.tsapps/desktop/src/app/startup/styles.cssapps/desktop/src/features/board/BoardColumns.tsxapps/desktop/src/features/board/BoardNoWorkflowState.tsxapps/desktop/src/features/board/BoardRoute.tsxapps/desktop/src/features/board/taskDetailRouteLifecycle.test.tsapps/desktop/src/features/board/taskDetailRouteLifecycle.tsapps/desktop/src/features/home/HomeRoute.tsxapps/desktop/src/features/home/ProjectRow.tsxapps/desktop/src/features/home/SidebarInboxNav.tsxapps/desktop/src/features/home/useHomeData.test.tsxapps/desktop/src/features/project-edit/ProjectDeleteButton.test.tsxapps/desktop/src/features/project-edit/ProjectDeleteButton.tsxapps/desktop/src/features/project-edit/ProjectEditRoute.tsxapps/desktop/src/features/task-detail/StandaloneTaskRoute.tsxapps/desktop/src/features/task-detail/TaskDependenciesArea.test.tsxapps/desktop/src/features/task-detail/TaskDependenciesArea.tsxapps/desktop/src/features/task-detail/TaskDetailContent.tsxapps/desktop/src/features/task-detail/TaskDetailList.tsxapps/desktop/src/features/task-detail/TaskDetailRetainedState.test.tsxapps/desktop/src/features/task-detail/TaskDetailSurface.sidebar.test.tsxapps/desktop/src/features/task-detail/TaskDetailSurface.tsxapps/desktop/src/features/tasks/NewTaskDialog.tsxapps/desktop/src/features/workflow-editor/WorkflowEditorRoute.tsxapps/desktop/src/features/workflows/LinkWorkflowSidebar.tsxapps/desktop/src/features/workflows/WorkflowCreateForm.tsxapps/desktop/src/features/workflows/WorkflowLibraryRoute.tsxapps/desktop/src/shared/workflow-deletion/useWorkflowDeleteLauncher.tsxapps/desktop/src/test-support/sidebar/index.tsapps/desktop/src/test-support/task-detail/index.tsapps/desktop/src/ui/VirtualizedInfiniteList.test.tsxapps/desktop/src/ui/VirtualizedInfiniteList.tsxapps/desktop/src/ui/index.tsapps/desktop/src/ui/virtualizedPixelOffsetRequest.tsdocs/dev/specs/desktop-gui.md
💤 Files with no reviewable changes (2)
- apps/desktop/src/features/board/taskDetailRouteLifecycle.test.ts
- apps/desktop/src/features/board/taskDetailRouteLifecycle.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51d3393dff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41537eb5ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca8baebd90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Product decisions
Verification
ReorderableList.tsxFast Refresh warning)git diff --checkFinal classifier from merge-base
c945238c9: production 37 files / 1,774 changed LoC; tests 14 files / 1,270 changed LoC; docs 1 file / 35 changed LoC; generated 0.Summary by CodeRabbit