fix(sdk): allow LLM config validation to to be disabled #99
Conversation
WalkthroughAdds a validation control flag that is threaded through profile addition and build steps, allowing optional profile validation and influencing default-profile selection and creation during AgentConfigBuilder.build(). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Builder as AgentConfigBuilder
participant Validator as ProfileValidator
participant ConfigRepo as ConfigDefaults
Client->>Builder: add_profiles(profiles, validate=True)
loop per profile
Builder->>Builder: add_profile(profile, validate)
alt validate == True
Builder->>Validator: validate(profile)
Validator-->>Builder: validation result
end
end
Client->>Builder: build(validate_profiles=True)
Builder->>Builder: if no profiles -> request default
alt no profiles
Builder->>ConfigRepo: get_default_minitap_llm_config(validate=validate_profiles)
ConfigRepo-->>Builder: default_profile
Builder->>Builder: add_profile(default_profile, validate=validate_profiles)
else profiles exist
Builder->>ConfigRepo: get_default_llm_config()
ConfigRepo-->>Builder: default_profile
Builder->>Builder: use existing profiles (no forced add)
end
Builder-->>Client: final configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
minitap/mobile_use/sdk/builders/agent_config_builder.py (5)
49-59: Document the new validate parameter and consider guarding duplicate names.
- Docstring omits validate; please document its purpose and side effects.
- Optional: prevent silent overwrites when adding a profile with an existing name.
Apply this doc tweak:
def add_profile(self, profile: AgentProfile, validate: bool = True) -> "AgentConfigBuilder": """ Add an agent profile to the mobile-use agent. Args: - profile: The agent profile to add + profile: The agent profile to add. + validate: When True (default), run profile.llm_config.validate_providers(). """ - self._agent_profiles[profile.name] = profile + # Optionally guard duplicates (consider raising or warning instead if you prefer): + # if profile.name in self._agent_profiles: + # raise ValueError(f"Profile '{profile.name}' already exists") + self._agent_profiles[profile.name] = profile if validate: profile.llm_config.validate_providers() return self
61-65: Propagate and document validate in add_profiles; consider Sequence over list.
- Docstring also omits validate.
- Using collections.abc.Sequence improves flexibility for callers.
Doc + type tweaks:
-from typing import List +from collections.abc import Sequence @@ - def add_profiles( - self, - profiles: list[AgentProfile], - validate: bool = True, - ) -> "AgentConfigBuilder": + def add_profiles( + self, + profiles: Sequence[AgentProfile], + validate: bool = True, + ) -> "AgentConfigBuilder": """ Add multiple agent profiles to the mobile-use agent. Args: - profiles: List of agent profiles to add + profiles: Agent profiles to add. + validate: When True (default), validate each profile’s providers. """ for profile in profiles: self.add_profile(profile=profile, validate=validate)Also applies to: 73-74
169-181: build() docstring still references default_profile arg; document validate_profiles and intent.The signature adds validate_profiles but the docstring mentions a non-existent argument. Please align docs and clarify what gets validated at build.
Doc fix:
def build(self, validate_profiles: bool = True) -> AgentConfig: """ Build the mobile-use AgentConfig object. - Args: - default_profile: Name of the default agent profile to use + Args: + validate_profiles: When True (default), validate providers for profiles created/added during build + (e.g., inferred default). Existing profiles added earlier are not revalidated unless you choose to + validate them below. Returns: A configured AgentConfig objectOptional: if you intend validate_profiles=True to validate all configured profiles at build time, add:
@@ - else: + else: available_profiles = ", ".join(self._agent_profiles.keys()) raise ValueError( f"You must call with_default_profile() to select one among: {available_profiles}" ) - return AgentConfig( + # Optional: validate all existing profiles when requested + if validate_profiles: + for p in self._agent_profiles.values(): + p.llm_config.validate_providers() + + return AgentConfig(If you prefer not to revalidate, consider renaming the arg to validate_default_profile for clarity.
192-203: Default-profile path: behavior when validate_profiles=False may mask missing credentials.With validate_profiles=False, get_default_minitap_llm_config(validate=False) returns a minitap config even without MINITAP_API_KEY, producing a default profile that will likely fail later at runtime.
- This matches the PR goal (disable validation), but consider logging a warning when returning a minitap config without credentials to aid debugging.
- Alternatively, prefer get_default_llm_config() unless an explicit “prefer_minitap” flag is set.
Would you like a follow-up patch to emit a warning when validate_profiles=False and MINITAP_API_KEY is unset?
212-220: Return-time immutability: copy agent_profiles to avoid external mutation post-build.Builder currently returns internal dict; subsequent external mutation can affect a built AgentConfig.
Minimal change:
- return AgentConfig( - agent_profiles=self._agent_profiles, + return AgentConfig( + agent_profiles=copy.deepcopy(self._agent_profiles), task_request_defaults=self._task_request_defaults or TaskRequestCommon(), default_profile=default_profile, device_id=self._device_id, device_platform=self._device_platform, servers=self._servers, graph_config_callbacks=self._graph_config_callbacks, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
minitap/mobile_use/sdk/builders/agent_config_builder.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
minitap/mobile_use/sdk/builders/agent_config_builder.py (3)
minitap/mobile_use/sdk/types/task.py (1)
AgentProfile(20-56)minitap/mobile_use/config.py (3)
validate_providers(165-172)get_default_minitap_llm_config(243-292)get_default_llm_config(193-240)minitap/mobile_use/sdk/types/agent.py (1)
AgentConfig(57-78)
Summary by CodeRabbit