feat(threads): dedicated threads controller with per-thread session scoping#590
Merged
senamakel merged 12 commits intotinyhumansai:mainfrom Apr 16, 2026
Merged
feat(threads): dedicated threads controller with per-thread session scoping#590senamakel merged 12 commits intotinyhumansai:mainfrom
senamakel merged 12 commits intotinyhumansai:mainfrom
Conversation
…reation and deletion - Added functionality to create new conversation threads with unique IDs and titles based on the current date and time. - Introduced a deleteThread action to remove existing threads, updating the selected thread accordingly. - Enhanced the UI to display threads in a sidebar, allowing users to select and delete threads easily. - Updated the Conversations component to handle thread loading and selection more efficiently, ensuring a smoother user experience. Also updated the OpenHuman package version to 0.52.15 in Cargo.lock files.
…and streamline thread creation logic - Updated the function name from `createThreadLocal` to `createNewThread` for clarity and consistency. - Simplified the thread creation process by removing the manual ID and title generation, leveraging the new API method for automatic thread creation. - Adjusted the Conversations component to utilize the new `handleCreateNewThread` function, enhancing readability and maintainability. - Removed unused thread ID and title generation functions to clean up the codebase. This refactor improves the overall structure and clarity of the thread management functionality.
…nctions - Eliminated unused functions related to listing, creating, updating, appending, and deleting conversation threads in memory operations. - This cleanup enhances code maintainability and reduces complexity in the memory module. - The refactor focuses on streamlining the conversation management logic, aligning with recent changes in the thread handling API.
…unctions - Renamed thread-related API methods to align with the new naming convention, improving clarity and consistency. - Removed deprecated functions related to thread creation, streamlining the API and enhancing maintainability. - Adjusted the implementation of existing methods to reflect the updated API structure, ensuring proper functionality.
…properties - Streamlined the thread state by removing the lastViewedAt property and related logic, enhancing clarity in unread message counting. - Updated the BottomTabBar component to reflect the new unread count logic, which now simply returns the length of threads. - Removed the setLastViewed action from the Conversations component, further simplifying the thread management logic. - Introduced a new deleteThread action to handle thread deletion, ensuring proper state updates and selection handling. - Overall, these changes improve maintainability and reduce complexity in the thread management system.
Contributor
📝 WalkthroughWalkthroughThis PR moves conversation thread/message RPCs from a legacy Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,230,255,0.5)
participant Client
participant Frontend
end
rect rgba(200,255,200,0.5)
participant API_Server
end
rect rgba(255,230,200,0.5)
participant Storage
participant WebChannel
end
Client->>Frontend: user selects/creates/deletes thread or sends message
Frontend->>API_Server: call `threads:*` RPC (list/create_new/messages_list/message_append/delete)
API_Server->>Storage: read/write thread & message records
API_Server->>WebChannel: on delete -> invalidate_thread_sessions(thread_id)
WebChannel->>Storage: evict session cache entries (key ends with ::thread_id)
API_Server-->>Frontend: RPC response (thread/message data)
Frontend-->>Client: update UI (sidebar, messages, selection)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Resolves conflicts in threadSlice.ts and Conversations.tsx: - Drop obsolete subscribeChatEvents useEffect — now handled by ChatRuntimeProvider from upstream (tinyhumansai#587). - Keep /new and /clear slash command handling from feat/threads. - Use composerBlocked guard (upstream) in handleSendMessage.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/store/threadSlice.ts (1)
51-215: 🛠️ Refactor suggestion | 🟠 MajorAdd namespaced debug logs around the new thread thunks.
The new create/delete/load/suggest flows ship without any
[threads]/[ui-flow]diagnostics, which makes thread-selection and RPC desyncs hard to trace end-to-end. Please add dev-only logs at thunk entry/exit and on failure, includingthreadIdwhere applicable.As per coding guidelines, "Add substantial, development-oriented logs on new/changed flows in TypeScript/React app code; use namespaced debug logs and dev-only detail as needed" and "Use grep-friendly log prefixes ([feature], domain name, or JSON-RPC method) in app code for correlation with sidecar and Tauri output".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/store/threadSlice.ts` around lines 51 - 215, The thunks (loadThreads, createNewThread, deleteThread, loadThreadMessages, addMessageLocal, addInferenceResponse, persistReaction, purgeThreads, fetchSuggestedQuestions) lack namespaced dev-only logs; add debug logs at thunk entry, successful exit (including returned ids/summary) and on failure, using a grep-friendly prefix like "[threads]" or "[ui-flow]"; guard the logs with a dev check (e.g. process.env.NODE_ENV === 'development') or existing isDev helper, and include threadId/messageId where applicable (e.g. deleteThread threadId, loadThreadMessages threadId, addInferenceResponse targetThreadId, persistReaction payload.threadId/messageId) and surface error.message in catch blocks so failures are logged before rejectWithValue is returned.
🧹 Nitpick comments (1)
app/src/pages/Conversations.tsx (1)
709-817: Add app-side debug logs for the new thread bootstrap/sidebar flow.This PR adds the main thread loading, creation, and selection path, but there is no namespaced trace for which branch ran or which thread was selected. That makes first-load and thread-switch issues much harder to correlate with RPC/socket logs.
As per coding guidelines: "Add substantial, development-oriented logs on new/changed flows in TypeScript/React app code; use namespaced debug logs and dev-only detail as needed".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 709 - 817, Add namespaced dev-only debug logs for the Conversations flow: insert console.debug calls (or your project's debug util) with a "Conversations" prefix in the handlers referenced here — inside handleCreateNewThread calls (log when invoked and the created thread id/title), inside the thread selection block where dispatch(setSelectedThread(thread.id)) and dispatch(loadThreadMessages(thread.id)) are called (log the selected thread id/title and that messages are being loaded), and in the sidebar toggle button (log showSidebar state after toggling); include minimal structured details (id, title, messageCount) to help correlate with RPC/socket logs and ensure logs are only emitted in dev builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 733-785: The outer clickable thread row must not be a <button>
containing another <button>; replace the outer <button> (the element rendering
each item in sortedThreads that calls dispatch(setSelectedThread(thread.id)) and
loadThreadMessages(thread.id)) with a non-button container (e.g., a <div> or
<li>) that has role="button", tabIndex={0}, and an onClick handler invoking
those dispatches, and add an onKeyDown that triggers the same behavior for
Enter/Space so keyboard users work. Keep the inner delete control as a real
<button> that calls deleteThread(thread.id), preserve the className/styles
(including selectedThreadId checks and formatRelativeTime(thread.lastMessageAt)
output), and ensure the container still has the same visual/interactive states
(hover/selected styles) so behavior and accessibility are fixed.
- Around line 382-389: The slash-command handler wrongly treats '/clear' like
'/new' and calls handleCreateNewThread; update handleSlashCommand so that cmd
=== '/new' clears input and calls handleCreateNewThread(), while cmd ===
'/clear' only calls setInputValue('') (no thread creation). Locate
handleSlashCommand and modify the branching to separate '/new' and '/clear'
cases, keeping return true for both when handled.
- Around line 225-245: The current bootstrap uses
dispatch(loadThreads()).unwrap() and dispatch(createNewThread()).unwrap()
fire-and-forget which can reject and cause unhandled promise rejections; update
handleCreateNewThread and the useEffect flow to properly await and catch errors:
wrap the await dispatch(createNewThread()).unwrap() in try/catch inside
handleCreateNewThread and surface failures (e.g., show error UI or retry)
instead of dropping the promise, and in the useEffect replace the chained .then
with an async IIFE or promise chain that awaits dispatch(loadThreads()).unwrap()
inside try/catch, handle the empty-threads case by calling the
try/catch-protected handleCreateNewThread, and ensure any calls to
setSelectedThread and loadThreadMessages are only executed after successful
dispatch results so errors are not swallowed.
In `@app/src/store/threadSlice.ts`:
- Around line 64-70: The current createNewThread thunk calls await
dispatch(loadThreads()).unwrap(), which causes the whole thunk to reject if the
refresh fails even when threadApi.createNewThread() succeeded; change it so the
refresh is best-effort: after successfully awaiting threadApi.createNewThread()
(in createNewThread), call dispatch(loadThreads()) without unwrap and wrap that
call in its own try/catch (log the error but do not rethrow) and then return the
created thread; apply the same pattern to the deleteThread thunk (call
threadApi.deleteThread(), then attempt a best-effort dispatch(loadThreads())
inside a try/catch, and finally return the deletion result) so write successes
are not turned into rejected thunks by a refresh failure.
- Around line 83-89: When auto-selecting a replacement thread after deleting the
active one, also dispatch loading its messages so the UI isn't blank; in the
block where you call setSelectedThread(remaining[0].id) (inside the thread
deletion branch that checks state.thread.selectedThreadId), follow that dispatch
with a dispatch(loadThreadMessages(remaining[0].id)) (or the existing action
that fetches/caches messages) so the newly selected thread's messages are
loaded, and keep clearSelectedThread() as-is for the empty case.
In `@src/core/all.rs`:
- Around line 140-141: namespace "threads" was registered via
controllers.extend(crate::openhuman::threads::all_threads_registered_controllers())
but namespace_description() still lacks an entry for "threads", causing help
output to show None; update the namespace_description function to return a
friendly description for "threads" (e.g., Some("Conversation thread and message
management")) by adding the "threads" match arm or map entry alongside the other
namespaces (also ensure you add the same description where namespace_description
is referenced around the similar registration at 187-188).
In `@src/openhuman/channels/providers/web.rs`:
- Around line 254-274: invalidate_thread_sessions currently only removes entries
from THREAD_SESSIONS but leaves any matching in-flight work running, which can
recreate a SessionEntry later; update invalidate_thread_sessions to also abort
or mark matching work in IN_FLIGHT (or a new THREAD_INVALIDATED marker) so tasks
handling entries in IN_FLIGHT either get canceled or detect the thread as
invalid before they reinsert a SessionEntry (refer to THREAD_SESSIONS,
IN_FLIGHT, and the code path that constructs/ reinserts SessionEntry); acquire
the appropriate locks (both THREAD_SESSIONS and IN_FLIGHT or update the shared
invalidation flag) and either remove/cancel IN_FLIGHT entries for the thread or
set a boolean invalidation marker that the reinsert logic checks and bails out
on.
- Around line 851-860: The code truncates thread_id to 12 chars before calling
agent.set_agent_definition_name which causes distinct threads with the same
prefix to alias transcripts; stop truncating and either use the full thread_id
when building the agent_definition_name (e.g.
format!("{target_agent_id}_{thread_id}")) or replace the truncation with a
deterministic collision-resistant hash of thread_id (e.g. SHA-256 hex) and use
that hash (or a long portion of it) in the agent.set_agent_definition_name call
so transcript lookup is unique and deterministic.
In `@src/openhuman/threads/ops.rs`:
- Around line 95-237: Add structured debug logs with stable prefixes ([threads]
and [rpc]) to trace entry/exit, key params, branch decisions, external calls and
error paths for the new threads RPC flow: instrument the handlers threads_list,
thread_upsert, thread_create_new, messages_list, message_append, message_update,
thread_delete, and threads_purge to log on entry (include request id if
available, method name and input IDs like thread_id), before/after
external/storage calls (conversations::list_threads,
conversations::ensure_thread, conversations::get_messages,
conversations::append_message, conversations::update_message,
conversations::ConversationStore::delete_thread, conversations::purge_threads,
web_channel::invalidate_thread_sessions) with outcomes/counts, and on
errors/failures include the error details and context; use consistent structured
messages and fields (e.g., "method", "thread_id", "count", "error") and place
exit/response logs showing results (counts or created/updated summary) so
frontend→sidecar lifecycle can be traced end-to-end.
- Around line 223-236: threads_purge currently removes on-disk threads but
leaves stale in-memory session state in THREAD_SESSIONS; update threads_purge to
also invalidate the in-memory sessions for the purged threads by obtaining the
list of deleted thread IDs from conversations::purge_threads (change its return
to include Vec<ThreadId> or similar) or, if you intend to purge everything, call
the appropriate THREAD_SESSIONS API to clear all entries; ensure you reference
THREAD_SESSIONS in threads_purge and remove each purged thread id (or clear the
entire cache) after calling conversations::purge_threads so subsequent
thread_upsert cannot pick up old session transcripts.
---
Outside diff comments:
In `@app/src/store/threadSlice.ts`:
- Around line 51-215: The thunks (loadThreads, createNewThread, deleteThread,
loadThreadMessages, addMessageLocal, addInferenceResponse, persistReaction,
purgeThreads, fetchSuggestedQuestions) lack namespaced dev-only logs; add debug
logs at thunk entry, successful exit (including returned ids/summary) and on
failure, using a grep-friendly prefix like "[threads]" or "[ui-flow]"; guard the
logs with a dev check (e.g. process.env.NODE_ENV === 'development') or existing
isDev helper, and include threadId/messageId where applicable (e.g. deleteThread
threadId, loadThreadMessages threadId, addInferenceResponse targetThreadId,
persistReaction payload.threadId/messageId) and surface error.message in catch
blocks so failures are logged before rejectWithValue is returned.
---
Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 709-817: Add namespaced dev-only debug logs for the Conversations
flow: insert console.debug calls (or your project's debug util) with a
"Conversations" prefix in the handlers referenced here — inside
handleCreateNewThread calls (log when invoked and the created thread id/title),
inside the thread selection block where dispatch(setSelectedThread(thread.id))
and dispatch(loadThreadMessages(thread.id)) are called (log the selected thread
id/title and that messages are being loaded), and in the sidebar toggle button
(log showSidebar state after toggling); include minimal structured details (id,
title, messageCount) to help correlate with RPC/socket logs and ensure logs are
only emitted in dev builds.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d17c0bc1-f78e-4168-9fce-834418366770
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
app/src/components/BottomTabBar.tsxapp/src/pages/Conversations.tsxapp/src/services/api/threadApi.test.tsapp/src/services/api/threadApi.tsapp/src/store/threadSlice.tssrc/core/all.rssrc/openhuman/agent/harness/session/runtime.rssrc/openhuman/channels/providers/web.rssrc/openhuman/memory/ops.rssrc/openhuman/memory/schemas.rssrc/openhuman/mod.rssrc/openhuman/threads/mod.rssrc/openhuman/threads/ops.rssrc/openhuman/threads/schemas.rs
💤 Files with no reviewable changes (1)
- app/src/components/BottomTabBar.tsx
Comment on lines
+225
to
245
| const handleCreateNewThread = async () => { | ||
| const thread = await dispatch(createNewThread()).unwrap(); | ||
| dispatch(setSelectedThread(thread.id)); | ||
| void dispatch(loadThreadMessages(thread.id)); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| if (selectedThreadId) dispatch(setLastViewed(selectedThreadId)); | ||
| }, [selectedThreadId, dispatch]); | ||
| void dispatch(loadThreads()) | ||
| .unwrap() | ||
| .then(data => { | ||
| if (data.threads.length > 0) { | ||
| const mostRecent = data.threads[0]; | ||
| dispatch(setSelectedThread(mostRecent.id)); | ||
| void dispatch(loadThreadMessages(mostRecent.id)); | ||
| } else { | ||
| void handleCreateNewThread(); | ||
| } | ||
| }); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [dispatch]); | ||
|
|
Contributor
There was a problem hiding this comment.
Handle bootstrap/create failures instead of dropping the promise.
Both dispatch(loadThreads()).unwrap() and dispatch(createNewThread()).unwrap() can reject, but this flow launches them fire-and-forget and never catches the error. A backend failure here becomes an unhandled promise rejection and can leave the page with no selected thread or user feedback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 225 - 245, The current
bootstrap uses dispatch(loadThreads()).unwrap() and
dispatch(createNewThread()).unwrap() fire-and-forget which can reject and cause
unhandled promise rejections; update handleCreateNewThread and the useEffect
flow to properly await and catch errors: wrap the await
dispatch(createNewThread()).unwrap() in try/catch inside handleCreateNewThread
and surface failures (e.g., show error UI or retry) instead of dropping the
promise, and in the useEffect replace the chained .then with an async IIFE or
promise chain that awaits dispatch(loadThreads()).unwrap() inside try/catch,
handle the empty-threads case by calling the try/catch-protected
handleCreateNewThread, and ensure any calls to setSelectedThread and
loadThreadMessages are only executed after successful dispatch results so errors
are not swallowed.
Comment on lines
+382
to
+389
| const handleSlashCommand = (command: string): boolean => { | ||
| const cmd = command.toLowerCase(); | ||
| if (cmd === '/new' || cmd === '/clear') { | ||
| setInputValue(''); | ||
| void handleCreateNewThread(); | ||
| return true; | ||
| } | ||
| return false; |
Contributor
There was a problem hiding this comment.
/clear shouldn't create a new thread.
This block treats /new and /clear identically, so /clear currently discards the draft and opens another conversation. Keep /clear local to the composer and reserve thread creation for /new.
Suggested fix
const handleSlashCommand = (command: string): boolean => {
const cmd = command.toLowerCase();
- if (cmd === '/new' || cmd === '/clear') {
+ if (cmd === '/clear') {
+ setInputValue('');
+ return true;
+ }
+ if (cmd === '/new') {
setInputValue('');
void handleCreateNewThread();
return true;
}
return false;
};📝 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.
Suggested change
| const handleSlashCommand = (command: string): boolean => { | |
| const cmd = command.toLowerCase(); | |
| if (cmd === '/new' || cmd === '/clear') { | |
| setInputValue(''); | |
| void handleCreateNewThread(); | |
| return true; | |
| } | |
| return false; | |
| const handleSlashCommand = (command: string): boolean => { | |
| const cmd = command.toLowerCase(); | |
| if (cmd === '/clear') { | |
| setInputValue(''); | |
| return true; | |
| } | |
| if (cmd === '/new') { | |
| setInputValue(''); | |
| void handleCreateNewThread(); | |
| return true; | |
| } | |
| return false; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 382 - 389, The slash-command
handler wrongly treats '/clear' like '/new' and calls handleCreateNewThread;
update handleSlashCommand so that cmd === '/new' clears input and calls
handleCreateNewThread(), while cmd === '/clear' only calls setInputValue('') (no
thread creation). Locate handleSlashCommand and modify the branching to separate
'/new' and '/clear' cases, keeping return true for both when handled.
Comment on lines
+733
to
+785
| sortedThreads.map(thread => ( | ||
| <button | ||
| key={thread.id} | ||
| onClick={() => { | ||
| dispatch(setSelectedThread(thread.id)); | ||
| void dispatch(loadThreadMessages(thread.id)); | ||
| }} | ||
| className={`w-full text-left px-4 py-3 border-b border-stone-50 transition-colors group ${ | ||
| selectedThreadId === thread.id | ||
| ? 'bg-primary-50 border-l-2 border-l-primary-500' | ||
| : 'hover:bg-stone-50' | ||
| }`}> | ||
| <div className="flex items-center justify-between"> | ||
| <p | ||
| className={`text-sm truncate flex-1 ${ | ||
| selectedThreadId === thread.id | ||
| ? 'font-medium text-primary-700' | ||
| : 'text-stone-700' | ||
| }`}> | ||
| {thread.title} | ||
| </p> | ||
| <button | ||
| onClick={e => { | ||
| e.stopPropagation(); | ||
| void dispatch(deleteThread(thread.id)); | ||
| }} | ||
| className="ml-2 p-1 rounded opacity-0 group-hover:opacity-100 hover:bg-stone-200 text-stone-400 hover:text-coral-500 transition-all flex-shrink-0" | ||
| title="Delete thread"> | ||
| <svg | ||
| className="w-3 h-3" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| viewBox="0 0 24 24"> | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth={2} | ||
| d="M6 18L18 6M6 6l12 12" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| <div className="flex items-center gap-2 mt-0.5"> | ||
| <span className="text-[10px] text-stone-400"> | ||
| {formatRelativeTime(thread.lastMessageAt)} | ||
| </span> | ||
| {thread.messageCount > 0 && ( | ||
| <span className="text-[10px] text-stone-400"> | ||
| {thread.messageCount} msg{thread.messageCount !== 1 ? 's' : ''} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Avoid nesting the delete button inside the thread row button.
A <button> inside another <button> is invalid HTML and breaks keyboard/screen-reader behavior. Split the row into a non-button container with separate controls, or move the delete action outside the clickable thread button.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 733 - 785, The outer clickable
thread row must not be a <button> containing another <button>; replace the outer
<button> (the element rendering each item in sortedThreads that calls
dispatch(setSelectedThread(thread.id)) and loadThreadMessages(thread.id)) with a
non-button container (e.g., a <div> or <li>) that has role="button",
tabIndex={0}, and an onClick handler invoking those dispatches, and add an
onKeyDown that triggers the same behavior for Enter/Space so keyboard users
work. Keep the inner delete control as a real <button> that calls
deleteThread(thread.id), preserve the className/styles (including
selectedThreadId checks and formatRelativeTime(thread.lastMessageAt) output),
and ensure the container still has the same visual/interactive states
(hover/selected styles) so behavior and accessibility are fixed.
Comment on lines
+64
to
+70
| export const createNewThread = createAsyncThunk( | ||
| 'thread/createNewThread', | ||
| async (_, { dispatch, rejectWithValue }) => { | ||
| try { | ||
| const created = await threadApi.createThread(payload); | ||
| const thread = await threadApi.createNewThread(); | ||
| await dispatch(loadThreads()).unwrap(); | ||
| return created; | ||
| return thread; |
Contributor
There was a problem hiding this comment.
Don’t turn a successful write into a rejected thunk because the follow-up refresh failed.
Both thunks await loadThreads().unwrap() after the server-side mutation. If the write succeeds and only the refresh fails, the thunk still rejects, so callers are likely to retry and create duplicates (createNewThread) or report a false delete failure (deleteThread). Make the refresh best-effort and return the mutation result once the write itself succeeds.
Suggested fix
export const createNewThread = createAsyncThunk(
'thread/createNewThread',
async (_, { dispatch, rejectWithValue }) => {
try {
const thread = await threadApi.createNewThread();
- await dispatch(loadThreads()).unwrap();
+ void dispatch(loadThreads());
return thread;
} catch (error) {
return rejectWithValue(error instanceof Error ? error.message : 'Failed to create thread');
}
}
);
export const deleteThread = createAsyncThunk(
'thread/deleteThread',
async (threadId: string, { dispatch, getState, rejectWithValue }) => {
try {
await threadApi.deleteThread(threadId);
const state = getState() as { thread: ThreadState };
if (state.thread.selectedThreadId === threadId) {
const remaining = state.thread.threads.filter(t => t.id !== threadId);
if (remaining.length > 0) {
dispatch(setSelectedThread(remaining[0].id));
} else {
dispatch(clearSelectedThread());
}
}
- await dispatch(loadThreads()).unwrap();
+ void dispatch(loadThreads());
return { threadId };
} catch (error) {
return rejectWithValue(error instanceof Error ? error.message : 'Failed to delete thread');
}
}
);Also applies to: 77-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/store/threadSlice.ts` around lines 64 - 70, The current
createNewThread thunk calls await dispatch(loadThreads()).unwrap(), which causes
the whole thunk to reject if the refresh fails even when
threadApi.createNewThread() succeeded; change it so the refresh is best-effort:
after successfully awaiting threadApi.createNewThread() (in createNewThread),
call dispatch(loadThreads()) without unwrap and wrap that call in its own
try/catch (log the error but do not rethrow) and then return the created thread;
apply the same pattern to the deleteThread thunk (call threadApi.deleteThread(),
then attempt a best-effort dispatch(loadThreads()) inside a try/catch, and
finally return the deletion result) so write successes are not turned into
rejected thunks by a refresh failure.
Comment on lines
+83
to
+89
| if (state.thread.selectedThreadId === threadId) { | ||
| const remaining = state.thread.threads.filter(t => t.id !== threadId); | ||
| if (remaining.length > 0) { | ||
| dispatch(setSelectedThread(remaining[0].id)); | ||
| } else { | ||
| dispatch(clearSelectedThread()); | ||
| } |
Contributor
There was a problem hiding this comment.
Load the replacement thread’s messages when you auto-select it after deletion.
setSelectedThread(remaining[0].id) only swaps local selection. Conversations.tsx still explicitly dispatches loadThreadMessages(...) after its own programmatic selections, so deleting the active thread can leave the newly selected thread blank unless it was already cached.
Suggested fix
if (state.thread.selectedThreadId === threadId) {
const remaining = state.thread.threads.filter(t => t.id !== threadId);
if (remaining.length > 0) {
- dispatch(setSelectedThread(remaining[0].id));
+ const nextThreadId = remaining[0].id;
+ dispatch(setSelectedThread(nextThreadId));
+ void dispatch(loadThreadMessages(nextThreadId));
} else {
dispatch(clearSelectedThread());
}
}📝 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.
Suggested change
| if (state.thread.selectedThreadId === threadId) { | |
| const remaining = state.thread.threads.filter(t => t.id !== threadId); | |
| if (remaining.length > 0) { | |
| dispatch(setSelectedThread(remaining[0].id)); | |
| } else { | |
| dispatch(clearSelectedThread()); | |
| } | |
| if (state.thread.selectedThreadId === threadId) { | |
| const remaining = state.thread.threads.filter(t => t.id !== threadId); | |
| if (remaining.length > 0) { | |
| const nextThreadId = remaining[0].id; | |
| dispatch(setSelectedThread(nextThreadId)); | |
| void dispatch(loadThreadMessages(nextThreadId)); | |
| } else { | |
| dispatch(clearSelectedThread()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/store/threadSlice.ts` around lines 83 - 89, When auto-selecting a
replacement thread after deleting the active one, also dispatch loading its
messages so the UI isn't blank; in the block where you call
setSelectedThread(remaining[0].id) (inside the thread deletion branch that
checks state.thread.selectedThreadId), follow that dispatch with a
dispatch(loadThreadMessages(remaining[0].id)) (or the existing action that
fetches/caches messages) so the newly selected thread's messages are loaded, and
keep clearSelectedThread() as-is for the empty case.
Comment on lines
+140
to
+141
| // Conversation thread and message management | ||
| controllers.extend(crate::openhuman::threads::all_threads_registered_controllers()); |
Contributor
There was a problem hiding this comment.
Add the CLI/help description for threads as well.
The new namespace is registered here, but namespace_description() still returns None for "threads", so help/discovery output loses the friendly label for these RPCs.
Also applies to: 187-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/all.rs` around lines 140 - 141, namespace "threads" was registered
via
controllers.extend(crate::openhuman::threads::all_threads_registered_controllers())
but namespace_description() still lacks an entry for "threads", causing help
output to show None; update the namespace_description function to return a
friendly description for "threads" (e.g., Some("Conversation thread and message
management")) by adding the "threads" match arm or map entry alongside the other
namespaces (also ensure you add the same description where namespace_description
is referenced around the similar registration at 187-188).
Comment on lines
+254
to
+274
| /// Invalidate all cached agent sessions for the given thread ID. | ||
| /// Called when a thread is deleted so stale sessions don't leak | ||
| /// into reused thread IDs. | ||
| pub async fn invalidate_thread_sessions(thread_id: &str) { | ||
| let mut sessions = THREAD_SESSIONS.lock().await; | ||
| let keys_to_remove: Vec<String> = sessions | ||
| .keys() | ||
| .filter(|k| k.ends_with(&format!("::{thread_id}"))) | ||
| .cloned() | ||
| .collect(); | ||
| for key in &keys_to_remove { | ||
| sessions.remove(key); | ||
| } | ||
| if !keys_to_remove.is_empty() { | ||
| log::debug!( | ||
| "[web-channel] invalidated {} cached session(s) for thread_id={}", | ||
| keys_to_remove.len(), | ||
| thread_id | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Invalidate active requests too, not just cached sessions.
This only clears THREAD_SESSIONS. If the deleted thread still has an in-flight chat task, that task can finish and reinsert a fresh SessionEntry at Lines 423-432, effectively reviving the deleted thread's session state. Abort matching IN_FLIGHT work or mark the thread invalid before reinserting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/channels/providers/web.rs` around lines 254 - 274,
invalidate_thread_sessions currently only removes entries from THREAD_SESSIONS
but leaves any matching in-flight work running, which can recreate a
SessionEntry later; update invalidate_thread_sessions to also abort or mark
matching work in IN_FLIGHT (or a new THREAD_INVALIDATED marker) so tasks
handling entries in IN_FLIGHT either get canceled or detect the thread as
invalid before they reinsert a SessionEntry (refer to THREAD_SESSIONS,
IN_FLIGHT, and the code path that constructs/ reinserts SessionEntry); acquire
the appropriate locks (both THREAD_SESSIONS and IN_FLIGHT or update the shared
invalidation flag) and either remove/cancel IN_FLIGHT entries for the thread or
set a boolean invalidation marker that the reinsert logic checks and bails out
on.
Comment on lines
+851
to
+860
| // Scope session transcripts per thread so each conversation | ||
| // gets its own transcript file instead of sharing one by | ||
| // agent type. Without this, new threads load the latest | ||
| // transcript for the agent name and inherit prior messages. | ||
| let short_thread = if thread_id.len() > 12 { | ||
| &thread_id[..12] | ||
| } else { | ||
| thread_id | ||
| }; | ||
| agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}")); |
Contributor
There was a problem hiding this comment.
Don't shorten the transcript namespace to 12 characters.
Transcript lookup/file creation keys off agent_definition_name, so truncating thread_id here aliases distinct threads that share the same prefix. That can cross-load another thread's transcript history. Use the full ID or a collision-resistant hash instead.
Suggested fix
- let short_thread = if thread_id.len() > 12 {
- &thread_id[..12]
- } else {
- thread_id
- };
- agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}"));
+ agent.set_agent_definition_name(format!("{target_agent_id}_{thread_id}"));📝 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.
Suggested change
| // Scope session transcripts per thread so each conversation | |
| // gets its own transcript file instead of sharing one by | |
| // agent type. Without this, new threads load the latest | |
| // transcript for the agent name and inherit prior messages. | |
| let short_thread = if thread_id.len() > 12 { | |
| &thread_id[..12] | |
| } else { | |
| thread_id | |
| }; | |
| agent.set_agent_definition_name(format!("{target_agent_id}_{short_thread}")); | |
| // Scope session transcripts per thread so each conversation | |
| // gets its own transcript file instead of sharing one by | |
| // agent type. Without this, new threads load the latest | |
| // transcript for the agent name and inherit prior messages. | |
| agent.set_agent_definition_name(format!("{target_agent_id}_{thread_id}")); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/channels/providers/web.rs` around lines 851 - 860, The code
truncates thread_id to 12 chars before calling agent.set_agent_definition_name
which causes distinct threads with the same prefix to alias transcripts; stop
truncating and either use the full thread_id when building the
agent_definition_name (e.g. format!("{target_agent_id}_{thread_id}")) or replace
the truncation with a deterministic collision-resistant hash of thread_id (e.g.
SHA-256 hex) and use that hash (or a long portion of it) in the
agent.set_agent_definition_name call so transcript lookup is unique and
deterministic.
Comment on lines
+95
to
+237
| /// Lists all conversation threads. | ||
| pub async fn threads_list( | ||
| _request: EmptyRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<ConversationThreadsListResponse>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let threads = conversations::list_threads(dir)? | ||
| .into_iter() | ||
| .map(thread_to_summary) | ||
| .collect::<Vec<_>>(); | ||
| let count = threads.len(); | ||
| Ok(envelope( | ||
| ConversationThreadsListResponse { threads, count }, | ||
| Some(counts([("num_threads", count)])), | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Creates or refreshes a conversation thread. | ||
| pub async fn thread_upsert( | ||
| request: UpsertConversationThreadRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<ConversationThreadSummary>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let thread = conversations::ensure_thread( | ||
| dir, | ||
| CreateConversationThread { | ||
| id: request.id, | ||
| title: request.title, | ||
| created_at: request.created_at, | ||
| }, | ||
| )?; | ||
| Ok(envelope( | ||
| thread_to_summary(thread), | ||
| Some(counts([("num_threads", 1)])), | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Creates a new conversation thread with auto-generated ID and title. | ||
| pub async fn thread_create_new( | ||
| _request: EmptyRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<ConversationThreadSummary>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let id = format!("thread-{}", uuid::Uuid::new_v4()); | ||
| let now = chrono::Local::now(); | ||
| let title = format!("Chat {} {}", now.format("%b %-d"), now.format("%-I:%M %p")); | ||
| let created_at = chrono::Utc::now().to_rfc3339(); | ||
| let thread = conversations::ensure_thread( | ||
| dir, | ||
| CreateConversationThread { | ||
| id, | ||
| title, | ||
| created_at, | ||
| }, | ||
| )?; | ||
| Ok(envelope( | ||
| thread_to_summary(thread), | ||
| Some(counts([("num_threads", 1)])), | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Lists messages for a conversation thread. | ||
| pub async fn messages_list( | ||
| request: ConversationMessagesRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<ConversationMessagesResponse>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let messages = conversations::get_messages(dir, &request.thread_id)? | ||
| .into_iter() | ||
| .map(message_to_record) | ||
| .collect::<Vec<_>>(); | ||
| let count = messages.len(); | ||
| Ok(envelope( | ||
| ConversationMessagesResponse { messages, count }, | ||
| Some(counts([("num_messages", count)])), | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Appends a message to a conversation thread. | ||
| pub async fn message_append( | ||
| request: AppendConversationMessageRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<ConversationMessageRecord>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let message = | ||
| conversations::append_message(dir, &request.thread_id, record_to_message(request.message))?; | ||
| Ok(envelope( | ||
| message_to_record(message), | ||
| Some(counts([("num_messages", 1)])), | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Updates metadata on an existing conversation message. | ||
| pub async fn message_update( | ||
| request: UpdateConversationMessageRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<ConversationMessageRecord>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let message = conversations::update_message( | ||
| dir, | ||
| &request.thread_id, | ||
| &request.message_id, | ||
| ConversationMessagePatch { | ||
| extra_metadata: request.extra_metadata, | ||
| }, | ||
| )?; | ||
| Ok(envelope( | ||
| message_to_record(message), | ||
| Some(counts([("num_messages", 1)])), | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Deletes a conversation thread and its message log. | ||
| pub async fn thread_delete( | ||
| request: DeleteConversationThreadRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<DeleteConversationThreadResponse>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let deleted = conversations::ConversationStore::new(dir) | ||
| .delete_thread(&request.thread_id, &request.deleted_at)?; | ||
| web_channel::invalidate_thread_sessions(&request.thread_id).await; | ||
| Ok(envelope( | ||
| DeleteConversationThreadResponse { deleted }, | ||
| None, | ||
| None, | ||
| )) | ||
| } | ||
|
|
||
| /// Purges all conversation threads and messages. | ||
| pub async fn threads_purge( | ||
| _request: EmptyRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<PurgeConversationThreadsResponse>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let stats = conversations::purge_threads(dir)?; | ||
| Ok(envelope( | ||
| PurgeConversationThreadsResponse { | ||
| messages_deleted: stats.message_count, | ||
| agent_threads_deleted: stats.thread_count, | ||
| agent_messages_deleted: stats.message_count, | ||
| }, | ||
| None, | ||
| None, | ||
| )) | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add structured Rust debug logs across the new threads RPC flow.
This new domain performs storage mutations plus cross-module session invalidation, but none of the handlers log entry/exit, thread_id, counts, or failure paths. Please add [threads] / [rpc] debug logs so thread/session lifecycle problems can be traced from the frontend through the sidecar.
As per coding guidelines, "Add substantial, development-oriented logs on new/changed flows; include logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths" and "Use structured, grep-friendly context in logs with stable prefixes (e.g., [domain], [rpc], [ui-flow]) and include correlation fields such as request IDs, method names, and entity IDs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/threads/ops.rs` around lines 95 - 237, Add structured debug
logs with stable prefixes ([threads] and [rpc]) to trace entry/exit, key params,
branch decisions, external calls and error paths for the new threads RPC flow:
instrument the handlers threads_list, thread_upsert, thread_create_new,
messages_list, message_append, message_update, thread_delete, and threads_purge
to log on entry (include request id if available, method name and input IDs like
thread_id), before/after external/storage calls (conversations::list_threads,
conversations::ensure_thread, conversations::get_messages,
conversations::append_message, conversations::update_message,
conversations::ConversationStore::delete_thread, conversations::purge_threads,
web_channel::invalidate_thread_sessions) with outcomes/counts, and on
errors/failures include the error details and context; use consistent structured
messages and fields (e.g., "method", "thread_id", "count", "error") and place
exit/response logs showing results (counts or created/updated summary) so
frontend→sidecar lifecycle can be traced end-to-end.
Comment on lines
+223
to
+236
| pub async fn threads_purge( | ||
| _request: EmptyRequest, | ||
| ) -> Result<RpcOutcome<ApiEnvelope<PurgeConversationThreadsResponse>>, String> { | ||
| let dir = workspace_dir().await?; | ||
| let stats = conversations::purge_threads(dir)?; | ||
| Ok(envelope( | ||
| PurgeConversationThreadsResponse { | ||
| messages_deleted: stats.message_count, | ||
| agent_threads_deleted: stats.thread_count, | ||
| agent_messages_deleted: stats.message_count, | ||
| }, | ||
| None, | ||
| None, | ||
| )) |
Contributor
There was a problem hiding this comment.
Purge should clear the in-memory per-thread session cache too.
thread_delete invalidates THREAD_SESSIONS, but threads_purge only removes the on-disk thread/message store. That leaves stale in-memory session state behind for every purged thread, and a later thread_upsert with the same ID can accidentally pick up the old session transcript.
Suggested fix
pub async fn threads_purge(
_request: EmptyRequest,
) -> Result<RpcOutcome<ApiEnvelope<PurgeConversationThreadsResponse>>, String> {
let dir = workspace_dir().await?;
+ let thread_ids = conversations::list_threads(dir.clone())?
+ .into_iter()
+ .map(|thread| thread.id)
+ .collect::<Vec<_>>();
let stats = conversations::purge_threads(dir)?;
+ for thread_id in thread_ids {
+ web_channel::invalidate_thread_sessions(&thread_id).await;
+ }
Ok(envelope(
PurgeConversationThreadsResponse {
messages_deleted: stats.message_count,
agent_threads_deleted: stats.thread_count,
agent_messages_deleted: stats.message_count,📝 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.
Suggested change
| pub async fn threads_purge( | |
| _request: EmptyRequest, | |
| ) -> Result<RpcOutcome<ApiEnvelope<PurgeConversationThreadsResponse>>, String> { | |
| let dir = workspace_dir().await?; | |
| let stats = conversations::purge_threads(dir)?; | |
| Ok(envelope( | |
| PurgeConversationThreadsResponse { | |
| messages_deleted: stats.message_count, | |
| agent_threads_deleted: stats.thread_count, | |
| agent_messages_deleted: stats.message_count, | |
| }, | |
| None, | |
| None, | |
| )) | |
| pub async fn threads_purge( | |
| _request: EmptyRequest, | |
| ) -> Result<RpcOutcome<ApiEnvelope<PurgeConversationThreadsResponse>>, String> { | |
| let dir = workspace_dir().await?; | |
| let thread_ids = conversations::list_threads(dir.clone())? | |
| .into_iter() | |
| .map(|thread| thread.id) | |
| .collect::<Vec<_>>(); | |
| let stats = conversations::purge_threads(dir)?; | |
| for thread_id in thread_ids { | |
| web_channel::invalidate_thread_sessions(&thread_id).await; | |
| } | |
| Ok(envelope( | |
| PurgeConversationThreadsResponse { | |
| messages_deleted: stats.message_count, | |
| agent_threads_deleted: stats.thread_count, | |
| agent_messages_deleted: stats.message_count, | |
| }, | |
| None, | |
| None, | |
| )) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/threads/ops.rs` around lines 223 - 236, threads_purge currently
removes on-disk threads but leaves stale in-memory session state in
THREAD_SESSIONS; update threads_purge to also invalidate the in-memory sessions
for the purged threads by obtaining the list of deleted thread IDs from
conversations::purge_threads (change its return to include Vec<ThreadId> or
similar) or, if you intend to purge everything, call the appropriate
THREAD_SESSIONS API to clear all entries; ensure you reference THREAD_SESSIONS
in threads_purge and remove each purged thread id (or clear the entire cache)
after calling conversations::purge_threads so subsequent thread_upsert cannot
pick up old session transcripts.
- Changed the model ID used in the chatSend function from `agentic-v1` to `reasoning-v1`, clarifying the purpose of each model. - Added comments to explain the distinction between the reasoning model and the agentic model, enhancing code readability and maintainability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
app/src/pages/Conversations.tsx (3)
384-392:⚠️ Potential issue | 🟡 Minor
/clearshould not create a new thread.Currently both
/newand/cleartriggerhandleCreateNewThread(). The/clearcommand should only clear the input field, not start a new conversation.Suggested fix
const handleSlashCommand = (command: string): boolean => { const cmd = command.toLowerCase(); - if (cmd === '/new' || cmd === '/clear') { + if (cmd === '/clear') { + setInputValue(''); + return true; + } + if (cmd === '/new') { setInputValue(''); void handleCreateNewThread(); return true; } return false; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 384 - 392, The slash-command handler handleSlashCommand treats both '/new' and '/clear' the same, causing '/clear' to start a new thread; change the logic so that when cmd === '/clear' you only call setInputValue('') and return true, while only cmd === '/new' calls handleCreateNewThread(); update handleSlashCommand to branch on these two cases accordingly (refer to handleSlashCommand, handleCreateNewThread, and setInputValue).
227-246:⚠️ Potential issue | 🟠 MajorUnhandled promise rejections in thread bootstrap flow.
Both
handleCreateNewThreadand the bootstrapuseEffectcan reject without error handling:
dispatch(createNewThread()).unwrap()at line 228 can throw if the RPC failsdispatch(loadThreads()).unwrap()at line 234-235 has no.catch()handler- Line 242 fires
handleCreateNewThread()without catching its rejectionA backend failure leaves the page in an inconsistent state with no user feedback.
Additionally, per coding guidelines, add development-oriented logs for this flow to aid debugging (e.g.,
console.debug('[threads] bootstrap: loaded N threads'),console.debug('[threads] creating new thread')).Suggested fix
- const handleCreateNewThread = async () => { - const thread = await dispatch(createNewThread()).unwrap(); - dispatch(setSelectedThread(thread.id)); - void dispatch(loadThreadMessages(thread.id)); - }; + const handleCreateNewThread = async () => { + try { + console.debug('[threads] creating new thread'); + const thread = await dispatch(createNewThread()).unwrap(); + console.debug('[threads] created thread', thread.id); + dispatch(setSelectedThread(thread.id)); + void dispatch(loadThreadMessages(thread.id)); + } catch (err) { + console.error('[threads] failed to create thread', err); + setSendError(chatSendError('thread_create_failed', 'Failed to create a new thread.')); + } + }; useEffect(() => { - void dispatch(loadThreads()) - .unwrap() - .then(data => { + void (async () => { + try { + console.debug('[threads] loading threads'); + const data = await dispatch(loadThreads()).unwrap(); + console.debug('[threads] loaded', data.threads.length, 'threads'); if (data.threads.length > 0) { const mostRecent = data.threads[0]; dispatch(setSelectedThread(mostRecent.id)); void dispatch(loadThreadMessages(mostRecent.id)); } else { - void handleCreateNewThread(); + await handleCreateNewThread(); } - }); + } catch (err) { + console.error('[threads] bootstrap failed', err); + setSendError(chatSendError('thread_load_failed', 'Failed to load threads.')); + } + })(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [dispatch]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 227 - 246, The bootstrap flow can throw unhandled promise rejections; wrap async calls in try/catch and add debug logs: update handleCreateNewThread to catch errors from dispatch(createNewThread()).unwrap(), log console.debug('[threads] creating new thread') before creating and console.debug on success/failure, and ensure useEffect awaits and wraps dispatch(loadThreads()).unwrap() in a try/catch (logging console.debug('[threads] bootstrap: loaded N threads') on success) and calls handleCreateNewThread with await inside a try/catch so its rejection is handled; also catch and handle errors around dispatch(loadThreadMessages(thread.id)) and dispatch(setSelectedThread(...)) so the UI can remain consistent and you can dispatch or surface an error state if needed.
736-787:⚠️ Potential issue | 🟠 MajorNested
<button>elements are invalid HTML.The delete button (lines 756-775) is nested inside the thread row button (line 736). This is invalid HTML and breaks keyboard navigation and screen reader behavior.
Replace the outer
<button>with a non-interactive container (<div>) that handles click viarole="button",tabIndex={0}, and keyboard event handlers.Suggested fix
- <button + <div + role="button" + tabIndex={0} key={thread.id} onClick={() => { dispatch(setSelectedThread(thread.id)); void dispatch(loadThreadMessages(thread.id)); }} + onKeyDown={e => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + dispatch(setSelectedThread(thread.id)); + void dispatch(loadThreadMessages(thread.id)); + } + }} className={`w-full text-left px-4 py-3 border-b border-stone-50 transition-colors group ${ selectedThreadId === thread.id ? 'bg-primary-50 border-l-2 border-l-primary-500' : 'hover:bg-stone-50' - }`}> + } cursor-pointer`}> {/* ... inner content unchanged ... */} - </button> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 736 - 787, The thread row uses a <button> that wraps another interactive delete <button>, which is invalid HTML and breaks accessibility; replace the outer clickable element (the element that currently uses selectedThreadId, setSelectedThread, and loadThreadMessages) with a non-interactive container (e.g., a <div>) given role="button", tabIndex={0}, onClick calling dispatch(setSelectedThread(thread.id)) and void dispatch(loadThreadMessages(thread.id)), and onKeyDown handling Enter/Space to invoke the same actions, while keeping the inner delete button (deleteThread) unchanged and ensuring its e.stopPropagation() remains so it doesn't trigger the row activation; preserve all CSS classes (including the conditional selectedThreadId styling) and existing children like the formatRelativeTime and messageCount display.
🧹 Nitpick comments (1)
app/src/pages/Conversations.tsx (1)
706-708: Consider memoizingsortedThreads.The spread-and-sort creates a new array on every render. While unlikely to cause issues with typical thread counts, wrapping in
useMemoavoids unnecessary re-sorting when unrelated state changes.Suggested optimization
+ import { useEffect, useMemo, useRef, useState } from 'react'; - import { useEffect, useRef, useState } from 'react'; - const sortedThreads = [...threads].sort( - (a, b) => new Date(b.lastMessageAt).getTime() - new Date(a.lastMessageAt).getTime() - ); + const sortedThreads = useMemo( + () => + [...threads].sort( + (a, b) => new Date(b.lastMessageAt).getTime() - new Date(a.lastMessageAt).getTime() + ), + [threads] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/Conversations.tsx` around lines 706 - 708, The current code recreates and sorts threads on every render by doing const sortedThreads = [...threads].sort(...); wrap this in React's useMemo so sorting only runs when threads changes: useMemo(() => [...threads].sort((a,b)=>...), [threads]); update any references to sortedThreads to use the memoized value and import useMemo if not already imported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 757-760: The onClick handler currently fire-and-forgets
dispatch(deleteThread(thread.id)); update it to handle errors and surface
feedback: await the dispatched thunk or return promise from deleteThread, catch
failures, and show a user-visible error (e.g., toast or modal) and/or rollback
any optimistic UI changes; ensure the onClick uses async/await (or .then/.catch)
and references deleteThread and thread.id so the UI only confirms deletion on
success and displays an error message on failure.
---
Duplicate comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 384-392: The slash-command handler handleSlashCommand treats both
'/new' and '/clear' the same, causing '/clear' to start a new thread; change the
logic so that when cmd === '/clear' you only call setInputValue('') and return
true, while only cmd === '/new' calls handleCreateNewThread(); update
handleSlashCommand to branch on these two cases accordingly (refer to
handleSlashCommand, handleCreateNewThread, and setInputValue).
- Around line 227-246: The bootstrap flow can throw unhandled promise
rejections; wrap async calls in try/catch and add debug logs: update
handleCreateNewThread to catch errors from dispatch(createNewThread()).unwrap(),
log console.debug('[threads] creating new thread') before creating and
console.debug on success/failure, and ensure useEffect awaits and wraps
dispatch(loadThreads()).unwrap() in a try/catch (logging
console.debug('[threads] bootstrap: loaded N threads') on success) and calls
handleCreateNewThread with await inside a try/catch so its rejection is handled;
also catch and handle errors around dispatch(loadThreadMessages(thread.id)) and
dispatch(setSelectedThread(...)) so the UI can remain consistent and you can
dispatch or surface an error state if needed.
- Around line 736-787: The thread row uses a <button> that wraps another
interactive delete <button>, which is invalid HTML and breaks accessibility;
replace the outer clickable element (the element that currently uses
selectedThreadId, setSelectedThread, and loadThreadMessages) with a
non-interactive container (e.g., a <div>) given role="button", tabIndex={0},
onClick calling dispatch(setSelectedThread(thread.id)) and void
dispatch(loadThreadMessages(thread.id)), and onKeyDown handling Enter/Space to
invoke the same actions, while keeping the inner delete button (deleteThread)
unchanged and ensuring its e.stopPropagation() remains so it doesn't trigger the
row activation; preserve all CSS classes (including the conditional
selectedThreadId styling) and existing children like the formatRelativeTime and
messageCount display.
---
Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 706-708: The current code recreates and sorts threads on every
render by doing const sortedThreads = [...threads].sort(...); wrap this in
React's useMemo so sorting only runs when threads changes: useMemo(() =>
[...threads].sort((a,b)=>...), [threads]); update any references to
sortedThreads to use the memoized value and import useMemo if not already
imported.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6c01376-1e5f-494f-a1b7-2d3a27f75b71
📒 Files selected for processing (1)
app/src/pages/Conversations.tsx
Comment on lines
+757
to
+760
| onClick={e => { | ||
| e.stopPropagation(); | ||
| void dispatch(deleteThread(thread.id)); | ||
| }} |
Contributor
There was a problem hiding this comment.
Consider handling delete errors.
The delete action fires-and-forgets without user feedback on failure. For destructive operations, users should know if deletion failed.
Suggested improvement
onClick={e => {
e.stopPropagation();
- void dispatch(deleteThread(thread.id));
+ void dispatch(deleteThread(thread.id))
+ .unwrap()
+ .catch(err => {
+ console.error('[threads] delete failed', err);
+ setSendError(chatSendError('thread_delete_failed', 'Failed to delete thread.'));
+ });
}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/Conversations.tsx` around lines 757 - 760, The onClick handler
currently fire-and-forgets dispatch(deleteThread(thread.id)); update it to
handle errors and surface feedback: await the dispatched thunk or return promise
from deleteThread, catch failures, and show a user-visible error (e.g., toast or
modal) and/or rollback any optimistic UI changes; ensure the onClick uses
async/await (or .then/.catch) and references deleteThread and thread.id so the
UI only confirms deletion on success and displays an error message on failure.
This was referenced Apr 24, 2026
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
openhuman.threads_*controller for conversation thread CRUD (list, create, upsert, delete, purge, messages).threads_create_newRPC.+ Newbutton, and/new//clearslash commands.Problem
Every chat defaulted to a single hardcoded
default-thread, and there was no way to create, select, or clear threads. Worse, session transcripts keyed purely by agent name (main_0.md,orchestrator_1.md) — so when the web channel created a newAgentfor a fresh thread,try_load_session_transcript()picked up the latest transcript for that agent and bled prior thread messages into the new conversation. Users reported messages from other threads appearing in fresh ones.Thread logic was also split awkwardly: IDs/titles generated in TS, storage in Rust
memory/conversations, and RPC methods living under thememorynamespace despite being a distinct domain.Solution
New
threadscontroller domain (src/openhuman/threads/) — mirrors the project's standardmod.rs+ops.rs+schemas.rslayout. 8 RPC methods under thethreadsnamespace; handlers delegate to the existingmemory::conversationsJSONL store. Registered viacore/all.rs.Per-thread session scoping — added
Agent::set_agent_definition_name()setter. The web channel now stamps{target_agent_id}_{thread_prefix}onto each session agent so transcripts land under e.g.orchestrator_thread-abc123_0.mdinstead of sharing one file by agent type. Also addedinvalidate_thread_sessions()called fromthread_deleteto clearTHREAD_SESSIONScache on delete.Server-side thread creation —
threads_create_newgenerates a UUID-based ID and human-readable title (Chat Apr 15 2:30 PM). Frontend drops itsgenerateThreadId/generateThreadTitlehelpers.Simplified frontend —
threadApi.tsnow calls the newopenhuman.threads_*methods.threadSlice.tsstripped to a minimal UI cache layer (no morepanelWidth,lastViewedAt,sendStatus/sendError/deleteStatus/purgeStatus).Conversations.tsxgains a sidebar, header with thread title + new button, and slash-command handling.Submission Checklist
cargo testcoverage onopenhuman::threads::schemas(namespace, registry, handler pairing, fallback) and existingopenhuman::memory::schemastests updated; Agent session runtime tests still pass.conversations-web-channel-flow.spec.tsexercises the chat path. Manual verification: creating a new thread no longer inherits prior messages in the session transcript.//!module headers on the newthreadssubmodules;///on every public fn; inline comment on the web channel session-scoping rationale.agent_definition_nameinbuild_session_agent(transcript path scoping) rather than the mechanical what.Impact
openhuman.memory_thread*/openhuman.memory_messages_*methods must migrate toopenhuman.threads_*. No stored-data migration needed —memory/conversations/JSONL layout is unchanged.sessions/DDMMYYYY/{agent}_N.mdremain on disk but will no longer be auto-loaded by new threads (which now look for{agent}_{thread_prefix}_N.md). Effectively a fresh-start per thread; legacy files are harmless.Mutexoperation on delete to invalidate the session cache.Related
memory/conversations/store underthreads/too so the domain is fully self-contained.Summary by CodeRabbit
New Features
/newto start a conversation and/clearto reset input.UI Updates