feat(chat): session-scoped sidebar rename + archive-on-delete#1758
feat(chat): session-scoped sidebar rename + archive-on-delete#1758arpitgupta1214 wants to merge 3 commits into
Conversation
Both mutations were still hitting the legacy rooms-shaped endpoints
(DELETE /api/chats with body { id }, PATCH /api/chats with { chatId,
topic }), which only touch the rooms table — workflow chats stayed
intact in chats / chat_messages / sessions. Repoint both at the
session-scoped routes so deletes cascade through chat_messages /
chat_reads, and renames update the canonical title field.
- deleteChat takes (sessionId, chatId, accessToken) and calls
DELETE /api/sessions/{sid}/chats/{cid}.
- updateChat takes (sessionId, chatId, title) and calls
PATCH /api/sessions/{sid}/chats/{cid} with { title }.
- useDeleteChat and DeleteConfirmationModal pass through sessionId
from the Conversation row.
- useRenameModal passes sessionId + title (was topic) through to
updateChat.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
No issues found across 5 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: This PR modifies API endpoints and payloads for chat deletion and renaming, moving from legacy rooms-shaped routes to session-scoped routes; such changes carry moderate risk of breakage if the new endpoints are not yet available or behave differently, and they depend on the sessionId field from...
Re-trigger cubic
…o feat/sidebar-delete-rename-session-scoped
Switch the sidebar's delete action to soft-delete via session archive
instead of removing the chat row. The api side filters chats whose
session is `archived` out of `GET /api/chats` (api#630), so the row
disappears from the sidebar; archive also triggers the existing
`stopSandboxOnArchive` lifecycle path on the api so we stop running
sandboxes for chats the user told us they're done with.
This also unblocks deleting the last chat in a session, which the
api previously refused with `400 Cannot delete the only chat in a
session` — archive has no such constraint.
- New `lib/sessions/archiveSession.ts` → `PATCH /api/sessions/{sid}
{ status: "archived" }`.
- `useDeleteChat` calls it (drops the `chatId` arg).
- `DeleteConfirmationModal` no longer passes `chatId`.
- Remove the now-unused `lib/chats/deleteChat.ts`.
Caveat for reviewers: if a session ever has >1 chat, archiving from
one row will hide the others too. Today every sidebar row corresponds
1:1 with its own session (each `POST /api/sessions` mints a session +
single chat), so this is a no-op in practice — but worth knowing
before we add a UI that creates additional chats inside an existing
session.
Depends on api#630 for the filter; until that lands, archived sessions
will still show up in the chat list.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useDeleteChat.ts">
<violation number="1" location="hooks/useDeleteChat.ts:29">
P1: Delete action now archives whole session, not target chat. Wrong endpoint and payload for chat delete. Restore chat-scoped delete call with `chatId`.</violation>
</file>
<file name="components/Sidebar/Modals/DeleteConfirmationModal.tsx">
<violation number="1" location="components/Sidebar/Modals/DeleteConfirmationModal.tsx:68">
P1: Missing chatId in delete call. Session-only delete cannot target one chat. Pass chatId through this flow.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ); | ||
| } | ||
| return deleteChat(roomId, accessToken); | ||
| return archiveSession(sessionId, accessToken); |
There was a problem hiding this comment.
P1: Delete action now archives whole session, not target chat. Wrong endpoint and payload for chat delete. Restore chat-scoped delete call with chatId.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useDeleteChat.ts, line 29:
<comment>Delete action now archives whole session, not target chat. Wrong endpoint and payload for chat delete. Restore chat-scoped delete call with `chatId`.</comment>
<file context>
@@ -1,27 +1,32 @@
);
}
- return deleteChat(sessionId, chatId, accessToken);
+ return archiveSession(sessionId, accessToken);
},
});
</file context>
|
|
||
| try { | ||
| await deleteChat(chat.id); | ||
| await deleteChat({ sessionId: chat.sessionId }); |
There was a problem hiding this comment.
P1: Missing chatId in delete call. Session-only delete cannot target one chat. Pass chatId through this flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/Sidebar/Modals/DeleteConfirmationModal.tsx, line 68:
<comment>Missing chatId in delete call. Session-only delete cannot target one chat. Pass chatId through this flow.</comment>
<file context>
@@ -52,15 +65,18 @@ const DeleteConfirmationModal = ({ isOpen, onClose, chatRoom, chatRooms, onDelet
try {
- await deleteChat({ sessionId: chat.sessionId, chatId: chat.id });
+ await deleteChat({ sessionId: chat.sessionId });
} catch (chatError) {
- console.error(`Error deleting chat ${chat.topic || "Chat"}:`, chatError);
</file context>
E2E re-test after consolidating archive changeTested the chat preview pointed at the api#630 preview so the full archive → list-filter path is exercised end-to-end. The pointing is done client-side via sessionStorage.setItem(
'recoup_api_override',
'https://api-git-feat-exclude-archived-from-chats-list-recoup.vercel.app'
);Setup: Taylor Swift with 3 chats, all in single-chat sessions (worst-case for the old delete guard).
All four PR test-plan bullets pass. Not exercised separately
|
Stacked on chat#1756. Two related changes to the sidebar's chat actions:
Rename — session-scoped PATCH
The rename flow was still hitting the legacy
PATCH /api/chatsshape ({ chatId, topic }), which only touched theroomstable. Repoint atPATCH /api/sessions/{sid}/chats/{cid}with{ title }so the canonical chat row is updated.updateChat({ sessionId, chatId, title })→PATCH /api/sessions/{sid}/chats/{cid}.useRenameModalsendstitle(wastopic).Delete — archive the owning session
Instead of removing the chat row, the "Delete chat" action now archives the owning session via
PATCH /api/sessions/{sid} { status: "archived" }. The api filters chats from archived sessions out ofGET /api/chats(api#630), so the row disappears from the sidebar; archive also triggersstopSandboxOnArchiveso we tear down running sandboxes for chats the user said they're done with. The whole thing is reversible from the admin side.This also sidesteps the
400 Cannot delete the only chat in a sessionguard that blocks chat-row deletes when a session has a single chat — archive has no such constraint.lib/sessions/archiveSession.ts→PATCH /api/sessions/{sid} { status: "archived" }.useDeleteChatcalls it (drops thechatIdarg).DeleteConfirmationModalno longer passeschatId.Caveat
If a session ever has >1 chat, archiving from one row hides the others too. Today every sidebar row maps 1:1 to its own session (
POST /api/sessionsmints a session + single chat), so this is a no-op in practice — but worth knowing before we add a UI that creates additional chats inside an existing session.Dependencies
GET /api/chats) live; until that lands, archived sessions still appear in the list.testonce chat#1756 merges.Test plan
GET /api/chatsthattitleupdated server-side.status: "archived"viaGET /api/sessions/{sid}.