feat: show custom profiles in chat header profile selector#4382
feat: show custom profiles in chat header profile selector#4382salarkhannn wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughThe chat header now renders agent profile buttons from a locale-aware sorted helper that excludes builtIn profiles, and the helper is covered by unit tests for filtering and ordering. ChangesAgent Profile Selector UI
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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.
🧹 Nitpick comments (1)
app/src/pages/Conversations.tsx (1)
2955-2960: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSort doesn't actually use the app's locale.
a.name.localeCompare(b.name)uses the runtime's default locale, not the app's selecteduiLocale(already available at Line 333 and used elsewhere in this component forchatSend). For non-English users this can order custom profiles inconsistently with the rest of the localized UI.🔧 Proposed fix
{agentProfiles .filter(p => !p.builtIn) .sort( (a, b) => - (a.sortOrder ?? 0) - (b.sortOrder ?? 0) || a.name.localeCompare(b.name) + (a.sortOrder ?? 0) - (b.sortOrder ?? 0) || + a.name.localeCompare(b.name, uiLocale) )🤖 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/Conversations.tsx` around lines 2955 - 2960, The custom agent profile sort in Conversations.tsx is using the runtime default locale via a plain localeCompare call instead of the app’s selected uiLocale. Update the comparator in the agentProfiles sort to pass the existing uiLocale value (the same one already used in chatSend and available in this component) into the name comparison so ordering matches the localized UI for non-English users.
🤖 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 `@app/src/pages/Conversations.tsx`:
- Around line 2955-2960: The custom agent profile sort in Conversations.tsx is
using the runtime default locale via a plain localeCompare call instead of the
app’s selected uiLocale. Update the comparator in the agentProfiles sort to pass
the existing uiLocale value (the same one already used in chatSend and available
in this component) into the name comparison so ordering matches the localized UI
for non-English users.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88b80c28-27a1-4018-ac17-c8bdde5b9ccc
📒 Files selected for processing (1)
app/src/pages/Conversations.tsx
There was a problem hiding this comment.
Pull request overview
This PR extends the chat header’s agent-profile selector so that user-created (non-built-in) profiles from Settings → Profiles are visible and selectable directly from the main chat surface, alongside the existing Quick/Reasoning options.
Changes:
- Render additional radio-style buttons for each non-built-in agent profile in the chat header.
- Sort custom profiles by
sortOrder, then byname.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .sort( | ||
| (a, b) => | ||
| (a.sortOrder ?? 0) - (b.sortOrder ?? 0) || a.name.localeCompare(b.name) | ||
| ) |
| {agentProfiles | ||
| .filter(p => !p.builtIn) | ||
| .sort( | ||
| (a, b) => | ||
| (a.sortOrder ?? 0) - (b.sortOrder ?? 0) || a.name.localeCompare(b.name) | ||
| ) | ||
| .map(profile => ( | ||
| <button | ||
| key={profile.id} | ||
| type="button" | ||
| role="radio" | ||
| aria-checked={selectedAgentProfileId === profile.id} | ||
| data-analytics-id={`chat-header-mode-${profile.id}`} | ||
| onClick={() => void handleSelectAgentProfile(profile.id)} | ||
| className={`rounded-full px-2.5 py-0.5 text-xs font-medium transition-all ${ |
d3e69ac to
22a7d71
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/pages/Conversations.tsx (1)
221-228: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider memoizing the sort result.
sortAgentProfilesis called inline during render (line 2964) rather than being memoized, so it re-filters/re-sorts on every render. Logic itself is correct.♻️ Optional refactor
-{sortAgentProfiles(agentProfiles, uiLocale).map(profile => ( +{sortedAgentProfiles.map(profile => (with
const sortedAgentProfiles = useMemo(() => sortAgentProfiles(agentProfiles, uiLocale), [agentProfiles, uiLocale]);declared near other memoized values.🤖 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/Conversations.tsx` around lines 221 - 228, Memoize the agent profile sorting result in Conversations to avoid re-filtering/re-sorting on every render. Update the `Conversations` component by introducing a `useMemo`-based `sortedAgentProfiles` near the other memoized values, using `sortAgentProfiles(agentProfiles, uiLocale)` with `agentProfiles` and `uiLocale` as dependencies, and then replace the inline call site with the memoized value.
🤖 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 `@app/src/pages/Conversations.tsx`:
- Around line 221-228: Memoize the agent profile sorting result in Conversations
to avoid re-filtering/re-sorting on every render. Update the `Conversations`
component by introducing a `useMemo`-based `sortedAgentProfiles` near the other
memoized values, using `sortAgentProfiles(agentProfiles, uiLocale)` with
`agentProfiles` and `uiLocale` as dependencies, and then replace the inline call
site with the memoized value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12e09cb6-c347-4b95-90e0-bfb1eae577a7
📒 Files selected for processing (2)
app/src/pages/Conversations.tsxapp/src/pages/__tests__/Conversations.test.tsx
Resolve rename conflict: app/src/pages/Conversations.tsx was moved to app/src/features/conversations/Conversations.tsx on main. Port the PR's AgentProfile import and sortAgentProfiles test import to the new location and corrected relative-path depth (../ -> ../../).
|
Maintainer review changes: merged current
No functional changes to the feature itself; this only re-homes the PR onto the relocated file. Letting CI verify. |
|
Re: CodeRabbit nitpicks (both non-blocking
|
Extract the chat-header profile selector into AgentProfileSelector so the new button JSX can be unit-tested without mounting the full Conversations page (avoids Redux/API/browser-API flakiness). sortAgentProfiles moves with it and is re-exported to keep existing imports resolving. Covers the previously-uncovered custom-profile render path to satisfy the 80% diff coverage gate.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be8c94d133
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type="button" | ||
| role="radio" | ||
| aria-checked={selectedProfileId === profile.id} | ||
| data-analytics-id={`chat-header-mode-${profile.id}`} |
There was a problem hiding this comment.
Use a stable analytics id for custom profiles
When a user clicks a custom profile, the app-wide interaction tracker reads data-analytics-id as the control_id for ui_click events, so interpolating profile.id sends a user-created entity identifier (the profile editor lets users type this id) into analytics. This leaks potentially sensitive/custom account data and creates unbounded event cardinality; use a fixed id such as chat-header-mode-custom and keep the actual profile id out of analytics attributes.
Useful? React with 👍 / 👎.
Summary
The chat header profile selector now surfaces custom (non-built-in) agent
profiles, so users can switch to any profile available on their account — not
just the built-in Default and Reasoning options.
Custom profiles are those with
builtIn: false. They are listed after the twobuilt-in options, sorted by
sortOrder(fallback0) and then by localizedname. The active profile is highlighted (
aria-checked), and selecting onefires
onSelect(profile.id).Changes
Conversationsinto a dedicatedAgentProfileSelectorcomponent(
app/src/features/conversations/components/AgentProfileSelector.tsx).sortAgentProfiles(profiles, locale)helper that filters tonon-built-in profiles and sorts by
sortOrderthenname.localeCompare(..., locale);re-exported from
Conversationsso existing tests still resolve it.(
AgentProfileSelector.test.tsx) covering: custom profiles render withprofile.name, the active profile is markedaria-checked, and selecting itinvokes the handler.
Test plan
yarn test/vitest run— newAgentProfileSelector.test.tsxpasses(i18n mocked).
yarn format:checkandyarn typecheckpass (Frontend Checks / compile lanes).confirm custom profiles appear, the active one is highlighted, and selecting
it updates the active profile.
Linked issue
Part of #3486