feat(thread): support manual editing of conversation thread titles#2525
Merged
senamakel merged 10 commits intoMay 25, 2026
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:
📝 WalkthroughWalkthroughAdds in-place thread title editing end-to-end: frontend UI and handlers, ChangesIn-place thread title editing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 1299-1314: The inline title input (editTitleInputRef /
editTitleValue) and the pencil icon button that toggles editing (the control
that calls setEditingTitle) lack accessible names; add explicit accessible
labels: give the input either an aria-label (e.g., "Conversation title") or
aria-labelledby pointing to a visible label/heading id, and add an aria-label on
the pencil button (e.g., "Edit conversation title") so screen readers announce
the controls; ensure labels are unique, update any related JSX for the pencil
button (the toggle that uses setEditingTitle) to include the aria-label, and
keep existing handlers (handleCommitTitle, onBlur, onKeyDown) intact.
🪄 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: 40847230-b2a0-46ac-a04f-d87872234181
📒 Files selected for processing (6)
app/src/pages/Conversations.tsxapp/src/services/api/threadApi.tsapp/src/store/threadSlice.tssrc/openhuman/memory/rpc_models.rssrc/openhuman/threads/ops.rssrc/openhuman/threads/schemas.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/threads/ops_tests.rs (1)
695-792: 🏗️ Heavy liftSplit
ops_tests.rs; this addition pushes an already oversized module further beyond the repo limit.This new test block is good coverage-wise, but
src/openhuman/threads/ops_tests.rsis now ~793 lines. Please move thisthread_update_title_*group into a dedicated sibling test module/file to keep responsibilities focused and stay within the size guideline.As per coding guidelines
**/*.{ts,tsx,rs}: "File size should not exceed approximately 500 lines. When a module grows beyond this threshold, split it into smaller, more focused modules with clear responsibilities."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/threads/ops_tests.rs` around lines 695 - 792, The tests for thread title updates are pushing ops_tests.rs over the size limit; extract the four tests thread_update_title_rejects_empty_title, thread_update_title_rejects_whitespace_only_title, thread_update_title_persists_new_title, and thread_update_title_returns_error_for_missing_thread into a new sibling test module file and import the same helpers/fixtures they rely on (EnvVarGuard, create_thread_with_title, thread_update_title, config::TEST_ENV_LOCK, tempfile). Ensure the new file declares the same async #[tokio::test] functions with identical names and that it has the necessary use statements so it compiles standalone; finally remove those tests from ops_tests.rs so the original file goes back under the size guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/threads/ops_tests.rs`:
- Around line 695-792: The tests for thread title updates are pushing
ops_tests.rs over the size limit; extract the four tests
thread_update_title_rejects_empty_title,
thread_update_title_rejects_whitespace_only_title,
thread_update_title_persists_new_title, and
thread_update_title_returns_error_for_missing_thread into a new sibling test
module file and import the same helpers/fixtures they rely on (EnvVarGuard,
create_thread_with_title, thread_update_title, config::TEST_ENV_LOCK, tempfile).
Ensure the new file declares the same async #[tokio::test] functions with
identical names and that it has the necessary use statements so it compiles
standalone; finally remove those tests from ops_tests.rs so the original file
goes back under the size guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7e7fc88-537b-4df1-9d3e-8cd72b136e31
📒 Files selected for processing (4)
app/src/services/api/threadApi.test.tsapp/src/store/__tests__/threadSlice.test.tssrc/openhuman/threads/ops_tests.rstests/json_rpc_e2e.rs
The tests assert that all_controller_schemas() and all_registered_controllers() match a hardcoded list. The new update_title controller was registered but not added to ALL_FUNCTIONS, causing CI to fail (16 actual vs 15 expected).
…coderabbitai) Add aria-label to the inline title input and pencil edit button for screen reader support. Adds 'chat.editThreadTitle' i18n key to en.ts and all locale chunk-2 files.
coderabbitai
Bot
added
the
feature
Exercises handleStartEditTitle, handleCommitTitle, and the inline edit JSX branches (Enter commit, Escape cancel, empty-title guard). Addresses diff-cover failure on Conversations.tsx changed lines.
coderabbitai
Bot
added
the
rust-core
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/pages/__tests__/Conversations.render.test.tsx (1)
1346-1377: ⚡ Quick winAssert the user-visible commit result, not just the API call.
This test currently validates an implementation detail (
threadApi.updateTitleinvocation). Add assertions that edit mode exits and the updated title appears.Suggested refactor
- await waitFor(() => { - expect(threadApi.updateTitle).toHaveBeenCalledWith('commit-title-thread', 'New Title'); - }); + await waitFor(() => { + expect(threadApi.updateTitle).toHaveBeenCalledWith('commit-title-thread', 'New Title'); + expect(screen.queryByLabelText('Edit thread title')).not.toBeInTheDocument(); + expect(screen.getByText('New Title')).toBeInTheDocument(); + });As per coding guidelines: "Prefer testing behavior over implementation details."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/__tests__/Conversations.render.test.tsx` around lines 1346 - 1377, The test currently only asserts threadApi.updateTitle was called; update it to assert the user-visible outcome: after firing Enter, wait for edit mode to exit (e.g., the 'Edit thread title' input is no longer in the DOM or its label is gone) and assert the updated title text "New Title" is rendered in the document (e.g., screen.getByText('New Title')) and that normal title UI (edit button via getByRole('button', { name: 'Edit thread title' })) is present again; keep existing calls to renderConversations, selectedThreadState, socketState and the mock for threadApi.updateTitle but add these DOM assertions instead of relying solely on the implementation detail.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/pages/__tests__/Conversations.render.test.tsx`:
- Around line 1316-1319: The suite currently calls vi.clearAllMocks() which
preserves mock implementations and can leak prior overrides of
mockUseUsageState; update the beforeEach to reset the usage-state mock (e.g.,
call mockUseUsageState.mockReset() or vi.resetAllMocks()) before setting
mockGetThreadMessages so this suite starts with a clean mockUseUsageState
implementation and avoids order-dependent flakes.
---
Nitpick comments:
In `@app/src/pages/__tests__/Conversations.render.test.tsx`:
- Around line 1346-1377: The test currently only asserts threadApi.updateTitle
was called; update it to assert the user-visible outcome: after firing Enter,
wait for edit mode to exit (e.g., the 'Edit thread title' input is no longer in
the DOM or its label is gone) and assert the updated title text "New Title" is
rendered in the document (e.g., screen.getByText('New Title')) and that normal
title UI (edit button via getByRole('button', { name: 'Edit thread title' })) is
present again; keep existing calls to renderConversations, selectedThreadState,
socketState and the mock for threadApi.updateTitle but add these DOM assertions
instead of relying solely on the implementation detail.
🪄 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: 536f6cd6-169c-4ff3-bf89-3d1db52771ec
📒 Files selected for processing (1)
app/src/pages/__tests__/Conversations.render.test.tsx
Addresses CodeRabbit review: vi.clearAllMocks() keeps existing mock implementations, so explicitly reset mockUseUsageState to prevent order-dependent flakiness.
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
2 participants
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_update_titleRPC method to the Rust core that persists a user-supplied title on a conversation thread, bypassing AI title generation.threads/schemas.rs) with request validation (empty-title guard).updateThreadTitleRedux thunk and updates the thread slice on success.cancels on Escape.
Problem
Thread titles were set exclusively by AI generation. Users had no way to rename a conversation, making it hard to find threads later or give them meaningful labels.
Solution
UpdateConversationThreadTitleRequest→ops::thread_update_title→conversations::update_thread_titlepersists the title and returns the updatedConversationThreadSummarywrapped in the standardApiEnvelope. Empty titles are rejected at the op layer before reaching storage.threadApi.updateTitlecalls the new RPC method. TheupdateThreadTitleasync thunk dispatches the call and thefulfilledreducer replaces the threadin-place in
state.threads. The UI uses localeditingTitle/editTitleValuestate with aref-focused<input>that auto-selects on open; on commit the thunk isdispatched and the display reverts to the resolved title.
Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will notmerge.
docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change)## Relateddocs/RELEASE-MANUAL-SMOKE.md)Closes #NNNin the## RelatedsectionImpact
update_thread_titleis additive; existing threads keep their AI-generated or default titles.ConversationThread.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
main978c655ecba99d9fb0613db453b52741d741180fValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm debug unit src/pages/Conversations·pnpm debug rust thread_update_titlecargo fmt --check && cargo check --manifest-path Cargo.tomlValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
commits; Escape cancels.
Parity Contract
logic.
rejectWithValue.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Backend / Persistence
Tests
Localization