-
Notifications
You must be signed in to change notification settings - Fork 20
feat: add model settings id parameter #1053
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
base: main
Are you sure you want to change the base?
Conversation
src/uipath/_cli/_evals/_runtime.py
Outdated
| logger.warning(f"Model settings ID '{self.context.model_settings_id}' not found in evaluation set") | ||
| return None | ||
|
|
||
| console.info(f"Found model settings: model='{target_model_settings.model_name}', temperature='{target_model_settings.temperature}'") |
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.
Please use logger, console is a CLI concept (imagine we extract the evals engine to another library)
src/uipath/_cli/_evals/_runtime.py
Outdated
| return None | ||
|
|
||
| # Load the original entrypoint file | ||
| entrypoint_path = Path(self.context.entrypoint or "agent.json") |
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.
We shouldn't leak agent.json concepts here .. similar comment: #1048 (review)
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.
+1.
The other PR uses protocols to break this. Maybe we need to update the protocol method to return EvaluationSetModelSettings instead of model_name.
| max_tokens: int | None = Field(default=None, alias="maxTokens") | ||
|
|
||
|
|
||
| class EvaluationSetModelSettings(BaseModel): |
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.
Should we reuse ModelSettings instead? EvaluationSetModelSettings seems to be a subset of ModelSettings, so it should be forward-compatible.
|
@mathurk @Chibionos we added this "metadata" bag to You can add these settings in the low-code agents repo, and access them here via the |
Development Package
uipath pack --nolockto get the latest dev build from this PR (requires version range).