-
Notifications
You must be signed in to change notification settings - Fork 59
feat(schema): add max_tokens field to AgentDef for per-agent override #471
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?
Changes from 2 commits
cd9a25b
2d5bf6e
c43e0f1
7268552
fb0cbb0
40b3c1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1529,6 +1529,13 @@ class AgentDef(BaseModel): | |||||||||||||
| max_agent_iterations: 200 instead of using the default limit. | ||||||||||||||
| """ | ||||||||||||||
|
|
||||||||||||||
| max_tokens: int | None = Field(None, ge=1, le=200000) | ||||||||||||||
| """Maximum output tokens per response for this agent. | ||||||||||||||
|
|
||||||||||||||
| Overrides the workflow-level runtime.max_tokens for this agent. | ||||||||||||||
| Only applies to provider-backed agents (not script or human_gate). | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| """ | ||||||||||||||
|
|
||||||||||||||
| session_key: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] | None = ( | ||||||||||||||
| None | ||||||||||||||
| ) | ||||||||||||||
|
|
@@ -2083,6 +2090,8 @@ def validate_agent_type(self) -> AgentDef: | |||||||||||||
| raise ValueError("script agents cannot have 'max_session_seconds'") | ||||||||||||||
| if self.max_agent_iterations is not None: | ||||||||||||||
| raise ValueError("script agents cannot have 'max_agent_iterations'") | ||||||||||||||
| if self.max_tokens is not None: | ||||||||||||||
| raise ValueError("script agents cannot have 'max_tokens'") | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block is repeated verbatim five times, and the copying is what let
The same shape replaces all five and closes the gap: if self.type not in (None, "agent") and self.max_tokens is not None:
raise ValueError(
f"'{self.type}' agents cannot have 'max_tokens' "
"(only provider-backed agents support this field)"
)It is a net reduction in lines, and nothing existing can break: Your five existing rejection tests still pass against this, since they only match on the field name. |
||||||||||||||
| if self.session_key is not None: | ||||||||||||||
| raise ValueError("script agents cannot have 'session_key'") | ||||||||||||||
| if self.retry is not None: | ||||||||||||||
|
|
@@ -2139,6 +2148,8 @@ def validate_agent_type(self) -> AgentDef: | |||||||||||||
| raise ValueError("workflow agents cannot have 'max_session_seconds'") | ||||||||||||||
| if self.max_agent_iterations is not None: | ||||||||||||||
| raise ValueError("workflow agents cannot have 'max_agent_iterations'") | ||||||||||||||
| if self.max_tokens is not None: | ||||||||||||||
| raise ValueError("workflow agents cannot have 'max_tokens'") | ||||||||||||||
| if self.session_key is not None: | ||||||||||||||
| raise ValueError("workflow agents cannot have 'session_key'") | ||||||||||||||
| if self.retry is not None: | ||||||||||||||
|
|
@@ -2196,6 +2207,8 @@ def validate_agent_type(self) -> AgentDef: | |||||||||||||
| raise ValueError("wait agents cannot have 'max_session_seconds'") | ||||||||||||||
| if self.max_agent_iterations is not None: | ||||||||||||||
| raise ValueError("wait agents cannot have 'max_agent_iterations'") | ||||||||||||||
| if self.max_tokens is not None: | ||||||||||||||
| raise ValueError("wait agents cannot have 'max_tokens'") | ||||||||||||||
| if self.session_key is not None: | ||||||||||||||
| raise ValueError("wait agents cannot have 'session_key'") | ||||||||||||||
| if self.retry is not None: | ||||||||||||||
|
|
@@ -2269,6 +2282,8 @@ def validate_agent_type(self) -> AgentDef: | |||||||||||||
| raise ValueError("set agents cannot have 'max_session_seconds'") | ||||||||||||||
| if self.max_agent_iterations is not None: | ||||||||||||||
| raise ValueError("set agents cannot have 'max_agent_iterations'") | ||||||||||||||
| if self.max_tokens is not None: | ||||||||||||||
| raise ValueError("set agents cannot have 'max_tokens'") | ||||||||||||||
| if self.session_key is not None: | ||||||||||||||
| raise ValueError("set agents cannot have 'session_key'") | ||||||||||||||
| if self.retry is not None: | ||||||||||||||
|
|
@@ -2339,6 +2354,8 @@ def validate_agent_type(self) -> AgentDef: | |||||||||||||
| raise ValueError("terminate agents cannot have 'max_session_seconds'") | ||||||||||||||
| if self.max_agent_iterations is not None: | ||||||||||||||
| raise ValueError("terminate agents cannot have 'max_agent_iterations'") | ||||||||||||||
| if self.max_tokens is not None: | ||||||||||||||
| raise ValueError("terminate agents cannot have 'max_tokens'") | ||||||||||||||
| if self.session_key is not None: | ||||||||||||||
| raise ValueError("terminate agents cannot have 'session_key'") | ||||||||||||||
| if self.max_depth is not None: | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -667,6 +667,73 @@ def test_allowed_on_regular_agent(self) -> None: | |||||
| assert agent.max_session_seconds == 90.0 | ||||||
|
|
||||||
|
|
||||||
| class TestAgentDefMaxTokens: | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All eleven tests here exercise
def test_agent_max_tokens_overrides_workflow_default(self) -> None:
"""A per-agent max_tokens must win over the workflow-level default."""
agent_def = AgentDef(name="sampler", max_tokens=1000)
pydantic_agent = build_agent(
agent_def, system_prompt="", rendered_prompt="", default_max_tokens=4096
)
assert pydantic_agent.model_settings["max_tokens"] == 1000
def test_workflow_default_used_when_agent_max_tokens_unset(self) -> None:
"""With no per-agent override the workflow default still applies."""
agent_def = AgentDef(name="sampler")
pydantic_agent = build_agent(
agent_def, system_prompt="", rendered_prompt="", default_max_tokens=4096
)
assert pydantic_agent.model_settings["max_tokens"] == 4096I ran both against this branch and they pass. One gap in the range coverage too: Worth adding a case for |
||||||
| """Tests for max_tokens on AgentDef.""" | ||||||
|
|
||||||
| def test_default_is_none(self) -> None: | ||||||
| """Test that max_tokens defaults to None.""" | ||||||
| agent = AgentDef(name="a", model="gpt-4", prompt="test") | ||||||
| assert agent.max_tokens is None | ||||||
|
|
||||||
| def test_accepts_valid_value(self) -> None: | ||||||
| """Test that max_tokens accepts a valid value.""" | ||||||
| agent = AgentDef(name="a", model="gpt-4", prompt="test", max_tokens=8192) | ||||||
| assert agent.max_tokens == 8192 | ||||||
|
|
||||||
| def test_rejects_zero(self) -> None: | ||||||
| """Test that max_tokens rejects zero.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="a", model="gpt-4", prompt="test", max_tokens=0) | ||||||
| assert "greater than or equal to 1" in str(exc_info.value) | ||||||
|
|
||||||
| def test_rejects_negative(self) -> None: | ||||||
| """Test that max_tokens rejects negative values.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="a", model="gpt-4", prompt="test", max_tokens=-100) | ||||||
| assert "greater than or equal to 1" in str(exc_info.value) | ||||||
|
|
||||||
| def test_allowed_on_regular_agent(self) -> None: | ||||||
| """Test that regular agents can have max_tokens.""" | ||||||
| agent = AgentDef(name="a", type="agent", model="gpt-4", prompt="test", max_tokens=32768) | ||||||
| assert agent.max_tokens == 32768 | ||||||
|
|
||||||
| def test_rejects_over_200000(self) -> None: | ||||||
| """Test that max_tokens rejects values above 200000.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="a", model="gpt-4", prompt="test", max_tokens=200001) | ||||||
| assert "less than or equal to 200000" in str(exc_info.value) | ||||||
|
|
||||||
| def test_rejected_on_script_agent(self) -> None: | ||||||
| """Test that script agents cannot have max_tokens.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="s", type="script", command="echo hi", max_tokens=8192) | ||||||
| assert "max_tokens" in str(exc_info.value) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion cannot fail for the reason it looks like it is checking. Pydantic v2 echoes the input dict into the error message, so
Suggested change
Same applies to the |
||||||
|
|
||||||
| def test_rejected_on_workflow_agent(self) -> None: | ||||||
| """Test that workflow agents cannot have max_tokens.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="w", type="workflow", workflow="./sub.yaml", max_tokens=8192) | ||||||
| assert "max_tokens" in str(exc_info.value) | ||||||
|
|
||||||
| def test_rejected_on_wait_agent(self) -> None: | ||||||
| """Test that wait agents cannot have max_tokens.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="w", type="wait", duration="5s", max_tokens=8192) | ||||||
| assert "max_tokens" in str(exc_info.value) | ||||||
|
|
||||||
| def test_rejected_on_set_agent(self) -> None: | ||||||
| """Test that set agents cannot have max_tokens.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="s", type="set", value="x", max_tokens=8192) | ||||||
| assert "max_tokens" in str(exc_info.value) | ||||||
|
|
||||||
| def test_rejected_on_terminate_agent(self) -> None: | ||||||
| """Test that terminate agents cannot have max_tokens.""" | ||||||
| with pytest.raises(ValidationError) as exc_info: | ||||||
| AgentDef(name="t", type="terminate", status="success", reason="done", max_tokens=8192) | ||||||
| assert "max_tokens" in str(exc_info.value) | ||||||
|
|
||||||
|
|
||||||
| class TestRuntimeConfig: | ||||||
| """Tests for RuntimeConfig model.""" | ||||||
|
|
||||||
|
|
||||||
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.
This validates clean on
copilot,hermes,acaandclaude-agent-sdk, and then has no effect on any of them.agent_builder.py:307is the only read in the repo and it only runs underclaude.The two fields directly above this one are honoured by every provider (
copilot:1296,1301,hermes:247,252,claude_agent_sdk:883,891,aca:763,764), so anyone readingAgentDefwould expect the same treatment here.Two ways to close it. Either wire up the remaining four providers, or declare the support explicitly and let validation refuse the rest. The second is what
session_keyandmax_session_secondsalready do:Then set
max_tokens=TrueonClaudeProviderand leave the default everywhere else. That also picks upclaude-agent-sdk, which today refusesruntime.max_tokensatfactory.py:195but lets this one through.