Skip to content

Conversation

@miguelg719
Copy link
Collaborator

@miguelg719 miguelg719 commented Oct 25, 2025

… defined on .env

Logging needs fixing for informed errors, not all logs are being properly printing. Addressing on another PR (preceding this one)

@changeset-bot
Copy link

changeset-bot bot commented Oct 25, 2025

⚠️ No Changeset found

Latest commit: a9cb986

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@miguelg719 miguelg719 marked this pull request as ready for review October 26, 2025 14:54
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.

Greptile Overview

Greptile Summary

This PR adds support for auto-loading the GEMINI_API_KEY environment variable as an alternative to GOOGLE_GENERATIVE_AI_API_KEY for Google/Gemini models. The implementation consistently prioritizes GEMINI_API_KEY over GOOGLE_GENERATIVE_AI_API_KEY across all initialization paths.

Key Changes

  • Enhanced providerEnvVarMap in utils.ts to support multiple env vars per provider using an array format
  • Updated loadApiKeyFromEnv to handle both single string and array of env var names with priority-based fallback
  • Applied consistent API key loading order (GEMINI_API_KEYGOOGLE_GENERATIVE_AI_API_KEY) in GoogleCUAClient, V3Evaluator, and eval initialization
  • Fixed incorrect env var reference (GOOGLE_API_KEYGEMINI_API_KEY) in initV3.ts

Notes

  • The try-catch block added in v3.ts:207-219 wraps a function that never throws, making it dead code

Confidence Score: 4/5

  • This PR is safe to merge with minor cleanup recommended
  • The implementation correctly adds support for both GEMINI_API_KEY and GOOGLE_GENERATIVE_AI_API_KEY with consistent fallback logic. The only issue is unnecessary try-catch block that wraps a function which never throws errors, making it dead code that should be removed for clarity.
  • packages/core/lib/v3/v3.ts contains unnecessary error handling that should be simplified

Important Files Changed

File Analysis

Filename Score Overview
packages/core/lib/utils.ts 5/5 Added support for multiple env vars for Google provider, allowing both GEMINI_API_KEY and GOOGLE_GENERATIVE_AI_API_KEY
packages/core/lib/v3/agent/GoogleCUAClient.ts 5/5 Added fallback to GOOGLE_GENERATIVE_AI_API_KEY when GEMINI_API_KEY is not set
packages/core/lib/v3/v3.ts 4/5 Added try-catch around loadApiKeyFromEnv, but the function never throws errors - try-catch may be unnecessary

Sequence Diagram

sequenceDiagram
    participant User
    participant V3
    participant Utils
    participant Env as Environment Variables
    participant GoogleCUAClient
    participant V3Evaluator
    
    Note over V3,Env: API Key Loading Flow
    
    User->>V3: Initialize with model name
    V3->>V3: Extract provider from model name
    V3->>Utils: loadApiKeyFromEnv("google")
    Utils->>Utils: Check providerEnvVarMap["google"]
    Note over Utils: Returns array: ["GEMINI_API_KEY", "GOOGLE_GENERATIVE_AI_API_KEY"]
    Utils->>Env: Check GEMINI_API_KEY
    alt GEMINI_API_KEY exists
        Env-->>Utils: Return GEMINI_API_KEY value
        Utils-->>V3: Return API key
    else GEMINI_API_KEY not found
        Utils->>Env: Check GOOGLE_GENERATIVE_AI_API_KEY
        alt GOOGLE_GENERATIVE_AI_API_KEY exists
            Env-->>Utils: Return GOOGLE_GENERATIVE_AI_API_KEY value
            Utils-->>V3: Return API key
        else Neither found
            Utils-->>V3: Return undefined
        end
    end
    
    Note over GoogleCUAClient,Env: Direct API Key Loading
    User->>GoogleCUAClient: Initialize with clientOptions
    GoogleCUAClient->>Env: Check GEMINI_API_KEY
    alt GEMINI_API_KEY exists
        Env-->>GoogleCUAClient: Use GEMINI_API_KEY
    else Check fallback
        GoogleCUAClient->>Env: Check GOOGLE_GENERATIVE_AI_API_KEY
        Env-->>GoogleCUAClient: Use GOOGLE_GENERATIVE_AI_API_KEY or ""
    end
    
    Note over V3Evaluator,Env: Evaluator API Key Loading
    User->>V3Evaluator: Initialize
    V3Evaluator->>Env: Check GEMINI_API_KEY
    alt GEMINI_API_KEY exists
        Env-->>V3Evaluator: Use GEMINI_API_KEY
    else Check fallback
        V3Evaluator->>Env: Check GOOGLE_GENERATIVE_AI_API_KEY
        Env-->>V3Evaluator: Use GOOGLE_GENERATIVE_AI_API_KEY or ""
    end
Loading

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +207 to +219
try {
apiKey = loadApiKeyFromEnv(
this.modelName.split("/")[0], // "openai", "anthropic", etc
this.logger,
);
} catch (error) {
this.logger({
category: "init",
message: `Error loading API key for model ${this.modelName}: ${error}. Continuing without LLM client.`,
level: 0,
});
throw error;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: loadApiKeyFromEnv never throws errors (it returns undefined on failure), making this try-catch unreachable.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/lib/v3/v3.ts
Line: 207:219

Comment:
**style:** `loadApiKeyFromEnv` never throws errors (it returns `undefined` on failure), making this try-catch unreachable.

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

@seanmcguire12 seanmcguire12 merged commit 3940f6e into v3 Oct 27, 2025
14 of 15 checks passed
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