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

Fixed new templating feature throwing an error for gemini #1021

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Conversation

ivanleomk
Copy link
Collaborator

@ivanleomk ivanleomk commented Sep 26, 2024

Fixed a bug with async client


Important

Fix async client bug with Jinja templating and update Google Generative AI dependency version.

  • Bug Fixes:
    • In new_create_async() in patch.py, check if "messages" exists in new_kwargs before applying handle_templating().
    • Extend error handling in retry_async() and retry_sync() in retry.py for Mode.GEMINI_JSON, Mode.GEMINI_TOOLS, Mode.VERTEXAI_TOOLS, and Mode.VERTEXAI_JSON.
  • Dependencies:
    • Bump google-generativeai version from ^0.5.4 to ^0.8.2 in pyproject.toml.

This description was created by Ellipsis for 79a9711. 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 6b66cd7 in 11 seconds

More details
  • Looked at 66 lines of code in 3 files
  • Skipped 1 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. instructor/patch.py:225
  • Draft comment:
    Check for 'messages' key before calling handle_templating to avoid KeyError. This is a good practice.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR description mentions a bug fix related to async client, which is addressed in the instructor/patch.py file. The change ensures that handle_templating is only called if messages is present in new_kwargs. This is a good practice to avoid KeyErrors.
2. instructor/retry.py:278
  • Draft comment:
    Added handling for new modes in retry logic, consistent with PR description.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR description mentions a bug fix related to async client, which is addressed in the instructor/retry.py file. The change adds handling for new modes in the retry logic, which is consistent with the PR description.
3. pyproject.toml:36
  • Draft comment:
    Updated google-generativeai version to ensure compatibility with latest features or bug fixes.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR updates the version of google-generativeai in pyproject.toml. This is consistent with the PR description and ensures compatibility with the latest features or bug fixes in the dependency.
4. pyproject.toml:36
  • Draft comment:
    When updating library versions, ensure that related documentation and tests are also updated to reflect any changes in functionality or usage.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The PR updates the version of the google-generativeai library in the pyproject.toml file. However, there is no indication that the documentation or tests have been updated to reflect this change.

Workflow ID: wflow_NAfQTipEjgbzF5U8


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

@@ -222,7 +222,8 @@ async def new_create_async(
response_model, new_kwargs = handle_response_model(
response_model=response_model, mode=mode, **kwargs
)
new_kwargs["messages"] = handle_templating(new_kwargs["messages"], context)
if "messages" in new_kwargs:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gemini has a kwargs of "content" so this throws an error for gemini with the new jinja support

Copy link

cloudflare-workers-and-pages bot commented Sep 26, 2024

Deploying instructor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9e39b7a
Status:⚡️  Build in progress...

View logs

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! Incremental review on 79a9711 in 11 seconds

More details
  • Looked at 29 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. pyproject.toml:36
  • Draft comment:
    The version for google-generativeai should be updated to ^0.8.2 as per the PR description, not ^0.5.4. Please correct this discrepancy.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. pyproject.toml:36
  • Draft comment:
    The version of google-generativeai should be updated to ^0.8.2 as mentioned in the PR description. Please ensure the version is consistent across the file.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_OHc057SXcGb0U0ox


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

@ivanleomk ivanleomk closed this Sep 26, 2024
@ivanleomk ivanleomk reopened this Sep 26, 2024
@ivanleomk ivanleomk changed the title Fixed up poetry dependencies and google gemini bug with jinja templating Fixed new templating feature throwing an error for gemini Sep 26, 2024
@jxnl jxnl merged commit f536202 into main Sep 26, 2024
11 of 15 checks passed
@jxnl jxnl deleted the gemini-async-fix branch September 26, 2024 16:03
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.

2 participants