perf: Flip the Pinned Flag Without a Full Conversation Save (#14862)

Pinning routed through saveConvo, which reads every message id for the
conversation and writes the whole array back just to set one boolean, and
can trigger a project-stats recompute on top.

None of that applies to a pin: it moves no chat between projects, changes
nothing the project workspace hides, and opens no retention window. A
dedicated setConvoPinned does the single findOneAndUpdate instead.

Measured against an in-memory MongoDB with the real message methods
wired in, on a 120-message chat: two driver commands and 3706 bytes
before, one command and 245 bytes after. The write scales with the
message count, so the gap widens on longer chats.

Archiving keeps using saveConvo, which it needs for exactly the project
stats and retention work a pin does not.
This commit is contained in:
Marco Beretta 2026-08-16 23:07:27 +02:00 committed by GitHub
parent 5d88b8453e
commit 7857a99d63
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 143 additions and 63 deletions

View file

@ -65,6 +65,7 @@ module.exports = {
getConvo: jest.fn(),
deleteConvos: jest.fn(),
saveConvo: jest.fn(),
setConvoPinned: jest.fn(),
deleteAllSharedLinks: jest.fn(),
deleteConvoSharedLink: jest.fn(),
deleteToolCalls: jest.fn(),

View file

@ -659,49 +659,45 @@ describe('Convos Routes', () => {
describe('POST /convos/pin', () => {
const mockConversationId = 'conv-123';
const { setConvoPinned } = require('~/models');
it('should pin a conversation', async () => {
const mockPinnedConvo = { conversationId: mockConversationId, pinned: true };
saveConvo.mockResolvedValue(mockPinnedConvo);
setConvoPinned.mockResolvedValue(mockPinnedConvo);
const response = await request(app).post('/api/convos/pin').send({ arg: mockPinnedConvo });
expect(response.status).toBe(200);
expect(response.body).toEqual(mockPinnedConvo);
expect(saveConvo).toHaveBeenCalledWith(
{ userId: 'test-user-123' },
{ conversationId: mockConversationId, pinned: true },
{
context: `POST /api/convos/pin ${mockConversationId}`,
preserveUpdatedAt: true,
noUpsert: true,
},
);
expect(setConvoPinned).toHaveBeenCalledWith('test-user-123', mockConversationId, true);
});
/** A pin is one boolean: it must not drag in `saveConvo`'s message-id refresh
* and project-stats recompute, which cost an extra read and a large write. */
it('does not route a pin through the full conversation save', async () => {
setConvoPinned.mockResolvedValue({ conversationId: mockConversationId, pinned: true });
await request(app)
.post('/api/convos/pin')
.send({ arg: { conversationId: mockConversationId, pinned: true } });
expect(setConvoPinned).toHaveBeenCalledTimes(1);
expect(saveConvo).not.toHaveBeenCalled();
});
it('should unpin a conversation', async () => {
const mockUnpinnedConvo = { conversationId: mockConversationId, pinned: false };
saveConvo.mockResolvedValue(mockUnpinnedConvo);
setConvoPinned.mockResolvedValue(mockUnpinnedConvo);
const response = await request(app).post('/api/convos/pin').send({ arg: mockUnpinnedConvo });
expect(response.status).toBe(200);
expect(response.body).toEqual(mockUnpinnedConvo);
expect(saveConvo).toHaveBeenCalledWith(
{ userId: 'test-user-123' },
{ conversationId: mockConversationId, pinned: false },
{
context: `POST /api/convos/pin ${mockConversationId}`,
preserveUpdatedAt: true,
noUpsert: true,
},
);
expect(setConvoPinned).toHaveBeenCalledWith('test-user-123', mockConversationId, false);
});
/** With timestamps suppressed, an upsert would insert a conversation carrying
* neither createdAt nor updatedAt, so an unknown id has to be rejected instead. */
it('should return 404 when the conversation does not exist', async () => {
saveConvo.mockResolvedValue(null);
setConvoPinned.mockResolvedValue(null);
const response = await request(app)
.post('/api/convos/pin')
@ -718,7 +714,7 @@ describe('Convos Routes', () => {
expect(response.status).toBe(400);
expect(response.body).toEqual({ error: 'conversationId is required' });
expect(saveConvo).not.toHaveBeenCalled();
expect(setConvoPinned).not.toHaveBeenCalled();
});
it('should return 400 when pinned is not a boolean', async () => {
@ -728,7 +724,7 @@ describe('Convos Routes', () => {
expect(response.status).toBe(400);
expect(response.body).toEqual({ error: 'pinned must be a boolean' });
expect(saveConvo).not.toHaveBeenCalled();
expect(setConvoPinned).not.toHaveBeenCalled();
});
it('should return 400 when pinned is missing', async () => {
@ -738,11 +734,11 @@ describe('Convos Routes', () => {
expect(response.status).toBe(400);
expect(response.body).toEqual({ error: 'pinned is required' });
expect(saveConvo).not.toHaveBeenCalled();
expect(setConvoPinned).not.toHaveBeenCalled();
});
it('should return 500 when saveConvo fails', async () => {
saveConvo.mockRejectedValue(new Error('Database error'));
it('should return 500 when the pin update fails', async () => {
setConvoPinned.mockRejectedValue(new Error('Database error'));
const response = await request(app)
.post('/api/convos/pin')

View file

@ -245,17 +245,7 @@ router.post('/pin', validateConvoAccess, async (req, res) => {
}
try {
const dbResponse = await db.saveConvo(
{ userId: req.user.id },
{ conversationId, pinned },
{
context: `POST /api/convos/pin ${conversationId}`,
/** Pinning is a bookmark, not activity, so the sidebar's `updatedAt` order stands. */
preserveUpdatedAt: true,
/** Without timestamps, an upsert would insert a conversation that has none. */
noUpsert: true,
},
);
const dbResponse = await db.setConvoPinned(req.user.id, conversationId, pinned);
if (!dbResponse) {
return res.status(404).json({ error: 'Conversation not found' });