fix: clean up cohere templating

[jxnl]

Important

Refactor Cohere templating by integrating it into handle_templating in templating.py, simplifying code, and updating tests.

• Refactoring:
  • Remove handle_cohere_templating from patch.py and integrate logic into handle_templating in templating.py.
  • Simplify from_cohere in client_cohere.py by removing new_create_async.
• Functionality:
  • handle_templating in templating.py now processes kwargs for Cohere, OpenAI, Anthropic, VertexAI, and Gemini formats.
  • Ensure kwargs contains message, messages, or contents in handle_templating.
• Tests:
  • Update tests in test_formatting.py to use handle_templating from templating.py and cover various message formats.
• Misc:
  • Remove unused imports in client_cohere.py.

^{This description was created by for e8f716e. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to c4ccc42 in 10 seconds

More details

• Looked at 230 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. instructor/patch.py:96

• Draft comment:
  The return type in the docstring is incorrect. It should be dict[str, Any] instead of list[dict[str, Any]].
• Reason this comment was not posted:
  Comment was on unchanged code.

2. instructor/patch.py:110

• Draft comment:
  Assertion error messages should be more descriptive. Consider providing more context in the error message to help with debugging.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The assertion error messages in the handle_templating function should be more descriptive.

Workflow ID: wflow_KECPvk0OQBJZn4E3

---

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

[cloudflare-workers-and-pages]

Deploying instructor with    Cloudflare Pages

Latest commit:	4ea5cd1
Status:	✅  Deploy successful!
Preview URL:	https://988ae834.instructor.pages.dev
Branch Preview URL:	https://fix-cohere-formatting.instructor.pages.dev

View logs

[ellipsis-dev]

👍 Looks good to me! Incremental review on 9629f48 in 9 seconds

More details

• Looked at 444 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. instructor/templating.py:15

• Draft comment:
  Consider moving the import statement for vertexai.generative_models to the top of the file to avoid repeated imports and improve performance.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The import statement for vertexai.generative_models is inside a function, which can lead to performance issues if the function is called frequently. It's better to move the import to the top of the file.

2. instructor/templating.py:56

• Draft comment:
  If handle_templating is part of the library's public API, ensure the documentation is updated to reflect the changes made to this function.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The function handle_templating in instructor/templating.py has been updated, but there is no corresponding update in the documentation. Since this is a library code change, the documentation should be updated accordingly.

3. tests/test_formatting.py:1

• Draft comment:
  Ensure that the documentation is updated to reflect the changes made to handle_templating if it's part of the public API.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The function handle_templating in instructor/templating.py has been updated, but there is no corresponding update in the documentation. Since this is a library code change, the documentation should be updated accordingly.

Workflow ID: wflow_QHjPdv0hUUBG83Zy

---

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

[ellipsis-dev]

❌ Changes requested. Incremental review on d2a5c7c in 17 seconds

More details

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

1. instructor/templating.py:1

• Draft comment:
  Avoid using # type: ignore[all]. Instead, address specific type issues to ensure type safety.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The use of # type: ignore[all] suppresses all type checking for this file, which can hide potential type-related issues. It's better to address specific type errors rather than ignoring all of them.

Workflow ID: wflow_azrUW2ProRxOjWzd

---

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.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 4093d2b in 10 seconds

More details

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

Workflow ID: wflow_Cn1hCsnExyxb9YNU

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 8a65396 in 35 seconds

More details

• Looked at 84 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. instructor/templating.py:8

• Draft comment:
  Using dict instead of Dict is not compatible with Python versions below 3.9. Consider using Dict for backward compatibility.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The change from using Dict to dict is not backward compatible with Python versions below 3.9. This could cause issues if the codebase needs to support older versions.

2. instructor/templating.py:13

• Draft comment:
  Using dict instead of Dict is not compatible with Python versions below 3.9. Consider using Dict for backward compatibility. (Also applicable on lines 13, 59, and 60)
• Reason this comment was not posted:
  Confidence changes required: 80%
  The same issue with using dict instead of Dict appears in multiple places in the file. It's important to address all occurrences for consistency and compatibility.

3. instructor/templating.py:94

• Draft comment:
  handle_templating should return a consistent type. Consider returning new_kwargs instead of None to avoid unexpected errors.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

4. tests/test_formatting.py:135

• Draft comment:
  The test expects a ValueError, but handle_templating no longer raises this error. Update the test to reflect the new behavior of handle_templating.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment claims that handle_templating no longer raises a ValueError, but there is no evidence in the provided diff or context to support this claim. The test is designed to catch a ValueError, and the change in the diff is only a stylistic one. Without evidence that handle_templating has changed its behavior, the comment seems speculative.
  I might be missing information about changes to handle_templating that are not visible in this diff. The comment could be based on changes in another part of the codebase.
  Given the rules, I should only consider the changes visible in this diff. Without evidence in the diff that handle_templating has changed, the comment should be considered speculative and removed.
  Remove the comment as it is speculative and not supported by evidence in the diff.

5. tests/test_formatting.py:132

• Draft comment:
  The handle_templating function no longer raises a ValueError for unknown formats, but this test still expects it. Update the test to reflect the new behavior.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_4PHs3uHnNUM5PLE3

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on e8f716e in 13 seconds

More details

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

1. tests/test_formatting.py:124

• Draft comment:
  Consider re-adding a test to ensure handle_templating raises an error for unknown formats. This ensures robustness against unexpected input.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The test test_handle_templating_raises_error_for_unknown_format has been removed, but the functionality to raise an error for unknown formats should still be tested to ensure robustness.

2. tests/test_formatting.py:124

• Draft comment:
  Assertions should include an error message for clarity. Please add an error message to the assertion in test_handle_templating_raises_error_for_unknown_format.
• Reason this comment was not posted:
  Confidence changes required: 80%
  The test function test_handle_templating_raises_error_for_unknown_format has an assertion without an error message, which violates the rule that assertions should always have an error message.

Workflow ID: wflow_sosxoYgJVd54lQOR

---

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