Skip to content

Conversation

@m2m6vrm5fp-source
Copy link

Summary

Fixes 400 Bad Request errors when using tools with OpenCode/OhMyOpenCode and Gemini models.

Problems Fixed

1. Gemini rejects empty properties object

Tools with properties: {} (like todoread) cause 400 Bad Request because Gemini's strict protobuf validation rejects empty properties.

Fix: Add placeholder property when properties object is empty in toGeminiSchema().

2. Schema loss when tool has 'custom' but no 'function'

When a tool only has 'custom' format (no 'function'), the schema was being set on 'custom.input_schema' but then 'custom' was deleted, leaving the tool with no schema.

Fix: Create 'function' from 'custom' before 'custom' is deleted.

3. wrapToolsAsFunctionDeclarations bypasses toGeminiSchema

Schemas extracted in wrapToolsAsFunctionDeclarations were not being transformed through toGeminiSchema(), causing empty properties to slip through.

Fix: Call toGeminiSchema() on all extracted schemas.

Testing

Tested with oh-my-opencode using Gemini 3 Pro via Antigravity:

  • todoread tool (no parameters) now works
  • Custom-format tools now work
  • All existing tools continue to work

…ools

1. Add placeholder for empty OBJECT properties in toGeminiSchema()
   - Gemini rejects properties: {} with 400 Bad Request

2. Create function from custom before custom is deleted
   - Prevents schema loss when tool only has 'custom' format

3. Pass schemas through toGeminiSchema() in wrapToolsAsFunctionDeclarations()
   - Ensures all schemas get the empty properties fix
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026

Walkthrough

Changes to src/plugin/transform/gemini.ts involve reformatting public function signatures to use multiline return type declarations. The wrapToolsAsFunctionDeclarations function now explicitly declares its return type WrapToolsResult across multiple lines, and normalizeGeminiTools reformats its inline object return type to a multiline structure. Minor formatting adjustments including parameter alignment and line breaks were applied throughout. The functional logic and runtime behavior remain unchanged across these modifications.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately summarizes the main fixes: preventing Gemini 400 errors caused by empty properties and custom-only tools.
Description check ✅ Passed The description comprehensively explains the problems fixed, solutions implemented, and testing performed, all directly related to the changeset.

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


🧹 Recent nitpick comments
src/plugin/transform/gemini.ts (1)

306-315: Duplicated placeholder schema with inconsistent description.

This placeholder schema duplicates the one in toGeminiSchema (lines 145-151) but uses a different description text:

  • Here: "Placeholder. Always pass true."
  • In toGeminiSchema: "Placeholder parameter. Always pass true."

Consider extracting a shared constant or helper function to ensure consistency and reduce duplication.

♻️ Suggested refactor
+const PLACEHOLDER_SCHEMA: Record<string, unknown> = {
+  type: "OBJECT",
+  properties: {
+    _placeholder: {
+      type: "BOOLEAN",
+      description: "Placeholder parameter. Always pass true.",
+    },
+  },
+  required: ["_placeholder"],
+};
+
 export function toGeminiSchema(schema: unknown): unknown {
   // ... existing code ...
   if (
     result.type === "OBJECT" &&
     result.properties &&
     typeof result.properties === "object" &&
     Object.keys(result.properties as Record<string, unknown>).length === 0
   ) {
-    result.properties = {
-      _placeholder: {
-        type: "BOOLEAN",
-        description: "Placeholder parameter. Always pass true.",
-      },
-    };
-    result.required = ["_placeholder"];
+    result.properties = { ...PLACEHOLDER_SCHEMA.properties };
+    result.required = [...(PLACEHOLDER_SCHEMA.required as string[])];
   }

Then use PLACEHOLDER_SCHEMA in normalizeGeminiTools as well.

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 58c9b78 and 1dc07b1.

📒 Files selected for processing (1)
  • src/plugin/transform/gemini.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/plugin/transform/gemini.ts (3)
scripts/check-quota.mjs (1)
  • lower (95-95)
src/plugin/transform/index.ts (3)
  • normalizeGeminiTools (57-57)
  • RequestPayload (14-14)
  • buildGemini25ThinkingConfig (55-55)
src/plugin/transform/types.ts (1)
  • RequestPayload (66-66)
🔇 Additional comments (9)
src/plugin/transform/gemini.ts (9)

10-15: LGTM!

Clean import organization with proper type-only imports for the types used throughout the module.


160-186: LGTM!

Model detection functions are straightforward and correctly use case-insensitive string matching.


228-265: LGTM!

Image generation config with proper aspect ratio validation and sensible defaults.


371-396: Core fix for custom-only tools looks correct.

The logic now properly creates function from custom (lines 371-378) before deleting the custom wrapper (lines 393-396). This ensures the schema is preserved when a tool has only custom format, directly addressing the PR objective.


584-590: Good fix: schemas now processed through toGeminiSchema().

Both extraction paths now correctly pass schemas through toGeminiSchema():

  • Line 584-589: For functionDeclarations arrays
  • Line 625: For individual tool schemas

This addresses PR objective #3: "wrapToolsAsFunctionDeclarations bypassed toGeminiSchema()".

Also applies to: 625-626


527-545: LGTM!

Good abstraction for detecting web search tools across multiple provider formats (Gemini native, Claude/Anthropic, and name-based).


433-503: LGTM!

The transformation orchestration correctly applies operations in order: thinking config → Google Search → tool normalization → function declarations wrapping.


273-276: LGTM!

Multiline return type formatting improves readability without changing the type semantics.

Also applies to: 547-549


139-152: Placeholder injection correctly handles nested empty objects.

The concern about nested empty properties being missed is unfounded. The toGeminiSchema function recursively processes each property schema at line 95 by calling toGeminiSchema(propSchema), which means every nested object—including those with empty properties—passes through the complete function logic, including the placeholder check at lines 139-152. This check runs at the end of every recursive call, not just at the top level. Nested empty properties correctly receive the _placeholder injection.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

This PR fixes three related issues causing 400 Bad Request errors when using Gemini models with OpenCode/OhMyOpenCode.

Problems Fixed

1. Empty properties object rejection (lines 139-152)

  • Gemini's strict protobuf validation rejects schemas with properties: {}
  • Fix: Added _placeholder boolean property when properties object is empty
  • Applies to tools like todoread that have no parameters

2. Schema loss for custom-only tools (lines 371-378)

  • When a tool only has custom format (no function), the schema was set on custom.input_schema but then custom was deleted, leaving the tool without a schema
  • Fix: Create function from custom before custom is deleted

3. Schema transformation bypass in wrapper (lines 584-589, 625)

  • Schemas extracted in wrapToolsAsFunctionDeclarations() were not being transformed through toGeminiSchema(), allowing empty properties to slip through
  • Fix: Call toGeminiSchema() on all extracted schemas, including fallback schemas

Changes Summary

  • Modified toGeminiSchema() to detect empty properties objects and inject a placeholder property
  • Modified normalizeGeminiTools() to create function from custom before custom deletion
  • Modified wrapToolsAsFunctionDeclarations() to transform all schemas through toGeminiSchema()
  • Added formatting changes (prettier/lint fixes)

The fixes ensure all tool schemas are Gemini-compatible before being sent to the API, preventing 400 errors.

Confidence Score: 4/5

  • Safe to merge after verifying tests pass - the changes fix real Gemini API validation issues with well-targeted fixes
  • The fixes directly address the 400 errors described in the PR. The logic is sound and follows existing patterns in the codebase. Score is 4 (not 5) because test at gemini.test.ts:1087 may need updating to match new behavior, and the required array overwrite on line 151 could be refined
  • Check that existing tests still pass, particularly gemini.test.ts:1087 which expects empty properties to remain unchanged

Important Files Changed

Filename Overview
src/plugin/transform/gemini.ts Adds fixes for Gemini 400 errors: placeholder for empty properties, function creation from custom, and schema transformation in wrapper

Sequence Diagram

sequenceDiagram
    participant Client as OpenCode/OhMyOpenCode
    participant Transform as applyGeminiTransforms
    participant Normalize as normalizeGeminiTools
    participant Wrap as wrapToolsAsFunctionDeclarations
    participant Schema as toGeminiSchema
    participant Gemini as Gemini API

    Client->>Transform: Request with tools
    Transform->>Normalize: Normalize tool formats
    
    Note over Normalize: For each tool
    Normalize->>Schema: Transform schema (line 325)
    Schema->>Schema: Convert types to uppercase
    Schema->>Schema: Remove unsupported fields
    alt Empty properties detected
        Schema->>Schema: Add _placeholder property (lines 139-152)
    end
    Schema-->>Normalize: Transformed schema
    
    alt Tool has custom but no function
        Normalize->>Normalize: Create function from custom (lines 371-378)
    end
    
    Normalize->>Normalize: Delete custom wrapper (line 394)
    Normalize-->>Transform: Normalized tools
    
    Transform->>Wrap: Wrap in functionDeclarations
    
    Note over Wrap: For each tool
    Wrap->>Schema: Transform parameters (lines 584, 625)
    alt Empty properties in fallback
        Schema->>Schema: Add _placeholder to fallback schema
    end
    Schema-->>Wrap: Gemini-compatible parameters
    
    Wrap->>Wrap: Build functionDeclarations array
    Wrap-->>Transform: Wrapped tools
    
    Transform-->>Client: Transformed request
    Client->>Gemini: Send request with valid schemas
    Gemini-->>Client: Success (no 400 errors)
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

name,
description,
parameters: schema,
parameters: toGeminiSchema(schema) as Record<string, unknown>,
Copy link
Contributor

Choose a reason for hiding this comment

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

check that test at gemini.test.ts:1087 still passes - it expects { type: "OBJECT", properties: {} } to remain unchanged, but this will now add the _placeholder property via toGeminiSchema()

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/transform/gemini.ts
Line: 625:625

Comment:
check that test at `gemini.test.ts:1087` still passes - it expects `{ type: "OBJECT", properties: {} }` to remain unchanged, but this will now add the `_placeholder` property via `toGeminiSchema()`

How can I resolve this? If you propose a fix, please make it concise.

description: "Placeholder parameter. Always pass true.",
},
};
result.required = ["_placeholder"];
Copy link
Contributor

Choose a reason for hiding this comment

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

unconditionally overwrites any existing required array - though unlikely with empty properties, consider preserving existing required items if they exist

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/plugin/transform/gemini.ts
Line: 151:151

Comment:
unconditionally overwrites any existing `required` array - though unlikely with empty properties, consider preserving existing required items if they exist

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@NoeFabris NoeFabris changed the base branch from main to dev February 5, 2026 19:10
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.

1 participant