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

gemini 1.5 pro & flash #1308

Merged
merged 2 commits into from
Dec 2, 2024
Merged

gemini 1.5 pro & flash #1308

merged 2 commits into from
Dec 2, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Dec 2, 2024

Important

Add configurations for gemini-1.5-pro and gemini-1.5-flash models with vision support and update README.

  • Configurations:
    • Added GEMINI_PRO configuration for gemini-1.5-pro with supports_vision=True and max_output_tokens=8192 in config_registry.py.
    • Added GEMINI_FLUSH configuration for gemini-1.5-flash with supports_vision=True and max_output_tokens=8192 in config_registry.py.
  • Documentation:
    • Updated README.md to include GEMINI_PRO and GEMINI_FLUSH in the list of supported LLM keys.

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

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 978b0cd in 36 seconds

More details
  • Looked at 23 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/api/llm/config_registry.py:263
  • Draft comment:
    The configuration key "GEMINI_FLUSH" might be a typo. Consider renaming it to "GEMINI_FLASH" for consistency with the naming convention.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is addressing a potential inconsistency between the configuration key 'GEMINI_FLUSH' and the model name 'gemini/gemini-1.5-flash'. This could be a valid point if the naming convention is to match the key with the model name. However, without additional context on naming conventions, it's speculative. The comment is about a change made in the diff, so it is relevant.
    I might be overestimating the importance of matching the configuration key with the model name. The naming might be intentional for reasons not visible in the diff.
    Without clear evidence that the naming is intentional, it's reasonable to consider the comment as potentially valid. However, the lack of strong evidence means the comment might be speculative.
    The comment is speculative and lacks strong evidence of an issue. It should be deleted as it does not meet the criteria for a necessary code change.

Workflow ID: wflow_SOF0Meli6WzHQCtK


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

@wintonzheng wintonzheng merged commit 6db01f4 into main Dec 2, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/use_gemini_pro branch December 3, 2024 00:00
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.

❌ Changes requested. Incremental review on 0aee3c2 in 41 seconds

More details
  • Looked at 14 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_UATD8vOAnHjwACgO


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -285,7 +285,8 @@ More extensive documentation can be found on our [documentation website](https:/
| `ENABLE_AZURE` | Register Azure OpenAI models | Boolean | `true`, `false` |
| `ENABLE_BEDROCK` | Register AWS Bedrock models| Boolean | `true`, `false` |
| `ENABLE_GEMINI` | Register Gemini models| Boolean | `true`, `false` |
| `LLM_KEY` | The name of the model you want to use | String | Currently supported llm keys: `OPENAI_GPT4_TURBO`, `OPENAI_GPT4V`, `OPENAI_GPT4O`, `OPENAI_GPT4O_MINI`, `ANTHROPIC_CLAUDE3`, `ANTHROPIC_CLAUDE3_OPUS`, `ANTHROPIC_CLAUDE3_SONNET`, `ANTHROPIC_CLAUDE3_HAIKU`, `ANTHROPIC_CLAUDE3.5_SONNET`, `BEDROCK_ANTHROPIC_CLAUDE3_OPUS`, `BEDROCK_ANTHROPIC_CLAUDE3_SONNET`, `BEDROCK_ANTHROPIC_CLAUDE3_HAIKU`, `BEDROCK_ANTHROPIC_CLAUDE3.5_SONNET`, `AZURE_OPENAI` |
| `LLM_KEY` | The name of the model you want to use | String | Currently supported llm keys: `OPENAI_GPT4_TURBO`, `OPENAI_GPT4V`, `OPENAI_GPT4O`, `OPENAI_GPT4O_MINI`, `ANTHROPIC_CLAUDE3`, `ANTHROPIC_CLAUDE3_OPUS`, `ANTHROPIC_CLAUDE3_SONNET`, `ANTHROPIC_CLAUDE3_HAIKU`, `ANTHROPIC_CLAUDE3.5_SONNET`, `BEDROCK_ANTHROPIC_CLAUDE3_OPUS`, `BEDROCK_ANTHROPIC_CLAUDE3_SONNET`, `BEDROCK_ANTHROPIC_CLAUDE3_HAIKU`, `BEDROCK_ANTHROPIC_CLAUDE3.5_SONNET`, `AZURE_OPENAI`, `GEMINI_PRO`, `GEMINI_FLUSH`|
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: GEMINI_FLUSH should be GEMINI_FLASH for consistency with the configuration name gemini-1.5-flash. This typo is also present in the SECONDARY_LLM_KEY description.

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