📄 feat: Model-Aware Bedrock Document Size Validation#12467
Merged
Conversation
Remove the hard 4.5MB clamp on Bedrock document uploads so that Claude 4+ (PDF) and Nova (PDF/DOCX) models can accept larger files per AWS documentation. The default 4.5MB limit is preserved for other models/formats, and fileConfig can now override it in either direction—consistent with every other provider.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Bedrock document/PDF validation to be model-aware (respecting AWS-documented exceptions to the 4.5MB default) and threads the selected model through the document encoding pipeline so validation can apply the correct limits.
Changes:
- Thread
modelfromBaseClient→encodeAndFormatDocuments()→validatePdf()/validateBedrockDocument(). - Add Bedrock model/type exemption logic (Claude 4+ PDFs, Nova PDF/DOCX) with a 32MB fallback limit and update validation/encoding tests.
- Large set of locale JSON updates (additions/removals/rewrites).
Reviewed changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/api/src/files/validation.ts | Adds model-aware Bedrock document limit exemptions and threads optional model into validatePdf/validateBedrockDocument. |
| packages/api/src/files/validation.spec.ts | Adds/updates tests for Bedrock model-specific exemptions and config override behavior. |
| packages/api/src/files/encode/document.ts | Threads model into validation calls during document encoding/formatting. |
| packages/api/src/files/encode/document.spec.ts | Updates mocks/assertions for the new optional model parameter. |
| api/app/clients/BaseClient.js | Passes selected model into document encoding/validation pipeline. |
| client/src/locales/zh-Hant/translation.json | Removes several translation keys. |
| client/src/locales/vi/translation.json | Adds/updates many translation keys. |
| client/src/locales/uk/translation.json | Removes several translation keys. |
| client/src/locales/tr/translation.json | Removes several translation keys. |
| client/src/locales/th/translation.json | Removes some keys and adds zoom-related keys. |
| client/src/locales/sv/translation.json | Adds/updates agents-related keys and removes some keys. |
| client/src/locales/sl/translation.json | Removes some keys. |
| client/src/locales/ru/translation.json | Adds/updates many keys; includes several incomplete/broken strings. |
| client/src/locales/pt-PT/translation.json | Adds/updates many keys; includes placeholder text for chat direction. |
| client/src/locales/pt-BR/translation.json | Adds/updates a few keys and removes some keys. |
| client/src/locales/nl/translation.json | Adds/updates many keys and removes some keys. |
| client/src/locales/nb/translation.json | Removes several translation keys. |
| client/src/locales/lv/translation.json | Adds/updates many keys and removes some keys. |
| client/src/locales/ko/translation.json | Removes several translation keys. |
| client/src/locales/ka/translation.json | Removes several translation keys. |
| client/src/locales/id/translation.json | Removes one translation key. |
| client/src/locales/hy/translation.json | Removes one translation key. |
| client/src/locales/hu/translation.json | Removes several translation keys. |
| client/src/locales/he/translation.json | Removes several translation keys and adds language list entries. |
| client/src/locales/fr/translation.json | Adds/updates many keys and removes some keys. |
| client/src/locales/fi/translation.json | Removes several translation keys. |
| client/src/locales/et/translation.json | Removes several translation keys. |
| client/src/locales/es/translation.json | Rewrites/adds many keys; includes several Spanish translation issues (including reversed zoom strings). |
| client/src/locales/en/translation.json | Reorders/adds keys and fixes placements; also used to anchor PR-description discrepancy comment. |
| client/src/locales/de/translation.json | Removes several translation keys. |
| client/src/locales/da/translation.json | Removes several translation keys. |
| client/src/locales/cs/translation.json | Updates some keys and removes several translation keys. |
| client/src/locales/ca/translation.json | Removes several translation keys. |
| client/src/locales/ar/translation.json | Adds agents-related keys; includes an incorrect agent card label string (missing placeholders). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "com_ui_enter_api_key": "Введите API-ключ", | ||
| "com_ui_enter_description": "Введите описание (опционально)", | ||
| "com_ui_enter_key": "Вставьте ключ", | ||
| "com_ui_enter_name": "Вве", |
There was a problem hiding this comment.
com_ui_enter_name is truncated ("Вве"), which will render an incomplete prompt in the UI. Please replace it with the full Russian translation.
Suggested change
| "com_ui_enter_name": "Вве", | |
| "com_ui_enter_name": "Введите имя", |
| "com_ui_on": "Вкл.", | ||
| "com_ui_open_var": "Открыть {{0}}", | ||
| "com_ui_openai": "OpenAI", | ||
| "com_ui_opens_new_tab": "(", |
There was a problem hiding this comment.
com_ui_opens_new_tab is set to just "(" which looks like a broken/incomplete translation and will likely display incorrectly. It should be a complete string (typically something like "(opens in a new tab)").
Suggested change
| "com_ui_opens_new_tab": "(", | |
| "com_ui_opens_new_tab": "(откроется в новой вкладке)", |
Comment on lines
+812
to
+814
| "com_ui_zoom_in": "Menos Zoom", | ||
| "com_ui_zoom_level": "Nivelar Zoom", | ||
| "com_ui_zoom_out": "Mas Zoom", |
There was a problem hiding this comment.
Spanish zoom labels appear reversed: com_ui_zoom_in is "Menos Zoom" (zoom out) and com_ui_zoom_out is "Mas Zoom" (zoom in). This will confuse users; please swap/correct these strings (and add the accent: "Más").
Suggested change
| "com_ui_zoom_in": "Menos Zoom", | |
| "com_ui_zoom_level": "Nivelar Zoom", | |
| "com_ui_zoom_out": "Mas Zoom", | |
| "com_ui_zoom_in": "Más Zoom", | |
| "com_ui_zoom_level": "Nivelar Zoom", | |
| "com_ui_zoom_out": "Menos Zoom", |
| "com_agents_category_rd_description": "Agentes centrados en procesos de I+D, innovación e investigación técnica", | ||
| "com_agents_category_sales": "Ventas", | ||
| "com_agents_category_sales_description": "Agentes enfocados en procesos de venta, relaciones con el cliente", | ||
| "com_agents_category_sales_description": "Agentes enfocados en procesos de venta, relaciones con el client", |
There was a problem hiding this comment.
com_agents_category_sales_description ends with "client" (missing the final "e"), which reads like a truncated word in Spanish.
Suggested change
| "com_agents_category_sales_description": "Agentes enfocados en procesos de venta, relaciones con el client", | |
| "com_agents_category_sales_description": "Agentes enfocados en procesos de venta, relaciones con el cliente", |
| "com_agents_error_generic": "Hemos encontrado un problema al cargar el contenido.", | ||
| "com_agents_error_invalid_request": "Petición inválida", | ||
| "com_agents_error_loading": "Error al cargar los agentes", | ||
| "com_agents_error_loading": "Error al cargar los agente", |
There was a problem hiding this comment.
com_agents_error_loading uses "los agente" (singular) which is grammatically incorrect in Spanish; it should be plural ("los agentes").
Suggested change
| "com_agents_error_loading": "Error al cargar los agente", | |
| "com_agents_error_loading": "Error al cargar los agentes", |
| "com_ui_date_previous_30_days": "За последние 30 дней", | ||
| "com_ui_date_previous_7_days": "Предыдущие 7 дней", | ||
| "com_ui_date_september": "Сентябрь", | ||
| "com_ui_date_sort": "Сорти", |
There was a problem hiding this comment.
com_ui_date_sort is truncated ("Сорти"). This will render as an incomplete label in the UI; please replace it with the full translation (e.g. "Сортировать").
Suggested change
| "com_ui_date_sort": "Сорти", | |
| "com_ui_date_sort": "Сортировать", |
Comment on lines
1
to
12
| { | ||
| "chat_direction_left_to_right": "Left to Right", | ||
| "chat_direction_right_to_left": "Right to Left", | ||
| "com_a11y_ai_composing": "The AI is still composing.", | ||
| "com_a11y_chats_date_section": "Chats from {{date}}", | ||
| "com_a11y_end": "The AI has finished their reply.", | ||
| "com_a11y_selected": "selected", | ||
| "com_a11y_start": "The AI has started their reply.", | ||
| "com_a11y_summarize_completed": "Context summarized.", | ||
| "com_a11y_summarize_failed": "Summarization failed, continuing with available context.", | ||
| "com_a11y_summarize_started": "Summarizing context.", | ||
| "com_a11y_start": "The AI has started their reply.", | ||
| "com_agents_agent_card_label": "{{name}} agent. {{description}}", |
There was a problem hiding this comment.
This PR includes large-scale locale/translation changes (many client/src/locales/*/translation.json edits) that aren’t mentioned in the PR description, which makes review/risk assessment harder. Please clarify why these translation updates are included here or split them into a separate PR.
| @@ -1,20 +1,35 @@ | |||
| { | |||
| "chat_direction_left_to_right": "algo precisa de ser feito aqui. estava vazio", | |||
There was a problem hiding this comment.
chat_direction_left_to_right is still a placeholder string ("algo precisa de ser feito aqui. estava vazio"). This should be replaced with an actual pt-PT translation (consistent with other locales, e.g. "Esquerda para a direita").
Suggested change
| "chat_direction_left_to_right": "algo precisa de ser feito aqui. estava vazio", | |
| "chat_direction_left_to_right": "Esquerda para a direita", |
| "com_a11y_ai_composing": "الذكاء الاصطناعي ما زال يكتب", | ||
| "com_a11y_end": "انتهى الذكاء الاصطناعي من الرد", | ||
| "com_a11y_start": "بدأ الذكاء الاصطناعي بالرد", | ||
| "com_agents_agent_card_label": "البوسنية", |
There was a problem hiding this comment.
com_agents_agent_card_label value appears incorrect and drops the {{name}}/{{description}} placeholders used by the UI. Without those placeholders the agent card label won’t include the agent name/description.
Suggested change
| "com_agents_agent_card_label": "البوسنية", | |
| "com_agents_agent_card_label": "{{name}}: {{description}}", |
| "com_ui_delete_action_confirm": "Вы действительно хотите удалить это действие?", | ||
| "com_ui_delete_agent": "Удалить агента", | ||
| "com_ui_delete_agent_confirm": "Вы действительно хотите удалить этого агента?", | ||
| "com_ui_delete_assistant": "Уд", |
There was a problem hiding this comment.
This label is truncated ("Уд"), which will show an incomplete UI string for the delete-assistant action. Please replace it with the full Russian translation (e.g. the equivalent of "Delete assistant").
Suggested change
| "com_ui_delete_assistant": "Уд", | |
| "com_ui_delete_assistant": "Удалить ассистента", |
…, add tests - Restore Math.min clamp for non-exempt Bedrock documents (fileConfig can only lower the hard 4.5 MB API limit, not raise it); only exempt models (Claude 4+ PDF, Nova PDF/DOCX) use ?? to allow fileConfig override - Replace copied isBedrockClaude4Plus regex with cleaner anchored pattern that correctly handles multi-digit version numbers (e.g. sonnet-40) and removes dead Alt 1 branch matching no real Bedrock model IDs - Tighten isBedrockNova from includes() to startsWith() to prevent substring matching in unexpected positions - Add integration test verifying model is threaded to validateBedrockDocument - Add boundary tests for exempt + low configuredFileSizeLimit, non-exempt + high configuredFileSizeLimit, and exempt model accepting files up to 32 MB - Revert two tests that were incorrectly inverted to prove wrong behavior - Fix inaccurate JSDoc and misleading test name
Make Bedrock consistent with all other providers — fileConfig sets the effective limit unconditionally via ?? rather than clamping with Math.min. The model-aware defaults (4.5 MB for non-exempt, 32 MB for exempt) remain as sensible fallbacks when no fileConfig is set.
1 task
danny-avila
changed the title
Owner
Author
|
@codex review |
![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: c518dbeddd
ℹ️ 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".
Bedrock cross-region inference profiles prepend a region code to the model ID (e.g. "us.amazon.nova-pro-v1:0", "eu.anthropic.claude-sonnet-4-..."). Both isBedrockNova and isBedrockClaude4Plus would miss these prefixed IDs, silently falling back to the 4.5 MB default for eligible models. Switch both matchers to use (?:^|\.) to anchor the vendor segment so the pattern matches with or without a leading region prefix.
yidianyiko
pushed a commit
to yidianyiko/LibreChat
that referenced
this pull request
Apr 13, 2026
* 📄 fix: Model-Aware Bedrock Document Size Validation Remove the hard 4.5MB clamp on Bedrock document uploads so that Claude 4+ (PDF) and Nova (PDF/DOCX) models can accept larger files per AWS documentation. The default 4.5MB limit is preserved for other models/formats, and fileConfig can now override it in either direction—consistent with every other provider. * address review: restore Math.min for non-exempt docs, tighten regexes, add tests - Restore Math.min clamp for non-exempt Bedrock documents (fileConfig can only lower the hard 4.5 MB API limit, not raise it); only exempt models (Claude 4+ PDF, Nova PDF/DOCX) use ?? to allow fileConfig override - Replace copied isBedrockClaude4Plus regex with cleaner anchored pattern that correctly handles multi-digit version numbers (e.g. sonnet-40) and removes dead Alt 1 branch matching no real Bedrock model IDs - Tighten isBedrockNova from includes() to startsWith() to prevent substring matching in unexpected positions - Add integration test verifying model is threaded to validateBedrockDocument - Add boundary tests for exempt + low configuredFileSizeLimit, non-exempt + high configuredFileSizeLimit, and exempt model accepting files up to 32 MB - Revert two tests that were incorrectly inverted to prove wrong behavior - Fix inaccurate JSDoc and misleading test name * simplify: allow fileConfig to override Bedrock limit in either direction Make Bedrock consistent with all other providers — fileConfig sets the effective limit unconditionally via ?? rather than clamping with Math.min. The model-aware defaults (4.5 MB for non-exempt, 32 MB for exempt) remain as sensible fallbacks when no fileConfig is set. * fix: handle cross-region inference profile IDs in Bedrock model matchers Bedrock cross-region inference profiles prepend a region code to the model ID (e.g. "us.amazon.nova-pro-v1:0", "eu.anthropic.claude-sonnet-4-..."). Both isBedrockNova and isBedrockClaude4Plus would miss these prefixed IDs, silently falling back to the 4.5 MB default for eligible models. Switch both matchers to use (?:^|\.) to anchor the vendor segment so the pattern matches with or without a leading region prefix.
jcbartle
pushed a commit
to jcbartle/LibreChat
that referenced
this pull request
May 11, 2026
* 📄 fix: Model-Aware Bedrock Document Size Validation Remove the hard 4.5MB clamp on Bedrock document uploads so that Claude 4+ (PDF) and Nova (PDF/DOCX) models can accept larger files per AWS documentation. The default 4.5MB limit is preserved for other models/formats, and fileConfig can now override it in either direction—consistent with every other provider. * address review: restore Math.min for non-exempt docs, tighten regexes, add tests - Restore Math.min clamp for non-exempt Bedrock documents (fileConfig can only lower the hard 4.5 MB API limit, not raise it); only exempt models (Claude 4+ PDF, Nova PDF/DOCX) use ?? to allow fileConfig override - Replace copied isBedrockClaude4Plus regex with cleaner anchored pattern that correctly handles multi-digit version numbers (e.g. sonnet-40) and removes dead Alt 1 branch matching no real Bedrock model IDs - Tighten isBedrockNova from includes() to startsWith() to prevent substring matching in unexpected positions - Add integration test verifying model is threaded to validateBedrockDocument - Add boundary tests for exempt + low configuredFileSizeLimit, non-exempt + high configuredFileSizeLimit, and exempt model accepting files up to 32 MB - Revert two tests that were incorrectly inverted to prove wrong behavior - Fix inaccurate JSDoc and misleading test name * simplify: allow fileConfig to override Bedrock limit in either direction Make Bedrock consistent with all other providers — fileConfig sets the effective limit unconditionally via ?? rather than clamping with Math.min. The model-aware defaults (4.5 MB for non-exempt, 32 MB for exempt) remain as sensible fallbacks when no fileConfig is set. * fix: handle cross-region inference profile IDs in Bedrock model matchers Bedrock cross-region inference profiles prepend a region code to the model ID (e.g. "us.amazon.nova-pro-v1:0", "eu.anthropic.claude-sonnet-4-..."). Both isBedrockNova and isBedrockClaude4Plus would miss these prefixed IDs, silently falling back to the 4.5 MB default for eligible models. Switch both matchers to use (?:^|\.) to anchor the vendor segment so the pattern matches with or without a leading region prefix.
ThomasVuNguyen
pushed a commit
to ThomasVuNguyen/LibreChat
that referenced
this pull request
Jul 15, 2026
* 📄 fix: Model-Aware Bedrock Document Size Validation Remove the hard 4.5MB clamp on Bedrock document uploads so that Claude 4+ (PDF) and Nova (PDF/DOCX) models can accept larger files per AWS documentation. The default 4.5MB limit is preserved for other models/formats, and fileConfig can now override it in either direction—consistent with every other provider. * address review: restore Math.min for non-exempt docs, tighten regexes, add tests - Restore Math.min clamp for non-exempt Bedrock documents (fileConfig can only lower the hard 4.5 MB API limit, not raise it); only exempt models (Claude 4+ PDF, Nova PDF/DOCX) use ?? to allow fileConfig override - Replace copied isBedrockClaude4Plus regex with cleaner anchored pattern that correctly handles multi-digit version numbers (e.g. sonnet-40) and removes dead Alt 1 branch matching no real Bedrock model IDs - Tighten isBedrockNova from includes() to startsWith() to prevent substring matching in unexpected positions - Add integration test verifying model is threaded to validateBedrockDocument - Add boundary tests for exempt + low configuredFileSizeLimit, non-exempt + high configuredFileSizeLimit, and exempt model accepting files up to 32 MB - Revert two tests that were incorrectly inverted to prove wrong behavior - Fix inaccurate JSDoc and misleading test name * simplify: allow fileConfig to override Bedrock limit in either direction Make Bedrock consistent with all other providers — fileConfig sets the effective limit unconditionally via ?? rather than clamping with Math.min. The model-aware defaults (4.5 MB for non-exempt, 32 MB for exempt) remain as sensible fallbacks when no fileConfig is set. * fix: handle cross-region inference profile IDs in Bedrock model matchers Bedrock cross-region inference profiles prepend a region code to the model ID (e.g. "us.amazon.nova-pro-v1:0", "eu.anthropic.claude-sonnet-4-..."). Both isBedrockNova and isBedrockClaude4Plus would miss these prefixed IDs, silently falling back to the 4.5 MB default for eligible models. Switch both matchers to use (?:^|\.) to anchor the vendor segment so the pattern matches with or without a leading region prefix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I removed the hard 4.5 MB clamp on Bedrock document uploads so that model-specific exceptions documented by AWS are respected. Previously, Bedrock was the only provider where
fileConfigcould not raise the size limit above the default — this change makes Bedrock consistent with every other provider.modelparameter threading fromBaseClient.addDocuments()throughencodeAndFormatDocumentstovalidateBedrockDocumentisBedrockClaude4Plus()andisBedrockNova()matchers to detect exempt model IDsMath.min(configuredFileSizeLimit, providerLimit)withconfiguredFileSizeLimit ?? providerLimitsofileConfigcan override in either direction, matching Anthropic/OpenAI/Google behaviorRef: AWS Bedrock inference API restrictions — Claude 4+ PDFs and Nova PDF/DOCX are exceptions to the 4.5 MB per-document limit.
Change Type
Testing
Ran the full validation and document encoding test suites (84 tests passing):
Key test scenarios added:
anthropic.claude-sonnet-4-20250514-v1:0) accepts PDFs >4.5 MB up to 32 MBamazon.nova-pro-v1:0) accepts PDFs and DOCX >4.5 MBfileConfigcan now raise or lower Bedrock limits (previously could only lower)Checklist