Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update gemini 2 flash thinking model name check #1283

Merged

Conversation

Gasser-Aly
Copy link
Contributor

@Gasser-Aly Gasser-Aly commented Jan 1, 2025

a Small change to update the check for gemini 2 flash thinking model to use contains instead of == to allow specifying a region if needed with OpenAI compatible proxies.

Ex: usage:

client<llm> GEMINI_V2_FLASH_THINKING {
  provider "openai-generic"
  retry_policy Exponential
  options {
    model "googlevertexai-us-central1:gemini-2.0-flash-thinking-exp-1219"
    api_key env.MY_TOKEN
    base_url "https://my-proxy.ai/v1"
  }
}

Important

Update content_part() in googleai_client.rs to use contains for model name check, allowing region-specific names.

  • Behavior:
    • Update content_part() in googleai_client.rs to use contains instead of == for model name check, allowing region-specific model names.
  • Misc:
    • No other changes or refactoring in the code.

This description was created by Ellipsis for 9f19ef8. It will automatically update as commits are pushed.

Copy link

vercel bot commented Jan 1, 2025

Someone is attempting to deploy a commit to the Gloo Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 9f19ef8 in 12 seconds

More details
  • Looked at 17 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. engine/baml-runtime/src/internal/llm_client/primitive/google/googleai_client.rs:470
  • Draft comment:
    Consider adding a comment to content_part to explain the use of contains for model name matching, as it allows for more flexible specification, including regions.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The change from == to contains is appropriate for the described use case, allowing for more flexible model name matching. However, the function content_part should be documented to reflect this change and its implications.

Workflow ID: wflow_mfIb1xzRh1u3tiG9


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@sxlijin
Copy link
Collaborator

sxlijin commented Jan 1, 2025

Thanks! We'll get this merged asap

@hellovai hellovai added this pull request to the merge queue Jan 3, 2025
Merged via the queue into BoundaryML:canary with commit 76ceeff Jan 3, 2025
6 of 7 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