Conversation
WalkthroughAdds a PlanView Kanban feature with new UI components and modals, integrates worktrees with tasks via a new WorktreeContext and IPC flows, extends task statuses ("backlog", "completed"), makes avatar URLs nullable with initials fallback, and applies broad formatting/style normalizations and several new scripts/docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as NewLayoutMain
participant Plan as PlanView
participant IPC as MainProcess
Note over UI,Plan: Mode switch / plan flow
User->>UI: toggle mode -> "plan"
UI->>Plan: render PlanView (tasks grouped by status)
Plan->>Plan: open TaskDetail or TaskPage on click
Note over Plan,IPC: Create/select worktree flow
User->>Plan: Create or Select Task
Plan->>IPC: request create/open worktree for branch
IPC-->>Plan: returns worktree
Plan->>UI: refresh worktrees & select worktree (switch to edit mode)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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/desktop/src/renderer/screens/main/components/NewLayout/TaskListItem.tsx (1)
12-12: Update Task interface to align with Avatar's nullable imageUrl.The
assigneeAvatarUrlis typed asstring, but the Avatar component now acceptsstring | null. This creates a type inconsistency.Apply this diff to align the types:
interface Task { id: string; slug: string; name: string; status: TaskStatus; assignee: string; - assigneeAvatarUrl: string; + assigneeAvatarUrl: string | null; lastUpdated: string; }apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (1)
33-149: Show the correct status label for every statusBecause the fallback chain only names three states, everything else currently renders as “Ready to Merge”, so backlog/completed tasks look wrong as soon as we hit the hover card. Let’s centralize the mapping so every status shares the same label we expose in the modal/status indicator.
-import type { Worktree } from "shared/types"; -import { StatusIndicator, type TaskStatus } from "./StatusIndicator"; +import type { Worktree } from "shared/types"; +import { StatusIndicator, type TaskStatus } from "./StatusIndicator"; import { TaskAssignee } from "./TaskAssignee"; // Extended Worktree type with optional task metadata export interface WorktreeWithTask extends Worktree { @@ }; } + +const STATUS_LABELS: Record<TaskStatus, string> = { + backlog: "Backlog", + todo: "Todo", + planning: "Planning", + working: "Working", + "needs-feedback": "Needs Feedback", + "ready-to-merge": "Ready to Merge", + completed: "Completed", + canceled: "Canceled", +}; @@ {worktrees.map((worktree) => { const hasTask = !!worktree.task; const task = worktree.task; const displayTitle = hasTask && task ? task.slug : worktree.description || worktree.branch; - const statusLabel = task - ? task.status === "planning" - ? "Planning" - : task.status === "working" - ? "Working" - : task.status === "needs-feedback" - ? "Needs Feedback" - : "Ready to Merge" - : ""; + const statusLabel = task ? STATUS_LABELS[task.status] : "";
🧹 Nitpick comments (2)
apps/website/src/trpc/react.tsx (1)
1-65: LGTM! Code style standardization applied consistently.The changes are purely stylistic—string literals converted to double quotes, indentation normalized, and spacing adjusted. No functional, behavioral, or API changes detected. Logic and TypeScript types remain identical.
Optional suggestion: Consider using an automated formatter like Prettier with a shared configuration to enforce consistent code style across the entire codebase automatically. This can prevent style inconsistencies and reduce manual formatting effort in future PRs.
apps/desktop/src/main/lib/terminal.ts (1)
121-125: Formatting improvement with opportunity to reduce duplication.The multiline formatting improves readability. However, this exact condition also appears at lines 90-94. Consider extracting it into a private helper method to follow DRY principles.
Example refactor:
private canSendToRenderer(): boolean { return ( this.mainWindow !== null && !this.mainWindow.isDestroyed() && !this.mainWindow.webContents.isDestroyed() ); }Then use
if (this.canSendToRenderer()) { ... }in both locations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
apps/desktop/package.json(1 hunks)apps/desktop/src/lib/mock-data.ts(1 hunks)apps/desktop/src/main/lib/terminal-ipcs.ts(1 hunks)apps/desktop/src/main/lib/terminal.ts(2 hunks)apps/desktop/src/main/lib/tmux-manager.ts(7 hunks)apps/desktop/src/renderer/contexts/WorktreeContext.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/DiffView/DiffContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/MainContent/Terminal.tsx(7 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/AddTaskModal.tsx(9 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/Avatar.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx(13 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskAssignee.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskListItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskPreview.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/CreateTaskModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/EditTaskModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskDetailModal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/PlanView/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/TopBar.tsx(2 hunks)apps/website/src/app/api/trpc/[trpc]/route.ts(1 hunks)apps/website/src/app/layout.tsx(1 hunks)apps/website/src/trpc/query-client.ts(1 hunks)apps/website/src/trpc/react.tsx(1 hunks)apps/website/src/trpc/server.tsx(1 hunks)packages/api/package.json(1 hunks)packages/api/src/env.ts(1 hunks)packages/api/src/index.ts(1 hunks)packages/api/src/root.ts(1 hunks)packages/api/src/router/organization.ts(1 hunks)packages/api/src/router/repository.ts(1 hunks)packages/api/src/router/task.ts(1 hunks)packages/api/src/router/user.ts(1 hunks)packages/api/src/trpc.ts(1 hunks)packages/api/tsconfig.json(1 hunks)packages/db/drizzle.config.ts(1 hunks)packages/db/drizzle/meta/0000_snapshot.json(1 hunks)packages/db/drizzle/meta/_journal.json(1 hunks)packages/db/src/client.ts(1 hunks)packages/db/src/env.ts(1 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/schema/enums.ts(1 hunks)packages/db/src/schema/index.ts(1 hunks)packages/db/src/schema/relations.ts(1 hunks)packages/db/src/schema/schema.ts(1 hunks)packages/ui/src/components/dialog.tsx(1 hunks)packages/ui/src/components/input.tsx(1 hunks)packages/ui/src/components/label.tsx(1 hunks)packages/ui/src/components/select.tsx(1 hunks)packages/ui/src/components/textarea.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (29)
apps/website/src/app/layout.tsx (1)
apps/website/src/trpc/react.tsx (1)
TRPCReactProvider(34-65)
apps/website/src/trpc/server.tsx (4)
apps/website/src/trpc/react.tsx (1)
createTRPCContext(26-26)packages/api/src/trpc.ts (1)
createTRPCContext(7-21)apps/website/src/trpc/query-client.ts (1)
createQueryClient(7-24)packages/api/src/root.ts (2)
AppRouter(14-14)appRouter(7-12)
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskListItem.tsx (1)
apps/desktop/src/renderer/screens/main/components/NewLayout/Avatar.tsx (1)
Avatar(9-48)
apps/website/src/app/api/trpc/[trpc]/route.ts (2)
packages/api/src/root.ts (1)
appRouter(7-12)packages/api/src/trpc.ts (1)
createTRPCContext(7-21)
packages/api/src/env.ts (1)
packages/db/src/env.ts (1)
env(4-38)
apps/website/src/trpc/react.tsx (3)
apps/website/src/trpc/query-client.ts (1)
createQueryClient(7-24)packages/api/src/trpc.ts (1)
createTRPCContext(7-21)packages/api/src/root.ts (1)
AppRouter(14-14)
packages/api/src/router/organization.ts (2)
packages/api/src/trpc.ts (2)
publicProcedure(55-55)protectedProcedure(57-73)packages/db/src/schema/schema.ts (2)
organizations(35-50)organizationMembers(55-75)
apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx (2)
packages/api/src/index.ts (1)
RouterOutputs(9-9)apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx (1)
TaskCard(22-62)
packages/api/src/router/user.ts (2)
packages/api/src/trpc.ts (1)
publicProcedure(55-55)packages/db/src/schema/schema.ts (1)
users(16-30)
apps/desktop/src/renderer/screens/main/components/PlanView/TaskCard.tsx (1)
packages/api/src/index.ts (1)
RouterOutputs(9-9)
packages/db/src/client.ts (1)
packages/db/src/env.ts (1)
env(4-38)
packages/db/drizzle.config.ts (1)
packages/db/src/env.ts (1)
env(4-38)
packages/api/src/router/repository.ts (2)
packages/api/src/trpc.ts (2)
publicProcedure(55-55)protectedProcedure(57-73)packages/db/src/schema/schema.ts (1)
repositories(80-104)
apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx (5)
packages/api/src/index.ts (1)
RouterOutputs(9-9)apps/desktop/src/lib/mock-data.ts (2)
mockTasks(135-393)mockUsers(24-51)apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx (1)
TaskPage(46-384)apps/desktop/src/renderer/screens/main/components/PlanView/KanbanColumn.tsx (1)
KanbanColumn(14-47)apps/desktop/src/renderer/screens/main/components/PlanView/CreateTaskModal.tsx (1)
CreateTaskModal(26-144)
packages/api/src/trpc.ts (2)
apps/website/src/trpc/react.tsx (1)
createTRPCContext(26-26)packages/api/src/env.ts (1)
env(4-12)
apps/desktop/src/renderer/contexts/WorktreeContext.tsx (1)
apps/desktop/src/shared/types.ts (1)
Worktree(58-67)
apps/desktop/src/renderer/screens/main/components/PlanView/TaskDetailModal.tsx (2)
packages/api/src/index.ts (1)
RouterOutputs(9-9)packages/ui/src/components/dialog.tsx (4)
Dialog(131-131)DialogContent(133-133)DialogHeader(136-136)DialogTitle(139-139)
packages/api/src/router/task.ts (3)
packages/api/src/trpc.ts (2)
publicProcedure(55-55)protectedProcedure(57-73)packages/db/src/schema/schema.ts (1)
tasks(109-148)packages/db/src/schema/enums.ts (1)
taskStatusEnumValues(3-12)
apps/desktop/src/renderer/screens/main/components/PlanView/EditTaskModal.tsx (5)
packages/api/src/index.ts (1)
RouterOutputs(9-9)packages/ui/src/components/dialog.tsx (4)
Dialog(131-131)DialogContent(133-133)DialogHeader(136-136)DialogTitle(139-139)packages/ui/src/components/label.tsx (1)
Label(22-22)packages/ui/src/components/input.tsx (1)
Input(21-21)packages/ui/src/components/button.tsx (1)
Button(61-61)
apps/desktop/src/renderer/screens/main/components/PlanView/TaskPage.tsx (2)
packages/api/src/index.ts (1)
RouterOutputs(9-9)packages/db/src/schema/schema.ts (1)
users(16-30)
packages/api/src/root.ts (1)
packages/api/src/trpc.ts (1)
createTRPCRouter(53-53)
apps/desktop/src/renderer/screens/main/components/PlanView/CreateTaskModal.tsx (5)
packages/api/src/index.ts (1)
RouterOutputs(9-9)packages/ui/src/components/dialog.tsx (4)
Dialog(131-131)DialogContent(133-133)DialogHeader(136-136)DialogTitle(139-139)packages/ui/src/components/label.tsx (1)
Label(22-22)packages/ui/src/components/input.tsx (1)
Input(21-21)packages/ui/src/components/button.tsx (1)
Button(61-61)
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (3)
apps/desktop/src/shared/types.ts (1)
Worktree(58-67)apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (2)
TaskStatus(3-9)StatusIndicator(53-113)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskAssignee.tsx (1)
TaskAssignee(10-40)
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (1)
packages/db/src/schema/enums.ts (1)
TaskStatus(14-14)
packages/db/src/env.ts (1)
packages/api/src/env.ts (1)
env(4-12)
packages/db/src/schema/schema.ts (1)
packages/db/src/schema/enums.ts (1)
taskStatusEnumValues(3-12)
apps/desktop/src/renderer/screens/main/components/NewLayout/AddTaskModal.tsx (3)
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (1)
TaskStatus(3-9)packages/db/src/schema/enums.ts (1)
TaskStatus(14-14)apps/desktop/src/renderer/screens/main/components/NewLayout/Avatar.tsx (1)
Avatar(9-48)
apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx (8)
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (1)
TaskStatus(3-9)apps/desktop/src/shared/types.ts (1)
Worktree(58-67)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (2)
WorktreeWithTask(15-28)TaskTabs(42-286)apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx (1)
PlanView(13-202)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
Sidebar(28-359)apps/desktop/src/renderer/screens/main/components/PlaceholderState.tsx (1)
PlaceholderState(7-44)apps/desktop/src/renderer/screens/main/components/TabContent/components/DiffTab/DiffTab.tsx (1)
DiffTab(18-165)apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx (1)
TabContent(31-155)
packages/db/src/schema/relations.ts (1)
packages/db/src/schema/schema.ts (5)
users(16-30)organizationMembers(55-75)tasks(109-148)organizations(35-50)repositories(80-104)
🔇 Additional comments (28)
packages/db/drizzle/meta/_journal.json (1)
1-13: Formatting-only change to Drizzle migration metadata—safe to merge.The file contains only whitespace/indentation standardization to tabs across the entire JSON structure. No keys, values, or migration metadata have changed, so this is a safe, cosmetic update. The migration entry
0000_initial_migrationremains intact.Since this is an auto-generated Drizzle ORM metadata file, confirm that it's intended to remain in version control (rather than in
.gitignore). If it is tracked, ensure your build/CI process doesn't regenerate and re-commit it on each run.packages/api/tsconfig.json (1)
2-8: No issues identified.All changes are whitespace/indentation standardization with no functional or semantic impact on the TypeScript configuration. The file structure and configuration values remain identical.
apps/desktop/package.json (1)
82-82: Formatting change looks good.The addition of a trailing newline is a minor formatting improvement that aligns with common linting and version control best practices. The package.json structure, dependencies, and scripts remain unchanged and appropriate for supporting the new features in this PR.
packages/db/src/env.ts (2)
1-2: Imports look good.Appropriate dependencies for type-safe environment configuration.
4-38: Configuration is clean and well-documented.The environment schema is properly structured with:
- Type-safe database URL validation
- Clear documentation comments explaining each configuration option
- Consistent formatting and indentation
- Database-specific credentials appropriately separated from API package
packages/ui/src/components/dialog.tsx (2)
1-5: Type-only React import properly aligns with React 19.The switch to
import type * as Reacton line 3 is correct for React 19's modern JSX transform, which doesn't require React to be in scope. All usages throughout the file (React.ComponentProps) are type-only, so this pattern is safe and follows current best practices. Import statement formatting is consistent with semicolons.
7-128: No functional changes; formatting is consistent throughout.All component definitions preserve their original behavior while adopting standardized formatting (consistent indentation, trailing commas in className merges). The
DialogContentcomponent's optionalshowCloseButtonprop (line 50) is properly typed and defaults totrue. Radix UI primitive integration remains intact withdata-slotattributes for styling/identification.packages/db/drizzle.config.ts (1)
1-10: Formatting clean-up looks good.Quote normalization keeps the config consistent with the rest of the DB package; no functional impact spotted.
packages/db/src/schema/enums.ts (1)
4-11: Enum formatting aligns with project style.The updated quoting keeps
taskStatusEnumValuesunchanged while matching the new formatting convention.packages/db/src/schema/index.ts (1)
1-3: Barrel export order/quoting tidy-up acknowledged.Reordering and switching to double quotes reads cleanly and preserves the exported surface.
packages/db/src/client.ts (1)
1-23: Client setup formatting confirmed.The import, config, and drizzle client blocks are untouched functionally; quote/style tweaks match the rest of the PR.
packages/db/src/index.ts (1)
1-2: Exports remain intact.Only quoting changed; module surface stays the same.
packages/ui/src/components/select.tsx (1)
1-187: LGTM! Clean formatting standardization.The formatting changes are consistent and follow modern TypeScript/React conventions. The code maintains correct usage of Radix UI primitives with no behavioral changes.
packages/ui/src/components/textarea.tsx (2)
1-1: LGTM: Type-only import optimization.The change to
import type * as Reactis correct since the React namespace is only used for typing (React.ComponentProps<"textarea">), not runtime values. This reduces bundle size and follows TypeScript best practices.
3-18: LGTM: Formatting standardization.The formatting changes (double quotes, indentation, trailing comma on Line 11, and semicolon on export) improve code consistency with no functional impact. The Tailwind utilities, including
field-sizing-content, are valid for Tailwind v4.apps/desktop/src/main/lib/terminal.ts (1)
90-94: Formatting improvement enhances readability.The multiline expansion of this guard condition makes it easier to read and understand the defensive checks against destroyed Electron objects.
packages/ui/src/components/input.tsx (2)
1-1: Good optimization with type-only import.Changing to
import type * as Reactis appropriate since React is only used for theComponentPropstype utility. This helps with tree-shaking and makes the type-only usage explicit.
3-21: LGTM! Clean formatting refactor.The formatting changes (semicolons, indentation, spacing) are consistent with the broader standardization effort in this PR. No functional changes detected.
packages/ui/src/components/label.tsx (2)
1-4: LGTM! Type-only import improves tree-shaking.The change to
import type * as React from "react"is appropriate since React is only used in type annotations (React.ComponentProps). This aligns with TypeScript best practices and can help reduce bundle size.
6-20: LGTM! Formatting standardization applied correctly.The component logic remains unchanged and correct. The refactoring to an explicit return statement with adjusted indentation aligns with the PR's formatting standardization goals.
packages/db/drizzle/meta/0000_snapshot.json (1)
620-635: No issues found — migration file exists and snapshot is correct.The migration file
packages/db/drizzle/0000_initial_migration.sqlproperly defines thetask_statusenum with all expected values, including "backlog" and "completed". The schema snapshot correctly reflects this migration, confirming the Drizzle ORM workflow is functioning as intended. The schema changes align with the plan view feature objectives.apps/desktop/src/renderer/screens/main/components/MainContent/TabContent.tsx (1)
188-192: LGTM! Improved readability.The multi-line formatting of the conditional makes the logic clearer without changing behavior.
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskAssignee.tsx (1)
6-6: LGTM! Type alignment with Avatar component.The type broadening to
string | nullcorrectly aligns with the updated Avatar component's imageUrl prop type.apps/desktop/src/renderer/screens/main/components/NewLayout/TaskListItem.tsx (1)
59-63: LGTM! Improved formatting.The multi-line Avatar component usage improves readability.
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
25-25: LGTM! Clean propagation of selectedWorktreeId.The optional
selectedWorktreeIdprop is properly integrated with appropriate fallback tocurrentWorkspace?.activeWorktreeIdusing nullish coalescing.Also applies to: 39-39, 323-323
apps/desktop/src/renderer/screens/main/components/TopBar/TopBar.tsx (1)
10-11: LGTM! Well-implemented mode toggle.The mode switching UI is cleanly integrated:
- Optional
onModeChangeenables progressive enhancement- Default
mode = "edit"preserves existing behavior- Proper button semantics with
type="button"- Conditional rendering avoids UI clutter when mode switching isn't needed
Also applies to: 19-20, 51-82
apps/desktop/src/renderer/screens/main/components/PlanView/index.ts (1)
1-1: LGTM! Standard index export.Clean public API export for the PlanView component.
apps/desktop/src/renderer/screens/main/components/DiffView/DiffContent.tsx (1)
145-153: LGTM! Formatting adjustment.The reformatted className template literal maintains the same conditional logic while improving code layout.
| // If no image URL, show initials | ||
| if (!imageUrl) { | ||
| const initials = name | ||
| .split(" ") | ||
| .map((word) => word[0]) | ||
| .join("") | ||
| .toUpperCase() | ||
| .slice(0, 2); | ||
|
|
||
| return ( | ||
| <div | ||
| className="rounded-full flex items-center justify-center bg-neutral-700 text-neutral-200 shrink-0" | ||
| style={{ | ||
| width: size, | ||
| height: size, | ||
| fontSize: size * 0.4, | ||
| }} | ||
| > | ||
| {initials} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Handle empty words in name splitting.
The initials generation could fail if name contains consecutive spaces or leading/trailing spaces, resulting in empty strings in the split array. Accessing word[0] on an empty string returns undefined.
Apply this diff to filter out empty words:
// If no image URL, show initials
if (!imageUrl) {
const initials = name
.split(" ")
+ .filter((word) => word.length > 0)
.map((word) => word[0])
.join("")
.toUpperCase()
.slice(0, 2);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If no image URL, show initials | |
| if (!imageUrl) { | |
| const initials = name | |
| .split(" ") | |
| .map((word) => word[0]) | |
| .join("") | |
| .toUpperCase() | |
| .slice(0, 2); | |
| return ( | |
| <div | |
| className="rounded-full flex items-center justify-center bg-neutral-700 text-neutral-200 shrink-0" | |
| style={{ | |
| width: size, | |
| height: size, | |
| fontSize: size * 0.4, | |
| }} | |
| > | |
| {initials} | |
| </div> | |
| ); | |
| } | |
| // If no image URL, show initials | |
| if (!imageUrl) { | |
| const initials = name | |
| .split(" ") | |
| .filter((word) => word.length > 0) | |
| .map((word) => word[0]) | |
| .join("") | |
| .toUpperCase() | |
| .slice(0, 2); | |
| return ( | |
| <div | |
| className="rounded-full flex items-center justify-center bg-neutral-700 text-neutral-200 shrink-0" | |
| style={{ | |
| width: size, | |
| height: size, | |
| fontSize: size * 0.4, | |
| }} | |
| > | |
| {initials} | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/NewLayout/Avatar.tsx around
lines 14 to 35, the initials generation can break when name contains extra
spaces because split produces empty strings and word[0] becomes undefined; fix
this by trimming the name, splitting on spaces, filtering out empty words (e.g.,
filter(Boolean) or word => word.length > 0), then map to the first character of
each remaining word, join, uppercase and slice to two characters so empty
segments are ignored.
| export type TaskStatus = | ||
| | "backlog" | ||
| | "planning" | ||
| | "working" | ||
| | "needs-feedback" | ||
| | "ready-to-merge" | ||
| | "completed"; |
There was a problem hiding this comment.
Add missing status cases to prevent runtime crash
With the Plan view you just added, both CreateTaskModal (Lines 111-119) and TaskPage (Lines 229-237) can emit "todo" and "canceled" statuses. When either of those flows drives the task tabs, STATUS_CONFIG[status] is undefined, so the render path dereferences config.type and blows up. Bringing the union and config up to date also keeps downstream code from handing unsupported strings to the backend. Please extend the union and config to cover every status we surface now.
export type TaskStatus =
- | "backlog"
+ | "backlog"
+ | "todo"
| "planning"
| "working"
| "needs-feedback"
| "ready-to-merge"
- | "completed";
+ | "completed"
+ | "canceled";
@@
const STATUS_CONFIG: Record<
TaskStatus,
{ label: string; color: string; type: "dashed" | "filled" | "pulsing" }
> = {
backlog: {
label: "Backlog",
color: "rgb(156, 163, 175)", // gray-400
type: "dashed",
},
+ todo: {
+ label: "Todo",
+ color: "rgb(59, 130, 246)", // blue-500
+ type: "dashed",
+ },
planning: {
label: "Planning",
color: "rgb(59, 130, 246)", // blue-500
type: "dashed",
},
@@
"ready-to-merge": {
label: "Ready to Merge",
color: "rgb(34, 197, 94)", // green-500
type: "filled",
},
completed: {
label: "Completed",
color: "rgb(34, 197, 94)", // green-500
type: "filled",
},
+ canceled: {
+ label: "Canceled",
+ color: "rgb(239, 68, 68)", // red-500
+ type: "filled",
+ },
};Also applies to: 21-51
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx
around lines 3-9 (and also update the STATUS_CONFIG block at lines ~21-51), the
TaskStatus union is missing "todo" and "canceled" which causes
STATUS_CONFIG[status] to be undefined at runtime; add "todo" and "canceled" to
the TaskStatus union and add corresponding entries in STATUS_CONFIG with the
same shape (type, label, color/icon fields) as the other statuses so renderers
can safely dereference config.type and downstream code cannot pass unsupported
strings to the backend.
| const tasksByStatus = useMemo(() => { | ||
| return { | ||
| backlog: tasks.filter((t) => t.status === "backlog"), | ||
| todo: tasks.filter((t) => t.status === "todo"), | ||
| planning: tasks.filter((t) => t.status === "planning"), | ||
| "needs-feedback": tasks.filter((t) => t.status === "needs-feedback"), | ||
| completed: tasks.filter((t) => t.status === "completed"), | ||
| }; | ||
| }, [tasks]); |
There was a problem hiding this comment.
Surface every status on the Kanban board.
Lines 32-40 only group tasks for backlog, todo, planning, needs-feedback, and completed, while Lines 160-190 render columns for that same subset. However, both CreateTaskModal and EditTaskModal let users set statuses such as “working”, “ready-to-merge”, and “canceled”. Any task assigned one of those statuses vanishes from Plan View entirely, so users immediately lose track of work they just updated. Please expand the grouping and columns so every selectable status remains visible.
Apply this diff to cover the missing statuses:
const tasksByStatus = useMemo(() => {
return {
backlog: tasks.filter((t) => t.status === "backlog"),
todo: tasks.filter((t) => t.status === "todo"),
planning: tasks.filter((t) => t.status === "planning"),
+ working: tasks.filter((t) => t.status === "working"),
"needs-feedback": tasks.filter((t) => t.status === "needs-feedback"),
+ "ready-to-merge": tasks.filter((t) => t.status === "ready-to-merge"),
completed: tasks.filter((t) => t.status === "completed"),
+ canceled: tasks.filter((t) => t.status === "canceled"),
};
}, [tasks]); <KanbanColumn
title="Pending"
tasks={tasksByStatus.planning}
onTaskClick={setViewingTask}
statusColor="bg-yellow-500"
/>
+ <KanbanColumn
+ title="Working"
+ tasks={tasksByStatus.working}
+ onTaskClick={setViewingTask}
+ statusColor="bg-amber-500"
+ />
<KanbanColumn
title="Needs Feedback"
tasks={tasksByStatus["needs-feedback"]}
onTaskClick={setViewingTask}
statusColor="bg-orange-500"
/>
+ <KanbanColumn
+ title="Ready to Merge"
+ tasks={tasksByStatus["ready-to-merge"]}
+ onTaskClick={setViewingTask}
+ statusColor="bg-emerald-500"
+ />
<KanbanColumn
title="Completed"
tasks={tasksByStatus.completed}
onTaskClick={setViewingTask}
statusColor="bg-green-600"
/>
+ <KanbanColumn
+ title="Canceled"
+ tasks={tasksByStatus.canceled}
+ onTaskClick={setViewingTask}
+ statusColor="bg-red-500"
+ />Also applies to: 160-190
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/PlanView/PlanView.tsx
around lines 32-40 (and the corresponding rendering block at lines ~160-190),
the tasks grouping and rendered Kanban columns only cover backlog, todo,
planning, needs-feedback, and completed, so tasks with statuses like working,
ready-to-merge, and canceled disappear; update the useMemo grouping to include
entries for every status allowed by CreateTaskModal/EditTaskModal (e.g.,
working, ready-to-merge, canceled — add any other selectable statuses), and
update the column rendering section (lines ~160-190) to render a column for each
of those statuses with appropriate column titles and task lists so no selectable
status is omitted from the Plan View.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ui/src/components/context-menu.tsx (1)
99-109: Consider systematic z-index management.The z-index was increased from
z-50toz-[9999]to ensure the context menu appears above PlanView modals. While this solves the immediate layering issue, consider:
- Using CSS variables or design system tokens for z-index values (e.g.,
--z-context-menu,--z-modal,--z-overlay)- Establishing a z-index scale to prevent "z-index wars" as more layered components are added
- Documenting the z-index hierarchy
Note: The explicit
container={document.body}is likely redundant since Radix portals default todocument.body, unless you're overriding a parent portal context.// Example: Define z-index tokens in a central location // e.g., in your Tailwind config or CSS variables const Z_INDEX = { contextMenu: 9999, modal: 9000, popover: 8000, // ... } as const; // Then use: z-[var(--z-context-menu)] or create a Tailwind plugin
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/src/components/context-menu.tsx(2 hunks)
🔇 Additional comments (1)
packages/ui/src/components/context-menu.tsx (1)
10-10: Verify accessibility impact through manual testing in actual PlanView context.The
modal={false}setting is a documented Radix UI option that disables focus trapping and scroll-locking. While this is a valid pattern (often used as a workaround for complex portal/modal scenarios), it requires verification that:
- Keyboard navigation (Tab, Escape, arrow keys) functions correctly across all three production usages (WorkspaceSwitcher, WorktreeItem, TabItem)
- Screen reader announcements remain accurate when menus appear alongside PlanView modals
- Focus management doesn't cause issues when multiple interactive layers are open
The codebase lacks dedicated accessibility test coverage for ContextMenu, so manual testing with the PlanView modals and assistive technologies is necessary before merging.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (4)
conductor-setup.sh (1)
1-46: Document environment variables and error handling.The script relies on several environment variables (
CONDUCTOR_ROOT_PATH,CONDUCTOR_WORKSPACE_PATH,CONDUCTOR_PROJECT_ID) and external commands with potential failure modes (network issues, auth, Neon API changes). Add comments and/or a helper section at the top explaining what variables are required and what can go wrong.Consider adding a documentation block near the top:
#!/usr/bin/env bash set -e + # Setup Superset workspace with Neon database branching + # + # Required environment variables: + # CONDUCTOR_ROOT_PATH: Path to repository root (usually set by .envrc) + # CONDUCTOR_WORKSPACE_PATH: (Optional) Workspace name; defaults to current directory basename + # CONDUCTOR_PROJECT_ID: Neon project ID for database branching + # + # Required tools: + # - bun (package manager) + # - neonctl (Neon CLI) + # - jq (JSON processor)apps/website/src/app/components/FeaturesSection/FeaturesSection.tsx (1)
35-80: Consider extracting repeated animation configuration.The animation properties
initial={{ opacity: 0, y: 20 }},whileInView={{ opacity: 1, y: 0 }}, andviewport={{ once: true, margin: "-100px" }}are repeated across multiple motion components. Consider extracting these to constants for better maintainability.Example refactor:
+const FADE_IN_ANIMATION = { + initial: { opacity: 0, y: 20 }, + whileInView: { opacity: 1, y: 0 }, + viewport: { once: true, margin: "-100px" }, +}; + export function FeaturesSection({ onOpenWaitlist }: FeaturesSectionProps) { return ( <section className="py-16 sm:py-24 md:py-32 px-4 sm:px-6 md:px-8 bg-black"> <div className="max-w-3xl mx-auto"> <motion.div - initial={{ opacity: 0, y: 20 }} - whileInView={{ opacity: 1, y: 0 }} - viewport={{ once: true, margin: "-100px" }} + {...FADE_IN_ANIMATION} transition={{ duration: 0.5 }}apps/website/src/app/components/ClientLogosSection/constants.ts (1)
1-11: Consider clarifying the logo field's purpose.The
logofield mixes domains (e.g., "numbies.xyz", "findmymachinist.com") with brand names (e.g., "Amazon", "Google"). If this field is intended for display text only, the current approach works. However, if you plan to use actual logo images or need consistent formatting, consider standardizing to brand names only or separating display text from image paths.apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/LitBackground.tsx (1)
27-32: Remove unusedglarePropertiesRefstateLine 27 introduces
glarePropertiesRef, but the ref is never read anywhere—Line 68 only writes to it. Dropping this ref (and the assignment) trims dead state and avoids keeping an unused object alive. Consider the diff below.- const glarePropertiesRef = useRef({ - isActive: false, - lightX: 0, - lightY: 0, - intensity: 0, - }); ... - const glareProps = calculateGlareProperties(glareStartTime, Date.now()); - glarePropertiesRef.current = glareProps; + const glareProps = calculateGlareProperties(glareStartTime, Date.now());Also applies to: 68-68
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
-
.claude/agents/project-structure-validator.md(1 hunks) -
.claude/agents/project-structure-validator/.gitignore(1 hunks) -
.claude/agents/project-structure-validator/build-component-graph.sh(1 hunks) -
.gitignore(1 hunks) -
AGENTS.md(2 hunks) -
apps/website/src/app/components/ClientLogosSection/ClientLogosSection.tsx(1 hunks) -
apps/website/src/app/components/ClientLogosSection/constants.ts(1 hunks) -
apps/website/src/app/components/ClientLogosSection/index.ts(1 hunks) -
apps/website/src/app/components/FeaturesSection/FeaturesSection.tsx(1 hunks) -
apps/website/src/app/components/FeaturesSection/index.ts(1 hunks) -
apps/website/src/app/components/Footer/index.ts(1 hunks) -
apps/website/src/app/components/Header/Header.tsx(1 hunks) -
apps/website/src/app/components/Header/index.ts(1 hunks) -
apps/website/src/app/components/HeroSection/HeroSection.tsx(1 hunks) -
apps/website/src/app/components/HeroSection/components/FadeUp/index.ts(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/HeroCanvas.tsx(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/LitBackground.tsx(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/index.ts(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/config.ts(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/helpers.ts(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/shaders/fragment.ts(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroCanvas/shaders/vertex.ts(1 hunks) -
apps/website/src/app/components/HeroSection/components/HeroParallax/index.ts(1 hunks) -
apps/website/src/app/components/HeroSection/index.ts(1 hunks) -
apps/website/src/app/components/WaitlistModal/index.ts(1 hunks) -
apps/website/src/app/page.tsx(1 hunks) -
apps/website/src/components/motion/TiltCard.tsx(0 hunks) -
apps/website/src/components/motion/index.ts(0 hunks) -
conductor-setup.sh(1 hunks) -
conductor.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/website/src/components/motion/index.ts
- apps/website/src/components/motion/TiltCard.tsx
✅ Files skipped from review due to trivial changes (5)
- apps/website/src/app/components/HeroSection/components/FadeUp/index.ts
- apps/website/src/app/components/Header/Header.tsx
- .gitignore
- .claude/agents/project-structure-validator.md
- .claude/agents/project-structure-validator/.gitignore
🧰 Additional context used
🧬 Code graph analysis (6)
apps/website/src/app/components/HeroSection/components/HeroCanvas/helpers.ts (1)
apps/website/src/app/components/HeroSection/components/HeroCanvas/config.ts (1)
TEXT_CONFIG(33-49)
apps/website/src/app/components/FeaturesSection/FeaturesSection.tsx (1)
apps/website/src/app/components/FeaturesSection/index.ts (1)
FeaturesSection(1-1)
apps/website/src/app/components/HeroSection/HeroSection.tsx (3)
apps/website/src/app/components/HeroSection/components/HeroParallax/HeroParallax.tsx (1)
HeroParallax(20-63)apps/website/src/app/components/HeroSection/components/HeroCanvas/HeroCanvas.tsx (1)
HeroCanvas(11-39)apps/website/src/app/components/HeroSection/components/FadeUp/FadeUp.tsx (1)
FadeUp(24-46)
apps/website/src/app/components/HeroSection/components/HeroCanvas/HeroCanvas.tsx (1)
apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/LitBackground.tsx (1)
LitBackground(19-258)
apps/website/src/app/components/ClientLogosSection/ClientLogosSection.tsx (1)
apps/website/src/app/components/ClientLogosSection/constants.ts (1)
CLIENT_LOGOS(1-11)
apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/LitBackground.tsx (1)
apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/index.ts (1)
LitBackground(1-1)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
.claude/agents/project-structure-validator/build-component-graph.sh
[error] 18-18: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
[error] 21-21: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
🔇 Additional comments (17)
apps/website/src/app/components/WaitlistModal/index.ts (1)
1-1: LGTM! Standard barrel export pattern verified.The barrel export correctly re-exports
WaitlistModalfrom the source module. Verification confirms thatWaitlistModal.tsxexists and exports theWaitlistModalfunction as a named export on line 10.conductor.json (1)
1-5: JSON structure looks good.Standard npm-style manifest that cleanly exposes the setup script via conductor's CLI.
apps/website/src/app/components/FeaturesSection/index.ts (1)
1-1: LGTM!Standard barrel export pattern that provides a clean public API surface for the FeaturesSection component.
apps/website/src/app/components/FeaturesSection/FeaturesSection.tsx (2)
1-5: LGTM!Clean import and interface definition. The
onOpenWaitlistcallback is properly typed.
7-29: No duplication concern found—code is fine.Verification shows that
SCALE_FEATURESandScaleFeaturesSectiondo not exist in the codebase. TheFEATURESconstant in FeaturesSection.tsx is standalone with no duplicate elsewhere. The comment at line 7 is misleading (it references a non-existent source), but this does not affect the code itself. No refactoring is needed.apps/website/src/app/components/ClientLogosSection/index.ts (1)
1-1: LGTM!Standard barrel export for ClientLogosSection component.
apps/website/src/app/components/Footer/index.ts (1)
1-1: LGTM!Standard barrel export for Footer component.
apps/website/src/app/components/HeroSection/components/HeroCanvas/components/LitBackground/index.ts (1)
1-1: LGTM!Standard barrel export for LitBackground component.
apps/website/src/app/components/HeroSection/index.ts (1)
1-1: LGTM!Standard barrel export for HeroSection component.
apps/website/src/app/components/HeroSection/components/HeroParallax/index.ts (1)
1-1: LGTM!Standard barrel export for HeroParallax component and its associated
useHeroVisibilityhook.apps/website/src/app/components/Header/index.ts (1)
1-1: LGTM!Standard barrel export for Header component.
apps/website/src/app/components/HeroSection/components/HeroCanvas/shaders/fragment.ts (1)
1-36: LGTM!The fragment shader implements a standard Blinn-Phong lighting model with appropriate attenuation, diffuse, and specular calculations. The lighting parameters appear well-tuned for the desired visual effect.
apps/website/src/app/components/HeroSection/components/HeroCanvas/HeroCanvas.tsx (1)
11-39: Well-structured Canvas wrapper.The component properly configures the Three.js canvas with performance optimizations (limited DPR, performance hints, high-performance WebGL preference) and uses Suspense for async loading. The inline styles for hardware acceleration (
willChange,translateZ) are appropriate for 3D rendering.Note: The
pointerEvents: "auto"style works correctly with the parent'spointer-events-noneclass mentioned in HeroSection.tsx (Line 7), allowing the canvas to remain interactive.apps/website/src/app/components/HeroSection/components/HeroCanvas/shaders/vertex.ts (1)
1-23: LGTM!The vertex shader correctly implements a dual sine wave animation with proper normal calculation from wave derivatives. The derivative scaling (0.05 vs 0.1) and normal computation are mathematically sound for achieving realistic lighting on the animated surface.
apps/website/src/app/components/HeroSection/HeroSection.tsx (1)
5-27: Well-composed hero section.The component effectively layers parallax effects, canvas background, and animated text. The Tailwind v4 gradient syntax (
bg-linear-to-b) is correct, and the component structure follows good composition patterns.Note: The large top margins (
mt-[20rem] sm:mt-[30rem]) combined with negative margins on the heading create a specific visual layout. Ensure this spacing remains stable across different viewport heights and content scenarios.apps/website/src/app/components/HeroSection/components/HeroCanvas/helpers.ts (1)
6-45: Clean glare animation logic.The function correctly implements cyclic glare animation with smooth ease-in-out interpolation and center-peaked intensity. The timing and progression calculations are sound.
AGENTS.md (1)
72-123: Excellent project structure documentation.The new Project Structure section provides clear, actionable guidance on component organization with practical examples. The co-location rules (usage-based nesting, one component per file, co-located dependencies) align with modern React best practices and will help maintain consistency across the codebase.
| first=true | ||
| find "$DIR" -name "*.tsx" -type f ! -path "*/node_modules/*" ! -name "*.test.tsx" ! -name "*.stories.tsx" | while read file; do | ||
| component=$(basename "$file" .tsx) | ||
| path="${file#$DIR/}" | ||
|
|
||
| # Count imports | ||
| count=$(grep -r "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ') | ||
|
|
||
| # Get importers | ||
| importers=$(grep -l "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" -r 2>/dev/null | grep -v "$file" | sed "s|^$DIR/||" | paste -sd "," -) | ||
|
|
||
| if [ "$first" = true ]; then | ||
| first=false | ||
| else | ||
| echo "," >> "$OUTPUT" | ||
| fi | ||
|
|
||
| echo -n " \"$path\": {\"component\": \"$component\", \"imports\": $count, \"importers\": [" >> "$OUTPUT" | ||
| if [ -n "$importers" ]; then | ||
| echo "$importers" | sed 's/,/","/g' | sed 's/^/"/' | sed 's/$/"/' | tr -d '\n' >> "$OUTPUT" | ||
| fi | ||
| echo -n "]}" >> "$OUTPUT" | ||
| done |
There was a problem hiding this comment.
Critical: Subshell causes JSON formatting to break.
The find ... | while read construct runs the loop in a subshell, so modifications to the first variable inside the loop don't persist. This means every entry will attempt to print a leading comma, resulting in invalid JSON. Additionally, any output file modifications in the subshell won't be visible to the parent process in the way you expect.
Refactor to avoid the subshell by using process substitution or by redirecting the entire block:
-find "$DIR" -name "*.tsx" -type f ! -path "*/node_modules/*" ! -name "*.test.tsx" ! -name "*.stories.tsx" | while read file; do
+while IFS= read -r file; do
component=$(basename "$file" .tsx)
path="${file#$DIR/}"
# Count imports
- count=$(grep -r "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ')
+ count=$(grep -r "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ')
# Get importers
- importers=$(grep -l "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" -r 2>/dev/null | grep -v "$file" | sed "s|^$DIR/||" | paste -sd "," -)
+ importers=$(grep -l "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" -r 2>/dev/null | grep -v "$file" | sed "s|^$DIR/||" | paste -sd "," -)
if [ "$first" = true ]; then
first=false
else
echo "," >> "$OUTPUT"
fi
echo -n " \"$path\": {\"component\": \"$component\", \"imports\": $count, \"importers\": [" >> "$OUTPUT"
if [ -n "$importers" ]; then
echo "$importers" | sed 's/,/","/g' | sed 's/^/"/' | sed 's/$/"/' | tr -d '\n' >> "$OUTPUT"
fi
echo -n "]}" >> "$OUTPUT"
-done
+done < <(find "$DIR" -name "*.tsx" -type f ! -path "*/node_modules/*" ! -name "*.test.tsx" ! -name "*.stories.tsx")🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 18-18: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
[error] 21-21: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
| find "$DIR" -name "*.tsx" -type f ! -path "*/node_modules/*" ! -name "*.test.tsx" ! -name "*.stories.tsx" | while read file; do | ||
| component=$(basename "$file" .tsx) | ||
| path="${file#$DIR/}" | ||
|
|
||
| # Count imports | ||
| count=$(grep -r "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ') | ||
|
|
||
| # Get importers | ||
| importers=$(grep -l "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" -r 2>/dev/null | grep -v "$file" | sed "s|^$DIR/||" | paste -sd "," -) | ||
|
|
||
| if [ "$first" = true ]; then | ||
| first=false | ||
| else | ||
| echo "," >> "$OUTPUT" | ||
| fi | ||
|
|
||
| echo -n " \"$path\": {\"component\": \"$component\", \"imports\": $count, \"importers\": [" >> "$OUTPUT" | ||
| if [ -n "$importers" ]; then | ||
| echo "$importers" | sed 's/,/","/g' | sed 's/^/"/' | sed 's/$/"/' | tr -d '\n' >> "$OUTPUT" | ||
| fi | ||
| echo -n "]}" >> "$OUTPUT" | ||
| done |
There was a problem hiding this comment.
O(n²) complexity: Recursive grep for every component is inefficient.
For each .tsx file found, the script runs a full recursive grep across the entire project. On a large codebase, this will be extremely slow. A single pass with grep -r collecting all imports would be far more efficient.
Consider a two-pass approach:
- Build a map of all imports in a single
grep -rcall - Look up each component in that map
This would reduce complexity from O(n²) to O(n log n).
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 18-18: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
[error] 21-21: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
🤖 Prompt for AI Agents
.claude/agents/project-structure-validator/build-component-graph.sh around lines
13 to 34: the current loop performs a recursive grep for every .tsx file which
yields O(n²) behavior; instead run one global grep once to collect all import
lines into a temporary map (or file) keyed by imported module/name, then iterate
your .tsx files and lookup the component name in that map to get count and
importers; implement a two-pass approach: (1) single grep -r across $DIR for
import patterns and normalize the imported module strings into a lookup (e.g.,
module -> list of importing paths), (2) while iterating .tsx files, derive the
component key and query the prebuilt map to produce counts and importer lists,
removing per-file recursive greps and using efficient text tools
(awk/sort/uniq/grep -F) to compute counts.
| path="${file#$DIR/}" | ||
|
|
||
| # Count imports | ||
| count=$(grep -r "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ') |
There was a problem hiding this comment.
Unquoted $component will break with special characters in component names.
When $component is used unquoted in the grep pattern, any special regex characters (like ., *, [) will be interpreted as regex metacharacters. Additionally, whitespace in component names will cause word splitting.
Quote the variable in both grep invocations:
- count=$(grep -r "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ')
+ count=$(grep -r "from.*['\"].*${component}['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ')
- importers=$(grep -l "from.*['\"].*$component['\"]" "$DIR" --include="*.tsx" --include="*.ts" -r 2>/dev/null | grep -v "$file" | sed "s|^$DIR/||" | paste -sd "," -)
+ importers=$(grep -l "from.*['\"].*${component}['\"]" "$DIR" --include="*.tsx" --include="*.ts" -r 2>/dev/null | grep -v "$file" | sed "s|^$DIR/||" | paste -sd "," -)Also address the Shellcheck SC1087 warnings by using braces when expanding the variable.
Also applies to: 21-21
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 18-18: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
🤖 Prompt for AI Agents
.claude/agents/project-structure-validator/build-component-graph.sh around lines
18 and 21: the unquoted $component (and bare expansions) in the grep patterns
will break on component names containing spaces or regex metacharacters and
triggers Shellcheck SC1087; update both grep invocations to quote the pattern
and use braced expansions (e.g. "${component}") and ensure other variables used
in the command (like "${file}" and "${DIR}") are also quoted so the pattern
string is passed intact and no word-splitting or unintended glob/regex
interpretation occurs.
Import pattern matching is too loose and matches in comments and strings.
The grep pattern "from.*['\"].*$component['\"]" will match import statements in comments, console.logs, error messages, or any string literal. This inflates the import count and produces false positives.
For more robust import detection, consider:
- Using a dedicated AST parser (e.g., with Node.js and
@babel/parser) - Restricting to lines that start with
importorfromkeywords - Excluding common comment patterns
- count=$(grep -r "from.*['\"].*${component}['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ')
+ count=$(grep -r "^[[:space:]]*import.*from.*['\"].*${component}['\"]" "$DIR" --include="*.tsx" --include="*.ts" 2>/dev/null | grep -v "$file" | wc -l | tr -d ' ')Or better: delegate to a proper parser external to this shell script.
Also applies to: 21-21
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 18-18: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
(SC1087)
🤖 Prompt for AI Agents
In .claude/agents/project-structure-validator/build-component-graph.sh around
lines 18 and 21, the grep pattern "from.*['\"].*$component['\"]" is too loose
and matches comments and strings; narrow the match to real import/require
statements by restricting grep to lines that begin with optional whitespace
followed by "import" and a "from" (e.g. use a POSIX ERE like ^[[:space:]]*import
.*from[[:space:]]+['\"]<component>['\"]) and add a second check for common
require(...) patterns, and filter out lines beginning with comment tokens (// or
/*) — or better, replace this fragile shell grep with a small Node.js helper
using @babel/parser to parse files and count actual import/require AST nodes for
the target component. Ensure the new approach is applied to both occurrences at
lines 18 and 21.
| if [ -n "$importers" ]; then | ||
| echo "$importers" | sed 's/,/","/g' | sed 's/^/"/' | sed 's/$/"/' | tr -d '\n' >> "$OUTPUT" | ||
| fi | ||
| echo -n "]}" >> "$OUTPUT" |
There was a problem hiding this comment.
Sed commands don't escape special characters in file paths.
If $importers contains paths with special sed delimiters (like /, &, or \), the sed commands will fail or produce incorrect output. The path substitution on line 31 is particularly vulnerable.
Escape special characters before piping to sed, or use a safer delimiter:
echo -n " \"$path\": {\"component\": \"$component\", \"imports\": $count, \"importers\": [" >> "$OUTPUT"
if [ -n "$importers" ]; then
- echo "$importers" | sed 's/,/","/g' | sed 's/^/"/' | sed 's/$/"/' | tr -d '\n' >> "$OUTPUT"
+ echo "$importers" | sed -e 's/,/","/g' -e 's/^/"/' -e 's/$/"/' | tr -d '\n' >> "$OUTPUT"
fiBetter yet, use jq or native parameter expansion to build valid JSON strings:
if [ -n "$importers" ]; then
# Convert comma-separated list to JSON array
+ echo "$importers" | tr ',' '\n' | sed 's/^/"/; s/$/"/' | paste -sd ',' - | tr ',' ' ' >> "$OUTPUT"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .claude/agents/project-structure-validator/build-component-graph.sh around
lines 30-33, the sed pipeline that transforms $importers into quoted JSON
strings fails when paths contain sed-special characters (/, &, \, "). Replace
the sed-based approach with one that safely JSON-escapes each path and writes a
properly quoted list: either iterate over the importers, escape backslashes and
double-quotes, wrap each escaped path in double quotes and join with commas,
then append to $OUTPUT, or invoke jq to read raw strings and emit a JSON array
(e.g. pass each path to jq -R --slurp --compact-output). Ensure the escape
handles backslash and double-quote characters so the resulting JSON is valid.
| ``` | ||
| app/ | ||
| ├── page.tsx | ||
| ├── dashboard/ | ||
| │ ├── page.tsx | ||
| │ └── components/ | ||
| │ └── MetricsChart/ | ||
| │ ├── MetricsChart.tsx | ||
| │ ├── MetricsChart.test.tsx # Tests co-located | ||
| │ ├── index.ts | ||
| │ ├── useMetricsData.ts # Hook used only here | ||
| │ └── constants.ts | ||
| └── components/ | ||
| ├── Sidebar/ | ||
| │ ├── Sidebar.tsx | ||
| │ ├── Sidebar.test.tsx # Tests co-located | ||
| │ ├── index.ts | ||
| │ ├── components/ # Used 2+ times IN Sidebar | ||
| │ │ └── SidebarButton/ # Shared by SidebarNav + SidebarFooter | ||
| │ │ ├── SidebarButton.tsx | ||
| │ │ ├── SidebarButton.test.tsx | ||
| │ │ └── index.ts | ||
| │ ├── SidebarNav/ | ||
| │ │ ├── SidebarNav.tsx | ||
| │ │ └── index.ts | ||
| │ └── SidebarFooter/ | ||
| │ ├── SidebarFooter.tsx | ||
| │ └── index.ts | ||
| └── HeroSection/ | ||
| ├── HeroSection.tsx | ||
| ├── HeroSection.test.tsx # Tests co-located | ||
| ├── index.ts | ||
| └── components/ # Used ONLY by HeroSection | ||
| └── HeroCanvas/ | ||
| ├── HeroCanvas.tsx | ||
| ├── HeroCanvas.test.tsx | ||
| ├── HeroCanvas.stories.tsx | ||
| ├── index.ts | ||
| └── config.ts | ||
|
|
||
| components/ # Used in 2+ pages (last resort) | ||
| └── Header/ | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The code block illustrating the project structure is missing a language specifier, which is flagged by markdownlint.
Apply this diff to fix the linting issue:
-```
+```plaintext
app/
├── page.tsx
├── dashboard/🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
76-76: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In AGENTS.md around lines 76 to 118 the fenced code block showing the project
tree lacks a language identifier, triggering markdownlint; update the opening
fence from ``` to ```plaintext (or another appropriate language like ```text) so
the code block includes a language specifier and the lint warning is resolved.
| <motion.div | ||
| className="flex gap-12 sm:gap-16 md:gap-24" | ||
| animate={{ | ||
| x: [0, -1000], | ||
| }} | ||
| transition={{ | ||
| x: { | ||
| repeat: Number.POSITIVE_INFINITY, | ||
| repeatType: "loop", | ||
| duration: 20, | ||
| ease: "linear", | ||
| }, | ||
| }} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the animation distance for seamless looping.
The horizontal animation uses a hardcoded -1000 pixel translation (Line 30), which may not match the actual width of the logo content. For a seamless infinite loop, the translation distance should equal the width of one complete logo set. With 10 logos and responsive gaps (gap-12 sm:gap-16 md:gap-24), the actual width could be significantly different, potentially causing visible gaps or overlaps.
Consider calculating the translation distance dynamically:
const LogoStrip = () => {
const ref = useRef<HTMLDivElement>(null);
const [offset, setOffset] = useState(-1000);
useEffect(() => {
if (ref.current) {
const width = ref.current.offsetWidth;
setOffset(-width / 3); // Divide by 3 since we render 3 sets
}
}, []);
return (
<motion.div
ref={ref}
animate={{ x: [0, offset] }}
// ... rest of config
/>
);
};I'll verify the animation implementation in the ClientLogosSection component.
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the full ClientLogosSection component
cat apps/website/src/app/components/ClientLogosSection/ClientLogosSection.tsx
Length of output: 1855
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the CLIENT_LOGOS constants
cat apps/website/src/app/components/ClientLogosSection/constants.tsx
Length of output: 96
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the constants file
fd -e tsx -e ts "const" apps/website/src/app/components/ClientLogosSection/
# Also search for CLIENT_LOGOS definition
rg -n "CLIENT_LOGOS" apps/website/src/app/components/ClientLogosSection/ --type ts
Length of output: 397
---
🏁 Script executed:
```shell
#!/bin/bash
# View the CLIENT_LOGOS array
cat apps/website/src/app/components/ClientLogosSection/constants.ts
Length of output: 419
Calculate animation distance dynamically to match actual content width.
The review comment correctly identifies a real issue. With 9 logos, responsive gaps (up to 96px at gap-24), and 3 duplicated sets for seamless looping, the actual width of one logo set is approximately 1400-1500px. The hardcoded -1000 pixel translation is insufficient and will cause visible gaps or stuttering when the animation loops.
The suggested dynamic calculation using useRef and offsetWidth is appropriate. Update the component to measure the actual rendered width of one logo set and translate accordingly:
const LogoStrip = () => {
const ref = useRef<HTMLDivElement>(null);
const [offset, setOffset] = useState(-1000);
useEffect(() => {
if (ref.current) {
const width = ref.current.offsetWidth;
setOffset(-width / 3); // Divide by 3 since we render 3 sets
}
}, []);
return (
<motion.div
ref={ref}
animate={{ x: [0, offset] }}
// ... rest of config
/>
);
};🤖 Prompt for AI Agents
In apps/website/src/app/components/ClientLogosSection/ClientLogosSection.tsx
around lines 27 to 40, the motion.div uses a hardcoded -1000px translation
causing visible gaps on loop; replace this with a measured width: add a ref to
the logo strip container, use useEffect to read ref.current.offsetWidth (and
update on window resize) and compute the translation as the negative width of
one duplicated set (e.g., -width / 3 when three sets are rendered), then use
that computed value for animate.x so the loop distance matches the actual
rendered content.
|
|
||
| # Link direnv config | ||
| echo "🔧 Linking .envrc..." | ||
| ln -sf "$CONDUCTOR_ROOT_PATH/.envrc" .envrc |
There was a problem hiding this comment.
Add validation for environment variables and jq output.
Line 25 uses $CONDUCTOR_ROOT_PATH without checking if it's set; if unset, ln -sf may fail silently. Lines 37–38 parse JSON without validating the output; if the connection URIs structure is unexpected, empty strings will be written to .env.
Apply these defensive checks:
# Link direnv config
echo "🔧 Linking .envrc..."
+ [ -z "$CONDUCTOR_ROOT_PATH" ] && error "CONDUCTOR_ROOT_PATH not set"
ln -sf "$CONDUCTOR_ROOT_PATH/.envrc" .envrc
success "direnv configured"
# Parse connection strings and create .env
DATABASE_URL=$(echo "$NEON_OUTPUT" | jq -r '.connection_uris[0].connection_uri')
- DATABASE_POOLED_URL=$(echo "$NEON_OUTPUT" | jq -r '.connection_uris[1].connection_uri // empty')
+ DATABASE_POOLED_URL=$(echo "$NEON_OUTPUT" | jq -r '.connection_uris[1].connection_uri // ""')
+
+ [ -z "$DATABASE_URL" ] && error "Failed to extract DATABASE_URL from Neon output"Also applies to: 37-38
| --project-id tiny-cherry-82420694 \ | ||
| --name "$WORKSPACE_NAME" \ | ||
| --output json) |
There was a problem hiding this comment.
Hardcoded Neon project ID is a blocker.
The project ID tiny-cherry-82420694 is hardcoded, which prevents multi-environment deployments and creates a hard-to-fix coupling to a single Neon project. This needs to be configurable.
Replace the hardcoded project ID with an environment variable:
NEON_OUTPUT=$(neonctl branches create \
- --project-id tiny-cherry-82420694 \
+ --project-id "${CONDUCTOR_PROJECT_ID:?CONDUCTOR_PROJECT_ID not set}" \
--name "$WORKSPACE_NAME" \
--output json)Ensure CONDUCTOR_PROJECT_ID is documented in .envrc or project README so contributors know how to set it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --project-id tiny-cherry-82420694 \ | |
| --name "$WORKSPACE_NAME" \ | |
| --output json) | |
| --project-id "${CONDUCTOR_PROJECT_ID:?CONDUCTOR_PROJECT_ID not set}" \ | |
| --name "$WORKSPACE_NAME" \ | |
| --output json) |
🤖 Prompt for AI Agents
In conductor-setup.sh around lines 32 to 34 the Neon project ID is hardcoded as
tiny-cherry-82420694 which prevents multi-environment deployments; replace that
literal with the environment variable CONDUCTOR_PROJECT_ID (e.g., use
"$CONDUCTOR_PROJECT_ID"), validate that the variable is set before use and exit
with a clear error if not, and add a note to .envrc or the project README
telling contributors to set CONDUCTOR_PROJECT_ID for their environment.
| cat > .env << EOF | ||
| DATABASE_URL=$DATABASE_URL | ||
| DATABASE_POOLED_URL=$DATABASE_POOLED_URL | ||
| EOF |
There was a problem hiding this comment.
Validate .env file before reporting success.
The script writes DATABASE_URL and DATABASE_POOLED_URL without checking they are non-empty. If extraction fails, the .env will be incomplete and downstream errors will be confusing.
Add validation after writing the .env file:
cat > .env << EOF
DATABASE_URL=$DATABASE_URL
DATABASE_POOLED_URL=$DATABASE_POOLED_URL
EOF
+
+ [ -f .env ] && [ -s .env ] || error "Failed to create .env file"
+ grep -q "DATABASE_URL=" .env || error ".env does not contain DATABASE_URL"
success "Neon branch created: $WORKSPACE_NAME"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cat > .env << EOF | |
| DATABASE_URL=$DATABASE_URL | |
| DATABASE_POOLED_URL=$DATABASE_POOLED_URL | |
| EOF | |
| cat > .env << EOF | |
| DATABASE_URL=$DATABASE_URL | |
| DATABASE_POOLED_URL=$DATABASE_POOLED_URL | |
| EOF | |
| [ -f .env ] && [ -s .env ] || error "Failed to create .env file" | |
| grep -q "DATABASE_URL=" .env || error ".env does not contain DATABASE_URL" | |
| success "Neon branch created: $WORKSPACE_NAME" |
🤖 Prompt for AI Agents
In conductor-setup.sh around lines 40 to 43, the script writes DATABASE_URL and
DATABASE_POOLED_URL to .env without validating they are set; add a validation
step after writing the file that reads or checks the two variables (or checks
the .env contents) and if either is empty, print a clear error to stderr and
exit non‑zero so the script does not report success when the .env is incomplete;
keep the error messages specific (which variable is missing) and exit with a
failure code.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Refactor