Repository navigation
🗂️ perf: Index Hot Lookups and Fix the Message Cursor - #17464
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3ab937fd5
ℹ️ 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".
| const cursorValue = | ||
| sortField === 'createdAt' ? last.createdAt : last[sortField as keyof IMessage]; | ||
| nextCursor = String(cursorValue ?? ''); | ||
| nextCursor = formatMessageCursor(last[sortField as keyof IMessage], last._id); |
There was a problem hiding this comment.
For the production GET /api/messages path, api/server/routes/messages.js passes select: CLIENT_MESSAGE_SELECT, which explicitly excludes _id (packages/data-schemas/src/methods/message.ts:528–529). Consequently, last._id is always undefined here and formatMessageCursor returns only the timestamp, so the new tie-breaking predicate never runs and messages sharing the page boundary's timestamp are still skipped. The added tests omit this projection and therefore miss the actual route behavior. Retain _id internally until the cursor is constructed, then remove it from the returned messages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0eff604 (rebased); the cursor keeps _id under the client projection.
| @@ -0,0 +1,13 @@ | |||
| export const DEFAULT_MESSAGES_PAGE_SIZE = 25; | |||
| /** Matches the search page ceiling, so no previously honored search page size changes. */ | |||
| export const MAX_MESSAGES_PAGE_SIZE = 1000; | |||
There was a problem hiding this comment.
The new ceiling unconditionally clamps conversation message requests to 1000, with no configSchema field or caller-supplied setting for deployments that need a different limit. Root AGENTS.md requires newly introduced limits to be configurable through librechat.yaml, with a compatibility-preserving default. Matching the existing search ceiling does not preserve the previously uncapped conversation-read behavior. Add the configuration field and pass its resolved value into the parser rather than imposing a fixed constant.
AGENTS.md reference: AGENTS.md:L91-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: messages.maxPageSize in configSchema, resolved per request.
codegraph-librechat
Bot
added
the
🗺️ DB Schemas
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b0312bdf2
ℹ️ 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".
| } | ||
|
|
||
| router.get('/', async (req, res) => { | ||
| router.get('/', configMiddleware, async (req, res) => { |
There was a problem hiding this comment.
On conversation loads with cold principal/config caches, adding configMiddleware here makes configuration database reads finish before either the ownership or message query starts. It also invokes code-environment authorization when stateful code sessions are configured, although this route only needs message limits. Previously, prepareToolCallPreviews resolved configuration concurrently with those reads using skipRuntimeAugmentation: true; it still performs its own resolution afterward because it does not consume req.config. This adds serial database latency to the visible conversation-loading path. Resolve the required configuration concurrently with ownership checking, skip unrelated runtime augmentation, and reuse that result for preview limits.
AGENTS.md reference: AGENTS.md:L59-L61
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5488b3: configMiddleware removed; the page size resolves beside the ownership read with skipRuntimeAugmentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8d1f089c8
ℹ️ 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".
| /** Observed here so a rejection while the page size resolves is not unhandled; it is rethrown at the await below. */ | ||
| ownershipRead.catch(() => {}); | ||
| const pageSize = await pageSizeRead; |
There was a problem hiding this comment.
The conversation-read path now adds promise-rejection handling and config-dependent query scheduling directly to the legacy CJS route. Root AGENTS.md explicitly requires newly added behavior—including edits to existing CJS—to live in packages/api, leaving /api as wiring. Moving only the page-size parser does not satisfy that boundary: extract the new read orchestration into a TypeScript handler/helper with injected database and configuration dependencies, and delegate to it from this route.
AGENTS.md reference: AGENTS.md:L78-L82
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c5488b3: the page size resolution and rejection observation live in packages/api; the route only calls them.
Add narrow indexes for session refresh-token deletes, user-only tool call deletes, queued-turn delivery and reconciliation claims, agent code environment references, and updatedAt message pages. Add a TTL index to the KeyvMongo collection. getMessagesByCursor now sorts and pages on a unique (sortField, _id) tiebreaker with a millisecond-precise cursor while still accepting legacy cursors, and GET /api/messages caps pageSize in a TypeScript helper.
berry-13
added
⚡ performance
6a85994 to
c5488b3
Compare
Summary
Adds the indexes the 2026-10-19 Devin performance scan found missing on request paths, fixes the message cursor, and caps
GET /api/messagespage size.deleteSession({ refreshToken }), user-onlydeleteToolCalls, queued-turn delivery retirement and reconciliation claims, andAgent.existson code environment ids all ran collection scans; each gets the narrowest index for its query shape (partial where the field is optional). The KeyvMongologscollection gets a TTL index onexpiresAt, so expired ban/violation entries no longer accumulate.getMessagesByCursorsorted onupdatedAtwith no index and paged with a cursor that dropped milliseconds and skipped rows sharing a timestamp. The sort now ends with_id, the cursor is<iso>|<_id>with a compound predicate (bare legacy cursors still parse), and a{conversationId, user, updatedAt, _id}index serves the default sort.pageSizeis parsed byparseMessagesPageSizeinpackages/apiand capped at 1000, the same asMEILI_SEARCH_LIMIT.Type of change
Testing
Tested environments/configuration: mongodb-memory-server.
Automated tests:
message.cursor.spec.ts: ties across asc/desc on both sort fields, ms precision, legacy cursor, andexplain()showing IXSCAN with no SORT stage.schema/lookupIndexes.spec.ts:explain()confirms each new index is chosen.keyvMongo.spec.ts,messages/page.spec.ts,message.spec.ts, queuedTurn/session specs, and themessages*route specs: all pass.tsc --noEmitinpackages/data-schemasandpackages/apiclean.Screenshots / recordings
No user-facing change.
Risk / compatibility
New indexes build on startup when auto-index is enabled; the message index adds some write cost on a busy collection. The TTL index deletes
logsentries Keyv already treats as expired (entries without a ttl storenulland are ignored). No current client sendspageSizeabove 1000. The people-picker regex,hasFilesfilter, trigger delivery claim sort and unbounded conversation reads need a design decision and are not in this PR.Checklist