Open
Conversation
📝 WalkthroughWalkthroughThis pull request adds Google Gemini API provider support to the codebase, including OAuth authentication flow, message and tool conversion utilities, streaming adapter, token estimation integration, model configuration mappings, and environment variable management for the Gemini provider. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant queryModelGemini as queryModelGemini<br/>(index.ts)
participant ModelResolver as ModelResolver<br/>(modelMapping.ts)
participant MessageConverter as MessageConverter<br/>(convertMessages.ts)
participant ToolConverter as ToolConverter<br/>(convertTools.ts)
participant StreamClient as StreamClient<br/>(client.ts)
participant GeminiAPI as Gemini API
participant StreamAdapter as StreamAdapter<br/>(streamAdapter.ts)
Client->>queryModelGemini: messages, tools, options
queryModelGemini->>ModelResolver: resolve model name
ModelResolver-->>queryModelGemini: gemini model ID
queryModelGemini->>MessageConverter: convert to Gemini format
MessageConverter-->>queryModelGemini: GeminiGenerateContentRequest
queryModelGemini->>ToolConverter: convert tool definitions
ToolConverter-->>queryModelGemini: Gemini tool schemas
queryModelGemini->>StreamClient: stream request
StreamClient->>GeminiAPI: POST :streamGenerateContent
GeminiAPI-->>StreamClient: SSE stream
StreamClient-->>queryModelGemini: GeminiStreamChunk iterator
queryModelGemini->>StreamAdapter: adapt stream
StreamAdapter->>StreamAdapter: parse chunks & convert events
StreamAdapter-->>queryModelGemini: AsyncIterable of Anthropic events
queryModelGemini-->>Client: yield AssistantMessage & StreamEvents
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (11)
.codex/environments/environment.toml (1)
1-11: Consider keeping autogenerated workspace metadata out of this feature PR.Line 1 indicates this file is autogenerated and it looks environment-specific. Splitting it into a separate tooling/config PR (or excluding it) would keep this Gemini feature diff focused and easier to review.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/environments/environment.toml around lines 1 - 11, This PR includes an autogenerated workspace file (.codex/environments/environment.toml with the "version = 1" header and sections like [setup] and [[actions]]), which should be kept out of this feature change; remove this file from the feature branch (or revert the commit that added it) and put the autogenerated environment metadata in a separate tooling/config PR or add it to the repo-level ignore so it isn't included in feature PRs.src/services/tokenEstimation.ts (1)
146-176: Reuse the cached provider variable consistentlyLine 173 and Line 307 call
getAPIProvider()again even thoughprovideris already computed at Line 146 / Line 260. Reusingproviderkeeps behavior deterministic and avoids redundant reads.♻️ Proposed cleanup
- const filteredBetas = - getAPIProvider() === 'vertex' + const filteredBetas = + provider === 'vertex' ? betas.filter(b => VERTEX_COUNT_TOKENS_ALLOWED_BETAS.has(b)) : betas @@ - const filteredBetas = - getAPIProvider() === 'vertex' + const filteredBetas = + provider === 'vertex' ? betas.filter(b => VERTEX_COUNT_TOKENS_ALLOWED_BETAS.has(b)) : betasAlso applies to: 260-309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/tokenEstimation.ts` around lines 146 - 176, The code computes provider once via const provider = getAPIProvider() but later re-calls getAPIProvider() (e.g., in the filteredBetas ternary and the later block around lines 260-309); update those subsequent calls to reuse the cached provider variable to ensure deterministic behavior and avoid redundant reads—specifically replace getAPIProvider() usages in the filteredBetas assignment and the other branch near the same function with provider, keeping the existing logic in roughTokenCountEstimationForAPIRequest, countTokensWithBedrock, getAnthropicClient and VERTEX_COUNT_TOKENS_ALLOWED_BETAS unchanged.src/services/api/gemini/__tests__/convertMessages.test.ts (1)
8-22: Consider replacingany[]fixture types with concrete block unionsUsing
any[]in helpers hides invalid fixture shapes. A narrower union type would make these tests catch conversion-contract drift earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/__tests__/convertMessages.test.ts` around lines 8 - 22, The helpers makeUserMsg and makeAssistantMsg accept content: string | any[] which hides invalid fixture shapes; replace any[] with a concrete union of the expected block shapes (for example TextBlock | ImageBlock | MarkdownBlock or their array forms) and update the tests to construct content using those concrete block types so invalid shapes fail type-checking; ensure both makeUserMsg and makeAssistantMsg signatures and any fixtures use the new union type and that casts to UserMessage/AssistantMessage remain valid.src/services/api/gemini/__tests__/modelMapping.test.ts (1)
19-21: Make env restoration explicit per key
Object.assign(process.env, originalEnv)is brittle for cleanup. Prefer explicit restore/delete per env key for deterministic isolation.♻️ Suggested teardown pattern
afterEach(() => { - Object.assign(process.env, originalEnv) + for (const [key, value] of Object.entries(originalEnv)) { + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/__tests__/modelMapping.test.ts` around lines 19 - 21, The test teardown currently uses Object.assign(process.env, originalEnv) which is brittle; update the afterEach block to explicitly restore environment keys: iterate over Object.keys(originalEnv) and set process.env[key] to originalEnv[key] (handling undefined/null as removal), and then iterate over Object.keys(process.env) to delete any keys that are not present in originalEnv to ensure no test-added vars leak. Replace the Object.assign call inside afterEach with this deterministic per-key restore/delete logic, referencing the afterEach function, process.env, and originalEnv to locate where to change.src/services/api/gemini/__tests__/convertTools.test.ts (1)
22-22: Optional: avoidas anyin core conversion assertionsReplacing broad casts with typed fixtures would make these tests stricter and better at catching schema-shape regressions.
Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/__tests__/convertTools.test.ts` at line 22, The test uses a broad cast (tools as any) when calling anthropicToolsToGemini which weakens schema verification; replace the cast by defining a properly typed test fixture (e.g., an object typed to the expected source Tools type or an imported TestTools fixture) and use that typed variable in the expect calls (both at the first assertion and the second occurrence around line 70) so the compiler enforces the input shape for anthropicToolsToGemini and the assertions become stricter.src/services/api/gemini/client.ts (1)
4-7: Use the repo alias fortypes.js.This new module introduces a relative local import; please switch it to the
src/*alias like the rest of the TypeScript code.As per coding guidelines,
Use src/* path alias for imports in TypeScript files (e.g., import { ... } from 'src/utils/...').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/client.ts` around lines 4 - 7, The import in client.ts currently uses a relative path for the types module; replace the relative import of "./types.js" with the repo TypeScript alias path (e.g., import from "src/services/api/gemini/types") so GeminiGenerateContentRequest and GeminiStreamChunk are imported via the src/* alias consistent with project conventions; update the import statement that references types.js accordingly.src/services/api/claude.ts (1)
1340-1341: Use thesrc/*alias for the Gemini adapter import.The new dynamic import is relative; please switch it to the repository alias for consistency with the TypeScript import rule.
As per coding guidelines,
Use src/* path alias for imports in TypeScript files (e.g., import { ... } from 'src/utils/...').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/claude.ts` around lines 1340 - 1341, Replace the relative dynamic import for the Gemini adapter with the repository path alias: when checking getAPIProvider() === 'gemini' and importing queryModelGemini, change the import target from a relative path like './gemini/index.js' to the aliased module path 'src/services/api/gemini/index.js' (or the correct src/... alias that matches the gemini adapter location) so the dynamic import uses the src/* alias consistently with TypeScript import rules.src/services/api/gemini/convertTools.ts (1)
2-5: Use thesrc/*alias for Gemini type imports.The new local type import should use the repository alias instead of a relative path.
As per coding guidelines,
Use src/* path alias for imports in TypeScript files (e.g., import { ... } from 'src/utils/...').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/convertTools.ts` around lines 2 - 5, The import of Gemini types uses a relative path; update the import for GeminiFunctionCallingConfig and GeminiTool in convertTools.ts to use the repository alias (src/*) instead of './types.js'—replace the current relative import with an alias import that points to the same module (e.g., import type { GeminiFunctionCallingConfig, GeminiTool } from 'src/services/api/gemini/types') so other files follow the codebase convention.src/services/api/gemini/streamAdapter.ts (1)
3-3: Use thesrc/*alias for Gemini types.The new local type import should use the repository alias instead of a relative path.
As per coding guidelines,
Use src/* path alias for imports in TypeScript files (e.g., import { ... } from 'src/utils/...').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/streamAdapter.ts` at line 3, Replace the relative type import for GeminiPart and GeminiStreamChunk with the repository alias: change the import in streamAdapter.ts from "./types.js" to the src alias (e.g., import type { GeminiPart, GeminiStreamChunk } from 'src/services/api/gemini/types.js') so the file uses the prescribed src/* path alias for TypeScript imports.src/services/api/gemini/index.ts (1)
3-29: Switch these imports tosrc/*aliases.This new module still uses deep relative imports throughout. Please move them to the configured alias to match the rest of the TypeScript code.
As per coding guidelines,
Use src/* path alias for imports in TypeScript files (e.g., import { ... } from 'src/utils/...').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/index.ts` around lines 3 - 29, The file uses deep relative imports; update all imports to use the project path alias (src/*) instead of relative paths so tooling and style are consistent — e.g., replace imports that bring in getEmptyToolPermissionContext, toolToAPISchema, logForDebugging, createAssistantAPIErrorMessage, normalizeContentFromAPI, normalizeMessagesForAPI, SystemPrompt, ThinkingConfig, Options, streamGeminiGenerateContent, anthropicMessagesToGemini, anthropicToolChoiceToGemini, anthropicToolsToGemini, resolveGeminiModel, adaptGeminiStreamToAnthropic, and GEMINI_THOUGHT_SIGNATURE_FIELD to import from their corresponding src/... modules (for example import { ... } from 'src/utils/...' or 'src/services/...') ensuring paths match the configured tsconfig path aliases.src/services/api/gemini/convertMessages.ts (1)
5-13: Usesrc/*aliases in this new module.This file is new and still uses deep relative imports; please switch them to the configured repository alias.
As per coding guidelines,
Use src/* path alias for imports in TypeScript files (e.g., import { ... } from 'src/utils/...').🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/api/gemini/convertMessages.ts` around lines 5 - 13, The module uses deep relative imports; update all import specifiers to use the project's src/* TypeScript path aliases instead of relative paths—replace imports for AssistantMessage and UserMessage, safeParseJSON, SystemPrompt, and the Gemini types/constants (GEMINI_THOUGHT_SIGNATURE_FIELD, GeminiContent, GeminiGenerateContentRequest, GeminiPart) so they import from 'src/types/message', 'src/utils/json', 'src/utils/systemPromptType', and 'src/services/api/gemini/types' (or the canonical src path for those symbols) respectively, keeping the same named imports and types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ConsoleOAuthFlow.tsx`:
- Around line 564-574: When initializing the 'gemini_api' branch in
setOAuthStatus, preserve an existing single GEMINI_MODEL by using
process.env.GEMINI_MODEL as a fallback for haikuModel, sonnetModel and opusModel
(i.e. set each of haikuModel/sonnetModel/opusModel to their specific
ANTHROPIC_DEFAULT_* env var if present, otherwise to process.env.GEMINI_MODEL),
so an existing GEMINI_MODEL-only config is loaded and won't fail the new
Haiku/Sonnet/Opus validation; apply the same fallback logic to the other similar
section mentioned (the block around the later handling of gemini_api).
- Around line 1118-1127: The Enter handler for Gemini fields moves the input
value to the next field but never updates activeField, causing the UI to remain
focused on the old field while holding the next field's content; fix
handleGeminiEnter by, in the else branch (where next is computed), also calling
the setter to update the active field (e.g., setActiveField(next)) so the form
focus/state and geminiInputValue remain synchronized (also keep existing updates
to geminiInputCursorOffset and setOAuthStatus/buildGeminiState as-is).
In `@src/services/api/gemini/__tests__/convertMessages.test.ts`:
- Around line 2-6: Replace the relative imports with the TS path alias imports:
import the types AssistantMessage and UserMessage and the function
anthropicMessagesToGemini from the project alias (e.g. "src/...") instead of
'../../../../types/message.js' and '../convertMessages.js'; update the import
statements referencing AssistantMessage, UserMessage, and
anthropicMessagesToGemini to use the src/* path alias so they follow the
TypeScript alias rule.
In `@src/services/api/gemini/__tests__/convertTools.test.ts`:
- Around line 2-5: Update the test import to use the project path alias instead
of a relative path: replace the relative import of anthropicToolChoiceToGemini
and anthropicToolsToGemini from '../convertTools.js' with an alias import from
'src/services/api/gemini/convertTools' (matching the module that exports those
functions), so the test file imports anthropicToolChoiceToGemini and
anthropicToolsToGemini via the src/* alias convention.
In `@src/services/api/gemini/__tests__/modelMapping.test.ts`:
- Line 2: Replace the relative import with the repo's TypeScript path alias by
importing resolveGeminiModel from the aliased module entry instead of using a
relative path; update the import that currently references '../modelMapping.js'
to the corresponding 'src/...' aliased module that exports resolveGeminiModel so
the test uses the repo-required src/* alias for TypeScript imports.
In `@src/services/api/gemini/__tests__/streamAdapter.test.ts`:
- Around line 2-3: The test is using relative imports; replace them with
repo-standard src/* alias imports: update the import of
adaptGeminiStreamToAnthropic (from '../streamAdapter.js') to import from
'src/services/api/gemini/streamAdapter' and the GeminiStreamChunk type (from
'../types.js') to import from 'src/services/api/gemini/types' so the test file
src/.../streamAdapter.test.ts uses the src path alias rather than relative
paths.
In `@src/services/api/gemini/client.ts`:
- Around line 57-96: The stream reader currently only calls reader.releaseLock()
in the finally block, which can leave the underlying fetch body running if the
consumer stops early or a JSON parse throws; modify the logic to actively cancel
the underlying stream: call response.body.cancel() (or reader.cancel() if
available) whenever you break/return early or before re-throwing inside the
JSON.parse catch blocks, and also invoke response.body.cancel() in the finally
block before calling reader.releaseLock() (wrap cancel in a try/catch to ignore
cancel errors). Update the code around reader, response, parseSSEFrames and the
JSON.parse error handlers for GeminiStreamChunk to ensure the body is cancelled
on early exit or parse failures.
In `@src/services/api/gemini/convertMessages.ts`:
- Around line 92-116: The function convertUserContentBlockToGeminiParts (and the
other converter covering lines 119-173) currently returns an empty array for
unknown/unsupported block types, silently dropping content; change both
converters to fail fast by throwing a descriptive error when encountering an
unsupported block.type (include the block.type and any identifying info such as
block id or the raw block when available) instead of returning [], so
unsupported user blocks (e.g., image, document, tool_reference, structured
assistant blocks, or future types) surface as provider errors and preserve
conversation semantics; update error messages in
convertUserContentBlockToGeminiParts and the analogous converter to reference
the function name and block type to aid debugging.
In `@src/services/api/gemini/convertTools.ts`:
- Around line 235-239: The current filter in anthropicToolsToGemini (function
anthropicToolsToGemini) wrongly permits almost every non-'server' internal tool;
change it to only accept tools that are explicitly convertible by using a
whitelist of allowed types: replace the predicate with one that returns true
only if tool.type === 'custom' OR (tool.type is present AND
allowedTypes.has(tool.type)), where allowedTypes is a Set<string> listing the
known convertible types (e.g., include any externally convertible types your app
supports) and do not treat missing type ('type' not in tool) as allowed; update
allowedTypes when new convertible tool types are introduced.
In `@src/services/api/gemini/index.ts`:
- Around line 159-174: The handler currently emits assistant messages at
content_block_stop but ignores message_delta/message_stop so final usage and
stop_reason (and terminal signals like MAX_TOKENS/refusal) are never attached;
update the stream processing in the switch handling (cases 'content_block_stop',
'message_delta', 'message_stop') to accumulate and apply final metadata: when
receiving message_delta or message_stop, extract and store usage and stop_reason
(and translate refusal/MAX_TOKENS stop reasons into assistant API errors), then
attach these fields to the previously yielded AssistantMessage (or emit a final
assistant message if none was produced) so non-stream callers always get a
complete message with usage/stop_reason and proper error translation. Ensure you
reference the same message object created where uuid/randomUUID() is set so you
update or emit that instance with the final metadata.
- Around line 45-71: The provider fails to include options.extraToolSchemas when
building Gemini tool schemas: update the logic that computes toolSchemas and
subsequent conversion to pass both the runtime tools and
options.extraToolSchemas into toolToAPISchema/standardTools conversion so
extraToolSchemas are preserved; specifically, when calling toolToAPISchema (and
when forming standardTools and calling anthropicToolsToGemini), merge or concat
options.extraToolSchemas with tools before mapping, ensuring the same behavior
as the Anthropic path that appends extraToolSchemas, and keep usage of
toolSchemas, standardTools, anthropicMessagesToGemini, and
anthropicToolsToGemini consistent.
In `@src/services/api/gemini/streamAdapter.ts`:
- Around line 226-243: The mapGeminiFinishReason function currently collapses
multiple Gemini refusal/error reasons into a normal completion; update it so
SAFETY, RECITATION, BLOCKLIST, PROHIBITED_CONTENT, SPII, and
MALFORMED_FUNCTION_CALL are mapped to a distinct terminal code (e.g., 'refusal'
or 'failure') instead of falling through to the default; keep MAX_TOKENS
returning 'max_tokens', ensure STOP still resolves to the normal end-turn
mapping, and preserve the default sawToolUse ? 'tool_use' : 'end_turn' behavior
for unspecified reasons.
In `@src/utils/messages.ts`:
- Around line 2252-2259: The no-tool-search path is still dropping provider
metadata because stripCallerFieldFromAssistantMessage rebuilds tool_use blocks
from a fixed field list; update stripCallerFieldFromAssistantMessage so it only
removes the caller property (preserve all other keys like
_geminiThoughtSignature) by copying the block via spreading and deleting caller,
or alternatively gate the stricter reconstruction to non-Gemini providers;
target the stripCallerFieldFromAssistantMessage function and ensure ToolUseBlock
spreads/restores all fields except caller (preserving _geminiThoughtSignature
and other provider metadata).
In `@src/utils/model/__tests__/providers.test.ts`:
- Around line 5-11: The test is using relative imports instead of the repo's
src/* TypeScript path alias; update the mocked and imported module paths so they
use the alias. Replace the mock.module call target
("../../settings/settings.js") with the corresponding alias import (e.g.,
"src/settings/settings") and change the dynamic import that yields
getAPIProvider and isFirstPartyAnthropicBaseUrl (currently "../providers") to
the alias path (e.g., "src/utils/model/providers"), ensuring the symbols
getAPIProvider and isFirstPartyAnthropicBaseUrl still resolve correctly.
In `@src/utils/settings/__tests__/config.test.ts`:
- Around line 478-483: The test suite name "gemini settings" should follow the
function/schema naming convention; rename the describe block to reference the
SettingsSchema function (e.g., describe("SettingsSchema", ...) ) so it matches
other tests, and keep the inner test title as a behavior description ("accepts
gemini modelType") and the existing assertion that SettingsSchema().safeParse({
modelType: "gemini" }) succeeds.
---
Nitpick comments:
In @.codex/environments/environment.toml:
- Around line 1-11: This PR includes an autogenerated workspace file
(.codex/environments/environment.toml with the "version = 1" header and sections
like [setup] and [[actions]]), which should be kept out of this feature change;
remove this file from the feature branch (or revert the commit that added it)
and put the autogenerated environment metadata in a separate tooling/config PR
or add it to the repo-level ignore so it isn't included in feature PRs.
In `@src/services/api/claude.ts`:
- Around line 1340-1341: Replace the relative dynamic import for the Gemini
adapter with the repository path alias: when checking getAPIProvider() ===
'gemini' and importing queryModelGemini, change the import target from a
relative path like './gemini/index.js' to the aliased module path
'src/services/api/gemini/index.js' (or the correct src/... alias that matches
the gemini adapter location) so the dynamic import uses the src/* alias
consistently with TypeScript import rules.
In `@src/services/api/gemini/__tests__/convertMessages.test.ts`:
- Around line 8-22: The helpers makeUserMsg and makeAssistantMsg accept content:
string | any[] which hides invalid fixture shapes; replace any[] with a concrete
union of the expected block shapes (for example TextBlock | ImageBlock |
MarkdownBlock or their array forms) and update the tests to construct content
using those concrete block types so invalid shapes fail type-checking; ensure
both makeUserMsg and makeAssistantMsg signatures and any fixtures use the new
union type and that casts to UserMessage/AssistantMessage remain valid.
In `@src/services/api/gemini/__tests__/convertTools.test.ts`:
- Line 22: The test uses a broad cast (tools as any) when calling
anthropicToolsToGemini which weakens schema verification; replace the cast by
defining a properly typed test fixture (e.g., an object typed to the expected
source Tools type or an imported TestTools fixture) and use that typed variable
in the expect calls (both at the first assertion and the second occurrence
around line 70) so the compiler enforces the input shape for
anthropicToolsToGemini and the assertions become stricter.
In `@src/services/api/gemini/__tests__/modelMapping.test.ts`:
- Around line 19-21: The test teardown currently uses Object.assign(process.env,
originalEnv) which is brittle; update the afterEach block to explicitly restore
environment keys: iterate over Object.keys(originalEnv) and set process.env[key]
to originalEnv[key] (handling undefined/null as removal), and then iterate over
Object.keys(process.env) to delete any keys that are not present in originalEnv
to ensure no test-added vars leak. Replace the Object.assign call inside
afterEach with this deterministic per-key restore/delete logic, referencing the
afterEach function, process.env, and originalEnv to locate where to change.
In `@src/services/api/gemini/client.ts`:
- Around line 4-7: The import in client.ts currently uses a relative path for
the types module; replace the relative import of "./types.js" with the repo
TypeScript alias path (e.g., import from "src/services/api/gemini/types") so
GeminiGenerateContentRequest and GeminiStreamChunk are imported via the src/*
alias consistent with project conventions; update the import statement that
references types.js accordingly.
In `@src/services/api/gemini/convertMessages.ts`:
- Around line 5-13: The module uses deep relative imports; update all import
specifiers to use the project's src/* TypeScript path aliases instead of
relative paths—replace imports for AssistantMessage and UserMessage,
safeParseJSON, SystemPrompt, and the Gemini types/constants
(GEMINI_THOUGHT_SIGNATURE_FIELD, GeminiContent, GeminiGenerateContentRequest,
GeminiPart) so they import from 'src/types/message', 'src/utils/json',
'src/utils/systemPromptType', and 'src/services/api/gemini/types' (or the
canonical src path for those symbols) respectively, keeping the same named
imports and types.
In `@src/services/api/gemini/convertTools.ts`:
- Around line 2-5: The import of Gemini types uses a relative path; update the
import for GeminiFunctionCallingConfig and GeminiTool in convertTools.ts to use
the repository alias (src/*) instead of './types.js'—replace the current
relative import with an alias import that points to the same module (e.g.,
import type { GeminiFunctionCallingConfig, GeminiTool } from
'src/services/api/gemini/types') so other files follow the codebase convention.
In `@src/services/api/gemini/index.ts`:
- Around line 3-29: The file uses deep relative imports; update all imports to
use the project path alias (src/*) instead of relative paths so tooling and
style are consistent — e.g., replace imports that bring in
getEmptyToolPermissionContext, toolToAPISchema, logForDebugging,
createAssistantAPIErrorMessage, normalizeContentFromAPI,
normalizeMessagesForAPI, SystemPrompt, ThinkingConfig, Options,
streamGeminiGenerateContent, anthropicMessagesToGemini,
anthropicToolChoiceToGemini, anthropicToolsToGemini, resolveGeminiModel,
adaptGeminiStreamToAnthropic, and GEMINI_THOUGHT_SIGNATURE_FIELD to import from
their corresponding src/... modules (for example import { ... } from
'src/utils/...' or 'src/services/...') ensuring paths match the configured
tsconfig path aliases.
In `@src/services/api/gemini/streamAdapter.ts`:
- Line 3: Replace the relative type import for GeminiPart and GeminiStreamChunk
with the repository alias: change the import in streamAdapter.ts from
"./types.js" to the src alias (e.g., import type { GeminiPart, GeminiStreamChunk
} from 'src/services/api/gemini/types.js') so the file uses the prescribed src/*
path alias for TypeScript imports.
In `@src/services/tokenEstimation.ts`:
- Around line 146-176: The code computes provider once via const provider =
getAPIProvider() but later re-calls getAPIProvider() (e.g., in the filteredBetas
ternary and the later block around lines 260-309); update those subsequent calls
to reuse the cached provider variable to ensure deterministic behavior and avoid
redundant reads—specifically replace getAPIProvider() usages in the
filteredBetas assignment and the other branch near the same function with
provider, keeping the existing logic in roughTokenCountEstimationForAPIRequest,
countTokensWithBedrock, getAnthropicClient and VERTEX_COUNT_TOKENS_ALLOWED_BETAS
unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba7275e6-7d5b-4274-b676-28bbdf7336f2
📒 Files selected for processing (28)
.codex/environments/environment.tomlAGENTS.mdfixtures/token-count-7ef7321f9e68.jsonfixtures/token-count-fafe44226655.jsonsrc/components/ConsoleOAuthFlow.tsxsrc/services/api/claude.tssrc/services/api/gemini/__tests__/convertMessages.test.tssrc/services/api/gemini/__tests__/convertTools.test.tssrc/services/api/gemini/__tests__/modelMapping.test.tssrc/services/api/gemini/__tests__/streamAdapter.test.tssrc/services/api/gemini/client.tssrc/services/api/gemini/convertMessages.tssrc/services/api/gemini/convertTools.tssrc/services/api/gemini/index.tssrc/services/api/gemini/modelMapping.tssrc/services/api/gemini/streamAdapter.tssrc/services/api/gemini/types.tssrc/services/tokenEstimation.tssrc/utils/__tests__/messages.test.tssrc/utils/auth.tssrc/utils/managedEnvConstants.tssrc/utils/messages.tssrc/utils/model/__tests__/providers.test.tssrc/utils/model/configs.tssrc/utils/model/providers.tssrc/utils/settings/__tests__/config.test.tssrc/utils/settings/types.tssrc/utils/status.tsx
Comment on lines
+564
to
+574
| } else if (value === 'gemini_api') { | ||
| logEvent('tengu_gemini_api_selected', {}) | ||
| setOAuthStatus({ | ||
| state: 'gemini_api', | ||
| baseUrl: process.env.GEMINI_BASE_URL ?? '', | ||
| apiKey: process.env.GEMINI_API_KEY ?? '', | ||
| haikuModel: process.env.ANTHROPIC_DEFAULT_HAIKU_MODEL ?? '', | ||
| sonnetModel: process.env.ANTHROPIC_DEFAULT_SONNET_MODEL ?? '', | ||
| opusModel: process.env.ANTHROPIC_DEFAULT_OPUS_MODEL ?? '', | ||
| activeField: 'base_url', | ||
| }) |
There was a problem hiding this comment.
Preserve the valid GEMINI_MODEL config path.
The Gemini provider already accepts a single GEMINI_MODEL, but this form neither loads that env var nor saves it. An existing GEMINI_MODEL-only setup will show up here with blank model fields and then fail the new Haiku/Sonnet/Opus validation unless the user rewrites their config into three separate defaults.
Also applies to: 1068-1096
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ConsoleOAuthFlow.tsx` around lines 564 - 574, When
initializing the 'gemini_api' branch in setOAuthStatus, preserve an existing
single GEMINI_MODEL by using process.env.GEMINI_MODEL as a fallback for
haikuModel, sonnetModel and opusModel (i.e. set each of
haikuModel/sonnetModel/opusModel to their specific ANTHROPIC_DEFAULT_* env var
if present, otherwise to process.env.GEMINI_MODEL), so an existing
GEMINI_MODEL-only config is loaded and won't fail the new Haiku/Sonnet/Opus
validation; apply the same fallback logic to the other similar section mentioned
(the block around the later handling of gemini_api).
Comment on lines
+1118
to
+1127
| const handleGeminiEnter = useCallback(() => { | ||
| const idx = GEMINI_FIELDS.indexOf(activeField) | ||
| setOAuthStatus(buildGeminiState(activeField, geminiInputValue)) | ||
| if (idx === GEMINI_FIELDS.length - 1) { | ||
| doGeminiSave() | ||
| } else { | ||
| const next = GEMINI_FIELDS[idx + 1]! | ||
| setGeminiInputValue(geminiDisplayValues[next] ?? '') | ||
| setGeminiInputCursorOffset((geminiDisplayValues[next] ?? '').length) | ||
| } |
There was a problem hiding this comment.
Enter before the last field can desynchronize the form state.
The else branch swaps geminiInputValue to the next field's value without moving activeField. After that, the UI is still editing the old field while holding the next field's content, so the next Tab/save can persist the wrong value under baseUrl, apiKey, or one of the model fields.
🛠️ Suggested fix
const handleGeminiEnter = useCallback(() => {
const idx = GEMINI_FIELDS.indexOf(activeField)
- setOAuthStatus(buildGeminiState(activeField, geminiInputValue))
if (idx === GEMINI_FIELDS.length - 1) {
+ setOAuthStatus(buildGeminiState(activeField, geminiInputValue))
doGeminiSave()
} else {
const next = GEMINI_FIELDS[idx + 1]!
+ setOAuthStatus(
+ buildGeminiState(activeField, geminiInputValue, next),
+ )
setGeminiInputValue(geminiDisplayValues[next] ?? '')
setGeminiInputCursorOffset((geminiDisplayValues[next] ?? '').length)
}
}, [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ConsoleOAuthFlow.tsx` around lines 1118 - 1127, The Enter
handler for Gemini fields moves the input value to the next field but never
updates activeField, causing the UI to remain focused on the old field while
holding the next field's content; fix handleGeminiEnter by, in the else branch
(where next is computed), also calling the setter to update the active field
(e.g., setActiveField(next)) so the form focus/state and geminiInputValue remain
synchronized (also keep existing updates to geminiInputCursorOffset and
setOAuthStatus/buildGeminiState as-is).
Comment on lines
+643
to
+658
| content: message.message.content.map((_, i) => { | ||
| const contentBlock = stripGeminiProviderMetadata(_) | ||
| return { | ||
| ...contentBlock, | ||
| ...(i === message.message.content.length - 1 && | ||
| contentBlock.type !== 'thinking' && | ||
| contentBlock.type !== 'redacted_thinking' && | ||
| (feature('CONNECTOR_TEXT') | ||
| ? !isConnectorTextBlock(contentBlock) | ||
| : true) | ||
| ? enablePromptCaching | ||
| ? { cache_control: getCacheControl({ querySource }) } | ||
| : {} | ||
| : {}), | ||
| } | ||
| }), |
There was a problem hiding this comment.
Use the last cacheable block, not just the last array element.
This still only applies cache_control when i === message.message.content.length - 1. If the tail block is thinking, redacted_thinking, or connector text, no block gets marked at all and prompt caching silently drops out for that assistant message.
Comment on lines
+2
to
+6
| import type { | ||
| AssistantMessage, | ||
| UserMessage, | ||
| } from '../../../../types/message.js' | ||
| import { anthropicMessagesToGemini } from '../convertMessages.js' |
There was a problem hiding this comment.
Switch imports to src/* aliases
This test currently uses relative imports. Please align with the TS import alias rule.
🔧 Suggested import updates
import type {
AssistantMessage,
UserMessage,
-} from '../../../../types/message.js'
-import { anthropicMessagesToGemini } from '../convertMessages.js'
+} from 'src/types/message.js'
+import { anthropicMessagesToGemini } from 'src/services/api/gemini/convertMessages.js'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { | |
| AssistantMessage, | |
| UserMessage, | |
| } from '../../../../types/message.js' | |
| import { anthropicMessagesToGemini } from '../convertMessages.js' | |
| import type { | |
| AssistantMessage, | |
| UserMessage, | |
| } from 'src/types/message.js' | |
| import { anthropicMessagesToGemini } from 'src/services/api/gemini/convertMessages.js' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/api/gemini/__tests__/convertMessages.test.ts` around lines 2 -
6, Replace the relative imports with the TS path alias imports: import the types
AssistantMessage and UserMessage and the function anthropicMessagesToGemini from
the project alias (e.g. "src/...") instead of '../../../../types/message.js' and
'../convertMessages.js'; update the import statements referencing
AssistantMessage, UserMessage, and anthropicMessagesToGemini to use the src/*
path alias so they follow the TypeScript alias rule.
Comment on lines
+2
to
+5
| import { | ||
| anthropicToolChoiceToGemini, | ||
| anthropicToolsToGemini, | ||
| } from '../convertTools.js' |
There was a problem hiding this comment.
Use src/* alias import for converter module
The test currently imports via relative path; this should use the repository alias convention.
🔧 Suggested import update
import {
anthropicToolChoiceToGemini,
anthropicToolsToGemini,
-} from '../convertTools.js'
+} from 'src/services/api/gemini/convertTools.js'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/api/gemini/__tests__/convertTools.test.ts` around lines 2 - 5,
Update the test import to use the project path alias instead of a relative path:
replace the relative import of anthropicToolChoiceToGemini and
anthropicToolsToGemini from '../convertTools.js' with an alias import from
'src/services/api/gemini/convertTools' (matching the module that exports those
functions), so the test file imports anthropicToolChoiceToGemini and
anthropicToolsToGemini via the src/* alias convention.
Comment on lines
+159
to
+174
| const message: AssistantMessage = { | ||
| message: { | ||
| ...partialMessage, | ||
| content: normalizeContentFromAPI([block], tools, options.agentId), | ||
| }, | ||
| requestId: undefined, | ||
| type: 'assistant', | ||
| uuid: randomUUID(), | ||
| timestamp: new Date().toISOString(), | ||
| } | ||
| yield message | ||
| break | ||
| } | ||
| case 'message_delta': | ||
| case 'message_stop': | ||
| break |
There was a problem hiding this comment.
Final usage and stop reason are dropped here.
Assistant messages are emitted at content_block_stop, but message_delta is ignored, so the yielded message never receives its final usage or stop_reason. That also means MAX_TOKENS / refusal terminations are never translated into assistant API errors, and a terminal Gemini chunk with no content block can still leave non-streaming callers with no assistant message at all.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/api/gemini/index.ts` around lines 159 - 174, The handler
currently emits assistant messages at content_block_stop but ignores
message_delta/message_stop so final usage and stop_reason (and terminal signals
like MAX_TOKENS/refusal) are never attached; update the stream processing in the
switch handling (cases 'content_block_stop', 'message_delta', 'message_stop') to
accumulate and apply final metadata: when receiving message_delta or
message_stop, extract and store usage and stop_reason (and translate
refusal/MAX_TOKENS stop reasons into assistant API errors), then attach these
fields to the previously yielded AssistantMessage (or emit a final assistant
message if none was produced) so non-stream callers always get a complete
message with usage/stop_reason and proper error translation. Ensure you
reference the same message object created where uuid/randomUUID() is set so you
update or emit that instance with the final metadata.
Comment on lines
+226
to
+243
| function mapGeminiFinishReason( | ||
| reason: string | undefined, | ||
| sawToolUse: boolean, | ||
| ): string { | ||
| switch (reason) { | ||
| case 'MAX_TOKENS': | ||
| return 'max_tokens' | ||
| case 'STOP': | ||
| case 'FINISH_REASON_UNSPECIFIED': | ||
| case 'SAFETY': | ||
| case 'RECITATION': | ||
| case 'BLOCKLIST': | ||
| case 'PROHIBITED_CONTENT': | ||
| case 'SPII': | ||
| case 'MALFORMED_FUNCTION_CALL': | ||
| default: | ||
| return sawToolUse ? 'tool_use' : 'end_turn' | ||
| } |
There was a problem hiding this comment.
Don't map Gemini refusal/error finishes to a normal stop.
SAFETY, RECITATION, BLOCKLIST, PROHIBITED_CONTENT, SPII, and MALFORMED_FUNCTION_CALL are terminal failure/refusal states. Converting all of them to end_turn/tool_use makes the Gemini path indistinguishable from a clean completion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/api/gemini/streamAdapter.ts` around lines 226 - 243, The
mapGeminiFinishReason function currently collapses multiple Gemini refusal/error
reasons into a normal completion; update it so SAFETY, RECITATION, BLOCKLIST,
PROHIBITED_CONTENT, SPII, and MALFORMED_FUNCTION_CALL are mapped to a distinct
terminal code (e.g., 'refusal' or 'failure') instead of falling through to the
default; keep MAX_TOKENS returning 'max_tokens', ensure STOP still resolves to
the normal end-turn mapping, and preserve the default sawToolUse ? 'tool_use' :
'end_turn' behavior for unspecified reasons.
Comment on lines
+2252
to
2259
| // When tool search is NOT enabled, strip tool-search-only fields | ||
| // like 'caller', but preserve other provider metadata attached to | ||
| // the block (for example Gemini thought signatures on tool_use). | ||
| const { caller: _caller, ...toolUseRest } = block as ToolUseBlock & | ||
| Record<string, unknown> & { caller?: unknown } | ||
| return { | ||
| ...toolUseRest, | ||
| type: 'tool_use' as const, |
There was a problem hiding this comment.
Provider metadata is still dropped later in the no-tool-search path.
Line 2252-Line 2259 preserves _geminiThoughtSignature, but in the no-tool-search pipeline src/services/api/claude.ts still routes assistant messages through stripCallerFieldFromAssistantMessage, which rebuilds tool_use blocks with only { type, id, name, input } and removes that metadata again. This means the new behavior is not preserved end-to-end.
[suggested fix] Make stripCallerFieldFromAssistantMessage strip only caller (not reconstruct from a fixed field list), or gate that stricter stripping to non-Gemini providers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/messages.ts` around lines 2252 - 2259, The no-tool-search path is
still dropping provider metadata because stripCallerFieldFromAssistantMessage
rebuilds tool_use blocks from a fixed field list; update
stripCallerFieldFromAssistantMessage so it only removes the caller property
(preserve all other keys like _geminiThoughtSignature) by copying the block via
spreading and deleting caller, or alternatively gate the stricter reconstruction
to non-Gemini providers; target the stripCallerFieldFromAssistantMessage
function and ensure ToolUseBlock spreads/restores all fields except caller
(preserving _geminiThoughtSignature and other provider metadata).
Comment on lines
+5
to
+11
| mock.module("../../settings/settings.js", () => ({ | ||
| getInitialSettings: () => | ||
| mockedModelType ? { modelType: mockedModelType } : {}, | ||
| })); | ||
|
|
||
| const { getAPIProvider, isFirstPartyAnthropicBaseUrl } = | ||
| await import("../providers"); |
There was a problem hiding this comment.
Use src/* aliases for mocked/imported modules
Lines 5-11 use relative paths in a TS test file; switch them to src/* aliases to match repo import conventions.
🔧 Suggested import path updates
-mock.module("../../settings/settings.js", () => ({
+mock.module("src/utils/settings/settings.js", () => ({
@@
-const { getAPIProvider, isFirstPartyAnthropicBaseUrl } =
- await import("../providers");
+const { getAPIProvider, isFirstPartyAnthropicBaseUrl } =
+ await import("src/utils/model/providers.js");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/model/__tests__/providers.test.ts` around lines 5 - 11, The test is
using relative imports instead of the repo's src/* TypeScript path alias; update
the mocked and imported module paths so they use the alias. Replace the
mock.module call target ("../../settings/settings.js") with the corresponding
alias import (e.g., "src/settings/settings") and change the dynamic import that
yields getAPIProvider and isFirstPartyAnthropicBaseUrl (currently
"../providers") to the alias path (e.g., "src/utils/model/providers"), ensuring
the symbols getAPIProvider and isFirstPartyAnthropicBaseUrl still resolve
correctly.
Comment on lines
+478
to
+483
| describe("gemini settings", () => { | ||
| test("accepts gemini modelType", () => { | ||
| const result = SettingsSchema().safeParse({ modelType: "gemini" }); | ||
| expect(result.success).toBe(true); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Align describe name with function/schema naming convention.
Please rename this suite to a function/schema-style name (e.g., SettingsSchema) to match test naming conventions used across test files.
♻️ Suggested adjustment
-describe("gemini settings", () => {
+describe("SettingsSchema", () => {
test("accepts gemini modelType", () => {
const result = SettingsSchema().safeParse({ modelType: "gemini" });
expect(result.success).toBe(true);
});
});As per coding guidelines: Use describe('functionName') and test('behavior description') for test naming, with descriptions in English.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| describe("gemini settings", () => { | |
| test("accepts gemini modelType", () => { | |
| const result = SettingsSchema().safeParse({ modelType: "gemini" }); | |
| expect(result.success).toBe(true); | |
| }); | |
| }); | |
| describe("SettingsSchema", () => { | |
| test("accepts gemini modelType", () => { | |
| const result = SettingsSchema().safeParse({ modelType: "gemini" }); | |
| expect(result.success).toBe(true); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/settings/__tests__/config.test.ts` around lines 478 - 483, The test
suite name "gemini settings" should follow the function/schema naming
convention; rename the describe block to reference the SettingsSchema function
(e.g., describe("SettingsSchema", ...) ) so it matches other tests, and keep the
inner test title as a behavior description ("accepts gemini modelType") and the
existing assertion that SettingsSchema().safeParse({ modelType: "gemini" })
succeeds.
Owner
|
麻烦清理一下不相关的代码哈,比如奇奇怪怪的 .codex 文件夹, json 和 AGENTS.md 文件 |
Author
|
清理完了，麻烦再审一下 |
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
2 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 by CodeRabbit
Release Notes