feat(langgraph): configurable recursion_limit for LangGraph agents#1305
feat(langgraph): configurable recursion_limit for LangGraph agents#1305opspawn wants to merge 6 commits intokagent-dev:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for configurable recursion_limit for LangGraph agents, addressing issue #1279. The recursion_limit parameter controls the maximum number of steps LangGraph will execute before raising a recursion error, which is important for complex multi-step tasks that may exceed the default 25-step limit.
Changes:
- Added
recursion_limitfield toLangGraphAgentExecutorConfigwith default value of 25 (matching LangGraph's default) - Made recursion_limit configurable via
LANGGRAPH_RECURSION_LIMITenvironment variable - Passed recursion_limit in RunnableConfig for both initial execution and resume flows
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
python/packages/kagent-langgraph/src/kagent/langgraph/_executor.py |
Added recursion_limit configuration field with env var support; applied to graph config creation |
python/packages/kagent-langgraph/tests/test_executor_config.py |
Added unit tests for default value, env var override, and explicit config override |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Maximum number of steps before LangGraph raises a recursion error. | ||
| # Configurable via LANGGRAPH_RECURSION_LIMIT env var. Default is 25 (LangGraph's default). | ||
| recursion_limit: int = int(os.getenv("LANGGRAPH_RECURSION_LIMIT", "25")) |
There was a problem hiding this comment.
The use of os.getenv() directly in the field default has a subtle issue: it's evaluated at module import time, not at instance creation time. This means that if the environment variable is changed after the module is first imported, new config instances won't see the updated value unless the module is reloaded.
Consider using Pydantic's Field(default_factory=...) pattern instead to ensure the environment variable is read at instance creation time. For example:
recursion_limit: int = Field(default_factory=lambda: int(os.getenv("LANGGRAPH_RECURSION_LIMIT", "25")))This approach is more aligned with how Pydantic handles dynamic defaults and makes the behavior more predictable in testing scenarios.
|
|
||
| # Maximum number of steps before LangGraph raises a recursion error. | ||
| # Configurable via LANGGRAPH_RECURSION_LIMIT env var. Default is 25 (LangGraph's default). | ||
| recursion_limit: int = int(os.getenv("LANGGRAPH_RECURSION_LIMIT", "25")) |
There was a problem hiding this comment.
The recursion_limit field lacks validation to prevent invalid values such as negative numbers or zero. LangGraph expects a positive integer for recursion_limit, and passing invalid values could lead to unexpected behavior or errors at runtime.
Consider adding Pydantic field validation using Field with constraints. For example:
from pydantic import BaseModel, Field
recursion_limit: int = Field(
default=25,
gt=0,
description="Maximum number of steps before LangGraph raises a recursion error"
)If environment variable configuration is needed, you can combine this with a model validator or use Field(default_factory=...) with validation logic.
| with patch.dict(os.environ, {"LANGGRAPH_RECURSION_LIMIT": "50"}): | ||
| # Re-import to pick up new env var value | ||
| import kagent.langgraph._executor as executor_mod | ||
|
|
||
| importlib.reload(executor_mod) | ||
| config = executor_mod.LangGraphAgentExecutorConfig() | ||
| assert config.recursion_limit == 50 | ||
|
|
||
| # Restore default | ||
| os.environ.pop("LANGGRAPH_RECURSION_LIMIT", None) | ||
| importlib.reload(executor_mod) | ||
|
|
||
|
|
There was a problem hiding this comment.
The test uses importlib.reload() to test environment variable behavior, but this approach has potential issues:
-
The cleanup at lines 27-28 happens outside the context manager, which means if the test fails before reaching that point, the environment variable won't be cleaned up and the module won't be reloaded.
-
Module reloading can have side effects on other tests if they run in the same process, as the reloaded module may not be in its expected state.
Consider using a more robust approach with proper cleanup via a try/finally block or pytest fixtures, or restructure the test to avoid module reloading entirely by using dependency injection or mocking the config instance creation.
| with patch.dict(os.environ, {"LANGGRAPH_RECURSION_LIMIT": "50"}): | |
| # Re-import to pick up new env var value | |
| import kagent.langgraph._executor as executor_mod | |
| importlib.reload(executor_mod) | |
| config = executor_mod.LangGraphAgentExecutorConfig() | |
| assert config.recursion_limit == 50 | |
| # Restore default | |
| os.environ.pop("LANGGRAPH_RECURSION_LIMIT", None) | |
| importlib.reload(executor_mod) | |
| import kagent.langgraph._executor as executor_mod | |
| try: | |
| with patch.dict(os.environ, {"LANGGRAPH_RECURSION_LIMIT": "50"}): | |
| # Re-import to pick up new env var value under patched environment | |
| importlib.reload(executor_mod) | |
| config = executor_mod.LangGraphAgentExecutorConfig() | |
| assert config.recursion_limit == 50 | |
| finally: | |
| # Ensure environment and module state are restored even if the test fails | |
| os.environ.pop("LANGGRAPH_RECURSION_LIMIT", None) | |
| importlib.reload(executor_mod) |
| def test_recursion_limit_explicit_override(): | ||
| """Test that explicit config value overrides env var default.""" | ||
| from kagent.langgraph._executor import LangGraphAgentExecutorConfig | ||
|
|
||
| config = LangGraphAgentExecutorConfig(recursion_limit=100) | ||
| assert config.recursion_limit == 100 |
There was a problem hiding this comment.
The test suite is missing coverage for invalid environment variable values. The current implementation will raise a ValueError if someone sets LANGGRAPH_RECURSION_LIMIT to a non-numeric value (e.g., "abc"), but this error case is not tested.
Consider adding a test case that verifies the behavior when the environment variable contains an invalid value, to ensure it either fails gracefully with a helpful error message or falls back to a sensible default.
Add a `recursion_limit` field to `LangGraphAgentExecutorConfig` that allows users to configure the maximum number of steps before LangGraph raises a recursion error. The value can be set via the `LANGGRAPH_RECURSION_LIMIT` environment variable or passed directly through the config. Defaults to 25 (LangGraph's default). The recursion_limit is passed in the graph config for both initial execution and resume flows. Closes kagent-dev#1279 Signed-off-by: opspawn <opspawn@users.noreply.github.com> Signed-off-by: OpSpawn Agent <agent@opspawn.com> Signed-off-by: opspawn <opspawn@users.noreply.github.com>
…r recursion_limit Address Copilot review feedback: - Use Field(default_factory=lambda: ...) so env var is read at instance creation rather than module import time. - Add gt=0 constraint to reject zero/negative values. - Fix test cleanup: remove importlib.reload since default_factory makes it unnecessary. Add validation tests for invalid values. Signed-off-by: opspawn <opspawn@users.noreply.github.com>
Address Copilot review feedback: add test verifying that a non-numeric LANGGRAPH_RECURSION_LIMIT env var raises an error at config creation. Signed-off-by: OpSpawn Agent <agent@opspawn.com> Signed-off-by: opspawn <opspawn@users.noreply.github.com>
413868d to
42c232a
Compare
Address remaining Copilot review feedback: - Remove unused `import importlib` (leftover from pre-default_factory version) - Make test_recursion_limit_default robust against env var pollution by explicitly clearing LANGGRAPH_RECURSION_LIMIT before asserting default Signed-off-by: opspawn <opspawn@users.noreply.github.com>
Signed-off-by: opspawn <opspawn@users.noreply.github.com>
Signed-off-by: opspawn <opspawn@users.noreply.github.com>
Summary
Adds configurable
recursion_limitfor LangGraph agents, addressing #1279.Changes
recursion_limitfield toLangGraphAgentExecutorConfig(default: 25, matching LangGraph's default)LANGGRAPH_RECURSION_LIMITenvironment variableUsage
Environment variable:
export LANGGRAPH_RECURSION_LIMIT=50Direct config:
Closes #1279