Repository navigation
✂️ refactor: Remove Dead Helpers, Classes and Dependencies From packages/api - #17118
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c35960a1d
ℹ️ 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".
| /** | ||
| * Builds the security-sensitive provider source/file association in typed |
There was a problem hiding this comment.
Preserve the documented compatibility export
Existing consumers that import collectModelBoundHistoricalFileIds from @librechat/api will fail after this removal: src/index.ts exports ./middleware, whose index exports this module, and the deleted comment explicitly identifies this function as the backward-compatible view. A repository-wide usage search only establishes that LibreChat itself no longer calls the helper; it does not make a published package export dead. Keep the delegating shim or defer its removal to a breaking release.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ab85b27: the collectModelBoundHistoricalFileIds shim is restored, and so are the other published exports this diff removed despite a compatibility or deprecation contract (the JSON schema to Zod converter and the deferDeliveryUntilSynchronized transport option).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab85b27bcc
ℹ️ 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".
| @@ -741,41 +634,6 @@ export function getModelMaxTokens( | |||
| return getModelTokenValue(modelName, maxTokensMap[endpoint as keyof typeof maxTokensMap]); | |||
| } | |||
There was a problem hiding this comment.
Preserve the published output-token lookup
When an installed consumer imports getModelMaxOutputTokens (or either exported output-token map) from @librechat/api, this removal makes the import undefined and breaks that consumer. These symbols were part of the published root entrypoint through src/utils/index.ts and src/index.ts, and the legacy API suite extensively exercised the helper before its tests were deleted here; repository-wide internal non-use does not make a published export safe to remove in a non-breaking refactor. Retain a compatibility export and its maps, or reserve their removal for a breaking release.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this one: @librechat/api is not published (npm view @librechat/api returns 404 on registry.npmjs.org), so its only consumer is /api in this repo, and a grep across api/, client/ and packages/ finds no remaining caller of getModelMaxOutputTokens or the output-token maps. The earlier restore kept only exports with an explicit compatibility or deprecation contract in their JSDoc; plain unused helpers like this one are what the cleanup removes.
ab85b27 to
5d36d51
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Summary
A dead-code scan of the repository flagged functions, classes, constants and scripts in
packages/apithat nothing calls anymore. Each one was re-checked against currentdev(whole-word grep acrossapi/,client/andpackages/, including destructuredrequire('@librechat/api')calls andjest.mockfactories in/api), and only code that is still unreferenced is removed here. Specs that only tested a removed helper go with it; specs that drove live behavior through a removed wrapper now call the live function directly, so their cases stay.This removes, among others: the superseded
isTokenIssuedBeforeCredentialChangepredicate (password-reset revocation runs throughisTokenRetired, which every JWT strategy, the share and file gates and image authorization already use, and whose spec covers the same cases), an unused cache store class and its factory branch, legacy PII helpers beside the live fragment predicate, unused token, compaction, file filter, MCP, schedule, stream and tool approval helpers, unused constants, the root OIDC integration test that no Jest config ever ran, the MCP demostart:*scripts and their bootstrap loader, and four dependencies nothing imports (@google/genai,@azure/search-documents,@babel/preset-react,@types/react). Behavior is unchanged.Skipped on purpose: exports whose JSDoc keeps them for external
@librechat/apiconsumers or marks them deprecated rather than removed (manual skill primes, auth log formatting, the S3 and CloudFront URL wrappers,createHandleLLMNewToken,collectModelBoundHistoricalFileIds, the JSON schema to Zod converter, thedeferDeliveryUntilSynchronizedtransport option), the paused LibreOffice preview, the generation job ready promise (threads through the streaming resume paths), helpers specs use as fixtures, andpackages/chathost types (active refactor).Type of change
Testing
Tested environments/configuration: Jest with mongodb-memory-server, no Redis, no running app.
Automated tests:
tsc --noEmitinpackages/api(specs included): no errorsnpx jest --findRelatedTestsinpackages/apiover the changed files: 14696 tests pass; the only failures were Redis-backed*_integrationsuites with no Redis available, and seven suites that timed out under load and pass when rerun alonenpm run buildinpackages/api, thenutils/tokens.spec.js,responses.unit.spec.jsandserver/services/Filesinapi: passnpm run static-checks -- --against origin/dev: passpackage-lock.jsonchecked withnpm install --package-lock-only(no further changes);bun.lockpruned of the same four entriesScreenshots / recordings
No user-facing change.
Checklist