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

fix(parallel): enhance error handling in get_types_array and add test case #423

Merged
merged 3 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ellipsis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: 1.1
pr_review:
auto_review_enabled: true
auto_summarize_pr: true
confidence_threshold: 0.9
confidence_threshold: 0.85
rules:
# Control what gets flagged during PR review with custom rules. Here are some to get you started:
- "Code should be DRY (Dont Repeat Yourself)"
Expand All @@ -13,3 +13,4 @@ pr_review:
- "If library code changes, expect documentation to be updated"
- "If library code changes, check if tests are updated"
- "If a new `md` file is created in `docs` make sure its added to mkdocs.yml"
- "Assertions should always have an error message that is formatted well. "
3 changes: 2 additions & 1 deletion instructor/dsl/parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ def from_response(

def get_types_array(typehint: Type[Iterable[Union[T]]]) -> Tuple[Type[T], ...]:
should_be_iterable = get_origin(typehint)
assert should_be_iterable is Iterable
if should_be_iterable is not Iterable:
raise TypeError(f"Model should be with Iterable instead if {typehint}")

if get_origin(get_args(typehint)[0]) is Union:
# works for Iterable[Union[int, str]]
Expand Down
3 changes: 2 additions & 1 deletion instructor/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,9 +339,10 @@ async def retry_async(
"name": response.choices[0]
.message.tool_calls[0]
.function.name,
"content": "failure",
"content": "Exceptions found\n{e}\nRecall the function correctly.",
}
)

kwargs["messages"].append(
{
"role": "user",
Expand Down
17 changes: 17 additions & 0 deletions tests/openai/test_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,23 @@ class GoogleSearch(BaseModel):
query: str


def test_sync_parallel_tools__error(client):
client = instructor.patch(client, mode=instructor.Mode.PARALLEL_TOOLS)

with pytest.raises(TypeError):
Copy link
Owner Author

Choose a reason for hiding this comment

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

@lukevs We should definitely be switching back to regular tools mode if we can detect that it's not an iterable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Need it still for the Boy | Girl case too right (to get the name disambiguation)

resp = client.chat.completions.create(
model="gpt-4-turbo-preview",
messages=[
{"role": "system", "content": "You must always use tools"},
{
"role": "user",
"content": "What is the weather in toronto and dallas and who won the super bowl?",
},
],
response_model=Weather,
)


def test_sync_parallel_tools_or(client):
client = instructor.patch(client, mode=instructor.Mode.PARALLEL_TOOLS)
resp = client.chat.completions.create(
Expand Down
Loading