feat(tasks): adopt Office-style /tasks list UX#1593
Merged
Conversation
📝 WalkthroughWalkthroughAdds persisted task-list sort/group preferences from backend settings through task listing, boot data, SSR, client state, list UI, and e2e coverage. Also adds ChangesTasks list sort/group preferences
Kanban header hideTitle and breakpoint
Estimated code review effort: 4 (Complex) | ~70 minutes Sequence Diagram(s)sequenceDiagram
participant TasksPage
participant TasksPageClient
participant listTasksByWorkspaceAction
participant Service
participant Repository
participant TasksListOptions
TasksPage->>TasksPageClient: initialSort and initialGroup
TasksPageClient->>TasksListOptions: parseTasksListSort / parseTasksListGroup
TasksPageClient->>listTasksByWorkspaceAction: fetch tasks with sort
listTasksByWorkspaceAction->>Service: ListTasksByWorkspace(..., sort, ...)
Service->>Repository: ListTasksByWorkspace(..., sort, ...)
Repository->>TasksListOptions: NormalizeTasksListSort
Repository-->>Service: ordered tasks
Service-->>TasksPageClient: tasks and total
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 2m 57s —— View job Code Review
Clean split — the new Issues (inline)
Nit (not inline-commentable — unchanged context line)
What looks good
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
| Filename | Overview |
|---|---|
| apps/web/app/tasks/tasks-list-view.tsx | New file — renders the list view with sections, task rows, actions, and grouping/sort controls. The buildTaskTree function's second pass over all tasks silently handles cyclic parent-child references but this intent is undocumented. |
| apps/web/app/tasks/tasks-page-client.tsx | Orchestration layer for the tasks page — manages fetch, sort/group sync with URL and user settings, workspace/workflow/repo filters. Sort is correctly forwarded to the API, resolving the previous per-page sort inconsistency. |
| apps/web/app/tasks/tasks-pagination.tsx | New pagination component with first/last/prev/next buttons, ellipsis page numbers, and rows-per-page select. Logic is correct. |
| apps/backend/internal/task/repository/sqlite/task.go | Adds server-side sort with a refactored taskScanColumns table, taskListOrderBy helper, and a derived-table wrapper for searchTasks so ORDER BY can be applied after DISTINCT. Validated sort strings are correctly used in the ORDER BY expression. |
| apps/backend/internal/user/models/tasks_list_preferences.go | New file — defines sort/group enum constants, validation helpers, and normalize functions. Clean and well-structured. |
| apps/web/components/kanban/kanban-header.tsx | Adds hideTitle prop and changes DesktopHeader search placement from actions area on non-home pages to centered on all pages when not narrow — a behavioral change to the kanban board, not just the tasks page. |
| apps/web/lib/tasks/tasks-list-options.ts | New shared module defining sort/group option constants, parse helpers, and client-side sortTasksForList used for optimistic updates before fetch resolves. |
| apps/backend/internal/backendapp/boot_state_routes.go | Boot data now resolves tasksListSort and tasksListGroup from URL params (with fallback to user settings), passes sort to tasksForWorkspace, and includes both in the route data payload. |
| apps/web/src/spa-routes.tsx | TasksDataRoute now resolves initialSort/initialGroup from URL → boot payload → user settings, and applies sortTasksForList to the SSR-provided tasks for consistent initial render. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant TasksPageClient
participant useTasksListPreferenceSync
participant BackendAPI
participant UserSettings
User->>TasksPageClient: Change sort / group
TasksPageClient->>useTasksListPreferenceSync: handleSortChange(sort)
useTasksListPreferenceSync->>TasksPageClient: setTasksListSort(sort)
useTasksListPreferenceSync->>TasksPageClient: setTasks(sortTasksForList) [optimistic]
useTasksListPreferenceSync->>TasksPageClient: "setPagination(pageIndex=0)"
useTasksListPreferenceSync->>TasksPageClient: writeUrl(sort, group)
useTasksListPreferenceSync->>UserSettings: "updateUserSettings({tasks_list_sort, tasks_list_group})"
TasksPageClient->>BackendAPI: "listTasksByWorkspace({sort, page: 1})"
BackendAPI-->>TasksPageClient: "{tasks (server-sorted), total}"
TasksPageClient->>TasksPageClient: setTasks(result.tasks) [replaces optimistic]
Note over User,UserSettings: On next page load (SSR boot)
User->>BackendAPI: "GET /tasks?sort=title_asc"
BackendAPI->>BackendAPI: tasksListSortForRoute(query, settings)
BackendAPI-->>User: "{tasks (sorted), tasksListSort, tasksListGroup}"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant TasksPageClient
participant useTasksListPreferenceSync
participant BackendAPI
participant UserSettings
User->>TasksPageClient: Change sort / group
TasksPageClient->>useTasksListPreferenceSync: handleSortChange(sort)
useTasksListPreferenceSync->>TasksPageClient: setTasksListSort(sort)
useTasksListPreferenceSync->>TasksPageClient: setTasks(sortTasksForList) [optimistic]
useTasksListPreferenceSync->>TasksPageClient: "setPagination(pageIndex=0)"
useTasksListPreferenceSync->>TasksPageClient: writeUrl(sort, group)
useTasksListPreferenceSync->>UserSettings: "updateUserSettings({tasks_list_sort, tasks_list_group})"
TasksPageClient->>BackendAPI: "listTasksByWorkspace({sort, page: 1})"
BackendAPI-->>TasksPageClient: "{tasks (server-sorted), total}"
TasksPageClient->>TasksPageClient: setTasks(result.tasks) [replaces optimistic]
Note over User,UserSettings: On next page load (SSR boot)
User->>BackendAPI: "GET /tasks?sort=title_asc"
BackendAPI->>BackendAPI: tasksListSortForRoute(query, settings)
BackendAPI-->>User: "{tasks (sorted), tasksListSort, tasksListGroup}"
Reviews (9): Last reviewed commit: "fix(tasks): address list review comments" | Re-trigger Greptile
![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 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Claude finished @carlosflorencio's task in 2m 12s —— View job Code Review
FindingsBlocker (must fix before merge)
Suggestion (recommended, doesn't block)
What the previous review flagged — both addressed ✅
What looks good
Summary
Verdict: Blocked — delete |
carlosflorencio
force-pushed
the
feature/new-list-ux-tasks-als
branch
from
22036f1 to
464e129
Compare
|
Claude finished @carlosflorencio's task in 3m 28s —— View job Code Review
FindingsBlocker (must fix before merge)
What the previous review flagged — both addressed ✅
What looks good
Summary
Verdict: Blocked — delete |
|
Claude finished @carlosflorencio's task in 1m 57s —— View job Code Review
FindingsBlocker (must fix before merge)
What looks good
Summary
Verdict: Blocked — |
|
Claude finished @carlosflorencio's task in 1s —— View job I'll analyze this and get back to you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/tasks/page.tsx (1)
55-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass the selected sort through the tasks fetch
apps/web/app/tasks/page.tsxandapps/web/app/tasks/tasks-page-client.tsxboth sort only the fetched page in memory. WithpageSize: 25, options like “Created oldest” only reorder the current slice, so pagination and the overall task order stay inconsistent. ThreadtasksListSortintolistTasksByWorkspaceAction/backend sorting instead of re-sorting the returned batch.🤖 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/web/app/tasks/page.tsx` around lines 55 - 90, The tasks list is only being sorted after fetching a single page, so pagination and “Created oldest”/similar options stay inconsistent. Update fetchWorkspaceData in tasks/page.tsx and the data flow from tasks-page-client.tsx to pass tasksListSort into listTasksByWorkspaceAction so the backend applies the sort before pagination. Keep sortTasksForList only if needed for a final stable pass, but do not rely on it to reorder the fetched batch.apps/web/app/tasks/tasks-page-client.tsx (1)
69-100: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPush task sorting into the API request
listTasksByWorkspacealready supportssort/order, but this code re-sorts only the current page after pagination and refetches on every sort change. That makes “Sort by …” and “Group by …” page-local on multi-page workspaces. Pass the selected sort through the request and drop the extra client-side resorting; grouping should be handled over the full result set if it’s meant to be global.🤖 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/web/app/tasks/tasks-page-client.tsx` around lines 69 - 100, The task list is being sorted only after fetching the current page, which makes sorting and grouping page-local instead of applying across the full workspace result set. Update the data load in tasks-page-client.tsx so listTasksByWorkspace receives the selected sort/grouping via its supported sort/order parameters, and remove the extra client-side sort call in the load flow. Keep the existing fetch logic in the same effect/useCallback area and ensure tasksListSort is translated into the API request rather than reordering result.tasks after pagination.
🧹 Nitpick comments (3)
apps/web/e2e/tests/kanban/kanban-board.spec.ts (1)
50-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "1100px" references left after breakpoint change to 800.
Lines 52 and 58-59 still describe the old
~1100pxnarrow threshold and reasoning ("now leaves the header below the 1100px narrow threshold even with no preview open"), even thoughDESKTOP_HEADER_NARROW_PXis now800(kanban-header.tsx Line 47). With the lowered threshold, a default 1280px viewport (~960px header width after the sidebar) would no longer be "below threshold," so this explanation is now inaccurate and could mislead future readers about why the test forces a wider viewport.📝 Suggested comment update
- // preview panel opens, the kanban area shrinks (`kanbanWidth = container - - // previewWidth`); the header narrows along with it. Below ~1100px there is + // preview panel opens, the kanban area shrinks (`kanbanWidth = container - + // previewWidth`); the header narrows along with it. Below ~800px there is // no longer room between the left/right action groups for the centered // search, so the header hides it (see useIsHeaderNarrow in kanban-header). // // Post-overhaul: the always-on AppSidebar (~320px expanded) permanently eats // horizontal space, so the header's own clientWidth is viewport − sidebar. - // The default Desktop Chrome 1280px viewport now leaves the header below the - // 1100px narrow threshold even with no preview open, so this test forces a + // The default Desktop Chrome 1280px viewport leaves ~960px for the header, + // which is above the 800px narrow threshold, so this test forces a // viewport wide enough that the centered search shows with the sidebar🤖 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/web/e2e/tests/kanban/kanban-board.spec.ts` around lines 50 - 62, The comment in kanban-board.spec.ts still refers to the old narrow breakpoint and should be updated to match the current 800px threshold used by useIsHeaderNarrow in kanban-header.tsx. Revise the explanatory block around the viewport setup to remove the stale “1100px” wording and adjust the reasoning so it explains that the default 1280px viewport is no longer sufficient once the AppSidebar is present, while the wider 1500px viewport keeps the header above the 800px cutoff until the preview opens. Keep the description aligned with the test’s actual viewport math and the current DESKTOP_HEADER_NARROW_PX value.apps/backend/internal/user/service/service_test.go (1)
107-141: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding a test for the empty-string default path.
applyTasksListPreferences(per the provided service.go snippet) also defaultsTasksListSort/TasksListGroupwhen the trimmed value is empty, but none of the three subtests here exercise that branch.✅ Suggested additional subtest
+ t.Run("empty string applies default", func(t *testing.T) { + settings := &models.UserSettings{} + req := &UpdateUserSettingsRequest{ + TasksListSort: ptr(" "), + TasksListGroup: ptr(""), + } + if err := applyBasicSettings(settings, req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if settings.TasksListSort != tasksListSortDefault { + t.Fatalf("TasksListSort = %q, want default", settings.TasksListSort) + } + if settings.TasksListGroup != tasksListGroupDefault { + t.Fatalf("TasksListGroup = %q, want default", settings.TasksListGroup) + } + })🤖 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/user/service/service_test.go` around lines 107 - 141, Add coverage for the empty-string default branch in applyBasicSettings/applyTasksListPreferences, since the current TestApplyBasicSettings_TasksListPreferences only checks valid and invalid explicit values. Introduce a subtest that sets TasksListSort and/or TasksListGroup to empty or whitespace-only input on UpdateUserSettingsRequest and assert the resulting models.UserSettings fields are defaulted as intended after applyBasicSettings runs.apps/web/app/tasks/tasks-list-options.ts (1)
50-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for new sort/parse utilities.
sortTasksForList,compareTasksForList,parseTasksListSort, andparseTasksListGroupcontain non-trivial branching logic (secondary tiebreakers, default fallbacks) but no accompanying*.test.tsfile is included in this cohort.As per coding guidelines,
apps/web/**/*.test.ts: "Frontend utility functions, hooks, API clients, and store slices must have tests in*.test.tsfiles when their logic changes."🤖 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/web/app/tasks/tasks-list-options.ts` around lines 50 - 75, Add frontend unit tests for the updated tasks list utilities because the branching and fallback logic in sortTasksForList, compareTasksForList, parseTasksListSort, and parseTasksListGroup is not covered. Create a *.test.ts file alongside tasks-list-options.ts and assert the key sort orders, secondary tie-breakers, and default/invalid input fallbacks for compareTasksForList and the parseTasksListSort/parseTasksListGroup helpers.Source: Coding guidelines
🤖 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/backend/internal/user/service/service.go`:
- Around line 28-29: The allowed values for tasks_list_sort and tasks_list_group
are duplicated across the defaults, the validTasksListSort/validTasksListGroup
checks, and the error strings in service.go. Consolidate each field into a
single source of truth (for example, one slice or map per field) and update the
validation logic and error construction in the same place so the default
constants, switch/validation paths, and messages all derive from that shared
list.
In `@apps/backend/internal/user/store/sqlite.go`:
- Around line 583-608: The tasks-list sort/group allowed values are duplicated
across normalizeTasksListSort, normalizeTasksListGroup, and the backend
validTasksListSort/validTasksListGroup checks, which risks the sets drifting.
Extract a single shared backend source of truth for these enums (for example,
shared constants/helpers in a models or dto package) and update sqlite.go and
boot_state_routes.go to use it instead of hardcoded string lists; then keep the
frontend list in sync with that backend definition.
In `@apps/web/lib/ssr/user-settings.ts`:
- Around line 21-45: The task sort/group parsing logic is duplicated across
parseTasksListSort and parseTasksListGroup, causing drift from the canonical
definitions. Replace the local switch-based implementations in user-settings.ts
with imports from tasks-list-options.ts, and update the other copies in
spa-routes.tsx and tasks-page-client.tsx to reuse the same parser (or the shared
option constants/defaults) instead of hardcoded literals like "updated_desc" and
"state". Use the existing DEFAULT_TASKS_LIST_SORT, DEFAULT_TASKS_LIST_GROUP,
parseTasksListSort, and parseTasksListGroup symbols as the single source of
truth.
---
Outside diff comments:
In `@apps/web/app/tasks/page.tsx`:
- Around line 55-90: The tasks list is only being sorted after fetching a single
page, so pagination and “Created oldest”/similar options stay inconsistent.
Update fetchWorkspaceData in tasks/page.tsx and the data flow from
tasks-page-client.tsx to pass tasksListSort into listTasksByWorkspaceAction so
the backend applies the sort before pagination. Keep sortTasksForList only if
needed for a final stable pass, but do not rely on it to reorder the fetched
batch.
In `@apps/web/app/tasks/tasks-page-client.tsx`:
- Around line 69-100: The task list is being sorted only after fetching the
current page, which makes sorting and grouping page-local instead of applying
across the full workspace result set. Update the data load in
tasks-page-client.tsx so listTasksByWorkspace receives the selected
sort/grouping via its supported sort/order parameters, and remove the extra
client-side sort call in the load flow. Keep the existing fetch logic in the
same effect/useCallback area and ensure tasksListSort is translated into the API
request rather than reordering result.tasks after pagination.
---
Nitpick comments:
In `@apps/backend/internal/user/service/service_test.go`:
- Around line 107-141: Add coverage for the empty-string default branch in
applyBasicSettings/applyTasksListPreferences, since the current
TestApplyBasicSettings_TasksListPreferences only checks valid and invalid
explicit values. Introduce a subtest that sets TasksListSort and/or
TasksListGroup to empty or whitespace-only input on UpdateUserSettingsRequest
and assert the resulting models.UserSettings fields are defaulted as intended
after applyBasicSettings runs.
In `@apps/web/app/tasks/tasks-list-options.ts`:
- Around line 50-75: Add frontend unit tests for the updated tasks list
utilities because the branching and fallback logic in sortTasksForList,
compareTasksForList, parseTasksListSort, and parseTasksListGroup is not covered.
Create a *.test.ts file alongside tasks-list-options.ts and assert the key sort
orders, secondary tie-breakers, and default/invalid input fallbacks for
compareTasksForList and the parseTasksListSort/parseTasksListGroup helpers.
In `@apps/web/e2e/tests/kanban/kanban-board.spec.ts`:
- Around line 50-62: The comment in kanban-board.spec.ts still refers to the old
narrow breakpoint and should be updated to match the current 800px threshold
used by useIsHeaderNarrow in kanban-header.tsx. Revise the explanatory block
around the viewport setup to remove the stale “1100px” wording and adjust the
reasoning so it explains that the default 1280px viewport is no longer
sufficient once the AppSidebar is present, while the wider 1500px viewport keeps
the header above the 800px cutoff until the preview opens. Keep the description
aligned with the test’s actual viewport math and the current
DESKTOP_HEADER_NARROW_PX value.
🪄 Autofix (Beta)
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
Run ID: 9e0badfb-b513-4092-9d4f-8e2a71a27ecc
📒 Files selected for processing (29)
apps/backend/internal/backendapp/boot_state_routes.goapps/backend/internal/user/controller/controller.goapps/backend/internal/user/dto/dto.goapps/backend/internal/user/models/models.goapps/backend/internal/user/service/service.goapps/backend/internal/user/service/service_test.goapps/backend/internal/user/store/sqlite.goapps/backend/pkg/api/v1/user.goapps/web/app/tasks/page.tsxapps/web/app/tasks/tasks-list-options.tsapps/web/app/tasks/tasks-list-view.tsxapps/web/app/tasks/tasks-page-client.tsxapps/web/components/kanban/kanban-header-mobile.tsxapps/web/components/kanban/kanban-header.tsxapps/web/components/settings/editors-settings-state.tsxapps/web/e2e/helpers/api-client.tsapps/web/e2e/tests/kanban/kanban-board.spec.tsapps/web/e2e/tests/task/mobile-task-list-search.spec.tsapps/web/e2e/tests/task/task-list-filters.spec.tsapps/web/e2e/tests/task/task-list.spec.tsapps/web/e2e/tests/task/workspace-switch-sidebar-isolation.spec.tsapps/web/hooks/use-ensure-user-settings.test.tsapps/web/hooks/use-user-display-settings.tsapps/web/lib/ssr/user-settings.tsapps/web/lib/state/slices/settings/settings-slice.tsapps/web/lib/state/slices/settings/types.tsapps/web/lib/types/http-user-settings.tsapps/web/src/boot-payload.tsapps/web/src/spa-routes.tsx
There was a problem hiding this comment.
8 issues found across 26 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Claude finished @carlosflorencio's task in 0s —— View job I'll analyze this and get back to you. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Claude finished @carlosflorencio's task in 0s —— View job I'll analyze this and get back to you. |
|
Claude finished @carlosflorencio's task in 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/internal/backendapp/boot_state_routes.go (1)
42-43: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd tests for the boot-state sort/group routing logic
CovertasksListSortForRoute,tasksListGroupForRoute, and the fallback task ordering intasksPageBootDatain a*_test.gofile alongsideapps/backend/internal/backendapp/boot_state_routes.go.🤖 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/backendapp/boot_state_routes.go` around lines 42 - 43, Add a new *_test.go alongside boot_state_routes.go to cover the routing behavior for tasksListSortForRoute and tasksListGroupForRoute, including explicit query values and empty/invalid inputs that should fall back to the settings defaults. Also add a test for tasksPageBootData that verifies the fallback task ordering is used when the route values are not provided, so the boot-state data matches the expected sort/group behavior.Source: Path instructions
🧹 Nitpick comments (2)
apps/backend/internal/backendapp/boot_state_routes.go (1)
648-660: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueParameter name shadows the package-level
queryValuefunction.
queryValueis also the name of the helper defined at Line 630 in this same file. Using it as a parameter name here works but shadows that function within this scope, which can confuse future edits (e.g., someone adding a call to the realqueryValue(req, ...)inside these functions will hit a compile error or unexpected behavior).♻️ Rename to avoid shadowing
-func tasksListSortForRoute(queryValue, settingsValue string) string { - if usermodels.IsValidTasksListSort(queryValue) { - return strings.TrimSpace(queryValue) +func tasksListSortForRoute(rawSort, settingsValue string) string { + if usermodels.IsValidTasksListSort(rawSort) { + return strings.TrimSpace(rawSort) } return usermodels.NormalizeTasksListSort(settingsValue) } -func tasksListGroupForRoute(queryValue, settingsValue string) string { - if usermodels.IsValidTasksListGroup(queryValue) { - return strings.TrimSpace(queryValue) +func tasksListGroupForRoute(rawGroup, settingsValue string) string { + if usermodels.IsValidTasksListGroup(rawGroup) { + return strings.TrimSpace(rawGroup) } return usermodels.NormalizeTasksListGroup(settingsValue) }🤖 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/backendapp/boot_state_routes.go` around lines 648 - 660, The parameters in tasksListSortForRoute and tasksListGroupForRoute shadow the existing queryValue helper in this file, which can cause confusion in future changes. Rename the parameter currently called queryValue in both functions to a distinct name, and update the internal references accordingly, keeping the logic in these route helpers unchanged.apps/web/app/tasks/tasks-list-view.tsx (1)
1-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFile bundles many components well past the 200-line guideline.
Per line-range details this file spans to ~603 lines and hosts
TasksListView,TasksListControls,ListOptionSelect,TaskRows,TaskListRow,TaskListSectionView,TaskRowActions, andTasksPaginationtogether. As per coding guidelines, "Keep components under 200 lines; prefer composition over props and extract domain components when a component grows too large." Consider splitting row/section rendering, controls, and pagination into separate files under a domain folder.🤖 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/web/app/tasks/tasks-list-view.tsx` around lines 1 - 104, The tasks list view file is too large and mixes several domain components in one module, so split it into smaller files and keep each component under the 200-line guideline. Extract `TasksListControls`, `ListOptionSelect`, `TaskRows`, `TaskListRow`, `TaskListSectionView`, `TaskRowActions`, and `TasksPagination` into separate domain components, while keeping `TasksListView` as the composition entry point that wires them together.Source: Coding guidelines
🤖 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/backend/internal/task/repository/sqlite/task.go`:
- Around line 495-513: The taskListOrderBy helper currently hardcodes COLLATE
NOCASE in the title ordering, which breaks non-SQLite dialects. Update
taskListOrderBy to use a dialect-aware case-insensitive title sort by checking
the repository’s existing SQLite/Postgres branching logic and selecting an
appropriate fallback for Postgres. Keep the existing behavior for SQLite, but
ensure every case that orders by title in taskListOrderBy remains valid across
dialects. Use the taskListOrderBy symbol and the repository’s existing
dialect-specific helpers/tests to locate and verify the change.
---
Outside diff comments:
In `@apps/backend/internal/backendapp/boot_state_routes.go`:
- Around line 42-43: Add a new *_test.go alongside boot_state_routes.go to cover
the routing behavior for tasksListSortForRoute and tasksListGroupForRoute,
including explicit query values and empty/invalid inputs that should fall back
to the settings defaults. Also add a test for tasksPageBootData that verifies
the fallback task ordering is used when the route values are not provided, so
the boot-state data matches the expected sort/group behavior.
---
Nitpick comments:
In `@apps/backend/internal/backendapp/boot_state_routes.go`:
- Around line 648-660: The parameters in tasksListSortForRoute and
tasksListGroupForRoute shadow the existing queryValue helper in this file, which
can cause confusion in future changes. Rename the parameter currently called
queryValue in both functions to a distinct name, and update the internal
references accordingly, keeping the logic in these route helpers unchanged.
In `@apps/web/app/tasks/tasks-list-view.tsx`:
- Around line 1-104: The tasks list view file is too large and mixes several
domain components in one module, so split it into smaller files and keep each
component under the 200-line guideline. Extract `TasksListControls`,
`ListOptionSelect`, `TaskRows`, `TaskListRow`, `TaskListSectionView`,
`TaskRowActions`, and `TasksPagination` into separate domain components, while
keeping `TasksListView` as the composition entry point that wires them together.
🪄 Autofix (Beta)
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
Run ID: b38f8bde-72d7-4f68-9cb6-674c627ae356
📒 Files selected for processing (32)
apps/backend/internal/backendapp/boot_state_routes.goapps/backend/internal/backendapp/e2e_reset.goapps/backend/internal/office/service/event_subscribers_fallback_test.goapps/backend/internal/office/service/service.goapps/backend/internal/office/service/workspace_deletion.goapps/backend/internal/office/service/workspace_deletion_test.goapps/backend/internal/orchestrator/executor/executor_mocks_test.goapps/backend/internal/task/handlers/process_handlers_test.goapps/backend/internal/task/handlers/task_http_handlers.goapps/backend/internal/task/repository/archive_repository_test.goapps/backend/internal/task/repository/interface.goapps/backend/internal/task/repository/sqlite/task.goapps/backend/internal/task/repository/task_repository_test.goapps/backend/internal/task/service/handoff_workspace_test.goapps/backend/internal/task/service/service_pr_search_test.goapps/backend/internal/task/service/service_resources.goapps/backend/internal/task/service/service_tasks.goapps/backend/internal/user/models/tasks_list_preferences.goapps/backend/internal/user/service/service.goapps/backend/internal/user/store/sqlite.goapps/web/app/actions/workspaces.tsapps/web/app/tasks/page.tsxapps/web/app/tasks/tasks-list-view.tsxapps/web/app/tasks/tasks-page-client.tsxapps/web/components/settings/editors-settings-state.tsxapps/web/hooks/use-user-display-settings.tsapps/web/lib/api/domains/kanban-api.tsapps/web/lib/ssr/user-settings.tsapps/web/lib/state/slices/settings/settings-slice.tsapps/web/lib/state/slices/settings/types.tsapps/web/lib/tasks/tasks-list-options.tsapps/web/src/spa-routes.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/backend/internal/task/service/handoff_workspace_test.go
- apps/backend/internal/task/repository/archive_repository_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/lib/state/slices/settings/types.ts
- apps/web/lib/state/slices/settings/settings-slice.ts
- apps/web/components/settings/editors-settings-state.tsx
- apps/web/hooks/use-user-display-settings.ts
- apps/web/src/spa-routes.tsx
- apps/backend/internal/user/store/sqlite.go
- apps/web/app/tasks/page.tsx
- apps/web/app/tasks/tasks-page-client.tsx
- apps/backend/internal/user/service/service.go
There was a problem hiding this comment.
2 issues found across 27 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/app/tasks/tasks-list-view.tsx">
<violation number="1" location="apps/web/app/tasks/tasks-list-view.tsx:328">
P2: Grouping by workflow or repository now uses ID-based section keys, which is good for disambiguation, but the fallback title when a lookup map misses an entry stays generic (`"No workflow"` / `"No repository"`). If several tasks reference distinct deleted or hidden workflow/repository IDs, the list will render multiple separate sections that all have the same fallback title, making grouping look broken.
Consider collapsing missing-map entries into a single fallback group (e.g., by using a shared `workflow:none` / `repository:none` key when the title is missing), or include the raw ID in the fallback title so sections remain distinguishable.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Claude finished @carlosflorencio's task in 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/backend/internal/task/repository/sqlite/task.go">
<violation number="1" location="apps/backend/internal/task/repository/sqlite/task.go:515">
P1: The PostgreSQL-specific `LOWER(t.title)` ordering added to `taskTitleOrder` will break `searchTasks` at runtime because `SELECT DISTINCT` in PostgreSQL requires every `ORDER BY` expression to appear in the select list. `LOWER(t.title)` is not included in `taskSelectColumns("t")`, so any non-empty task search on PostgreSQL will raise `for SELECT DISTINCT, ORDER BY expressions must appear in select list`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Claude finished @carlosflorencio's task in 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Claude finished @carlosflorencio's task in 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Member
Author
|
@codex review |
![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: 926bc12df6
ℹ️ 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".
|
Claude finished @carlosflorencio's task in 0s —— View job I'll analyze this and get back to you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- Replace the kanban-style /tasks table layout with an Office-like list page (toolbar + bordered rows + pagination).\n- Move search into the list toolbar so it is always visible/usable in mobile flows.\n- Preserve existing task behavior (create, archive, delete, navigation, pagination) while reusing current task data/actions.\n- Split page orchestration and rendering into
tasks-page-client.tsxandtasks-list-view.tsxto keep client file size in bounds.\n- Update mobile and desktop Playwright specs to target the new list-row/list container DOM model (no table semantics).\n- Cleanup mobile search state on mount/unmount to avoid stale open state after removing topbar search UI.\n\n## Validation\n- Web lint (changed files only): passed\n- New and existing Playwright specs for task list flows were updated and aligned to new interactions.