feat: vertex files api - #4151
Merged
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughImplements GCS-backed file operations for the Vertex provider (upload direct/resumable, list, retrieve, delete, download), relaxes upload validation for Vertex, updates HTTP handlers to accept optional file bytes and decode file IDs, and enables Vertex batch API key configuration in the UI. ChangesVertex GCS File Operations
Sequence DiagramssequenceDiagram
participant Client
participant HTTPHandler as HTTP Handler
participant VertexProvider
participant GCSAPI as GCS API
rect rgba(0, 150, 200, 0.5)
Note over Client,GCSAPI: FileUpload - Direct Multipart Path
Client->>HTTPHandler: POST /v1/files (with file bytes)
HTTPHandler->>HTTPHandler: Parse file bytes + metadata
HTTPHandler->>VertexProvider: FileUpload(request)
VertexProvider->>GCSAPI: multipart/related (metadata + bytes)
GCSAPI-->>VertexProvider: 200 + gcsObjectMetadata
VertexProvider-->>HTTPHandler: BifrostFileUploadResponse (processed)
HTTPHandler-->>Client: 200 + response
end
rect rgba(0, 150, 200, 0.5)
Note over Client,GCSAPI: FileUpload - Resumable Path
Client->>HTTPHandler: POST /v1/files (no file bytes)
HTTPHandler->>HTTPHandler: Parse filename + ExtraParams
HTTPHandler->>VertexProvider: FileUpload(request)
VertexProvider->>GCSAPI: initiate resumable (uploadType=resumable)
GCSAPI-->>VertexProvider: 200 + Location URL
VertexProvider-->>HTTPHandler: BifrostFileUploadResponse (pending + UploadURL)
HTTPHandler-->>Client: 200 + UploadURL
Client->>GCSAPI: resumable PUT (via returned URL)
end
rect rgba(200, 150, 0, 0.5)
Note over Client,GCSAPI: FileRetrieve with URL Decoding
Client->>HTTPHandler: GET /v1/files/{file_id}
HTTPHandler->>HTTPHandler: url.PathUnescape(file_id)
HTTPHandler->>VertexProvider: FileRetrieve(decoded_id)
VertexProvider->>GCSAPI: objects.get(objectName)
GCSAPI-->>VertexProvider: gcsObjectMetadata
VertexProvider-->>HTTPHandler: BifrostFileRetrieveResponse
HTTPHandler-->>Client: 200 + metadata
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 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)
Comment |
TejasGhatte
marked this pull request as ready for review
June 8, 2026 11:02
|
|
Contributor
Confidence Score: 5/5The GCS file operations are self-contained and additive; existing Vertex inference paths are unchanged. The one identified issue is an optional GCS hint that is silently dropped rather than causing incorrect behavior. All five file operations follow correct fasthttp acquire/release patterns, authentication token handling is consistent with the rest of the Vertex provider, and the transport-layer changes are backward-compatible. The only issue found is that X-Upload-Content-Length is never forwarded to GCS from HTTP transport callers because the type assertion expects float64 but form fields arrive as strings — this is an optional hint and the upload succeeds regardless. core/providers/vertex/vertex.go — the content_length extra param handling in gcsFileUploadResumable. Important Files Changed
Reviews (6): Last reviewed commit: "feat: vertex files api" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (1)
ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx (1)
386-386:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winExclude Vertex from the generic batch field to prevent duplicate rendering.
The backend schema includes
UseForBatchAPI *boolmapped to JSON fielduse_for_batch_api, and Bifrost filters keys based onUseForBatchAPIfor batch/file operations. Now that Vertex is inBATCH_SUPPORTED_PROVIDERS(line 17), the condition at line 386 will render aBatchAPIFormFieldfor Vertex. However, line 665 also rendersBatchAPIFormFieldinside theisVertexsection. This causes duplicate rendering of the same form field for Vertex keys, which can corrupt form state and confuse users.Apply the same exclusion pattern used for Azure and Bedrock:
-{supportsBatchAPI && !isBedrock && !isAzure && <BatchAPIFormField control={control} form={form} />} +{supportsBatchAPI && !isBedrock && !isAzure && !isVertex && <BatchAPIFormField control={control} form={form} />}🤖 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 `@ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx` at line 386, The BatchAPIFormField is being rendered twice for Vertex because supportsBatchAPI currently excludes only isBedrock and isAzure; update the conditional that renders BatchAPIFormField (the line using supportsBatchAPI && !isBedrock && !isAzure) to also exclude Vertex (i.e., add && !isVertex) so Vertex keys are not rendered by the generic branch and only rendered once inside the isVertex-specific section; refer to BatchAPIFormField, supportsBatchAPI, isVertex, isBedrock, isAzure, and BATCH_SUPPORTED_PROVIDERS when locating and updating the condition.
🤖 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 `@core/bifrost.go`:
- Line 2201: The current check if len(req.File) == 0 && req.Provider !=
schemas.Vertex incorrectly rejects custom providers that are based on Vertex;
update the condition to allow empty req.File when the provider is either
schemas.Vertex or a custom provider whose CustomProviderConfig.BaseProviderType
== schemas.Vertex by querying the provider's config
(CustomProviderConfig.BaseProviderType) for req.Provider and only rejecting when
neither is Vertex. Locate the conditional around req.File/req.Provider in
core/bifrost.go and change it to consult the custom provider config
(CustomProviderConfig.BaseProviderType) before deciding to fail resumable
uploads.
In `@core/providers/vertex/vertex.go`:
- Around line 2813-2816: The resumable upload path (FileUpload ->
gcsFileUploadResumable) currently returns the original request.Filename which
can be empty when the code generated a UUID fallback; update
gcsFileUploadResumable to return the resolved filename (the UUID-generated one
stored in object key/metadata) in its response so callers receive the actual
filename used. Locate usages of request.Filename and the functions
gcsFileUploadResumable and gcsFileUploadDirect and ensure the resumable response
populates Filename with the computed fallback (same value as used for
bucket/objectKey/gcsMeta) before returning; mirror the behavior implemented in
gcsFileUploadDirect for consistency. Ensure similar fix is applied to the other
affected block(s) around lines 2900-2961 where resumable uploads return the
filename.
- Around line 2880-2882: The error branches returning parseGCSAPIError(...) need
to evict the cached TokenSource first: detect when resp.StatusCode() is 401 or
403 and call removeVertexClient(...) (the cache-eviction helper in this file)
before returning parseGCSAPIError(...). Update the branch around the upload
response check (the block using resp.StatusCode(), parseGCSAPIError and
resp.Body()) and make the same change in the analogous blocks at the other
locations noted (the branches around lines handling responses that call
parseGCSAPIError at the same pattern) so that on 401/403 you call
removeVertexClient(...) then return parseGCSAPIError(...).
- Around line 2827-2875: Currently the code builds the entire multipart/related
body into buf (bytes.Buffer) which buffers the whole file in memory; replace
this with a streaming multipart upload using io.Pipe: create pr, pw :=
io.Pipe(), use multipart.NewWriter(pw) (instead of mw backed by buf), set
req.SetBodyStream(pr, -1) and req.Header.SetContentType("multipart/related;
boundary="+mw.Boundary()), then spawn a goroutine that writes the metadata part
(metaObj/json) and streams the file bytes into the file part (io.Copy to the
multipart file part from request.File), closes the multipart writer and pw when
done, and propagate any write errors by calling pw.CloseWithError(err) so
MakeRequestWithContext (and provider.client) can send the request without
buffering the full payload; keep request, resp, authHeader, gcsUploadBase,
bucket and MakeRequestWithContext usage unchanged.
In `@transports/bifrost-http/handlers/inference.go`:
- Around line 3064-3074: The handler currently returns generic Internal Server
Error on file open/read failures without logging details; update the file open
and io.ReadAll error branches (around the code that calls file, file.Close(),
and io.ReadAll) to log the underlying error before calling SendError — e.g., use
the existing logger (logger.Warn or logger.Errorf) to record "Failed to open
uploaded file" or "Failed to read uploaded file" with the err value, then return
the same client-facing error via SendError(ctx,
fasthttp.StatusInternalServerError, "Internal Server Error").
- Around line 3217-3219: The url.PathUnescape(fileID) call currently swallows
errors; update each handler that does this (the three file ID handlers handling
retrieve/delete/content at the other occurrences) to log a warning when err !=
nil while keeping the existing fallback to the original fileID; include both the
raw fileID value and the error in the log message for debugging (use the
handler's existing logger instance — e.g., request-scoped logger or package
logger — and call its Warn/Warnf or Error/Errorf method) so malformed URL
encodings are visible in logs.
---
Outside diff comments:
In `@ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx`:
- Line 386: The BatchAPIFormField is being rendered twice for Vertex because
supportsBatchAPI currently excludes only isBedrock and isAzure; update the
conditional that renders BatchAPIFormField (the line using supportsBatchAPI &&
!isBedrock && !isAzure) to also exclude Vertex (i.e., add && !isVertex) so
Vertex keys are not rendered by the generic branch and only rendered once inside
the isVertex-specific section; refer to BatchAPIFormField, supportsBatchAPI,
isVertex, isBedrock, isAzure, and BATCH_SUPPORTED_PROVIDERS when locating and
updating the condition.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8df14f7a-97d3-429c-94fe-6c769de00561
📒 Files selected for processing (6)
core/bifrost.gocore/providers/vertex/types.gocore/providers/vertex/vertex.gocore/schemas/files.gotransports/bifrost-http/handlers/inference.goui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
TejasGhatte
force-pushed
the
06-03-feat_vertex_files_api
branch
from
June 8, 2026 20:24
67c8118 to
f6ba673
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
core/bifrost.go (1)
2201-2201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVertex empty-file upload gate still rejects Vertex-based custom providers (Line 2201).
The condition only exempts
req.Provider == schemas.Vertex, so custom providers withCustomProviderConfig.BaseProviderType == schemas.Vertexstill fail resumable uploads with emptyreq.File.Suggested fix
- if len(req.File) == 0 && req.Provider != schemas.Vertex { + allowEmptyFile := req.Provider == schemas.Vertex + if !allowEmptyFile { + if cfg, cfgErr := bifrost.account.GetConfigForProvider(req.Provider); cfgErr == nil && cfg != nil && + cfg.CustomProviderConfig != nil && cfg.CustomProviderConfig.BaseProviderType == schemas.Vertex { + allowEmptyFile = true + } + } + if len(req.File) == 0 && !allowEmptyFile { return nil, &schemas.BifrostError{ IsBifrostError: false, Error: &schemas.ErrorField{ Message: "file content is required for file upload request", },🤖 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 `@core/bifrost.go` at line 2201, The empty-file gate currently exempts only when req.Provider == schemas.Vertex but still rejects requests for custom providers whose CustomProviderConfig.BaseProviderType == schemas.Vertex; update the condition to allow empty req.File when either req.Provider == schemas.Vertex OR (req.CustomProviderConfig != nil && req.CustomProviderConfig.BaseProviderType == schemas.Vertex). Concretely, change the if check around req.File to include a nil-safe check of req.CustomProviderConfig.BaseProviderType so Vertex-based custom providers are treated the same as schemas.Vertex for resumable uploads.
🤖 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 `@core/providers/vertex/vertex.go`:
- Around line 2927-2931: The code only handles ExtraParams["content_length"]
when it's a float64; update the handling in the block around
request.ExtraParams/content_length so it also accepts string and integer forms:
check for string (use strconv.ParseInt or ParseUint to convert and verify >0)
and for integer types (int, int64, uint64) via type assertions, then set
X-Upload-Content-Length with fmt.Sprintf("%d", parsedValue) as currently done;
keep the existing float64 branch but consolidate into a single parsed
int64/uint64 value check before calling req.Header.Set.
- Around line 2665-2679: gcsResolveBucket currently trusts untrusted inputs (gcs
param and extraParams keys "gcs_bucket"/"gcs_prefix") and returns bucket/prefix
used for GCS operations; change this so you validate and enforce an allowlist
before any provider call: ensure gcsResolveBucket (and every caller that parses
"gs://..." IDs or uses its return values) checks the resolved bucket and prefix
against a configured allowlist or an allowlisted value passed from the
handler/core layer, and return an error (or empty + error) if not allowed; do
the authorization/validation step before any GCS client calls (list/read/delete)
and prefer threading an approved bucket/prefix from the higher-level handler
instead of trusting request-controlled extraParams.
- Around line 3253-3279: The code currently copies resp.Body() into a second
byte slice (in the FileContent flow) causing double-buffering of large GCS
objects; instead, avoid materializing the whole body by using fasthttp's
streaming API or enforcing a size cap before reading. Update the download path
around MakeRequestWithContext / resp to either (a) use resp.BodyStream() (or the
repo's large-response/streaming helper) and io.Copy to a provided writer /
return an io.ReadCloser so the object is streamed without a full in-memory copy,
or (b) if streaming isn't feasible, check resp.Header.ContentLength (via
resp.Header.Peek or resp.Header.ContentLength) and return an error if it exceeds
the configured max before calling resp.Body(); also remove the manual copy from
resp.Body() to the content slice and eliminate double buffering; keep existing
error handling that calls parseGCSAPIError and removeVertexClient as-is.
In `@ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx`:
- Line 17: BATCH_SUPPORTED_PROVIDERS now includes "vertex", which causes the
generic render of BatchAPIFormField (bound to key.use_for_batch_api) and the
Vertex-specific render to both appear; remove the duplicate by deleting the
Vertex-specific BatchAPIFormField render inside the Vertex section (leave the
generic BatchAPIFormField controlled by BATCH_SUPPORTED_PROVIDERS), or
alternatively remove "vertex" from BATCH_SUPPORTED_PROVIDERS—prefer keeping the
generic path and removing the Vertex-specific render to avoid two controls bound
to key.use_for_batch_api.
---
Duplicate comments:
In `@core/bifrost.go`:
- Line 2201: The empty-file gate currently exempts only when req.Provider ==
schemas.Vertex but still rejects requests for custom providers whose
CustomProviderConfig.BaseProviderType == schemas.Vertex; update the condition to
allow empty req.File when either req.Provider == schemas.Vertex OR
(req.CustomProviderConfig != nil && req.CustomProviderConfig.BaseProviderType ==
schemas.Vertex). Concretely, change the if check around req.File to include a
nil-safe check of req.CustomProviderConfig.BaseProviderType so Vertex-based
custom providers are treated the same as schemas.Vertex for resumable uploads.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e1f000c5-53c8-46b8-a9dd-af84ed0f8349
📒 Files selected for processing (6)
core/bifrost.gocore/providers/vertex/types.gocore/providers/vertex/vertex.gocore/schemas/files.gotransports/bifrost-http/handlers/inference.goui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
TejasGhatte
force-pushed
the
06-03-feat_vertex_files_api
branch
from
June 9, 2026 05:26
f6ba673 to
12bdf28
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@core/providers/vertex/vertex.go`:
- Around line 2871-2875: The GCS multipart upload requests build the
fasthttp.Request manually (see calls like req.SetRequestURI,
req.Header.SetMethod, req.Header.SetContentType, req.SetBody) but never call
providerUtils.SetExtraHeaders, so request-scoped x-bf-eh-* and provider
ExtraHeaders are dropped; fix by invoking providerUtils.SetExtraHeaders(req,
&provider.Config{ExtraHeaders: cfg.ExtraHeaders}) (or the existing provider
config object) after setting headers like Authorization and Content-Type and
before sending the request; apply the same change to the other similar request
sites that construct GCS requests (the blocks around the
SetRequestURI/SetContentType/SetBody patterns cited) so all file operations
forward filtered extra headers.
- Around line 3007-3018: The code builds URL params but never copies
request.ExtraParams into params, so caller-supplied query params (like pageToken
or GCS filters) are ignored; fix by iterating request.ExtraParams and adding
each key/value into params before you set the explicit fields, then keep the
existing logic that sets prefix (if prefix != ""), computes/sets maxResults from
request.Limit, and sets pageToken only when nativeCursor != "" so the internal
cursor still overrides a provided pageToken when present; reference the params
variable, request.ExtraParams, nativeCursor, prefix, and maxResults/pageToken
keys when making the change.
In `@transports/bifrost-http/handlers/inference.go`:
- Around line 3052-3079: The handler currently allows both fileData (from
form.File) and filename (from form.Value["filename"]) to be empty which sends
nil/empty values to the provider; add a validation after the block that extracts
fileHeaders/filename (after the form parsing and before calling the provider)
that checks if len(fileHeaders)==0 && filename=="" and if so calls
SendError(ctx, fasthttp.StatusBadRequest, "either file or filename is required")
and returns; reference the variables fileHeaders, filename, fileData and the
SendError(ctx, ...) call so the check is placed in the same scope as the
existing file extraction logic.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0fead73b-7f0a-46da-9c7b-2a5796c59a58
📒 Files selected for processing (6)
core/bifrost.gocore/providers/vertex/types.gocore/providers/vertex/vertex.gocore/schemas/files.gotransports/bifrost-http/handlers/inference.goui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
TejasGhatte
force-pushed
the
06-03-feat_vertex_files_api
branch
from
June 9, 2026 09:09
12bdf28 to
4c8b9d5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@core/providers/vertex/vertex.go`:
- Around line 2758-2765: The FileUpload and FileList handlers currently only
read bucket/prefix from request.StorageConfig.GCS, causing requests that supply
gcs_bucket/gcs_prefix via ExtraParams to fail; update both functions (FileUpload
and FileList) to fallback to request.ExtraParams values when StorageConfig.GCS
is nil or its fields are empty: if bucket == "" then set bucket =
request.ExtraParams["gcs_bucket"] (and similarly prefix =
request.ExtraParams["gcs_prefix"]) before returning the
NewBifrostOperationError, ensuring the same logic is applied at the other
occurrence noted (around lines handling bucket/prefix at the second location).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d7ee353-1098-4a16-bacb-30faf330ee07
📒 Files selected for processing (6)
core/bifrost.gocore/providers/vertex/types.gocore/providers/vertex/vertex.gocore/schemas/files.gotransports/bifrost-http/handlers/inference.goui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
TejasGhatte
force-pushed
the
06-03-feat_vertex_files_api
branch
from
June 9, 2026 11:28
4c8b9d5 to
1315a19
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@core/bifrost.go`:
- Around line 4720-4722: When enforceRoutingAllowlist(...) returns a non-nil
allowlistErr, you must invoke the same downstream drain/hooks that flush
PreRequestHook logs before returning to avoid log bleed; update the error-return
branch that checks (fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req,
provider, model, fallbacks); allowlistErr != nil) to call the downstream drain
hook(s) used elsewhere (the routine that flushes plugin logs for PreRequestHook)
with the current ctx/req/provider/model/fallbacks, then return allowlistErr.
In `@core/providers/vertex/vertex.go`:
- Around line 2804-2807: The branch incorrectly treats len(request.File) == 0 as
“no file provided”; instead add an explicit presence flag (e.g.,
request.FileProvided bool or make File nullable) at the handler/schema level and
use that to decide between provider.gcsFileUploadResumable and
provider.gcsFileUploadDirect; update the upload decision in the code that calls
gcsFileUploadResumable/gcsFileUploadDirect to check request.FileProvided (true
=> call gcsFileUploadDirect, even if file length is 0 to allow zero-byte direct
uploads; false => call gcsFileUploadResumable) and add tests covering both an
omitted-file resumable initiation and a zero-byte direct upload.
- Around line 2677-2687: parseGCSURI currently accepts bucket-only URIs and
returns (bucket, "", nil), which lets callers like the GCS
retrieve/delete/content request builders proceed with an empty object key;
change parseGCSURI so that when no '/' is found after the "gs://" prefix (i.e.,
bucket-only URIs like "gs://bucket") it returns an error instead of a nil error
and empty objectKey. Update the error message to clearly state the URI must
include an object path (e.g., "invalid GCS URI %q: must be in format
gs://bucket/object"), keeping the function name parseGCSURI as the locus of
validation so callers (retrieve/delete/content request builders) never receive
an empty objectKey.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70f1f706-a1c9-417f-9b33-dada869655cc
📒 Files selected for processing (6)
core/bifrost.gocore/providers/vertex/types.gocore/providers/vertex/vertex.gocore/schemas/files.gotransports/bifrost-http/handlers/inference.goui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
💤 Files with no reviewable changes (2)
- ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
- transports/bifrost-http/handlers/inference.go
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🤖 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 `@core/bifrost.go`:
- Around line 4720-4722: When enforceRoutingAllowlist(...) returns a non-nil
allowlistErr, you must invoke the same downstream drain/hooks that flush
PreRequestHook logs before returning to avoid log bleed; update the error-return
branch that checks (fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req,
provider, model, fallbacks); allowlistErr != nil) to call the downstream drain
hook(s) used elsewhere (the routine that flushes plugin logs for PreRequestHook)
with the current ctx/req/provider/model/fallbacks, then return allowlistErr.
In `@core/providers/vertex/vertex.go`:
- Around line 2804-2807: The branch incorrectly treats len(request.File) == 0 as
“no file provided”; instead add an explicit presence flag (e.g.,
request.FileProvided bool or make File nullable) at the handler/schema level and
use that to decide between provider.gcsFileUploadResumable and
provider.gcsFileUploadDirect; update the upload decision in the code that calls
gcsFileUploadResumable/gcsFileUploadDirect to check request.FileProvided (true
=> call gcsFileUploadDirect, even if file length is 0 to allow zero-byte direct
uploads; false => call gcsFileUploadResumable) and add tests covering both an
omitted-file resumable initiation and a zero-byte direct upload.
- Around line 2677-2687: parseGCSURI currently accepts bucket-only URIs and
returns (bucket, "", nil), which lets callers like the GCS
retrieve/delete/content request builders proceed with an empty object key;
change parseGCSURI so that when no '/' is found after the "gs://" prefix (i.e.,
bucket-only URIs like "gs://bucket") it returns an error instead of a nil error
and empty objectKey. Update the error message to clearly state the URI must
include an object path (e.g., "invalid GCS URI %q: must be in format
gs://bucket/object"), keeping the function name parseGCSURI as the locus of
validation so callers (retrieve/delete/content request builders) never receive
an empty objectKey.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70f1f706-a1c9-417f-9b33-dada869655cc
📒 Files selected for processing (6)
core/bifrost.gocore/providers/vertex/types.gocore/providers/vertex/vertex.gocore/schemas/files.gotransports/bifrost-http/handlers/inference.goui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
💤 Files with no reviewable changes (2)
- ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
- transports/bifrost-http/handlers/inference.go
🛑 Comments failed to post (3)
core/bifrost.go (1)
4720-4722:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlush plugin logs before allowlist early-return.
Line 4720 and Line 4853 return before downstream drain hooks run; this can leave
PreRequestHooklogs buffered on reused contexts and leak them into later traces/requests.💡 Suggested patch
- if fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req, provider, model, fallbacks); allowlistErr != nil { + if fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req, provider, model, fallbacks); allowlistErr != nil { + flushPluginLogs(ctx) return nil, allowlistErr } ... - if fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req, provider, model, fallbacks); allowlistErr != nil { + if fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req, provider, model, fallbacks); allowlistErr != nil { + flushPluginLogs(ctx) return nil, allowlistErr }Also applies to: 4853-4855
🤖 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 `@core/bifrost.go` around lines 4720 - 4722, When enforceRoutingAllowlist(...) returns a non-nil allowlistErr, you must invoke the same downstream drain/hooks that flush PreRequestHook logs before returning to avoid log bleed; update the error-return branch that checks (fallbacks, allowlistErr = enforceRoutingAllowlist(ctx, req, provider, model, fallbacks); allowlistErr != nil) to call the downstream drain hook(s) used elsewhere (the routine that flushes plugin logs for PreRequestHook) with the current ctx/req/provider/model/fallbacks, then return allowlistErr.Source: Coding guidelines
core/providers/vertex/vertex.go (2)
2677-2687:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject bucket-only
gs://URIs before issuing object requests.
parseGCSURIcurrently returns(bucket, "", nil)forgs://bucket, and the retrieve/delete/content paths then build/o/requests from that empty object key. That turns a simple validation error into a provider call with malformed input.As per coding guidelines, validate all untrusted input before provider calls.
♻️ Minimal fix
func parseGCSURI(uri string) (bucket, objectKey string, err error) { if !strings.HasPrefix(uri, "gs://") { return "", "", fmt.Errorf("invalid GCS URI %q: must start with gs://", uri) } rest := strings.TrimPrefix(uri, "gs://") idx := strings.IndexByte(rest, '/') - if idx < 0 { - return rest, "", nil + if idx <= 0 || idx == len(rest)-1 { + return "", "", fmt.Errorf("invalid GCS URI %q: must include bucket and object path", uri) } return rest[:idx], rest[idx+1:], nil }🤖 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 `@core/providers/vertex/vertex.go` around lines 2677 - 2687, parseGCSURI currently accepts bucket-only URIs and returns (bucket, "", nil), which lets callers like the GCS retrieve/delete/content request builders proceed with an empty object key; change parseGCSURI so that when no '/' is found after the "gs://" prefix (i.e., bucket-only URIs like "gs://bucket") it returns an error instead of a nil error and empty objectKey. Update the error message to clearly state the URI must include an object path (e.g., "invalid GCS URI %q: must be in format gs://bucket/object"), keeping the function name parseGCSURI as the locus of validation so callers (retrieve/delete/content request builders) never receive an empty objectKey.Source: Coding guidelines
2804-2807:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't infer “resumable upload” from
len(request.File) == 0.This now conflates two different cases: “the client omitted the
filefield” and “the client uploaded a real zero-byte file”. With the new transport contract, an empty direct upload will be misclassified as a resumable session and come back aspending_uploadinstead of creating the empty object.Please thread an explicit presence signal from the handler/schema (for example
FileProvided boolor a nullable file field) and add coverage for both zero-byte direct uploads and omitted-file resumable initiation. Based on PR objectives, the multipartfilefield is now optional, so size-based branching here is no longer a safe discriminator.🤖 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 `@core/providers/vertex/vertex.go` around lines 2804 - 2807, The branch incorrectly treats len(request.File) == 0 as “no file provided”; instead add an explicit presence flag (e.g., request.FileProvided bool or make File nullable) at the handler/schema level and use that to decide between provider.gcsFileUploadResumable and provider.gcsFileUploadDirect; update the upload decision in the code that calls gcsFileUploadResumable/gcsFileUploadDirect to check request.FileProvided (true => call gcsFileUploadDirect, even if file length is 0 to allow zero-byte direct uploads; false => call gcsFileUploadResumable) and add tests covering both an omitted-file resumable initiation and a zero-byte direct upload.
Collaborator
Merge activity
|
Pratham-Mishra04
force-pushed
the
06-03-feat_vertex_files_api
branch
from
June 9, 2026 13:58
1315a19 to
d450ea9
Compare
18 tasks
akshaydeo
pushed a commit
that referenced
this pull request
Jun 12, 2026
## Summary Implements the full GCS-backed File API for the Vertex AI provider, replacing the previous stub implementations that returned `UnsupportedOperation` errors. Vertex AI uses Google Cloud Storage rather than a native file store, so all file operations (upload, list, retrieve, delete, content download) are mapped to GCS JSON API calls authenticated via the existing Vertex credential chain. ## Changes - **Vertex `FileUpload`**: Supports two modes — direct upload (multipart/related to GCS when file bytes are provided) and resumable session initiation (returns a GCS `Location` URL when no bytes are provided, allowing the client to PUT bytes directly to GCS). A new `UploadURL` field is added to `BifrostFileUploadResponse` to carry the session URL. - **Vertex `FileList`**: Lists GCS objects under a configurable bucket/prefix. Supports cursor-based pagination via `pageToken`. - **Vertex `FileRetrieve`**: Fetches GCS object metadata by `gs://` URI. - **Vertex `FileDelete`**: Deletes a GCS object by `gs://` URI. Treats 404 as success for idempotency. - **Vertex `FileContent`**: Downloads raw object bytes from GCS by `gs://` URI. - **GCS helpers**: Added `gcsResolveBucket`, `gcsObjectKey`, `gcsEncodeObjectName`, `parseGCSURI`, `gcsMetadataToFileObject`, `gcsGetAuthHeader`, and `parseGCSAPIError` to support the above operations. Bucket and prefix can be supplied via `StorageConfig.GCS` or `extra_params["gcs_bucket"]`/`extra_params["gcs_prefix"]`. - **New GCS types**: `gcsObjectMetadata`, `gcsObjectListResponse`, and `gcsErrorBody` added to `vertex/types.go`. - **`FileStatusPendingUpload`**: New `FileStatus` constant representing a resumable session that has been minted but whose bytes have not yet been received. - **`bifrost.go` validation**: The empty-file guard is skipped for Vertex, since resumable uploads intentionally omit file bytes. - **HTTP transport `fileUpload`**: The `file` multipart field is now optional. When absent, a `filename` form field is accepted instead. `content_type` and arbitrary extra form fields (e.g. `gcs_bucket`, `gcs_prefix`) are forwarded to the provider. - **HTTP transport `fileList`**: Unknown query args are collected and forwarded as `ExtraParams` so storage-backed providers can receive `gcs_bucket` etc. - **HTTP transport file ID decoding**: `fileRetrieve`, `fileDelete`, and `fileContent` now percent-decode the file ID path segment, allowing `gs://` and `s3://` URIs to be passed safely in URL paths. - **`DisablePathNormalizing`**: Enabled on the Vertex fasthttp client to prevent path normalization from mangling percent-encoded GCS object names. - **UI**: Vertex is added to `BATCH_SUPPORTED_PROVIDERS` and the missing `BatchAPIFormField` is rendered for providers that support batch. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./... # Direct upload (file bytes provided) curl -X POST http://localhost:8080/v1/files \ -F "provider=vertex" \ -F "purpose=batch" \ -F "gcs_bucket=my-bucket" \ -F "file=@/path/to/file.jsonl" # Expected: 200 with status=processed and storage_uri=gs://my-bucket/... # Resumable upload session (no file bytes) curl -X POST http://localhost:8080/v1/files \ -F "provider=vertex" \ -F "purpose=batch" \ -F "gcs_bucket=my-bucket" \ -F "filename=input.jsonl" \ -F "content_type=application/jsonl" # Expected: 200 with status=pending_upload and upload_url set # List files curl "http://localhost:8080/v1/files?provider=vertex&gcs_bucket=my-bucket" # Retrieve metadata (gs:// URI must be percent-encoded in path) curl "http://localhost:8080/v1/files/gs%3A%2F%2Fmy-bucket%2Fvertex-files%2F...?provider=vertex" # Delete curl -X DELETE "http://localhost:8080/v1/files/gs%3A%2F%2Fmy-bucket%2Fvertex-files%2F...?provider=vertex" # Download content curl "http://localhost:8080/v1/files/gs%3A%2F%2Fmy-bucket%2Fvertex-files%2F.../content?provider=vertex" ``` GCS bucket must be provided either in `StorageConfig.GCS.Bucket` or via the `gcs_bucket` extra param. An optional `gcs_prefix` scopes object keys within the bucket. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations - GCS requests are authenticated using the existing Vertex credential chain (`getAuthTokenSource`). Tokens are short-lived and refreshed automatically; stale token sources are evicted on refresh failure. - File IDs for Vertex are `gs://` URIs. Callers supplying arbitrary file IDs to retrieve/delete/content endpoints should validate that URIs reference expected buckets before forwarding to the API. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Vertex provider: full file support — upload (multipart & resumable with returned upload URL), list, retrieve, delete, and download. * UI: Vertex keys can be marked for batch API usage. * **Improvements** * File uploads may omit bytes; filename, content_type, and unknown form/query fields are preserved as extra params. * File IDs with special characters are percent-decoded. * Deletes are idempotent (missing objects treated as success). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 tasks
akhsaul
pushed a commit
to akhsaul/bifrost
that referenced
this pull request
Aug 27, 2026
## Summary Implements the full GCS-backed File API for the Vertex AI provider, replacing the previous stub implementations that returned `UnsupportedOperation` errors. Vertex AI uses Google Cloud Storage rather than a native file store, so all file operations (upload, list, retrieve, delete, content download) are mapped to GCS JSON API calls authenticated via the existing Vertex credential chain. ## Changes - **Vertex `FileUpload`**: Supports two modes — direct upload (multipart/related to GCS when file bytes are provided) and resumable session initiation (returns a GCS `Location` URL when no bytes are provided, allowing the client to PUT bytes directly to GCS). A new `UploadURL` field is added to `BifrostFileUploadResponse` to carry the session URL. - **Vertex `FileList`**: Lists GCS objects under a configurable bucket/prefix. Supports cursor-based pagination via `pageToken`. - **Vertex `FileRetrieve`**: Fetches GCS object metadata by `gs://` URI. - **Vertex `FileDelete`**: Deletes a GCS object by `gs://` URI. Treats 404 as success for idempotency. - **Vertex `FileContent`**: Downloads raw object bytes from GCS by `gs://` URI. - **GCS helpers**: Added `gcsResolveBucket`, `gcsObjectKey`, `gcsEncodeObjectName`, `parseGCSURI`, `gcsMetadataToFileObject`, `gcsGetAuthHeader`, and `parseGCSAPIError` to support the above operations. Bucket and prefix can be supplied via `StorageConfig.GCS` or `extra_params["gcs_bucket"]`/`extra_params["gcs_prefix"]`. - **New GCS types**: `gcsObjectMetadata`, `gcsObjectListResponse`, and `gcsErrorBody` added to `vertex/types.go`. - **`FileStatusPendingUpload`**: New `FileStatus` constant representing a resumable session that has been minted but whose bytes have not yet been received. - **`bifrost.go` validation**: The empty-file guard is skipped for Vertex, since resumable uploads intentionally omit file bytes. - **HTTP transport `fileUpload`**: The `file` multipart field is now optional. When absent, a `filename` form field is accepted instead. `content_type` and arbitrary extra form fields (e.g. `gcs_bucket`, `gcs_prefix`) are forwarded to the provider. - **HTTP transport `fileList`**: Unknown query args are collected and forwarded as `ExtraParams` so storage-backed providers can receive `gcs_bucket` etc. - **HTTP transport file ID decoding**: `fileRetrieve`, `fileDelete`, and `fileContent` now percent-decode the file ID path segment, allowing `gs://` and `s3://` URIs to be passed safely in URL paths. - **`DisablePathNormalizing`**: Enabled on the Vertex fasthttp client to prevent path normalization from mangling percent-encoded GCS object names. - **UI**: Vertex is added to `BATCH_SUPPORTED_PROVIDERS` and the missing `BatchAPIFormField` is rendered for providers that support batch. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./... # Direct upload (file bytes provided) curl -X POST http://localhost:8080/v1/files \ -F "provider=vertex" \ -F "purpose=batch" \ -F "gcs_bucket=my-bucket" \ -F "file=@/path/to/file.jsonl" # Expected: 200 with status=processed and storage_uri=gs://my-bucket/... # Resumable upload session (no file bytes) curl -X POST http://localhost:8080/v1/files \ -F "provider=vertex" \ -F "purpose=batch" \ -F "gcs_bucket=my-bucket" \ -F "filename=input.jsonl" \ -F "content_type=application/jsonl" # Expected: 200 with status=pending_upload and upload_url set # List files curl "http://localhost:8080/v1/files?provider=vertex&gcs_bucket=my-bucket" # Retrieve metadata (gs:// URI must be percent-encoded in path) curl "http://localhost:8080/v1/files/gs%3A%2F%2Fmy-bucket%2Fvertex-files%2F...?provider=vertex" # Delete curl -X DELETE "http://localhost:8080/v1/files/gs%3A%2F%2Fmy-bucket%2Fvertex-files%2F...?provider=vertex" # Download content curl "http://localhost:8080/v1/files/gs%3A%2F%2Fmy-bucket%2Fvertex-files%2F.../content?provider=vertex" ``` GCS bucket must be provided either in `StorageConfig.GCS.Bucket` or via the `gcs_bucket` extra param. An optional `gcs_prefix` scopes object keys within the bucket. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations - GCS requests are authenticated using the existing Vertex credential chain (`getAuthTokenSource`). Tokens are short-lived and refreshed automatically; stale token sources are evicted on refresh failure. - File IDs for Vertex are `gs://` URIs. Callers supplying arbitrary file IDs to retrieve/delete/content endpoints should validate that URIs reference expected buckets before forwarding to the API. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Vertex provider: full file support — upload (multipart & resumable with returned upload URL), list, retrieve, delete, and download. * UI: Vertex keys can be marked for batch API usage. * **Improvements** * File uploads may omit bytes; filename, content_type, and unknown form/query fields are preserved as extra params. * File IDs with special characters are percent-decoded. * Deletes are idempotent (missing objects treated as success). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
3 participants
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
Implements the full GCS-backed File API for the Vertex AI provider, replacing the previous stub implementations that returned
UnsupportedOperationerrors. Vertex AI uses Google Cloud Storage rather than a native file store, so all file operations (upload, list, retrieve, delete, content download) are mapped to GCS JSON API calls authenticated via the existing Vertex credential chain.Changes
FileUpload: Supports two modes — direct upload (multipart/related to GCS when file bytes are provided) and resumable session initiation (returns a GCSLocationURL when no bytes are provided, allowing the client to PUT bytes directly to GCS). A newUploadURLfield is added toBifrostFileUploadResponseto carry the session URL.FileList: Lists GCS objects under a configurable bucket/prefix. Supports cursor-based pagination viapageToken.FileRetrieve: Fetches GCS object metadata bygs://URI.FileDelete: Deletes a GCS object bygs://URI. Treats 404 as success for idempotency.FileContent: Downloads raw object bytes from GCS bygs://URI.gcsResolveBucket,gcsObjectKey,gcsEncodeObjectName,parseGCSURI,gcsMetadataToFileObject,gcsGetAuthHeader, andparseGCSAPIErrorto support the above operations. Bucket and prefix can be supplied viaStorageConfig.GCSorextra_params["gcs_bucket"]/extra_params["gcs_prefix"].gcsObjectMetadata,gcsObjectListResponse, andgcsErrorBodyadded tovertex/types.go.FileStatusPendingUpload: NewFileStatusconstant representing a resumable session that has been minted but whose bytes have not yet been received.bifrost.govalidation: The empty-file guard is skipped for Vertex, since resumable uploads intentionally omit file bytes.fileUpload: Thefilemultipart field is now optional. When absent, afilenameform field is accepted instead.content_typeand arbitrary extra form fields (e.g.gcs_bucket,gcs_prefix) are forwarded to the provider.fileList: Unknown query args are collected and forwarded asExtraParamsso storage-backed providers can receivegcs_bucketetc.fileRetrieve,fileDelete, andfileContentnow percent-decode the file ID path segment, allowinggs://ands3://URIs to be passed safely in URL paths.DisablePathNormalizing: Enabled on the Vertex fasthttp client to prevent path normalization from mangling percent-encoded GCS object names.BATCH_SUPPORTED_PROVIDERSand the missingBatchAPIFormFieldis rendered for providers that support batch.Type of change
Affected areas
How to test
GCS bucket must be provided either in
StorageConfig.GCS.Bucketor via thegcs_bucketextra param. An optionalgcs_prefixscopes object keys within the bucket.Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
getAuthTokenSource). Tokens are short-lived and refreshed automatically; stale token sources are evicted on refresh failure.gs://URIs. Callers supplying arbitrary file IDs to retrieve/delete/content endpoints should validate that URIs reference expected buckets before forwarding to the API.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
Improvements