Skip to content

fix: remove fallbacks key from gemini requests - #3464

Merged
akshaydeo merged 1 commit into
mainfrom
05-13-fix_remove_fallbacks_key_from_gemini_requests
May 13, 2026
Merged

fix: remove fallbacks key from gemini requests#3464
akshaydeo merged 1 commit into
mainfrom
05-13-fix_remove_fallbacks_key_from_gemini_requests

Conversation

@TejasGhatte

@TejasGhatte TejasGhatte commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Ensures the fallbacks field is stripped from request bodies sent to the Gemini provider, preventing unrecognized fields from being forwarded to the Gemini API. Also applies raw body normalization to the VideoGeneration endpoint and normalizes raw bodies in BatchCreate before further processing.

Changes

  • normalizeRawGenerateContentBody now unconditionally removes the fallbacks field from the JSON body, regardless of whether raw body mode is active.
  • VideoGeneration now calls normalizeRawGenerateContentBody before constructing the HTTP request, bringing it in line with other endpoints.
  • BatchCreate applies NormalizeRawGenerateContentRequestForCompatibility to raw request bodies immediately upon extraction, and explicitly deletes the fallbacks field from the final jsonData before sending.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Send a request to the Gemini VideoGeneration or BatchCreate endpoints with a fallbacks field included in the request body and verify it is not forwarded to the Gemini API.

go test ./...

Breaking changes

  • Yes
  • No

Related issues

Security considerations

None.

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

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved normalization of raw request bodies to strip Gemini-incompatible fields (including fallbacks) and ensure batch and video generation requests are sent in a compatible form, preventing downstream errors and improving reliability.
  • Tests
    • Added regression tests verifying incompatible fields are removed while preserving valid fields, and that empty or minimal inputs are handled safely.

Walkthrough

Normalize raw Gemini generate-content requests for compatibility and remove the JSON "fallbacks" field in the core normalizer, and apply that normalization in VideoGeneration and BatchCreate; includes a utils update, types additions, and a regression test.

Changes

Gemini request body normalization and fallback field removal

Layer / File(s) Summary
Core normalization function update
core/providers/gemini/gemini.go
normalizeRawGenerateContentBody conditionally applies compatibility normalization for raw-request contexts and attempts to remove the "fallbacks" JSON field from the request body.
VideoGeneration normalization
core/providers/gemini/gemini.go
VideoGeneration applies normalizeRawGenerateContentBody to request JSON before sending the :predictLongRunning request.
BatchCreate normalization and fallback removal
core/providers/gemini/gemini.go
BatchCreate normalizes provided raw request bodies using the compatibility transform and removes "fallbacks" from the marshaled batch payload before issuing the :batchGenerateContent request.
Utils: remove generationConfig.fallbacks
core/providers/gemini/utils.go
Adds fallbacks to the set of fields stripped by NormalizeRawGenerateContentRequestForCompatibility.
Types: add fallbacks fields
core/providers/gemini/types.go
Adds optional fallbacks JSON fields to GeminiEmbeddingRequest and GeminiVideoGenerationRequest.
Regression test for compatibility normalization
core/providers/gemini/gemini_test.go
Adds TestNormalizeRawGenerateContentRequestForCompatibility asserting fallbacks and incompatible generationConfig.* fields are removed and valid fields are preserved; tests empty/nil behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through JSON, light and quick,
Snipping "fallbacks" with a nimble flick,
Video and Batch now march in line,
Clean requests sent, neat and fine. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: removing the fallbacks key from Gemini requests. It directly relates to the main modifications across multiple files.
Description check ✅ Passed The description follows the template structure with all required sections completed, including Summary, Changes, Type of change, Affected areas, How to test, and Breaking changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-13-fix_remove_fallbacks_key_from_gemini_requests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tejas ghatte seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@TejasGhatte
TejasGhatte marked this pull request as ready for review May 13, 2026 12:16
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #3464

@greptile-apps

greptile-apps Bot commented May 13, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

The change is a targeted, low-risk fix with good test coverage; the core normalization logic is correct and the safety-net deletions are harmless no-ops when the field is already absent.

All three changed code paths behave correctly. The two observations about Embedding() and GeminiEmbeddingRequest.Fallbacks are improvements outside the PR's stated scope and do not affect the correctness of the changes made here.

The Embedding() function in gemini.go and the Fallbacks field tag on GeminiEmbeddingRequest in types.go are worth revisiting in a follow-up.

Important Files Changed

Filename Overview
core/providers/gemini/gemini.go Adds normalizeRawGenerateContentBody to VideoGeneration and a DeleteJSONField("fallbacks") safety net to BatchCreate; Embedding() is left without equivalent stripping
core/providers/gemini/utils.go Adds "fallbacks" to the list of fields stripped by NormalizeRawGenerateContentRequestForCompatibility; straightforward and correct
core/providers/gemini/types.go Adds Fallbacks []string json:"fallbacks,omitempty" to GeminiEmbeddingRequest and GeminiVideoGenerationRequest; the video type has a stripping safety net but the embedding type does not
core/providers/gemini/gemini_test.go Adds comprehensive table-driven tests for NormalizeRawGenerateContentRequestForCompatibility covering fallbacks stripping, compat-field stripping, and edge cases

Reviews (3): Last reviewed commit: "fix: remove fallbacks key from gemini re..." | Re-trigger Greptile

Comment thread core/providers/gemini/gemini.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 13, 2026
@TejasGhatte
TejasGhatte force-pushed the 05-13-fix_remove_fallbacks_key_from_gemini_requests branch from c7a3125 to e93bf8a Compare May 13, 2026 12:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
core/providers/gemini/gemini_test.go (1)

3163-3218: ⚡ Quick win

Add regression coverage for nested generationConfig.fallbacks.

Current cases verify top-level fallbacks stripping, but not generationConfig.fallbacks. Adding this case prevents path-specific regressions.

♻️ Proposed test case
@@
 		{
+			name:  "StripsGenerationConfigFallbacksField",
+			input: `{"contents":[{"parts":[{"text":"Hi"}]}],"generationConfig":{"temperature":0.7,"fallbacks":["openai/gpt-4o"]}}`,
+			validate: func(t *testing.T, m map[string]interface{}) {
+				gc := genConfig(m)
+				require.NotNil(t, gc)
+				assert.NotContains(t, gc, "fallbacks")
+				assert.Contains(t, gc, "temperature")
+			},
+		},
+		{
 			name:  "StripsFallbacksAlongsideCompatFields",
 			input: `{"contents":[{"parts":[{"text":"Hi"}]}],"fallbacks":["openai/gpt-4o"],"generationConfig":{"temperature":0.5,"presencePenalty":0.2}}`,
 			validate: func(t *testing.T, m map[string]interface{}) {
 				assert.NotContains(t, m, "fallbacks")
🤖 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/gemini/gemini_test.go` around lines 3163 - 3218, Add a new
table-driven test case in the tests slice that covers nested
generationConfig.fallbacks: construct input JSON where "generationConfig"
contains a "fallbacks" array (e.g.
`{"generationConfig":{"temperature":0.6,"fallbacks":["openai/gpt-4o"]},"contents":[{"parts":[{"text":"Hi"}]}]}`),
then in the validate func use genConfig(m) to get the nested map and assert that
the nested "fallbacks" key is not present while other valid fields like
"temperature" and top-level "contents" are preserved; place this alongside the
other cases (e.g. near "StripsFallbacksAlongsideCompatFields") so it runs with
the existing test harness.
🤖 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.

Nitpick comments:
In `@core/providers/gemini/gemini_test.go`:
- Around line 3163-3218: Add a new table-driven test case in the tests slice
that covers nested generationConfig.fallbacks: construct input JSON where
"generationConfig" contains a "fallbacks" array (e.g.
`{"generationConfig":{"temperature":0.6,"fallbacks":["openai/gpt-4o"]},"contents":[{"parts":[{"text":"Hi"}]}]}`),
then in the validate func use genConfig(m) to get the nested map and assert that
the nested "fallbacks" key is not present while other valid fields like
"temperature" and top-level "contents" are preserved; place this alongside the
other cases (e.g. near "StripsFallbacksAlongsideCompatFields") so it runs with
the existing test harness.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 85e1916c-2b9e-4ac4-9da3-2a9b6f0f0556

📥 Commits

Reviewing files that changed from the base of the PR and between c7a3125 and e93bf8a.

📒 Files selected for processing (3)
  • core/providers/gemini/gemini.go
  • core/providers/gemini/gemini_test.go
  • core/providers/gemini/utils.go
✅ Files skipped from review due to trivial changes (1)
  • core/providers/gemini/utils.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • core/providers/gemini/gemini.go

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 13, 2026
@TejasGhatte
TejasGhatte force-pushed the 05-13-fix_remove_fallbacks_key_from_gemini_requests branch from e93bf8a to 81b1f91 Compare May 13, 2026 12:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/gemini/types.go`:
- Line 1169: The Fallbacks field is being serialized to outbound Gemini DTOs;
make it non-serializable by removing it from outbound structs or changing its
struct tag to prevent JSON marshaling (e.g., update the Fallbacks field
declaration in core/providers/gemini/types.go to use json:"-" so it is omitted
from JSON output). Locate the Fallbacks field in the relevant DTO definition and
either delete the field from the outbound DTO or replace its tag with json:"-"
(and any other relevant tags like yaml:"-"/protobuf omission if used) so it
cannot be forwarded accidentally.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5c8992dc-95c1-4cb2-a0f5-6dee409a621d

📥 Commits

Reviewing files that changed from the base of the PR and between e93bf8a and 81b1f91.

📒 Files selected for processing (4)
  • core/providers/gemini/gemini.go
  • core/providers/gemini/gemini_test.go
  • core/providers/gemini/types.go
  • core/providers/gemini/utils.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • core/providers/gemini/utils.go
  • core/providers/gemini/gemini.go
  • core/providers/gemini/gemini_test.go

Comment thread core/providers/gemini/types.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants