feat(gateway): document support for Gemini AI Studio - #2354
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughAdds OpenAI-style ChangesDocument Input Support
Estimated code review effort: Possibly related PRs:
Suggested reviewers:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60df122907
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/api/src/routes/chats.ts (2)
922-933:⚠️ Potential issue | 🟠 Major | ⚡ Quick winShared chat snapshots currently drop
documents.Line 903 selects
documents, but Line 922-933 does not serialize it intochat_share.messages. Shared views/forks will silently lose document attachments.💡 Suggested fix in snapshot mapping
messages: messages.map((message) => ({ id: message.id, role: message.role, content: message.content, images: message.images, audios: message.audios, + documents: message.documents, reasoning: message.reasoning, tools: message.tools, metadata: message.metadata, sequence: message.sequence, createdAt: message.createdAt.toISOString(), })),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/chats.ts` around lines 922 - 933, The messages mapping that builds chat_share.messages drops the documents field, so update the messages.map block (the code that produces chat_share.messages) to include the documents property from each message (e.g., documents: message.documents) and ensure each document is serialized into a JSON-safe form (e.g., map document objects to plain fields or call toJSON/serialize) so shared snapshots preserve attachments; locate the messages.map call in this module and add documents serialization alongside id, role, content, images, audios, reasoning, tools, metadata, sequence, and createdAt.
1675-1686:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude
documentsin add-message response payload.Line 1659 persists
documents, but the response object (Line 1675-1686) omits it, creating a contract mismatch with read endpoints.💡 Suggested response fix
message: { id: newMessage.id, role: newMessage.role as "user" | "assistant" | "system", content: newMessage.content, images: newMessage.images, audios: (newMessage as any).audios ?? null, + documents: (newMessage as any).documents ?? null, reasoning: newMessage.reasoning, tools: newMessage.tools ?? null, metadata: newMessage.metadata ?? null, sequence: newMessage.sequence, createdAt: newMessage.createdAt.toISOString(), },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/chats.ts` around lines 1675 - 1686, The add-message response omits the persisted documents field, causing a mismatch with read endpoints; update the response object constructed for the new message (the block that builds message: { id: newMessage.id, role: ..., createdAt: ... }) to include documents (e.g., documents: newMessage.documents ?? null) so the payload mirrors what was persisted and matches the read endpoints' contract.apps/playground/src/components/playground/chat-page-client.tsx (1)
1066-1152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve documents in the edit/retry flow.
This wires documents into create/load, but
buildEditedUserMessage()andhandleEditUserMessage()still only keep images/audio. Editing a user message with documents will resend without the files, so the regenerated response no longer matches what the UI shows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/playground/src/components/playground/chat-page-client.tsx` around lines 1066 - 1152, The edit/retry flow drops documents because buildEditedUserMessage and handleEditUserMessage only preserve images/audios; update both to also preserve and pass documents (e.g., include documents?: string[] or serialized JSON in the same spots images/audio are handled), ensure the edited message body includes ...(documents?.length ? { documents: JSON.stringify(documents) } : {}) when constructing the payload and when calling addMessage.mutateAsync or retry send so edited/resend operations include the original document attachments.apps/playground/src/components/playground/chat-ui.tsx (1)
1023-1078:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject unsupported attachments instead of silently dropping them.
When
supportsDocumentsis true the picker accepts any file, but this submit path only forwards MIME families that are enabled for the selected model. If a user picks an image/audio file for a model that lacks that capability, the attachment is discarded with no feedback. Please validatefilesbefore clearing/submitting and surface an error for unsupported types.Also applies to: 1328-1345
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/playground/src/components/playground/chat-ui.tsx` around lines 1023 - 1078, Before building parts, validate the selected files against the model capabilities (supportsImages, supportsAudio, supportsDocuments) and reject unsupported attachments instead of dropping them: iterate the files array and for each file check its mediaType (using mediaType?.startsWith("image/"), mediaType?.startsWith("audio/"), and isDocumentMediaType()/getDocumentMediaType()) and if any file’s type isn’t allowed given the current supports* flags, surface a user-visible error (e.g., set form error or toast) and abort submission/clearing so parts isn’t built with silently dropped files; ensure this validation runs in the same submit flow that constructs parts and also in the other similar block referenced (around the 1328-1345 area).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/chats.ts`:
- Line 648: Remove the unnecessary `as any` casts on the message properties and
access them directly (use message.audios and message.documents like you already
do for message.images, message.reasoning, and message.tools); update the code in
the chats route where `documents: (message as any).documents ?? null` and the
`audios` line appear to simply read `documents: message.documents ?? null` and
`audios: message.audios ?? null`; if TypeScript flags a type mismatch, fix the
local `message` variable's type (or the handler's input type) to the correct
message schema/interface so the compiler recognizes `audios` and `documents` as
defined.
In `@apps/gateway/src/app.ts`:
- Around line 142-149: The 400 response created by the c.json call in app.ts
currently omits structured metadata; update the returned JSON to include
mimeType and providerTarget from the thrown error (e.g., error.mimeType and
error.providerTarget or fields on the document-format error object) so clients
can consume unsupported-document errors reliably. Locate the c.json call that
returns { error: true, status: 400, message: error.message } and add mimeType
and providerTarget properties (falling back to null/undefined if absent) to that
response object; ensure you still return HTTP 400.
In `@apps/gateway/src/chat/chat.ts`:
- Around line 427-429: The specific-provider rate-limit fallback filters in this
file currently omit the documents capability check and can reroute hasDocuments
requests to providers lacking file support; update those fallback predicate(s)
(the code paths invoked when a provider is 429’ed) to include the same documents
guard used by filterEligibleModelProviders(): if (options.hasDocuments &&
provider.document !== true) return false (or equivalent boolean check), and
apply this change to all similar fallback blocks in the file (the other
specific-provider rate-limit fallback predicates referenced alongside
filterEligibleModelProviders()) so document requests are never forwarded to
providers without document support.
In `@apps/gateway/src/chat/schemas/completions.ts`:
- Around line 56-69: The current z.object for the "file" content block allows an
empty file object because file_data and file_id are optional; update the inner
z.object (the one assigned to the "file" key inside the z.literal("file")
branch) to add a .refine(...) that enforces at least one of file_data or file_id
is present, providing a clear error message (e.g., "either file_data or file_id
must be provided"); keep file_data and file_id as optional strings but use the
refine predicate to check Boolean(file_data) || Boolean(file_id) and attach the
message to the schema so validation fails early.
In `@apps/gateway/src/chat/tools/messages-contain-documents.spec.ts`:
- Around line 59-63: The test currently silently skips missing models in the
loop over expectedDocumentModelIds by using "if (!model) { continue; }", which
can hide a missing Gemini model; change this to explicitly fail the test when a
model is not found by replacing the continue with an assertion or throw (e.g.,
use expect(model).toBeDefined() or throw new Error(`Missing model ${id}`)) so
that the test fails if any expectedDocumentModelIds entry has no matching item
in models; update references in the spec to expectedDocumentModelIds, models,
and model accordingly.
In `@packages/actions/src/transform-google-messages.ts`:
- Around line 157-170: The parseFileDataUrl function currently lowercases the
captured MIME with match[1].toLowerCase(), which mutates user-supplied MIME;
change parseFileDataUrl to return the MIME verbatim (use match[1] unchanged) so
file_data MIME is preserved exactly as provided, and update any downstream
tests/allowlist expectations (e.g., the mixed-case MIME test) to match the
preserved-case behavior.
---
Outside diff comments:
In `@apps/api/src/routes/chats.ts`:
- Around line 922-933: The messages mapping that builds chat_share.messages
drops the documents field, so update the messages.map block (the code that
produces chat_share.messages) to include the documents property from each
message (e.g., documents: message.documents) and ensure each document is
serialized into a JSON-safe form (e.g., map document objects to plain fields or
call toJSON/serialize) so shared snapshots preserve attachments; locate the
messages.map call in this module and add documents serialization alongside id,
role, content, images, audios, reasoning, tools, metadata, sequence, and
createdAt.
- Around line 1675-1686: The add-message response omits the persisted documents
field, causing a mismatch with read endpoints; update the response object
constructed for the new message (the block that builds message: { id:
newMessage.id, role: ..., createdAt: ... }) to include documents (e.g.,
documents: newMessage.documents ?? null) so the payload mirrors what was
persisted and matches the read endpoints' contract.
In `@apps/playground/src/components/playground/chat-page-client.tsx`:
- Around line 1066-1152: The edit/retry flow drops documents because
buildEditedUserMessage and handleEditUserMessage only preserve images/audios;
update both to also preserve and pass documents (e.g., include documents?:
string[] or serialized JSON in the same spots images/audio are handled), ensure
the edited message body includes ...(documents?.length ? { documents:
JSON.stringify(documents) } : {}) when constructing the payload and when calling
addMessage.mutateAsync or retry send so edited/resend operations include the
original document attachments.
In `@apps/playground/src/components/playground/chat-ui.tsx`:
- Around line 1023-1078: Before building parts, validate the selected files
against the model capabilities (supportsImages, supportsAudio,
supportsDocuments) and reject unsupported attachments instead of dropping them:
iterate the files array and for each file check its mediaType (using
mediaType?.startsWith("image/"), mediaType?.startsWith("audio/"), and
isDocumentMediaType()/getDocumentMediaType()) and if any file’s type isn’t
allowed given the current supports* flags, surface a user-visible error (e.g.,
set form error or toast) and abort submission/clearing so parts isn’t built with
silently dropped files; ensure this validation runs in the same submit flow that
constructs parts and also in the other similar block referenced (around the
1328-1345 area).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6d1f6a2e-e3ba-42f2-b4f4-1aa7001799db
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (17)
apps/api/src/routes/chats.tsapps/api/src/routes/internal-models.tsapps/gateway/src/app.tsapps/gateway/src/chat/chat.tsapps/gateway/src/chat/schemas/completions.tsapps/gateway/src/chat/tools/messages-contain-documents.spec.tsapps/gateway/src/chat/tools/messages-contain-documents.tsapps/gateway/src/chat/tools/validate-model-capabilities.tsapps/playground/src/components/playground/chat-page-client.tsxapps/playground/src/components/playground/chat-ui.tsxapps/playground/src/lib/fetch-models.tspackages/actions/src/transform-google-messages.spec.tspackages/actions/src/transform-google-messages.tspackages/db/src/schema.tspackages/models/src/models.tspackages/models/src/models/google.tspackages/models/src/types.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ca1691302
ℹ️ 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".
| function parseFileDataUrl( | ||
| fileData: string, | ||
| ): { mimeType: string; data: string } | null { | ||
| const match = fileData.match(/^data:([^;,]+);base64,(.*)$/i); |
There was a problem hiding this comment.
Parse file_data URLs with MIME parameters
The file_data parser only accepts data:<mime>;base64,... and rejects valid data URLs like data:text/plain;charset=utf-8;base64,... because of the strict regex. This causes legitimate document payloads to be treated as invalid before reaching Google, breaking text document uploads that include MIME parameters.
Useful? React with 👍 / 👎.
- Drop unnecessary `as any` casts on message.audios / message.documents - Return mimeType and providerTarget on UnsupportedDocumentFormatError 400 - Add hasDocuments guard to specific-provider rate-limit fallback filter - Require file_data or file_id on `file` content blocks via schema refine - Fail document-capability spec when expected model is missing - Preserve MIME case verbatim in parseFileDataUrl - Throw typed InvalidFileContentError so invalid file blocks return 400 - Preserve documents in shared/forked chat snapshots Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parse and emit `documents` as file parts in the public shared chat snapshot so uploaded files appear alongside text/images on /share/[id]. Regenerate API clients to expose the new field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/routes/chats.ts (1)
1679-1690:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReturn
documentsin theaddMessageresponse payload.
documentsis accepted and persisted, but the created-message response omits it (around Line 1684). Clients using the POST response directly won’t receive document attachments until a refetch.Suggested fix
message: { id: newMessage.id, role: newMessage.role as "user" | "assistant" | "system", content: newMessage.content, images: newMessage.images, audios: newMessage.audios ?? null, + documents: newMessage.documents ?? null, reasoning: newMessage.reasoning, tools: newMessage.tools ?? null, metadata: newMessage.metadata ?? null, sequence: newMessage.sequence, createdAt: newMessage.createdAt.toISOString(), },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/routes/chats.ts` around lines 1679 - 1690, The addMessage response payload in chats.ts builds the message object from newMessage but omits the persisted documents field; update the response construction inside the addMessage handler to include documents: newMessage.documents (or newMessage.documents ?? null) alongside images/audios/tools/metadata so clients receive document attachments immediately in the created message response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/api/src/routes/chats.ts`:
- Around line 1679-1690: The addMessage response payload in chats.ts builds the
message object from newMessage but omits the persisted documents field; update
the response construction inside the addMessage handler to include documents:
newMessage.documents (or newMessage.documents ?? null) alongside
images/audios/tools/metadata so clients receive document attachments immediately
in the created message response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6d92fa04-ad6d-4723-8d34-7b933f8416e3
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (10)
apps/api/src/routes/chats.tsapps/api/src/routes/public-chat-shares.tsapps/gateway/src/app.tsapps/gateway/src/chat/chat.tsapps/gateway/src/chat/schemas/completions.tsapps/gateway/src/chat/tools/messages-contain-documents.spec.tsapps/playground/src/app/share/[shareId]/page.tsxpackages/actions/src/transform-google-messages.spec.tspackages/actions/src/transform-google-messages.tspackages/db/src/schema.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/models/src/models/google.ts (1)
42-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
document: truetogoogle-vertexprovider capability configs to match AI StudioIn
packages/models/src/models/google.ts, allgoogle-ai-studioprovider entries that support documents setdocument: true, while the correspondinggoogle-vertexentries for the same GeminimodelNames omit it (0/22 document-capable mappings for Vertex). Sincepackages/models/src/models.tsdefinesdocumentas input to themodel: "auto"router/capability validator, this omission can stop Vertex from being selected/allowed forfile/inline document inputs even though Google Vertex Gemini supports document parts via RESTinlineData(base64 +mimeType). [1]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/models/src/models/google.ts` at line 42, In packages/models/src/models/google.ts update the provider capability configs for "google-vertex" entries to include document: true for each Gemini modelName that already has document: true under "google-ai-studio" (i.e., mirror the document-capable mappings); locate the provider entries named "google-vertex" and add the document: true flag in their capability object so the auto router/capability validator in packages/models/src/models.ts recognizes Vertex as supporting file/inline document inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/models/src/models/google.ts`:
- Line 42: In packages/models/src/models/google.ts update the provider
capability configs for "google-vertex" entries to include document: true for
each Gemini modelName that already has document: true under "google-ai-studio"
(i.e., mirror the document-capable mappings); locate the provider entries named
"google-vertex" and add the document: true flag in their capability object so
the auto router/capability validator in packages/models/src/models.ts recognizes
Vertex as supporting file/inline document inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8044d7f6-7ce1-400d-8737-963a1d9225e1
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (2)
apps/gateway/src/chat/chat.tspackages/models/src/models/google.ts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Head branch was pushed to by a user without write access
Summary
Test plan
Summary by CodeRabbit
New Features
Improvements
Tests
Documentation