Refine create_agent model defaults and AgentRuntime behavior#28
Merged
JohnRichard4096 merged 4 commits intomainfrom Mar 15, 2026
Merged
Refine create_agent model defaults and AgentRuntime behavior#28JohnRichard4096 merged 4 commits intomainfrom
JohnRichard4096 merged 4 commits intomainfrom
Conversation
Contributor
Reviewer's GuideIntroduces required system-train handling and model defaults for agents, refines MCP generics and hook control-flow typing/coverage hints, makes SessionData fields lazily constructed, updates docs and demos to the new create_agent signature and behavior, bumps the package version, and adds comprehensive tests for AgentRuntime and create_agent. Sequence diagram for the updated create_agent flowsequenceDiagram
actor User
participant create_agent
participant Config as AmritaConfig
participant ModelConfig
participant ModelPreset
participant AgentRuntime
participant Message
User ->> create_agent: create_agent(url, key, model, train, model_config, config, **kwargs)
alt train is None
create_agent ->> create_agent: train = "You are a helpful assistant."
end
alt config is None
create_agent ->> Config: get_config()
Config -->> create_agent: final_config
else config provided
create_agent ->> create_agent: final_config = config
end
alt model_config is dict
create_agent ->> ModelConfig: ModelConfig(**model_config)
ModelConfig -->> create_agent: model_config_obj
else model_config is None
create_agent ->> ModelConfig: ModelConfig()
ModelConfig -->> create_agent: model_config_obj
else model_config is ModelConfig
create_agent ->> create_agent: model_config_obj = model_config
end
create_agent ->> ModelPreset: ModelPreset(name=temp_xxxx, base_url=url, api_key=key, config=model_config_obj, model=model)
ModelPreset -->> create_agent: preset
create_agent ->> Message: Message(content=train, role=system)
Message -->> create_agent: system_message
create_agent ->> AgentRuntime: AgentRuntime(config=final_config, preset=preset, train=system_message, **filtered_kwargs)
AgentRuntime -->> User: agent_instance
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
Author
|
@sourcery-ai title |
sourcery-ai
Bot
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
AgentRuntime.__init__signature now requirestrain(no default/None), which may break existing callers that relied on the previous optional parameter; consider keepingtrainoptional at the public API level (with a sensible default) and enforcing non-Noneinternally instead. - The new
MCP_SERVER_SCRIPT_TYPEis declaredcovariant=Truebut is used in parameter positions (e.g.get_client_by_script(self, server_script: MCP_SERVER_SCRIPT_TYPE)), which is invalid for a covariant type variable and will confuse type checkers; consider removing covariance or splitting into separate input/output type variables.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `AgentRuntime.__init__` signature now requires `train` (no default/`None`), which may break existing callers that relied on the previous optional parameter; consider keeping `train` optional at the public API level (with a sensible default) and enforcing non-`None` internally instead.
- The new `MCP_SERVER_SCRIPT_TYPE` is declared `covariant=True` but is used in parameter positions (e.g. `get_client_by_script(self, server_script: MCP_SERVER_SCRIPT_TYPE)`), which is invalid for a covariant type variable and will confuse type checkers; consider removing covariance or splitting into separate input/output type variables.
## Individual Comments
### Comment 1
<location path="src/amrita_core/agent/functions.py" line_range="181-183" />
<code_context>
)
```
"""
+ train = train or "You are a helpful assistant."
final_config = config or get_config()
if isinstance(model_config, dict):
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `train = train or ...` may overwrite intentional empty prompts.
Because `or` treats empty strings and other falsy values as missing, this will replace an explicitly empty system prompt with the default. If you only want a default when no value is provided, check specifically for `None`, e.g. `if train is None: train = "You are a helpful assistant."`.
```suggestion
"""
if train is None:
train = "You are a helpful assistant."
final_config = config or get_config()
```
</issue_to_address>
### Comment 2
<location path="src/amrita_core/tools/mcp.py" line_range="30-38" />
<code_context>
)
-MCP_SERVER_SCRIPT_TYPE = ClientTransportT
+MCP_SERVER_SCRIPT_TYPE = TypeVar(
+ "MCP_SERVER_SCRIPT_TYPE",
+ str,
+ ClientTransport,
+ AnyUrl,
+ FastMCP,
+ MCPConfig,
+ dict[str, Any],
+ covariant=True,
+)
</code_context>
<issue_to_address>
**suggestion:** The new `MCP_SERVER_SCRIPT_TYPE` TypeVar is very broad and may not align well with `Client`’s expectations.
This TypeVar now accepts many unrelated types and is covariant, yet is passed straight into `Client(server_script)` in `_connect`. That combination reduces type safety and makes it easier for incorrect usages to pass static checks. If all these variants are truly needed, consider a precise union type alias instead; otherwise, narrow `MCP_SERVER_SCRIPT_TYPE` closer to the original `ClientTransportT`-like constraint to better match `Client`’s real expectations.
Suggested implementation:
```python
from typing import Any
from fastmcp.mcp_config import MCPConfig
from mcp.types import TextContent
from pydantic import AnyUrl
from typing_extensions import Self
from zipp import Path
```
```python
MCP_SERVER_SCRIPT_TYPE = (
str
| ClientTransport
| AnyUrl
| FastMCP
| MCPConfig
| dict[str, Any]
)
```
If `MCP_SERVER_SCRIPT_TYPE` was previously used as a `TypeVar` in generic classes or functions (e.g., `Generic[MCP_SERVER_SCRIPT_TYPE]` or as a bound type parameter), you should:
1. Replace those generic uses with a non-generic annotation based on the new union alias semantics, for example:
- From `class Foo(Generic[MCP_SERVER_SCRIPT_TYPE]): ...` to `class Foo: ...`
- From `def bar(x: MCP_SERVER_SCRIPT_TYPE) -> MCP_SERVER_SCRIPT_TYPE: ...` to the same but now interpreted as a union instead of a type parameter.
2. Ensure that `Client`’s constructor parameter for `server_script` is annotated compatibly with this union (if you control that code). If `fastmcp.Client` already defines a similar union type alias, consider importing and reusing that alias instead of redefining it here for maximum consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Deploying amritacore with
|
| Latest commit: |
6239609
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e0a76ba4.amritacore.pages.dev |
| Branch Preview URL: | https://patch-agent-docs.amritacore.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Improve agent factory ergonomics, default behaviors, and documentation while tightening type hints and session defaults.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: