[SILO-766] feat: add Agent Runs API and related models, activities, and tests #13
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughThis PR introduces a new Agent Runs API module to the Plane SDK, including comprehensive data models for agent runs and activities, API resource classes for CRUD operations, client integration, and unit tests with test fixtures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (6)
.env (1)
2-3: Address dotenv-linter warnings: key ordering and missing trailing newline.Minor formatting issues flagged by dotenv-linter:
- Keys should be ordered alphabetically (PLANE_API_KEY before PLANE_BASE_URL)
- Add a blank line at the end of the file
+PLANE_API_KEY=plane_api_97e535f1b6dd4d90b89eae0bb1da3105 PLANE_BASE_URL=http://localhost:8000 -PLANE_API_KEY=plane_api_97e535f1b6dd4d90b89eae0bb1da3105 WORKSPACE_SLUG=oauth-enhance +plane/api/agent_runs/base.py (1)
57-73: Consider ifresumeshould support optional parameters.The
resumemethod performs a POST without a request body. If the API might later support options (e.g., resume with specific state or metadata), you may want to add an optionaldataparameter. However, if the current API contract doesn't require it, this is fine as-is.tests/unit/test_agent_runs.py (3)
41-51: Consider usingreturninstead ofyieldwhen no teardown is needed.Since there's no cleanup code after the yield (as noted in the comment), using
returnwould be more semantically appropriate. Theyieldkeyword implies teardown follows.@pytest.fixture def agent_run( self, client: PlaneClient, workspace_slug: str, agent_run_data: CreateAgentRun, ) -> AgentRun: """Create a test agent run and yield it.""" - agent_run = client.agent_runs.create(workspace_slug, agent_run_data) - yield agent_run - # Note: Agent runs typically don't have a delete endpoint + # Note: Agent runs typically don't have a delete endpoint + return client.agent_runs.create(workspace_slug, agent_run_data)
95-113: Duplicate fixtures could be consolidated at module level.
agent_run_dataandagent_runfixtures are duplicated betweenTestAgentRunsAPIandTestAgentRunActivitiesAPI. Consider moving them to module level to reduce duplication.+@pytest.fixture +def agent_run_data(agent_slug: str, project: Project) -> CreateAgentRun: + """Create test agent run data.""" + return CreateAgentRun( + agent_slug=agent_slug, + project=project.id, + type=AgentRunType.COMMENT_THREAD, + ) + + +@pytest.fixture +def agent_run( + client: PlaneClient, + workspace_slug: str, + agent_run_data: CreateAgentRun, +) -> AgentRun: + """Create a test agent run.""" + # Note: Agent runs typically don't have a delete endpoint + return client.agent_runs.create(workspace_slug, agent_run_data) + + class TestAgentRunsAPI: """Test AgentRuns API resource.""" - - @pytest.fixture - def agent_run_data(self, agent_slug: str, project: Project) -> CreateAgentRun: - ... - - @pytest.fixture - def agent_run(self, ...): - ...
124-136: Same observation:yieldwithout teardown.Similar to the
agent_runfixture, consider usingreturninstead ofyieldwhen no cleanup is performed.plane/models/agent_runs.py (1)
87-106: Consider adding explicit discriminator for robustness.The union type relies on implicit discrimination via the
typefield. While this works in Pydantic v2, adding an explicit discriminator improves clarity and error messages.+from typing import Annotated +from pydantic import Discriminator + -AgentRunActivityContent = AgentRunActivityActionContent | AgentRunActivityTextContent +AgentRunActivityContent = Annotated[ + AgentRunActivityActionContent | AgentRunActivityTextContent, + Discriminator("type"), +]Also, verify if
parameters: dict[str, str]on line 94 is intentionally restrictive, or ifdict[str, Any]would better accommodate diverse parameter values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.env(1 hunks)plane/__init__.py(2 hunks)plane/api/agent_runs/__init__.py(1 hunks)plane/api/agent_runs/activities.py(1 hunks)plane/api/agent_runs/base.py(1 hunks)plane/client/plane_client.py(2 hunks)plane/models/agent_runs.py(1 hunks)tests/unit/test_agent_runs.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
plane/client/plane_client.py (1)
plane/api/agent_runs/base.py (1)
AgentRuns(8-72)
tests/unit/test_agent_runs.py (4)
plane/client/plane_client.py (1)
PlaneClient(20-57)plane/models/agent_runs.py (6)
AgentRun(48-70)AgentRunActivity(109-127)AgentRunType(22-25)CreateAgentRun(73-84)CreateAgentRunActivity(130-140)PaginatedAgentRunActivityResponse(143-156)plane/api/agent_runs/activities.py (2)
create(64-84)retrieve(43-62)plane/api/agent_runs/base.py (3)
create(20-38)retrieve(40-55)resume(57-72)
plane/__init__.py (1)
plane/api/agent_runs/base.py (1)
AgentRuns(8-72)
plane/models/agent_runs.py (1)
tests/unit/test_agent_runs.py (3)
agent_slug(21-26)agent_run(42-51)agent_run(105-113)
plane/api/agent_runs/__init__.py (1)
plane/api/agent_runs/base.py (1)
AgentRuns(8-72)
🪛 dotenv-linter (4.0.0)
.env
[warning] 2-2: [UnorderedKey] The PLANE_API_KEY key should go before the PLANE_BASE_URL key
(UnorderedKey)
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (23)
plane/api/agent_runs/__init__.py (1)
1-4: LGTM!Clean package initialization that correctly re-exports
AgentRunsfrom the base module. TheAgentRunActivitiessub-resource is appropriately accessed throughAgentRuns.activitiesrather than being exported separately.plane/client/plane_client.py (2)
1-1: LGTM!Import correctly added for the new
AgentRunsresource.
57-57: LGTM!Resource initialization follows the established pattern used by other API resources in the client.
plane/__init__.py (2)
1-1: LGTM!Import correctly added to expose
AgentRunsat the package level.
28-28: LGTM!Public export added to
__all__, enabling direct imports likefrom plane import AgentRuns.plane/api/agent_runs/activities.py (4)
1-19: LGTM!Clean imports and class setup. The
AgentRunActivitiesresource follows the established pattern for API resources in this SDK.
21-41: LGTM!The
listmethod correctly handles optional pagination parameters and validates the response into a paginated model.
43-62: LGTM!The
retrievemethod follows the standard pattern for fetching a single resource by ID.
64-85: LGTM!The
createmethod correctly serializes the input model withexclude_none=Trueand validates the response.plane/api/agent_runs/base.py (3)
1-18: LGTM!Clean imports and class setup. The sub-resource pattern with
self.activitiesis a good approach for nested API resources.
20-38: LGTM!The
createmethod correctly serializes the input model and validates the API response.
40-55: LGTM!Standard retrieval pattern for fetching an agent run by ID.
tests/unit/test_agent_runs.py (4)
1-17: LGTM!Imports are well-organized and include all necessary dependencies for the test module.
20-26: LGTM!Fixture correctly handles missing environment variable with skip behavior and appropriate module scope.
53-89: LGTM!Tests cover create, retrieve, and resume operations with appropriate assertions on key fields.
138-196: LGTM!Activity tests provide good coverage of list, paginated list, create, and retrieve operations with appropriate assertions.
plane/models/agent_runs.py (7)
1-6: LGTM!Imports are clean and include all necessary dependencies for the model definitions.
9-46: LGTM!Enums are well-defined with
strinheritance for JSON serialization compatibility.
48-71: LGTM!Model is well-structured with appropriate
ConfigDictsettings for a response model. Theextra="allow"configuration handles potential additional fields from the API gracefully.
73-85: LGTM!Request model appropriately uses
extra="ignore"to prevent unexpected fields from being sent to the API.
109-128: LGTM!Activity model is well-structured with proper union type for content and enum usage for signal and type fields.
143-157: LGTM!Pagination model is well-structured with support for both cursor-based and page-based pagination patterns.
130-141: No issue—this is intentional model design. The exclusion of"prompt"fromCreateAgentRunActivity.typeis by design:CreateAgentRunActivitydefines user-creatable activities (action, elicitation, error, response, thought), whileAgentRunActivityTextContentdefines system-generated responses (elicitation, error, prompt, response, thought). Prompts are generated by the server, not created by users, so this separation is correct.
.env
Outdated
| @@ -0,0 +1,3 @@ | |||
| PLANE_BASE_URL=http://localhost:8000 | |||
| PLANE_API_KEY=plane_api_97e535f1b6dd4d90b89eae0bb1da3105 | |||
There was a problem hiding this comment.
Remove hardcoded API key; use environment-specific or example files instead.
The PLANE_API_KEY contains what appears to be a real or example secret that should never be committed to version control. Secrets should be kept in a .env.local file (excluded via .gitignore) or provided at runtime via CI/CD secrets management.
Recommended approaches:
-
Create a
.env.examplewith placeholder values:PLANE_BASE_URL=http://localhost:8000 PLANE_API_KEY=your_api_key_here WORKSPACE_SLUG=oauth-enhance
-
Add
.envto.gitignoreif not already present. -
Ensure developers use
.env.localor load secrets from CI/CD environment.
If this is a test/demo key, please confirm it is not associated with any production credentials.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [UnorderedKey] The PLANE_API_KEY key should go before the PLANE_BASE_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env around line 2 the PLANE_API_KEY is hardcoded which exposes secrets in
version control; remove the key from this file and replace it with a placeholder
or delete the entry, add .env to .gitignore (if not already), create a
.env.example with a descriptive placeholder (e.g.
PLANE_API_KEY=your_plane_api_key_here) for onboarding, and document in the
README that developers should put real secrets into a local-only file (e.g.
.env.local) or supply them via CI/CD secret management.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_agent_runs.py (1)
18-24: Remove unused fixture.The
agent_slugfixture is defined but never used in any test. Theagent_runfixture fromconftest.py(lines 82-84) already handles theAGENT_SLUGenvironment variable check, making this fixture redundant.🔎 Apply this diff to remove the unused fixture:
-@pytest.fixture(scope="module") -def agent_slug() -> str: - """Get agent slug from environment variable.""" - slug = os.getenv("AGENT_SLUG") - if not slug: - pytest.skip("AGENT_SLUG environment variable not set") - return slug - - class TestAgentRunsAPI:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)plane/api/agent_runs/base.py(1 hunks)tests/unit/conftest.py(2 hunks)tests/unit/test_agent_runs.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🧬 Code graph analysis (3)
plane/api/agent_runs/base.py (2)
plane/models/agent_runs.py (2)
AgentRun(48-70)CreateAgentRun(73-84)plane/api/agent_runs/activities.py (3)
AgentRunActivities(12-84)create(64-84)retrieve(43-62)
tests/unit/conftest.py (4)
plane/models/agent_runs.py (2)
AgentRun(48-70)CreateAgentRun(73-84)plane/client/plane_client.py (1)
PlaneClient(20-57)tests/unit/test_agent_runs.py (1)
agent_slug(19-24)plane/api/agent_runs/base.py (1)
create(20-38)
tests/unit/test_agent_runs.py (5)
tests/unit/conftest.py (4)
client(45-54)workspace_slug(36-41)agent_run(81-105)project(65-77)plane/client/plane_client.py (1)
PlaneClient(20-57)plane/models/agent_runs.py (4)
AgentRun(48-70)AgentRunActivity(109-127)CreateAgentRunActivity(130-140)PaginatedAgentRunActivityResponse(143-156)plane/api/agent_runs/base.py (2)
retrieve(40-55)create(20-38)plane/api/agent_runs/activities.py (2)
retrieve(43-62)create(64-84)
🔇 Additional comments (7)
plane/api/agent_runs/base.py (3)
1-12: LGTM!The imports and class structure follow the established patterns in the codebase. The class correctly inherits from
BaseResourceand has clear documentation.
14-38: LGTM!The initialization and create method follow best practices:
- Proper initialization of the sub-resource for nested activities
- Correct use of
model_dump(exclude_none=True)for request serialization- Appropriate response validation with
model_validate
40-55: LGTM!The retrieve method is correctly implemented with proper URL construction and response validation.
tests/unit/conftest.py (1)
9-11: LGTM!The new imports are necessary for the
agent_runfixture and are correctly specified.tests/unit/test_agent_runs.py (3)
1-15: LGTM!The module docstring clearly describes the test purpose, and all imports are relevant to the test implementation.
27-41: LGTM!The test correctly validates the retrieve operation, checking essential fields (id, workspace, status) to ensure the API returns expected data.
44-122: LGTM! Tests provide good coverage of activity operations.The test class comprehensively covers the agent run activities API:
- Listing activities with and without pagination
- Creating activities
- Retrieving specific activities
The assertions appropriately validate response types and key fields. Note that the
activityfixture (lines 56-66) doesn't include cleanup logic, similar to theagent_runfixture in conftest.py—verify if cleanup is needed.
| @pytest.fixture(scope="session") | ||
| def agent_run(client: PlaneClient, workspace_slug: str, project: Project) -> AgentRun: | ||
| agent_slug = os.getenv("AGENT_SLUG") | ||
| if not agent_slug: | ||
| pytest.skip("AGENT_SLUG environment variable not set") | ||
| """Create a test agent run and yield it.""" | ||
| work_item = client.work_items.create( | ||
| workspace_slug, | ||
| project.id, | ||
| CreateWorkItem(name="Test Work Item"), | ||
| ) | ||
| comment = client.work_items.comments.create( | ||
| workspace_slug, | ||
| project.id, | ||
| work_item.id, | ||
| CreateWorkItemComment(comment_html="<p>This is a test comment</p>"), | ||
| ) | ||
| agent_run = client.agent_runs.create( | ||
| workspace_slug, | ||
| CreateAgentRun( | ||
| agent_slug=agent_slug, | ||
| project=project.id, | ||
| comment=comment.id, | ||
| ), | ||
| ) | ||
| yield agent_run |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the conftest.py file and examine the agent_run fixture
fd -t f "conftest.py" tests/unit/
# Check if the file exists and view the agent_run fixture
if [ -f "tests/unit/conftest.py" ]; then
echo "=== Lines 80-105 of tests/unit/conftest.py ==="
sed -n '80,105p' tests/unit/conftest.py
echo ""
echo "=== Lines 65-77 of tests/unit/conftest.py (project fixture for comparison) ==="
sed -n '65,77p' tests/unit/conftest.py
fiRepository: makeplane/plane-python-sdk
Length of output: 1599
🏁 Script executed:
#!/bin/bash
# Search for AgentRuns delete method in the codebase
rg -n "def delete" --type py | grep -i agent || echo "No delete method found for agent_runs"
# Also search for the AgentRuns class definition
fd -t f "*agent_run*.py" plane/api/ 2>/dev/null || echo "Agent runs module not found"
# Search more broadly for agent_runs module
rg -l "class AgentRuns" --type pyRepository: makeplane/plane-python-sdk
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Examine the agent_runs base module to see if delete method exists
cat plane/api/agent_runs/base.pyRepository: makeplane/plane-python-sdk
Length of output: 1461
🏁 Script executed:
#!/bin/bash
# Also search for any delete or cleanup references in agent_runs related files
rg -n "delete|cleanup" plane/api/agent_runs/ -A 2Repository: makeplane/plane-python-sdk
Length of output: 52
Move docstring to immediately follow the function signature.
The docstring on line 85 appears after the skip logic instead of immediately following the function definition on line 81. Move it to line 82 to follow Python convention.
Unlike the project fixture, cleanup is not feasible for agent runs because the AgentRuns API does not expose a delete method—only create() and retrieve() are available.
🤖 Prompt for AI Agents
In tests/unit/conftest.py around lines 80 to 105, the fixture's docstring is
placed after the skip logic; move the triple-quoted docstring so it immediately
follows the function signature on line 82 (directly under def agent_run(...):)
and keep the existing skip logic and creation code unchanged; ensure the
docstring is correctly indented to match the function body and no cleanup code
is added since AgentRuns cannot be deleted.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.