🎚️ fix: Correct Client Image Resize Threshold Handling - #12475
mattdaniell wants to merge 5 commits into
Conversation
Add configurable minFileSizeKB support to clientImageResize and pass it through to shouldResizeImage so typical user photos are eligible for resize. Also document minFileSizeKB in librechat.example.yaml with a practical default. Made-with: Cursor
![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: 5996880be1
ℹ️ 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".
| // Return original file if it doesn't need resizing | ||
| if (!shouldResizeImage(file)) { | ||
| // Return original file if it's below the minimum size threshold | ||
| if (!shouldResizeImage(file, minFileSizeBytes)) { |
There was a problem hiding this comment.
minFileSizeKB is documented as the minimum file size to start resizing, but this call passes it directly into shouldResizeImage, whose implementation applies an additional * 0.1 threshold check. With the new default (1024 KB), files are skipped only below ~102 KB, not below 1 MB as intended, so many sub-1MB images still go through resize processing. This makes the new config inaccurate and can trigger unnecessary client-side work for files that should be excluded.
Useful? React with 👍 / 👎.
- Remove internal `* 0.1` multiplier in `shouldResizeImage`; rename its second parameter to `minSizeBytes` so the threshold matches what callers pass, fixing the documented behavior of `minFileSizeKB`. - Add `minFileSizeKB` to `FileConfig` and `FileConfigInput` types so the field is visible to typed consumers; remove `as any` cast and the `eslint-disable` line in `useClientResize`. - Tighten Zod schema to `z.number().int().min(0)` to reject fractional KB values; update YAML comment to describe the actual semantics (including `minFileSizeKB: 0` as "disable threshold"). - Introduce `DEFAULT_MIN_FILE_SIZE_KB` shared constant to dedupe the default across config, hook, and tests. - Memoize `config` in `useClientResize` so its reference is stable when `fileConfig` is unchanged, satisfying `react-hooks/exhaustive-deps` without suppression. - Expand `imageResize.test.ts` with boundary coverage (exact threshold, one byte below, 150KB regression case, `minSizeBytes: 0` disables gate, non-image and GIF gating) and add an integration test for `useClientResize` exercising enabled/disabled, threshold skip, threshold resize, and default fallback paths.
danny-avila
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ 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". |
This branch has not been deployed
2 participants
Pull Request Template
Summary
This PR fixes
clientImageResizebeing a no-op for typical user photos due to an incorrect size threshold path in the client hook.useClientResizepreviously calledshouldResizeImage(file)without passing a threshold argument, which causedshouldResizeImageto use a 512MB-based default and skip all images under ~51MB. In practice, most uploads (1-10MB photos) were never reaching canvas resize, even whenfileConfig.clientImageResize.enabled: true.This change introduces
clientImageResize.minFileSizeKB(default1024) and wires it through toshouldResizeImage(file, minFileSizeBytes). This preserves the optimization for truly tiny files while allowing normal photos to proceed to dimension/quality resize logic.Also updated
librechat.example.yamlto document the newminFileSizeKBoption.Change Type
Testing
Manual testing performed with
clientImageResize.enabled: trueand resize limits configured:Test Configuration:
fileConfig.clientImageResize.enabled: truefileConfig.clientImageResize.maxWidth: 1900fileConfig.clientImageResize.maxHeight: 1900fileConfig.clientImageResize.quality: 0.92fileConfig.clientImageResize.minFileSizeKB: 1024Checklist
Made with Cursor