shows build#368
Open
shmbhvi101 wants to merge 5 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughFetches recipes via RecipeService and merges them with chats in ChatHistoryPanel; extends the Chat interface with Changes
Sequence DiagramsequenceDiagram
participant UI as ChatHistoryPanel UI
participant Logic as Component Logic
participant ChatAPI as Chat Service
participant RecipeAPI as Recipe Service
participant Router as Navigation Router
Logic->>ChatAPI: Fetch chats
Logic->>RecipeAPI: Fetch recipes
ChatAPI-->>Logic: Return chats[]
RecipeAPI-->>Logic: Return recipes[]
Logic->>Logic: Merge into allItems, filter & sort (pinned first, created_at desc)
Logic-->>UI: Render filteredItems
UI->>Logic: User clicks item
Logic->>Logic: Inspect item.type
alt item.type == 'recipe'
Logic->>Logic: Compute redirect via getRecipeRedirectUrl
Logic->>Router: Navigate to recipe URL
else item.type == 'chat'
Logic->>Router: Navigate to chat view
end
Router-->>UI: Update view
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
Cloud Run service deployed: https://pr-fix-show-build-cj6r7x3fpa-uc.a.run.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 587-590: The active highlighting logic only treats chats as active
via isActiveChat, so builds never receive active styling; update the active
determination inside the filteredItems.map block (where isActive is computed) to
derive activity from the current pathname and the same redirect/route logic used
in handleChatClick: check the pathname segments or parsed recipe id and compare
to item.id for both item.type === 'chat' and item.type === 'build' (or recipe),
using the same route resolution rules as handleChatClick so a clicked build row
remains styled as active.
- Around line 642-711: The menu shows Share/Rename/Delete for items with
item.type === "recipe" but the handlers
(openShareDialog/openRenameDialog/openDeleteDialog → which call handleCopyLink,
handleRename, handleDelete and ultimately ChatService) still target chat flows;
either hide those actions for recipes or implement recipe-specific handlers
using RecipeService. Update the component so when item.type === "recipe" you
either (A) conditionally omit the Share/Rename/Delete DropdownMenuItem nodes, or
(B) wire those buttons to new recipe handlers (e.g., handleRecipeShare,
handleRecipeRename, handleRecipeDelete) that call RecipeService methods instead
of ChatService and build correct recipe URLs (don’t use
/chat/${selectedChat.id}). Keep handlePinChat as-is for pin/unpin behavior.
- Around line 206-241: filteredItems' current logic collects pinnedItemList
before applying the search filter, causing pinned items to bypass
debouncedSearchTerm; fix by applying the same filtering predicate to both pinned
and non-pinned lists. Inside the useMemo that computes filteredItems, extract
the search/filter logic into a single predicate (referencing debouncedSearchTerm
and checks on item.title, item.repository, item.branch, item.agent_id,
item.status), then build pinnedItemList = allItems.filter(item =>
pinnedChats.has(item.id) && predicate(item)) and nonPinnedItems =
allItems.filter(item => !pinnedChats.has(item.id) && predicate(item)); finally
sort sortedPinned and sortedNonPinned by created_at and return [...sortedPinned,
...sortedNonPinned].
- Around line 156-161: The Recipe query is currently globally keyed so cached
builds can leak between users; update the useQuery call that invokes
RecipeService.getAllRecipes to include the current user's identity (e.g.,
user?.uid) in the queryKey and set the query's enabled (or equivalent) to only
run when auth is present; this ensures the key is unique per user and the query
is gated on auth so cached results aren't reused across logins.
🪄 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: c55bbcdd-2290-4049-b8e2-6eb4e4fe1ea8
📒 Files selected for processing (2)
components/Layouts/ChatHistoryPanel.tsxcomponents/Layouts/Sidebar.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
components/Layouts/ChatHistoryPanel.tsx (4)
157-161:⚠️ Potential issue | 🟠 MajorScope recipe cache by user and gate the query on auth.
This query key is still global, so builds can be reused across account switches in the same session. It should include user identity and be disabled until auth is present.
Suggested fix
-import RecipeService from "@/services/RecipeService"; +import RecipeService, { type Recipe } from "@/services/RecipeService"; ... -const { data: recipes, isLoading: recipesLoading } = useQuery<any[]>({ - queryKey: ["sidebar-recipes"], +const { data: recipes, isLoading: recipesLoading } = useQuery<Recipe[]>({ + queryKey: ["sidebar-recipes", user?.uid], queryFn: () => RecipeService.getAllRecipes(0, 100), + enabled: !!user?.uid, staleTime: 60000, // 1 minute });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 157 - 161, The query for recipes is global and runs without auth, causing cache leakage across accounts; update the useQuery in ChatHistoryPanel to scope the queryKey to the authenticated user (e.g., ["sidebar-recipes", currentUser?.id or currentUser?.email]) and add an enabled flag (e.g., enabled: !!currentUser) so the query is disabled until auth is present; keep the same queryFn (RecipeService.getAllRecipes) but reference currentUser in the key and enabled option to prevent reuse between users.
210-227:⚠️ Potential issue | 🟡 MinorPinned items still bypass search filtering.
pinnedItemListis built without applyingdebouncedSearchTerm, so unrelated pinned rows remain visible while searching. Apply one shared predicate to both pinned and non-pinned lists.Suggested fix
const filteredItems = useMemo(() => { if (!allItems) return []; + + const matchesSearch = (item: Chat) => { + if (!debouncedSearchTerm) return true; + const searchLower = debouncedSearchTerm.toLowerCase(); + return ( + item.title?.toLowerCase().includes(searchLower) || + item.repository?.toLowerCase().includes(searchLower) || + item.branch?.toLowerCase().includes(searchLower) || + item.agent_id?.toLowerCase().includes(searchLower) || + item.status?.toLowerCase().includes(searchLower) + ); + }; - const pinnedItemList = allItems.filter((item) => { - return pinnedChats.has(item.id); - }); + const pinnedItemList = allItems.filter( + (item) => pinnedChats.has(item.id) && matchesSearch(item) + ); - const nonPinnedItems = allItems.filter((item) => { - if (pinnedChats.has(item.id)) return false; - if (!debouncedSearchTerm) return true; - const searchLower = debouncedSearchTerm.toLowerCase(); - return ( - item.title?.toLowerCase().includes(searchLower) || - item.repository?.toLowerCase().includes(searchLower) || - item.branch?.toLowerCase().includes(searchLower) || - item.agent_id?.toLowerCase().includes(searchLower) || - item.status?.toLowerCase().includes(searchLower) - ); - }); + const nonPinnedItems = allItems.filter( + (item) => !pinnedChats.has(item.id) && matchesSearch(item) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 210 - 227, The pinned items bypass the search because pinnedItemList is built from allItems without applying the debouncedSearchTerm predicate; update the logic so both pinnedItemList and nonPinnedItems use the same search/filter predicate: extract the predicate (uses debouncedSearchTerm, lowercasing and checks against item.title, item.repository, item.branch, item.agent_id, item.status) and then compute pinnedItemList = allItems.filter(item => pinnedChats.has(item.id) && predicate(item)) and nonPinnedItems = allItems.filter(item => !pinnedChats.has(item.id) && predicate(item)); ensure you reuse the same predicate function/closure to avoid duplication and keep behavior identical for both lists.
587-590:⚠️ Potential issue | 🟡 MinorBuild rows still never become active.
isActiveonly evaluates chat routes, so clicked recipe/build rows don’t keep active styling. Derive active state for recipes from the current route using the same redirect resolution used byhandleChatClick.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 587 - 590, filtered rows never get the active styling because isActive only checks chat routes; update the active check in the filteredItems.map block so it also marks recipe/build rows active by resolving the current route the same way handleChatClick does (extract the redirect resolution logic from handleChatClick into a small helper or call the same resolver) and compare the resolved route id to item.id; keep existing checks for pinnedChats, hoveredChatId and use isActiveChat for pure 'chat' items but for items with type 'recipe'/'build' derive their active state from the route resolver used by handleChatClick.
647-716:⚠️ Potential issue | 🟠 MajorRecipe menu still exposes chat-only actions.
Share/Rename/Delete are rendered for recipe items, but those flows call chat-specific URLs/APIs (
ChatService,/chat/{id}). With currentRecipeServicecapabilities, these actions are broken for recipes. Hide them for recipes (or add recipe-specific handlers first).Suggested fix (hide unsupported recipe actions for now)
-<> - <DropdownMenuItem onClick={(e) => openShareDialog(item, e)} ...>...</DropdownMenuItem> - <DropdownMenuItem onClick={(e) => openRenameDialog(item, e)} ...>...</DropdownMenuItem> +<> + {item.type === "chat" && ( + <> + <DropdownMenuItem onClick={(e) => openShareDialog(item, e)} ...>...</DropdownMenuItem> + <DropdownMenuItem onClick={(e) => openRenameDialog(item, e)} ...>...</DropdownMenuItem> + </> + )} <DropdownMenuItem onClick={(e) => handlePinChat(item.id, e)} ...>...</DropdownMenuItem> - <DropdownMenuSeparator className="bg-[`#E6E8E9`]" /> - <DropdownMenuItem onClick={(e) => openDeleteDialog(item, e)} ...> - <span>Delete {item.type === 'recipe' ? 'Build' : 'Chat'}</span> - </DropdownMenuItem> + {item.type === "chat" && ( + <> + <DropdownMenuSeparator className="bg-[`#E6E8E9`]" /> + <DropdownMenuItem onClick={(e) => openDeleteDialog(item, e)} ...> + <span>Delete Chat</span> + </DropdownMenuItem> + </> + )} </>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 647 - 716, The recipe items are showing chat-only actions; update the JSX in ChatHistoryPanel (the dropdown rendering around DropdownMenuItem elements) to hide Share, Rename and Delete for items where item.type === 'recipe' by conditionally rendering those DropdownMenuItem blocks only when item.type !== 'recipe' (leave handlePinChat and pin/unpin UI as-is), and ensure the Delete label logic (span text using item.type === 'recipe' ? 'Build' : 'Chat') is only used if the delete action is rendered; this keeps openShareDialog, openRenameDialog and openDeleteDialog unchanged but prevents calling chat-specific handlers for recipes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 157-161: The query for recipes is global and runs without auth,
causing cache leakage across accounts; update the useQuery in ChatHistoryPanel
to scope the queryKey to the authenticated user (e.g., ["sidebar-recipes",
currentUser?.id or currentUser?.email]) and add an enabled flag (e.g., enabled:
!!currentUser) so the query is disabled until auth is present; keep the same
queryFn (RecipeService.getAllRecipes) but reference currentUser in the key and
enabled option to prevent reuse between users.
- Around line 210-227: The pinned items bypass the search because pinnedItemList
is built from allItems without applying the debouncedSearchTerm predicate;
update the logic so both pinnedItemList and nonPinnedItems use the same
search/filter predicate: extract the predicate (uses debouncedSearchTerm,
lowercasing and checks against item.title, item.repository, item.branch,
item.agent_id, item.status) and then compute pinnedItemList =
allItems.filter(item => pinnedChats.has(item.id) && predicate(item)) and
nonPinnedItems = allItems.filter(item => !pinnedChats.has(item.id) &&
predicate(item)); ensure you reuse the same predicate function/closure to avoid
duplication and keep behavior identical for both lists.
- Around line 587-590: filtered rows never get the active styling because
isActive only checks chat routes; update the active check in the
filteredItems.map block so it also marks recipe/build rows active by resolving
the current route the same way handleChatClick does (extract the redirect
resolution logic from handleChatClick into a small helper or call the same
resolver) and compare the resolved route id to item.id; keep existing checks for
pinnedChats, hoveredChatId and use isActiveChat for pure 'chat' items but for
items with type 'recipe'/'build' derive their active state from the route
resolver used by handleChatClick.
- Around line 647-716: The recipe items are showing chat-only actions; update
the JSX in ChatHistoryPanel (the dropdown rendering around DropdownMenuItem
elements) to hide Share, Rename and Delete for items where item.type ===
'recipe' by conditionally rendering those DropdownMenuItem blocks only when
item.type !== 'recipe' (leave handlePinChat and pin/unpin UI as-is), and ensure
the Delete label logic (span text using item.type === 'recipe' ? 'Build' :
'Chat') is only used if the delete action is rendered; this keeps
openShareDialog, openRenameDialog and openDeleteDialog unchanged but prevents
calling chat-specific handlers for recipes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed30541e-2ad6-45db-b8d6-6505c045c714
⛔ Files ignored due to path filters (1)
public/images/pined_chat.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
components/Layouts/ChatHistoryPanel.tsx
|
Cloud Run service deployed: https://pr-fix-show-build-cj6r7x3fpa-uc.a.run.app |
|
Cloud Run service deployed: https://pr-fix-show-build-cj6r7x3fpa-uc.a.run.app |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
components/Layouts/ChatHistoryPanel.tsx (4)
210-227:⚠️ Potential issue | 🟡 MinorApply the search predicate to pinned items too.
Line [210] builds
pinnedItemListbefore text filtering, so pinned results bypass search and can block the empty-search state.Suggested fix
+ const matchesSearch = (item: Chat) => { + if (!debouncedSearchTerm) return true; + const searchLower = debouncedSearchTerm.toLowerCase(); + return ( + item.title?.toLowerCase().includes(searchLower) || + item.repository?.toLowerCase().includes(searchLower) || + item.branch?.toLowerCase().includes(searchLower) || + item.agent_id?.toLowerCase().includes(searchLower) || + item.status?.toLowerCase().includes(searchLower) + ); + }; + - const pinnedItemList = allItems.filter((item) => { - return pinnedChats.has(item.id); - }); + const pinnedItemList = allItems.filter( + (item) => pinnedChats.has(item.id) && matchesSearch(item) + ); - const nonPinnedItems = allItems.filter((item) => { - if (pinnedChats.has(item.id)) return false; - - if (!debouncedSearchTerm) return true; - const searchLower = debouncedSearchTerm.toLowerCase(); - return ( - item.title?.toLowerCase().includes(searchLower) || - item.repository?.toLowerCase().includes(searchLower) || - item.branch?.toLowerCase().includes(searchLower) || - item.agent_id?.toLowerCase().includes(searchLower) || - item.status?.toLowerCase().includes(searchLower) - ); - }); + const nonPinnedItems = allItems.filter( + (item) => !pinnedChats.has(item.id) && matchesSearch(item) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 210 - 227, pinnedItemList is built from allItems without applying the search predicate so pinned items bypass debouncedSearchTerm filtering; update the logic that creates pinnedItemList to apply the same search predicate used for nonPinnedItems (reuse the same checks against debouncedSearchTerm, searchLower, and fields item.title, item.repository, item.branch, item.agent_id, item.status) or extract the predicate into a helper and call it for both pinnedItemList and nonPinnedItems, ensuring pinnedChats membership is still required for pinnedItemList.
157-161:⚠️ Potential issue | 🟠 MajorScope recipe cache by user identity and auth state.
Line [158] uses a global React Query key and no auth gating, so cached builds can be reused across account switches in one client session.
Suggested fix
- const { data: recipes, isLoading: recipesLoading } = useQuery<any[]>({ - queryKey: ["sidebar-recipes"], + const { data: recipes, isLoading: recipesLoading } = useQuery<any[]>({ + queryKey: ["sidebar-recipes", user?.uid], queryFn: () => RecipeService.getAllRecipes(0, 100), + enabled: !!user?.uid, staleTime: 60000, // 1 minute });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 157 - 161, The query for recipes uses a static queryKey and no auth gating which allows cached data to leak across account switches; update the useQuery call (the useQuery invocation referring to queryKey and queryFn -> RecipeService.getAllRecipes) to include the current user identity/auth state in the queryKey (e.g., user.id or session token) and add an enabled flag (enabled: !!user?.id || isAuthenticated) so the query only runs when authenticated; this scopes cache entries per user and prevents cross-account reuse.
652-721:⚠️ Potential issue | 🟠 MajorRecipe actions are wired to chat-only handlers.
Lines [654], [667], and [709] expose Share/Rename/Delete for recipes, but those paths call
ChatServiceflows (shareConversation,renameChat,deleteChat).services/RecipeService.tscurrently exposes read methods only, so these actions are broken for builds.Suggested safe interim fix (hide unsupported recipe actions)
- <> - <DropdownMenuItem - onClick={(e) => openShareDialog(item, e)} - className="focus:bg-[`#F4F4F4`] cursor-pointer" - > - ... - <span>Share</span> - </DropdownMenuItem> - <DropdownMenuItem - onClick={(e) => openRenameDialog(item, e)} - className="focus:bg-[`#F4F4F4`] cursor-pointer" - > - ... - <span>Rename</span> - </DropdownMenuItem> + <> + {item.type === "chat" && ( + <> + <DropdownMenuItem + onClick={(e) => openShareDialog(item, e)} + className="focus:bg-[`#F4F4F4`] cursor-pointer" + > + ... + <span>Share</span> + </DropdownMenuItem> + <DropdownMenuItem + onClick={(e) => openRenameDialog(item, e)} + className="focus:bg-[`#F4F4F4`] cursor-pointer" + > + ... + <span>Rename</span> + </DropdownMenuItem> + </> + )} <DropdownMenuItem onClick={(e) => handlePinChat(item.id, e)} className="focus:bg-[`#F4F4F4`] cursor-pointer" > ... </DropdownMenuItem> - <DropdownMenuSeparator className="bg-[`#E6E8E9`]" /> - <DropdownMenuItem - onClick={(e) => openDeleteDialog(item, e)} - className="text-red-600 focus:text-red-600 focus:bg-[`#F4F4F4`] cursor-pointer" - > - ... - <span>Delete {item.type === 'recipe' ? 'Build' : 'Chat'}</span> - </DropdownMenuItem> + {item.type === "chat" && ( + <> + <DropdownMenuSeparator className="bg-[`#E6E8E9`]" /> + <DropdownMenuItem + onClick={(e) => openDeleteDialog(item, e)} + className="text-red-600 focus:text-red-600 focus:bg-[`#F4F4F4`] cursor-pointer" + > + ... + <span>Delete Chat</span> + </DropdownMenuItem> + </> + )} </>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 652 - 721, The Share/Rename/Delete handlers (openShareDialog, openRenameDialog, openDeleteDialog) and the DropdownMenuItem entries are wired to chat flows (e.g., ChatService.shareConversation, ChatService.renameChat, ChatService.deleteChat) but recipes/builds are read-only via services/RecipeService.ts; update the UI to only show Share, Rename and Delete for non-recipe items (e.g., conditionally render those DropdownMenuItem blocks when item.type !== 'recipe') or alternatively implement corresponding methods in RecipeService and wire them instead; the safe interim fix is to hide these three actions for recipes by gating the DropdownMenuItem render logic on item.type.
589-590:⚠️ Potential issue | 🟡 MinorBuild rows never become active.
Line [589] marks active rows only for chats, so clicked recipe/build rows never keep active styling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 589 - 590, The active-row check currently only marks items with item.type === 'chat' using isActiveChat, so recipe/build rows never get active styling; change the isActive assignment to not restrict by type (e.g. replace "const isActive = item.type === 'chat' && isActiveChat(item.id);" with a plain "const isActive = isActiveChat(item.id);" or, if you have a generic helper, use it (e.g. isActiveItem(item.id) or isActiveById) and update/implement that helper so it returns true for recipe/build IDs as appropriate; adjust any related logic in ChatHistoryPanel.tsx that relies on item.type if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 538-539: The panel header text "Your chats" in the
ChatHistoryPanel component should reflect that it lists both chats and builds;
update the two occurrences of the literal heading string "Your chats" (rendered
in the ChatHistoryPanel JSX/return) to a clearer label such as "Your chats and
builds" (or "Your chats & builds") so both header instances display the updated
copy.
---
Duplicate comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 210-227: pinnedItemList is built from allItems without applying
the search predicate so pinned items bypass debouncedSearchTerm filtering;
update the logic that creates pinnedItemList to apply the same search predicate
used for nonPinnedItems (reuse the same checks against debouncedSearchTerm,
searchLower, and fields item.title, item.repository, item.branch, item.agent_id,
item.status) or extract the predicate into a helper and call it for both
pinnedItemList and nonPinnedItems, ensuring pinnedChats membership is still
required for pinnedItemList.
- Around line 157-161: The query for recipes uses a static queryKey and no auth
gating which allows cached data to leak across account switches; update the
useQuery call (the useQuery invocation referring to queryKey and queryFn ->
RecipeService.getAllRecipes) to include the current user identity/auth state in
the queryKey (e.g., user.id or session token) and add an enabled flag (enabled:
!!user?.id || isAuthenticated) so the query only runs when authenticated; this
scopes cache entries per user and prevents cross-account reuse.
- Around line 652-721: The Share/Rename/Delete handlers (openShareDialog,
openRenameDialog, openDeleteDialog) and the DropdownMenuItem entries are wired
to chat flows (e.g., ChatService.shareConversation, ChatService.renameChat,
ChatService.deleteChat) but recipes/builds are read-only via
services/RecipeService.ts; update the UI to only show Share, Rename and Delete
for non-recipe items (e.g., conditionally render those DropdownMenuItem blocks
when item.type !== 'recipe') or alternatively implement corresponding methods in
RecipeService and wire them instead; the safe interim fix is to hide these three
actions for recipes by gating the DropdownMenuItem render logic on item.type.
- Around line 589-590: The active-row check currently only marks items with
item.type === 'chat' using isActiveChat, so recipe/build rows never get active
styling; change the isActive assignment to not restrict by type (e.g. replace
"const isActive = item.type === 'chat' && isActiveChat(item.id);" with a plain
"const isActive = isActiveChat(item.id);" or, if you have a generic helper, use
it (e.g. isActiveItem(item.id) or isActiveById) and update/implement that helper
so it returns true for recipe/build IDs as appropriate; adjust any related logic
in ChatHistoryPanel.tsx that relies on item.type if needed.
🪄 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: 5593776f-f2d5-4450-a40e-4b44b11d7a67
📒 Files selected for processing (1)
components/Layouts/ChatHistoryPanel.tsx
|
Cloud Run service deployed: https://pr-fix-show-build-cj6r7x3fpa-uc.a.run.app |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
components/Layouts/ChatHistoryPanel.tsx (3)
210-227:⚠️ Potential issue | 🟡 MinorPinned items still bypass the search filter.
Pinned rows are currently included regardless of the search term, so search results are inconsistent and the empty state can be masked.
Suggested fix
const filteredItems = useMemo(() => { if (!allItems) return []; - // Get all pinned items - const pinnedItemList = allItems.filter((item) => { - return pinnedChats.has(item.id); - }); + const matchesSearch = (item: Chat) => { + if (!debouncedSearchTerm) return true; + const searchLower = debouncedSearchTerm.toLowerCase(); + return ( + item.title?.toLowerCase().includes(searchLower) || + item.repository?.toLowerCase().includes(searchLower) || + item.branch?.toLowerCase().includes(searchLower) || + item.agent_id?.toLowerCase().includes(searchLower) || + item.status?.toLowerCase().includes(searchLower) + ); + }; + + const pinnedItemList = allItems.filter( + (item) => pinnedChats.has(item.id) && matchesSearch(item) + ); - // Get filtered non-pinned items - const nonPinnedItems = allItems.filter((item) => { - if (pinnedChats.has(item.id)) return false; - - if (!debouncedSearchTerm) return true; - const searchLower = debouncedSearchTerm.toLowerCase(); - return ( - item.title?.toLowerCase().includes(searchLower) || - item.repository?.toLowerCase().includes(searchLower) || - item.branch?.toLowerCase().includes(searchLower) || - item.agent_id?.toLowerCase().includes(searchLower) || - item.status?.toLowerCase().includes(searchLower) - ); - }); + const nonPinnedItems = allItems.filter( + (item) => !pinnedChats.has(item.id) && matchesSearch(item) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 210 - 227, Pinned items are currently included unfiltered because pinnedItemList is created from allItems without applying debouncedSearchTerm; update the logic so pinnedItemList uses the same search predicate as nonPinnedItems (or derive it by filtering nonPinnedItems + pinned flag) — i.e., apply the debouncedSearchTerm check (using searchLower against item.title, item.repository, item.branch, item.agent_id, item.status) when building pinnedItemList or compute a single filtered list from allItems and then split into pinned (pinnedChats.has(item.id)) and non-pinned groups to ensure pinned rows respect the search filter.
626-629:⚠️ Potential issue | 🟡 MinorBuild active-state check misses repo query-param routes.
For repo pages, matching
item.idagainstpathnamewon’t work because query params are not part of the pathname, so active styling is lost.In Next.js App Router, does `usePathname()` include URL query parameters, or only the path segment?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 626 - 629, The active-state check in the ChatHistoryPanel uses pathname.includes(item.id) for repo items but pathname (from usePathname) does not include URL query parameters, so repo routes with id in search params won't match; update the logic in the isActive computation (the ternary that uses isActiveChat and pathname checks) to also read the query param (e.g. via useSearchParams or extracting URLSearchParams) and compare the repo id query value to item.id, so repo pages set isActive when either the pathname contains the id or the relevant search param equals item.id.
574-575:⚠️ Potential issue | 🟡 MinorUI copy still says “chats” while the list includes builds too.
The title/placeholder should match the mixed list to avoid user confusion.
Suggested fix
- Your chats + Your chats & builds ... - placeholder="Search chats.." + placeholder="Search chats or builds..."Also applies to: 595-595
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 574 - 575, Update the UI copy in the ChatHistoryPanel component so the header and any related placeholder no longer read "Your chats" (which is inaccurate) and instead reflect the mixed list of "chats and builds" (or a neutral label like "Your activity" if preferred); locate the JSX in the ChatHistoryPanel component where the literal "Your chats" string is rendered (and the other occurrence referenced in the review) and replace both occurrences with the new label so the title and placeholder match the mixed list content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 487-501: The link-share flow can call the publish API twice
because handleShare() first publishes (via RecipeService.shareRecipe or
ChatService.shareConversation) and then handleCopyLink() may also trigger a
publish while pending changes are true; update the flow so handleCopyLink() does
not re-publish if a share just occurred or if pending changes are true: ensure
handleShare awaits the share call to completion, set a local/shared "isShared"
or "shareInProgress" flag when calling RecipeService.shareRecipe /
ChatService.shareConversation, and have handleCopyLink check that flag or the
shared state before invoking any publish logic (or skip publish entirely and
only copy the existing link); also clear the flag after completion and handle
error paths so duplicate API writes are prevented.
---
Duplicate comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 210-227: Pinned items are currently included unfiltered because
pinnedItemList is created from allItems without applying debouncedSearchTerm;
update the logic so pinnedItemList uses the same search predicate as
nonPinnedItems (or derive it by filtering nonPinnedItems + pinned flag) — i.e.,
apply the debouncedSearchTerm check (using searchLower against item.title,
item.repository, item.branch, item.agent_id, item.status) when building
pinnedItemList or compute a single filtered list from allItems and then split
into pinned (pinnedChats.has(item.id)) and non-pinned groups to ensure pinned
rows respect the search filter.
- Around line 626-629: The active-state check in the ChatHistoryPanel uses
pathname.includes(item.id) for repo items but pathname (from usePathname) does
not include URL query parameters, so repo routes with id in search params won't
match; update the logic in the isActive computation (the ternary that uses
isActiveChat and pathname checks) to also read the query param (e.g. via
useSearchParams or extracting URLSearchParams) and compare the repo id query
value to item.id, so repo pages set isActive when either the pathname contains
the id or the relevant search param equals item.id.
- Around line 574-575: Update the UI copy in the ChatHistoryPanel component so
the header and any related placeholder no longer read "Your chats" (which is
inaccurate) and instead reflect the mixed list of "chats and builds" (or a
neutral label like "Your activity" if preferred); locate the JSX in the
ChatHistoryPanel component where the literal "Your chats" string is rendered
(and the other occurrence referenced in the review) and replace both occurrences
with the new label so the title and placeholder match the mixed list content.
🪄 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: 8fa072e1-4bf0-47e2-80f3-7c07f5113931
📒 Files selected for processing (2)
components/Layouts/ChatHistoryPanel.tsxservices/RecipeService.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
components/Layouts/ChatHistoryPanel.tsx (4)
211-227:⚠️ Potential issue | 🟡 MinorPinned items still bypass search filtering.
Line 211 builds
pinnedItemListwithout applying the search predicate, so pinned rows can remain visible while searching unrelated text.Suggested fix
+ const matchesSearch = (item: Chat) => { + if (!debouncedSearchTerm) return true; + const searchLower = debouncedSearchTerm.toLowerCase(); + return ( + item.title?.toLowerCase().includes(searchLower) || + item.repository?.toLowerCase().includes(searchLower) || + item.branch?.toLowerCase().includes(searchLower) || + item.agent_id?.toLowerCase().includes(searchLower) || + item.status?.toLowerCase().includes(searchLower) + ); + }; + - const pinnedItemList = allItems.filter((item) => { - return pinnedChats.has(item.id); - }); + const pinnedItemList = allItems.filter( + (item) => pinnedChats.has(item.id) && matchesSearch(item) + ); - const nonPinnedItems = allItems.filter((item) => { - if (pinnedChats.has(item.id)) return false; - - if (!debouncedSearchTerm) return true; - const searchLower = debouncedSearchTerm.toLowerCase(); - return ( - item.title?.toLowerCase().includes(searchLower) || - item.repository?.toLowerCase().includes(searchLower) || - item.branch?.toLowerCase().includes(searchLower) || - item.agent_id?.toLowerCase().includes(searchLower) || - item.status?.toLowerCase().includes(searchLower) - ); - }); + const nonPinnedItems = allItems.filter( + (item) => !pinnedChats.has(item.id) && matchesSearch(item) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 211 - 227, pinnedItemList is built from allItems without applying the search predicate, so pinned rows bypass search; update the logic that constructs pinnedItemList to apply the same filtering used for nonPinnedItems (i.e., check debouncedSearchTerm and match title, repository, branch, agent_id, or status) or extract the search predicate into a shared function and reuse it for both pinnedItemList and nonPinnedItems; reference the variables/pieces to change: pinnedItemList, nonPinnedItems, pinnedChats, debouncedSearchTerm, and the search checks on item.title/repository/branch/agent_id/status.
627-630:⚠️ Potential issue | 🟡 MinorRecipe active-state check misses
/repo?recipeId=...routes.
pathnamedoes not include query params, so Line 629 cannot matchitem.idon repo routes. Build rows can lose active styling after navigation.Suggested fix
-import { useRouter, usePathname } from "next/navigation"; +import { useRouter, usePathname, useSearchParams } from "next/navigation"; ... + const searchParams = useSearchParams(); ... - const isActive = item.type === 'chat' - ? isActiveChat(item.id) - : pathname?.includes(`/task/${item.id}`) || pathname?.includes('repo') && pathname?.includes(item.id); + const isActive = + item.type === "chat" + ? isActiveChat(item.id) + : pathname?.startsWith(`/task/${item.id}`) || + (pathname === "/repo" && searchParams.get("recipeId") === item.id);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 627 - 630, The active-state logic for recipe rows fails to catch `/repo?recipeId=...` because pathname excludes query params; update the ternary that computes isActive (around variable isActive using isActiveChat(item.id) and pathname checks) to also inspect the current location.search (e.g., new URLSearchParams(search)) and treat the row as active when either the pathname includes `/task/${item.id}`, or the pathname includes 'repo' and the query param recipeId equals item.id; reference the variables pathname, search (or window.location.search), isActiveChat, and item.id when implementing this change so build rows correctly retain active styling.
486-502:⚠️ Potential issue | 🟠 MajorShare-with-link path can still publish twice.
Line 500 updates state, then Line 501 immediately calls
handleCopyLink(). Because state updates are async,handleCopyLinkcan still seehasPendingChanges === trueand re-call publish.Suggested fix
- setHasPendingChanges(false); - handleCopyLink(); + setHasPendingChanges(false); + const shareUrl = + selectedChat.type === "recipe" + ? `${process.env.NEXT_PUBLIC_APP_URL}/task/${selectedChat.id}` + : `${process.env.NEXT_PUBLIC_APP_URL}/chat/${selectedChat.id}`; + await navigator.clipboard.writeText(shareUrl); + toast.success("URL copied to clipboard");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` around lines 486 - 502, The share-with-link branch can trigger a second publish because setHasPendingChanges(false) is async and handleCopyLink reads hasPendingChanges; fix by making handleCopyLink accept an explicit flag to skip publishing (e.g., handleCopyLink(skipPublish: boolean = false)) and, in this branch after successfully calling RecipeService.shareRecipe or ChatService.shareConversation and setting setHasPendingChanges(false), call handleCopyLink(true) so handleCopyLink will not re-publish; update other callers to use the new optional parameter or default behavior remains unchanged.
575-575:⚠️ Potential issue | 🟡 MinorList/search copy is still chats-only while list includes builds.
Line 575 and Line 596 should reflect mixed chat/build content for consistency with the merged data model.
Also applies to: 596-596
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/ChatHistoryPanel.tsx` at line 575, The UI copy in ChatHistoryPanel still says "Your chats" and a chats-only search label even though the list contains both chats and builds; update the two text nodes in the ChatHistoryPanel component (the heading string "Your chats" and the search input/placeholder text around line 596) to reflect mixed content (e.g., "Your chats & builds" and "Search chats and builds..." or similar consistent wording) so the heading and search copy match the merged data model.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@components/Layouts/ChatHistoryPanel.tsx`:
- Around line 211-227: pinnedItemList is built from allItems without applying
the search predicate, so pinned rows bypass search; update the logic that
constructs pinnedItemList to apply the same filtering used for nonPinnedItems
(i.e., check debouncedSearchTerm and match title, repository, branch, agent_id,
or status) or extract the search predicate into a shared function and reuse it
for both pinnedItemList and nonPinnedItems; reference the variables/pieces to
change: pinnedItemList, nonPinnedItems, pinnedChats, debouncedSearchTerm, and
the search checks on item.title/repository/branch/agent_id/status.
- Around line 627-630: The active-state logic for recipe rows fails to catch
`/repo?recipeId=...` because pathname excludes query params; update the ternary
that computes isActive (around variable isActive using isActiveChat(item.id) and
pathname checks) to also inspect the current location.search (e.g., new
URLSearchParams(search)) and treat the row as active when either the pathname
includes `/task/${item.id}`, or the pathname includes 'repo' and the query param
recipeId equals item.id; reference the variables pathname, search (or
window.location.search), isActiveChat, and item.id when implementing this change
so build rows correctly retain active styling.
- Around line 486-502: The share-with-link branch can trigger a second publish
because setHasPendingChanges(false) is async and handleCopyLink reads
hasPendingChanges; fix by making handleCopyLink accept an explicit flag to skip
publishing (e.g., handleCopyLink(skipPublish: boolean = false)) and, in this
branch after successfully calling RecipeService.shareRecipe or
ChatService.shareConversation and setting setHasPendingChanges(false), call
handleCopyLink(true) so handleCopyLink will not re-publish; update other callers
to use the new optional parameter or default behavior remains unchanged.
- Line 575: The UI copy in ChatHistoryPanel still says "Your chats" and a
chats-only search label even though the list contains both chats and builds;
update the two text nodes in the ChatHistoryPanel component (the heading string
"Your chats" and the search input/placeholder text around line 596) to reflect
mixed content (e.g., "Your chats & builds" and "Search chats and builds..." or
similar consistent wording) so the heading and search copy match the merged data
model.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9191e025-7531-4945-b14c-9a3719d61d06
📒 Files selected for processing (1)
components/Layouts/ChatHistoryPanel.tsx
|
Cloud Run service deployed: https://pr-fix-show-build-cj6r7x3fpa-uc.a.run.app |
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 by CodeRabbit
New Features
Style