-
Notifications
You must be signed in to change notification settings - Fork 53
Remove modelRouter and add model_store #224
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
…del store BREAKING CHANGE: Removed ModelRouter and ModelRegistry classes. Users must now use direct model injection.
…raises an Error when executing from cache)
…nd AskUiInferenceLocateApi
docs/01_Setup.md
Outdated
| **Problem**: Error connecting to Agent OS | ||
|
|
||
| **Solutions**: | ||
| 1. Check if Agent OS is running (look for the system tray icon) |
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.
The agent OS doesn’t have a tray icon.
docs/01_Setup.md
Outdated
|
|
||
| **Solutions**: | ||
| 1. Check if Agent OS is running (look for the system tray icon) | ||
| 2. Restart Agent OS from your applications menu |
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.
The agent OS is not listed in the application menu.
| custom_settings = ActSettings( | ||
| messages=MessageSettings( | ||
| max_tokens=8192, | ||
| temperature=0.5, | ||
| betas=["computer-use-2025-01-24"], | ||
| ) | ||
| ) | ||
|
|
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.
Which system prompt is used in this example?
Could you please remove the betas?
docs/01_Setup.md
Outdated
|
|
||
| ## Python Package Installation | ||
|
|
||
| AskUI Vision Agent requires Python 3.10 or higher. |
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.
requires-python = ">=3.10,<3.14"
docs/01_Setup.md
Outdated
| ```bash | ||
| pip install askui[anthropic] # Anthropic Claude support | ||
| pip install askui[openrouter] # OpenRouter support | ||
| pip install askui[documents] # PDF, Excel, Word support | ||
| ``` |
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.
SDK dosent support these targets
| super().__init__(self.message) | ||
|
|
||
|
|
||
| class AnthropicModelSettings(BaseSettings): |
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.
Currently unused.
| self, | ||
| locator: str | Locator, | ||
| image: ImageSource, | ||
| locate_settings: LocateSettings, # noqa: ARG002 |
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.
Are the Locate settings only needed for LLM-based locators?
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.
idea was to have a general settings object for all locate commands here
| max_tokens: int = 4096 | ||
| temperature: float = Field(default=0.5, ge=0.0, le=1.0) | ||
| system_prompt: GetSystemPrompt | None = None | ||
| timeout: float | None = None | ||
|
|
||
|
|
||
| class LocateSettings(BaseModel): | ||
| """Settings for LocateModel operations (UI element location).""" | ||
|
|
||
| model_config = ConfigDict(arbitrary_types_allowed=True) | ||
|
|
||
| query_type: str | None = None | ||
| confidence_threshold: float = Field(default=0.8, ge=0.0, le=1.0) | ||
| max_detections: int = 10 | ||
| timeout: float | None = None | ||
| system_prompt: LocateSystemPrompt | None = None |
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.
Currently, only the system prompt is being used.
| confidence_threshold: float = Field(default=0.8, ge=0.0, le=1.0) | ||
| max_detections: int = 10 | ||
| timeout: float | None = None | ||
| system_prompt: LocateSystemPrompt | None = None |
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.
The Locate system prompt should not be configurable because the expected return is currently hard-coded. Changing the system prompt would cause the Locate code to fail.
| timeout: float | None = None | ||
|
|
||
|
|
||
| class LocateSettings(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.
What do you think about removing the Locate and Get settings?
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.
I think we should generally discuss how "configurable" get and locate should be. This also includes if we want to support BYOM for these commands or if that should only be possible for act
This PR:
(especially the last 2 bloat the PR a bit, sorry for that)
Summary
Key Changes
Breaking Change