-
-
Notifications
You must be signed in to change notification settings - Fork 695
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
Writer Pipeline check #1202
Writer Pipeline check #1202
Conversation
…ntire item at the end
…nd deleted failing multitask.py task
There was a problem hiding this 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 c2924c4 in 2 minutes and 12 seconds
More details
- Looked at
1778
lines of code in28
files - Skipped
1
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. docs/blog/posts/writer-support.md:120
- Draft comment:
There's a typo in the comment:eadline
should bedeadline
. - Reason this comment was not posted:
Confidence changes required:10%
The PR introduces a new feature for Writer integration, but there is a minor typo in the blog post that needs correction.
2. docs/blog/posts/writer-support.md:160
- Draft comment:
There's a typo in the text:sentimen
should besentiment
. - Reason this comment was not posted:
Confidence changes required:10%
The PR introduces a new feature for Writer integration, but there is a minor typo in the blog post that needs correction.
3. tests/llm/test_writer/evals/test_classification_enums.py:56
- Draft comment:
Assertions should include error messages for clarity. This applies to other test files as well. - Reason this comment was not posted:
Confidence changes required:80%
The assertion statements in the test files lack error messages. Adding error messages will make it easier to understand the cause of a test failure.
Workflow ID: wflow_DCgu8wYlqfzpnyT0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Deploying instructor-py with Cloudflare Pages
|
There was a problem hiding this 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 0c51c63 in 22 seconds
More details
- Looked at
22
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. tests/llm/test_openai/test_multimodal.py:73
- Draft comment:
Consider using a more stable image source or mocking the image request to avoid potential test failures due to changes in the image URL. - Reason this comment was not posted:
Confidence changes required:50%
The test functiontest_multimodal_image_description
uses a hardcoded URL for an image. This could lead to test failures if the image is removed or changed. It's better to use a more stable image source or mock the image request.
2. tests/llm/test_openai/test_multimodal.py:105
- Draft comment:
Ensure that the updated image URL is valid and accessible for the test to function correctly. - Reason this comment was not posted:
Confidence changes required:50%
The change in the URL in the test function does not violate any of the specified rules.
Workflow ID: wflow_wdUrtd9Sa1zSvo38
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Integrates Writer's Palmyra-X-004 model for structured outputs, updates documentation, and adds tests.
instructor
.from_writer()
function inclient_writer.py
for Writer client initialization.Mode
enum inmode.py
to includeWRITER_TOOLS
.writer-support.md
blog post announcing Writer integration.index.md
andwriter.md
inintegrations
to include Writer.process_response.py
,function_calls.py
,reask.py
, andutils.py
to handle Writer-specific logic.__init__.py
to conditionally importfrom_writer
.tests/llm/test_writer
for classification, sentiment analysis, entity extraction, and streaming with Writer.conftest.py
to skip tests ifWRITER_API_KEY
is not set.writer-sdk
topyproject.toml
as an optional dependency.This description was created by for 0c51c63. It will automatically update as commits are pushed.