fix: tests for vertex files api - #4256
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 3 minutes and 58 seconds. Learn how PR review limits work. To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds Vertex (GCS) Files API support: normalize resumable upload content length, make gs:// file IDs opaque/path-safe, extract GCS storage config from requests, update Makefile/test harness, and add Postman and Python integration tests for Vertex file flows. ChangesVertex GCS Files API Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 |
|
|
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
TejasGhatte
marked this pull request as ready for review
June 10, 2026 14:14
Contributor
Confidence Score: 4/5Safe to merge with a minor test-coverage gap: Bedrock file-retrieve is silently skipped after the scenario rename. The core logic changes are well-structured — the base64 encoding/decoding helpers use tests/integrations/python/config.yml — the Bedrock provider block is missing a Important Files Changed
Reviews (5): Last reviewed commit: "fix: tests for 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: 5
🤖 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 3566-3580: The string form of content_length in
request.ExtraParams is being parsed with gcsParseSize which uses fmt.Sscanf and
accepts partial strings (e.g. "123abc"); change the string handling in the
switch for request.ExtraParams["content_length"] so that it strictly validates
and parses the entire string (use a full-match parse like
strconv.ParseInt/ParseUint with base 10 and check for errors or a regexp that
ensures only digits) and handle parse errors explicitly (e.g., reject/mask the
value or return an error) before assigning to contentLength; update gcsParseSize
or replace its usage from the string case to ensure no partial/malformed input
can set X-Upload-Content-Length.
In `@Makefile`:
- Around line 1898-1900: Update the Makefile HELP text to document the three new
Vertex env vars that are forwarded to Newman: add entries for VERTEX_GCS_BUCKET
(GCS bucket for Vertex file operations, passed to Newman as vertexGcsBucket),
VERTEX_GCS_PREFIX (GCS object prefix, passed as vertexGcsPrefix), and
VERTEX_API_KEY (Vertex service account credentials, passed as vertexKey); follow
the existing Bedrock example style used for
BEDROCK_GUARDRAIL_IDENTIFIER/BEDROCK_GUARDRAIL_VERSION in the HELP block and add
the same three lines to the equivalent documentation blocks referenced (also
update the help text near the occurrences at the other two locations
corresponding to the parallel/sequential CI execution paths).
In `@tests/integrations/python/tests/test_openai.py`:
- Line 2862: The test function test_41_file_upload has an unused fixture
parameter test_config causing a lint ARG002; remove the unused parameter or
rename it to start with an underscore (e.g., _test_config) in the
test_41_file_upload signature so Ruff no longer flags it, keeping other
parameters (provider, model, vk_enabled) intact.
In `@transports/bifrost-http/handlers/inference.go`:
- Around line 3009-3018: The encodeStorageFileID function only encodes gs://
URIs and uses base64.StdEncoding which can produce '/' and '+' that break URL
paths; update encodeStorageFileID to treat both "gs://" and "s3://" prefixes
(e.g., strings.HasPrefix(id, "gs://") || strings.HasPrefix(id, "s3://")) and
encode using base64.URLEncoding.EncodeToString to produce URL-safe base64, and
update the corresponding decodeStorageFileID to use
base64.URLEncoding.DecodeString so decoding matches the new encoding.
In `@transports/bifrost-http/integrations/openai.go`:
- Around line 1730-1734: The converter currently encodes resp.ID for
schemas.Bedrock and schemas.Vertex with base64.StdEncoding which can emit "/"
and break single-segment routing, and extractFileIDFromPath silently ignores
decode errors; change the encoder in the switch handling schemas.Bedrock and
schemas.Vertex to use a URL-safe base64 alphabet (base64.URLEncoding or
base64.RawURLEncoding) for Vertex IDs (and optionally Raw for padding behavior)
while keeping a fallback decode path in extractFileIDFromPath that first
attempts URL-safe decoding and, if that fails, tries StdEncoding for backward
compatibility with existing Bedrock IDs; if both decodes fail, return a
transport-level 4xx error (fail closed) instead of passing the malformed id to
provider 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 Plus
Run ID: 76788d67-6c8c-41e3-8a90-8489c3770a1e
📒 Files selected for processing (7)
Makefilecore/providers/vertex/vertex.gotests/e2e/api/collections/provider-harness.jsontests/integrations/python/config.ymltests/integrations/python/tests/test_openai.pytransports/bifrost-http/handlers/inference.gotransports/bifrost-http/integrations/openai.go
TejasGhatte
force-pushed
the
06-10-fix_tests_for_vertex_files_api
branch
from
June 10, 2026 20:00
c351a43 to
e4b407d
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
core/providers/vertex/vertex.go (1)
3571-3580:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse strict integer parsing for
content_lengthstring/float inputs.
gcsParseSize(viafmt.Sscanf) accepts partial strings, so malformed values like"123abc"can silently setX-Upload-Content-Lengthto123. Also,float64values are currently truncated without integer validation. Please only accept fully valid positive integers.💡 Suggested fix
@@ import ( @@ "fmt" @@ + "math" @@ + "strconv" @@ - case float64: - contentLength = int64(cl) + case float64: + if cl > 0 && cl == math.Trunc(cl) && cl <= float64(math.MaxInt64) { + contentLength = int64(cl) + } @@ - case string: - contentLength = gcsParseSize(cl) + case string: + if parsed, err := strconv.ParseInt(strings.TrimSpace(cl), 10, 64); err == nil && parsed > 0 { + contentLength = parsed + }As per coding guidelines, validate all untrusted input and keep explicit error handling for request-derived values.
🤖 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 3571 - 3580, The code currently accepts partial/malformed content_length via gcsParseSize and truncates float64 values; update the handling of request.ExtraParams["content_length"] so strings are parsed with strict integer parsing (use strconv.ParseInt with 10, 64-bit and verify the entire string was numeric and >0) instead of gcsParseSize, and floats are only accepted if they represent exact integers (check math.Modf or compare int64 casting back) and positive; if parsing/validation fails, do not set contentLength (or return an error) so X-Upload-Content-Length is not populated incorrectly. Locate the switch around request.ExtraParams["content_length"], gcsParseSize, and the contentLength variable to implement these checks.Source: Coding guidelines
transports/bifrost-http/handlers/inference.go (2)
3020-3033:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtend decoding to
s3://URIs (Bedrock files).
decodeStorageFileIDonly checks for thegs://prefix after base64 decoding. To fully support Bedrock file operations (which uses3://URIs), the prefix check should accept both storage providers:func decodeStorageFileID(id string) string { if unescaped, err := url.PathUnescape(id); err == nil { id = unescaped } - if decoded, err := base64.RawURLEncoding.DecodeString(id); err == nil && strings.HasPrefix(string(decoded), "gs://") { + if decoded, err := base64.RawURLEncoding.DecodeString(id); err == nil && (strings.HasPrefix(string(decoded), "gs://") || strings.HasPrefix(string(decoded), "s3://")) { return string(decoded) } return id }This change completes the path-safety encoding/decoding for all storage-backed file providers (Vertex GCS and Bedrock S3).
🤖 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 `@transports/bifrost-http/handlers/inference.go` around lines 3020 - 3033, The decodeStorageFileID function only treats base64-decoded values as storage URIs if they start with "gs://", so s3:// Bedrock file URIs are missed; update decodeStorageFileID to consider both "gs://" and "s3://" prefixes after base64.RawURLEncoding.DecodeString succeeds (i.e., check strings.HasPrefix(string(decoded), "gs://") || strings.HasPrefix(string(decoded), "s3://")), ensuring PathUnescape behavior remains the same and the function returns the decoded URI for either storage provider.
3009-3018:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtend encoding to
s3://URIs (Bedrock files).
encodeStorageFileIDonly base64-encodesgs://URIs, buts3://URIs (used by Bedrock file operations) face the same path-safety issue—slashes ins3://bucket/key/pathcannot appear raw in URL path segments. Without encoding, Bedrock file IDs with path-like structure will break retrieval/deletion requests unless clients manually percent-encode every slash.The code already uses
base64.RawURLEncoding(URL-safe, correct), but it should handle both storage providers:func encodeStorageFileID(id string) string { - if strings.HasPrefix(id, "gs://") { + if strings.HasPrefix(id, "gs://") || strings.HasPrefix(id, "s3://") { return base64.RawURLEncoding.EncodeToString([]byte(id)) } return id }🤖 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 `@transports/bifrost-http/handlers/inference.go` around lines 3009 - 3018, encodeStorageFileID currently base64-encodes only "gs://" URIs so "s3://" (Bedrock) storage URIs with slashes break path usage; update the function (encodeStorageFileID) to treat both "gs://" and "s3://" prefixes as opaque by encoding them with base64.RawURLEncoding.EncodeToString([]byte(id)) and returning unmodified for other providers, and adjust the function comment to reflect support for both providers.
🤖 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.
Duplicate comments:
In `@core/providers/vertex/vertex.go`:
- Around line 3571-3580: The code currently accepts partial/malformed
content_length via gcsParseSize and truncates float64 values; update the
handling of request.ExtraParams["content_length"] so strings are parsed with
strict integer parsing (use strconv.ParseInt with 10, 64-bit and verify the
entire string was numeric and >0) instead of gcsParseSize, and floats are only
accepted if they represent exact integers (check math.Modf or compare int64
casting back) and positive; if parsing/validation fails, do not set
contentLength (or return an error) so X-Upload-Content-Length is not populated
incorrectly. Locate the switch around request.ExtraParams["content_length"],
gcsParseSize, and the contentLength variable to implement these checks.
In `@transports/bifrost-http/handlers/inference.go`:
- Around line 3020-3033: The decodeStorageFileID function only treats
base64-decoded values as storage URIs if they start with "gs://", so s3://
Bedrock file URIs are missed; update decodeStorageFileID to consider both
"gs://" and "s3://" prefixes after base64.RawURLEncoding.DecodeString succeeds
(i.e., check strings.HasPrefix(string(decoded), "gs://") ||
strings.HasPrefix(string(decoded), "s3://")), ensuring PathUnescape behavior
remains the same and the function returns the decoded URI for either storage
provider.
- Around line 3009-3018: encodeStorageFileID currently base64-encodes only
"gs://" URIs so "s3://" (Bedrock) storage URIs with slashes break path usage;
update the function (encodeStorageFileID) to treat both "gs://" and "s3://"
prefixes as opaque by encoding them with
base64.RawURLEncoding.EncodeToString([]byte(id)) and returning unmodified for
other providers, and adjust the function comment to reflect support for both
providers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4d366d12-68af-435a-ae74-315d38d7fa7e
📒 Files selected for processing (7)
Makefilecore/providers/vertex/vertex.gotests/e2e/api/collections/provider-harness.jsontests/integrations/python/config.ymltests/integrations/python/tests/test_openai.pytransports/bifrost-http/handlers/inference.gotransports/bifrost-http/integrations/openai.go
👮 Files not reviewed due to content moderation or server errors (1)
- tests/e2e/api/collections/provider-harness.json
TejasGhatte
force-pushed
the
06-10-fix_tests_for_vertex_files_api
branch
from
June 11, 2026 04:51
e4b407d to
ff6c165
Compare
Contributor
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 (2)
transports/bifrost-http/handlers/inference.go (2)
3690-3690:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecode
file_idin JSON body before passing to provider.
containerFileCreateaccepts afile_idin the JSON body (line 3690) to copy an existing file into the container. If the client obtained this file ID from a priorfileUploadresponse, it would be base64-encoded (for Vertex/GCS storage URIs). The provider expects the rawgs://URI, not the opaque base64 string. ApplydecodeStorageFileIDafter parsing the JSON body to maintain the encode-on-response / decode-on-request symmetry established in the file handlers.🔧 Proposed fix
if reqBody.FileID == "" { SendError(ctx, fasthttp.StatusBadRequest, "file_id is required in JSON body") return } - bifrostContainerFileReq.FileID = bifrost.Ptr(reqBody.FileID) + bifrostContainerFileReq.FileID = bifrost.Ptr(decodeStorageFileID(reqBody.FileID)) if reqBody.FilePath != "" { bifrostContainerFileReq.Path = bifrost.Ptr(reqBody.FilePath) }🤖 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 `@transports/bifrost-http/handlers/inference.go` at line 3690, The JSON body field reqBody.FileID must be decoded before passing to the provider: after parsing the request and before assigning to bifrostContainerFileReq.FileID (in the containerFileCreate handler), call decodeStorageFileID(reqBody.FileID) and assign the decoded storage URI (not the base64 token) to bifrostContainerFileReq.FileID; this preserves the encode-on-response/decode-on-request symmetry used by fileUpload and ensures the provider receives the raw gs:// URI.
2776-2776:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDecode
input_file_idbefore passing to provider.
batchCreateacceptsinput_file_idon line 2776 and passes it through to the provider without decoding. If the client obtained this file ID from a priorfileUploadresponse, it would be base64-encoded (for Vertex/GCS storage URIs). The provider expects the rawgs://URI, not the opaque base64 string. ApplydecodeStorageFileIDbefore constructingBifrostBatchCreateRequestto maintain the encode-on-response / decode-on-request symmetry established in the file handlers.🔧 Proposed fix
// Build Bifrost batch create request + inputFileID := req.InputFileID + if inputFileID != "" { + inputFileID = decodeStorageFileID(inputFileID) + } bifrostBatchReq := &schemas.BifrostBatchCreateRequest{ Provider: schemas.ModelProvider(provider), Model: model, - InputFileID: req.InputFileID, + InputFileID: inputFileID, InputBlob: req.InputBlob,🤖 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 `@transports/bifrost-http/handlers/inference.go` at line 2776, The InputFileID is passed to the provider without decoding in batchCreate; call decodeStorageFileID on req.InputFileID before constructing the BifrostBatchCreateRequest so the provider receives the raw gs:// URI. Update the batchCreate handler where BifrostBatchCreateRequest is built (reference: batchCreate, BifrostBatchCreateRequest, InputFileID) to replace direct use of req.InputFileID with the decoded value from decodeStorageFileID(req.InputFileID) and handle any decode error before sending to the provider.
♻️ Duplicate comments (1)
transports/bifrost-http/handlers/inference.go (1)
3009-3018:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffExtend encoding to s3:// URIs and use URL-safe base64.
encodeStorageFileIDonly base64-encodesgs://URIs, buts3://URIs (used by Bedrock file operations) face the same path-safety issue—slashes ins3://bucket/key/pathcannot appear raw in URL path segments. Without encoding, Bedrock file IDs with path-like structure will break retrieval/deletion requests unless clients manually percent-encode every slash.Additionally,
base64.StdEncodingcan produce/characters in its output (when encoding byte patterns that map to the 6-bit value 63), requiring clients to percent-encode the base64 string itself (as noted in thedecodeStorageFileIDcomment). This is fragile and error-prone. The idiomatic solution for URL-path-safe identifiers isbase64.URLEncoding, which uses-and_instead of+and/, eliminating the need for client-side percent-encoding of the ID body (only=padding needs encoding, which is unavoidable).Suggested fix: handle s3:// and switch to URLEncoding
func encodeStorageFileID(id string) string { - if strings.HasPrefix(id, "gs://") { - return base64.RawURLEncoding.EncodeToString([]byte(id)) + if strings.HasPrefix(id, "gs://") || strings.HasPrefix(id, "s3://") { + return base64.RawURLEncoding.EncodeToString([]byte(id)) } return id }Update
decodeStorageFileIDsimilarly:func decodeStorageFileID(id string) string { if unescaped, err := url.PathUnescape(id); err == nil { id = unescaped } - if decoded, err := base64.RawURLEncoding.DecodeString(id); err == nil && strings.HasPrefix(string(decoded), "gs://") { + if decoded, err := base64.RawURLEncoding.DecodeString(id); err == nil && (strings.HasPrefix(string(decoded), "gs://") || strings.HasPrefix(string(decoded), "s3://")) { return string(decoded) } return id }This change ensures Bedrock file IDs are path-safe and removes the client burden of percent-encoding
/in base64 strings.🤖 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 `@transports/bifrost-http/handlers/inference.go` around lines 3009 - 3018, Extend encodeStorageFileID to also encode s3:// URIs (in addition to gs://) and switch from base64.RawURLEncoding to base64.URLEncoding so the output uses URL-safe characters; likewise update decodeStorageFileID to decode using base64.URLEncoding to match. Locate the functions encodeStorageFileID and decodeStorageFileID and change their logic to treat strings.HasPrefix(id, "gs://") || strings.HasPrefix(id, "s3://") as the branch to base64.URLEncoding.EncodeToString/DecodeString, leaving non-storage IDs unchanged.
🤖 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 `@transports/bifrost-http/handlers/inference.go`:
- Line 3690: The JSON body field reqBody.FileID must be decoded before passing
to the provider: after parsing the request and before assigning to
bifrostContainerFileReq.FileID (in the containerFileCreate handler), call
decodeStorageFileID(reqBody.FileID) and assign the decoded storage URI (not the
base64 token) to bifrostContainerFileReq.FileID; this preserves the
encode-on-response/decode-on-request symmetry used by fileUpload and ensures the
provider receives the raw gs:// URI.
- Line 2776: The InputFileID is passed to the provider without decoding in
batchCreate; call decodeStorageFileID on req.InputFileID before constructing the
BifrostBatchCreateRequest so the provider receives the raw gs:// URI. Update the
batchCreate handler where BifrostBatchCreateRequest is built (reference:
batchCreate, BifrostBatchCreateRequest, InputFileID) to replace direct use of
req.InputFileID with the decoded value from decodeStorageFileID(req.InputFileID)
and handle any decode error before sending to the provider.
---
Duplicate comments:
In `@transports/bifrost-http/handlers/inference.go`:
- Around line 3009-3018: Extend encodeStorageFileID to also encode s3:// URIs
(in addition to gs://) and switch from base64.RawURLEncoding to
base64.URLEncoding so the output uses URL-safe characters; likewise update
decodeStorageFileID to decode using base64.URLEncoding to match. Locate the
functions encodeStorageFileID and decodeStorageFileID and change their logic to
treat strings.HasPrefix(id, "gs://") || strings.HasPrefix(id, "s3://") as the
branch to base64.URLEncoding.EncodeToString/DecodeString, leaving non-storage
IDs unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0bcb817a-c72a-48bc-a778-d0fca1f1cc4e
📒 Files selected for processing (7)
Makefilecore/providers/vertex/vertex.gotests/e2e/api/collections/provider-harness.jsontests/integrations/python/config.ymltests/integrations/python/tests/test_openai.pytransports/bifrost-http/handlers/inference.gotransports/bifrost-http/integrations/openai.go
👮 Files not reviewed due to content moderation or server errors (1)
- tests/e2e/api/collections/provider-harness.json
TejasGhatte
force-pushed
the
06-10-fix_tests_for_vertex_files_api
branch
from
June 11, 2026 06:15
ff6c165 to
bfa4997
Compare
18 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@Makefile`:
- Around line 1746-1747: The HELP text is inconsistent between VERTEX_GCS_BUCKET
and VERTEX_GCS_PREFIX; update the printf string for VERTEX_GCS_PREFIX to match
the sourcing annotation used for VERTEX_GCS_BUCKET (i.e., include
"(.env/Infisical)"), so both help lines consistently indicate "Env-sourced
(.env/Infisical)"; locate the two printf calls referencing VERTEX_GCS_BUCKET and
VERTEX_GCS_PREFIX and make the description text identical in format.
- Around line 1746-1747: Add forwarding for VERTEX_API_KEY to the Newman runs
and a help line: update the help block to include a printf for "VERTEX_API_KEY"
similar to the existing "VERTEX_GCS_BUCKET"/"VERTEX_GCS_PREFIX" entries, and
modify the run-provider-harness-test target's two newman run invocations (the
parallel and sequential invocations in the Makefile where
VERTEX_GCS_BUCKET/PREFIX are forwarded) to include $${VERTEX_API_KEY:+--env-var
"genaiKey=$$VERTEX_API_KEY"} \ so that the genaiKey env var in
provider-harness.json is set from VERTEX_API_KEY; place this new --env-var line
alongside the existing VERTEX_GCS_BUCKET / VERTEX_GCS_PREFIX --env-var blocks.
In `@tests/e2e/api/collections/provider-harness.json`:
- Around line 2152-2178: Add a "content_length" form field to the session-mint
formdata so the resumable smoke test exercises the
gcsParseSize/X-Upload-Content-Length path: in the formdata array (the same array
containing keys like "purpose", "filename", "content_type", "gcs_bucket",
"gcs_prefix") add an entry with key "content_length" and value set as a string
equal to the uploaded fixture size for "harness-video.bin" so the native
resumable flow is actually covered.
- Line 1933: The OpenAI drop-in Authorization header was removed from the
Vertex-backed file CRUD requests (the JSON entries where "header": [] for
endpoints under "/openai/v1/*"), which can cause 401/403 before provider=vertex
is considered; restore an Authorization header with "Bearer {{openaiKey}}" in
those request definitions (i.e., add an element like
{"name":"Authorization","value":"Bearer {{openaiKey}}"} to the header arrays for
the file CRUD requests and the other mentioned entries) so the harness uses the
Bifrost virtual key while still routing to provider=vertex.
In `@tests/integrations/python/tests/test_openai.py`:
- Around line 275-309: get_file_storage_config currently treats any non-"vertex"
provider as Bedrock/S3 which causes unrelated providers to be tied to S3 config
and skipped; change it so S3 config is returned only when provider explicitly
indicates Bedrock/S3 (e.g., provider == "bedrock" or "s3") and for all other
providers return None or an empty dict so callers won't add storage_config;
update callers that build extra_query/extra_body (the list/upload paths) to
include storage_config only when get_file_storage_config(...) returns a truthy
value (apply the same conditional include for extra_query in list calls).
🪄 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 Plus
Run ID: 25b68734-d885-42a4-9cd7-392029eb4195
📒 Files selected for processing (7)
Makefilecore/providers/vertex/vertex.gotests/e2e/api/collections/provider-harness.jsontests/integrations/python/config.ymltests/integrations/python/tests/test_openai.pytransports/bifrost-http/handlers/inference.gotransports/bifrost-http/integrations/openai.go
TejasGhatte
force-pushed
the
06-10-fix_tests_for_vertex_files_api
branch
from
June 11, 2026 07:42
bfa4997 to
9cadffb
Compare
18 tasks
Collaborator
Merge activity
|
Pratham-Mishra04
force-pushed
the
06-10-fix_tests_for_vertex_files_api
branch
2 times, most recently
from
June 11, 2026 07:48
30b1291 to
7ca617c
Compare
Pratham-Mishra04
force-pushed
the
06-10-fix_tests_for_vertex_files_api
branch
from
June 11, 2026 07:49
7ca617c to
f63ef86
Compare
akshaydeo
pushed a commit
that referenced
this pull request
Jun 12, 2026
## Summary Extends the Vertex provider's Files API to work with customer-owned GCS buckets via the OpenAI-compatible drop-in (`/openai/v1/files`) and the native resumable upload path. Previously, file operations (upload, list, retrieve, delete, content download) were only wired for Bedrock (S3) and Gemini. This PR adds the same CRUD surface for Vertex using GCS as the backing store, fixes a `content_length` type-coercion bug in the resumable upload path, and introduces opaque base64 encoding for `gs://` file IDs so they round-trip safely through URL path segments without requiring callers to percent-encode slashes. ## Changes - **`gs://` file ID encoding**: `gs://` URIs returned by Vertex contain slashes that break single-segment path routing on retrieve/delete/content endpoints. Upload, list, retrieve, and delete responses now base64-encode `gs://` IDs via `encodeStorageFileID`; incoming path parameters are decoded via `decodeStorageFileID` (which also falls back to percent-decoding for raw or percent-encoded URIs passed directly). - **OpenAI integration layer**: Extended the `Bedrock`-only base64 encode/decode branches in `CreateOpenAIFileRouteConfigs` and `extractFileIDFromPath` to also cover `Vertex`. Added GCS bracket-notation query/form parsing (`storage_config[gcs][bucket]`, `storage_config[gcs][prefix]`) in `extractFileListQueryParams` and `parseOpenAIFileUploadMultipartRequest`. - **`content_length` type coercion fix**: The resumable GCS upload session minter previously only accepted `float64` for `content_length` in `ExtraParams`. It now handles `int`, `int64`, and `string` (via `gcsParseSize`) so the `X-Upload-Content-Length` header is set correctly regardless of how the value arrives. - **Provider harness test collection**: Added a new folder `11b. Vertex GCS Files` with seven `[PREVIEW]`-tagged requests covering upload, list, retrieve, content download, delete (OpenAI drop-in), mint resumable session, PUT bytes directly to GCS, and resumable cleanup (native). Content-shape validation is skipped for `/files/.../content` and direct GCS storage URLs to avoid false positives. - **Python integration tests**: File tests (41–45) are refactored from Bedrock-only to provider-agnostic via a new `get_file_storage_config` helper that returns the appropriate `s3` or `gcs` storage config and skips when the backing bucket is not configured. Vertex file scenarios are enabled in `config.yml`. - **Makefile**: `VERTEX_GCS_BUCKET`, `VERTEX_GCS_PREFIX`, and `VERTEX_API_KEY` environment variables are forwarded to Newman as `vertexGcsBucket`, `vertexGcsPrefix`, and `vertexKey` in all three harness runner branches. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh # Unit / build go test ./... # Provider harness (requires a configured Vertex key + GCS bucket) VERTEX_API_KEY=<key> \ VERTEX_GCS_BUCKET=<bucket> \ VERTEX_GCS_PREFIX=bifrost-e2e/ \ make run-provider-harness-test FOLDER="11b. Vertex GCS Files" # Python integration tests cd tests/integrations/python pytest tests/test_openai.py -k "test_41 or test_42 or test_43 or test_44 or test_45" ``` Set the following environment variables to enable Vertex GCS file tests: | Variable | Description | |---|---| | `VERTEX_API_KEY` | Vertex AI API key (forwarded as `vertexKey` to Newman) | | `VERTEX_GCS_BUCKET` | GCS bucket name used for file storage | | `VERTEX_GCS_PREFIX` | Object prefix within the bucket (e.g. `bifrost-e2e/`) | ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations GCS bucket names and prefixes are passed as user-supplied form fields and query parameters. They are forwarded directly to the Vertex provider and used only to construct GCS object paths; no credentials are derived from them. The base64 encoding of `gs://` IDs is for URL-safety only and provides no confidentiality guarantee — callers should treat file IDs as opaque handles. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] 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-backed file management with GCS: upload (including resumable), list, retrieve, download, and delete via Files API. * **Bug Fixes** * Safer file ID handling for URLs by making Vertex/GCS IDs opaque and path-safe. * More robust handling of content-length for Vertex uploads. * **Tests** * Expanded e2e and integration tests covering Vertex GCS file flows and resumable uploads; shared test helpers for storage config. * **Chores** * Test harness help output documents Vertex GCS env vars and forwards them to test runs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 tasks
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
Extends the Vertex provider's Files API to work with customer-owned GCS buckets via the OpenAI-compatible drop-in (
/openai/v1/files) and the native resumable upload path. Previously, file operations (upload, list, retrieve, delete, content download) were only wired for Bedrock (S3) and Gemini. This PR adds the same CRUD surface for Vertex using GCS as the backing store, fixes acontent_lengthtype-coercion bug in the resumable upload path, and introduces opaque base64 encoding forgs://file IDs so they round-trip safely through URL path segments without requiring callers to percent-encode slashes.Changes
gs://file ID encoding:gs://URIs returned by Vertex contain slashes that break single-segment path routing on retrieve/delete/content endpoints. Upload, list, retrieve, and delete responses now base64-encodegs://IDs viaencodeStorageFileID; incoming path parameters are decoded viadecodeStorageFileID(which also falls back to percent-decoding for raw or percent-encoded URIs passed directly).Bedrock-only base64 encode/decode branches inCreateOpenAIFileRouteConfigsandextractFileIDFromPathto also coverVertex. Added GCS bracket-notation query/form parsing (storage_config[gcs][bucket],storage_config[gcs][prefix]) inextractFileListQueryParamsandparseOpenAIFileUploadMultipartRequest.content_lengthtype coercion fix: The resumable GCS upload session minter previously only acceptedfloat64forcontent_lengthinExtraParams. It now handlesint,int64, andstring(viagcsParseSize) so theX-Upload-Content-Lengthheader is set correctly regardless of how the value arrives.11b. Vertex GCS Fileswith seven[PREVIEW]-tagged requests covering upload, list, retrieve, content download, delete (OpenAI drop-in), mint resumable session, PUT bytes directly to GCS, and resumable cleanup (native). Content-shape validation is skipped for/files/.../contentand direct GCS storage URLs to avoid false positives.get_file_storage_confighelper that returns the appropriates3orgcsstorage config and skips when the backing bucket is not configured. Vertex file scenarios are enabled inconfig.yml.VERTEX_GCS_BUCKET,VERTEX_GCS_PREFIX, andVERTEX_API_KEYenvironment variables are forwarded to Newman asvertexGcsBucket,vertexGcsPrefix, andvertexKeyin all three harness runner branches.Type of change
Affected areas
How to test
Set the following environment variables to enable Vertex GCS file tests:
VERTEX_API_KEYvertexKeyto Newman)VERTEX_GCS_BUCKETVERTEX_GCS_PREFIXbifrost-e2e/)Breaking changes
Related issues
Security considerations
GCS bucket names and prefixes are passed as user-supplied form fields and query parameters. They are forwarded directly to the Vertex provider and used only to construct GCS object paths; no credentials are derived from them. The base64 encoding of
gs://IDs is for URL-safety only and provides no confidentiality guarantee — callers should treat file IDs as opaque handles.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores