Repository navigation
fix(chat): single onboarding thread in sidebar after wizard - #1116
Merged
senamakel merged 9 commits intoMay 3, 2026
Merged
Conversation
- Tag welcome thread with onboarding label at creation - Avoid createNewThread when loadThreads is empty but welcomeThreadId exists - Filter sidebar to welcome thread during lock; label as Onboarding - Prefer welcomeThreadId for proactive:* routing to prevent orphan threads - Show slim header during lock (sidebar toggle + title) Co-authored-by: Steven Enamakel <senamakel@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughAdds an onboarding welcome-thread label constant and threads that label into onboarding completion, Conversations selection/filtering/UI lockdown, and proactive thread resolution so a labeled welcome thread is created, prioritized, displayed, and protected during welcome lockdown. ChangesWelcome Lockdown / Onboarding Welcome Thread
Sequence Diagram(s)sequenceDiagram
participant Onboard as OnboardingLayout
participant Store as Redux Store
participant UI as Conversations (Sidebar/Header)
participant Runtime as ChatRuntimeProvider
Onboard->>Store: threadApi.createNewThread([ONBOARDING_WELCOME_THREAD_LABEL])
Store-->>Onboard: new thread id (welcomeThreadId)
Onboard->>Store: setSelectedThread(welcomeThreadId)
Onboard->>UI: navigate("/home")
UI->>Store: loadThreads()
Store-->>UI: threads (includes welcomeThreadId)
UI->>Store: isWelcomeLocked? -> true
UI->>Store: select welcomeThreadId, load messages
Runtime->>Store: resolveVisibleThreadForProactive()
Store-->>Runtime: includes welcomeThreadId as candidate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 3/5 reviews remaining, refill in 13 minutes and 49 seconds. 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: 2
🤖 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 241-250: The welcome-lock flow currently lets a non-welcome thread
get selected when server threads are non-empty; update the guard that uses
isWelcomeLocked(snap) and threadState.welcomeThreadId so it always forces
selection of the welcome thread and aborts further thread selection/creation
regardless of loadThreads() results: inside the Conversations
mounting/initialization logic replace the conditional so that if
isWelcomeLocked(getCoreStateSnapshot().snapshot) &&
store.getState().thread.welcomeThreadId you immediately dispatch
setSelectedThread(store.getState().thread.welcomeThreadId) and dispatch
loadThreadMessages(...) and return, ensuring this check runs before any other
code that picks or creates threads (referenced symbols: isWelcomeLocked,
getCoreStateSnapshot, store.getState().thread.welcomeThreadId,
setSelectedThread, loadThreadMessages).
- Around line 810-821: The current filter applies selectedLabel before welcome
lockdown, causing the onboarding welcome thread to be dropped if it doesn't
match the previously selected label; change the filter order so welcome lockdown
handling runs first: when welcomeLocked is true, narrow threads to either the
specific welcomeThreadId (if present) or exclude threads with
ONBOARDING_WELCOME_THREAD_LABEL, and only then apply selectedLabel filtering (or
ensure the welcome thread is always included regardless of selectedLabel).
Update the block that computes `base`/filtered threads (the memoized selector
using `threads`, `selectedLabel`, `welcomeLocked`, `welcomeThreadId`) to perform
welcome lock logic before/independently of `selectedLabel`.
🪄 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: 6c16a7a9-3d0e-4135-9608-5f305004c889
📒 Files selected for processing (4)
app/src/constants/onboardingChat.tsapp/src/pages/Conversations.tsxapp/src/pages/onboarding/OnboardingLayout.tsxapp/src/providers/ChatRuntimeProvider.tsx
…aviour - Invert fallback filter in `filteredThreads`: when `welcomeThreadId` is null but `welcomeLocked` is true, keep only `'onboarding'`-labelled threads (was incorrectly hiding them, causing an empty sidebar on hot-reload). - Guard the non-empty `loadThreads` path against selecting a stale prior-session thread during welcome lockdown — now both the empty and non-empty server responses honour `welcomeThreadId` before falling back. - Extract `resolveThreadDisplayTitle` helper so the "Onboarding" label logic is defined once and shared by the sidebar list and the header title. - Clamp `showSidebar` to `true` while locked (`effectiveShowSidebar`) so the sidebar toggle cannot leave the user with no thread list during onboarding. - Add explanatory comment to `resolveVisibleThreadForProactive` fallback chain. - Improve JSDoc on `ONBOARDING_WELCOME_THREAD_LABEL` to clarify thread lifecycle.
…1116 diff-cover gate Add Vitest unit tests for the welcome-lockdown features added in a21786b (filteredThreads, resolveThreadDisplayTitle, effectiveShowSidebar, delete-button guard) and the OnboardingLayout createNewThread label change (line 97). Exercises all diff-cover-reported missing lines in Conversations.tsx and OnboardingLayout.tsx to bring changed-line coverage above the 80% merge gate.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/__tests__/Conversations.welcomeLock.test.tsx`:
- Around line 230-272: The sidebar assertions are currently global and can be
satisfied by the header; update the two tests that call renderConversations to
scope queries to the thread list (e.g., use the thread-list region rendered by
renderConversations or a test-id like "thread-list") by grabbing that container
(screen.getByTestId('thread-list') or the element returned by
renderConversations) and then use within(container).getAllByText('Onboarding') /
within(container).getAllByText('Labelled Welcome') or count rendered thread
items, and assert that within(container).queryByText('Other') /
within(container).queryByText('Plain') is not in the document; keep using
renderConversations, welcomeLocked, welcomeThreadId and
ONBOARDING_WELCOME_THREAD_LABEL as the locating symbols.
In `@app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx`:
- Around line 139-168: The tests race the fire-and-forget completeAndExit flow;
after clicking the element with test id 'complete-btn' wait for the async work
to finish before asserting: use testing-library's waitFor (or await a promise
that resolves when createNewThread/setOnboardingCompletedFlag/Redux dispatch
completes) to assert that mockCreateNewThreadArg was called with
[ONBOARDING_WELCOME_THREAD_LABEL], that mockSetOnboardingCompletedFlag was
called with true, and that store.getState().thread.welcomeThreadId equals
'welcome-thread-id'—target the createNewThread spy,
mockSetOnboardingCompletedFlag, and the Redux state (thread.welcomeThreadId) in
your waitFor assertions so the tests no longer race the async operations.
🪄 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: 0abd96ca-1e07-4c2d-a7f4-a3e2191e1c03
📒 Files selected for processing (2)
app/src/pages/__tests__/Conversations.welcomeLock.test.tsxapp/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx
Comment on lines
+230
to
+272
| it('shows only the welcome thread when welcomeLocked=true and welcomeThreadId is set', async () => { | ||
| const welcomeThread = makeThread({ | ||
| id: 'wt-1', | ||
| title: 'Welcome', | ||
| labels: [ONBOARDING_WELCOME_THREAD_LABEL], | ||
| }); | ||
| const otherThread = makeThread({ id: 'other-1', title: 'Other' }); | ||
|
|
||
| await renderConversations({ | ||
| welcomeLocked: true, | ||
| threads: [welcomeThread, otherThread], | ||
| selectedThreadId: 'wt-1', | ||
| welcomeThreadId: 'wt-1', | ||
| }); | ||
|
|
||
| // The welcome thread title is replaced by "Onboarding" — see resolveThreadDisplayTitle. | ||
| // It may appear in both the sidebar list and the header (getAllByText handles multiples). | ||
| expect(screen.getAllByText('Onboarding').length).toBeGreaterThanOrEqual(1); | ||
| // The other thread must not appear | ||
| expect(screen.queryByText('Other')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('falls back to onboarding-labelled threads when welcomeThreadId is null but welcomeLocked=true', async () => { | ||
| const labelledThread = makeThread({ | ||
| id: 'wt-2', | ||
| title: 'Labelled Welcome', | ||
| labels: [ONBOARDING_WELCOME_THREAD_LABEL], | ||
| }); | ||
| const unlabelledThread = makeThread({ id: 'plain-1', title: 'Plain' }); | ||
|
|
||
| await renderConversations({ | ||
| welcomeLocked: true, | ||
| threads: [labelledThread, unlabelledThread], | ||
| selectedThreadId: 'wt-2', | ||
| welcomeThreadId: null, // not yet set | ||
| }); | ||
|
|
||
| // Labelled thread title is NOT replaced (welcomeThreadId is null, so the | ||
| // label-only guard runs — it doesn't rename to "Onboarding"). | ||
| // getAllByText handles potential multi-occurrence (sidebar + header). | ||
| expect(screen.getAllByText('Labelled Welcome').length).toBeGreaterThanOrEqual(1); | ||
| expect(screen.queryByText('Plain')).not.toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Scope the sidebar assertions to the thread list.
getAllByText('Onboarding') / getAllByText('Labelled Welcome') can pass because the header renders the same title, even if filteredThreads is broken. Please assert against the sidebar/thread-list region or count the rendered thread items directly.
As per coding guidelines, prefer behavior over implementation details in app/src/**/*.test.tsx.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx` around lines 230
- 272, The sidebar assertions are currently global and can be satisfied by the
header; update the two tests that call renderConversations to scope queries to
the thread list (e.g., use the thread-list region rendered by
renderConversations or a test-id like "thread-list") by grabbing that container
(screen.getByTestId('thread-list') or the element returned by
renderConversations) and then use within(container).getAllByText('Onboarding') /
within(container).getAllByText('Labelled Welcome') or count rendered thread
items, and assert that within(container).queryByText('Other') /
within(container).queryByText('Plain') is not in the document; keep using
renderConversations, welcomeLocked, welcomeThreadId and
ONBOARDING_WELCOME_THREAD_LABEL as the locating symbols.
Comment on lines
+139
to
+168
| it('calls createNewThread with the onboarding welcome label on completeAndExit', async () => { | ||
| await setupLayout(); | ||
|
|
||
| await act(async () => { | ||
| fireEvent.click(screen.getByTestId('complete-btn')); | ||
| }); | ||
|
|
||
| expect(mockCreateNewThreadArg).toHaveBeenCalledWith([ONBOARDING_WELCOME_THREAD_LABEL]); | ||
| }); | ||
|
|
||
| it('calls setOnboardingCompletedFlag(true) during completeAndExit', async () => { | ||
| const { mockSetOnboardingCompletedFlag } = await setupLayout(); | ||
|
|
||
| await act(async () => { | ||
| fireEvent.click(screen.getByTestId('complete-btn')); | ||
| }); | ||
|
|
||
| expect(mockSetOnboardingCompletedFlag).toHaveBeenCalledWith(true); | ||
| }); | ||
|
|
||
| it('records the welcome thread id in the Redux store after thread creation', async () => { | ||
| const { store } = await setupLayout(); | ||
|
|
||
| await act(async () => { | ||
| fireEvent.click(screen.getByTestId('complete-btn')); | ||
| }); | ||
|
|
||
| // The dispatch(setWelcomeThreadId(newThread.id)) should have updated state | ||
| const { thread } = store.getState() as { thread: { welcomeThreadId: string | null } }; | ||
| expect(thread.welcomeThreadId).toBe('welcome-thread-id'); |
Contributor
There was a problem hiding this comment.
Wait for the async onboarding flow before asserting.
completeAndExit() is fire-and-forget here, so these expectations can race the createNewThread / Redux updates. Please wait for the promise chain to settle before checking the spy or the store.
Suggested fix
-import { act, fireEvent, render, screen } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
...
- await act(async () => {
- fireEvent.click(screen.getByTestId('complete-btn'));
- });
-
- expect(mockCreateNewThreadArg).toHaveBeenCalledWith([ONBOARDING_WELCOME_THREAD_LABEL]);
+ fireEvent.click(screen.getByTestId('complete-btn'));
+ await waitFor(() =>
+ expect(mockCreateNewThreadArg).toHaveBeenCalledWith([ONBOARDING_WELCOME_THREAD_LABEL])
+ );📝 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
| it('calls createNewThread with the onboarding welcome label on completeAndExit', async () => { | |
| await setupLayout(); | |
| await act(async () => { | |
| fireEvent.click(screen.getByTestId('complete-btn')); | |
| }); | |
| expect(mockCreateNewThreadArg).toHaveBeenCalledWith([ONBOARDING_WELCOME_THREAD_LABEL]); | |
| }); | |
| it('calls setOnboardingCompletedFlag(true) during completeAndExit', async () => { | |
| const { mockSetOnboardingCompletedFlag } = await setupLayout(); | |
| await act(async () => { | |
| fireEvent.click(screen.getByTestId('complete-btn')); | |
| }); | |
| expect(mockSetOnboardingCompletedFlag).toHaveBeenCalledWith(true); | |
| }); | |
| it('records the welcome thread id in the Redux store after thread creation', async () => { | |
| const { store } = await setupLayout(); | |
| await act(async () => { | |
| fireEvent.click(screen.getByTestId('complete-btn')); | |
| }); | |
| // The dispatch(setWelcomeThreadId(newThread.id)) should have updated state | |
| const { thread } = store.getState() as { thread: { welcomeThreadId: string | null } }; | |
| expect(thread.welcomeThreadId).toBe('welcome-thread-id'); | |
| import { fireEvent, render, screen, waitFor } from '@testing-library/react'; | |
| ... | |
| it('calls createNewThread with the onboarding welcome label on completeAndExit', async () => { | |
| await setupLayout(); | |
| fireEvent.click(screen.getByTestId('complete-btn')); | |
| await waitFor(() => | |
| expect(mockCreateNewThreadArg).toHaveBeenCalledWith([ONBOARDING_WELCOME_THREAD_LABEL]) | |
| ); | |
| }); | |
| it('calls setOnboardingCompletedFlag(true) during completeAndExit', async () => { | |
| const { mockSetOnboardingCompletedFlag } = await setupLayout(); | |
| fireEvent.click(screen.getByTestId('complete-btn')); | |
| await waitFor(() => | |
| expect(mockSetOnboardingCompletedFlag).toHaveBeenCalledWith(true) | |
| ); | |
| }); | |
| it('records the welcome thread id in the Redux store after thread creation', async () => { | |
| const { store } = await setupLayout(); | |
| fireEvent.click(screen.getByTestId('complete-btn')); | |
| await waitFor(() => { | |
| // The dispatch(setWelcomeThreadId(newThread.id)) should have updated state | |
| const { thread } = store.getState() as { thread: { welcomeThreadId: string | null } }; | |
| expect(thread.welcomeThreadId).toBe('welcome-thread-id'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx` around lines
139 - 168, The tests race the fire-and-forget completeAndExit flow; after
clicking the element with test id 'complete-btn' wait for the async work to
finish before asserting: use testing-library's waitFor (or await a promise that
resolves when createNewThread/setOnboardingCompletedFlag/Redux dispatch
completes) to assert that mockCreateNewThreadArg was called with
[ONBOARDING_WELCOME_THREAD_LABEL], that mockSetOnboardingCompletedFlag was
called with true, and that store.getState().thread.welcomeThreadId equals
'welcome-thread-id'—target the createNewThread spy,
mockSetOnboardingCompletedFlag, and the Redux state (thread.welcomeThreadId) in
your waitFor assertions so the tests no longer race the async operations.
… gate Add targeted Vitest tests for lines 243-245 (loadThreads welcome-lock guard), 868 (resolveThreadDisplayTitle label mismatch fallback), 892/1037 (new-thread button onClick handlers), 955-967 (delete button click → modal state), and 1018/1020 (sidebar toggle button including title flip) in Conversations.tsx. Tests extend the existing welcomeLock suite without touching production code.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/pages/__tests__/Conversations.welcomeLock.test.tsx (1)
245-250:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winScope sidebar assertions to the thread-list container.
These checks can pass from header text alone, so they don’t reliably prove sidebar filtering behavior. Please scope to the list region (e.g.,
within(threadList)), then assert inclusion/exclusion there.As per coding guidelines,
app/src/**/*.test.{ts,tsx}should “prefer testing behavior over implementation details.”Also applies to: 267-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx` around lines 245 - 250, The sidebar assertions in Conversations.welcomeLock.test.tsx currently use screen.getAllByText/screen.queryByText which can match header text; update the two checks to scope them to the thread list container by finding the thread list element (e.g., const threadList = screen.getByTestId('thread-list') or the appropriate selector) and use within(threadList).getAllByText('Onboarding') and within(threadList).queryByText('Other') so the includes/excludes assert only the sidebar list; apply the same change to the analogous assertions at the other location referenced (around the other block at lines ~267-271).
🧹 Nitpick comments (2)
app/src/pages/__tests__/Conversations.welcomeLock.test.tsx (2)
70-70: ⚡ Quick winReplace inline object-shape type literals with named interfaces.
There are several inline object-shape types in function signatures/props; please move these to named
interfaces for consistency with repo conventions.As per coding guidelines,
**/*.{ts,tsx}says to “preferinterfacefor defining object shapes in TypeScript.”Also applies to: 141-145, 167-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx` at line 70, Replace the inline object-shape type literals with named interfaces: define an Item interface (with label: string and value: string) and an ItemsProps interface (items: Item[]) and update the default exported function signature currently written as ({ items }: { items: { label: string; value: string }[] }) => (…) to use ItemsProps instead; apply the same change to the other inline types referenced around the test file (the inline prop types at the blocks noted near lines 141-145 and 167-172) so all function/component props use the named interfaces.
340-341: ⚡ Quick winAvoid class-based
querySelectorassertions for header title tests.Using
document.querySelector('h3.text-sm.font-medium')is brittle. Prefer role/name queries (getByRole) so tests validate user-visible behavior, not styling structure.Suggested update
- const headerEl = document.querySelector('h3.text-sm.font-medium'); - expect(headerEl?.textContent).toBe('Onboarding'); + expect(screen.getByRole('heading', { level: 3, name: 'Onboarding' })).toBeInTheDocument(); - const headerEl = document.querySelector('h3.text-sm.font-medium'); - expect(headerEl?.textContent).toBe('Select a thread'); + expect(screen.getByRole('heading', { level: 3, name: 'Select a thread' })).toBeInTheDocument();As per coding guidelines,
app/src/**/*.test.{ts,tsx}should “prefer testing behavior over implementation details.”Also applies to: 353-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx` around lines 340 - 341, The test uses a brittle class-based DOM query (headerEl via document.querySelector) to assert the header text; replace it with an accessible role/name query such as getByRole or within(...).getByRole targeting a heading (e.g., heading level or name "Onboarding") so the assertion checks visible text not classes—update the assertion that references headerEl and make the same change for the similar check at the other location noted (lines 353-354) to use getByRole('heading', { name: /Onboarding/i }) or equivalent.
🤖 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/__tests__/Conversations.welcomeLock.test.tsx`:
- Around line 659-662: The test currently only verifies the button renders;
update it to also assert the new-thread side effect by mocking/spying on
threadApi.createNewThread and verifying its call count increases after clicking
newBtn (use the existing newBtn variable and the waitFor block that follows the
click). Specifically, before simulating the click, set up a jest.spyOn or mock
for threadApi.createNewThread (or ensure the mock is reset), then fire the click
on newBtn and use waitFor/expect to assert threadApi.createNewThread was called
(or callCount increased) to validate behavior rather than just "no crash".
---
Duplicate comments:
In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx`:
- Around line 245-250: The sidebar assertions in
Conversations.welcomeLock.test.tsx currently use
screen.getAllByText/screen.queryByText which can match header text; update the
two checks to scope them to the thread list container by finding the thread list
element (e.g., const threadList = screen.getByTestId('thread-list') or the
appropriate selector) and use within(threadList).getAllByText('Onboarding') and
within(threadList).queryByText('Other') so the includes/excludes assert only the
sidebar list; apply the same change to the analogous assertions at the other
location referenced (around the other block at lines ~267-271).
---
Nitpick comments:
In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx`:
- Line 70: Replace the inline object-shape type literals with named interfaces:
define an Item interface (with label: string and value: string) and an
ItemsProps interface (items: Item[]) and update the default exported function
signature currently written as ({ items }: { items: { label: string; value:
string }[] }) => (…) to use ItemsProps instead; apply the same change to the
other inline types referenced around the test file (the inline prop types at the
blocks noted near lines 141-145 and 167-172) so all function/component props use
the named interfaces.
- Around line 340-341: The test uses a brittle class-based DOM query (headerEl
via document.querySelector) to assert the header text; replace it with an
accessible role/name query such as getByRole or within(...).getByRole targeting
a heading (e.g., heading level or name "Onboarding") so the assertion checks
visible text not classes—update the assertion that references headerEl and make
the same change for the similar check at the other location noted (lines
353-354) to use getByRole('heading', { name: /Onboarding/i }) or equivalent.
🪄 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: 3c9a646f-97e6-4796-aba5-f191808b894b
📒 Files selected for processing (1)
app/src/pages/__tests__/Conversations.welcomeLock.test.tsx
Comment on lines
+659
to
+662
| // createNewThread may be called asynchronously; just verify no crash | ||
| await waitFor(() => { | ||
| expect(newBtn).toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
Assert the new-thread side effect, not just “no crash.”
These tests currently pass even if the click handler is disconnected. Please assert that threadApi.createNewThread call count increases after click.
Suggested assertion pattern
+ const callsBefore = vi.mocked(threadApi.createNewThread).mock.calls.length;
fireEvent.click(newBtn);
- await waitFor(() => {
- expect(newBtn).toBeInTheDocument();
- });
+ await waitFor(() => {
+ expect(vi.mocked(threadApi.createNewThread).mock.calls.length).toBeGreaterThan(callsBefore);
+ });As per coding guidelines, app/src/**/*.test.{ts,tsx} should “prefer testing behavior over implementation details.”
Also applies to: 713-715
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/pages/__tests__/Conversations.welcomeLock.test.tsx` around lines 659
- 662, The test currently only verifies the button renders; update it to also
assert the new-thread side effect by mocking/spying on threadApi.createNewThread
and verifying its call count increases after clicking newBtn (use the existing
newBtn variable and the waitFor block that follows the click). Specifically,
before simulating the click, set up a jest.spyOn or mock for
threadApi.createNewThread (or ensure the mock is reset), then fire the click on
newBtn and use waitFor/expect to assert threadApi.createNewThread was called (or
callCount increased) to validate behavior rather than just "no crash".
Contributor
There was a problem hiding this comment.
@senamakel-droid Understood, I'll drop this suggestion. 👍
😔 ( o_o )
🧠 Learnings used
Learnt from: graycyrus
Repo: tinyhumansai/openhuman PR: 167
File: app/src/components/OnboardingOverlay.tsx:30-39
Timestamp: 2026-04-01T13:52:17.936Z
Learning: In React components, do not perform synchronous `setState` (or other state-updating calls) directly inside `useEffect` bodies. The lint rule `react-hooks/set-state-in-effect` (enforced in this codebase) disallows patterns such as resetting state flags at the top of an effect before returning. Refactor to avoid calling state setters in the effect; prefer deriving state from props/render, using refs, or restructuring logic so state updates occur outside `useEffect`.
Learnt from: graycyrus
Repo: tinyhumansai/openhuman PR: 1055
File: app/src/pages/__tests__/Welcome.test.tsx:75-78
Timestamp: 2026-04-30T15:15:38.496Z
Learning: In this repo’s Vitest environment, `IS_DEV` from `app/src/utils/config` (which is `import.meta.env.DEV`) evaluates to `true`. Therefore, tests that assert `IS_DEV=true` behavior should be deterministic and generally do not need to explicitly mock `IS_DEV`.
Avoid testing the `IS_DEV=false` path by factory-mocking `../../utils/config` (or otherwise mocking `IS_DEV` to `false`) in test files that use module-level mocks, because that can break the jsdom environment setup chain (as seen in `app/src/pages/__tests__/Welcome.test.tsx`). Instead, validate the `IS_DEV=false` behavior via build-time/tree-shaking (i.e., confirm the `IS_DEV && ...` block is eliminated at build time), rather than by runtime mocking in those tests.
9 tasks done
jwalin-shah
added a commit
to jwalin-shah/openhuman
that referenced
this pull request
May 5, 2026
* feat(remotion): Ghosty character library with transparent MOV variants (tinyhumansai#1059) Co-authored-by: WOZCODE <contact@withwoz.com> * feat(composio/gmail): sync into memory tree (Slack-parity) (tinyhumansai#1056) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(scheduler-gate): throttle background AI on battery / busy CPU (tinyhumansai#1062) * fix(core,cef): run core in-process and stop orphaning CEF helpers on Cmd+Q (tinyhumansai#1061) * ci: add dedicated staging release workflow (tinyhumansai#1066) * fix(sentry): Rust source context + per-release deploy marker (tinyhumansai#405) (tinyhumansai#1067) * fix(welcome): re-enable OAuth buttons with focus/timeout recovery (tinyhumansai#1049) (tinyhumansai#1069) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(dependencies): update pnpm-lock.yaml and Cargo.lock for package… (tinyhumansai#1082) * fix(onboarding): personalize welcome agent greeting with user identity (tinyhumansai#1078) * fix(chat): make agent message bubbles fit content width (tinyhumansai#1083) * Feat/dmg checks (tinyhumansai#1084) * fix(linux): Add X11 platform flags to .deb package launcher (tinyhumansai#1087) Co-authored-by: unn-Known1 <unn-known1@users.noreply.github.com> * fix(sentry): auto-send React events; collapse core→tauri for desktop (tinyhumansai#1086) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * fix(cef): run blank reload guard on the CEF UI thread (tinyhumansai#1092) * fix(app): reload webview instead of restart_app in dev mode (tinyhumansai#1068) (tinyhumansai#1071) * fix(linux): deliver X11 ozone flags via custom .desktop template (tinyhumansai#1091) * fix(webview-accounts): retry data-dir purge so CEF handle race doesn't leak cookies (tinyhumansai#1076) (tinyhumansai#1081) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * fix(webview/slack): media perms + deep-link isolation (tinyhumansai#1074) (tinyhumansai#1080) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * ci(release): split staging vs production workflows; promote staging tags (tinyhumansai#1094) * Update release-staging.yml (tinyhumansai#1097) * chore(staging): v0.53.5 * chore(staging): v0.53.6 * ci(staging): cut staging from main; add act local-debug helper (tinyhumansai#1099) * chore(staging): v0.53.7 * fix(ci): correct sentry-cli download URL and trap scope (tinyhumansai#1100) * chore(staging): v0.53.8 * feat(chat): forward thread_id to backend for KV cache locality (tinyhumansai#1095) * fix(ci): bump pinned sentry-cli to 3.4.1 (2.34.2 was never published) (tinyhumansai#1102) * chore(staging): v0.53.9 * fix(ci): drop bash trap in upload_sentry_symbols.sh; inline cleanup (tinyhumansai#1103) * chore(staging): v0.53.10 * refactor(session): flatten session_raw/, switch md to YYYY_MM_DD (tinyhumansai#1098) * Add full Composio managed-auth toolkit catalog (tinyhumansai#1093) * ci: add diff-aware 80% coverage gate (Vitest + cargo-llvm-cov) (tinyhumansai#1104) * feat(scripts): pnpm work + pnpm debug for agent-driven workflows (tinyhumansai#1105) * ci: pull pnpm into CI image, drop redundant setup steps (tinyhumansai#1107) * docs: add Cursor Cloud specific instructions to AGENTS.md (tinyhumansai#1106) Co-authored-by: Cursor Agent <cursoragent@cursor.com> * chore(staging): v0.53.11 * docs: surface 80% coverage gate and scripts/debug runners (tinyhumansai#1108) * feat(app): show Composio integrations as sorted icon grid on Skills (tinyhumansai#1109) Co-authored-by: Cursor Agent <cursoragent@cursor.com> * feat(composio): client-side trigger enable/disable toggles (tinyhumansai#1110) * feat(skills): channels grid + integrations card polish; tolerant Composio trigger decode (tinyhumansai#1112) * chore(staging): v0.53.12 * feat(home): early-bird banner + assistant→agent terminology (tinyhumansai#1113) * feat(updater): in-app auto-update with auto-download + restart prompt (tinyhumansai#677) (tinyhumansai#1114) * chore(claude): add ship-and-babysit slash command (tinyhumansai#1115) * feat(home): EarlyBirdyBanner + agent terminology + LinkedIn enrichment model pin (tinyhumansai#1118) * fix(chat): single onboarding thread in sidebar after wizard (tinyhumansai#1116) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Steven Enamakel <senamakel@users.noreply.github.com> * fix: filter out global namespace from citation chips (tinyhumansai#1124) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: senamakel-droid <281415773+senamakel-droid@users.noreply.github.com> * feat(nav): enable Memory tab in BottomTabBar (tinyhumansai#1125) * feat(memory): singleton ingestion + status RPC + UI pill (tinyhumansai#1126) * feat(human): mascot tab with viseme-driven lipsync (staging only) (tinyhumansai#1127) * Fix CEF zombie processes on full app close and restart (tinyhumansai#1128) Co-authored-by: senamakel-droid <281415773+senamakel-droid@users.noreply.github.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * Update issue templates for GitHub issue types (tinyhumansai#1146) * feat(human): expand mascot expressions and tighten reply-speech state machine (tinyhumansai#1147) * feat(memory): ingestion pipeline + tree-architecture docs + ops/schemas split (tinyhumansai#1142) * feat(threads): surface live subagent work in parent thread (tinyhumansai#1122) (tinyhumansai#1159) * fix(human): keep mascot mouth animating when TTS ships no viseme data (tinyhumansai#1160) * feat(composio): consume backend markdownFormatted for LLM output (tinyhumansai#1165) * fix(subagent): lazy-register toolkit actions filtered out of fuzzy top-K (tinyhumansai#1162) * feat(memory): user-facing long-term memory window preset (tinyhumansai#1137) (tinyhumansai#1161) * fix(tauri-shell): proactively kill stale openhuman RPC on startup (tinyhumansai#1166) * chore(staging): v0.53.13 * fix(composio): per-action tool consumes backend markdownFormatted (tinyhumansai#1167) * fix(threads): persist selectedThreadId across reloads (tinyhumansai#1168) * feat(memory_tree): switch embed model to bge-m3 (1024-dim, 8K context) (tinyhumansai#1174) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): drop redundant [Memory context] recall injection (tinyhumansai#1173) * chore(memory_tree): drop body-read timeouts on Ollama HTTP calls (tinyhumansai#1171) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(transcript): emit thread_id + fix orchestrator missing cost (tinyhumansai#1169) * fix(composio/gmail): phase out html2md, prefer text/plain MIME part (tinyhumansai#1170) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tools): markdown output for internal tool results (tinyhumansai#1172) * feat(security): enforce prompt-injection guard before model and tool execution (tinyhumansai#1175) * fix(cef): popup paint dies after first frame — skip blank-page guard for popups (tinyhumansai#1079) (tinyhumansai#1182) Co-authored-by: Steven Enamakel <31011319+senamakel@users.noreply.github.com> * chore(sentry): rename OPENHUMAN_SENTRY_DSN → OPENHUMAN_CORE_SENTRY_DSN (tinyhumansai#1186) * feat(remotion): add yellow mascot character with all animation variants (tinyhumansai#1193) Co-authored-by: Neel Mistry <neelmistry@Neels-MacBook-Pro.local> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(composio): hide raw connection ID, derive friendly label (tinyhumansai#1153) (tinyhumansai#1185) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * fix(windows): align install.ps1 MSI with per-machine scope (tinyhumansai#913) (tinyhumansai#1187) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tauri): deterministic CEF teardown on full app close (tinyhumansai#1120) (tinyhumansai#1189) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(composio): cap Gmail HTML body before strip (crash mitigation) (tinyhumansai#1191) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(auth): stop stale chat threads after signup (tinyhumansai#1192) Co-authored-by: Cursor <cursoragent@cursor.com> * feat(sentry): staging-only "Trigger Sentry Test" button (tinyhumansai#1072) (tinyhumansai#1183) * chore(staging): v0.53.14 * chore(staging): v0.53.15 * feat(composio): format trigger slugs into human-readable labels (tinyhumansai#1129) (tinyhumansai#1179) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * fix(ui): hide unsupported permission UI on non-macOS for Screen Intelligence (tinyhumansai#1194) Co-authored-by: Cursor <cursoragent@cursor.com> * chore(tauri-shell): retire embedded Gmail webview-account flow (tinyhumansai#1181) * feat(onboarding): replace welcome-agent bot with react-joyride walkthrough (tinyhumansai#1180) * chore(release): v0.53.16 * fix(threads): preserve selectedThreadId on cold-boot identity hydration (tinyhumansai#1196) * feat(core): version/shutdown/update RPCs + mid-thread integration refresh (tinyhumansai#1195) * fix(mascot): swap to yellow mascot via @remotion/player (tinyhumansai#1200) * feat(memory_tree): cloud-default LLM, queue priority, entity filter, Memory tab UI (tinyhumansai#1198) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Persist turn state + restore conversation history on cold-boot (tinyhumansai#1202) * feat(mascot): floating desktop mascot via native NSPanel + WKWebView (macOS) (tinyhumansai#1203) * fix(memory/tree): emit summary children as Obsidian wikilinks (tinyhumansai#1210) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tools): coding-harness baseline primitives (tinyhumansai#1205) (tinyhumansai#1208) * docs: add Codex PR checklist for remote agents --------- Co-authored-by: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Co-authored-by: WOZCODE <contact@withwoz.com> Co-authored-by: sanil-23 <sanil@vezures.xyz> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Co-authored-by: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Co-authored-by: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Co-authored-by: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Co-authored-by: Gaurang Patel <ptelgm.yt@gmail.com> Co-authored-by: unn-Known1 <unn-known1@users.noreply.github.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Steven Enamakel <senamakel@users.noreply.github.com> Co-authored-by: Steven Enamakel's Droid <enamakel.agent@tinyhumans.ai> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: senamakel-droid <281415773+senamakel-droid@users.noreply.github.com> Co-authored-by: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Co-authored-by: Neil <neil@maha.xyz> Co-authored-by: Neel Mistry <neelmistry@Neels-MacBook-Pro.local> Co-authored-by: obchain <167975049+obchain@users.noreply.github.com> Co-authored-by: Jwalin Shah <jshah1331@gmail.com>
6 tasks done
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
…nsai#1116) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Steven Enamakel <senamakel@users.noreply.github.com>
senamakel
added a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 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
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
Problem
Solution
Submission Checklist
docs/TESTING-STRATEGY.mddiff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge.docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change)## Relateddocs/TESTING-STRATEGY.md)docs/RELEASE-MANUAL-SMOKE.md)Closes #NNNin the## RelatedsectionImpact
Related
Summary by CodeRabbit
New Features
Behavior Changes
Tests