Skip to content

📌 fix: Fetch Pinned Chats Independently of the Chats List - #14860

Open
berry-13 wants to merge 6 commits into
devfrom
fix/pinned-section-always-fetch
Open

📌 fix: Fetch Pinned Chats Independently of the Chats List#14860
berry-13 wants to merge 6 commits into
devfrom
fix/pinned-section-always-fetch

Conversation

@berry-13

Copy link
Copy Markdown
Collaborator

Summary

The sidebar's Pinned section filtered pinned chats out of the paginated Chats list. That list holds the 25 most recently updated conversations, so once 25 newer chats existed, a reload hid the pin entirely until you scrolled Chats far enough to fetch the page it lived on.

Pins now come from their own request, GET /api/convos?pinned=true, behind a dedicated usePinnedConversationsQuery. Every pin paints with the sidebar regardless of where it falls in the Chats list. Pin and unpin invalidate that query, and the shared conversation cache helpers (updateConvoInAllQueries, removeConvoFromAllQueries) keep it in step so a rename, delete or archive is reflected without waiting for a refetch.

Pins are a hand-curated, deliberately small set, so the query fetches them whole rather than paginating. They stay out of the Chats date groups, which groupConversationsByDate already handled.

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Testing

Reproduced the original bug and confirmed the fix against a real database: pinned a conversation whose updatedAt is 2026-03-05, rank 45 of 46, so it falls outside page 1 (page 1's oldest is 2026-07-31). GET /api/convos?limit=25 does not contain it; GET /api/convos?pinned=true&limit=100 returns exactly it. In the browser, a full reload renders the Pinned section with that chat on first paint while Chats shows only Today / Previous 7 days / Previous 30 days, and the chat is absent from those groups. Pin and unpin both verified through the UI, in light and dark mode.

Test Configuration:

Local dev server, MongoDB, Chromium.

cd packages/data-schemas && npx jest src/methods/conversation.spec.ts   # 72 passed
cd api && npx jest server/routes/__tests__/convos.spec.js               # 38 passed
cd client && npx jest src/data-provider src/components/Conversations src/components/UnifiedSidebar
cd client && npx tsc --noEmit

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

The sidebar's pinned section filtered pinned chats out of the paginated
chats list, which only holds the 25 most recently updated conversations.
Once 25 newer chats existed, a reload hid the pin until the list was
scrolled far enough to fetch the page it lived on.

Pins are now fetched directly via GET /api/convos?pinned=true behind a
dedicated query, so every pin paints with the sidebar regardless of where
it falls in the chats list. Pin and unpin invalidate that query, and the
shared conversation cache helpers keep it in step so a rename, delete or
archive is reflected without waiting for a refetch.

Pins stay out of the date groups, which groupConversationsByDate already
handled.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 616fcdfbfb

ℹ️ 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".

Comment thread client/src/data-provider/queries.ts Outdated
Comment thread client/src/data-provider/queries.ts Outdated
Comment thread client/src/utils/convos.ts Outdated
Comment thread client/src/components/UnifiedSidebar/ConversationsSection.tsx Outdated
Comment thread packages/data-schemas/src/methods/conversation.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the UnifiedSidebar “Pinned” section so pinned chats are fetched independently from the paginated “Chats” list, ensuring pinned conversations always render on initial load even when they fall outside the first page of recent conversations.

Changes:

  • Added a pinned filter parameter end-to-end (API route → data-schemas query method → data-provider query params/types).
  • Introduced usePinnedConversationsQuery (with its own query key) and wired sidebar rendering to use a dedicated pinned request.
  • Added a new PinnedSection UI component and updated tests to reflect the new pinned rendering path and cache synchronization behavior.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/data-schemas/src/methods/conversation.ts Adds pinned as an optional filter to getConvosByCursor so Mongo queries can return only pinned conversations.
packages/data-schemas/src/methods/conversation.spec.ts Adds unit tests validating pinned filtering behavior (including the “outside first page” scenario).
packages/data-provider/src/types/queries.ts Extends conversation list params/types to include limit and pinned, and includes pinned in MinimalConversation.
packages/data-provider/src/react-query/react-query-service.ts Invalidates pinned conversations query when clearing conversations.
packages/data-provider/src/keys.ts Adds a dedicated pinnedConversations React Query key.
client/src/utils/convos.ts Keeps pinned query cache in sync when conversations are updated/removed via shared cache helpers.
client/src/data-provider/queries.ts Adds usePinnedConversationsQuery and a pinned fetch limit constant.
client/src/data-provider/mutations.ts Invalidates pinned conversations query after pin/unpin mutation and integrates with shared cache update helpers.
client/src/data-provider/tests/pinnedConversations.test.tsx Adds tests for pinned query fetching, pin-triggered refetch, and pinned-cache synchronization.
client/src/components/UnifiedSidebar/ConversationsSection.tsx Uses usePinnedConversationsQuery and renders PinnedSection between Projects and Chats.
client/src/components/UnifiedSidebar/tests/ConversationsSection.spec.tsx Updates sidebar tests to mock pinned query and assert section ordering.
client/src/components/Conversations/PinnedSection.tsx New collapsible pinned conversations section component for the sidebar.
client/src/components/Conversations/Conversations.tsx Removes pinned header/items from the paginated Chats list (pins now live in PinnedSection).
client/src/components/Conversations/tests/PinnedSection.spec.tsx Adds unit tests for PinnedSection rendering and collapse/expand behavior.
client/src/components/Conversations/tests/Conversations.test.tsx Updates tests to assert pinned header is no longer rendered inside Chats list.
api/server/routes/convos.js Adds pinned query parsing and forwards it to getConvosByCursor.
api/server/routes/tests/convos.spec.js Adds route tests verifying pinned forwarding and default behavior when absent.
api/server/routes/test-utils/convos-route-mocks.js Updates isEnabled mock to match real query-flag parsing behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread client/src/data-provider/__tests__/pinnedConversations.test.tsx Outdated
Comment thread client/src/data-provider/mutations.ts
- Drain the cursor rather than capping the pinned request at 100. Since
  pins are kept out of the chats date groups, anything this query dropped
  was invisible in the sidebar entirely, not merely further down a list.
- Apply the active bookmark filter to the pinned request and key its
  cache by it, matching the chats list beside it.
- Move a pin to the top of the section when the caller asks for it, so a
  pin that just received a message leads the way it does in the chats
  list instead of waiting for a refetch.
- Invalidate the pinned list when a conversation is unarchived, since
  archiving removes it from that cache and nothing put it back.
- Index the pinned lookup: it filters on user + pinned and sorts by
  updatedAt, which no existing compound index covered.
- Protect `pinned` from saveMessageToDatabase's unset sweep. Any
  persisted field missing from endpointOptions is unset, so sending a
  message in a pinned chat silently unpinned it.
@berry-13

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed7a22908a

ℹ️ 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".

Comment thread client/src/components/UnifiedSidebar/ConversationsSection.tsx Outdated
Comment thread client/src/data-provider/queries.ts Outdated
Comment thread client/src/components/Conversations/PinnedSection.tsx
Comment thread client/src/utils/convos.ts
Comment thread client/src/components/Conversations/PinnedSection.tsx
@berry-13

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed7a22908a

ℹ️ 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".

Comment thread client/src/data-provider/queries.ts Outdated
Comment thread client/src/data-provider/queries.ts
Second review pass on the independent pinned query.

- Fall back to the pins already loaded in the chats pages when the
  dedicated request fails. Pins are stripped from the date groups, so an
  error otherwise emptied the section and hid them everywhere.
- Restore default focus and reconnect refetching, matching the
  conversations query. A pin changed in another tab is only reconciled by
  a refetch, since that tab's mutation never touched this cache.
- Invalidate the pinned list from the mutations that can produce or alter
  a pinned chat without going through pin itself: duplicate, fork,
  import, project assignment, and shared-link deletion.
@berry-13

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18f369a585

ℹ️ 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".

Comment thread client/src/data-provider/queries.ts
Comment thread client/src/data-provider/Projects/mutations.ts
Third review pass, same class as the last: the pinned query is keyed by
the active bookmark filter, so changing a chat's tags can move it in or
out of that filtered set, and deleting a project unsets chatProjectId on
its chats, pinned ones included.
@berry-13

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c91c118d3

ℹ️ 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".

Comment thread client/src/utils/convos.ts
Deletion cancelled the regular and archived queries but not the pinned
one, so a pinned GET issued before the delete could resolve after the row
was stripped and write the deleted conversation back, leaving a row that
navigates to a missing chat. Restoring default focus and reconnect
refetching in the previous commit made those in-flight fetches more
likely, so this widened rather than appeared.

Cancelled on mutate, and invalidated on success since cancelling a race
is best effort.
The conversation cache helpers now run a second, pinned-keyed findAll
pass. This mock ignored its key argument and always returned an
allConversations entry, so those pinned writes were attributed to
allConversations and the write-count assertions saw three instead of two.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97a6771ea6

ℹ️ 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".

updater: (c: TConversation) => TConversation,
moveToTop = false,
) {
updatePinnedConvosQuery(queryClient, conversationId, updater, moveToTop);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Synchronize pins from conversation upsert paths

When an existing pinned conversation goes through a root-level SSE update or resumable settlement, useEventHandlers and useResumableSSE call upsertConvoInAllQueries rather than this update helper. Because pinned-cache synchronization is only invoked here, those paths reorder or refresh allConversations while leaving the independently cached pinned row at its old position and with stale metadata until a later refetch. Fresh evidence beyond the earlier ordering fix is the remaining upsertConvoInAllQueries calls in those SSE recovery paths; make the upsert helper reconcile existing pinned rows as well. CLAUDE.mdL151-L155

Useful? React with 👍 / 👎.

shouldShowFavorites,
search.query,
]);
}, [groupedConversations, isLoading, isChatsExpanded, shouldShowFavorites]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Continue pagination when a page contains only pins

When the first conversations page consists entirely of pinned chats but has a non-null nextCursor—for example, 25 recently updated pins followed by older unpinned chats—groupConversationsByDate removes every row and this flattened list becomes empty. Since onRowsRendered is the only path that invokes loadMoreConversations, a zero-row react-virtualized list has no rendered row to approach and the older Chats pages remain unreachable. Keep a pagination sentinel while another page exists or proactively fetch again whenever filtering leaves the current result empty. CLAUDE.mdL164-L169

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants