-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add support for allowing clients to limit which finish_reasons they allow. #1026
base: canary
Are you sure you want to change the base?
Conversation
…llow. Clients can do: ``` client<llm> MyClient { provider "openai" options { finish_reason_whitelist ["stop"] } } client<llm> MyClient { provider "openai" options { finish_reason_blacklist ["content_policy"] } } ```
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
❌ Changes requested. Reviewed everything up to e56e099 in 2 minutes and 57 seconds
More details
- Looked at
5332
lines of code in41
files - Skipped
0
files when reviewing. - Skipped posting
7
drafted comments based on config settings.
1. engine/baml-runtime/src/internal/llm_client/primitive/openai/properties/azure.rs:6
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
2. engine/baml-runtime/src/internal/llm_client/primitive/openai/properties/generic.rs:6
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
3. engine/baml-runtime/src/internal/llm_client/primitive/openai/properties/ollama.rs:6
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
4. engine/baml-runtime/src/internal/llm_client/primitive/openai/properties/openai.rs:6
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
5. engine/baml-runtime/src/internal/llm_client/primitive/vertex/vertex_client.rs:2
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
6. engine/baml-runtime/src/internal/llm_client/strategy/fallback.rs:11
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
7. engine/baml-runtime/src/internal/llm_client/strategy/roundrobin.rs:19
- Draft comment:
Typo in module nameproperties_hander
. It should beproperties_handler
. This typo is present in multiple files. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_qu1hOzTDbk2lbddE
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.
|
||
use super::AllowedMetadata; | ||
|
||
pub enum FinishReasonOptions { |
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.
Typo in module name properties_hander
. It should be properties_handler
. This typo is present in multiple files.
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 535156c in 1 minute and 31 seconds
More details
- Looked at
149
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. engine/baml-runtime/src/internal/llm_client/properties_hander.rs:78
- Draft comment:
The error message still uses 'whitelist'. It should be updated to 'allowlist' for consistency.
None => anyhow::bail!("finish_reason_allowlist must be an array of strings"),
- Reason this comment was not posted:
Marked as duplicate.
2. engine/baml-runtime/src/internal/llm_client/properties_hander.rs:89
- Draft comment:
The error message still uses 'blacklist'. It should be updated to 'denylist' for consistency.
None => anyhow::bail!("finish_reason_denylist must be an array of strings"),
- Reason this comment was not posted:
Marked as duplicate.
3. engine/baml-runtime/src/internal/llm_client/properties_hander.rs:98
- Draft comment:
The error message still uses 'whitelist' and 'blacklist'. It should be updated to 'allowlist' and 'denylist' for consistency.
"Only one of finish_reason_allowlist or finish_reason_denylist can be specified"
- Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_YH5HkKmtCKnRPu3O
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Clients can do:
Important
Add support for client-configurable finish reason whitelists and blacklists with updated error handling and tests.
finish_reason_whitelist
andfinish_reason_blacklist
options to client configurations for controlling acceptable finish reasons.PropertiesHandler
to handle these options and validate finish reasons.BamlValidationError
for non-terminal finish reasons.OpenAIClient
,AnthropicClient
,GoogleAIClient
,AwsClient
, andVertexClient
.resolve_properties
functions to extract and store finish reason options.integ-tests/python/tests/test_functions.py
andinteg-tests/typescript/tests/integ-tests.test.ts
to verify finish reason handling.This description was created by for 535156c. It will automatically update as commits are pushed.